fix: let implement-issue run end to end without permission prompts

[johanzander]

Summary

• implement-issue stalled mid-run waiting for approvals nobody was awake to give. The profile blocked the one command Step 11 prescribes and permitted several it never runs.
• Every command the skill runs from fetch to a ready PR now resolves to allow; the maintainer's calls still ask.
• Adds a test pinning the command inventory extracted from the skills, so a skill gaining a new command fails CI instead of stalling a run at 2am.

Root cause

Bash(gh api *) sat in ask. Reading inline review comments (pulls/<n>/comments) has no gh pr view equivalent — that endpoint is the only way to get the feed, and it is a plain GET. So the review loop's own prescribed command (SKILL.md:698) was the one thing gated, while gh auth token and gh issue close were permitted outright.

Separately, the four scripts the skills actually drive — request-pr-review.sh, gh-agent.sh, worktree-setup.sh, quality-check.sh — matched no rule in any category. Permission rules match only the top-level command string, so each fell through to the auto-mode classifier and behaved unpredictably run to run.

Fix

Category	Change
allow	the four scripts; gh pr checks/ready/comment, gh run list/view/watch, gh api reads; git * (Step 4's prune uses git -C <path> …, which a narrow git branch * would miss); build/test commands
ask	gh issue close; gh api write-forms in both flag orderings — gh api * -X * alone cannot match gh api -X POST <url>, since the leading * cannot span the empty prefix
deny	git reset*, git push --force*/-f* (SKILL.md:176), gh auth token*
removed	blanket Bash(gh api) / Bash(gh api *) from ask

git push is deliberately not gated: main, beta and tags are protected by rulesets with empty bypass lists, so a client-side prompt could only ever fire on the routine feature-branch case.

gh pr merge, gh issue close and the release verbs stay in ask for the opposite reason — the server cannot distinguish the agent calling gh with the maintainer's credentials from the maintainer clicking the button.

Test plan

backend/tests/test_agent_permissions.py — 46 assertions over three tables (runs-unattended / needs-approval / forbidden), plus the flag-ordering case.

46 passed in 0.03s
2138 passed, 50 skipped, 562 deselected   # -m "not slow"

Evidence the test discriminates

Run against the pre-fix settings it fails 29 of 46 — including every one of the four scripts, the Step 11 gh api read, and gh auth token resolving to allow.

Scope assessment

Config plus one test file; no production code. No CHANGELOG entry — agent tooling has no user-visible effect.

Known limitation

The test models the matcher as deny > ask > allow with fnmatch, so it asserts the rule set expresses the intent, not that the harness enforces it that way. The one behaviour verified against the real matcher by hand: mid-string wildcards do match (git -c core.pager=cat stash --help is caught by Bash(git -* stash --*)), which the gh api write-form rules depend on.

Review round 1

CHANGES_REQUESTED correctly caught Bash(git reset*) in deny as a blocker rather than a follow-up. deny never prompts, and rules.md:36-40 prescribes git reset --soft HEAD~1 as the WIP-commit recovery pattern now that git stash is denied repo-wide — so the rule removed a documented hard-constraint workflow repo-wide, for humans too. Narrowed to Bash(git reset --hard*), the only form that discards committed work.

Also added a MUST_NOT_BE_DENIED table: the FORBIDDEN table only exercised --hard, so the suite could not have caught the regression it introduced. A deny pattern broader than the danger it targets now fails CI.

🤖 Generated with Claude Code

[bess-agent]

@claude-bot review

[bess-manager-claude-bot]

Summary

Scope is clean: config + one test file, no production code, well-evidenced root cause (the gh api ask/allow inversion, backed by a 29/46-failing-before-fix count). The permission-model reasoning for git push staying ungated (server-side rulesets cover the dangerous case) is sound and clearly stated. No linked issue — this reads as self-directed tooling work discovered from a stalled run, which is fine, but nothing to cross-check a "root cause" claim against.

Blocking: Bash(git reset*) in deny blocks a documented, required workflow with no escape hatch

.claude/settings.json (deny, new): "Bash(git reset*)",

docs/agents/rules.md:36-40 (required reading, hard constraints) documents the WIP-commit recovery pattern:

To set work aside on the branch you are already on ... use a WIP commit ... then git reset --soft HEAD~1 to pick it back up.

I ran the PR's own decide() model (from backend/tests/test_agent_permissions.py) against the PR's actual .claude/settings.json:

git reset --soft HEAD~1        -> ('deny', 'git reset*')
git reset HEAD backend/app.py  -> ('deny', 'git reset*')

deny has no override — per rules.md:21-24 itself, deny "blocks every mutating form outright ... with no override." So this PR silently removes the only documented way to un-stage a WIP commit, for every agent and every human-supervised session in this repo, not just implement-issue. That's a harder failure mode than the prompt-stalls this PR sets out to fix — no prompt, no path forward, just a rejected command.

The PR description's own "Follow-up worth considering" section identifies this exact problem and even names the fix:

Narrowing to git reset --hard* would keep the real protection.

Given the fix is a one-line pattern narrowing, and this is a documented hard-constraint workflow the PR itself would break, I'd ask for that narrowing in this PR rather than deferring it — "fix it in the same cycle" is the project's own debugging-protocol convention (rules.md:147) for a bug found while working the same area.

Secondary: the FORBIDDEN test table only asserts git reset --hard origin/main -> deny; it never exercises git reset --soft HEAD~1 or a bare git reset HEAD <file>, so the test suite the PR adds doesn't catch this regression either. Worth a case in RUNS_UNATTENDED (or a new "does not deny" table) once the pattern is narrowed, so it can't drift back.

Test coverage

Otherwise strong — three tables (unattended/needs-approval/forbidden) plus the flag-ordering case are a reasonable pin of the command inventory, and the "evidence the test discriminates" section (29/46 failing pre-fix) is exactly the kind of proof this checklist looks for.

Everything else

• No Optional[x], hasattr/getattr, new classes, or exception-string-matching — N/A, no production code touched.
• Comments in the new test file are WHY-only (incident context, caveats about the fnmatch model) — compliant with the no-what-comments rule.
• Could not run black/ruff/mypy locally in this review environment (no .venv available here) — relying on the PR's reported 46 passed / quality-check output; worth a CI-green confirmation before merge if not already checked.

[bess-agent]

@claude-bot review

[bess-manager-claude-bot]

Summary

No linked issue — this is a self-diagnosed tooling fix, not an issue-driven change, so the "matches linked issue root cause" checklist item doesn't apply. Root cause is verified directly against .claude/settings.json.

Root cause matches the diff. Confirmed by reading the pre-PR file: "ask": ["Bash(gh api)", "Bash(gh api *)", ...] sat above the blanket gh * isn't present — so gh api reads genuinely had no matching allow rule, while gh auth token/gh issue close had no rule at all and fell to the unpredictable auto-classifier. The fix's characterization is accurate.

The test discriminates — verified by hand, not just trusted. I checked out the PR branch in an isolated worktree and ran backend/tests/test_agent_permissions.py against both settings.json versions:

• Post-fix: 48/48 pass.
• Pre-fix (git show main:.claude/settings.json): 29/48 fail, including all three FORBIDDEN cases (git push --force, git push -f, gh auth token all resolved to unmatched instead of deny) and the gh api write-form case. This is real RED→GREEN evidence per docs/agents/testing.md, not just an assertion in the PR body.

The self-correction in commit 2 (Bash(git reset*) → Bash(git reset --hard*)) is exactly right — git reset --soft HEAD~1 is the WIP-recovery pattern rules.md:36-40 prescribes now that stash is denied repo-wide, and the new MUST_NOT_BE_DENIED table (test_agent_permissions.py:150-158) pins that so a future overly-broad deny fails CI instead of silently removing the escape hatch again.

Scope is minimal. Only .claude/settings.json and the new test file — no production code, matches the PR's own scope assessment.

Observation (non-blocking). The new Bash(git *) allow rule (.claude/settings.json:57) is a blanket allow needed because git -C <path> ... puts the worktree flag between the binary and subcommand, which a narrower git branch *-style pattern would miss — the PR explains this tradeoff and deliberately covers git reset --hard/git push --force/-f in deny. It does not add equivalent protection for git clean -f/-fd (deletes untracked files, just as irreversible as reset --hard) or git checkout -- . (discards uncommitted changes) — both called out alongside git reset --hard in the root CLAUDE.md's "Executing actions with care" section as needing confirmation. I grepped .claude/skills/*/SKILL.md and docs/ and found no current use of git clean in any skill, so there's no live path that exercises this today — before this PR the same commands were merely "unmatched" (unpredictable), not safely gated either, so this isn't a regression the PR introduces. Worth a follow-up deny rule for git clean -f*/-fd* given the stated design principle (irreversible-and-uncovered-by-server-rulesets → deny) applies equally here.

No rule violations found against docs/agents/rules.md / .github/claude-bot.md checklist. CI (Docker build, E2E, Fast tests, Frontend checks, Code quality, Merge gate) all pass.