diff --git a/src/main/daemon/daemon-init.test.ts b/src/main/daemon/daemon-init.test.ts index 4df0a8f85..77881045c 100644 --- a/src/main/daemon/daemon-init.test.ts +++ b/src/main/daemon/daemon-init.test.ts @@ -44,7 +44,9 @@ const { localFallbackProvider, setLocalPtyProviderMock, unbindLocalProviderListenersMock, - rebindLocalProviderListenersMock + rebindLocalProviderListenersMock, + trackDaemonReplacedMock, + trackDaemonRetiredMock } = vi.hoisted(() => { const getPathMock = vi.fn(() => '/fake/userData') const getAppPathMock = vi.fn(() => '/fake/app') @@ -147,6 +149,8 @@ const { const setLocalPtyProviderMock = vi.fn() const unbindLocalProviderListenersMock = vi.fn() const rebindLocalProviderListenersMock = vi.fn() + const trackDaemonReplacedMock = vi.fn() + const trackDaemonRetiredMock = vi.fn() return { getPathMock, @@ -181,7 +185,9 @@ const { localFallbackProvider, setLocalPtyProviderMock, unbindLocalProviderListenersMock, - rebindLocalProviderListenersMock + rebindLocalProviderListenersMock, + trackDaemonReplacedMock, + trackDaemonRetiredMock } }) @@ -199,7 +205,7 @@ type MockAdapter = { socketPath: string tokenPath: string historyPath?: string - respawn?: () => Promise + respawn?: (reason: 'daemon_died' | 'unhealthy_resolver') => Promise protocolVersion?: number } getActiveSessionIds: ReturnType @@ -252,6 +258,11 @@ vi.mock('./daemon-health', () => ({ vi.mock('./client', () => ({ DaemonClient: daemonClientMock })) +vi.mock('./daemon-lifecycle-event', () => ({ + trackDaemonReplaced: trackDaemonReplacedMock, + trackDaemonRetired: trackDaemonRetiredMock +})) + vi.mock('./daemon-spawner', () => ({ DaemonSpawner: class MockDaemonSpawner { readonly launcher: unknown @@ -391,6 +402,8 @@ async function importFresh() { setLocalPtyProviderMock.mockClear() unbindLocalProviderListenersMock.mockClear() rebindLocalProviderListenersMock.mockClear() + trackDaemonReplacedMock.mockClear() + trackDaemonRetiredMock.mockClear() checkDaemonHealthMock.mockClear() checkDaemonHealthMock.mockResolvedValue('healthy') healthCheckDaemonMock.mockClear() @@ -400,7 +413,10 @@ async function importFresh() { getDaemonLaunchIdentityMock.mockClear() isDaemonStaleForCurrentBundleMock.mockReset() isDaemonStaleForCurrentBundleMock.mockReturnValue(false) - killStaleDaemonMock.mockClear() + // mockReset (not mockClear) also drops an unconsumed *Once queue, so a test that bails early + // can't leak a queued false into the next test's confirmedReplacement gate. + killStaleDaemonMock.mockReset() + killStaleDaemonMock.mockResolvedValue(true) getAppPathMock.mockReset() getAppPathMock.mockReturnValue('/fake/app') forkMock.mockReset() @@ -791,9 +807,19 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { // The replacement adapter's respawn closure must drive the *same* original spawner (see daemon-init.ts step 5). originalSpawner.resetHandle.mockClear() originalSpawner.ensureRunning.mockClear() - await replacementAdapter.options.respawn?.() + await replacementAdapter.options.respawn?.('daemon_died') expect(originalSpawner.resetHandle).toHaveBeenCalledTimes(1) expect(originalSpawner.ensureRunning).toHaveBeenCalledTimes(1) + // STA-2376: death → respawn retires, exactly once. + expect(trackDaemonRetiredMock).toHaveBeenCalledTimes(1) + expect(trackDaemonRetiredMock).toHaveBeenCalledWith('died_respawn') + trackDaemonRetiredMock.mockClear() + trackDaemonReplacedMock.mockClear() + // STA-2376: the resolver respawn attributes rather than emits — the launch it triggers reports it. + // Emitting here too would double-count, and would fire before the outcome is known. + await replacementAdapter.options.respawn?.('unhealthy_resolver') + expect(trackDaemonRetiredMock).not.toHaveBeenCalled() + expect(trackDaemonReplacedMock).not.toHaveBeenCalled() // Still only one spawner in the whole test — nobody new was constructed. expect(spawnerInstances).toHaveLength(1) }) @@ -822,6 +848,50 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { expect(rebindOrder).toBeGreaterThan(swapOrder) }) + // STA-2376: a manual restart kills the daemon while the outgoing adapter is still live, so a pane + // respawning on its synthetic exit reaches the death path for a user action. That must not land in + // the crash bucket. Driven from inside the restart's ensureRunning so restartInFlight is genuinely + // set, rather than asserting the guard against a flag the test poked itself. + it('does not report a retirement for a death observed during a manual restart', async () => { + const mod = await importFresh() + await mod.initDaemonPtyProvider() + const outgoingRespawn = adapterInstances[0].options.respawn + trackDaemonRetiredMock.mockClear() + + let respawnedMidRestart = false + ensureRunningOverrides.push(async () => { + await outgoingRespawn?.('daemon_died') + respawnedMidRestart = true + return { socketPath: '/fake/restarted-socket', tokenPath: '/fake/restarted-token' } + }) + + await mod.restartDaemon() + + expect(respawnedMidRestart).toBe(true) + expect(trackDaemonRetiredMock).not.toHaveBeenCalled() + + // The same closure still retires once the restart has settled, so the guard is scoped, not permanent. + await outgoingRespawn?.('daemon_died') + expect(trackDaemonRetiredMock).toHaveBeenCalledTimes(1) + expect(trackDaemonRetiredMock).toHaveBeenCalledWith('died_respawn') + + // The restart installs its own adapter, whose closure is a second copy of the guard — and the one + // that actually runs in the field from the second restart onward, since the first adapter is gone. + const restartedRespawn = adapterInstances[1].options.respawn + trackDaemonRetiredMock.mockClear() + let respawnedMidSecondRestart = false + ensureRunningOverrides.push(async () => { + await restartedRespawn?.('daemon_died') + respawnedMidSecondRestart = true + return { socketPath: '/fake/restarted-socket-2', tokenPath: '/fake/restarted-token-2' } + }) + + await mod.restartDaemon() + + expect(respawnedMidSecondRestart).toBe(true) + expect(trackDaemonRetiredMock).not.toHaveBeenCalled() + }) + it('preserves legacy adapter instances by identity, drains outgoing router via disposeRouterOnly, and re-discovers legacy sessions on the new router', async () => { const mod = await importFresh() @@ -1143,6 +1213,9 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { ]), expect.objectContaining({ cwd: '/fake/userData', detached: true }) ) + // STA-2376: different-app-path replacement, emitted exactly once. + expect(trackDaemonReplacedMock).toHaveBeenCalledTimes(1) + expect(trackDaemonReplacedMock).toHaveBeenCalledWith('different_app_path', 0) }) it('holds a full adoption pair before a healthy launcher resolves', async () => { @@ -1351,6 +1424,9 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { ]), expect.objectContaining({ cwd: '/fake/userData', detached: true }) ) + // STA-2376: the launcher is the sole emitter for a resolver replace, and fires exactly once. + expect(trackDaemonReplacedMock).toHaveBeenCalledTimes(1) + expect(trackDaemonReplacedMock).toHaveBeenCalledWith('unhealthy_resolver', 0) }) it('preserves a resolver-unhealthy daemon when it owns live sessions', async () => { @@ -1392,6 +1468,8 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { expect(getDaemonLaunchIdentityMock).not.toHaveBeenCalled() expect(killStaleDaemonMock).not.toHaveBeenCalled() expect(forkMock).not.toHaveBeenCalled() + // STA-2376: preserving a daemon is not a lifecycle transition — no event. + expect(trackDaemonReplacedMock).not.toHaveBeenCalled() }) it('preserves a resolver-unhealthy daemon when live session state cannot be verified', async () => { @@ -1477,6 +1555,180 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { ]), expect.objectContaining({ detached: true }) ) + // STA-2376: an unreachable daemon with no live sessions is replaced via the failed-health path, once. + expect(trackDaemonReplacedMock).toHaveBeenCalledTimes(1) + expect(trackDaemonReplacedMock).toHaveBeenCalledWith('failed_health_check', 0) + }) + + it('does not report a replacement when startup finds no daemon to remove', async () => { + const mod = await importFresh() + await mod.initDaemonPtyProvider() + checkDaemonHealthMock.mockResolvedValue('unreachable') + killStaleDaemonMock.mockResolvedValueOnce(false) + forkMock.mockImplementationOnce(() => { + throw new Error('stop after replacement decision') + }) + const launcher = spawnerInstances[0].launcher as ( + socketPath: string, + tokenPath: string + ) => Promise<{ shutdown(): Promise }> + + await expect(launcher('/fake/socket', '/fake/token')).rejects.toThrow( + 'stop after replacement decision' + ) + + expect(trackDaemonReplacedMock).not.toHaveBeenCalled() + }) + + // STA-2376 regression: dropping the adapter's last authenticated client is enough to make an idle + // daemon self-retire, so by the time the launcher runs there is nothing to kill and its own + // confirmed-kill gate reports nothing. The attributed reason is what keeps the runtime resolver + // replacement on the wire — and keeps it off the failed_health_check bucket it would otherwise land in. + it('reports the runtime resolver replacement even after the daemon self-retired', async () => { + const mod = await importFresh() + await mod.initDaemonPtyProvider() + const adapterOptions = adapterInstances[0].options + trackDaemonReplacedMock.mockClear() + + // The daemon is gone before the launcher looks: nothing answers, nothing left to kill. + checkDaemonHealthMock.mockResolvedValue('unreachable') + killStaleDaemonMock.mockResolvedValueOnce(false).mockResolvedValueOnce(false) + forkMock.mockImplementationOnce(() => { + throw new Error('stop after replacement decision') + }) + const launcher = spawnerInstances[0].launcher as ( + socketPath: string, + tokenPath: string + ) => Promise<{ shutdown(): Promise }> + + await adapterOptions.respawn?.('unhealthy_resolver') + expect(trackDaemonReplacedMock).not.toHaveBeenCalled() + + await expect(launcher('/fake/socket', '/fake/token')).rejects.toThrow( + 'stop after replacement decision' + ) + expect(trackDaemonReplacedMock).toHaveBeenCalledTimes(1) + expect(trackDaemonReplacedMock).toHaveBeenCalledWith('unhealthy_resolver', 0) + + // One-shot: a later unrelated launch must not inherit the attribution. + trackDaemonReplacedMock.mockClear() + forkMock.mockImplementationOnce(() => { + throw new Error('stop after replacement decision') + }) + await expect(launcher('/fake/socket', '/fake/token')).rejects.toThrow( + 'stop after replacement decision' + ) + expect(trackDaemonReplacedMock).not.toHaveBeenCalled() + }) + + // STA-2376: the attribution covers the case the confirmed-kill gate cannot see; it must not + // overwrite a reason this launch proved against the daemon it actually removed. Otherwise a + // resolver that recovers mid-flight bills a real stale-bundle replacement to the resolver bucket. + it('prefers a proven replacement reason over the attributed one', async () => { + const mod = await importFresh() + await mod.initDaemonPtyProvider() + const adapterOptions = adapterInstances[0].options + trackDaemonReplacedMock.mockClear() + + // Resolver recovered by the time the launcher looks, but the daemon is genuinely from another path. + getMacDaemonSystemResolverHealthMock.mockReturnValue('healthy') + getDaemonLaunchIdentityMock.mockReturnValueOnce('mismatch') + forkMock.mockImplementationOnce(() => { + throw new Error('stop after replacement decision') + }) + const launcher = spawnerInstances[0].launcher as ( + socketPath: string, + tokenPath: string + ) => Promise<{ shutdown(): Promise }> + + await adapterOptions.respawn?.('unhealthy_resolver') + await expect(launcher('/fake/socket', '/fake/token')).rejects.toThrow( + 'stop after replacement decision' + ) + + expect(trackDaemonReplacedMock).toHaveBeenCalledTimes(1) + expect(trackDaemonReplacedMock).toHaveBeenCalledWith('different_app_path', 0) + }) + + // STA-2376: failed_health_check is the residual bucket, not an identification, so it must not + // absorb the attribution. The same dead login session that fails the resolver also fails the PTY + // spawn probe, and with zero live sessions that lands here instead of the degraded preserve — + // so this is the likely shape of the incident, not a corner case. + it('keeps the attributed reason when the launcher only reaches failed_health_check', async () => { + const mod = await importFresh() + await mod.initDaemonPtyProvider() + const adapterOptions = adapterInstances[0].options + trackDaemonReplacedMock.mockClear() + + // Daemon survived the disconnect (non-alive sessions keep it non-idle) but fails the spawn probe. + checkDaemonHealthMock.mockResolvedValue('pty-spawn-unhealthy') + forkMock.mockImplementationOnce(() => { + throw new Error('stop after replacement decision') + }) + const launcher = spawnerInstances[0].launcher as ( + socketPath: string, + tokenPath: string + ) => Promise<{ shutdown(): Promise }> + + await adapterOptions.respawn?.('unhealthy_resolver') + await expect(launcher('/fake/socket', '/fake/token')).rejects.toThrow( + 'stop after replacement decision' + ) + + expect(trackDaemonReplacedMock).toHaveBeenCalledTimes(1) + expect(trackDaemonReplacedMock).toHaveBeenCalledWith('unhealthy_resolver', 0) + }) + + // STA-2376: in the field the identified reasons confirm via cleanupDaemonForProtocol().cleaned, not + // via killStaleDaemon — the daemon is healthy, so cleanup shuts it down over RPC and unlinks its pid, + // leaving nothing for the kill to find. The other tests reach confirmedReplacement through the kill, + // so without this one the `.cleaned` half could be dropped and every identified reason would go + // silent in production with the suite still green. + it('reports a replacement confirmed by cleanup alone, with no stale daemon left to kill', async () => { + const mod = await importFresh() + await mod.initDaemonPtyProvider() + trackDaemonReplacedMock.mockClear() + + getDaemonLaunchIdentityMock.mockReturnValueOnce('mismatch') + killStaleDaemonMock.mockResolvedValueOnce(false) + // The daemon answers cleanup's liveness probe, then the endpoint goes away so the self-shutdown + // wait succeeds and cleanup reports cleaned:true. + probeSocketExistsMock.mockReturnValue(true) + netConnectMock.mockImplementationOnce(() => { + const handlers: Record void)[]> = { connect: [], error: [] } + return { + on(event: string, cb: () => void) { + handlers[event]?.push(cb) + if (event === 'connect') { + queueMicrotask(() => cb()) + } + return this + }, + removeListener(event: string, cb: () => void) { + handlers[event] = handlers[event]?.filter((handler) => handler !== cb) ?? [] + return this + }, + destroy() {} + } + }) + forkMock.mockImplementationOnce(() => { + throw new Error('stop after replacement decision') + }) + const launcher = spawnerInstances[0].launcher as ( + socketPath: string, + tokenPath: string + ) => Promise<{ shutdown(): Promise }> + + await expect(launcher('/fake/socket', '/fake/token')).rejects.toThrow( + 'stop after replacement decision' + ) + + expect(killStaleDaemonMock).toHaveBeenCalled() + expect(trackDaemonReplacedMock).toHaveBeenCalledTimes(1) + expect(trackDaemonReplacedMock).toHaveBeenCalledWith('different_app_path', 0) + + // beforeEach only mockClear()s this one, so hand it back rather than leaving later tests probing a live endpoint. + probeSocketExistsMock.mockReturnValue(false) }) it('removes detached daemon startup listeners after readiness', async () => { @@ -2693,6 +2945,9 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { ]), expect.objectContaining({ detached: true }) ) + // STA-2376: stale-bundle replacement, emitted exactly once. + expect(trackDaemonReplacedMock).toHaveBeenCalledTimes(1) + expect(trackDaemonReplacedMock).toHaveBeenCalledWith('stale_bundle', 0) }) it('preserves a packaged daemon that predates the current app bundle when it owns live sessions', async () => { diff --git a/src/main/daemon/daemon-init.ts b/src/main/daemon/daemon-init.ts index 488b75ef6..24fc9ef4d 100644 --- a/src/main/daemon/daemon-init.ts +++ b/src/main/daemon/daemon-init.ts @@ -16,7 +16,7 @@ import { type DaemonLauncher, type DaemonProcessHandle } from './daemon-spawner' -import { DaemonPtyAdapter } from './daemon-pty-adapter' +import { DaemonPtyAdapter, type DaemonRespawnReason } from './daemon-pty-adapter' import { DaemonPtyRouter } from './daemon-pty-router' import { DaemonClient } from './client' import { @@ -39,6 +39,8 @@ import { pruneOldDaemonHosts } from './daemon-host-relocation' import { DegradedDaemonPtyProvider } from './degraded-daemon-pty-provider' +import { trackDaemonReplaced, trackDaemonRetired } from './daemon-lifecycle-event' +import type { DaemonReplaceReason } from '../../shared/daemon-lifecycle-telemetry' import { getLocalPtyProvider, setLocalPtyProvider, @@ -321,6 +323,12 @@ async function shouldPreserveDaemonWithLiveSessions( return true } +// Why: the adapter decides a runtime resolver replacement, but the launcher completes it — and by +// then the daemon has usually self-retired (dropping its last authenticated client is enough), so +// there is nothing left to kill and the launcher's own confirmed-kill gate would report nothing. +// The adapter hands the reason across so the launch it triggers reports what actually drove it. +let attributedReplaceReason: DaemonReplaceReason | null = null + function createOutOfProcessLauncher( runtimeDir: string, macosLoginSessionWatch = false @@ -329,6 +337,18 @@ function createOutOfProcessLauncher( const entryPath = getDaemonEntryPath() const pidPath = suppliedPidPath ?? getDaemonPidPath(runtimeDir) const launchNonce = suppliedLaunchNonce ?? randomUUID() + // One-shot: whichever launch consumes it owns the attribution, so a later unrelated launch can't + // reuse it. The write in the respawn closure reaches here without an intervening await, which is + // what makes a bare module-scoped slot safe — keep it that way or a concurrent launch can steal it. + const attributedReason = attributedReplaceReason + attributedReplaceReason = null + let pendingReplacement: + | { + reason: Parameters[0] + liveSessionCount: number | null + } + | undefined + let confirmedReplacement = false let adoptionClient: DaemonClient | null = new DaemonClient({ socketPath, tokenPath }) try { // Why: acquire the full pair before control-only probes so an expired inherited deadline can't fire in the probe-to-adoption gap. @@ -364,7 +384,9 @@ function createOutOfProcessLauncher( return preserveDaemon() } console.warn('[daemon] Replacing daemon with unavailable macOS system resolver') - await cleanupDaemonForProtocol(runtimeDir, PROTOCOL_VERSION) + pendingReplacement = { reason: 'unhealthy_resolver', liveSessionCount } + confirmedReplacement = (await cleanupDaemonForProtocol(runtimeDir, PROTOCOL_VERSION)) + .cleaned } else { // Why: a protocol-healthy daemon can outlive its launching app bundle (dev worktree rebuild, or packaged update replacing the app path). const identity = await getDaemonLaunchIdentity( @@ -396,7 +418,13 @@ function createOutOfProcessLauncher( ? '[daemon] Replacing daemon launched before the current app bundle was installed' : '[daemon] Replacing daemon launched from a different app path' ) - await cleanupDaemonForProtocol(runtimeDir, PROTOCOL_VERSION) + // liveSessionCount is 0: shouldPreserveDaemonWithLiveSessions() only falls through at exactly 0. + pendingReplacement = { + reason: stalePackagedBundle ? 'stale_bundle' : 'different_app_path', + liveSessionCount: 0 + } + confirmedReplacement = (await cleanupDaemonForProtocol(runtimeDir, PROTOCOL_VERSION)) + .cleaned } else { // Why: healthy daemon from a previous session answered a protocol ping — safe to reuse. return preserveDaemon() @@ -439,12 +467,35 @@ function createOutOfProcessLauncher( `[daemon] Replacing daemon that failed the health check (health=${health}, liveSessions=${liveSessionCount ?? 'unverifiable'}, graceRetries=${graceRetry})` ) } + // Why: unlike the log above, telemetry gates on confirmedReplacement below — the + // post-kill truth — so a cold start that killed nothing never reports a replacement. + pendingReplacement = { reason: 'failed_health_check', liveSessionCount } } // Why: a raw socket can outlive a broken daemon; kill by PID before respawn so the new daemon doesn't race the stale one. adoptionClient?.disconnect() adoptionClient = null - await killStaleDaemon(runtimeDir, socketPath, tokenPath) + confirmedReplacement = + (await killStaleDaemon(runtimeDir, socketPath, tokenPath)) || confirmedReplacement + // Why: rank by how well each reason is evidenced. A confirmed kill whose reason positively + // identified the daemon outranks the attribution, so a stale bundle caught here is not billed + // to the resolver. failed_health_check is the residual "couldn't tell" bucket though — it also + // absorbs wedges and crashes — so the adapter's attribution beats it. That case is not exotic: + // the same dead login session that fails the resolver also fails the PTY spawn probe, and with + // zero live sessions that lands here rather than in the degraded preserve above. + const identifiedReplacement = + pendingReplacement && + confirmedReplacement && + pendingReplacement.reason !== 'failed_health_check' + ? pendingReplacement + : null + if (identifiedReplacement) { + trackDaemonReplaced(identifiedReplacement.reason, identifiedReplacement.liveSessionCount) + } else if (attributedReason) { + trackDaemonReplaced(attributedReason, 0) + } else if (pendingReplacement && confirmedReplacement) { + trackDaemonReplaced(pendingReplacement.reason, pendingReplacement.liveSessionCount) + } const userDataPath = app.getPath('userData') // Why: on win32 packaged, stage a daemon-host copy in userData so its image escapes the NSIS updater's kill zone; lazy so it's off first-paint. Fail-open: null → in-dir host. @@ -679,8 +730,22 @@ export async function initDaemonPtyProvider( tokenPath: info.tokenPath, historyPath: getHistoryDir(), // Why: on daemon death, ensureConnected() detects the dead socket and calls this to fork a replacement before retrying. - respawn: async () => { - console.warn('[daemon] Daemon process died — respawning') + respawn: async (reason: DaemonRespawnReason) => { + // Why: attribute rather than emit — the launcher below is the one that completes the + // replacement, and emitting here would fire before the outcome is known. + // Caveat: a wedged-but-alive daemon (#8689) can still report died_respawn here and + // failed_health_check from the launcher — the app cannot tell wedged from dead at this point. + if (reason === 'daemon_died') { + console.warn('[daemon] Daemon process died — respawning') + // Why: a manual restart tears the daemon down under a still-live adapter, so a pane + // respawning on its synthetic exit would bill a user action to the crash bucket. + if (!restartInFlight) { + trackDaemonRetired('died_respawn') + } + } else if (reason === 'unhealthy_resolver') { + // Must reach the launcher below without an await in between; see the consume site. + attributedReplaceReason = 'unhealthy_resolver' + } newSpawner.resetHandle() await newSpawner.ensureRunning() return takeDaemonAdoptionLeaseRelease(newSpawner.getHandle()) @@ -861,8 +926,22 @@ async function runRestartDaemon(): Promise { socketPath: info.socketPath, tokenPath: info.tokenPath, historyPath: getHistoryDir(), - respawn: async () => { - console.warn('[daemon] Daemon process died — respawning') + respawn: async (reason: DaemonRespawnReason) => { + // Why: attribute rather than emit — the launcher below is the one that completes the + // replacement, and emitting here would fire before the outcome is known. + // Caveat: a wedged-but-alive daemon (#8689) can still report died_respawn here and + // failed_health_check from the launcher — the app cannot tell wedged from dead at this point. + if (reason === 'daemon_died') { + console.warn('[daemon] Daemon process died — respawning') + // Why: a manual restart tears the daemon down under a still-live adapter, so a pane + // respawning on its synthetic exit would bill a user action to the crash bucket. + if (!restartInFlight) { + trackDaemonRetired('died_respawn') + } + } else if (reason === 'unhealthy_resolver') { + // Must reach the launcher below without an await in between; see the consume site. + attributedReplaceReason = 'unhealthy_resolver' + } currentSpawner.resetHandle() await currentSpawner.ensureRunning() return takeDaemonAdoptionLeaseRelease(currentSpawner.getHandle()) diff --git a/src/main/daemon/daemon-lifecycle-event.test.ts b/src/main/daemon/daemon-lifecycle-event.test.ts new file mode 100644 index 000000000..c8007079c --- /dev/null +++ b/src/main/daemon/daemon-lifecycle-event.test.ts @@ -0,0 +1,76 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { bucketDaemonLiveSessionCount } from '../../shared/daemon-lifecycle-telemetry' +import { validate } from '../telemetry/validator' + +const { trackMock } = vi.hoisted(() => ({ trackMock: vi.fn() })) +vi.mock('../telemetry/client', () => ({ track: trackMock })) + +import { trackDaemonReplaced, trackDaemonRetired } from './daemon-lifecycle-event' + +beforeEach(() => { + trackMock.mockClear() +}) + +describe('bucketDaemonLiveSessionCount', () => { + it('buckets counts and maps null to unknown', () => { + expect(bucketDaemonLiveSessionCount(null)).toBe('unknown') + expect(bucketDaemonLiveSessionCount(0)).toBe('0') + expect(bucketDaemonLiveSessionCount(1)).toBe('1') + expect(bucketDaemonLiveSessionCount(2)).toBe('2-5') + expect(bucketDaemonLiveSessionCount(5)).toBe('2-5') + expect(bucketDaemonLiveSessionCount(6)).toBe('6+') + expect(bucketDaemonLiveSessionCount(999)).toBe('6+') + }) +}) + +// Revert-sensitive: asserts each emitter fires `daemon_lifecycle` with a payload the real +// runtime validator accepts. If the event, emitter, or schema is reverted, these fail. +describe('daemon lifecycle emitters', () => { + it('emits a validator-accepted replace payload', () => { + trackDaemonReplaced('stale_bundle', 0) + expect(trackMock).toHaveBeenCalledTimes(1) + const [name, props] = trackMock.mock.calls[0] + expect(name).toBe('daemon_lifecycle') + expect(props).toEqual({ + transition: 'replaced', + reason: 'stale_bundle', + live_session_count_bucket: '0' + }) + expect(validate('daemon_lifecycle', props).ok).toBe(true) + }) + + it('maps an unverifiable session count to the unknown bucket', () => { + trackDaemonReplaced('different_app_path', null) + const [, props] = trackMock.mock.calls[0] + expect(props).toEqual({ + transition: 'replaced', + reason: 'different_app_path', + live_session_count_bucket: 'unknown' + }) + expect(validate('daemon_lifecycle', props).ok).toBe(true) + }) + + // Why: both emitters run on the daemon launch/respawn path, where a throw would cost every terminal. + it('swallows a throwing telemetry client instead of failing the caller', () => { + trackMock.mockImplementationOnce(() => { + throw new Error('posthog exploded') + }) + expect(() => trackDaemonReplaced('failed_health_check', null)).not.toThrow() + trackMock.mockImplementationOnce(() => { + throw new Error('posthog exploded') + }) + expect(() => trackDaemonRetired('died_respawn')).not.toThrow() + }) + + it('emits a validator-accepted retirement payload', () => { + trackDaemonRetired('died_respawn') + const [name, props] = trackMock.mock.calls[0] + expect(name).toBe('daemon_lifecycle') + expect(props).toEqual({ + transition: 'retired', + reason: 'died_respawn', + live_session_count_bucket: 'unknown' + }) + expect(validate('daemon_lifecycle', props).ok).toBe(true) + }) +}) diff --git a/src/main/daemon/daemon-lifecycle-event.ts b/src/main/daemon/daemon-lifecycle-event.ts new file mode 100644 index 000000000..8dd8a54ac --- /dev/null +++ b/src/main/daemon/daemon-lifecycle-event.ts @@ -0,0 +1,42 @@ +// App-side emitters for the `daemon_lifecycle` telemetry event (STA-2376). Kept out of daemon-init +// so the replace/retire call sites stay one line and this stays a clean unit-test/mocking seam. +// No-op in dev/contributor builds (see telemetry/client `track`); rare in the field (≪1/user/day). + +import { + bucketDaemonLiveSessionCount, + type DaemonReplaceReason, + type DaemonRetireReason +} from '../../shared/daemon-lifecycle-telemetry' +import { track } from '../telemetry/client' + +// Why: both call sites sit on the daemon launch/respawn path, where a throw costs the user every +// terminal. Diagnostics must never be able to do that, so failures die here. +function trackQuietly(props: Parameters>[1]): void { + try { + track('daemon_lifecycle', props) + } catch { + // Telemetry is best-effort; a dropped event must not fail a daemon launch. + } +} + +// Replaced a still-connectable daemon (startup launcher decided to kill and re-fork it). +export function trackDaemonReplaced( + reason: DaemonReplaceReason, + liveSessionCount: number | null +): void { + trackQuietly({ + transition: 'replaced', + reason, + live_session_count_bucket: bucketDaemonLiveSessionCount(liveSessionCount) + }) +} + +// Adapter observed the daemon die and forked a replacement; the app can't see the daemon-internal +// exit cause, so the live-session count is unknowable here and buckets to `unknown`. +export function trackDaemonRetired(reason: DaemonRetireReason): void { + trackQuietly({ + transition: 'retired', + reason, + live_session_count_bucket: bucketDaemonLiveSessionCount(null) + }) +} diff --git a/src/main/daemon/daemon-pty-adapter.test.ts b/src/main/daemon/daemon-pty-adapter.test.ts index 49edc357e..37614abe0 100644 --- a/src/main/daemon/daemon-pty-adapter.test.ts +++ b/src/main/daemon/daemon-pty-adapter.test.ts @@ -2389,7 +2389,8 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { // Next spawn should detect the dead socket, call respawn, and succeed const r2 = await respawnAdapter.spawn({ cols: 80, rows: 24 }) expect(r2.id).toBeDefined() - expect(respawnFn).toHaveBeenCalledOnce() + expect(respawnFn).toHaveBeenCalledTimes(1) + expect(respawnFn).toHaveBeenCalledWith('daemon_died') respawnAdapter.dispose() await respawnServer?.shutdown() @@ -2428,7 +2429,8 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { try { const result = await respawnAdapter.spawn({ cols: 80, rows: 24 }) expect(result.id).toBeDefined() - expect(respawnFn).toHaveBeenCalledOnce() + expect(respawnFn).toHaveBeenCalledTimes(1) + expect(respawnFn).toHaveBeenCalledWith('daemon_died') } finally { ensureConnectedSpy.mockRestore() respawnAdapter.dispose() @@ -2461,7 +2463,8 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { ]) expect(r1.id).toBeDefined() expect(r2.id).toBeDefined() - expect(respawnFn).toHaveBeenCalledOnce() + expect(respawnFn).toHaveBeenCalledTimes(1) + expect(respawnFn).toHaveBeenCalledWith('daemon_died') respawnAdapter.dispose() await respawnServer?.shutdown() @@ -2536,7 +2539,8 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { tokenPath, respawnAdapter.protocolVersion ) - expect(respawnFn).toHaveBeenCalledOnce() + expect(respawnFn).toHaveBeenCalledTimes(1) + expect(respawnFn).toHaveBeenCalledWith('unhealthy_resolver') expect(exits).toEqual([]) expect(replacement.id).toBeDefined() diff --git a/src/main/daemon/daemon-pty-adapter.ts b/src/main/daemon/daemon-pty-adapter.ts index 4622fa630..10efd2ec4 100644 --- a/src/main/daemon/daemon-pty-adapter.ts +++ b/src/main/daemon/daemon-pty-adapter.ts @@ -97,10 +97,12 @@ export type DaemonPtyAdapterOptions = { protocolVersion?: number /** Directory for disk-based terminal history; when set, raw PTY output is written to disk for cold restore on daemon crash. */ historyPath?: string - /** Called when the daemon socket is unreachable; forks a fresh daemon so the next connect can succeed. */ - respawn?: () => Promise void)> + /** Forks a fresh daemon after endpoint death or a confirmed resolver-health replacement. */ + respawn?: (reason: DaemonRespawnReason) => Promise void)> } +export type DaemonRespawnReason = 'daemon_died' | 'unhealthy_resolver' + const MAX_TOMBSTONES = 1000 const MAX_CONCURRENT_CHECKPOINTS = 4 @@ -125,7 +127,7 @@ export class DaemonPtyAdapter implements IPtyProvider { private client: DaemonClient private historyManager: HistoryManager | null private historyReader: HistoryReader | null - private respawnFn: (() => Promise void)>) | null + private respawnFn: DaemonPtyAdapterOptions['respawn'] | null private pendingRespawnAdoptionRelease: (() => void) | null = null private respawnAdoptionClosed = false // Why: concurrent spawn() calls hitting a dead daemon would each fork their own; this promise coalesces respawns so only the first forks and the rest await it. @@ -1721,7 +1723,8 @@ export class DaemonPtyAdapter implements IPtyProvider { this.fanoutSyntheticExits(-1) if (!this.respawnPromise) { this.respawnPromise = this.doRespawn( - '[daemon] macOS system resolver unavailable - respawning daemon' + '[daemon] macOS system resolver unavailable - respawning daemon', + 'unhealthy_resolver' ).finally(() => { this.respawnPromise = null }) @@ -1746,12 +1749,15 @@ export class DaemonPtyAdapter implements IPtyProvider { } } - private async doRespawn(message = '[daemon] Daemon died — respawning'): Promise { + private async doRespawn( + message = '[daemon] Daemon died — respawning', + reason: DaemonRespawnReason = 'daemon_died' + ): Promise { console.warn(message) this.removeEventListener?.() this.removeEventListener = null this.client.disconnect() - const releaseAdoptionLease = await this.respawnFn!() + const releaseAdoptionLease = await this.respawnFn!(reason) if (this.respawnAdoptionClosed) { // Why: app teardown may win mid-respawn; a late result must not reinstall a lease nobody owns. releaseAdoptionLease?.() diff --git a/src/main/daemon/daemon-self-retirement-respawn.test.ts b/src/main/daemon/daemon-self-retirement-respawn.test.ts index a71842721..a5df5453f 100644 --- a/src/main/daemon/daemon-self-retirement-respawn.test.ts +++ b/src/main/daemon/daemon-self-retirement-respawn.test.ts @@ -86,6 +86,7 @@ describe('daemon self-retirement respawn', () => { ]) expect(respawn).toHaveBeenCalledTimes(1) + expect(respawn).toHaveBeenCalledWith('daemon_died') adapter.dispose() }) diff --git a/src/shared/daemon-lifecycle-telemetry.ts b/src/shared/daemon-lifecycle-telemetry.ts new file mode 100644 index 000000000..590419531 --- /dev/null +++ b/src/shared/daemon-lifecycle-telemetry.ts @@ -0,0 +1,47 @@ +// Enums + bucketing for the `daemon_lifecycle` telemetry event (STA-2376). +// The daemon's own retirement cause (pam-rejections/probe-timeouts) lives in the +// subprocess and never crosses into the app, so every reason here is one the app +// itself decides: a startup replace, or an observed death→respawn. + +// Startup launcher replaced a still-connectable daemon (each maps 1:1 to a `daemon-init.ts` decision). +export const DAEMON_REPLACE_REASONS = [ + 'unhealthy_resolver', + 'stale_bundle', + 'different_app_path', + 'failed_health_check' +] as const +export type DaemonReplaceReason = (typeof DAEMON_REPLACE_REASONS)[number] + +// Adapter observed the daemon die and forked a replacement. +export const DAEMON_RETIRE_REASONS = ['died_respawn'] as const +export type DaemonRetireReason = (typeof DAEMON_RETIRE_REASONS)[number] + +export const DAEMON_LIFECYCLE_TRANSITIONS = ['replaced', 'retired'] as const +export type DaemonLifecycleTransition = (typeof DAEMON_LIFECYCLE_TRANSITIONS)[number] + +export const DAEMON_LIFECYCLE_REASONS = [ + ...DAEMON_REPLACE_REASONS, + ...DAEMON_RETIRE_REASONS +] as const +export type DaemonLifecycleReason = (typeof DAEMON_LIFECYCLE_REASONS)[number] + +// Bucketed, never raw: exact live-session counts could fingerprint heavy users. `unknown` when +// the count couldn't be verified (null) — e.g. a wedged daemon or an already-dead respawn target. +export const DAEMON_LIFECYCLE_SESSION_BUCKETS = ['0', '1', '2-5', '6+', 'unknown'] as const +export type DaemonLifecycleSessionBucket = (typeof DAEMON_LIFECYCLE_SESSION_BUCKETS)[number] + +export function bucketDaemonLiveSessionCount(count: number | null): DaemonLifecycleSessionBucket { + if (count === null) { + return 'unknown' + } + if (count <= 0) { + return '0' + } + if (count === 1) { + return '1' + } + if (count <= 5) { + return '2-5' + } + return '6+' +} diff --git a/src/shared/telemetry-events.test.ts b/src/shared/telemetry-events.test.ts index 8abdddba6..e172d03a7 100644 --- a/src/shared/telemetry-events.test.ts +++ b/src/shared/telemetry-events.test.ts @@ -338,6 +338,82 @@ describe('agent_error schema', () => { }) }) +describe('daemon_lifecycle schema', () => { + it('round-trips a startup replace payload', () => { + const parsed = eventSchemas.daemon_lifecycle.safeParse({ + transition: 'replaced', + reason: 'stale_bundle', + live_session_count_bucket: '0' + }) + expect(parsed.success).toBe(true) + }) + + it('round-trips a retirement payload', () => { + const parsed = eventSchemas.daemon_lifecycle.safeParse({ + transition: 'retired', + reason: 'died_respawn', + live_session_count_bucket: 'unknown' + }) + expect(parsed.success).toBe(true) + }) + + // Core privacy invariant: enum-only + bucketed counts. If this flips, the lane is leaking + // paths/versions/exact counts — revert the offending schema change (STA-2376). + // Both union members, so neither can lose .strict() unnoticed. + it('rejects raw paths, versions, and unbucketed counts via .strict()', () => { + const bases = [ + { transition: 'replaced', reason: 'failed_health_check', live_session_count_bucket: '2-5' }, + { transition: 'retired', reason: 'died_respawn', live_session_count_bucket: 'unknown' } + ] + for (const base of bases) { + for (const leak of [ + { daemon_path: '/Users/alice/Orca.app' }, + { daemon_app_version: '1.4.129' }, + { live_session_count: 3 } + ]) { + const parsed = eventSchemas.daemon_lifecycle.safeParse({ ...base, ...leak }) + expect(parsed.success).toBe(false) + } + // Sanity: the base itself must be valid, so the rejections above are the leak, not the base. + expect(eventSchemas.daemon_lifecycle.safeParse(base).success).toBe(true) + } + }) + + it('rejects unknown reason and bucket enum values', () => { + expect( + eventSchemas.daemon_lifecycle.safeParse({ + transition: 'replaced', + reason: 'made_up_reason', + live_session_count_bucket: '0' + }).success + ).toBe(false) + expect( + eventSchemas.daemon_lifecycle.safeParse({ + transition: 'replaced', + reason: 'stale_bundle', + live_session_count_bucket: '99' + }).success + ).toBe(false) + }) + + it('rejects reasons and fields that do not belong to the transition', () => { + expect( + eventSchemas.daemon_lifecycle.safeParse({ + transition: 'replaced', + reason: 'died_respawn', + live_session_count_bucket: 'unknown' + }).success + ).toBe(false) + expect( + eventSchemas.daemon_lifecycle.safeParse({ + transition: 'retired', + reason: 'failed_health_check', + live_session_count_bucket: 'unknown' + }).success + ).toBe(false) + }) +}) + describe('workspace_created schema', () => { it('rejects unknown source', () => { const parsed = eventSchemas.workspace_created.safeParse({ diff --git a/src/shared/telemetry-events.ts b/src/shared/telemetry-events.ts index 2ad6cbf7e..9cbfaac20 100644 --- a/src/shared/telemetry-events.ts +++ b/src/shared/telemetry-events.ts @@ -21,6 +21,12 @@ import { FEATURE_INTERACTION_USAGE_BUCKETS, getFeatureInteractionCategory } from './feature-interactions' +import { + DAEMON_LIFECYCLE_SESSION_BUCKETS, + DAEMON_LIFECYCLE_TRANSITIONS, + DAEMON_REPLACE_REASONS, + DAEMON_RETIRE_REASONS +} from './daemon-lifecycle-telemetry' import { SETUP_SCRIPT_IMPORT_PROVIDERS } from './setup-script-import-providers' import { WORKSPACE_SOURCE_VALUES, type WorkspaceSource } from './workspace-source' import { appStarSourceSchema } from './gh-star-source' @@ -364,6 +370,26 @@ const agentErrorSchema = z // Why: daemon start-failure signal (fleet-wide outage like v1.4.129-rc.1); enum-only so raw stderr never reaches the wire. const daemonStartFailedSchema = z.object({ error_class: errorClassSchema }).strict() +// Why: daemon replace/retire lifecycle signal — issue #7936 was undiagnosable without asking a user for daemon.log. +// Enum-only + bucketed session count so no paths, raw versions, or exact counts reach the wire. +// The union keeps each reason pinned to its transition, so a death can't be reported as a replace. +const daemonLifecycleSchema = z.discriminatedUnion('transition', [ + z + .object({ + transition: z.literal(DAEMON_LIFECYCLE_TRANSITIONS[0]), + reason: z.enum(DAEMON_REPLACE_REASONS), + live_session_count_bucket: z.enum(DAEMON_LIFECYCLE_SESSION_BUCKETS) + }) + .strict(), + z + .object({ + transition: z.literal(DAEMON_LIFECYCLE_TRANSITIONS[1]), + reason: z.enum(DAEMON_RETIRE_REASONS), + live_session_count_bucket: z.enum(DAEMON_LIFECYCLE_SESSION_BUCKETS) + }) + .strict() +]) + // Rollout signal for granting Codex hook trust via codex app-server RPCs // instead of Orca's self-computed trusted_hash. `fallback`/`verify_failed` // spikes mean the RPC lane is not taking; steady-state ledger skips are not @@ -1300,6 +1326,7 @@ export const eventSchemas = { agent_hook_unattributed: agentHookUnattributedSchema, daemon_start_failed: daemonStartFailedSchema, + daemon_lifecycle: daemonLifecycleSchema, codex_trust_grant: codexTrustGrantSchema,