diff --git a/docker/Dockerfile b/docker/Dockerfile index dd8eac5f3..ad86dcb2a 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -9,37 +9,41 @@ WORKDIR /source COPY design-system/. ./design-system/ COPY web/. ./web/ WORKDIR /source/web -# The SPA suite runs here except for two files (ersatztv#819), excluded for OVERLAPPING reasons — -# one needs the git binary, the other needs the binary AND a checkout. Reading them as two separate -# reasons is what broke this stage once already: -# * `web/src/api/pageSizeCallSites.guard.test.ts` needs a git CHECKOUT — and, through it, the -# binary. It derives its file population from `git ls-files` rather than a directory walk, and -# refuses rather than falling back. This stage's context is `web/` + `design-system/` only, so -# there is no `.git`. -# * `web/vite-plugins/trackedSourceFiles.realgit.test.ts` needs the BINARY but no checkout: it -# builds its own temp repository to prove the derivation by executing it. -# `node:22-bookworm-slim` ships no git (`command -v git` -> not found), so it dies with -# `spawnSync git ENOENT`. -# So this is NOT checkout-versus-binary, and adding a `COPY .git` would not let either run here — -# the binary would still be missing. Excluding only the first is not enough either, and a replica -# that merely deletes `.git` cannot show that: verify any change here with the git binary off -# `PATH`, not just with the directory absent. -# Everything else — all but those two files — runs fine gitless and is kept, rather than dropping -# the whole suite for one file as an earlier fix here did. -# The excluded pair is not skipped overall: `docker-build.yml`'s `Build & test (.NET)` job runs the -# whole suite on a real checkout, and `build` (the job that invokes this Dockerfile) carries -# `needs: [test, migrations, scan]`. State that chain precisely, because the `needs:` edge is not -# all of it: `Test SPA` is also gated on `docs_only` and on the #420 revalidate skip, and `build` is -# not gated on `revalidate`. On a push whose tree is byte-identical to an already-green head the -# suite is skipped and the image still builds — carried there by #420's byte-identical-tree -# argument. The `docs_only` arm cannot ship an image at all (`Build and push` is gated on it too), -# and `ci-detect-docs-only.sh` classifies by PATH SHAPE (`docs/` or `*.md`), not by directory, so a -# `web/*.md` would count as docs — there are none today, but do not restate it as "any `web/**`". -RUN npm run lint && npm run typecheck && \ - npm test -- --run \ - --exclude 'src/api/pageSizeCallSites.guard.test.ts' \ - --exclude 'vite-plugins/trackedSourceFiles.realgit.test.ts' && \ - npm run build +# THE VITEST SUITE IS NOT RUN IN THIS STAGE, AND MUST NOT BE RE-ADDED (ersatztv#887). +# This stage is gitless in both senses at once. It copies only `web/` and `design-system/`, so it +# holds no `.git` — a property of the STAGE, not of the context, which is the repository root +# (`context: .`) and whose `.dockerignore` does not exclude `.git`: the directory is there to be +# copied and simply is not. And `node:22-bookworm-slim` ships no git binary (`command -v git` -> not +# found), which is why a `COPY .git` would not help either. Members of the SPA suite need one or the +# other: `pageSizeCallSites.guard.test.ts` and `completeAnnotations.guard.test.ts` derive their file +# population from `git ls-files` and refuse to fall back to a directory walk +# (`testing.guard-derives-population-from-source`), and `trackedSourceFiles.realgit.test.ts` builds +# its own temp repository. (Not every `*.guard.test.ts` — `completeRequest.guard.test.ts` needs no +# git at all.) +# +# Running the suite here anyway therefore costs a hand-maintained list of the members that cannot +# run — and that list is a population nothing derives. It went stale the first time a guard was +# added without updating it, and because `Build & push image (amd64)` is `if: github.event_name != +# 'pull_request'`, the resulting red is unreachable on a PR and lands on `main` and on the `v*` tag +# path: every image build failed and `:latest` stopped being republished. Excluding one more file +# re-arms that; the list is removed instead. +# +# WHAT STILL VALIDATES THE SUITE. `docker-build.yml`'s `test` job runs it UNFILTERED on a real +# checkout, and `build` — the job that invokes this Dockerfile — carries `needs: [test, migrations, +# scan]`, so an image cannot be published past a red suite. Two skips exist inside `test` and +# neither leaves an image unvalidated: `docs_only` also gates `Build and push`, so that arm ships +# nothing — and that arm IS asserted, by `test_the_DOCS_ONLY_arm_cannot_publish_an_image`. The #420 +# revalidate skip fires only on a tree byte-identical to a head that already carried a green combined +# status; that arm is NOT asserted here — it rests on `scripts/ci-detect-already-validated.sh`, which +# carries no mutation proof of its own, so it is a dependency rather than a check. Held by +# `scripts/tests/test_image_build_delegates_the_spa_suite.py`. That guard PINS the command line +# below as text: any edit to it reddens, whatever the edit says. Update the pin in the same commit, +# and do not use it to re-add a suite run. +# +# `lint` and `typecheck` stay. They are gitless-safe with no member that is not, so they carry no +# list and no trap; whether the image build should run them at all is a separate question this +# does not answer. +RUN npm run lint && npm run typecheck && npm run build FROM --platform=linux/amd64 192.168.1.95:3000/timothy/ersatztv-ffmpeg:8.1.2 AS runtime-base COPY --from=dotnet-runtime /usr/share/dotnet /usr/share/dotnet diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 320f9a6bd..210ccd8de 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -54,6 +54,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `ci.gate-trigger-base-resolved` | The workflow that writes the branch-protection-required `review-verdict/h10` status triggers on `pull_request_target` with `branches: [main]`, never on plain `pull_request`. Gitea resolves a `pull_request` workflow DEFINITION from the PR's own head commit, so under that trigger a PR editing `.gitea/workflows/review-verdict.yml` ran its own rewritten copy and could post `h10=success` for itself; `pull_request_target` resolves the definition from the base instead. The `branches: [main]` filter is part of the rule, not a refinement of it: base resolution only relocates the rewrite from the head to the base, so without the filter a PR opened into an attacker-pushed base branch runs that branch's gate. `pull_request_target` is safe HERE only because this job never checks out or executes head-supplied code — it checks out `base.sha` and runs only that tree's scripts (`ci.shared-pr-file-enumeration`); reintroducing a head checkout under this trigger would be worse than the bug it fixed. This closes the rewrite route through THIS workflow and does NOT close the class. Gitea injects `GITEA_TOKEN` into EVERY job and it USED to be write-capable everywhere, so any ref-resolved workflow could forge `review-verdict/h10`; since #748 (2026-08-27) all six workflows here declare `permissions:` — five `code: read`, and `review-verdict.yml`'s gate job `code: write`, deliberately, because it IS the gate (the five rest on the TOP-LEVEL form, which is INFERRED rather than probed — the gate job's JOB-LEVEL form is the one measured; see `ci.actions-credential-scoping`). That narrows the INJECTED token and only that: deliberately NO "only X can post a status" summary is stated here, because any such summary generalizes from the injected token, which `permissions:` scopes, to every credential a job can reach, which it does not touch — `RENOVATE_TOKEN` is in the same secret store and is referenced by `renovate.yml`, one of the five holding `code: read`. `ci-image.yml` was the last holdout — its unfiltered `push:` made it the worst-placed of the six — and #744 closed that trigger route (`ci.toolchain-image-publish-is-a-dispatch`) while #748 scoped its token. So the route is NARROWED, not closed, and the SHAPE is open — a newly added workflow declaring write, or omitting a declaration and inheriting the owner default, is a route again, which is why `test_pr_changed_files.py` asserts every tracked workflow declares a block. And a collaborator's own API token still can, since branch protection binds the context and not its issuer. The credential half is now RESOLVED in `ci.actions-credential-scoping` (#697): CI's registry secret was the ADMIN account's basic auth and is now a PAT that cannot post a status, which removes the ADMIN escalation and that credential's route (an ALLOW-LISTED user credential's forgery carries a matching `creator` and is inherited as a verdict; an Actions job's carries `creator: null` and is re-derived, as since #742 is every account outside `H10_REVIEWERS` — but do NOT read that asymmetry as protection: re-derivation fires only on the trigger's `types`, and posting a status is not one of them, so a POST timed after the last PR event simply stands). It does not remove EVERY route: `RENOVATE_TOKEN` is a `write:repository` bot PAT in the same secret store, reachable by any PR-added workflow. The owner-level Restricted default (server-management#714) remains a separate OPEN lever — its effect on a declared `code: write` is unmeasured, and the version half of that condition has been satisfied since the 1.25.4 -> 1.27.1 upgrade on 2026-08-05, so it is not something to wait on a Gitea release for. A collaborator's own token remains unfixable; the exemption path has its own separate defects in #698. | 2026-07-28 | [link](records/ci/gate-trigger-base-resolved.md) | | `ci.gitea-milestone-filter-noop` | Never filter issues with the server-side `?milestones=` parameter — fetch all open issues once and filter LOCALLY on each issue's `.milestone.title`. | 2026-07-21 | [link](records/ci/gitea-milestone-filter-noop.md) | | `ci.grep-q-pipefail-inversion` | In any script running under `set -o pipefail`, a security or classification predicate of the form `producer \| grep -q…` is FORBIDDEN: `grep -q` exits at its first match, the producer then takes SIGPIPE and exits 141 once the data exceeds the pipe buffer (~64K), so `pipefail` reports the pipeline as FAILED even though grep MATCHED — inverting the predicate exactly when the input is large. A here-string (`grep -q… <<< "$data"`) is ALSO forbidden: bash materialises a large here-string via temporary storage, so it fails when temp space is full or unwritable, and inside an `if`/`!` that failure flips the predicate the same way. COUNT instead — `n=$(printf '%s\n' "$data" \| grep -cE "$re")` — because `grep -c` drains stdin (no early exit, no SIGPIPE) over an ordinary pipe (no temp file). Read grep's status honestly: exit 1 means a zero count and is a legitimate answer, anything >1 is a real error. Evaluate the counts ONCE at TOP LEVEL, never inline inside an `if`/`elif` condition: inside `$( )` an `exit` leaves only the subshell and `set -e` does not fire, so an error silently reads as "no match". Validate that each result is numeric and fail closed if not. This applies to both the enforced gate `.gitea/workflows/review-verdict.yml` and the advisory hook `.claude/hooks/pretooluse-merge-consent.sh`. | 2026-07-29 | [link](records/ci/grep-q-pipefail-inversion.md) | +| `ci.image-build-delegates-the-spa-suite` | `docker/Dockerfile` runs no vitest suite. Its web-build stage lints, typechecks and BUILDS the SPA; the suite runs once, unfiltered, in `docker-build.yml`'s `test` job on a real checkout, and `build` carries `needs: [test, migrations, scan]` so no image is published past a red suite. THAT EDGE IS NOW THE ONLY LAYER, so the guard checks it is real and not merely present — and it does so by PINNING TEXT rather than parsing it. The commands each SPA-carrying Dockerfile stage runs, and the gating step's `run:` body and `if:`, are compared as strings against a declared pin; the step and its job must carry no `continue-on-error` in any spelling, the job no job-level `if:`, and the publish step must keep its own `docs_only` gate. A guard that asks what a command MEANS was wrong nine times in three review rounds. A pin is immune to a different SPELLING of the command, which is that whole class; it is not immune to the same text meaning something else, so the routes to that are pinned or refused too: `working-directory` and the step's own `shell:`, the workflow `defaults.run.shell` and the job-level `defaults` overriding it, a stage `SHELL`, `web/package.json`'s script map (pinned WHOLE — selecting on the literal `vitest` missed `npm run test`, `npm t` and the `prebuild`/`preinstall` lifecycle hooks), `web/vite.config.ts` PINNED WHOLE — it decides what the suite collects and what `vite build` loads, and pinning a BLOCK of it was defeated seven measured ways across three rounds, so the partial match was withdrawn rather than respelled an eighth time; and the ABSENCE of any config file that outranks it — `vitest.config.*` for vitest, and `vite.config.js`/`.mjs` for `vite build`, whose `DEFAULT_CONFIG_FILES` lists them BEFORE `.ts`. Pinning one file is worthless while a second can outrank it, and that family went short twice. The gating job's `container:` image is left to `ci.image-pin-population` rather than guarded twice. What is unmodelled is a LIST, not an "only": an `ENV` changing what a pinned `RUN` resolves, the plugin BODIES, and a publish through another action. The count of such routes lives in the `docs/guard-inventory.md` row and nowhere else — it briefly existed in three places with two different values, which is how a number stays wrong. It is a running total, wrong at every count so far. They share one shape, which is the transferable part: A PIN ASSUMES IT IS PINNING THE ARTIFACT THAT STILL DECIDES, and every route found so far is authority moving to another file, another occurrence, another workflow, or a hook the pinned command invokes. Do NOT restore a filtered in-image run by naming the specs that cannot run there: that list is a population nothing derives, and the red it produces when it goes stale is unreachable on a PR — `Build & push image (amd64)` is `if: github.event_name != 'pull_request'` — so it lands on `main` and on the `v*` tag path, where it fails the release cut. Held in both directions by `scripts/tests/test_image_build_delegates_the_spa_suite.py`. | 2026-08-30 | [link](records/ci/image-build-delegates-the-spa-suite.md) | | `ci.infra-shaped-red-under-load` | When a job dies inside a setup/cache step before your code compiles, check the runner host's load before diagnosing the diff, and never file a CI bug off one sample under pressure. | 2026-07-21 | [link](records/ci/infra-shaped-red-under-load.md) | | `ci.jq-version-contract` | Every shell gate that shells out to `jq` is authored to the jq 1.6-compatible subset, because the CI runner ships jq 1.6 while every developer Mac ships 1.8.x. `scripts/jq-preflight.sh` (no args) prints the parsed version and asserts a floor of 1.6 in every gate job's log; `scripts/jq-preflight.sh --expect 1.6` additionally pins and fails loudly, but ONLY in the `script-tests` job. `review-verdict.yml` never pins — it writes the branch-protection-required `review-verdict/h10` status, so a hard pin there would turn any jq bump into a repo-wide merge deadlock. | 2026-07-26 | [link](records/ci/jq-version-contract.md) | | `ci.killed-job-triage` | Never trust a job's `conclusion` field alone — read the log tail and require an `❌ Failure - Main …` marker before treating a red as a real failure. | 2026-07-21 | [link](records/ci/killed-job-triage.md) | diff --git a/docs/decisions/records/ci/image-build-delegates-the-spa-suite.md b/docs/decisions/records/ci/image-build-delegates-the-spa-suite.md new file mode 100644 index 000000000..45aee9c77 --- /dev/null +++ b/docs/decisions/records/ci/image-build-delegates-the-spa-suite.md @@ -0,0 +1,74 @@ +--- +key: ci.image-build-delegates-the-spa-suite +title: '2026-08-30 — the image build builds the SPA and does not test it (#887)' +status: active +since: '2026-08-30' +supersedes: none +superseded-by: none +rule: '`docker/Dockerfile` runs no vitest suite. Its web-build stage lints, typechecks and BUILDS the SPA; the suite runs once, unfiltered, in `docker-build.yml`''s `test` job on a real checkout, and `build` carries `needs: [test, migrations, scan]` so no image is published past a red suite. THAT EDGE IS NOW THE ONLY LAYER, so the guard checks it is real and not merely present — and it does so by PINNING TEXT rather than parsing it. The commands each SPA-carrying Dockerfile stage runs, and the gating step''s `run:` body and `if:`, are compared as strings against a declared pin; the step and its job must carry no `continue-on-error` in any spelling, the job no job-level `if:`, and the publish step must keep its own `docs_only` gate. A guard that asks what a command MEANS was wrong nine times in three review rounds. A pin is immune to a different SPELLING of the command, which is that whole class; it is not immune to the same text meaning something else, so the routes to that are pinned or refused too: `working-directory` and the step''s own `shell:`, the workflow `defaults.run.shell` and the job-level `defaults` overriding it, a stage `SHELL`, `web/package.json`''s script map (pinned WHOLE — selecting on the literal `vitest` missed `npm run test`, `npm t` and the `prebuild`/`preinstall` lifecycle hooks), `web/vite.config.ts` PINNED WHOLE — it decides what the suite collects and what `vite build` loads, and pinning a BLOCK of it was defeated seven measured ways across three rounds, so the partial match was withdrawn rather than respelled an eighth time; and the ABSENCE of any config file that outranks it — `vitest.config.*` for vitest, and `vite.config.js`/`.mjs` for `vite build`, whose `DEFAULT_CONFIG_FILES` lists them BEFORE `.ts`. Pinning one file is worthless while a second can outrank it, and that family went short twice. The gating job''s `container:` image is left to `ci.image-pin-population` rather than guarded twice. What is unmodelled is a LIST, not an "only": an `ENV` changing what a pinned `RUN` resolves, the plugin BODIES, and a publish through another action. The count of such routes lives in the `docs/guard-inventory.md` row and nowhere else — it briefly existed in three places with two different values, which is how a number stays wrong. It is a running total, wrong at every count so far. They share one shape, which is the transferable part: A PIN ASSUMES IT IS PINNING THE ARTIFACT THAT STILL DECIDES, and every route found so far is authority moving to another file, another occurrence, another workflow, or a hook the pinned command invokes. Do NOT restore a filtered in-image run by naming the specs that cannot run there: that list is a population nothing derives, and the red it produces when it goes stale is unreachable on a PR — `Build & push image (amd64)` is `if: github.event_name != ''pull_request''` — so it lands on `main` and on the `v*` tag path, where it fails the release cut. Held in both directions by `scripts/tests/test_image_build_delegates_the_spa_suite.py`.' +signals: 'image build fails on main only · every image build fails · latest not republished · release cut fails at the image build · gitless build STAGE not context · node:22-bookworm-slim ships no git · virtual:etv-tracked-source-files could not read the git index · hand-maintained exclude list · exclusion list is the underived population · suite members needing the git binary vs a checkout · COPY .git would not help · needs edge is the only gate · unfiltered gating run · paths: `docker/Dockerfile`, `.gitea/workflows/docker-build.yml`, `scripts/tests/test_image_build_delegates_the_spa_suite.py`, `web/vite-plugins/trackedSourceFiles.ts` · issues: #887, #883, #819, #806, #420' +mechanics: 'Populations are DERIVED from the git index (tracked Dockerfiles and workflows, and the stages within them carrying the SPA source); the command lines in those places are PINNED as text. A population decides what is CHECKED, so a hand-written one goes silently short; a pin decides what is EXPECTED, so a stale one goes loudly red. Publishing jobs come from the `docker/build-push-action` step and the Dockerfile each builds from that step''s `file:` input, so `ci-image.yml` is out of scope by derivation rather than by an exemption. 73 mutants, 0 missed (development battery).' +--- + +The web-build stage is gitless twice over. It copies only `web/` and `design-system/`, so it holds +no `.git`. That is a property of the STAGE, not of the build context — the context is the repository +root (`context: .`) and `.dockerignore` does not exclude `.git`, so the directory is available to be +copied and simply is not; stating it the other way invites a reader to check, find `.git` in the +context, and conclude the whole note is stale. And `node:22-bookworm-slim` ships no git binary. Members of the SPA +suite need one or the other: `pageSizeCallSites.guard.test.ts` and +`completeAnnotations.guard.test.ts` derive their file population from `git ls-files` +and refuse to fall back to a directory walk (`testing.guard-derives-population-from-source`), and +`trackedSourceFiles.realgit.test.ts` builds a real temporary repository. Because the two +prerequisites OVERLAP rather than divide, adding a `COPY .git` fixes neither: the binary is still +missing. + +So running the suite there at all costs a list of the members that cannot run, maintained by hand +beside a suite that grows. That list went stale the first time a guard was added without updating +it, and the failure it produced was invisible where it could have been cheap and fatal where it was +not: green on every PR, red on `main` and on the release tag. + +**Why not the alternatives.** Each was considered and rejected on its own evidence, not by analogy: + +* **Add a third `--exclude`.** Re-arms the same trap for the next guard, which is what the issue was + filed to prevent. +* **Derive the exclusions from source.** Buildable, but it requires a predicate for "needs git" over + spec text — two unrelated shapes today (a virtual-module import, a `git` spawn) and no reason to + think that stays two. A string predicate over source is the wrong instrument for a set whose + members are defined by what they DO at runtime. +* **Give the container git** (`apt-get install git` plus a real `.git`). It works, and it costs a + cache-busting `COPY` of VCS metadata on every commit, so the whole lint/typecheck/test/build layer + re-runs for changes that do not touch `web/`. Synthesising a repository in the image instead — + `git init && git add -A` — is worse than it looks: the resulting index is a filesystem walk with + extra steps, which is precisely the population the guards refuse. + +**What the removal gives up, enumerated rather than waved through** +(`process.enumerate-workaround-behaviors-before-deleting`): + +1. *Blocking a publish on a red suite* — retained, by the `needs:` edge. Two skips exist inside + `test`. `docs_only` also gates `Build and push`, so that arm ships nothing — ASSERTED, by + `test_the_DOCS_ONLY_arm_cannot_publish_an_image`, after cold review pointed out it had been + stated as fact with nothing checking it. The #420 revalidate skip fires only on a tree + byte-identical to a head that already carried a green combined status; that arm is a DEPENDENCY + on `scripts/ci-detect-already-validated.sh` rather than something asserted here, and that script + is graded `MUTATION: NONE`. Said plainly because #887 is what made the second arm load-bearing: + before it, the in-image run covered that case. +2. *Running the suite under `node:22-bookworm-slim` specifically* — **lost**, and this is the real + cost. It is small because the suite is jsdom and pure JS, and because the artifact the image + actually ships is the output of `vite build`, which still runs there: a test that passes in the + CI toolchain image and fails in bookworm-slim would be telling us about the test environment, not + about the image. +3. *`docker build` running the suite for a developer locally* — lost; `npm test -- --run` is the + direct route and is what `docs/testing.md` documents. + +**The guard's own mechanism was replaced once, and the reason belongs here rather than only in the commit log, because the next person to widen it needs it.** Three versions asked whether a given command runs the suite and whether its failure can be swallowed. That predicate was wrong nine times across three cold-review rounds — executed heredocs treated as data, `#` truncating a command mid-word (including the live `${#reports[@]}` idiom), compound punctuation welding commands, `npm t` and `./node_modules/.bin/vitest` and `timeout`/`su -c` wrappers unrecognised, `true || npm test` counted as a run it never performs, and `continue-on-error: ${{ … }}` slipping a two-literal check. Twice, a clause added to remove a FALSE RED opened a FALSE GREEN on the guard's headline assertion. The mechanism was withdrawn rather than patched a tenth time: the risky command lines are now compared against a pin, so no spelling has to be recognised in order to be rejected. The cost is real and is the point — a legitimate edit to those lines reddens the guard and its author updates the pin deliberately. + +A fourth cold review then attacked the pin itself and found the honest limit: the replacement removes the SPELLING problem outright, but the same pinned text can be made to mean something else, and two mutants re-armed this very defect through `web/package.json` — `RUN npm run build` executes whatever that file says — with every pin still matching. A fifth round then found the same mistake inside the fix: the package.json check SELECTED on the literal `vitest` where a PIN was available, and four one-line edits that never spell it — `npm run test`, `npm t`, and the `prebuild`/`preinstall` lifecycle hooks — each re-armed the defect with every other pin matching. The whole script map is pinned now, and so is `web/vite.config.ts` — WHOLE, after three further +rounds each defeated one spelling of a partial match of it, and two more routes bypassed the marker +entirely because `defineConfig` is the identity function and a later spread replaces what was +matched. + +The general lesson is worth more than any of the fixes: replacing a predicate with a pin moves the problem from "can the checker recognise this?" to "is the checked text still the whole story?". The second question has a short, workable answer where the first did not — but it is still a question, the route list has been wrong at every count so far, and the honest form of that is a running total with its history attached rather than a closure claim. + +`lint` and `typecheck` stay in the stage. They are gitless-safe with no member that is not, so they +carry no list and no trap. Whether the image build should run them at all is a separate question +this does not answer, and leaving them is not an argument that it should. diff --git a/docs/guard-inventory.md b/docs/guard-inventory.md index 133280191..d5d033583 100644 --- a/docs/guard-inventory.md +++ b/docs/guard-inventory.md @@ -74,7 +74,9 @@ with have sat in these gaps. Treat anything not named here as unassessed, not as and the pair `web/vite-plugins/trackedSourceFiles.test.ts` + its `.realgit.test.ts` sibling, which together prove the population `pageSizeCallSites.guard.test.ts` derives (split because the `realgit` half - needs the git BINARY, which the Docker web-build stage has not got). Note what that costs: this + needs the git BINARY and its thirteen injected-runner siblings do not, so the prerequisite is + confined to one file — it is NOT the Docker exclusion it was originally written for, which + ersatztv#887 removed along with the whole in-image suite run). Note what that costs: this list is a HAND-WRITTEN mirror of a population nothing derives, so it goes stale silently and CI stays green — #807 added the third entry, and nothing mechanical caught that `pageSizeCallSites.guard.test.ts` had become the only one named. @@ -171,6 +173,7 @@ The audit #806 asked for, recorded whichever way it came out, because "we looked | `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: 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 | +| `test_image_build_delegates_the_spa_suite.py` | DERIVED: tracked Dockerfiles and workflows from the git index; the stages within them that carry the SPA source. PINNED (hand-written, and the distinction is the whole design): the commands each such stage runs, and the gating `Test SPA` step's `run:` body and `if:` | **yes on the derived populations, by pin rather than by predicate** — every SPA-carrying stage must have a pin and match it; every image-publishing job whose Dockerfile has such a stage must transitively `needs:` the gating job | ersatztv#887. **THE PARSER WAS WITHDRAWN, and that is the finding worth carrying.** Three versions of this guard asked "does this command RUN the suite, and can it FAIL?" of arbitrary shell text, and that predicate was wrong NINE times across three cold-review rounds, always the same mechanism: heredoc bodies skipped as data when BuildKit EXECUTES `RUN < `run: >` folds two commands into one; (3) Gitea's treatment of a SKIPPED `needs:` job is not modelled — the guard forbids the job-level `if:` that would create one rather than reasoning about it; (4) the pins are hand-written and a legitimate edit reddens this guard, which is the intended review trigger and the stated cost | | `web/src/api/pageSizeCallSites.guard.test.ts` | `import.meta.glob` over `web/src/**/*.{ts,tsx,mts,cts}`, INTERSECTED with the git index | **yes** — an unregistered discovered site fails | converted under #819. The population is the glob INTERSECTED with the git index, and the walk/index divergence is asserted in BOTH directions WITHIN a shared scope: a key the index does not carry is dropped, and a tracked, on-disk, in-scope path that never reached the scanner FAILS rather than shrinking the population silently (that direction catches a dotfile, which the glob cannot match, and a disk/index spelling divergence, which `core.ignorecase` and NFD/NFC normalisation make permanent). The SCOPE itself cancels out of that equality — both sides call one `isInScopeSourcePath` — so narrowing it moves both sets together and no assertion comparing the two sides can see it. That is closed separately, by re-deriving the predicate from its two components over the whole tracked index (`the shared scope predicate is EXACTLY its two documented components`). Two mechanisms carry that. FIRST, the scope predicate is proved by a CLOSED-FORM restatement — it may share no helper, at any depth, with the predicate it checks, since anything shared sits on both sides of the comparison and cancels. FOUR earlier attempts were each measured going green while removing real files: a table of example paths (four of `src/`'s eight directories, missed a 23-file narrowing), a decomposition delegating its filename half to `isScannableSourceFileName` (11 files), one still sharing a `basename` helper (15 files, and blind to a planted call site), and one still sharing the tracked-file array every comparison was derived from (8 files, also blind). SECOND — because that last one showed a restatement cannot police the population it reads — the plugin runs a separate `git ls-files --others` query and the guard requires every in-scope walked path to appear in `tracked ∪ others`; narrowing `tracked` cannot suppress that, since it adds nothing to `others`. SIX residuals, NOT a claim of closure, each with its direction MEASURED by planting a real `pageSize` call site rather than reasoned about. FAIL-NOISY (they redden a checkout; they cannot hide a call site): (2) a scope term matching no tracked path today survives until the day it first matches one; (4) the WIRING in `listSourceFiles`, indistinguishable on a clean checkout where the walk set and the index set agree on every in-scope key (they are not literally equal — the index also carries `.css`, which the glob never yields), so no assertion COMPARING THE TWO POPULATIONS can tell which is passed; (5) `ABSENT_FROM_DISK` emptied. BLIND — each hid a planted `pageSize: 100` with the whole suite green: (1) a COORDINATED edit of the scope predicate and BOTH closed-form restatements, which is three sites and not two, since a second restatement guards `expectedTrackedSources` (measured: the two-site edit reddens, the three-site edit does not) — tolerated because a three-site policy edit is review-visible in a way a one-line slip is not; and (3) a misdescription by the plugin's THREE outputs — not only a mispartition of `tracked`/`others`, which preserves the union any consumer compares, but a FATTENED `absentFromDisk`, which subtracts real files from the hole-detection direction. The fattening is now caught wherever it overlaps a file the walk found (a walked path is on disk by construction, so it can never be legitimately absent); restricted to paths the walk cannot see anyway — a dotfile, a case-divergent name — it stays invisible from the consuming side and is answerable only by testing the derivation directly, and the real-git tests that do so catch an UNCONDITIONAL misdescription, not one keyed on a path pattern. Neither blind residual is tolerated for being harmless. A sixth is specific to WATCH mode and is stale in BOTH directions, so it belongs to neither group: the virtual module has no backing file, so the index is read once per dev-server lifetime while the glob refreshes. A file CREATED mid-session reddens the cross-check misleadingly (noisy); a file already UNTRACKED when the watcher started keeps that classification when staged mid-session, so its call sites are never scanned — MEASURED green across both phases while `npx vitest run` on the same tree reports `UNREGISTERED`, i.e. blind, and a window that did not exist before #819 because the population was then the walk. A watch-mode green is therefore not authoritative for this guard; restarting the watcher clears the noisy case and opens the blind one, so the remedy is `npm test -- --run`. Invalidating the module from `configureServer` was implemented, measured and REJECTED: it fixes the created-mid-session red and additionally blinds the create-then-stage sequence, while the already-untracked-then-staged sequence is blind either way, since `git add` fires no watcher event in either design. The index is read by `web/vite-plugins/trackedSourceFiles.ts` in Vite's own Node context and handed to the app project as the virtual module `virtual:etv-tracked-source-files`. That is what reaches the index without admitting `@types/node` to `tsconfig.app.json` — the obstacle that deferred this, since wiring those types in was tried and reverted (it leaked Node's `setTimeout` into the app project and broke three unrelated tests). The plugin throws rather than falling back to the unfiltered walk when git fails, reports zero files, or cannot learn the Vite root. Residual, stated: a file tracked but DELETED in the working tree is subtracted deliberately, since an unstaged deletion is a normal developer state and a guard red on one gets ignored (#806); it is distinguished from the hole cases above by an on-disk existence check, not conflated with them | | `web/src/api/completeAnnotations.guard.test.ts` | TWO derived populations: the `Complete<…>` annotations across `web/src/**/*.{ts,tsx,mts,cts}` INTERSECTED with the git index, and the droppable SCHEMAS parsed out of the generated `src/api/generated/v1.d.ts` | **partial, and the split is stated** — set equality holds for the SCHEMA population (a new optional member fails until dispositioned) and every tracked in-scope path must be supplied by the glob; there is NO closed-form restatement of the scope predicate, so a coordinated edit to it is caught by five named path pins AND a 0.95 population-ratio floor, not by a completeness proof | ersatztv#820. Turns `test_optional_request_members.py`'s `COVERED` disposition — worded "the builder is annotated `Complete`", a claim about ANOTHER LANGUAGE'S source that nothing checked — into a check, and makes `docs/spa-conventions.md` §4b's prohibitions executable as asserted ABSENCES. It found one live defect: `playouts.ts` declared two request types as hand-written mirrors SHADOWING generated schemas of the same name, so their `Complete<>` was checking a local copy rather than the contract. **NARROWED after four BLOCKED review rounds, and the narrowing is the point.** It originally also derived the write WRAPPERS and required the annotation on the wrapper parameter. Every blocker across those rounds came out of that one mechanism — the obligation attached to the wrong population; reachability mistaken for protection (`Complete` is shallow, so a wrapper annotation never reached a nested schema); body discovery keyed on a parameter NAME, then on parameter-versus-local; and finally an `export function` -> `export const` refactor that removed real protection while both the AST scan and its supposedly independent regex cross-check stayed blind TOGETHER, because both keyed on the same token. Five defects from one mechanism is the `process.enumerate-workaround-behaviors-before-deleting` signal to remove it rather than patch a sixth time, and ~250 lines went with it. WHAT IS NOT COVERED, listed because the earlier version of this row stated residuals in the direction that was actually covered: (1) the obligation is per-SCHEMA, not per-SITE and not per-WRAPPER — deleting the `Complete<>` from an API wrapper stays green as long as some production file still names that schema, so the wrapper half of §4b rests on review; (2) it is a TOKEN-PRESENCE check, so a dead `export type X = Complete` that nothing uses discharges the obligation as well as a live builder does — it catches deletion, which is the failure actually observed in #807, not substitution; (3) the PHANTOM direction needs a fresh literal in a contextually typed position and is not checked at all (sites-in-code, #777); (4) the test-file exclusion covers `*.test.*`, `*.spec.*` and the ONE setup file `vite.config.ts` names, whose path is pinned so a rename reddens — a SECOND `setupFiles` entry is loaded by vitest every run and WOULD discharge the obligation; that needs a three-site coordinated edit and is tolerated on the terms the sibling guard states for its own; (5) a MUST-NOT-ANNOTATE violation inside a `*.guard.test.ts` is not seen, since that file class is excluded to keep `completeRequest.guard.test.ts`'s synthetic `Complete<{…}>` fixtures out of the resolver. The disposition VALUES are cross-checked against the authoritative Python table by `scripts/tests/test_complete_annotation_dispositions.py` — without it, flipping one row from `ANNOTATED` to `CREATE` silently retired the requirement, which cold review demonstrated. Nine mutations were witnessed by hand across development, NOT re-executed per suite, so this guard claims no standing `MUTATION` grade — the same footing as `pageSizeCallSites.guard.test.ts`. (Its Python cross-check `test_complete_annotation_dispositions.py` DOES carry a declared harness-executed mutation and is graded accordingly.) A SIXTH residual, shared with its sibling: every population here derives from `trackedSources.tracked`, so a narrowing inside `web/vite-plugins/trackedSourceFiles.ts` cancels out of every comparison — this guard never reads the plugin's separate `others` query, and relies on `pageSizeCallSites.guard.test.ts` and `trackedSourceFiles.test.ts` policing the plugin. A coordinated scope edit is caught by five named path pins AND a 0.95 population-ratio floor whose denominator is computed by a different expression, not by a completeness proof | | `test_ci_release_path_scan_job.py` | `.gitea/workflows/*.y*ml` + `scripts/**` | **no** — a fixture assembling a tmp harness, asserted about behaviour not membership | takes its file LIST from the index anyway, for hermeticity not completeness: `shutil.copytree` copied whatever was on disk, including untracked files and `scripts/__pycache__`, into a tree whose behaviour the probes then measure. Content still comes from the working tree. The copy is not a git repo, so the two files this step RUNS may not use the helper — see the fixture docstring | @@ -249,6 +252,7 @@ recorded as unexamined rather than as cleared. | `scripts/tests/test_guard_inventory.py` | the `script-tests` job, on an unclassified guard or a stale proof ref | GUARD | MUTATION | `test_guard_inventory.py::test_the_inventory_covers_exactly_the_guards_that_exist` | | `scripts/tests/test_guard_populations_derive_from_git.py` | the `script-tests` job, on a guard whose file population admits a file git does not track | GUARD | MUTATION | `test_guard_populations_derive_from_git.py::test_no_derivation_admits_an_untracked_file` | | `scripts/tests/test_hook_fire_log.py` | the `script-tests` job, on a hook that stops reporting that it fired, or whose reporting changes what the harness sees | GUARD | MUTATION | `test_hook_fire_log.py::test_a_hook_that_LOSES_its_instrumentation_is_DETECTED` | +| `scripts/tests/test_image_build_delegates_the_spa_suite.py` | the `script-tests` job, on ANY change to the commands an SPA-carrying Dockerfile stage runs, on ANY change to the gating `Test SPA` step (its `run:` body, its `if:`, or a `continue-on-error` in any spelling), on the publish step losing its `docs_only` gate, on an image-publishing job that stops being gated on the job holding that step, or on any other step mentioning the suite | GUARD | MUTATION | `test_image_build_delegates_the_spa_suite.py::test_every_SPA_CARRYING_STAGE_runs_exactly_its_pinned_commands` | | `scripts/tests/test_jq_preflight.py` | the `script-tests` job | PROOF | NONE | — | | `scripts/tests/test_mcp_smoke.py` | the `script-tests` job, when the MCP smoke checker accepts a server it should refuse — an impostor identity, a reply carrying only an id, a missing expected tool, or an `initialize` pre-answered before the request (the `tools/list` twin is held by two mechanisms jointly and is covered behaviourally, not by the declared clause) | GUARD | MUTATION | `test_mcp_smoke.py::test_MUTATION_a_PRE_ANSWERED_id_is_refused_because_the_request_ids_are_UNGUESSABLE` | | `scripts/tests/test_merge_consent_base_change.py` | the `script-tests` job | PROOF | NONE | — | @@ -375,7 +379,7 @@ clause-provable and the entry is regraded. ## What the numbers say -45 guards, 6 tooling scripts, 23 proof files. **25 guards carry a mutation proof; 5 are +46 guards, 6 tooling scripts, 23 proof files. **26 guards carry a mutation proof; 5 are behaviour-only; 15 have none.** These figures are asserted against the table by `test_the_summary_counts_match_the_table`, because a hand-maintained summary of a table is a second copy of it — the duplication family this change argues against, and one that has already drifted diff --git a/docs/testing.md b/docs/testing.md index 0d93d1685..dbc04b5d5 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -20,7 +20,7 @@ before adding tests, not just `docs/contributing.md` §8 (which now just points | `ErsatzTV.Scanner.Tests` | Library scanning: scan handlers, folder scanners, NFO readers | Handler tests substitute the folder scanners + `ILibraryRepository` and assert the resulting repository writes (e.g. `ScanLocalLibraryHandlerTests` pins which `LastScan` levels a scan records — ersatztv#264). Fakes/`Testably` back the file-system-facing scanners. ~1,485 tests (approximate on purpose — an exact count goes stale on every PR that adds one). Additionally contains `Core/FFmpeg/TranscodingTests` — `[Explicit]` + `[Combinatorial]`, so it never runs in CI or a plain `dotnet test` (it needs real ffmpeg/hardware) and contributes 0 to that count; run it by name when touching the transcoding pipeline. | | `ErsatzTV.Architecture.Tests` | Layering rules via NetArchTest.eNhancedEdition | Core↛Infra/App/EF; FFmpeg↛all; App↛concrete providers. 5 tests. See `docs/contributing.md` §1. | | `ErsatzTV.FFmpeg.Tests` | FFmpeg command construction | Build a pipeline, assert the exact rendered arg string (`PipelineBuilderBaseTests.cs`). | -| `web/` (vitest) | React SPA unit tests | Run alongside typecheck + build (see below). Collects `src/**`, `web/scripts/**` *and* `web/vite-plugins/**`, but deliberately **excludes** `web/e2e/**` (the Playwright specs — vitest's default `**/*.spec.*` glob would otherwise run them under jsdom). Two files have git prerequisites since ersatztv#819. `web/src/api/pageSizeCallSites.guard.test.ts` needs a git **checkout** AND, through it, the **binary** — it derives its file population from `git ls-files` via `web/vite-plugins/trackedSourceFiles.ts` rather than a directory walk, and refuses rather than falling back. `web/vite-plugins/trackedSourceFiles.realgit.test.ts` needs the **binary** but no checkout: it builds its own temp repository to prove that derivation by executing it. So it is not checkout-versus-binary — supplying a `.git` alone would not let either run. Every other file runs fine with neither. `docker/Dockerfile`'s web-build stage runs the suite with exactly those two `--exclude`d, because its context carries no `.git` and `node:22-bookworm-slim` ships no git — and the two exclusions overlap rather than divide — both files need the binary. | +| `web/` (vitest) | React SPA unit tests | Run alongside typecheck + build (see below). Collects `src/**`, `web/scripts/**` *and* `web/vite-plugins/**`, but deliberately **excludes** `web/e2e/**` (the Playwright specs — vitest's default `**/*.spec.*` glob would otherwise run them under jsdom). Some files have git prerequisites since ersatztv#819, and the set is not fixed — ersatztv#883 added one. `web/src/api/pageSizeCallSites.guard.test.ts` and `web/src/api/completeAnnotations.guard.test.ts` need a git **checkout** AND, through it, the **binary**: they derive their file population from `git ls-files` via `web/vite-plugins/trackedSourceFiles.ts` rather than a directory walk, and refuse rather than falling back. `web/vite-plugins/trackedSourceFiles.realgit.test.ts` needs the **binary** but no checkout — it builds its own temp repository to prove that derivation by executing it. So it is not checkout-versus-binary: supplying a `.git` alone would not let any of them run. **The suite therefore runs only where git is present, and `docker/Dockerfile` is not such a place** — its web-build stage builds the SPA and does not test it (ersatztv#887). Enumerating the git-dependent files as Docker `--exclude`s was tried and REVERSED: that list is a population nothing derives, it went stale the first time a guard was added, and the resulting red is unreachable on a PR — `Build & push image (amd64)` is `if: github.event_name != 'pull_request'` — so it landed on `main` and on the release tag instead. The image is gated on `docker-build.yml`'s `test` job running the whole suite on a real checkout, held by `scripts/tests/test_image_build_delegates_the_spa_suite.py`. | | `web/e2e/` (Playwright) | UI-interactive E2E flows against a **live** instance | Not a unit suite and **not** part of `npm test` — needs a running server, so it runs via `scripts/e2e-ui.sh` (boots its own fresh instance) and in CI as a step of the `functional-e2e` job. Headless Chromium, `serial`, `retries: 0`. Scope rule: assert only what the curl harness structurally cannot. See `docs/e2e-local.md` → "UI-E2E harness". | ## Golden-file nets diff --git a/scripts/tests/mutation_manifest.py b/scripts/tests/mutation_manifest.py index d09fde233..68f305534 100644 --- a/scripts/tests/mutation_manifest.py +++ b/scripts/tests/mutation_manifest.py @@ -403,6 +403,26 @@ MUTATIONS: tuple[Mutation, ...] = ( "that it moved). Disarming the comparison inside the script instead would prove the second " "and say nothing about the first, which is the half that was missing.", ), + Mutation( + guard="scripts/tests/test_image_build_delegates_the_spa_suite.py", + target="docker/Dockerfile", + clause="RUN npm run lint && npm run typecheck && npm run build", + replacement="RUN npm run lint && npm run typecheck && npm test -- --run && npm run build", + proof="test_image_build_delegates_the_spa_suite.py::test_every_SPA_CARRYING_STAGE_runs_exactly_its_pinned_commands", + granularity=CLAUSE, + expect="docker/Dockerfile::web-build does not run its pinned commands", + why="The target is not the guard, per the checker-guard exception: the guard IS a test, so " + "disarming it makes it absent rather than red, and the mutation goes into the guarded " + "ARTIFACT. The clause is the web-build stage's whole command line, and the replacement is " + "the defect itself rather than a caricature of it — ersatztv#883 put a vitest run back into " + "a gitless stage, and every image build failed from that commit until #887. WHAT THIS PROVES " + "IS NARROWER THAN IT LOOKS, and saying so is the point: the guard no longer decides whether " + "a command RUNS the suite (that predicate was wrong nine times across three review rounds), " + "it compares the stage's commands against a pin. So this mutation proves the pin is " + "compared and reported — not that any particular spelling is recognised, because none needs " + "to be. The mutant is deliberately the UNFILTERED spelling: the filtered one is what broke, " + "but a pin rejects both identically and choosing the narrower case would suggest otherwise.", + ), ) diff --git a/scripts/tests/test_guard_populations_derive_from_git.py b/scripts/tests/test_guard_populations_derive_from_git.py index 12fc69450..ecc437bb5 100644 --- a/scripts/tests/test_guard_populations_derive_from_git.py +++ b/scripts/tests/test_guard_populations_derive_from_git.py @@ -43,6 +43,7 @@ from scripts.tests import test_ci_image_pin_population as image_pin from scripts.tests import test_ci_status_context_uniqueness as ctx_unique 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_image_build_delegates_the_spa_suite as image_build 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 @@ -108,6 +109,8 @@ def _as_relative_strings(members) -> set[str]: DERIVATIONS = ( ("test_guard_inventory.derived_guard_files", guard_inventory.derived_guard_files, 25), ("test_hook_fire_log.hook_scripts", hook_fire.hook_scripts, 10), + ("test_image_build_delegates_the_spa_suite.dockerfiles", image_build.dockerfiles, 4), + ("test_image_build_delegates_the_spa_suite.workflow_files", image_build.workflow_files, 5), ("test_ci_image_pin_population.workflow_files", image_pin.workflow_files, 5), ("test_ci_status_context_uniqueness.workflow_files", ctx_unique.workflow_files, 5), ("test_remote_state_inventory.derived_population", remote_state.derived_population, 40), diff --git a/scripts/tests/test_image_build_delegates_the_spa_suite.py b/scripts/tests/test_image_build_delegates_the_spa_suite.py new file mode 100644 index 000000000..297f5ac6a --- /dev/null +++ b/scripts/tests/test_image_build_delegates_the_spa_suite.py @@ -0,0 +1,1053 @@ +"""The SPA vitest suite never runs inside an image build, and the image is gated on a step that does +run it, unfiltered and non-advisory (ersatztv#887). + +WHAT THIS IS PROTECTING. `docker/Dockerfile`'s `web-build` stage is gitless twice over. It copies +only `web/` and `design-system/`, so it holds no `.git` — a property of the STAGE, not of the build +context, which is the repository root (`context: .`) and whose `.dockerignore` does not exclude +`.git`: the directory is there to be copied and simply is not. And `node:22-bookworm-slim` ships no +git binary, which is why a `COPY .git` would not help either. Members of the SPA suite need one or +the other — `pageSizeCallSites.guard.test.ts` and `completeAnnotations.guard.test.ts` derive their +file population from `git ls-files` and refuse to fall back to a directory walk +(`testing.guard-derives-population-from-source`), and `trackedSourceFiles.realgit.test.ts` builds a +real temporary repository. + +Running the suite there anyway therefore requires a list of the members that cannot run, and that +list is a population nothing derives: ersatztv#883 added a third member without updating a +hand-written pair of `--exclude`s, and every image build failed from that commit on. The red was +structurally unreachable on a PR — `Build & push image (amd64)` carries +`if: github.event_name != 'pull_request'` — so it landed on `main` and on the `v*` tag path, where a +release cut fails at the image build and `:latest` stops being republished. + +## WHY THIS GUARD PINS TEXT INSTEAD OF PARSING IT + +The first three versions of this file asked "does this command RUN the suite, and can it FAIL?" of +arbitrary shell text. That predicate was wrong nine times across three review rounds, and every one +of the nine was the same mechanism: + + * heredoc bodies — skipped as data, but BuildKit EXECUTES `RUN <` folds them into one and the whole-text comparison could + not see it. + * Gitea's own evaluation of a skipped `needs:` job is not modelled; the guard forbids the job-level + `if:` that would create one rather than reasoning about it. +""" + +from __future__ import annotations + +import json +import re +from fnmatch import fnmatch +from pathlib import Path + +import yaml + +from scripts.tests import tracked_files +from scripts.tests.tracked_files import tracked_paths + +REPO_ROOT = Path(__file__).resolve().parents[2] +WORKFLOW_DIR = ".gitea/workflows" +WORKFLOWS = (WORKFLOW_DIR, ("*.yml", "*.yaml")) + +# The action every image publish in this repo goes through. Matched on the last two path segments +# for the reason `test_workflow_persist_credentials.py` gives: Gitea accepts a full action URL, and +# a host is not required to contain a dot, so no hostname heuristic is safe. +PUBLISH_ACTION = "docker/build-push-action" + +# Instructions that bring files from the build context into the image. +COPY_INSTRUCTIONS = frozenset({"ADD", "COPY"}) + +# --------------------------------------------------------------------------------------------- +# THE PINS. Update these deliberately, in the same commit as the change they describe, and say why. +# --------------------------------------------------------------------------------------------- + +# Every `RUN` in a Dockerfile stage that carries the SPA source, keyed `::`. Whitespace +# is normalised (see `_normalise`) so a reflow is not a red, but the tokens are exact. +PINNED_STAGE_COMMANDS: dict[str, tuple[str, ...]] = { + "docker/Dockerfile::web-build": ( + "RUN npm ci", + # Lints, typechecks and BUILDS the SPA. It must not TEST it: this stage has neither a git + # checkout nor the git binary, and naming the specs that cannot run there is the + # hand-maintained population ersatztv#887 removed. + "RUN npm run lint && npm run typecheck && npm run build", + ), +} + +# The step that runs the suite and gates the image, pinned whole. Its body being exact is what makes +# "unfiltered", "actually executes", and "cannot be suppressed" true without parsing any of it. +GATING_WORKFLOW = ".gitea/workflows/docker-build.yml" +GATING_JOB = "test" +GATING_STEP_NAME = "Test SPA" +GATING_STEP_RUN = '"${GITHUB_WORKSPACE:-.}/scripts/ci-step-ran.sh" mark web-test\nnpm test -- --run' +# `docs_only` (ersatztv#416) and the #420 revalidate skip. Both are safe for a DIFFERENT reason and +# the difference is why this is pinned rather than pattern-matched: docs-only cannot ship an image at +# all, which is asserted below rather than assumed; the revalidate skip fires only on a tree +# byte-identical to a head that already carried a green combined status. +GATING_STEP_IF = "steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true'" +# `npm test` means nothing without the directory it runs in: pointed elsewhere it runs a different +# package, or none. Part of the pin because it is part of what the step does. +GATING_STEP_WORKDIR = "web" +# `shell: bash` is `bash -e`, which is what makes a failing command fail the step. A workflow-level +# change here alters that for the gating step without touching the step at all. +GATING_WORKFLOW_SHELL = "bash" + +# The premise under the `docs_only` half of `GATING_STEP_IF`: the publish step is gated on it too, so +# the arm that skips the suite ships nothing. Round-1 review found this stated as fact with nothing +# checking it, one level out from the same defect. +PUBLISH_STEP_NAME = "Build and push" +PUBLISH_STEP_IF = "steps.detect.outputs.docs_only != 'true'" + +# `RUN npm run build` and `RUN npm ci` execute whatever `web/package.json` says they do, and that +# file is neither a Dockerfile nor a workflow. Cold review re-armed ersatztv#887 through it twice +# without touching any pinned line: `"build": "vitest run && tsc -b && vite build"` puts the suite +# back into the gitless stage, and `"prepare"` is run by `npm ci`. So the SCRIPTS THAT MENTION VITEST +# are pinned too — exactly one, and its body. +PACKAGE_JSON = "web/package.json" +# THE WHOLE MAP, not the scripts that mention vitest. Selecting on the literal `vitest` was a +# SELECTOR where a PIN was available, and cold review re-armed ersatztv#887 through four spellings +# none of which contain it: `"build": "npm run test -- --run && …"`, `"build": "npm t -- …"`, and the +# lifecycle hooks `prebuild` (npm runs it before `npm run build`) and `preinstall` (before `npm ci`). +# Both pinned RUN lines in the gitless stage execute whatever this map says, so the map is pinned. +PINNED_PACKAGE_SCRIPTS: dict[str, str] = { + "dev": "vite --host 0.0.0.0", + "build": "tsc -b && vite build", + "generate:api": "node scripts/generate-openapi-types.mjs", + "check:api": "npm run generate:api && git diff --exit-code -- src/api/generated/v1.d.ts", + "lint": "eslint .", + "test": "vitest", + "test:ui-e2e": "playwright test", + "typecheck": "tsc -b --pretty false", + "prepare": "cd .. && husky", +} + +# `npm test -- --run` collects whatever this config says, and `vite build` in the gitless stage +# loads it too — so the WHOLE FILE is pinned, not a block within it. Three rounds each closed one +# SPELLING of a marker match (`test: {`, then `test: {`, then `test : {` and `"test": {`), and two +# more routes never touched the marker at all (`].concat([evil])` after the pinned array, +# `...moreTest` after the pinned block — `defineConfig` is the identity function, so a later spread +# REPLACES what the pin matched). That is `every-blocker-was-one-mechanism-so-delete-it` at the +# count where it says withdraw: the mechanism was partial matching, and it is gone. +# +# STATED COST: any edit that changes a LINE'S TOKEN SEQUENCE reddens, a comment's words included. +# Whitespace and blank lines are free — `_normalise_lines` collapses them, which is what lets a +# reflow through. That tolerance is inert for this file's current content (no template literal and +# no ASI-sensitive token: every `return`/`=>`/backtick in it sits inside a `//` comment) and would +# stop being inert if one were introduced. It is 48 lines, nothing generates it, and it is the same +# move already made for `web/package.json`'s script map — a PIN where a partial match was defended. +VITE_CONFIG = "web/vite.config.ts" +PINNED_VITE_CONFIG = """import { fileURLToPath, URL } from 'node:url'; +import react from '@vitejs/plugin-react'; +import { configDefaults, defineConfig } from 'vitest/config'; +import { trackedSourceFilesPlugin } from './vite-plugins/trackedSourceFiles'; + +const aspNetHost = 'http://localhost:8409'; + +export default defineConfig({ + base: '/app/', + plugins: [react(), trackedSourceFilesPlugin()], + resolve: { + alias: { + '@': fileURLToPath(new URL('./src', import.meta.url)), + '@design-system': fileURLToPath(new URL('../design-system', import.meta.url)) + } + }, + build: { + outDir: '../ErsatzTV/wwwroot/app', + emptyOutDir: true + }, + server: { + port: 5173, + strictPort: false, + proxy: { + '/api': aspNetHost, + '/iptv': aspNetHost, + '/artwork': aspNetHost + } + }, + test: { + environment: 'jsdom', + environmentOptions: { + jsdom: { + url: 'http://localhost/app/' + } + }, + // `e2e/` holds the Playwright UI-E2E specs (ersatztv#445), which drive a REAL browser against a + // running instance. Vitest's default `include` glob (`**/*.{test,spec}.*`) would otherwise pick + // them up and run them under jsdom, where `@playwright/test`'s `test()` has no runner and the + // whole file dies. Spread the defaults rather than replacing `exclude` outright — a bare + // `exclude: ['e2e/**']` would drop `**/node_modules/**` and make vitest crawl deps. + // NOTE: excluding (not narrowing `include` to `src/**`) is deliberate — `scripts/` holds a real + // vitest test too (`generate-openapi-types.test.mjs`), which an `src/**`-only include would + // silently stop running. + exclude: [...configDefaults.exclude, 'e2e/**'], + setupFiles: './src/setupTests.ts' + } +}); +""" + +# Every config filename that can outrank the pinned one, for EITHER tool. Read out of the pinned +# tarballs rather than remembered: vite 8.1.3's `DEFAULT_CONFIG_FILES` is +# `["vite.config.js", "vite.config.mjs", "vite.config.ts", …]` — so a `vite.config.js` beats the +# pinned `.ts` for `vite build`, which is what the gitless stage runs. Vitest 4.1.9 resolves +# `vitest.config.*` before `vite.config.*`. MEASURED: a `web/vite.config.js` whose plugin shells out +# to the suite ran it in the gitless stage with all 1411 tests green. +OUTRANKING_CONFIG_PREFIXES = ("vitest.config.", "vitest.workspace.", "vitest.projects.", "vite.config.") + +# Substrings that mean "a human should look at this step". NOT a semantic predicate — the pinned step +# is allowed to contain them and every other step is not. +SUITE_MENTIONS = ("npm t", "npm run test", "yarn test", "pnpm test", "bun test", "vitest") +# `npm t` rather than `npm test`: it is npm's own documented alias, it SUBSUMES `npm test` as a +# substring, and this file enumerates it among the spellings that defeated the parser. Leaving it out +# meant an unpinned stage running `npm t -- --run` passed the sweep — measured by cold review. + + +def _normalise(text: str) -> str: + """Collapse runs of whitespace so a reflow is not a red, keeping every token exact.""" + return " ".join(text.split()) + + +def _normalise_lines(text: str) -> list[str]: + """Per-line normalisation, so indentation is free but the LINE BOUNDARIES are not. + + A newline separates two commands; collapsing it merges them into one. That is not a cosmetic + difference and the whole-text form cannot see it. + """ + return [_normalise(line) for line in text.strip().splitlines() if line.strip()] + + +def _canonical_action(uses: str) -> str: + """The last two path segments of a `uses:` value, without its `@version`. + + Same normalisation as `test_workflow_persist_credentials._canonical_action`, and deliberately a + second copy rather than an import: that guard's population is checkout steps, this one's is + publish steps, and sharing the helper would mean a narrowing there silently narrows here too. + """ + ref = uses.strip().strip("'\"").rsplit("@", 1)[0].strip().lower() + ref = re.sub(r"^https?://", "", ref) + segments = [seg for seg in ref.split("/") if seg] + if segments: + segments[-1] = re.sub(r"\.git$", "", segments[-1]) + return "/".join(segments[-2:]) if len(segments) >= 2 else ref + + +def dockerfiles() -> list[str]: + """Tracked Dockerfiles, by repo-relative path, in every spelling docker recognises. + + `tracked_files._git_ls_files()` through the MODULE, never a `from … import` binding. The shared + proof in `test_guard_populations_derive_from_git.py` narrows the index by patching that module + attribute, and a name bound at import time is not reachable from there — the derivation would go + on returning every Dockerfile after git stopped tracking them, and the proof would say so. + """ + return sorted( + path + for path in tracked_files._git_ls_files() + if path.rpartition("/")[2].startswith("Dockerfile") or path.endswith(".Dockerfile") + ) + + +def workflow_files() -> list[Path]: + return tracked_paths(*WORKFLOWS) + + +def _read(rel: str) -> str: + return (REPO_ROOT / rel).read_text(encoding="utf-8") + + +def instructions(text: str) -> list[str]: + """The Dockerfile instructions, comments dropped and `\\` continuations joined. + + This is the whole of the file's syntax handling. It does not interpret what a `RUN` DOES — the + string is compared against a pin — so the shell traps that defeated three earlier versions + (heredocs, `#` inside a command, compound punctuation) cannot reach any decision here. + """ + joined: list[str] = [] + buffer = "" + for raw in text.splitlines(): + if raw.lstrip().startswith("#"): + continue + stripped = raw.rstrip() + if stripped.endswith("\\"): + buffer += stripped[:-1] + " " + continue + buffer += stripped + if buffer.strip(): + joined.append(_normalise(buffer)) + buffer = "" + if buffer.strip(): + joined.append(_normalise(buffer)) + return joined + + +def stages(text: str) -> list[tuple[str, list[str]]]: + """`(stage name, its instructions)` per build stage, in file order. + + A stage is delimited by `FROM`, which is the one Dockerfile construct with no shell content and + therefore nothing to misparse. An unnamed stage is keyed by its index so it is still checkable. + """ + found: list[tuple[str, list[str]]] = [] + for line in instructions(text): + if re.match(r"^FROM\b", line, re.I): + match = re.search(r"\bAS\s+(\S+)\s*$", line, re.I) + found.append((match.group(1) if match else f"", [])) + elif found: + found[-1][1].append(line) + return found + + +def copies_spa_source(command: str) -> bool: + """Does this ONE instruction bring the SPA source tree into the image? + + Every spelling counts, because the negative assertion SKIPS every stage answered False — a miss + here drops a stage out of the check silently rather than reddening. `ADD` as well as `COPY`; the + JSON exec form `COPY ["web/", "/dest/"]`, with or without a preceding flag; a source without a + trailing slash (`COPY web /dest`); and `COPY . …`, which carries the tree without naming it. + + A `COPY --from=` is not a context copy: its source is another stage, so `--from=build .` + would not bring the repo in. Flags are dropped before the source arguments are read. + """ + tokens = command.split() + if not tokens or tokens[0].upper() not in COPY_INSTRUCTIONS: + return False + remainder = " ".join(token for token in tokens[1:] if not token.startswith("--")).strip() + if remainder.startswith("["): + # Parse the exec form BEFORE the `--from=` branch: splitting `["/source/web", "/dest"]` on + # whitespace leaves a trailing comma on the first token, so the segment test never matched + # and the stage went unpinned. Measured False. + try: + parsed = json.loads(remainder) + except ValueError: + return True + remainder = " ".join(str(item) for item in parsed) if isinstance(parsed, list) else "" + if any(token.startswith("--from=") for token in tokens[1:]): + # A stage copy is not a CONTEXT copy, so `--from=build .` brings in that stage's `/`, not the + # repository. But it can still carry the SPA SOURCE across from a stage that has it, and a + # stage answered False here is UNPINNED and unchecked — measured: `COPY --from=web-build + # /source/web /source/web` was False, so a suite run in the receiving stage was invisible. + # The test is a whole `web` path SEGMENT in the source, which distinguishes the source tree + # from the built artifact this repo actually copies (`/source/ErsatzTV/wwwroot/app/.` has no + # such segment). + return any("web" in source.strip("/").split("/") for source in remainder.split()[:-1]) + # `remainder` is already a plain space-joined string by here — the exec form was parsed above, + # before the `--from=` branch. A second `startswith("[")` test used to sit here and was dead + # code: measured by poisoning it with a `raise`, which left every test green. + arguments = remainder.split() + if len(arguments) < 2: + return False + return any(_source_reaches_web(source) for source in arguments[:-1]) + + +def _source_reaches_web(source: str) -> bool: + """Could this COPY source bring `web/` in? + + Exact (`web`, `web/…`), whole-context (`.`, `./`), and GLOB: `COPY web*/ ./` matches `web/` and + so carries the source, while `webhooks/` does not — measured False for the glob by cold review, + which left the receiving stage unpinned. `fnmatch` against the literal segment `web` is what + separates the two without hand-listing patterns. + """ + normalised = source.removeprefix("./").lstrip("/").rstrip("/") + if normalised in {"", ".", "web"} or normalised.startswith("web/"): + return True + head = normalised.split("/")[0] + return any(char in head for char in "*?[") and fnmatch("web", head) + + +def spa_carrying_stages() -> list[tuple[str, str, list[str]]]: + """`(dockerfile, stage name, RUN commands)` for every stage carrying the SPA source.""" + found: list[tuple[str, str, list[str]]] = [] + for rel in dockerfiles(): + for name, lines in stages(_read(rel)): + if any(copies_spa_source(line) for line in lines): + found.append((rel, name, [line for line in lines if re.match(r"^RUN\b", line)])) + return found + + +def stage_shell_overrides() -> list[str]: + """Every `SHELL` instruction inside a pinned stage, which the pin alone cannot see. + + `SHELL ["/bin/sh", "-c"]` redefines what every later `RUN` in the stage EXECUTES while leaving + the `RUN` text — and therefore the pin — untouched. Reported rather than modelled: there is no + `SHELL` in this repo, so the honest move is to refuse the construct here rather than reason about + what a replacement interpreter would do. + """ + overrides: list[str] = [] + for rel in dockerfiles(): + for name, lines in stages(_read(rel)): + if f"{rel}::{name}" not in PINNED_STAGE_COMMANDS: + continue + overrides.extend(f"{rel}::{name}: {line!r}" for line in lines if re.match(r"^SHELL\b", line)) + return overrides + + +def _jobs(doc: object) -> dict[str, dict]: + jobs = doc.get("jobs") if isinstance(doc, dict) else None + if not isinstance(jobs, dict): + return {} + return {str(key): value for key, value in jobs.items() if isinstance(value, dict)} + + +def _steps(job: dict) -> list[dict]: + steps = job.get("steps") + return [step for step in steps if isinstance(step, dict)] if isinstance(steps, list) else [] + + +def _needs(job: dict) -> list[str]: + needs = job.get("needs") + if isinstance(needs, str): + return [needs] + return [str(item) for item in needs] if isinstance(needs, list) else [] + + +def _transitive_needs(job_id: str, jobs: dict[str, dict]) -> set[str]: + seen: set[str] = set() + frontier = list(_needs(jobs.get(job_id, {}))) + while frontier: + current = frontier.pop() + if current in seen: + continue + seen.add(current) + frontier.extend(_needs(jobs.get(current, {}))) + return seen + + +def workflow_jobs(rel: str) -> dict[str, dict]: + return _jobs(yaml.safe_load(_read(rel))) + + +def find_step(rel: str, job_id: str, name: str) -> dict | None: + for step in _steps(workflow_jobs(rel).get(job_id, {})): + if str(step.get("name", "")).strip() == name: + return step + return None + + +def publishing_jobs() -> list[tuple[str, str, str, dict[str, dict]]]: + """`(workflow rel path, job id, Dockerfile rel path, that workflow's jobs)` per publishing job. + + The Dockerfile comes from the step's own `file:` input, defaulting to docker's own default + (`Dockerfile` at the context root) when the input is absent, so a step that stops naming one is + still resolved rather than dropped from the population. + """ + found: list[tuple[str, str, str, dict[str, dict]]] = [] + for path in workflow_files(): + rel = path.relative_to(REPO_ROOT).as_posix() + jobs = workflow_jobs(rel) + for job_id, job in jobs.items(): + for step in _steps(job): + uses = step.get("uses") + if not isinstance(uses, str) or _canonical_action(uses) != PUBLISH_ACTION: + continue + with_block = step.get("with") + named = with_block.get("file") if isinstance(with_block, dict) else None + dockerfile = str(named or "Dockerfile").strip() + found.append((rel, job_id, re.sub(r"^\./", "", dockerfile), jobs)) + return found + + +# ------------------------------------------------------------------------------------------------ +# NEGATIVE — the gitless stages run exactly what they are pinned to run +# ------------------------------------------------------------------------------------------------ + + +def test_every_SPA_CARRYING_STAGE_runs_exactly_its_pinned_commands() -> None: + """Set equality on command TEXT, not a predicate over it. + + A suite run put back in ANY spelling — a heredoc, `sh -c`, `npm t`, `timeout npm test`, a + `#`-suffixed line — is simply not equal to the pin. That is the whole point: three earlier + versions asked what a command meant and were wrong nine times; this one asks whether it is the + command that was reviewed. + """ + faults: list[str] = [] + seen: set[str] = set() + for rel, stage, commands in spa_carrying_stages(): + key = f"{rel}::{stage}" + seen.add(key) + pinned = PINNED_STAGE_COMMANDS.get(key) + if pinned is None: + faults.append( + f"{key} carries the SPA source but has no entry in `PINNED_STAGE_COMMANDS`, so " + f"nothing checks what it runs. It runs: {commands}. Add the entry — and if any of " + f"those commands runs the vitest suite, that is the ersatztv#887 defect: the stage " + f"has no git checkout and no git binary, so the suite's git-dependent members " + f"cannot run there, and the red is unreachable on a PR." + ) + continue + if tuple(commands) != pinned: + faults.append( + f"{key} does not run its pinned commands.\n pinned: {list(pinned)}\n actual: {commands}\n" + f" If this change is deliberate, update `PINNED_STAGE_COMMANDS` in the same commit " + f"and say why. If it added a vitest run, do not: this stage cannot run the suite " + f"(ersatztv#887), and excluding the specs that fail there is the hand-maintained " + f"population that broke every image build in ersatztv#883." + ) + for orphan in sorted(set(PINNED_STAGE_COMMANDS) - seen): + faults.append( + f"`PINNED_STAGE_COMMANDS` names {orphan}, which no longer exists or no longer carries " + f"the SPA source. A pin for a stage that is not checked is a sentence nobody is reading." + ) + assert not faults, "\n\n".join(faults) + + +# ------------------------------------------------------------------------------------------------ +# POSITIVE — the image is gated on the job holding the pinned step +# ------------------------------------------------------------------------------------------------ + + +def test_every_IMAGE_PUBLISHING_job_is_gated_on_the_job_that_runs_the_suite() -> None: + """Without this, deleting `needs:` is silently green.""" + known = set(dockerfiles()) + faults: list[str] = [] + for rel, job_id, dockerfile, jobs in publishing_jobs(): + if dockerfile not in known: + faults.append( + f"{rel}: job `{job_id}` builds `{dockerfile}`, which is not a tracked Dockerfile. " + f"The population here is derived from the git index, so this job's image is " + f"UNCHECKED — fix the path or widen `dockerfiles()`." + ) + continue + if not any(stage_rel == dockerfile for stage_rel, _, _ in spa_carrying_stages()): + continue + gates = _transitive_needs(job_id, jobs) + # `needs:` names a job in the SAME workflow, so the edge only reaches the pinned step when + # the publishing job lives in `GATING_WORKFLOW`. Matching a bare job id anywhere was a false + # green: a second workflow publishing this Dockerfile while `needs:`-ing its OWN unrelated + # job called `test` (`run: echo ok`) satisfied it — measured, and it escaped every + # workflow-population guard in the repo, 588 tests green. + if rel != GATING_WORKFLOW or GATING_JOB not in gates: + faults.append( + f"{rel}: job `{job_id}` publishes an image built from `{dockerfile}`, which carries " + f"the SPA source, but does not transitively `needs:` `{GATING_JOB}` IN " + f"`{GATING_WORKFLOW}` — the job holding the pinned suite step. It needs: " + f"{sorted(gates)}. A `needs:` edge resolves within its own workflow, so a job named " + f"`{GATING_JOB}` in a DIFFERENT workflow is a different job and gates nothing. Since " + f"ersatztv#887 the image build does not run the suite itself, so this edge is the " + f"ONLY thing between a red suite and a published image." + ) + assert not faults, "\n".join(faults) + + +# ------------------------------------------------------------------------------------------------ +# EFFECTIVE — the gating step really runs, really fails, and really runs everything +# ------------------------------------------------------------------------------------------------ + + +def test_the_GATING_STEP_is_exactly_what_was_reviewed() -> None: + """Its body, its `if:`, and the absence of `continue-on-error` — all pinned. + + Pinning the BODY is what makes "unfiltered" and "not suppressed" true without parsing: a + `--exclude`, a `|| true`, a pipe, a `set +e`, a `true || npm test` that never executes, are each + a different string from the pin. Three earlier versions tried to recognise those individually and + missed `true || npm test` and `continue-on-error: ${{ … }}` among others. + """ + named = [ + candidate + for candidate in _steps(workflow_jobs(GATING_WORKFLOW).get(GATING_JOB, {})) + if str(candidate.get("name", "")).strip() == GATING_STEP_NAME + ] + assert len(named) == 1, ( + f"{GATING_WORKFLOW}: job `{GATING_JOB}` has {len(named)} steps named {GATING_STEP_NAME!r}, " + f"not one. That name is what both this assertion and the substring sweep key on, so a " + f"SECOND step wearing it inherits the exemption and is never checked — measured by cold " + f"review with a duplicate carrying `continue-on-error: true` and a spec filter." + ) + step = named[0] + + faults: list[str] = [] + body = step.get("run") + # PER LINE, not over the whole body: `_normalise` collapses newlines, so changing `run: |` to + # `run: >` folds the two lines into one command whose normalised text is byte-identical to the + # pin. Measured by cold review — the marker script then swallows the suite as its arguments. + if not isinstance(body, str) or _normalise_lines(body) != _normalise_lines(GATING_STEP_RUN): + faults.append( + f"the gating step's `run:` is not the pinned command.\n pinned: {GATING_STEP_RUN!r}\n" + f" actual: {body!r}\n Anything but this — a spec filter, a `|| true`, a pipe, a " + f"`set +e`, a suite on the right of a `||` that may never execute — changes whether a " + f"red suite blocks the image. Update the pin deliberately, in the same commit, with a " + f"reason." + ) + + workdir = step.get("working-directory") + if workdir is None or str(workdir).strip() != GATING_STEP_WORKDIR: + faults.append( + f"the gating step's `working-directory:` is {workdir!r}, not the pinned " + f"{GATING_STEP_WORKDIR!r}. `npm test` run anywhere else runs a different package, or " + f"none, while the pinned command text is unchanged." + ) + + shell = ((yaml.safe_load(_read(GATING_WORKFLOW)) or {}).get("defaults") or {}).get("run", {}).get("shell") + if shell is None or str(shell).strip() != GATING_WORKFLOW_SHELL: + faults.append( + f"{GATING_WORKFLOW}'s `defaults.run.shell` is {shell!r}, not the pinned " + f"{GATING_WORKFLOW_SHELL!r}. `bash` here means `bash -e`, which is what makes a failing " + f"command fail the step; changing it changes whether a red suite blocks the image, " + f"without touching the step." + ) + + if stage_shell_overrides(): + faults.append( + f"a pinned Dockerfile stage carries a `SHELL` instruction: {stage_shell_overrides()}. " + f"That redefines what every later `RUN` executes while leaving its text — and therefore " + f"the pin — unchanged." + ) + + condition = step.get("if") + if condition is None or _normalise(str(condition)) != _normalise(GATING_STEP_IF): + faults.append( + f"the gating step's `if:` is not the pinned condition.\n pinned: {GATING_STEP_IF!r}\n" + f" actual: {condition!r}\n A third term here lets the suite not run at all while the " + f"image still publishes." + ) + + # ANY spelling, not two literals. `continue-on-error: ${{ … }}` is the idiomatic conditional form + # and a check comparing against `True`/`"true"` waves it through — a presence test cannot see + # polarity, and this one is fail-OPEN in exactly the direction that matters. + job = workflow_jobs(GATING_WORKFLOW).get(GATING_JOB, {}) + for scope, holder in (("step", step), ("job", job)): + if "continue-on-error" in holder and holder["continue-on-error"] not in (False, "false"): + faults.append( + f"the gating {scope} carries `continue-on-error: {holder['continue-on-error']!r}`, " + f"so a red suite reports success and the `needs:` edge that gates the image blocks " + f"nothing. Only absent, `false` or `False` is acceptable here." + ) + + if "shell" in step: + faults.append( + f"the gating step carries its own `shell: {step['shell']!r}`, which overrides the pinned " + f"workflow default. The default is pinned because `bash` means `bash -e`; a step-level " + f"override moves that decision somewhere nothing checks." + ) + if "defaults" in job: + faults.append( + f"the job `{GATING_JOB}` carries `defaults: {job['defaults']!r}`, which overrides the " + f"pinned workflow default for every step in it, including the gating one." + ) + + if "if" in job: + faults.append( + f"the job `{GATING_JOB}` carries a job-level `if:` ({job['if']!r}). None did when " + f"ersatztv#887 made this `needs:` edge the only gate, and a job that does not run cannot " + f"gate anything — Gitea reports it `skipped`, and what that does to a dependent job is " + f"not something this repo relies on." + ) + assert not faults, "\n".join(faults) + + +def test_the_npm_SCRIPT_MAP_is_exactly_what_was_reviewed() -> None: + """The whole map, because a pinned `RUN npm …` executes whatever it says. + + An earlier version pinned only the scripts whose body contained `vitest`, which is a SELECTOR — + the category this file elsewhere calls the worst-behaved, because going short is silent. It went + short four ways, each re-arming ersatztv#887 in the gitless stage with every other pin matching: + `npm run test`, `npm t`, and the `prebuild`/`preinstall` LIFECYCLE HOOKS, which npm runs for + `npm run build` and `npm ci` without anything naming them. Pinning the map removes the category: + a script that does not exist cannot be a hook, and one that changes is not equal. + """ + scripts = json.loads(_read(PACKAGE_JSON)).get("scripts", {}) + assert scripts == PINNED_PACKAGE_SCRIPTS, ( + f"`{PACKAGE_JSON}`'s scripts are not the pinned map.\n pinned: {PINNED_PACKAGE_SCRIPTS}\n" + f" actual: {scripts}\n Both pinned `RUN npm …` lines in the gitless `web-build` stage " + f"execute whatever this says, INCLUDING npm's `pre*`/`post*` lifecycle hooks, which nothing " + f"in the Dockerfile names. A suite run added here reaches that stage with no Dockerfile or " + f"workflow line changed — ersatztv#887 through a file nobody would think to check against " + f"this guard. Update the pin deliberately, in the same commit, with a reason." + ) + + +def test_the_VITE_CONFIG_is_exactly_what_was_reviewed() -> None: + """The whole file, because every partial match of it was defeated. + + `test.exclude`/`include` decide which specs the gating run collects, and `plugins:` can shell out + to the suite from a build hook — both inside a file the Dockerfile never mentions. Pinning a + BLOCK of it was defeated seven measured ways across three review rounds: a decoy copy above + `defineConfig` with the real member respelled `test: {`, `test : {` or `"test": {`; the same for + `plugins:`; and two that never touched the marker — `[…].concat([evil])` and a trailing + `...moreTest` spread, which replaces the pinned object because `defineConfig` is identity in both + vite and vitest. Comparing the file entire has no marker to respell and nothing after the span. + + WHAT THE COMPARISON CANNOT SEE, measured: `_normalise_lines` drops blank lines and collapses + whitespace WITHIN a line, so a reflow, an indentation change and a change to the spacing inside + a STRING LITERAL are all invisible. The first two carry no meaning; the third could, and does not + here — no string in this file has consequential internal spacing. Line ORDER and any token change + are caught (both measured). Stated because "pinned whole" invites the reader to assume byte + equality, and it is not. + """ + actual = _normalise_lines(_read(VITE_CONFIG)) + pinned = _normalise_lines(PINNED_VITE_CONFIG) + assert actual == pinned, ( + f"`{VITE_CONFIG}` is not the pinned file. First difference:\n" + + "\n".join( + f" line {index + 1}:\n pinned: {pinned[index] if index < len(pinned) else ''}\n" + f" actual: {actual[index] if index < len(actual) else ''}" + for index in range(max(len(actual), len(pinned))) + if index >= min(len(actual), len(pinned)) or actual[index] != pinned[index] + ) + + "\n\n This file decides which specs the gating run collects and what `vite build` loads " + "in the gitless stage. Any edit that changes a line's TOKENS reddens — a comment's words " + "included, whitespace and blank lines not — deliberately, because every " + "partial match of it was defeated. Update `PINNED_VITE_CONFIG` in the same commit, with a " + "reason." + ) + + +def test_NO_OTHER_vitest_CONFIG_outranks_the_pinned_one() -> None: + """A pinned config is worthless if a second file can take precedence over it. + + Vitest prefers `vitest.config.*` (and `vitest.workspace.*`/`vitest.projects.*`) to + `vite.config.*`. MEASURED: dropping a `web/vitest.config.ts` with `include: ['nope/**']` and + `passWithNoTests: true` beside the pinned file made the suite collect zero specs and exit 0 — the + gating step green having run nothing at all, which is worse than the filtered run ersatztv#887 + removed. The construct is refused rather than modelled: there is no such file today, so the + honest move is to assert none appears. + """ + intruders = sorted( + path + for path in tracked_files._git_ls_files() + # DIRECT children of `web/` only: vite resolves its config from the project ROOT, so a + # `web/e2e/vite.config.ts` outranks nothing — and `web/e2e/` is a real directory. Reddening + # on it would be fail-noisy with a message that is simply false. + if path.startswith("web/") + and "/" not in path[len("web/") :] + and path.rpartition("/")[2].startswith(OUTRANKING_CONFIG_PREFIXES) + and path != VITE_CONFIG + ) + assert not intruders, ( + f"these files outrank the pinned `{VITE_CONFIG}` in vite's or vitest's config resolution: " + f"{intruders}. Whatever they say decides what the gating run COLLECTS and what `vite build` " + f"loads in the gitless stage, and the pin on `{VITE_CONFIG}` cannot see either — a suite " + f"that collects nothing still exits 0, and a plugin loaded from a `.js` config still runs. " + f"Either fold the settings into `{VITE_CONFIG}` and its pin, or pin this file too." + ) + + +def test_the_DOCS_ONLY_arm_cannot_publish_an_image() -> None: + """The premise under half of `GATING_STEP_IF`, asserted rather than assumed. + + The gating step is allowed to skip when `docs_only` is true. That is only safe because the + publish step skips on the same condition, so the arm ships nothing. Round-1 review found this + stated as fact in the prose with nothing checking it — the same shape as the defect one level in. + """ + step = find_step(GATING_WORKFLOW, "build", PUBLISH_STEP_NAME) + assert step is not None, f"{GATING_WORKFLOW}: job `build` has no step named {PUBLISH_STEP_NAME!r}." + condition = step.get("if") + assert condition is not None and _normalise(str(condition)) == _normalise(PUBLISH_STEP_IF), ( + f"the publish step's `if:` is {condition!r}, not the pinned {PUBLISH_STEP_IF!r}. The gating " + f"step is permitted to skip on `docs_only`, and that is safe ONLY because this step skips " + f"on it too. Without this, a docs-only push to `main` skips the SPA suite and publishes an " + f"image anyway." + ) + + +def test_only_the_PINNED_step_mentions_the_suite() -> None: + """A substring sweep, deliberately not a semantic predicate. + + Every earlier version tried to decide whether a command RUNS the suite and was wrong nine times. + This asks the much weaker question — does any OTHER step's shell body mention it at all — whose + failure mode is a false red asking a human to look, never a false green. + """ + strays: list[str] = [] + for path in workflow_files(): + rel = path.relative_to(REPO_ROOT).as_posix() + for job_id, job in workflow_jobs(rel).items(): + for step in _steps(job): + body = step.get("run") + if not isinstance(body, str): + continue + if ( + rel == GATING_WORKFLOW + and job_id == GATING_JOB + and str(step.get("name", "")).strip() == GATING_STEP_NAME + ): + continue + hit = [mention for mention in SUITE_MENTIONS if mention in body] + if hit: + strays.append(f"{rel} :: job `{job_id}` step {str(step.get('name', '?'))!r} mentions {hit}") + assert not strays, ( + "these steps mention the SPA suite and are not the pinned gating step:\n " + + "\n ".join(strays) + + "\n\nThis is a SUBSTRING sweep, not a claim that they run it — look, and either move the " + "run into the pinned step or record why this one is not a second, unpinned gate." + ) + + +def test_no_DOCKERFILE_outside_a_pinned_stage_mentions_the_suite() -> None: + """The same weak sweep over Dockerfiles, covering stages that carry no SPA source. + + A stage without `web/` has no suite to run, so `PINNED_STAGE_COMMANDS` does not cover it. This + reports a mention there anyway rather than leaving the class unexamined. + """ + strays: list[str] = [] + for rel in dockerfiles(): + for name, lines in stages(_read(rel)): + if f"{rel}::{name}" in PINNED_STAGE_COMMANDS: + continue + for line in lines: + hit = [mention for mention in SUITE_MENTIONS if mention in line] + if hit: + strays.append(f"{rel} :: stage `{name}`: {line!r} mentions {hit}") + assert not strays, "these unpinned Dockerfile stages mention the SPA suite:\n " + "\n ".join(strays) + + +# ------------------------------------------------------------------------------------------------ +# Anti-vacuity and negative controls +# ------------------------------------------------------------------------------------------------ + + +def test_no_run_BODY_builds_or_pushes_an_image() -> None: + """`publishing_jobs()` keys on the ACTION, which is a selector; assert nothing publishes around it. + + A job doing `docker build -f ./docker/Dockerfile … && docker push …` in a shell body is not in + that population, so the positive invariant is silently not applied to it — and anti-vacuity does + not notice, because the real `build` job is still there. Measured MISSED by cold review after + this assertion was dropped in the rewrite; it is restored rather than reasoned away. + """ + stray: list[str] = [] + for path in workflow_files(): + rel = path.relative_to(REPO_ROOT).as_posix() + for job_id, job in workflow_jobs(rel).items(): + for step in _steps(job): + body = step.get("run") + if not isinstance(body, str): + continue + for line in body.splitlines(): + if line.lstrip().startswith("#"): + continue + if re.search(r"\bdocker\s+(?:push|build|buildx\s+build)\b", line): + stray.append(f"{rel} :: job `{job_id}`: {line.strip()!r}") + assert not stray, ( + f"image build/push outside `{PUBLISH_ACTION}`:\n " + "\n ".join(stray) + "\n\n" + "`publishing_jobs()` derives from that action, so these jobs are NOT checked for the suite " + "gate — extend it before adding one." + ) + + +def test_the_populations_are_NOT_empty() -> None: + """Every assertion above passes over an empty set, which is how a completeness guard dies.""" + assert dockerfiles(), "`git ls-files` reported no Dockerfile — the derivation is broken, not the repo." + assert workflow_files(), f"`git ls-files` reported no {WORKFLOW_DIR}/*.yml — the derivation is broken." + + carrying = spa_carrying_stages() + assert carrying, ( + f"no stage in {dockerfiles()} was read as carrying the SPA source, so the negative assertion " + "checked nothing and the positive one skipped every job. `copies_spa_source` or `stages` " + "stopped matching." + ) + assert PINNED_STAGE_COMMANDS, "the pin table is empty; the negative assertion asserts nothing." + + published = publishing_jobs() + assert published, ( + f"no job in {WORKFLOW_DIR} uses `{PUBLISH_ACTION}`, so the gate assertion ran over an empty " + "population. Either the repo stopped publishing images or `_canonical_action` stopped " + "recognising the action." + ) + assert any(dockerfile in {rel for rel, _, _ in carrying} for _, _, dockerfile, _ in published), ( + "no publishing job builds an SPA-carrying Dockerfile, so the gate assertion is vacuous." + ) + + +def test_the_STAGE_SPLIT_finds_the_real_stages() -> None: + """`stages()` is the only Dockerfile syntax this file interprets; pin what it returns. + + A split that stopped recognising `FROM … AS name` would put every instruction in one bucket, the + pin key would never match, and `test_every_SPA_CARRYING_STAGE_runs_exactly_its_pinned_commands` + would report an orphan pin — loud, but for the wrong reason. Pinned so the cause is legible. + """ + names = [name for name, _ in stages(_read("docker/Dockerfile"))] + assert names[:3] == ["dotnet-runtime", "web-build", "runtime-base"], names + assert [stage for _, stage, _ in spa_carrying_stages()] == ["web-build"], spa_carrying_stages() + + +def test_the_SPA_CARRYING_predicate_reads_every_COPY_SPELLING() -> None: + """Pinned both ways: every stage answered False is SKIPPED by the negative assertion. + + The synthetic spellings were MEASURED False by cold review before this was widened; a new stage + using any of them would have gone unpinned and unchecked. + """ + for spelling in ( + 'COPY ["web/", "/source/web/"]', + 'COPY --chown=node:node ["web/", "/source/web/"]', + "COPY web /source/web", + "ADD web/ /source/web/", + "COPY web/. ./web/", + "COPY . /source/", + "COPY ./ /source/", + "COPY --chown=node:node web/ /source/web/", + "COPY web/package*.json ./web/", + # A GLOB that matches `web/`. Measured False before `_source_reaches_web`, which left a + # stage copying the source this way unpinned and unchecked. + "COPY web*/ ./", + "COPY * /source/", + ): + assert copies_spa_source(spelling), spelling + + # A STAGE copy can still carry the SPA source across. Measured False before this clause: the + # receiving stage was then unpinned, so a suite run in it was invisible. + assert copies_spa_source("COPY --from=web-build /source/web /source/web") + assert copies_spa_source("COPY --from=builder /app/web ./web") + # The EXEC form of the same thing. Measured False before the parse order was fixed — splitting + # `["/source/web", "/dest"]` on whitespace left a trailing comma on the first token, so the + # segment test never matched. Pinned here because only the development battery witnessed it, and + # that battery is explicitly not standing. + assert copies_spa_source('COPY --from=web-build ["/source/web", "/dest"]') + assert copies_spa_source('COPY --from=web-build ["/source/web","/dest"]') + assert not copies_spa_source('COPY --from=web-build ["/source/ErsatzTV/wwwroot/app/.", "/dest"]') + + for spelling in ( + "COPY design-system/. ./design-system/", + "COPY --from=web-build /source/ErsatzTV/wwwroot/app/. ./app/", + "COPY webhooks/. ./webhooks/", # `web` must be a whole path segment + "RUN npm ci", + "COPY web/", # a single argument is not a copy + ): + assert not copies_spa_source(spelling), spelling + + +def test_the_INSTRUCTION_JOIN_survives_what_defeated_the_PARSER() -> None: + """The shapes that broke three earlier versions must not break the pin comparison. + + None of these needs interpreting — they only need to arrive at the comparison intact, so that a + stage running them is NOT EQUAL to its pin. Each was a measured false green when this file tried + to decide what the command meant. + """ + heredoc = "RUN < None: + """The nine shapes that defeated the predicate, run against the pin instead. + + Not one of them is equal to `RUN npm run lint && npm run typecheck && npm run build`, which is + the entire argument for replacing the mechanism: the pin does not need to recognise a spelling to + reject it. + """ + pinned = PINNED_STAGE_COMMANDS["docker/Dockerfile::web-build"] + for spelling in ( + "RUN < { // Residual: this asserts the message the MISSING-BINARY path also emits, so in an environment with // no git it passes for the wrong reason. Not silent overall — its two siblings fail ENOENT there, - // loudly — and that is exactly the environment this file is excluded from in `docker/Dockerfile`. + // loudly. Since ersatztv#887 no such environment runs this suite: `docker/Dockerfile` stopped + // running it rather than excluding the files that cannot run, so the residual is now reachable + // only on a developer machine with no git installed. it('really throws outside a repository, rather than returning an empty population', () => { const notARepo = mkdtempSync(join(tmpdir(), 'etv-819-bare-')); repos.push(notARepo); diff --git a/web/vite-plugins/trackedSourceFiles.ts b/web/vite-plugins/trackedSourceFiles.ts index 1624307fe..ede5ddbe0 100644 --- a/web/vite-plugins/trackedSourceFiles.ts +++ b/web/vite-plugins/trackedSourceFiles.ts @@ -150,7 +150,9 @@ export function trackedSourceFilesPlugin(runGit?: GitRunner, fileExists?: FileEx return id === TRACKED_SOURCE_FILES_ID ? RESOLVED_ID : undefined; }, // Deliberately LAZY: git runs only when something imports the virtual module. `vite build` - // never resolves this id and so never shells out to git. + // never resolves this id and so never shells out to git. LOAD-BEARING FOR THE IMAGE BUILD since + // ersatztv#887: `docker/Dockerfile`'s web-build stage runs `npm run build` in a stage with no git + // binary, so making this eager would break every image build for #887's original root cause. load(id) { if (id !== RESOLVED_ID) { return undefined;