Skip to content

Commit c0770d0

Browse files
claude[bot]claude
andauthored
fix(scripts): answer a check-type-check-coverage refusal with exit 3, not a finding's 1 (#13982)
Every refusal in this gate reached node's uncaught handler, which exits 1 -- the code the gate reserves for "a package's recorded debt went UP", whose prescribed remedy ends at a ledger only a maintainer may raise. So an unmeasurable run pointed the reader at the one place that evidence must never send them, and anything reading exit codes rather than prose could not tell the two apart at all. Routes all eleven refusal sites through a new refusePrerequisite(), which prints the raising site's message VERBATIM under a PREREQUISITE NOT MET frame and exits 3 -- the code check-test-completeness.mjs states in its own failure text and check-dual-build-cjs-loads.mjs already answers the identical "reads built output, tree has no dist/" condition with. Exit 1 now means only a finding: a structural problem, or a ledger entry that drifted upward. A tsconfig.json checked into the tree that does not parse stays a finding. The self-test pins the class over the refusing FUNCTION BODIES rather than over a constant, because the regression shape is one careless `throw new Error(` added by an author not thinking about exit codes -- invisible to every consumer, all of which treat any non-zero as failure. Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC Co-authored-by: Claude <noreply@anthropic.com>
1 parent 715fcef commit c0770d0

1 file changed

Lines changed: 201 additions & 13 deletions

File tree

scripts/check-type-check-coverage.mjs

Lines changed: 201 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,40 @@
4646
// # -- see BUILT CLOSURE.
4747
// node scripts/check-type-check-coverage.mjs --self-test
4848
//
49+
// ## Exit codes -- and why a REFUSAL has one of its own
50+
//
51+
// 0 the tree was read and every invariant above holds.
52+
// 1 a FINDING: a structural problem, or -- under `--re-measure` -- a ledger
53+
// entry that drifted UPWARD. A claim about the tree.
54+
// 3 PREREQUISITE NOT MET. The gate refused to measure, so nothing was
55+
// measured and the run says NOTHING about the ledger. NOT a pass, and
56+
// NOT a finding.
57+
//
58+
// The split is the sibling convention, not a local invention:
59+
// `check-test-completeness.mjs` states it in its own failure text ("Exit code
60+
// 3, distinct from a finding's 1") and `check-dual-build-cjs-loads.mjs`
61+
// answers the IDENTICAL condition -- a gate that reads built output, run
62+
// against a tree with no `dist/` -- with 3. This gate used to answer it by
63+
// letting the refusal reach node's uncaught handler, which exits 1.
64+
//
65+
// ⭐ Why the class matters HERE in particular, more than it does for a gate
66+
// whose 1 means "something is wrong somewhere". Exit 1 from this gate has one
67+
// specific meaning: a package's recorded debt went UP. The remedy that meaning
68+
// prescribes ends at the ledger below, and raising a DEBT/TEST_DEBT entry is a
69+
// MAINTAINER-only act this file spells out at length. So a reader who takes an
70+
// unmeasurable run for that red is pointed straight at the one place this
71+
// evidence must never send them. The prose said so all along -- but the prose
72+
// is not what an exit-code reader reads. A CI step, a wrapper, or an agent
73+
// reconciling a derived gate family sees the number and nothing else.
74+
//
75+
// ⛔ The boundary, deliberately. Exit 3 is for a prerequisite the WORLD failed
76+
// to supply and that the caller clears with a named command: an unbuilt or
77+
// stale dependency closure, a closure that does not build, an absent `turbo`
78+
// or `tsc` binary, a tsc that could not be spawned or could not read the
79+
// project it was handed. A malformed `tsconfig.json` checked INTO the tree
80+
// stays exit 1 -- that is a fact about the tree, which is what a finding is,
81+
// and `readTsconfig` keeps throwing it.
82+
//
4983
// Invariants, per workspace package (the root workspace package included --
5084
// #4311's audit counted its top-level TypeScript like any other package's):
5185
//
@@ -468,6 +502,74 @@ import {
468502
const ROOT = resolve(import.meta.dirname, '..');
469503
const SELF = 'scripts/check-type-check-coverage.mjs';
470504
const TRACKING_ISSUE = 'https://github.com/objectstack-ai/objectstack/issues/4311';
505+
506+
// The exit-code contract, NAMED rather than spelled inline at each site, so the
507+
// self-test pins the value each path actually returns instead of a comment
508+
// about it -- the shape `check-test-completeness.mjs` uses for the same split.
509+
//
510+
// ⛔ Module-local, NOT exported, and that is a decision rather than an
511+
// oversight: this file's top level RUNS (it is a gate, invoked as a script and
512+
// nothing else), so exporting any binding at all would make it importable for
513+
// that binding and run the whole gate inside the importer -- the class
514+
// `check:entry-guard` refuses, and it caught this constant block on its first
515+
// run. The sibling that does export its codes carries `isEntrypoint` for
516+
// exactly this reason. Nothing imports this file today; the day something
517+
// needs to, the guard comes with the export.
518+
const EXIT_OK = 0;
519+
const EXIT_FINDINGS = 1;
520+
const EXIT_PREREQUISITE_NOT_MET = 3;
521+
522+
/**
523+
* The text a refusal prints, as a VALUE -- so the self-test can assert on the
524+
* advisory without spawning a process or stubbing `process.exit`, the split
525+
* `import-prerequisite.mjs` documents for the same reason.
526+
*
527+
* The gate's own refusal message is embedded VERBATIM. Every one of them was
528+
* already argued at length at its throw site, and none of that reasoning is
529+
* this frame's to restate, shorten or improve -- the frame adds only the two
530+
* things a reader could not get from the message: what CLASS of result this is,
531+
* and which exit code carries it.
532+
*
533+
* The pipe advisory is not decoration. `EXIT=$?` written after `cmd | tail -40`
534+
* reads TAIL's status, and `head`/`tail` essentially never fail -- so a refusal
535+
* and a green run are the same `0` there, which is the one reading this whole
536+
* exit-code split exists to make impossible.
537+
*
538+
* @param {string} message the refusal, in the words of the site that raised it
539+
* @returns {string}
540+
*/
541+
function prerequisiteNotMetText(message) {
542+
return (
543+
`\ncheck-type-check-coverage: PREREQUISITE NOT MET\n\n` +
544+
`${message}\n\n` +
545+
` ⛔ This is NOT a pass and NOT a finding: nothing was measured, so this run says\n` +
546+
` NOTHING about whether any DEBT or TEST_DEBT number is still correct. In particular\n` +
547+
` it is NOT evidence that a recorded number went up, and ⛔ no ledger entry below may\n` +
548+
` be raised on it -- raising one is a maintainer's act even when the evidence is real.\n` +
549+
` (Exit code ${EXIT_PREREQUISITE_NOT_MET}, distinct from a finding's ${EXIT_FINDINGS} — capture it BEFORE any pipe:\n` +
550+
` \`node ${SELF} --re-measure > /tmp/type-check-debt.log 2>&1; echo "EXIT=$?"\`.\n` +
551+
` Piped, \`$?\` is the LAST command's status, and \`head\`/\`tail\` essentially never fail — that\n` +
552+
` is the false green. \`\${PIPESTATUS[0]}\`/\`pipefail\` do recover this gate's own code.)`
553+
);
554+
}
555+
556+
/**
557+
* Refuse to measure, and say so in the exit code as well as in the prose.
558+
*
559+
* ⛔ Prints and EXITS rather than throwing. A thrown refusal reaches node's
560+
* uncaught handler, which exits 1 -- the code this gate reserves for "a
561+
* recorded debt number went UP" -- and prints a stack trace over a message
562+
* whose whole job is to be read. The self-test pins that the three functions
563+
* that refuse contain no bare `throw new Error(` for exactly this reason: the
564+
* rot this repairs is one careless `throw` away from returning.
565+
*
566+
* @param {string} message the refusal, in the words of the site that raised it
567+
* @returns {never}
568+
*/
569+
function refusePrerequisite(message) {
570+
console.error(prerequisiteNotMetText(message));
571+
process.exit(EXIT_PREREQUISITE_NOT_MET);
572+
}
471573
// An `exclude` pattern that names tests (`**/*.test.ts`, `**/*.spec.tsx`, ...)
472574
// and the files such a pattern hides. Kept deliberately broad: the question is
473575
// "does this config steer tsc away from the test layer", not "which exact glob".
@@ -1532,12 +1634,14 @@ function gitIgnoredPaths(rels) {
15321634
maxBuffer: 16 * 1024 * 1024,
15331635
});
15341636
if (res.error) {
1535-
throw new Error(`git check-ignore could not run, so GENERATED_COVERED cannot be judged`, { cause: res.error });
1637+
refusePrerequisite(
1638+
`git check-ignore could not run, so GENERATED_COVERED cannot be judged: ${res.error.message}`,
1639+
);
15361640
}
15371641
// 0 = some path is ignored, 1 = none is. Anything else (128: not a git
15381642
// checkout, bad option) is a failed MEASUREMENT, not a clean tree.
15391643
if (res.status !== 0 && res.status !== 1) {
1540-
throw new Error(
1644+
refusePrerequisite(
15411645
`git check-ignore exited ${res.status}, so GENERATED_COVERED cannot be judged: ${String(res.stderr).trim()}`,
15421646
);
15431647
}
@@ -2701,14 +2805,14 @@ function workspaceBuildGraph(packages) {
27012805
function refreshBuiltClosure() {
27022806
const bin = join(ROOT, 'node_modules', '.bin', 'turbo');
27032807
if (!existsSync(bin)) {
2704-
throw new Error(`--re-measure needs the workspace's own turbo at ${bin}; run \`pnpm install\` first.`);
2808+
refusePrerequisite(`--re-measure needs the workspace's own turbo at ${bin}; run \`pnpm install\` first.`);
27052809
}
27062810
const args = ['run', 'build', '--filter=./packages/*', '--filter=./packages/*/*'];
27072811
const run = spawnSync(bin, args, { cwd: ROOT, encoding: 'utf8', maxBuffer: 256 * 1024 * 1024 });
2708-
if (run.error) throw new Error(`the closure build could not be run: ${run.error.message}`);
2812+
if (run.error) refusePrerequisite(`the closure build could not be run: ${run.error.message}`);
27092813
if (run.status !== 0) {
27102814
const output = `${run.stdout ?? ''}${run.stderr ?? ''}`.trim();
2711-
throw new Error(
2815+
refusePrerequisite(
27122816
`--re-measure cannot run: the ledgered packages' dependency closure does not build, so there is no `
27132817
+ `world to measure against. Fix the build first -- every number in DEBT and TEST_DEBT is measured `
27142818
+ `with tsc resolving workspace imports through each dependency's built \`dist/*.d.ts\`.\n`
@@ -2929,7 +3033,7 @@ const REMEASURE_HEAP = remeasureHeapCeiling({
29293033
function tscErrorCount(project, options = {}) {
29303034
const bin = join(ROOT, 'node_modules', '.bin', 'tsc');
29313035
if (!existsSync(bin)) {
2932-
throw new Error(`--re-measure needs the workspace's own tsc at ${bin}; run \`pnpm install\` first.`);
3036+
refusePrerequisite(`--re-measure needs the workspace's own tsc at ${bin}; run \`pnpm install\` first.`);
29333037
}
29343038
const run = spawnSync(bin, ['--noEmit', '--pretty', 'false', '-p', project], {
29353039
cwd: ROOT,
@@ -2941,10 +3045,10 @@ function tscErrorCount(project, options = {}) {
29413045
// dressed as a different one.
29423046
env: heapCappedEnv(process.env, REMEASURE_HEAP.mb),
29433047
});
2944-
if (run.error) throw new Error(`tsc could not be run for ${project}: ${run.error.message}`);
3048+
if (run.error) refusePrerequisite(`tsc could not be run for ${project}: ${run.error.message}`);
29453049
const output = `${run.stdout ?? ''}${run.stderr ?? ''}`;
29463050
if (TSC_SETUP_ERROR.test(output)) {
2947-
throw new Error(`tsc could not read ${project} -- the measurement is invalid, not zero:\n${output.trim()}`);
3051+
refusePrerequisite(`tsc could not read ${project} -- the measurement is invalid, not zero:\n${output.trim()}`);
29483052
}
29493053
const errors = countTscErrors(output, options);
29503054
// Exit 0 means a clean program; anything else must have produced diagnostics
@@ -2956,7 +3060,7 @@ function tscErrorCount(project, options = {}) {
29563060
// the `return 0` below -- refusing there would turn "this test layer is clean
29573061
// apart from an artefact of our own generated config" into a hard crash.
29583062
if (run.status !== 0 && countTscErrors(output) === 0) {
2959-
throw new Error(
3063+
refusePrerequisite(
29603064
`tsc exited ${run.status} for ${project} but printed no recognisable diagnostics -- ` +
29613065
`refusing to record 0:\n${output.trim().slice(0, 2000)}`,
29623066
);
@@ -3199,10 +3303,20 @@ function measureTestDebt(dir, hiddenTests = []) {
31993303
const holder = mkdtempSync(join(tmpdir(), 'objectstack-debt-remeasure-'));
32003304
const configPath = join(holder, REMEASURE_CONFIG);
32013305
writeFileSync(configPath, `${JSON.stringify(project, null, 2)}\n`);
3306+
// Registered on `exit` as WELL as in the `finally`, because `tscErrorCount`
3307+
// can now REFUSE, and a refusal calls `process.exit` -- which runs `exit`
3308+
// handlers and does NOT run `finally`. Belt and braces on purpose: the
3309+
// `finally` keeps the directory's lifetime visible where it is created, and
3310+
// the handler is what makes the refusal path leave nothing behind. `off`
3311+
// first in the `finally` so a run measuring 34 entries does not accumulate 34
3312+
// live handlers on a directory each has already removed.
3313+
const cleanup = () => rmSync(holder, { force: true, recursive: true });
3314+
process.once('exit', cleanup);
32023315
try {
32033316
return tscErrorCount(configPath, { dropRootDirDiagnostics: true });
32043317
} finally {
3205-
rmSync(holder, { force: true, recursive: true });
3318+
process.off('exit', cleanup);
3319+
cleanup();
32063320
}
32073321
}
32083322

@@ -3228,7 +3342,7 @@ function measureLedgers(packages, rootName, state) {
32283342
.filter((name) => dirOf.has(name));
32293343
const unbuilt = unbuiltClosure(ledgered, workspaceBuildGraph(packages));
32303344
if (unbuilt.length > 0) {
3231-
throw new Error(
3345+
refusePrerequisite(
32323346
`--re-measure cannot run: ${unbuilt.length} workspace dependenc(ies) of the ledgered packages have `
32333347
+ `no built type entry point on disk -- ${unbuilt.join(', ')}.\n`
32343348
+ `Every number in DEBT and TEST_DEBT is measured with tsc resolving workspace imports through each `
@@ -3262,7 +3376,7 @@ function measureLedgers(packages, rootName, state) {
32623376
// caller can act on.
32633377
const stale = staleClosure(ledgered, workspaceBuildGraph(packages));
32643378
if (stale.length > 0) {
3265-
throw new Error(
3379+
refusePrerequisite(
32663380
`--re-measure cannot run: ${stale.length} workspace dependenc(ies) of the ledgered packages still have `
32673381
+ `a type entry point OLDER than their own sources after a full closure build -- ${stale.join(', ')}.\n`
32683382
+ `The build covers \`./packages/*\` and \`./packages/*/*\`, so a package that survives it is one those `
@@ -5378,6 +5492,79 @@ function selfTest() {
53785492
}
53795493
}
53805494

5495+
// ── THE EXIT-CODE CLASS ───────────────────────────────────────────────────
5496+
//
5497+
// Pinned because it is exactly the kind of fact that rots back silently. The
5498+
// defect this replaces was not a wrong number typed anywhere: it was a
5499+
// refusal that reached node's UNCAUGHT handler, which exits 1 -- so the
5500+
// regression shape is one careless `throw new Error(...)` added to a refusing
5501+
// function by an author who never thought about exit codes at all, and it
5502+
// announces itself with a green CI (every consumer of this gate treats any
5503+
// non-zero as failure, so 1-instead-of-3 is invisible to all of them) and a
5504+
// human-readable message that still says the right thing. Nothing else in
5505+
// this file would notice.
5506+
//
5507+
// So the pin is over the FUNCTION BODIES, not over a constant. Reading the
5508+
// real `Function.prototype.toString()` of the four functions that refuse is
5509+
// what makes a re-added `throw` fail here rather than in six weeks, on a card
5510+
// about something else.
5511+
const REFUSING = [refreshBuiltClosure, tscErrorCount, measureLedgers, gitIgnoredPaths];
5512+
for (const fn of REFUSING) {
5513+
const body = fn.toString();
5514+
if (/throw new Error\(/.test(body)) {
5515+
failures.push(
5516+
`${fn.name} raises a bare \`throw new Error(\` — an uncaught throw exits 1, the code this gate ` +
5517+
`reserves for a ledger entry that drifted UPWARD. A refusal must go through ` +
5518+
`refusePrerequisite() so it exits ${EXIT_PREREQUISITE_NOT_MET}.`,
5519+
);
5520+
}
5521+
if (!body.includes('refusePrerequisite(')) {
5522+
failures.push(`${fn.name} no longer refuses through refusePrerequisite() — the exit-code class is unpinned`);
5523+
}
5524+
}
5525+
// The NEGATIVE control, and the reason the loop above is a measurement rather
5526+
// than a tautology over an empty set: `readTsconfig` still throws, on purpose.
5527+
// A `tsconfig.json` checked into the tree that does not parse is a fact about
5528+
// the TREE -- a finding -- not a prerequisite the caller forgot to supply, so
5529+
// it keeps exit 1 and the pin above must be able to SEE a bare throw.
5530+
if (!/throw new Error\(/.test(readTsconfig.toString())) {
5531+
failures.push(
5532+
'readTsconfig no longer throws — the bare-throw pin above can no longer fail, so it stopped measuring',
5533+
);
5534+
}
5535+
5536+
const exitCodeCases = [
5537+
{ label: 'the refusal code is 3', ok: EXIT_PREREQUISITE_NOT_MET === 3 },
5538+
{ label: 'a finding is 1', ok: EXIT_FINDINGS === 1 },
5539+
{ label: 'the refusal code is distinct from a finding and from a pass',
5540+
ok: EXIT_PREREQUISITE_NOT_MET !== EXIT_FINDINGS && EXIT_PREREQUISITE_NOT_MET !== EXIT_OK },
5541+
];
5542+
for (const c of exitCodeCases) {
5543+
if (!c.ok) failures.push(`exit-code contract — ${c.label}`);
5544+
}
5545+
5546+
// The refusal TEXT. Four load-bearing clauses, each one a thing a reader who
5547+
// sees only the exit code cannot get anywhere else.
5548+
const refusalFixture = '--re-measure cannot run: 48 workspace dependenc(ies) have no built type entry point';
5549+
const refusalText = prerequisiteNotMetText(refusalFixture);
5550+
const textCases = [
5551+
{ label: 'carries the raising site\'s own message VERBATIM', ok: refusalText.includes(refusalFixture) },
5552+
{ label: 'names the class', ok: refusalText.includes('PREREQUISITE NOT MET') },
5553+
{ label: 'says it is neither a pass nor a finding', ok: /NOT a pass and NOT a finding/.test(refusalText) },
5554+
{ label: 'names its own code and the finding code it is distinct from',
5555+
ok: refusalText.includes(`Exit code ${EXIT_PREREQUISITE_NOT_MET}`)
5556+
&& refusalText.includes(`a finding's ${EXIT_FINDINGS}`) },
5557+
// The specific misreading this whole change exists to stop: taking an
5558+
// unmeasurable run for "a recorded debt went up" routes the reader to the
5559+
// ledger, and raising an entry there is a maintainer's act.
5560+
{ label: 'turns the reader away from the ledger rather than toward it',
5561+
ok: /no ledger entry below may\n?\s*be raised on it/.test(refusalText) },
5562+
{ label: 'warns that the code must be captured before any pipe', ok: refusalText.includes('BEFORE any pipe') },
5563+
];
5564+
for (const c of textCases) {
5565+
if (!c.ok) failures.push(`prerequisiteNotMetText — ${c.label}`);
5566+
}
5567+
53815568
// The shared workspace enumerator is a plain module with no CI invocation of
53825569
// its own (#11510); every gate that consolidated onto it folds in its checks.
53835570
failures.push(...workspaceEnumeratorSelfTest({ root: ROOT }));
@@ -5396,7 +5583,8 @@ function selfTest() {
53965583
+ ceilingCases.length + heapEnvCases.length} re-measure case(s) + ` +
53975584
`${typeEntryCases.length + closureCases.length + staleCases.length + sourceFileCases.length} ` +
53985585
`built-closure case(s) + ` +
5399-
`${planCases.length + rewriteCases.length + roundTripCases.length} auto-lowering case(s) hold.`,
5586+
`${planCases.length + rewriteCases.length + roundTripCases.length} auto-lowering case(s) + ` +
5587+
`${REFUSING.length * 2 + 1 + exitCodeCases.length + textCases.length} exit-code case(s) hold.`,
54005588
);
54015589
}
54025590

0 commit comments

Comments
 (0)