fix: run dispatched agents headless with a container-scoped permission mode - #671
Conversation
…n mode A one-shot `-p` dispatch died at two gates the interactive flow expects a human to answer: Step 3 asked "should I proceed?" in its final message and ended its turn (the container dies with it), and Step 6 backgrounded the quality gate and waited for a re-invocation that never comes. A headless agent has no interactive channel, so the invocation now states the mode explicitly and forbids ending at the gates or while background work is pending. Separately, the clone brings the repo's tracked .claude/settings.json -- host ask/deny policy that means nothing to a headless agent, and a headless ask is a hard denial (`--dangerously-skip-permissions` does not override a project's ask rules; the first live dispatch died on `gh api`). Shadow the clone's file with a read-only bind mount of container/agent-settings.json so the container runs bypassPermissions by design, and gate the host permission-surface pins (quality-check.sh and test_agent_permissions.py) on BESS_HEADLESS_MODE since they validate a policy the container intentionally replaces. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@claude-bot review |
There was a problem hiding this comment.
Review summary
Root cause match: No formal linked issue — this is an infra fast-follow to #670 (containerized agent dispatch, Phase 1), driven by evidence from three failed live dispatches. The PR body cites concrete evidence for each of the three fixes (the live dispatch's actual failure at gh api, the Step-3/Step-6 gate-ending behavior, and the resulting need to skip host-only permission pins). I traced each fix against the code it changes and the reasoning holds:
scripts/lib/agent-dispatch.sh:213-223— bind-mountscontainer/agent-settings.jsonread-only over$clone_dir/.claude/settings.json. Correct: the clone is a full checkout, so.claude/settings.jsonis tracked at that exact path, and project settings win over user settings in Claude Code — which is exactly why the prior belt-and-braces~/.claude/settings.jsoninContainerfile.agentwasn't enough on its own.Containerfile.agent's updated comment (lines 176-186) now correctly documents that this bind mount, not the baked-in file, is what actually makes the container permissive.scripts/run-agent.sh:155-165— theheadless_directiveappended to the-pprompt is a reasonable direct fix for a one-shot turn following an interactive-mode gate; matches the described failure (ended turn at Step 3, backgrounded Step 6).backend/tests/test_agent_permissions.py/scripts/quality-check.sh— both correctly gate onBESS_HEADLESS_MODE=1, which was already threaded throughdispatch_run_argsby #670, so both dispatchers (run-agent.shdev role andrun-po.shpo role, which sharedispatch_run_args) pick it up uniformly. No scope creep there.
Regression coverage: No new automated test was added for the mount-override behavior itself (no bats/shell test asserts dispatch_run_args includes the new -v line, and no test asserts quality-check.sh skips correctly under BESS_HEADLESS_MODE=1). The only test-file change is a skipif on existing tests. Verification here is the live dispatch (#666 issue mentioned in the description) plus bash -n, which is consistent with how this repo verifies shell/container infra elsewhere, but it means a future regression in the mount wiring would not be caught by ./scripts/quality-check.sh or CI — only by another live dispatch.
Minimality — two nits, neither blocking:
-
scripts/quality-check.sh:48-53(the newPYTEST_IGNORE_PERM="--ignore=backend/tests/test_agent_permissions.py"underBESS_HEADLESS_MODE=1) looks redundant with thepytestmark = pytest.mark.skipif(os.environ.get("BESS_HEADLESS_MODE") == "1", ...)added tobackend/tests/test_agent_permissions.pyin the same PR. Skipped tests aren't failures —pyproject.toml's[tool.pytest.ini_options]has no--strict-markersorfilterwarnings=error— so.venv/bin/pytest -m "not slow"should already exit 0 with that file's tests skipped, without the--ignore. I don't see anything at module level in that test file that reads.claude/settings.jsonoutside a test/fixture body (the read only happens inside_rules(), called lazily by therulesfixture, which a fully-skipped module never invokes), so I don't think--ignoreis load-bearing here. Two independent mechanisms for the same skip is more than the fix needs, and risks drifting out of sync if one is edited later without the other. -
scripts/lib/agent-dispatch.sh:216— the new bind-mount is guarded byif [ -f "$DISPATCH_REPO_ROOT/container/agent-settings.json" ]. Since that file is added in this very PR, the guard is currently dead weight, and its effect is a silent fallback: if the file were ever missing (renamed, accidentally deleted), dispatch would silently continue without the override and reproduce the exactgh apihard-denial failure this PR exists to fix, with no error message. Given this codebase's stated preference for explicit failure over silent degradation elsewhere, an unconditional mount (so a missing file surfaces as a normal podman "no such file" error) seems more in keeping with the rest of the fix than a quiet skip.
Rule/checklist compliance: No Optional[x]/hasattr/getattr-with-default, no new classes, no hardcoded entity IDs, no exception-string matching, no credentials or injection vectors introduced. Nothing here touches core BESS/optimizer code, sensor access, or API response shaping, so those checklist items don't apply to this diff.
Verdict: APPROVE. Both findings above are nits (unnecessary-but-harmless redundancy, and a defensive guard that trades a loud failure for a silent one) — worth a follow-up cleanup but not blocking, and everything else checks out against the PR's own stated evidence.
Summary
Three fixes that made the first live sandboxed dispatch (#666) able to run at all, discovered across three failed runs:
.claude/settings.json(host ask/deny policy). A headless ask is a hard denial and--dangerously-skip-permissionsdoes NOT override projectaskrules — the first dispatch died ongh api. A read-only bind mount ofcontainer/agent-settings.json(bypassPermissions) now shadows the clone's file, applied byscripts/lib/agent-dispatch.sh.-pdispatch followed the INTERACTIVE Step-3 gate ("should I proceed?") and ended its turn, killing the container; a later run backgrounded the quality gate and waited for a re-invocation that never comes.scripts/run-agent.shnow appends an explicit headless-mode directive: never end at gates, post to the issue, block in-process onwait-for-reply.sh, run Step 6 in the foreground.scripts/quality-check.shandbackend/tests/test_agent_permissions.pyvalidate the HOST.claude/settings.jsonprofile, which a dispatched container deliberately replaces. Both now skip underBESS_HEADLESS_MODE=1; host enforcement is unchanged.Known pre-existing failure (not from this PR)
backend/tests/test_dashboard_api.py::TestHistoricalDataStatus::test_dismiss_persists_across_requestsfails only in full-suite runs (test-order pollution), passes standalone (1 passed verified here). Reproduced independently by the #666 agent on two full-suite runs. Touches no file in this diff; flagged so a red CI run isn't mistaken for a regression.Test plan
./scripts/quality-check.shon host: mypy/TS/ESLint/frontend-tests/permission-surface all green; only the pre-existing dashboard flake failsbash -non the three edited scriptsmount,.claude/settings.jsonshown as the read-only shadow)🤖 Generated with Claude Code