diff --git a/scripts/check-turbo-task-graph.mjs b/scripts/check-turbo-task-graph.mjs index b8a9895c78..f2d4b9a601 100644 --- a/scripts/check-turbo-task-graph.mjs +++ b/scripts/check-turbo-task-graph.mjs @@ -105,6 +105,49 @@ * a dependency on a generic task no package declares still runs nothing, and the * exemption would be a hole shaped exactly like the entry above. * + * ## `//#` -- turbo's ROOT-manifest spelling, which is NOT a missing package + * + * `//` is turbo's reserved token for the repo root, so `//#` binds the + * ROOT package.json's own script. It is a working spelling, and before this arm + * existed the gate refused it as an unknown package -- telling the author to + * "delete the entry" about configuration that was doing its job. + * + * Measured on turbo 2.10.12 (what `^2.10.10` resolves to), on the same kind of + * fixture workspace as above, each case driven as a real run as well as a + * `--dry=json`: + * + * `//#lint` where the ROOT manifest declares `lint` + * -> EXIT 0, `//#lint` IS in the graph, `package: "//"`, carrying the root + * script as its real command with the turbo.json override resolved onto + * it (`cache: false`). A real `turbo run lint` prints that script's own + * output and reports `1 successful, 1 total`. + * + * `//#nope` where the ROOT manifest declares no `nope` + * -> EXIT 0, `//#nope` IS in the graph, and a real `turbo run nope` reports + * `No tasks were executed as part of this run`, `0 total`. + * + * So the root token needs BOTH directions, and it is judged against the root + * manifest's scripts exactly as a member key is judged against its own: a script + * that exists makes the entry legitimate, and one that does not leaves the same + * inert entry the second arm above is written for. + * + * The two defects are one defect. The unknown-package arm's sentence "the + * override never reaches the task graph" is measured TRUE for a package that + * does not exist (`@fx/nope#build`: absent from the graph entirely) and measured + * FALSE only for `//` -- so the false sentence is reachable through no input but + * this one, and correcting it and correcting the verdict are the same edit. That + * is also why the shared sentence is left alone: hedging it would trade an + * accurate diagnosis on every member key for a vaguer one, in order to describe + * a case that no longer reaches that arm. + * + * WARNING: this does NOT make the root manifest a workspace member. `holdersOf` + * and the generic limb still enumerate members only -- a generic key held solely + * by the root package.json is still inert and still a finding, which is the whole + * of #12373's measurement. `--self-test` drives that case with `test:e2e` present + * in the root script set, so folding the root into `holdersOf` fails it. The root + * manifest is addressable ONLY under the explicit `//` token, which the + * enumerator can never produce. + * * ## Refusals, never quiet passes (#4690) * * An unreadable or non-JSON turbo.json, a missing `tasks` table, a workspace @@ -126,17 +169,35 @@ import { workspacePackages, WorkspaceEnumerationError } from './workspace-enumer const ROOT = join(fileURLToPath(new URL('.', import.meta.url)), '..'); /** - * The repo-root file this gate opens, in the SUBTREE spelling the dispatch - * derivation can match. See the header for why the bare filename cannot be a + * The repo-root files this gate opens, in the SUBTREE spelling the dispatch + * derivation can match. See the header for why a bare filename cannot be a * watch hint and why this is the escape rather than a fabricated declaration. - * `--self-test` pins the exact string: a reword back to `'turbo.json'` would + * `--self-test` pins the exact strings: a reword back to `'turbo.json'` would * cost the derivation silently, and this gate's whole second purpose with it. + * + * `package.json/**` names the ROOT manifest and only it -- measured: + * `hintCovers('package.json/**', 'packages/spec/package.json')` is false. The + * MEMBER manifests stay undeclared on purpose (the enumerator owns them and + * declares none); this hint is here because the `//#` arm opens the root + * manifest DIRECTLY, from this file, and a bare `'package.json'` literal builds + * no hint at all -- the same trap documented above for turbo.json. */ -export const ROOT_FILE_WATCH_HINTS = ['turbo.json/**']; +export const ROOT_FILE_WATCH_HINTS = ['turbo.json/**', 'package.json/**']; /** The file this gate judges, as the reader spells it on disk. */ const TURBO_CONFIG_FILE = 'turbo.json'; +/** The root manifest, whose scripts a `//#` key is judged against. */ +const ROOT_MANIFEST_FILE = 'package.json'; + +/** + * Turbo's reserved token for the repo root inside a package-task key. It is not + * a package name and the workspace enumerator can never produce it, which is + * exactly why a plain membership lookup reports it as a missing package instead + * of as the root. + */ +export const ROOT_PACKAGE_TOKEN = '//'; + /** Thrown for conditions that must fail the gate rather than shrink its coverage. */ export class TaskGraphReadError extends Error {} @@ -233,18 +294,26 @@ export function holdersOf(task, scriptsByPackage) { * The rule, as a pure function over already-parsed inputs, so `--self-test` can * drive it with the adversarial task tables a clean tree does not contain. * - * The two populations are counted separately and BOTH are returned: a limb that + * The populations are counted separately and ALL are returned: a limb that * quietly stops matching reports zero findings exactly like a limb that found * nothing wrong, so `main()` needs each count to assert it read something. * + * WARNING: `rootJudged` is the exception and is returned for `--self-test` to + * read, NOT for a non-vacuity refusal in `main()`. A clean tree carries no `//#` + * key at all, so zero root tasks is the NORMAL state here rather than evidence + * of a limb that stopped matching -- completing the symmetry with the two counts + * above would fail this repo's turbo.json today, on every PR. + * * @param {Record} tasks turbo.json's `tasks` table * @param {Map>} scriptsByPackage package name -> script names - * @returns {{ problems: string[], judged: number, genericJudged: number }} + * @param {Set} rootScripts the ROOT manifest's script names, for `//#` + * @returns {{ problems: string[], judged: number, genericJudged: number, rootJudged: number }} */ -export function verdict(tasks, scriptsByPackage) { +export function verdict(tasks, scriptsByPackage, rootScripts) { const problems = []; let judged = 0; let genericJudged = 0; + let rootJudged = 0; for (const key of Object.keys(tasks)) { const split = splitTaskKey(key); if (!split) { @@ -270,6 +339,30 @@ export function verdict(tasks, scriptsByPackage) { } judged += 1; const { pkg, task } = split; + // The ROOT token, before any membership lookup: `//` is not a package name, + // so `scriptsByPackage.get('//')` is always a miss and the arm below would + // report turbo's own root spelling as a package that does not exist. Ablating + // this block restores exactly that — a named self-test failure, not a crash. + if (pkg === ROOT_PACKAGE_TOKEN) { + rootJudged += 1; + if (!rootScripts.has(task)) { + const near = nearestNames(task, rootScripts); + problems.push( + `"${key}" configures the task "${task}" on the repo ROOT manifest, which declares\n` + + ` no script by that name.\n` + + ` Turbo accepts this SILENTLY — measured on 2.10.12: exit 0, the task IS placed in\n` + + ` the graph as "${key}", and a real run reports "No tasks were executed as part of\n` + + ` this run". The entry is inert.\n` + + ` ⚠️ "${ROOT_PACKAGE_TOKEN}" is turbo's reserved token for the root ${ROOT_MANIFEST_FILE} — NOT a missing\n` + + ` package, and NOT a workspace member. It is judged against the root manifest's\n` + + ` own scripts, and a root script that DOES exist makes "${ROOT_PACKAGE_TOKEN}#" a legitimate\n` + + ` entry that turbo puts in the graph and runs for real.\n` + + (near.length ? ` Did you mean: ${near.join(', ')}?\n` : '') + + ` Fix the task name, add the script to the root ${ROOT_MANIFEST_FILE}, or delete the entry.`, + ); + } + continue; + } const scripts = scriptsByPackage.get(pkg); if (!scripts) { const near = nearestNames(pkg, scriptsByPackage.keys()); @@ -301,7 +394,7 @@ export function verdict(tasks, scriptsByPackage) { ); } } - return { problems, judged, genericJudged }; + return { problems, judged, genericJudged, rootJudged }; } /** @@ -338,6 +431,58 @@ export function readWorkspaceScripts(root) { return byName; } +/** + * The ROOT manifest's declared script names. + * + * Read directly rather than through the enumerator, because the root is NOT a + * workspace member -- that non-membership is load-bearing for the generic limb + * (#12373) and must not be softened by folding the root into the member map. + * Keeping it a separate value is precisely what lets `//#` be judged while + * `holdersOf` stays members-only. + * + * A root manifest that reads as zero scripts refuses, on the same ground as the + * zero-members refusal above: this repo's root declares well over a hundred, so + * zero means the file was not read, never that it was read and found empty -- + * and a silently empty set would flag every legitimate `//#` key at once. + * + * @param {string} root + * @returns {Set} + */ +export function readRootScripts(root) { + const path = join(root, ROOT_MANIFEST_FILE); + let parsed; + try { + parsed = JSON.parse(readFileSync(path, 'utf8')); + } catch (err) { + throw new TaskGraphReadError(`cannot read the root ${ROOT_MANIFEST_FILE}: ${err.message}`); + } + return rootScriptNames(parsed); +} + +/** + * The script names of an already-parsed root manifest, or a refusal. + * + * Split from the read for the same reason `verdict` is a pure function: the + * refusal below is a branch a clean tree can never reach, so `--self-test` has + * to be able to drive it without a filesystem fixture whose own contents would + * then decide whether the pin holds. + * + * @param {unknown} manifest + * @returns {Set} + */ +export function rootScriptNames(manifest) { + const table = manifest?.scripts; + const scripts = table && typeof table === 'object' && !Array.isArray(table) ? Object.keys(table) : []; + if (scripts.length === 0) { + throw new TaskGraphReadError( + `the root ${ROOT_MANIFEST_FILE} declares no scripts — a "${ROOT_PACKAGE_TOKEN}#" key is judged against\n` + + `exactly that table, so an empty one is a file this gate could not read, never a root\n` + + `manifest with nothing to run. Every such key would otherwise be reported inert at once.`, + ); + } + return new Set(scripts); +} + /** * turbo.json's `tasks` table. * @@ -365,9 +510,11 @@ export function readTasks(root) { function main() { let tasks; let scriptsByPackage; + let rootScripts; try { tasks = readTasks(ROOT); scriptsByPackage = readWorkspaceScripts(ROOT); + rootScripts = readRootScripts(ROOT); } catch (err) { if (err instanceof TaskGraphReadError) { console.error(`FAIL: check-turbo-task-graph could not read its input.\n ${err.message}`); @@ -376,7 +523,7 @@ function main() { throw err; } - const { problems, judged, genericJudged } = verdict(tasks, scriptsByPackage); + const { problems, judged, genericJudged, rootJudged } = verdict(tasks, scriptsByPackage, rootScripts); if (genericJudged === 0) { console.error( @@ -422,9 +569,13 @@ function main() { console.log( `OK: ${judged} package-scoped and ${genericJudged} generic turbo task(s) judged against ` + - `${scriptsByPackage.size} workspace package(s) — every package-scoped one names a package ` + - `that exists and a script it declares, and every generic one is declared by at least one ` + - `member.`, + `${scriptsByPackage.size} workspace package(s)` + + (rootJudged + ? `, ${rootJudged} of them on the root manifest's ${rootScripts.size} script(s)` + : '') + + ` — every package-scoped one names a package that exists (or the root token ` + + `"${ROOT_PACKAGE_TOKEN}") and a script it declares, and every generic one is declared by at ` + + `least one member.`, ); } @@ -448,8 +599,14 @@ export function selfTest() { ['create-objectstack', new Set(['build', 'test'])], ]); + // The ROOT manifest's scripts, which are NOT the workspace's. `test:e2e` is in + // here deliberately: it is the exact entry #12373 was written for — held by the + // root package.json and by ZERO members — so the generic case below fails the + // moment anyone folds these into `holdersOf`. + const rootScripts = new Set(['lint', 'test:e2e', 'dev']); + // ── Direction 1: the rule must go RED on each silent shape turbo accepts ── - const unknownPkg = verdict({ '@objectstack/plugins-auth#typecheck': {} }, workspace); + const unknownPkg = verdict({ '@objectstack/plugins-auth#typecheck': {} }, workspace, rootScripts); t('an unknown package in a task key is a finding', unknownPkg.problems.length === 1); t('the unknown-package finding is counted as judged', unknownPkg.judged === 1); t( @@ -457,7 +614,7 @@ export function selfTest() { unknownPkg.problems[0]?.includes('@objectstack/plugin-auth'), ); - const missingScript = verdict({ '@objectstack/spec#typecheck': {} }, workspace); + const missingScript = verdict({ '@objectstack/spec#typecheck': {} }, workspace, rootScripts); t('a task the package has no script for is a finding', missingScript.problems.length === 1); t( 'the missing-script finding prints what the package DOES declare', @@ -474,10 +631,14 @@ export function selfTest() { // ── The generic limb (#12373), both directions ── // The red case is the entry this limb was written for, spelled as it stood on // 2026-08-26: a key every signal calls fine, held by nobody who could run it. - const inertGeneric = verdict({ 'test:e2e': {} }, workspace); + const inertGeneric = verdict({ 'test:e2e': {} }, workspace, rootScripts); t('a generic task no package declares is a finding', inertGeneric.problems.length === 1); t('the inert generic key is counted in the generic population', inertGeneric.genericJudged === 1); t('the inert generic key is NOT counted as a package task', inertGeneric.judged === 0); + // The root manifest DECLARES `test:e2e` in the fixture above, and the key is + // still a finding — the members are the whole population of the generic limb. + t('a generic key held ONLY by the root manifest is still a finding', inertGeneric.problems.length === 1); + t('a generic key is never counted in the root population', inertGeneric.rootJudged === 0); // Pinned as text because this sentence is the whole defence of the measurement: // the root manifest holds `test:e2e` and is not a member, and a reader who // counts files instead of members reads the finding as already refuted. @@ -485,7 +646,7 @@ export function selfTest() { 'the inert generic finding rules out the root manifest by name', inertGeneric.problems[0]?.includes('ROOT package.json does NOT count'), ); - const nearGeneric = verdict({ typechek: {} }, workspace); + const nearGeneric = verdict({ typechek: {} }, workspace, rootScripts); t('a misspelled generic key is a finding', nearGeneric.problems.length === 1); t('a misspelled generic key suggests the script that exists', nearGeneric.problems[0]?.includes('typecheck')); @@ -495,6 +656,49 @@ export function selfTest() { ); t('holdersOf is empty for a script no package declares', holdersOf('test:e2e', workspace).length === 0); + // ── The ROOT token `//` (#12465), both directions ── + // Measured on turbo 2.10.12: `//#lint` lands in the graph bound to the ROOT + // manifest's script and RUNS (1 successful), while `//#nope` lands in the graph + // and runs nothing. Before this arm both were reported as a package that does + // not exist, over a sentence a dry run contradicts. + const rootLive = verdict({ '//#lint': {} }, workspace, rootScripts); + t('a root task whose script the root manifest declares is GREEN', rootLive.problems.length === 0); + t('a root task counts as a package-scoped key', rootLive.judged === 1); + t('a root task is NOT counted as a generic key', rootLive.genericJudged === 0); + t('a root task is counted in the root population', rootLive.rootJudged === 1); + + const rootInert = verdict({ '//#nope': {} }, workspace, rootScripts); + t('a root task the root manifest declares no script for is a finding', rootInert.problems.length === 1); + t('the inert root task is counted in the root population', rootInert.rootJudged === 1); + // The DIAGNOSIS is pinned as text, not just the count: this input was already + // reported before the fix, and a pin on `problems.length` alone passes just as + // well with the false sentence put back. + t( + 'the inert root finding names the root manifest', + rootInert.problems[0]?.includes('repo ROOT manifest'), + ); + t( + 'the inert root finding does NOT call the root token a missing package', + !rootInert.problems[0]?.includes('is not in this pnpm workspace'), + ); + t( + 'the inert root finding drops the sentence a dry run contradicts', + !rootInert.problems[0]?.includes('never reaches the task graph'), + ); + t( + 'the inert root finding states what a real run DOES report', + rootInert.problems[0]?.includes('No tasks were executed'), + ); + t( + 'the inert root finding suggests a near ROOT script', + verdict({ '//#lnt': {} }, workspace, rootScripts).problems[0]?.includes('Did you mean: lint?'), + ); + // The member arms must not have been widened to accept `//`-ish names: only the + // exact token is the root, and anything else is still an unknown package. + const notRoot = verdict({ '///#build': {} }, workspace, rootScripts); + t('a near-miss of the root token is still an unknown package', notRoot.problems.length === 1); + t('a near-miss of the root token is not in the root population', notRoot.rootJudged === 0); + // ── Direction 2: the rule must stay GREEN on every legitimate shape ── // `gen:schema` stands where `test:e2e` used to: a generic key held by exactly // ONE package is legitimate, and this case is what keeps the limb above from @@ -508,6 +712,7 @@ export function selfTest() { 'create-objectstack#test': {}, }, workspace, + rootScripts, ); t('legitimate package tasks are green', clean.problems.length === 0); t('package-scoped keys are counted separately', clean.judged === 3); @@ -525,7 +730,15 @@ export function selfTest() { // A reword of the hint back to a bare filename is invisible in every other // signal this gate emits: production stays green, CI stays green, and the // only thing lost is that a turbo.json card can name this gate at all. - t('the root file is declared in the SUBTREE spelling', ROOT_FILE_WATCH_HINTS.join(',') === 'turbo.json/**'); + t( + 'the root files are declared in the SUBTREE spelling', + ROOT_FILE_WATCH_HINTS.join(',') === 'turbo.json/**,package.json/**', + ); + t( + 'the declaration names exactly the two root files this gate opens', + ROOT_FILE_WATCH_HINTS.map((h) => h.replace(/\/\*+$/, '')).join(',') === + `${TURBO_CONFIG_FILE},${ROOT_MANIFEST_FILE}`, + ); // ── Refusals must refuse (#4690) ── const refuses = (label, fn) => { @@ -537,12 +750,22 @@ export function selfTest() { } }; refuses('a turbo.json that cannot be read refuses', () => readTasks(join(ROOT, 'scripts', 'no-such-dir-12046'))); + refuses('a root package.json that cannot be read refuses', () => + readRootScripts(join(ROOT, 'scripts', 'no-such-dir-12465')), + ); + // The zero-scripts refusal, driven directly: a clean tree cannot reach it, and + // a silently empty set would report every legitimate `//#` key inert at once. + refuses('a root manifest with no scripts table refuses', () => rootScriptNames({})); + refuses('a root manifest with an empty scripts table refuses', () => rootScriptNames({ scripts: {} })); + refuses('a root manifest whose scripts table is an array refuses', () => rootScriptNames({ scripts: ['lint'] })); + t('a root manifest with scripts yields exactly its script names', + [...rootScriptNames({ scripts: { lint: 'eslint .', dev: 'node x' } })].sort().join(',') === 'dev,lint'); // ── Non-vacuity, on the LIVE tree ── // The cases above are all synthetic; this is the one that fails when the gate // is wired to a file or a workspace it cannot actually reach. try { - const live = verdict(readTasks(ROOT), readWorkspaceScripts(ROOT)); + const live = verdict(readTasks(ROOT), readWorkspaceScripts(ROOT), readRootScripts(ROOT)); t('the live turbo.json presents package tasks to judge', live.judged > 0); t('the live turbo.json presents generic tasks to judge', live.genericJudged > 0); } catch (err) {