fix(786,789): workflows declare their own per-job metadata; guard populations derive from it (#872)
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 7s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 25s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m41s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m21s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 5m58s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m8s
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 7s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 25s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m41s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m21s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 5m58s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m8s
Closes #786 and #789, bundled because working either alone would build the artifact the other removes. Every job in all six tracked workflows declares `env.CI_JOB_ROLE` (guard/report-only/none); the `docker-build.yml` jobs also declare `env.CI_EXECUTION_CLASS` (toolchain/bare-runner). Both guard populations derive from those markers; the `TOOLCHAIN_JOBS`/`BARE_RUNNER_JOBS` literals are deleted. A missing or unrecognised marker is a hard failure in both checkers. #789's literal had a real justification — set equality between two DERIVED sets is blind to a member leaving both at once — so the marker is the anchor that replaces it, and the cost (proximity to the `container:` block) is paid by a THIRD derivation from each job's own steps, which is also the only check that sees the failure #789 filed: a .NET step moved into a bare-runner job, where no set changes. The residual is disclosed: drop the block, flip the marker AND hide the tool behind a script and all three go blind, bounded by the failure mode being a loud missing-binary crash. #786's guard jobs join a machine-checked population: a new `test_workflow_job_guards.py` asserts set equality both ways against a new "Workflow-job guards" table, and the four jobs with no dropped-step guard each carry a recorded decision. Two issue claims were refuted by measurement: #789's "editing docker-build.yml re-points the pin" (the pathspec is `docker/ci` only) and #786's job count (17, not 15). Four cold adversarial review rounds across two model families; rounds 1-3 BLOCKED, all findings fixed and each fix demonstrated by reproducing the reviewer's own test. The recurring defect class was prose drifting from code, including a mechanism claim in the decision record that execution refuted. All five mutation proofs redden when their shipped detector is disarmed. New decision record: `testing.workflow-declares-its-own-job-metadata`. Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
This commit was merged in pull request #872.
This commit is contained in:
@@ -111,6 +111,8 @@ jobs:
|
||||
# branch dispatch), so it costs the ubuntu-latest lane almost nothing, and
|
||||
# ci-runner (.127) runs no prod workload.
|
||||
runs-on: ubuntu-latest
|
||||
env:
|
||||
CI_JOB_ROLE: none
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v4
|
||||
|
||||
@@ -47,6 +47,8 @@ jobs:
|
||||
scan:
|
||||
name: NuGet vulnerable packages
|
||||
runs-on: ubuntu-latest
|
||||
env:
|
||||
CI_JOB_ROLE: guard
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v4
|
||||
|
||||
@@ -141,6 +141,9 @@ jobs:
|
||||
toolchain-preflight:
|
||||
name: CI toolchain image resolves
|
||||
runs-on: small
|
||||
env:
|
||||
CI_EXECUTION_CLASS: bare-runner
|
||||
CI_JOB_ROLE: guard
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v4
|
||||
@@ -165,6 +168,9 @@ jobs:
|
||||
credentials:
|
||||
username: ${{ secrets.REGISTRY_USER }}
|
||||
password: ${{ secrets.REGISTRY_PASSWORD }}
|
||||
env:
|
||||
CI_EXECUTION_CLASS: toolchain
|
||||
CI_JOB_ROLE: guard
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v4
|
||||
@@ -438,6 +444,9 @@ jobs:
|
||||
--health-interval=5s
|
||||
--health-timeout=5s
|
||||
--health-retries=30
|
||||
env:
|
||||
CI_EXECUTION_CLASS: toolchain
|
||||
CI_JOB_ROLE: guard
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v4
|
||||
@@ -605,6 +614,9 @@ jobs:
|
||||
credentials:
|
||||
username: ${{ secrets.REGISTRY_USER }}
|
||||
password: ${{ secrets.REGISTRY_PASSWORD }}
|
||||
env:
|
||||
CI_EXECUTION_CLASS: toolchain
|
||||
CI_JOB_ROLE: guard
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v4
|
||||
@@ -749,6 +761,9 @@ jobs:
|
||||
scan:
|
||||
name: Delimiter ban (release path)
|
||||
runs-on: small
|
||||
env:
|
||||
CI_EXECUTION_CLASS: bare-runner
|
||||
CI_JOB_ROLE: guard
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v4
|
||||
@@ -823,6 +838,9 @@ jobs:
|
||||
# where an image is published and never booted.
|
||||
needs: [test, migrations, scan]
|
||||
if: github.event_name != 'pull_request'
|
||||
env:
|
||||
CI_EXECUTION_CLASS: bare-runner
|
||||
CI_JOB_ROLE: none
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v4
|
||||
@@ -1010,6 +1028,9 @@ jobs:
|
||||
username: ${{ secrets.REGISTRY_USER }}
|
||||
password: ${{ secrets.REGISTRY_PASSWORD }}
|
||||
if: github.event_name == 'pull_request'
|
||||
env:
|
||||
CI_EXECUTION_CLASS: toolchain
|
||||
CI_JOB_ROLE: guard
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v4
|
||||
@@ -1112,6 +1133,9 @@ jobs:
|
||||
username: ${{ secrets.REGISTRY_USER }}
|
||||
password: ${{ secrets.REGISTRY_PASSWORD }}
|
||||
if: github.event_name == 'pull_request'
|
||||
env:
|
||||
CI_EXECUTION_CLASS: toolchain
|
||||
CI_JOB_ROLE: guard
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v4
|
||||
|
||||
@@ -76,6 +76,8 @@ jobs:
|
||||
name: CI image pin matches docker/ci
|
||||
runs-on: small
|
||||
if: github.event_name == 'pull_request'
|
||||
env:
|
||||
CI_JOB_ROLE: guard
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v4
|
||||
@@ -171,6 +173,8 @@ jobs:
|
||||
name: Docs update reminder
|
||||
runs-on: small # seconds-long git diff; keep it off the build runners
|
||||
if: github.event_name == 'pull_request'
|
||||
env:
|
||||
CI_JOB_ROLE: report-only
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v4
|
||||
@@ -255,6 +259,8 @@ jobs:
|
||||
name: decisions lifecycle
|
||||
runs-on: small
|
||||
if: github.event_name == 'pull_request'
|
||||
env:
|
||||
CI_JOB_ROLE: guard
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v4
|
||||
@@ -304,6 +310,8 @@ jobs:
|
||||
name: "Fix proofs (Proves trailers)"
|
||||
runs-on: small
|
||||
if: github.event_name == 'pull_request'
|
||||
env:
|
||||
CI_JOB_ROLE: guard
|
||||
steps:
|
||||
- name: Checkout
|
||||
# Full history: prove-fix.sh reverts each commit against its PARENT, so a shallow
|
||||
@@ -422,6 +430,8 @@ jobs:
|
||||
name: Script lint and tests (ruff + pytest)
|
||||
runs-on: small
|
||||
if: github.event_name == 'pull_request'
|
||||
env:
|
||||
CI_JOB_ROLE: guard
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v4
|
||||
|
||||
@@ -63,6 +63,8 @@ jobs:
|
||||
runs-on: ubuntu-latest
|
||||
container:
|
||||
image: renovate/renovate:43
|
||||
env:
|
||||
CI_JOB_ROLE: none
|
||||
steps:
|
||||
- name: Run Renovate
|
||||
env:
|
||||
|
||||
@@ -137,6 +137,8 @@ jobs:
|
||||
code: write # POST /statuses/{sha} (the gate) + the /commits/{sha}/status reads
|
||||
issues: read # GET /issues/{n}/timeline — the retarget-count fence (#706)
|
||||
pull-requests: read # scripts/pr-changed-files.sh -> GET /pulls/{n}/files (#698 exemption path)
|
||||
env:
|
||||
CI_JOB_ROLE: guard
|
||||
steps:
|
||||
# BASE, not head — see the header. `fetch-depth: 1` is enough: nothing here reads history,
|
||||
# only the working tree's `scripts/`.
|
||||
|
||||
@@ -32,6 +32,7 @@ doc below, or that changes which sections a task signal points to.**
|
||||
| Testing a surface gated by config / an env var / a credential | `docs/decisions/records/testing/deny-path-at-production-config-value.md` — cover the setting absent, at its production value, and each opt-out, and assert the DENY branch |
|
||||
| Touching a full-replace write path or a hand-built request object | `docs/decisions/records/testing/full-replace-asserts-field-list.md` — derive the field list from the DTO and assert set equality; reconcile by id where child state exists. In the SPA the same rule is enforced by the type system: `docs/spa-conventions.md` §4b — build the body as `Complete<T>`, annotating BOTH the wrapper parameter and every construction site |
|
||||
| Writing or editing any doc, or answering a review finding in prose | `docs/decisions/records/docs/no-session-narrative.md` — the doc records the END STATE; the path to it goes in the commit message. Apply the who-benefits test, and read the carve-out before you cut (dated measurements, stated snapshot boundaries and tested-and-rejected results stay) |
|
||||
| Adding, renaming or removing a workflow JOB | `docs/ci-cd.md` → "Per-job declarations" — every job declares `env.CI_JOB_ROLE` (and, in `docker-build.yml`, `env.CI_EXECUTION_CLASS`); a missing or unknown value fails `scripts/tests/test_workflow_job_guards.py` / `…/test_ci_image_pin_population.py`, and a `guard` **or `report-only`** job also needs a row in `docs/guard-inventory.md` → "Workflow-job guards". Rationale: `docs/decisions/records/testing/workflow-declares-its-own-job-metadata.md` |
|
||||
| Adding / changing / deleting a guard file | `docs/guard-inventory.md` — every guard's row is machine-checked by `scripts/tests/test_guard_inventory.py`, so a new guard must acquire a row before the suite goes green, and a row graded `MUTATION` must also acquire a declared clause in `scripts/tests/mutation_manifest.py` |
|
||||
| Writing code that reads live Gitea/remote state and then acts on it | `docs/decisions/records/process/check-and-use-pins-a-version.md`, then `docs/remote-state-inventory.md` — a new executable under `scripts/` (**excluding `scripts/tests/`**), `.claude/hooks/`, `.husky/` or `.gitea/workflows/` must acquire a row there before `scripts/tests/test_remote_state_inventory.py` goes green |
|
||||
| Finding every site that references a symbol (multi-site fix/sweep) | `docs/local-lsp-tooling.md` — which of the three surfaces answers, and why a delegated agent must be pointed at an MCP server (`csharp-lsp`, or `serena` after an `activate_project`) rather than the `LSP` tool, which no subagent has been observed to reach |
|
||||
|
||||
@@ -341,6 +341,74 @@ the point** — ersatztv#835 was deliberately held back rather than shipped with
|
||||
there the exemption would still have passed and the guard would have been permanently blind to the
|
||||
file it was written for.
|
||||
|
||||
## Per-job declarations (`env.CI_EXECUTION_CLASS`, `env.CI_JOB_ROLE` — ersatztv#786/#789)
|
||||
|
||||
**Every job in every tracked workflow declares `CI_JOB_ROLE`; the jobs in `docker-build.yml` also
|
||||
declare `CI_EXECUTION_CLASS`** (the scopes differ — see the table). They are metadata, not runtime
|
||||
inputs: nothing reads them at execution time. They exist so that a guard comparing sets of jobs
|
||||
derives its expected set from the workflow instead of from a literal list in the checker.
|
||||
|
||||
| Marker | Values | Scope | Derived by |
|
||||
| --- | --- | --- | --- |
|
||||
| `CI_EXECUTION_CLASS` | `toolchain`, `bare-runner` | `docker-build.yml` only — the sole workflow using the toolchain image | `scripts/tests/test_ci_image_pin_population.py` |
|
||||
| `CI_JOB_ROLE` | `guard`, `report-only`, `none` | every tracked workflow | `scripts/tests/test_workflow_job_guards.py` |
|
||||
|
||||
**A missing marker and an unrecognised value both fail the suite.** Neither defaults: a scheme whose
|
||||
absent value reads as some safe class stops applying the moment a job is added and nobody notices.
|
||||
|
||||
**`CI_JOB_ROLE` is decided by what the job PRODUCES.** A `guard` job's output is a **verdict**; a
|
||||
`none` job's output is an **artifact**, and a red there means the build did not work rather than that
|
||||
an invariant was violated. `docker-build.yml::build` publishes an image and smoke-tests it, and is
|
||||
`none`. Which jobs are `none` is read off the markers rather than listed here — a list in prose is a
|
||||
second copy of the workflow that rots on the next job added. Every `guard`/`report-only` job holds a
|
||||
row in `docs/guard-inventory.md` → "Workflow-job guards", cross-checked both ways.
|
||||
|
||||
`report-only` is the third value: a verdict the job **cannot enforce**, because every check step
|
||||
carries `continue-on-error: true`.
|
||||
|
||||
**A rejected alternative, so it is not re-adopted:** "a guard enforces an invariant about the
|
||||
*repository*, so a job exercising the *product* is `none`" reads as more principled and is wrong —
|
||||
it puts `test` and `migrations` outside the population, and those are the two **required** status
|
||||
contexts on `main`, i.e. precisely where a failure to fire is fail-open against branch protection.
|
||||
|
||||
**Why not a literal list**, since one stood here until #789 with a real argument behind it: set
|
||||
equality between two *derived* sets is blind to a member leaving both at once, so a job that loses
|
||||
its `container:` block leaves the declared and the pinned set together and the comparison stays
|
||||
balanced. That needs an anchor that does not move with the block — and a reviewed literal was the
|
||||
only one available. The marker is the same anchor, better placed: it lives in the job it describes,
|
||||
so it is reachable by anyone reading that job and it moves with the job when the job is renamed.
|
||||
(Not "in the same hunk" — measured, the marker sits 6–7 lines from the `container:` block in four
|
||||
jobs and **58** in `migrations`, where the `services:` block sits between them. Proximity is a
|
||||
convenience; the property the guard rests on is that the marker does not move when the block does.)
|
||||
|
||||
**Its cost is proximity**, so the scheme does not stop there. A literal in a distant file survives a
|
||||
careless workflow edit; a marker a handful of lines from the `container:` block can go with it. So
|
||||
`test_no_job_needs_the_toolchain_without_declaring_it` derives the requirement a third time — from
|
||||
each job's own step bodies, looking for tools that exist only in the CI image (`dotnet`,
|
||||
`dotnet-ef`, `node`, `npm`, `npx`, `ffmpeg`, `ffprobe`, `playwright`, `reportgenerator` — the list
|
||||
lives in `TOOLCHAIN_ONLY_TOOLS`, and this sentence is a summary of it, not a second copy to keep in
|
||||
sync). `git`/`jq`/`python3`/`curl`/`tar` are **excluded** because they do not discriminate: the
|
||||
bare-runner lanes provide or provision them too. That check depends on neither the marker nor the
|
||||
block, and it is the only one that catches a .NET step **moved** into a bare-runner job, where no
|
||||
set changes at all.
|
||||
|
||||
It is a **necessary condition only**: a job whose *only* toolchain use sat behind a script would be
|
||||
invisible to it. Measured — the blind-spot set is **empty today**: all five declared toolchain jobs
|
||||
are detected directly, `functional-e2e` included (it runs `dotnet restore` and `npm ci` in its own
|
||||
steps before handing off to `scripts/e2e-local.sh`). The converse is still not asserted, because a
|
||||
future job could be written that way and a red on a correct tree is how a guard gets deleted.
|
||||
|
||||
**The residual, stated because it is the one that matters.** All three checks go BLIND together
|
||||
(none of them reddens) under one plausible edit: drop the `container:` block, flip the marker, *and* move the invocation into a
|
||||
script. Both set comparisons then stay balanced and the step scan sees no token. What bounds this is
|
||||
the failure **mode**, not its likelihood — the job runs a missing binary and dies with
|
||||
`dotnet: command not found`, loud and immediate, where ersatztv#774's original defect sent a
|
||||
*required* check green on the bare runner. Trading a silent pass for a noisy crash is the point;
|
||||
it is not a claim the hole is closed.
|
||||
|
||||
Full rationale: `docs/decisions/records/testing/workflow-declares-its-own-job-metadata.md`.
|
||||
|
||||
|
||||
## The workflow: `.gitea/workflows/docker-build.yml`
|
||||
|
||||
Single workflow. Gating jobs `test` + `migrations` run in parallel and gate `build`; a
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -0,0 +1,64 @@
|
||||
---
|
||||
key: testing.workflow-declares-its-own-job-metadata
|
||||
title: '2026-08-28 — a workflow declares its own per-job metadata, and guard populations derive from those markers rather than from literals in the checker (#786, #789)'
|
||||
status: active
|
||||
since: '2026-08-28'
|
||||
supersedes: none
|
||||
superseded-by: none
|
||||
rule: 'A population of workflow JOBS is declared by the workflow itself, one machine-readable marker per job under `env:`, and every set a guard compares is DERIVED from those markers rather than from a literal list in the checker. A missing marker and an unrecognised value are both HARD FAILURES, never a default: a scheme whose absent value reads as some safe class stops applying the moment somebody adds a job and forgets, and the job nobody remembered to mark is the job nobody reviewed. Two markers exist under this rule. `CI_EXECUTION_CLASS` (toolchain / bare-runner) replaced the `TOOLCHAIN_JOBS` and `BARE_RUNNER_JOBS` literals in `test_ci_image_pin_population.py` (#789). `CI_JOB_ROLE` (guard / report-only / none) is the population of workflow-job guards that `docs/guard-inventory.md` was blind to (#786), and it is decided by what the job PRODUCES: a `guard` job output is a VERDICT, a `none` job output is an ARTIFACT, and a red in a `none` job means the build did not work rather than that an invariant was violated. `docker-build.yml::build` publishes an image and smoke-tests it and is `none`. Which jobs are `none` is read off the markers and is deliberately NOT enumerated here — a list in prose is a second copy of the workflow that rots on the next job added, and nothing checks it. TWO READINGS ARE REJECTED and recorded so neither returns. Anything-that-can-fail makes every job a guard and the table distinguishes nothing. And the more principled-sounding repository-versus-product split is worse: it puts `test` and `migrations` outside the population, and those are the two REQUIRED status contexts on `main`, precisely where a failure to fire is fail-OPEN against branch protection and precisely the gap #786 exists to close. THE VALUE IS `report-only` AND NOT `advisory` ON PURPOSE: `docs/ci-cd.md` already calls `functional-e2e` an advisory job in a different sense — not a required context, though it can certainly fail — so reusing the word would make a reader infer the wrong marker for exactly the job whose marker is `guard`. WHY THE MARKER BEATS THE LITERAL it replaced, since the literal had a real justification: set equality between two DERIVED sets is blind to a member leaving both at once, so a job that loses its `container:` block leaves the declared and the pinned set together and the comparison stays balanced. The population therefore needs an anchor that does not move with the thing it guards, and a reviewed literal was the only one available. The marker is that anchor and is strictly better placed: it lives NEXT TO the job, so a reviewer of that job reaches it, and it moves with the job when the job is renamed. THE COST IS ADJACENCY and must be paid explicitly rather than argued away — a literal in a distant file survives a careless workflow edit, whereas a marker a handful of lines from the `container:` block can be deleted along with it in one plausible slip. So a marker scheme is not sufficient on its own: it ships with a THIRD derivation independent of both the marker and the thing it describes. For the toolchain class that is `jobs_whose_steps_need_the_toolchain`, which reads the job OWN step bodies for tools present only in the CI image, and it is the only one of the three that can see the failure #789 was filed for — a .NET-dependent step MOVED into a bare-runner job, where no set changes and every equality stays balanced. That third check is NECESSARY-CONDITION ONLY and says so: step text cannot see a tool reached only through a script, so visible use implies the declaration and the converse is not asserted. MEASURED rather than asserted — the blind-spot set is EMPTY today, all five declared toolchain jobs being detected directly. AND THE RESIDUAL IS REAL: all three checks fail together under one edit that drops the container block, flips the marker AND moves the invocation into a script. What bounds it is the failure MODE — the job then dies on a missing binary, loudly, where the original defect would send a REQUIRED check green on the bare runner. A silent pass traded for a noisy crash, not a hole closed. Filtering to the jobs that visibly invoke a tool is legitimate subject selection for a PER-MEMBER property under `testing.guard-derives-population-from-source`, not a filtered completeness claim; the completeness claims are that every job declares a marker and that the two sets match both ways. WHAT THIS DOES NOT DO, stated because the checks read stronger than they are: it proves every job was CLASSIFIED by someone and that the table and the workflow agree, never that the classification is CORRECT, and an inline YAML assertion cannot be proven to work without running the job, so such a row carries `Proof: NONE` unless a pytest genuinely covers PART of it — `ci-image-pin` and `set-verdict-status` each cite one, and each row says which part, because a proof reference that covers a fraction of a job must not read as covering the job.'
|
||||
signals: 'CI_EXECUTION_CLASS · CI_JOB_ROLE · per-job marker · workflow declares its own metadata · TOOLCHAIN_JOBS removed · BARE_RUNNER_JOBS removed · missing marker is a hard failure · unknown marker value is a hard failure · the anchor that does not move · adjacency is the cost · third derivation from the job own steps · toolchain-only tools · a step MOVED into a bare-runner job · guard jobs vs guard files · role records purpose not failability · paths: `scripts/tests/test_ci_image_pin_population.py`, `scripts/tests/test_workflow_job_guards.py`, `docs/guard-inventory.md`, `.gitea/workflows/docker-build.yml` · issues: #786, #789, #774, #787, #806'
|
||||
mechanics: '`CI_EXECUTION_CLASS` is scoped to `docker-build.yml`, the only workflow using the toolchain image — a claim `test_docker_build_is_the_ONLY_workflow_pinning_the_toolchain_image` checks against the tracked workflow set. `CI_JOB_ROLE` spans every tracked workflow. Both populations come from the GIT INDEX per `testing.guard-derives-population-from-source`. The toolchain-only tool list is drawn from `docker/ci/Dockerfile` and deliberately excludes git, jq, python3 and curl. The reason is that they do not DISCRIMINATE: the bare-runner lanes provide or provision them (`small` ships git and adds Python via `actions/setup-python`; `build` runs on `ubuntu-latest`), so a match on one says nothing about whether the job needs the toolchain image. Measured, they flag 4 of the 8 jobs, not all of them.'
|
||||
---
|
||||
|
||||
**A hand-written list of workflow jobs is a filter frozen at authoring time** — correct the day it
|
||||
was written, and unable to report the day it stopped being. `TOOLCHAIN_JOBS` carried a careful
|
||||
argument for why it had to be a literal, and the argument was half right: the failure it named is
|
||||
real, and the conclusion did not follow.
|
||||
|
||||
## The failure a literal was protecting against
|
||||
|
||||
Comparing `container_jobs(doc)` to `pinned_jobs(doc)` and nothing else is blind in the worst
|
||||
direction. Delete a `container:` block and the job leaves BOTH sets together; the comparison stays
|
||||
balanced, reports green, and a required check now runs on the bare runner. A guard whose expected
|
||||
set shrinks in step with the thing it guards is not a guard. The population needs an anchor that
|
||||
does not move — and in 2026-08 the only anchor available was a reviewed list.
|
||||
|
||||
## What the marker changes
|
||||
|
||||
The workflow can hold the anchor itself. `env.CI_EXECUTION_CLASS` is just as independent of the
|
||||
`container:` block as a literal in a Python file is, and it is better placed: a reviewer editing the
|
||||
job reaches it, and it moves with the job when the job is renamed or removed. Not "in the same
|
||||
hunk": measured, the marker sits 6-7 lines from the `container:` block in four jobs and 58 in
|
||||
`migrations`, where the `services:` block sits between them.
|
||||
|
||||
The cost is that same proximity. A literal three directories away survives a careless workflow edit;
|
||||
a marker a handful of lines from the block does not. That is why the scheme is not a marker alone.
|
||||
The third derivation — reading each job's own steps for tools that exist only in the CI image — depends on
|
||||
neither the marker nor the block, so removing the pair together still reddens.
|
||||
|
||||
It is also the only check that sees the defect #789 actually filed:
|
||||
|
||||
> move a .NET-dependent step into `build` or `scan` while leaving that job in `BARE_RUNNER_JOBS`.
|
||||
> Every test stays green while the job now needs the toolchain image and does not have it.
|
||||
|
||||
No set changes, so no set-equality can see it. Only the job's own steps can.
|
||||
|
||||
## Guard JOBS, not only guard files
|
||||
|
||||
`docs/guard-inventory.md` derived its population from guard FILES, so a guard written inline in
|
||||
workflow YAML belonged to no population at all — which is where `pr-checks.yml:ci-image-pin` was
|
||||
sitting when #774 found it stating an invariant it did not check. #786 asked for a reviewed registry
|
||||
cross-checked both ways; `CI_JOB_ROLE` supplies the discovery side and the inventory table is the
|
||||
reviewed side.
|
||||
|
||||
The judgement #774 said the filesystem cannot supply is still a human one. What changed is that it
|
||||
is recorded next to the job, and a job that acquires no marker fails the suite rather than passing
|
||||
unnoticed.
|
||||
|
||||
## The residue
|
||||
|
||||
Both checks prove *classification happened*, never that a classification is *right*. An inline YAML
|
||||
assertion cannot be proven to work without running the job, so such a row carries `Proof: NONE`
|
||||
rather than borrowing credibility from a neighbouring file's proof — unless a pytest genuinely
|
||||
covers part of it, which two rows do, and each says which part. Recorded here because a
|
||||
table that reads as coverage it does not have is the failure this corpus keeps repeating.
|
||||
+105
-9
@@ -47,15 +47,23 @@ exists because arguments of that shape have been wrong here six times.
|
||||
|
||||
## Scope limit, stated rather than implied
|
||||
|
||||
This inventory covers guard **files**, discovered by reading `.claude/hooks/*.sh`, `.husky/*` and
|
||||
`scripts/tests/test_*.py` **out of the git index**, plus every `scripts/…` path referenced by a
|
||||
workflow or a hook. The classes below are outside that population. They are listed because the first
|
||||
version of this section named only the first one, and cold review found that the very guards this
|
||||
inventory shipped with were sitting in the gap:
|
||||
The `## Inventory` table covers guard **files**, discovered by reading `.claude/hooks/*.sh`,
|
||||
`.husky/*` and `scripts/tests/test_*.py` **out of the git index**, plus every `scripts/…` path
|
||||
referenced by a workflow or a hook. Guard **jobs** are covered separately by
|
||||
`## Workflow-job guards` below. The classes below are outside BOTH populations, and are enumerated
|
||||
rather than summarised: an unlisted class reads as covered, and guards this inventory itself shipped
|
||||
with have sat in these gaps. Treat anything not named here as unassessed, not as included:
|
||||
|
||||
1. **Guards inline in workflow YAML** — most importantly `pr-checks.yml:ci-image-pin`. "Which jobs
|
||||
are guards" needs a judgement call per job the filesystem cannot supply. Two were audited under
|
||||
#774 and one fixed; extending the population is tracked in #786.
|
||||
1. ~~**Guards inline in workflow YAML**~~ — **CLOSED by ersatztv#786**, and kept here rather than
|
||||
deleted because the entry named the shape and the shape is worth keeping visible. `pr-checks.yml`'s
|
||||
`ci-image-pin` — which stated an invariant it did not check (#774) — sat in this gap. Workflow jobs
|
||||
now declare `env.CI_JOB_ROLE`, and `scripts/tests/test_workflow_job_guards.py` asserts set equality
|
||||
between the guard-declaring jobs and `## Workflow-job guards` in both directions. The judgement
|
||||
#774 said the filesystem cannot supply is still a human one; what changed is that it is now
|
||||
RECORDED, next to the job, and a job that acquires no marker fails the suite. **Two residuals**:
|
||||
the check proves each job was classified, never that the classification is CORRECT; and an
|
||||
`inline` assertion cannot be proven to work without running the job, so those rows carry
|
||||
`Proof: NONE` honestly instead of borrowing a neighbouring file's proof.
|
||||
2. **C# and TypeScript guards** — `ErsatzTV.Mcp.Tests/ToolCatalogTests.cs`,
|
||||
`web/src/api/pageSizeCallSites.guard.test.ts` and `web/src/api/completeRequest.guard.test.ts`
|
||||
are all structural guards and none has a row. Note what that costs: this list is a HAND-WRITTEN
|
||||
@@ -144,6 +152,7 @@ The audit #806 asked for, recorded whichever way it came out, because "we looked
|
||||
| `test_guard_inventory.py` | `.claude/hooks/*.sh`, `.husky/*`, `scripts/tests/test_*.py`, workflow/hook callers | **yes** — set equality against this table | converted to the index; `.husky/_/` had been excluded only because `_` is a directory, so the obvious "make it recursive" edit would have reintroduced #778's defect here |
|
||||
| `test_hook_fire_log.py` | `.claude/hooks/*.sh` | **yes** — every hook must be instrumented | converted; an untracked scratch `.sh` used to demand instrumentation and redden the suite on that checkout alone |
|
||||
| `test_ci_image_pin_population.py` | `.gitea/workflows/*.yml` + `*.yaml` | **yes** — "docker-build is the ONLY workflow pinning the toolchain image" | converted, and `*.yaml` added: Gitea accepts both spellings, so a `.yaml` workflow was structurally invisible while the test read as covering all of them |
|
||||
| `test_workflow_job_guards.py` | `.gitea/workflows/*.y*ml` | **yes** — every tracked workflow's jobs must each declare a `CI_JOB_ROLE`, and the guard-declaring ones must match `## Workflow-job guards` both ways | index-derived from the start (ersatztv#786); registered in `test_guard_populations_derive_from_git.py` so the shared proof covers it |
|
||||
| `test_pr_changed_files.py` | `.gitea/workflows/*.y*ml` | **yes** — "no OTHER workflow writes the review-verdict status", and since ersatztv#748 "every tracked workflow declares a `permissions:` block", with NO exemption list (the one `ci-image.yml` briefly needed was deleted when #744 landed, per #835) | converted. Not on #806's list and found by cold review on the pushed head: an untracked `.yaml` dropped in `.gitea/workflows/` reddened two guards while absent from the index — the issue's list of files to assess was a starting point, not the population |
|
||||
| `test_workflow_persist_credentials.py` | `.gitea/workflows/*.yml` + `*.yaml` | **yes** — every `actions/checkout` in every workflow must drop the persisted credential | derived from the index from the start (#835). Shipped with NO exemption list, which is why it waited for #744: the one non-compliant checkout would otherwise have bought a permanent entry that keeps passing after its reason expires |
|
||||
| `web/src/api/pageSizeCallSites.guard.test.ts` | `web/src/**/*.{ts,tsx,mts,cts}` via `import.meta.glob` | **yes** — an unregistered discovered site fails | DEFERRED to #819, assessed not skipped. The glob is a documented workaround: `@types/node` is deliberately out of `tsconfig.app.json`, and wiring it in was tried and reverted (it leaked Node's `setTimeout` into the app project and broke three unrelated tests), so there is no `node:child_process` to reach the index from. Over-enumerates, so it fails loudly rather than going blind |
|
||||
@@ -237,8 +246,95 @@ recorded as unexamined rather than as cleared.
|
||||
| `scripts/tests/test_review_verdict_vocabulary.py` | the `script-tests` job, when the H10 verdict vocabulary stops being shared — a word added to the one declaration that fails to reach the write side or the read side | GUARD | MUTATION | `test_review_verdict_vocabulary.py::test_a_word_added_to_the_shared_source_reaches_BOTH_sides` |
|
||||
| `scripts/tests/test_remote_state_inventory.py` | the `script-tests` job, on an executable that talks to a remote service with no row in `docs/remote-state-inventory.md` | GUARD | MUTATION | `test_remote_state_inventory.py::test_every_in_scope_file_has_a_row_and_every_row_names_a_real_file` |
|
||||
| `scripts/tests/test_workflow_persist_credentials.py` | the `script-tests` job, when an `actions/checkout` step no longer sets `persist-credentials: false` | GUARD | MUTATION | `test_workflow_persist_credentials.py::test_every_actions_checkout_DROPS_the_persisted_credential` |
|
||||
| `scripts/tests/test_workflow_job_guards.py` | the `script-tests` job, on a workflow JOB that is a guard with no row in the workflow-job table below, or a job with no `CI_JOB_ROLE` at all | GUARD | MUTATION | `test_workflow_job_guards.py::test_the_inventory_covers_exactly_the_guard_JOBS_that_exist` |
|
||||
| `scripts/tests/test_worktree_ownership_guard.py` | the `script-tests` job | PROOF | NONE | — |
|
||||
|
||||
## Workflow-job guards (ersatztv#786)
|
||||
|
||||
The `## Inventory` table above covers guard **files**. A guard can also be a workflow **job** — its
|
||||
assertion written inline in YAML, or delegated to a script that already holds a file row. Those jobs
|
||||
were outside every population here until #786; `pr-checks.yml:ci-image-pin`, which stated an
|
||||
invariant it did not check (#774), was sitting in exactly that gap.
|
||||
|
||||
**"Which jobs are guards" is a judgement the filesystem cannot supply**, so it is declared where the
|
||||
job is: every job in every tracked workflow carries `env.CI_JOB_ROLE`, one of `guard`, `report-only` or
|
||||
`none`. `report-only` is the third value and needs its own sentence, because "guard vs none" does not
|
||||
imply it: a `report-only` job produces a **verdict it cannot enforce** — every check step carries
|
||||
`continue-on-error: true`, so it reports and never fails. `docs-reminder` is the only one.
|
||||
Note the limit: nothing detects a `guard` job whose checks are all `continue-on-error`, because the
|
||||
checker compares the marker against the ROW, never against the job's ability to fail.
|
||||
|
||||
`scripts/tests/test_workflow_job_guards.py` derives the population from the parsed workflows
|
||||
in the **git index** and asserts set equality against this table in both directions — a new guard job
|
||||
acquires a row before the suite goes green, and a row naming a job that stopped being a guard is
|
||||
reported too.
|
||||
|
||||
**The line is what the job PRODUCES, not whether a step can fail.** A `guard` job's output is a
|
||||
**verdict** — it exists to pass or fail on some condition. A `none` job's output is an **artifact**:
|
||||
an image, a set of PRs. A `none` job can still go red, but that is an ERROR in producing the thing,
|
||||
not a finding about the repo. `docker-build.yml::build` publishes an image and runs a smoke test on
|
||||
it, and is `none`: the smoke test failing means the build did not work, not that an invariant was
|
||||
violated.
|
||||
|
||||
Most jobs are therefore guards, and that is the correct outcome rather than a sign the marker
|
||||
distinguishes nothing: CI here is overwhelmingly checks, and a rule that excluded them would have to
|
||||
draw a line no one could restate. **Which jobs are `none` is read off the markers, not restated
|
||||
here** — a list in prose is a second copy of the workflow that rots on the next job added, and the
|
||||
table below is the only enumeration that a check keeps honest.
|
||||
|
||||
**A rejected alternative, recorded so it is not re-adopted.** Drawing the line at "a guard enforces
|
||||
an invariant about the REPOSITORY, so a job exercising the PRODUCT is `none`" reads as more
|
||||
principled and is wrong: it puts `test` and `migrations` outside the table, and those are the two
|
||||
REQUIRED status contexts on `main` — precisely the jobs where a failure to fire is fail-OPEN against
|
||||
branch protection, and precisely the gap #786 exists to close. `test` also runs the C# and
|
||||
TypeScript structural guards named in scope-limit item 2, so "exercises the product" was never a
|
||||
clean description of it.
|
||||
|
||||
`Assertion` says where the logic actually lives, because that determines what a proof could even
|
||||
look like: an **inline** assertion cannot be unit-tested without running the job, so most carry
|
||||
`NONE` and say so, rather than borrowing credibility from a neighbouring file's proof. Two inline
|
||||
rows do cite a pytest — `ci-image-pin` and `set-verdict-status` — and each names which PART of the
|
||||
job it covers, because a proof reference that covers a fraction must not read as covering the job.
|
||||
|
||||
| Job | Blocks | Kind | Assertion | Proof |
|
||||
| --- | --- | --- | --- | --- |
|
||||
| `dependency-scan.yml::scan` | a weekly/dispatch run, when `dotnet list package --vulnerable` reports a vulnerable direct or transitive package | GUARD | inline — `grep -q "has the following vulnerable packages"` over the restore output | NONE |
|
||||
| `docker-build.yml::toolchain-preflight` | a merge, through the COMBINED commit status the consent gate reads (#598). **Not** the other jobs: nothing `needs:` it and it is not a required context, so it reports rather than gates. It exists because a job that CONSUMED the vanished image could not run to say so (#772) | GUARD | `scripts/ci-toolchain-image-resolves.sh` | that script's row above |
|
||||
| `docker-build.yml::test` | a merge — the REQUIRED context `Build ErsatzTV Image / Build & test (.NET) (pull_request)` | GUARD | the `dotnet test` / `npm test` suites, which include the C# and TypeScript structural guards named in scope-limit item 2 above | `scripts/ci-step-ran.sh` covers a dropped step — every consequential step `mark`s and the final step `assert`s, which is the mechanism `test_ci_dropped_step_guard.py` enforces on this job; the suites carry their own assertions |
|
||||
| `docker-build.yml::migrations` | a merge — the REQUIRED context `Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request)` | GUARD | `dotnet ef migrations has-pending-model-changes` plus apply-to-fresh-DB, per provider | `scripts/ci-step-ran.sh` covers a dropped step — same `mark`/`assert` mechanism, enforced on this job by `test_ci_dropped_step_guard.py` |
|
||||
| `docker-build.yml::functional-e2e` | a PR or a push to `main`, on a broken HTTP contract or UI flow | GUARD | `scripts/e2e-functional.sh`, which holds the HTTP contract assertions and is the only `GUARD` of the three. `scripts/e2e-ui.sh` DRIVES the Playwright specs (the UI assertions live in those specs, not in the script) and `scripts/e2e-local.sh` only BOOTS the instance they run against; both are `TOOLING` above, and naming either as the assertion would credit the coverage to a script that does not provide it | `scripts/e2e-functional.sh`'s row above (`GUARD`, `NONE`); no dropped-step guard — see the decision table below |
|
||||
| `docker-build.yml::scan` | a merge and the release path, when a banned expression delimiter appears in the `run:` body of a delimiter-ban job (`DELIMITER_BAN_JOBS` = the required contexts **plus `build`**, which is wider than the marked set). It carries no `if:`, so it runs on PRs too and a red blocks through the combined status | GUARD | the ban assertion is `test_ci_dropped_step_guard.py::test_the_delimiter_banned_jobs_have_NO_expression_delimiter_in_any_run_body` — note that file is graded `PROOF` above for its `ci-step-ran.sh` role, and also carries this GUARD-shaped assertion; `test_ci_release_path_scan_job.py` (`GUARD`) protects the job that runs it, and `scripts/ci-prove-ban-detects.sh` proves the ban still detects | `scripts/ci-prove-ban-detects.sh`'s row above — which itself carries `NONE`, as do the two pytest files, so this row claims no proof at all |
|
||||
| `docker-build.yml::api-docs` | a PR whose committed OpenAPI spec, endpoint index or SPA client types are stale | GUARD | inline — `git diff --exit-code` after re-running `scripts/update-openapi.sh` | NONE |
|
||||
| `docker-build.yml::format` | a PR whose changed `.cs` files do not conform to `.editorconfig` | GUARD | inline — `dotnet format --verify-no-changes` | NONE |
|
||||
| `pr-checks.yml::ci-image-pin` | a PR whose `ersatztv-ci` pin is not the 7-char sha of the last commit to touch `docker/ci` | GUARD | inline | `test_ci_image_pin_population.py::test_a_single_job_losing_its_pin_is_DETECTED` covers the POPULATION half only; the git-history half has no test |
|
||||
| `pr-checks.yml::docs-reminder` | **nothing.** Both of its check steps — and the `Set up Python` between them — carry `continue-on-error: true`, so no finding it makes can fail the job. (`Checkout` does not, so the job can still go red on infrastructure — never on a finding.) | REPORT-ONLY | inline + `scripts/check-doc-narrative.py` | n/a — it cannot go red |
|
||||
| `pr-checks.yml::decisions-guard` | a PR whose decision records fail lifecycle validation, whose catalog is stale, or whose kickoff file drifted | GUARD | `scripts/decisions_validate.py`, `scripts/build_decisions_catalog.py`, `scripts/check-kickoff-guard.sh` | those scripts' rows above |
|
||||
| `pr-checks.yml::prove-fix` | a PR whose `Proves:` trailer names a test that passes without the fix | GUARD | inline + `scripts/prove-fix.sh` | that script's row above |
|
||||
| `pr-checks.yml::script-tests` | a PR failing ruff or the `scripts/tests` suite — this job is the RUNNER for every `scripts/tests/` row above | GUARD | inline (the ruff population guards) + pytest | the suite it runs |
|
||||
| `review-verdict.yml::set-verdict-status` | a MERGE, by withholding the branch-protection-required `review-verdict/h10` status | GUARD | inline | `scripts/tests/test_pr_changed_files.py` guards its changed-file derivation |
|
||||
|
||||
### The four jobs with no dropped-step guard, decided per job (ersatztv#786)
|
||||
|
||||
`scripts/ci-step-ran.sh` turns a step the runner silently DROPPED into a red job. Its scope is
|
||||
derived from `.gitea/required-status-contexts.json` (#787), so today it is enforced on `test` and
|
||||
`migrations` — the two required Actions contexts, where a drop is fail-OPEN against branch
|
||||
protection. `toolchain-preflight` and `scan` carry the `mark`/`assert` protocol voluntarily without
|
||||
being in the enforced scope. That leaves the jobs below, which #786 listed as unexplained;
|
||||
each is recorded here rather than given a marker reflexively.
|
||||
|
||||
| Job | Decision |
|
||||
| --- | --- |
|
||||
| `functional-e2e` | **No guard.** Not a required context, and it runs only on PRs and pushes to `main`. A dropped step there cannot send a required check green; it costs coverage on a run a human is already reading. |
|
||||
| `api-docs` | **No guard.** Not a required context. Its single assertion is the LAST step, and a dropped `git diff --exit-code` leaves the regeneration steps' output uncommitted — visible in the job log rather than silently green. |
|
||||
| `format` | **No guard.** Not a required context, and the job is three steps: a drop is the difference between a formatting red and no formatting check, on a PR whose other gates still run. |
|
||||
| `build` | **Partial, and the residual is genuine.** Its delimiter class is covered — the `scan` job re-runs the ban test on the release path and is a `needs:` of `build` (#767). Per-step markers would additionally catch a drop caused by something else, and need a bucket modelling the publish-ref `if:` that the guard's always/gated buckets do not express. Stated at `.gitea/workflows/docker-build.yml`'s "Smoke + IPTV E2E" step. |
|
||||
|
||||
Widening `ci-step-ran.sh` beyond the required contexts is deliberately not done here:
|
||||
`test_ci_dropped_step_guard.py` argues the ban is scoped to where a drop is CONSEQUENTIAL, and a
|
||||
marker protocol applied everywhere costs a maintained bucket per job for jobs whose failure mode is
|
||||
"a human reads a shorter log".
|
||||
|
||||
|
||||
## The `MUTATION` column is executed, not asserted
|
||||
|
||||
Every row graded `MUTATION` carries a DECLARED clause mutation in `scripts/tests/mutation_manifest.py`.
|
||||
@@ -259,7 +355,7 @@ clause-provable and the entry is regraded.
|
||||
|
||||
## What the numbers say
|
||||
|
||||
43 guards, 6 tooling scripts, 21 proof files. **21 guards carry a mutation proof; 6 are
|
||||
44 guards, 6 tooling scripts, 21 proof files. **22 guards carry a mutation proof; 6 are
|
||||
behaviour-only; 16 have none.** These figures are asserted against the table by
|
||||
`test_the_summary_counts_match_the_table` — they were wrong in the first draft (28/4/6/3/19 against
|
||||
a table holding 27/5/6/3/18), because a hand-maintained summary of a table is a second copy of it,
|
||||
|
||||
@@ -149,15 +149,17 @@ MUTATIONS: tuple[Mutation, ...] = (
|
||||
Mutation(
|
||||
guard="scripts/tests/test_ci_image_pin_population.py",
|
||||
target="scripts/tests/test_ci_image_pin_population.py",
|
||||
clause="for name in sorted(TOOLCHAIN_JOBS - set(pinned)):",
|
||||
clause="for name in sorted(expected - set(pinned)):",
|
||||
replacement="for name in sorted(set()):",
|
||||
proof="test_ci_image_pin_population.py::test_a_single_job_losing_its_pin_is_DETECTED",
|
||||
granularity=CLAUSE,
|
||||
expect="the population check accepted a workflow in which a container job no longer runs",
|
||||
why="The against-the-registry direction, and the one the other two clauses cannot cover: a job "
|
||||
"that loses its `container:` block leaves `declared` and `pinned` equal, so only this "
|
||||
"comparison notices it has moved to the bare runner. This is the clause #790 asked for "
|
||||
"instead of neutering `pin_population_faults` wholesale.",
|
||||
why="The against-the-DECLARED-CLASS direction, and the one the other two clauses cannot "
|
||||
"cover: a job that loses its `container:` block leaves `declared` and `pinned` equal, so "
|
||||
"only this comparison notices it has moved to the bare runner. This is the clause #790 "
|
||||
"asked for instead of neutering `pin_population_faults` wholesale. `expected` is "
|
||||
"`toolchain_declared(doc)` since ersatztv#789 replaced the `TOOLCHAIN_JOBS` literal this "
|
||||
"clause used to name; the comparison and its role are unchanged.",
|
||||
),
|
||||
Mutation(
|
||||
guard="scripts/tests/test_guard_populations_derive_from_git.py",
|
||||
@@ -197,6 +199,22 @@ MUTATIONS: tuple[Mutation, ...] = (
|
||||
"hand-written table omitted, because `...Model` reads as a response model while it is in "
|
||||
"fact reachable from the full-replace PUT /channels/{id}.",
|
||||
),
|
||||
Mutation(
|
||||
guard="scripts/tests/test_workflow_job_guards.py",
|
||||
target="docs/guard-inventory.md",
|
||||
clause="| `pr-checks.yml::ci-image-pin` |",
|
||||
replacement="| `pr-checks.yml::ci-image-pin-RENAMED` |",
|
||||
proof="test_workflow_job_guards.py::test_the_inventory_covers_exactly_the_guard_JOBS_that_exist",
|
||||
granularity=CLAUSE,
|
||||
expect="guard/report-only but have NO row in",
|
||||
why="THE GUARD IS A TEST, so the mutation goes into the guarded ARTIFACT — the workflow-job "
|
||||
"table — rather than into the checker, per the checker-guard exception. RENAMED rather than "
|
||||
"deleted, so the row count is unchanged and the red cannot come from an empty table: the "
|
||||
"anti-vacuity test still sees rows, and what fails is the set equality itself, in BOTH "
|
||||
"directions at once (MISSING for the real job, PHANTOM for the renamed key). "
|
||||
"`ci-image-pin` is the right row to name — it is the guard job #774 found stating an "
|
||||
"invariant it did not check, and the case #786 was filed to bring into a population.",
|
||||
),
|
||||
Mutation(
|
||||
guard="scripts/tests/test_guard_inventory.py",
|
||||
target="docs/guard-inventory.md",
|
||||
|
||||
@@ -64,30 +64,108 @@ _PIN = re.compile(rf"^{re.escape(IMAGE_REPO)}:([0-9a-f]+)$")
|
||||
|
||||
_DOC = yaml.safe_load(WORKFLOW.read_text())
|
||||
|
||||
# THE HAND-REVIEWED REGISTRY of jobs that must run inside the CI toolchain image, cross-checked in
|
||||
# BOTH directions against what the workflow actually declares. This is deliberately a literal, and
|
||||
# the reason is the one case set equality between two DERIVED sets cannot cover.
|
||||
# THE EXECUTION CLASS IS DECLARED BY THE WORKFLOW, one marker per job, and every set below is
|
||||
# derived from it (ersatztv#789). What stood here before was `TOOLCHAIN_JOBS` /
|
||||
# `BARE_RUNNER_JOBS` — two hand-written literals enumerating jobs by name.
|
||||
#
|
||||
# WHY A LITERAL WAS THERE AT ALL, because the reason is real and the replacement has to answer it.
|
||||
# The first version of this file compared `container_jobs(doc)` against `pinned_jobs(doc)` and
|
||||
# nothing else. That is blind to the mutation that matters most: delete a `container:` block and the
|
||||
# job leaves BOTH sets together, so the comparison stays balanced and reports green — which is the
|
||||
# very fail-open this file was written to close, reproduced one level up. A guard whose expected set
|
||||
# shrinks in step with the thing it is guarding is not a guard.
|
||||
# job leaves BOTH sets together, so the comparison stays balanced and reports green — the very
|
||||
# fail-open this file exists to close, reproduced one level up. A guard whose expected set shrinks
|
||||
# in step with the thing it guards is not a guard. So the population needs an ANCHOR that does not
|
||||
# move when the `container:` block does.
|
||||
#
|
||||
# So the population needs one anchor that does NOT move when the workflow does, and a reviewed list
|
||||
# is the only available one: nothing else in the repo records which jobs are supposed to need the
|
||||
# toolchain. This is the same shape as `web/src/api/pageSizeCallSites.guard.test.ts` — discovery on
|
||||
# one side, a reviewed registry on the other, compared both ways — and it is the SCOPE half of
|
||||
# `testing.guard-derives-population-from-source`, not an exception to it. Editing this set is a
|
||||
# reviewable act; a job silently losing its container block is not.
|
||||
TOOLCHAIN_JOBS = frozenset({"test", "migrations", "functional-e2e", "api-docs", "format"})
|
||||
# `env.CI_EXECUTION_CLASS` is that anchor, and it is a strictly better one than the literal: it is
|
||||
# still an independent declaration (deleting a `container:` block leaves the marker behind, so the
|
||||
# job stays in the expected set and the comparison goes red), but it lives IN THE JOB it describes
|
||||
# instead of in a Python file three directories away — so it is reachable by anyone reading that job,
|
||||
# and it moves with the job when the job is renamed or removed.
|
||||
#
|
||||
# NOT "in the same diff hunk", which an earlier version of this note claimed. MEASURED distance from
|
||||
# each `container:` block to its marker: test 6 lines, functional-e2e 6, api-docs 7, format 7 —
|
||||
# and migrations 58, because the whole `services:` block sits between them. A container-only edit to
|
||||
# `migrations` shows no marker in its hunk at all. Adjacency is a convenience that mostly holds; the
|
||||
# property the guard actually rests on is INDEPENDENCE — the marker does not move when the
|
||||
# `container:` block does.
|
||||
#
|
||||
# THE COST, stated rather than left for review to find: proximity cuts both ways. The literal sat in
|
||||
# a file a careless workflow edit would not touch; the marker sits a handful of lines from the
|
||||
# `container:` block in four of the five jobs, so deleting both together is one plausible slip
|
||||
# rather than two deliberate acts. That is
|
||||
# why this file does not stop at the marker. `test_no_job_needs_the_toolchain_without_declaring_it`
|
||||
# below derives the requirement a THIRD time — from the job's own steps — and it is independent of
|
||||
# both the marker and the container block, so removing the pair still goes red. It is also the check
|
||||
# that answers #789's concrete complaint, which neither the literal nor the marker could: move a
|
||||
# .NET-dependent step into `build` or `scan` and every set-equality here stays balanced while the
|
||||
# job now needs a toolchain it does not have.
|
||||
EXECUTION_CLASS_KEY = "CI_EXECUTION_CLASS"
|
||||
TOOLCHAIN = "toolchain"
|
||||
BARE_RUNNER = "bare-runner"
|
||||
# A VOCABULARY, not a population: these are the two legal values of the marker, not a list of jobs.
|
||||
# `testing.guard-derives-population-from-source` draws exactly that line — the members are derived,
|
||||
# the value set they are drawn from is a reviewed policy choice.
|
||||
EXECUTION_CLASSES = frozenset({TOOLCHAIN, BARE_RUNNER})
|
||||
|
||||
# `scan`, `build` and `toolchain-preflight` deliberately run on the bare runner: `scan` is
|
||||
# `runs-on: small` and needs only python, `build` drives docker/buildx on the host, and
|
||||
# `toolchain-preflight` exists to report that the pinned toolchain image is GONE — a job that
|
||||
# consumed that image could not run to say so (ersatztv#772). Listed here so their ABSENCE above
|
||||
# reads as a decision rather than an oversight.
|
||||
BARE_RUNNER_JOBS = frozenset({"scan", "build", "toolchain-preflight"})
|
||||
# Tools present ONLY in the CI toolchain image, used to derive a job's toolchain requirement from
|
||||
# its own steps. Drawn from docker/ci/Dockerfile: the image adds the .NET SDK, the two dotnet global
|
||||
# tools, Node/npm and Playwright on top of the ffmpeg base. Deliberately EXCLUDED are `git`, `jq`,
|
||||
# `python3`, `curl` and `tar`. The Dockerfile installs them, but they do not DISCRIMINATE: the
|
||||
# bare-runner lanes provide or provision them too — `small` is git-only and adds Python via
|
||||
# `actions/setup-python` where a job needs it, and `build` runs on stock `ubuntu-latest`. So a match
|
||||
# on one says nothing about whether the job needs the toolchain image. MEASURED, not assumed: those
|
||||
# tokens flag 4 of the 8 jobs, so "would flag every job" would have been the wrong reason and the
|
||||
# wrong number.
|
||||
TOOLCHAIN_ONLY_TOOLS = (
|
||||
"dotnet",
|
||||
"dotnet-ef",
|
||||
"node",
|
||||
"npm",
|
||||
"npx",
|
||||
"ffmpeg",
|
||||
"ffprobe",
|
||||
"playwright",
|
||||
"reportgenerator",
|
||||
)
|
||||
# LONGEST FIRST. Python alternation is first-match, not longest-match, so a bare `dotnet` listed
|
||||
# ahead of `dotnet-ef` would match the prefix, fail the trailing `(?![\w.-])` on the hyphen, and
|
||||
# only then backtrack. Sorting removes the dependence on that backtrack rather than relying on it.
|
||||
_TOOL_ALTERNATION = "|".join(sorted((re.escape(t) for t in TOOLCHAIN_ONLY_TOOLS), key=len, reverse=True))
|
||||
# Command position: not part of a longer word and not the tail of a path. `(?<![\w./-])` keeps
|
||||
# `/usr/share/dotnet/dotnet` from matching, and `(?![\w.-])` keeps `nodejs` and `npm-cache` out.
|
||||
_TOOL_USE = re.compile(rf"(?<![\w./-])({_TOOL_ALTERNATION})(?![\w.-])")
|
||||
|
||||
# Full-line shell comments, stripped before matching. WHAT THIS IS AND IS NOT, because the obvious
|
||||
# reading overstates it: it is PREVENTIVE, not a fix for a live red. Measured on the current tree,
|
||||
# NO toolchain token appears anywhere in the `run:` bodies of the three bare-runner jobs, comment
|
||||
# lines included, so the stripping does not change today's verdict for any job. What it removes is
|
||||
# the cheapest way to acquire a spurious red later — a bare-runner job whose comment merely MENTIONS
|
||||
# dotnet — and "a correct guard going red on a correct tree is the fastest route to that guard being
|
||||
# deleted" is this repo's own finding. The substitution is safe in the other direction too: `.sub`
|
||||
# on a `^…$` match leaves the newline, so no two lines are ever joined and no NEW match can be
|
||||
# manufactured.
|
||||
#
|
||||
# ONLY full-line comments, and the FULL residue is enumerated rather than papered over, because a
|
||||
# predicate described as tight stops being re-examined. It is wrong in both directions.
|
||||
#
|
||||
# FALSE POSITIVES (spurious red on a correct tree) — a TRAILING `# …` comment, an
|
||||
# `echo "run dotnet build"`, and a heredoc BODY, since stripping is line-based and not shell-aware.
|
||||
# FALSE NEGATIVES (a real need, unseen) — an absolute path (`/usr/share/dotnet/dotnet build`); a
|
||||
# tool reached only through a script; a `#`-prefixed line inside a heredoc that a later command
|
||||
# strips and executes, which is the converse of the heredoc false positive; and indirection
|
||||
# through a variable — but MEASURED, and only in one of its two forms: `$SDK_CMD build` is
|
||||
# invisible when the assignment comes from a step/job `env:` or an earlier step, while
|
||||
# `SDK_CMD=dotnet` written in the SAME body is caught, because the assignment line itself carries
|
||||
# the bare token.
|
||||
#
|
||||
# Parsing those correctly means parsing shell, and `testing.fixing-a-parser-bug-introduces-the-next-one`
|
||||
# prices a string predicate over source at three or more rounds. It is not attempted. The residue is
|
||||
# tolerable because of its DIRECTION at each end. A false positive is a LOUD red naming the job and
|
||||
# the token, with an obvious remedy (reword the line). A false negative costs a detection this check
|
||||
# never promised — it is a NECESSARY condition, and the two set comparisons above are the primary
|
||||
# guards. What none of the three can do is see a job that drops its container block, flips its
|
||||
# marker AND hides its tools at once; that is the disclosed hole, not this list.
|
||||
_FULL_LINE_COMMENT = re.compile(r"^[ \t]*#.*$", re.M)
|
||||
|
||||
|
||||
def _jobs(doc) -> dict:
|
||||
@@ -117,6 +195,108 @@ def pinned_jobs(doc) -> dict[str, str]:
|
||||
return out
|
||||
|
||||
|
||||
def declared_classes(doc) -> dict[str, str | None]:
|
||||
"""job -> the execution class it DECLARES, or None where the marker is absent.
|
||||
|
||||
`None` is kept rather than dropped, because a job with no marker is the defect
|
||||
`class_declaration_faults` reports and a dict that silently omitted it could not.
|
||||
"""
|
||||
out: dict[str, str | None] = {}
|
||||
for name, job in _jobs(doc).items():
|
||||
if not isinstance(job, dict):
|
||||
continue
|
||||
env = job.get("env") or {}
|
||||
value = env.get(EXECUTION_CLASS_KEY)
|
||||
out[name] = str(value) if value is not None else None
|
||||
return out
|
||||
|
||||
|
||||
def class_declaration_faults(doc) -> list[str]:
|
||||
"""MISSING OR UNKNOWN IS A HARD FAILURE, which is the half of #789 that is easy to skip.
|
||||
|
||||
A marker scheme whose absent value defaults to anything at all is a scheme that stops applying
|
||||
the moment someone adds a job and forgets — the failure mode is silence, and it arrives on the
|
||||
one job nobody reviewed. So an unmarked job is a fault here, and an unrecognised value is a
|
||||
fault too: `CI_EXECUTION_CLASS: container` would otherwise read as 'not toolchain' and move the
|
||||
job to the bare-runner side of every comparison below without anyone deciding that.
|
||||
"""
|
||||
faults = []
|
||||
for name, value in sorted(declared_classes(doc).items()):
|
||||
if value is None:
|
||||
faults.append(
|
||||
f"job '{name}' declares no {EXECUTION_CLASS_KEY}. Every job must declare one under "
|
||||
f"its `env:` — {sorted(EXECUTION_CLASSES)} — so that 'runs on the bare runner' is a "
|
||||
"recorded decision rather than an omission."
|
||||
)
|
||||
elif value not in EXECUTION_CLASSES:
|
||||
faults.append(
|
||||
f"job '{name}' declares {EXECUTION_CLASS_KEY}: {value!r}, which is not one of "
|
||||
f"{sorted(EXECUTION_CLASSES)}. An unrecognised value is rejected rather than treated "
|
||||
"as bare-runner, because the silent reading would move the job out of the pin check."
|
||||
)
|
||||
return faults
|
||||
|
||||
|
||||
def toolchain_declared(doc) -> set[str]:
|
||||
"""The jobs DECLARING that they run in the toolchain image. The expected set, derived."""
|
||||
return {n for n, v in declared_classes(doc).items() if v == TOOLCHAIN}
|
||||
|
||||
|
||||
def _step_bodies(job) -> list[str]:
|
||||
"""Each step's `run:` body with full-line comments removed. See `_FULL_LINE_COMMENT`."""
|
||||
return [
|
||||
_FULL_LINE_COMMENT.sub("", s["run"])
|
||||
for s in (job.get("steps") or [])
|
||||
if isinstance(s, dict) and isinstance(s.get("run"), str)
|
||||
]
|
||||
|
||||
|
||||
def jobs_whose_steps_need_the_toolchain(doc) -> dict[str, set[str]]:
|
||||
"""job -> the toolchain-only tools its OWN step bodies invoke. A THIRD, independent derivation.
|
||||
|
||||
Independent of both the marker and the `container:` block, which is the point: it is what still
|
||||
reddens when a careless edit removes that adjacent pair together, and it is the only one of the
|
||||
three that can see #789's concrete failure — a .NET-dependent step MOVED into a bare-runner job,
|
||||
where every set-equality stays balanced because no set changed.
|
||||
|
||||
THE FILTER HERE IS LEGITIMATE and the distinction matters, because this file's own rule bans
|
||||
filtering a population. `testing.guard-derives-population-from-source` draws the line at what the
|
||||
claim is over: filtering to select the SUBJECT of a PER-MEMBER property is fine, because the
|
||||
excluded members satisfy it vacuously. The claim below is per-job — 'a job that visibly invokes a
|
||||
toolchain tool declares the toolchain class' — and a job invoking none satisfies it with nothing
|
||||
to check. It is NOT a completeness claim over jobs; the completeness claims are
|
||||
`class_declaration_faults` (every job declares) and `pin_population_faults` (both directions).
|
||||
|
||||
ONE-DIRECTIONAL ON PURPOSE, and this is the limit to state rather than let review find. Step
|
||||
text is a NECESSARY condition, never a sufficient one: a job whose ONLY toolchain use sat behind
|
||||
a script would be invisible here. MEASURED, because the first version of this note asserted the
|
||||
blind spot was live and it is not — all five declared toolchain jobs are detected directly,
|
||||
`functional-e2e` included, which runs `dotnet restore` and `npm ci` in its own steps before
|
||||
handing off to `scripts/e2e-local.sh`. The blind-spot set is EMPTY today. The converse —
|
||||
'declared toolchain, so some step must name a tool' — is still not asserted, because a future
|
||||
job could be written that way and a red on a correct tree is how a guard gets deleted.
|
||||
|
||||
THE RESIDUAL THIS LEAVES, which is the one that matters and is not the same sentence. The three
|
||||
checks fail together under one plausible edit: drop the `container:` block, flip the marker to
|
||||
bare-runner, AND move the tool invocation into a script. Then the two set comparisons stay
|
||||
balanced (nothing is in either set) and this check sees no token, so nothing reddens. That is a
|
||||
real hole and it is stated rather than argued away. What bounds it is the failure MODE, not the
|
||||
probability: the job then runs a missing binary and dies with `dotnet: command not found`, which
|
||||
is loud and immediate — unlike ersatztv#774's original defect, where a required check WOULD go
|
||||
green on the bare runner having silently skipped the toolchain. (Subjunctive on purpose: that
|
||||
hole was found and closed by inspection, and no run of it was ever observed.) Trading a silent pass for a noisy
|
||||
crash is the point of the change; it is not a claim that the hole is closed.
|
||||
"""
|
||||
out = {}
|
||||
for name, job in _jobs(doc).items():
|
||||
if not isinstance(job, dict):
|
||||
continue
|
||||
found = {m for body in _step_bodies(job) for m in _TOOL_USE.findall(body)}
|
||||
if found:
|
||||
out[name] = found
|
||||
return out
|
||||
|
||||
|
||||
def pin_population_faults(doc) -> list[str]:
|
||||
"""Set equality in BOTH directions, plus tag agreement. Accumulated, never fail-fast.
|
||||
|
||||
@@ -127,19 +307,21 @@ def pin_population_faults(doc) -> list[str]:
|
||||
"""
|
||||
declared = container_jobs(doc)
|
||||
pinned = pinned_jobs(doc)
|
||||
expected = toolchain_declared(doc)
|
||||
|
||||
faults = []
|
||||
# AGAINST THE REGISTRY FIRST. This is the direction the two derived sets cannot cover: a job
|
||||
# that loses its `container:` block leaves `declared` and `pinned` together, so their equality
|
||||
# survives untouched while the job quietly moves to the bare runner.
|
||||
for name in sorted(TOOLCHAIN_JOBS - set(pinned)):
|
||||
# AGAINST THE DECLARED CLASS FIRST. This is the direction the two derived sets cannot cover: a
|
||||
# job that loses its `container:` block leaves `declared` and `pinned` together, so their
|
||||
# equality survives untouched while the job quietly moves to the bare runner. The marker does
|
||||
# not move with the block, so this comparison still notices.
|
||||
for name in sorted(expected - set(pinned)):
|
||||
faults.append(
|
||||
f"job '{name}' is registered as needing the CI toolchain image but does not pin it "
|
||||
f"job '{name}' declares {EXECUTION_CLASS_KEY}: {TOOLCHAIN} but does not pin the image "
|
||||
"(its container: block is missing or points elsewhere) — it is running on the bare "
|
||||
"runner"
|
||||
)
|
||||
for name in sorted(set(pinned) - TOOLCHAIN_JOBS):
|
||||
faults.append(f"job '{name}' pins the toolchain image but is not in TOOLCHAIN_JOBS")
|
||||
for name in sorted(set(pinned) - expected):
|
||||
faults.append(f"job '{name}' pins the toolchain image but does not declare {EXECUTION_CLASS_KEY}: {TOOLCHAIN}")
|
||||
|
||||
for name in sorted(declared - set(pinned)):
|
||||
image = str((_jobs(doc)[name].get("container") or {}).get("image", ""))
|
||||
@@ -175,48 +357,69 @@ def test_every_container_job_pins_the_CI_toolchain_image():
|
||||
)
|
||||
|
||||
|
||||
def test_the_registry_and_the_workflow_agree_on_which_jobs_use_the_toolchain():
|
||||
"""BOTH directions against the reviewed registry — the anchor that does not move.
|
||||
def test_the_declared_class_and_the_workflow_agree_on_which_jobs_use_the_toolchain():
|
||||
"""BOTH directions against the DECLARED class — the anchor that does not move (ersatztv#789).
|
||||
|
||||
Left-to-right catches a job silently LOSING its container block (the mutation set equality
|
||||
between two derived sets cannot see, because both sides shrink together). Right-to-left catches
|
||||
a NEW container job nobody registered. Neither direction is optional and the messages differ,
|
||||
because the two are opposite mistakes.
|
||||
a NEW container job that declared itself bare-runner. Neither direction is optional and the
|
||||
messages differ, because the two are opposite mistakes.
|
||||
"""
|
||||
declared = container_jobs(_DOC)
|
||||
assert TOOLCHAIN_JOBS - declared == frozenset(), (
|
||||
f"these jobs are registered as needing the CI toolchain image but no longer declare a "
|
||||
f"container: block — {sorted(TOOLCHAIN_JOBS - declared)}. They are now running on the bare "
|
||||
"runner. If that is deliberate, move them to BARE_RUNNER_JOBS in this file and say why in "
|
||||
"the PR."
|
||||
expected = toolchain_declared(_DOC)
|
||||
assert expected - declared == set(), (
|
||||
f"these jobs declare {EXECUTION_CLASS_KEY}: {TOOLCHAIN} but no longer declare a container: "
|
||||
f"block — {sorted(expected - declared)}. They are now running on the bare runner. If that "
|
||||
f"is deliberate, change the marker to {BARE_RUNNER} in the workflow and say why in the PR."
|
||||
)
|
||||
assert declared - TOOLCHAIN_JOBS == frozenset(), (
|
||||
f"these jobs declare a container: but are not in TOOLCHAIN_JOBS — "
|
||||
f"{sorted(declared - TOOLCHAIN_JOBS)}. Add them to the registry so the pin guard covers "
|
||||
"them, or they will run on an image nothing checks."
|
||||
assert declared - expected == set(), (
|
||||
f"these jobs declare a container: but their {EXECUTION_CLASS_KEY} is not {TOOLCHAIN} — "
|
||||
f"{sorted(declared - expected)}. Fix the marker, or they will run on an image nothing checks."
|
||||
)
|
||||
|
||||
|
||||
def test_the_registry_partitions_every_job_in_the_workflow():
|
||||
"""No job may be in neither list. ANTI-VACUITY with teeth, and the reason it is here:
|
||||
def test_no_job_needs_the_toolchain_without_declaring_it():
|
||||
"""THE THIRD DERIVATION, and the only one that can see a step MOVED into a bare-runner job.
|
||||
|
||||
a floor like `len(container_jobs) >= 3` would be satisfied by a broken parse that happened to
|
||||
find four jobs, and would say nothing about a NEW job appearing in a third state nobody
|
||||
considered. Partitioning the whole `jobs:` map means every job is a decision someone recorded.
|
||||
This is #789's concrete complaint: "move a .NET-dependent step into `build` or `scan` while
|
||||
leaving that job in `BARE_RUNNER_JOBS`. Every test stays green while the job now needs the
|
||||
toolchain image and does not have it." Nothing above can see it — no set changes, so every
|
||||
equality stays balanced. Only reading the job's own steps can.
|
||||
|
||||
Necessary-condition only; `jobs_whose_steps_need_the_toolchain` states why the converse is not
|
||||
asserted.
|
||||
"""
|
||||
all_jobs = set(_jobs(_DOC))
|
||||
unclassified = all_jobs - TOOLCHAIN_JOBS - BARE_RUNNER_JOBS
|
||||
assert not unclassified, (
|
||||
f"docker-build.yml has job(s) {sorted(unclassified)} that are in neither TOOLCHAIN_JOBS nor "
|
||||
"BARE_RUNNER_JOBS. Every job must be one or the other, so that 'runs on the bare runner' is "
|
||||
"always a recorded decision rather than an omission."
|
||||
)
|
||||
assert not (TOOLCHAIN_JOBS & BARE_RUNNER_JOBS), "a job cannot be in both lists"
|
||||
assert all_jobs == TOOLCHAIN_JOBS | BARE_RUNNER_JOBS, (
|
||||
f"the registry names jobs that do not exist: {sorted((TOOLCHAIN_JOBS | BARE_RUNNER_JOBS) - all_jobs)}"
|
||||
needed = jobs_whose_steps_need_the_toolchain(_DOC)
|
||||
expected = toolchain_declared(_DOC)
|
||||
faults = [
|
||||
f"job '{name}' invokes {sorted(tools)} in its own steps but declares "
|
||||
f"{EXECUTION_CLASS_KEY}: {declared_classes(_DOC).get(name)}"
|
||||
for name, tools in sorted(needed.items())
|
||||
if name not in expected
|
||||
]
|
||||
assert not faults, (
|
||||
"these jobs run a tool that exists ONLY in the CI toolchain image, on the bare runner:\n "
|
||||
+ "\n ".join(faults)
|
||||
+ f"\n\nEither declare {EXECUTION_CLASS_KEY}: {TOOLCHAIN} and add the container: block, or "
|
||||
"move the step back. Neither bare-runner lane provides the CI image's toolchain: `small` "
|
||||
"is git-only (Python is added by actions/setup-python where a job needs it) and `build` "
|
||||
"runs on a stock ubuntu-latest."
|
||||
)
|
||||
|
||||
|
||||
def test_every_job_declares_a_known_execution_class():
|
||||
"""ANTI-VACUITY with teeth: no job may be unclassified, and no value may be improvised.
|
||||
|
||||
A floor like `len(container_jobs) >= 3` would be satisfied by a broken parse that happened to
|
||||
find four jobs, and would say nothing about a NEW job appearing in a third state nobody
|
||||
considered. Requiring a marker on every job means every job is a decision someone recorded — the
|
||||
property the old `TOOLCHAIN_JOBS | BARE_RUNNER_JOBS` partition provided, now held without either
|
||||
literal.
|
||||
"""
|
||||
faults = class_declaration_faults(_DOC)
|
||||
assert not faults, "docker-build.yml has jobs with no usable execution class:\n " + "\n ".join(faults)
|
||||
|
||||
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
# MUTATION PROOFS — disarm the invariant one way at a time, each must be DETECTED (ersatztv#775)
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
@@ -246,6 +449,12 @@ def _mutants():
|
||||
skewed["jobs"][job]["container"]["image"] = f"{IMAGE_REPO}:deadbee"
|
||||
yield f"{job}-tag-skewed", skewed
|
||||
|
||||
# The marker flipped while the container: block stays. The mutation the LITERAL registry
|
||||
# could also catch, kept so replacing it with a declaration demonstrably lost nothing.
|
||||
misdeclared = copy.deepcopy(_DOC)
|
||||
misdeclared["jobs"][job].setdefault("env", {})[EXECUTION_CLASS_KEY] = BARE_RUNNER
|
||||
yield f"{job}-declared-bare-runner", misdeclared
|
||||
|
||||
|
||||
_MUTANTS = list(_mutants())
|
||||
|
||||
@@ -268,11 +477,15 @@ def test_the_mutation_set_is_not_empty():
|
||||
nothing. That is the vacuous-by-sampling shape this whole issue is about, and it is reachable
|
||||
here through a single broken helper.
|
||||
"""
|
||||
expected = 4 * len(TOOLCHAIN_JOBS)
|
||||
expected = 5 * len(toolchain_declared(_DOC))
|
||||
assert len(_MUTANTS) == expected, (
|
||||
f"expected 4 mutations per registered job ({expected}), got {len(_MUTANTS)}. A floor rather "
|
||||
"than an equality here would let a `container_jobs()` that degraded to 3 of 5 jobs pass "
|
||||
"while silently testing less — the message would still claim 4 per job."
|
||||
f"expected 5 mutations per declared toolchain job ({expected}), got {len(_MUTANTS)}. A floor "
|
||||
"rather than an equality here would let a `container_jobs()` that degraded to 3 of 5 jobs "
|
||||
"pass while silently testing less — the message would still claim 5 per job."
|
||||
)
|
||||
assert toolchain_declared(_DOC), (
|
||||
"no job declares the toolchain class, so every mutation above is vacuous. Either the marker "
|
||||
"key changed or the parse broke."
|
||||
)
|
||||
|
||||
|
||||
@@ -288,8 +501,10 @@ def test_docker_build_is_the_ONLY_workflow_pinning_the_toolchain_image():
|
||||
|
||||
Found by cold review, which correctly noted this file criticises `MARKED_JOBS` for exactly this
|
||||
and then shipped the same shape without even the dated comment `MARKED_JOBS` then carried.
|
||||
`MARKED_JOBS` has since been derived (#787); `TOOLCHAIN_JOBS` below is the remaining literal and
|
||||
is tracked in #789.
|
||||
`MARKED_JOBS` has since been derived (#787), and `TOOLCHAIN_JOBS`/`BARE_RUNNER_JOBS` — the
|
||||
literals this file once carried — were replaced by the per-job `env.CI_EXECUTION_CLASS` marker
|
||||
in #789. This scope check is what still keeps `WORKFLOW` honest: the marker says which class a
|
||||
job is in, never which FILE the guard reads.
|
||||
|
||||
The population comes from the GIT INDEX (ersatztv#806). A `Path.glob` here answered a question
|
||||
about the machine rather than about the repo: an untracked scratch workflow left in
|
||||
@@ -353,3 +568,82 @@ def test_the_shell_guards_grep_sees_the_same_tags_the_jobs_run():
|
||||
"header comment) makes the shell guard's 'MORE THAN ONE pin' check fire on prose; a tag "
|
||||
"run but not greppable means the shell guard is not checking that job at all."
|
||||
)
|
||||
|
||||
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
# MUTATION PROOFS for the two checks the pin comparison cannot make (ersatztv#789)
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _declaration_mutants():
|
||||
"""Every job in turn loses or corrupts its marker. Both must be reported, not defaulted."""
|
||||
for job in sorted(_jobs(_DOC)):
|
||||
removed = copy.deepcopy(_DOC)
|
||||
(removed["jobs"][job].get("env") or {}).pop(EXECUTION_CLASS_KEY, None)
|
||||
yield f"{job}-marker-removed", removed
|
||||
|
||||
unknown = copy.deepcopy(_DOC)
|
||||
unknown["jobs"][job].setdefault("env", {})[EXECUTION_CLASS_KEY] = "container"
|
||||
yield f"{job}-marker-unknown-value", unknown
|
||||
|
||||
|
||||
_DECLARATION_MUTANTS = list(_declaration_mutants())
|
||||
|
||||
|
||||
@pytest.mark.parametrize("doc", [m for _, m in _DECLARATION_MUTANTS], ids=[i for i, _ in _DECLARATION_MUTANTS])
|
||||
def test_a_missing_or_UNKNOWN_execution_class_is_DETECTED(doc):
|
||||
"""Proves the hard-failure half of #789 is real rather than declared.
|
||||
|
||||
A marker scheme is worth nothing if an absent marker reads as a default: the job nobody
|
||||
remembered to mark is exactly the job nobody reviewed.
|
||||
"""
|
||||
assert class_declaration_faults(doc), "a job with no execution class, or an unrecognised one, was accepted"
|
||||
|
||||
|
||||
def test_the_declaration_mutation_set_is_not_empty():
|
||||
"""NON-EMPTINESS first: `2N == 2N` holds at N=0 and would pass over an empty parametrisation."""
|
||||
assert _jobs(_DOC), (
|
||||
"the workflow parsed to ZERO jobs, so every declaration mutation is vacuous and the "
|
||||
"parametrised proof above collected nothing."
|
||||
)
|
||||
expected = 2 * len(_jobs(_DOC))
|
||||
assert len(_DECLARATION_MUTANTS) == expected, (
|
||||
f"expected 2 declaration mutations per job ({expected}), got {len(_DECLARATION_MUTANTS)}"
|
||||
)
|
||||
|
||||
|
||||
def _requirement_mutants():
|
||||
"""A toolchain-only step MOVED into each bare-runner job in turn — #789's concrete failure.
|
||||
|
||||
Appended as a real step rather than by editing the marker, because the whole point is that this
|
||||
defect changes NO set: the job keeps its marker, keeps its lack of a container block, and every
|
||||
equality above stays balanced.
|
||||
"""
|
||||
bare = sorted(n for n, v in declared_classes(_DOC).items() if v == BARE_RUNNER)
|
||||
for job in bare:
|
||||
moved = copy.deepcopy(_DOC)
|
||||
moved["jobs"][job].setdefault("steps", []).append(
|
||||
{"name": "moved here by the mutation harness", "run": "dotnet build ErsatzTV.sln"}
|
||||
)
|
||||
yield f"{job}-gains-a-dotnet-step", moved
|
||||
|
||||
|
||||
_REQUIREMENT_MUTANTS = list(_requirement_mutants())
|
||||
|
||||
|
||||
@pytest.mark.parametrize("doc", [m for _, m in _REQUIREMENT_MUTANTS], ids=[i for i, _ in _REQUIREMENT_MUTANTS])
|
||||
def test_a_toolchain_step_MOVED_into_a_bare_runner_job_is_DETECTED(doc):
|
||||
needed = jobs_whose_steps_need_the_toolchain(doc)
|
||||
expected = toolchain_declared(doc)
|
||||
assert [n for n in needed if n not in expected], (
|
||||
"a bare-runner job gained a step invoking a tool that exists only in the toolchain image, "
|
||||
"and nothing reported it — which is exactly the failure ersatztv#789 filed"
|
||||
)
|
||||
|
||||
|
||||
def test_the_requirement_mutation_set_is_not_empty():
|
||||
bare = {n for n, v in declared_classes(_DOC).items() if v == BARE_RUNNER}
|
||||
assert bare, "no job declares bare-runner, so the requirement mutations are vacuous"
|
||||
assert len(_REQUIREMENT_MUTANTS) == len(bare), (
|
||||
f"expected one mutation per bare-runner job ({len(bare)}), got {len(_REQUIREMENT_MUTANTS)}"
|
||||
)
|
||||
|
||||
@@ -151,10 +151,26 @@ def wired_hook_files() -> set[str]:
|
||||
return {rel for rel in tracked_children(*HOOKS) if rel.rpartition("/")[2] in text}
|
||||
|
||||
|
||||
def section(heading: str) -> str:
|
||||
"""The body of ONE `## ` section of the inventory.
|
||||
|
||||
Scoped since ersatztv#786 added `## Workflow-job guards`, a second five-column table whose
|
||||
subject is workflow JOBS rather than guard FILES. `_ROW` is keyed on SHAPE, not on location, so
|
||||
an unscoped `findall` over the whole document would pull those rows into the set-equality below
|
||||
and report every one of them as a guard file that does not exist. A missing heading raises here
|
||||
rather than returning an empty string: a silent empty section is the vacuous-population failure
|
||||
this file exists to prevent.
|
||||
"""
|
||||
text = INVENTORY.read_text()
|
||||
start = text.index(f"\n## {heading}\n")
|
||||
nxt = text.find("\n## ", start + 1)
|
||||
return text[start : nxt if nxt != -1 else len(text)]
|
||||
|
||||
|
||||
def inventory_rows() -> list[tuple[str, str, str, str]]:
|
||||
"""(guard, kind, proof, proof_ref) for each table row."""
|
||||
"""(guard, kind, proof, proof_ref) for each row of the `## Inventory` table."""
|
||||
rows = []
|
||||
for guard, _blocks, kind, proof, ref in _ROW.findall(INVENTORY.read_text()):
|
||||
for guard, _blocks, kind, proof, ref in _ROW.findall(section("Inventory")):
|
||||
rows.append((guard.strip(), kind.strip(), proof.strip(), ref.strip()))
|
||||
return rows
|
||||
|
||||
|
||||
@@ -45,6 +45,7 @@ from scripts.tests import test_guard_inventory as guard_inventory
|
||||
from scripts.tests import test_hook_fire_log as hook_fire
|
||||
from scripts.tests import test_pr_changed_files as pr_changed
|
||||
from scripts.tests import test_remote_state_inventory as remote_state
|
||||
from scripts.tests import test_workflow_job_guards as job_guards
|
||||
from scripts.tests import test_workflow_persist_credentials as persist_creds
|
||||
from scripts.tests import tracked_files
|
||||
|
||||
@@ -112,6 +113,7 @@ DERIVATIONS = (
|
||||
("test_remote_state_inventory.derived_population", remote_state.derived_population, 40),
|
||||
("test_pr_changed_files._workflow_files", pr_changed._workflow_files, 5),
|
||||
("test_workflow_persist_credentials.workflow_files", persist_creds.workflow_files, 5),
|
||||
("test_workflow_job_guards.workflow_files", job_guards.workflow_files, 5),
|
||||
)
|
||||
|
||||
# The floor matters only to the anti-vacuity test; the two property tests take the pair, so an
|
||||
|
||||
@@ -0,0 +1,558 @@
|
||||
"""Every workflow JOB that is a guard has a row in docs/guard-inventory.md (ersatztv#786).
|
||||
|
||||
WHAT THIS CLOSES. The inventory's population was guard FILES — `.claude/hooks/`, `.husky/`, and the
|
||||
`scripts/…` paths workflows and hooks reference. A guard written INLINE in workflow YAML belonged to
|
||||
no population at all, and the inventory said so in its own scope limit rather than implying coverage.
|
||||
The motivating case lived exactly there: `pr-checks.yml:ci-image-pin` stated an invariant in its
|
||||
error text — "Every container: job must pin ersatztv-ci:<7-char-sha>" — and did not check it (#774).
|
||||
It was fixed by adding a structural test, not by bringing the job into any inventory, so the next one
|
||||
would have been just as invisible.
|
||||
|
||||
WHY A MARKER RATHER THAN A LIST. "Which jobs are guards" is a judgement per job, and #786 says so:
|
||||
the filesystem cannot supply it. The available shapes were a hand-written registry in this file, or a
|
||||
declaration in the workflow. This takes the second, for the reason #789 rejected the first in
|
||||
`test_ci_image_pin_population.py`: a literal list of members is a filter frozen at authoring time,
|
||||
correct the day it is written and unable to report the day it stopped being. The judgement still has
|
||||
to be made by a human — it is made in the workflow diff, next to the job, where a reviewer changing
|
||||
that job sees it.
|
||||
|
||||
WHAT THE MARKER MEANS, because the obvious wrong readings make the population useless. THE LINE IS
|
||||
WHAT THE JOB PRODUCES: a `guard` job's output is a VERDICT, a `none` job's output is an ARTIFACT.
|
||||
A `none` job can still go red — that is an ERROR in producing the thing, not a finding about the
|
||||
repo. `docker-build.yml::build` publishes an image and smoke-tests it, and is `none`; a failed smoke
|
||||
test means the build did not work. Which jobs are `none` is read off the markers, not listed here:
|
||||
a list in prose is a second copy of the workflow that rots on the next job added.
|
||||
|
||||
`report-only` is the third value and needs saying, because "guard vs none" does not imply it: a
|
||||
`report-only` job produces a VERDICT IT CANNOT ENFORCE — every check step carries
|
||||
`continue-on-error: true`. The limit, since this file is where someone will look for it: nothing
|
||||
here detects a `guard` job whose checks are all `continue-on-error`. The comparison is marker
|
||||
against ROW, never marker against the job's ability to fail.
|
||||
|
||||
TWO READINGS THAT DO NOT WORK, recorded so neither is re-adopted. "Any job containing a step that
|
||||
can fail" makes every job a guard and the table distinguishes nothing. "A guard enforces an invariant
|
||||
about the REPOSITORY, so a job exercising the PRODUCT is `none`" sounds more principled and is worse:
|
||||
it puts `test` and `migrations` outside the population, and those are the two REQUIRED status
|
||||
contexts on `main` — precisely where a failure to fire is fail-OPEN against branch protection, and
|
||||
precisely the gap #786 exists to close. `test` also runs the C# and TypeScript structural guards
|
||||
`docs/guard-inventory.md`'s scope-limit item 2 names, so "exercises the product" never described it.
|
||||
|
||||
THE RESIDUE, stated rather than left for review. This proves every guard JOB has been classified by
|
||||
someone and has a row; it cannot prove the classification is CORRECT, and it cannot prove an inline
|
||||
assertion works — that needs the job to run. `Proof: NONE` on an inline row is therefore the honest
|
||||
entry and not a gap to be filled with a neighbouring file's proof. Two rows do cite a pytest, and
|
||||
each names the PART of the job it covers, because a reference covering a fraction must not read as
|
||||
covering the whole. Same residue `test_guard_inventory.py` records for its own table, and for the
|
||||
same reason: the table is what review reads.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
import yaml
|
||||
|
||||
from scripts.tests.tracked_files import tracked_paths
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
INVENTORY = REPO_ROOT / "docs" / "guard-inventory.md"
|
||||
SECTION = "Workflow-job guards (ersatztv#786)"
|
||||
|
||||
# From the GIT INDEX, never `Path.glob` (ersatztv#806): an untracked scratch workflow left in the
|
||||
# directory would otherwise join the population on one checkout and not in CI — red locally, green on
|
||||
# the server, which is #778's third shape. `*.yaml` alongside `*.yml` because Gitea accepts both.
|
||||
WORKFLOWS = (".gitea/workflows", ("*.yml", "*.yaml"))
|
||||
|
||||
ROLE_KEY = "CI_JOB_ROLE"
|
||||
GUARD, REPORT_ONLY, NONE = "guard", "report-only", "none"
|
||||
# A VOCABULARY, not a population — the legal values of the marker, not a list of jobs.
|
||||
ROLES = frozenset({GUARD, REPORT_ONLY, NONE})
|
||||
# role -> the Kind the row must carry. `none` is absent by construction: it takes no row.
|
||||
ROLE_KIND = {GUARD: "GUARD", REPORT_ONLY: "REPORT-ONLY"}
|
||||
|
||||
# Keyed on the `<workflow>.yml::<job>` shape of the first cell, so this cannot match a row of the
|
||||
# file-guard table (or any other) even before `inventory_section()` narrows the text.
|
||||
#
|
||||
# A MALFORMED ROW FOR AN OTHERWISE-UNLISTED JOB FAILS CLOSED, and the qualifier is the whole of it.
|
||||
# `[^|]*` for the Blocks cell means a raw `|` inside it (a code span such as `grep … | cut …`) stops
|
||||
# the row matching. When that job has no OTHER row, the unmatched row is an ABSENT row and the set
|
||||
# equality reports a guard job with no entry — measured: injecting a pipe into one Blocks cell
|
||||
# reddens `test_the_inventory_covers_exactly_the_guard_JOBS_that_exist` naming that job.
|
||||
#
|
||||
# THE CASE IT DOES NOT COVER, because an earlier version of this note claimed "every malformed row":
|
||||
# a malformed DUPLICATE is invisible. Its job is already satisfied by the well-formed row, so set
|
||||
# equality holds, `duplicate_row_faults` never sees the second occurrence, and the Kind check has
|
||||
# nothing to compare. A contradictory row that also happens to be malformed therefore passes. No
|
||||
# checker here can see it; a reader can.
|
||||
_ROW = re.compile(r"^\|\s*`([a-z0-9.-]+\.ya?ml::[A-Za-z0-9_-]+)`\s*\|([^|]*)\|\s*([A-Z-]+)\s*\|", re.M)
|
||||
|
||||
|
||||
def workflow_files() -> list[Path]:
|
||||
"""THE WORKFLOW POPULATION, from the git index. Named so the shared proof in
|
||||
`test_guard_populations_derive_from_git.py` can assert it never admits an untracked file."""
|
||||
return tracked_paths(*WORKFLOWS)
|
||||
|
||||
|
||||
def declared_roles(paths: list[Path] | None = None) -> dict[str, str | None]:
|
||||
"""`<workflow>::<job>` -> the role it declares, or None where the marker is absent.
|
||||
|
||||
None is KEPT rather than dropped: an unmarked job is the defect `role_faults` reports, and a dict
|
||||
that silently omitted it could not report anything.
|
||||
|
||||
`paths` defaults to the tracked population and exists so a proof can drive THIS function over a
|
||||
fixture. Without the seam a test wanting to check the reader had to re-implement the read, which
|
||||
is how a proof ends up asserting a fact about PyYAML instead of about the checker.
|
||||
"""
|
||||
out: dict[str, str | None] = {}
|
||||
for path in paths if paths is not None else workflow_files():
|
||||
doc = yaml.safe_load(path.read_text()) or {}
|
||||
for jid, job in (doc.get("jobs") or {}).items():
|
||||
if not isinstance(job, dict):
|
||||
continue
|
||||
value = (job.get("env") or {}).get(ROLE_KEY)
|
||||
out[f"{path.name}::{jid}"] = str(value) if value is not None else None
|
||||
return out
|
||||
|
||||
|
||||
def env_shadow_faults(docs: list[tuple[str, dict]] | None = None) -> list[str]:
|
||||
"""THE DETECTOR for a job-level `env:` key that shadows a workflow-level one.
|
||||
|
||||
`docs` is `(name, parsed workflow)` pairs and defaults to the tracked population, so the proof
|
||||
below drives THIS function over a fixture instead of rebuilding the set intersection itself —
|
||||
a proof that reimplements the comparison stays green when the shipped one is disarmed.
|
||||
"""
|
||||
if docs is None:
|
||||
docs = [(p.name, yaml.safe_load(p.read_text()) or {}) for p in workflow_files()]
|
||||
faults = []
|
||||
for name, doc in docs:
|
||||
workflow_env = set(doc.get("env") or {})
|
||||
if not workflow_env:
|
||||
continue
|
||||
for jid, job in (doc.get("jobs") or {}).items():
|
||||
if not isinstance(job, dict):
|
||||
continue
|
||||
clash = workflow_env & set(job.get("env") or {})
|
||||
if clash:
|
||||
faults.append(
|
||||
f"{name}::{jid} declares job-level {sorted(clash)}, which also exists at "
|
||||
"workflow level. The job value WINS and the workflow value is silently lost."
|
||||
)
|
||||
return faults
|
||||
|
||||
|
||||
def role_faults(roles: dict[str, str | None]) -> list[str]:
|
||||
"""MISSING OR UNKNOWN IS A HARD FAILURE.
|
||||
|
||||
A marker whose absence defaults to `none` would stop applying the moment someone adds a job and
|
||||
forgets — and the job nobody remembered to mark is the job nobody reviewed. An unrecognised value
|
||||
is rejected for the same reason rather than read as "not a guard".
|
||||
"""
|
||||
faults = []
|
||||
for job, value in sorted(roles.items()):
|
||||
if value is None:
|
||||
faults.append(
|
||||
f"job '{job}' declares no {ROLE_KEY}. Every job in every tracked workflow must "
|
||||
f"declare one under its `env:` — {sorted(ROLES)} — so that 'not a guard' is a "
|
||||
"recorded decision rather than an omission."
|
||||
)
|
||||
elif value not in ROLES:
|
||||
faults.append(f"job '{job}' declares {ROLE_KEY}: {value!r}, which is not one of {sorted(ROLES)}.")
|
||||
return faults
|
||||
|
||||
|
||||
def inventory_section() -> str:
|
||||
"""The body of the workflow-job section alone.
|
||||
|
||||
Scoped, not a whole-file scan: `docs/guard-inventory.md` holds several tables and the file-guard
|
||||
one is read by `test_guard_inventory.py`. Two shape-keyed regexes over one document would each
|
||||
pull in the other's rows and report them as phantoms. A missing heading raises rather than
|
||||
returning empty — a silent empty section is the vacuous population this file exists to prevent.
|
||||
"""
|
||||
text = INVENTORY.read_text()
|
||||
start = text.index(f"\n## {SECTION}\n")
|
||||
nxt = text.find("\n## ", start + 1)
|
||||
return text[start : nxt if nxt != -1 else len(text)]
|
||||
|
||||
|
||||
def inventory_row_list() -> list[tuple[str, str]]:
|
||||
"""(`<workflow>::<job>`, Kind) for each row, IN ORDER and WITH duplicates preserved."""
|
||||
return [(job.strip(), kind.strip()) for job, _blocks, kind in _ROW.findall(inventory_section())]
|
||||
|
||||
|
||||
def duplicate_row_faults(rows: list[tuple[str, str]]) -> list[str]:
|
||||
"""THE DETECTOR, named so its live test and its mutation proof share one implementation.
|
||||
|
||||
An earlier draft inlined this loop in the live test and had the proof rebuild the comparison
|
||||
with its own `len(set(...))`. That proof stayed green with the live check disarmed, because it
|
||||
was exercising arithmetic it had written itself rather than the code that ships — the shape
|
||||
`testing.guard-ships-with-mutation-proof` calls a behavioural test dressed as a proof.
|
||||
"""
|
||||
seen: dict[str, str] = {}
|
||||
dupes = []
|
||||
for job, kind in rows:
|
||||
if job in seen:
|
||||
dupes.append(f"'{job}' has rows claiming {seen[job]} and {kind}")
|
||||
seen[job] = kind
|
||||
return dupes
|
||||
|
||||
|
||||
def kind_faults(roles: dict[str, str | None], listed: dict[str, str]) -> list[str]:
|
||||
"""THE DETECTOR for a row whose Kind contradicts the job's marker. Shared with its proof."""
|
||||
return [
|
||||
f"'{job}' declares {ROLE_KEY}: {roles.get(job)} (Kind {ROLE_KIND.get(roles.get(job))}) but its row says {kind}"
|
||||
for job, kind in sorted(listed.items())
|
||||
if ROLE_KIND.get(roles.get(job)) != kind
|
||||
]
|
||||
|
||||
|
||||
def guard_jobs() -> set[str]:
|
||||
"""The jobs whose declared role obliges them to hold a row."""
|
||||
return {j for j, r in declared_roles().items() if r in ROLE_KIND}
|
||||
|
||||
|
||||
def coverage_faults(declared: set[str], listed: set[str]) -> tuple[set[str], set[str]]:
|
||||
"""(missing, phantom) — THE DETECTOR the live check and its three proofs all call.
|
||||
|
||||
Named for the same reason `duplicate_row_faults` is: three `test_MUTATION_*` below used to
|
||||
rebuild `declared - listed` inline, and a proof that reimplements the comparison stays green
|
||||
when the shipped one is disarmed — a behavioural test wearing a MUTATION name. Cold review
|
||||
demonstrated exactly that by flattening the live assertion and watching the file stay green.
|
||||
"""
|
||||
return declared - listed, listed - declared
|
||||
|
||||
|
||||
def inventory_jobs() -> dict[str, str]:
|
||||
"""`<workflow>::<job>` -> Kind. One entry per job; see `test_no_job_has_TWO_rows`.
|
||||
|
||||
A dict is the right shape for the comparisons below and the WRONG shape for detecting a
|
||||
duplicate: two rows for one job collapse, last one wins, and a table that contradicts itself
|
||||
reports green. That is checked separately against `inventory_row_list`, which keeps duplicates,
|
||||
rather than by making every caller handle a list.
|
||||
"""
|
||||
return dict(inventory_row_list())
|
||||
|
||||
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
# ANTI-VACUITY FIRST — a population or a row regex that stopped matching would make every assertion
|
||||
# below compare two empty sets and report a fully-covered inventory. That is the failure this file
|
||||
# exists to prevent, so it is checked before anything depends on it.
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_the_workflow_population_is_not_empty():
|
||||
files = workflow_files()
|
||||
assert files, (
|
||||
"no tracked workflow files were found — `tracked_paths` returned nothing. Every assertion "
|
||||
"below would compare empty sets and pass having checked nothing."
|
||||
)
|
||||
roles = declared_roles()
|
||||
assert roles, f"{len(files)} workflow file(s) parsed but no jobs were found in any of them"
|
||||
|
||||
|
||||
def test_the_workflow_job_table_actually_parsed():
|
||||
rows = inventory_jobs()
|
||||
assert rows, (
|
||||
f"the `## {SECTION}` table parsed to ZERO rows. Either the table is gone or `_ROW` no "
|
||||
"longer matches its shape; both make the set equality below vacuous."
|
||||
)
|
||||
|
||||
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
# THE LIVE ASSERTIONS
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_every_job_declares_a_known_role():
|
||||
faults = role_faults(declared_roles())
|
||||
assert not faults, "workflow jobs with no usable role:\n " + "\n ".join(faults)
|
||||
|
||||
|
||||
def test_the_inventory_covers_exactly_the_guard_JOBS_that_exist():
|
||||
"""SET EQUALITY, both directions — the two failures are opposite mistakes.
|
||||
|
||||
Left-to-right: a job declared a guard with no row, which is the gap #786 was filed for.
|
||||
Right-to-left: a row for a job that no longer declares itself a guard (renamed, deleted, or
|
||||
downgraded), which is how a table becomes a comforting fiction.
|
||||
"""
|
||||
missing, phantom = coverage_faults(guard_jobs(), set(inventory_jobs()))
|
||||
assert missing == set(), (
|
||||
f"these workflow jobs declare {ROLE_KEY} guard/report-only but have NO row in "
|
||||
f"`## {SECTION}` — {sorted(missing)}. Add a row saying what each blocks, where "
|
||||
"its assertion lives and what proof it carries."
|
||||
)
|
||||
assert phantom == set(), (
|
||||
f"`## {SECTION}` has rows for jobs that no longer declare a guard/report-only role — "
|
||||
f"{sorted(phantom)}. Either the job was renamed or removed, or its marker "
|
||||
"changed; the row must follow."
|
||||
)
|
||||
|
||||
|
||||
def test_no_job_has_TWO_rows():
|
||||
"""A duplicate row is a self-contradicting registry that every other check reports green.
|
||||
|
||||
`inventory_jobs()` is a dict, so a second row for the same job silently overwrites the first and
|
||||
set equality still holds in both directions. The realistic arrival is a merge conflict or a
|
||||
copy/paste that leaves a `REPORT-ONLY` and a `GUARD` row for one job: the table then states both,
|
||||
the checker picks whichever is later, and nothing reports the contradiction. Counting rows
|
||||
against distinct keys is the only place that can see it.
|
||||
"""
|
||||
rows = inventory_row_list()
|
||||
assert rows, "the table parsed to zero rows, so this check would be vacuous"
|
||||
dupes = duplicate_row_faults(rows)
|
||||
assert not dupes, (
|
||||
f"`## {SECTION}` lists the same job more than once:\n "
|
||||
+ "\n ".join(dupes)
|
||||
+ "\n\nThe dict built from these rows keeps only the LAST, so every other assertion in this "
|
||||
"file passes while the table contradicts itself."
|
||||
)
|
||||
|
||||
|
||||
def test_MUTATION_a_DUPLICATE_row_is_reported():
|
||||
"""Proof for the check above, with its own negative control."""
|
||||
rows = inventory_row_list()
|
||||
assert not duplicate_row_faults(rows), "NEGATIVE CONTROL FAILED: the unmutated table already contains a duplicate."
|
||||
victim = rows[0][0]
|
||||
mutated = rows + [(victim, "REPORT-ONLY" if rows[0][1] != "REPORT-ONLY" else "GUARD")]
|
||||
faults = duplicate_row_faults(mutated)
|
||||
assert faults and victim in faults[0], f"the DETECTOR did not report a second row for '{victim}': {faults}"
|
||||
|
||||
|
||||
def test_each_row_carries_the_KIND_its_marker_declares():
|
||||
"""The row and the workflow must not disagree about WHICH kind.
|
||||
|
||||
Set equality alone is satisfied by a `report-only` job holding a `GUARD` row — the table would then
|
||||
claim a merge-blocking check where the workflow has one that cannot fail. `docs-reminder` is
|
||||
exactly that job (`continue-on-error: true` on every check step), so the case is live, not
|
||||
hypothetical.
|
||||
"""
|
||||
faults = kind_faults(declared_roles(), inventory_jobs())
|
||||
assert not faults, "row Kind disagrees with the job's declared role:\n " + "\n ".join(faults)
|
||||
|
||||
|
||||
def test_no_job_env_key_SHADOWS_a_workflow_level_one():
|
||||
"""Adding a job-level `env:` to a workflow that has a workflow-level one must not shadow it.
|
||||
|
||||
WHY THIS EXISTS. Before ersatztv#786 no job in `docker-build.yml` declared `env:` at all, and its
|
||||
workflow-level `env:` is memory-critical: `UseSharedCompilation`,
|
||||
`DOTNET_CLI_USE_MSBUILD_SERVER` and `MSBUILDDISABLENODEREUSE` are what keep a 7.8 GB
|
||||
VBCSCompiler off a RAM-oversubscribed runner (ersatztv#406). Actions merges the two scopes with
|
||||
the more specific one winning, so a job-level key of the SAME NAME silently replaces the
|
||||
workflow-level value — and the loss is invisible: the job still runs, just with the shared
|
||||
compiler back on.
|
||||
|
||||
THAT THE SCOPES LAYER RATHER THAN REPLACE is not assumed here, and the evidence is in-repo and
|
||||
live: `build`'s "Smoke + IPTV E2E" step declares its own step-level `env:`
|
||||
(`SMOKE_SHORT_SHA`, `SMOKE_RUN_ID`) and on the same line consumes the WORKFLOW-level `${IMAGE}`
|
||||
— `IMG="${IMAGE}:${SMOKE_SHORT_SHA}"` — on pushes to `main` and `v*` tags (its `if:` also skips
|
||||
a docs-only push). A replacing runner would have broken that long ago.
|
||||
|
||||
THE GAP IN THAT EVIDENCE, stated because it is a scope narrower than the claim: it is STEP-level
|
||||
layering, and what this branch newly relies on is JOB-level. On `origin/main` no job anywhere
|
||||
declared both a job-level `env:` and a consumed workflow-level var, so the job scope had no
|
||||
in-repo witness. The inference is that a runner layering one scope layers the other; the
|
||||
exposure if it did not is the two publish jobs — `ci-image.yml::build` reads `CI_IMAGE` and
|
||||
`docker-build.yml::build` reads `IMAGE`. NEITHER RUNS ON A PR, by two different mechanisms worth
|
||||
keeping straight: `docker-build.yml::build` carries `if: github.event_name != 'pull_request'`,
|
||||
while `ci-image.yml` declares no `pull_request` TRIGGER at all, so its job is never created on a
|
||||
PR rather than being created and skipped. Either way PR CI cannot exercise the job scope, and
|
||||
the first push to `main` would try to publish `":<sha>"` and fail loudly rather than silently.
|
||||
|
||||
So this check is not what proves merging; it removes the remaining exposure, which is a NAME
|
||||
COLLISION. Merging is only safe while no job-level key shadows a workflow-level one, and that is
|
||||
a property of the files rather than of the runner, so it is the half worth asserting here.
|
||||
|
||||
It also catches the reverse mistake — someone "documenting" `REGISTRY` on a job and pinning it to
|
||||
a stale value.
|
||||
"""
|
||||
faults = env_shadow_faults()
|
||||
assert not faults, "job-level env shadows a workflow-level env key:\n " + "\n ".join(faults)
|
||||
|
||||
|
||||
def test_MUTATION_a_SHADOWING_env_key_is_reported():
|
||||
"""Proof for the check above, with its own negative control."""
|
||||
doc = {
|
||||
"env": {"UseSharedCompilation": "false"},
|
||||
"jobs": {"a": {"env": {"CI_JOB_ROLE": "none"}}},
|
||||
}
|
||||
assert not env_shadow_faults([("fixture.yml", doc)]), (
|
||||
"NEGATIVE CONTROL FAILED: the unmutated fixture already reports a collision"
|
||||
)
|
||||
doc["jobs"]["a"]["env"]["UseSharedCompilation"] = "true"
|
||||
faults = env_shadow_faults([("fixture.yml", doc)])
|
||||
assert faults and "UseSharedCompilation" in faults[0], (
|
||||
f"the DETECTOR did not report a shadowing job-level key: {faults}"
|
||||
)
|
||||
|
||||
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
# MUTATION PROOFS — THE GUARD IS A TEST, so each mutation goes into the guarded ARTIFACT (the
|
||||
# workflow, or the table) rather than into an assertion here. Disarming a checker makes it ABSENT
|
||||
# rather than red; `testing.guard-ships-with-mutation-proof`'s checker-guard exception is what makes
|
||||
# introducing the real defect the admissible proof.
|
||||
# ------------------------------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _role_mutants():
|
||||
for job in sorted(declared_roles()):
|
||||
removed = dict(declared_roles())
|
||||
removed[job] = None
|
||||
yield f"{job}-marker-removed", removed
|
||||
|
||||
unknown = dict(declared_roles())
|
||||
unknown[job] = "checker"
|
||||
yield f"{job}-marker-unknown-value", unknown
|
||||
|
||||
|
||||
_ROLE_MUTANTS = list(_role_mutants())
|
||||
|
||||
|
||||
@pytest.mark.parametrize("roles", [m for _, m in _ROLE_MUTANTS], ids=[i for i, _ in _ROLE_MUTANTS])
|
||||
def test_a_missing_or_UNKNOWN_role_is_DETECTED(roles):
|
||||
assert role_faults(roles), "a job with no role, or an unrecognised one, was accepted"
|
||||
|
||||
|
||||
def test_the_role_mutation_set_is_not_empty():
|
||||
"""The NON-EMPTINESS assertion comes first, and it is the whole point of this test.
|
||||
|
||||
`len(_ROLE_MUTANTS) == 2 * len(declared_roles())` alone is `2N == 2N` — a tautology that holds
|
||||
at N=0, so a `declared_roles()` degraded to empty would satisfy it while every parametrised
|
||||
proof above collected zero cases and the file reported green. That is the vacuous-positive-
|
||||
control shape this suite exists to prevent, reproduced inside its own control.
|
||||
"""
|
||||
roles = declared_roles()
|
||||
assert roles, (
|
||||
"declared_roles() is EMPTY, so every role mutation above is vacuous and the parametrised "
|
||||
"proofs collected nothing. Either the workflow population broke or the marker key moved."
|
||||
)
|
||||
expected = 2 * len(roles)
|
||||
assert len(_ROLE_MUTANTS) == expected, f"expected 2 role mutations per job ({expected}), got {len(_ROLE_MUTANTS)}"
|
||||
|
||||
|
||||
def test_MUTATION_a_guard_job_with_its_row_DELETED_is_reported():
|
||||
"""The defect this guard exists to catch: a guard job that acquired no row.
|
||||
|
||||
Mutating the guarded ARTIFACT (the table), not this checker's population — a shrunken population
|
||||
would make every real row report as a PHANTOM, demonstrating a false POSITIVE while proving
|
||||
nothing about the missing-row detection the inventory row claims.
|
||||
"""
|
||||
declared = guard_jobs()
|
||||
listed = set(inventory_jobs())
|
||||
assert declared and listed, "nothing to mutate — the population or the table is empty"
|
||||
# NEGATIVE CONTROL, the same one its phantom twin carries. Without it a PRE-EXISTING
|
||||
# `declared - listed` gap satisfies every iteration below, so the loop would pass while proving
|
||||
# nothing about the deletion — the live assertion would be red in that state, but this proof
|
||||
# would be claiming coverage it does not have.
|
||||
assert coverage_faults(declared, listed)[0] == set(), (
|
||||
f"NEGATIVE CONTROL FAILED: guard jobs {sorted(coverage_faults(declared, listed)[0])} "
|
||||
"already have no row, so every deletion below would be satisfied by the pre-existing gap."
|
||||
)
|
||||
for victim in sorted(listed):
|
||||
missing, _ = coverage_faults(declared, listed - {victim})
|
||||
assert missing == {victim}, (
|
||||
f"the DETECTOR did not report deleting the row for '{victim}' as exactly that job missing"
|
||||
)
|
||||
|
||||
|
||||
def test_MUTATION_a_PHANTOM_row_is_reported():
|
||||
"""The opposite direction: a row for a job that does not declare a guard role.
|
||||
|
||||
Carries its own NEGATIVE control. Asserting only that the mutated set contains a phantom is
|
||||
true even when `inventory_jobs()` returns nothing — the added literal supplies the phantom by
|
||||
itself, so the assertion could not fail and proved nothing about the parser. Pinning the
|
||||
UNMUTATED set to zero phantoms first is what makes the second assertion mean anything.
|
||||
"""
|
||||
declared = guard_jobs()
|
||||
listed = set(inventory_jobs())
|
||||
assert listed, "the table parsed to nothing, so this proof would be vacuous"
|
||||
assert coverage_faults(declared, listed)[1] == set(), (
|
||||
f"NEGATIVE CONTROL FAILED: the unmutated table already reports phantoms "
|
||||
f"{sorted(coverage_faults(declared, listed)[1])}, so the mutation below would prove nothing."
|
||||
)
|
||||
phantom = "docker-build.yml::a-job-that-was-deleted"
|
||||
_, reported = coverage_faults(declared, listed | {phantom})
|
||||
assert reported == {phantom}, (
|
||||
"the DETECTOR did not report a row naming a non-guard job as exactly the phantom introduced"
|
||||
)
|
||||
|
||||
|
||||
def test_MUTATION_a_row_whose_KIND_disagrees_is_reported():
|
||||
"""`report-only` holding a `GUARD` row must not pass — the case `docs-reminder` makes live."""
|
||||
roles = declared_roles()
|
||||
report_only = [j for j, r in roles.items() if r == REPORT_ONLY]
|
||||
assert report_only, "no report-only job exists, so this proof is vacuous"
|
||||
listed = dict(inventory_jobs())
|
||||
assert not kind_faults(roles, listed), (
|
||||
"NEGATIVE CONTROL FAILED: the unmutated table already disagrees with a marker"
|
||||
)
|
||||
listed[report_only[0]] = "GUARD"
|
||||
faults = kind_faults(roles, listed)
|
||||
assert faults and report_only[0] in faults[0], (
|
||||
f"the DETECTOR did not report a report-only job carrying a GUARD row: {faults}"
|
||||
)
|
||||
|
||||
|
||||
def test_a_workflow_job_added_without_a_marker_is_DETECTED():
|
||||
"""A NEW job is the realistic arrival path, and it must not default to `none`."""
|
||||
roles = dict(declared_roles())
|
||||
roles["docker-build.yml::a-brand-new-job"] = None
|
||||
assert role_faults(roles), "a newly added job with no role marker was accepted"
|
||||
|
||||
|
||||
def test_every_tracked_job_APPEARS_in_the_parse(tmp_path):
|
||||
"""Key completeness, plus the ATTRIBUTION property a line-based reader would break.
|
||||
|
||||
The first half is completeness only — every job of every tracked workflow appears as a key. It
|
||||
deliberately does NOT claim to catch a grep-based reader: a grep returning the right keys with
|
||||
wrong VALUES satisfies it. That regression is covered by
|
||||
`test_a_commented_out_marker_does_NOT_count`, which drives the reader over a fixture.
|
||||
|
||||
The second half is the part a completeness check cannot express and that is worth its own
|
||||
fixture: a marker belongs to the job whose `env:` block contains it. A line-oriented reader that
|
||||
scanned for `CI_JOB_ROLE:` and attached the value to the nearest preceding job header would
|
||||
SMEAR the marker of one job onto a neighbour that declares none — and the smear is invisible to
|
||||
any assertion phrased over keys, because both jobs have keys either way.
|
||||
"""
|
||||
roles = declared_roles()
|
||||
for path in workflow_files():
|
||||
doc = yaml.safe_load(path.read_text()) or {}
|
||||
for jid in doc.get("jobs") or {}:
|
||||
assert f"{path.name}::{jid}" in roles, f"{path.name}::{jid} was not parsed"
|
||||
assert all("::" in k for k in roles)
|
||||
|
||||
wf = tmp_path / "two.yml"
|
||||
wf.write_text(f"jobs:\n first:\n steps: []\n second:\n env:\n {ROLE_KEY}: {GUARD}\n steps: []\n")
|
||||
assert declared_roles([wf]) == {"two.yml::first": None, "two.yml::second": GUARD}, (
|
||||
"the reader attributed a marker to the wrong job — `first` declares none and must read as "
|
||||
"None, `second` declares one and must read as its value"
|
||||
)
|
||||
|
||||
|
||||
def test_a_commented_out_marker_does_NOT_count(tmp_path):
|
||||
"""A commented-out marker must read as ABSENT — asserted through `declared_roles` ITSELF.
|
||||
|
||||
An earlier draft called `yaml.safe_load` on a literal and asserted comments are not values. That
|
||||
is a fact about PyYAML, which this suite does not own and no change here could break: replacing
|
||||
`declared_roles` with a grep-based reader left that version GREEN. It now drives the real reader
|
||||
over a real file through the `paths` seam, so a reader that ever became a text scan fails here.
|
||||
"""
|
||||
wf = tmp_path / "commented.yml"
|
||||
wf.write_text("jobs:\n x:\n env:\n # CI_JOB_ROLE: guard\n OTHER: 1\n steps: []\n")
|
||||
|
||||
roles = declared_roles([wf])
|
||||
assert roles == {"commented.yml::x": None}, (
|
||||
f"the real reader returned {roles!r}; a commented-out marker must read as absent"
|
||||
)
|
||||
# And the fault function must REPORT it, not merely fail to see it.
|
||||
assert role_faults(roles), "a job whose only marker is commented out was accepted as classified"
|
||||
|
||||
# The text a grep-based reader would match IS in the file — which is what makes this a real
|
||||
# guard against the parser regressing to a text scan rather than a restatement of YAML rules.
|
||||
assert ROLE_KEY in wf.read_text()
|
||||
|
||||
# A REAL marker in the same shape must still be read, so the assertion above cannot be satisfied
|
||||
# by a reader that returns None for everything.
|
||||
ok = tmp_path / "live.yml"
|
||||
ok.write_text("jobs:\n x:\n env:\n CI_JOB_ROLE: guard\n steps: []\n")
|
||||
assert declared_roles([ok]) == {"live.yml::x": GUARD}, "the reader stopped seeing a real marker"
|
||||
Reference in New Issue
Block a user