Files
orca/src/main/codex/codex-app-server-process-teardown.test.ts
T
Brennan Benson c5330d0d52 fix(native-chat): stop killing processes that only inherited a chat's spawn tag (#23460)
* fix(native-chat): stop signalling processes that only inherited a spawn token

A spawn token is an environment variable, so every descendant of a provider child
carries it. The Linux-only startup scan treated any carrier no lease claimed as a lost
provider child and sent it SIGTERM, which also hit editors, tmux servers and nested
Orca processes the agent had started. Remove that scan's killing consumer; the token
scan stays for the reservation probe, and recorded owners are still stopped by
identity during recovery.

* fix(codex): remove the token-scan kill path from app-server teardown

Every descendant inherits the spawn token, so killing each pid that carries it can
reach processes the agent started that are not the provider. Production never
injected this path; teardown always uses the process-group and descendant-snapshot
proof. Drop it, its deps, and the now-unused spawn-token argument.
2026-09-28 15:25:24 -07:00

169 lines
5.6 KiB
TypeScript

import type { ChildProcess } from 'node:child_process'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import {
findSelfInitiatedTreeKills,
resetSelfInitiatedTreeKillLogForTest
} from '../crash-reporting/self-initiated-tree-kill-log'
import { terminateCodexAppServerProcessTree } from './codex-app-server-process-teardown'
/** Above pid_max on every supported POSIX host, so the group signal is a real ESRCH. */
const UNREACHABLE_PGID = 2_147_483_647
function child() {
return {
pid: 1234,
kill: vi.fn(() => true) as ChildProcess['kill']
}
}
describe('terminateCodexAppServerProcessTree', () => {
beforeEach(() => {
resetSelfInitiatedTreeKillLogForTest()
})
it('waits for the Windows tree kill before releasing the wrapper', async () => {
const target = child()
const release = Promise.withResolvers<void>()
const terminateWindowsTree = vi.fn(() => release.promise)
const teardown = terminateCodexAppServerProcessTree(target, {
platform: 'win32',
terminateWindowsTree
})
expect(target.kill).not.toHaveBeenCalled()
release.resolve()
await teardown
expect(terminateWindowsTree).toHaveBeenCalledWith(1234, { site: 'codex-app-server-teardown' })
expect(target.kill).toHaveBeenCalledWith('SIGKILL')
})
it('waits for an owned POSIX snapshot before killing the wrapper', async () => {
const target = child()
const snapshot = { rootPgid: 1234, descendants: [], capturedAtMs: 1 }
const release = Promise.withResolvers<boolean>()
const teardown = terminateCodexAppServerProcessTree(target, {
platform: 'darwin',
captureDescendants: async () => snapshot,
terminateDescendants: () => release.promise
})
await vi.waitFor(() => expect(target.kill).toHaveBeenCalledWith('SIGSTOP'))
expect(target.kill).not.toHaveBeenCalledWith('SIGKILL')
release.resolve(true)
await teardown
expect(target.kill).toHaveBeenLastCalledWith('SIGKILL')
})
it('signals a proven dedicated POSIX process group without scanning descendants', async () => {
const target = child()
const captureDescendants = vi.fn()
const signalProcessGroup = vi.fn()
await expect(
terminateCodexAppServerProcessTree(target, {
platform: 'darwin',
dedicatedProcessGroup: true,
captureDescendants,
signalProcessGroup
})
).resolves.toBe(true)
expect(signalProcessGroup).toHaveBeenCalledWith(1234, 'SIGKILL')
expect(captureDescendants).not.toHaveBeenCalled()
expect(target.kill).not.toHaveBeenCalled()
})
it('keeps the dedicated-group wrapper reachable when signalling is unproven', async () => {
const target = child()
await expect(
terminateCodexAppServerProcessTree(target, {
platform: 'linux',
dedicatedProcessGroup: true,
signalProcessGroup: () => {
throw Object.assign(new Error('denied'), { code: 'EPERM' })
}
})
).resolves.toBe(false)
expect(target.kill).not.toHaveBeenCalled()
})
/**
* `selfInitiatedTreeKillCount` decides whether a `render-process-gone` was
* ours. A group that had already exited was killed by nobody, so crediting it
* puts a suspect in the five-second window that Orca never issued. Exercised
* through the real `process.kill(-pgid)` because the swallow being tested
* lives in the production default, not in an injectable seam.
*/
it('does not claim a snapshot group that was already gone', async () => {
const target = { pid: UNREACHABLE_PGID, kill: vi.fn(() => true) as ChildProcess['kill'] }
await expect(
terminateCodexAppServerProcessTree(target, {
platform: 'darwin',
captureDescendants: async () => ({
rootPgid: UNREACHABLE_PGID,
descendants: [],
capturedAtMs: 1
}),
terminateDescendants: async () => true
})
).resolves.toBe(true)
expect(target.kill).toHaveBeenLastCalledWith('SIGKILL')
expect(findSelfInitiatedTreeKills(Date.now())).toEqual([])
})
it('claims a snapshot group the signal actually reached', async () => {
const target = child()
const signalProcessGroup = vi.fn()
await expect(
terminateCodexAppServerProcessTree(target, {
platform: 'darwin',
captureDescendants: async () => ({ rootPgid: 1234, descendants: [], capturedAtMs: 1 }),
terminateDescendants: async () => true,
signalProcessGroup
})
).resolves.toBe(true)
expect(signalProcessGroup).toHaveBeenCalledWith(1234, 'SIGKILL')
expect(findSelfInitiatedTreeKills(Date.now())).toEqual([
expect.objectContaining({
pid: 1234,
site: 'codex-app-server-teardown',
scope: 'posix-process-group'
})
])
})
it('tears down 40 dedicated groups without process-table scans or cross-group fanout', async () => {
const killMocks = Array.from({ length: 40 }, () => vi.fn(() => true))
const targets = killMocks.map((kill, index) => ({
pid: 10_000 + index,
kill: kill as ChildProcess['kill']
}))
const captureDescendants = vi.fn()
const signalProcessGroup = vi.fn()
const results = await Promise.all(
targets.map((target) =>
terminateCodexAppServerProcessTree(target, {
platform: 'linux',
dedicatedProcessGroup: true,
captureDescendants,
signalProcessGroup
})
)
)
expect(results).toEqual(Array.from({ length: targets.length }, () => true))
expect(signalProcessGroup.mock.calls).toEqual(targets.map((target) => [target.pid, 'SIGKILL']))
expect(captureDescendants).not.toHaveBeenCalled()
expect(killMocks.every((kill) => kill.mock.calls.length === 0)).toBe(true)
})
})