diff --git a/.agents/skills/backlog-burndown/SKILL.md b/.agents/skills/backlog-burndown/SKILL.md index bb4c6119..91317aa8 100644 --- a/.agents/skills/backlog-burndown/SKILL.md +++ b/.agents/skills/backlog-burndown/SKILL.md @@ -114,7 +114,9 @@ handoff chain `AGENTS.md` "Session Scope" defines, so it records work to do next its own, and an open one is present by design for as long as that chain is in use. Counting it inflates the number this run reports as the backlog by one for every lane in use, and a backlog count this fleet reports wrong is a failure with its own history, so filter the label out of the -ranking and out of every count of the open backlog rather than out of the ranking alone. +ranking and out of every count of the open backlog rather than out of the ranking alone. A link +carrying `blocked` as well, as a parked lane does, is still a link, so this rule controls over the +`blocked` rule below and the link is neither ranked nor counted. An issue carrying the `blocked` label is counted and is not ranked while its blocker stands. It records real work this repository owes, which is why it stays in the count, and the label says the diff --git a/.agents/skills/dotnet-codestyle/SKILL.md b/.agents/skills/dotnet-codestyle/SKILL.md index 7eb20897..b13de18d 100644 --- a/.agents/skills/dotnet-codestyle/SKILL.md +++ b/.agents/skills/dotnet-codestyle/SKILL.md @@ -214,7 +214,7 @@ parameterized tests. A test project on `xunit.v3` 4.0.0 or later is MTP-based, a ## Project configuration -.NET 10.0 target, AOT-compatible (`IsAotCompatible=true`, `VerifyReferenceAotCompatibility=true`), +.NET 10.0 target, AOT-compatible (`IsAotCompatible=true`, with `VerifyReferenceAotCompatibility=true` only under the `PublishAot` condition), SourceLink, embedded untracked sources, `InternalsVisibleTo` for test/benchmark access. See `references/project-config.md` for the full property list. diff --git a/.agents/skills/dotnet-codestyle/references/project-config.md b/.agents/skills/dotnet-codestyle/references/project-config.md index 42b8fd1c..658e589c 100644 --- a/.agents/skills/dotnet-codestyle/references/project-config.md +++ b/.agents/skills/dotnet-codestyle/references/project-config.md @@ -1,8 +1,14 @@ # .NET Project Configuration 1. **Target framework**: .NET 10.0 (`net10.0`). -2. **AOT compatibility**: `true`, - `true`. +2. **AOT compatibility**: `true` unconditionally, and + `true` only in a + ``, placed in the `.csproj` after it sets + `PublishAot` or in `Directory.Build.targets`, never in `Directory.Build.props`, which is imported + before the project body and so never sees a `PublishAot` the `.csproj` sets. Reference + verification reports `IL3058` for every referenced assembly that lacks `IsAotCompatible` metadata + set to `true`, and `TreatWarningsAsErrors` turns that into a failed build on any such + dependency, so it runs only where an AOT publish needs it. 3. **Assembly information**: use semantic versioning, include SourceLink (`true`), embed untracked sources (`true`). diff --git a/.agents/skills/session-handoff/SKILL.md b/.agents/skills/session-handoff/SKILL.md index d0504cbc..d5eda1ae 100644 --- a/.agents/skills/session-handoff/SKILL.md +++ b/.agents/skills/session-handoff/SKILL.md @@ -143,8 +143,9 @@ section is the pointer to what to read rather than the answer, and `AGENTS.md` " already says stale context is worse than absent. A handoff is context by construction, so this is that rule applied to the one artifact built to outlive the session that wrote it. -Read the current link's comments too, with `gh issue view "" --comments`, since `resume` prints -only the body, and a parking comment or a closing session's answers land in the comments. +Read the current link's comments too, with `gh issue view "" --repo OWNER/NAME --comments`, since +`resume` prints only the body, and a parking comment or a closing session's answers land in the +comments. Read the chain before re-attempting anything. `resume` prints the current body and indexes the closed links behind it, and `chain --grep` searches the bodies it walks for a regular expression, @@ -161,7 +162,9 @@ rather than `resume`. Re-derive a count rather than copying one, and read it with an explicit page size. `gh issue list` returns 30 rows unless told otherwise, and a truncated count reads exactly like a repository with -30 issues, which is worse than an absent count because it gets stated. +30 issues, which is worse than an absent count because it gets stated. The same holds at any page +size, so where a read returns as many rows as its limit, raise the limit and read again until it +returns fewer. ## The Attended Session @@ -169,15 +172,17 @@ A maintainer resuming work says little, often only "resume the handoff", and tha the whole procedure below. Run every step without being reminded of any of them. 1. **Pick the link.** Where the maintainer names an issue, take it. Where none is named, read - `gh issue list --label handoff --state open --limit 100 --json number,title,labels,updatedAt`, - since `tracks` prints neither labels nor exact update times. Take a link carrying `blocked` - first, newest update first among them, since its blocker is a decision only the maintainer can - make and the maintainer is now present. Otherwise take the newest update among the rest. An - `auto-*` link not carrying `blocked` is skipped unless named, since an `unattended-handoff` - worker may hold it right now, and where one is named, confirm with the maintainer that no - unattended run is live before working it. Where every open link is skipped, say so and ask the - maintainer which to take. Say which link was picked in one line before anything else, so a wrong pick costs one reply - rather than a round. + `gh issue list --repo OWNER/NAME --label handoff --state open --limit 100 --json + number,title,labels,updatedAt`, since `tracks` prints neither labels nor exact update times. + Where it returns as many rows as the limit, the list may be truncated, so raise the limit and + read again until it returns fewer, rather than picking from a partial list. Take a link carrying + `blocked` first, newest update first among them, since its blocker is a decision only the + maintainer can make and the maintainer is now present. Otherwise take the newest update among the + rest. An `auto-*` link not carrying `blocked` is skipped unless named, since an + `unattended-handoff` worker may hold it right now, and where one is named, confirm with the + maintainer that no unattended run is live before working it. Where every open link is skipped, + say so and ask the maintainer which to take. Say which link was picked in one line before + anything else, so a wrong pick costs one reply rather than a round. 2. **Resume it** per "Resuming" above, comments included, re-deriving live state rather than trusting the body. 3. **Ask what it is blocked on first.** Where the link carries `blocked`, the parking comment names diff --git a/.agents/skills/unattended-handoff/SKILL.md b/.agents/skills/unattended-handoff/SKILL.md index b6bf958c..7fd62266 100644 --- a/.agents/skills/unattended-handoff/SKILL.md +++ b/.agents/skills/unattended-handoff/SKILL.md @@ -150,23 +150,24 @@ does not qualify, since skipping one costs nothing and a guess costs a revert an lacks. - **It reverses no settled decision** recorded in an issue, a handoff, or the rule text. - **Nothing has worked it or is working it.** The track `auto-` has no link, open or closed, - which `handoff.py chain --track "auto-" --limit 1` answers with its refusal naming no - handoff on that track. Any other refusal from it is a `STOP` rather than a yes. No open pull - request names it, and no pull request whose squash commit is in `origin/main..origin/develop` - names it anywhere in its body, since a fix merged to develop leaves its issue open until it is - promoted, whoever merged it. No open handoff on any track names it in its next steps, and no - comment on it claims it for a `backlog-burndown` group, since both mark work that has no pull - request yet. + which `handoff.py chain --repo "/" --track "auto-" --limit 1` answers with its + refusal naming no handoff on that track. Any other refusal from it is a `STOP` rather than a yes. + No open pull request names it, and no pull request whose squash commit is in + `origin/main..origin/develop` names it anywhere in its body, since a fix merged to develop leaves + its issue open until it is promoted, whoever merged it. No open handoff on any track names it in + its next steps, and no comment on it claims it for a `backlog-burndown` group, since both mark + work that has no pull request yet. ## The Picker 1. **Check the promotion first** under `main` or `release`. Where an open `decision` issue names the open develop -> main pull request, return `STOP` before picking anything, since every worker this run dispatched would meet that same decision after merging its own work to develop. -2. **Read the open handoffs** with labels and update times, `gh issue list --label handoff --state - open --limit 100 --json number,title,labels,updatedAt`, since `handoff.py tracks` prints - neither. Reach `scripts/handoff.py` from a hub checkout, per `session-handoff` "Running the - Chain". +2. **Read the open handoffs** with labels and update times, `gh issue list --repo "/" + --label handoff --state open --limit 100 --json number,title,labels,updatedAt`, since `handoff.py + tracks` prints neither. Where it returns as many rows as the limit, the list may be truncated, so + raise the limit and read again until it returns fewer, rather than ranking a partial list. Reach + `scripts/handoff.py` from a hub checkout, per `session-handoff` "Running the Chain". 3. **Prefer an open `auto-*` handoff not carrying `blocked`**, oldest first. That is a lane an earlier run parked and the maintainer has since unblocked, or one whose worker died, and a live link is work already framed. Handoffs on any other track belong to the maintainer's attended @@ -175,23 +176,27 @@ does not qualify, since skipping one costs nothing and a guess costs a revert an the session handing the lane back removes the label, per `GOVERNANCE.md` "Durable Knowledge and Self-Improvement". 4. **Otherwise pick from the backlog.** Rank the open issues by `backlog-burndown`'s "Ranking" - criteria, keep the auto-resolvable ones, and take the top one. Read the list with an explicit - page size, since `gh issue list` returns 30 rows unless told otherwise. -5. **Create its handoff** with `handoff.py new --track "auto-"`, `--dry-run` first. The body - carries the sections `session-handoff` "What Goes in the Body" names, with the next steps naming - the issue and what done looks like. That skill's rules on the body bind it. + criteria, keep the auto-resolvable ones, and take the top one. Read the list with `gh issue list + --repo "/" --state open` and an explicit `--limit`, since it returns 30 rows unless + told otherwise, and apply step 2's full-page check to it. +5. **Create its handoff** with `handoff.py new --repo "/" --track "auto-"`, + adding the `--title` and `--body-file` it also requires, as `session-handoff` "Running the Chain" + shows, and `--dry-run` first. The body carries the sections `session-handoff` "What Goes in the + Body" names, with the next steps naming the issue and what done looks like. That skill's rules on + the body bind it. 6. **Choose the worker's tier** by `backlog-burndown`'s "Choosing the Worker's Model Tier". 7. **Reply with one line.** A picker writes nothing but the handoff it creates, and returns `STOP` where a read it needs cannot run. ## The Worker -1. **Resume the handoff** with `handoff.py resume --track ""`, then read its comments with - `gh issue view "" --comments`, since `resume` prints only the body and a parked lane's state - is in its parking comment. A lane handed back by an attended session has a closed predecessor - holding that comment, so read the predecessor's comments too. Where either names a decision - issue, read the answer recorded there and follow it, since it is what unblocked the lane. - Re-derive live state rather than trusting any of them, per `session-handoff` "Resuming". +1. **Resume the handoff** with `handoff.py resume --repo "/" --track ""`, then + read its comments with `gh issue view "" --repo "/" --comments`, since `resume` + prints only the body and a parked lane's state is in its parking comment. A lane handed back by + an attended session has a closed predecessor holding that comment, so read the predecessor's + comments too. Where either names a decision issue, read the answer recorded there and follow it, + since it is what unblocked the lane. Re-derive live state rather than trusting any of them, per + `session-handoff` "Resuming". 2. **Isolate** in a worktree of its own, per `repo-worktree`, on the branch the handoff names or on `feature/`. 3. **Fix and drive.** Run `local-strict-review` before every push, and drive the pull request with diff --git a/.agents/skills/workflow-ci-contract/references/d-guarantees.md b/.agents/skills/workflow-ci-contract/references/d-guarantees.md index 1a396623..e293915b 100644 --- a/.agents/skills/workflow-ci-contract/references/d-guarantees.md +++ b/.agents/skills/workflow-ci-contract/references/d-guarantees.md @@ -11,11 +11,11 @@ The required behaviors, organized by domain. Each is a **MUST**, and its `Output ### D1 - PR Fast-Feedback (Smoke) - **D1.1 Only changed targets build.** Input: a PR touching some targets. Output: the paths-filter marks exactly those targets and only their smoke builds run. Unchanged targets skip. A repo's own targets MUST each have a filter entry (so a touched target is never silently skipped), and that entry lists paths rather than negating them, so a change matching no entry marks nothing and every smoke build skips. *Prevents: rebuilding everything, and a changed target slipping through unbuilt.* -- **D1.2 A validation job always runs.** Input: any PR. Output: a validation job runs unconditionally and the aggregator `needs:` it. That job is the caller's own job reaching the reusable validator, named `validate` in every shipped stub, and that name is what the aggregator's `needs:` carries. The validator's internal jobs (`lint`, `unit-test` and `validate` in the hub's `validate-task.yml`) are not addressable from a caller, so a `validate` in a caller's `needs:` list always names the caller's own job rather than the validator's internal one of the same name. The validator detects the tree rather than the repo's language, running the doc and repo gates everywhere, the `dotnet test` path only where a `*Tests*.csproj` project exists, and the `pytest` path only where a root `tests/` directory sits beside a root `pyproject.toml` and a root `uv.lock` or `requirements*.txt`, so a non-.NET repo calls the same one rather than replacing it. A repo whose tests take another shape can run them from its own `.github/actions/validate/action.yml` hook, which the validator runs where that file exists. That hook receives no secret, so a repo whose other-shape tests owe D1.6's coverage upload is one whose validation the validator cannot express. A repo whose validation it cannot express **replaces** the call (not deletes it) with its own validator and re-points the aggregator's `needs:` to the replacement. `smoke-build` `needs:` the `changes` job rather than the validation job, so no second `needs:` moves with it. *Prevents: a PR merging with no validation, or a dangling `needs:` that stops the whole workflow from loading.* +- **D1.2 A validation job always runs.** Input: any PR. Output: a validation job runs unconditionally and the aggregator `needs:` it. That job is the caller's own job reaching the reusable validator, named `validate` in every shipped stub, and that name is what the aggregator's `needs:` carries. The validator's internal jobs (`lint`, `unit-test` and `validate` in the hub's `validate-task.yml`) are not addressable from a caller, so a `validate` in a caller's `needs:` list always names the caller's own job rather than the validator's internal one of the same name. The validator detects the tree rather than the repo's language, running the doc and repo gates everywhere, the `dotnet test` path only where a `*Tests*.csproj` project exists, and the `pytest` path only where a root `tests/` directory sits beside a root `pyproject.toml` and a root `uv.lock` or `requirements*.txt`, so a non-.NET repo calls the same one rather than replacing it. It checks out the triggering commit, `github.sha`, with no `ref` input, because a reusable workflow reads its caller's `github` context, so a publisher's call validates the same commit its publish job pins. A repo whose tests take another shape can run them from its own `.github/actions/validate/action.yml` hook, which the validator runs where that file exists. That hook receives no secret, so a repo whose other-shape tests owe D1.6's coverage upload is one whose validation the validator cannot express. A repo whose validation it cannot express **replaces** the call (not deletes it) with its own validator and re-points the aggregator's `needs:` to the replacement. `smoke-build` `needs:` the `changes` job rather than the validation job, so no second `needs:` moves with it. *Prevents: a PR merging with no validation, or a dangling `needs:` that stops the whole workflow from loading.* - **D1.3 Smoke never publishes and never uploads.** Input: `smoke: true`. Output: full compile/lint/test, but no registry/image push, no release, and **no** artifact uploads (every `upload-artifact`, including any aggregation job, is gated on smoke being false, written `!inputs.smoke` at the workflow layer and `inputs.smoke != 'true'` in a composite action, whose inputs are strings). *Prevents: a PR publishing, and orphaned artifacts churning the storage quota.* - **D1.4 Workflow-file changes are not smoke-built.** Input: a PR changing only `.github/workflows/**`. Output: the paths-filter marks no target, so smoke-build skips. An inclusion list satisfying D1.1 reaches this by leaving workflow paths out of every target's entry. *Implication: a workflow-only change is not smoke-built, but actionlint still validates it in CI.* - **D1.5 One required aggregator gates merge.** Input: any PR. Output: a single aggregator job must **succeed**, run under `if: always()` so a failed or skipped dependency cannot skip the gate itself, `needs:` the validation job, and the `changes` and `smoke-build` jobs too wherever the repo has a smoke build, treat a **skipped** smoke build as pass, and **block** on `failure`/`cancelled`. Its name is ruleset-bound: the job `name:` and the ruleset `context:` are the same string and MUST be renamed together, never independently. *Prevents: a paths-filter error letting a target-changing PR merge unbuilt.* -- **D1.6 Coverage is reported to Codecov (C# and Python).** Input: a C# or Python repo that has tests for that type. Output: the validation job runs those tests under coverage collection (`dotnet test --coverage --coverage-output-format cobertura --results-directory ./coverage`, leaving `--coverage-output` unset so each test project writes its own report rather than overwriting a shared one, or `pytest --cov-report=xml` over a repo whose own `pyproject.toml` selects what to measure) and a `codecov/codecov-action` step uploads the report, **best-effort** (`continue-on-error` and/or `fail_ci_if_error: false`, so a Codecov outage or an absent token never reds the gate). The Python leg **fails its test step when no report was written**, since `--cov-report=xml` alone selects nothing to measure. The C# leg renames each report to `coverage-.cobertura.xml` before the upload step reads the directory, `codecov-cli`'s own finder not matching the default name, and a repo owning its validator rather than calling the hub's owes that rename itself. `CODECOV_TOKEN` lives in the repo's **actions** and **dependabot** secret stores, the second because a run triggered by a Dependabot pull request reads the Dependabot store and the upload would otherwise skip silently on every bot pull request. A caller reaching the reusable validator across repositories names the secret it passes (`secrets:` with `CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}`), on its pull request path and its publisher path alike, because `secrets: inherit` is documented for a caller in the same organization or enterprise, which a personal account is not. A call by local path stays inside one repository, where the caller's own store is the one the callee reads, so `secrets: inherit` is available there instead of naming each secret. The repo ships a **`codecov.yml`** setting the project and patch statuses to **`informational: true`** so a coverage delta never gates a pull request (a repo whose quality bar requires a threshold may turn that off), and excluding intentionally-untested, non-shipped code (an example or benchmark project) from the denominator via `ignore`. Coverage output is a build artifact, so `.gitignore` excludes it. The C# invocation runs under **Microsoft.Testing.Platform**, and the runner declaration, package references, and version floor an MTP-based test project needs are `CODESTYLE.md`'s .NET side. The Python invocation needs **`pytest-cov`** and a coverage selector, which are `CODESTYLE.md`'s Python side. N/A for a repo carrying no tests for that type, and for a `lint-only` profile for it (per the hub's `registry/repos.json`). *Prevents: coverage silently going unreported, and a coverage regression blocking an unrelated pull request.* +- **D1.6 Coverage is reported to Codecov (C# and Python).** Input: a C# or Python repo that has tests for that type. Output: the validation job runs those tests under coverage collection (`dotnet test --coverage --coverage-output-format cobertura --results-directory ./coverage`, leaving `--coverage-output` unset so each test project writes its own report rather than overwriting a shared one, or `pytest --cov-report=xml` over a repo whose own `pyproject.toml` selects what to measure) and a `codecov/codecov-action` step uploads the report, **best-effort** (`continue-on-error` and/or `fail_ci_if_error: false`, so a Codecov outage or an absent token never reds the gate). Each leg **fails its test step when the run wrote no report**, since the best-effort upload reads a missing report exactly as it reads a healthy run, and on the Python leg `--cov-report=xml` alone selects nothing to measure. The C# leg renames each report to `coverage-.cobertura.xml` before the upload step reads the directory, `codecov-cli`'s own finder not matching the default name, and a repo owning its validator rather than calling the hub's owes that rename itself. `CODECOV_TOKEN` lives in the repo's **actions** and **dependabot** secret stores, the second because a run triggered by a Dependabot pull request reads the Dependabot store and the upload would otherwise skip silently on every bot pull request. A caller reaching the reusable validator across repositories names the secret it passes (`secrets:` with `CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}`), on its pull request path and its publisher path alike, because `secrets: inherit` is documented for a caller in the same organization or enterprise, which a personal account is not. A call by local path stays inside one repository, where the caller's own store is the one the callee reads, so `secrets: inherit` is available there instead of naming each secret. The repo ships a **`codecov.yml`** setting the project and patch statuses to **`informational: true`** so a coverage delta never gates a pull request (a repo whose quality bar requires a threshold may turn that off), and excluding intentionally-untested, non-shipped code (an example or benchmark project) from the denominator via `ignore`. Coverage output is a build artifact, so `.gitignore` excludes it. The C# invocation runs under **Microsoft.Testing.Platform**, and the runner declaration, package references, and version floor an MTP-based test project needs are `CODESTYLE.md`'s .NET side. The Python invocation needs **`pytest-cov`** and a coverage selector, which are `CODESTYLE.md`'s Python side. N/A for a repo carrying no tests for that type, and for a `lint-only` profile for it (per the hub's `registry/repos.json`). *Prevents: coverage silently going unreported, and a coverage regression blocking an unrelated pull request.* ### D2 - Input/State Validation at Entry @@ -76,7 +76,7 @@ The required behaviors, organized by domain. Each is a **MUST**, and its `Output `GOVERNANCE.md` "Workflow YAML Conventions" names the tool D9.1 excepts and states the suffix rules D9.2 requires. -- **D9.1** Every action or reusable workflow referenced from another repository is SHA-pinned with a version comment (sole exception: the documented lagging-tag tool). A local (`./`) or self-repository (`$/`) reference names no ref and takes no pin. +- **D9.1** Every action or reusable workflow referenced from another repository is SHA-pinned with a comment naming the release tag at that SHA, spelled as the referenced repository publishes it (sole exception: the documented lagging-tag tool). A local (`./`) or self-repository (`$/`) reference names no ref and takes no pin, `$/` being GitHub's syntax, in a workflow file, for a path in the repository holding that file, resolved at that file's own commit. - **D9.2** File/workflow/job/step names follow the suffix rules. A ruleset-bound job's `name:` equals its ruleset `context:` (renamed together). - **D9.3** Multi-line bash `run:` blocks start `set -Eeuo pipefail`. Multi-line `if:` uses `>-`. - **D9.4** Docker layer cache targets a registry tag, not `type=gha`. `cache-to` writes only the built branch's `:buildcache-` and only on push, while `cache-from` reads both branches. A multi-image repo varies the cache **repository** rather than the tag, `:buildcache-` per image, the tag alone being unable to distinguish two images. diff --git a/.agents/skills/workflow-ci-contract/references/test-methodology.md b/.agents/skills/workflow-ci-contract/references/test-methodology.md index f7690a0a..36f359f2 100644 --- a/.agents/skills/workflow-ci-contract/references/test-methodology.md +++ b/.agents/skills/workflow-ci-contract/references/test-methodology.md @@ -18,7 +18,7 @@ Cite what each verdict rests on. That is `file:line` for a file in the audited r ### 5B. End-to-End Trace Scenarios (No Execution, Deterministic from the YAML) -For each *applicable* scenario, evaluate every job's `if:`/`needs:` against the inputs and emit the predicted **run/skip + version + release + artifact-end-state** table, then compare to the expected. A scenario governing a construct the repo does not contain is N/A, per `WORKFLOW.md` section 1, and an absent trigger is such a construct. Each scenario's trigger belongs to one workflow, so read that workflow's own `on:` block rather than the repo's type: S1 to S4 the pull request workflow's, S5 to S10 the publisher's, S11 the upstream tracker's, and S12 and S13 the deploy workflow's. A publisher carrying only `workflow_dispatch` therefore records S5, S6 and S9 N/A, their push and schedule paths never firing there, and a repo with no publisher at all records S5 to S10 N/A together. Where a scenario's path runs through a workflow or composite action the repo only **calls**, trace that callee as the repo reaches it, read at the SHA the caller pins rather than at the callee's current default branch, which is the same evidence rule 5A states. Predicting from the callee's `main` predicts a table for YAML the audited repo never runs. A local (`./`) or self-repository (`$/`) call carries no pin of its own and runs at the workflow commit, so it is traced at whatever SHA the outermost pinning caller fixed. Minimum set: +For each *applicable* scenario, evaluate every job's `if:`/`needs:` against the inputs and emit the predicted **run/skip + version + release + artifact-end-state** table, then compare to the expected. A scenario governing a construct the repo does not contain is N/A, per `WORKFLOW.md` section 1, and an absent trigger is such a construct. Each scenario's trigger belongs to one workflow, so read that workflow's own `on:` block rather than the repo's type: S1 to S4 the pull request workflow's, S5 to S10 the publisher's, S11 the upstream tracker's, and S12 and S13 the deploy workflow's. A publisher carrying only `workflow_dispatch` therefore records S5, S6 and S9 N/A, their push and schedule paths never firing there, and a repo with no publisher at all records S5 to S10 N/A together. Where a scenario's path runs through a workflow or composite action the repo only **calls**, trace that callee as the repo reaches it, read at the SHA the caller pins rather than at the callee's current default branch, which is the same evidence rule 5A states. Predicting from the callee's `main` predicts a table for YAML the audited repo never runs. A self-repository (`$/`) call or a job-level local (`./`) reusable-workflow call carries no pin of its own and resolves at the calling workflow file's commit, so it is traced at that file's own commit, which is the SHA fixed by the pin that reached that file, or the audited commit where that file is one of the audited repository's own workflows. A local (`./`) action reference carries no pin either, but it resolves against whatever the job checked out, so it is traced at that checkout's commit, which is the caller's where a reusable workflow called from another repository checks out its caller. Minimum set: | # | Input | Expected output | Exercises | | --- | --- | --- | --- | diff --git a/.claude-plugin/fleet-skills/.source-digests/backlog-burndown b/.claude-plugin/fleet-skills/.source-digests/backlog-burndown index e30ce78a..becc2b93 100644 --- a/.claude-plugin/fleet-skills/.source-digests/backlog-burndown +++ b/.claude-plugin/fleet-skills/.source-digests/backlog-burndown @@ -1 +1 @@ -686a83cdd2f01a9a +c97175a5727deced diff --git a/.claude-plugin/fleet-skills/.source-digests/dotnet-codestyle b/.claude-plugin/fleet-skills/.source-digests/dotnet-codestyle index a221fb7b..f6d020a2 100644 --- a/.claude-plugin/fleet-skills/.source-digests/dotnet-codestyle +++ b/.claude-plugin/fleet-skills/.source-digests/dotnet-codestyle @@ -1 +1 @@ -770653a908016ec0 +4e9d6c62e1680a27 diff --git a/.claude-plugin/fleet-skills/.source-digests/session-handoff b/.claude-plugin/fleet-skills/.source-digests/session-handoff index a08f2e05..eb379d44 100644 --- a/.claude-plugin/fleet-skills/.source-digests/session-handoff +++ b/.claude-plugin/fleet-skills/.source-digests/session-handoff @@ -1 +1 @@ -d4779b1b69890b2e +26629801974b6dfc diff --git a/.claude-plugin/fleet-skills/.source-digests/unattended-handoff b/.claude-plugin/fleet-skills/.source-digests/unattended-handoff index cd61a77a..336cf136 100644 --- a/.claude-plugin/fleet-skills/.source-digests/unattended-handoff +++ b/.claude-plugin/fleet-skills/.source-digests/unattended-handoff @@ -1 +1 @@ -00496091bfa41fa9 +1a69a9029eef4d65 diff --git a/.claude-plugin/fleet-skills/.source-digests/workflow-ci-contract b/.claude-plugin/fleet-skills/.source-digests/workflow-ci-contract index e9204663..a40930e9 100644 --- a/.claude-plugin/fleet-skills/.source-digests/workflow-ci-contract +++ b/.claude-plugin/fleet-skills/.source-digests/workflow-ci-contract @@ -1 +1 @@ -9c7418eadc5708fa +ac509cee1085405f diff --git a/.claude-plugin/fleet-skills/skills/backlog-burndown/SKILL.md b/.claude-plugin/fleet-skills/skills/backlog-burndown/SKILL.md index bb4c6119..91317aa8 100644 --- a/.claude-plugin/fleet-skills/skills/backlog-burndown/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/backlog-burndown/SKILL.md @@ -114,7 +114,9 @@ handoff chain `AGENTS.md` "Session Scope" defines, so it records work to do next its own, and an open one is present by design for as long as that chain is in use. Counting it inflates the number this run reports as the backlog by one for every lane in use, and a backlog count this fleet reports wrong is a failure with its own history, so filter the label out of the -ranking and out of every count of the open backlog rather than out of the ranking alone. +ranking and out of every count of the open backlog rather than out of the ranking alone. A link +carrying `blocked` as well, as a parked lane does, is still a link, so this rule controls over the +`blocked` rule below and the link is neither ranked nor counted. An issue carrying the `blocked` label is counted and is not ranked while its blocker stands. It records real work this repository owes, which is why it stays in the count, and the label says the diff --git a/.claude-plugin/fleet-skills/skills/dotnet-codestyle/SKILL.md b/.claude-plugin/fleet-skills/skills/dotnet-codestyle/SKILL.md index 7eb20897..b13de18d 100644 --- a/.claude-plugin/fleet-skills/skills/dotnet-codestyle/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/dotnet-codestyle/SKILL.md @@ -214,7 +214,7 @@ parameterized tests. A test project on `xunit.v3` 4.0.0 or later is MTP-based, a ## Project configuration -.NET 10.0 target, AOT-compatible (`IsAotCompatible=true`, `VerifyReferenceAotCompatibility=true`), +.NET 10.0 target, AOT-compatible (`IsAotCompatible=true`, with `VerifyReferenceAotCompatibility=true` only under the `PublishAot` condition), SourceLink, embedded untracked sources, `InternalsVisibleTo` for test/benchmark access. See `references/project-config.md` for the full property list. diff --git a/.claude-plugin/fleet-skills/skills/dotnet-codestyle/references/project-config.md b/.claude-plugin/fleet-skills/skills/dotnet-codestyle/references/project-config.md index 42b8fd1c..658e589c 100644 --- a/.claude-plugin/fleet-skills/skills/dotnet-codestyle/references/project-config.md +++ b/.claude-plugin/fleet-skills/skills/dotnet-codestyle/references/project-config.md @@ -1,8 +1,14 @@ # .NET Project Configuration 1. **Target framework**: .NET 10.0 (`net10.0`). -2. **AOT compatibility**: `true`, - `true`. +2. **AOT compatibility**: `true` unconditionally, and + `true` only in a + ``, placed in the `.csproj` after it sets + `PublishAot` or in `Directory.Build.targets`, never in `Directory.Build.props`, which is imported + before the project body and so never sees a `PublishAot` the `.csproj` sets. Reference + verification reports `IL3058` for every referenced assembly that lacks `IsAotCompatible` metadata + set to `true`, and `TreatWarningsAsErrors` turns that into a failed build on any such + dependency, so it runs only where an AOT publish needs it. 3. **Assembly information**: use semantic versioning, include SourceLink (`true`), embed untracked sources (`true`). diff --git a/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md b/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md index d0504cbc..d5eda1ae 100644 --- a/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md @@ -143,8 +143,9 @@ section is the pointer to what to read rather than the answer, and `AGENTS.md` " already says stale context is worse than absent. A handoff is context by construction, so this is that rule applied to the one artifact built to outlive the session that wrote it. -Read the current link's comments too, with `gh issue view "" --comments`, since `resume` prints -only the body, and a parking comment or a closing session's answers land in the comments. +Read the current link's comments too, with `gh issue view "" --repo OWNER/NAME --comments`, since +`resume` prints only the body, and a parking comment or a closing session's answers land in the +comments. Read the chain before re-attempting anything. `resume` prints the current body and indexes the closed links behind it, and `chain --grep` searches the bodies it walks for a regular expression, @@ -161,7 +162,9 @@ rather than `resume`. Re-derive a count rather than copying one, and read it with an explicit page size. `gh issue list` returns 30 rows unless told otherwise, and a truncated count reads exactly like a repository with -30 issues, which is worse than an absent count because it gets stated. +30 issues, which is worse than an absent count because it gets stated. The same holds at any page +size, so where a read returns as many rows as its limit, raise the limit and read again until it +returns fewer. ## The Attended Session @@ -169,15 +172,17 @@ A maintainer resuming work says little, often only "resume the handoff", and tha the whole procedure below. Run every step without being reminded of any of them. 1. **Pick the link.** Where the maintainer names an issue, take it. Where none is named, read - `gh issue list --label handoff --state open --limit 100 --json number,title,labels,updatedAt`, - since `tracks` prints neither labels nor exact update times. Take a link carrying `blocked` - first, newest update first among them, since its blocker is a decision only the maintainer can - make and the maintainer is now present. Otherwise take the newest update among the rest. An - `auto-*` link not carrying `blocked` is skipped unless named, since an `unattended-handoff` - worker may hold it right now, and where one is named, confirm with the maintainer that no - unattended run is live before working it. Where every open link is skipped, say so and ask the - maintainer which to take. Say which link was picked in one line before anything else, so a wrong pick costs one reply - rather than a round. + `gh issue list --repo OWNER/NAME --label handoff --state open --limit 100 --json + number,title,labels,updatedAt`, since `tracks` prints neither labels nor exact update times. + Where it returns as many rows as the limit, the list may be truncated, so raise the limit and + read again until it returns fewer, rather than picking from a partial list. Take a link carrying + `blocked` first, newest update first among them, since its blocker is a decision only the + maintainer can make and the maintainer is now present. Otherwise take the newest update among the + rest. An `auto-*` link not carrying `blocked` is skipped unless named, since an + `unattended-handoff` worker may hold it right now, and where one is named, confirm with the + maintainer that no unattended run is live before working it. Where every open link is skipped, + say so and ask the maintainer which to take. Say which link was picked in one line before + anything else, so a wrong pick costs one reply rather than a round. 2. **Resume it** per "Resuming" above, comments included, re-deriving live state rather than trusting the body. 3. **Ask what it is blocked on first.** Where the link carries `blocked`, the parking comment names diff --git a/.claude-plugin/fleet-skills/skills/unattended-handoff/SKILL.md b/.claude-plugin/fleet-skills/skills/unattended-handoff/SKILL.md index b6bf958c..7fd62266 100644 --- a/.claude-plugin/fleet-skills/skills/unattended-handoff/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/unattended-handoff/SKILL.md @@ -150,23 +150,24 @@ does not qualify, since skipping one costs nothing and a guess costs a revert an lacks. - **It reverses no settled decision** recorded in an issue, a handoff, or the rule text. - **Nothing has worked it or is working it.** The track `auto-` has no link, open or closed, - which `handoff.py chain --track "auto-" --limit 1` answers with its refusal naming no - handoff on that track. Any other refusal from it is a `STOP` rather than a yes. No open pull - request names it, and no pull request whose squash commit is in `origin/main..origin/develop` - names it anywhere in its body, since a fix merged to develop leaves its issue open until it is - promoted, whoever merged it. No open handoff on any track names it in its next steps, and no - comment on it claims it for a `backlog-burndown` group, since both mark work that has no pull - request yet. + which `handoff.py chain --repo "/" --track "auto-" --limit 1` answers with its + refusal naming no handoff on that track. Any other refusal from it is a `STOP` rather than a yes. + No open pull request names it, and no pull request whose squash commit is in + `origin/main..origin/develop` names it anywhere in its body, since a fix merged to develop leaves + its issue open until it is promoted, whoever merged it. No open handoff on any track names it in + its next steps, and no comment on it claims it for a `backlog-burndown` group, since both mark + work that has no pull request yet. ## The Picker 1. **Check the promotion first** under `main` or `release`. Where an open `decision` issue names the open develop -> main pull request, return `STOP` before picking anything, since every worker this run dispatched would meet that same decision after merging its own work to develop. -2. **Read the open handoffs** with labels and update times, `gh issue list --label handoff --state - open --limit 100 --json number,title,labels,updatedAt`, since `handoff.py tracks` prints - neither. Reach `scripts/handoff.py` from a hub checkout, per `session-handoff` "Running the - Chain". +2. **Read the open handoffs** with labels and update times, `gh issue list --repo "/" + --label handoff --state open --limit 100 --json number,title,labels,updatedAt`, since `handoff.py + tracks` prints neither. Where it returns as many rows as the limit, the list may be truncated, so + raise the limit and read again until it returns fewer, rather than ranking a partial list. Reach + `scripts/handoff.py` from a hub checkout, per `session-handoff` "Running the Chain". 3. **Prefer an open `auto-*` handoff not carrying `blocked`**, oldest first. That is a lane an earlier run parked and the maintainer has since unblocked, or one whose worker died, and a live link is work already framed. Handoffs on any other track belong to the maintainer's attended @@ -175,23 +176,27 @@ does not qualify, since skipping one costs nothing and a guess costs a revert an the session handing the lane back removes the label, per `GOVERNANCE.md` "Durable Knowledge and Self-Improvement". 4. **Otherwise pick from the backlog.** Rank the open issues by `backlog-burndown`'s "Ranking" - criteria, keep the auto-resolvable ones, and take the top one. Read the list with an explicit - page size, since `gh issue list` returns 30 rows unless told otherwise. -5. **Create its handoff** with `handoff.py new --track "auto-"`, `--dry-run` first. The body - carries the sections `session-handoff` "What Goes in the Body" names, with the next steps naming - the issue and what done looks like. That skill's rules on the body bind it. + criteria, keep the auto-resolvable ones, and take the top one. Read the list with `gh issue list + --repo "/" --state open` and an explicit `--limit`, since it returns 30 rows unless + told otherwise, and apply step 2's full-page check to it. +5. **Create its handoff** with `handoff.py new --repo "/" --track "auto-"`, + adding the `--title` and `--body-file` it also requires, as `session-handoff` "Running the Chain" + shows, and `--dry-run` first. The body carries the sections `session-handoff` "What Goes in the + Body" names, with the next steps naming the issue and what done looks like. That skill's rules on + the body bind it. 6. **Choose the worker's tier** by `backlog-burndown`'s "Choosing the Worker's Model Tier". 7. **Reply with one line.** A picker writes nothing but the handoff it creates, and returns `STOP` where a read it needs cannot run. ## The Worker -1. **Resume the handoff** with `handoff.py resume --track ""`, then read its comments with - `gh issue view "" --comments`, since `resume` prints only the body and a parked lane's state - is in its parking comment. A lane handed back by an attended session has a closed predecessor - holding that comment, so read the predecessor's comments too. Where either names a decision - issue, read the answer recorded there and follow it, since it is what unblocked the lane. - Re-derive live state rather than trusting any of them, per `session-handoff` "Resuming". +1. **Resume the handoff** with `handoff.py resume --repo "/" --track ""`, then + read its comments with `gh issue view "" --repo "/" --comments`, since `resume` + prints only the body and a parked lane's state is in its parking comment. A lane handed back by + an attended session has a closed predecessor holding that comment, so read the predecessor's + comments too. Where either names a decision issue, read the answer recorded there and follow it, + since it is what unblocked the lane. Re-derive live state rather than trusting any of them, per + `session-handoff` "Resuming". 2. **Isolate** in a worktree of its own, per `repo-worktree`, on the branch the handoff names or on `feature/`. 3. **Fix and drive.** Run `local-strict-review` before every push, and drive the pull request with diff --git a/.claude-plugin/fleet-skills/skills/workflow-ci-contract/references/d-guarantees.md b/.claude-plugin/fleet-skills/skills/workflow-ci-contract/references/d-guarantees.md index 1a396623..e293915b 100644 --- a/.claude-plugin/fleet-skills/skills/workflow-ci-contract/references/d-guarantees.md +++ b/.claude-plugin/fleet-skills/skills/workflow-ci-contract/references/d-guarantees.md @@ -11,11 +11,11 @@ The required behaviors, organized by domain. Each is a **MUST**, and its `Output ### D1 - PR Fast-Feedback (Smoke) - **D1.1 Only changed targets build.** Input: a PR touching some targets. Output: the paths-filter marks exactly those targets and only their smoke builds run. Unchanged targets skip. A repo's own targets MUST each have a filter entry (so a touched target is never silently skipped), and that entry lists paths rather than negating them, so a change matching no entry marks nothing and every smoke build skips. *Prevents: rebuilding everything, and a changed target slipping through unbuilt.* -- **D1.2 A validation job always runs.** Input: any PR. Output: a validation job runs unconditionally and the aggregator `needs:` it. That job is the caller's own job reaching the reusable validator, named `validate` in every shipped stub, and that name is what the aggregator's `needs:` carries. The validator's internal jobs (`lint`, `unit-test` and `validate` in the hub's `validate-task.yml`) are not addressable from a caller, so a `validate` in a caller's `needs:` list always names the caller's own job rather than the validator's internal one of the same name. The validator detects the tree rather than the repo's language, running the doc and repo gates everywhere, the `dotnet test` path only where a `*Tests*.csproj` project exists, and the `pytest` path only where a root `tests/` directory sits beside a root `pyproject.toml` and a root `uv.lock` or `requirements*.txt`, so a non-.NET repo calls the same one rather than replacing it. A repo whose tests take another shape can run them from its own `.github/actions/validate/action.yml` hook, which the validator runs where that file exists. That hook receives no secret, so a repo whose other-shape tests owe D1.6's coverage upload is one whose validation the validator cannot express. A repo whose validation it cannot express **replaces** the call (not deletes it) with its own validator and re-points the aggregator's `needs:` to the replacement. `smoke-build` `needs:` the `changes` job rather than the validation job, so no second `needs:` moves with it. *Prevents: a PR merging with no validation, or a dangling `needs:` that stops the whole workflow from loading.* +- **D1.2 A validation job always runs.** Input: any PR. Output: a validation job runs unconditionally and the aggregator `needs:` it. That job is the caller's own job reaching the reusable validator, named `validate` in every shipped stub, and that name is what the aggregator's `needs:` carries. The validator's internal jobs (`lint`, `unit-test` and `validate` in the hub's `validate-task.yml`) are not addressable from a caller, so a `validate` in a caller's `needs:` list always names the caller's own job rather than the validator's internal one of the same name. The validator detects the tree rather than the repo's language, running the doc and repo gates everywhere, the `dotnet test` path only where a `*Tests*.csproj` project exists, and the `pytest` path only where a root `tests/` directory sits beside a root `pyproject.toml` and a root `uv.lock` or `requirements*.txt`, so a non-.NET repo calls the same one rather than replacing it. It checks out the triggering commit, `github.sha`, with no `ref` input, because a reusable workflow reads its caller's `github` context, so a publisher's call validates the same commit its publish job pins. A repo whose tests take another shape can run them from its own `.github/actions/validate/action.yml` hook, which the validator runs where that file exists. That hook receives no secret, so a repo whose other-shape tests owe D1.6's coverage upload is one whose validation the validator cannot express. A repo whose validation it cannot express **replaces** the call (not deletes it) with its own validator and re-points the aggregator's `needs:` to the replacement. `smoke-build` `needs:` the `changes` job rather than the validation job, so no second `needs:` moves with it. *Prevents: a PR merging with no validation, or a dangling `needs:` that stops the whole workflow from loading.* - **D1.3 Smoke never publishes and never uploads.** Input: `smoke: true`. Output: full compile/lint/test, but no registry/image push, no release, and **no** artifact uploads (every `upload-artifact`, including any aggregation job, is gated on smoke being false, written `!inputs.smoke` at the workflow layer and `inputs.smoke != 'true'` in a composite action, whose inputs are strings). *Prevents: a PR publishing, and orphaned artifacts churning the storage quota.* - **D1.4 Workflow-file changes are not smoke-built.** Input: a PR changing only `.github/workflows/**`. Output: the paths-filter marks no target, so smoke-build skips. An inclusion list satisfying D1.1 reaches this by leaving workflow paths out of every target's entry. *Implication: a workflow-only change is not smoke-built, but actionlint still validates it in CI.* - **D1.5 One required aggregator gates merge.** Input: any PR. Output: a single aggregator job must **succeed**, run under `if: always()` so a failed or skipped dependency cannot skip the gate itself, `needs:` the validation job, and the `changes` and `smoke-build` jobs too wherever the repo has a smoke build, treat a **skipped** smoke build as pass, and **block** on `failure`/`cancelled`. Its name is ruleset-bound: the job `name:` and the ruleset `context:` are the same string and MUST be renamed together, never independently. *Prevents: a paths-filter error letting a target-changing PR merge unbuilt.* -- **D1.6 Coverage is reported to Codecov (C# and Python).** Input: a C# or Python repo that has tests for that type. Output: the validation job runs those tests under coverage collection (`dotnet test --coverage --coverage-output-format cobertura --results-directory ./coverage`, leaving `--coverage-output` unset so each test project writes its own report rather than overwriting a shared one, or `pytest --cov-report=xml` over a repo whose own `pyproject.toml` selects what to measure) and a `codecov/codecov-action` step uploads the report, **best-effort** (`continue-on-error` and/or `fail_ci_if_error: false`, so a Codecov outage or an absent token never reds the gate). The Python leg **fails its test step when no report was written**, since `--cov-report=xml` alone selects nothing to measure. The C# leg renames each report to `coverage-.cobertura.xml` before the upload step reads the directory, `codecov-cli`'s own finder not matching the default name, and a repo owning its validator rather than calling the hub's owes that rename itself. `CODECOV_TOKEN` lives in the repo's **actions** and **dependabot** secret stores, the second because a run triggered by a Dependabot pull request reads the Dependabot store and the upload would otherwise skip silently on every bot pull request. A caller reaching the reusable validator across repositories names the secret it passes (`secrets:` with `CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}`), on its pull request path and its publisher path alike, because `secrets: inherit` is documented for a caller in the same organization or enterprise, which a personal account is not. A call by local path stays inside one repository, where the caller's own store is the one the callee reads, so `secrets: inherit` is available there instead of naming each secret. The repo ships a **`codecov.yml`** setting the project and patch statuses to **`informational: true`** so a coverage delta never gates a pull request (a repo whose quality bar requires a threshold may turn that off), and excluding intentionally-untested, non-shipped code (an example or benchmark project) from the denominator via `ignore`. Coverage output is a build artifact, so `.gitignore` excludes it. The C# invocation runs under **Microsoft.Testing.Platform**, and the runner declaration, package references, and version floor an MTP-based test project needs are `CODESTYLE.md`'s .NET side. The Python invocation needs **`pytest-cov`** and a coverage selector, which are `CODESTYLE.md`'s Python side. N/A for a repo carrying no tests for that type, and for a `lint-only` profile for it (per the hub's `registry/repos.json`). *Prevents: coverage silently going unreported, and a coverage regression blocking an unrelated pull request.* +- **D1.6 Coverage is reported to Codecov (C# and Python).** Input: a C# or Python repo that has tests for that type. Output: the validation job runs those tests under coverage collection (`dotnet test --coverage --coverage-output-format cobertura --results-directory ./coverage`, leaving `--coverage-output` unset so each test project writes its own report rather than overwriting a shared one, or `pytest --cov-report=xml` over a repo whose own `pyproject.toml` selects what to measure) and a `codecov/codecov-action` step uploads the report, **best-effort** (`continue-on-error` and/or `fail_ci_if_error: false`, so a Codecov outage or an absent token never reds the gate). Each leg **fails its test step when the run wrote no report**, since the best-effort upload reads a missing report exactly as it reads a healthy run, and on the Python leg `--cov-report=xml` alone selects nothing to measure. The C# leg renames each report to `coverage-.cobertura.xml` before the upload step reads the directory, `codecov-cli`'s own finder not matching the default name, and a repo owning its validator rather than calling the hub's owes that rename itself. `CODECOV_TOKEN` lives in the repo's **actions** and **dependabot** secret stores, the second because a run triggered by a Dependabot pull request reads the Dependabot store and the upload would otherwise skip silently on every bot pull request. A caller reaching the reusable validator across repositories names the secret it passes (`secrets:` with `CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}`), on its pull request path and its publisher path alike, because `secrets: inherit` is documented for a caller in the same organization or enterprise, which a personal account is not. A call by local path stays inside one repository, where the caller's own store is the one the callee reads, so `secrets: inherit` is available there instead of naming each secret. The repo ships a **`codecov.yml`** setting the project and patch statuses to **`informational: true`** so a coverage delta never gates a pull request (a repo whose quality bar requires a threshold may turn that off), and excluding intentionally-untested, non-shipped code (an example or benchmark project) from the denominator via `ignore`. Coverage output is a build artifact, so `.gitignore` excludes it. The C# invocation runs under **Microsoft.Testing.Platform**, and the runner declaration, package references, and version floor an MTP-based test project needs are `CODESTYLE.md`'s .NET side. The Python invocation needs **`pytest-cov`** and a coverage selector, which are `CODESTYLE.md`'s Python side. N/A for a repo carrying no tests for that type, and for a `lint-only` profile for it (per the hub's `registry/repos.json`). *Prevents: coverage silently going unreported, and a coverage regression blocking an unrelated pull request.* ### D2 - Input/State Validation at Entry @@ -76,7 +76,7 @@ The required behaviors, organized by domain. Each is a **MUST**, and its `Output `GOVERNANCE.md` "Workflow YAML Conventions" names the tool D9.1 excepts and states the suffix rules D9.2 requires. -- **D9.1** Every action or reusable workflow referenced from another repository is SHA-pinned with a version comment (sole exception: the documented lagging-tag tool). A local (`./`) or self-repository (`$/`) reference names no ref and takes no pin. +- **D9.1** Every action or reusable workflow referenced from another repository is SHA-pinned with a comment naming the release tag at that SHA, spelled as the referenced repository publishes it (sole exception: the documented lagging-tag tool). A local (`./`) or self-repository (`$/`) reference names no ref and takes no pin, `$/` being GitHub's syntax, in a workflow file, for a path in the repository holding that file, resolved at that file's own commit. - **D9.2** File/workflow/job/step names follow the suffix rules. A ruleset-bound job's `name:` equals its ruleset `context:` (renamed together). - **D9.3** Multi-line bash `run:` blocks start `set -Eeuo pipefail`. Multi-line `if:` uses `>-`. - **D9.4** Docker layer cache targets a registry tag, not `type=gha`. `cache-to` writes only the built branch's `:buildcache-` and only on push, while `cache-from` reads both branches. A multi-image repo varies the cache **repository** rather than the tag, `:buildcache-` per image, the tag alone being unable to distinguish two images. diff --git a/.claude-plugin/fleet-skills/skills/workflow-ci-contract/references/test-methodology.md b/.claude-plugin/fleet-skills/skills/workflow-ci-contract/references/test-methodology.md index f7690a0a..36f359f2 100644 --- a/.claude-plugin/fleet-skills/skills/workflow-ci-contract/references/test-methodology.md +++ b/.claude-plugin/fleet-skills/skills/workflow-ci-contract/references/test-methodology.md @@ -18,7 +18,7 @@ Cite what each verdict rests on. That is `file:line` for a file in the audited r ### 5B. End-to-End Trace Scenarios (No Execution, Deterministic from the YAML) -For each *applicable* scenario, evaluate every job's `if:`/`needs:` against the inputs and emit the predicted **run/skip + version + release + artifact-end-state** table, then compare to the expected. A scenario governing a construct the repo does not contain is N/A, per `WORKFLOW.md` section 1, and an absent trigger is such a construct. Each scenario's trigger belongs to one workflow, so read that workflow's own `on:` block rather than the repo's type: S1 to S4 the pull request workflow's, S5 to S10 the publisher's, S11 the upstream tracker's, and S12 and S13 the deploy workflow's. A publisher carrying only `workflow_dispatch` therefore records S5, S6 and S9 N/A, their push and schedule paths never firing there, and a repo with no publisher at all records S5 to S10 N/A together. Where a scenario's path runs through a workflow or composite action the repo only **calls**, trace that callee as the repo reaches it, read at the SHA the caller pins rather than at the callee's current default branch, which is the same evidence rule 5A states. Predicting from the callee's `main` predicts a table for YAML the audited repo never runs. A local (`./`) or self-repository (`$/`) call carries no pin of its own and runs at the workflow commit, so it is traced at whatever SHA the outermost pinning caller fixed. Minimum set: +For each *applicable* scenario, evaluate every job's `if:`/`needs:` against the inputs and emit the predicted **run/skip + version + release + artifact-end-state** table, then compare to the expected. A scenario governing a construct the repo does not contain is N/A, per `WORKFLOW.md` section 1, and an absent trigger is such a construct. Each scenario's trigger belongs to one workflow, so read that workflow's own `on:` block rather than the repo's type: S1 to S4 the pull request workflow's, S5 to S10 the publisher's, S11 the upstream tracker's, and S12 and S13 the deploy workflow's. A publisher carrying only `workflow_dispatch` therefore records S5, S6 and S9 N/A, their push and schedule paths never firing there, and a repo with no publisher at all records S5 to S10 N/A together. Where a scenario's path runs through a workflow or composite action the repo only **calls**, trace that callee as the repo reaches it, read at the SHA the caller pins rather than at the callee's current default branch, which is the same evidence rule 5A states. Predicting from the callee's `main` predicts a table for YAML the audited repo never runs. A self-repository (`$/`) call or a job-level local (`./`) reusable-workflow call carries no pin of its own and resolves at the calling workflow file's commit, so it is traced at that file's own commit, which is the SHA fixed by the pin that reached that file, or the audited commit where that file is one of the audited repository's own workflows. A local (`./`) action reference carries no pin either, but it resolves against whatever the job checked out, so it is traced at that checkout's commit, which is the caller's where a reusable workflow called from another repository checks out its caller. Minimum set: | # | Input | Expected output | Exercises | | --- | --- | --- | --- | diff --git a/.github/actions/repo-gate/repo_gate.py b/.github/actions/repo-gate/repo_gate.py index 275cb9da..2d43d651 100755 --- a/.github/actions/repo-gate/repo_gate.py +++ b/.github/actions/repo-gate/repo_gate.py @@ -52,6 +52,7 @@ # Reading any of those as absence fails a correct pin, which is the direction that costs most. ABSENT = {"404", "422"} GH_TIMEOUT = 20 +CONTROL = re.compile(r"[\x00-\x1f\x7f-\x9f]") # How a check says it did less than its name. # A gate that quietly degrades to a weaker reading prints the same clean line as one that ran. @@ -62,7 +63,12 @@ def sh(*args: str) -> str: return subprocess.run( - args, capture_output=True, text=True, encoding="utf-8", check=False + args, + capture_output=True, + text=True, + encoding="utf-8", + errors="surrogateescape", + check=False, ).stdout @@ -125,18 +131,51 @@ def tracked(root: Path, exclude: list[str] | None = None) -> list[str]: A caller vendoring a subtree it does not author, per GOVERNANCE.md's carry-versus-reach test, can scope every check out of that subtree this way. No check itself needs to change. Additive only: an empty or absent `exclude` scans exactly what it always has. + + Git's quoting is pinned on, so the listing is ASCII whatever a config inherits and each quoted + name reaches `unquote_path` in the one form it decodes. Git's stderr carries no such quoting, and + a failure naming a root that is not UTF-8 echoes that name raw, so the decode tolerates it. """ - args = ["git", "-C", str(root), "ls-files"] + args = ["git", "-C", str(root), "-c", "core.quotePath=true", "ls-files"] if exclude: args += ["--", *(f":!{pattern}" for pattern in exclude)] - result = subprocess.run(args, capture_output=True, text=True, encoding="utf-8", check=False) + result = subprocess.run( + args, + capture_output=True, + text=True, + encoding="utf-8", + errors="surrogateescape", + check=False, + ) if result.returncode != 0: # A failed command's stdout is never trusted, even where it is non-empty. # A partial listing read as complete is a scan that missed files and said nothing. reason = result.stderr.strip() or f"exit {result.returncode}, no stderr" - print(f"git ls-files failed: {reason}", file=sys.stderr) + print(f"git ls-files failed: {printable(reason)}", file=sys.stderr) return [] - return [l for l in result.stdout.split("\n") if l] + return [unquote_path(l) for l in result.stdout.split("\n") if l] + + +def unquote_path(name: str) -> str: + """The real name behind one `git ls-files` line, which git quotes when the name needs it. + + Git quotes a name holding any byte at or above 0x80, and one holding a quote, a backslash, + or a control character, escaping it the way C does. Read as a literal path, the quoted form + names no file on disk, so a check reading the file would pass over it and report clean. + + The caller pins `core.quotePath=true`, which is what makes a quoted line ASCII and so what + this decode assumes. Turning the setting off instead would not do: it stops git quoting the + first of those three routes and leaves the other two quoting a name whose non-ASCII bytes sit + raw inside the quotes, which this decode cannot carry. + + A byte that is not valid UTF-8 comes back as a surrogate escape, the form `Path` and + `resolved_eol` both encode back to the original byte. Latin-1 carries each unescaped byte + through unchanged on the way there. + """ + if name.startswith('"') and name.endswith('"') and len(name) > 1: + unescaped = name[1:-1].encode("latin-1", "backslashreplace").decode("unicode-escape") + return unescaped.encode("latin-1", "surrogateescape").decode("utf-8", "surrogateescape") + return name def workflow_files(files: list[str]) -> list[str]: @@ -179,8 +218,9 @@ def resolved_eol(root: Path, paths: list[str]) -> dict[str, str] | None: def check_sha_pin(root: Path, files: list[str]) -> list[str]: """Every external `uses:` is a 40-hex SHA, and one under this owner resolves. - A local or self-repository ref names the running commit and is skipped. References under - another owner are shape-checked but not resolved. + A local (`./`) or self-repository (`$/`) ref names no ref to pin and is skipped, and so is + one starting with a bare `.github/`, unvalidated. References under another owner are + shape-checked but not resolved. Resolution is scoped to the scanned repository's own owner, because that is where the fleet's own actions live and where the decay this catches comes from: a squash merge deletes the @@ -360,7 +400,36 @@ def check_eol_coverage(root: Path, files: list[str]) -> list[str]: CHECKS = {"sha-pin": check_sha_pin, "eol": check_eol, "eol-coverage": check_eol_coverage} +def printable(line: str) -> str: + """`line` with each control character spelled as an escape, so one finding prints as one line. + + `unquote_path` hands back a tracked name exactly as it is on disk, and a name may hold a + newline or an escape sequence. Printed raw, such a name forges a line of the gate's own output + or drives the reader's terminal, so each C0 or C1 control and DEL is escaped here, where it is + shown. A byte that is not UTF-8 arrives as a lone surrogate, which a strict stream refuses to + encode, so it is spelled as an escape too, leaving the result safe on any stream. + """ + escaped = CONTROL.sub(lambda m: f"\\x{ord(m.group()):02x}", line) + return escaped.encode("utf-8", "backslashreplace").decode("utf-8") + + +def report_paths_that_are_not_utf8() -> None: + """Let a path holding a byte that is not UTF-8 print rather than ending the run. + + `unquote_path` decodes such a name with surrogateescape so it opens on disk, which leaves the + lone surrogate in the name to reach this program's own output. Encoding it strictly raises at + the line printing that name, so every check after it is lost along with the run's verdict, and + the exit code becomes a traceback's rather than the gate's. Escaping it costs the reader one + unreadable byte in one name. + """ + for stream in (sys.stdout, sys.stderr): + reconfigure = getattr(stream, "reconfigure", None) + if reconfigure is not None: + reconfigure(errors="backslashreplace") + + def main(argv: list[str] | None = None) -> int: + report_paths_that_are_not_utf8() ap = argparse.ArgumentParser() ap.add_argument("--root", default=".") ap.add_argument("--check", action="append", choices=sorted(CHECKS)) @@ -410,10 +479,10 @@ def main(argv: list[str] | None = None) -> int: status = "FAIL" if hits else "ok" print(f"[{status:4}] {name:12} {len(hits)} issue(s)") for h in hits: - print(f" {h}") + print(f" {printable(h)}") # After the findings and outside the count, since a note is not one. for note in NOTES: - print(f" note: {note}") + print(f" note: {printable(note)}") total += len(hits) return 1 if total else 0 diff --git a/.github/skills/backlog-burndown/SKILL.md b/.github/skills/backlog-burndown/SKILL.md index bb4c6119..91317aa8 100644 --- a/.github/skills/backlog-burndown/SKILL.md +++ b/.github/skills/backlog-burndown/SKILL.md @@ -114,7 +114,9 @@ handoff chain `AGENTS.md` "Session Scope" defines, so it records work to do next its own, and an open one is present by design for as long as that chain is in use. Counting it inflates the number this run reports as the backlog by one for every lane in use, and a backlog count this fleet reports wrong is a failure with its own history, so filter the label out of the -ranking and out of every count of the open backlog rather than out of the ranking alone. +ranking and out of every count of the open backlog rather than out of the ranking alone. A link +carrying `blocked` as well, as a parked lane does, is still a link, so this rule controls over the +`blocked` rule below and the link is neither ranked nor counted. An issue carrying the `blocked` label is counted and is not ranked while its blocker stands. It records real work this repository owes, which is why it stays in the count, and the label says the diff --git a/.github/skills/dotnet-codestyle/SKILL.md b/.github/skills/dotnet-codestyle/SKILL.md index 7eb20897..b13de18d 100644 --- a/.github/skills/dotnet-codestyle/SKILL.md +++ b/.github/skills/dotnet-codestyle/SKILL.md @@ -214,7 +214,7 @@ parameterized tests. A test project on `xunit.v3` 4.0.0 or later is MTP-based, a ## Project configuration -.NET 10.0 target, AOT-compatible (`IsAotCompatible=true`, `VerifyReferenceAotCompatibility=true`), +.NET 10.0 target, AOT-compatible (`IsAotCompatible=true`, with `VerifyReferenceAotCompatibility=true` only under the `PublishAot` condition), SourceLink, embedded untracked sources, `InternalsVisibleTo` for test/benchmark access. See `references/project-config.md` for the full property list. diff --git a/.github/skills/dotnet-codestyle/references/project-config.md b/.github/skills/dotnet-codestyle/references/project-config.md index 42b8fd1c..658e589c 100644 --- a/.github/skills/dotnet-codestyle/references/project-config.md +++ b/.github/skills/dotnet-codestyle/references/project-config.md @@ -1,8 +1,14 @@ # .NET Project Configuration 1. **Target framework**: .NET 10.0 (`net10.0`). -2. **AOT compatibility**: `true`, - `true`. +2. **AOT compatibility**: `true` unconditionally, and + `true` only in a + ``, placed in the `.csproj` after it sets + `PublishAot` or in `Directory.Build.targets`, never in `Directory.Build.props`, which is imported + before the project body and so never sees a `PublishAot` the `.csproj` sets. Reference + verification reports `IL3058` for every referenced assembly that lacks `IsAotCompatible` metadata + set to `true`, and `TreatWarningsAsErrors` turns that into a failed build on any such + dependency, so it runs only where an AOT publish needs it. 3. **Assembly information**: use semantic versioning, include SourceLink (`true`), embed untracked sources (`true`). diff --git a/.github/skills/session-handoff/SKILL.md b/.github/skills/session-handoff/SKILL.md index d0504cbc..d5eda1ae 100644 --- a/.github/skills/session-handoff/SKILL.md +++ b/.github/skills/session-handoff/SKILL.md @@ -143,8 +143,9 @@ section is the pointer to what to read rather than the answer, and `AGENTS.md` " already says stale context is worse than absent. A handoff is context by construction, so this is that rule applied to the one artifact built to outlive the session that wrote it. -Read the current link's comments too, with `gh issue view "" --comments`, since `resume` prints -only the body, and a parking comment or a closing session's answers land in the comments. +Read the current link's comments too, with `gh issue view "" --repo OWNER/NAME --comments`, since +`resume` prints only the body, and a parking comment or a closing session's answers land in the +comments. Read the chain before re-attempting anything. `resume` prints the current body and indexes the closed links behind it, and `chain --grep` searches the bodies it walks for a regular expression, @@ -161,7 +162,9 @@ rather than `resume`. Re-derive a count rather than copying one, and read it with an explicit page size. `gh issue list` returns 30 rows unless told otherwise, and a truncated count reads exactly like a repository with -30 issues, which is worse than an absent count because it gets stated. +30 issues, which is worse than an absent count because it gets stated. The same holds at any page +size, so where a read returns as many rows as its limit, raise the limit and read again until it +returns fewer. ## The Attended Session @@ -169,15 +172,17 @@ A maintainer resuming work says little, often only "resume the handoff", and tha the whole procedure below. Run every step without being reminded of any of them. 1. **Pick the link.** Where the maintainer names an issue, take it. Where none is named, read - `gh issue list --label handoff --state open --limit 100 --json number,title,labels,updatedAt`, - since `tracks` prints neither labels nor exact update times. Take a link carrying `blocked` - first, newest update first among them, since its blocker is a decision only the maintainer can - make and the maintainer is now present. Otherwise take the newest update among the rest. An - `auto-*` link not carrying `blocked` is skipped unless named, since an `unattended-handoff` - worker may hold it right now, and where one is named, confirm with the maintainer that no - unattended run is live before working it. Where every open link is skipped, say so and ask the - maintainer which to take. Say which link was picked in one line before anything else, so a wrong pick costs one reply - rather than a round. + `gh issue list --repo OWNER/NAME --label handoff --state open --limit 100 --json + number,title,labels,updatedAt`, since `tracks` prints neither labels nor exact update times. + Where it returns as many rows as the limit, the list may be truncated, so raise the limit and + read again until it returns fewer, rather than picking from a partial list. Take a link carrying + `blocked` first, newest update first among them, since its blocker is a decision only the + maintainer can make and the maintainer is now present. Otherwise take the newest update among the + rest. An `auto-*` link not carrying `blocked` is skipped unless named, since an + `unattended-handoff` worker may hold it right now, and where one is named, confirm with the + maintainer that no unattended run is live before working it. Where every open link is skipped, + say so and ask the maintainer which to take. Say which link was picked in one line before + anything else, so a wrong pick costs one reply rather than a round. 2. **Resume it** per "Resuming" above, comments included, re-deriving live state rather than trusting the body. 3. **Ask what it is blocked on first.** Where the link carries `blocked`, the parking comment names diff --git a/.github/skills/unattended-handoff/SKILL.md b/.github/skills/unattended-handoff/SKILL.md index b6bf958c..7fd62266 100644 --- a/.github/skills/unattended-handoff/SKILL.md +++ b/.github/skills/unattended-handoff/SKILL.md @@ -150,23 +150,24 @@ does not qualify, since skipping one costs nothing and a guess costs a revert an lacks. - **It reverses no settled decision** recorded in an issue, a handoff, or the rule text. - **Nothing has worked it or is working it.** The track `auto-` has no link, open or closed, - which `handoff.py chain --track "auto-" --limit 1` answers with its refusal naming no - handoff on that track. Any other refusal from it is a `STOP` rather than a yes. No open pull - request names it, and no pull request whose squash commit is in `origin/main..origin/develop` - names it anywhere in its body, since a fix merged to develop leaves its issue open until it is - promoted, whoever merged it. No open handoff on any track names it in its next steps, and no - comment on it claims it for a `backlog-burndown` group, since both mark work that has no pull - request yet. + which `handoff.py chain --repo "/" --track "auto-" --limit 1` answers with its + refusal naming no handoff on that track. Any other refusal from it is a `STOP` rather than a yes. + No open pull request names it, and no pull request whose squash commit is in + `origin/main..origin/develop` names it anywhere in its body, since a fix merged to develop leaves + its issue open until it is promoted, whoever merged it. No open handoff on any track names it in + its next steps, and no comment on it claims it for a `backlog-burndown` group, since both mark + work that has no pull request yet. ## The Picker 1. **Check the promotion first** under `main` or `release`. Where an open `decision` issue names the open develop -> main pull request, return `STOP` before picking anything, since every worker this run dispatched would meet that same decision after merging its own work to develop. -2. **Read the open handoffs** with labels and update times, `gh issue list --label handoff --state - open --limit 100 --json number,title,labels,updatedAt`, since `handoff.py tracks` prints - neither. Reach `scripts/handoff.py` from a hub checkout, per `session-handoff` "Running the - Chain". +2. **Read the open handoffs** with labels and update times, `gh issue list --repo "/" + --label handoff --state open --limit 100 --json number,title,labels,updatedAt`, since `handoff.py + tracks` prints neither. Where it returns as many rows as the limit, the list may be truncated, so + raise the limit and read again until it returns fewer, rather than ranking a partial list. Reach + `scripts/handoff.py` from a hub checkout, per `session-handoff` "Running the Chain". 3. **Prefer an open `auto-*` handoff not carrying `blocked`**, oldest first. That is a lane an earlier run parked and the maintainer has since unblocked, or one whose worker died, and a live link is work already framed. Handoffs on any other track belong to the maintainer's attended @@ -175,23 +176,27 @@ does not qualify, since skipping one costs nothing and a guess costs a revert an the session handing the lane back removes the label, per `GOVERNANCE.md` "Durable Knowledge and Self-Improvement". 4. **Otherwise pick from the backlog.** Rank the open issues by `backlog-burndown`'s "Ranking" - criteria, keep the auto-resolvable ones, and take the top one. Read the list with an explicit - page size, since `gh issue list` returns 30 rows unless told otherwise. -5. **Create its handoff** with `handoff.py new --track "auto-"`, `--dry-run` first. The body - carries the sections `session-handoff` "What Goes in the Body" names, with the next steps naming - the issue and what done looks like. That skill's rules on the body bind it. + criteria, keep the auto-resolvable ones, and take the top one. Read the list with `gh issue list + --repo "/" --state open` and an explicit `--limit`, since it returns 30 rows unless + told otherwise, and apply step 2's full-page check to it. +5. **Create its handoff** with `handoff.py new --repo "/" --track "auto-"`, + adding the `--title` and `--body-file` it also requires, as `session-handoff` "Running the Chain" + shows, and `--dry-run` first. The body carries the sections `session-handoff` "What Goes in the + Body" names, with the next steps naming the issue and what done looks like. That skill's rules on + the body bind it. 6. **Choose the worker's tier** by `backlog-burndown`'s "Choosing the Worker's Model Tier". 7. **Reply with one line.** A picker writes nothing but the handoff it creates, and returns `STOP` where a read it needs cannot run. ## The Worker -1. **Resume the handoff** with `handoff.py resume --track ""`, then read its comments with - `gh issue view "" --comments`, since `resume` prints only the body and a parked lane's state - is in its parking comment. A lane handed back by an attended session has a closed predecessor - holding that comment, so read the predecessor's comments too. Where either names a decision - issue, read the answer recorded there and follow it, since it is what unblocked the lane. - Re-derive live state rather than trusting any of them, per `session-handoff` "Resuming". +1. **Resume the handoff** with `handoff.py resume --repo "/" --track ""`, then + read its comments with `gh issue view "" --repo "/" --comments`, since `resume` + prints only the body and a parked lane's state is in its parking comment. A lane handed back by + an attended session has a closed predecessor holding that comment, so read the predecessor's + comments too. Where either names a decision issue, read the answer recorded there and follow it, + since it is what unblocked the lane. Re-derive live state rather than trusting any of them, per + `session-handoff` "Resuming". 2. **Isolate** in a worktree of its own, per `repo-worktree`, on the branch the handoff names or on `feature/`. 3. **Fix and drive.** Run `local-strict-review` before every push, and drive the pull request with diff --git a/.github/skills/workflow-ci-contract/references/d-guarantees.md b/.github/skills/workflow-ci-contract/references/d-guarantees.md index 1a396623..e293915b 100644 --- a/.github/skills/workflow-ci-contract/references/d-guarantees.md +++ b/.github/skills/workflow-ci-contract/references/d-guarantees.md @@ -11,11 +11,11 @@ The required behaviors, organized by domain. Each is a **MUST**, and its `Output ### D1 - PR Fast-Feedback (Smoke) - **D1.1 Only changed targets build.** Input: a PR touching some targets. Output: the paths-filter marks exactly those targets and only their smoke builds run. Unchanged targets skip. A repo's own targets MUST each have a filter entry (so a touched target is never silently skipped), and that entry lists paths rather than negating them, so a change matching no entry marks nothing and every smoke build skips. *Prevents: rebuilding everything, and a changed target slipping through unbuilt.* -- **D1.2 A validation job always runs.** Input: any PR. Output: a validation job runs unconditionally and the aggregator `needs:` it. That job is the caller's own job reaching the reusable validator, named `validate` in every shipped stub, and that name is what the aggregator's `needs:` carries. The validator's internal jobs (`lint`, `unit-test` and `validate` in the hub's `validate-task.yml`) are not addressable from a caller, so a `validate` in a caller's `needs:` list always names the caller's own job rather than the validator's internal one of the same name. The validator detects the tree rather than the repo's language, running the doc and repo gates everywhere, the `dotnet test` path only where a `*Tests*.csproj` project exists, and the `pytest` path only where a root `tests/` directory sits beside a root `pyproject.toml` and a root `uv.lock` or `requirements*.txt`, so a non-.NET repo calls the same one rather than replacing it. A repo whose tests take another shape can run them from its own `.github/actions/validate/action.yml` hook, which the validator runs where that file exists. That hook receives no secret, so a repo whose other-shape tests owe D1.6's coverage upload is one whose validation the validator cannot express. A repo whose validation it cannot express **replaces** the call (not deletes it) with its own validator and re-points the aggregator's `needs:` to the replacement. `smoke-build` `needs:` the `changes` job rather than the validation job, so no second `needs:` moves with it. *Prevents: a PR merging with no validation, or a dangling `needs:` that stops the whole workflow from loading.* +- **D1.2 A validation job always runs.** Input: any PR. Output: a validation job runs unconditionally and the aggregator `needs:` it. That job is the caller's own job reaching the reusable validator, named `validate` in every shipped stub, and that name is what the aggregator's `needs:` carries. The validator's internal jobs (`lint`, `unit-test` and `validate` in the hub's `validate-task.yml`) are not addressable from a caller, so a `validate` in a caller's `needs:` list always names the caller's own job rather than the validator's internal one of the same name. The validator detects the tree rather than the repo's language, running the doc and repo gates everywhere, the `dotnet test` path only where a `*Tests*.csproj` project exists, and the `pytest` path only where a root `tests/` directory sits beside a root `pyproject.toml` and a root `uv.lock` or `requirements*.txt`, so a non-.NET repo calls the same one rather than replacing it. It checks out the triggering commit, `github.sha`, with no `ref` input, because a reusable workflow reads its caller's `github` context, so a publisher's call validates the same commit its publish job pins. A repo whose tests take another shape can run them from its own `.github/actions/validate/action.yml` hook, which the validator runs where that file exists. That hook receives no secret, so a repo whose other-shape tests owe D1.6's coverage upload is one whose validation the validator cannot express. A repo whose validation it cannot express **replaces** the call (not deletes it) with its own validator and re-points the aggregator's `needs:` to the replacement. `smoke-build` `needs:` the `changes` job rather than the validation job, so no second `needs:` moves with it. *Prevents: a PR merging with no validation, or a dangling `needs:` that stops the whole workflow from loading.* - **D1.3 Smoke never publishes and never uploads.** Input: `smoke: true`. Output: full compile/lint/test, but no registry/image push, no release, and **no** artifact uploads (every `upload-artifact`, including any aggregation job, is gated on smoke being false, written `!inputs.smoke` at the workflow layer and `inputs.smoke != 'true'` in a composite action, whose inputs are strings). *Prevents: a PR publishing, and orphaned artifacts churning the storage quota.* - **D1.4 Workflow-file changes are not smoke-built.** Input: a PR changing only `.github/workflows/**`. Output: the paths-filter marks no target, so smoke-build skips. An inclusion list satisfying D1.1 reaches this by leaving workflow paths out of every target's entry. *Implication: a workflow-only change is not smoke-built, but actionlint still validates it in CI.* - **D1.5 One required aggregator gates merge.** Input: any PR. Output: a single aggregator job must **succeed**, run under `if: always()` so a failed or skipped dependency cannot skip the gate itself, `needs:` the validation job, and the `changes` and `smoke-build` jobs too wherever the repo has a smoke build, treat a **skipped** smoke build as pass, and **block** on `failure`/`cancelled`. Its name is ruleset-bound: the job `name:` and the ruleset `context:` are the same string and MUST be renamed together, never independently. *Prevents: a paths-filter error letting a target-changing PR merge unbuilt.* -- **D1.6 Coverage is reported to Codecov (C# and Python).** Input: a C# or Python repo that has tests for that type. Output: the validation job runs those tests under coverage collection (`dotnet test --coverage --coverage-output-format cobertura --results-directory ./coverage`, leaving `--coverage-output` unset so each test project writes its own report rather than overwriting a shared one, or `pytest --cov-report=xml` over a repo whose own `pyproject.toml` selects what to measure) and a `codecov/codecov-action` step uploads the report, **best-effort** (`continue-on-error` and/or `fail_ci_if_error: false`, so a Codecov outage or an absent token never reds the gate). The Python leg **fails its test step when no report was written**, since `--cov-report=xml` alone selects nothing to measure. The C# leg renames each report to `coverage-.cobertura.xml` before the upload step reads the directory, `codecov-cli`'s own finder not matching the default name, and a repo owning its validator rather than calling the hub's owes that rename itself. `CODECOV_TOKEN` lives in the repo's **actions** and **dependabot** secret stores, the second because a run triggered by a Dependabot pull request reads the Dependabot store and the upload would otherwise skip silently on every bot pull request. A caller reaching the reusable validator across repositories names the secret it passes (`secrets:` with `CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}`), on its pull request path and its publisher path alike, because `secrets: inherit` is documented for a caller in the same organization or enterprise, which a personal account is not. A call by local path stays inside one repository, where the caller's own store is the one the callee reads, so `secrets: inherit` is available there instead of naming each secret. The repo ships a **`codecov.yml`** setting the project and patch statuses to **`informational: true`** so a coverage delta never gates a pull request (a repo whose quality bar requires a threshold may turn that off), and excluding intentionally-untested, non-shipped code (an example or benchmark project) from the denominator via `ignore`. Coverage output is a build artifact, so `.gitignore` excludes it. The C# invocation runs under **Microsoft.Testing.Platform**, and the runner declaration, package references, and version floor an MTP-based test project needs are `CODESTYLE.md`'s .NET side. The Python invocation needs **`pytest-cov`** and a coverage selector, which are `CODESTYLE.md`'s Python side. N/A for a repo carrying no tests for that type, and for a `lint-only` profile for it (per the hub's `registry/repos.json`). *Prevents: coverage silently going unreported, and a coverage regression blocking an unrelated pull request.* +- **D1.6 Coverage is reported to Codecov (C# and Python).** Input: a C# or Python repo that has tests for that type. Output: the validation job runs those tests under coverage collection (`dotnet test --coverage --coverage-output-format cobertura --results-directory ./coverage`, leaving `--coverage-output` unset so each test project writes its own report rather than overwriting a shared one, or `pytest --cov-report=xml` over a repo whose own `pyproject.toml` selects what to measure) and a `codecov/codecov-action` step uploads the report, **best-effort** (`continue-on-error` and/or `fail_ci_if_error: false`, so a Codecov outage or an absent token never reds the gate). Each leg **fails its test step when the run wrote no report**, since the best-effort upload reads a missing report exactly as it reads a healthy run, and on the Python leg `--cov-report=xml` alone selects nothing to measure. The C# leg renames each report to `coverage-.cobertura.xml` before the upload step reads the directory, `codecov-cli`'s own finder not matching the default name, and a repo owning its validator rather than calling the hub's owes that rename itself. `CODECOV_TOKEN` lives in the repo's **actions** and **dependabot** secret stores, the second because a run triggered by a Dependabot pull request reads the Dependabot store and the upload would otherwise skip silently on every bot pull request. A caller reaching the reusable validator across repositories names the secret it passes (`secrets:` with `CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}`), on its pull request path and its publisher path alike, because `secrets: inherit` is documented for a caller in the same organization or enterprise, which a personal account is not. A call by local path stays inside one repository, where the caller's own store is the one the callee reads, so `secrets: inherit` is available there instead of naming each secret. The repo ships a **`codecov.yml`** setting the project and patch statuses to **`informational: true`** so a coverage delta never gates a pull request (a repo whose quality bar requires a threshold may turn that off), and excluding intentionally-untested, non-shipped code (an example or benchmark project) from the denominator via `ignore`. Coverage output is a build artifact, so `.gitignore` excludes it. The C# invocation runs under **Microsoft.Testing.Platform**, and the runner declaration, package references, and version floor an MTP-based test project needs are `CODESTYLE.md`'s .NET side. The Python invocation needs **`pytest-cov`** and a coverage selector, which are `CODESTYLE.md`'s Python side. N/A for a repo carrying no tests for that type, and for a `lint-only` profile for it (per the hub's `registry/repos.json`). *Prevents: coverage silently going unreported, and a coverage regression blocking an unrelated pull request.* ### D2 - Input/State Validation at Entry @@ -76,7 +76,7 @@ The required behaviors, organized by domain. Each is a **MUST**, and its `Output `GOVERNANCE.md` "Workflow YAML Conventions" names the tool D9.1 excepts and states the suffix rules D9.2 requires. -- **D9.1** Every action or reusable workflow referenced from another repository is SHA-pinned with a version comment (sole exception: the documented lagging-tag tool). A local (`./`) or self-repository (`$/`) reference names no ref and takes no pin. +- **D9.1** Every action or reusable workflow referenced from another repository is SHA-pinned with a comment naming the release tag at that SHA, spelled as the referenced repository publishes it (sole exception: the documented lagging-tag tool). A local (`./`) or self-repository (`$/`) reference names no ref and takes no pin, `$/` being GitHub's syntax, in a workflow file, for a path in the repository holding that file, resolved at that file's own commit. - **D9.2** File/workflow/job/step names follow the suffix rules. A ruleset-bound job's `name:` equals its ruleset `context:` (renamed together). - **D9.3** Multi-line bash `run:` blocks start `set -Eeuo pipefail`. Multi-line `if:` uses `>-`. - **D9.4** Docker layer cache targets a registry tag, not `type=gha`. `cache-to` writes only the built branch's `:buildcache-` and only on push, while `cache-from` reads both branches. A multi-image repo varies the cache **repository** rather than the tag, `:buildcache-` per image, the tag alone being unable to distinguish two images. diff --git a/.github/skills/workflow-ci-contract/references/test-methodology.md b/.github/skills/workflow-ci-contract/references/test-methodology.md index f7690a0a..36f359f2 100644 --- a/.github/skills/workflow-ci-contract/references/test-methodology.md +++ b/.github/skills/workflow-ci-contract/references/test-methodology.md @@ -18,7 +18,7 @@ Cite what each verdict rests on. That is `file:line` for a file in the audited r ### 5B. End-to-End Trace Scenarios (No Execution, Deterministic from the YAML) -For each *applicable* scenario, evaluate every job's `if:`/`needs:` against the inputs and emit the predicted **run/skip + version + release + artifact-end-state** table, then compare to the expected. A scenario governing a construct the repo does not contain is N/A, per `WORKFLOW.md` section 1, and an absent trigger is such a construct. Each scenario's trigger belongs to one workflow, so read that workflow's own `on:` block rather than the repo's type: S1 to S4 the pull request workflow's, S5 to S10 the publisher's, S11 the upstream tracker's, and S12 and S13 the deploy workflow's. A publisher carrying only `workflow_dispatch` therefore records S5, S6 and S9 N/A, their push and schedule paths never firing there, and a repo with no publisher at all records S5 to S10 N/A together. Where a scenario's path runs through a workflow or composite action the repo only **calls**, trace that callee as the repo reaches it, read at the SHA the caller pins rather than at the callee's current default branch, which is the same evidence rule 5A states. Predicting from the callee's `main` predicts a table for YAML the audited repo never runs. A local (`./`) or self-repository (`$/`) call carries no pin of its own and runs at the workflow commit, so it is traced at whatever SHA the outermost pinning caller fixed. Minimum set: +For each *applicable* scenario, evaluate every job's `if:`/`needs:` against the inputs and emit the predicted **run/skip + version + release + artifact-end-state** table, then compare to the expected. A scenario governing a construct the repo does not contain is N/A, per `WORKFLOW.md` section 1, and an absent trigger is such a construct. Each scenario's trigger belongs to one workflow, so read that workflow's own `on:` block rather than the repo's type: S1 to S4 the pull request workflow's, S5 to S10 the publisher's, S11 the upstream tracker's, and S12 and S13 the deploy workflow's. A publisher carrying only `workflow_dispatch` therefore records S5, S6 and S9 N/A, their push and schedule paths never firing there, and a repo with no publisher at all records S5 to S10 N/A together. Where a scenario's path runs through a workflow or composite action the repo only **calls**, trace that callee as the repo reaches it, read at the SHA the caller pins rather than at the callee's current default branch, which is the same evidence rule 5A states. Predicting from the callee's `main` predicts a table for YAML the audited repo never runs. A self-repository (`$/`) call or a job-level local (`./`) reusable-workflow call carries no pin of its own and resolves at the calling workflow file's commit, so it is traced at that file's own commit, which is the SHA fixed by the pin that reached that file, or the audited commit where that file is one of the audited repository's own workflows. A local (`./`) action reference carries no pin either, but it resolves against whatever the job checked out, so it is traced at that checkout's commit, which is the caller's where a reusable workflow called from another repository checks out its caller. Minimum set: | # | Input | Expected output | Exercises | | --- | --- | --- | --- | diff --git a/.github/workflows/validate-task.yml b/.github/workflows/validate-task.yml index 02ecc411..a6078ac8 100644 --- a/.github/workflows/validate-task.yml +++ b/.github/workflows/validate-task.yml @@ -338,11 +338,21 @@ jobs: if: hashFiles('**/*Tests*.csproj') != '' run: | set -Eeuo pipefail + rm -rf ./coverage dotnet test --coverage --coverage-output-format cobertura --results-directory ./coverage + written=0 for report in ./coverage/*.cobertura.xml; do [ -e "$report" ] || continue - mv "$report" "./coverage/coverage-$(basename "$report")" + renamed="./coverage/coverage-$(basename "$report")" + mv "$report" "$renamed" + if [ -s "$renamed" ]; then + written=1 + fi done + if [ "$written" -eq 0 ]; then + echo "::error::This run wrote no non-empty Cobertura report under ./coverage. Reference Microsoft.Testing.Extensions.CodeCoverage from each test project, per CODESTYLE.md's .NET side." + exit 1 + fi # Report-only: fail_ci_if_error is false so a Codecov hiccup or an absent token never fails the gate. - name: Upload coverage to Codecov step diff --git a/GOVERNANCE.md b/GOVERNANCE.md index f6d43804..c1d34443 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -234,7 +234,7 @@ These conventions bind every workflow. Several of them [`WORKFLOW.md`](./WORKFLO An overlap with `WORKFLOW.md` resolves **by subject**, never by blanket precedence. This section keeps the full style rules and wins on them, stating each in more detail than the guarantee that carries it, while `WORKFLOW.md` wins on the architecture, the contract, and the test methodology. `WORKFLOW.md` section 2 points at this section rather than restating it. Throughout, a job is named by its id and a step by its `name:`. The `workflow-ci-contract` Skill at `.agents/skills/workflow-ci-contract/SKILL.md` in the hub, not a repo-relative link since that path is hub-local and not carried into every fleet repo, surfaces it. -- **Action pinning**: pin **every** action, first-party (`actions/*`) and third-party alike, to a commit SHA with a trailing `# vX.Y.Z` comment, so Renovate / Dependabot can still bump it but a tag swap can't change the executed code. This binds a `uses:` wherever it appears, in a workflow and in a composite action under `.github/actions/**` alike, except a local (`./`) or self-repository (`$/`) reference, which names no ref to pin. Use `# vX` (major-only) only when the upstream's floating major tag doesn't correspond to a specific patch/minor release SHA, since pinning to the floating-tag SHA still gives the SHA guarantee, the version comment just records the major line. Documented exception (no SHA pin at all): `dotnet/nbgv` is consumed via `@master` because the upstream tag stream lags `master` substantially and Dependabot's tag-tracking would propose a downgrade. **This applies to repo-owned build-layer leaves too**, since a leaf owning its build specifics is not a reason to use floating tags, and Dependabot still bumps SHA pins (updating the SHA + version comment). +- **Action pinning**: pin **every** action, first-party (`actions/*`) and third-party alike, to a commit SHA with a trailing comment naming the release tag at that SHA, so Renovate / Dependabot can still bump it but a tag swap can't change the executed code. The comment spells the tag exactly as the referenced repository publishes it, `# vX.Y.Z` where its tags carry a `v` and the bare tag, such as `# 1.4.2`, where they do not, so the comment never names a tag that does not exist. This binds a `uses:` wherever it appears, in a workflow and in a composite action under `.github/actions/**` alike, except a local (`./`) reference or a self-repository (`$/`) one, which names no ref to pin. In a workflow file, `$/` is GitHub's syntax for a path in the repository holding that file, resolved at that file's own commit. Use a major-only comment, `# vX` or the bare `# X` by the same spelling rule, only when the upstream's floating major tag doesn't correspond to a specific patch/minor release SHA, since pinning to the floating-tag SHA still gives the SHA guarantee, the version comment just records the major line. Documented exception (no SHA pin at all): `dotnet/nbgv` is consumed via `@master` because the upstream tag stream lags `master` substantially and Dependabot's tag-tracking would propose a downgrade. **This applies to repo-owned build-layer leaves too**, since a leaf owning its build specifics is not a reason to use floating tags, and Dependabot still bumps SHA pins (updating the SHA + version comment). - **Filename**: a workflow declaring `on: workflow_call` ends in `-task.yml`, **whatever else it is also triggered by**, since that is the half the suffix is about. A workflow without `workflow_call` is an entry point (`push`, `pull_request`, `pull_request_target`, `schedule`, `workflow_dispatch`) and takes no `-task` suffix, ending instead with what it does: `-pull-request.yml`, `-release.yml`. The suffix says the file is meant to be `uses:`-d, which stays true of a file that is also dispatchable. Composite actions are named by their path (`.github/actions//action.yml`), so these suffix rules do not reach them. - **Workflow `name:`** (the top-level `name:` field): a workflow declaring `workflow_call` takes a name ending in **"task"** (e.g. `Build project release task`), matching the filename rule above and covering a file that is also dispatchable, and every other workflow takes one ending in **"action"** (e.g. `Publish project release action`, `Test pull request action`). The suffix tells an orchestrator from a callee while reading the source tree, and on the runs list for an entry point. It does not do that in the Actions UI for a callee: a called reusable workflow's jobs appear nested inside the caller's run as ` / `, and the runs list shows the caller's workflow name rather than the callee's own. - **Job and step `name:` suffixes**: every job's `name:` ends in **"job"** and every step's `name:` ends in **"step"**, including the PR-gate aggregator, whose `name:` is a required-status-check `context:` in a branch ruleset (`Check pull request workflow status job` in `test-pull-request.yml`). A trailing parenthetical qualifier after the suffix is allowed and is the only exception (`Upload coverage to Codecov step (Python)`), and nothing enforces the rule mechanically. A ruleset-bound job's `name:` and its ruleset `context:` are the **same string**: rename them **together**, or required-status-check enforcement silently breaks. Every surface whose staleness breaks that enforcement moves in the same change, never one without the others. In a repository the surfaces are the live ruleset and its own workflow. A rename of the fleet-wide string additionally moves the hub's `repo-config/` payloads, its `spec/files.json` `requiredCheckName`, and each adopter-facing stub in its `catalog/` and `docs/reusable-workflows.md`, which exist only in the hub. Prose naming the old string goes stale rather than breaking, and follows behind. diff --git a/WORKFLOW.md b/WORKFLOW.md index d4c2ff98..0fb02dcf 100644 --- a/WORKFLOW.md +++ b/WORKFLOW.md @@ -139,11 +139,11 @@ The required behaviors, organized by domain. Each is a **MUST**, and its `Output ### D1 - PR Fast-Feedback (Smoke) - **D1.1 Only changed targets build.** Input: a PR touching some targets. Output: the paths-filter marks exactly those targets and only their smoke builds run. Unchanged targets skip. A repo's own targets MUST each have a filter entry (so a touched target is never silently skipped), and that entry lists paths rather than negating them, so a change matching no entry marks nothing and every smoke build skips. *Prevents: rebuilding everything, and a changed target slipping through unbuilt.* -- **D1.2 A validation job always runs.** Input: any PR. Output: a validation job runs unconditionally and the aggregator `needs:` it. That job is the caller's own job reaching the reusable validator, named `validate` in every shipped stub, and that name is what the aggregator's `needs:` carries. The validator's internal jobs (`lint`, `unit-test` and `validate` in the hub's `validate-task.yml`) are not addressable from a caller, so a `validate` in a caller's `needs:` list always names the caller's own job rather than the validator's internal one of the same name. The validator detects the tree rather than the repo's language, running the doc and repo gates everywhere, the `dotnet test` path only where a `*Tests*.csproj` project exists, and the `pytest` path only where a root `tests/` directory sits beside a root `pyproject.toml` and a root `uv.lock` or `requirements*.txt`, so a non-.NET repo calls the same one rather than replacing it. A repo whose tests take another shape can run them from its own `.github/actions/validate/action.yml` hook, which the validator runs where that file exists. That hook receives no secret, so a repo whose other-shape tests owe D1.6's coverage upload is one whose validation the validator cannot express. A repo whose validation it cannot express **replaces** the call (not deletes it) with its own validator and re-points the aggregator's `needs:` to the replacement. `smoke-build` `needs:` the `changes` job rather than the validation job, so no second `needs:` moves with it. *Prevents: a PR merging with no validation, or a dangling `needs:` that stops the whole workflow from loading.* +- **D1.2 A validation job always runs.** Input: any PR. Output: a validation job runs unconditionally and the aggregator `needs:` it. That job is the caller's own job reaching the reusable validator, named `validate` in every shipped stub, and that name is what the aggregator's `needs:` carries. The validator's internal jobs (`lint`, `unit-test` and `validate` in the hub's `validate-task.yml`) are not addressable from a caller, so a `validate` in a caller's `needs:` list always names the caller's own job rather than the validator's internal one of the same name. The validator detects the tree rather than the repo's language, running the doc and repo gates everywhere, the `dotnet test` path only where a `*Tests*.csproj` project exists, and the `pytest` path only where a root `tests/` directory sits beside a root `pyproject.toml` and a root `uv.lock` or `requirements*.txt`, so a non-.NET repo calls the same one rather than replacing it. It checks out the triggering commit, `github.sha`, with no `ref` input, because a reusable workflow reads its caller's `github` context, so a publisher's call validates the same commit its publish job pins. A repo whose tests take another shape can run them from its own `.github/actions/validate/action.yml` hook, which the validator runs where that file exists. That hook receives no secret, so a repo whose other-shape tests owe D1.6's coverage upload is one whose validation the validator cannot express. A repo whose validation it cannot express **replaces** the call (not deletes it) with its own validator and re-points the aggregator's `needs:` to the replacement. `smoke-build` `needs:` the `changes` job rather than the validation job, so no second `needs:` moves with it. *Prevents: a PR merging with no validation, or a dangling `needs:` that stops the whole workflow from loading.* - **D1.3 Smoke never publishes and never uploads.** Input: `smoke: true`. Output: full compile/lint/test, but no registry/image push, no release, and **no** artifact uploads (every `upload-artifact`, including any aggregation job, is gated on smoke being false, written `!inputs.smoke` at the workflow layer and `inputs.smoke != 'true'` in a composite action, whose inputs are strings). *Prevents: a PR publishing, and orphaned artifacts churning the storage quota.* - **D1.4 Workflow-file changes are not smoke-built.** Input: a PR changing only `.github/workflows/**`. Output: the paths-filter marks no target, so smoke-build skips. An inclusion list satisfying D1.1 reaches this by leaving workflow paths out of every target's entry. *Implication: a workflow-only change is not smoke-built, but actionlint still validates it in CI.* - **D1.5 One required aggregator gates merge.** Input: any PR. Output: a single aggregator job must **succeed**, run under `if: always()` so a failed or skipped dependency cannot skip the gate itself, `needs:` the validation job, and the `changes` and `smoke-build` jobs too wherever the repo has a smoke build, treat a **skipped** smoke build as pass, and **block** on `failure`/`cancelled`. Its name is ruleset-bound: the job `name:` and the ruleset `context:` are the same string and MUST be renamed together, never independently. *Prevents: a paths-filter error letting a target-changing PR merge unbuilt.* -- **D1.6 Coverage is reported to Codecov (C# and Python).** Input: a C# or Python repo that has tests for that type. Output: the validation job runs those tests under coverage collection (`dotnet test --coverage --coverage-output-format cobertura --results-directory ./coverage`, leaving `--coverage-output` unset so each test project writes its own report rather than overwriting a shared one, or `pytest --cov-report=xml` over a repo whose own `pyproject.toml` selects what to measure) and a `codecov/codecov-action` step uploads the report, **best-effort** (`continue-on-error` and/or `fail_ci_if_error: false`, so a Codecov outage or an absent token never reds the gate). The Python leg **fails its test step when no report was written**, since `--cov-report=xml` alone selects nothing to measure. The C# leg renames each report to `coverage-.cobertura.xml` before the upload step reads the directory, `codecov-cli`'s own finder not matching the default name, and a repo owning its validator rather than calling the hub's owes that rename itself. `CODECOV_TOKEN` lives in the repo's **actions** and **dependabot** secret stores, the second because a run triggered by a Dependabot pull request reads the Dependabot store and the upload would otherwise skip silently on every bot pull request. A caller reaching the reusable validator across repositories names the secret it passes (`secrets:` with `CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}`), on its pull request path and its publisher path alike, because `secrets: inherit` is documented for a caller in the same organization or enterprise, which a personal account is not. A call by local path stays inside one repository, where the caller's own store is the one the callee reads, so `secrets: inherit` is available there instead of naming each secret. The repo ships a **`codecov.yml`** setting the project and patch statuses to **`informational: true`** so a coverage delta never gates a pull request (a repo whose quality bar requires a threshold may turn that off), and excluding intentionally-untested, non-shipped code (an example or benchmark project) from the denominator via `ignore`. Coverage output is a build artifact, so `.gitignore` excludes it. The C# invocation runs under **Microsoft.Testing.Platform**, and the runner declaration, package references, and version floor an MTP-based test project needs are `CODESTYLE.md`'s .NET side. The Python invocation needs **`pytest-cov`** and a coverage selector, which are `CODESTYLE.md`'s Python side. N/A for a repo carrying no tests for that type, and for a `lint-only` profile for it (per the hub's `registry/repos.json`). *Prevents: coverage silently going unreported, and a coverage regression blocking an unrelated pull request.* +- **D1.6 Coverage is reported to Codecov (C# and Python).** Input: a C# or Python repo that has tests for that type. Output: the validation job runs those tests under coverage collection (`dotnet test --coverage --coverage-output-format cobertura --results-directory ./coverage`, leaving `--coverage-output` unset so each test project writes its own report rather than overwriting a shared one, or `pytest --cov-report=xml` over a repo whose own `pyproject.toml` selects what to measure) and a `codecov/codecov-action` step uploads the report, **best-effort** (`continue-on-error` and/or `fail_ci_if_error: false`, so a Codecov outage or an absent token never reds the gate). Each leg **fails its test step when the run wrote no report**, since the best-effort upload reads a missing report exactly as it reads a healthy run, and on the Python leg `--cov-report=xml` alone selects nothing to measure. The C# leg renames each report to `coverage-.cobertura.xml` before the upload step reads the directory, `codecov-cli`'s own finder not matching the default name, and a repo owning its validator rather than calling the hub's owes that rename itself. `CODECOV_TOKEN` lives in the repo's **actions** and **dependabot** secret stores, the second because a run triggered by a Dependabot pull request reads the Dependabot store and the upload would otherwise skip silently on every bot pull request. A caller reaching the reusable validator across repositories names the secret it passes (`secrets:` with `CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}`), on its pull request path and its publisher path alike, because `secrets: inherit` is documented for a caller in the same organization or enterprise, which a personal account is not. A call by local path stays inside one repository, where the caller's own store is the one the callee reads, so `secrets: inherit` is available there instead of naming each secret. The repo ships a **`codecov.yml`** setting the project and patch statuses to **`informational: true`** so a coverage delta never gates a pull request (a repo whose quality bar requires a threshold may turn that off), and excluding intentionally-untested, non-shipped code (an example or benchmark project) from the denominator via `ignore`. Coverage output is a build artifact, so `.gitignore` excludes it. The C# invocation runs under **Microsoft.Testing.Platform**, and the runner declaration, package references, and version floor an MTP-based test project needs are `CODESTYLE.md`'s .NET side. The Python invocation needs **`pytest-cov`** and a coverage selector, which are `CODESTYLE.md`'s Python side. N/A for a repo carrying no tests for that type, and for a `lint-only` profile for it (per the hub's `registry/repos.json`). *Prevents: coverage silently going unreported, and a coverage regression blocking an unrelated pull request.* ### D2 - Input/State Validation at Entry @@ -204,7 +204,7 @@ The required behaviors, organized by domain. Each is a **MUST**, and its `Output `GOVERNANCE.md` "Workflow YAML Conventions" names the tool D9.1 excepts and states the suffix rules D9.2 requires. -- **D9.1** Every action or reusable workflow referenced from another repository is SHA-pinned with a version comment (sole exception: the documented lagging-tag tool). A local (`./`) or self-repository (`$/`) reference names no ref and takes no pin. +- **D9.1** Every action or reusable workflow referenced from another repository is SHA-pinned with a comment naming the release tag at that SHA, spelled as the referenced repository publishes it (sole exception: the documented lagging-tag tool). A local (`./`) or self-repository (`$/`) reference names no ref and takes no pin, `$/` being GitHub's syntax, in a workflow file, for a path in the repository holding that file, resolved at that file's own commit. - **D9.2** File/workflow/job/step names follow the suffix rules. A ruleset-bound job's `name:` equals its ruleset `context:` (renamed together). - **D9.3** Multi-line bash `run:` blocks start `set -Eeuo pipefail`. Multi-line `if:` uses `>-`. - **D9.4** Docker layer cache targets a registry tag, not `type=gha`. `cache-to` writes only the built branch's `:buildcache-` and only on push, while `cache-from` reads both branches. A multi-image repo varies the cache **repository** rather than the tag, `:buildcache-` per image, the tag alone being unable to distinguish two images. @@ -226,7 +226,7 @@ Cite what each verdict rests on. That is `file:line` for a file in the audited r ### 5B. End-to-End Trace Scenarios (No Execution, Deterministic from the YAML) -For each *applicable* scenario, evaluate every job's `if:`/`needs:` against the inputs and emit the predicted **run/skip + version + release + artifact-end-state** table, then compare to the expected. A scenario governing a construct the repo does not contain is N/A, per `WORKFLOW.md` section 1, and an absent trigger is such a construct. Each scenario's trigger belongs to one workflow, so read that workflow's own `on:` block rather than the repo's type: S1 to S4 the pull request workflow's, S5 to S10 the publisher's, S11 the upstream tracker's, and S12 and S13 the deploy workflow's. A publisher carrying only `workflow_dispatch` therefore records S5, S6 and S9 N/A, their push and schedule paths never firing there, and a repo with no publisher at all records S5 to S10 N/A together. Where a scenario's path runs through a workflow or composite action the repo only **calls**, trace that callee as the repo reaches it, read at the SHA the caller pins rather than at the callee's current default branch, which is the same evidence rule 5A states. Predicting from the callee's `main` predicts a table for YAML the audited repo never runs. A local (`./`) or self-repository (`$/`) call carries no pin of its own and runs at the workflow commit, so it is traced at whatever SHA the outermost pinning caller fixed. Minimum set: +For each *applicable* scenario, evaluate every job's `if:`/`needs:` against the inputs and emit the predicted **run/skip + version + release + artifact-end-state** table, then compare to the expected. A scenario governing a construct the repo does not contain is N/A, per `WORKFLOW.md` section 1, and an absent trigger is such a construct. Each scenario's trigger belongs to one workflow, so read that workflow's own `on:` block rather than the repo's type: S1 to S4 the pull request workflow's, S5 to S10 the publisher's, S11 the upstream tracker's, and S12 and S13 the deploy workflow's. A publisher carrying only `workflow_dispatch` therefore records S5, S6 and S9 N/A, their push and schedule paths never firing there, and a repo with no publisher at all records S5 to S10 N/A together. Where a scenario's path runs through a workflow or composite action the repo only **calls**, trace that callee as the repo reaches it, read at the SHA the caller pins rather than at the callee's current default branch, which is the same evidence rule 5A states. Predicting from the callee's `main` predicts a table for YAML the audited repo never runs. A self-repository (`$/`) call or a job-level local (`./`) reusable-workflow call carries no pin of its own and resolves at the calling workflow file's commit, so it is traced at that file's own commit, which is the SHA fixed by the pin that reached that file, or the audited commit where that file is one of the audited repository's own workflows. A local (`./`) action reference carries no pin either, but it resolves against whatever the job checked out, so it is traced at that checkout's commit, which is the caller's where a reusable workflow called from another repository checks out its caller. Minimum set: | # | Input | Expected output | Exercises | | --- | --- | --- | --- | diff --git a/host-setup/agent-safety/claude/install.py b/host-setup/agent-safety/claude/install.py index 74b3ea33..3f352be4 100755 --- a/host-setup/agent-safety/claude/install.py +++ b/host-setup/agent-safety/claude/install.py @@ -15,6 +15,7 @@ Usage: python3 install.py (installs to ~/.claude) python3 install.py --report (read-only: is this machine current?) CLAUDE_HOME=/x python3 install.py (override target, for testing) + AGENT_SAFETY_DIRTY_OVERRIDE=0/1 python3 install.py (force the dirty-checkout signal, for testing) """ import argparse @@ -214,6 +215,13 @@ def source_ref(): A dirty tree is reported rather than hidden: the SHA still names a commit, but the bytes installed are not that commit's, and a stamp that claims otherwise is the thing this exists to prevent. A checkout that is not a git tree at all (an extracted tarball) says so. + + AGENT_SAFETY_DIRTY_OVERRIDE, read as exactly "0" or "1", replaces the git-derived dirty + signal with a fixed one, and honoring it prints a warning below. Any other value, unset + included, falls through to the git status read below, so a stray or misspelled setting + cannot silently force a verdict. This exists so a test can assert a verdict without + depending on whether this very kit happens to be mid-edit while the suite runs. It never + changes what a real, unset-env install reports. """ def git(*args): @@ -238,8 +246,17 @@ def git(*args): branch = git("rev-parse", "--abbrev-ref", "HEAD") if branch and branch != "HEAD": ref["branch"] = branch - status = git("status", "--porcelain", "--", *PAYLOAD_FILES) - ref["dirty"] = bool(status) + override = os.environ.get("AGENT_SAFETY_DIRTY_OVERRIDE") + if override in ("0", "1"): + ref["dirty"] = override == "1" + print( + f"AGENT_SAFETY_DIRTY_OVERRIDE={override} is forcing the dirty-checkout signal. " + "The real git status was not read.", + file=sys.stderr, + ) + else: + status = git("status", "--porcelain", "--", *PAYLOAD_FILES) + ref["dirty"] = bool(status) return ref diff --git a/host-setup/agent-safety/claude/test_install.py b/host-setup/agent-safety/claude/test_install.py index bbccc1c1..76f3d22b 100755 --- a/host-setup/agent-safety/claude/test_install.py +++ b/host-setup/agent-safety/claude/test_install.py @@ -24,9 +24,15 @@ import install -def run(home, *args): - """Invoke the installer as a subprocess, the way a host actually runs it.""" - env = dict(os.environ, CLAUDE_HOME=str(home)) +def run(home, *args, dirty=False): + """Invoke the installer as a subprocess, the way a host actually runs it. + + dirty forces the dirty-checkout signal install.py's own source_ref() would otherwise read + live from this checkout, via AGENT_SAFETY_DIRTY_OVERRIDE, so a verdict this suite asserts + depends on the fixture rather than on whether host-setup happens to be mid-edit while the + suite runs. The default is clean, since that is what every case but one below needs. + """ + env = dict(os.environ, CLAUDE_HOME=str(home), AGENT_SAFETY_DIRTY_OVERRIDE="1" if dirty else "0") return subprocess.run( [sys.executable, str(INSTALL), *args], capture_output=True, @@ -45,8 +51,8 @@ def setUp(self): self.stamp = self.home / "agent-safety-stamp.json" self.md = self.home / "CLAUDE.md" - def install(self): - r = run(self.home) + def install(self, dirty=False): + r = run(self.home, dirty=dirty) self.assertEqual(r.returncode, 0, r.stdout + r.stderr) return r @@ -66,6 +72,16 @@ def test_install_then_report_is_current(self): self.assertEqual(r.returncode, 0, r.stdout + r.stderr) self.assertIn("CURRENT", r.stdout) + def test_installing_from_a_dirty_checkout_reports_stale(self): + """The bytes on disk are not this commit's, whatever this checkout's real state is.""" + self.install(dirty=True) + stamp = json.loads(self.stamp.read_text(encoding="utf-8")) + if stamp["source"].get("vcs") != "git": + self.skipTest("this checkout is not a git tree, so there is no dirty signal to force") + r = run(self.home, "--report") + self.assertEqual(r.returncode, 1, r.stdout + r.stderr) + self.assertIn("installed from a dirty checkout", r.stdout) + def test_a_changed_payload_reports_stale(self): self.install() target = HERE / "claude-md-safety.md" @@ -142,6 +158,37 @@ def test_an_absent_file_yields_no_blocks_rather_than_raising(self): self.assertEqual(install.blocks_present(self.home / "nothing.md"), {}) +class TestSourceRef(unittest.TestCase): + """Calls install.source_ref() directly, with AGENT_SAFETY_DIRTY_OVERRIDE unset. + + Every StampCase test above forces that override through run(), so none of them exercises + the git status read this class alone still calls. Skips rather than fails when this + checkout cannot supply a clean baseline itself, the same case TestDegradedEnvironments + covers by removing PATH: a tarball with no git, or a real edit already sitting in a + payload file while the suite runs. + """ + + def setUp(self): + saved_override = os.environ.pop("AGENT_SAFETY_DIRTY_OVERRIDE", None) + if saved_override is None: + self.addCleanup(os.environ.pop, "AGENT_SAFETY_DIRTY_OVERRIDE", None) + else: + self.addCleanup(os.environ.__setitem__, "AGENT_SAFETY_DIRTY_OVERRIDE", saved_override) + baseline = install.source_ref() + if baseline.get("vcs") != "git": + self.skipTest("this checkout is not a git tree, so source_ref() reads no status") + if baseline.get("dirty"): + self.skipTest("a payload file is already dirty in this checkout") + + def test_dirtying_a_payload_file_is_detected(self): + """Proves the git-status branch itself, which the override lets every other test skip.""" + target = HERE / install.GUARD_NAME + original = target.read_bytes() + self.addCleanup(target.write_bytes, original) + target.write_bytes(original + b"\n# dirtied by TestSourceRef, restored by addCleanup\n") + self.assertTrue(install.source_ref()["dirty"]) + + class TestInstalledContent(StampCase): """Presence is not currency. These are the cases markers and versions cannot see.""" @@ -469,6 +516,16 @@ def test_an_entry_with_a_defect_is_not_also_reported_absent(self): data["hooks"]["PreToolUse"][0]["matcher"] = "Edit" self._write(data) problems = install.registration_problems(self.home) + sweep_defect = "a SessionEnd entry names the sweep but does not run the deployed one" + matcher_defect = "so that group never fires" + self.assertTrue( + any(sweep_defect in p for p in problems), + f"the planted SessionEnd type defect was not reported: {problems!r}", + ) + self.assertTrue( + any(matcher_defect in p for p in problems), + f"the planted PreToolUse matcher defect was not reported: {problems!r}", + ) self.assertEqual([p for p in problems if "is not registered" in p], [], problems) self.assertEqual([p for p in problems if "never runs" in p], [], problems) diff --git a/host-setup/linux/install-tools.sh b/host-setup/linux/install-tools.sh index dc68f5ac..3082a14f 100755 --- a/host-setup/linux/install-tools.sh +++ b/host-setup/linux/install-tools.sh @@ -884,15 +884,28 @@ tool_status() { fi } -# Where PATH currently resolves $1 to, when that is not $BIN_DIR/$1, or empty when it already is. +# Where PATH currently resolves $1 to, when that directory precedes $BIN_DIR, or empty otherwise. # Shared by the report, which only names the shadow, and --install/--upgrade, which act on it (apply_tool decides when it is safe to remove). -# "type -P" skips aliases and shell functions, which "command -v" answers for with no file behind them. +# Walks PATH itself, since "type -P" answers with the same path whether that directory is ahead of $BIN_DIR or behind it. tool_shadow_path() { - local name="$1" resolved - resolved=$(type -P "$name" 2>/dev/null || true) - # A relative PATH entry (".", "./bin") makes this relative to the caller's current directory, not a real shadow. - # What this returns gets removed by tool_unshadow, so only an absolute path is ever trusted as one. - [[ $resolved == /* && $resolved != "$BIN_DIR/$name" ]] && printf '%s' "$resolved" + local name="$1" dir candidate is_absolute + local -a path_dirs + IFS=':' read -ra path_dirs <<<"$PATH" + for dir in "${path_dirs[@]}"; do + # A trailing slash names the same directory, so strip it before comparing or building a path. + # "/" itself is the one entry with none to strip. + [[ $dir == / ]] || dir="${dir%/}" + [[ $dir == "$BIN_DIR" ]] && return 0 + is_absolute=false + [[ $dir == /* ]] && is_absolute=true + # A relative PATH entry (".", "./bin") makes this relative to the caller's current directory, not a real shadow. + # What this returns gets removed by tool_unshadow, so only an absolute path is ever trusted as one. + candidate="${dir:-.}/$name" + if [[ -x $candidate && -f $candidate ]]; then + [[ $is_absolute == true ]] && printf '%s' "$candidate" + return 0 + fi + done return 0 } diff --git a/repo-config/README.md b/repo-config/README.md index 8a503968..06fe9c09 100644 --- a/repo-config/README.md +++ b/repo-config/README.md @@ -5,7 +5,7 @@ Hub-only repository and branch configuration held as committed files, kept out o - `main.json`, `develop.json`, and `operational/develop.json`: the canonical branch rulesets as the managed part of the writable API subset (`name`, `target`, `enforcement`, `conditions`, `rules`). `main.json` is shared. `develop.json` serves release repos, and `operational/develop.json` serves operational repos. `repo-config/configure.sh check owner/repo release|operational` compares the selected payloads with the live rulesets. `bypass_actors` is writable and deliberately unmanaged, so no payload declares one and nothing diffs it: who may bypass a ruleset is a human decision taken in the UI, which `repo-config/configure.sh` preserves on `apply` and reports without asserting on `check`. - `labels.json`: the fleet label set, one `name`, `color`, and `description` per label. `repo-config/configure.sh apply owner/repo release|operational` creates or updates every declared label by name and deletes nothing, so a label a repo adds of its own stays. `check` asserts each declared label on all three fields and reports the undeclared ones without judging them. - `project.json`: the fleet project every repository is linked to, declared as the project owner, the project number, and the project title. `repo-config/configure.sh apply owner/repo release|operational` resolves the number to a live project, refuses the run when that project's title is not the declared one, and writes the link only when the repository does not already hold it, so a second apply is a read. `check` asserts the link and counts the repository's other project links without judging them, the same way it treats a label the payload never declared. -- `configure.sh`: run from a hub checkout at `main`, per [GOVERNANCE.md "Hub-Hosted Tooling"][governance-hub-hosted-tooling]. It resolves every payload path against the hub's `repo-config/` directory. Name the target repository explicitly, since the command defaults to whichever repository the shell is sitting in. `repo-config/configure.sh apply owner/repo release|operational` creates or updates the settings, Dependabot security features, labels, rulesets, and the fleet project link idempotently. `repo-config/configure.sh check owner/repo release|operational` is the read-only counterpart and exits non-zero on drift. It is not an exact inverse: it also asserts that every environment the registry's `environments` declares for the repo exists and carries the declared deployment-branch policy, neither of which `apply` writes, for the reason [docs/repo-config.md][repo-config-doc] "Deployment Environments" gives. The model defaults to the registry `workflowModel` lookup. Pass it explicitly for a repository outside the registry. +- `configure.sh`: run from a hub checkout at `main`, per [GOVERNANCE.md "Hub-Hosted Tooling"][governance-hub-hosted-tooling]. It resolves every payload path against the hub's `repo-config/` directory. Name the target repository explicitly, since the command defaults to whichever repository the shell is sitting in. `repo-config/configure.sh apply owner/repo release|operational` creates or updates the settings, Dependabot security features, labels, rulesets, and the fleet project link idempotently. `repo-config/configure.sh check owner/repo release|operational` is the read-only counterpart and exits non-zero on drift. It is not an exact inverse: it also asserts that every environment the registry's `environments` declares for the repo exists and carries the declared deployment-branch policy, neither of which `apply` writes, for the reason [docs/repo-config.md][repo-config-doc] "Deployment Environments" gives. The model defaults to the registry `workflowModel` lookup. Pass it explicitly for a repository outside the registry. Both commands exit early, writing nothing, when the target's registry entry carries `status: "archived"`: GitHub rejects every write to an archived repository, so `apply` could never clear the drift `check` would otherwise report there forever. ## Rulesets diff --git a/repo-config/configure.sh b/repo-config/configure.sh index be08557f..3d9b0701 100755 --- a/repo-config/configure.sh +++ b/repo-config/configure.sh @@ -61,6 +61,18 @@ registry="$script_dir/../registry/repos.json" # The registry key is the entry name rather than the owner/repo identity parsed from its url. # Reviewers keep proposing the switch, and the answer is that a name resolves to at most one entry: spec/resolve_description.py raises on a duplicate or a case-only near-miss and exits this script below, and spec/validate.py refuses both when it is run. name="${repo##*/}" + +if [ -f "$registry" ]; then + if ! archived_status="$(jq -r --arg n "$name" '(.repos[] | select(.name==$n) | .status) // empty' "$registry" | sed $'s/\r$//')"; then + echo "Failed to read status from $registry (invalid JSON?)." >&2 + exit 1 + fi + if [ "$archived_status" = archived ]; then + echo "$repo is archived in $registry. GitHub rejects a write to an archived repository, so $cmd makes none and stops here." + exit 0 + fi +fi + if [ -z "$model" ]; then if [ -f "$registry" ]; then # Fail fast on a jq or parse error (a malformed registry) rather than silently applying the release default. @@ -135,7 +147,7 @@ ruleset_id() { return 1 fi # shellcheck disable=SC2016 # $n is a jq --arg variable, not a shell expansion - ids="$(jq -r --arg n "$1" '.[] | select(.name==$n) | .id' <<<"$out")" + ids="$(jqr --arg n "$1" '.[] | select(.name==$n) | .id' <<<"$out")" if [ -z "$ids" ]; then return 0; fi # Pre-existing drift can leave more than one ruleset with the same name. # Use the first and warn, so the duplicates get resolved rather than silently operating on the wrong one. diff --git a/scripts/README.md b/scripts/README.md index 4ffc84c0..5e3792e8 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -111,7 +111,7 @@ Every rule in the default set is clean tree-wide except `comment-added`, which r Three deterministic checks: -- `sha-pin`: every external action or reusable-workflow `uses:` reference is a 40-hex commit SHA, with the documented `dotnet/nbgv@master` exception. References under the scanned repository's owner are also resolved through GitHub. References under another owner are shape-checked only. Local (`./`) and self-repository (`$/`) references run at the workflow commit, so they need no separate pin. +- `sha-pin`: every external action or reusable-workflow `uses:` reference is a 40-hex commit SHA, with the documented `dotnet/nbgv@master` exception. References under the scanned repository's owner are also resolved through GitHub. References under another owner are shape-checked only. Local (`./`) and self-repository (`$/`) references name no ref, so they take no pin, and the check also skips a reference starting with a bare `.github/` without validating it. They resolve differently, though. A `$/` reference and a job-level `./` reusable-workflow call resolve at the calling workflow file's commit, while a `./` action reference resolves against whatever the job checked out, which is the caller's tree where a reusable workflow called from another repository checks out its caller. - `eol`: every path pinned LF in [`.gitattributes`][gitattributes] has the matching [`.editorconfig`][editorconfig] override the line-ending rule requires, with EditorConfig brace syntax expanded. One direction only: an `.editorconfig` LF glob with no git pin is legitimate, since `.editorconfig` governs what the editor writes where git enforces a class it must not guess at. - `eol-coverage`: the same pins read against the tree instead. A tracked file opening `#!` that git does not resolve to `eol=lf` is an interpreter line a CRLF checkout breaks, and a pin matching no tracked file is dead unless its block is marked `forward-declared`. diff --git a/scripts/pr_review.py b/scripts/pr_review.py index d01d40de..7f165f73 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -3092,19 +3092,35 @@ def describe(thread: dict) -> str: ) +_TYPOGRAPHIC_FOLD = str.maketrans( + { + "\u2018": "'", # left single quotation mark + "\u2019": "'", # right single quotation mark + "\u201c": '"', # left double quotation mark + "\u201d": '"', # right double quotation mark + "\u2013": "-", # en dash + "\u2014": "-", # em dash + "\u2026": "...", # horizontal ellipsis + } +) + + def matching_threads(threads: list[dict], match: str, path: str | None) -> list[dict]: """Threads whose finding text contains `match`, narrowed by `path` where one is given. Matched on the finding's own words rather than on a line number, because a fix push moves the line and every lookup keyed to one then misses: replies posted against nothing while the resolves still succeeded, so the threads closed carrying no answer. Case-insensitive, since - the text is quoted back out of a digest by a reader rather than compared by a machine. + the text is quoted back out of a digest by a reader rather than compared by a machine. Both + sides are folded through `_TYPOGRAPHIC_FOLD` first, since the pattern is a substring copied + from a rendered finding whose typographic quotes, dashes, or ellipsis may not survive that + copy in ASCII. """ - needle = match.lower() + needle = match.translate(_TYPOGRAPHIC_FOLD).lower() return [ t for t in threads - if needle in (first_comment(t).get("body") or "").lower() + if needle in (first_comment(t).get("body") or "").translate(_TYPOGRAPHIC_FOLD).lower() and (path is None or t.get("path") == path) ] @@ -3149,7 +3165,8 @@ def reply_to_thread( Every refusal below is a stop rather than a fallback. There is no id to guess at, no second-best thread to settle for, and no resolve on a reply that did not land, because each of those closes a finding while leaving it unanswered, which is the state a reviewer reads as - addressed. + addressed. A no-match names the unresolved count, since zero and several otherwise read the + same without the reader counting the lines the refusal prints below it. """ ok, why = in_scope(owner) if not ok: @@ -3163,6 +3180,7 @@ def reply_to_thread( f"status=NO_MATCH nothing was written: no unresolved thread on {owner}/{repo} " f"#{num} carries {match!r}" + (f" at {path}" if path else "") + + f", of {len(threads)} unresolved thread(s) total" + ". Widen the words or drop --path rather than reaching for an id, since the " "thread may also be resolved already, which reads the same from here." ) diff --git a/scripts/tests/test_configure_archived.py b/scripts/tests/test_configure_archived.py new file mode 100755 index 00000000..e24cf161 --- /dev/null +++ b/scripts/tests/test_configure_archived.py @@ -0,0 +1,188 @@ +#!/usr/bin/env python3 +"""Exercise repo-config/configure.sh's archived-repository exemption by running its own lines. + +The shell is lifted out of the file rather than restated here, so an edit that removes the +behavior fails these tests instead of leaving a reimplementation to agree with itself. The +region under test makes no `gh` call of its own, and the harness assigns every variable the +region reads. So the cases running the lifted region need no `gh` stub: a cataloged or absent +registry falls through to a marker printed just after that region, and an archived one exits +before reaching it. +""" + +from __future__ import annotations + +import json +import os +import re +import shlex +import shutil +import subprocess +import tempfile +import unittest +from pathlib import Path + +CONFIGURE = Path(__file__).resolve().parents[2] / "repo-config" / "configure.sh" + + +def lift(pattern: str) -> str: + """The one region of configure.sh matching `pattern`, or a failure naming what was missing.""" + text = CONFIGURE.read_text(encoding="utf-8") + found = re.findall(pattern, text, re.MULTILINE | re.DOTALL) + if len(found) != 1: + raise AssertionError( + f"expected one match in {CONFIGURE.name} for {pattern!r}, found {len(found)}" + ) + return found[0] + + +def require(*tools: str) -> str: + """The bash path, skipping instead of failing where a tool the harness shells out to is absent.""" + for tool in tools: + if shutil.which(tool) is None: + raise unittest.SkipTest(f"no {tool} on PATH, so the script's own lines cannot be run") + return str(shutil.which("bash")) + + +def run_bash(script: str, *tools: str) -> subprocess.CompletedProcess[str]: + """The script under the same shell options configure.sh sets, with a bounded wait. + + The options are lifted rather than typed, because a harness running without them is blind to + exactly the error handling the lines under test rely on. + """ + bash = require("bash", *tools) + options = lift(r"^(set -[A-Za-z]+ [a-z]+)$") + return subprocess.run( + [bash, "-c", f"{options}\n{script}"], + capture_output=True, + text=True, + encoding="utf-8", + timeout=30, + check=False, + ) + + +ARCHIVED_EXEMPTION = lift(r'(if \[ -f "\$registry" \]; then\n if ! archived_status=.*?\n^fi\n)') + + +class ArchivedRepositoryCase(unittest.TestCase): + """A repository the registry marks archived is out of scope, for apply and check alike.""" + + def registry(self, tmp: str, entries: list[dict[str, object]]) -> Path: + path = Path(tmp) / "repos.json" + path.write_text(json.dumps({"repos": entries}), encoding="utf-8") + return path + + def harness(self, registry: Path, name: str, cmd: str) -> subprocess.CompletedProcess[str]: + script = ( + f"registry={shlex.quote(str(registry))}\nname={shlex.quote(name)}\n" + f"cmd={shlex.quote(cmd)}\nrepo={shlex.quote('owner/' + name)}\n" + f"{ARCHIVED_EXEMPTION}echo REACHED_NEXT\n" + ) + return run_bash(script, "jq") + + def test_an_archived_entry_exits_before_the_next_line(self) -> None: + for cmd in ("check", "apply"): + with self.subTest(cmd=cmd), tempfile.TemporaryDirectory() as tmp: + registry = self.registry(tmp, [{"name": "Fixture", "status": "archived"}]) + result = self.harness(registry, "Fixture", cmd) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("owner/Fixture is archived", result.stdout) + self.assertIn(f"so {cmd} makes none and stops here", result.stdout) + self.assertNotIn("REACHED_NEXT", result.stdout) + + def test_a_cataloged_entry_falls_through_unchanged(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + registry = self.registry(tmp, [{"name": "Fixture", "status": "cataloged"}]) + result = self.harness(registry, "Fixture", "check") + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout, "REACHED_NEXT\n") + + def test_a_name_absent_from_the_registry_falls_through_unchanged(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + registry = self.registry(tmp, [{"name": "Other", "status": "archived"}]) + result = self.harness(registry, "Fixture", "check") + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout, "REACHED_NEXT\n") + + def test_no_registry_file_falls_through_unchanged(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + missing = Path(tmp) / "does-not-exist.json" + result = self.harness(missing, "Fixture", "check") + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout, "REACHED_NEXT\n") + + def test_a_registry_that_will_not_parse_fails_the_run_rather_than_falling_through( + self, + ) -> None: + with tempfile.TemporaryDirectory() as tmp: + broken = Path(tmp) / "repos.json" + broken.write_text("not json", encoding="utf-8") + result = self.harness(broken, "Fixture", "check") + self.assertEqual(result.returncode, 1) + self.assertIn("Failed to read status from", result.stderr) + self.assertNotIn("REACHED_NEXT", result.stdout) + + def test_a_trailing_carriage_return_from_jq_is_stripped(self) -> None: + """Windows' native jq -r appends \\r to its output, which a bare `=` compare would miss.""" + with tempfile.TemporaryDirectory() as tmp: + bin_dir = Path(tmp) / "bin" + bin_dir.mkdir() + stub = bin_dir / "jq" + stub.write_text("#!/bin/sh\nprintf 'archived\\r\\n'\n", encoding="utf-8") + stub.chmod(0o755) + registry = self.registry(tmp, [{"name": "Fixture", "status": "archived"}]) + script = ( + f"PATH={shlex.quote(str(bin_dir))}:$PATH\n" + f"registry={shlex.quote(str(registry))}\nname=Fixture\ncmd=check\nrepo=owner/Fixture\n" + f"{ARCHIVED_EXEMPTION}echo REACHED_NEXT\n" + ) + result = run_bash(script) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("so check makes none and stops here", result.stdout) + self.assertNotIn("REACHED_NEXT", result.stdout) + + +class EndToEndCase(unittest.TestCase): + """Run on an archived repository given by name, the real script exits before any `gh` call. + + The script is copied whole, and both apply and check are covered. + """ + + def run_configure(self, status: str, cmd: str) -> subprocess.CompletedProcess[str]: + bash = require("bash", "jq") + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + (root / "repo-config").mkdir() + (root / "registry").mkdir() + (root / "bin").mkdir() + shutil.copy(CONFIGURE, root / "repo-config" / "configure.sh") + (root / "registry" / "repos.json").write_text( + json.dumps({"repos": [{"name": "Fixture", "status": status}]}), encoding="utf-8" + ) + gh_stub = root / "bin" / "gh" + gh_stub.write_text( + "#!/bin/sh\necho 'gh should never run for an archived repo' >&2\nexit 1\n", + encoding="utf-8", + ) + gh_stub.chmod(0o755) + env = dict(os.environ, PATH=f"{root / 'bin'}:{os.environ.get('PATH', os.defpath)}") + return subprocess.run( + [bash, str(root / "repo-config" / "configure.sh"), cmd, "owner/Fixture"], + capture_output=True, + text=True, + encoding="utf-8", + timeout=30, + check=False, + env=env, + ) + + def test_an_archived_target_never_calls_gh(self) -> None: + for cmd in ("check", "apply"): + with self.subTest(cmd=cmd): + result = self.run_configure("archived", cmd) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertNotIn("gh should never run", result.stderr) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/tests/test_pr_review.py b/scripts/tests/test_pr_review.py index 299e4544..eaf42575 100755 --- a/scripts/tests/test_pr_review.py +++ b/scripts/tests/test_pr_review.py @@ -5220,6 +5220,64 @@ def test_the_match_reads_the_finding_text_rather_than_a_line_number(self) -> Non self.assertEqual(0, self.run_reply("--resolve")) self.assertIn("REPLIED_AND_RESOLVED", self.out.getvalue()) + def test_an_ascii_pattern_selects_a_body_written_with_typographic_punctuation(self) -> None: + """A `--match` string copied from a rendered finding is ASCII, and the body may not be.""" + body = ( + "The helper wasn\u2019t clear on \u201cwidening the words\u201d \u2014 it just " + "refuses\u2026" + ) + self.wire(page([rthread("t1", body=body)])) + self.assertEqual( + 0, + self.run_reply( + "--resolve", "--match", 'wasn\'t clear on "widening the words" - it just refuses' + ), + ) + self.assertIn("REPLIED_AND_RESOLVED", self.out.getvalue()) + + def test_a_typographic_pattern_selects_a_body_written_in_ascii(self) -> None: + """The fold runs both ways: a pasted pattern can carry the typographic form too.""" + self.wire(page([rthread("t1", body='The helper says "widen the words" - try again.')])) + self.assertEqual( + 0, + self.run_reply("--resolve", "--match", "\u201cwiden the words\u201d \u2014 try again"), + ) + self.assertIn("REPLIED_AND_RESOLVED", self.out.getvalue()) + + def test_every_documented_character_selects_across_the_ascii_boundary(self) -> None: + """Each of the issue's seven characters folds on its own, an expectation independent of + `_TYPOGRAPHIC_FOLD` itself, so dropping one from that table still fails this.""" + expected_folds = { + 0x2018: "'", # left single quotation mark + 0x2019: "'", # right single quotation mark + 0x201C: '"', # left double quotation mark + 0x201D: '"', # right double quotation mark + 0x2013: "-", # en dash + 0x2014: "-", # em dash + 0x2026: "...", # horizontal ellipsis + } + for code_point, ascii_form in expected_folds.items(): + char = chr(code_point) + with self.subTest(char=repr(char)): + self.out.seek(0) + self.out.truncate(0) + self.wire(page([rthread("t1", body=f"The finding reads foo{char}bar plainly.")])) + self.assertEqual(0, self.run_reply("--resolve", "--match", f"foo{ascii_form}bar")) + self.assertIn("REPLIED_AND_RESOLVED", self.out.getvalue()) + + def test_no_match_names_how_many_unresolved_threads_there_are(self) -> None: + """Zero and several unresolved threads otherwise read the same, with nothing to tell them apart.""" + self.wire( + page( + [ + rthread("t1", body="An unrelated finding about naming."), + rthread("t2", body="A second, also unrelated finding."), + ] + ) + ) + self.assertEqual(60, self.run_reply("--resolve")) + self.assertIn("of 2 unresolved thread(s) total", self.out.getvalue()) + class TestReplyConfirmsBeforeResolving(ReplyCase): def test_a_reply_returning_no_url_leaves_the_thread_open(self) -> None: diff --git a/scripts/tests/test_prose_lint.py b/scripts/tests/test_prose_lint.py index 6d467246..94bae961 100755 --- a/scripts/tests/test_prose_lint.py +++ b/scripts/tests/test_prose_lint.py @@ -1230,7 +1230,7 @@ def test_css_has_block_comments_only(self) -> None: self.assertEqual(["comment-wrap"], self.flag("a.css", "/* One thing. Another thing. */\n")) def test_a_version_pin_is_machinery_rather_than_prose(self) -> None: - """The action-pinning rule requires a trailing `# vX.Y.Z`, which is a label, not a sentence.""" + """The action-pinning rule requires a trailing release-tag comment such as `# vX.Y.Z`, which is a label, not a sentence.""" self.assertEqual([], self.flag("a.yml", " uses: x@sha # v7.0.0\n")) self.assertEqual([], self.flag("a.yml", " uses: x@sha # v3\n")) diff --git a/scripts/tests/test_release_guards.py b/scripts/tests/test_release_guards.py index 894b0745..6cbd9768 100755 --- a/scripts/tests/test_release_guards.py +++ b/scripts/tests/test_release_guards.py @@ -1011,6 +1011,84 @@ def test_validator_pytest_leg_fans_out_over_every_named_interpreter(self) -> Non # Without a flag naming its leg, each upload merges into one number that hides which leg it came from. self.assertIn(" flags: python-${{ matrix.python-version }}\n", job) + def test_validator_checks_out_the_triggering_commit(self) -> None: + """A ref: on any checkout moves the gate off the commit a publisher releases. + + A bare checkout of the workflow repository fetches github.sha, and a reusable workflow reads + its caller's github context, so the bare form is what validates the exact commit a publisher + pins. No step in the task carries a ref: at all, asserted file-wide over block, flow, and + quoted keys, so no step shape or mapping style carries one past the check. + """ + workflow = (REPO / ".github/workflows/validate-task.yml").read_text(encoding="utf-8") + self.assertIn("actions/checkout@", workflow) + self.assertNotRegex(workflow, r"(?m)(^[ \t-]*|[{,][ \t]*)[\"']?ref[\"']?[ \t]*:") + + @unittest.skipUnless( + shutil.which("bash") and os.name == "posix", + "the step runs under bash against a stand-in dotnet that only a POSIX host can execute", + ) + def test_validator_dotnet_leg_fails_when_no_report_was_written(self) -> None: + """The best-effort upload reads an empty directory exactly as it reads a healthy run. + + The step's whole script is run against a stand-in dotnet, since a presence check on the + assertion would stay green if its condition were inverted. Every case starts from a tree + already holding a renamed report, which is what a committed one looks like, so a check that + counted it would pass a run that measured nothing. + """ + workflow = (REPO / ".github/workflows/validate-task.yml").read_text(encoding="utf-8") + job = workflow.split("\n unit-test:\n", 1)[1].split("\n validate:\n", 1)[0] + marker = " - name: Run unit tests step\n" + self.assertIn(marker, job) + body = job.split(marker, 1)[1] + opener = re.search(r"(?m)^ run: \|-?\n", body) + self.assertIsNotNone(opener, "the step's script must be a literal block scalar") + assert opener is not None + lines: list[str] = [] + for line in body[opener.end() :].splitlines(): + if line and not line.startswith(" " * 10): + break + lines.append(line[10:]) + script = "\n".join(lines) + + writers = { + "report": 'mkdir -p ./coverage && echo "" > "./coverage/0f1e2d3c.cobertura.xml"', + "empty report": 'mkdir -p ./coverage && : > "./coverage/0f1e2d3c.cobertura.xml"', + "nothing": "mkdir -p ./coverage", + "no directory": ":", + } + expected = {"report": 0, "empty report": 1, "nothing": 1, "no directory": 1} + for case, writer in writers.items(): + with self.subTest(case=case), tempfile.TemporaryDirectory() as scratch: + bin_dir = Path(scratch) / "bin" + bin_dir.mkdir() + stand_in = bin_dir / "dotnet" + stand_in.write_text(f"#!/usr/bin/env bash\n{writer}\n", encoding="utf-8") + stand_in.chmod(0o755) + work = Path(scratch) / "work" + (work / "coverage").mkdir(parents=True) + (work / "coverage/coverage-committed.cobertura.xml").write_text( + "", encoding="utf-8" + ) + verdict = run( + ["bash", "-c", script], + cwd=work, + env={ + **os.environ, + "PATH": f"{bin_dir}{os.pathsep}{os.environ.get('PATH', os.defpath)}", + }, + capture_output=True, + text=True, + encoding="utf-8", + check=False, + ) + self.assertEqual( + expected[case], verdict.returncode, verdict.stdout + verdict.stderr + ) + if expected[case]: + self.assertIn("::error::", verdict.stdout) + else: + self.assertTrue((work / "coverage/coverage-0f1e2d3c.cobertura.xml").exists()) + @unittest.skipUnless(shutil.which("jq"), "jq is what the step under test runs") def test_validator_refuses_a_python_versions_value_fromjson_would_admit(self) -> None: """fromJSON admits a JSON array of numbers, which is not a list of interpreter versions. diff --git a/scripts/tests/test_repo_gate.py b/scripts/tests/test_repo_gate.py index 0ce17d69..be6a25c8 100755 --- a/scripts/tests/test_repo_gate.py +++ b/scripts/tests/test_repo_gate.py @@ -12,6 +12,7 @@ import contextlib import io +import os import re import shutil import subprocess @@ -467,6 +468,81 @@ def test_the_note_carries_every_count_including_the_zeroes(self) -> None: ) +class TestQuotedNames(GitTreeCase): + """A tracked name git quotes reaches every check as the name on disk. + + The constructed name holds a byte that is not valid UTF-8, the case that both crashed the + listing under `core.quotePath=false` and, at the default, left an escaped spelling no file + answers to. + """ + + def setUp(self) -> None: + super().setUp() + try: + self.NAME = os.fsdecode(b"run-\xff-tool") + (self.tmp / self.NAME).write_text("#!/bin/sh\nexit 0\n", encoding="utf-8") + except (OSError, ValueError): + self.skipTest("this filesystem refuses a name that is not valid UTF-8") + + def test_an_inherited_quote_path_false_lists_the_real_name_rather_than_raising(self) -> None: + self.git("config", "core.quotePath", "false") + self.git("add", "-A") + self.assertEqual([self.NAME], repo_gate.tracked(self.tmp)) + + def test_a_quoted_shebang_name_is_still_checked(self) -> None: + gitattributes = TestEolCoverage.GITATTRIBUTES.replace("eol=lf", "eol=crlf", 1) + hits = self.coverage(gitattributes, {}) + self.assertIn(self.NAME, repo_gate.tracked(self.tmp)) + self.assertTrue(any(f"{self.NAME}: tracked shebang path" in hit for hit in hits), hits) + + def test_the_run_prints_such_a_name_under_a_strict_output_encoding(self) -> None: + gitattributes = TestEolCoverage.GITATTRIBUTES.replace("eol=lf", "eol=crlf", 1) + self.coverage(gitattributes, {}) + out = io.TextIOWrapper(io.BytesIO(), encoding="utf-8", errors="strict") + err = io.TextIOWrapper(io.BytesIO(), encoding="utf-8", errors="strict") + with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): + rc = repo_gate.main(["--root", str(self.tmp), "--check", "eol-coverage"]) + out.flush() + self.assertEqual(1, rc) + self.assertIn(b"run-\\udcff-tool: tracked shebang path", out.buffer.getvalue()) + + def test_a_missing_root_named_in_bytes_that_are_not_utf8_fails_without_raising(self) -> None: + missing = self.tmp / self.NAME / "gone" + err = io.TextIOWrapper(io.BytesIO(), encoding="utf-8", errors="strict") + with contextlib.redirect_stderr(err): + self.assertEqual([], repo_gate.tracked(missing)) + err.flush() + self.assertIn(b"git ls-files failed", err.buffer.getvalue()) + self.assertIn(b"run-\\udcff-tool", err.buffer.getvalue()) + + +class TestQuotedPlainNames(GitTreeCase): + def test_a_name_quoted_for_a_quote_or_backslash_decodes_to_itself(self) -> None: + odd = 'say "hi"\\now' + try: + (self.tmp / odd).write_text("x\n", encoding="utf-8") + except OSError: + self.skipTest("this filesystem refuses a quote or a backslash in a name") + self.git("add", "-A") + self.assertIn(odd, repo_gate.tracked(self.tmp)) + + def test_a_control_character_in_a_name_prints_escaped_rather_than_raw(self) -> None: + name = "run\n[ok ] forged\x1b[2J" + gitattributes = TestEolCoverage.GITATTRIBUTES.replace("eol=lf", "eol=crlf", 1) + try: + (self.tmp / name).write_text("#!/bin/sh\nexit 0\n", encoding="utf-8") + except OSError: + self.skipTest("this filesystem refuses a control character in a name") + self.coverage(gitattributes, {}) + out = io.StringIO() + with contextlib.redirect_stdout(out): + rc = repo_gate.main(["--root", str(self.tmp), "--check", "eol-coverage"]) + self.assertEqual(1, rc) + self.assertIn("run\\x0a[ok ] forged\\x1b[2J: tracked shebang path", out.getvalue()) + self.assertNotIn("\x1b", out.getvalue()) + self.assertFalse(any(l.startswith("[ok ] forged") for l in out.getvalue().splitlines())) + + class TestGovernanceCoupling(unittest.TestCase): def test_the_exception_set_matches_what_the_doc_documents(self) -> None: """The doc calls it the one documented exception, so the code must not carry a second."""