Skip to content

Commit df7bc49

Browse files
Merge pull request #836 from corbitsdev/cl-7517-background-mode-for-shell-commands-so-long-runs-dont-hold
Add background mode for shell commands so long runs don't hold steers
2 parents 2c67dac + c356bda commit df7bc49

22 files changed

Lines changed: 895 additions & 35 deletions

docs/ARCHITECTURE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ tool call
369369
- **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.
370370
- **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.
371371
- **Permission** (`permission-plugin.ts`) — Delegates consequential calls to the permission gate.
372-
- **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`.
372+
- **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`.
373373
- **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.
374374
- **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.
375375
- **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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ src/
118118
permission-plugin.ts Tiered operator approval
119119
shell/
120120
run-shell-authz.ts Shared run_shell deny policy (authz + permission)
121+
background-shell.ts Background run_shell registry (start/collect/cancel/disposeAll)
121122
verify-plugin.ts Write/edit verification (per-path lock)
122123
file-mutation-lock.ts Serialize mutations per file for verify
123124
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
185186

186187
`ChatInputProps` carries `isProcessing?: boolean` and `onInterrupt?: (message: string) => void`. When `isProcessing` is true, drain timing is **parent-idle** vs **session-idle**:
187188

188-
- **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.
189+
- **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.
190+
191+
#### Background shell mode
192+
193+
`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-<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`). Toolset dispose calls `disposeAll("session closed")` before the posix teardown, so `/clear`, interrupt, and reload kill every live background process group.
194+
189195
- **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.
190196

191197
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
250256
}
251257
```
252258

253-
- `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.
259+
- `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).
254260
- `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.
255261

256262
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):
Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
import { describe, expect, test } from "bun:test";
2+
import { spawnSync } from "node:child_process";
3+
import { randomUUID } from "node:crypto";
4+
import { createPermissionGate } from "../permission/gate.js";
5+
import { createAgentToolset } from "./tools.js";
6+
import type { BackgroundShellExit } from "../shell/background-shell.js";
7+
import { buildShellBackgroundMessage } from "../session/runtime-assembly.js";
8+
import { OPERATOR_ORIGINATED_FLAG } from "../agent/message-provenance.js";
9+
10+
function gate(cwd: string) {
11+
return createPermissionGate({
12+
approvals: [],
13+
interactive: false,
14+
skipPermissions: true,
15+
reactorGated: false,
16+
cwd,
17+
});
18+
}
19+
20+
describe("background shell through the agent toolset", () => {
21+
test("run_shell background:true returns a handle and delivers the exit on exit", async () => {
22+
const exits: BackgroundShellExit[] = [];
23+
const toolset = await createAgentToolset({
24+
cwd: process.cwd(),
25+
permissionGate: gate(process.cwd()),
26+
onOperatorGate: async () => ({ kind: "cancel" as const }),
27+
onBackgroundShellExit: (exit) => exits.push(exit),
28+
});
29+
try {
30+
const started = await toolset.dynamicRunner.run(
31+
{
32+
id: "bg-start",
33+
name: "run_shell",
34+
arguments: { command: "sleep 0.4; echo bg-done", background: true },
35+
},
36+
new AbortController().signal,
37+
);
38+
expect(started.isError).not.toBe(true);
39+
const parsed = JSON.parse(String(started.content)) as { shell_id: string };
40+
const snapshotNow = await toolset.dynamicRunner.run(
41+
{
42+
id: "bg-collect",
43+
name: "shell_collect",
44+
arguments: { shell_id: parsed.shell_id, action: "collect" },
45+
},
46+
new AbortController().signal,
47+
);
48+
expect(JSON.parse(String(snapshotNow.content))).toMatchObject({ status: "running" });
49+
const final = await toolset.dynamicRunner.run(
50+
{
51+
id: "bg-collect2",
52+
name: "shell_collect",
53+
arguments: { shell_id: parsed.shell_id, action: "collect", wait_ms: 5_000 },
54+
},
55+
new AbortController().signal,
56+
);
57+
const result = JSON.parse(String(final.content)) as {
58+
status: string;
59+
exit_code: number;
60+
output: string;
61+
};
62+
expect(result).toMatchObject({ status: "completed", exit_code: 0 });
63+
expect(result.output).toContain("bg-done");
64+
await new Promise((r) => setTimeout(r, 50));
65+
expect(exits).toHaveLength(1);
66+
expect(exits[0]!.id).toBe(parsed.shell_id);
67+
const message = buildShellBackgroundMessage(exits[0]!);
68+
expect(message.headers.messageId).toBe(`bg-shell-${parsed.shell_id}@local`);
69+
expect(message.ref.mailbox).toBe("system");
70+
expect(message.flags).not.toContain(OPERATOR_ORIGINATED_FLAG);
71+
expect(message.content).toContain("exit code 0");
72+
expect(message.content).toContain("bg-done");
73+
} finally {
74+
await toolset.dispose();
75+
}
76+
});
77+
78+
test("shell_collect cancel kills the session's own child process group", async () => {
79+
const token = `ic_toolset_cancel_${randomUUID()}`;
80+
const toolset = await createAgentToolset({
81+
cwd: process.cwd(),
82+
permissionGate: gate(process.cwd()),
83+
onOperatorGate: async () => ({ kind: "cancel" as const }),
84+
});
85+
try {
86+
const started = await toolset.dynamicRunner.run(
87+
{
88+
id: "c-start",
89+
name: "run_shell",
90+
arguments: {
91+
command: `sleep 600 # ${token}`,
92+
background: true,
93+
},
94+
},
95+
new AbortController().signal,
96+
);
97+
const { shell_id } = JSON.parse(String(started.content)) as { shell_id: string };
98+
const cancelled = await toolset.dynamicRunner.run(
99+
{
100+
id: "c-cancel",
101+
name: "shell_collect",
102+
arguments: { shell_id, action: "cancel" },
103+
},
104+
new AbortController().signal,
105+
);
106+
expect(JSON.parse(String(cancelled.content))).toMatchObject({ status: "cancelling" });
107+
await new Promise((r) => setTimeout(r, 300));
108+
const probe = spawnSync("pgrep", ["-f", token], { encoding: "utf8" });
109+
expect(probe.stdout?.trim() ?? "").toBe("");
110+
expect(probe.status).not.toBe(0);
111+
} finally {
112+
await toolset.dispose();
113+
}
114+
});
115+
116+
test("toolset dispose kills every live background process group", async () => {
117+
const token = `ic_toolset_dispose_${randomUUID()}`;
118+
const toolset = await createAgentToolset({
119+
cwd: process.cwd(),
120+
permissionGate: gate(process.cwd()),
121+
onOperatorGate: async () => ({ kind: "cancel" as const }),
122+
});
123+
const started = await toolset.dynamicRunner.run(
124+
{
125+
id: "d-start",
126+
name: "run_shell",
127+
arguments: { command: `sleep 600 # ${token}`, background: true },
128+
},
129+
new AbortController().signal,
130+
);
131+
expect(started.isError).not.toBe(true);
132+
await toolset.dispose();
133+
await new Promise((r) => setTimeout(r, 300));
134+
const probe = spawnSync("pgrep", ["-f", token], { encoding: "utf8" });
135+
expect(probe.stdout?.trim() ?? "").toBe("");
136+
expect(probe.status).not.toBe(0);
137+
});
138+
});

src/agent/background-shell-tool.ts

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
import { type } from "arktype";
2+
import type { ToolDefinition } from "@intx/types/runtime";
3+
import {
4+
createBackgroundShellRegistry,
5+
type BackgroundShellExit,
6+
type BackgroundShellRegistry,
7+
} from "../shell/background-shell.js";
8+
import type { SpillBlobWriter } from "../plugins/result-truncation-plugin.js";
9+
10+
const ShellCollectArgs = type({
11+
shell_id: "string>0",
12+
action: "'collect' | 'cancel'",
13+
"wait_ms?": "number",
14+
});
15+
type ShellCollectArgs = typeof ShellCollectArgs.infer;
16+
17+
export const shellCollectDefinition: ToolDefinition = {
18+
name: "shell_collect",
19+
description:
20+
"Collect or cancel a background run_shell (started with background: true). " +
21+
'action="collect" returns the result once finished (or status running); ' +
22+
'action="cancel" kills the process group. Completion also arrives as a ' +
23+
"system message on a later turn — collect is for polling or retrieving " +
24+
"output again after eviction risk.",
25+
inputSchema: {
26+
type: "object",
27+
properties: {
28+
shell_id: { type: "string", description: "shell_id from the background run_shell start." },
29+
action: {
30+
type: "string",
31+
enum: ["collect", "cancel"],
32+
description: '"collect" retrieves status/output; "cancel" kills the process group.',
33+
},
34+
wait_ms: {
35+
type: "number",
36+
description:
37+
'For action="collect": milliseconds to wait for completion before returning "running" (default 0, non-blocking).',
38+
},
39+
},
40+
required: ["shell_id", "action"],
41+
},
42+
};
43+
44+
export function createSpillingBackgroundShellExitNotifier(args: {
45+
getBlobWriter?: () => SpillBlobWriter | undefined;
46+
notify: (exit: BackgroundShellExit) => void;
47+
}): (exit: BackgroundShellExit) => void {
48+
return (exit) => {
49+
void (async () => {
50+
// Truncated output spills to the session blob store so the completion
51+
// message can point at a readable tool-output:/// URI.
52+
let spillUri: string | undefined;
53+
if (exit.outputTruncated) {
54+
const writeBlob = args.getBlobWriter?.();
55+
if (writeBlob !== undefined) {
56+
const key = `bg-shell-${exit.id}`;
57+
await writeBlob(key, new TextEncoder().encode(exit.output), "text/plain");
58+
spillUri = `tool-output:///${key}`;
59+
}
60+
}
61+
args.notify(spillUri !== undefined ? { ...exit, spillUri } : exit);
62+
})();
63+
};
64+
}
65+
66+
export function createShellCollectTool(
67+
registry: BackgroundShellRegistry = createBackgroundShellRegistry(),
68+
) {
69+
return {
70+
definition: shellCollectDefinition,
71+
handler: async (rawArgs: Record<string, unknown>): Promise<string> => {
72+
const parsed = ShellCollectArgs(rawArgs);
73+
if (parsed instanceof type.errors) {
74+
return "Error: shell_collect requires shell_id (string) and action ('collect' | 'cancel').";
75+
}
76+
const { shell_id, action } = parsed;
77+
if (action === "cancel") {
78+
if (!registry.cancel(shell_id)) {
79+
return `No running background shell with id ${shell_id}; it may have already finished or been collected.`;
80+
}
81+
return JSON.stringify({ shell_id, status: "cancelling" });
82+
}
83+
const snapshot = await registry.collect(shell_id, parsed.wait_ms ?? 0);
84+
if (snapshot.state === "running") {
85+
return JSON.stringify({ shell_id, status: "running" });
86+
}
87+
if (snapshot.state === "not-found") {
88+
return (
89+
`No background shell with id ${shell_id}. It may have been evicted from the ` +
90+
"completed ring; if its output was truncated, the completion message carried " +
91+
"a tool-output:/// URI for the full output."
92+
);
93+
}
94+
const { exit } = snapshot;
95+
return JSON.stringify({
96+
shell_id,
97+
status: "completed",
98+
exit_code: exit.exitCode,
99+
timed_out: exit.timedOut,
100+
...(exit.spillUri !== undefined ? { output_uri: exit.spillUri } : {}),
101+
output: exit.output,
102+
});
103+
},
104+
};
105+
}

0 commit comments

Comments
 (0)