Files
orca/src/main/codex/codex-app-server-process-teardown.test.ts
T
Neil 9acfba401a fix(crash-reporting): stop claiming kills that never landed, and leave proof when the own-Chromium pid set is unreadable (#18578)
* fix(crash-reporting): stop the codex POSIX teardown claiming a group that was already gone

terminatePosixTree's default group signal swallowed every process.kill error and
then recorded a self_tree_kill unconditionally, so an ESRCH — proof the group was
already gone and this teardown killed nothing — still put a suspect in the
five-second render-process-gone attribution window.

Every sibling group-kill in the tree already records only on a proven signal:
terminateDedicatedPosixGroup in this same file, forceKillPosixPtyProcessGroups,
and the claude account-login teardown. This makes the outlier match them.

* fix(crash-reporting): leave proof when the own-Chromium pid set cannot be read

`readOrcaChromiumProcessPids` returns an empty set when `getAppMetrics()`
throws, which is the right decision — refusing every kill would orphan every
PTY, git, codex and notebook tree main tears down, and on main a refusal from
`killSourceControlAgentProcess` releases the managed-home lock with the agent
still alive. But the empty set was byte-identical to "no Chromium on this
host", so the fail-open was invisible in a field bundle.

Keeps the decision, adds a coalesced durable `own_chromium_pids_unreadable`
crumb so the two cases are distinguishable. Coalesced because the gate reads
this set on every tree kill.

* style(crash-reporting): tighten the group-signal comments to the WHY
2026-09-04 00:53:31 -07:00

209 lines
6.9 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, undefined, {
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('kills exact Linux spawn-token PIDs before the recorded wrapper', async () => {
const target = child()
const findSpawnTokenProcesses = vi
.fn<() => Promise<number[] | null>>()
.mockResolvedValueOnce([1234, 2345, 3456])
.mockResolvedValueOnce([1234])
.mockResolvedValueOnce([1234])
const signalPid = vi.fn()
await expect(
terminateCodexAppServerProcessTree(target, 'spawn-1', {
platform: 'linux',
findSpawnTokenProcesses,
signalPid,
isPidPresent: () => false,
wait: async () => undefined
})
).resolves.toBe(true)
expect(signalPid.mock.calls).toEqual([
[2345, 'SIGKILL'],
[3456, 'SIGKILL']
])
expect(target.kill).toHaveBeenCalledTimes(1)
expect(target.kill).toHaveBeenCalledWith('SIGKILL')
})
it('keeps the wrapper reachable when Linux cannot prove descendant exit', async () => {
const target = child()
await expect(
terminateCodexAppServerProcessTree(target, 'spawn-1', {
platform: 'linux',
findSpawnTokenProcesses: async () => null
})
).resolves.toBe(false)
expect(target.kill).not.toHaveBeenCalled()
})
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, undefined, {
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, undefined, {
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, undefined, {
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, undefined, {
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, undefined, {
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, undefined, {
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)
})
})