From 23b98e62a317dacabc810f1d5331abe0c5f5e76e Mon Sep 17 00:00:00 2001 From: Brennan Benson Date: Fri, 10 Jul 2026 17:42:54 -0700 Subject: [PATCH] fix(daemon): bound legacy probes and keep host under lint limit --- src/main/daemon/daemon-init.test.ts | 47 ++++++++++++++ src/main/daemon/daemon-init.ts | 23 ++++++- src/main/daemon/terminal-host-contract.ts | 64 +++++++++++++++++++ src/main/daemon/terminal-host.ts | 76 +++-------------------- 4 files changed, 139 insertions(+), 71 deletions(-) create mode 100644 src/main/daemon/terminal-host-contract.ts diff --git a/src/main/daemon/daemon-init.test.ts b/src/main/daemon/daemon-init.test.ts index 1659936cc91..bfb7fd9dc5e 100644 --- a/src/main/daemon/daemon-init.test.ts +++ b/src/main/daemon/daemon-init.test.ts @@ -845,6 +845,53 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { expect(killStaleDaemonMock).not.toHaveBeenCalled() }) + it('bounds an unresponsive legacy session query and fail-safe adopts the daemon', async () => { + vi.useFakeTimers() + try { + const mod = await importFresh() + probeSocketExistsMock.mockImplementation( + (p?: string) => p?.endsWith('daemon-v9.sock') ?? false + ) + netConnectMock.mockImplementation(() => { + 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() {} + } + }) + const disconnectMock = vi.fn() + daemonClientMock.mockImplementationOnce(function MockHungLegacyDaemonClient() { + return { + ensureConnected: vi.fn(async () => {}), + request: vi.fn(() => new Promise(() => {})), + disconnect: disconnectMock + } + }) + + const initPromise = mod.initDaemonPtyProvider() + await vi.advanceTimersByTimeAsync(1000) + await initPromise + + const { DaemonPtyRouter } = await import('./daemon-pty-router') + expect(mod.getDaemonProvider()).toBeInstanceOf(DaemonPtyRouter) + expect(adapterInstances.some((instance) => instance.protocolVersion === 9)).toBe(true) + expect(disconnectMock).toHaveBeenCalledOnce() + expect(killStaleDaemonMock).not.toHaveBeenCalled() + } finally { + vi.useRealTimers() + } + }) + it('restart path with no legacy adapters yields a bare DaemonPtyAdapter (not wrapped in a router)', async () => { const mod = await importFresh() await mod.initDaemonPtyProvider() diff --git a/src/main/daemon/daemon-init.ts b/src/main/daemon/daemon-init.ts index 29d09ec8cf7..cd2aa74bf1c 100644 --- a/src/main/daemon/daemon-init.ts +++ b/src/main/daemon/daemon-init.ts @@ -74,6 +74,7 @@ let adapter: DaemonProvider | null = null // the second entry would read the already-disposed current adapter and // race cleanupDaemonForProtocol against a half-spawned replacement. let restartInFlight: Promise | null = null +const LEGACY_SESSION_QUERY_TIMEOUT_MS = 1000 function getRuntimeDir(): string { const dir = join(app.getPath('userData'), 'daemon') @@ -160,13 +161,31 @@ async function getAliveDaemonSessionCount( protocolVersion = PROTOCOL_VERSION ): Promise { const client = new DaemonClient({ socketPath, tokenPath, protocolVersion }) + let timeout: ReturnType | null = null try { - await client.ensureConnected() - const result = await client.request('listSessions', undefined) + // Why: legacy versions are checked serially during startup. A daemon that + // accepts sockets but stops answering RPCs must fail-safe to adoption + // promptly instead of multiplying the client's 30s timeout by versions. + const result = await Promise.race([ + (async () => { + await client.ensureConnected() + return client.request('listSessions', undefined) + })(), + new Promise((resolve) => { + timeout = setTimeout(() => resolve(null), LEGACY_SESSION_QUERY_TIMEOUT_MS) + timeout.unref?.() + }) + ]) + if (!result) { + return null + } return result.sessions.filter((session) => session.isAlive).length } catch { return null } finally { + if (timeout) { + clearTimeout(timeout) + } client.disconnect() } } diff --git a/src/main/daemon/terminal-host-contract.ts b/src/main/daemon/terminal-host-contract.ts new file mode 100644 index 00000000000..a22350d43d4 --- /dev/null +++ b/src/main/daemon/terminal-host-contract.ts @@ -0,0 +1,64 @@ +import type { StartupCommandDelivery } from '../../shared/codex-startup-delivery' +import type { SubprocessHandle } from './session' +import type { ShellReadyState, TakePendingOutputResult, TerminalSnapshot } from './types' + +export type CreateOrAttachOptions = { + sessionId: string + cols: number + rows: number + cwd?: string + env?: Record + envToDelete?: string[] + command?: string + startupCommandDelivery?: StartupCommandDelivery + /** Explicit shell the renderer asked for (e.g. 'wsl.exe' for "New WSL + * terminal" from the "+" menu). Forwarded to the subprocess spawner so the + * daemon path honors per-tab shell selection the same way LocalPtyProvider + * does. */ + shellOverride?: string + terminalWindowsWslDistro?: string | null + terminalWindowsPowerShellImplementation?: 'auto' | 'powershell.exe' | 'pwsh.exe' + shellReadySupported?: boolean + shellReadyTimeoutMs?: number + historySeed?: string + streamClient: { onData: (data: string) => void; onExit: (code: number) => void } +} + +export type CreateOrAttachResult = { + isNew: boolean + snapshot: TerminalSnapshot | null + pid: number | null + shellState: ShellReadyState + historySeeded?: boolean + attachToken: symbol +} + +export type TerminalHostOptions = { + spawnSubprocess: (opts: { + sessionId: string + cols: number + rows: number + cwd?: string + env?: Record + envToDelete?: string[] + command?: string + startupCommandDelivery?: StartupCommandDelivery + shellOverride?: string + terminalWindowsWslDistro?: string | null + terminalWindowsPowerShellImplementation?: 'auto' | 'powershell.exe' | 'pwsh.exe' + }) => SubprocessHandle + // Why: on graceful shutdown, the host writes final checkpoints for all live + // sessions before killing them. This bypasses the RPC round-trip — the daemon + // writes checkpoints in-process, guaranteeing completion before teardown. + onFinalCheckpoint?: ( + sessionId: string, + snapshot: TerminalSnapshot, + records: TakePendingOutputResult['records'] + ) => void + // Why: production keeps a large cap, but tests need a small deterministic cap + // without spawning thousands of full terminal sessions. + maxTombstones?: number + // Why: lets the daemon server re-evaluate idle-exit eligibility on every + // session-map transition instead of polling the host. + onSessionCountChange?: () => void +} diff --git a/src/main/daemon/terminal-host.ts b/src/main/daemon/terminal-host.ts index a64364410c8..bcfda73ef30 100644 --- a/src/main/daemon/terminal-host.ts +++ b/src/main/daemon/terminal-host.ts @@ -1,79 +1,17 @@ -import { Session, type SubprocessHandle } from './session' +import { Session } from './session' import { normalizePtySize } from './daemon-pty-size' import { resolveProcessCwd } from '../providers/process-cwd' -import type { StartupCommandDelivery } from '../../shared/codex-startup-delivery' import { buildStartupCommandSubmission } from '../../shared/startup-command-submission' -import type { - SessionInfo, - TakePendingOutputResult, - TerminalSnapshot, - ShellReadyState -} from './types' +import type { SessionInfo, TakePendingOutputResult, TerminalSnapshot } from './types' import { SessionNotFoundError } from './types' +import type { + CreateOrAttachOptions, + CreateOrAttachResult, + TerminalHostOptions +} from './terminal-host-contract' const DEFAULT_MAX_TOMBSTONES = 1000 -export type CreateOrAttachOptions = { - sessionId: string - cols: number - rows: number - cwd?: string - env?: Record - envToDelete?: string[] - command?: string - startupCommandDelivery?: StartupCommandDelivery - /** Explicit shell the renderer asked for (e.g. 'wsl.exe' for "New WSL - * terminal" from the "+" menu). Forwarded to the subprocess spawner so the - * daemon path honors per-tab shell selection the same way LocalPtyProvider - * does. */ - shellOverride?: string - terminalWindowsWslDistro?: string | null - terminalWindowsPowerShellImplementation?: 'auto' | 'powershell.exe' | 'pwsh.exe' - shellReadySupported?: boolean - shellReadyTimeoutMs?: number - historySeed?: string - streamClient: { onData: (data: string) => void; onExit: (code: number) => void } -} - -export type CreateOrAttachResult = { - isNew: boolean - snapshot: TerminalSnapshot | null - pid: number | null - shellState: ShellReadyState - historySeeded?: boolean - attachToken: symbol -} - -export type TerminalHostOptions = { - spawnSubprocess: (opts: { - sessionId: string - cols: number - rows: number - cwd?: string - env?: Record - envToDelete?: string[] - command?: string - startupCommandDelivery?: StartupCommandDelivery - shellOverride?: string - terminalWindowsWslDistro?: string | null - terminalWindowsPowerShellImplementation?: 'auto' | 'powershell.exe' | 'pwsh.exe' - }) => SubprocessHandle - // Why: on graceful shutdown, the host writes final checkpoints for all live - // sessions before killing them. This bypasses the RPC round-trip — the daemon - // writes checkpoints in-process, guaranteeing completion before teardown. - onFinalCheckpoint?: ( - sessionId: string, - snapshot: TerminalSnapshot, - records: TakePendingOutputResult['records'] - ) => void - // Why: production keeps a large cap, but tests need a small deterministic cap - // without spawning thousands of full terminal sessions. - maxTombstones?: number - // Why: lets the daemon server re-evaluate idle-exit eligibility on every - // session-map transition instead of polling the host. - onSessionCountChange?: () => void -} - export class TerminalHost { private sessions = new Map() private killedTombstones = new Map()