diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e97e0ff8..91519d8a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,6 +38,12 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 + # First, because it needs nothing and takes a second: the gate that stops + # an unstamped bundle shipping was deleted by injection on PR #69 and the + # assembly still exited 0 (improvise-w8ih). It guards the assemble step + # below, so it fails under this job's name rather than another's. + - name: The stamp gate refuses what it claims to + run: ./scripts/assemble-stamp-test.sh - uses: dtolnay/rust-toolchain@stable with: targets: wasm32-unknown-unknown diff --git a/context/code/clients.md b/context/code/clients.md index c4d3a2f4..88ef19ad 100644 --- a/context/code/clients.md +++ b/context/code/clients.md @@ -1560,3 +1560,10 @@ rather than an accident: the moment a `web/tests/*.test.js` file needs `web/pkg/`, it inherits the four-minute build and the argument for it evaporates. That is why the one test that does need the bundle lives in `web/tests/bundle/` and runs in the browser job instead. + +Until improvise-4ktt the mechanism was a directory convention and a glob, and +nothing checked it. `bundle_freedom.test.js` does now: it reads its own sibling +files and refuses one that names the bundle directory, and its companion +asserts `web/tests/bundle/` still holds a test that *does* — a scoping rule is +uninteresting if nothing falls outside it. Evidence in +`context/test-evidence/tooling.md`. diff --git a/context/test-evidence/tooling.md b/context/test-evidence/tooling.md new file mode 100644 index 00000000..0f0b61c6 --- /dev/null +++ b/context/test-evidence/tooling.md @@ -0,0 +1,48 @@ +# Test evidence — the repository's own gates + +Evidence for the checks in `scripts/` and for the tests that hold CI's shape. +The arguments behind the checkers themselves are in `context/code/tooling.md`; +this file records what fault each test was watched to catch. + +Note what is deliberately absent. `scripts/check-comment-runs.py --self-test`, +`scripts/kb-test.sh` and `scripts/assemble-stamp-test.sh` are check-the-checker +suites whose cases are already injected faults, and none of their case names is +a Rust `fn` or a JS `test(` the evidence checker could resolve — so their +evidence is the case list itself plus `context/code/tooling.md`, and inventing +headings for them here would be headings naming nothing. + +### "no test in web/tests/ reaches for a built bundle" + +The `js` CI job runs `node --test web/tests/*.test.js` and installs nothing; the +`browser` job runs `web/tests/bundle/*.test.js` after three wasm-pack builds. +The moment a flat `web/tests/` file reads `web/pkg/`, the fast job inherits the +slow one's dependency — and the likely repair is to add a build step to it, +silently spending the ~4m30s the split exists to avoid. Until improvise-4ktt +that constraint was a directory convention and a glob, checked by nothing, and +it failed in the direction that looks like ordinary breakage: a confusing ENOENT +in CI on a machine where the bundle happens not to be built. + +Fault injected, 2026-09-21: appended +`const injected = readFileSync("../pkg/improvise_wasm_client.js");` to +`web/tests/grid_plan.test.js`. The test failed naming that file and pointing at +`web/tests/bundle/`. Reverted; green again. + +Two things the test does that a simpler one would not. It asserts its own +filename is among the files it scans, so it cannot become the one place a +violation may hide. And it strips comments before matching, because this file's +own header and the test's header both name the directory in prose — a lexical +grep over raw text would have had to exempt them, and an exemption is where the +next violation goes. + +### "the subdirectory that MAY use a bundle is not the one being checked" + +The companion, and the one that stops the first from passing vacuously. If +`web/tests/bundle/` were emptied or its files stopped touching the bundle, the +first test would still be green while having nothing to be scoped *against* — +"no flat test uses the bundle" is uninteresting if no test anywhere does. This +asserts the other half of the split exists and really does what the flat half +is forbidden to do. + +Fault injected, 2026-09-21: moved `web/tests/bundle/wasm_surface.test.js` out of +the tree. The test failed with "web/tests/bundle/ is empty — the split lost its +other half" rather than passing. Restored; green again. diff --git a/scripts/assemble-stamp-test.sh b/scripts/assemble-stamp-test.sh new file mode 100755 index 00000000..d8362202 --- /dev/null +++ b/scripts/assemble-stamp-test.sh @@ -0,0 +1,73 @@ +#!/usr/bin/env bash +# check_the_checker for scripts/assemble-stamp.sh. The gate it holds was +# deleted on PR #69 by injection and the assembly still exited 0 with both +# failures printed -- a guard nobody had watched fail (improvise-w8ih). +set -uo pipefail + +here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +RUN="$here/assemble-stamp.sh" +work="$(mktemp -d)" +trap 'rm -rf "$work"' EXIT + +pass=0; fail=0 +ok() { printf ' ok %s\n' "$1"; pass=$((pass+1)); } +bad() { printf ' FAIL %s (exit %s, wanted %s)\n' "$1" "$2" "$3"; fail=$((fail+1)); } + +# $1 name, $2 expected exit, $3 expected stderr substring ("" for none), rest: args +run_case() { + local name="$1" want="$2" says="$3"; shift 3 + local err + err="$(sh "$RUN" "$@" 2>&1 >/dev/null)" + local got=$? + if [ "$got" != "$want" ]; then + bad "$name" "$got" "$want" + elif [ -n "$says" ] && [[ "$err" != *"$says"* ]]; then + bad "$name: stderr did not name it" "$err" "$says" + else + ok "$name" + fi +} + +printf 'import "./pkg/improvise_wasm_client.js?v=abc";\n' > "$work/stamped.js" +printf 'import "./worker-pkg/improvise_worker_server.js?v=abc";\n' > "$work/stamped2.js" +printf 'import "./pkg/improvise_wasm_client.js";\n' > "$work/bare.js" +printf 'import "./worker-pkg/improvise_worker_server.js";\n' > "$work/bare2.js" + +run_case "a stamped file passes" 0 "" \ + "$work/stamped.js" "pkg/improvise_wasm_client.js?v=abc" + +run_case "an unstamped file is refused and named" 1 "bare.js" \ + "$work/bare.js" "pkg/improvise_wasm_client.js?v=abc" + +# The bug the gate exists for: without the final `test -z`, the misses print +# and the script exits 0. Both must be NAMED, which is what exiting on the +# first one cost. +run_case "two misses are both named, not just the first" 1 "bare2.js" \ + "$work/bare.js" "pkg/improvise_wasm_client.js?v=abc" \ + "$work/bare2.js" "worker-pkg/improvise_worker_server.js?v=abc" + +run_case "a miss among hits is still refused" 1 "bare2.js" \ + "$work/stamped.js" "pkg/improvise_wasm_client.js?v=abc" \ + "$work/bare2.js" "worker-pkg/improvise_worker_server.js?v=abc" + +run_case "all four stamped is the shipping case" 0 "" \ + "$work/stamped.js" "pkg/improvise_wasm_client.js?v=abc" \ + "$work/stamped2.js" "worker-pkg/improvise_worker_server.js?v=abc" + +# A file the assembly never copied is indistinguishable from an unstamped one +# to `grep`, which errors rather than failing quietly -- but only because the +# -f test runs first. Without it `set -e` would abort mid-list and the later +# files would go unreported, which is the failure this gate was built to end. +run_case "a file that does not exist is a miss, not an abort" 1 "vanished.js" \ + "$work/vanished.js" "anything" \ + "$work/bare2.js" "worker-pkg/improvise_worker_server.js?v=abc" + +# A caller that drops one argument would otherwise have the last FILE read as +# a PATTERN over the file before it -- a check that passes by accident. +run_case "an odd number of arguments is a usage error" 2 "pairs" \ + "$work/stamped.js" "pkg/improvise_wasm_client.js?v=abc" "$work/bare.js" + +run_case "no arguments at all is a usage error, not a pass" 2 "usage" + +printf '\nassemble-stamp-test: %d passed, %d failed\n' "$pass" "$fail" +[ "$fail" -eq 0 ] diff --git a/scripts/assemble-stamp.sh b/scripts/assemble-stamp.sh new file mode 100755 index 00000000..1c82f9df --- /dev/null +++ b/scripts/assemble-stamp.sh @@ -0,0 +1,38 @@ +#!/bin/sh +# Every cache-bust stamp web/assemble.sh writes must be readable back. +# +# Usage: scripts/assemble-stamp.sh FILE PATTERN [FILE PATTERN ...] +# +# It RECORDS every miss and fails once at the end, naming all of them. Exiting +# on the first named one file and never reached the rest, so a shape change +# that broke two stamps cost two wasm-pack rebuilds to diagnose -- the slowest +# loop in this repo (improvise-x4wv). +# +# It lives here rather than inside assemble.sh because the gate is the whole +# guard and deleting it was invisible: both misses still printed and the +# assembly exited 0 (improvise-w8ih). Exercising it in assemble.sh would need a +# fabricated web/pkg, which is a second answer to "what does the site assemble +# from"; exercising it here needs nothing. scripts/assemble-stamp-test.sh is +# the check_the_checker, and CI runs it. +set -eu + +test $# -ge 2 || { echo "usage: assemble-stamp.sh FILE PATTERN [FILE PATTERN ...]" >&2; exit 2; } +test $((($# % 2))) -eq 0 || { echo "assemble-stamp: FILE and PATTERN come in pairs" >&2; exit 2; } + +missing="" +while [ $# -ge 2 ]; do + file=$1 + pattern=$2 + shift 2 + if [ -f "$file" ] && grep -q "$pattern" "$file"; then + grep -n "$pattern" "$file" + else + echo "assemble: $file is missing the cache-bust stamp: $pattern" >&2 + missing="${missing} $file" + fi +done + +test -z "$missing" || { + echo "assemble: refusing to ship; unstamped:${missing}" >&2 + exit 1 +} diff --git a/web/assemble.sh b/web/assemble.sh index a1492d1e..94307415 100755 --- a/web/assemble.sh +++ b/web/assemble.sh @@ -71,33 +71,15 @@ sedi "s|'improvise_worker_server_bg\.wasm'|'improvise_worker_server_bg.wasm?v=${ # One `grep` over all four files succeeds when ANY one of them matches, so # three failed substitutions could still produce a "successful" assembly under -# `set -e` (found by Copilot on PR #24). These checks are the only guard -# against the generated glue changing shape, so each stamped reference is -# verified on its own and names itself when it is missing. -# -# `stamped` RECORDS rather than exits, and the gate below is what fails the -# build. Exiting on the first failure named one file and never reached the -# rest, so a shape change that broke two stamps cost two wasm-pack rebuilds to -# diagnose -- the slowest loop in this repo (improvise-x4wv). The gate is the -# whole guard: without it every failure prints and the assembly succeeds, which -# is the check-that-does-not-check this whole file is full of fixes for. -missing="" -stamped() { - if grep -q "$2" "$1"; then - grep -n "$2" "$1" - else - echo "assemble: $1 is missing the cache-bust stamp: $2" >&2 - missing="${missing} $1" - fi -} -stamped "$app/app.js" "pkg/improvise_wasm_client.js?v=${token}" -stamped "$app/worker.js" "worker-pkg/improvise_worker_server.js?v=${token}" -stamped "$app/pkg/improvise_wasm_client.js" "improvise_wasm_client_bg.wasm?v=${token}" -stamped "$app/worker-pkg/improvise_worker_server.js" "improvise_worker_server_bg.wasm?v=${token}" -test -z "$missing" || { - echo "assemble: refusing to ship; unstamped:${missing}" >&2 - exit 1 -} +# `set -e` (found by Copilot on PR #24). Each stamped reference is therefore +# verified on its own, by scripts/assemble-stamp.sh -- which is a separate +# script so its refusals can be exercised without fabricating a web/pkg +# (improvise-w8ih). Its argument is FILE PATTERN, four times. +sh "$root/scripts/assemble-stamp.sh" \ + "$app/app.js" "pkg/improvise_wasm_client.js?v=${token}" \ + "$app/worker.js" "worker-pkg/improvise_worker_server.js?v=${token}" \ + "$app/pkg/improvise_wasm_client.js" "improvise_wasm_client_bg.wasm?v=${token}" \ + "$app/worker-pkg/improvise_worker_server.js" "improvise_worker_server_bg.wasm?v=${token}" # Every relative import the two entry points make must have been copied. The # cp list above is a second answer to "what ships", and a missing sibling is a diff --git a/web/tests/bundle_freedom.test.js b/web/tests/bundle_freedom.test.js new file mode 100644 index 00000000..8a06b9dc --- /dev/null +++ b/web/tests/bundle_freedom.test.js @@ -0,0 +1,45 @@ +// The `js` CI job must never need a built wasm bundle. See +// context/code/clients.md; the split is what keeps it seconds, not ~4m30s. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync, readdirSync } from "node:fs"; +import { fileURLToPath } from "node:url"; + +const dir = fileURLToPath(new URL(".", import.meta.url)); + +/// Built by concatenation so this file carries no literal of the thing it +/// forbids, and is therefore checked by itself rather than exempt from itself. +const FORBIDDEN = "p" + "kg"; + +/// Comments may name the directory — that is what the header above does. Only +/// what the code reaches for is a dependency, so strip comments first, exactly +/// as render_surface.test.js does. +const code = (name) => + readFileSync(dir + name, "utf8") + .replace(/\/\*[\s\S]*?\*\//g, " ") + .replace(/(^|[^:"'`])\/\/.*$/gm, "$1"); + +test("no test in web/tests/ reaches for a built bundle", () => { + const siblings = readdirSync(dir).filter((f) => f.endsWith(".test.js")); + assert.ok(siblings.length > 1, `found ${siblings.length} test files — the glob moved`); + assert.ok(siblings.includes("bundle_freedom.test.js"), "this file must check itself"); + + const offenders = siblings.filter((f) => code(f).includes(FORBIDDEN)); + assert.deepEqual( + offenders, + [], + `these belong in web/tests/bundle/, which the browser job runs: ${offenders}`, + ); +}); + +test("the subdirectory that MAY use a bundle is not the one being checked", () => { + // Reading it proves the exemption is a real directory rather than a + // convention nobody kept, and that this check is scoped to the flat listing. + const nested = readdirSync(dir + "bundle").filter((f) => f.endsWith(".test.js")); + assert.ok(nested.length > 0, "web/tests/bundle/ is empty — the split lost its other half"); + assert.ok( + nested.some((f) => readFileSync(dir + "bundle/" + f, "utf8").includes(FORBIDDEN)), + "no bundle test reaches for a bundle, so this check has nothing to be scoped against", + ); +});