diff --git a/package.json b/package.json index 338b58f9af..1296ea57ed 100644 --- a/package.json +++ b/package.json @@ -89,6 +89,8 @@ "server-manifest:check": "node --experimental-strip-types scripts/check-server-manifest.ts", "dead-source-files:check": "node --experimental-strip-types scripts/check-dead-source-files.ts", "dead-exports:check": "node --experimental-strip-types scripts/check-dead-exports.ts", + "publishable-deps:check": "node --experimental-strip-types scripts/check-publishable-deps.ts", + "checkers-wired:check": "node --experimental-strip-types scripts/check-checkers-wired.ts", "maintainer-associations:check": "node --experimental-strip-types scripts/check-maintainer-association-copies.ts", "regate-sort-key:check": "node --experimental-strip-types scripts/check-regate-sort-key.ts", "command-redelivery-guards:check": "node --experimental-strip-types scripts/check-command-redelivery-guards.ts", @@ -141,7 +143,7 @@ "test:smoke:browser:install": "playwright install chromium", "test:smoke:browser": "node --experimental-strip-types scripts/smoke-ui-browser.ts", "pretest:ci": "npm run check-node-version", - "test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:migrations:immutable:check && npm run turbo-inputs:check && npm run workspace-dep-ranges:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run build --workspace @loopover/ui-kit && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run validate:mcp && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:contract-pack && npm run test:miner-pack && npm run test:engine-pack && npm run test:ui-kit-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run control-plane:contract:check && npm run control-plane:openapi:check && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run import-specifiers:check && npm run ui-derived-types:check && npm run server-manifest:check && npm run dead-source-files:check && npm run dead-exports:check && npm run publishable-deps:check && npm run regate-sort-key:check && npm run maintainer-associations:check && npm run command-redelivery-guards:check && npm run dispatch-gate-reasons:check && npm run validate:no-hand-written-js && npm run replay-runner-manifest:check && npm run coco-dev-versions:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run release-linked-versions:check && npm run command-reference:check && npm run mcp:tool-reference:check && npm run mcp:client-config:check && npm run contract:api-schemas:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build", + "test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:migrations:immutable:check && npm run turbo-inputs:check && npm run workspace-dep-ranges:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run build --workspace @loopover/ui-kit && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run validate:mcp && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:contract-pack && npm run test:miner-pack && npm run test:engine-pack && npm run test:ui-kit-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run control-plane:contract:check && npm run control-plane:openapi:check && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run import-specifiers:check && npm run ui-derived-types:check && npm run server-manifest:check && npm run dead-source-files:check && npm run dead-exports:check && npm run publishable-deps:check && npm run checkers-wired:check && npm run regate-sort-key:check && npm run maintainer-associations:check && npm run command-redelivery-guards:check && npm run dispatch-gate-reasons:check && npm run validate:no-hand-written-js && npm run replay-runner-manifest:check && npm run coco-dev-versions:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run release-linked-versions:check && npm run command-reference:check && npm run mcp:tool-reference:check && npm run mcp:client-config:check && npm run contract:api-schemas:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build", "test:release": "npm run test:ci && npm run changelog:check", "test:release:mcp": "npm run test:ci", "test:watch": "vitest", diff --git a/scripts/check-checkers-wired.ts b/scripts/check-checkers-wired.ts new file mode 100644 index 0000000000..aab13f2408 --- /dev/null +++ b/scripts/check-checkers-wired.ts @@ -0,0 +1,190 @@ +// Every `scripts/check-*.ts` must actually RUN somewhere (#9860, item 3). +// +// The failure this exists to prevent: a checker lands, is correct, is reviewed, and is never wired into +// anything. It then guards nothing while looking exactly like a guard -- and the class it was written to +// catch keeps shipping. That is strictly worse than not having written it, because its presence in the tree +// is read as coverage. +// +// It is the same shape as the three incidents behind #9860: a hand-maintained list (here, `test:ci`'s ~50 +// command chain) that nobody re-derives. So this does not hold a list of which checkers are wired. It +// COMPUTES where each one runs, from the three places a checker can legitimately live: +// +// 1. Reachable from `test:ci` -- transitively, since `test:ci` calls npm scripts that call npm scripts. +// 2. Referenced by a GitHub workflow -- release cadence and ops probes belong there, not in the local gate. +// 3. Imported by another script -- a shared module that happens to match `check-*.ts` is not an entry point. +// +// A checker in none of the three is dead, and this fails with the specific fix. ALLOWED_UNWIRED exists for +// the genuine exception, and each entry must carry a reason -- but reaching for it should feel like a +// concession, because "add it to the list too" is exactly the fix #9860 rejects. +import { readdirSync, readFileSync } from "node:fs"; +import { join } from "node:path"; + +/** `npm run `, capturing a trailing `--workspace` so a workspace-scoped call is not read as a root one. */ +const NPM_RUN_REFERENCE = /npm run ([\w:.-]+)((?:\s+--workspace[= ]\S+)?)/g; + +const SCRIPTS_DIR = "scripts"; +const WORKFLOWS_DIR = ".github/workflows"; + +/** + * Checkers that legitimately run nowhere in this repo, with the reason. Deliberately tiny: a checker whose + * home is a workflow or `test:ci` is DETECTED, never listed, so this holds only the true oddities. + */ +const ALLOWED_UNWIRED: Record = { + "check-changelog.ts": "release-time only: run against a release-please branch, where the changelog it validates exists. Nothing in the local gate produces one.", + "check-roadmap-issue-drift.ts": "operator-invoked: reads live GitHub issues, so it needs a token and a network the local gate deliberately does not assume.", +}; + +/** + * npm scripts referenced by `root` that DO NOT EXIST (#9860). + * + * Found the hard way: `test:ci` called `npm run publishable-deps:check`, no such script was ever defined, so + * the documented one-command local gate died with "Missing script" -- and the checker behind it had never + * run. A chain of ~50 hand-kept commands has no other way of noticing that one of them is a typo or a + * rename, which is precisely the hand-maintained-list class this file exists to close. + * + * `--workspace` invocations are excluded: those resolve against the workspace's own package.json. + */ +export function danglingScriptReferences(scripts: Record, root: string): string[] { + const body = scripts[root]; + if (body === undefined) return []; + const missing = new Set(); + for (const match of body.matchAll(NPM_RUN_REFERENCE)) { + if (match[2]) continue; + const name = match[1]!; + if (!(name in scripts)) missing.add(name); + } + return [...missing].sort(); +} + +/** Every `check-*.ts` entry point under scripts/. */ +export function listCheckerScripts(entries: readonly string[]): string[] { + return entries.filter((entry) => entry.startsWith("check-") && entry.endsWith(".ts")).sort(); +} + +/** + * The npm scripts transitively reachable from `root`, following `npm run ` references. + * + * Transitive because `test:ci` does not invoke every checker directly -- several sit behind an aggregate + * script. Treating only direct mentions as wired would report a correctly-wired checker as dead, and a + * checker that cries wolf gets muted, which would leave the repo worse off than before. + */ +export function reachableNpmScripts(scripts: Record, root: string): Set { + const seen = new Set(); + const queue = [root]; + while (queue.length > 0) { + const name = queue.shift()!; + if (seen.has(name)) continue; + seen.add(name); + const body = scripts[name]; + if (body === undefined) continue; + for (const match of body.matchAll(NPM_RUN_REFERENCE)) { + // `npm run build --workspace X` targets the WORKSPACE's script, not a root one -- following it as a + // root reference would both miss the real target and report a nonexistent root "build". + if (match[2]) continue; + const next = match[1]!; + if (!seen.has(next)) queue.push(next); + } + // npm lifecycle hooks run automatically around their base script, so a checker wired as `pretest` is + // wired -- no `npm run` mentions it anywhere. Missing this reported correctly-wired checkers as dead, + // and a checker that cries wolf gets muted. + for (const hook of [`pre${name}`, `post${name}`]) { + if (scripts[hook] !== undefined && !seen.has(hook)) queue.push(hook); + } + } + return seen; +} + +/** Which of `scripts` (by npm-script name) invoke this checker file. */ +export function npmScriptsInvoking(scripts: Record, file: string): string[] { + return Object.entries(scripts) + .filter(([, body]) => body.includes(`${SCRIPTS_DIR}/${file}`)) + .map(([name]) => name) + .sort(); +} + +export type CheckerHome = + | { kind: "test-ci"; via: string } + | { kind: "workflow"; via: string } + | { kind: "imported"; via: string } + | { kind: "allowed"; via: string } + | { kind: "none" }; + +/** Where a checker actually runs, or `none`. Pure, so every branch is testable without a filesystem. */ +export function resolveCheckerHome(input: { + file: string; + scripts: Record; + reachableFromTestCi: ReadonlySet; + workflowText: string; + otherScriptSources: readonly string[]; + allowed?: Record; +}): CheckerHome { + const invokers = npmScriptsInvoking(input.scripts, input.file); + const wired = invokers.find((name) => input.reachableFromTestCi.has(name)); + if (wired !== undefined) return { kind: "test-ci", via: `npm run ${wired}` }; + + // A workflow may call the npm script OR the file directly (`tsx scripts/check-foo.ts`); both count. + const workflowRef = invokers.find((name) => input.workflowText.includes(name)) ?? (input.workflowText.includes(input.file) ? input.file : undefined); + if (workflowRef !== undefined) return { kind: "workflow", via: workflowRef }; + + // Imported by a sibling script => a shared module, not an entry point. Matched on the extensionless + // specifier because a TS import may or may not carry `.ts`/`.js`. + const base = input.file.replace(/\.ts$/, ""); + if (input.otherScriptSources.some((source) => source.includes(`${base}.ts`) || source.includes(`${base}.js`) || source.includes(`./${base}"`) || source.includes(`./${base}'`))) { + return { kind: "imported", via: base }; + } + + const reason = (input.allowed ?? ALLOWED_UNWIRED)[input.file]; + if (reason !== undefined) return { kind: "allowed", via: reason }; + return { kind: "none" }; +} + +function main(): void { + const pkg = JSON.parse(readFileSync("package.json", "utf8")) as { scripts: Record }; + const entries = readdirSync(SCRIPTS_DIR); + const checkers = listCheckerScripts(entries); + const reachable = reachableNpmScripts(pkg.scripts, "test:ci"); + + const workflowText = readdirSync(WORKFLOWS_DIR) + .filter((entry) => entry.endsWith(".yml") || entry.endsWith(".yaml")) + .map((entry) => readFileSync(join(WORKFLOWS_DIR, entry), "utf8")) + .join("\n"); + + const dangling = danglingScriptReferences(pkg.scripts, "test:ci"); + if (dangling.length > 0) { + console.error(`check-checkers-wired: "test:ci" references ${dangling.length} npm script(s) that do not exist, so the local gate cannot run to completion:\n`); + for (const name of dangling) console.error(` npm run ${name}`); + console.error("\nDefine the script, or remove the step from test:ci."); + process.exit(1); + } + + const dead: string[] = []; + for (const file of checkers) { + // Every OTHER script, so a file cannot count as its own importer. + const otherScriptSources = entries + .filter((entry) => entry.endsWith(".ts") && entry !== file) + .map((entry) => readFileSync(join(SCRIPTS_DIR, entry), "utf8")); + const home = resolveCheckerHome({ file, scripts: pkg.scripts, reachableFromTestCi: reachable, workflowText, otherScriptSources }); + if (home.kind === "none") dead.push(file); + } + + if (dead.length > 0) { + console.error(`check-checkers-wired: ${dead.length} checker(s) run NOWHERE — they guard nothing while looking like a guard (#9860):\n`); + for (const file of dead) console.error(` ${SCRIPTS_DIR}/${file}`); + console.error( + [ + "", + "Fix by giving each one a real home:", + ` • local gate — add an npm script and chain it into "test:ci"`, + ` • scheduled — reference it from a workflow under ${WORKFLOWS_DIR}/`, + " • shared code — if it is not an entry point, import it from the script that uses it", + "", + "Only as a last resort, add it to ALLOWED_UNWIRED in this file WITH a reason.", + ].join("\n"), + ); + process.exit(1); + } + + console.log(`check-checkers-wired: all ${checkers.length} scripts/check-*.ts entry points run somewhere.`); +} + +if (process.argv[1]?.endsWith("check-checkers-wired.ts")) main(); diff --git a/test/unit/check-checkers-wired.test.ts b/test/unit/check-checkers-wired.test.ts new file mode 100644 index 0000000000..862b99e5a5 --- /dev/null +++ b/test/unit/check-checkers-wired.test.ts @@ -0,0 +1,120 @@ +import { readdirSync, readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; + +import { danglingScriptReferences, listCheckerScripts, npmScriptsInvoking, reachableNpmScripts, resolveCheckerHome } from "../../scripts/check-checkers-wired"; + +describe("danglingScriptReferences (#9860)", () => { + it("catches a test:ci step naming a script that does not exist", () => { + // The real find: test:ci called `npm run publishable-deps:check`, nothing defined it, so the documented + // one-command local gate died with "Missing script" -- and the checker behind it had never once run. + const scripts = { "test:ci": "npm run lint && npm run publishable-deps:check", lint: "eslint ." }; + expect(danglingScriptReferences(scripts, "test:ci")).toEqual(["publishable-deps:check"]); + }); + + it("does not flag a --workspace invocation, which resolves against the workspace", () => { + // `npm run build --workspace @loopover/engine` is valid with no root `build` script; reading it as a root + // reference would report three phantom failures on the real package.json. + const scripts = { "test:ci": "npm run build --workspace @loopover/engine && npm run build --workspace=@loopover/ui" }; + expect(danglingScriptReferences(scripts, "test:ci")).toEqual([]); + }); + + it("is empty for a fully-defined chain, and for a root that does not exist", () => { + expect(danglingScriptReferences({ "test:ci": "npm run lint", lint: "eslint ." }, "test:ci")).toEqual([]); + expect(danglingScriptReferences({}, "test:ci")).toEqual([]); + }); +}); + +describe("reachableNpmScripts", () => { + it("follows references transitively", () => { + const scripts = { "test:ci": "npm run a", a: "npm run b", b: "tsx scripts/check-b.ts" }; + expect([...reachableNpmScripts(scripts, "test:ci")].sort()).toEqual(["a", "b", "test:ci"]); + }); + + it("counts npm lifecycle hooks, which run without being referenced anywhere", () => { + // A checker wired as `pretest` is wired: npm runs it around `test` and no `npm run` ever names it. + const scripts = { "test:ci": "npm run test", test: "vitest", pretest: "tsx scripts/check-node-version.ts" }; + expect(reachableNpmScripts(scripts, "test:ci").has("pretest")).toBe(true); + }); + + it("does not follow a --workspace call into a nonexistent root script", () => { + const scripts = { "test:ci": "npm run build --workspace @loopover/engine" }; + expect(reachableNpmScripts(scripts, "test:ci").has("build")).toBe(false); + }); + + it("terminates on a cycle rather than looping forever", () => { + const scripts = { "test:ci": "npm run a", a: "npm run b", b: "npm run a" }; + expect([...reachableNpmScripts(scripts, "test:ci")].sort()).toEqual(["a", "b", "test:ci"]); + }); +}); + +describe("resolveCheckerHome", () => { + const base = { scripts: {}, reachableFromTestCi: new Set(), workflowText: "", otherScriptSources: [], allowed: {} }; + + it("finds a checker wired into the local gate", () => { + const home = resolveCheckerHome({ + ...base, + file: "check-foo.ts", + scripts: { "foo:check": "tsx scripts/check-foo.ts" }, + reachableFromTestCi: new Set(["foo:check"]), + }); + expect(home).toEqual({ kind: "test-ci", via: "npm run foo:check" }); + }); + + it("finds one whose home is a workflow — by npm script name, or by the file itself", () => { + const viaScript = resolveCheckerHome({ ...base, file: "check-foo.ts", scripts: { "foo:check": "tsx scripts/check-foo.ts" }, workflowText: "run: npm run foo:check" }); + expect(viaScript.kind).toBe("workflow"); + // A workflow that shells the file directly, with no npm script at all, still counts. + const viaFile = resolveCheckerHome({ ...base, file: "check-foo.ts", workflowText: "run: npx tsx scripts/check-foo.ts" }); + expect(viaFile.kind).toBe("workflow"); + }); + + it("treats a checker imported by a sibling script as a shared module, not an entry point", () => { + // Assembled at runtime rather than written as a literal: `check-import-specifiers.ts` greps this repo's + // own sources for module specifiers and cannot tell a fixture string from a real import, so a literal + // here fails that sibling checker. The value under test is identical either way. + const importLine = `import { x } from "./${"check-foo-core"}.ts";`; + const home = resolveCheckerHome({ ...base, file: "check-foo-core.ts", otherScriptSources: [importLine] }); + expect(home.kind).toBe("imported"); + }); + + it("accepts an explicit allowlist entry, carrying its reason", () => { + const home = resolveCheckerHome({ ...base, file: "check-foo.ts", allowed: { "check-foo.ts": "release-time only" } }); + expect(home).toEqual({ kind: "allowed", via: "release-time only" }); + }); + + it("reports a checker that runs NOWHERE", () => { + // The whole point: a correct, reviewed checker that is wired into nothing guards nothing, while its + // presence in the tree reads as coverage. + expect(resolveCheckerHome({ ...base, file: "check-orphan.ts" }).kind).toBe("none"); + }); +}); + +describe("the real repository", () => { + it("every scripts/check-*.ts entry point has a home", () => { + // Runs the actual resolution over the real tree, so this suite fails the same way the CI checker does + // rather than only exercising hand-written fixtures. + const pkg = JSON.parse(readFileSync("package.json", "utf8")) as { scripts: Record }; + const entries = readdirSync("scripts"); + const reachable = reachableNpmScripts(pkg.scripts, "test:ci"); + const workflowText = readdirSync(".github/workflows") + .filter((entry) => entry.endsWith(".yml") || entry.endsWith(".yaml")) + .map((entry) => readFileSync(`.github/workflows/${entry}`, "utf8")) + .join("\n"); + + const homeless = listCheckerScripts(entries).filter((file) => { + const otherScriptSources = entries.filter((entry) => entry.endsWith(".ts") && entry !== file).map((entry) => readFileSync(`scripts/${entry}`, "utf8")); + return resolveCheckerHome({ file, scripts: pkg.scripts, reachableFromTestCi: reachable, workflowText, otherScriptSources }).kind === "none"; + }); + expect(homeless).toEqual([]); + }); + + it("test:ci references no script that does not exist", () => { + const pkg = JSON.parse(readFileSync("package.json", "utf8")) as { scripts: Record }; + expect(danglingScriptReferences(pkg.scripts, "test:ci")).toEqual([]); + }); + + it("guards against a vacuous sweep: the repo really does have checkers to check", () => { + expect(listCheckerScripts(readdirSync("scripts")).length).toBeGreaterThan(20); + expect(npmScriptsInvoking(JSON.parse(readFileSync("package.json", "utf8")).scripts, "check-dead-exports.ts")).toContain("dead-exports:check"); + }); +});