Read what a reference points at, rather than what it looks like - #602
Conversation
Two gates of the same shape, each confirming that a reference in a committed artifact still resolves. Both failures are silent, and both are caught by a reviewer or not at all. `sha-pin` resolved a pin's shape and never the pin. Forty hex characters is a format any fabricated string satisfies, and an agent hand-writing a plausible SHA into a workflow is a failure this repo has seen. Resolving also catches the neighboring case, a pin whose commit was reachable only from a branch since squashed and deleted, which breaks a downstream gate long after the change that caused it. The `gh-write-guard` hook cannot cover either, since it watches Bash and an editor tool writing the same string into a file never reaches it. Resolution is scoped to the scanned repository's own owner, which is where the fleet's actions live and where that decay comes from. A third-party tag is stable by comparison, and reading one would make every local run depend on a stranger's repository answering. The cost is real: a fabricated pin on a third-party action is still only shape-checked. So every run now prints what the pass actually covered, because a scope that resolves nothing prints the same `0 issue(s)` a full sweep does. On the hub that is zero today, since all 45 pinned refs here are under other owners and the `ptr727` pins live in the downstream repos that consume `prose-gate`, reached by `--root`. A pin is a finding only where GitHub answered absent, meaning 404 or 422. An offline host, a missing `gh`, a rate limit and a narrow token report as unread and leave the pin on its shape. A 404 is confirmed against the repository itself first, since an absent commit and an unreadable repository are the same answer from here and a repository-scoped CI token is narrower than the fleet. `pr_review.py claims` reads a description against the branch it describes. Three stale descriptions in one session generated six review findings between them. Its first design scanned the body for bare SHAs, and the corpus rejected that outright. Over the 25 most recent merged pull requests it raised four findings and every one was correct prose: a `develop` commit named as history, a SHA inside a pasted digest, and two commits in another repository written without a URL. Nothing in the shape of a bare SHA separates those from a real claim, and separating them by meaning is the similarity heuristic `spec/section-model.md` already rules out. A path arm measured on the same corpus is worse, flagging 54 of 215 backticked candidates. What survives is the verb: a commit counts only where the body says it was fixed, landed, shipped, added, introduced, corrected, resolved, carried or amended in it. That raises exactly one reference over the same 25, and that one is true. It is an inclusion list, so a phrasing nobody thought of costs a detection rather than inventing a finding. A commit passes on ancestry, so a description may cite one it inherited from the base branch, and the two failing shapes are named apart: a commit the repository does not carry at all, and one this head does not descend from. The `uses:` refs are matched against the whole tree at head, as one archive rather than a request per file, because this repo carries `uses:` lines in snippets and docs as well as under `.github/`. Undecided is a third answer here too, and where every reference is undecided the run exits 71 rather than printing the `stale=0` a clean read prints. TODO.md: the cluster is deleted, and the one thing it surfaced that this does not carry becomes a `decision` cluster, whether the hub should consume its own `prose-gate` action so its pins have something under this owner to resolve against. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR strengthens two governance-style gates that previously validated only the shape of references (e.g., “40 hex chars”) by adding logic/tests to actually resolve what those references point to (or explicitly report when resolution could not be performed), and updates backlog/docs accordingly.
Changes:
- Enhance
repo_gate.pysha-pinto resolve same-owneruses:SHA pins via GitHub (while preserving “offline/unreadable” as a non-failing, explicitly noted outcome). - Add a new
pr_review.py claimssubcommand to validate that a PR description’s claimed commits and quoteduses:refs still match the PR head. - Expand unit tests and documentation to cover the new behaviors and remove the completed TODO cluster.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| TODO.md | Removes the completed “read what a claim points at” cluster and replaces it with a follow-up decision item. |
| scripts/repo_gate.py | Adds GitHub-backed resolvability checks for same-owner pinned uses: SHAs plus per-check “note:” reporting. |
| scripts/test_repo_gate.py | Adds test coverage for resolvability behavior, caching, and HTTP-status interpretation. |
| scripts/pr_review.py | Adds claims subcommand to compare PR-description references against the PR head (commits + uses: refs). |
| scripts/test_pr_review.py | Adds coverage for body reference extraction and claims behavior across stale/unread/clean cases. |
| scripts/README.md | Documents the new sha-pin resolvability behavior and the new claims command. |
`head_carries` reads bytes, so it cannot go through `gh_rest`, and it was left without that helper's two guards: an absent `gh` or a hung download raised out of `claims` rather than reading as undecided, which is what every other unreadable answer in the subcommand does. It now catches both and bounds the download, which is longer than an ordinary read because it fetches a repository rather than a field. Three cases lock it, including one holding the call to actually passing a timeout. The `sha-pin` note merged two states that are not the same. A checkout with no readable `origin` skips every pin, this owner's own included, and the note reported all of them as `under another owner`. That is the false clean the note exists to prevent, one level in. The two are counted apart now and the line carries one fixed shape every run, so a zero reads as plainly as a count. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (3)
scripts/test_pr_review.py:994
HEREis defined but never used, which adds noise and can confuse future edits (it looks like it should scope the tests but it does not).
HERE = 'ptr727/ProjectTemplate'
PIN = 'actions/checkout@' + '9' * 40
scripts/repo_gate.py:118
- The
check_sha_pin()docstring says "Every workflowuses:is a 40-hex SHA", but the implementation explicitly skips localuses:refs (e.g../.github/...). Tightening this wording avoids implying local refs must be pinned/resolvable.
"""Every workflow `uses:` is a 40-hex SHA, and one under this owner is a SHA that resolves.
scripts/README.md:81
- This summary bullet says every workflow
uses:resolves, butsha-pinintentionally skips localuses:refs (and separately has a documented exception). Clarify the scope here so the README matches the check’s actual behavior.
- `sha-pin`: every workflow `uses:` is a 40-hex commit SHA that resolves, with the one documented `dotnet/nbgv@master` exception allowed.
`HERE` went dead when `body_references` stopped taking the owner and name, and a constant that looks like it scopes the cases while scoping nothing is worse than no constant. Both scope statements overclaimed. `check_sha_pin` skips a `uses:` that names a local reusable workflow, since that names no action and carries no ref to pin, and the docstring and the README bullet both read as though every `uses:` must be a resolvable pin. Neither behavior changes, only what each says it does. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Answering the three suppressed findings from round 2, all three accepted and fixed in
Correct. It went dead when the corpus pushed
Both correct and both the same overclaim, so they are fixed together. The docstring now reads "Every |
Clears the `A Home for a Disproved Finding` cluster from `TODO.md`. Its `Checked` anchor was re-verified against `develop` at `756a53e` before anything was written: the file still says the reviewer is sometimes factually wrong, still requires a decline to carry evidence, and still keeps its list of known non-working request paths, with nowhere to put the proof itself. ## The gap A decline that carries proof proves something about this tree, and [GOVERNANCE.md "Every Finding Ends in an Action"](https://github.com/ptr727/ProjectTemplate/blob/develop/GOVERNANCE.md#every-finding-ends-in-an-action) is right that the thread is where it belongs while the pull request is open. Afterwards the thread is the wrong place. The pull request merges, the next round starts with no memory of the last, and the second occurrence of the same claim reaches a maintainer with no way to tell it from a first. `Disproved Claims` is a new `###` under the runbook, so the three declared `##` sections in `spec/files.json` are unchanged. An entry names the claim, what was run or read to disprove it, the revision it was proved against, and what ends it. **It is deliberately not a list to append to**: an entry outliving the code it was proved against becomes a reason not to check, which is strictly worse than proving the claim again, so an entry whose subject moves is deleted by the change that moves it rather than edited to look current. Two guards ride with it. The record answers a repeated claim and never dismisses a new one, so a finding is judged on its merits first and matched second, and a reply carries the proof re-read rather than a pointer to a file the reviewer cannot open. And the entries are this repository's own, so a repository holding a copy carries the shape and deletes an entry whose subject it does not carry. ## The three worked examples **`keys_unsorted` requires jq 1.6.** A suppressed finding on #555 against the normalizer in `repo-config/configure.sh`, reasoning by analogy from the `walk/1` failure #553 fixed. Re-run in this session rather than quoted from the thread, on `jq-1.5-1-a5b5cbe` in `ubuntu:18.04`, the build that reproduces `walk/1`: ```console jq-1.5-1-a5b5cbe $ echo '{"b":1,"a":2}' | jq -c 'keys_unsorted' ["b","a"] $ echo '{"b":1}' | jq -c 'walk(.)' jq: error: walk/1 is not defined at <top-level>, line 1: jq: 1 compile error ``` **The write-guard's fallback parse, from #601.** Declined on the ground that the arm cannot execute, since `punctuation_chars` arrived in Python 3.6, the module uses f-strings throughout, and `install.py` refuses below 3.7. That is exactly the kind of disproof that expires, which is why the entry names the floor as what ends it. It also records that the finding earned a test case rather than a change, since only `ValueError` from unbalanced quoting reaches that path in practice and nothing covered it. **The bare-SHA design, from #602.** This one came from this repository's own backlog rather than from a reviewer, and it is here because a rejected method costs the same to re-propose as a declined finding costs to re-derive, while a backlog has a place for a claim the tree contradicts and none for a method a measurement rejects. Over the 25 most recent merged pull requests the bare-SHA arm raised four references and all four were correct prose, and a path arm flagged 54 of 215 backticked candidates. Both #601 and #602 are folded under disposition **Amends "Record what was tested and against which revision, and delete an entry whose subject changes."** They land as the second and third worked examples in the shipped section rather than as `Settled` bullets, because the entry ships in the same change. ## Verification - `python3 scripts/prose_lint.py . --diff develop` clean, `markdownlint-cli2` clean on both files. - `python3 -m unittest discover -s scripts` reports 372 tests OK and `python3 spec/audit.py --selftest` passes, neither touched by this change but both run because the file is carried. - Every factual claim in the new section was read against the tree at `756a53e`: the `keys_unsorted` call in `repo-config/configure.sh`, `_git_subcommand_arglists` and `_PUNCTUATION_CHARS` in `gh-write-guard.py`, the `(3, 7)` floor in `install.py`, and the verb alternation in `pr_review.py`. ## TODO.md The cluster is deleted per the file's own step 9. What this change does not carry becomes a new `ready` cluster: `GOVERNANCE.md` outcome 2 ends at the thread and nothing agent-agnostic points at the record, so an agent that never opens the provider runbook posts a decline and has nowhere to put the proof. That is a byte-locked carried section and a fleet re-vendor, which is why it is a separate change rather than a second surface in this one. ## Downstream `.github/copilot-instructions.md` is carried `whole` at `intent` with `appliesTo: "*"`, so this joins the re-vendor the ledger already lists. A repository taking it carries the section and its rules, and starts its own entries empty. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
… that blocks on what it cannot read (#609) Promotes 14 commits from `develop`. Merge with a **merge commit** (`gh pr merge --merge`), never a squash, and **without `--delete-branch`**, since this pull request's head is `develop` itself. Closes #607 through the closing keyword already carried in `530cf71`, which is why it is not repeated here. ## The prose backlog, cleared end to end `#600`, `#604`, `#605`, `#606` took the tree from **557 findings across 45 files to 0 across 0**, in four batches ordered by surface: snippets, comments, hub-only Markdown, then the carried files. Each batch measured the checker's own exemption against the live corpus *before* sweeping, and twice the measured answer was **"do not change the checker"**, which is a result of that pass rather than a skipped one. `#594` added the floor that makes those numbers trustworthy: a diff-scoped run now asserts what it actually scanned, since a check whose scan matches nothing reports zero findings and reads exactly like a pass. One finding from that work is worth carrying up: an exemption that is too **loose** produces silence rather than false positives. #519 recorded the governance files as clean; today's checker reports 38 findings against those same files as they stood at the commit that measured them. ## A review loop that fails closed `#599`, `#601`, `#602`, `#603` and `#608` are one arc on `scripts/pr_review.py`, each removing a shape in which the loop reported a clean pass over a review it had misread: - **`#599`** removed the shape a reply kept failing in, by taking the thread's *words* rather than an id, so there is no argument a hand-typed `PRRT_...` fits in. - **`#602`** made `claims` resolve what a description points at rather than what it looks like. - **`#603`** gave a disproved claim a home the next round reads. - **`#608`** reads the file-coverage line, and then generalizes: every reader keys on a structural marker, so a marker that changes spelling is a section the reader stops finding and reports as absent. The digest now vets headings, `<summary>` texts, metadata labels, coverage wordings and the reviewer login against an inventory measured from **332 review bodies**, and **blocks on anything outside it**, exit `43`, with the remedy stated as filing an issue on the hub. Whether to merge regardless is the maintainer's decision. `GOVERNANCE.md` merge gate went from four preconditions to **five** accordingly. ## Governance and tooling - **`#593`** states which checkout an agent works in and what the hub is, which is the host-wide routing the repositories that most need it cannot carry. - **`#596`** gates the pattern-detectable half of the representative-data rule, honest that no pattern closes the name-shaped case. - **`#598`** declares where a repository states what CI cannot verify. - **`#592`** regrouped `TODO.md` by what ships rather than by what it touches, so a `###` heading is one pull request. - **`#601`** ended a `gh push` argument list at a newline rather than only at `&&`, fixing a write-guard over-block. ## Verification Run on `develop` at `530cf71` immediately before opening this: `test_pr_review.py` (174), `test_prose_lint.py`, `test_repo_gate.py`, `spec/audit.py --selftest`, `gh-write-guard.py --selftest`, `spec/validate.py`, `repo_gate.py`, the prose gate in both CI invocations, markdownlint and editorconfig-checker. All clean. ## Not carried by this promotion - **#519 is complete and still open.** `TODO.md` holds its closing evidence under "Verified Complete, Awaiting Close". Closing it is the maintainer's call, so no keyword for it appears here. - **The re-vendor debt is now nine files.** `#606` queued seven, and `#608` changed `GOVERNANCE.md` "PR Review Etiquette" (`verbatim`) and `.github/copilot-instructions.md` (`intent`) on top. The `intent` half produces no hash and therefore no audit finding, which is why the Fleet Sweeps entry names those files by hand. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Two gates of the same shape, each confirming that a reference in a committed artifact still resolves. Both failures are silent, and both are caught by a reviewer or not at all. This clears the
Two Checks That Read What a Claim Points Atcluster fromTODO.md, whose twoCheckedanchors were re-verified againstdevelopatdbd1cdcbefore anything was written:check_sha_pinstill matched^[0-9a-f]{40}$and never read the ref, and no subcommand read a description at all.sha-pinresolves the pin, not just its shapeForty hex characters is a format any fabricated string satisfies, and an agent hand-writing a plausible SHA into a workflow is a failure this repo has seen. Resolving also catches the neighboring case, a pin whose commit was reachable only from a branch since squashed and deleted, which breaks a downstream gate long after the change that caused it. The
gh-write-guardhook cannot cover either, since it watches Bash and an editor tool writing the same string into a file never reaches it.Resolution is scoped to the scanned repository's own owner, which is where the fleet's actions live and where that decay comes from. A third-party tag is stable by comparison, and reading one would make every local run of the gate depend on a stranger's repository answering. The cost is real and is stated rather than hidden: a fabricated pin on a third-party action is still only shape-checked. So every run prints what the pass actually covered, because a scope that resolves nothing prints the same
0 issue(s)a full sweep does. On the hub that is zero today, since all 45 pinned refs here are under other owners, and theptr727pins live in the downstream repos that consumeprose-gate, reached by running this gate from a hub checkout with--root.A pin is a finding only where GitHub answered absent, meaning a 404 or a 422. An offline host, a missing
gh, a rate limit and a narrow token all report as unread and leave the pin on its shape, so the gate stays usable with no network instead of failing a correct tree. A 404 is confirmed against the repository itself before it becomes a finding, since an absent commit and an unreadable repository are the same answer from here and a repository-scoped CI token is narrower than the fleet.pr_review.py claimsreads a description against its own branchThree stale descriptions in one session generated six review findings between them.
Its first design scanned the body for bare SHAs, and the corpus rejected that outright. Over the 25 most recent merged pull requests it raised four findings and every one was correct prose: a
developcommit named as history, a SHA inside a pasted digest, and two commits in another repository written without a URL. Nothing in the shape of a bare SHA separates those from a real claim, and separating them by meaning is the similarity heuristicspec/section-model.mdalready rules out, which is the same reason the entry put prose claims out of scope. A path arm measured on the same corpus is worse, flagging 54 of 215 backticked candidates, nearly all bare basenames,origin/develop, and other repositories.What survives is the verb. A commit counts only where the body says it was fixed, landed, shipped, added, introduced, corrected, resolved, carried or amended in it. That alternation raises exactly one reference over the same 25, and that one is true. It is an inclusion list, so a phrasing nobody thought of costs a detection rather than inventing a finding, and a claimed SHA still carries a digit as a backstop on the list growing later.
A commit passes on ancestry, so a description may cite one it inherited from the base branch, and the two failing shapes are named apart: a commit the repository does not carry at all, and one this head does not descend from. The
uses:refs are matched against the whole tree at head, as one archive rather than a request per file, because this repo carriesuses:lines in snippets and docs as well as under.github/. Undecided is a third answer here too, and where every reference is undecided the run exits71rather than printing thestale=0a clean read prints.Evidence
python3 -m unittest discover -s scriptsreports 367 tests OK, up from 340.claimsrun over the 25 most recent merged pull requests reportsstale=0on every one, with one commit reference actually read and checked.TODO.md
The cluster is deleted per the file's own step 9. The one thing it surfaced that this does not carry becomes a new
decisioncluster: whether the hub should consume its ownprose-gateaction, so its pins have something under this owner to resolve against. That changes what a workflow runs rather than what a gate reports, so it is the maintainer's call rather than this pull request's.🤖 Generated with Claude Code