fix(permissions): say why the sandbox blocks a command in plain English

[jonathanKingston]

Problem

The "Run outside sandbox?" dialog printed the classifier's rule identifiers verbatim. A python3 - <<'EOF' … EOF prototype script produced:

This command needs access the macOS project sandbox blocks (heredoc script fed to an interpreter; inline script (interpreter -c/-e/--eval)).

Three things wrong with that:

1. "interpreter -c/-e/--eval" is a rule name, not an explanation. It's REASON_INTERPRETER_INLINE in packages/shell-guard/src/shell-scope.ts, matched by the inline-code regex and reinforced by the argv[0] token pass. A user cannot act on it.
2. The same fact is stated twice. The heredoc rule and the inline-code rule both mean "this runs code the classifier can't read", and both fire on a command that does both.
3. The sentence is missing its relative pronoun ("needs access that the … sandbox blocks"), so it garden-paths on "the macOS project sandbox blocks" reading as a verb phrase. It also claims to be macOS-only, which is wrong on Linux, where bubblewrap is the boundary.

docs/plans/docs-site.md already flags this area: "A user cannot read what the dialog is asking them."

Approach

Reason strings have to stay identifiers — the regex and token passes dedupe on them verbatim, and every answered prompt writes them into the decision spine. So this adds a copy layer rather than renaming them.

SCOPE_REASON_TEXT in shell-scope.ts holds one plain sentence per reason, and describeShellScopeReasons resolves a reason list at the moment a prompt is built. Logs, hooks and decision records are untouched.

Two properties hold it together:

• Every rule has copy. ScopeReason is derived from the table's keys and annotates the pattern tables, the shared reason constants, and the accumulators both classifier passes push through, so a new classifier rule whose reason has no sentence fails to typecheck. The list widens to string[] at exactly one point — a copy, so the widened list can never write a plain string back into the typed one — where the runtime-built absolute path outside workspace: … joins it. That one is matched by prefix, and anything still unrecognised is shown verbatim rather than dropped.
• One concern, one line. Deduping happens on the resolved sentence, so every rule meaning "code this analysis cannot read before it runs" collapses to a single line: a -c body, a heredoc, and eval/exec/base64 all share one sentence, so node --eval x (which trips two of them) says it once. ~/ and $HOME likewise share the home-directory line.

Prompts render one reason per line instead of a semicolon-joined parenthetical, and the sandbox-escape prompts name no platform (they only appear while a project sandbox is active — seatbelt on macOS, bubblewrap on Linux).

Result

BEFORE: This command needs access the macOS project sandbox blocks (inline script
        (interpreter -c/-e/--eval); heredoc script fed to an interpreter).

AFTER : The project sandbox would block this command:
        • Runs code written or built inside the command itself, so Copse can't tell what it does

BEFORE: This command needs access the macOS project sandbox blocks (network download
        (curl/wget); home directory path (~/)).

AFTER : The project sandbox would block this command:
        • Downloads from the internet (curl/wget)
        • Reads or writes in your home directory, outside the project

Every prompt variant that carries classifier reasons is covered: the two up-front escape prompts, the expects_sandbox_block one, the in-sandbox "Run shell command?" footer, the Guarded YOLO harm prompt, and the install / ephemeral-runner prompts.

Changes

File	Change
packages/shell-guard/src/shell-scope.ts	SCOPE_REASON_TEXT, ScopeReason, describeShellScopeReasons; pattern tables, shared constants and both accumulators typed against the union
src/main/services/security/permission-policy.ts	Bulleted reason rendering in the shell prompt formatters; rewritten advice sentences; the harm prompt resolves the same sentences
src/main/services/security/sandbox-failure.ts, packages/hooks-dialects/src/command-hook-runner.ts, packages/hooks-dialects/src/sandbox-failure-detection.ts	Drop the macOS-only claim from the sibling copy
src/renderer/views/approval-dialog.ts, src/renderer/styles/global/approval.css	Each reason bullet is its own inline-block, so a wrapped line hangs under its own text instead of reading as another bullet
src/shared/demo-scenarios.ts, tests/demo/approval-grouped-shell-commands.demo.ts	Demo copy and its assertion follow the new wording
docs/shell-permissions.md	New "What an approval prompt says" section pinning the contract

Validation

CI is green on 35dc90b: precheck, check (full unit suite), build, all eight e2e shards, screenshot-artifacts and CI Passed.

New coverage:

• src/main/services/security/shell-prompt-copy.test.ts — 8 cases over the three formatters, driven from real analyzeShellCommand output.
• describeShellScopeReasons cases in shell-scope.test.ts, including a regression test that node --eval reports the unreadable-code concern once.
• Two rendering cases in approval-dialog-batch.test.ts: multi-line advice, and the bullet-span structure that carries the hanging indent.
• The Guarded YOLO cap test uses distinct operands and enough of them to overrun the total budget, pinned at exactly 1200 characters.

Visual evidence

The demo scenarios were rendered from dist/demo over CDP while developing, which is where the wrapped-bullet defect showed up and was fixed. The e2e tier has since exercised the same scenarios in a real browser session, so the updated approval-grouped-shell-commands.demo.ts assertion is confirmed against the shipped renderer.

Needs a human

Reference screenshots — and note the screenshot review PR is incomplete. tests/e2e/screenshots/approval-grouped-shell-commands.png and approval-light-accent.png both change, but the candidate filter holds them as out-of-scope: computeScreenshotGate (test-oracle.mts:694) counts only src/** and tests/e2e/** as render-affecting and affectedScreenshots (:663) maps from the e2e selection alone, so a demo-tier shot can never be oracle-owned — not even when the diff edits the demo spec that renders it. Screenshot PR #2341 therefore carries only unrelated re-renders.

To land the two that matter: recover them from the screenshots-demo artifact on run 33859836149 and commit them (which also makes them branchOwned for later runs), or add the update-screenshots label and re-run. The scope gap itself is worth a separate issue.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Q9xawQf1Nu7VGA5Ktew66E

[github-actions]

🖥️ PR preview

• Marketing site
• Browser demo
  Interactive, with a mocked backend — jump to a scenario:
• Builds a cupcake site
• Agent-proposed edits open the Changes panel
• Markdown list indentation
• Responsive composer footer
• Subagent display visual reference
• Subagent stays ungrouped beside reading tools
• Semantic search subagent markdown
• Light-theme approval with a bright accent
• Grouped outside-sandbox command approval
• Remote desktop discovered-port list with one selected
• Settings scroll + sticky footer geometry
• Per-model reasoning / temperature / top-p
• Thread switch hydration notice
• Thread switch hydration failure
• Chat layout styling
  _{Static demo of the renderer against a mock API — the browser pane, terminal, and all main-process IPC are stubbed, so it never proves main-process behaviour. Rebuilt whenever a push changes the demo bundle; these links go live once the queued deploy lands, shortly after this comment. Removed when the PR closes.}

[jonathanKingston]

precheck is red on 3f246ed, and it is not this PR's failure.

Every step of the job passed — lint, format:check, check:dead-code — and it then died on pnpm audit --audit-level=high (ci.yml:214) with a network timeout reaching the npm registry, after exhausting both of pnpm's own retries:

WARN  POST https://registry.npmjs.org/-/npm/v1/security/audits/quick error (ERR_SOCKET_TIMEOUT). Will retry in 10 seconds. 2 retries left.
WARN  POST https://registry.npmjs.org/-/npm/v1/security/audits/quick error (ERR_SOCKET_TIMEOUT). Will retry in 1 minute. 1 retries left.
ERR_SOCKET_TIMEOUT  request to .../security/audits/quick failed, reason: Socket timeout

This diff changes no dependency, manifest or lockfile — it is TypeScript copy, tests and docs — so it cannot affect an audit of the dependency tree. The same step is green on the base branch (abbada3, run 10416).

There is no fix to port: a registry socket timeout has no code remedy. Re-running the failed job once, which is the one re-run I'll spend here. If this recurs, the durable fix belongs in CI rather than in this PR — pnpm audit is the only network-dependent step in precheck without the retry wrapper the lint step already has for OOM kills (ci.yml), so the same for attempt in … loop around it, or --audit-level=high || echo "::warning::audit unreachable", would stop a registry blip failing unrelated PRs. Happy to open that as a separate change if you want it.

---

Generated by Claude Code

[jonathanKingston]

Review: plain-English sandbox reasons

Verdict: the copy layer is sound and well tested, but the branch does not merge into current main, and the string the PR edits in the hook runner has moved. Mechanical, but blocking.

CI

The latest precheck failure is not this PR's: pnpm audit --audit-level=high timed out against registry.npmjs.org (ERR_SOCKET_TIMEOUT after three retries) before typecheck or lint ran. A re-run should clear it; everything below passed locally.

Check	Result
pnpm run typecheck	pass
oxfmt --check on the 11 changed files	pass
eslint on the changed .ts files	pass
pnpm test -- shell-scope shell-prompt-copy approval-dialog-batch sandbox-failure	124/124 pass
permission-gate.test.ts	cannot load node-pty on this Linux box (env issue, not the PR); its new assertions bundled and run ad hoc against the sources pass
pnpm run check:dead-code	pass
pnpm run oracle	broad (demo-scenarios.ts and a demo spec changed)
Dry merge with origin/main (1 behind)	CONFLICT in src/main/services/hooks/command-hook-runner.ts and src/main/services/security/sandbox-failure.ts

Blocking

Rebase and re-target the hook-runner edit. #2319 moved the hook runner into packages/hooks-dialects. On main the string this PR changes now lives at packages/hooks-dialects/src/command-hook-runner.ts:213 and still reads "blocked by the macOS project sandbox"; sandbox-failure.ts is now a re-export of @copse/hooks-dialects/sandbox-failure-detection.ts, whose header comment also says macOS. Resolving the conflict in favour of main silently drops the change, so a Linux (bubblewrap) user whose hook is blocked still sees the wrong platform in the Sources error, contradicting the new paragraph at docs/shell-permissions.md:88-89.

Non-blocking

• The "fails to typecheck" guarantee is narrower than the docs say. packages/shell-guard/src/shell-scope.ts:463 declares addReason = (reason: string), and REASON_INTERPRETER_FILE, REASON_INTERPRETER_INLINE, REASON_BUILD_DRIVER (lines 305 to 309) are only emitted through it, as are the pushes at 524, 710 and 725. I checked all 60 emitted literals and every one has a SCOPE_REASON_TEXT key today, but a future edit to one of those constants compiles and falls through to "shown verbatim", while docs/shell-permissions.md:82-83 promises a typecheck failure. Typing addReason and those three constants as ScopeReason makes the doc true as written.
• --eval still yields two near-duplicate lines. The comment at shell-scope.ts:812-815 says the interpreter and dynamic-execution matchers dedupe for --eval, but the two identifiers map to different sentences. node --eval "x" renders "Runs a script written inside the command itself, so Copse can't tell what it does" and "Builds and runs code as it goes (eval/exec/base64), so Copse can't tell what it does". Either share one sentence or fix the comment.
• The Guarded YOLO cap test no longer exercises the cap. src/main/services/security/permission-gate.test.ts:1533-1539 passes three identical operands; describeShellScopeReasons now dedupes them to one, so the advice is 267 chars and the 1200-char branch at permission-policy.ts:434-437 is untested. Use three distinct operands.
• docs/shell-permissions.md:76-77 says one bullet per line, but formatGuardedYoloHarmPromptAdvice (permission-policy.ts:432-433) still joins with ; . Worth a qualifier.
• Both tests/e2e/screenshots/approval-grouped-shell-commands.png and approval-light-accent.png will change (the light-accent scenario's bodyAdvice changed at src/shared/demo-scenarios.ts:600-601 and that demo captures the same element). The updated grouped-spec assertion satisfies the focused-spec rule; please make sure the screenshot child PR lands with this one and eyeball the wrapped second bullet.
• ScopeReason is exported from shell-scope.ts:920 with no importer. Harmless.

Checked and fine

Spine and log paths still receive identifiers (permission-gate.ts:917 passes decision.reasons untouched; describeShellScopeReasons is only called inside the formatters). The only runtime-built reason ("absolute path outside workspace: ...", lines 644 and 647) is covered by the prefix table at 926 to 935. No other test, fixture or demo matched on the old prompt text. .approval-advice and .approval-footer are white-space: pre-wrap, so the one-per-line rendering works. Only as const satisfies is used, no object-literal casts, no suppressions.

---

Generated by Claude Code

[jonathanKingston]

Thanks — all of it addressed, in 7b204d4 (merge) and af961db (fixes).

Blocking: rebase and re-target

Merged origin/main and resolved both conflicts in favour of main, then re-applied the platform fix where #2319 moved the code:

• packages/hooks-dialects/src/command-hook-runner.ts:213 — the runtimeError string.
• packages/hooks-dialects/src/sandbox-failure-detection.ts:2 — the header comment.

You were right that resolving in favour of main alone drops it silently; grep for macOS project sandbox across src/ and packages/ now returns only src/main/tools/shell-tool.ts:290, which is the agent-facing tool description rather than user copy, so I left it out of scope.

Non-blocking

Typecheck guarantee. Fixed rather than documented down. addReason, both accumulators (collectExternalReasons, tokenBasedExternalReasons, dangerousInSandboxReasons), the four shared constants, and the two verdict-note literals are now ScopeReason. The list widens to string[] at exactly one place, with a comment saying why:

const { reasons: scopeReasons, hasHard } = collectExternalReasons(trimmed)
// Widened here and only here: `referencesOutsideWorkspace` can return the one
// runtime-built reason, which by construction is not a `ScopeReason` key.
const reasons: string[] = scopeReasons

--eval. Took the "fix the comment" branch: the two rules describe different facts, and collapsing them onto one sentence would lose the base64/exec case, which fires with no interpreter at all. The comment now states that --eval reports two lines, and I dropped the trailing clause the dynamic-execution sentence shared with the interpreter one, so they read as two facts rather than one said twice:

• Runs a script written inside the command itself, so Copse can't tell what it does
• Builds and runs code as it goes (eval/exec/base64)

Cap test. Distinct operands, and enough of them to overrun the total budget — the single-reason case lands at 267 chars (per-reason cap) and the twelve-reason case at exactly 1200 (total cap), so both branches are now covered.

Docs. Qualified the bullet-per-line claim for the harm prompt, and rewrote the typing paragraph to match what the code now guarantees.

One thing your last item turned up

Eyeballing the wrapped bullet found a real defect, so I fixed it rather than just confirming it. With white-space: pre-wrap on a single element, the second line of a long reason returned to the left margin, flush with the bullets — it read as a third bullet:

• Runs a package straight from the registry (npx/dlx/bunx/uvx/pipx), which can fetch unpinned
code                                       ← looks like a new item
• Installs from a non-default package registry — check you trust it

Each bullet is now its own inline-block with a hanging indent (.approval-advice-item), so a wrapped line sits under its own text. The newlines stay as text nodes between the spans, so .approval-advice's textContent is still exactly the string the main process sent — every existing assertion on it, including the demo spec's toHaveText, is unchanged. Verified by rendering the demo bundle through CDP; there's a new test pinning the span structure so the indent can't be silently dropped.

This is scope I added on my own, so push back if you'd rather it were separate.

Not done

I can't regenerate the two reference PNGs here — they're pixel-rendered on the CI runner, and this container can't run WDIO at all (the network policy blocks its chromedriver download, and the local chromedriver majors don't match its Chromium). Both will need the usual candidate-artifact pass, and the hanging indent changes them a little further than your review anticipated.

ScopeReason I left exported: it's the package's public vocabulary for its own reason strings, and check:dead-code is happy. Say the word if you'd rather it were module-private.

Local: typecheck (node + web), lint, format:check clean; full suite 6558 pass / 163 fail, all 163 the node-pty prebuild gap you also hit, one-to-one with the failing files.

---

Generated by Claude Code

[copse-release-bot]

Reference screenshots ready for review

Review GitHub’s image diffs in screenshot PR #2341.
Merge it (or enable auto-merge) to apply the accepted PNGs to claude/interpreter-eval-ux-dv8apk.

Rendered for 35dc90b17492 by CI run 33859836149; the immutable artifact is reference-screenshot-candidates-33859836149.

If this source branch moves, a later successful render closes the stale review PR and replaces this link.

[jonathanKingston]

⚠️ Screenshot PR #2337 does not contain the two shots this PR actually changes. Merging it as-is leaves the approval references showing the old copy.

The filter held them, from the screenshot-artifacts log on run 33814429514:

scope  tests/e2e/screenshots/approval-grouped-shell-commands.png
       (out of scope: no selected spec renders this shot for this diff; 6655 px vs HEAD)
scope  tests/e2e/screenshots/approval-light-accent.png
       (out of scope: no selected spec renders this shot for this diff; differs)

I checked the committed approval-grouped-shell-commands.png and it still reads "This command needs access the macOS project sandbox blocks (corepack downloads package-manager binaries)." — the exact sentence this PR replaces. The 14 PNGs #2337 does carry are unrelated re-renders that cleared the noise threshold.

Why the scope rule holds them

computeScreenshotGate (scripts/test-oracle.mts:694) treats only src/** and tests/e2e/** as render-affecting, and affectedScreenshots maps shots from sel.selectedE2e alone (:663). Both of these shots are rendered by the demo tier (tests/demo/approval-grouped-shell-commands.demo.ts:55, approval-light-accent.demo.ts:47), which no selection ever reaches — so no diff can own a demo-tier shot through the oracle, even one that edits the demo spec itself. Per scripts/lib/screenshot-scope.mts, ownership is oracle-affected or branchOwned (a non-bot commit on the branch), so demo shots only ever land via the second route or the label.

That looks like a gap worth its own issue rather than something to fix here.

Options

1. Recover them from the screenshots-demo artifact (same run, expires 2026-09-09) and commit the two PNGs. The demo job collects changed shots before the filter runs, so the correct CI-rendered versions should be in there. Committing them also makes them branchOwned, so later runs stop holding them. Lowest noise, and it keeps chore(e2e): review screenshots for #2331 #2337 for the unrelated churn.
2. Add the update-screenshots label and re-run. Documented escape hatch for exactly this ("the case where the oracle could not map a render-affecting file to its shots"), but it disables noise and flap filtering too, so the review PR gets much larger.

I can't do either from my environment — I have no way to download the artifact, and hand-rendering PNGs here would be wrong fidelity and would then win as "contested" against CI's own render.

Everything else on the PR is green and mergeable.

---

Generated by Claude Code

[jonathanKingston]

Your 7b204d4 merge and af961db already covered the rebase, the hook-runner string in packages/hooks-dialects, the ScopeReason typing and the distinct YOLO operands. I pushed two small follow-ups on top: fa1c3d2 gives the heredoc, inline-script and dynamic-execution reasons one shared sentence ("Runs code written or built inside the command itself, so Copse can't tell what it does") so node --eval "x" really does collapse to one line as the comment claims, with a test for it, and fixes one aliasing nit where analyzeShellCommand pushed the runtime-built outside-path string into the ScopeReason[] array (now copied). 35dc90b tightens the Guarded YOLO cap assertion from <= 1300 to exactly MAX_GUARDED_YOLO_HARM_ADVICE_CHARS and qualifies the "one concern, one line" doc bullet for the paragraph-joined harm prompt. Full lint, typecheck, dead-code and 297 targeted tests pass.

---

Generated by Claude Code