Merged
Conversation
Copilot answers a pull request it will not take on with a formal review,
`state: COMMENTED`, carrying the correct `commit.oid` and zero inline
threads, whose whole body is a refusal. `pr_review.py` treated a review
carrying the head as coverage, so the digest read
pr=108 head=9f56a472 rounds=1 review_on_head=yes threads=0
unresolved=0 suppressed=0 answered_outside_review=no requested=no
merge=CLEAN
which is the clean pass byte for byte, and `wait` exited 0. Observed on a
pull request of 301 changed files, one over the reviewer's limit of 300,
which was one command from merging on a round that never ran.
The existing guard does not reach it. Exit 40 rests on a comment carrying
no commit, so it satisfies no coverage check whatever it says. This
carries the right commit. Only the delivery mechanism differs, and the
mechanism is what the guard keys on.
## The reading
A refusal is not coverage. `reviewed_head` and the digest's
`review_on_head` both exclude it, so the state reports as NO, and the
summary line carries a `refusal=` field of its own, because
`rounds=1 review_on_head=NO` is equally what a stale round looks like and
the two want opposite responses. `wait` exits 41 and the body prints
whole, since the wording is the only thing separating a file-count
refusal, cleared by splitting the pull request, from a quota one, cleared
by waiting. The script reads neither cause.
Head-scoped, unlike a suppressed finding: a refusal is a statement about
one commit that a push retires, and a genuine review of the same head
outranks it. Both surfaces spend it on that coverage, the exit code and
the field alike, or the line reads `review_on_head=yes refusal=YES` and
tells a reader to split a pull request the reviewer has just reviewed.
The liveness query still carries no bodies, so a refusal reads there as
ordinary coverage, which ends the wait, and the full read every wait
finishes with is what tells the two apart. No exit code comes from the
cheaper reading, so the per-poll cost is unchanged.
## The unit is the body's opening line
Matching anywhere in the body reports this pull request as a refusal of
itself, the same false positive the suppressed matcher already had once.
Two lines is not enough either: a review's first line is its heading and
its second is the overview prose, which is exactly where a review
describing this check states the wording. A case pins that, and it is the
case that failed while every other one passed. The cost runs the other
way, that a refusal introduced by a heading would be missed.
The alternation is the runbook's, and a case asserts the script's pattern
is the published one rather than a copy that can drift. The dot spans
both spellings of the apostrophe and keeps the published `--jq` filter
usable inside single quotes, which neither spelling survives.
## Docs
The same guarantee was stated on four surfaces, so each moves: the
runbook's clean-pass rule and its coverage-confirmation paragraph, and
`GOVERNANCE.md` merge-gate condition 2 and review-loop step 3, which now
say the matched review is read rather than only counted. The runbook also
records the reviewer-side fact an agent hitting this needs, that the
limit is 300 changed files and the remedy is to split the pull request.
`GOVERNANCE.md` is verbatim-carried, so a fleet re-vendor is owed.
The issue's second point, `--repo` defaulting to this repository, was
fixed in #575 and needs nothing here.
Fixes #581.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
"Alphabetized by reference name" is ambiguous wherever one label is a prefix of another, and the two readings disagree on that pair alone. Sorting the bare label puts `[governance]` first, while sorting the whole definition line puts `[governance-branching-model]` first, because `-` (0x2D) precedes `]` (0x5D). A block ordered either way is internally consistent, so a reviewer reads the other one as a defect and asks for a re-sort on every pull request that touches a definition block. ## The reading is already in the tree Every reference-definition group this repository carries, twelve of them across ten files, sorts by the bare label. `[governance]` above `[governance-branching-model]`, `[repo-config]` above `[repo-config-settings]`, `[vscode-tasks]` above `[vscode-tasks-python]`. None sorts by the line. The rule's own wording says "by reference name", so the bare label is both what the sentence says and what the corpus does, and the line reading would mean re-sorting `README.md`, `AUDIT.md`, `CODESTYLE.md`, `STANDUP.md`, `TODO.md`, and five more against a rule that never asked for it. `GOVERNANCE.md` now names the key rather than leaving it inferred: the text inside the brackets, never the whole line, so a shorter name sorts above one it prefixes. It also records the trap, which is that a plain `sort -c` over the block passes on the inverted order, since that command compares the line. A block can therefore be mechanically "sorted" and still be wrong, which is how a consistent block reaches review. `spec/readme-structure.md` restates the same rule for a repository that reads the spec without the governance file, so it carries the key too. Leaving it at "alphabetized within each group" would hand the same guess to exactly the reader who has nothing else to check against. ## Two blocks were unsorted under either reading `WORKFLOW.md` and `docs/repo-config-carry.md` each carry a `<!-- Repo -->` group ordered by neither collation. Both are fixed here, so the canonical satisfies the rule it states. ## Verification `prose_lint.py --diff`, `repo_gate.py`, and `markdownlint-cli2` are clean over the change, and a scan of every definition group in the tree reports zero violating the stated order. `GOVERNANCE.md` is verbatim-carried, so a fleet re-vendor is owed. Fixes #583. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The operational lint workflow's prescribed trigger set was push to develop plus pull_request to main, so a pull request opened against develop matched no trigger at all: no validation job, no aggregator report, and a CLEAN mergeable state with an empty check list. That is the exact output D1.2 forbids on any pull request, so the two halves of WORKFLOW.md disagreed. Direct commit to develop is an allowance rather than a substitute for review. The ruleset drops the pull-request requirement, it does not withdraw the pull request, and on the pull-request path the change has not landed yet, which is where validation is worth the most. Widen the prescription to pull_request: branches: [ main, develop ], keeping the push trigger, and record the duplicate run a merged pull request now costs, stated without assuming a squash and with the concurrency key given as the workflow name plus the ref. Add branch.operational.prtriggers as a letter check, since the trigger set is now concrete enough to assert, and note the four downstream operational repos that still need the one-line change. Refs #585 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
GOVERNANCE.md "PR Review Etiquette" told a downstream reader to run scripts/pr_review.py, and scripts/ appears in no spec/files.json baseline entry, so the path is absent from every repository that carries the rule. The cost is measured rather than anticipated: several onboarding sessions built their own worse versions of pr_review.py and prose_lint.py and had to be redirected by hand. A pointer that resolves nowhere teaches the reader that a pointer in carried text is decorative. #580 offered two resolutions, carrying the script or dropping its name, and named a third from TODO.md. Take the third. Add GOVERNANCE.md "Hub-Hosted Tooling" as a verbatim carried section settling the three questions that held the model open. A repository carries what it is audited against and what describes it, and reaches machinery holding nothing per-repo. A tool is named by its path in a hub checkout read at main and fetched immediately before use, its target is named explicitly rather than defaulted, and the working directory stays the tool's own contract. CI reaches the same rules as a composite action pinned to a commit SHA, which is the action-pinning rule unchanged and which .github/actions/prose-gate already implements. An unreachable hub means the tool did not run, reported as not run rather than answered with a reconstruction. The rule that blocked this was "Carried files carry no coordination references", which conflated where a file came from with what its reader runs. Give it a second exception for a hub-hosted tool the reader executes. The audit's template-reference check needs no widening, since the new section defers to AGENTS.md "Fleet Bootstrap" for the destination and GOVERNANCE.md still holds no occurrence of the hub name. De-vendor repo-config/configure.sh, the fleet's only carried script, which holds nothing per-repo and whose last hub fix immediately became a re-vendor owed by every repository carrying it. It resolves every payload path against its own directory, so a run from a hub checkout compares a repo against the hub's payloads and never opens the carried copies. Those copies stay, being what a repo is audited against and what its own AUDIT.md diffs the live rulesets against. A downstream copy is now retired rather than converged, which the divergence ledger says with a new retire disposition. Require the workflow model as an argument wherever the script is invoked, since a run from a hub checkout finds the registry and resolves an unregistered repo through defaults.workflowModel to release, which would apply the release develop ruleset to an operational repo. Repoint the two CODESTYLE.md definitions into hub-only catalog/, dead in every downstream copy since catalog/ became hub-only, and separate the two files they conflated: a repo carries the .NET task definitions in its own .vscode/tasks.json, and the hub snippet is the canonical it is written against. Refs #580 Refs #559 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Promotes recent develop changes into main, tightening the PR-review automation and fleet governance around (1) recognizing Copilot “refusal” reviews, (2) defining reference-link collation, (3) ensuring operational CI runs on PRs into develop, and (4) formalizing hub-hosted tooling vs carried content.
Changes:
- Extend
scripts/pr_review.py(and its test suite/docs) to treat Copilot “unable to review” formal reviews as non-coverage with a dedicated digest field and exit code. - Clarify and enforce reference-link definition ordering semantics (sort key is the bracketed reference name).
- Update operational workflow guidance/spec to require
pull_requesttriggers on bothmainanddevelop, and introduceretireas a divergence disposition to support hub-hosted tooling.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| WORKFLOW.md | Document PR path into operational develop and require PR triggers on both branches. |
| TODO.md | Update backlog to reflect hub-hosted tooling decisions and downstream trigger sweep. |
| STANDUP.md | Update standup instructions to run configure.sh from a hub checkout with explicit args. |
| spec/validate.py | Allow new retire disposition in divergence ledger validation. |
| spec/section-model.md | Add “Hub-Hosted Tooling” as a verbatim-carried governance section. |
| spec/readme-structure.md | Clarify reference-definition sort key as the bracketed reference name. |
| spec/project-types.json | Add letter check for operational PR trigger shape (branch.operational.prtriggers). |
| spec/files.json | Add carried GOV section for “Hub-Hosted Tooling”; stop requiring repo-config/configure.sh. |
| spec/fidelity_honesty.py | Add retire to disposition ordering for reporting. |
| spec/divergences.schema.json | Extend schema to include retire disposition. |
| spec/divergences.json | Move repo-config/configure.sh to a retire gap entry. |
| scripts/test_pr_review.py | Add refusal-detection test coverage and runbook-pattern pinning tests. |
| scripts/README.md | Document hub-hosted tooling model and new refusal exit code behavior. |
| scripts/pr_review.py | Implement refusal detection, digest reporting, and wait exit code 41. |
| repo-config/README.md | Reframe configure.sh as hub-hosted and clarify operational PR allowance. |
| OPERATIONS.md | Update operational runbooks to reflect hub-hosted tooling usage. |
| GOVERNANCE.md | Add “Hub-Hosted Tooling”; update operational PR guidance and doc/link rules. |
| docs/repo-config-carry.md | Update carry/apply guidance for hub-hosted configure.sh. |
| CODESTYLE.md | Repoint VS Code task snippet links to hub URLs and update guidance text. |
| AGENTS.md | Add router row for “Hub-Hosted Tooling”. |
| .github/copilot-instructions.md | Document the refusal-review exception and matching strategy. |
#586 named the collation for reference-definition sort order as the text inside the brackets, and #588 then inserted into two blocks without applying it. Found by Copilot on the promotion pull request #589, which is blocked on both threads. ## The two blocks - `CODESTYLE.md` appended the two `vscode-tasks` links after `ruff-link` when it repointed them into hub-only `catalog/`, which stranded `uv-link` below them. `uv-link` belongs between `ruff-link` and `vscode-tasks-link`. - `docs/repo-config-carry.md` put `governance-hub-hosted-tooling` at the head of a `<!-- Repo -->` group #586 had itself just sorted, above `governance-documentation-style` and `governance-git-and-commit-rules`. Both are pure reorderings. No link target, anchor, or reference name changes, so no rendered output changes. ## Why a mechanical check did not catch it This is exactly the trap #586 recorded when it named the key. A plain `sort -c` over either block compares the whole line rather than the bracketed label, and both blocks pass that reading, so a "sorted" block reaches review looking consistent. Nothing in `repo_gate.py` or `prose_lint.py` asserts the order today, which is why the reviewer is the only surface that flagged it. ## Verification A sweep over every reference-definition group in every markdown file in the tree, keyed on the bare label, reports these two groups and no others before the change, and zero after it. `scripts/prose_lint.py --diff origin/develop` and `scripts/repo_gate.py` are both clean. Refs #583 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Promotes the five commits
develophas carried since the previous promotion (#582) tomain. Merge commit only, no squash, anddevelopis not deleted.What lands
commit.oidand zero inline threads, whichpr_review.pyscored as a clean pass, sowaitexited 0 on a round that never ran.reviewed_headandreview_on_headnow exclude it, the digest carries its ownrefusal=field,waitexits 41 and prints the body whole, and the matcher is scoped to the body's opening line so a review describing the check is not a refusal of itself.GOVERNANCE.mdandspec/readme-structure.mdnow name the key as the text inside the brackets, record that a plainsort -cpasses on the inverted order, and the two blocks that were unsorted under either reading are fixed.develop. The prescribed trigger set matched no trigger for that pull request, producing the empty check list D1.2 forbids, so the two halves ofWORKFLOW.mddisagreed. The prescription widens topull_request: branches: [ main, develop ], the duplicate run is recorded, andbranch.operational.prtriggersis added as a letter check.GOVERNANCE.md"Hub-Hosted Tooling" section settles how a carried rule names a tool the reader executes, the coordination-reference ban gains a second exception for that case,repo-config/configure.shis de-vendored with a newretiredisposition in the divergence ledger, the workflow model becomes a required argument so an unregistered repo cannot resolve to the release ruleset, and the twoCODESTYLE.mddefinitions are repointed out of hub-onlycatalog/.Issues closed
None remain open. #581, #583, and #585 were closed by their constituent pull requests, and #580 and #559 were hand-closed with evidence when #588 merged, so this promotion carries no closing keyword.
Fleet debt this promotion creates
GOVERNANCE.mdis verbatim-carried and three of these commits change it, so a fleet re-vendor is owed — now including an entirely new carried section. #588 additionally converts every downstreamrepo-config/configure.shcopy into a deletion rather than a convergence, and #587 leaves the one-line trigger change owed by four operational repos.Nothing mechanical asserts the definition sort order that #590 had to fix by hand. Neither
repo_gate.pynorprose_lint.pychecks it, and as #586 recorded, a plainsort -ccompares the whole line and passes on the inverted order, so the reviewer was the only surface that caught it. A check keyed on the bare bracketed label belongs inrepo_gate.py.Verification
git merge-tree --write-tree origin/main origin/developreports no conflict, and the tree it writes is byte-identical toorigin/develop, so this promotion needs no resolution branch. A sweep over every reference-definition group in every markdown file ondevelop, keyed on the bare bracketed label, reports zero violations. All five constituent pull requests merged with their review loops closed.