fix(shell): bound collect wait and fail closed without collect (#1100)

* fix(shell): bound collect wait and fail closed without collect

A grandchild holding stdout parked collect after the child had
already exited, and intern persist could leak background children
because shell_collect is not mounted. Collect now settles on
process exit with a short stdio drain, wait_ms is capped, and
surfaces without collect hide background and unwire the registry.

* test(shell): catch intern reap leaks without pinning prompt copy

Author: TheGreatAxios

docs/ARCHITECTURE.md

@@ -396,7 +396,7 @@ tool call
 - **Secret Guard** (`secret-guard-plugin.ts`) — Hard-denies path-keyed tool calls (`read_file`, `write_file`, …) that would put a sensitive file into (or write it from) the model context. Runs before the permission plugin, so the path-arg deny holds even under `--dangerously-skip-permissions`. Shell commands that _reference_ a sensitive path (tokenized so `cat .env`, `bun --env-file=.env run …`, and quote/env-assignment forms are detected) are not hard-denied here: they require operator approval via the permission gate, and auto mode forces an ask through the auto-shell policy (`sensitive-path` rule). Once the operator approves, the command runs. Shell detection is best-effort: token matching defeats quoting and env-assignment/redirection forms but not dynamic path construction (variable indirection, `printf` assembly). Tool-result secret scrub still redacts credential-shaped output.
 - **Authorization** (`run-shell-authz.ts`, enforced by the permission gate) — Denies catastrophic shell command patterns by regex, and hard-blocks shell `find`, head-position `rg`, and recursive `grep -r` (they can walk huge trees and OOM the host). Bounded `grep`/`search_files` tools remain practical alternatives (timeout + output caps); the patterns match those three command shapes only — an `ls -R`, `fd`, or scripted `os.walk` is just as unbounded and is not caught, so the block message tells the model not to substitute one. The gate hard-denies these at the top of its verdict path — before auto-allow, prompting, grants, and skipPermissions — so no mode or stored grant can admit them.
 - **Permission** (`permission-plugin.ts`) — Delegates consequential calls to the permission gate.
-- **Shell Guard** (`shell-guard-plugin.ts`) — Corbits Code-only replacement for stock `run_shell` (interchange stays unpatched): 120s foreground default (`settings.shell.timeoutMs` overrides that default only; a positive per-call `timeout` is the bound with no ceiling — `maxTimeoutMs` does not clamp it), background `run_shell` has no default (only a per-call timeout arms a timer), 512KB display cap with head+tail retention (the process keeps running when the cap is hit), process-group kill on timeout, abort, and plugin dispose (live children tracked in the plugin and reaped by `posixTools.dispose`), and `background: true` — the call returns a `shell_id` at once (registry in `src/shell/background-shell.ts`), the process group keeps running past the turn, completion is delivered on a later turn via `buildShellBackgroundMessage`, and `shell_collect` retrieves or cancels (schema advertised by `advertiseShellGuardTimeout`; evaluated by the permission chain at start time like any shell call). Foreground expiry is exit 124 + `timedOut:true` plus a nudge to retry with `background:true`. Also applies a 10s wall-clock budget to `grep`/`search_files`. Ripgrep detached spawns are not tracked.
+- **Shell Guard** (`shell-guard-plugin.ts`) — Corbits Code-only replacement for stock `run_shell` (interchange stays unpatched): 120s foreground default (`settings.shell.timeoutMs` overrides that default only; a positive per-call `timeout` is the bound with no ceiling — `maxTimeoutMs` does not clamp it), background `run_shell` has no default (only a per-call timeout arms a timer), 512KB display cap with head+tail retention (the process keeps running when the cap is hit), process-group kill on timeout, abort, and plugin dispose (live children tracked in the plugin and reaped by `posixTools.dispose`), and `background: true` — the call returns a `shell_id` at once (registry in `src/shell/background-shell.ts`), the process group keeps running past the turn, completion is process-exit (stdio-close is not required), delivered on a later turn via `buildShellBackgroundMessage`, and `shell_collect` retrieves or cancels with a 300s wait cap (schema advertised by `advertiseShellGuardTimeout` only when `shell_collect` is mounted; evaluated by the permission chain at start time like any shell call). Foreground expiry is exit 124 + `timedOut:true` plus a nudge to retry with `background:true`. Also applies a 10s wall-clock budget to `grep`/`search_files`. Ripgrep detached spawns are not tracked.
 - **Read File Guard** (`read-file-guard-plugin.ts`) — Corbits Code-only short-circuit for `read_file` on real filesystem paths and configured `tool-output://` URIs (interchange stays unpatched): streaming reads that never decode the whole file in one pass, caps model-facing output at 50KB, defaults to 2000 lines, truncates long lines with recovery hints, samples the first chunk to reject binary, and stops at an 8MB scan ceiling. Emits `offset` continuation notices so the model can page without losing file or spill content on disk.
 - **Verify** (`verify-plugin.ts`) — Re-reads after `write_file` / `edit_file` and errors on mismatch. Per-path serialization (`file-mutation-lock.ts`) prevents parallel edits on one file from tripping verification.
 - **Edit file line range** (`edit-file-line-range-plugin.ts`) — Corbits Code-only short-circuit for `edit_file` mode B (`start_line`/`end_line`/`new_string`), same pattern as shell-guard; schema advertised via `advertiseEditFileLineRange`. Modes are mutually exclusive: a call supplying both `old_string` and `start_line`/`end_line` is rejected with a recoverable error naming which fields to omit (no file-content disambiguation).

docs/IMPLEMENTATION.md

@@ -208,11 +208,11 @@ Listings are list-free, dumps are dump-locked: a bounded `ls`/`tree` prints name
 
 #### Background shell mode
 
-`run_shell` accepts `background: true` (shell-guard plugin, after the permission chain — a denied command spawns nothing). The starting call resolves `cwd` as usual and applies a per-call `timeout` only when passed (no 120s default on background), skips the pwd probe, spawns a detached process group via the registry in `src/shell/background-shell.ts`, and returns `{shell_id, status: "running"}` immediately; the retained shell cwd is never mutated by a background run. Limits: 8 running, 8 completed entries (ring; evicted ids collect as not-found — truncated output is spilled to a `tool-output:///bg-shell-<id>` blob named in the completion message). On process exit the host delivers `buildShellBackgroundMessage(exit)` (exit status, timed-out marker, ~2KB output preview, spill URI) through the same continuation channel as compaction — wired in all three loop hosts (TUI, exec, sub-agent). `shell_collect` (`{shell_id, action: "collect"|"cancel", wait_ms?}`, default non-blocking) retrieves status/output or kills the process group; it is ungated by design (cancel only kills the session's own child). Timeout keeps its meaning: expiry kills the group and reports exit code 124 with `timed_out: true`. The tool watchdog exempts background starts and `shell_collect` (same list as `spawn_agent`/`wait_agents`/`ask_director`). Toolset dispose calls `disposeAll("session closed")` before the posix teardown, so `/clear`, interrupt, and reload kill every live background process group.
+`run_shell` accepts `background: true` (shell-guard plugin, after the permission chain — a denied command spawns nothing). The starting call resolves `cwd` as usual and applies a per-call `timeout` only when passed (no 120s default on background), skips the pwd probe, spawns a detached process group via the registry in `src/shell/background-shell.ts`, and returns `{shell_id, status: "running"}` immediately; the retained shell cwd is never mutated by a background run. Limits: 8 running, 8 completed entries (ring; evicted ids collect as not-found — truncated output is spilled to a `tool-output:///bg-shell-<id>` blob named in the completion message). Completion is process-exit, not stdio-close: a grandchild holding the pipe does not park `shell_collect`. On process exit the host delivers `buildShellBackgroundMessage(exit)` (exit status, timed-out marker, ~2KB output preview, spill URI) through the same continuation channel as compaction — wired in all three loop hosts (TUI, exec, sub-agent). `shell_collect` (`{shell_id, action: "collect"|"cancel", wait_ms?}`, default non-blocking, `wait_ms` capped at 300s) retrieves status/output or kills the process group; it is ungated by design (cancel only kills the session's own child). Timeout keeps its meaning: expiry kills the group and reports exit code 124 with `timed_out: true`. The tool watchdog exempts background starts and `shell_collect` (same list as `spawn_agent`/`wait_agents`/`ask_director`) because collect's own wait cap bounds the poll. Surfaces that do not mount `shell_collect` (intern, migrator) do not advertise `background: true` and the registry is unwired — background starts fail closed. Parent abort, deadline, and `close_agent` release parked collect waiters and dispose live background groups; `interrupt_agent` only releases waiters so the session (and its children) stay resumable. Toolset dispose calls `disposeAll("session closed")` before the posix teardown, so `/clear`, interrupt, and reload kill every live background process group.
 
 - **Alt+Enter** queues a follow-up (kind `"queue"`) delivered only on **session-idle** — parent-idle **and** no live fleet lanes (`run` goes idle). Session-idle Alt+Enter is a no-op. **Ctrl+C** stops the run.
 
-Idle-with-fleet is shipped: after a non-blocking `spawn_agent` dispatch the parent turn can settle while workers keep running. The runner emits a `fleet` event carrying the live-lane count; the bridge holds the run busy on that count, so mid-hold Enter upgrades to a new primary turn (sent immediately) instead of queueing a steer, follow-ups keep waiting for true session-idle, and any steer left pending at the hold's engagement delivers immediately — the parent it was steering has already stopped. While the hold is up and the parent is not processing, occupancy flushes mailbox mail (`driveMailboxMail` + `buildMailboxMailMessage`) on store subscribe and idle-with-fleet settle — one child done while siblings run is enough. Skip that shot when a fleet-dry open-task continuation is latched. Exec-primary `wait_agents` mounts with no yield predicate (`createWaitAgentsTool({ sessions, fleetRecords })`) and blocks to ready/timeout/abort; `shouldYieldWait` remains a supported-but-unwired tool option (no production mount passes it).
+Idle-with-fleet is shipped: after a non-blocking `spawn_agent` dispatch the parent turn can settle while workers keep running. The runner emits a `fleet` event carrying the live-lane count; the bridge holds the run busy on that count, so mid-hold Enter upgrades to a new primary turn (sent immediately) instead of queueing a steer, follow-ups keep waiting for true session-idle, and any steer left pending at the hold's engagement delivers immediately — the parent it was steering has already stopped. While the hold is up and the parent is not processing, occupancy flushes mailbox mail (`driveMailboxMail` + `buildMailboxMailMessage`) on store subscribe and idle-with-fleet settle — one child done while siblings run is enough. Skip that shot when a fleet-dry open-task continuation is latched. Exec-primary `wait_agents` mounts with no yield predicate (`createWaitAgentsTool({ sessions, fleetRecords })`) and blocks to ready/timeout/abort; parent interrupt/abort cancels that parked waiter via the tool AbortSignal (`timed_out`, workers stay running) rather than a second waiter table. `interrupt_agent` on a child unblocks wait as interrupted through the mailbox overlay. `shouldYieldWait` remains a supported-but-unwired tool option (no production mount passes it).
 
 `src/tui/stream-event-map.ts` maps reactor events onto the bridge's inbound events, and `src/tui/turn-state.ts` tracks the turn's status. `src/tui/turns-to-blocks.ts` hydrates a resumed session's stored turns into the same content blocks.
 

src/agent/background-shell-tool.ts

@@ -2,6 +2,7 @@ import { type } from "arktype";
 import type { ToolDefinition } from "@intx/types/runtime";
 import {
   createBackgroundShellRegistry,
+  MAX_SHELL_COLLECT_WAIT_MS,
   type BackgroundShellExit,
   type BackgroundShellRegistry,
 } from "../shell/background-shell.js";
@@ -41,7 +42,8 @@ export const shellCollectDefinition: ToolDefinition = {
       wait_ms: {
         type: "number",
         description:
-          'For action="collect": milliseconds to wait for completion before returning "running" (default 0, non-blocking).',
+          'For action="collect": milliseconds to wait for completion before returning "running" (default 0, non-blocking, capped at ' +
+          `${MAX_SHELL_COLLECT_WAIT_MS}).`,
       },
     },
     required: ["shell_id", "action"],

src/agent/directors/intern/package.test.ts

@@ -28,6 +28,12 @@ describe("internPackage", () => {
     expect(p).toMatch(/You do NOT:[\s\S]*Debug failures/);
   });
 
+  test("fail-closes without shell_collect rather than pinning prompt copy", () => {
+    const allow = internPackage.tools?.allow ?? [];
+    expect(allow).toContain("run_shell");
+    expect(allow).not.toContain("shell_collect");
+  });
+
   test("spawn.maySpawn is false", () => {
     expect(internPackage.spawn.maySpawn).toBe(false);
   });
@@ -40,6 +46,7 @@ describe("internPackage", () => {
     expect(allow).toContain("write_file");
     expect(allow).toContain("edit_file");
     expect(allow).toContain("delete_file");
+    expect(allow).not.toContain("shell_collect");
     for (const name of [
       "grep",
       "search_files",

src/agent/directors/intern/package.ts

@@ -60,6 +60,7 @@ You do NOT:
 - Do not overthink or get creative with the implementation
 - If you're given step-by-step instructions, follow them exactly as written
 - If instructions are ambiguous or unclear, STOP and report Blockers for the parent (Skywalker)
+- Background shells are forbidden (background: true starts are uncollectable without shell_collect, which is not mounted) — use foreground calls with timeouts only
 
 **When to STOP and Report Blockers**
 

src/agent/directors/migrator/package.test.ts

@@ -90,6 +90,7 @@ describe("migratorPackage", () => {
       "list_agents",
       "search_agents",
       "wait_agents",
+      "shell_collect",
     ] as const) {
       expect(allow).not.toContain(verb);
     }

src/agent/directors/tool-sets.test.ts

@@ -138,6 +138,7 @@ describe("REVIEW_TOOLS / INTERN_TOOLS", () => {
     expect(INTERN_TOOLS).toContain("run_shell");
     expect(INTERN_TOOLS).toContain("read_file");
     expect(INTERN_TOOLS).toContain("list_dir");
+    expect(INTERN_TOOLS as readonly string[]).not.toContain("shell_collect");
     for (const name of [
       "grep",
       "search_files",

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

@@ -610,6 +610,23 @@ describe("advertiseShellGuardTimeout", () => {
       /does not change the retained shell cwd/i,
     );
   });
+
+  test("omits background when collect is not mounted", () => {
+    const rewritten = advertiseShellGuardTimeout(
+      runShellDef(),
+      undefined,
+      undefined,
+      false,
+    );
+    const properties = rewritten.inputSchema["properties"] as Record<
+      string,
+      unknown
+    >;
+    expect(properties["background"]).toBeUndefined();
+    expect(timeoutSchema(rewritten)?.description).not.toMatch(
+      /background:true/,
+    );
+  });
 });
 
 describe("shellGuardPlugin", () => {

src/plugins/shell-guard-plugin.ts

@@ -89,12 +89,15 @@ export function formatShellTimeoutNotice(timeoutMs: number): string {
  * Schema `default` is the foreground omit path. Description says omit
  * timeout on background:true so the model does not copy 120000 onto
  * background calls (a copied value becomes a requested timeout and kills
- * the job).
+ * the job). Surfaces that do not mount shell_collect pass
+ * advertiseBackground=false so background is not advertised without a
+ * collect path.
  */
 export function advertiseShellGuardTimeout(
   definition: ToolDefinition,
   defaultMs?: number,
   maxMs?: number,
+  advertiseBackground = true,
 ): ToolDefinition {
   if (definition.name !== "run_shell") return definition;
   const schema = definition.inputSchema;
@@ -120,7 +123,9 @@ export function advertiseShellGuardTimeout(
   ) {
     nextProperties["timeout"] = {
       ...(timeout as Record<string, unknown>),
-      description: `Timeout in milliseconds (foreground default: ${advertisedDefault}; omit on background:true for no timeout)`,
+      description: advertiseBackground
+        ? `Timeout in milliseconds (foreground default: ${advertisedDefault}; omit on background:true for no timeout)`
+        : `Timeout in milliseconds (foreground default: ${advertisedDefault})`,
       default: advertisedDefault,
     };
   }
@@ -131,14 +136,18 @@ export function advertiseShellGuardTimeout(
         "Optional working directory for this call only (does not change the session shell cwd retained across calls)",
     };
   }
-  nextProperties["background"] = {
-    type: "boolean",
-    description:
-      "Set true to run without holding the turn open (prefer this for builds, test suites, and dev servers). " +
-      "Returns a shell_id immediately; the exit status and output are delivered when the process finishes. " +
-      "Use shell_collect to collect or cancel. Omit timeout for no timeout. " +
-      "Does not change the retained shell cwd.",
-  };
+  if (advertiseBackground) {
+    nextProperties["background"] = {
+      type: "boolean",
+      description:
+        "Set true to run without holding the turn open (prefer this for builds, test suites, and dev servers). " +
+        "Returns a shell_id immediately; the exit status and output are delivered when the process finishes. " +
+        "Use shell_collect to collect or cancel. Omit timeout for no timeout. " +
+        "Does not change the retained shell cwd.",
+    };
+  } else {
+    delete nextProperties["background"];
+  }
   return {
     ...definition,
     inputSchema: {