Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 13 additions & 8 deletions src/main/acp/acp-agent-connection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { EventEmitter } from 'node:events'
import { PassThrough } from 'node:stream'
import { afterEach, describe, expect, it, vi } from 'vitest'
import type { spawnProcess } from '../../shared/child-process/run-process'
import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from '../provider-process/provider-process-supervisor'
import { SUPERVISED_PROVIDER_GRACEFUL_EXIT_MS } from '../provider-process/provider-process-supervisor'
import { ROOT_ONLY_GRACEFUL_EXIT_MS } from '../provider-process/provider-process-close'
import type { terminateProviderProcessTree } from '../provider-process/provider-process-teardown'
import {
Expand All @@ -12,16 +12,17 @@ import {
} from './acp-agent-connection'
import { AcpScriptedAgent, deferred, tick } from './acp-scripted-agent.test-support'

const UNOBSERVED = { tree: 'unverifiable', providerKilled: false } as const
const teardown = vi.hoisted(() =>
vi.fn<typeof terminateProviderProcessTree>(async () => 'unverifiable')
vi.fn<typeof terminateProviderProcessTree>(async () => UNOBSERVED)
)
vi.mock('../provider-process/provider-process-teardown', () => ({
terminateProviderProcessTree: teardown
}))

const opened: { connection: AcpAgentConnection; exit: () => void }[] = []
const grace =
process.platform === 'win32' ? ROOT_ONLY_GRACEFUL_EXIT_MS : PROVIDER_SUPERVISOR_MAX_STOP_MS
process.platform === 'win32' ? ROOT_ONLY_GRACEFUL_EXIT_MS : SUPERVISED_PROVIDER_GRACEFUL_EXIT_MS
const start = { cwd: '/execution-host/folder', mcpServers: [] }
const prompt = [{ type: 'text', text: 'hello' }] as const

Expand Down Expand Up @@ -66,7 +67,7 @@ afterEach(async () => {
exit()
await connection.close()
}
teardown.mockReset().mockResolvedValue('unverifiable')
teardown.mockReset().mockResolvedValue(UNOBSERVED)
vi.useRealTimers()
})

Expand All @@ -93,7 +94,11 @@ describe('ACP process-owning connection', () => {
connection.resumeReading()
expect(child.stdout.isPaused()).toBe(false)
expect(await connection.close()).toBe(true)
expect(connection.lastCloseResult).toEqual({ root: 'exited', tree: null })
expect(connection.lastCloseResult).toEqual({
root: 'exited',
tree: null,
providerKilled: false
})
})

it('settles prompts and permission signals on proven exit with stdout still open', async () => {
Expand Down Expand Up @@ -187,7 +192,7 @@ describe('ACP process-owning connection', () => {
expect(teardown).toHaveBeenCalledOnce()
teardown.mockImplementationOnce(async () => {
child.emit('exit', null, 'SIGKILL')
return 'unverifiable'
return UNOBSERVED
})
const retry = connection.close()
await vi.advanceTimersByTimeAsync(grace + 1_000)
Expand All @@ -203,15 +208,15 @@ describe('ACP process-owning connection', () => {
async (tree) => {
vi.useFakeTimers()
const { connection, child } = fixture({}, { exitOnEnd: false })
teardown.mockResolvedValueOnce(tree)
teardown.mockResolvedValueOnce({ tree, providerKilled: false })
const close = connection.close()
await vi.advanceTimersByTimeAsync(grace + 1_000)
expect(await close).toBe(false)
child.emit('exit', null, 'SIGKILL')
expect(connection.processTreeUnproven).toBe(true)
expect(await connection.close()).toBe(true)
expect(connection.processTreeUnproven).toBe(true)
expect(connection.lastCloseResult).toEqual({ root: 'live', tree })
expect(connection.lastCloseResult).toEqual({ root: 'live', tree, providerKilled: false })
expect(teardown).toHaveBeenCalledOnce()
}
)
Expand Down
128 changes: 128 additions & 0 deletions src/main/claude/claude-agent-sdk-exit-proof-supervised.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
// The Claude reaper's forced step, per platform: on POSIX the root is the provider supervisor and
// Claude leads a group of its own; on Windows the root is Claude.

import type { ChildProcess } from 'node:child_process'
import { describe, expect, it, vi } from 'vitest'
import type { DescendantTreeVerdict } from '../pty-descendant-exit-verification'
import type { DescendantSnapshot } from '../pty-descendant-termination'
import type { WindowsDescendantSnapshot } from '../windows-descendant-exit-verification'
import { createClaudeChildTreeReaper } from './claude-agent-sdk-exit-proof'

const SUPERVISOR = 424_242
const CLAUDE = 424_243

/** The supervisor's walk: Claude leads its own group, with one tool in it. */
function supervisorWalk(): DescendantSnapshot {
const started = 'Mon Jan 1 00:00:00 2026'
return {
root: { pid: SUPERVISOR, startedAt: started },
rootPgid: SUPERVISOR,
descendants: [
{ pid: CLAUDE, ppid: SUPERVISOR, pgid: CLAUDE, startedAt: started },
{ pid: CLAUDE + 1, ppid: CLAUDE, pgid: CLAUDE, startedAt: started }
],
capturedAtMs: 1
}
}

function root(kills: string[], delivered = true) {
return {
pid: SUPERVISOR,
kill: vi.fn<ChildProcess['kill']>((signal) => {
kills.push(`root ${String(signal)}`)
return delivered
})
}
}

describe('claude reaper forced step on a supervised POSIX root', () => {
it("kills Claude's own group before the supervisor, then verifies the rest of the tree", async () => {
const kills: string[] = []
const verify = Promise.withResolvers<DescendantTreeVerdict>()
const terminateDescendants = vi.fn(() => {
kills.push('verify')
return verify.promise
})
const tree = createClaudeChildTreeReaper(root(kills), {
platform: 'darwin',
supervised: true,
captureDescendants: async () => supervisorWalk(),
terminateDescendants,
signalProcessGroup: (pgid, signal) => kills.push(`group ${pgid} ${signal}`)
})

const reaped = tree.reap()
await vi.waitFor(() => expect(terminateDescendants).toHaveBeenCalled())
expect(tree.providerKilled).toBe(true)
verify.resolve('exited')

await expect(reaped).resolves.toBe('exited')
expect(kills).toEqual(['root SIGSTOP', `group ${CLAUDE} SIGKILL`, 'root SIGKILL', 'verify'])
})

// Before: no snapshot meant a SIGKILL to the supervisor alone, orphaning Claude in its own group
// with nobody left to stop it, and the root's exit then read as Claude gone.
it('never kills the supervisor alone when the process table cannot be read', async () => {
const kills: string[] = []
const signalProcessGroup = vi.fn()
const tree = createClaudeChildTreeReaper(root(kills), {
platform: 'linux',
supervised: true,
captureDescendants: async () => null,
signalProcessGroup
})

await expect(tree.reap()).resolves.toBe('unverifiable')
expect(kills).toEqual(['root SIGSTOP', 'root SIGCONT'])
expect(signalProcessGroup).not.toHaveBeenCalled()
expect(tree.providerKilled).toBe(false)
})

it('signals nothing once the supervised root has exited, and only verifies the tree', async () => {
const kills: string[] = []
let exited = false
const terminateDescendants = vi.fn(async () => 'exited' as const)
const tree = createClaudeChildTreeReaper(root(kills), {
platform: 'darwin',
supervised: true,
exited: () => exited,
captureDescendants: async () => supervisorWalk(),
terminateDescendants
})
await tree.capture()
exited = true

await expect(tree.reap()).resolves.toBe('exited')
expect(kills).toEqual([])
expect(tree.providerKilled).toBe(false)
})
})

describe('claude reaper forced step on Windows, where the root is Claude', () => {
function windowsWalk(): WindowsDescendantSnapshot {
return {
root: { pid: SUPERVISOR, creationTimeMs: 1_700_000_000_001 },
descendants: [],
unidentifiedCount: 0,
capturedAtMs: 1
}
}

it.each([
[true, 'delivered'],
[false, 'not delivered']
])('reads providerKilled=%s from a TerminateProcess %s', async (delivered) => {
const kills: string[] = []
const tree = createClaudeChildTreeReaper(root(kills, delivered), {
platform: 'win32',
verifyRootIdentity: async () => true,
captureWindowsDescendants: async () => windowsWalk(),
terminateWindowsTree: async () => undefined,
terminateWindowsDescendants: async () => 'exited'
})

await tree.reap()
expect(kills).toEqual(['root SIGKILL'])
expect(tree.providerKilled).toBe(delivered)
})
})
3 changes: 2 additions & 1 deletion src/main/claude/claude-agent-sdk-exit-proof.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,8 @@ function mockTree(verdicts: DescendantTreeVerdict[]): ClaudeChildTreeReaper & {
}),
get treeVerdict() {
return treeVerdict
}
},
providerKilled: false
}
}

Expand Down
60 changes: 45 additions & 15 deletions src/main/claude/claude-agent-sdk-exit-proof.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
} from '../windows-descendant-exit-verification'
import { mergeClaudeCapturedTrees, type ClaudeCapturedTree } from './claude-child-tree-snapshot'
import { terminateClaudeRoot, terminateClaudeWindowsRoot } from './claude-child-root-termination'
import { killSupervisedProviderGroup } from '../provider-process/provider-supervised-group-kill'
import {
proveClaudeChildExitWithReaper,
type ClaudeChildExitProofInput
Expand Down Expand Up @@ -61,6 +62,9 @@ export type ClaudeChildTreeReaperDeps = {
platform?: NodeJS.Platform
/** Whether the root's exit has been observed; only a live root can be walked. */
exited?: () => boolean
/** The root is the POSIX provider supervisor, whose provider runs in a group of its own. */
supervised?: boolean
signalProcessGroup?: (pgid: number, signal: NodeJS.Signals) => void
captureDescendants?: (rootPid: number) => Promise<DescendantSnapshot | null>
terminateDescendants?: (snapshot: DescendantSnapshot) => Promise<DescendantTreeVerdict>
terminateWindowsTree?: (root: WindowsProcessIdentity) => Promise<void>
Expand Down Expand Up @@ -93,6 +97,8 @@ export type ClaudeChildTreeReaper = {
* which no later caller may collapse into "unknown".
*/
readonly treeVerdict: DescendantTreeVerdict
/** Latched once a reap's kill reached Claude itself; never an observed exit. */
readonly providerKilled: boolean
}

/**
Expand All @@ -119,6 +125,7 @@ export function createClaudeChildTreeReaper(
let queuedRefresh: Promise<void> | null = null
let inFlight: Promise<DescendantTreeVerdict> | null = null
let treeVerdict: DescendantTreeVerdict = 'unverifiable'
let providerKilled = false

// Consulted only on win32: POSIX signals descendants by revalidated identity
// and reaches the root solely through Node's handle, so neither needs a probe.
Expand Down Expand Up @@ -262,7 +269,10 @@ export function createClaudeChildTreeReaper(

/** The only source of a tree verdict: every `exited` here is an observation. */
async function judgeTree(): Promise<DescendantTreeVerdict> {
const killRoot = (): boolean => terminateClaudeRoot({ child, exited })
// Only an unsupervised root is Claude itself; a supervisor is killed by the forced step below.
const killRoot = (): void => {
providerKilled ||= terminateClaudeRoot({ child, exited })
}
const rootPid = child.pid
if (!rootPid) {
// Never spawned, so the OS never created a tree to orphan.
Expand All @@ -272,7 +282,7 @@ export function createClaudeChildTreeReaper(
if (platform === 'win32') {
// Why taskkill's own outcome is never the verdict: it resolves identically
// on a timeout, an access denial, a recycled root and a real kill.
const { rootVerified } = await terminateClaudeWindowsRoot({
const { rootVerified, rootKilled } = await terminateClaudeWindowsRoot({
snapshot: snapshot?.platform === 'win32' ? snapshot.tree : null,
exited,
verifyRoot: (root) => verifyRoot(root),
Expand All @@ -282,8 +292,10 @@ export function createClaudeChildTreeReaper(
: terminateIdentifiedWindowsProcessTree(root, {
ownsRoot: () => !exited()
}).then(() => undefined),
killRoot
// Windows runs Claude without a supervisor: the root is Claude.
killRoot: () => terminateClaudeRoot({ child, exited })
})
providerKilled ||= rootKilled
if (!rootVerified && !exited()) {
return 'unverifiable'
}
Expand All @@ -293,30 +305,44 @@ export function createClaudeChildTreeReaper(
)
: 'unverifiable'
}
if (deps.supervised && !exited()) {
// Killing the supervisor alone would orphan Claude in its own group; this kills that group
// first, and leaves the supervisor running its own stop when the group cannot be named.
const forced = await killSupervisedProviderGroup(child, rootPid, {
site: 'claude-provider-teardown',
...(deps.captureDescendants ? { captureDescendants: deps.captureDescendants } : {}),
...(deps.signalProcessGroup ? { signalProcessGroup: deps.signalProcessGroup } : {})
})
providerKilled ||= forced.provider !== 'unknown'
}
if (snapshot?.platform !== 'posix') {
killRoot()
if (!deps.supervised) {
killRoot()
}
return 'unverifiable'
}
if (snapshot.tree.descendants.length === 0) {
// Read while the root was alive and childless: a later table read has no
// row it could match, so it would add nothing to this observation.
killRoot()
if (!deps.supervised) {
killRoot()
}
return 'exited'
}
// Why the root is killed while verification is already running, and never
// SIGSTOPped first the way the Codex non-group path does: measured on macOS, a
// killed child of a stopped parent stays a zombie row in ps with its lstart
// and pgid intact, so verification cannot pass until the root is dead. The
// descendants are signalled by the verifier as soon as it revalidates their
// identities; the root's death then reparents any zombies to init, which
// reaps them. After a root exit the kill is a no-op: Node drops the handle
// on exit and never signals a possibly recycled pid.
// Why an unsupervised root is killed while verification is already running: measured on
// macOS, a killed child of a stopped or living parent that has not reaped it stays a zombie row
// in ps with its lstart and pgid intact, so verification cannot pass until the root is dead.
// A supervised root is already dead here when the forced step killed Claude's group. Either
// way the root's death reparents zombies to init, which reaps them. After a root exit the
// kill is a no-op: Node drops the handle on exit and never signals a possibly recycled pid.
const verdictPromise = deps.terminateDescendants
? deps.terminateDescendants(snapshot.tree)
: terminateDescendantSnapshotWithVerdict(snapshot.tree, {
requireIdentityBeforeSignal: true
})
killRoot()
if (!deps.supervised) {
killRoot()
}
// What the verification observed is the verdict: a kill that reports no
// signal means the handle was already gone, never that the tree survived.
return verdictPromise
Expand Down Expand Up @@ -346,6 +372,9 @@ export function createClaudeChildTreeReaper(
},
get treeVerdict() {
return treeVerdict
},
get providerKilled() {
return providerKilled
}
}
}
Expand All @@ -362,7 +391,8 @@ export function createClaudeChildTreeReaper(
export function proveClaudeChildExit(input: ClaudeChildExitProofInput): Promise<boolean> {
return proveClaudeChildExitWithReaper(input, () =>
createClaudeChildTreeReaper(input.managed.child, {
exited: () => input.managed.rootVerdict === 'exited'
exited: () => input.managed.rootVerdict === 'exited',
supervised: input.managed.supervised
})
)
}
Loading
Loading