Skip to content

Commit c25bfb3

Browse files
Elon Muskclaude
andauthored
fix(scripts): name the i18n gates' full build-prerequisite closure once (#13167)
* wip(scripts): derive the i18n gates' full build-prerequisite closure * feat(scripts): name the i18n gates' full build-prerequisite closure once * fix(scripts): state the empty-closure failure accurately * style(scripts): read the CLI-only remedy after the closure, not before --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 882e1ce commit c25bfb3

3 files changed

Lines changed: 324 additions & 19 deletions

File tree

scripts/check-i18n-bundles.mjs

Lines changed: 104 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,11 @@ import {
9292
atRepoRoot,
9393
CLI,
9494
CLI_BUILD_FIX,
95+
closureBuildFix,
9596
looksLikeMissingCliCommand,
9697
looksLikeStaleWorkspaceDist,
9798
oclifCommandFileFor,
99+
owningPackageOf,
98100
resolveCliCommandFile,
99101
workspaceBuildFix,
100102
} from './cli-build-prerequisite.mjs';
@@ -296,6 +298,49 @@ function discoverExtractConfigs() {
296298
.sort();
297299
}
298300

301+
/**
302+
* This gate's WHOLE build prerequisite, named once (#12564) — the CLI plus the
303+
* build closure of every package whose extract config it runs, derived from the
304+
* population rather than written down.
305+
*
306+
* `CLI_BUILD_FIX` alone under-prescribes here and always did: `os i18n extract`
307+
* loads the package it is pointed at, so a tree with only the CLI built still
308+
* cannot be measured. What made that expensive rather than merely incomplete is
309+
* that the remedy a reader reaches for next is the ONE package the diagnosis
310+
* names — and that one is all node can name, because it stops resolving a
311+
* config's imports at the first specifier with no `dist/`. One package per
312+
* round, however many are missing.
313+
*
314+
* The walk can throw (#11647), and main() turns that into its own worded verdict.
315+
* A throw at MODULE scope would print a node stack instead of that verdict, so
316+
* this catches and degrades to the coarser remedy — never to a partial one.
317+
*/
318+
const POPULATION_CLOSURE = (() => {
319+
try {
320+
return closureBuildFix(discoverExtractConfigs());
321+
} catch (err) {
322+
return { unknown: `the population could not be walked (${String(err?.message ?? err)})` };
323+
}
324+
})();
325+
326+
/** …that closure as a command, or the strict superset when it cannot be derived. */
327+
const WORKSPACE_CLOSURE_FIX = POPULATION_CLOSURE.command ?? 'pnpm build';
328+
329+
/**
330+
* Why the one-command remedy is the one to run, said once so the two CLI-shaped
331+
* prerequisites cannot drift apart on it.
332+
*/
333+
const CLOSURE_FIX_NOTE = [
334+
`That is this gate's WHOLE prerequisite in ONE command (#12564) — the CLI, plus the`,
335+
`build closure of every package whose extract config it runs. Clearing only the CLI`,
336+
`(\`${CLI_BUILD_FIX}\`)`,
337+
`moves the wall rather than removing it: \`os i18n extract\` loads the package it is`,
338+
`pointed at, so that package needs its own build output too.`,
339+
`⛔ Clearing it one package at a time does NOT converge: node stops resolving a`,
340+
`config's imports at the FIRST one with no \`dist/\`, so each round can name exactly`,
341+
`one more, however many are missing. Run the closure once.`,
342+
];
343+
299344
/**
300345
* The detail for a walk that threw. Pure, so `--self-test` can pin the one
301346
* property that matters: it says the checkout is broken, NOT that the reader
@@ -782,6 +827,44 @@ function selfTest() {
782827
offRootPopulation.every((c) => !c.startsWith('/') && !c.includes(REPO_ROOT)),
783828
`absolute paths would leak into the rerun command and the extractor argv; got ${JSON.stringify(offRootPopulation.slice(0, 2))}`,
784829
);
830+
831+
// The build-prerequisite CLOSURE (#12564). `CLI_BUILD_FIX` alone was never
832+
// enough for THIS gate — `os i18n extract` loads the package it is pointed at —
833+
// and the remedy is only worth naming if ONE round of it clears the whole
834+
// population. So the property is COMPLETENESS, checked per config against the
835+
// manifests the derivation read rather than against a list written here.
836+
const missingFromClosure = onRootPopulation
837+
.map((configPath) => owningPackageOf(configPath))
838+
.filter((owner) => !owner || !WORKSPACE_CLOSURE_FIX.includes(`--filter=${owner}`));
839+
expect(
840+
'#12564 the closure names every package this gate extracts',
841+
POPULATION_CLOSURE.command !== undefined && missingFromClosure.length === 0,
842+
`${missingFromClosure.length} owner(s) absent (${missingFromClosure.join(', ')})${
843+
POPULATION_CLOSURE.unknown ? ` — no closure was derived: ${POPULATION_CLOSURE.unknown}` : ''
844+
} — a closure missing one package leaves the reader the round-trip it exists to remove`,
845+
);
846+
expect(
847+
'#12564 …and the CLI it spawns',
848+
WORKSPACE_CLOSURE_FIX.includes('--filter=@objectstack/cli'),
849+
'the closure is offered as the remedy for the CLI prerequisite, so it must clear it',
850+
);
851+
// ⛔ ALL-OR-NOTHING, the floor: a closure that names SOME of the population is
852+
// specific, looks derived, and still does not converge. Both degenerate inputs
853+
// must come back as a REASON rather than a command.
854+
expect(
855+
'#12564 an empty population names no closure, and one unowned config refuses the whole',
856+
closureBuildFix([]).command === undefined &&
857+
closureBuildFix([...onRootPopulation, 'no/such/place/scripts/i18n-extract.config.ts']).command === undefined,
858+
'an empty population would render as the CLI-only remedy under the closure\'s name, and a partial closure ' +
859+
"is this defect wearing a derivation's clothes",
860+
);
861+
// ⛔ Fence: the remedy got longer; the REFUSAL did not become a pass. The
862+
// prerequisite reporter still exits non-zero and still says nothing was judged.
863+
expect(
864+
'#12564 the closure remedy is not the CLI-only one',
865+
WORKSPACE_CLOSURE_FIX !== CLI_BUILD_FIX && WORKSPACE_CLOSURE_FIX !== POPULATION_FIX,
866+
'the three remedies answer different failures and must not collapse into one another',
867+
);
785868
expect(
786869
'#11647 …and the bare spelling demonstrably would not have',
787870
bareWalkOffRoot === 'ENOENT',
@@ -854,7 +937,8 @@ function selfTest() {
854937
console.log(
855938
'✓ check:i18n --self-test — bundle-drift, undeclared-authoring-key, missing-CLI-build, ' +
856939
'stale-workspace-dist and empty-population classifiers all go red, and stay distinct; ' +
857-
'the population walk is CWD-independent.',
940+
'the population walk is CWD-independent; and the build-prerequisite closure names every ' +
941+
'package this gate extracts plus the CLI, refusing whole rather than naming some.',
858942
);
859943
}
860944

@@ -989,13 +1073,17 @@ function checkCliBuildPrerequisite() {
9891073
// all, so anchoring the read without anchoring this would have traded a harmless
9901074
// deferral for a false hard failure.
9911075
if (existsSync(atRepoRoot(resolved.file))) return;
992-
reportPrerequisiteNotMet('the workspace CLI is not built', [
993-
`This gate runs the BUILT CLI. ${CLI} is only a source stub that hands`,
994-
`off to oclif, which resolves \`os ${EXTRACT_COMMAND_ID.join(' ')}\` from the compiled`,
995-
`output — and that command is not there:`,
996-
``,
997-
` ${resolved.file}`,
998-
]);
1076+
reportPrerequisiteNotMet(
1077+
'the workspace CLI is not built',
1078+
[
1079+
`This gate runs the BUILT CLI. ${CLI} is only a source stub that hands`,
1080+
`off to oclif, which resolves \`os ${EXTRACT_COMMAND_ID.join(' ')}\` from the compiled`,
1081+
`output — and that command is not there:`,
1082+
``,
1083+
` ${resolved.file}`,
1084+
],
1085+
{ fix: WORKSPACE_CLOSURE_FIX, alsoFix: CLOSURE_FIX_NOTE },
1086+
);
9991087
}
10001088

10011089
checkCliBuildPrerequisite();
@@ -1107,7 +1195,7 @@ for (const [index, config] of configs.entries()) {
11071195
`Every remaining package would fail the same way for the same one reason, so the`,
11081196
`loop stopped here rather than reporting it ${configs.length} times as bundle problems.`,
11091197
],
1110-
{ scanned: index },
1198+
{ fix: WORKSPACE_CLOSURE_FIX, alsoFix: CLOSURE_FIX_NOTE, scanned: index },
11111199
);
11121200
}
11131201
const stale = looksLikeStaleWorkspaceDist(`${stdout}\n${stderr}`);
@@ -1117,7 +1205,13 @@ for (const [index, config] of configs.entries()) {
11171205
staleWorkspaceDistDetail(stale, { pkg, status: run.status, remaining: configs.length - index - 1 }),
11181206
{
11191207
fix: workspaceBuildFix(stale.pkg),
1120-
alsoFix: ['…or `pnpm build`, on a tree whose other packages may be stale too.'],
1208+
alsoFix: [
1209+
`…or the whole prerequisite at once, on a tree whose other packages may be stale`,
1210+
`too. node names only the FIRST unresolvable import per round, so rebuilding the`,
1211+
`one package above and re-running can simply name the next (#12564):`,
1212+
``,
1213+
` ${WORKSPACE_CLOSURE_FIX}`,
1214+
],
11211215
scanned: index,
11221216
},
11231217
);

scripts/check-i18n-coverage.mjs

Lines changed: 126 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,10 @@ import { randomUUID } from 'node:crypto';
151151
import {
152152
CLI,
153153
CLI_BUILD_FIX,
154+
closureBuildFix,
154155
looksLikeMissingCliCommand,
155156
oclifCommandFileFor,
157+
owningPackageOf,
156158
resolveCliCommandFile,
157159
} from './cli-build-prerequisite.mjs';
158160

@@ -186,15 +188,44 @@ const at = (rel) => join(REPO_ROOT, rel);
186188
/** The one command this gate invokes per config, as oclif topic/command parts. */
187189
const LINT_COMMAND_ID = ['lint'];
188190

191+
/**
192+
* This gate's WHOLE build prerequisite, named once (#12564) — the CLI plus the
193+
* build closure of every config in the population, derived from the population
194+
* itself rather than written down.
195+
*
196+
* Computed at module scope on purpose: it is one command for the ROUND, not one
197+
* per config, which is what keeps it eligible for `SHARED_REMEDIES` below. A
198+
* remedy narrowed to the configs that actually failed would have to reach a
199+
* classifier, and a classifier that can see the population is a classifier that
200+
* can put per-round detail in a cause's identity — the #11395 hole, re-opened
201+
* from the other side. Round-constant, so `groupFailuresByCause` keys exactly as
202+
* it did before.
203+
*/
204+
const POPULATION_CLOSURE = closureBuildFix([...discoverExamples(), ...discoverPackages()]);
205+
189206
/**
190207
* The remedy when a config's OWN workspace dependencies were never built (#6033) —
191208
* distinct from `CLI_BUILD_FIX`, which clears only the CLI. An example config
192209
* imports workspace packages by name, so a tree with just the CLI built still
193210
* cannot be linted, and the two remedies must not be confused for each other.
211+
*
212+
* ⭐ It used to say `pnpm build`, which is CORRECT and is not what a reader does
213+
* (#12564). Sitting directly under it is a `why:` line naming ONE package, and
214+
* that line is the specific, actionable-looking one — so a reader builds that
215+
* package, re-runs, and is told the next one, because node stops resolving a
216+
* config's imports at the first missing `dist/` (the mechanism is written out
217+
* over `closureBuildFix`). Measured: four locked build rounds to get one reading,
218+
* and the walk was six rounds long — the reporter escaped at four only by
219+
* abandoning the diagnosis and building an owning package's closure instead.
220+
* Naming the closure here is what makes the specific line and the remedy agree.
221+
*
222+
* Falls back to the coarser `pnpm build` when the closure cannot be derived: a
223+
* strict superset costs time, never coverage, and a PARTIAL closure would be this
224+
* card's own defect wearing a derivation's clothes (see `closureBuildFix`).
194225
*/
195-
const WORKSPACE_BUILD_FIX = 'pnpm build';
226+
const WORKSPACE_BUILD_FIX = POPULATION_CLOSURE.command ?? 'pnpm build';
196227
/** …and when the package is not on disk at all, a build alone cannot help. */
197-
const INSTALL_THEN_BUILD_FIX = 'pnpm install && pnpm build';
228+
const INSTALL_THEN_BUILD_FIX = `pnpm install && ${WORKSPACE_BUILD_FIX}`;
198229

199230
const update = process.argv.includes('--update');
200231

@@ -831,11 +862,24 @@ function selfTest() {
831862
// about them may move. Pinned as the literal remedy strings a reader acts on.
832863
expect(
833864
'#11395 the classified branches are untouched',
834-
explainConfigFailure(REAL_UNBUILT_DEP_ERROR).fix === 'pnpm build' &&
835-
explainConfigFailure("Cannot find package '@objectstack/nope' imported from /repo/x/y.ts").fix === 'pnpm install && pnpm build',
865+
explainConfigFailure(REAL_UNBUILT_DEP_ERROR).fix === WORKSPACE_BUILD_FIX &&
866+
explainConfigFailure("Cannot find package '@objectstack/nope' imported from /repo/x/y.ts").fix ===
867+
INSTALL_THEN_BUILD_FIX,
836868
'the two classified remedies are the text a reader runs; they must not move',
837869
);
838870

871+
// The two remedies are still DISTINCT and still say what they used to say
872+
// about each other: a package that is on disk but unbuilt needs a build, and
873+
// one that is not there at all needs an install FIRST. #12564 changed what the
874+
// build half spells, not which branch prescribes it.
875+
expect(
876+
'#12564 the two classified remedies stay distinct',
877+
WORKSPACE_BUILD_FIX !== INSTALL_THEN_BUILD_FIX &&
878+
INSTALL_THEN_BUILD_FIX === `pnpm install && ${WORKSPACE_BUILD_FIX}` &&
879+
WORKSPACE_BUILD_FIX !== CLI_BUILD_FIX,
880+
'the unbuilt / not-installed / CLI-only remedies must not collapse into one another',
881+
);
882+
839883
// -------------------------------------------------------------------------
840884
// Root anchoring and the population classifier (#10907). These are the only
841885
// assertions in this file that can fail over a CORRECT tree in a WRONG place,
@@ -885,6 +929,71 @@ function selfTest() {
885929
`absolute paths would silently re-key every baseline entry; got ${JSON.stringify(offRoot.slice(0, 2))}`,
886930
);
887931

932+
// -------------------------------------------------------------------------
933+
// The build-prerequisite CLOSURE (#12564). What makes the remedy worth naming
934+
// is that ONE round of it clears the whole population — so the property to pin
935+
// is COMPLETENESS, and the failure to refuse is a closure that names SOME of
936+
// the population. A partial closure is the worse half of this card's defect: it
937+
// looks derived, it is specific, and it still does not converge.
938+
// -------------------------------------------------------------------------
939+
const derivedClosure = closureBuildFix(onRoot);
940+
expect(
941+
'#12564 the live population yields a closure',
942+
typeof derivedClosure.command === 'string' && derivedClosure.command.length > 0,
943+
`no closure could be named for ${onRoot.length} config(s): ${derivedClosure.unknown ?? '(no reason given)'}`,
944+
);
945+
// Every config's OWN owner must be in the command. Compared per config against
946+
// the same manifests the derivation read, not against a list written here — a
947+
// list would be the hand-maintained note this derivation exists to avoid.
948+
const missingOwners = onRoot
949+
.map((configPath) => owningPackageOf(configPath))
950+
.filter((owner) => !owner || !(derivedClosure.command ?? '').includes(`--filter=${owner}`));
951+
expect(
952+
'#12564 …naming every config in the population',
953+
missingOwners.length === 0,
954+
`${missingOwners.length} owner(s) absent from the closure (${missingOwners.join(', ')}) — a closure that ` +
955+
'misses one config leaves the reader exactly the round-trip this remedy exists to remove',
956+
);
957+
// The CLI is in it too. Without that, the command printed under PREREQUISITE
958+
// NOT MET would not clear the prerequisite it is printed for — a remedy whose
959+
// success condition it cannot itself reach.
960+
expect(
961+
'#12564 …and the CLI the gate spawns',
962+
(derivedClosure.command ?? '').includes('--filter=@objectstack/cli'),
963+
'the closure must clear the CLI prerequisite it is offered as the remedy for',
964+
);
965+
// ⛔ ALL-OR-NOTHING. A broken scan is the sharp case: with the CLI always
966+
// seeded, an EMPTY population would render as `--filter=@objectstack/cli`
967+
// alone — byte-for-byte the CLI-only remedy this card exists to replace,
968+
// presented as though it were the whole closure. That is a guard whose
969+
// total-failure output is indistinguishable from a plausible success. One
970+
// unowned config is the same defect part-way. Both must come back as a REASON,
971+
// never a command — the rule `emptyPopulationVerdict` states for the verdict.
972+
const emptyClosure = closureBuildFix([]);
973+
const partialClosure = closureBuildFix([...onRoot, 'no/such/place/objectstack.config.ts']);
974+
expect(
975+
'#12564 an empty population names no closure',
976+
emptyClosure.command === undefined && typeof emptyClosure.unknown === 'string',
977+
`an empty population produced a command (${emptyClosure.command}) — a broken scan must not render as the ` +
978+
'CLI-only remedy wearing the closure\'s name',
979+
);
980+
expect(
981+
'#12564 …and one unowned config refuses the WHOLE closure',
982+
partialClosure.command === undefined && typeof partialClosure.unknown === 'string',
983+
'a closure missing one config is this card\'s own defect wearing a derivation\'s clothes',
984+
);
985+
// The refusals this gate is CREDITED for do not move (#12564 fence 2). The
986+
// remedy got longer; nothing about it may turn a refusal into a pass, so the
987+
// two headlines stay reachable and the partial round still declines to judge.
988+
expect(
989+
'#12564 the refusal semantics did not move',
990+
measureAllConfigs(['x.ts', 'y.ts'], (c) =>
991+
c === 'x.ts' ? { failure: explainConfigFailure(REAL_UNBUILT_DEP_ERROR) } : { count: 0 },
992+
).failures.length === 1,
993+
'a partial round must still collect a failure — a round that reports green over an unmeasured config is ' +
994+
'strictly worse than the four rounds this card is about',
995+
);
996+
888997
// The SHARED probe's own anchoring (#11394). #10907 anchored this FILE; the
889998
// module it asks "is the CLI built?" kept reading `packages/cli/package.json`
890999
// CWD-relatively, so from any other cwd the probe ENOENTed and deferred — and
@@ -974,7 +1083,9 @@ function selfTest() {
9741083
`✓ check:i18n-coverage --self-test — the missing-CLI-build, i18n-rule and per-config-failure classifiers all go red, ` +
9751084
`stay distinct, and a failing config does not end the round; all ${FAILURE_BRANCHES.length} failure branches state ` +
9761085
`one shared cause ONCE, with no config path in the key; the population resolves to ${onRoot.length} config(s) ` +
977-
`from outside the repo root as well as inside it, and an empty one is refused rather than reported OK.`,
1086+
`from outside the repo root as well as inside it, and an empty one is refused rather than reported OK; ` +
1087+
`the build-prerequisite closure names all ${onRoot.length} of them plus the CLI, and refuses whole rather ` +
1088+
`than naming some.`,
9781089
);
9791090
}
9801091

@@ -1009,9 +1120,16 @@ function reportPrerequisiteNotMet(headline, detail) {
10091120
console.error(
10101121
`\ncheck-i18n-coverage: PREREQUISITE NOT MET — ${headline}\n\n` +
10111122
detail.map((l) => (l ? ` ${l}` : '')).join('\n') +
1012-
`\n\n Fix: ${CLI_BUILD_FIX}\n` +
1013-
` …and on a tree that has never been built, \`pnpm build\`: this gate also\n` +
1014-
` lints \`examples/*\`, whose configs import other workspace packages.\n\n` +
1123+
`\n\n Fix: ${WORKSPACE_BUILD_FIX}\n\n` +
1124+
` That is this gate's WHOLE prerequisite in ONE command (#12564) — the CLI,\n` +
1125+
` plus the build closure of every config in its population. Clearing only\n` +
1126+
` the CLI (\`${CLI_BUILD_FIX}\`)\n` +
1127+
` moves the wall rather than removing it: this gate also lints \`examples/*\`,\n` +
1128+
` whose configs import workspace packages by name.\n` +
1129+
` ⛔ And clearing it one package at a time does NOT converge. node stops\n` +
1130+
` resolving a config's imports at the FIRST one with no \`dist/\`, so each\n` +
1131+
` round can name exactly one more, however many are missing — measured at\n` +
1132+
` six rounds down a single config's import list. Run the closure once.\n\n` +
10151133
` Nothing was measured: no config was linted and no count was compared, so this\n` +
10161134
` result says NOTHING about whether any declared label went untranslated — and\n` +
10171135
` the baseline was left exactly as committed (\`--update\` included).\n` +

0 commit comments

Comments
 (0)