diff --git a/src/main/app-relaunch.test.ts b/src/main/app-relaunch.test.ts index 26c98c98bb3..7afc9f251eb 100644 --- a/src/main/app-relaunch.test.ts +++ b/src/main/app-relaunch.test.ts @@ -10,7 +10,7 @@ vi.mock('./crash-reporting/durable-crash-breadcrumb', () => ({ recordDurableCrashBreadcrumb: recordDurableCrashBreadcrumbMock })) -import { relaunchApp } from './app-relaunch' +import { isAppRelaunchRequested, relaunchApp } from './app-relaunch' beforeEach(() => { appRelaunchMock.mockReset() @@ -19,8 +19,11 @@ beforeEach(() => { describe('relaunchApp', () => { it('durably records the reason before scheduling the replacement process', () => { + expect(isAppRelaunchRequested()).toBe(false) + relaunchApp('gpu-fallback', { processReason: 'crashed', exitCode: 5 }) + expect(isAppRelaunchRequested()).toBe(true) expect(recordDurableCrashBreadcrumbMock).toHaveBeenCalledOnce() expect(recordDurableCrashBreadcrumbMock).toHaveBeenCalledWith('app_relaunch_requested', { processReason: 'crashed', diff --git a/src/main/app-relaunch.ts b/src/main/app-relaunch.ts index ab7a725c847..0d2844f1029 100644 --- a/src/main/app-relaunch.ts +++ b/src/main/app-relaunch.ts @@ -9,7 +9,14 @@ export type AppRelaunchReason = | 'profile-transfer' | 'renderer-request' +let relaunchRequested = false + +export function isAppRelaunchRequested(): boolean { + return relaunchRequested +} + export function relaunchApp(reason: AppRelaunchReason, data?: CrashReportBreadcrumbData): void { + relaunchRequested = true // Why: the current process can exit immediately after app.relaunch(), so // persist the cause before Electron schedules the replacement process. recordDurableCrashBreadcrumb('app_relaunch_requested', { ...data, reason }) diff --git a/src/main/daemon/daemon-health.ts b/src/main/daemon/daemon-health.ts index 2989f9b2268..e6cae9921d4 100644 --- a/src/main/daemon/daemon-health.ts +++ b/src/main/daemon/daemon-health.ts @@ -648,42 +648,13 @@ export async function killStaleDaemon( let killedDaemon = false try { const parsedPid = parseDaemonPidFile(readFileSync(pidPath, 'utf8')) - if ( - parsedPid && - (await isDaemonProcess(parsedPid.pid, socketPath, tokenPath, parsedPid.startedAtMs)) - ) { - const { pid, startedAtMs } = parsedPid - process.kill(pid, 'SIGTERM') - const deadline = Date.now() + KILL_WAIT_MS - let exited = false - while (Date.now() < deadline) { - try { - process.kill(pid, 0) - } catch { - exited = true - break - } - await new Promise((resolve) => setTimeout(resolve, KILL_POLL_MS)) - } - if (!exited) { - // Why: re-check process identity before SIGKILL. The SIGTERM-then-wait - // window is long enough for the pid to be recycled if the original - // daemon died during the wait. Without this, we'd SIGKILL an unrelated - // process that happens to now own the same pid. - if (!(await isDaemonProcess(pid, socketPath, tokenPath, startedAtMs))) { - console.warn('[daemon] Skipping SIGKILL for stale daemon: reason=pid_recycled') - exited = true - killedDaemon = true - } else { - try { - process.kill(pid, 'SIGKILL') - exited = true - } catch { - // Already dead - } - } - } - killedDaemon = killedDaemon || exited + if (parsedPid) { + killedDaemon = await terminateDaemonProcessIdentity( + parsedPid.pid, + socketPath, + tokenPath, + parsedPid.startedAtMs + ) } } catch { // PID file missing or process already dead @@ -705,3 +676,35 @@ export async function killStaleDaemon( } return killedDaemon } + +export async function terminateDaemonProcessIdentity( + pid: number, + socketPath: string, + tokenPath: string, + startedAtMs: number | null +): Promise { + if (!(await isDaemonProcess(pid, socketPath, tokenPath, startedAtMs))) { + return false + } + process.kill(pid, 'SIGTERM') + const deadline = Date.now() + KILL_WAIT_MS + while (Date.now() < deadline) { + try { + process.kill(pid, 0) + } catch { + return true + } + await new Promise((resolve) => setTimeout(resolve, KILL_POLL_MS)) + } + // Why: the graceful wait is long enough for PID reuse; re-verify before force-killing. + if (!(await isDaemonProcess(pid, socketPath, tokenPath, startedAtMs))) { + console.warn('[daemon] Skipping SIGKILL for stale daemon: reason=pid_recycled') + return true + } + try { + process.kill(pid, 'SIGKILL') + } catch { + // Already dead + } + return true +} diff --git a/src/main/daemon/daemon-init.test.ts b/src/main/daemon/daemon-init.test.ts index 77881045cb0..6c9e1d083f0 100644 --- a/src/main/daemon/daemon-init.test.ts +++ b/src/main/daemon/daemon-init.test.ts @@ -95,7 +95,9 @@ const { const daemonClientMock = vi.fn().mockImplementation(function MockDaemonClient() { return { ensureConnected: vi.fn(async () => {}), + ensureConnectedWithin: vi.fn(async () => {}), request: vi.fn(async () => ({ sessions: [] })), + hasObservedAuthenticatedDisconnect: vi.fn(() => false), disconnect: vi.fn() } }) @@ -426,7 +428,9 @@ async function importFresh() { daemonClientMock.mockImplementation(function MockDaemonClient() { return { ensureConnected: vi.fn(async () => {}), + ensureConnectedWithin: vi.fn(async () => {}), request: vi.fn(async () => ({ sessions: [] })), + hasObservedAuthenticatedDisconnect: vi.fn(() => false), disconnect: vi.fn() } }) @@ -524,6 +528,29 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { ) }) + it('fences daemon initialization when shutdown starts before publication', async () => { + const mod = await importFresh() + let finishEnsureRunning!: (value: { socketPath: string; tokenPath: string }) => void + ensureRunningOverrides.push( + () => + new Promise((resolve) => { + finishEnsureRunning = resolve + }) + ) + const initialization = mod.initDaemonPtyProvider().catch((error: unknown) => error) + await vi.waitFor(() => expect(spawnerInstances).toHaveLength(1)) + + const shutdown = mod.shutdownDaemon() + finishEnsureRunning({ socketPath: '/fake/pending-socket', tokenPath: '/fake/pending-token' }) + + await shutdown + expect(await initialization).toEqual( + expect.objectContaining({ message: expect.stringContaining('interrupted by shutdown') }) + ) + expect(spawnerInstances[0].shutdown).toHaveBeenCalled() + expect(setLocalPtyProviderMock).not.toHaveBeenCalled() + }) + it('uses daemon-owned idle retirement when a fresh launch fails permanent adoption', async () => { const mod = await importFresh() ensureRunningOverrides.push(async () => ({ @@ -638,6 +665,32 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { expect(mod.getDaemonProvider()).toBeNull() }) + it('starts legacy teardown before stalled startup discovery settles', async () => { + const mod = await importFresh() + probeSocketExistsMock.mockImplementation((p?: string) => p?.endsWith('daemon-v9.sock') ?? false) + mockOnlyDaemonSocketAlive('daemon-v9.sock') + let resolveDiscovery!: (sessions: { sessionId: string }[]) => void + listProcessesControl.current = () => + new Promise((resolve) => { + resolveDiscovery = resolve + }) + const started = mod.initDaemonPtyProvider().catch((error: unknown) => error) + await vi.waitFor(() => expect(resolveDiscovery).toBeTypeOf('function')) + const clientsBeforeShutdown = daemonClientMock.mock.calls.length + + const shutdown = mod.shutdownDaemon() + await vi.waitFor(() => + expect(daemonClientMock.mock.calls.length).toBeGreaterThan(clientsBeforeShutdown) + ) + resolveDiscovery([]) + + await shutdown + expect(await started).toEqual( + expect.objectContaining({ message: expect.stringContaining('interrupted by shutdown') }) + ) + expect(mod.getDaemonProvider()).toBeNull() + }) + it('retains every adapter cleanup failure when legacy router setup aborts', async () => { const mod = await importFresh() probeSocketExistsMock.mockImplementation( @@ -788,6 +841,32 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { expect(originalSpawner.ensureRunning).toHaveBeenCalledTimes(2) }) + it('prevents an in-progress manual restart from publishing after shutdown', async () => { + const mod = await importFresh() + await mod.initDaemonPtyProvider() + const originalProvider = mod.getDaemonProvider() + let finishRestartLaunch!: (value: { socketPath: string; tokenPath: string }) => void + ensureRunningOverrides.push( + () => + new Promise((resolve) => { + finishRestartLaunch = resolve + }) + ) + const restart = mod.restartDaemon().catch((error: unknown) => error) + await vi.waitFor(() => expect(spawnerInstances[0].ensureRunning).toHaveBeenCalledTimes(2)) + + const shutdown = mod.shutdownDaemon() + finishRestartLaunch({ socketPath: '/fake/restart-race', tokenPath: '/fake/restart-token' }) + + await shutdown + expect(await restart).toEqual( + expect.objectContaining({ message: expect.stringContaining('interrupted by shutdown') }) + ) + expect(mod.getDaemonProvider()).toBeNull() + expect(setLocalPtyProviderMock).toHaveBeenCalledOnce() + expect(setLocalPtyProviderMock).toHaveBeenCalledWith(originalProvider) + }) + it('builds a fresh adapter whose respawn callback closes over the same spawner', async () => { const mod = await importFresh() await mod.initDaemonPtyProvider() @@ -1027,6 +1106,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { return { ensureConnected: ensureConnectedMock, request: requestMock, + hasObservedAuthenticatedDisconnect: vi.fn(() => false), disconnect: disconnectMock } }) diff --git a/src/main/daemon/daemon-init.ts b/src/main/daemon/daemon-init.ts index 24fc9ef4dd2..5ebae5d5988 100644 --- a/src/main/daemon/daemon-init.ts +++ b/src/main/daemon/daemon-init.ts @@ -31,7 +31,8 @@ import { checkDaemonHealth, isDaemonStaleForCurrentBundle, killStaleDaemon, - parseDaemonPidFile + parseDaemonPidFile, + terminateDaemonProcessIdentity } from './daemon-health' import { collectPinnedDaemonVersions, @@ -53,6 +54,10 @@ import { confirmSeededClaudeLivePtys, hasSeededUnconfirmedClaudePtys } from '../claude-accounts/live-pty-gate' +import { + runGracefulDaemonShutdownWithFallback, + shutdownAdoptedDaemonGenerations +} from './daemon-quit-teardown' // 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 { @@ -66,11 +71,17 @@ export const WEDGED_DAEMON_GRACE_RETRIES = 11 const DAEMON_SELF_SHUTDOWN_WAIT_MS = 5_000 const DAEMON_CHILD_TERMINATION_GRACE_MS = 5_000 const DAEMON_CHILD_FORCE_EXIT_WAIT_MS = 1_000 +const DAEMON_QUIT_RPC_TIMEOUT_MS = 1_500 +const DAEMON_QUIT_FALLBACK_DELAY_MS = 8_000 let spawner: DaemonSpawner | null = null type DaemonProvider = DaemonPtyRouter | DaemonPtyAdapter | DegradedDaemonPtyProvider let adapter: DaemonProvider | null = null +let initializationInFlight: Promise | null = null +let initializingSpawner: DaemonSpawner | null = null +let initializingLegacyProtocolVersions: number[] = [] +let daemonShutdownRequested = false // Why: coalesce concurrent restartDaemon() calls so two entries can't race the 7-step sequence against a half-spawned replacement. let restartInFlight: Promise | null = null @@ -694,6 +705,24 @@ function createOutOfProcessLauncher( export async function initDaemonPtyProvider( signal?: AbortSignal, options: { macosLoginSessionWatch?: boolean } = {} +): Promise { + if (daemonShutdownRequested) { + throw new Error('Daemon initialization was interrupted by shutdown') + } + const initialization = runDaemonPtyProviderInitialization(signal, options) + initializationInFlight = initialization + try { + await initialization + } finally { + if (initializationInFlight === initialization) { + initializationInFlight = null + } + } +} + +async function runDaemonPtyProviderInitialization( + signal?: AbortSignal, + options: { macosLoginSessionWatch?: boolean } = {} ): Promise { logDaemonMilestone('daemon-init-start') // Why: e2e coverage for the startup PTY gate (#5232) needs a daemon init that deterministically outlasts the first-window timeout. @@ -701,15 +730,23 @@ export async function initDaemonPtyProvider( if (Number.isFinite(e2eInitDelayMs) && e2eInitDelayMs > 0) { await new Promise((resolve) => setTimeout(resolve, e2eInitDelayMs)) } + if (daemonShutdownRequested) { + throw new Error('Daemon initialization was interrupted by shutdown') + } const runtimeDir = getRuntimeDir() const newSpawner = new DaemonSpawner({ runtimeDir, launcher: createOutOfProcessLauncher(runtimeDir, options.macosLoginSessionWatch ?? false) }) + initializingSpawner = newSpawner // Why: assign the module-level spawner/adapter only after both succeed, so a failed ensureRunning() leaves no stale spawner. const info = await newSpawner.ensureRunning() + if (daemonShutdownRequested) { + await newSpawner.shutdown() + throw new Error('Daemon initialization was interrupted by shutdown') + } // Why: reclaim superseded daemon-host copies on EVERY launch (spawns are rare), keeping current + live-daemon-pinned versions. pruneOldDaemonHosts(collectPinnedDaemonVersions(runtimeDir)) const launchMode = newSpawner.getHandle()?.mode @@ -731,6 +768,7 @@ export async function initDaemonPtyProvider( historyPath: getHistoryDir(), // Why: on daemon death, ensureConnected() detects the dead socket and calls this to fork a replacement before retrying. respawn: async (reason: DaemonRespawnReason) => { + throwIfDaemonShutdownRequested() // 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 @@ -759,6 +797,10 @@ export async function initDaemonPtyProvider( releaseDaemonAdoptionLease(newSpawner.getHandle()) legacyAdapters = await createLegacyDaemonAdapters(runtimeDir) + initializingLegacyProtocolVersions = legacyAdapters.map((legacy) => legacy.protocolVersion) + if (daemonShutdownRequested) { + throw new Error('Daemon initialization was interrupted by shutdown') + } routedAdapter = launchMode === 'degraded-new-pty-fallback' ? new DegradedDaemonPtyProvider({ @@ -778,12 +820,21 @@ export async function initDaemonPtyProvider( } else if (routedAdapter instanceof DaemonPtyRouter) { await routedAdapter.discoverLegacySessions() } + throwIfDaemonShutdownRequested() if (signal?.aborted) { // Why: same late-swap guard after legacy discovery; release uninstalled adapter leases without killing live sessions. await routedAdapter.disconnectOnly() return } } catch (error) { + if (daemonShutdownRequested) { + releaseDaemonAdoptionLease(newSpawner.getHandle()) + newAdapter.dispose() + for (const legacy of legacyAdapters) { + legacy.dispose() + } + throw error + } try { await cleanupFailedDaemonAdoption(newSpawner, newAdapter, legacyAdapters) } catch (cleanupError) { @@ -798,6 +849,10 @@ export async function initDaemonPtyProvider( rebindLocalProviderListeners() logDaemonMilestone('daemon-init-done', { legacyAdapters: legacyAdapters.length }) await reconcileSeededClaudeLivePtys(routedAdapter) + if (initializingSpawner === newSpawner) { + initializingSpawner = null + initializingLegacyProtocolVersions = [] + } } // Why: release gate ids only for daemon-confirmed-dead sessions; keep seeds on listing failure since releasing early can rotate a live CLI's refresh token. @@ -867,6 +922,7 @@ export type RestartDaemonResult = { // Why: the 7-step restart sequence from docs/daemon-staleness-ux.md §Phase 1; current-protocol only (legacy adapters preserved). export async function restartDaemon(): Promise { + throwIfDaemonShutdownRequested() if (restartInFlight) { return restartInFlight } @@ -877,6 +933,7 @@ export async function restartDaemon(): Promise { } async function runRestartDaemon(): Promise { + throwIfDaemonShutdownRequested() const currentSpawner = spawner const currentAdapter = adapter if (!currentSpawner || !currentAdapter) { @@ -911,10 +968,12 @@ async function runRestartDaemon(): Promise { let info: Awaited> try { await cleanupDaemonForProtocol(runtimeDir, PROTOCOL_VERSION) + throwIfDaemonShutdownRequested() // Step 4: reuse the existing spawner so the respawn closure baked into long-lived adapters stays valid (do NOT new one). currentSpawner.resetHandle() info = await currentSpawner.ensureRunning() + throwIfDaemonShutdownRequested() } catch (error) { // Why: old provider stays authoritative until the final swap; rebind since relaunch failed after teardown. rebindLocalProviderListeners() @@ -927,6 +986,7 @@ async function runRestartDaemon(): Promise { tokenPath: info.tokenPath, historyPath: getHistoryDir(), respawn: async (reason: DaemonRespawnReason) => { + throwIfDaemonShutdownRequested() // 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 @@ -951,6 +1011,7 @@ async function runRestartDaemon(): Promise { try { // Temporary launcher lease overlaps this permanent pair so a manual restart can't strand a newly spawned daemon during adoption. await newCurrent.establishLifecycleLease() + throwIfDaemonShutdownRequested() releaseDaemonAdoptionLease(currentSpawner.getHandle()) // Re-wrap in a router only if legacy adapters exist; they're preserved by reference and still route to their pre-upgrade daemons. @@ -960,8 +1021,16 @@ async function runRestartDaemon(): Promise { : newCurrent if (newProvider instanceof DaemonPtyRouter) { await newProvider.discoverLegacySessions() + throwIfDaemonShutdownRequested() } } catch (error) { + if (daemonShutdownRequested) { + if (newProvider instanceof DaemonPtyRouter) { + newProvider.disposeRouterOnly() + } + newCurrent.dispose() + throw error + } let cleanupError: unknown try { if (newProvider instanceof DaemonPtyRouter) { @@ -980,6 +1049,7 @@ async function runRestartDaemon(): Promise { } // Drain the old router's subscriptions via the router-only variant (plain dispose() would tear down the shared legacy adapters), after the new provider exists (no unhandled events) and before the swap (atomic for the renderer). + throwIfDaemonShutdownRequested() disposeProviderSubscriptionsOnly(currentAdapter) // Step 6: swap module state (adapter + localProvider) atomically. @@ -991,6 +1061,12 @@ async function runRestartDaemon(): Promise { return { killedCount } } +function throwIfDaemonShutdownRequested(): void { + if (daemonShutdownRequested) { + throw new Error('Daemon operation was interrupted by shutdown') + } +} + // Disconnect without killing: the daemon survives app quit so sessions stay warm for reattach. // Leave history sessions marked "unclean" so a daemon crash while Orca is closed stays recoverable. export async function disconnectDaemon(): Promise { @@ -1000,10 +1076,111 @@ export async function disconnectDaemon(): Promise { /** Kill the daemon and all its sessions. Use for full cleanup only. */ export async function shutdownDaemon(): Promise { - adapter?.dispose() + daemonShutdownRequested = true + const pendingInitialization = initializationInFlight + const pendingRestart = restartInFlight + const runtimeDir = getRuntimeDir() + const eagerLegacyProtocolVersions = [ + ...(adapter && !(adapter instanceof DaemonPtyAdapter) + ? adapter.getLegacyAdapters().map((legacy) => legacy.protocolVersion) + : []), + ...initializingLegacyProtocolVersions + ] + const legacyShutdowns = new Map>() + for (const protocolVersion of new Set(eagerLegacyProtocolVersions)) { + legacyShutdowns.set(protocolVersion, shutdownDaemonProtocolAndWait(runtimeDir, protocolVersion)) + } + const currentSpawner = spawner ?? initializingSpawner + const currentShutdown = currentSpawner + ? (async () => { + try { + await shutdownDaemonProtocolAndWait(runtimeDir, PROTOCOL_VERSION) + } finally { + await currentSpawner.shutdown() + } + })() + : undefined + + await Promise.all([pendingInitialization?.catch(() => {}), pendingRestart?.catch(() => {})]) + const ownedAdapter = adapter + const legacyProtocolVersions = + ownedAdapter && !(ownedAdapter instanceof DaemonPtyAdapter) + ? ownedAdapter.getLegacyAdapters().map((legacy) => legacy.protocolVersion) + : [] + legacyProtocolVersions.push(...initializingLegacyProtocolVersions) adapter = null - await spawner?.shutdown() + ownedAdapter?.dispose() spawner = null + initializingSpawner = null + await shutdownAdoptedDaemonGenerations({ + ...(currentShutdown + ? { + shutdownCurrent: () => currentShutdown + } + : {}), + legacyProtocolVersions, + shutdownLegacy: async (protocolVersion) => { + let shutdown = legacyShutdowns.get(protocolVersion) + if (!shutdown) { + shutdown = shutdownDaemonProtocolAndWait(runtimeDir, protocolVersion) + legacyShutdowns.set(protocolVersion, shutdown) + } + await shutdown + } + }) +} + +async function shutdownDaemonProtocolAndWait( + runtimeDir: string, + protocolVersion: number +): Promise { + const socketPath = getDaemonSocketPath(runtimeDir, protocolVersion) + const tokenPath = getDaemonTokenPath(runtimeDir, protocolVersion) + let identity: + | (NonNullable> & { launchNonce: string | null }) + | null = null + try { + const pidContents = readFileSync(getDaemonPidPath(runtimeDir, protocolVersion), 'utf8') + const parsed = parseDaemonPidFile(pidContents) + if (parsed) { + const pidRecord = JSON.parse(pidContents) as { launchNonce?: unknown } + identity = { + ...parsed, + launchNonce: typeof pidRecord.launchNonce === 'string' ? pidRecord.launchNonce : null + } + } + } catch { + // The spawner handle still provides a current-generation fallback. + } + const graceful = async (): Promise => { + await cleanupDaemonForProtocol(runtimeDir, protocolVersion, { + rpcTimeoutMs: DAEMON_QUIT_RPC_TIMEOUT_MS + }) + } + if (!identity) { + await graceful() + return + } + await runGracefulDaemonShutdownWithFallback({ + graceful, + fallback: async () => { + await terminateDaemonProcessIdentity( + identity.pid, + socketPath, + tokenPath, + identity.startedAtMs + ) + }, + fallbackDelayMs: DAEMON_QUIT_FALLBACK_DELAY_MS + }) + await terminateDaemonProcessIdentity(identity.pid, socketPath, tokenPath, identity.startedAtMs) + if (identity.launchNonce) { + unlinkOwnedDaemonPidFile( + getDaemonPidPath(runtimeDir, protocolVersion), + identity.pid, + identity.launchNonce + ) + } } export type OrphanedDaemonCleanupResult = { @@ -1015,7 +1192,8 @@ export type OrphanedDaemonCleanupResult = { export async function cleanupDaemonForProtocol( runtimeDir: string, - protocolVersion: number + protocolVersion: number, + options: { rpcTimeoutMs?: number } = {} ): Promise { const socketPath = getDaemonSocketPath(runtimeDir, protocolVersion) const tokenPath = getDaemonTokenPath(runtimeDir, protocolVersion) @@ -1048,16 +1226,29 @@ export async function cleanupDaemonForProtocol( let didRequestShutdown = false let didKillStaleDaemon = false try { - await client.ensureConnected() - const sessions = await client - .request('listSessions', undefined) - .catch(() => ({ sessions: [] })) + await (options.rpcTimeoutMs === undefined + ? client.ensureConnected() + : client.ensureConnectedWithin(options.rpcTimeoutMs)) + const sessionsRequest = + options.rpcTimeoutMs === undefined + ? client.request('listSessions', undefined) + : client.request('listSessions', undefined, options.rpcTimeoutMs) + const sessions = await sessionsRequest.catch(() => ({ sessions: [] })) killedCount = sessions.sessions.filter((s) => s.isAlive).length // Use the single-shot `shutdown` RPC (kills all sessions then exits) to avoid racing per-session `kill` calls against the daemon exiting. - await client.request('shutdown', { killSessions: true }).catch(() => { - // Daemon exits immediately after the RPC, so the socket may close before the reply arrives; treat as success. - }) + const shutdownRequest = + options.rpcTimeoutMs === undefined + ? client.request('shutdown', { killSessions: true }) + : client.request('shutdown', { killSessions: true }, options.rpcTimeoutMs) + try { + await shutdownRequest + } catch (error) { + // A post-auth socket close is the expected no-reply exit; timeouts still require PID cleanup. + if (!client.hasObservedAuthenticatedDisconnect()) { + throw error + } + } didRequestShutdown = true } catch { // Previous-protocol daemons may be wedged or too old for the RPC path; fall back to PID cleanup (only unlinks a live socket after proving the process is killed). diff --git a/src/main/daemon/daemon-quit-teardown.test.ts b/src/main/daemon/daemon-quit-teardown.test.ts new file mode 100644 index 00000000000..88654ff1cd0 --- /dev/null +++ b/src/main/daemon/daemon-quit-teardown.test.ts @@ -0,0 +1,117 @@ +import { describe, expect, it, vi } from 'vitest' +import { + resolveDaemonQuitMode, + runGracefulDaemonShutdownWithFallback, + shutdownAdoptedDaemonGenerations +} from './daemon-quit-teardown' + +describe('resolveDaemonQuitMode', () => { + const ordinaryQuit = { + updateQuitInProgress: false, + relaunchRequested: false, + devParentShutdownRequested: false + } + + it('shuts down daemons for an ordinary Windows quit', () => { + expect(resolveDaemonQuitMode({ ...ordinaryQuit, platform: 'win32' })).toBe('shutdown') + }) + + it('preserves warm reattach for Windows updates and intentional relaunches', () => { + expect( + resolveDaemonQuitMode({ ...ordinaryQuit, platform: 'win32', updateQuitInProgress: true }) + ).toBe('disconnect') + expect( + resolveDaemonQuitMode({ ...ordinaryQuit, platform: 'win32', relaunchRequested: true }) + ).toBe('disconnect') + }) + + it('keeps normal macOS and Linux quit behavior unchanged', () => { + expect(resolveDaemonQuitMode({ ...ordinaryQuit, platform: 'darwin' })).toBe('disconnect') + expect(resolveDaemonQuitMode({ ...ordinaryQuit, platform: 'linux' })).toBe('disconnect') + }) + + it('shuts down an ownerless dev daemon on every platform', () => { + expect( + resolveDaemonQuitMode({ + ...ordinaryQuit, + platform: 'darwin', + updateQuitInProgress: true, + relaunchRequested: true, + devParentShutdownRequested: true + }) + ).toBe('shutdown') + }) +}) + +describe('shutdownAdoptedDaemonGenerations', () => { + it('shuts down current and each discovered legacy generation once', async () => { + const shutdownCurrent = vi.fn(async () => {}) + const shutdownLegacy = vi.fn(async () => {}) + + await shutdownAdoptedDaemonGenerations({ + shutdownCurrent, + legacyProtocolVersions: [21, 22, 21], + shutdownLegacy + }) + + expect(shutdownCurrent).toHaveBeenCalledOnce() + expect(shutdownLegacy.mock.calls).toEqual([[21], [22]]) + }) + + it('attempts every generation before reporting failures', async () => { + const shutdownCurrent = vi.fn(async () => { + throw new Error('current failed') + }) + const shutdownLegacy = vi.fn(async (protocolVersion: number) => { + if (protocolVersion === 21) { + throw new Error('legacy failed') + } + }) + + await expect( + shutdownAdoptedDaemonGenerations({ + shutdownCurrent, + legacyProtocolVersions: [21, 22], + shutdownLegacy + }) + ).rejects.toThrow('Daemon generation shutdown failed') + expect(shutdownLegacy.mock.calls).toEqual([[21], [22]]) + }) +}) + +describe('runGracefulDaemonShutdownWithFallback', () => { + it('starts the fallback on schedule when graceful RPC stalls', async () => { + vi.useFakeTimers() + let finishGraceful!: () => void + const graceful = vi.fn(() => new Promise((resolve) => (finishGraceful = resolve))) + const fallback = vi.fn(async () => {}) + const shutdown = runGracefulDaemonShutdownWithFallback({ + graceful, + fallback, + fallbackDelayMs: 8_000 + }) + + await vi.advanceTimersByTimeAsync(7_999) + expect(fallback).not.toHaveBeenCalled() + await vi.advanceTimersByTimeAsync(1) + expect(fallback).toHaveBeenCalledOnce() + finishGraceful() + await shutdown + vi.useRealTimers() + }) + + it('cancels the fallback after graceful shutdown succeeds', async () => { + vi.useFakeTimers() + const fallback = vi.fn(async () => {}) + + await runGracefulDaemonShutdownWithFallback({ + graceful: async () => {}, + fallback, + fallbackDelayMs: 8_000 + }) + await vi.runAllTimersAsync() + + expect(fallback).not.toHaveBeenCalled() + vi.useRealTimers() + }) +}) diff --git a/src/main/daemon/daemon-quit-teardown.ts b/src/main/daemon/daemon-quit-teardown.ts new file mode 100644 index 00000000000..b82c14b4c46 --- /dev/null +++ b/src/main/daemon/daemon-quit-teardown.ts @@ -0,0 +1,89 @@ +export type DaemonQuitMode = 'disconnect' | 'shutdown' + +export function resolveDaemonQuitMode(options: { + platform: NodeJS.Platform + updateQuitInProgress: boolean + relaunchRequested: boolean + devParentShutdownRequested: boolean +}): DaemonQuitMode { + if (options.devParentShutdownRequested) { + return 'shutdown' + } + if (options.updateQuitInProgress || options.relaunchRequested) { + return 'disconnect' + } + return options.platform === 'win32' ? 'shutdown' : 'disconnect' +} + +export async function shutdownAdoptedDaemonGenerations(options: { + shutdownCurrent?: () => Promise + legacyProtocolVersions: readonly number[] + shutdownLegacy: (protocolVersion: number) => Promise +}): Promise { + const tasks: { label: string; promise: Promise }[] = [] + if (options.shutdownCurrent) { + tasks.push({ label: 'current', promise: options.shutdownCurrent() }) + } + for (const protocolVersion of new Set(options.legacyProtocolVersions)) { + tasks.push({ + label: `protocol-v${protocolVersion}`, + promise: options.shutdownLegacy(protocolVersion) + }) + } + + const results = await Promise.allSettled(tasks.map(({ promise }) => promise)) + const failures = results.flatMap((result, index) => + result.status === 'rejected' + ? [new Error(`Failed to shut down ${tasks[index].label}`, { cause: result.reason })] + : [] + ) + if (failures.length > 0) { + throw new AggregateError(failures, 'Daemon generation shutdown failed') + } +} + +export async function runGracefulDaemonShutdownWithFallback(options: { + graceful: () => Promise + fallback: () => Promise + fallbackDelayMs: number +}): Promise { + let releaseFallback!: (run: boolean) => void + let timer: ReturnType | undefined + const fallbackGate = new Promise((resolve) => { + releaseFallback = resolve + timer = setTimeout(() => resolve(true), options.fallbackDelayMs) + }) + const gracefulResult = options.graceful().then( + () => { + clearTimeout(timer) + releaseFallback(false) + return null + }, + (error: unknown) => { + clearTimeout(timer) + releaseFallback(true) + return error + } + ) + const fallbackResult = fallbackGate.then(async (run) => { + if (run) { + await options.fallback() + } + }) + const [gracefulError, fallbackOutcome] = await Promise.all([ + gracefulResult, + fallbackResult.then( + () => null, + (error: unknown) => error + ) + ]) + if (fallbackOutcome) { + throw new AggregateError( + gracefulError ? [gracefulError, fallbackOutcome] : [fallbackOutcome], + 'Daemon fallback shutdown failed' + ) + } + if (gracefulError && !(await fallbackGate)) { + throw gracefulError + } +} diff --git a/src/main/daemon/daemon-spawner.test.ts b/src/main/daemon/daemon-spawner.test.ts index 613b9e6a149..7db07966466 100644 --- a/src/main/daemon/daemon-spawner.test.ts +++ b/src/main/daemon/daemon-spawner.test.ts @@ -154,6 +154,27 @@ describe('DaemonSpawner', () => { }) describe('shutdown', () => { + it('fences and shuts down a launch that is still in flight', async () => { + let finishLaunch!: (handle: { shutdown: () => Promise }) => void + const handle = { shutdown: vi.fn(async () => {}) } + const launcher = vi.fn( + () => + new Promise<{ shutdown: () => Promise }>((resolve) => { + finishLaunch = resolve + }) + ) + spawner = new DaemonSpawner({ runtimeDir: dir, launcher }) + + const launching = spawner.ensureRunning() + const shuttingDown = spawner.shutdown() + finishLaunch(handle) + + await shuttingDown + await expect(launching).rejects.toThrow('interrupted by shutdown') + expect(handle.shutdown).toHaveBeenCalledOnce() + expect(spawner.getHandle()).toBeNull() + }) + it('stops the daemon', async () => { const s = createSpawner() const info = await s.ensureRunning() diff --git a/src/main/daemon/daemon-spawner.ts b/src/main/daemon/daemon-spawner.ts index ba0ea239c67..762b49416b4 100644 --- a/src/main/daemon/daemon-spawner.ts +++ b/src/main/daemon/daemon-spawner.ts @@ -41,6 +41,8 @@ export class DaemonSpawner { private socketPath: string private tokenPath: string private pidPath: string + private launchInFlight: Promise | null = null + private shutdownInFlight: Promise | null = null constructor(opts: DaemonSpawnerOptions) { this.runtimeDir = opts.runtimeDir @@ -55,9 +57,28 @@ export class DaemonSpawner { return { socketPath: this.socketPath, tokenPath: this.tokenPath } } - // Why: a detached daemon may clean up after its parent exits. A unique - // launch identity keeps it from deleting a replacement daemon's PID file. - this.handle = await this.launcher(this.socketPath, this.tokenPath, this.pidPath, randomUUID()) + if (this.shutdownInFlight) { + await this.shutdownInFlight + throw new Error('Daemon launch was interrupted by shutdown') + } + const launch = + this.launchInFlight ?? + this.launcher(this.socketPath, this.tokenPath, this.pidPath, randomUUID()).then((handle) => { + this.handle = handle + return handle + }) + this.launchInFlight = launch + try { + await launch + } finally { + if (this.launchInFlight === launch) { + this.launchInFlight = null + } + } + if (this.shutdownInFlight) { + await this.shutdownInFlight + throw new Error('Daemon launch was interrupted by shutdown') + } return { socketPath: this.socketPath, tokenPath: this.tokenPath } } @@ -74,12 +95,23 @@ export class DaemonSpawner { } async shutdown(): Promise { - if (!this.handle) { - return + if (this.shutdownInFlight) { + return this.shutdownInFlight + } + const shutdown = (async () => { + await this.launchInFlight?.catch(() => {}) + const handle = this.handle + this.handle = null + await handle?.shutdown() + })() + this.shutdownInFlight = shutdown + try { + await shutdown + } finally { + if (this.shutdownInFlight === shutdown) { + this.shutdownInFlight = null + } } - const handle = this.handle - this.handle = null - await handle.shutdown() } } diff --git a/src/main/index.ts b/src/main/index.ts index 5a4a687999a..74c7fd6a600 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -15,13 +15,14 @@ import { ensureActiveOrcaProfile, initOrcaProfilePaths } from './orca-profiles/p import { getOrcaCloudAuthConfig } from './orca-profiles/profile-cloud-auth-config' import { getProfileUserDataPath } from './orca-profiles/profile-storage-paths' import { applyAppIcon } from './app-icon' -import { relaunchApp } from './app-relaunch' +import { isAppRelaunchRequested, relaunchApp } from './app-relaunch' import { StatsCollector, initStatsPath } from './stats/collector' import { ClaudeUsageStore, initClaudeUsagePath } from './claude-usage/store' import { CodexUsageStore, initCodexUsagePath } from './codex-usage/store' import { OpenCodeUsageStore, initOpenCodeUsagePath } from './opencode-usage/store' import { killAllPty } from './ipc/pty' import { initDaemonPtyProvider, disconnectDaemon, shutdownDaemon } from './daemon/daemon-init' +import { resolveDaemonQuitMode } from './daemon/daemon-quit-teardown' import { closeAllWatchers } from './ipc/filesystem-watcher' import { disposeWorktreeBaseDirectoryWatchers } from './ipc/worktree-base-directory-watcher' import { registerCoreHandlers } from './ipc/register-core-handlers' @@ -2817,8 +2818,14 @@ app.on('will-quit', (e) => { : Promise.resolve() // Why: allSettled (not all) keeps fail-open — a daemon-disconnect rejection still quits instead of hanging. // Why: telemetry flush folds in before app.quit() (bounded 2s); catch defensively so a flush failure can't cancel the quit chain. - // Why: normal quits keep the detached daemon for warm reattach, but a dead dev parent leaves the temp/dev profile ownerless. - const daemonTeardown = isDevParentShutdownRequested() ? shutdownDaemon() : disconnectDaemon() + // Why: ordinary Windows exit must not orphan a detached daemon; relaunch/update still preserve warm sessions. + const daemonQuitMode = resolveDaemonQuitMode({ + platform: process.platform, + updateQuitInProgress, + relaunchRequested: isAppRelaunchRequested(), + devParentShutdownRequested: isDevParentShutdownRequested() + }) + const daemonTeardown = daemonQuitMode === 'shutdown' ? shutdownDaemon() : disconnectDaemon() // Why: a wedged transport (half-open post-sleep socket) can leave one // member unsettled forever and block app.quit() until Force Quit (#9447). settleTeardownWithinDeadline([ diff --git a/tests/e2e/helpers/electron-process-shutdown.ts b/tests/e2e/helpers/electron-process-shutdown.ts index cbc880d2a95..7e26971131a 100644 --- a/tests/e2e/helpers/electron-process-shutdown.ts +++ b/tests/e2e/helpers/electron-process-shutdown.ts @@ -139,7 +139,12 @@ async function forceKillProcessTree(proc: ChildProcess): Promise { } export async function closeElectronAppForE2E(app: ElectronApplication): Promise { - const proc = app.process() + let proc: ChildProcess + try { + proc = app.process() + } catch { + return + } try { await withTimeout(app.close(), GRACEFUL_CLOSE_TIMEOUT_MS, 'Timed out closing Electron app') if (proc) { diff --git a/tests/e2e/helpers/orca-restart.ts b/tests/e2e/helpers/orca-restart.ts index a503508cd46..f4bb5100a37 100644 --- a/tests/e2e/helpers/orca-restart.ts +++ b/tests/e2e/helpers/orca-restart.ts @@ -49,7 +49,7 @@ type LaunchOptions = { type RestartSession = { userDataDir: string launch: (options?: LaunchOptions) => Promise - /** Gracefully close a launch, letting beforeunload flush session state. */ + /** End a launch through Orca's production restart path without auto-spawning its replacement. */ close: (app: ElectronApplication) => Promise /** Remove the shared userDataDir after the test is done. */ dispose: () => Promise @@ -179,7 +179,31 @@ export function createRestartSession( } const close = async (app: ElectronApplication): Promise => { - await closeElectronAppForE2E(app) + let proc: ReturnType + try { + proc = app.process() + } catch { + return + } + try { + const page = await app.firstWindow() + await app.evaluate(({ app: electronApp }) => { + // Why: the fixture launches the replacement itself against the same profile. + Object.defineProperty(electronApp, 'relaunch', { configurable: true, value: () => {} }) + }) + const exited = new Promise((resolve) => proc.once('exit', () => resolve())) + await page.evaluate(() => { + void window.api.app.restart() + }) + await Promise.race([ + exited, + delay(30_000).then(() => { + throw new Error('Timed out waiting for Orca restart shutdown') + }) + ]) + } catch { + await closeElectronAppForE2E(app) + } } const dispose = async (): Promise => { diff --git a/tests/e2e/windows-app-quit-daemon.spec.ts b/tests/e2e/windows-app-quit-daemon.spec.ts new file mode 100644 index 00000000000..97c4e7b754d --- /dev/null +++ b/tests/e2e/windows-app-quit-daemon.spec.ts @@ -0,0 +1,79 @@ +import { existsSync, readFileSync } from 'node:fs' +import path from 'node:path' +import { getDaemonPidPath } from '../../src/main/daemon/daemon-spawner' +import { PROTOCOL_VERSION } from '../../src/main/daemon/types' +import { test, expect } from './helpers/orca-app' +import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store' +import { waitForActivePanePtyId } from './helpers/terminal' + +type DaemonIdentity = { + pid: number + startedAtMs: number | null + launchNonce: string | null +} + +function readDaemonIdentity(userDataDir: string): { + identity: DaemonIdentity + pidPath: string +} { + const pidPath = getDaemonPidPath(path.join(userDataDir, 'daemon'), PROTOCOL_VERSION) + const record = JSON.parse(readFileSync(pidPath, 'utf8')) as Partial + if (!Number.isInteger(record.pid) || (record.pid ?? 0) <= 0) { + throw new Error(`Invalid daemon PID record: ${pidPath}`) + } + return { + identity: { + pid: record.pid!, + startedAtMs: record.startedAtMs ?? null, + launchNonce: record.launchNonce ?? null + }, + pidPath + } +} + +function isProcessAlive(pid: number): boolean { + try { + process.kill(pid, 0) + return true + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ESRCH') { + return false + } + throw error + } +} + +test('normal Windows quit reaps the scoped daemon with a live terminal', async ({ + electronApp, + orcaPage +}) => { + test.skip(process.platform !== 'win32', 'Windows app-exit lifecycle only') + + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + await ensureTerminalVisible(orcaPage) + await waitForActivePanePtyId(orcaPage) + + const userDataDir = await electronApp.evaluate(({ app }) => app.getPath('userData')) + const { identity, pidPath } = readDaemonIdentity(userDataDir) + expect(identity.startedAtMs).not.toBeNull() + expect(identity.launchNonce).not.toBeNull() + expect(isProcessAlive(identity.pid)).toBe(true) + + const appProcess = electronApp.process() + const appExited = new Promise((resolve) => appProcess.once('exit', () => resolve())) + await electronApp.evaluate(({ app }) => { + setImmediate(() => app.quit()) + }) + await appExited + + await expect + .poll( + () => ({ + daemonAlive: isProcessAlive(identity.pid), + pidRecordExists: existsSync(pidPath) + }), + { timeout: 10_000 } + ) + .toEqual({ daemonAlive: false, pidRecordExists: false }) +})