timothy/ersatztv
1
0
Fork 0
Code Issues 54 Pull Requests 6 Actions Packages Projects Releases 9 Wiki Activity

The SPA page-size guard enumerates web/src with a Vite glob while claiming completeness #819

New Issue
Closed
opened 2026-08-22 17:38:54 +02:00 by timothy · 2 comments
timothy commented 2026-08-22 17:38:54 +02:00
Owner
Copy Link
Copy Source

Found by the cold review of #806 (PR #818), by sweeping the whole repo rather than the file list #806 named — the same method that turned up test_pr_changed_files.py inside that PR.

The defect

web/src/api/pageSizeCallSites.guard.test.ts:352 derives its population from the filesystem:

const rawSourceModules = import.meta.glob('/src/**/*.{ts,tsx,mts,cts}', {
  query: '?raw', import: 'default', eager: true
}) as Record<string, string>;

and then asserts exact completeness against REGISTRY — a newly discovered, unregistered site fails, which is the whole point of the guard (#650 could otherwise recur invisibly).

That is testing.guard-derives-population-from-source's file-population case, which #806 instantiated: a directory enumeration is not an authoritative source. An untracked .ts/.tsx under web/src/ containing a page-size call — a scratch file, a half-finished component, an editor dropping — enters the population and fails as unregistered on that checkout, while CI never sees it. Developer-red / CI-green, the #778 shape.

The direction is the safe one (it over-enumerates, so it cannot go silently blind), but it is the shape #806 exists to remove.

Why it was not fixed in #818

The glob is not incidental — it is a documented, deliberately-chosen workaround, and the obvious fix runs straight into a prior reverted attempt. From the comment above it:

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 too, and a file-local /// <reference types="node" /> was tried and reverted — under tsc -b's 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).

So filtering the glob's result against git ls-files needs node:child_process in a project that deliberately excludes Node types, and the last attempt to admit them broke three unrelated tests. That is a real piece of work with a known obstacle, not a one-line conversion, and #818's worktree cannot even run the web suite.

Shape of a fix

The glob is resolved by Vite at transform time and cannot consult git. The filter can, though, if it runs somewhere with Node available:

  • keep import.meta.glob as the discovery mechanism, and intersect its keys with a tracked-file list obtained at test time — either through a small Vite plugin / define that injects git ls-files output at config time (where Node IS available), or a vitest globalSetup, both of which sidestep the tsconfig.app.json problem entirely;
  • or state the residual in the guard, the way #806 required for every population it assessed either way.

Either is acceptable; silence is not, which is the rule #774 and #806 both land on.

Related

  • #806 / PR #818 — the file-population instantiation, and the audit table in docs/guard-inventory.md that now carries this as a deferred row
  • docs/guard-inventory.md scope-limit item 2 already records that C# and TypeScript guards sit outside the inventory population, so this guard has no row and no MUTATION grade either
  • ErsatzTV.Mcp.Tests/ToolCatalogTests.cs is the other guard in that class; it derives from the generated OpenAPI document rather than a directory walk, so it is not affected

Done-when

  • pageSizeCallSites.guard.test.ts's population is derived from the git index, or the residual is stated in the guard with its reason
  • If converted: a regression case proving an untracked file matching the scope does not enter the population
  • docs/guard-inventory.md's deferred row updated or removed
  • Adversarial review passed
Found by the cold review of #806 (PR #818), by sweeping the whole repo rather than the file list #806 named — the same method that turned up `test_pr_changed_files.py` inside that PR. ## The defect `web/src/api/pageSizeCallSites.guard.test.ts:352` derives its population from the filesystem: ```ts const rawSourceModules = import.meta.glob('/src/**/*.{ts,tsx,mts,cts}', { query: '?raw', import: 'default', eager: true }) as Record<string, string>; ``` and then asserts **exact completeness** against `REGISTRY` — a newly discovered, unregistered site fails, which is the whole point of the guard (#650 could otherwise recur invisibly). That is `testing.guard-derives-population-from-source`'s file-population case, which #806 instantiated: a directory enumeration is not an authoritative source. An untracked `.ts`/`.tsx` under `web/src/` containing a page-size call — a scratch file, a half-finished component, an editor dropping — enters the population and fails as unregistered on that checkout, while CI never sees it. Developer-red / CI-green, the #778 shape. The direction is the safe one (it over-enumerates, so it cannot go silently blind), but it is the shape #806 exists to remove. ## Why it was not fixed in #818 The glob is not incidental — it is a documented, deliberately-chosen workaround, and the obvious fix runs straight into a prior reverted attempt. From the comment above it: > 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 too, and a file-local `/// <reference types="node" />` was tried and reverted — under `tsc -b`'s 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). So filtering the glob's result against `git ls-files` needs `node:child_process` in a project that deliberately excludes Node types, and the last attempt to admit them broke three unrelated tests. That is a real piece of work with a known obstacle, not a one-line conversion, and #818's worktree cannot even run the web suite. ## Shape of a fix The glob is resolved by Vite at transform time and cannot consult git. The filter can, though, if it runs somewhere with Node available: - keep `import.meta.glob` as the discovery mechanism, and intersect its keys with a tracked-file list obtained at test time — either through a small Vite plugin / `define` that injects `git ls-files` output at config time (where Node IS available), or a vitest `globalSetup`, both of which sidestep the `tsconfig.app.json` problem entirely; - or state the residual in the guard, the way #806 required for every population it assessed either way. Either is acceptable; silence is not, which is the rule #774 and #806 both land on. ## Related - #806 / PR #818 — the file-population instantiation, and the audit table in `docs/guard-inventory.md` that now carries this as a deferred row - `docs/guard-inventory.md` scope-limit item 2 already records that C# and TypeScript guards sit outside the inventory population, so this guard has no row and no `MUTATION` grade either - `ErsatzTV.Mcp.Tests/ToolCatalogTests.cs` is the other guard in that class; it derives from the generated OpenAPI document rather than a directory walk, so it is not affected ## Done-when - [x] `pageSizeCallSites.guard.test.ts`'s population is derived from the git index, or the residual is stated in the guard with its reason - [x] If converted: a regression case proving an untracked file matching the scope does not enter the population - [x] `docs/guard-inventory.md`'s deferred row updated or removed - [x] Adversarial review passed
timothy added the ci-cdfrontendpriority: medium labels 2026-08-22 18:08:43 +02:00
timothy added the in-progress label 2026-08-28 22:12:10 +02:00
timothy commented 2026-08-28 22:12:10 +02:00
Author
Owner
Copy Link
Copy Source

Claiming this (Claude Code session, 2026-08-28). Pre-claim checks all clear: no open PR references #819, git ls-remote --heads origin '*819*' is empty, no prior comments on the issue, and origin/main fetched fresh at e11d57719.

Plan: keep import.meta.glob as the discovery mechanism and intersect its keys with a git ls-files-derived tracked set obtained where Node IS available (vitest globalSetup / a config-time seam), sidestepping the tsconfig.app.json Node-types obstacle entirely. Will also update the deferred row in docs/guard-inventory.md.

Not bundling #820 — it is the same defect class but requires a whole new compiler-API guard, which is its own session.

Claiming this (Claude Code session, 2026-08-28). Pre-claim checks all clear: no open PR references #819, `git ls-remote --heads origin '*819*'` is empty, no prior comments on the issue, and `origin/main` fetched fresh at `e11d57719`. Plan: keep `import.meta.glob` as the discovery mechanism and intersect its keys with a `git ls-files`-derived tracked set obtained where Node IS available (vitest `globalSetup` / a config-time seam), sidestepping the `tsconfig.app.json` Node-types obstacle entirely. Will also update the deferred row in `docs/guard-inventory.md`. Not bundling #820 — it is the same defect class but requires a whole new compiler-API guard, which is its own session.
timothy commented 2026-08-29 11:14:43 +02:00
Author
Owner
Copy Link
Copy Source

Closing record

Outcome: Shipped in PR #875. web/src/api/pageSizeCallSites.guard.test.ts now derives its file
population from the git index instead of import.meta.glob, via a new Vite plugin
(web/vite-plugins/trackedSourceFiles.ts) that reaches git in Vite's own Node context and hands the
result to the app project as a virtual module — which is how the index is reached without admitting
@types/node to tsconfig.app.json, the obstacle that deferred this in #818.

Root cause: The guard asserted EXACT completeness over a population enumerated by a directory
walk. A walk answers a question about the machine, not about the repo, so an untracked .ts/.tsx
under web/src/ entered the population and failed as unregistered on that developer's checkout while
CI — which only ever checks out tracked files — stayed green.

Decisions/conventions changed: testing.guard-derives-population-from-source gains two
additions (catalog regenerated):

  • a bounded exception to its assert-existence rule — a guard MAY filter on existence where the
    OTHER direction reports the on-disk hole, so the absent member stays representable;
  • the closed-form criterion for a restatement proof: where a completeness claim rests on a POLICY
    predicate with no external source, the restatement proving it must share no helper, at any
    depth    , with the predicate it checks, and the population it reads must be cross-checked against an
    independently derived list.

Reusable knowledge: A proof that shares anything with its subject proves nothing about the
shared part. Sixteen review rounds all found one mechanism — the shared thing sits on both sides of
the comparison and cancels, so a narrowing shrinks both and passes. It recurred through five distinct
carriers: the scope itself, a table of example paths (4 of 8 directories), a delegated sub-predicate,
a basename helper that read as plumbing rather than policy, and finally the population array every
comparison was derived from. Two of those were blind — they hid a planted call site with the
whole suite green.

Two things ended it. First, stating the criterion as a checkable property rather than an
instruction: "restate the scope" gives no way to tell when you are done; "share no helper at any
depth" does. Second, where a restatement cannot police the population it reads, cross-check against
a genuinely different query (ls-files --others beside ls-files) rather than restating harder.

A second, sharper lesson: for the last six rounds the only defects were false coverage/direction
claims in prose — sentences asserting that something was caught, pinned, or harmless, written
without measurement, in a change whose entire subject is the difference between a check and the
appearance of one. Four of them were mine. One was actively harmful: docs/testing.md told
developers to "restart the watcher", which resolves a noisy red by putting them into a blind green.
Fail-direction is the argument for tolerating a residual, so it must be measured, never inferred.

Third: a "gitless replica" that deletes .git but leaves the git binary on PATH reproduces only
half the condition. That gap shipped a Dockerfile line which would have failed docker build on
every push to main and every release tag.

Verification: npx vitest run 119 files / 1272 tests; tsc -b --force; eslint .;
npm run build; pytest scripts/tests 1228 passed / 2 skipped; decisions_validate OK; catalog
zero-diff. Defect witnessed before the fix and each residual's direction measured by planting a real
pageSize call site. Docker web-build stage replicated with both .git absent and git off
PATH: rc=0, 117 files / 1226 tests. Sixteen cold-context adversarial review rounds in isolated
worktrees; the final two returned MERGEABLE with nothing at any severity.

Deferred: Six residuals, each stated in the guard with its measured direction — (2), (4) and (5)
fail-noisy; (1) a three-site coordinated edit and (3) a union-preserving mispartition are BLIND;
(6) watch-mode staleness is each in turn. (6)'s obvious fix (configureServer invalidation) was
implemented, measured and rejected: git add fires no watcher event, so it blinds the
create-then-stage sequence. No follow-up issue — these are properties of the mechanism, recorded
where the next author edits, not open work.

Docs updated: docs/guard-inventory.md (audit row converted from DEFERRED, scope item, residual
list), docs/testing.md (the checkout-vs-binary distinction and the watch-mode caveat),
docs/spa-conventions.md (§1 gains the web/vite-plugins/ build-time seam),
docs/decisions/records/testing/guard-derives-population-from-source.md + regenerated
docs/decisions/README.md.

## Closing record **Outcome:** Shipped in PR #875. `web/src/api/pageSizeCallSites.guard.test.ts` now derives its file population from the git index instead of `import.meta.glob`, via a new Vite plugin (`web/vite-plugins/trackedSourceFiles.ts`) that reaches git in Vite's own Node context and hands the result to the app project as a virtual module — which is how the index is reached without admitting `@types/node` to `tsconfig.app.json`, the obstacle that deferred this in #818. **Root cause:** The guard asserted EXACT completeness over a population enumerated by a directory walk. A walk answers a question about the machine, not about the repo, so an untracked `.ts`/`.tsx` under `web/src/` entered the population and failed as unregistered on that developer's checkout while CI — which only ever checks out tracked files — stayed green. **Decisions/conventions changed:** `testing.guard-derives-population-from-source` gains two additions (catalog regenerated): - a **bounded exception** to its assert-existence rule — a guard MAY filter on existence where the OTHER direction reports the on-disk hole, so the absent member stays representable; - the **closed-form criterion** for a restatement proof: where a completeness claim rests on a POLICY predicate with no external source, the restatement proving it must share **no helper, at any depth**, with the predicate it checks, and the population it reads must be cross-checked against an independently derived list. **Reusable knowledge:** *A proof that shares anything with its subject proves nothing about the shared part.* Sixteen review rounds all found one mechanism — the shared thing sits on both sides of the comparison and cancels, so a narrowing shrinks both and passes. It recurred through five distinct carriers: the scope itself, a table of example paths (4 of 8 directories), a delegated sub-predicate, a `basename` helper that read as plumbing rather than policy, and finally the population array every comparison was derived from. Two of those were **blind** — they hid a planted call site with the whole suite green. Two things ended it. First, stating the criterion as a **checkable property** rather than an instruction: "restate the scope" gives no way to tell when you are done; "share no helper at any depth" does. Second, where a restatement cannot police the population it reads, **cross-check against a genuinely different query** (`ls-files --others` beside `ls-files`) rather than restating harder. A second, sharper lesson: for the last six rounds the only defects were **false coverage/direction claims in prose** — sentences asserting that something was caught, pinned, or harmless, written without measurement, in a change whose entire subject is the difference between a check and the appearance of one. Four of them were mine. One was actively harmful: `docs/testing.md` told developers to "restart the watcher", which resolves a noisy red by putting them into a blind green. **Fail-direction is the argument for tolerating a residual, so it must be measured, never inferred.** Third: a "gitless replica" that deletes `.git` but leaves the `git` binary on `PATH` reproduces only half the condition. That gap shipped a Dockerfile line which would have failed `docker build` on every push to `main` and every release tag. **Verification:** `npx vitest run` 119 files / 1272 tests; `tsc -b --force`; `eslint .`; `npm run build`; `pytest scripts/tests` 1228 passed / 2 skipped; `decisions_validate` OK; catalog zero-diff. Defect witnessed before the fix and each residual's direction measured by planting a real `pageSize` call site. Docker web-build stage replicated with **both** `.git` absent **and** `git` off `PATH`: rc=0, 117 files / 1226 tests. Sixteen cold-context adversarial review rounds in isolated worktrees; the final two returned MERGEABLE with nothing at any severity. **Deferred:** Six residuals, each stated in the guard with its measured direction — (2), (4) and (5) fail-noisy; (1) a three-site coordinated edit and (3) a union-preserving mispartition are BLIND; (6) watch-mode staleness is each in turn. (6)'s obvious fix (`configureServer` invalidation) was implemented, measured and **rejected**: `git add` fires no watcher event, so it blinds the create-then-stage sequence. No follow-up issue — these are properties of the mechanism, recorded where the next author edits, not open work. **Docs updated:** `docs/guard-inventory.md` (audit row converted from DEFERRED, scope item, residual list), `docs/testing.md` (the checkout-vs-binary distinction and the watch-mode caveat), `docs/spa-conventions.md` (§1 gains the `web/vite-plugins/` build-time seam), `docs/decisions/records/testing/guard-derives-population-from-source.md` + regenerated `docs/decisions/README.md`.
timothy removed the in-progress label 2026-08-29 11:14:46 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
ad-hoc
One-off / ad-hoc work not tracked by a dedicated issue
 api
REST API / HTTP endpoints
 bug
Something isn't working
 ci-cd
Build, test, deploy pipeline
 content
Channel content / schedules / playlists
 dependencies
Dependency updates (Renovate)
 enhancement
New feature or improvement
 frontend
ChicoryTV React SPA frontend
 in-progress
Claimed by an active session — do not pick up
 jellyfin
Jellyfin tuner / IPTV integration
 parked
Excluded from automatic queue pickup; work only when explicitly selected
 priority: high
priority: low
priority: medium
review
Adversarial review finding
 security
Security / vulnerability fix
No labels ci-cd frontend priority: medium
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
renovate timothy (Timothy)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: timothy/ersatztv#819