Skip to content

Commit 75d8b29

Browse files
committed
fix(telemetry): carry website anonymous identity into Desktop (GTM-277)
1 parent bd1e234 commit 75d8b29

35 files changed

Lines changed: 1785 additions & 1241 deletions

‎packages/comfyui-desktop-bridge-types/comfyDesktopBridge.d.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,14 @@ export interface LogsOutputMsg {
2929
}
3030
export type ComfyDesktop2TelemetryValue = string | number | boolean | null;
3131
export type ComfyDesktop2TelemetryProperties = Record<string, ComfyDesktop2TelemetryValue | ComfyDesktop2TelemetryValue[]>;
32+
export type ComfyDesktop2FirebaseAuthState = {
33+
status: 'pending';
34+
} | {
35+
status: 'signed_out';
36+
} | {
37+
status: 'signed_in';
38+
userId: string;
39+
};
3240
export interface ComfyDesktop2TerminalBridge {
3341
subscribe(installationId?: string): Promise<TerminalRestore>;
3442
unsubscribe(installationId?: string): Promise<void>;
@@ -47,6 +55,8 @@ export interface ComfyDesktop2LogsBridge {
4755
}
4856
export interface ComfyDesktop2TelemetryBridge {
4957
capture(event: string, properties?: ComfyDesktop2TelemetryProperties): void;
58+
/** Report the hosted view's complete Firebase state for process-wide consensus. */
59+
reportFirebaseAuthState?(state: ComfyDesktop2FirebaseAuthState): void;
5060
}
5161
export interface ComfyDesktop2Bridge {
5262
isRemote(): boolean;

‎packages/comfyui-desktop-bridge-types/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@comfyorg/comfyui-desktop-bridge-types",
3-
"version": "0.1.2",
3+
"version": "0.1.3",
44
"description": "TypeScript definitions for the Comfy Desktop hosted frontend bridge",
55
"type": "module",
66
"main": "./index.js",

‎scripts/installer.nsh‎

Lines changed: 35 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -242,60 +242,61 @@
242242
Pop $1
243243
!macroend
244244

245-
!macro persistDownloadTokenFromInstallerName
245+
!macro persistWebsiteAnonymousIdFromInstallerName
246246
Push $R0
247247
Push $R1
248248
Push $R2
249249
Push $R3
250250
Push $R4
251251
Push $R5
252252

253-
; Windows MVP for GTM-104: the website/download proxy can serve the same
254-
; signed installer binary with a tokenized Content-Disposition filename, e.g.
255-
; Comfy-Desktop-dt_AbC123xYz789.exe. Persist only the 12-char opaque token;
256-
; the app validates the exact token shape before sending anything to telemetry.
253+
; GTM-277 direct website identity carrier. The Router serves the signed
254+
; installer as:
257255
;
258-
; The "dt_" search is done inline with StrCpy rather than StrFunc's ${StrStr}:
259-
; StrFunc emits a standalone install function at include time, and electron-
260-
; builder compiles this script a second time for the uninstaller (where the
261-
; customInstall call site is not emitted). In that pass StrStr is unreferenced,
262-
; which trips NSIS warning 6010 — fatal under the release builder's -WX.
263-
${GetBaseName} "$EXEPATH" $R0
256+
; Comfy-Desktop-Setup-phid1_<payload>.exe
257+
;
258+
; where payload is unpadded RFC 4648 base64url of the website PostHog
259+
; $device_id's exact UTF-8 bytes (1..160 bytes, therefore 2..214 chars).
260+
; Persist only the filename-safe payload. Desktop performs strict alphabet,
261+
; length, canonical base64url, and UTF-8 validation before adopting it.
262+
; Renamed/malformed installers simply have no carrier.
263+
SetShellVarContext current
264+
StrCpy $R2 "$APPDATA\Comfy Desktop"
265+
; The current installer filename is authoritative. A plain or malformed
266+
; installer must not leave a valid carrier from an earlier unlaunched setup.
267+
Delete "$R2\pending-website-anonymous-id.txt"
268+
269+
${GetFileName} "$EXEPATH" $R0
264270
StrLen $R5 "$R0"
265-
266-
StrCpy $R1 0
267271
StrCpy $R4 ""
268-
${Do}
269-
IntOp $R3 $R1 + 3
270-
${If} $R3 > $R5
271-
${ExitDo}
272-
${EndIf}
273-
StrCpy $R3 "$R0" 3 $R1
274-
; S== is case-sensitive (LogicLib == is case-insensitive StrCmp); the proxy
275-
; emits a lowercase "dt_" marker, matching StrFunc's case-sensitive StrStr.
276-
${If} $R3 S== "dt_"
277-
IntOp $R3 $R1 + 3
278-
StrCpy $R4 "$R0" 12 $R3
279-
${ExitDo}
272+
273+
; Exact case-sensitive grammar. Prefix is 26 chars and extension is 4;
274+
; complete filenames are 32..244 chars inclusive.
275+
${If} $R5 >= 32
276+
${AndIf} $R5 <= 244
277+
StrCpy $R1 "$R0" 26
278+
StrCpy $R3 "$R0" 4 -4
279+
${If} $R1 S== "Comfy-Desktop-Setup-phid1_"
280+
${AndIf} $R3 S== ".exe"
281+
IntOp $R3 $R5 - 30
282+
StrCpy $R4 "$R0" $R3 26
280283
${EndIf}
281-
IntOp $R1 $R1 + 1
282-
${Loop}
284+
${EndIf}
283285

284286
StrLen $R3 "$R4"
285-
${If} $R3 == 12
287+
${If} $R3 >= 2
288+
${AndIf} $R3 <= 214
286289
; Matches Electron's packaged userData path documented in README:
287290
; %APPDATA%\Comfy Desktop.
288-
SetShellVarContext current
289-
StrCpy $R2 "$APPDATA\Comfy Desktop"
290291
CreateDirectory "$R2"
291292
ClearErrors
292-
FileOpen $R3 "$R2\pending-download-token.txt" w
293+
FileOpen $R3 "$R2\pending-website-anonymous-id.txt" w
293294
${IfNot} ${Errors}
294295
FileWrite $R3 "$R4$\r$\n"
295296
FileClose $R3
296-
DetailPrint " Download attribution token stored."
297+
DetailPrint " Website attribution identity stored."
297298
${Else}
298-
DetailPrint " Download attribution token could not be stored."
299+
DetailPrint " Website attribution identity could not be stored."
299300
${EndIf}
300301
${EndIf}
301302

@@ -322,7 +323,7 @@
322323
; like an install log instead of jumping from "Step 2 — Extracting…"
323324
; straight to the Finish page.
324325
SetDetailsPrint both
325-
!insertmacro persistDownloadTokenFromInstallerName
326+
!insertmacro persistWebsiteAnonymousIdFromInstallerName
326327
DetailPrint " Application files installed to: $INSTDIR"
327328
DetailPrint " Registered with Add or Remove Programs"
328329
DetailPrint " Start Menu shortcut created"

‎src/main/auth/firebaseBridge/index.ts‎

Lines changed: 2 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -14,54 +14,6 @@ import * as i18n from '../../lib/i18n'
1414
import * as mainTelemetry from '../../lib/telemetry'
1515
import { extractErrorClass } from '../../../shared/errorEvent'
1616

17-
/**
18-
* Tie the anonymous `installation_id` to the signed-in user so PostHog
19-
* merges the two identities. Both auth paths (Google server-side,
20-
* GitHub popup) converge on the resolved Firebase `user` record, so this
21-
* is the single hook that covers every sign-in.
22-
*
23-
* Person properties shipped:
24-
* - `email` — raw address. Industry-standard for product analytics
25-
* and the only practical way to support "what did <person> do"
26-
* lookups for support / debugging without a two-system round-trip
27-
* through Firebase Admin. PostHog's `$email` field also lights up
28-
* the person card with an avatar + email so persons-view search
29-
* by email works as expected.
30-
* - `email_domain` — cohort filter (e.g. comfy.org A/B targeting).
31-
* Kept alongside the raw email so existing filters / experiments
32-
* don't have to derive it at query time.
33-
* - `signed_in_via: 'desktop_2'` — every event from this person from
34-
* here on inherits this, so cloud-side events (when the user later
35-
* interacts with the embedded cloud workspace) are attributable to
36-
* a desktop-originated sign-in. Covers the case where the OAuth
37-
* flow itself runs in the system browser (no cloud.comfy.org
38-
* pageview during auth, so utm_source alone can't carry it).
39-
* - `signed_in_at_ms` — epoch-ms of the most recent sign-in. Useful
40-
* for "users who signed in within the last N days" cohorts.
41-
*
42-
* `bindUserId` is consent-gated downstream — a user who declined
43-
* telemetry binds nothing. Wrapped so a telemetry failure can never
44-
* break the auth flow.
45-
*/
46-
function bindSignedInUser(user: Record<string, unknown>): void {
47-
try {
48-
const uid = typeof user.uid === 'string' && user.uid.length > 0 ? user.uid : null
49-
if (!uid) return
50-
const email = typeof user.email === 'string' && user.email.length > 0 ? user.email : null
51-
const at = email ? email.lastIndexOf('@') : -1
52-
const emailDomain = at >= 0 ? email!.slice(at + 1).toLowerCase() : null
53-
const properties: Record<string, string | number> = {
54-
signed_in_via: 'desktop_2',
55-
signed_in_at_ms: Date.now()
56-
}
57-
if (email) properties.email = email
58-
if (emailDomain) properties.email_domain = emailDomain
59-
mainTelemetry.bindUserId(uid, properties)
60-
} catch {
61-
// telemetry must never break the auth flow
62-
}
63-
}
64-
6517
export { extractProviderId, isFirebaseAuthHandlerUrl } from './intercept'
6618

6719
export interface HandleFirebasePopupOpts {
@@ -183,8 +135,8 @@ export async function handleFirebasePopup(
183135
return
184136
}
185137
// Sign-in funnel: started -> (app:user_logged_in | auth.sign_in_failed).
186-
// `provider` splits Google vs GitHub conversion + failure rates. The
187-
// success leg is emitted by bindSignedInUser's app:user_logged_in.
138+
// `provider` splits Google vs GitHub conversion + failure rates. The success
139+
// leg fires after the reloaded hosted view reports Firebase consensus.
188140
mainTelemetry.capture('comfy.desktop.auth.sign_in_started', { provider: providerId })
189141
const env = detectFirebaseEnv(url)
190142

@@ -223,10 +175,6 @@ export async function handleFirebasePopup(
223175
// the Cloud view so users can finish sign-in in a non-default browser.
224176
showCopyLinkBanner(comfyContents, loginUrl)
225177
const { user, apiKey } = await handle.signInPromise
226-
// Bind PostHog identity as soon as we have the user — independent of
227-
// the embedded-view reload below, so the merge happens even if the
228-
// window is torn down before the reload completes.
229-
bindSignedInUser(user)
230178
if (comfyContents.isDestroyed()) return
231179
// Hold for a beat so the user actually sees the "You're signed in"
232180
// page (with its synchronised countdown) before we yank focus back

‎src/main/host/attach.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ import { noteCloudEntered } from '../lib/cloudEntry'
1212
import { noteCanvasRendered } from '../lib/canvasEntry'
1313
import { forwardDatadogError } from '../lib/processErrorHandlers'
1414
import { recordInstanceSurface } from '../lib/lastSession'
15+
import {
16+
activateFirebaseAuthReporter,
17+
deactivateFirebaseAuthReporter
18+
} from '../lib/firebaseAuthIdentity'
1519
import { convertLevelToZoomPercent } from '../lib/zoom'
1620
import { clearPendingTemplateOpen, installationEvents, type InstallationRecord } from '../installations'
1721
import { buildTemplateDeeplink } from '../sources/standalone/curatedTemplates'
@@ -139,6 +143,7 @@ export function attachInstall(entry: ComfyWindowEntry, opts: AttachInstallOpts):
139143
const comfyContents = entry.comfyView.webContents
140144
const comfyWindow = entry.window
141145
const titleBarView = entry.titleBarView
146+
activateFirebaseAuthReporter(comfyContents)
142147

143148
// Seed entry install state. The secondary index is the source of
144149
// truth for `getEntryByInstallationId(id)` — keep it in lockstep
@@ -626,6 +631,7 @@ export function attachInstall(entry: ComfyWindowEntry, opts: AttachInstallOpts):
626631
entry._installCleanup = (): void => {
627632
if (entry._installCleanup === null) return
628633
entry._installCleanup = null
634+
deactivateFirebaseAuthReporter(comfyContents)
629635
installationEvents.off('updated', onInstallationUpdated)
630636
cancelFailRetry()
631637
if (!comfyContents.isDestroyed()) {

‎src/main/host/createHostWindow.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {
2828
} from '../lib/ipc/shared'
2929
import * as mainTelemetry from '../lib/telemetry'
3030
import { getUserTier } from '../lib/userTier'
31+
import { trackFirebaseAuthReporter } from '../lib/firebaseAuthIdentity'
3132
import { forwardDatadogError } from '../lib/processErrorHandlers'
3233
import { recordDashboardSurface, recordInstanceSurface } from '../lib/lastSession'
3334
import * as settings from '../settings'
@@ -1136,6 +1137,7 @@ export function buildComfyView(
11361137
comfyView.setBackgroundColor(COMFY_BG)
11371138

11381139
const comfyContents = comfyView.webContents
1140+
trackFirebaseAuthReporter(comfyContents)
11391141
// Eagerly attach the will-download handler to the comfy view's
11401142
// session so any `session.downloadURL(...)` call below — or a server-
11411143
// initiated `Content-Disposition: attachment` response — flows

‎src/main/index.ts‎

Lines changed: 21 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -82,18 +82,14 @@ import { AUTO_LAUNCH_NONE } from './settings'
8282
import { lookupInstallUpdateOverride, recordIpcInvocation } from './lib/e2eOverrides'
8383
import * as mainTelemetry from './lib/telemetry'
8484
import {
85-
clearPendingDownloadToken,
86-
markDownloadTokenAttributed,
87-
readPendingDownloadToken
88-
} from './lib/downloadAttribution'
89-
import {
90-
clearPendingAlias,
85+
clearLegacyIdentityRetryMarker,
9186
consumeFirstLaunch,
9287
getDeviceId,
9388
getIdClass,
9489
initDeviceId,
9590
markIdentityMigrationCompleted
9691
} from './lib/deviceId'
92+
import { getInitialAnonymousDistinctId } from './lib/websiteAnonymousIdentity'
9793
import { initExperiments } from './lib/experiments'
9894
import { initCloudCapacity } from './lib/cloudCapacity'
9995
import { initUserTier } from './lib/userTier'
@@ -1414,21 +1410,21 @@ if (app.isPackaged && !app.requestSingleInstanceLock()) {
14141410
mainTelemetry.setConsentState(initialConsent)
14151411
mainTelemetry.installAppHooks()
14161412

1417-
// Initialize the deterministic device identity. Replaces the legacy
1418-
// random-UUID device-id.txt with SHA-256(machine_id + salt) so the id
1419-
// survives a clean reinstall and can be matched against the same hash
1420-
// computed by other Comfy products on the same machine. The legacy id,
1421-
// if any, is persisted in pending-identity-alias.txt by initDeviceId
1422-
// so a denied / undecided consent state at first boot does not lose
1423-
// the migration — it ships on the next consent-grant transition.
1413+
// Initialize installation metadata, then bind a separate persisted random
1414+
// PostHog anonymous id. installation_id is never used as an identity.
14241415
const { legacyId } = await initDeviceId()
1416+
// Desktop no longer performs legacy PostHog aliases. Remove any retry
1417+
// marker left by an older build, including when its migration guard exists.
1418+
clearLegacyIdentityRetryMarker()
14251419
const installationId = getDeviceId()
1426-
1427-
// Bind the anonymous distinct id before any capture runs. Does NOT
1428-
// `$identify` the installation_id (that would block the login stitch —
1429-
// see identity model in lib/telemetry.ts); the props below ship as a
1430-
// capture-`$set`.
1431-
mainTelemetry.identify(installationId, {
1420+
// A fresh Windows install can inherit the exact anonymous PostHog
1421+
// $device_id W carried in the Router's Content-Disposition filename. The
1422+
// installer stores only its filename-safe payload; this resolves and
1423+
// durably persists W before any capture. Existing Desktop state wins, and
1424+
// missing/invalid carriers fall back to a persisted/generated random D.
1425+
const anonymousDistinctId = getInitialAnonymousDistinctId()
1426+
1427+
mainTelemetry.bindAnonymousId(anonymousDistinctId, installationId, {
14321428
app_version: APP_VERSION,
14331429
platform: process.platform,
14341430
arch: process.arch,
@@ -1442,42 +1438,18 @@ if (app.isPackaged && !app.requestSingleInstanceLock()) {
14421438
mainTelemetry.registerPersonProperties(settings.getTrackedSettingsTelemetryProperties())
14431439

14441440
const isFirstLaunch = consumeFirstLaunch()
1445-
const pendingDownloadToken = readPendingDownloadToken()
1446-
if (pendingDownloadToken) {
1447-
mainTelemetry.deferDownloadTokenAlias({
1448-
downloadToken: pendingDownloadToken.token,
1449-
installationId,
1450-
source: pendingDownloadToken.source,
1451-
attachToFirstLaunch: isFirstLaunch,
1452-
onAliased: () => {
1453-
clearPendingDownloadToken()
1454-
markDownloadTokenAttributed()
1455-
}
1456-
})
1457-
}
1458-
14591441
if (legacyId) {
1460-
// Queue the alias instead of awaiting it on the boot critical path.
1461-
// - Fires as soon as consent is granted (synchronously if already so,
1462-
// on the next setConsentState('granted') transition otherwise).
1463-
// - Persisted pending-alias file (in deviceId.ts) is the source of
1464-
// truth across boots — clear it AND mark migration complete only
1465-
// inside the onAliased callback so a denied user does not skip the
1466-
// alias permanently.
1467-
mainTelemetry.deferMigrationAlias({
1468-
legacyId,
1469-
installationId,
1470-
idClass: getIdClass(),
1471-
onAliased: () => {
1472-
clearPendingAlias()
1473-
markIdentityMigrationCompleted()
1474-
}
1475-
})
1442+
// Historical random installation ids are reconciled directly in
1443+
// PostHog, not by Desktop alias writes. Complete only the local migration.
1444+
markIdentityMigrationCompleted()
14761445
}
14771446

14781447
// Boot the experiments cache. Synchronously loads the on-disk flag
14791448
// values for `getFlag()`, then kicks off a background refresh whose
14801449
// result lands on disk for the NEXT boot. Does not block boot.
1450+
// Flag evaluation uses the installation-stable property key only. It never
1451+
// captures or identifies this value, so W/D rotation cannot change an
1452+
// experiment arm or create a PostHog person.
14811453
void initExperiments({
14821454
distinctId: installationId,
14831455
personProperties: {

0 commit comments

Comments
 (0)