diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index cf856d7c..fc8ba252 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -37,6 +37,18 @@ Auto-review on push is configured (via the branch ruleset's `copilot_code_review **A review with no inline comments is still a completed review, not a failure, and not a reason to ask the maintainer to re-trigger.** Copilot very often posts a single formal review (GraphQL `state: COMMENTED`) whose body ends with "...reviewed N of N changed files ... and generated no comments" and adds **zero** inline threads. That review carries the head `commit.oid` and fully satisfies the loop, and it is the clean-pass success case. Never read "no inline comments" as "the review didn't run," and never re-request or escalate to the maintainer because comments are absent. +**The one exception is a review that says it did not review, and it is delivered in exactly that shape.** 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: "Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again." Every coverage check passes, the rule above says an empty review is the clean pass, and the two together read a round that never happened as a round that found nothing. Observed on a pull request of 301 changed files, one over the limit, which was one command from merging on it. **The limit is 300 changed files and the remedy is to split the pull request**, since re-requesting the same head repeats the refusal: the file count is what it declined on and re-requesting does not change it. A repository committing binary or generated data alongside code crosses that line easily. Match the refusal on the body's **opening line** rather than anywhere in it, because a review discussing the wording is not one carrying it, and one line rather than two, because a review's first line is its heading and its second is the overview prose where such a description sits. Match an alternation for the same reason the suppressed heading takes one: + +```sh +# A review whose opening line declines the round. That line is the unit, since a refusal is +# the whole body and a match further down is a review quoting the wording rather than refusing. +# The dot spans both spellings of the apostrophe, the typographic one Copilot writes and the +# ASCII one, and it also keeps this filter usable inside single quotes, which neither survives. +gh api repos///pulls//reviews --jq \ + '.[] | select([(.body // "") | split("\n")[] | select(. != "")][0] // "" + | test("wasn.t able to review|was not able to review|unable to review")) | {commit_id, body}' +``` + **Read the low-confidence findings, which are not inline threads.** A review body can carry a collapsed `
` block of findings Copilot withheld from the inline threads, and those findings appear nowhere in `reviewThreads`, so a loop that polls threads alone never sees them and reports a clean pass. **Match the block on more than one phrasing.** Its heading has appeared both as `Suppressed comments (N)` and as "Comments suppressed due to low confidence", so a filter keyed on either one alone silently reports zero suppressed findings on a review that has them, the same false clean this rule exists to prevent, one level up in the detection. **The section moves as well as it is worded, so match the heading wherever it sits.** It has appeared as its own `
` wrapper with a matching ``, as a bare heading in the body, and as a Markdown heading nested inside the `Review details` wrapper, whose `` names the wrapper and not the section. A filter reading a wrapper's `` reports zero on the nested shape, and the count it needs is the heading's own `(N)` rather than the wrapper's. They have been right repeatedly, including a rule stated more broadly than its check enforced and a check that skipped fenced blocks in every rule but one. Read the body of every review, investigate each suppressed finding on the same footing as an inline one, and answer it in the PR conversation, since a suppressed finding has no thread to reply on or resolve. ```sh @@ -141,7 +153,7 @@ gh api repos///issues//comments --jq \ '[.[] | select(.user.login=="copilot-pull-request-reviewer[bot]")] | last | {created_at, body: .body[:200]}' ``` -Coverage is confirmed when (1) exits 0, and **a formal review with no inline comments still satisfies path (1)**, because coverage is about the head SHA, not the comment count. For issue comments (path 2), body content is the only reliable signal, and `created_at` is not: `git log -1 --format=%cI` is the **commit** timestamp, not the push timestamp, so amended or rebased commits can have an earlier timestamp and an older Copilot comment could satisfy a time check even though Copilot never saw the current head. Treat path (2) as confirmed only when the comment body explicitly refers to the current changes. +Coverage is confirmed when (1) exits 0, and **a formal review with no inline comments still satisfies path (1)**, because coverage is about the head SHA, not the comment count. The exception is the refusal above, which is a formal review on the head with no inline comments and covers nothing, so path (1) exits 0 over a round that never ran. Read the body of the review the SHA matched, not only the SHA. For issue comments (path 2), body content is the only reliable signal, and `created_at` is not: `git log -1 --format=%cI` is the **commit** timestamp, not the push timestamp, so amended or rebased commits can have an earlier timestamp and an older Copilot comment could satisfy a time check even though Copilot never saw the current head. Treat path (2) as confirmed only when the comment body explicitly refers to the current changes. ### Bounded Retry Workflow diff --git a/AGENTS.md b/AGENTS.md index a192ae47..0b6459c1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -75,6 +75,7 @@ Every rule below is a level-two section of [`GOVERNANCE.md`](./GOVERNANCE.md). R | Releasing, version bumps, publishing | `Release Model` | | A live config repo rather than a code repo | `Operational Repositories` | | Onboarding a repo or running a conformance sweep | `Repository Onboarding and Conformance` (hub only, not carried) | +| Running a fleet gate, the review digest, or the config script | `Hub-Hosted Tooling` | | Writing a commit message or pull request title | `Pull Request Title and Commit Message Conventions` | | Any prose, comment, doc, or line-ending change | `Documentation Style Conventions` | | Proving work actually happened | `Verification Discipline` | diff --git a/CODESTYLE.md b/CODESTYLE.md index 2414a1ea..5088aafd 100644 --- a/CODESTYLE.md +++ b/CODESTYLE.md @@ -52,8 +52,8 @@ This is the style guide for any **.NET projects** in this repo. **CRITICAL**: All builds must complete without warnings. The project enforces this through: 1. **The `.NET Format` clean-compile task** (see [Clean-Compile Verification][clean-compile-verification]) - - 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]. - - 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] 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. + - The .NET clean-compile is the **`.NET Format`** VS Code task, which chains `CSharpier Format` -> `.NET Build` -> `dotnet format style --verify-no-changes`. A repo carries those three definitions in its own `.vscode/tasks.json`, matching the canonical 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 exactly (`CSharpier Format`, then `.NET Build`, then the `dotnet format style --verify-no-changes --severity=info ...` verify) without dropping or loosening any argument, reading it from [`vscode-tasks.json`][vscode-tasks-link], which is the canonical command spec a repo's own `tasks.json` is written against. Bare `dotnet format` alone, skipping CSharpier or the build, is not sufficient. 2. **Analyzer configuration** - `true` with `latest-all` and `All` (full analyzer set enabled) @@ -361,7 +361,7 @@ This is the style guide for any **Python project(s)** in this repo. - **Dependency declaration** - `[dependency-groups]`, or PEP 621 `[project.optional-dependencies]` (dev tools installed with `uv sync --extra `). - **Versioning / publishing** - a published package (`_version.py` + a version source + `uv build` + a PyPI publish step), or a **source-only** repo with a static `version` and no publish step (see [Versioning][versioning-section]). - **Disabled markdownlint rules** - repo-specific. `.markdownlint-cli2.jsonc` at the repo root is the source of truth, not any example rule named here. -- **VS Code config home** - editor **settings/extensions** may live in `.vscode/*.json` **or** the `.code-workspace`, while **tasks / launch / debug** configs can only be external `.vscode/*.json` (they cannot live in the workspace file). A `[vscode-tasks]` reference must point wherever the repo actually keeps `tasks.json`. +- **VS Code config home** - editor **settings/extensions** may live in `.vscode/*.json` **or** the `.code-workspace`, while **tasks / launch / debug** configs can only be external `.vscode/*.json` (they cannot live in the workspace file). The repo's own `tasks.json` sits wherever it keeps it, and the canonical task definitions it is written against are the hub snippet the [`vscode-tasks.json`][vscode-tasks-link] reference names, which resolves the same way from every repo. **Two profiles.** A repo's Python is one of two shapes, declared as the `build` or `lint-only` profile and validated against the `pyproject.toml` shape. The rest of this section (uv project, `uv.lock`, `uv run`, `src` layout, pytest coverage) describes the **Project** shape (the `build` profile). The two differ by whether the Python has **third-party runtime dependencies**, which shows up structurally in `pyproject.toml`, so the audit reads the shape there (`python.profile.detect`): @@ -398,7 +398,7 @@ uv run pytest # run tests uv build # produce wheel + sdist in ./dist (published packages only) ``` -The Python clean-compile (see [Clean-Compile Verification][clean-compile-verification]) is `uv run ruff format` + `uv run ruff check` + the repo's type checker: `uv run pyright`, or `uv run mypy src` where mypy is the CI checker, or both where the repo runs both (see Type checking above); run it (plus `uv run pytest`) before committing. These are documented commands; an optional VS Code tasks mirror (all `type: process`, no `&&` shell chaining, so it runs the same on any task shell) is in [`vscode-tasks-python.json`][vscode-tasks-python]. CI runs the same clean-compile commands as the authoritative backstop. Git hooks are opt-in; wire `pre-commit` for `ruff` and the type checker yourself if you want local enforcement. +The Python clean-compile (see [Clean-Compile Verification][clean-compile-verification]) is `uv run ruff format` + `uv run ruff check` + the repo's type checker: `uv run pyright`, or `uv run mypy src` where mypy is the CI checker, or both where the repo runs both (see Type checking above). Run it, plus `uv run pytest`, before committing. These are documented commands, and an optional VS Code tasks mirror (all `type: process`, no `&&` shell chaining, so it runs the same on any task shell) is in [`vscode-tasks-python.json`][vscode-tasks-python-link]. CI runs the same clean-compile commands as the authoritative backstop. Git hooks are opt-in, so wire `pre-commit` for `ruff` and the type checker yourself if you want local enforcement. ### Layout @@ -502,8 +502,6 @@ Before pushing or opening a PR: [readme]: ./README.md [root]: ./.editorconfig [versioning-section]: #versioning -[vscode-tasks]: ./catalog/snippets/configs/vscode-tasks.json -[vscode-tasks-python]: ./catalog/snippets/configs/vscode-tasks-python.json @@ -514,3 +512,5 @@ Before pushing or opening a PR: [pyright-link]: https://microsoft.github.io/pyright/ [ruff-link]: https://docs.astral.sh/ruff/ [uv-link]: https://docs.astral.sh/uv/ +[vscode-tasks-link]: https://github.com/ptr727/ProjectTemplate/blob/main/catalog/snippets/configs/vscode-tasks.json +[vscode-tasks-python-link]: https://github.com/ptr727/ProjectTemplate/blob/main/catalog/snippets/configs/vscode-tasks-python.json diff --git a/GOVERNANCE.md b/GOVERNANCE.md index 29c665fa..e6d37680 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -101,6 +101,7 @@ The registry `workflowModel` field is `release` (the default) or `operational`. **Operational** repos track a live service's running state rather than shipping versioned units of delivery: live-service config such as Home Assistant, ESPHome, Vantage, and home automation. - **Commit configuration directly to `develop`.** There is no feature branch, and the maintainer commits straight to `develop` and *occasionally* opens a `develop -> main` PR to bless a known-good snapshot. The `develop` ruleset drops the PR and status-check gate, so direct signed pushes are allowed (force-push, deletion, and unsigned commits are still blocked) and CI runs on the push as **advisory** feedback that never rejects a commit. +- **A PR into `develop` stays available, and CI runs on it.** Dropping the requirement permits the direct push, it does not withdraw the pull request, so a change worth reviewing takes one and both paths into `develop` are legitimate. The lint workflow's `pull_request` trigger therefore names `develop` alongside `main`, since a trigger set naming `main` alone matches nothing on a `develop` PR and that PR would merge with no validation at all, which is the one output [WORKFLOW.md](./WORKFLOW.md) D1.2 forbids on *any* PR. The result is **reported and not required** there, because a required status check on `develop` would gate the direct push too and dissolve the allowance the model is built on. The asymmetry is the point: on the direct-commit path CI can only be advisory after the fact, while on the PR path the change has not landed yet, so validating it is both pre-merge and actionable. - **The `main` promotion gate is unchanged.** The [`main` ruleset](./repo-config/main.json) is shared with `release` repos, so the `develop -> main` PR still **enforces** the required `Check pull request workflow status job`. For an operational repo that check is lint/validation only, meaning editorconfig/EOL plus domain linters (a Home Assistant or ESPHome config validation, a firmware build) and never unit tests, so `develop` stays the live surface and a broken config can never reach `main`. - **Release only by manual dispatch.** Operational repos carry `releaseTrigger: dispatch-only` and run no codegen or auto-publish bots, so they publish **only** on a manual `workflow_dispatch`, the source-only release the publisher already supports (tag + source zip + README + LICENSE, NBGV-versioned), never automatically. The `develop -> main` promotion just blesses a known-good snapshot, and a release is a separate, deliberate dispatch. - **Fleet sync still applies.** Dependabot's dual-target sync and the App-signed merge-bot run on **every** tier, operational included, so both branches stay in sync and a promotion stays a clean forward merge. @@ -115,6 +116,20 @@ Every fleet repo is a standard-style repo the hub audits **downward** against it - **The registry is ground truth about reality, not intent.** After any conformance change, reconcile the repo's `registry/repos.json` entry (`status`, `types`, `releaseTrigger`, `workflowModel`, `driftNotes`) to what the repo actually is. `spec/validate.py` proves the catalog is self-consistent. It does **not** prove the catalog matches the live repo. A `validate.py`-clean entry can still be false, and a stale entry is a finding. - **Prove the docs suffice.** The onboarding docs are sufficient only when a context-free agent stands each supported repo shape (project type(s) plus workflow model) up from them alone. The cold-start self-test in `STANDUP.md` and the conformance matrix track that. A shape that cannot be stood up cold is a documentation defect, fixed in the hub, not worked around per repo. +## Hub-Hosted Tooling + +The fleet's tooling lives in the hub once and a repository runs it from there rather than holding a copy. A carried script is current only until the next fix to it, and a repository that misses the sweep does not fail loudly, it audits itself with an older gate while reporting the same command in its output. Removing the copy removes the sweep, the stale-copy detection, and the disposition each stale copy earns, all at once. The hub is the repository [`AGENTS.md`](./AGENTS.md) "Fleet Bootstrap" names, and that section is the entry point whenever nothing else present says where it is. + +**What a repository carries and what it reaches is decided by what the content is.** It carries the content it is audited against and the configuration that describes it, meaning its rule text, the files the manifest declares, and the ruleset payloads its own audit diffs the live configuration against. It reaches machinery whose content is identical in every repository, meaning the prose and repository gates, the review digest, and the configuration script, because a file holding no per-repo content is a copy whose only future is to go stale. A tool named in a carried rule is therefore named as the hub's, since the alternative is a pointer to a path the reader does not have, and a pointer that resolves nowhere teaches the reader that a pointer in carried text is decorative. + +**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. Name the tool by its path in that checkout and name the target explicitly, since a tool that defaults to the current directory or the current repository resolves somewhere either way, and a result computed against the wrong repository is well-formed. Which directory the command runs in is the tool's own contract rather than a rule here, so a gate reading a working tree runs in the repository under test while a tool taking its target as an argument runs anywhere and is given one. What the rule forbids is letting a default decide which repository the answer is about. + +**A report or finding a hub tool produces names the hub commit it ran from.** The tool moves independently of the repository it measures, so a verdict carrying no hub commit cannot be re-run, and two runs that disagree cannot be attributed to the tree or to the tool. The obligation is the runner's rather than the tool's, since a tool reports on the repository it measures rather than on itself, so the commit is read from the hub checkout and written into the report beside the verdict. This is the same requirement "Verification Discipline" places on any claim that gets acted on. + +**CI reaches the same tooling as a pinned action.** A runner holds no hub checkout, so a workflow consumes the hub's composite action and pins it to a commit SHA, per the action-pinning rule under "Workflow YAML Conventions". The pin is what makes a released repository's gate reproducible, since an unpinned consume lets a later hub commit fail a re-run of a change that already passed. Branch-dependent behavior belongs inside the consumed action, because `uses:` takes no expressions and a per-branch ref therefore cannot be selected in the workflow file. + +**An unreachable hub means the tool did not run, and that is the result reported.** A carried copy still works offline and a reached one does not, which is the cost this model trades away and the reason to state the failure rather than route around it. A check that cannot run reports itself as not run, never as clean, which is the silent-narrowing failure "Verification Discipline" names. A hand-rolled substitute is not the tool either: a reconstructed gate encodes its author's reading of the rule rather than the rule, agrees with no other repository, and is the duplicated effort this model exists to end, so an agent that cannot reach the hub says so and stops. + ## Pull Request Title and Commit Message Conventions ### Format @@ -141,11 +156,11 @@ Clarify devcontainer setup steps in README ## Documentation Style Conventions -- **Carried files carry no coordination references.** In the files the fleet carries (`AGENTS.md`, `GOVERNANCE.md`, `CODESTYLE.md`, `WORKFLOW.md`, `.github/copilot-instructions.md`, the `repo-config/` and `spec/` files, the carried `AUDIT.md`), two things are banned. **Any reference to the template repo**, in prose or in a link: the coordination flow is machinery a consumer of that repo should never have to see, and naming where a file came from is the derived-from framing the present-tense rule below independently forbids. Where a carried file must express a template-level behavior ("report a rule discrepancy upstream"), state the behavior rather than the destination. **The one exception is a verbatim section, and `AGENTS.md` "Fleet Bootstrap" is why it exists.** That section's whole function is to name where the canonical rules live, for an agent in a repository whose carried copies are stale, partial, or absent, which is exactly when no other file present can say it. Its bytes are fixed fleet-wide, so a repository cannot edit the reference out without failing the verbatim check instead, and a rule banning it would be unsatisfiable rather than merely strict. The exception is scoped to the verbatim region and never leaks past it: the same document's own prose is governed normally, so a repository that names the destination in a paragraph it owns is still in breach. A reference that reaches a verbatim section is a defect in the canonical, fixed once at the source rather than reported against every repository carrying it. The maintainer supplies the destination out of band. And **a sibling fleet repo named as an illustrative example** ("repo X does it this way", "see repo Y's adoption"), which couples the repos and rots as they diverge. To point at a current good example, name it in the onboarding or conformance issue, never in a carried doc. **A contextually relevant link to a related project is not a coordination reference, and is expected.** Where another repo is part of this repo's subject matter (the image that consumes this config, the builder that generates this hardware, a library this depends on), link it normally. The test is whether the link serves a reader of *this* repo's content, not whether the target happens to be in the fleet. This rule governs carried template content. A repo's own `README.md` and topical docs are its own content, not carried verbatim, and it does not reach them. This pairs with the present-tense rule below: state the current shape, not a history of which repo it came from. +- **Carried files carry no coordination references.** In the files the fleet carries (`AGENTS.md`, `GOVERNANCE.md`, `CODESTYLE.md`, `WORKFLOW.md`, `.github/copilot-instructions.md`, the `repo-config/` and `spec/` files, the carried `AUDIT.md`), two things are banned. **Any reference to the template repo**, in prose or in a link: the coordination flow is machinery a consumer of that repo should never have to see, and naming where a file came from is the derived-from framing the present-tense rule below independently forbids. Where a carried file must express a template-level behavior ("report a rule discrepancy upstream"), state the behavior rather than the destination. **The first exception is a verbatim section, and `AGENTS.md` "Fleet Bootstrap" is why it exists.** That section's whole function is to name where the canonical rules live, for an agent in a repository whose carried copies are stale, partial, or absent, which is exactly when no other file present can say it. Its bytes are fixed fleet-wide, so a repository cannot edit the reference out without failing the verbatim check instead, and a rule banning it would be unsatisfiable rather than merely strict. The exception is scoped to the verbatim region and never leaks past it: the same document's own prose is governed normally, so a repository that names the destination in a paragraph it owns is still in breach. A reference that reaches a verbatim section is a defect in the canonical, fixed once at the source rather than reported against every repository carrying it. The maintainer supplies the destination out of band. **The second exception is a hub-hosted tool the reader is told to run, which is a different kind of reference.** A rule naming a gate, a script, or a reference snippet the reader executes or copies states an instruction rather than a provenance, and an instruction with no destination is unfollowable, which is precisely how a pointer in carried text comes to read as decorative. The test is whether the reference is something the reader does or something that happened to this file, so where the content came from stays out and what the reader runs stays in. Such a pointer names the hub's canonical rather than this repository's provenance, so it is the hub's to keep resolving and never a repository's to edit out or to re-point at a local path, which is what left the reference resolving differently in every copy before. In `AGENTS.md` and `GOVERNANCE.md` it belongs in verbatim rule text, the same region the first exception already covers, so the whole fleet reads one wording and no repository is asked to answer for a reference it did not write. What is reached rather than carried, and how, is in [Hub-Hosted Tooling](#hub-hosted-tooling). And **a sibling fleet repo named as an illustrative example** ("repo X does it this way", "see repo Y's adoption"), which couples the repos and rots as they diverge. To point at a current good example, name it in the onboarding or conformance issue, never in a carried doc. **A contextually relevant link to a related project is not a coordination reference, and is expected.** Where another repo is part of this repo's subject matter (the image that consumes this config, the builder that generates this hardware, a library this depends on), link it normally. The test is whether the link serves a reader of *this* repo's content, not whether the target happens to be in the fleet. This rule governs carried template content. A repo's own `README.md` and topical docs are its own content, not carried verbatim, and it does not reach them. This pairs with the present-tense rule below: state the current shape, not a history of which repo it came from. ### Markdown -- **Reference-style links in human-facing docs.** Every Markdown file uses reference-style links only, **except** the four agent-instruction files that keep inline links: `AGENTS.md`, `GOVERNANCE.md`, `OPERATIONS.md`, and `.github/copilot-instructions.md`. Those are read one section at a time rather than end to end, so an inline target resolves where it is read while a definition at the bottom of the file is never reached. The exception is that closed list rather than a category to argue from, so every other Markdown file follows the rule whatever its audience. Under the rule: every URI (internal path, anchor, external URL, or shield image) is defined at the **bottom of the file**, split into groups by type under an HTML-comment header (e.g. ``, ``, ``, ``) with each group's definitions alphabetized by reference name. **Reference names are contextual and encode the target and its group**: `foo-shield` for a shield image, `foo-link` for an external URL, and a bare `foo` for a local path or anchor (e.g. `[license-shield]`, `[releases-link]`, `[repo-config]`), never numeric (`[1]`) or opaque. No inline `[text](uri)` targets in prose. **A URL inside a fenced code block stays inline**, because reference links do not resolve in code blocks, so do not extract it, and exclude fenced code from any link-integrity check (bracket literals like `["a", "b"]` otherwise read as undefined references). **Removing a link also removes its reference definition**, since an orphaned definition fails the no-unused-defs rule. The one exception is the Table of Contents, whose entries stay inline anchor links (see Table of Contents below). +- **Reference-style links in human-facing docs.** Every Markdown file uses reference-style links only, **except** the four agent-instruction files that keep inline links: `AGENTS.md`, `GOVERNANCE.md`, `OPERATIONS.md`, and `.github/copilot-instructions.md`. Those are read one section at a time rather than end to end, so an inline target resolves where it is read while a definition at the bottom of the file is never reached. The exception is that closed list rather than a category to argue from, so every other Markdown file follows the rule whatever its audience. Under the rule: every URI (internal path, anchor, external URL, or shield image) is defined at the **bottom of the file**, split into groups by type under an HTML-comment header (e.g. ``, ``, ``, ``) with each group's definitions alphabetized by reference name. **The sort key is the reference name alone**, the text inside the brackets, never the whole definition line: where one name is a prefix of another, the shorter one sorts first (`[governance]` above `[governance-branching-model]`, `[repo-config]` above `[repo-config-settings]`). Sorting the line instead inverts every such pair, because `-` precedes `]` in byte order, so the two readings disagree on exactly the names a reader looks up together, and a plain `sort -c` over the block passes on the inverted order. **Reference names are contextual and encode the target and its group**: `foo-shield` for a shield image, `foo-link` for an external URL, and a bare `foo` for a local path or anchor (e.g. `[license-shield]`, `[releases-link]`, `[repo-config]`), never numeric (`[1]`) or opaque. No inline `[text](uri)` targets in prose. **A URL inside a fenced code block stays inline**, because reference links do not resolve in code blocks, so do not extract it, and exclude fenced code from any link-integrity check (bracket literals like `["a", "b"]` otherwise read as undefined references). **Removing a link also removes its reference definition**, since an orphaned definition fails the no-unused-defs rule. The one exception is the Table of Contents, whose entries stay inline anchor links (see Table of Contents below). - **Table of Contents.** Generate it with the Markdown All in One extension, which fills and auto-updates the list on save, so leave the `## Table of Contents` heading for the extension to populate and never hand-author or hand-edit the entries. Exclude a heading with an inline `` marker on it (the badge/build header block and the `## Table of Contents` heading itself carry it). The workspace sets which heading levels appear. - One logical paragraph per line, with no hard-wrap line-length limit. For an intentional hard line break within a block (stacked badges, status, or license lines), end the line with a trailing backslash (`\`). This explicit form is preferred over trailing whitespace and is not treated as a paragraph split. - Headings follow the title-case-with-short-bind-words rule from the PR-title section. @@ -246,7 +261,7 @@ The repo runs a review loop on every PR: local agent iteration plus remote autom **Do not merge, and do not enable auto-merge, unless ALL of these hold:** 1. Required status checks are green (`mergeStateStatus: CLEAN`), **and** -2. A Copilot review is confirmed on the **current head SHA** by matching the review's commit SHA to the head, not an earlier push, because a push makes required checks go green **before** the re-review lands, so a green merge-state can precede the current-head review and never signals readiness on its own, **and** +2. A Copilot review is confirmed on the **current head SHA** by matching the review's commit SHA to the head, not an earlier push, because a push makes required checks go green **before** the re-review lands, so a green merge-state can precede the current-head review and never signals readiness on its own, and the matched review is **read** rather than only counted, because Copilot declines a pull request it will not take on with a formal review carrying that same head SHA and no findings, which matches the SHA and covers nothing, **and** 3. **Every** Copilot finding on that head SHA is closed out, with all review threads resolved, **and** any issue-level Copilot comments (which have no resolve action) triaged and replied to, **and** the low-confidence findings collapsed in the review body investigated and answered, since those appear in no thread and a loop that polls threads alone reports a clean pass while they stand, so zero outstanding findings remain, **and** 4. The maintainer has given **explicit** permission to merge. @@ -258,7 +273,7 @@ The repo runs a review loop on every PR: local agent iteration plus remote autom 1. Push changes to the PR branch. 2. Re-request a review for the **current head SHA**. Auto-trigger is unreliable, so request it explicitly via the `requestReviews` GraphQL mutation (reliable end-to-end, per the runbook). The UI is only a fallback. -3. Wait for review activity on that head. A completed review that raises **no findings** is a valid terminal outcome for that head, so proceed. Do not re-trigger it or treat the absence of comments as a missing review. +3. Wait for review activity on that head. A completed review that raises **no findings** is a valid terminal outcome for that head, so proceed. Do not re-trigger it or treat the absence of comments as a missing review. A review whose body says it did not review is the one exception, and it is terminal in the other direction: nothing follows it, re-requesting the same head repeats it, and the body names what has to change first. 4. Triage findings. 5. Apply fixes or write a rationale for declines. 6. Reply to each thread and resolve what was addressed. @@ -298,7 +313,7 @@ After the final push on a PR, sweep older threads from earlier rounds whose code **Answering a suppressed finding is a different act from replying in a thread, and it carries its own pairing.** A threaded reply sits under the comment it answers and the UI shows whether it is resolved. A suppressed finding has neither, so an answer that does not carry its own context is unverifiable: the maintainer cannot tell that it was seen, which finding it addresses, or whether any were skipped, and has to ask. An answer therefore **quotes the finding** in a blockquote, with its `file:line` anchor and enough of Copilot's own words to identify it, **carries one bold verdict per finding** (`Fixed in `, `Disproven`, or `No change needed`) so the outcomes are scannable without reading prose, **states the `(N)` count** the block heading gives so N answers can be checked against N findings, and **links the review** that raised them, since a PR accumulates rounds and an unlinked answer is ambiguous about which one it closes. One comment per review round keeps the answers together. -**Read every round, not only the head.** A suppressed finding has no resolved state, so a push does not retire it: the finding simply stops appearing in a head-scoped query while remaining unanswered. Treating "superseded by a push" as "answered" is how rounds of findings go unanswered. `scripts/pr_review.py status --repo /` reports every round and marks which are from earlier ones, and it names the repository because a pull request number resolves in every repository and a digest of the wrong one is well-formed. +**Read every round, not only the head.** A suppressed finding has no resolved state, so a push does not retire it: the finding simply stops appearing in a head-scoped query while remaining unanswered. Treating "superseded by a push" as "answered" is how rounds of findings go unanswered. The hub's `scripts/pr_review.py status --repo /` reports every round and marks which are from earlier ones, and it names the repository because a pull request number resolves in every repository and a digest of the wrong one is well-formed. That script is hub-hosted rather than carried, so no repository holds a copy to run and it is invoked from a hub checkout per "Hub-Hosted Tooling" above. **The review's own overview cannot be trusted to say whether findings exist.** A body that reads "Copilot reviewed N out of N changed files and generated no new comments" routinely carries a collapsed block of suppressed findings directly beneath that sentence. Read the body for the block rather than the summary line, because the summary line and `reviewDecision` and an empty unresolved-thread list all agree that a review with four outstanding findings is clean. @@ -411,9 +426,9 @@ Every repo's GitHub repository details (the About panel) follow a fixed conventi - [`GOVERNANCE.md`](./GOVERNANCE.md), [`CODESTYLE.md`](./CODESTYLE.md), [`WORKFLOW.md`](./WORKFLOW.md), [`AUDIT.md`](./AUDIT.md): the governance and audit docs. This file is the cross-cutting-rules authority. - [`spec/`](./spec/): the machine-readable ground truth, covering project-type requirements, the file/section baseline, secrets, and the preferred README structure. - [`registry/repos.json`](./registry/repos.json): the fleet registry, holding every project, its type(s), and its status. -- [`repo-config/`](./repo-config/): branch rulesets and the apply script, kept out of `.github/` (which is Actions-owned). +- [`repo-config/`](./repo-config/): branch rulesets, the fleet settings, and the apply script every repo is configured by, kept out of `.github/` (which is Actions-owned). The payloads carry to the fleet and the script is reached here. - [`catalog/`](./catalog/): reusable reference snippets (workflows, configs, devcontainers) the audit compares implementations against. -- [`scripts/`](./scripts/): local lint and review tooling this repo runs by hand, with the deterministic checks also gating CI, for the documented rules no CI linter checks. Hub-only, not carried. +- [`scripts/`](./scripts/): the fleet's lint and review tooling, with the deterministic checks also gating CI, for the documented rules no CI linter checks. Hosted here and reached rather than carried, per "Hub-Hosted Tooling". - [`docs/`](./docs/): hub-only procedure and measurement docs (host setup, config carry, signing, agent token cost), none of them carried downstream. - [`reports/`](./reports/): per-repo audit output. - [`.github/workflows/`](./.github/workflows/): this repo's own lint-only CI and source-only release, the workflow contract it dogfoods. diff --git a/OPERATIONS.md b/OPERATIONS.md index 84a98154..6e74419c 100644 --- a/OPERATIONS.md +++ b/OPERATIONS.md @@ -1,6 +1,6 @@ # Operations -How this repository is run. It ships no application code, so its operations are the fleet audit, the local gates that mirror CI, and the script that applies repository configuration. +How this repository is run. It ships no application code, so its operations are the fleet audit, the gates that mirror CI, and the script that applies repository configuration. Those gates and that script serve the whole fleet from this checkout rather than being carried into each repository, per [GOVERNANCE.md "Hub-Hosted Tooling"](./GOVERNANCE.md#hub-hosted-tooling), so every run below is a run from here against a repository named on the command line. ## Runbooks @@ -48,11 +48,11 @@ Findings are a point-in-time snapshot read live over the API. Re-run before acti ### Apply or verify repository configuration ```sh -repo-config/configure.sh check [owner/repo] [release|operational] -repo-config/configure.sh apply [owner/repo] [release|operational] +repo-config/configure.sh check owner/repo release|operational +repo-config/configure.sh apply owner/repo release|operational ``` -**Always pass the command.** A bare `repo-config/configure.sh` with no arguments defaults to `apply` against the current repo, so an invocation meant to test whether the script runs performs a live write instead. Never run it without a command. +**Always pass the command, the repository, and the model.** A bare `repo-config/configure.sh` with no arguments defaults to `apply` against the current repo, so an invocation meant to test whether the script runs performs a live write instead. Never run it without a command. The repository argument matters for the same reason now that the fleet runs this copy rather than its own: an omitted target resolves to this repository, and applying the fleet configuration to the hub while meaning to configure a downstream repo is a well-formed write to the wrong place. The model is the third argument for the same reason. This checkout has the registry beside the script, so a repo the registry does not yet name resolves through `defaults.workflowModel` to `release` rather than aborting, and an operational repo then takes the release `develop` ruleset. `check` is read-only and exits non-zero on drift. `apply` is idempotent and drives entirely from the committed payloads, so it is a no-op on a conformant repo. @@ -100,7 +100,7 @@ Two `gh` limitations on the current host, both worked around rather than fixed: - [spec/](./spec/) is the machine-readable ground truth, holding project types, the file and section baseline, and required or forbidden secrets. - [registry/repos.json](./registry/repos.json) is the fleet registry, naming every project with its types, publish mechanism, and status. -- [repo-config/](./repo-config/) holds the branch rulesets and the apply script. It sits outside `.github/`, which is Actions-owned. +- [repo-config/](./repo-config/) holds the branch rulesets, the fleet settings, and the apply script. The payloads carry to the fleet and the script is reached here. It sits outside `.github/`, which is Actions-owned. - [catalog/](./catalog/) holds reference snippets the audit compares implementations against. - [reports/](./reports/) holds per-repo audit output. -- [scripts/](./scripts/) holds the gates that run in CI and locally. +- [scripts/](./scripts/) holds the gates that run in CI and locally, and that every fleet repository reaches rather than carries. diff --git a/STANDUP.md b/STANDUP.md index 209c16ad..5e978e26 100644 --- a/STANDUP.md +++ b/STANDUP.md @@ -119,7 +119,7 @@ Three conditions fail here, and the two commands together are what separate them Each is step 0A's escalation rather than something to work around. -Run `repo-config/configure.sh apply [owner/repo] [release|operational]` (the repo defaults to the current one, the model to the registry lookup or, absent a registry, to the carried payload) 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. 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. +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. ## 5. Verify: Run the Audit diff --git a/TODO.md b/TODO.md index 2ac4df94..c510d827 100644 --- a/TODO.md +++ b/TODO.md @@ -24,8 +24,8 @@ The gates in [`scripts/`][scripts] are hub-only and are the fleet's main defense - Teach the `sha-pin` check in [`repo_gate.py`][repo-gate] to verify a pin **resolves**, not merely that it is shaped like a SHA. Forty hex characters is a format any fabricated string satisfies, and an agent hand-writing a plausible SHA into a workflow is a real failure mode rather than a hypothetical one. A resolvability check also catches the neighboring case, a pin whose commit was reachable only from a branch that has since been squashed and deleted, which breaks a downstream gate long after the change that caused it. Scope the network call to same-owner repositories, where the fleet's own actions live, and skip rather than fail when the host is offline so the local gate stays usable. Note that the existing `gh-write-guard` hook cannot cover this, since it watches Bash and an editor tool writing the same string into a file never reaches it. - Add a check that a pull request's **description** does not contradict its own branch. Three stale descriptions in one session generated six review findings between them, each one a reviewer noticing that the body named a commit, a branch, or a behavior the branch no longer carried. The cheap and precise form is to extract SHAs and `uses:` refs quoted in the body and confirm each still appears in the head tree, since those are the claims that go stale silently and the ones a reviewer actually catches. Prose claims are out of scope, and deliberately so: judging those needs a similarity heuristic, which [`spec/section-model.md`][section-model] already rejects for exactly the reason it would fail here. - Reconsider whether the pre-commit hook should run the doc gates now that they are diff-scoped. [`scripts/README.md`][scripts] records the current decision and its reason, that doc linters stay out of the hook so it stays fast, and that reason was sound when the only mode was a whole-tree sweep. A `--diff` run reads the lines one commit touches and finishes in about a second, so the trade has moved. The failure it would prevent is the most repeated one on record: comment sentences wrapped across lines, caught by CI or by a reviewer after the commit rather than before it, over and over within a single session. Weigh it against the standing preference for a fast hook, and against the risk of a hook that runs the gate from the wrong directory, which is its own false clean. -- Audit the fleet's shell surface by size and branching, and decide per script whether Python with unit tests is the cheaper form. The evidence is the review record rather than a language preference: a non-trivial shell script earns findings round after round, while every gate under [`scripts/`][scripts] carries a `test_