feat(claude-permissions): allow gh pr review in the fleet floor - #228
Conversation
Observed-usage evidence per the component's change discipline: recurring real-session need to submit PR reviews from agent sessions (review verb only; merge remains prompted and `gh pr merge --auto` remains denied). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9zbRxtRuTx1WsQavFc2Gi
|
Claude finished @kyle-sexton's task in 1m 38s —— View job Code Review
File reviewed: Change: SummaryClean, minimal change. The addition is correct, well-placed, and fits the established FindingsAlphabetical placement — correct. The sequence at lines 18–20:
Change discipline — satisfied. The component README requires "observed-usage evidence (recurring prompt patterns from real sessions)" for Allow posture consistency — the existing allow list already permits
No PowerShell counterpart — consistent with existing pattern. Write/mutating gh operations ( 🟡 Nit — wildcard covers The entry If the intent were to restrict to Overall: LGTM with the nit above acknowledged. The change is correct, properly evidenced, and in scope. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e91d02a3e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…iews The bare wildcard also covered --approve and --request-changes, which can satisfy or block required-review gates unattended. Only --comment spellings are floor-allowed; approval and request-changes forms stay prompted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9zbRxtRuTx1WsQavFc2Gi
|
Claude finished @kyle-sexton's task in 1m 48s —— View job Code Review
File reviewed: Change (commit "Bash(gh pr review * --comment *)",
"Bash(gh pr review * --comment)",
"Bash(gh pr review --comment *)",SummaryThe narrowing correctly addresses both the Codex P1 finding and the nit raised in the previous Claude review. The three patterns collectively cover the realistic FindingsWildcard bypass risk — mitigated by CLI mutual-exclusivity — The pattern Alphabetical ordering — Correct per ASCII value. All lexicographically ordered. ✅ Coverage of realistic agent forms — The three entries cover: (1) PR number with body args, (2) PR number without inline body (rare interactive form), and (3) current-branch inference with body args. The
No PowerShell mirror — Consistent with the established pattern (write/mutating operations are Bash-only in the allow list). ✅ No REVIEW.md blocking criteria — No authorization bypass, no tenant-scoped data path, no secrets in tracked source, no injection surface, no atomicity concern. ✅ 🟡 Nit — The current-branch form with no inline body ( Overall: LGTM. The narrowing is correct, the wildcard bypass is closed at the CLI layer, and the change satisfies the component's change-discipline requirement. Good fix. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2455888a34
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "Bash(gh pr diff *)", | ||
| "Bash(gh pr list *)", | ||
| "Bash(gh pr ready *)", | ||
| "Bash(gh pr review * --comment *)", |
There was a problem hiding this comment.
Fresh evidence after the wildcard was narrowed: this glob still auto-allows commands such as gh pr review 123 --body --comment --approve, where --comment is just the string value for GitHub CLI's --body <string> flag and the actual review action is --approve (or similarly --request-changes). The gh pr review implementation then sets ReviewApprove when flagApprove is true, so an unattended session with review permission can still approve or block a PR while matching this line's textual --comment pattern; constrain the allow rule so --comment cannot be consumed as another option's value.
Useful? React with 👍 / 👎.
…ins (#267) ## What Re-derives the `claude-permissions` allow floor against Claude Code auto mode's built-in coverage (fleet default is now `permissions.defaultMode: "auto"`; migration tracked in melodic-software/dotfiles#309, this PR is its P1b phase). Allow floor 90 → 81; deny grows by 14 pull-spelling rows (`--no-verify` and forced-fetch families mirrored onto `git pull`, both shells — the review round that also dropped the rerun grant); the component carries no ask array. An earlier shape of this PR trimmed 45 rows on the premise "auto mode covers read-only commands promptless." That premise was then MEASURED and found false for everything except read-only git (empirical record: the operator-local migration file `.work/auto-mode-defaults/RESEARCH.md`, "Addendum — built-in read-only set, MEASURED" — not repo-reachable, so the decisive figures are inlined here; `claude -p --setting-sources "" --permission-mode default`, v2.1.219): **9/9 git inspection spellings ran promptless** (`status`, `diff`, `log`, `show`, `branch`, `branch --list`, `ls-files`, `merge-base`, `rev-parse`); **16/16 non-git commands were denied** — every `gh` read verb, every third-party linter, even bare `--version`. Membership is command identity, not flag safety, and it is tool-agnostic (git ran promptless through `PowerShell()` too). The trim was re-derived accordingly: - **TRIM 16** — read-only *git* inspection only (10 `Bash()` + 6 `PowerShell()` spellings). The built-in read-only command set runs these promptless in every mode, so floor entries for them are dead weight everywhere, not just under auto. - **RESTORE 29** — the read-only `gh` verbs, third-party linters, and `claude plugin` inspection rows the earlier shape wrongly trimmed. Measured denied without them; they are load-bearing in every non-auto lane (workflow subagents always run `acceptEdits`; headless `-p`/SDK runs fail with nobody to answer a prompt). - **REPLACE 2 dead rows with 4 live spellings** — the bare-wrapper rows `Bash(source-control-babysit-merge *)` / `Bash(source-control-babysit-resolve-thread *)` match nothing today: the plugin `bin/` directory is not on the shell's PATH and the skill invokes each wrapper as `bash "${CLAUDE_PLUGIN_ROOT}/bin/<wrapper>"` (claude-code-plugins `3fc72d351c`). They are dropped, and the wrappers instead carry the same interim path-form convention as the five plugin scripts already on the floor — quoted and unquoted `Bash(bash …${CLAUDE_PLUGIN_ROOT}/bin/<wrapper>…*)` spellings, 4 rows. Bare-name rows return when claude-code-plugins#843 makes the bare name resolve. - **DO NOT PROMOTE the 4 `pytest` rows** the earlier shape added. A test runner executes whatever test files are on disk, and a session that can write files plus a blanket `pytest` grant compose into a general code-execution grant wearing a narrow name — the same refusal melodic-software/dotfiles#315's git entry records three times. Test runs are judged per session. - **PROMOTE 6** — additions with observed-usage evidence or a reviewed unattended-lane need, per the component's change discipline: `git pull` (bare + starred) and four gh write verbs (rationale below). No PowerShell parity rows: the PS-mirror convention covers read-only inspection only, and write verbs have never carried mirrors. ### Promote — issue/PR metadata verbs and CI re-run - `gh issue close *`, `gh issue reopen *`, `gh pr edit *` — the same class as floor incumbents already present (`gh issue comment/create/edit`, `gh pr comment/create/ready`); excluding them was an arbitrary gap in an otherwise-consistent issue/PR metadata-write posture. ### Considered, not promoted (decision record) - `Bash(gh api graphql *)`, `Bash(gh api orgs/*)`, `Bash(gh api repos/*)` — empirical evidence in melodic-software/claude-code-plugins#1235 (gap 2) shows prefix allow rules of exactly this shape did NOT short-circuit the classifier's category safety layer for external writes. They also widen the granted surface in non-auto sessions, which is precisely the posture the floor's rows exist to define. Kept machine-local. - `Bash(gh workflow run *)` — dispatches arbitrary workflows with operator-controlled inputs; a CI-execution trigger is a different risk tier from issue/PR metadata edits, and the melodic-software/claude-code-plugins#697 precedent keeps high-blast-radius write verbs (merge) off the floor. Kept machine-local. - `Edit(//**/github.com/melodic-software/**)` — encodes this machine's checkout layout; a fleet-distributed floor must not carry a machine's directory layout. Kept machine-local. ## Scope limit under auto mode These grants are deterministic only outside auto mode's `classifyAllShell`. That key is NOT yet set fleet-wide: G3a split it out of melodic-software/dotfiles#315 into its own gated follow-up draft PR in dotfiles, which lands only after the operator's prose-grant test (dotfiles#309 P3 test 3) passes. Until then the narrow pre-classifier short-circuit these rows provide still operates in auto-mode sessions for matching spellings. Once the key lands, every shell allow rule is suspended in auto-mode sessions and this floor is the fallback posture for non-auto lanes — which the measurement above shows are exactly the lanes that need it. `deny` is unaffected in every mode and every regime — `classifyAllShell` suspends allow rules only; the 262 deny entries stay pre-classifier and non-overridable, which is exactly why G1 keeps all of them. ## Why - The floor predates auto mode: it was built (#210, #223, #228; lineage melodic-software/claude-code-plugins#695) to pre-approve safe commands so unattended loops never prompt. Auto mode provides that natively for read-only git only — the measured set — so exactly those entries no longer pay for their maintenance surface. - Merge-verb policy is unchanged: `gh pr merge` stays out of the allow floor and `gh pr merge --auto` stays denied, per melodic-software/claude-code-plugins#697; babysit merges keep going through the guarded gate wrapper. - Deny/ask keep-all is a locked decision (dotfiles#309 G1): explicit deny is pre-classifier and non-overridable, while built-in coverage is consent-gated soft_deny — trimming deny would downgrade hard blocks to consent gates. ## Per-entry disposition (reconciles to 90) Baseline 90 = TRIM 16 + DROP 2 + KEEP 72; final 81 = KEEP 72 + PROMOTE 5 + REPLACE 4 (the wrappers' path-form spellings). A review round later dropped the sixth PROMOTE candidate (`gh run rerun *` — a bare run id names any accessible run, so no floor glob holds it to merged workflow code; judged per session until a guarded wrapper exists). <details> <summary>TRIM — 16 entries (read-only git, built-in-covered, measured)</summary> | Entry | | --- | | `Bash(git branch --list *)` | | `Bash(git branch)` | | `Bash(git diff *)` | | `Bash(git log *)` | | `Bash(git ls-files *)` | | `Bash(git merge-base *)` | | `Bash(git rev-parse *)` | | `Bash(git show *)` | | `Bash(git status *)` | | `Bash(git status)` | | `PowerShell(git diff *)` | | `PowerShell(git log *)` | | `PowerShell(git merge-base *)` | | `PowerShell(git rev-parse *)` | | `PowerShell(git show *)` | | `PowerShell(git status *)` | </details> <details> <summary>DROP 2 / REPLACE 4 — bare wrappers out, path-form spellings in</summary> Dropped (match nothing until ccp#843): | Entry | | --- | | `Bash(source-control-babysit-merge *)` | | `Bash(source-control-babysit-resolve-thread *)` | Added in their place (the real invocation shape, parity with the five script rows): | Entry | | --- | | `Bash(bash "${CLAUDE_PLUGIN_ROOT}/bin/source-control-babysit-merge"*)` | | `Bash(bash ${CLAUDE_PLUGIN_ROOT}/bin/source-control-babysit-merge*)` | | `Bash(bash "${CLAUDE_PLUGIN_ROOT}/bin/source-control-babysit-resolve-thread"*)` | | `Bash(bash ${CLAUDE_PLUGIN_ROOT}/bin/source-control-babysit-resolve-thread*)` | </details> <details> <summary>KEEP — 72 entries</summary> Everything else in the baseline: the non-destructive working verbs, the gh write incumbents, the RESTORED read-only `gh`/linter/`claude plugin` rows (measured load-bearing), the 10 interim `${CLAUDE_PLUGIN_ROOT}` script-path rules (README carries the interim note; end state is bare wrappers on the plugin `bin/` PATH, trigger claude-code-plugins#843), and the PowerShell read-only mirrors outside the built-in git set. </details> <details> <summary>PROMOTE — 6 entries</summary> | Entry | Evidence | | --- | --- | | `Bash(gh issue close *)` | metadata-write parity with floor incumbents | | `Bash(gh issue reopen *)` | metadata-write parity with floor incumbents | | `Bash(gh pr edit *)` | metadata-write parity with floor incumbents | | `Bash(git pull *)` | operator runtime keeper (dotfiles#309 P2 keep set) | | `Bash(git pull)` | operator runtime keeper (dotfiles#309 P2 keep set) | </details> ## Status DRAFT — stays draft pending operator review of this amended body; merges AFTER melodic-software/dotfiles#315 (merging this first would trigger the standards→dotfiles sync bot to rewrite `.chezmoidata/claude-permissions.json` mid-flight under #315). ## Related No linked issue. This is the P1b phase of a cross-repo migration; it closes no issue in this repository. - melodic-software/dotfiles#309 — auto-mode migration (P1b phase) - melodic-software/dotfiles#315 — host-side auto-mode hardening (merge-order dependency) - melodic-software/claude-code-plugins#697 — merge-verb / classifier-tuning policy - melodic-software/claude-code-plugins#843 — bare-wrapper PATH end state - melodic-software/claude-code-plugins#695 — component lineage (introduced in #210) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 participant
What
Adds
gh pr reviewcomment-form spellings to theclaude-permissionsallow floor, alphabetically betweengh pr readyandgh pr view:Bash(gh pr review * --comment *)Bash(gh pr review * --comment)Bash(gh pr review --comment *)--approveand--request-changesforms are deliberately NOT floor-allowed — they can satisfy or block required-review gates, so they stay prompted per session.Why
Component change discipline requires observed-usage evidence for
allowadditions: recurring real-session need to submit PR review comments from agent sessions (recorded in the plugin-batch session handoff, 2026-07-21). Fits the existing "PR and issue CRUD" allow posture;gh pr merge --autoremains denied.Distribution
Reaches the dotfiles consumer (
.chezmoidata/claude-permissions.json) through the ordinary sync PR; the modify-template union then composes it into~/.claude/settings.json.Related
No linked issue. Component introduced in #210; observed-usage evidence recorded in the plugin-batch session handoff (claude-code-plugins
.work/handoffs/20260721T053034Z-handoff-plugin-batch-complete.md).🤖 Generated with Claude Code
https://claude.ai/code/session_01U9zbRxtRuTx1WsQavFc2Gi