diff --git a/package.json b/package.json index 551c09ae..4857abc3 100644 --- a/package.json +++ b/package.json @@ -305,7 +305,7 @@ ] }, "scripts": { - "compile": "tsc -p ./ && tsc -p ./test/extension/tsconfig.unit.json && tsc -p ./test/webview/tsconfig.json && tsc -p ./test/webview-browser/tsconfig.json", + "compile": "tsc -p ./ && tsc -p ./test/extension/tsconfig.unit.json && tsc -p ./test/webview/tsconfig.json && tsc -p ./test/webview-browser/tsconfig.json && tsc -p ./test/build/tsconfig.json", "compile:webview": "tsc -p ./src/webview", "build": "pnpm compile && pnpm compile:webview && node esbuild.config.mjs --production", "watch": "node esbuild.config.mjs --watch", diff --git a/src/shared/quoll-perf-flag.d.ts b/src/shared/quoll-perf-flag.d.ts index 67dac213..87f503f9 100644 --- a/src/shared/quoll-perf-flag.d.ts +++ b/src/shared/quoll-perf-flag.d.ts @@ -13,6 +13,12 @@ // `const PERF_ENABLED = … ; if (PERF_ENABLED)` does NOT (esbuild keeps // `const o=!1; o&&fn()`). Do NOT reintroduce an intermediate const. // -// All four tsc programs include `src/shared/**`, so this one file is visible -// everywhere with no tsconfig `include` edits. +// Every tsc program that compiles a `QUOLL_PERF` reference includes +// `src/shared/**`, so this one file reaches all of them with no tsconfig +// `include` edits. Not every program in the repo does: the E2E emit config +// (`test/extension/tsconfig.json`, `rootDir: "."`) pulls in no `src/` file at +// all — it deliberately mirrors the wire constants rather than importing them, +// and it references no QUOLL_PERF. Before widening this claim, enumerate with +// `git ls-files '*tsconfig*.json'`; reading the `compile` script instead misses +// the configs no script chains. declare const QUOLL_PERF: boolean; diff --git a/test/build/compile-chain.test.ts b/test/build/compile-chain.test.ts new file mode 100644 index 00000000..e0ea3dda --- /dev/null +++ b/test/build/compile-chain.test.ts @@ -0,0 +1,140 @@ +// Pins the tsc project chain behind `pnpm compile`. +// +// Every type-level guarantee in this repo holds only because `compile` runs +// tsc over the project that owns it — the `@ts-expect-error` pins in +// test/build, the branded-offset assertions in test/webview, the protocol +// drift guard in test/extension. Neither vitest (esbuild transpile-only) nor +// `pnpm build`'s bundling step type-checks anything on its own, so a project +// dropped from this chain by a reformat, a bad merge or a "tidy the scripts" +// pass leaves `pnpm build`, `pnpm test` and CI ALL GREEN while every assertion +// that project gated goes permanently vacuous. That silent revert is the exact +// failure mode test/build/tsconfig.json was added to close, and this test is +// its tripwire. Pinning all five projects rather than just test/build's is +// deliberate: the accident is not specific to this directory. +// +// The roster is written down, not derived from disk, because membership is a +// judgement call: `git ls-files '*tsconfig*.json'` lists EIGHT configs and +// three of them belong outside this chain on purpose — tsconfig.base.json is +// extends-only, src/webview/tsconfig.json runs as the separate +// `compile:webview` script, and test/extension/tsconfig.json is the E2E emit +// program driven by `pnpm test:e2e:run`. A derived list would quietly adopt +// whatever config lands next instead of forcing that call to be reviewed. +// +// What this does NOT cover — written down so nobody assumes wider protection +// than exists. It pins the CHAIN, not the contents of each link: shrinking a +// project's `include`, setting `noCheck`, or flipping `strict` in +// tsconfig.base.json all leave these four assertions green — measured. The E2E +// `include` check below is not an exception to that list: it fires when that +// config GAINS a src path, not when any project's checking is weakened. Nor +// does this file pin its own execution path — the vitest `include` and the CI +// step that runs `pnpm test:unit` could stop invoking it. And dist/ can still +// be produced outside the gate by `pnpm watch` or a direct esbuild call, which +// is by design; the release path is safe only because ci.yml and publish.yml +// both go through `pnpm build`, and that wiring is outside this file's +// guarantee. +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; + +const pkg = JSON.parse( + readFileSync(fileURLToPath(new URL("../../package.json", import.meta.url)), "utf8") +); + +// Spelled exactly as the `-p` arguments appear in the script. Order matches the +// script for a simple exact compare; it is not load-bearing, so reorder both +// sides freely if the script's order ever changes for a real reason. +const CHAINED_PROJECTS = [ + "./", + "./test/extension/tsconfig.unit.json", + "./test/webview/tsconfig.json", + "./test/webview-browser/tsconfig.json", + "./test/build/tsconfig.json", +]; + +// tsconfig files in this repo are JSONC. Only lines that are ENTIRELY a `//` +// comment are dropped: a strip-to-end-of-line would corrupt a `//` sitting +// inside a string literal (a URL, a protocol-relative path). Block comments and +// trailing commas are still unsupported and would throw — no config here uses +// them. Without this, adding a doc comment to a config read below turns a clean +// assertion failure into a SyntaxError, i.e. a doc edit breaking `pnpm test`. +const parseJsonc = (source: string) => + JSON.parse( + source + .split("\n") + .filter((line) => !line.trimStart().startsWith("//")) + .join("\n") + ); + +describe("pnpm compile project chain", () => { + it("type-checks exactly the five reviewed projects, &&-chained with nothing else", () => { + // Pin the WHOLE script by exact equality rather than extracting its `-p` + // arguments. A partial extraction never sees what sits between and around + // the steps, so all of these stay green while the gate is dead: + // `tsc -p X ; tsc -p Y` (in sh the exit status is the LAST command's, which + // demotes the first four projects to non-gates), `tsc -p X || true`, and an + // `echo` in front of a step. Substring matching is weaker still — a bare + // `toContain("tsc -p ./")` is satisfied by any of the four nested projects, + // so the root pin would never go red. + expect(pkg.scripts.compile).toBe(CHAINED_PROJECTS.map((p) => `tsc -p ${p}`).join(" && ")); + }); + + // The scoped claim in src/shared/quoll-perf-flag.d.ts names this program as + // THE exception: the E2E config compiles no src/ file, which is why "every + // program includes src/shared" was false. Three separate passes over this PR + // asserted the universal anyway, so pin the exception rather than trusting + // the next reader to re-derive it. Widening this include is allowed — but it + // falsifies that comment, and this is what makes that visible. + it("keeps the E2E program free of src/, as the perf-flag comment states", () => { + const e2e = parseJsonc( + readFileSync(fileURLToPath(new URL("../extension/tsconfig.json", import.meta.url)), "utf8") + ); + // Match `src` as a whole path SEGMENT: a bare directory include spelled + // "../../src" (no trailing slash) is valid tsconfig and pulls in the same + // files, but `includes("src/")` does not see it. + expect(e2e.include.filter((p: string) => /(^|\/)src(\/|$)/.test(p))).toEqual([]); + // rootDir "." is what makes reaching into ../../src a TS6059 error rather + // than a silent widening, so it is part of the same guarantee. + expect(e2e.compilerOptions.rootDir).toBe("."); + }); + + it("reaches the bundle step only through both type-check gates", () => { + // `build` emitting dist/ without the gates would restore the same silent + // green: the bundle would ship from source no compiler ever looked at. + // BOTH gates are pinned, though only `compile` is load-bearing TODAY: + // test/webview-browser already compiles src/webview with a superset + // include and an emptied `types`, so it currently subsumes what + // `compile:webview` checks (measured — a Node-global probe in + // src/webview/shell.ts reddens both). `compile:webview` is pinned as + // defence in depth: that subsumption rests on the browser project's + // `include`, which nothing here pins (see the boundary note at the top). + // Do not "simplify" this gate away on the strength of today's overlap. + // + // Pinned by exact equality, like `compile`, because every weaker shape + // tried here turned out to be bypassable. Splitting on `&&` and comparing + // indices cannot see short-circuiting, so it accepted + // `… && false || node esbuild…`. Pinning only the leading prefix left the + // tail free, so it accepted `… --production || node esbuild…` (bundles + // ungated when a gate fails) and `… --production || true` (exits 0 on a + // failed type-check). The obvious next patch — reject `[&|;\n]` in the + // tail — is a denylist, and this file's whole history is denylists missing + // a case: it would still admit `>`, `$(…)`, backticks and `#`. Exact + // equality has no such gap. + // + // The cost is deliberate: changing `build` now requires updating this + // string. `compile` already carries that same contract, and the measured + // price is nil — `build` has not changed since the initial commit. + expect(pkg.scripts.build).toBe( + "pnpm compile && pnpm compile:webview && node esbuild.config.mjs --production" + ); + }); + + it("keeps compile:webview a real tsc pass", () => { + // `build` calling `compile:webview` means nothing if that script stops + // type-checking: `"echo skip"` or a trailing `|| true` leaves every other + // assertion here green while a tsc pass over src/webview disappears — the + // same accident this file exists to catch, one level down. (It is not the + // only such pass today; see the gate comment above for why it is pinned + // anyway.) The body is pinned exactly, as `compile` already is. + expect(pkg.scripts["compile:webview"]).toBe("tsc -p ./src/webview"); + }); +}); diff --git a/test/build/doc-sync.test.ts b/test/build/doc-sync.test.ts index 6652020f..d42b8844 100644 --- a/test/build/doc-sync.test.ts +++ b/test/build/doc-sync.test.ts @@ -13,7 +13,10 @@ // separately by the local `pnpm check:doc-sync` run, not by this suite. // // @ts-nocheck — importing a plain .mjs with no bundled types; vitest runs -// this transpile-only and tsc does not include test/build/ in `pnpm compile`. +// this transpile-only. `test/build/tsconfig.json` DOES type-check this +// directory under `pnpm compile`, but this file opts out wholesale via the +// file-level directive, so nothing here is checked; swapping it for a +// line-scoped `@ts-expect-error` on the import alone is a tracked follow-up. import { describe, expect, it } from "vitest"; import { diff --git a/test/build/notice-covers-bundled-deps.test.ts b/test/build/notice-covers-bundled-deps.test.ts index 7704e9ba..30f8b6ef 100644 --- a/test/build/notice-covers-bundled-deps.test.ts +++ b/test/build/notice-covers-bundled-deps.test.ts @@ -12,6 +12,8 @@ import { dirname, resolve } from "node:path"; import { fileURLToPath } from "node:url"; import esbuild from "esbuild"; import { beforeAll, describe, expect, it } from "vitest"; +// @ts-expect-error — esbuild.config.mjs is plain JS with no .d.ts; the runtime +// shape (a factory returning esbuild BuildOptions) is exercised below. import { createBuildConfigs } from "../../esbuild.config.mjs"; const root = resolve(dirname(fileURLToPath(import.meta.url)), "../.."); @@ -60,7 +62,20 @@ describe("NOTICE covers every bundled third-party package", () => { // Every value is a config emitting into the packaged dist/ (host, webview, // test-harness). Union them all so a dependency bundled into ANY shipped // output must be attributed — not just host + webview. - const configs = Object.values(createBuildConfigs({ production: true })); + // A CAST, deliberately — not a checked declaration. createBuildConfigs comes + // from the untyped .mjs above, so Object.values() over it widens each entry + // to `unknown`, which `.map(shippedPackages)` rejects; the cast is what + // narrows it to the shape esbuild.build actually requires. But it only + // ASSERTS that shape — nothing here verifies the factory really emits it. + // Written as an annotation it would READ like a compile-time pin while doing + // exactly this much: contextual typing feeds the annotation back into + // Object.values, and `any` satisfies whatever T it picks. Spelled as a + // cast so the file does not claim a check it is not performing. Giving + // esbuild.config.mjs a companion .d.mts (tracked in TODO.md) is what turns + // this back into a real pin. + const configs = Object.values( + createBuildConfigs({ production: true }) + ) as esbuild.BuildOptions[]; const sets = await Promise.all(configs.map(shippedPackages)); bundled = [...new Set(sets.flatMap((s) => [...s]))].sort(); notice = readFileSync(resolve(root, "NOTICE"), "utf8"); diff --git a/test/build/preview-server-theme.test.ts b/test/build/preview-server-theme.test.ts index 2d12b3ac..64563bfd 100644 --- a/test/build/preview-server-theme.test.ts +++ b/test/build/preview-server-theme.test.ts @@ -24,7 +24,10 @@ // /instance route only fills the template. No esbuild bundle is built on this path. // // @ts-nocheck — importing a plain .mjs with no bundled types; vitest runs this -// transpile-only and tsc does not include test/build/ in `pnpm compile`. +// transpile-only. `test/build/tsconfig.json` DOES type-check this directory +// under `pnpm compile`, but this file opts out wholesale via the file-level +// directive, so nothing here is checked; swapping it for a line-scoped +// `@ts-expect-error` on the import alone is a tracked follow-up. import type { Server } from "node:http"; import { afterEach, describe, expect, it } from "vitest"; diff --git a/test/build/publish-workflow-sbom-config.test.ts b/test/build/publish-workflow-sbom-config.test.ts index 26c12f27..0bcf2820 100644 --- a/test/build/publish-workflow-sbom-config.test.ts +++ b/test/build/publish-workflow-sbom-config.test.ts @@ -168,13 +168,14 @@ describe("CI rehearses the release SBOM sequence", () => { it.each(SHARED_STEPS)("keeps the load-bearing content of `%s`", (step) => { // Assert the needle EXISTS before using it. The `Record` type - // above looks like it already makes that impossible, but nothing enforces it - // here: `test/build/` is in none of the four tsconfigs `pnpm compile` runs - // (the trap CLAUDE.md documents for test/markdown and test/shared), so the - // exhaustiveness check never executes — and at runtime a missing entry makes - // this `expect(…).toMatch(undefined)`, which vitest PASSES silently. Without - // this line a forgotten entry ships with zero enforcement: verified by - // deleting one and watching the whole suite stay green. + // above IS enforced now — this file carries no `@ts-nocheck` and + // `test/build/tsconfig.json` type-checks the directory as `pnpm compile`'s + // 5th project, so a forgotten entry fails the build (measured: dropping the + // "Generate SBOM (SPDX)" entry yields TS2741 at the LOAD_BEARING literal). + // Kept as a runtime belt-and-braces check anyway, because the failure mode + // it covers is silent: if that type-level guarantee ever regresses, a + // missing entry degrades this to `expect(…).toMatch(undefined)`, which + // vitest PASSES. const needle = LOAD_BEARING[step]; expect(needle).toBeDefined(); expect(stepBlock(sbomJob, step, "ci.yml")).toMatch(needle); diff --git a/test/build/stale-todo-markers.test.ts b/test/build/stale-todo-markers.test.ts index 06e11457..f05f401e 100644 --- a/test/build/stale-todo-markers.test.ts +++ b/test/build/stale-todo-markers.test.ts @@ -17,7 +17,10 @@ // as test/build/todo-hygiene.test.ts. // // @ts-nocheck — importing a plain .mjs with no bundled types; vitest runs this -// transpile-only and tsc does not include test/build/ in `pnpm compile`. +// transpile-only. `test/build/tsconfig.json` DOES type-check this directory +// under `pnpm compile`, but this file opts out wholesale via the file-level +// directive, so nothing here is checked; swapping it for a line-scoped +// `@ts-expect-error` on the import alone is a tracked follow-up. import { describe, expect, it } from "vitest"; import { diff --git a/test/build/theme-palettes.test.ts b/test/build/theme-palettes.test.ts index dc8cd2cc..3c9d2e0b 100644 --- a/test/build/theme-palettes.test.ts +++ b/test/build/theme-palettes.test.ts @@ -22,15 +22,16 @@ import { readFileSync } from "node:fs"; import { describe, expect, it } from "vitest"; +// Namespace import so the module specifier — where TS7016 is reported — stays on +// the same line as the directive. A named import wide enough to hold all four +// bindings wraps past Biome's line width, which pushes the specifier out from +// under the directive and leaves the suppression unused (and the error live). // @ts-expect-error — plain .mjs with no bundled types; vitest transpiles it. -import { - bodyThemeAttrs, - PALETTES, - THEME_KINDS, - themeVarsCss, -} from "../../scripts/preview/vscode-theme-palettes.mjs"; +import * as themePalettes from "../../scripts/preview/vscode-theme-palettes.mjs"; import { THEME_KINDS as WIRE_THEME_KINDS } from "../../src/shared/protocol"; +const { bodyThemeAttrs, PALETTES, THEME_KINDS, themeVarsCss } = themePalettes; + const stylesCss = readFileSync(new URL("../../src/webview/styles.css", import.meta.url), "utf8"); // Live rules only — see the comment-strip rationale at the selector assertions below. const liveCss = stylesCss.replace(/\/\*[\s\S]*?\*\//g, ""); diff --git a/test/build/todo-hygiene.test.ts b/test/build/todo-hygiene.test.ts index 0f443ea6..18c12ed3 100644 --- a/test/build/todo-hygiene.test.ts +++ b/test/build/todo-hygiene.test.ts @@ -12,7 +12,10 @@ // `pnpm check:todo-hygiene` run, not by this suite. // // @ts-nocheck — importing a plain .mjs with no bundled types; vitest runs -// this transpile-only and tsc does not include test/build/ in `pnpm compile`. +// this transpile-only. `test/build/tsconfig.json` DOES type-check this +// directory under `pnpm compile`, but this file opts out wholesale via the +// file-level directive, so nothing here is checked; swapping it for a +// line-scoped `@ts-expect-error` on the import alone is a tracked follow-up. import { describe, expect, it } from "vitest"; import { diff --git a/test/build/tsconfig.json b/test/build/tsconfig.json new file mode 100644 index 00000000..940db9f9 --- /dev/null +++ b/test/build/tsconfig.json @@ -0,0 +1,48 @@ +{ + // Type-check program for the build/tooling suite. vitest runs these files + // through esbuild transpile-only, so without this gate every type-level + // contract in test/build (the `@ts-expect-error` pins on the untyped .mjs + // imports, the workflow/SBOM parsing helpers) is erased and never enforced. + // + // These tests exercise the BUILD side of the repo — esbuild configs, the + // scripts/ tooling, the packaged workflow YAML — so the compiler view is + // Node + ESM, with no DOM lib and no vscode-webview types. + // + // The scripts/ and esbuild.config.mjs modules they import are plain .mjs with + // no bundled types. The shape to reach for is a line-scoped `@ts-expect-error` + // on the import alone — it self-verifies (goes red the day the module gains + // types) and leaves everything the test itself authors checked. Five suites + // still use a file-level `@ts-nocheck` instead, which switches the WHOLE file + // off — a type-level assertion in one of those is still vacuous. + // + // To list them, anchor the grep to the directive itself: + // `git grep -l '^// @ts-nocheck' test/build` returns exactly those five. The + // unanchored `git grep -l '@ts-nocheck' test/build` returns EIGHT and is + // misleading — three of those hits are prose ABOUT the directive rather than + // the directive: this file, plus theme-palettes.test.ts and + // publish-workflow-sbom-config.test.ts, which each note that they carry none. + // The roster is deliberately not enumerated here: the tracked follow-up swaps + // the files over one at a time, so a hand-copied list goes stale on the first + // swap. Two places do name them — the TODO.md follow-up entry, where the + // roster IS the work item, and the 2026-08-22 LEARNING.md entry — and keeping + // both current is part of that follow-up. + // + // Do not read this config as proof that every file under it is checked. + // + // noEmit (inherited): this is a CI gate, not a build step — vitest still runs + // through its own transpiler. rootDir is pinned to the repo root explicitly + // (matching the sibling test tsconfigs) so the program's containment boundary + // is stated rather than inferred — the ../../src/shared files sit outside + // test/build/, and an inferred rootDir would silently widen with the include. + // It plays no part in resolving that cross-layer import: rootDir governs emit + // layout and the TS6059 containment check only, never module resolution. + "extends": "../../tsconfig.base.json", + "compilerOptions": { + "module": "esnext", + "moduleResolution": "bundler", + "lib": ["es2022"], + "types": ["node"], + "rootDir": "../.." + }, + "include": ["**/*.ts", "../../src/shared/**/*.ts"] +} diff --git a/test/webview/tsconfig.json b/test/webview/tsconfig.json index 788de19f..86c13675 100644 --- a/test/webview/tsconfig.json +++ b/test/webview/tsconfig.json @@ -14,7 +14,9 @@ // // noEmit (inherited): this is a CI gate, not a build step — vitest still // runs through its own transpiler. rootDir is the repo root so the - // cross-layer imports from ../../src/* resolve. + // cross-layer files under ../../src/* stay inside the program's containment + // boundary (TS6059) — it plays no part in resolving those imports, which is + // moduleResolution's job. "extends": "../../tsconfig.base.json", "compilerOptions": { "module": "esnext",