Sandboxed sessions: the footer chip, grants and the tool policy

[ghackett]

Stacked on #566 (sandbox/1-the-box); PR 2 of the sandboxed-sessions chain (~/specs/collins/sandboxed-sessions.md, "PR 2, the chip and the policy"): the footer chip with the per-workspace grants and Restart to apply, the sandboxed panel shell, and the policy for what a sandboxed session may ask Collins to do through run_in_terminal, read_terminal and start_session.

The chip

A Sandboxed chip (a shield, a filled-path icon in data/icons/…/actions/) leads the footer of a sandboxed tab and is absent everywhere else. It is a Gtk.MenuButton with a TOP popover built like the model chip (collins/sandboxchip.py, which knows no tab or app: it takes callables), filled on every show from the plan the session was launched with (sandboxplan.load_plan, never re-derived): the workspace, whether the GitHub CLI login and the SSH agent went in, whether ~/.claude/settings.json is protected. Under that, the workspace's allowed directories from state.json, each with a remove button and an "after restart" tag when the running box lacks it, Allow a directory… through Gtk.FileDialog.select_folder (the guard's verdict — a secret, an ancestor of one, $HOME, /, Collins' own state, the sandbox home, the workspace itself, not a directory — goes out as a toast, as does a success, since the modal chooser closes the popover), Restart to apply when the plan the state would build now differs from the launched one on the policy inputs (grants, the two shares, settings protection — SandboxHost.plan_stale), and Sandboxed shell.

Restart to apply is offered only where it would restart something: never on a fork (the tab holds its origin's id), never before the transcript resolver has bound an id (a rebuild would replace the conversation with a fresh session), and never on a tab running a plan adopted from another session (a start_session sibling: its box was built for its parent's workspace, and a rebuild takes this tab's own cwd, which would narrow it). Where the row can't be offered and the plan is stale the chip says so, so the "after restart" tags above it aren't left unexplained. It is TerminalTab.restart_sandboxed: the CLI's exit keystroke, a poll that re-nudges as the window's graceful close does and answers the worktree keep/remove dialog, then — once the shell has the terminal back — the launch command typed again with a fresh plan, the old one released: the same shell, tab and row. The command building is now _launch_command, shared with _finish_spawn; a resolved session id wins over a --continue override so a resolved tab resumes itself. Never for a fork tab (it holds its origin's id; a resume would fork it again).

The sandboxed shell

PanelTerminal(number, plan_lookup=…) is the same page class with sandboxed True: page_kind stays shell, so history, busy checks, close confirmations and layout persistence hold (page_state adds "sandboxed": true, which panellayout keeps and the dock's restore hands back to strip.new_shell(sandboxed=True)). It spawns python3 sandboxrun.py <plan> -- $SHELL — the launcher the session's own typed line starts with, so the shell runs in exactly the session's box — with a queued cd to the agent's directory when it lies inside the workspace, is titled Sandboxed shell N on the dock-wide numbering and wears the shield. Busy detection compares the pty's foreground against the shell inside the box (proctree.inner_shell_pid, the first descendant below the launcher and the two bwraps): bash inside takes the foreground for itself and the kernel reports its group in host pids — measured under a real box on this machine (idle at the prompt, busy during sleep, idle after Ctrl+C). A shell keeps the box it spawned in (sandbox_plan, the plan file it launched through): --die-with-parent ties that box to the shell's own pty rather than to the session, so a Restart to apply leaves it running in the pre-restart box — which may still hold a directory the user revoked in the very change they restarted for. The tab says so in its scrollback and mcptools.tool_shells stops handing it to the agent, so it stays the user's and leaves the box's reach. Ctrl+J never binds to one; the strip's tab menu offers New sandboxed shell while there is a plan; with no plan (a layout restored before the launch settled, or into an unsandboxed tab) nothing is spawned and the next show retries.

The policy

mcptools.run_tool_call now hands every handler a third argument, sandboxed — Collins' own reading of the calling tab, never the caller's word — and the blanket SANDBOX_HOST_TOOLS refusal from PR 1 goes. read_terminal and run_in_terminal filter through mcptools.tool_shells: a sandboxed session sees only sandboxed shells (named Sandboxed shell N in the reply) and run_in_terminal opens one when none is idle — the user's own Ctrl+J shell is never typed into or read from inside a box. start_session decides the sibling's sandbox through mcptools.sibling_sandboxed (a sandboxed parent's sibling is always sandboxed; the unit test says an unsandboxed sibling from a sandboxed parent is never possible, in those words) and, for a sandboxed parent, derives the sibling's plan from the parent's launched one (sandboxplan.derive_plan: only --chdir moves, so the sibling sees exactly what the parent sees, with the grants, shares and settings protection as launched); a cwd outside the plan's workspace or grants is refused with the reason (sandboxplan.plan_reaches). The derived plan travels in SessionOptions.sandbox_plan, the sibling's tab adopts and releases it, and a spawn that never made a tab releases it. bypassPermissions is granted to a sibling, explicit or inherited, only when it is sandboxed — which under this rule is exactly when the parent is.

sandboxplan.SandboxHost(app_id, state) replaces terminal.SANDBOX_PLANNER as the tabs' host object (prepare_launch, grants / allow / revoke, plan_stale, derive).

Tests

Unit: load_plan / plan_reaches / derive_plan and the host's allow, revoke, stale and derive (test_sandboxplan); the handler flag, tool_shells, sibling_sandboxed, the reply's shell kind (test_mcptools); inner_shell_pid over a real process tree (test_proctree); the layout flag (test_panellayout); the shell argv (test_providers). E2E: scripts/check_sandbox_policy.py — a real App and a session launched sandboxed through a fake COLLINS_BWRAP that records the --args payload and execs the command: the chip, both terminal tools from the box, Ctrl+J, the layout round trip, a grant refused / recorded / stale / restarted-into / revoked, a sibling derived inside and refused outside (78 checks, 17 s). Staged under ~/.cache/collins-e2e with HOME moved into the scratch tree.

python3 -m pytest tests/ -q: 3697 passed. ruff check collins/ tests/: clean. The e2e check passes under the headless display.

Deviations from the spec

• The chip's directory grant lands in state.json and applies at the next launch; Restart to apply is how it reaches a running session (the spec's v1; live grants through FUSE are phase 2). The chip tags a grant the running box lacks with "after restart" rather than hiding it.
• A sibling from a sandboxed parent whose workspace is a linked worktree is refused: the sibling's cwd collapses to the repo root (the resolver's rule from PR 1), which the worktree's workspace mount doesn't reach. The spec's rule ("inside the parent's workspace or a grant") is applied as written.
• A sibling running its parent's derived plan is offered no restart at all (see above): the derived plan is a launch record of another session's box, and a rebuild here would be for this tab's own, narrower workspace. The parent's restart is the way to apply a change to that box.
• The panel-menu item is the strip's tab context menu (New sandboxed shell), which is where the dock's page menu lives; the chip also carries Sandboxed shell.
• The e2e check is in this PR rather than PR 3: the chip, the shell and the policy have no other proof with real widgets. A real-box launch check stays PR 3's.
• The plan file's directory falls back from the runtime dir to $XDG_STATE_HOME/collins/sandbox/<app id> when there is no XDG_RUNTIME_DIR (a Collins started outside a desktop login session, and CI): mcptools.runtime_dir falls back to the temp directory, which every box shares read-write, and a plan the box can reach is exactly what the protect-check refuses — it refused every launch there, which is what the e2e check caught on CI alone.
• Translations: new strings fall back to English until the release-cut refresh, as the repo's rules allow.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GBtUXcSBWv7SCfkzDN6bBd

[ghackett]

@claude review

[claude]

Claude finished @ghackett's task in 6m 23s —— View job

---

Review of #567

• Read repo CLAUDE.md and relevant skills
• Diff against origin/sandbox/1-the-box
• Review collins/sandboxchip.py (new)
• Review collins/sandboxplan.py changes (load_plan, plan_reaches, derive_plan, SandboxHost)
• Review collins/mcptools.py policy changes (tool_shells, sibling_sandboxed, run_tool_call)
• Review collins/terminal.py restart/shell changes
• Review collins/proctree.py, panelstrip.py, panellayout.py, paneldock.py, window.py
• Review tests
• Check GPL modification-notice dates
• Post final review feedback

Summary

This is a well-scoped, carefully-reasoned implementation of PR 2 of the sandboxed-sessions chain. The policy design in mcptools.py (sandboxed flag threaded through every handler rather than a blanket tool refusal, tool_shells filtering by both sandbox membership and the specific box a shell was spawned in) and sandboxplan.py's read-back/derive machinery (load_plan re-validating shape rather than trusting a file Collins itself wrote, plan_reaches/derive_plan used identically for the chip's staleness check and for start_session siblings) are both solid. The restart flow in terminal.py (_launch_command/can_restart_sandboxed/restart_sandboxed) correctly excludes forks and adopted-plan siblings, and _mark_stale_sandboxed_shells handling the --die-with-parent box-outlives-plan case is a nice touch that the tests back up. GPL modification-notice dates on touched pre-fork files (app.py, terminal.py, providers.py, window.py, mcptools.py [fork-new], sandboxplan.py [fork-new]) all check out correctly against the commit that last touched each file. Docs (docs/guide/features.md, docs/releases.md) are updated consistently with the described behavior.

Two smaller findings:

1. tests/test_providers.py — three blank lines before the new test (likely fails ruff E303)

    assert claude.continue_command(opts) == "/usr/bin/claude --continue"


                                                          <- 3 blank lines here
def test_sandboxed_shell_argv_runs_the_shell_through_the_launcher():

CLAUDE.md pins ruff rules E F W I UP B, and E303 ("too many blank lines") caps this at 2. The PR description says ruff check collins/ tests/ is clean — I wasn't able to run ruff myself in this sandboxed environment (Bash execution is restricted here) to double check, but by inspection this looks like it would flag. Trivial to fix by dropping one blank line.
Fix this →

2. collins/sandboxchip.py:237 — GLib.idle_add where the project convention is GLib.timeout_add for unbidden dock surgery

def _open_shell(self, *_args) -> None:
    self.get_popover().popdown()
    def open_shell() -> bool:
        self._on_open_shell()
        return GLib.SOURCE_REMOVE
    GLib.idle_add(open_shell, priority=GLib.PRIORITY_DEFAULT)

_on_open_shell is TerminalTab.open_sandboxed_shell, which can call PanelDock.open_page — real dock surgery (potentially _split_leaf). CLAUDE.md's rule ("Unbidden dock surgery waits a beat… schedule such opens on a timeout_add") and the existing precedent for this exact class of "open a panel page after some other event settles" (TerminalTab._on_hub_pr_attached, _autodock_attachments, both GLib.timeout_add(_PR_PAGE_SETTLE_MS, …)) consistently use timeout_add, not idle_add. This is the only idle_add call in the codebase that drives an open_page/open_sandboxed_shell-style dock open rather than a grab_page_focus/refresh. Given the documented history of this exact pattern (an idle-scheduled dock open) segfaulting GTK's Wayland backend, and that this one is deliberately timed to run after "the popover's own focus restore" (implying it may itself be racing another idle-priority callback), I'd suggest switching this to GLib.timeout_add(<small delay>, open_shell) to match the established idiom — plausible, not confirmed to reproduce, since the trigger here is a direct click rather than a nested idle cascade, but the deviation from the documented convention seems worth a second look before it ships.
Fix this →

I could not run python3 -m pytest tests/ -q or ruff check myself in this sandboxed review environment (Bash tool invocations were denied), so I relied on direct code inspection rather than confirming the PR's stated "3697 passed" / "ruff clean" results.
· branch: sandbox/2-chip-and-policy

[ghackett]

🤖 Both addressed in 1cc35c4.

The chip's shell open now waits the dock's own settle (GLib.timeout_add(250, …)) instead of riding an idle, matching _on_hub_pr_attached / _autodock_attachments and the "unbidden dock surgery waits a beat" rule. Good catch — the delay still lands after the popover's focus restore, which is what the idle was there for in the first place.

The blank line is dropped. One correction on the premise, for the record: ruff as this repo pins it (0.16.4, rules E F W I UP B) does not flag E303 — ruff's whitespace rules (E1/E2/E3) are preview-only, so ruff check collins/ tests/ was and is clean. Changed anyway, since two blank lines is what the rest of the file uses.

[ghackett]

🤖 Two updates since the reply above, now that the stack has been rebased onto the updated 566:

• 1cc35c4 is no longer on the branch. Both fixes it names now live in 0497754: the chip's shell open on GLib.timeout_add(_DOCK_SETTLE_MS, …) in collins/sandboxchip.py, and the dropped blank line in tests/test_providers.py.
• The branch gained d20de31. scripts/check_sandbox_policy.py now clears its scratch tree under ~/.cache/collins-e2e however it exits: an atexit hook, plus the watchdog clearing the tree before its os._exit (which skips exit handlers). The review of 568 raised this against the sibling check. The same commit stops the sandboxed-sessions skill calling the footer chip "the next PR" — this is the PR that adds it.

CI is green on d20de31, e2e included.