From 7ed70293ee6ecf1219abdc9d37df0c235a514f0d Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 27 Aug 2026 14:48:48 -0700 Subject: [PATCH] fix(daemon): record post-startup process exits --- .../daemon/daemon-child-exit-observer.test.ts | 61 ++++++++++++ src/main/daemon/daemon-child-exit-observer.ts | 93 +++++++++++++++++++ src/main/daemon/daemon-entry.ts | 11 +-- .../daemon-init-child-readiness.test.ts | 38 ++++++-- src/main/daemon/daemon-init.ts | 75 +++++++++------ 5 files changed, 235 insertions(+), 43 deletions(-) create mode 100644 src/main/daemon/daemon-child-exit-observer.test.ts create mode 100644 src/main/daemon/daemon-child-exit-observer.ts diff --git a/src/main/daemon/daemon-child-exit-observer.test.ts b/src/main/daemon/daemon-child-exit-observer.test.ts new file mode 100644 index 00000000000..1cd394131c9 --- /dev/null +++ b/src/main/daemon/daemon-child-exit-observer.test.ts @@ -0,0 +1,61 @@ +import { EventEmitter } from 'node:events' +import { PassThrough } from 'node:stream' +import { describe, expect, it, vi } from 'vitest' +import { observeDaemonChildExit } from './daemon-child-exit-observer' + +function createChild() { + const child = new EventEmitter() + const stderr = new PassThrough() as PassThrough & { unref: () => void } + stderr.unref = vi.fn<() => void>() + return Object.assign(child, { stderr }) +} + +describe('observeDaemonChildExit', () => { + it('reports an exact post-readiness exit with the bounded stderr tail', () => { + const child = createChild() + const onExit = vi.fn() + const observer = observeDaemonChildExit(child, onExit, 12) + + child.stderr.write('discarded startup\n') + observer.markReady() + child.stderr.write('prefix-FATAL') + child.emit('close', 134, null) + + expect(child.stderr.unref).toHaveBeenCalledOnce() + expect(onExit).toHaveBeenCalledWith({ + verdict: 'exited', + exitCode: 134, + signal: null, + stderrTail: 'prefix-FATAL' + }) + }) + + it('does not infer an exit from stderr or contact loss', () => { + const child = createChild() + const onExit = vi.fn() + const observer = observeDaemonChildExit(child, onExit) + + child.stderr.write('socket closed') + observer.markReady() + child.emit('disconnect') + child.stderr.emit('error', new Error('broken pipe')) + + expect(onExit).not.toHaveBeenCalled() + observer.stop() + }) + + it('stops observing and destroys the startup pipe on launch failure', () => { + const child = createChild() + const destroy = vi.spyOn(child.stderr, 'destroy') + const onExit = vi.fn() + const observer = observeDaemonChildExit(child, onExit) + + child.stderr.write('startup failure') + expect(observer.startupStderrTail()).toBe('startup failure') + observer.stop({ destroyStderr: true }) + child.emit('close', 1, null) + + expect(destroy).toHaveBeenCalledOnce() + expect(onExit).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/daemon/daemon-child-exit-observer.ts b/src/main/daemon/daemon-child-exit-observer.ts new file mode 100644 index 00000000000..94d9101884b --- /dev/null +++ b/src/main/daemon/daemon-child-exit-observer.ts @@ -0,0 +1,93 @@ +import type { ChildProcess } from 'node:child_process' + +const DEFAULT_STDERR_TAIL_BYTES = 8 * 1024 + +type UnrefableChildStderr = NonNullable & { + unref?: () => void +} + +type DaemonCloseListener = (exitCode: number | null, signal: NodeJS.Signals | null) => void + +type ObservableDaemonChild = { + stderr: UnrefableChildStderr | null + on(event: 'close', listener: DaemonCloseListener): unknown + off(event: 'close', listener: DaemonCloseListener): unknown +} + +export type DaemonChildExitObservation = { + verdict: 'exited' + exitCode: number | null + signal: NodeJS.Signals | null + stderrTail: string +} + +export type DaemonChildExitObserver = { + startupStderrTail(): string + markReady(): void + stop(options?: { destroyStderr?: boolean }): void +} + +export function observeDaemonChildExit( + child: ObservableDaemonChild, + onExit: (observation: DaemonChildExitObservation) => void, + maxStderrBytes = DEFAULT_STDERR_TAIL_BYTES +): DaemonChildExitObserver { + let stderrTail = Buffer.alloc(0) + let ready = false + let stopped = false + + const onStderr = (chunk: Buffer | string): void => { + const bytes = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk) + stderrTail = Buffer.concat([stderrTail, bytes]).subarray(-maxStderrBytes) + } + const onStderrError = (): void => {} + const onClose = (exitCode: number | null, signal: NodeJS.Signals | null): void => { + if (stopped || !ready) { + return + } + stopped = true + child.off('close', onClose) + child.stderr?.off('data', onStderr) + child.stderr?.off('error', onStderrError) + try { + onExit({ + verdict: 'exited', + exitCode, + signal, + stderrTail: stderrTail.toString('utf8').trim() + }) + } catch { + // Diagnostics must never escape daemon lifecycle handling. + } + } + + child.stderr?.on('data', onStderr) + + return { + startupStderrTail(): string { + return stderrTail.toString('utf8').trim() + }, + markReady(): void { + if (ready || stopped) { + return + } + ready = true + stderrTail = Buffer.alloc(0) + child.stderr?.on('error', onStderrError) + child.stderr?.unref?.() + child.on('close', onClose) + }, + stop(options = {}): void { + if (stopped) { + return + } + stopped = true + child.off('close', onClose) + child.stderr?.off('data', onStderr) + child.stderr?.off('error', onStderrError) + if (options.destroyStderr) { + child.stderr?.destroy() + } + } + } +} diff --git a/src/main/daemon/daemon-entry.ts b/src/main/daemon/daemon-entry.ts index 72537f64cbf..48144c626d8 100644 --- a/src/main/daemon/daemon-entry.ts +++ b/src/main/daemon/daemon-entry.ts @@ -102,11 +102,8 @@ export function parseArgs(argv: string[]): ParsedDaemonArgs { } async function main(): Promise { - // Why: the parent captures daemon startup stderr then destroys its end of the - // pipe once the daemon is ready. A later write here (e.g. the uncaughtException - // console.error below) would then hit a broken pipe and emit 'error' on - // process.stderr — with no listener that becomes an unhandled error that kills - // an otherwise healthy detached daemon. Swallow it: stderr is diagnostic only. + // Why: older parents destroy their stderr pipe after readiness, so diagnostic + // writes must not turn a broken pipe into a second fatal error. process.stderr.on('error', () => {}) const { @@ -127,8 +124,8 @@ async function main(): Promise { daemonLog.log('startup', { protocolVersion: PROTOCOL_VERSION, socketPath }) void warmPwshAvailabilityCache() - // Why: detached daemons destroy stderr, so the preflight's console.warn is lost; - // surface a degraded TCC attribution here where it's diagnosable (F2). + // Why: adopted daemons can lack a stderr observer, so surface degraded TCC + // attribution in the durable lifecycle log (F2). const runMacosLoginPreflight = async (): Promise => { const outcome = await prepareMacosTccLoginShell() if (outcome && !outcome.ok) { diff --git a/src/main/daemon/daemon-init-child-readiness.test.ts b/src/main/daemon/daemon-init-child-readiness.test.ts index 60d3c9c596d..b27af78a376 100644 --- a/src/main/daemon/daemon-init-child-readiness.test.ts +++ b/src/main/daemon/daemon-init-child-readiness.test.ts @@ -1,6 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { join } from 'node:path' import { PROTOCOL_VERSION } from './types' -import { FAKE_DAEMON_ENTRY_PATH } from './daemon-init-test-harness' +import { FAKE_DAEMON_ENTRY_PATH, FAKE_USER_DATA_PATH } from './daemon-init-test-harness' const { writeFileSyncMock, @@ -156,7 +157,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { ) }) - it('destroys the daemon stderr pipe once the daemon signals ready', async () => { + it('keeps daemon stderr observable after readiness without retaining Electron', async () => { const mod = await importFresh() checkDaemonHealthMock.mockResolvedValue('unreachable') await mod.initDaemonPtyProvider() @@ -168,16 +169,23 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { const handlers: Record void)[]> = { message: [], error: [], - exit: [] + exit: [], + close: [] } const stderrOff = vi.fn() const stderrDestroy = vi.fn() + const stderrUnref = vi.fn() + const stderrDataHandlers: ((chunk: Buffer) => void)[] = [] const stderr = { - on() { + on(event: string, callback: (chunk: Buffer) => void) { + if (event === 'data') { + stderrDataHandlers.push(callback) + } return this }, off: stderrOff, - destroy: stderrDestroy + destroy: stderrDestroy, + unref: stderrUnref } const child = { pid: 12345, @@ -201,10 +209,26 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { await launcher('/fake/socket', '/fake/token') - expect(stderrOff).toHaveBeenCalledWith('data', expect.any(Function)) - expect(stderrDestroy).toHaveBeenCalledOnce() + expect(stderrOff).not.toHaveBeenCalledWith('data', expect.any(Function)) + expect(stderrDestroy).not.toHaveBeenCalled() + expect(stderrUnref).toHaveBeenCalledOnce() expect(child.disconnect).toHaveBeenCalledOnce() expect(child.unref).toHaveBeenCalledOnce() + + for (const handler of stderrDataHandlers) { + handler(Buffer.from('FATAL ERROR: heap limit reached')) + } + for (const handler of handlers.close) { + handler(134) + } + + expect(writeFileSyncMock).toHaveBeenCalledWith( + join(FAKE_USER_DATA_PATH, 'logs', 'daemon.log'), + expect.stringContaining('"event":"process-exit-observed","verdict":"exited","exitCode":134'), + { flag: 'a', mode: 0o600 } + ) + expect(writeFileSyncMock.mock.calls.at(-1)?.[1]).toContain('"launchNonce":') + expect(writeFileSyncMock.mock.calls.at(-1)?.[1]).toContain('FATAL ERROR: heap limit reached') }) it('accepts the daemon self-reported start time when the OS query returns null', async () => { diff --git a/src/main/daemon/daemon-init.ts b/src/main/daemon/daemon-init.ts index 77edbed6027..8d1c06dfa5c 100644 --- a/src/main/daemon/daemon-init.ts +++ b/src/main/daemon/daemon-init.ts @@ -3,7 +3,7 @@ restart, teardown); the "swap the provider atomically" invariant keeps restart + import { join } from 'node:path' import { randomUUID } from 'node:crypto' import { getAppEnvironment } from '../../shared/app-environment' -import { mkdirSync, existsSync, readFileSync, unlinkSync } from 'node:fs' +import { mkdirSync, existsSync, readFileSync, unlinkSync, writeFileSync } from 'node:fs' import { fork, type ChildProcess } from 'node:child_process' import { connect } from 'node:net' import { @@ -59,6 +59,10 @@ import { } from '../claude-accounts/live-pty-gate' import { parseDaemonReadyIdentity, readDaemonProcessIncarnation } from './daemon-ready-identity' import type { DaemonEndpointIdentity } from './daemon-hello-protocol' +import { + observeDaemonChildExit, + type DaemonChildExitObservation +} from './daemon-child-exit-observer' // Why: daemon init runs concurrent with window load, so an in-process t timestamp (not harness stderr timing) measures cold-start. function logDaemonMilestone(event: string, details: Record = {}): void { @@ -135,14 +139,45 @@ function resolvePackagedDarwinAppVersion(): string | null { } // Why: pass a log-file arg so field failures are diagnosable, but honor the ORCA_DIAGNOSTICS_DISABLED privacy switch. -function daemonLogArgs(): string[] { +function daemonDiagnosticsDisabled(): boolean { const disabled = (process.env.ORCA_DIAGNOSTICS_DISABLED ?? '').trim().toLowerCase() - if (disabled === '1' || disabled === 'true') { + return disabled === '1' || disabled === 'true' +} + +function daemonLogArgs(): string[] { + if (daemonDiagnosticsDisabled()) { return [] } return ['--log-file', getDaemonLogFilePath()] } +function recordDaemonChildExit( + identity: DaemonEndpointIdentity | null, + observation: DaemonChildExitObservation +): void { + console.warn( + `[daemon] Daemon process exit observed (pid=${identity?.pid ?? 'unknown'}, code=${observation.exitCode ?? 'null'}, signal=${observation.signal ?? 'null'})${observation.stderrTail ? `\n${observation.stderrTail}` : ''}` + ) + if (daemonDiagnosticsDisabled()) { + return + } + try { + writeFileSync( + getDaemonLogFilePath(), + `${JSON.stringify({ + src: 'daemon-launcher', + ts: new Date().toISOString(), + ...identity, + event: 'process-exit-observed', + ...observation + })}\n`, + { flag: 'a', mode: 0o600 } + ) + } catch { + // Diagnostics are fail-open. + } +} + // Why: a socket that accepts a connection proves a daemon survived a previous app session and can be reused. function probeSocket(socketPath: string): Promise { return new Promise((resolve) => { @@ -700,7 +735,7 @@ function createOutOfProcessLauncher( { // Why: detached daemons outlive dev worktrees; userData keeps process.cwd() valid after a repo/worktree is deleted. cwd: userDataPath, - // Why: detached+unref outlives Electron; stdout 'ignore' (else blocks exit), stderr 'pipe' captures startup crashes lost in v1.4.129-rc.1. + // Why: detached+unref outlives Electron; stdout is ignored, while an unrefed stderr pipe retains a bounded fatal-error tail. detached: true, stdio: ['ignore', 'ignore', 'pipe', 'ipc'], // Why: run the byte-identical relocated Orca.exe so the image path sits outside the updater's kill zone. @@ -715,29 +750,11 @@ function createOutOfProcessLauncher( } ) - // Why: keep only the startup-window stderr tail so a crash cause is visible without unbounded memory. - const STARTUP_STDERR_MAX_BYTES = 8192 - let startupStderr = '' - let collectingStderr = true - const onStartupStderr = (chunk: Buffer): void => { - if (!collectingStderr) { - return - } - startupStderr += chunk.toString('utf8') - if (startupStderr.length > STARTUP_STDERR_MAX_BYTES) { - startupStderr = startupStderr.slice(-STARTUP_STDERR_MAX_BYTES) - } - } - child.stderr?.on('data', onStartupStderr) - // Why: release the detached daemon's stderr once up/failed — a live piped stream refs the parent loop and blocks Electron exit. - const releaseStderr = (): void => { - collectingStderr = false - child.stderr?.off('data', onStartupStderr) - child.stderr?.destroy() - } - // Wait for the daemon to signal readiness via IPC let launchedIdentity: DaemonEndpointIdentity | null = null + const childExitObserver = observeDaemonChildExit(child, (observation) => + recordDaemonChildExit(launchedIdentity, observation) + ) let endpointUnavailableReason: string | null = null const startupSignal = new Promise((resolve, reject) => { let timer: ReturnType | undefined @@ -757,11 +774,11 @@ function createOutOfProcessLauncher( settled = true cleanupStartupListeners() // Why: attach the captured stderr tail to the thrown error and log it so a startup crash isn't just "exited with code 1". - const stderrTail = startupStderr.trim() + const stderrTail = childExitObserver.startupStderrTail() if (stderrTail) { console.warn(`[daemon] startup failed; captured stderr tail:\n${stderrTail}`) } - releaseStderr() + childExitObserver.stop({ destroyStderr: true }) const startupError = stderrTail ? new Error(`${error.message}\nDaemon stderr (tail):\n${stderrTail}`) : error @@ -810,8 +827,8 @@ function createOutOfProcessLauncher( settled = true // Why: daemon is detached after readiness; detach startup listeners so the launch promise closure isn't retained. cleanupStartupListeners() - // Why: release IPC/stderr and unref so Electron can exit without waiting; the daemon keeps running detached. - releaseStderr() + // Why: keep a bounded stderr tail while unrefing its pipe so later fatal output is observable without blocking Electron exit. + childExitObserver.markReady() child.disconnect() child.unref() resolve()