Skip to content

fix(telemetry): never identify the anonymous installation_id (restore login stitch)#1114

Merged
deepme987 merged 3 commits into
mainfrom
deepme987/desktop/fix-identity-stitch
Jun 16, 2026
Merged

fix(telemetry): never identify the anonymous installation_id (restore login stitch)#1114
deepme987 merged 3 commits into
mainfrom
deepme987/desktop/fix-identity-stitch

Conversation

@deepme987

Copy link
Copy Markdown
Collaborator

The bug

Anonymous desktop activity never stitches onto the account at login: 0 of 13,528 device ids ever stitched (per James's PostHog audit). On sign-in, the user keeps a fresh identity and all pre-login device history (sessions, installs, first-use, generations) is orphaned under the anonymous installation_id.

Root cause

PostHog marks any id passed to identify() as an identified person, and it will not merge one identified id into another. We were calling client.identify() with the anonymous installation_id in four places, which "burned" it:

  1. Boot (index.ts): identify(installationId, {app_version, platform, arch, id_class}) fired a $identify on the anon id on every consented launch, before any login.
  2. Logout (unbindUserId): re-identify()d the anon id with {is_authenticated:false}, re-burning it for the next login.
  3. registerPersonProperties and registerPersonPropertiesOnce: any anonymous person-property write (e.g. first_generation_at, set on first generation which usually precedes login) emitted a $identify.

With the anon id identified, the login-time alias(installation_id → user_id) in bindUserId is a silent no-op. That is the entire bug.

The fix — one invariant

client.identify() is called with exactly one kind of id: the authenticated Firebase user_id, only inside bindUserId. The anonymous installation_id is never the distinctId of an identify() call.

All anonymous person-property writes ($set / $set_once) now route through a new capturePersonProperties() helper that fires them as a $set/$set_once on a dedicated low-volume comfy.desktop.person.set capture event. In posthog-node, $set on a captured event updates the person profile without emitting $identify, so the id stays anonymous and the login alias merges. The same path is correct post-login (the distinct id is then the already-identified uid).

Sites fixed: boot bind, unbindUserId, registerPersonProperties, registerPersonPropertiesOnce, and the deferred-flush path. bindUserId's alias + single identify(user_id) is unchanged and now actually merges.

Scope / companion work

This fixes the desktop shell's own posthog-node events (the installation_id stream). Desktop also ships desktop_device_id on cloud URLs (cloudUrl.ts) for a frontend-side stitch of the cloud-webview stream, but the embedded frontend has no telemetry pipeline in desktop builds yet — see #1069, which would relay frontend logs through desktop. A companion ComfyUI_frontend change fires identify(uid) with desktop_device_id for that stream. Neither stream stitched while the anon id was burned; this PR unblocks the desktop one.

Validation

  • telemetry.test.ts + experiments.test.ts: 72 passing. New assertions verify boot/logout/person-prop writes emit no $identify on the anon id, and bindUserId still emits the alias + a single identify(uid) + app:user_logged_in.
  • Full typecheck (node/web/e2e/integration) and eslint: green.

… login stitch)

PostHog marks any id passed to identify() as an identified person and
refuses to merge one identified id into another. Calling identify() with
the anonymous installation_id at boot (and on logout, and on every
anonymous person-property write) burned it, so the login-time
alias(installation_id -> user_id) was a silent no-op: 0 of 13,528 device
ids ever stitched onto an account.

Invariant: client.identify() is only ever called with the authenticated
Firebase user_id, in bindUserId. All anonymous person-property writes
($set / $set_once) now route through a capture-$set on a dedicated
comfy.desktop.person.set event, which updates the person without emitting
$identify. Fixes the boot bind, unbindUserId (logout), and both
registerPersonProperties paths.
@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 43f444c0-7f02-4bf9-8c65-5015cb29227d

📥 Commits

Reviewing files that changed from the base of the PR and between 336a1e4 and c7b361a.

📒 Files selected for processing (3)
  • src/main/index.ts
  • src/main/lib/telemetry.test.ts
  • src/main/lib/telemetry.ts

📝 Walkthrough

Walkthrough

Telemetry person-property writes are refactored to avoid calling client.identify() on the anonymous distinct id. A new capturePersonProperties(set, setOnce) helper emits a dedicated comfy.desktop.person.set capture event instead. Internal deferred state fields, flush logic, identify(), unbindUserId(), registerPersonProperties/Once(), startup comments, and tests are all updated accordingly.

Changes

Capture-set refactor for anonymous person properties

Layer / File(s) Summary
Identity model docs and startup comment
src/main/lib/telemetry.ts, src/main/index.ts
Comment block in telemetry.ts documents the anonymous distinct-id invariant and aliasing lifecycle; startup comment in index.ts notes capture-set semantics replace the prior identify path.
Pending state rename and capturePersonProperties helper
src/main/lib/telemetry.ts
Renames deferred fields from pendingIdentifyProperties* to pendingPersonSet*/pendingPersonSetOnce*, resets them in _resetForTest(), introduces consent-gated capturePersonProperties(set, setOnce) that scrubs and emits comfy.desktop.person.set via capture, and updates tryFlushDeferred() to flush through the new helper.
identify() and unbindUserId() call-site updates
src/main/lib/telemetry.ts
identify() queues provided properties into pendingPersonSet for capture-$set instead of pendingIdentifyProperties; unbindUserId() calls capturePersonProperties for the is_authenticated: false flip instead of client.identify().
registerPersonProperties and registerPersonPropertiesOnce rewrites
src/main/lib/telemetry.ts
Both methods queue into pendingPersonSet/pendingPersonSetOnce pre-consent and flush via capturePersonProperties on grant, replacing the identify-based $set/$set_once path.
Test updates for capture-set and no-identify assertions
src/main/lib/telemetry.test.ts
Tests now assert merged property writes land in a single comfy.desktop.person.set capture on consent grant, $set_once is never sent via identify(), and unbindUserId flips is_authenticated through capture rather than identify.

Possibly related PRs

  • Comfy-Org/Comfy-Desktop#1080: Modifies registerPersonPropertiesOnce consent and $set_once queuing behavior in the same telemetry.ts module — directly overlapping code paths with this PR's capture-set refactor (it's a pun-derful coincidence they rhyme!).

Suggested reviewers

  • Kosinkadink
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch deepme987/desktop/fix-identity-stitch
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch deepme987/desktop/fix-identity-stitch

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install timed out. The project may have too many dependencies for the sandbox.


Comment @coderabbitai help to get the list of available commands and usage tips.

Trim the verbose docstrings added with the stitch fix to the load-bearing
points, and correct a header reference to a non-existent
setAnonymousDistinctId() (the function is identify()).
@deepme987 deepme987 marked this pull request as ready for review June 16, 2026 00:09
@deepme987 deepme987 merged commit d74e912 into main Jun 16, 2026
12 checks passed
@deepme987 deepme987 deleted the deepme987/desktop/fix-identity-stitch branch June 16, 2026 00:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants