Route GitHub Writes Through Portable Tooling

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

@@ -124,7 +124,8 @@ reviewer's own words to identify it), give one bold verdict per finding (`Fixed
 `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.
+head-scoped query while still unanswered. Post the answer with `scripts/pr_review.py comment`
+from a hub checkout. Do not use a provider connector or reconstruct the GitHub mutation.
 
 ## Escalate to the maintainer when
 
@@ -138,7 +139,8 @@ head-scoped query while still unanswered.
 
 This skill is the provider-agnostic contract. Use `scripts/pr_review.py` from a hub checkout for
 the GitHub-specific API operations. `status` reports coverage, threads, body-only findings, and
-shapes in one call. `wait` requests and polls in-process. `reply` resolves a thread by matching
-the finding's own words instead of a line number a fix push can move. The repository's
+shapes in one call. `wait` requests and polls in-process. `comment` posts a PR-conversation
+answer after it reads the PR node ID. `reply` resolves a thread by matching the finding's own
+words instead of a line number a fix push can move. The repository's
 `.github/copilot-instructions.md` bootstraps Copilot into the `code-review` skill and its stable
 coverage marker. Do not reconstruct the API operations by hand.

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

@@ -1 +1 @@
-ac73ed965d3dfe48
+a4b2a2c434e2ab63

.claude-plugin/fleet-skills/skills/pr-review-conduct/SKILL.md

@@ -124,7 +124,8 @@ reviewer's own words to identify it), give one bold verdict per finding (`Fixed
 `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.
+head-scoped query while still unanswered. Post the answer with `scripts/pr_review.py comment`
+from a hub checkout. Do not use a provider connector or reconstruct the GitHub mutation.
 
 ## Escalate to the maintainer when
 
@@ -138,7 +139,8 @@ head-scoped query while still unanswered.
 
 This skill is the provider-agnostic contract. Use `scripts/pr_review.py` from a hub checkout for
 the GitHub-specific API operations. `status` reports coverage, threads, body-only findings, and
-shapes in one call. `wait` requests and polls in-process. `reply` resolves a thread by matching
-the finding's own words instead of a line number a fix push can move. The repository's
+shapes in one call. `wait` requests and polls in-process. `comment` posts a PR-conversation
+answer after it reads the PR node ID. `reply` resolves a thread by matching the finding's own
+words instead of a line number a fix push can move. The repository's
 `.github/copilot-instructions.md` bootstraps Copilot into the `code-review` skill and its stable
 coverage marker. Do not reconstruct the API operations by hand.

.github/copilot-instructions.md

@@ -38,9 +38,10 @@ For every review:
 5. End the review body with the exact machine-readable marker required by the `code-review` skill.
 
 The review automation is `scripts/pr_review.py`, run from a hub checkout. Use its `status`, `wait`,
-and `reply --resolve` commands instead of reconstructing GraphQL queries or copying review
-identifiers by hand. Its status gate verifies the current head, diff coverage, output shape,
-inline threads, body-only findings, and required checks.
+`comment`, and `reply --resolve` commands instead of reconstructing GraphQL queries or copying
+review identifiers by hand. Use `comment` for a suppressed-finding answer in the pull request
+conversation. Its status gate verifies the current head, diff coverage, output shape, inline
+threads, body-only findings, and required checks.
 
 A formal review with no findings is complete only when it covers the current head and states full
 diff coverage. A refusal, partial or absent coverage statement, unrecognized output shape,

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

@@ -124,7 +124,8 @@ reviewer's own words to identify it), give one bold verdict per finding (`Fixed
 `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.
+head-scoped query while still unanswered. Post the answer with `scripts/pr_review.py comment`
+from a hub checkout. Do not use a provider connector or reconstruct the GitHub mutation.
 
 ## Escalate to the maintainer when
 
@@ -138,7 +139,8 @@ head-scoped query while still unanswered.
 
 This skill is the provider-agnostic contract. Use `scripts/pr_review.py` from a hub checkout for
 the GitHub-specific API operations. `status` reports coverage, threads, body-only findings, and
-shapes in one call. `wait` requests and polls in-process. `reply` resolves a thread by matching
-the finding's own words instead of a line number a fix push can move. The repository's
+shapes in one call. `wait` requests and polls in-process. `comment` posts a PR-conversation
+answer after it reads the PR node ID. `reply` resolves a thread by matching the finding's own
+words instead of a line number a fix push can move. The repository's
 `.github/copilot-instructions.md` bootstraps Copilot into the `code-review` skill and its stable
 coverage marker. Do not reconstruct the API operations by hand.

GOVERNANCE.md

@@ -25,6 +25,7 @@ This section keeps the full rules and is surfaced at its decision moment by the
 A state-changing GitHub call is the highest-blast-radius thing an agent does here: it runs under the maintainer's identity, so one wrong target writes to another owner's repository as the maintainer, an outward-facing and hard-to-reverse act. These rules bound every write (a git push, an API mutation, a comment, a label, a merge) on any platform, and they bound a write to a checkout on disk as well, since a blanket add or a hard reset in a working tree another task is using destroys work without ever reaching GitHub. Reads are unrestricted, and how far a local read can be trusted is governed under "Verification Discipline" rather than here. The bounds below are on writes.
 
 - **Write only within the owner of the current project's repository.** Every state-changing call targets this project's `origin` or another repository under the same owner, which is the fleet the maintainer already administers. A broad or logged-in identity is capability, not permission: a token that *can* reach another owner's repository does not authorize writing to it. Writing under a **different owner** needs explicit human permission naming that repository, granted deliberately rather than assumed from a token's reach, and a "harmless test" write is still a write, so there is no probe exception. That boundary is where the harm sits, since the incident this rule exists for was a stray comment on a stranger's repository, not work across the maintainer's own projects. Reads from anywhere are fine.
+- **Provider connectors are read-only for fleet work.** Use a provider's GitHub connector for reads where it helps. Perform each GitHub mutation through the documented hub tool, or through authenticated `gh` where no tool owns the operation. This gives Codex, Claude, opencode, and a terminal session one write path with the same checks. It also avoids a connector mutation that predictably lacks repository authorization while the verified `gh` session already has it. A provider-specific instruction may explain how to reach the common path. It never replaces that path with its own mutation surface.
 - **Never fabricate, guess, or reuse an identifier passed to a write.** Every id a state-changing call consumes (a node id, a numeric id, a thread or comment id) is captured from a live query in the **same** session into a variable and passed from there. Do not hand-type an id, guess it, recall it from memory or an earlier session, or copy it from documentation or an example. Ids commonly resolve **globally**, so a wrong-but-valid id does not fail. It writes to the wrong target, in someone else's repository. If a query returns no id, stop rather than invent one to proceed.
 - **A write is never a probe, and a write's output is never suppressed.** Never fire a state-changing call to see whether it works: decide it should happen, make it happen, and read the result. Never append output-discarding redirection or a force-success tail to a mutation (for example `>/dev/null`, `2>/dev/null`, `&>/dev/null`, `|| true`, `|| :`, `|| echo`), because the write's output is exactly what must be read. A write that appears to fail is **verified, not assumed harmless**, because the operation may have succeeded on the server while the client reported an error, so confirm the actual state before retrying or moving on. The ban targets hiding a *failure*. An ad-hoc call's response is the only signal you get, so `>/dev/null 2>&1`, `|| true`, and `|| echo`, which swallow the error stream or force success, are never acceptable on one. A committed script under `set -e` is a narrow exception: it may send a write's *stdout* to `/dev/null` to drop the success-response noise, because stderr stays visible and a failed write still aborts loudly (the hub's own `repo-config/configure.sh` does exactly this, and a repository reaches it there rather than carrying a copy). The exception is stdout-only suppression inside a reviewed, fail-loud script, never `2>&1` or a force-success tail, and never an ad-hoc command.
 - **A refused write is reported, never re-shaped, and the maintainer's say-so does not lift a refusal by the harness.** These are two different permissions and only one of them is the maintainer's to give. When the agent harness refuses a write, the maintainer authorizing it in conversation does not change the outcome, and the identical call is refused again, so a second attempt is not worth making and reading the second refusal as a flake is how an agent starts hunting for another shape of the same request. **That hunt is the failure this rule exists to stop.** Re-expressing a refused `gh` command as a raw `gh api -X POST` reaches the same endpoint with the same identity and the same blast radius, having defeated the one control that stopped it, and it is the more dangerous version because the agent believes it has permission. So a refused write is never re-attempted through a different API surface, a different tool, or a rephrasing, and it is never routed around by the agent writing itself a permission rule, which is self-authorization whatever the maintainer said. Two routes remain, both of them the maintainer's: they add the permission rule themselves, or they run the command themselves. Raise it as a blocked decision naming those two (see "Communicating with the User"), and where the work needs the result rather than the call, say what the agent will verify once the maintainer has run it. **A refusal is also a fact about the contract, not just about the session**: where a required verification can only be performed by a write the agent is refused, the document requiring it says so and names who runs it, since a check that is mandatory and unperformable is quietly dropped and then reported as done.

scripts/README.md

@@ -131,12 +131,14 @@ A repository adds its own `host-tools.json` at its root and the gate layers it o
 
 ## `pr_review.py`
 
-One compact digest of a pull request's Copilot review state, replacing a sequence of one-`gh`-call-per-turn polls. `status` prints the digest, `wait` runs the backoff in-process so a long review wait costs one agent turn instead of one per poll, `reply` answers one thread and resolves it, and `claims` reads the description against the branch it describes. Re-requesting a review stays out and its runbook is in [`.github/copilot-instructions.md`][copilot-instructions].
+One compact interface to a pull request's Copilot review loop. `status` prints the digest, and `wait` runs the backoff in-process. `comment` posts a PR-conversation answer, while `reply` answers one thread and optionally resolves it. `claims` reads the description against the branch it describes. The runbook is in [`.github/copilot-instructions.md`][copilot-instructions].
 
 ```sh
 python3 scripts/pr_review.py status 452 --repo ptr727/ProjectTemplate
 python3 scripts/pr_review.py claims 452 --repo ptr727/ProjectTemplate
 python3 scripts/pr_review.py wait 452 --repo ptr727/ProjectTemplate --timeout 2700
+python3 scripts/pr_review.py comment 452 --repo ptr727/ProjectTemplate \
+  --body "Suppressed findings (1): **Disproven** - the target is checked before the write."
 python3 scripts/pr_review.py reply 452 --repo ptr727/ProjectTemplate \
   --match "retry count is off by one" --body "Fixed in abc1234: the loop now stops at n." --resolve
 ```
@@ -178,11 +180,13 @@ The timeout path prints the full digest for the same reason, as a bare `PENDING`
 
 The digest also reports the **suppressed findings** a review body collapses into a `<details>` block. Those reach no review thread, so a loop that polls threads alone reports a clean pass while they stand, and the [merge gate][governance] counts them as outstanding findings either way. `suppressed=N` counts findings rather than blocks, reading the `(N)` the heading carries, since one body holds one block per round and counting blocks reports two findings as one. It covers **every** round rather than the current head, because a suppressed finding has no resolved state for a push to retire: head-scoping read "superseded by a push" as "answered", and a finding nobody replied to left the digest the moment the branch moved, so the run reported zero. That is how four rounds went unanswered across three pull requests in one day, each found by the maintainer rather than by this script. The summary line splits the count as `suppressed=N (on_head=N earlier=N)` and each block is marked with the round that raised it, since a finding on an older round may since be moot and deciding that is the reader's call rather than one the count should make for them. Each block prints whole where a thread body truncates, because a thread can be re-read at its id and a suppressed finding cannot, and it prints under a marker naming what closing it takes: no thread exists to reply on or resolve, so the answer goes in the PR conversation.
 
+`comment` posts one answer in the pull request conversation. A suppressed finding has no thread, so this is the path that records its disposition. The command reads the pull request node ID in the same run and exposes no ID argument. It confirms the returned comment URL and exact body before it reports success. A missing target exits `65`, and an unconfirmed response exits `66`. An unconfirmed response may still have landed, so its output tells the reader to inspect the conversation before retrying.
+
 `reply` posts one answer and resolves one thread, and it exists because the hand-run form keeps failing the same way rather than because a wrapper is tidier. Three instances are on record, each an agent that had read the rule against hand-typing a node id and reached for the literal regardless, the last of them refused by the `gh-write-guard` hook an hour after quoting that same rule in a pull request body. A shape that fails while the reader knows the rule is a shape to remove rather than a rule to restate, so the selector is the finding's own words and there is deliberately no argument a `PRRT_...` id fits in: the id is read from the query in the same run and passed straight to the mutation, and a case asserts the id the mutation carries is the one that run just read. The words are matched against the thread's opening comment rather than against a line number, because a fix push moves the line and every lookup keyed to one then misses, which is how three replies posted against nothing while the resolves still succeeded and closed the threads carrying no answer. Matching is case-insensitive, since the text is quoted back out of a digest by a reader.
 
 Every failure is a stop rather than a fallback, because each alternative closes a finding while leaving it unanswered, which is the state a reviewer reads as addressed. No match exits `60` and prints the open threads, since a no-match and an already-answered thread read identically from here. Two matches exit `61` and print both candidates rather than taking the first, `head -n 1` being how a reply lands on the wrong finding. A reply whose response carries no `url`, or a body that came back empty, exits `62` **without** resolving. A resolve that does not report `isResolved` exits `63`, with the reply already posted, so the thread is open behind an answer rather than silently assumed closed. `--resolve` is opt-in rather than the default, because a decline is resolved only once its evidence is in the thread. A target under an owner other than this checkout's exits `64` before anything is read at all, and that refusal takes no environment-variable escape: a grant this process can be handed is one the caller sets on the command that runs it, and a grant the caller writes for itself is not a grant, so the cross-owner case goes through the runbook's explicit `gh` path where the hook reads the maintainer's grant from the session instead.
 
-What this trades away is stated rather than glossed. A mutation spelled as a `gh` command in a shell is read by the `gh-write-guard` hook and one this script performs is not, since the hook sees `python3 pr_review.py reply` and no `gh` write. That is a real loss of a second pair of eyes, and it is taken because what the hook guards against there is a fabricated id, which this removes at the source instead of catching after the fact. The guard's other rule is re-implemented here rather than assumed: the owner check above is the same scope rule, enforced in-process, and it is honest that it stops a mistake rather than a determined caller. The whole-source guard against every other state-changing call stays and was narrowed to these two documents rather than dropped when the first of them arrived.
+What this trades away is stated rather than glossed. A mutation spelled as a `gh` command in a shell is read by the `gh-write-guard` hook and one this script performs is not. The script removes that failure at its source instead: every node ID comes from a live query in the same run. The owner check is also enforced in-process. The whole-source test guards against every other state-changing call and counts each reviewed mutation document.
 
 `claims` checks that a description does not contradict its own branch, and it exists for the same reason `sha-pin` now resolves: a reference that points at nothing is a silent failure caught by a reviewer or not at all. Three stale descriptions in one session generated six review findings between them, each a reviewer noticing that the body named a commit or a behavior the branch no longer carried. It reads the commits a body **claims** the branch carries and the `uses:` refs it quotes, confirms the head tree still carries each, and exits `70` where one does not. Prose claims stay **out of scope**, since judging those needs a similarity heuristic, which [`spec/section-model.md`][section-model] rejects for exactly the reason it would fail here.