Skip to content

Commit 39239df

Browse files
authored
chore(build): make turbo's typecheck inputs self-verifying, and lock two runtime assumptions into tsc (#9853)
turbo.json's //#typecheck `inputs` list is a hand-maintained approximation of tsc's real transitive surface. Its own comment says so -- "a snapshot ... not a structural guarantee" -- and asks a human to "re-run the same grep ... before trusting this list again". Nobody re-runs a grep on request, and it HAD drifted. Four paths that src/+test/ statically import were unhashed: packages/loopover-mcp/lib/** (mcp-cli-error, mcp-gateway tests) packages/loopover-mcp/bin/** (mcp-cli-completion-spec imports CLI_COMMAND_SPEC) packages/loopover-miner/scripts/** (miner-benchmark, miner-env-reference tests) packages/discovery-index/scripts/** (validate-posthog-release test) Editing any of them could leave a stale cache HIT on a typecheck a real `tsc --noEmit` would fail -- the #5082 class this list exists to prevent. The bin/** case also falsified the comment's own stated reason for excluding it ("never a static import"), which is now corrected rather than left misleading. Add scripts/check-turbo-typecheck-inputs.ts: compute the reach every CI run instead of remembering it. A path is covered by a glob OR by a dependsOn build (turbo hashes a dependency's own inputs), so engine/contract sources are not false-positived. Paths that do not exist on disk are ignored -- the checker-test fixtures embed import statements inside STRINGS naming a package renamed away, which a naive scan reports as real reach. Also enable isolatedModules + verbatimModuleSyntax: both cost ZERO errors today (measured) and turn assumptions the runtime already makes into compile-time guarantees -- per-file transpilability for `--experimental-strip-types` and esbuild, and type-only imports that can never be silently elided from (or dragged into) the self-host bundle. Deliberately NOT enabling noImplicitReturns: 3 errors, and two are documented, intentional implicit-undefined switch arms. Churn for marginal value.
1 parent 235723a commit 39239df

5 files changed

Lines changed: 251 additions & 2 deletions

File tree

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
"engine:coverage": "node --experimental-strip-types scripts/engine-coverage.ts",
6060
"db:migrations:check": "tsx scripts/check-migrations.ts",
6161
"db:migrations:immutable:check": "tsx scripts/check-released-migrations-immutable.ts",
62+
"turbo-inputs:check": "tsx scripts/check-turbo-typecheck-inputs.ts",
6263
"workspace-dep-ranges:check": "tsx scripts/check-workspace-dep-ranges.ts",
6364
"db:schema-drift:check": "tsx scripts/check-schema-drift.ts",
6465
"actionlint": "node --experimental-strip-types scripts/actionlint.ts",
@@ -139,7 +140,7 @@
139140
"test:smoke:browser:install": "playwright install chromium",
140141
"test:smoke:browser": "node --experimental-strip-types scripts/smoke-ui-browser.ts",
141142
"pretest:ci": "npm run check-node-version",
142-
"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",
143+
"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",
143144
"test:release": "npm run test:ci && npm run changelog:check",
144145
"test:release:mcp": "npm run test:ci",
145146
"test:watch": "vitest",
Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
#!/usr/bin/env node
2+
// turbo's `//#typecheck` inputs must cover everything tsc actually reads (#9848).
3+
//
4+
// THE HAZARD, in turbo.json's own words: that inputs list is "a snapshot of test/'s real cross-package reach
5+
// as of the audit that added it, not a structural guarantee -- a future test file importing from a NOT-yet-
6+
// listed package/app would reopen the same silent-stale-cache gap." It then asks a human to "re-run the same
7+
// grep ... before trusting this list again."
8+
//
9+
// Nobody re-runs a grep on request, and the list HAD already drifted when this check was written: `src/` and
10+
// `test/` import from `packages/loopover-mcp/lib/**` and `packages/loopover-miner/scripts/**`, neither of
11+
// which was hashed. Editing either could therefore leave a stale cache HIT on a typecheck that a real
12+
// `tsc --noEmit` would fail -- the exact class of bug PR #5082 already burned this repo on once, and the
13+
// reason the list exists at all.
14+
//
15+
// So: compute the reach instead of remembering it. This is the grep that comment asks for, run every CI.
16+
//
17+
// WHAT COUNTS AS COVERED. A path is fine if it is matched by an inputs glob, OR if it belongs to a workspace
18+
// this task already `dependsOn` -- turbo hashes a dependency task's own inputs, so `@loopover/engine#build`
19+
// covers the engine's sources without them being listed here. Anything else is unhashed and reported.
20+
import { existsSync, readFileSync, readdirSync } from "node:fs";
21+
import { join } from "node:path";
22+
import { fileURLToPath, URL } from "node:url";
23+
24+
/** A cross-boundary import found in src/ or test/, as `<group>/<workspace>/<first-segment>`. */
25+
export type CrossBoundaryReach = { path: string; importedBy: string };
26+
27+
/** Strip `//` line comments and trailing commas so turbo.json (JSONC) parses. Deliberately not a full JSONC
28+
* parser: this file is ours, its comment style is known, and a dependency for one read would be worse. */
29+
export function parseJsonc(text: string): unknown {
30+
return JSON.parse(text.replace(/^\s*\/\/.*$/gm, "").replace(/,(\s*[}\]])/g, "$1"));
31+
}
32+
33+
/**
34+
* PURE core: every cross-workspace path `src/`+`test/` reach that no glob and no dependency covers.
35+
*
36+
* `globs` are turbo `inputs` entries; `coveredWorkspaces` are the workspace directory names whose builds this
37+
* task depends on. Matching is prefix-based on the glob's literal head, which is all turbo's own globs use
38+
* here (`packages/x/lib/**`) -- a stricter matcher would reject valid entries and a looser one would let a
39+
* real gap through.
40+
*/
41+
export function findUnhashedReach(
42+
reach: readonly CrossBoundaryReach[],
43+
globs: readonly string[],
44+
coveredWorkspaces: ReadonlySet<string>,
45+
): CrossBoundaryReach[] {
46+
const prefixes = globs.map((glob) => glob.replace(/\*\*.*$/, "").replace(/\/$/, ""));
47+
return reach.filter((entry) => {
48+
const workspace = entry.path.split("/").slice(0, 2).join("/");
49+
if (coveredWorkspaces.has(workspace)) return false;
50+
return !prefixes.some((prefix) => prefix.length > 0 && (entry.path === prefix || entry.path.startsWith(`${prefix}/`) || prefix.startsWith(entry.path)));
51+
});
52+
}
53+
54+
function walk(dir: string, out: string[]): void {
55+
// Typed via the call's own return rather than `ReturnType<typeof readdirSync>`: that resolves to the
56+
// Buffer-named overload under this tsconfig, which the string form is not assignable to.
57+
let entries: ReadonlyArray<{ name: string; isDirectory(): boolean }>;
58+
try {
59+
entries = readdirSync(dir, { withFileTypes: true });
60+
} catch {
61+
return; // a directory that does not exist here is not an error
62+
}
63+
for (const entry of entries) {
64+
const path = join(dir, entry.name);
65+
if (entry.isDirectory()) {
66+
if (entry.name !== "node_modules") walk(path, out);
67+
} else if (/\.(ts|tsx)$/.test(entry.name)) out.push(path);
68+
}
69+
}
70+
71+
/** Every `packages/<x>/<seg>` or `apps/<x>/<seg>` a relative import from src/ or test/ reaches. */
72+
export function collectCrossBoundaryReach(root: string): CrossBoundaryReach[] {
73+
const files: string[] = [];
74+
walk(join(root, "src"), files);
75+
walk(join(root, "test"), files);
76+
77+
const reach = new Map<string, string>();
78+
for (const file of files) {
79+
const source = readFileSync(file, "utf8");
80+
// Both `from "..."` and bare `import "..."`, since a side-effect import is type-checked too.
81+
for (const match of source.matchAll(/(?:from|import)\s+"((?:\.\.\/)+[^"]+)"/g)) {
82+
const specifier = match[1];
83+
if (!specifier) continue;
84+
const segments = /(?:^|\/)(packages|apps)\/([^/]+)\/([^/"]+)/.exec(specifier);
85+
if (!segments) continue;
86+
const path = `${segments[1]}/${segments[2]}/${segments[3]}`;
87+
// Must exist on disk. The checker-testing files (check-import-specifiers-script.test.ts et al.) embed
88+
// import statements INSIDE FIXTURE STRINGS -- `"src/foo.ts": 'import ... from "../packages/engine/..."'`
89+
// -- and those name packages that were renamed away or never existed. A path tsc cannot resolve is not
90+
// part of its real surface, so requiring the directory to exist filters exactly those without needing
91+
// to parse TypeScript to tell code from a string literal.
92+
if (!existsSync(join(root, path))) continue;
93+
if (!reach.has(path)) reach.set(path, file);
94+
}
95+
}
96+
return [...reach].map(([path, importedBy]) => ({ path, importedBy })).sort((a, b) => a.path.localeCompare(b.path));
97+
}
98+
99+
/** Workspace dirs whose build this task depends on — turbo hashes their inputs transitively. */
100+
export function coveredWorkspacesFromDependsOn(dependsOn: readonly string[], root: string): Set<string> {
101+
const covered = new Set<string>();
102+
for (const dependency of dependsOn) {
103+
const name = dependency.split("#")[0];
104+
if (!name) continue;
105+
for (const group of ["packages", "apps"]) {
106+
let dirs: string[];
107+
try {
108+
dirs = readdirSync(join(root, group), { withFileTypes: true }).filter((entry) => entry.isDirectory()).map((entry) => entry.name);
109+
} catch {
110+
continue;
111+
}
112+
for (const dir of dirs) {
113+
try {
114+
const manifest = JSON.parse(readFileSync(join(root, group, dir, "package.json"), "utf8")) as { name?: string };
115+
if (manifest.name === name) covered.add(`${group}/${dir}`);
116+
} catch {
117+
// not a workspace package
118+
}
119+
}
120+
}
121+
}
122+
return covered;
123+
}
124+
125+
function main(): void {
126+
const root = join(fileURLToPath(new URL(".", import.meta.url)), "..");
127+
const turbo = parseJsonc(readFileSync(join(root, "turbo.json"), "utf8")) as {
128+
tasks?: Record<string, { inputs?: string[]; dependsOn?: string[] }>;
129+
};
130+
const task = turbo.tasks?.["//#typecheck"];
131+
if (!task) {
132+
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.');
133+
process.exit(1);
134+
}
135+
136+
const unhashed = findUnhashedReach(
137+
collectCrossBoundaryReach(root),
138+
task.inputs ?? [],
139+
coveredWorkspacesFromDependsOn(task.dependsOn ?? [], root),
140+
);
141+
142+
if (unhashed.length > 0) {
143+
console.error("turbo //#typecheck does not hash everything tsc reads:\n");
144+
for (const entry of unhashed) console.error(` ${entry.path} (e.g. imported by ${entry.importedBy})`);
145+
console.error(
146+
"\n tsc's real surface is everything transitively imported from src/ + test/, wherever it lives. A path\n" +
147+
" reached from there but absent from `inputs` (and not covered by a dependsOn build) is NOT hashed, so\n" +
148+
" editing it can leave a stale cache HIT on a typecheck a real `tsc --noEmit` would fail — the #5082\n" +
149+
" class of bug this inputs list exists to prevent.\n\n" +
150+
' Fix: add the path (e.g. "packages/x/lib/**") to //#typecheck\'s `inputs` in turbo.json.',
151+
);
152+
process.exit(1);
153+
}
154+
console.log("turbo-typecheck-inputs: OK — every cross-workspace path src/+test/ reach is hashed.");
155+
}
156+
157+
if (process.argv[1]?.endsWith("check-turbo-typecheck-inputs.ts")) main();
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import { describe, expect, it } from "vitest";
2+
import { collectCrossBoundaryReach, coveredWorkspacesFromDependsOn, findUnhashedReach, parseJsonc } from "../../scripts/check-turbo-typecheck-inputs";
3+
4+
// turbo.json's //#typecheck inputs are a hand-maintained approximation of tsc's real transitive surface. Its
5+
// own comment admits it: "a snapshot ... not a structural guarantee", and asks a human to re-run a grep
6+
// before trusting it. Nobody does, and it HAD drifted — four unhashed paths, one of which invalidated the
7+
// comment's own stated reason for excluding it. This runs that grep every CI instead.
8+
9+
const reach = (path: string) => ({ path, importedBy: "test/unit/x.test.ts" });
10+
11+
describe("findUnhashedReach", () => {
12+
it("REGRESSION: reports a reached path no glob covers — the real drift this found", () => {
13+
const unhashed = findUnhashedReach(
14+
[reach("packages/loopover-mcp/lib"), reach("packages/loopover-miner/lib")],
15+
["src/**", "packages/loopover-miner/lib/**"],
16+
new Set(),
17+
);
18+
expect(unhashed.map((entry) => entry.path)).toEqual(["packages/loopover-mcp/lib"]);
19+
});
20+
21+
it("a dependsOn build covers its whole workspace — turbo hashes the dependency's own inputs", () => {
22+
// @loopover/engine#build means the engine's sources are hashed without appearing in this task's inputs.
23+
// Reporting them would be a false positive that trains people to ignore the check.
24+
expect(findUnhashedReach([reach("packages/loopover-engine/src")], [], new Set(["packages/loopover-engine"]))).toEqual([]);
25+
});
26+
27+
it("a glob matches its own directory and anything under it", () => {
28+
expect(findUnhashedReach([reach("packages/x/lib")], ["packages/x/lib/**"], new Set())).toEqual([]);
29+
expect(findUnhashedReach([reach("packages/x/lib")], ["packages/x/**"], new Set())).toEqual([]);
30+
});
31+
32+
it("INVARIANT: a NARROWER glob than the reached path still counts as covered", () => {
33+
// "apps/loopover-ui/src/lib/**" covers a reach recorded as "apps/loopover-ui/src": the hashed set is a
34+
// subset of the directory, which is what the existing entry means, and flagging it would be noise.
35+
expect(findUnhashedReach([reach("apps/loopover-ui/src")], ["apps/loopover-ui/src/lib/**"], new Set())).toEqual([]);
36+
});
37+
38+
it("no globs and no dependencies reports everything, rather than passing vacuously", () => {
39+
expect(findUnhashedReach([reach("packages/a/lib"), reach("apps/b/src")], [], new Set())).toHaveLength(2);
40+
});
41+
});
42+
43+
describe("collectCrossBoundaryReach", () => {
44+
it("REGRESSION: ignores import statements inside FIXTURE STRINGS by requiring the path to exist", () => {
45+
// check-import-specifiers-script.test.ts embeds `'import ... from "../packages/engine/lib/..."'` as test
46+
// DATA. packages/engine was renamed away, so a naive scan reported it as unhashed reach — a false
47+
// positive that would have been "fixed" by hashing a directory that does not exist.
48+
const paths = collectCrossBoundaryReach(process.cwd()).map((entry) => entry.path);
49+
expect(paths).not.toContain("packages/engine/lib");
50+
expect(paths).not.toContain("packages/engine/src");
51+
// And it still finds the real ones.
52+
expect(paths).toContain("packages/loopover-mcp/lib");
53+
});
54+
55+
it("INVARIANT: this repo's own turbo.json covers its real reach", async () => {
56+
// The check the CI script performs, asserted here too so a `turbo.json` edit that drops a path fails in
57+
// the test suite as well as the standalone checker.
58+
const { readFileSync } = await import("node:fs");
59+
const turbo = parseJsonc(readFileSync("turbo.json", "utf8")) as { tasks: Record<string, { inputs?: string[]; dependsOn?: string[] }> };
60+
const task = turbo.tasks["//#typecheck"]!;
61+
const unhashed = findUnhashedReach(
62+
collectCrossBoundaryReach(process.cwd()),
63+
task.inputs ?? [],
64+
coveredWorkspacesFromDependsOn(task.dependsOn ?? [], process.cwd()),
65+
);
66+
expect(unhashed.map((entry) => entry.path)).toEqual([]);
67+
});
68+
});
69+
70+
describe("parseJsonc", () => {
71+
it("strips line comments and trailing commas so turbo.json parses", () => {
72+
expect(parseJsonc('{\n // a comment\n "a": 1,\n}')).toEqual({ a: 1 });
73+
});
74+
});

tsconfig.json

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,15 @@
1010
"exactOptionalPropertyTypes": true,
1111
"noImplicitOverride": true,
1212
"noFallthroughCasesInSwitch": true,
13+
// #9848: both cost ZERO errors today (measured) and lock in what this repo's RUNTIME already assumes.
14+
// `isolatedModules` makes tsc enforce that every file is independently transpilable -- exactly the
15+
// contract `node --experimental-strip-types` and esbuild's per-file transform already rely on, and which
16+
// nothing was checking. `verbatimModuleSyntax` requires type-only imports to say `import type`, so a
17+
// value import can never be silently elided out of the emitted bundle (or, worse, kept and pulled into
18+
// the self-host bundle that must have zero `cloudflare:*` imports). Enabling them turns two runtime
19+
// assumptions into compile-time guarantees rather than conventions.
20+
"isolatedModules": true,
21+
"verbatimModuleSyntax": true,
1322
// #9553. Dead code is the substrate every drift bug in the 2026-07-27 audit grew on: a stale import or an
1423
// orphaned constant reads exactly like a live wire, so the next person greps, finds it, and reasons about
1524
// a code path that no longer runs. Enabling these made the compiler enumerate all 515 instances, and

turbo.json

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,11 @@
2323
// engine package" comment) already burned this repo on once. packages/loopover-mcp/bin/** is
2424
// deliberately NOT listed: every test reference to it is a runtime path string
2525
// (`join(process.cwd(), "packages/loopover-mcp/bin/loopover-mcp.js")`, spawned as a subprocess or
26-
// read as raw text), never a static import, so its contents don't affect tsc's type-checked surface.
26+
// read as raw text) -- that WAS true when written and is not any more: mcp-cli-completion-spec.test.ts
27+
// statically imports CLI_COMMAND_SPEC/CLI_FLAG_SPEC from bin/loopover-mcp, so the .ts IS on tsc's
28+
// surface and is now listed below. This list is no longer trusted to memory either:
29+
// scripts/check-turbo-typecheck-inputs.ts recomputes the real reach every CI run (#9848), which is
30+
// what caught this claim going stale, along with three other unhashed paths.
2731
// This list is a snapshot of test/'s real cross-package reach as of the audit that added it, not a
2832
// structural guarantee -- a future test file importing from a NOT-yet-listed package/app would
2933
// reopen the same silent-stale-cache gap. Re-run the same grep (every src/+test/ import crossing a
@@ -47,6 +51,10 @@
4751
"packages/loopover-miner/bin/**",
4852
"packages/loopover-mcp/package.json",
4953
"packages/discovery-index/src/**",
54+
"packages/discovery-index/scripts/**",
55+
"packages/loopover-mcp/bin/**",
56+
"packages/loopover-mcp/lib/**",
57+
"packages/loopover-miner/scripts/**",
5058
"apps/loopover-ui/src/lib/**"
5159
],
5260
"outputs": []

0 commit comments

Comments
 (0)