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.

Author: TheGreatAxios

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

@@ -4,8 +4,8 @@ import { join } from "node:path";
 import { tmpdir } from "node:os";
 import { realpathSync } from "node:fs";
 import type { ToolCall, ToolResult } from "@intx/types/runtime";
-
-import { spawnSync } from "node:child_process";
+import { EventEmitter } from "node:events";
+import { spawnSync, type ChildProcess } from "node:child_process";
 import { randomUUID } from "node:crypto";
 
 import { createBackgroundShellRegistry } from "../shell/background-shell.js";
@@ -14,6 +14,7 @@ import {
   MAX_SHELL_OUTPUT_BYTES,
   advertiseShellGuardTimeout,
   resolveShellTimeoutMs,
+  reapLiveChildren,
   runGuardedShell,
   shellGuardPlugin,
 } from "./shell-guard-plugin.js";
@@ -780,4 +781,15 @@ describe("shellGuardPlugin", () => {
       spawnSync("pkill", ["-9", "-f", token]);
     }
   });
+
+  test("dispose fails when a child survives the reap window", async () => {
+    const child = Object.assign(new EventEmitter(), {
+      exitCode: null,
+      signalCode: null,
+      kill: () => true,
+    }) as ChildProcess;
+    await expect(reapLiveChildren(new Set([child]))).rejects.toThrow(
+      /still live after 2000ms reap/,
+    );
+  }, 10_000);
 });

src/plugins/shell-guard-plugin.ts

@@ -217,17 +217,34 @@ function waitChildClose(child: ChildProcess): Promise<void> {
 
 const SHELL_GUARD_DISPOSE_REAP_MS = 2_000;
 
-async function reapLiveChildren(liveChildren: Set<ChildProcess>): Promise<void> {
+function childStillLive(child: ChildProcess): boolean {
+  return child.exitCode === null && child.signalCode === null;
+}
+
+// Abort SIGKILLs the process group immediately (runGuardedShell onAbort). This
+// window is only a backstop for children still tracked at dispose. Leftovers
+// after it must fail teardown; do not stretch the process-exit 2s deadline.
+export async function reapLiveChildren(liveChildren: Set<ChildProcess>): Promise<void> {
   const remaining = [...liveChildren];
   for (const child of remaining) killProcessTree(child);
   if (remaining.length === 0) return;
-  await Promise.race([
-    Promise.all(remaining.map(waitChildClose)),
-    new Promise<void>((resolve) => {
-      const timer = setTimeout(resolve, SHELL_GUARD_DISPOSE_REAP_MS);
-      if (typeof timer.unref === "function") timer.unref();
-    }),
-  ]);
+  const closed = Promise.all(remaining.map(waitChildClose));
+  let timer: ReturnType<typeof setTimeout> | undefined;
+  const timedOut = new Promise<"timeout">((resolve) => {
+    timer = setTimeout(() => resolve("timeout"), SHELL_GUARD_DISPOSE_REAP_MS);
+  });
+  try {
+    const winner = await Promise.race([closed.then(() => "closed" as const), timedOut]);
+    if (winner === "closed") return;
+    const stillLive = remaining.filter(childStillLive);
+    if (stillLive.length > 0) {
+      throw new Error(
+        `${stillLive.length} shell child process${stillLive.length === 1 ? "" : "es"} still live after ${SHELL_GUARD_DISPOSE_REAP_MS}ms reap`,
+      );
+    }
+  } finally {
+    if (timer !== undefined) clearTimeout(timer);
+  }
 }
 export async function runGuardedShell(
   args: RunShellArgs,