diff --git a/AGENTS.md b/AGENTS.md index c9c26919..6edc24e4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -129,6 +129,14 @@ Two checks run in `executeBatch` on every non-terminal call, in this order, befo Pinned by [src/tools/control-marker-guard.test.ts](src/tools/control-marker-guard.test.ts), [src/tools/unknown-argument-guard.test.ts](src/tools/unknown-argument-guard.test.ts), the guard cases in [src/agent/batch-executor.test.ts](src/agent/batch-executor.test.ts) and [src/tools/os/shell.test.ts](src/tools/os/shell.test.ts), the string-body rule in [src/llm/grammar/build-grammar.test.ts](src/llm/grammar/build-grammar.test.ts), [src/tools/os/fs-replace-guard.test.ts](src/tools/os/fs-replace-guard.test.ts) and the restore cases in [src/tools/os/os-tools.test.ts](src/tools/os/os-tools.test.ts). +### A command still running at the default timeout is detached, not killed + +`os.shell.run` without `timeoutMs` used to run unbounded — a recursive grep over a home directory ran for twenty minutes until a person killed it, and `agent.toolTimeoutMs` never applied to the shell. Since config v67 the operator's `tools.shell.defaultTimeoutMs` (600 000) bounds the **wait**, not the command: when it elapses the call returns `ok` with `details.detached: true`, `details.jobId`, `details.pid`, the output so far, and a first line `still running after 10 min (job 3) — output so far below; os.shell.run {"wait": 3} keeps waiting (up to another 10 min per call), {"kill": 3} stops it, pass timeoutMs for a longer first wait`. A build the operator's default interrupted is not a build the model wanted stopped. The process keeps running in its own process group — [src/sandbox/command-job.ts](src/sandbox/command-job.ts) (`startCommandJob`) spawns it `detached` on POSIX so `-pid` reaches a subshell's `sleep 30 &` (the tree-kill on Windows), and captures stdout/stderr head + tail into 1 MiB per stream ([capped-output.ts](src/sandbox/capped-output.ts)); `runCommand` is untouched and still kills its direct child. The job forms of the same tool ([shell-job-calls.ts](src/tools/os/shell-job-calls.ts)): `{wait: id, timeoutMs?, keep?}` returns the exit as a fresh result would (`details.jobId` kept) or the same still-running result when the wait — the explicit `timeoutMs`, else the default — elapses; a **cancelled turn returns from a `wait` at once and leaves the job running** (the turn's end then stops it unless kept). `{kill: id}` is the group kill (`SIGTERM`, `SIGKILL` after 2 s), `details.killed: true` with the tail. `{jobs: true}` lists this session's jobs. Unknown keys stay refused (F40); a call that mixes forms is refused with the forms it named. The still-running and killed results show only the last lines of output because the result compressor keeps a twelve-line tail and the notice above the command line must be inside it. An **explicit** `timeoutMs` still kills at its limit: the model asked for a bound. The per-session registry is [shell-jobs.ts](src/tools/os/shell-jobs.ts), owned by the bootstrap: a job dies when its turn ends (`executeTurn`'s `finally`, the choke point every turn passes) unless the call that started it or a later `wait` carried `keep: true`; every job dies when its session is deleted, when `finish` completes it, at shutdown, and at `tools.shell.jobMaxMs` (3 600 000) from its start; at most `tools.shell.maxJobs` (3) run per session — the next detach stops the oldest un-kept job (the oldest kept one when all are kept) and the result says so. A fusion worker's jobs die with its turn regardless. A tool built without a registry (embedders, tests) gets a private one whose jobs die only at the ceiling. Not covered: a runtime killed with `SIGKILL` cannot stop its jobs — they run on until they exit or write to the closed pipe; the ceiling timer died with the process. Pinned by [src/tools/os/shell-detach.test.ts](src/tools/os/shell-detach.test.ts), [shell-jobs.test.ts](src/tools/os/shell-jobs.test.ts), [shell.test.ts](src/tools/os/shell.test.ts), [src/sandbox/command-job.test.ts](src/sandbox/command-job.test.ts) and [capped-output.test.ts](src/sandbox/capped-output.test.ts). + +### Reads are confined to the working directory + +Writes and commands were always gated; reads never were, and a bench run showed what that costs: a plain cloud session read a sibling run's solution, the harness's screen dumps and the benchmark's own checker from far outside its working directory. Since config v67 (`agent.readScope`, default `"working-dir"`) every session's filesystem reads are checked at the registry ([src/tools/read-scope/](src/tools/read-scope/), `confineReads`, installed by the bootstrap after the native tools, with the ladder's `DangerousToolOptions` — an install that passes `readScope` without `approvals` throws, so the scope cannot be half-wired into a silent refusal): a read-class call (`os.fs.read/list/grep/glob/diff/hash/watch/read_document/archive.*`, `vision.describe`, `verify.syntax`) whose target resolves outside the **working directory, the paths the user named and the directories approved so far** is **asked about through the approval ladder** ([read-scope-approval.ts](src/tools/read-scope/read-scope-approval.ts), category `fs_read_outside`, label `read outside the working directory`, pinned at level 5 like `trust_config` and `other`, grantable) — the same gate, router and surfaces (TUI modal, CLI stdin prompt, Telegram bridge) every other gated action uses. The prompt names the path, the working directory and what a yes means: `read — outside the working directory (); approving allows reads under for the rest of this session`. **A `y` is one question per place, not per file**: it widens the session's read roots to the directory the call named (or the parent of the file it named — `widenedReadRoot`; a path absent from disk counts as a file), remembered on the gate in `ReadScopeGrants` ([src/approval/read-scope-grants.ts](src/approval/read-scope-grants.ts)), keyed by session id and dropped by `clearSessionGrants` with the category grants, so the next read under it runs unasked; `[s]` grants the category, which is "read anywhere this session"; a `n` answers the model with the refusal it would have got before — one line, `reads are confined to the working directory () and the paths the user named; ask the user to name or to set agent.readScope: unrestricted` — never a throw, so it costs one step and reads as an instruction. Level 5 / `--no-approval` never asks. Questions are asked **one at a time per session** (a per-session queue inside `ReadOutsideApprover`): read tools run in parallel inside a batch, and a second prompt raised while the first waits would strand one of them on a surface that shows a single pending request; the second read waits for the answer and re-checks against the widened roots, so three reads under one new directory ask once. "Named" means an absolute or `~`-prefixed path in the user's **own** messages (`userNamedPaths`, recomputed from the transcript every step by the step executor and handed down as `ToolContext.readRoots` — a named file widens to that file, a named directory to that directory, and a path named mid-turn counts on the next call); nothing the model wrote widens the scope, so a path the user described but did not name ("my Downloads folder") is asked about, which is the intended behaviour. Lexical or canonical (realpath) containment both count, so a symlinked checkout does not question a path that is really inside. The OS temp directory (`os.tmpdir()`, plus `/tmp` and `/var/tmp` on POSIX) is **scratch space and always in scope** for a session, fs and shell alike — a helper written to `/tmp` and run, an output redirected there and read back, are the model's own work; the threat was other users' homes and a benchmark's sibling trees, which the home rule and the `..` rule cover. `os.shell.run` gets the same scope as a **narrow token check** before its own guard runs (`read-scope-shell.ts`): a whitespace token of `cmd` / `args` / an `sh -c` body (plus `cwd` and a `--flag=path` value) that is an absolute path under the user's home or the directory homes live in, and lies outside every root, asks under the same category with the command as the preview (`run \`grep -rn x \` — reads outside the working directory (): ; approving allows reads under …`); a `..` climb that escapes every root likewise; `/usr /bin /sbin /opt /dev /etc /System /Library /Applications` (`C:\Windows`, `C:\Program Files`) and the temp directory are never questioned; nothing else about shell commands changes — no command allowlist, no parsing beyond tokens, and the shell's own `shell` prompt (level 4) still follows when the guard wants it. The batch planner's per-tool category table (`APPROVAL_CATEGORIES_BY_TOOL`) deliberately does **not** list `fs_read_outside` under `os.shell.run` / `verify.run`: doing so would pin every shell batch solo below level 5 for a check that fires only when a command names an outside path, and a denial there changes no state. Fusion workers keep their older, narrower rule (working directory plus the fan-out's write scope, never the brief, no scratch allowance on reads) as a **hard refusal** for reads and shell alike — an ephemeral session has nobody to ask, and `WORKER_READ_REFUSAL_REASON` stays. The operator's opt-out is one line, `agent.readScope: "unrestricted"`, read per call so it needs no restart, under which nothing asks. Pinned by [src/tools/read-scope/read-scope-approval.test.ts](src/tools/read-scope/read-scope-approval.test.ts) (asks with the path; a yes widens; a no refuses with the sentence; `[s]`; level 5; `unrestricted`; workers; shell; parallel reads ask once; aborted turn; install without a ladder throws), [read-scope.test.ts](src/tools/read-scope/read-scope.test.ts), [read-scope-roots.test.ts](src/tools/read-scope/read-scope-roots.test.ts), [read-scope-shell.test.ts](src/tools/read-scope/read-scope-shell.test.ts), [src/approval/read-scope-grants.test.ts](src/approval/read-scope-grants.test.ts) and the unchanged worker cases in [worker-read-scope.test.ts](src/tools/read-scope/worker-read-scope.test.ts). + ### Locked invariants (pinned by tests) Pinned by [src/agent/batch-executor.test.ts](src/agent/batch-executor.test.ts), [src/agent/step-executor.test.ts](src/agent/step-executor.test.ts), [src/agent/parallel-tool-calls.integration.test.ts](src/agent/parallel-tool-calls.integration.test.ts), [src/agent/loop-detector.test.ts](src/agent/loop-detector.test.ts), [src/llm/grammar/tool-call-grammar.test.ts](src/llm/grammar/tool-call-grammar.test.ts), [src/llm/grammar/build-grammar.test.ts](src/llm/grammar/build-grammar.test.ts), [src/tracing/trace/trace-recorder.test.ts](src/tracing/trace/trace-recorder.test.ts): @@ -441,7 +449,7 @@ The TUI is clickable. Ink has no mouse layer, so this is built in `src/tui/mouse | `src/tools/` | Tool registry + individual tools. OS tools: `shell.run` (direct-exec by default; routes to a `sh -c` subshell when `needsShellInterpretation` sees shell metacharacters `\| & ; > < $ \`` or a pre-joined command line in `cmd` with empty `args` — the common ENOENT trap where the model puts a whole command line in `cmd`; the guard still inspects a tokenised view of the full line so hardline/dangerous rules match), `fs.read` (w/ `offset`/`limit`/`lineNumbers`), `fs.write`, `fs.list`, `fs.glob`, `fs.locate_project` (fuzzy project-name → directory over bounded sources, see §"Project path resolution"), `fs.grep` (bundled ripgrep), `fs.edit` (atomic string replace), `fs.read_document` (PDF/DOCX/XLSX/RTF/ODT/PPTX/legacy .doc → plain text via pure-JS), `fs.archive.list` / `fs.archive.read_entry` / `fs.archive.extract` (zip/tar/tar.gz/gz via pure-JS; zip-slip + bomb guards), `fs.hash` (md5/sha1/sha256/sha512 streaming), `fs.diff` (unified diff, jsdiff), `fs.patch` (dry-run default, all-or-nothing apply), `fs.watch` (chokidar one-shot, timeout-capped), `git.status` / `git.log` / `git.diff` / `git.show` / `git.blame` / `git.branch` (read-only shell-out with structured parse), `git.init` / `git.add` / `git.checkout` / `git.commit` / `git.push` and the network verbs `git.remote` / `git.fetch` / `git.pull` / `git.clone` (writes; local ones ask like a file write in the repository, `commit` forces `-c commit.gpgsign=false` because the agent has no terminal for a pinentry; the network ones need Remote sync on and ask under `git_remote`, see §"GitHub integration"), `proc.list` / `proc.kill` (ps/tasklist + approval), `http.request` (curl + host allowlist + `config.http.approvalMode`), `web.search` (configured provider; keyless Exa with a DuckDuckGo fallback by default, SearXNG/Brave selectable via `web.search.*`; Exa/Brave use an env API key when present, see §"Web search reliability"), `web.fetch` (read a known URL as markdown/text), `clipboard.*`, `window.*`, `notify`, `email.inbox` / `email.send` (the agent's own Atomic Mail inbox; sending is approval-gated, see §"Atomic Mail"). | | `src/compressor/` | Result compressor, log summariser | | `src/sandbox/` | git worktree + sandboxed command runner | -| `src/approval/` | Approval gate and event wiring | +| `src/approval/` | Approval gate and event wiring: the ladder (`approval-level`), the gate and its session grants (`approval-gate`), the per-session router, `requireApproval`, the fan-out write scope (`fanout-scope`) and the read-scope grants a `fs_read_outside` yes leaves behind (`read-scope-grants`) | | `src/tracing/` | Structured logger + metrics + trace recorder (`src/tracing/trace/`) | | `src/replay/` | Trace-based replay: drift detection + optional LLM re-inference | | `src/memory/` | Memory fabric: ProfileStore (key/value facts, pinned + contextual) + MemoryStore (FTS5 freeform notes) + async end-of-turn reflection that writes into both. See [MEMORY.md](MEMORY.md). | @@ -463,6 +471,7 @@ The TUI is clickable. Ink has no mouse layer, so this is built in `src/tui/mouse | `src/atomic-mail/` | The agent's own `@atomicmail.ai` inbox (atomicmail.ai, JMAP over HTTPS). `atomic-mail-auth` (scrypt proof-of-work with the service's fixed salt; challenge → session (1 h) → capability (2 min) JWTs), `atomic-mail-client` (JMAP session discovery, send, inbox), `atomic-mail-batches` (the pure JMAP method batches), `atomic-mail-store` (`ATOMIC_MAIL_API_KEY` in `.env`, the session cache at `/atomic-mail/session.json` 0600, the `atomicMail` config block, v53), `atomic-mail-service` (register / owner-code / verify / send / inbox — the one object every caller uses), `templates/` (the CRT-styled mails: a 5×5 block-glyph font, tables and inline styles only — inboxes strip everything else). Hub tenant: `src/integrations/atomic-mail-integration.ts` (`r` registers, saving *Your e-mail* mails a code, *Verification code* is a `transient` field the orchestrator checks and never stores — the code is kept as a sha256 with a 10-minute expiry). The owner's address is only ever mailed once they typed the code back. See §"Atomic Mail". | | `src/notifications/` | Out-of-band pings that need no channel loop: `download-notifier` sends one plain-text message (Telegram via the fetch-backed `one-shot-sender`, Discord via `DiscordApi`) when a background model download ends, from inside the detached worker. Reads the bot tokens from the process env that `loadConfig` fills from `/.env`; never logs them (`scrubErrorMessage`). The request rides in `/downloads/.notify`, the outcome in the record's `notified` block. Remembered answer: config v52 `notifications.downloads.channel`. | | `src/tools/fusion/` | The fusion fan-out: `worker-tool-policy` (what a worker may call + the approval refusal), `delegate-args` (argument caps), `worker-prompt` (the worker's brief), `worker-runner` (the bounded pool of ephemeral worker turns), `worker-result` (per-task status, tally, rendering), `fusion-delegate` (the `fusion.delegate` tool). Progress lines are rendered by `src/tui/format-fusion-worker-line.ts`. See §"Run modes (Local / Cloud / Fusion)". | +| `src/tools/read-scope/` | Where a session may read (config v67, `agent.readScope`): `read-scope-targets` (which tools read, and which argument names what), `read-scope-roots` (`userNamedPaths` — the absolute / `~` paths parsed out of the user's own messages), `read-scope` (the session and worker checks + the refusals), `read-scope-approval` (`ReadOutsideApprover` — the `fs_read_outside` prompt, the per-session question queue, the `y` that widens the roots), `read-scope-shell` (the narrow token check for `os.shell.run`), `confine-reads` (the registry install; `confineWorkerReads` is the worker-only install kept under its old name). The remembered directories live on the gate (`src/approval/read-scope-grants.ts`). See §"Reads are confined to the working directory". | | `src/tui/mouse/` | TUI mouse layer: `mouse-tracking` (1000+1006 enable/disable), `parse-mouse-events` (SGR + legacy X10 decoder), `mouse-stdin` (splits mouse bytes out of the stream Ink reads), `mouse-registry` (Yoga-based hit testing), `selection-passthrough` / `drag-intent` (hands the terminal its drag-to-select back for a 10 s window), `mouse-context` / `mouse-list-row` (React glue + the shared click-to-select-then-activate row), `synthetic-key` (wheel/second-click → the panel's own key handler). See §"Mouse support". | ## Atomic Mail @@ -1511,7 +1520,7 @@ The ladder (`agent.approvalLevel`, config v37; the binary `agent.approvalRequire | 2 | workspace | `fs_write_workspace`: `os.fs.{write,edit,patch,restore}` strictly inside the session cwd (realpath containment via [src/tools/os/fs-approval-scope.ts](src/tools/os/fs-approval-scope.ts); symlinks pointing outside are classified by their target); `os.git.{init,add,commit,checkout}` on a repository rooted inside the workspace ride the same category | | 3 | home | + `fs_write_home` (writes anywhere under the home directory, plus `os.fs.archive.extract` even into the workspace), `fs_trash`, `http` (SSRF guard is not part of the gate and stays on) | | 4 | operator | + `shell` (guard verdict `approval_required` only), `script` (`skill.run_script`), `proc_kill`, `git_remote` (`os.git.{push,pull,fetch,clone}` and `os.git.remote add|set-url|remove` — refused outright before the ladder while `git.remoteSync` is off) | -| 5 | full trust | everything, including `browser_nonweb` (file://, javascript:), `trust_config`, and `other` | +| 5 | full trust | everything, including `browser_nonweb` (file://, javascript:), `trust_config`, `fs_read_outside` (a read or shell path outside the working directory and the paths the user named — §"Reads are confined to the working directory"), and `other` | Categorisation lives at the call sites (fs tools resolve workspace/home/outside from the target path + `ctx.workingDir`); a write outside both the workspace and home maps to `other`, which asks on every level except 5 — the conservative default for anything a call site cannot place. Hardline shell-guard rules fire before the gate and block at **every** level, and so does the guard's **policy layer** — operator refusals injected by the bootstrap, today only the git remote-sync switch (`git.remoteSync`, see §"Integrations hub" invariant 12), which blocks `git push|fetch|pull|clone` through the shell while it is off. MCP tools from a server at the default `approval_gated` trust go through the gate as category `other` (tools advertising `annotations.readOnlyHint === true` at discovery are exempt); `pure_read` servers bypass it — see §"MCP client". @@ -2530,7 +2539,7 @@ On the fusion route the strip walks **four** controls, not three: `backend ⇄ p The fourth control, `workers` ([src/tui/composer-switch/composer-switch-worker-rows.ts](src/tui/composer-switch/composer-switch-worker-rows.ts)), is the **local** half: the downloaded models the workers can run, then `1..8 workers`. A model row goes through `LocalModelsOrchestrator.setActive`, which restarts the managed daemon and writes only `localModels.*` — never `activeTextProvider` — so fusion survives the pick; `triggerLlmPrimary` is deliberately not used, because its local branch also makes `local-llama` the active text provider. Row `active` is read from `localModelsPanel.rows[].active` (which model the daemon serves), not from the LLM pane's row, whose `active` additionally requires `local-llama` to be the chat route — under fusion it never is. A count row goes through `setFusionWorkersInConfig`, which moves `llm.runMode.fusion.workers` and `localModels.managed.parallel` in one write: they are one fact seen from two sides, and a running daemon keeps its old slot count until restarted, which the orchestrator says in a notice. `/runmode workers N` is the same call. ### The orchestrator and its workers -`fusion.delegate` ([src/tools/fusion/](src/tools/fusion/)) is the tool that makes the mode more than a label. The orchestrator plans, then hands independent parts down in one call: `{ tasks: [{ id, instructions, title?, deliverable?, files? }] (1..8), maxWorkers?, contract? }` (`title` defaults to the humanised id). The optional `contract` ([contract.ts](src/tools/fusion/contract.ts): `owners` path → task, `provides` — a `symbol` / `file` / `id` / `endpoint` / `env` / `flag` / `other` a task must produce, `requires`, and `checks` as `verify.run` specs) is the interface between the parts: it is validated against the task ids (≤ 64 provides, ≤ 16 checks, ≤ 8,000 rendered chars), and the gate in front of a slow model refuses only what cannot run at all — no tasks, empty instructions, a `provides` on an unknown task — in ONE refusal that names every problem of the call (up to `MAX_REPORTED_PROBLEMS`, 32), so a local orchestrator pays one regeneration rather than one per field (live, four consecutive refusals cost ~20 minutes of generation before any worker ran); a `requires` that nothing provides and a `provides` with nowhere to be looked for are warnings, carried into every brief's CONTRACT block and the result's `contract:` line, never refusals, prepended to every brief with a per-task "You own / You provide / You may rely on", and after the fan-out each provide is checked for presence by language-agnostic means and the checks run through the injected `runChecks` seam ([contract-checks.ts](src/tools/fusion/contract-checks.ts)) — a missing provide is a note on its owner's row and a `contract:` line in the status table; a failing declared check makes its task `failed`; checks with no runner wired are reported as not run, never as passed. `maxWorkers` has no parser ceiling — an over-ambitious number runs as wide as the machine allows rather than coming back as a validation error the model has to notice and retry; only a value below one is invalid. Each task becomes one ephemeral worker session running a turn pinned to the local leg — `origin: "fusion"`, `maxSteps: workerMaxSteps`, `taskMaxDurationMs: workerTimeoutMs`, `toolFilter: isWorkerVisibleTool`, an approval policy of `refuse` — and the call returns every reply plus a per-task status (`ok` / `no_changes` / `failed` / `cancelled` / `max_steps` / `needs_orchestrator`). The brief ([worker-prompt.ts](src/tools/fusion/worker-prompt.ts)) tells the worker it has no memory of the parent conversation, that it must never ask a question, and that a refused approval is handed back up rather than retried. It also quotes the operator's **original request** above the task, labelled "context only; your task is below" and clipped at `ORIGINAL_REQUEST_CHAR_BUDGET` (16,000 chars) with an explicit note: orchestrator briefs are summaries, and thin ones made workers build the wrong thing or scavenge the disk for the spec. `executeTurn` records the request per session for the running turn (`pickOriginalRequest` — the turn's message, plus the message before it when the turn started with a short follow-up like "continue") and the tool reads it through `resolveOriginalRequest`. With the spec carried that way, `instructions` is capped at 32,000 chars, and the rejection names the limit and the exact overage. Statuses are ground-truthed rather than taken from the reply: a `reply` on the loop's forced finalization step comes back with `RunTurnResult.stopCause` and is reported `max_steps`, as is a worker stopped by its own `workerTimeoutMs` (which is not a cancellation); an `ok` task whose `files` entry does not exist afterwards is downgraded to `failed` ([declared-files.ts](src/tools/fusion/declared-files.ts)), an existing but untouched entry is only a note, since `files` may be inputs, and an `ok` task that declared `files` but made no successful write / edit / patch call and changed none of them on disk is `no_changes` (a task without `files` never is). A failed worker's loop/provider error lands on the task's head line with a remediation hint for the recognised classes (context exceeded, first-token/idle timeout, 402/429). A worker's filesystem **reads** are confined to its working directory plus the fan-out's write scope ([worker-read-scope.ts](src/tools/fusion/worker-read-scope.ts), installed on the registry by `confineWorkerReads` at boot); other sessions are unaffected. `### fusion` in the stable prefix ([src/prompt/fusion-guidance.ts](src/prompt/fusion-guidance.ts)) is what makes a cloud model reach for the tool at all — it is present only while the descriptor is mounted, so a non-fusion install's prefix is byte-identical to a build without the feature. It carries two things the orchestrator cannot get anywhere else: a push to delegate whenever the work splits into independent, self-contained parts (with the honest limit — a part that only makes sense with this conversation in front of it stays the orchestrator's), and one line of **machine facts** ([fusion-machine-facts.ts](src/prompt/fusion-machine-facts.ts)) so the width it picks is informed rather than guessed: the llama-server request slots, the local model behind them, and therefore how many workers run at once before the rest queue. Those facts are read from the already-loaded config by `buildPrompt` — no probe at prompt-build time — and a fact the runtime does not own (an `external` server's `--parallel`) is left unsaid rather than guessed, because a guessed number is one the model will plan against. They are config values, so they move only when the operator writes the config file, which is the same event that already flips the descriptor gate; nothing per-turn goes in that line or the KV cache would drop on every step. +`fusion.delegate` ([src/tools/fusion/](src/tools/fusion/)) is the tool that makes the mode more than a label. The orchestrator plans, then hands independent parts down in one call: `{ tasks: [{ id, instructions, title?, deliverable?, files? }] (1..8), maxWorkers?, contract? }` (`title` defaults to the humanised id). The optional `contract` ([contract.ts](src/tools/fusion/contract.ts): `owners` path → task, `provides` — a `symbol` / `file` / `id` / `endpoint` / `env` / `flag` / `other` a task must produce, `requires`, and `checks` as `verify.run` specs) is the interface between the parts: it is validated against the task ids (≤ 64 provides, ≤ 16 checks, ≤ 8,000 rendered chars), and the gate in front of a slow model refuses only what cannot run at all — no tasks, empty instructions, a `provides` on an unknown task — in ONE refusal that names every problem of the call (up to `MAX_REPORTED_PROBLEMS`, 32), so a local orchestrator pays one regeneration rather than one per field (live, four consecutive refusals cost ~20 minutes of generation before any worker ran); a `requires` that nothing provides and a `provides` with nowhere to be looked for are warnings, carried into every brief's CONTRACT block and the result's `contract:` line, never refusals, prepended to every brief with a per-task "You own / You provide / You may rely on", and after the fan-out each provide is checked for presence by language-agnostic means and the checks run through the injected `runChecks` seam ([contract-checks.ts](src/tools/fusion/contract-checks.ts)) — a missing provide is a note on its owner's row and a `contract:` line in the status table; a failing declared check makes its task `failed`; checks with no runner wired are reported as not run, never as passed. `maxWorkers` has no parser ceiling — an over-ambitious number runs as wide as the machine allows rather than coming back as a validation error the model has to notice and retry; only a value below one is invalid. Each task becomes one ephemeral worker session running a turn pinned to the local leg — `origin: "fusion"`, `maxSteps: workerMaxSteps`, `taskMaxDurationMs: workerTimeoutMs`, `toolFilter: isWorkerVisibleTool`, an approval policy of `refuse` — and the call returns every reply plus a per-task status (`ok` / `no_changes` / `failed` / `cancelled` / `max_steps` / `needs_orchestrator`). The brief ([worker-prompt.ts](src/tools/fusion/worker-prompt.ts)) tells the worker it has no memory of the parent conversation, that it must never ask a question, and that a refused approval is handed back up rather than retried. It also quotes the operator's **original request** above the task, labelled "context only; your task is below" and clipped at `ORIGINAL_REQUEST_CHAR_BUDGET` (16,000 chars) with an explicit note: orchestrator briefs are summaries, and thin ones made workers build the wrong thing or scavenge the disk for the spec. `executeTurn` records the request per session for the running turn (`pickOriginalRequest` — the turn's message, plus the message before it when the turn started with a short follow-up like "continue") and the tool reads it through `resolveOriginalRequest`. With the spec carried that way, `instructions` is capped at 32,000 chars, and the rejection names the limit and the exact overage. Statuses are ground-truthed rather than taken from the reply: a `reply` on the loop's forced finalization step comes back with `RunTurnResult.stopCause` and is reported `max_steps`, as is a worker stopped by its own `workerTimeoutMs` (which is not a cancellation); an `ok` task whose `files` entry does not exist afterwards is downgraded to `failed` ([declared-files.ts](src/tools/fusion/declared-files.ts)), an existing but untouched entry is only a note, since `files` may be inputs, and an `ok` task that declared `files` but made no successful write / edit / patch call and changed none of them on disk is `no_changes` (a task without `files` never is). A failed worker's loop/provider error lands on the task's head line with a remediation hint for the recognised classes (context exceeded, first-token/idle timeout, 402/429). A worker's filesystem **reads** are confined to its working directory plus the fan-out's write scope ([src/tools/read-scope/](src/tools/read-scope/), `checkWorkerRead`, installed on the registry by `confineReads` at boot) — narrower than the session scope every other session gets (§"Reads are confined to the working directory"), and never widened by the brief, which is model output. `### fusion` in the stable prefix ([src/prompt/fusion-guidance.ts](src/prompt/fusion-guidance.ts)) is what makes a cloud model reach for the tool at all — it is present only while the descriptor is mounted, so a non-fusion install's prefix is byte-identical to a build without the feature. It carries two things the orchestrator cannot get anywhere else: a push to delegate whenever the work splits into independent, self-contained parts (with the honest limit — a part that only makes sense with this conversation in front of it stays the orchestrator's), and one line of **machine facts** ([fusion-machine-facts.ts](src/prompt/fusion-machine-facts.ts)) so the width it picks is informed rather than guessed: the llama-server request slots, the local model behind them, and therefore how many workers run at once before the rest queue. Those facts are read from the already-loaded config by `buildPrompt` — no probe at prompt-build time — and a fact the runtime does not own (an `external` server's `--parallel`) is left unsaid rather than guessed, because a guessed number is one the model will plan against. They are config values, so they move only when the operator writes the config file, which is the same event that already flips the descriptor gate; nothing per-turn goes in that line or the KV cache would drop on every step. **Nothing about the mode is decided at boot.** The tool is registered unconditionally, because its own live `resolveRunMode()` refusal is the correct and only gate it needs; and `bootstrap.ts` resolves the descriptor gate (`fusion: { enabled: … }`) on *every* read of `effectiveToolDescriptors()`, memoised on the gate's own value so the array identity — and therefore the prefix bytes — only changes when the mode does. Both were boot-time `if`s once, and the pair made a mid-session switch inert: the operator got the chip, the tint and the config, no `fusion.delegate`, no `### fusion`, and a fusion mode that silently did nothing until a restart. When the gate does flip, that session's KV cache drops once — the same cost, for the same reason, as installing a skill or live-adding an MCP server (`refreshMcp`): the tool catalog changed, so the prefix must. The GBNF grammar's `tool-name` rule lists `fusion.delegate` unconditionally (a local orchestrator has to be able to emit it — see the comment in `grammars/tool-call.gbnf`); a worker's per-request grammar drops it again through `toolFilter`, and the tool refuses from a worker session in any case. diff --git a/src/agent/batch-executor.test.ts b/src/agent/batch-executor.test.ts index be386190..53338b7d 100644 --- a/src/agent/batch-executor.test.ts +++ b/src/agent/batch-executor.test.ts @@ -115,6 +115,34 @@ describe("executeBatch", () => { expect(elapsed).toBeLessThan(250); }); + it("hands the step's readRoots to every call's tool context, unchanged", async () => { + // The read scope (`src/tools/read-scope/`) widens by what the user + // named; the step computes that once and the batch must not lose it. + const seen: (readonly string[] | undefined)[] = []; + const registry = new ToolRegistry(); + registry.register({ + name: "os.fs.read", + description: "read", + readonly: true, + run: async (_args, toolCtx) => { + seen.push(toolCtx.readRoots); + return okResult("os.fs.read"); + }, + }); + const inputs = toBatchInputs([ + { tool: "os.fs.read", args: { path: "a" } }, + { tool: "os.fs.read", args: { path: "b" } }, + ]); + const ctrl = new AbortController(); + await executeBatch(inputs, registry, { + ...ctx(ctrl.signal), + readRoots: ["/named/one"], + }); + expect(seen).toEqual([["/named/one"], ["/named/one"]]); + await executeBatch(inputs.slice(0, 1), registry, ctx(ctrl.signal)); + expect(seen[2]).toBeUndefined(); + }); + it("chunks pure_read fan-out into bounded waves when maxWaveSize is set", async () => { // 5 reads with a wave size of 2 → waves of [0,1], [2,3], [4]. Track // peak concurrency: it must never exceed 2, and all 5 must run. @@ -1613,12 +1641,12 @@ describe("executeBatch refuses a call with unknown argument keys (F40)", () => { const result = out.results[0]!.compressed!; expect(result.status).toBe("error"); expect(result.summary).toBe( - 'unknown argument `-e` for os.shell.run (expected: cmd, args, cwd, timeoutMs; put the script in args: ["-c", "…"]) — the call was not run; re-emit it with the right keys', + 'unknown argument `-e` for os.shell.run (expected: cmd, args, cwd, timeoutMs, keep, wait, kill, jobs; put the script in args: ["-c", "…"]) — the call was not run; re-emit it with the right keys', ); expect(result.summary).not.toContain("rename"); expect(result.details).toEqual({ unknownKeys: ["-e"], - expectedKeys: ["cmd", "args", "cwd", "timeoutMs"], + expectedKeys: ["cmd", "args", "cwd", "timeoutMs", "keep", "wait", "kill", "jobs"], }); expect(out.cancelled).toBe(false); }); @@ -1638,7 +1666,7 @@ describe("executeBatch refuses a call with unknown argument keys (F40)", () => { ); expect(run).not.toHaveBeenCalled(); expect(out.results[0]!.compressed!.summary).toBe( - "unknown argument `-args` for os.shell.run (expected: cmd, args, cwd, timeoutMs; did you mean `args`?) — the call was not run; re-emit it with the right keys", + "unknown argument `-args` for os.shell.run (expected: cmd, args, cwd, timeoutMs, keep, wait, kill, jobs; did you mean `args`?) — the call was not run; re-emit it with the right keys", ); }); diff --git a/src/agent/batch-executor.ts b/src/agent/batch-executor.ts index 000f4e16..7a279cc9 100644 --- a/src/agent/batch-executor.ts +++ b/src/agent/batch-executor.ts @@ -107,6 +107,12 @@ export interface BatchExecutionContext { sessionId: string; stepIndex: number; signal: AbortSignal; + /** + * The paths the user named in this session's messages, for the read + * scope (`ToolContext.readRoots`). Computed by the step from the + * transcript and handed to every call of the batch unchanged. + */ + readRoots?: readonly string[]; /** * Fired immediately before the registry is invoked for each call. * Order: matches the order the executor reaches each call (within a @@ -421,6 +427,7 @@ export async function executeBatch( stepIndex: ctx.stepIndex, signal: ctx.signal, ...(ctx.toolRole !== undefined ? { toolRole: ctx.toolRole } : {}), + ...(ctx.readRoots !== undefined ? { readRoots: ctx.readRoots } : {}), }); } catch (err) { if (ctx.signal.aborted) { diff --git a/src/agent/step-executor.ts b/src/agent/step-executor.ts index 04172f54..89662cf6 100644 --- a/src/agent/step-executor.ts +++ b/src/agent/step-executor.ts @@ -101,6 +101,7 @@ import { toolResultTurn, } from "../session/conversation-turn.js"; import type { ToolRegistry } from "../tools/tool-registry.js"; +import { userNamedPaths } from "../tools/read-scope/index.js"; import { hashPrefix, type SlotManager } from "../llm/slot-manager.js"; import { NO_SERVER_TEMPLATE, @@ -1487,12 +1488,17 @@ async function executeStepInner( // these is short-circuited inside `executeBatch` with a terse pointer // instead of re-reading and re-dumping the body. const loadedSkillNames = new Set(ctx.session.loadedSkills.map((s) => s.name)); + // The paths the user named so far, for the read scope: re-read from the + // transcript every step so a path named mid-turn (steering) counts on + // the next call, and nothing the model wrote ever widens it. + const readRoots = userNamedPaths(ctx.session.turns); const runBatch = runInOrder ? executeCallsInOrder : executeBatch; const batchOutcome = await runBatch(inputs, deps.registry, { workingDir: ctx.session.workingDir, sessionId: ctx.session.id, stepIndex: ctx.stepIndex, signal: ctx.signal, + ...(readRoots.length > 0 ? { readRoots } : {}), ...(deps.tracker ? { tracker: deps.tracker } : {}), ...(ctx.terminalOnly ? { terminalOnly: true } : {}), ...(deps.isPlanMode ? { isPlanMode: deps.isPlanMode } : {}), diff --git a/src/approval/approval-gate.ts b/src/approval/approval-gate.ts index fdd5de65..28e31c00 100644 --- a/src/approval/approval-gate.ts +++ b/src/approval/approval-gate.ts @@ -1,4 +1,5 @@ import { FanoutScopeRegistry } from "./fanout-scope.js"; +import { ReadScopeGrants } from "./read-scope-grants.js"; import { randomUUID } from "node:crypto"; import { clampApprovalLevel, @@ -164,6 +165,15 @@ export class ApprovalGate { */ readonly fanoutScopes = new FanoutScopeRegistry(); + /** + * Directories a session may read in without asking again — the `y` + * of an `fs_read_outside` prompt, remembered per session (see + * `read-scope-grants.ts`). A session grant in every sense but its + * unit (a path, not a category), so it lives here and is dropped by + * `clearSessionGrants` with the rest. + */ + readonly readScopeGrants = new ReadScopeGrants(); + constructor(options: { emit: ApprovalEmitter; level?: ApprovalLevel }) { this.emitter = options.emit; this.level = options.level ?? MIN_APPROVAL_LEVEL; @@ -207,6 +217,7 @@ export class ApprovalGate { * standing level is untouched: it is a durable posture, grants are not. */ clearSessionGrants(sessionId?: string): void { + this.readScopeGrants.clear(sessionId); if (sessionId === undefined) { this.grantsBySession.clear(); return; diff --git a/src/approval/approval-level.test.ts b/src/approval/approval-level.test.ts index 72d82674..08ceda57 100644 --- a/src/approval/approval-level.test.ts +++ b/src/approval/approval-level.test.ts @@ -27,6 +27,7 @@ const ALL_CATEGORIES: readonly ApprovalCategory[] = [ "browser_nonweb", "trust_config", "email", + "fs_read_outside", "other", ]; @@ -66,6 +67,9 @@ describe("approval ladder", () => { browser_nonweb: 5, trust_config: 5, email: 5, + // Reads outside the working directory ask until full trust: a + // wandering read is what the read scope exists to stop. + fs_read_outside: 5, other: 5, }; for (const [category, from] of Object.entries(silentFrom) as [ @@ -94,6 +98,9 @@ describe("approval ladder", () => { expect(formatApprovalCategory("trust_config")).toBe("agent trust config"); expect(formatApprovalCategory("shell")).toBe("shell command"); expect(formatApprovalCategory("git_remote")).toBe("git · remote"); + expect(formatApprovalCategory("fs_read_outside")).toBe( + "read outside the working directory", + ); }); it("level 1 asks for every category and level 5 for none (cumulative ladder)", () => { @@ -109,6 +116,7 @@ describe("approval ladder", () => { "browser_nonweb", "trust_config", "email", + "fs_read_outside", "other", ]; for (const category of categories) { diff --git a/src/approval/approval-level.ts b/src/approval/approval-level.ts index f5be206e..6c1fe4af 100644 --- a/src/approval/approval-level.ts +++ b/src/approval/approval-level.ts @@ -48,6 +48,16 @@ export type ApprovalCategory = * unrelated prompt should be able to silence it. */ | "fusion_fanout" + /** + * A filesystem read (or a shell command naming a path) outside the + * session's working directory and the paths the user named + * (`src/tools/read-scope/`). Pinned at level 5: wandering reads are + * what the scope exists to stop, so nothing short of full trust runs + * them unasked. A `y` at the prompt also widens the session's read + * roots to the directory it named (`ReadScopeGrants`), so one answer + * covers the reads that follow under it. + */ + | "fs_read_outside" | "other"; /** @@ -67,8 +77,8 @@ export type ApprovalCategory = * stricter than the escape hatch. The remote-sync switch * (`git.remoteSync`) is checked before this ladder is consulted. * - level 5 (full trust): everything, including browser navigation to - * non-web URLs, writes to the agent's own trust config, and - * uncategorised requests. + * non-web URLs, writes to the agent's own trust config, reads outside + * the working directory, and uncategorised requests. * * `trust_config` is deliberately pinned at 5: a write to the file that * holds `agent.approvalLevel` (or the `.env` holding API tokens) is the @@ -91,6 +101,7 @@ const AUTO_APPROVE_FROM_LEVEL: Record = { browser_nonweb: 5, trust_config: 5, email: 5, + fs_read_outside: 5, other: 5, }; @@ -154,6 +165,10 @@ const GRANTABLE_CATEGORY: Record = { // A session grant would let the agent mail anyone for the rest of // the session; each mail is its own decision. email: false, + // "Read anywhere this session" is a legitimate answer for an operator + // who would otherwise set `agent.readScope: unrestricted`; the + // narrower answer, one directory, is the prompt's plain `y`. + fs_read_outside: true, other: true, }; @@ -182,6 +197,7 @@ export const APPROVAL_CATEGORY_LABELS: Record = { browser_nonweb: "browser · non-web URL", trust_config: "agent trust config", email: "e-mail send", + fs_read_outside: "read outside the working directory", other: "uncategorised", }; diff --git a/src/approval/index.ts b/src/approval/index.ts index 1c93acf6..a65c26a5 100644 --- a/src/approval/index.ts +++ b/src/approval/index.ts @@ -24,6 +24,7 @@ export { resolveBootApprovalLevel, } from "./approval-level.js"; export type { ApprovalCategory, ApprovalLevel } from "./approval-level.js"; +export { ReadScopeGrants } from "./read-scope-grants.js"; export { ApprovalRouter } from "./approval-router.js"; export type { ApprovalHandler } from "./approval-router.js"; export { requireApproval, ApprovalDeniedError } from "./dangerous-tool.js"; diff --git a/src/approval/read-scope-grants.test.ts b/src/approval/read-scope-grants.test.ts new file mode 100644 index 00000000..870a7024 --- /dev/null +++ b/src/approval/read-scope-grants.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it } from "vitest"; + +import { ApprovalGate } from "./approval-gate.js"; +import { ReadScopeGrants } from "./read-scope-grants.js"; + +describe("ReadScopeGrants", () => { + it("remembers a directory per session and keeps sessions apart", () => { + const grants = new ReadScopeGrants(); + grants.widen("s-1", "/srv/homes/me/Desktop"); + expect(grants.rootsFor("s-1")).toEqual(["/srv/homes/me/Desktop"]); + expect(grants.rootsFor("s-2")).toEqual([]); + }); + + it("keeps the roots a disjoint set: a covered directory is a no-op, a wider one folds the narrower in", () => { + const grants = new ReadScopeGrants(); + grants.widen("s", "/srv/homes/me/Desktop"); + grants.widen("s", "/srv/homes/me/Desktop/reports"); + expect(grants.rootsFor("s")).toEqual(["/srv/homes/me/Desktop"]); + grants.widen("s", "/srv/homes/me"); + expect(grants.rootsFor("s")).toEqual(["/srv/homes/me"]); + // A sibling is its own root; a lookalike prefix is not containment. + grants.widen("s", "/srv/homes/me-backup"); + expect(grants.rootsFor("s")).toEqual(["/srv/homes/me", "/srv/homes/me-backup"]); + }); + + it("ignores a relative directory", () => { + const grants = new ReadScopeGrants(); + grants.widen("s", "Desktop"); + expect(grants.rootsFor("s")).toEqual([]); + }); + + it("clears one session or every session", () => { + const grants = new ReadScopeGrants(); + grants.widen("s-1", "/a"); + grants.widen("s-2", "/b"); + grants.clear("s-1"); + expect(grants.rootsFor("s-1")).toEqual([]); + expect(grants.rootsFor("s-2")).toEqual(["/b"]); + grants.clear(); + expect(grants.rootsFor("s-2")).toEqual([]); + }); + + it("is dropped by the gate's clearSessionGrants, both forms, like the category grants", () => { + const gate = new ApprovalGate({ emit: () => undefined }); + gate.readScopeGrants.widen("s-1", "/a"); + gate.readScopeGrants.widen("s-2", "/b"); + gate.clearSessionGrants("s-1"); + expect(gate.readScopeGrants.rootsFor("s-1")).toEqual([]); + expect(gate.readScopeGrants.rootsFor("s-2")).toEqual(["/b"]); + gate.clearSessionGrants(); + expect(gate.readScopeGrants.rootsFor("s-2")).toEqual([]); + }); +}); diff --git a/src/approval/read-scope-grants.ts b/src/approval/read-scope-grants.ts new file mode 100644 index 00000000..f624f8aa --- /dev/null +++ b/src/approval/read-scope-grants.ts @@ -0,0 +1,67 @@ +import { isAbsolute, resolve } from "node:path"; + +import { isInside } from "./fanout-scope.js"; + +/** + * Directories a session may READ in without being asked again. + * + * A read outside the working directory and the paths the user named + * asks through the ladder as `fs_read_outside` (`src/tools/read-scope/`). + * The operator's plain `y` is not a one-shot: it names a directory — + * the one the model reached for, or the parent of the file it reached + * for — and every later read under that directory in the same session + * runs unasked. One question per place, not one per file: a model + * summarising a folder of reports would otherwise ask once per report, + * and answering the same question fifty times is attrition, not + * consent. + * + * **Why this is not an `ApprovalGate` category grant.** A category + * grant (`[s]`) is "read anywhere this session" — the honest scope for + * an operator who would otherwise set `agent.readScope: unrestricted`, + * and it is offered too. This registry is the narrower answer, and it + * has to be keyed by a path, which the gate's grants have nowhere to + * hold (the same reason `fanout-scope.ts` gives). It lives on the gate + * beside them so it shares their lifetime: `clearSessionGrants` drops + * both when the operator leaves the session. + * + * The roots here join the session's working directory and user-named + * paths (`ToolContext.readRoots`) when the read scope is checked; they + * are never written back into the transcript, so a saved session starts + * its next run with only what the user named. + */ +export class ReadScopeGrants { + private readonly rootsBySession = new Map(); + + /** + * Remember that `sessionId` may read under `dir`. A directory already + * covered by an earlier root is a no-op; a root the new one covers is + * folded into it, so the list stays a set of disjoint roots. + */ + widen(sessionId: string, dir: string): void { + if (!isAbsolute(dir)) return; + const root = resolve(dir); + const roots = this.rootsBySession.get(sessionId) ?? []; + if (roots.some((known) => isInside(known, root))) return; + this.rootsBySession.set(sessionId, [ + ...roots.filter((known) => !isInside(root, known)), + root, + ]); + } + + /** The directories `sessionId` was granted, in the order they were. */ + rootsFor(sessionId: string): readonly string[] { + return this.rootsBySession.get(sessionId) ?? []; + } + + /** + * Drop a session's roots — or every session's, with no argument — the + * same two forms `ApprovalGate.clearSessionGrants` takes. + */ + clear(sessionId?: string): void { + if (sessionId === undefined) { + this.rootsBySession.clear(); + return; + } + this.rootsBySession.delete(sessionId); + } +} diff --git a/src/config/config-schema.test.ts b/src/config/config-schema.test.ts index 78ccf1de..71ebf7df 100644 --- a/src/config/config-schema.test.ts +++ b/src/config/config-schema.test.ts @@ -703,6 +703,39 @@ describe("parseUserConfigFile", () => { } }); + it("defaults agent.readScope to working-dir and accepts only the two scopes (v67)", () => { + expect( + parseUserConfigFile({ version: USER_CONFIG_VERSION }).agent.readScope, + ).toBe("working-dir"); + expect( + parseUserConfigFile({ + version: USER_CONFIG_VERSION, + agent: { readScope: "unrestricted" }, + }).agent.readScope, + ).toBe("unrestricted"); + // `null` is "absent" here, as for every other `agent.*` field. + for (const bad of ["everywhere", "", 1, true]) { + expect(() => + parseUserConfigFile({ + version: USER_CONFIG_VERSION, + agent: { readScope: bad }, + }), + ).toThrow(/agent.readScope/); + } + }); + + it("upgrades a v66 file to v67 with the confined read scope", () => { + // The default-behaviour change: an older file has no field and takes + // `working-dir`; what it did carry is kept. + const parsed = parseUserConfigFile({ + version: 66, + agent: { toolTimeoutMs: 45_000 }, + }); + expect(parsed.version).toBe(USER_CONFIG_VERSION); + expect(parsed.agent.readScope).toBe("working-dir"); + expect(parsed.agent.toolTimeoutMs).toBe(45_000); + }); + it("accepts conversationMaxTokens: 0 as the auto sentinel", () => { // `0` is not a request for a zero-token transcript: it is "let the // window decide", the same sentinel `localModels.managed.contextSize` @@ -1530,6 +1563,70 @@ describe("parseUserConfigFile", () => { ).toThrow(/projects\.roots/); }); + it("accepts a v66 file and fills in the tools.shell defaults transparently", () => { + // v67: an existing file has no `tools` block; it takes the defaults so + // an omitted `timeoutMs` detaches at ten minutes from the next start, + // a detached job dies within the hour, and three may run at once. + const parsed = parseUserConfigFile({ version: 66 }); + expect(parsed.version).toBe(USER_CONFIG_VERSION); + expect(parsed.tools).toEqual({ + shell: { defaultTimeoutMs: 600_000, jobMaxMs: 3_600_000, maxJobs: 3 }, + }); + }); + + it("preserves explicit tools.shell.jobMaxMs and maxJobs, and rejects non-positive ones", () => { + const pinned = parseUserConfigFile({ + version: USER_CONFIG_VERSION, + tools: { shell: { jobMaxMs: 7_200_000, maxJobs: 1 } }, + }); + expect(pinned.tools.shell.jobMaxMs).toBe(7_200_000); + expect(pinned.tools.shell.maxJobs).toBe(1); + // A ceiling of 0 would be "kill at once" and a job limit of 0 "never + // detach" — neither is what the fields mean, so both are refused. + for (const field of ["jobMaxMs", "maxJobs"] as const) { + for (const value of [0, -1, 1.5, Infinity, NaN, "many", true]) { + expect(() => + parseUserConfigFile({ + version: USER_CONFIG_VERSION, + tools: { shell: { [field]: value } }, + }), + ).toThrow(new RegExp(`tools\\.shell\\.${field}`)); + } + } + }); + + it("keeps the shell default timeout at ten minutes or more", () => { + // A long install or test suite has to fit; the timeout message tells + // the model what to pass for longer, so the default must not shrink. + expect( + USER_CONFIG_DEFAULTS.tools.shell.defaultTimeoutMs, + ).toBeGreaterThanOrEqual(600_000); + }); + + it("preserves an explicit tools.shell.defaultTimeoutMs, including 0 (no default)", () => { + const pinned = parseUserConfigFile({ + version: USER_CONFIG_VERSION, + tools: { shell: { defaultTimeoutMs: 1_800_000 } }, + }); + expect(pinned.tools.shell.defaultTimeoutMs).toBe(1_800_000); + const unbounded = parseUserConfigFile({ + version: USER_CONFIG_VERSION, + tools: { shell: { defaultTimeoutMs: 0 } }, + }); + expect(unbounded.tools.shell.defaultTimeoutMs).toBe(0); + }); + + it("rejects a tools.shell.defaultTimeoutMs that is not a non-negative finite integer", () => { + for (const defaultTimeoutMs of [-1, 1.5, Infinity, NaN, "soon", true]) { + expect(() => + parseUserConfigFile({ + version: USER_CONFIG_VERSION, + tools: { shell: { defaultTimeoutMs } }, + }), + ).toThrow(/tools\.shell\.defaultTimeoutMs/); + } + }); + it("preserves an explicit telegram.progressIndicator=false", () => { const parsed = parseUserConfigFile({ version: USER_CONFIG_VERSION, diff --git a/src/config/config-schema.ts b/src/config/config-schema.ts index 576a66f7..82cb7787 100644 --- a/src/config/config-schema.ts +++ b/src/config/config-schema.ts @@ -334,6 +334,8 @@ export interface AtomicAgentConfig { autoContinue: boolean; }; toolTimeoutMs: number; + /** Where reads may go: the working directory and user-named paths, or anywhere. */ + readScope: ReadScope; /** * Boot value for the five-step approval ladder (1 = ask for * everything … 5 = approve everything). The live value is owned by @@ -515,6 +517,25 @@ export interface AtomicAgentConfig { projects: { roots: string[]; }; + /** + * Per-tool operator settings. Mirrors `UserConfigFile.tools`. + */ + tools: { + shell: { + /** + * Wall-clock wait for an `os.shell.run` call whose `timeoutMs` + * the model omitted; a command still running then is detached as + * a job, not killed. `0` = no default (the command runs until it + * exits or the turn is cancelled). An explicit `timeoutMs`, + * including `0`, always wins over this — and kills at its limit. + */ + defaultTimeoutMs: number; + /** Absolute ceiling for a detached job, from its start. */ + jobMaxMs: number; + /** Detached jobs running per session; the next detach evicts the oldest. */ + maxJobs: number; + }; + }; log: { level: LogLevel; }; @@ -1311,6 +1332,36 @@ export function parseLocalTemplateSetting( ); } +/** + * Where a session's tools may READ (config v67). + * + * - `working-dir`: the working directory plus every absolute or + * `~`-prefixed path the user named in this session's own messages + * read unasked; anything else asks through the approval ladder + * (`fs_read_outside`, silent at level 5) and a yes widens the + * session's roots (see `src/tools/read-scope/`). The default. + * - `unrestricted`: the pre-v67 behaviour — reads anywhere on disk, + * never asked about. + * + * Fusion workers are confined regardless (and more narrowly). + */ +export type ReadScope = "working-dir" | "unrestricted"; + +export const READ_SCOPES: readonly ReadScope[] = ["working-dir", "unrestricted"]; + +export function parseReadScope(raw: unknown, field: string): ReadScope { + if ( + typeof raw === "string" && + (READ_SCOPES as readonly string[]).includes(raw) + ) { + return raw as ReadScope; + } + throw new ConfigValidationError( + field, + `expected ${READ_SCOPES.join("|")}, got ${JSON.stringify(raw)}`, + ); +} + export interface UserManagedLocalLlmConfig { modelId: string | null; port: number; @@ -1540,6 +1591,13 @@ export interface UserConfigFile { autoContinue: boolean; }; toolTimeoutMs: number; + /** + * Where a session's reads may go (config v67). `working-dir` (the + * default) confines filesystem reads and shell path arguments to the + * working directory and the paths the user named in the conversation; + * `unrestricted` is the pre-v67 behaviour. + */ + readScope: ReadScope; /** * Five-step approval ladder (config v37). Replaces the binary * `approvalRequired`; the legacy key is still read once for @@ -1594,6 +1652,36 @@ export interface UserConfigFile { projects: { roots: string[]; }; + /** + * Per-tool operator settings (config v67). + */ + tools: { + shell: { + /** + * Wall-clock wait, in milliseconds, for an `os.shell.run` call + * whose `timeoutMs` the model omitted. Default 600 000 (10 min). + * A command still running then is not killed: it is detached as a + * job the model can `wait` for or `kill`, with its output so far + * in the result. `0` = no default, which is the pre-v67 behaviour. + * The model can still pass an explicit `timeoutMs` per call (`0` + * for none); that always wins and kills at its limit. A + * non-negative integer. + */ + defaultTimeoutMs: number; + /** + * Absolute ceiling, in milliseconds, for a detached job, counted + * from its start — kept or not, waited on or not. Default + * 3 600 000 (1 h). A positive integer. + */ + jobMaxMs: number; + /** + * Detached jobs that may run at once per session. Default 3. The + * next detach stops the oldest un-kept job first and says so. A + * positive integer. + */ + maxJobs: number; + }; + }; tracing: { trace: { enabled: boolean | null; @@ -2277,7 +2365,29 @@ export interface UserConfigFile { // matching prefix (see `swa-full.ts`). Additive: an older file has no // field and gets `"auto"`, which is off unless the full-SWA KV estimate // fits the launch's memory budget. -export const USER_CONFIG_VERSION = 66; +// v67: session-boundary fields. +// `tools.shell.defaultTimeoutMs` (default 600 000) — the wall-clock +// wait for an `os.shell.run` call whose `timeoutMs` the model omitted. +// Before v67 an omitted `timeoutMs` meant no limit at all, and a +// recursive grep over a home directory ran for twenty minutes until a +// person killed it; `agent.toolTimeoutMs` never applied to the shell. +// A command still running when the default elapses is detached as a +// job (`src/tools/os/shell-jobs.ts`) rather than killed; the model +// reaches it through the `wait` / `kill` / `jobs` forms of the tool. +// `0` keeps the old unbounded behaviour; an explicit per-call +// `timeoutMs` (including `0`) always wins and kills at its limit. +// `tools.shell.jobMaxMs` (default 3 600 000) — the absolute ceiling for +// a detached job, from its start. `tools.shell.maxJobs` (default 3) — +// detached jobs running at once per session; the next detach evicts the +// oldest un-kept one. All three additive: an older file has no field +// and takes the default. +// `agent.readScope` (`"working-dir"` | `"unrestricted"`, default +// `"working-dir"`) — a session's filesystem reads and shell path +// arguments are confined to the working directory and the paths the user +// named in the conversation (`src/tools/read-scope/`). A DEFAULT-BEHAVIOUR +// CHANGE: an older file has no field and takes `"working-dir"`; the +// pre-v67 behaviour is one line away (`agent.readScope: "unrestricted"`). +export const USER_CONFIG_VERSION = 67; /** * Config v21+ flips the full memory-v2 fabric on by default. Upgrades @@ -2432,6 +2542,7 @@ const SUPPORTED_INPUT_VERSIONS: readonly number[] = [ 63, 64, 65, + 66, USER_CONFIG_VERSION, ]; @@ -2490,6 +2601,7 @@ export const USER_CONFIG_DEFAULTS: UserConfigFile = { autoContinue: true, }, toolTimeoutMs: 60_000, + readScope: "working-dir", approvalLevel: 1, // `0` = let the model's context window decide (CONVERSATION_CAP_AUTO); // the fixed 32K fallback applies only when no window is known. @@ -2537,6 +2649,20 @@ export const USER_CONFIG_DEFAULTS: UserConfigFile = { projects: { roots: [], }, + tools: { + shell: { + // Ten minutes: long enough for an install or a test suite, short + // enough that a runaway scan is reported the same hour it started. + // The detach notice tells the model how to wait for or stop it. + defaultTimeoutMs: 600_000, + // An hour: a build or a download that has not finished by then is + // not going to, and nobody is watching it any more. + jobMaxMs: 3_600_000, + // Three concurrent jobs is a server, a watcher and a build; more + // is a model that has stopped waiting for anything. + maxJobs: 3, + }, + }, tracing: { trace: { enabled: null, @@ -4212,6 +4338,8 @@ export function parseUserConfigFile(raw: unknown): UserConfigFile { const http = (obj.http as Record | undefined) ?? {}; const web = (obj.web as Record | undefined) ?? {}; const projects = (obj.projects as Record | undefined) ?? {}; + const tools = (obj.tools as Record | undefined) ?? {}; + const toolsShell = (tools.shell as Record | undefined) ?? {}; const webSearch = (web.search as Record | undefined) ?? {}; const webFetch = (web.fetch as Record | undefined) ?? {}; const webSearchProvider = parseWebSearchProviderName( @@ -4467,6 +4595,11 @@ export function parseUserConfigFile(raw: unknown): UserConfigFile { agent.toolTimeoutMs ?? USER_CONFIG_DEFAULTS.agent.toolTimeoutMs, "agent.toolTimeoutMs", ), + // The upgrade step for a pre-v67 file: no field, the default. + readScope: parseReadScope( + agent.readScope ?? USER_CONFIG_DEFAULTS.agent.readScope, + "agent.readScope", + ), approvalLevel: resolveApprovalLevel( agent.approvalLevel, agent.approvalRequired, @@ -4630,6 +4763,25 @@ export function parseUserConfigFile(raw: unknown): UserConfigFile { "projects.roots", ) ?? [], }, + tools: { + shell: { + // Non-negative rather than positive: `0` is "no default", the + // behaviour every pre-v67 file had. + defaultTimeoutMs: parseNonNegativeInt( + toolsShell.defaultTimeoutMs ?? + USER_CONFIG_DEFAULTS.tools.shell.defaultTimeoutMs, + "tools.shell.defaultTimeoutMs", + ), + jobMaxMs: parsePositiveInt( + toolsShell.jobMaxMs ?? USER_CONFIG_DEFAULTS.tools.shell.jobMaxMs, + "tools.shell.jobMaxMs", + ), + maxJobs: parsePositiveInt( + toolsShell.maxJobs ?? USER_CONFIG_DEFAULTS.tools.shell.maxJobs, + "tools.shell.maxJobs", + ), + }, + }, tracing: { trace: { enabled: parseBoolOrNull( diff --git a/src/config/index.ts b/src/config/index.ts index 7cfe3d1c..fade22a8 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -6,6 +6,7 @@ export type { LocalLlmMode, LogLevel, OnboardingState, + ReadScope, SessionRailConfig, TelegramConfig, NotificationsConfig, @@ -22,9 +23,11 @@ export type { } from "./config-schema.js"; export { ConfigValidationError, + READ_SCOPES, USER_CONFIG_DEFAULTS, USER_CONFIG_VERSION, parseOnboardingState, + parseReadScope, parseSessionRailConfig, parseUserConfigFile, parseWhileBusySubmit, diff --git a/src/config/load-config.test.ts b/src/config/load-config.test.ts index 7d9e6f68..632c9d96 100644 --- a/src/config/load-config.test.ts +++ b/src/config/load-config.test.ts @@ -151,6 +151,9 @@ describe("loadConfig", () => { toolTimeoutMs: 12_000, approvalLevel: 5, }, + tools: { + shell: { defaultTimeoutMs: 1_800_000, jobMaxMs: 7_200_000, maxJobs: 5 }, + }, }); const config = loadConfig(); expect(config.localModels.url).toBe("http://llama.internal:4444"); @@ -158,6 +161,11 @@ describe("loadConfig", () => { expect(config.agent.maxSteps).toBe(42); expect(config.agent.toolTimeoutMs).toBe(12_000); expect(config.agent.approvalLevel).toBe(5); + expect(config.tools.shell).toEqual({ + defaultTimeoutMs: 1_800_000, + jobMaxMs: 7_200_000, + maxJobs: 5, + }); }); it("maps llm.runMode from the file onto the runtime config", () => { diff --git a/src/config/load-config.ts b/src/config/load-config.ts index 58d78ac2..0dabcfaa 100644 --- a/src/config/load-config.ts +++ b/src/config/load-config.ts @@ -223,6 +223,7 @@ export function loadConfig(): AtomicAgentConfig { providerWait: user.agent.providerWait, task: user.agent.task, toolTimeoutMs: user.agent.toolTimeoutMs, + readScope: user.agent.readScope, approvalLevel: user.agent.approvalLevel, stablePrefixHashSalt: readEnv("ATOMIC_AGENT_STABLE_PREFIX_SALT") ?? @@ -342,6 +343,13 @@ export function loadConfig(): AtomicAgentConfig { projects: { roots: [...user.projects.roots], }, + tools: { + shell: { + defaultTimeoutMs: user.tools.shell.defaultTimeoutMs, + jobMaxMs: user.tools.shell.jobMaxMs, + maxJobs: user.tools.shell.maxJobs, + }, + }, log: { level: logLevel }, tasks: { enabled: readBool( diff --git a/src/prompt/default-tool-args-schemas.test.ts b/src/prompt/default-tool-args-schemas.test.ts index 45965a01..d3828107 100644 --- a/src/prompt/default-tool-args-schemas.test.ts +++ b/src/prompt/default-tool-args-schemas.test.ts @@ -27,11 +27,15 @@ describe("default tool argsJsonSchema map", () => { expect(missing).toEqual([]); }); - it("pins os.shell.run schema shape (cmd + args required, args is string[])", () => { + it("pins os.shell.run schema shape (four forms, nothing required, args is string[])", () => { + // `{cmd, args}` runs a command; `{wait}`, `{kill}`, `{jobs}` act on a + // job the default timeout detached (F47). Nothing can be required at + // the schema level because each form requires a different key; the + // tool refuses a call that mixes them or names none. const schema = getDefaultArgsJsonSchema("os.shell.run"); expect(schema).toMatchObject({ type: "object", - required: ["cmd", "args"], + required: [], additionalProperties: false, }); const properties = (schema as { properties: Record }) @@ -41,6 +45,10 @@ describe("default tool argsJsonSchema map", () => { type: "array", items: { type: "string" }, }); + expect(properties.wait).toEqual({ type: "integer" }); + expect(properties.kill).toEqual({ type: "integer" }); + expect(properties.jobs).toEqual({ type: "boolean" }); + expect(properties.keep).toEqual({ type: "boolean" }); }); it("pins memory.profile.set schema (key + value required, keywords is string[])", () => { diff --git a/src/prompt/default-tool-args-schemas.ts b/src/prompt/default-tool-args-schemas.ts index 71d6411b..bddc4884 100644 --- a/src/prompt/default-tool-args-schemas.ts +++ b/src/prompt/default-tool-args-schemas.ts @@ -117,15 +117,21 @@ const DEFAULT_TOOL_ARGS_SCHEMAS: ReadonlyMap = new Map< // ── os.shell ───────────────────────────────────────────────────────────── [ "os.shell.run", - obj( - { - cmd: stringSchema, - args: stringArraySchema, - cwd: stringSchema, - timeoutMs: numberSchema, - }, - ["cmd", "args"], - ), + // Four forms share one schema: `{cmd, args, …}` runs a command; + // `{wait}`, `{kill}` and `{jobs}` act on a job the default timeout + // detached (F47). Nothing is required at the schema level because + // each form requires a different key; the tool refuses a call that + // mixes them or names none. + obj({ + cmd: stringSchema, + args: stringArraySchema, + cwd: stringSchema, + timeoutMs: numberSchema, + keep: booleanSchema, + wait: integerSchema, + kill: integerSchema, + jobs: booleanSchema, + }), ], // ── os.fs ──────────────────────────────────────────────────────────────── diff --git a/src/prompt/default-tool-descriptors-a.ts b/src/prompt/default-tool-descriptors-a.ts index 95ae8b9c..d93bd3d9 100644 --- a/src/prompt/default-tool-descriptors-a.ts +++ b/src/prompt/default-tool-descriptors-a.ts @@ -43,9 +43,9 @@ export const DEFAULT_TOOL_DESCRIPTORS_A: readonly ToolDescriptor[] = [ { name: "os.shell.run", summary: - "Run a shell command in the working directory (may require approval). Not for deleting user files — use os.fs.trash when the user wants paths removed.", + "Run a shell command in the working directory (may require approval). Not for deleting user files — use os.fs.trash when the user wants paths removed. A command still running at the default timeout comes back as a job: wait for it, kill it, or list jobs.", argsSchema: - "{ cmd: string, args: string[], cwd?: string, timeoutMs?: number }", + "{ cmd: string, args: string[], cwd?: string, timeoutMs?: number, keep?: boolean } | { wait: number /* job id */, timeoutMs?: number, keep?: boolean } | { kill: number } | { jobs: true }", }, { name: "os.fs.read", diff --git a/src/runtime/bootstrap.ts b/src/runtime/bootstrap.ts index 24d8f9d4..e854b540 100644 --- a/src/runtime/bootstrap.ts +++ b/src/runtime/bootstrap.ts @@ -62,7 +62,7 @@ import { replyTool } from "../tools/conversation/index.js"; import { buildBrowserTools } from "../tools/browser/index.js"; import { PlaywrightBackend } from "../tools/browser/playwright-backend.js"; import type { BrowserBackend } from "../tools/browser/browser-backend.js"; -import { registerOsTools } from "../tools/os/index.js"; +import { registerOsTools, ShellJobRegistry } from "../tools/os/index.js"; import { registerVerifyTools, runChecks } from "../tools/verify/index.js"; import { registerGithubTools } from "../tools/github/index.js"; import { resolveGithubToken } from "../github/index.js"; @@ -72,9 +72,9 @@ import { registerMemoryTools } from "../tools/memory/index.js"; import { registerTaskTools } from "../tools/tasks/index.js"; import { buildFusionDelegateTool, - confineWorkerReads, pickOriginalRequest, } from "../tools/fusion/index.js"; +import { confineReads } from "../tools/read-scope/index.js"; import type { ToolRole } from "../tools/tool-roles.js"; import { resolveRunMode, type ResolvedRunMode } from "../llm/run-mode/index.js"; import { registerVisionTools } from "../tools/vision/index.js"; @@ -1417,13 +1417,23 @@ export async function createAgentRuntime( // column-only `listRecentWorkingDirs` projection, so the store must // exist by the time `registerOsTools` wires the closure below. const sessionStore = new SessionStore(); - // Drop a session's trace recorder when the session itself is deleted, so - // the map shrinks on teardown instead of relying on the cap to push - // entries out. Wrapped here rather than at each call site (the TUI and the + // The commands `os.shell.run` detached at the default timeout (F47). + // One registry for the runtime, so the turn-end (`executeTurn`), + // session-delete and shutdown paths below can stop what a session + // left running. + const shellJobs = new ShellJobRegistry({ + jobMaxMs: config.tools.shell.jobMaxMs, + maxJobs: config.tools.shell.maxJobs, + }); + // Drop a session's trace recorder — and stop its detached shell jobs, + // kept ones included — when the session itself is deleted, so the map + // shrinks on teardown instead of relying on the cap to push entries + // out. Wrapped here rather than at each call site (the TUI and the // HTTP route both delete sessions) so every caller gets it. const deleteSession = sessionStore.delete.bind(sessionStore); sessionStore.delete = (id: string): void => { dropRecorder(id); + shellJobs.endSession(id); deleteSession(id); }; @@ -1437,7 +1447,12 @@ export async function createAgentRuntime( } registerOsTools(toolRegistry, { ...dangerous, - config: { http: config.http, web: config.web, projects: config.projects }, + config: { + http: config.http, + web: config.web, + projects: config.projects, + tools: config.tools, + }, listRecentSessionDirs: (limit) => sessionStore.listRecentWorkingDirs(limit), // The trust surface (`config.json` + `.env`) is resolved once, here, // and injected into the fs tools — the tools layer must not know @@ -1456,6 +1471,7 @@ export async function createAgentRuntime( shellPolicy: { isGitRemoteSyncEnabled: () => getConfig().git.remoteSync, }, + shellJobs, }); // The read-only `verify.*` family: syntax per file, and (below) a // command / service / page run against a throwaway copy of the @@ -2531,6 +2547,9 @@ export async function createAgentRuntime( // Nothing will drain the inbox after this point; drop pending // steers so a message cannot resurface in a later process. steeringInbox.clearAll(); + // Every detached shell job, kept or not: nothing will wait on it + // once this process is gone, and its ceiling timer dies with us. + shellJobs.endAll(); // Cancel any in-flight reflection before tearing down the profile // store — otherwise a late-arriving completion could try to write // into a closed SQLite connection. @@ -2870,6 +2889,8 @@ export async function createAgentRuntime( // The prompt_captured hook still records the worker's window // occupancy under its id; nothing persists it, so drop it. lastTurnContextUsage.delete(session.id); + // A worker's turn is its whole life: nothing waits on its jobs. + shellJobs.endSession(session.id); } }); } @@ -2931,8 +2952,15 @@ export async function createAgentRuntime( }, }; sessionStore.save(finished); + // `finish` ended the whole session: its kept jobs go with it. + if (finished.status === "completed") shellJobs.endSession(session.id); return { ...result, session: finished }; } finally { + // The turn is over, however it ended: the shell jobs it started + // and did not `keep` are stopped here — the one choke point + // every turn passes through (§"A turn is a task, not a step + // budget"). + shellJobs.endTurn(session.id); lastTurnContextUsage.delete(session.id); turnRequests.delete(session.id); activeTraceSessions.delete(session.id); @@ -3149,12 +3177,21 @@ export async function createAgentRuntime( logger, }), ); - // A fusion worker reads inside its working directory and its fan-out's - // write scope, never the rest of the disk (`worker-read-scope.ts`). - // Installed here, after every native filesystem tool is registered; - // other sessions' reads are untouched. - confineWorkerReads(toolRegistry, { + // Every session reads inside its working directory and the paths the + // user named unasked, by default (`agent.readScope`, + // `src/tools/read-scope/`); a read outside that asks through the + // ladder as `fs_read_outside` — the same gate and surfaces as every + // other gated action — and a `y` widens the session's roots. A fusion + // worker is confined more narrowly still — its working directory and + // its fan-out's write scope, never the brief's — and refused, since + // nobody is at the other end of its prompt. The shell gets the same + // scope as a token check. Installed here, after every native + // filesystem tool and the shell are registered. The scope is re-read + // per call, so `agent.readScope: "unrestricted"` needs no restart. + confineReads(toolRegistry, { grantedDirs: (sessionId) => approvals.fanoutScopes.scopeFor(sessionId), + readScope: () => getConfig().agent.readScope, + approvals: dangerous, }); const scheduler = diff --git a/src/sandbox/capped-output.test.ts b/src/sandbox/capped-output.test.ts new file mode 100644 index 00000000..16a7b586 --- /dev/null +++ b/src/sandbox/capped-output.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, it } from "vitest"; + +import { CappedOutput } from "./capped-output.js"; + +describe("CappedOutput", () => { + it("keeps everything under the cap verbatim", () => { + const out = new CappedOutput(100); + out.append(Buffer.from("hello ")); + out.append(Buffer.from("world")); + expect(out.snapshot()).toEqual({ + text: "hello world", + bytes: 11, + droppedBytes: 0, + truncated: false, + }); + }); + + it("keeps the head and the newest tail, dropping the middle with a marker", () => { + // Cap 40 with a quarter for the head: 10 bytes of head, 30 of tail. + const out = new CappedOutput(40, 0.25); + for (let i = 0; i < 10; i += 1) out.append(Buffer.from(`line${i}\n`)); + const snap = out.snapshot(); + expect(snap.bytes).toBe(60); + expect(snap.droppedBytes).toBe(20); + expect(snap.truncated).toBe(true); + expect(snap.text.startsWith("line0\nline")).toBe(true); + expect(snap.text).toContain("… [20 bytes dropped]"); + expect(snap.text.endsWith("line5\nline6\nline7\nline8\nline9\n")).toBe(true); + }); + + it("trims a single chunk larger than the tail to its newest bytes", () => { + const out = new CappedOutput(8, 0); + out.append(Buffer.from("0123456789abcdef")); + const snap = out.snapshot(); + expect(snap.text.endsWith("89abcdef")).toBe(true); + expect(snap.droppedBytes).toBe(8); + }); +}); diff --git a/src/sandbox/capped-output.ts b/src/sandbox/capped-output.ts new file mode 100644 index 00000000..6c909ea8 --- /dev/null +++ b/src/sandbox/capped-output.ts @@ -0,0 +1,85 @@ +/** + * A byte-capped capture of one stream that keeps the head and the tail. + * + * A command that runs for an hour writes more than any tool result can + * carry; what the model needs is how it started (the command echo, the + * first error) and how it is going (the last lines). So the first + * `headShare` of the cap is kept verbatim, the rest is a window over + * the newest bytes, and the middle is dropped with a marker saying how + * much. Used by `startCommandJob` for a job's stdout and stderr. + */ + +export interface CappedOutputSnapshot { + /** Head, a drop marker when anything was dropped, then the tail. */ + text: string; + /** Every byte the stream produced, dropped ones included. */ + bytes: number; + droppedBytes: number; + /** `droppedBytes > 0`. */ + truncated: boolean; +} + +export class CappedOutput { + private readonly headCap: number; + private readonly tailCap: number; + private readonly head: Buffer[] = []; + private headBytes = 0; + private tail: Buffer[] = []; + private tailBytes = 0; + private droppedBytes = 0; + private totalBytes = 0; + + /** + * `maxBytes` bounds head + tail together; `headShare` (default a + * quarter) is the part of it kept from the start of the stream. + */ + constructor(maxBytes: number, headShare = 0.25) { + const cap = Math.max(0, Math.floor(maxBytes)); + this.headCap = Math.floor(cap * Math.min(1, Math.max(0, headShare))); + this.tailCap = cap - this.headCap; + } + + append(chunk: Buffer): void { + this.totalBytes += chunk.length; + let rest = chunk; + if (this.headBytes < this.headCap) { + const take = Math.min(rest.length, this.headCap - this.headBytes); + this.head.push(rest.subarray(0, take)); + this.headBytes += take; + rest = rest.subarray(take); + } + if (rest.length === 0) return; + this.tail.push(rest); + this.tailBytes += rest.length; + // Drop from the front of the tail until it fits: whole chunks while + // they are wholly over, then a slice of the first survivor. + while (this.tailBytes > this.tailCap && this.tail.length > 0) { + const first = this.tail[0]!; + const over = this.tailBytes - this.tailCap; + if (first.length <= over) { + this.tail.shift(); + this.tailBytes -= first.length; + this.droppedBytes += first.length; + } else { + this.tail[0] = first.subarray(over); + this.tailBytes -= over; + this.droppedBytes += over; + } + } + } + + snapshot(): CappedOutputSnapshot { + const headText = Buffer.concat(this.head).toString("utf8"); + const tailText = Buffer.concat(this.tail).toString("utf8"); + const text = + this.droppedBytes > 0 + ? `${headText}\n… [${this.droppedBytes.toLocaleString("en-US")} bytes dropped]\n${tailText}` + : headText + tailText; + return { + text, + bytes: this.totalBytes, + droppedBytes: this.droppedBytes, + truncated: this.droppedBytes > 0, + }; + } +} diff --git a/src/sandbox/command-job.test.ts b/src/sandbox/command-job.test.ts new file mode 100644 index 00000000..57743ad5 --- /dev/null +++ b/src/sandbox/command-job.test.ts @@ -0,0 +1,116 @@ +import { describe, expect, it } from "vitest"; + +import { + awaitJobExit, + JOB_STOP_GRACE_MS, + startCommandJob, +} from "./command-job.js"; + +/** + * The primitive behind `os.shell.run`'s detached jobs: output captured + * across waits, a stop that takes the whole process group. POSIX only: + * on Windows the group is a tree-kill, pinned in + * command-runner-kill.test.ts. + */ + +const cwd = process.cwd(); + +async function waitForExit(pid: number): Promise { + for (let i = 0; i < 80; i += 1) { + try { + process.kill(pid, 0); + } catch { + return; + } + await new Promise((resolve) => setTimeout(resolve, 25)); + } + throw new Error(`process ${pid} survived the stop`); +} + +describe.skipIf(process.platform === "win32")("startCommandJob", () => { + it("captures both streams and reports the exit", async () => { + const job = startCommandJob("sh", ["-c", "echo one; echo err >&2; exit 3"], { + cwd, + }); + expect(await job.waitFor(5_000)).toBe("exited"); + expect(job.exited()).toMatchObject({ exitCode: 3, signal: null }); + expect(job.output().stdout).toContain("one"); + expect(job.output().stderr).toContain("err"); + }); + + it("a wait that elapses leaves the job running; a later wait sees the exit and the output since", async () => { + // A shell takes ~200 ms to start inside a loaded test worker, so the + // first wait is well past that and the sleep well past the wait. + const job = startCommandJob("sh", ["-c", "echo early; sleep 2.5; echo late"], { + cwd, + }); + expect(await job.waitFor(1_000)).toBe("elapsed"); + expect(job.exited()).toBeNull(); + expect(job.output().stdout).toContain("early"); + expect(job.output().stdout).not.toContain("late"); + expect(await job.waitFor(10_000)).toBe("exited"); + expect(job.output().stdout).toContain("late"); + expect(job.exited()?.exitCode).toBe(0); + }); + + it("an aborted wait returns at once and leaves the job running", async () => { + const controller = new AbortController(); + const job = startCommandJob("sleep", ["30"], { cwd }); + setTimeout(() => controller.abort(), 100); + expect(await job.waitFor(0, controller.signal)).toBe("aborted"); + expect(job.exited()).toBeNull(); + job.kill(); + expect((await awaitJobExit(job)).signal).toBe("SIGKILL"); + }); + + it("stop takes a subshell's background child with it", async () => { + const job = startCommandJob("sh", ["-c", "sleep 30 & echo $!; sleep 30"], { + cwd, + }); + // Long enough for the shell to have run `echo` on a loaded host. + await job.waitFor(1_500); + const pid = Number(job.output().stdout.trim()); + expect(pid).toBeGreaterThan(0); + job.stop(); + const exit = await awaitJobExit(job); + expect(exit.signal).toBe("SIGTERM"); + // Without the group the orphaned `sleep` would keep stdout open and + // the exit would not close for 30 s. + expect(exit.durationMs).toBeLessThan(5_000); + await waitForExit(pid); + }); + + it("escalates to SIGKILL after the grace when the group ignores SIGTERM", async () => { + // `trap "" TERM` is inherited across exec, so nothing in the group + // honours the polite stop. + const job = startCommandJob("sh", ["-c", 'trap "" TERM; sleep 30'], { cwd }); + // Long enough for the trap to be in place on a loaded host. + await job.waitFor(1_000); + const stoppedAt = Date.now(); + job.stop(); + const exit = await awaitJobExit(job); + expect(exit.signal).toBe("SIGKILL"); + expect(Date.now() - stoppedAt).toBeGreaterThanOrEqual(JOB_STOP_GRACE_MS - 50); + }, 10_000); + + it("rejects the first wait when the command cannot be spawned", async () => { + const job = startCommandJob("/nonexistent/definitely-missing-binary", [], { + cwd, + }); + await expect(job.waitFor(1_000)).rejects.toThrow(/ENOENT/); + }); + + it("caps the output per stream, keeping the head and the tail", async () => { + const job = startCommandJob( + "sh", + ["-c", "i=0; while [ $i -lt 2000 ]; do echo line$i; i=$((i+1)); done"], + { cwd, maxOutputBytes: 2_000 }, + ); + expect(await job.waitFor(10_000)).toBe("exited"); + const out = job.output(); + expect(out.truncated).toBe(true); + expect(out.stdout).toContain("line0\n"); + expect(out.stdout).toContain("line1999"); + expect(out.stdout).toContain("bytes dropped"); + }); +}); diff --git a/src/sandbox/command-job.ts b/src/sandbox/command-job.ts new file mode 100644 index 00000000..1e25e84d --- /dev/null +++ b/src/sandbox/command-job.ts @@ -0,0 +1,242 @@ +import { spawn, type ChildProcess } from "node:child_process"; + +import { CappedOutput } from "./capped-output.js"; +import { killProcessTree } from "./kill-process-tree.js"; + +const IS_WINDOWS = process.platform === "win32"; + +/** + * How long a stopped job has to honour `SIGTERM` before it is killed + * outright. Long enough for a build tool to flush and remove its temp + * files, short enough not to hold up a tool result. + */ +export const JOB_STOP_GRACE_MS = 2_000; + +/** Per stream, head + tail (`CappedOutput`); 1 MiB. */ +export const DEFAULT_JOB_OUTPUT_BYTES = 1024 * 1024; + +/** How long `awaitJobExit` gives a stopped job to close: the grace plus a margin. */ +export const JOB_EXIT_SETTLE_MS = JOB_STOP_GRACE_MS + 3_000; + +export interface CommandJobOptions { + cwd: string; + env?: NodeJS.ProcessEnv; + /** Cap per stream; the head and the tail survive, the middle is dropped. */ + maxOutputBytes?: number; +} + +export interface CommandJobExit { + exitCode: number | null; + signal: NodeJS.Signals | null; + /** From the spawn to the close of the child's stdio. */ + durationMs: number; +} + +export interface CommandJobOutput { + stdout: string; + stderr: string; + /** Either stream dropped bytes to stay under its cap. */ + truncated: boolean; +} + +/** Why `waitFor` returned: the child closed, the wait ran out, the signal fired. */ +export type CommandJobWait = "exited" | "elapsed" | "aborted"; + +/** + * A running command whose lifetime is not tied to one `await`: the + * `os.shell.run` job that the default timeout detaches instead of + * killing. Output keeps being captured after every `waitFor` returns, + * so a later wait (or a kill) can report what happened in between. + */ +export interface CommandJob { + readonly command: string; + readonly args: readonly string[]; + readonly pid: number | undefined; + readonly startedAt: number; + /** Settles when the child's stdio closes; rejects when it could not be spawned. */ + readonly exit: Promise; + /** The exit once known, else `null`. */ + exited(): CommandJobExit | null; + /** A snapshot of what both streams have produced so far. */ + output(): CommandJobOutput; + /** + * Wait up to `ms` (`0` = unbounded) for the child to close, or until + * `signal` aborts — which returns promptly and leaves the child + * running. Rejects only when the spawn itself failed. + */ + waitFor(ms: number, signal?: AbortSignal): Promise; + /** A polite stop: `SIGTERM` to the group, `SIGKILL` after the grace. */ + stop(): void; + /** `SIGKILL` to the group at once. */ + kill(): void; +} + +/** + * The job's exit, bounded. A job that was stopped answers within the + * grace; one whose stdio is held open by something the group kill did + * not reach (a `setsid` grandchild) is reported with a null exit after + * `maxWaitMs` rather than holding a tool result forever. A job that + * could not be spawned reports the same null exit — the spawn error + * itself is delivered by the first `waitFor`. + */ +export async function awaitJobExit( + job: CommandJob, + maxWaitMs = JOB_EXIT_SETTLE_MS, +): Promise { + const known = job.exited(); + if (known) return known; + await job.waitFor(maxWaitMs).catch(() => undefined); + return ( + job.exited() ?? { + exitCode: null, + signal: null, + durationMs: Date.now() - job.startedAt, + } + ); +} + +/** + * Signal the child's whole process group. On POSIX the child leads its + * own group (spawned `detached`), so `-pid` reaches everything the + * command started — a subshell's `sleep 30 &` included. Without that a + * background grandchild survives the kill and, still holding the stdio + * pipes, keeps the job from closing until it exits on its own. A group + * that is already gone falls back to the direct kill, a no-op then. On + * Windows the tree-kill (`taskkill /T`) walks the descendants instead. + */ +function signalGroup(child: ChildProcess, signal: NodeJS.Signals): void { + if (IS_WINDOWS) { + killProcessTree(child, { force: signal === "SIGKILL" }); + return; + } + if (typeof child.pid === "number") { + try { + process.kill(-child.pid, signal); + return; + } catch { + // group gone — fall through to the direct child + } + } + try { + child.kill(signal); + } catch { + // process already exited + } +} + +/** + * Spawn `command` in its own process group with both streams captured + * into capped head + tail buffers. Nothing is written to stdin; it is + * closed at once, so a command that reads it sees EOF. + */ +export function startCommandJob( + command: string, + args: string[], + options: CommandJobOptions, +): CommandJob { + const startedAt = Date.now(); + const cap = options.maxOutputBytes ?? DEFAULT_JOB_OUTPUT_BYTES; + const child = spawn(command, args, { + cwd: options.cwd, + env: options.env ?? process.env, + stdio: ["pipe", "pipe", "pipe"], + // A group of its own on POSIX; on Windows `detached` would open a + // console, and the tree-kill needs no group. + ...(IS_WINDOWS ? { windowsHide: true } : { detached: true }), + }); + const stdout = new CappedOutput(cap); + const stderr = new CappedOutput(cap); + let exit: CommandJobExit | null = null; + let spawnError: Error | null = null; + let stopTimer: ReturnType | null = null; + const clearStop = () => { + if (stopTimer) clearTimeout(stopTimer); + stopTimer = null; + }; + + const exitPromise = new Promise((resolve, reject) => { + child.on("error", (err) => { + if (exit || spawnError) return; + spawnError = err; + clearStop(); + reject(err); + }); + child.on("close", (code, signal) => { + if (exit || spawnError) return; + exit = { + exitCode: code, + signal, + durationMs: Date.now() - startedAt, + }; + clearStop(); + resolve(exit); + }); + }); + // The rejection is delivered through `waitFor`; nobody may be + // awaiting `exit` itself when a spawn fails. + exitPromise.catch(() => undefined); + + child.stdout.on("data", (chunk: Buffer) => stdout.append(chunk)); + child.stderr.on("data", (chunk: Buffer) => stderr.append(chunk)); + // Nothing is written, so a far end that closed early is not an error. + child.stdin.on("error", () => undefined); + child.stdin.end(); + + return { + command, + args, + pid: child.pid, + startedAt, + exit: exitPromise, + exited: () => exit, + output: () => { + const out = stdout.snapshot(); + const err = stderr.snapshot(); + return { + stdout: out.text, + stderr: err.text, + truncated: out.truncated || err.truncated, + }; + }, + waitFor(ms, signal) { + if (spawnError) return Promise.reject(spawnError); + if (exit) return Promise.resolve("exited"); + if (signal?.aborted) return Promise.resolve("aborted"); + return new Promise((resolve, reject) => { + const timer = + ms > 0 && Number.isFinite(ms) + ? setTimeout(() => settle("elapsed"), ms) + : null; + const onAbort = () => settle("aborted"); + const cleanup = () => { + if (timer) clearTimeout(timer); + signal?.removeEventListener("abort", onAbort); + }; + const settle = (outcome: CommandJobWait) => { + cleanup(); + resolve(outcome); + }; + signal?.addEventListener("abort", onAbort, { once: true }); + exitPromise.then( + () => settle("exited"), + (err: unknown) => { + cleanup(); + reject(err instanceof Error ? err : new Error(String(err))); + }, + ); + }); + }, + stop() { + if (exit || spawnError || stopTimer) return; + // A polite stop first, so a build tool can flush; the escalation + // is armed regardless, because a polite stop is free to be ignored. + signalGroup(child, "SIGTERM"); + stopTimer = setTimeout(() => signalGroup(child, "SIGKILL"), JOB_STOP_GRACE_MS); + }, + kill() { + if (exit || spawnError) return; + clearStop(); + signalGroup(child, "SIGKILL"); + }, + }; +} diff --git a/src/sandbox/command-runner.ts b/src/sandbox/command-runner.ts index 00fedff9..683d76f0 100644 --- a/src/sandbox/command-runner.ts +++ b/src/sandbox/command-runner.ts @@ -69,7 +69,12 @@ export interface CommandResult { * Timeout semantics: when `timeoutMs` is omitted it falls back to 60s. * A non-positive or non-finite `timeoutMs` (e.g. `0`) disables the timeout * entirely — the command runs unbounded and is only stoppable via the abort - * signal. Long-running tools (e.g. `brew install`) rely on this. + * signal. Long-running tools (e.g. `brew install`) rely on this. Whatever + * was captured before the stop is kept in the result. + * + * The stop is a tree-kill of the direct child. A command whose lifetime + * must outlast one `await` — the shell tool's detached jobs — goes + * through `startCommandJob` (command-job.ts) instead. */ export async function runCommand( command: string, @@ -117,6 +122,13 @@ export async function runCommand( const onAbort = () => killIt("abort"); options.signal?.addEventListener("abort", onAbort, { once: true }); + /** One exit for every terminal path: timers off, listener off. */ + const finish = () => { + settled = true; + if (timer) clearTimeout(timer); + options.signal?.removeEventListener("abort", onAbort); + }; + child.stdout.on("data", (chunk: Buffer) => { if (stdoutBytes + chunk.length > maxOutputBytes) { const slice = chunk.slice(0, Math.max(0, maxOutputBytes - stdoutBytes)); @@ -146,16 +158,12 @@ export async function runCommand( child.on("error", (err) => { if (settled) return; - settled = true; - if (timer) clearTimeout(timer); - options.signal?.removeEventListener("abort", onAbort); + finish(); reject(err); }); child.on("close", (code, signal) => { if (settled) return; - settled = true; - if (timer) clearTimeout(timer); - options.signal?.removeEventListener("abort", onAbort); + finish(); resolve({ command, args, @@ -186,9 +194,7 @@ export async function runCommand( return; } if (settled) return; - settled = true; - if (timer) clearTimeout(timer); - options.signal?.removeEventListener("abort", onAbort); + finish(); reject(err); }); diff --git a/src/sandbox/index.ts b/src/sandbox/index.ts index f144ae12..115d4327 100644 --- a/src/sandbox/index.ts +++ b/src/sandbox/index.ts @@ -1,5 +1,19 @@ export { isBrokenPipe, runCommand } from "./command-runner.js"; export type { CommandOptions, CommandResult } from "./command-runner.js"; +export { + DEFAULT_JOB_OUTPUT_BYTES, + JOB_STOP_GRACE_MS, + startCommandJob, +} from "./command-job.js"; +export type { + CommandJob, + CommandJobExit, + CommandJobOptions, + CommandJobOutput, + CommandJobWait, +} from "./command-job.js"; +export { CappedOutput } from "./capped-output.js"; +export type { CappedOutputSnapshot } from "./capped-output.js"; export { killProcessTree } from "./kill-process-tree.js"; export type { KillableChild, diff --git a/src/tools/fusion/index.ts b/src/tools/fusion/index.ts index cd341854..f60da49a 100644 --- a/src/tools/fusion/index.ts +++ b/src/tools/fusion/index.ts @@ -97,13 +97,15 @@ export { inspectDeclaredFiles, } from "./declared-files.js"; export type { DeclaredFileReport } from "./declared-files.js"; +// The worker read scope lives with the session one now +// (`src/tools/read-scope/`); the worker names are kept here as aliases. export { checkWorkerRead, confineWorkerReads, isOutsideReadRoots, WORKER_READ_REFUSAL_REASON, WORKER_READ_TOOL_TARGETS, -} from "./worker-read-scope.js"; +} from "../read-scope/index.js"; export { runWorkerTasks } from "./worker-runner.js"; export type { WorkerRunnerDeps, diff --git a/src/tools/fusion/worker-read-scope.ts b/src/tools/fusion/worker-read-scope.ts deleted file mode 100644 index f5e25ecb..00000000 --- a/src/tools/fusion/worker-read-scope.ts +++ /dev/null @@ -1,217 +0,0 @@ -import { realpathSync } from "node:fs"; -import { basename, dirname, resolve } from "node:path"; - -import { isInside } from "../../approval/fanout-scope.js"; -import { - compressToolResult, - type CompressedToolResult, -} from "../../compressor/result-compressor.js"; -import { isFusionWorkerSessionId } from "../../session/fusion-worker-session.js"; -import { resolveUserPath } from "../os/expand-home.js"; -import type { - ToolContext, - ToolDefinition, - ToolRegistry, -} from "../tool-registry.js"; - -/** - * Where a fusion worker may READ. - * - * Writes and commands were already confined — the fan-out's approval - * names directories, and anything outside them hits the worker's refuse - * policy. Reads never went through the gate at all, and a real run - * showed what that costs: workers with thin briefs read - * `../../01-cloud-flash/work/js/main.js` (a sibling benchmark's output) - * and a harness screen dump, and spent their steps reverse-engineering - * code that was not theirs to match. - * - * So a worker's filesystem reads must resolve inside its working - * directory, or inside a directory this fan-out may write in (a worker - * has to be able to read back what it was authorised to write, and a - * fan-out scope can sit outside the working directory). This is scope - * discipline against wandering, not a sandbox: a path that is inside - * lexically OR by canonical (realpath) form is allowed, so a symlinked - * `node_modules` keeps working and `/tmp` vs `/private/tmp` does not - * refuse a path that is really inside. - * - * Only worker sessions are affected; every other session's reads are - * untouched. The refusal is a tool result, not a throw, so it costs the - * worker one step and reads as an instruction. - */ - -type TargetsOf = (args: Record) => string[]; - -function nonEmpty(value: unknown): string | undefined { - return typeof value === "string" && value.length > 0 ? value : undefined; -} - -const pathArg: TargetsOf = (args) => { - const path = nonEmpty(args.path); - return path === undefined ? [] : [path]; -}; - -/** - * The read-class filesystem tools and the argument(s) naming what they - * read. A search tool's omitted root is the working directory, which is - * inside by definition, so it contributes nothing to check. - */ -export const WORKER_READ_TOOL_TARGETS: ReadonlyMap = - new Map([ - ["os.fs.read", pathArg], - ["os.fs.list", pathArg], - ["os.fs.hash", pathArg], - ["os.fs.watch", pathArg], - ["os.fs.read_document", pathArg], - ["os.fs.archive.list", pathArg], - ["os.fs.archive.read_entry", pathArg], - ["os.fs.grep", pathArg], - [ - "os.fs.glob", - // `cwd` wins over `path` inside the tool; either names the root. - (args) => { - const root = nonEmpty(args.cwd) ?? nonEmpty(args.path); - return root === undefined ? [] : [root]; - }, - ], - [ - "os.fs.diff", - (args) => - [nonEmpty(args.aPath), nonEmpty(args.bPath)].filter( - (path): path is string => path !== undefined, - ), - ], - [ - "vision.describe", - (args) => - [ - nonEmpty(args.path), - ...(Array.isArray(args.paths) ? args.paths.map(nonEmpty) : []), - ].filter((path): path is string => path !== undefined), - ], - [ - // A syntax check reads every file it is handed. - "verify.syntax", - (args) => - (Array.isArray(args.files) ? args.files.map(nonEmpty) : []).filter( - (path): path is string => path !== undefined, - ), - ], - ]); - -/** `https://…`, `data:…` — not a filesystem path, not this module's business. */ -const URL_LIKE = /^[a-z][a-z0-9+.-]*:(?:\/\/|[^\\/])/i; - -export const WORKER_READ_REFUSAL_REASON = "worker-read-outside-scope"; - -/** - * The realpath of the deepest existing ancestor with the rest appended, - * so a path that does not exist yet still canonicalises. - */ -function canonical(path: string): string { - const rest: string[] = []; - let current = path; - for (;;) { - try { - return resolve(realpathSync.native(current), ...rest.reverse()); - } catch { - const parent = dirname(current); - if (parent === current) return path; - rest.push(basename(current)); - current = parent; - } - } -} - -/** Whether absolute `target` lies outside every root, lexically and canonically. */ -export function isOutsideReadRoots( - target: string, - roots: readonly string[], -): boolean { - if (roots.some((root) => isInside(resolve(root), target))) return false; - const real = canonical(target); - return !roots.some((root) => isInside(canonical(resolve(root)), real)); -} - -/** - * The refusal for a worker read outside its roots, or `null` when the - * call may run (not a worker, not a read tool, or every target inside). - */ -export function checkWorkerRead( - tool: string, - args: Record, - ctx: Pick, - grantedDirs: readonly string[], -): CompressedToolResult | null { - if (!isFusionWorkerSessionId(ctx.sessionId)) return null; - const targetsOf = WORKER_READ_TOOL_TARGETS.get(tool); - if (targetsOf === undefined) return null; - const roots = [ctx.workingDir, ...grantedDirs]; - for (const raw of targetsOf(args)) { - if (URL_LIKE.test(raw)) continue; - let absolute: string; - try { - absolute = resolveUserPath(raw, ctx.workingDir); - } catch { - // Unresolvable here means unresolvable in the tool too; its own - // error is the better message. - continue; - } - if (!isOutsideReadRoots(absolute, roots)) continue; - const alsoScope = - grantedDirs.length > 0 - ? ` or the directories this fan-out may write in (${grantedDirs.join(", ")})` - : ""; - const output = - `${tool} refused: ${absolute} is outside this worker's working directory (${ctx.workingDir})${alsoScope}, and a worker reads only inside those. ` + - `Do not search elsewhere for context: use your task, its FILES and the original request, and name anything missing in your reply.`; - return compressToolResult( - { - tool, - status: "error", - output, - details: { - reason: WORKER_READ_REFUSAL_REASON, - path: absolute, - allowedRoots: roots, - }, - }, - // Paths are long; a clipped refusal loses the instruction at its end. - { maxSummaryLength: Math.max(1000, output.length + 50) }, - ); - } - return null; -} - -/** Definitions this module produced, so a second install does not wrap twice. */ -const CONFINED = new WeakSet(); - -/** - * Wrap every registered read-class tool so a worker session's call is - * checked before it runs. Call once, after the native tools are - * registered. Returns the names it confined. - * - * Wrapping at the registry rather than inside each tool keeps the rule - * in one place owned by fusion, and `registry.invoke` is the single - * path every call — native, batched, or recovered from text — takes. - */ -export function confineWorkerReads( - registry: Pick, - options: { grantedDirs: (sessionId: string) => readonly string[] }, -): string[] { - const confined: string[] = []; - for (const name of WORKER_READ_TOOL_TARGETS.keys()) { - if (!registry.has(name)) continue; - const inner = registry.get(name); - confined.push(name); - if (CONFINED.has(inner)) continue; - const wrapped: ToolDefinition = { - ...inner, - run: async (args, ctx) => - checkWorkerRead(name, args, ctx, options.grantedDirs(ctx.sessionId)) ?? - inner.run(args, ctx), - }; - CONFINED.add(wrapped); - registry.register(wrapped); - } - return confined; -} diff --git a/src/tools/os/fs-replace-guard.test.ts b/src/tools/os/fs-replace-guard.test.ts index e94f2e0a..2a272fd6 100644 --- a/src/tools/os/fs-replace-guard.test.ts +++ b/src/tools/os/fs-replace-guard.test.ts @@ -381,6 +381,9 @@ describe("replace guard (F36)", () => { approvals: gate, approvalRequired: true, config: { + tools: { + shell: { defaultTimeoutMs: 600_000, jobMaxMs: 3_600_000, maxJobs: 3 }, + }, http: { enabled: true, approvalMode: "writes", diff --git a/src/tools/os/index.ts b/src/tools/os/index.ts index 89544b90..ab5bfc64 100644 --- a/src/tools/os/index.ts +++ b/src/tools/os/index.ts @@ -4,6 +4,7 @@ import type { DangerousToolOptions } from "../../approval/dangerous-tool.js"; import type { AtomicAgentConfig } from "../../config/index.js"; import { buildOsShellTool } from "./shell.js"; import type { ShellGuardPolicy } from "./shell-command-guard/index.js"; +import type { ShellJobRegistry } from "./shell-jobs.js"; import { osFsReadTool } from "./fs-read.js"; import { buildOsFsWriteTool } from "./fs-write.js"; import { buildOsFsTrashTool } from "./fs-trash.js"; @@ -103,9 +104,32 @@ export { export { osProcListTool, buildOsProcKillTool } from "./proc/index.js"; export { isGogCommand } from "./shell-command-guard/index.js"; export type { ShellGuardPolicy } from "./shell-command-guard/index.js"; +export { + describeShellTimeoutDefault, + formatShellDetachNotice, + formatShellDuration, + formatShellElapsed, + formatShellTimeoutNotice, + resolveShellTimeout, +} from "./shell-timeout.js"; +export type { + ResolvedShellTimeout, + ShellTimeoutSource, +} from "./shell-timeout.js"; +export { + DEFAULT_SHELL_JOB_MAX_MS, + DEFAULT_SHELL_MAX_JOBS, + ShellJobRegistry, +} from "./shell-jobs.js"; +export type { + ShellJobRecord, + ShellJobRegistryOptions, + ShellJobState, + ShellJobStopReason, +} from "./shell-jobs.js"; export interface RegisterOsToolsOptions extends DangerousToolOptions { - config: Pick; + config: Pick; /** * Column-only recent-session projection for `os.fs.locate_project` * (`SessionStore.listRecentWorkingDirs`). A closure so the caller @@ -138,6 +162,13 @@ export interface RegisterOsToolsOptions extends DangerousToolOptions { * disables the policy layer (embedders, tests). */ shellPolicy?: ShellGuardPolicy; + /** + * The registry of commands `os.shell.run` detached at the default + * timeout (F47). The bootstrap owns it so the turn-end, session-end + * and shutdown paths can stop the jobs; omitted (embedders, tests) + * the tool keeps a private one whose jobs die only at the ceiling. + */ + shellJobs?: ShellJobRegistry; } export function registerOsTools( @@ -148,9 +179,11 @@ export function registerOsTools( buildOsShellTool({ approvals: options.approvals, approvalRequired: options.approvalRequired, + defaultTimeoutMs: options.config.tools.shell.defaultTimeoutMs, ...(options.shellPolicy === undefined ? {} : { shellPolicy: options.shellPolicy }), + ...(options.shellJobs === undefined ? {} : { jobs: options.shellJobs }), }), ); // One option bag for every tool that replaces file content, so the diff --git a/src/tools/os/os-tools.test.ts b/src/tools/os/os-tools.test.ts index 5d7916c7..6a2d625c 100644 --- a/src/tools/os/os-tools.test.ts +++ b/src/tools/os/os-tools.test.ts @@ -544,6 +544,9 @@ describe("registerOsTools", () => { approvals: gate, approvalRequired: false, config: { + tools: { + shell: { defaultTimeoutMs: 600_000, jobMaxMs: 3_600_000, maxJobs: 3 }, + }, http: { enabled: true, approvalMode: "writes", diff --git a/src/tools/os/shell-detach.test.ts b/src/tools/os/shell-detach.test.ts new file mode 100644 index 00000000..cbf581a4 --- /dev/null +++ b/src/tools/os/shell-detach.test.ts @@ -0,0 +1,285 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { mkdtemp, readFile, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { ApprovalGate } from "../../approval/approval-gate.js"; +import type { ToolContext } from "../tool-registry.js"; +import { ShellJobRegistry } from "./shell-jobs.js"; +import { buildOsShellTool } from "./shell.js"; + +/** + * F47 rework: at the operator's default timeout a command is detached, + * not killed. The whole contract through the tool, with real processes: + * the detached result, `wait`, `kill`, `jobs`, turn end vs `keep`, the + * job limit, the ceiling — and that an explicit `timeoutMs` still kills. + */ + +const SESSION = "test-session"; + +function shellTool(defaultTimeoutMs: number, jobs?: ShellJobRegistry) { + const gate = new ApprovalGate({ + emit: (req) => gate.resolve({ approvalId: req.approvalId, approved: true }), + }); + return buildOsShellTool({ + approvals: gate, + approvalRequired: true, + defaultTimeoutMs, + ...(jobs === undefined ? {} : { jobs }), + }); +} + +function isAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +async function waitForExit(pid: number): Promise { + for (let i = 0; i < 120; i += 1) { + if (!isAlive(pid)) return; + await new Promise((resolve) => setTimeout(resolve, 25)); + } + throw new Error(`process ${pid} is still alive`); +} + +/** + * The pid a command wrote, once it has: a shell takes ~200 ms to start + * inside a loaded test worker, so the file is polled rather than read. + */ +async function readPid(file: string): Promise { + for (let i = 0; i < 200; i += 1) { + try { + const pid = Number((await readFile(file, "utf8")).trim()); + if (pid > 0) return pid; + } catch { + // not written yet + } + await new Promise((resolve) => setTimeout(resolve, 25)); + } + throw new Error(`${file} was never written`); +} + +/** Every test's default wait; past the shell's startup on a loaded host. */ +const DEFAULT_WAIT_MS = 1_000; + +describe.skipIf(process.platform === "win32")( + "os.shell.run detaches at the default timeout (F47)", + () => { + let dir: string; + let jobs: ShellJobRegistry; + + beforeEach(async () => { + dir = await mkdtemp(join(tmpdir(), "atomic-shell-detach-")); + jobs = new ShellJobRegistry(); + }); + + afterEach(async () => { + // Whatever a test left running goes with its session. + jobs.endAll(); + await rm(dir, { recursive: true, force: true }); + }); + + function ctx(signal = new AbortController().signal): ToolContext { + return { workingDir: dir, sessionId: SESSION, stepIndex: 0, signal }; + } + + it("returns ok with a job id, pid and the output so far; the process stays alive", async () => { + const pidFile = join(dir, "pid"); + const result = await shellTool(DEFAULT_WAIT_MS, jobs).run( + { cmd: `echo $$ > "${pidFile}"; echo partial; sleep 30` }, + ctx(), + ); + expect(result.status).toBe("ok"); + expect(result.details).toMatchObject({ + detached: true, + jobId: 1, + timedOut: false, + keep: false, + }); + expect(result.summary).toContain( + 'still running after 1 s (job 1) — output so far below; os.shell.run {"wait": 1} keeps waiting (up to another 1 s per call), {"kill": 1} stops it, pass timeoutMs for a longer first wait', + ); + expect(result.summary).toContain("still running (job 1, pid "); + expect(result.summary).toContain("partial"); + const pid = await readPid(pidFile); + expect(result.details.pid).toBe(pid); + expect(isAlive(pid)).toBe(true); + }); + + it("wait returns the still-running result again, then the exit with the full output", async () => { + const tool = shellTool(DEFAULT_WAIT_MS, jobs); + // Three seconds: past the first wait and the second, short of the third. + const started = await tool.run( + { cmd: "echo start; sleep 3; echo done; exit 4" }, + ctx(), + ); + expect(started.details.detached).toBe(true); + const again = await tool.run({ wait: 1 }, ctx()); + expect(again.status).toBe("ok"); + expect(again.details).toMatchObject({ detached: true, jobId: 1 }); + expect(again.summary).toContain("still running after another 1 s (job 1)"); + const finished = await tool.run({ wait: 1, timeoutMs: 10_000 }, ctx()); + expect(finished.status).toBe("error"); + expect(finished.details).toMatchObject({ + exitCode: 4, + jobId: 1, + timedOut: false, + }); + expect(finished.details.detached).toBeUndefined(); + expect(finished.summary).toContain("exit: 4"); + expect(finished.summary).toContain("start"); + expect(finished.summary).toContain("done"); + // Collected: the id is gone. + const gone = await tool.run({ wait: 1 }, ctx()); + expect(gone.status).toBe("error"); + expect(gone.summary).toContain("unknown job 1"); + expect(jobs.list(SESSION)).toEqual([]); + }); + + it("kill stops the whole group and reports the tail", async () => { + const pidFile = join(dir, "background.pid"); + const tool = shellTool(DEFAULT_WAIT_MS, jobs); + await tool.run( + { cmd: `sleep 30 & echo $! > "${pidFile}"; echo tail-line; sleep 30` }, + ctx(), + ); + const pid = await readPid(pidFile); + expect(isAlive(pid)).toBe(true); + const killed = await tool.run({ kill: 1 }, ctx()); + expect(killed.status).toBe("ok"); + expect(killed.details).toMatchObject({ + killed: true, + jobId: 1, + stopReason: "kill", + }); + expect(killed.summary).toContain("killed (job 1, on request)"); + expect(killed.summary).toContain("tail-line"); + await waitForExit(pid); + expect(jobs.list(SESSION)).toEqual([]); + }); + + it("jobs lists this session's jobs with their state", async () => { + const tool = shellTool(DEFAULT_WAIT_MS, jobs); + expect((await tool.run({ jobs: true }, ctx())).summary).toContain( + "no jobs in this session", + ); + await tool.run({ cmd: "sleep 30", keep: true }, ctx()); + await tool.run({ cmd: "sleep 30" }, ctx()); + const listed = await tool.run({ jobs: true }, ctx()); + expect(listed.status).toBe("ok"); + expect(listed.summary).toMatch(/job 1: sleep 30 — running \d+ s, kept \(pid \d+\)/); + expect(listed.summary).toMatch(/job 2: sleep 30 — running \d+ s \(pid \d+\)/); + const rows = listed.details.jobs as Array>; + expect(rows.map((row) => [row.id, row.state, row.keep])).toEqual([ + [1, "running", true], + [2, "running", false], + ]); + }); + + it("the turn's end kills un-kept jobs; keep: true on the call or on a later wait survives it", async () => { + const tool = shellTool(DEFAULT_WAIT_MS, jobs); + const plainPid = join(dir, "plain.pid"); + const keptPid = join(dir, "kept.pid"); + const laterPid = join(dir, "later.pid"); + await tool.run({ cmd: `echo $$ > "${plainPid}"; sleep 30` }, ctx()); + await tool.run({ cmd: `echo $$ > "${keptPid}"; sleep 30`, keep: true }, ctx()); + await tool.run({ cmd: `echo $$ > "${laterPid}"; sleep 30` }, ctx()); + await tool.run({ wait: 3, keep: true }, ctx()); + const [plain, kept, later] = await Promise.all( + [plainPid, keptPid, laterPid].map(readPid), + ); + jobs.endTurn(SESSION); + await waitForExit(plain!); + expect(isAlive(kept!)).toBe(true); + expect(isAlive(later!)).toBe(true); + expect(jobs.list(SESSION).map((r) => r.id)).toEqual([2, 3]); + jobs.endSession(SESSION); + await waitForExit(kept!); + await waitForExit(later!); + }); + + it("at maxJobs the next detach stops the oldest un-kept job and says so", async () => { + jobs = new ShellJobRegistry({ maxJobs: 1 }); + const tool = shellTool(DEFAULT_WAIT_MS, jobs); + const firstPid = join(dir, "first.pid"); + await tool.run({ cmd: `echo $$ > "${firstPid}"; sleep 30` }, ctx()); + const first = await readPid(firstPid); + const second = await tool.run({ cmd: "sleep 30" }, ctx()); + expect(second.details).toMatchObject({ detached: true, jobId: 2, evictedJobId: 1 }); + expect(second.summary).toContain( + "job 1 (echo $$ >", + ); + expect(second.summary).toContain( + "was stopped to stay within 1 running jobs — the oldest un-kept job", + ); + await waitForExit(first); + expect(jobs.get(SESSION, 1)?.state).toBe("killed"); + expect(jobs.get(SESSION, 2)?.state).toBe("running"); + }); + + it("jobMaxMs is an absolute ceiling from the start; a later wait reports the stop", async () => { + // The ceiling is two seconds from the spawn: one past the detach. + jobs = new ShellJobRegistry({ jobMaxMs: 2_000 }); + const tool = shellTool(DEFAULT_WAIT_MS, jobs); + const pidFile = join(dir, "pid"); + await tool.run({ cmd: `echo $$ > "${pidFile}"; sleep 30`, keep: true }, ctx()); + const pid = await readPid(pidFile); + await waitForExit(pid); + const collected = await tool.run({ wait: 1, timeoutMs: 10_000 }, ctx()); + expect(collected.status).toBe("ok"); + expect(collected.details).toMatchObject({ killed: true, stopReason: "ceiling" }); + expect(collected.summary).toContain("killed (job 1, at the job ceiling)"); + }); + + it("an explicit timeoutMs still kills the group, and registers no job", async () => { + const pidFile = join(dir, "background.pid"); + const result = await shellTool(600_000, jobs).run( + { cmd: `sleep 30 & echo $! > "${pidFile}"; sleep 30`, timeoutMs: 1_500 }, + ctx(), + ); + expect(result.status).toBe("error"); + expect(result.details).toMatchObject({ + timedOut: true, + timeoutMs: 1_500, + source: "explicit", + }); + expect(result.details.detached).toBeUndefined(); + expect(result.summary).toContain("stopped after 1.5 s (timeoutMs)"); + await waitForExit(await readPid(pidFile)); + expect(jobs.list(SESSION)).toEqual([]); + }); + + it("a cancelled turn does not wait on a wait: it returns at once and the job keeps running", async () => { + const tool = shellTool(DEFAULT_WAIT_MS, jobs); + const pidFile = join(dir, "pid"); + await tool.run({ cmd: `echo $$ > "${pidFile}"; sleep 30` }, ctx()); + const controller = new AbortController(); + const pending = tool.run({ wait: 1, timeoutMs: 0 }, ctx(controller.signal)); + setTimeout(() => controller.abort(), 100); + const startedAt = Date.now(); + const result = await pending; + expect(Date.now() - startedAt).toBeLessThan(2_000); + expect(result.details).toMatchObject({ detached: true, jobId: 1 }); + expect(isAlive(await readPid(pidFile))).toBe(true); + }); + + it("refuses a call that mixes forms, a wait that is not a job id, and an unknown job", async () => { + const tool = shellTool(DEFAULT_WAIT_MS, jobs); + const mixed = await tool.run({ cmd: "ls", wait: 1 }, ctx()); + expect(mixed.status).toBe("error"); + expect(mixed.summary).toContain("pass one of cmd, wait, kill, jobs (got cmd, wait)"); + const bad = await tool.run({ wait: "soon" }, ctx()); + expect(bad.status).toBe("error"); + expect(bad.summary).toContain("wait must be a job id"); + const unknown = await tool.run({ kill: 9 }, ctx()); + expect(unknown.status).toBe("error"); + expect(unknown.summary).toContain("unknown job 9 in this session"); + // No form at all is the pre-existing `cmd` refusal. + await expect(tool.run({}, ctx())).rejects.toThrow(/`cmd` must be a non-empty string/); + }); + }, +); diff --git a/src/tools/os/shell-interpretation.ts b/src/tools/os/shell-interpretation.ts new file mode 100644 index 00000000..cd11aa29 --- /dev/null +++ b/src/tools/os/shell-interpretation.ts @@ -0,0 +1,165 @@ +/** + * How `os.shell.run` reads its `cmd` / `args`: the argument coercion, the + * direct-exec vs subshell decision, and the interpreter shapes whose + * approval grant is withheld. Split out of shell.ts, which keeps the + * tool itself. + */ + +/** + * Coerce the model-supplied `args` field into a string array. Returns + * the parsed list when the input is well-formed, or `null` when the + * input has the wrong shape so the caller can return a structured + * error to the model. Accepts: + * - `undefined` / missing -> [] (no extra args) + * - `string[]` -> coerced via String() + * - JSON-stringified array literal (some cloud providers + * double-serialise tool_call arguments) -> parsed + coerced + * Anything else (object, scalar string with no JSON shape, number, + * etc.) returns `null` and triggers the structured error path. + */ +export function coerceShellArgs(value: unknown): string[] | null { + if (value === undefined || value === null) return []; + if (Array.isArray(value)) { + return value.map((v) => String(v)); + } + if (typeof value === "string") { + const trimmed = value.trim(); + if (trimmed.length === 0) return []; + if (trimmed.startsWith("[") && trimmed.endsWith("]")) { + try { + const parsed = JSON.parse(trimmed) as unknown; + if (Array.isArray(parsed)) { + return parsed.map((v) => String(v)); + } + } catch { + // fall through to error + } + } + } + return null; +} + +export function describeArgsShape(value: unknown): string { + if (value === undefined) return "undefined"; + if (value === null) return "null"; + if (Array.isArray(value)) return "array"; + return typeof value; +} + +/** + * Shell metacharacters that only mean something inside a subshell (pipes, + * sequencing, redirects, command/parameter substitution, grouping). Note + * `*`/`?` are deliberately excluded — argv globs are expanded by + * `expandShellGlobArgs` on the direct-exec path, so a bare `{cmd:"ls", + * args:["*.png"]}` keeps working without spawning a subshell. + */ +const SHELL_METACHAR_RE = /[|&;<>$`(){}]/; + +/** + * `cmd.exe` internal commands that have no standalone executable on PATH. + * A direct `spawn("echo", …)` fails with ENOENT on Windows because these + * only exist inside the command interpreter — they must be routed through + * the `cmd.exe` subshell. Real executables (`where.exe`, `find.exe`, + * `sort.exe`, `more.com`) are intentionally excluded so they keep their + * direct-exec argv semantics. + */ +const WINDOWS_CMD_BUILTINS: ReadonlySet = new Set([ + "assoc", + "call", + "cd", + "chdir", + "cls", + "color", + "copy", + "date", + "del", + "dir", + "echo", + "erase", + "ftype", + "md", + "mkdir", + "mklink", + "move", + "path", + "pause", + "popd", + "prompt", + "pushd", + "rd", + "rem", + "ren", + "rename", + "rmdir", + "set", + "start", + "time", + "title", + "type", + "ver", + "verify", + "vol", +]); + +function isWindowsCmdBuiltin(cmd: string): boolean { + // Builtins are never invoked by path, so a direct lowercase lookup is + // sufficient — no basename stripping needed. + return WINDOWS_CMD_BUILTINS.has(cmd.trim().toLowerCase()); +} + +/** + * Decide whether `cmd` must be run through the OS subshell (`sh -c` / + * `cmd.exe /c`) instead of a direct `spawn(cmd, args)`. Models routinely + * emit a full shell command line in the `cmd` field (e.g. + * `"ffprobe -v quiet ... f.mp3"` or `"pip3 list | grep foo"`). With a + * direct exec that string is treated as a literal executable name and + * fails with ENOENT. We route to a subshell when `cmd` carries shell + * metacharacters, when it looks like a pre-joined command line (whitespace + * present and no separate `args`), or — on Windows — when `cmd` is a + * `cmd.exe` builtin (`echo`, `dir`, `type`, …) that has no standalone + * executable to spawn directly. + */ +export function needsShellInterpretation( + cmd: string, + args: readonly string[], +): boolean { + if (SHELL_METACHAR_RE.test(cmd)) return true; + // On Windows the model may emit `%VAR%` expansion, which only means + // something inside a `cmd.exe` subshell. `$` (POSIX) is already covered + // by SHELL_METACHAR_RE above. + if (process.platform === "win32" && /%[^%\s]+%/.test(cmd)) return true; + // A bare cmd.exe builtin must go through the interpreter or `spawn` + // ENOENTs. `cmd` here is a single token (metachar/pre-joined cases are + // handled above), so a straight builtin lookup is safe. + if ( + process.platform === "win32" && + !/\s/.test(cmd.trim()) && + isWindowsCmdBuiltin(cmd) + ) { + return true; + } + if (args.length === 0 && /\s/.test(cmd.trim())) return true; + return false; +} + +/** + * Interpreter / wrapper binaries whose danger lives in their arguments, + * not their name (`bash -c ""`). The shell tool withholds the + * shape grant for these: a grant keyed on `bash` would silence + * arbitrary code for the rest of the session. Matches the shells + * covered by the guard's `dangerous.shell_dash_c` rule. The category + * grant (the whole shell category) and a plain approve (this call only) + * stay available. + */ +const OPAQUE_INTERPRETER_SHAPES: ReadonlySet = new Set([ + "bash", + "sh", + "zsh", + "dash", + "ksh", +]); + +/** True when `[a]` (shape grant) must be withheld for `shape`. */ +export function isOpaqueInterpreterShape(shape: string): boolean { + return OPAQUE_INTERPRETER_SHAPES.has(shape); +} diff --git a/src/tools/os/shell-job-calls.ts b/src/tools/os/shell-job-calls.ts new file mode 100644 index 00000000..767939b7 --- /dev/null +++ b/src/tools/os/shell-job-calls.ts @@ -0,0 +1,291 @@ +import { + compressToolResult, + type CompressedToolResult, +} from "../../compressor/result-compressor.js"; +import { + awaitJobExit, + type CommandJobExit, + type CommandJobOutput, +} from "../../sandbox/command-job.js"; +import type { + ShellJobRecord, + ShellJobRegistry, + ShellJobStopReason, +} from "./shell-jobs.js"; +import { + formatExitStatus, + joinShellOutput, + renderShellExit, + renderShellResult, + tailShellOutput, +} from "./shell-result.js"; +import { + formatShellDetachNotice, + formatShellElapsed, + resolveShellTimeout, +} from "./shell-timeout.js"; + +/** + * The job forms of `os.shell.run` — `{wait}`, `{kill}`, `{jobs}` — and + * the results a detached job produces. The tool (shell.ts) classifies + * the call and hands these the registry; the `cmd` form stays there. + */ + +/** + * Output lines a still-running or killed result shows. The result + * compressor keeps the last twelve non-blank lines, so the notice above + * the command line survives only when the body is shorter than that. + */ +const RUNNING_TAIL_LINES = 8; +const KILLED_TAIL_LINES = 9; + +export type ShellCallForm = + | { kind: "cmd" } + | { kind: "jobs" } + | { kind: "wait"; id: number } + | { kind: "kill"; id: number } + | { kind: "invalid"; message: string }; + +function isPresent(value: unknown): boolean { + return value !== undefined && value !== null && value !== false; +} + +function parseJobId(raw: unknown): number | null { + const value = + typeof raw === "number" + ? raw + : typeof raw === "string" && /^\d+$/.test(raw.trim()) + ? Number(raw.trim()) + : NaN; + return Number.isInteger(value) && value > 0 ? value : null; +} + +/** Which form a call is; exactly one of `cmd` / `wait` / `kill` / `jobs`. */ +export function classifyShellCall( + rawArgs: Record, +): ShellCallForm { + const present = ["cmd", "wait", "kill", "jobs"].filter((key) => + isPresent(rawArgs[key]), + ); + if (present.length > 1) { + return { + kind: "invalid", + message: `os.shell.run: pass one of cmd, wait, kill, jobs (got ${present.join(", ")})`, + }; + } + if (isPresent(rawArgs.jobs)) return { kind: "jobs" }; + for (const kind of ["wait", "kill"] as const) { + if (!isPresent(rawArgs[kind])) continue; + const id = parseJobId(rawArgs[kind]); + if (id === null) { + return { + kind: "invalid", + message: `os.shell.run: ${kind} must be a job id (a positive integer), e.g. {"${kind}": 3}; {"jobs": true} lists them`, + }; + } + return { kind, id }; + } + return { kind: "cmd" }; +} + +export interface ShellJobCallContext { + jobs: ShellJobRegistry; + sessionId: string; + /** The per-call wait a bare `{wait}` gets; `0` = unbounded. */ + defaultTimeoutMs: number; + /** The turn's cancel: a wait returns promptly, the job keeps running. */ + signal: AbortSignal; +} + +/** The first characters of a command line, for a list or a notice. */ +export function headOfCommand(commandLine: string, max = 60): string { + const line = commandLine.replace(/\s+/g, " ").trim(); + return line.length > max ? `${line.slice(0, max - 1)}…` : line; +} + +const STOP_WORDING: Record = { + kill: "on request", + turn_end: "at the turn's end", + session_end: "at the session's end", + evicted: "to stay within the job limit", + ceiling: "at the job ceiling", + shutdown: "at shutdown", +}; + +export interface ShellDetachRender { + waitedMs: number; + again: boolean; + defaultTimeoutMs: number; + evicted?: ShellJobRecord | null; + maxJobs?: number; + notices?: readonly string[]; +} + +/** A job given back still running: the notice, the tail, the id. */ +export function renderShellDetached( + record: ShellJobRecord, + input: ShellDetachRender, +): CompressedToolResult { + const output = record.job.output(); + const runningMs = Date.now() - record.job.startedAt; + const notices = [...(input.notices ?? [])]; + if (input.evicted) { + const which = input.evicted.keep ? "job (kept ones included)" : "un-kept job"; + notices.push( + `job ${input.evicted.id} (${headOfCommand(input.evicted.facts.commandLine)}) was stopped to stay within ${input.maxJobs ?? "the limit of"} running jobs — the oldest ${which}`, + ); + } + notices.push( + formatShellDetachNotice({ + jobId: record.id, + waitedMs: input.waitedMs, + again: input.again, + defaultTimeoutMs: input.defaultTimeoutMs, + }), + ); + return renderShellResult({ + facts: record.facts, + status: "ok", + notices, + statusLine: `still running (job ${record.id}, pid ${record.job.pid ?? "?"}, ${formatShellElapsed(runningMs)}${record.keep ? ", kept" : ""})`, + body: tailShellOutput(joinShellOutput(output), RUNNING_TAIL_LINES), + details: { + detached: true, + jobId: record.id, + pid: record.job.pid ?? null, + runningMs, + keep: record.keep, + timedOut: false, + truncated: output.truncated, + ...(input.evicted ? { evictedJobId: input.evicted.id } : {}), + }, + }); +} + +/** A job that was stopped — by a `kill`, an eviction or the ceiling — once it is gone. */ +function renderShellKilled( + record: ShellJobRecord, + exit: CommandJobExit, + output: CommandJobOutput, +): CompressedToolResult { + const reason = record.stopReason ?? "kill"; + return renderShellResult({ + facts: record.facts, + status: "ok", + notices: [], + statusLine: `killed (job ${record.id}, ${STOP_WORDING[reason]}) after ${formatShellElapsed(exit.durationMs)}, exit: ${formatExitStatus(exit)}`, + body: tailShellOutput(joinShellOutput(output), KILLED_TAIL_LINES), + details: { + killed: true, + jobId: record.id, + pid: record.job.pid ?? null, + stopReason: reason, + exitCode: exit.exitCode, + signal: exit.signal, + durationMs: exit.durationMs, + timedOut: false, + truncated: output.truncated, + }, + }); +} + +function unknownJob(id: number): CompressedToolResult { + return compressToolResult({ + tool: "os.shell.run", + status: "error", + output: `unknown job ${id} in this session — os.shell.run {"jobs": true} lists the jobs it has (a job that was not kept is dropped when its turn ends)`, + details: { jobId: id, unknownJob: true }, + }); +} + +/** Report a finished job the way a fresh result would, and forget it. */ +async function collect( + jobs: ShellJobRegistry, + record: ShellJobRecord, +): Promise { + const exit = await awaitJobExit(record.job); + jobs.drop(record); + const output = record.job.output(); + if (record.state === "killed") return renderShellKilled(record, exit, output); + return renderShellExit(record.facts, exit, output, { jobId: record.id }); +} + +/** + * `{wait: id, timeoutMs?, keep?}`: block until the job exits or the + * wait (the explicit `timeoutMs`, else the default) elapses. A turn + * cancelled mid-wait gets the still-running result at once. + */ +export async function runShellWait( + ctx: ShellJobCallContext, + id: number, + rawTimeoutMs: unknown, + keep: boolean, +): Promise { + const record = ctx.jobs.get(ctx.sessionId, id); + if (!record) return unknownJob(id); + if (keep) ctx.jobs.markKeep(record); + const timeout = resolveShellTimeout(rawTimeoutMs, ctx.defaultTimeoutMs); + if (record.state === "running") { + const outcome = await record.job + .waitFor(timeout.timeoutMs, ctx.signal) + .catch(() => "exited" as const); + if (outcome !== "exited") { + return renderShellDetached(record, { + waitedMs: timeout.timeoutMs, + again: true, + defaultTimeoutMs: ctx.defaultTimeoutMs, + }); + } + } + return collect(ctx.jobs, record); +} + +/** `{kill: id}`: stop the job's whole process group and report its tail. */ +export async function runShellKill( + ctx: ShellJobCallContext, + id: number, +): Promise { + const record = ctx.jobs.get(ctx.sessionId, id); + if (!record) return unknownJob(id); + ctx.jobs.stop(record, "kill"); + return collect(ctx.jobs, record); +} + +function describeJobState(record: ShellJobRecord): string { + const exit = record.job.exited(); + if (record.state === "running") { + return `running ${formatShellElapsed(Date.now() - record.job.startedAt)}${record.keep ? ", kept" : ""} (pid ${record.job.pid ?? "?"})`; + } + const collectHint = `{"wait": ${record.id}} collects the output`; + if (record.state === "killed") { + return `stopped ${STOP_WORDING[record.stopReason ?? "kill"]} — ${collectHint}`; + } + return `exited ${exit ? formatExitStatus(exit) : "?"} after ${formatShellElapsed(exit?.durationMs ?? 0)} — ${collectHint}`; +} + +/** `{jobs: true}`: this session's jobs — id, command head, started, state. */ +export function listShellJobs(ctx: ShellJobCallContext): CompressedToolResult { + const records = ctx.jobs.list(ctx.sessionId); + const lines = records.map( + (record) => + `job ${record.id}: ${headOfCommand(record.facts.commandLine)} — ${describeJobState(record)}`, + ); + return compressToolResult({ + tool: "os.shell.run", + status: "ok", + output: lines.length > 0 ? lines.join("\n") : "no jobs in this session", + details: { + jobs: records.map((record) => ({ + id: record.id, + cmd: headOfCommand(record.facts.commandLine, 200), + startedAt: new Date(record.job.startedAt).toISOString(), + state: record.state, + pid: record.job.pid ?? null, + keep: record.keep, + runningMs: Date.now() - record.job.startedAt, + exitCode: record.job.exited()?.exitCode ?? null, + stopReason: record.stopReason, + })), + }, + }); +} diff --git a/src/tools/os/shell-jobs.test.ts b/src/tools/os/shell-jobs.test.ts new file mode 100644 index 00000000..84a339e6 --- /dev/null +++ b/src/tools/os/shell-jobs.test.ts @@ -0,0 +1,183 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import type { CommandJob, CommandJobExit } from "../../sandbox/command-job.js"; +import { checkShellCommandGuard } from "./shell-command-guard/index.js"; +import { ShellJobRegistry } from "./shell-jobs.js"; +import type { ShellCommandFacts } from "./shell-result.js"; + +/** + * The registry's ownership rules, with a fake job so no process is + * involved: ids per session, turn-end vs `keep`, session-end, the job + * limit's eviction order, the ceiling. + */ + +interface FakeJob extends CommandJob { + stops: number; + finish(exit: CommandJobExit): void; +} + +function fakeJob(startedAt = Date.now()): FakeJob { + let resolveExit!: (exit: CommandJobExit) => void; + let exited: CommandJobExit | null = null; + const exit = new Promise((resolve) => { + resolveExit = resolve; + }); + const job: FakeJob = { + command: "sleep", + args: ["30"], + pid: 4242, + startedAt, + exit, + stops: 0, + exited: () => exited, + output: () => ({ stdout: "", stderr: "", truncated: false }), + waitFor: () => Promise.resolve("elapsed" as const), + stop() { + job.stops += 1; + }, + kill() { + job.stops += 1; + }, + finish(value) { + exited = value; + resolveExit(value); + }, + }; + return job; +} + +function facts(cmd: string): ShellCommandFacts { + return { + cmd, + args: [], + rawArgs: [], + cwd: "/tmp", + shell: false, + commandLine: cmd, + noArguments: false, + gog: false, + guard: checkShellCommandGuard({ cmd, rawArgs: [], cwd: "/tmp" }), + }; +} + +describe("ShellJobRegistry", () => { + it("numbers jobs per session from 1 and hides them from other sessions", () => { + const registry = new ShellJobRegistry(); + const a1 = registry.register("a", fakeJob(), facts("one"), false).record; + const a2 = registry.register("a", fakeJob(), facts("two"), false).record; + const b1 = registry.register("b", fakeJob(), facts("three"), false).record; + expect([a1.id, a2.id, b1.id]).toEqual([1, 2, 1]); + expect(registry.get("a", 2)).toBe(a2); + expect(registry.get("b", 2)).toBeUndefined(); + expect(registry.list("a").map((r) => r.id)).toEqual([1, 2]); + }); + + it("endTurn stops the un-kept jobs and leaves kept ones for a later turn", () => { + const registry = new ShellJobRegistry(); + const plain = fakeJob(); + const kept = fakeJob(); + registry.register("s", plain, facts("plain"), false); + const keptRecord = registry.register("s", kept, facts("kept"), true).record; + const stopped = registry.endTurn("s"); + expect(stopped.map((r) => r.facts.cmd)).toEqual(["plain"]); + expect(plain.stops).toBe(1); + expect(kept.stops).toBe(0); + expect(registry.list("s")).toEqual([keptRecord]); + expect(keptRecord.state).toBe("running"); + }); + + it("a later markKeep protects a job the call did not keep", () => { + const registry = new ShellJobRegistry(); + const job = fakeJob(); + const record = registry.register("s", job, facts("x"), false).record; + registry.markKeep(record); + expect(registry.endTurn("s")).toEqual([]); + expect(job.stops).toBe(0); + }); + + it("endSession and endAll stop kept jobs too", () => { + const registry = new ShellJobRegistry(); + const a = fakeJob(); + const b = fakeJob(); + registry.register("s", a, facts("a"), true); + registry.register("t", b, facts("b"), true); + expect(registry.endSession("s").length).toBe(1); + expect(a.stops).toBe(1); + expect(registry.list("s")).toEqual([]); + expect(registry.endAll().map((r) => r.facts.cmd)).toEqual(["b"]); + expect(b.stops).toBe(1); + }); + + it("stop marks the record killed with its reason, and only once", () => { + const registry = new ShellJobRegistry(); + const job = fakeJob(); + const record = registry.register("s", job, facts("x"), false).record; + expect(registry.stop(record, "kill")).toBe(true); + expect(registry.stop(record, "ceiling")).toBe(false); + expect(record).toMatchObject({ state: "killed", stopReason: "kill" }); + expect(job.stops).toBe(1); + }); + + it("marks a job exited when its process closes, and drop forgets it", async () => { + const registry = new ShellJobRegistry(); + const job = fakeJob(); + const record = registry.register("s", job, facts("x"), false).record; + job.finish({ exitCode: 0, signal: null, durationMs: 10 }); + await job.exit; + expect(record.state).toBe("exited"); + registry.drop(record); + expect(registry.get("s", 1)).toBeUndefined(); + }); + + it("at maxJobs the next detach evicts the oldest un-kept job, then the oldest kept one", () => { + const registry = new ShellJobRegistry({ maxJobs: 2 }); + const first = fakeJob(); + const second = fakeJob(); + const r1 = registry.register("s", first, facts("first"), true).record; + registry.register("s", second, facts("second"), false); + const third = registry.register("s", fakeJob(), facts("third"), false); + // `second` is the oldest un-kept one; `first` is older but kept. + expect(third.evicted?.facts.cmd).toBe("second"); + expect(second.stops).toBe(1); + expect(first.stops).toBe(0); + // Now first (kept) and third (un-kept) run; keep third too, so the + // next detach has no un-kept job and takes the oldest kept one. + registry.markKeep(third.record); + const fourth = registry.register("s", fakeJob(), facts("fourth"), false); + expect(fourth.evicted).toBe(r1); + expect(first.stops).toBe(1); + expect(r1.stopReason).toBe("evicted"); + }); + + describe("ceiling", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + afterEach(() => { + vi.useRealTimers(); + }); + + it("stops a job jobMaxMs after its start, whether kept or not", () => { + const registry = new ShellJobRegistry({ jobMaxMs: 10_000 }); + // Started 4 s before the detach: 6 s of ceiling remain. + const job = fakeJob(Date.now() - 4_000); + const record = registry.register("s", job, facts("x"), true).record; + vi.advanceTimersByTime(5_900); + expect(job.stops).toBe(0); + vi.advanceTimersByTime(200); + expect(job.stops).toBe(1); + expect(record).toMatchObject({ state: "killed", stopReason: "ceiling" }); + }); + + it("does not fire for a job that was dropped or already stopped", () => { + const registry = new ShellJobRegistry({ jobMaxMs: 1_000 }); + const dropped = fakeJob(); + const stopped = fakeJob(); + registry.drop(registry.register("s", dropped, facts("a"), false).record); + registry.stop(registry.register("s", stopped, facts("b"), false).record, "kill"); + vi.advanceTimersByTime(2_000); + expect(dropped.stops).toBe(0); + expect(stopped.stops).toBe(1); + }); + }); +}); diff --git a/src/tools/os/shell-jobs.ts b/src/tools/os/shell-jobs.ts new file mode 100644 index 00000000..65497978 --- /dev/null +++ b/src/tools/os/shell-jobs.ts @@ -0,0 +1,217 @@ +import type { CommandJob } from "../../sandbox/command-job.js"; +import type { ShellCommandFacts } from "./shell-result.js"; + +/** + * The jobs `os.shell.run` detached instead of killing (F47), per + * session. A command still running when the operator's default timeout + * elapses keeps running here, in its own process group, with its output + * captured; the model reaches it by id through the `wait` / `kill` / + * `jobs` forms of the same tool. + * + * Ownership: a job dies when its turn ends unless the call that started + * it (or a later `wait`) carried `keep: true`; every job dies when its + * session ends, at the process's shutdown, and at the absolute ceiling + * `tools.shell.jobMaxMs` counted from its start. At most + * `tools.shell.maxJobs` run per session: the next detach stops the + * oldest un-kept one (the oldest kept one when all are kept) and the + * result says so. The bootstrap owns the one registry and calls + * `endTurn` / `endSession` / `endAll` from the turn, session and + * shutdown paths; a tool built without one gets a private registry + * whose jobs die at the ceiling only. + */ + +export const DEFAULT_SHELL_JOB_MAX_MS = 3_600_000; +export const DEFAULT_SHELL_MAX_JOBS = 3; +/** Finished, uncollected records a session may hold before the oldest goes. */ +const MAX_FINISHED_RECORDS = 20; + +export type ShellJobState = "running" | "exited" | "killed"; + +export type ShellJobStopReason = + | "kill" + | "turn_end" + | "session_end" + | "evicted" + | "ceiling" + | "shutdown"; + +export interface ShellJobRecord { + /** Per session, from 1 — what the model quotes back. */ + readonly id: number; + readonly sessionId: string; + /** What the command was, so a collected result reads like a fresh one. */ + readonly facts: ShellCommandFacts; + readonly job: CommandJob; + keep: boolean; + /** `killed` is set when the stop is sent, before the process is gone. */ + state: ShellJobState; + stopReason: ShellJobStopReason | null; +} + +export interface ShellJobRegistryOptions { + /** Absolute ceiling per job, from its start. Default one hour. */ + jobMaxMs?: number; + /** Running jobs per session. Default 3. */ + maxJobs?: number; +} + +function positiveOr(value: number | undefined, fallback: number): number { + return typeof value === "number" && Number.isFinite(value) && value > 0 + ? Math.floor(value) + : fallback; +} + +export class ShellJobRegistry { + readonly jobMaxMs: number; + readonly maxJobs: number; + private readonly bySession = new Map(); + private readonly nextIds = new Map(); + private readonly ceilings = new Map< + ShellJobRecord, + ReturnType + >(); + + constructor(options: ShellJobRegistryOptions = {}) { + this.jobMaxMs = positiveOr(options.jobMaxMs, DEFAULT_SHELL_JOB_MAX_MS); + this.maxJobs = positiveOr(options.maxJobs, DEFAULT_SHELL_MAX_JOBS); + } + + /** + * Take a job the default timeout detached. When the session already + * runs `maxJobs`, the oldest un-kept running job (the oldest kept one + * when all are kept) is stopped first and returned as `evicted`. + */ + register( + sessionId: string, + job: CommandJob, + facts: ShellCommandFacts, + keep: boolean, + ): { record: ShellJobRecord; evicted: ShellJobRecord | null } { + const records = this.records(sessionId); + let evicted: ShellJobRecord | null = null; + const running = records.filter((r) => r.state === "running"); + if (running.length >= this.maxJobs) { + // Records are in start order, so the first match is the oldest. + evicted = running.find((r) => !r.keep) ?? running[0]!; + this.stop(evicted, "evicted"); + } + const id = this.nextIds.get(sessionId) ?? 1; + this.nextIds.set(sessionId, id + 1); + const record: ShellJobRecord = { + id, + sessionId, + facts, + job, + keep, + state: "running", + stopReason: null, + }; + records.push(record); + const settle = () => { + if (record.state === "running") record.state = "exited"; + this.clearCeiling(record); + }; + job.exit.then(settle, settle); + // The ceiling counts from the spawn, not from the detach. + const remaining = Math.max(0, this.jobMaxMs - (Date.now() - job.startedAt)); + const timer = setTimeout(() => this.stop(record, "ceiling"), remaining); + // The registry must never be what keeps the process alive. + timer.unref(); + this.ceilings.set(record, timer); + this.trimFinished(records); + return { record, evicted }; + } + + /** A job of this session only — another session's id is unknown here. */ + get(sessionId: string, id: number): ShellJobRecord | undefined { + return this.bySession.get(sessionId)?.find((r) => r.id === id); + } + + /** This session's records, oldest first; finished ones until collected. */ + list(sessionId: string): ShellJobRecord[] { + return [...(this.bySession.get(sessionId) ?? [])]; + } + + markKeep(record: ShellJobRecord): void { + record.keep = true; + } + + /** + * Stop a running job politely (`SIGTERM`, then `SIGKILL` after the + * grace) and mark why. `false` when it was not running. + */ + stop(record: ShellJobRecord, reason: ShellJobStopReason): boolean { + if (record.state !== "running") return false; + record.state = "killed"; + record.stopReason = reason; + this.clearCeiling(record); + record.job.stop(); + return true; + } + + /** Forget a record whose result was reported (a `wait` or a `kill` collected it). */ + drop(record: ShellJobRecord): void { + this.clearCeiling(record); + const records = this.bySession.get(record.sessionId); + if (!records) return; + const index = records.indexOf(record); + if (index >= 0) records.splice(index, 1); + if (records.length === 0) this.bySession.delete(record.sessionId); + } + + /** + * The turn ended: every un-kept job of the session is stopped and + * forgotten, collected or not. Kept ones stay, running or exited, + * for a later turn's `wait`. Returns the jobs that were stopped. + */ + endTurn(sessionId: string): ShellJobRecord[] { + return this.endRecords(this.list(sessionId).filter((r) => !r.keep), "turn_end"); + } + + /** The session ended: every job of it, kept or not. */ + endSession(sessionId: string): ShellJobRecord[] { + return this.endRecords(this.list(sessionId), "session_end"); + } + + /** The process is shutting down: every job of every session. */ + endAll(): ShellJobRecord[] { + const all = [...this.bySession.keys()].flatMap((id) => this.list(id)); + return this.endRecords(all, "shutdown"); + } + + private endRecords( + records: ShellJobRecord[], + reason: ShellJobStopReason, + ): ShellJobRecord[] { + const stopped: ShellJobRecord[] = []; + for (const record of records) { + if (this.stop(record, reason)) stopped.push(record); + this.drop(record); + } + return stopped; + } + + private records(sessionId: string): ShellJobRecord[] { + let records = this.bySession.get(sessionId); + if (!records) { + records = []; + this.bySession.set(sessionId, records); + } + return records; + } + + private clearCeiling(record: ShellJobRecord): void { + const timer = this.ceilings.get(record); + if (timer) clearTimeout(timer); + this.ceilings.delete(record); + } + + /** Bound the memory a session's uncollected output can hold. */ + private trimFinished(records: ShellJobRecord[]): void { + let finished = records.filter((r) => r.state !== "running"); + while (finished.length > MAX_FINISHED_RECORDS) { + this.drop(finished[0]!); + finished = finished.slice(1); + } + } +} diff --git a/src/tools/os/shell-result.ts b/src/tools/os/shell-result.ts new file mode 100644 index 00000000..03677056 --- /dev/null +++ b/src/tools/os/shell-result.ts @@ -0,0 +1,159 @@ +import { + compressToolResult, + type CompressedToolResult, +} from "../../compressor/result-compressor.js"; +import type { + CommandJobExit, + CommandJobOutput, +} from "../../sandbox/command-job.js"; +import type { GuardVerdict } from "./shell-command-guard/index.js"; +import { + formatShellTimeoutNotice, + type ResolvedShellTimeout, +} from "./shell-timeout.js"; + +/** + * How an `os.shell.run` result is put together, whichever way the + * command ended — at its exit, at an explicit timeout, detached at the + * default one, or collected later by a `wait` / `kill`. One renderer so + * a job's result reads exactly like a fresh run's. + */ + +export const GOG_MAX_OUTPUT_BYTES = 4 * 1024 * 1024; +const GOG_COMPRESS_OPTIONS = { + maxSummaryLength: 64_000, + maxTailLines: 10_000, +} as const; + +/** What a result says about the command, fixed when it was started. */ +export interface ShellCommandFacts { + cmd: string; + /** The argv that ran (globs expanded on the direct-exec path). */ + args: string[]; + rawArgs: string[]; + cwd: string; + /** Ran through the OS subshell rather than a direct exec. */ + shell: boolean; + commandLine: string; + /** A bare interpreter ran with nothing after it (F40) — said on the command line. */ + noArguments: boolean; + /** `gog` output is kept far longer than any other command's. */ + gog: boolean; + guard: GuardVerdict; +} + +export interface ShellResultInput { + facts: ShellCommandFacts; + status: "ok" | "error"; + /** Lines said first, above the command line. */ + notices: readonly string[]; + /** The line under the command: `exit: 0`, `still running (job 3, …)`. */ + statusLine: string; + body: string; + /** Result-specific fields, placed between the command's and the guard's. */ + details: Record; +} + +export function renderShellResult(input: ShellResultInput): CompressedToolResult { + const { facts } = input; + const header = `$ ${facts.commandLine}${facts.noArguments ? " (ran with no arguments)" : ""}\n${input.statusLine}`; + const output = [...input.notices, header, input.body] + .filter((part) => part.length > 0) + .join("\n"); + return compressToolResult( + { + tool: "os.shell.run", + status: input.status, + output, + details: { + cmd: facts.cmd, + args: facts.args, + rawArgs: facts.rawArgs, + cwd: facts.cwd, + shell: facts.shell, + ...input.details, + guardVerdict: facts.guard.action, + guardRule: facts.guard.rule, + guardReason: facts.guard.reason, + }, + }, + facts.gog ? GOG_COMPRESS_OPTIONS : {}, + ); +} + +/** stdout and stderr, the non-empty ones, separated. */ +export function joinShellOutput(output: CommandJobOutput): string { + return [output.stdout, output.stderr] + .filter((s) => s.trim().length > 0) + .join("\n---\n"); +} + +/** + * The last `count` non-blank lines, with a marker for what was left + * out. The result compressor keeps only a short tail of the output; a + * notice above a long output survives only when the body is already + * short enough for the notice to be inside that tail. + */ +export function tailShellOutput(text: string, count: number): string { + const lines = text.split("\n").filter((line) => line.trim().length > 0); + if (lines.length <= count) return lines.join("\n"); + return `… [${lines.length - count} earlier lines]\n${lines.slice(-count).join("\n")}`; +} + +export function formatExitStatus(exit: CommandJobExit): string { + return `${exit.exitCode ?? "signal:" + exit.signal}`; +} + +/** The result of a command that ran to its exit — fresh, or collected by a `wait`. */ +export function renderShellExit( + facts: ShellCommandFacts, + exit: CommandJobExit, + output: CommandJobOutput, + extra: { notices?: readonly string[]; jobId?: number } = {}, +): CompressedToolResult { + return renderShellResult({ + facts, + status: exit.exitCode === 0 ? "ok" : "error", + notices: extra.notices ?? [], + statusLine: `exit: ${formatExitStatus(exit)}`, + body: joinShellOutput(output), + details: { + exitCode: exit.exitCode, + signal: exit.signal, + durationMs: exit.durationMs, + timedOut: false, + ...(extra.jobId === undefined ? {} : { jobId: extra.jobId }), + truncated: output.truncated, + }, + }); +} + +/** + * The result of a command stopped at the model's own `timeoutMs`. Said + * first, above the command line: which limit stopped it and what to + * pass for a longer run. + */ +export function renderShellTimedOut( + facts: ShellCommandFacts, + exit: CommandJobExit, + output: CommandJobOutput, + timeout: ResolvedShellTimeout, + notices: readonly string[] = [], +): CompressedToolResult { + return renderShellResult({ + facts, + status: "error", + notices: [...notices, formatShellTimeoutNotice(timeout)], + statusLine: `exit: ${formatExitStatus(exit)} (timed out)`, + body: joinShellOutput(output), + details: { + exitCode: exit.exitCode, + signal: exit.signal, + durationMs: exit.durationMs, + timedOut: true, + timeoutMs: timeout.timeoutMs, + source: timeout.source, + truncated: output.truncated, + }, + }); +} diff --git a/src/tools/os/shell-timeout.test.ts b/src/tools/os/shell-timeout.test.ts new file mode 100644 index 00000000..0e2e1f86 --- /dev/null +++ b/src/tools/os/shell-timeout.test.ts @@ -0,0 +1,127 @@ +import { describe, expect, it } from "vitest"; + +import { + describeShellTimeoutDefault, + formatShellDetachNotice, + formatShellDuration, + formatShellElapsed, + formatShellTimeoutNotice, + resolveShellTimeout, +} from "./shell-timeout.js"; + +describe("resolveShellTimeout", () => { + it("takes the configured default when timeoutMs is omitted", () => { + expect(resolveShellTimeout(undefined, 600_000)).toEqual({ + timeoutMs: 600_000, + source: "default", + }); + }); + + it("lets an explicit timeoutMs win over the default, 0 included", () => { + expect(resolveShellTimeout(5_000, 600_000)).toEqual({ + timeoutMs: 5_000, + source: "explicit", + }); + expect(resolveShellTimeout(0, 600_000)).toEqual({ + timeoutMs: 0, + source: "explicit", + }); + }); + + it("reads a negative explicit value as no limit", () => { + expect(resolveShellTimeout(-1, 600_000)).toEqual({ + timeoutMs: 0, + source: "explicit", + }); + }); + + it("treats a non-numeric timeoutMs as omitted", () => { + for (const raw of [null, "5000", NaN, Infinity, true, {}]) { + expect(resolveShellTimeout(raw, 600_000)).toEqual({ + timeoutMs: 600_000, + source: "default", + }); + } + }); + + it("a default of 0 is no limit", () => { + expect(resolveShellTimeout(undefined, 0)).toEqual({ + timeoutMs: 0, + source: "default", + }); + }); +}); + +describe("formatShellDuration", () => { + it("uses minutes for whole minutes and seconds otherwise", () => { + expect(formatShellDuration(600_000)).toBe("10 min"); + expect(formatShellDuration(60_000)).toBe("1 min"); + expect(formatShellDuration(90_000)).toBe("90 s"); + expect(formatShellDuration(5_000)).toBe("5 s"); + expect(formatShellDuration(1_500)).toBe("1.5 s"); + expect(formatShellDuration(300)).toBe("0.3 s"); + }); +}); + +describe("formatShellElapsed", () => { + it("rounds to the unit a person would say", () => { + expect(formatShellElapsed(12_345)).toBe("12 s"); + expect(formatShellElapsed(754_321)).toBe("13 min"); + expect(formatShellElapsed(3_600_000)).toBe("1 h"); + expect(formatShellElapsed(4_000_000)).toBe("1 h 7 min"); + }); +}); + +describe("formatShellTimeoutNotice", () => { + it("names the model's own timeoutMs and what to pass", () => { + expect( + formatShellTimeoutNotice({ timeoutMs: 5_000, source: "explicit" }), + ).toBe( + "stopped after 5 s (timeoutMs) — output so far below; pass a larger timeoutMs for a longer run, or 0 for no limit", + ); + }); +}); + +describe("formatShellDetachNotice", () => { + it("names the job and the three forms on the first detach", () => { + expect( + formatShellDetachNotice({ + jobId: 3, + waitedMs: 600_000, + again: false, + defaultTimeoutMs: 600_000, + }), + ).toBe( + 'still running after 10 min (job 3) — output so far below; os.shell.run {"wait": 3} keeps waiting (up to another 10 min per call), {"kill": 3} stops it, pass timeoutMs for a longer first wait', + ); + }); + + it("says 'another' when a wait elapsed, and drops the per-call cap when there is none", () => { + expect( + formatShellDetachNotice({ + jobId: 3, + waitedMs: 30_000, + again: true, + defaultTimeoutMs: 0, + }), + ).toBe( + 'still running after another 30 s (job 3) — output so far below; os.shell.run {"wait": 3} keeps waiting, {"kill": 3} stops it, pass timeoutMs with the wait for a longer one', + ); + }); +}); + +describe("describeShellTimeoutDefault", () => { + it("states the default, that it detaches rather than kills, and the forms", () => { + const text = describeShellTimeoutDefault(600_000); + expect(text).toContain("default 10 min"); + expect(text).toContain("not killed but detached as a job"); + for (const form of ['{"wait": id}', '{"kill": id}', '{"jobs": true}', "keep: true"]) { + expect(text).toContain(form); + } + expect(text).toContain("Pass `timeoutMs` to stop the command at an explicit limit instead, `0` for none."); + }); + + it("says there is no timeout when the default is 0", () => { + expect(describeShellTimeoutDefault(0)).toContain("no timeout"); + }); +}); diff --git a/src/tools/os/shell-timeout.ts b/src/tools/os/shell-timeout.ts new file mode 100644 index 00000000..076d00d4 --- /dev/null +++ b/src/tools/os/shell-timeout.ts @@ -0,0 +1,105 @@ +/** + * Timeout resolution and wording for `os.shell.run` (F47). + * + * Before config v67 an omitted `timeoutMs` meant no limit at all, and a + * recursive grep over a home directory ran for twenty minutes until a + * person killed it — `agent.toolTimeoutMs` never applied to the shell. + * Now the operator's `tools.shell.defaultTimeoutMs` fills the gap, and + * the two sources part ways when they elapse: at the model's own + * `timeoutMs` the command is killed (it asked for a bound); at the + * default it is detached as a job that the model can `wait` for or + * `kill` (shell-jobs.ts) — a build the operator's default interrupted + * is not a build the model wanted stopped. + */ + +export type ShellTimeoutSource = "default" | "explicit"; + +export interface ResolvedShellTimeout { + /** Milliseconds; `0` = no limit. */ + timeoutMs: number; + /** Where the number came from — decides kill (explicit) vs detach (default). */ + source: ShellTimeoutSource; +} + +/** + * An explicit `timeoutMs` (a finite number; `0` or negative = none) + * wins; anything else — omitted, `null`, a string — takes the configured + * default, where `0` (or nothing configured) again means no limit. + */ +export function resolveShellTimeout( + rawTimeoutMs: unknown, + defaultTimeoutMs: number, +): ResolvedShellTimeout { + if (typeof rawTimeoutMs === "number" && Number.isFinite(rawTimeoutMs)) { + return { timeoutMs: Math.max(0, rawTimeoutMs), source: "explicit" }; + } + const fallback = + Number.isFinite(defaultTimeoutMs) && defaultTimeoutMs > 0 + ? defaultTimeoutMs + : 0; + return { timeoutMs: fallback, source: "default" }; +} + +/** `600000` → `10 min`, `5000` → `5 s`, `1500` → `1.5 s`. Exact; for limits. */ +export function formatShellDuration(ms: number): string { + if (ms >= 60_000 && ms % 60_000 === 0) return `${ms / 60_000} min`; + const seconds = ms / 1000; + return `${Number.isInteger(seconds) ? seconds : seconds.toFixed(1)} s`; +} + +/** `754321` → `13 min`, `4000000` → `1 h 7 min`, `12345` → `12 s`. Rounded; for elapsed time. */ +export function formatShellElapsed(ms: number): string { + const seconds = Math.round(ms / 1000); + if (seconds < 60) return `${seconds} s`; + const minutes = Math.round(seconds / 60); + if (minutes < 60) return `${minutes} min`; + const hours = Math.floor(minutes / 60); + const rest = minutes % 60; + return rest === 0 ? `${hours} h` : `${hours} h ${rest} min`; +} + +/** + * The first line of a result stopped at the model's own `timeoutMs`: + * what stopped it and what to pass for a longer run. + */ +export function formatShellTimeoutNotice(timeout: ResolvedShellTimeout): string { + return `stopped after ${formatShellDuration(timeout.timeoutMs)} (timeoutMs) — output so far below; pass a larger timeoutMs for a longer run, or 0 for no limit`; +} + +export interface ShellDetachNoticeInput { + jobId: number; + /** How long this call waited before giving the job back. */ + waitedMs: number; + /** A `wait` call that elapsed again, rather than the first detach. */ + again: boolean; + /** The per-call wait a bare `{"wait": id}` gets; `0` = unbounded. */ + defaultTimeoutMs: number; +} + +/** + * The first line of a detached result: the job's id and the three ways + * to act on it. Said above the output so the model reads it before the + * tail of a build log. + */ +export function formatShellDetachNotice(input: ShellDetachNoticeInput): string { + const { jobId } = input; + const after = formatShellDuration(input.waitedMs); + const perCall = + input.defaultTimeoutMs > 0 + ? ` (up to another ${formatShellDuration(input.defaultTimeoutMs)} per call)` + : ""; + const forms = `os.shell.run {"wait": ${jobId}} keeps waiting${perCall}, {"kill": ${jobId}} stops it`; + if (!input.again) { + return `still running after ${after} (job ${jobId}) — output so far below; ${forms}, pass timeoutMs for a longer first wait`; + } + return `still running after another ${after} (job ${jobId}) — output so far below; ${forms}, pass timeoutMs with the wait for a longer one`; +} + +/** The timeout sentence of the tool description, for the configured default. */ +export function describeShellTimeoutDefault(defaultTimeoutMs: number): string { + if (Number.isFinite(defaultTimeoutMs) && defaultTimeoutMs > 0) { + const limit = formatShellDuration(defaultTimeoutMs); + return `Timeout: default ${limit} — a command still running then is not killed but detached as a job, and the result names it: {"wait": id} waits for it (up to another ${limit} per call; timeoutMs sets the wait), {"kill": id} stops it, {"jobs": true} lists this session's jobs; a job dies when the turn ends unless the call carried keep: true. Pass \`timeoutMs\` to stop the command at an explicit limit instead, \`0\` for none.`; + } + return "By default there is no timeout (the command runs until it exits or the turn is cancelled); pass `timeoutMs` to set an explicit limit."; +} diff --git a/src/tools/os/shell.test.ts b/src/tools/os/shell.test.ts index eca4d6ed..e908669d 100644 --- a/src/tools/os/shell.test.ts +++ b/src/tools/os/shell.test.ts @@ -1,11 +1,124 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; -import { mkdtemp, rm } from "node:fs/promises"; +import { mkdtemp, readFile, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { ApprovalGate } from "../../approval/approval-gate.js"; import type { ToolContext } from "../tool-registry.js"; import { buildOsShellTool, isOpaqueInterpreterShape } from "./shell.js"; +function approvingTool(defaultTimeoutMs?: number) { + const gate = new ApprovalGate({ + emit: (req) => gate.resolve({ approvalId: req.approvalId, approved: true }), + }); + return buildOsShellTool({ + approvals: gate, + approvalRequired: true, + ...(defaultTimeoutMs === undefined ? {} : { defaultTimeoutMs }), + }); +} + +async function waitForExit(pid: number): Promise { + for (let i = 0; i < 80; i += 1) { + try { + process.kill(pid, 0); + } catch { + return; + } + await new Promise((resolve) => setTimeout(resolve, 25)); + } + throw new Error(`process ${pid} survived the timeout kill`); +} + +describe("os.shell.run describes its timeout (F47)", () => { + it("states the configured default and how to override it", () => { + expect(approvingTool(600_000).description).toContain( + "default 10 min — a command still running then is not killed but detached as a job", + ); + }); + + it("says there is no timeout when none is configured", () => { + expect(approvingTool(0).description).toContain("no timeout"); + expect(approvingTool().description).toContain("no timeout"); + }); +}); + +// The default timeout detaches rather than kills; that path is pinned in +// shell-detach.test.ts. Here: an explicit `timeoutMs` still kills. +describe.skipIf(process.platform === "win32")( + "os.shell.run explicit timeoutMs (F47)", + () => { + let dir: string; + + beforeEach(async () => { + dir = await mkdtemp(join(tmpdir(), "atomic-shell-timeout-")); + }); + + afterEach(async () => { + await rm(dir, { recursive: true, force: true }); + }); + + function makeCtx(): ToolContext { + return { + workingDir: dir, + sessionId: "test-session", + stepIndex: 0, + signal: new AbortController().signal, + }; + } + + it("an explicit timeoutMs of 0 disables the default", async () => { + const result = await approvingTool(300).run( + { cmd: "sleep", args: ["1"], timeoutMs: 0 }, + makeCtx(), + ); + expect(result.status).toBe("ok"); + expect(result.details.timedOut).toBe(false); + expect(result.details.source).toBeUndefined(); + }); + + it("an explicit timeoutMs wins over the default, and the text says so", async () => { + const result = await approvingTool(30_000).run( + { cmd: "sleep", args: ["30"], timeoutMs: 300 }, + makeCtx(), + ); + expect(result.status).toBe("error"); + expect(result.details.timedOut).toBe(true); + expect(result.details.timeoutMs).toBe(300); + expect(result.details.source).toBe("explicit"); + expect(result.summary).toContain("stopped after 0.3 s (timeoutMs)"); + expect(result.summary).not.toContain("default timeout"); + }); + + it("kills the whole process group, not just the shell", async () => { + const pidFile = join(dir, "background.pid"); + // The `&` routes this through `sh -c`. The background sleep is the + // shell's child: a kill aimed at the shell alone leaves it running + // for 30 s, holding stdout open — and the result with it. + // 1.5 s: long enough for the shell to have run `echo` on a loaded + // host; the kill itself is what is measured, not the fuse. + const result = await approvingTool(600_000).run( + { cmd: `sleep 30 & echo $! > "${pidFile}"; sleep 30`, timeoutMs: 1_500 }, + makeCtx(), + ); + expect(result.details.timedOut).toBe(true); + expect(result.details.source).toBe("explicit"); + expect(result.details.durationMs).toBeLessThan(5_000); + const pid = Number((await readFile(pidFile, "utf8")).trim()); + expect(pid).toBeGreaterThan(0); + await waitForExit(pid); + }); + + it("keeps the output captured before the stop", async () => { + const result = await approvingTool(600_000).run( + { cmd: "echo partial; sleep 30", timeoutMs: 1_500 }, + makeCtx(), + ); + expect(result.details.timedOut).toBe(true); + expect(result.summary).toContain("partial"); + }); + }, +); + describe("isOpaqueInterpreterShape (shape-grant suppression)", () => { it("withholds [a] for shell interpreters whose danger lives in their args", () => { // `bash -c ""` and friends: the binary name hides what diff --git a/src/tools/os/shell.ts b/src/tools/os/shell.ts index 9881d40f..a53e7f32 100644 --- a/src/tools/os/shell.ts +++ b/src/tools/os/shell.ts @@ -1,6 +1,9 @@ import { compressToolResult } from "../../compressor/result-compressor.js"; import type { ToolDefinition } from "../tool-registry.js"; -import { runCommand } from "../../sandbox/command-runner.js"; +import { + awaitJobExit, + startCommandJob, +} from "../../sandbox/command-job.js"; import { buildSubshellInvocation, quoteCmdArg, @@ -18,171 +21,35 @@ import { isGogCommand, type ShellGuardPolicy, } from "./shell-command-guard/index.js"; +import { + coerceShellArgs, + describeArgsShape, + isOpaqueInterpreterShape, + needsShellInterpretation, +} from "./shell-interpretation.js"; +import { + classifyShellCall, + listShellJobs, + renderShellDetached, + runShellKill, + runShellWait, +} from "./shell-job-calls.js"; +import { ShellJobRegistry } from "./shell-jobs.js"; +import { + GOG_MAX_OUTPUT_BYTES, + renderShellExit, + renderShellTimedOut, + type ShellCommandFacts, +} from "./shell-result.js"; +import { + describeShellTimeoutDefault, + resolveShellTimeout, +} from "./shell-timeout.js"; -const GOG_MAX_OUTPUT_BYTES = 4 * 1024 * 1024; -const GOG_COMPRESS_OPTIONS = { - maxSummaryLength: 64_000, - maxTailLines: 10_000, -} as const; - -/** - * Coerce the model-supplied `args` field into a string array. Returns - * the parsed list when the input is well-formed, or `null` when the - * input has the wrong shape so the caller can return a structured - * error to the model. Accepts: - * - `undefined` / missing -> [] (no extra args) - * - `string[]` -> coerced via String() - * - JSON-stringified array literal (some cloud providers - * double-serialise tool_call arguments) -> parsed + coerced - * Anything else (object, scalar string with no JSON shape, number, - * etc.) returns `null` and triggers the structured error path. - */ -function coerceShellArgs(value: unknown): string[] | null { - if (value === undefined || value === null) return []; - if (Array.isArray(value)) { - return value.map((v) => String(v)); - } - if (typeof value === "string") { - const trimmed = value.trim(); - if (trimmed.length === 0) return []; - if (trimmed.startsWith("[") && trimmed.endsWith("]")) { - try { - const parsed = JSON.parse(trimmed) as unknown; - if (Array.isArray(parsed)) { - return parsed.map((v) => String(v)); - } - } catch { - // fall through to error - } - } - } - return null; -} - -function describeArgsShape(value: unknown): string { - if (value === undefined) return "undefined"; - if (value === null) return "null"; - if (Array.isArray(value)) return "array"; - return typeof value; -} - -/** - * Shell metacharacters that only mean something inside a subshell (pipes, - * sequencing, redirects, command/parameter substitution, grouping). Note - * `*`/`?` are deliberately excluded — argv globs are expanded by - * `expandShellGlobArgs` on the direct-exec path, so a bare `{cmd:"ls", - * args:["*.png"]}` keeps working without spawning a subshell. - */ -const SHELL_METACHAR_RE = /[|&;<>$`(){}]/; - -/** - * `cmd.exe` internal commands that have no standalone executable on PATH. - * A direct `spawn("echo", …)` fails with ENOENT on Windows because these - * only exist inside the command interpreter — they must be routed through - * the `cmd.exe` subshell. Real executables (`where.exe`, `find.exe`, - * `sort.exe`, `more.com`) are intentionally excluded so they keep their - * direct-exec argv semantics. - */ -const WINDOWS_CMD_BUILTINS: ReadonlySet = new Set([ - "assoc", - "call", - "cd", - "chdir", - "cls", - "color", - "copy", - "date", - "del", - "dir", - "echo", - "erase", - "ftype", - "md", - "mkdir", - "mklink", - "move", - "path", - "pause", - "popd", - "prompt", - "pushd", - "rd", - "rem", - "ren", - "rename", - "rmdir", - "set", - "start", - "time", - "title", - "type", - "ver", - "verify", - "vol", -]); - -function isWindowsCmdBuiltin(cmd: string): boolean { - // Builtins are never invoked by path, so a direct lowercase lookup is - // sufficient — no basename stripping needed. - return WINDOWS_CMD_BUILTINS.has(cmd.trim().toLowerCase()); -} - -/** - * Decide whether `cmd` must be run through the OS subshell (`sh -c` / - * `cmd.exe /c`) instead of a direct `spawn(cmd, args)`. Models routinely - * emit a full shell command line in the `cmd` field (e.g. - * `"ffprobe -v quiet ... f.mp3"` or `"pip3 list | grep foo"`). With a - * direct exec that string is treated as a literal executable name and - * fails with ENOENT. We route to a subshell when `cmd` carries shell - * metacharacters, when it looks like a pre-joined command line (whitespace - * present and no separate `args`), or — on Windows — when `cmd` is a - * `cmd.exe` builtin (`echo`, `dir`, `type`, …) that has no standalone - * executable to spawn directly. - */ -export function needsShellInterpretation( - cmd: string, - args: readonly string[], -): boolean { - if (SHELL_METACHAR_RE.test(cmd)) return true; - // On Windows the model may emit `%VAR%` expansion, which only means - // something inside a `cmd.exe` subshell. `$` (POSIX) is already covered - // by SHELL_METACHAR_RE above. - if (process.platform === "win32" && /%[^%\s]+%/.test(cmd)) return true; - // A bare cmd.exe builtin must go through the interpreter or `spawn` - // ENOENTs. `cmd` here is a single token (metachar/pre-joined cases are - // handled above), so a straight builtin lookup is safe. - if ( - process.platform === "win32" && - !/\s/.test(cmd.trim()) && - isWindowsCmdBuiltin(cmd) - ) { - return true; - } - if (args.length === 0 && /\s/.test(cmd.trim())) return true; - return false; -} - -/** - * Interpreter / wrapper binaries whose danger lives in their arguments, - * not their name (`bash -c ""`). The shell tool withholds the - * shape grant for these: a grant keyed on `bash` would silence - * arbitrary code for the rest of the session. Matches the shells - * covered by the guard's `dangerous.shell_dash_c` rule. The category - * grant (the whole shell category) and a plain approve (this call only) - * stay available. - */ -const OPAQUE_INTERPRETER_SHAPES: ReadonlySet = new Set([ - "bash", - "sh", - "zsh", - "dash", - "ksh", -]); - -/** True when `[a]` (shape grant) must be withheld for `shape`. */ -export function isOpaqueInterpreterShape(shape: string): boolean { - return OPAQUE_INTERPRETER_SHAPES.has(shape); -} +export { + isOpaqueInterpreterShape, + needsShellInterpretation, +} from "./shell-interpretation.js"; export interface OsShellToolOptions extends DangerousToolOptions { /** @@ -191,15 +58,54 @@ export interface OsShellToolOptions extends DangerousToolOptions { * and tests, which then get the static rule set. */ shellPolicy?: ShellGuardPolicy; + /** + * `tools.shell.defaultTimeoutMs`: after this long a call whose + * `timeoutMs` the model omitted is detached as a job; `0` = never. + * Omitted by embedders and tests, which then get the unbounded + * pre-v67 behaviour. + */ + defaultTimeoutMs?: number; + /** + * Where detached jobs live, shared with the bootstrap's turn-end and + * session-end hooks. Omitted (embedders, tests) ⇒ a private registry + * whose jobs die only at the ceiling. + */ + jobs?: ShellJobRegistry; } export function buildOsShellTool(options: OsShellToolOptions): ToolDefinition { + const defaultTimeoutMs = options.defaultTimeoutMs ?? 0; + const jobs = options.jobs ?? new ShellJobRegistry(); return { name: "os.shell.run", description: - "Run an OS command in the session working directory. Prefer the structured form `{cmd, args:[...]}` (argv globs `*`/`?` are expanded). Shell metacharacters (`|`, `&&`, `;`, `>`, `<`, `$`, backticks) are interpreted via the OS subshell (`sh -c` on macOS/Linux, `cmd.exe /c` on Windows) — a full command line passed as `cmd` (e.g. `\"ffprobe -v quiet … f.mp3\"` or `\"pip3 list | grep foo\"`) runs as written. Do not use for deleting user files — use `os.fs.trash` unless the user explicitly requests permanent shell deletion. Runs through a pre-exec guard: safe commands run directly, risky commands require approval, catastrophic commands are blocked without execution. By default there is no timeout (the command runs until it exits or the turn is cancelled); pass `timeoutMs` to set an explicit limit.", + "Run an OS command in the session working directory. Prefer the structured form `{cmd, args:[...]}` (argv globs `*`/`?` are expanded). Shell metacharacters (`|`, `&&`, `;`, `>`, `<`, `$`, backticks) are interpreted via the OS subshell (`sh -c` on macOS/Linux, `cmd.exe /c` on Windows) — a full command line passed as `cmd` (e.g. `\"ffprobe -v quiet … f.mp3\"` or `\"pip3 list | grep foo\"`) runs as written. Do not use for deleting user files — use `os.fs.trash` unless the user explicitly requests permanent shell deletion. Runs through a pre-exec guard: safe commands run directly, risky commands require approval, catastrophic commands are blocked without execution. " + + describeShellTimeoutDefault(defaultTimeoutMs), readonly: false, async run(rawArgs, ctx) { + // The job forms act on what this session already started; they + // need no guard and no approval of their own. + const form = classifyShellCall(rawArgs); + const jobCtx = { + jobs, + sessionId: ctx.sessionId, + defaultTimeoutMs, + signal: ctx.signal, + }; + if (form.kind === "invalid") { + return compressToolResult({ + tool: "os.shell.run", + status: "error", + output: form.message, + details: { invalidCall: true }, + }); + } + if (form.kind === "jobs") return listShellJobs(jobCtx); + if (form.kind === "wait") { + return runShellWait(jobCtx, form.id, rawArgs.timeoutMs, rawArgs.keep === true); + } + if (form.kind === "kill") return runShellKill(jobCtx, form.id); + const cmd = rawArgs.cmd; if (typeof cmd !== "string" || cmd.length === 0) { throw new Error("os.shell.run: `cmd` must be a non-empty string"); @@ -228,15 +134,13 @@ export function buildOsShellTool(options: OsShellToolOptions): ToolDefinition { typeof rawArgs.cwd === "string" && rawArgs.cwd.length > 0 ? resolveUserPath(rawArgs.cwd, ctx.workingDir) : ctx.workingDir; - // No default timeout: when the model does not pass `timeoutMs` - // explicitly the command runs unbounded (long installs like - // `brew install` need this). `0` signals "no timeout" to the - // command runner; the turn's abort signal stays the safety valve. - const timeoutMs = - typeof rawArgs.timeoutMs === "number" && - Number.isFinite(rawArgs.timeoutMs) - ? rawArgs.timeoutMs - : 0; + // An explicit `timeoutMs` wins (`0` = none — long installs like + // `brew install` need it) and kills at its limit: the model asked + // for a bound. The operator's default (F47: a scan of a home + // directory used to run until someone killed it) detaches + // instead — a build the default interrupted is not one the model + // wanted stopped. The turn's abort signal stays the safety valve. + const timeout = resolveShellTimeout(rawArgs.timeoutMs, defaultTimeoutMs); // Two execution modes. Direct-exec (`spawn(cmd, args)`) keeps argv // semantics and shell-glob expansion. Subshell (`sh -c `) is @@ -322,63 +226,62 @@ export function buildOsShellTool(options: OsShellToolOptions): ToolDefinition { execArgs.length > 0 && process.platform === "win32" ? [cmd, ...execArgs.map(quoteCmdArg)].join(" ") : commandLine; - const subshell = buildSubshellInvocation(subshellCommandLine); - const result = useShell - ? await runCommand(subshell.command, subshell.args, { - cwd, - timeoutMs, - signal: ctx.signal, - ...(isGogCommand(gogProbe) - ? { maxOutputBytes: GOG_MAX_OUTPUT_BYTES } - : {}), - }) - : await runCommand(cmd, execArgs, { - cwd, - timeoutMs, - signal: ctx.signal, - ...(isGogCommand(cmd) - ? { maxOutputBytes: GOG_MAX_OUTPUT_BYTES } - : {}), - }); - const status = result.exitCode === 0 ? "ok" : "error"; - // A bare interpreter — `python3` with nothing after it — exits 0 - // having done nothing, and nothing in its output says so (F40: - // the model had put the script under a key the tool does not - // know). Said on the command line, where the model looks first. - // The subshell path is excluded: there the arguments live inside - // `cmd` itself. - const noArguments = !useShell && execArgs.length === 0; - const header = `$ ${commandLine}${noArguments ? " (ran with no arguments)" : ""}\nexit: ${result.exitCode ?? "signal:" + result.signal}${result.timedOut ? " (timed out)" : ""}`; - const body = [result.stdout, result.stderr] - .filter((s) => s.trim().length > 0) - .join("\n---\n"); - // `node --check a b c` exits 0 having read only `a`. Said here, - // first, because nothing in node's own output says it — and a - // reply built on that exit code claims a check that never ran. + const spawnSpec = useShell + ? buildSubshellInvocation(subshellCommandLine) + : { command: cmd, args: execArgs }; + const facts: ShellCommandFacts = { + cmd, + args: execArgs, + rawArgs: rawArgList, + cwd, + shell: useShell, + commandLine, + // A bare interpreter — `python3` with nothing after it — exits 0 + // having done nothing, and nothing in its output says so (F40). + // The subshell path is excluded: there the arguments live inside + // `cmd` itself. + noArguments: !useShell && execArgs.length === 0, + gog: isGogCommand(gogProbe), + guard: guardVerdict, + }; + // Its own process group, so a stop reaches what the command + // started too (`sleep 30 &` behind a subshell used to outlive the + // shell and hold the result until it ended). + const job = startCommandJob(spawnSpec.command, spawnSpec.args, { + cwd, + ...(facts.gog ? { maxOutputBytes: GOG_MAX_OUTPUT_BYTES } : {}), + }); + // A spawn failure (ENOENT) rejects here, as the runner's always did. + const outcome = await job.waitFor(timeout.timeoutMs, ctx.signal); + // `node --check a b c` exits 0 having read only `a`. Said first, + // because nothing in node's own output says it — and a reply built + // on that exit code claims a check that never ran. const checkNotice = nodeCheckMultiFileNotice(commandLine, cwd); - return compressToolResult( - { - tool: "os.shell.run", - status, - output: `${checkNotice === null ? "" : `${checkNotice}\n`}${header}\n${body}`, - details: { - cmd, - args: execArgs, - rawArgs: rawArgList, - cwd, - shell: useShell, - exitCode: result.exitCode, - signal: result.signal, - durationMs: result.durationMs, - timedOut: result.timedOut, - truncated: result.truncated, - guardVerdict: guardVerdict.action, - guardRule: guardVerdict.rule, - guardReason: guardVerdict.reason, - }, - }, - isGogCommand(gogProbe) ? GOG_COMPRESS_OPTIONS : {}, - ); + const notices = checkNotice === null ? [] : [checkNotice]; + if (outcome === "elapsed" && timeout.source === "default") { + const { record, evicted } = jobs.register( + ctx.sessionId, + job, + facts, + rawArgs.keep === true, + ); + return renderShellDetached(record, { + waitedMs: timeout.timeoutMs, + again: false, + defaultTimeoutMs, + evicted, + maxJobs: jobs.maxJobs, + notices, + }); + } + if (outcome === "elapsed") job.stop(); + else if (outcome === "aborted") job.kill(); + const exit = await awaitJobExit(job); + const output = job.output(); + if (outcome === "elapsed") { + return renderShellTimedOut(facts, exit, output, timeout, notices); + } + return renderShellExit(facts, exit, output, { notices }); }, }; } diff --git a/src/tools/read-scope/confine-reads.ts b/src/tools/read-scope/confine-reads.ts new file mode 100644 index 00000000..63f45d01 --- /dev/null +++ b/src/tools/read-scope/confine-reads.ts @@ -0,0 +1,152 @@ +import type { DangerousToolOptions } from "../../approval/dangerous-tool.js"; +import type { ReadScope } from "../../config/index.js"; +import { isFusionWorkerSessionId } from "../../session/fusion-worker-session.js"; +import type { ToolDefinition, ToolRegistry } from "../tool-registry.js"; +import { + checkWorkerRead, + findSessionReadOutside, + workerReadRefusal, +} from "./read-scope.js"; +import { + ReadOutsideApprover, + readOutsidePrompt, + shellReadOutsidePrompt, +} from "./read-scope-approval.js"; +import { + findShellPathOutsideScope, + shellCommandLine, + type ShellScopeEnv, +} from "./read-scope-shell.js"; +import { READ_TOOL_TARGETS, SHELL_TOOL } from "./read-scope-targets.js"; + +export interface ConfineReadsOptions { + /** The directories a fusion worker's fan-out may write in. */ + grantedDirs: (sessionId: string) => readonly string[]; + /** + * The live `agent.readScope`. Read on every call, so a config change + * takes effect without a restart. Omitted: only fusion workers are + * confined and the shell is not wrapped — the pre-v67 install + * (`confineWorkerReads`). + */ + readScope?: () => ReadScope; + /** + * The ladder a read outside the scope asks through (`fs_read_outside`, + * `read-scope-approval.ts`). Required with `readScope`: the session + * scope is a question, and a question needs a gate — an install that + * forgot the gate would silently be a refusal, which is exactly the + * half-wiring the throw below exists to catch. + */ + approvals?: DangerousToolOptions; + /** The shell check's notion of home and platform; a test seam. */ + shellEnv?: ShellScopeEnv; +} + +/** Definitions this module produced, so a second install does not wrap twice. */ +const CONFINED = new WeakSet(); + +/** + * Wrap every registered read-class tool (and, under a session scope, the + * shell) so a call is checked before it runs. Call once, after the + * native tools are registered. Returns the names it confined. + * + * Wrapping at the registry rather than inside each tool keeps the rule + * in one place, and `registry.invoke` is the single path every call — + * native, batched, or recovered from text — takes. A worker session + * gets the worker check first (always on, a refusal); then, when the + * scope is `working-dir`, every other session's call outside the roots + * is asked about, and runs or is refused by the answer. + */ +export function confineReads( + registry: Pick, + options: ConfineReadsOptions, +): string[] { + if (options.readScope !== undefined && options.approvals === undefined) { + throw new Error( + "confineReads: a session read scope asks through the approval ladder; pass `approvals` with `readScope`", + ); + } + const asker = + options.approvals === undefined + ? null + : new ReadOutsideApprover(options.approvals); + const confined: string[] = []; + const scoped = () => options.readScope?.() === "working-dir"; + const install = (name: string, run: ToolDefinition["run"]): void => { + const inner = registry.get(name); + confined.push(name); + if (CONFINED.has(inner)) return; + const wrapped: ToolDefinition = { ...inner, run }; + CONFINED.add(wrapped); + registry.register(wrapped); + }; + + for (const name of READ_TOOL_TARGETS.keys()) { + if (!registry.has(name)) continue; + const inner = registry.get(name); + install(name, async (args, ctx) => { + const worker = checkWorkerRead( + name, + args, + ctx, + options.grantedDirs(ctx.sessionId), + ); + if (worker !== null) return worker; + if (asker === null || !scoped()) return inner.run(args, ctx); + const refusal = await asker.admit( + name, + ctx, + (roots) => findSessionReadOutside(name, args, ctx, roots), + (path, root) => readOutsidePrompt(path, ctx.workingDir, root), + ); + return refusal ?? inner.run(args, ctx); + }); + } + + if (asker !== null && registry.has(SHELL_TOOL)) { + const inner = registry.get(SHELL_TOOL); + install(SHELL_TOOL, async (args, ctx) => { + if (!scoped()) return inner.run(args, ctx); + if (isFusionWorkerSessionId(ctx.sessionId)) { + // A worker's world is its task's directories, and there is + // nobody to ask: the refusal, as for its reads. + const granted = options.grantedDirs(ctx.sessionId); + const outside = findShellPathOutsideScope( + args, + ctx, + [ctx.workingDir, ...granted], + options.shellEnv, + ); + return outside === null + ? inner.run(args, ctx) + : workerReadRefusal(SHELL_TOOL, outside, ctx, granted); + } + const refusal = await asker.admit( + SHELL_TOOL, + ctx, + (roots) => findShellPathOutsideScope(args, ctx, roots, options.shellEnv), + (path, root) => + shellReadOutsidePrompt( + shellCommandLine(args), + path, + ctx.workingDir, + root, + ), + ); + return refusal ?? inner.run(args, ctx); + }); + } + return confined; +} + +/** + * The fusion-only install: workers confined to their working directory + * and fan-out scope, every other session untouched, the shell not + * wrapped. Kept for callers that want exactly that; the runtime installs + * `confineReads` with the live `agent.readScope` and the ladder instead. + */ +export function confineWorkerReads( + registry: Pick, + options: { grantedDirs: (sessionId: string) => readonly string[] }, +): string[] { + return confineReads(registry, { grantedDirs: options.grantedDirs }); +} diff --git a/src/tools/read-scope/index.ts b/src/tools/read-scope/index.ts new file mode 100644 index 00000000..0f9acc32 --- /dev/null +++ b/src/tools/read-scope/index.ts @@ -0,0 +1,44 @@ +export { + READ_TOOL_TARGETS, + SHELL_TOOL, + URL_LIKE, +} from "./read-scope-targets.js"; +export type { ReadTargetsOf } from "./read-scope-targets.js"; +export { + canonical, + checkWorkerRead, + findSessionReadOutside, + isOutsideReadRoots, + isScratchPath, + isUnderAny, + READ_REFUSAL_REASON, + scratchDirs, + sessionReadRefusal, + sessionReadRoots, + WORKER_READ_REFUSAL_REASON, + workerReadRefusal, +} from "./read-scope.js"; +export { + looksLikeNamedPath, + pathsNamedIn, + userNamedPaths, +} from "./read-scope-roots.js"; +export type { UserNamedPathOptions } from "./read-scope-roots.js"; +export { + defaultShellScopeEnv, + findShellPathOutsideScope, + shellCommandLine, + shellTokens, +} from "./read-scope-shell.js"; +export type { ShellScopeEnv } from "./read-scope-shell.js"; +export { + ReadOutsideApprover, + readOutsidePrompt, + shellReadOutsidePrompt, + widenedReadRoot, +} from "./read-scope-approval.js"; +export type { ReadOutsidePrompt } from "./read-scope-approval.js"; +export { confineReads, confineWorkerReads } from "./confine-reads.js"; +export type { ConfineReadsOptions } from "./confine-reads.js"; +// The worker-era name of the targets map, kept so fusion code reads the same. +export { READ_TOOL_TARGETS as WORKER_READ_TOOL_TARGETS } from "./read-scope-targets.js"; diff --git a/src/tools/read-scope/read-scope-approval.test.ts b/src/tools/read-scope/read-scope-approval.test.ts new file mode 100644 index 00000000..ca47d326 --- /dev/null +++ b/src/tools/read-scope/read-scope-approval.test.ts @@ -0,0 +1,236 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; + +import { + ApprovalGate, + type ApprovalLevel, + type ApprovalRequest, +} from "../../approval/index.js"; +import { compressToolResult } from "../../compressor/result-compressor.js"; +import type { ReadScope } from "../../config/index.js"; +import { FUSION_WORKER_ID_PREFIX } from "../../session/fusion-worker-session.js"; +import { ToolRegistry, type ToolContext } from "../tool-registry.js"; +import { confineReads } from "./confine-reads.js"; +import { widenedReadRoot } from "./read-scope-approval.js"; +import { READ_REFUSAL_REASON, WORKER_READ_REFUSAL_REASON } from "./read-scope.js"; + +const WORKER = `${FUSION_WORKER_ID_PREFIX}1`; + +describe("a read outside the scope asks through the ladder", () => { + let work: string; + let registry: ToolRegistry; + let gate: ApprovalGate; + let prompts: ApprovalRequest[]; + let answer: { approved: boolean; grant?: "category" }; + let innerReads: number; + let innerShells: number; + let readScope: ReadScope; + // Lexical and off the temp directory, which is scratch and never asked about. + const home = "/srv/homes/me"; + const elsewhere = "/srv/homes/other-run/work"; + const solution = join(elsewhere, "solution.js"); + + const ctx = (sessionId = "s-plain", readRoots?: readonly string[]): ToolContext => ({ + sessionId, + workingDir: work, + stepIndex: 0, + signal: new AbortController().signal, + ...(readRoots ? { readRoots } : {}), + }); + const read = (path: string, c = ctx()) => registry.invoke("os.fs.read", { path }, c); + const shell = (args: Record, c = ctx()) => + registry.invoke("os.shell.run", args, c); + const stub = (name: string, count: () => void) => ({ + name, + description: name, + readonly: true, + run: async () => { + count(); + return compressToolResult({ tool: name, status: "ok", output: "", details: {} }); + }, + }); + + beforeEach(() => { + work = mkdtempSync(join(tmpdir(), "read-ask-")); + mkdirSync(join(work, "src"), { recursive: true }); + writeFileSync(join(work, "src", "main.ts"), "mine"); + prompts = []; + answer = { approved: true }; + innerReads = 0; + innerShells = 0; + readScope = "working-dir"; + gate = new ApprovalGate({ + level: 3 as ApprovalLevel, + emit: (request) => { + prompts.push(request); + // Answer on the next tick, the way a surface would. + queueMicrotask(() => + gate.resolve({ approvalId: request.approvalId, ...answer }), + ); + }, + }); + registry = new ToolRegistry(); + registry.register(stub("os.fs.read", () => (innerReads += 1))); + registry.register(stub("os.shell.run", () => (innerShells += 1))); + expect( + confineReads(registry, { + grantedDirs: () => [], + readScope: () => readScope, + approvals: { approvals: gate, approvalRequired: true }, + shellEnv: { home, platform: "linux" }, + }), + ).toEqual(["os.fs.read", "os.shell.run"]); + }); + + afterEach(() => { + rmSync(work, { recursive: true, force: true }); + }); + + it("asks at an asking level, naming the path and the directory a yes widens to", async () => { + expect((await read(solution)).status).toBe("ok"); + expect(prompts).toHaveLength(1); + const prompt = prompts[0]!; + expect(prompt.tool).toBe("os.fs.read"); + expect(prompt.category).toBe("fs_read_outside"); + expect(prompt.sessionId).toBe("s-plain"); + expect(prompt.reason).toBe( + `read ${solution} — outside the working directory (${work}); approving allows reads under ${elsewhere} for the rest of this session`, + ); + expect(prompt.affectedResources).toEqual([elsewhere]); + expect(prompt.commandShape).toBeUndefined(); + expect(prompt.redirectablePath).toBeUndefined(); + expect(innerReads).toBe(1); + }); + + it("a yes widens the session's roots: the next read under that directory does not ask", async () => { + await read(solution); + await read(join(elsewhere, "lib", "other.js")); + expect(prompts).toHaveLength(1); + expect(innerReads).toBe(2); + expect(gate.readScopeGrants.rootsFor("s-plain")).toEqual([elsewhere]); + // A sibling directory is a new question. + await read(join(dirname(elsewhere), "sibling", "x.js")); + expect(prompts).toHaveLength(2); + // Another session rides nothing of it. + await read(solution, ctx("s-other")); + expect(prompts).toHaveLength(3); + }); + + it("a no is the refusal, with the existing sentence, and widens nothing", async () => { + answer = { approved: false }; + const refused = await read(solution); + expect(refused.status).toBe("error"); + expect(refused.details.reason).toBe(READ_REFUSAL_REASON); + expect(refused.summary).toBe( + `os.fs.read refused: reads are confined to the working directory (${work}) and the paths the user named; ` + + `ask the user to name ${solution} or to set agent.readScope: unrestricted`, + ); + expect(innerReads).toBe(0); + expect(gate.readScopeGrants.rootsFor("s-plain")).toEqual([]); + // The model may try again; it is asked again. + answer = { approved: true }; + expect((await read(solution)).status).toBe("ok"); + expect(prompts).toHaveLength(2); + }); + + it("an [s] grant is 'read anywhere this session': later reads elsewhere do not ask", async () => { + answer = { approved: true, grant: "category" }; + await read(solution); + await read(join(dirname(elsewhere), "sibling", "x.js")); + expect(prompts).toHaveLength(1); + expect(innerReads).toBe(2); + }); + + it("level 5 runs without asking; a user-named path and the working directory never ask at any level", async () => { + gate.setLevel(5); + expect((await read(solution)).status).toBe("ok"); + gate.setLevel(1); + expect((await read("src/main.ts")).status).toBe("ok"); + expect((await read(solution, ctx("s-plain", [elsewhere]))).status).toBe("ok"); + expect(prompts).toHaveLength(0); + expect(innerReads).toBe(3); + }); + + it("`unrestricted` never asks, live, without a reinstall", async () => { + readScope = "unrestricted"; + gate.setLevel(1); + expect((await read(solution)).status).toBe("ok"); + expect((await shell({ cmd: `cat ${solution}` })).status).toBe("ok"); + expect(prompts).toHaveLength(0); + }); + + it("a fusion worker is still refused outright, reads and shell alike", async () => { + const worker = await read(solution, ctx(WORKER, [elsewhere])); + expect(worker.status).toBe("error"); + expect(worker.details.reason).toBe(WORKER_READ_REFUSAL_REASON); + const cmd = await shell({ cmd: `cat ${solution}` }, ctx(WORKER, [elsewhere])); + expect(cmd.status).toBe("error"); + expect(cmd.details.reason).toBe(WORKER_READ_REFUSAL_REASON); + expect(cmd.summary).toContain("outside this worker's working directory"); + expect(prompts).toHaveLength(0); + }); + + it("a shell path outside the scope asks under the same category, with the command", async () => { + // The fixture is lexical (nothing under /srv exists here), so the + // widened root is the named file's parent — as for a read. + expect( + (await shell({ cmd: "grep", args: ["-n", "x", solution] })).status, + ).toBe("ok"); + expect(prompts).toHaveLength(1); + const prompt = prompts[0]!; + expect(prompt.tool).toBe("os.shell.run"); + expect(prompt.category).toBe("fs_read_outside"); + expect(prompt.reason).toBe( + `run \`grep -n x ${solution}\` — reads outside the working directory (${work}): ${solution}; approving allows reads under ${elsewhere} for the rest of this session`, + ); + expect(prompt.preview).toBe(`grep -n x ${solution}`); + expect(prompt.commandShape).toBeUndefined(); + expect(innerShells).toBe(1); + // The yes covers a read tool under the same directory too. + expect((await read(solution)).status).toBe("ok"); + expect(prompts).toHaveLength(1); + // Denied: the same refusal the read gets. + answer = { approved: false }; + const refused = await shell({ cmd: `cat ${join(home, "notes.txt")}` }); + expect(refused.status).toBe("error"); + expect(refused.summary).toContain("reads are confined to the working directory"); + expect(innerShells).toBe(1); + }); + + it("parallel reads under one directory ask once: the second waits and re-checks", async () => { + const results = await Promise.all([ + read(solution), + read(join(elsewhere, "b.js")), + read(join(elsewhere, "c.js")), + ]); + expect(results.map((r) => r.status)).toEqual(["ok", "ok", "ok"]); + expect(prompts).toHaveLength(1); + expect(gate.pendingCount()).toBe(0); + }); + + it("an aborted turn does not park a read on an unanswered prompt", async () => { + const controller = new AbortController(); + controller.abort(); + await expect( + read(solution, { ...ctx(), signal: controller.signal }), + ).rejects.toThrow(/approval aborted/); + expect(innerReads).toBe(0); + }); + + it("refuses to install a session scope without a ladder to ask through", () => { + expect(() => + confineReads(new ToolRegistry(), { + grantedDirs: () => [], + readScope: () => "working-dir", + }), + ).toThrow(/approvals/); + }); + + it("widens to a directory itself, or to a file's parent", () => { + expect(widenedReadRoot(join(work, "src"))).toBe(join(work, "src")); + expect(widenedReadRoot(join(work, "src", "main.ts"))).toBe(join(work, "src")); + expect(widenedReadRoot(join(work, "src", "absent.ts"))).toBe(join(work, "src")); + }); +}); diff --git a/src/tools/read-scope/read-scope-approval.ts b/src/tools/read-scope/read-scope-approval.ts new file mode 100644 index 00000000..f333dcb2 --- /dev/null +++ b/src/tools/read-scope/read-scope-approval.ts @@ -0,0 +1,172 @@ +import { statSync } from "node:fs"; +import { dirname } from "node:path"; + +import { + ApprovalDeniedError, + requireApproval, + type DangerousToolOptions, +} from "../../approval/dangerous-tool.js"; +import type { CompressedToolResult } from "../../compressor/result-compressor.js"; +import type { ToolContext } from "../tool-registry.js"; +import { sessionReadRefusal, sessionReadRoots } from "./read-scope.js"; + +/** + * A read outside the scope is a QUESTION, not a refusal. + * + * The scope (`read-scope.ts`) says where a session may read unasked; + * this module is what happens at the edge of it. A read-class call or a + * shell command naming a path outside every root goes through the + * ladder as `fs_read_outside` — the same prompt every other gated + * action gets, on whichever surface owns the session — and the answer + * is remembered: a `y` widens the session's roots to the directory it + * named (`ReadScopeGrants` on the gate), so one question covers the + * reads that follow under it; `[s]` grants the whole category, which is + * "read anywhere this session". A `n` is the refusal the model would + * have got before: one line naming the path, the working directory and + * the way out. Level 5 / `--no-approval` never asks, like every other + * category pinned there. Fusion workers never reach this module — their + * check refuses first, because nobody is at the other end of a worker's + * prompt. + * + * Questions are asked one at a time per session. Read tools run in + * parallel inside a batch, and two prompts raised at once would leave + * one of them stranded behind the other on a surface that shows a + * single pending request. So the check-and-ask runs under a per-session + * queue: the second read waits for the first's answer and re-checks + * against the widened roots — usually to find it no longer needs to ask. + */ + +/** + * The directory an approval widens the session's reads to: the path + * itself when it is a directory, otherwise its parent. A file's parent + * rather than the file, because a model asked to summarise one report + * in a folder will reach for the next one, and the operator who said + * yes to the folder's first file has seen where the model is reading. + */ +export function widenedReadRoot(path: string): string { + try { + if (statSync(path).isDirectory()) return path; + } catch { + // Absent or unreadable: the parent is the honest unit either way. + } + return dirname(path); +} + +export interface ReadOutsidePrompt { + reason: string; + preview?: string; +} + +const consequence = (root: string): string => + `approving allows reads under ${root} for the rest of this session`; + +/** The prompt for a read-class call: what is read, from where, and what a `y` means. */ +export function readOutsidePrompt( + path: string, + workingDir: string, + root: string, +): ReadOutsidePrompt { + return { + reason: `read ${path} — outside the working directory (${workingDir}); ${consequence(root)}`, + }; +} + +/** The prompt for a shell command naming a path outside the scope. */ +export function shellReadOutsidePrompt( + commandLine: string, + path: string, + workingDir: string, + root: string, +): ReadOutsidePrompt { + return { + reason: `run \`${commandLine}\` — reads outside the working directory (${workingDir}): ${path}; ${consequence(root)}`, + preview: commandLine, + }; +} + +/** One task at a time per session, in arrival order; a failure does not block the next. */ +class SessionQueue { + private readonly tails = new Map>(); + + run(sessionId: string, task: () => Promise): Promise { + const previous = this.tails.get(sessionId) ?? Promise.resolve(); + const result = previous.then(task); + const tail = result.then( + () => undefined, + () => undefined, + ); + this.tails.set(sessionId, tail); + void tail.then(() => { + if (this.tails.get(sessionId) === tail) this.tails.delete(sessionId); + }); + return result; + } +} + +type ScopedContext = Pick< + ToolContext, + "sessionId" | "workingDir" | "readRoots" | "signal" +>; + +/** + * The asking half of the read scope, one per registry install. Holds + * the ladder wiring (`DangerousToolOptions`) and the per-session queue; + * the roots it checks against are the session's own plus what the gate + * remembers for it. + */ +export class ReadOutsideApprover { + private readonly queue = new SessionQueue(); + + constructor(private readonly options: DangerousToolOptions) {} + + /** Working directory, user-named paths, and the directories approved so far. */ + roots(ctx: Pick): string[] { + return [ + ...sessionReadRoots(ctx), + ...this.options.approvals.readScopeGrants.rootsFor(ctx.sessionId), + ]; + } + + /** + * Let `tool`'s call through, or not. `outsideOf(roots)` names the + * first path the call reaches outside `roots` (or `null`); when there + * is one, the operator is asked with `promptFor(path, root)`. Resolves + * `null` when the call may run (nothing outside, or approved — and + * then `root` is remembered), or the refusal when it was denied. + */ + admit( + tool: string, + ctx: ScopedContext, + outsideOf: (roots: readonly string[]) => string | null, + promptFor: (path: string, root: string) => ReadOutsidePrompt, + ): Promise { + return this.queue.run(ctx.sessionId, async () => { + const roots = this.roots(ctx); + const path = outsideOf(roots); + if (path === null) return null; + const root = widenedReadRoot(path); + const prompt = promptFor(path, root); + try { + await requireApproval( + this.options, + { + sessionId: ctx.sessionId, + tool, + category: "fs_read_outside", + reason: prompt.reason, + ...(prompt.preview !== undefined ? { preview: prompt.preview } : {}), + affectedResources: [root], + }, + ctx.signal, + ); + } catch (err) { + if (err instanceof ApprovalDeniedError) { + return sessionReadRefusal(tool, path, ctx, roots); + } + throw err; + } + this.options.approvals.readScopeGrants.widen(ctx.sessionId, root); + return null; + }); + } +} diff --git a/src/tools/read-scope/read-scope-roots.test.ts b/src/tools/read-scope/read-scope-roots.test.ts new file mode 100644 index 00000000..cef6a5c8 --- /dev/null +++ b/src/tools/read-scope/read-scope-roots.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, it } from "vitest"; + +import { + assistantReplyTurn, + assistantToolCallTurn, + toolResultTurn, + userTurn, +} from "../../session/conversation-turn.js"; +import { pathsNamedIn, userNamedPaths } from "./read-scope-roots.js"; + +const home = "/Users/someone"; +const posixOnly = process.platform === "win32"; + +describe.skipIf(posixOnly)("paths the user named", () => { + it("finds absolute and ~-prefixed paths, expanding ~ to the home directory", () => { + expect( + pathsNamedIn("summarize ~/Desktop/report.pdf and /srv/data/log.txt", { + home, + }), + ).toEqual(["/Users/someone/Desktop/report.pdf", "/srv/data/log.txt"]); + expect(pathsNamedIn("look in ~ for it", { home })).toEqual([home]); + }); + + it("keeps a quoted path whole, spaces included", () => { + expect( + pathsNamedIn(`open "~/My Documents/report v2.pdf" please`, { home }), + ).toEqual(["/Users/someone/My Documents/report v2.pdf"]); + expect(pathsNamedIn("read '/tmp/a b/c.txt' now", { home })).toEqual([ + "/tmp/a b/c.txt", + ]); + expect(pathsNamedIn("run `~/bin/tool`", { home })).toEqual([ + "/Users/someone/bin/tool", + ]); + }); + + it("scans a quoted span that is not itself a path word by word", () => { + expect(pathsNamedIn(`he said "check /tmp/x and /tmp/y"`, { home })).toEqual( + ["/tmp/x", "/tmp/y"], + ); + }); + + it("strips the punctuation a sentence glues on", () => { + expect( + pathsNamedIn( + "compare /tmp/a.txt, /tmp/b.txt; then (~/notes/todo.md) — done?", + { home }, + ), + ).toEqual(["/tmp/a.txt", "/tmp/b.txt", "/Users/someone/notes/todo.md"]); + expect(pathsNamedIn("it is in /tmp/proj/src/.", { home })).toEqual([ + "/tmp/proj/src", + ]); + }); + + it("ignores URLs, the bare root, relative paths and prose", () => { + expect( + pathsNamedIn( + "see https://example.com/a/b and src/tools/x.ts, or / alone, and/or this", + { home }, + ), + ).toEqual([]); + }); + + it("recognises a Windows drive path as written", () => { + expect(pathsNamedIn("open C:\\Users\\me\\file.txt", { home })).toEqual([ + "C:\\Users\\me\\file.txt", + ]); + }); + + it("reads only user turns, deduplicates, and grows with the conversation", () => { + const turns = [ + userTurn("summarize ~/Desktop/report.pdf"), + assistantToolCallTurn({ + tool: "os.fs.read", + args: { path: "/Users/other/secret.txt" }, + }), + toolResultTurn({ + tool: "os.fs.read", + status: "ok", + summary: "/Users/other/leaked.txt", + }), + assistantReplyTurn("done; also see /Users/other/more.txt"), + userTurn("and ~/Desktop/report.pdf again, plus /srv/data"), + ]; + expect(userNamedPaths(turns, { home })).toEqual([ + "/Users/someone/Desktop/report.pdf", + "/srv/data", + ]); + expect(userNamedPaths([], { home })).toEqual([]); + }); +}); diff --git a/src/tools/read-scope/read-scope-roots.ts b/src/tools/read-scope/read-scope-roots.ts new file mode 100644 index 00000000..0bfda810 --- /dev/null +++ b/src/tools/read-scope/read-scope-roots.ts @@ -0,0 +1,102 @@ +import { homedir } from "node:os"; +import { resolve } from "node:path"; + +import type { ConversationTurn } from "../../session/conversation-turn.js"; + +/** + * The paths the USER named, parsed out of their own messages. + * + * A session's reads are confined to its working directory — but + * "summarize ~/Desktop/report.pdf" has to keep working untouched, so + * every absolute or `~`-prefixed path the user typed widens the scope: + * a named file to that file, a named directory to that directory. Only + * user turns count. A path the model wrote (a reply, a tool argument, a + * worker's brief) never widens anything, or the scope would be the + * model's to grow. + * + * Recomputed from the transcript on every step rather than stored, so + * it grows as the conversation grows and there is no second copy to + * drift. Pure text work: nothing here touches the disk. + */ + +export interface UserNamedPathOptions { + /** The home directory `~` expands to. Defaults to the OS one. */ + home?: string; +} + +/** `"…"`, `'…'` or `` `…` ``: a quoted span is one path, spaces and all. */ +const QUOTED = /"([^"\n]+)"|'([^'\n]+)'|`([^`\n]+)`/g; +/** Brackets a path is wrapped in, and the punctuation a sentence glues on. */ +const LEADING = /^[(\[{<]+/; +const TRAILING = /[.,;:!?)\]}>]+$/; +const DRIVE = /^[A-Za-z]:[\\/]/; + +/** Whether a bare token is a path the user named, by its first characters. */ +export function looksLikeNamedPath(token: string): boolean { + if (token === "~" || token.startsWith("~/") || token.startsWith("~\\")) { + return true; + } + if (DRIVE.test(token)) return true; + // `/` alone is the whole disk and `//…` is the tail of a URL: neither + // is a path the user named. + return token.startsWith("/") && token.length > 1 && !token.startsWith("//"); +} + +function expandNamed(token: string, home: string): string { + if (token === "~") return home; + if (token.startsWith("~/") || token.startsWith("~\\")) { + return resolve(home, token.slice(2)); + } + // A drive path is left as written: `resolve` would rebase it onto the + // current directory on POSIX, where it cannot mean anything anyway. + return DRIVE.test(token) ? token : resolve(token); +} + +/** The absolute paths named in one message, in order, deduplicated. */ +export function pathsNamedIn( + text: string, + options: UserNamedPathOptions = {}, +): string[] { + const home = options.home ?? homedir(); + const found: string[] = []; + const add = (raw: string): void => { + const token = raw.replace(LEADING, "").replace(TRAILING, ""); + if (token.length === 0 || !looksLikeNamedPath(token)) return; + const path = expandNamed(token, home); + if (!found.includes(path)) found.push(path); + }; + // Quoted spans first and whole, so "~/My Documents/report.pdf" keeps + // its space; a quoted span that is not itself a path is scanned word + // by word like the rest. + const rest = text.replace( + QUOTED, + (_match, a?: string, b?: string, c?: string) => { + const inner = (a ?? b ?? c ?? "").trim(); + if (looksLikeNamedPath(inner)) { + add(inner); + return " "; + } + return ` ${inner} `; + }, + ); + for (const token of rest.split(/\s+/)) add(token); + return found; +} + +/** + * Every path the user named across a session's transcript. Only `user` + * turns are read; everything the model produced is skipped by design. + */ +export function userNamedPaths( + turns: readonly ConversationTurn[], + options: UserNamedPathOptions = {}, +): string[] { + const found: string[] = []; + for (const turn of turns) { + if (turn.kind !== "user") continue; + for (const path of pathsNamedIn(turn.text, options)) { + if (!found.includes(path)) found.push(path); + } + } + return found; +} diff --git a/src/tools/read-scope/read-scope-shell.test.ts b/src/tools/read-scope/read-scope-shell.test.ts new file mode 100644 index 00000000..3572bc3b --- /dev/null +++ b/src/tools/read-scope/read-scope-shell.test.ts @@ -0,0 +1,123 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { mkdirSync, mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join, relative } from "node:path"; + +import { + findShellPathOutsideScope, + type ShellScopeEnv, + shellTokens, +} from "./read-scope-shell.js"; + +const posixOnly = process.platform === "win32"; + +describe.skipIf(posixOnly)("shell read scope (token check)", () => { + let work: string; + // The homes are lexical, off the temp directory: the temp directory is + // scratch and always in scope, so nothing "outside" may live there. + const home = "/srv/homes/me"; + const other = "/srv/homes/me/other"; + const someone = "/srv/homes/someone"; + const env: ShellScopeEnv = { home, platform: "linux" }; + + beforeEach(() => { + work = mkdtempSync(join(tmpdir(), "shell-scope-")); + mkdirSync(join(work, "src"), { recursive: true }); + }); + + afterEach(() => { + rmSync(work, { recursive: true, force: true }); + }); + + const check = (args: Record, roots: readonly string[] = [work]) => + findShellPathOutsideScope(args, { workingDir: work }, roots, env); + + it("refuses a search rooted in someone's home, from a pre-joined line or argv", () => { + // Another user's home is under the directory homes live in. + expect(check({ cmd: `grep -rn x ${someone}` })).toBe(someone); + expect(check({ cmd: "grep", args: ["-rn", "x", someone] })).toBe(someone); + expect(check({ cmd: "cat", args: [join(other, "notes.txt")] })).toBe( + join(other, "notes.txt"), + ); + // A home elsewhere on disk is not this user's area. + expect(check({ cmd: "grep -rn x /Users/someone" })).toBeNull(); + }); + + it("never refuses the OS's own prefixes", () => { + for (const cmd of [ + "ls /usr/local/bin", + "cat /etc/hosts", + "/opt/homebrew/bin/node --version", + "ls /Applications /Library /System /bin /sbin", + "echo x 2>/dev/null", + ]) { + expect(check({ cmd }), cmd).toBeNull(); + } + }); + + it("refuses a `..` climb that escapes every root and allows one that stays inside", () => { + const secret = join(other, "notes.txt"); + const climb = relative(work, secret); + expect(check({ cmd: "cat", args: [climb] })).toBe(secret); + expect(check({ cmd: `cat ../${join(work.split("/").pop()!, "src/main.ts")}` })).toBeNull(); + // A climb the user made legitimate by naming the target. + expect(check({ cmd: `cat ${climb}` }, [work, secret])).toBeNull(); + // A climb into a system prefix is the OS's, not wandering. + expect(check({ cmd: `cat ${relative(work, "/usr/share/x")}` })).toBeNull(); + }); + + it("treats the temp directory as scratch: always in scope, by path or by climb", () => { + for (const path of [ + join(tmpdir(), "helper.py"), + "/tmp/out.txt", + "/var/tmp/cache.bin", + ]) { + expect(check({ cmd: `python3 ${path}` }), path).toBeNull(); + expect(check({ cmd: "cat", args: [path] }), path).toBeNull(); + expect(check({ cmd: "cat", args: [relative(work, path)] }), path).toBeNull(); + } + }); + + it("checks `cwd`, an `sh -c` body, a `--flag=path` value, a `~` path and a JSON-string argv", () => { + expect(check({ cmd: "npm test", cwd: other })).toBe(other); + expect(check({ cmd: "npm test", cwd: "src" })).toBeNull(); + expect(check({ cmd: "sh", args: ["-c", `cat ${other}/notes.txt`] })).toBe( + join(other, "notes.txt"), + ); + expect(check({ cmd: "tool", args: [`--dir=${other}`] })).toBe(other); + expect(check({ cmd: "cat ~/other/notes.txt" })).toBe(join(other, "notes.txt")); + expect(check({ cmd: "cat", args: `["${other}/a.txt"]` })).toBe( + join(other, "a.txt"), + ); + }); + + it("leaves everything else alone: the working directory, elsewhere on disk, patterns, URLs", () => { + for (const args of [ + { cmd: "cat", args: [join(work, "src", "main.ts")] }, + { cmd: "ls", args: ["/Volumes/data"] }, + { cmd: "grep", args: ["/^foo/", "src/main.ts"] }, + { cmd: "curl https://example.com/a/b" }, + { cmd: "sed", args: ["s/a/b/", "src/main.ts"] }, + { cmd: "echo", args: ["and/or", "a..b"] }, + ]) { + expect(check(args), JSON.stringify(args)).toBeNull(); + } + }); + + it("tokenises cmd, argv and a double-serialised argv alike", () => { + expect(shellTokens({ cmd: "grep -rn x", args: ["a b", "c"] })).toEqual([ + "grep", + "-rn", + "x", + "a", + "b", + "c", + ]); + expect(shellTokens({ cmd: "ls", args: '["-la", "src"]' })).toEqual([ + "ls", + "-la", + "src", + ]); + expect(shellTokens({ cmd: "ls", args: "not json" })).toEqual(["ls"]); + }); +}); diff --git a/src/tools/read-scope/read-scope-shell.ts b/src/tools/read-scope/read-scope-shell.ts new file mode 100644 index 00000000..a0a283a2 --- /dev/null +++ b/src/tools/read-scope/read-scope-shell.ts @@ -0,0 +1,167 @@ +import { homedir } from "node:os"; +import { dirname, resolve } from "node:path"; + +import { resolveUserPath } from "../os/expand-home.js"; +import type { ToolContext } from "../tool-registry.js"; +import { + isOutsideReadRoots, + isScratchPath, + isUnderAny, +} from "./read-scope.js"; + +/** + * The shell's share of the read scope: a NARROW pre-exec check over the + * command's tokens, nothing more. A token that is an absolute path + * under the user's home directory (or the directory homes live in) and + * lies outside every root is refused, as is a `..` climb that escapes + * every root; the OS's own prefixes and the temp directory are never + * refused. No allowlist of commands, no parsing beyond whitespace + * tokens: `grep -rn x /Users/someone` is what this catches, + * `ls /usr/local/bin` and `python3 /tmp/helper.py` are what it leaves + * alone. + */ + +export interface ShellScopeEnv { + /** The user's home directory. */ + home: string; + platform: NodeJS.Platform; +} + +export function defaultShellScopeEnv(): ShellScopeEnv { + return { home: homedir(), platform: process.platform }; +} + +/** Prefixes the OS owns: reading there is never wandering. */ +const SYSTEM_PREFIXES: Record<"posix" | "win32", readonly string[]> = { + posix: [ + "/usr", + "/bin", + "/sbin", + "/opt", + "/dev", + "/etc", + "/System", + "/Library", + "/Applications", + ], + win32: ["C:\\Windows", "C:\\Program Files", "C:\\Program Files (x86)"], +}; + +const DRIVE = /^[A-Za-z]:[\\/]/; +/** Quotes glued to a token, a redirection ahead of it, separators after it. */ +const QUOTES = /^["'`]+|["'`]+$/g; +const REDIRECT = /^[0-9]*[<>]+/; +const TRAILING = /[;|&),\]]+$/; + +function hasClimb(token: string): boolean { + return token.split(/[\\/]/).includes(".."); +} + +/** + * `cmd` followed by every argument — an array, or the JSON-string form + * of `args` some providers double-serialise — as the shell tool would + * join them. The prompt's rendering of the command; not what runs. + */ +export function shellCommandLine(args: Record): string { + const parts: string[] = []; + if (typeof args.cmd === "string") parts.push(args.cmd); + const list = args.args; + if (Array.isArray(list)) { + parts.push(...list.map(String)); + } else if (typeof list === "string" && list.trim().startsWith("[")) { + try { + const parsed = JSON.parse(list) as unknown; + if (Array.isArray(parsed)) parts.push(...parsed.map(String)); + } catch { + // Not JSON: the shell tool refuses this shape itself. + } + } + return parts.join(" "); +} + +/** + * The whitespace tokens of the command: `cmd` (a pre-joined line splits), + * every argument (an `sh -c` body splits too), and the JSON-string form + * of `args`. + */ +export function shellTokens(args: Record): string[] { + return shellCommandLine(args) + .split(/\s+/) + .filter((token) => token.length > 0); +} + +interface Candidate { + path: string; + /** A relative token that climbs with `..`, resolved against `cwd`. */ + climb: boolean; +} + +/** What a token names on disk, if anything this check cares about. */ +function candidateOf( + token: string, + cwd: string, + env: ShellScopeEnv, +): Candidate | null { + let t = token.replace(QUOTES, "").replace(REDIRECT, "").replace(TRAILING, ""); + // `--dir=/x`, `FOO=/x`: the value is what names a path. + const eq = t.indexOf("="); + if (eq > 0) { + const value = t.slice(eq + 1); + if (/^(~|\/|[A-Za-z]:[\\/])/.test(value)) t = value; + } + if (t === "~") return { path: env.home, climb: false }; + if (t.startsWith("~/") || t.startsWith("~\\")) { + return { path: resolve(env.home, t.slice(2)), climb: false }; + } + if (env.platform === "win32") { + // `/w` is a switch on Windows, never a path. + if (DRIVE.test(t)) return { path: resolve(t), climb: false }; + } else if (t.startsWith("/")) { + // `//…` is the tail of a URL. + return t.startsWith("//") ? null : { path: resolve(t), climb: false }; + } + if (hasClimb(t) && !DRIVE.test(t)) return { path: resolve(cwd, t), climb: true }; + return null; +} + +/** + * The first path a shell call names outside every root that this check + * refuses, or `null` when the command may run. + */ +export function findShellPathOutsideScope( + args: Record, + ctx: Pick, + roots: readonly string[], + env: ShellScopeEnv = defaultShellScopeEnv(), +): string | null { + const system = SYSTEM_PREFIXES[env.platform === "win32" ? "win32" : "posix"]; + // The user's area: their home and the directory homes live in (so + // another user's home counts too). + const homeParent = dirname(env.home); + const userArea = [ + env.home, + ...(homeParent === env.home || dirname(homeParent) === homeParent + ? [] + : [homeParent]), + ]; + let cwd = ctx.workingDir; + if (typeof args.cwd === "string" && args.cwd.length > 0) { + try { + cwd = resolveUserPath(args.cwd, ctx.workingDir); + } catch { + cwd = ctx.workingDir; + } + } + const candidates: Candidate[] = []; + if (cwd !== ctx.workingDir) candidates.push({ path: cwd, climb: false }); + for (const token of shellTokens(args)) { + const candidate = candidateOf(token, cwd, env); + if (candidate !== null) candidates.push(candidate); + } + for (const { path, climb } of candidates) { + if (isUnderAny(path, system) || isScratchPath(path)) continue; + if (!isOutsideReadRoots(path, roots)) continue; + if (climb || isUnderAny(path, userArea)) return path; + } + return null; +} diff --git a/src/tools/read-scope/read-scope-targets.ts b/src/tools/read-scope/read-scope-targets.ts new file mode 100644 index 00000000..8e1535a1 --- /dev/null +++ b/src/tools/read-scope/read-scope-targets.ts @@ -0,0 +1,72 @@ +/** + * Which registered tools READ the filesystem, and which of their + * arguments name what they read. Shared by the session read scope and + * the narrower fusion-worker one (`read-scope.ts`). + */ + +export type ReadTargetsOf = (args: Record) => string[]; + +function nonEmpty(value: unknown): string | undefined { + return typeof value === "string" && value.length > 0 ? value : undefined; +} + +const pathArg: ReadTargetsOf = (args) => { + const path = nonEmpty(args.path); + return path === undefined ? [] : [path]; +}; + +/** + * The read-class filesystem tools and the argument(s) naming what they + * read. A search tool's omitted root is the working directory, which is + * inside by definition, so it contributes nothing to check. + */ +export const READ_TOOL_TARGETS: ReadonlyMap = new Map< + string, + ReadTargetsOf +>([ + ["os.fs.read", pathArg], + ["os.fs.list", pathArg], + ["os.fs.hash", pathArg], + ["os.fs.watch", pathArg], + ["os.fs.read_document", pathArg], + ["os.fs.archive.list", pathArg], + ["os.fs.archive.read_entry", pathArg], + ["os.fs.grep", pathArg], + [ + "os.fs.glob", + // `cwd` wins over `path` inside the tool; either names the root. + (args) => { + const root = nonEmpty(args.cwd) ?? nonEmpty(args.path); + return root === undefined ? [] : [root]; + }, + ], + [ + "os.fs.diff", + (args) => + [nonEmpty(args.aPath), nonEmpty(args.bPath)].filter( + (path): path is string => path !== undefined, + ), + ], + [ + "vision.describe", + (args) => + [ + nonEmpty(args.path), + ...(Array.isArray(args.paths) ? args.paths.map(nonEmpty) : []), + ].filter((path): path is string => path !== undefined), + ], + [ + // A syntax check reads every file it is handed. + "verify.syntax", + (args) => + (Array.isArray(args.files) ? args.files.map(nonEmpty) : []).filter( + (path): path is string => path !== undefined, + ), + ], +]); + +/** The shell tool, checked by token rather than by a named argument. */ +export const SHELL_TOOL = "os.shell.run"; + +/** `https://…`, `data:…` — not a filesystem path, not this module's business. */ +export const URL_LIKE = /^[a-z][a-z0-9+.-]*:(?:\/\/|[^\\/])/i; diff --git a/src/tools/read-scope/read-scope.test.ts b/src/tools/read-scope/read-scope.test.ts new file mode 100644 index 00000000..d20088e0 --- /dev/null +++ b/src/tools/read-scope/read-scope.test.ts @@ -0,0 +1,143 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { FUSION_WORKER_ID_PREFIX } from "../../session/fusion-worker-session.js"; +import { + checkWorkerRead, + findSessionReadOutside, + READ_REFUSAL_REASON, + sessionReadRefusal, + sessionReadRoots, +} from "./read-scope.js"; + +const WORKER = `${FUSION_WORKER_ID_PREFIX}1`; + +describe("session read scope (the check)", () => { + let work: string; + // Everything "outside" is lexical and off the temp directory: the temp + // directory is scratch and always in scope, so a fixture there would + // never be questioned. The checks canonicalise a path that does not exist. + const elsewhere = "/srv/homes/other-run/work"; + const named = "/srv/homes/me/Desktop"; + + beforeEach(() => { + work = mkdtempSync(join(tmpdir(), "read-scope-")); + mkdirSync(join(work, "src"), { recursive: true }); + writeFileSync(join(work, "src", "main.ts"), "mine"); + }); + + afterEach(() => { + rmSync(work, { recursive: true, force: true }); + }); + + const session = (readRoots?: readonly string[]) => ({ + sessionId: "s-plain", + workingDir: work, + ...(readRoots ? { readRoots } : {}), + }); + const outside = ( + tool: string, + args: Record, + ctx = session(), + ) => findSessionReadOutside(tool, args, ctx, sessionReadRoots(ctx)); + + it("names the first path a read reaches outside the working directory", () => { + expect(outside("os.fs.read", { path: join(elsewhere, "solution.js") })).toBe( + join(elsewhere, "solution.js"), + ); + }); + + it("allows reads inside the working directory, by relative or absolute path", () => { + for (const [tool, args] of [ + ["os.fs.read", { path: "src/main.ts" }], + ["os.fs.read", { path: join(work, "src", "main.ts") }], + ["os.fs.list", { path: "." }], + ["os.fs.grep", { pattern: "x" }], + ["os.fs.glob", { pattern: "**/*.ts", cwd: "src" }], + ] as const) { + expect(outside(tool, { ...args }), tool).toBeNull(); + } + }); + + it("allows a path the user named — the file itself, or anything under a named directory", () => { + const file = join(named, "report.pdf"); + expect( + outside("os.fs.read_document", { path: file }, session([file])), + ).toBeNull(); + expect(outside("os.fs.read", { path: file }, session([named]))).toBeNull(); + expect(outside("os.fs.list", { path: named }, session([named]))).toBeNull(); + // A named file widens to that file only. + expect( + outside("os.fs.read", { path: join(named, "other.pdf") }, session([file])), + ).toBe(join(named, "other.pdf")); + }); + + it("allows anything under a root handed in beyond the context's — an approved directory", () => { + const ctx = session(); + expect( + findSessionReadOutside( + "os.fs.read", + { path: join(elsewhere, "solution.js") }, + ctx, + [...sessionReadRoots(ctx), elsewhere], + ), + ).toBeNull(); + }); + + it("checks every path a multi-path tool names and skips URLs", () => { + expect( + outside("os.fs.diff", { + aPath: "src/main.ts", + bPath: join(elsewhere, "solution.js"), + }), + ).toBe(join(elsewhere, "solution.js")); + expect( + outside("vision.describe", { path: "https://example.com/a.png" }), + ).toBeNull(); + }); + + it("treats the temp directory as scratch: always in scope, whatever the roots", () => { + for (const path of [ + join(tmpdir(), "helper.py"), + ...(process.platform === "win32" ? [] : ["/tmp/out.txt", "/var/tmp/x"]), + ]) { + expect(outside("os.fs.read", { path }), path).toBeNull(); + } + // A worker gets no such allowance: its rule predates the scope. + expect( + checkWorkerRead( + "os.fs.read", + { path: join(tmpdir(), "helper.py") }, + { sessionId: WORKER, workingDir: work }, + [], + ), + ).not.toBeNull(); + }); + + it("leaves worker sessions to the worker check", () => { + const ctx = { sessionId: WORKER, workingDir: work }; + expect( + findSessionReadOutside( + "os.fs.read", + { path: join(elsewhere, "solution.js") }, + ctx, + sessionReadRoots(ctx), + ), + ).toBeNull(); + }); + + it("the refusal names the path, the working directory and the way out", () => { + const path = join(elsewhere, "solution.js"); + const refusal = sessionReadRefusal("os.fs.read", path, session(), [work]); + expect(refusal.status).toBe("error"); + expect(refusal.summary).toBe( + `os.fs.read refused: reads are confined to the working directory (${work}) and the paths the user named; ` + + `ask the user to name ${path} or to set agent.readScope: unrestricted`, + ); + expect(refusal.details.reason).toBe(READ_REFUSAL_REASON); + expect(refusal.details.path).toBe(path); + expect(refusal.details.allowedRoots).toEqual([work]); + }); +}); diff --git a/src/tools/read-scope/read-scope.ts b/src/tools/read-scope/read-scope.ts new file mode 100644 index 00000000..acfa4709 --- /dev/null +++ b/src/tools/read-scope/read-scope.ts @@ -0,0 +1,237 @@ +import { realpathSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { basename, dirname, resolve } from "node:path"; + +import { isInside } from "../../approval/fanout-scope.js"; +import { + compressToolResult, + type CompressedToolResult, +} from "../../compressor/result-compressor.js"; +import { isFusionWorkerSessionId } from "../../session/fusion-worker-session.js"; +import { resolveUserPath } from "../os/expand-home.js"; +import type { ToolContext } from "../tool-registry.js"; +import { READ_TOOL_TARGETS, URL_LIKE } from "./read-scope-targets.js"; + +/** + * Where a session may READ. + * + * Writes and commands were always gated; reads never were, and a real + * run showed what that costs: a plain session read a sibling run's + * solution, a harness's screen dumps and the benchmark's own checker + * from far outside its working directory, and fusion workers with thin + * briefs spent their steps reverse-engineering code that was not theirs + * to match. + * + * So by default (`agent.readScope: "working-dir"`) a session's + * filesystem reads must resolve inside its working directory, under a + * path the user named in their own messages (`ctx.readRoots`, + * `read-scope-roots.ts`), or under a directory the operator approved + * earlier this session (`ReadScopeGrants`); anything else ASKS through + * the approval ladder as `fs_read_outside` (`read-scope-approval.ts`) + * and is refused only when the operator says no. A fusion worker's + * reads stay inside its working directory or a directory its fan-out + * may write in — the worker rule is the older and narrower one, a hard + * refusal (an ephemeral session has nobody to ask), and it never widens + * from the brief, which is model output. This is scope discipline + * against wandering, not a sandbox: a path that is inside lexically OR + * by canonical (realpath) form is allowed, so a symlinked + * `node_modules` keeps working and `/tmp` vs `/private/tmp` does not + * question a path that is really inside. The OS temp directory is + * scratch space — a helper written to `/tmp` and run, an output + * redirected there and read back — and is always in scope for a + * session; the threat was other users' homes and a benchmark's sibling + * trees, never scratch. A refusal is a tool result, not a throw, so it + * costs one step and reads as an instruction: ask the user, or opt out + * with `agent.readScope: "unrestricted"`. + */ + +export const READ_REFUSAL_REASON = "read-outside-scope"; +export const WORKER_READ_REFUSAL_REASON = "worker-read-outside-scope"; + +/** + * The realpath of the deepest existing ancestor with the rest appended, + * so a path that does not exist yet still canonicalises. + */ +export function canonical(path: string): string { + const rest: string[] = []; + let current = path; + for (;;) { + try { + return resolve(realpathSync.native(current), ...rest.reverse()); + } catch { + const parent = dirname(current); + if (parent === current) return path; + rest.push(basename(current)); + current = parent; + } + } +} + +/** Whether absolute `target` lies outside every root, lexically and canonically. */ +export function isOutsideReadRoots( + target: string, + roots: readonly string[], +): boolean { + if (roots.some((root) => isInside(resolve(root), target))) return false; + const real = canonical(target); + return !roots.some((root) => isInside(canonical(resolve(root)), real)); +} + +/** Whether `path` is under any of `prefixes`, lexically or canonically. */ +export function isUnderAny(path: string, prefixes: readonly string[]): boolean { + if (prefixes.some((prefix) => isInside(prefix, path))) return true; + const real = canonical(path); + return prefixes.some((prefix) => isInside(canonical(prefix), real)); +} + +/** + * The OS temp directory, plus the conventional POSIX ones — macOS's + * `os.tmpdir()` is a per-user folder, and models write to `/tmp`. Read + * per call so a test (or an operator) steering `TMPDIR` is honoured. + */ +export function scratchDirs(): string[] { + return process.platform === "win32" + ? [tmpdir()] + : [tmpdir(), "/tmp", "/var/tmp"]; +} + +/** Scratch space: always in scope for a session, whatever the roots. */ +export function isScratchPath(path: string): boolean { + return isUnderAny(path, scratchDirs()); +} + +/** A session's roots: the working directory plus the paths the user named. */ +export function sessionReadRoots( + ctx: Pick, +): string[] { + return [ctx.workingDir, ...(ctx.readRoots ?? [])]; +} + +// Paths are long; a clipped refusal loses the instruction at its end. +const uncut = (output: string) => ({ + maxSummaryLength: Math.max(1000, output.length + 50), +}); + +/** The refusal for a worker read outside its working directory and fan-out scope. */ +export function workerReadRefusal( + tool: string, + path: string, + ctx: Pick, + grantedDirs: readonly string[], +): CompressedToolResult { + const alsoScope = + grantedDirs.length > 0 + ? ` or the directories this fan-out may write in (${grantedDirs.join(", ")})` + : ""; + const output = + `${tool} refused: ${path} is outside this worker's working directory (${ctx.workingDir})${alsoScope}, and a worker reads only inside those. ` + + `Do not search elsewhere for context: use your task, its FILES and the original request, and name anything missing in your reply.`; + return compressToolResult( + { + tool, + status: "error", + output, + details: { + reason: WORKER_READ_REFUSAL_REASON, + path, + allowedRoots: [ctx.workingDir, ...grantedDirs], + }, + }, + uncut(output), + ); +} + +/** + * The refusal for a session read outside its roots — the operator said + * no at the `fs_read_outside` prompt, or there was no ladder to ask. + */ +export function sessionReadRefusal( + tool: string, + path: string, + ctx: Pick, + roots: readonly string[], +): CompressedToolResult { + const output = `${tool} refused: reads are confined to the working directory (${ctx.workingDir}) and the paths the user named; ask the user to name ${path} or to set agent.readScope: unrestricted`; + return compressToolResult( + { + tool, + status: "error", + output, + details: { reason: READ_REFUSAL_REASON, path, allowedRoots: [...roots] }, + }, + uncut(output), + ); +} + +/** + * The first target of a read tool call that resolves outside every root, + * or `null`. With `scratchInScope`, the temp directory never counts as + * outside. + */ +function firstTargetOutside( + tool: string, + args: Record, + ctx: Pick, + roots: readonly string[], + scratchInScope: boolean, +): string | null { + const targetsOf = READ_TOOL_TARGETS.get(tool); + if (targetsOf === undefined) return null; + for (const raw of targetsOf(args)) { + if (URL_LIKE.test(raw)) continue; + let absolute: string; + try { + absolute = resolveUserPath(raw, ctx.workingDir); + } catch { + // Unresolvable here means unresolvable in the tool too; its own + // error is the better message. + continue; + } + if (scratchInScope && isScratchPath(absolute)) continue; + if (isOutsideReadRoots(absolute, roots)) return absolute; + } + return null; +} + +/** + * The refusal for a worker read outside its roots, or `null` when the + * call may run (not a worker, not a read tool, or every target inside). + */ +export function checkWorkerRead( + tool: string, + args: Record, + ctx: Pick, + grantedDirs: readonly string[], +): CompressedToolResult | null { + if (!isFusionWorkerSessionId(ctx.sessionId)) return null; + // The worker rule predates the session one and keeps its shape: no + // scratch allowance — a worker's world is its task's directories. + const outside = firstTargetOutside( + tool, + args, + ctx, + [ctx.workingDir, ...grantedDirs], + false, + ); + return outside === null + ? null + : workerReadRefusal(tool, outside, ctx, grantedDirs); +} + +/** + * The first path a session read names outside `roots` — the working + * directory, the paths the user named and the directories approved so + * far (`sessionReadRoots` plus `ReadScopeGrants`) — or `null` when the + * call may run. The caller asks about it (`read-scope-approval.ts`). + * A worker session is not this check's business (`checkWorkerRead` is + * narrower and refuses outright). + */ +export function findSessionReadOutside( + tool: string, + args: Record, + ctx: Pick, + roots: readonly string[], +): string | null { + if (isFusionWorkerSessionId(ctx.sessionId)) return null; + return firstTargetOutside(tool, args, ctx, roots, true); +} diff --git a/src/tools/fusion/worker-read-scope.test.ts b/src/tools/read-scope/worker-read-scope.test.ts similarity index 98% rename from src/tools/fusion/worker-read-scope.test.ts rename to src/tools/read-scope/worker-read-scope.test.ts index 5fe5661d..4349eafa 100644 --- a/src/tools/fusion/worker-read-scope.test.ts +++ b/src/tools/read-scope/worker-read-scope.test.ts @@ -13,13 +13,13 @@ import { compressToolResult } from "../../compressor/result-compressor.js"; import { DEFAULT_TOOL_DESCRIPTORS } from "../../prompt/tool-descriptors.js"; import { FUSION_WORKER_ID_PREFIX } from "../../session/fusion-worker-session.js"; import { ToolRegistry, type ToolContext } from "../tool-registry.js"; +import { FUSION_WORKER_APPROVAL_MARKER } from "../fusion/worker-tool-policy.js"; import { checkWorkerRead, confineWorkerReads, WORKER_READ_REFUSAL_REASON, WORKER_READ_TOOL_TARGETS, -} from "./worker-read-scope.js"; -import { FUSION_WORKER_APPROVAL_MARKER } from "./worker-tool-policy.js"; +} from "./index.js"; const WORKER = `${FUSION_WORKER_ID_PREFIX}1`; diff --git a/src/tools/tool-registry.ts b/src/tools/tool-registry.ts index d5f627e0..b07410fc 100644 --- a/src/tools/tool-registry.ts +++ b/src/tools/tool-registry.ts @@ -15,6 +15,14 @@ export interface ToolContext { * (load it). Absent ⇒ `full`. */ toolRole?: ToolRole; + /** + * Absolute paths the user named in this session's own messages + * (`userNamedPaths`, `src/tools/read-scope/`), recomputed by the step + * from the transcript. Under `agent.readScope: "working-dir"` a read + * may go under any of these as well as under `workingDir`. Absent ⇒ + * nothing named. + */ + readRoots?: readonly string[]; } export interface ToolDefinition { diff --git a/src/tools/unknown-argument-guard.test.ts b/src/tools/unknown-argument-guard.test.ts index 4ba2f4c4..93036b40 100644 --- a/src/tools/unknown-argument-guard.test.ts +++ b/src/tools/unknown-argument-guard.test.ts @@ -2,6 +2,8 @@ import { describe, expect, it } from "vitest"; import { findUnknownArguments, suggestKey } from "./unknown-argument-guard.js"; const NOT_RUN = "— the call was not run; re-emit it with the right keys"; +/** `os.shell.run`'s schema keys in schema order: the command form, then the job forms (F47). */ +const SHELL_KEYS = ["cmd", "args", "cwd", "timeoutMs", "keep", "wait", "kill", "jobs"]; describe("findUnknownArguments", () => { it("refuses the live Gemma call: a flag used as a key, the script under it", () => { @@ -11,10 +13,10 @@ describe("findUnknownArguments", () => { }); expect(report).not.toBeNull(); expect(report!.unknownKeys).toEqual(["-e"]); - expect(report!.expectedKeys).toEqual(["cmd", "args", "cwd", "timeoutMs"]); + expect(report!.expectedKeys).toEqual(SHELL_KEYS); expect(report!.nearest).toEqual([]); expect(report!.message).toBe( - "unknown argument `-e` for os.shell.run (expected: cmd, args, cwd, timeoutMs; " + + `unknown argument \`-e\` for os.shell.run (expected: ${SHELL_KEYS.join(", ")}; ` + `put the script in args: ["-c", "…"]) ${NOT_RUN}`, ); }); @@ -35,7 +37,7 @@ describe("findUnknownArguments", () => { expect(report!.unknownKeys).toEqual(["-args"]); expect(report!.nearest).toEqual([{ received: "-args", expected: "args" }]); expect(report!.message).toBe( - "unknown argument `-args` for os.shell.run (expected: cmd, args, cwd, timeoutMs; " + + `unknown argument \`-args\` for os.shell.run (expected: ${SHELL_KEYS.join(", ")}; ` + `did you mean \`args\`?) ${NOT_RUN}`, ); }); @@ -82,7 +84,7 @@ describe("findUnknownArguments", () => { }); expect(report!.unknownKeys).toEqual(["-e", "-args"]); expect(report!.message).toBe( - "unknown arguments `-e`, `-args` for os.shell.run (expected: cmd, args, cwd, timeoutMs; " + + `unknown arguments \`-e\`, \`-args\` for os.shell.run (expected: ${SHELL_KEYS.join(", ")}; ` + "did you mean `args` instead of `-args`?; " + `put the script in args: ["-c", "…"]) ${NOT_RUN}`, ); @@ -140,9 +142,9 @@ describe("suggestKey", () => { expect(suggestKey("oldstring", ["path", "oldString"])).toBe("oldString"); }); - it("suggests nothing for a bare flag: `-e` is not `cmd`", () => { - expect(suggestKey("-e", ["cmd", "args", "cwd", "timeoutMs"])).toBeNull(); - expect(suggestKey("-c", ["cmd", "args", "cwd", "timeoutMs"])).toBeNull(); + it("suggests nothing for a bare flag: `-e` is not `cmd`, nor `keep`", () => { + expect(suggestKey("-e", SHELL_KEYS)).toBeNull(); + expect(suggestKey("-c", SHELL_KEYS)).toBeNull(); expect(suggestKey("-", ["cmd"])).toBeNull(); }); });