diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 1b1f8f494..2ad65ebbe 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -369,7 +369,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`, wired by `authz-plugin.ts`) — 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 permission gate’s shell auto-allow path consults the same policy so it never pre-approves a command authz would reject. - **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): no built-in default timeout (optional per-call or `settings.shell.timeoutMs`; `maxTimeoutMs` clamps only a resolved timeout), 512KB display cap with head+tail retention (the process keeps running when the cap is hit), process-group kill on timeout/abort only. Also applies a 10s wall-clock budget to `grep`/`search_files`. +- **Shell Guard** (`shell-guard-plugin.ts`) — Corbits Code-only replacement for stock `run_shell` (interchange stays unpatched): no built-in default timeout (optional per-call or `settings.shell.timeoutMs`; `maxTimeoutMs` clamps only a resolved timeout), 512KB display cap with head+tail retention (the process keeps running when the cap is hit), process-group kill on timeout/abort only, 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). Also applies a 10s wall-clock budget to `grep`/`search_files`. - **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). diff --git a/docs/IMPLEMENTATION.md b/docs/IMPLEMENTATION.md index 98e528db7..77e65f37b 100644 --- a/docs/IMPLEMENTATION.md +++ b/docs/IMPLEMENTATION.md @@ -118,6 +118,7 @@ src/ permission-plugin.ts Tiered operator approval shell/ run-shell-authz.ts Shared run_shell deny policy (authz + permission) + background-shell.ts Background run_shell registry (start/collect/cancel/disposeAll) verify-plugin.ts Write/edit verification (per-path lock) file-mutation-lock.ts Serialize mutations per file for verify lsp-hint-plugin.ts TS/JS LSP setup hint on unavailable server @@ -185,7 +186,12 @@ Unmatched shell auto-allows, including contained non-force `git worktree add`/`r `ChatInputProps` carries `isProcessing?: boolean` and `onInterrupt?: (message: string) => void`. When `isProcessing` is true, drain timing is **parent-idle** vs **session-idle**: -- **Enter** soft-steers while the parent is busy — enqueues kind `"steer"` and delivers at the next **parent** `tool.boundary` (the parent tool finishing, not a child). Does not interrupt. **Parent-idle** is when the primary Skywalker turn is not inside an in-flight parent tool; a long parent `run_shell` or awaiting `wait_agents` is parent-busy and holds steers. +- **Enter** soft-steers while the parent is busy — enqueues kind `"steer"` and delivers at the next **parent** `tool.boundary` (the parent tool finishing, not a child). Does not interrupt. **Parent-idle** is when the primary Skywalker turn is not inside an in-flight parent tool; a long parent **foreground** `run_shell` or awaiting `wait_agents` is parent-busy and holds steers. A `run_shell` started with `background: true` returns at once and releases the boundary; its completion is delivered as a system message (`buildShellBackgroundMessage`, mailbox `system`, no operator-originated flag — it re-enters the reactor without counting as operator input) on a later turn. + +#### 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`/`timeout` as usual, 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-` 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`). 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. @@ -250,7 +256,7 @@ Provider and model configuration lives in JSON settings files. The global file h } ``` - - `timeoutMs` / `maxTimeoutMs` — outer execution watchdog around each tool `run()`. Unset leaves the watchdog unarmed; set these to arm it. `maxTimeoutMs` clamps non-shell tools when set and does not cap a longer requested `run_shell`. Fleet wait tools are exempt: a dispatched sub-agent is bounded by stall, opt-in `deadlineMs`, and operator cancel, not the generic per-tool budget. + - `timeoutMs` / `maxTimeoutMs` — outer execution watchdog around each tool `run()`. Unset leaves the watchdog unarmed; set these to arm it. `maxTimeoutMs` clamps non-shell tools when set and does not cap a longer requested `run_shell`. Fleet wait tools are exempt: a dispatched sub-agent is bounded by stall, opt-in `deadlineMs`, and operator cancel, not the generic per-tool budget. Background shell is exempt too: a `run_shell` with `background: true` arms nothing (the process's own timeout bounds it) and `shell_collect` never arms (a bounded poll over a process that outlives the turn). - `waitForApproval` (default **true** when unset) — freeze that budget while a permission prompt is open so a late approve still runs the tool. **Settings → Tools** toggles this live for the next tool call and persists it here. When **false**, the budget keeps ticking during the prompt; on expiry the tool is skipped and the modal is auto-dismissed. The freeze is bounded: after **30 minutes** with the prompt still unanswered the budget resumes ticking on its own, so a prompt that never becomes visible (overlay open, UI gone) cannot hang a tool run indefinitely. Optional `mcp` block bounds MCP tool calls (`mcp__*` names) specifically — unlike `tools.*`, this arms **unconditionally** even with no settings at all, defaulting to **5 minutes**, since a wedged MCP server otherwise hangs a call forever with nothing to bound it (CL-6895): diff --git a/src/agent/background-shell-tool.test.ts b/src/agent/background-shell-tool.test.ts new file mode 100644 index 000000000..33125b151 --- /dev/null +++ b/src/agent/background-shell-tool.test.ts @@ -0,0 +1,138 @@ +import { describe, expect, test } from "bun:test"; +import { spawnSync } from "node:child_process"; +import { randomUUID } from "node:crypto"; +import { createPermissionGate } from "../permission/gate.js"; +import { createAgentToolset } from "./tools.js"; +import type { BackgroundShellExit } from "../shell/background-shell.js"; +import { buildShellBackgroundMessage } from "../session/runtime-assembly.js"; +import { OPERATOR_ORIGINATED_FLAG } from "../agent/message-provenance.js"; + +function gate(cwd: string) { + return createPermissionGate({ + approvals: [], + interactive: false, + skipPermissions: true, + reactorGated: false, + cwd, + }); +} + +describe("background shell through the agent toolset", () => { + test("run_shell background:true returns a handle and delivers the exit on exit", async () => { + const exits: BackgroundShellExit[] = []; + const toolset = await createAgentToolset({ + cwd: process.cwd(), + permissionGate: gate(process.cwd()), + onOperatorGate: async () => ({ kind: "cancel" as const }), + onBackgroundShellExit: (exit) => exits.push(exit), + }); + try { + const started = await toolset.dynamicRunner.run( + { + id: "bg-start", + name: "run_shell", + arguments: { command: "sleep 0.4; echo bg-done", background: true }, + }, + new AbortController().signal, + ); + expect(started.isError).not.toBe(true); + const parsed = JSON.parse(String(started.content)) as { shell_id: string }; + const snapshotNow = await toolset.dynamicRunner.run( + { + id: "bg-collect", + name: "shell_collect", + arguments: { shell_id: parsed.shell_id, action: "collect" }, + }, + new AbortController().signal, + ); + expect(JSON.parse(String(snapshotNow.content))).toMatchObject({ status: "running" }); + const final = await toolset.dynamicRunner.run( + { + id: "bg-collect2", + name: "shell_collect", + arguments: { shell_id: parsed.shell_id, action: "collect", wait_ms: 5_000 }, + }, + new AbortController().signal, + ); + const result = JSON.parse(String(final.content)) as { + status: string; + exit_code: number; + output: string; + }; + expect(result).toMatchObject({ status: "completed", exit_code: 0 }); + expect(result.output).toContain("bg-done"); + await new Promise((r) => setTimeout(r, 50)); + expect(exits).toHaveLength(1); + expect(exits[0]!.id).toBe(parsed.shell_id); + const message = buildShellBackgroundMessage(exits[0]!); + expect(message.headers.messageId).toBe(`bg-shell-${parsed.shell_id}@local`); + expect(message.ref.mailbox).toBe("system"); + expect(message.flags).not.toContain(OPERATOR_ORIGINATED_FLAG); + expect(message.content).toContain("exit code 0"); + expect(message.content).toContain("bg-done"); + } finally { + await toolset.dispose(); + } + }); + + test("shell_collect cancel kills the session's own child process group", async () => { + const token = `ic_toolset_cancel_${randomUUID()}`; + const toolset = await createAgentToolset({ + cwd: process.cwd(), + permissionGate: gate(process.cwd()), + onOperatorGate: async () => ({ kind: "cancel" as const }), + }); + try { + const started = await toolset.dynamicRunner.run( + { + id: "c-start", + name: "run_shell", + arguments: { + command: `sleep 600 # ${token}`, + background: true, + }, + }, + new AbortController().signal, + ); + const { shell_id } = JSON.parse(String(started.content)) as { shell_id: string }; + const cancelled = await toolset.dynamicRunner.run( + { + id: "c-cancel", + name: "shell_collect", + arguments: { shell_id, action: "cancel" }, + }, + new AbortController().signal, + ); + expect(JSON.parse(String(cancelled.content))).toMatchObject({ status: "cancelling" }); + await new Promise((r) => setTimeout(r, 300)); + const probe = spawnSync("pgrep", ["-f", token], { encoding: "utf8" }); + expect(probe.stdout?.trim() ?? "").toBe(""); + expect(probe.status).not.toBe(0); + } finally { + await toolset.dispose(); + } + }); + + test("toolset dispose kills every live background process group", async () => { + const token = `ic_toolset_dispose_${randomUUID()}`; + const toolset = await createAgentToolset({ + cwd: process.cwd(), + permissionGate: gate(process.cwd()), + onOperatorGate: async () => ({ kind: "cancel" as const }), + }); + const started = await toolset.dynamicRunner.run( + { + id: "d-start", + name: "run_shell", + arguments: { command: `sleep 600 # ${token}`, background: true }, + }, + new AbortController().signal, + ); + expect(started.isError).not.toBe(true); + await toolset.dispose(); + await new Promise((r) => setTimeout(r, 300)); + const probe = spawnSync("pgrep", ["-f", token], { encoding: "utf8" }); + expect(probe.stdout?.trim() ?? "").toBe(""); + expect(probe.status).not.toBe(0); + }); +}); diff --git a/src/agent/background-shell-tool.ts b/src/agent/background-shell-tool.ts new file mode 100644 index 000000000..459fbd831 --- /dev/null +++ b/src/agent/background-shell-tool.ts @@ -0,0 +1,105 @@ +import { type } from "arktype"; +import type { ToolDefinition } from "@intx/types/runtime"; +import { + createBackgroundShellRegistry, + type BackgroundShellExit, + type BackgroundShellRegistry, +} from "../shell/background-shell.js"; +import type { SpillBlobWriter } from "../plugins/result-truncation-plugin.js"; + +const ShellCollectArgs = type({ + shell_id: "string>0", + action: "'collect' | 'cancel'", + "wait_ms?": "number", +}); +type ShellCollectArgs = typeof ShellCollectArgs.infer; + +export const shellCollectDefinition: ToolDefinition = { + name: "shell_collect", + description: + "Collect or cancel a background run_shell (started with background: true). " + + 'action="collect" returns the result once finished (or status running); ' + + 'action="cancel" kills the process group. Completion also arrives as a ' + + "system message on a later turn — collect is for polling or retrieving " + + "output again after eviction risk.", + inputSchema: { + type: "object", + properties: { + shell_id: { type: "string", description: "shell_id from the background run_shell start." }, + action: { + type: "string", + enum: ["collect", "cancel"], + description: '"collect" retrieves status/output; "cancel" kills the process group.', + }, + wait_ms: { + type: "number", + description: + 'For action="collect": milliseconds to wait for completion before returning "running" (default 0, non-blocking).', + }, + }, + required: ["shell_id", "action"], + }, +}; + +export function createSpillingBackgroundShellExitNotifier(args: { + getBlobWriter?: () => SpillBlobWriter | undefined; + notify: (exit: BackgroundShellExit) => void; +}): (exit: BackgroundShellExit) => void { + return (exit) => { + void (async () => { + // Truncated output spills to the session blob store so the completion + // message can point at a readable tool-output:/// URI. + let spillUri: string | undefined; + if (exit.outputTruncated) { + const writeBlob = args.getBlobWriter?.(); + if (writeBlob !== undefined) { + const key = `bg-shell-${exit.id}`; + await writeBlob(key, new TextEncoder().encode(exit.output), "text/plain"); + spillUri = `tool-output:///${key}`; + } + } + args.notify(spillUri !== undefined ? { ...exit, spillUri } : exit); + })(); + }; +} + +export function createShellCollectTool( + registry: BackgroundShellRegistry = createBackgroundShellRegistry(), +) { + return { + definition: shellCollectDefinition, + handler: async (rawArgs: Record): Promise => { + const parsed = ShellCollectArgs(rawArgs); + if (parsed instanceof type.errors) { + return "Error: shell_collect requires shell_id (string) and action ('collect' | 'cancel')."; + } + const { shell_id, action } = parsed; + if (action === "cancel") { + if (!registry.cancel(shell_id)) { + return `No running background shell with id ${shell_id}; it may have already finished or been collected.`; + } + return JSON.stringify({ shell_id, status: "cancelling" }); + } + const snapshot = await registry.collect(shell_id, parsed.wait_ms ?? 0); + if (snapshot.state === "running") { + return JSON.stringify({ shell_id, status: "running" }); + } + if (snapshot.state === "not-found") { + return ( + `No background shell with id ${shell_id}. It may have been evicted from the ` + + "completed ring; if its output was truncated, the completion message carried " + + "a tool-output:/// URI for the full output." + ); + } + const { exit } = snapshot; + return JSON.stringify({ + shell_id, + status: "completed", + exit_code: exit.exitCode, + timed_out: exit.timedOut, + ...(exit.spillUri !== undefined ? { output_uri: exit.spillUri } : {}), + output: exit.output, + }); + }, + }; +} diff --git a/src/agent/directors/skywalker/package.ts b/src/agent/directors/skywalker/package.ts index 46113bd94..734b5ac83 100644 --- a/src/agent/directors/skywalker/package.ts +++ b/src/agent/directors/skywalker/package.ts @@ -16,7 +16,7 @@ You do not do the specialists' jobs by default. For tiny bounded product edits, Do not run long-blocking jobs on the parent (evals, full test suites, long installs, long-running implementation). Dispatch intern (mechanical shell), tester (suite / repro), or builder (substantial code). Path tools (write_file/edit_file/delete_file) are the DIY surface; shell file-writes stay denied. -Idle-orchestrator: fire one or more spawn_agent calls in a turn — each returns immediately with an agent_id and does not hold the parent. Then **reply to the operator** with who is running and what happens next before you block. Prefer ending that turn (or calling wait_agents with a short timeout_ms) so Enter can land; do not immediately fuse into a long wait_agents right after spawn. wait_agents later on the targets you need (or omit targets to wait on this session's own uncollected spawns — never a sibling's). list_agents shows that same fleet without blocking. Use mode="all" when you need every target to finish; interrupt_agent unblocks wait_agents immediately. A timeout means still running — do not tight-loop wait_agents hoping for a different answer. Enter mid-run delivers at the next parent tool.boundary — a long parent run_shell or awaiting wait_agents holds those steers. A bare spawn_agent does not. When the fleet goes dry the runtime re-enters with collected reports. +Idle-orchestrator: fire one or more spawn_agent calls in a turn — each returns immediately with an agent_id and does not hold the parent. Then **reply to the operator** with who is running and what happens next before you block. Prefer ending that turn (or calling wait_agents with a short timeout_ms) so Enter can land; do not immediately fuse into a long wait_agents right after spawn. wait_agents later on the targets you need (or omit targets to wait on this session's own uncollected spawns — never a sibling's). list_agents shows that same fleet without blocking. Use mode="all" when you need every target to finish; interrupt_agent unblocks wait_agents immediately. A timeout means still running — do not tight-loop wait_agents hoping for a different answer. Enter mid-run delivers at the next parent tool.boundary — a long parent foreground run_shell or awaiting wait_agents holds those steers (start long commands with run_shell background:true instead, and collect later). A bare spawn_agent does not. When the fleet goes dry the runtime re-enters with collected reports. # Operator updates (mandatory while fleet is live) diff --git a/src/agent/directors/tool-sets.ts b/src/agent/directors/tool-sets.ts index fcc46512a..52eb13834 100644 --- a/src/agent/directors/tool-sets.ts +++ b/src/agent/directors/tool-sets.ts @@ -12,6 +12,7 @@ export const READ_TOOLS = [ "list_dir", "lsp", "run_shell", + "shell_collect", "web_fetch", "web_search", ] as const; @@ -49,7 +50,7 @@ export const BUILD_TOOLS = [ * dependency, so it is not excluded alongside `shell`). */ export const DOCS_TOOLS = [ - ...READ_TOOLS.filter((t) => t !== "run_shell"), + ...READ_TOOLS.filter((t) => t !== "run_shell" && t !== "shell_collect"), ...PRODUCT_WRITE_TOOLS, "apply_patch", "update_plan", diff --git a/src/agent/posix-tool-plugins.test.ts b/src/agent/posix-tool-plugins.test.ts index f07f260d0..6ae08a40f 100644 --- a/src/agent/posix-tool-plugins.test.ts +++ b/src/agent/posix-tool-plugins.test.ts @@ -29,6 +29,41 @@ function findMiddlewareIndex( } describe("buildCorePosixToolPlugins", () => { + test("a permission-denied background run_shell spawns nothing", async () => { + const cwd = await mkdtemp(join(tmpdir(), "ic-bg-deny-")); + try { + const gate = createPermissionGate({ + approvals: [], + interactive: false, + skipPermissions: false, + reactorGated: false, + auto: false, + cwd, + }); + const runner = createPosixTools({ + cwd, + plugins: buildCorePosixToolPlugins({ cwd, permissionGate: gate }), + }); + const markerPath = join(cwd, "spawned.txt"); + const denied = await runner.run( + { + id: "bg-deny", + name: "run_shell", + arguments: { + command: `touch ${JSON.stringify(markerPath)}`, + background: true, + }, + }, + new AbortController().signal, + ); + expect(denied.isError).toBe(true); + // Nothing spawned: the marker file the command would have created is absent. + await expect(readFile(markerPath)).rejects.toThrow(); + } finally { + await rm(cwd, { recursive: true, force: true }); + } + }); + test("applies permission gate and result truncation like the main agent stack", async () => { const cwd = await mkdtemp(join(tmpdir(), "ic-posix-plugins-")); try { diff --git a/src/agent/posix-tool-plugins.ts b/src/agent/posix-tool-plugins.ts index 90e6b8898..905134738 100644 --- a/src/agent/posix-tool-plugins.ts +++ b/src/agent/posix-tool-plugins.ts @@ -17,6 +17,7 @@ import { } from "../plugins/result-truncation-plugin.js"; import { toolResultSecretScrubPlugin } from "../plugins/tool-result-secret-scrub-plugin.js"; import { shellGuardPlugin, type ShellTimeoutConfig } from "../plugins/shell-guard-plugin.js"; +import type { BackgroundShellRegistry } from "../shell/background-shell.js"; import { readFileGuardPlugin, type ReadFileGuardPluginOptions, @@ -37,6 +38,9 @@ export interface CorePosixToolPluginsArgs { getContextDir?: () => string | undefined; // Per-project settings.env, merged into the run_shell spawn environment. shellEnv?: Record; + // Live getter for the background-shell registry (run_shell background:true). + // Omitted makes background runs fail closed in shell-guard. + getBackgroundShellRegistry?: () => BackgroundShellRegistry | undefined; } // Middleware order matches docs/ARCHITECTURE.md: path escape through truncation, @@ -71,6 +75,7 @@ export function buildCorePosixToolPlugins(args: CorePosixToolPluginsArgs): ToolP getBlobWriter, getContextDir, shellEnv, + getBackgroundShellRegistry, } = args; // Pre-gate sandboxes honor yolo mode so outside-workspace path tools and shell // cwd are not hard-denied after the gate already auto-allows. Pass a live @@ -94,7 +99,10 @@ export function buildCorePosixToolPlugins(args: CorePosixToolPluginsArgs): ToolP secretGuardPlugin(), authzPlugin(), permissionPlugin(permissionGate), - shellGuardPlugin(cwd, shellTimeout, shellEnv, { allowOutsideCwd: allowOutside }), + shellGuardPlugin(cwd, shellTimeout, shellEnv, { + allowOutsideCwd: allowOutside, + ...(getBackgroundShellRegistry !== undefined ? { getBackgroundShellRegistry } : {}), + }), readFileGuardPlugin(cwd, readFileGuard), ripgrepPlugin(cwd), // Verify wraps the line-range short-circuit (composeMiddleware runs plugins diff --git a/src/agent/prompts.ts b/src/agent/prompts.ts index d9646f217..e7369845b 100644 --- a/src/agent/prompts.ts +++ b/src/agent/prompts.ts @@ -77,7 +77,7 @@ export function buildHarnessFacts( ]), "- Use the provided tools for file reads/searches instead of shelling out as a substitute.", "- read_file accepts a filesystem path or a tool-output:///{callId} URI from a prior tool result when the harness exposes one; prefer the URI over re-reading huge blobs.", - "- run_shell has no default timeout; pass timeout for builds, tests, and other long commands.", + "- run_shell has no default timeout; pass timeout for builds, tests, and other long commands. Prefer background:true for builds, test suites, and dev servers: it returns a shell_id at once, the result is delivered when the process finishes (foreground runs hold steers; background runs do not), and shell_collect collects or cancels later. background does not change the retained shell cwd.", "- Shell find, rg, and grep -r are blocked — they can walk huge trees and OOM the host. Prefer the bounded grep/search_files tools, and do not substitute another unbounded walk (fd, ls -R, scripted os.walk).", ...(subAgent ? [ diff --git a/src/agent/tool-classification.ts b/src/agent/tool-classification.ts index 644fe8f0e..628120b33 100644 --- a/src/agent/tool-classification.ts +++ b/src/agent/tool-classification.ts @@ -43,15 +43,20 @@ export const SEARCH_QUERY_TOOLS: ReadonlySet = new Set(["grep", "search_ * Tools that never need an approval prompt because they cannot change the * workspace: the director's read surface minus run_shell/web_fetch/web_search * (which get their own, narrower auto-allow rules — see - * isAutoAllowedShellCommand and the webfetch/websearch permission classes), - * plus manage_tasks (side-effect-free by the time the tool executes — see - * classify.ts). SECURITY-RELEVANT: this gates auto-allow. A tool added here - * is auto-approved everywhere; get it wrong in either direction deliberately, - * not by accident. + * isAutoAllowedShellCommand and the webfetch/websearch permission classes) + * and minus shell_collect (ungated by design at its handler: cancel only + * kills the session's own background child), plus manage_tasks (side-effect-free + * by the time the tool executes — see classify.ts). SECURITY-RELEVANT: this + * gates auto-allow. A tool added here is auto-approved everywhere; get it + * wrong in either direction deliberately, not by accident. */ export const AUTO_ALLOW_READ_TOOLS: ReadonlySet = new Set([ ...DIRECTOR_READ_TOOLS.filter( - (tool) => tool !== "run_shell" && tool !== "web_fetch" && tool !== "web_search", + (tool) => + tool !== "run_shell" && + tool !== "web_fetch" && + tool !== "web_search" && + tool !== "shell_collect", ), "manage_tasks", ]); diff --git a/src/agent/tool-search.ts b/src/agent/tool-search.ts index f91eb3822..9d42e0a22 100644 --- a/src/agent/tool-search.ts +++ b/src/agent/tool-search.ts @@ -29,6 +29,7 @@ export const CORE_TOOL_NAMES: readonly string[] = [ "delete_file", "lsp", "run_shell", + "shell_collect", "ask_operator", "manage_tasks", "tool_search", diff --git a/src/agent/tools.ts b/src/agent/tools.ts index bc7cd08d4..631bfff4d 100644 --- a/src/agent/tools.ts +++ b/src/agent/tools.ts @@ -69,6 +69,14 @@ import { createSendInputTool, } from "../subagent/lifecycle-tools.js"; import { parseManageTasksArgs } from "./tasks.js"; +import { + createShellCollectTool, + createSpillingBackgroundShellExitNotifier, +} from "./background-shell-tool.js"; +import { + createBackgroundShellRegistry, + type BackgroundShellExit, +} from "../shell/background-shell.js"; import { createListDirTool } from "../util/list-dir.js"; import { createExaMCPWebFetchTool, createWebFetchTool } from "../tools/web-fetch.js"; import { createWebSearchTool, disposeWebSearchClients } from "../tools/web-search.js"; @@ -158,6 +166,10 @@ export interface AgentToolsetArgs { getContextDir?: () => string | undefined; // Per-project settings.env, merged into the run_shell tool's spawn environment. shellEnv?: Record; + // Called when a background run_shell (background: true) process exits. Hosts + // deliver the exit as a system message so the reactor re-enters on a later + // turn; omit it and background runs still start/collect but never notify. + onBackgroundShellExit?: (exit: BackgroundShellExit) => void; // Whether a workflow is currently running. submit_output rides the wire // every turn (workflow or not), so the model can call it with nothing active; // this lets its handler report an honest no-op instead of a false advance. @@ -282,6 +294,19 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise backgroundShells, }), }); @@ -458,6 +484,10 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise): Promise => { @@ -966,6 +996,9 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise client.close().catch(() => undefined)), ); connectedClients.clear(); + // Kill every live background process group before the posix teardown so + // /clear, interrupt, and reload cannot leave orphans behind. + backgroundShells.disposeAll("session closed"); await posixTools.dispose(); await disposeWebSearchClients(); })(); diff --git a/src/exec/runner.ts b/src/exec/runner.ts index 2920de75d..2ec6d9a52 100644 --- a/src/exec/runner.ts +++ b/src/exec/runner.ts @@ -49,6 +49,7 @@ import { resolveExecRunStatus, type RunSink } from "../session/run-sink.js"; import { createRunSummary } from "../session/hooks.js"; import { buildCompactionContinuationMessage, + buildShellBackgroundMessage, buildSubAgentProvider, createSessionPruningCompactor, loadSessionChatPrompt, @@ -441,6 +442,10 @@ export async function runExec(config: Config): Promise { ...(localSettingsForMode?.env !== undefined ? { shellEnv: localSettingsForMode.env } : {}), getBlobWriter: () => currentStorage?.writeBlob, getContextDir: () => workdir, + // Background run_shell completions re-enter the reactor on a later turn. + onBackgroundShellExit: (exit) => { + currentAgent?.deliver(buildShellBackgroundMessage(exit)); + }, getBlobReader: () => { if (currentAgent === null) { throw new Error("blob reader requested before agent init"); diff --git a/src/plugins/shell-guard-plugin.test.ts b/src/plugins/shell-guard-plugin.test.ts index 585bdfbb5..348145195 100644 --- a/src/plugins/shell-guard-plugin.test.ts +++ b/src/plugins/shell-guard-plugin.test.ts @@ -8,6 +8,7 @@ import type { ToolCall, ToolResult } from "@intx/types/runtime"; import { spawnSync } from "node:child_process"; import { randomUUID } from "node:crypto"; +import { createBackgroundShellRegistry } from "../shell/background-shell.js"; import { BoundedShellOutput, MAX_SHELL_OUTPUT_BYTES, @@ -196,6 +197,79 @@ describe("resolveShellTimeoutMs", () => { }); }); +describe("background run_shell (shellGuardPlugin)", () => { + const fallback = async (call: ToolCall): Promise => ({ + callId: call.id, + content: "FALLBACK", + }); + + function handlerWith(registry?: ReturnType) { + return shellGuardPlugin(process.cwd(), undefined, undefined, { + ...(registry !== undefined ? { getBackgroundShellRegistry: () => registry } : {}), + }).middleware!(fallback); + } + + function runWith(registry: ReturnType, call: ToolCall) { + return handlerWith(registry)(call, neverAbort()); + } + + test("background:true returns a running handle immediately", async () => { + const registry = createBackgroundShellRegistry(); + const start = Date.now(); + const result = await runWith(registry, { + id: "bg1", + name: "run_shell", + arguments: { command: "sleep 0.5; echo finished", background: true }, + }); + expect(result.isError).toBeUndefined(); + const parsed = JSON.parse(String(result.content)) as { shell_id: string; status: string }; + expect(parsed.status).toBe("running"); + expect(parsed.shell_id.length).toBeGreaterThan(0); + // The START call resolved while the child was still sleeping. + expect(Date.now() - start).toBeLessThan(400); + const snapshot = await registry.collect(parsed.shell_id, 5_000); + expect(snapshot.state).toBe("completed"); + registry.disposeAll("test done"); + }); + + test("without a registry wired, background fails closed and spawns nothing", async () => { + const result = await handlerWith(undefined)( + { id: "bg2", name: "run_shell", arguments: { command: "echo hi", background: true } }, + neverAbort(), + ); + expect(result.isError).toBe(true); + expect(String(result.content)).toContain("not available"); + }); + + test("a background cd does not mutate the retained foreground shell cwd", async () => { + const registry = createBackgroundShellRegistry(); + const handler = handlerWith(registry); + await handler( + { id: "bg3", name: "run_shell", arguments: { command: "cd /", background: true } }, + neverAbort(), + ); + const after = await handler( + { id: "bg4", name: "run_shell", arguments: { command: "pwd" } }, + neverAbort(), + ); + expect(String(after.content).trim()).toBe(process.cwd()); + registry.disposeAll("test done"); + }); + + test("foreground run_shell is unchanged when background is unset", async () => { + const registry = createBackgroundShellRegistry(); + const result = await runWith(registry, { + id: "fg1", + name: "run_shell", + arguments: { command: "echo direct" }, + }); + expect(result.isError).toBeUndefined(); + expect(String(result.content)).toContain("direct"); + expect(registry.runningCount()).toBe(0); + registry.disposeAll("test done"); + }); +}); + describe("advertiseShellGuardTimeout", () => { test("rewrites run_shell timeout description when a settings default is set", () => { const rewritten = advertiseShellGuardTimeout( @@ -255,6 +329,24 @@ describe("advertiseShellGuardTimeout", () => { }; expect(advertiseShellGuardTimeout(def)).toBe(def); }); + + test("advertises background:true with collect/cancel guidance", () => { + const rewritten = advertiseShellGuardTimeout({ + name: "run_shell", + description: "Execute a shell command", + inputSchema: { + type: "object", + properties: { command: { type: "string" } }, + required: ["command"], + }, + }); + const background = ( + rewritten.inputSchema["properties"] as Record + )["background"]; + expect(background).toBeDefined(); + expect(background?.description).toContain("shell_collect"); + expect(background?.description).toMatch(/does not change the retained shell cwd/i); + }); }); describe("shellGuardPlugin", () => { diff --git a/src/plugins/shell-guard-plugin.ts b/src/plugins/shell-guard-plugin.ts index f96363aa7..db1309f5b 100644 --- a/src/plugins/shell-guard-plugin.ts +++ b/src/plugins/shell-guard-plugin.ts @@ -1,6 +1,7 @@ -import { spawn, type ChildProcess } from "node:child_process"; +import { spawn } from "node:child_process"; import { realpathSync } from "node:fs"; import type { ToolPlugin } from "@intx/tools-posix"; +import { killProcessTree, type BackgroundShellRegistry } from "../shell/background-shell.js"; import { formatSearchTimeoutMessage, TIMEOUT_PREFIX } from "./tool-time-budget.js"; import { BUDGET_EXPIRED, budgetExpiry, withTimeout } from "../util/budget-race.js"; import type { ToolDefinition } from "@intx/types/runtime"; @@ -81,6 +82,13 @@ 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. Does not change the retained shell cwd.", + }; return { ...definition, inputSchema: { @@ -199,25 +207,6 @@ export class BoundedShellOutput { } } -function killProcessTree(child: ChildProcess): void { - if (child.pid === undefined) return; - try { - if (process.platform === "win32") { - child.kill("SIGKILL"); - } else { - // Negative PID signals the whole process group. With detached:true the - // shell is the group leader, so grandchildren (find, grep, …) die too. - process.kill(-child.pid, "SIGKILL"); - } - } catch { - try { - child.kill("SIGKILL"); - } catch { - // already exited - } - } -} - export async function runGuardedShell( args: RunShellArgs, signal: AbortSignal, @@ -336,6 +325,9 @@ export interface ShellGuardPluginOptions { // outside the session root. A getter is resolved per call so `/yolo` // mid-session takes effect without rebuilding the plugin stack. allowOutsideCwd?: boolean | (() => boolean); + // Live getter for the background-shell registry. Unwired (undefined result) + // makes `background: true` fail closed: nothing spawns, no handle returns. + getBackgroundShellRegistry?: () => BackgroundShellRegistry | undefined; } function resolveAllowOutsideCwd(value: boolean | (() => boolean) | undefined): boolean { @@ -412,6 +404,33 @@ export function shellGuardPlugin( defaultMs, timeoutConfig?.maxMs, ); + if (call.arguments.background === true) { + const registry = options.getBackgroundShellRegistry?.(); + if (registry === undefined) { + return { + callId: call.id, + content: "background shell is not available in this session", + isError: true, + }; + } + const started = registry.start({ + command, + cwd: executionCwd, + ...(effectiveTimeout !== undefined ? { timeoutMs: effectiveTimeout } : {}), + maxOutputBytes, + ...(env !== undefined ? { env } : {}), + }); + if ("error" in started) { + return { callId: call.id, content: started.error, isError: true }; + } + // No pwd probe and no retained-cwd mutation: the background shell + // never runs in the foreground shell's session, so a `cd` inside it + // affects only its own process. + return { + callId: call.id, + content: JSON.stringify({ shell_id: started.id, status: "running" }), + }; + } const wrappedCommand = wrapCommandWithPwdProbe(command); try { const { output, exitCode, timedOut, outputTruncated } = await runGuardedShell( diff --git a/src/session/runtime-assembly.ts b/src/session/runtime-assembly.ts index d5cbbc3c9..8a6d0a4a9 100644 --- a/src/session/runtime-assembly.ts +++ b/src/session/runtime-assembly.ts @@ -401,3 +401,49 @@ export function buildFleetDryContinuationMessage(text: string): InboundMessage { signatureStatus: "missing", }; } + +// Preview cap for a background shell's inline output; the full output stays in +// the registry (shell_collect) and, when truncated, in the spill blob. +const BACKGROUND_SHELL_PREVIEW_CHARS = 2_000; + +/** + * Content-bearing inbound the host delivers when a background run_shell process + * exits. Mailbox "system" and empty flags: loop protection treats it as + * system-originated, so it re-enters the reactor without counting as operator + * input (see message-provenance.ts). + */ +export function buildShellBackgroundMessage(exit: { + id: string; + command: string; + exitCode: number; + timedOut: boolean; + output: string; + spillUri?: string; +}): InboundMessage { + const status = exit.timedOut + ? `timed out and was killed (exit code ${exit.exitCode})` + : `exit code ${exit.exitCode}`; + const lines = [`Background shell ${exit.id} finished: ${status}.`, `command: ${exit.command}`]; + if (exit.output.length > 0) { + const preview = + exit.output.length > BACKGROUND_SHELL_PREVIEW_CHARS + ? `${exit.output.slice(0, BACKGROUND_SHELL_PREVIEW_CHARS)}\n[...preview truncated]` + : exit.output; + lines.push(`output:\n${preview}`); + } + if (exit.spillUri !== undefined) { + lines.push(`Full output was spilled to ${exit.spillUri} (readable via read_file).`); + } + return { + ref: { uid: 0, mailbox: "system" }, + headers: { + from: "user@local", + to: ["agent@local"], + date: new Date().toISOString(), + messageId: `bg-shell-${exit.id}@local`, + }, + flags: [], + content: lines.join("\n"), + signatureStatus: "missing", + }; +} diff --git a/src/shell/background-shell.test.ts b/src/shell/background-shell.test.ts new file mode 100644 index 000000000..d9d8e1db0 --- /dev/null +++ b/src/shell/background-shell.test.ts @@ -0,0 +1,116 @@ +import { describe, expect, test } from "bun:test"; +import { spawnSync } from "node:child_process"; +import { randomUUID } from "node:crypto"; +import { + MAX_COMPLETED_BACKGROUND_SHELLS, + MAX_RUNNING_BACKGROUND_SHELLS, + createBackgroundShellRegistry, +} from "./background-shell.js"; + +const tmpCwd = process.cwd(); + +describe("background shell registry", () => { + test("start returns a handle immediately while the process runs", async () => { + const registry = createBackgroundShellRegistry(); + const started = registry.start({ command: "sleep 1; echo done", cwd: tmpCwd }); + if ("error" in started) throw new Error(started.error); + const snapshot = await registry.collect(started.id, 0); + expect(snapshot.state).toBe("running"); + const exited = await registry.collect(started.id, 5_000); + expect(exited.state).toBe("completed"); + if (exited.state !== "completed") return; + expect(exited.exit.exitCode).toBe(0); + expect(exited.exit.output).toContain("done"); + expect(exited.exit.timedOut).toBe(false); + }); + + test("onExit fires with exit status and output", async () => { + const exits: unknown[] = []; + const registry = createBackgroundShellRegistry({ onExit: (exit) => exits.push(exit) }); + const started = registry.start({ command: "echo hi", cwd: tmpCwd }); + if ("error" in started) throw new Error(started.error); + await registry.collect(started.id, 5_000); + await new Promise((r) => setTimeout(r, 50)); + expect(exits).toHaveLength(1); + }); + + test("timeout kills the group and reports exit 124 + timedOut", async () => { + const registry = createBackgroundShellRegistry(); + const started = registry.start({ + command: "echo early; sleep 60", + cwd: tmpCwd, + timeoutMs: 150, + }); + if ("error" in started) throw new Error(started.error); + const exited = await registry.collect(started.id, 5_000); + expect(exited.state).toBe("completed"); + if (exited.state !== "completed") return; + expect(exited.exit.timedOut).toBe(true); + expect(exited.exit.exitCode).toBe(124); + expect(exited.exit.output).toContain("early"); + }); + + test("cancel kills the whole process group", async () => { + if (process.platform === "win32") return; + const token = `ic_bg_cancel_${randomUUID()}`; + const registry = createBackgroundShellRegistry(); + const started = registry.start({ + command: `bash -c 'TAG=${token} sleep 600 & TAG=${token} exec sleep 600'`, + cwd: tmpCwd, + }); + if ("error" in started) throw new Error(started.error); + expect(registry.cancel(started.id)).toBe(true); + const exited = await registry.collect(started.id, 5_000); + expect(exited.state).toBe("completed"); + await new Promise((r) => setTimeout(r, 300)); + const probe = spawnSync("pgrep", ["-f", token], { encoding: "utf8" }); + expect(probe.stdout?.trim() ?? "").toBe(""); + expect(probe.status).not.toBe(0); + }); + + test("cancel on an unknown id returns false", () => { + const registry = createBackgroundShellRegistry(); + expect(registry.cancel("nope")).toBe(false); + }); + + test("completed ring evicts the oldest entry (collect reports not-found)", async () => { + const registry = createBackgroundShellRegistry(); + const ids: string[] = []; + for (let i = 0; i <= MAX_COMPLETED_BACKGROUND_SHELLS; i++) { + const started = registry.start({ command: "true", cwd: tmpCwd }); + if ("error" in started) throw new Error(started.error); + ids.push(started.id); + await registry.collect(started.id, 5_000); + } + expect(ids).toHaveLength(MAX_COMPLETED_BACKGROUND_SHELLS + 1); + const evicted = await registry.collect(ids[0]!, 0); + expect(evicted.state).toBe("not-found"); + const retained = await registry.collect(ids[ids.length - 1]!, 0); + expect(retained.state).toBe("completed"); + }); + + test("running cap fails closed with an error instead of spawning", async () => { + const registry = createBackgroundShellRegistry(); + for (let i = 0; i < MAX_RUNNING_BACKGROUND_SHELLS; i++) { + const started = registry.start({ command: "sleep 30", cwd: tmpCwd }); + expect("error" in started).toBe(false); + } + const over = registry.start({ command: "sleep 30", cwd: tmpCwd }); + expect("error" in over).toBe(true); + registry.disposeAll("test done"); + }); + + test("disposeAll kills running children", async () => { + const token = `ic_bg_dispose_${randomUUID()}`; + const registry = createBackgroundShellRegistry(); + const started = registry.start({ command: `sleep 600 # ${token}`, cwd: tmpCwd }); + if ("error" in started) throw new Error(started.error); + registry.disposeAll("session closed"); + await new Promise((r) => setTimeout(r, 300)); + const probe = spawnSync("pgrep", ["-f", token], { encoding: "utf8" }); + expect(probe.stdout?.trim() ?? "").toBe(""); + expect(probe.status).not.toBe(0); + const after = await registry.collect(started.id, 0); + expect(after.state).toBe("not-found"); + }); +}); diff --git a/src/shell/background-shell.ts b/src/shell/background-shell.ts new file mode 100644 index 000000000..95af28abc --- /dev/null +++ b/src/shell/background-shell.ts @@ -0,0 +1,183 @@ +import { spawn, type ChildProcess } from "node:child_process"; +import { randomUUID } from "node:crypto"; +import { BoundedShellOutput, MAX_SHELL_OUTPUT_BYTES } from "../plugins/shell-guard-plugin.js"; + +// Background run_shell: the starting tool call returns a handle at once and the +// process keeps running past the end of the turn, so tool.boundary fires and +// queued steers land while builds, test suites, and dev servers run. On exit +// the result is pushed to the host via onExit (which delivers it to the +// reactor as a system message on a later turn); shell_collect retrieves or +// cancels by id. + +export const MAX_RUNNING_BACKGROUND_SHELLS = 8; +export const MAX_COMPLETED_BACKGROUND_SHELLS = 8; + +/** + * Signals the whole process group. With detached:true the shell is the group + * leader, so grandchildren (build watchers, test runners' workers) die too. + */ +export function killProcessTree(child: ChildProcess): void { + if (child.pid === undefined) return; + try { + if (process.platform === "win32") { + child.kill("SIGKILL"); + } else { + // Negative PID signals the whole process group. + process.kill(-child.pid, "SIGKILL"); + } + } catch { + try { + child.kill("SIGKILL"); + } catch { + // already exited + } + } +} + +export interface StartBackgroundShellArgs { + command: string; + cwd: string; + timeoutMs?: number; + maxOutputBytes?: number; + env?: Record; +} + +export interface BackgroundShellExit { + id: string; + command: string; + exitCode: number; + timedOut: boolean; + output: string; + outputTruncated: boolean; + // Set by the toolset wrapper when truncated output was spilled to the + // session blob store; the completion message carries the URI. + spillUri?: string; +} + +export type BackgroundShellSnapshot = + { state: "running" } | { state: "completed"; exit: BackgroundShellExit } | { state: "not-found" }; + +export interface BackgroundShellRegistry { + start: (args: StartBackgroundShellArgs) => { id: string } | { error: string }; + collect: (id: string, waitMs?: number) => Promise; + cancel: (id: string) => boolean; + disposeAll: (reason: string) => void; + runningCount: () => number; +} + +export function createBackgroundShellRegistry( + options: { onExit?: (exit: BackgroundShellExit) => void } = {}, +): BackgroundShellRegistry { + const { onExit } = options; + const running = new Map(); + const completed = new Map(); + const exitWaiters = new Map void>(); + + const record = + (id: string, command: string, collector: BoundedShellOutput) => + (exitCode: number, timedOut: boolean): void => { + if (!running.delete(id)) return; + const { output, truncated } = collector.build(); + const exit: BackgroundShellExit = { + id, + command, + exitCode, + timedOut, + output, + outputTruncated: truncated, + }; + completed.set(id, exit); + // Ring eviction drops the oldest completed entry; the completion message + // already carried a spill URI for truncated output, so nothing is lost. + while (completed.size > MAX_COMPLETED_BACKGROUND_SHELLS) { + const oldest = completed.keys().next().value; + if (oldest === undefined) break; + completed.delete(oldest); + } + exitWaiters.get(id)?.(); + exitWaiters.delete(id); + onExit?.(exit); + }; + + const start = (args: StartBackgroundShellArgs): { id: string } | { error: string } => { + if (running.size >= MAX_RUNNING_BACKGROUND_SHELLS) { + return { + error: `background shell limit reached (${MAX_RUNNING_BACKGROUND_SHELLS} running); collect or cancel one first`, + }; + } + const id = randomUUID(); + const collector = new BoundedShellOutput(args.maxOutputBytes ?? MAX_SHELL_OUTPUT_BYTES); + // detached so the shell leads a process group and cancel/timeout can + // SIGKILL the whole tree. + const child = spawn(args.command, { + shell: true, + stdio: ["ignore", "pipe", "pipe"], + cwd: args.cwd, + detached: process.platform !== "win32", + env: args.env !== undefined ? { ...process.env, ...args.env } : undefined, + }); + running.set(id, child); + const finish = record(id, args.command, collector); + child.stdout?.on("data", (chunk: Buffer) => collector.append(chunk)); + child.stderr?.on("data", (chunk: Buffer) => collector.append(chunk)); + let timer: ReturnType | undefined; + if (args.timeoutMs !== undefined && args.timeoutMs > 0) { + timer = setTimeout(() => { + killProcessTree(child); + finish(124, true); + }, args.timeoutMs); + } + child.on("error", () => { + if (timer !== undefined) clearTimeout(timer); + finish(1, false); + }); + child.on("close", (code, sig) => { + if (timer !== undefined) clearTimeout(timer); + if (!running.has(id)) return; + finish(code ?? (sig !== null ? 128 : 1), false); + }); + return { id }; + }; + + const collect = async (id: string, waitMs = 0): Promise => { + const done = completed.get(id); + if (done !== undefined) return { state: "completed", exit: done }; + if (!running.has(id)) return { state: "not-found" }; + if (waitMs > 0) { + await new Promise((resolve) => { + exitWaiters.set(id, resolve); + setTimeout(resolve, waitMs); + }).finally(() => exitWaiters.delete(id)); + const finished = completed.get(id); + if (finished !== undefined) return { state: "completed", exit: finished }; + } + return { state: "running" }; + }; + + const cancel = (id: string): boolean => { + const child = running.get(id); + if (child === undefined) return false; + killProcessTree(child); + return true; + }; + + const disposeAll = (reason: string): void => { + for (const child of running.values()) killProcessTree(child); + running.clear(); + completed.clear(); + for (const wake of exitWaiters.values()) wake(); + exitWaiters.clear(); + // onExit is intentionally not fired for disposed shells: the session is + // gone, so there is no later turn to deliver to (`reason` is for callers + // that log it). + void reason; + }; + + return { + start, + collect, + cancel, + disposeAll, + runningCount: () => running.size, + }; +} diff --git a/src/subagent/run.ts b/src/subagent/run.ts index e71e6a686..35915c04e 100644 --- a/src/subagent/run.ts +++ b/src/subagent/run.ts @@ -63,8 +63,14 @@ import { normalizeToolDefinitionsForProvider } from "../agent/tool-schema-normal import { buildCompactionContinuationMessage, + buildShellBackgroundMessage, createSessionPruningCompactor, } from "../session/runtime-assembly.js"; +import { + createBackgroundShellRegistry, + type BackgroundShellExit, +} from "../shell/background-shell.js"; +import { createShellCollectTool } from "../agent/background-shell-tool.js"; import { createAttachmentRehydrateTransform } from "../session/attachment-store.js"; import { createModelSummarizer } from "../session/summarizer.js"; import { gatherEnvironment } from "../agent/environment.js"; @@ -456,6 +462,12 @@ async function runSubAgentInner( const submitResultState = createSubmitResultState(); const askDirectorState = createAskDirectorState(); const spawnRegistry = createSubAgentSpawnRegistryPlugin(); + // Assigned inside the try once the agent handle exists; before that (or after + // close) a completion is dropped, matching the continuation contract. + let backgroundExitSink: ((exit: BackgroundShellExit) => void) | null = null; + const backgroundShells = createBackgroundShellRegistry({ + onExit: (exit) => backgroundExitSink?.(exit), + }); // Child tools resolve spills against the child's own store first, then // the parent's: parent tool-output:// URIs handed in the brief must // remain readable after spawn, and the child's own spills stay local. @@ -470,6 +482,7 @@ async function runSubAgentInner( ...(params.shellTimeout !== undefined ? { shellTimeout: params.shellTimeout } : {}), ...(params.shellEnv !== undefined ? { shellEnv: params.shellEnv } : {}), readFileGuard: { blobReader: sessionBlobReader }, + getBackgroundShellRegistry: () => backgroundShells, extraToolPlugins: [...(params.extraToolPlugins ?? []), spawnRegistry.plugin], }), }); @@ -525,7 +538,11 @@ async function runSubAgentInner( })); const inherited = params.inheritMcpTools?.() ?? []; - tools = [...tools, ...coreSubAgentWebTools(inherited)]; + tools = [ + ...tools, + ...coreSubAgentWebTools(inherited), + stringTool(createShellCollectTool(backgroundShells)), + ]; if (inherited.length > 0) { tools = [...tools, ...inherited]; @@ -933,6 +950,13 @@ async function runSubAgentInner( // resolve without dropping the parent fallback. childBlobReader = agent.blobReader; agentHandle = agent; + backgroundExitSink = (exit) => { + try { + agentHandle?.deliver(buildShellBackgroundMessage(exit)); + } catch { + // Agent may be closing; a dropped completion is harmless. + } + }; // Collect tool activity for the parent-facing report, and optionally forward // progress without dumping the full sub-agent event stream into the chat @@ -1080,6 +1104,7 @@ async function runSubAgentInner( // close is idempotent; ignore races with disposeSubAgentSession. } try { + backgroundShells.disposeAll("sub-agent closed"); await posixTools.dispose(); } catch { // ignore @@ -1317,6 +1342,7 @@ async function runSubAgentInner( // stays open until close_agent (or a later failed/aborted run) tears it // down. if (!persisting) { + backgroundShells.disposeAll("sub-agent closed"); await disposeSubAgentSession({ signal: runController.signal, ...(closeOnAbort !== undefined ? { closeOnAbort } : {}), diff --git a/src/tui/runner/session.ts b/src/tui/runner/session.ts index bbef5dc6d..832ec8901 100644 --- a/src/tui/runner/session.ts +++ b/src/tui/runner/session.ts @@ -50,6 +50,7 @@ import { createApprovalResume } from "../../session/approval-resume.js"; import { createReactorAuthorize } from "../../permission/reactor-authorize.js"; import { buildCompactionContinuationMessage, + buildShellBackgroundMessage, createLiveSubAgentSources, createSessionPruningCompactor, loadSessionChatPrompt, @@ -262,6 +263,13 @@ export async function assembleTUISession( getBlobReader: () => liveAgent(state).blobReader, getBlobWriter: () => state.currentStorage?.writeBlob, getContextDir: () => state.workdir, + // A background run_shell completion re-enters the reactor on a later turn, + // so queued steers are not blocked by a long foreground run. + onBackgroundShellExit: (exit) => { + state.enqueueAgentDeliver?.(() => + liveAgent(state).deliver(buildShellBackgroundMessage(exit)), + ); + }, isWorkflowActive: () => workflowControllerHolder.instance?.isActive() === true, completeWorkflowStep: (stepId) => workflowControllerHolder.instance?.complete(stepId) ?? "not-current", diff --git a/src/tui/tool-execution-watchdog.test.ts b/src/tui/tool-execution-watchdog.test.ts index d62515e6d..fc10cfbe0 100644 --- a/src/tui/tool-execution-watchdog.test.ts +++ b/src/tui/tool-execution-watchdog.test.ts @@ -64,6 +64,32 @@ describe("tool execution watchdog", () => { ).toBeUndefined(); }); + test("background run_shell start is exempt; foreground arms requested+slack", () => { + const background = { + id: "1", + name: "run_shell", + arguments: { command: "sleep 60", timeout: 5_000, background: true }, + }; + expect(resolveToolExecutionTimeoutMs({ defaultMs: 660_000 }, background)).toBeUndefined(); + const foreground = { + id: "2", + name: "run_shell", + arguments: { command: "sleep 60", timeout: 5_000 }, + }; + expect(resolveToolExecutionTimeoutMs({ defaultMs: 660_000 }, foreground)).toBe( + 5_000 + RUN_SHELL_WATCHDOG_SLACK_MS, + ); + }); + + test("shell_collect is exempt from the settings watchdog", () => { + const call = { + id: "1", + name: "shell_collect", + arguments: { shell_id: "x", action: "collect", wait_ms: 4_000 }, + }; + expect(resolveToolExecutionTimeoutMs({ defaultMs: 660_000 }, call)).toBeUndefined(); + }); + test("ask_director with no settings timeout is unbounded", () => { expect( resolveToolExecutionTimeoutMs(undefined, { id: "1", name: "ask_director", arguments: {} }), diff --git a/src/tui/tool-execution-watchdog.ts b/src/tui/tool-execution-watchdog.ts index 806383fbd..63a76a79b 100644 --- a/src/tui/tool-execution-watchdog.ts +++ b/src/tui/tool-execution-watchdog.ts @@ -90,11 +90,18 @@ export function resolveToolExecutionTimeoutMs( if ( call?.name === "spawn_agent" || call?.name === "wait_agents" || - call?.name === "ask_director" + call?.name === "ask_director" || + // shell_collect with wait_ms is a bounded poll over a background shell that + // outlives the turn; aborting the collect would not stop the process. + call?.name === "shell_collect" ) { return undefined; } if (call?.name === "run_shell") { + // A background run returns at once and finishes after tool.boundary; the + // watchdog must not arm requested+slack against the START call or it would + // abort mid-run. The process's own timeout still bounds it. + if (call.arguments.background === true) return undefined; const requested = requestedRunShellTimeoutMs(call); if (requested !== undefined) { return requested + RUN_SHELL_WATCHDOG_SLACK_MS;