From 63afc5f743ec35974f71298ca0377824dfb373b4 Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 28 Aug 2026 22:22:13 +0200 Subject: [PATCH 01/17] fix(819): derive the SPA page-size guard's population from the git index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pageSizeCallSites.guard.test.ts` asserts EXACT completeness over the files it discovers, but discovered them with `import.meta.glob` over `web/src`. A directory walk is not an authoritative source: an untracked `.ts`/`.tsx` — a scratch file, a half-finished component, an editor dropping — entered the population and failed as unregistered on that checkout, while CI, which only ever checks out tracked files, stayed green. Developer-red / CI-green, the #778 shape #806 removed from every other guard here. Witnessed before the fix: planting an untracked `Scratch819Untracked.tsx` with a `pageSize: 25` literal reddened the guard with `UNREGISTERED (1) — screens/Scratch819Untracked.tsx:literal:25`. The glob stays, as the source of file CONTENT. The POPULATION is now `git ls-files`, 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 in #818, since wiring those types in was tried and reverted (it leaked Node's ambient `setTimeout` into the app project and broke three unrelated `window.setTimeout` mocks). The plugin fails rather than degrading: a failed or empty `git ls-files` throws, because answering "could not tell" with the very population the index replaced is a permanent no-op that reports green (#772's shape). It reads the index lazily, inside `load()`, so `vite build` — including the Docker image build, whose context carries no `.git` — never shells out to git. Verified by building in a copy of `web/` outside any repository. Proofs, all executed: - clause mutation: deleting `if (!isTrackedSourcePath(...))` from `listSourceFilesFrom` reddens exactly one named test, `EXCLUDES an untracked file that matches the scope in every other way (#819)`, and nothing else. The pipeline was extracted to take injected inputs precisely so that clause is provable — over real source on a clean checkout it removes nothing, so a plant-a-real-file check pins nothing, the same argument `isScannableSourceFileName` already records. - negative control: the same planted file, once `git add`ed, is caught again. Untracked is ignored, tracked is caught — the guard did not go blind. - positive control against a clause mutated to reject everything. - anti-vacuity on the consuming side: the real index is >50 paths and contains named known files, so a plugin that stopped being registered cannot pass an empty population off as a satisfied one. Residual, stated rather than implied: a file tracked but DELETED in the working tree has no content and drops out. Not asserted — an unstaged deletion is a normal developer state, and a guard red on one gets ignored (#806) — and a REGISTERED site in such a file still fails loudly on the STALE direction. fixes #819 --- docs/guard-inventory.md | 2 +- web/src/api/pageSizeCallSites.guard.test.ts | 125 +++++++++++++++++++- web/src/vite-env.d.ts | 9 ++ web/tsconfig.node.json | 2 +- web/vite-plugins/trackedSourceFiles.test.ts | 95 +++++++++++++++ web/vite-plugins/trackedSourceFiles.ts | 91 ++++++++++++++ web/vite.config.ts | 3 +- 7 files changed, 318 insertions(+), 9 deletions(-) create mode 100644 web/vite-plugins/trackedSourceFiles.test.ts create mode 100644 web/vite-plugins/trackedSourceFiles.ts diff --git a/docs/guard-inventory.md b/docs/guard-inventory.md index a2a1d4ffe..6dc4ed59a 100644 --- a/docs/guard-inventory.md +++ b/docs/guard-inventory.md @@ -155,7 +155,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 and found by cold review on the pushed head: an untracked `.yaml` dropped in `.gitea/workflows/` reddened two guards while absent from the index — the issue's list of files to assess was a starting point, not the population | | `test_workflow_persist_credentials.py` | `.gitea/workflows/*.yml` + `*.yaml` | **yes** — every `actions/checkout` in every workflow must drop the persisted credential | derived from the index from the start (#835). Shipped with NO exemption list, which is why it waited for #744: the one non-compliant checkout would otherwise have bought a permanent entry that keeps passing after its reason expires | -| `web/src/api/pageSizeCallSites.guard.test.ts` | `web/src/**/*.{ts,tsx,mts,cts}` via `import.meta.glob` | **yes** — an unregistered discovered site fails | DEFERRED to #819, assessed not skipped. The glob is a documented workaround: `@types/node` is deliberately out of `tsconfig.app.json`, and wiring it in was tried and reverted (it leaked Node's `setTimeout` into the app project and broke three unrelated tests), so there is no `node:child_process` to reach the index from. Over-enumerates, so it fails loudly rather than going blind | +| `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 glob still supplies file CONTENT; the POPULATION is `git ls-files`, 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 or reports zero files. Residual, stated: a file that is tracked but DELETED in the working tree has no content and drops out. Not asserted — an unstaged deletion is a normal developer state and a guard red on one gets ignored (#806) — and a REGISTERED site in such a file still fails loudly on the STALE direction | | `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 | | `test_ci_dropped_step_guard.py` | the parsed workflow document (its OWN subject only — the release-path `scan` job runs it in a non-git copy, so it may not derive from the index) | **no filesystem population at all** | unchanged by #806; the SCOPE residual it left open — `MARKED_JOBS` as a hand-written mirror of the required contexts on `main` — was closed by #787, which derives it from `.gitea/required-status-contexts.json` and reconciles that snapshot against the server in `scripts/check-required-contexts.sh`. The cross-workflow half of that claim lives in `test_ci_status_context_uniqueness.py`, which CAN use the index | diff --git a/web/src/api/pageSizeCallSites.guard.test.ts b/web/src/api/pageSizeCallSites.guard.test.ts index d729f0e3a..a175f82a0 100644 --- a/web/src/api/pageSizeCallSites.guard.test.ts +++ b/web/src/api/pageSizeCallSites.guard.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from 'vitest'; +import trackedSourceFiles from 'virtual:etv-tracked-source-files'; import { scanPageSizeSites } from './pageSizeScan'; /** @@ -339,7 +340,7 @@ const REGISTRY: RegistryEntry[] = [ } ]; -// Enumerates every source file under `src/` via Vite's `import.meta.glob` — eagerly, as raw text +// Reads every source file under `src/` via Vite's `import.meta.glob` — eagerly, as raw text // (`query: '?raw', import: 'default'`) — INSTEAD OF Node's `fs`/`path`/`url` (#650 follow-up). // This is the only file under `src` that ever needed real filesystem access, and `@types/node` // isn't wired into `tsconfig.app.json`'s project (deliberately: it covers production browser code @@ -347,14 +348,46 @@ const REGISTRY: RegistryEntry[] = [ // single-program compilation it leaked Node's ambient `setTimeout` into the whole app project, // breaking three unrelated `window.setTimeout` mocks that expect the DOM signature). `import.meta // .glob` needs neither `node:fs` nor a tsconfig change: it's resolved by Vite at transform time, -// natively available in the browser/app project, and is the idiomatic Vite/vitest way to enumerate -// source files. Keys are POSIX paths from the project root, e.g. `/src/api/pageSizeScan.ts`. +// natively available in the browser/app project. Keys are POSIX paths from the project root, e.g. +// `/src/api/pageSizeScan.ts`. +// +// The glob supplies CONTENT, not the POPULATION (#819). A directory walk is not an authoritative +// source: an untracked `.ts`/`.tsx` under `web/src/` — a scratch file, a half-finished component, +// an editor dropping — used to enter the population and fail as unregistered on that checkout, +// while CI, which only ever checks out tracked files, stayed green. Developer-red / CI-green, the +// #778 shape that #806 removed from every other guard here. So the keys are intersected with the +// git index (`virtual:etv-tracked-source-files`, resolved by `vite-plugins/trackedSourceFiles.ts` +// in Vite's Node context, which is how the index is reached without admitting `@types/node` to the +// app project). const rawSourceModules = import.meta.glob('/src/**/*.{ts,tsx,mts,cts}', { query: '?raw', import: 'default', eager: true }) as Record; +/** + * The git index, as the guard compares it: paths relative to the Vite root (`web/`), e.g. + * `src/api/pageSizeScan.ts`. Built once — membership is asked per globbed file. + */ +const TRACKED_SOURCE_PATHS: ReadonlySet = new Set(trackedSourceFiles); + +/** `import.meta.glob` keys are root-absolute (`/src/...`); the index is root-relative. */ +export function globKeyToRootRelativePath(key: string): string { + return key.startsWith('/') ? key.slice(1) : key; +} + +/** + * The #819 fix, extracted so it is pinned BY ITSELF rather than by the repo happening to be clean. + * + * Testing this by planting a real untracked file proves the behaviour exists today and pins + * nothing — the same argument `isScannableSourceFileName` records above, and the reason that + * predicate was extracted too. A guard whose proof depends on the checkout's incidental contents + * cannot report the day it stopped working. + */ +export function isTrackedSourcePath(key: string, tracked: ReadonlySet): boolean { + return tracked.has(globKeyToRootRelativePath(key)); +} + function basename(path: string): string { const idx = path.lastIndexOf('/'); return idx === -1 ? path : path.slice(idx + 1); @@ -386,20 +419,49 @@ interface ScannableSource { text: string; } -function listSourceFiles(): ScannableSource[] { +/** + * The discovery pipeline, over INJECTED inputs so each of its filters can be mutated and witnessed + * (`testing.guard-ships-with-mutation-proof`). `listSourceFiles` below is the real-source caller. + * + * Extracted for the #819 tracked-file clause specifically: with a clean checkout the clause is a + * no-op over real inputs, so deleting it from a `listSourceFiles` that reads only real source + * leaves the whole suite green — the guard would ship unproven, which is the exact failure this + * repo has recorded six times. + */ +export function listSourceFilesFrom( + modules: Record, + tracked: ReadonlySet +): ScannableSource[] { const out: ScannableSource[] = []; - for (const [key, text] of Object.entries(rawSourceModules)) { + for (const [key, text] of Object.entries(modules)) { if (key.includes('/generated/')) { continue; } if (!isScannableSourceFileName(basename(key))) { continue; } + // #819: the population is the git index, not the directory. This narrows the glob's results; + // it is NOT the "filter before a completeness claim" that + // `testing.guard-derives-population-from-source` forbids — that rule forbids narrowing the + // population so the ABSENT member becomes unrepresentable, and this restores the population to + // the authoritative source the rule names. The residual, stated rather than implied: a file + // that is tracked but DELETED in the working tree has no content to scan and so drops out + // here. That direction is deliberately not asserted, because an unstaged deletion is a normal + // developer state and a guard red on one teaches its readers to ignore it (#806). It is also + // self-announcing for anything that matters: a deleted file holding a REGISTERED site fails + // loudly on the STALE direction below. + if (!isTrackedSourcePath(key, tracked)) { + continue; + } out.push({ file: key.replace(/^\/src\//, ''), text }); } return out; } +function listSourceFiles(): ScannableSource[] { + return listSourceFilesFrom(rawSourceModules, TRACKED_SOURCE_PATHS); +} + interface DiscoveredSite { file: string; line: number; @@ -512,7 +574,58 @@ describe('pageSize call-site guard (#650)', () => { } ); - it('scans a healthy number of source files (anti-vacuity: a broken glob must not pass on zero input)', () => { + // ---- #819: the population is the git index, not the directory walk ------------------- + // + // These four are the CLAUSE proof for the tracked-file filter. Over real source on a clean + // checkout that clause removes nothing, so nothing below may depend on real source: delete + // `if (!isTrackedSourcePath(...))` from `listSourceFilesFrom` and the first of these goes red on + // its own, which is what `testing.guard-ships-with-mutation-proof` requires and what a + // plant-a-real-file check could never give. + + it('EXCLUDES an untracked file that matches the scope in every other way (#819)', () => { + const modules = { + '/src/screens/Tracked.tsx': 'const a = { pageNum: 0, pageSize: 25 };', + '/src/screens/Untracked.tsx': 'const b = { pageNum: 0, pageSize: 25 };' + }; + const tracked = new Set(['src/screens/Tracked.tsx']); + + expect(listSourceFilesFrom(modules, tracked).map((source) => source.file)).toEqual(['screens/Tracked.tsx']); + }); + + it('INCLUDES both when both are tracked — the filter keys on the index, not on the name', () => { + // Positive control. Without it, a clause mutated to reject everything would still satisfy the + // exclusion test above while emptying the population entirely. + const modules = { + '/src/screens/Tracked.tsx': 'const a = { pageNum: 0, pageSize: 25 };', + '/src/screens/AlsoTracked.tsx': 'const b = { pageNum: 0, pageSize: 25 };' + }; + const tracked = new Set(['src/screens/Tracked.tsx', 'src/screens/AlsoTracked.tsx']); + + expect(listSourceFilesFrom(modules, tracked).map((source) => source.file).sort()).toEqual([ + 'screens/AlsoTracked.tsx', + 'screens/Tracked.tsx' + ]); + }); + + it('normalises a root-absolute glob key to the root-relative path the index uses', () => { + expect(globKeyToRootRelativePath('/src/api/pageSizeScan.ts')).toBe('src/api/pageSizeScan.ts'); + // Already relative — idempotent rather than stripping a second character. + expect(globKeyToRootRelativePath('src/api/pageSizeScan.ts')).toBe('src/api/pageSizeScan.ts'); + }); + + it('receives a REAL git index from the Vite plugin (the population is wired, not just declared)', () => { + // "The guard being WIRED is not the guard RUNNING." If `vite-plugins/trackedSourceFiles.ts` + // stopped being registered, or handed back an empty list, every completeness claim below would + // pass over an empty population while looking identical to a satisfied one. The plugin throws + // on a failed or empty `git ls-files`; this is the assertion on the consuming side. + expect(TRACKED_SOURCE_PATHS.size).toBeGreaterThan(50); + expect(TRACKED_SOURCE_PATHS.has('src/api/pageSizeScan.ts')).toBe(true); + expect(TRACKED_SOURCE_PATHS.has('src/api/pageSizeCallSites.guard.test.ts')).toBe(true); + // ...and it is a real index rather than "every path anyone asks about". + expect(TRACKED_SOURCE_PATHS.has('src/screens/NoSuchFile819.tsx')).toBe(false); + }); + + it('scans a healthy number of TRACKED source files (anti-vacuity: a broken glob or a broken index read must not pass on zero input)', () => { const files = listSourceFiles(); expect(files.length).toBeGreaterThan(50); }); diff --git a/web/src/vite-env.d.ts b/web/src/vite-env.d.ts index 11f02fe2a..39bcb7bff 100644 --- a/web/src/vite-env.d.ts +++ b/web/src/vite-env.d.ts @@ -1 +1,10 @@ /// + +// ersatztv#819: the tracked-source-file population, supplied by `vite-plugins/trackedSourceFiles.ts` +// from `git ls-files`. Declared here rather than imported from the plugin so the app project never +// pulls in `@types/node` — see that plugin's doc comment for why that matters. +declare module 'virtual:etv-tracked-source-files' { + /** Paths relative to the Vite root (`web/`), e.g. `src/api/pageSizeScan.ts`. */ + const trackedSourceFiles: string[]; + export default trackedSourceFiles; +} diff --git a/web/tsconfig.node.json b/web/tsconfig.node.json index 8fd95d50f..e2ac3d79d 100644 --- a/web/tsconfig.node.json +++ b/web/tsconfig.node.json @@ -14,5 +14,5 @@ // The Playwright UI-E2E config + specs (ersatztv#445) belong here, not in tsconfig.app.json: they // run in Node (not the bundled browser app), and `tsc -b` then type-checks them as part of // `npm run build`, so a broken spec fails the build instead of only failing at test time. - "include": ["vite.config.ts", "eslint.config.js", "playwright.config.ts", "e2e/**/*.ts"] + "include": ["vite.config.ts", "eslint.config.js", "playwright.config.ts", "e2e/**/*.ts", "vite-plugins/**/*.ts"] } diff --git a/web/vite-plugins/trackedSourceFiles.test.ts b/web/vite-plugins/trackedSourceFiles.test.ts new file mode 100644 index 000000000..9df8dfafd --- /dev/null +++ b/web/vite-plugins/trackedSourceFiles.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, it } from 'vitest'; +import { + TRACKED_SOURCE_FILES_ID, + type GitRunner, + resolveTrackedSourceFiles, + trackedSourceFilesPlugin +} from './trackedSourceFiles'; + +/** + * The population this plugin supplies is what `web/src/api/pageSizeCallSites.guard.test.ts` asserts + * completeness over (#819), so the plugin is verification code and carries its own proof rather + * than inheriting the guard's — `testing.verification-code-needs-its-own-proof`. + */ + +const RESOLVED_ID = `\0${TRACKED_SOURCE_FILES_ID}`; + +function runnerReturning(output: string): { run: GitRunner; calls: (readonly string[])[] } { + const calls: (readonly string[])[] = []; + return { + calls, + run: (args) => { + calls.push(args); + return output; + } + }; +} + +describe('resolveTrackedSourceFiles (#819)', () => { + it('splits git\'s NUL-separated output into paths, dropping the trailing empty field', () => { + const { run } = runnerReturning('src/a.ts\0src/b.tsx\0'); + expect(resolveTrackedSourceFiles('/repo/web', run)).toEqual(['src/a.ts', 'src/b.tsx']); + }); + + it('asks git for NUL-separated output, scoped to src/, in the given root', () => { + // Pins `-z` specifically. Without it git applies `core.quotePath` to a non-ASCII name and + // returns a C-quoted string that no longer equals the path Vite reports, which would drop that + // file from the population silently — the guard would then under-enumerate while looking green. + const { run, calls } = runnerReturning('src/a.ts\0'); + resolveTrackedSourceFiles('/repo/web', run); + expect(calls).toEqual([['-C', '/repo/web', 'ls-files', '-z', '--', 'src']]); + }); + + it('keeps a path containing spaces intact rather than splitting on whitespace', () => { + const { run } = runnerReturning('src/screens/My Screen.tsx\0src/b.ts\0'); + expect(resolveTrackedSourceFiles('/repo/web', run)).toEqual(['src/screens/My Screen.tsx', 'src/b.ts']); + }); + + it('THROWS when git fails, instead of degrading to an unfiltered walk', () => { + // The failure this rejects is the fail-open one: answering "could not tell" with the very + // population the index was brought in to replace is a permanent no-op that reports green. + const boom: GitRunner = () => { + throw new Error('not a git repository'); + }; + expect(() => resolveTrackedSourceFiles('/repo/web', boom)).toThrow(/could not read the git index/); + }); + + it('THROWS when git reports zero tracked files (anti-vacuity at the source)', () => { + const { run } = runnerReturning(''); + expect(() => resolveTrackedSourceFiles('/repo/web', run)).toThrow(/ZERO tracked files/); + }); +}); + +describe('trackedSourceFilesPlugin (#819)', () => { + it('resolves only its own virtual id', () => { + const plugin = trackedSourceFilesPlugin(runnerReturning('src/a.ts\0').run); + const resolveId = plugin.resolveId as (id: string) => string | undefined; + + expect(resolveId.call(plugin, TRACKED_SOURCE_FILES_ID)).toBe(RESOLVED_ID); + expect(resolveId.call(plugin, 'react')).toBeUndefined(); + expect(resolveId.call(plugin, './pageSizeScan')).toBeUndefined(); + }); + + it('emits the tracked list as a default-exported module', () => { + const plugin = trackedSourceFilesPlugin(runnerReturning('src/a.ts\0src/b.tsx\0').run); + const load = plugin.load as (id: string) => string | undefined; + + expect(load.call(plugin, RESOLVED_ID)).toBe('export default ["src/a.ts","src/b.tsx"];\n'); + }); + + it('does NOT shell out to git unless the virtual module is actually loaded', () => { + // `vite build` — including the Docker image build, whose context carries no `.git` — never + // imports this module. Constructing the plugin, or loading anything else, must therefore not + // run git, or registering it in vite.config.ts would break every gitless build. + const { run, calls } = runnerReturning('src/a.ts\0'); + const plugin = trackedSourceFilesPlugin(run); + const load = plugin.load as (id: string) => string | undefined; + + expect(calls).toHaveLength(0); + expect(load.call(plugin, '\0some-other-virtual-module')).toBeUndefined(); + expect(calls).toHaveLength(0); + + load.call(plugin, RESOLVED_ID); + expect(calls).toHaveLength(1); + }); +}); diff --git a/web/vite-plugins/trackedSourceFiles.ts b/web/vite-plugins/trackedSourceFiles.ts new file mode 100644 index 000000000..e2137ccde --- /dev/null +++ b/web/vite-plugins/trackedSourceFiles.ts @@ -0,0 +1,91 @@ +import { execFileSync } from 'node:child_process'; +import type { Plugin } from 'vite'; + +/** + * ersatztv#819: hands the SPA's structural guards the set of source files git actually tracks. + * + * `web/src/api/pageSizeCallSites.guard.test.ts` asserts EXACT completeness over the files it + * discovers, so its population has to be the git index — `testing.guard-derives-population-from-source`. + * A directory walk is not an authoritative source: an untracked `.ts`/`.tsx` under `web/src/` (a + * scratch file, a half-finished component, an editor dropping) enters the population and fails as + * unregistered on that checkout while CI, which only ever sees tracked files, stays green. That is + * the developer-red / CI-green shape #778 named and #806 removed from every other guard here. + * + * Why a Vite plugin rather than `node:child_process` in the guard itself: `@types/node` is + * deliberately absent from `tsconfig.app.json`, which covers production browser code too, and + * wiring it in was tried and reverted — under `tsc -b`'s single-program compilation it leaked + * Node's ambient `setTimeout` into the whole app project and broke three unrelated + * `window.setTimeout` mocks expecting the DOM signature. A plugin runs in Vite's own Node context, + * where `@types/node` is already available via `tsconfig.node.json`, and hands the result to the + * app project as plain data. No app-project type change, no `/// `. + */ +export const TRACKED_SOURCE_FILES_ID = 'virtual:etv-tracked-source-files'; + +/** Rollup convention: a resolved virtual id is `\0`-prefixed so no other plugin claims it. */ +const RESOLVED_ID = `\0${TRACKED_SOURCE_FILES_ID}`; + +/** Injectable so `resolveTrackedSourceFiles` is testable without a real repository. */ +export type GitRunner = (args: readonly string[]) => string; + +const runGitDefault: GitRunner = (args) => + execFileSync('git', [...args], { encoding: 'utf8', maxBuffer: 64 * 1024 * 1024 }); + +/** + * Tracked paths under `src/`, relative to the Vite root (`web/`) — e.g. `src/api/pageSizeScan.ts`. + * + * `-z` rather than plain `ls-files`: git applies `core.quotePath` to non-ASCII names by default and + * would hand back a C-quoted string that no longer matches the path Vite reports, silently dropping + * that file from the population. NUL-separated output is never quoted. + */ +export function resolveTrackedSourceFiles(root: string, runGit: GitRunner = runGitDefault): string[] { + let raw: string; + try { + raw = runGit(['-C', root, 'ls-files', '-z', '--', 'src']); + } catch (cause) { + // FAIL, never degrade to the unfiltered walk. "Could not tell" is a third outcome distinct from + // pass and fail, and a check that answers it by falling back to the population it exists to + // replace is a permanent no-op that reports green — the shape ersatztv#772 was filed for. + throw new Error( + `${TRACKED_SOURCE_FILES_ID}: could not read the git index under ${root}. The SPA guards derive ` + + `their population from it and must not fall back to a filesystem walk — fix the checkout ` + + `rather than the guard.`, + { cause } + ); + } + + const files = raw.split('\0').filter((path) => path.length > 0); + if (files.length === 0) { + // Anti-vacuity at the source. An empty population makes every completeness claim downstream + // pass while proving nothing, which is the characteristic failure of a completeness check. + throw new Error( + `${TRACKED_SOURCE_FILES_ID}: git reported ZERO tracked files under ${root}/src. That is never ` + + `true of this repository, so it means the index was not readable rather than that the ` + + `population is genuinely empty.` + ); + } + + return files; +} + +export function trackedSourceFilesPlugin(runGit?: GitRunner): Plugin { + let root = process.cwd(); + + return { + name: 'etv:tracked-source-files', + configResolved(config) { + root = config.root; + }, + resolveId(id) { + return id === TRACKED_SOURCE_FILES_ID ? RESOLVED_ID : undefined; + }, + // Deliberately LAZY: git runs only when something imports the virtual module, which only the + // guard suite does. `vite build` (including the Docker image build, whose context carries no + // `.git`) never resolves this id and so never shells out to git. + load(id) { + if (id !== RESOLVED_ID) { + return undefined; + } + return `export default ${JSON.stringify(resolveTrackedSourceFiles(root, runGit))};\n`; + } + }; +} diff --git a/web/vite.config.ts b/web/vite.config.ts index 493e8146a..dd796b7fa 100644 --- a/web/vite.config.ts +++ b/web/vite.config.ts @@ -1,12 +1,13 @@ 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()], + plugins: [react(), trackedSourceFilesPlugin()], resolve: { alias: { '@': fileURLToPath(new URL('./src', import.meta.url)), -- 2.47.3 From 327ca5916239e8580af6682b8308be740002bde9 Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 28 Aug 2026 22:28:51 +0200 Subject: [PATCH 02/17] docs(819): state that the meta-guard's register does not reach the TS guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Making the page-size guard index-derived makes it READ as covered by `scripts/tests/test_guard_populations_derive_from_git.py`, which proves exactly this property over every guard it knows about. It does not reach this one: that register detects Python modules importing `scripts/tests/tracked_files.py`, and its own docstring already names the gap — "a module deriving a file population some other way — shelling out to `git ls-files` itself — is invisible to it." An unstated gap here would be worse than the one #819 closed, because a reader would infer mechanical coverage from the conversion itself. refs #819 --- docs/guard-inventory.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/guard-inventory.md b/docs/guard-inventory.md index 6dc4ed59a..5e1770a12 100644 --- a/docs/guard-inventory.md +++ b/docs/guard-inventory.md @@ -69,6 +69,12 @@ with have sat in these gaps. Treat anything not named here as unassessed, not as are all structural guards and none has a row. 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 only review caught that the second had become the only one named. + Since #819 the page-size guard derives its file population from the git index like the Python + ones, which makes it read as covered by `test_guard_populations_derive_from_git.py`. It is not: + that register detects Python modules IMPORTING `scripts/tests/tracked_files.py`, and its own + docstring names this gap ("a module deriving a file population some other way — shelling out to + `git ls-files` itself — is invisible to it"). So the TS guard's derivation is re-checked by + review, not mechanically. 3. **Mentions counted as call sites.** The `scripts/…` scrape matches any occurrence, including inside a comment or an `::error::` string. `scripts/update-openapi.sh` is named in a `pr-checks.yml` error message, so removing the step that runs it would leave its row intact. -- 2.47.3 From eedf56b8ad7339958886a7ac3f0fa1a2af6f1f38 Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 28 Aug 2026 22:49:50 +0200 Subject: [PATCH 03/17] fix(819): close the review's blocker and the blind spot it measured MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects found by cold review of 9e5c288db, all reproduced here before fixing and re-measured after. BLOCKER — the Docker image build failed on the previous commit. `docker/Dockerfile`'s web-build stage copies only `web/` and `design-system/`, so it has no `.git`, and `node:22-bookworm-slim` ships no git binary. Its `RUN` line ran `npm test -- --run`, which loads the virtual module, which shells out to git and throws by design. Measured in an exact gitless replica of the stage: `Test Files 1 failed | 117 passed`, against `117 passed` on the parent. `build` is gated `needs: [test, migrations, scan]` and the `test` job — which carries no `if:`, so it runs on PR, main and tag alike — already runs the identical suite on a real checkout, so the Dockerfile's run was duplicated work and is dropped rather than propped up with a `.git` copy and a git install. The previous commit message asserted this build was safe on the strength of having verified `vite build` alone; the test step on the same line was never measured. HIGH — a tracked file could still leave the population silently, and one way was a REGRESSION. `import.meta.glob` matches no dotfile or dot-directory, and on `core.ignorecase=true` a disk spelling that diverges from the index spelling (`caseprobe.tsx` vs `CaseProbe.tsx`) is tracked, present, and invisible to the intersection — measured blind here, and measured CAUGHT by the parent commit. The stated residual named only the deleted-file case, so the prose claimed a completeness the code did not have. Fixed by asserting the other direction: the plugin now reports which tracked paths are absent from disk, and the guard requires every tracked, on-disk, in-scope path to have reached the scanner. A mid-edit deletion stays tolerated — measured, still green — because that is transient, while a hole is now named and red. This is the rule `scripts/tests/tracked_files.py` already states for the Python derivations: existence is ASSERTED, never filtered. MEDIUM — the clause proof was fixture-only. Every clause test drove `listSourceFilesFrom` with injected inputs, so `... && tracked.size < 10` was live over the 1-2 entry fixtures and dead over the real 260-path index: the fix fully regressed, suite green. Measured. Added a liveness test that drives the REAL glob and REAL index with one synthetic key. That mutation now reddens it, and disarming the clause outright reddens two named tests. Also: the plugin refuses instead of guessing when `configResolved` never ran (a wrong root was a silent wrong answer), its doc comment states the `src/`-only pathspec so a future caller cannot inherit a truncated population, and git's stderr is piped so the framed error is the only message. Docs: `docs/spa-conventions.md` §1 gains the `web/vite-plugins/` layout entry; `docs/guard-inventory.md`'s hand-written TS/C# list gains the fourth file the review noted it had already gone stale against, and the row's Notes no longer overstate the population as "`git ls-files`" when it is the walk intersected with the index. refs #819 --- docker/Dockerfile | 11 ++- docs/guard-inventory.md | 7 +- docs/spa-conventions.md | 5 ++ web/src/api/pageSizeCallSites.guard.test.ts | 83 +++++++++++++++++-- web/src/vite-env.d.ts | 10 ++- web/vite-plugins/trackedSourceFiles.test.ts | 69 ++++++++++++---- web/vite-plugins/trackedSourceFiles.ts | 89 +++++++++++++++------ 7 files changed, 220 insertions(+), 54 deletions(-) diff --git a/docker/Dockerfile b/docker/Dockerfile index 1cefcf67f..d9e317388 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -9,7 +9,16 @@ 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 +# No `npm test` here, deliberately (ersatztv#819). This stage's context is `web/` and +# `design-system/` only — no `.git`, and `node:22-bookworm-slim` ships no git binary — while the +# SPA's structural guards derive their file population from `git ls-files` rather than a directory +# walk, so the suite cannot run in a context with no index. It is not lost: `docker-build.yml`'s +# `Build & test (.NET)` job runs the identical `npm test -- --run` on a real checkout, and `build` +# (the job that invokes this Dockerfile) carries `needs: [test, migrations, scan]`, so no image is +# ever produced from a tree whose web suite has not passed. Running it again here only duplicated +# that work. Building the image by hand still lints, typechecks and builds; run `npm test` in a +# checkout for the rest. +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/guard-inventory.md b/docs/guard-inventory.md index 5e1770a12..02aa29c4a 100644 --- a/docs/guard-inventory.md +++ b/docs/guard-inventory.md @@ -65,8 +65,9 @@ with have sat in these gaps. Treat anything not named here as unassessed, not as `inline` assertion cannot be proven to work without running the job, so those rows carry `Proof: NONE` honestly instead of borrowing a neighbouring file's proof. 2. **C# and TypeScript guards** — `ErsatzTV.Mcp.Tests/ToolCatalogTests.cs`, - `web/src/api/pageSizeCallSites.guard.test.ts` and `web/src/api/completeRequest.guard.test.ts` - are all structural guards and none has a row. Note what that costs: this list is a HAND-WRITTEN + `web/src/api/pageSizeCallSites.guard.test.ts`, `web/src/api/completeRequest.guard.test.ts` and + `web/vite-plugins/trackedSourceFiles.test.ts` (the proof for the population the first of those + derives) are all structural guards or proofs of one, and none has a row. 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 only review caught that the second had become the only one named. Since #819 the page-size guard derives its file population from the git index like the Python @@ -161,7 +162,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 and found by cold review on the pushed head: an untracked `.yaml` dropped in `.gitea/workflows/` reddened two guards while absent from the index — the issue's list of files to assess was a starting point, not the population | | `test_workflow_persist_credentials.py` | `.gitea/workflows/*.yml` + `*.yaml` | **yes** — every `actions/checkout` in every workflow must drop the persisted credential | derived from the index from the start (#835). Shipped with NO exemption list, which is why it waited for #744: the one non-compliant checkout would otherwise have bought a permanent entry that keeps passing after its reason expires | -| `web/src/api/pageSizeCallSites.guard.test.ts` | `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 glob still supplies file CONTENT; the POPULATION is `git ls-files`, 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 or reports zero files. Residual, stated: a file that is tracked but DELETED in the working tree has no content and drops out. Not asserted — an unstaged deletion is a normal developer state and a guard red on one gets ignored (#806) — and a REGISTERED site in such a file still fails loudly on the STALE direction | +| `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 both directions are asserted: 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 case divergence, which `core.ignorecase` makes permanent). 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 | | `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 | | `test_ci_dropped_step_guard.py` | the parsed workflow document (its OWN subject only — the release-path `scan` job runs it in a non-git copy, so it may not derive from the index) | **no filesystem population at all** | unchanged by #806; the SCOPE residual it left open — `MARKED_JOBS` as a hand-written mirror of the required contexts on `main` — was closed by #787, which derives it from `.gitea/required-status-contexts.json` and reconciles that snapshot against the server in `scripts/check-required-contexts.sh`. The cross-workflow half of that claim lives in `test_ci_status_context_uniqueness.py`, which CAN use the index | diff --git a/docs/spa-conventions.md b/docs/spa-conventions.md index 6f7e56ac3..f3df017b6 100644 --- a/docs/spa-conventions.md +++ b/docs/spa-conventions.md @@ -23,6 +23,11 @@ Vite + React + TypeScript, builds to `ErsatzTV/wwwroot/app` (see `web/vite.confi - **Screens**: `web/src/screens/*.tsx`, one file per top-level screen, generally with a colocated `*.test.tsx`. - **API clients**: `web/src/api/.ts` (see §4). +- **Build-time plugins**: `web/vite-plugins/*.ts` — Vite plugins that run in NODE, not in the + browser bundle, and are type-checked under `tsconfig.node.json` (never `tsconfig.app.json`, + which must stay free of `@types/node`). This is the seam for anything a test needs that only + Node can answer — `trackedSourceFiles.ts` reads `git ls-files` so the SPA's structural guards + derive their file population from the git index rather than a directory walk (#819). - **Styling**: `web/src/shell.css` (+ `web/src/components/components.css`) — utility classes with a `ctv-` prefix (~690 occurrences across those two files). Reuse an existing `ctv-*` class before inventing a new one. `shell.css` carries the only base reset — `html, body { margin: 0 }` plus diff --git a/web/src/api/pageSizeCallSites.guard.test.ts b/web/src/api/pageSizeCallSites.guard.test.ts index a175f82a0..d61e24ec6 100644 --- a/web/src/api/pageSizeCallSites.guard.test.ts +++ b/web/src/api/pageSizeCallSites.guard.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import trackedSourceFiles from 'virtual:etv-tracked-source-files'; +import trackedSources from 'virtual:etv-tracked-source-files'; import { scanPageSizeSites } from './pageSizeScan'; /** @@ -369,7 +369,10 @@ const rawSourceModules = import.meta.glob('/src/**/*.{ts,tsx,mts,cts}', { * The git index, as the guard compares it: paths relative to the Vite root (`web/`), e.g. * `src/api/pageSizeScan.ts`. Built once — membership is asked per globbed file. */ -const TRACKED_SOURCE_PATHS: ReadonlySet = new Set(trackedSourceFiles); +const TRACKED_SOURCE_PATHS: ReadonlySet = new Set(trackedSources.tracked); + +/** Tracked paths with no file on disk — see `expectedTrackedSources` for why they are separated. */ +const ABSENT_FROM_DISK: ReadonlySet = new Set(trackedSources.absentFromDisk); /** `import.meta.glob` keys are root-absolute (`/src/...`); the index is root-relative. */ export function globKeyToRootRelativePath(key: string): string { @@ -444,12 +447,14 @@ export function listSourceFilesFrom( // it is NOT the "filter before a completeness claim" that // `testing.guard-derives-population-from-source` forbids — that rule forbids narrowing the // population so the ABSENT member becomes unrepresentable, and this restores the population to - // the authoritative source the rule names. The residual, stated rather than implied: a file - // that is tracked but DELETED in the working tree has no content to scan and so drops out - // here. That direction is deliberately not asserted, because an unstaged deletion is a normal - // developer state and a guard red on one teaches its readers to ignore it (#806). It is also - // self-announcing for anything that matters: a deleted file holding a REGISTERED site fails - // loudly on the STALE direction below. + // the authoritative source the rule names. + // + // Dropping a key here is only safe because the OTHER direction is asserted: every tracked, + // on-disk, scannable path must come back out of this function, checked by + // `supplies content for every tracked source file on disk`. Without that, a tracked file the + // WALK cannot see — a dotfile, or a name whose disk case diverges from the index case, which + // `core.ignorecase` makes persistent on macOS — would vanish from the population silently, + // which is the same defect one level down from the one this clause fixes. if (!isTrackedSourcePath(key, tracked)) { continue; } @@ -462,6 +467,31 @@ function listSourceFiles(): ScannableSource[] { return listSourceFilesFrom(rawSourceModules, TRACKED_SOURCE_PATHS); } +/** + * What the index says the population SHOULD be: every tracked path under `src/` that is in scope + * for the scanner and actually exists on disk. + * + * `absentFromDisk` is subtracted rather than ignored, and the distinction is the point. A tracked + * path with no file is a mid-edit deletion — transient, and reddening on it would make the guard + * red on an ordinary working state, which teaches readers to ignore it (#806). A tracked path that + * DOES exist but never reached the walk is a hole: the glob cannot see dotfiles or dot-directories, + * and on a case-insensitive filesystem `git` keeps the index spelling while the disk keeps another, + * so the intersection misses it permanently. Comparing against this set is what tells those two + * apart instead of silently shrinking the population — the rule `scripts/tests/tracked_files.py` + * states for the Python derivations ("Existence is ASSERTED, never filtered"). + */ +export function expectedTrackedSources( + tracked: readonly string[], + absentFromDisk: ReadonlySet +): string[] { + return tracked.filter( + (path) => + !path.includes('/generated/') && + isScannableSourceFileName(basename(path)) && + !absentFromDisk.has(path) + ); +} + interface DiscoveredSite { file: string; line: number; @@ -607,6 +637,43 @@ describe('pageSize call-site guard (#650)', () => { ]); }); + it('applies the tracked filter to the REAL population, not only to injected fixtures', () => { + // Liveness, which the fixture tests above cannot give. Those drive `listSourceFilesFrom` with + // 1-2 injected entries, so a clause narrowed to `... && tracked.size < 10` stays green over + // them while being dead over the real 260-path index — the fix fully regressed, suite green. + // "The guard being WIRED is not the guard RUNNING." This drives the REAL glob and the REAL + // index, with one synthetic key added, so the clause has to be live on the path that ships. + const imposter = { + ...rawSourceModules, + '/src/screens/NotTracked819Imposter.tsx': 'const a = { pageNum: 0, pageSize: 25 };' + }; + + expect(listSourceFilesFrom(imposter, TRACKED_SOURCE_PATHS).map((source) => source.file)).not.toContain( + 'screens/NotTracked819Imposter.tsx' + ); + }); + + it('supplies content for every tracked source file on disk (a shortfall is a HOLE, not a skip)', () => { + // The other direction of the set equality. `listSourceFilesFrom` drops any key the index does + // not carry; this asserts nothing was dropped that the index DOES carry and that exists on + // disk. Two real holes it closes, both measured on this repo: `import.meta.glob` matches no + // dotfile or dot-directory, and on `core.ignorecase=true` a file whose disk spelling diverges + // from its index spelling (`caseprobe.tsx` vs `CaseProbe.tsx`) is tracked, present, and + // invisible to the intersection — the guard went blind to it rather than reporting it. + const produced = new Set(listSourceFiles().map((source) => `src/${source.file}`)); + const missing = expectedTrackedSources(trackedSources.tracked, ABSENT_FROM_DISK).filter( + (path) => !produced.has(path) + ); + + expect( + missing, + `Tracked, on-disk, in-scope source file(s) that never reached the scanner. The walk cannot ` + + `see them, so the population silently shrank. Usual causes: a dotfile or dot-directory ` + + `(import.meta.glob matches neither), or a name whose disk case diverges from the index ` + + `case. Fix the file or widen the glob — do NOT drop this assertion.` + ).toEqual([]); + }); + it('normalises a root-absolute glob key to the root-relative path the index uses', () => { expect(globKeyToRootRelativePath('/src/api/pageSizeScan.ts')).toBe('src/api/pageSizeScan.ts'); // Already relative — idempotent rather than stripping a second character. diff --git a/web/src/vite-env.d.ts b/web/src/vite-env.d.ts index 39bcb7bff..50339c663 100644 --- a/web/src/vite-env.d.ts +++ b/web/src/vite-env.d.ts @@ -4,7 +4,11 @@ // from `git ls-files`. Declared here rather than imported from the plugin so the app project never // pulls in `@types/node` — see that plugin's doc comment for why that matters. declare module 'virtual:etv-tracked-source-files' { - /** Paths relative to the Vite root (`web/`), e.g. `src/api/pageSizeScan.ts`. */ - const trackedSourceFiles: string[]; - export default trackedSourceFiles; + const trackedSources: { + /** Paths relative to the Vite root (`web/`), e.g. `src/api/pageSizeScan.ts`. */ + tracked: string[]; + /** Tracked paths with no file on disk — deleted in the working tree, not yet staged. */ + absentFromDisk: string[]; + }; + export default trackedSources; } diff --git a/web/vite-plugins/trackedSourceFiles.test.ts b/web/vite-plugins/trackedSourceFiles.test.ts index 9df8dfafd..6971d3214 100644 --- a/web/vite-plugins/trackedSourceFiles.test.ts +++ b/web/vite-plugins/trackedSourceFiles.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest'; import { TRACKED_SOURCE_FILES_ID, + type FileExists, type GitRunner, resolveTrackedSourceFiles, trackedSourceFilesPlugin @@ -13,6 +14,7 @@ import { */ const RESOLVED_ID = `\0${TRACKED_SOURCE_FILES_ID}`; +const allPresent: FileExists = () => true; function runnerReturning(output: string): { run: GitRunner; calls: (readonly string[])[] } { const calls: (readonly string[])[] = []; @@ -25,10 +27,17 @@ function runnerReturning(output: string): { run: GitRunner; calls: (readonly str }; } +/** Drives `load()` through the plugin's own `configResolved`, as Vite does. */ +function loadedBy(plugin: ReturnType, root = '/repo/web'): string | undefined { + const configResolved = plugin.configResolved as (config: { root: string }) => void; + configResolved.call(plugin, { root }); + return (plugin.load as (id: string) => string | undefined).call(plugin, RESOLVED_ID); +} + describe('resolveTrackedSourceFiles (#819)', () => { - it('splits git\'s NUL-separated output into paths, dropping the trailing empty field', () => { + it("splits git's NUL-separated output into paths, dropping the trailing empty field", () => { const { run } = runnerReturning('src/a.ts\0src/b.tsx\0'); - expect(resolveTrackedSourceFiles('/repo/web', run)).toEqual(['src/a.ts', 'src/b.tsx']); + expect(resolveTrackedSourceFiles('/repo/web', run, allPresent).tracked).toEqual(['src/a.ts', 'src/b.tsx']); }); it('asks git for NUL-separated output, scoped to src/, in the given root', () => { @@ -36,13 +45,34 @@ describe('resolveTrackedSourceFiles (#819)', () => { // returns a C-quoted string that no longer equals the path Vite reports, which would drop that // file from the population silently — the guard would then under-enumerate while looking green. const { run, calls } = runnerReturning('src/a.ts\0'); - resolveTrackedSourceFiles('/repo/web', run); + resolveTrackedSourceFiles('/repo/web', run, allPresent); expect(calls).toEqual([['-C', '/repo/web', 'ls-files', '-z', '--', 'src']]); }); it('keeps a path containing spaces intact rather than splitting on whitespace', () => { const { run } = runnerReturning('src/screens/My Screen.tsx\0src/b.ts\0'); - expect(resolveTrackedSourceFiles('/repo/web', run)).toEqual(['src/screens/My Screen.tsx', 'src/b.ts']); + expect(resolveTrackedSourceFiles('/repo/web', run, allPresent).tracked).toEqual([ + 'src/screens/My Screen.tsx', + 'src/b.ts' + ]); + }); + + it('REPORTS tracked paths with no file on disk rather than dropping them', () => { + // The consumer needs a mid-edit deletion (transient, tolerated) told apart from a tracked file + // the walk cannot see (a real hole). Collapsing them is how a population silently shrinks. + const { run } = runnerReturning('src/present.ts\0src/deleted.ts\0'); + const seen: string[] = []; + const exists: FileExists = (path) => { + seen.push(path); + return !path.endsWith('deleted.ts'); + }; + + const result = resolveTrackedSourceFiles('/repo/web', run, exists); + + expect(result.tracked).toEqual(['src/present.ts', 'src/deleted.ts']); + expect(result.absentFromDisk).toEqual(['src/deleted.ts']); + // Existence is checked against the ROOT-joined path, not the repo-relative one. + expect(seen).toEqual(['/repo/web/src/present.ts', '/repo/web/src/deleted.ts']); }); it('THROWS when git fails, instead of degrading to an unfiltered walk', () => { @@ -51,18 +81,18 @@ describe('resolveTrackedSourceFiles (#819)', () => { const boom: GitRunner = () => { throw new Error('not a git repository'); }; - expect(() => resolveTrackedSourceFiles('/repo/web', boom)).toThrow(/could not read the git index/); + expect(() => resolveTrackedSourceFiles('/repo/web', boom, allPresent)).toThrow(/could not read the git index/); }); it('THROWS when git reports zero tracked files (anti-vacuity at the source)', () => { const { run } = runnerReturning(''); - expect(() => resolveTrackedSourceFiles('/repo/web', run)).toThrow(/ZERO tracked files/); + expect(() => resolveTrackedSourceFiles('/repo/web', run, allPresent)).toThrow(/ZERO tracked files/); }); }); describe('trackedSourceFilesPlugin (#819)', () => { it('resolves only its own virtual id', () => { - const plugin = trackedSourceFilesPlugin(runnerReturning('src/a.ts\0').run); + const plugin = trackedSourceFilesPlugin(runnerReturning('src/a.ts\0').run, allPresent); const resolveId = plugin.resolveId as (id: string) => string | undefined; expect(resolveId.call(plugin, TRACKED_SOURCE_FILES_ID)).toBe(RESOLVED_ID); @@ -70,21 +100,32 @@ describe('trackedSourceFilesPlugin (#819)', () => { expect(resolveId.call(plugin, './pageSizeScan')).toBeUndefined(); }); - it('emits the tracked list as a default-exported module', () => { - const plugin = trackedSourceFilesPlugin(runnerReturning('src/a.ts\0src/b.tsx\0').run); + it('emits the tracked list and the absent set as a default-exported module', () => { + const plugin = trackedSourceFilesPlugin(runnerReturning('src/a.ts\0src/b.tsx\0').run, allPresent); + + expect(loadedBy(plugin)).toBe('export default {"tracked":["src/a.ts","src/b.tsx"],"absentFromDisk":[]};\n'); + }); + + it('REFUSES rather than guessing when configResolved never ran', () => { + // A wrong root is a silent wrong answer. Seeding it with `process.cwd()` would happen to work + // in this repo's layout and quietly derive the population from the wrong tree elsewhere. + const { run, calls } = runnerReturning('src/a.ts\0'); + const plugin = trackedSourceFilesPlugin(run, allPresent); const load = plugin.load as (id: string) => string | undefined; - expect(load.call(plugin, RESOLVED_ID)).toBe('export default ["src/a.ts","src/b.tsx"];\n'); + expect(() => load.call(plugin, RESOLVED_ID)).toThrow(/configResolved` hook never ran/); + expect(calls).toHaveLength(0); }); it('does NOT shell out to git unless the virtual module is actually loaded', () => { - // `vite build` — including the Docker image build, whose context carries no `.git` — never - // imports this module. Constructing the plugin, or loading anything else, must therefore not - // run git, or registering it in vite.config.ts would break every gitless build. + // Constructing the plugin, or loading anything else, must not run git — otherwise registering + // it in vite.config.ts would break every build in a context without a repository. const { run, calls } = runnerReturning('src/a.ts\0'); - const plugin = trackedSourceFilesPlugin(run); + const plugin = trackedSourceFilesPlugin(run, allPresent); + const configResolved = plugin.configResolved as (config: { root: string }) => void; const load = plugin.load as (id: string) => string | undefined; + configResolved.call(plugin, { root: '/repo/web' }); expect(calls).toHaveLength(0); expect(load.call(plugin, '\0some-other-virtual-module')).toBeUndefined(); expect(calls).toHaveLength(0); diff --git a/web/vite-plugins/trackedSourceFiles.ts b/web/vite-plugins/trackedSourceFiles.ts index e2137ccde..9a2264163 100644 --- a/web/vite-plugins/trackedSourceFiles.ts +++ b/web/vite-plugins/trackedSourceFiles.ts @@ -1,15 +1,18 @@ import { execFileSync } from 'node:child_process'; +import { existsSync } from 'node:fs'; +import { join } from 'node:path'; import type { Plugin } from 'vite'; /** - * ersatztv#819: hands the SPA's structural guards the set of source files git actually tracks. + * ersatztv#819: hands `web/src/api/pageSizeCallSites.guard.test.ts` the set of files under `src/` + * that git actually tracks. * - * `web/src/api/pageSizeCallSites.guard.test.ts` asserts EXACT completeness over the files it - * discovers, so its population has to be the git index — `testing.guard-derives-population-from-source`. - * A directory walk is not an authoritative source: an untracked `.ts`/`.tsx` under `web/src/` (a - * scratch file, a half-finished component, an editor dropping) enters the population and fails as - * unregistered on that checkout while CI, which only ever sees tracked files, stays green. That is - * the developer-red / CI-green shape #778 named and #806 removed from every other guard here. + * That guard asserts EXACT completeness over the files it discovers, so its population has to be + * the git index — `testing.guard-derives-population-from-source`. A directory walk is not an + * authoritative source: an untracked `.ts`/`.tsx` under `web/src/` (a scratch file, a half-finished + * component, an editor dropping) enters the population and fails as unregistered on that checkout + * while CI, which only ever sees tracked files, stays green. That is the developer-red / CI-green + * shape #778 named and #806 removed from every other guard here. * * Why a Vite plugin rather than `node:child_process` in the guard itself: `@types/node` is * deliberately absent from `tsconfig.app.json`, which covers production browser code too, and @@ -18,26 +21,48 @@ import type { Plugin } from 'vite'; * `window.setTimeout` mocks expecting the DOM signature. A plugin runs in Vite's own Node context, * where `@types/node` is already available via `tsconfig.node.json`, and hands the result to the * app project as plain data. No app-project type change, no `/// `. + * + * SCOPE: the pathspec is `src/` only. A future guard over `web/e2e`, `web/scripts` or + * `web/vite-plugins` must not import this module expecting its own files — it would receive a + * silently truncated population, which is the defect this exists to remove. Widen the pathspec + * (and say so here) rather than reusing it out of scope. */ export const TRACKED_SOURCE_FILES_ID = 'virtual:etv-tracked-source-files'; /** Rollup convention: a resolved virtual id is `\0`-prefixed so no other plugin claims it. */ const RESOLVED_ID = `\0${TRACKED_SOURCE_FILES_ID}`; +export interface TrackedSources { + /** Tracked paths under `src/`, relative to the Vite root (`web/`), e.g. `src/api/pageSizeScan.ts`. */ + tracked: string[]; + /** + * The subset of `tracked` with no file on disk — i.e. deleted in the working tree but not yet + * staged. Reported rather than silently removed: the consumer needs to tell a mid-edit deletion + * (a normal, transient developer state) apart from a tracked file the WALK cannot see, which is + * a real hole. Collapsing the two is how a population silently shrinks. + */ + absentFromDisk: string[]; +} + /** Injectable so `resolveTrackedSourceFiles` is testable without a real repository. */ export type GitRunner = (args: readonly string[]) => string; +/** Injectable for the same reason; defaults to a real on-disk existence check. */ +export type FileExists = (path: string) => boolean; const runGitDefault: GitRunner = (args) => - execFileSync('git', [...args], { encoding: 'utf8', maxBuffer: 64 * 1024 * 1024 }); + execFileSync('git', [...args], { + encoding: 'utf8', + maxBuffer: 64 * 1024 * 1024, + // Keep git's own `fatal: not a git repository` off the terminal so the framed error below is + // the only message a reader sees. + stdio: ['ignore', 'pipe', 'pipe'] + }); -/** - * Tracked paths under `src/`, relative to the Vite root (`web/`) — e.g. `src/api/pageSizeScan.ts`. - * - * `-z` rather than plain `ls-files`: git applies `core.quotePath` to non-ASCII names by default and - * would hand back a C-quoted string that no longer matches the path Vite reports, silently dropping - * that file from the population. NUL-separated output is never quoted. - */ -export function resolveTrackedSourceFiles(root: string, runGit: GitRunner = runGitDefault): string[] { +export function resolveTrackedSourceFiles( + root: string, + runGit: GitRunner = runGitDefault, + fileExists: FileExists = existsSync +): TrackedSources { let raw: string; try { raw = runGit(['-C', root, 'ls-files', '-z', '--', 'src']); @@ -53,8 +78,11 @@ export function resolveTrackedSourceFiles(root: string, runGit: GitRunner = runG ); } - const files = raw.split('\0').filter((path) => path.length > 0); - if (files.length === 0) { + // `-z` rather than plain `ls-files`: git applies `core.quotePath` to non-ASCII names by default + // and would hand back a C-quoted string that no longer matches the path Vite reports, silently + // dropping that file from the population. NUL-separated output is never quoted. + const tracked = raw.split('\0').filter((path) => path.length > 0); + if (tracked.length === 0) { // Anti-vacuity at the source. An empty population makes every completeness claim downstream // pass while proving nothing, which is the characteristic failure of a completeness check. throw new Error( @@ -64,11 +92,17 @@ export function resolveTrackedSourceFiles(root: string, runGit: GitRunner = runG ); } - return files; + return { + tracked, + absentFromDisk: tracked.filter((path) => !fileExists(join(root, path))) + }; } -export function trackedSourceFilesPlugin(runGit?: GitRunner): Plugin { - let root = process.cwd(); +export function trackedSourceFilesPlugin(runGit?: GitRunner, fileExists?: FileExists): Plugin { + // Deliberately NOT seeded with `process.cwd()`. A wrong root is a silent wrong answer — git would + // report zero files and the anti-vacuity throw would fire, but only by luck of this repo's + // layout. Refusing is the honest outcome. + let root: string | undefined; return { name: 'etv:tracked-source-files', @@ -78,14 +112,19 @@ export function trackedSourceFilesPlugin(runGit?: GitRunner): Plugin { resolveId(id) { return id === TRACKED_SOURCE_FILES_ID ? RESOLVED_ID : undefined; }, - // Deliberately LAZY: git runs only when something imports the virtual module, which only the - // guard suite does. `vite build` (including the Docker image build, whose context carries no - // `.git`) never resolves this id and so never shells out to git. + // Deliberately LAZY: git runs only when something imports the virtual module. `vite build` + // never resolves this id and so never shells out to git. load(id) { if (id !== RESOLVED_ID) { return undefined; } - return `export default ${JSON.stringify(resolveTrackedSourceFiles(root, runGit))};\n`; + if (root === undefined) { + throw new Error( + `${TRACKED_SOURCE_FILES_ID}: the plugin's \`configResolved\` hook never ran, so the Vite ` + + `root is unknown. Refusing to guess a root rather than derive a population from the wrong one.` + ); + } + return `export default ${JSON.stringify(resolveTrackedSourceFiles(root, runGit, fileExists))};\n`; } }; } -- 2.47.3 From d8c29fd11e3aea2ac4218615cc2823541158d30f Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 28 Aug 2026 23:34:25 +0200 Subject: [PATCH 04/17] fix(819): mutation-proof the fix that closed the blind spot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 2's cold review found the previous commit had reproduced, one layer up, the exact defect it was fixing: `expectedTrackedSources` — the function that closes the dot-directory / case-divergence hole — had no clause proof. Measured: replacing its body with `return []` left all 1249 tests green while the hole it exists to catch went undetected, and deleting the `absentFromDisk` subtraction (the whole point of the plugin's second field) was green too. Three proofs added, each MEASURED red against the mutation it names: - a direct fixture test pinning all four terms of `expectedTrackedSources` (`return []` and dropping the subtraction now each redden it); - an anti-vacuity floor INSIDE the shortfall test, so an empty expected set cannot pass the comparison it feeds; - a table pinning `isInScopeSourcePath`. That one closes a hole the review measured separately: the walk and the index side applied the SAME scope filters, so scope cancelled out of the equality between them — excluding `src/components/` dropped 13 real files with the whole suite green. The two copies of that predicate are now ONE named function, and the table is what reddens a change to it, since no assertion comparing the two sides can. Also corrected, all found by the same review: - The comment justifying the `absentFromDisk` subtraction cited `scripts/tests/tracked_files.py` as stating the rule it follows. That module states the OPPOSITE and enforces it with `assert absolute.is_file()`. It is now recorded as a deliberate DEVIATION with its reason (that population is a handful of hooks where a missing file is remarkable; this one is ~260 SPA sources a developer edits continuously), rather than as an instance of it. - `absentFromDisk` is named for what it measures — no READABLE file — not for its common cause. `existsSync` follows links, so a tracked BROKEN SYMLINK is a committed, permanent state landing in that set, which "deleted in the working tree" licensed subtracting without saying so. - The Dockerfile comment claimed the `needs: [test, …]` edge alone guarantees no image ships from an untested tree. `Test SPA` is also gated on `docs_only` and the #420 revalidate skip, and `build` is not gated on `revalidate`; the property survives via #420's byte-identical-tree argument, which the comment now names instead of implying the wrong mechanism. - `docs/testing.md`'s web row was falsified by this PR: `web/vite-plugins/**` is a third collection root, and `npm test` now hard-requires a git checkout. - Two prose slips: guard-inventory counted "the first of those" onto the wrong file, and both `docs/spa-conventions.md` and the Dockerfile said "guards" plural where exactly one derives a population. Rebased onto 4b3f6f6c9 (#872), which rewrote `docs/guard-inventory.md`. refs #819 --- docker/Dockerfile | 17 ++- docs/guard-inventory.md | 6 +- docs/spa-conventions.md | 5 +- docs/testing.md | 2 +- web/src/api/pageSizeCallSites.guard.test.ts | 115 +++++++++++++++----- web/src/vite-env.d.ts | 2 +- web/vite-plugins/trackedSourceFiles.ts | 18 ++- 7 files changed, 120 insertions(+), 45 deletions(-) diff --git a/docker/Dockerfile b/docker/Dockerfile index d9e317388..5efb036d9 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -11,13 +11,18 @@ COPY web/. ./web/ WORKDIR /source/web # No `npm test` here, deliberately (ersatztv#819). This stage's context is `web/` and # `design-system/` only — no `.git`, and `node:22-bookworm-slim` ships no git binary — while the -# SPA's structural guards derive their file population from `git ls-files` rather than a directory -# walk, so the suite cannot run in a context with no index. It is not lost: `docker-build.yml`'s +# SPA's one population-deriving guard (`web/src/api/pageSizeCallSites.guard.test.ts`) takes its file +# population from `git ls-files` rather than a directory walk, and refuses rather than falling back, +# so the suite cannot run in a context with no index. It is not lost: `docker-build.yml`'s # `Build & test (.NET)` job runs the identical `npm test -- --run` on a real checkout, and `build` -# (the job that invokes this Dockerfile) carries `needs: [test, migrations, scan]`, so no image is -# ever produced from a tree whose web suite has not passed. Running it again here only duplicated -# that work. Building the image by hand still lints, typechecks and builds; run `npm test` in a -# checkout for the rest. +# (the job that invokes this Dockerfile) carries `needs: [test, migrations, scan]`. State the chain +# precisely, because that `needs:` edge is not the whole of it: the `Test SPA` step is gated on +# `docs_only != 'true' && revalidate.skip != 'true'`, and `build` is NOT gated on `revalidate`. So +# on a push to `main` whose tree is byte-identical to an already-green head, the suite is skipped +# and the image still builds — the property holds there via #420's byte-identical-tree argument, +# not via the dependency edge. Either way the suite ran against that tree; running it a second time +# here only duplicated the work. Building the image by hand still lints, typechecks and builds; run +# `npm test` in a checkout for the rest. 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 diff --git a/docs/guard-inventory.md b/docs/guard-inventory.md index 02aa29c4a..a2f270e42 100644 --- a/docs/guard-inventory.md +++ b/docs/guard-inventory.md @@ -66,8 +66,8 @@ with have sat in these gaps. Treat anything not named here as unassessed, not as `Proof: NONE` honestly instead of borrowing a neighbouring file's proof. 2. **C# and TypeScript guards** — `ErsatzTV.Mcp.Tests/ToolCatalogTests.cs`, `web/src/api/pageSizeCallSites.guard.test.ts`, `web/src/api/completeRequest.guard.test.ts` and - `web/vite-plugins/trackedSourceFiles.test.ts` (the proof for the population the first of those - derives) are all structural guards or proofs of one, and none has a row. Note what that costs: this list is a HAND-WRITTEN + `web/vite-plugins/trackedSourceFiles.test.ts` (the proof for the population + `pageSizeCallSites.guard.test.ts` derives) are all structural guards or proofs of one, and none has a row. 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 only review caught that the second had become the only one named. Since #819 the page-size guard derives its file population from the git index like the Python @@ -162,7 +162,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 and found by cold review on the pushed head: an untracked `.yaml` dropped in `.gitea/workflows/` reddened two guards while absent from the index — the issue's list of files to assess was a starting point, not the population | | `test_workflow_persist_credentials.py` | `.gitea/workflows/*.yml` + `*.yaml` | **yes** — every `actions/checkout` in every workflow must drop the persisted credential | derived from the index from the start (#835). Shipped with NO exemption list, which is why it waited for #744: the one non-compliant checkout would otherwise have bought a permanent entry that keeps passing after its reason expires | -| `web/src/api/pageSizeCallSites.guard.test.ts` | `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 both directions are asserted: 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 case divergence, which `core.ignorecase` makes permanent). 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/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; that predicate is pinned by its own table of cases instead. 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 | | `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 | | `test_ci_dropped_step_guard.py` | the parsed workflow document (its OWN subject only — the release-path `scan` job runs it in a non-git copy, so it may not derive from the index) | **no filesystem population at all** | unchanged by #806; the SCOPE residual it left open — `MARKED_JOBS` as a hand-written mirror of the required contexts on `main` — was closed by #787, which derives it from `.gitea/required-status-contexts.json` and reconciles that snapshot against the server in `scripts/check-required-contexts.sh`. The cross-workflow half of that claim lives in `test_ci_status_context_uniqueness.py`, which CAN use the index | diff --git a/docs/spa-conventions.md b/docs/spa-conventions.md index f3df017b6..c20ac74a4 100644 --- a/docs/spa-conventions.md +++ b/docs/spa-conventions.md @@ -26,8 +26,9 @@ Vite + React + TypeScript, builds to `ErsatzTV/wwwroot/app` (see `web/vite.confi - **Build-time plugins**: `web/vite-plugins/*.ts` — Vite plugins that run in NODE, not in the browser bundle, and are type-checked under `tsconfig.node.json` (never `tsconfig.app.json`, which must stay free of `@types/node`). This is the seam for anything a test needs that only - Node can answer — `trackedSourceFiles.ts` reads `git ls-files` so the SPA's structural guards - derive their file population from the git index rather than a directory walk (#819). + Node can answer — `trackedSourceFiles.ts` reads `git ls-files` so `pageSizeCallSites.guard.test.ts`, + the one SPA guard that derives a file population, takes it from the git index rather than a + directory walk (#819). - **Styling**: `web/src/shell.css` (+ `web/src/components/components.css`) — utility classes with a `ctv-` prefix (~690 occurrences across those two files). Reuse an existing `ctv-*` class before inventing a new one. `shell.css` carries the only base reset — `html, body { margin: 0 }` plus diff --git a/docs/testing.md b/docs/testing.md index dc0be608d..59a8fd974 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 | 995 tests across 105 files; run alongside typecheck + build (see below). Collects `src/**` *and* `web/scripts/**`, but deliberately **excludes** `web/e2e/**` (the Playwright specs — vitest's default `**/*.spec.*` glob would otherwise run them under jsdom). | +| `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). **Requires a git checkout and a `git` binary** since ersatztv#819: `web/src/api/pageSizeCallSites.guard.test.ts` 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, so the suite cannot run against an exported tree with no index. `docker/Dockerfile` no longer runs it for exactly that reason. | | `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/web/src/api/pageSizeCallSites.guard.test.ts b/web/src/api/pageSizeCallSites.guard.test.ts index d61e24ec6..f8cf100c7 100644 --- a/web/src/api/pageSizeCallSites.guard.test.ts +++ b/web/src/api/pageSizeCallSites.guard.test.ts @@ -391,6 +391,18 @@ export function isTrackedSourcePath(key: string, tracked: ReadonlySet): return tracked.has(globKeyToRootRelativePath(key)); } +/** + * Whether a root-relative path is in scope for the scanner at all — the ONE place that decides it. + * + * Both sides of the comparison call this: the walk (`listSourceFilesFrom`) and the index + * (`expectedTrackedSources`). Two copies of this predicate would be the classic one-helper-two-callers + * divergence, and a divergence between them is a bug in EITHER direction — the walk quietly scanning + * something the index side does not expect, or vice versa. + */ +export function isInScopeSourcePath(rootRelativePath: string): boolean { + return !rootRelativePath.includes('/generated/') && isScannableSourceFileName(basename(rootRelativePath)); +} + function basename(path: string): string { const idx = path.lastIndexOf('/'); return idx === -1 ? path : path.slice(idx + 1); @@ -437,10 +449,7 @@ export function listSourceFilesFrom( ): ScannableSource[] { const out: ScannableSource[] = []; for (const [key, text] of Object.entries(modules)) { - if (key.includes('/generated/')) { - continue; - } - if (!isScannableSourceFileName(basename(key))) { + if (!isInScopeSourcePath(globKeyToRootRelativePath(key))) { continue; } // #819: the population is the git index, not the directory. This narrows the glob's results; @@ -450,11 +459,17 @@ export function listSourceFilesFrom( // the authoritative source the rule names. // // Dropping a key here is only safe because the OTHER direction is asserted: every tracked, - // on-disk, scannable path must come back out of this function, checked by + // on-disk, in-scope path must come back out of this function, checked by // `supplies content for every tracked source file on disk`. Without that, a tracked file the - // WALK cannot see — a dotfile, or a name whose disk case diverges from the index case, which - // `core.ignorecase` makes persistent on macOS — would vanish from the population silently, - // which is the same defect one level down from the one this clause fixes. + // WALK cannot see — a dotfile, or a name whose disk spelling diverges from the index spelling, + // which `core.ignorecase` (and NFD/NFC normalisation) makes persistent on macOS — would vanish + // from the population silently, the same defect one level down from the one this clause fixes. + // + // What that direction does NOT prove, stated because "both directions are asserted" would + // otherwise read as more than it is: both sides share `isInScopeSourcePath`, so the SCOPE + // itself cancels out of the equality. A change to what counts as in-scope moves both sets + // together and is invisible here — it is caught, if at all, by that predicate's own table of + // cases and by the anti-vacuity floor below. if (!isTrackedSourcePath(key, tracked)) { continue; } @@ -472,24 +487,26 @@ function listSourceFiles(): ScannableSource[] { * for the scanner and actually exists on disk. * * `absentFromDisk` is subtracted rather than ignored, and the distinction is the point. A tracked - * path with no file is a mid-edit deletion — transient, and reddening on it would make the guard - * red on an ordinary working state, which teaches readers to ignore it (#806). A tracked path that - * DOES exist but never reached the walk is a hole: the glob cannot see dotfiles or dot-directories, - * and on a case-insensitive filesystem `git` keeps the index spelling while the disk keeps another, - * so the intersection misses it permanently. Comparing against this set is what tells those two - * apart instead of silently shrinking the population — the rule `scripts/tests/tracked_files.py` - * states for the Python derivations ("Existence is ASSERTED, never filtered"). + * path with no readable file is a mid-edit deletion — transient, and reddening on it would make the + * guard red on an ordinary working state, which teaches readers to ignore it (#806). A tracked path + * that DOES exist but never reached the walk is a hole: the glob matches no dotfile or + * dot-directory, and on a case-insensitive filesystem `git` keeps the index spelling while the disk + * keeps another (likewise NFD/NFC under `core.precomposeunicode`), so the intersection misses it + * permanently. Comparing against this set is what tells those two apart instead of collapsing them. + * + * This is a DELIBERATE DEVIATION from `scripts/tests/tracked_files.py`, not an instance of it. That + * module asserts existence — `assert absolute.is_file()`, on the argument that reporting a mid-edit + * tree "is strictly better than silently shrinking the population" — and this filters on it. The + * deviation is taken because the two populations differ in kind: that one covers a handful of hooks + * and scripts, where a missing file is remarkable, while this one covers ~260 SPA sources a + * developer edits continuously, where an unstaged deletion is routine. The cost is recorded rather + * than hidden: see the residual note in `docs/guard-inventory.md`'s row. */ export function expectedTrackedSources( tracked: readonly string[], absentFromDisk: ReadonlySet ): string[] { - return tracked.filter( - (path) => - !path.includes('/generated/') && - isScannableSourceFileName(basename(path)) && - !absentFromDisk.has(path) - ); + return tracked.filter((path) => isInScopeSourcePath(path) && !absentFromDisk.has(path)); } interface DiscoveredSite { @@ -653,6 +670,46 @@ describe('pageSize call-site guard (#650)', () => { ); }); + it.each([ + ['src/api/pageSizeScan.ts', true], + ['src/screens/TrashScreen.tsx', true], + ['src/components/Button.tsx', true], + ['src/builder/libraryBrowse.mts', true], + ['src/api/generated/v1.ts', false], + ['src/api/pageSizeScan.test.ts', false], + ['src/api/pageSizeCallSites.guard.test.ts', false], + ['src/shell.css', false] + ])( + 'isInScopeSourcePath(%s) === %s — the SCOPE both directions share, pinned on its own', + (path, expected) => { + // Both `listSourceFilesFrom` and `expectedTrackedSources` call this, so scope cancels out of + // the equality between them: narrowing it moves both sets together and the shortfall + // assertion stays green. MEASURED on the previous round — excluding `src/components/` + // removed 13 real files with the whole suite green. Pinning the predicate here is what + // reddens that, since no assertion comparing the two sides can. + expect(isInScopeSourcePath(path)).toBe(expected); + } + ); + + it('expectedTrackedSources keeps in-scope, on-disk paths and drops each other kind', () => { + // The clause proof for the OTHER direction, mirroring the two `listSourceFilesFrom` fixture + // tests. Without it every term here is unpinned: the whole body could `return []` — or the + // `absentFromDisk` subtraction, the entire point of the plugin's second field, could be + // deleted — with the full suite green, which is the shape `testing.guard-ships-with-mutation-proof` + // exists to forbid and the one the previous round had to fix a layer down. + const tracked = [ + 'src/a.ts', + 'src/screens/Keep.tsx', + 'src/gone.ts', + 'src/api/generated/v1.ts', + 'src/a.test.ts', + 'src/api/pageSizeCallSites.guard.test.ts', + 'src/shell.css' + ]; + + expect(expectedTrackedSources(tracked, new Set(['src/gone.ts']))).toEqual(['src/a.ts', 'src/screens/Keep.tsx']); + }); + it('supplies content for every tracked source file on disk (a shortfall is a HOLE, not a skip)', () => { // The other direction of the set equality. `listSourceFilesFrom` drops any key the index does // not carry; this asserts nothing was dropped that the index DOES carry and that exists on @@ -661,16 +718,22 @@ describe('pageSize call-site guard (#650)', () => { // from its index spelling (`caseprobe.tsx` vs `CaseProbe.tsx`) is tracked, present, and // invisible to the intersection — the guard went blind to it rather than reporting it. const produced = new Set(listSourceFiles().map((source) => `src/${source.file}`)); - const missing = expectedTrackedSources(trackedSources.tracked, ABSENT_FROM_DISK).filter( - (path) => !produced.has(path) - ); + const expected = expectedTrackedSources(trackedSources.tracked, ABSENT_FROM_DISK); + + // Anti-vacuity, and it is load-bearing rather than decorative: without it, + // `expectedTrackedSources` returning `[]` leaves this test — and the whole 1249-test suite — + // green while the holes above go undetected, i.e. this fix reduced to a no-op with no signal. + expect(expected.length).toBeGreaterThan(50); + + const missing = expected.filter((path) => !produced.has(path)); expect( missing, `Tracked, on-disk, in-scope source file(s) that never reached the scanner. The walk cannot ` + `see them, so the population silently shrank. Usual causes: a dotfile or dot-directory ` + - `(import.meta.glob matches neither), or a name whose disk case diverges from the index ` + - `case. Fix the file or widen the glob — do NOT drop this assertion.` + `(import.meta.glob matches neither), or a name whose disk spelling diverges from its index ` + + `spelling — case under core.ignorecase, or NFD/NFC under core.precomposeunicode. Rename the ` + + `file to match the index, or widen the glob — do NOT drop this assertion.` ).toEqual([]); }); diff --git a/web/src/vite-env.d.ts b/web/src/vite-env.d.ts index 50339c663..962b521c1 100644 --- a/web/src/vite-env.d.ts +++ b/web/src/vite-env.d.ts @@ -7,7 +7,7 @@ declare module 'virtual:etv-tracked-source-files' { const trackedSources: { /** Paths relative to the Vite root (`web/`), e.g. `src/api/pageSizeScan.ts`. */ tracked: string[]; - /** Tracked paths with no file on disk — deleted in the working tree, not yet staged. */ + /** Tracked paths with no READABLE file: usually an unstaged deletion, also a broken symlink. */ absentFromDisk: string[]; }; export default trackedSources; diff --git a/web/vite-plugins/trackedSourceFiles.ts b/web/vite-plugins/trackedSourceFiles.ts index 9a2264163..4c1c69cc3 100644 --- a/web/vite-plugins/trackedSourceFiles.ts +++ b/web/vite-plugins/trackedSourceFiles.ts @@ -4,8 +4,8 @@ import { join } from 'node:path'; import type { Plugin } from 'vite'; /** - * ersatztv#819: hands `web/src/api/pageSizeCallSites.guard.test.ts` the set of files under `src/` - * that git actually tracks. + * ersatztv#819: hands `web/src/api/pageSizeCallSites.guard.test.ts` — the one SPA guard that derives a + * file population — the set of files under `src/` that git actually tracks. * * That guard asserts EXACT completeness over the files it discovers, so its population has to be * the git index — `testing.guard-derives-population-from-source`. A directory walk is not an @@ -36,10 +36,16 @@ export interface TrackedSources { /** Tracked paths under `src/`, relative to the Vite root (`web/`), e.g. `src/api/pageSizeScan.ts`. */ tracked: string[]; /** - * The subset of `tracked` with no file on disk — i.e. deleted in the working tree but not yet - * staged. Reported rather than silently removed: the consumer needs to tell a mid-edit deletion - * (a normal, transient developer state) apart from a tracked file the WALK cannot see, which is - * a real hole. Collapsing the two is how a population silently shrinks. + * The subset of `tracked` with NO READABLE FILE at that path. Usually a working-tree deletion + * that is not yet staged, which is the case the consumer tolerates — but the test is + * `existsSync`, so it also covers a tracked BROKEN SYMLINK (a committed, permanent state, not a + * mid-edit one), a sparse checkout that excludes the path, and an unreadable parent directory. + * Named for what it measures rather than for the common cause, because a consumer subtracts this + * set and a wrong name there licenses subtracting more than intended. + * + * Reported rather than silently removed: the consumer needs to tell these apart from a tracked + * file the WALK cannot see, which is a real hole. Collapsing the two is how a population shrinks + * without anyone noticing. */ absentFromDisk: string[]; } -- 2.47.3 From 2cc4abbcb3441c8e3886d3109d114ea6e8d739a0 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 29 Aug 2026 00:15:52 +0200 Subject: [PATCH 05/17] fix(819): prove the shared scope by decomposition, not by a table of examples MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 3's cold review found the same defect class for the third round running, and this time in the fix for round 2's finding. That fix pinned the shared `isInScopeSourcePath` with a hand-written table of 8 example paths. The table names four of `src/`'s eight directories, so it proved only the one mutation the previous round happened to use. MEASURED: narrowing the predicate to skip `media|app|mediaSources|schedules` removed 23 real, tracked, on-disk files — 16% of the SPA — with all 35 tests green, and `/media/` alone was green too. That is precisely what the rule it was meant to satisfy calls out: "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." So the mechanism is deleted rather than extended. The scope is now proved by DECOMPOSITION over the whole tracked population: the predicate must equal its two documented components — `!includes('/generated/')` and `isScannableSourceFileName` — each already independently pinned by the 14-case filename table. Any new term, naming any directory or any filename shape, makes the two sets differ. MEASURED red against all four mutations above plus a filename-shaped one (`!endsWith('.d.ts')`) that a path table could never have seen. The table stays as worked examples and now says so instead of claiming to be the proof. Also from round 3: - The anti-vacuity floor's stated reason was falsified by the commit that wrote it: with the fixture test present, `expectedTrackedSources -> []` reddens even without the floor. Restated as what the floor uniquely covers — vacuity in the REAL plugin output, which injected fixtures structurally cannot reach — and the floor is now derived from the index size rather than a literal that was true once. The stale "1249-test suite" figure is gone with it. - `testing.guard-derives-population-from-source` states "assert existence rather than filtering on it" in its `rule:` field, so the catalog row a reader resolves by topic said the opposite of what this guard does. The record now carries the bounded exception — filtering on existence is admissible only where the OTHER direction reports the on-disk hole, so the absent member stays representable — and the catalog is regenerated. Without that pairing it remains the original defect. - Three cross-references the refactor invalidated: the `/generated/` exclusion moved to `isInScopeSourcePath`, and one comment said "above" of a predicate defined below. - guard-inventory: the docstring quote is attributed to `test_every_index_derived_module_is_registered` rather than the module, with the elided `Path.rglob` clause restored, and the over-long line rewrapped. Refs: #819 Decisions-Edit: yes --- docs/decisions/README.md | 2 +- .../guard-derives-population-from-source.md | 15 +++++- docs/guard-inventory.md | 10 ++-- web/src/api/pageSizeCallSites.guard.test.ts | 48 ++++++++++++++----- 4 files changed, 56 insertions(+), 19 deletions(-) diff --git a/docs/decisions/README.md b/docs/decisions/README.md index ffefc6e75..6126a7c41 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. 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