Skip to content

Commit 35c1ca3

Browse files
claude[bot]claude
andauthored
fix(scripts): anchor check-i18n-coverage to a module-derived root and refuse an empty population (#11397)
* fix(scripts): anchor check-i18n-coverage to a module-derived root and refuse an empty population `scripts/check-i18n-coverage.mjs` resolved every path CWD-relatively — `examples`, `packages`, the baseline, and the CLI stub it spawns. Run from anywhere but the repo root it discovered no configs, compared nothing, and printed `OK (0 config(s), 0 baselined untranslated string(s), none new)` with exit 0 — the same sentence and the same exit code a real pass uses. What made that silent rather than merely wrong is an interlock: this is a two-sided ratchet, so a config that vanishes is normally caught by the DOWN direction. But the population and the baseline were resolved the same way, so a wrong root emptied both together and left the comparison with nothing to disagree about. Two halves: 1. Every read is anchored to a root derived from `import.meta.url`, as `check-skills-token-ratchet.mjs` and `check-ratchet-remedy-authority.mjs` do. The repo-relative spellings stay — they are the committed baseline's KEYS and the text a reader acts on — and `at()` is the one seam between the two. The `os lint` spawn gets `cwd: REPO_ROOT`, which is what resolves the repo-relative `CLI` and config paths it is handed. 2. An empty population is refused rather than returned: zero is a broken scan, not a repo with nothing to translate, the rule `trackedFiles` states in `scripts/pm/dispatch-gates.mjs`. Judged on the union, not per half, so it cannot preempt the legitimate ratchet-DOWN path when a single config is retired. It is placed before any CLI is spawned and before `--update` can write, which over an empty population would have discarded all twelve baselined entries. The self-test gains the non-vacuity proof this gate could not previously give: it chdirs out of the repo and asserts discovery still resolves the same 12 configs, spelled repo-relative, and pins the empty-population classifier red on nothing and silent on real work. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015ahemw8RcTgqtxrj15PEZx * fix(pm): record the bare-root verdict PACKAGES_DIR owes the invisible-species worklist `node scripts/pm/bare-root-worklist.mjs --self-test` went red on f5fe23b: FRESH: check:i18n-coverage PACKAGES_DIR packages. Naming the previously anonymous `'packages'` default parameter `PACKAGES_DIR` made it match `POPULATION_CONSTANT`, so the sweep saw it for the first time and demanded a verdict. The invisibility is not new — the literal was always a bare single-segment word the dispatch derivation cannot build a hint from — but it was previously unnameable by the sweep too, so nothing recorded it. The row now records a population that was already there. Verdict: REFUSE-UNSPELLABLE, measured. `discoverPackages` admits files named `i18n-extract.config.ts` beneath a `scripts` segment — 9 of 5035 tracked files under the root (0.18%), the narrowest row on the list. That is a FILENAME filter, and `collapseHint` can only ever name a whole subtree, so the sole spellable declaration would name this gate for 5035 files to reach 9 — the costlier error `hintCovers` prices, and the same shape its `EXAMPLES_DIR examples` sibling was already refused for at 1.3%. Also records the refusal beside the constants themselves, as `check-driver-conformance.mjs` and `check-examples-live-imports.mjs` do, so the next reader meets the reasoning at the population rather than only in the triage. The gate is not weakened, narrowed or skipped, and the scope expansion that surfaced the row stands. Executable content of `check-i18n-coverage.mjs` is byte-identical to f5fe23b with comments masked and blank lines removed (23418 == 23418); the only non-comment change is one added blank line. 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 29d0676 commit 35c1ca3

2 files changed

Lines changed: 233 additions & 16 deletions

File tree

scripts/check-i18n-coverage.mjs

Lines changed: 224 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,37 @@
8989
// converse, and one discipline serves both: measure EVERY config, then report every
9090
// DISTINCT cause once, with the configs it covers. A config that cannot be linted is
9191
// now collected (`measureI18nIssues` returns a failure), never thrown.
92+
//
93+
// Both of those are about a config that could not be MEASURED. #10907 is about
94+
// the population itself, and it is the failure this gate could not report at all:
95+
// every path here was resolved CWD-RELATIVELY — `examples`, `packages`, the
96+
// baseline, the CLI stub — so run from anywhere but the repo root the gate found
97+
// no configs, compared nothing, and printed
98+
//
99+
// check-i18n-coverage: OK (0 config(s), 0 baselined untranslated string(s), none new).
100+
// exit 0
101+
//
102+
// — the same sentence, and the same exit code, a real pass uses. What makes that
103+
// silent rather than merely wrong is an INTERLOCK: this is a two-sided ratchet, so
104+
// a config that vanishes is normally caught by the DOWN direction ("baselined
105+
// config is gone"). But the population and the baseline were resolved the same
106+
// way, so a wrong root emptied them TOGETHER and left the comparison with nothing
107+
// to disagree about. Anchoring one side alone would not have been a fix — it would
108+
// have turned the silence into twelve spurious errors.
109+
//
110+
// Two halves, and the file keeps both. (1) Every read is anchored to a root
111+
// derived from `import.meta.url`, as `check-skills-token-ratchet.mjs` and
112+
// `check-ratchet-remedy-authority.mjs` do; the repo-relative spellings stay, since
113+
// they are the committed baseline's KEYS and the text a reader acts on, and `at()`
114+
// is the one seam between the two. (2) An empty population is REFUSED rather than
115+
// returned — zero is a broken scan, not a repo with nothing to translate (#4690),
116+
// the same rule `trackedFiles` states in `scripts/pm/dispatch-gates.mjs`. Half 1
117+
// makes the off-root run correct; half 2 is what keeps "green over nothing"
118+
// unreachable by the routes half 1 does not know about.
92119
import { execFileSync } from 'node:child_process';
93120
import { readdirSync, readFileSync, writeFileSync, existsSync, openSync, closeSync, unlinkSync } from 'node:fs';
94-
import { join } from 'node:path';
121+
import { dirname, join, resolve } from 'node:path';
122+
import { fileURLToPath } from 'node:url';
95123
import { tmpdir } from 'node:os';
96124
import { randomUUID } from 'node:crypto';
97125
import {
@@ -102,9 +130,33 @@ import {
102130
resolveCliCommandFile,
103131
} from './cli-build-prerequisite.mjs';
104132

133+
const HERE = dirname(fileURLToPath(import.meta.url));
134+
/** This script lives in `scripts/`, so the repo root is one level up (#10907). */
135+
const REPO_ROOT = resolve(HERE, '..');
136+
137+
// Repo-relative ON PURPOSE — these spellings are the committed baseline's KEYS,
138+
// the paths in every error message, and the commands `rerunFix` tells a reader to
139+
// run. Making them absolute would silently re-key all twelve baseline entries.
140+
// `at()` below is the ONE seam that turns a repo-relative path into a path on
141+
// disk, so the vocabulary stays relative while every READ is anchored (#10907).
105142
// NOTE: covers both `examples/*` and every package with an extract config.
106143
const EXAMPLES_DIR = 'examples';
144+
const PACKAGES_DIR = 'packages';
107145
const BASELINE_PATH = 'scripts/i18n-coverage-baseline.json';
146+
147+
// ⛔ Neither root above is declared to the dispatch derivation, and that is a
148+
// recorded REFUSAL rather than an omission. Both populations are FILENAME
149+
// filters — one `objectstack.config.ts` per example directory (3 of 240), and
150+
// files named `i18n-extract.config.ts` beneath a `scripts` segment (9 of 5035).
151+
// The `ROOT_DIR_WATCH_HINTS` idiom can only name a whole subtree, so the only
152+
// spellable claim here would name this gate for 5035 files to reach 9 — the
153+
// costlier error, per `hintCovers`. Both verdicts are recorded as
154+
// REFUSE-UNSPELLABLE in the triage that `scripts/pm/bare-root-worklist.mjs`
155+
// self-tests on every PR; giving `PACKAGES_DIR` a population-constant name is
156+
// what made this root visible to that sweep at all.
157+
158+
/** A repo-relative path, resolved against the module-derived root. */
159+
const at = (rel) => join(REPO_ROOT, rel);
108160
/** The one command this gate invokes per config, as oclif topic/command parts. */
109161
const LINT_COMMAND_ID = ['lint'];
110162

@@ -120,13 +172,13 @@ const INSTALL_THEN_BUILD_FIX = 'pnpm install && pnpm build';
120172

121173
const update = process.argv.includes('--update');
122174

123-
/** Every bundled example that has a stack config. */
175+
/** Every bundled example that has a stack config. Root-anchored, repo-relative out. */
124176
function discoverExamples() {
125-
if (!existsSync(EXAMPLES_DIR)) return [];
126-
return readdirSync(EXAMPLES_DIR, { withFileTypes: true })
177+
if (!existsSync(at(EXAMPLES_DIR))) return [];
178+
return readdirSync(at(EXAMPLES_DIR), { withFileTypes: true })
127179
.filter((e) => e.isDirectory())
128180
.map((e) => join(EXAMPLES_DIR, e.name, 'objectstack.config.ts'))
129-
.filter((p) => existsSync(p))
181+
.filter((p) => existsSync(at(p)))
130182
.sort();
131183
}
132184

@@ -139,9 +191,9 @@ function discoverExamples() {
139191
* Studio. Covering only `examples/` is how `platform-objects` sat on 77
140192
* untranslated navigation and widget labels per locale without anything saying so.
141193
*/
142-
function discoverPackages(dir = 'packages', out = []) {
143-
if (!existsSync(dir)) return out;
144-
for (const e of readdirSync(dir, { withFileTypes: true })) {
194+
function discoverPackages(dir = PACKAGES_DIR, out = []) {
195+
if (!existsSync(at(dir))) return out;
196+
for (const e of readdirSync(at(dir), { withFileTypes: true })) {
145197
if (e.name === 'node_modules' || e.name === 'dist' || e.name.startsWith('.')) continue;
146198
const p = join(dir, e.name);
147199
if (e.isDirectory()) discoverPackages(p, out);
@@ -241,6 +293,47 @@ function packageNameFromSpecifier(specifier) {
241293
return name.endsWith('.mjs') || name.endsWith('.js') || name.endsWith('.ts') ? '' : name;
242294
}
243295

296+
/**
297+
* The POPULATION classifier (#10907): is there anything to judge at all?
298+
*
299+
* Pure — list in, verdict out — so `--self-test` drives both directions. That
300+
* matters more here than for any other classifier in this file: an empty
301+
* population is the one failure that RENDERS AS A PASS, so a gate that had only
302+
* ever observed its own green path could not tell the two apart. That is #4690
303+
* exactly, and `trackedFiles` in `scripts/pm/dispatch-gates.mjs` refuses an empty
304+
* listing on the same grounds — "zero is a broken scan, not a clean repo".
305+
*
306+
* Judged on the UNION, deliberately, not per half. A single vanished config is
307+
* already this gate's business: the two-sided ratchet reports it as a DOWN and
308+
* tells the reader to `--update`. Refusing on an empty HALF would preempt that
309+
* legitimate path — the last example being retired is a real event, not a broken
310+
* scan. Only a total wipe is indistinguishable from a scan that read nothing, and
311+
* only a total wipe is refused here.
312+
*
313+
* @param {string[]} configPaths the discovered population, repo-relative
314+
* @returns {{ headline: string, detail: string[] } | null} null when there is work to do
315+
*/
316+
function emptyPopulationVerdict(configPaths) {
317+
if (configPaths.length > 0) return null;
318+
return {
319+
headline: 'the config population came back EMPTY — there is nothing to measure',
320+
detail: [
321+
`Neither \`${EXAMPLES_DIR}/*/objectstack.config.ts\` nor any`,
322+
`\`${PACKAGES_DIR}/**/scripts/i18n-extract.config.ts\` was found under the root this`,
323+
`script derives from its own location:`,
324+
``,
325+
` ${REPO_ROOT}`,
326+
``,
327+
`Zero is a broken scan, not a repo with nothing to translate (#4690). Note what`,
328+
`an empty population costs THIS gate specifically: it is a two-sided ratchet, so`,
329+
`a config that really did vanish is caught by the DOWN direction — but the`,
330+
`population and the baseline are read the same way, so a tree this script cannot`,
331+
`read empties BOTH, leaving the comparison with nothing to disagree about and a`,
332+
`verdict identical to a real pass. Refusing rather than reporting OK over nothing.`,
333+
],
334+
};
335+
}
336+
244337
/**
245338
* Distinct CAUSES, each carrying the configs it explains. Keyed on the CONCLUSION
246339
* (reason + fix) rather than the raw reading, because one unbuilt package produces
@@ -284,7 +377,13 @@ function measureI18nIssues(configPath) {
284377
try {
285378
let stderr = '';
286379
try {
380+
// `CLI` and `configPath` are both REPO-RELATIVE, so the child's cwd is what
381+
// resolves them — anchoring it here is what makes an off-root run measure the
382+
// real population instead of failing twelve times (#10907). It also keeps
383+
// `os lint`'s own workspace resolution pointed at the repo, not at wherever
384+
// the caller happened to stand.
287385
execFileSync(process.execPath, [CLI, 'lint', configPath, '--json'], {
386+
cwd: REPO_ROOT,
288387
stdio: ['ignore', fd, 'pipe'],
289388
});
290389
} catch (err) {
@@ -553,12 +652,84 @@ function selfTest() {
553652
expect('#6033 one cause is stated once', sharedCause.length === 1, `got ${sharedCause.length} cause(s) for one missing package`);
554653
expect('#6033 …carrying every config it covers', sharedCause[0]?.configPaths.length === 3, `got ${JSON.stringify(sharedCause[0]?.configPaths)}`);
555654

655+
// -------------------------------------------------------------------------
656+
// Root anchoring and the population classifier (#10907). These are the only
657+
// assertions in this file that can fail over a CORRECT tree in a WRONG place,
658+
// and that is the whole point: every other classifier here is proven red with a
659+
// recorded string, but "did this gate look at anything at all?" can only be
660+
// proven by looking.
661+
// -------------------------------------------------------------------------
662+
663+
// The derivation must land on THIS repo's root — one level off would still find
664+
// a `scripts/` directory, so pin files only the root has, this gate's own two
665+
// included.
666+
expect(
667+
'#10907 derives the repo root',
668+
existsSync(at('package.json')) && existsSync(at(BASELINE_PATH)) && existsSync(at('scripts/check-i18n-coverage.mjs')),
669+
`REPO_ROOT does not look like this repo's root: ${REPO_ROOT}`,
670+
);
671+
672+
// The anchoring itself, proven the only way that means anything: from a cwd that
673+
// is NOT the repo root. This single assertion is the #10907 defect — before the
674+
// fix both discoveries read a bare `examples` / `packages`, came back empty from
675+
// anywhere else, and the gate printed `OK (0 config(s))` and exited 0. cwd is
676+
// restored in a `finally`: it is process-global state, and a self-test that
677+
// leaves it moved would corrupt every measurement after it.
678+
const cwdBefore = process.cwd();
679+
let offRoot;
680+
try {
681+
process.chdir(tmpdir());
682+
offRoot = [...discoverExamples(), ...discoverPackages()];
683+
} finally {
684+
process.chdir(cwdBefore);
685+
}
686+
const onRoot = [...discoverExamples(), ...discoverPackages()];
687+
expect(
688+
'#10907 discovery is CWD-independent',
689+
offRoot.length > 0,
690+
`discovery from ${tmpdir()} found ${offRoot.length} config(s) — the population is still resolved CWD-relatively, ` +
691+
'which is the whole defect: an empty population renders as a pass',
692+
);
693+
expect(
694+
'#10907 …and finds exactly the population the root does',
695+
offRoot.join('\n') === onRoot.join('\n'),
696+
`off-root found ${offRoot.length} config(s), on-root ${onRoot.length} — anchoring must not change WHAT is scanned`,
697+
);
698+
expect(
699+
'#10907 …spelled repo-relative, as the baseline keys are',
700+
offRoot.every((p) => !p.startsWith('/') && !p.includes(REPO_ROOT)),
701+
`absolute paths would silently re-key every baseline entry; got ${JSON.stringify(offRoot.slice(0, 2))}`,
702+
);
703+
704+
// Anti-#4690 on the population itself. Red on nothing is the assertion that
705+
// matters — this is the one verdict whose failure mode is a green line.
706+
expect('#10907 an empty population is refused', !!emptyPopulationVerdict([]), 'zero configs must never be a pass');
707+
expect(
708+
'#10907 …with a complete verdict',
709+
!!emptyPopulationVerdict([])?.headline && (emptyPopulationVerdict([])?.detail?.length ?? 0) > 0,
710+
`a refusal with no reading under it is not auditable; got ${JSON.stringify(emptyPopulationVerdict([]))}`,
711+
);
712+
expect(
713+
'#10907 a real population is not refused',
714+
emptyPopulationVerdict(['examples/app-crm/objectstack.config.ts']) === null,
715+
'one config is work to do, not a broken scan — refusing it would preempt the ratchet-DOWN path',
716+
);
717+
expect(
718+
'#10907 the live population is not refused',
719+
emptyPopulationVerdict(onRoot) === null,
720+
`the real tree resolved to ${onRoot.length} config(s) and must be judged, not refused`,
721+
);
722+
556723
if (failures.length) {
557724
console.error(`✗ check:i18n-coverage --self-test — ${failures.length} failure(s)\n`);
558725
for (const f of failures) console.error(` ${f}`);
559726
process.exit(1);
560727
}
561-
console.log('✓ check:i18n-coverage --self-test — the missing-CLI-build, i18n-rule and per-config-failure classifiers all go red, stay distinct, and a failing config does not end the round.');
728+
console.log(
729+
`✓ check:i18n-coverage --self-test — the missing-CLI-build, i18n-rule and per-config-failure classifiers all go red, ` +
730+
`stay distinct, and a failing config does not end the round; the population resolves to ${onRoot.length} config(s) ` +
731+
`from outside the repo root as well as inside it, and an empty one is refused rather than reported OK.`,
732+
);
562733
}
563734

564735
if (process.argv.includes('--self-test')) {
@@ -654,6 +825,35 @@ function reportUnmeasuredConfigs(failures, measuredCount) {
654825
process.exit(1);
655826
}
656827

828+
/**
829+
* The refusal for an empty population (#10907) — reached before a single CLI is
830+
* spawned and before `--update` can write, which is the point on both counts.
831+
*
832+
* `--update` is the sharper of the two: it runs BEFORE any comparison, so over an
833+
* empty population it would write `{}` and ratchet all twelve baselined configs
834+
* out of existence — real, frozen debt discarded by a command whose whole purpose
835+
* is to record it. Same invariant the two reports above state, and for the same
836+
* reason: nothing measured, nothing written.
837+
*
838+
* Exits 1, the code every other verdict here uses.
839+
*
840+
* @param {{ headline: string, detail: string[] }} verdict
841+
*/
842+
function reportEmptyPopulation(verdict) {
843+
console.error(
844+
`\ncheck-i18n-coverage: POPULATION EMPTY — ${verdict.headline}\n\n` +
845+
verdict.detail.map((l) => (l ? ` ${l}` : '')).join('\n') +
846+
`\n\n Fix: run this gate from a complete checkout of the repo. \`pnpm check:i18n-coverage\`\n` +
847+
` is the invocation CI uses, and pnpm runs it from the repo root.\n\n` +
848+
` Nothing was measured: no config was linted and no count was compared, so this\n` +
849+
` result says NOTHING about whether any declared label went untranslated — and\n` +
850+
` the baseline was left exactly as committed (\`--update\` included).\n` +
851+
` (Exit code 1 — but piping this gate reports the PIPE's status, so\n` +
852+
` \`pnpm check:i18n-coverage | tail -4\` reads green either way. Use \`echo "EXIT=$?"\`.)`,
853+
);
854+
process.exit(1);
855+
}
856+
657857
/**
658858
* Answered once, before the per-config loop — so a missing build costs one
659859
* verdict instead of an exception thrown from inside the first example, and
@@ -676,7 +876,12 @@ function checkCliBuildPrerequisite() {
676876
console.error(`check-i18n-coverage: ${resolved.unknown} — build prerequisite not pre-checked`);
677877
return;
678878
}
679-
if (existsSync(resolved.file)) return;
879+
// `resolved.file` is repo-relative (`packages/cli/dist/commands/lint.js`), so it
880+
// needs the same anchoring as everything else — unanchored, an off-root run that
881+
// got this far would report "the workspace CLI is not built" about a CLI that is
882+
// built, which is the #5862 defect (a confident diagnosis pointing somewhere
883+
// innocent) rebuilt one layer down.
884+
if (existsSync(at(resolved.file))) return;
680885
reportPrerequisiteNotMet('the workspace CLI is not built', [
681886
`This gate counts what \`os lint\` reports, and it runs the BUILT CLI.`,
682887
`${CLI} is only a source stub that hands off to oclif, which`,
@@ -689,21 +894,24 @@ function checkCliBuildPrerequisite() {
689894

690895
checkCliBuildPrerequisite();
691896

692-
const { current, failures: unmeasured } = measureAllConfigs(
693-
[...discoverExamples(), ...discoverPackages()],
694-
measureI18nIssues,
695-
);
897+
const configPaths = [...discoverExamples(), ...discoverPackages()];
898+
// Before a single CLI is spawned, and before `--update` can write: a round with no
899+
// population has no verdict to give and no baseline to rewrite (#10907).
900+
const emptyPopulation = emptyPopulationVerdict(configPaths);
901+
if (emptyPopulation) reportEmptyPopulation(emptyPopulation);
902+
903+
const { current, failures: unmeasured } = measureAllConfigs(configPaths, measureI18nIssues);
696904
// Before `--update` writes anything, and before any comparison: a round that could
697905
// not measure every config has no verdict to give and no baseline to rewrite.
698906
if (unmeasured.length) reportUnmeasuredConfigs(unmeasured, Object.keys(current).length);
699907

700908
if (update) {
701-
writeFileSync(BASELINE_PATH, JSON.stringify(current, null, 2) + '\n');
909+
writeFileSync(at(BASELINE_PATH), JSON.stringify(current, null, 2) + '\n');
702910
console.log(`i18n coverage baseline updated: ${Object.keys(current).length} config(s).`);
703911
process.exit(0);
704912
}
705913

706-
const baseline = existsSync(BASELINE_PATH) ? JSON.parse(readFileSync(BASELINE_PATH, 'utf8')) : {};
914+
const baseline = existsSync(at(BASELINE_PATH)) ? JSON.parse(readFileSync(at(BASELINE_PATH), 'utf8')) : {};
707915

708916
const errors = [];
709917
for (const [file, count] of Object.entries(current)) {

scripts/pm/bare-root-worklist.mjs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,15 @@ const TRIAGE = new Map([
210210
verdict: 'REFUSE-UNSPELLABLE',
211211
why: 'one named config file per child directory — 3 of 238 (1.3%)',
212212
}],
213+
['check:i18n-coverage PACKAGES_DIR packages', {
214+
verdict: 'REFUSE-UNSPELLABLE',
215+
why: 'files named i18n-extract.config.ts beneath a scripts segment — 9 of 5035 (0.18%), the '
216+
+ 'narrowest row on this list. Same filename-filter shape as its EXAMPLES_DIR sibling above, '
217+
+ 'and refused with it rather than split: a subtree hint would name this gate for 5035 files '
218+
+ 'to reach 9. The root was ALWAYS this invisible — it reached the sweep only once the fix '
219+
+ 'for #10907 gave the literal a population-constant name, so this row records a population '
220+
+ 'that was previously unnameable rather than one the fix introduced',
221+
}],
213222
['scripts/check-skills-token-ratchet.mjs SKILLS_DIR skills', {
214223
verdict: 'REFUSE-UNSPELLABLE',
215224
why: 'one named file per child directory, 11 of 50 (22%). It already reaches its own cards '

0 commit comments

Comments
 (0)