Package PR Review Conduct and Copilot Instructions Upkeep as Skills

.agents/skills/copilot-instructions-keeper/SKILL.md

@@ -0,0 +1,94 @@
+---
+name: copilot-instructions-keeper
+description: >-
+  Helps keep a repo's .github/copilot-instructions.md in sync with the ptr727/ProjectTemplate hub
+  canonical, and stops the one mistake specific to this file: silently wiping its repo-local
+  "Disproved Claims" ledger entries during a resync. Use this whenever about to edit, overwrite,
+  re-vendor, or carry .github/copilot-instructions.md into a repo, whenever checking a repo for
+  drift against the hub or running a conformance sweep that touches this file, whenever GitHub
+  Copilot's review mechanics in this file look stale, wrong, or missing something the fleet
+  runbook should cover, or whenever standing up a new repo and carrying this file for the first
+  time. Also triggers on "why isn't the audit catching that this file is out of date," since the
+  fleet's mechanical audit checks this file for presence only, at intent fidelity, never for
+  content drift, so nothing else notices staleness here except a live check like this one.
+---
+
+# Copilot Instructions Keeper
+
+## Why this exists
+
+`.github/copilot-instructions.md` is read directly by the GitHub Copilot review bot, which
+cannot read a `SKILL.md`. Unlike other carried governance content, its rule text is never
+pointer-ized into a skill, this file always stays fully intact in every repo that carries it.
+This skill is about maintaining that file's carried copy, not a substitute for it.
+
+`spec/files.json` declares it `intent` fidelity, `whole: true`, covering three named sections
+(`Commit Messages and Pull Request Titles`, `Reviewing Carried Fleet Content`, `GitHub Copilot
+Review Runbook`), with `<owner>`, `<repo>`, and `<N>` placeholders filled per repo. **`intent`
+fidelity is checked by the fleet audit for presence only, never content** (`RESYNC.md`), so a hub
+revision to this file produces no finding anywhere in the mechanical audit. If this file is
+stale, nothing else in the fleet's tooling will say so, noticing has to happen in a live session
+like this one.
+
+## The one thing this file has that others don't: repo-local ledger entries
+
+The file's own "Disproved Claims" section states its rule plainly. **The section's shape and
+governing rules are carried, but its entries are not.** Each entry records a finding that was
+raised against this specific repository and disproved against this repository's code at a named
+revision. A repository carrying a copy of this file carries the shape and rules, deletes any
+entry whose subject it does not hold, and records what it has proved for itself.
+
+This means a blind re-vendor of the hub's canonical `copilot-instructions.md` over a downstream
+repo's copy is wrong in both directions:
+
+- Copying the hub's own "Disproved Claims" entries (about `ProjectTemplate` itself) into a
+  downstream repo attaches proofs about code that repo does not carry.
+- Overwriting a downstream repo's copy wholesale deletes any entries that repo itself has earned,
+  a live disproof, run against that repo's own tree, thrown away with no record.
+
+**Before touching this file in any repo other than the hub itself:**
+
+1. Read the current "Disproved Claims" section in that repo's copy, if it has one, and preserve
+   every entry that names a file or behavior that repo actually carries.
+2. Update everything else, the runbook mechanics, the three named sections, the rule text, to
+   match the hub canonical.
+3. Never carry the hub's own repo-specific "Disproved Claims" entries downstream. They name
+   `ProjectTemplate`'s own files and revisions, not the target repo's.
+4. If in doubt whether an entry is still valid for the current tree, treat it per the guard skill
+   below rather than guessing.
+
+This is the same failure class [`carried-instruction-file-guard`](../carried-instruction-file-guard/SKILL.md)
+exists to stop for `AGENTS.md`, `GOVERNANCE.md`, `CODESTYLE.md`, and `WORKFLOW.md`: a
+routine-sounding overwrite silently deleting content that is not a stale copy of the hub. Run
+that skill's distinctive-phrase probe against this file too before any full-file replace. It is
+not in that skill's own file list because its failure mode, ledger entries rather than fleet
+rules, is specific enough to warrant its own skill, but the underlying discipline, probe before
+overwrite, give a local addition a destination rather than deleting it, is the same.
+
+## Checking a repo's copy for drift
+
+1. Fetch the hub (`github.com/ptr727/ProjectTemplate`) `main` branch fresh. A stale local clone
+   answers confidently instead of failing.
+2. Compare the target repo's `.github/copilot-instructions.md` against the hub's, section by
+   section, at **intent** fidelity, judged by meaning, not by byte match. A content-identical
+   file with different `<owner>`/`<repo>` placeholder fills is current, not drifted.
+3. Read the "Disproved Claims" section separately from the rest. Judge its **shape and rules**
+   against the hub, and judge its **entries** only against what that repo itself carries (see
+   above), never against the hub's own entries.
+4. Report what is actually stale (a runbook mechanic that changed, a rule that moved, a new
+   section) versus what only looks different because it is correctly repo-specific.
+
+## Carrying it fresh, new repo or full resync
+
+Follow `RESYNC.md`'s general apply order for carried files, with the ledger rule above applied at
+the point this file is touched: carry the hub's current rule text and runbook mechanics, keep the
+target repo's own "Disproved Claims" entries (if any existed pre-resync) rather than replacing
+them with the hub's, and start a new repo's ledger empty rather than seeded from the hub's own
+proofs.
+
+## What this skill does not cover
+
+Content-style rules for other carried files (`AGENTS.md`, `GOVERNANCE.md`, `CODESTYLE.md`,
+`WORKFLOW.md`) are `carried-instruction-file-guard`'s job. The review-loop contract this file's
+runbook implements, the merge gate, triage, escalation, is `pr-review-conduct`'s job. This skill
+is narrowly about keeping this one file's carried copy correct.

.agents/skills/pr-review-conduct/SKILL.md

@@ -0,0 +1,126 @@
+---
+name: pr-review-conduct
+description: >-
+  Governs opening, driving, and merging a pull request review loop in a ptr727/ProjectTemplate
+  fleet repo: requesting a review after a push, triaging findings (including suppressed
+  low-confidence ones), replying and resolving threads, and deciding whether a PR is actually
+  mergeable. Use this whenever about to merge a PR, enable auto-merge, ask the maintainer for
+  merge permission, push a fix and move on without re-checking review state, or judge a PR
+  "green" or "clean" from CI or mergeStateStatus alone. Triggers even when the request sounds
+  routine, such as "merge this" or "it's all green, go ahead," because mergeStateStatus: CLEAN
+  reflects only required status checks and never open review findings, so a clean-looking PR can
+  still carry an unresolved or suppressed finding. Also triggers when a review loop looks stuck
+  (no review landing, findings that keep reappearing) or when deciding a finding is real, false,
+  deferred, or a deliberate decline. Provider-specific mechanics (GitHub Copilot's request/poll/
+  reply API calls) live in .github/copilot-instructions.md's "GitHub Copilot Review Runbook,"
+  this skill is the contract that runbook implements, not a replacement for it.
+---
+
+# PR Review Conduct
+
+## Why this exists
+
+`mergeStateStatus: CLEAN` reports only required status checks. It says nothing about an open
+review thread, a suppressed low-confidence finding sitting uncollapsed in a review body, or a
+review that covered the head SHA but read only part of the diff. A PR that looks done, green
+checks, no visible comments, routinely still carries a finding nobody has answered. Treating
+"green" as "mergeable" is the single most common way this loop gets skipped.
+
+## Merge Gate, check this before merging or enabling auto-merge
+
+**Do not merge, and do not enable auto-merge, unless ALL of these hold:**
+
+1. Required status checks are green, and where they are not, the reason is **read**, never
+   inferred. `BLOCKED` covers a failed check, a required check nothing is running, an unresolved
+   thread, and a missing approval alike, and the response differs by cause.
+2. A review is confirmed on the **current head SHA**, matched by commit SHA rather than assumed
+   from a green merge-state. A push makes checks go green *before* the re-review lands, and the
+   matched review is **read**, not just counted. A review can carry the head SHA and still decline
+   the PR outright, or say it read only part of the changed files.
+3. **Every** finding on that head SHA is closed: threads resolved, issue-level comments (which
+   have no resolve action) triaged and replied to, **and** the low-confidence findings collapsed
+   in the review body investigated and answered. Those appear in no thread, so polling threads
+   alone reports a clean pass while they stand.
+4. Nothing in the review was a shape the tooling could not read (an unrecognized heading, a moved
+   section, an unfamiliar coverage wording). An unrecognized shape blocks the gate on its own.
+   File an issue naming it and quoting the body, rather than guessing what the new wording
+   probably meant.
+5. The maintainer has given **explicit** permission to merge.
+
+The agent never merges on its own. A green or CLEAN PR with one open finding is not mergeable,
+full stop, whatever the merge-state field says.
+
+## Expected review loop
+
+1. Push changes to the PR branch.
+2. Re-request a review for the **current head SHA**. Auto-trigger is unreliable, so request it
+   explicitly (mechanics in the Copilot runbook). The UI is a fallback only.
+3. Wait for review activity on that head. A completed review raising **no findings** is a valid
+   terminal outcome, so don't re-trigger it or read silence as a missing review. A review whose
+   body says it declined to review is the one exception, and it is terminal the other way:
+   nothing follows it, and re-requesting the same head just repeats the decline.
+4. Triage findings (see below).
+5. Apply fixes or write a rationale for declines.
+6. Reply to each thread and resolve what was addressed.
+7. Re-run the loop after every fix push until no actionable finding remains.
+
+Drive to green, a review confirmed on the latest head SHA and every actionable finding closed,
+then apply the Merge Gate above. **Never exit the loop early.** A round count is not a stopping
+condition, and neither is patience running out.
+
+## Every finding ends in one of five outcomes
+
+1. **Real, so fix it.** Reply with the fixing commit SHA.
+2. **Not real, so disprove it in the thread**, with the command and its output, the code path
+   that makes it impossible, or the rule that governs it. An assertion ("this is fine") does not
+   close a finding, a decline needs evidence the reviewer itself could check.
+3. **Real, but deliberately not fixed, the maintainer's call, not the agent's.** State the
+   finding and why the fix is unwanted, and get an explicit answer. Never resolve this one by
+   silence.
+4. **Real and worth doing later, so file the issue first, then reply with its link.** A deferral
+   noted only in a thread is lost the moment the PR merges.
+5. **Keeps recurring, so fix the class, not the instance.** A finding raised repeatedly against
+   correct code means the code is not communicating something: add the comment, sharpen the name,
+   narrow the interface, or fix the rule if the rule is wrong. Bouncing the same point across
+   rounds is the signal to escalate the rule itself, not to keep re-arguing it.
+
+## Triaging findings
+
+**A low-confidence (suppressed) finding is not a low-value one.** Judge each against the code,
+never against its confidence label. Classify before responding:
+
+- **Bug**, wrong behavior, missing coverage, a real code or doc divergence. Fix it.
+- **Style or convention**. If the cited rule matches the existing tree, fix the code. If the rule
+  contradicts the tree or industry norm, **fix the rule, not the code**, and take it to the
+  maintainer (outcome 5) rather than bouncing the same code across rounds.
+- **Architectural opinion**, a proposed redesign. Surface it with a recommendation, never apply
+  it unilaterally.
+
+## Answering a suppressed finding
+
+A suppressed finding has no thread and no resolved or unresolved state, so an answer needs to
+carry its own context: quote the finding (with its `file:line` anchor and enough of the
+reviewer's own words to identify it), give one bold verdict per finding (`Fixed in <SHA>`,
+`Disproven`, or `No change needed`), state the `(N)` count the block gave so answers can be
+checked against findings, and link the review round. **Read every round, not only the head.** A
+suppressed finding does not retire when a later push supersedes it, it just stops showing up in a
+head-scoped query while still unanswered.
+
+## Escalate to the maintainer when
+
+- A genuine design trade-off surfaces (fail-open vs. fail-closed, refactor scope).
+- A finding keeps recurring. Bring the pattern and a recommended fix (rule change or code
+  change), don't keep silently re-declining it.
+- A finding is judged real but should not be fixed. That decision is never the agent's alone.
+- An architectural redesign is proposed rather than a bug fix.
+
+## Mechanics live elsewhere
+
+This skill is the provider-agnostic contract. For the actual GitHub API calls, requesting a
+Copilot review, polling for it, matching the suppressed-findings heading (its wording has moved
+more than once), verifying head-SHA and diff coverage, replying and resolving threads without a
+hand-typed id, see `.github/copilot-instructions.md` "GitHub Copilot Review Runbook" in the repo
+being worked in, and reach for `scripts/pr_review.py` (hub-hosted, invoked from a hub checkout)
+before hand-rolling any of it. `status` reports coverage, threads, suppressed findings, and shapes
+in one call, `wait` backs off in-process, and `reply` resolves a thread by matching the finding's
+own words rather than a line number a fix push can move.

.claude-plugin/fleet-skills/.claude-plugin/plugin.json

@@ -6,6 +6,8 @@
     "name": "ptr727"
   },
   "skills": [
-    "./skills/carried-instruction-file-guard"
+    "./skills/carried-instruction-file-guard",
+    "./skills/copilot-instructions-keeper",
+    "./skills/pr-review-conduct"
   ]
 }

.claude-plugin/fleet-skills/.source-digest

@@ -1 +1 @@
-948c01300ae03ac7
+7dbd5d23c3fd2a8d