From 9bfcb98c8c81f685899a532153f451903fcbeae8 Mon Sep 17 00:00:00 2001 From: castrojo Date: Sat, 8 Aug 2026 18:48:19 -0400 Subject: [PATCH 1/6] docs(skills): maintainer-driven tool actions and duplicate-cluster resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit human-gates: the Merge 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 still enforced by GitHub, is the human acting at the gate; --admin overrides, submitting reviews, and non-interactive batch mutation remain forbidden for any tool. This ambiguity had already produced a walker stricter than the doctrine it implements. pr-review: competing-pair detection could find a duplicate cluster but had no verb for resolving it. Add the ordered procedure: survivor from diff evidence, arm the survivor first with --match-head-commit, comment before close, never 'not planned', then the linked-issue re-check — halting on the first failure. Assisted-by: Claude Opus 4.8 via GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/skills/human-gates.md | 11 +++- docs/skills/index.json | 10 ++-- docs/skills/index.md | 2 +- docs/skills/pr-review/SKILL.md | 5 +- .../pr-review/references/duplicate-cluster.md | 60 +++++++++++++++++++ 5 files changed, 78 insertions(+), 10 deletions(-) create mode 100644 docs/skills/pr-review/references/duplicate-cluster.md diff --git a/docs/skills/human-gates.md b/docs/skills/human-gates.md index bfadb449..0715fcad 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.1" +last_updated: "2026-08-08" 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,13 @@ 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. 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 09d2539c..b4a759fd 100644 --- a/docs/skills/index.json +++ b/docs/skills/index.json @@ -1,5 +1,5 @@ { - "generated_at": "2026-09-20", + "generated_at": "2026-09-22", "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.1", + "last_updated": "2026-08-08", "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 ca53a82b..a56fb104 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-09-20 · schema 1.0 · 40 skills +Generated: 2026-09-22 · 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..da046e8b 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). 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..1ea09ef4 --- /dev/null +++ b/docs/skills/pr-review/references/duplicate-cluster.md @@ -0,0 +1,60 @@ +# Duplicate-Cluster Resolution + +A competing pair that shares a *closing issue* — or two Renovate PRs that +normalize to the *same dependency* — is one piece of work submitted twice, not +an ordering hazard. Ordering does not fix it; one of the PRs has to go away. + +Resolve the cluster as a unit, halting on the first failure. + +## Procedure + +**1. The human 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: + +```bash +gh pr diff +gh pr diff +``` + +**2. Arm the survivor before touching anything else.** + +Read the head SHA live and pin the merge to it: + +```bash +sha=$(gh pr view --json headRefOid --jq .headRefOid) +gh pr merge --squash --auto --match-head-commit "$sha" +``` + +`--match-head-commit` makes a push that lands between your read and the merge a +server-side refusal rather than a silent merge of unreviewed code. + +**3. Comment on each superseded PR** naming the survivor and the evidence. + +Use `--body-file` — never pass prose through a shell with `--body`: + +```bash +gh pr comment --body-file /tmp/superseded.md +``` + +**4. Close the superseded PR.** + +```bash +gh pr close +``` + +Never `--reason "not planned"`, and never a label swap in place of a close. +Both misreport why 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. + +## 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. From 793b1653963a9ca7e0b5461a1618c7e21e0d9cf2 Mon Sep 17 00:00:00 2001 From: castrojo Date: Thu, 10 Sep 2026 19:30:17 -0400 Subject: [PATCH 2/6] docs(skills): gate duplicate-cluster mutations Require human confirmation that a cluster is truly duplicate, preserve complementary PRs, and gate each merge, comment, and close action individually. Forbid batch closure so the documented maintainer tool cannot infer or automate destructive actions. Assisted-by: GPT-5.6 Luna via GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../pr-review/references/duplicate-cluster.md | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/docs/skills/pr-review/references/duplicate-cluster.md b/docs/skills/pr-review/references/duplicate-cluster.md index 1ea09ef4..95519edf 100644 --- a/docs/skills/pr-review/references/duplicate-cluster.md +++ b/docs/skills/pr-review/references/duplicate-cluster.md @@ -1,14 +1,16 @@ # Duplicate-Cluster Resolution A competing pair that shares a *closing issue* — or two Renovate PRs that -normalize to the *same dependency* — is one piece of work submitted twice, not -an ordering hazard. Ordering does not fix it; one of the PRs has to go away. +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 the cluster as a unit, halting on the first failure. +Resolve a confirmed cluster as a unit, halting on the first failure. ## Procedure -**1. The human names the survivor.** +**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: @@ -18,7 +20,12 @@ gh pr diff gh pr diff ``` -**2. Arm the survivor before touching anything else.** +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.** Read the head SHA live and pin the merge to it: @@ -30,7 +37,9 @@ gh pr merge --squash --auto --match-head-commit "$sha" `--match-head-commit` makes a push that lands between your read and the merge a server-side refusal rather than a silent merge of unreviewed code. -**3. Comment on each superseded PR** naming the survivor and the evidence. +**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`: @@ -38,12 +47,15 @@ Use `--body-file` — never pass prose through a shell with `--body`: gh pr comment --body-file /tmp/superseded.md ``` -**4. Close the superseded PR.** +**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 `--reason "not planned"`, and never a label swap in place of a close. Both misreport why the work went away. From 9ab971367ff4ec64c2bb16d2c9c09129d6460c3a Mon Sep 17 00:00:00 2001 From: "kubestellar-hive[bot]" <280983584+kubestellar-hive[bot]@users.noreply.github.com> Date: Fri, 11 Sep 2026 22:55:40 -0400 Subject: [PATCH 3/6] docs(skills): bound the merge-gate tool carve-out per review feedback Address hanthor's review on #970 in the text of human-gates.md: - Name the reviewed vehicle: the pr-review card loop, one keypress per card, never a confirm-all prompt spanning items. - State that close carries the same weight as merge: the carve-out covers keypress-confirmed closes only, so an auto-close without a per-item keypress (e.g. the obsolescence remedy proposed in #1073) cannot lean on this paragraph. - State that arming auto-merge on the keypress is the same decision deferred until checks pass, in scope only with --match-head-commit pinning the reviewed head so drift fails server-side. human-gates 1.1 -> 1.2; index.json/index.md regenerated with scripts/generate_skill_index.py --write. check-skill-frontmatter.sh, generate_skill_index.py --check, check-doc-links.sh, and tests/test_skill_docs.py (10 passed) all green. Assisted-by: Kimi K3 via GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/skills/human-gates.md | 17 +++++++++++++---- docs/skills/index.json | 4 ++-- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/docs/skills/human-gates.md b/docs/skills/human-gates.md index 0715fcad..ec2d70e5 100644 --- a/docs/skills/human-gates.md +++ b/docs/skills/human-gates.md @@ -1,7 +1,7 @@ --- name: human-gates -version: "1.1" -last_updated: "2026-08-08" +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 @@ -90,8 +90,17 @@ Agents never self-merge, never bypass branch protection, and never force-push to 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. What remains forbidden for any -tool: `--admin` overrides, submitting an approving review, and any +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 to the reviewed head with +`--match-head-commit`, so drift between keypress 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. --- diff --git a/docs/skills/index.json b/docs/skills/index.json index b4a759fd..fda27e36 100644 --- a/docs/skills/index.json +++ b/docs/skills/index.json @@ -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.1", - "last_updated": "2026-08-08", + "version": "1.2", + "last_updated": "2026-09-11", "doc_type": "reference" }, { From 20f285ed818773f46fb04dd733a3c0e37ad4808f Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Sun, 20 Sep 2026 01:27:16 -0400 Subject: [PATCH 4/6] docs(skills): pin merge to the reviewed head, not a live re-read Step 2 of the duplicate-cluster procedure read the survivor head SHA at keypress time, so a push landing between the step-1 diff evidence and the keypress became the pinned head and merged unreviewed. Capture the SHA with the diff in step 1, pin to that, and treat a --match-head-commit refusal as a signal to re-present the diff and retake the keypress rather than an error to retry around. Tighten the matching human-gates claim so it describes the head SHA captured alongside the reviewed diff instead of "the reviewed head" generally. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/skills/human-gates.md | 5 ++-- .../pr-review/references/duplicate-cluster.md | 23 ++++++++++++++----- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/docs/skills/human-gates.md b/docs/skills/human-gates.md index ec2d70e5..a8734387 100644 --- a/docs/skills/human-gates.md +++ b/docs/skills/human-gates.md @@ -97,8 +97,9 @@ 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 to the reviewed head with -`--match-head-commit`, so drift between keypress and landing fails +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. diff --git a/docs/skills/pr-review/references/duplicate-cluster.md b/docs/skills/pr-review/references/duplicate-cluster.md index 95519edf..a17f7a02 100644 --- a/docs/skills/pr-review/references/duplicate-cluster.md +++ b/docs/skills/pr-review/references/duplicate-cluster.md @@ -13,9 +13,12 @@ Resolve a confirmed cluster as a unit, halting on the first failure. **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: +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 ``` @@ -27,15 +30,23 @@ survivor from the shared issue or dependency alone. **2. Arm the survivor after an explicit per-item merge keypress and before touching anything else.** -Read the head SHA live and pin the merge to it: +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. ```bash -sha=$(gh pr view --json headRefOid --jq .headRefOid) -gh pr merge --squash --auto --match-head-commit "$sha" +gh pr merge --squash --auto --match-head-commit "$sha_S" ``` -`--match-head-commit` makes a push that lands between your read and the merge a -server-side refusal rather than a silent merge of unreviewed code. +`--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. + +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 From f1ddcd87b08dcf1307ffdf3c0a5270a5f175329b Mon Sep 17 00:00:00 2001 From: "sec-check[bot]" Date: Sun, 20 Sep 2026 02:14:50 -0400 Subject: [PATCH 5/6] docs(skills): fix duplicate-cluster close reason and reference table gh pr close has no --reason flag, so move the "not planned" prohibition to the linked-issue re-check where gh issue close --reason applies. Add the new duplicate-cluster.md row to the pr-review References table. Assisted-by: Claude Opus 5 via GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/skills/pr-review/SKILL.md | 1 + docs/skills/pr-review/references/duplicate-cluster.md | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/skills/pr-review/SKILL.md b/docs/skills/pr-review/SKILL.md index da046e8b..5ffa7392 100644 --- a/docs/skills/pr-review/SKILL.md +++ b/docs/skills/pr-review/SKILL.md @@ -167,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 index a17f7a02..e43932a8 100644 --- a/docs/skills/pr-review/references/duplicate-cluster.md +++ b/docs/skills/pr-review/references/duplicate-cluster.md @@ -67,14 +67,18 @@ gh pr close Never close the whole cluster from a script or batch command. -Never `--reason "not planned"`, and never a label swap in place of a close. -Both misreport why the work went away. +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 From 5a7dce5fc0f4fbd435ca556e3e17752db21be2a3 Mon Sep 17 00:00:00 2001 From: "sec-check[bot]" Date: Sun, 20 Sep 2026 02:48:05 -0400 Subject: [PATCH 6/6] docs(skills): define sha_S and cover the clean-status merge fallback Step 2 of the duplicate-cluster procedure used an undefined `$sha_S`, which expands to an empty `--match-head-commit ""` when copy-pasted. Name it as the survivor's SHA from step 1. `gh pr merge --auto` is also rejected with "Pull request is in clean status" when the survivor is already green and the repo has no merge queue. Because the procedure halts on the first failure, that common case stopped the whole cluster resolution. Document the direct-merge fallback with the same `--match-head-commit` pin, and scope which form applies where. Assisted-by: Claude Opus 5 via GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../pr-review/references/duplicate-cluster.md | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/docs/skills/pr-review/references/duplicate-cluster.md b/docs/skills/pr-review/references/duplicate-cluster.md index e43932a8..59dd4c05 100644 --- a/docs/skills/pr-review/references/duplicate-cluster.md +++ b/docs/skills/pr-review/references/duplicate-cluster.md @@ -35,7 +35,11 @@ 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" ``` @@ -44,6 +48,27 @@ 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.