diff --git a/apps/desktop/main/capabilities/broker-workspace.test.ts b/apps/desktop/main/capabilities/broker-workspace.test.ts index 121187fe6..c7f3f0a9e 100644 --- a/apps/desktop/main/capabilities/broker-workspace.test.ts +++ b/apps/desktop/main/capabilities/broker-workspace.test.ts @@ -35,6 +35,9 @@ class Grants implements GrantProvider { expiresAt: Number.MAX_SAFE_INTEGER, mode: "read_write", label: "project", + // Default OFF, stated explicitly: a fixture that omitted it would be the + // one place absence is allowed to mean something other than "no commands". + shellEnabled: false, status: "active", createdAt: 1, updatedAt: 1, diff --git a/apps/desktop/main/capabilities/broker.test.ts b/apps/desktop/main/capabilities/broker.test.ts index 63dd73325..3f15c02bd 100644 --- a/apps/desktop/main/capabilities/broker.test.ts +++ b/apps/desktop/main/capabilities/broker.test.ts @@ -28,6 +28,9 @@ function makeGrant(overrides: Partial = {}): Grant { root: "/data/private", mode: "read_only", label: "private", + // Default OFF, stated explicitly: a fixture that omitted it would be the + // one place absence is allowed to mean something other than "no commands". + shellEnabled: false, status: "active", createdAt: 1, updatedAt: 1, @@ -213,6 +216,11 @@ describe("CapabilityBroker", () => { "mode", "mount", "root", + // PRD-shell-execution §7.3 prerequisite 3 — the runtime's ONLY read of + // the per-workspace command decision. Listed here because this assertion + // is the broker wire's shape contract, and an unlisted key would mean the + // flag reached the worker without anyone deciding it should. + "shellEnabled", "status", ]); expect(g.root).toBe("/data/private"); @@ -246,6 +254,30 @@ describe("CapabilityBroker", () => { expect(text).not.toContain("/data/gone"); }); + it("a detached workspace hands out no COMMAND capability either", async () => { + // The same rule as `root`, for the flag the runtime reads as §7.1's third + // prerequisite. A revoked (or expired — the store reports those revoked too) + // grant authorizes nothing, so shipping `shellEnabled: true` for it would + // let a worker treat a folder the user explicitly detached as still + // command-capable. Absent decodes on the Python side as `False`. + grants.grants = [ + makeGrant({ grantId: "live", status: "active", shellEnabled: true }), + makeGrant({ grantId: "gone", status: "revoked", shellEnabled: true }), + ]; + + const res = await fetch(`${baseUrl}/v1/grants/list`, { + method: "POST", + headers: H(), + body: "{}", + }); + const body = (await res.json()) as { + grants: Array>; + }; + + expect(body.grants[0].shellEnabled).toBe(true); + expect(body.grants[1]).not.toHaveProperty("shellEnabled"); + }); + it("snapshots only active grants, path-free with a stable mount id", async () => { grants.grants = [ makeGrant({ grantId: "a", status: "active" }), diff --git a/apps/desktop/main/capabilities/channels.ts b/apps/desktop/main/capabilities/channels.ts index a33698ced..44c0b45c3 100644 --- a/apps/desktop/main/capabilities/channels.ts +++ b/apps/desktop/main/capabilities/channels.ts @@ -19,6 +19,18 @@ export const CAPABILITY_CHANNELS = { listGrants: "capability.list-grants", /** Renderer → main: revoke a grant by id. */ revokeGrant: "capability.revoke-grant", + /** + * Renderer → main: turn per-workspace SHELL EXECUTION on/off for one grant + * (PRD-shell-execution §7.3). The FIFTH capability channel, and the only one + * that mutates an existing grant — before it, `GrantStore` had `create` and + * `revoke` and nothing else, so the renderer had no way to reach a change of + * this kind at all. + * + * It carries a decision, never an execution. Nothing on this channel runs a + * command, and the broker gains no execute verb; main records what the user + * chose and the runtime reads it back off the active-grant snapshot. + */ + setGrantShellEnabled: "capability.set-grant-shell-enabled", /** Renderer → main: decide one digest-pinned workspace stage revision. */ decideWorkspaceApproval: "capability.decide-workspace-approval", } as const; diff --git a/apps/desktop/main/capabilities/feature-gate.test.ts b/apps/desktop/main/capabilities/feature-gate.test.ts index 7c3532ac0..e9142114c 100644 --- a/apps/desktop/main/capabilities/feature-gate.test.ts +++ b/apps/desktop/main/capabilities/feature-gate.test.ts @@ -123,6 +123,7 @@ const fakeCapability = { requestFolderGrant: async () => null, listGrants: async () => [], revokeGrant: async () => null, + setGrantShellEnabled: async () => null, }; function registerWithGate(env: Record) { @@ -144,11 +145,12 @@ function registerWithGate(env: Record) { } describe("capability subsystem gate → IPC registration (G4)", () => { - it("default (flag unset): all three capability channels are registered", () => { + it("default (flag unset): all four capability channels are registered", () => { const ipcMain = registerWithGate({}); expect(ipcMain.has(CAPABILITY_CHANNELS.requestFolderGrant)).toBe(true); expect(ipcMain.has(CAPABILITY_CHANNELS.listGrants)).toBe(true); expect(ipcMain.has(CAPABILITY_CHANNELS.revokeGrant)).toBe(true); + expect(ipcMain.has(CAPABILITY_CHANNELS.setGrantShellEnabled)).toBe(true); }); it("explicit opt-out: capability channels are NOT registered (calls fail closed)", () => { @@ -156,6 +158,11 @@ describe("capability subsystem gate → IPC registration (G4)", () => { expect(ipcMain.has(CAPABILITY_CHANNELS.requestFolderGrant)).toBe(false); expect(ipcMain.has(CAPABILITY_CHANNELS.listGrants)).toBe(false); expect(ipcMain.has(CAPABILITY_CHANNELS.revokeGrant)).toBe(false); + // PRD-shell-execution §7.3. With the capability subsystem off there are no + // grants, so there is nothing to enable commands ON — and the one channel + // that can turn command execution on must not outlive the subsystem that + // owns the authority list it writes to. + expect(ipcMain.has(CAPABILITY_CHANNELS.setGrantShellEnabled)).toBe(false); }); it("unreadable flag value: also NOT registered (fail closed, not fail open)", () => { @@ -163,6 +170,7 @@ describe("capability subsystem gate → IPC registration (G4)", () => { expect(ipcMain.has(CAPABILITY_CHANNELS.requestFolderGrant)).toBe(false); expect(ipcMain.has(CAPABILITY_CHANNELS.listGrants)).toBe(false); expect(ipcMain.has(CAPABILITY_CHANNELS.revokeGrant)).toBe(false); + expect(ipcMain.has(CAPABILITY_CHANNELS.setGrantShellEnabled)).toBe(false); }); }); diff --git a/apps/desktop/main/capabilities/grant-shell-projection.test.ts b/apps/desktop/main/capabilities/grant-shell-projection.test.ts new file mode 100644 index 000000000..0380d1207 --- /dev/null +++ b/apps/desktop/main/capabilities/grant-shell-projection.test.ts @@ -0,0 +1,119 @@ +// @vitest-environment node +// +// PRD-shell-execution §7.3 — where the per-workspace command flag is allowed to +// GO, and where it must not. +// +// `Grant` holds the decision; three projections carry it (or refuse to) to three +// different audiences. Each has a different reason, and each fails differently: +// +// toRendererGrant → the Settings toggle. Needs the CURRENT state, so the +// control shows what main holds rather than what the +// renderer last asked for. +// toBrokerGrant → the runtime worker. THE READ PATH (§7.1 prerequisite +// 3). Present only while the grant is live. +// toHostSessionGrant → the C2 write-authority bootstrap. Must NOT carry it: +// `_assert_host_session_wire_is_private` allow-lists +// five grant keys by name and raises +// `BrokerProtocolError` on any sixth, so a leak here +// fails every live host session closed while every test +// that builds the payload by hand stays green. +// +// A projection test is cheap and this one is not decorative: the flag decides +// whether an agent may run code on the machine, so "which audience learns it" +// is a security question with three different right answers. + +import { describe, expect, it } from "vitest"; + +import type { Grant } from "./types"; +import { toBrokerGrant, toHostSessionGrant, toRendererGrant } from "./types"; + +function makeGrant(overrides: Partial = {}): Grant { + return { + grantId: "g_atlas", + root: "/Users/x/projects/atlas", + mode: "read_write", + label: "atlas", + shellEnabled: false, + status: "active", + createdAt: 1, + updatedAt: 1, + ...overrides, + }; +} + +describe("toRendererGrant — the Settings toggle's view", () => { + it("reports the flag so the toggle can show the state main actually holds", () => { + expect( + toRendererGrant(makeGrant({ shellEnabled: true })).shellEnabled, + ).toBe(true); + expect( + toRendererGrant(makeGrant({ shellEnabled: false })).shellEnabled, + ).toBe(false); + }); + + it("a grant that authorizes nothing advertises nothing", () => { + // `GrantStore.list` reports an expired grant as revoked, so `status` here is + // already the EFFECTIVE one and expiry rides this same line. Without it a + // detached workspace's row would read "commands allowed" over authority + // that has lapsed. + expect( + toRendererGrant(makeGrant({ shellEnabled: true, status: "revoked" })) + .shellEnabled, + ).toBe(false); + }); + + it("stays path-free — the flag did not become a path oracle", () => { + const projected = toRendererGrant(makeGrant({ shellEnabled: true })); + expect(Object.keys(projected).sort()).toEqual([ + "grantId", + "label", + "mode", + "shellEnabled", + "status", + ]); + expect(JSON.stringify(projected)).not.toContain("/Users/x"); + }); +}); + +describe("toBrokerGrant — the runtime's read path (§7.1 prerequisite 3)", () => { + it("carries the flag for a live grant", () => { + const projected = toBrokerGrant(makeGrant({ shellEnabled: true }), "mnt_1"); + expect(projected.shellEnabled).toBe(true); + expect(projected.root).toBe("/Users/x/projects/atlas"); + }); + + it("OMITS the flag entirely for a revoked grant, alongside the root", () => { + // Absence is the same answer an older Electron main gives, and the Python + // side defaults `shell_enabled` to False — so every route by which the fact + // can be missing means "this workspace cannot run commands", never "it can". + const projected = toBrokerGrant( + makeGrant({ shellEnabled: true, status: "revoked" }), + "mnt_1", + ); + expect(projected).not.toHaveProperty("shellEnabled"); + expect(projected).not.toHaveProperty("root"); + }); +}); + +describe("toHostSessionGrant — the one projection that must NOT carry it", () => { + it("emits exactly the five allow-listed keys", () => { + // Adding a sixth key here does not merely widen a contract: the ai-backend + // asserts this wire field-by-field and fails the whole host session closed + // on anything it does not recognise. The flag has no business here anyway — + // this bootstrap carries WRITE authority for staged effects, and shell + // enablement is read off `/v1/grants/snapshot`. + const projected = toHostSessionGrant( + makeGrant({ shellEnabled: true }), + "mnt_1", + ); + expect(Object.keys(projected).sort()).toEqual([ + "grantId", + "label", + "mode", + "mount", + "status", + ]); + expect(projected).not.toHaveProperty("shellEnabled"); + expect(projected).not.toHaveProperty("root"); + }); +}); diff --git a/apps/desktop/main/capabilities/grant-store.test.ts b/apps/desktop/main/capabilities/grant-store.test.ts index aadcace0f..5042910b2 100644 --- a/apps/desktop/main/capabilities/grant-store.test.ts +++ b/apps/desktop/main/capabilities/grant-store.test.ts @@ -1,5 +1,12 @@ // @vitest-environment node -import { mkdtempSync, readdirSync, readFileSync, rmSync } from "node:fs"; +import { + mkdirSync, + mkdtempSync, + readdirSync, + readFileSync, + rmSync, + writeFileSync, +} from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -391,3 +398,322 @@ describe("GrantStore — sensitive-root policy (G2)", () => { ); }); }); + +// --------------------------------------------------------------------------- +// Per-workspace SHELL EXECUTION enablement (PRD-shell-execution §7.3). +// +// This block is about ONE boolean, and it is long because the boolean decides +// whether an agent may run code on someone's machine. The four properties it +// pins are the four ways the flag could be wrong: +// +// 1. A workspace with no flag REFUSES. Absence is the common case — every +// grant minted before the field existed lacks it — so "off" has to be what +// happens when the value is missing, not only when it is explicitly false. +// 2. Enabling is an EXPLICIT ACT. There is exactly one way in, it is not the +// attach flow, and nothing about creating or re-creating a grant reaches it. +// 3. The flag is PER-WORKSPACE and does not leak across grants. +// 4. A malformed record FAILS CLOSED — a store we cannot decode authorizes +// nothing, rather than decoding to whatever JS finds truthy. +// --------------------------------------------------------------------------- +describe("GrantStore — per-workspace shell enablement (§7.3)", () => { + let tmp: string; + + beforeEach(() => { + idCounter = 0; + tmp = mkdtempSync(join(tmpdir(), "cap-grants-shell-")); + }); + + afterEach(() => { + rmSync(tmp, { recursive: true, force: true }); + }); + + function makeStore(clock: () => number = () => 1000, grantTtlMs?: number) { + return new GrantStore({ + userDataDir: tmp, + homeDir: TEST_HOME, + safeStorage: makeFakeSafeStorage(true), + uuid: seqUuid, + clock, + grantTtlMs, + }); + } + + /** + * Write a grant row straight to disk, bypassing `create`, so the decode seam + * can be tested against shapes `create` would never mint — including the + * legacy row that has no `shellEnabled` key at all. Plaintext because the + * fake cipher is not the thing under test here. + */ + function seedRawStore(rows: readonly Record[]): GrantStore { + mkdirSync(join(tmp, "capabilities"), { recursive: true, mode: 0o700 }); + writeFileSync( + join(tmp, "capabilities", "grants.bin"), + `ATLASCAPv1:plaintext:${JSON.stringify({ version: 1, grants: rows })}`, + ); + return new GrantStore({ + userDataDir: tmp, + homeDir: TEST_HOME, + safeStorage: makeFakeSafeStorage(false), + allowPlaintextFallback: true, + uuid: seqUuid, + clock: () => 1000, + }); + } + + function legacyRow( + overrides: Record = {}, + ): Record { + return { + grantId: "00000000-0000-4000-8000-000000009001", + root: "/Users/x/projects/legacy", + mode: "read_write", + label: "legacy", + status: "active", + createdAt: 1, + updatedAt: 1, + ...overrides, + }; + } + + // --- 1. Absent means off ------------------------------------------------- + + it("a freshly created grant cannot run commands", async () => { + const store = makeStore(); + const grant = await store.create({ + root: "/Users/x/projects/atlas", + mode: "read_write", + label: "atlas", + }); + expect(grant.shellEnabled).toBe(false); + }); + + it("a grant persisted BEFORE the field existed decodes as off, not undefined", async () => { + // The common case, and the one an `undefined`-tolerant reader gets wrong. + // `shellEnabled` is REQUIRED on `Grant`, so this asserts the fold happens at + // the decode seam — the row has no such key at all. + const store = seedRawStore([legacyRow()]); + const [grant] = await store.list(); + expect(grant.shellEnabled).toBe(false); + expect(Object.prototype.hasOwnProperty.call(grant, "shellEnabled")).toBe( + true, + ); + }); + + it("an explicit false on disk is off, and an explicit true is honoured", async () => { + // The `true` half exists so the `false`/absent halves above cannot pass by + // the flag being unreadable in every direction. + const off = seedRawStore([legacyRow({ shellEnabled: false })]); + expect((await off.list())[0].shellEnabled).toBe(false); + + rmSync(join(tmp, "capabilities"), { recursive: true, force: true }); + const on = seedRawStore([legacyRow({ shellEnabled: true })]); + expect((await on.list())[0].shellEnabled).toBe(true); + }); + + // --- 2. Enabling is an explicit act ------------------------------------- + + it("setShellEnabled is the ONLY way in — create ignores a smuggled flag", async () => { + // `CreateGrantInput` has no `shellEnabled`, so this cannot be written + // without a cast; the cast is the point. An attach flow that "helpfully" + // forwarded the field would compile against a looser type one refactor from + // now, and this asserts the store still mints `false`. + const store = makeStore(); + const grant = await store.create({ + root: "/Users/x/projects/atlas", + mode: "read_write", + label: "atlas", + shellEnabled: true, + } as unknown as Parameters[0]); + expect(grant.shellEnabled).toBe(false); + expect((await store.get(grant.grantId))?.shellEnabled).toBe(false); + }); + + it("setShellEnabled(true) turns it on and the decision survives a reopen", async () => { + const store = makeStore(); + const grant = await store.create({ + root: "/Users/x/projects/atlas", + mode: "read_write", + label: "atlas", + }); + const updated = await store.setShellEnabled(grant.grantId, true); + expect(updated?.shellEnabled).toBe(true); + expect(updated?.grantId).toBe(grant.grantId); // NOT a new grant + + const reopened = new GrantStore({ + userDataDir: tmp, + safeStorage: makeFakeSafeStorage(true), + }); + expect((await reopened.get(grant.grantId))?.shellEnabled).toBe(true); + }); + + it("setShellEnabled(false) turns it back off", async () => { + const store = makeStore(); + const grant = await store.create({ + root: "/Users/x/projects/atlas", + mode: "read_write", + label: "atlas", + }); + await store.setShellEnabled(grant.grantId, true); + const off = await store.setShellEnabled(grant.grantId, false); + expect(off?.shellEnabled).toBe(false); + }); + + it("returns null for an id the store has never seen", async () => { + const store = makeStore(); + expect( + await store.setShellEnabled("00000000-0000-4000-8000-00000000dead", true), + ).toBeNull(); + }); + + it("re-attaching the same folder does NOT carry command authority forward", async () => { + // `#supersedeSameRoot` retires the previous grant when a root is re-picked. + // A fresh grant is a fresh consent: the obvious "carry the setting forward" + // convenience would re-grant command authority as a side effect of + // re-picking a folder in a dialog about SHARING one. + const store = makeStore(); + const root = "/Users/x/projects/atlas"; + const first = await store.create({ root, mode: "read_write", label: "a" }); + await store.setShellEnabled(first.grantId, true); + + const second = await store.create({ root, mode: "read_write", label: "a" }); + expect(second.grantId).not.toBe(first.grantId); + expect(second.shellEnabled).toBe(false); + // And the superseded grant is gone as authority, not lingering enabled. + expect((await store.get(first.grantId))?.status).toBe("revoked"); + }); + + it("enabling is refused on a revoked grant, but disabling always lands", async () => { + // Asymmetric on purpose: a control that REMOVES authority must never be + // blocked by the state of the record. A control that GRANTS it must be. + const store = makeStore(); + const grant = await store.create({ + root: "/Users/x/projects/atlas", + mode: "read_write", + label: "atlas", + }); + await store.setShellEnabled(grant.grantId, true); + await store.revoke(grant.grantId); + + const refused = await store.setShellEnabled(grant.grantId, false); + expect(refused?.shellEnabled).toBe(false); + + const reEnabled = await store.setShellEnabled(grant.grantId, true); + // Reported as the record stands — NOT as a success. A revoked workspace + // must not be sitting there command-capable for whatever re-activates it. + expect(reEnabled?.shellEnabled).toBe(false); + expect(reEnabled?.status).toBe("revoked"); + }); + + it("enabling is refused on an EXPIRED grant", async () => { + // Expiry is the failure mode `status === "active"` alone would miss: the + // stored literal still says active, and only `isLive` knows better. + let now = 1000; + const store = makeStore(() => now, 500); + const grant = await store.create({ + root: "/Users/x/projects/atlas", + mode: "read_write", + label: "atlas", + }); + now = 1_501; + const result = await store.setShellEnabled(grant.grantId, true); + expect(result?.shellEnabled).toBe(false); + expect(result?.status).toBe("revoked"); + }); + + it("is idempotent — re-asserting the current value writes nothing", async () => { + let now = 1000; + const store = makeStore(() => now); + const grant = await store.create({ + root: "/Users/x/projects/atlas", + mode: "read_write", + label: "atlas", + }); + now = 2000; + const noop = await store.setShellEnabled(grant.grantId, false); + expect(noop?.updatedAt).toBe(1000); // not bumped + const changed = await store.setShellEnabled(grant.grantId, true); + expect(changed?.updatedAt).toBe(2000); + }); + + // --- 3. Per-workspace, no leakage --------------------------------------- + + it("enabling one workspace leaves every other workspace off", async () => { + const store = makeStore(); + const atlas = await store.create({ + root: "/Users/x/projects/atlas", + mode: "read_write", + label: "atlas", + }); + const notes = await store.create({ + root: "/Users/x/projects/notes", + mode: "read_write", + label: "notes", + }); + const docs = await store.create({ + root: "/Users/x/Documents", + mode: "read_only", + label: "Documents", + }); + + await store.setShellEnabled(atlas.grantId, true); + + const byId = new Map( + (await store.list()).map((g) => [g.grantId, g.shellEnabled]), + ); + expect(byId.get(atlas.grantId)).toBe(true); + expect(byId.get(notes.grantId)).toBe(false); + expect(byId.get(docs.grantId)).toBe(false); + + // And the same seen through the snapshot the runtime actually reads. + const snapshot = await store.snapshotActive(); + expect( + snapshot.grants.filter((g) => g.shellEnabled).map((g) => g.grantId), + ).toEqual([atlas.grantId]); + }); + + it("does not widen mode — a read-only folder stays read-only", async () => { + // Command execution is a different authority, not more file access. + const store = makeStore(); + const grant = await store.create({ + root: "/Users/x/Documents", + mode: "read_only", + label: "Documents", + }); + const updated = await store.setShellEnabled(grant.grantId, true); + expect(updated?.mode).toBe("read_only"); + expect(updated?.shellEnabled).toBe(true); + }); + + // --- 4. A malformed record fails closed --------------------------------- + + it.each([ + ['the string "true"', "true"], + ["the number 1", 1], + ["an object", {}], + ["an array", []], + ["null", null], + ])("refuses to decode a store whose flag is %s", async (_label, value) => { + // Every one of these is TRUTHY in JS (except null, which is the shape a + // sloppy `?? false` would silently accept), so admitting them and coercing + // later is exactly how a tampered authority file becomes command authority. + // The throw is the fail-closed behaviour: an undecodable store yields no + // grants, therefore no command capability, rather than a best guess. + const store = seedRawStore([legacyRow({ shellEnabled: value })]); + await expect(store.list()).rejects.toThrow(/invalid fields/u); + }); + + it("one malformed row poisons the whole store, not just its own grant", async () => { + // Deliberate. A partially-decoded authority list is a list whose contents + // depend on which rows happened to parse, and silently dropping the + // unreadable one is how the store's answer stops matching the user's + // decisions without anything reporting it. + const store = seedRawStore([ + legacyRow({ shellEnabled: false }), + legacyRow({ + grantId: "00000000-0000-4000-8000-000000009002", + shellEnabled: "yes", + }), + ]); + await expect(store.list()).rejects.toThrow(/invalid fields/u); + }); +}); diff --git a/apps/desktop/main/capabilities/grant-store.ts b/apps/desktop/main/capabilities/grant-store.ts index bc5d94814..b430dc5d0 100644 --- a/apps/desktop/main/capabilities/grant-store.ts +++ b/apps/desktop/main/capabilities/grant-store.ts @@ -67,6 +67,27 @@ export interface GrantStoreConfig { readonly grantTtlMs?: number; } +/** + * What `create` accepts. It deliberately has NO `shellEnabled`. + * + * PRD §7.3 item 1 proposes the field here "so the attach flow can set it at + * mint time". We went narrower on purpose, and the narrower direction is the + * safe one for a default-off execution flag: + * + * * **Enabling stays exactly one act.** With a mint-time parameter there are + * two ways a workspace becomes command-capable, and the attach flow's way is + * a single click on a dialog the user opened to share a FOLDER. Without it + * there is one path — `setShellEnabled` — reached only from the Settings + * toggle, which names the folder and states §11.5's sentence. + * * **Re-attaching cannot launder the flag.** `#supersedeSameRoot` retires the + * previous grant whenever a root is re-picked, so a mint-time parameter would + * be the obvious place to "carry the setting forward" — quietly re-granting + * command authority as a side effect of re-picking a folder. A fresh grant is + * a fresh consent, and it starts off. + * + * The cost is one extra click for a user who wants both at once. That is the + * right trade for the one capability in this app that runs code as them. + */ export interface CreateGrantInput { /** Canonical, realpath-resolved absolute directory (from the picker). */ readonly root: string; @@ -167,6 +188,10 @@ export class GrantStore implements GrantProvider { mode: input.mode, label: input.label, status: "active", + // OFF BY DEFAULT, stated as a literal rather than left to a default so + // this line has to be edited (and reviewed) for a newly attached folder + // to ever arrive command-capable. See `CreateGrantInput`. + shellEnabled: false, createdAt: now, updatedAt: now, rootIdentity, @@ -260,6 +285,60 @@ export class GrantStore implements GrantProvider { return revoked; } + /** + * Turn per-workspace SHELL EXECUTION on or off for ONE existing grant + * (PRD-shell-execution §7.3). Returns the updated record, or `null` for an + * id the store has never seen. + * + * THIS IS THE WRITE PATH THAT DID NOT EXIST. Before it, `GrantStore` had + * exactly two mutations — `create` and `revoke` — and no way to change an + * existing grant at all. That absence is why "the grant record gains a + * boolean" is not a field: without this method the only compiling way to + * implement the toggle is `create`, and `create` is the wrong tool three + * times over. It mints a NEW `grantId` (so every run context, mount table and + * renderer pill keyed on the old id is now pointing at a retired grant), it + * runs `#supersedeSameRoot` (so the act of enabling a checkbox silently + * revokes and reissues the user's folder authority), and the renderer cannot + * call it anyway — `requestFolderGrant` needs a host path the renderer is + * never given, so the "toggle" would open a native folder picker. + * + * THE ASYMMETRY IS DELIBERATE. Disabling always succeeds — it can only ever + * remove authority, so it must never be blocked by the state of the record. + * ENABLING requires a live grant: `isLive` is the one definition of "still + * authority" (active AND unexpired), and enabling execution on a revoked or + * expired workspace would resurrect it as command-capable the moment anything + * re-activated it. An enable against a non-live grant is a NO-OP that + * persists nothing and returns the record as it stands, so the caller sees + * `shellEnabled: false` and can say so rather than reporting a success that + * did not happen. + * + * Idempotent: setting the value it already holds writes nothing and does not + * bump `updatedAt`, so a re-render that re-asserts the current state cannot + * churn the encrypted authority file. + */ + async setShellEnabled( + grantId: string, + enabled: boolean, + ): Promise { + await this.#ensureLoaded(); + const existing = this.#grants.get(grantId); + if (existing === undefined) return null; + const now = this.#clock(); + // Enabling needs live authority; disabling never does. `asOf` is what the + // caller would have seen from `get`/`list`, so a refused enable reports the + // same effective record every other read reports. + if (enabled && !isLive(existing, now)) return asOf(existing, now); + if (existing.shellEnabled === enabled) return asOf(existing, now); + const updated: Grant = { + ...existing, + shellEnabled: enabled, + updatedAt: now, + }; + this.#grants.set(grantId, updated); + await this.#persist(); + return asOf(updated, now); + } + // --- GrantProvider (broker read-side) --- async listAll(): Promise { @@ -456,6 +535,7 @@ function coerceGrant(row: unknown): Grant { const deviceId = r.deviceId; const allowedPathPrefixes = r.allowedPathPrefixes; const expiresAt = r.expiresAt; + const shellEnabled = r.shellEnabled; if ( typeof grantId !== "string" || typeof root !== "string" || @@ -472,7 +552,14 @@ function coerceGrant(row: unknown): Grant { (allowedPathPrefixes !== undefined && (!Array.isArray(allowedPathPrefixes) || allowedPathPrefixes.some((value) => typeof value !== "string"))) || - (expiresAt !== undefined && typeof expiresAt !== "number") + (expiresAt !== undefined && typeof expiresAt !== "number") || + // ABSENT IS FINE — it is the common case, and it is what every grant minted + // before shell execution existed looks like. PRESENT-BUT-NOT-A-BOOLEAN is + // not: `"true"`, `1` and `{}` are all truthy in JS, so admitting them and + // coercing later is how a tampered or corrupted row turns into command + // authority. It throws for the same reason a bad `mode` throws — this file + // is an authority list, and a list we cannot decode authorizes nothing. + (shellEnabled !== undefined && typeof shellEnabled !== "boolean") ) { throw new Error("grant row has invalid fields"); } @@ -482,6 +569,11 @@ function coerceGrant(row: unknown): Grant { mode, label, status, + // Absence is folded to `false` HERE, once, so no reader downstream ever + // meets `undefined` and has to remember which way to fold it. Spelled + // `=== true` rather than `?? false` on purpose: it is the form that still + // fails closed if someone later loosens the type check above. + shellEnabled: shellEnabled === true, createdAt, updatedAt, rootIdentity: rootIdentity as GrantRootIdentity | undefined, diff --git a/apps/desktop/main/capabilities/named-folder-grant.test.ts b/apps/desktop/main/capabilities/named-folder-grant.test.ts index 448fe44e0..226b76110 100644 --- a/apps/desktop/main/capabilities/named-folder-grant.test.ts +++ b/apps/desktop/main/capabilities/named-folder-grant.test.ts @@ -202,6 +202,11 @@ describe("always-allow mints a grant on the named folder", () => { "grantId", "label", "mode", + // Per-workspace shell enablement (PRD-shell-execution §7.3). It is a + // boolean, not a path, so the path-free property this assertion guards is + // unchanged — but the key COUNT is part of the guard, so it is named here + // rather than allowed to slip in unlisted. + "shellEnabled", "status", ]); expect(JSON.stringify(view)).not.toContain(reports); diff --git a/apps/desktop/main/capabilities/run-context.test.ts b/apps/desktop/main/capabilities/run-context.test.ts index bc511667f..dbaa4a754 100644 --- a/apps/desktop/main/capabilities/run-context.test.ts +++ b/apps/desktop/main/capabilities/run-context.test.ts @@ -10,6 +10,9 @@ function grant(overrides: Partial = {}): Grant { root: "/data/private", mode: "read_write", label: "private", + // Default OFF, stated explicitly: a fixture that omitted it would be the + // one place absence is allowed to mean something other than "no commands". + shellEnabled: false, status: "active", createdAt: 1, updatedAt: 1, diff --git a/apps/desktop/main/capabilities/schemas.ts b/apps/desktop/main/capabilities/schemas.ts index 0c5440741..6b267ac41 100644 --- a/apps/desktop/main/capabilities/schemas.ts +++ b/apps/desktop/main/capabilities/schemas.ts @@ -65,6 +65,25 @@ export const RevokeGrantParamsSchema = z .strict(); export type RevokeGrantParams = z.infer; +// capability.set-grant-shell-enabled — the ONLY way `Grant.shellEnabled` +// becomes true (PRD-shell-execution §7.3). +// +// `enabled` is a REQUIRED boolean rather than an optional one or a toggle verb. +// An optional field would let `{ grantId }` mean "flip it", which makes the +// renderer's request depend on a state main holds — so two clicks racing, or a +// retry after a dropped reply, could land on "on" when the user's last act was +// "off". The renderer states the value it wants; main records exactly that. +// `.strict()` so no mode, path, label, or command rides along. +export const SetGrantShellEnabledParamsSchema = z + .object({ + grantId: z.string().uuid(), + enabled: z.boolean(), + }) + .strict(); +export type SetGrantShellEnabledParams = z.infer< + typeof SetGrantShellEnabledParamsSchema +>; + // The ONLY grant shape allowed to cross the IPC boundary. `.strict()` is the // structural guarantee that no host `root` (or any other field) leaks: parsing // an internal Grant through this schema throws on the extra key. @@ -74,6 +93,13 @@ export const RendererGrantSchema = z mode: GrantModeSchema, label: z.string(), status: z.enum(["active", "revoked"]), + // Whether this workspace may run commands. `z.boolean()` is REQUIRED, not + // `.optional()`: the outbound projection always sets it, and a required + // field is what makes an older main (or a future refactor that forgets the + // key) fail this parse loudly instead of shipping a row the renderer would + // read as `undefined` — which renders as an unchecked toggle that is + // indistinguishable from an honest "off". + shellEnabled: z.boolean(), }) .strict(); export type RendererGrantOut = z.infer; diff --git a/apps/desktop/main/capabilities/service.test.ts b/apps/desktop/main/capabilities/service.test.ts index 28811cb46..a9bfb86e6 100644 --- a/apps/desktop/main/capabilities/service.test.ts +++ b/apps/desktop/main/capabilities/service.test.ts @@ -90,6 +90,11 @@ describe("CapabilityService", () => { "grantId", "label", "mode", + // Per-workspace shell enablement (PRD-shell-execution §7.3). It is a + // boolean, not a path, so the path-free property this assertion guards is + // unchanged — but the key COUNT is part of the guard, so it is named here + // rather than allowed to slip in unlisted. + "shellEnabled", "status", ]); // The host path must NOT be present anywhere in the renderer payload. @@ -194,4 +199,44 @@ describe("CapabilityService", () => { await service.revokeGrant("00000000-0000-4000-8000-000000000000"), ).toBeNull(); }); + + // PRD-shell-execution §7.3 — the service method the Settings toggle reaches. + // It exists because `CapabilityService` had NO mutation of an existing grant + // at all: `requestFolderGrant` / `listGrants` / `revokeGrant` and nothing + // else, so a toggle had no method to call and the only compiling stand-in + // (`requestFolderGrant`) mints a new grantId, supersedes the folder's + // existing authority, and opens a native picker. + it("setGrantShellEnabled flips the flag on the SAME grant, and only that one", async () => { + const { service } = makeService( + async () => ({ canceled: false, filePaths: ["/data/reports"] }), + tmp, + async () => "/data/reports", + ); + const created = await service.requestFolderGrant({ mode: "read_write" }); + expect(created!.shellEnabled).toBe(false); + + const updated = await service.setGrantShellEnabled(created!.grantId, true); + expect(updated!.grantId).toBe(created!.grantId); + expect(updated!.shellEnabled).toBe(true); + // It does not widen file access: commands are a different authority. + expect(updated!.mode).toBe("read_write"); + // And it still leaks no path. + expect(JSON.stringify(updated)).not.toContain("/data/reports"); + + const off = await service.setGrantShellEnabled(created!.grantId, false); + expect(off!.shellEnabled).toBe(false); + }); + + it("setGrantShellEnabled returns null for an unknown id", async () => { + const { service } = makeService( + async () => ({ canceled: true, filePaths: [] }), + tmp, + ); + expect( + await service.setGrantShellEnabled( + "00000000-0000-4000-8000-000000000000", + true, + ), + ).toBeNull(); + }); }); diff --git a/apps/desktop/main/capabilities/service.ts b/apps/desktop/main/capabilities/service.ts index 9606c7fdf..89b6a7637 100644 --- a/apps/desktop/main/capabilities/service.ts +++ b/apps/desktop/main/capabilities/service.ts @@ -111,6 +111,35 @@ export class CapabilityService { return grant === null ? null : toRendererGrant(grant); } + /** + * Turn per-workspace SHELL EXECUTION on or off (PRD-shell-execution §7.3). + * Returns the updated renderer view, or null for an unknown grant. + * + * The service adds nothing to `GrantStore.setShellEnabled` and that is the + * point: this method exists so the renderer has a reachable seam at all. + * `CapabilityService` previously exposed `requestFolderGrant` / `listGrants` / + * `revokeGrant` and no mutation of an existing grant, so a Settings toggle had + * no method to call — which is how "add a boolean" turns into a channel, a + * schema, a handler and a store method. + * + * WHAT IT REFUSES TO DO: it never mints, never re-picks, and never widens + * `mode`. Enabling commands on a folder does not change what may be read or + * written there, and detaching the folder remains the way to take everything + * away at once. + * + * The caller is expected to compare the returned `shellEnabled` against what + * it asked for. A refused enable (the grant is revoked or expired — see the + * store) comes back as the record UNCHANGED rather than as a throw, so the + * honest report is "still off", not "done". + */ + async setGrantShellEnabled( + grantId: string, + enabled: boolean, + ): Promise { + const grant = await this.#store.setShellEnabled(grantId, enabled); + return grant === null ? null : toRendererGrant(grant); + } + // --- broker lifecycle (main-owned) --- startBroker(): Promise { diff --git a/apps/desktop/main/capabilities/types.ts b/apps/desktop/main/capabilities/types.ts index 8d13d3d85..c5fc2caf9 100644 --- a/apps/desktop/main/capabilities/types.ts +++ b/apps/desktop/main/capabilities/types.ts @@ -63,6 +63,29 @@ export interface Grant { /** Epoch millis. Expired grants are treated as revoked for authority checks. */ readonly expiresAt?: number; readonly mode: GrantMode; + /** + * Per-workspace SHELL EXECUTION enablement (PRD-shell-execution §7.3). + * + * OFF BY DEFAULT, and `false` is the ONLY value `GrantStore.create` ever + * mints. It is a SEPARATE flag rather than a fourth `GrantMode` because the + * three modes describe FILE ACCESS and are ordered (`MODE_RANK`); a fourth + * member would imply `read_write < execute`, which is not true — running a + * command is not more file access, it is a different kind of authority + * entirely, and it is not a superset of anything (see PRD §7.3). + * + * REQUIRED, not optional, and that is the point. "Absent" is the common case + * — every grant minted before this field existed lacks it — and an optional + * boolean makes `undefined` a third state that each reader has to remember to + * fold to `false`. Absence is folded ONCE, at the decode seam + * (`coerceGrant`), so nothing downstream can be handed `undefined` and get it + * wrong. The compiler then forces every construction site to say which it is. + * + * The one way this becomes `true` is `GrantStore.setShellEnabled`, reached + * only from the Settings toggle over its own IPC channel. No runtime call, no + * broker verb, and no attach-time parameter can set it — see + * `CreateGrantInput`. + */ + readonly shellEnabled: boolean; /** Sanitized display label (folder basename or a renderer-supplied hint). */ readonly label: string; readonly status: GrantStatus; @@ -81,6 +104,16 @@ export interface RendererGrant { readonly mode: GrantMode; readonly label: string; readonly status: GrantStatus; + /** + * Whether this workspace may run commands (§7.3). Carried to the renderer + * because the Settings toggle has to render the CURRENT state of a security + * flag; a toggle that shows what the renderer last asked for rather than what + * main actually holds is the "pill that outlives its grant" defect wearing a + * different hat. It is a boolean, not a path, so it cannot become a path + * oracle — `RendererGrant` stays path-free, and `type PathFree` still + * guards that. + */ + readonly shellEnabled: boolean; } export function toRendererGrant(grant: Grant): RendererGrant { @@ -89,6 +122,11 @@ export function toRendererGrant(grant: Grant): RendererGrant { mode: grant.mode, label: grant.label, status: grant.status, + // A grant that authorizes nothing advertises nothing. `status` here is the + // EFFECTIVE one (`GrantStore.list` reports an expired grant as revoked), so + // an expired workspace's toggle reads off rather than showing a live-looking + // "commands allowed" over authority that has lapsed. + shellEnabled: grant.status === "active" && grant.shellEnabled, }; } @@ -135,6 +173,23 @@ export interface BrokerGrant { * meant: no rule, so that folder keeps asking. */ readonly root?: string; + /** + * Whether this workspace may run commands (§7.3) — present ONLY while the + * grant is active, for the same reason `root` is. + * + * This is THE READ PATH. PRD §7.1 lists four independent prerequisites for + * `run_command` to exist at all, and prerequisite 3 is this field, read off + * the active-grant snapshot the worker already pins at run start. No new IPC + * verb, no broker `ADVERTISED_METHODS` entry, and specifically no execution + * verb over the broker: the broker reports what the user decided, it does not + * run anything. + * + * ABSENT MEANS OFF. An older Electron main that does not send it decodes on + * the Python side to `shell_enabled=False` (`BrokerGrant` there defaults it, + * and the model is `extra="ignore"`), so a version skew degrades to "this + * workspace cannot run commands" — never to "it can". + */ + readonly shellEnabled?: boolean; } /** @@ -149,8 +204,17 @@ export interface BrokerGrant { * `toBrokerGrant` therefore did not just widen a contract — it broke the * channel, silently and only outside the tests, because that assertion is what * the real wire meets. + * + * `shellEnabled` is excluded for exactly that reason and is not an oversight. + * `_assert_host_session_wire_is_private` allow-lists SIX grant keys by name + * (`grantId`/`grant_id`, `mount`, `mode`, `label`, `status`) and raises + * `BrokerProtocolError` on any seventh. Projecting the shell flag here would + * fail every live host session closed while every test that builds the payload + * by hand stayed green. The flag has no business here anyway: this bootstrap + * carries WRITE authority for staged effects, and shell enablement is read off + * `/v1/grants/snapshot` (see `BrokerGrant.shellEnabled`). */ -export type HostSessionGrant = Omit; +export type HostSessionGrant = Omit; /** Path-free projection of a `GrantSnapshot` for the broker audience. */ export interface BrokerGrantSnapshot { @@ -182,7 +246,7 @@ export function toBrokerGrant(grant: Grant, mount: string): BrokerGrant { // already the effective one — `GrantStore.list` reports an expired grant as // revoked — so expiry is covered by the same test. return grant.status === "active" - ? { ...projected, root: grant.root } + ? { ...projected, root: grant.root, shellEnabled: grant.shellEnabled } : projected; } diff --git a/apps/desktop/main/capabilities/workspace-authority.test.ts b/apps/desktop/main/capabilities/workspace-authority.test.ts index 7c2f0fe86..18d3d478a 100644 --- a/apps/desktop/main/capabilities/workspace-authority.test.ts +++ b/apps/desktop/main/capabilities/workspace-authority.test.ts @@ -27,6 +27,9 @@ function grant(overrides: Partial = {}): Grant { expiresAt: Number.MAX_SAFE_INTEGER, mode: "read_write", label: "project", + // Default OFF, stated explicitly: a fixture that omitted it would be the + // one place absence is allowed to mean something other than "no commands". + shellEnabled: false, status: "active", createdAt: 1, updatedAt: 1, diff --git a/apps/desktop/main/index.ts b/apps/desktop/main/index.ts index 2d6bd6898..b734fbeea 100644 --- a/apps/desktop/main/index.ts +++ b/apps/desktop/main/index.ts @@ -1014,6 +1014,12 @@ function wireTransportAndIpc( capabilityService!.requestFolderGrant(params), listGrants: () => capabilityService!.listGrants(), revokeGrant: (grantId) => capabilityService!.revokeGrant(grantId), + // PRD-shell-execution §7.3: the Settings toggle's only route to + // main. Wired on the SAME condition as the other three, so a build + // with the capability subsystem off has no shell-enablement channel + // either — there is nothing to toggle when there are no grants. + setGrantShellEnabled: (grantId, enabled) => + capabilityService!.setGrantShellEnabled(grantId, enabled), }, workspaceApproval, // AC9 connector channels. Wired only against a real facade; returns only diff --git a/apps/desktop/main/ipc/handlers.test.ts b/apps/desktop/main/ipc/handlers.test.ts index eaa9aa424..cf98797a7 100644 --- a/apps/desktop/main/ipc/handlers.test.ts +++ b/apps/desktop/main/ipc/handlers.test.ts @@ -701,6 +701,7 @@ describe("capability.* channels", () => { mode: "read_write" as const, label: "proj", status: "active" as const, + shellEnabled: false, })), listGrants: vi.fn(async () => [ { @@ -708,6 +709,7 @@ describe("capability.* channels", () => { mode: "read_only" as const, label: "proj", status: "active" as const, + shellEnabled: false, }, ]), revokeGrant: vi.fn(async () => ({ @@ -715,7 +717,17 @@ describe("capability.* channels", () => { mode: "read_only" as const, label: "proj", status: "revoked" as const, + shellEnabled: false, })), + setGrantShellEnabled: vi.fn( + async (grantId: string, enabled: boolean) => ({ + grantId, + mode: "read_write" as const, + label: "proj", + status: "active" as const, + shellEnabled: enabled, + }), + ), }; const teardown = registerIpcHandlers({ ipcMain: ipcMain as unknown as Parameters< @@ -727,11 +739,57 @@ describe("capability.* channels", () => { return { ipcMain, capability, teardown }; } - it("registers the three capability channels only when a handler is provided", () => { + it("registers the four capability channels only when a handler is provided", () => { const { ipcMain } = setupCapability(); expect(ipcMain.has(CAPABILITY_CHANNELS.requestFolderGrant)).toBe(true); expect(ipcMain.has(CAPABILITY_CHANNELS.listGrants)).toBe(true); expect(ipcMain.has(CAPABILITY_CHANNELS.revokeGrant)).toBe(true); + expect(ipcMain.has(CAPABILITY_CHANNELS.setGrantShellEnabled)).toBe(true); + }); + + // PRD-shell-execution §7.3. The channel exists because there was no other way + // to reach main: `CapabilityService` had no mutation of an existing grant at + // all, so a Settings toggle had nothing to call. + it("set-grant-shell-enabled forwards the exact requested value", async () => { + const { ipcMain, capability } = setupCapability(); + const result = await ipcMain.invoke( + CAPABILITY_CHANNELS.setGrantShellEnabled, + 1, + { + grantId: "22222222-2222-4222-8222-222222222222", + enabled: true, + }, + ); + expect(capability.setGrantShellEnabled).toHaveBeenCalledWith( + "22222222-2222-4222-8222-222222222222", + true, + ); + expect(result).toEqual({ + grantId: "22222222-2222-4222-8222-222222222222", + mode: "read_write", + label: "proj", + status: "active", + shellEnabled: true, + }); + }); + + // `enabled` is required and `.strict()` — a renderer cannot mean "flip it", + // and cannot smuggle a mode/path/command alongside the decision. + it("set-grant-shell-enabled rejects a payload that omits or exceeds the contract", async () => { + const { ipcMain, capability } = setupCapability(); + await expect( + ipcMain.invoke(CAPABILITY_CHANNELS.setGrantShellEnabled, 1, { + grantId: "22222222-2222-4222-8222-222222222222", + }), + ).rejects.toThrow(); + await expect( + ipcMain.invoke(CAPABILITY_CHANNELS.setGrantShellEnabled, 1, { + grantId: "22222222-2222-4222-8222-222222222222", + enabled: true, + command: "rm -rf ~", + }), + ).rejects.toThrow(); + expect(capability.setGrantShellEnabled).not.toHaveBeenCalled(); }); it("does not register capability channels when no handler is provided", () => { @@ -759,6 +817,9 @@ describe("capability.* channels", () => { }); expect(res).toEqual({ grantId: "22222222-2222-4222-8222-222222222222", + // A freshly minted grant is never command-capable (§7.3): `create` mints + // `shellEnabled: false` and `CreateGrantInput` has no way to say otherwise. + shellEnabled: false, mode: "read_write", label: "proj", status: "active", diff --git a/apps/desktop/main/ipc/handlers.ts b/apps/desktop/main/ipc/handlers.ts index b60b619f8..0569090e8 100644 --- a/apps/desktop/main/ipc/handlers.ts +++ b/apps/desktop/main/ipc/handlers.ts @@ -17,6 +17,7 @@ import { RendererGrantSchema, RequestFolderGrantParamsSchema, RevokeGrantParamsSchema, + SetGrantShellEnabledParamsSchema, WorkspaceApprovalHostDecisionRequestSchema, WorkspaceApprovalHostDecisionResultSchema, type RequestFolderGrantParams, @@ -90,6 +91,11 @@ export interface CapabilityHandlers { ): Promise; listGrants(): Promise; revokeGrant(grantId: string): Promise; + /** Per-workspace shell enablement (PRD-shell-execution §7.3). */ + setGrantShellEnabled( + grantId: string, + enabled: boolean, + ): Promise; } // Connector connect handlers (AC9). The renderer asks main to fetch the @@ -459,6 +465,29 @@ export function registerIpcHandlers(deps: RegisterHandlersDeps): () => void { return grant === null ? null : toSafeRendererGrant(grant); }, ); + + // Per-workspace shell enablement (PRD-shell-execution §7.3). The fifth + // capability channel, and the only one that mutates an existing grant. + // + // Note what does NOT happen here: nothing is executed, no host path is + // accepted or returned, and the reply is the SAME strict-parsed + // `RendererGrant` every other grant channel answers with — so the renderer + // learns the state main actually holds rather than the state it asked for. + ipcMain.handle( + CAPABILITY_CHANNELS.setGrantShellEnabled, + async (_event, raw: unknown) => { + const params = parseOrThrow( + CAPABILITY_CHANNELS.setGrantShellEnabled, + SetGrantShellEnabledParamsSchema, + raw, + ); + const grant = await capability.setGrantShellEnabled( + params.grantId, + params.enabled, + ); + return grant === null ? null : toSafeRendererGrant(grant); + }, + ); } const workspaceApproval = deps.workspaceApproval; @@ -569,6 +598,7 @@ export function registerIpcHandlers(deps: RegisterHandlersDeps): () => void { CAPABILITY_CHANNELS.requestFolderGrant, CAPABILITY_CHANNELS.listGrants, CAPABILITY_CHANNELS.revokeGrant, + CAPABILITY_CHANNELS.setGrantShellEnabled, ); } if (workspaceApproval) { diff --git a/apps/desktop/preload/bridge.test.ts b/apps/desktop/preload/bridge.test.ts index bc58db21c..6bda47d30 100644 --- a/apps/desktop/preload/bridge.test.ts +++ b/apps/desktop/preload/bridge.test.ts @@ -140,6 +140,15 @@ describe("preload bridge capability-channel allowlist", () => { await bridge.ipc.invoke(CAPABILITY_CHANNELS.revokeGrant, { grantId: "x", }); + // PRD-shell-execution §7.3. The fifth capability channel, and the only route + // the Settings toggle has to main — a channel main registers but preload + // will not forward is a control that silently does nothing. It rides + // `isCapabilityChannel`, so it is allow-listed by being in + // `CAPABILITY_CHANNELS` at all; this asserts that derivation still holds. + await bridge.ipc.invoke(CAPABILITY_CHANNELS.setGrantShellEnabled, { + grantId: "x", + enabled: true, + }); await bridge.ipc.invoke(CAPABILITY_CHANNELS.decideWorkspaceApproval, { snapshot: { runId: "run_c3_001", @@ -167,7 +176,11 @@ describe("preload bridge capability-channel allowlist", () => { decision: "approve", }, ); - expect(electron.ipcRenderer.invoke).toHaveBeenCalledTimes(4); + expect(electron.ipcRenderer.invoke).toHaveBeenCalledWith( + CAPABILITY_CHANNELS.setGrantShellEnabled, + { grantId: "x", enabled: true }, + ); + expect(electron.ipcRenderer.invoke).toHaveBeenCalledTimes(5); }); it("still rejects a channel that is in neither allowlist", async () => { diff --git a/apps/desktop/renderer/FirstRunGate.test.tsx b/apps/desktop/renderer/FirstRunGate.test.tsx index d839df51a..2bdc402aa 100644 --- a/apps/desktop/renderer/FirstRunGate.test.tsx +++ b/apps/desktop/renderer/FirstRunGate.test.tsx @@ -832,6 +832,9 @@ describe("FirstRunSurfaceMount — P8 §7 modelBlocked threading", () => { label: "kaleidoscope", mode: "read_only", status: "active", + // Exact key set — see `parseGrant`. A first-run grant is never + // command-capable; enabling is a separate, later, explicit act. + shellEnabled: false, }, ]), }); diff --git a/apps/desktop/renderer/SettingsMount.tsx b/apps/desktop/renderer/SettingsMount.tsx index 339b27fff..9cf1187a3 100644 --- a/apps/desktop/renderer/SettingsMount.tsx +++ b/apps/desktop/renderer/SettingsMount.tsx @@ -38,6 +38,7 @@ import { ModelsPage, ModelBehaviorPage, useConnectorSuggestions, + useWorkspaceFolderGrants, NotificationsPage, PrivacyPage, ProfilePage, @@ -93,6 +94,7 @@ import { import type { LinkWalletOutcome } from "@0x-copilot/chat-surface"; import { publishWorkspaceDefaults } from "./workspaceDefaultsStore"; +import { bridgeWorkspaceGrantPort } from "./workspaceGrantPort"; import { SECURE_STORAGE_CHANNELS } from "../main/services/secure-storage-channels"; interface SecureStorageStatus { @@ -933,6 +935,17 @@ export function SettingsMount({ recheckLocalModels(); }, [recheckLocalModels]); + // Per-workspace shell execution (PRD-shell-execution §7.3, §14.4). The SAME + // hook the composer's folder bar uses, over the SAME memoized port, so + // Settings and the composer can never disagree about which folders are + // attached — a second source of that list is how a toggle ends up governing a + // grant the user already detached somewhere else. + // + // `bridgeWorkspaceGrantPort()` returns undefined when there is no bridge (the + // browser-hosted renderer used in tests and dev), and the hook then yields + // `setShellEnabled: null`, which makes the section render nothing at all. + const workspaceGrants = useWorkspaceFolderGrants(bridgeWorkspaceGrantPort()); + const renderSection = ( slug: SettingsSectionSlug, controller: SettingsSurfaceController, @@ -1122,6 +1135,22 @@ export function SettingsMount({ // above it, so the two never disagree about what is muted. mutedConnectors={connectorSuggestions.muted} onUnmuteConnector={connectorSuggestions.unmute} + // PRD-shell-execution §7.3/§14.4 — the "Run commands" list, under + // the approval policy. Applies IMMEDIATELY through its own host + // callback rather than riding this page's SaveBar: a security + // permission must not sit unsaved behind a Save the user may never + // press, in either direction. + workspaceShellAccess={{ + grants: workspaceGrants.grants, + onSetShellEnabled: + workspaceGrants.setShellEnabled === null + ? null + : (grantId, enabled) => { + void workspaceGrants.setShellEnabled?.(grantId, enabled); + }, + error: workspaceGrants.error, + busy: workspaceGrants.busy, + }} cloudModels={mbCloudModels} onChange={(patch) => { if (patch.defaultModel !== undefined) { diff --git a/apps/desktop/renderer/composer/RunEmptyComposer.test.tsx b/apps/desktop/renderer/composer/RunEmptyComposer.test.tsx index aadd6c513..2bd490df8 100644 --- a/apps/desktop/renderer/composer/RunEmptyComposer.test.tsx +++ b/apps/desktop/renderer/composer/RunEmptyComposer.test.tsx @@ -374,6 +374,10 @@ describe("RunEmptyComposer", () => { label: "kaleidoscope", mode: "read_only", status: "active", + // `parseGrant` checks the renderer projection's key set EXACTLY, so a + // fixture missing this is rejected and the bar renders nothing — the + // same guard that catches a host root appearing in this payload. + shellEnabled: false, }, ], }); diff --git a/apps/desktop/renderer/destinationBinders.workspaceGrant.test.tsx b/apps/desktop/renderer/destinationBinders.workspaceGrant.test.tsx index 51402d5c9..53442b82a 100644 --- a/apps/desktop/renderer/destinationBinders.workspaceGrant.test.tsx +++ b/apps/desktop/renderer/destinationBinders.workspaceGrant.test.tsx @@ -240,6 +240,9 @@ describe("desktop Run cockpit — the mid-run folder ask", () => { mode: "read_only", label: "Downloads", status: "active", + // Exact key set — see `parseGrant`. A freshly minted grant is never + // command-capable (`CreateGrantInput` has no way to say otherwise). + shellEnabled: false, }, [CAPABILITY_CHANNELS.listGrants]: [], }); diff --git a/apps/desktop/renderer/workspaceGrantPort.test.ts b/apps/desktop/renderer/workspaceGrantPort.test.ts index 89e479119..f977725c7 100644 --- a/apps/desktop/renderer/workspaceGrantPort.test.ts +++ b/apps/desktop/renderer/workspaceGrantPort.test.ts @@ -23,6 +23,7 @@ function rendererGrant( mode: string; label: string; status: string; + shellEnabled: unknown; }> = {}, ): Record { return { @@ -30,6 +31,10 @@ function rendererGrant( mode: "read_only", label: "Downloads", status: "active", + // FIVE keys since PRD-shell-execution §7.3. `parseGrant` checks the key set + // exactly, on BOTH ends, so a fixture that omits this is rejected — which is + // the same guard that catches a host root appearing in this payload. + shellEnabled: false, ...overrides, }; } @@ -84,6 +89,10 @@ describe("requestGrant", () => { mount: "11111111-1111-4111-8111-111111111111", label: "Downloads", mode: "read_only", + // A newly minted grant is never command-capable (PRD-shell-execution + // §7.3). `CreateGrantInput` has no `shellEnabled`, so there is no + // parameter the attach flow could pass to make this `true`. + shellEnabled: false, }, }); }); @@ -248,6 +257,7 @@ describe("listGrants", () => { mount: "g-active", label: "Downloads", mode: "read_only", + shellEnabled: false, }, ]); }); diff --git a/apps/desktop/renderer/workspaceGrantPort.ts b/apps/desktop/renderer/workspaceGrantPort.ts index 068451cfb..ad3254188 100644 --- a/apps/desktop/renderer/workspaceGrantPort.ts +++ b/apps/desktop/renderer/workspaceGrantPort.ts @@ -31,6 +31,7 @@ import type { WorkspaceGrantPort, WorkspaceGrantRequestInput, WorkspaceRevokeOutcome, + WorkspaceShellAccessOutcome, } from "@0x-copilot/chat-surface"; import { CAPABILITY_CHANNELS } from "../main/capabilities/channels"; @@ -186,6 +187,48 @@ export function createDesktopWorkspaceGrantPort( }; } }, + + async setShellEnabled( + grantId: string, + enabled: boolean, + ): Promise { + try { + const raw = await bridge.ipc.invoke( + CAPABILITY_CHANNELS.setGrantShellEnabled, + { grantId, enabled }, + ); + // Main answers null for an id it does not hold. Unlike revoke, this is + // NOT the end state the caller asked for — "turn commands on for a + // folder that no longer exists" did not happen — so it is a failure and + // is reported as one. + if (raw === null) { + return { + status: "failed", + message: "That folder is no longer shared with the agent.", + }; + } + const parsed = parseGrant(raw); + if (parsed === null) { + return { + status: "failed", + message: "Couldn't confirm that folder's command permission.", + }; + } + // REPORT WHAT MAIN HOLDS, not what we asked for. Main refuses to enable + // commands on a revoked or expired grant and answers with the record + // unchanged; echoing `enabled` here would turn that refusal into a + // checked box over authority nothing recorded. + return { status: "ok", applied: parsed.grant.shellEnabled }; + } catch (cause) { + return { + status: "failed", + message: messageOf( + cause, + "Couldn't change that folder's command permission.", + ), + }; + } + }, }); } @@ -240,22 +283,32 @@ function parseGrant( ): { readonly grant: WorkspaceGrant; readonly status: string } | null { if (!isRecord(value)) return null; const keys = Object.keys(value).sort(); + // FIVE keys since PRD-shell-execution §7.3. This check is exact on BOTH ends + // — a missing key and an extra key each fail — which is why adding + // `shellEnabled` to `RendererGrant` without editing this line would not have + // been a cosmetic miss: every row would have failed to parse, `listGrants` + // would have thrown "Couldn't read your shared folders", and the app would + // have looked like it had lost the grant subsystem rather than like it had + // gained a field. Sorted order, so `shellEnabled` sits between `mode` and + // `status`. if ( - keys.length !== 4 || + keys.length !== 5 || keys[0] !== "grantId" || keys[1] !== "label" || keys[2] !== "mode" || - keys[3] !== "status" + keys[3] !== "shellEnabled" || + keys[4] !== "status" ) { return null; } - const { grantId, label, mode, status } = value; + const { grantId, label, mode, shellEnabled, status } = value; if ( typeof grantId !== "string" || grantId === "" || typeof label !== "string" || typeof status !== "string" || (status !== "active" && status !== "revoked") || + typeof shellEnabled !== "boolean" || typeof mode !== "string" || !GRANT_MODES.includes(mode as WorkspaceGrantMode) ) { @@ -266,6 +319,11 @@ function parseGrant( grantId, label, mode: mode as WorkspaceGrantMode, + // A revoked row is command-incapable no matter what it stored. Main + // already projects it that way (`toRendererGrant`); asserting it again + // here means neither side alone can make a retired workspace look + // runnable. + shellEnabled: shellEnabled && status === "active", // `mount` is the broker's opaque per-boot handle, and the renderer // projection does not carry one (`RendererGrant` is grantId/mode/label/ // status, and only the broker holds the salt that derives a mount). The diff --git a/packages/chat-surface/src/composer/AssistantComposer.folderGrants.test.tsx b/packages/chat-surface/src/composer/AssistantComposer.folderGrants.test.tsx index 2c3d7337f..394ab1339 100644 --- a/packages/chat-surface/src/composer/AssistantComposer.folderGrants.test.tsx +++ b/packages/chat-surface/src/composer/AssistantComposer.folderGrants.test.tsx @@ -62,6 +62,7 @@ const DOWNLOADS: WorkspaceGrant = { mount: "m_9f2c", label: "Downloads", mode: "read_only", + shellEnabled: false, }; const KALEIDOSCOPE: WorkspaceGrant = { @@ -69,6 +70,7 @@ const KALEIDOSCOPE: WorkspaceGrant = { mount: "m_41ab", label: "kaleidoscope", mode: "read_only", + shellEnabled: false, }; const NOTES: WorkspaceGrant = { @@ -76,6 +78,7 @@ const NOTES: WorkspaceGrant = { mount: "m_77de", label: "notes", mode: "read_only", + shellEnabled: false, }; function makeGrantPort( diff --git a/packages/chat-surface/src/composer/AssistantComposer.projectFiling.test.tsx b/packages/chat-surface/src/composer/AssistantComposer.projectFiling.test.tsx index 0fa512111..22836bacd 100644 --- a/packages/chat-surface/src/composer/AssistantComposer.projectFiling.test.tsx +++ b/packages/chat-surface/src/composer/AssistantComposer.projectFiling.test.tsx @@ -65,6 +65,7 @@ const DOWNLOADS: WorkspaceGrant = { mount: "m_9f2c", label: "Downloads", mode: "read_only", + shellEnabled: false, }; /** A host that HAS the folder capability — what gates the bar above the frame. */ diff --git a/packages/chat-surface/src/composer/index.ts b/packages/chat-surface/src/composer/index.ts index 4b71054ce..a6234bd5e 100644 --- a/packages/chat-surface/src/composer/index.ts +++ b/packages/chat-surface/src/composer/index.ts @@ -90,6 +90,7 @@ export { export { mostRecentFirst, useWorkspaceFolderGrants, + WORKSPACE_SHELL_ACCESS_NOTICE, type WorkspaceFolderGrantsState, } from "./useWorkspaceFolderGrants"; // === end Workspace folder grants === diff --git a/packages/chat-surface/src/composer/useWorkspaceFolderGrants.shellEnabled.test.tsx b/packages/chat-surface/src/composer/useWorkspaceFolderGrants.shellEnabled.test.tsx new file mode 100644 index 000000000..9dbb7e071 --- /dev/null +++ b/packages/chat-surface/src/composer/useWorkspaceFolderGrants.shellEnabled.test.tsx @@ -0,0 +1,148 @@ +// PRD-shell-execution §7.3 — the hook half of the per-workspace command flag. +// +// The hook is the seam between "the host can do this" and "the surface draws a +// control for it", and it is also the last place that can turn a host's refusal +// into a visible sentence. Both are asserted here, because the failure mode of +// getting either wrong is the same: a toggle that looks on over a permission +// nothing recorded. + +import { act, renderHook, waitFor } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; + +import type { + WorkspaceGrant, + WorkspaceGrantPort, + WorkspaceShellAccessOutcome, +} from "../ports/WorkspaceGrantPort"; + +import { useWorkspaceFolderGrants } from "./useWorkspaceFolderGrants"; + +function grant(shellEnabled = false): WorkspaceGrant { + return { + grantId: "g_atlas", + mount: "m_atlas", + label: "atlas", + mode: "read_write", + shellEnabled, + }; +} + +/** A port WITHOUT `setShellEnabled` — the shape web and an old desktop have. */ +function portWithoutShell(): WorkspaceGrantPort { + return { + requestGrant: vi.fn(async () => ({ + status: "granted" as const, + grant: grant(), + })), + listGrants: vi.fn(async () => [grant()]), + revokeGrant: vi.fn(async () => ({ status: "revoked" as const })), + }; +} + +function portWithShell( + outcome: WorkspaceShellAccessOutcome | (() => Promise), +): WorkspaceGrantPort & { setShellEnabled: ReturnType } { + const setShellEnabled = vi.fn( + typeof outcome === "function" ? outcome : async () => outcome, + ); + return { ...portWithoutShell(), setShellEnabled }; +} + +describe("useWorkspaceFolderGrants — shell enablement", () => { + it("yields NULL when the host has no setShellEnabled", async () => { + // Null, not a no-op function. A no-op would let a surface render a toggle + // that silently does nothing; null forces the caller to answer "can this + // host do it at all?" before it draws the control. + const { result } = renderHook(() => + useWorkspaceFolderGrants(portWithoutShell()), + ); + await waitFor(() => expect(result.current.grants).toHaveLength(1)); + expect(result.current.setShellEnabled).toBeNull(); + }); + + it("yields null for a null port too", async () => { + const { result } = renderHook(() => useWorkspaceFolderGrants(null)); + expect(result.current.setShellEnabled).toBeNull(); + }); + + it("forwards the exact grantId and value, then re-reads the list", async () => { + const port = portWithShell({ status: "ok", applied: true }); + const { result } = renderHook(() => useWorkspaceFolderGrants(port)); + await waitFor(() => expect(result.current.setShellEnabled).not.toBeNull()); + + await act(async () => { + await result.current.setShellEnabled?.("g_atlas", true); + }); + + expect(port.setShellEnabled).toHaveBeenCalledExactlyOnceWith( + "g_atlas", + true, + ); + expect(result.current.error).toBeNull(); + // Re-read, so the rendered state is the host's and not our optimism. + expect(port.listGrants).toHaveBeenCalledTimes(2); + }); + + it("surfaces a host failure as a sentence", async () => { + const port = portWithShell({ + status: "failed", + message: "That folder is no longer shared with the agent.", + }); + const { result } = renderHook(() => useWorkspaceFolderGrants(port)); + await waitFor(() => expect(result.current.setShellEnabled).not.toBeNull()); + + await act(async () => { + await result.current.setShellEnabled?.("g_atlas", true); + }); + + expect(result.current.error).toBe( + "That folder is no longer shared with the agent.", + ); + }); + + it("says so when the host applied something OTHER than what was asked", async () => { + // The defect this exists to prevent: main refuses to enable commands on a + // revoked or expired grant and answers with the record UNCHANGED, which is + // `status: "ok"`. Reading only the status reports that refusal as a success, + // and the toggle snaps back with no explanation — a UI glitch the user + // would reasonably read as "it worked, the render is just slow". + const port = portWithShell({ status: "ok", applied: false }); + const { result } = renderHook(() => useWorkspaceFolderGrants(port)); + await waitFor(() => expect(result.current.setShellEnabled).not.toBeNull()); + + await act(async () => { + await result.current.setShellEnabled?.("g_atlas", true); + }); + + expect(result.current.error).toMatch(/Couldn't allow commands/i); + }); + + it("an agreeing outcome sets no error", async () => { + // The mirror of the test above, so it cannot pass by the error being set + // unconditionally. + const port = portWithShell({ status: "ok", applied: false }); + const { result } = renderHook(() => useWorkspaceFolderGrants(port)); + await waitFor(() => expect(result.current.setShellEnabled).not.toBeNull()); + + await act(async () => { + await result.current.setShellEnabled?.("g_atlas", false); + }); + + expect(result.current.error).toBeNull(); + }); + + it("a thrown port call leaves an error and clears busy", async () => { + const port = portWithShell(async () => { + throw new Error("bridge is gone"); + }); + const { result } = renderHook(() => useWorkspaceFolderGrants(port)); + await waitFor(() => expect(result.current.setShellEnabled).not.toBeNull()); + + await act(async () => { + await result.current.setShellEnabled?.("g_atlas", true); + }); + + expect(result.current.error).toBe("bridge is gone"); + expect(result.current.busy).toBe(false); + }); +}); diff --git a/packages/chat-surface/src/composer/useWorkspaceFolderGrants.test.ts b/packages/chat-surface/src/composer/useWorkspaceFolderGrants.test.ts index ad7bb3ebf..35935198c 100644 --- a/packages/chat-surface/src/composer/useWorkspaceFolderGrants.test.ts +++ b/packages/chat-surface/src/composer/useWorkspaceFolderGrants.test.ts @@ -13,7 +13,13 @@ import type { WorkspaceGrant } from "../ports/WorkspaceGrantPort"; import { mostRecentFirst } from "./useWorkspaceFolderGrants"; function grant(id: string, label: string): WorkspaceGrant { - return { grantId: id, mount: `m_${id}`, label, mode: "read_only" }; + return { + grantId: id, + mount: `m_${id}`, + label, + mode: "read_only", + shellEnabled: false, + }; } const NOTES = grant("g1", "notes"); diff --git a/packages/chat-surface/src/composer/useWorkspaceFolderGrants.ts b/packages/chat-surface/src/composer/useWorkspaceFolderGrants.ts index b08b070a5..e60f75a0c 100644 --- a/packages/chat-surface/src/composer/useWorkspaceFolderGrants.ts +++ b/packages/chat-surface/src/composer/useWorkspaceFolderGrants.ts @@ -26,6 +26,19 @@ import type { WorkspaceGrantRequestInput, } from "../ports/WorkspaceGrantPort"; +/** + * Copy the shell-enablement control MUST state, verbatim (PRD §11.5, §14.4). + * + * It is exported so the surface and its test read the same bytes. There is no + * OS sandbox in v1, and the sentence is the product's only honest account of + * that: a toggle labelled "run commands in this folder" reads as a confinement + * claim the cwd binding does not make — the binding decides where a command + * STARTS, not where it can reach. + */ +export const WORKSPACE_SHELL_ACCESS_NOTICE = + "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."; + export interface WorkspaceFolderGrantsState { /** Active grants, newest read of the broker's own list, IN THE BROKER'S ORDER. */ readonly grants: readonly WorkspaceGrant[]; @@ -53,6 +66,19 @@ export interface WorkspaceFolderGrantsState { */ readonly requestGrant: (input?: WorkspaceGrantRequestInput) => Promise; readonly revokeGrant: (grantId: string) => Promise; + /** + * Turn command execution on or off for ONE workspace (PRD §7.3), or `null` + * when the host has not wired `WorkspaceGrantPort.setShellEnabled`. + * + * NULL, not a no-op function, on purpose. A no-op would let a surface render + * a toggle that silently does nothing — which is the same lie as a pill that + * outlives its grant, in the one place where the lie is "the agent may run + * commands here". Null makes "can this host do it at all?" a question the + * caller has to answer before it draws the control. + */ + readonly setShellEnabled: + | ((grantId: string, enabled: boolean) => Promise) + | null; /** Re-read the active set (mount, and after any change). */ readonly refresh: () => Promise; /** Dismiss the failure line without retrying. */ @@ -196,6 +222,48 @@ export function useWorkspaceFolderGrants( [port, refresh], ); + // Bound to the port's IDENTITY, not to the method, so a host that supplies no + // implementation yields `null` and the surface draws no toggle at all. + const supportsShell = typeof port?.setShellEnabled === "function"; + const applyShellEnabled = useCallback( + async (grantId: string, enabled: boolean): Promise => { + if (!port || typeof port.setShellEnabled !== "function") return; + setBusy(true); + setError(null); + try { + const outcome = await port.setShellEnabled(grantId, enabled); + if (outcome.status === "failed") { + if (alive.current) setError(outcome.message); + } else if (outcome.applied !== enabled && alive.current) { + // The host answered with what it actually holds and it is not what was + // asked for — the grant lapsed, or main declined. Say so; a toggle + // that snapped back with no sentence reads as a UI glitch, and the + // user would reasonably assume the agent can run commands. + setError( + enabled + ? "Couldn't allow commands in that folder. Re-attach it and try again." + : "Couldn't stop commands in that folder.", + ); + } + // Re-read either way. Same rule as revoke: after a failure we do not + // know what the host's set is, and guessing is what we are avoiding. + await refresh(); + } catch (cause) { + if (alive.current) { + setError( + messageOf( + cause, + "Couldn't change that folder's command permission.", + ), + ); + } + } finally { + if (alive.current) setBusy(false); + } + }, + [port, refresh], + ); + const clearError = useCallback((): void => setError(null), []); // A remembered id outlives its grant when the folder is revoked (here or in @@ -214,6 +282,7 @@ export function useWorkspaceFolderGrants( error, requestGrant, revokeGrant, + setShellEnabled: supportsShell ? applyShellEnabled : null, refresh, clearError, }; diff --git a/packages/chat-surface/src/destinations/run/RunDestination.tsx b/packages/chat-surface/src/destinations/run/RunDestination.tsx index f03ff93f8..c40154b42 100644 --- a/packages/chat-surface/src/destinations/run/RunDestination.tsx +++ b/packages/chat-surface/src/destinations/run/RunDestination.tsx @@ -4700,6 +4700,14 @@ export function RunDestination(props: RunDestinationProps): ReactElement { } connector={gate.connector} params={writeGateApproval(gate.gateId)?.params ?? []} + // The shaped evidence and the command, off the SAME projected + // approval the params come from. Omitting them was what made + // the canvas twin of a command ask (PRD-shell-execution §14.1) + // a title over an enabled Approve — the payload the decision is + // made ON, absent from the surface making it, while the chat + // row for the same gate showed it in full. + presentation={writeGateApproval(gate.gateId)?.presentation} + commandText={writeGateApproval(gate.gateId)?.command} ledgerId={gate.ledgerId} // Was hardcoded false, so a DESTRUCTIVE gate printed "You can // undo this from the connector if it's wrong." on the canvas diff --git a/packages/chat-surface/src/destinations/run/useWorkspaceGrantCardStates.test.ts b/packages/chat-surface/src/destinations/run/useWorkspaceGrantCardStates.test.ts index 3d6592006..0d5fb413e 100644 --- a/packages/chat-surface/src/destinations/run/useWorkspaceGrantCardStates.test.ts +++ b/packages/chat-surface/src/destinations/run/useWorkspaceGrantCardStates.test.ts @@ -21,6 +21,7 @@ const DOWNLOADS: WorkspaceGrant = { mount: "m_9f2c", label: "Downloads", mode: "read_only", + shellEnabled: false, }; const ASK: WorkspaceGrantRequest = { @@ -57,6 +58,12 @@ describe("useWorkspaceGrantCardStates", () => { await waitFor(() => expect(result.current.states["appr-fs-1"]).toBe("granted"), ); + // EXACTLY these three keys. `WorkspaceGrantRequestInput` deliberately has + // no `shellEnabled` (PRD-shell-execution §7.3): attaching a folder is not + // how command execution gets turned on, and a mid-run ask — authored by the + // model, answered in one dialog about SHARING a folder — is the last place + // that decision should be reachable. `toHaveBeenCalledWith` is exact, so + // this assertion fails if the field is ever smuggled onto the request. expect(port.requestGrant).toHaveBeenCalledWith({ path: "/Users/ada/Downloads", mode: "read_only", diff --git a/packages/chat-surface/src/index.ts b/packages/chat-surface/src/index.ts index 2091b84fe..986736d4c 100644 --- a/packages/chat-surface/src/index.ts +++ b/packages/chat-surface/src/index.ts @@ -2063,15 +2063,31 @@ export type { WorkspaceGrantPort, WorkspaceGrantRequestInput, WorkspaceRevokeOutcome, + WorkspaceShellAccessOutcome, } from "./ports/WorkspaceGrantPort"; export { mostRecentFirst, useWorkspaceFolderGrants, WorkspaceFolderBar, WORKSPACE_FOLDER_BAR_EMPTY_LABEL, + WORKSPACE_SHELL_ACCESS_NOTICE, type WorkspaceFolderBarProps, type WorkspaceFolderGrantsState, } from "./composer"; +// Per-workspace shell enablement (PRD-shell-execution §7.3, §14.4). The +// Settings section that SETS the flag; the flag itself rides `WorkspaceGrant` +// and is read by the runtime off the broker's active-grant snapshot, never off +// this surface. +export { + WorkspaceShellAccess, + WORKSPACE_SHELL_ACCESS_TITLE, + WORKSPACE_SHELL_ACCESS_META, + WORKSPACE_SHELL_ACCESS_EMPTY, + WORKSPACE_SHELL_ACCESS_CONFIRM_LABEL, + WORKSPACE_SHELL_ACCESS_CANCEL_LABEL, + workspaceShellAccessConfirmPrompt, + type WorkspaceShellAccessProps, +} from "./settings"; // `BypassPill` and its vocabulary are hoisted once, in the Filesystem-bypass // block above — PRD-FS-11's shape (mode + scope + host-owned mount) supersedes // the FS-10 selection-only export that stood here. diff --git a/packages/chat-surface/src/ports/WorkspaceGrantPort.test.ts b/packages/chat-surface/src/ports/WorkspaceGrantPort.test.ts index e93b07ec7..c4f60f796 100644 --- a/packages/chat-surface/src/ports/WorkspaceGrantPort.test.ts +++ b/packages/chat-surface/src/ports/WorkspaceGrantPort.test.ts @@ -18,6 +18,7 @@ const DOWNLOADS: WorkspaceGrant = { mount: "m_9f2c", label: "Downloads", mode: "read_only", + shellEnabled: false, }; // The path-free property, enforced at COMPILE time rather than asserted over a @@ -127,6 +128,7 @@ describe("WorkspaceGrantPort contract", () => { mount: "m_31aa", label: "notes", mode: "read_write", + shellEnabled: false, }, ]; const port: WorkspaceGrantPort = { diff --git a/packages/chat-surface/src/ports/WorkspaceGrantPort.ts b/packages/chat-surface/src/ports/WorkspaceGrantPort.ts index 4020a4e99..f74915373 100644 --- a/packages/chat-surface/src/ports/WorkspaceGrantPort.ts +++ b/packages/chat-surface/src/ports/WorkspaceGrantPort.ts @@ -59,6 +59,23 @@ export interface WorkspaceGrant { readonly mount: string; readonly label: string; readonly mode: WorkspaceGrantMode; + /** + * Whether the agent may RUN COMMANDS in this workspace + * (PRD-shell-execution §7.3). Off for every workspace until a human turns it + * on for that one folder, in Settings. + * + * It is NOT a fourth `WorkspaceGrantMode`. The three modes are ordered file + * access and a fourth member would read as "more than read_write", which is + * the wrong shape twice: running a command is a different kind of authority, + * and it is not a superset of write (a read-only folder the user chooses to + * run a build in is a coherent thing to want). + * + * REQUIRED so a host cannot ship a grant list whose command-capability is + * `undefined`. A surface that has to fold `undefined` itself is a surface + * that eventually folds it the wrong way, and the wrong way here reads as + * "commands allowed" on a workspace nobody enabled. + */ + readonly shellEnabled: boolean; } /** @@ -97,6 +114,20 @@ export type WorkspaceRevokeOutcome = | { readonly status: "revoked" } | { readonly status: "failed"; readonly message: string }; +/** + * The result of asking to change one workspace's command permission. + * + * `applied` carries what the host ACTUALLY holds now, not what was asked for. + * The host may honour a request to turn commands OFF on a grant it will not + * honour a request to turn them ON for (a revoked or expired workspace), and a + * toggle that reported its own optimism as success is how a control ends up + * claiming an authority the machine never recorded. A caller compares `applied` + * against what it asked and says so when they differ. + */ +export type WorkspaceShellAccessOutcome = + | { readonly status: "ok"; readonly applied: boolean } + | { readonly status: "failed"; readonly message: string }; + export interface WorkspaceGrantPort { /** * Ask the user for a folder. Resolves `granted` with the minted grant, @@ -116,4 +147,22 @@ export interface WorkspaceGrantPort { /** Take one grant away. The agent loses that mount on its next read. */ revokeGrant(grantId: string): Promise; + + /** + * Turn command execution on or off for ONE workspace (§7.3). + * + * OPTIONAL, and its absence is the whole gate on the control: a host that has + * not wired shell execution supplies no implementation, and every surface in + * this package renders no toggle rather than a toggle that cannot work — the + * same contract that keeps the folder bar off the web. Web will never + * implement it; there is no shell in a browser tab. + * + * It states a VALUE rather than flipping one. "Toggle" makes the outcome + * depend on state the caller does not hold, so a retry after a dropped reply, + * or two clicks racing, can land on "on" when the user's last act was "off". + */ + setShellEnabled?( + grantId: string, + enabled: boolean, + ): Promise; } diff --git a/packages/chat-surface/src/settings/ModelBehaviorPage.test.tsx b/packages/chat-surface/src/settings/ModelBehaviorPage.test.tsx index d8273aeed..3f6223224 100644 --- a/packages/chat-surface/src/settings/ModelBehaviorPage.test.tsx +++ b/packages/chat-surface/src/settings/ModelBehaviorPage.test.tsx @@ -485,3 +485,91 @@ describe("", () => { expect(onRetry).toHaveBeenCalledTimes(1); }); }); + +// PRD-shell-execution §7.3 / §14.4 — the "Run commands" list is MOUNTED under +// the approval policy, and only when the host supplies it. +// +// The section's own behaviour is pinned in `WorkspaceShellAccess.test.tsx`. +// What these two assert is the thing an export alone does not: that the surface +// is reachable from the page a user actually opens, and that it disappears +// cleanly on a host that has no shell. A component that is exported, tested and +// never rendered is the "landed, not wired" defect — and here it would mean the +// only control that can turn command execution on does not exist in the product. +describe(" — Run commands (§14.4)", () => { + it("omits the section entirely when the host supplies nothing", () => { + render( + , + ); + expect( + screen.queryByTestId("workspace-shell-access"), + ).not.toBeInTheDocument(); + }); + + it("omits it again when the host supplies a list but no setter", () => { + // Two independent gates. The host may know the grants (it renders the + // composer's folder bar from the same hook) and still have no way to change + // this flag — web is exactly that shape. + render( + , + ); + expect( + screen.queryByTestId("workspace-shell-access"), + ).not.toBeInTheDocument(); + }); + + it("renders it after the approval policy when the host wires it", () => { + render( + , + ); + const section = screen.getByTestId("workspace-shell-access"); + expect(section).toBeInTheDocument(); + expect( + screen.getByTestId("workspace-shell-toggle-g_atlas"), + ).not.toBeChecked(); + + // ORDER, not just presence. §14.4 places it under the tool-policy surface; + // a control that grants command authority must not sit above the policy + // that governs how each command is approved. + const policy = screen.getByTestId("approval-policy"); + expect( + policy.compareDocumentPosition(section) & + Node.DOCUMENT_POSITION_FOLLOWING, + ).toBeTruthy(); + }); +}); diff --git a/packages/chat-surface/src/settings/ModelBehaviorPage.tsx b/packages/chat-surface/src/settings/ModelBehaviorPage.tsx index 63cb57e60..6ce466e9c 100644 --- a/packages/chat-surface/src/settings/ModelBehaviorPage.tsx +++ b/packages/chat-surface/src/settings/ModelBehaviorPage.tsx @@ -14,6 +14,9 @@ // `ToolUsePolicyPanel` (FR-5.17). // Spend guardrail Monthly API cap ($, across all provider keys) + Pause-runs- // at-cap toggle (FR-5.18). +// Run commands the per-workspace list +// (PRD-shell-execution §7.3, §14.4). Host-supplied and +// OPTIONAL — omitted entirely on a host with no shell. // // SUBSTRATE-AGNOSTIC + CONTROLLED. The page reflects `value` and reports each // edit through `onChange` (a shallow `Partial`; nested @@ -44,6 +47,10 @@ import { ApprovalPolicy, type ApprovalPolicyValue } from "./ApprovalPolicy"; import { Frow, SecTitle, SetCard, SetNote } from "./SettingsChrome"; import type { SettingsSurfaceController } from "./SettingsSurface"; import type { MutedConnector } from "./useConnectorSuggestions"; +import { + WorkspaceShellAccess, + type WorkspaceShellAccessProps, +} from "./WorkspaceShellAccess"; // --------------------------------------------------------------------------- // Vocabulary. @@ -185,6 +192,23 @@ export interface ModelBehaviorPageProps { readonly mutedConnectors?: readonly MutedConnector[]; /** Un-mute a slug; omitted → the rows render inert. */ readonly onUnmuteConnector?: (slug: string) => void; + /** + * Per-workspace SHELL EXECUTION (PRD-shell-execution §7.3, §14.4) — the + * "Run commands" list, rendered directly under the approval policy because + * §14.4 puts it "under the existing tool-policy surface". + * + * PROPS, NOT A PORT. This page stays substrate-agnostic and calls no hook: + * the host owns `useWorkspaceFolderGrants` and hands the live grant list plus + * the setter down. That is also the gate — a host that has not wired shell + * execution (web, which has no shell, and any desktop build with the feature + * off) passes nothing and the section is ABSENT rather than present-and-inert. + * + * NOT part of `value`/`onChange`. Those feed the dirty computation and the + * SaveBar; a security permission must not sit unsaved behind a Save button + * the user might never press, so this applies immediately through its own + * host callback. + */ + readonly workspaceShellAccess?: WorkspaceShellAccessProps | null; } // --------------------------------------------------------------------------- @@ -261,6 +285,7 @@ export function ModelBehaviorPage({ onRetry, mutedConnectors = [], onUnmuteConnector, + workspaceShellAccess = null, }: ModelBehaviorPageProps): ReactElement { const reactId = useId(); const defaultModelId = `${reactId}-default-model`; @@ -494,6 +519,17 @@ export function ModelBehaviorPage({ onChange={(approvalPolicy) => onChange({ approvalPolicy })} /> + {/* Per-workspace shell execution (PRD-shell-execution §7.3, §14.4). + Directly under the approval policy: the policy above says HOW an + action is approved, this says WHERE commands may run at all — and + both are the same question ("what may the agent do") at different + altitudes. Absent when the host supplies nothing, and absent again + when the host supplies no setter (the component returns null), so a + build without shell execution shows no trace of it. */} + {workspaceShellAccess !== null ? ( + + ) : null} + {/* Suggestion appetite. Posture, like the approval policy above it — which app the agent may reach is the Tools destination's job. */} ", () => { + it("renders NOTHING when the host supplies no setter", () => { + // The capability gate. Web has no shell and never will; a desktop build + // with shell execution off has none either. Both must show no trace of the + // feature rather than a row of switches that cannot work — a disabled + // control advertises a capability, and this one would advertise the ability + // to run commands on the machine. + const { container } = render( + , + ); + expect(container).toBeEmptyDOMElement(); + }); + + it("says what to do next when nothing is attached", () => { + render(); + expect( + screen.getByTestId("workspace-shell-access-empty"), + ).toHaveTextContent(WORKSPACE_SHELL_ACCESS_EMPTY); + }); + + it("states the residual-risk sentence verbatim, at the top of the section", () => { + // §11.5 has no OS sandbox in v1. The sentence is the product's only honest + // account of that, and it is asserted against the exported constant so a + // paraphrase in either place fails rather than quietly softening the claim. + render( + , + ); + expect( + screen.getByTestId("workspace-shell-access-notice"), + ).toHaveTextContent(WORKSPACE_SHELL_ACCESS_NOTICE); + }); + + it("a workspace with the flag off reads off, and one with it on reads on", () => { + render( + , + ); + expect( + screen.getByTestId("workspace-shell-toggle-g_atlas"), + ).not.toBeChecked(); + expect(screen.getByTestId("workspace-shell-toggle-g_on")).toBeChecked(); + }); + + it("ENABLING does not apply from the switch — it arms a confirm that names the folder", async () => { + // The one property that matters most here. A single click on a switch is + // exactly the interaction a user performs without reading, and this is the + // control that lets an agent run code as them. + const onSetShellEnabled = vi.fn(); + const user = userEvent.setup(); + render( + , + ); + + await user.click(screen.getByTestId("workspace-shell-toggle-g_atlas")); + expect(onSetShellEnabled).not.toHaveBeenCalled(); + + const confirm = screen.getByTestId("workspace-shell-confirm-g_atlas"); + expect(confirm).toHaveTextContent( + workspaceShellAccessConfirmPrompt("atlas"), + ); + // The risk sentence is restated INSIDE the confirm, not only above the list. + expect(confirm).toHaveTextContent(WORKSPACE_SHELL_ACCESS_NOTICE); + expect( + within(confirm).getByRole("button", { + name: WORKSPACE_SHELL_ACCESS_CONFIRM_LABEL, + }), + ).toBeInTheDocument(); + + // ARMED IS NOT ON. The switch still reports the state the host holds, so a + // user who abandons the prompt does not return to a control that looks + // enabled over a permission nothing recorded. + expect( + screen.getByTestId("workspace-shell-toggle-g_atlas"), + ).not.toBeChecked(); + }); + + it("only the confirm applies the enable, and it applies to THAT grant", async () => { + const onSetShellEnabled = vi.fn(); + const user = userEvent.setup(); + render( + , + ); + + await user.click(screen.getByTestId("workspace-shell-toggle-g_notes")); + await user.click(screen.getByTestId("workspace-shell-allow-g_notes")); + + expect(onSetShellEnabled).toHaveBeenCalledTimes(1); + expect(onSetShellEnabled).toHaveBeenCalledWith("g_notes", true); + // Per-workspace: the other folder was never mentioned. + expect(onSetShellEnabled).not.toHaveBeenCalledWith("g_atlas", true); + expect( + screen.queryByTestId("workspace-shell-confirm-g_notes"), + ).not.toBeInTheDocument(); + }); + + it("cancelling the confirm applies nothing at all", async () => { + const onSetShellEnabled = vi.fn(); + const user = userEvent.setup(); + render( + , + ); + + await user.click(screen.getByTestId("workspace-shell-toggle-g_atlas")); + await user.click( + screen.getByRole("button", { + name: WORKSPACE_SHELL_ACCESS_CANCEL_LABEL, + }), + ); + + expect(onSetShellEnabled).not.toHaveBeenCalled(); + expect( + screen.queryByTestId("workspace-shell-confirm-g_atlas"), + ).not.toBeInTheDocument(); + }); + + it("arming a second folder closes the first, so two prompts are never open at once", async () => { + const user = userEvent.setup(); + render( + , + ); + + await user.click(screen.getByTestId("workspace-shell-toggle-g_atlas")); + await user.click(screen.getByTestId("workspace-shell-toggle-g_notes")); + + expect( + screen.queryByTestId("workspace-shell-confirm-g_atlas"), + ).not.toBeInTheDocument(); + expect( + screen.getByTestId("workspace-shell-confirm-g_notes"), + ).toBeInTheDocument(); + }); + + it("DISABLING applies immediately and asks nothing", async () => { + // Asymmetric on purpose: a control that removes authority must never be + // harder to reach than the one that grants it. + const onSetShellEnabled = vi.fn(); + const user = userEvent.setup(); + render( + , + ); + + await user.click(screen.getByTestId("workspace-shell-toggle-g_on")); + + expect(onSetShellEnabled).toHaveBeenCalledExactlyOnceWith("g_on", false); + expect( + screen.queryByTestId("workspace-shell-confirm-g_on"), + ).not.toBeInTheDocument(); + }); + + it("shows a host failure instead of pretending nothing is attached", () => { + render( + , + ); + expect( + screen.getByTestId("workspace-shell-access-error"), + ).toHaveTextContent("Couldn't allow commands in that folder."); + // The list is still there — an error is not an empty state. + expect( + screen.queryByTestId("workspace-shell-access-empty"), + ).not.toBeInTheDocument(); + }); + + it("is inert while a host call is in flight", () => { + render( + , + ); + expect(screen.getByTestId("workspace-shell-toggle-g_atlas")).toBeDisabled(); + }); +}); diff --git a/packages/chat-surface/src/settings/WorkspaceShellAccess.tsx b/packages/chat-surface/src/settings/WorkspaceShellAccess.tsx new file mode 100644 index 000000000..d9da061df --- /dev/null +++ b/packages/chat-surface/src/settings/WorkspaceShellAccess.tsx @@ -0,0 +1,215 @@ +// — the per-workspace "may the agent run commands here?" +// list (PRD-shell-execution §7.3, §14.4). +// +// WHY PER-FOLDER AND NOT ONE SWITCH. "I trust the agent to run commands in +// ~/code/my-project" is a different sentence from "I trust the agent to run +// commands", and the product must be able to express the first without implying +// the second. The grant is already the unit the user thinks in — they attached +// THAT folder — so the permission rides the grant. +// +// WHY IT IS NOT A FOURTH GrantMode. The three modes (`read_only` < +// `read_write_no_delete` < `read_write`) are ORDERED file access. A fourth member +// would read as "more than read_write", which is wrong twice: running a command +// is a different kind of authority, and it is not a superset of write — a +// read-only folder someone wants to run a build in is a coherent thing to want. +// +// PRESENTATIONAL. No port, no fetching, no browser primitive. The caller owns +// `useWorkspaceFolderGrants` and decides whether to mount this at all — which is +// how a host with no `WorkspaceGrantPort.setShellEnabled` (web, and any desktop +// build with shell execution off) gets an ABSENT section rather than a control +// that cannot work. +// +// THE ASYMMETRY IS THE DESIGN. Turning commands ON is a two-step act: the switch +// arms a confirm that NAMES THE FOLDER and states §11.5's sentence verbatim, +// and only the confirm applies it. Turning them OFF applies immediately and +// asks nothing — a control that removes authority must never be harder to reach +// than the one that grants it. + +import { Button, Toggle } from "@0x-copilot/design-system"; +import { type CSSProperties, type ReactElement, useState } from "react"; + +import { WORKSPACE_SHELL_ACCESS_NOTICE } from "../composer/useWorkspaceFolderGrants"; +import type { WorkspaceGrant } from "../ports/WorkspaceGrantPort"; + +import { Frow, SetCard, SetNote } from "./SettingsChrome"; + +/** Section copy. Exported so the test reads the same bytes the surface renders. */ +export const WORKSPACE_SHELL_ACCESS_TITLE = "Run commands"; +export const WORKSPACE_SHELL_ACCESS_META = + "Which attached folders the agent may run commands in. Off for every folder until you turn it on."; +/** + * The empty state. It says "attach a folder first" rather than "no folders", + * because the section's absence and an empty section mean different things and + * a user who reads only this one must still know what to do next. + */ +export const WORKSPACE_SHELL_ACCESS_EMPTY = + "No folders are attached yet. Attach one from the composer to choose whether the agent may run commands in it."; + +/** The confirm's verb. Pinned by the test — it must not soften to "OK". */ +export const WORKSPACE_SHELL_ACCESS_CONFIRM_LABEL = "Allow commands"; +export const WORKSPACE_SHELL_ACCESS_CANCEL_LABEL = "Cancel"; + +/** The confirm's sentence, with the folder named. */ +export function workspaceShellAccessConfirmPrompt(label: string): string { + return `Allow the agent to run commands in ${label}?`; +} + +export interface WorkspaceShellAccessProps { + /** Active grants, path-free. Empty renders the empty note, not nothing. */ + readonly grants: readonly WorkspaceGrant[]; + /** + * Apply a decision, or `null` when the host cannot — in which case NOTHING is + * rendered. `useWorkspaceFolderGrants().setShellEnabled` is already this + * shape; see that hook for why it is null rather than a no-op. + */ + readonly onSetShellEnabled: + | ((grantId: string, enabled: boolean) => void) + | null; + /** A failed read/change, SHOWN. Never rendered as "no folders". */ + readonly error?: string | null; + /** A host call is in flight; every control is inert while it is. */ + readonly busy?: boolean; +} + +const listStyle: CSSProperties = { display: "block" }; + +const confirmStyle: CSSProperties = { + display: "flex", + alignItems: "center", + justifyContent: "flex-end", + gap: "var(--space-sm)", + padding: "10px 14px", + borderTop: "1px solid var(--color-border)", +}; + +/** + * The section body. Returns `null` when the host supplies no setter — the + * capability is absent, and a disabled row of switches would advertise a + * feature this build does not have. + */ +export function WorkspaceShellAccess({ + grants, + onSetShellEnabled, + error = null, + busy = false, +}: WorkspaceShellAccessProps): ReactElement | null { + // Which folder is mid-confirm, by grantId. At most one — arming a second + // replaces the first, so two "allow commands" prompts can never be open with + // only one of them visible to the user. + const [arming, setArming] = useState(null); + + if (onSetShellEnabled === null) return null; + + return ( + + {/* The residual-risk sentence, stated ONCE at the top of the section and + again inside every confirm. There is no OS sandbox in v1 (§11.5), and + a per-folder switch without this reads as a confinement claim that the + cwd binding does not make — it decides where a command STARTS, not + where it can reach. */} + + {WORKSPACE_SHELL_ACCESS_NOTICE} + + + {error !== null ? ( + + {error} + + ) : null} + + {grants.length === 0 ? ( + + {WORKSPACE_SHELL_ACCESS_EMPTY} + + ) : ( +
+ {grants.map((grant) => { + const armed = arming === grant.grantId; + return ( +
+ + { + if (event.currentTarget.checked) { + // Enabling never applies from the switch itself. + setArming(grant.grantId); + return; + } + setArming(null); + onSetShellEnabled(grant.grantId, false); + }} + /> + + {armed ? ( +
+

+ {/* The folder is NAMED in the confirm (§14.4). A prompt + that said "this folder" would be answerable without + knowing which one, and the whole point of the flag is + that it is per-folder. */} + + {workspaceShellAccessConfirmPrompt(grant.label)} + {" "} + {WORKSPACE_SHELL_ACCESS_NOTICE} +

+ + +
+ ) : null} +
+ ); + })} +
+ )} +
+ ); +} diff --git a/packages/chat-surface/src/settings/index.ts b/packages/chat-surface/src/settings/index.ts index 93926bd3b..fd896234f 100644 --- a/packages/chat-surface/src/settings/index.ts +++ b/packages/chat-surface/src/settings/index.ts @@ -321,6 +321,20 @@ export { DEFAULT_APPROVAL_POLICY, type ApprovalPolicyPort, } from "./data/toolUsePolicy"; +// Per-workspace shell enablement (PRD-shell-execution §7.3, §14.4). The +// section body only; the grant list and the setter are host-owned and arrive +// from `useWorkspaceFolderGrants`, so a host with no +// `WorkspaceGrantPort.setShellEnabled` mounts nothing. +export { + WorkspaceShellAccess, + WORKSPACE_SHELL_ACCESS_TITLE, + WORKSPACE_SHELL_ACCESS_META, + WORKSPACE_SHELL_ACCESS_EMPTY, + WORKSPACE_SHELL_ACCESS_CONFIRM_LABEL, + WORKSPACE_SHELL_ACCESS_CANCEL_LABEL, + workspaceShellAccessConfirmPrompt, + type WorkspaceShellAccessProps, +} from "./WorkspaceShellAccess"; // === end Phase 5 (PR-5.6) === // === Phase 5 (PR-5.9) — Advanced group (Key storage & app lock · Developer diff --git a/packages/chat-surface/src/thread-canvas/TcWriteGateCard.test.tsx b/packages/chat-surface/src/thread-canvas/TcWriteGateCard.test.tsx index de68f83ba..f784857fe 100644 --- a/packages/chat-surface/src/thread-canvas/TcWriteGateCard.test.tsx +++ b/packages/chat-surface/src/thread-canvas/TcWriteGateCard.test.tsx @@ -7,10 +7,15 @@ // **Connect** button for a connector that was already connected, while the // actual decision sat in the chat column. +import { readFileSync } from "node:fs"; +import { dirname, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; + import { describe, expect, it, vi } from "vitest"; import { render, screen } from "@testing-library/react"; -import { TcWriteGateCard } from "./TcWriteGateCard"; +import { TcWriteGateCard, type TcWriteGateCardProps } from "./TcWriteGateCard"; +import type { TcWriteGatePayloadProps } from "./TcWriteGatePayload"; function card(overrides: Partial[0]> = {}) { const props = { @@ -108,3 +113,156 @@ describe("TcWriteGateCard — degraded input", () => { expect(params.querySelector("img")).toBeNull(); }); }); + +/* ── the Studio-canvas gap, closed (PRD-shell-execution §14.1) ────────────── + * + * The card rendered `TcWriteGatePayload` and passed it `params` and `ledgerId` + * only. Every other field on that component is absent-means-omitted, so the + * under-fed surface drew a SMALLER card rather than failing — which is why the + * hole survived: a command ask on the canvas showed its title and an enabled + * Approve, with the command itself only in the chat column. + */ + +/** The `run_command` ask as `approvalProjection` freezes it: the write gate's + * wire shape carries no `arguments` and no presentation, so `command` is the + * only evidence on the card. */ +function commandCard(command: string) { + card({ + title: "Run `pytest -q` in my-project", + connector: null, + params: [], + commandText: command, + irreversible: true, + }); +} + +describe("TcWriteGateCard — the command ask", () => { + it("shows the exact command the decision is made over", () => { + commandCard("pytest -q"); + expect(screen.getByTestId("tc-write-gate-card-command").textContent).toBe( + "pytest -q", + ); + }); + + it("keeps the bytes a multi-line command was written in", () => { + // What runs is `/bin/sh -c ""` with the bytes the model sent. A + // surface that re-flowed them would be showing a different command from the + // one being approved — the card/process disagreement this lane exists to + // prevent. + const multi = 'set -e\nprintf "a\\n"\nls -la'; + commandCard(multi); + const block = screen.getByTestId("tc-write-gate-card-command"); + expect(block.textContent).toBe(multi); + expect(block.tagName.toLowerCase()).toBe("pre"); + }); + + it("carries the no-undo line the row carries", () => { + commandCard("rm -rf build"); + expect(screen.getByTestId("tc-write-gate-card-no-undo").textContent).toBe( + "Changes made by a command can't be undone from here.", + ); + }); + + it("replaces the reversibility line rather than stacking on it", () => { + // Two "cannot be undone" sentences three lines apart reads as a bug and + // trains people to skim the one that matters. + commandCard("rm -rf build"); + expect(screen.queryByTestId("tc-write-gate-card-reversibility")).toBeNull(); + }); + + it("renders a hostile command as text", () => { + const hostile = 'echo ""'; + commandCard(hostile); + const block = screen.getByTestId("tc-write-gate-card-command"); + expect(block.textContent).toBe(hostile); + expect(block.querySelector("img")).toBeNull(); + }); + + it("wraps an unbreakable token instead of widening the card", () => { + // `.tc-write-gate__command` is `white-space: pre-wrap`, which breaks at + // spaces and nowhere else. The row's expanded body has had a per-CONTAINER + // `overflow-wrap: anywhere` since a 138-character token measured a 356px + // column out to 1022px; this card renders the same payload OUTSIDE that + // container, so it needs — and now has — its own copy of the rule. + // + // Asserted against the real stylesheet, because the rule is the fix: jsdom + // performs no layout, so the resolved cascade is what is knowable here. + const sheet = document.createElement("style"); + sheet.textContent = readFileSync( + resolve( + typeof import.meta.dirname === "string" + ? import.meta.dirname + : dirname(fileURLToPath(import.meta.url)), + "review-surfaces.css", + ), + "utf-8", + ); + document.head.appendChild(sheet); + try { + commandCard(`curl https://example.test/${"a".repeat(400)}`); + expect( + getComputedStyle(screen.getByTestId("tc-write-gate-card-command")) + .overflowWrap, + ).toBe("anywhere"); + } finally { + sheet.remove(); + } + }); +}); + +describe("TcWriteGateCard — the shaped evidence", () => { + it("renders the batch the canvas used to drop", () => { + // Not a command-lane fix. `presentation` had the same hole, so a rows card + // showed twelve payees in the chat column and a params-less heading on the + // canvas. + card({ + params: [], + presentation: { + layout: "rows", + rows: [ + { + rowId: "r1", + label: "Mira Patel", + value: "$1,200.00", + note: null, + initials: "MP", + status: "pending", + decidable: false, + }, + ], + preview: null, + approveLabel: null, + rejectLabel: null, + provenance: null, + }, + }); + const rows = screen.getByTestId("tc-write-gate-card-rows"); + expect(rows.textContent).toContain("Mira Patel"); + expect(rows.textContent).toContain("$1,200.00"); + }); +}); + +describe("TcWriteGateCard — prop parity with the payload", () => { + it("accepts every field the shared payload renders", () => { + // A TYPE assertion, enforced by `npm run typecheck` rather than at runtime, + // because the failure this guards is a compile-time omission that produces + // no error: every payload field is absent-means-omitted, so a card that + // stopped forwarding one would keep rendering — smaller, and silently. + // That is exactly how `presentation` and `commandText` came to be missing. + // + // `testIdPrefix` is excluded: it is the one prop that is deliberately + // per-surface, since both surfaces can be mounted at once for one gate. + type PayloadEvidence = Exclude< + keyof TcWriteGatePayloadProps, + "testIdPrefix" + >; + type Missing = Exclude; + // `[Missing] extends [never]`, tuple-wrapped: a bare `extends never` + // distributes and is `never` for BOTH answers, so it would pass whatever + // the card is missing. Annotating with a conditional type is what makes + // this a compile error rather than a green tautology — `= true` does not + // typecheck against `false`. + const parity: [Missing] extends [never] ? true : false = true; + expect(parity).toBe(true); + }); +}); diff --git a/packages/chat-surface/src/thread-canvas/TcWriteGateCard.tsx b/packages/chat-surface/src/thread-canvas/TcWriteGateCard.tsx index 490a628c4..621407f68 100644 --- a/packages/chat-surface/src/thread-canvas/TcWriteGateCard.tsx +++ b/packages/chat-surface/src/thread-canvas/TcWriteGateCard.tsx @@ -27,6 +27,7 @@ import type { CSSProperties, ReactElement } from "react"; +import type { ApprovalPresentation } from "../approvals/presentation"; import type { ActivityParam } from "../approvals/types"; import { TcWriteGatePayload } from "./TcWriteGatePayload"; @@ -37,6 +38,35 @@ export interface TcWriteGateCardProps { readonly connector: string | null; /** The call's arguments, already projected + sanitised for display. */ readonly params: readonly ActivityParam[]; + /** + * The server-projected SHAPE of this call — the batch it will execute, or the + * draft it will send. Same field, same default, same meaning as + * `TcWriteGateRow.presentation`: absent-means-omitted, so a gate without one + * renders byte-for-byte what this card rendered before shapes existed. + */ + readonly presentation?: ApprovalPresentation | null; + /** + * The exact command a `run_command` ask will execute (PRD-shell-execution + * §14.1). + * + * IT IS THE WHOLE CARD, on a command ask. The write gate's wire shape carries + * no `arguments` and no presentation, so without this the canvas twin of a + * command approval was a title, a no-undo-less reversibility line, and an + * enabled Approve button — a decision offered over a payload the surface had + * not shown. `TcWriteGatePayload`'s own header called that out as the one + * payload where "the card and the process disagree about what was approved" + * is the entire risk, and this prop is that gap closed. + * + * Approve is NOT additionally gated on the payload having rendered, unlike + * `TcWriteGateRow`'s. The row gates because it can be COLLAPSED — its rule is + * "no approval before the payload has been seen", and a collapsed row has not + * shown it. This card has no disclosure: the payload is on screen whenever + * there is one, so the rule is satisfied structurally. Adding the predicate + * here would instead disable Approve on every ordinary MCP write gate, whose + * wire shape legitimately carries neither params nor presentation — the + * dead-control failure `TcWriteGateRow` documents at length. + */ + readonly commandText?: string | null; /** `r·` — the audit anchor this decision will be recorded under. */ readonly ledgerId: string; /** True when the write cannot be undone from inside the app. */ @@ -50,6 +80,8 @@ export function TcWriteGateCard({ title, connector, params, + presentation = null, + commandText = null, ledgerId, irreversible, onApprove, @@ -58,7 +90,13 @@ export function TcWriteGateCard({ }: TcWriteGateCardProps): ReactElement { return (
{/* The same component the transcript row's expanded body renders, so the - two surfaces cannot disagree about what is being approved. */} + two surfaces cannot disagree about what is being approved — and now + with the same PROPS, which is what that sentence needed to be true. + Passing the component but not its evidence made the two surfaces + disagree in the one direction that matters: the row showed the command + and the canvas showed a heading. */} 's + * min-content width, which the card stretches to, which the gate region then + * has to scroll horizontally. The row's rule above is written per-CONTAINER for + * exactly this reason — "anything that lands in the body inherits it, including + * whatever gets added next" — and this is the second container that body now + * renders into. */ +.tc-write-gate-card, +.tc-write-gate-card * { + overflow-wrap: anywhere; +} + .tc-write-gate__reason { color: var(--color-text-muted); font-size: var(--font-size-xs); diff --git a/packages/chat-surface/src/thread-canvas/toolViews.test.tsx b/packages/chat-surface/src/thread-canvas/toolViews.test.tsx index b7416e48e..1a060cc18 100644 --- a/packages/chat-surface/src/thread-canvas/toolViews.test.tsx +++ b/packages/chat-surface/src/thread-canvas/toolViews.test.tsx @@ -398,6 +398,127 @@ describe("run_command view", () => { expect(screen.queryByTestId("tc-tool-command-exit")).toBeNull(); }); + it("says WHY there is no exit code, rather than drawing nothing", () => { + // The other half of the assertion above, and the reason it is not enough on + // its own: with no chip at all, a settled timeout renders exactly like a + // command that is still running — same command row, same absent chip. "It + // could not run" and "it ran and returned 1" are different facts and the + // card has to be able to say both. + render( + , + ); + const chip = screen.getByTestId("tc-tool-command-status"); + expect(chip.textContent).toBe("timed out"); + expect(chip.getAttribute("data-command-status")).toBe("timeout"); + expect(chip.getAttribute("data-exit")).toBe("none"); + // Neither of the two outcome hues: the process reported nothing, so there + // is no result to colour. Amber here would put a policy refusal and a + // user's own Stop in the same register as a failing test suite. + expect(chip.getAttribute("style")).not.toContain("--color-warning"); + expect(chip.getAttribute("style")).not.toContain("--color-success"); + expect(chip.getAttribute("style")).not.toContain("--color-danger"); + }); + + it.each([ + ["timeout", "timed out"], + ["cancelled", "cancelled"], + ["refused", "refused"], + ["unavailable", "unavailable"], + ])("labels the %s status", (status, label) => { + render( + , + ); + expect(screen.getByTestId("tc-tool-command-status").textContent).toBe( + label, + ); + }); + + it("draws no status chip for a completed command, whatever its exit", () => { + // "completed" is not an outcome the chip slot can carry — the exit code is. + // A completed result that lost its `exit_code` in transit has nothing to + // report, and "completed" printed where the code should be would read as a + // success claim nobody made. + render( + , + ); + expect(screen.queryByTestId("tc-tool-command-status")).toBeNull(); + expect(screen.queryByTestId("tc-tool-command-exit")).toBeNull(); + }); + + it("shows an unrecognised status word rather than hiding it", () => { + // `status` is a closed `Literal` server-side, so a word this build does not + // know means the contract grew. A chip reading an unfamiliar word is a + // smaller failure than a settled command that looks like it is still + // running — and the record still parses, so the output survives too. + render( + , + ); + const chip = screen.getByTestId("tc-tool-command-status"); + expect(chip.textContent).toBe("quarantined"); + expect(chip.getAttribute("data-command-status")).toBe("quarantined"); + expect(screen.getByTestId("tc-tool-command-output").textContent).toContain( + "halted", + ); + }); + + it("clips a status token so a long one cannot become the row", () => { + render( + , + ); + expect( + (screen.getByTestId("tc-tool-command-status").textContent as string) + .length, + ).toBeLessThanOrEqual(24); + }); + + it("keeps a running command chip-free", () => { + // Nothing has settled, so neither chip may claim anything. This is the + // state the timeout chip above exists to stop being confused with. + render( + vitest run\n", + })} + />, + ); + expect(screen.queryByTestId("tc-tool-command-exit")).toBeNull(); + expect(screen.queryByTestId("tc-tool-command-status")).toBeNull(); + }); + it("colours a non-zero exit as a warning, not as a failure", () => { // `grep` exits 1 on no matches. Painting that in the destructive hue would // make the transcript's one "something is wrong" colour mean "a command @@ -602,6 +723,239 @@ describe("run_command view", () => { expect(output).not.toBe("not ours"); }); + it("says the runtime truncated the bytes, and how many there were", () => { + // §13/§14.3. This is read from the STRUCTURED fields, not from the notice + // §13 has the runtime put inside `output` — see the next test for why that + // distinction is the whole point. + render( + , + ); + expect(screen.getByTestId("tc-tool-command-truncated").textContent).toBe( + "Output truncated — this is the tail of 4.2 MB. The full output was saved.", + ); + }); + + it("survives the line clip that throws the runtime's own notice away", () => { + // The reason the card re-states it. §13 has the runtime PREPEND its notice + // to the output; this view keeps the LAST COMMAND_MAX_LINES — so on any + // output long enough to be truncated, the in-string notice is the first + // thing dropped. Read from the fields, the fact is not part of the clipped + // text and cannot be clipped with it. + const runtimeNotice = + "...output truncated (kept the last 64 KiB of 4.2 MB)"; + render( + `line ${i}`), + ].join("\n"), + exit_code: 0, + truncated: true, + output_total_bytes: 4_404_019, + }), + })} + />, + ); + // Gone from the text, exactly as described… + expect( + screen.getByTestId("tc-tool-command-output").textContent, + ).not.toContain("output truncated (kept the last"); + // …and still on the card. + expect(screen.getByTestId("tc-tool-command-truncated").textContent).toBe( + "Output truncated — this is the tail of 4.2 MB.", + ); + // Two cuts, two sentences: the runtime's bytes and the card's lines are + // different facts and neither number describes the other. + expect(screen.getByTestId("tc-tool-command-clipped").textContent).toBe( + `${5002 - COMMAND_MAX_LINES} earlier lines not shown`, + ); + }); + + it("promises a saved file only when one was reported", () => { + render( + , + ); + // No `output_total_bytes` and no `output_ref`: the card says the one thing + // it knows and invents neither the size nor the file. + expect(screen.getByTestId("tc-tool-command-truncated").textContent).toBe( + "Output truncated — this is the tail.", + ); + }); + + it("says nothing about truncation when nothing was truncated", () => { + render( + , + ); + expect(screen.queryByTestId("tc-tool-command-truncated")).toBeNull(); + }); + + it("does not attach the settled truncation claim to the live tail", () => { + // The frame is showing `outputPreview` because the result is unreadable, so + // any volumetric sentence would be describing a string that is not on + // screen. + render( + , + ); + expect(screen.queryByTestId("tc-tool-command-truncated")).toBeNull(); + }); + + it("renders the runtime's exit note beside a non-zero exit", () => { + // §4.3's `exit_note`. It is the sentence that stops "exit 1" reading as a + // crash, and it is the runtime's own words rather than a table this client + // would have to keep in sync with every CLI in the world. + render( + , + ); + // The command printed NOTHING, which is exactly the case the note explains + // — and exactly the case that has no output frame for it to live in. So it + // sits above, on the panel, not inside the output block. + expect(screen.queryByTestId("tc-tool-command-output")).toBeNull(); + expect(screen.getByTestId("tc-tool-command-exit-note").textContent).toBe( + "grep exits 1 when there are no matches.", + ); + expect(screen.getByTestId("tc-tool-command-exit").textContent).toBe( + "exit 1", + ); + }); + + it("omits the exit note when the runtime sent none", () => { + render( + , + ); + expect(screen.queryByTestId("tc-tool-command-exit-note")).toBeNull(); + }); + + it("makes no truncation or exit-note claim from an unrecognised result", () => { + // The raw-text fallback carries the text and nothing else. A default of + // `truncated: true` or a scavenged note would be this card asserting facts + // about a payload whose shape it just admitted it does not know. + render( + , + ); + expect(screen.queryByTestId("tc-tool-command-truncated")).toBeNull(); + expect(screen.queryByTestId("tc-tool-command-exit-note")).toBeNull(); + expect(screen.queryByTestId("tc-tool-command-status")).toBeNull(); + }); + + it("bounds the output block's height and scrolls it in its own box", () => { + // §16.5 asks for LAYOUT, not DOM presence — this repo has shipped a + // disclosure clipped to 6% of its ink under a green suite. jsdom performs + // no layout, so what is assertable is the resolved cascade: the block has a + // ceiling and owns its own scroll, which is what keeps a 200-line command + // from pushing the transcript around and its long lines from making the + // page body scroll sideways (AC8.4). + render( + , + ); + const style = getComputedStyle( + screen.getByTestId("tc-tool-command-output"), + ); + expect(style.maxHeight).toBe("320px"); + expect(style.overflow).toBe("auto"); + }); + + it("has no class names for a host stylesheet to re-declare", () => { + // §14.3 wants the command view's CSS shipped inside this package with a + // gate asserting no host sheet re-declares its class names. The view + // answers that by construction rather than by a guard: it is styled + // entirely with inline `CSSProperties`, and an inline style cannot be + // shadowed by a host sheet at all. This is the assertion that keeps that + // property true — the moment someone reaches for a `className` here, the + // PR #459 failure mode (rules stranded in `apps/frontend/src/styles.css`, + // surface unstyled on desktop) becomes reachable again and this fails. + render( + , + ); + const block = screen.getByTestId("tc-tool-command"); + for (const node of [block, ...block.querySelectorAll("*")]) { + expect(node.getAttribute("class")).toBeNull(); + } + }); + it("collapses a multi-line command on the one-line row only", () => { const multi = 'printf "a\\n" \\\n && ls -la'; render( diff --git a/packages/chat-surface/src/thread-canvas/toolViews.tsx b/packages/chat-surface/src/thread-canvas/toolViews.tsx index ff4587d80..2689e8757 100644 --- a/packages/chat-surface/src/thread-canvas/toolViews.tsx +++ b/packages/chat-surface/src/thread-canvas/toolViews.tsx @@ -31,6 +31,13 @@ import type { CSSProperties, ReactElement, ReactNode } from "react"; import { Icon } from "../icons/Icon"; +// The package's one byte formatter, reused rather than re-derived. It is a pure +// function with no substrate touchpoints and no tokens (its own header says +// so), and the alternative is a THIRD copy of the same eight lines — there is +// already one in `settings/` and one in `artifacts/useArtifactSurface`. Binary +// units labelled KB/MB, which is the house answer; picking a different one here +// would make the same 4.2 MB read two ways in two surfaces. +import { formatBytes } from "../settings/localModelsFormat"; import type { ToolCallEntry } from "./eventProjector"; import { computeLineDiff } from "./lineDiff"; import { TcFileDiff } from "./TcFileDiff"; @@ -223,15 +230,31 @@ const SEARCH_MAX_ROWS = 40; * TWO SOURCES, AND THE LIFECYCLE PICKS. While the call is open there is no * result, only `outputPreview` — the rolling tail the projector holds from * `tool_call_delta` frames. Once it settles, the tool's own result is - * authoritative: the runtime tail-keeps it to 64 KiB and puts its truncation - * notice inside the string (§13), so it says more than the live tail ever can. - * The preview stays as the fallback for the one case where the result is - * missing — a run cancelled mid-command — rather than being preferred. + * authoritative: the runtime tail-keeps it to 64 KiB and carries the truncation + * and exit facts as structured fields (§4.3), so it says more than the live + * tail ever can. The preview stays as the fallback for the one case where the + * result is missing — a run cancelled mid-command — rather than being + * preferred. * - * THE EXIT CODE IS RENDERED, in three states not two. `status === "error"` - * means the TOOL failed; a non-zero exit means the tool worked and the command - * did not, and OpenCode showing neither is the defect §14.3 cites. Non-zero is - * amber, never the destructive hue: `grep` exits 1 on no matches. + * THE OUTCOME IS RENDERED, in FOUR states, not two. `status === "error"` on + * the card means the TOOL failed; below it, + * + * exit 0 the command ran and succeeded success token + * exit N the command ran and reported something WARNING token + * timed out / the command never produced an exit code neutral token + * cancelled / + * refused / + * unavailable + * (no chip) nothing has settled yet + * + * The middle two are the ones worth being careful about. A non-zero exit is + * frequently the POINT — a failing test suite is information, `grep` exits 1 on + * no matches — so it is amber and never the destructive hue (§14.3), and + * OpenCode showing neither code nor chip is the defect that section cites. And + * "the command could not run" is a DIFFERENT fact from "the command returned + * 1": §4.3 gives `exit_code: None` to every status but `completed`, so drawing + * nothing for those four left a settled timeout looking exactly like a command + * still running. The chip is what separates them. */ function CommandBody({ toolCall }: ToolViewBodyProps): ReactElement | null { const command = commandText(toolCall); @@ -246,7 +269,13 @@ function CommandBody({ toolCall }: ToolViewBodyProps): ReactElement | null { // Nothing known yet — arguments still streaming, no output, no result. An // empty bordered frame claims there is something to look at. if (command === null && output === null) return null; - const exitCode = settled?.exitCode ?? null; + // Only ever said ABOUT the settled output. When the frame below is showing + // the live tail instead, the runtime's byte truncation is a fact about a + // string that is not on screen, and printing it there would attach the wrong + // volumetric claim to the wrong text. + const truncation = + settled !== null && output !== null ? truncationNote(settled) : null; + const exitNote = settled?.exitNote ?? null; return (
@@ -260,25 +289,147 @@ function CommandBody({ toolCall }: ToolViewBodyProps): ReactElement | null { {command} - {exitCode === null ? null : ( - - exit {exitCode} - - )} +
)} - {output === null ? null : } + {/* The runtime's own sentence about the exit code ("grep exit 1 means no + matches", §4.3). It sits ABOVE the output rather than inside it, + because the command it most needs to explain is the one that printed + NOTHING — and that command has no output frame for it to live in. */} + {exitNote === null ? null : ( +

+ {exitNote} +

+ )} + {output === null ? null : ( + + )}
); } +/** + * The outcome chip — an exit code when there is one, otherwise WHY there is + * not. + * + * Two testids, deliberately, rather than one that changes meaning: + * `…-command-exit` keeps saying exactly what it said before this existed — the + * process ran and this is its code — and `…-command-status` is the new fact. + * A reader (or a journey) asking "did this command exit non-zero" gets the same + * answer it always did, and a timeout can no longer answer it by silence. + */ +function OutcomeChip({ + outcome, +}: { + readonly outcome: CommandOutcome | null; +}): ReactElement | null { + if (outcome === null) return null; + if (outcome.exitCode !== null) { + const ok = outcome.exitCode === 0; + return ( + + exit {outcome.exitCode} + + ); + } + const status = didNotCompleteStatus(outcome.status); + if (status === null) return null; + return ( + + {status.label} + + ); +} + +/** + * The four §4.3 statuses that carry no exit code, as a chip label — or `null` + * when there is nothing honest to draw. + * + * `completed` returns null on purpose: a completed run whose `exit_code` was + * lost in transit has no outcome to report, and "completed" in the slot the + * exit code should occupy would read as a success claim nobody made. + * + * An UNRECOGNISED token renders as itself rather than as nothing, which is this + * file's standing rule pointed at an enum instead of a payload shape. The + * runtime's `status` is a closed `Literal`, so a token that is not one of these + * five means the contract grew — and a chip reading a word the client does not + * know is a smaller failure than a settled command that looks like it is still + * running. Clipped, because it is still a string off a wire. + */ +function didNotCompleteStatus( + status: string | null, +): { readonly token: string; readonly label: string } | null { + if (status === null) return null; + const token = status.trim(); + if (token === "" || token === "completed") return null; + const label = DID_NOT_COMPLETE_LABELS[token]; + return { + token: token.slice(0, COMMAND_STATUS_TOKEN_CAP), + label: label ?? token.slice(0, COMMAND_STATUS_TOKEN_CAP), + }; +} + +/** Copy for the four §4.3 non-`completed` statuses. Lower case, so the chip + * reads as a state beside "exit 1" rather than as a heading. */ +const DID_NOT_COMPLETE_LABELS: Readonly> = { + timeout: "timed out", + cancelled: "cancelled", + refused: "refused", + unavailable: "unavailable", +}; + +const COMMAND_STATUS_TOKEN_CAP = 24; + +/** + * "Output truncated — this is the tail of 4.2 MB. The full output was saved." + * + * WHY THE CARD SAYS THIS AT ALL, when §13 has the runtime put the same sentence + * inside `output`: the runtime PREPENDS it, and this view keeps the LAST + * `COMMAND_MAX_LINES` — so on any output long enough to be truncated, the + * in-string notice is the first thing the line clip throws away. The reader + * would lose the fact exactly where it matters most. Read from the structured + * fields instead (§4.3 `truncated` / `output_total_bytes` / `output_ref`), it + * survives the clip because it is not part of the clipped text. + * + * It can therefore be said twice — once here, once in the surviving head of the + * output — on the narrow class of outputs that hit 64 KiB in under 200 lines + * (i.e. averaging 327+ bytes a line). Saying a true thing twice is the right + * side to err on against losing it entirely, and de-duplicating would mean + * substring-matching a sentence the runtime owns. + * + * "The full output was saved" is gated on `output_ref` actually being present, + * because a promise that a file exists is not one to make from a default. + * The virtual path itself is not rendered: `read_file` is the model's way back + * to it (§13), and it is not an address a human here can use. + */ +function truncationNote(outcome: CommandOutcome): string | null { + if (!outcome.truncated) return null; + const size = + outcome.totalBytes === null ? null : formatBytes(outcome.totalBytes); + const head = + size === null + ? "Output truncated — this is the tail." + : `Output truncated — this is the tail of ${size}.`; + return outcome.savedFullOutput ? `${head} The full output was saved.` : head; +} + /** Text worth a frame, or `null`. A command that printed only a newline has * printed nothing a reader needs to see. */ function nonEmpty(text: string | undefined): string | null { @@ -304,19 +455,43 @@ function nonEmpty(text: string | undefined): string | null { * no escapes at all. Quietly STRIPPING them instead would be the worst of the * three options: it makes the missing feature look shipped. */ -function CommandOutput({ text }: { readonly text: string }): ReactElement { +function CommandOutput({ + text, + truncation, +}: { + readonly text: string; + readonly truncation: string | null; +}): ReactElement { const lines = text.split("\n"); const shown = lines.length > COMMAND_MAX_LINES ? lines.slice(lines.length - COMMAND_MAX_LINES) : lines; const omitted = lines.length - shown.length; + // TWO CUTS, TWO SENTENCES, and they are not the same fact. The runtime cut + // BYTES at 64 KiB before the string ever left the process; this view cut + // LINES to bound what it mounts. Merging them into one line would have to + // pick a number, and neither number describes the other cut. + // + // Both sit ABOVE the output, unlike the read/search footnotes: the tail is + // what was kept, so the reader meets "there was more before this" before the + // text rather than after scrolling to the bottom of it. + const notes = truncation !== null || omitted > 0; return (
- {omitted > 0 ? ( -

- {omitted} earlier {omitted === 1 ? "line" : "lines"} not shown -

+ {notes ? ( +
+ {truncation === null ? null : ( +

+ {truncation} +

+ )} + {omitted > 0 ? ( +

+ {omitted} earlier {omitted === 1 ? "line" : "lines"} not shown +

+ ) : null} +
) : null}
         {shown.join("\n")}
@@ -365,11 +540,23 @@ function commandSubtitle(toolCall: ToolCallEntry): string | null {
  * `output` is always a string — the empty one when the command printed nothing,
  * which is a fact and not an absence. `exitCode` is genuinely nullable: §4.3
  * defines it as `None` for every status other than `completed`, so a timeout
- * has no exit code to show.
+ * has no exit code to show — which is why `status` is carried beside it rather
+ * than used only as a shape gate. The two together are what let the card say
+ * "returned 1" and "could not run" in different words.
+ *
+ * `savedFullOutput` is a BOOLEAN, not the `output_ref` path. §4.3 makes that
+ * path a virtual agent-scratch address the model resolves with `read_file`; it
+ * is not something a reader here can open, so the card keeps the only part of
+ * it that is addressed to a human — that there is one.
  */
 interface CommandOutcome {
+  readonly status: string | null;
   readonly output: string;
   readonly exitCode: number | null;
+  readonly exitNote: string | null;
+  readonly truncated: boolean;
+  readonly totalBytes: number | null;
+  readonly savedFullOutput: boolean;
 }
 
 /**
@@ -392,7 +579,28 @@ function readCommandOutcome(toolCall: ToolCallEntry): CommandOutcome | null {
   if (direct !== null) return direct;
   const text = resultText(toolCall);
   if (text === undefined || text.trim() === "") return null;
-  return parseCommandResult(text) ?? { output: text, exitCode: null };
+  return parseCommandResult(text) ?? unrecognisedResult(text);
+}
+
+/**
+ * A result whose shape this file does not know, carried as its own text and
+ * nothing else.
+ *
+ * Every claim-bearing field is null/false rather than defaulted, so an
+ * unrecognised string can only ever produce the output frame — never an exit
+ * chip, never a "could not run", never a volumetric sentence about a size
+ * nobody reported.
+ */
+function unrecognisedResult(text: string): CommandOutcome {
+  return {
+    status: null,
+    output: text,
+    exitCode: null,
+    exitNote: null,
+    truncated: false,
+    totalBytes: null,
+    savedFullOutput: false,
+  };
 }
 
 /**
@@ -401,20 +609,46 @@ function readCommandOutcome(toolCall: ToolCallEntry): CommandOutcome | null {
  * Gated on `status` AND `output` — both required by the contract — because a
  * command whose own stdout happens to be JSON would otherwise be mistaken for
  * the envelope and rendered as its own metadata.
+ *
+ * `status` is checked as a SHAPE (a string) and not for membership in the
+ * §4.3 literal set. A payload that arrives with a status word this build has
+ * not heard of is still a `RunCommandResult` and its output is still the
+ * command's output; rejecting the record over the enum would drop the whole
+ * thing back to the raw-text path, losing the exit code and the truncation
+ * facts to a field that was never the point of the gate.
  */
 function readCommandResultRecord(
   record: Record,
 ): CommandOutcome | null {
-  if (typeof record["status"] !== "string") return null;
+  const status = record["status"];
+  if (typeof status !== "string") return null;
   const output = record["output"];
   if (typeof output !== "string") return null;
   const exitCode = record["exit_code"];
+  const exitNote = record["exit_note"];
+  const totalBytes = record["output_total_bytes"];
+  const outputRef = record["output_ref"];
   return {
+    status,
     output,
     exitCode:
       typeof exitCode === "number" && Number.isInteger(exitCode)
         ? exitCode
         : null,
+    exitNote:
+      typeof exitNote === "string" && exitNote.trim() !== "" ? exitNote : null,
+    // `=== true`, never truthiness: a truncation claim is a claim about how
+    // much of the command's output the reader is NOT seeing, and "1" or "yes"
+    // arriving where a boolean was contracted means the producer changed shape
+    // rather than that the answer is yes.
+    truncated: record["truncated"] === true,
+    totalBytes:
+      typeof totalBytes === "number" &&
+      Number.isFinite(totalBytes) &&
+      totalBytes > 0
+        ? totalBytes
+        : null,
+    savedFullOutput: typeof outputRef === "string" && outputRef.trim() !== "",
   };
 }
 
@@ -694,18 +928,48 @@ const exitNonZeroStyle: CSSProperties = {
   color: "var(--color-warning)",
 };
 
+/**
+ * NEITHER of the other two, and that is the whole point of a third style.
+ *
+ * "Timed out", "cancelled", "refused", "unavailable" are not results the
+ * command reported — they are the app, the user, or a configured bound saying
+ * the process never got to report one. Success green would claim an outcome;
+ * amber would put a command that a policy correctly refused, or one the user
+ * chose to stop, in the same register as a failing test run. The quiet token
+ * says "no result" without editorialising about whose decision it was.
+ */
+const statusChipStyle: CSSProperties = {
+  ...exitChipStyle,
+  borderColor: "var(--color-border)",
+  color: "var(--color-text-muted)",
+};
+
 const commandOutputWrapStyle: CSSProperties = {
   display: "flex",
   flexDirection: "column",
   minWidth: 0,
 };
 
-/** The clip notice sits ABOVE the output, unlike the read/search footnotes: the
- *  tail is what was kept, so the reader meets "there was more before this"
- *  before the text rather than after scrolling to the bottom of it. Hence the
- *  border flips sides — the command row above already draws one. */
-const commandClipNoteStyle: CSSProperties = {
-  ...footNoteStyle,
+/** One quiet line of card-authored commentary — the runtime's exit note, the
+ *  truncation sentence, the line clip. Borderless on its own; the strip below
+ *  draws the single rule, so N notes never stack N hairlines. */
+const commandNoteStyle: CSSProperties = {
+  color: "var(--color-text-subtle)",
+  fontFamily: "var(--font-mono)",
+  fontSize: "var(--font-size-mono-9-5)",
+  lineHeight: "14px",
+  margin: 0,
+  padding: "5px 10px",
+};
+
+/** The exit note when output follows it, so the two are separated. Omitted when
+ *  it is the last thing in the panel — a bottom border there would draw a
+ *  second hairline directly above the panel's own. */
+const commandNoteRuledStyle: CSSProperties = {
+  ...commandNoteStyle,
+  borderBottom: "1px solid var(--color-border)",
+};
+
+const commandNoteStripStyle: CSSProperties = {
   borderBottom: "1px solid var(--color-border)",
-  borderTop: 0,
 };
diff --git a/services/ai-backend/src/agent_runtime/capabilities/desktop/broker_client.py b/services/ai-backend/src/agent_runtime/capabilities/desktop/broker_client.py
index f06ca7f86..ab337b2ed 100644
--- a/services/ai-backend/src/agent_runtime/capabilities/desktop/broker_client.py
+++ b/services/ai-backend/src/agent_runtime/capabilities/desktop/broker_client.py
@@ -423,6 +423,39 @@ class BrokerGrant(_BrokerModel):
     status: Literal["active", "revoked"] = "active"
     mount: str
     root: str | None = None
+    shell_enabled: bool = Field(default=False, alias="shellEnabled", strict=True)
+    """Whether the user enabled SHELL EXECUTION for this workspace.
+
+    PRD-shell-execution §7.3 puts the flag on the grant rather than on the app,
+    because "I trust the agent to run commands in ~/code/my-project" is a
+    different sentence from "I trust the agent to run commands". This field is
+    the runtime's whole read of that decision — §7.1's prerequisite 3 — taken off
+    the active-grant snapshot the worker already pins at run start. There is no
+    new broker verb, and specifically no execution verb: the broker reports what
+    the user chose, it does not run anything.
+
+    DEFAULTS TO FALSE, and that default is load-bearing three times over. The
+    model is ``extra="ignore"`` (see :class:`_BrokerModel`), so an Electron main
+    that predates the field sends nothing and lands here as ``False``. A grant
+    minted before the field existed decodes on the TypeScript side as ``false``
+    and is projected as ``false``. And ``toBrokerGrant`` omits it entirely for a
+    revoked or expired grant, which arrives here as ``False`` again. Every route
+    by which the fact can be missing therefore means "this workspace cannot run
+    commands" — never "it can".
+
+    ``strict=True`` IS THE POINT, and it is the one field on this model that
+    carries it. :class:`_BrokerModel` is lax, and Pydantic's lax bool coercion
+    accepts the JSON strings ``"true"``, ``"yes"``, ``"on"`` and the integer
+    ``1`` — so without it, a wire value of ``"yes"`` decoded to command
+    authority. That is fail-OPEN at the one boundary where the answer decides
+    whether an agent may run code on someone's machine. Strict here refuses
+    anything that is not a JSON ``true``/``false``, and a refusal is a
+    ``ValidationError`` the snapshot read already treats as a protocol failure —
+    i.e. no grants, therefore no command capability. Deliberately scoped to this
+    field rather than the model: making the whole model strict would change how
+    every other field decodes (``capturedAt`` would stop accepting an integer)
+    for reasons that have nothing to do with this flag.
+    """
 
 
 class BrokerGrantSnapshot(_BrokerModel):
diff --git a/services/ai-backend/src/agent_runtime/capabilities/desktop/host_filesystem.py b/services/ai-backend/src/agent_runtime/capabilities/desktop/host_filesystem.py
index b2f2166bb..2e078cba0 100644
--- a/services/ai-backend/src/agent_runtime/capabilities/desktop/host_filesystem.py
+++ b/services/ai-backend/src/agent_runtime/capabilities/desktop/host_filesystem.py
@@ -169,6 +169,26 @@ class GrantedRoot:
 
     path: str
     writable: bool = True
+    shell_enabled: bool = False
+    """Whether the user enabled SHELL EXECUTION for this workspace (§7.3).
+
+    The terminus of the per-workspace enablement read path: Settings toggle →
+    ``GrantStore`` → ``toBrokerGrant`` → ``/v1/grants/snapshot`` →
+    ``BrokerGrant.shell_enabled`` → ``WorkspaceMount`` → here.
+
+    SEPARATE FROM ``writable``, and defaulted OFF. PRD §7.1 requires FOUR
+    independent things before ``run_command`` exists at all — the process env
+    flag, a writable granted root, THIS field on that root's grant, and the
+    ``single_user_desktop`` profile — and a tool factory has to be able to check
+    each one. Folding this into ``writable`` would collapse two of the four into
+    one and make "the user turned commands on for this folder" unobservable.
+
+    It changes NOTHING about the filesystem rules. ``HostFilesystemRules.build``
+    and ``HostFilesystemFloor`` never read it; a command is invisible to every
+    path-keyed control we own (§2.2), which is exactly why enablement is a
+    separate, explicit, per-workspace decision rather than a consequence of
+    file access.
+    """
 
     def __post_init__(self) -> None:
         if not self.path.startswith("/"):
@@ -180,7 +200,9 @@ def __post_init__(self) -> None:
             raise ValueError(f"granted root must not contain '..': {self.path!r}")
 
     @classmethod
-    def from_host_path(cls, path: str, *, writable: bool = True) -> GrantedRoot:
+    def from_host_path(
+        cls, path: str, *, writable: bool = True, shell_enabled: bool = False
+    ) -> GrantedRoot:
         r"""Build a root from a host path in EITHER platform's grammar.
 
         `__post_init__` demands the POSIX-shaped canonical spelling because that
@@ -199,7 +221,11 @@ def from_host_path(cls, path: str, *, writable: bool = True) -> GrantedRoot:
         classified = HostPathClassifier.classify(path)
         if not classified.is_host:
             raise ValueError(f"granted root is not a host folder: {path!r}")
-        return cls(path=classified.canonical, writable=writable)
+        return cls(
+            path=classified.canonical,
+            writable=writable,
+            shell_enabled=shell_enabled,
+        )
 
     def glob(self) -> str:
         """Match the root itself and everything beneath it."""
diff --git a/services/ai-backend/src/agent_runtime/capabilities/desktop/workspace_backend.py b/services/ai-backend/src/agent_runtime/capabilities/desktop/workspace_backend.py
index ebe258e52..140c72406 100644
--- a/services/ai-backend/src/agent_runtime/capabilities/desktop/workspace_backend.py
+++ b/services/ai-backend/src/agent_runtime/capabilities/desktop/workspace_backend.py
@@ -262,6 +262,13 @@ class WorkspaceMount:
     label: str | None = None
     mode: GrantMode = "read_only"
     host_root: str | None = None
+    #: Whether the user enabled SHELL EXECUTION for this workspace
+    #: (PRD-shell-execution §7.3). Carried through from ``BrokerGrant`` so the
+    #: run's granted roots can report it; it is INDEPENDENT of ``mode``, because
+    #: file access and command execution are different authorities and neither
+    #: is a superset of the other. Defaults to ``False`` — a mount built by any
+    #: caller that does not know about the flag is command-incapable.
+    shell_enabled: bool = False
 
     def __post_init__(self) -> None:
         """Reject empty or separator-bearing mount names — they must be one segment."""
@@ -347,6 +354,10 @@ def _mount_for(cls, grant: BrokerGrant, *, name: str) -> WorkspaceMount | None:
                 grant_id=grant.grant_id,
                 label=grant.label or None,
                 mode=grant.mode,
+                # PRD-shell-execution §7.3 prerequisite 3, verbatim off the
+                # snapshot. Nothing is derived here — a mount is command-capable
+                # exactly when the grant the user decided about says so.
+                shell_enabled=grant.shell_enabled,
                 # The grant's real host root, when the broker sends one.
                 # This is what `HostFilesystemRules` turns into an `allow`
                 # rule, so a folder the user explicitly attached stops
@@ -368,6 +379,7 @@ def _mount_for(cls, grant: BrokerGrant, *, name: str) -> WorkspaceMount | None:
                 grant_id=grant.grant_id,
                 label=grant.label or None,
                 mode=grant.mode,
+                shell_enabled=grant.shell_enabled,
                 host_root=None,
             )
         except ValueError:
@@ -412,6 +424,13 @@ def granted_roots(cls, mounts: Sequence[WorkspaceMount]) -> tuple[object, ...]:
                         # Only a grant that actually permits writing may site a
                         # writable scratch dir; read-only stays read-only.
                         writable=mount.mode != "read_only",
+                        # Kept SEPARATE from ``writable`` on purpose. PRD §7.1
+                        # lists "a writable granted root" and "that grant
+                        # carries shell enablement" as two INDEPENDENT
+                        # prerequisites, and the tool factory requires both;
+                        # folding them together here would quietly make one of
+                        # the four prerequisites unobservable.
+                        shell_enabled=mount.shell_enabled,
                     )
                 )
             except ValueError:
diff --git a/services/ai-backend/src/agent_runtime/capabilities/operations/builtin_operation_catalog.json b/services/ai-backend/src/agent_runtime/capabilities/operations/builtin_operation_catalog.json
index 7f1207b7c..5757ce283 100644
--- a/services/ai-backend/src/agent_runtime/capabilities/operations/builtin_operation_catalog.json
+++ b/services/ai-backend/src/agent_runtime/capabilities/operations/builtin_operation_catalog.json
@@ -89,6 +89,15 @@
     "execution": "gateway",
     "model_visible": true
   },
+  {
+    "tool_name": "run_command",
+    "capability": "builtin",
+    "op": "run_command",
+    "source": "capabilities.shell.run_command_tool",
+    "kind": "builtin",
+    "execution": "internal",
+    "model_visible": true
+  },
   {
     "tool_name": "stage_rowset_write",
     "capability": "builtin",
diff --git a/services/ai-backend/src/agent_runtime/capabilities/operations/conformance.py b/services/ai-backend/src/agent_runtime/capabilities/operations/conformance.py
index 046479bd7..b2a3ebd4a 100644
--- a/services/ai-backend/src/agent_runtime/capabilities/operations/conformance.py
+++ b/services/ai-backend/src/agent_runtime/capabilities/operations/conformance.py
@@ -192,6 +192,7 @@ def current_capability_registrations() -> tuple[CapabilityRegistration, ...]:
         ("builtin", "run_code_mode", "execution.factory._model_visible_tools"),
         ("builtin", "run_tool_program", "execution.factory._model_visible_tools"),
         ("builtin", "run_in_sandbox", "execution.factory._model_visible_tools"),
+        ("builtin", "run_command", "execution.factory._model_visible_tools"),
         ("builtin", "stage_rowset_write", "execution.factory._model_visible_tools"),
         ("builtin", "write_todos", "langchain.agents.middleware.TodoListMiddleware"),
         ("builtin", "execute", "deepagents.middleware.filesystem.FilesystemMiddleware"),
diff --git a/services/ai-backend/src/agent_runtime/capabilities/operations/operation_descriptors.json b/services/ai-backend/src/agent_runtime/capabilities/operations/operation_descriptors.json
index ed001b722..f74ce830d 100644
--- a/services/ai-backend/src/agent_runtime/capabilities/operations/operation_descriptors.json
+++ b/services/ai-backend/src/agent_runtime/capabilities/operations/operation_descriptors.json
@@ -193,6 +193,23 @@
     "timeout_ms": 120000,
     "unknown_arguments_tighten_to_unknown": true
   },
+  {
+    "descriptor": {
+      "capability": "builtin",
+      "op": "run_command",
+      "executor": "builtin",
+      "effect_class": "internal_reversible",
+      "result_kind": "activity",
+      "supports_prepare": false,
+      "supports_reconcile": false,
+      "required_gate_kinds": ["capability", "policy"],
+      "max_inline_result_bytes": 65536
+    },
+    "descriptor_version": "2026-07-24.1",
+    "display_name": "shell command",
+    "timeout_ms": 600000,
+    "unknown_arguments_tighten_to_unknown": true
+  },
   {
     "descriptor": {
       "capability": "builtin",
diff --git a/services/ai-backend/src/agent_runtime/capabilities/shell/__init__.py b/services/ai-backend/src/agent_runtime/capabilities/shell/__init__.py
new file mode 100644
index 000000000..4c32e9691
--- /dev/null
+++ b/services/ai-backend/src/agent_runtime/capabilities/shell/__init__.py
@@ -0,0 +1,30 @@
+"""Shell execution (``run_command``) — the contracts, the environment, the executor.
+
+PRD: ``docs/plan/agent-execution/PRD-shell-execution.md``. This package is Phase
+1's core: the model-facing schema and result, the deployment-trusted
+configuration, the allowlist-built child environment, and the one subprocess
+call site in ``agent_runtime``.
+
+**The capability is OFF unless ``RUNTIME_ENABLE_SHELL_EXECUTION`` says otherwise.**
+
+**Every command asks.** There is no auto-approve path in this phase and
+``irreversible: true`` rides every approval card, so no command is approvable in
+one click. The one exception to "asks" is the direction that costs the command
+rather than a click: a never-list hit returns a refusal with **no approval card
+at all**, so there is nothing to click through.
+
+**No OS-level sandboxing exists in this phase, and this package does not pretend
+otherwise.** A command runs as the user, with the user's permissions and the
+user's real ``HOME``, and the network is unrestricted. The bound working
+directory constrains where a command *starts*, not where it can reach. Those
+residual risks are documented in-product and deferred to Phase 2 (scratch
+``HOME`` plus SPIKE-S1's per-platform confinement). Half-building a sandbox here
+would be worse than not having one, because a partial confinement reads as a
+boundary.
+
+There is deliberately no re-export block and no per-module recap here: every
+consumer imports from the submodule that authors the symbol, so both would be a
+second place to edit that no reader reaches and nothing keeps honest.
+"""
+
+from __future__ import annotations
diff --git a/services/ai-backend/src/agent_runtime/capabilities/shell/config.py b/services/ai-backend/src/agent_runtime/capabilities/shell/config.py
new file mode 100644
index 000000000..6ed64eba1
--- /dev/null
+++ b/services/ai-backend/src/agent_runtime/capabilities/shell/config.py
@@ -0,0 +1,284 @@
+"""Deployment-trusted configuration for ``run_command`` (PRD-shell-execution §4.4).
+
+Everything the model is forbidden to influence — whether commands can run at
+all, the timeout ceiling, the output cap, the per-run command budget, and which
+shell binary is invoked — is resolved here, **once, from the process
+environment**. The model never reaches this module, and there is deliberately no
+per-request override for any of it: a request-supplied environment is a way to
+smuggle a credential into a child process, and a request-supplied shell is a way
+to reintroduce rc-file sourcing.
+
+**This is not a second settings mechanism and must not be folded into
+``RuntimeSettings``**, which is the *process* composition root — merged once at
+boot and injected everywhere. A capability config gates whether one capability
+exists at all, so it must resolve from a bare mapping in a test and its parse
+failure must disable that capability rather than refuse to boot the service.
+Twelve other ``from_env`` capability configs in ``agent_runtime`` resolve theirs
+the same way (``sandbox``, ``interpreter``, ``desktop``, ``mcp.credentials``,
+``surfaces``, ``api.connector_policy_client``, …): this is the house pattern,
+not a fork of it — and unlike ``sandbox.config`` it keeps its readers inside the
+class, per this service's "no module-level helper functions" rule.
+
+Gating: the capability is OFF unless ``RUNTIME_ENABLE_SHELL_EXECUTION`` holds an
+enabling token. The parse fails closed in both directions — an unrecognised
+enable token leaves it disabled, and **any** malformed numeric override disables
+the capability outright rather than silently falling back to a default.
+
+The three numbers are the ones this repo already uses, so there is one house
+answer rather than two: 120 s matches ``RemoteSandboxConfig.command_timeout_s``,
+64 KiB matches ``combined_command_preview_bytes`` and the ``execute``
+descriptor's ``max_inline_result_bytes``, and 64 commands matches PRD-FS-08's
+per-session budget.
+"""
+
+from __future__ import annotations
+
+from collections.abc import Mapping
+import os
+from typing import Final
+
+from pydantic import Field, ValidationError, model_validator
+
+from agent_runtime.capabilities.shell.contracts import (
+    ShellRefusal,
+    ShellRefusalReason,
+    ShellRefusedError,
+)
+from agent_runtime.execution.contracts import RuntimeContract
+
+
+class _EnvFields:
+    """Environment variable names (single source of truth).
+
+    There is deliberately no ``RUNTIME_SHELL_PATH``. ``shell_path`` is a
+    constructor field so a test can point at a stub, but it is never derived
+    from the environment: an env-settable shell is one export away from being
+    the user's login shell, which is the exact thing §11.4 refuses.
+    """
+
+    ENABLE: Final = "RUNTIME_ENABLE_SHELL_EXECUTION"
+    DEFAULT_TIMEOUT_S: Final = "RUNTIME_SHELL_DEFAULT_TIMEOUT_S"
+    MAX_TIMEOUT_S: Final = "RUNTIME_SHELL_MAX_TIMEOUT_S"
+    OUTPUT_PREVIEW_BYTES: Final = "RUNTIME_SHELL_OUTPUT_PREVIEW_BYTES"
+    MAX_COMMANDS_PER_RUN: Final = "RUNTIME_SHELL_MAX_COMMANDS_PER_RUN"
+
+    #: Model field → environment variable, for every numeric override. The
+    #: defaults are deliberately absent: they live on the fields themselves, so
+    #: ``from_env`` omits an unset key rather than re-stating its value.
+    NUMERIC: Final = {
+        "default_timeout_s": DEFAULT_TIMEOUT_S,
+        "max_timeout_s": MAX_TIMEOUT_S,
+        "combined_output_preview_bytes": OUTPUT_PREVIEW_BYTES,
+        "max_commands_per_run": MAX_COMMANDS_PER_RUN,
+    }
+
+    #: Mirrors ``RemoteSandboxConfig``'s enabling tokens, so one spelling turns
+    #: on either capability. Copied rather than imported: importing would make
+    #: ``shell`` depend on ``sandbox`` for four string literals. The real fix is
+    #: one shared token set for all 18 copies in this service (``settings.
+    #: _BOOL_TRUTHY`` among them), which is a change to files this module does
+    #: not own.
+    TRUTHY: Final = frozenset({"1", "true", "yes", "on"})
+
+
+class _Note:
+    """Model-facing refusal sentences, authored rather than interpolated.
+
+    They name the real constraint — §4.2's argument for refusing a too-large
+    timeout instead of clamping it is precisely that the model learns the
+    ceiling in one turn instead of concluding from a surprise timeout that the
+    command is broken. The closed ``reason`` code, not this prose, is what the
+    product contract is made of.
+    """
+
+    DISABLED: Final = (
+        "Running commands is turned off for this workspace. Nothing was run."
+    )
+    BUDGET_EXHAUSTED: Final = (
+        "This run has used its whole command allowance ({limit} commands). "
+        "Nothing was run."
+    )
+    TIMEOUT_TOO_LARGE: Final = (
+        "timeout_s={requested} is above the maximum of {maximum} seconds. "
+        "Nothing was run - ask again with timeout_s at or below {maximum}, or "
+        "run something shorter."
+    )
+
+
+class ShellExecutionConfig(RuntimeContract):
+    """Resolved, deployment-trusted shell-execution configuration for one process.
+
+    ``enabled=False`` means the capability is absent: no tool is registered, the
+    model is told in its prompt that it has no shell, and nothing here is
+    reachable.
+    """
+
+    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)
+    #: NOT the user's ``$SHELL``, and not a login shell. A login shell sources
+    #: ``~/.zshrc``, which is arbitrary user code running with the child's
+    #: environment and able to re-export everything the allowlist just removed.
+    #: Consequence to state out loud in the tool description: aliases and shell
+    #: functions are unavailable — ``ll`` will not work, ``ls -la`` will.
+    shell_path: str = Field(default="/bin/sh", min_length=1)
+
+    @model_validator(mode="after")
+    def _check_ceiling_is_above_default(self) -> "ShellExecutionConfig":
+        """A default above the ceiling would refuse every call that omits a timeout."""
+
+        if self.default_timeout_s > self.max_timeout_s:
+            raise ValueError(
+                "default_timeout_s must not exceed max_timeout_s; "
+                "a default above the ceiling refuses every unqualified command"
+            )
+        return self
+
+    @classmethod
+    def from_env(
+        cls, environ: Mapping[str, str] | None = None
+    ) -> "ShellExecutionConfig":
+        """Resolve config from the environment, failing closed on bad input.
+
+        Two independent closes:
+
+        * an enable value that is not one of :attr:`_EnvFields.TRUTHY` leaves
+          ``enabled=False``;
+        * any numeric override that is unparseable, out of range, or mutually
+          inconsistent yields a **disabled** config rather than a partly-applied
+          one. A deployment that meant to lower the ceiling and typoed the value
+          gets no shell, not the default ceiling it never asked for.
+        """
+
+        source = environ if environ is not None else os.environ
+        enabled = (source.get(_EnvFields.ENABLE) or "").strip().lower() in (
+            _EnvFields.TRUTHY
+        )
+        if not enabled:
+            return cls()
+        try:
+            overrides: dict[str, int] = {
+                field: value
+                for field, key in _EnvFields.NUMERIC.items()
+                if (value := cls._read_int(source, key)) is not None
+            }
+            return cls(enabled=True, **overrides)
+        except (ValueError, TypeError, ValidationError):
+            # Fail closed: an unreadable limit is not a limit.
+            return cls()
+
+    @staticmethod
+    def _read_int(source: Mapping[str, str], key: str) -> int | None:
+        """Parse one integer override, or ``None`` when the variable is unset.
+
+        Raising on anything malformed is what :meth:`from_env` converts into a
+        disabled config. ``None`` for "unset" is deliberate: the field's own
+        ``Field(default=...)`` then applies, so each of the four numbers is
+        written in exactly one place instead of once here and once there.
+        """
+
+        raw = (source.get(key) or "").strip()
+        return int(raw) if raw else None
+
+    def resolve_timeout_s(self, requested: int | None) -> int:
+        """Return the timeout for one call, refusing anything above the ceiling.
+
+        Refused, never clamped (§4.2). A silent clamp means a model that asked
+        for thirty minutes and got two concludes from the timeout that the
+        command is broken and retries it; a refusal tells it the real constraint
+        in one turn.
+        """
+
+        if requested is None:
+            return self.default_timeout_s
+        if requested > self.max_timeout_s:
+            raise ShellRefusedError(
+                ShellRefusal.refused(
+                    ShellRefusalReason.TIMEOUT_ABOVE_MAXIMUM,
+                    _Note.TIMEOUT_TOO_LARGE.format(
+                        requested=requested, maximum=self.max_timeout_s
+                    ),
+                )
+            )
+        return requested
+
+    def require_enabled(self) -> None:
+        """Raise the typed refusal when commands are off for this deployment.
+
+        A second check beside the tool being absent from the toolset: the tool
+        is not registered when the capability is off, so this only fires if some
+        future call path reaches the executor without going through
+        registration. It fails closed rather than assuming registration is the
+        only door.
+        """
+
+        if not self.enabled:
+            raise ShellRefusedError(
+                ShellRefusal.unavailable(
+                    ShellRefusalReason.SHELL_EXECUTION_DISABLED, _Note.DISABLED
+                )
+            )
+
+
+class ShellCommandBudget:
+    """The per-run command allowance, as a counter that can only go one way.
+
+    Run-scoped and in-memory: the budget bounds one run's spawns, so it is
+    created beside the run and dies with it. Deliberately mutable — it is the
+    one thing in this package that is not a frozen contract, because it is a
+    consumable rather than a value.
+    """
+
+    def __init__(self, limit: int) -> None:
+        self._limit = limit
+        self._spent = 0
+
+    @property
+    def limit(self) -> int:
+        """How many commands this run may spawn in total."""
+
+        return self._limit
+
+    @property
+    def spent(self) -> int:
+        """How many have been spawned so far."""
+
+        return self._spent
+
+    @property
+    def remaining(self) -> int:
+        """How many are left. Never negative."""
+
+        return max(self._limit - self._spent, 0)
+
+    def require_available(self) -> None:
+        """Raise the typed refusal when the run has no allowance left.
+
+        The read half of :meth:`consume`, so the caller can check before a human
+        is shown a card and claim only after the decision — asking someone to
+        approve a command the run has no allowance left to spawn is a card that
+        cannot lead anywhere. Lives here rather than at the call site because
+        ``spent >= limit`` is the counter's own business; a caller that had to
+        spell it out would be a second place to get the comparison wrong.
+        """
+
+        if self._spent >= self._limit:
+            raise ShellRefusedError(
+                ShellRefusal.refused(
+                    ShellRefusalReason.COMMAND_BUDGET_EXHAUSTED,
+                    _Note.BUDGET_EXHAUSTED.format(limit=self._limit),
+                )
+            )
+
+    def consume(self) -> None:
+        """Claim one command, or raise the typed refusal when the run is out.
+
+        Claimed *before* the spawn, so a command that hangs and is killed still
+        counts. Counting completions instead would let a run that times out
+        repeatedly spawn without bound.
+        """
+
+        self.require_available()
+        self._spent += 1
diff --git a/services/ai-backend/src/agent_runtime/capabilities/shell/contracts.py b/services/ai-backend/src/agent_runtime/capabilities/shell/contracts.py
new file mode 100644
index 000000000..4979b8d18
--- /dev/null
+++ b/services/ai-backend/src/agent_runtime/capabilities/shell/contracts.py
@@ -0,0 +1,573 @@
+"""Typed contracts for the ``run_command`` capability (PRD-shell-execution §4.1-§4.3).
+
+This module is the single source of truth for what the model may say and what it
+is told back. Every value that crosses the tool boundary is a frozen
+``RuntimeContract``, so model output is coerced and validated at the edge rather
+than trusted. Each structural property is argued once, on the class or validator
+that enforces it, rather than a second time here.
+
+WHY THIS IS NOT ``deepagents.backends.protocol.ExecuteResponse``
+---------------------------------------------------------------
+Upstream ships a result shape for exactly this job, and it was checked rather
+than assumed. ``ExecuteResponse`` is a three-field plain dataclass — ``output``,
+``exit_code``, ``truncated`` — with no validation and, decisively, **no status
+field**. ``LocalShellBackend.execute`` therefore has nowhere to put "how this
+ended", so it encodes a timeout as ``exit_code=124`` plus the English sentence
+``"Error: Command timed out after N seconds..."`` inside ``output``, and a failed
+spawn as ``exit_code=1`` plus ``f"Error executing command ({type(e).__name__}):
+{e}"``. Those are the two things this package exists to refuse: a model inferring
+failure from prose (§4.3, AC1.3), and an exception interpolated into
+model-visible text. A command that legitimately printed ``Error:`` would be
+indistinguishable from a runtime that never spawned one.
+
+So ours is a superset, and these are the added fields rather than a restyled
+copy. :class:`ShellExecutionOutcome` adds ``status`` (the five-way outcome),
+``output_total_bytes`` (the true total, which is what lets a truncation notice
+say "the last 64 KiB of 4.2 MB" honestly after those bytes are gone),
+``duration_ms``, and the two spill flags. :class:`RunCommandResult` adds, beyond
+those, ``reason`` — a closed ten-code refusal taxonomy — plus ``exit_note``,
+``workspace`` (the label, never the host path) and ``output_ref``. Nothing here
+re-declares a field ``ExecuteResponse`` already carries usefully; the three it
+does carry are kept under their upstream names on purpose.
+
+**Two shapes look like each other and must not be merged.**
+:class:`ShellExecutionRequest` / :class:`ShellExecutionOutcome` are the
+*executor's* IO, one layer below :class:`RunCommandResult`. The overlap is six
+field names, and it is not duplication: the executor knows no workspace label,
+no scratch layout and no policy decision, and its ``output_total_bytes`` is an
+unconditional count while the result's is half of a truncation notice and absent
+without one. Collapsing them would push labels and refusal codes into the one
+module that must stay able to just spawn, bound and reap.
+"""
+
+from __future__ import annotations
+
+from enum import StrEnum
+from pathlib import Path
+from typing import Final
+
+from pydantic import ConfigDict, Field, field_validator, model_validator
+
+from agent_runtime.execution.contracts import RuntimeContract
+
+
+class _Text:
+    """Character-class constants. Kept off the Pydantic models on purpose.
+
+    An underscore-prefixed class attribute on a ``BaseModel`` is claimed by
+    Pydantic as a private attribute, so shared constants live on a plain class
+    (the same reason ``settings.py`` keeps ``_EXECUTION`` at module level).
+    """
+
+    #: The only C0 control characters a command may contain. Everything else in
+    #: ``0x00``-``0x1F`` is refused: NUL truncates at the exec boundary, and the
+    #: rest are invisible in the approval card.
+    ALLOWED_CONTROL: Final = frozenset("\t\n\r")
+
+    #: Exclusive upper bound of the C0 range.
+    C0_CEILING: Final = " "
+
+    MAX_COMMAND_CHARS: Final = 8192
+    MAX_LABEL_CHARS: Final = 128
+
+    @classmethod
+    def reject_control_characters(cls, value: str, *, control: str, blank: str) -> str:
+        """Refuse the C0 range bar tab/newline/CR, and a whitespace-only value.
+
+        One scan, applied to both the command and the workspace label: they are
+        the same rule with different messages, and a second copy of a
+        security-relevant scan is how a fix lands on one of them only.
+        """
+
+        for character in value:
+            if character < cls.C0_CEILING and character not in cls.ALLOWED_CONTROL:
+                raise ValueError(control)
+        if not value.strip():
+            raise ValueError(blank)
+        return value
+
+
+class _Message:
+    """Safe, value-free validation messages.
+
+    None of these quote the offending value. A command is model output and may
+    carry content ingested from a connector; a validation error is one of the
+    places that content would otherwise reach a log line.
+    """
+
+    CONTROL_CHARACTERS: Final = (
+        "command contains a control character; only tab, newline and carriage "
+        "return are permitted"
+    )
+    BLANK_COMMAND: Final = "command must contain something other than whitespace"
+    LABEL_CONTROL_CHARACTERS: Final = "workspace label contains a control character"
+    BLANK_LABEL: Final = "workspace label must not be blank"
+    EXIT_CODE_ON_NON_COMPLETED: Final = (
+        "exit_code is set only for a completed command; every other status "
+        "reports exit_code=None"
+    )
+    MISSING_EXIT_CODE: Final = "a completed command must report an exit_code"
+    REASON_REQUIRED: Final = "refused and unavailable results must carry a reason"
+    REASON_FORBIDDEN: Final = "reason is set only for refused or unavailable results"
+    OUTPUT_ON_REFUSAL: Final = (
+        "a refused or unavailable call ran no process and therefore has no output"
+    )
+    TOTAL_BYTES_REQUIRES_TRUNCATION: Final = (
+        "output_total_bytes and output_ref describe a truncation and are set "
+        "only when truncated is true"
+    )
+    TRUNCATION_REQUIRES_TOTAL: Final = (
+        "a truncated result must report output_total_bytes"
+    )
+    WORKSPACE_REQUIRED: Final = (
+        "a command that reached a process must name the workspace it ran in"
+    )
+    REFUSAL_STATUS: Final = (
+        "a refusal describes a call that never spawned; its status must be "
+        "refused or unavailable"
+    )
+    OUTCOME_STATUS: Final = (
+        "an execution outcome describes a call that spawned; its status must be "
+        "completed, timeout or cancelled"
+    )
+    OUTPUT_REF_NOT_VIRTUAL: Final = (
+        "output_ref must be a virtual agent-scratch path, never a host path"
+    )
+    CWD_NOT_ABSOLUTE: Final = "cwd must be an absolute directory path"
+
+
+class ShellContract(RuntimeContract):
+    """``RuntimeContract`` plus ``hide_input_in_errors``, for one reason.
+
+    Every model in this package holds text nobody in this repository wrote: a
+    command is model output that may have been shaped by content ingested from a
+    connector, and command output is whatever the process printed. Pydantic
+    renders ``input_value=...`` into ``str(ValidationError)``, so without this a
+    validation failure is a path by which that text reaches a log line — the one
+    place §13's "length-clipped in logs, never value-scanned" precedent would be
+    silently violated, because a traceback is not a log call anybody audits.
+
+    ``model_config`` is merged across the MRO, so ``extra="forbid"``,
+    ``frozen=True`` and ``validate_assignment=True`` are all still in force;
+    a test pins that rather than trusting the merge.
+    """
+
+    model_config = ConfigDict(hide_input_in_errors=True)
+
+
+class ShellExecutionStatus(StrEnum):
+    """How one ``run_command`` call ended.
+
+    Wire values are exactly the PRD's ``Literal`` set; the enum is the house form
+    (``CLAUDE.md``: "use enums, literals, constrained strings") and serialises
+    identically.
+    """
+
+    #: The process ran to completion and reported an exit status. This says
+    #: nothing about success — a non-zero ``exit_code`` is still ``completed``.
+    COMPLETED = "completed"
+    #: The process outlived its timeout and its process group was killed.
+    TIMEOUT = "timeout"
+    #: The run was cancelled while the process was live; the group was killed.
+    CANCELLED = "cancelled"
+    #: Nothing was spawned, and nothing could have been: a floor decision.
+    REFUSED = "refused"
+    #: Nothing was spawned because the capability was not available for this
+    #: call (disabled, no writable workspace, grant detached, spawn failed).
+    UNAVAILABLE = "unavailable"
+
+
+class ShellStatusGroups:
+    """The two halves of :class:`ShellExecutionStatus`, by whether a process ran.
+
+    A plain class rather than members on the enum: an annotated assignment in an
+    ``Enum`` body is still an assignment, so a ``frozenset`` there would be
+    coerced into an enum member and a ``StrEnum`` would raise at import.
+    """
+
+    #: Statuses that reached a real process and therefore carry a workspace
+    #: label and (possibly partial) output.
+    SPAWNED: Final = frozenset(
+        {
+            ShellExecutionStatus.COMPLETED,
+            ShellExecutionStatus.TIMEOUT,
+            ShellExecutionStatus.CANCELLED,
+        }
+    )
+
+    #: Statuses that never reached a process and therefore carry a ``reason``.
+    NOT_SPAWNED: Final = frozenset(
+        {ShellExecutionStatus.REFUSED, ShellExecutionStatus.UNAVAILABLE}
+    )
+
+    @staticmethod
+    def check_exit_code(status: ShellExecutionStatus, exit_code: int | None) -> None:
+        """Raise unless ``exit_code`` is present for exactly ``COMPLETED``.
+
+        One invariant, read by both the executor's outcome and the model-facing
+        result. Stated once because two copies are how the two shapes drift
+        into disagreeing about what a timeout reports.
+        """
+
+        if status is ShellExecutionStatus.COMPLETED:
+            if exit_code is None:
+                raise ValueError(_Message.MISSING_EXIT_CODE)
+        elif exit_code is not None:
+            raise ValueError(_Message.EXIT_CODE_ON_NON_COMPLETED)
+
+
+class ShellRefusalReason(StrEnum):
+    """Closed vocabulary for why a call produced no process.
+
+    Closed on purpose (§4.3): a free-form reason is a way for deployment
+    configuration to leak into model-visible output. The prose that *does* reach
+    the model is :attr:`ShellRefusal.note`, which is authored here, never
+    interpolated from an exception.
+    """
+
+    #: ``RUNTIME_ENABLE_SHELL_EXECUTION`` is not set for this deployment.
+    SHELL_EXECUTION_DISABLED = "shell_execution_disabled"
+    #: No attached folder is writable, so there is nothing to bind a cwd to.
+    NO_WRITABLE_WORKSPACE = "no_writable_workspace"
+    #: The ``workspace`` label is not one of the bound labels. Never falls back
+    #: to a default root (§4.2).
+    UNKNOWN_WORKSPACE = "unknown_workspace"
+    #: The grant went away between run start and this call (§16.1).
+    WORKSPACE_UNAVAILABLE = "workspace_unavailable"
+    #: ``timeout_s`` exceeded ``max_timeout_s``. Refused, never clamped (§4.2).
+    TIMEOUT_ABOVE_MAXIMUM = "timeout_above_maximum"
+    #: The command is on the never-list. Unappealable: no card is created (§9.3).
+    COMMAND_NOT_PERMITTED = "command_not_permitted"
+    #: The run used its whole ``max_commands_per_run`` allowance (§4.4).
+    COMMAND_BUDGET_EXHAUSTED = "command_budget_exhausted"
+    #: The process could not be started at all (missing shell, unusable cwd).
+    EXECUTION_UNAVAILABLE = "execution_unavailable"
+    #: A human was shown the command and declined it (§8). Distinct from
+    #: :attr:`COMMAND_NOT_PERMITTED` on purpose: that one is a floor and is
+    #: unappealable, this one is a person's answer to this call and says nothing
+    #: about the next one. Collapsing the two would tell a model to retry an
+    #: unappealable refusal, or to give up after a decline the user might
+    #: reverse — the exact failure a coarsened error taxonomy produces.
+    COMMAND_DECLINED = "command_declined"
+    #: The command needed approval in a run with no approval channel wired, so
+    #: the GATE failed closed. A configuration fact, not a policy judgement, and
+    #: permanent for this run.
+    COMMAND_APPROVAL_UNAVAILABLE = "command_approval_unavailable"
+
+
+class RunCommandInput(ShellContract):
+    """Model-facing schema. Command text and an opaque workspace label.
+
+    Never a path, never an environment variable, never a timeout above the
+    configured cap. ``RuntimeContract`` supplies ``extra="forbid", frozen=True``:
+    a model that invents ``env=`` or ``cwd=`` must fail loudly at the boundary,
+    not have it silently dropped, because the approval card is built from the
+    validated arguments and would never show the field the human is approving.
+    """
+
+    command: str = Field(
+        min_length=1,
+        max_length=_Text.MAX_COMMAND_CHARS,
+        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 && ...`. Shell aliases "
+            "and functions are not available."
+        ),
+    )
+
+    workspace: str | None = Field(
+        default=None,
+        max_length=_Text.MAX_LABEL_CHARS,
+        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 tab / newline / carriage-return 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.
+
+        The value is never echoed back — see :class:`_Message`. Note what is
+        deliberately *not* rejected: ``0x7F`` (DEL) and the zero-width /
+        bidirectional Unicode characters are still permitted, because they do
+        not truncate at ``execve`` and refusing them here would be a rendering
+        rule enforced in the wrong layer. §16.3 owns the rendering half.
+        """
+
+        return _Text.reject_control_characters(
+            value,
+            control=_Message.CONTROL_CHARACTERS,
+            blank=_Message.BLANK_COMMAND,
+        )
+
+    @field_validator("workspace")
+    @classmethod
+    def _reject_label_control_characters(cls, value: str | None) -> str | None:
+        """Hold the label to the same rule as the command.
+
+        Stricter than §4.2, which validates only ``command``. A label is
+        rendered on the approval card beside the command (AC2.3), so an
+        invisible character in it has the same "card lies to the human" shape.
+        The label is resolved against a closed set of bound labels afterwards;
+        this only keeps an unrenderable one from reaching that resolution.
+        """
+
+        if value is None:
+            return None
+        return _Text.reject_control_characters(
+            value,
+            control=_Message.LABEL_CONTROL_CHARACTERS,
+            blank=_Message.BLANK_LABEL,
+        )
+
+
+class RunCommandResult(ShellContract):
+    """What the model is told. Every field is either a runtime fact or absent.
+
+    Returned as a JSON string rather than prose, so a model learns that
+    something failed by reading ``exit_code`` instead of parsing English.
+
+    ``workspace`` is the **label**. The host-absolute path is not in the result,
+    not in the event payload, and not in the transcript — the same rule the
+    sandbox lane holds itself to for its own event sink.
+
+    One deviation from §4.3, made deliberately: ``workspace`` defaults to ``""``
+    rather than being unconditionally required, because a refusal can happen
+    before any workspace is bound and requiring it there would force the code to
+    invent a label. :meth:`_check_status_invariants` keeps the real rule.
+    """
+
+    status: ShellExecutionStatus
+    exit_code: int | None = None
+    output: str = ""
+    truncated: bool = False
+    output_ref: str | None = Field(default=None, max_length=1024)
+    output_total_bytes: int | None = Field(default=None, ge=0)
+    duration_ms: int = Field(default=0, ge=0)
+    workspace: str = Field(default="", max_length=_Text.MAX_LABEL_CHARS)
+    exit_note: str | None = Field(default=None, max_length=2048)
+    reason: ShellRefusalReason | None = None
+
+    @field_validator("output_ref")
+    @classmethod
+    def _reject_host_shaped_ref(cls, value: str | None) -> str | None:
+        """Reject the two spellings that are host paths on their face.
+
+        A Windows drive letter (``C:/...``) or a UNC prefix (``//host/share``)
+        cannot be a virtual agent-scratch path. This is a guard, not a proof:
+        the caller that mints ``output_ref`` owns the invariant, and a POSIX
+        host path is indistinguishable from a virtual one at this layer. Said
+        plainly rather than implied, so nobody reads the validator as the
+        control.
+        """
+
+        if value is None:
+            return None
+        candidate = value.replace("\\", "/")
+        drive_shaped = len(candidate) >= 2 and candidate[1] == ":"
+        if drive_shaped or candidate.startswith("//"):
+            raise ValueError(_Message.OUTPUT_REF_NOT_VIRTUAL)
+        return value
+
+    @model_validator(mode="after")
+    def _check_status_invariants(self) -> "RunCommandResult":
+        """Keep the status and the rest of the row from disagreeing."""
+
+        spawned = self.status in ShellStatusGroups.SPAWNED
+        ShellStatusGroups.check_exit_code(self.status, self.exit_code)
+
+        if spawned:
+            if self.reason is not None:
+                raise ValueError(_Message.REASON_FORBIDDEN)
+            if not self.workspace:
+                raise ValueError(_Message.WORKSPACE_REQUIRED)
+        else:
+            if self.reason is None:
+                raise ValueError(_Message.REASON_REQUIRED)
+            if self.output:
+                raise ValueError(_Message.OUTPUT_ON_REFUSAL)
+
+        if self.truncated:
+            if self.output_total_bytes is None:
+                raise ValueError(_Message.TRUNCATION_REQUIRES_TOTAL)
+        elif self.output_total_bytes is not None or self.output_ref is not None:
+            raise ValueError(_Message.TOTAL_BYTES_REQUIRES_TRUNCATION)
+        return self
+
+
+class ShellRefusal(ShellContract):
+    """One call that produced no process, and why.
+
+    Carried by :class:`ShellRefusedError` so the decision can be raised at the
+    point that makes it and converted into a :class:`RunCommandResult` once, at
+    the tool boundary. ``reason`` is the closed code; ``note`` is the
+    model-facing sentence, authored from constants rather than interpolated from
+    an exception, so a refusal cannot leak deployment configuration.
+    """
+
+    status: ShellExecutionStatus
+    reason: ShellRefusalReason
+    note: str = Field(min_length=1, max_length=2048)
+
+    @model_validator(mode="after")
+    def _check_status_did_not_spawn(self) -> "ShellRefusal":
+        if self.status not in ShellStatusGroups.NOT_SPAWNED:
+            raise ValueError(_Message.REFUSAL_STATUS)
+        return self
+
+    @classmethod
+    def refused(cls, reason: ShellRefusalReason, note: str) -> "ShellRefusal":
+        """A floor decision: this command may not run, and there is nothing to click."""
+
+        return cls(status=ShellExecutionStatus.REFUSED, reason=reason, note=note)
+
+    @classmethod
+    def unavailable(cls, reason: ShellRefusalReason, note: str) -> "ShellRefusal":
+        """The capability could not serve this call; the command itself is not judged."""
+
+        return cls(status=ShellExecutionStatus.UNAVAILABLE, reason=reason, note=note)
+
+    def as_result(self, *, duration_ms: int = 0) -> RunCommandResult:
+        """Project the refusal into the model-facing result shape.
+
+        ``output`` stays empty because no process wrote anything; the sentence
+        goes to ``exit_note``, which is the result's one model-facing prose
+        field. ``workspace`` is left empty for the same reason the field has a
+        default at all.
+        """
+
+        return RunCommandResult(
+            status=self.status,
+            output="",
+            duration_ms=duration_ms,
+            exit_note=self.note,
+            reason=self.reason,
+        )
+
+
+class ShellRefusedError(Exception):
+    """Typed domain error raised where a refusal is decided.
+
+    Raised rather than returned so a refusal cannot be dropped by a caller that
+    forgot to inspect a union. The tool boundary catches it exactly once and
+    calls :meth:`ShellRefusal.as_result`.
+    """
+
+    def __init__(self, refusal: ShellRefusal) -> None:
+        super().__init__(refusal.note)
+        self._refusal = refusal
+
+    @property
+    def refusal(self) -> ShellRefusal:
+        """The typed decision, safe to project into model-facing output."""
+
+        return self._refusal
+
+
+class ShellExecutionRequest(ShellContract):
+    """Everything the executor needs, and nothing it may decide for itself.
+
+    Not a model-facing shape: by the time one of these exists the never-list has
+    run, the workspace label has been resolved to a real directory, the timeout
+    has been checked against the configured ceiling, and the environment has
+    been *built* by :mod:`~agent_runtime.capabilities.shell.environment`. The
+    executor's whole job is to spawn, bound, and reap — it makes no policy
+    decision, which is why it can be the one subprocess call site without also
+    being the place every rule accretes.
+    """
+
+    command: str = Field(min_length=1, max_length=_Text.MAX_COMMAND_CHARS)
+    cwd: Path
+    timeout_s: int = Field(ge=1)
+    #: Constructed by allowlist; never ``os.environ``. ``repr=False`` keeps it
+    #: out of exception rendering as a matter of hygiene, not because it is
+    #: expected to hold anything sensitive — by construction it does not.
+    env: dict[str, str] = Field(default_factory=dict, repr=False)
+    shell_path: str = Field(min_length=1)
+    output_cap_bytes: int = Field(ge=1)
+    #: Where the full output spills once it passes ``output_cap_bytes``. A
+    #: virtual-scratch-backed host path minted by the caller; ``None`` means
+    #: overflow is counted and dropped rather than kept.
+    spill_path: Path | None = None
+    #: Hard ceiling on the spill file. Beyond the PRD, which caps only what the
+    #: model sees: without this, "print a gigabyte" is bounded in memory and
+    #: unbounded on the user's disk.
+    spill_cap_bytes: int = Field(default=8 * 1024 * 1024, ge=0)
+
+    @field_validator("cwd")
+    @classmethod
+    def _require_absolute_cwd(cls, value: Path) -> Path:
+        """A relative cwd would resolve against the worker's own directory."""
+
+        if not value.is_absolute():
+            raise ValueError(_Message.CWD_NOT_ABSOLUTE)
+        return value
+
+
+class ShellExecutionOutcome(ShellContract):
+    """What the executor observed. No labels, no policy, no scratch vocabulary.
+
+    ``output`` is the tail, decoded, already inside ``output_cap_bytes``.
+    ``output_total_bytes`` is the true total the process wrote, which is what
+    makes the truncation notice able to say "kept the last 64 KiB of 4.2 MB"
+    honestly even though those bytes are long gone from memory.
+    """
+
+    status: ShellExecutionStatus
+    exit_code: int | None = None
+    output: str = ""
+    truncated: bool = False
+    output_total_bytes: int = Field(default=0, ge=0)
+    duration_ms: int = Field(default=0, ge=0)
+    #: True when the spill file itself hit ``spill_cap_bytes`` and stopped
+    #: growing. The model's ``output`` is still the true tail; the spill then
+    #: holds the head. Surfaced rather than hidden so the transcript can say so.
+    spill_truncated: bool = False
+    #: Set when a spill file was actually written.
+    spill_written: bool = False
+
+    @model_validator(mode="after")
+    def _check_exit_code_matches_status(self) -> "ShellExecutionOutcome":
+        ShellStatusGroups.check_exit_code(self.status, self.exit_code)
+        if self.status not in ShellStatusGroups.SPAWNED:
+            raise ValueError(_Message.OUTCOME_STATUS)
+        return self
+
+
+class ShellCommandCancelled(Exception):
+    """The run was cancelled while a command was live.
+
+    Carries the partial outcome (AC5.2) so the boundary can answer with
+    ``status="cancelled"`` and the output captured up to the cancellation rather
+    than an empty string. Raised instead of returned because swallowing an
+    ``asyncio.CancelledError`` and returning normally would absorb a cancel the
+    caller may still need to propagate — that decision belongs to the boundary,
+    not to the executor.
+    """
+
+    def __init__(self, outcome: ShellExecutionOutcome) -> None:
+        super().__init__("command cancelled")
+        self._outcome = outcome
+
+    @property
+    def outcome(self) -> ShellExecutionOutcome:
+        """The partial execution, already bounded and decoded."""
+
+        return self._outcome
diff --git a/services/ai-backend/src/agent_runtime/capabilities/shell/descriptor.py b/services/ai-backend/src/agent_runtime/capabilities/shell/descriptor.py
new file mode 100644
index 000000000..d40233fe2
--- /dev/null
+++ b/services/ai-backend/src/agent_runtime/capabilities/shell/descriptor.py
@@ -0,0 +1,169 @@
+"""The PDP inputs for ``run_command`` — identity, principal, allowlist (§4.1, §8.1).
+
+``run_command`` is the **first non-MCP capability through the PDP**
+(:class:`~agent_runtime.capabilities.policy.service.PdpPolicyService`), whose
+authorization stage was written connector-shaped. This module supplies the three
+builtin analogues §8.1 (OQ-1) asks for: a ``ConnectorState``, an empty
+``scopes`` tuple, and an allowlist port. Nothing here decides anything — the
+decision is ``PdpPolicyService.decide``, the enforcement point is
+:mod:`agent_runtime.capabilities.shell.policy_gate`.
+
+**The trap, and why this module exists at all.** ``PdpPolicyService._has_scopes``
+is fail-closed at the *connector* level: ``connector_scopes.get(connector) is
+None`` ⇒ ``False``, so a connector absent from the map is unauthorized **even
+when nothing is required**. A run's ``connector_scopes`` is keyed by MCP
+connector slug and will never carry ``shell``, so a naive principal makes Stage 2
+deny every command with ``permission_denied`` — a dead capability that looks like
+a policy decision.
+
+It is smaller than the MCP descriptor source it is modelled on
+(``mcp/descriptor_source.py``, 357 lines) because that one must *derive* action,
+trust and connector state from a catalog kind, tool annotations, and
+health-plus-pause-plus-access-mode. All three are constants for a builtin, so
+the code that would compute them is absent rather than stubbed.
+"""
+
+from __future__ import annotations
+
+from typing import Final
+
+from agent_runtime.capabilities.policy.contracts import (
+    Action,
+    CapabilityDescriptor,
+    CapabilityUrn,
+    ConnectorState,
+    Principal,
+    Trust,
+)
+from agent_runtime.capabilities.policy.service import ConnectorAllowlist
+from agent_runtime.capabilities.surfaces.builtin import server_slug
+from agent_runtime.execution.contracts import AgentRuntimeContext
+
+
+class ShellCapability:
+    """The one identity ``run_command`` is policed under.
+
+    ``OP`` is deliberately **not** ``execute``: that name is already claimed in
+    all three occupancy declarations by deepagents' filesystem placeholder
+    (``capabilities/operations/conformance.py``, ``builtin_operation_catalog.json``,
+    ``operation_descriptors.json``), and a collision there is a silent identity
+    merge — two capabilities resolving to one URN and inheriting each other's
+    rules.
+    """
+
+    #: The middle URN segment. ``server_slug`` normalises it, so the constant and
+    #: the parsed namespace agree byte-for-byte.
+    NAMESPACE: Final = "shell"
+
+    #: The trailing URN segment AND the model-facing tool name. One string, so a
+    #: rename cannot leave the policy identity pointing at the old tool.
+    OP: Final = "run_command"
+
+    #: ``builtin:shell:run_command``. The first ``for_builtin`` URN in
+    #: production — every URN built in ``src/`` before this one was ``for_mcp``.
+    URN: Final = CapabilityUrn.for_builtin(NAMESPACE, OP)
+
+    #: The connector key Stage 2 parses out of the URN and looks up in
+    #: ``principal.connector_scopes``. Precomputed so :class:`ShellPrincipal` and
+    #: the PDP cannot disagree about the spelling.
+    CONNECTOR: Final = server_slug(NAMESPACE)
+
+
+class RunCommandDescriptor:
+    """Builds the :class:`CapabilityDescriptor` the PDP polices one call against.
+
+    Not a cached constant: ``connector_state`` carries the §7.2 recheck, so the
+    descriptor is rebuilt for every call from the availability the tool boundary
+    has just re-read. A module-level singleton would freeze ``LIVE`` at
+    registration and quietly keep a shell pointed at a folder the user revoked.
+    """
+
+    @staticmethod
+    def for_availability(*, available: bool) -> CapabilityDescriptor:
+        """The descriptor for one call. ``available=False`` denies at Stage 1.
+
+        ``trust`` is ``TRUSTED`` and that costs nothing: :class:`Trust` is read
+        by exactly one rung of the ladder (3.7, the untrusted-READ gate), which
+        an ``EXECUTE`` action never reaches. Declaring ``UNTRUSTED`` would read
+        as a second, inert safety control and invite someone to rely on it. The
+        control for a command is the EXECUTE rung at 3.5½, which pauses in every
+        posture unless the axis itself is authored to ``auto``.
+        """
+
+        return CapabilityDescriptor(
+            urn=ShellCapability.URN,
+            action=Action.EXECUTE,
+            trust=Trust.TRUSTED,
+            # Empty, and load-bearing: an empty required set is what makes
+            # ShellPrincipal's overlay a totality fix rather than a grant.
+            scopes=(),
+            source="builtin",
+            connector_state=(ConnectorState.LIVE if available else ConnectorState.OFF),
+        )
+
+
+class ShellPrincipal:
+    """The run's :class:`Principal` for one command, seeded so Stage 2 is total.
+
+    The overlay is exactly one entry, ``{"shell": frozenset()}``, which grants
+    nothing — the required set is empty on both sides — and only makes the stage
+    total for a capability whose authority lives elsewhere (see the module
+    docstring's trap).
+
+    **Deliberately not a dataclass of its own.** ``AgentRuntimeContext`` already
+    *is* the production ``Principal`` and carries exactly the five fields the PDP
+    reads, so re-declaring them here would be a third copy of one row — the
+    context, this class, and ``mcp.policy_allowlist.McpConnectorPrincipal``,
+    which is that shape spelled out field-by-field with a different overlay
+    value. ``model_copy`` is non-destructive in both directions: present
+    connector entries survive, and the caller's context is not mutated.
+    """
+
+    __slots__ = ()
+
+    @staticmethod
+    def for_run(context: AgentRuntimeContext) -> Principal:
+        """Overlay the one empty ``shell`` entry onto this run's principal."""
+
+        return context.model_copy(
+            update={
+                "connector_scopes": {
+                    **context.connector_scopes,
+                    ShellCapability.CONNECTOR: frozenset(),
+                }
+            }
+        )
+
+
+class BuiltinCapabilityAllowlist:
+    """A ``ConnectorAllowlistPort`` that imposes no org/user restriction.
+
+    Total by construction — the shell URN, a malformed one, someone else's, all
+    return the empty :class:`ConnectorAllowlist` the PDP reads as "no
+    restriction", and it never raises.
+
+    Not a hole. The org/user allowlist answers *which tenants may reach a
+    registered connector*, read from the connector registry by URN; a builtin has
+    no registry row, so there is no allowlist to read. The question a command
+    actually raises — may this workspace run commands at all — is answered
+    upstream by the per-workspace grant flag and the deployment gate (§7.1),
+    before a descriptor exists.
+    """
+
+    __slots__ = ()
+
+    _EMPTY: Final = ConnectorAllowlist()
+
+    def allowlist_for(self, urn: str) -> ConnectorAllowlist:
+        """Return the empty allowlist for any URN."""
+
+        del urn
+        return self._EMPTY
+
+
+__all__ = [
+    "BuiltinCapabilityAllowlist",
+    "RunCommandDescriptor",
+    "ShellCapability",
+    "ShellPrincipal",
+]
diff --git a/services/ai-backend/src/agent_runtime/capabilities/shell/environment.py b/services/ai-backend/src/agent_runtime/capabilities/shell/environment.py
new file mode 100644
index 000000000..57f731453
--- /dev/null
+++ b/services/ai-backend/src/agent_runtime/capabilities/shell/environment.py
@@ -0,0 +1,188 @@
+"""The child environment, built by allowlist (PRD-shell-execution §11.3).
+
+**This is an allowlist, not a filter.** Nothing is removed from ``os.environ``;
+``os.environ`` is never the starting point. The builder reads a fixed tuple of
+names and writes a fixed tuple of names, and it never iterates the parent
+environment at all — which is the property, not the list. A denylist is the
+wrong default here because our secret set grows with every provider we add: the
+worker process holds the provider keys, the loopback broker bearer
+(``DESKTOP_WORKSPACE_BROKER_TOKEN``, with ``DESKTOP_BROKER_TOKEN`` as the legacy
+fallback) and ``ENTERPRISE_SERVICE_TOKEN``, and ``env | grep -i key`` would print
+all of it.
+
+Security invariant §5 is what is at stake: provider keys ride
+``AgentRuntimeContext.provider_keys`` with ``exclude=True, repr=False`` so they
+never appear in a payload, an event or a repr. Inheriting ``os.environ`` for one
+child process would defeat that in a single call, because a command is the one
+call shape with no path for the filesystem controls to key on.
+
+The test that guards this is deliberately **name-independent** (AC6.2): it puts
+a freshly generated, never-mentioned variable into the parent environment and
+asserts it is absent from the child's. A test that asserts on a list of known
+secret names is only a regression pin — an earlier draft of the PRD asserted on
+``COPILOT_BROKER_TOKEN``, a string that appears nowhere in this repository, and
+would have passed over an environment leaking the real token.
+"""
+
+from __future__ import annotations
+
+from collections.abc import Mapping
+import os
+from pathlib import Path
+from typing import Final
+
+
+class ShellEnvironment:
+    """The names and values that make up a command's environment.
+
+    Constants live here rather than on the builder so a guardrail test can
+    assert on the allowlist itself without constructing anything.
+    """
+
+    #: Read from the parent environment when present, otherwise defaulted or
+    #: omitted. This tuple is the entire read surface: no other name in the
+    #: worker's environment can reach a child process.
+    PASSTHROUGH: Final = ("LANG", "LC_ALL", "TZ", "USER", "LOGNAME")
+
+    #: Passthrough names that get a deterministic value when the parent has
+    #: none. ``USER``/``LOGNAME`` are absent rather than invented — a wrong
+    #: username is worse than a missing one.
+    PASSTHROUGH_DEFAULTS: Final = {
+        "LANG": "C.UTF-8",
+        "LC_ALL": "C.UTF-8",
+        "TZ": "UTC",
+    }
+
+    #: Written unconditionally, from runtime facts rather than from the parent.
+    PATH: Final = "PATH"
+    HOME: Final = "HOME"
+    TMPDIR: Final = "TMPDIR"
+    PWD: Final = "PWD"
+
+    #: Fixed values. ``TERM=dumb`` and ``NO_COLOR=1`` suppress most ANSI at the
+    #: source (§13); ``CI=1`` makes most test runners non-interactive and
+    #: deterministic, which matters because stdin is closed.
+    FIXED: Final = {"TERM": "dumb", "NO_COLOR": "1", "CI": "1"}
+
+    #: Prepended to ``PATH``, relative to the bound root, so a project's own
+    #: toolchain wins over the system one — which is what makes ``pytest``
+    #: resolve to the workspace's ``.venv`` rather than to nothing. The cost is
+    #: stated rather than hidden: a repository can shadow a system binary this
+    #: way. That is not a new exposure, because §11.5 already says a command
+    #: runs as the user with the user's permissions and no OS isolation.
+    LOCAL_BIN_DIRS: Final = (".venv/bin", "venv/bin", "node_modules/.bin", "bin")
+
+    #: The system half of ``PATH``. A constructed list, not the worker's own
+    #: ``PATH`` verbatim: the worker's ``PATH`` is staged by the desktop runtime
+    #: and points at bundled interpreters we do not want a command resolving
+    #: against. The Homebrew prefixes are listed unconditionally; a ``PATH``
+    #: entry that does not exist costs nothing and probing the filesystem here
+    #: would make the built environment depend on the host it was built on.
+    SYSTEM_BIN_DIRS: Final = (
+        "/usr/local/bin",
+        "/opt/homebrew/bin",
+        "/opt/homebrew/sbin",
+        "/usr/bin",
+        "/bin",
+        "/usr/sbin",
+        "/sbin",
+    )
+
+    #: Never copied, and not because they are on a denylist — they are simply
+    #: not on the allowlist. Named here only so the reason survives: an
+    #: inherited ``VIRTUAL_ENV``/``CONDA_PREFIX`` makes a command in workspace A
+    #: install into workspace B's interpreter.
+    NOTABLY_ABSENT: Final = ("VIRTUAL_ENV", "CONDA_PREFIX", "PYTHONPATH", "SHELL")
+
+
+class ShellEnvironmentBuilder:
+    """Builds one child environment from runtime facts plus a fixed allowlist.
+
+    Stateless and total: it raises nothing, and the request cannot influence it
+    because the request never reaches it. Its only inputs are the bound root
+    (chosen by the runtime from the writable grants, never by the model), the
+    agent's own scratch directory, and the parent mapping it is allowed to read
+    five names out of.
+    """
+
+    def build(
+        self,
+        *,
+        bound_root: Path,
+        scratch_dir: Path,
+        source: Mapping[str, str] | None = None,
+    ) -> dict[str, str]:
+        """Return the complete environment for one command.
+
+        ``source`` defaults to ``os.environ``. It is read strictly by
+        ``.get(name)`` over :attr:`ShellEnvironment.PASSTHROUGH` plus ``HOME``;
+        it is never iterated, so a variable this module has not been told about
+        has no path into the result.
+        """
+
+        parent = source if source is not None else os.environ
+        env: dict[str, str] = {
+            ShellEnvironment.PATH: self._build_path(bound_root),
+            ShellEnvironment.HOME: self._resolve_home(parent, scratch_dir),
+            ShellEnvironment.TMPDIR: str(scratch_dir),
+            ShellEnvironment.PWD: str(bound_root),
+        }
+        env.update(ShellEnvironment.FIXED)
+        for name in ShellEnvironment.PASSTHROUGH:
+            value = self._clean(parent.get(name))
+            if value is None:
+                value = ShellEnvironment.PASSTHROUGH_DEFAULTS.get(name)
+            if value is not None:
+                env[name] = value
+        return env
+
+    def _build_path(self, bound_root: Path) -> str:
+        """Workspace-local bin directories first, then a constructed system PATH.
+
+        Any entry containing the path separator is dropped: a bound root with a
+        colon in its name would otherwise split into two ``PATH`` entries, one
+        of which is a directory nobody chose.
+        """
+
+        entries = [
+            str(bound_root / relative) for relative in ShellEnvironment.LOCAL_BIN_DIRS
+        ]
+        entries.extend(ShellEnvironment.SYSTEM_BIN_DIRS)
+        return os.pathsep.join(
+            entry for entry in entries if os.pathsep not in entry and entry
+        )
+
+    def _resolve_home(self, parent: Mapping[str, str], scratch_dir: Path) -> str:
+        """v1: the user's real home. Phase 2 replaces this with a per-run scratch.
+
+        Stated plainly rather than left implicit: with the real ``HOME``, a
+        command can reach ``~/.ssh``, ``~/.aws`` and every dotfile the user
+        owns, by expansion. That is a documented residual risk of Phase 1
+        (§11.5), not an oversight, and the never-list is defence in depth
+        against the plausible accident rather than a boundary.
+
+        The scratch directory is the fallback only when the parent has no
+        ``HOME`` and the platform cannot resolve one — a command with no
+        ``HOME`` at all breaks tools in confusing ways.
+        """
+
+        explicit = self._clean(parent.get(ShellEnvironment.HOME))
+        if explicit:
+            return explicit
+        try:
+            return str(Path.home())
+        except RuntimeError:
+            return str(scratch_dir)
+
+    @staticmethod
+    def _clean(value: str | None) -> str | None:
+        """Drop an unusable passthrough value rather than passing it to ``execve``.
+
+        A NUL in an environment value raises at the exec boundary, which would
+        turn a strange-but-harmless parent environment into a capability that
+        cannot spawn anything.
+        """
+
+        if value is None or "\x00" in value:
+            return None
+        return value
diff --git a/services/ai-backend/src/agent_runtime/capabilities/shell/executor.py b/services/ai-backend/src/agent_runtime/capabilities/shell/executor.py
new file mode 100644
index 000000000..8e4c6e765
--- /dev/null
+++ b/services/ai-backend/src/agent_runtime/capabilities/shell/executor.py
@@ -0,0 +1,540 @@
+"""The one subprocess call site in ``agent_runtime`` (PRD-shell-execution §11.1-§11.2, §13).
+
+Everything above this module decides *whether* a command may run. This module
+only runs it, and bounds it. It makes no policy decision, resolves no path,
+reads no environment variable and knows nothing about workspace labels — which
+is what lets it be the single spawn point without also becoming the place every
+rule accretes. §16.6 pins that with a guardrail test: no ``shell=True`` and no
+``create_subprocess_shell`` anywhere in ``agent_runtime`` outside this file.
+
+**What is bounded, and by what.** Every one of these has a defined outcome
+rather than a hope:
+
+============================  ==========================================================
+Unbounded thing               Bound
+============================  ==========================================================
+A command that never exits    ``timeout_s`` -> the **process group** is killed, status
+                              ``timeout``, ``exit_code=None``, partial output kept.
+A command that ignores TERM   SIGTERM, a grace window, then SIGKILL to the group.
+A command that spawns         ``start_new_session=True`` puts the whole tree in one
+children                      group, so the kill reaches the children too.
+A command that prints GBs     A ring buffer bounded at ``2 x cap`` -> memory never holds
+                              more than that, whatever the process writes.
+The spill file                ``spill_cap_bytes``. Beyond the PRD, which bounds only what
+                              the model sees; without it "print a gigabyte" is bounded in
+                              memory and unbounded on the user's disk.
+A run that is cancelled       The group is killed and the partial output is carried out
+                              on a typed exception rather than lost.
+A command that never runs     A failed spawn is a typed ``unavailable`` refusal, never an
+                              ``OSError`` reaching the model as a paraphrase.
+============================  ==========================================================
+
+**What is NOT bounded here, said plainly rather than half-built.** There is no
+OS-level sandboxing in this phase. A command runs as the user, with the user's
+permissions: it can read any file the user can read, write any file the user can
+write, and reach the network. The bound working directory constrains where a
+command *starts*, not where it can reach, and ``HOME`` is the user's real home
+(§11.5). Those residual risks are documented in-product and deferred to Phase 2,
+where the scratch ``HOME`` and SPIKE-S1's per-platform confinement live. Nothing
+in this module should be read as claiming isolation it does not have.
+
+**On killing the group rather than the child.** OpenCode sets ``detached: true``
+and then never calls its own ``killTree`` helper, so whether its kill reaches the
+group is unverified. Here the teardown is an explicit ``os.killpg`` escalation
+that waits on the group, and the group id is checked against the worker's own
+before any signal is sent — signalling our own process group would kill the
+worker along with the command.
+"""
+
+from __future__ import annotations
+
+import asyncio
+import contextlib
+import os
+import signal
+import time
+from collections.abc import Callable
+from pathlib import Path
+from typing import BinaryIO, Final
+
+from agent_runtime.capabilities.shell.contracts import (
+    ShellCommandCancelled,
+    ShellExecutionOutcome,
+    ShellExecutionRequest,
+    ShellExecutionStatus,
+    ShellRefusal,
+    ShellRefusalReason,
+    ShellRefusedError,
+)
+
+
+class _Note:
+    """Model-facing sentences for the failures this module can produce.
+
+    Authored here, never interpolated from an exception: an ``OSError`` message
+    can name a host path, and this result is model-visible.
+    """
+
+    SPAWN_FAILED: Final = (
+        "The command could not be started. Nothing ran, and nothing changed."
+    )
+    TIMEOUT: Final = (
+        "The command did not finish within {timeout}s and was stopped, along "
+        "with anything it started. The output below is what it had produced by "
+        "then. Re-run something shorter, or ask again with a larger timeout_s "
+        "(up to the configured maximum)."
+    )
+    TRUNCATED: Final = (
+        "...output truncated (kept the last {kept} of {total})...\n"
+        "Full output: {ref}\n\n"
+    )
+    TRUNCATED_NO_REF: Final = (
+        "...output truncated (kept the last {kept} of {total}; the rest was "
+        "not retained)...\n\n"
+    )
+
+
+class _OutputCollector:
+    """Bounded output accumulation: the tail in memory, the rest on disk.
+
+    Three rules, each of which is load-bearing:
+
+    * **Tail, not head.** The error is at the end of a build log, so the tail is
+      what the model needs. This is the opposite of the generic head-first
+      truncation used elsewhere; here there is one rule.
+    * **Bounded at ``2 x cap``.** The ring is trimmed back to ``cap`` whenever it
+      passes twice that, so a process writing a gigabyte never causes a gigabyte
+      to be held. Trimming at ``2 x cap`` rather than at ``cap`` is what keeps
+      the trim amortised instead of per-chunk. Stated exactly: after every
+      ``feed`` the ring is at most ``2 x cap``, and its transient peak *during*
+      one ``feed`` is that plus the chunk being appended — 64 KiB in the
+      executor, whatever the process wrote.
+    * **A codepoint is never split.** After clipping to the last ``cap`` bytes,
+      the leading UTF-8 continuation bytes are walked past, so the tail starts
+      at a character boundary rather than half-way through one.
+
+    The spill file, when one is configured, receives the output from the first
+    byte and stops at ``spill_cap_bytes``. So on an enormous output the model
+    reads the true tail and the file holds the head — the two halves that are
+    actually worth keeping — and the collector says so rather than implying the
+    file is complete.
+    """
+
+    def __init__(
+        self,
+        *,
+        cap_bytes: int,
+        spill_path: Path | None = None,
+        spill_cap_bytes: int = 0,
+    ) -> None:
+        self._cap = max(cap_bytes, 1)
+        self._spill_path = spill_path
+        self._spill_cap = spill_cap_bytes
+        self._tail = bytearray()
+        self._total = 0
+        self._spilled = 0
+        self._spill: BinaryIO | None = None
+        self._spill_truncated = False
+        self._spill_failed = False
+
+    @property
+    def total_bytes(self) -> int:
+        """Every byte the process wrote, including the ones no longer held."""
+
+        return self._total
+
+    @property
+    def truncated(self) -> bool:
+        """Whether the model is seeing less than the process wrote."""
+
+        return self._total > self._cap
+
+    @property
+    def spill_written(self) -> bool:
+        """Whether a spill file was actually created and holds bytes."""
+
+        return self._spilled > 0
+
+    @property
+    def spill_truncated(self) -> bool:
+        """Whether the spill file hit its own ceiling and stopped growing."""
+
+        return self._spill_truncated
+
+    def feed(self, chunk: bytes) -> None:
+        """Accept one read. Never grows unboundedly, whatever the chunk size."""
+
+        if not chunk:
+            return
+        was_within_cap = self._total <= self._cap
+        self._total += len(chunk)
+        self._tail.extend(chunk)
+        if was_within_cap and self._total > self._cap:
+            # First overflow. The ring has not been trimmed yet (it only trims
+            # above 2 x cap and everything before this chunk fitted in cap), so
+            # it still holds the complete output from byte zero - which is
+            # exactly what the spill file needs as its opening.
+            self._open_spill()
+            self._write_spill(bytes(self._tail))
+        elif self._spill is not None:
+            self._write_spill(chunk)
+        if len(self._tail) > 2 * self._cap:
+            del self._tail[: len(self._tail) - self._cap]
+
+    def tail_text(self) -> str:
+        """The decoded tail, inside the cap, never split mid-codepoint."""
+
+        data = bytes(self._tail[-self._cap :])
+        if self.truncated:
+            data = self._skip_continuation_bytes(data)
+        return data.decode("utf-8", errors="replace")
+
+    def notice(self, output_ref: str | None) -> str:
+        """The in-band truncation notice, or an empty string when nothing was cut.
+
+        In the output string as well as in the structured fields, so a model
+        that reads only ``output`` still learns that it was cut.
+        """
+
+        if not self.truncated:
+            return ""
+        kept = self._humanise(min(self._cap, self._total))
+        total = self._humanise(self._total)
+        if output_ref and self.spill_written:
+            return _Note.TRUNCATED.format(kept=kept, total=total, ref=output_ref)
+        return _Note.TRUNCATED_NO_REF.format(kept=kept, total=total)
+
+    def close(self) -> None:
+        """Release the spill handle. Safe to call more than once."""
+
+        if self._spill is not None:
+            with contextlib.suppress(OSError):
+                self._spill.close()
+            self._spill = None
+
+    def _open_spill(self) -> None:
+        if self._spill_path is None or self._spill_cap <= 0 or self._spill_failed:
+            return
+        try:
+            self._spill_path.parent.mkdir(parents=True, exist_ok=True)
+            self._spill = self._spill_path.open("wb")
+        except OSError:
+            # A spill we cannot write is not a reason to fail the command: the
+            # model still gets the tail and an honest "not retained" notice.
+            self._spill = None
+            self._spill_failed = True
+
+    def _write_spill(self, data: bytes) -> None:
+        if self._spill is None:
+            return
+        room = self._spill_cap - self._spilled
+        if room <= 0:
+            self._spill_truncated = True
+            return
+        if len(data) > room:
+            data = data[:room]
+            self._spill_truncated = True
+        try:
+            self._spill.write(data)
+        except OSError:
+            self.close()
+            self._spill_failed = True
+            return
+        self._spilled += len(data)
+
+    @staticmethod
+    def _skip_continuation_bytes(data: bytes) -> bytes:
+        """Advance past leading UTF-8 continuation bytes (``0b10xxxxxx``).
+
+        A byte clip can land in the middle of a multi-byte character; without
+        this walk the decoder would replace the orphaned tail bytes with U+FFFD
+        and the model would read a corrupted first character.
+        """
+
+        index = 0
+        limit = min(len(data), 4)
+        while index < limit and (data[index] & 0xC0) == 0x80:
+            index += 1
+        return data[index:]
+
+    @staticmethod
+    def _humanise(size: int) -> str:
+        """Byte counts for the in-band notice. Whole units, no false precision."""
+
+        if size < 1024:
+            return f"{size} B"
+        if size < 1024 * 1024:
+            return f"{size / 1024:.0f} KiB"
+        return f"{size / (1024 * 1024):.1f} MiB"
+
+
+class ShellCommandExecutor:
+    """Spawns one command, bounds it, and reaps it. The only spawn point.
+
+    Stateless between calls: one request in, one outcome out, no persistent
+    shell and no ``cd`` that survives to the next call. The grace windows are
+    constructor arguments so a test can compress them without touching the
+    escalation logic itself.
+    """
+
+    #: How long a process gets to handle SIGTERM before SIGKILL.
+    DEFAULT_SIGTERM_GRACE_S: Final = 5.0
+    #: How long the group gets to disappear after SIGKILL before we stop waiting.
+    DEFAULT_REAP_GRACE_S: Final = 2.0
+    #: Read size. Large enough that a flooding process does not cost a syscall
+    #: per line, small enough that the ring's overshoot stays trivial.
+    DEFAULT_READ_CHUNK_BYTES: Final = 64 * 1024
+
+    def __init__(
+        self,
+        *,
+        sigterm_grace_s: float = DEFAULT_SIGTERM_GRACE_S,
+        reap_grace_s: float = DEFAULT_REAP_GRACE_S,
+        read_chunk_bytes: int = DEFAULT_READ_CHUNK_BYTES,
+        clock: Callable[[], float] = time.monotonic,
+    ) -> None:
+        self._sigterm_grace_s = sigterm_grace_s
+        self._reap_grace_s = reap_grace_s
+        self._read_chunk_bytes = read_chunk_bytes
+        self._clock = clock
+
+    async def run(
+        self, request: ShellExecutionRequest, *, output_ref: str | None = None
+    ) -> ShellExecutionOutcome:
+        """Run one command to completion, timeout, or cancellation.
+
+        ``output_ref`` is the *virtual* path the model will be given for the
+        spill file, used only to compose the in-band truncation notice; the
+        executor never mints it and never puts a host path in the output.
+
+        Raises :class:`ShellRefusedError` when the process could not be started
+        at all, and :class:`ShellCommandCancelled` — carrying the partial
+        outcome — when the surrounding task is cancelled while the command is
+        live.
+        """
+
+        started = self._clock()
+        collector = _OutputCollector(
+            cap_bytes=request.output_cap_bytes,
+            spill_path=request.spill_path,
+            spill_cap_bytes=request.spill_cap_bytes,
+        )
+        process = await self._spawn(request)
+        # No await between the spawn and the try, so a cancellation cannot
+        # arrive here and strand a live process outside the teardown path.
+        group = self._process_group(process)
+        reader = asyncio.create_task(self._drain(process, collector))
+        try:
+            status, exit_code = await self._await_completion(
+                process, reader, group, request.timeout_s
+            )
+        except asyncio.CancelledError:
+            # Converted into a typed domain signal rather than propagated, so
+            # the partial output survives (AC5.2). The signals below are
+            # synchronous, so the group dies even if the awaits here are
+            # interrupted again. Whether to re-raise the cancellation is the
+            # boundary's decision, not the executor's.
+            reader.cancel()
+            with contextlib.suppress(Exception, asyncio.CancelledError):
+                await self._terminate(process, group)
+            collector.close()
+            raise ShellCommandCancelled(
+                self._outcome(
+                    ShellExecutionStatus.CANCELLED, None, collector, started, output_ref
+                )
+            ) from None
+        finally:
+            reader.cancel()
+            # Suppressed broadly on purpose: this is cleanup running while
+            # another exception may already be in flight, and a pipe error here
+            # must not replace the outcome the caller is about to be given.
+            with contextlib.suppress(asyncio.CancelledError, Exception):
+                await reader
+            collector.close()
+        return self._outcome(status, exit_code, collector, started, output_ref)
+
+    async def _spawn(
+        self, request: ShellExecutionRequest
+    ) -> asyncio.subprocess.Process:
+        """The one ``create_subprocess_exec`` call in the service.
+
+        ``-c`` with an explicit shell binary, never ``shell=True`` and never
+        ``create_subprocess_shell``: the shell that interprets the command is
+        the configured one, not whatever the platform picks, and it is not a
+        login shell, so no rc file is sourced into the child.
+
+        ``stdin=DEVNULL`` closes the door on interactive prompting and on
+        password entry — which is also what makes ``sudo -S`` pointless as well
+        as never-listed.
+        """
+
+        try:
+            return await asyncio.create_subprocess_exec(
+                request.shell_path,
+                "-c",
+                request.command,
+                stdout=asyncio.subprocess.PIPE,
+                stderr=asyncio.subprocess.STDOUT,
+                stdin=asyncio.subprocess.DEVNULL,
+                cwd=str(request.cwd),
+                env=dict(request.env),
+                start_new_session=os.name == "posix",
+            )
+        except (OSError, ValueError) as error:
+            # A missing shell, an unusable cwd, or an environment value the exec
+            # boundary rejects. The typed refusal keeps the OSError's text -
+            # which can name a host path - away from the model.
+            raise ShellRefusedError(
+                ShellRefusal.unavailable(
+                    ShellRefusalReason.EXECUTION_UNAVAILABLE, _Note.SPAWN_FAILED
+                )
+            ) from error
+
+    async def _await_completion(
+        self,
+        process: asyncio.subprocess.Process,
+        reader: asyncio.Task[None],
+        group: int | None,
+        timeout_s: int,
+    ) -> tuple[ShellExecutionStatus, int | None]:
+        """Wait for EOF and exit inside one budget, or kill the group.
+
+        Draining and waiting are two separate waits against one shared deadline:
+        a process can close stdout and keep running, and a process can exit
+        while a child of it still holds the pipe open. Both are bounded.
+        """
+
+        deadline = self._clock() + timeout_s
+        try:
+            await asyncio.wait_for(reader, timeout=timeout_s)
+            remaining = max(deadline - self._clock(), 0.0)
+            exit_code = await asyncio.wait_for(process.wait(), timeout=remaining)
+        except TimeoutError:
+            await self._terminate(process, group)
+            return ShellExecutionStatus.TIMEOUT, None
+        return ShellExecutionStatus.COMPLETED, exit_code
+
+    async def _drain(
+        self, process: asyncio.subprocess.Process, collector: _OutputCollector
+    ) -> None:
+        """Read combined stdout+stderr until EOF, feeding the bounded collector.
+
+        Reading continuously is not only about capture: a process whose pipe
+        fills up blocks on write, so a tool that stopped reading would hang any
+        command that printed more than a pipe buffer.
+        """
+
+        stream = process.stdout
+        if stream is None:
+            return
+        while True:
+            try:
+                chunk = await stream.read(self._read_chunk_bytes)
+            except (OSError, ValueError):
+                # The pipe went away underneath us. That ends the capture; it
+                # does not fail the command, and the bytes already collected
+                # stay in the outcome.
+                return
+            if not chunk:
+                return
+            collector.feed(chunk)
+
+    async def _terminate(
+        self, process: asyncio.subprocess.Process, group: int | None
+    ) -> None:
+        """SIGTERM the group, wait, then SIGKILL it. The kill is guaranteed.
+
+        The escalation is in a ``finally`` and the signals themselves are
+        synchronous, so a cancellation arriving mid-teardown cannot leave the
+        process group alive: SIGKILL is delivered before the awaited reap.
+        """
+
+        if process.returncode is not None:
+            return
+        self._signal(process, group, signal.SIGTERM)
+        try:
+            await self._wait_bounded(process, self._sigterm_grace_s)
+        finally:
+            if process.returncode is None:
+                self._signal(process, group, signal.SIGKILL)
+                with contextlib.suppress(asyncio.CancelledError):
+                    await self._wait_bounded(process, self._reap_grace_s)
+
+    async def _wait_bounded(
+        self, process: asyncio.subprocess.Process, timeout_s: float
+    ) -> None:
+        """Wait for exit, giving up after ``timeout_s`` rather than hanging."""
+
+        with contextlib.suppress(TimeoutError):
+            await asyncio.wait_for(process.wait(), timeout=timeout_s)
+
+    def _signal(
+        self,
+        process: asyncio.subprocess.Process,
+        group: int | None,
+        number: signal.Signals,
+    ) -> None:
+        """Signal the whole process group, falling back to the direct child.
+
+        ``ProcessLookupError`` is the ordinary case, not an error: the process
+        finished between the check and the signal.
+        """
+
+        try:
+            if group is not None:
+                os.killpg(group, number)
+                return
+        except ProcessLookupError:
+            return
+        except OSError:
+            pass
+        with contextlib.suppress(ProcessLookupError, OSError, ValueError):
+            process.send_signal(number)
+
+    @staticmethod
+    def _process_group(process: asyncio.subprocess.Process) -> int | None:
+        """The child's process group, or ``None`` when it cannot be trusted.
+
+        Two refusals to return a group, both deliberate: a platform without
+        ``killpg`` (Windows, which Phase 1 does not target) and a group id equal
+        to our own. The second is the one that matters — signalling the worker's
+        own group would take down the run, the worker, and every other command
+        with it.
+        """
+
+        if not hasattr(os, "killpg") or not hasattr(os, "getpgid"):
+            return None
+        try:
+            group = os.getpgid(process.pid)
+            if group == os.getpgid(0):
+                return None
+        except (ProcessLookupError, OSError):
+            return None
+        return group
+
+    def _outcome(
+        self,
+        status: ShellExecutionStatus,
+        exit_code: int | None,
+        collector: _OutputCollector,
+        started: float,
+        output_ref: str | None,
+    ) -> ShellExecutionOutcome:
+        """Assemble the bounded, decoded result of one execution."""
+
+        return ShellExecutionOutcome(
+            status=status,
+            exit_code=exit_code,
+            output=collector.notice(output_ref) + collector.tail_text(),
+            truncated=collector.truncated,
+            output_total_bytes=collector.total_bytes,
+            duration_ms=max(int((self._clock() - started) * 1000), 0),
+            spill_truncated=collector.spill_truncated,
+            spill_written=collector.spill_written,
+        )
+
+    @staticmethod
+    def timeout_note(timeout_s: int) -> str:
+        """The model-facing hint for a timed-out command, naming the value (AC5.4)."""
+
+        return _Note.TIMEOUT.format(timeout=timeout_s)
diff --git a/services/ai-backend/src/agent_runtime/capabilities/shell/never_list.py b/services/ai-backend/src/agent_runtime/capabilities/shell/never_list.py
new file mode 100644
index 000000000..6fdb50d8c
--- /dev/null
+++ b/services/ai-backend/src/agent_runtime/capabilities/shell/never_list.py
@@ -0,0 +1,1340 @@
+"""The never-list: one data table, two readers (PRD-shell-execution §9).
+
+**What this module is for, stated the way §9.4 asks for it.** 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 here should be read as
+claiming otherwise. The controls that actually hold against someone trying are
+the constructed environment (§11), the scratch ``HOME`` and the OS profile
+(Phase 2) — and the human reading the command. This module's job is to make the
+handful of shapes that are *never* a legitimate agent action unreachable, and to
+be honest in its own test corpus about everything it lets past
+(``test_never_list_corpus.py``).
+
+Two readers, different powers
+-----------------------------
+* **The pre-PDP lexical screen — the primary mechanism (§9.3).**
+  :meth:`CommandNeverList.screen` sees the full, untruncated command string and
+  **tokenises** it, so it can say "``argv[0]`` is ``sudo``", "this ``rm`` operand
+  resolves to ``/``", "this pipeline's sink is an interpreter". It runs *before*
+  the PEP is entered, so a hit means **no approval card is ever created**
+  (AC7.1) — there is nothing to click through.
+* **The ``_never`` ruleset — the floor.** :meth:`CommandNeverList.floor`
+  compiles the same table for a reader that cannot tokenise. A
+  ``PermissionRule`` ``fullmatch``es two globs and ANDs them
+  (``policy/rules.py:148-153``); the subject half sees one opaque string, so its
+  rows are coarse whole-command patterns. Kept because it survives a bug in the
+  screen and it is the rung ``PdpPolicyService`` evaluates above every rule and
+  every posture (``policy/service.py:328-333``), so it holds if a second call
+  path is ever wired that skips the screen.
+
+One table, two readers — never two tables, because a second table is a second
+thing to keep correct and it is the copy that will be forgotten.
+
+The parsing under the screen is ``shlex``, not ours
+---------------------------------------------------
+"Tokenises" above used to mean ~400 lines of hand-rolled lexer in this file. It
+is now :class:`ParsedCommandLine` — ``shlex`` plus a segment split — and that
+class's docstring carries the reasoning, including which of the old lexer's
+three stated objections to ``shlex`` survived. The rule predicates are unchanged
+in behaviour; they are shorter because what is beneath them is stdlib.
+
+⚠️ **One judgement must stay off the tokens.** ``shlex`` does not model shell
+FUNCTION DEFINITIONS: ``:(){ :|:& };:`` tokenises to ``[':', '()', '{', ':',
+'|', ':', '&', '}', ';', ':']``, which is garbage for the purpose. So
+:meth:`CommandNeverList._fork_bomb` reads the RAW string and must not be
+"tidied up" onto the token stream. Any predicate whose hazard is a syntactic
+FORM rather than an executable NAME carries the same constraint; a sweep of the
+other seven found no second instance, because each of them keys on a command
+name, an operand or a flag.
+
+⚠️ A floor row is a WHOLE-COMMAND glob, not a path pattern
+----------------------------------------------------------
+Three mechanical facts, each verified against ``policy/rules.py`` and each the
+reason an obvious authoring of this table does not work:
+
+1. **There is one subject and it is the entire command line.**
+   ``PolicySubjects.of`` folds the URN plus every top-level string argument
+   (``rules.py:345-359``). ``RunCommandInput`` has exactly one command field, so
+   the subject list is ``(urn, "cat ~/.ssh/id_rsa", label, grant-subject)``.
+   There is no path subject for a path pattern to match against.
+2. **Matching is ``fullmatch`` over that whole subject** (``rules.py:98-101``).
+   So ``~/.ssh/**`` never fires on ``cat ~/.ssh/id_rsa``; ``*/.ssh/*`` does.
+   Machine-checked, and pinned by ``test_never_list.py``.
+3. **``Wildcard.expand`` rewrites a LEADING ``~`` / ``$HOME`` on the PATTERN
+   side only** (``rules.py:104-122``); the command *text* is never expanded,
+   because the child shell expands it at exec time. A row authored ``~/.ssh/**``
+   is therefore rewritten to ``/Users//.ssh/**`` and can never meet the
+   literal ``~`` the model typed. **Every row here begins with ``*`` or with a
+   literal binary name**, and a test asserts every row survives
+   ``Wildcard.expand`` byte-identically.
+
+The pattern vocabulary is exactly: ``*`` → ``.*``, ``?`` → ``.``, everything in
+``.+^${}()|[]\\`` escaped to a literal, compiled with ``re.DOTALL``
+(``rules.py:125-134``). No character classes, no alternation, no anchors, no
+word boundaries, and case-sensitive. ``-rf`` and ``-fr`` are two rows.
+
+**Over-refusal here does NOT cost a click — it costs the command.** A floor hit
+is unappealable by construction. That is why the rows are deliberately narrow,
+why the screen (which tokenises) is the mechanism that decides, and why the
+known over-refusals the rows still carry are pinned as *tests* rather than left
+to be discovered: see ``test_never_list_corpus.py::KNOWN_OVER_REFUSALS``.
+
+Ordering (§9.5)
+---------------
+``PermissionRuleset.evaluate`` is **last-match-wins** and ``merge``
+concatenates, so the shipped floor must be merged **LAST** into ``_never`` or a
+user row of ``{"pattern": "*", "action": "allow"}`` would sit after it and win.
+``ShellCommandPolicyGate._pdp`` does the merge in that order
+(``never=authored_never.merge(self._never_list.floor())``); this module only
+supplies the rows, and ``test_never_list.py`` asserts the property end to end.
+"""
+
+from __future__ import annotations
+
+import re
+import shlex
+from dataclasses import dataclass
+from typing import ClassVar, Final
+
+from agent_runtime.capabilities.policy.rules import (
+    PermissionRule,
+    PermissionRuleset,
+    RuleAction,
+)
+from agent_runtime.capabilities.shell.contracts import (
+    ShellRefusal,
+    ShellRefusalReason,
+)
+from agent_runtime.capabilities.shell.vendored_deepagents_safety import (
+    RECOMMENDED_SAFE_SHELL_COMMANDS,
+    contains_dangerous_patterns,
+    is_shell_command_allowed,
+)
+
+
+class _Note:
+    """Model-facing refusal sentences, one per hazard class.
+
+    Authored constants, never interpolated from the command: the command may
+    carry connector-ingested content, and a note is model-visible output.
+
+    Differentiated rather than collapsed to one line on purpose. The closed
+    ``reason`` code stays ``COMMAND_NOT_PERMITTED`` for every one of these, so
+    nothing about deployment configuration is distinguishable; what differs is
+    the sentence that tells the model *which shape* it hit, which is the
+    difference between one retry and six. Every line about a well-formed command
+    says the refusal is permanent, because a deterministic refusal described as
+    temporary sends a model into a loop it cannot win. :data:`UNPARSEABLE` is
+    the one exception and says why.
+    """
+
+    _SUFFIX: Final = (
+        " Nothing was run, there is no approval to ask for, and retrying will "
+        "not change it."
+    )
+
+    PRIVILEGE: Final = "Commands are never run with elevated privileges." + _SUFFIX
+    DESTRUCTIVE_DELETE: Final = (
+        "A recursive delete of the filesystem root or the home directory is "
+        "never permitted." + _SUFFIX
+    )
+    FILESYSTEM_DESTRUCTION: Final = (
+        "Commands that write to a raw device or format a filesystem are never "
+        "permitted." + _SUFFIX
+    )
+    MACHINE_STATE: Final = (
+        "Commands that shut down, reboot or halt the machine are never "
+        "permitted." + _SUFFIX
+    )
+    FORK_BOMB: Final = "That command is a fork bomb and is never permitted." + _SUFFIX
+    #: The one note that does NOT end in :data:`_SUFFIX`, and deliberately so.
+    #: Every other sentence here describes a permanent judgement about a
+    #: well-formed command; this one describes a command we could not read.
+    #: Telling the model "retrying will not change it" would be false — closing
+    #: the quote changes it — and a refusal note that misdescribes the fix is
+    #: how a model burns a turn re-sending the identical broken string.
+    UNPARSEABLE: Final = (
+        "That command has an unbalanced quote or a dangling escape, so it could "
+        "not be read and nothing was run. Re-sending the same text will not "
+        "change it; send the command again with the quoting closed."
+    )
+    PIPE_TO_INTERPRETER: Final = (
+        "Piping a download straight into an interpreter is never permitted. "
+        "Download to a file, and the file can then be read before anything "
+        "runs it." + _SUFFIX
+    )
+    CREDENTIAL_PATH: Final = (
+        "That command names a credential directory, which is never readable "
+        "through a command." + _SUFFIX
+    )
+    CREDENTIAL_FILE: Final = (
+        "That command names a credential or key file, which is never readable "
+        "through a command." + _SUFFIX
+    )
+
+
+class SensitivePathPolicy:
+    """The credential-path and credential-filename policy — **a duplicate**.
+
+    ⚠️ **Change this together with**
+    ``apps/desktop/main/capabilities/path-validation.ts``:
+    ``SENSITIVE_ROOT_SEGMENTS`` (``:313-323``), ``SENSITIVE_FILE_RULES``
+    (``:854-871``) and ``isSensitiveFileName`` (``:879-890``). Python cannot
+    import TypeScript and ``packages/service-contracts`` is Python-only, so this
+    is the SIWE precedent — a byte-identical duplicate with a "change both
+    together" comment (root ``CLAUDE.md``). **The divergence is not left to a
+    comment**: ``test_never_list.py::TestTypeScriptParity`` parses the ``.ts``
+    file and asserts these three tables equal it, so drift reds CI rather than
+    silently un-refusing a credential file.
+
+    §5 recommends option 2 (ship the JSON in ``service-contracts`` and assert
+    equality from a desktop test) and records it as **OQ-3, unresolved**. This
+    module deliberately does not invent that cross-language artefact ahead of
+    the sign-off; it takes the already-accepted pattern and adds the equality
+    test, which is the half of option 2 that carries the actual guarantee.
+
+    Why a command must obey a *read* rule: the broker already refuses to return
+    these files' contents, and a command must not be the way around a rule the
+    read path enforces.
+    """
+
+    #: Directory basenames that must never appear anywhere in a path.
+    #: Case-INSENSITIVE in the TypeScript source (``s.toLowerCase()``), so this
+    #: screen lowercases too. The floor cannot: ``Wildcard.match`` is
+    #: case-sensitive. See :meth:`CommandNeverList._segment_rows`.
+    ROOT_SEGMENTS: Final = (
+        ".ssh",
+        ".aws",
+        ".gnupg",
+        ".gpg",
+        ".password-store",
+        ".docker",
+        ".kube",
+        ".azure",
+        "keychains",
+    )
+
+    #: Suffixes denoting key / certificate / keystore material. ``endsWith``.
+    FILE_SUFFIXES: Final = (
+        ".pem",
+        ".key",
+        ".p12",
+        ".pfx",
+        ".pkcs12",
+        ".keystore",
+        ".keychain",
+        ".asc",
+        ".ppk",
+    )
+
+    #: Prefixes for conventional SSH private-key files. ``startsWith``.
+    FILE_PREFIXES: Final = ("id_rsa", "id_ed25519", "id_dsa", "id_ecdsa")
+
+    #: Exact credential-store filenames. Equality.
+    FILE_EXACT: Final = (
+        "credentials",
+        ".netrc",
+        ".pgpass",
+        ".htpasswd",
+        ".dockercfg",
+    )
+
+    #: ``isSensitiveFileName``'s first check, special-cased there and here:
+    #: ``.env`` itself plus every ``.env.``. It is not a suffix, a
+    #: prefix or an exact entry, so nothing else in this class covers it.
+    DOTENV: Final = ".env"
+
+    @classmethod
+    def is_sensitive_file_name(cls, name: str) -> bool:
+        """Port of ``isSensitiveFileName`` (``path-validation.ts:879-890``).
+
+        The three groups use three DIFFERENT predicates and that difference is
+        load-bearing both here and in the floor rows: ``prefixes`` is
+        ``startsWith``, ``suffixes`` is ``endsWith``, ``exact`` is equality.
+        """
+
+        lower = name.lower()
+        if lower == cls.DOTENV or lower.startswith(f"{cls.DOTENV}."):
+            return True
+        if lower in cls.FILE_EXACT:
+            return True
+        if lower.startswith(cls.FILE_PREFIXES):
+            return True
+        return lower.endswith(cls.FILE_SUFFIXES)
+
+    @classmethod
+    def has_sensitive_segment(cls, token: str) -> bool:
+        """True when any ``/``-separated part of ``token`` is a credential dir.
+
+        Backslashes are folded to ``/`` first, mirroring ``Wildcard.match``'s
+        own ``value.replace("\\\\", "/")`` (``rules.py:101``) so the screen and
+        the floor agree about what a path separator is.
+        """
+
+        folded = token.replace("\\", "/")
+        return any(part.lower() in cls.ROOT_SEGMENTS for part in folded.split("/"))
+
+    @classmethod
+    def leaf_is_sensitive(cls, token: str) -> bool:
+        """True when ``token``'s leaf name satisfies :meth:`is_sensitive_file_name`."""
+
+        folded = token.replace("\\", "/")
+        return cls.is_sensitive_file_name(folded.rsplit("/", 1)[-1])
+
+
+@dataclass(frozen=True, slots=True)
+class CommandSegment:
+    """One command in the line, plus the operator that introduced it.
+
+    ``lead`` is ``""`` for the first segment and otherwise the operator token
+    that preceded it, which is how
+    :meth:`CommandNeverList._pipe_to_interpreter` tells a pipeline (``|``) from
+    a list (``&&``, ``;``) without re-parsing. ``argv`` is already dequoted —
+    ``shlex`` in POSIX mode removed the quoting, which is exactly what hazard
+    detection wants (``"sudo" rm`` really does invoke ``sudo``) and exactly what
+    :meth:`CommandNeverList.always_grant_patterns` must NOT have, so that method
+    reads :attr:`ParsedCommandLine.first_word` instead.
+    """
+
+    #: Reserved words and wrapper binaries that are followed by the command
+    #: actually being run. Skipping them WIDENS hazard detection (``time sudo
+    #: x`` is a ``sudo`` invocation) and is safe in that direction only — §8.3's
+    #: always-grant deliberately does NOT see through them.
+    INTRODUCERS: ClassVar[frozenset[str]] = frozenset(
+        {
+            "if",
+            "then",
+            "else",
+            "elif",
+            "while",
+            "until",
+            "do",
+            "!",
+            "{",
+            "}",
+            "time",
+            "env",
+            "nice",
+            "nohup",
+            "xargs",
+            "timeout",
+            "stdbuf",
+            "script",
+        }
+    )
+    _ASSIGNMENT: ClassVar[re.Pattern[str]] = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*=")
+
+    lead: str
+    argv: tuple[str, ...]
+    redirect_targets: tuple[str, ...] = ()
+
+    @classmethod
+    def is_assignment(cls, word: str) -> bool:
+        """True for a ``NAME=value`` prefix, which is not the command."""
+
+        return cls._ASSIGNMENT.match(word) is not None
+
+    @property
+    def candidates(self) -> tuple[str, ...]:
+        """The words in this segment that could be the command being run.
+
+        Command position is the one property the ``_never`` floor cannot
+        express, and it is what separates ``sudo rm -rf /`` from
+        ``git commit -m "no sudo here"``: a glob with no word boundaries cannot
+        tell a binary from a substring.
+
+        Assignment prefixes are skipped (``FOO=1 sudo x`` runs ``sudo``), and so
+        is each :data:`INTRODUCERS` word together with its own flags, so the
+        walk keeps going until it reaches something that is not a wrapper. Every
+        word it passed through is returned, not just the last, because ``xargs``
+        is itself worth refusing in some classes and ``sudo`` is worth refusing
+        in all of them.
+        """
+
+        found: list[str] = []
+        walking = False
+        for word in self.argv:
+            if self.is_assignment(word) or word.startswith("-"):
+                continue
+            if walking and word[:1].isdigit():
+                # A wrapper's own operand: the ``5`` in ``nice -n 5 pytest``,
+                # the ``30`` in ``timeout 30 pytest``. Only skipped once a
+                # wrapper has been passed, so a command whose name starts with a
+                # digit (``7z``) is still read as the command.
+                continue
+            found.append(word)
+            if word not in self.INTRODUCERS:
+                break
+            walking = True
+        return tuple(found)
+
+    def operands(self, after: str) -> tuple[str, ...]:
+        """Non-flag words following the ``after`` word.
+
+        Everything after a bare ``--`` is an operand, matching the convention
+        every one of these binaries follows.
+        """
+
+        if after not in self.argv:
+            return ()
+        found: list[str] = []
+        end_of_flags = False
+        for word in self.argv[self.argv.index(after) + 1 :]:
+            if word == "--":
+                end_of_flags = True
+                continue
+            if not end_of_flags and word.startswith("-"):
+                continue
+            found.append(word)
+        return tuple(found)
+
+    @property
+    def short_flags(self) -> frozenset[str]:
+        """Every letter in every ``-abc`` cluster, plus every ``--long`` word."""
+
+        letters: set[str] = set()
+        for word in self.argv:
+            if word.startswith("--"):
+                letters.add(word)
+            elif word.startswith("-") and len(word) > 1:
+                letters.update(word[1:])
+        return frozenset(letters)
+
+
+class ParsedCommandLine:
+    """Command-position structure over a ``shlex``-tokenised line.
+
+    **Nothing here is a hand-rolled lexer, and that is the point.** It used to
+    be: ~400 lines of ``CommandToken`` / ``CommandLexer`` / ``_read_single`` /
+    ``_read_double`` / ``_operator_at`` / ``_split``, carrying a docstring that
+    gave three reasons ``shlex`` would not do. Two of the three dissolved under
+    inspection and the third costs four lines:
+
+    1. *"shlex.split raises on an unbalanced quote."* It does — and the answer
+       is to CATCH it and fail **closed** (:attr:`malformed`). A line the shell
+       itself would reject as a syntax error is not a line we owe a parse, and
+       refusing it is strictly safer than the old lexer's run-to-end-of-string.
+       Upstream ``deepagents_code`` catches the same ``ValueError`` and returns
+       "not allowed" for the same reason.
+    2. *"shlex discards operators, and* ``&&`` *vs* ``|`` *is the distinction
+       §8.3 is built on."* True only of the ``shlex.split`` convenience wrapper.
+       ``shlex.shlex(punctuation_chars=…)`` emits operators as their own tokens,
+       so the distinction survives.
+
+       ⚠️ It does **not** survive upstream's four-line
+       ``re.split(r"&&|\\|\\||[|;]", …)``-then-``shlex.split`` composition, and
+       this is the one place we deliberately do NOT copy ``deepagents_code``.
+       Splitting the raw string first splits **inside quotes**, so each half
+       carries an unbalanced quote and the parse fails — every one of the seven
+       ordinary commands in
+       ``test_never_list.py::TestWhyNotUpstreamsFourLineComposition``
+       (``git commit -m "fix; drop the table"``, ``grep -R "foo|bar" .``,
+       ``echo "a && b"`` among them) comes back ``ValueError`` under it. For
+       upstream that is harmless, because failing closed on an ALLOW-list only
+       means "ask the human". Here it would mean :meth:`CommandNeverList.screen`
+       refusing a well-quoted command unappealably, with a note telling the
+       author to fix quoting that was never broken. Quoting must be applied
+       FIRST; that is the whole reason this is ``shlex.shlex`` and not the
+       shorter composition.
+    3. *"shlex cannot report the raw spelling of a token."* This one **survives**
+       — and it costs a second ``_lex`` at ``posix=False`` rather than a lexer.
+       Both streams are load-bearing in opposite directions: only the verbatim
+       one still reads ``cat C:\\Users\\me\\.ssh\\id_rsa`` as a path (POSIX mode
+       eats the backslashes), and only the dequoted one sees ``.env`` in
+       ``cat '.env'``. See :attr:`spellings` and :attr:`first_word`.
+
+       The second stream turned out to be load-bearing for a fourth reason
+       nobody predicted, and it is the one that would have shipped a hole:
+       **the POSIX stream cannot tell a quoted operator from a real one**, so
+       segmenting it alone passed ``rm -rf '|' /``. :attr:`segments` carries
+       both parses because of it.
+
+    ``punctuation_chars`` is extended past ``shlex``'s default ``();<>|&`` with
+    the backtick and the two line terminators, because all three open a command
+    position and none is punctuation to ``shlex``. ``whitespace`` is narrowed to
+    space and tab for the same reason — a newline must reach the token stream as
+    a separator rather than be eaten as whitespace, or ``pytest\\nsudo ls`` reads
+    as one command with two arguments.
+    """
+
+    #: Characters ``shlex`` returns as their own tokens. Consecutive runs come
+    #: back as ONE token (``&&``, ``>>``, ``()``, ``|||``), which is why the
+    #: classifiers below test characters rather than whole strings.
+    _PUNCTUATION: Final = "();<>|&`\n\r"
+    #: Narrowed from ``shlex``'s default ``" \t\r\n"`` so ``\n`` and ``\r`` stay
+    #: punctuation instead of being eaten as whitespace.
+    _TOKEN_WHITESPACE: Final = " \t"
+    #: A punctuation run containing either of these is a REDIRECTION, whose next
+    #: word is a target and never a command — so ``echo x > sudo`` does not read
+    #: as running ``sudo``, and so the target is available to the raw-device
+    #: check. Tested BEFORE the separator case, so ``>&`` is a redirection.
+    _REDIRECTION_CHARS: Final = "<>"
+
+    __slots__ = ("_malformed", "_operators", "_raw_words", "_segments", "_words")
+
+    def __init__(self, command: str) -> None:
+        posix = self._lex(command, posix=True)
+        verbatim = self._lex(command, posix=False)
+        # ``malformed`` is the POSIX answer alone, deliberately. It means "the
+        # shell itself would reject this line", and only the POSIX lex is
+        # faithful enough to say so: the verbatim lex raises on ordinary
+        # commands such as ``rm -rf x'|'y /`` (see :meth:`_lex`), and a
+        # verbatim-driven refusal would be an unappealable no to a line that is
+        # perfectly valid.
+        self._malformed = posix is None
+        posix_tokens = posix or ()
+        # The operator list answers "is this ONE simple command", which is a
+        # question about the line the SHELL sees — so it reads the POSIX stream,
+        # where quoting has been applied.
+        self._operators = tuple(
+            token for token in posix_tokens if self._is_operator(token)
+        )
+        self._words = tuple(
+            token for token in posix_tokens if not self._is_operator(token)
+        )
+        verbatim_tokens = verbatim or ()
+        self._raw_words = tuple(
+            token for token in verbatim_tokens if not self._is_operator(token)
+        )
+        # Both parses, but only once when they agree — which is the ordinary
+        # case, since the two differ only over quoting that contains an
+        # operator character. A verbatim lex that RAISED contributes nothing:
+        # its empty segment would carry no candidates, and ``malformed`` has
+        # already decided that a genuinely unparseable line is refused.
+        # See :attr:`segments`.
+        faithful = self._split(posix_tokens, verbatim=False)
+        second = () if verbatim is None else self._split(verbatim_tokens, verbatim=True)
+        self._segments = faithful if second in ((), faithful) else faithful + second
+
+    @classmethod
+    def of(cls, command: str) -> "ParsedCommandLine":
+        """Tokenise and structure ``command``. Never raises."""
+
+        return cls(command)
+
+    @property
+    def malformed(self) -> bool:
+        """True when the line does not tokenise — unbalanced quote, dangling escape.
+
+        The narrowing answer rather than an exception: the ``CommandNeverList``
+        Protocol ``policy_gate`` declares is total, so a line we cannot parse
+        becomes a REFUSAL upstream, never a broken run.
+        """
+
+        return self._malformed
+
+    @property
+    def segments(self) -> tuple[CommandSegment, ...]:
+        """Both parses' segments, concatenated. A hazard in **either** is a hazard.
+
+        ⚠️ This is not belt-and-braces. The two streams get quoting wrong in
+        OPPOSITE directions, and each direction has a measured case where the
+        error cuts an operand away from its command and the hazard stops being
+        visible at all:
+
+        * the POSIX stream has already discarded the quotes, so ``rm -rf '|' /``
+          arrives as ``('rm', '-rf', '|', '/')``, the third token reads as a
+          real pipe, and ``/`` lands in a different segment from ``rm``. The old
+          hand-rolled lexer refused that line; segmenting the POSIX stream alone
+          passed it;
+        * the verbatim stream keeps the quotes, but ``shlex`` in non-POSIX mode
+          only enters quote state at the START of a token — so a MID-word quote
+          is copied literally and ``awk -F';' …`` splits at a ``;`` the shell
+          would never see, which cuts operands away exactly the same way.
+
+        Neither stream dominates, so the fail-closed reading is the union.
+        Detectors iterate this tuple and stop at the first hit, so a hazard seen
+        in either parse is refused. The cost is over-refusal on lines where the
+        two disagree — measured, that is escaped operators (``echo a\\;sudo``) —
+        which is the direction tier NEVER is allowed to err in.
+
+        The concatenation is safe for :meth:`CommandNeverList._pipe_to_interpreter`,
+        whose ``fetched`` flag carries across segments, because :meth:`_split`
+        always emits its first segment with an empty ``lead`` — so the second
+        parse resets the carry rather than joining the first parse's tail.
+        """
+
+        return self._segments
+
+    @property
+    def operators(self) -> tuple[str, ...]:
+        """Every control / redirection operator token, in order."""
+
+        return self._operators
+
+    @property
+    def spellings(self) -> tuple[str, ...]:
+        """Every word, in BOTH the dequoted and the verbatim spelling.
+
+        The two credential detectors ask "does any word name a credential",
+        which is a set question — so this is a flat concatenation rather than an
+        aligned pairing, and it has to be, because the two streams are not
+        alignable in general (``a\\ b`` is one token in POSIX mode and two
+        without it). It is deliberately NOT deduplicated: the union is also what
+        ``test_never_list.py`` subtracts from the command to assert that no
+        character of the input is invisible to this scan, and a dedup would make
+        that property untestable to save nothing on a list this short.
+
+        Both spellings are load-bearing, in OPPOSITE directions, and each has a
+        test pinning the case only it catches:
+
+        * POSIX mode strips the escapes from ``C:\\Users\\me\\.ssh\\id_rsa``
+          exactly as the shell does, leaving ``C:Usersme.sshid_rsa`` — so only
+          the **verbatim** spelling still reads as a path;
+        * the verbatim spelling of ``cat '.env'`` keeps the quotes, so ``.env``
+          is not the leaf name — only the **dequoted** spelling matches.
+        """
+
+        return (*self._words, *self._raw_words)
+
+    @property
+    def first_word(self) -> tuple[str, str] | None:
+        """``argv[0]`` as ``(dequoted, verbatim)``, or ``None`` when there is none.
+
+        The one place the two spellings are COMPARED rather than unioned. §8.3
+        may only offer a run-scoped grant for a command whose literal text is
+        what a glob will later be matched against, so a head that quoting
+        changed — ``"sudo" rm``, ``s'u'do rm`` — forfeits it. Both of those
+        dequote to ``sudo``, which is why comparing the two spellings is the
+        check and a prefix test on the raw string is not.
+        """
+
+        if not self._words or not self._raw_words:
+            return None
+        return self._words[0], self._raw_words[0]
+
+    @classmethod
+    def _lex(cls, command: str, *, posix: bool) -> tuple[str, ...] | None:
+        """``shlex`` tokens, or ``None`` when the line cannot be parsed.
+
+        ⚠️ ``commenters`` is cleared, and that line is load-bearing. ``shlex``
+        defaults it to ``#`` and implements a comment by calling
+        ``instream.readline()`` — which swallows the rest of the line
+        **including its newline**. Since a newline is a separator here and not
+        whitespace (see :data:`_TOKEN_WHITESPACE`), the default made
+        ``echo hi #c\\nsudo rm -rf /`` tokenise as ONE segment with ``sudo`` in
+        argument position rather than command position, and the screen passed
+        it. Machine-checked, and pinned by ``test_never_list.py``. With ``#``
+        an ordinary word character the same line splits at the newline and
+        ``sudo`` is refused; a genuine trailing comment merely becomes an
+        operand, which no detector reads as a command.
+        """
+
+        lexer = shlex.shlex(command, posix=posix, punctuation_chars=cls._PUNCTUATION)
+        lexer.whitespace = cls._TOKEN_WHITESPACE
+        lexer.whitespace_split = True
+        lexer.commenters = ""
+        try:
+            return tuple(lexer)
+        except ValueError:
+            return None
+
+    @staticmethod
+    def _dequote(token: str) -> str:
+        """One verbatim token with its quoting removed, or unchanged on failure.
+
+        A verbatim token contains no unquoted whitespace by construction, so
+        ``shlex.split`` returns exactly one element or raises. The failure case
+        is a dangling escape (``a\\``), which the whole-line POSIX lex has
+        already reported as :attr:`malformed`; returning the token unchanged
+        keeps this total rather than adding a second way to signal the same
+        thing.
+        """
+
+        try:
+            return "".join(shlex.split(token))
+        except ValueError:
+            return token
+
+    @classmethod
+    def _is_operator(cls, token: str) -> bool:
+        """True for a token made entirely of punctuation characters.
+
+        Correct on the verbatim stream and only approximate on the POSIX one,
+        which is the asymmetry :attr:`segments` exists to absorb: a token that
+        is a QUOTED operator and nothing else (``echo '|'``) has already had its
+        quotes removed by POSIX tokenising and is indistinguishable here from a
+        real one. Splitting there is not merely a widening — it also cuts the
+        operands that follow away from the command that owns them, which is how
+        ``rm -rf '|' /`` came to pass a screen the old lexer refused. The
+        verbatim stream still holds ``"'|'"``, which is not a punctuation run,
+        so asking both parses recovers it.
+        """
+
+        return bool(token) and all(char in cls._PUNCTUATION for char in token)
+
+    @classmethod
+    def _is_redirection(cls, token: str) -> bool:
+        """True for a punctuation run that redirects rather than separates."""
+
+        return any(char in cls._REDIRECTION_CHARS for char in token)
+
+    @classmethod
+    def _split(
+        cls, tokens: tuple[str, ...], *, verbatim: bool
+    ) -> tuple[CommandSegment, ...]:
+        """Group one token stream into segments; dequote words if ``verbatim``.
+
+        A redirection does NOT start a new segment: it consumes exactly the one
+        word that follows it and the rest of the line stays with the command it
+        belongs to, so ``rm -rf > /tmp/log /`` still has ``/`` among ``rm``'s
+        operands.
+
+        On the verbatim stream the classification happens BEFORE the dequoting,
+        which is the whole reason that stream is segmented at all — see
+        :attr:`segments` for why one stream is not enough.
+        """
+
+        segments: list[CommandSegment] = []
+        lead = ""
+        argv: list[str] = []
+        targets: list[str] = []
+        pending_target = False
+
+        for token in tokens:
+            if cls._is_operator(token):
+                if cls._is_redirection(token):
+                    pending_target = True
+                    continue
+                segments.append(
+                    CommandSegment(
+                        lead=lead, argv=tuple(argv), redirect_targets=tuple(targets)
+                    )
+                )
+                lead, argv, targets, pending_target = token, [], [], False
+                continue
+            word = cls._dequote(token) if verbatim else token
+            if pending_target:
+                targets.append(word)
+                pending_target = False
+                continue
+            argv.append(word)
+
+        segments.append(
+            CommandSegment(lead=lead, argv=tuple(argv), redirect_targets=tuple(targets))
+        )
+        return tuple(segments)
+
+
+class CommandNeverList:
+    """The §9 floor: the tokenising screen, the compiled rows, the grant patterns.
+
+    Implements the ``CommandNeverList`` Protocol that
+    ``capabilities/shell/policy_gate.py`` declares. Every method is total; a
+    line that cannot be parsed returns the narrowing answer
+    (:attr:`ParsedCommandLine.malformed` ⇒ a refusal) rather than raising into
+    the tool path.
+
+    ⚠️ **Three tiers, and only two of them live here.** The names in this file
+    and the names in ``vendored_deepagents_safety`` answer different questions,
+    and reading one as the other would ship a severe regression:
+
+    ``NEVER``
+        Refuse outright; no human may approve. :meth:`screen` and :meth:`floor`.
+        Credential paths, ``sudo``, ``rm -rf /``, fork bombs, ``mkfs``,
+        pipe-to-interpreter.
+    ``ASK``
+        Run it, but a human approves it. **This is where** ``pytest``,
+        ``npm test``, ``git status`` **and** ``make`` **live**, and it is the
+        whole product promise of Phase 1. Everything not caught by tier NEVER is
+        in this tier today.
+    ``AUTO``
+        Safe enough to run without asking — :meth:`auto_approvable`, and
+        **nothing in this repository consults it yet**. See that method for why
+        wiring it as the gate would be a blocker-grade regression.
+    """
+
+    # -- hazard vocabulary -------------------------------------------------
+
+    _PRIVILEGE: Final = frozenset({"sudo", "doas", "su"})
+    #: ``-S`` makes ``sudo`` read a password from stdin. We close stdin, so this
+    #: is credential brute-forcing rather than an invocation (Hermes
+    #: ``approval.py:486-515``). Checked anywhere, not only in command position.
+    _STDIN_PASSWORD_FLAG: Final = "S"
+    _MACHINE_STATE: Final = frozenset({"shutdown", "reboot", "halt", "poweroff"})
+    _SYSTEMCTL: Final = "systemctl"
+    _SYSTEMCTL_TARGETS: Final = frozenset({"poweroff", "reboot", "halt"})
+    _INIT: Final = "init"
+    _INIT_RUNLEVELS: Final = frozenset({"0", "6"})
+    _REMOVE: Final = "rm"
+    _RECURSIVE_FLAGS: Final = frozenset({"r", "R", "--recursive"})
+    #: The whole set of operands that make a recursive delete catastrophic,
+    #: after :meth:`_normalise_operand` has stripped a trailing ``/`` or ``/*``.
+    _CATASTROPHIC_ROOTS: Final = frozenset({"/", "~", "$HOME", "${HOME}"})
+    _MKFS: Final = "mkfs"
+    _DD: Final = "dd"
+    _DD_OUTPUT: Final = "of="
+    _DEVICE_ROOT: Final = "/dev/"
+    #: Raw block devices. ``rdisk`` is macOS's raw device node and is the one
+    #: that matters on the platform this app ships on; §9.2 lists
+    #: ``disk|sd|nvme`` and the addition is called out in the report.
+    _RAW_DEVICES: Final = ("disk", "rdisk", "sd", "nvme", "hd", "vd")
+    _FETCHERS: Final = frozenset({"curl", "wget", "iwr", "invoke-webrequest", "aria2c"})
+    _INTERPRETERS: Final = frozenset(
+        {
+            "sh",
+            "bash",
+            "zsh",
+            "dash",
+            "ksh",
+            "fish",
+            "csh",
+            "tcsh",
+            "perl",
+            "ruby",
+            "node",
+            "deno",
+            "bun",
+            "php",
+            "osascript",
+            "pwsh",
+            "powershell",
+            "iex",
+        }
+    )
+    _INTERPRETER_PREFIX: Final = "python"
+    _PIPE_OPERATORS: Final = frozenset({"|", "|&"})
+    #: ADDED (not in §9.2). A shell invoked with ``-c`` carries a whole command
+    #: line inside ONE quoted word, so every tokenising judgement above is blind
+    #: to it and every end-anchored floor row misses it too: machine-checked,
+    #: ``sh -c "rm -rf /"`` passed both readers before this. One level of
+    #: recursion closes it; the payload is screened as what it is, a command
+    #: line, so this adds no false-positive shape that the top level does not
+    #: already have.
+    _SHELL_BINARIES: Final = frozenset(
+        {"sh", "bash", "zsh", "dash", "ksh", "fish", "csh", "tcsh"}
+    )
+    #: The short-flag letter that means "the next operand is a command string".
+    #: Matched inside a cluster so ``bash -lc "…"`` is seen as well as ``-c``.
+    _COMMAND_STRING_FLAG: Final = "c"
+    #: One level only. A shell inside a shell inside a shell is not a shape we
+    #: owe coverage to, and an unbounded walk is an unbounded loop on the tool
+    #: path.
+    _MAX_NESTED_DEPTH: Final = 1
+    #: ``name () { ... }`` with a body that pipes the name into itself and
+    #: backgrounds it. Every metacharacter is matched literally; the body is
+    #: bounded so a pathological line cannot make this quadratic.
+    _FORK_BOMB: ClassVar[re.Pattern[str]] = re.compile(
+        r"([^\s(){};|&]{1,32})\s*\(\s*\)\s*\{(.{0,256}?)\}", re.DOTALL
+    )
+    #: Binaries that forfeit the §8.3 always-grant because ``argv[0]`` is not
+    #: the command actually being run. §8.3's list verbatim, with ``make``
+    #: resolved IN (OQ-2, the narrowing direction: it runs a file the agent may
+    #: have written), plus the additions marked below.
+    _WRAPPER_BINARIES: Final = frozenset(
+        {
+            "env",
+            "sh",
+            "bash",
+            "zsh",
+            "sudo",
+            "doas",
+            "nice",
+            "nohup",
+            "xargs",
+            "time",
+            "timeout",
+            "script",
+            "ssh",
+            "docker",
+            "make",
+            # additions, each the same class as an entry above:
+            "su",
+            "dash",
+            "ksh",
+            "fish",  # other spellings of "a shell"
+            "command",
+            "exec",
+            "eval",
+            "builtin",  # POSIX re-dispatchers
+            "npx",
+            "bunx",
+            "uvx",
+            "pipx",  # fetch-then-run launchers
+        }
+    )
+    #: Characters that cannot appear in an ``argv[0]`` a grant pattern is built
+    #: from. ``*`` and ``?`` are the only two ``_compile`` treats as
+    #: metacharacters, so either would silently widen the grant to everything.
+    _PATTERN_METACHARACTERS: Final = "*?"
+    #: Tier AUTO's allow-list, upstream's table verbatim. Held as a list because
+    #: that is ``is_shell_command_allowed``'s parameter type, and built once so
+    #: the vendored predicate is not handed a fresh list per call. Consulted
+    #: ONLY by :meth:`auto_approvable` — see that method for why.
+    _AUTO_SAFE: ClassVar[list[str]] = list(RECOMMENDED_SAFE_SHELL_COMMANDS)
+
+    _floor_rules: ClassVar[PermissionRuleset | None] = None
+
+    # -- the Protocol ------------------------------------------------------
+
+    def screen(self, command: str) -> ShellRefusal | None:
+        """The §9.3 lexical screen. A refusal means **no card is ever drawn**.
+
+        Runs on the full, untruncated command string, before the PEP is entered
+        and before the capability is even described — a never-listed command
+        must not be carded, and the 1024-character subject cap that blinds the
+        floor (``PolicySubjects._MAX_CHARS``, ``rules.py:342``) does not apply
+        here.
+        """
+
+        return self._screen(command, depth=0)
+
+    def _screen(self, command: str, *, depth: int) -> ShellRefusal | None:
+        """One pass of the screen, plus one recursion into a ``sh -c`` payload."""
+
+        line = ParsedCommandLine.of(command)
+        for detector in (
+            self._privilege,
+            self._destructive_delete,
+            self._filesystem_destruction,
+            self._machine_state,
+            self._pipe_to_interpreter,
+            self._credential_path,
+            self._credential_file,
+        ):
+            note = detector(line)
+            if note is not None:
+                return self._refusal(note)
+        if self._fork_bomb(command):
+            return self._refusal(_Note.FORK_BOMB)
+        if line.malformed:
+            # Fail CLOSED, after the raw-string check that does not need tokens.
+            # The old hand-rolled lexer was total by running an unterminated
+            # quote to the end of the string and guessing; refusing is at least
+            # as safe and is what upstream's `is_shell_command_allowed` does on
+            # the same `ValueError`. Nothing is lost: the shell would reject the
+            # same line as a syntax error.
+            return self._refusal(_Note.UNPARSEABLE)
+        if depth >= self._MAX_NESTED_DEPTH:
+            return None
+        for payload in self._command_strings(line):
+            nested = self._screen(payload, depth=depth + 1)
+            if nested is not None:
+                return nested
+        return None
+
+    def floor(self) -> PermissionRuleset:
+        """The coarse whole-command DENY rows for the PDP's ``_never`` ruleset.
+
+        Compiled once and shared: the rows are constants and
+        ``PermissionRuleset`` is frozen, so a per-call rebuild would only add
+        allocation to the hottest path in the runtime.
+        """
+
+        cached = CommandNeverList._floor_rules
+        if cached is None:
+            cached = PermissionRuleset(
+                rules=tuple(
+                    PermissionRule(pattern=pattern, action=RuleAction.DENY)
+                    for pattern in self.rows()
+                )
+            )
+            CommandNeverList._floor_rules = cached
+        return cached
+
+    def always_grant_patterns(self, command: str) -> tuple[str, ...]:
+        """``argv[0]``-keyed rule patterns for a run-scoped grant, or ``()``.
+
+        ``()`` means this command may not earn a standing yes. Parsing never
+        widens here — it only narrows what we offer — so every ambiguity
+        returns ``()`` and costs a click rather than granting one.
+
+        The pair is ``argv0`` and ``argv0 *``, never ``argv0*``: a rule matches
+        by ``fullmatch`` over the whole command line, and the trailing space is
+        the only word boundary the vocabulary has. Without it a ``pytest`` grant
+        would also cover ``pytest-watch --exec "curl … | sh"`` (§8.3).
+
+        ⚠️ **This is tier ASK, not tier AUTO** — the name is the trap. What it
+        offers is written only after a human answers ``always`` on a card they
+        read, and it expires with the run. Gating it on
+        :meth:`auto_approvable`'s twenty-five-reader allow-list would mean a
+        human could never say "always" to ``pytest``, which is §8.3's own worked
+        example.
+
+        The metacharacter guard IS upstream's, though, and that is a real
+        improvement rather than a rename. ``contains_dangerous_patterns`` covers
+        everything the hand-rolled operator scan covered, plus four shapes that
+        were live holes — a bare ``$VAR``, ANSI-C ``$'…'``, a here-string, and a
+        bare tab. ``pytest $EXTRA`` and ``pytest $IFS`` both used to earn a
+        standing grant, because the old check only looked for the braced
+        ``${``: the text the human approved and the text the shell runs were
+        different strings.
+        """
+
+        if contains_dangerous_patterns(command):
+            return ()
+        line = ParsedCommandLine.of(command)
+        if line.malformed or line.operators:
+            return ()
+        head = line.first_word
+        if head is None:
+            return ()
+        name, verbatim = head
+        # Quoting changed the head, so the literal text a glob will be matched
+        # against is not the binary name. ``"sudo" rm`` and ``s'u'do rm`` both
+        # dequote to ``sudo``; comparing the two spellings catches both, where a
+        # prefix test on the raw string catches only the first.
+        if not name or name != verbatim:
+            return ()
+        if CommandSegment.is_assignment(name):
+            return ()
+        if any(char in name for char in self._PATTERN_METACHARACTERS):
+            return ()
+        if name in self._WRAPPER_BINARIES or name in CommandSegment.INTRODUCERS:
+            return ()
+        if self.screen(command) is not None:
+            return ()
+        return (name, f"{name} *")
+
+    # -- tier AUTO, which is not this module's gate -------------------------
+
+    @classmethod
+    def auto_approvable(cls, command: str) -> bool:
+        """Upstream's **auto-approve** judgement — tier AUTO. ⚠️ **NOT the gate.**
+
+        ``RECOMMENDED_SAFE_SHELL_COMMANDS`` holds twenty-five readers (``ls``,
+        ``cat``, ``grep``, ``ps``, ``wc``) and **no** ``pytest``, ``npm``,
+        ``git`` or ``make``. Two ways to misread it, both of which this
+        docstring exists to stop:
+
+        * **As tier NEVER's gate.** Swapping :meth:`screen`'s deny-list for this
+          allow-list looks like a large, principled deletion and is a
+          blocker-grade regression: the agent could no longer run your test
+          suite at all, which is the entire product promise of Phase 1.
+        * **As tier NEVER's whole answer.** It is a COMMAND allow-list, so
+          ``is_shell_command_allowed("cat ~/.ssh/id_rsa", …)`` is ``True`` —
+          ``cat`` is an allowed reader. Every path rule in
+          :class:`SensitivePathPolicy` must survive independently of it, or
+          approving a reader hands over ``id_rsa``.
+
+        Nothing consults this yet, and that is honest rather than dead: Phase 1
+        asks for a card on every command, so tier AUTO has no rung to sit on. It
+        is here so the tier boundary is machine-checked
+        (``test_never_list.py::TestTheThreeTiers``) rather than asserted in a
+        comment, and so the vendored table has one named home.
+        """
+
+        return is_shell_command_allowed(command, cls._AUTO_SAFE)
+
+    # -- the rows ----------------------------------------------------------
+
+    @classmethod
+    def rows(cls) -> tuple[str, ...]:
+        """Every whole-command glob merged into ``_never``, in table order.
+
+        Exposed so tests can assert on the literal rows: that none of them
+        begins with ``~`` or ``$HOME`` (so ``Wildcard.expand`` leaves every one
+        byte-identical), and that the deliberate misses stay misses.
+        """
+
+        rows: list[str] = []
+        # Root/home recursive delete. Anchored at end of line on purpose, so
+        # `rm -rf /Users/me/build` passes; `rm -rf / --no-preserve-root` is a
+        # deliberate floor MISS that the screen catches.
+        for flags in ("-rf", "-fr"):
+            rows += [f"*rm {flags} {root}" for root in ("/", "~", "$HOME")]
+        # Filesystem destruction.
+        rows += [
+            "mkfs*",
+            "*mkfs *",
+            "*dd *of=/dev/*",
+            "* >/dev/disk*",
+            "* > /dev/disk*",
+        ]
+        # Machine state. Two rows per binary: the bare invocation, and the
+        # invocation with any argument.
+        for binary in sorted(cls._MACHINE_STATE):
+            rows += [binary, f"*{binary} *"]
+        rows += ["*init 0", "*init 6", "*systemctl poweroff*", "*systemctl reboot*"]
+        # Privilege. `su` is omitted on purpose: `*su *` fires on `echo su x`,
+        # and a two-letter binary has no usable glob form. The screen has it.
+        rows += ["sudo", "*sudo *", "doas", "*doas *", "*sudo -S*"]
+        # Fork bomb. Every metacharacter in this row is escaped to a literal by
+        # `_compile`, which is exactly why the one row works.
+        rows += ["*:(){*};:*"]
+        # Pipe-to-interpreter from the network. `|` compiles to a literal pipe.
+        rows += [
+            "*curl *|*sh*",
+            "*curl *|*bash*",
+            "*curl *|*python*",
+            "*wget *|*sh*",
+            "*wget *|*bash*",
+            # ADDED (not in §9.2): the curl table has a python row and the wget
+            # table did not, so `wget -qO- https://x | python3` was a floor
+            # miss. Machine-checked: same shape, same false-positive class.
+            "*wget *|*python*",
+            "*iwr *|*iex*",
+        ]
+        rows += cls._segment_rows()
+        rows += cls._filename_rows()
+        return tuple(rows)
+
+    @classmethod
+    def _segment_rows(cls) -> tuple[str, ...]:
+        """Three rows per credential directory — end, deeper, then an argument.
+
+        A ``fullmatch`` needs one row for each way a segment can end the line,
+        carry more path, or be followed by a space. **Never author these as
+        ``~/.ssh/**``**: ``Wildcard.expand`` rewrites the leading ``~`` on the
+        pattern side while the command text keeps its literal ``~``, so pattern
+        and subject can never agree.
+
+        The capitalised variant is emitted for any segment without a leading
+        dot, which today means ``keychains`` only. It is not cosmetic: the
+        TypeScript rule is case-INSENSITIVE and ``Wildcard.match`` is not, and
+        the real macOS path is ``~/Library/Keychains`` — so without this row the
+        entire ``keychains`` entry is inert on the one platform that has it.
+        """
+
+        rows: list[str] = []
+        for segment in SensitivePathPolicy.ROOT_SEGMENTS:
+            spellings = [segment]
+            if not segment.startswith("."):
+                spellings.append(segment.capitalize())
+            for spelling in spellings:
+                rows += [f"*/{spelling}", f"*/{spelling}/*", f"*/{spelling} *"]
+        return tuple(rows)
+
+    @classmethod
+    def _filename_rows(cls) -> tuple[str, ...]:
+        """Two rows per credential filename, and a third for prefixes only.
+
+        ⚠️ The third form ``*.*`` is faithful to ``startsWith`` and to
+        nothing else. ``suffixes`` is ``endsWith`` and ``exact`` is equality, so
+        emitting it there converts an end-anchor into an infix and equality into
+        a prefix — machine-checked, that over-generalisation refuses thirteen
+        probe files ``isSensitiveFileName`` calls **not** sensitive
+        (``cert.pem.bak``, ``tls.pem.example``, ``docs/credentials.md``,
+        ``server.key.tpl``, ``.htpasswd.example``, ``.netrc.sample``,
+        ``.pgpass.template``). A floor row is unappealable, so each of those
+        would be a command nobody can run and nobody can approve.
+
+        ``.env`` is the reason the third form exists at all: it is not a suffix,
+        a prefix or an exact entry, so the first two forms emit nothing for it.
+        Machine-checked — ``*.env`` matches ``cat .env`` but not
+        ``cat .env.local``; ``*.env *`` matches neither; ``*.env.*`` matches
+        ``cat .env.local`` but not ``cat .env``. **Do not collapse the triple to
+        ``*.env*``**: that fires on ``echo x.envelope``.
+        """
+
+        rows: list[str] = []
+        two_form = (
+            *SensitivePathPolicy.FILE_SUFFIXES,
+            *SensitivePathPolicy.FILE_EXACT,
+        )
+        for entry in two_form:
+            rows += [f"*{entry}", f"*{entry} *"]
+        for entry in SensitivePathPolicy.FILE_PREFIXES:
+            rows += [f"*{entry}", f"*{entry} *", f"*{entry}.*"]
+        dotenv = SensitivePathPolicy.DOTENV
+        rows += [f"*{dotenv}", f"*{dotenv} *", f"*{dotenv}.*"]
+        return tuple(rows)
+
+    # -- detectors ---------------------------------------------------------
+
+    def _privilege(self, line: ParsedCommandLine) -> str | None:
+        """``sudo`` / ``doas`` / ``su`` in command position, or ``sudo -S``.
+
+        Command position is the whole point: ``git commit -m "no sudo here"``
+        lexes to four words, the last of which is the single token
+        ``no sudo here``, so nothing here fires on it. (The ``*sudo *`` floor
+        row does — that is the coarseness §9.2 accepts, and the corpus pins it.)
+        """
+
+        for segment in line.segments:
+            candidates = segment.candidates
+            if self._PRIVILEGE & set(candidates):
+                return _Note.PRIVILEGE
+            if self._PRIVILEGE & set(segment.argv) and (
+                self._STDIN_PASSWORD_FLAG in segment.short_flags
+            ):
+                return _Note.PRIVILEGE
+        return None
+
+    def _destructive_delete(self, line: ParsedCommandLine) -> str | None:
+        """A recursive ``rm`` whose operand resolves to ``/``, ``~`` or ``$HOME``.
+
+        §9.2 specifies "a recursive+force flag set". This requires **recursive
+        only**: the operand set is exactly three values for which no legitimate
+        invocation exists, so ``rm -r /`` and ``rm -r ~`` cost nothing to refuse
+        and are catastrophic to miss. The widening is deliberate and reported.
+        """
+
+        for segment in line.segments:
+            if self._REMOVE not in segment.candidates:
+                continue
+            if not (self._RECURSIVE_FLAGS & segment.short_flags):
+                continue
+            for operand in segment.operands(self._REMOVE):
+                if self._normalise_operand(operand) in self._CATASTROPHIC_ROOTS:
+                    return _Note.DESTRUCTIVE_DELETE
+        return None
+
+    def _filesystem_destruction(self, line: ParsedCommandLine) -> str | None:
+        """``mkfs*``, ``dd of=`` under ``/dev/``, or a redirect to a raw device."""
+
+        for segment in line.segments:
+            for candidate in segment.candidates:
+                if candidate.startswith(self._MKFS):
+                    return _Note.FILESYSTEM_DESTRUCTION
+                if candidate != self._DD:
+                    continue
+                for word in segment.argv:
+                    if word.startswith(self._DD_OUTPUT) and word[
+                        len(self._DD_OUTPUT) :
+                    ].startswith(self._DEVICE_ROOT):
+                        return _Note.FILESYSTEM_DESTRUCTION
+            for target in segment.redirect_targets:
+                if self._is_raw_device(target):
+                    return _Note.FILESYSTEM_DESTRUCTION
+        return None
+
+    def _machine_state(self, line: ParsedCommandLine) -> str | None:
+        """``shutdown`` / ``reboot`` / ``halt`` / ``poweroff``, ``init 0|6``, ``systemctl``."""
+
+        for segment in line.segments:
+            candidates = set(segment.candidates)
+            if self._MACHINE_STATE & candidates:
+                return _Note.MACHINE_STATE
+            if self._INIT in candidates and (
+                self._INIT_RUNLEVELS & set(segment.operands(self._INIT))
+            ):
+                return _Note.MACHINE_STATE
+            if self._SYSTEMCTL in candidates and (
+                self._SYSTEMCTL_TARGETS & set(segment.operands(self._SYSTEMCTL))
+            ):
+                return _Note.MACHINE_STATE
+        return None
+
+    def _pipe_to_interpreter(self, line: ParsedCommandLine) -> str | None:
+        """A pipeline whose source fetches from the network and whose sink runs code.
+
+        Grouped by ``|`` specifically, not by every control operator:
+        ``curl x && sh y`` is two commands and is a documented miss, while
+        ``curl x | tee /tmp/a | sh`` is one pipeline and is a hit.
+        """
+
+        fetched = False
+        for segment in line.segments:
+            if segment.lead not in self._PIPE_OPERATORS:
+                fetched = False
+            candidates = segment.candidates
+            if fetched and any(self._is_interpreter(name) for name in candidates):
+                return _Note.PIPE_TO_INTERPRETER
+            if self._FETCHERS & set(candidates):
+                fetched = True
+        return None
+
+    def _credential_path(self, line: ParsedCommandLine) -> str | None:
+        """Any word naming a path under a credential directory.
+
+        Both spellings, because they lose different things: POSIX tokenising
+        removes the ``\\`` from ``C:\\Users\\me\\.ssh\\id_rsa`` exactly as the
+        shell does, and the verbatim spelling is what still reads as a path.
+        """
+
+        if any(map(SensitivePathPolicy.has_sensitive_segment, line.spellings)):
+            return _Note.CREDENTIAL_PATH
+        return None
+
+    def _credential_file(self, line: ParsedCommandLine) -> str | None:
+        """Any word whose leaf name satisfies ``isSensitiveFileName``."""
+
+        if any(map(SensitivePathPolicy.leaf_is_sensitive, line.spellings)):
+            return _Note.CREDENTIAL_FILE
+        return None
+
+    def _command_strings(self, line: ParsedCommandLine) -> tuple[str, ...]:
+        """The ``-c`` payloads of any shell invoked in this line.
+
+        Only a shell: ``python -c`` carries Python source, not a command line,
+        and screening it as one would be a category error rather than extra
+        coverage.
+        """
+
+        payloads: list[str] = []
+        for segment in line.segments:
+            if not (self._SHELL_BINARIES & set(segment.candidates)):
+                continue
+            argv = segment.argv
+            for index, word in enumerate(argv):
+                if not self._is_command_string_flag(word):
+                    continue
+                payload = next(
+                    (
+                        operand
+                        for operand in argv[index + 1 :]
+                        if not operand.startswith("-")
+                    ),
+                    None,
+                )
+                if payload:
+                    payloads.append(payload)
+                break
+        return tuple(payloads)
+
+    @classmethod
+    def _is_command_string_flag(cls, word: str) -> bool:
+        """True for ``-c`` or any short cluster containing it, never ``--c…``."""
+
+        return (
+            word.startswith("-")
+            and not word.startswith("--")
+            and cls._COMMAND_STRING_FLAG in word[1:]
+        )
+
+    @classmethod
+    def _fork_bomb(cls, command: str) -> bool:
+        """``:(){ :|:& };:`` and its spelling variants.
+
+        Read off the raw text rather than the tokens because the shape *is*
+        punctuation: a definition whose body pipes its own name into itself and
+        backgrounds it. Whitespace is removed from the body before the check, so
+        ``: () { : | : & }; :`` is the same hit.
+        """
+
+        for match in cls._FORK_BOMB.finditer(command):
+            name = match.group(1)
+            body = "".join(match.group(2).split())
+            if f"{name}|{name}" in body and "&" in body:
+                return True
+        return False
+
+    # -- helpers -----------------------------------------------------------
+
+    @staticmethod
+    def _refusal(note: str) -> ShellRefusal:
+        """Every screen hit is the same closed reason; only the sentence differs."""
+
+        return ShellRefusal.refused(ShellRefusalReason.COMMAND_NOT_PERMITTED, note)
+
+    @staticmethod
+    def _normalise_operand(operand: str) -> str:
+        """Strip a trailing ``/`` or ``/*`` so ``/``, ``/*`` and ``~/`` compare equal."""
+
+        stripped = operand.rstrip("*").rstrip("/")
+        return stripped or "/"
+
+    @classmethod
+    def _is_raw_device(cls, target: str) -> bool:
+        """True for ``/dev/disk2``, ``/dev/rdisk0``, ``/dev/sda`` and friends."""
+
+        if not target.startswith(cls._DEVICE_ROOT):
+            return False
+        node = target[len(cls._DEVICE_ROOT) :]
+        return node.startswith(cls._RAW_DEVICES)
+
+    @classmethod
+    def _is_interpreter(cls, name: str) -> bool:
+        """True for a shell, ``python*``, or any other code-running binary."""
+
+        leaf = name.rsplit("/", 1)[-1]
+        return leaf in cls._INTERPRETERS or leaf.startswith(cls._INTERPRETER_PREFIX)
+
+
+__all__ = [
+    "CommandNeverList",
+    "CommandSegment",
+    "ParsedCommandLine",
+    "SensitivePathPolicy",
+]
diff --git a/services/ai-backend/src/agent_runtime/capabilities/shell/policy_gate.py b/services/ai-backend/src/agent_runtime/capabilities/shell/policy_gate.py
new file mode 100644
index 000000000..d917ed0fe
--- /dev/null
+++ b/services/ai-backend/src/agent_runtime/capabilities/shell/policy_gate.py
@@ -0,0 +1,547 @@
+"""The PEP for ``run_command`` — every command reaches a decision (§5, §8, §9).
+
+This is the Policy **Enforcement** Point. The Policy **Decision** Point is
+:class:`~agent_runtime.capabilities.policy.service.PdpPolicyService`, and the
+policy *data* it decides from is authored in ``services/backend`` and delivered
+once, at run-create, on ``AgentRuntimeContext.user_policies_json``. That split is
+the root ``CLAUDE.md`` rule this module exists to honour:
+
+    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.**
+
+Nothing here opens a socket. The ``execute`` mode arrives in the same snapshot
+the read / write / destructive modes already ride, and
+:class:`ShellCommandPolicyGate` reads it out of the run context it was
+constructed with.
+
+THE ORDER IS THE DESIGN
+-----------------------
+One call flows exactly:
+
+1. :meth:`CommandNeverList.screen` — the **pre-PDP lexical screen** (§9.3). It
+   runs on the full, untruncated command **before the PDP is entered**, because
+   it is the only reader that can tokenise: a ``PermissionRule``'s subject half
+   is one glob ``fullmatch``ed against one opaque string, with no word
+   boundaries, so it cannot express "``sudo`` in command position" — the
+   property that separates ``sudo rm -rf /`` from
+   ``git commit -m "no sudo here"``. A screen hit raises, and **no approval card
+   is ever created**: there is nothing to click past.
+2. :meth:`PdpPolicyService.decide` — availability, then authorization, then the
+   posture ladder, whose 3.5½ rung GATEs ``EXECUTE`` in every posture unless the
+   axis itself is authored ``auto``. That rung, not this module, is why
+   ``BYPASS`` does not auto-run a command.
+3. ``GATE`` ⇒ :meth:`ToolAccessGate.park_command_for_approval` — park the run on
+   a LangGraph interrupt, resume on the human's decision, in the same run.
+
+There is **no fourth path**. :meth:`ShellCommandPolicyGate.authorize` either
+returns a :class:`ShellAuthorization` — which only the ALLOW arm and the
+approved-resume arm construct — or raises :class:`ShellRefusedError`. A caller
+cannot obtain permission to run without a decision having produced it, because
+there is no other constructor of the permission object.
+
+FAIL-CLOSED POINTS, NAMED
+-------------------------
+* ``never_list`` is a **required** constructor argument, not an optional one. An
+  optional screen has an unwired state, and an unwired state on this seam is a
+  command reaching the PDP without §9.3 having run.
+* ``gate is None`` (no approval channel wired for this run) ⇒ a typed refusal,
+  never a silent dispatch. The invariant is
+  ``capabilities/mcp/middleware/policy_tool.py``'s, verbatim.
+* The shipped never-list floor is merged **LAST** into the PDP's ``_never``
+  ruleset, because ``PermissionRuleset.evaluate`` is last-match-wins and
+  ``merge`` concatenates (§9.5).
+* An always-grant is offered only when the never-list vouches for the command as
+  a single simple command, and the rule it writes is keyed on the tokeniser's
+  patterns — an empty pattern tuple writes nothing, so an ``always`` that
+  arrives for a command nobody vouched for degrades to ``once``.
+"""
+
+from __future__ import annotations
+
+from collections.abc import Mapping
+from enum import StrEnum
+from typing import Final, Protocol
+
+from agent_runtime.capabilities.actions.policy import ConnectorWritePolicyOverrides
+from agent_runtime.capabilities.policy.contracts import (
+    CapabilityDescriptor,
+    PolicyDecision,
+    Posture,
+)
+from agent_runtime.capabilities.policy.decisions import (
+    DecisionScope,
+    PendingAsk,
+    RunDecisionLedgers,
+)
+from agent_runtime.capabilities.policy.rules import PermissionRuleset
+from agent_runtime.capabilities.policy.service import PdpPolicyService
+from agent_runtime.capabilities.shell.contracts import (
+    ShellRefusal,
+    ShellRefusalReason,
+    ShellRefusedError,
+)
+from agent_runtime.capabilities.shell.descriptor import (
+    BuiltinCapabilityAllowlist,
+    RunCommandDescriptor,
+    ShellCapability,
+    ShellPrincipal,
+)
+from agent_runtime.capabilities.tools.permissions import ToolUsePolicySnapshot
+from agent_runtime.execution.contracts import AgentRuntimeContext, RuntimeContract
+from agent_runtime.execution.filesystem_bypass import FilesystemBypassMode
+from agent_runtime.surfaces_v2.gate import ToolAccessGate
+
+
+class _Note:
+    """Model-facing refusal sentences for decisions this module makes.
+
+    Authored constants, never interpolated from a PDP reason code: the codes are
+    deliberately coarse so model output cannot distinguish "missing scope" from
+    "not allowlisted", and rendering one into a sentence would un-coarsen it.
+    Each line says whether the condition is permanent, following the convention
+    ``PolicyStageMessages`` sets — a deterministic refusal described as
+    temporary sends the model into a retry loop that cannot succeed.
+    """
+
+    NOT_PERMITTED: Final = (
+        "This command is not permitted in this workspace. Nothing was run, and "
+        "there is nothing to approve: retrying will not change it."
+    )
+    WORKSPACE_GONE: Final = (
+        "The workspace this command would run in is no longer available. "
+        "Nothing was run."
+    )
+    DECLINED: Final = (
+        "You declined this command, so it was not run. Nothing on disk was "
+        "changed by it."
+    )
+    APPROVAL_UNAVAILABLE: Final = (
+        "Running a command needs your approval, which is not available for this "
+        "run, so it was not run. Retrying the same way will not change it."
+    )
+
+
+class _PdpReason:
+    """The one PDP DENY code this stage distinguishes.
+
+    Mirrors ``PdpPolicyService._Reason.CONNECTOR_UNAVAILABLE`` — the availability
+    denial, which for the command lane means the sealed enablement or the bound
+    grant went away (§7.2). Every other DENY (never-list floor, workspace
+    ``BLOCK``, an authored ``deny`` rule) collapses to "not permitted", exactly
+    as in the MCP stage: the PDP coarsens them on purpose and this table must not
+    un-coarsen them.
+    """
+
+    CONNECTOR_UNAVAILABLE: Final = "connector_unavailable"
+
+
+class DecisionBasis(StrEnum):
+    """What authorised one command. Carried into the §15.3 audit row.
+
+    Three values because there are exactly three ways a command becomes
+    runnable, and an audit row that cannot tell them apart cannot answer "who
+    approved this".
+    """
+
+    #: The PDP returned ALLOW without a card — the ``execute`` axis is authored
+    #: ``auto``, or a run-scoped grant from an earlier ``always`` answered it.
+    POLICY = "policy"
+    #: A human approved this call, and only this call.
+    APPROVED_ONCE = "once"
+    #: A human approved this call and authorised its ``argv[0]`` for the rest of
+    #: this run, in this workspace.
+    APPROVED_ALWAYS = "always"
+
+
+class ShellAuthorization(RuntimeContract):
+    """Permission to run **one** command, and the record of how it was obtained.
+
+    The only object that says "dispatch". It is constructed in exactly two
+    places in this module — the ALLOW arm and the approved-resume arm — and both
+    are downstream of :meth:`PdpPolicyService.decide`. That is the structural
+    form of "no path returns ALLOW without a decision": there is no other
+    constructor, so a future edit that skipped the PDP would have nothing to
+    return.
+    """
+
+    basis: DecisionBasis
+    #: The PDP's stable machine reason code for the decision that produced this.
+    #: Empty string for a plain ALLOW, matching ``_Reason.ALLOW``.
+    reason: str = ""
+    #: The approval id the card, the ledger and the resume joined on. ``None``
+    #: when no card was drawn.
+    approval_id: str | None = None
+    #: Whether the human was offered a run-scoped grant for this command.
+    always_offered: bool = False
+
+
+class CommandNeverList(Protocol):
+    """The tokenising judgements about a command line (§9.2, §9.3, §8.3).
+
+    Implemented by :mod:`agent_runtime.capabilities.shell.never_list`, which owns
+    the one data table both readers are derived from. Declared here as a port
+    because the PEP needs the *judgements*, not the table: this module must be
+    able to state the ordering property (screen strictly before the PDP) and the
+    fail-closed property (no patterns ⇒ no grant) without also owning a shell
+    tokeniser.
+
+    Every method is total. A tokeniser that cannot parse must return the
+    narrowing answer, never raise into the tool path.
+    """
+
+    def screen(self, command: str) -> ShellRefusal | None:
+        """The §9.3 lexical screen. A refusal means **no card is ever drawn**."""
+        ...
+
+    def floor(self) -> PermissionRuleset:
+        """The coarse whole-command DENY rows for the PDP's ``_never`` ruleset.
+
+        The same data table as :meth:`screen`, compiled for a reader that cannot
+        tokenise. Kept as a floor rather than dropped: it survives a bug in the
+        screen and it is the rung the PDP evaluates above every rule and every
+        posture, so it holds if a second call path ever skips the screen.
+        """
+        ...
+
+    def always_grant_patterns(self, command: str) -> tuple[str, ...]:
+        """``argv[0]``-keyed rule patterns for a run-scoped grant, or ``()``.
+
+        ``()`` means "this command may not earn a standing yes" — it is
+        compound, or its first token is a wrapper binary (``env``, ``sh``,
+        ``sudo``, ``xargs``…) that is not the command actually being run. The
+        emptiness does double duty: it withholds the ``allow_always`` control on
+        the card AND makes an ``always`` reply write no rule, so the two cannot
+        disagree.
+        """
+        ...
+
+
+class ShellCommandPolicyGate:
+    """Decide, park, and refuse — the one enforcement point for ``run_command``.
+
+    Constructed once per run by the tool factory and bound to the tool at
+    registration, so nothing model-supplied participates in choosing *what* is
+    being authorized: the binding is the tool, not the payload (the first of the
+    three properties ``PolicyToolMiddleware`` is built around).
+    """
+
+    __slots__ = ("_runtime_context", "_never_list", "_gate")
+
+    #: The approval-id namespace for this lane. Deliberately NOT ``mcp_write:``.
+    #:
+    #: Minting a lookalike would make two client predicates
+    #: (``allowsRunScopedGrant``, ``writeGateCallId``) start matching, which is
+    #: tempting and wrong: the prefix is the client's proof that the id's
+    #: suffix is an MCP tool-call id from the MCP policy middleware, and
+    #: borrowing it to mean something else is how a join silently binds the
+    #: wrong call. The command card binds to its tool call by ``tool_name``
+    #: instead — the second join ``eventProjector.ts::matchAskToCall`` already
+    #: implements for exactly the lanes whose id is not that shape, which is why
+    #: ``tool_name`` is on the interrupt payload.
+    _APPROVAL_PREFIX: Final = "shell_exec"
+
+    #: The key under which the root-scoped grant subject is offered to the PDP.
+    #: See :meth:`_policy_arguments` for why it exists.
+    _GRANT_SUBJECT_ARG: Final = "workspace_scoped_command"
+
+    def __init__(
+        self,
+        *,
+        runtime_context: AgentRuntimeContext,
+        never_list: CommandNeverList,
+        gate: ToolAccessGate | None,
+    ) -> None:
+        """Bind the run's policy inputs, its screen, and its approval channel.
+
+        ``never_list`` has no default **because there is no safe default**. An
+        optional screen is a screen that can be absent, and an absent screen is
+        §9.3 not running — which would be invisible, because the PDP floor would
+        still catch the coarse cases and the suite would look green.
+        """
+
+        self._runtime_context = runtime_context
+        self._never_list = never_list
+        self._gate = gate
+
+    async def authorize(
+        self,
+        *,
+        command: str,
+        workspace_label: str,
+        available: bool,
+        tool_call_id: str | None = None,
+    ) -> ShellAuthorization:
+        """Return permission to run ``command``, or raise a typed refusal.
+
+        ``available`` is the caller's §7.2 recheck — the sealed enablement AND
+        the bound root still being granted and still writable, re-read at call
+        time. It is threaded into the descriptor rather than checked here so the
+        recheck flows *through* the PDP (Stage 1) instead of becoming a second
+        gate that has to agree with the first.
+        """
+
+        # 1 — The pre-PDP lexical screen. Above everything, including the
+        #     descriptor: a never-listed command must not even be described,
+        #     let alone carded.
+        refusal = self._never_list.screen(command)
+        if refusal is not None:
+            raise ShellRefusedError(refusal)
+
+        descriptor = RunCommandDescriptor.for_availability(available=available)
+        arguments = self._policy_arguments(
+            command=command, workspace_label=workspace_label
+        )
+        decision, reason = self._pdp().decide(
+            principal=ShellPrincipal.for_run(self._runtime_context),
+            descriptor=descriptor,
+            args=arguments,
+            posture=self._posture(),
+        )
+        if decision is PolicyDecision.ALLOW:
+            return ShellAuthorization(basis=DecisionBasis.POLICY, reason=reason)
+        if decision is PolicyDecision.DENY:
+            raise ShellRefusedError(self._denial(reason))
+        return await self._park(
+            command=command,
+            workspace_label=workspace_label,
+            descriptor=descriptor,
+            arguments=arguments,
+            reason=reason,
+            tool_call_id=tool_call_id,
+        )
+
+    # -- decision inputs ---------------------------------------------------
+
+    def _pdp(self) -> PdpPolicyService:
+        """Compose the PDP from this run's sealed policy snapshot.
+
+        Rebuilt per call, from frozen run-scoped data plus the run ledger, for
+        the same reason ``McpDispatchPolicy.evaluate`` rebuilds it: the ledger's
+        rules change when a human answers ``always``, and a PDP cached at
+        registration would answer with the ruleset the run started under.
+
+        Two orderings are load-bearing and both are stated in one place here:
+
+        * ``rules`` is ``authored ⧺ this run's grants``. The ruleset is
+          last-match-wins, so a grant the user just answered overrides a broader
+          authored default and never the reverse.
+        * ``never`` is ``authored ⧺ the shipped floor``. The floor is merged
+          LAST so a permissive user row cannot sit after it and win (§9.5).
+        """
+
+        authored, authored_never = PermissionRuleset.authored(
+            self._runtime_context.user_policies_json
+        )
+        return PdpPolicyService(
+            snapshot=self._snapshot(),
+            overrides=ConnectorWritePolicyOverrides.from_user_policies(
+                self._runtime_context.user_policies_json
+            ),
+            allowlist=BuiltinCapabilityAllowlist(),
+            rules=authored.merge(
+                RunDecisionLedgers.for_run(self._runtime_context.run_id).rules
+            ),
+            never=authored_never.merge(self._never_list.floor()),
+        )
+
+    def _snapshot(self) -> ToolUsePolicySnapshot:
+        """The run's ``(axis → mode)`` map, folded once at run-create.
+
+        Imported lazily for the reason ``McpDispatchPolicy._snapshot`` is: the
+        tool-enforcement stack must not load on every import path that only
+        needs the shell contracts.
+        """
+
+        from agent_runtime.capabilities.tools.tool_use_enforcement import (  # noqa: PLC0415
+            ToolUsePolicyResolver,
+        )
+
+        return ToolUsePolicyResolver.resolve(self._runtime_context)
+
+    def _posture(self) -> Posture:
+        """Map the run's filesystem-bypass mode onto the approval posture.
+
+        Value-identical enums, mapped explicitly so ``BYPASS`` is only ever
+        selected for the literal bypass mode and never by a truthy accident.
+        The mapping is what makes §8.2's claim testable: the composer's bypass
+        pill reaches the ladder as ``Posture.BYPASS``, and rung 3.5½ GATEs it.
+        """
+
+        if self._runtime_context.filesystem_bypass.mode is FilesystemBypassMode.BYPASS:
+            return Posture.BYPASS
+        return Posture.MANUAL
+
+    @classmethod
+    def _policy_arguments(
+        cls, *, command: str, workspace_label: str
+    ) -> Mapping[str, object]:
+        """What the PDP sees as this call's arguments.
+
+        The first two are the call's real arguments, and folding them into
+        :class:`PolicySubjects` is what makes the command string itself a policy
+        subject — matched by the same rule engine that matches MCP tool
+        arguments.
+
+        The third is **not an argument the model supplied**, and it is here for a
+        mechanical reason worth stating rather than hiding. §8.3 keys a
+        run-scoped grant on ``(run_id, bound_root_label, argv[0])``, but the rule
+        engine has no conjunction: ``PermissionRuleset.verdict`` folds
+        independently over each subject and returns the strictest single match,
+        so no pair of patterns over ``command`` and ``workspace`` can express
+        "this ``argv[0]`` **and** this root". One subject carrying both facts
+        can. Hence ``run_command@