Review-loop skills disagree with each other and stop short of decisions the loop needs

[ptr727]

The canonical-review gate added in #1148 forced the first whole-text read of the review-loop skills merged in #1109 and #1125, on the develop -> main promotion PR #1163. Eight units were read whole. The wrong-command and false-claim defects were fixed in that promotion's own feature PR. The items below are the ones that need a decision rather than a wording correction, so they are recorded here rather than guessed at.

The rules disagree with each other

• When the class sweep is owed. agent-conduct binds it on the first occurrence ("A review flags an instance, so fix the class: sweep for the siblings before replying, because reviewers sample rather than enumerate"), and GOVERNANCE.md "Verification Discipline" says the same. pr-review-conduct outcome 5 makes it conditional on a finding that "keeps recurring". An agent holding only the five outcomes fixes the instance and replies.
• What local-strict-review's pass binds. It says the pass runs "before the pull request opens" and, under outcome 1, "before any fix push". GOVERNANCE.md binds every push toward a pull request, and agent-conduct says "Every push toward a pull request owes one". A push answering outcome 4 or 5, a rebase, or a CI-only push is exempted in one place and not the other.

Rules that stop short of a decision

• A terminal review decline versus the Merge Gate. pr-review-conduct says a decline "is terminal the other way. Nothing follows it", while the Merge Gate requires a review confirmed on the current head SHA, which a terminal decline can then never satisfy. pr_review.py exit 46 carries the missing route (proceed on the coverage other reviewers give); the skill carries none.
• Who may resolve a declined thread. Outcome 2 says "Either shape closes the thread on its own evidence", while the loop's step 7 says "resolve what was addressed", and a decline is not addressed. Whether the agent may resolve its own evidence-backed decline is unstated, and it recurs every review round.
• Which repository outcome 4 files in. "File the issue first, then reply with its link" names no repository. For a finding against carried content the fix belongs at the hub, so followed literally in a carrier the deferral is filed where the fix cannot land.

Completeness

• agent-conduct "Before Claiming Done" lists 11 checks where the source section has about 20, and the dropped ones include two that fire at exactly that moment: "A workflow change is only fully exercised by CI" and "A test runner failing to spawn is not evidence that no test coverage applies here." The list reads as the complete claim-time set.
• GOVERNANCE.md "Durable Knowledge and Self-Improvement" omits GOVERNANCE.md and .agents/skills/ from where a durable rule belongs, listing AGENTS.md, CODESTYLE.md, WORKFLOW.md and a committed backlog. AGENTS.md designates GOVERNANCE.md as the file holding rule text and forbids restating rules in AGENTS.md, so following this bullet literally puts a new cross-cutting rule where AGENTS.md says it must not go.
• GOVERNANCE.md's upstream-proposal remedy names no upstream and no mechanism, and in the hub itself its precondition never holds, so the section's only remedy sentence is inert in the repository that owns it. Its own skill summary carries a stricter rule the section does not: search open and closed issues first, then update the matching issue or file a new one.

Permission surface

• drive-pr's Drive Loop prescribes opening a pull request, squash-merging it, and deleting a remote branch, three outward-facing and hard-to-reverse writes, with no permission gate stated in that section. The authorization lives in a separate section of the same skill. A reader who reaches the loop directly is told to perform all three unconditionally.

Worked examples a carrier cannot check

GOVERNANCE.md "Durable Knowledge and Self-Improvement" is carried verbatim and grounds its three-condition test on gh-write-guard and the hub's .husky/pre-push. A carrier holds neither artifact and gets no pointer to either, so the reader who most needs the worked example is the one who cannot check it.

Method note

Every item above was found by reading one unit's whole current text rather than a diff of it, which is what #1138 predicted and what the gate in #1148 now forces at the promotion. The eight units are recorded in reports/canonical-review.json with their finding counts.

[ptr727]

One more from the verification pass over the fixed text, same category as the rest here.

The gh-write-guard worked example claims decidability the hook itself declines. GOVERNANCE.md "Durable Knowledge and Self-Improvement" grounds its three-condition test on a worktree-isolation lapse, saying "is this command's target a primary checkout" is a plain directory comparison and so was promoted to a hook rule. host-setup/agent-safety/README.md says of that same requirement that the incident's own literal commands are deliberately exempt, and that "The concurrent-access hazard those commands still carried ... is not decidable from the command text alone, so it stays the prose rule's job." What was actually promoted is the blanket-add class, not the shape the worked example names. The section is carried verbatim, so every repository receives the example and none but the hub can check it against the hook.

[ptr727]

Two more from the second verification pass, both needing a decision rather than a wording fix.

A suppressed finding has no verdict for outcomes 3 and 4. pr-review-conduct "Answering a suppressed finding" says to "give one bold verdict per finding (`Fixed in `, `Disproven`, or `No change needed`)", presented as exhaustive. Those three cover outcomes 1 and 2 only. A suppressed finding deferred behind a filed issue (outcome 4) or put to the maintainer (outcome 3) has no verdict to give, and, having no thread, no place to carry the issue link instead.

local-strict-review's stated cross-checkout hazard is not the real one. "The Carried-Content Pass" says "Point an engine in one checkout at another checkout's tree and the second is measured with the first's unit model, while `record` stamps the ledger with a commit read from the second." Every command path resolves its root from the working directory (local_review.repo_root(), git rev-parse --show-toplevel); ROOT = Path(__file__) is used only for sys.path and two path constants. So the manifest, the tracked-file set and the ledger all come from the measured checkout. Pointed at a non-hub tree the engine refuses outright, exit 2, "this working tree holds no spec/files.json". The warning is worth keeping if there is a real hazard behind it, but as written it describes a mechanism the code does not have.

Also worth recording: the manifest carries no .husky/ entry in trees or baseline, and parity.hooks checks only .husky/pre-commit / .pre-commit-config.yaml. Four carried units now say a repository has no pre-push hook "unless it adds one", which is accurate, but nothing in the fleet mechanism would ever carry one, so whether the pre-push gate is meant to reach downstream at all is an open question.

[ptr727]

One policy question raised on #1165, recorded here rather than decided there.

Whether the agent should delete a remote feature branch at all. drive-pr's post-merge cleanup verifies the remote ref still matches the captured headRefOid, then deletes by name. That is a genuine time-of-check-to-time-of-use window: a concurrent push between the two operations would let the name-based delete remove a newer commit. The skill already names the trade and declines the atomic form, "Never --force-with-lease here, git-commit-conventions forbids it unconditionally, this plain verify-then-delete is the safety gate, not a compare-and-swap at delete time", and there is no other atomic option, since git push --delete matches by name and the REST ref delete takes no SHA precondition.

So the choice is between keeping the narrow window with the verify in front of it, or stopping after the merge and leaving the remote branch for an explicit maintainer operation. The second is what a reviewer proposed. Either is defensible and it is a fleet policy call rather than a wording fix, which is why it is here.

[ptr727]

Two more in GOVERNANCE.md "Durable Knowledge and Self-Improvement", from a canonical pass over that unit's post-merge text on PR #1157. Both are in the same bullet your "Completeness" section already covers, so they belong here rather than in a new issue.

The named fix cannot fix the named problem

A skill's own trigger going unread by the session at all, by contrast, is a loading problem, fixed by getting the rule into context (the CLAUDE.md importing AGENTS.md), not by a hook.

A CLAUDE.md that imports AGENTS.md loads the instruction files. It loads no Skill. A Skill's trigger goes unread when the machine's Skills install is missing or stale, which AGENTS.md states directly, "A rule that keeps needing to be restated is a sign the install is missing or stale, not that the rule does not exist", and which is fixed by running the hub's scripts/skills_install.py.

A carrier whose skill never fires follows this sentence, confirms its CLAUDE.md import is present and correct, concludes the rule is loaded, and never touches the actual stale install, so the skill still never fires. The bullet is the fleet's own worked example of when prose beats a hook, and its counterexample points at the wrong remedy.

"it was promoted" attaches to the lapse rather than the rule

A worktree-isolation lapse met all three (...), so it was promoted to a gh-write-guard hook rule.

The trailing "it" attaches to "A worktree-isolation lapse", the only available noun, but a lapse is not what a hook encodes. What was promoted is the worktree-isolation rule. The bullet's subject everywhere else is "a durable rule", so the one sentence a reader uses to calibrate the three conditions is the one where the promoted object is the failure rather than the rule, inviting the reading that a hook is built per incident rather than per rule.

Already settled, for the record

Your "Completeness" item about this bullet's list omitting GOVERNANCE.md is fixed on develop: #1163 wrote "GOVERNANCE.md for a cross-cutting rule, AGENTS.md, CODESTYLE.md, WORKFLOW.md, or a committed backlog the repository already keeps". PR #1157 hit the same omission independently and its merge kept your wording, which is the more precise of the two. The .agents/skills/ half of that item is untouched and is a model question rather than an omission, since the fleet's model is that GOVERNANCE.md holds rule text and Skills package it.