Skip to content

Commit acd920d

Browse files
os-steveclaude
andauthored
fix(scripts): anchor check-i18n-bundles' population at the repo root (#11675)
* fix(scripts): anchor check-i18n-bundles' population at the repo root `findExtractConfigs`'s first parameter is its ABSOLUTE walk root — its own docstring says so, and the module's other consumer (scripts/pm/dispatch-gates.mjs) has always passed one. This gate passed the repo-relative vocabulary word for BOTH parameters, so the walk landed on `<cwd>/packages`: right by coincidence at the repo root, and from anywhere else an uncaught `ENOENT ... scandir 'packages'` with a `node:fs` stack. The anchor goes at the call site, not in the shared module: that module's contract is already correct and already honoured by its other caller, so moving the anchor into it would silently re-root a second consumer's population. The direction is LOUD, not silently green — the cost was a wrong first diagnosis, not a false pass. What makes it worth fixing is that the throw bypassed every worded channel this gate owns (#5217, #7681, reportPrerequisiteNotMet), which exist so an environment fact never reaches the reader as a content verdict. Anchoring a scan without refusing an empty result would trade the loud crash for a silent green, so the population verdict is a pure classifier splitting the two causes: an empty population is a prerequisite failure (#4690), while a --filter that matched nothing is a typo and must not describe a healthy repo as broken. Also anchors the reads the population feeds (the config docstring, the documented --out=) and the child extractor's cwd — all three resolve repo-relative argv, and without them the anchored walk would only move the failure one line down. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015ahemw8RcTgqtxrj15PEZx * chore(pm): record check:i18n's bare-root verdict as REFUSE-UNSPELLABLE Anchoring the population gave the bare literal a population-constant name (`PACKAGES_DIR`), which is what made it visible to the bare-root sweep. A bare single-segment word yields no watch hint, so the row is real and needed a verdict. REFUSE-UNSPELLABLE, not REFUSE-WIDE: the population is a filename-and-segment filter, 9 of 5093 tracked files under the root (0.18%), so a subtree declaration would be false rather than merely wide. That is the table's own definition of the two refusals, and it ties the check:i18n-coverage sibling row for the narrowest on the list — the two gates select the same nine configs by the same test, so they are refused alike. No narrower declaration is spellable, measured rather than assumed: collapseHint deletes glob segments, so every glob spelling of the real population reduces to a malformed double-separator prefix that hintCovers matches against nothing. Such a hint would be live and cover zero files, which is worse than the refusal. The miss is also smaller than the row: check:i18n already reaches the cards that can actually move a bundle through the convention triggers -- a package owning an extract config, and a metadata form module -- both verified live against the derivation. Data ledger only: 16 insertions, 0 deletions, no change to the recogniser, the sweep or the self-test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015ahemw8RcTgqtxrj15PEZx --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ffbb7a1 commit acd920d

2 files changed

Lines changed: 310 additions & 10 deletions

File tree

scripts/check-i18n-bundles.mjs

Lines changed: 294 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,147 @@ function passthroughStderrLines(text) {
237237
.filter((l) => l.trim() && !UNDECLARED_KEY_SIGNATURE.test(l));
238238
}
239239

240+
// ---------------------------------------------------------------------------
241+
// The POPULATION: what this gate is going to grade, and the two ways that
242+
// question can fail before a single bundle is compared (#11647).
243+
// ---------------------------------------------------------------------------
244+
245+
/**
246+
* The directory the population is walked from. Repo-relative ON PURPOSE — it is
247+
* the spelling every message here is written in, exactly as `CLI` and the
248+
* documented `--out=` are, and `atRepoRoot` is the one seam that turns it into a
249+
* path on disk. The same division cli-build-prerequisite.mjs states for its own
250+
* vocabulary (#11394), and check-i18n-coverage.mjs for its `at()` (#10907).
251+
*/
252+
const PACKAGES_DIR = 'packages';
253+
254+
/**
255+
* The repo root as a cwd for the child extractor. Taken from the SHARED seam
256+
* rather than derived again from `import.meta.url`: this file already imports
257+
* `atRepoRoot`, and a second derivation two lines from the first is the
258+
* duplication #11394 removed when it exported this one.
259+
*/
260+
const REPO_ROOT = atRepoRoot('.');
261+
262+
/**
263+
* A population problem is never fixed by building the CLI, so it must not
264+
* inherit `reportPrerequisiteNotMet`'s default `fix`. Prescribing a rebuild for
265+
* a missing `packages/` is the confident-wrong-diagnosis shape #5862 removed.
266+
*/
267+
const POPULATION_FIX = `check out this repository — this gate reads ${PACKAGES_DIR}/ from its own location, not from the cwd`;
268+
269+
/**
270+
* Every extract config in the repo, repo-relative and sorted — from ANY cwd.
271+
*
272+
* `findExtractConfigs`'s first parameter is its ABSOLUTE walk root; its own
273+
* docstring says so ("Every extract config under `absDir`"), and the module's
274+
* other consumer — scripts/pm/dispatch-gates.mjs — has always passed one
275+
* (`findExtractConfigs(join(ROOT, 'packages'), 'packages')`). This gate passed
276+
* the repo-relative vocabulary word for BOTH parameters, so the walk landed on
277+
* `<cwd>/packages`: at the repo root that is the right directory by
278+
* coincidence, and from anywhere else `readdirSync` threw an uncaught
279+
* `ENOENT … scandir 'packages'` with a `node:fs` stack (#11647).
280+
*
281+
* The anchor goes HERE and not in the shared module: the module's contract is
282+
* already correct and already honoured by its other caller, so moving the
283+
* anchor into it would silently re-root a second consumer's population — the
284+
* defect this gate family keeps paying for, in reverse.
285+
*
286+
* Note the direction, because it is not #10907's: that stack is LOUD. The cost
287+
* was a wrong first diagnosis (a reader sent to node's filesystem module), not
288+
* a false pass. What makes it worth fixing anyway is that it bypassed every
289+
* worded channel this gate owns — #5217, #7681 and `reportPrerequisiteNotMet`
290+
* exist precisely so an environment fact never reaches the reader as a content
291+
* verdict, and an uncaught throw reaches them as neither.
292+
*/
293+
function discoverExtractConfigs() {
294+
return findExtractConfigs(atRepoRoot(PACKAGES_DIR), PACKAGES_DIR)
295+
.map((c) => c.rel)
296+
.sort();
297+
}
298+
299+
/**
300+
* The detail for a walk that threw. Pure, so `--self-test` can pin the one
301+
* property that matters: it says the checkout is broken, NOT that the reader
302+
* stood in the wrong place — post-#11647 the cwd cannot cause this.
303+
*/
304+
function unreadablePopulationDetail(err) {
305+
const message = String(err?.message ?? err);
306+
return [
307+
`Walking for extract configs failed before any bundle was compared:`,
308+
``,
309+
` ${message.length > 160 ? `${message.slice(0, 160)}…` : message}`,
310+
``,
311+
`This gate resolves \`${PACKAGES_DIR}/\` against its OWN location rather than the cwd, so`,
312+
`this is not a "run it from the repo root" problem — the directory is missing or`,
313+
`unreadable in the checkout this script lives in:`,
314+
``,
315+
` ${atRepoRoot(PACKAGES_DIR)}`,
316+
];
317+
}
318+
319+
/**
320+
* Why the population is unusable, or `null`. Pure over an already-walked list,
321+
* so `--self-test` drives both directions — the shape every other classifier in
322+
* this file has.
323+
*
324+
* TWO causes, and only ONE of them is a prerequisite. Keeping them apart is the
325+
* whole of this function:
326+
*
327+
* - an EMPTY POPULATION is an environment fact, and refusing it is #4690's
328+
* rule: a scan that found nothing must never render as a pass. #10907 had
329+
* to re-learn that one file over, where an unanchored walk came back empty
330+
* and the gate printed `OK (0 config(s))` and exited 0. This gate has
331+
* always exited 1 here, so anchoring the walk did not introduce the guard —
332+
* but anchoring it WITHOUT this check would have converted #11647's loud
333+
* crash into exactly that silent green, which is strictly worse than the
334+
* bug being fixed.
335+
*
336+
* No legitimate tree of this repo has zero: `lint.yml` runs this gate
337+
* because packages here ship translation bundles, and a checkout with none
338+
* is not one this gate can grade. So the condition is the plain `=== 0`,
339+
* and it is stated here rather than assumed.
340+
*
341+
* - a FILTER that matched nothing is a typo in an argument the developer just
342+
* typed. It is not an environment fact: it must not borrow the "nothing was
343+
* checked" apparatus, must not prescribe a rebuild, and must not describe a
344+
* repository that is fine as broken. Before #11647 both causes shared one
345+
* sentence, so `--filter=platform_objects` for `platform-objects` read as a
346+
* repo with no i18n configs at all.
347+
*/
348+
function populationVerdict(population, activeFilter) {
349+
if (population.length === 0) {
350+
return {
351+
prerequisite: true,
352+
headline: `this gate has no population — no extract config exists under \`${PACKAGES_DIR}/\``,
353+
detail: [
354+
`The walk reached \`${PACKAGES_DIR}/\` and came back empty, so there is nothing to compare.`,
355+
`Every package that ships a translation bundle documents its extract in`,
356+
`\`${PACKAGES_DIR}/<pkg>/scripts/i18n-extract.config.ts\`, and CI runs this gate because some do.`,
357+
``,
358+
`Walked: ${atRepoRoot(PACKAGES_DIR)}`,
359+
``,
360+
`Reported as a prerequisite rather than as a pass on purpose (#4690): an empty`,
361+
`scan rendered as OK is a gate that has stopped grading without saying so.`,
362+
],
363+
};
364+
}
365+
if (activeFilter && !population.some((c) => c.includes(activeFilter))) {
366+
return {
367+
prerequisite: false,
368+
headline: `--filter=${activeFilter} matched none of the ${population.length} extract config(s)`,
369+
detail: [
370+
`The repository is fine and the population was found — this is the filter, not`,
371+
`the tree. \`--filter\` is a plain substring test against these repo-relative`,
372+
`paths, and none of them contain that text:`,
373+
``,
374+
...population.map((c) => ` ${c}`),
375+
],
376+
};
377+
}
378+
return null;
379+
}
380+
240381
// ---------------------------------------------------------------------------
241382
// Self-test — the proof that each classifier can go red, and that the two
242383
// verdicts do not contaminate each other.
@@ -592,14 +733,128 @@ function selfTest() {
592733
).join('\n');
593734
expect('#7681 long evidence is truncated', longSentence.includes(`${'S'.repeat(160)}…`), 'a 400-char sentence must not be pasted whole');
594735

736+
// -------------------------------------------------------------------------
737+
// Fifth classifier (#11647): the POPULATION — is there anything to grade, and
738+
// did the walk for it land on this repo or on the caller's cwd?
739+
//
740+
// These are the only assertions here that can fail over a CORRECT tree in a
741+
// WRONG place, which is exactly why they are worth their cost: every other
742+
// classifier in this file is proven red against a recorded string, but "did
743+
// this gate look at anything at all?" can only be proven by looking.
744+
// -------------------------------------------------------------------------
745+
746+
const popCwdBefore = process.cwd();
747+
let offRootPopulation;
748+
let bareWalkOffRoot;
749+
try {
750+
process.chdir(tmpdir());
751+
offRootPopulation = discoverExtractConfigs();
752+
// POSITIVE CONTROL for the assertion below. Without it, "the anchored walk
753+
// works off-root" is compatible with a cwd that happened to contain a
754+
// `packages/` — and on a tree where the bare spelling ALSO resolved, the
755+
// anchoring assertion would be proving nothing. This records what the
756+
// pre-#11647 line 748 did from here: throw.
757+
try {
758+
findExtractConfigs(PACKAGES_DIR, PACKAGES_DIR);
759+
bareWalkOffRoot = 'resolved';
760+
} catch (err) {
761+
bareWalkOffRoot = err.code ?? 'threw';
762+
}
763+
} finally {
764+
process.chdir(popCwdBefore);
765+
}
766+
const onRootPopulation = discoverExtractConfigs();
767+
768+
expect(
769+
'#11647 the population is CWD-independent',
770+
offRootPopulation.length > 0,
771+
`the walk from ${tmpdir()} found ${offRootPopulation.length} config(s) — the population is still ` +
772+
"resolved CWD-relatively, which is the defect: off-root the gate cannot start at all",
773+
);
774+
expect(
775+
'#11647 …and finds exactly the population the root does',
776+
offRootPopulation.join('\n') === onRootPopulation.join('\n'),
777+
`off-root found ${offRootPopulation.length} config(s), on-root ${onRootPopulation.length} — ` +
778+
'anchoring must not change WHAT is scanned',
779+
);
780+
expect(
781+
'#11647 …spelled repo-relative, as every message and the child argv are',
782+
offRootPopulation.every((c) => !c.startsWith('/') && !c.includes(REPO_ROOT)),
783+
`absolute paths would leak into the rerun command and the extractor argv; got ${JSON.stringify(offRootPopulation.slice(0, 2))}`,
784+
);
785+
expect(
786+
'#11647 …and the bare spelling demonstrably would not have',
787+
bareWalkOffRoot === 'ENOENT',
788+
`the pre-fix spelling did not fail from ${tmpdir()} (got ${bareWalkOffRoot}), so the assertions above ` +
789+
'prove nothing about anchoring',
790+
);
791+
792+
// #4690, carried over from #10907: an empty population is a REFUSAL. Anchoring
793+
// a scan without this trades a loud crash for a silent green.
794+
const emptyVerdict = populationVerdict([], '');
795+
expect('#4690 an empty population is refused', !!emptyVerdict && emptyVerdict.prerequisite === true, `got ${JSON.stringify(emptyVerdict)}`);
796+
expect(
797+
'#4690 …through the WORDED channel, not as a pass',
798+
!!emptyVerdict && /no population|came back empty/.test(`${emptyVerdict.headline}\n${emptyVerdict.detail.join('\n')}`),
799+
'the refusal has to say in words that nothing was gradeable',
800+
);
801+
expect(
802+
'#4690 …and does not prescribe the CLI build, which would change nothing',
803+
POPULATION_FIX !== CLI_BUILD_FIX,
804+
'a population problem is not fixed by rebuilding the CLI',
805+
);
806+
807+
// The other cause, and the reason `=== 0` alone is not the whole condition: a
808+
// filter that matched nothing is a typo, not an environment fact.
809+
const filterVerdict = populationVerdict(onRootPopulation, 'no-such-package');
810+
expect('#11647 an unmatched --filter is refused', !!filterVerdict, 'a filter matching nothing must not render as OK (0 package(s))');
811+
expect(
812+
'#11647 …but NOT as a prerequisite',
813+
!!filterVerdict && filterVerdict.prerequisite === false,
814+
'a typo in an argument the developer just typed is not an environment fact, and must not print "nothing was checked"',
815+
);
816+
expect(
817+
'#11647 …and the two population verdicts do not contaminate each other',
818+
!!emptyVerdict &&
819+
!!filterVerdict &&
820+
!emptyVerdict.headline.includes('--filter') &&
821+
!filterVerdict.detail.join('\n').includes('came back empty') &&
822+
filterVerdict.detail.join('\n').includes('The repository is fine'),
823+
'each cause must name itself: a broken checkout and a mistyped filter send the reader to different places',
824+
);
825+
expect(
826+
'#11647 a healthy population yields no verdict at all',
827+
populationVerdict(onRootPopulation, '') === null && populationVerdict(onRootPopulation, 'platform-objects') === null,
828+
'the classifier must be silent over the tree CI actually runs on',
829+
);
830+
831+
// The walk-threw path: it must blame the CHECKOUT, never the caller's cwd —
832+
// post-#11647 the cwd cannot cause it, so a message that says "run from the
833+
// repo root" would send the reader somewhere that changes nothing.
834+
const unreadable = unreadablePopulationDetail(Object.assign(new Error("ENOENT: no such file or directory, scandir 'packages'"), { code: 'ENOENT' })).join('\n');
835+
expect('#11647 the unreadable-walk detail carries the evidence', unreadable.includes("scandir 'packages'"), `a conclusion with no reading under it is not auditable; got ${JSON.stringify(unreadable)}`);
836+
expect(
837+
'#11647 …names the tree it actually walked',
838+
unreadable.includes(atRepoRoot(PACKAGES_DIR)),
839+
'the reader has to be told WHICH packages/ was missing, not merely that one was',
840+
);
841+
expect(
842+
'#11647 …and does not blame the cwd',
843+
/not a "run it from the repo root" problem/.test(unreadable),
844+
'after the anchor the cwd cannot cause this, and a wrong remedy costs the reader the diagnosis again',
845+
);
846+
const longWalkError = unreadablePopulationDetail(new Error('E'.repeat(400))).join('\n');
847+
expect('#11647 long walk errors are truncated', longWalkError.includes(`${'E'.repeat(160)}…`), 'a 400-char message must not be pasted whole');
848+
595849
if (failures.length) {
596850
console.error(`✗ check:i18n --self-test — ${failures.length} failure(s)\n`);
597851
for (const f of failures) console.error(` ${f}`);
598852
process.exit(1);
599853
}
600854
console.log(
601-
'✓ check:i18n --self-test — bundle-drift, undeclared-authoring-key, missing-CLI-build and ' +
602-
'stale-workspace-dist classifiers all go red, and stay distinct.',
855+
'✓ check:i18n --self-test — bundle-drift, undeclared-authoring-key, missing-CLI-build, ' +
856+
'stale-workspace-dist and empty-population classifiers all go red, and stay distinct; ' +
857+
'the population walk is CWD-independent.',
603858
);
604859
}
605860

@@ -745,29 +1000,51 @@ function checkCliBuildPrerequisite() {
7451000

7461001
checkCliBuildPrerequisite();
7471002

748-
const configs = findExtractConfigs('packages', 'packages')
749-
.map((c) => c.rel)
750-
.sort()
751-
.filter((c) => !filter || c.includes(filter));
752-
if (configs.length === 0) {
753-
console.error(`check-i18n-bundles: no extract configs matched${filter ? ` --filter=${filter}` : ''}`);
1003+
let population;
1004+
try {
1005+
population = discoverExtractConfigs();
1006+
} catch (err) {
1007+
// Before #11647 this threw straight out of the module and printed a `node:fs`
1008+
// stack. It is a worded verdict now, through the same channel the other two
1009+
// prerequisites already use.
1010+
reportPrerequisiteNotMet(`this gate's population could not be enumerated`, unreadablePopulationDetail(err), {
1011+
fix: POPULATION_FIX,
1012+
});
1013+
}
1014+
1015+
const populationProblem = populationVerdict(population, filter);
1016+
if (populationProblem?.prerequisite) {
1017+
reportPrerequisiteNotMet(populationProblem.headline, populationProblem.detail, { fix: POPULATION_FIX });
1018+
}
1019+
if (populationProblem) {
1020+
// NOT a prerequisite: the repo is fine and the population was found, so this
1021+
// must not borrow the "nothing was checked" apparatus or prescribe a rebuild.
1022+
console.error(`\ncheck-i18n-bundles: ${populationProblem.headline}\n`);
1023+
for (const line of populationProblem.detail) console.error(line ? ` ${line}` : '');
7541024
process.exit(1);
7551025
}
7561026

1027+
const configs = population.filter((c) => !filter || c.includes(filter));
1028+
7571029
const drifted = [];
7581030
const broken = [];
7591031
/** One entry per package that authored a key the schema does not declare. */
7601032
const undeclared = [];
7611033
for (const [index, config] of configs.entries()) {
7621034
const pkg = config.replace(/^packages\//, '').replace(/\/scripts\/i18n-extract\.config\.ts$/, '');
763-
const flags = flagsFromDocstring(config);
1035+
// `config` is repo-relative VOCABULARY — it is what every message below and the
1036+
// rerun command print, and what the child is handed as argv. The READ of it
1037+
// goes through the one seam (#11647).
1038+
const flags = flagsFromDocstring(atRepoRoot(config));
7641039
const out = flags.find((f) => f.startsWith('--out='));
7651040
if (!out) {
7661041
broken.push(`${pkg}: its docstring documents no --out=<dir>, so the gate cannot tell where the bundles live`);
7671042
continue;
7681043
}
7691044
const outDir = out.slice('--out='.length);
770-
if (!existsSync(outDir)) {
1045+
// Same seam: `--out=` is documented repo-relative (`--out=packages/<pkg>/src/...`),
1046+
// so asking the filesystem about it unanchored asks about the cwd (#11647).
1047+
if (!existsSync(atRepoRoot(outDir))) {
7711048
broken.push(`${pkg}: documented --out directory does not exist: ${outDir}`);
7721049
continue;
7731050
}
@@ -782,6 +1059,13 @@ for (const [index, config] of configs.entries()) {
7821059
const run = spawnSync(process.execPath, args, {
7831060
encoding: 'utf8',
7841061
maxBuffer: 64 * 1024 * 1024,
1062+
// `CLI`, `config` and the documented `--out=` are ALL repo-relative, so the
1063+
// child's cwd is what resolves them — anchoring it is what makes an off-root
1064+
// run extract the real bundles instead of failing nine times with an
1065+
// environment fact dressed as `N bundle problem(s)` (#11647). The same line,
1066+
// for the same reason, that check-i18n-coverage.mjs carries over its own
1067+
// spawn (#10907).
1068+
cwd: REPO_ROOT,
7851069
});
7861070
if (run.error) {
7871071
broken.push(`${pkg}: could not run the extractor — ${run.error.message}`);

scripts/pm/bare-root-worklist.mjs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,22 @@ const TRIAGE = new Map([
219219
+ 'for #10907 gave the literal a population-constant name, so this row records a population '
220220
+ 'that was previously unnameable rather than one the fix introduced',
221221
}],
222+
['check:i18n PACKAGES_DIR packages', {
223+
verdict: 'REFUSE-UNSPELLABLE',
224+
why: 'files named i18n-extract.config.ts beneath a scripts segment — 9 of 5093 (0.18%), tying '
225+
+ 'its check:i18n-coverage sibling above for the narrowest row on this list. The two gates '
226+
+ 'select the same nine configs by the same filename-and-segment test, so they are refused '
227+
+ 'alike: a subtree hint would name this gate for 5093 files to reach 9. Nothing narrower is '
228+
+ 'spellable, measured rather than assumed — every glob spelling of the real population '
229+
+ 'collapses to a malformed double-separator prefix that hintCovers matches against NOTHING, '
230+
+ 'so a narrow declaration would not be a precise hint but a live hint covering zero files. '
231+
+ 'The miss is also smaller than the row: this gate already reaches the cards that can '
232+
+ 'actually move a bundle through the convention triggers (a package that owns an extract '
233+
+ 'config, and a metadata form module), both verified live, and a wholesale root declaration '
234+
+ 'would drown that precision rather than add to it. The root reached the sweep only once the '
235+
+ 'fix for #11647 gave the literal a population-constant name, so this row records a '
236+
+ 'population that was previously unnameable rather than one the fix introduced',
237+
}],
222238
['scripts/check-skills-token-ratchet.mjs SKILLS_DIR skills', {
223239
verdict: 'REFUSE-UNSPELLABLE',
224240
why: 'one named file per child directory, 11 of 50 (22%). It already reaches its own cards '

0 commit comments

Comments
 (0)