diff --git a/scripts/check-cross-package-test-inputs.mjs b/scripts/check-cross-package-test-inputs.mjs index 9766c2346c..a54c6d40d2 100644 --- a/scripts/check-cross-package-test-inputs.mjs +++ b/scripts/check-cross-package-test-inputs.mjs @@ -227,7 +227,7 @@ import { join, resolve, relative, dirname, sep, isAbsolute } from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; import process from 'node:process'; -import { CROSS_PACKAGE_TEST_INPUTS } from './cross-package-test-inputs.mjs'; +import { CROSS_PACKAGE_TEST_INPUTS, NODE_MODULES_EXCLUSION, NODE_MODULES_SEGMENT } from './cross-package-test-inputs.mjs'; import { matchesAny, selfTest as globMatchSelfTest } from './glob-match.mjs'; import { isEntrypoint } from './invoked-as.mjs'; @@ -264,11 +264,12 @@ const SELF_TEST_BATTERIES = Object.freeze({ 'the RESOLVER half (#10452)': 43, 'the entry guard, driven for real': 2, 'the SPLIT test:repo task (#16466)': 16, + 'the node_modules REACH rule (#16555)': 18, }); // DELETING an entry silences that battery's floor exactly as effectively as // zeroing it, so the roster's own size is pinned too. -const SELF_TEST_BATTERY_FLOOR = 8; +const SELF_TEST_BATTERY_FLOOR = 9; // The key an assertion is filed under when no battery is open. It is not a // declared battery, so it reds by the same set difference rather than silently @@ -1548,6 +1549,93 @@ export function readSplit(pkgDir) { return { split: true, listed }; } +/** + * ── Layer B's second question: what else does the declared glob MATCH? (#16555) + * + * Layer B above asks whether turbo hashes the declared globs. It never asked + * what those globs hash BESIDES the declaration, and that is a whole defect + * class it cannot see: a `$TURBO_ROOT$` glob resolves against the FILESYSTEM, + * so `packages/**` descends into every installed dependency tree under + * `packages/` while the walks these globs are declared against all skip + * `node_modules`. The declaration is wider than the walk in the one direction + * a glob cannot narrow by itself, and the task stops being cacheable at all -- + * vitest rewrites its own `results.json` under `node_modules/.vite` on every + * run, so on a runner that ran any vitest earlier the hash has already moved + * before the task is hashed. + * + * That is why the rule is not "fix the glob". A glob CANNOT express the + * exclusion (measured, and recorded where `NODE_MODULES_EXCLUSION` is + * declared: turbo drops a negation carrying the root token instead of applying + * it). What the next declaration needs is the EXCLUSION on the task, and this + * rule is what makes its absence red instead of silent. + * + * The reach question is answered by the MATCHER, never by sniffing the glob's + * text for a wildcard: a witness path is assembled with the excluded segment in + * each wildcard position and handed to `matchesAny`, so a glob is judged by the + * same semantics Layers A and B judge it by. A declaration of literal file + * paths -- most rows in the table -- yields no witness and reaches nothing, so + * it owes no exclusion and stays green. + * + * @param {string} glob repo-relative, as declared + * @returns {boolean} whether some path with a `node_modules` segment matches it + */ +export function globReachesNodeModules(glob) { + const segments = glob.split('/'); + // A wildcard segment stands in for one concrete segment: `**` spans zero or + // more, so one is a legal instance of it, and `*` stays inside its own. + const concrete = (seg) => (seg === '**' ? 'd' : seg.replace(/\*/g, 'x')); + for (let i = 0; i < segments.length; i++) { + if (!segments[i].includes('*')) continue; + const head = segments.slice(0, i).map(concrete); + const tail = segments.slice(i + 1).map(concrete); + // A trailing wildcard covers a SUBTREE, so the witness puts a file under + // the excluded directory rather than ending at its bare name. + const witness = [...head, NODE_MODULES_SEGMENT, ...(tail.length ? tail : ['x'])].join('/'); + if (matchesAny(witness, [glob])) return true; + } + return false; +} + +/** + * The `node_modules` half of Layer B for one package: the globs that reach + * there, and the exclusion the owning task therefore owes. + * + * Two grains, because they fail differently. A glob that NAMES the excluded + * segment is refused outright -- the exclusion would cancel it, leaving a + * declaration that reads as a radius and hashes nothing. A glob that merely + * REACHES there is legitimate (every walking test's radius does) and owes the + * exclusion on its task instead. + */ +export function nodeModulesProblems(name, globs, task, owner) { + const problems = []; + const naming = globs.filter((g) => g.split('/').includes(NODE_MODULES_SEGMENT)); + if (naming.length) { + problems.push( + `${name} declares glob(s) that NAME the ${NODE_MODULES_SEGMENT} directory:\n` + + naming.map((g) => ` ${g}`).join('\n') + + `\n An installed dependency is not a repo source input, and "${NODE_MODULES_EXCLUSION}" on the\n` + + ` task cancels the glob anyway -- declared, hashing nothing, reading as a radius.\n` + + ` Declare the source path the test really reads.`, + ); + } + const reaching = globs.filter(globReachesNodeModules); + if (reaching.length && !(task?.inputs ?? []).includes(NODE_MODULES_EXCLUSION)) { + problems.push( + `turbo.json "${owner}" hashes glob(s) that reach into ${NODE_MODULES_SEGMENT}/ and carries no\n` + + ` exclusion for it:\n` + + reaching.map((g) => ` $TURBO_ROOT$/${g}`).join('\n') + + `\n A $TURBO_ROOT$ glob is resolved against the FILESYSTEM, not git's tracked set, so\n` + + ` it descends into every installed dependency tree it spans -- including vitest's own\n` + + ` results cache, which is rewritten by every run, so the task can never replay from\n` + + ` cache on a runner that ran any vitest before it.\n` + + ` Add ${JSON.stringify(NODE_MODULES_EXCLUSION)} to this task's inputs. ⛔ Not a negated\n` + + ` $TURBO_ROOT$ input -- turbo drops that form instead of applying it (measured; see\n` + + ` NODE_MODULES_EXCLUSION in scripts/cross-package-test-inputs.mjs).`, + ); + } + return problems; +} + /** * Layer B for one package: the task that must hash the declared globs, and -- * for a split package -- the task that must not. `tasks` is turbo.json's map. @@ -1556,6 +1644,7 @@ export function turboInputProblems(name, globs, tasks, split) { const problems = []; const owner = split ? `${name}#${REPO_TASK}` : `${name}#test`; const task = tasks?.[owner]; + problems.push(...nodeModulesProblems(name, globs, task, owner)); if (!task) { problems.push( `turbo.json has no "${owner}" task. Without it the package's test cache is\n` + @@ -1632,7 +1721,19 @@ export function repoProjectProblems(name, escapingTests, listed) { } function expectedInputs(globs) { - return ['$TURBO_DEFAULT$', '!dist/**', '!coverage/**', '!.turbo/**', ...globs.map((g) => `$TURBO_ROOT$/${g}`)]; + // The `node_modules` exclusion is prescribed only when a glob really reaches + // there, so the prescription this gate prints is one that its own rule would + // accept -- and a literal-path declaration is not handed an entry it does not + // owe. + const excluded = globs.some(globReachesNodeModules) ? [NODE_MODULES_EXCLUSION] : []; + return [ + '$TURBO_DEFAULT$', + '!dist/**', + '!coverage/**', + '!.turbo/**', + ...excluded, + ...globs.map((g) => `$TURBO_ROOT$/${g}`), + ]; } /** @@ -2556,10 +2657,15 @@ function selfTest() { battery('the SPLIT test:repo task (#16466)'); { const T = (inputs) => ({ inputs }); - const G = ['content/**']; + // A LITERAL path, not a subtree glob: this battery is about which TASK + // hashes the radius, and the #16555 rule beside it reds a reaching glob on + // a task with no `node_modules` exclusion. A wide fixture here would fail + // these cases for that unrelated reason and blur which rule they pin, so + // the two batteries stay orthogonal -- reach is exercised in its own. + const G = [['content', 'docs', 'index.mdx'].join('/')]; const local = T(['$TURBO_DEFAULT$', '!dist/**']); - const wideOnTest = T(['$TURBO_DEFAULT$', '$TURBO_ROOT$/content/**']); - const repoTask = T(['$TURBO_DEFAULT$', '$TURBO_ROOT$/content/**']); + const wideOnTest = T(['$TURBO_DEFAULT$', `$TURBO_ROOT$/${G[0]}`]); + const repoTask = T(['$TURBO_DEFAULT$', `$TURBO_ROOT$/${G[0]}`]); // Layer B, both shapes ok('unsplit: the radius on #test is green', turboInputProblems('p', G, { 'p#test': wideOnTest }, false).length === 0); ok('unsplit: a glob missing from #test reds', turboInputProblems('p', G, { 'p#test': local }, false).some((m) => m.includes('missing the declared glob'))); @@ -2589,6 +2695,103 @@ function selfTest() { } } + // ── the node_modules REACH rule (#16555) ───────────────────────────────── + // + // Layer B asks whether turbo hashes the declared globs. This battery is the + // other question -- what ELSE the glob matches -- and it is pinned in both + // directions on purpose. A rule that reds every root-scoped glob would pass + // every "it catches something" case above and break the repo, so the cases + // that must stay GREEN (a literal-path declaration, a task that carries the + // exclusion) are as load-bearing here as the ones that must go red. + battery('the node_modules REACH rule (#16555)'); + { + const NM = NODE_MODULES_SEGMENT; + const P = (...segments) => segments.join('/'); + const WIDE = P('packages', '**', '*.json'); + const SUBTREE = P('packages', 'lint', 'src', '**'); + const LITERAL = P('scripts', 'js-comment-mask.mjs'); + const ONE_SEGMENT = P('packages', '*.json'); + const NAMED = P('packages', '**', NM, '**'); + + // reach, answered by the matcher + ok('a `**` glob under packages/ reaches node_modules', globReachesNodeModules(WIDE)); + ok('a per-directory subtree glob reaches it too', globReachesNodeModules(SUBTREE)); + ok('a declaration of one literal file path reaches nothing', !globReachesNodeModules(LITERAL)); + ok('a single-segment `*` cannot span into a node_modules subtree', !globReachesNodeModules(ONE_SEGMENT)); + ok('a glob that names the directory outright reaches it', globReachesNodeModules(NAMED)); + + // the rule, on a task map + const withExclusion = { inputs: ['$TURBO_DEFAULT$', NODE_MODULES_EXCLUSION, `$TURBO_ROOT$/${WIDE}`] }; + const without = { inputs: ['$TURBO_DEFAULT$', `$TURBO_ROOT$/${WIDE}`] }; + ok( + 'a reaching glob on a task with no exclusion reds, naming the entry to add', + nodeModulesProblems('p', [WIDE], without, 'p#test').some((m) => m.includes(NODE_MODULES_EXCLUSION)), + ); + ok( + 'the same task carrying the exclusion is green', + nodeModulesProblems('p', [WIDE], withExclusion, 'p#test').length === 0, + ); + ok( + 'NEGATIVE CONTROL: a literal-path declaration owes no exclusion and stays green', + nodeModulesProblems('p', [LITERAL], without, 'p#test').length === 0, + ); + ok( + 'a glob that NAMES node_modules is refused outright, exclusion or not', + nodeModulesProblems('p', [NAMED], withExclusion, 'p#test').some((m) => m.includes('NAME')), + ); + ok( + 'a missing task (no inputs at all) reds for the reaching glob rather than throwing', + nodeModulesProblems('p', [WIDE], undefined, 'p#test').length === 1, + ); + + // and through Layer B's own entry point, which is what runs in CI + const T = (inputs) => ({ inputs }); + const complete = T(['$TURBO_DEFAULT$', NODE_MODULES_EXCLUSION, `$TURBO_ROOT$/${WIDE}`]); + const missingExclusion = T(['$TURBO_DEFAULT$', `$TURBO_ROOT$/${WIDE}`]); + ok( + 'Layer B reds a complete radius that is hashed over node_modules', + turboInputProblems('p', [WIDE], { 'p#test': missingExclusion }, false).some((m) => m.includes('reach into')), + ); + ok( + 'Layer B is green once the exclusion is there', + turboInputProblems('p', [WIDE], { 'p#test': complete }, false).length === 0, + ); + ok( + 'NEGATIVE CONTROL: Layer B stays green for a literal-path declaration with no exclusion', + turboInputProblems('p', [LITERAL], { 'p#test': T(['$TURBO_DEFAULT$', `$TURBO_ROOT$/${LITERAL}`]) }, false).length === 0, + ); + + // the prescription this gate prints must be one its own rule accepts + ok('the prescribed inputs carry the exclusion for a reaching glob', expectedInputs([WIDE]).includes(NODE_MODULES_EXCLUSION)); + ok('and do NOT carry it for a literal-path declaration', !expectedInputs([LITERAL]).includes(NODE_MODULES_EXCLUSION)); + + // the exclusion's own spelling: the measured trap is the root token + ok( + 'the exclusion is package-relative -- a negation carrying the root token is inert in turbo', + NODE_MODULES_EXCLUSION.startsWith('!') && !NODE_MODULES_EXCLUSION.includes('$TURBO_ROOT$'), + ); + + // the live tree, not a fixture: every declaring package's owning task is + // judged by the same rule CI runs, so this battery cannot pass on a repo + // whose turbo.json has drifted back. + const liveTasks = JSON.parse(readFileSync(join(REPO_ROOT, 'turbo.json'), 'utf8')).tasks; + const liveReaching = Object.entries(CROSS_PACKAGE_TEST_INPUTS).filter(([, { globs }]) => globs.some(globReachesNodeModules)); + ok('the live table still has packages whose globs reach node_modules', liveReaching.length > 0); + ok( + 'the live table still has packages whose globs do NOT -- the negative control is populated', + liveReaching.length < Object.keys(CROSS_PACKAGE_TEST_INPUTS).length, + ); + ok( + 'every live task whose globs reach node_modules carries the exclusion', + liveReaching.every(([name]) => { + const info = findEscapingPackages().get(name); + const { split } = info ? readSplit(join(REPO_ROOT, info.dir)) : { split: false }; + const owner = split ? `${name}#${REPO_TASK}` : `${name}#test`; + return (liveTasks?.[owner]?.inputs ?? []).includes(NODE_MODULES_EXCLUSION); + }), + ); + } + battery('the entry guard, driven for real'); const importProbe = spawnSync( process.execPath, diff --git a/scripts/cross-package-test-inputs.mjs b/scripts/cross-package-test-inputs.mjs index 9ba93e27e0..259d34db0e 100644 --- a/scripts/cross-package-test-inputs.mjs +++ b/scripts/cross-package-test-inputs.mjs @@ -1315,3 +1315,61 @@ export const CROSS_PACKAGE_TEST_INPUTS = { }, }, }; + +/** + * The one input entry that keeps a declared radius from being hashed over + * `node_modules/` (#16555). + * + * ## The defect, and why the glob table alone cannot state it + * + * A `$TURBO_ROOT$` glob is resolved against the FILESYSTEM, not against git's + * tracked set, so `packages/**` descends into every installed dependency tree + * under `packages/`. The globs above are declared against what the escaping + * tests WALK, and every walk in this repo skips `node_modules` -- the + * declaration was simply wider than the walk, in the one direction a glob + * cannot narrow by itself. + * + * What that costs is not a wasted read, it is a task that can never replay from + * cache. vitest rewrites `node_modules/.vite/vitest//results.json` on + * every run, so on a runner that ran any vitest earlier the hash has already + * moved before the task is hashed. Measured on the card's tree: 9,404 input + * keys for `@objectstack/spec#test:repo`, exactly ONE changed hash between two + * consecutive dry-runs, and that file was it -- while the package-local + * `@objectstack/spec#test` (`$TURBO_DEFAULT$`, git-scoped) read HIT across the + * same experiment. + * + * ## Why this spelling and not a negated `$TURBO_ROOT$` input + * + * Measured on turbo 2.10.10 against `@objectstack/spec#test:repo`, three + * probe files planted under three packages' dependency trees (a real tracked + * `.json` under `packages/` staying an input throughout, as the firing + * control): + * + * baseline 7307 keys, 3 under node_modules + * a negation carrying the root token 7307 keys, 3 -- INERT + * the same, `*` instead of `**` 7307 keys, 3 -- INERT + * the same, rooted at the repo instead 7307 keys, 3 -- INERT + * THIS spelling (package-relative) 7304 keys, 0 + * + * ⇒ turbo drops a negation that carries the root token instead of applying it, + * silently. The negation mechanism itself is live -- a package-relative + * `"!LICENSE"` on the same task removed exactly that one key (7307 -> 7306) -- + * so the three zeros above are a reading about the ROOT TOKEN, not about + * negation. ⛔ Do not "tidy" this entry into the `$TURBO_ROOT$` form the globs + * beside it use: it reads as consistent and enforces nothing. + * + * Package-relative is also why it needs no per-package depth: `**` spans zero + * or more segments, so this one string covers the task's own tree and the + * `..//...` keys a root-scoped glob contributes alike. + * + * `check-cross-package-test-inputs.mjs` requires it on the turbo task of every + * package that declares a glob able to reach a `node_modules` path, so the next + * root-scoped declaration cannot re-commit this silently. + */ +export const NODE_MODULES_EXCLUSION = '!**/node_modules/**'; + +/** + * The path segment `NODE_MODULES_EXCLUSION` excludes, as the gate's rule needs + * it: a name to test a declared glob's reach against, not a path. + */ +export const NODE_MODULES_SEGMENT = 'node_modules'; diff --git a/turbo.json b/turbo.json index 4117be685e..65ae1fe55c 100644 --- a/turbo.json +++ b/turbo.json @@ -96,6 +96,7 @@ "!dist/**", "!coverage/**", "!.turbo/**", + "!**/node_modules/**", "$TURBO_ROOT$/packages/**/package.json" ] }, @@ -108,6 +109,7 @@ "!dist/**", "!coverage/**", "!.turbo/**", + "!**/node_modules/**", "$TURBO_ROOT$/scripts/nightly-tiers.mjs", "$TURBO_ROOT$/scripts/nightly-tiers.d.mts", "$TURBO_ROOT$/packages/verify/src/**", @@ -153,6 +155,7 @@ "!dist/**", "!coverage/**", "!.turbo/**", + "!**/node_modules/**", "$TURBO_ROOT$/packages/runtime/src/route-ledger.ts", "$TURBO_ROOT$/packages/rest/src/rest-route-ledger.ts", "$TURBO_ROOT$/packages/services/service-storage/src/storage-route-ledger.ts", @@ -180,6 +183,7 @@ "!dist/**", "!coverage/**", "!.turbo/**", + "!**/node_modules/**", "$TURBO_ROOT$/packages/cli/src/commands/**", "$TURBO_ROOT$/packages/metadata-protocol/src/**", "$TURBO_ROOT$/packages/objectql/src/validation/**", @@ -235,6 +239,7 @@ "!dist/**", "!coverage/**", "!.turbo/**", + "!**/node_modules/**", "$TURBO_ROOT$/packages/**/*.object.ts", "$TURBO_ROOT$/scripts/js-comment-mask.mjs", "$TURBO_ROOT$/scripts/js-comment-mask.d.mts", @@ -312,6 +317,7 @@ "!dist/**", "!coverage/**", "!.turbo/**", + "!**/node_modules/**", "$TURBO_ROOT$/packages/**/*.object.ts", "$TURBO_ROOT$/packages/core/src/security/**", "$TURBO_ROOT$/packages/runtime/src/**", @@ -379,6 +385,7 @@ "!dist/**", "!coverage/**", "!.turbo/**", + "!**/node_modules/**", "$TURBO_ROOT$/packages/spec/src/security/**", "$TURBO_ROOT$/packages/spec/src/identity/**" ] @@ -392,6 +399,7 @@ "!dist/**", "!coverage/**", "!.turbo/**", + "!**/node_modules/**", "$TURBO_ROOT$/packages/client/src/**", "$TURBO_ROOT$/packages/mcp/src/**", "$TURBO_ROOT$/packages/plugins/plugin-hono-server/src/**", @@ -429,6 +437,7 @@ "!dist/**", "!coverage/**", "!.turbo/**", + "!**/node_modules/**", "$TURBO_ROOT$/packages/spec/src/security/rls.zod.ts", "$TURBO_ROOT$/skills/objectstack-formula/**" ] @@ -454,6 +463,7 @@ "!dist/**", "!coverage/**", "!.turbo/**", + "!**/node_modules/**", "$TURBO_ROOT$/packages/spec/src/**", "$TURBO_ROOT$/packages/spec/package.json" ] @@ -467,6 +477,7 @@ "!dist/**", "!coverage/**", "!.turbo/**", + "!**/node_modules/**", "$TURBO_ROOT$/content/**", "$TURBO_ROOT$/scripts/sync-template-versions.mjs", "$TURBO_ROOT$/scripts/invoked-as.mjs", @@ -504,6 +515,7 @@ "!dist/**", "!coverage/**", "!.turbo/**", + "!**/node_modules/**", "$TURBO_ROOT$/packages/**/*.object.ts", "$TURBO_ROOT$/packages/lint/src/**", "$TURBO_ROOT$/content/docs/references/**", @@ -544,6 +556,7 @@ "!dist/**", "!coverage/**", "!.turbo/**", + "!**/node_modules/**", "$TURBO_ROOT$/packages/**/*.ts" ] }, @@ -555,6 +568,7 @@ "!dist/**", "!coverage/**", "!.turbo/**", + "!**/node_modules/**", "$TURBO_ROOT$/packages/**/*.ts" ] }, @@ -566,6 +580,7 @@ "!dist/**", "!coverage/**", "!.turbo/**", + "!**/node_modules/**", "$TURBO_ROOT$/scripts/js-comment-mask.mjs", "$TURBO_ROOT$/scripts/js-comment-mask.d.mts", "$TURBO_ROOT$/packages/**/*.ts", @@ -580,6 +595,7 @@ "!dist/**", "!coverage/**", "!.turbo/**", + "!**/node_modules/**", "$TURBO_ROOT$/packages/**/*.ts", "$TURBO_ROOT$/scripts/cross-package-test-inputs.mjs" ] @@ -592,6 +608,7 @@ "!dist/**", "!coverage/**", "!.turbo/**", + "!**/node_modules/**", "$TURBO_ROOT$/content/docs/protocol/objectql/state-machine.mdx", "$TURBO_ROOT$/skills/objectstack-automation/SKILL.md", "$TURBO_ROOT$/content/**",