diff --git a/config/tsconfig.cli.json b/config/tsconfig.cli.json index 7c5d3509e55c..725820f664e5 100644 --- a/config/tsconfig.cli.json +++ b/config/tsconfig.cli.json @@ -86,6 +86,21 @@ "../src/main/windows-native-registry.ts", "../src/main/windows/windows-command-line-recovery-health.ts", "../src/main/windows/windows-process-table.ts", + "../src/main/windows/windows-pty-job.ts", + "../src/main/crash-reporting/crash-breadcrumb-store.ts", + "../src/main/crash-reporting/durable-crash-breadcrumb.ts", + "../src/main/crash-reporting/main-process-lifecycle-identity.ts", + "../src/main/crash-reporting/self-initiated-tree-kill-log.ts", + "../src/main/observability/redactor.ts", + "../src/main/observability/tracer.ts", + "../src/main/orca-chromium-process-pids.ts", + "../src/main/own-chromium-tree-kill-guard.ts", + "../src/main/providers/windows-foreground-process-rows.ts", + "../src/main/pty-descendant-exit-verification.ts", + "../src/main/pty-descendant-termination.ts", + "../src/main/pty-process-table-parser.ts", + "../src/main/windows-process-tree-kill.ts", + "../src/main/windows-pty-root-identity.ts", "../src/main/windows/windows-process-table-cim-scan.ts", "../src/main/daemon/daemon-process-start-time.ts", "../src/main/daemon/daemon-process-identity-query.ts", @@ -157,7 +172,12 @@ "../src/main/codex/codex-hook-wsl-runtime.ts", "../src/main/codex/codex-managed-trust-reconciliation.ts", "../src/main/provider-process/provider-process-exit-deadline.ts", + "../src/main/provider-process/managed-provider-process.ts", + "../src/main/provider-process/provider-process-close.ts", "../src/main/provider-process/provider-process-launch.ts", + "../src/main/provider-process/provider-process-supervisor.ts", + "../src/main/provider-process/provider-process-teardown.ts", + "../src/main/provider-process/provider-spawn-failure-report.ts", "../src/main/codex/codex-real-home-hook-entry-plan.ts", "../src/main/codex/codex-real-home-hook-install.ts", "../src/main/codex/codex-real-home-hook-sweep.ts", diff --git a/src/main/codex-accounts/service-account-add-login.test.ts b/src/main/codex-accounts/service-account-add-login.test.ts index 4bde86486cc2..d9b963d8bc92 100644 --- a/src/main/codex-accounts/service-account-add-login.test.ts +++ b/src/main/codex-accounts/service-account-add-login.test.ts @@ -82,6 +82,8 @@ describe('CodexAccountService config sync', () => { ) vi.doMock('node:child_process', () => ({ + // The supervised Codex teardown's process-table reader binds execFile when it loads. + execFile: vi.fn(), execFileSync: vi.fn(), spawn: spawnMock })) @@ -146,6 +148,8 @@ describe('CodexAccountService config sync', () => { ) vi.doMock('node:child_process', () => ({ + // The supervised Codex teardown's process-table reader binds execFile when it loads. + execFile: vi.fn(), execFileSync: vi.fn(), spawn: spawnMock })) @@ -254,6 +258,8 @@ describe('CodexAccountService config sync', () => { ) vi.doMock('node:child_process', () => ({ + // The supervised Codex teardown's process-table reader binds execFile when it loads. + execFile: vi.fn(), execFileSync: vi.fn(), spawn: spawnMock })) @@ -411,6 +417,8 @@ describe('CodexAccountService config sync', () => { } ) vi.doMock('node:child_process', () => ({ + // The supervised Codex teardown's process-table reader binds execFile when it loads. + execFile: vi.fn(), execFileSync: vi.fn(), spawn: spawnMock })) @@ -486,6 +494,8 @@ describe('CodexAccountService config sync', () => { const spawnMock = vi.fn() vi.doMock('node:child_process', () => ({ + // The supervised Codex teardown's process-table reader binds execFile when it loads. + execFile: vi.fn(), execFileSync: vi.fn(), spawn: spawnMock })) @@ -532,6 +542,8 @@ describe('CodexAccountService config sync', () => { const spawnMock = vi.fn() vi.doMock('node:child_process', () => ({ + // The supervised Codex teardown's process-table reader binds execFile when it loads. + execFile: vi.fn(), execFileSync: vi.fn(), spawn: spawnMock })) diff --git a/src/main/codex/codex-app-server-client.test.ts b/src/main/codex/codex-app-server-client.test.ts index ce98288bdf06..65b8e2d8b203 100644 --- a/src/main/codex/codex-app-server-client.test.ts +++ b/src/main/codex/codex-app-server-client.test.ts @@ -12,6 +12,10 @@ import { type CodexHookTrustGrantRequest } from './codex-app-server-client' import { killCodexAppServerProcessTree } from './codex-app-server-process-tree-kill' +import { + PROVIDER_SIGTERM_GRACE_MS, + PROVIDER_SUPERVISOR_MAX_STOP_MS +} from '../provider-process/provider-process-supervisor' import { runCodexAppServerSession } from './codex-app-server-session' // Stub codex app-server speaking the same JSONL protocol: initialize → @@ -19,6 +23,11 @@ import { runCodexAppServerSession } from './codex-app-server-session' // via STUB_CONFIG so each test controls listings, errors, and hangs. const STUB_SERVER_SOURCE = ` const config = JSON.parse(process.env.STUB_CONFIG) +// A wedged server ignores SIGTERM and its stdin end, so only SIGKILL ends it. +if (config.scenario === 'wedged') { + process.on('SIGTERM', () => {}) + setInterval(() => {}, 60000) +} require('node:fs').writeFileSync(config.pidFile, String(process.pid)) const trusted = new Set(config.hooks.filter(h => h.trustStatus === 'trusted').map(h => h.key)) let buffer = '' @@ -68,7 +77,7 @@ process.stdin.on('data', (chunk) => { continue } if (message.method === 'initialized') continue - if (config.scenario === 'hang') continue + if (config.scenario === 'hang' || config.scenario === 'wedged') continue if (message.method === 'hooks/list') { if (config.scenario === 'unknown-method') { send({ id: message.id, error: { code: -32601, message: 'Method not found' } }) @@ -88,7 +97,7 @@ process.stdin.on('data', (chunk) => { } } }) -process.stdin.on('end', () => process.exit(0)) +process.stdin.on('end', () => { if (config.scenario !== 'wedged') process.exit(0) }) function writeFileSyncSafe(file, contents) { require('node:fs').writeFileSync(file, contents) } ` @@ -149,6 +158,9 @@ function createStubRequest(options: { } const MANAGED_COMMAND = "/bin/sh '/tmp/orca/codex-hook.sh'" +// The deadline starts at spawn and the stub writes its pid only once it runs; on a loaded host the +// supervisor and stub have taken over 2 s to start, so a shorter deadline can stop it first. +const STUB_START_DEADLINE_MS = 8_000 function managedHook(key: string, trustStatus = 'untrusted'): StubHook { return { key, command: MANAGED_COMMAND, currentHash: `sha256:hash-of-${key}`, trustStatus } @@ -393,34 +405,59 @@ describe('runCodexHookTrustGrantSession', () => { hooks: keys.map((key) => managedHook(key)), expectedTrustKeys: keys, managedCommand: MANAGED_COMMAND, - timeoutMs: 500 + timeoutMs: STUB_START_DEADLINE_MS }) const startedAt = Date.now() await expect(runCodexHookTrustGrantSession(request)).rejects.toBeInstanceOf( CodexAppServerTimeoutError ) - // Why: the reap path must not stack the grace periods on top of the - // deadline — a wedged server may ignore everything but SIGKILL. - expect(Date.now() - startedAt).toBeLessThan(5_000) + // Why: this stub exits on its stdin end and on SIGTERM, so the reap adds no grace on top of + // the deadline; a server that ignores both is the case below. + expect(Date.now() - startedAt - STUB_START_DEADLINE_MS).toBeLessThan(2_000) const childPid = Number(readFileSync(pidFile, 'utf8')) expect(() => process.kill(childPid, 0)).toThrow() }) + it.runIf(process.platform !== 'win32')( + 'stops a server that ignores its stdin end and SIGTERM after the SIGTERM grace', + async () => { + const keys = ['/home/a/.codex/hooks.json:session_start:0:0'] + const { request, pidFile } = createStubRequest({ + scenario: 'wedged', + hooks: keys.map((key) => managedHook(key)), + expectedTrustKeys: keys, + managedCommand: MANAGED_COMMAND, + timeoutMs: STUB_START_DEADLINE_MS + }) + + const startedAt = Date.now() + await expect(runCodexHookTrustGrantSession(request)).rejects.toBeInstanceOf( + CodexAppServerTimeoutError + ) + // The supervisor SIGTERMs its group at the deadline and SIGKILLs it after the grace. + const pastDeadline = Date.now() - startedAt - STUB_START_DEADLINE_MS + expect(pastDeadline).toBeGreaterThanOrEqual(PROVIDER_SIGTERM_GRACE_MS - 100) + expect(pastDeadline).toBeLessThan(PROVIDER_SUPERVISOR_MAX_STOP_MS + 1_000) + const childPid = Number(readFileSync(pidFile, 'utf8')) + expect(() => process.kill(childPid, 0)).toThrow() + } + ) + it('bounds a callback that stalls between RPC requests', async () => { const { request, pidFile } = createStubRequest({ scenario: 'happy', hooks: [], expectedTrustKeys: [], managedCommand: MANAGED_COMMAND, - timeoutMs: 500 + timeoutMs: STUB_START_DEADLINE_MS }) const startedAt = Date.now() await expect( runCodexAppServerSession(request.invocation, async () => new Promise(() => {})) ).rejects.toBeInstanceOf(CodexAppServerTimeoutError) - expect(Date.now() - startedAt).toBeLessThan(5_000) + expect(Date.now() - startedAt - STUB_START_DEADLINE_MS).toBeLessThan(2_000) const childPid = Number(readFileSync(pidFile, 'utf8')) expect(() => process.kill(childPid, 0)).toThrow() }) diff --git a/src/main/codex/codex-app-server-session-stop.test.ts b/src/main/codex/codex-app-server-session-stop.test.ts new file mode 100644 index 000000000000..3972613394f2 --- /dev/null +++ b/src/main/codex/codex-app-server-session-stop.test.ts @@ -0,0 +1,134 @@ +import { EventEmitter } from 'node:events' +import { PassThrough } from 'node:stream' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from '../provider-process/provider-process-supervisor' +import { terminateProviderProcessTree } from '../provider-process/provider-process-teardown' +import type * as ProcessTreeKill from './codex-app-server-process-tree-kill' +import { + killCodexAppServerProcessTree, + type CodexAppServerSpawn +} from './codex-app-server-process-tree-kill' +import { CodexAppServerTimeoutError, runCodexAppServerSession } from './codex-app-server-session' + +vi.mock('../provider-process/provider-process-teardown', () => ({ + terminateProviderProcessTree: vi.fn(async () => 'exited' as const) +})) +vi.mock('./codex-app-server-process-tree-kill', async (importOriginal) => ({ + ...(await importOriginal()), + killCodexAppServerProcessTree: vi.fn() +})) + +type FakeChild = EventEmitter & { + pid: number + stdin: PassThrough + stdout: PassThrough + stderr: PassThrough + kill: ReturnType boolean>> +} + +/** A child that answers initialize and then nothing; it exits only on `exitsOn`. */ +function fakeChild(exitsOn: NodeJS.Signals | null): FakeChild { + const child: FakeChild = Object.assign(new EventEmitter(), { + pid: 4242, + stdin: new PassThrough(), + stdout: new PassThrough(), + stderr: new PassThrough(), + kill: vi.fn((signal?: NodeJS.Signals) => { + if (signal === exitsOn) { + child.emit('exit', null, signal) + child.emit('close', null, signal) + } + return true + }) + }) + child.stdin.on('data', (chunk: Buffer) => { + const message = JSON.parse(chunk.toString()) + if (message.method === 'initialize') { + child.stdout.write(`${JSON.stringify({ id: message.id, result: {} })}\n`) + } + }) + return child +} + +const originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform') + +afterEach(() => { + vi.useRealTimers() + vi.clearAllMocks() + if (originalPlatform) { + Object.defineProperty(process, 'platform', originalPlatform) + } +}) + +function startWedgedSession(child: FakeChild, platform: NodeJS.Platform): Promise { + Object.defineProperty(process, 'platform', { configurable: true, value: platform }) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the session reads only pid, stdio, kill and the exit/close/error events, which the fake implements. + const spawnImpl: CodexAppServerSpawn = () => child as never + return runCodexAppServerSession( + { command: '/bin/codex', cliPath: null, args: ['app-server'], timeoutMs: 1_000 }, + () => new Promise(() => {}), + spawnImpl + ).catch((error: unknown) => error) +} + +describe('runCodexAppServerSession stop', () => { + it('SIGTERMs a supervised session at its deadline instead of waiting out its stdin grace', async () => { + vi.useFakeTimers() + const child = fakeChild('SIGTERM') + let stdinEnded = false + child.stdin.on('finish', () => (stdinEnded = true)) + + const outcome = startWedgedSession(child, 'darwin') + await vi.advanceTimersByTimeAsync(1_000) + + expect(child.kill.mock.calls).toEqual([['SIGTERM']]) + expect(stdinEnded).toBe(true) + expect(await outcome).toBeInstanceOf(CodexAppServerTimeoutError) + expect(terminateProviderProcessTree).not.toHaveBeenCalled() + expect(killCodexAppServerProcessTree).not.toHaveBeenCalled() + }) + + it('tears a supervised session down only once the supervisor has had its full stop time', async () => { + vi.useFakeTimers() + const child = fakeChild(null) + + const outcome = startWedgedSession(child, 'linux') + await vi.advanceTimersByTimeAsync(1_000 + PROVIDER_SUPERVISOR_MAX_STOP_MS - 1) + expect(terminateProviderProcessTree).not.toHaveBeenCalled() + await vi.advanceTimersByTimeAsync(1) + + expect(terminateProviderProcessTree).toHaveBeenCalledWith(child, { + site: 'codex-app-server-session', + platform: 'linux' + }) + await vi.advanceTimersByTimeAsync(1_000) + expect(await outcome).toBeInstanceOf(CodexAppServerTimeoutError) + // The session's only own signal is the deadline SIGTERM; only the teardown forces. + expect(child.kill.mock.calls).toEqual([['SIGTERM']]) + expect(killCodexAppServerProcessTree).not.toHaveBeenCalled() + }) + + it('keeps the Windows deadline kill and its 1.5 s close wait', async () => { + vi.useFakeTimers() + const child = fakeChild(null) + + const outcome = startWedgedSession(child, 'win32') + await vi.advanceTimersByTimeAsync(1_000) + expect(killCodexAppServerProcessTree).toHaveBeenCalledTimes(1) + await vi.advanceTimersByTimeAsync(1_499) + expect(terminateProviderProcessTree).not.toHaveBeenCalled() + await vi.advanceTimersByTimeAsync(1) + // The shared close's Windows teardown (taskkill of the tree), as the Codex connection's. + expect(terminateProviderProcessTree).toHaveBeenCalledWith(child, { + site: 'codex-app-server-session', + platform: 'win32' + }) + + await vi.advanceTimersByTimeAsync(1_000) + expect(await outcome).toBeInstanceOf(CodexAppServerTimeoutError) + // The session sends no signal of its own on Windows (a SIGTERM there is TerminateProcess); + // the teardowns, mocked here, own the kill. + expect(child.kill).not.toHaveBeenCalled() + expect(killCodexAppServerProcessTree).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/main/codex/codex-app-server-session-supervision.test.ts b/src/main/codex/codex-app-server-session-supervision.test.ts new file mode 100644 index 000000000000..c7649e3d4f76 --- /dev/null +++ b/src/main/codex/codex-app-server-session-supervision.test.ts @@ -0,0 +1,219 @@ +import { EventEmitter } from 'node:events' +import { existsSync, readFileSync } from 'node:fs' +import { PassThrough } from 'node:stream' +import { join } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import { + PROVIDER_STDIN_END_GRACE_MS, + PROVIDER_SUPERVISOR_MAX_STOP_MS +} from '../provider-process/provider-process-supervisor' +import type { CodexAppServerSpawn } from './codex-app-server-process-tree-kill' +import { CodexAppServerTimeoutError, runCodexAppServerSession } from './codex-app-server-session' +import { classifyCodexTrustGrantError } from './codex-trust-grant-telemetry' +import { PROVIDER_SPAWN_FAILURE_MARKER } from '../provider-process/provider-spawn-failure-report' +import { + alive, + createSupervisedProbeRig, + waitFor, + type SupervisedProbeRig +} from './supervised-probe-owner.test-fixture' + +// Answers initialize, then nothing; it ignores its stdin end, as a wedged Codex can. +const WEDGED_APP_SERVER = String.raw` +require('node:readline').createInterface({ input: process.stdin }).on('line', (line) => { + const message = JSON.parse(line) + if (message.method === 'initialize') { + process.stdout.write(JSON.stringify({ id: message.id, result: {} }) + '\n') + } +}) +` + +const OWNER = String.raw` +import { runCodexAppServerSession } from './codex-app-server-session' +void runCodexAppServerSession( + { command: process.env.ORCA_TEST_STAND_IN!, cliPath: null, args: ['app-server'], timeoutMs: 120_000 }, + () => new Promise(() => {}) +).catch(() => {}) +setInterval(() => {}, 60_000) +` + +const originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform') +let rig: SupervisedProbeRig | null = null + +afterEach(() => { + if (originalPlatform) { + Object.defineProperty(process, 'platform', originalPlatform) + } + rig?.cleanup() + rig = null +}) + +type CapturedSpawn = { program: string; args: string[]; options: Record } + +function captureSpawn(): { spawnImpl: CodexAppServerSpawn; calls: CapturedSpawn[] } { + const calls: CapturedSpawn[] = [] + return { + calls, + spawnImpl: (program, args, options) => { + calls.push({ program, args, options }) + throw new Error('spawn captured') + } + } +} + +function decodedSupervisorSpec(env: unknown): unknown { + const encoded = + typeof env === 'object' && env !== null && 'ORCA_PROVIDER_SUPERVISOR_SPEC' in env + ? String(env.ORCA_PROVIDER_SUPERVISOR_SPEC) + : '' + return JSON.parse(Buffer.from(encoded, 'base64').toString() || 'null') +} + +describe('short-lived Codex app-server session spawn', () => { + it('runs a POSIX session under the provider supervisor in session lifetime', async () => { + Object.defineProperty(process, 'platform', { configurable: true, value: 'darwin' }) + const { spawnImpl, calls } = captureSpawn() + + await expect( + runCodexAppServerSession( + { command: '/bin/codex', cliPath: null, args: ['app-server'], timeoutMs: 1_000 }, + async () => null, + spawnImpl + ) + ).rejects.toThrow('spawn captured') + + const [{ program, args, options }] = calls + expect(program).toBe(process.execPath) + expect(args.slice(args.indexOf('--') + 1)).toEqual(['/bin/codex', 'app-server']) + expect(options.detached).toBe(true) + expect(decodedSupervisorSpec(options.env)).toMatchObject({ + lifetime: 'session', + ownerPid: process.pid + }) + }) + + it('never shows the supervisor spawn-failure marker in an early-exit error', async () => { + Object.defineProperty(process, 'platform', { configurable: true, value: 'darwin' }) + const report = { thrown: false, code: 'ENOENT', message: 'spawn /bin/codex ENOENT' } + const child = Object.assign(new EventEmitter(), { + pid: 4242, + stdin: new PassThrough(), + stdout: new PassThrough(), + stderr: new PassThrough(), + kill: () => true + }) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the session reads only pid, stdio, kill and the exit/close/error events, which the fake implements. + const spawnImpl: CodexAppServerSpawn = () => child as never + // A report on an exit other than 127 is no spawn failure, but its text still reaches the user. + child.stderr.once('end', () => { + child.emit('exit', 1, null) + child.emit('close', 1, null) + }) + child.stderr.end(`${PROVIDER_SPAWN_FAILURE_MARKER}${JSON.stringify(report)}\n`) + + const error = await runCodexAppServerSession( + { command: '/bin/codex', cliPath: null, args: ['app-server'], timeoutMs: 5_000 }, + async () => null, + spawnImpl + ).catch((caught: unknown) => caught) + + expect(String(error)).toContain('spawn /bin/codex ENOENT') + expect(String(error)).not.toContain(PROVIDER_SPAWN_FAILURE_MARKER) + }) + + it('spawns a Windows session directly, as before', async () => { + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + const { spawnImpl, calls } = captureSpawn() + + await expect( + runCodexAppServerSession( + { command: 'C:\\codex.exe', cliPath: null, args: ['app-server'], timeoutMs: 1_000 }, + async () => null, + spawnImpl + ) + ).rejects.toThrow('spawn captured') + + const [{ program, args, options }] = calls + expect(decodedSupervisorSpec(options.env)).toBeNull() + expect(program).toBe('C:\\codex.exe') + expect(args).toEqual(['app-server']) + expect(options).not.toHaveProperty('detached') + }) +}) + +describe.runIf(process.platform !== 'win32')('supervised Codex app-server sessions', () => { + it('stops a wedged session at its deadline with SIGTERM to its whole group', async () => { + rig = createSupervisedProbeRig() + const standIn = rig.writeStandIn('codex', WEDGED_APP_SERVER) + + const readPids = rig.readPids + const reported: { pids?: { provider: number; grandchild: number } } = {} + // The deadline starts at spawn, so it outlasts the stand-in's whole start budget: a slow host + // fails the pid read inside that budget instead of stopping a stand-in that never armed. + const startBudgetMs = 8_000 + const session = runCodexAppServerSession( + { + command: standIn, + cliPath: null, + args: ['app-server'], + env: rig.env, + timeoutMs: startBudgetMs + 2_000 + }, + async () => { + reported.pids = await readPids(startBudgetMs) + return new Promise(() => {}) + } + ) + + await expect(session).rejects.toBeInstanceOf(CodexAppServerTimeoutError) + const { pids } = reported + expect(pids).toBeDefined() + if (!pids) { + return + } + // Graceful first: the old deadline SIGKILLed the root, which a Codex mid-write cannot survive. + expect(existsSync(rig.signalFile) && readFileSync(rig.signalFile, 'utf8')).toBe('SIGTERM') + expect(alive(pids.provider)).toBe(false) + // The group ladder reaches a descendant that ignores SIGTERM; a root-only kill misses it. + expect(alive(pids.grandchild)).toBe(false) + }) + + it('drains, then stops the session group, when its owner is SIGKILLed mid-session', async () => { + rig = createSupervisedProbeRig() + const standIn = rig.writeStandIn('codex', WEDGED_APP_SERVER) + const bundle = await rig.bundleOwner(OWNER, __dirname) + const owner = rig.launchOwner(bundle, { ORCA_TEST_STAND_IN: standIn }) + const pids = await rig.readPids() + + owner.kill('SIGKILL') + + expect( + await waitFor( + () => !alive(pids.provider) && !alive(pids.grandchild), + PROVIDER_SUPERVISOR_MAX_STOP_MS + 1_000 + ) + ).toBe(true) + // A gone owner gets the close its owner makes: the stdin end, its grace, and only then SIGTERM. + const events = rig.readEvents() + expect(events['stdin-end']).toBeDefined() + expect((events.SIGTERM ?? 0) - (events['stdin-end'] ?? 0)).toBeGreaterThanOrEqual( + PROVIDER_STDIN_END_GRACE_MS - 50 + ) + }) + + it('reports a missing Codex binary as the spawn error, not an early exit', async () => { + rig = createSupervisedProbeRig() + const error = await runCodexAppServerSession( + { + command: join(rig.dir, 'missing', 'codex'), + cliPath: null, + args: ['app-server'], + timeoutMs: 5_000 + }, + async () => null + ).catch((caught: unknown) => caught) + + expect(error).toMatchObject({ code: 'ENOENT' }) + expect(classifyCodexTrustGrantError(error)).toBe('binary-missing') + }) +}) diff --git a/src/main/codex/codex-app-server-session.ts b/src/main/codex/codex-app-server-session.ts index b9dc838f86c4..122d95c16f54 100644 --- a/src/main/codex/codex-app-server-session.ts +++ b/src/main/codex/codex-app-server-session.ts @@ -1,8 +1,12 @@ import type { ChildProcessWithoutNullStreams } from 'node:child_process' -import { waitForProcessExitUntil } from '../provider-process/provider-process-exit-deadline' +import { spawnManagedProviderProcess } from '../provider-process/managed-provider-process' import { resolveProviderChildEnv } from '../provider-process/provider-process-launch' import { stderrIndicatesMissingAppServer } from './codex-app-server-capability-signal' import { withCliRuntimeOnPath } from '../../shared/node-cli-command-resolution' +import { + providerStderrForDisplay, + supervisedProviderSpawnFailure +} from '../provider-process/provider-spawn-failure-report' import { killCodexAppServerProcessTree, spawnCodexAppServerProcess, @@ -34,7 +38,7 @@ export type CodexAppServerInvocation = { /** Env keys stripped from the inherited environment before spawn (e.g. an inherited * CODEX_HOME, so `listCodexHooks` with no `codexHome` lists the real ~/.codex). */ envToDelete?: readonly string[] - /** Whole-session deadline. The codex child is SIGKILLed when it lapses. */ + /** Whole-session deadline. The codex child is stopped when it lapses. */ timeoutMs: number } @@ -76,7 +80,8 @@ export type CodexAppServerRpc = { } const JSON_RPC_METHOD_NOT_FOUND = -32601 -const STDERR_TAIL_MAX_BYTES = 8192 +const STDERR_DETAIL_MAX_CHARS = 400 +const CODEX_APP_SERVER_KILL_SITE = 'codex-app-server-session' /** Codex answering "no such method" is the only response that proves the RPC * surface is absent rather than temporarily failing. */ @@ -94,7 +99,8 @@ export function isCodexMethodNotFoundError(error: unknown): boolean { /** * Runs one short-lived `codex app-server` session over stdio JSON-RPC (JSONL): * spawn → initialize → initialized → body(rpc) → EOF/reap. The child is reaped - * on every path; the session deadline SIGKILLs it. + * on every path; the session deadline stops it. On POSIX it runs under the provider + * supervisor, so an Orca that quits or dies mid-session still stops its group. */ export async function runCodexAppServerSession( invocation: CodexAppServerInvocation, @@ -107,14 +113,26 @@ export async function runCodexAppServerSession( const pairedEnv = invocation.cliPath ? withCliRuntimeOnPath(invocation.cliPath, childEnv) : childEnv - const child = spawnImpl(invocation.command, invocation.args, { - env: pairedEnv, - stdio: ['pipe', 'pipe', 'pipe'], - windowsHide: true - }) as ChildProcessWithoutNullStreams + // The Codex connection's managed process and close policy, so a session closes the same way. + const managed = spawnManagedProviderProcess( + { command: invocation.command, args: invocation.args }, + { + site: CODEX_APP_SERVER_KILL_SITE, + inheritedEnv: pairedEnv, + spawnImpl: (spec) => + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: every stdio slot is 'pipe', so stdin, stdout and stderr exist. + spawnImpl(spec.program, [...(spec.args ?? [])], { + env: spec.env, + stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, + ...(spec.detached ? { detached: true } : {}) + }) as ChildProcessWithoutNullStreams + } + ) + const { child } = managed - let stderrTail = '' let exited = false + let exitCode: number | null = null let nextRequestId = 1 let timedOut = false const pending = new Map< @@ -122,11 +140,9 @@ export async function runCodexAppServerSession( { resolve: (r: JsonRpcResponse) => void; reject: (e: Error) => void } >() - const exitPromise = new Promise((resolve) => { - child.on('exit', () => { - exited = true - resolve() - }) + managed.onExit((exit) => { + exited = true + exitCode = exit.code }) // Why: 'error' fires instead of 'exit' when the spawn itself fails // (ENOENT); surface it to every in-flight request or they wait forever. @@ -141,11 +157,6 @@ export async function runCodexAppServerSession( child.on('close', () => { failPending(buildEarlyExitError()) }) - // Why: JSONL can contain non-ASCII hook paths. Stream decoding must retain a - // multibyte character split across pipe chunks or the response becomes invalid JSON. - child.stderr.setEncoding('utf8').on('data', (chunk: string) => { - stderrTail = (stderrTail + chunk).slice(-STDERR_TAIL_MAX_BYTES) - }) // Why: a child can exit between the liveness check and stdin.write(); an // EPIPE must reject the RPC instead of becoming an unhandled stream error. child.stdin.on('error', (error) => { @@ -188,7 +199,10 @@ export async function runCodexAppServerSession( const error = new CodexAppServerTimeoutError( `codex app-server session exceeded ${invocation.timeoutMs}ms (${invocation.command})` ) - killCodexAppServerProcessTree(child) + // A supervised child is stopped by the finally below, which this rejection runs at once. + if (!managed.supervised) { + killCodexAppServerProcessTree(child) + } failPending(error) rejectDeadline(error) }, invocation.timeoutMs) @@ -260,14 +274,26 @@ export async function runCodexAppServerSession( return response.result } + // The supervisor's spawn-failure report reads as Node's own spawn error line. + function stderrDetail(): string { + return providerStderrForDisplay(managed.stderrTail()).trim().slice(0, STDERR_DETAIL_MAX_CHARS) + } + function buildEarlyExitError(): Error { - if (stderrIndicatesMissingAppServer(stderrTail)) { + // A supervisor reports a provider it could not start as exit 127; callers classify that error. + const spawnFailure = managed.supervised + ? supervisedProviderSpawnFailure(exitCode, managed.stderrTail()) + : null + if (spawnFailure) { + return spawnFailure.error + } + if (stderrIndicatesMissingAppServer(managed.stderrTail())) { return new CodexAppServerUnsupportedError( - `codex CLI does not support the app-server subcommand: ${stderrTail.trim().slice(0, 400)}` + `codex CLI does not support the app-server subcommand: ${stderrDetail()}` ) } return new Error( - `codex app-server exited before completing the session${stderrTail ? `: ${stderrTail.trim().slice(0, 400)}` : ''}` + `codex app-server exited before completing the session${managed.stderrTail() ? `: ${stderrDetail()}` : ''}` ) } @@ -287,28 +313,25 @@ export async function runCodexAppServerSession( error instanceof Error && !(error instanceof CodexAppServerUnsupportedError) && !(error instanceof CodexAppServerTimeoutError) && - stderrIndicatesMissingAppServer(stderrTail) + stderrIndicatesMissingAppServer(managed.stderrTail()) ) { throw new CodexAppServerUnsupportedError( - `codex CLI does not support the app-server subcommand: ${stderrTail.trim().slice(0, 400)}` + `codex CLI does not support the app-server subcommand: ${stderrDetail()}` ) } throw error } finally { - try { - child.stdin.end() - } catch { - // stdin may already be destroyed after a kill; reaping below still runs. - } - if (!exited) { - // Why: the server exits promptly on stdin EOF; the grace period only - // bounds a wedged child before the guaranteed SIGKILL reap. - await waitForProcessExitUntil(exitPromise, 1500) - if (!exited) { - killCodexAppServerProcessTree(child) - await waitForProcessExitUntil(exitPromise, 1000) + // A session past its deadline is wedged; its stdin end would only add a grace. + if (timedOut && managed.supervised && !exited) { + try { + child.stdin.end() + } catch { + // A destroyed stdin still leaves the SIGTERM and the close below. } + child.kill('SIGTERM') } + // Ends stdin, gives a supervisor its full stop time (a direct child 1.5 s), then the tree. + await managed.close() clearTimeout(deadline) } } diff --git a/src/main/codex/codex-model-catalog-probe.ts b/src/main/codex/codex-model-catalog-probe.ts index 2cf354259d74..66def74598c2 100644 --- a/src/main/codex/codex-model-catalog-probe.ts +++ b/src/main/codex/codex-model-catalog-probe.ts @@ -10,8 +10,8 @@ import type { AgentModelCatalogSuccess } from '../native-chat/agent-model-catalog/agent-model-catalog-store' -// Why 15s: the whole probe session is SIGKILLed at the deadline, and a cold -// `model/list` may pay one network /models fetch behind provider auth. +// Why 15s: the whole probe session is stopped at the deadline (on POSIX its supervisor then gets +// up to PROVIDER_SUPERVISOR_MAX_STOP_MS), and a cold `model/list` may pay one /models fetch. const CODEX_MODEL_CATALOG_PROBE_TIMEOUT_MS = 15_000 export type CodexModelCatalogProbeDeps = Pick< diff --git a/src/main/codex/codex-state-db-backfill-recovery-process.test.ts b/src/main/codex/codex-state-db-backfill-recovery-process.test.ts new file mode 100644 index 000000000000..7f7659dc5376 --- /dev/null +++ b/src/main/codex/codex-state-db-backfill-recovery-process.test.ts @@ -0,0 +1,79 @@ +import { EventEmitter } from 'node:events' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { ChildProcessHandle } from '../../shared/child-process/process-spec' +import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from '../provider-process/provider-process-supervisor' +import { terminateProviderProcessTree } from '../provider-process/provider-process-teardown' +import { stopCodexBackfillRecoveryProcess } from './codex-state-db-backfill-recovery-process' + +vi.mock('../provider-process/provider-process-teardown', () => ({ + terminateProviderProcessTree: vi.fn(async () => 'exited' as const) +})) + +type FakeSupervisor = EventEmitter & { + pid: number + exitCode: number | null + signalCode: NodeJS.Signals | null + stdin: { end: ReturnType } + kill: ReturnType boolean>> +} + +function fakeSupervisor(exitsOnStdinEnd: boolean): FakeSupervisor { + const child: FakeSupervisor = Object.assign(new EventEmitter(), { + pid: 4242, + exitCode: null, + signalCode: null, + stdin: { + end: vi.fn(() => { + if (exitsOnStdinEnd) { + child.exitCode = 0 + child.emit('exit', 0, null) + } + }) + }, + kill: vi.fn(() => true) + }) + return child +} + +function asHandle(child: FakeSupervisor): ChildProcessHandle { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the stop reads only pid, exit state, stdin, kill and the exit event, which the fake implements. + return child as unknown as ChildProcessHandle +} + +afterEach(() => { + vi.useRealTimers() + vi.clearAllMocks() +}) + +describe('stopCodexBackfillRecoveryProcess for a supervised app-server', () => { + it('ends its input and leaves the group stop to the supervisor', async () => { + const child = fakeSupervisor(true) + + await stopCodexBackfillRecoveryProcess(asHandle(child), true) + + expect(child.stdin.end).toHaveBeenCalledOnce() + // A direct SIGTERM would skip the stdin-end grace a Codex close gets. + expect(child.kill).not.toHaveBeenCalled() + expect(terminateProviderProcessTree).not.toHaveBeenCalled() + }) + + it('tears the tree down only once the supervisor has had its full stop time', async () => { + vi.useFakeTimers() + const child = fakeSupervisor(false) + + const stopped = stopCodexBackfillRecoveryProcess(asHandle(child), true) + await vi.advanceTimersByTimeAsync(PROVIDER_SUPERVISOR_MAX_STOP_MS - 1) + expect(terminateProviderProcessTree).not.toHaveBeenCalled() + await vi.advanceTimersByTimeAsync(1) + expect(terminateProviderProcessTree).toHaveBeenCalledWith(child, { + site: 'codex-state-db-backfill-recovery' + }) + // The shared close then waits, bounded, for the forced root's exit. + await vi.advanceTimersByTimeAsync(1_000) + await stopped + + // The stop signals nothing itself: the stdin end asks, and only the teardown forces. + expect(child.stdin.end).toHaveBeenCalledOnce() + expect(child.kill).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/codex/codex-state-db-backfill-recovery-process.ts b/src/main/codex/codex-state-db-backfill-recovery-process.ts new file mode 100644 index 000000000000..77bd3e537322 --- /dev/null +++ b/src/main/codex/codex-state-db-backfill-recovery-process.ts @@ -0,0 +1,75 @@ +import type { ChildProcessHandle } from '../../shared/child-process/process-spec' +import { withCliRuntimeOnPath } from '../../shared/node-cli-command-resolution' +import { parseWslUncPath } from '../../shared/wsl-paths' +import { buildWslCodexAppServerArgs } from '../codex-accounts/wsl-codex-command' +import { CODEX_READ_ONLY_APP_SERVER_ARGS } from '../codex-cli/codex-read-only-app-server-args' +import { terminateCodexProbeChild } from '../rate-limits/codex-probe-termination' +import { + createProviderSpawnSpec, + type ProviderCloseRequest +} from '../provider-process/provider-process-supervisor' +import type { CodexAppServerSpawn } from './codex-app-server-process-tree-kill' +import { stopSupervisedChildProcess } from '../provider-process/supervised-child-process-stop' + +const BACKFILL_RECOVERY_KILL_SITE = 'codex-state-db-backfill-recovery' +// Codex drains on its stdin end, as the Codex connection's root-only close asks; one value +// for the spawn spec (a gone Orca) and the stop (Orca stopping it), so they cannot drift. +const BACKFILL_RECOVERY_CLOSE_REQUEST: ProviderCloseRequest = 'stdin-end' + +export type CodexBackfillRecoveryProcess = { child: ChildProcessHandle; supervised: boolean } + +/** Starts the read-only app-server whose presence lets Codex finish its own backfill. */ +export function spawnCodexBackfillRecoveryProcess( + codexHomePath: string, + spawnProcess: CodexAppServerSpawn, + resolveCommand: () => string +): CodexBackfillRecoveryProcess { + const wslHome = process.platform === 'win32' ? parseWslUncPath(codexHomePath) : null + if (wslHome) { + const child = spawnProcess( + 'wsl.exe', + buildWslCodexAppServerArgs( + wslHome.distro, + wslHome.linuxPath, + CODEX_READ_ONLY_APP_SERVER_ARGS + ), + { + stdio: ['pipe', 'ignore', 'ignore'], + windowsHide: true, + env: process.env + } + ) + return { child, supervised: false } + } + const command = resolveCommand() + // Kept alive up to an hour; on POSIX the supervisor stops it with Orca, however Orca exits. + const spawnSpec = createProviderSpawnSpec( + { command, args: [...CODEX_READ_ONLY_APP_SERVER_ARGS], cwd: codexHomePath }, + withCliRuntimeOnPath(command, { ...process.env, CODEX_HOME: codexHomePath }), + process.platform, + { lifetime: 'session', closeRequest: BACKFILL_RECOVERY_CLOSE_REQUEST } + ) + const child = spawnProcess(spawnSpec.program, spawnSpec.args, { + cwd: codexHomePath, + stdio: ['pipe', 'ignore', 'ignore'], + windowsHide: true, + env: spawnSpec.env, + ...(spawnSpec.supervised ? { detached: true } : {}) + }) + return { child, supervised: spawnSpec.supervised } +} + +export async function stopCodexBackfillRecoveryProcess( + child: ChildProcessHandle, + supervised: boolean +): Promise { + if (supervised) { + // A session supervisor turns stdin end into its group stop, as a Codex connection close does. + await stopSupervisedChildProcess(child, { + site: BACKFILL_RECOVERY_KILL_SITE, + closeRequest: BACKFILL_RECOVERY_CLOSE_REQUEST + }) + return + } + await terminateCodexProbeChild(child) +} diff --git a/src/main/codex/codex-state-db-backfill-recovery-supervision.test.ts b/src/main/codex/codex-state-db-backfill-recovery-supervision.test.ts new file mode 100644 index 000000000000..cc3277ecd37b --- /dev/null +++ b/src/main/codex/codex-state-db-backfill-recovery-supervision.test.ts @@ -0,0 +1,146 @@ +import { ChildProcess } from 'node:child_process' +import { existsSync, readFileSync } from 'node:fs' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { CODEX_READ_ONLY_APP_SERVER_ARGS } from '../codex-cli/codex-read-only-app-server-args' +import { + PROVIDER_STDIN_END_GRACE_MS, + PROVIDER_SUPERVISOR_MAX_STOP_MS +} from '../provider-process/provider-process-supervisor' +import type { CodexAppServerSpawn } from './codex-app-server-process-tree-kill' +import type { CodexStateDbBackfillStatus } from './codex-state-db' +import { runCodexStateDbBackfillRecovery } from './codex-state-db-backfill-recovery' +import { + alive, + createSupervisedProbeRig, + waitFor, + type SupervisedProbeRig +} from './supervised-probe-owner.test-fixture' + +const INCOMPLETE: CodexStateDbBackfillStatus = { + kind: 'incomplete', + stateDbPath: '/state.sqlite', + status: 'running' +} + +const OWNER = String.raw` +import { runCodexStateDbBackfillRecovery } from './codex-state-db-backfill-recovery' +void runCodexStateDbBackfillRecovery(process.env.ORCA_TEST_CODEX_HOME!, new AbortController().signal, { + resolveCommand: () => process.env.ORCA_TEST_STAND_IN!, + readStatus: () => ({ kind: 'incomplete', stateDbPath: '/state.sqlite', status: 'running' }) +}).catch(() => {}) +setInterval(() => {}, 60_000) +` + +const originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform') +let rig: SupervisedProbeRig | null = null + +afterEach(() => { + if (originalPlatform) { + Object.defineProperty(process, 'platform', originalPlatform) + } + vi.unstubAllEnvs() + rig?.cleanup() + rig = null +}) + +function decodedSupervisorSpec(env: unknown): unknown { + const encoded = + typeof env === 'object' && env !== null && 'ORCA_PROVIDER_SUPERVISOR_SPEC' in env + ? String(env.ORCA_PROVIDER_SUPERVISOR_SPEC) + : '' + return JSON.parse(Buffer.from(encoded, 'base64').toString() || 'null') +} + +describe('Codex backfill recovery app-server supervision', () => { + it('runs the POSIX recovery app-server under a session-lifetime supervisor', async () => { + Object.defineProperty(process, 'platform', { configurable: true, value: 'linux' }) + // Never signalled: terminate is a stub here, and a handle-less ChildProcess must not be killed. + const child = new ChildProcess() + const calls: { program: string; args: string[]; options: Record }[] = [] + const spawnProcess: CodexAppServerSpawn = (program, args, options) => { + calls.push({ program, args, options }) + return child + } + const terminate = vi.fn(async () => {}) + const readStatus = vi + .fn() + .mockReturnValueOnce(INCOMPLETE) + .mockReturnValue({ kind: 'complete', stateDbPath: '/state.sqlite' }) + + await expect( + runCodexStateDbBackfillRecovery('/managed-home', new AbortController().signal, { + spawnProcess, + resolveCommand: () => '/bin/codex', + readStatus, + terminate, + sleep: vi.fn(async () => {}), + now: vi.fn(() => 1_000) + }) + ).resolves.toEqual({ outcome: 'completed', spawnCount: 1 }) + + const [{ program, args, options }] = calls + expect(program).toBe(process.execPath) + expect(args.slice(args.indexOf('--') + 1)).toEqual([ + '/bin/codex', + ...CODEX_READ_ONLY_APP_SERVER_ARGS + ]) + expect(options).toMatchObject({ cwd: '/managed-home', detached: true }) + expect(options.env).toMatchObject({ CODEX_HOME: '/managed-home' }) + expect(decodedSupervisorSpec(options.env)).toMatchObject({ + lifetime: 'session', + cwd: '/managed-home' + }) + expect(terminate).toHaveBeenCalledWith(child, true) + }) +}) + +describe.runIf(process.platform !== 'win32')('supervised Codex backfill recovery processes', () => { + it('stops the whole recovery group when the recovery is aborted', async () => { + rig = createSupervisedProbeRig() + for (const [key, value] of Object.entries(rig.env)) { + vi.stubEnv(key, value) + } + const standIn = rig.writeStandIn('codex') + const controller = new AbortController() + + const recovery = runCodexStateDbBackfillRecovery(rig.dir, controller.signal, { + resolveCommand: () => standIn, + readStatus: () => INCOMPLETE + }) + const pids = await rig.readPids() + controller.abort() + + await expect(recovery).resolves.toEqual({ outcome: 'stopped', spawnCount: 1 }) + // Its stdin end comes first; the supervisor then SIGTERMs, and SIGKILLs the group after. + expect(existsSync(rig.signalFile) && readFileSync(rig.signalFile, 'utf8')).toBe('SIGTERM') + expect(alive(pids.provider)).toBe(false) + // A SIGTERM to the root alone left this descendant running for good. + expect(alive(pids.grandchild)).toBe(false) + }) + + it('drains, then stops the recovery group, when its owner is SIGKILLed', async () => { + rig = createSupervisedProbeRig() + const standIn = rig.writeStandIn('codex') + const bundle = await rig.bundleOwner(OWNER, __dirname) + const owner = rig.launchOwner(bundle, { + ORCA_TEST_STAND_IN: standIn, + ORCA_TEST_CODEX_HOME: rig.dir + }) + const pids = await rig.readPids() + + owner.kill('SIGKILL') + + expect( + await waitFor( + () => !alive(pids.provider) && !alive(pids.grandchild), + PROVIDER_SUPERVISOR_MAX_STOP_MS + 1_000 + ) + ).toBe(true) + // A gone owner gets the close its owner makes: the stdin end, its grace, and only then SIGTERM. + const events = rig.readEvents() + expect(events['stdin-end']).toBeDefined() + expect((events.SIGTERM ?? 0) - (events['stdin-end'] ?? 0)).toBeGreaterThanOrEqual( + PROVIDER_STDIN_END_GRACE_MS - 50 + ) + }) +}) diff --git a/src/main/codex/codex-state-db-backfill-recovery.test.ts b/src/main/codex/codex-state-db-backfill-recovery.test.ts index a76c5f65a5a6..6577bf3bd53e 100644 --- a/src/main/codex/codex-state-db-backfill-recovery.test.ts +++ b/src/main/codex/codex-state-db-backfill-recovery.test.ts @@ -357,7 +357,7 @@ describe('Codex state DB backfill recovery', () => { expect(spawnProcess).toHaveBeenCalledTimes(2) expect(timerDurations).toEqual([5_000, 2_000, 5_000]) expect(terminate).toHaveBeenCalledTimes(1) - expect(terminate).toHaveBeenCalledWith(children[1]) + expect(terminate).toHaveBeenCalledWith(children[1], process.platform !== 'win32') }) it('keeps the successful app-server claimant alive until Codex marks its DB complete', async () => { @@ -377,7 +377,7 @@ describe('Codex state DB backfill recovery', () => { now: vi.fn(() => 1_000) }) ).resolves.toEqual({ outcome: 'completed', spawnCount: 1 }) - expect(terminate).toHaveBeenCalledWith(child) + expect(terminate).toHaveBeenCalledWith(child, process.platform !== 'win32') }) it('hands the Codex shim itself to the spawn chokepoint for native Windows recovery', async () => { @@ -389,6 +389,7 @@ describe('Codex state DB backfill recovery', () => { .fn() .mockReturnValueOnce({ kind: 'incomplete', stateDbPath: 'state.sqlite', status: 'running' }) .mockReturnValue({ kind: 'complete', stateDbPath: 'state.sqlite' }) + const terminate = vi.fn(async () => {}) await runCodexStateDbBackfillRecovery( 'C:\\Users\\alice\\.codex', @@ -397,7 +398,7 @@ describe('Codex state DB backfill recovery', () => { spawnProcess: spawnProcess as never, resolveCommand: () => codexCommand, readStatus, - terminate: vi.fn(async () => {}), + terminate, sleep: vi.fn(async () => {}), now: vi.fn(() => 1_000) } @@ -421,6 +422,9 @@ describe('Codex state DB backfill recovery', () => { ]) expect(spawnOptions.cwd).toBe('C:\\Users\\alice\\.codex') expect(spawnOptions.env?.CODEX_HOME).toBe('C:\\Users\\alice\\.codex') + // Windows has no supervisor: the drain-first probe termination stays its stop. + expect(spawnOptions).not.toHaveProperty('detached') + expect(terminate).toHaveBeenCalledWith(child, false) }) it('retries a live foreign lease until one durable claimant can recover it', async () => { diff --git a/src/main/codex/codex-state-db-backfill-recovery.ts b/src/main/codex/codex-state-db-backfill-recovery.ts index b4acef3b3375..85635bf0e6ab 100644 --- a/src/main/codex/codex-state-db-backfill-recovery.ts +++ b/src/main/codex/codex-state-db-backfill-recovery.ts @@ -5,16 +5,16 @@ import { normalizeRuntimePathForComparison } from '../../shared/cross-platform-p import { parseWslUncPath } from '../../shared/wsl-paths' import { withManagedHookInstallLock } from '../agent-hooks/managed-hook-install-lock' import { readManagedHookHostIdentity } from '../agent-hooks/managed-hook-owner-identity' -import { buildWslCodexAppServerArgs } from '../codex-accounts/wsl-codex-command' import { resolveCodexCommand } from '../codex-cli/command' -import { withCliRuntimeOnPath } from '../../shared/node-cli-command-resolution' -import { CODEX_READ_ONLY_APP_SERVER_ARGS } from '../codex-cli/codex-read-only-app-server-args' -import { terminateCodexProbeChild } from '../rate-limits/codex-probe-termination' import type { ChildProcessHandle } from '../../shared/child-process/process-spec' import { spawnCodexAppServerProcess, type CodexAppServerSpawn } from './codex-app-server-process-tree-kill' +import { + spawnCodexBackfillRecoveryProcess, + stopCodexBackfillRecoveryProcess +} from './codex-state-db-backfill-recovery-process' import { getOrcaUserDataPath } from './codex-home-paths' import { BACKFILL_PENDING_MIN_SESSION_FILES, @@ -43,7 +43,7 @@ type RecoveryDependencies = { countSessions: (sessionsRoot: string, limit: number) => number now: () => number sleep: (ms: number, signal: AbortSignal) => Promise - terminate: (child: ChildProcessHandle) => Promise + terminate: (child: ChildProcessHandle, supervised: boolean) => Promise } const defaultDependencies: RecoveryDependencies = { @@ -53,7 +53,7 @@ const defaultDependencies: RecoveryDependencies = { countSessions: countCodexSessionFilesUpTo, now: Date.now, sleep: async (ms, signal) => await delay(ms, undefined, { signal }), - terminate: async (child) => await terminateCodexProbeChild(child) + terminate: stopCodexBackfillRecoveryProcess } function finish( @@ -86,35 +86,6 @@ function initialRecoveryDecision( return null } -function spawnRecoveryProcess( - codexHomePath: string, - dependencies: RecoveryDependencies -): ChildProcessHandle { - const wslHome = process.platform === 'win32' ? parseWslUncPath(codexHomePath) : null - if (wslHome) { - return dependencies.spawnProcess( - 'wsl.exe', - buildWslCodexAppServerArgs( - wslHome.distro, - wslHome.linuxPath, - CODEX_READ_ONLY_APP_SERVER_ARGS - ), - { - stdio: ['pipe', 'ignore', 'ignore'], - windowsHide: true, - env: process.env - } - ) - } - const command = dependencies.resolveCommand() - return dependencies.spawnProcess(command, [...CODEX_READ_ONLY_APP_SERVER_ARGS], { - cwd: codexHomePath, - stdio: ['pipe', 'ignore', 'ignore'], - windowsHide: true, - env: withCliRuntimeOnPath(command, { ...process.env, CODEX_HOME: codexHomePath }) - }) -} - /** Keeps a sanctioned app-server claimant alive until Codex completes its own backfill. */ export async function runCodexStateDbBackfillRecovery( codexHomePath: string, @@ -130,7 +101,12 @@ export async function runCodexStateDbBackfillRecovery( const deadline = dependencies.now() + RECOVERY_MAX_TOTAL_MS let spawnCount = 0 while (!signal.aborted && dependencies.now() < deadline && spawnCount < RECOVERY_MAX_SPAWNS) { - const child = spawnRecoveryProcess(codexHomePath, dependencies) + const { child, supervised } = spawnCodexBackfillRecoveryProcess( + codexHomePath, + dependencies.spawnProcess, + dependencies.resolveCommand + ) + const terminate = (): Promise => dependencies.terminate(child, supervised) spawnCount += 1 let childDown = false child.once('error', () => { @@ -145,27 +121,27 @@ export async function runCodexStateDbBackfillRecovery( await dependencies.sleep(RECOVERY_POLL_INTERVAL_MS, signal) const status = dependencies.readStatus(codexHomePath) if (status.kind === 'complete') { - await dependencies.terminate(child) + await terminate() return finish('completed', spawnCount) } if (status.kind === 'unreadable') { - await dependencies.terminate(child) + await terminate() return finish('unreadable', spawnCount) } } } catch (error) { if (!signal.aborted) { - await dependencies.terminate(child) + await terminate() throw error } } if (signal.aborted) { - await dependencies.terminate(child) + await terminate() return finish('stopped', spawnCount) } if (!childDown) { - await dependencies.terminate(child) + await terminate() return finish('gave-up', spawnCount) } if (spawnCount >= RECOVERY_MAX_SPAWNS) { diff --git a/src/main/codex/supervised-probe-owner.test-fixture.ts b/src/main/codex/supervised-probe-owner.test-fixture.ts new file mode 100644 index 000000000000..e5d9b647e00f --- /dev/null +++ b/src/main/codex/supervised-probe-owner.test-fixture.ts @@ -0,0 +1,151 @@ +import { spawn, type ChildProcess } from 'node:child_process' +import { chmodSync, existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { build } from 'esbuild' + +// A stand-in agent CLI that, like `opencode serve` or a wedged Codex, ignores its stdin end. It +// leaves a SIGTERM-ignoring grandchild in its group, so only a group SIGKILL ends everything, and +// records its stdin end and a SIGTERM (exiting on the latter). `body` adds the caller's protocol. +function standInSource(body: string): string { + return String.raw` +const { spawn } = require('node:child_process') +const fs = require('node:fs') +const record = (event) => + fs.appendFileSync(process.env.ORCA_TEST_EVENTS_FILE, JSON.stringify({ event, at: Date.now() }) + '\n') +process.on('SIGTERM', () => { + record('SIGTERM') + fs.writeFileSync(process.env.ORCA_TEST_SIGNAL_FILE, 'SIGTERM') + process.exit(0) +}) +process.stdin.on('error', () => {}).on('end', () => record('stdin-end')).resume() +const grandchild = spawn( + process.execPath, + ['-e', "process.on('SIGTERM', () => {}); process.stdout.write('armed'); setInterval(() => {}, 60000)"], + { stdio: ['ignore', 'pipe', 'ignore'] } +) +grandchild.stdout.once('data', () => { + // Renamed into place, so a reader never sees a half-written file. + const partial = process.env.ORCA_TEST_PID_FILE + '.partial' + fs.writeFileSync(partial, JSON.stringify({ provider: process.pid, grandchild: grandchild.pid })) + fs.renameSync(partial, process.env.ORCA_TEST_PID_FILE) +}) +setInterval(() => {}, 60000) +${body} +` +} + +export type StandInEvent = { event: 'stdin-end' | 'SIGTERM'; at: number } + +export type SupervisedProbeRig = { + dir: string + pidFile: string + signalFile: string + /** When the stand-in saw its stdin end and its SIGTERM, by its own clock (this host's). */ + readEvents: () => Partial> + env: Record + /** An executable stand-in, run as ` ` like a real agent CLI. */ + writeStandIn: (name: string, body?: string) => string + /** Bundles a TypeScript owner entry, resolved from `resolveDir`, into one plain-Node file. */ + bundleOwner: (source: string, resolveDir: string) => Promise + launchOwner: (bundle: string, env?: Record) => ChildProcess + readPids: (timeoutMs?: number) => Promise<{ provider: number; grandchild: number }> + cleanup: () => void +} + +export function alive(pid: number): boolean { + try { + process.kill(pid, 0) + return true + } catch (error) { + return !(error instanceof Error && 'code' in error && error.code === 'ESRCH') + } +} + +export 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 +} + +export function createSupervisedProbeRig(): SupervisedProbeRig { + const dir = mkdtempSync(join(tmpdir(), 'orca-supervised-probe-')) + const pidFile = join(dir, 'pids.json') + const signalFile = join(dir, 'signal') + const eventsFile = join(dir, 'events.jsonl') + const recordedPids = new Set() + const env = { + ORCA_TEST_PID_FILE: pidFile, + ORCA_TEST_SIGNAL_FILE: signalFile, + ORCA_TEST_EVENTS_FILE: eventsFile + } + return { + dir, + pidFile, + signalFile, + readEvents: () => { + const events: Partial> = {} + const lines = existsSync(eventsFile) ? readFileSync(eventsFile, 'utf8').split('\n') : [] + for (const line of lines.filter(Boolean)) { + const { event, at }: StandInEvent = JSON.parse(line) + events[event] ??= at + } + return events + }, + env, + writeStandIn: (name, body = '') => { + const script = join(dir, `${name}.cjs`) + writeFileSync(script, standInSource(body)) + const launcher = join(dir, name) + writeFileSync(launcher, `#!/bin/sh\nexec "${process.execPath}" "${script}" "$@"\n`) + chmodSync(launcher, 0o755) + return launcher + }, + bundleOwner: async (source, resolveDir) => { + const outfile = join(dir, 'owner.cjs') + await build({ + stdin: { contents: source, resolveDir, loader: 'ts', sourcefile: 'owner.ts' }, + bundle: true, + platform: 'node', + format: 'cjs', + target: 'node22', + outfile, + logLevel: 'silent' + }) + return outfile + }, + launchOwner: (bundle, ownerEnv = {}) => { + const owner = spawn(process.execPath, [bundle], { + env: { ...process.env, ...env, ...ownerEnv }, + stdio: 'ignore' + }) + recordedPids.add(owner.pid!) + return owner + }, + readPids: async (timeoutMs = 20_000) => { + if (!(await waitFor(() => existsSync(pidFile), timeoutMs))) { + throw new Error('the stand-in never reported its pids') + } + const pids: { provider: number; grandchild: number } = JSON.parse( + readFileSync(pidFile, 'utf8') + ) + recordedPids.add(pids.provider) + recordedPids.add(pids.grandchild) + return pids + }, + cleanup: () => { + // Only pids this rig recorded, so a failed run never leaves a stand-in behind. + for (const pid of recordedPids) { + if (alive(pid)) { + process.kill(pid, 'SIGKILL') + } + } + rmSync(dir, { recursive: true, force: true }) + } + } +} diff --git a/src/main/opencode/opencode-launch-model-context-cleanup.test.ts b/src/main/opencode/opencode-launch-model-context-cleanup.test.ts index 4acf1ba72de4..cf5df3a7d7f1 100644 --- a/src/main/opencode/opencode-launch-model-context-cleanup.test.ts +++ b/src/main/opencode/opencode-launch-model-context-cleanup.test.ts @@ -9,12 +9,17 @@ import { } from '../../shared/child-process/process-tree-termination' import { probeOpenCodeLaunchModelContext } from './opencode-launch-model-context' import { readFetchResponseJsonWithinLimit } from '../../shared/fetch-response-body' +import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from '../provider-process/provider-process-supervisor' +import { terminateProviderProcessTree } from '../provider-process/provider-process-teardown' vi.mock('../../shared/child-process/run-process', () => ({ spawnProcess: vi.fn() })) vi.mock('../../shared/child-process/process-tree-termination', () => ({ signalProcessTree: vi.fn(), forceTerminateProcessTree: vi.fn() })) +vi.mock('../provider-process/provider-process-teardown', () => ({ + terminateProviderProcessTree: vi.fn() +})) vi.mock('../../shared/fetch-response-body', () => ({ readFetchResponseJsonWithinLimit: vi.fn() })) @@ -24,6 +29,9 @@ class ProbeChild extends ChildProcess { override stderr = new PassThrough() override pid = 123456 override exitCode: number | null = null + override signalCode: NodeJS.Signals | null = null + // A handle-less ChildProcess's own kill can signal this test's process group. + override kill = vi.fn((_signal?: NodeJS.Signals | number): boolean => true) override stdio: [PassThrough, PassThrough, PassThrough, undefined, undefined] = [ this.stdin, this.stdout, @@ -52,11 +60,33 @@ function close() { child.emit('exit', 0, null) child.emit('close', 0, null) } +/** How a supervisor ends: its stop signal re-raised once the group is gone, or an exit code. */ +function supervisorExits(code: number | null, signal: NodeJS.Signals | null) { + child.exitCode = code + child.signalCode = signal + child.emit('exit', code, signal) + child.emit('close', code, signal) +} const options = { executable: '/private/opencode', cwd: directory, env: {} } +function decodedSupervisorSpec(env: unknown): unknown { + const encoded = + typeof env === 'object' && env !== null && 'ORCA_PROVIDER_SUPERVISOR_SPEC' in env + ? String(env.ORCA_PROVIDER_SUPERVISOR_SPEC) + : '' + return JSON.parse(Buffer.from(encoded, 'base64').toString() || 'null') +} + +async function untilStopRequested(): Promise { + while (child.kill.mock.calls.length === 0) { + await new Promise((resolve) => setImmediate(resolve)) + } +} + describe('OpenCode model probe termination evidence', () => { beforeEach(() => { vi.resetAllMocks() + Object.defineProperty(process, 'platform', { value: 'darwin' }) vi.mocked(readFetchResponseJsonWithinLimit).mockImplementation((response) => response.json()) child = new ProbeChild() closeDuringFetch = false @@ -82,9 +112,16 @@ describe('OpenCode model probe termination evidence', () => { return true }) vi.mocked(forceTerminateProcessTree).mockResolvedValue(false) + child.kill.mockImplementation((signal) => { + if (signal === 'SIGTERM') { + supervisorExits(null, 'SIGTERM') + } + return true + }) }) afterEach(() => { + vi.useRealTimers() if (platform) { Object.defineProperty(process, 'platform', platform) } @@ -100,7 +137,7 @@ describe('OpenCode model probe termination evidence', () => { ) expect(await probeOpenCodeLaunchModelContext(options)).toBeNull() expect(cancel).toHaveBeenCalledTimes(4) - expect(signalProcessTree).toHaveBeenCalledOnce() + expect(child.kill).toHaveBeenCalledWith('SIGTERM') }) it('accepts an already-closed Windows probe without signaling its former pid', async () => { @@ -112,14 +149,103 @@ describe('OpenCode model probe termination evidence', () => { expect(forceTerminateProcessTree).not.toHaveBeenCalled() }) - it('does not force a POSIX group after the graceful close', async () => { - Object.defineProperty(process, 'platform', { value: 'darwin' }) + it('runs a POSIX probe under a one-shot supervisor and stops it through that supervisor', async () => { expect(await probeOpenCodeLaunchModelContext(options)).toMatchObject({ primaryAgent: 'build' }) - expect(signalProcessTree).toHaveBeenCalledOnce() + + const [spec] = vi.mocked(spawnProcess).mock.calls[0] + expect(spec).toMatchObject({ program: process.execPath, cwd: directory, detached: true }) + const args = spec.args ?? [] + expect(args.slice(args.indexOf('--') + 1)).toEqual([ + '/private/opencode', + 'serve', + '--hostname', + '127.0.0.1', + '--port', + '0' + ]) + // Its stdin ends at once, so a session lifetime would stop the server a second later. + expect(decodedSupervisorSpec(spec.env)).toMatchObject({ lifetime: 'one-shot', cwd: directory }) + expect(child.kill.mock.calls).toEqual([['SIGTERM']]) + // Signalling the supervisor's own group, or SIGKILLing it, would orphan the server's group. + expect(signalProcessTree).not.toHaveBeenCalled() expect(forceTerminateProcessTree).not.toHaveBeenCalled() + expect(terminateProviderProcessTree).not.toHaveBeenCalled() + }) + + it('forces a POSIX supervisor tree only after its full stop time, and trusts no forced stop', async () => { + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }) + child.kill.mockReturnValue(true) + // A teardown that found no descendants to check proves nothing about the server's group. + vi.mocked(terminateProviderProcessTree).mockImplementation(async () => { + supervisorExits(null, 'SIGKILL') + return null + }) + + const probe = probeOpenCodeLaunchModelContext(options) + await untilStopRequested() + await vi.advanceTimersByTimeAsync(PROVIDER_SUPERVISOR_MAX_STOP_MS - 1) + expect(terminateProviderProcessTree).not.toHaveBeenCalled() + await vi.advanceTimersByTimeAsync(1) + + expect(await probe).toBeNull() + expect(terminateProviderProcessTree).toHaveBeenCalledWith(child, { + site: 'opencode-launch-model-preflight' + }) + }) + + it('trusts no forced POSIX stop, even one whose teardown reports the tree gone', async () => { + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }) + child.kill.mockReturnValue(true) + // 'exited' covers the supervisor's tree only; the server leads its own detached group. + vi.mocked(terminateProviderProcessTree).mockImplementation(async () => { + supervisorExits(null, 'SIGKILL') + return 'exited' + }) + + const probe = probeOpenCodeLaunchModelContext(options) + await untilStopRequested() + await vi.advanceTimersByTimeAsync(PROVIDER_SUPERVISOR_MAX_STOP_MS) + + expect(await probe).toBeNull() }) - it('keeps a root exit with inherited pipes unverified and avoids its former pid', async () => { + it('trusts no POSIX stop whose supervisor exited 1, which a failed group reap also exits', async () => { + child.kill.mockImplementation(() => { + supervisorExits(1, null) + return true + }) + + expect(await probeOpenCodeLaunchModelContext(options)).toBeNull() + expect(terminateProviderProcessTree).not.toHaveBeenCalled() + }) + + it('accepts a POSIX supervisor that relayed the server exiting on its own', async () => { + child.kill.mockImplementation(() => { + supervisorExits(0, null) + return true + }) + + expect(await probeOpenCodeLaunchModelContext(options)).toMatchObject({ primaryAgent: 'build' }) + }) + + it('never forces a POSIX supervisor whose root exited without closing its pipes', async () => { + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }) + child.kill.mockImplementation(() => { + child.exitCode = 0 + child.emit('exit', 0, null) + return true + }) + + const probe = probeOpenCodeLaunchModelContext(options) + await untilStopRequested() + await vi.advanceTimersByTimeAsync(PROVIDER_SUPERVISOR_MAX_STOP_MS) + + expect(await probe).toBeNull() + expect(terminateProviderProcessTree).not.toHaveBeenCalled() + }) + + it('keeps a Windows root exit with inherited pipes unverified and avoids its former pid', async () => { + Object.defineProperty(process, 'platform', { value: 'win32' }) vi.mocked(signalProcessTree).mockImplementation(async () => { child.exitCode = 0 child.emit('exit', 0, null) @@ -129,7 +255,8 @@ describe('OpenCode model probe termination evidence', () => { expect(forceTerminateProcessTree).not.toHaveBeenCalled() }) - it('requires verified termination when a still-live probe does not close', async () => { + it('requires verified termination when a still-live Windows probe does not close', async () => { + Object.defineProperty(process, 'platform', { value: 'win32' }) vi.mocked(signalProcessTree).mockResolvedValue(true) expect(await probeOpenCodeLaunchModelContext(options)).toBeNull() expect(forceTerminateProcessTree).toHaveBeenCalledOnce() diff --git a/src/main/opencode/opencode-launch-model-context-supervision.test.ts b/src/main/opencode/opencode-launch-model-context-supervision.test.ts new file mode 100644 index 000000000000..2ffa6d7a8913 --- /dev/null +++ b/src/main/opencode/opencode-launch-model-context-supervision.test.ts @@ -0,0 +1,96 @@ +import { existsSync, readFileSync } from 'node:fs' +import { afterEach, describe, expect, it } from 'vitest' +import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from '../provider-process/provider-process-supervisor' +import { + alive, + createSupervisedProbeRig, + waitFor, + type SupervisedProbeRig +} from '../codex/supervised-probe-owner.test-fixture' +import { probeOpenCodeLaunchModelContext } from './opencode-launch-model-context' + +// Like `opencode serve`, it never exits on stdin end. It starts listening only after the session +// stdin-end grace, so a supervisor that treated the probe's stdin end as a stop would kill it first. +const SERVE = String.raw` +setTimeout(() => { + const server = require('node:http').createServer((request, response) => { + const url = new URL(request.url, 'http://127.0.0.1') + const location = { directory: url.searchParams.get('location[directory]') } + const model = { id: 'model', providerID: 'provider', enabled: true } + const bodies = { + '/api/model': { location, data: [model] }, + '/api/agent': { location, data: [{ id: 'build', mode: 'primary', hidden: false }] }, + '/api/model/default': { location, data: model }, + '/api/config': [{ type: 'document', info: { default_agent: 'build' } }] + } + response.setHeader('content-type', 'application/json') + response.end(JSON.stringify(bodies[url.pathname])) + }) + server.listen(0, '127.0.0.1', () => { + process.stdout.write('server listening on http://127.0.0.1:' + server.address().port + '\n') + }) +}, 1500) +` + +const OWNER = String.raw` +import { probeOpenCodeLaunchModelContext } from './opencode-launch-model-context' +void probeOpenCodeLaunchModelContext({ + executable: process.env.ORCA_TEST_STAND_IN!, + cwd: process.cwd(), + env: process.env +}).catch(() => {}) +setInterval(() => {}, 60_000) +` + +let rig: SupervisedProbeRig | null = null + +afterEach(() => { + rig?.cleanup() + rig = null +}) + +describe.runIf(process.platform !== 'win32')('supervised OpenCode launch-context probe', () => { + it('reads the context after its stdin end, then stops the whole server group', async () => { + rig = createSupervisedProbeRig() + const executable = rig.writeStandIn('opencode', SERVE) + + const context = await probeOpenCodeLaunchModelContext({ + executable, + cwd: rig.dir, + env: { ...process.env, ...rig.env } + }) + const stoppedAt = Date.now() + const pids = await rig.readPids() + + expect(context).toEqual({ + primaryAgent: 'build', + availableModels: ['provider/model'], + primaryModel: null + }) + expect(existsSync(rig.signalFile) && readFileSync(rig.signalFile, 'utf8')).toBe('SIGTERM') + expect(alive(pids.provider)).toBe(false) + // A descendant that ignores SIGTERM holds none of the probe's pipes, so a closed pipe never + // proved it gone; only the supervisor's group SIGKILL ends it. + expect(alive(pids.grandchild)).toBe(false) + // Once the server exits on its SIGTERM, the rest of its group is killed at once, not after the + // SIGTERM grace; every launch waits on this preflight. + expect(stoppedAt - (rig.readEvents().SIGTERM ?? 0)).toBeLessThan(1_000) + }) + + it('stops the server group when its owner is SIGKILLed mid-probe', async () => { + rig = createSupervisedProbeRig() + const executable = rig.writeStandIn('opencode') + const bundle = await rig.bundleOwner(OWNER, __dirname) + const owner = rig.launchOwner(bundle, { ORCA_TEST_STAND_IN: executable }) + const pids = await rig.readPids() + + owner.kill('SIGKILL') + + expect( + await waitFor( + () => !alive(pids.provider) && !alive(pids.grandchild), + PROVIDER_SUPERVISOR_MAX_STOP_MS + 1_000 + ) + ).toBe(true) + }) +}) diff --git a/src/main/opencode/opencode-launch-model-context.ts b/src/main/opencode/opencode-launch-model-context.ts index ce1098d8acfb..134b7b4ab92e 100644 --- a/src/main/opencode/opencode-launch-model-context.ts +++ b/src/main/opencode/opencode-launch-model-context.ts @@ -10,6 +10,13 @@ import { withTimeout } from '../../shared/promise-timeout-fallback' import { createOutputSink } from '../../shared/child-process/bounded-output-sink' import { readFetchResponseJsonWithinLimit } from '../../shared/fetch-response-body' import { cancelUnreadResponseBody } from '../lib/unread-response-body' +import type { ChildProcessHandle } from '../../shared/child-process/process-spec' +import { createProviderSpawnSpec } from '../provider-process/provider-process-supervisor' +import { stopSupervisedChildProcess } from '../provider-process/supervised-child-process-stop' + +const OPENCODE_PREFLIGHT_KILL_SITE = 'opencode-launch-model-preflight' +// A supervisor's pipes are its own, so they close as it exits. +const SUPERVISOR_PIPE_CLOSE_MS = 1_000 const model = z.object({ id: z.string(), providerID: z.string() }) const availableModel = model.extend({ enabled: z.boolean() }) @@ -94,15 +101,22 @@ export async function probeOpenCodeLaunchModelContext(options: { expectedPrimaryModel?: string }): Promise { const password = randomBytes(32).toString('base64url') + const spawnSpec = createProviderSpawnSpec( + { + command: options.executable, + args: ['serve', '--hostname', '127.0.0.1', '--port', '0'], + cwd: options.cwd + }, + { ...options.env, OPENCODE_SERVER_USERNAME: 'opencode', OPENCODE_SERVER_PASSWORD: password }, + process.platform, + // `opencode serve` never exits on stdin end, so on POSIX only its supervisor stops it with Orca. + { lifetime: 'one-shot' } + ) const child = spawnProcess({ - program: options.executable, - args: ['serve', '--hostname', '127.0.0.1', '--port', '0'], + program: spawnSpec.program, + args: spawnSpec.args, cwd: options.cwd, - env: { - ...options.env, - OPENCODE_SERVER_USERNAME: 'opencode', - OPENCODE_SERVER_PASSWORD: password - }, + env: spawnSpec.env, detached: true }) const output = createOutputSink(4096) @@ -193,14 +207,42 @@ export async function probeOpenCodeLaunchModelContext(options: { } catch { context = null } finally { - if (!childClosed && !rootExited) { - await signalProcessTree(child, 'SIGTERM') - } - stopped = childClosed || (await withTimeout(closed, 1_500, false)) - // A reaped root's PID can be reused while descendants still hold its pipes. - if (!stopped && !rootExited) { - stopped = await forceTerminateProcessTree(child) + if (spawnSpec.supervised) { + stopped = await stopSupervisedPreflight(child, closed) + } else { + if (!childClosed && !rootExited) { + await signalProcessTree(child, 'SIGTERM') + } + stopped = childClosed || (await withTimeout(closed, 1_500, false)) + // A reaped root's PID can be reused while descendants still hold its pipes. + if (!stopped && !rootExited) { + stopped = await forceTerminateProcessTree(child) + } } } return stopped ? context : null } + +/** + * True only when the server's group is proven gone: the supervisor exited on its own stop or + * relayed the server's exit. A forced stop proves nothing: its teardown reaches the supervisor's + * group, not the server's own detached group. Exit 1 can be a group reap that failed. + */ +async function stopSupervisedPreflight( + child: ChildProcessHandle, + closed: Promise +): Promise { + if (!child.pid) { + // Never started, so there is nothing to stop. + return true + } + const { root, tree } = await stopSupervisedChildProcess(child, { + site: OPENCODE_PREFLIGHT_KILL_SITE + }) + const forced = tree !== null || child.signalCode === 'SIGKILL' + const failedReap = child.signalCode === null && child.exitCode === 1 + if (root !== 'exited' || forced || failedReap) { + return false + } + return withTimeout(closed, SUPERVISOR_PIPE_CLOSE_MS, false) +} diff --git a/src/main/rate-limits/codex-fetcher-auth-errors.test.ts b/src/main/rate-limits/codex-fetcher-auth-errors.test.ts index 5ce9df8675eb..20405233c9df 100644 --- a/src/main/rate-limits/codex-fetcher-auth-errors.test.ts +++ b/src/main/rate-limits/codex-fetcher-auth-errors.test.ts @@ -11,6 +11,11 @@ vi.mock('node:child_process', () => ({ spawn: childSpawnMock })) +// The recovery's supervised stop imports the process-table reader, which needs the real execFile. +vi.mock('../codex/codex-state-db-backfill-recovery', () => ({ + startCodexStateDbBackfillRecoveryInBackground: vi.fn() +})) + vi.mock('../codex-cli/command', () => ({ resolveCodexCommand: resolveCodexCommandMock })) diff --git a/src/main/rate-limits/codex-fetcher-backend.test.ts b/src/main/rate-limits/codex-fetcher-backend.test.ts index 3fefe5e3a2fd..b1c01a2a6e8c 100644 --- a/src/main/rate-limits/codex-fetcher-backend.test.ts +++ b/src/main/rate-limits/codex-fetcher-backend.test.ts @@ -15,6 +15,11 @@ vi.mock('./codex-auth-presence', () => ({ probeCodexAuthPresence: vi.fn(async () => 'present') })) +// The recovery's supervised stop imports the process-table reader, which needs the real execFile. +vi.mock('../codex/codex-state-db-backfill-recovery', () => ({ + startCodexStateDbBackfillRecoveryInBackground: vi.fn() +})) + import { consumeCodexRateLimitResetCredit, fetchCodexRateLimits } from './codex-fetcher' describe('Codex backend rate-limit requests', () => { diff --git a/src/main/rate-limits/codex-fetcher-credit-balance.test.ts b/src/main/rate-limits/codex-fetcher-credit-balance.test.ts index 780687c9a09d..016357735e94 100644 --- a/src/main/rate-limits/codex-fetcher-credit-balance.test.ts +++ b/src/main/rate-limits/codex-fetcher-credit-balance.test.ts @@ -14,6 +14,11 @@ vi.mock('./codex-auth-presence', () => ({ probeCodexAuthPresence: vi.fn(async () => 'present') })) +// The recovery's supervised stop imports the process-table reader, which needs the real execFile. +vi.mock('../codex/codex-state-db-backfill-recovery', () => ({ + startCodexStateDbBackfillRecoveryInBackground: vi.fn() +})) + import { fetchCodexRateLimits } from './codex-fetcher' function parseRpcRequest(line: string): { id?: number; method?: string } { diff --git a/src/main/rate-limits/codex-fetcher-probe-shutdown.test.ts b/src/main/rate-limits/codex-fetcher-probe-shutdown.test.ts index d5b2d56b6817..791bcf4c7bbf 100644 --- a/src/main/rate-limits/codex-fetcher-probe-shutdown.test.ts +++ b/src/main/rate-limits/codex-fetcher-probe-shutdown.test.ts @@ -12,6 +12,11 @@ vi.mock('node:child_process', () => ({ spawn: childSpawnMock })) +// The recovery's supervised stop imports the process-table reader, which needs the real execFile. +vi.mock('../codex/codex-state-db-backfill-recovery', () => ({ + startCodexStateDbBackfillRecoveryInBackground: vi.fn() +})) + vi.mock('node:fs/promises', () => ({ readFile: readFileMock })) diff --git a/src/main/rate-limits/codex-fetcher-session-supplement.test.ts b/src/main/rate-limits/codex-fetcher-session-supplement.test.ts index 232bfc9a83dc..44c086137215 100644 --- a/src/main/rate-limits/codex-fetcher-session-supplement.test.ts +++ b/src/main/rate-limits/codex-fetcher-session-supplement.test.ts @@ -14,6 +14,11 @@ vi.mock('./codex-auth-presence', () => ({ probeCodexAuthPresence: vi.fn(async () => 'present') })) +// The recovery's supervised stop imports the process-table reader, which needs the real execFile. +vi.mock('../codex/codex-state-db-backfill-recovery', () => ({ + startCodexStateDbBackfillRecoveryInBackground: vi.fn() +})) + import { fetchCodexRateLimits } from './codex-fetcher' function makeRpcChild(rateLimitResetCredits?: unknown) {