feat(telemetry): register client + deployment platform axes on PostHog (#13469)

## Problem

Filtering PostHog for the three product surfaces — **desktop local**,
**desktop cloud**, **web cloud** — currently requires a different hack
per pipe. For this repo's cloud build, the desktop-embedded frontend and
a plain browser are indistinguishable except by sniffing `Electron` in
`$raw_user_agent` (~124K desktop-cloud vs ~844K web-cloud execution
events/week get separated that way today).

## Change

Register the two standardized platform axes as PostHog super properties
at SDK init in `PostHogTelemetryProvider`:

- **`client`** — which surface emitted the event: `'desktop'` when the
desktop preload bridge (`window.__comfyDesktop2`) is present, else
`'web'`. The bridge is injected by Electron before any page script runs,
so detection is deterministic — unlike the existing utm-based
`source_app` attribution, which only covers sessions that *entered* via
a desktop link.
- **`deployment`** — which backend runs the work: pinned to `'cloud'`.

The register happens before the pre-init event queue flushes, so events
captured during the posthog-js dynamic-import window carry the axes too.

## Why pinning `deployment: 'cloud'` is safe (including
embedded-in-desktop)

The cloud bundle also runs **embedded in Comfy Desktop** — a cloud
install loads this same bundle in Electron, where `isCloud` and the host
bridge are both true. Two things happen there:

1. `main.ts` runs `initHostTelemetry()` *after* `initTelemetry()`, and
(when remote config `enable_telemetry` is on) it **replaces** the
registry with `HostTelemetrySink` — so tracked events
(`execution_start`, …) route through the desktop main process, bypassing
this provider. Those are tagged the same `client`/`deployment` values
main-side from the install's source category
([Comfy-Desktop#1229](https://github.com/Comfy-Org/Comfy-Desktop/pull/1229)).
2. posthog-js keeps capturing independently of the registry (pageviews,
web vitals, identify) — those are what these super properties cover in
the embedded case, and `deployment: 'cloud'` is correct for them because
the cloud bundle always talks to the cloud backend regardless of
embedding; the embedding itself is what `client: 'desktop'` captures.

The only way a cloud build runs against a non-cloud backend is a dev
setup, where `window.__CONFIG__.posthog_project_token` is absent
(injected by the cloud server) and the provider disables itself before
registering anything.

The locally-served frontend (desktop/localhost builds) never runs this
provider: `__DISTRIBUTION__` is a compile-time define, so the
`initTelemetry()` call folds away, with a runtime `IS_CLOUD_BUILD` guard
as backstop.

With both PRs, the three platforms become clean property filters:

| Surface | Filter |
|---|---|
| Desktop local | `client=desktop, deployment=local` |
| Desktop cloud | `client=desktop, deployment=cloud` |
| Web cloud | `client=web, deployment=cloud` |

## Testing

- `vitest run` on `PostHogTelemetryProvider.test.ts` — 45 passing,
including new coverage: web default, bridge-present → `client=desktop`,
and register-before-queue-flush ordering. The two desktop-entry tests
that asserted `register` is never called were narrowed to assert no
`source_app` register call.
- `pnpm typecheck` + eslint/oxlint on touched files — clean.

Ref
[MAR-51](https://linear.app/comfyorg/issue/MAR-51/foundation-desktop-sdk-dual-send-to-posthog-alongside-mixpanel)

---------

Co-authored-by: AustinMroz <austin@comfy.org>
This commit is contained in:
Benjamin Lu
2026-07-07 18:11:01 -07:00
committed by GitHub
parent 95b121bed9
commit 684b0b08b0
2 changed files with 65 additions and 2 deletions

View File

@@ -195,6 +195,46 @@ describe('PostHogTelemetryProvider', () => {
})
})
describe('platform axes (client / deployment)', () => {
afterEach(() => {
delete window.__comfyDesktop2
})
it('registers client=web and deployment=cloud in a plain browser', async () => {
createProvider()
await vi.dynamicImportSettled()
expect(hoisted.mockRegister).toHaveBeenCalledWith({
client: 'web',
deployment: 'cloud'
})
})
it('registers client=desktop when the desktop preload bridge is present', async () => {
window.__comfyDesktop2 = {
isRemote: () => false,
Telemetry: { capture: vi.fn() }
}
createProvider()
await vi.dynamicImportSettled()
expect(hoisted.mockRegister).toHaveBeenCalledWith({
client: 'desktop',
deployment: 'cloud'
})
})
it('registers platform axes before flushing pre-init queued events', async () => {
const provider = createProvider()
provider.trackSignupOpened()
await vi.dynamicImportSettled()
const registerOrder = hoisted.mockRegister.mock.invocationCallOrder[0]
const captureOrder = hoisted.mockCapture.mock.invocationCallOrder[0]
expect(registerOrder).toBeLessThan(captureOrder)
})
})
describe('desktop entry capture', () => {
function setLocation(search: string): void {
Object.defineProperty(window.location, 'search', {
@@ -208,12 +248,20 @@ describe('PostHogTelemetryProvider', () => {
setLocation('')
})
// The platform-axes register (client/deployment) always fires, so these
// assert no register call carrying desktop-entry attribution props.
function desktopEntryRegisterCalls(): unknown[][] {
return hoisted.mockRegister.mock.calls.filter(
([props]) => props && 'source_app' in (props as Record<string, unknown>)
)
}
it('does not register desktop props when utm_source is absent', async () => {
setLocation('')
createProvider()
await vi.dynamicImportSettled()
expect(hoisted.mockRegister).not.toHaveBeenCalled()
expect(desktopEntryRegisterCalls()).toHaveLength(0)
})
it('does not register desktop props when utm_source is not comfy.desktop', async () => {
@@ -221,7 +269,7 @@ describe('PostHogTelemetryProvider', () => {
createProvider()
await vi.dynamicImportSettled()
expect(hoisted.mockRegister).not.toHaveBeenCalled()
expect(desktopEntryRegisterCalls()).toHaveLength(0)
})
it('registers source_app and desktop_device_id when arriving from desktop', async () => {

View File

@@ -143,6 +143,9 @@ export class PostHogTelemetryProvider implements TelemetryProvider {
before_send: createPostHogBeforeSend()
})
this.isInitialized = true
// Before flushEventQueue so pre-init events also carry the
// platform super properties.
this.registerPlatformProps()
this.flushEventQueue()
this.registerDesktopEntryProps()
@@ -285,6 +288,18 @@ export class PostHogTelemetryProvider implements TelemetryProvider {
)
}
private registerPlatformProps(): void {
if (!this.posthog) return
try {
this.posthog.register({
client: window.__comfyDesktop2 ? 'desktop' : 'web',
deployment: 'cloud'
})
} catch (error) {
console.error('Failed to register platform props:', error)
}
}
private registerDesktopEntryProps(): void {
if (!this.posthog) return
const props = readDesktopEntryProps()