chore: name the gh release verbs so reading releases stays unattended - #617
Conversation
A blanket `Bash(gh release*)` ask prompted on `gh release list` and `gh
release view`, which read GitHub and change nothing there. That fails the
first half of the stated standard for the ask list ("the effect escapes the
repo AND git cannot undo it"), and it prompted on the exact command the
release rule requires — checking the currently published version before
tagging.
Split it the way the stash twins already are, by verb: create, edit, delete,
delete-asset and upload still ask; list, view and download run unattended.
quality-check.sh pins both directions by command string so the split cannot
collapse back into a blanket rule or into a hole.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ApGTeBbaAZgigkDdAPqWyp
|
@claude-bot review |
![bess-manager-claude-bot[bot]](https://avatars.githubusercontent.com/in/3024553?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: PR #617 — name the gh release verbs
Scope check: Touches only .claude/settings.json, CLAUDE.md, scripts/quality-check.sh — permission-config/doc only, no Python or TypeScript, so most of the checklist (Python rules, API layer, sensor access, exception handling) doesn't apply here. No scope creep; the three files are exactly what the fix requires.
Root cause match: Confirmed. The problem statement (gh release list prompting under the blanket Bash(gh release*) ask rule) is real and verifiable against the current rule at .claude/settings.json:95 on main — a bare gh release* prefix-glob catches every subcommand, including the read-only ones. gh release has exactly 8 subcommands (create, delete, delete-asset, download, edit, list, upload, view); the PR's split — create/edit/delete/delete-asset/upload stay in ask, list/view/download fall through to auto — is a complete, non-overlapping partition of that set. Mirrors the existing git stash verb-naming pattern in the same file, per the stated standard ("the effect escapes the repo and git cannot undo it").
Would the added test catch a regression? Mostly yes. I ran the MUST_BE_DENIED/MUST_BE_GUARDED/MUST_NOT_BE_GUARDED gate logic from scripts/quality-check.sh directly against the PR's .claude/settings.json (PR head 6a14b3f) — it passes, and the REQUIRED["ask"] presence check also passes. One gap:
scripts/quality-check.sh:325-329 — the new MUST_NOT_BE_GUARDED block:
"gh release list -L 5 -R johanzander/bess-manager-beta",
"gh release list", "gh release view v9.9.0",
"gh release view --json tagName",only asserts list and view stay unattended. download is claimed unattended in the PR body's table ("list, view, download — unattended") and in CLAUDE.md:292 ("list, view and download run unattended"), but there's no gh release download … string in this list to pin it. I confirmed the behavior is correct — gh release download v9.9.0 matches no ask/deny rule in the PR's settings.json, so it is in fact unattended — but that's currently unverified by the gate itself, in a file whose own comments describe "a rule that looks right and matches nothing" as the exact failure mode being guarded against. Not a blocker (no hole is opened; download only writes into the sandboxed repo), but worth a one-line addition alongside the other two to close the gap this PR is otherwise very careful about.
Minimality: Yes — every hunk is either the settings split, the matching test-list split, or the prose explaining it. No unrelated edits.
Fix-scope statement: This is a local, mechanical fix (naming existing verbs in an existing rule/list structure) — doesn't require the "structural fix" scope-assessment statement from rules.md step 9.
Verified locally: fetched the PR branch, ran the gate's Python permission-check logic standalone against the PR's .claude/settings.json — passes. Also independently confirmed gh release's full subcommand list to check the create/edit/delete/delete-asset/upload vs. list/view/download split is exhaustive.
#617 landed the gh release verb split, which touches the same three files. Both conflicts were in lists that each PR appends to: - CLAUDE.md's escapes-to-GitHub row: keep this branch's per-shape push clause and main's `delete-asset` addition. - quality-check.sh MUST_NOT_BE_GUARDED: keep both blocks, the feature-branch pushes and the read-only gh release verbs. Also pins `gh release download v9.9.0`, which #617's review flagged as claimed-unattended in its body and CLAUDE.md but pinned by no command string. Behaviour was already correct; only the verification was missing -- the same "rule that looks right and matches nothing" failure this list exists to catch. Gate after resolution: 95 command shapes checked, 20 require deny, 25 must stay unattended, Errors: 0.
Review of #622 found the previous commit's fix incomplete, and it was right. `pr-review.yml:75` documents COMMENT as a legitimate FINAL verdict ("questions/observations only"), submitted with `gh pr review --comment`, which produces the same state == "COMMENTED" as the bot's inline-notes placeholder. Treating every COMMENTED as non-terminal therefore swallowed a real COMMENT verdict: the loop waited out the full timeout and reported "never submitted a summary" while a summary with findings sat on the PR. That over-generalised "the placeholder is COMMENTED" into "COMMENTED is always the placeholder". Fixed at the source and in the consumer. Source: pr-review.yml step 3 permitted `gh pr review` for inline notes, and that is what submits the extra review. It now requires `gh api .../pulls/N/comments`, so exactly ONE review is submitted per run -- the step 4 summary. No placeholder means no ambiguity. Consumer: the script no longer decides by state alone, and deliberately does NOT parse the placeholder's body -- that text is bot-generated prose with no contract behind it. APPROVED/CHANGES_REQUESTED return immediately; a COMMENTED-only state is held `grace` seconds (180, against observed placeholder-to-summary gaps of 16s on #622 and 50s on #617) to let a summary supersede it, and is returned as the verdict if none does. The grace window is what keeps this correct for reviews already on older PRs and if the bot regresses. Verified against #622's real review history: with both reviews visible the decisive branch returns CHANGES_REQUESTED and grace is never entered; in a window containing only the placeholder the COMMENTED branch finds it while the decisive branch is empty, so grace holds. The timeout message is also now correct rather than merely different: a COMMENTED-only run can no longer reach it, so reaching it means no review of any state was submitted -- a trigger fault, which is what #619 hit twice. SKILL.md's Step 11 said "It will never hand you COMMENTED"; a COMMENTED that now reaches the caller IS the verdict, so it is documented as carrying findings and not earning the ready flag, same as CHANGES_REQUESTED.
2 participants
Problem
.claude/settings.jsonguarded releases with a blanketBash(gh release*)ask, so a plain read prompted:That fails the standard CLAUDE.md states for the ask list — the effect escapes the repo and git cannot undo it.
gh release list/gh release viewfail the first half outright: they read GitHub and change nothing on it. Worse, the prompt fired on the exact command the release rule mandates ("always check the current published version before tagging") and on every beta-vs-stable version comparison.Fix
Name the verbs, exactly as the
git stashtwins already do — a blanket rule there hard-blockedgit stash listfor the same reason.create,edit,delete,delete-asset,uploadlist,view,downloadscripts/quality-check.shpins both directions by command string, matching how the file already treats push/stash: the mutating spellings go inMUST_BE_GUARDED, the read-only ones inMUST_NOT_BE_GUARDED. So the split cannot silently collapse back into a blanket rule, nor into a hole.Verification
The permission gate runs green:
✅ Permission surface intact (76 command shapes checked, 20 require deny, 18 must stay unattended)— up from 68 shapes.No behaviour change outside the permission surface; no Python touched.
🤖 Generated with Claude Code
https://claude.ai/code/session_01ApGTeBbaAZgigkDdAPqWyp