mirror of
https://github.com/stablyai/orca.git
synced 2026-10-18 00:03:20 +00:00
fix(terminal): kill agent descendant processes on session teardown (STA-1800)
Agent CLIs spawn tool children in detached process groups that PTY SIGHUP can never reach. Killing an agent session (tab close, retire, sleep) left those children running as orphans — eight orphaned git processes burned ~8 cores for up to 11.5h under the agents-running keep-awake and drained a battery to 8%. New pty-descendant-termination module: snapshot the ppid tree BEFORE signalling (a dead root's descendants reparent to pid 1 and become unfindable), SIGTERM the root group and every descendant, then after a 2s grace SIGKILL survivors gated on a pid+start-time identity re-check so a recycled pid is never signalled. Snapshot is bounded and never rejects; failures degrade to today's shell-only kill. Wired for agent sessions only (plain terminals keep nohup semantics) at all three POSIX kill sites: local provider shutdown, daemon TerminalHost immediate kill (the pty:kill path — force-kill bypassed Session.kill entirely), and daemon Session graceful kill. Verified live in the built app: an agent pane with a detached-pgid child; the child survived on the unwired build (three control runs) and dies within ~5s with the fix. Windows ConPTY and SSH-hosted PTYs keep the previous foreground-tree contract (documented follow-ups).
This commit is contained in:
@@ -74,8 +74,13 @@ could invalidate intentional long-lived `worktree-sleep` checkpoints.
|
||||
|
||||
- Changing terminal output, snapshot, replay, query-response, or hidden-delivery
|
||||
behavior.
|
||||
- Killing processes that deliberately daemonize away from the terminal process
|
||||
group. This contract covers the PTY and its attached foreground process tree.
|
||||
- Killing processes that deliberately daemonize away from a plain user
|
||||
terminal's process group (nohup-style survivors remain user intent there).
|
||||
For **agent sessions**, close/kill additionally terminates the snapshotted
|
||||
descendant tree — including detached-pgid children the PTY's SIGHUP cannot
|
||||
reach — via `pty-descendant-termination.ts` (SIGTERM, grace window, then
|
||||
identity-checked SIGKILL). Windows and SSH-hosted PTYs keep the previous
|
||||
foreground-tree contract for now.
|
||||
- Changing agent-provider resume commands or permission flags.
|
||||
- Making a UI close wait for a remote process to exit before the tab disappears.
|
||||
- Replacing worktree sleep with tab close. Sleep remains resumable by design.
|
||||
|
||||
@@ -1,6 +1,12 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { PRODUCER_PAUSE_FAILSAFE_MS, Session } from './session'
|
||||
import type { SessionState, ShellReadyState } from './types'
|
||||
import type { TuiAgent } from '../../shared/types'
|
||||
|
||||
const killWithDescendantSweepMock = vi.hoisted(() => vi.fn())
|
||||
vi.mock('../pty-descendant-termination', () => ({
|
||||
killWithDescendantSweep: killWithDescendantSweepMock
|
||||
}))
|
||||
|
||||
// Stub the subprocess — Session talks to it via an interface, not child_process directly.
|
||||
function createMockSubprocess() {
|
||||
@@ -86,6 +92,7 @@ describe('Session', () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers()
|
||||
subprocess = createMockSubprocess()
|
||||
killWithDescendantSweepMock.mockReset()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
@@ -98,11 +105,13 @@ describe('Session', () => {
|
||||
shellReadyTimeoutMs?: number
|
||||
cols?: number
|
||||
rows?: number
|
||||
launchAgent?: TuiAgent
|
||||
}): Session {
|
||||
session = new Session({
|
||||
sessionId: 'test-session',
|
||||
cols: opts?.cols ?? 80,
|
||||
rows: opts?.rows ?? 24,
|
||||
...(opts?.launchAgent ? { launchAgent: opts.launchAgent } : {}),
|
||||
subprocess,
|
||||
shellReadySupported: opts?.shellReadySupported ?? false,
|
||||
...(opts?.shellReadyTimeoutMs !== undefined
|
||||
@@ -406,6 +415,33 @@ describe('Session', () => {
|
||||
expect(session.isTerminating).toBe(true)
|
||||
})
|
||||
|
||||
it('non-agent kill stays synchronous and never routes through the descendant sweep', () => {
|
||||
createSession()
|
||||
session.kill()
|
||||
expect(subprocess.killed).toBe(true)
|
||||
expect(killWithDescendantSweepMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('agent kill routes through the descendant sweep with the subprocess as root', () => {
|
||||
createSession({ launchAgent: 'claude' })
|
||||
session.kill()
|
||||
expect(killWithDescendantSweepMock).toHaveBeenCalledWith(subprocess.pid, expect.any(Function))
|
||||
// The root kill is deferred to the sweep's snapshot-first sequencing.
|
||||
expect(subprocess.killed).toBe(false)
|
||||
const killRoot = killWithDescendantSweepMock.mock.calls[0][1] as () => void
|
||||
killRoot()
|
||||
expect(subprocess.killed).toBe(true)
|
||||
})
|
||||
|
||||
it('agent kill root callback is a no-op after the session already exited', () => {
|
||||
createSession({ launchAgent: 'claude' })
|
||||
session.kill()
|
||||
const killRoot = killWithDescendantSweepMock.mock.calls[0][1] as () => void
|
||||
subprocess.simulateExit(0)
|
||||
killRoot()
|
||||
expect(subprocess.killed).toBe(false)
|
||||
})
|
||||
|
||||
it('notifies attached clients on exit after kill', async () => {
|
||||
vi.useRealTimers()
|
||||
createSession()
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
type ShellReadyScanState
|
||||
} from '../shell-ready-marker-scanner'
|
||||
import { isPowerShellProcess } from '../../shared/shell-process-detection'
|
||||
import { killWithDescendantSweep } from '../pty-descendant-termination'
|
||||
import type { TuiAgent } from '../../shared/types'
|
||||
import type {
|
||||
PendingOutputRecord,
|
||||
@@ -267,7 +268,19 @@ export class Session {
|
||||
// Why: a paused child can be blocked inside write(); resume before
|
||||
// signalling so it can run signal handlers and actually exit.
|
||||
this.releaseProducerPause({ resume: true })
|
||||
this.subprocess.kill()
|
||||
if (!this.launchAgent) {
|
||||
this.subprocess.kill()
|
||||
} else {
|
||||
// Why: agent tool children live in detached process groups a dying
|
||||
// shell's SIGHUP never reaches. The bounded snapshot briefly defers the
|
||||
// signal; the kill timer below starts now, so force-dispose timing is
|
||||
// unaffected.
|
||||
killWithDescendantSweep(this.subprocess.pid, () => {
|
||||
if (this._state !== 'exited') {
|
||||
this.subprocess.kill()
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
this.killTimer = setTimeout(() => {
|
||||
if (this._state !== 'exited') {
|
||||
|
||||
@@ -2,6 +2,11 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { Session, type SubprocessHandle } from './session'
|
||||
import { TerminalHost } from './terminal-host'
|
||||
|
||||
const killWithDescendantSweepMock = vi.hoisted(() => vi.fn())
|
||||
vi.mock('../pty-descendant-termination', () => ({
|
||||
killWithDescendantSweep: killWithDescendantSweepMock
|
||||
}))
|
||||
|
||||
function createMockSubprocess(
|
||||
options: { startupCommandDeliveredInShellArgs?: boolean; shellPath?: string } = {}
|
||||
): SubprocessHandle {
|
||||
@@ -56,6 +61,7 @@ describe('TerminalHost', () => {
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
killWithDescendantSweepMock.mockReset()
|
||||
spawnFn = vi.fn(() => {
|
||||
const sub = createMockSubprocess() as ReturnType<typeof createMockSubprocess> & {
|
||||
_onDataCb: ((data: string) => void) | null
|
||||
@@ -393,6 +399,43 @@ describe('TerminalHost', () => {
|
||||
it('throws for non-existent session', () => {
|
||||
expect(() => host.kill('missing')).toThrow('Session not found')
|
||||
})
|
||||
|
||||
it('non-agent immediate kill stays synchronous and never routes through the descendant sweep', async () => {
|
||||
await host.createOrAttach({
|
||||
sessionId: 'plain-1',
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
streamClient: { onData: vi.fn(), onExit: vi.fn() }
|
||||
})
|
||||
|
||||
host.kill('plain-1', { immediate: true })
|
||||
|
||||
expect(lastSubprocess.forceKill).toHaveBeenCalled()
|
||||
expect(killWithDescendantSweepMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('agent immediate kill routes through the descendant sweep and defers the force-kill to it', async () => {
|
||||
await host.createOrAttach({
|
||||
sessionId: 'agent-1',
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
launchAgent: 'claude',
|
||||
streamClient: { onData: vi.fn(), onExit: vi.fn() }
|
||||
})
|
||||
|
||||
host.kill('agent-1', { immediate: true })
|
||||
|
||||
// Why order matters: force-killing first would let orphans reparent to
|
||||
// pid 1 and escape the sweep's ppid walk entirely.
|
||||
expect(killWithDescendantSweepMock).toHaveBeenCalledWith(99999, expect.any(Function))
|
||||
expect(lastSubprocess.forceKill).not.toHaveBeenCalled()
|
||||
expect(host.isKilled('agent-1')).toBe(true)
|
||||
|
||||
const finish = killWithDescendantSweepMock.mock.calls[0][1] as () => void
|
||||
finish()
|
||||
expect(lastSubprocess.forceKill).toHaveBeenCalled()
|
||||
expect(lastSubprocess.dispose).toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe('signal', () => {
|
||||
|
||||
@@ -6,6 +6,7 @@ import type { StartupCommandDelivery } from '../../shared/codex-startup-delivery
|
||||
import { buildStartupCommandSubmission } from '../../shared/startup-command-submission'
|
||||
import type { SessionInfo, TakePendingOutputResult, TerminalSnapshot } from './types'
|
||||
import { SessionNotFoundError } from './types'
|
||||
import { killWithDescendantSweep } from '../pty-descendant-termination'
|
||||
import type { CreateOrAttachOptions, CreateOrAttachResult } from './terminal-host-create-contract'
|
||||
|
||||
export type { CreateOrAttachOptions, CreateOrAttachResult } from './terminal-host-create-contract'
|
||||
@@ -197,11 +198,20 @@ export class TerminalHost {
|
||||
const session = this.getAliveSession(sessionId)
|
||||
this.recordTombstone(sessionId)
|
||||
if (opts.immediate) {
|
||||
session.forceKillAndDisposeSubprocess()
|
||||
// Why: the immediate path tears down synchronously without firing the
|
||||
// session's onExit hook, so reap it here. The graceful path below funnels
|
||||
// through Session.handleSubprocessExit -> onExit -> reapSession.
|
||||
this.reapSession(sessionId)
|
||||
const finish = (): void => {
|
||||
session.forceKillAndDisposeSubprocess()
|
||||
// Why: the immediate path tears down synchronously without firing the
|
||||
// session's onExit hook, so reap it here. The graceful path below
|
||||
// funnels through Session.handleSubprocessExit -> onExit -> reapSession.
|
||||
this.reapSession(sessionId)
|
||||
}
|
||||
if (!session.launchAgent) {
|
||||
finish()
|
||||
return
|
||||
}
|
||||
// Why: agent tool children live in detached process groups the shell's
|
||||
// death never signals; the bounded snapshot defers teardown at most ~1s.
|
||||
killWithDescendantSweep(session.pid, finish)
|
||||
return
|
||||
}
|
||||
session.kill()
|
||||
|
||||
@@ -54,6 +54,10 @@ import { WINDOWS_GIT_BASH_SHELL } from '../../shared/windows-terminal-shell'
|
||||
import { resolveAgentForegroundProcessWithAvailability } from './agent-foreground-process'
|
||||
import { getAgentForegroundContextPaths } from './agent-foreground-context-paths'
|
||||
import { recognizeAgentProcessFromCommandLine } from '../../shared/agent-process-recognition'
|
||||
import {
|
||||
captureDescendantSnapshot,
|
||||
terminateDescendantSnapshot
|
||||
} from '../pty-descendant-termination'
|
||||
import { readWindowsConptyProcessIds } from './windows-conpty-process-membership'
|
||||
import { shouldUseShellReadyStartupDelivery } from '../../shared/codex-startup-delivery'
|
||||
import { assertSafeAgentStartupCwd, resolveSafePtyDefaultCwd } from './pty-default-cwd'
|
||||
@@ -68,6 +72,11 @@ const PANE_IDENTITY_ENV_KEYS = [
|
||||
|
||||
let ptyCounter = 0
|
||||
const ptyProcesses = new Map<string, pty.IPty>()
|
||||
// Why: only agent sessions get descendant tree-kill on shutdown. Agent CLIs
|
||||
// spawn tool children in detached process groups the PTY's SIGHUP can never
|
||||
// reach; plain user terminals keep classic semantics where deliberately
|
||||
// detached (nohup-style) children survive the pane.
|
||||
const ptyAgentSessionIds = new Set<string>()
|
||||
const ptyShellName = new Map<string, string>()
|
||||
const ptyAgentForegroundContextPaths = new Map<string, string[]>()
|
||||
const ptyTerminalHandle = new Map<string, string>()
|
||||
@@ -180,6 +189,7 @@ function clearPtyState(id: string): void {
|
||||
runPtyCleanup(id)
|
||||
disposePtyListeners(id)
|
||||
ptyProcesses.delete(id)
|
||||
ptyAgentSessionIds.delete(id)
|
||||
ptyShellName.delete(id)
|
||||
ptyAgentForegroundContextPaths.delete(id)
|
||||
ptyTerminalHandle.delete(id)
|
||||
@@ -689,6 +699,12 @@ export class LocalPtyProvider implements IPtyProvider {
|
||||
|
||||
const proc = spawnResult.process
|
||||
ptyProcesses.set(id, proc)
|
||||
// Why both signals: launchAgent is the caller's explicit intent and
|
||||
// survives command rewriting (e.g. auth env prefixes); recognition covers
|
||||
// callers that pass a bare agent command line without the flag.
|
||||
if (args.launchAgent || startupAgentRecognition) {
|
||||
ptyAgentSessionIds.add(id)
|
||||
}
|
||||
ptyShellName.set(id, getSpawnedShellName(shellPath))
|
||||
if (finalEnv.ORCA_TERMINAL_HANDLE) {
|
||||
ptyTerminalHandle.set(id, finalEnv.ORCA_TERMINAL_HANDLE)
|
||||
@@ -891,23 +907,36 @@ export class LocalPtyProvider implements IPtyProvider {
|
||||
if (!proc) {
|
||||
return
|
||||
}
|
||||
// Why: disposePtyListeners removes the onExit callback, so the natural
|
||||
// exit cleanup path from node-pty won't fire. Cleanup and notification
|
||||
// must happen unconditionally after the try/catch.
|
||||
// Timer/writer cleanup must happen here too: disposing listeners prevents
|
||||
// the natural onExit callback from running the usual clearPtyState path.
|
||||
runPtyCleanup(id)
|
||||
disposePtyListeners(id)
|
||||
try {
|
||||
proc.kill()
|
||||
} catch {
|
||||
/* Process may already be dead */
|
||||
// Why: the snapshot must precede any signal/destroy — once the shell dies,
|
||||
// surviving descendants reparent to pid 1 and a ppid walk can't find them.
|
||||
const descendants = ptyAgentSessionIds.has(id)
|
||||
? await captureDescendantSnapshot(proc.pid)
|
||||
: null
|
||||
// Why the handle re-check: the snapshot is this method's only await, and a
|
||||
// natural exit (or same-id respawn) may have raced it. That path already
|
||||
// ran clearPtyState and notified exit — only the descendant sweep remains.
|
||||
if (ptyProcesses.get(id) === proc) {
|
||||
// Why: disposePtyListeners removes the onExit callback, so the natural
|
||||
// exit cleanup path from node-pty won't fire. Cleanup and notification
|
||||
// must happen unconditionally after the try/catch.
|
||||
// Timer/writer cleanup must happen here too: disposing listeners prevents
|
||||
// the natural onExit callback from running the usual clearPtyState path.
|
||||
runPtyCleanup(id)
|
||||
disposePtyListeners(id)
|
||||
try {
|
||||
proc.kill()
|
||||
} catch {
|
||||
/* Process may already be dead */
|
||||
}
|
||||
destroyPtyProcess(proc, { alreadyKilled: true })
|
||||
clearPtyState(id)
|
||||
this.opts.onExit?.(id, -1)
|
||||
for (const cb of exitListeners) {
|
||||
cb({ id, code: -1 })
|
||||
}
|
||||
}
|
||||
destroyPtyProcess(proc, { alreadyKilled: true })
|
||||
clearPtyState(id)
|
||||
this.opts.onExit?.(id, -1)
|
||||
for (const cb of exitListeners) {
|
||||
cb({ id, code: -1 })
|
||||
if (descendants) {
|
||||
terminateDescendantSnapshot(descendants)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,190 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
captureDescendantSnapshot,
|
||||
collectDescendantRows,
|
||||
DESCENDANT_KILL_GRACE_MS,
|
||||
killWithDescendantSweep,
|
||||
parseProcessTable,
|
||||
terminateDescendantSnapshot,
|
||||
type ProcessTableRow
|
||||
} from './pty-descendant-termination'
|
||||
|
||||
function row(
|
||||
pid: number,
|
||||
ppid: number,
|
||||
pgid: number,
|
||||
startedAt = `Mon Jul 13 12:54:47 2026 #${pid}`
|
||||
): ProcessTableRow {
|
||||
return { pid, ppid, pgid, startedAt }
|
||||
}
|
||||
|
||||
describe('parseProcessTable', () => {
|
||||
it('parses pid/ppid/pgid and keeps the space-containing lstart verbatim', () => {
|
||||
const rows = parseProcessTable(
|
||||
[
|
||||
' 101 1 101 Mon Jul 13 12:54:47 2026',
|
||||
'42017 101 42017 Tue Jul 14 01:02:03 2026 ',
|
||||
'',
|
||||
'not a process line'
|
||||
].join('\n')
|
||||
)
|
||||
expect(rows).toEqual([
|
||||
{ pid: 101, ppid: 1, pgid: 101, startedAt: 'Mon Jul 13 12:54:47 2026' },
|
||||
{ pid: 42017, ppid: 101, pgid: 42017, startedAt: 'Tue Jul 14 01:02:03 2026' }
|
||||
])
|
||||
})
|
||||
})
|
||||
|
||||
describe('collectDescendantRows', () => {
|
||||
it('walks multi-level descendants including detached-pgid grandchildren', () => {
|
||||
// shell(10) -> agent(20, own job pgid) -> detached tool shell(30, own
|
||||
// session-style pgid) -> git(31). 99 is unrelated.
|
||||
const table = [
|
||||
row(10, 1, 10),
|
||||
row(20, 10, 20),
|
||||
row(30, 20, 30),
|
||||
row(31, 30, 30),
|
||||
row(99, 1, 99)
|
||||
]
|
||||
const snapshot = collectDescendantRows(10, table)
|
||||
expect(snapshot.rootPgid).toBe(10)
|
||||
expect(snapshot.descendants.map((r) => r.pid)).toEqual([20, 30, 31])
|
||||
})
|
||||
|
||||
it('returns a null root pgid when the root row is already gone', () => {
|
||||
const snapshot = collectDescendantRows(10, [row(20, 10, 20)])
|
||||
expect(snapshot.rootPgid).toBeNull()
|
||||
expect(snapshot.descendants.map((r) => r.pid)).toEqual([20])
|
||||
})
|
||||
})
|
||||
|
||||
describe('captureDescendantSnapshot', () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers()
|
||||
})
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('resolves the descendant tree on POSIX', async () => {
|
||||
const readTable = vi.fn().mockResolvedValue([row(10, 1, 10), row(20, 10, 20)])
|
||||
const snapshot = await captureDescendantSnapshot(10, { readTable, platform: 'darwin' })
|
||||
expect(snapshot).toEqual({ rootPgid: 10, descendants: [row(20, 10, 20)] })
|
||||
})
|
||||
|
||||
it('is a null no-op on Windows', async () => {
|
||||
const readTable = vi.fn()
|
||||
expect(await captureDescendantSnapshot(10, { readTable, platform: 'win32' })).toBeNull()
|
||||
expect(readTable).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('degrades to null when ps fails', async () => {
|
||||
const readTable = vi.fn().mockRejectedValue(new Error('ps exploded'))
|
||||
expect(await captureDescendantSnapshot(10, { readTable, platform: 'linux' })).toBeNull()
|
||||
})
|
||||
|
||||
it('degrades to null when ps hangs past the timeout instead of blocking teardown', async () => {
|
||||
const readTable = vi.fn().mockReturnValue(new Promise<ProcessTableRow[]>(() => {}))
|
||||
const pending = captureDescendantSnapshot(10, {
|
||||
readTable,
|
||||
platform: 'darwin',
|
||||
timeoutMs: 1_000
|
||||
})
|
||||
await vi.advanceTimersByTimeAsync(1_000)
|
||||
expect(await pending).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe('terminateDescendantSnapshot', () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers()
|
||||
})
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('SIGTERMs the root group and every descendant immediately', () => {
|
||||
const sendSignal = vi.fn()
|
||||
terminateDescendantSnapshot(
|
||||
{ rootPgid: 10, descendants: [row(20, 10, 20), row(30, 20, 30)] },
|
||||
{ sendSignal, readTable: vi.fn().mockResolvedValue([]) }
|
||||
)
|
||||
expect(sendSignal.mock.calls).toEqual([
|
||||
[-10, 'SIGTERM'],
|
||||
[20, 'SIGTERM'],
|
||||
[30, 'SIGTERM']
|
||||
])
|
||||
})
|
||||
|
||||
it('SIGKILLs only identity-matched survivors after the grace window', async () => {
|
||||
const survivor = row(30, 20, 30)
|
||||
const exited = row(20, 10, 20)
|
||||
const recycled = row(40, 30, 40)
|
||||
const sendSignal = vi.fn()
|
||||
// At escalation time: 30 survives unchanged, 20 is gone, 40's pid now
|
||||
// belongs to a different (recycled) process with a different start time.
|
||||
const readTable = vi
|
||||
.fn()
|
||||
.mockResolvedValue([survivor, { ...recycled, startedAt: 'Tue Jul 14 09:00:00 2026' }])
|
||||
terminateDescendantSnapshot(
|
||||
{ rootPgid: 10, descendants: [exited, survivor, recycled] },
|
||||
{ sendSignal, readTable }
|
||||
)
|
||||
sendSignal.mockClear()
|
||||
await vi.advanceTimersByTimeAsync(DESCENDANT_KILL_GRACE_MS)
|
||||
expect(sendSignal.mock.calls).toEqual([[30, 'SIGKILL']])
|
||||
})
|
||||
|
||||
it('never escalates when the identity re-read fails', async () => {
|
||||
const sendSignal = vi.fn()
|
||||
const readTable = vi.fn().mockRejectedValue(new Error('ps exploded'))
|
||||
terminateDescendantSnapshot(
|
||||
{ rootPgid: 10, descendants: [row(20, 10, 20)] },
|
||||
{ sendSignal, readTable }
|
||||
)
|
||||
sendSignal.mockClear()
|
||||
await vi.advanceTimersByTimeAsync(DESCENDANT_KILL_GRACE_MS)
|
||||
expect(sendSignal).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('schedules no escalation for an empty descendant set', () => {
|
||||
const sendSignal = vi.fn()
|
||||
const readTable = vi.fn()
|
||||
terminateDescendantSnapshot({ rootPgid: 10, descendants: [] }, { sendSignal, readTable })
|
||||
expect(vi.getTimerCount()).toBe(0)
|
||||
expect(sendSignal.mock.calls).toEqual([[-10, 'SIGTERM']])
|
||||
})
|
||||
})
|
||||
|
||||
describe('killWithDescendantSweep', () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers()
|
||||
})
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('kills the root only after the snapshot resolves, then sweeps it', async () => {
|
||||
const sendSignal = vi.fn()
|
||||
const readTable = vi.fn().mockResolvedValue([row(10, 1, 10), row(20, 10, 20)])
|
||||
const killRoot = vi.fn()
|
||||
killWithDescendantSweep(10, killRoot, { readTable, sendSignal, platform: 'darwin' })
|
||||
expect(killRoot).not.toHaveBeenCalled()
|
||||
await vi.advanceTimersByTimeAsync(0)
|
||||
expect(killRoot).toHaveBeenCalledOnce()
|
||||
expect(sendSignal.mock.calls).toEqual([
|
||||
[-10, 'SIGTERM'],
|
||||
[20, 'SIGTERM']
|
||||
])
|
||||
})
|
||||
|
||||
it('still kills the root when the snapshot is unavailable', async () => {
|
||||
const sendSignal = vi.fn()
|
||||
const readTable = vi.fn().mockRejectedValue(new Error('ps exploded'))
|
||||
const killRoot = vi.fn()
|
||||
killWithDescendantSweep(10, killRoot, { readTable, sendSignal, platform: 'darwin' })
|
||||
await vi.advanceTimersByTimeAsync(0)
|
||||
expect(killRoot).toHaveBeenCalledOnce()
|
||||
expect(sendSignal).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,202 @@
|
||||
import { execFile } from 'node:child_process'
|
||||
|
||||
export const DESCENDANT_KILL_GRACE_MS = 2_000
|
||||
export const DESCENDANT_SNAPSHOT_TIMEOUT_MS = 1_000
|
||||
// Why: a full process table on a busy host can exceed execFile's 1MB default;
|
||||
// truncation would silently drop descendants from the snapshot.
|
||||
const PS_MAX_BUFFER_BYTES = 32 * 1024 * 1024
|
||||
|
||||
export type ProcessTableRow = {
|
||||
pid: number
|
||||
ppid: number
|
||||
pgid: number
|
||||
/** ps lstart text, kept verbatim. (pid, startedAt) is the identity token that
|
||||
* gates the delayed SIGKILL so it can never land on a recycled pid. */
|
||||
startedAt: string
|
||||
}
|
||||
|
||||
export type DescendantSnapshot = {
|
||||
rootPgid: number | null
|
||||
descendants: ProcessTableRow[]
|
||||
}
|
||||
|
||||
export type ProcessTableReader = () => Promise<ProcessTableRow[]>
|
||||
export type SignalSender = (pid: number, signal: NodeJS.Signals) => void
|
||||
|
||||
export function parseProcessTable(psOutput: string): ProcessTableRow[] {
|
||||
const rows: ProcessTableRow[] = []
|
||||
for (const line of psOutput.split('\n')) {
|
||||
// lstart itself contains spaces ("Mon Jul 13 12:54:47 2026"), so only the
|
||||
// three leading numeric columns are positional.
|
||||
const match = line.match(/^\s*(\d+)\s+(\d+)\s+(\d+)\s+(.+?)\s*$/)
|
||||
if (!match) {
|
||||
continue
|
||||
}
|
||||
rows.push({
|
||||
pid: Number(match[1]),
|
||||
ppid: Number(match[2]),
|
||||
pgid: Number(match[3]),
|
||||
startedAt: match[4]
|
||||
})
|
||||
}
|
||||
return rows
|
||||
}
|
||||
|
||||
function readProcessTable(): Promise<ProcessTableRow[]> {
|
||||
return new Promise((resolve, reject) => {
|
||||
execFile(
|
||||
'ps',
|
||||
['-axo', 'pid=,ppid=,pgid=,lstart='],
|
||||
{ maxBuffer: PS_MAX_BUFFER_BYTES },
|
||||
(error, stdout) => {
|
||||
if (error) {
|
||||
reject(error)
|
||||
return
|
||||
}
|
||||
resolve(parseProcessTable(stdout))
|
||||
}
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
export function collectDescendantRows(
|
||||
rootPid: number,
|
||||
table: ProcessTableRow[]
|
||||
): DescendantSnapshot {
|
||||
const childrenByPpid = new Map<number, ProcessTableRow[]>()
|
||||
let rootRow: ProcessTableRow | null = null
|
||||
for (const row of table) {
|
||||
if (row.pid === rootPid) {
|
||||
rootRow = row
|
||||
continue
|
||||
}
|
||||
const siblings = childrenByPpid.get(row.ppid)
|
||||
if (siblings) {
|
||||
siblings.push(row)
|
||||
} else {
|
||||
childrenByPpid.set(row.ppid, [row])
|
||||
}
|
||||
}
|
||||
const descendants: ProcessTableRow[] = []
|
||||
const queue = [rootPid]
|
||||
while (queue.length > 0) {
|
||||
const pid = queue.shift()!
|
||||
for (const child of childrenByPpid.get(pid) ?? []) {
|
||||
descendants.push(child)
|
||||
queue.push(child.pid)
|
||||
}
|
||||
}
|
||||
return { rootPgid: rootRow?.pgid ?? null, descendants }
|
||||
}
|
||||
|
||||
type SnapshotDeps = {
|
||||
readTable?: ProcessTableReader
|
||||
platform?: NodeJS.Platform
|
||||
timeoutMs?: number
|
||||
}
|
||||
|
||||
/**
|
||||
* Snapshots a PTY root's live descendant tree. Must run BEFORE the root is
|
||||
* signalled: once the root dies, surviving descendants reparent to pid 1 and
|
||||
* can no longer be found by a ppid walk. Resolves null (never rejects) on
|
||||
* Windows, ps failure, or timeout — callers then degrade to today's
|
||||
* shell-only kill.
|
||||
*/
|
||||
export async function captureDescendantSnapshot(
|
||||
rootPid: number,
|
||||
deps: SnapshotDeps = {}
|
||||
): Promise<DescendantSnapshot | null> {
|
||||
const platform = deps.platform ?? process.platform
|
||||
if (platform === 'win32' || !Number.isInteger(rootPid) || rootPid <= 0) {
|
||||
return null
|
||||
}
|
||||
const readTable = deps.readTable ?? readProcessTable
|
||||
const timeoutMs = deps.timeoutMs ?? DESCENDANT_SNAPSHOT_TIMEOUT_MS
|
||||
try {
|
||||
// Why the timeout: the snapshot sits in front of the actual kill; a wedged
|
||||
// ps must degrade to shell-only kill, not block session teardown.
|
||||
const table = await Promise.race([
|
||||
readTable(),
|
||||
new Promise<null>((resolve) => setTimeout(() => resolve(null), timeoutMs).unref?.())
|
||||
])
|
||||
if (!table) {
|
||||
return null
|
||||
}
|
||||
return collectDescendantRows(rootPid, table)
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Standard agent-session kill sequencing: snapshot the descendant tree, only
|
||||
* then run the caller's root kill, then sweep the snapshot. Callers must not
|
||||
* signal the root before this runs — a dead root's descendants reparent to
|
||||
* pid 1 and become unfindable. Snapshot failure degrades to killRoot alone.
|
||||
*/
|
||||
export function killWithDescendantSweep(
|
||||
rootPid: number,
|
||||
killRoot: () => void,
|
||||
deps: SnapshotDeps & TerminateDeps = {}
|
||||
): void {
|
||||
void captureDescendantSnapshot(rootPid, deps).then((snapshot) => {
|
||||
killRoot()
|
||||
if (snapshot) {
|
||||
terminateDescendantSnapshot(snapshot, deps)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
function defaultSendSignal(pid: number, signal: NodeJS.Signals): void {
|
||||
try {
|
||||
process.kill(pid, signal)
|
||||
} catch {
|
||||
/* already gone */
|
||||
}
|
||||
}
|
||||
|
||||
type TerminateDeps = {
|
||||
readTable?: ProcessTableReader
|
||||
sendSignal?: SignalSender
|
||||
graceMs?: number
|
||||
}
|
||||
|
||||
/**
|
||||
* Terminates a snapshotted descendant tree: SIGTERM now (root process group +
|
||||
* every descendant, reaching detached-pgid children the PTY's SIGHUP cannot),
|
||||
* then after a grace window SIGKILL whatever from the snapshot is still alive.
|
||||
* The SIGKILL wave re-reads the process table and only signals pids whose
|
||||
* start time still matches the snapshot, so a recycled pid is never killed.
|
||||
*/
|
||||
export function terminateDescendantSnapshot(
|
||||
snapshot: DescendantSnapshot,
|
||||
deps: TerminateDeps = {}
|
||||
): void {
|
||||
const sendSignal = deps.sendSignal ?? defaultSendSignal
|
||||
const readTable = deps.readTable ?? readProcessTable
|
||||
if (snapshot.rootPgid !== null && snapshot.rootPgid > 0) {
|
||||
sendSignal(-snapshot.rootPgid, 'SIGTERM')
|
||||
}
|
||||
for (const row of snapshot.descendants) {
|
||||
sendSignal(row.pid, 'SIGTERM')
|
||||
}
|
||||
if (snapshot.descendants.length === 0) {
|
||||
return
|
||||
}
|
||||
const timer = setTimeout(() => {
|
||||
void readTable().then(
|
||||
(table) => {
|
||||
const liveByPid = new Map(table.map((row) => [row.pid, row]))
|
||||
for (const row of snapshot.descendants) {
|
||||
if (liveByPid.get(row.pid)?.startedAt === row.startedAt) {
|
||||
sendSignal(row.pid, 'SIGKILL')
|
||||
}
|
||||
}
|
||||
},
|
||||
() => {
|
||||
/* table read failed — leave survivors rather than risk recycled pids */
|
||||
}
|
||||
)
|
||||
}, deps.graceMs ?? DESCENDANT_KILL_GRACE_MS)
|
||||
timer.unref?.()
|
||||
}
|
||||
@@ -0,0 +1,97 @@
|
||||
import { chmodSync, existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { test, expect } from './helpers/orca-app'
|
||||
import { waitForActiveWorktree, waitForSessionReady } from './helpers/store'
|
||||
|
||||
function isProcessAlive(pid: number): boolean {
|
||||
try {
|
||||
process.kill(pid, 0)
|
||||
return true
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
// Reproduces the orphaned-descendant battery-drain incident (STA-1800): an
|
||||
// agent CLI spawns a tool child in a detached process group, the session is
|
||||
// killed, and the child must not survive. The stand-in agent's first command
|
||||
// token is literally `claude` so PTY spawn recognition marks the session as an
|
||||
// agent; tab close → pty.kill routing is already covered by
|
||||
// terminal-parked-close-retirement.spec.ts, so this spec drives pty.kill.
|
||||
test('killing an agent PTY terminates its detached-pgid descendants', async ({ orcaPage }) => {
|
||||
test.skip(process.platform === 'win32', 'descendant tree-kill is POSIX-only for now')
|
||||
|
||||
const stage = mkdtempSync(join(tmpdir(), 'orca-agent-descendant-'))
|
||||
const markerPath = join(stage, 'detached-child.pid')
|
||||
const spawnerPath = join(stage, 'spawn-detached.cjs')
|
||||
writeFileSync(
|
||||
spawnerPath,
|
||||
[
|
||||
"const { spawn } = require('node:child_process')",
|
||||
// detached:true → setsid → own pgid/session, exactly the topology of an
|
||||
// agent CLI's tool subprocess that a dying shell's SIGHUP cannot reach.
|
||||
"const child = spawn('sleep', ['31337'], { detached: true, stdio: 'ignore' })",
|
||||
'child.unref()',
|
||||
"require('node:fs').writeFileSync(process.argv[2], String(child.pid))",
|
||||
// Stay alive like a real agent at its prompt: the detached child's ppid
|
||||
// must remain intact at kill time — a pre-orphaned child is the separate
|
||||
// crash-path scenario that only the PR-2 sweep can catch.
|
||||
'setInterval(() => {}, 1000)',
|
||||
''
|
||||
].join('\n')
|
||||
)
|
||||
const fakeAgentPath = join(stage, 'claude')
|
||||
writeFileSync(fakeAgentPath, `#!/bin/sh\nexec "${process.execPath}" "${spawnerPath}" "$1"\n`)
|
||||
chmodSync(fakeAgentPath, 0o755)
|
||||
|
||||
let detachedChildPid = 0
|
||||
try {
|
||||
await waitForSessionReady(orcaPage)
|
||||
const worktreeId = await waitForActiveWorktree(orcaPage)
|
||||
|
||||
const ptyId = await orcaPage.evaluate(
|
||||
async ({ command, cwd, worktreeId: wt }) => {
|
||||
const result = await window.api.pty.spawn({
|
||||
cols: 120,
|
||||
rows: 40,
|
||||
cwd,
|
||||
command,
|
||||
launchAgent: 'claude',
|
||||
worktreeId: wt
|
||||
})
|
||||
return result.id
|
||||
},
|
||||
{ command: `'${fakeAgentPath}' '${markerPath}'`, cwd: stage, worktreeId }
|
||||
)
|
||||
expect(ptyId).toBeTruthy()
|
||||
|
||||
await expect
|
||||
.poll(() => existsSync(markerPath), {
|
||||
timeout: 20_000,
|
||||
message: 'stand-in agent never spawned its detached child'
|
||||
})
|
||||
.toBe(true)
|
||||
detachedChildPid = Number(readFileSync(markerPath, 'utf8').trim())
|
||||
expect(detachedChildPid).toBeGreaterThan(0)
|
||||
expect(isProcessAlive(detachedChildPid)).toBe(true)
|
||||
|
||||
await orcaPage.evaluate((id) => window.api.pty.kill(id), ptyId)
|
||||
|
||||
await expect
|
||||
.poll(() => isProcessAlive(detachedChildPid), {
|
||||
timeout: 15_000,
|
||||
message: `detached descendant ${detachedChildPid} survived the agent PTY kill`
|
||||
})
|
||||
.toBe(false)
|
||||
} finally {
|
||||
if (detachedChildPid > 0 && isProcessAlive(detachedChildPid)) {
|
||||
try {
|
||||
process.kill(detachedChildPid, 'SIGKILL')
|
||||
} catch {
|
||||
/* already gone */
|
||||
}
|
||||
}
|
||||
rmSync(stage, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
Reference in New Issue
Block a user