fix: stop Codex and OpenCode helper servers when Orca quits or crashes (STA-9254, 2 of 2)

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",

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
     }))

src/main/codex/codex-app-server-client.test.ts

@@ -12,13 +12,22 @@ 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 →
 // initialized → hooks/list → config/batchWrite → hooks/list. Scenario-driven
 // 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<never>(() => {}))
     ).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()
   })

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<typeof ProcessTreeKill>()),
+  killCodexAppServerProcessTree: vi.fn()
+}))
+
+type FakeChild = EventEmitter & {
+  pid: number
+  stdin: PassThrough
+  stdout: PassThrough
+  stderr: PassThrough
+  kill: ReturnType<typeof vi.fn<(signal?: NodeJS.Signals) => 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<unknown> {
+  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<never>(() => {}),
+    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)
+  })
+})