From 8aeacd534a8501343ee07cda249518e8c0ccd7cc Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 29 Aug 2026 09:14:15 +0000 Subject: [PATCH] fix(819): derive the SPA page-size guard population from the git index (#875) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guard asserted EXACT completeness over a population enumerated by a directory walk, so an untracked .ts/.tsx under web/src/ entered it and failed as unregistered on that developer's checkout while CI — which only ever checks out tracked files — stayed green. The glob still supplies file CONTENT; the POPULATION is now the git index, read by web/vite-plugins/trackedSourceFiles.ts in Vite's own Node context and handed to the app project as a virtual module. That reaches the index without admitting @types/node to tsconfig.app.json, the obstacle that deferred this in #818. Three mechanisms carry the proof, each added because the previous was measured insufficient: a closed-form restatement of the shared scope predicate (sharing no helper at any depth with what it checks); a second independent `ls-files --others` query cross-checking the population; and real-git tests that execute the derivation against a temp repository. Six residuals are stated with their MEASURED fail-directions, and testing.guard-derives-population-from-source gains a bounded exception plus the closed-form criterion. fixes #819 Co-authored-by: Timothy --- docker/Dockerfile | 32 +- docs/decisions/README.md | 2 +- .../guard-derives-population-from-source.md | 35 +- docs/guard-inventory.md | 23 +- docs/spa-conventions.md | 6 + docs/testing.md | 10 +- web/src/api/pageSizeCallSites.guard.test.ts | 600 +++++++++++++++++- web/src/vite-env.d.ts | 23 + web/tsconfig.node.json | 2 +- .../trackedSourceFiles.realgit.test.ts | 97 +++ web/vite-plugins/trackedSourceFiles.test.ts | 176 +++++ web/vite-plugins/trackedSourceFiles.ts | 167 +++++ web/vite.config.ts | 3 +- 13 files changed, 1149 insertions(+), 27 deletions(-) create mode 100644 web/vite-plugins/trackedSourceFiles.realgit.test.ts create mode 100644 web/vite-plugins/trackedSourceFiles.test.ts create mode 100644 web/vite-plugins/trackedSourceFiles.ts diff --git a/docker/Dockerfile b/docker/Dockerfile index 1cefcf67f..dd8eac5f3 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -9,7 +9,37 @@ WORKDIR /source COPY design-system/. ./design-system/ COPY web/. ./web/ WORKDIR /source/web -RUN npm run lint && npm run typecheck && npm test -- --run && npm run build +# 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 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 ffefc6e75..85dd76ade 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -210,7 +210,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `testing.enumerating-guard-identity-not-position` | A guard that cross-checks a hand-reviewed registry against call sites discovered across the whole repo must key each entry on properties INTRINSIC to the site — file, kind, and the value source text — and never on its absolute line or column. A registry keyed on position is a function of every other file in the repo, so a branch that never touches the guard can invalidate it; and because each PR is green against its own base, that failure is structurally invisible pre-merge and lands on `main` after review and after the merge gate. Dropping the position keeps every mutation the guard exists for — a NEW site, a REMOVED site and a CHANGED value each still fail, since each changes the identity multiset — and costs exactly ONE case, which must be stated rather than implied: a SAME-IDENTITY SUBSTITUTION within one file (delete a registered site, add a different unreviewed one with the same kind and value token, net-zero count) now passes. A REPORTED failure still prints the discovered line:column, because identity and diagnostics need not share a format. Comparison stays a MULTISET count rather than set membership, so two sites in one file sharing an identity must be discovered exactly that many times and a third occurrence still fails. A SCANNER test that asserts real AST positions against FIXED inline fixtures is the opposite case and keeps its line/column identity — it has no churn, because its input does not move. | 2026-07-27 | [link](records/testing/enumerating-guard-identity-not-position.md) | | `testing.fix-ships-a-witnessed-red-test` | A commit claiming to fix something may carry a `Proves: ` trailer; when it does, `scripts/prove-fix.sh` must show that selector GREEN with the fix and RED with the code side reverted, and CI enforces it per-PR. The trailer is opt-in — an unproven commit is allowed — but a claimed proof that does not hold fails the build. | 2026-08-16 | [link](records/testing/fix-ships-a-witnessed-red-test.md) | | `testing.full-replace-asserts-field-list` | Any path that writes a WHOLE entity or a WHOLE child collection — a PUT-replace handler, a hand-built request object, a test comparer standing in for one — derives its field list from the authoritative type and asserts SET EQUALITY against it, rather than enumerating the fields by hand. A hand-written list is correct on the day it is written and structurally unable to report the day it stops being: the field that drifts is the one nobody wrote a line for, so no amount of care in the existing lines can reach it. The failure is silent by construction — a full replace with a field omitted returns HTTP 200 and destroys that field's value (#754 drifted from a 28-property DTO by one and cleared it; the symptom arrived hours later as missing pixels). SECOND CLAUSE, separable from the first: where a replaced child row carries state keyed to its identity — progression, ordering, an enumerator position — the handler RECONCILES BY ID rather than delete-and-reinsert, because reinsertion silently resets state a client never asked to touch (#252: a schedule PUT reset fill-group progression; #500: a dedup fix became permanent data loss because the add filter and the remove filter used different keys, so the two halves must agree on the key). Delete-and-reinsert is acceptable ONLY where no such state exists, and that emptiness is a fact about today's schema that a later feature can silently invalidate — so record it where the handler is, dated, rather than leaving it to be re-derived. The canonical worked example is `ToolCatalogTests.Every_Write_Tool_Should_Declare_Exactly_Its_OpenApi_Request_Body_Fields`, which reads the accepted fields from the generated OpenAPI document and compares both directions. ON THE SPA SIDE the same rule is enforced by the TYPE SYSTEM rather than by a test: a full-replace body is built as `Complete` (`web/src/api/completeRequest.ts`), a mapped type that makes every member of a generated request type required, so a builder that omits one fails `npm run typecheck`. Annotate BOTH the API-wrapper parameter (so later callers inherit it) AND each construction site including every `.map` callback return type, because the excess-property check that catches a PHANTOM field fires only on a fresh literal in a contextually typed position and a generic `.map` callback is not one. Do not infer from "most builders already typecheck" that the gap is closed: a member is omittable exactly when it is absent from the schema `required` array in the ASP.NET-produced OpenAPI document, and two live cases (#807) sat unchecked inside a large majority of checked ones. | 2026-08-21 | [link](records/testing/full-replace-asserts-field-list.md) | -| `testing.guard-derives-population-from-source` | A guard that asserts a COMPLETENESS property enumerates its population from a machine-readable authoritative source — the enum, the generated OpenAPI document, the parsed workflow YAML, the provider list — and asserts SET EQUALITY in BOTH directions against it. It may not narrow that population with a filter, a `Where`, a `grep` or an early `continue` before the assertion, because a filter cannot see the member that is MISSING: the member whose absence is the defect is precisely the one the predicate excludes. A hand-written literal list of members is the same defect in slower motion — a filter frozen at authoring time, correct on the day it was written and unable to report the day it stopped being. Two boundaries bound the rule rather than weaken it. FIRST, filtering to select the SUBJECT of a PER-MEMBER property is legitimate and is not this defect: the excluded members satisfy the property vacuously, so the filtered walk and the whole walk assert the same thing (`ToolCatalogTests.Every_Query_Parameter_Should_Be_A_Declared_Property` filters to tools that declare query parameters, and a tool declaring none has nothing to check). The defect is filtering the population before a COMPLETENESS claim, which is what makes an absent member unrepresentable (#757 filtered on `QueryParameters is {Count: > 0}` and so could not see a tool that should have declared one and did not). SECOND, a population of VALUES always has an external authoritative source and this rule applies directly; a population of SITES IN CODE has no such list, needs find-all-references tooling, and is tracked separately in #777 — do not stretch a set-equality assertion over it. Distinguish the guard SCOPE (which subsystems it covers — a reviewed policy choice, legitimately hand-written) from the guard POPULATION (the members inside that scope — always derived). When the scope itself MIRRORS an authoritative source, the mirror needs its own equality check against that source, or the guard is complete within a scope that has silently gone stale. A DATED STALENESS MARKER does not discharge this and is no longer offered as an alternative (#787): a date records when someone last looked, so it goes stale in exactly the circumstance it exists to report, and nothing reddens when it does. The canonical worked example in this repo is `ToolCatalogTests.Every_Tool_Should_Declare_Exactly_Its_OpenApi_Query_Parameters`. When a scope mirrors an authoritative source across a CREDENTIAL BOUNDARY, split the invariant at that boundary rather than dating a comment: commit the mirror verbatim, derive the scope from it offline, and reconcile the mirror against the live source wherever a credential exists — `MARKED_JOBS` in `scripts/tests/test_ci_dropped_step_guard.py` was this repo's residual gap until #787 closed it that way. WHEN THE POPULATION IS FILES (#806), the authoritative source is the GIT INDEX and never a filesystem walk. A walk is not merely a weaker enumerator, it answers a question about the MACHINE rather than about the repo: it reports build output, generated shims and editor droppings, and it differs between CI and every checkout, so the same guard asserts a different population in each place. Derive with `git ls-files`, take direct children only unless a nested population is stated and wanted, and assert existence rather than filtering on it, because filtering is what makes a missing member unrepresentable. This is an instantiation and not a blanket rewrite: the question per guard remains whether it makes a COMPLETENESS claim over TRACKED files, and a walk that assembles a fixture or selects the SUBJECT of a per-member property stays a walk with its reason written down. | 2026-08-13 | [link](records/testing/guard-derives-population-from-source.md) | +| `testing.guard-derives-population-from-source` | A guard that asserts a COMPLETENESS property enumerates its population from a machine-readable authoritative source — the enum, the generated OpenAPI document, the parsed workflow YAML, the provider list — and asserts SET EQUALITY in BOTH directions against it. It may not narrow that population with a filter, a `Where`, a `grep` or an early `continue` before the assertion, because a filter cannot see the member that is MISSING: the member whose absence is the defect is precisely the one the predicate excludes. A hand-written literal list of members is the same defect in slower motion — a filter frozen at authoring time, correct on the day it was written and unable to report the day it stopped being. Two boundaries bound the rule rather than weaken it. FIRST, filtering to select the SUBJECT of a PER-MEMBER property is legitimate and is not this defect: the excluded members satisfy the property vacuously, so the filtered walk and the whole walk assert the same thing (`ToolCatalogTests.Every_Query_Parameter_Should_Be_A_Declared_Property` filters to tools that declare query parameters, and a tool declaring none has nothing to check). The defect is filtering the population before a COMPLETENESS claim, which is what makes an absent member unrepresentable (#757 filtered on `QueryParameters is {Count: > 0}` and so could not see a tool that should have declared one and did not). SECOND, a population of VALUES always has an external authoritative source and this rule applies directly; a population of SITES IN CODE has no such list, needs find-all-references tooling, and is tracked separately in #777 — do not stretch a set-equality assertion over it. Distinguish the guard SCOPE (which subsystems it covers — a reviewed policy choice, legitimately hand-written) from the guard POPULATION (the members inside that scope — always derived). When the scope itself MIRRORS an authoritative source, the mirror needs its own equality check against that source, or the guard is complete within a scope that has silently gone stale. A DATED STALENESS MARKER does not discharge this and is no longer offered as an alternative (#787): a date records when someone last looked, so it goes stale in exactly the circumstance it exists to report, and nothing reddens when it does. The canonical worked example in this repo is `ToolCatalogTests.Every_Tool_Should_Declare_Exactly_Its_OpenApi_Query_Parameters`. When a scope mirrors an authoritative source across a CREDENTIAL BOUNDARY, split the invariant at that boundary rather than dating a comment: commit the mirror verbatim, derive the scope from it offline, and reconcile the mirror against the live source wherever a credential exists — `MARKED_JOBS` in `scripts/tests/test_ci_dropped_step_guard.py` was this repo's residual gap until #787 closed it that way. WHEN THE POPULATION IS FILES (#806), the authoritative source is the GIT INDEX and never a filesystem walk. A walk is not merely a weaker enumerator, it answers a question about the MACHINE rather than about the repo: it reports build output, generated shims and editor droppings, and it differs between CI and every checkout, so the same guard asserts a different population in each place. Derive with `git ls-files`, take direct children only unless a nested population is stated and wanted, and assert existence rather than filtering on it, because filtering is what makes a missing member unrepresentable. ONE BOUNDED EXCEPTION to that existence half (#819): a guard MAY filter on existence where the OTHER direction reports the on-disk hole — where a tracked path that never reached the walk is itself compared and NAMED — since what makes filtering dangerous is the absent member becoming unrepresentable, and a paired assertion represents it. That buys tolerance of a state the assert-existence form cannot distinguish, which matters where the population is large and edited continuously and an unstaged deletion is routine rather than remarkable. Filtering on existence WITHOUT that paired direction stays the original defect. AND, where a guard PROVES a predicate by restating it (#819), that restatement must be CLOSED FORM over its raw input: it may share NO helper, at ANY depth, with the predicate it checks, and the population it reads must be cross-checked against an INDEPENDENTLY derived list. Anything shared sits on both sides of the comparison and cancels, so a narrowing there shrinks both and passes — five review rounds each fixed one shared thing and left the next: the scope shared between the two sides being compared, then an example table, then a delegated sub-predicate, then a `basename` helper, then the population array itself. Where the population comes from a derivation you also own, cross-check it against a SECOND query of that source rather than restating anything — and accept that a derivation lying consistently in both outputs is answerable only by testing the derivation directly. This is an instantiation and not a blanket rewrite: the question per guard remains whether it makes a COMPLETENESS claim over TRACKED files, and a walk that assembles a fixture or selects the SUBJECT of a per-member property stays a walk with its reason written down. | 2026-08-13 | [link](records/testing/guard-derives-population-from-source.md) | | `testing.guard-ships-with-mutation-proof` | A guard is not considered tested because a test involving it passes. It ships with a MUTATION PROOF: remove or disarm THAT GUARD'S CLAUSE ALONE, and a NAMED test must go red. ONE NAMED EXCEPTION, with its limits, because the rule degenerates without it: where the guard IS a test (a checker enforcing a repo invariant, with no separate script behind it), disarming it makes it ABSENT rather than red, so the proof is the contrapositive — INTRODUCE THE DEFECT THE GUARD EXISTS TO CATCH into an isolated copy of the guarded artifact, and the named test must go red. That is a mutation of the guarded SYSTEM rather than of the assertion, and it is admissible ONLY for checker-guards and ONLY when the mutation was executed and witnessed. It is NOT a licence to grade an ordinary script-guard MUTATION for having a bad-input test: feeding a script an input its clause rejects is BEHAVIOUR-ONLY, which is what three rows were regraded for. A file-level grade under this exception covers the clause its cited case actually mutates, not every assertion that later lands in the same file. Three things this excludes, each of which has already shipped here as a green suite over a dead check. FIRST, a behavioural test — one that feeds the guard a good input and a bad input and checks it passes and fails — proves the guard REACTS, never that it is LOAD-BEARING; #685 had two guards on one condition where deleting either left the whole suite green while every behavioural test passed. SECOND, mutating the WHOLE FILE does not count (#510): a whole-file revert cannot show that a test reaches a particular clause, so the mutation must target the clause. THIRD, the guard being WIRED is not the guard RUNNING — #631's suite was invoked by no CI job, #751's step was dropped by the runner and the job reported success in 6s against a normal 14-17s, and #719's new logic was never connected to stdin. Every guard that DERIVES A POPULATION also carries an ANTI-VACUITY assertion, because the characteristic failure of a completeness check is reporting that it proved everything while its population was empty; a guard with no population has nothing for such an assertion to be about, and stating it universally reads as coverage the unproven rows do not have. Mechanical enforcement is possible for the BOOKKEEPING and not for the JUDGEMENT, and the split is the decision: `docs/guard-inventory.md` lists every guard file with its Kind, its Proof class (`MUTATION`/`BEHAVIOUR-ONLY`/`NONE`) and a `file::function` ref, and `scripts/tests/test_guard_inventory.py` derives the guard population from the GIT INDEX and the call sites (#806), asserts SET EQUALITY against the rows, and resolves every claimed ref to a real `def`. So a new guard cannot ship unclassified and a renamed test cannot leave a row silently claiming coverage. Whether a row claiming `MUTATION` is telling the truth is no longer left to review: `testing.mutation-claims-are-executed` (#790) requires each such row to carry a DECLARED clause mutation that is applied to an isolated copy of the repository on every run, with the row's own named test required to go red. | 2026-08-13 | [link](records/testing/guard-ships-with-mutation-proof.md) | | `testing.hook-reports-its-own-execution` | Every script in `.claude/hooks/` sources `scripts/hook-fire-log.sh` and calls `etv_hook_fire_begin