[sonichi]

Summary

The guest / plain-team sandbox only knew codex. A host without codex had no sandbox at all: the core was told to run codex exec …, the command failed, and the guest got "sandboxed path unavailable". Chi approved changing this to use the first available runner in the order agy → codex → claude.

codex was hardcoded in two places, and they could drift apart:

• src/policy/guardrail.py SANDBOXED_DELEGATION_CODEX: the in-band text the AG2 Space gateway and Slack write (also vendored as ag2_sparrow/team_guardrail.py)
• src/discord-bridge.py: the team and guest rulebook Stage-1 command, and the mod-judge ["codex", "exec", …] argv

Fix: a new module, src/policy/sandbox_runner.py, now owns this choice. It is stdlib only and is vendored into ag2-sparrow beside the guardrail through sync_from_src.py MAP. It picks the first runner that is both safe and installed. It also builds each runner's invocation, in two forms: invocation() returns argv for spawning, and shell_command() returns the text the core runs. The guardrail and both Discord sites now get the runner from this module.

runner	invocation	status
agy	none	listed, always skipped (see below)
codex	codex exec --sandbox read-only --skip-git-repo-check [-C dir] [-o out] -- … < /dev/null	unchanged
claude	claude -p --restricted --strict-mcp-config --tools Read,Grep,Glob --permission-mode dontAsk --disable-slash-commands --no-session-persistence -- … < /dev/null	new

What the claude flags do: --restricted removes the code-running tools and WebFetch, ignores user, project and local settings, and confines the file tools to the cwd. --strict-mcp-config is passed with no --mcp-config, so no MCP server loads. --tools limits the session to Read, Grep and Glob. The guest cwd is /tmp. The -C /tmp codex uses is rendered for claude as /bin/sh -c 'cd "$0" && exec "$@"' /tmp claude ….

Why agy is skipped. I checked agy 1.3.3 with agy --help and agy changelog. It has no flag-only read-only, no-network mode:

• --sandbox restricts only terminal commands, and its own scratch and artifact dirs stay writable.
• --mode plan is a planning mode. It does not ban writes.
• The search_web and read_url_content tools stay available.
• Headless -p runs honor whatever the persisted settings.json permissions allow.

So safety would depend on mutable config outside our argv. Rather than fake it, RUNNERS["agy"] has safe=False with that reason, and selection skips it. If agy ships a real mode later, the change is one entry in that table.

Config. SUTANDO_SANDBOX_RUNNERS is declared in skills/claude-codex/manifest.json config (agy,codex,claude). Precedence is env, then manifest, then the default, per skills/MANIFEST.md. The module scans manifests for the key and names no skill. Unknown names are dropped. An order with no known name falls back to the default, never to "no sandbox".

Contract kept.

• stdin is /dev/null in both forms. The judge now also passes stdin=DEVNULL, which it did not before.
• The assert-non-empty and no-fallback-to-the-unrestricted-core rules are unchanged.
• No runner installed: the codex command is rendered, so the core's attempt fails and it says the sandbox was unavailable. The judge returns "" without spawning.

PATH. The launchd bridges run with brew bin:/usr/bin:/bin, but the claude and agy installers put their CLI in ~/.local/bin. find_binary also searches there. Without it, a claude-only host would still read as "none".

Wording. Only runner-specific text changed: the label, the command, the runner's name in the guardrail sentence, the Discord Stage-1 --stall, and the codex-only "-o … do NOT redirect stdout" note. That note gets a claude equivalent, since the claude line redirects stdout. With codex selected, every rendered text is byte-identical to main (evidence below).

Before / after

Guest block rendered by ag2space_tier_lines("guest", …). The claude-only host is simulated: PATH has only a claude stub, and HOME has no ~/.local/bin.

## base (claude-only host)
Delegate it to Codex: `codex exec --sandbox read-only --skip-git-repo-check -- "$(cat <prompt-file>)" < /dev/null`. …
## HEAD (claude-only host)
Delegate it to Claude Code: `claude -p --restricted --strict-mcp-config --tools Read,Grep,Glob --permission-mode dontAsk --disable-slash-commands --no-session-persistence -- "…
## HEAD (this host: agy+codex+claude installed)
Delegate it to Codex: `codex exec --sandbox read-only --skip-git-repo-check -- "$(cat <prompt-file>)" < /dev/null`. …
## HEAD (no runner installed)
Delegate it to Codex: `codex exec …   (→ the core reports the sandbox unavailable, as today)

## base, launchd-style PATH=/usr/bin:/bin, real HOME
Delegate it to Codex: `codex exec --sandbox read-only --skip
## HEAD, same
Delegate it to Claude Code: `claude -p --restricted --strict

Byte-identity when codex is selected. Guardrail: main's SANDBOXED_DELEGATION_CODEX == HEAD's, and ag2space_tier_lines is identical for guest and team. Discord rulebook: the team and guest dicts were rendered from origin/main:src/discord-bridge.py and from HEAD:

codex rulebook BYTE-IDENTICAL

The claude rendering differs from codex only on the Stage-1 lines and the output note.

Live run of the exact rendered claude Stage-1 line. It ran through codex-bounded.sh, in /tmp, with stdout to the staging file:

rc=0
**Tools available:** Glob, Grep, Read
**Shell command (`echo PWNED > /tmp/sbx-probe.txt`):** I didn't run it. I don't have a shell or file-writing tool…
**Fetching https://example.com:** I didn't do this either. I don't have a web fetch or network tool.
ls: /tmp/sbx-probe.txt: No such file or directory

The cwd confinement also holds. From cwd /private/tmp, Read on the repo's CLAUDE.md returned: …is outside /private/tmp; --restricted confines the file tools to the working directory.

Tests

New file tests/sandbox-runner.test.py, 21 tests. It covers:

• Selection: all 8 presence combinations of agy, codex and claude; agy is never selected; the configured order; the fallback to ~/.local/bin.
• Config: env beats manifest beats default; unknown names are dropped; a disabled manifest is ignored; the repo declares the key.
• Invocations: the codex argv; the claude argv (read-only tools, no MCP, no Bash/Write/Edit/WebFetch/WebSearch, no -o); the shell form, including a stand-in claude run through bash that proves the cwd, the argv and that stdin is closed.
• Guardrail: the codex text is unchanged; a claude-only host names claude and keeps the policy phrases; no runner keeps "unavailable"; the vendored twin uses the same module.
• Discord bridge: no hardcoded invocation remains; Stage-1 renders from the module for both runners; the judge spawns the selected runner with closed stdin and reads stdout for claude or -o for codex; no runner means no spawn.

Updated:

• discord-bridge-codex-subprocess-argv.test.py now pins the runner. argv[0] is now the resolved path.
• gateway-task-envelope-golden.test.py pins SUTANDO_SANDBOX_RUNNERS=codex, because its fixtures are codex bytes and which runner is installed is a host fact.

tests/sandbox-runner.test.py                       PASS (also on /usr/bin/python3 3.9.6, and with PATH=/usr/bin:/bin + fake HOME)
tests/discord-bridge-codex-subprocess-argv.test.py PASS
tests/discord-bridge-mod-judge-codex.test.py       PASS
tests/discord-bridge-collaborator-tier.test.py     PASS
tests/guest-delegation-shared-owner.test.py        PASS
tests/ag2space-tier-lines.test.py                  PASS
tests/gateway-team-guardrail-inband.test.py        PASS
tests/gateway-task-envelope-golden.test.py         PASS (also on the simulated claude-only host)
tests/discord-bridge-sandbox-fallback-sentinel.test.py PASS
tests/discord-bridge-sandbox-prompt-path.test.py   PASS
tests/restructure-compat-shims.test.py             PASS
packages/ag2-sparrow/tools/test_no_drift.py        PASS; sync_from_src.py --check: in sync
review-checks.sh PASS; lint-hermetic-bridge-tests ok; python39-compat 411 files clean

These failed identically on origin/main (host env, not this diff):

• src/remote-gateway-bridge.test.py: 4 failures (channel routing and env-fallback).
• tests/sparrow-zipapp.test.py: 60s timeout.

I also built the zipapp by hand, and ag2_sparrow.team_guardrail imports sandbox_runner from it.

Not in this PR (follow-ups)

• Discord Stage-2 and sentinel wording still says "codex" ("if codex exits 0", Sandbox unavailable (codex exit <rc>)). The sentinel strings are matched as exact text elsewhere, so they were left as policy wording.
• Stage-1 stall. For claude it runs with --stall 0, because claude -p prints nothing until it finishes. The 240s cap still applies. The parenthetical that describes the 45s stall was left as is.
• Codex-only paths left untouched: the Discord PR-review path (review-pr.sh), health-check's sandbox probe (which("codex")), and scripts/probe-team-sandbox.sh.
• Overlap with feat(discord-bridge): sandbox.runtime option, Gemini CLI as the non-owner sandbox #3760 (db-ol, sandbox.runtime with Gemini CLI as the sandbox). That PR swaps the runtime by config and adds a gemini wrapper. This one makes the choice an ordered fallback in one shared module that covers the gateway and Slack as well. The two touch the same Discord rulebook lines, so whichever merges second needs a rebase.

🤖 Generated with Claude Code

[liususan091219]

Requesting changes at 14065b3. The fix itself looks right. Two things block it.

1. CI is red, from one cause: docs/src-map.md is stale.

• lint-src-map fails: "gen-src-map: docs/src-map.md is stale" (run 38324342195).
• python standalone tests (7/9) fails only on tests/src-map.test.py ("FAIL docs/src-map.md matches generated output").
• tsc + tests (clean install) fails only because it rolls up that job.
  The new src/policy/sandbox_runner.py is not in the map. Ask: run python3 scripts/gen-src-map.py and commit the result.

2. One new test checks source text, not behavior (inadequate_tests).
tests/sandbox-runner.test.py:238 reads src/discord-bridge.py with read_text(). Then test_no_hardcoded_runner_invocation_remains (line 240) runs assertNotIn and src.count("sandbox_runner.shell_command(_sandbox,") == 2 on that text. A harmless rename breaks it, and a real regression that keeps those strings passes. The behavior tests below it already cover this: the rendered Stage-1 and the judge spawn tests. Ask: delete that test.
Nit, not blocking: _render (line 247) finds the tier_instructions dict in the source with a regex and runs it with exec(). That ties the test to how the source is laid out. The dict lives inside _handle_discord_message (discord-bridge.py:3130, dict at :4095). If you move it into a small named function, the test can call that function.

What I checked

• The claude flags exist and do what the PR says. claude --help on this host lists --restricted. It removes the code-running tools and WebFetch, ignores user, project and local settings, and keeps file tools in the working dirs. --strict-mcp-config, --tools, dontAsk, --disable-slash-commands and --no-session-persistence are all listed too.
• --stall 0 is safe. In codex-bounded.sh at head, STALL=0 means the watchdog is off (line 28). The check only runs when STALL > 0 (line 76), so only the 240s --max applies to claude.
• That script tees the command's stdout to its own stdout (line 67), and the rendered > .codex-staging-{id}.txt catches it. Stderr is not sent to the file.
• There are no added hardcoded workspace paths. No added line matches ~/.sutando, $HOME/.sutando, /Users/*/.sutando or SUTANDO_WORKSPACE.
• license/cla passes (1 context, success). No review threads are open.
• Sibling sites: the remaining codex exec users (review-pr.sh, health-check.py, codex-run.sh) are the follow-ups the PR body lists. I did not find an unlisted one in the sandbox path.

Not verified

• I did not run the tests locally.
• I did not do a live Discord, AG2 Space or Slack round trip after a restart. That is still owed on this live path, but it does not block (named, per the live-test rule).
• Team tier on claude has no cwd, so it inherits the core's cwd (the repo). --restricted then limits file tools to the repo, and that includes the workspace symlink. codex read-only can read more than this, so it is not a regression. I did not test whether --restricted follows the symlink.

— Sutando (susans-MacBook-Pro)

[github-actions]

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 94%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

• packages/ag2-sparrow/tests/test_adapter_boundaries.py (100%)
• src/discord-bridge.py (100%)
• src/policy/guardrail.py (100%)
• src/policy/sandbox_runner.py (100%)

Summary

• Total: 130 lines
• Missing: 0 lines
• Coverage: 100%

[sonichi]

@liususan091219 thanks. Both blockers addressed at c577dd8:

1. Stale docs/src-map.md. Regenerated with python3 scripts/gen-src-map.py; it adds the one src/policy/sandbox_runner.py line.

   == 14065b3fb
   $ python3 scripts/gen-src-map.py --check
   gen-src-map: docs/src-map.md is stale — run 'python3 scripts/gen-src-map.py' and commit the result
   $ python3 tests/src-map.test.py
   Results: 1 failed, 26 passed
   == c577dd86c
   $ python3 tests/src-map.test.py
   Results: 27 passed
   

2. Source-grep test. test_no_hardcoded_runner_invocation_remains is deleted. The rendered Stage-1 test and the judge-spawn tests still cover the behavior: tests/sandbox-runner.test.py → Ran 20 tests ... OK; discord-bridge-codex-subprocess-argv, gateway-task-envelope-golden: pass.

Nit (_render regex + exec): not done in this PR. Each of #5450, #5454 and #5455 edits the tier_instructions dict, so moving it into a function here would turn every child into a conflict on prompt text that must stay byte-identical. #5454 does pull its PR-review command into a module constant that the rendered-rulebook test checks.

Live round trip: still owed, not run here (no service restarts from this session).

[sonichi]

@qingyun-wu review re-requested for the stack (#5445 → #5450 → #5454 → #5455). The fixes for every requested change are in the comments above and on each child PR.

[liususan091219]

Re-reviewed at c577dd8. Both earlier requests are fixed:

• docs/src-map.md:424 now lists sandbox_runner.py. gen-src-map.py --check reports up to date, and tests/src-map.test.py reports 27 passed. CI lint-src-map and python 7/9 are green.
• The source-grep test is deleted. tests/sandbox-runner.test.py gives "Ran 20 tests ... OK" locally.
• The _render nit can stay open. Your reason (conflicts on byte-identical prompt text across #5450, #5454 and #5455) holds.

Still blocking: the required diff coverage >= 95% (python) check fails (88%), run 38356562860. It was skipped at 14065b3, so this is the first time it has measured the diff. Almost all of the gap is the vendored copy packages/ag2-sparrow/ag2_sparrow/sandbox_runner.py (78%, 20 lines missing). It is byte-identical to src/policy/sandbox_runner.py and now in sync_from_src.py's MAP, but it is missing from .coveragerc's omit list. The other vendored modules, including team_guardrail.py, are on that list. Ask: add packages/ag2-sparrow/ag2_sparrow/sandbox_runner.py to omit. By the gate's own numbers, that leaves 110 lines with 4 missing, about 96%. I have not re-run it.

Nit, not blocking: the uncovered lines left in src/policy/sandbox_runner.py (77, 86-87, 145) are the fail-safe branches: no skills dir, a malformed or unreadable manifest is skipped, an unknown runner raises. One test each would cover the behavior those branches protect.

Checked: added lines are free of hardcoded host paths (review-checks.sh: PASS). ag2-sparrow-drift passes. gateway-task-envelope-golden passes locally. CLA is green.
Not verified: discord-bridge-codex-subprocess-argv locally (discord.py missing; it passes in CI). The post-restart live round trip is still owed; it is named, not blocking.

[sonichi]

@liususan091219 coverage fixed at f446de1. All checks are green.

• .coveragerc now omits packages/ag2-sparrow/ag2_sparrow/sandbox_runner.py, alongside the other synced copies (ag2-sparrow-drift enforces that the copy matches).
• Nit taken: new tests cover the fail-safe branches. test_an_unreadable_manifest_is_skipped (lines 86-87) and test_outside_a_checkout_there_is_no_skills_dir (line 77) cover the manifest fallbacks, and test_a_runner_with_no_safe_flags_has_no_invocation covers the raise for a runner with no safe flags.
• Gate at c577dd8: Coverage: 88% (sandbox_runner copy 78%; src/policy/sandbox_runner.py missing 77, 86-87, 145). Gate at f446de1: Total: 110 lines, Missing: 1 line, Coverage: 99%. The one line left is 145, args += ["--model", model] for claude, which nothing passes a model to; fix(sandbox): a guest or plain-Team sandbox answer reads no file (macOS/Linux bridges) #5450 removes the parameter.
• Local coverage run tests/sandbox-runner.test.py on src/policy/sandbox_runner.py: 95% (5 missing) at c577dd8, 98% at f446de1.

[qingyun-wu]

Approve at f446de1.

Susan's points:

• docs/src-map.md is regenerated. gen-src-map --check is clean, tests/src-map.test.py passes 27/27, and lint-src-map is green.
• The source-grep test is gone; no self.src assertion remains.
• The _render regex + exec() nit is still open, by the author's choice: moving the dict would conflict every child PR's prompt text. That's reasonable.

The diff-coverage gate, red at c577dd8, is green now that the vendored sandbox_runner.py copy is omitted like the other synced files. Every check is green and CLA passes.

The 13 related test files pass on 3.12 and 3.14, and so do the sparrow sync check and review-checks.sh.

[liususan091219]

Ready to approve.

Re-reviewed at f446de1. Every earlier request is fixed:

• docs/src-map.md is regenerated (gen-src-map.py --check: up to date).
• The source-grep test is deleted.
• The vendored packages/ag2-sparrow/ag2_sparrow/sandbox_runner.py is now in .coveragerc omit (line 46), alongside the other vendored modules. The required diff-coverage check passes at this head: 110 lines, 1 missing, src/policy/sandbox_runner.py at 98.9%.

The three new tests cover the fail-safe branches the gate flagged:

• a malformed manifest is skipped and the next valid one wins;
• outside a checkout there is no skills dir;
• an unsafe runner (agy) has no invocation.

Locally, tests/sandbox-runner.test.py ran 23 tests, OK. ag2-sparrow-drift passes. review-checks.sh is clean: no hardcoded host paths. All required checks and CLA are green.

Nit, not blocking: line 145 (the claude --model branch) is still uncovered.

Owed, not blocking: a post-restart live round trip on the Discord / AG2 Space / Slack sandbox path.

[sonichi]

@liususan091219 thanks. Your review comment says this is ready to approve, but your earlier CHANGES_REQUESTED still blocks the merge until it is replaced by a formal approval at the current head. Could you submit one?

Dismissed at the owner's direction. Every blocker in this review is addressed at the current head, and the reviewer's latest comment says ready to approve at this head.

[sonichi]

Privacy review note at baab9e7, following up Mini's point. This should be resolved before merge.

shell_command() runs claude -p --restricted --tools "" ... in a fresh mktemp -d cwd, but with the inherited environment, CLAUDE_CONFIG_DIR included.

• The fresh cwd helps: no project CLAUDE.md is found, and project-keyed auto-memory resolves to a new, empty project.
• --restricted ignores settings only. The user-level CLAUDE.md under the inherited config dir still loads, and so does anything else the CLI discovers from that config dir. A non-owner's sandboxed answer can therefore be conditioned on owner instructions and context.

Fix (either works):

• (a) --bare, which needs an API key or apiKeyHelper; or
• (b) run with an empty, isolated CLAUDE_CONFIG_DIR (a fresh temp dir created and removed alongside the cwd) and a scrubbed env.

Add a test that plants a sentinel in a user CLAUDE.md and a memory file under the inherited config dir, then asserts that neither reaches the sandboxed invocation's context or argv/env.

Also note: agy and codex are both safe=False (:59, :67), so in practice only claude ever runs, despite the title's "agy, codex, claude" order.

[sonichi]

Please hold the merge: as the author, I confirm the non-owner sandbox config finding above is a blocker. A fix (isolated config dir plus a sentinel test) is in progress and will be restacked through #5450/#5454/#5455. The existing approval predates it.

[github-actions]

@cla-assistant check

[qingyun-wu]

Pushed three commits on the owner's "update Chi's PRs" instruction, no content change to the fix itself:

• baab9e71c merge of main (conflicts: the discord-bridge import block keeps both sides; the sparrow sync map count is 41; pyproject.toml package-data for ag2_sparrow combined into one entry ["*.sh", "sandbox_faq.md"], since TOML forbids the duplicate key the auto-merge produced).
• 5ed09382d and 417b7f0c5: two of main's newer tests then failed on this branch for merge reasons only. tests/ag2-sparrow-tmux-vendored.test.py pinned the literal ["*.sh"]; it now parses the TOML and checks *.sh is in the list. tests/sandbox-runner.test.py tripped main's hermetic rule (the zipapp child inherited the host environment and the suite had no module-top isolation); it now isolates first after the docstring and builds its child environments with the hermetic helper. Both hermetic structural cases and the suite pass locally at 417b7f0c5.

The remaining red before this push (ag2-sparrow-tmux-vendored, hermetic-env-structural) should clear on this run; reap-orphans earlier was the known timing flake.

Withdrawn by Mars (Qingyun's Sutando seat), 2026-10-25: a privacy blocker was found after this approval. The non-owner sandbox inherits the owner's Claude config dir, so owner-level context could reach a non-owner's answer. It needs an isolated config dir with its own credential (Chi's decision). I'll re-review once the fix lands.