From aa468cbb67f7ea4213dd01e698da7285ab26ea14 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Mon, 31 Aug 2026 07:46:49 -0700 Subject: [PATCH 1/7] Review Hub Canonical Content Here, Before a Carrier Is the One to Read It Hub-owned content under .agents/skills/, GOVERNANCE.md, WORKFLOW.md, AGENTS.md and AUDIT.md is written and merged here against a diff of a few lines, and only ever reaches a reviewer as a new file, in full, when a downstream repository carries it for the first time. So the first real read of a rule happens in the one repository that cannot act on it: the tree is manifest-owned, the copy is digest-compared, and a local edit there is drift on the next fidelity check. Every carrier after that re-discovers the same defect, and the finding arrives in a session holding no hub checkout and no standing to test the claim. scripts/canonical_review.py reproduces that read here. A unit is what a reviewer reads whole, one level-two section of a Markdown canonical and one file otherwise, matching the fidelity unit spec/section-model.md declares and the unit key spec/divergences.json already uses. A file the manifest carries by named sections contributes exactly those, so a section this hub keeps for itself is not a unit. Coverage is keyed on content rather than on a commit, so editing a unit retires its pass and editing its neighbor does not. The gate is on what a branch changes. check refuses only the units this branch's own diff moved, measured from the merge-base, so the ordering is fixed going forward without blocking unrelated work on the 283 units nothing has read here yet. Those are a burn-down in reports/canonical-review.md, the way reports/divergences.md carries fidelity. .husky/pre-push runs the check beside its sibling, and the hub's own validate hook runs it on every pull request, which is where it binds, since a push can bypass a hook. The six units this change itself moves carry recorded passes, which raised eleven findings, all fixed here. Closes #1138 --- .agents/skills/local-strict-review/SKILL.md | 50 +- .claude-plugin/fleet-skills/.source-digest | 2 +- .../skills/local-strict-review/SKILL.md | 50 +- .github/actions/validate/action.yml | 11 + .github/skills/local-strict-review/SKILL.md | 50 +- .husky/pre-push | 36 +- AGENTS.md | 3 +- GOVERNANCE.md | 3 +- OPERATIONS.md | 3 + reports/canonical-review.json | 53 ++ reports/canonical-review.md | 459 +++++++++++ scripts/README.md | 32 +- scripts/canonical_review.py | 775 ++++++++++++++++++ scripts/tests/test_canonical_review.py | 553 +++++++++++++ 14 files changed, 2048 insertions(+), 32 deletions(-) create mode 100644 reports/canonical-review.json create mode 100644 reports/canonical-review.md create mode 100755 scripts/canonical_review.py create mode 100755 scripts/tests/test_canonical_review.py diff --git a/.agents/skills/local-strict-review/SKILL.md b/.agents/skills/local-strict-review/SKILL.md index 15797d52..1c4d3e55 100644 --- a/.agents/skills/local-strict-review/SKILL.md +++ b/.agents/skills/local-strict-review/SKILL.md @@ -1,7 +1,7 @@ --- name: local-strict-review description: >- - Runs one read-only, adversarial review pass against this branch's current diff against its + Runs a read-only, adversarial review pass against this branch's current diff against its target branch, full file context included, on the strongest model tier the session can reach, before a unit of work is pushed toward a pull request or claimed done. Use this whenever staged, committed, or untracked work is about to be pushed on a PR-bound branch, and whenever @@ -11,7 +11,11 @@ description: >- the exact gap this skill exists to close before a PR-hosted reviewer closes it instead. Reuses `code-review`'s "Review the Change" criteria rather than restating them, and owns only this local, pre-PR moment. Once a pull request exists, `pr-review-conduct` and `drive-pr` own - triaging and disposing of what a PR-hosted reviewer finds. + triaging and disposing of what a PR-hosted reviewer finds. Also triggers whenever a change + edits rule text, a Skill, or any other canonical content this repository authors and other + repositories carry, because that content reaches a reviewer whole only when a repository + carries it for the first time, and a second pass reading each changed unit's whole text is + what moves that read into the repository that can act on what it finds. --- # Local Strict Review @@ -78,6 +82,41 @@ Record the pass whatever it found, including nothing. The key covers the net con The engine is hub-hosted per `GOVERNANCE.md` "Hub-Hosted Tooling", so a downstream repository reaches a hub checkout's copy rather than carrying one, which is what the path above is for. +## The Carried-Content Pass + +A second pass under the same rule, run in the repository that authors canonical content other repositories carry, which in this fleet is the hub. It answers an ordering defect rather than a quality one. Such content is written and merged against a diff of a few lines, and reaches a reviewer as a new file, in full, only when a repository carries it for the first time, so the first real read of a rule happens where nothing can be done about the result: the tree is manifest-owned, the copy is digest-compared, and a local edit there is drift on the next fidelity check. Every carrier after that re-discovers the same defect, and the finding arrives in a session holding no checkout of the authoring repository and no standing to test the claim. + +**The unit is what a reviewer reads whole**, one level-two section of a Markdown canonical and one file otherwise. The pass reads that unit's whole current text rather than the diff that moved it, because reproducing the carrier's read is the entire point, and a diff with surrounding context is a different read that has already been done by the pass above. + +Run it at the same model tier and in the same delegation shape, changing only the brief: + +```text +Task: adversarial review of one canonical unit, read as a repository carrying it for the first + time reads it, whole, knowing nothing about what this branch changed in it. +Paths: , read in full out of the file that key names. + Read the whole unit, never a diff of it. +Rules that bind this task: , and judge the text + as a reader who has only this unit: a claim it makes about a tool, a path, a command, or + another rule is a defect wherever that claim is false, stale, or unverifiable from the unit + itself, and an instruction it gives is a defect wherever following it literally fails. +Return: one finding per line, the sentence quoted, and what is wrong with it. No severity theater. +Bounds: read-only. Report a rule that looks incomplete rather than guessing at what it meant. + +``` + +```sh +engine="/scripts/canonical_review.py" # in the hub itself, scripts/canonical_review.py +python3 "$engine" check # each uncovered unit, with the digest to hand back +# run the pass above over each unit it named, then, per unit: +python3 "$engine" record --reviewer agent-skill --unit '=' [--findings N] +``` + +Both commands run with the authoring repository as the working directory, which is the hub checkout the path above already points into, since the engine takes no `--repo` and reads whichever repository it is run in. `check` names each uncovered unit with the digest to hand back, so nothing has to be looked up separately, and `list` is there for reading the whole set rather than for this loop. + +The digest is bound to the read for the same reason `--expect-digest` is above: recording a unit by name alone would stamp whatever the file holds at record time, so an edit between the review and the record would be attested to by a reviewer who never saw it. Record each unit whatever the pass found, including nothing. + +**A unit nothing has read here yet is not this branch's debt.** `check` refuses only the units this branch's own diff moved, and the hub's `reports/canonical-review.md` carries the rest as a burn-down. Working one of those off is worthwhile, and it is its own change rather than a tax on an unrelated one. + ## Disposing of Findings Every finding maps to one of `pr-review-conduct`'s five outcomes before the pull request opens: fixed, evidence-disproven, filed as a deferred issue, escalated to the maintainer for an explicit call, or, if it keeps recurring, taken as a signal to fix the class. A finding this pass raised and not fixed is never the agent's own call to just leave. Per outcome 3, that decision needs the maintainer's explicit answer, the same way a PR-hosted finding would. Running this pass is required before every push toward a pull request, per `agent-conduct`. Two claims sit next to each other here and they point opposite ways, so they are stated apart rather than in one sentence. **The pass is mandatory**, and where a capture point enforces it, a push carrying content no recorded pass covers is refused. That refusal is the gate working rather than a fault to route around. **The findings stay advisory**, and the count a pass raises gates nothing at all, since a pass records that a review ran and never that the content is clean. The disposition above is what closes each finding, the same posture local lint holds today. It posts nothing to GitHub, it only reports to the session driving the work. A finding raised here and not fixed is not thereby resolved: the same finding shape reaching a PR-hosted reviewer later still gets its own fresh disposition, per `pr-review-conduct`'s "a disposition decided on one PR does not carry to the next." @@ -87,8 +126,9 @@ Every finding maps to one of `pr-review-conduct`'s five outcomes before the pull - Before the first push toward a pull request (`drive-pr`'s Drive Loop step 2, `pr-review-conduct`'s Expected review loop step 1). - Before pushing a fix for a reviewer finding, the same self-review blind spot applies to a fix as to the original diff (`drive-pr`'s "Disposing of Every Finding", `pr-review-conduct`'s outcome 1). - Whenever `agent-conduct`'s "about to claim work is done, verified, green, or fixed" trigger fires for work that will become, or already is, a pull request. +- Before pushing a change that edits canonical content other repositories carry, over each unit that change moved, per "The Carried-Content Pass" above. -This repository's `.husky/pre-push` hook checks the receipt at the push itself, so the moments above are where the pass is run rather than the only places it is noticed. A blocked push usually means one of them was skipped. The hook is a backstop under this skill and not a replacement for it: it fires only in a clone that enabled `core.hooksPath`, it says nothing about a repository that carries no such hook, and it is bypassable by design, `--no-verify` being the documented route for a genuine pickle rather than for a diff nobody read. +This repository's `.husky/pre-push` hook checks the receipt, and the canonical-unit coverage beside it, at the push itself, so the moments above are where each pass is run rather than the only places it is noticed. The hub's own `.github/actions/validate` hook runs the canonical-unit half again on every pull request, which is the one capture point a push cannot bypass. A blocked push usually means one of them was skipped. The pre-push hook is a backstop under this skill and not a replacement for it, the pull request one being the layer that does not depend on the skill having been read: it fires only in a clone that enabled `core.hooksPath`, it says nothing about a repository that carries no such hook, and it is bypassable by design, `--no-verify` being the documented route for a genuine pickle rather than for a diff nobody read. **Not every refusal is a missing pass, and re-running the pass at one of these does nothing.** Read the refusal itself, which names its own case. Some of these the hook decides before the engine runs, so there is no engine message under them, and the rows below say where each one's detail comes from. @@ -97,9 +137,10 @@ This repository's `.husky/pre-push` hook checks the receipt at the push itself, | Tracked content differs from HEAD | A push delivers HEAD while a receipt covers the index and working tree, so the receipt does not describe this push | Commit what is being pushed, then the pass, then the record | | The commit is not this worktree's HEAD | `git push origin some-other-branch` from a checkout sitting elsewhere, and the engine reads the checkout it runs in | Check the pushed branch out in its own worktree, per `repo-worktree` | | Any wording saying the gate did not or could not run | An execution boundary rather than a verdict, which blocks because a gate that waves a push through when it could not run has stopped gating. The cause is named in that same message or in the engine error printed above it, and it is a missing Python interpreter, an unresolvable target, an unreadable receipt, a git command that failed, or any unexpected failure | Whatever the message names, most often installing an interpreter per `docs/host-setup.md` or fetching the target branch. Never another pass | +| This branch changes N carried canonical unit(s) that no recorded pass covers | The carried-content pass was skipped for a unit this change moved, and the refusal names each one with the digest to hand back | One carried-content pass per named unit, then `canonical_review.py record` for each | | A recorded pass names a branch the check did not measure | The hook reads `develop` and nothing else, so a branch based elsewhere is measured against `develop` whatever the pass targeted, and the engine deliberately prints no record command, since the one it would print records a pass over a diff nobody read | One more pass against the branch this work actually targets, where it does target the measured one. Where it does not, the gate cannot judge the branch at all and the bypass is its answer | -This table is the fleet's one enumeration of these, and every other surface states the principle and routes here rather than listing shapes or counting them. That is deliberate: through this skill's own review the count went from two to four, and every round left at least one restatement behind. +This table is the fleet's one enumeration of these, and every other surface states the principle and routes here rather than listing shapes or counting them. That is deliberate: through this skill's own review the count went from two to four, and it has grown again since, with every round leaving at least one restatement behind. ## Mechanics Live Elsewhere @@ -108,3 +149,4 @@ This table is the fleet's one enumeration of these, and every other surface stat - Branch base rule (`develop` unless the task is explicitly `main`-only): `repo-worktree`. - Finding disposition once a pull request exists, the Merge Gate, `scripts/pr_review.py`: `pr-review-conduct`, `drive-pr`. - The receipt's key, its backends, and the three-valued exit contract a capture point folds: `scripts/README.md` "`local_review.py`". +- The unit model, the coverage ledger, and the burn-down report: `scripts/README.md` "`canonical_review.py`". diff --git a/.claude-plugin/fleet-skills/.source-digest b/.claude-plugin/fleet-skills/.source-digest index 398a17a6..c632f9a7 100644 --- a/.claude-plugin/fleet-skills/.source-digest +++ b/.claude-plugin/fleet-skills/.source-digest @@ -1 +1 @@ -438bd561ef5f233b +937ab0ace9632ce2 diff --git a/.claude-plugin/fleet-skills/skills/local-strict-review/SKILL.md b/.claude-plugin/fleet-skills/skills/local-strict-review/SKILL.md index 15797d52..1c4d3e55 100644 --- a/.claude-plugin/fleet-skills/skills/local-strict-review/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/local-strict-review/SKILL.md @@ -1,7 +1,7 @@ --- name: local-strict-review description: >- - Runs one read-only, adversarial review pass against this branch's current diff against its + Runs a read-only, adversarial review pass against this branch's current diff against its target branch, full file context included, on the strongest model tier the session can reach, before a unit of work is pushed toward a pull request or claimed done. Use this whenever staged, committed, or untracked work is about to be pushed on a PR-bound branch, and whenever @@ -11,7 +11,11 @@ description: >- the exact gap this skill exists to close before a PR-hosted reviewer closes it instead. Reuses `code-review`'s "Review the Change" criteria rather than restating them, and owns only this local, pre-PR moment. Once a pull request exists, `pr-review-conduct` and `drive-pr` own - triaging and disposing of what a PR-hosted reviewer finds. + triaging and disposing of what a PR-hosted reviewer finds. Also triggers whenever a change + edits rule text, a Skill, or any other canonical content this repository authors and other + repositories carry, because that content reaches a reviewer whole only when a repository + carries it for the first time, and a second pass reading each changed unit's whole text is + what moves that read into the repository that can act on what it finds. --- # Local Strict Review @@ -78,6 +82,41 @@ Record the pass whatever it found, including nothing. The key covers the net con The engine is hub-hosted per `GOVERNANCE.md` "Hub-Hosted Tooling", so a downstream repository reaches a hub checkout's copy rather than carrying one, which is what the path above is for. +## The Carried-Content Pass + +A second pass under the same rule, run in the repository that authors canonical content other repositories carry, which in this fleet is the hub. It answers an ordering defect rather than a quality one. Such content is written and merged against a diff of a few lines, and reaches a reviewer as a new file, in full, only when a repository carries it for the first time, so the first real read of a rule happens where nothing can be done about the result: the tree is manifest-owned, the copy is digest-compared, and a local edit there is drift on the next fidelity check. Every carrier after that re-discovers the same defect, and the finding arrives in a session holding no checkout of the authoring repository and no standing to test the claim. + +**The unit is what a reviewer reads whole**, one level-two section of a Markdown canonical and one file otherwise. The pass reads that unit's whole current text rather than the diff that moved it, because reproducing the carrier's read is the entire point, and a diff with surrounding context is a different read that has already been done by the pass above. + +Run it at the same model tier and in the same delegation shape, changing only the brief: + +```text +Task: adversarial review of one canonical unit, read as a repository carrying it for the first + time reads it, whole, knowing nothing about what this branch changed in it. +Paths: , read in full out of the file that key names. + Read the whole unit, never a diff of it. +Rules that bind this task: , and judge the text + as a reader who has only this unit: a claim it makes about a tool, a path, a command, or + another rule is a defect wherever that claim is false, stale, or unverifiable from the unit + itself, and an instruction it gives is a defect wherever following it literally fails. +Return: one finding per line, the sentence quoted, and what is wrong with it. No severity theater. +Bounds: read-only. Report a rule that looks incomplete rather than guessing at what it meant. + +``` + +```sh +engine="/scripts/canonical_review.py" # in the hub itself, scripts/canonical_review.py +python3 "$engine" check # each uncovered unit, with the digest to hand back +# run the pass above over each unit it named, then, per unit: +python3 "$engine" record --reviewer agent-skill --unit '=' [--findings N] +``` + +Both commands run with the authoring repository as the working directory, which is the hub checkout the path above already points into, since the engine takes no `--repo` and reads whichever repository it is run in. `check` names each uncovered unit with the digest to hand back, so nothing has to be looked up separately, and `list` is there for reading the whole set rather than for this loop. + +The digest is bound to the read for the same reason `--expect-digest` is above: recording a unit by name alone would stamp whatever the file holds at record time, so an edit between the review and the record would be attested to by a reviewer who never saw it. Record each unit whatever the pass found, including nothing. + +**A unit nothing has read here yet is not this branch's debt.** `check` refuses only the units this branch's own diff moved, and the hub's `reports/canonical-review.md` carries the rest as a burn-down. Working one of those off is worthwhile, and it is its own change rather than a tax on an unrelated one. + ## Disposing of Findings Every finding maps to one of `pr-review-conduct`'s five outcomes before the pull request opens: fixed, evidence-disproven, filed as a deferred issue, escalated to the maintainer for an explicit call, or, if it keeps recurring, taken as a signal to fix the class. A finding this pass raised and not fixed is never the agent's own call to just leave. Per outcome 3, that decision needs the maintainer's explicit answer, the same way a PR-hosted finding would. Running this pass is required before every push toward a pull request, per `agent-conduct`. Two claims sit next to each other here and they point opposite ways, so they are stated apart rather than in one sentence. **The pass is mandatory**, and where a capture point enforces it, a push carrying content no recorded pass covers is refused. That refusal is the gate working rather than a fault to route around. **The findings stay advisory**, and the count a pass raises gates nothing at all, since a pass records that a review ran and never that the content is clean. The disposition above is what closes each finding, the same posture local lint holds today. It posts nothing to GitHub, it only reports to the session driving the work. A finding raised here and not fixed is not thereby resolved: the same finding shape reaching a PR-hosted reviewer later still gets its own fresh disposition, per `pr-review-conduct`'s "a disposition decided on one PR does not carry to the next." @@ -87,8 +126,9 @@ Every finding maps to one of `pr-review-conduct`'s five outcomes before the pull - Before the first push toward a pull request (`drive-pr`'s Drive Loop step 2, `pr-review-conduct`'s Expected review loop step 1). - Before pushing a fix for a reviewer finding, the same self-review blind spot applies to a fix as to the original diff (`drive-pr`'s "Disposing of Every Finding", `pr-review-conduct`'s outcome 1). - Whenever `agent-conduct`'s "about to claim work is done, verified, green, or fixed" trigger fires for work that will become, or already is, a pull request. +- Before pushing a change that edits canonical content other repositories carry, over each unit that change moved, per "The Carried-Content Pass" above. -This repository's `.husky/pre-push` hook checks the receipt at the push itself, so the moments above are where the pass is run rather than the only places it is noticed. A blocked push usually means one of them was skipped. The hook is a backstop under this skill and not a replacement for it: it fires only in a clone that enabled `core.hooksPath`, it says nothing about a repository that carries no such hook, and it is bypassable by design, `--no-verify` being the documented route for a genuine pickle rather than for a diff nobody read. +This repository's `.husky/pre-push` hook checks the receipt, and the canonical-unit coverage beside it, at the push itself, so the moments above are where each pass is run rather than the only places it is noticed. The hub's own `.github/actions/validate` hook runs the canonical-unit half again on every pull request, which is the one capture point a push cannot bypass. A blocked push usually means one of them was skipped. The pre-push hook is a backstop under this skill and not a replacement for it, the pull request one being the layer that does not depend on the skill having been read: it fires only in a clone that enabled `core.hooksPath`, it says nothing about a repository that carries no such hook, and it is bypassable by design, `--no-verify` being the documented route for a genuine pickle rather than for a diff nobody read. **Not every refusal is a missing pass, and re-running the pass at one of these does nothing.** Read the refusal itself, which names its own case. Some of these the hook decides before the engine runs, so there is no engine message under them, and the rows below say where each one's detail comes from. @@ -97,9 +137,10 @@ This repository's `.husky/pre-push` hook checks the receipt at the push itself, | Tracked content differs from HEAD | A push delivers HEAD while a receipt covers the index and working tree, so the receipt does not describe this push | Commit what is being pushed, then the pass, then the record | | The commit is not this worktree's HEAD | `git push origin some-other-branch` from a checkout sitting elsewhere, and the engine reads the checkout it runs in | Check the pushed branch out in its own worktree, per `repo-worktree` | | Any wording saying the gate did not or could not run | An execution boundary rather than a verdict, which blocks because a gate that waves a push through when it could not run has stopped gating. The cause is named in that same message or in the engine error printed above it, and it is a missing Python interpreter, an unresolvable target, an unreadable receipt, a git command that failed, or any unexpected failure | Whatever the message names, most often installing an interpreter per `docs/host-setup.md` or fetching the target branch. Never another pass | +| This branch changes N carried canonical unit(s) that no recorded pass covers | The carried-content pass was skipped for a unit this change moved, and the refusal names each one with the digest to hand back | One carried-content pass per named unit, then `canonical_review.py record` for each | | A recorded pass names a branch the check did not measure | The hook reads `develop` and nothing else, so a branch based elsewhere is measured against `develop` whatever the pass targeted, and the engine deliberately prints no record command, since the one it would print records a pass over a diff nobody read | One more pass against the branch this work actually targets, where it does target the measured one. Where it does not, the gate cannot judge the branch at all and the bypass is its answer | -This table is the fleet's one enumeration of these, and every other surface states the principle and routes here rather than listing shapes or counting them. That is deliberate: through this skill's own review the count went from two to four, and every round left at least one restatement behind. +This table is the fleet's one enumeration of these, and every other surface states the principle and routes here rather than listing shapes or counting them. That is deliberate: through this skill's own review the count went from two to four, and it has grown again since, with every round leaving at least one restatement behind. ## Mechanics Live Elsewhere @@ -108,3 +149,4 @@ This table is the fleet's one enumeration of these, and every other surface stat - Branch base rule (`develop` unless the task is explicitly `main`-only): `repo-worktree`. - Finding disposition once a pull request exists, the Merge Gate, `scripts/pr_review.py`: `pr-review-conduct`, `drive-pr`. - The receipt's key, its backends, and the three-valued exit contract a capture point folds: `scripts/README.md` "`local_review.py`". +- The unit model, the coverage ledger, and the burn-down report: `scripts/README.md` "`canonical_review.py`". diff --git a/.github/actions/validate/action.yml b/.github/actions/validate/action.yml index f7792dad..1e5d4959 100644 --- a/.github/actions/validate/action.yml +++ b/.github/actions/validate/action.yml @@ -42,6 +42,17 @@ runs: shell: bash run: python3 scripts/build_dist.py --check + # The carried-content half of the local-review rule, per GOVERNANCE.md "Verification Discipline". + # The .husky/pre-push hook runs the same check and is bypassable by construction, so this is where it actually binds. + # Pull requests only, since a canonical unit's change is measured against the branch it is proposed into and a push carrying no pull request has none. + # The base commit reaches the script through the environment rather than through an expression inside it, so a ref value can never be read as shell. + - name: Check carried canonical content review coverage step + if: github.event.pull_request != null + shell: bash + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + run: python3 scripts/canonical_review.py check --target "$BASE_SHA" + # Warn-only, and visible rather than absent: an unrun check is one nobody acts on. # A finding here names a character no tier covers, and classifying it is a fleet-law edit rather than a prose fix. - name: Report unclassified characters step diff --git a/.github/skills/local-strict-review/SKILL.md b/.github/skills/local-strict-review/SKILL.md index 15797d52..1c4d3e55 100644 --- a/.github/skills/local-strict-review/SKILL.md +++ b/.github/skills/local-strict-review/SKILL.md @@ -1,7 +1,7 @@ --- name: local-strict-review description: >- - Runs one read-only, adversarial review pass against this branch's current diff against its + Runs a read-only, adversarial review pass against this branch's current diff against its target branch, full file context included, on the strongest model tier the session can reach, before a unit of work is pushed toward a pull request or claimed done. Use this whenever staged, committed, or untracked work is about to be pushed on a PR-bound branch, and whenever @@ -11,7 +11,11 @@ description: >- the exact gap this skill exists to close before a PR-hosted reviewer closes it instead. Reuses `code-review`'s "Review the Change" criteria rather than restating them, and owns only this local, pre-PR moment. Once a pull request exists, `pr-review-conduct` and `drive-pr` own - triaging and disposing of what a PR-hosted reviewer finds. + triaging and disposing of what a PR-hosted reviewer finds. Also triggers whenever a change + edits rule text, a Skill, or any other canonical content this repository authors and other + repositories carry, because that content reaches a reviewer whole only when a repository + carries it for the first time, and a second pass reading each changed unit's whole text is + what moves that read into the repository that can act on what it finds. --- # Local Strict Review @@ -78,6 +82,41 @@ Record the pass whatever it found, including nothing. The key covers the net con The engine is hub-hosted per `GOVERNANCE.md` "Hub-Hosted Tooling", so a downstream repository reaches a hub checkout's copy rather than carrying one, which is what the path above is for. +## The Carried-Content Pass + +A second pass under the same rule, run in the repository that authors canonical content other repositories carry, which in this fleet is the hub. It answers an ordering defect rather than a quality one. Such content is written and merged against a diff of a few lines, and reaches a reviewer as a new file, in full, only when a repository carries it for the first time, so the first real read of a rule happens where nothing can be done about the result: the tree is manifest-owned, the copy is digest-compared, and a local edit there is drift on the next fidelity check. Every carrier after that re-discovers the same defect, and the finding arrives in a session holding no checkout of the authoring repository and no standing to test the claim. + +**The unit is what a reviewer reads whole**, one level-two section of a Markdown canonical and one file otherwise. The pass reads that unit's whole current text rather than the diff that moved it, because reproducing the carrier's read is the entire point, and a diff with surrounding context is a different read that has already been done by the pass above. + +Run it at the same model tier and in the same delegation shape, changing only the brief: + +```text +Task: adversarial review of one canonical unit, read as a repository carrying it for the first + time reads it, whole, knowing nothing about what this branch changed in it. +Paths: , read in full out of the file that key names. + Read the whole unit, never a diff of it. +Rules that bind this task: , and judge the text + as a reader who has only this unit: a claim it makes about a tool, a path, a command, or + another rule is a defect wherever that claim is false, stale, or unverifiable from the unit + itself, and an instruction it gives is a defect wherever following it literally fails. +Return: one finding per line, the sentence quoted, and what is wrong with it. No severity theater. +Bounds: read-only. Report a rule that looks incomplete rather than guessing at what it meant. + +``` + +```sh +engine="/scripts/canonical_review.py" # in the hub itself, scripts/canonical_review.py +python3 "$engine" check # each uncovered unit, with the digest to hand back +# run the pass above over each unit it named, then, per unit: +python3 "$engine" record --reviewer agent-skill --unit '=' [--findings N] +``` + +Both commands run with the authoring repository as the working directory, which is the hub checkout the path above already points into, since the engine takes no `--repo` and reads whichever repository it is run in. `check` names each uncovered unit with the digest to hand back, so nothing has to be looked up separately, and `list` is there for reading the whole set rather than for this loop. + +The digest is bound to the read for the same reason `--expect-digest` is above: recording a unit by name alone would stamp whatever the file holds at record time, so an edit between the review and the record would be attested to by a reviewer who never saw it. Record each unit whatever the pass found, including nothing. + +**A unit nothing has read here yet is not this branch's debt.** `check` refuses only the units this branch's own diff moved, and the hub's `reports/canonical-review.md` carries the rest as a burn-down. Working one of those off is worthwhile, and it is its own change rather than a tax on an unrelated one. + ## Disposing of Findings Every finding maps to one of `pr-review-conduct`'s five outcomes before the pull request opens: fixed, evidence-disproven, filed as a deferred issue, escalated to the maintainer for an explicit call, or, if it keeps recurring, taken as a signal to fix the class. A finding this pass raised and not fixed is never the agent's own call to just leave. Per outcome 3, that decision needs the maintainer's explicit answer, the same way a PR-hosted finding would. Running this pass is required before every push toward a pull request, per `agent-conduct`. Two claims sit next to each other here and they point opposite ways, so they are stated apart rather than in one sentence. **The pass is mandatory**, and where a capture point enforces it, a push carrying content no recorded pass covers is refused. That refusal is the gate working rather than a fault to route around. **The findings stay advisory**, and the count a pass raises gates nothing at all, since a pass records that a review ran and never that the content is clean. The disposition above is what closes each finding, the same posture local lint holds today. It posts nothing to GitHub, it only reports to the session driving the work. A finding raised here and not fixed is not thereby resolved: the same finding shape reaching a PR-hosted reviewer later still gets its own fresh disposition, per `pr-review-conduct`'s "a disposition decided on one PR does not carry to the next." @@ -87,8 +126,9 @@ Every finding maps to one of `pr-review-conduct`'s five outcomes before the pull - Before the first push toward a pull request (`drive-pr`'s Drive Loop step 2, `pr-review-conduct`'s Expected review loop step 1). - Before pushing a fix for a reviewer finding, the same self-review blind spot applies to a fix as to the original diff (`drive-pr`'s "Disposing of Every Finding", `pr-review-conduct`'s outcome 1). - Whenever `agent-conduct`'s "about to claim work is done, verified, green, or fixed" trigger fires for work that will become, or already is, a pull request. +- Before pushing a change that edits canonical content other repositories carry, over each unit that change moved, per "The Carried-Content Pass" above. -This repository's `.husky/pre-push` hook checks the receipt at the push itself, so the moments above are where the pass is run rather than the only places it is noticed. A blocked push usually means one of them was skipped. The hook is a backstop under this skill and not a replacement for it: it fires only in a clone that enabled `core.hooksPath`, it says nothing about a repository that carries no such hook, and it is bypassable by design, `--no-verify` being the documented route for a genuine pickle rather than for a diff nobody read. +This repository's `.husky/pre-push` hook checks the receipt, and the canonical-unit coverage beside it, at the push itself, so the moments above are where each pass is run rather than the only places it is noticed. The hub's own `.github/actions/validate` hook runs the canonical-unit half again on every pull request, which is the one capture point a push cannot bypass. A blocked push usually means one of them was skipped. The pre-push hook is a backstop under this skill and not a replacement for it, the pull request one being the layer that does not depend on the skill having been read: it fires only in a clone that enabled `core.hooksPath`, it says nothing about a repository that carries no such hook, and it is bypassable by design, `--no-verify` being the documented route for a genuine pickle rather than for a diff nobody read. **Not every refusal is a missing pass, and re-running the pass at one of these does nothing.** Read the refusal itself, which names its own case. Some of these the hook decides before the engine runs, so there is no engine message under them, and the rows below say where each one's detail comes from. @@ -97,9 +137,10 @@ This repository's `.husky/pre-push` hook checks the receipt at the push itself, | Tracked content differs from HEAD | A push delivers HEAD while a receipt covers the index and working tree, so the receipt does not describe this push | Commit what is being pushed, then the pass, then the record | | The commit is not this worktree's HEAD | `git push origin some-other-branch` from a checkout sitting elsewhere, and the engine reads the checkout it runs in | Check the pushed branch out in its own worktree, per `repo-worktree` | | Any wording saying the gate did not or could not run | An execution boundary rather than a verdict, which blocks because a gate that waves a push through when it could not run has stopped gating. The cause is named in that same message or in the engine error printed above it, and it is a missing Python interpreter, an unresolvable target, an unreadable receipt, a git command that failed, or any unexpected failure | Whatever the message names, most often installing an interpreter per `docs/host-setup.md` or fetching the target branch. Never another pass | +| This branch changes N carried canonical unit(s) that no recorded pass covers | The carried-content pass was skipped for a unit this change moved, and the refusal names each one with the digest to hand back | One carried-content pass per named unit, then `canonical_review.py record` for each | | A recorded pass names a branch the check did not measure | The hook reads `develop` and nothing else, so a branch based elsewhere is measured against `develop` whatever the pass targeted, and the engine deliberately prints no record command, since the one it would print records a pass over a diff nobody read | One more pass against the branch this work actually targets, where it does target the measured one. Where it does not, the gate cannot judge the branch at all and the bypass is its answer | -This table is the fleet's one enumeration of these, and every other surface states the principle and routes here rather than listing shapes or counting them. That is deliberate: through this skill's own review the count went from two to four, and every round left at least one restatement behind. +This table is the fleet's one enumeration of these, and every other surface states the principle and routes here rather than listing shapes or counting them. That is deliberate: through this skill's own review the count went from two to four, and it has grown again since, with every round leaving at least one restatement behind. ## Mechanics Live Elsewhere @@ -108,3 +149,4 @@ This table is the fleet's one enumeration of these, and every other surface stat - Branch base rule (`develop` unless the task is explicitly `main`-only): `repo-worktree`. - Finding disposition once a pull request exists, the Merge Gate, `scripts/pr_review.py`: `pr-review-conduct`, `drive-pr`. - The receipt's key, its backends, and the three-valued exit contract a capture point folds: `scripts/README.md` "`local_review.py`". +- The unit model, the coverage ledger, and the burn-down report: `scripts/README.md` "`canonical_review.py`". diff --git a/.husky/pre-push b/.husky/pre-push index 8f2ff39d..5634e630 100755 --- a/.husky/pre-push +++ b/.husky/pre-push @@ -1,5 +1,5 @@ #!/usr/bin/env bash -# Refuses a branch push that no recorded local review pass covers, per GOVERNANCE.md "Verification Discipline". +# Refuses a branch push whose diff no recorded local review pass covers, and one carrying a canonical unit no full-content pass covers, per GOVERNANCE.md "Verification Discipline". # Enable it per clone with `git config core.hooksPath .husky`, and see the pre-commit hook for why `_/husky.sh` is not sourced. set -Eeuo pipefail @@ -77,23 +77,29 @@ else exit 1 fi +# Two gates under one rule. +# The first asks whether a recorded pass covers this branch's diff, and the second whether one covers each canonical unit the branch moved, which a repository carrying that content reads whole rather than as a diff. +# Both run before either verdict is read, so one blocked push names every reason it was refused rather than only the first, which would otherwise cost a second push to discover the next. # No --target: a target read from the environment is set inline on the very command being gated. -status=0 -run_py scripts/local_review.py check || status=$? -if [ "$status" -eq 0 ]; then +diff_status=0 +run_py scripts/local_review.py check || diff_status=$? +unit_status=0 +run_py scripts/canonical_review.py check || unit_status=$? +if [ "$diff_status" -eq 0 ] && [ "$unit_status" -eq 0 ]; then exit 0 fi -if [ "$status" -eq 1 ]; then - echo "" >&2 - # The engine names its own case, and only some of them are a missing pass. - if [ "$py_name" != python3 ]; then - echo "pre-push: this host runs the engine as '$py_name', so read any 'python3' above as '$py_name'." >&2 - fi +echo "" >&2 +# The engines name their own cases, and only some of them are a missing pass. +if [ "$py_name" != python3 ]; then + echo "pre-push: this host runs the engines as '$py_name', so read any 'python3' above as '$py_name'." >&2 +fi +if [ "$diff_status" -eq 1 ] || [ "$unit_status" -eq 1 ]; then echo "pre-push: this push is blocked, which is the gate working. Read the engine message above for the case." >&2 - echo "pre-push: the local-strict-review skill's refusal table says what clears each one." >&2 - exit 1 + echo "pre-push: the local-strict-review skill's refusal table says what clears a diff-pass refusal," >&2 + echo "pre-push: and its carried-content section what clears a canonical-unit one." >&2 +fi +if [ "$diff_status" -gt 1 ] || [ "$unit_status" -gt 1 ]; then + echo "pre-push: a review gate could not run, which is not a verdict either way." >&2 + echo "pre-push: fix what it reported above, most often by fetching the target branch." >&2 fi -echo "" >&2 -echo "pre-push: the local review gate could not run, which is not a verdict either way." >&2 -echo "pre-push: fix what it reported above, most often by fetching the target branch." >&2 exit 1 diff --git a/AGENTS.md b/AGENTS.md index 8b8cc5b8..365845ed 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -92,6 +92,7 @@ Every rule below is a level-two section of [`GOVERNANCE.md`](./GOVERNANCE.md). R | Writing a commit message or pull request title | `Pull Request Title and Commit Message Conventions`, packaged as the `comment-and-doc-style` Skill | | Any prose, comment, doc, or line-ending change | `Documentation Style Conventions`, packaged as the `comment-and-doc-style` Skill | | Proving work actually happened | `Verification Discipline`, surfaced at its decision moment by the `agent-conduct` Skill, and the section keeps the full rules | +| Editing rule text, a Skill, or any other content other repos carry | `Verification Discipline`'s carried-content rule, whose pass the `local-strict-review` Skill runs and whose record `scripts/canonical_review.py` keeps | | Opening a pull request, or requesting, monitoring, answering, or closing a review | `PR Review Etiquette`, packaged as the `pr-review-conduct` Skill | | Reviewing a pull request, patch, or change set | `code-review`, which routes to the applicable general, language, documentation, and workflow skills | | Reporting progress or asking the user something | `Communicating with the User`, surfaced at its decision moment by the `agent-conduct` Skill, and the section keeps the full rules | @@ -110,4 +111,4 @@ Adding or changing a managed host tool is packaged as `add-host-tool`. It keeps Driving a pull request through its review loop, from a feature branch into `develop` and, when asked, on to a mergeable `develop -> main` promotion PR, disposing of every reviewer finding along the way per `pr-review-conduct`, is packaged as `drive-pr`, new content rather than a rule extracted from a section. Merging a ready promotion PR and dispatching the release it unblocks, refreshing this machine's installed Skills first when the repo is this hub, is `merge-and-release`, its own new-content package, invoked separately from `drive-pr` so the promotion merge and the release dispatch each keep their own explicit go-ahead. -Running one read-only, adversarial review pass against a branch's current diff against its target branch, full file context included, on the strongest model tier the session can reach, before a unit of PR-bound work is pushed toward a pull request or claimed done, is packaged as `local-strict-review`, new content rather than a rule extracted from a section. `drive-pr`, `pr-review-conduct`, and `agent-conduct` each reference it at the moment they already govern, rather than restating what it does. The rule itself lives in [`GOVERNANCE.md`](./GOVERNANCE.md) "Verification Discipline", `scripts/local_review.py` is the engine that records a pass so a capture point can check one, and a repository carrying a `.husky/pre-push` hook enforces it at the push itself, the skill staying the primary and agent-agnostic layer with the hook a bypassable backstop under it. +Running one read-only, adversarial review pass against a branch's current diff against its target branch, full file context included, on the strongest model tier the session can reach, before a unit of PR-bound work is pushed toward a pull request or claimed done, is packaged as `local-strict-review`, new content rather than a rule extracted from a section. `drive-pr`, `pr-review-conduct`, and `agent-conduct` each reference it at the moment they already govern, rather than restating what it does. The rule itself lives in [`GOVERNANCE.md`](./GOVERNANCE.md) "Verification Discipline", `scripts/local_review.py` is the engine that records a pass so a capture point can check one, and a repository carrying a `.husky/pre-push` hook enforces it at the push itself, the skill staying the primary and agent-agnostic layer with the hook a bypassable backstop under it. That skill carries a second pass under the same rule, over canonical content this repository authors and others carry, read one whole unit at a time rather than as a diff, because a diff-scoped read leaves the first real review of a rule to whichever repository carries it next, which is the one repository that cannot act on what it finds. `scripts/canonical_review.py` is that pass's engine, and the backlog it has yet to reach is `reports/canonical-review.md` in the hub, not a repo-relative link here since that path is hub-local like the Skills tree above. diff --git a/GOVERNANCE.md b/GOVERNANCE.md index 71ae2065..3d9a6c7b 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -182,6 +182,7 @@ The checks that separate work actually done from work that merely reports succes - **Config with a uniqueness rule is validated on read, and its consumers assert what it promised.** A repeated key in a lookup table is not a precedence question to settle quietly, it is two answers to one question, and keeping whichever came last picks one of them where the reader sees no choice being made. Fail on the duplicate at the point the config is read, so the code downstream can rely on the invariant instead of re-deriving it. - **Validate and read on the same normalized key.** A guard that compares stripped names while the join looks up the raw one passes a padded key and then matches nothing, so the exact fault the guard exists to stop is sitting inside the guard. Normalize once at the boundary and use that one value for both the check and the lookup. - **Every push toward a pull request is preceded by a local adversarial review of the branch's whole diff, and the pass is recorded.** The rule binds every push rather than the first one, so a fix push answering a reviewer's finding owes a pass exactly as the branch's first push did, and that is the round it is actually skipped on: the fix looks small, the branch was reviewed once already, and what goes up is content no review has read. Skipping it does not save the round, it moves it, into the fix-commit and review-comment cycle that spends wall-clock, Actions runtime, and agent tokens finding what a local pass would have. The pass itself, its delegation shape, and its model tier are the `local-strict-review` Skill's, and `scripts/local_review.py` records it keyed on the content the reviewer actually saw, so a capture point can ask whether a receipt still covers what is about to be pushed rather than trusting the rule to have been remembered. The pass is mandatory and its findings are advisory, which are opposite claims worth keeping apart: a pass is recorded whether it raised ten findings or none, and disposing of each one is judgment, per "PR Review Etiquette" below. +- **Canonical content one repo authors and others carry is reviewed the way a carrier reads it, whole, in the repo that can fix it.** Such content is written and merged against a diff of a few lines, and reaches a reviewer as a new file, in full, only when a repo carries it for the first time, so the first real read of a rule happens where nothing can be done about the result: the tree is manifest-owned, the copy is digest-compared, and a local edit there is drift on the next fidelity check. Every carrier after that re-discovers the same defect, and the finding arrives in a session holding no checkout of the authoring repo and no standing to test the claim. The unit is one level-two section of a Markdown canonical and one file otherwise, matching the fidelity unit `spec/section-model.md` declares, and the read is of that unit's whole current text rather than of the diff that moved it. `scripts/canonical_review.py` records a pass keyed on the content the reviewer saw and answers whether one still covers each unit a change moved, so a capture point can refuse a push carrying a unit nothing has read rather than trusting the rule to have been remembered. A section edited today is therefore read today, and a section nothing has read yet is a burn-down entry in `reports/canonical-review.md` rather than a block on unrelated work. Like the pass above, this one is mandatory and its findings are advisory. - **Run the repo's whole lint gate before every push, not the parts that look relevant.** CI runs all of them, so a partial local run only defers the failure, and the tool most likely to catch a given change is often the one it seems least about (an edit that manipulates line endings is exactly when `editorconfig-checker` matters). The repo documents each linter's known-working invocation, and this rule is that **all** of them run. - **Editing CRLF files programmatically: `.` matches `\r` in a regex**, so a captured line keeps its carriage return and rejoining with `\r\n` yields `CRCRLF`. A text-mode rewrite has the mirror failure, silently flattening CRLF to LF. Prefer line-based edits (`splitlines(keepends=True)`) or literal replacement over regex reassembly. In Python the text-mode failure is the *default*: `Path.read_text()` decodes through universal newlines and `write_text()` writes `\n` back, so a read-edit-write round trip flattens the whole file while the edit itself looks correct. Pass `newline=''` to both, or work in bytes. This is the mechanism behind the Line Endings warning above, and it is worth naming because the corruption is invisible in a rendered diff. - **Scope a check by what the project declares, not by the file that prompted it.** A check written while editing one file tends to cover that file's language and stop, and then reports success on every other surface the rule governs. Read the declared types, or the config that enumerates them, and cover each one, then assert a floor per surface so a table that narrows fails loudly instead of passing quietly. A rule about comments means every comment syntax the project ships, and a format that carries comments in practice counts even where its specification says otherwise. @@ -246,7 +247,7 @@ CI runs the full lint set, but run the linters locally before pushing to catch i - **A working local hook is strongly suggested fleet-wide, and its absence is a measured audit finding, not an invisible gap.** `spec/project-types.json`'s `parity.hooks` check reads this section for its rationale, judged by hand during an `AUDIT.md` run like every sibling check in its dimension, never mechanized by `spec/audit.py`: a repo with no local hook mechanism wired at all is a `linter-parity` defect, the same severity a missing markdownlint config already gets, while a repo mid-convergence (below) stays operational. CI remains the authoritative run regardless. Two catalog snippets carry the canonical shape, `catalog/snippets/husky/` (Husky.Net, for .NET or any project including Python) and `catalog/snippets/pre-commit/` (the Python `pre-commit` framework, for a repo with no `.husky/` tree), each carrying a copy of `catalog/snippets/hub-fetch-run.py` alongside it. - **The hook** runs **language formatting** and the **diff-scoped doc gates**, never Docker, so it stays fast. The formatting half is whatever the repo's own language needs, CSharpier and `dotnet format` for .NET or ruff and the repo's type checker for Python, via native tooling. A repo adds each half once its tree passes that half, since a gate that fails on the corpus it guards blocks every commit from the moment it lands, so a hook running one half is a repo mid-convergence rather than a repo out of conformance. The doc half runs each gate at the scope that fits it. The prose gate is scoped to the working tree diff against `HEAD` rather than swept over the tree, which is the difference between about 2.2 seconds and about 0.13 and is what makes it affordable in a hook at all. That scope is the working tree rather than the staged index, so a partially staged file is judged on all of its edits, not only the staged ones, and CI re-checks the whole tree regardless, which is what makes that scope affordable in a hook. A whole-repo check belongs there too when it is already fast and takes no file list, which the line-ending consistency check is, so scope is a property of the gate rather than a rule the hook applies to all of them. `repo_gate.py --check sha-pin` stays out of the hook regardless, since it resolves a same-owner pin against the GitHub API. - **The doc gates reach a downstream repo by fetching `.github/actions/prose-gate/prose_lint.py` and `.github/actions/repo-gate/repo_gate.py` fresh from `ptr727/ProjectTemplate`'s `main` branch, via `hub-fetch-run.py`, never vendored and never pinned.** Pinning anything Dependabot does not maintain goes stale by construction, and CI (this repo's own, and the hub's) is the backstop that catches a change landing broken on `main` before a locally fetched copy does real damage. These are the only network calls the doc gates make, one per fetched script. A Python repo's `uvx`-run ruff and type checker can also reach the network, to resolve `@latest` on a cache miss or refresh, the same category of dependency as the Docker pulls the VS Code Lint tasks already do routinely, not a new one. A fetch failure fails the commit, and it never silently skips the gate. The **hub's own** `.husky/pre-commit` is the one exception, staying local and offline, since it already carries `scripts/prose_lint.py` and `scripts/repo_gate.py` directly and has no hub to reach. A repo enables its hook per clone with `git config core.hooksPath .husky` or (`uv tool install pre-commit` once, then) `pre-commit install`. The Husky.Net snippet needs one more step per clone, `dotnet tool restore` then `dotnet husky install`, which generates `.husky/_/husky.sh`, the file the hook sources. CI remains the authoritative run either way. -- **The hub carries a second hook, `.husky/pre-push`, gating the local-review rule rather than lint.** It runs `scripts/local_review.py check` and refuses a branch push carrying content no recorded review pass covers, per "Verification Discipline" above. It fires only for a branch update, so a tag push and a branch delete pass through, and a branch holding no net content against its target has nothing for a review to cover. It refuses rather than guesses in every state it cannot speak for, since the engine reads the checkout it runs in while a push delivers a commit, and those describe the same content only sometimes. The order that keeps them describing the same content, and the one every capture point is built around, is to commit, then review, then record, then push. It reads `develop` and accepts no override from the environment, since an environment variable is set inline on the very command being gated, by whoever is being gated, which is the same reason an authorization is never read from a channel the agent itself can set. The cost is that it measures a branch based on anything but `develop` against `develop` anyway, so its refusal there is not a verdict about that branch. Each refusal names its own case, and the `local-strict-review` Skill carries the fleet's one enumeration of them with what clears each, deliberately in one place rather than restated here. It folds the engine's three-valued exit honestly: a check that could not run blocks too, and says so in different words than a check that ran and found no pass, because a gate that waves a push through when it could not run is a gate that stopped gating. It is a backstop rather than a seal, and it is bypassable by construction: `--no-verify` is the documented route out of a pickle and is not the only one, since a git hook cannot police its own invocation. A Claude Code session meets a narrower surface, that flag being denied unconditionally by the agent-safety hook's explicit-bypass rule, and that denial is Claude Code's alone, since Codex and opencode carry no such hook yet. So the committed hook raises the cost of skipping the rule for one agent and lowers it for none, and the prose layer above stays the agent-agnostic one that actually binds. It is hub-only for now. `local_review.py` is hub-hosted per "Hub-Hosted Tooling", and a downstream repo reaches it as a hub checkout's copy run with its own worktree as the working directory, so a catalog snippet carrying this hook fleet-wide is a later step rather than part of this one. +- **The hub carries a second hook, `.husky/pre-push`, gating the local-review rule rather than lint.** It runs `scripts/local_review.py check` and `scripts/canonical_review.py check`, refusing a branch push whose diff no recorded review pass covers and one whose changed canonical units none covers, per "Verification Discipline" above. Both run before either verdict is read, so a blocked push names every reason it was refused rather than only the first, which would otherwise cost a second push to discover the next. It fires only for a branch update, so a tag push and a branch delete pass through, and a branch holding no net content against its target has nothing for a review to cover. It refuses rather than guesses in every state it cannot speak for, since the engine reads the checkout it runs in while a push delivers a commit, and those describe the same content only sometimes. The order that keeps them describing the same content, and the one every capture point is built around, is to commit, then review, then record, then push. It reads `develop` and accepts no override from the environment, since an environment variable is set inline on the very command being gated, by whoever is being gated, which is the same reason an authorization is never read from a channel the agent itself can set. The cost is that it measures a branch based on anything but `develop` against `develop` anyway, so its refusal there is not a verdict about that branch. Each refusal names its own case, and the `local-strict-review` Skill carries the fleet's one enumeration of them with what clears each, deliberately in one place rather than restated here. It folds the engine's three-valued exit honestly: a check that could not run blocks too, and says so in different words than a check that ran and found no pass, because a gate that waves a push through when it could not run is a gate that stopped gating. It is a backstop rather than a seal, and it is bypassable by construction: `--no-verify` is the documented route out of a pickle and is not the only one, since a git hook cannot police its own invocation. A Claude Code session meets a narrower surface, that flag being denied unconditionally by the agent-safety hook's explicit-bypass rule, and that denial is Claude Code's alone, since Codex and opencode carry no such hook yet. So the committed hook raises the cost of skipping the rule for one agent and lowers it for none, and the prose layer above stays the agent-agnostic one that actually binds. It is hub-only for now. `local_review.py` and `canonical_review.py` are both hub-hosted per "Hub-Hosted Tooling", and a downstream repo reaches either as a hub checkout's copy run with its own worktree as the working directory, so a catalog snippet carrying this hook fleet-wide is a later step rather than part of this one. The canonical-unit half is also run by the hub's own `.github/actions/validate` hook on every pull request, which is where it actually binds, since a hook a push can bypass raises the cost of skipping the rule without ever settling it. That step is scoped to a pull request because a unit's change is measured against the branch it is proposed into, and a push carrying no pull request names none. - **The VS Code Lint tasks** run the full doc-lint set via Docker `:latest` on demand, the local surface for Markdown, spelling, workflow, and EditorConfig checks, plus `Lint: Prose` and `Lint: EOL`, the same two hook gates in whole-repo mode rather than diff-scoped, for on-demand full-tree validation. The Docker invocations below run the same tools and configs as the VS Code tasks. Their headless form separates the image pull and minimizes repository exposure for an agent executor. diff --git a/OPERATIONS.md b/OPERATIONS.md index 15dce873..4c366bb7 100644 --- a/OPERATIONS.md +++ b/OPERATIONS.md @@ -27,6 +27,7 @@ uvx coverage@latest run --source=scripts,spec,host-setup --append host-setup/age uvx coverage@latest run --source=scripts,spec,host-setup --append host-setup/agent-safety/claude/test_install.py uvx coverage@latest report python3 scripts/build_dist.py --check +python3 scripts/canonical_review.py check python3 scripts/repo_gate.py python3 scripts/prose_lint.py . --check charset --check semicolon --check dash --check dupword --check spelling --check comment-wrap --check comment-case --check home-path --check dead-path python3 scripts/prose_lint.py . --check charset-unknown --summary @@ -35,6 +36,8 @@ python3 spec/validate.py python3 scripts/docker_lint.py ``` +The canonical-review check sits in CI's own list only for a pull request, since a canonical unit's change is measured against the branch it is proposed into and a push carrying no pull request names none. The local run above takes the default target, `develop`, which is the same measurement for an ordinary feature branch and the wrong one for a branch based on `main`, where it needs `--target main` to mean anything. + The cache directory is unique to this verification run and remains outside the checkout. The operating system can reap it with other temporary data. A restricted executor may deny the first `uvx` network request, Docker socket access, or third-party image access to the repository. Record that denial as an execution boundary, then rerun the required command with scoped approval. Persist repository-exposure approval only when the executor constrains the read-only mount, disabled networking, and digest together. Only the rerun's tool output is a lint or test verdict. Provider-specific host configuration lives in [`docs/host-setup.md`](./docs/host-setup.md) "Agent Worktree Access". The `test_install.py` line behaves differently here than in CI, stated so its failure reads as the verdict it is. Its report cases install from this checkout and assert the machine then reads as current. An install from a checkout carrying uncommitted changes records a dirty stamp that reads as stale. So on a working tree mid-change those cases fail by design where CI's clean checkout passes. The remedy is to run them again once the change is committed, not to read the failure as a regression. diff --git a/reports/canonical-review.json b/reports/canonical-review.json new file mode 100644 index 00000000..9d5be5d5 --- /dev/null +++ b/reports/canonical-review.json @@ -0,0 +1,53 @@ +{ + "note": "What full-content reviews of hub canonical content have covered, one entry per unit, holding the most recent pass. Written by scripts/canonical_review.py record, never by hand, and git keeps the history. A unit is covered while its digest here matches the content's, so an edit to the unit retires the pass. See ptr727/ProjectTemplate#1138 for why the record exists.", + "passes": [ + { + "unit": ".agents/skills/local-strict-review/SKILL.md > (preamble)", + "digest": "sha256:e607d4a5c7ad14232516261ab7f0941de11a4312415a65dc054b394a645a106d", + "reviewer": "agent-skill", + "findings": 1, + "hubCommit": "68117017a1483706765a8075ab8a29bb67bfe711", + "stamp": "2026-08-31T14:38:05Z" + }, + { + "unit": ".agents/skills/local-strict-review/SKILL.md > Mechanics Live Elsewhere", + "digest": "sha256:9190c95f50cfed12a0157f1cbb55e080c0fc7cbe2dbbbaf6fd2f744736c365b5", + "reviewer": "agent-skill", + "findings": 0, + "hubCommit": "68117017a1483706765a8075ab8a29bb67bfe711", + "stamp": "2026-08-31T14:38:06Z" + }, + { + "unit": ".agents/skills/local-strict-review/SKILL.md > The Carried-Content Pass", + "digest": "sha256:d983c6d9954a8b06ef0de583d42be863bdca2cccbd026c46a0012f8134cd7e60", + "reviewer": "agent-skill", + "findings": 3, + "hubCommit": "68117017a1483706765a8075ab8a29bb67bfe711", + "stamp": "2026-08-31T14:38:05Z" + }, + { + "unit": ".agents/skills/local-strict-review/SKILL.md > When to Run It", + "digest": "sha256:42be4454bae1fbefcc7d5ad6656727bb049b50b14caef730c7effd7a7a99dceb", + "reviewer": "agent-skill", + "findings": 4, + "hubCommit": "68117017a1483706765a8075ab8a29bb67bfe711", + "stamp": "2026-08-31T14:38:05Z" + }, + { + "unit": "AGENTS.md > Where the Rules Live", + "digest": "sha256:c339c7cca92d462dd3fab2adae3692c1ded67a75d3561b9e5457c368183f3027", + "reviewer": "agent-skill", + "findings": 1, + "hubCommit": "68117017a1483706765a8075ab8a29bb67bfe711", + "stamp": "2026-08-31T14:38:06Z" + }, + { + "unit": "GOVERNANCE.md > Verification Discipline", + "digest": "sha256:7d24dbb61ef94e29b3343f82a0f07f10a98fa405e99db7fa0ce06404bf1a655a", + "reviewer": "agent-skill", + "findings": 2, + "hubCommit": "68117017a1483706765a8075ab8a29bb67bfe711", + "stamp": "2026-08-31T14:38:06Z" + } + ] +} diff --git a/reports/canonical-review.md b/reports/canonical-review.md new file mode 100644 index 00000000..ab9a5063 --- /dev/null +++ b/reports/canonical-review.md @@ -0,0 +1,459 @@ +# Canonical content review coverage + +Generated by `python3 scripts/canonical_review.py report` - do not hand-edit. Records are written by `canonical_review.py record` into [`reports/canonical-review.json`][ledger]. Git dates this file. + +A unit is one level-two section of a Markdown canonical, or one whole file otherwise. It is **covered** when a recorded full-content pass names its current text, **stale** when a pass named earlier text, and **never** when no pass has read it here at all. A never-read unit is the backlog [ptr727/ProjectTemplate#1138][issue] records: the first real review of it happens in whichever repository carries it next. + +## Coverage + +- units: 289 +- covered: 6 +- stale: 0 +- never read here: 283 + +## Burn-down + +### .agents/skills/README.md + +- **(whole file)** - never + +### .agents/skills/add-host-tool/SKILL.md + +- **(preamble)** - never +- **Establish the Contract** - never +- **Implement Each Platform** - never +- **Update the Complete Surface** - never +- **Verify** - never + +### .agents/skills/agent-conduct/SKILL.md + +- **(preamble)** - never +- **Before Assuming** - never +- **Before Claiming Done** - never +- **Delegation, in One Paragraph** - never +- **When a Failure Surfaces a Lesson** - never +- **Why This Exists** - never + +### .agents/skills/audit-a-repo/SKILL.md + +- **(preamble)** - never +- **After the Report** - never +- **Before Measuring Anything** - never +- **Measuring** - never +- **Reporting** - never +- **Why This Exists** - never + +### .agents/skills/carried-instruction-file-guard/SKILL.md + +- **(preamble)** - never +- **Before you touch any of these four files** - never +- **If you are not sure which case you are in** - never +- **What is actually safe to overwrite without this procedure** - never +- **Why this exists** - never + +### .agents/skills/code-review/SKILL.md + +- **(preamble)** - never +- **Establish the Contract** - never +- **Publish Every Finding** - never +- **Review the Change** - never + +### .agents/skills/comment-and-doc-style/SKILL.md + +- **(preamble)** - never +- **Carried files reference no coordination machinery** - never +- **Character set** - never +- **Comments** - never +- **Docker lint authorization** - never +- **Line endings** - never +- **Markdown files: linting and spelling** - never +- **Markdown formatting** - never +- **Naming tools in prose** - never +- **PR titles and commit messages** - never +- **Quantitative claims** - never +- **Sentence structure** - never +- **Why this exists** - never + +### .agents/skills/comment-and-doc-style/references/carried-doc-references.md + +- **(preamble)** - never +- **The two exceptions** - never +- **What is banned** - never +- **What is not a coordination reference** - never +- **Which files this governs** - never + +### .agents/skills/comment-and-doc-style/references/line-endings.md + +- **(preamble)** - never +- **Auditing** - never +- **Choosing an ending for a new file type** - never +- **Editing discipline** - never +- **Operational (config) repos** - never +- **Scripts and extensionless executables** - never +- **The defaults** - never + +### .agents/skills/comment-and-doc-style/references/markdown-links.md + +- **(preamble)** - never +- **Mechanics** - never +- **Naming a reference** - never +- **The definition block** - never +- **Where the rule applies** - never + +### .agents/skills/copilot-instructions-keeper/SKILL.md + +- **(preamble)** - never +- **Carrying it fresh, new repo or full resync** - never +- **Checking a repo's copy for drift** - never +- **The one thing this file has that others don't: repo-local ledger entries** - never +- **What this skill does not cover** - never +- **Why this exists** - never + +### .agents/skills/dotnet-codestyle/SKILL.md + +- **(preamble)** - never +- **Analyzer suppressions (.NET)** - never +- **Best practices** - never +- **Build requirements** - never +- **Code patterns** - never +- **Coding standards and conventions** - never +- **Error handling and logging** - never +- **Project configuration** - never +- **Testing conventions** - never +- **Tooling and editor** - never +- **Why this exists** - never + +### .agents/skills/dotnet-codestyle/references/conventions.md + +- **(preamble)** - never +- **C# language features** - never +- **Code structure** - never +- **Comments and documentation** - never +- **Naming conventions** - never + +### .agents/skills/dotnet-codestyle/references/project-config.md + +- **(whole file)** - never + +### .agents/skills/dotnet-codestyle/references/testing.md + +- **(whole file)** - never + +### .agents/skills/drive-pr/SKILL.md + +- **(preamble)** - never +- **Disposing of Every Finding** - never +- **How Far to Drive** - never +- **Mechanics Live Elsewhere** - never +- **Stop and Ask, Beyond the How-Far Question** - never +- **The Drive Loop** - never +- **What Invoking This Skill Authorizes** - never +- **Why This Exists** - never + +### .agents/skills/fleet-conformance-check/SKILL.md + +- **(preamble)** - never +- **Answering "why isn't a fleet rule applying"** - never +- **Refresh cadence** - never +- **What it checks** - never +- **What it escalates instead of touching** - never +- **What it is safe to fix on its own** - never +- **Why this exists** - never + +### .agents/skills/git-commit-conventions/SKILL.md + +- **(preamble)** - never +- **History rewrites re-identify only what changed** - never +- **Identity, verified not set** - never +- **Never force push** - never +- **Never run destructive git commands without being asked** - never +- **Signing, verified not configured** - never +- **Staging versus committing** - never +- **Why this exists** - never + +### .agents/skills/git-commit-conventions/references/history-rewrite.md + +- **(whole file)** - never + +### .agents/skills/local-strict-review/SKILL.md + +- **Disposing of Findings** - never +- **Recording the Pass** - never +- **Running It** - never +- **What It Does** - never +- **Why This Exists** - never + +### .agents/skills/merge-and-release/SKILL.md + +- **(preamble)** - never +- **How Far to Go** - never +- **Mechanics Live Elsewhere** - never +- **Stop and Report, Never Guess** - never +- **The Procedure** - never +- **What Invoking This Skill Authorizes** - never +- **Why This Exists** - never + +### .agents/skills/operational-vs-release-workflow/SKILL.md + +- **(preamble)** - never +- **Branching (release model)** - never +- **Operational repositories (the complete delta)** - never +- **Publishing (release model)** - never +- **Which model this repo uses** - never +- **Why this exists** - never + +### .agents/skills/operational-vs-release-workflow/references/branch-protection-and-promotion.md + +- **(preamble)** - never +- **App-token workflows use Client ID, not App ID** - never +- **Codegen determinism** - never +- **Configuring branch protection: don't hand-build the rules** - never +- **Dual-target bots** - never +- **Executing a `develop -> main` promotion safely** - never +- **Why both rulesets omit "Require branches to be up to date before merging"** - never + +### .agents/skills/operational-vs-release-workflow/references/release-publish-mechanics.md + +- **(preamble)** - never +- **Map your outputs to the right seam** - never +- **No-op republish guarantee** - never +- **Orchestration vs. build: the override seam** - never +- **Per-target subsetting** - never +- **Reusable-task parameter contract** - never +- **Wrapper repos that track an upstream release** - never + +### .agents/skills/pr-review-conduct/SKILL.md + +- **(preamble)** - never +- **Answering a suppressed finding** - never +- **Escalate to the maintainer when** - never +- **Every finding ends in one of five outcomes** - never +- **Expected review loop** - never +- **Mechanics Live Elsewhere** - never +- **Merge Gate, check this before merging or enabling auto-merge** - never +- **Triaging findings** - never +- **Why this exists** - never + +### .agents/skills/python-codestyle/SKILL.md + +- **(preamble)** - never +- **Code style** - never +- **Layout** - never +- **Linter cleanliness** - never +- **Local development loop** - never +- **Tests** - never +- **Toolchain** - never +- **Two profiles** - never +- **Versioning** - never +- **Why this exists** - never + +### .agents/skills/python-codestyle/references/code-style.md + +- **(preamble)** - never +- **Comments** - never +- **Docstrings** - never +- **Formatting and linting** - never +- **Imports** - never +- **Naming** - never +- **Patterns to avoid** - never +- **Type hints** - never + +### .agents/skills/python-codestyle/references/profiles.md + +- **(preamble)** - never +- **Adapt before propagating** - never +- **Two profiles: full specification** - never +- **Versioning** - never + +### .agents/skills/python-codestyle/references/testing.md + +- **(whole file)** - never + +### .agents/skills/repo-worktree/SKILL.md + +- **(preamble)** - never +- **Agent-Specific Worktree Tools** - never +- **Creating a Worktree** - never +- **Listing and Cleanup** - never +- **Preparing Git Hooks** - never +- **The Base Branch** - never +- **The Mandate** - never +- **Why This Exists** - never + +### .agents/skills/resync-a-repo/SKILL.md + +- **(preamble)** - never +- **Apply, in this order** - never +- **Confirm the procedure before starting** - never +- **Reach the hub and measure before changing anything** - never +- **Ship it** - never +- **Why this exists** - never + +### .agents/skills/shell-codestyle/SKILL.md + +- **(preamble)** - never +- **Rules** - never +- **When shell, not Python** - never +- **Why this exists** - never + +### .agents/skills/skill-lifecycle/SKILL.md + +- **(preamble)** - never +- **Changing or Retiring a Skill** - never +- **Creating a Skill** - never +- **Deciding a Topic Deserves a Skill** - never +- **The Doc-Packaging Pattern** - never +- **The Pipeline** - never +- **Why This Exists** - never + +### .agents/skills/standup-a-repo/SKILL.md + +- **(preamble)** - never +- **Apply, in order** - never +- **Before starting** - never +- **Onboarding a new repo type** - never +- **Ship it** - never +- **Why this exists** - never + +### .agents/skills/upstream-contribution-workflow/SKILL.md + +- **(preamble)** - never +- **The two-branch shape** - never +- **Use the upstream repo's own conventions, not the fleet's** - never +- **What stays governed by the fleet's own rules** - never +- **Why this exists** - never + +### .agents/skills/workflow-ci-contract/SKILL.md + +- **(preamble)** - never +- **After Any Workflow Edit** - never +- **How the Contract Is Read** - never +- **Style Rules That Break in One-Line Diffs** - never +- **The Core Behavioral Spine** - never +- **Why This Exists** - never + +### .agents/skills/workflow-ci-contract/references/d-guarantees.md + +- **(preamble)** - never +- **D1: PR Fast-Feedback (Smoke)** - never +- **D2: Validation at Entry** - never +- **D3: Versioning and Classification** - never +- **D4: Release and Publish** - never +- **D5: Resource Cleanup** - never +- **D6: Seam Conformance** - never +- **D7: Concurrency, Permissions, Safety** - never +- **D8: Bots and Automation** - never +- **D9: Style and Static** - never + +### .agents/skills/workflow-ci-contract/references/test-methodology.md + +- **(preamble)** - never +- **5A: Static Audit** - never +- **5B: Trace Scenarios** - never +- **5C: Live Probe** - never +- **Verdict** - never + +### .editorconfig + +- **(whole file)** - never + +### .editorconfig-checker.json + +- **(whole file)** - never + +### .gitattributes + +- **(whole file)** - never + +### .github/copilot-instructions.md + +- **(preamble)** - never +- **Commit Messages and Pull Request Titles** - never +- **GitHub Copilot Review Runbook** - never +- **Reviewing Carried Fleet Content** - never +- **When in Doubt** - never + +### .markdownlint-cli2.jsonc + +- **(whole file)** - never + +### AGENTS.md + +- **Context and Delegation Discipline** - never +- **Fleet Bootstrap** - never + +### AUDIT.md + +- **(preamble)** - never +- **0. When to Run and What "Done" Means** - never +- **1. Scope and Ground-Truth Branch** - never +- **10. Converge: Apply the Fixes** - never +- **2. Resolve the Repo's Type(s)** - never +- **3. Applicability Gate** - never +- **4. Per-Dimension Checks (Letter and Intent)** - never +- **5. Assert the Actions Implement WORKFLOW.md** - never +- **6. Validate Settings, Rulesets, and Secrets** - never +- **7. Verdict Model** - never +- **8. Report** - never +- **9. Escalate** - never + +### CLAUDE.md + +- **(whole file)** - never + +### CODESTYLE.md + +- **(preamble)** - never +- **.NET** - never +- **General** - never +- **Python** - never +- **Shell** - never + +### GOVERNANCE.md + +- **Branching Model** - never +- **Communicating with the User** - never +- **Devcontainer** - never +- **Documentation Style Conventions** - never +- **Durable Knowledge and Self-Improvement** - never +- **Editor and Tasks** - never +- **Foundational Principles** - never +- **Git and Commit Rules** - never +- **Hub-Hosted Tooling** - never +- **Operational Repositories** - never +- **PR Review Etiquette** - never +- **Pull Request Title and Commit Message Conventions** - never +- **Release Model** - never +- **Repository Boundaries and Write Safety** - never +- **Repository Details** - never +- **Repository Layout** - never +- **Representative Data in Agent-Authored Text** - never +- **Supported Development Platforms** - never +- **Workflow YAML Conventions** - never + +### WORKFLOW.md + +- **(preamble)** - never +- **1. Purpose and How to Use This Document** - never +- **2. Workflow Style Conventions** - never +- **3. Architecture** - never +- **4. Behavioral Contract: Expected Outcomes** - never +- **5. Test Methodology** - never +- **6. Per-Project-Type Test Walkthroughs** - never + +### cspell.json + +- **(whole file)** - never + +### version.json + +- **(whole file)** - never + +## Declared but not held here + +A manifest path this hub does not itself carry, which is ordinary for one scoped to a project type this hub is not, and for a section the manifest names that the file does not hold. + +- codecov.yml + +[ledger]: ./canonical-review.json +[issue]: https://github.com/ptr727/ProjectTemplate/issues/1138 diff --git a/scripts/README.md b/scripts/README.md index c860eb30..2553932e 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -4,15 +4,16 @@ The fleet's checks and review tooling, run by hand, with the deterministic ones **Hosted here and reached, never carried.** These are not declared in [`spec/files.json`][files], so the audit does not expect a downstream repo to ship them, the same footing as `spec/audit.py`. That is the fleet model rather than an omission: a gate holding no per-repo content has one canonical implementation for the fleet. The prose and repository gate implementations live beside their composite actions under `.github/actions/`, which lets reusable workflows reach the implementation at their pinned hub commit through `$/.github/actions/` without checking out the hub. The same-named files in `scripts/` are thin local entry points that call those action-owned implementations, so maintainers and hooks retain the established commands from a hub checkout without creating a second copy of either gate. A repository that cannot reach the hub reports a local check as not run rather than reconstructing it, since a rebuilt gate encodes its author's reading of the rule and agrees with no other repository. -Python only, standard library only, no third-party packages. Every check script is read-only and exits non-zero on a finding. `build_dist.py`, `local_review.py`, and `skills_install.py` below are the exceptions, since a generator, a recorder, and an installer all exist to write. `build_dist.py --check` and `skills_install.py --report` are read-only modes for CI and for asking without changing anything. `local_review.py status` and `check` answer without recording anything, but they are not read-only in that same sense: reading the working tree stages it, which writes inside `.git` and runs any `filter.*.clean` the repository configures, and only the receipt is left untouched. +Python only, standard library only, no third-party packages. Every check script is read-only and exits non-zero on a finding. `build_dist.py`, `local_review.py`, `canonical_review.py`, and `skills_install.py` below are the exceptions, since a generator, two recorders, and an installer all exist to write. `build_dist.py --check` and `skills_install.py --report` are read-only modes for CI and for asking without changing anything. `local_review.py status` and `check` answer without recording anything, but they are not read-only in that same sense: reading the working tree stages it, which writes inside `.git` and runs any `filter.*.clean` the repository configures, and only the receipt is left untouched. `canonical_review.py` reads files rather than staging them, so its `list`, `status`, and `check` are read-only outright, and only `record` and `report` write. -The directory separates its kinds by name and by tree. A gate checks and exits non-zero on a finding, and its name carries a `_lint` or `_gate` suffix saying what it gates. The `prose_lint.py` and `repo_gate.py` entry points delegate to the action-owned implementations that gate this tree in CI, and `host_gate.py` gates the machine it runs on. A utility does work rather than gating and carries no suffix: `build_dist.py`, `local_review.py`, `pr_review.py`, `skills_install.py`. The unit tests live apart under [`scripts/tests/`][tests], one `test_