diff --git a/docs/skills/human-gates.md b/docs/skills/human-gates.md index bfadb449..a8734387 100644 --- a/docs/skills/human-gates.md +++ b/docs/skills/human-gates.md @@ -1,7 +1,7 @@ --- name: human-gates -version: "1.0" -last_updated: "2026-06-23" +version: "1.2" +last_updated: "2026-09-11" id: human-gates one_line_purpose: Decide when to stop for Design, Security, Breakage, or Merge review. entry_point: docs/skills/human-gates.md @@ -87,6 +87,23 @@ This gate is always human. CI passing plus an approving review from a human revi Agents never self-merge, never bypass branch protection, and never force-push to a protected branch. +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. The reviewed vehicle is the +`pr-review` card loop: one keypress per card, never a confirm-all prompt +spanning items. Close carries the same weight as merge — GitHub enforces +nothing on a close — so this carve-out covers keypress-confirmed closes only; +closing pull requests without a per-item keypress is agent mutation of +human-visible state and stays forbidden. Arming auto-merge +(`gh pr merge --auto`) on the keypress is the same human decision deferred +until checks pass; it is in scope only pinned with `--match-head-commit` to the +head SHA captured alongside the diff the human reviewed — not to a head re-read +at keypress time — so drift between review and landing fails +server-side instead of merging unreviewed code. What remains forbidden for +any tool: `--admin` overrides, submitting an approving review, and any +non-interactive batch mutation. + --- ## How to Signal a Gate diff --git a/docs/skills/index.json b/docs/skills/index.json index 6bc2915d..c6e2ecb5 100644 --- a/docs/skills/index.json +++ b/docs/skills/index.json @@ -1,5 +1,5 @@ { - "generated_at": "2026-08-18", + "generated_at": "2026-09-11", "schema_version": "1.0", "skills": [ { @@ -380,8 +380,8 @@ "security" ], "description": "The four human decision gates \u2014 Design, Security, Breakage, and Merge \u2014 when an agent must stop and request human input. Use when uncertain whether a change requires human review, or to verify evidence requirements before opening a PR.", - "version": "1.0", - "last_updated": "2026-06-23", + "version": "1.2", + "last_updated": "2026-09-11", "doc_type": "reference" }, { @@ -521,8 +521,8 @@ "backlog" ], "description": "Human-decides, agent-lands PR and issue backlog review. Present one card at a time, take the human verdict, execute it immediately, then advance. Use when reviewing the PR queue or triaging the issue backlog.", - "version": "3.5", - "last_updated": "2026-08-08", + "version": "3.6", + "last_updated": "2026-09-06", "doc_type": "procedure" }, { diff --git a/docs/skills/index.md b/docs/skills/index.md index 8dd84c2a..a2407248 100644 --- a/docs/skills/index.md +++ b/docs/skills/index.md @@ -3,7 +3,7 @@ This file is a human-readable mirror of `index.json`. Both are generated by `scripts/generate_skill_index.py` — do not hand-edit either file. -Generated: 2026-08-18 · schema 1.0 · 40 skills +Generated: 2026-09-11 · schema 1.0 · 40 skills | id | category | status | one-line purpose | |---|---|---|---| diff --git a/docs/skills/pr-review/SKILL.md b/docs/skills/pr-review/SKILL.md index ce68dfa9..5ffa7392 100644 --- a/docs/skills/pr-review/SKILL.md +++ b/docs/skills/pr-review/SKILL.md @@ -1,7 +1,7 @@ --- name: pr-review -version: "3.5" -last_updated: "2026-08-08" +version: "3.6" +last_updated: "2026-09-06" id: pr-review one_line_purpose: Run human-decides, agent-lands backlog review one card at a time. entry_point: docs/skills/pr-review/SKILL.md @@ -80,6 +80,7 @@ gh pr list --limit 60 \ Filter on `author.is_bot` (real boolean). Fetch a WIDE window then slice to 5 *after* filtering. For a bot sweep, invert to `select(.author.is_bot)`. Present each PR as a one-screen card. Field definitions and the `mergeStateStatus` table: [references/card-fields.md](references/card-fields.md). - **Competing-pair detection (mandatory):** pairwise-intersect file paths and `closingIssuesReferences` across the batch. Print `⚠️ COMPETING PAIR` on any overlap — human must resolve before both can be voted `merge`. +- **Duplicate-cluster resolution:** a pair sharing a *closing issue*, or two Renovate PRs normalizing to the *same dependency*, is one piece of work twice — resolve as a unit, arming the survivor before closing the rest. Full procedure: [references/duplicate-cluster.md](references/duplicate-cluster.md). - **CI card classification:** classify every red before it costs a human slot. Full procedure: [references/red-check-triage.md](references/red-check-triage.md). - **Dismissed-approval check (mandatory):** diff current head against the approved commit SHA. Full procedure: [references/dismissed-approval.md](references/dismissed-approval.md). @@ -166,6 +167,7 @@ queue state reading, branch update, and fork PR rebase. | File | Contents | |---|---| | [references/card-fields.md](references/card-fields.md) | Full card field reference and `mergeStateStatus` table | +| [references/duplicate-cluster.md](references/duplicate-cluster.md) | Duplicate-cluster resolution: arm the survivor, then close the rest | | [references/red-check-triage.md](references/red-check-triage.md) | Classifying red checks, infra-flake correlation, `gh` CLI traps | | [references/dismissed-approval.md](references/dismissed-approval.md) | Dismissed-approval regression check procedure | | [references/worked-example.md](references/worked-example.md) | Worked example session | diff --git a/docs/skills/pr-review/references/duplicate-cluster.md b/docs/skills/pr-review/references/duplicate-cluster.md new file mode 100644 index 00000000..59dd4c05 --- /dev/null +++ b/docs/skills/pr-review/references/duplicate-cluster.md @@ -0,0 +1,112 @@ +# Duplicate-Cluster Resolution + +A competing pair that shares a *closing issue* — or two Renovate PRs that +normalize to the *same dependency* — is a **candidate** duplicate cluster, +not proof that one PR must be closed. Compare the actual diffs: complementary +work stays as separate PRs. Resolve as a duplicate only after the human +confirms that it is the same work and names the survivor. + +Resolve a confirmed cluster as a unit, halting on the first failure. + +## Procedure + +**1. The human confirms the duplicate and names the survivor.** + +Present diff evidence first, then let the human choose. `gh pr diff` works for +fork heads, so there is no reason to decide from titles alone. Capture each +head SHA *with* the diff, so the evidence and the SHA describe the same code: + +```bash +sha_A=$(gh pr view --json headRefOid --jq .headRefOid) +sha_B=$(gh pr view --json headRefOid --jq .headRefOid) +gh pr diff +gh pr diff +``` + +If the diffs are complementary, stop this procedure and leave both PRs open; +return to the competing-pair review instead. The tool must not infer a +survivor from the shared issue or dependency alone. + +**2. Arm the survivor after an explicit per-item merge keypress and before +touching anything else.** + +Pin the merge to the SHA you captured in step 1 — the head the human actually +reviewed. Never re-read the head at keypress time: a push that lands between +the evidence and the keypress would become the pinned head and merge +unreviewed. + +`sha_S` is whichever of `sha_A` / `sha_B` belongs to the survivor the human +named — substitute that variable, do not re-read the head: + +```bash +sha_S=$sha_A # or $sha_B — the survivor's SHA from step 1 +gh pr merge --squash --auto --match-head-commit "$sha_S" +``` + +`--match-head-commit` makes any head that is not the reviewed one a +server-side refusal rather than a silent merge of unreviewed code. Reading the +SHA before rendering the diff (step 1) keeps drift in that safe direction: the +worst case is a refusal, never an unreviewed merge. + +`--auto` only arms a merge that is still waiting on something. On a repo +without a merge queue, a survivor whose checks already pass has nothing to +queue, and GitHub rejects the request with `Pull request is in clean status`. +That is the common case for an already-green survivor, and it is **not** a +failure that should halt the cluster. Re-run without `--auto`, keeping the +same pin: + +```bash +gh pr merge --squash --match-head-commit "$sha_S" +``` + +The pin is the invariant, not the arming mode: both forms refuse if the head +moved off `$sha_S`. Never drop `--match-head-commit` to get a merge through, +and never reach for `--admin` without explicit human instruction. + +On `common`, where `main` has a merge queue, the arming form is the one that +works and the direct form is the one that gets rejected — see +[`merge-queue.md`](merge-queue.md). Read the error before choosing: only +`in clean status` justifies the direct form. Any other rejection stops the +procedure. + +A refusal is not an error to retry around. It means the survivor moved after +the human looked at it: go back to step 1, re-present the fresh diff, and take +a new keypress. Never re-read the SHA to make the merge succeed. + +**3. After a separate explicit keypress for each item, comment on each +superseded PR** naming the survivor and the evidence. Run each command +individually; never use a loop, `xargs`, or another batch mutation. + +Use `--body-file` — never pass prose through a shell with `--body`: + +```bash +gh pr comment --body-file /tmp/superseded.md +``` + +**4. After a separate explicit per-item close keypress, close one superseded +PR at a time.** + +```bash +gh pr close +``` + +Never close the whole cluster from a script or batch command. + +Never swap a label in place of a close: the PR stays open while the board +claims the work went away. + +**5. Re-check the linked issues.** + +A still-open issue whose last open PR you just closed is a **finding to +report**, not something to silently fix. Surface it to the human. + +Do not close that issue yourself, and never reach for +`gh issue close --reason "not planned"` to tidy it up. The work was superseded, +not abandoned, so that reason misreports why the issue went away. + +## Why the order matters + +Arming the survivor first (step 2) means that if anything later in the sequence +fails, the work still lands. Closing first and failing to arm leaves the +cluster with no open PR and an open issue — strictly worse than where you +started.