diff --git a/package.json b/package.json index 874759237d..4d99fd7493 100644 --- a/package.json +++ b/package.json @@ -59,6 +59,7 @@ "engine:coverage": "node --experimental-strip-types scripts/engine-coverage.ts", "db:migrations:check": "tsx scripts/check-migrations.ts", "db:migrations:immutable:check": "tsx scripts/check-released-migrations-immutable.ts", + "turbo-inputs:check": "tsx scripts/check-turbo-typecheck-inputs.ts", "workspace-dep-ranges:check": "tsx scripts/check-workspace-dep-ranges.ts", "db:schema-drift:check": "tsx scripts/check-schema-drift.ts", "actionlint": "node --experimental-strip-types scripts/actionlint.ts", @@ -139,7 +140,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 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 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 regate-sort-key: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-turbo-typecheck-inputs.ts b/scripts/check-turbo-typecheck-inputs.ts new file mode 100644 index 0000000000..0963c954fa --- /dev/null +++ b/scripts/check-turbo-typecheck-inputs.ts @@ -0,0 +1,157 @@ +#!/usr/bin/env node +// turbo's `//#typecheck` inputs must cover everything tsc actually reads (#9848). +// +// THE HAZARD, in turbo.json's own words: that inputs list is "a snapshot of test/'s real cross-package reach +// as of the audit that added it, not a structural guarantee -- a future test file importing from a NOT-yet- +// listed package/app would reopen the same silent-stale-cache gap." It then asks a human to "re-run the same +// grep ... before trusting this list again." +// +// Nobody re-runs a grep on request, and the list HAD already drifted when this check was written: `src/` and +// `test/` import from `packages/loopover-mcp/lib/**` and `packages/loopover-miner/scripts/**`, neither of +// which was hashed. Editing either could therefore leave a stale cache HIT on a typecheck that a real +// `tsc --noEmit` would fail -- the exact class of bug PR #5082 already burned this repo on once, and the +// reason the list exists at all. +// +// So: compute the reach instead of remembering it. This is the grep that comment asks for, run every CI. +// +// WHAT COUNTS AS COVERED. A path is fine if it is matched by an inputs glob, OR if it belongs to a workspace +// this task already `dependsOn` -- turbo hashes a dependency task's own inputs, so `@loopover/engine#build` +// covers the engine's sources without them being listed here. Anything else is unhashed and reported. +import { existsSync, readFileSync, readdirSync } from "node:fs"; +import { join } from "node:path"; +import { fileURLToPath, URL } from "node:url"; + +/** A cross-boundary import found in src/ or test/, as `//`. */ +export type CrossBoundaryReach = { path: string; importedBy: string }; + +/** Strip `//` line comments and trailing commas so turbo.json (JSONC) parses. Deliberately not a full JSONC + * parser: this file is ours, its comment style is known, and a dependency for one read would be worse. */ +export function parseJsonc(text: string): unknown { + return JSON.parse(text.replace(/^\s*\/\/.*$/gm, "").replace(/,(\s*[}\]])/g, "$1")); +} + +/** + * PURE core: every cross-workspace path `src/`+`test/` reach that no glob and no dependency covers. + * + * `globs` are turbo `inputs` entries; `coveredWorkspaces` are the workspace directory names whose builds this + * task depends on. Matching is prefix-based on the glob's literal head, which is all turbo's own globs use + * here (`packages/x/lib/**`) -- a stricter matcher would reject valid entries and a looser one would let a + * real gap through. + */ +export function findUnhashedReach( + reach: readonly CrossBoundaryReach[], + globs: readonly string[], + coveredWorkspaces: ReadonlySet, +): CrossBoundaryReach[] { + const prefixes = globs.map((glob) => glob.replace(/\*\*.*$/, "").replace(/\/$/, "")); + return reach.filter((entry) => { + const workspace = entry.path.split("/").slice(0, 2).join("/"); + if (coveredWorkspaces.has(workspace)) return false; + return !prefixes.some((prefix) => prefix.length > 0 && (entry.path === prefix || entry.path.startsWith(`${prefix}/`) || prefix.startsWith(entry.path))); + }); +} + +function walk(dir: string, out: string[]): void { + // Typed via the call's own return rather than `ReturnType`: that resolves to the + // Buffer-named overload under this tsconfig, which the string form is not assignable to. + let entries: ReadonlyArray<{ name: string; isDirectory(): boolean }>; + try { + entries = readdirSync(dir, { withFileTypes: true }); + } catch { + return; // a directory that does not exist here is not an error + } + for (const entry of entries) { + const path = join(dir, entry.name); + if (entry.isDirectory()) { + if (entry.name !== "node_modules") walk(path, out); + } else if (/\.(ts|tsx)$/.test(entry.name)) out.push(path); + } +} + +/** Every `packages//` or `apps//` a relative import from src/ or test/ reaches. */ +export function collectCrossBoundaryReach(root: string): CrossBoundaryReach[] { + const files: string[] = []; + walk(join(root, "src"), files); + walk(join(root, "test"), files); + + const reach = new Map(); + for (const file of files) { + const source = readFileSync(file, "utf8"); + // Both `from "..."` and bare `import "..."`, since a side-effect import is type-checked too. + for (const match of source.matchAll(/(?:from|import)\s+"((?:\.\.\/)+[^"]+)"/g)) { + const specifier = match[1]; + if (!specifier) continue; + const segments = /(?:^|\/)(packages|apps)\/([^/]+)\/([^/"]+)/.exec(specifier); + if (!segments) continue; + const path = `${segments[1]}/${segments[2]}/${segments[3]}`; + // Must exist on disk. The checker-testing files (check-import-specifiers-script.test.ts et al.) embed + // import statements INSIDE FIXTURE STRINGS -- `"src/foo.ts": 'import ... from "../packages/engine/..."'` + // -- and those name packages that were renamed away or never existed. A path tsc cannot resolve is not + // part of its real surface, so requiring the directory to exist filters exactly those without needing + // to parse TypeScript to tell code from a string literal. + if (!existsSync(join(root, path))) continue; + if (!reach.has(path)) reach.set(path, file); + } + } + return [...reach].map(([path, importedBy]) => ({ path, importedBy })).sort((a, b) => a.path.localeCompare(b.path)); +} + +/** Workspace dirs whose build this task depends on — turbo hashes their inputs transitively. */ +export function coveredWorkspacesFromDependsOn(dependsOn: readonly string[], root: string): Set { + const covered = new Set(); + for (const dependency of dependsOn) { + const name = dependency.split("#")[0]; + if (!name) continue; + for (const group of ["packages", "apps"]) { + let dirs: string[]; + try { + dirs = readdirSync(join(root, group), { withFileTypes: true }).filter((entry) => entry.isDirectory()).map((entry) => entry.name); + } catch { + continue; + } + for (const dir of dirs) { + try { + const manifest = JSON.parse(readFileSync(join(root, group, dir, "package.json"), "utf8")) as { name?: string }; + if (manifest.name === name) covered.add(`${group}/${dir}`); + } catch { + // not a workspace package + } + } + } + } + return covered; +} + +function main(): void { + const root = join(fileURLToPath(new URL(".", import.meta.url)), ".."); + const turbo = parseJsonc(readFileSync(join(root, "turbo.json"), "utf8")) as { + tasks?: Record; + }; + const task = turbo.tasks?.["//#typecheck"]; + if (!task) { + console.error('turbo-typecheck-inputs: turbo.json has no "//#typecheck" task — this check can no longer verify anything, so it fails rather than passing silently.'); + process.exit(1); + } + + const unhashed = findUnhashedReach( + collectCrossBoundaryReach(root), + task.inputs ?? [], + coveredWorkspacesFromDependsOn(task.dependsOn ?? [], root), + ); + + if (unhashed.length > 0) { + console.error("turbo //#typecheck does not hash everything tsc reads:\n"); + for (const entry of unhashed) console.error(` ${entry.path} (e.g. imported by ${entry.importedBy})`); + console.error( + "\n tsc's real surface is everything transitively imported from src/ + test/, wherever it lives. A path\n" + + " reached from there but absent from `inputs` (and not covered by a dependsOn build) is NOT hashed, so\n" + + " editing it can leave a stale cache HIT on a typecheck a real `tsc --noEmit` would fail — the #5082\n" + + " class of bug this inputs list exists to prevent.\n\n" + + ' Fix: add the path (e.g. "packages/x/lib/**") to //#typecheck\'s `inputs` in turbo.json.', + ); + process.exit(1); + } + console.log("turbo-typecheck-inputs: OK — every cross-workspace path src/+test/ reach is hashed."); +} + +if (process.argv[1]?.endsWith("check-turbo-typecheck-inputs.ts")) main(); diff --git a/test/unit/check-turbo-typecheck-inputs.test.ts b/test/unit/check-turbo-typecheck-inputs.test.ts new file mode 100644 index 0000000000..5624617f2c --- /dev/null +++ b/test/unit/check-turbo-typecheck-inputs.test.ts @@ -0,0 +1,74 @@ +import { describe, expect, it } from "vitest"; +import { collectCrossBoundaryReach, coveredWorkspacesFromDependsOn, findUnhashedReach, parseJsonc } from "../../scripts/check-turbo-typecheck-inputs"; + +// turbo.json's //#typecheck inputs are a hand-maintained approximation of tsc's real transitive surface. Its +// own comment admits it: "a snapshot ... not a structural guarantee", and asks a human to re-run a grep +// before trusting it. Nobody does, and it HAD drifted — four unhashed paths, one of which invalidated the +// comment's own stated reason for excluding it. This runs that grep every CI instead. + +const reach = (path: string) => ({ path, importedBy: "test/unit/x.test.ts" }); + +describe("findUnhashedReach", () => { + it("REGRESSION: reports a reached path no glob covers — the real drift this found", () => { + const unhashed = findUnhashedReach( + [reach("packages/loopover-mcp/lib"), reach("packages/loopover-miner/lib")], + ["src/**", "packages/loopover-miner/lib/**"], + new Set(), + ); + expect(unhashed.map((entry) => entry.path)).toEqual(["packages/loopover-mcp/lib"]); + }); + + it("a dependsOn build covers its whole workspace — turbo hashes the dependency's own inputs", () => { + // @loopover/engine#build means the engine's sources are hashed without appearing in this task's inputs. + // Reporting them would be a false positive that trains people to ignore the check. + expect(findUnhashedReach([reach("packages/loopover-engine/src")], [], new Set(["packages/loopover-engine"]))).toEqual([]); + }); + + it("a glob matches its own directory and anything under it", () => { + expect(findUnhashedReach([reach("packages/x/lib")], ["packages/x/lib/**"], new Set())).toEqual([]); + expect(findUnhashedReach([reach("packages/x/lib")], ["packages/x/**"], new Set())).toEqual([]); + }); + + it("INVARIANT: a NARROWER glob than the reached path still counts as covered", () => { + // "apps/loopover-ui/src/lib/**" covers a reach recorded as "apps/loopover-ui/src": the hashed set is a + // subset of the directory, which is what the existing entry means, and flagging it would be noise. + expect(findUnhashedReach([reach("apps/loopover-ui/src")], ["apps/loopover-ui/src/lib/**"], new Set())).toEqual([]); + }); + + it("no globs and no dependencies reports everything, rather than passing vacuously", () => { + expect(findUnhashedReach([reach("packages/a/lib"), reach("apps/b/src")], [], new Set())).toHaveLength(2); + }); +}); + +describe("collectCrossBoundaryReach", () => { + it("REGRESSION: ignores import statements inside FIXTURE STRINGS by requiring the path to exist", () => { + // check-import-specifiers-script.test.ts embeds `'import ... from "../packages/engine/lib/..."'` as test + // DATA. packages/engine was renamed away, so a naive scan reported it as unhashed reach — a false + // positive that would have been "fixed" by hashing a directory that does not exist. + const paths = collectCrossBoundaryReach(process.cwd()).map((entry) => entry.path); + expect(paths).not.toContain("packages/engine/lib"); + expect(paths).not.toContain("packages/engine/src"); + // And it still finds the real ones. + expect(paths).toContain("packages/loopover-mcp/lib"); + }); + + it("INVARIANT: this repo's own turbo.json covers its real reach", async () => { + // The check the CI script performs, asserted here too so a `turbo.json` edit that drops a path fails in + // the test suite as well as the standalone checker. + const { readFileSync } = await import("node:fs"); + const turbo = parseJsonc(readFileSync("turbo.json", "utf8")) as { tasks: Record }; + const task = turbo.tasks["//#typecheck"]!; + const unhashed = findUnhashedReach( + collectCrossBoundaryReach(process.cwd()), + task.inputs ?? [], + coveredWorkspacesFromDependsOn(task.dependsOn ?? [], process.cwd()), + ); + expect(unhashed.map((entry) => entry.path)).toEqual([]); + }); +}); + +describe("parseJsonc", () => { + it("strips line comments and trailing commas so turbo.json parses", () => { + expect(parseJsonc('{\n // a comment\n "a": 1,\n}')).toEqual({ a: 1 }); + }); +}); diff --git a/tsconfig.json b/tsconfig.json index 761bffe908..033ac9848f 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -10,6 +10,15 @@ "exactOptionalPropertyTypes": true, "noImplicitOverride": true, "noFallthroughCasesInSwitch": true, + // #9848: both cost ZERO errors today (measured) and lock in what this repo's RUNTIME already assumes. + // `isolatedModules` makes tsc enforce that every file is independently transpilable -- exactly the + // contract `node --experimental-strip-types` and esbuild's per-file transform already rely on, and which + // nothing was checking. `verbatimModuleSyntax` requires type-only imports to say `import type`, so a + // value import can never be silently elided out of the emitted bundle (or, worse, kept and pulled into + // the self-host bundle that must have zero `cloudflare:*` imports). Enabling them turns two runtime + // assumptions into compile-time guarantees rather than conventions. + "isolatedModules": true, + "verbatimModuleSyntax": true, // #9553. Dead code is the substrate every drift bug in the 2026-07-27 audit grew on: a stale import or an // orphaned constant reads exactly like a live wire, so the next person greps, finds it, and reasons about // a code path that no longer runs. Enabling these made the compiler enumerate all 515 instances, and diff --git a/turbo.json b/turbo.json index 16f11c81ed..0fabc4a174 100644 --- a/turbo.json +++ b/turbo.json @@ -23,7 +23,11 @@ // engine package" comment) already burned this repo on once. packages/loopover-mcp/bin/** is // deliberately NOT listed: every test reference to it is a runtime path string // (`join(process.cwd(), "packages/loopover-mcp/bin/loopover-mcp.js")`, spawned as a subprocess or - // read as raw text), never a static import, so its contents don't affect tsc's type-checked surface. + // read as raw text) -- that WAS true when written and is not any more: mcp-cli-completion-spec.test.ts + // statically imports CLI_COMMAND_SPEC/CLI_FLAG_SPEC from bin/loopover-mcp, so the .ts IS on tsc's + // surface and is now listed below. This list is no longer trusted to memory either: + // scripts/check-turbo-typecheck-inputs.ts recomputes the real reach every CI run (#9848), which is + // what caught this claim going stale, along with three other unhashed paths. // This list is a snapshot of test/'s real cross-package reach as of the audit that added it, not a // structural guarantee -- a future test file importing from a NOT-yet-listed package/app would // reopen the same silent-stale-cache gap. Re-run the same grep (every src/+test/ import crossing a @@ -47,6 +51,10 @@ "packages/loopover-miner/bin/**", "packages/loopover-mcp/package.json", "packages/discovery-index/src/**", + "packages/discovery-index/scripts/**", + "packages/loopover-mcp/bin/**", + "packages/loopover-mcp/lib/**", + "packages/loopover-miner/scripts/**", "apps/loopover-ui/src/lib/**" ], "outputs": []