fix(claude): verify the Windows tree after taskkill instead of trusting that it ran

`terminateWindowsProcessTree` resolves from taskkill's callback whatever the
error says, so a timeout, an access denial, a recycled root and a surviving
descendant all looked identical to the reaper — which then returned a proven
exit unconditionally. close() reported true and the lease was released with an
MCP descendant potentially still live.

The Windows branch now snapshots the root's descendants while it is alive and,
after taskkill, polls a fresh process table to a bounded deadline: a row still
matching by pid AND creation time is `live`, an unreadable table is
`unverifiable`, and only a table with no match is `exited`. Creation time is
the PID-reuse guard the POSIX path gets from ps lstart, so a descendant that
denied a creation-time query is omitted rather than signalled on a bare pid.
A root already observed exited is never taskkilled: `/T /F` on a recycled pid
would take an unrelated tree down with it.

The captured tree is tagged by platform so neither verifier can be handed the
other's rows.

Claude-Session: https://claude.ai/code/session_01BSmXgkWSsNHft8jFkdBFG9
This commit is contained in:
Merge Sim
2026-09-02 01:42:52 -07:00
parent 84b7f63cc5
commit fe73cde773
4 changed files with 361 additions and 11 deletions
@@ -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<void>()
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()
+59 -10
View File
@@ -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<SpawnedProcess, 'pid' | 'kill'>
/** 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<DescendantSnapshot | null>
terminateDescendants?: (snapshot: DescendantSnapshot) => Promise<DescendantTreeVerdict>
terminateWindowsTree?: (rootPid: number) => Promise<void>
captureWindowsDescendants?: (rootPid: number) => Promise<WindowsDescendantSnapshot | null>
terminateWindowsDescendants?: (
snapshot: WindowsDescendantSnapshot
) => Promise<DescendantTreeVerdict>
}
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<void> | null = null
let inFlight: Promise<DescendantTreeVerdict> | 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
}
@@ -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')
})
})
@@ -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<void>
verifyMs?: number
}
function delay(ms: number): Promise<void> {
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<WindowsDescendantSnapshot | null> {
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<DescendantTreeVerdict> {
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
}