diff --git a/.agents/skills/local-strict-review/SKILL.md b/.agents/skills/local-strict-review/SKILL.md index 15797d52..8e5d12f2 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 @@ -22,7 +26,7 @@ A coding agent that finishes a unit of work, judges it ready, and opens the pull ## What It Does -Dispatches one read-only subagent against this branch's full diff since it forked from its target branch. Resolve `` once, `develop` unless `repo-worktree`'s base-branch rule put this branch on `main` instead, then fetch it, `git fetch origin `, and diff against the merge-base, `git diff "$(git merge-base origin/ HEAD)"`. Stop and report a failed fetch rather than running the merge-base or diff commands anyway: an existing local `origin/` ref can still resolve after a failed fetch, and reviewing against it silently trades the current target for a stale one. Use the same resolved `` in every command below, never a literal `develop` alongside it. Naming the target branch explicitly matters: the branch's own `@{u}` tracking ref points at the branch's own remote once it has been pushed, not at the branch it targets, so anchoring there silently narrows a later run to only the diff since the last push instead of the full accumulated diff. That merge-base diff covers every commit already on the branch plus whatever is currently staged or unstaged, so it never reviews only the latest increment, at any of the moments this skill is invoked from. An empty diff is not the same as nothing to review, and it is never the signal to stop: it reports no untracked file at all, and it reports nothing for content a commit carries that the working tree has since put back. The untracked-file list below covers the first of those. The second is why this skill commits before reviewing, since a removal or a restore that is committed leaves no net content to miss, and why the engine reads HEAD rather than this diff, its change set coming from the merge base against HEAD, the index and the working tree, so the two answer different questions. A fresh review of the full accumulated diff is what catches what per-push review misses, the exact evidence this skill exists to act on. +Dispatches one read-only subagent against this branch's full diff since it forked from its target branch. Resolve `` once, `develop` unless `repo-worktree`'s base-branch rule put this branch on `main` instead, then fetch it, `git fetch origin `, and diff against the merge-base, `git diff "$(git merge-base origin/ HEAD)"`. Stop and report a failed fetch rather than running the merge-base or diff commands anyway: an existing local `origin/` ref can still resolve after a failed fetch, and reviewing against it silently trades the current target for a stale one. Use the same resolved `` in every command below, never a literal `develop` alongside it. Naming the target branch explicitly matters: the branch's own `@{u}` tracking ref points at the branch's own remote once it has been pushed, not at the branch it targets, so anchoring there silently narrows a later run to only the diff since the last push instead of the full accumulated diff. That merge-base diff covers every commit already on the branch plus whatever is currently staged or unstaged, so it never reviews only the latest increment, at any of the moments this skill is invoked from. An empty diff is not the same as nothing to review, and it is never the signal to stop: it reports no untracked file at all, and it reports nothing for content a commit carries that the working tree has since put back. The untracked-file list below covers the first of those. The second is why the diff pass commits before reviewing, the carried-content pass below running against uncommitted content instead, since a removal or a restore that is committed leaves no net content to miss, and why the engine reads HEAD rather than this diff, its change set coming from the merge base against HEAD, the index and the working tree, so the two answer different questions. A fresh review of the full accumulated diff is what catches what per-push review misses, the exact evidence this skill exists to act on. `git diff` never reports a path `git add` has not touched, so a newly created file sitting untracked would otherwise go unread. List it explicitly, `git ls-files --others --exclude-standard`, and read each result in full alongside the diff, the same as any other file the diff touches. @@ -78,6 +82,45 @@ 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. `GOVERNANCE.md` "Verification Discipline" states the rule and why the ordering it corrects is a defect, and is not restated here. What it requires of a run is below. + +**The unit is what a reviewer reads whole**, and `spec/files.json` rather than the document decides which, down to which files carry units at all. `canonical_review.py list` names the whole set and is the authority on it, so the rules are not paraphrased here, where a paraphrase can only drift from them. In the ordinary case a unit is one level-two section of a carried Markdown canonical, and `check` names each one it wants exactly as `record` takes it. 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 the pass above has already done. + +Run it at the same model tier and in the same delegation shape as the pass above. The brief, the engine, its flags, and the point in the sequence where the record is written each differ, and all four are below. + +```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 +git fetch origin # stop and report a failed fetch rather than measuring past it +python3 scripts/canonical_review.py check --target # each uncovered unit, with its digest +# run the pass above over each unit it named, then, per unit: +python3 scripts/canonical_review.py record --reviewer agent-skill --unit '=' [--findings N] +``` + +These run in the authoring repository itself, which is the only repository this pass ever runs in, so the engine path is the plain one and there is no downstream side needing the `/` form the pass above shows for its own reach. 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. + +`` is the branch this work targets, resolved once as the pass above resolves it and passed to `check` explicitly. Left off it defaults to `develop`, so a branch based on `main` is measured from a fork point nobody read. The fetch matters for the same reason it does above: the engine resolves `origin/` if it already exists and never fetches it, so a stale remote-tracking ref moves the fork point without saying so. Lagging, which is the ordinary way to be stale, moves it back and gates units this change never touched, and the reverse case, where the branch restores text the target has since changed, drops one it did move. Neither is announced, so the fetch is what keeps the fork point meaning what the reviewer read against. `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. Fixing a finding is itself such an edit, so `record` then refuses the digest you were holding: that refusal is the content having moved rather than a fault in the record, and the answer is a read of the unit's new text, which is what a carrier will actually receive, recorded at its new digest. + +**This pass records before the commit, where the pass above records after it**, and the two orders are opposite because the two records live in different places. A receipt sits in the worktree's git directory and can never be committed, so it is written once the commit has fixed what a push will deliver. This ledger and its burn-down are tracked files the commit has to carry, so writing them after that commit leaves the tree differing from HEAD, which is a state the pre-push hook refuses before either gate runs. The shortest order meeting both, and the one the refusal table below assumes, is: run this pass and record each unit, commit that together with the change, then read the digest, run the diff pass, record its receipt, and push. Committing the change first and the ledger in a second commit satisfies the same constraint and costs a commit. + +**A unit nothing has read here yet is not this branch's debt.** `check` refuses the units this change moved, meaning the ones whose text it edited and the ones it newly carried, since widening the manifest hands a carrier content for the first time exactly as writing it would. Everything else is a burn-down entry in the hub's `reports/canonical-review.md` rather than a block on unrelated work. 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,19 +130,23 @@ 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, or that newly carries some by widening the manifest, over each unit `check` names, 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. +In the hub, `.husky/pre-push` 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. A blocked push usually means one of those passes was skipped. Both capture points, that hook and the pull request one named below, are the hub's own, and a repository carrying this Skill has neither until one is carried to it, which is what makes the moments above the layer that actually binds everywhere. 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. That route is not open in every seat. A Claude Code session running the fleet's agent-safety hook has the flag denied unconditionally, so where the rows below say a bypass is the answer, the answer in that seat is to report the state and hand the push to the maintainer rather than to force it. The hub's own `.github/actions/validate` composite action runs the canonical-unit half again as a step on every pull request into `main` or `develop`, which is what its workflow triggers on. That one needs no hooks path, runs whether or not any clone enabled one, and `--no-verify` does not reach it, which is what makes it the capture point a push cannot bypass where it applies. -**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. +**Read the refusal itself, which names its own case.** Some of the rows below are cleared by running a pass and some are cleared by nothing of the kind, and each row says which, so no count of either is kept here to go stale against the table. Some the hook decides before either engine runs, so there is no engine message under them, and the rows say where each one's detail comes from. | The refusal says | What it means | What clears it | | --- | --- | --- | -| 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 | -| 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 | +| No local review covers this branch's current content | The ordinary missing pass: no recorded receipt covers what this push delivers, either because none was recorded or because the content moved after one was | One pass over the branch's whole diff, recorded per "Recording the Pass" above | +| 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. The hook prints the same headline for an unresolved merge and for a `git update-index --refresh` that exited above 1, naming each on its own line | Commit what is being pushed, then the pass, then the record. Where the change also moved a canonical unit, follow "The Carried-Content Pass" order instead, since committing first strands that ledger after the commit and each fix then lands on another row. Resolve the merge first where the hook names one, and run `git status` first where it names the refresh, since the content may not differ at all | +| The commit is not this worktree's HEAD | Any pushed branch ref carrying an object id that is neither this worktree's HEAD nor the all-zero id of a delete, which a push from a checkout sitting elsewhere reaches and so does a multi-ref push such as `git push --all` | Push one branch, the one this worktree holds. Where another branch is the one wanted, check it out in its own worktree first, 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, a manifest or ledger the engine could not read, 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 or newly carried, 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, in the order "The Carried-Content Pass" above gives. The ledger that writes is tracked content, so the commit has to carry it and the diff pass comes after | +| A canonical refusal naming units this branch never touched | The fork point is not where the reader thinks it is. Either `origin/` does not hold the commit this branch forked from, since neither engine ever fetches it, or the branch is based on something other than `develop` and the hook, which passes no `--target`, measured it against `develop` regardless. Unlike the row below it still prints a record command, and taking that one records passes over units nobody read | `git fetch origin `, then `canonical_review.py check --target ` by hand for the real set, then pass and record what that names and commit the ledger with the change, per row 5. Where the branch targets something the hook does not measure, no pass clears it, so the gate cannot judge that branch at all and the bypass is its answer, as in the row below | +| The recorded pass was run against X and this check measured Y, printed under row 1's headline | 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 the shapes. That is deliberate: every review round that added a shape also left a restatement of it somewhere else, and keeping one table is what stops the next round doing the same. ## Mechanics Live Elsewhere @@ -108,3 +155,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..91f37063 100644 --- a/.claude-plugin/fleet-skills/.source-digest +++ b/.claude-plugin/fleet-skills/.source-digest @@ -1 +1 @@ -438bd561ef5f233b +fdc184b741106834 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..8e5d12f2 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 @@ -22,7 +26,7 @@ A coding agent that finishes a unit of work, judges it ready, and opens the pull ## What It Does -Dispatches one read-only subagent against this branch's full diff since it forked from its target branch. Resolve `` once, `develop` unless `repo-worktree`'s base-branch rule put this branch on `main` instead, then fetch it, `git fetch origin `, and diff against the merge-base, `git diff "$(git merge-base origin/ HEAD)"`. Stop and report a failed fetch rather than running the merge-base or diff commands anyway: an existing local `origin/` ref can still resolve after a failed fetch, and reviewing against it silently trades the current target for a stale one. Use the same resolved `` in every command below, never a literal `develop` alongside it. Naming the target branch explicitly matters: the branch's own `@{u}` tracking ref points at the branch's own remote once it has been pushed, not at the branch it targets, so anchoring there silently narrows a later run to only the diff since the last push instead of the full accumulated diff. That merge-base diff covers every commit already on the branch plus whatever is currently staged or unstaged, so it never reviews only the latest increment, at any of the moments this skill is invoked from. An empty diff is not the same as nothing to review, and it is never the signal to stop: it reports no untracked file at all, and it reports nothing for content a commit carries that the working tree has since put back. The untracked-file list below covers the first of those. The second is why this skill commits before reviewing, since a removal or a restore that is committed leaves no net content to miss, and why the engine reads HEAD rather than this diff, its change set coming from the merge base against HEAD, the index and the working tree, so the two answer different questions. A fresh review of the full accumulated diff is what catches what per-push review misses, the exact evidence this skill exists to act on. +Dispatches one read-only subagent against this branch's full diff since it forked from its target branch. Resolve `` once, `develop` unless `repo-worktree`'s base-branch rule put this branch on `main` instead, then fetch it, `git fetch origin `, and diff against the merge-base, `git diff "$(git merge-base origin/ HEAD)"`. Stop and report a failed fetch rather than running the merge-base or diff commands anyway: an existing local `origin/` ref can still resolve after a failed fetch, and reviewing against it silently trades the current target for a stale one. Use the same resolved `` in every command below, never a literal `develop` alongside it. Naming the target branch explicitly matters: the branch's own `@{u}` tracking ref points at the branch's own remote once it has been pushed, not at the branch it targets, so anchoring there silently narrows a later run to only the diff since the last push instead of the full accumulated diff. That merge-base diff covers every commit already on the branch plus whatever is currently staged or unstaged, so it never reviews only the latest increment, at any of the moments this skill is invoked from. An empty diff is not the same as nothing to review, and it is never the signal to stop: it reports no untracked file at all, and it reports nothing for content a commit carries that the working tree has since put back. The untracked-file list below covers the first of those. The second is why the diff pass commits before reviewing, the carried-content pass below running against uncommitted content instead, since a removal or a restore that is committed leaves no net content to miss, and why the engine reads HEAD rather than this diff, its change set coming from the merge base against HEAD, the index and the working tree, so the two answer different questions. A fresh review of the full accumulated diff is what catches what per-push review misses, the exact evidence this skill exists to act on. `git diff` never reports a path `git add` has not touched, so a newly created file sitting untracked would otherwise go unread. List it explicitly, `git ls-files --others --exclude-standard`, and read each result in full alongside the diff, the same as any other file the diff touches. @@ -78,6 +82,45 @@ 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. `GOVERNANCE.md` "Verification Discipline" states the rule and why the ordering it corrects is a defect, and is not restated here. What it requires of a run is below. + +**The unit is what a reviewer reads whole**, and `spec/files.json` rather than the document decides which, down to which files carry units at all. `canonical_review.py list` names the whole set and is the authority on it, so the rules are not paraphrased here, where a paraphrase can only drift from them. In the ordinary case a unit is one level-two section of a carried Markdown canonical, and `check` names each one it wants exactly as `record` takes it. 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 the pass above has already done. + +Run it at the same model tier and in the same delegation shape as the pass above. The brief, the engine, its flags, and the point in the sequence where the record is written each differ, and all four are below. + +```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 +git fetch origin # stop and report a failed fetch rather than measuring past it +python3 scripts/canonical_review.py check --target # each uncovered unit, with its digest +# run the pass above over each unit it named, then, per unit: +python3 scripts/canonical_review.py record --reviewer agent-skill --unit '=' [--findings N] +``` + +These run in the authoring repository itself, which is the only repository this pass ever runs in, so the engine path is the plain one and there is no downstream side needing the `/` form the pass above shows for its own reach. 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. + +`` is the branch this work targets, resolved once as the pass above resolves it and passed to `check` explicitly. Left off it defaults to `develop`, so a branch based on `main` is measured from a fork point nobody read. The fetch matters for the same reason it does above: the engine resolves `origin/` if it already exists and never fetches it, so a stale remote-tracking ref moves the fork point without saying so. Lagging, which is the ordinary way to be stale, moves it back and gates units this change never touched, and the reverse case, where the branch restores text the target has since changed, drops one it did move. Neither is announced, so the fetch is what keeps the fork point meaning what the reviewer read against. `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. Fixing a finding is itself such an edit, so `record` then refuses the digest you were holding: that refusal is the content having moved rather than a fault in the record, and the answer is a read of the unit's new text, which is what a carrier will actually receive, recorded at its new digest. + +**This pass records before the commit, where the pass above records after it**, and the two orders are opposite because the two records live in different places. A receipt sits in the worktree's git directory and can never be committed, so it is written once the commit has fixed what a push will deliver. This ledger and its burn-down are tracked files the commit has to carry, so writing them after that commit leaves the tree differing from HEAD, which is a state the pre-push hook refuses before either gate runs. The shortest order meeting both, and the one the refusal table below assumes, is: run this pass and record each unit, commit that together with the change, then read the digest, run the diff pass, record its receipt, and push. Committing the change first and the ledger in a second commit satisfies the same constraint and costs a commit. + +**A unit nothing has read here yet is not this branch's debt.** `check` refuses the units this change moved, meaning the ones whose text it edited and the ones it newly carried, since widening the manifest hands a carrier content for the first time exactly as writing it would. Everything else is a burn-down entry in the hub's `reports/canonical-review.md` rather than a block on unrelated work. 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,19 +130,23 @@ 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, or that newly carries some by widening the manifest, over each unit `check` names, 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. +In the hub, `.husky/pre-push` 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. A blocked push usually means one of those passes was skipped. Both capture points, that hook and the pull request one named below, are the hub's own, and a repository carrying this Skill has neither until one is carried to it, which is what makes the moments above the layer that actually binds everywhere. 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. That route is not open in every seat. A Claude Code session running the fleet's agent-safety hook has the flag denied unconditionally, so where the rows below say a bypass is the answer, the answer in that seat is to report the state and hand the push to the maintainer rather than to force it. The hub's own `.github/actions/validate` composite action runs the canonical-unit half again as a step on every pull request into `main` or `develop`, which is what its workflow triggers on. That one needs no hooks path, runs whether or not any clone enabled one, and `--no-verify` does not reach it, which is what makes it the capture point a push cannot bypass where it applies. -**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. +**Read the refusal itself, which names its own case.** Some of the rows below are cleared by running a pass and some are cleared by nothing of the kind, and each row says which, so no count of either is kept here to go stale against the table. Some the hook decides before either engine runs, so there is no engine message under them, and the rows say where each one's detail comes from. | The refusal says | What it means | What clears it | | --- | --- | --- | -| 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 | -| 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 | +| No local review covers this branch's current content | The ordinary missing pass: no recorded receipt covers what this push delivers, either because none was recorded or because the content moved after one was | One pass over the branch's whole diff, recorded per "Recording the Pass" above | +| 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. The hook prints the same headline for an unresolved merge and for a `git update-index --refresh` that exited above 1, naming each on its own line | Commit what is being pushed, then the pass, then the record. Where the change also moved a canonical unit, follow "The Carried-Content Pass" order instead, since committing first strands that ledger after the commit and each fix then lands on another row. Resolve the merge first where the hook names one, and run `git status` first where it names the refresh, since the content may not differ at all | +| The commit is not this worktree's HEAD | Any pushed branch ref carrying an object id that is neither this worktree's HEAD nor the all-zero id of a delete, which a push from a checkout sitting elsewhere reaches and so does a multi-ref push such as `git push --all` | Push one branch, the one this worktree holds. Where another branch is the one wanted, check it out in its own worktree first, 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, a manifest or ledger the engine could not read, 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 or newly carried, 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, in the order "The Carried-Content Pass" above gives. The ledger that writes is tracked content, so the commit has to carry it and the diff pass comes after | +| A canonical refusal naming units this branch never touched | The fork point is not where the reader thinks it is. Either `origin/` does not hold the commit this branch forked from, since neither engine ever fetches it, or the branch is based on something other than `develop` and the hook, which passes no `--target`, measured it against `develop` regardless. Unlike the row below it still prints a record command, and taking that one records passes over units nobody read | `git fetch origin `, then `canonical_review.py check --target ` by hand for the real set, then pass and record what that names and commit the ledger with the change, per row 5. Where the branch targets something the hook does not measure, no pass clears it, so the gate cannot judge that branch at all and the bypass is its answer, as in the row below | +| The recorded pass was run against X and this check measured Y, printed under row 1's headline | 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 the shapes. That is deliberate: every review round that added a shape also left a restatement of it somewhere else, and keeping one table is what stops the next round doing the same. ## Mechanics Live Elsewhere @@ -108,3 +155,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..7e4bb468 100644 --- a/.github/actions/validate/action.yml +++ b/.github/actions/validate/action.yml @@ -42,6 +42,38 @@ 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: | + set -Eeuo pipefail + # This job checks out at the default depth, and the check measures each unit from the merge-base with the pull request's base, which a depth-1 clone holds neither side of. + # Deepening here rather than raising the job's own fetch-depth keeps the cost with the hub, whose hook this is, rather than on every repository that reuses this workflow. + # The engine reports an unresolvable base as a boundary rather than as a verdict, so without this the step fails loudly instead of passing blind, which is how the gap was found. + if [ "$(git rev-parse --is-shallow-repository)" = true ]; then + git fetch --no-tags --prune --unshallow origin + fi + # The base is an ancestor of the ref this event checks out, so unshallowing normally supplies it, and it is fetched by name where it does not. + if ! git rev-parse --verify --quiet "$BASE_SHA^{commit}"; then + git fetch --no-tags origin "$BASE_SHA" + fi + python3 scripts/canonical_review.py check --target "$BASE_SHA" + + # Read-only: fails where the committed burn-down no longer describes the ledger and the tree. + # Runs on every event, unlike the step above, because staleness is a property of the commit rather than of a comparison against a base. + # `!cancelled()` so an earlier failing step does not skip it, since a run that names one reason and hides the next costs a whole round to discover the second, which is the reason .husky/pre-push runs both gates before reading either verdict. + # Not `always()`, which also runs after a cancellation, where nothing is waiting for the answer. + - name: Check the canonical review burn-down is current step + if: ${{ !cancelled() }} + shell: bash + run: python3 scripts/canonical_review.py report --check + # 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..8e5d12f2 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 @@ -22,7 +26,7 @@ A coding agent that finishes a unit of work, judges it ready, and opens the pull ## What It Does -Dispatches one read-only subagent against this branch's full diff since it forked from its target branch. Resolve `` once, `develop` unless `repo-worktree`'s base-branch rule put this branch on `main` instead, then fetch it, `git fetch origin `, and diff against the merge-base, `git diff "$(git merge-base origin/ HEAD)"`. Stop and report a failed fetch rather than running the merge-base or diff commands anyway: an existing local `origin/` ref can still resolve after a failed fetch, and reviewing against it silently trades the current target for a stale one. Use the same resolved `` in every command below, never a literal `develop` alongside it. Naming the target branch explicitly matters: the branch's own `@{u}` tracking ref points at the branch's own remote once it has been pushed, not at the branch it targets, so anchoring there silently narrows a later run to only the diff since the last push instead of the full accumulated diff. That merge-base diff covers every commit already on the branch plus whatever is currently staged or unstaged, so it never reviews only the latest increment, at any of the moments this skill is invoked from. An empty diff is not the same as nothing to review, and it is never the signal to stop: it reports no untracked file at all, and it reports nothing for content a commit carries that the working tree has since put back. The untracked-file list below covers the first of those. The second is why this skill commits before reviewing, since a removal or a restore that is committed leaves no net content to miss, and why the engine reads HEAD rather than this diff, its change set coming from the merge base against HEAD, the index and the working tree, so the two answer different questions. A fresh review of the full accumulated diff is what catches what per-push review misses, the exact evidence this skill exists to act on. +Dispatches one read-only subagent against this branch's full diff since it forked from its target branch. Resolve `` once, `develop` unless `repo-worktree`'s base-branch rule put this branch on `main` instead, then fetch it, `git fetch origin `, and diff against the merge-base, `git diff "$(git merge-base origin/ HEAD)"`. Stop and report a failed fetch rather than running the merge-base or diff commands anyway: an existing local `origin/` ref can still resolve after a failed fetch, and reviewing against it silently trades the current target for a stale one. Use the same resolved `` in every command below, never a literal `develop` alongside it. Naming the target branch explicitly matters: the branch's own `@{u}` tracking ref points at the branch's own remote once it has been pushed, not at the branch it targets, so anchoring there silently narrows a later run to only the diff since the last push instead of the full accumulated diff. That merge-base diff covers every commit already on the branch plus whatever is currently staged or unstaged, so it never reviews only the latest increment, at any of the moments this skill is invoked from. An empty diff is not the same as nothing to review, and it is never the signal to stop: it reports no untracked file at all, and it reports nothing for content a commit carries that the working tree has since put back. The untracked-file list below covers the first of those. The second is why the diff pass commits before reviewing, the carried-content pass below running against uncommitted content instead, since a removal or a restore that is committed leaves no net content to miss, and why the engine reads HEAD rather than this diff, its change set coming from the merge base against HEAD, the index and the working tree, so the two answer different questions. A fresh review of the full accumulated diff is what catches what per-push review misses, the exact evidence this skill exists to act on. `git diff` never reports a path `git add` has not touched, so a newly created file sitting untracked would otherwise go unread. List it explicitly, `git ls-files --others --exclude-standard`, and read each result in full alongside the diff, the same as any other file the diff touches. @@ -78,6 +82,45 @@ 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. `GOVERNANCE.md` "Verification Discipline" states the rule and why the ordering it corrects is a defect, and is not restated here. What it requires of a run is below. + +**The unit is what a reviewer reads whole**, and `spec/files.json` rather than the document decides which, down to which files carry units at all. `canonical_review.py list` names the whole set and is the authority on it, so the rules are not paraphrased here, where a paraphrase can only drift from them. In the ordinary case a unit is one level-two section of a carried Markdown canonical, and `check` names each one it wants exactly as `record` takes it. 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 the pass above has already done. + +Run it at the same model tier and in the same delegation shape as the pass above. The brief, the engine, its flags, and the point in the sequence where the record is written each differ, and all four are below. + +```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 +git fetch origin # stop and report a failed fetch rather than measuring past it +python3 scripts/canonical_review.py check --target # each uncovered unit, with its digest +# run the pass above over each unit it named, then, per unit: +python3 scripts/canonical_review.py record --reviewer agent-skill --unit '=' [--findings N] +``` + +These run in the authoring repository itself, which is the only repository this pass ever runs in, so the engine path is the plain one and there is no downstream side needing the `/` form the pass above shows for its own reach. 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. + +`` is the branch this work targets, resolved once as the pass above resolves it and passed to `check` explicitly. Left off it defaults to `develop`, so a branch based on `main` is measured from a fork point nobody read. The fetch matters for the same reason it does above: the engine resolves `origin/` if it already exists and never fetches it, so a stale remote-tracking ref moves the fork point without saying so. Lagging, which is the ordinary way to be stale, moves it back and gates units this change never touched, and the reverse case, where the branch restores text the target has since changed, drops one it did move. Neither is announced, so the fetch is what keeps the fork point meaning what the reviewer read against. `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. Fixing a finding is itself such an edit, so `record` then refuses the digest you were holding: that refusal is the content having moved rather than a fault in the record, and the answer is a read of the unit's new text, which is what a carrier will actually receive, recorded at its new digest. + +**This pass records before the commit, where the pass above records after it**, and the two orders are opposite because the two records live in different places. A receipt sits in the worktree's git directory and can never be committed, so it is written once the commit has fixed what a push will deliver. This ledger and its burn-down are tracked files the commit has to carry, so writing them after that commit leaves the tree differing from HEAD, which is a state the pre-push hook refuses before either gate runs. The shortest order meeting both, and the one the refusal table below assumes, is: run this pass and record each unit, commit that together with the change, then read the digest, run the diff pass, record its receipt, and push. Committing the change first and the ledger in a second commit satisfies the same constraint and costs a commit. + +**A unit nothing has read here yet is not this branch's debt.** `check` refuses the units this change moved, meaning the ones whose text it edited and the ones it newly carried, since widening the manifest hands a carrier content for the first time exactly as writing it would. Everything else is a burn-down entry in the hub's `reports/canonical-review.md` rather than a block on unrelated work. 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,19 +130,23 @@ 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, or that newly carries some by widening the manifest, over each unit `check` names, 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. +In the hub, `.husky/pre-push` 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. A blocked push usually means one of those passes was skipped. Both capture points, that hook and the pull request one named below, are the hub's own, and a repository carrying this Skill has neither until one is carried to it, which is what makes the moments above the layer that actually binds everywhere. 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. That route is not open in every seat. A Claude Code session running the fleet's agent-safety hook has the flag denied unconditionally, so where the rows below say a bypass is the answer, the answer in that seat is to report the state and hand the push to the maintainer rather than to force it. The hub's own `.github/actions/validate` composite action runs the canonical-unit half again as a step on every pull request into `main` or `develop`, which is what its workflow triggers on. That one needs no hooks path, runs whether or not any clone enabled one, and `--no-verify` does not reach it, which is what makes it the capture point a push cannot bypass where it applies. -**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. +**Read the refusal itself, which names its own case.** Some of the rows below are cleared by running a pass and some are cleared by nothing of the kind, and each row says which, so no count of either is kept here to go stale against the table. Some the hook decides before either engine runs, so there is no engine message under them, and the rows say where each one's detail comes from. | The refusal says | What it means | What clears it | | --- | --- | --- | -| 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 | -| 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 | +| No local review covers this branch's current content | The ordinary missing pass: no recorded receipt covers what this push delivers, either because none was recorded or because the content moved after one was | One pass over the branch's whole diff, recorded per "Recording the Pass" above | +| 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. The hook prints the same headline for an unresolved merge and for a `git update-index --refresh` that exited above 1, naming each on its own line | Commit what is being pushed, then the pass, then the record. Where the change also moved a canonical unit, follow "The Carried-Content Pass" order instead, since committing first strands that ledger after the commit and each fix then lands on another row. Resolve the merge first where the hook names one, and run `git status` first where it names the refresh, since the content may not differ at all | +| The commit is not this worktree's HEAD | Any pushed branch ref carrying an object id that is neither this worktree's HEAD nor the all-zero id of a delete, which a push from a checkout sitting elsewhere reaches and so does a multi-ref push such as `git push --all` | Push one branch, the one this worktree holds. Where another branch is the one wanted, check it out in its own worktree first, 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, a manifest or ledger the engine could not read, 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 or newly carried, 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, in the order "The Carried-Content Pass" above gives. The ledger that writes is tracked content, so the commit has to carry it and the diff pass comes after | +| A canonical refusal naming units this branch never touched | The fork point is not where the reader thinks it is. Either `origin/` does not hold the commit this branch forked from, since neither engine ever fetches it, or the branch is based on something other than `develop` and the hook, which passes no `--target`, measured it against `develop` regardless. Unlike the row below it still prints a record command, and taking that one records passes over units nobody read | `git fetch origin `, then `canonical_review.py check --target ` by hand for the real set, then pass and record what that names and commit the ledger with the change, per row 5. Where the branch targets something the hook does not measure, no pass clears it, so the gate cannot judge that branch at all and the bypass is its answer, as in the row below | +| The recorded pass was run against X and this check measured Y, printed under row 1's headline | 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 the shapes. That is deliberate: every review round that added a shape also left a restatement of it somewhere else, and keeping one table is what stops the next round doing the same. ## Mechanics Live Elsewhere @@ -108,3 +155,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..4f0be8cc 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 each refusal means and what clears it," >&2 + echo "pre-push: for both gates, and its carried-content section says how the canonical pass itself is run." >&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..fcc758b3 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 compared against the authoring repo's, byte for byte wherever the declared fidelity is verbatim, and a local edit there is drift on the next fidelity check. Where the fidelity is intent the carrier may adapt its own copy, and the defect still has to be fixed at the source, since every other carrier holds it too. 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 what a reviewer reads whole, and the carry manifest rather than the document decides which, down to which files carry units at all, so the engine that reads that manifest is the authority on the set rather than any restatement of its rules. In the ordinary case a unit is one level-two section of a carried Markdown canonical, which is the fidelity unit `spec/section-model.md` declares. The read is of the unit's whole current text rather than of the diff that moved it, and the pass itself, its delegation shape, and its model tier are the `local-strict-review` Skill's, exactly as they are for the pass above. `scripts/canonical_review.py` records each pass keyed on the content the reviewer saw and answers whether one still covers each unit a change moved or newly carried, so a capture point can refuse exactly those rather than trusting the rule to have been remembered. A unit edited today is therefore read today, while a unit nothing has read here yet is left to the burn-down that engine's `report` writes and is never a block on unrelated work. Recording a pass writes two tracked files, that ledger and that burn-down, so where those two files land relative to the commit is a real ordering rather than a preference. Both are committed before the push, since a capture point that gates a push refuses a tree differing from HEAD before it runs either gate, while the diff receipt above is not tracked and is recorded after the last commit instead. So the ledger goes in ahead of the commit that carries it and the receipt is written after that commit, which is why the two records sit on opposite sides of it. Which repos hold such a capture point at all is a separate question, and the rule binds whether or not one is installed. 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 is to run the carried-content pass and record it, commit that together with the change, then review the diff, record its receipt, and push. What the two gates actually require is narrower than that sequence, since the canonical ledger and burn-down are tracked and need only be committed before the push, while the diff receipt is not tracked and must be recorded after the last commit. The sequence above is the shortest one meeting both, which is why every capture point's wording assumes it. 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. `canonical_review.py` is hub-hosted too and is not reached that way at all: it reads `spec/files.json` to learn what is carried, `spec/` is hub-hosted rather than carried, and run against a downstream worktree it exits 2 saying that tree describes no carried set. It runs in the repository that authors the content, which is this one. The canonical-unit half is also run by the hub's own `.github/actions/validate` composite action, as a step 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..14217308 100644 --- a/OPERATIONS.md +++ b/OPERATIONS.md @@ -27,6 +27,8 @@ 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/canonical_review.py report --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 +37,10 @@ python3 spec/validate.py python3 scripts/docker_lint.py ``` +`report --check` is read-only and runs on every event in CI, where the coverage check beside it runs only for a pull request, because a stale burn-down is a property of the commit rather than of a comparison against a base. It also carries `!cancelled()`, so an earlier failing step does not skip it and one run names both verdicts. It fails where the committed report no longer describes the ledger and the tree, which a deleted unit produces while every other gate stays green, since deleting one changes no recorded digest and leaves `check` covered. Renaming a section of a file the manifest carries by name, meaning `AGENTS.md` or `GOVERNANCE.md`, does the same. Renaming one in a file carried whole does not, since `check` then names the new unit and demands a pass for it. `python3 scripts/canonical_review.py report` rewrites it. + +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..09630c03 --- /dev/null +++ b/reports/canonical-review.json @@ -0,0 +1,61 @@ +{ + "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:1bd1396389709aa388ba5b984cd11cc1335bb7b13da107c32bcc336feb816154", + "reviewer": "agent-skill", + "findings": 0, + "hubCommit": "e97708dfe4c92e275118a0705556822993de9f32", + "stamp": "2026-08-31T16:17:12Z" + }, + { + "unit": ".agents/skills/local-strict-review/SKILL.md > What It Does", + "digest": "sha256:2784f7147fcb397f28d475cb2932857cff248344d3b1a6c32547fba1c91c8876", + "reviewer": "agent-skill", + "findings": 0, + "hubCommit": "e97708dfe4c92e275118a0705556822993de9f32", + "stamp": "2026-08-31T16:17:12Z" + }, + { + "unit": ".agents/skills/local-strict-review/SKILL.md > When to Run It", + "digest": "sha256:dd91a8b1631100ffe6a092c87141ae96500c32656fd9a2ef34bd598cddee0502", + "reviewer": "agent-skill", + "findings": 0, + "hubCommit": "e97708dfe4c92e275118a0705556822993de9f32", + "stamp": "2026-08-31T16:44:52Z" + }, + { + "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:0f7d427f2fd507aef0b81e84c3829aa13887838b96647cd37b6cbd7016da50c3", + "reviewer": "agent-skill", + "findings": 0, + "hubCommit": "e97708dfe4c92e275118a0705556822993de9f32", + "stamp": "2026-08-31T16:26:36Z" + } + ] +} diff --git a/reports/canonical-review.md b/reports/canonical-review.md new file mode 100644 index 00000000..f8407ff9 --- /dev/null +++ b/reports/canonical-review.md @@ -0,0 +1,454 @@ +# Canonical content review coverage + +Generated by `python3 scripts/canonical_review.py report`, and never hand-edited. Records are written by `canonical_review.py record` into [`reports/canonical-review.json`][ledger]. Git dates this file. + +A unit is what a reviewer reads whole, decided by the carry manifest rather than by the document. In the ordinary case that is one level-two section of a carried Markdown canonical, and `canonical_review.py list` names the whole set. 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: 288 +- covered: 7 +- stale: 0 +- never read here: 281 + +## Burn-down + +### .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 +- **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..2ed19840 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_