Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
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
104 changes: 100 additions & 4 deletions src/main/claude/claude-agent-sdk-process-spawn.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,22 @@ import { describe, expect, it, vi } from 'vitest'
import type { SpawnOptions as SdkSpawnOptions } from '@anthropic-ai/claude-agent-sdk'
import { resolveSpawn, type spawnProcess } from '../../shared/child-process/run-process'
import type { ProcessSpec } from '../../shared/child-process/process-spec'
import type * as ProviderSupervisor from '../codex/codex-app-server-posix-supervisor'
import { createProviderSpawnSpec } from '../codex/codex-app-server-posix-supervisor'
import { createClaudeCodeProcessSpawn } from './claude-agent-sdk-process-spawn'
import { proveClaudeChildExitWithReaper } from './claude-child-exit-proof-ladder'

vi.mock('../codex/codex-app-server-posix-supervisor', async (importOriginal) => {
const actual = await importOriginal<typeof ProviderSupervisor>()
return { ...actual, createProviderSpawnSpec: vi.fn(actual.createProviderSpawnSpec) }
})

type FakeChild = EventEmitter & {
pid: number
stdin: PassThrough
stdout: PassThrough
stderr: PassThrough
kill: ReturnType<typeof vi.fn>
kill: ReturnType<typeof vi.fn<(signal?: NodeJS.Signals | number) => boolean>>
}

function fakeSpawn() {
Expand All @@ -20,7 +28,7 @@ function fakeSpawn() {
child.stdin = new PassThrough()
child.stdout = new PassThrough()
child.stderr = new PassThrough()
child.kill = vi.fn(() => true)
child.kill = vi.fn((_signal?: NodeJS.Signals | number) => true)
const specs: ProcessSpec[] = []
const spawnImpl = ((spec: ProcessSpec) => {
specs.push(spec)
Expand All @@ -43,7 +51,7 @@ function sdkOptions(overrides: Partial<SdkSpawnOptions> = {}): SdkSpawnOptions {
describe('claude agent SDK process spawn', () => {
it('routes the SDK spawn through Orca and retains the pid the lease adjudicates on', () => {
const process = fakeSpawn()
const spawn = createClaudeCodeProcessSpawn(process.spawnImpl)
const spawn = createClaudeCodeProcessSpawn(process.spawnImpl, 'win32')

expect(spawn.pid).toBeUndefined()
expect(spawn.child).toBeNull()
Expand All @@ -52,15 +60,103 @@ describe('claude agent SDK process spawn', () => {
expect(child).toBe(process.child)
expect(spawn.child).toBe(process.child)
expect(spawn.pid).toBe(4321)
// Windows has no supervisor: Claude itself is the child.
expect(spawn.supervised).toBe(false)
expect(process.specs[0]).toEqual({
program: '/usr/local/bin/claude',
args: ['--output-format', 'stream-json'],
cwd: '/work/repo',
env: { PATH: '/usr/bin', CLAUDE_CONFIG_DIR: '/accounts/one' },
detached: false,
stdio: ['pipe', 'pipe', 'pipe']
})
})

it.each(['darwin', 'linux'] as const)(
'starts Claude under the provider supervisor on %s, which is then the pid the lease records',
(platform) => {
const process = fakeSpawn()
const spawn = createClaudeCodeProcessSpawn(process.spawnImpl, platform)
spawn.spawn(sdkOptions())

const [spec] = process.specs
if (!spec) {
throw new Error('the spawner never built a spec')
}
expect(spawn.supervised).toBe(true)
expect(spawn.pid).toBe(4321)
expect(spec.program).toBe(globalThis.process.execPath)
expect(spec.args?.[0]).toBe('-e')
expect(spec.detached).toBe(true)
expect(spec.cwd).toBe('/work/repo')
const supervisorSpec = JSON.parse(
Buffer.from(String(spec.env?.ORCA_PROVIDER_SUPERVISOR_SPEC), 'base64').toString()
)
expect(supervisorSpec).toMatchObject({
command: '/usr/local/bin/claude',
args: ['--output-format', 'stream-json'],
cwd: '/work/repo',
ownerPid: globalThis.process.pid
})
// The supervisor passes its env to Claude minus its own two keys.
expect(spec.env).toMatchObject({ PATH: '/usr/bin', CLAUDE_CONFIG_DIR: '/accounts/one' })
}
)

it.each([
{ platform: 'darwin', specSupervised: false },
{ platform: 'win32', specSupervised: true }
] as const)(
'stops Claude by the spawn spec\u2019s supervision on $platform, never the platform',
async ({ platform, specSupervised }) => {
const actual = await vi.importActual<typeof ProviderSupervisor>(
'../codex/codex-app-server-posix-supervisor'
)
vi.mocked(createProviderSpawnSpec).mockImplementationOnce((...args) => ({
...actual.createProviderSpawnSpec(...args),
supervised: specSupervised
}))
const process = fakeSpawn()
const spawn = createClaudeCodeProcessSpawn(process.spawnImpl, platform)
spawn.spawn(sdkOptions())
expect(spawn.supervised).toBe(specSupervised)

let exited = false
let settle = (): void => {}
const exitPromise = new Promise<void>((resolve) => {
settle = resolve
})
// Claude leaves shortly after stdin ends, so the ladder never needs its forced rung.
process.child.stdin.on('finish', () =>
setTimeout(() => {
exited = true
settle()
}, 10)
)
const tree = {
capture: vi.fn(async () => {}),
reap: vi.fn(async () => 'exited' as const),
treeVerdict: 'exited' as const
}
await proveClaudeChildExitWithReaper(
{
child: process.child,
exitPromise,
exited: () => exited,
tree,
supervised: spawn.supervised
},
() => tree
)
// SIGTERM to an unsupervised Claude on Windows is TerminateProcess; a skipped one leaves it running.
if (specSupervised) {
expect(process.child.kill).toHaveBeenCalledWith('SIGTERM')
} else {
expect(process.child.kill).not.toHaveBeenCalled()
}
}
)

it('keeps the child out of the SDK abort path so exit proof stays Orca-owned', () => {
const process = fakeSpawn()
const controller = new AbortController()
Expand All @@ -86,7 +182,7 @@ describe('claude agent SDK process spawn', () => {

it('hands a Windows .cmd shim to Orca\u2019s argument encoder', () => {
const process = fakeSpawn()
createClaudeCodeProcessSpawn(process.spawnImpl).spawn(
createClaudeCodeProcessSpawn(process.spawnImpl, 'win32').spawn(
sdkOptions({
command: 'C:\\Users\\dev\\AppData\\npm\\claude.cmd',
args: ['--setting-sources=user,project,local', '--session-id', 'a b&c']
Expand Down
38 changes: 32 additions & 6 deletions src/main/claude/claude-agent-sdk-process-spawn.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { SpawnOptions as ClaudeAgentSdkSpawnOptions } from '@anthropic-ai/claude-agent-sdk'
import { spawnProcess } from '../../shared/child-process/run-process'
import { createProviderSpawnSpec } from '../codex/codex-app-server-posix-supervisor'

/** Derived rather than imported: only src/shared/child-process may name node:child_process. */
type ClaudeCodeChild = ReturnType<typeof spawnProcess>
Expand All @@ -11,8 +12,13 @@ export type ClaudeCodeProcessSpawn = {
spawn: (options: ClaudeAgentSdkSpawnOptions) => ClaudeCodeChild
/** The retained child, so Orca keeps its own tree-kill and exit-proof ladder. Null until the SDK spawns. */
readonly child: ClaudeCodeChild | null
/** Ownership proof: the durable lease adjudicates on this pid plus start time plus the spawn token. */
/**
* Ownership proof: the durable lease adjudicates on this pid plus start time plus the spawn
* token. On POSIX it is the provider supervisor's, which outlives Claude by construction.
*/
readonly pid: number | undefined
/** The spawn spec's verdict, so the close ladder never re-decides it. False until the SDK spawns. */
readonly supervised: boolean
readonly stderrTail: string
}

Expand All @@ -31,24 +37,41 @@ function definedEnv(env: Record<string, string | undefined>): Record<string, str
*
* Two independent reasons: the SDK's `SpawnedProcess` has no pid, and Orca's
* spawner is the only path that encodes `.cmd` arguments safely on Windows.
*
* On POSIX Claude runs under the provider supervisor, so an Orca that dies stops
* it instead of leaving it to finish its turn, tools and edits included, with
* nobody watching. Windows has no supervisor and spawns Claude directly.
*/
export function createClaudeCodeProcessSpawn(
spawnImpl: typeof spawnProcess = spawnProcess
spawnImpl: typeof spawnProcess = spawnProcess,
platform: NodeJS.Platform = process.platform
): ClaudeCodeProcessSpawn {
let child: ClaudeCodeChild | null = null
let stderrTail = ''
let supervised = false
return {
spawn: (options) => {
const spec = createProviderSpawnSpec(
{
command: options.command,
args: [...options.args],
...(options.cwd === undefined ? {} : { cwd: options.cwd })
},
definedEnv(options.env),
platform
)
Comment on lines +54 to +62

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Tools survive supervisor escalation If Orca dies while Claude is running a Bash tool and Claude ignores SIGTERM, the supervisor eventually SIGKILLs only Claude’s process group. The tool runs in a separate group, so it can continue working unwatched after Claude exits. The SIGTERM-ignoring test checks Claude’s exit but does not check the tool; the owner-death test checks the tool only when Claude handles SIGTERM.

// Why `options.signal` is dropped: it would let the SDK kill the child outside
// Orca's ladder, and close() may never report an exit it did not observe.
const spawned = spawnImpl({
program: options.command,
args: [...options.args],
...(options.cwd === undefined ? {} : { cwd: options.cwd }),
env: definedEnv(options.env),
program: spec.program,
args: spec.args,
cwd: spec.cwd,
env: spec.env,
detached: spec.detached,
stdio: ['pipe', 'pipe', 'pipe']
})
child = spawned
supervised = spec.supervised
// The SDK drains stderr only for its own local spawn, so a custom spawner must:
// otherwise the child blocks on a full pipe and exit errors lose their tail.
spawned.stderr.setEncoding('utf8').on('data', (chunk: string) => {
Expand All @@ -62,6 +85,9 @@ export function createClaudeCodeProcessSpawn(
get pid() {
return child?.pid
},
get supervised() {
return supervised
},
get stderrTail() {
return stderrTail
}
Expand Down
72 changes: 72 additions & 0 deletions src/main/claude/claude-child-exit-proof-ladder.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import { describe, expect, it, vi } from 'vitest'
import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from '../codex/codex-app-server-posix-supervisor'
import type { ClaudeChildTreeReaper } from './claude-agent-sdk-exit-proof'
import { proveClaudeChildExitWithReaper } from './claude-child-exit-proof-ladder'

function fakeTree(): ClaudeChildTreeReaper & { reap: ReturnType<typeof vi.fn> } {
return {
capture: vi.fn(async () => {}),
refresh: vi.fn(async () => {}),
reap: vi.fn(async () => 'exited' as const),
treeVerdict: 'exited'
}
}

/** A root that leaves only once a SIGTERM has had `stopMs` to act, the way a supervisor does. */
function rootStoppedBySigterm(stopMs: number) {
let exited = false
let settle = (): void => {}
const exitPromise = new Promise<void>((resolve) => {
settle = resolve
})
const kill = vi.fn((signal?: NodeJS.Signals | number) => {
if (signal === 'SIGTERM') {
setTimeout(() => {
exited = true
settle()
}, stopMs)
}
return true
})
const stdin = { end: vi.fn() }
return {
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The ladder reads only pid, kill and stdin.end from its child.
child: { pid: 4321, kill, stdin } as unknown as Parameters<
typeof proveClaudeChildExitWithReaper
>[0]['child'],
kill,
stdin,
exitPromise,
exited: () => exited
}
}

describe('Claude child exit proof ladder', () => {
it('stops a supervised child with SIGTERM and waits out the supervisor stop before forcing', async () => {
// Slower than the unsupervised 1.5 s grace, still inside the supervisor's own bound.
const root = rootStoppedBySigterm(PROVIDER_SUPERVISOR_MAX_STOP_MS - 500)
const tree = fakeTree()

await expect(
proveClaudeChildExitWithReaper({ ...root, supervised: true, tree }, () => tree)
).resolves.toBe(true)

expect(root.stdin.end).toHaveBeenCalled()
expect(root.kill).toHaveBeenCalledWith('SIGTERM')
// Forcing here would SIGKILL the supervisor mid-stop and orphan Claude in its own group.
expect(root.kill).not.toHaveBeenCalledWith('SIGKILL')
expect(tree.reap).not.toHaveBeenCalled()
}, 10_000)

it('never signals an unsupervised child for the graceful stop', async () => {
const root = rootStoppedBySigterm(0)
const tree = fakeTree()

await proveClaudeChildExitWithReaper({ ...root, tree }, () => tree)

// On Windows a direct SIGTERM is TerminateProcess: stdin end stays the only graceful rung.
expect(root.stdin.end).toHaveBeenCalled()
expect(root.kill).not.toHaveBeenCalledWith('SIGTERM')
expect(tree.reap).toHaveBeenCalled()
}, 10_000)
})
19 changes: 16 additions & 3 deletions src/main/claude/claude-child-exit-proof-ladder.ts
Original file line number Diff line number Diff line change
@@ -1,32 +1,45 @@
import type { SpawnedProcess } from '../../shared/child-process/run-process'
import { waitForProcessExitUntil } from '../codex/codex-process-exit-deadline'
import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from '../codex/codex-app-server-posix-supervisor'
import type { ClaudeChildTreeReaper } from './claude-agent-sdk-exit-proof'

const GRACEFUL_EXIT_MS = 1_500
export const GRACEFUL_EXIT_MS = 1_500
// A signalled supervisor escalates on its own; forcing it sooner kills it and orphans Claude.
export const SUPERVISED_GRACEFUL_EXIT_MS = PROVIDER_SUPERVISOR_MAX_STOP_MS + 500
const FORCED_EXIT_MS = 1_000

export type ClaudeChildExitProofInput = {
child: Pick<SpawnedProcess, 'pid' | 'kill' | 'stdin'>
exitPromise: Promise<void>
exited: () => boolean
tree?: ClaudeChildTreeReaper
/** The child is the POSIX provider supervisor: SIGTERM stops Claude, which reaps its tools. */
supervised?: boolean
}

export async function proveClaudeChildExitWithReaper(
input: ClaudeChildExitProofInput,
createTree: () => ClaudeChildTreeReaper
): Promise<boolean> {
const tree = input.tree ?? createTree()
// Arm before stdin closes: only a live root can identify its descendants.
// Arm before the stop: only a live root can identify its descendants.
await tree.capture()
try {
input.child.stdin?.end()
} catch {
// The reap below still owns the process.
}
// Stdin end alone lets Claude finish its turn, tools and edits included; a close is a stop.
// Windows has no supervisor, and a direct SIGTERM there is TerminateProcess.
if (input.supervised && !input.exited()) {
input.child.kill('SIGTERM')
}
let reaped = false
if (!input.exited()) {
await waitForProcessExitUntil(input.exitPromise, GRACEFUL_EXIT_MS)
await waitForProcessExitUntil(
input.exitPromise,
input.supervised ? SUPERVISED_GRACEFUL_EXIT_MS : GRACEFUL_EXIT_MS
)
if (!input.exited()) {
reaped = true
await tree.refresh?.()
Expand Down
3 changes: 3 additions & 0 deletions src/main/claude/claude-child-root-termination.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ type RootTerminationInput = {
* nothing. A probe here could only let an unreadable process table cost the tree
* the one fallback that still works once every table read has failed.
*
* On POSIX the root is the provider supervisor, killed only after its own stop had
* its whole bound; Claude, in its own group, is reached by the descendant kill.
*
* False means no signal was sent, because the root had already left.
*/
export function terminateClaudeRoot(input: RootTerminationInput): boolean {
Expand Down
Loading
Loading