[GTM-277] Carry website anonymous ID through Desktop auth

[benceruleanlu]

Summary

Carries the website's anonymous PostHog identity into a fresh Windows Desktop install, keeps every pre-auth Desktop event anonymous, and identifies the Firebase UID only after all live hosted Cloud views agree on the user.

Linear: GTM-277 · Parent: GTM-93

Identity contract

• The website's PostHog cookie $device_id is W.
• comfy-router #34 returns the installer as Comfy-Desktop-Setup-phid1_<W>.exe, carrying W verbatim; the Router only emits the carrier when W is a 36-character lowercase hyphenated UUID (posthog-js's $device_id format, version/variant nibbles not pinned).
• The Windows installer extracts the fixed 36-character payload after the 26-character prefix and ignores everything after it, so browser duplicate-download names (… (1).exe) keep their carrier. Before any telemetry capture, Desktop validates the exact lowercase-UUID shape, then durably seeds its anonymous-ID store with exact W.
• A persisted Desktop anonymous ID always wins; a later installer cannot silently replace an existing active D. Missing, malformed, or unpersistable carriers fall back to persisted/generated random D.
• Every pre-auth capture uses W or D with $process_person_profile: false. Pre-auth person properties remain in memory.
• The sole PostHog identify path is identify(F, { $anon_distinct_id: W_or_D, $set, $set_once }), and it runs only after auth consensus.
• Desktop durably reserves a fresh D before identify, then adopts it on logout, account switch, or the next process.

Auth consensus

• ComfyUI_frontend #13687 reports declarative pending, signed_out, or signed_in(F) state through the optional reportFirebaseAuthState Desktop bridge method.
• Desktop accepts reports only from an attached, trusted Cloud main frame and ties each report to the exact committed renderer process/routing IDs. Unloading, detached, subframe, untrusted-origin, and stale-document IPC cannot affect identity.
• Every live trusted reporter must resolve to the same Firebase UID before Desktop identifies. Any pending reporter keeps the process anonymous; all signed-out reporters keep it anonymous.
• Mixed signed-in/signed-out state or different Firebase UIDs quarantine the current anonymous epoch. The quarantine is durable, and Desktop rotates to a clean D before any later identify so conflicting users can never merge through the same anonymous ID.
• Navigation commits and canceled provisional loads preserve the committed-frame gate without waiting for resource loading to finish. A committed document can report while isLoadingMainFrame() is still true.
• Public imperative bindUserId / unbindUserId bridge methods are removed. Deprecated main-process compatibility exports are no-ops, so Desktop auth #1222 cannot bypass consensus during integration.

Integration with Desktop auth #1222

The current #1272 and #1222 heads have one expected textual conflict in src/main/auth/firebaseBridge/index.ts: #1222 refactors the login flow while this PR removes the old native imperative identity bind. Rebase #1272 after #1222 and resolve that line by keeping authentication/injection from #1222 while leaving flowShared.bindSignedInUser routed only to the deprecated no-op compatibility hook. The synthetic resolved stack passes typecheck and 185 focused tests; only applyFirebaseUserConsensus may identify.

Removed

• Deletes the 12-character download-token parser, pending-token file contract, attribution guard, download_token event properties, and comfy.desktop.identity.download_attributed event.
• Removes every Desktop PostHog alias path, including installer and legacy installation-ID migration aliases.
• Keeps installation_id as an event/person property only.
• Adds no Cloud endpoint, redemption call, canonical-ID lookup, Redis, or other storage dependency.

Review order

1. Review this PR first: it defines the installer carrier consumer, trusted bridge contract, and process-wide identity policy.
2. Review ComfyUI_frontend #13687: it is the declarative Firebase-state producer.
3. Review comfy-router #34: it is the website-cookie-to-installer carrier producer.

Deployment order

1. Deploy ComfyUI_frontend #13687. Its optional bridge call is a no-op on older Desktop builds.
2. Release this Desktop PR.
3. Deploy comfy-router fix: whitelist electron/esbuild postinstall scripts for pnpm #34 so newly downloaded installers begin carrying W.

Older Desktop releases safely ignore the new filename suffix, but cannot complete the deterministic website-to-Desktop join.

Verification

• pnpm run typecheck
• pnpm run lint
• pnpm run build
• pnpm run test — 198 files passed; 2,978 tests passed, 1 skipped
• focused Firebase identity coordinator and telemetry IPC suites — 32 tests passed
• git diff --check

No user-facing UI changes.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Changes

Telemetry identity and Firebase consensus

Layer / File(s)	Summary
Anonymous identity storage and website carriers src/main/lib/opaqueIdentifier.ts, src/main/lib/anonymousIdentity.ts, src/main/lib/websiteAnonymousIdentity.ts, scripts/installer.nsh, src/main/lib/*test.ts	Anonymous IDs and website-carrier payloads are validated, persisted, rotated, and tested, including failure and unmergeable-epoch handling.
Anonymous telemetry lifecycle src/main/lib/telemetry.ts, src/main/lib/telemetry.test.ts, src/main/lib/experiments*, src/main/lib/cloudCapacity.ts	Telemetry binds anonymous IDs, defers person writes until Firebase binding, applies person-processing policy, retains dropped one-shot events, and disables implicit feature-flag events.
Startup migration and legacy cleanup src/main/index.ts, src/main/lib/deviceId.ts, src/main/lib/deviceId.test.ts, src/main/auth/firebaseBridge/index.ts	Startup initializes anonymous telemetry, clears legacy retry state, and completes local identity migration without Desktop aliasing.
Firebase auth consensus tracking src/main/lib/firebaseAuthIdentity.ts, src/main/lib/firebaseAuthIdentity.test.ts, src/main/host/attach.ts, src/main/host/createHostWindow.ts	Hosted WebContents reporters track trusted navigation and reconcile Firebase auth states across active views, including navigation failures and durable epoch conflicts.
Trusted telemetry bridge contract packages/comfyui-desktop-bridge-types/*, src/types/comfyDesktopBridge.ts, src/main/lib/trustedCloudUrl.ts, src/main/lib/ipc/*, src/preload/*, src/types/ipc.ts, src/renderer/src/lib/rendererBootstrap.ts	The bridge adds typed Firebase auth reporting, validates trusted Cloud main-frame payloads, centralizes telemetry IPC sends, and removes legacy bind/unbind methods.
Cloud URL identifier cleanup src/main/lib/cloudUrl.ts, src/main/lib/cloudUrl.test.ts, src/main/lib/ipc/sessionActions/launch.ts	Cloud URLs retain UTM handling while removing Desktop device and installation identifiers.

Sequence Diagram(s)

sequenceDiagram
  participant HostedView
  participant PreloadBridge
  participant TelemetryIPC
  participant FirebaseAuthIdentity
  participant MainTelemetry
  HostedView->>PreloadBridge: reportFirebaseAuthState(state)
  PreloadBridge->>TelemetryIPC: send telemetry:firebaseAuthState
  TelemetryIPC->>FirebaseAuthIdentity: validate trusted frame and auth state
  FirebaseAuthIdentity->>MainTelemetry: apply Firebase user or anonymous consensus

Loading

Possibly related PRs

• Comfy-Org/Comfy-Desktop#1271: Also changes exported telemetry capture behavior and renderer forwarding around the telemetry.ts refactor.

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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch agent/gtm-277-web-anon-identity

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch agent/gtm-277-web-anon-identity

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/main/lib/cloudUrl.ts`:
- Around line 34-36: Replace the historical explanation above the cloud URL
handling with a concise present-tense description of the behavior: remove both
the legacy identity name and property name when supplied by stale callers. Keep
the implementation unchanged.

In `@src/main/lib/telemetry.ts`:
- Around line 611-628: Update the deferred handling around
capturePersonProperties() and capture() so each returns an admission result
indicating whether the write was accepted. Clear
pendingPersonSet/pendingPersonSetOnce only after capturePersonProperties()
succeeds; clear pendingDownloadTokenAttribution and invoke
attribution.onAttributed() only after capture() succeeds, otherwise retain the
deferred state for retry.
- Around line 802-808: Update unbindUserId() to clear pendingPersonSet and
pendingPersonSetOnce before checking boundUserId, ensuring deferred-login logout
cannot retain values for the next UID. Preserve the existing early return after
clearing both buffers, and add coverage for logout while UID binding is
deferred.

In `@src/preload/comfyPreload.ts`:
- Around line 87-100: Extract the duplicated ipcRenderer.send try/catch into a
shared safeTelemetrySend helper near the bridge or Telemetry declaration,
preserving the existing ignored-error behavior. Update bindUserId and
unbindUserId to call the helper, and reuse it for capture if it has the same
telemetry-send pattern.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 11f8f461-05fe-438d-8bd0-ee5b679b6760

📥 Commits

Reviewing files that changed from the base of the PR and between bd1e234 and 47544b2.

📒 Files selected for processing (22)

• packages/comfyui-desktop-bridge-types/comfyDesktopBridge.d.ts
• packages/comfyui-desktop-bridge-types/package.json
• src/main/index.ts
• src/main/lib/anonymousIdentity.test.ts
• src/main/lib/anonymousIdentity.ts
• src/main/lib/cloudUrl.test.ts
• src/main/lib/cloudUrl.ts
• src/main/lib/deviceId.test.ts
• src/main/lib/deviceId.ts
• src/main/lib/experiments.test.ts
• src/main/lib/ipc/registerTelemetryHandlers.test.ts
• src/main/lib/ipc/registerTelemetryHandlers.ts
• src/main/lib/ipc/sessionActions/launch.ts
• src/main/lib/opaqueIdentifier.ts
• src/main/lib/telemetry.test.ts
• src/main/lib/telemetry.ts
• src/main/lib/trustedCloudUrl.test.ts
• src/main/lib/trustedCloudUrl.ts
• src/preload/api.ts
• src/preload/comfyPreload.ts
• src/types/comfyDesktopBridge.ts
• src/types/ipc.ts

💤 Files with no reviewable changes (1)

• src/preload/api.ts

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 47544b25df

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[benceruleanlu]

Took over per GTM-93. Three additive commits on top of 02ace26, all red-green tested (144 tests green locally, typecheck+eslint via hooks):

• b985891 — deferred one-shot payloads (session.started, first_launch, queued person props) are retained when a capture is dropped by the SDK or volume guards, instead of being cleared and lost; capture() now reports admission.
• 91d48e9 — PostHog-illegal distinct IDs ('undefined', 'anonymous', '[object Object]', '0', whitespace, …) are rejected at carrier decode and persisted-ID read/write. An illegal W would pool unrelated installs onto one shared distinct_id and PostHog would silently refuse the $anon_distinct_id merge at identify, stranding pre-login history (prod already has a 1.1M-event/7d blackhole on the literal 'anonymous').
• 8b05b95 — the Firebase auth consensus settles did-fail-load terminals (dedup'd against provisional failures) and compares reporter frames by processId/routingId. Previously a failed main-frame navigation left the reporter stuck pending — blocking the user consensus that triggers the anonymous→Firebase identify — and an equivalent main-frame wrapper object was rejected as a different frame.

Also verified at this head: buffer clearing on signed-out consensus (cross-account leak), experiments/flags keyed to installationId with no person creation, and the full identity contract (no aliases, personless pre-login, reserve-before-merge rotation) — no violations found.

Flagged, not fixed: inbound UID validation (registerTelemetryHandlers → normalizeOpaqueIdentifier) doesn't apply the illegal-ID check to F itself. Low risk (trusted cloud frames only, Firebase UIDs are alphanumeric) — fine as a follow-up.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8b05b95234

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[coderabbitai]

Actionable comments posted: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/main/lib/telemetry.ts (1)

847-869: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Do not consume volume-guard budget for rejected SDK writes.

The rate-limit stamp and session count are committed before client.capture() succeeds, so repeated SDK failures can leave retained one-shots permanently capped—quite the retry trap. Commit or roll back guard accounting on failure.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/lib/telemetry.ts` around lines 847 - 869, The capture function
currently commits rate-limit and session-count accounting before client.capture
succeeds. Move the _eventsCapturedThisProcess increment and corresponding
_checkRateLimit commitment into the successful SDK-write path, or explicitly
roll both back in the catch block, so rejected writes do not consume
volume-guard budget while preserving existing early rejection behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/installer.nsh`:
- Around line 263-267: Update the pending-website-anonymous-id carrier handling
around SetShellVarContext current so elevated /ALLUSERS installs do not read or
delete the file in the administrator’s profile. Persist the carrier through the
unelevated interactive user context, or skip this carrier path for that install
mode, while preserving cleanup for the intended user profile.

In `@src/main/index.ts`:
- Around line 1416-1418: Update the comments at src/main/index.ts:1416-1418 to
state that startup removes the compatibility marker regardless of guard state;
at src/main/lib/deviceId.ts:90-92 to describe the marker as compatibility-only
and cleared at boot; at src/main/lib/deviceId.ts:170-172 to simply describe the
helper as removing the compatibility marker; and at
src/main/lib/deviceId.ts:208-216 to explain marker precedence and deferred
cleanup without release-history references.

In `@src/main/lib/cloudUrl.test.ts`:
- Line 40: In the test covering withCloudDistributionUtm, extract the repeated
“not a url” literal into an input variable and use that variable both as the
function argument and expected assertion value, matching the adjacent non-cloud
URLs test structure.

In `@src/main/lib/firebaseAuthIdentity.ts`:
- Around line 142-148: Update the anonymous conflict handling around
anonymousEpochIsUnmergeable and markAnonymousEpochUnmergeable in
src/main/lib/firebaseAuthIdentity.ts:142-148 to fail closed when persistence
fails by durably rotating the epoch or suspending capture/binding; do not rely
on the in-memory taint surviving a restart. Add coverage in
src/main/lib/firebaseAuthIdentity.test.ts:530-547 that simulates restart after
the marker write failure and verifies the prior epoch cannot bind.
- Around line 152-156: Update the signed-in user flow around
applyFirebaseUserConsensus to validate userId with the existing PostHog
illegal-ID guard before assigning requestedUserId or binding consensus. Return
without binding when the guard rejects the UID, while preserving the current
clearUnmergeableEpoch behavior for valid IDs.

In `@src/main/lib/ipc/registerTelemetryHandlers.ts`:
- Around line 46-48: The signed-in telemetry normalization path must reject
Firebase UIDs that match the existing illegal PostHog distinct-ID validator. In
src/main/lib/ipc/registerTelemetryHandlers.ts lines 46-48, apply that validator
after normalizeOpaqueIdentifier and return null for matching values; in
src/main/lib/ipc/registerTelemetryHandlers.test.ts lines 204-223, add coverage
asserting every reserved value is rejected in a signed-in payload.

In `@src/main/lib/telemetry.ts`:
- Around line 635-637: Update applyFirebaseUserConsensus to reject Firebase UIDs
flagged by isIllegalPostHogDistinctId() before rotating the anonymous ID, while
retaining the existing normalizeOpaqueIdentifier validation. Add coverage for
illegal values such as “anonymous” and “0”, ensuring they leave the current
distinct ID unchanged.

In `@src/renderer/src/lib/rendererBootstrap.ts`:
- Line 386: Remove the orphaned “Person-property upserts…” comment from the
callback near getDeviceId(), or move it beside the actual
registerTelemetryProperties calls. Ensure the comment no longer implies that
this callback performs person-property upserts.
- Around line 375-376: Reword the comment near registerCohortContext to
distinguish its unconditional registration from conditional Datadog effects:
clarify that Datadog context writes and setUser only apply when
isDatadogInitialized is true, while PostHog person properties continue routing
through main.

---

Outside diff comments:
In `@src/main/lib/telemetry.ts`:
- Around line 847-869: The capture function currently commits rate-limit and
session-count accounting before client.capture succeeds. Move the
_eventsCapturedThisProcess increment and corresponding _checkRateLimit
commitment into the successful SDK-write path, or explicitly roll both back in
the catch block, so rejected writes do not consume volume-guard budget while
preserving existing early rejection behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a98f8659-2c8f-4280-a725-fd93812bd7a2

📥 Commits

Reviewing files that changed from the base of the PR and between 47544b2 and 8b05b95.

📒 Files selected for processing (36)

• packages/comfyui-desktop-bridge-types/comfyDesktopBridge.d.ts
• packages/comfyui-desktop-bridge-types/package.json
• scripts/installer.nsh
• src/main/auth/firebaseBridge/index.ts
• src/main/host/attach.ts
• src/main/host/createHostWindow.ts
• src/main/index.ts
• src/main/lib/anonymousIdentity.test.ts
• src/main/lib/anonymousIdentity.ts
• src/main/lib/cloudCapacity.ts
• src/main/lib/cloudUrl.test.ts
• src/main/lib/cloudUrl.ts
• src/main/lib/deviceId.test.ts
• src/main/lib/deviceId.ts
• src/main/lib/downloadAttribution.test.ts
• src/main/lib/downloadAttribution.ts
• src/main/lib/experiments.test.ts
• src/main/lib/experiments.ts
• src/main/lib/firebaseAuthIdentity.test.ts
• src/main/lib/firebaseAuthIdentity.ts
• src/main/lib/ipc/registerTelemetryHandlers.test.ts
• src/main/lib/ipc/registerTelemetryHandlers.ts
• src/main/lib/ipc/sessionActions/launch.ts
• src/main/lib/opaqueIdentifier.ts
• src/main/lib/telemetry.test.ts
• src/main/lib/telemetry.ts
• src/main/lib/trustedCloudUrl.test.ts
• src/main/lib/trustedCloudUrl.ts
• src/main/lib/userTier.test.ts
• src/main/lib/websiteAnonymousIdentity.test.ts
• src/main/lib/websiteAnonymousIdentity.ts
• src/preload/api.ts
• src/preload/comfyPreload.ts
• src/renderer/src/lib/rendererBootstrap.ts
• src/types/comfyDesktopBridge.ts
• src/types/ipc.ts

💤 Files with no reviewable changes (3)

• src/main/lib/downloadAttribution.test.ts
• src/main/lib/downloadAttribution.ts
• src/preload/api.ts

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/main/lib/telemetry.ts`:
- Around line 1045-1046: The privacy claims in telemetry.ts at lines 1045-1046
and cloudCapacity.ts at lines 7-8 should be scoped to the evaluation request
payload: state that the payload contains only the installation-stable evaluation
key and flag key, without claiming those are the only data reaching the
endpoint.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 078c2dd1-32e0-4105-9635-5ccbe48ca2aa

📥 Commits

Reviewing files that changed from the base of the PR and between 8b05b95 and 7215d66.

📒 Files selected for processing (36)

• packages/comfyui-desktop-bridge-types/comfyDesktopBridge.d.ts
• packages/comfyui-desktop-bridge-types/package.json
• scripts/installer.nsh
• src/main/auth/firebaseBridge/index.ts
• src/main/host/attach.ts
• src/main/host/createHostWindow.ts
• src/main/index.ts
• src/main/lib/anonymousIdentity.test.ts
• src/main/lib/anonymousIdentity.ts
• src/main/lib/cloudCapacity.ts
• src/main/lib/cloudUrl.test.ts
• src/main/lib/cloudUrl.ts
• src/main/lib/deviceId.test.ts
• src/main/lib/deviceId.ts
• src/main/lib/downloadAttribution.test.ts
• src/main/lib/downloadAttribution.ts
• src/main/lib/experiments.test.ts
• src/main/lib/experiments.ts
• src/main/lib/firebaseAuthIdentity.test.ts
• src/main/lib/firebaseAuthIdentity.ts
• src/main/lib/ipc/registerTelemetryHandlers.test.ts
• src/main/lib/ipc/registerTelemetryHandlers.ts
• src/main/lib/ipc/sessionActions/launch.ts
• src/main/lib/opaqueIdentifier.ts
• src/main/lib/telemetry.test.ts
• src/main/lib/telemetry.ts
• src/main/lib/trustedCloudUrl.test.ts
• src/main/lib/trustedCloudUrl.ts
• src/main/lib/userTier.test.ts
• src/main/lib/websiteAnonymousIdentity.test.ts
• src/main/lib/websiteAnonymousIdentity.ts
• src/preload/api.ts
• src/preload/comfyPreload.ts
• src/renderer/src/lib/rendererBootstrap.ts
• src/types/comfyDesktopBridge.ts
• src/types/ipc.ts

💤 Files with no reviewable changes (3)

• src/main/lib/downloadAttribution.ts
• src/main/lib/downloadAttribution.test.ts
• src/preload/api.ts

[coderabbitai]

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Scope the feature-flag privacy claim to request payload fields.

GeoIP remains enabled, so the endpoint receives network metadata beyond the evaluation and flag keys.

• src/main/lib/telemetry.ts#L1045-L1046: say the evaluation request payload contains only the stable key and flag key.
• src/main/lib/cloudCapacity.ts#L7-L8: use the same payload-scoped wording.

📍 Affects 2 files

• src/main/lib/telemetry.ts#L1045-L1046 (this comment)
• src/main/lib/cloudCapacity.ts#L7-L8

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/lib/telemetry.ts` around lines 1045 - 1046, The privacy claims in
telemetry.ts at lines 1045-1046 and cloudCapacity.ts at lines 7-8 should be
scoped to the evaluation request payload: state that the payload contains only
the installation-stable evaluation key and flag key, without claiming those are
the only data reaching the endpoint.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

scripts/installer.nsh (1)

270-308: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Indent the block for readability.

The commands inside this ${If} block aren't indented. A little indentation goes a long way to keep confusion at bay, and makes the code as neat as a pin, I say!

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/installer.nsh` around lines 270 - 308, Indent all commands within the
`${If} $R3 >= 2` block, including its nested `${AndIf}`, comment, directory
creation, file-writing operations, and `${EndIf}`, without changing the
installer logic.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@scripts/installer.nsh`:
- Around line 270-308: Indent all commands within the `${If} $R3 >= 2` block,
including its nested `${AndIf}`, comment, directory creation, file-writing
operations, and `${EndIf}`, without changing the installer logic.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6be240d7-6c56-4441-b190-46488b1bda5b

📥 Commits

Reviewing files that changed from the base of the PR and between 7215d66 and baf47e6.

📒 Files selected for processing (9)

• scripts/installer.nsh
• src/main/index.ts
• src/main/lib/anonymousIdentity.test.ts
• src/main/lib/anonymousIdentity.ts
• src/main/lib/cloudUrl.ts
• src/main/lib/telemetry.test.ts
• src/main/lib/telemetry.ts
• src/main/lib/websiteAnonymousIdentity.test.ts
• src/main/lib/websiteAnonymousIdentity.ts

💤 Files with no reviewable changes (1)

• src/main/lib/cloudUrl.ts

[deepme987]

Did a deep pass on this (identity model + trust boundary + consensus state machine + the installer carrier). Really solid work — the two things that could have sunk it both hold up:

• No two-user merge. Every identify() consumes a freshly reserved, single-use anon id that's persisted before the identify call, so one D can never be handed to two different UIDs. I tried hard to force a merge across races/logout-switch/quarantine and couldn't.
• Trust boundary holds. Subframe / untrusted-origin / stale-document / cross-process-nav reports all fail closed, trustedCloudUrl is exact-host (not suffix), and bindUserId/unbindUserId are gone from the renderer surface.

Also 👍 on moving off the deprecated alias path onto identify(F, { $anon_distinct_id }) — that's the right modernization.

One thing to reconcile before merge

The description says the #1222 conflict was resolved by routing bindSignedInUser to a no-op, but the code keeps it live (commit 614921bf): bindSignedInUser → bindUserId → applyFirebaseUserConsensus → identify(). So there are two paths to identify() — the hosted-view consensus and the interactive OAuth sign-in.

It's safe (the OAuth UID is main-authoritative, not renderer-supplied, so no spoof/merge). But there's an asymmetry worth a decision: bindSignedInUser sets boundUserId but never requestedUserId, and requestAnonymousIdentity() early-returns when requestedUserId === null. So a unanimous signed-out consensus revokes a consensus-bound user but is a no-op for an OAuth-bound user — events keep flowing under F while every live view reports signed-out. (A real conflict — mixed/different UID — does revoke it, so it's narrow.)

• Update the PR description to match the code (two identify paths, not a no-op)
• Decide: should an OAuth-bound user be demotable by a unanimous signed-out? If yes, have requestedUserId track boundUserId consistently.

Should-fix (your call, none blocking)

• installation_id dual-namespace is untouched — capture() still merges per-call over the machine-hash default, and call sites still pass the per-install inst-… id. fix(telemetry): installation_id is always the machine id (per-install id → install_id) #1159 isn't in this branch, so this PR lands the website→person merge but not the execution-level join. Worth softening the docstring (it reads as settled) and making the fix(telemetry): installation_id is always the machine id (per-install id → install_id) #1159 merge order deliberate.
• websiteAnonymousIdentity.ts:96-102 — a valid W can be permanently lost on a transient split write-failure (W-persist fails, then random-D persist succeeds → D wins forever). Silently drops the exact attribution this PR exists to capture. Suggest: if W was present but unpersistable, skip the getOrCreate write this boot and leave the pending carrier for retry.
• websiteAnonymousIdentity.ts:88-96 — a corrupt-but-present D falls through and adopts the carrier W; a present D file (readable or not) should probably suppress the carrier and force regeneration.
• Per-hard-reload churn — a hard reload of the cloud view unbinds → rotates anon → re-identify(F, new anon). Re-merges to the same F (no data loss) but fragments anonymous history + a disk write per reload. Short-circuit a re-report of the just-bound UID.
• firebaseAuthIdentity.ts provisionalFailureTerminals — unbounded Map; grows per-reporter on unpaired provisional failures over a long session (fails closed, but "every Map needs bounds").
• Bridge type has two hand-synced copies — comfyPreload imports the local type, #13687 consumes the published .d.ts, nothing keeps them in sync. Generate the package .d.ts in CI (or import the package type) so a future edit can't ship a lie to the frontend.

Confirm-intent (all fail safe, toward anonymous)

An activate without a following navigation would wedge the whole process anonymous (not just itself); a hung main-frame nav blocks identify until Electron times out; unanimous signed-out discards a conflict taint. All safe — just worth a one-line comment stating the invariant.

Net: no blocking security or merge bug. Reconcile the description/precedence item and the rest is polish. Nice architecture.

[deepme987]

adding preemptive approval if you're blocked on this. Posting [agent] review table

[benceruleanlu]

I'm fixing this one later, it has a very funny backstory.

Bridge type has two hand-synced copies — comfyPreload imports the local type, #13687 consumes the published .d.ts, nothing keeps them in sync. Generate the package .d.ts in CI (or import the package type) so a future edit can't ship a lie to the frontend.

[benceruleanlu]

@deepme987 Thanks for the deep pass. Should-fix items are addressed in 92fb15e:

• Split write failure — fixed. When W is valid but its persist fails, we return an ephemeral ID for that boot instead of letting getOrCreate race a durable random D to disk; the carrier survives for retry. Test covers the split (only the first write fails; the next boot adopts W).
• Corrupt-but-present D — fixed. An identity file that exists but can't be decoded now suppresses the carrier and regenerates D (existsSync gate; our writer is tmp+rename atomic, so a corrupt file always means external interference).
• provisionalFailureTerminals — fixed. Flushed on every main-frame commit: canceled (ERR_ABORTED) loads never emit the paired did-fail-load echo, so entries were unpairable — and a stale entry could swallow a later genuine same-key terminal and wedge the reporter pending. New test locks the post-commit settle (verified red without the fix).
• installation_id — docstring-only here, as you suggested. The Identity-model section and the defaultEventProperties comment now state the per-call inst-… override caveat and point at fix(telemetry): installation_id is always the machine id (per-install id → install_id) #1159, which should rebase over this branch.
• Reload churn — deliberately unchanged. The unbind does no disk write (it adopts the pre-reserved D), and the interim D is brand-new, so the re-identify is either a person-wise no-op or exactly the merge that carries reload-window events into F; a consensus-layer short-circuit would strand telemetry on the anonymous D. The one real artifact is a duplicate app:user_logged_in per hard reload — can add a same-UID dedup in a follow-up if login-count fidelity matters.

Heads-up since your pass: the carrier switched from base64url to the raw $device_id UUID (0283ac5, with comfy-router#34 updated to match), so the websiteAnonymousIdentity.ts line refs moved.