← all tickets

66 — Cards-page guard machinery: round-3 test-hygiene hardening

Status: done (commit 4781010, 2026-08-04)

Blocked by: 68

Re-scoped 2026-08-04 (grilling session on ticket 67; see ticket 68): the repo is getting a real JS test harness (node --test, zero-npm), so **do not harden the source scanners — retire them.** Per finding:

tests/test_api.py and _LOCAL_ASSET, which are pytest territory regardless of the harness. (Note 68 may fix finding 9's tag-shape limits itself if it needs to — check before starting.)

statusCells order behaviorally under node.

_editor_fn and the scanner machinery's regex blind spots don't get fixed — after 68 lands, this ticket's job is to migrate the remaining scanner-guarded *logic* checks (the editor.js and game.js function guards) to node tests on extracted modules, and delete each scanner helper when it loses its last call site. Structural page checks (headers, asset tags) stay in pytest.

The original findings follow, for the record and for the migration worklist.

Where this came from: ticket 49's review panel, round 3 — nine P3 findings that survived the review cap. All are in the *guard machinery* (the tests and scanners that protect the cards-page status columns), not in shipped behavior; none affects what a user sees today. Recorded in ticket 49's comments and filed here for the hardening pass so they do not rot in a closed ticket.

Context: ticket 49 moved the column decision into server/routers/cards.py::status_columns(), added source-scanning test helpers (_js_fn, _js_code, _matching) to tests/test_static_pages.py, and content-hash cache busting via _versioned()/_LOCAL_ASSET in server/routers/pages.py.

1. _game_fn was never consolidated (glm) — tests/test_static_pages.py:661 still uses re.search(rf"function {name}\(.*?\n}}"), the column-0 closing-brace regex the round-1 fix replaced everywhere else. _js_fn is a drop-in replacement. 2. Naive // stripper survives at tests/test_static_pages.py:469 (glm) — re.sub(r"//[^\n]*", "", _editor_fn("makeDeckRow")), the exact pattern round 2 replaced in _js_code. 3. Identity invariant is pinned only over hardcoded sets (glm, terra) — tests/test_api.py:244-272 checks the 7 sg codes and 2 vanilla codes; the one implemented vp identity, 36017 Hiram "0mission" Svensson, falls through every bucket, and no assertion pins the endpoint's raw manifest passthrough for the complete identity set (test_every_card_..._carries_its_status_cells recomputes both sides from the same dict, so it is a tautology). 4. VANILLA_IDENTITIES = ["30076", "30077"] (glm) — the same snapshot anti-pattern round 2 removed from the unimplemented count; adding a third vanilla card turns the suite red for correct behavior. Derive from the manifests instead. 5. Unimplemented identities forced to has_tests: false (terra) — tests/test_api.py:268-269 rejects a truthful manifest that gains card_35001_* tests before implemented flips to true. 6. Browser cell order is unpinned (terra) — appending .reverse() to statusCells()'s map renders the columns backwards under the right headers and every current assertion still passes. 7. Source scanners are not regex-literal aware (terra) — a literal such as /[//]/ or /[}]/ in a scanned function misleads _matching()/_js_code(). 8. _js_fn function discovery can select a commented-out example (terra) — the re.search that locates the function is not syntax-aware, so // function statusCells(c) { ... } placed above the real one wins. 9. _LOCAL_ASSET only versions double-quoted, query-free, flat paths (terra, luna) — src='/static/cards.js', src="/static/cards.js?module=1" or static/js/cards.js would silently ship unversioned, recreating the stale-JS failure ticket 49 fixed.

Already rejected by the orchestrator — do not re-litigate: manifest schema validation for malformed values (manifests are generated and covered by test_card_manifest_honesty.py); status_columns on /api/cards/sg|pool|startup (no consumer); content-hash in-place-deploy race (deployment practice, not a code defect).

Blocked by: none.

Comments

2026-08-04 — done. make test: cargo 284, pytest 265, node 60, all green. The scanners are gone. _js_fn, _js_code, _matching, _editor_fn, _game_fn, _game_js, _sheet_key_handler and the TRICKY_JS fixture with its five self-tests are deleted from tests/test_static_pages.py; nothing in the suite reads a named function's source text any more, so findings 1, 2, 7 and 8 are moot rather than fixed, as the re-scope said.

Where the guards went. Two new modules, same split ticket 68 made for cards.js:

influenceSpent, readControl, activatesRead, ROW_CONTROLS, sheetMeta, sheetStats, sheetText, openSheetSteps, closeSheetSteps, focusables, sheetKeyAction. editor.js keeps appendColumns (one element per column descriptor) and a switch that applies sheet steps, and decides nothing.

ticket 60's rule; game.js builds the button and the entry divs.

Both pages now load their script with type="module" and carry a one-entry import map, so the .mjs gets a versioned URL the same way cards-logic.mjs does. tests/test_static_pages.py's import-map, MIME-type and type="module" tests are parametrized over all three module pages now instead of being cards-only.

Two guards became stronger rather than moving. "The status/influence column is on every row" used to be an append-count over one function's text; it is now a browser row has the same columns whatever the card is, which compares the whole column list across an implemented card, an unimplemented one and a banned-and-unusable one — a column added under any condition at all fails it. And the sheet's inert-before-focus ordering, which was an index() < index() over source text, is now an assertion about the ordered effect list closeSheetSteps() returns (see the new section in tests/js/README.md: making the sequence a value is what let it be tested without a DOM).

Three guards were dropped, not migrated, because they assert glue and glue is untested by the harness contract: the three cardSheetClose/cardSheetBackdrop.addEventListener('click', closeCardSheet) greps in test_every_close_path_restores_focus_to_the_invoking_row. What each path *does* once it reaches closeCardSheet is pinned in node; that the two elements are wired to it is a one-line listener in a file the contract says we do not test. Two whole-file greps of editor.js survive on purpose (matchMedia('(max-width: 700px)') and the bar-filters DOM move) — both pin the script agreeing with the *page's* stylesheet or markup, which no module can see, and neither needs a parser.

Findings 3, 4, 5 (tests/test_api.py): SG_IDENTITIES and VANILLA_IDENTITIES are gone. The passthrough invariant now runs over every identity in the manifests, comparing the endpoint's payload against the manifest *files* — an independent source, so it is no longer a tautology and 36017 Hiram "0mission" Svensson is covered by construction. The buckets (implemented, unimplemented, vanilla) are derived from the manifests with a non-vacuousness test standing in for the deleted snapshots, and nothing forces `has_tests: false on an unimplemented card: its cells come from status_columns()` applied to the manifest entry, so a truthful manifest that gains card_35001_* tests early stays green. Verified by mutation: an endpoint that lies about 36017 was green under the old tests and is red under the new ones; the two *truthful* manifest changes that used to turn the suite red (a third vanilla identity, an unimplemented card gaining tests) now pass.

Finding 9 (_LOCAL_ASSET): the stamper now recognizes either quote, a URL that already carries query parameters of its own (they survive; its own v= is replaced, not repeated), a path with or without its leading slash, and subdirectories under static/. A .. in the path is refused rather than resolved, and a URL naming no file is still left alone. Six tag shapes are parametrized in test_every_shape_of_asset_tag_gets_versioned.

2026-08-04 — review follow-up. Three P2s from the review of de5b4a7, fixed in one pass; two further findings were rejected by the orchestrator (a failed refetch leaving rendered full-log entries on screen — pre-existing glue, the viewer's own data; and the retired scanners leaving editor.js/game.js glue untested — ticket 68's decided design).

/static/cards.js?module=1#top was stamped ...#top&v=<digest> and the browser read the version as fragment text — the asset shipped unversioned, which is finding 9's failure again. The fragment is its own group now, re-appended after the query. test_a_url_fragment_does_not_swallow_the_version.

/outside.js, and static_dir / "/outside.js" discards static_dir, so the ..-only guard passed and a file outside the tree got hashed. Absolute paths are refused and the resolved path is confirmed to be under static_dir. The double-slash form (both a real file outside tmp_path and /etc/hostname) joins test_an_asset_the_server_does_not_serve_is_left_alone.

showHiddenInfo alone, which game.js persists in localStorage across games, while reveal_hidden only reaches the server when the checkbox is *changed*. Game B therefore opened with the setting on, no log_full in the payload, and a button whose only effect was to redraw the redacted log. The toggle now also requires Array.isArray(state.log_full); an empty log_full still counts. there is no toggle to press when the payload carries no full log.

make test: cargo 284, pytest 266, node 61, all green.

Counts: pytest 263 → 265 (test_static_pages.py 59 → 59: fourteen scanner guards and self-tests out, fourteen served-page and stamper tests in; test_api.py 18 → 20). node 24 → 60: the migrated editor and game-log guards became 36 cases, since running the code reaches shapes no scan could ask about (no identity chosen yet, a deck row for a card the pool does not know, an empty dialog, a state whose log_full has not arrived). Every new node test was checked by mutation — a conditional influence column, a badge appended to the row, focus before inert, a trap that ignores escaped focus, and a full log trusted without the setting each turn the suite red.