diff --git a/.claude/skills/ci-preflight/SKILL.md b/.claude/skills/ci-preflight/SKILL.md index fe213cb..1d1233c 100644 --- a/.claude/skills/ci-preflight/SKILL.md +++ b/.claude/skills/ci-preflight/SKILL.md @@ -7,7 +7,7 @@ description: Use before pushing a branch, opening or updating a PR, or whenever ## Overview -`.github/workflows/ci.yml` is 15 blocking steps across two jobs. Every one of them can run +`.github/workflows/ci.yml` is 17 blocking steps across two jobs. Every one of them can run locally, so a red CI run is always avoidable — `bun test` passing is not evidence, and it is the single most common reason work gets handed over broken. @@ -19,10 +19,10 @@ node .claude/skills/ci-preflight/run-ci.mjs ~2.5 minutes warm on a green tree. Full output per step goes to `node_modules/.cache/ci-preflight/.log`; only a summary and a tail of each failure -reach stdout, so a red run costs a few hundred tokens rather than the ~40k that fourteen +reach stdout, so a red run costs a few hundred tokens rather than the ~40k that sixteen raw `bun run` calls would. -Do not hand-run the fourteen commands instead. Two things go wrong when you do: +Do not hand-run the sixteen commands instead. Two things go wrong when you do: - **Order is load-bearing.** `test`, `api`, `lint:publish` and every `verify:*` gate resolve `@dexpace/core` by package name, which lands in `packages/core/dist/`. Run any of them @@ -35,7 +35,7 @@ Do not hand-run the fourteen commands instead. Two things go wrong when you do: 1. **Run it.** Add `--skip-install` only if you have not touched `package.json` since the last install. **Before you push, add `--clean`** — a warm tree is blind to a whole class of defect CI hits on its first step. (The pinned Bun needs no flag; it is the default.) -2. **All green** → say so plainly: CI is all good, naming the count (`all 15 steps passed`). +2. **All green** → say so plainly: CI is all good, naming the count (`all 17 steps passed`). Nothing else to do. 3. **Anything red** → report the findings to the user *first*: which gates failed, what each one means, and the fix you intend. One line per finding, not a transcript dump. @@ -107,6 +107,7 @@ Both of these will make you report a passing gate that CI rejects. | `lint` | Formatting **and** type-aware rules; formatting is an error, not a warning. | `bun run fix` first — it clears every prettier finding. Hand-fix what survives: 70-line function cap, `max-depth` 3, `max-params` 3, explicit return types on exported members. Every `eslint-disable` needs a `-- reason`. | | `build` | Emit failed. **Blocks the eleven gates below it**, which the runner reports `SKIP`. | Fix this before reading anything else; the skipped gates are unknown, not passing. | | `test` | A failing test, *or* the silent coverage floor (see above). | If the tail says `0 fail`, it is coverage — find the file that dropped below 0.8 in the printed table and test it. Otherwise fix the test or the code. | +| `test:scripts` | A gate's own logic broke, or the knowledge corpus shifted under an assertion that pins its shape. | `node --test scripts/.test.mjs` for detail. If it is `knowledge.test.mjs`'s ID-less-topic count, a corpus edit gave a previously ID-less topic its first requirement ID — confirm that was intended, then move the number in the test, `CLAUDE.md` and `knowledge-lookup/SKILL.md` together. Otherwise fix the gate; never relax the assertion to match a degraded gate. | | `api` | The committed `etc/.api.md` no longer matches the built surface, or an export lacks TSDoc. | Intended export change: `cd packages/ && bun run api:local`, then commit the regenerated report. `(undocumented)` in the diff means the export needs a `@public` block, plus `@throws` naming each catchable error class. **Unintended** change: revert the export, don't bless the report. | | `lint:publish` | `publint` + `attw` on every built package's `exports` map, `types`/`main` fields, and declaration resolution. | Fix the manifest. `cjs-resolves-to-esm` is already ignored by design (ESM-only); every other rule is real. | | `verify:dual-consumption` | A built package is no longer importable and runnable by plain `node` through its package name. | Usually a broken `exports` map or a subpath that ships no JS. | @@ -114,9 +115,10 @@ Both of these will make you report a passing gate that CI rejects. | `verify:seam-1` | A package gained a runtime dependency outside the allow-list, or dropped its committed empty `dependencies` object (an omitted field is a violation too). | Remove the dependency — SEAM-1 is the constraint, not the gate. `@dexpace/core` is a **peer** of the satellites, never a dependency. | | `verify:sse-37` | Core's SSE code reached for serde or a codec package. | Remove the import; SSE-37/38 forbid the coupling. | | `verify:runtime-floor` | `engines.node` and the `target`/`lib` a package compiles to have drifted apart. | Move both together, deliberately — never raise one to silence this. | +| `verify:test-partition` | One of the five strings that keep `tests/conformance/` (Bun) and `tests/node-conformance/` (`node --test`) apart has drifted — see CLAUDE.md's hard rule. Every way this breaks is silent: Bun runs `node:test` files and reports them **passing**, Bun ignores an unrecognized `[test]` key with no warning, and `node --test` over a glob matching nothing exits 0. | The assertion names the file and the string. Fix all five together — `bunfig.toml`, `package.json`, `eslint.config.js`, `run-ci.mjs`, `tests/node-conformance/README.md` — never one alone. Unlike the gates around it this one reads files only, so it still reports through a red `build` rather than going `SKIP`. | | `verify:reproducible-build` | Two clean builds of an identical source tree disagreed (NFR-12) — either in an emitted `dist/` file or in an `npm pack` tarball. | The assertion names what differed. A wall-clock or random value reaching a build-time codegen step is the usual cause; `packages/core/scripts/gen-version.mjs` is the only such step today, and injecting a `Date.now()` there is this gate's own negative test (it fails naming `packages/core/dist/generated/version.js` and `npm-pack:dexpace-core-0.0.0.tgz`). If the log instead ends in `tsc` errors, the **build inside the gate** failed and there is no difference to read — fix that first. The gate sweeps every `dist/` and rebuilds twice itself, so it is last in the job and leaves the tree freshly built. | | `audit` | A high-severity advisory in production dependencies. | `bun audit --prod` for detail. Note the tree is tiny (zero runtime deps by design), so a hit here is usually a transitive dev-dep misclassification worth reading carefully. | -| `test:node` | Bun-vs-Node runtime divergence, almost always in `packages/core/src/io/` — Web Streams, `AbortSignal`, `Uint8Array` chunking. | Fix against Node's semantics. A phase touching a runtime-divergent surface should be *adding* cases here; see `test/node-conformance/README.md`. | +| `test:node` | Bun-vs-Node runtime divergence, almost always in `packages/core/src/io/` — Web Streams, `AbortSignal`, `Uint8Array` chunking. | Fix against Node's semantics. A phase touching a runtime-divergent surface should be *adding* cases here; see `tests/node-conformance/README.md`. | **A `timeout` verdict is not the same finding as a red one.** Every step is capped at `STEP_TIMEOUT_MS` (10 minutes) in `run-ci.mjs`, and a step that hits the cap is reported `timeout` @@ -136,7 +138,7 @@ your report when they matter: not prove the floor. `--node-floor` runs the floor leg via `mise`/`fnm`/`nvm` (downloading the toolchain once); the runner prints a note when the active major is not 20. - **Run it whenever the change adds or edits a file under `test/node-conformance/`**, touches + **Run it whenever the change adds or edits a file under `tests/node-conformance/`**, touches `io/`, reaches for a new built-in, or moves the floor. This gap is not theoretical: Phase 8a's `transport.test.mjs` passed on Node 26 and failed 20 of 22 cases on 20.3.0, because an async *root-level* `before` hook does not complete before subtests inside a `describe` when @@ -148,9 +150,9 @@ your report when they matter: CI also runs `node-conformance` only after the `ci` job succeeds — so locally, a `test:node` failure alongside other failures is the same signal, just surfaced earlier. -Not in CI at all, so the runner does not include them: `bun run test:scripts` (tests the -gates themselves — run it by hand after touching `scripts/`), and changesets (a -consumer-facing change still needs `bun run changeset`). +Not in CI at all, so the runner does not include it: changesets (a consumer-facing change +still needs `bun run changeset`). `test:scripts` used to be on this list; Phase 10 wired it +into the `ci` job (open-items H13), so the runner covers it now. ## Runner flags diff --git a/.claude/skills/ci-preflight/run-ci.mjs b/.claude/skills/ci-preflight/run-ci.mjs index 287f63c..d39be13 100644 --- a/.claude/skills/ci-preflight/run-ci.mjs +++ b/.claude/skills/ci-preflight/run-ci.mjs @@ -4,7 +4,7 @@ // Runs every blocking step of `.github/workflows/ci.yml` against the working tree, in CI's own // order, and reports all failures at once rather than stopping at the first. // -// Two things make this more than a shell alias for fourteen `bun run` calls: +// Two things make this more than a shell alias for sixteen `bun run` calls: // // * Ordering is load-bearing. `bun test`, `api`, `lint:publish` and every `verify:*` gate resolve // `@dexpace/core` by package name, which lands in `packages/core/dist/`. Run them before @@ -62,6 +62,17 @@ const STEPS = [ ' test. Find the file that dropped below it in the table above.' : null, }, + { + // Tier `static`, not `gate`. `tier` has exactly one consumer -- the build-failed SKIP rule at + // the bottom of this file, which tests for `gate` -- so `static` means "runs even when `build` + // is red". Correct here and for verify:test-partition below: both read files and resolve no + // workspace package by name, so a failed build does not make either meaningless the way it does + // the gates around them. + id: 'test:scripts', + ci: 'Gate self-tests (scripts/*.test.mjs)', + cmd: 'bun run test:scripts', + tier: 'static', + }, { id: 'api', ci: 'API surface check', @@ -105,6 +116,14 @@ const STEPS = [ cmd: 'bun run verify:runtime-floor', tier: 'gate', }, + { + // Tier `static` — see `test:scripts` above. + id: 'verify:test-partition', + ci: 'Test-partition check (tests/ vs tests/node-conformance/)', + cmd: 'bun run verify:test-partition', + tier: 'static', + fix: 'the assertion names the string that drifted — change all five together, never one alone', + }, { // Last among the gates, matching ci.yml: it sweeps every dist/ and rebuilds twice, so running it // earlier would pull the tree out from under any step that resolves a workspace package by name. @@ -320,19 +339,24 @@ function report(results, skipped, opts) { return 1; } +// The three globs below duplicate `package.json`'s `test:node`, deliberately: this leg runs the +// suite under a pinned Node, so it cannot go through `bun run`. They are three of the five strings +// that hold the `tests/` partition (CLAUDE.md's hard rule) and are checked by +// `scripts/verify-test-partition.mjs` -- a stale glob here makes `node --test` match nothing and +// exit 0, which reads as a clean floor run over zero cases. function runNodeFloor(opts, childEnv) { const managers = [ [ 'mise', - `mise x node@${NODE_FLOOR} -- node --test test/node-conformance/*.test.mjs`, + `mise x node@${NODE_FLOOR} -- node --test tests/node-conformance/*.test.mjs`, ], [ 'fnm', - `fnm exec --using=${NODE_FLOOR} node --test test/node-conformance/*.test.mjs`, + `fnm exec --using=${NODE_FLOOR} node --test tests/node-conformance/*.test.mjs`, ], [ 'nvm', - `bash -lc 'nvm exec ${NODE_FLOOR} node --test test/node-conformance/*.test.mjs'`, + `bash -lc 'nvm exec ${NODE_FLOOR} node --test tests/node-conformance/*.test.mjs'`, ], ]; const found = managers.find( diff --git a/.claude/skills/knowledge-lookup/SKILL.md b/.claude/skills/knowledge-lookup/SKILL.md index 642625e..3e4855b 100644 --- a/.claude/skills/knowledge-lookup/SKILL.md +++ b/.claude/skills/knowledge-lookup/SKILL.md @@ -7,7 +7,7 @@ description: Use when starting a numbered task from a docs/superpowers/plans/ fi ## Overview -`docs/knowledge/` is 39 topic files and ~1470 harvested entries — 512 KB, past what belongs +`docs/knowledge/` is 39 topic files and ~1470 harvested entries, past what belongs in context. `bun run knowledge` filters it. A requirement-ID query runs ~120–580 tokens (median ~230) against a topic file of ~1800–5200 (median ~2300): roughly 9× smaller, and much more than that when the ID you want lives in a file you'd never have guessed. @@ -39,7 +39,7 @@ Different filters AND together; multiple values inside one filter OR. So ## Check the result is real before trusting it -**A `--req` hit is not proof the corpus knows anything.** 256 of the 641 cited IDs resolve +**A `--req` hit is not proof the corpus knows anything.** 255 of the 645 canonical IDs resolve *only* to an appendix-B conformance roll-up — one sentence naming three to five IDs and stating none of them. It exits 0, so nothing else will warn you. @@ -70,7 +70,7 @@ bun run knowledge --list-topics # 39 topics, entry and ID cou bun run knowledge --topic pipeline --section rules --brief cursor fork ``` -**16 of the 39 topics carry no requirement ID at all** — every styleguide-derived one, +**15 of the 39 topics carry no requirement ID at all** — every styleguide-derived one, including `data-modeling`, `error-handling`, `assertions`, `testing`, `api-design`. ID-first cannot reach them. `--list-topics` shows which; don't work from a memorised list. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a3a4025..c4c1bc7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,6 +34,15 @@ jobs: - name: Test (with coverage) run: bun run test --coverage + # The gates' own tests (`scripts/*.test.mjs`), on `node --test`. Deliberately outside + # `bun run test`: bunfig scopes discovery and the 80% coverage floor to `packages`, and that + # floor is a statement about `packages/core`, not about repo tooling. What this protects is a + # gate's logic silently degrading — a bad glob, a swallowed assertion — which no other step + # would notice, since a degraded gate still exits 0. Closes open-items H13, whose trigger had + # already fired: `knowledge.test.mjs` was failing on `main` and nothing ran it. + - name: Gate self-tests (scripts/*.test.mjs) + run: bun run test:scripts + - name: API surface check run: bun run api @@ -62,6 +71,12 @@ jobs: - name: Runtime-floor consistency check run: bun run verify:runtime-floor + # Reads the five files that must agree on `tests/node-conformance/`. The rule and the reasons + # it exists live in CLAUDE.md, "HARD RULE — the `tests/` partition"; what matters here is only + # that every way it breaks is silent, so nothing else in this workflow would catch it. + - name: Test-partition check (tests/ vs tests/node-conformance/) + run: bun run verify:test-partition + # NFR-12. Deliberately last in this job: it sweeps every dist/ and rebuilds # the workspace twice, so it would otherwise pull the rug from under any # step above that resolves a workspace package through its dist/. diff --git a/CLAUDE.md b/CLAUDE.md index 90e78e6..bdf371c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -21,7 +21,7 @@ All run from the repo root unless noted. bun install --frozen-lockfile bun run build:core # tsc -b of core's declarations; incremental -bun run build:deps # build:core + transport-shared — every package another package's src +bun run build:deps # build:core + the other packages another package's src or tests/ # imports BY NAME; a prerequisite of the four below bun run typecheck # build:deps, then tsc --noEmit per package bun run lint # build:deps, then gts lint . — formatting AND type-aware rules; fatal @@ -61,10 +61,18 @@ transport rows passed on 1.4.0 and failed three ways on the pinned 1.3.14. `--cl difference between "the gates pass here" and "CI will be green". **There are two test trees, and `bun run test` is the only command that runs both.** Colocated unit tests -live under `packages/*/src/`; cross-package conformance suites that drive a composed pipeline over a real -socket live under `tests/` (styleguide 11-testing: integration tests crossing a process or network boundary -belong in a top-level `tests/`, not beside one module). The root script is `bun test ./packages ./tests` — -two trees, one process, one coverage report, one exit code. +live under `packages/*/src/`; everything that crosses a process, a network, or a *runtime* boundary lives +under `tests/`. Styleguide 11-testing scopes that rule to process and network boundaries; this repo reads a +**runtime** boundary the same way, and the Node suite is why — see the hard rule below. The root script is +`bun test ./packages ./tests` — two trees, one process, one coverage report, one exit code. + +`tests/` in turn holds one subdirectory per **runner**, and they are not interchangeable: + +``` +tests/ + conformance/xcut/ # Bun runner, part of `bun run test` + node-conformance/ # node --test, run by `bun run test:node`, against the built dist/ +``` **A bare `bun test` silently runs only the first tree.** `bunfig.toml`'s `[test] root = "packages"` governs discovery, so a bare invocation never visits `tests/` and reports green over a suite it never opened, with @@ -73,16 +81,20 @@ argument is treated as a name filter and matches nothing, which is its own quiet does still fire on the combined run (confirmed by raising `coverageThreshold` and watching it exit 1), so CI's Test step is `bun run test --coverage` rather than the bare form. +**Neither form reaches `tests/node-conformance/`**, and that is enforced by a config key rather than by the +file system — read the hard rule below before touching it. + **Either form needs `bun run build` to have run first**, from Phase 6a on: `@dexpace/codec-json`'s tests reach core through its published entry point, which Bun resolves to `packages/core/dist/`. On a fresh clone they cannot resolve core at all; against a stale `dist/` they report green over yesterday's core. CI is safe — its Build step precedes its Test step. The root `test` script deliberately does not build first, so the inner loop stays fast; rebuild when you have changed `packages/core/src/`. -`test:node` is a separate, thin layer under `test/node-conformance/` that runs the same built package under +`test:node` is a separate, thin layer under `tests/node-conformance/` that runs the same built package under `node --test`, because Bun's Web Streams / `AbortSignal` / `Uint8Array` behavior is an independent implementation of Node's and `src/io/` is where they diverge. **A phase that touches a runtime-divergent -surface adds a case there, not only to `bun run test`** — see `test/node-conformance/README.md`. +surface adds a case there, not only to `bun run test`** — see `tests/node-conformance/README.md`. Cases sit +flat in that directory and are named `*.test.mjs`; the runner glob does not descend. Single test file or single test: @@ -110,16 +122,60 @@ bun run verify:consumer-types # the built .d.ts compiles on the declared `li bun run test:node # CI runs this as a matrix over engines.node's floor and current LTS bun run verify:seam-1 # zero runtime dependencies in EVERY package, plus the @dexpace/core # peer-dependency rule that guards the dual-package hazard +bun run verify:sse-37 # no serde dependency and no reconnect path in core SSE bun run verify:runtime-floor # tsconfig target vs package engines.node consistency +bun run verify:test-partition # the five files that keep tests/ and tests/node-conformance/ apart +bun run verify:reproducible-build # two clean builds of one source tree agree, dist/ and tarball (NFR-12) +bun run test:scripts # the gates' OWN tests (node --test scripts/*.test.mjs) bun run audit # bun audit --audit-level=high --prod ``` **Every one of these is a blocking CI step** (`.github/workflows/ci.yml`). Run the full set before claiming work is done — `bun run test` passing is not sufficient evidence. -`bun run test:scripts` (`node --test scripts/*.test.mjs`) tests the *gates themselves* — the knowledge CLI and -`verify-seam-1.mjs`. It is **not** wired into CI yet (`docs/open-items.md` H13), so run it by hand after -touching anything in `scripts/`. +`test:scripts` tests the *gates themselves* — the knowledge CLI, `verify-seam-1.mjs`, `verify-sse-37.mjs`, +`verify-test-partition.mjs`. Phase 10 made it a blocking CI step, closing `docs/open-items.md` H13. It was +not one before, and the proof that it should have been is that `knowledge.test.mjs` had been failing on +`main` since `36c3f96` with nobody noticing. A gate whose own logic degrades still exits 0, so nothing else +in the run would. + +### HARD RULE — the `tests/` partition + +`tests/` holds two suites. They must never run together. `tests/conformance/` runs on Bun, as part of +`bun run test`. `tests/node-conformance/` runs on `node --test`, through `bun run test:node`, against the +built `dist/`. It **must not** run on Bun. That is the only reason the tree exists. + +Before Phase 10, the file system held this separation. The Node tree was at `test/`, and no Bun command +could reach it. One path — `tests/node-conformance/` — now holds it instead, written into five files that +must agree: + +| File | What it holds | +|---|---| +| `bunfig.toml` | `[test] pathIgnorePatterns` — keeps the Node tree out of `bun test` | +| `package.json` | the `test:node` glob — the only command that runs the Node tree | +| `eslint.config.js` | the `.mjs` override — without it, `console`, `URL`, and the Web Streams globals fail `no-undef` | +| `.claude/skills/ci-preflight/run-ci.mjs` | three globs in the `--node-floor` path | +| `tests/node-conformance/README.md` | the membership rule, and the paths that name the tree | + +**The key is `pathIgnorePatterns`. The key is not `testPathIgnorePatterns`.** Bun accepts an unknown +`[test]` key without complaint. A wrong key gives no warning, does not fail, and does not stop the run. Bun +then collects the Node suite. Bun runs `node:test` files without an error and reports them as passing. The +run reports success over a suite that proves nothing about Node. Measured on `bun run test`, pinned Bun +1.3.14: with the key, 164 files; without it, 178. Thirteen of the fourteen extra files pass silently; the +run goes red only because the fourteenth trips an unrelated timer assertion, which points nowhere near the +cause. Treat the exit code as an accident, not a control. + +Never change one of these five files alone. Change one, then change all of them. Then run +`node scripts/verify-test-partition.mjs`. That gate catches the wrong key name, and CI blocks on it. + +Do not remove the bunfig key and narrow the root script to `bun test ./packages ./tests/conformance` +instead. That protects the root script only. A command typed by hand, such as `bun test ./tests`, would +still collect the Node suite. The gate checks for this. + +Keep `[test] root = "packages"`. It controls discovery for a bare `bun test`, and it keeps +`scripts/*.test.mjs` out of *that* run's coverage floor. It is a second mechanism, and it is independent. +It does not replace the ignore glob, which is what governs the explicit `./tests` path the root script +passes. The gate checks this too. ## Documentation hierarchy @@ -137,7 +193,7 @@ requirement ID. ### Querying `docs/knowledge/` -`docs/knowledge/` is 518 KB across 39 topic files — never read a topic file whole when a filtered query +`docs/knowledge/` is 39 topic files — never read a topic file whole when a filtered query answers the question. `bun run knowledge` parses the corpus into entries and filters them; a requirement-ID query returns ~170 tokens against a ~5700-token file read. @@ -150,10 +206,13 @@ bun run knowledge --section conflicts --brief # open design-vs-styleguide c Different filters AND together, values within one filter OR; `--help` lists the rest. Each result carries its `` provenance line — the citation for test-file headers and deferral notes, though styleguide paths are absolute to a sibling repo and need their machine prefix stripped first. **A `--req` hit is not proof of -knowledge:** 256 of 645 IDs are named only by an appendix-B conformance roll-up, tagged `[appendix-B roll-up]` -in output; only 385 have a substantive entry (`--coverage` breaks this down). 16 of the 39 topics carry no -requirement ID at all and are reachable only via `--topic`/`--chapter` (`--list-topics`). Nothing in CI runs -this. The `.claude/skills/knowledge-lookup` skill carries the full workflow. +knowledge:** 255 of 645 IDs are named only by an appendix-B conformance roll-up, tagged `[appendix-B roll-up]` +in output; 386 have a substantive entry and 4 are cited nowhere at all (`--coverage` breaks this down). 15 of +the 39 topics carry no requirement ID at all and are reachable only via `--topic`/`--chapter` +(`--list-topics`). Every count in this paragraph moves when the corpus is edited, so +`scripts/knowledge.test.mjs` pins all four against the live corpus and its failure message names the two docs +to update alongside. No CI step gates corpus *content*; CI does run the CLI's own suite (`test:scripts`), +which parses the real corpus. The `.claude/skills/knowledge-lookup` skill carries the full workflow. ## Requirement-ID conventions (enforced by review, not tooling) diff --git a/bunfig.toml b/bunfig.toml index a2206bf..a7c5d31 100644 --- a/bunfig.toml +++ b/bunfig.toml @@ -1,11 +1,26 @@ [test] -# Scope discovery to the workspace packages. Without this, `bun test` also collects -# test/node-conformance/*.test.mjs -- the Node-only layer that exists precisely because it must NOT -# run on Bun (checkpoint 5.9). Running it under both runners would inflate the unit count and quietly -# erase the distinction the suite was added to draw. It likewise keeps `scripts/*.test.mjs` -- repo -# tooling, run via `bun run test:scripts` (`node --test`) -- out of both the run and the coverage -# floor, which is a statement about `packages/*/src`. +# The rule these two keys enforce, and why it exists, live in ONE place: CLAUDE.md, "HARD RULE -- +# the `tests/` partition". Read that before changing either. What belongs here is only what is local +# to this file. +# +# `root` scopes discovery for a BARE `bun test`. It does NOT govern an explicit path argument, and +# the root script passes `./packages ./tests`, so for the run CI actually performs its effect is to +# keep `scripts/*.test.mjs` -- repo tooling, run via `bun run test:scripts` -- out of neither: those +# are outside both paths already. It matters for a bare invocation, and for the coverage floor, +# which is a statement about `packages/*/src`. +# +# `pathIgnorePatterns` is what excludes the Node suite on that explicit `./tests` path, and its +# spelling is the whole game. Bun accepts an unrecognized `[test]` key in silence -- no warning, no +# error, no effect -- so `testPathIgnorePatterns` reads as configured and does nothing. (Naming the +# wrong key here is safe: `scripts/verify-test-partition.mjs` checks for a DECLARATION, not a +# mention, so this file is free to warn about the trap it sits next to.) +# +# Measured on `bun run test`, pinned Bun 1.3.14: with the key, 164 files and exit 0; without it, +# 178 files -- the 14 Node files collected by a runner that cannot prove anything about Node. That +# run exits 1 rather than 0, but only by accident: 13 of the 14 pass silently under Bun and the +# 14th fails on an unrelated timer assertion, pointing nowhere near the real cause. root = "packages" +pathIgnorePatterns = ["tests/node-conformance/**"] coverage = true coverageThreshold = 0.8 coverageSkipTestFiles = true diff --git a/docs/open-items.md b/docs/open-items.md index e188108..f60d4de 100644 --- a/docs/open-items.md +++ b/docs/open-items.md @@ -160,8 +160,9 @@ observable and owes a real test. ### B1 — NFR-10 / NFR-17: CI never runs on the declared minimum runtime — **RESOLVED** (2026-08-26) Closed by the `node-conformance` job in `.github/workflows/ci.yml`, which runs `test:node` against the built -artifact as a matrix over `['20.3.0', 'lts/*']` — the declared floor and current LTS. `test/node-conformance/` -holds 36 cases. Re-verified 2026-08-26. Original finding kept below for provenance. +artifact as a matrix over `['20.3.0', 'lts/*']` — the declared floor and current LTS. `tests/node-conformance/` +(at `test/node-conformance/` until Phase 10 moved it) holds 36 cases. Re-verified 2026-08-26. Original finding +kept below for provenance. #### Original finding — **ACT** (trigger has now fired) @@ -877,11 +878,30 @@ holds. **Trigger:** any phase touching `packages/core/src/seams/`. The fix is to delete the file outright, not to maintain it; out of scope for a review pass because it is Phase 2 surface. -### H13 — `test:scripts` runs in no CI job — **OPEN** +### H13 — `test:scripts` runs in no CI job — **RESOLVED** (2026-08-31) + +Closed by the `Gate self-tests (scripts/*.test.mjs)` step in the `ci` job, placed directly after the `Test` +step and mirrored in `.claude/skills/ci-preflight/run-ci.mjs` as step id `test:scripts`. The glob now covers +`knowledge.test.mjs`, `verify-seam-1.test.mjs`, `verify-sse-37.test.mjs` and `verify-test-partition.test.mjs` +— 64 cases. + +**The trigger had already fired when this was closed.** `knowledge.test.mjs` was failing on `main`: +`--list-topics` reports how many of the 39 topic files carry no requirement ID, the test pinned that at 16, +and the corpus said 15. Cause was `36c3f96` (PR #59), whose Phase 10 correction to +`docs/knowledge/deliberate-deviations.md` cites `CFG-1` and so gave a previously ID-less topic its first ID. +A legitimate corpus edit, not a regression in extraction — but it sat unnoticed exactly as long as nothing ran +the suite. That count is quoted to readers in `CLAUDE.md` and `.claude/skills/knowledge-lookup/SKILL.md`; both +were stale too, and all three now move together, with the assertion's message naming the other two. The test +also gained an assertion that the summary line agrees with the table it summarises, which holds whatever the +corpus says. + +Original finding kept below for provenance. + +#### Original finding — **ACT** `scripts/*.test.mjs` runs only under `bun run test:scripts`, and `.github/workflows/ci.yml` has no step that -invokes it. As of 6b that glob covers `scripts/knowledge.test.mjs`, `scripts/verify-seam-1.test.mjs`, and -`scripts/verify-sse-37.test.mjs`. +invokes it. As of 6b that glob covered `scripts/knowledge.test.mjs`, `scripts/verify-seam-1.test.mjs`, and +`scripts/verify-sse-37.test.mjs`; Phase 10's `tests/` merge added `scripts/verify-test-partition.test.mjs`. The script was named `test:knowledge` until the Phase 6a reader pass renamed it: the glob had outgrown the name the moment `verify-seam-1.test.mjs` landed, and both places that cite it had to explain the mismatch in @@ -896,7 +916,30 @@ assertions. Not fixed here: `.github/workflows/ci.yml` was out of scope for this pass. Raised in the Phase 6a shape review as F9. -**Trigger:** immediate — add a `bun run test:scripts` step to the `ci` job. One line. +**Trigger:** immediate — add a `bun run test:scripts` step to the `ci` job. One line. **Fired and closed +2026-08-31; see above.** + +### H20 — the coverage floor measures only the Bun run — **RECORDED, no gate** (2026-08-31) + +`bunfig.toml`'s `coverageThreshold = 0.8` is enforced by `bun test` alone. `bun run test:node` contributes +nothing to it: `node --test` collects no coverage here, and the two runs do not share a report. So a line +reached only by `tests/node-conformance/` counts as uncovered, and a line covered only there cannot lift the +number. + +Surfaced by the issue-55 audit, which named three gaps in the pre-Phase-10 arrangement. The naming gap was +closed by the tree move. The static-checks gap (`.mjs` gets the gts/format baseline only, and `tsc` never +opens the subtree) is recorded in `tests/tsconfig.json`'s own comment with its compensating control — CI runs +that suite on two Node versions. This is the third, and it was the one left unrecorded. + +Not obviously a defect. The floor is a statement about `packages/*/src`, and the Node suite is deliberately +thin and additive rather than a second unit suite (`tests/node-conformance/README.md`), so its lines are +mostly re-assertions of behaviour `bun test` already covers. Merging the two reports would also mean +producing coverage from `node --test` over the BUILT `dist/`, which maps back to `src/` only through source +maps. Recorded so that "the floor covers everything" is never assumed. + +**Trigger:** a requirement whose ONLY test is a `tests/node-conformance/` case — at that point the floor is +actively misreporting, and the case needs either a `bun test` counterpart or an explicit note in its phase +checklist. ### H14 — `decodeSuccessResponse`'s 4xx/5xx branch is unprotected against a teardown failure — **OPEN** @@ -982,7 +1025,7 @@ that SERDE-20 names two positions and only one has code. A ~20k-deep object encodes successfully under Bun (whose `JSON.stringify` is iterative) and raises a stack-overflow `RangeError` under Node, which `encodeToText` correctly wraps as `SerializationError`. **Both outcomes are correct** — one succeeds, the other reports an unencodable value through the stable serde type — -so no `test/node-conformance/` case was added: a test asserting "either encodes or throws `SerializationError`" +so no `tests/node-conformance/` case was added: a test asserting "either encodes or throws `SerializationError`" asserts nothing a reader can act on. Recorded only so a future reader who trips over the difference does not file it as a bug. diff --git a/eslint.config.js b/eslint.config.js index 7cca4d7..a7b4de7 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -25,17 +25,23 @@ export default tseslint.config( { // The root config, the `.mjs` verification scripts, the Node-runtime // conformance suite, and the `.claude/skills` runners belong to no - // TypeScript project; they get the - // gts/format baseline only, never the type-aware tiers below. gts scopes - // its own Node globals to a fixed list of filenames that includes none of - // these, so declare them here or `console`/`URL` trip `no-undef` — and, in - // the conformance suite, so do the Web Streams and `AbortSignal` globals - // that are the whole point of running it on Node. + // TypeScript project; they get the gts/format baseline only, never the + // type-aware tiers below. gts scopes its own Node globals to a fixed list + // of filenames that includes none of these, so declare them here or + // `console`/`URL` trip `no-undef` — and, in the conformance suite, so do + // the Web Streams and `AbortSignal` globals that are the whole point of + // running it on Node. + // + // The Node-conformance entry below is one of the five files holding the + // `tests/` partition (CLAUDE.md's hard rule), and + // `scripts/verify-test-partition.mjs` blocks CI if it stops matching a real + // file. A stale glob here does not error: it silently drops the Node + // globals and buries the suite in `no-undef`. files: [ 'eslint.config.js', 'scripts/*.mjs', 'packages/*/scripts/*.mjs', - 'test/node-conformance/*.mjs', + 'tests/node-conformance/*.mjs', '.claude/skills/*/*.mjs', ], languageOptions: {sourceType: 'module', globals: globals.node}, diff --git a/package.json b/package.json index 900b974..19aa31d 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,7 @@ "test": "bun test ./packages ./tests", "knowledge": "node scripts/knowledge.mjs", "test:scripts": "node --test 'scripts/*.test.mjs'", - "test:node": "node --test test/node-conformance/*.test.mjs", + "test:node": "node --test tests/node-conformance/*.test.mjs", "shrink-test": "bun test ./packages/shrink-test", "bench": "bun run packages/core/src/io/byte-queue.bench.ts", "api": "cd packages/core && bun run api:ci && cd ../codec-json && bun run api:ci && cd ../logging-pino && bun run api:ci && cd ../logging-debug && bun run api:ci && cd ../body-file && bun run api:ci && cd ../transport-shared && bun run api:ci && cd ../transport-fetch && bun run api:ci && cd ../transport-undici && bun run api:ci && cd ../rx && bun run api:ci", @@ -65,6 +65,7 @@ "verify:seam-1": "node scripts/verify-seam-1.mjs", "verify:sse-37": "node scripts/verify-sse-37.mjs", "verify:runtime-floor": "node scripts/verify-runtime-floor.mjs", + "verify:test-partition": "node scripts/verify-test-partition.mjs", "verify:reproducible-build": "node scripts/verify-reproducible-build.mjs" } } diff --git a/packages/core/src/auth/bearer-cache.test.ts b/packages/core/src/auth/bearer-cache.test.ts index 1b58bce..b4b6f3a 100644 --- a/packages/core/src/auth/bearer-cache.test.ts +++ b/packages/core/src/auth/bearer-cache.test.ts @@ -276,7 +276,7 @@ describe('BearerTokenCache: a signal outliving many fetches (AUTH-34)', () => { // removal a caller signal that outlives many token fetches -- one request driving a long // paginated sweep, say -- accumulates one dead listener per fetch until Node's // MaxListenersExceededWarning fires. The listener COUNT is asserted directly in - // `test/node-conformance/auth.test.mjs`, where `node:events`' `getEventListeners` is available; + // `tests/node-conformance/auth.test.mjs`, where `node:events`' `getEventListeners` is available; // this is the behavioural half, on Bun: after many settled fetches the signal must still drive // exactly the one waiter outstanding when it fires, not a backlog of stale ones. const cache = new BearerTokenCache(); diff --git a/scripts/knowledge.test.mjs b/scripts/knowledge.test.mjs index 720a681..f289c9b 100644 --- a/scripts/knowledge.test.mjs +++ b/scripts/knowledge.test.mjs @@ -372,5 +372,65 @@ test('--list-topics covers every topic file and counts the ID-less ones', () => `${name} missing from --list-topics`, ); } - assert.match(report, /39 topic files\. 16 carry no requirement ID at all/); + // The prose count must agree with the table it summarises. This half tests the renderer, and it + // holds whatever the corpus says. + const stated = Number( + /(\d+) carry no requirement ID at all/.exec(report)?.[1], + ); + const zeroIdRows = report + .split('\n') + .filter(line => /^\S+\t\d+\t0$/.test(line)).length; + assert.equal( + stated, + zeroIdRows, + "--list-topics' summary line disagrees with its own table", + ); + + // Corpus-shape canary, hardcoded on purpose. It fires when a topic that carried no requirement ID + // gains its first one, which is a real event rather than noise: ID-less topics are reachable only + // via `--topic`/`--chapter`, so the count is quoted to readers in two documents outside this file. + // When it fires, confirm the corpus edit was intended, then move this assertion together with + // CLAUDE.md's "Querying `docs/knowledge/`" section and `.claude/skills/knowledge-lookup/SKILL.md`. + // Last moved 16 -> 15 by 36c3f96 (PR #59), whose Phase 10 correction to + // `docs/knowledge/deliberate-deviations.md` cites CFG-1. + assert.equal( + stated, + 15, + 'ID-less topic count changed — update CLAUDE.md and knowledge-lookup/SKILL.md with it', + ); +}); + +test('--coverage pins the substantive / roll-up / uncited split the docs quote', () => { + // Same canary, for the other three numbers in CLAUDE.md's "Querying `docs/knowledge/`" paragraph. + // Those had drifted by one apiece and nothing noticed, because the only assertion in this file + // covering that sentence was the topic count above. A count quoted to a reader and re-verified by + // nothing is how the corpus and its documentation part company. + const report = renderCoverage( + canonicalIds, + citationIndex(loadCorpus(prefixes)), + ); + const numbers = + /(\d+)\/(\d+) canonical IDs have a substantive entry\. (\d+) more are named only by an appendix-B conformance roll-up[^.]*\. (\d+) are cited nowhere/.exec( + report, + ); + assert.ok( + numbers, + `--coverage summary line not found in:\n${report.slice(-400)}`, + ); + const [, substantive, total, rollup, uncited] = numbers.map(Number); + assert.equal( + total, + canonicalIds.size, + 'appendix C and --coverage disagree on the ID total', + ); + assert.equal( + substantive + rollup + uncited, + total, + 'the three buckets do not account for every canonical ID', + ); + assert.deepEqual( + {substantive, rollup, uncited}, + {substantive: 386, rollup: 255, uncited: 4}, + 'corpus coverage changed — update CLAUDE.md and knowledge-lookup/SKILL.md with the new numbers', + ); }); diff --git a/scripts/verify-test-partition.mjs b/scripts/verify-test-partition.mjs new file mode 100644 index 0000000..1219b10 --- /dev/null +++ b/scripts/verify-test-partition.mjs @@ -0,0 +1,547 @@ +// SPDX-License-Identifier: MIT +// scripts/verify-test-partition.mjs +// +// Guards the separation between the two suites under `tests/`. The rule itself, and the reasoning +// behind it, live in ONE place: CLAUDE.md, "HARD RULE -- the `tests/` partition". This file is the +// enforcement, not a second copy of the argument. +// +// In one sentence: `tests/conformance/` runs on Bun as part of `bun run test`, +// `tests/node-conformance/` runs on `node --test` against the built `dist/`, and nothing may make +// the second run on the first's runner. Until Phase 10 the file system held that -- the Node tree +// lived at `test/`, where no Bun command could reach it. It now lives inside `tests/`, so a path +// written into five files holds it instead, and those five must agree. +// +// Reads files only. Runs neither suite. + +import {readdirSync, readFileSync} from 'node:fs'; +import {join} from 'node:path'; +import {fileURLToPath} from 'node:url'; + +const REPO_ROOT = fileURLToPath(new URL('..', import.meta.url)); + +const BUNFIG = 'bunfig.toml'; +const PACKAGE_JSON = 'package.json'; +const ESLINT_CONFIG = 'eslint.config.js'; +const RUN_CI = '.claude/skills/ci-preflight/run-ci.mjs'; +const README = 'tests/node-conformance/README.md'; + +const TESTS_ROOT = 'tests'; +const NODE_TREE = 'tests/node-conformance'; +const PACKAGES_ROOT = 'packages'; + +// Bun ignores an unrecognized `[test]` key in silence -- no warning, no error, no effect. This is +// the key that works; the decoy is the near-miss that reads as configured and is not. +const IGNORE_KEY = 'pathIgnorePatterns'; +const DECOY_IGNORE_KEY = 'testPathIgnorePatterns'; +const EXPECTED_BUN_ROOT = 'packages'; + +// `run-ci.mjs`'s `--node-floor` leg repeats the runner glob once per version manager it supports +// (mise, fnm, nvm). A minimum rather than an exact count: adding a fourth manager is fine, losing +// one silently is not. +const RUN_CI_MIN_GLOBS = 3; + +// --- path and glob primitives ------------------------------------------------------------------- + +/** + * @param {string} segment one path segment, no separators + * @returns {string} regexp source + */ +function segmentToRegExp(segment) { + let out = ''; + for (const ch of segment) { + if (ch === '*') out += '[^/]*'; + else if (ch === '?') out += '[^/]'; + else out += ch.replace(/[.+^${}()|[\]\\]/g, '\\$&'); + } + return out; +} + +/** + * Translate a shell-style glob to an anchored RegExp over POSIX-separated, repo-relative paths. + * + * `**` spans whole path segments and only as a whole segment, which is what Bun does: Bun does not + * ignore `tests/wideZa.test.mjs` for a pattern whose last segment is `Za.test.mjs` behind a `**`. + * An earlier draft compiled `**` to a bare `.*` everywhere and matched it -- the dangerous + * direction for a gate, green-lighting a config Bun reads differently. Within a segment, `*` and + * `?` stop at the separator; everything else is a literal. + * + * Deliberately small. No brace expansion, no character classes, no extglob -- Bun expands those and + * this does not, so such a pattern fails the gate rather than passing it wrongly. If one is ever + * needed, that is the signal to reach for a real matcher, not to grow this. + * + * @param {string} glob + * @returns {RegExp} + */ +function globToRegExp(glob) { + const segments = glob.split('/'); + let out = '^'; + for (let i = 0; i < segments.length; i++) { + const isLast = i === segments.length - 1; + if (segments[i] === '**') { + // Zero or more whole segments. A trailing `**` also matches an empty remainder. + out += isLast ? '(?:[^/]+(?:/[^/]+)*)?' : '(?:[^/]+/)*'; + continue; + } + out += segmentToRegExp(segments[i]); + if (!isLast) out += '/'; + } + return new RegExp(`${out}$`); +} + +/** + * `a/b/c.mjs` -> `['a', 'a/b', 'a/b/c.mjs']`. + * + * @param {string} file + * @returns {string[]} + */ +function pathPrefixes(file) { + const parts = file.split('/'); + return parts.map((_, i) => parts.slice(0, i + 1).join('/')); +} + +/** + * Would Bun skip this file for these patterns? + * + * Every directory prefix counts, not just the full path. Bun applies `pathIgnorePatterns` while + * WALKING, so a pattern naming a directory prunes that whole subtree without matching any file path + * -- measured: adding `tests/conformance/fixtures` to the list dropped `fixtures/settle.test.mjs` + * from the run. Testing full paths alone left the widening check below blind to exactly the pattern + * shape a maintainer reaches for first. It is also what makes a bare `tests/node-conformance` (no + * `/**`) read as covering the tree, which is what Bun does with it. + * + * @param {string} file repo-relative POSIX path + * @param {RegExp[]} matchers + * @returns {boolean} + */ +function isIgnored(file, matchers) { + return pathPrefixes(file).some(prefix => + matchers.some(matcher => matcher.test(prefix)), + ); +} + +// --- source readers ----------------------------------------------------------------------------- + +/** + * Every file under `dir`, as repo-relative POSIX paths. + * + * `withFileTypes` rather than a `statSync` per entry: `statSync` throws on a broken symlink, and an + * uncaught `ENOENT` stack trace is the least useful thing a gate can emit. A directory that is + * genuinely absent returns `[]`, and the caller reports that as its own violation; anything else -- + * a permission error, a descriptor limit -- propagates, because "unreadable" recovered to "empty" + * is a wrong diagnosis rather than a known-good state. + * + * @param {string} root + * @param {string} dir repo-relative + * @returns {string[]} + */ +function listFiles(root, dir) { + let entries; + try { + entries = readdirSync(join(root, dir), {withFileTypes: true}); + } catch (error) { + if (error.code === 'ENOENT' || error.code === 'ENOTDIR') return []; + throw error; + } + const found = []; + for (const entry of entries) { + const child = `${dir}/${entry.name}`; + if (entry.isDirectory()) found.push(...listFiles(root, child)); + else found.push(child); + } + return found.sort(); +} + +/** + * Blank out a `#` comment, respecting quoted strings. + * + * A naive `/#.*$/` corrupts `pathIgnorePatterns = ["tests/#node/**"]` into an unterminated line, + * whose array then swallows the rest of the section. + * + * @param {string} line + * @returns {string} + */ +function stripTomlComment(line) { + let out = ''; + let quote = null; + for (const ch of line) { + if (quote) { + out += ch; + if (ch === quote) quote = null; + } else if (ch === '"' || ch === "'") { + quote = ch; + out += ch; + } else if (ch === '#') { + break; + } else { + out += ch; + } + } + return out; +} + +/** + * Is the offset inside a JS comment? + * + * Line-scoped on purpose. The obvious implementation -- blank out `/*...*\/` and `//...` over the + * whole source, as `verify-sse-37.mjs` does -- is wrong for THESE files specifically, because the + * strings they hold are globs: `packages/*\/scripts/*.mjs` contains `/*` and then `*\/`, so a + * block-comment matcher treats the middle of the `files:` array as a comment and deletes the very + * entry this gate exists to find. Looking only at what precedes the match on its own line cannot + * make that mistake. + * + * @param {string} source + * @param {number} index + * @returns {boolean} + */ +function isCommented(source, index) { + const prefix = source.slice(source.lastIndexOf('\n', index) + 1, index); + return ( + prefix.includes('//') || prefix.includes('/*') || /^\s*\*/.test(prefix) + ); +} + +/** + * The array `key` holds inside TOML section `[section]`. + * + * Hand-rolled rather than a TOML dependency: the gate must run with none, and the question is + * narrow -- is this exact key declared in this exact section, and what does it hold. + * + * @param {string} source + * @param {string} section + * @param {string} key + * @returns {{declared: boolean, malformed: boolean, values: string[]}} + */ +function readTomlStringArray(source, section, key) { + const lines = source.split('\n').map(line => stripTomlComment(line).trim()); + let current = ''; + for (let i = 0; i < lines.length; i++) { + const header = /^\[([^\]]+)\]$/.exec(lines[i]); + if (header) { + current = header[1]; + continue; + } + if (current !== section) continue; + const assignment = new RegExp(`^${key}\\s*=\\s*(.*)$`).exec(lines[i]); + if (!assignment) continue; + let raw = assignment[1]; + // Tolerate an array spread over several lines, but stop at anything starting a new key or + // section -- otherwise an unterminated array silently absorbs the next key's value. + while (!raw.includes(']') && i + 1 < lines.length) { + const next = lines[i + 1]; + if (/^\[/.test(next) || /^[\w.-]+\s*=/.test(next)) break; + raw += lines[++i]; + } + if (!raw.includes(']')) + return {declared: true, malformed: true, values: []}; + return { + declared: true, + malformed: false, + values: [...raw.matchAll(/["']([^"']*)["']/g)].map(match => match[1]), + }; + } + return {declared: false, malformed: false, values: []}; +} + +/** + * The string `key` holds inside TOML section `[section]`, or null. + * + * @param {string} source + * @param {string} section + * @param {string} key + * @returns {string | null} + */ +function readTomlString(source, section, key) { + let current = ''; + for (const line of source.split('\n').map(l => stripTomlComment(l).trim())) { + const header = /^\[([^\]]+)\]$/.exec(line); + if (header) { + current = header[1]; + continue; + } + if (current !== section) continue; + const found = new RegExp(`^${key}\\s*=\\s*["']([^"']*)["']`).exec(line); + if (found) return found[1]; + } + return null; +} + +/** + * Path-shaped globs naming the Node tree, lifted out of an arbitrary source. Occurrences, not a + * set: the caller decides whether repetition is meaningful. + * + * Requires a `*`, which is what separates a glob from prose -- `run-ci.mjs` carries + * `node-conformance (matrix)` as a CI step label, and that is not a pattern anything matches + * against. The consequence is that a hard-coded, star-free path slips past, which is acceptable: a + * literal path either resolves or visibly does not, whereas a glob matching nothing fails open. + * + * `skipComments` is what stops a source file from satisfying the check with prose ABOUT its own + * glob: `eslint.config.js`'s comment quotes the `files:` entry beneath it, and without this the + * check stayed green after that entry was deleted -- the sentence explaining the guarantee was what + * voided it. Markdown passes `false`, since there the prose IS the artifact. + * + * @param {string} source + * @param {boolean} [skipComments] + * @returns {string[]} + */ +function extractNodeTreeGlobs(source, skipComments = false) { + const found = []; + for (const match of source.matchAll( + /[\w.*/-]*\/node-conformance\/[\w.*/-]+/g, + )) { + if (!match[0].includes('*')) continue; + if (skipComments && isCommented(source, match.index)) continue; + found.push(match[0]); + } + return found; +} + +/** + * @typedef {object} PartitionSources + * @property {string} bunfig + * @property {string} packageJson + * @property {string} eslintConfig + * @property {string} runCi + * @property {string} readme + * @property {string[]} nodeTreeFiles repo-relative POSIX paths under tests/node-conformance/ + * @property {string[]} bunTreeFiles everything else under tests/ that a runner would collect + * @property {string[]} packageFiles colocated unit tests under packages/ + */ + +/** + * Read every file and tree the checks operate on. + * + * Separated from the checks so `findPartitionViolations` is pure and TOTAL: a caller supplies the + * whole world or none of it, never a mixture that silently reads live repo state it never named. + * + * @param {string} [root] + * @returns {PartitionSources} + */ +export function readPartitionSources(root = REPO_ROOT) { + const read = name => readFileSync(join(root, name), 'utf8'); + const underTests = listFiles(root, TESTS_ROOT); + const inNodeTree = file => file.startsWith(`${NODE_TREE}/`); + return { + bunfig: read(BUNFIG), + packageJson: read(PACKAGE_JSON), + eslintConfig: read(ESLINT_CONFIG), + runCi: read(RUN_CI), + readme: read(README), + nodeTreeFiles: underTests.filter(inNodeTree), + // Everything else under `tests/`, rather than a hardcoded sibling: check 4 has to stay + // meaningful if `tests/conformance/` is renamed, and it covers any future `tests//` + // for free. + bunTreeFiles: underTests.filter( + file => !inNodeTree(file) && /\.(?:ts|tsx|mjs|cjs|js)$/.test(file), + ), + packageFiles: listFiles(root, PACKAGES_ROOT).filter(file => + file.endsWith('.test.ts'), + ), + }; +} + +// --- the checks --------------------------------------------------------------------------------- + +/** Check 1 — the bunfig key exists, under [test], and is not the silent near-miss. */ +function checkIgnoreKeyDeclared(sources, ignore, fail) { + // Declaration-only, so `bunfig.toml` stays free to NAME the near-miss in a comment. A raw + // substring scan made the one file where that warning belongs the one file forbidden to carry it. + if (readTomlStringArray(sources.bunfig, 'test', DECOY_IGNORE_KEY).declared) { + fail( + 1, + `${BUNFIG} declares \`${DECOY_IGNORE_KEY}\`. Bun does not read that key and does not warn` + + ` about it; the run then collects ${NODE_TREE}/ and reports it passing. The key is` + + ` \`${IGNORE_KEY}\`.`, + ); + } + if (ignore.malformed) { + fail(1, `${BUNFIG}'s \`[test] ${IGNORE_KEY}\` array is not terminated.`); + } else if (!ignore.declared) { + fail( + 1, + `${BUNFIG} has no \`${IGNORE_KEY}\` key under [test]. Without it, \`bun test ./tests\`` + + ` collects ${NODE_TREE}/ and reports node:test files as passing.`, + ); + } else if (ignore.values.length === 0) { + fail(1, `${BUNFIG}'s \`[test] ${IGNORE_KEY}\` is empty.`); + } +} + +/** Check 2 — every file in the Node tree is kept out of `bun test`. */ +function checkNodeTreeIgnored(sources, globs, matchers, fail) { + if (sources.nodeTreeFiles.length === 0) { + fail( + 2, + `${NODE_TREE}/ holds no files. The Node suite is the only thing that runs on Node.`, + ); + } + for (const file of sources.nodeTreeFiles) { + if (!isIgnored(file, matchers)) { + fail( + 2, + `${file} is not matched by \`[test] ${IGNORE_KEY}\` (${globs.join(', ')}), so` + + ' `bun run test` collects it.', + ); + } + } +} + +/** Check 4 — nothing Bun is supposed to run is caught by the ignore glob. */ +function checkOtherTreesNotIgnored(sources, globs, matchers, fail) { + if (sources.bunTreeFiles.length === 0) { + fail( + 4, + `${TESTS_ROOT}/ holds no Bun-runner files outside ${NODE_TREE}/. Either the Bun suite moved` + + ' or it is gone; either way this check has stopped meaning anything.', + ); + } + for (const file of [...sources.bunTreeFiles, ...sources.packageFiles]) { + if (isIgnored(file, matchers)) { + fail( + 4, + `${file} IS matched by \`[test] ${IGNORE_KEY}\` (${globs.join(', ')}), so \`bun run test\`` + + ' silently skips it.', + ); + } + } +} + +/** Check 3 — every file in the Node tree is either the README or reached by `test:node`. */ +function checkRunnerReachesEveryCase(sources, fail) { + const testNode = JSON.parse(sources.packageJson).scripts?.['test:node']; + if (typeof testNode !== 'string') { + fail(3, `${PACKAGE_JSON} has no \`test:node\` script.`); + return; + } + const globs = testNode.split(/\s+/).filter(token => token.endsWith('.mjs')); + if (globs.length === 0) { + fail(3, `\`test:node\` names no .mjs path: ${testNode}`); + return; + } + const matchers = globs.map(globToRegExp); + for (const file of sources.nodeTreeFiles) { + if (file === README || matchers.some(matcher => matcher.test(file))) { + continue; + } + fail( + 3, + `${file} is not matched by \`test:node\` (${globs.join(', ')}), so no command in the repo` + + ' runs it. `node --test` over a glob matching nothing exits 0, so this is silent.', + ); + } +} + +/** Check 5 — every Node-tree glob the other four files carry still reaches the whole suite. */ +function checkDocumentedGlobs(sources, fail) { + const cases = sources.nodeTreeFiles.filter(file => + file.endsWith('.test.mjs'), + ); + const sites = [ + [RUN_CI, sources.runCi, true, RUN_CI_MIN_GLOBS], + [ESLINT_CONFIG, sources.eslintConfig, true, 1], + // Prose, not code: the README names the tree to its reader, so its comments are not skipped. + [README, sources.readme, false, 1], + ]; + for (const [name, source, skipComments, minimum] of sites) { + const globs = extractNodeTreeGlobs(source, skipComments); + if (globs.length < minimum) { + fail( + 5, + `${name} carries ${globs.length} ${NODE_TREE}/ glob(s), expected at least ${minimum}.` + + ' Losing one fails open.', + ); + } + for (const glob of new Set(globs)) { + const matcher = globToRegExp(glob); + const missed = cases.filter(file => !matcher.test(file)); + if (missed.length > 0) { + fail( + 5, + `${name}'s glob \`${glob}\` misses ${missed.length} of ${cases.length} cases under` + + ` ${NODE_TREE}/, starting with ${missed[0]}.`, + ); + } + } + } +} + +/** Checks 6 and 7 — the two further things CLAUDE.md's hard rule tells the reader to keep. */ +function checkRootScriptAndBunRoot(sources, fail) { + const test = JSON.parse(sources.packageJson).scripts?.test; + // Whole arguments, never a substring: `bun test ./packages ./tests/conformance` CONTAINS + // `./tests` while being precisely the narrowing the hard rule forbids -- it protects the root + // script and leaves a hand-typed `bun test ./tests` collecting the Node suite. + const args = typeof test === 'string' ? test.split(/\s+/) : []; + if (typeof test !== 'string') { + fail(6, `${PACKAGE_JSON} has no \`test\` script.`); + } else if ( + !args.includes(`./${PACKAGES_ROOT}`) || + !args.includes(`./${TESTS_ROOT}`) + ) { + fail( + 6, + `\`test\` must pass both trees whole (\`./${PACKAGES_ROOT} ./${TESTS_ROOT}\`); it is` + + ` \`${test}\`. A bare \`bun test\` never visits ${TESTS_ROOT}/, and narrowing to a subtree` + + ` protects only this script -- a hand-typed \`bun test ./${TESTS_ROOT}\` still collects` + + ` ${NODE_TREE}/.`, + ); + } + const root = readTomlString(sources.bunfig, 'test', 'root'); + if (root !== EXPECTED_BUN_ROOT) { + fail( + 7, + `${BUNFIG}'s \`[test] root\` is ${root === null ? 'absent' : `"${root}"`}, expected` + + ` "${EXPECTED_BUN_ROOT}". It scopes a bare \`bun test\` and keeps scripts/*.test.mjs out of` + + " that run's coverage floor.", + ); + } +} + +/** + * Checks 1-5 are issue #55's. 6 and 7 guard two further rules CLAUDE.md's hard rule states and + * nothing enforced: the root script must name both trees, and `[test] root` must stay `"packages"`. + * + * @param {PartitionSources} sources + * @returns {{check: number, message: string}[]} empty when the partition holds + */ +export function findPartitionViolations(sources) { + const violations = []; + const fail = (check, message) => violations.push({check, message}); + + const ignore = readTomlStringArray(sources.bunfig, 'test', IGNORE_KEY); + checkIgnoreKeyDeclared(sources, ignore, fail); + if (ignore.declared && !ignore.malformed) { + const matchers = ignore.values.map(globToRegExp); + checkNodeTreeIgnored(sources, ignore.values, matchers, fail); + checkOtherTreesNotIgnored(sources, ignore.values, matchers, fail); + } + checkRunnerReachesEveryCase(sources, fail); + checkDocumentedGlobs(sources, fail); + checkRootScriptAndBunRoot(sources, fail); + + return violations; +} + +// --- CLI ----------------------------------------------------------------------------------------- + +const isDirect = + process.argv[1] && fileURLToPath(import.meta.url) === process.argv[1]; + +if (isDirect) { + const violations = findPartitionViolations(readPartitionSources()); + if (violations.length > 0) { + for (const {check, message} of violations) { + console.error(`test-partition violation (check ${check}): ${message}`); + } + console.error( + `\nThe ${TESTS_ROOT}/conformance/ and ${NODE_TREE}/ suites must never run together. Five` + + ` files hold that apart and they must agree: ${BUNFIG}, ${PACKAGE_JSON},` + + ` ${ESLINT_CONFIG}, ${RUN_CI}, and ${README}. Change one, then change all of them. See` + + ' CLAUDE.md, "HARD RULE -- the `tests/` partition".', + ); + process.exit(1); + } + console.log( + `test-partition OK: ${BUNFIG}, ${PACKAGE_JSON}, ${ESLINT_CONFIG}, ${RUN_CI} and ${README}` + + ` agree on ${NODE_TREE}/, and nothing else under ${TESTS_ROOT}/ is caught by the ignore glob.`, + ); +} diff --git a/scripts/verify-test-partition.test.mjs b/scripts/verify-test-partition.test.mjs new file mode 100644 index 0000000..89d82e0 --- /dev/null +++ b/scripts/verify-test-partition.test.mjs @@ -0,0 +1,534 @@ +// SPDX-License-Identifier: MIT +// scripts/verify-test-partition.test.mjs +// +// Tests the GATE, not a copy of its logic. The CLI half spawns the real script against throwaway +// fixture trees and reads its exit code and output, following `verify-seam-1.test.mjs`: a suite that +// only calls the detector passes just as happily when the CLI has stopped exiting non-zero, which is +// the one failure that would leave CI green over a dead gate. The detector half then drives the +// individual checks, which is where the interesting inputs are. +// +// Lives in `scripts/` and runs under `node --test` via `bun run test:scripts`, so `node:fs` and +// `node:child_process` are permitted here -- the zero-`node:` invariant governs `packages/*/src`, +// not build tooling. +import assert from 'node:assert/strict'; +import {spawnSync} from 'node:child_process'; +import { + copyFileSync, + mkdirSync, + mkdtempSync, + rmSync, + writeFileSync, +} from 'node:fs'; +import {tmpdir} from 'node:os'; +import {dirname, join} from 'node:path'; +import {test} from 'node:test'; +import {fileURLToPath} from 'node:url'; + +import {findPartitionViolations} from './verify-test-partition.mjs'; + +const repoRoot = fileURLToPath(new URL('..', import.meta.url)); +const script = join(repoRoot, 'scripts', 'verify-test-partition.mjs'); + +// --- the shape of a well-formed repo, as file contents and as a sources object -------------------- + +const BUNFIG = [ + '[test]', + 'root = "packages"', + 'pathIgnorePatterns = ["tests/node-conformance/**"]', + 'coverage = true', +].join('\n'); + +const PACKAGE_JSON = JSON.stringify({ + scripts: { + test: 'bun test ./packages ./tests', + 'test:node': 'node --test tests/node-conformance/*.test.mjs', + }, +}); + +const ESLINT_CONFIG = + "export default [{files: ['tests/node-conformance/*.mjs']}];"; + +// Three, because `run-ci.mjs`'s `--node-floor` leg carries one per version manager. +const RUN_CI = [ + '`mise x node@20.3.0 -- node --test tests/node-conformance/*.test.mjs`,', + '`fnm exec --using=20.3.0 node --test tests/node-conformance/*.test.mjs`,', + "`bash -lc 'nvm exec 20.3.0 node --test tests/node-conformance/*.test.mjs'`,", +].join('\n'); + +const README = + 'Run by `bun run test:node` (`node --test tests/node-conformance/*.test.mjs`).'; + +const NODE_CASES = [ + 'tests/node-conformance/retry.test.mjs', + 'tests/node-conformance/seams.test.mjs', +]; + +/** A complete, well-formed sources object; overrides replace individual fields. */ +function sources(overrides = {}) { + return { + bunfig: BUNFIG, + packageJson: PACKAGE_JSON, + eslintConfig: ESLINT_CONFIG, + runCi: RUN_CI, + readme: README, + nodeTreeFiles: [...NODE_CASES, 'tests/node-conformance/README.md'], + bunTreeFiles: ['tests/conformance/xcut/retry-safety.conformance.test.ts'], + packageFiles: ['packages/core/src/http/headers.test.ts'], + ...overrides, + }; +} + +/** Which checks fired, each once — one drifted string usually trips its check per file. */ +const checks = violations => [...new Set(violations.map(v => v.check))].sort(); + +/** The message a given check produced, so no assertion depends on array position. */ +const messageFor = (violations, check) => + violations.find(v => v.check === check)?.message ?? ''; + +// --- the CLI, driven end to end ------------------------------------------------------------------- + +// Copies the real script into a throwaway tree and runs it there. `REPO_ROOT` is resolved from the +// script's own location, so the copy reads the fixture's files rather than this repository's -- the +// same trick `verify-seam-1.test.mjs` uses, and what makes the failure path reachable through the +// ACTUAL CLI, exit code included. +function runAgainstFixture(files) { + const dir = mkdtempSync(join(tmpdir(), 'dexpace-partition-')); + mkdirSync(join(dir, 'scripts'), {recursive: true}); + copyFileSync(script, join(dir, 'scripts', 'verify-test-partition.mjs')); + for (const [name, contents] of Object.entries(files)) { + const target = join(dir, name); + mkdirSync(dirname(target), {recursive: true}); + writeFileSync(target, contents); + } + try { + const result = spawnSync( + process.execPath, + [join(dir, 'scripts', 'verify-test-partition.mjs')], + {encoding: 'utf8'}, + ); + return {status: result.status, output: `${result.stdout}${result.stderr}`}; + } finally { + rmSync(dir, {recursive: true, force: true}); + } +} + +/** A fixture tree the gate should accept. */ +function cleanFixture(overrides = {}) { + return { + 'bunfig.toml': BUNFIG, + 'package.json': PACKAGE_JSON, + 'eslint.config.js': ESLINT_CONFIG, + '.claude/skills/ci-preflight/run-ci.mjs': RUN_CI, + 'tests/node-conformance/README.md': README, + 'tests/node-conformance/retry.test.mjs': '// a case', + 'tests/node-conformance/seams.test.mjs': '// a case', + 'tests/conformance/xcut/a.conformance.test.ts': '// a case', + 'packages/core/src/headers.test.ts': '// a case', + ...overrides, + }; +} + +test('the CLI exits 0 and names all five files when the partition holds', () => { + const {status, output} = runAgainstFixture(cleanFixture()); + assert.equal(status, 0, output); + assert.match(output, /test-partition OK:/); + for (const named of [ + 'bunfig.toml', + 'package.json', + 'eslint.config.js', + 'run-ci.mjs', + 'README.md', + ]) { + assert.ok( + output.includes(named), + `${named} missing from the OK line:\n${output}`, + ); + } +}); + +test('the CLI exits 1 and names the failing check when a string has drifted', () => { + const {status, output} = runAgainstFixture( + cleanFixture({'bunfig.toml': '[test]\nroot = "packages"\n'}), + ); + assert.equal(status, 1, `expected a non-zero exit:\n${output}`); + assert.match(output, /test-partition violation \(check 1\)/); + assert.match(output, /Change one, then change all of them/); +}); + +test('the CLI exits 0 on this repository as committed', () => { + const result = spawnSync(process.execPath, [script], { + encoding: 'utf8', + cwd: repoRoot, + }); + assert.equal(result.status, 0, `${result.stdout}${result.stderr}`); +}); + +// --- check 1: the bunfig key ---------------------------------------------------------------------- + +test('the detector fails when bunfig declares testPathIgnorePatterns in place of pathIgnorePatterns', () => { + const found = findPartitionViolations( + sources({ + bunfig: + '[test]\nroot = "packages"\ntestPathIgnorePatterns = ["tests/node-conformance/**"]', + }), + ); + assert.match(messageFor(found, 1), /declares `testPathIgnorePatterns`/); +}); + +test('the detector fails when the decoy key is declared alongside the correct one', () => { + const found = findPartitionViolations( + sources({bunfig: `${BUNFIG}\ntestPathIgnorePatterns = ["x/**"]`}), + ); + assert.match(messageFor(found, 1), /declares `testPathIgnorePatterns`/); +}); + +test('the detector stays quiet when the decoy key is only NAMED in a comment', () => { + // bunfig.toml must be free to explain the hazard it is configured against. A gate that fails on + // its own requirement's explanation is a gate the next person deletes instead of the comment. + const found = findPartitionViolations( + sources({ + bunfig: `${BUNFIG}\n# The key is not testPathIgnorePatterns; Bun would ignore that in silence.`, + }), + ); + assert.deepEqual(found, []); +}); + +test('the detector fails when the ignore key is absent', () => { + const found = findPartitionViolations( + sources({bunfig: '[test]\nroot = "packages"\n'}), + ); + assert.ok(checks(found).includes(1)); +}); + +test('the detector fails when the ignore key holds an empty array', () => { + const found = findPartitionViolations( + sources({bunfig: '[test]\nroot = "packages"\npathIgnorePatterns = []'}), + ); + assert.ok(checks(found).includes(1)); +}); + +test('the detector fails when the ignore key sits under a section other than [test]', () => { + const found = findPartitionViolations( + sources({ + bunfig: + '[test]\nroot = "packages"\n[install]\npathIgnorePatterns = ["tests/node-conformance/**"]', + }), + ); + assert.ok(checks(found).includes(1)); +}); + +test('the detector fails when the ignore array is left unterminated', () => { + // The continuation reader must not swallow the next key's value and report a plausible array. + const found = findPartitionViolations( + sources({ + bunfig: + '[test]\nroot = "packages"\npathIgnorePatterns = [\n"tests/node-conformance/**"\ncoverage = true', + }), + ); + assert.match(messageFor(found, 1), /not terminated/); +}); + +test('the detector accepts a # inside a quoted pattern rather than truncating the line', () => { + const found = findPartitionViolations( + sources({ + bunfig: + '[test]\nroot = "packages"\npathIgnorePatterns = ["tests/node-conformance/**", "a/#b/**"]', + }), + ); + assert.deepEqual(found, []); +}); + +// --- check 2: the Node tree stays out of `bun test` ------------------------------------------------ + +test('the detector fails when a Node file escapes the ignore glob', () => { + const found = findPartitionViolations( + sources({ + bunfig: + '[test]\nroot = "packages"\npathIgnorePatterns = ["tests/node-conformance/*.test.mjs"]', + }), + ); + assert.match(messageFor(found, 2), /README\.md is not matched/); +}); + +test('the detector accepts a bare directory pattern, which is what Bun prunes on', () => { + const found = findPartitionViolations( + sources({ + bunfig: + '[test]\nroot = "packages"\npathIgnorePatterns = ["tests/node-conformance"]', + }), + ); + assert.deepEqual(found, []); +}); + +test('the detector fails when the Node tree holds no files', () => { + const found = findPartitionViolations(sources({nodeTreeFiles: []})); + assert.ok(checks(found).includes(2)); +}); + +test('the detector treats ** as spanning whole segments, the way Bun does', () => { + // `tests/**/s.test.mjs` must not cover `tests/node-conformance/wideSs.test.mjs`. Compiling `**` + // to a bare `.*` matched it and Bun does not -- the direction that green-lights a config Bun + // reads differently. + const found = findPartitionViolations( + sources({ + bunfig: + '[test]\nroot = "packages"\npathIgnorePatterns = ["tests/**/s.test.mjs"]', + nodeTreeFiles: ['tests/node-conformance/wideSs.test.mjs'], + }), + ); + assert.ok(checks(found).includes(2)); +}); + +// --- check 3: every case is reachable by the runner ------------------------------------------------ + +test('the detector fails when a Node case sits in a subdirectory the runner glob cannot reach', () => { + const found = findPartitionViolations( + sources({ + nodeTreeFiles: [ + ...NODE_CASES, + 'tests/node-conformance/io/byte-stream.test.mjs', + ], + }), + ); + assert.match( + messageFor(found, 3), + /io\/byte-stream\.test\.mjs is not matched/, + ); +}); + +test('the detector fails when a case is misnamed so no runner glob reaches it', () => { + // Ignored by Bun, unmatched by `test:node`, run by nothing — and `node --test` over a glob that + // matches nothing exits 0, so without this the file is simply never mentioned again. + for (const orphan of [ + 'tests/node-conformance/retry.mjs', + 'tests/node-conformance/orphan.test.ts', + ]) { + const found = findPartitionViolations( + sources({nodeTreeFiles: [...NODE_CASES, orphan]}), + ); + assert.ok(checks(found).includes(3), `${orphan} slipped through`); + } +}); + +test('the detector exempts the README from the runner glob', () => { + assert.deepEqual(findPartitionViolations(sources()), []); +}); + +test('the detector fails when test:node points at the pre-Phase-10 tree', () => { + const found = findPartitionViolations( + sources({ + packageJson: JSON.stringify({ + scripts: { + test: 'bun test ./packages ./tests', + 'test:node': 'node --test test/node-conformance/*.test.mjs', + }, + }), + }), + ); + assert.ok(checks(found).includes(3)); +}); + +test('the detector fails when package.json carries no test:node script at all', () => { + const found = findPartitionViolations( + sources({ + packageJson: JSON.stringify({ + scripts: {test: 'bun test ./packages ./tests'}, + }), + }), + ); + assert.match(messageFor(found, 3), /no `test:node` script/); +}); + +test('the detector fails when test:node names no .mjs path', () => { + const found = findPartitionViolations( + sources({ + packageJson: JSON.stringify({ + scripts: { + test: 'bun test ./packages ./tests', + 'test:node': 'node --test', + }, + }), + }), + ); + assert.match(messageFor(found, 3), /names no \.mjs path/); +}); + +// --- check 4: nothing else is caught by the ignore glob -------------------------------------------- + +test('the detector fails when the ignore glob widens over the Bun tree', () => { + const found = findPartitionViolations( + sources({ + bunfig: '[test]\nroot = "packages"\npathIgnorePatterns = ["tests/**"]', + }), + ); + assert.ok(checks(found).includes(4)); +}); + +test('the detector fails when a bare directory prunes part of the Bun tree', () => { + // Bun applies these patterns while walking, so naming a directory drops everything beneath it + // without matching any file path. Measured: adding `tests/conformance/fixtures` silently removed + // `fixtures/settle.test.mjs` from the run while every file-path check stayed green. + const found = findPartitionViolations( + sources({ + bunfig: + '[test]\nroot = "packages"\npathIgnorePatterns = ["tests/node-conformance/**", "tests/conformance/xcut"]', + }), + ); + assert.match(messageFor(found, 4), /silently skips it/); +}); + +test('the detector fails when the ignore glob reaches the packages tree', () => { + const found = findPartitionViolations( + sources({ + bunfig: + '[test]\nroot = "packages"\npathIgnorePatterns = ["tests/node-conformance/**", "packages/**"]', + }), + ); + assert.ok(checks(found).includes(4)); +}); + +test('the detector fails when the Bun tree has vanished, rather than passing vacuously', () => { + const found = findPartitionViolations( + sources({ + bunfig: '[test]\nroot = "packages"\npathIgnorePatterns = ["tests/**"]', + bunTreeFiles: [], + }), + ); + assert.ok(checks(found).includes(4)); +}); + +// --- check 5: the globs the other files carry ------------------------------------------------------ + +test('the detector fails when run-ci.mjs still names the pre-Phase-10 tree', () => { + const found = findPartitionViolations( + sources({runCi: '`node --test test/node-conformance/*.test.mjs`'}), + ); + assert.ok(checks(found).includes(5)); +}); + +test('the detector fails when run-ci.mjs loses one of its three globs', () => { + const found = findPartitionViolations( + sources({runCi: RUN_CI.split('\n').slice(0, 2).join('\n')}), + ); + assert.match(messageFor(found, 5), /expected at least 3/); +}); + +test('the detector fails when a glob reaches only some of the cases', () => { + const narrowed = RUN_CI.replaceAll( + 'tests/node-conformance/*.test.mjs', + 'tests/node-conformance/seams*.test.mjs', + ); + const found = findPartitionViolations(sources({runCi: narrowed})); + assert.match(messageFor(found, 5), /misses 1 of 2 cases/); +}); + +test('the detector fails when the eslint override glob goes stale', () => { + const found = findPartitionViolations( + sources({eslintConfig: "files: ['tests/node-conformance/*.cjs'],"}), + ); + assert.ok(checks(found).includes(5)); +}); + +test('the detector fails when the eslint override is deleted, leaving only prose about it', () => { + // The comment above that entry quotes the glob. Scanning source and comments alike kept this + // green after the real entry was gone: the sentence explaining the guarantee was what voided it. + const found = findPartitionViolations( + sources({ + eslintConfig: [ + '// The `tests/node-conformance/*.mjs` entry is one of the five strings.', + "export default [{files: ['scripts/*.mjs']}];", + ].join('\n'), + }), + ); + assert.match( + messageFor(found, 5), + /carries 0 tests\/node-conformance\/ glob/, + ); +}); + +test('the detector fails when the README stops naming the tree it documents', () => { + const found = findPartitionViolations( + sources({readme: 'This suite runs on Node. Nothing here names a path.'}), + ); + assert.ok(checks(found).includes(5)); +}); + +test('the detector does not mistake a run-ci step label for a glob', () => { + // `node-conformance (matrix)` carries no slashes, so it never reaches the star filter. + const found = findPartitionViolations( + sources({ + runCi: [ + " {ci: 'node-conformance (matrix)', cmd: 'bun run test:node'},", + RUN_CI, + ].join('\n'), + }), + ); + assert.deepEqual(found, []); +}); + +// --- checks 6 and 7: the two further rules the hard rule states ------------------------------------ + +test('the detector fails when the root test script stops naming both trees', () => { + const found = findPartitionViolations( + sources({ + packageJson: JSON.stringify({ + scripts: { + test: 'bun test', + 'test:node': 'node --test tests/node-conformance/*.test.mjs', + }, + }), + }), + ); + assert.match(messageFor(found, 6), /must pass both trees whole/); +}); + +test('the detector fails when the root test script is narrowed to one subtree', () => { + const found = findPartitionViolations( + sources({ + packageJson: JSON.stringify({ + scripts: { + test: 'bun test ./packages ./tests/conformance', + 'test:node': 'node --test tests/node-conformance/*.test.mjs', + }, + }), + }), + ); + assert.ok(checks(found).includes(6)); +}); + +test('the detector fails when bunfig loses its [test] root key', () => { + const found = findPartitionViolations( + sources({ + bunfig: '[test]\npathIgnorePatterns = ["tests/node-conformance/**"]', + }), + ); + assert.match(messageFor(found, 7), /`\[test\] root` is absent/); +}); + +test('the detector fails when [test] root is repointed away from packages', () => { + const found = findPartitionViolations( + sources({ + bunfig: + '[test]\nroot = "."\npathIgnorePatterns = ["tests/node-conformance/**"]', + }), + ); + assert.match(messageFor(found, 7), /expected "packages"/); +}); + +// --- several at once ------------------------------------------------------------------------------- + +test('the detector reports every drifted string, not only the first', () => { + const found = findPartitionViolations( + sources({ + bunfig: '[test]\npathIgnorePatterns = ["tests/node-conformance/**"]', + packageJson: JSON.stringify({ + scripts: { + test: 'bun test', + 'test:node': 'node --test test/node-conformance/*.test.mjs', + }, + }), + eslintConfig: "files: ['test/node-conformance/*.mjs'],", + }), + ); + assert.deepEqual(checks(found), [3, 5, 6, 7]); +}); diff --git a/test/node-conformance/README.md b/tests/node-conformance/README.md similarity index 54% rename from test/node-conformance/README.md rename to tests/node-conformance/README.md index 6f23c3f..68d3671 100644 --- a/test/node-conformance/README.md +++ b/tests/node-conformance/README.md @@ -1,7 +1,17 @@ # Node-runtime conformance suite +`tests/node-conformance/` — run by `bun run test:node` (`node --test tests/node-conformance/*.test.mjs`), +never by `bun test`. + Closes checkpoint §5.9 (`docs/superpowers/plans/2026-07-25-checkpoint-scaffold-through-phase3a.md:341`). +> **This tree must not run on Bun.** That is the only reason it exists. Until Phase 10 it lived at +> `test/node-conformance/`, outside anything `bun test` could reach; it now sits inside `tests/`, so +> `bunfig.toml`'s `[test] pathIgnorePatterns` holds the line instead — along with four other files that +> carry the same path, which `scripts/verify-test-partition.mjs` blocks CI on. **Read CLAUDE.md's "HARD RULE +> — the `tests/` partition" before moving, renaming, or nesting anything here.** It is the one place that +> rule and its reasoning are written down; this file only states what is local to the tree. + `bun test` runs the whole unit suite on **Bun's** runtime and proves nothing about the runtime this SDK actually ships to. Bun's Web Streams, `AbortSignal`, and `Uint8Array`/async-iteration behavior are independent implementations of Node's, and `packages/core/src/io/` — chunk boundaries, backpressure timing, reader-lock @@ -17,6 +27,9 @@ Node). ## Rules +- **Name every case `*.test.mjs`, flat in this directory.** `test:node`'s glob does not descend, and + `node --test` over a glob that matches nothing exits **0** — a case parked in a subdirectory is not a + failure, it is a silence. `verify:test-partition` turns that silence into a red CI step. - **Import the built artifact, never `src/`.** Public surface comes in through the `@dexpace/core` specifier; `io/` is `@internal` with no public subpath in `exports`, so it is reached by direct `dist/` file path. Run `bun run build` first — `test:node` does not build for you, because the CI job builds once and then runs the @@ -36,18 +49,20 @@ Node). ## Membership rule -**A phase that touches a runtime-divergent surface adds a case here, not only to `bun test`** (§5.9:378). That -means Phase 4 (pipelines, where `NFR-11`'s async-framework-leak check lands) and Phase 8 — both halves: 8a's -concrete `fetch`/`undici` transports, where this stops being precautionary and becomes the point, and 8b's -RxJS bridge, whose entire reason for being hand-written is a cancellation path the runtime decides. - -## Files - -| File | Surface | -|---|---| -| `seams.test.mjs` | `AbortSignal.any()` composition — folded in from the retired `scripts/verify-node-floor.mjs`, whose two assertions were the only Node coverage that existed before this suite — plus the `globalThis.crypto` floor assertion, made from ESM on purpose (Node 18 exposed `crypto` to CommonJS while leaving it undefined in ES modules) | -| `io-byte-stream.test.mjs` | Phase 3a's `ByteQueue`, `BufferedSource` + views, `BufferedSink`, `TeeSink`, `writeAll` | -| `body-lifecycle.test.mjs` | Phase 3b's public body surface over real Node Web Streams — reader-lock discipline, `pipeTo` ownership, multipart framing, error-body buffering | -| `transport.test.mjs` | Phase 8a's two concrete transports against a real `node:http` server on Node's own `fetch`/`undici`, `AbortSignal`, and Web Streams — redirect passthrough, timeout and no-response classification, a single-use streaming request body, lazy response bodies, `SEAM-16`'s abort-after-delivery rule, and concurrency | -| `redirect.test.mjs` | Phase 5b's Location resolution on Node's own WHATWG `URL` parser (relative resolution, percent-encoding preservation, userinfo clearing, bracketed IPv6, which malformed forms throw versus resolve as a relative reference) plus `PIPE-40`'s per-hop close discipline over real Node Web Streams | -| `rx-bridge.test.mjs` | Phase 8b's `@dexpace/rx` cancellation path — unsubscribing an idle `sseEvents$`/`typedSse$` must reach the source, which depends on Node's `ReadableStream.cancel()` settling a suspended read and on Node's async-generator `return()` queueing behind an in-flight `next()`; plus `pages$`'s mid-walk page release | +**A phase that touches a runtime-divergent surface adds a case here, not only to `bun test`** (§5.9:378). +Since Phase 4 that has meant most phases — pipelines, retry, redirect, auth, serde, SSE, pagination, +configuration, observability, the two concrete transports, and the RxJS bridge all have cases here. Two are +worth naming as the shape to aim for: 8a's `fetch`/`undici` transports, where this stops being precautionary +and becomes the point, and 8b's RxJS bridge, whose reason for being hand-written is a cancellation path the +runtime decides. + +## Which cases exist + +`ls` this directory. An earlier revision kept a table of file-to-surface descriptions here; it listed 6 of +14 by the time anyone checked, because nothing regenerated it. What each case covers, and which requirement +IDs it discharges, is recorded once — in that phase's checklist under `docs/superpowers/plans/`. + +One piece of provenance the tree cannot show: `seams.test.mjs` absorbed the retired +`scripts/verify-node-floor.mjs`, whose two `AbortSignal.any()` assertions were the only Node coverage that +existed before this suite. Its `globalThis.crypto` assertion is made from ESM on purpose — Node 18 exposed +`crypto` to CommonJS while leaving it undefined in ES modules, which is what sets the 20.3 floor. diff --git a/test/node-conformance/auth.test.mjs b/tests/node-conformance/auth.test.mjs similarity index 99% rename from test/node-conformance/auth.test.mjs rename to tests/node-conformance/auth.test.mjs index 58dd39e..cdc72f7 100644 --- a/test/node-conformance/auth.test.mjs +++ b/tests/node-conformance/auth.test.mjs @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// test/node-conformance/auth.test.mjs +// tests/node-conformance/auth.test.mjs // // Phase 5c reaches three runtime-provided globals that Bun implements independently of Node, and every // one of them fails SILENTLY rather than loudly if the two disagree: diff --git a/test/node-conformance/body-lifecycle.test.mjs b/tests/node-conformance/body-lifecycle.test.mjs similarity index 99% rename from test/node-conformance/body-lifecycle.test.mjs rename to tests/node-conformance/body-lifecycle.test.mjs index 68614e5..ec37f82 100644 --- a/test/node-conformance/body-lifecycle.test.mjs +++ b/tests/node-conformance/body-lifecycle.test.mjs @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// test/node-conformance/body-lifecycle.test.mjs +// tests/node-conformance/body-lifecycle.test.mjs // // Phase 3b's public body surface, driven through the `@dexpace/core` specifier — the path a real consumer // takes — on Node's Web Streams rather than Bun's. diff --git a/test/node-conformance/config-primitives.test.mjs b/tests/node-conformance/config-primitives.test.mjs similarity index 99% rename from test/node-conformance/config-primitives.test.mjs rename to tests/node-conformance/config-primitives.test.mjs index cec8fa6..c3f808e 100644 --- a/test/node-conformance/config-primitives.test.mjs +++ b/tests/node-conformance/config-primitives.test.mjs @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// test/node-conformance/config-primitives.test.mjs +// tests/node-conformance/config-primitives.test.mjs // // Phase 7a's runtime-divergent surfaces, driven through the `@dexpace/core` specifier on Node rather than // Bun. Three things here are independent implementations, not shared code: diff --git a/test/node-conformance/io-byte-stream.test.mjs b/tests/node-conformance/io-byte-stream.test.mjs similarity index 99% rename from test/node-conformance/io-byte-stream.test.mjs rename to tests/node-conformance/io-byte-stream.test.mjs index 542e609..c9165ce 100644 --- a/test/node-conformance/io-byte-stream.test.mjs +++ b/tests/node-conformance/io-byte-stream.test.mjs @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// test/node-conformance/io-byte-stream.test.mjs +// tests/node-conformance/io-byte-stream.test.mjs // // Phase 3a's byte-stream surface, on Node. §5.9:358 names this layer specifically: "chunk boundaries, // backpressure timing, queueMicrotask ordering" are where Bun's and Node's independent Web Streams diff --git a/test/node-conformance/observability.test.mjs b/tests/node-conformance/observability.test.mjs similarity index 98% rename from test/node-conformance/observability.test.mjs rename to tests/node-conformance/observability.test.mjs index 76a5547..27300a1 100644 --- a/test/node-conformance/observability.test.mjs +++ b/tests/node-conformance/observability.test.mjs @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// test/node-conformance/observability.test.mjs +// tests/node-conformance/observability.test.mjs // // Phase 7b's runtime-divergent surfaces, driven through the `@dexpace/core` specifier on Node.js: // * AsyncLocalStorage store propagation across native Node promises, microtasks, and macrotask timers (OBS-10, OBS-24). diff --git a/test/node-conformance/pagination.test.mjs b/tests/node-conformance/pagination.test.mjs similarity index 99% rename from test/node-conformance/pagination.test.mjs rename to tests/node-conformance/pagination.test.mjs index 5bcc4bb..1bf5ca9 100644 --- a/test/node-conformance/pagination.test.mjs +++ b/tests/node-conformance/pagination.test.mjs @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// test/node-conformance/pagination.test.mjs +// tests/node-conformance/pagination.test.mjs // // Phase 6c's runtime-divergent surface, run against the BUILT artifact on real Node. // diff --git a/test/node-conformance/recovery-chain.test.mjs b/tests/node-conformance/recovery-chain.test.mjs similarity index 99% rename from test/node-conformance/recovery-chain.test.mjs rename to tests/node-conformance/recovery-chain.test.mjs index de0a215..849768c 100644 --- a/test/node-conformance/recovery-chain.test.mjs +++ b/tests/node-conformance/recovery-chain.test.mjs @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// test/node-conformance/recovery-chain.test.mjs +// tests/node-conformance/recovery-chain.test.mjs // // Phase 4b (`RECOV-12`) is a runtime-divergent surface for one specific reason: the `SuppressedError` // global. Bun ships it and so does current Node, but it is a V8 global from the full Explicit Resource diff --git a/test/node-conformance/redirect.test.mjs b/tests/node-conformance/redirect.test.mjs similarity index 99% rename from test/node-conformance/redirect.test.mjs rename to tests/node-conformance/redirect.test.mjs index f4058ec..9bb1006 100644 --- a/test/node-conformance/redirect.test.mjs +++ b/tests/node-conformance/redirect.test.mjs @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// test/node-conformance/redirect.test.mjs +// tests/node-conformance/redirect.test.mjs // // Phase 5b is a runtime-divergent surface at two specific points, and both fail silently rather than // loudly if the runtimes disagree: diff --git a/test/node-conformance/retry.test.mjs b/tests/node-conformance/retry.test.mjs similarity index 99% rename from test/node-conformance/retry.test.mjs rename to tests/node-conformance/retry.test.mjs index 14fdf74..8fb48d7 100644 --- a/test/node-conformance/retry.test.mjs +++ b/tests/node-conformance/retry.test.mjs @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// test/node-conformance/retry.test.mjs +// tests/node-conformance/retry.test.mjs // // Phase 5a is a runtime-divergent surface at three specific points, and each one fails silently rather // than loudly if the runtimes disagree: diff --git a/test/node-conformance/rx-bridge.test.mjs b/tests/node-conformance/rx-bridge.test.mjs similarity index 99% rename from test/node-conformance/rx-bridge.test.mjs rename to tests/node-conformance/rx-bridge.test.mjs index eb73364..2c1e90a 100644 --- a/test/node-conformance/rx-bridge.test.mjs +++ b/tests/node-conformance/rx-bridge.test.mjs @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// test/node-conformance/rx-bridge.test.mjs +// tests/node-conformance/rx-bridge.test.mjs // // Phase 8b's runtime-divergent surface, run against the BUILT artifact on real Node. // diff --git a/test/node-conformance/seams.test.mjs b/tests/node-conformance/seams.test.mjs similarity index 99% rename from test/node-conformance/seams.test.mjs rename to tests/node-conformance/seams.test.mjs index 37cc03c..ba8f420 100644 --- a/test/node-conformance/seams.test.mjs +++ b/tests/node-conformance/seams.test.mjs @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// test/node-conformance/seams.test.mjs +// tests/node-conformance/seams.test.mjs // // Folded in from the retired `scripts/verify-node-floor.mjs`, whose two assertions were the entirety of // this repo's Node coverage before this suite existed (checkpoint §5.9). Keeping a second parallel Node diff --git a/test/node-conformance/serde.test.mjs b/tests/node-conformance/serde.test.mjs similarity index 99% rename from test/node-conformance/serde.test.mjs rename to tests/node-conformance/serde.test.mjs index 6c334db..696aa69 100644 --- a/test/node-conformance/serde.test.mjs +++ b/tests/node-conformance/serde.test.mjs @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// test/node-conformance/serde.test.mjs +// tests/node-conformance/serde.test.mjs // // Phase 6a's runtime-divergent surface, run against the BUILT artifact on real Node. // diff --git a/test/node-conformance/sse.test.mjs b/tests/node-conformance/sse.test.mjs similarity index 99% rename from test/node-conformance/sse.test.mjs rename to tests/node-conformance/sse.test.mjs index 187f282..9dd056c 100644 --- a/test/node-conformance/sse.test.mjs +++ b/tests/node-conformance/sse.test.mjs @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// test/node-conformance/sse.test.mjs +// tests/node-conformance/sse.test.mjs // // Phase 6b's runtime-divergent SSE surface, run against the BUILT artifact on real Node. // diff --git a/test/node-conformance/transport.test.mjs b/tests/node-conformance/transport.test.mjs similarity index 99% rename from test/node-conformance/transport.test.mjs rename to tests/node-conformance/transport.test.mjs index fdd8bd9..c6e16f6 100644 --- a/test/node-conformance/transport.test.mjs +++ b/tests/node-conformance/transport.test.mjs @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// test/node-conformance/transport.test.mjs +// tests/node-conformance/transport.test.mjs // // Phase 8a's Node layer. This is the file the suite's membership rule was written for: `bun test` runs both // transports against *Bun's* `fetch`, `AbortSignal`, and Web Streams, and the shipping runtime is Node's — diff --git a/tests/tsconfig.json b/tests/tsconfig.json index 192445d..b51866d 100644 --- a/tests/tsconfig.json +++ b/tests/tsconfig.json @@ -1,4 +1,17 @@ { + // Covers `tests/conformance/` ONLY. `tests/node-conformance/` is `.mjs`, and this project sets + // neither `allowJs` nor `checkJs`, so `tsc` never opens that subtree — deliberately. Those files + // run on `node --test` against the built `dist/`, and typing them would mean either checking JS + // against declarations they reach by raw `dist/` file path, or converting a suite whose whole + // point is to run on Node with no build step in front of it. + // + // The consequence worth stating: `types: ["bun"]` below does not apply to `node-conformance/`, + // and neither does the `strictTypeChecked` tier in `eslint.config.js`. That subtree gets the + // gts/format baseline and `globals.node` (`eslint.config.js`'s `.mjs` override) and nothing more. + // It is the tree that tests the shipped artifact and the tree with the fewest static checks over + // it; the compensating control is that CI runs it on two Node versions. + // + // See CLAUDE.md, "HARD RULE — the `tests/` partition". "extends": "../tsconfig.base.json", "compilerOptions": { "rootDir": ".",