Host the fleet tooling in the hub instead of carrying copies - #588
Conversation
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. None of them did anything wrong. They were pointed at a tool absent from their tree by a document that never says how to reach it, and 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. An agent reads hub main from a checkout, fetched immediately before use, and names the hub commit in any report, where 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. The ruleset payloads stay carried, being what a repository 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. Repoint the two CODESTYLE.md definitions into hub-only catalog/, dead in every downstream copy since catalog/ became hub-only. CODESTYLE.md:364 asked each repository to re-point one of them at its own tasks.json, which is what left the reference resolving differently in every copy. Refs #580 Refs #559 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Several updated docs describe behavior that does not match current tooling defaults (model selection when a repo is not in the registry, and when prose-gate uses bundled vs hub-develop rules), creating a risk of incorrect configuration or misinterpreted gate provenance.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR updates the fleet governance and supporting spec/docs to formalize a “hub-hosted tooling” model: downstream repos carry the content they are audited against, but run shared tooling from a hub checkout (or via pinned composite actions in CI), reducing stale copies and re-vendor churn.
Changes:
- Add
GOVERNANCE.md“Hub-Hosted Tooling” and route it fromAGENTS.md, plus update “PR Review Etiquette” to reference hub-hostedscripts/pr_review.py. - De-vendor
repo-config/configure.shfrom the carried baseline and update standup/config docs to run it from the hub. - Add a
retiredivergence disposition and update the divergence ledger/schema + ordering logic.
File summaries
| File | Description |
|---|---|
| TODO.md | Updates planning entries to reflect the new hub-hosting model and links prose-gate. |
| STANDUP.md | Updates standup procedure to run repo-config/configure.sh from a hub checkout. |
| spec/validate.py | Accepts retire as a valid divergence disposition. |
| spec/section-model.md | Declares the new “Hub-Hosted Tooling” section. |
| spec/files.json | Adds “Hub-Hosted Tooling” section and removes repo-config/configure.sh from the baseline. |
| spec/fidelity_honesty.py | Includes retire in disposition ordering/report buckets. |
| spec/divergences.schema.json | Extends schema enums to include retire. |
| spec/divergences.json | Moves repo-config/configure.sh handling to a retire gap entry and updates disposition vocabulary. |
| scripts/README.md | Documents hub-hosted scripts and CI consumption via prose-gate. |
| repo-config/README.md | Documents configure.sh as hub-hosted (payloads carried, script reached). |
| OPERATIONS.md | Reframes ops model around hub-run gates/config tooling. |
| GOVERNANCE.md | Adds “Hub-Hosted Tooling” section and updates coordination-reference exception + PR review guidance. |
| docs/repo-config-carry.md | Updates repo-config carry/apply procedure to hub-host configure.sh. |
| CODESTYLE.md | Repoints VS Code tasks references to hub canonical snippet URLs. |
| AGENTS.md | Adds a router row for “Hub-Hosted Tooling”. |
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 4
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Four review findings, all confirmed against the code. configure.sh resolves the model from the registry beside it and falls through defaults.workflowModel to release for a repo the registry does not name, and the carried-payload inference runs only where there is no registry file at all. Prescribing a run from a hub checkout therefore removes that inference, so STANDUP.md and docs/repo-config-carry.md described a fallback that a run from the hub never reaches, and a repo mid-standup is exactly the one the registry has not got yet. Pass the model as an argument and say why. The prose-gate action selects hub develop for every target that is not main, a feature-branch push included, rather than for a develop-targeted run alone, and rules-ref overrides both. No hub tool prints the commit it ran from, so state the report obligation as the runner's rather than as something the tools already emit. Refs #580 Refs #559 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The updated docs contain at least one inconsistent command example (STANDUP check still shows an optional model) and an ambiguous execution-context sentence in GOV that should be aligned before merge.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (2)
STANDUP.md:122
- This line still documents
check owner/repo [release|operational]with the model optional, but the surrounding guidance explicitly says to pass the model because an unregistered operational repo will otherwise default torelease. Make thecheckexample require the model as well for consistency and to avoid copy/paste applying the wrong develop ruleset.
Run `repo-config/configure.sh apply owner/repo release|operational` from a hub checkout, naming the repo being stood up and its model, to apply the fleet settings, the Dependabot security features, and the two rulesets idempotently (import the JSON, never hand-build it, per [`docs/repo-config-carry.md`][repo-config-carry]), then `repo-config/configure.sh check owner/repo [release|operational]` to validate the repo and exit non-zero on any drift. The script is hub-hosted rather than carried, so the repo being stood up holds no copy of it and never needs one, and naming the target is what keeps the write off the checkout the command runs in. Pass the model explicitly here rather than relying on the lookup. Run from a hub checkout the registry is present, so a repo not yet registered resolves through `defaults.workflowModel` to `release` and applies the wrong `develop` ruleset to an operational repo, and a repo being stood up is exactly the one the registry has not got yet. Reconcile its registry entry in step 6 either way. Configure every required secret per [`spec/secrets.json`][secrets] (the registry `requiredSecrets[]` list plus the implicit baseline) in the right store(s), meaning Actions plus Dependabot where the mechanism needs it, and confirm no forbidden secret is present. The required check binds by name (`Check pull request workflow status job`) and turns green only after the PR workflow has run once, which is why this step follows step 3 rather than preceding it. A ruleset requiring a name no run has ever reported leaves the first pull request waiting on a status nothing produces, and on an operational repo the `develop -> main` promotion is a pull request too, so the same wait applies there.
GOVERNANCE.md:125
- This sentence says to run hub-hosted tooling "from the repository being worked on" while also describing reaching it via a hub checkout. Elsewhere (STANDUP.md, OPERATIONS.md, repo-config docs) the model is to run from a hub checkout and always pass the target repo explicitly; align this wording so the execution context is unambiguous.
**Reaching it is a checkout of the hub rather than a copy of one file.** A tool reads the payloads, tables, and sibling modules beside it, so a single file lifted out of the tree runs against whatever the caller happens to have, which is the copy problem again in a shorter loop. Read `main`, the promoted and gated state, and fetch immediately before running, because a clone is whatever it last fetched rather than the branch it names, and a stale clone answers confidently instead of failing. Run from the repository being worked on, naming the tool by its path in the hub checkout and the target repository by the tool's own argument, since a tool that defaults to the current directory or the current repository resolves somewhere either way and a result computed against the wrong one is well-formed.
- Files reviewed: 15/15 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Two suppressed findings, both real. The check example in STANDUP.md still bracketed the model as optional on the same line whose prose says to pass it, and repo-config/README.md bracketed both arguments in both examples. A bracketed example is what gets copied, so the brackets undercut the rule stated beside them. The parser genuinely does take a lone model positional, which is now stated as the hazard it is rather than as a shortcut, since it resolves against whichever repository the shell is in instead of erroring on a missing target. "Run from the repository being worked on" contradicted every other site, which says run from a hub checkout. Both are right for different tools: a gate reading a working tree runs in the repository under test, while a tool taking its target as an argument runs anywhere. The working directory is therefore the tool's contract rather than a rule here, and what the rule forbids is a default deciding which repository the answer is about. Refs #580 Refs #559 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Answering the 2 suppressed findings from the round-2 review, which raise no thread. Both are real and both are fixed in
Fixed in That file also ended with
Fixed in The rule now separates the two. Naming the tool by its hub path and naming the target explicitly is the rule, the working directory is the tool's own contract rather than something this section dictates, and what the rule forbids is a default deciding which repository the answer is about. That is the invariant both tools actually share, and it is the one the All gates re-run green on Correction to an earlier revision of this comment. The review link carried a review id that was written rather than read, so it pointed at nothing. The id above is the round-2 review on head |
There was a problem hiding this comment.
🟡 Changes recommended
A couple of updated docs use wording that can mislead readers about what is actually “carried” vs hub-canonical, and should be clarified for correctness.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (2)
repo-config/README.md:7
- This sentence says “the script that reads them”, which can be read as configure.sh consuming the repo’s carried payloads. In reality, configure.sh reads payloads from its own checkout (script_dir), so it’s clearer to avoid implying it reads the carried payloads in the target repo.
- `configure.sh`: **hosted in the hub and run from a hub checkout, not carried here**, per [GOVERNANCE.md "Hub-Hosted Tooling"][governance-hub-hosted-tooling]. The payloads above are what this repo is audited against and stay with it, while the script that reads them holds nothing per-repo and is one copy for the fleet. Name the target repository explicitly, since the command defaults to whichever repository the shell is sitting in. Two modes over the GitHub API. `configure.sh apply owner/repo release|operational` creates-or-updates the settings, the Dependabot security features, and the rulesets idempotently (a full-payload update). `configure.sh check owner/repo release|operational` is the read-only inverse and exits non-zero on any drift, with the ruleset and settings assertions driven by the committed payloads so they stay repo-agnostic (rule presence, merge methods, and required checks, not a byte diff, so a GitHub-normalized stored ruleset does not false-positive). The command defaults to `apply`, the repo to the current one, and the model to the registry `workflowModel` lookup, falling back to inference from the carried `develop` payload where there is no registry to consult and aborting rather than guessing on an ambiguous layout. Pass the model rather than leaving it to the lookup. A hub checkout has the registry, so a repo the registry does not yet name resolves through `defaults.workflowModel` to `release`, which applies the release `develop` ruleset to an operational repo. Both arguments are optional to the parser rather than to the caller, which is the hazard: a lone positional naming a model is read as the model, so `configure.sh check operational` resolves against whichever repository the shell is in rather than erroring on a missing target.
CODESTYLE.md:56
- The wording “carried verbatim” is misleading here now that the link points to the hub catalog snippet rather than a file carried in each downstream repo. Also, the parenthetical says “tasks.json is the canonical command spec”, but this bullet now points readers at the hub snippet as the canonical source for the task chain.
- The .NET clean-compile is the **`.NET Format`** VS Code task, which chains `CSharpier Format` -> `.NET Build` -> `dotnet format style --verify-no-changes`. These three task definitions are carried verbatim in [`vscode-tasks.json`][vscode-tasks-link].
- After any code change it must pass before commit. Run the `.NET Format` task. To run it natively instead, reproduce that task chain from [`vscode-tasks.json`][vscode-tasks-link] exactly (`CSharpier Format`, then `.NET Build`, then the `dotnet format style --verify-no-changes --severity=info ...` verify) without dropping or loosening any argument (tasks.json is the canonical command spec). Bare `dotnet format` alone, skipping CSharpier or the build, is not sufficient.
- Files reviewed: 15/15 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Two suppressed findings, both real. repo-config/README.md called configure.sh "the script that reads them" directly after listing this repo's carried payloads, which reads as the script consuming them. It does not. Every payload path resolves against the script's own directory, so a run from a hub checkout compares this repo against the hub's payloads and never opens the copies beside the sentence. That distinction is the whole point of hosting the script rather than carrying it, so the ambiguity sat exactly where it did the most harm. CODESTYLE.md said the .NET task definitions are carried verbatim in what is now the hub catalog snippet, and called tasks.json the canonical spec while pointing at the snippet. Both readings were true before the link moved. Say the repo carries the 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>
|
Answering the 2 suppressed findings from the round-3 review on head The other two findings that review lists are from the earlier round on
Fixed in The sentence sat one clause after the list of this repo's own carried payloads, so "the script that reads them" pointed straight at them. That ambiguity is worth more than a wording nit, because the split it blurs is the entire reason for hosting the script rather than carrying it: an apply or check run from a hub checkout compares this repo against the hub's payloads, which is the single source, while the carried copies remain what this repo's own
Fixed in Split them rather than picking one. The first bullet now says a repo carries the definitions in its own All gates re-run green on |
There was a problem hiding this comment.
🔵 Human review recommended
It changes fleet-wide governance/spec semantics (including de-vendoring and new divergence disposition) with broad downstream operational impact that merits final human review.
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
#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>
…, and the operational pull-request trigger (#589) Promotes the five commits develop carried since #582: #584 (a Copilot refusal is not coverage), #586 (the reference-definition collation), #587 (CI on a pull request into an operational develop), #588 (hub-hosted tooling, de-vendoring configure.sh), and #590 (the two definition blocks #588 pushed out of order). Merge commit, no squash, develop not deleted. git merge-tree reported no conflict and wrote a tree byte-identical to origin/develop. GOVERNANCE.md is verbatim-carried and three of these commits change it, so a fleet re-vendor is owed, including an entirely new carried section. #588 also turns every downstream repo-config/configure.sh copy into a deletion rather than a convergence, and #587 leaves a one-line trigger change owed by four operational repos.
GOVERNANCE.md"PR Review Etiquette" tells a downstream reader to runscripts/pr_review.py, andscripts/appears in nospec/files.jsonbaseline entry, so the path is absent from every repository carrying the rule. The cost is measured rather than anticipated, per #580: several onboarding sessions built their own worse versions ofpr_review.pyandprose_lint.pyand 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. This takes the third.The model, stated as a carried rule
New verbatim section
GOVERNANCE.md"Hub-Hosted Tooling", declared inspec/files.jsonandspec/section-model.md, with a router row inAGENTS.md"Where the Rules Live". It settles the three questionsTODO.mdsaid had to be settled before either half moved:mainfrom a checkout, fetched immediately before use, and names the hub commit in any report it produces. CI reaches the same rules as a composite action pinned to a commit SHA, which is "Action pinning" applied unchanged and which.github/actions/prose-gate/action.ymlalready implements.The rule that was blocking it
"Carried files carry no coordination references" conflated where a file came from with what its reader runs, so a carried rule could not name a hub tool at all. It now carries a second exception for a hub-hosted tool the reader executes, with the test stated: where the content came from stays out, what the reader runs stays in.
The audit's
template_ref_outside_verbatimcheck needs no widening. The new section defers toAGENTS.md"Fleet Bootstrap" for the destination, soGOVERNANCE.mdstill holds zero occurrences of the hub name.What moves as a result
scripts/pr_review.pyis named as the hub's in "PR Review Etiquette", with how to run it. That is Carried rules name hub-only tooling that no downstream repo can run, so agents rebuild it by hand #580's literal case.repo-config/configure.shis de-vendored, the fleet's only carried script. Its last hub fix immediately became a re-vendor owed by every repository carrying it, and a repository that misses such a sweep does not fail loudly, it audits itself with an older script. The manifest entry is gone, andSTANDUP.md,docs/repo-config-carry.md,OPERATIONS.mdand the carriedrepo-config/README.mdnow say run it from a hub checkout naming the target repo. The ruleset payloads stay carried, being what a repository is audited against and what its ownAUDIT.mddiffs the live rulesets against.retiredisposition in the divergence ledger, its schema,validate.pyandfidelity_honesty.py, meaning the copy is deleted rather than converged.configure.shmoves from a six-repo re-vendor to it.CODESTYLE.mddefinitions pointing into hub-onlycatalog/are absolute hub URLs atmain. The private-repository objection the issue raised no longer holds.CODESTYLE.md:364asked each repository to re-point one of them at its owntasks.json, which is precisely what left the reference resolving differently in every copy, so it now names the hub canonical instead.Follow-through this leaves
configure.shcopy keeps it until next visited, and nothing flags it now that the manifest entry is gone. Cross-repo work. Regeneratereports/divergences.mdbefore using it as the work list, since the committed copy predates this decision and dispositions both carrier sets for a re-vendor that no longer applies to either.AGENTS.mdrouter row. The audit reporting the new section as missing fleet-wide is the mechanism working.Verification
All local gates green on this branch:
test_prose_lint.py(163),test_repo_gate.py(23),test_pr_review.py(82),spec/audit.py --selftest,repo_gate.py,spec/validate.py,jq emptyover every JSON manifest,editorconfig-checker,markdownlint-cli2over 44 files (0 issues),cspellover README and HISTORY, andprose_lint.py --diff developclean. Three pre-existing semicolons on the touchedCODESTYLE.mdline were fixed under the correct-as-next-edited rule. CRLF preserved on every file, verified bygit diff --numstatshowing no whole-file rewrite.Refs #580, #559
🤖 Generated with Claude Code