diff --git a/src/main/claude/claude-agent-sdk-exit-proof.test.ts b/src/main/claude/claude-agent-sdk-exit-proof.test.ts index 60a3ac7528f..2e29301a07a 100644 --- a/src/main/claude/claude-agent-sdk-exit-proof.test.ts +++ b/src/main/claude/claude-agent-sdk-exit-proof.test.ts @@ -5,6 +5,7 @@ import { describe, expect, it, vi } from 'vitest' import { spawnProcess, type SpawnedProcess } from '../../shared/child-process/run-process' 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, proveClaudeChildExit, @@ -133,6 +134,13 @@ function mockTree(verdicts: DescendantTreeVerdict[]): ClaudeChildTreeReaper & { } } +function windowsSnapshotOf(descendantPid: number): WindowsDescendantSnapshot { + return { + descendants: [{ pid: descendantPid, creationTimeMs: 1_700_000_000_000 }], + capturedAtMs: 1 + } +} + function snapshotOf(descendantPid: number): DescendantSnapshot { return { rootPgid: 1, @@ -458,21 +466,95 @@ describe('claude child tree reaper', () => { const release = Promise.withResolvers() const terminateWindowsTree = vi.fn(() => release.promise) const captureDescendants = vi.fn() + const terminateWindowsDescendants = vi.fn(async () => 'exited' as const) const tree = createClaudeChildTreeReaper(child, { platform: 'win32', captureDescendants, - terminateWindowsTree + captureWindowsDescendants: vi.fn(async () => windowsSnapshotOf(4243)), + terminateWindowsTree, + terminateWindowsDescendants }) const reap = tree.reap() await vi.waitFor(() => expect(terminateWindowsTree).toHaveBeenCalledWith(424242)) expect(child.kill).not.toHaveBeenCalled() + expect(terminateWindowsDescendants).not.toHaveBeenCalled() release.resolve() await expect(reap).resolves.toBe('exited') expect(child.kill).toHaveBeenCalledWith('SIGKILL') + expect(terminateWindowsDescendants).toHaveBeenCalledWith(windowsSnapshotOf(4243)) expect(captureDescendants).not.toHaveBeenCalled() }) + it('stays unproven on Windows when taskkill fails and a descendant is still observed', async () => { + const child = mockChild() + const tree = createClaudeChildTreeReaper(child, { + platform: 'win32', + captureWindowsDescendants: vi.fn(async () => windowsSnapshotOf(4243)), + terminateWindowsTree: vi.fn(async () => { + throw new Error('taskkill: access denied') + }), + terminateWindowsDescendants: vi.fn(async () => 'live' as const) + }) + + // taskkill's own outcome is not the proof; the table read after it is. + await expect(tree.reap()).resolves.toBe('live') + expect(tree.treeVerdict).toBe('live') + expect(child.kill).toHaveBeenCalledWith('SIGKILL') + }) + + it('stays unproven on Windows when taskkill resolves but a descendant survives it', async () => { + const child = mockChild() + const terminateWindowsTree = vi.fn(async () => {}) + const tree = createClaudeChildTreeReaper(child, { + platform: 'win32', + captureWindowsDescendants: vi.fn(async () => windowsSnapshotOf(4243)), + terminateWindowsTree, + terminateWindowsDescendants: vi.fn(async () => 'live' as const) + }) + + await expect(tree.reap()).resolves.toBe('live') + expect(terminateWindowsTree).toHaveBeenCalledTimes(1) + expect(tree.treeVerdict).toBe('live') + }) + + it('never taskkills a Windows root that already exited, but still verifies its snapshot', async () => { + const child = mockChild() + let exited = false + const terminateWindowsTree = vi.fn(async () => {}) + const terminateWindowsDescendants = vi.fn(async () => 'exited' as const) + const tree = createClaudeChildTreeReaper(child, { + platform: 'win32', + exited: () => exited, + captureWindowsDescendants: vi.fn(async () => windowsSnapshotOf(4243)), + terminateWindowsTree, + terminateWindowsDescendants + }) + + await tree.capture() + exited = true + await expect(tree.reap()).resolves.toBe('exited') + // A dead root's pid may already belong to a stranger: taskkill /T /F on it + // would take down an unrelated tree. + expect(terminateWindowsTree).not.toHaveBeenCalled() + expect(terminateWindowsDescendants).toHaveBeenCalledWith(windowsSnapshotOf(4243)) + }) + + it('treats an unreadable Windows table as unproven', async () => { + const child = mockChild() + const terminateWindowsDescendants = vi.fn() + const tree = createClaudeChildTreeReaper(child, { + platform: 'win32', + captureWindowsDescendants: vi.fn(async () => null), + terminateWindowsTree: vi.fn(async () => {}), + terminateWindowsDescendants + }) + + await expect(tree.reap()).resolves.toBe('unverifiable') + expect(terminateWindowsDescendants).not.toHaveBeenCalled() + expect(child.kill).toHaveBeenCalledWith('SIGKILL') + }) + it('has nothing to reap for a child that never spawned', async () => { const child = mockChild(null) const captureDescendants = vi.fn() diff --git a/src/main/claude/claude-agent-sdk-exit-proof.ts b/src/main/claude/claude-agent-sdk-exit-proof.ts index e314d5067ac..2c0f84922c8 100644 --- a/src/main/claude/claude-agent-sdk-exit-proof.ts +++ b/src/main/claude/claude-agent-sdk-exit-proof.ts @@ -5,6 +5,11 @@ import { type DescendantTreeVerdict } from '../pty-descendant-exit-verification' import { captureDescendantSnapshot, type DescendantSnapshot } from '../pty-descendant-termination' +import { + captureWindowsDescendantSnapshot, + verifyWindowsDescendantSnapshotExit, + type WindowsDescendantSnapshot +} from '../windows-descendant-exit-verification' import { terminateWindowsProcessTree } from '../windows-process-tree-kill' const GRACEFUL_EXIT_MS = 1_500 @@ -12,6 +17,32 @@ const FORCED_EXIT_MS = 1_000 type ReapableChild = Pick +/** One platform's descendant tree, tagged so neither verifier can be handed the other's rows. */ +type CapturedTree = + | { platform: 'posix'; tree: DescendantSnapshot } + | { platform: 'win32'; tree: WindowsDescendantSnapshot } + +/** + * A walk is only admissible while the root it walked was alive. A POSIX walk + * that found no root says so with a null pgid; either platform's walk can also + * have raced the root's death. Both can only have missed descendants that + * already reparented away, so neither is evidence about the tree. + */ +function admissibleTree( + captured: DescendantSnapshot | WindowsDescendantSnapshot | null, + platform: NodeJS.Platform, + exited: boolean +): CapturedTree | null { + if (!captured || exited) { + return null + } + if (platform === 'win32') { + return { platform: 'win32', tree: captured as WindowsDescendantSnapshot } + } + const tree = captured as DescendantSnapshot + return tree.rootPgid === null ? null : { platform: 'posix', tree } +} + export type ClaudeChildTreeReaperDeps = { platform?: NodeJS.Platform /** Whether the root's exit has been observed; only a live root can be walked. */ @@ -19,6 +50,10 @@ export type ClaudeChildTreeReaperDeps = { captureDescendants?: (rootPid: number) => Promise terminateDescendants?: (snapshot: DescendantSnapshot) => Promise terminateWindowsTree?: (rootPid: number) => Promise + captureWindowsDescendants?: (rootPid: number) => Promise + terminateWindowsDescendants?: ( + snapshot: WindowsDescendantSnapshot + ) => Promise } export type ClaudeChildTreeReaper = { @@ -60,7 +95,7 @@ export function createClaudeChildTreeReaper( // Undefined until captured; null when no admissible snapshot exists — the root // was already gone, or the table could not be read while it was alive — which // no later read can make up for. - let snapshot: DescendantSnapshot | null | undefined + let snapshot: CapturedTree | null | undefined let capturing: Promise | null = null let inFlight: Promise | null = null let treeVerdict: DescendantTreeVerdict = 'unverifiable' @@ -73,15 +108,19 @@ export function createClaudeChildTreeReaper( return capturing } const rootPid = child.pid - if (!rootPid || platform === 'win32' || exited()) { + if (!rootPid || exited()) { return Promise.resolve() } - capturing = (deps.captureDescendants ?? captureDescendantSnapshot)(rootPid) + const capture = + platform === 'win32' + ? (deps.captureWindowsDescendants ?? captureWindowsDescendantSnapshot) + : (deps.captureDescendants ?? captureDescendantSnapshot) + capturing = capture(rootPid) .catch(() => null) .then((captured) => { // A walk that found no root, or that raced the root's death, can only // have missed descendants that already reparented away. - snapshot = captured && captured.rootPgid !== null && !exited() ? captured : null + snapshot = admissibleTree(captured, platform, exited()) }) .finally(() => { capturing = null @@ -96,18 +135,26 @@ export function createClaudeChildTreeReaper( // Never spawned, so the OS never created a tree to orphan. return 'exited' } + await captureOnce() if (platform === 'win32') { - await (deps.terminateWindowsTree ?? terminateWindowsProcessTree)(rootPid) + // 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. + if (!exited()) { + // A dead root's pid can already belong to a stranger, and `/T /F` would + // take that stranger's whole tree down with it. + await (deps.terminateWindowsTree ?? terminateWindowsProcessTree)(rootPid).catch(() => {}) + } // taskkill owns the tree; this preserves the direct-child fallback when it fails. child.kill('SIGKILL') - return 'exited' + return snapshot?.platform === 'win32' + ? (deps.terminateWindowsDescendants ?? verifyWindowsDescendantSnapshotExit)(snapshot.tree) + : 'unverifiable' } - await captureOnce() - if (!snapshot) { + if (snapshot?.platform !== 'posix') { child.kill('SIGKILL') return 'unverifiable' } - if (snapshot.descendants.length === 0) { + 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. child.kill('SIGKILL') @@ -121,7 +168,9 @@ export function createClaudeChildTreeReaper( // parent links are still real; 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. - const verdict = (deps.terminateDescendants ?? terminateDescendantSnapshotWithVerdict)(snapshot) + const verdict = (deps.terminateDescendants ?? terminateDescendantSnapshotWithVerdict)( + snapshot.tree + ) child.kill('SIGKILL') return verdict } diff --git a/src/main/windows-descendant-exit-verification.test.ts b/src/main/windows-descendant-exit-verification.test.ts new file mode 100644 index 00000000000..e4f8ce5a672 --- /dev/null +++ b/src/main/windows-descendant-exit-verification.test.ts @@ -0,0 +1,103 @@ +import { describe, expect, it, vi } from 'vitest' +import { + captureWindowsDescendantSnapshot, + verifyWindowsDescendantSnapshotExit, + type WindowsDescendantSnapshot +} from './windows-descendant-exit-verification' + +function snapshot( + descendants: { pid: number; creationTimeMs: number }[] +): WindowsDescendantSnapshot { + return { descendants, capturedAtMs: 1_700_000_000_000 } +} + +describe('captureWindowsDescendantSnapshot', () => { + it('keeps only descendants the table can re-identify by creation time', async () => { + const captured = await captureWindowsDescendantSnapshot(100, { + readDescendants: vi.fn(async () => [{ pid: 200 }, { pid: 300 }]), + // 300 denied a creation-time query, so no later read could tell it from a + // recycled pid; signalling it would risk an unrelated process. + readTable: vi.fn(async () => [ + { pid: 100, creationTimeMs: 5 }, + { pid: 200, creationTimeMs: 7 }, + { pid: 300 } + ]), + now: () => 42 + }) + + expect(captured).toEqual({ descendants: [{ pid: 200, creationTimeMs: 7 }], capturedAtMs: 42 }) + }) + + it('reports an unreadable descendant walk as no snapshot rather than an empty one', async () => { + await expect( + captureWindowsDescendantSnapshot(100, { readDescendants: vi.fn(async () => null) }) + ).resolves.toBeNull() + await expect( + captureWindowsDescendantSnapshot(100, { + readDescendants: vi.fn(async () => [{ pid: 200 }]), + readTable: vi.fn(async () => { + throw new Error('table unavailable') + }) + }) + ).resolves.toBeNull() + }) + + it('refuses an invalid root pid', async () => { + const readDescendants = vi.fn() + await expect(captureWindowsDescendantSnapshot(0, { readDescendants })).resolves.toBeNull() + expect(readDescendants).not.toHaveBeenCalled() + }) +}) + +describe('verifyWindowsDescendantSnapshotExit', () => { + it('proves an empty tree without reading the table', async () => { + const readTable = vi.fn() + await expect(verifyWindowsDescendantSnapshotExit(snapshot([]), { readTable })).resolves.toBe( + 'exited' + ) + expect(readTable).not.toHaveBeenCalled() + }) + + it('reports exited once no identity-matched row remains', async () => { + const readTable = vi + .fn() + .mockResolvedValueOnce([{ pid: 200, creationTimeMs: 7 }]) + // The pid came back on a different process; that is a recycle, not a survivor. + .mockResolvedValueOnce([{ pid: 200, creationTimeMs: 99 }]) + + await expect( + verifyWindowsDescendantSnapshotExit(snapshot([{ pid: 200, creationTimeMs: 7 }]), { + readTable, + wait: async () => {}, + now: vi.fn().mockReturnValueOnce(0).mockReturnValue(1) + }) + ).resolves.toBe('exited') + expect(readTable).toHaveBeenCalledTimes(2) + }) + + it('reports live for a descendant still matched at the deadline', async () => { + let clock = 0 + await expect( + verifyWindowsDescendantSnapshotExit(snapshot([{ pid: 200, creationTimeMs: 7 }]), { + readTable: vi.fn(async () => [{ pid: 200, creationTimeMs: 7 }]), + wait: async () => { + clock += 100 + }, + now: () => clock, + verifyMs: 250 + }) + ).resolves.toBe('live') + }) + + it('reports unverifiable when the table cannot be read at the deadline', async () => { + await expect( + verifyWindowsDescendantSnapshotExit(snapshot([{ pid: 200, creationTimeMs: 7 }]), { + readTable: vi.fn(async () => { + throw new Error('table unavailable') + }), + wait: async () => {}, + now: vi.fn().mockReturnValueOnce(0).mockReturnValue(9_999) + }) + ).resolves.toBe('unverifiable') + }) +}) diff --git a/src/main/windows-descendant-exit-verification.ts b/src/main/windows-descendant-exit-verification.ts new file mode 100644 index 00000000000..16e31173267 --- /dev/null +++ b/src/main/windows-descendant-exit-verification.ts @@ -0,0 +1,116 @@ +import type { DescendantTreeVerdict } from './pty-descendant-exit-verification' +import { queryWindowsProcessDescendants } from './providers/windows-foreground-process-rows' +import { readWindowsProcessTableFresh } from './windows/windows-process-table' + +export const WINDOWS_DESCENDANT_KILL_VERIFY_MS = 3_500 +const WINDOWS_DESCENDANT_POLL_MS = 100 + +/** + * A Windows descendant tree captured while its root was alive, with the + * PID-reuse guard the POSIX snapshot gets from ps lstart: a row only counts as + * the same process when its creation time still matches. Rows without a + * creation time are omitted, because a bare pid cannot be re-identified. + */ +export type WindowsDescendantSnapshot = { + descendants: { pid: number; creationTimeMs: number }[] + capturedAtMs: number +} + +export type WindowsDescendantVerificationDeps = { + readDescendants?: (rootPid: number) => Promise<{ pid: number }[] | null> + readTable?: () => Promise<{ pid: number; creationTimeMs?: number }[]> + now?: () => number + wait?: (ms: number) => Promise + verifyMs?: number +} + +function delay(ms: number): Promise { + return new Promise((resolve) => { + const timer = setTimeout(resolve, ms) + timer.unref?.() + }) +} + +/** + * Snapshot a Windows root's descendants while it is still alive. Resolves null + * (never rejects) when the table is unreadable or the root is absent — the same + * contract as the POSIX walk, because "cannot see" is never "nothing is there". + */ +export async function captureWindowsDescendantSnapshot( + rootPid: number, + deps: WindowsDescendantVerificationDeps = {} +): Promise { + if (!Number.isInteger(rootPid) || rootPid <= 0) { + return null + } + const capturedAtMs = (deps.now ?? Date.now)() + const descendants = await ( + deps.readDescendants ?? ((pid: number) => queryWindowsProcessDescendants(pid, { fresh: true })) + )(rootPid).catch(() => null) + if (!descendants) { + return null + } + const rows = await readIdentifiedRows(descendants, deps) + return rows && { descendants: rows, capturedAtMs } +} + +async function readIdentifiedRows( + descendants: { pid: number }[], + deps: WindowsDescendantVerificationDeps +): Promise<{ pid: number; creationTimeMs: number }[] | null> { + const wanted = new Set(descendants.map((row) => row.pid)) + if (wanted.size === 0) { + return [] + } + const table = await (deps.readTable ?? readWindowsProcessTableFresh)().catch(() => null) + if (!table) { + return null + } + const identified: { pid: number; creationTimeMs: number }[] = [] + for (const row of table) { + if (wanted.has(row.pid) && typeof row.creationTimeMs === 'number') { + identified.push({ pid: row.pid, creationTimeMs: row.creationTimeMs }) + } + } + return identified +} + +/** + * Whether a snapshotted Windows tree is gone, polled to a bounded deadline. + * + * Why a verification pass at all: `taskkill /T /F` resolves the same way on a + * timeout, an access denial and a recycled root as it does on a successful + * kill, so its completion is never evidence. Only a table read that no longer + * shows an identity-matched row is. + */ +export async function verifyWindowsDescendantSnapshotExit( + snapshot: WindowsDescendantSnapshot, + deps: WindowsDescendantVerificationDeps = {} +): Promise { + if (snapshot.descendants.length === 0) { + return 'exited' + } + const now = deps.now ?? Date.now + const readTable = deps.readTable ?? readWindowsProcessTableFresh + const deadline = now() + (deps.verifyMs ?? WINDOWS_DESCENDANT_KILL_VERIFY_MS) + let verdict: DescendantTreeVerdict = 'unverifiable' + do { + const table = await readTable().catch(() => null) + if (!table) { + verdict = 'unverifiable' + } else { + const live = new Map(table.map((row) => [row.pid, row.creationTimeMs])) + verdict = snapshot.descendants.some((row) => live.get(row.pid) === row.creationTimeMs) + ? 'live' + : 'exited' + if (verdict === 'exited') { + return verdict + } + } + if (now() >= deadline) { + return verdict + } + await (deps.wait ?? delay)(WINDOWS_DESCENDANT_POLL_MS) + } while (now() < deadline) + return verdict +}