diff --git a/src/main/claude/claude-agent-sdk-process-spawn.test.ts b/src/main/claude/claude-agent-sdk-process-spawn.test.ts index cd3520cf6d5..1b6c192d433 100644 --- a/src/main/claude/claude-agent-sdk-process-spawn.test.ts +++ b/src/main/claude/claude-agent-sdk-process-spawn.test.ts @@ -4,14 +4,22 @@ import { describe, expect, it, vi } from 'vitest' import type { SpawnOptions as SdkSpawnOptions } from '@anthropic-ai/claude-agent-sdk' import { resolveSpawn, type spawnProcess } from '../../shared/child-process/run-process' import type { ProcessSpec } from '../../shared/child-process/process-spec' +import type * as ProviderSupervisor from '../codex/codex-app-server-posix-supervisor' +import { createProviderSpawnSpec } from '../codex/codex-app-server-posix-supervisor' import { createClaudeCodeProcessSpawn } from './claude-agent-sdk-process-spawn' +import { proveClaudeChildExitWithReaper } from './claude-child-exit-proof-ladder' + +vi.mock('../codex/codex-app-server-posix-supervisor', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, createProviderSpawnSpec: vi.fn(actual.createProviderSpawnSpec) } +}) type FakeChild = EventEmitter & { pid: number stdin: PassThrough stdout: PassThrough stderr: PassThrough - kill: ReturnType + kill: ReturnType boolean>> } function fakeSpawn() { @@ -20,7 +28,7 @@ function fakeSpawn() { child.stdin = new PassThrough() child.stdout = new PassThrough() child.stderr = new PassThrough() - child.kill = vi.fn(() => true) + child.kill = vi.fn((_signal?: NodeJS.Signals | number) => true) const specs: ProcessSpec[] = [] const spawnImpl = ((spec: ProcessSpec) => { specs.push(spec) @@ -43,7 +51,7 @@ function sdkOptions(overrides: Partial = {}): SdkSpawnOptions { describe('claude agent SDK process spawn', () => { it('routes the SDK spawn through Orca and retains the pid the lease adjudicates on', () => { const process = fakeSpawn() - const spawn = createClaudeCodeProcessSpawn(process.spawnImpl) + const spawn = createClaudeCodeProcessSpawn(process.spawnImpl, 'win32') expect(spawn.pid).toBeUndefined() expect(spawn.child).toBeNull() @@ -52,15 +60,103 @@ describe('claude agent SDK process spawn', () => { expect(child).toBe(process.child) expect(spawn.child).toBe(process.child) expect(spawn.pid).toBe(4321) + // Windows has no supervisor: Claude itself is the child. + expect(spawn.supervised).toBe(false) expect(process.specs[0]).toEqual({ program: '/usr/local/bin/claude', args: ['--output-format', 'stream-json'], cwd: '/work/repo', env: { PATH: '/usr/bin', CLAUDE_CONFIG_DIR: '/accounts/one' }, + detached: false, stdio: ['pipe', 'pipe', 'pipe'] }) }) + it.each(['darwin', 'linux'] as const)( + 'starts Claude under the provider supervisor on %s, which is then the pid the lease records', + (platform) => { + const process = fakeSpawn() + const spawn = createClaudeCodeProcessSpawn(process.spawnImpl, platform) + spawn.spawn(sdkOptions()) + + const [spec] = process.specs + if (!spec) { + throw new Error('the spawner never built a spec') + } + expect(spawn.supervised).toBe(true) + expect(spawn.pid).toBe(4321) + expect(spec.program).toBe(globalThis.process.execPath) + expect(spec.args?.[0]).toBe('-e') + expect(spec.detached).toBe(true) + expect(spec.cwd).toBe('/work/repo') + const supervisorSpec = JSON.parse( + Buffer.from(String(spec.env?.ORCA_PROVIDER_SUPERVISOR_SPEC), 'base64').toString() + ) + expect(supervisorSpec).toMatchObject({ + command: '/usr/local/bin/claude', + args: ['--output-format', 'stream-json'], + cwd: '/work/repo', + ownerPid: globalThis.process.pid + }) + // The supervisor passes its env to Claude minus its own two keys. + expect(spec.env).toMatchObject({ PATH: '/usr/bin', CLAUDE_CONFIG_DIR: '/accounts/one' }) + } + ) + + it.each([ + { platform: 'darwin', specSupervised: false }, + { platform: 'win32', specSupervised: true } + ] as const)( + 'stops Claude by the spawn spec\u2019s supervision on $platform, never the platform', + async ({ platform, specSupervised }) => { + const actual = await vi.importActual( + '../codex/codex-app-server-posix-supervisor' + ) + vi.mocked(createProviderSpawnSpec).mockImplementationOnce((...args) => ({ + ...actual.createProviderSpawnSpec(...args), + supervised: specSupervised + })) + const process = fakeSpawn() + const spawn = createClaudeCodeProcessSpawn(process.spawnImpl, platform) + spawn.spawn(sdkOptions()) + expect(spawn.supervised).toBe(specSupervised) + + let exited = false + let settle = (): void => {} + const exitPromise = new Promise((resolve) => { + settle = resolve + }) + // Claude leaves shortly after stdin ends, so the ladder never needs its forced rung. + process.child.stdin.on('finish', () => + setTimeout(() => { + exited = true + settle() + }, 10) + ) + const tree = { + capture: vi.fn(async () => {}), + reap: vi.fn(async () => 'exited' as const), + treeVerdict: 'exited' as const + } + await proveClaudeChildExitWithReaper( + { + child: process.child, + exitPromise, + exited: () => exited, + tree, + supervised: spawn.supervised + }, + () => tree + ) + // SIGTERM to an unsupervised Claude on Windows is TerminateProcess; a skipped one leaves it running. + if (specSupervised) { + expect(process.child.kill).toHaveBeenCalledWith('SIGTERM') + } else { + expect(process.child.kill).not.toHaveBeenCalled() + } + } + ) + it('keeps the child out of the SDK abort path so exit proof stays Orca-owned', () => { const process = fakeSpawn() const controller = new AbortController() @@ -86,7 +182,7 @@ describe('claude agent SDK process spawn', () => { it('hands a Windows .cmd shim to Orca\u2019s argument encoder', () => { const process = fakeSpawn() - createClaudeCodeProcessSpawn(process.spawnImpl).spawn( + createClaudeCodeProcessSpawn(process.spawnImpl, 'win32').spawn( sdkOptions({ command: 'C:\\Users\\dev\\AppData\\npm\\claude.cmd', args: ['--setting-sources=user,project,local', '--session-id', 'a b&c'] diff --git a/src/main/claude/claude-agent-sdk-process-spawn.ts b/src/main/claude/claude-agent-sdk-process-spawn.ts index a2b1ad7f158..bbb4b295aad 100644 --- a/src/main/claude/claude-agent-sdk-process-spawn.ts +++ b/src/main/claude/claude-agent-sdk-process-spawn.ts @@ -1,5 +1,6 @@ import type { SpawnOptions as ClaudeAgentSdkSpawnOptions } from '@anthropic-ai/claude-agent-sdk' import { spawnProcess } from '../../shared/child-process/run-process' +import { createProviderSpawnSpec } from '../codex/codex-app-server-posix-supervisor' /** Derived rather than imported: only src/shared/child-process may name node:child_process. */ type ClaudeCodeChild = ReturnType @@ -11,8 +12,13 @@ export type ClaudeCodeProcessSpawn = { spawn: (options: ClaudeAgentSdkSpawnOptions) => ClaudeCodeChild /** The retained child, so Orca keeps its own tree-kill and exit-proof ladder. Null until the SDK spawns. */ readonly child: ClaudeCodeChild | null - /** Ownership proof: the durable lease adjudicates on this pid plus start time plus the spawn token. */ + /** + * Ownership proof: the durable lease adjudicates on this pid plus start time plus the spawn + * token. On POSIX it is the provider supervisor's, which outlives Claude by construction. + */ readonly pid: number | undefined + /** The spawn spec's verdict, so the close ladder never re-decides it. False until the SDK spawns. */ + readonly supervised: boolean readonly stderrTail: string } @@ -31,24 +37,41 @@ function definedEnv(env: Record): Record { + const spec = createProviderSpawnSpec( + { + command: options.command, + args: [...options.args], + ...(options.cwd === undefined ? {} : { cwd: options.cwd }) + }, + definedEnv(options.env), + platform + ) // Why `options.signal` is dropped: it would let the SDK kill the child outside // Orca's ladder, and close() may never report an exit it did not observe. const spawned = spawnImpl({ - program: options.command, - args: [...options.args], - ...(options.cwd === undefined ? {} : { cwd: options.cwd }), - env: definedEnv(options.env), + program: spec.program, + args: spec.args, + cwd: spec.cwd, + env: spec.env, + detached: spec.detached, stdio: ['pipe', 'pipe', 'pipe'] }) child = spawned + supervised = spec.supervised // The SDK drains stderr only for its own local spawn, so a custom spawner must: // otherwise the child blocks on a full pipe and exit errors lose their tail. spawned.stderr.setEncoding('utf8').on('data', (chunk: string) => { @@ -62,6 +85,9 @@ export function createClaudeCodeProcessSpawn( get pid() { return child?.pid }, + get supervised() { + return supervised + }, get stderrTail() { return stderrTail } diff --git a/src/main/claude/claude-child-exit-proof-ladder.test.ts b/src/main/claude/claude-child-exit-proof-ladder.test.ts new file mode 100644 index 00000000000..2d8da7f629d --- /dev/null +++ b/src/main/claude/claude-child-exit-proof-ladder.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it, vi } from 'vitest' +import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from '../codex/codex-app-server-posix-supervisor' +import type { ClaudeChildTreeReaper } from './claude-agent-sdk-exit-proof' +import { proveClaudeChildExitWithReaper } from './claude-child-exit-proof-ladder' + +function fakeTree(): ClaudeChildTreeReaper & { reap: ReturnType } { + return { + capture: vi.fn(async () => {}), + refresh: vi.fn(async () => {}), + reap: vi.fn(async () => 'exited' as const), + treeVerdict: 'exited' + } +} + +/** A root that leaves only once a SIGTERM has had `stopMs` to act, the way a supervisor does. */ +function rootStoppedBySigterm(stopMs: number) { + let exited = false + let settle = (): void => {} + const exitPromise = new Promise((resolve) => { + settle = resolve + }) + const kill = vi.fn((signal?: NodeJS.Signals | number) => { + if (signal === 'SIGTERM') { + setTimeout(() => { + exited = true + settle() + }, stopMs) + } + return true + }) + const stdin = { end: vi.fn() } + return { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The ladder reads only pid, kill and stdin.end from its child. + child: { pid: 4321, kill, stdin } as unknown as Parameters< + typeof proveClaudeChildExitWithReaper + >[0]['child'], + kill, + stdin, + exitPromise, + exited: () => exited + } +} + +describe('Claude child exit proof ladder', () => { + it('stops a supervised child with SIGTERM and waits out the supervisor stop before forcing', async () => { + // Slower than the unsupervised 1.5 s grace, still inside the supervisor's own bound. + const root = rootStoppedBySigterm(PROVIDER_SUPERVISOR_MAX_STOP_MS - 500) + const tree = fakeTree() + + await expect( + proveClaudeChildExitWithReaper({ ...root, supervised: true, tree }, () => tree) + ).resolves.toBe(true) + + expect(root.stdin.end).toHaveBeenCalled() + expect(root.kill).toHaveBeenCalledWith('SIGTERM') + // Forcing here would SIGKILL the supervisor mid-stop and orphan Claude in its own group. + expect(root.kill).not.toHaveBeenCalledWith('SIGKILL') + expect(tree.reap).not.toHaveBeenCalled() + }, 10_000) + + it('never signals an unsupervised child for the graceful stop', async () => { + const root = rootStoppedBySigterm(0) + const tree = fakeTree() + + await proveClaudeChildExitWithReaper({ ...root, tree }, () => tree) + + // On Windows a direct SIGTERM is TerminateProcess: stdin end stays the only graceful rung. + expect(root.stdin.end).toHaveBeenCalled() + expect(root.kill).not.toHaveBeenCalledWith('SIGTERM') + expect(tree.reap).toHaveBeenCalled() + }, 10_000) +}) diff --git a/src/main/claude/claude-child-exit-proof-ladder.ts b/src/main/claude/claude-child-exit-proof-ladder.ts index 85ed629f1b9..37d4e47508d 100644 --- a/src/main/claude/claude-child-exit-proof-ladder.ts +++ b/src/main/claude/claude-child-exit-proof-ladder.ts @@ -1,8 +1,11 @@ import type { SpawnedProcess } from '../../shared/child-process/run-process' import { waitForProcessExitUntil } from '../codex/codex-process-exit-deadline' +import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from '../codex/codex-app-server-posix-supervisor' import type { ClaudeChildTreeReaper } from './claude-agent-sdk-exit-proof' -const GRACEFUL_EXIT_MS = 1_500 +export const GRACEFUL_EXIT_MS = 1_500 +// A signalled supervisor escalates on its own; forcing it sooner kills it and orphans Claude. +export const SUPERVISED_GRACEFUL_EXIT_MS = PROVIDER_SUPERVISOR_MAX_STOP_MS + 500 const FORCED_EXIT_MS = 1_000 export type ClaudeChildExitProofInput = { @@ -10,6 +13,8 @@ export type ClaudeChildExitProofInput = { exitPromise: Promise exited: () => boolean tree?: ClaudeChildTreeReaper + /** The child is the POSIX provider supervisor: SIGTERM stops Claude, which reaps its tools. */ + supervised?: boolean } export async function proveClaudeChildExitWithReaper( @@ -17,16 +22,24 @@ export async function proveClaudeChildExitWithReaper( createTree: () => ClaudeChildTreeReaper ): Promise { const tree = input.tree ?? createTree() - // Arm before stdin closes: only a live root can identify its descendants. + // Arm before the stop: only a live root can identify its descendants. await tree.capture() try { input.child.stdin?.end() } catch { // The reap below still owns the process. } + // Stdin end alone lets Claude finish its turn, tools and edits included; a close is a stop. + // Windows has no supervisor, and a direct SIGTERM there is TerminateProcess. + if (input.supervised && !input.exited()) { + input.child.kill('SIGTERM') + } let reaped = false if (!input.exited()) { - await waitForProcessExitUntil(input.exitPromise, GRACEFUL_EXIT_MS) + await waitForProcessExitUntil( + input.exitPromise, + input.supervised ? SUPERVISED_GRACEFUL_EXIT_MS : GRACEFUL_EXIT_MS + ) if (!input.exited()) { reaped = true await tree.refresh?.() diff --git a/src/main/claude/claude-child-root-termination.ts b/src/main/claude/claude-child-root-termination.ts index bed422532e1..bba29919578 100644 --- a/src/main/claude/claude-child-root-termination.ts +++ b/src/main/claude/claude-child-root-termination.ts @@ -19,6 +19,9 @@ type RootTerminationInput = { * nothing. A probe here could only let an unreadable process table cost the tree * the one fallback that still works once every table read has failed. * + * On POSIX the root is the provider supervisor, killed only after its own stop had + * its whole bound; Claude, in its own group, is reached by the descendant kill. + * * False means no signal was sent, because the root had already left. */ export function terminateClaudeRoot(input: RootTerminationInput): boolean { diff --git a/src/main/claude/claude-stream-json-connection.test.ts b/src/main/claude/claude-stream-json-connection.test.ts index 6d3f0d89f30..e3682f03306 100644 --- a/src/main/claude/claude-stream-json-connection.test.ts +++ b/src/main/claude/claude-stream-json-connection.test.ts @@ -107,6 +107,18 @@ async function open( return connection } +function launchedArgv(spec: ProcessSpec | undefined): string[] { + const supervised = spec?.env?.ORCA_PROVIDER_SUPERVISOR_SPEC + if (!supervised) { + return [spec?.program ?? '', ...(spec?.args ?? [])] + } + const launch: unknown = JSON.parse(Buffer.from(supervised, 'base64').toString()) + if (!launch || typeof launch !== 'object' || !('command' in launch) || !('args' in launch)) { + return [] + } + return [String(launch.command), ...(Array.isArray(launch.args) ? launch.args.map(String) : [])] +} + function childEnv(): Record { return (spawned.at(-1)?.env ?? {}) as Record } @@ -201,8 +213,9 @@ describe('Claude stream-json connection', () => { const report = await until(() => readReportSafely(scenario), 'the scripted CLI report') expect(report.argv[0]).toBe(FAKE_CLI) // The .mjs fixture makes the SDK run it under node; a real CLI path is the program - // itself. Either way the resolved path is what Orca's spawner is asked to execute. - expect([spawned.at(-1)?.program, ...(spawned.at(-1)?.args ?? [])]).toContain(FAKE_CLI) + // itself. Either way the resolved path is what Orca's spawner is asked to execute, + // through the POSIX supervisor's spec where there is one. + expect(launchedArgv(spawned.at(-1))).toContain(FAKE_CLI) expect(report.argv).toContain('--replay-user-messages') expect(report.argv).toContain(`--session-id=${SESSION_ID}`) }) @@ -647,37 +660,63 @@ describe('Claude stream-json connection', () => { 20_000 ) - it('settles a spawn error followed by close as processless and closes idempotently', async () => { - const scenario = scriptScenario([HOLD_OPEN]) - const missingCli = join(scenario.cwd, 'claude-that-does-not-exist') - let fault: Error | null = null - let exit: Error | null = null - const connection = await open( - { ...launchFor(scenario), pathToClaudeCodeExecutable: missingCli }, - { - onFault: (error) => { - fault = error - }, - onExit: (error) => { - exit = error + it.skipIf(process.platform === 'win32')( + 'reports a missing CLI under the supervisor as a root exit that names the spawn error', + async () => { + const scenario = scriptScenario([HOLD_OPEN]) + const missingCli = join(scenario.cwd, 'claude-that-does-not-exist') + let exit: Error | null = null + const connection = await open( + { ...launchFor(scenario), pathToClaudeCodeExecutable: missingCli }, + { + onExit: (error) => { + exit = error + } } - } - ) + ) - await until( - () => (connection.exitVerdict.root === 'processless' ? connection.exitVerdict : null), - 'the processless spawn settlement' - ) - expect(connection.pid).toBeUndefined() - expect(fault).toBeInstanceOf(Error) - expect(exit).toBeNull() - await expect(Promise.all([connection.close(), connection.close()])).resolves.toEqual([ - true, - true - ]) - await expect(connection.close()).resolves.toBe(true) - expect(connection.exitVerdict).toEqual({ root: 'processless', tree: 'exited' }) - }) + const reported = await until(() => exit, 'the supervised spawn failure') + // The supervisor spawned, so this is its exit; only its stderr can say why. + expect(reported.message).toMatch(/\(code 127\).*ENOENT/s) + // A first-hand root exit, which releases the lease like a processless start did. + expect(connection.exitVerdict.root).toBe('exited') + } + ) + + it.skipIf(process.platform !== 'win32')( + 'settles a spawn error followed by close as processless and closes idempotently', + async () => { + const scenario = scriptScenario([HOLD_OPEN]) + const missingCli = join(scenario.cwd, 'claude-that-does-not-exist') + let fault: Error | null = null + let exit: Error | null = null + const connection = await open( + { ...launchFor(scenario), pathToClaudeCodeExecutable: missingCli }, + { + onFault: (error) => { + fault = error + }, + onExit: (error) => { + exit = error + } + } + ) + + await until( + () => (connection.exitVerdict.root === 'processless' ? connection.exitVerdict : null), + 'the processless spawn settlement' + ) + expect(connection.pid).toBeUndefined() + expect(fault).toBeInstanceOf(Error) + expect(exit).toBeNull() + await expect(Promise.all([connection.close(), connection.close()])).resolves.toEqual([ + true, + true + ]) + await expect(connection.close()).resolves.toBe(true) + expect(connection.exitVerdict).toEqual({ root: 'processless', tree: 'exited' }) + } + ) it('does not treat a child error event as first-hand root exit proof', async () => { const scenario = scriptScenario([HOLD_OPEN]) diff --git a/src/main/claude/claude-stream-json-connection.ts b/src/main/claude/claude-stream-json-connection.ts index 486407c0137..6f67e04b36e 100644 --- a/src/main/claude/claude-stream-json-connection.ts +++ b/src/main/claude/claude-stream-json-connection.ts @@ -309,15 +309,16 @@ export async function openClaudeStreamJsonConnection( closePromise ??= (async () => { closing = true resumeReading() - // Arm the descendant proof before ending stdin. The SDK may exit the root - // immediately; a post-exit walk cannot recover descendants that reparented. + // Arm the descendant proof before the stop. The root may exit immediately; + // a post-exit walk cannot recover descendants that reparented. await (tree.refresh?.() ?? tree.capture()) inbox.end() const proven = await proveClaudeChildExit({ child, exitPromise, exited: rootSettled, - tree + tree, + supervised: spawner.supervised }) inbox.fail(new Error('claude stream-json connection closed')) if (!proven) { diff --git a/src/main/claude/claude-structured-requested-stop.test.ts b/src/main/claude/claude-structured-requested-stop.test.ts new file mode 100644 index 00000000000..993109f4a37 --- /dev/null +++ b/src/main/claude/claude-structured-requested-stop.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, it } from 'vitest' +import type { + AgentJournalItemBody, + AgentJournalItemIdentity +} from '../../shared/agent-session-journal-types' +import { readAgentJournalTurn } from '../../shared/agent-session-turn-record' +import type { StructuredAgentSessionEventSink } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' +import type { ClaudeStructuredSessionEvent } from './claude-structured-session-state' +import { + PROVIDER_SESSION_ID, + USER_MESSAGE, + adapterFor, + fakeClaude, + identityFor +} from './claude-structured-session-test-support' + +function journalSink() { + const items: { identity: AgentJournalItemIdentity; body: AgentJournalItemBody }[] = [] + const sink: StructuredAgentSessionEventSink = { + appendItem: (identity, body) => items.push({ identity, body }), + appendTombstone: () => {}, + publish: () => {} + } + return { sink, items } +} + +function frame(type: 'assistant' | 'user', uuid: string, content: unknown[]) { + return { + type, + uuid, + session_id: PROVIDER_SESSION_ID, + parent_tool_use_id: null, + message: { role: type, content } + } +} + +describe('a requested stop of a structured Claude chat', () => { + it('reads interrupted, not failed or crashed, through the frames the stop makes Claude emit', async () => { + const claude = fakeClaude({ replayUuid: 'turn-1' }) + const events: ClaudeStructuredSessionEvent[] = [] + const adapter = adapterFor(claude, {}, events) + const journal = journalSink() + await adapter.acquire({ + identity: identityFor(), + fence: 7, + spawnToken: 'spawn-9', + events: journal.sink + }) + await adapter.dispatch({ + sessionId: 'session-1', + clientMessageId: 'client-1', + body: USER_MESSAGE, + fence: 7 + }) + const connection = claude.connections[0]! + connection.handlers.onMessage?.( + frame('assistant', 'assistant-1', [ + { type: 'tool_use', id: 'tool-1', name: 'Bash', input: { command: 'sleep 120' } } + ]) + ) + // What Claude 2.1.283 emitted within 35 ms of the supervised SIGTERM mid-tool, recorded + // against the real CLI: the killed tool's result, a status frame, and no `result` frame. + connection.close = async () => { + connection.handlers.onMessage?.( + frame('user', 'user-2', [ + { type: 'tool_result', tool_use_id: 'tool-1', is_error: true, content: 'Exit code 137' } + ]) + ) + connection.handlers.onMessage?.({ + type: 'system', + subtype: 'status', + uuid: 'status-1', + session_id: PROVIDER_SESSION_ID + }) + connection.closed = true + return true + } + + await expect(adapter.closeSession('session-1')).resolves.toBe(true) + + const turns = journal.items.flatMap((item) => { + const turn = readAgentJournalTurn(item.body) + return turn ? [turn] : [] + }) + expect(turns.at(-1)).toMatchObject({ turnId: 'turn-1', state: 'interrupted' }) + const ended = events.filter((event) => event.type === 'ended') + expect(ended).toEqual([expect.objectContaining({ reason: 'claude session closed' })]) + expect(ended[0]).not.toHaveProperty('cause') + }) +}) diff --git a/src/main/claude/claude-supervised-stop-real-cli.test.ts b/src/main/claude/claude-supervised-stop-real-cli.test.ts new file mode 100644 index 00000000000..82432e3fd6e --- /dev/null +++ b/src/main/claude/claude-supervised-stop-real-cli.test.ts @@ -0,0 +1,244 @@ +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { homedir, tmpdir } from 'node:os' +import { join } from 'node:path' +import { randomUUID } from 'node:crypto' +import { afterEach, describe, expect, it } from 'vitest' +import { runProcess } from '../../shared/child-process/run-process' +import { resolveSessionFilePath } from '../native-chat/session-file-resolver' +import { SUPERVISED_GRACEFUL_EXIT_MS } from './claude-child-exit-proof-ladder' +import { + openClaudeStreamJsonConnection, + type ClaudeStreamJsonConnection +} from './claude-stream-json-connection' +import { + CLAUDE_STRUCTURED_BASE_OPTIONS, + claudeStructuredPermissionOptions +} from './claude-structured-launch-resolution' + +// Opt-in only: spends a real (haiku) turn per case. Run it in an isolated HOME with +// ORCA_REAL_CLAUDE_BIN set, and ORCA_REAL_CLAUDE_SETTINGS when auth lives in a settings file. +const CLAUDE_BIN = process.env.ORCA_REAL_CLAUDE_BIN ?? '' +const enabled = + process.env.ORCA_REAL_CLAUDE_SUPERVISED_STOP === '1' && + process.platform !== 'win32' && + CLAUDE_BIN.length > 0 +const FRAMES_OUT = process.env.ORCA_REAL_CLAUDE_FRAMES_OUT +const TOOL_MARKER = 'orca_supervised_stop_probe' +const TOOL_PROMPT = `Use the Bash tool to run exactly this command, with no timeout argument, and nothing else: python3 -c "import time; time.sleep(120) # ${TOOL_MARKER}"` + +type Frame = { at: number; message: Record } +type Row = { pid: number; ppid: number; command: string } + +const recordedPids = new Set() +const tempDirs: string[] = [] +const connections: ClaudeStreamJsonConnection[] = [] + +function alive(pid: number): boolean { + try { + process.kill(pid, 0) + return true + } catch (error) { + return !(error instanceof Error && 'code' in error && error.code === 'ESRCH') + } +} + +async function processTable(): Promise { + const result = await runProcess({ program: 'ps', args: ['-axo', 'pid=,ppid=,command='] }) + return result.stdout.split('\n').flatMap((line) => { + const match = /^\s*(\d+)\s+(\d+)\s+(.*)$/.exec(line) + return match ? [{ pid: Number(match[1]), ppid: Number(match[2]), command: match[3] }] : [] + }) +} + +async function descendantsOf(rootPid: number): Promise { + const rows = await processTable() + const found: Row[] = [] + const frontier = [rootPid] + while (frontier.length > 0) { + const parent = frontier.pop() + for (const row of rows.filter((candidate) => candidate.ppid === parent)) { + found.push(row) + frontier.push(row.pid) + } + } + return found +} + +async function until(read: () => Promise | T | null, what: string, ms = 90_000) { + const deadline = Date.now() + ms + for (;;) { + const value = await read() + if (value !== null) { + return value + } + if (Date.now() >= deadline) { + throw new Error(`timed out waiting for ${what}`) + } + await new Promise((resolve) => setTimeout(resolve, 250)) + } +} + +async function open(sessionId: string, cwd: string, resume: boolean, frames: Frame[]) { + const settings = process.env.ORCA_REAL_CLAUDE_SETTINGS + const permission = claudeStructuredPermissionOptions('bypassPermissions') + const connection = await openClaudeStreamJsonConnection( + { + pathToClaudeCodeExecutable: CLAUDE_BIN, + options: { + ...CLAUDE_STRUCTURED_BASE_OPTIONS, + model: 'haiku', + extraArgs: { + ...CLAUDE_STRUCTURED_BASE_OPTIONS.extraArgs, + ...permission.extraArgs, + ...(settings ? { settings } : {}) + }, + ...(resume ? { resume: sessionId } : { sessionId }) + }, + cwd + }, + { onMessage: (message) => frames.push({ at: Date.now(), message }) } + ) + connections.push(connection) + recordedPids.add(connection.pid!) + return connection +} + +function userMessage(text: string): Record { + return { + type: 'user', + message: { role: 'user', content: [{ type: 'text', text }] }, + parent_tool_use_id: null, + session_id: '' + } +} + +async function transcriptLines(sessionId: string): Promise { + const configDir = process.env.CLAUDE_CONFIG_DIR?.trim() || join(homedir(), '.claude') + const path = await until( + () => + resolveSessionFilePath('claude', sessionId, { + claudeProjectsDir: join(configDir, 'projects') + }), + 'the transcript', + 15_000 + ) + return readFileSync(path, 'utf8') +} + +function expectLineAtomic(contents: string): void { + expect(contents.length).toBeGreaterThan(0) + expect(contents.endsWith('\n')).toBe(true) + for (const line of contents.split('\n').filter((entry) => entry.trim())) { + expect(() => JSON.parse(line)).not.toThrow() + } +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null +} + +function summarize(frames: Frame[], since: number): string[] { + return frames + .filter((frame) => frame.at >= since) + .map(({ at, message }) => { + const inner = isRecord(message.message) ? message.message.content : undefined + const blocks = (Array.isArray(inner) ? inner : []) + .filter(isRecord) + .map((block) => + block.type === 'tool_result' + ? `tool_result(is_error=${String(block.is_error)}:${JSON.stringify(block.content).slice(0, 80)})` + : String(block.type) + ) + return `+${at - since}ms ${String(message.type)}/${String(message.subtype ?? '')}${ + message.type === 'result' ? `(is_error=${String(message.is_error)})` : '' + } ${blocks.join(',')}`.trim() + }) +} + +async function resumeAndAsk(sessionId: string, cwd: string): Promise { + const frames: Frame[] = [] + const resumed = await open(sessionId, cwd, true, frames) + await resumed.send(userMessage('In one short line: what command did I ask you to run?')) + const result = await until( + () => frames.find((frame) => frame.message.type === 'result')?.message ?? null, + 'the resumed turn result' + ) + expect(result.subtype).toBe('success') + await expect(resumed.close()).resolves.toBe(true) +} + +type StopCase = 'close mid-tool' | 'SIGTERM to the supervisor mid-tool' | 'close while idle' + +afterEach(async () => { + for (const pid of recordedPids) { + if (alive(pid)) { + process.kill(pid, 'SIGKILL') + } + } + recordedPids.clear() + connections.splice(0) + for (const dir of tempDirs.splice(0)) { + rmSync(dir, { recursive: true, force: true }) + } +}) + +describe.runIf(enabled)('real Claude stopped through the POSIX supervisor', () => { + it.each(['close mid-tool', 'SIGTERM to the supervisor mid-tool', 'close while idle'])( + '%s: stops Claude and its tool, keeps the transcript line-atomic, and resumes', + async (stopCase) => { + const cwd = mkdtempSync(join(tmpdir(), 'orca-real-claude-stop-')) + tempDirs.push(cwd) + const sessionId = randomUUID() + const frames: Frame[] = [] + const connection = await open(sessionId, cwd, false, frames) + const supervisor = connection.pid! + const midTool = stopCase !== 'close while idle' + await connection.send(userMessage(midTool ? TOOL_PROMPT : 'Reply with the single word: ok')) + const descendants = midTool + ? await until(async () => { + const rows = await descendantsOf(supervisor) + return rows.some((row) => row.command.includes(TOOL_MARKER)) ? rows : null + }, 'the Bash tool').catch((error: unknown) => { + console.log('[no tool] frames so far:', summarize(frames, 0)) + throw error + }) + : await until( + async () => + frames.some((frame) => frame.message.type === 'result') + ? await descendantsOf(supervisor) + : null, + 'the idle turn' + ) + for (const row of descendants) { + recordedPids.add(row.pid) + } + + const signalledAt = Date.now() + if (stopCase === 'SIGTERM to the supervisor mid-tool') { + process.kill(supervisor, 'SIGTERM') + } else { + await expect(connection.close()).resolves.toBe(true) + } + const gone = await until( + () => ([supervisor, ...descendants.map((row) => row.pid)].some(alive) ? null : true), + 'the supervisor, Claude and its tools to exit', + SUPERVISED_GRACEFUL_EXIT_MS + 2_000 + ) + const stoppedMs = Date.now() - signalledAt + const after = summarize(frames, signalledAt) + console.log(`[${stopCase}] stopped in ${stoppedMs} ms; frames after the stop:`, after) + if (FRAMES_OUT) { + writeFileSync( + `${FRAMES_OUT}.${stopCase.replaceAll(' ', '-')}.json`, + JSON.stringify({ stoppedMs, after }, null, 2) + ) + } + expect(gone).toBe(true) + + expectLineAtomic(await transcriptLines(sessionId)) + await resumeAndAsk(sessionId, cwd) + expectLineAtomic(await transcriptLines(sessionId)) + }, + 180_000 + ) +}) diff --git a/src/main/claude/claude-supervised-stop.integration.test.ts b/src/main/claude/claude-supervised-stop.integration.test.ts new file mode 100644 index 00000000000..d0a5f9ea0cc --- /dev/null +++ b/src/main/claude/claude-supervised-stop.integration.test.ts @@ -0,0 +1,225 @@ +import { spawn, type ChildProcess } from 'node:child_process' +import { existsSync, mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import type { SpawnOptions as SdkSpawnOptions } from '@anthropic-ai/claude-agent-sdk' +import { spawnProcess } from '../../shared/child-process/run-process' +import type { ProcessSpec } from '../../shared/child-process/process-spec' +import { + PROVIDER_SIGTERM_GRACE_MS, + PROVIDER_STDIN_END_GRACE_MS, + PROVIDER_SUPERVISOR_MAX_STOP_MS +} from '../codex/codex-app-server-posix-supervisor' +import { proveClaudeChildExit } from './claude-agent-sdk-exit-proof' +import { createClaudeCodeProcessSpawn } from './claude-agent-sdk-process-spawn' + +// Stands in for Claude mid-turn: stdin end does not stop it, the way EOF lets the real CLI finish +// its turn. Its tool leads its own group, as the CLI's Bash tool shells do, and only Claude's own +// SIGTERM handler reaps it. The daemon double-forks out of the tree before anything looks. +const FAKE_CLAUDE = String.raw` + const { spawn } = require('node:child_process') + const { writeFileSync } = require('node:fs') + process.stdin.resume() + process.stdout.on('error', () => {}) + const tool = spawn(process.execPath, ['-e', 'setInterval(() => {}, 60000)'], { + detached: true, + stdio: 'ignore' + }) + const forker = spawn( + process.execPath, + ['-e', "const d = require('node:child_process').spawn(process.execPath, ['-e', 'setInterval(() => {}, 60000)'], { detached: true, stdio: 'ignore' }); d.unref(); process.stdout.write(String(d.pid))"], + { stdio: ['ignore', 'pipe', 'ignore'] } + ) + let daemon = '' + forker.stdout.on('data', (chunk) => { daemon += chunk }) + forker.once('exit', () => { + process.stdout.write(JSON.stringify({ claude: process.pid, tool: tool.pid, daemon: Number(daemon) }) + '\n') + }) + process.on('SIGTERM', () => { + if (process.env.ORCA_TEST_CLAUDE_IGNORES_SIGTERM) return + try { process.kill(-tool.pid, 'SIGKILL') } catch {} + writeFileSync(process.env.ORCA_TEST_SIGTERM_MARKER, 'reaped') + process.exit(143) + }) + setInterval(() => {}, 60000) +` + +// Stands in for Orca's main: starts exactly the spec Claude's spawner built, then can be SIGKILLed. +const OWNER = String.raw` + const { spawn } = require('node:child_process') + const spec = JSON.parse(process.env.ORCA_TEST_SPAWN_SPEC) + const env = { ...spec.env } + if (env.ORCA_PROVIDER_SUPERVISOR_SPEC) { + const supervisor = JSON.parse(Buffer.from(env.ORCA_PROVIDER_SUPERVISOR_SPEC, 'base64').toString()) + supervisor.ownerPid = process.pid + env.ORCA_PROVIDER_SUPERVISOR_SPEC = Buffer.from(JSON.stringify(supervisor)).toString('base64') + } + const child = spawn(spec.program, spec.args, { + cwd: spec.cwd, + env, + detached: spec.detached, + stdio: ['pipe', 'pipe', 'ignore'] + }) + process.stdout.write(JSON.stringify({ root: child.pid }) + '\n') + child.stdout.pipe(process.stdout) + setInterval(() => {}, 60000) +` + +const recordedPids = new Set() +const tempDirs: string[] = [] + +function alive(pid: number): boolean { + try { + process.kill(pid, 0) + return true + } catch (error) { + return !(error instanceof Error && 'code' in error && error.code === 'ESRCH') + } +} + +async function waitFor(predicate: () => boolean, timeoutMs: number): Promise { + const deadline = Date.now() + timeoutMs + while (!predicate()) { + if (Date.now() >= deadline) { + return false + } + await new Promise((resolve) => setTimeout(resolve, 25)) + } + return true +} + +function readPids(child: ChildProcess, keys: readonly string[]): Promise> { + return new Promise((resolve, reject) => { + const pids: Record = {} + let buffered = '' + const timeout = setTimeout(() => reject(new Error(`no ${keys.join('/')} pids`)), 10_000) + const onData = (chunk: Buffer): void => { + buffered += chunk.toString() + const lines = buffered.split('\n') + buffered = lines.pop() ?? '' + for (const line of lines) { + const parsed: unknown = JSON.parse(line) + for (const [key, pid] of Object.entries(parsed ?? {})) { + if (typeof pid === 'number' && pid > 0) { + pids[key] = pid + recordedPids.add(pid) + } + } + } + if (keys.every((key) => key in pids)) { + clearTimeout(timeout) + child.stdout!.off('data', onData) + child.stdout!.resume() + resolve(pids) + } + } + child.stdout!.on('data', onData) + }) +} + +function sdkOptions(env: Record): SdkSpawnOptions { + const dir = mkdtempSync(join(tmpdir(), 'orca-claude-supervised-')) + tempDirs.push(dir) + return { + command: process.execPath, + args: ['-e', FAKE_CLAUDE], + cwd: dir, + env: { + ...process.env, + ORCA_TEST_SIGTERM_MARKER: join(dir, 'sigterm-reap'), + ...env + }, + signal: new AbortController().signal + } +} + +/** Claude spawned through Orca's own spawner and stopped by Orca's own close ladder. */ +async function spawnClaude(env: Record = {}) { + const spawner = createClaudeCodeProcessSpawn(spawnProcess) + const options = sdkOptions(env) + const child = spawner.spawn(options) + recordedPids.add(child.pid!) + let exited = false + const exit = new Promise<{ code: number | null; signal: NodeJS.Signals | null }>((resolve) => + child.once('exit', (code, signal) => { + exited = true + resolve({ code, signal }) + }) + ) + const pids = await readPids(child, ['claude', 'tool', 'daemon']) + const close = (): Promise => + proveClaudeChildExit({ + child, + exitPromise: exit.then(() => undefined), + exited: () => exited, + supervised: spawner.supervised + }) + return { child, exit, pids, close, marker: String(options.env.ORCA_TEST_SIGTERM_MARKER) } +} + +afterEach(() => { + for (const pid of recordedPids) { + if (alive(pid)) { + process.kill(pid, 'SIGKILL') + } + } + recordedPids.clear() + for (const dir of tempDirs.splice(0)) { + rmSync(dir, { recursive: true, force: true }) + } +}) + +describe.runIf(process.platform !== 'win32')('Claude under the POSIX provider supervisor', () => { + it('stops a mid-turn Claude on close instead of letting stdin end finish its turn', async () => { + const { child, exit, pids, close, marker } = await spawnClaude() + expect(child.pid).not.toBe(pids.claude) + + const startedAt = Date.now() + await expect(close()).resolves.toBe(true) + + // Claude's own SIGTERM reap ran at once, not after the supervisor's stdin-end grace. + expect(Date.now() - startedAt).toBeLessThan(PROVIDER_STDIN_END_GRACE_MS) + expect(existsSync(marker)).toBe(true) + await expect(exit).resolves.toEqual({ code: null, signal: 'SIGTERM' }) + expect(alive(pids.claude)).toBe(false) + expect(alive(pids.tool)).toBe(false) + }) + + it('lets the supervisor escalate a Claude that ignores SIGTERM, and exits only after it', async () => { + const { exit, pids, close } = await spawnClaude({ ORCA_TEST_CLAUDE_IGNORES_SIGTERM: '1' }) + + const startedAt = Date.now() + await expect(close()).resolves.toBe(true) + + const elapsed = Date.now() - startedAt + expect(elapsed).toBeGreaterThanOrEqual(PROVIDER_SIGTERM_GRACE_MS) + expect(elapsed).toBeLessThan(PROVIDER_SUPERVISOR_MAX_STOP_MS + 1_000) + // The supervisor's own SIGTERM stop finished the job; nothing forced the supervisor itself. + await expect(exit).resolves.toEqual({ code: null, signal: 'SIGTERM' }) + expect(alive(pids.claude)).toBe(false) + }) + + it("stops Claude and its tool when Orca's main dies, and leaves a daemon that left its tree alone", async () => { + const specs: ProcessSpec[] = [] + createClaudeCodeProcessSpawn((spec) => { + specs.push(spec) + return spawnProcess({ program: 'true' }) + }).spawn(sdkOptions({})) + const owner = spawn(process.execPath, ['-e', OWNER], { + env: { ...process.env, ORCA_TEST_SPAWN_SPEC: JSON.stringify(specs[0]) }, + stdio: ['ignore', 'pipe', 'ignore'] + }) + recordedPids.add(owner.pid!) + const pids = await readPids(owner, ['root', 'claude', 'tool', 'daemon']) + + owner.kill('SIGKILL') + + expect(await waitFor(() => !alive(pids.claude), PROVIDER_SUPERVISOR_MAX_STOP_MS)).toBe(true) + expect(await waitFor(() => !alive(pids.root), PROVIDER_SUPERVISOR_MAX_STOP_MS)).toBe(true) + // In its own group, so only Claude's SIGTERM handler could have reaped it. + expect(alive(pids.tool)).toBe(false) + // Not the conversation's writer: nothing proves it orphaned, so the stop never reaches it. + expect(alive(pids.daemon)).toBe(true) + }) +}) diff --git a/src/main/codex/codex-app-server-connection.ts b/src/main/codex/codex-app-server-connection.ts index ef566bca336..5bed3b594d7 100644 --- a/src/main/codex/codex-app-server-connection.ts +++ b/src/main/codex/codex-app-server-connection.ts @@ -46,7 +46,7 @@ export type CodexAppServerLaunch = { } const DEFAULT_REQUEST_TIMEOUT_MS = 30_000 -const GRACEFUL_EXIT_MS = 1_500 +export const GRACEFUL_EXIT_MS = 1_500 const FORCED_EXIT_MS = 1_000 const STDERR_TAIL_MAX_BYTES = 8192 diff --git a/src/main/codex/codex-app-server-posix-supervisor.integration.test.ts b/src/main/codex/codex-app-server-posix-supervisor.integration.test.ts index e23cdbc88c3..846afaa973f 100644 --- a/src/main/codex/codex-app-server-posix-supervisor.integration.test.ts +++ b/src/main/codex/codex-app-server-posix-supervisor.integration.test.ts @@ -52,8 +52,11 @@ const RECORDS_SIGTERM_PROVIDER = String.raw` // Stands in for Orca: launches the supervisor as its own child, then can be killed outright. A // second child holds the supervisor's stdin open, so only the parent-death watch can notice. +// A clean-quit owner has no holder and exits normally on SIGUSR2, the way Orca quits. const OWNER = String.raw` const { spawn } = require('node:child_process') + const quitsCleanly = Boolean(process.env.ORCA_TEST_OWNER_QUITS_CLEANLY) + if (quitsCleanly) process.on('SIGUSR2', () => process.exit(0)) const spec = JSON.parse(Buffer.from(process.env.ORCA_PROVIDER_SUPERVISOR_SPEC, 'base64').toString()) spec.ownerPid = process.pid const supervisor = spawn(process.execPath, ['-e', process.env.ORCA_TEST_SUPERVISOR_SCRIPT], { @@ -61,10 +64,12 @@ const OWNER = String.raw` stdio: ['pipe', 'pipe', 'ignore'], detached: true }) - const holder = spawn(process.execPath, ['-e', 'setInterval(() => {}, 60000)'], { - stdio: ['ignore', supervisor.stdin, 'ignore'] - }) - process.stdout.write(JSON.stringify({ supervisor: supervisor.pid, holder: holder.pid }) + '\n') + const holder = quitsCleanly + ? null + : spawn(process.execPath, ['-e', 'setInterval(() => {}, 60000)'], { + stdio: ['ignore', supervisor.stdin, 'ignore'] + }) + process.stdout.write(JSON.stringify({ supervisor: supervisor.pid, ...(holder && { holder: holder.pid }) }) + '\n') supervisor.stdout.pipe(process.stdout) setInterval(() => {}, 60000) ` @@ -175,7 +180,7 @@ async function launchUnderOwner( stdio: ['ignore', 'pipe', 'ignore'] }) recordedPids.add(owner.pid!) - const pids = await readPids(owner, ['supervisor', 'holder', 'provider', 'grandchild']) + const pids = await readPids(owner, ['supervisor', 'provider', 'grandchild']) return { owner, pids } } @@ -318,6 +323,23 @@ describe.runIf(process.platform !== 'win32')('POSIX provider supervisor processe expect(alive(provider)).toBe(false) }) + it('reaps the provider group and exits when its owner quits cleanly', async () => { + const { owner, pids } = await launchUnderOwner( + { sigtermGraceMs: 300 }, + { ORCA_TEST_OWNER_QUITS_CLEANLY: '1' } + ) + const ownerExit = new Promise((resolve) => + owner.once('exit', (code, signal) => resolve({ code, signal })) + ) + + owner.kill('SIGUSR2') + + await expect(ownerExit).resolves.toEqual({ code: 0, signal: null }) + expect(await waitFor(() => !alive(-pids.provider), 3_000)).toBe(true) + expect(await waitFor(() => !alive(pids.supervisor), 3_000)).toBe(true) + expect(alive(pids.grandchild)).toBe(false) + }) + it('asks the provider to stop with SIGTERM when its owner dies', async () => { const signalFile = join(tempDir(), 'provider-signal') const { owner, pids } = await launchUnderOwner( diff --git a/src/main/codex/codex-app-server-posix-supervisor.test.ts b/src/main/codex/codex-app-server-posix-supervisor.test.ts index 28a088716f6..6dbe03e5b2a 100644 --- a/src/main/codex/codex-app-server-posix-supervisor.test.ts +++ b/src/main/codex/codex-app-server-posix-supervisor.test.ts @@ -72,7 +72,8 @@ describe('structured provider supervision', () => { args: ['app-server', '--flag'], env: { PATH: '/bin' }, cwd: '/work/repo', - detached: false + detached: false, + supervised: false }) }) }) diff --git a/src/main/codex/codex-app-server-posix-supervisor.ts b/src/main/codex/codex-app-server-posix-supervisor.ts index 30714ee7911..21dd56aeb5b 100644 --- a/src/main/codex/codex-app-server-posix-supervisor.ts +++ b/src/main/codex/codex-app-server-posix-supervisor.ts @@ -113,9 +113,10 @@ timer = setInterval(() => { if (ownerGone()) stopProviderGroup(null) }, 100) timer.unref() -child.once('error', () => { +child.once('error', (error) => { clearInterval(timer) - process.exit(127) + // The owner sees only this pid's exit; stderr is where a missing provider binary can say so. + process.stderr.write(String(error && error.message) + '\\n', () => process.exit(127)) }) child.once('exit', (code, signal) => { void reapProviderExit(code, signal) @@ -176,13 +177,22 @@ export function createProviderSpawnSpec( launch: CodexAppServerLaunch, childEnv: NodeJS.ProcessEnv, platform: NodeJS.Platform -): { program: string; args: string[]; env: NodeJS.ProcessEnv; cwd: string; detached: boolean } { - const supervised = platform === 'win32' ? null : supervisedPosixLaunch(launch, childEnv) +): { + program: string + args: string[] + env: NodeJS.ProcessEnv + cwd: string + detached: boolean + /** The child is the supervisor, whose SIGTERM stops the provider and then itself. */ + supervised: boolean +} { + const supervisor = platform === 'win32' ? null : supervisedPosixLaunch(launch, childEnv) return { - program: supervised?.command ?? launch.command, - args: supervised?.args ?? launch.args, - env: supervised?.env ?? childEnv, + program: supervisor?.command ?? launch.command, + args: supervisor?.args ?? launch.args, + env: supervisor?.env ?? childEnv, cwd: launch.cwd ?? process.cwd(), - detached: platform !== 'win32' + detached: platform !== 'win32', + supervised: supervisor !== null } } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.test.ts index 760b2603d99..1baaafb6f6b 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.test.ts @@ -1,17 +1,23 @@ import { describe, expect, it, vi } from 'vitest' +import { WILL_QUIT_TEARDOWN_DEADLINE_MS } from '../../../shared/quit-teardown-deadline' +import { + GRACEFUL_EXIT_MS as CLAUDE_WINDOWS_GRACEFUL_EXIT_MS, + SUPERVISED_GRACEFUL_EXIT_MS +} from '../../claude/claude-child-exit-proof-ladder' +import { GRACEFUL_EXIT_MS as CODEX_WINDOWS_GRACEFUL_EXIT_MS } from '../../codex/codex-app-server-connection' import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from '../../codex/codex-app-server-posix-supervisor' import { SNAPSHOT_DRAIN_TIMEOUT_MS } from './structured-agent-session-eviction' import { CHILD_EVICTION_TIMEOUT_MS, + EVICTION_MARGIN_MS, + RESUME_MARKER_RECORD_TIMEOUT_MS, structuredAgentSessionHostTeardownPhases } from './structured-agent-session-host-teardown' -// Codex close observes the supervisor's exit after its timers fire, which run late on a loaded host. -const SUPERVISOR_EXIT_OBSERVATION_HEADROOM_MS = 1_000 +const noop = async (): Promise => undefined describe('structured agent-session host teardown', () => { it('names every phase, so the quit-path order is pinned rather than incidental', () => { - const noop = async (): Promise => undefined const phases = structuredAgentSessionHostTeardownPhases({ idleSweep: { dispose: noop }, runtimeState: { stopLeaseRenewal: () => undefined, flushAllEventSinks: noop }, @@ -31,14 +37,54 @@ describe('structured agent-session host teardown', () => { ]) }) - it("fits the Codex supervisor's longest stop inside quit's child-eviction bound", () => { - // A quit that times out first leaves the provider running and its lease unreleased. Eviction - // drains the sink for the resume offer before it stops the child, inside the same bound. - expect( - SNAPSHOT_DRAIN_TIMEOUT_MS + - PROVIDER_SUPERVISOR_MAX_STOP_MS + - SUPERVISOR_EXIT_OBSERVATION_HEADROOM_MS - ).toBeLessThan(CHILD_EVICTION_TIMEOUT_MS) + it("derives quit's child-eviction bound from every provider's supervised close", () => { + // Eviction drains the sink for the resume offer before it stops the child, inside one bound. + // Each close's tree-kill fallback is deliberately outside it: once main exits the supervisor + // stops its group itself, and next launch's recovery settles the lease. + const closes = { + claude: SUPERVISED_GRACEFUL_EXIT_MS, + codex: PROVIDER_SUPERVISOR_MAX_STOP_MS, + // No supervisor on Windows, so its closes wait less than any supervised one. + claudeWindows: CLAUDE_WINDOWS_GRACEFUL_EXIT_MS, + codexWindows: CODEX_WINDOWS_GRACEFUL_EXIT_MS + } + expect(CHILD_EVICTION_TIMEOUT_MS).toBe( + SNAPSHOT_DRAIN_TIMEOUT_MS + Math.max(closes.claude, closes.codex) + EVICTION_MARGIN_MS + ) + for (const closeMs of Object.values(closes)) { + expect(SNAPSHOT_DRAIN_TIMEOUT_MS + closeMs + EVICTION_MARGIN_MS).toBeLessThanOrEqual( + CHILD_EVICTION_TIMEOUT_MS + ) + } + // The resume markers recorded after eviction still fit under quit's global deadline. + expect(CHILD_EVICTION_TIMEOUT_MS + RESUME_MARKER_RECORD_TIMEOUT_MS).toBeLessThan( + WILL_QUIT_TEARDOWN_DEADLINE_MS + ) + }) + + it('ends child eviction as soon as every chat has closed, not at its bound', async () => { + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }) + const evict = structuredAgentSessionHostTeardownPhases({ + idleSweep: { dispose: noop }, + runtimeState: { stopLeaseRenewal: () => undefined, flushAllEventSinks: noop }, + tasks: { drainAttaches: noop }, + evictOwnedSessions: noop, + beginResumeMarkers: () => {}, + recordResumeMarkers: noop + }).find((phase) => phase.name === 'evict-owned-sessions') + try { + let finished = false + const run = Promise.resolve(evict?.run()).then(() => { + finished = true + }) + // No timer advances: the phase settles with the eviction, not at CHILD_EVICTION_TIMEOUT_MS. + await vi.advanceTimersByTimeAsync(0) + expect(finished).toBe(true) + await run + expect(vi.getTimerCount()).toBe(0) + } finally { + vi.useRealTimers() + } }) it('bounds stalled recovery publication without preventing later cleanup', async () => { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.ts index eade95a7dd2..bc9a48c5dac 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.ts @@ -7,6 +7,9 @@ // nothing can ever close them. import type { AgentSessionResumeTrigger } from '../../../shared/agent-session-resume-marker' +import { SUPERVISED_GRACEFUL_EXIT_MS } from '../../claude/claude-child-exit-proof-ladder' +import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from '../../codex/codex-app-server-posix-supervisor' +import { SNAPSHOT_DRAIN_TIMEOUT_MS } from './structured-agent-session-eviction' import type { StructuredAgentSessionRestartResume } from './structured-agent-session-restart-resume-host' import { abandonQueuedStructuredAgentSessionMessages, @@ -23,12 +26,20 @@ export type StructuredAgentSessionTeardownPhase = { } /** Advisory persistence must not hold shutdown open. */ -const RESUME_MARKER_RECORD_TIMEOUT_MS = 2_000 +export const RESUME_MARKER_RECORD_TIMEOUT_MS = 2_000 -/** Eight steps at ten seconds each would outlast the global quit deadline, and a quit that dies - * mid-eviction leaves the lease unreleased — the exact state restart has to clean up. Bounded - * well below that deadline so the phases after this one still get to run. */ -export const CHILD_EVICTION_TIMEOUT_MS = 8_000 +/** Covers a provider's stop observed late on a loaded host. */ +export const EVICTION_MARGIN_MS = 1_000 + +/** A quit that dies mid-eviction leaves the lease unreleased — the exact state restart has to + * clean up — so this covers the sink drain plus the longest supervised provider close, well below + * the global quit deadline so later phases still run. A close's tree-kill fallback is outside it: + * once main exits the supervisor stops its group itself, and next launch's recovery settles the + * lease. Windows closes have no supervisor and wait less. */ +export const CHILD_EVICTION_TIMEOUT_MS = + SNAPSHOT_DRAIN_TIMEOUT_MS + + Math.max(SUPERVISED_GRACEFUL_EXIT_MS, PROVIDER_SUPERVISOR_MAX_STOP_MS) + + EVICTION_MARGIN_MS /** Bounds a phase without swallowing its failure, which `withTimeout` alone would. */ async function withPhaseTimeout(run: () => Promise, timeoutMs: number): Promise { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.ts index 36d950a09a4..385d438f7a0 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.ts @@ -30,8 +30,8 @@ export type StructuredSessionRecoveryResolutionDeps = { } const STOP_PROBE_INTERVAL_MS = 250 -// A POSIX Codex owner is its provider supervisor, which exits only after its provider group. A -// SIGKILL that lands first leaves the group running, so SIGTERM outlasts the supervisor's stop. +// A POSIX structured owner is its provider supervisor, which exits only after its provider +// group. A SIGKILL that lands first leaves the group running, so SIGTERM outlasts its stop. const STOP_PROBES: Record = { SIGTERM: Math.ceil(PROVIDER_SUPERVISOR_MAX_STOP_MS / STOP_PROBE_INTERVAL_MS) + 1, SIGKILL: 4