Skip to content

Commit 8e81fb9

Browse files
committed
Fail teardown when shell children survive reap
A leftover after the two-second backstop must reject dispose so the exit 1 path can fire. Abort already SIGKILLs the process group at abort start.
1 parent cc1873d commit 8e81fb9

2 files changed

Lines changed: 39 additions & 10 deletions

File tree

src/plugins/shell-guard-plugin.test.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,16 @@ import { join } from "node:path";
44
import { tmpdir } from "node:os";
55
import { realpathSync } from "node:fs";
66
import type { ToolCall, ToolResult } from "@intx/types/runtime";
7-
8-
import { spawnSync } from "node:child_process";
7+
import { EventEmitter } from "node:events";
8+
import { spawnSync, type ChildProcess } from "node:child_process";
99
import { randomUUID } from "node:crypto";
1010

1111
import {
1212
BoundedShellOutput,
1313
MAX_SHELL_OUTPUT_BYTES,
1414
advertiseShellGuardTimeout,
1515
resolveShellTimeoutMs,
16+
reapLiveChildren,
1617
runGuardedShell,
1718
shellGuardPlugin,
1819
} from "./shell-guard-plugin.js";
@@ -688,4 +689,15 @@ describe("shellGuardPlugin", () => {
688689
spawnSync("pkill", ["-9", "-f", token]);
689690
}
690691
});
692+
693+
test("dispose fails when a child survives the reap window", async () => {
694+
const child = Object.assign(new EventEmitter(), {
695+
exitCode: null,
696+
signalCode: null,
697+
kill: () => true,
698+
}) as ChildProcess;
699+
await expect(reapLiveChildren(new Set([child]))).rejects.toThrow(
700+
/still live after 2000ms reap/,
701+
);
702+
}, 10_000);
691703
});

src/plugins/shell-guard-plugin.ts

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -228,17 +228,34 @@ function waitChildClose(child: ChildProcess): Promise<void> {
228228

229229
const SHELL_GUARD_DISPOSE_REAP_MS = 2_000;
230230

231-
async function reapLiveChildren(liveChildren: Set<ChildProcess>): Promise<void> {
231+
function childStillLive(child: ChildProcess): boolean {
232+
return child.exitCode === null && child.signalCode === null;
233+
}
234+
235+
// Abort SIGKILLs the process group immediately (runGuardedShell onAbort). This
236+
// window is only a backstop for children still tracked at dispose. Leftovers
237+
// after it must fail teardown; do not stretch the process-exit 2s deadline.
238+
export async function reapLiveChildren(liveChildren: Set<ChildProcess>): Promise<void> {
232239
const remaining = [...liveChildren];
233240
for (const child of remaining) killProcessTree(child);
234241
if (remaining.length === 0) return;
235-
await Promise.race([
236-
Promise.all(remaining.map(waitChildClose)),
237-
new Promise<void>((resolve) => {
238-
const timer = setTimeout(resolve, SHELL_GUARD_DISPOSE_REAP_MS);
239-
if (typeof timer.unref === "function") timer.unref();
240-
}),
241-
]);
242+
const closed = Promise.all(remaining.map(waitChildClose));
243+
let timer: ReturnType<typeof setTimeout> | undefined;
244+
const timedOut = new Promise<"timeout">((resolve) => {
245+
timer = setTimeout(() => resolve("timeout"), SHELL_GUARD_DISPOSE_REAP_MS);
246+
});
247+
try {
248+
const winner = await Promise.race([closed.then(() => "closed" as const), timedOut]);
249+
if (winner === "closed") return;
250+
const stillLive = remaining.filter(childStillLive);
251+
if (stillLive.length > 0) {
252+
throw new Error(
253+
`${stillLive.length} shell child process${stillLive.length === 1 ? "" : "es"} still live after ${SHELL_GUARD_DISPOSE_REAP_MS}ms reap`,
254+
);
255+
}
256+
} finally {
257+
if (timer !== undefined) clearTimeout(timer);
258+
}
242259
}
243260

244261
export async function runGuardedShell(

0 commit comments

Comments
 (0)