Reap posix children before waiting on agent close

A hung agent.close used to run before process-group reap, so teardown
could report success while detached run_shell children were still live.
Dispose first, fail a close deadline instead of succeeding, and clear
the two-second host timer when dispose wins.

Author: TheGreatAxios

src/agent/tools.ts

@@ -951,6 +951,11 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
     mcpAbortController.abort(new Error("MCP toolset disposed"));
     disposal = (async () => {
       const failures: unknown[] = [];
+      try {
+        await posixTools.dispose();
+      } catch (err: unknown) {
+        failures.push(err);
+      }
       const fleetSessions = fleetSessionsForDispose;
       if (fleetSessions !== undefined) {
         try {
@@ -974,11 +979,6 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
         [...connectedClients.values()].map((client) => client.close().catch(() => undefined)),
       );
       connectedClients.clear();
-      try {
-        await posixTools.dispose();
-      } catch (err: unknown) {
-        failures.push(err);
-      }
       await disposeWebSearchClients();
       rethrowToolsetDisposeFailures(failures);
     })();

src/exec/runner.ts

@@ -126,11 +126,11 @@ export function execUserFailureMessage(
 }
 
 /**
- * Headless analogue of TUI `runtime-shutdown`: abort live workers, then close
- * the primary agent and dispose the toolset. `cancelAll` is awaited so a
- * leftover-child throw is visible; hang-forever close is still deadline-bounded.
- * Once-only per runtime object so the send path, `finally`, and signal host
- * cannot double-dispose.
+ * Headless analogue of TUI `runtime-shutdown`: dispose the toolset (posix
+ * process-group reap) before waiting on agent.close so a hung close cannot
+ * skip killing detached run_shell children. `cancelAll` is awaited so a
+ * leftover-child throw is visible. Once-only per runtime object so the send
+ * path, `finally`, and signal host cannot double-dispose.
  */
 const execDisposeInFlight = new WeakMap<object, Promise<void>>();
 
@@ -163,6 +163,16 @@ async function runExecDispose(args: {
   subAgentSessions: Pick<SubAgentSessionStore, "cancelAll"> | null;
 }): Promise<void> {
   const failures: unknown[] = [];
+  if (args.toolset !== null) {
+    try {
+      await args.toolset.dispose();
+    } catch (err: unknown) {
+      logger.debug("toolset.dispose during exec finally failed: {error}", {
+        error: formatCaughtError(err),
+      });
+      failures.push(err);
+    }
+  }
   try {
     await args.subAgentSessions?.cancelAll("Session closed");
   } catch (err) {
@@ -178,16 +188,6 @@ async function runExecDispose(args: {
       failures.push(err);
     }
   }
-  if (args.toolset !== null) {
-    try {
-      await args.toolset.dispose();
-    } catch (err: unknown) {
-      logger.debug("toolset.dispose during exec finally failed: {error}", {
-        error: formatCaughtError(err),
-      });
-      failures.push(err);
-    }
-  }
   rethrowExecDisposeFailures(failures);
 }
 

src/index.ts

@@ -123,11 +123,12 @@ export const RUNTIME_TEARDOWN_DEADLINE_MS = 2_000;
 async function awaitActiveDisposeHost(context: string): Promise<void> {
   const dispose = getActiveDisposeHost();
   if (dispose === null) return;
+  let timer: ReturnType<typeof setTimeout> | undefined;
   try {
     await Promise.race([
       Promise.resolve(dispose()),
       new Promise<never>((_, reject) => {
-        const timer = setTimeout(() => {
+        timer = setTimeout(() => {
           reject(new Error(`runtime teardown exceeded ${RUNTIME_TEARDOWN_DEADLINE_MS}ms`));
         }, RUNTIME_TEARDOWN_DEADLINE_MS);
         if (typeof timer.unref === "function") timer.unref();
@@ -137,6 +138,8 @@ async function awaitActiveDisposeHost(context: string): Promise<void> {
     process.stderr.write(
       `host dispose failed ${context}: ${disposeErr instanceof Error ? disposeErr.message : String(disposeErr)}\n`,
     );
+  } finally {
+    if (timer !== undefined) clearTimeout(timer);
   }
 }
 

src/subagent/dispose.ts

@@ -45,9 +45,39 @@ export const DEFAULT_CLOSE_DEADLINE_MS = 30_000;
  * tracked in a global registry.
  */
 export const SUBAGENT_PLUGIN_SPAWN_TEARDOWN_LIMITS =
-  "Per sub-agent session Corbits Code runs agent.close(), drains in-flight tool middleware (best-effort), then posixTools.dispose() (LSP and plugin dispose callbacks). " +
+  "Per sub-agent session Corbits Code runs posixTools.dispose() (LSP and plugin dispose callbacks, including in-flight tool drain), then agent.close() and stream drain. " +
   "run_shell children are tracked in the shell-guard plugin and killed on posixTools.dispose; ripgrep detached spawns are not tracked in a global registry.";
 
+/** Fail a hung close instead of resolving as successful teardown. */
+export async function awaitBoundedTeardown(
+  teardown: Promise<void>,
+  deadlineMs: number,
+): Promise<void> {
+  let teardownError: unknown;
+  let timedOut = false;
+  let timer: ReturnType<typeof setTimeout> | undefined;
+  try {
+    await Promise.race([
+      teardown.then(
+        () => undefined,
+        (err: unknown) => {
+          teardownError = err;
+        },
+      ),
+      new Promise<void>((resolve) => {
+        timer = setTimeout(() => {
+          timedOut = true;
+          resolve();
+        }, deadlineMs);
+      }),
+    ]);
+  } finally {
+    if (timer !== undefined) clearTimeout(timer);
+  }
+  if (teardownError !== undefined) throw teardownError;
+  if (timedOut) throw new Error(`session close exceeded ${deadlineMs}ms`);
+}
+
 export interface SubAgentSpawnSnapshot {
   inFlightToolCalls: number;
   inFlightByTool: Readonly<Record<string, number>>;
@@ -110,6 +140,12 @@ export async function disposeSubAgentSession(input: SubAgentSessionDisposeInput)
   if (input.signal !== undefined && input.closeOnAbort !== undefined) {
     input.signal.removeEventListener("abort", input.closeOnAbort);
   }
+  let posixError: unknown;
+  try {
+    await input.posixTools.dispose();
+  } catch (err: unknown) {
+    posixError = err;
+  }
   try {
     await input.agent?.close();
   } catch {
@@ -120,5 +156,5 @@ export async function disposeSubAgentSession(input: SubAgentSessionDisposeInput)
   } catch {
     // ignore
   }
-  await input.posixTools.dispose();
+  if (posixError !== undefined) throw posixError;
 }

src/subagent/index.test.ts

@@ -72,6 +72,33 @@ describe("sub-agent teardown", () => {
     expect(disposeCount).toBe(2);
   });
 
+  test("disposeSubAgentSession reaps posix tools before waiting on agent.close", async () => {
+    const order: string[] = [];
+    let releaseClose!: () => void;
+    const closeGate = new Promise<void>((resolve) => {
+      releaseClose = resolve;
+    });
+    const pending = disposeSubAgentSession({
+      agent: {
+        close: async () => {
+          order.push("close-start");
+          await closeGate;
+          order.push("close-end");
+        },
+      },
+      posixTools: {
+        dispose: async () => {
+          order.push("posix");
+        },
+      },
+    });
+    await new Promise((resolve) => setTimeout(resolve, 20));
+    expect(order).toEqual(["posix", "close-start"]);
+    releaseClose();
+    await pending;
+    expect(order).toEqual(["posix", "close-start", "close-end"]);
+  });
+
   test("disposeSubAgentSession does not treat a throwing posix dispose as success", async () => {
     const posixTools = {
       dispose: async () => {

src/subagent/lifecycle-tools.test.ts

@@ -86,9 +86,10 @@ describe("close_agent", () => {
     // Exercise the store directly with a short deadline (the tool itself
     // uses the real ~30s bound, which would make this test slow).
     const started = Date.now();
-    const childStatus = await sessions.closeOne(wedgedChild.id, 25);
+    await expect(sessions.closeOne(wedgedChild.id, 25)).rejects.toThrow(
+      /session close exceeded 25ms/,
+    );
     expect(Date.now() - started).toBeLessThan(500);
-    expect(childStatus).toBe("shutdown");
   });
 
   test("closes remaining siblings after a leftover-child throw, then fails", async () => {

src/subagent/lifecycle-tools.ts

@@ -41,8 +41,8 @@ export const closeAgentToolDefinition: ToolDefinition = {
   description:
     "Permanently close a worker session by agent_id, closing its descendants first. Bounded " +
     `by a ~${Math.round(DEFAULT_CLOSE_DEADLINE_MS / 1000)}s cleanup deadline per session so a wedged worker cannot hang ` +
-    "this call — a session that misses the deadline is still marked shutdown; its teardown just " +
-    "keeps running in the background. Unblocks any in-flight wait_agents on these ids immediately with " +
+    "this call — a session that misses the deadline is still marked shutdown and the call fails " +
+    "instead of reporting success while children may still be live. Unblocks any in-flight wait_agents on these ids immediately with " +
     "status 'interrupted'. Closing is permanent: a closed session cannot be resumed.",
   inputSchema: {
     type: "object",

src/subagent/run-persist-close.test.ts

@@ -94,4 +94,59 @@ describe("persist close_agent leftover dispose", () => {
         ),
     );
   });
+
+  test("onAgentReady close reaps posix tools before a hung agent.close and fails the deadline", async () => {
+    const cwd = await mkdtemp(join(tmpdir(), "corbits-persist-close-hung-"));
+    let posixDisposed = false;
+
+    await withMockedModuleDuring(
+      import.meta.resolve("@intx/tools-posix"),
+      (real: typeof import("@intx/tools-posix")) => ({
+        ...real,
+        createPosixTools: (opts: Parameters<typeof real.createPosixTools>[0]) =>
+          Object.assign(real.createPosixTools(opts), {
+            dispose: async () => {
+              posixDisposed = true;
+            },
+          }),
+      }),
+      async () =>
+        withMockedModuleDuring(
+          import.meta.resolve("../agent/live-tool-dispatch.js"),
+          (real: typeof import("../agent/live-tool-dispatch.js")) => ({
+            ...real,
+            createAgentWithLiveToolDispatch: async () =>
+              ({
+                ...stubAgent(),
+                close: () => new Promise<void>(() => {}),
+              }) as unknown as Awaited<ReturnType<typeof real.createAgentWithLiveToolDispatch>>,
+          }),
+          async () => {
+            const { runSubAgent } = await import("./run.js");
+            let handles:
+              | {
+                  close: (deadlineMs?: number) => Promise<void>;
+                }
+              | undefined;
+            const params: RunSubAgentParams = {
+              cwd,
+              workdirBase: join(cwd, ".ctx"),
+              permissionGate,
+              provider: { providerName: "test", baseURL: "http://localhost", model: "test-model" },
+              description: "persist close hung close probe",
+              prompt: "finish the first turn",
+              persist: true,
+              onAgentReady: (h) => {
+                handles = h;
+              },
+            };
+            const result = await runSubAgent(params);
+            expect(result.agentRetained).toBe(true);
+            if (handles === undefined) throw new Error("onAgentReady never fired");
+            await expect(handles.close(50)).rejects.toThrow(/session close exceeded 50ms/);
+            expect(posixDisposed).toBe(true);
+          },
+        ),
+    );
+  });
 });

src/subagent/run.ts

@@ -122,6 +122,7 @@ import {
   disposeSubAgentSession,
   isSubAgentCancelError,
   DEFAULT_CLOSE_DEADLINE_MS,
+  awaitBoundedTeardown,
 } from "./dispose.js";
 import {
   createFleetMailbox,
@@ -1089,38 +1090,33 @@ async function runSubAgentInner(
     // Hand the caller a bounded, idempotent close it can call at any time
     // (close_agent) — independent of whether this run ends up retained.
     // Aborting first stops a still-running turn before tearing down; on an
-    // already-finished turn the abort is a no-op. The timeout races teardown
-    // itself so a wedged descendant cannot hang the caller — see dispose.ts
-    // for the close()-ordering issue that can stall it.
+    // already-finished turn the abort is a no-op. posix dispose/reap runs
+    // before waiting on agent.close so a wedged close cannot skip killing
+    // detached run_shell children. The deadline abandons a hung close and
+    // fails rather than reporting success while children may still be live.
     if (params.onAgentReady !== undefined) {
       const boundedClose = async (deadlineMs = DEFAULT_CLOSE_DEADLINE_MS): Promise<void> => {
         if (!runController.signal.aborted) runController.abort(new Error("closed by close_agent"));
-        let disposeError: unknown;
-        const teardown = disposeSubAgentSession({
-          signal: runController.signal,
-          ...(closeOnAbort !== undefined ? { closeOnAbort } : {}),
-          agent,
-          ...(streamPromise !== undefined ? { streamPromise } : {}),
-          posixTools,
-        }).then(
-          () => undefined,
-          (err: unknown) => {
-            disposeError = err;
-          },
-        );
-        await Promise.race([
-          teardown,
-          new Promise<void>((resolve) => setTimeout(resolve, deadlineMs)),
-        ]);
-        // The finally block kept the parent-abort forwarding listener alive
-        // for a persisted session (see runController.dispose's doc); now that
-        // this session is actually closing, tear it down for real.
-        runController.dispose();
-        if (disposeError !== undefined) throw disposeError;
+        try {
+          await awaitBoundedTeardown(
+            disposeSubAgentSession({
+              signal: runController.signal,
+              ...(closeOnAbort !== undefined ? { closeOnAbort } : {}),
+              agent,
+              ...(streamPromise !== undefined ? { streamPromise } : {}),
+              posixTools,
+            }),
+            deadlineMs,
+          );
+        } finally {
+          // The finally block kept the parent-abort forwarding listener alive
+          // for a persisted session (see runController.dispose's doc); now that
+          // this session is actually closing, tear it down for real.
+          runController.dispose();
+        }
       };
       // Interrupt only fires interruptController — never runController/
-      // close, so it cannot hit the close()-ordering wedge documented in
-      // dispose.ts.
+      // close, so it cannot hang teardown on a wedged agent.close.
       const interrupt = (): void => {
         if (!interruptController.signal.aborted) {
           interruptController.abort(new Error("interrupted by interrupt_agent"));

src/subagent/session-store.test.ts

@@ -569,15 +569,14 @@ describe("CL-6943 reusable worker sessions", () => {
     expect(cancelThrew === false && closeThrew === false && closeStatus === "shutdown").toBe(false);
   });
 
-  test("closeOne is bounded by its deadline when the registered close hangs forever", async () => {
+  test("closeOne fails a hung close instead of reporting shutdown success", async () => {
     const store = createSubAgentSessionStore();
     const session = store.start({ description: "d", agentId: "a", brief: "b", retained: true });
     store.registerClose(session.id, () => new Promise<void>(() => {})); // never resolves
 
     const started = Date.now();
-    const status = await store.closeOne(session.id, 25);
+    await expect(store.closeOne(session.id, 25)).rejects.toThrow(/session close exceeded 25ms/);
     expect(Date.now() - started).toBeLessThan(500);
-    expect(status).toBe("shutdown");
     expect(store.get(session.id)?.lifecycleStatus).toBe("shutdown");
     expect(store.get(session.id)?.retained).toBe(false);
   });