docs(skills): maintainer-driven tool actions and duplicate-cluster resolution

[castrojo]

Why

Downstream in projectbluefin/review, the maintainer's PR-walk tool (bluefin-review queue) detects duplicate clusters but could not act on them: a local test banned pr merge outright — stricter than the doctrine it implements. The ambiguity sat here: human-gates said "agents never self-merge" without distinguishing a tool executing the maintainer's explicit keypress, and pr-review had competing-pair detection with no cluster resolution verb.

What changes

• human-gates.md (1.1): the Merge Gate now distinguishes agent self-merge (never) from a tool executing the maintainer's explicit per-item keypress with rulesets still enforced (the documented human-decides loop). --admin, submitting reviews, and non-interactive batch mutation stay forbidden for any tool.
• pr-review/SKILL.md (3.5): new "Duplicate-cluster resolution" subsection — survivor from diff evidence, arm the survivor first with --match-head-commit, comment before close, never --reason "not planned", linked-issue re-check, halt on first failure. Compressed to fit the 500-line hard cap (exactly 500; one stray blank line reclaimed).

Design memo driving this: projectbluefin/review session artifact, org review walk design (§0, §4, §9).

Validation

check-skill-frontmatter.sh exit 0 (500/500, warnings pre-existing), generate_skill_index.py --write + check-skill-index.sh clean.

Assisted-by: Claude Opus 4.8 via GitHub Copilot
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

[hanthor]

Approving. The carve-out is correctly bounded and the duplicate-cluster procedure is careful in the places that matter.

On the human-gates amendment. The distinction it draws is the right one — the concern behind "agents never self-merge" is unreviewed change reaching a protected branch, not the mechanical identity of the process invoking the API. A per-item maintainer decision with rulesets still enforced server-side doesn't create that risk. Retaining the three hard prohibitions is what makes it safe to state:

• no --admin (the only one that actually bypasses protection)
• no submitting an approving review (an agent must never manufacture the approval it then acts on — this is the load-bearing one)
• no non-interactive batch mutation (preserves per-item intent)

--match-head-commit <sha> with the SHA read live is the best detail in here. Converting "the branch moved under us" from a race into a server-side refusal is exactly right, and it's the kind of thing that's easy to omit and impossible to notice until it bites.

Also correct: --body-file rather than --body with prose through a shell, and "a still-open issue with no remaining open PR is a finding to report, not something to silently fix." The latter keeps the agent from quietly papering over a state the human should see.

---

One gap I'd like addressed in the text: --auto is a deferred action, and the amendment's justification doesn't cover it.

The new human-gates paragraph rests on "the maintainer's explicit per-item keypress." But step (2) arms gh pr merge <S> --squash --auto, which lands the merge at some later point once checks pass — possibly minutes or hours after the keypress, unattended. That's still legitimate, but it isn't the same thing as the human pressing a key and the merge happening; it's the human pre-authorizing a conditional future merge.

--match-head-commit covers the obvious drift (a new push to the head), but not everything that can change between authorization and landing — the base branch advancing, a required check being reconfigured, or the superseded PRs in the cluster being reopened. Worth saying explicitly in human-gates.md that deferred/auto-merge is in scope for this carve-out and why the head-SHA pin is considered sufficient. Right now a reader could reasonably conclude auto-merge falls under "non-interactive batch mutation" and is forbidden, which contradicts the procedure in pr-review/SKILL.md. Since these two files are meant to be read together, that ambiguity is worth closing.

Minor: the ordering in the cluster procedure — arm survivor (2), then close superseded (3–4) — is the safe direction, since a failure mid-sequence leaves duplicates open rather than everything closed with nothing landing. Worth stating that rationale inline; "halting on the first failure" says what but not why this order, and the next editor might reasonably swap them to "clean up first."

Housekeeping: currently DIRTY against main — needs a rebase. Since index.json / index.md are generated by scripts/generate_skill_index.py, regenerate rather than hand-resolve those two if the conflict lands there.

[hanthor]

Technically the cleanest PR in this queue — but the human-gates.md paragraph is a policy change, not a docs change

Verified against current main:

• merges clean, validate green
• python3 scripts/generate_skill_index.py --check → docs/skills/index.json and index.md are up to date (40 skills) — the index is properly regenerated here (2026-08-18 → 2026-09-06, moving forward, 40 skills preserved, with the matching version/last_updated bumps for human-gates and pr-review). Several other open docs PRs in this queue regress that file backwards; this one is the correct pattern.
• python3 scripts/check-doc-links.sh → exit 0, so the new references/duplicate-cluster.md link from pr-review/SKILL.md resolves
• bash scripts/check-skill-frontmatter.sh → only the pre-existing brew-lifecycle/SKILL.md is 208 lines (soft max 200) warning, unchanged from main
• python3 -m pytest tests/test_skill_docs.py → 10 passed
• nothing under system_files/, so no bluefin / bluefin-lts / dakota blast radius

The duplicate-cluster half is uncontroversial and, judging by this queue, needed — #942 is superseded by merged #1020, #1018 duplicates a bluespeed recipe already on main, and #1080/#1073 are a dependent pair. "Arm the survivor before closing the rest" is exactly the right ordering.

The part that deserves a deliberate yes, not a skim

The human-gates.md addition materially loosens the Merge gate:

This gate binds agents, not the maintainer's own hands. A review tool that executes a merge or close only on the maintainer's explicit per-item keypress — with rulesets and branch protection still enforced by GitHub — is the human acting at the gate, not an agent self-merging.

Read narrowly this is obviously true: a maintainer pressing a key per item is a human decision. But it is being written into the document that defines what agents may not do, and the load-bearing words are all in the carve-out — "explicit", "per-item", "keypress". Two questions worth answering in the thread before this merges, because after it merges this paragraph is what future agents will cite:

1. What enforces "per-item"? The prohibition list that follows forbids "non-interactive batch mutation", but nothing here distinguishes fifteen individually-confirmed keypresses from a fifteen-item confirm-all prompt that is technically per-item. If pr-review's card loop is the intended vehicle, saying so by name would bound this to the reviewed implementation rather than to any tool that claims a keypress.
2. Does "close" carry the same weight as "merge"? Merge is gated by branch protection and rulesets, as the paragraph notes. Close is not — GitHub enforces nothing, so a mis-keyed close is unprotected by anything except the human's attention. The roadmap in #1073 separately proposes auto-closing superseded agent PRs, which is the same authority with the keypress removed. Worth being explicit that the carve-out covers keypress-confirmed close only, so #1073's item 5 cannot lean on this paragraph.

Neither is a defect in the diff; both are about how narrowly this reads in six months. If the answers are "the pr-review card loop, one keypress per card, close included and auto-close excluded", then saying that in the text makes this a clean merge from my side.

---

Generated by Claude Code

[castrojo]

K3 final batch audit (970/1000/1073/1080 cohort) — 970 verified and armed for landing.

Verified against current main (56d81ef):

• All 4 batch PRs squash-merge in landing order with zero conflicts (disjoint files).
• check-doc-links.sh exit 0; generate_skill_index.py --check clean (40 skills); check-skill-frontmatter.sh exit 0 (only pre-existing brew-lifecycle warning); test_skill_docs.py 10 passed — all run on the combined tree.
• All of hanthor's 2026-09-12 points are in head f831267: the carve-out names the pr-review card loop as the reviewed vehicle (one keypress per card, no confirm-all), keypress-confirmed closes only, --auto bounded to --match-head-commit pins, and duplicate-cluster.md carries the "Why the order matters" rationale.
• Cross-PR coherence: the keypress language added here is exactly what docs(roadmap): add org-wide ROADMAP.md consolidating 17 strategic findings into 3 phases #1073 Phase 0 item 5 cites; docs(roadmap): add org-wide ROADMAP.md consolidating 17 strategic findings into 3 phases #1073's "flag for human close" phrasing cannot lean on this carve-out — consistent in both directions. No conflict with docs(contributing): add reviewer-ladder.md — concrete proposal for the reviewer-scaling rung #1080's rung-2 proposal (human approval rights vs. tool-executed actions; maintainer-still-merges invariant holds in both).

State: hanthor's approval stands (1 of 2 required by the main — merge queue ruleset). This PR is castrojo-authored, so the batch reviewer's castrojo-token approval cannot count here — a second human maintainer approval is needed (ahmedadan / repires requested). Auto-merge is armed; it enters the squash merge queue automatically once the second approval lands.