Skip to content

Commit d5eae53

Browse files
fix(scripts): dead scan roots are a hard error in three more check gates (#4930) (#4941)
`check-single-authz-resolver`, `check-startup-registry-verdict` and `check-driver-conformance` each concluded from a walk that opened with `try { readdirSync(dir) } catch { return out/[]; }`. A root that was renamed, moved or made unreadable produced zero files, and zero files produce zero findings — the same verdict as a clean workspace. Same shape as #4916/#4934. Each script now resolves every declared root before drawing any conclusion from the scan, and fails naming the root and the reason ("does not exist" / "exists but is not a directory"). The swallows inside the walks are gone too: an error mid-walk means the corpus was only partly read, which must not print as a clean pass either. No whitelist and no `optional: true` flag — every root is a git-tracked directory, so an optional marker would be a supported way to silence the failure instead of fixing the rename. Measured on this tree, pre-fix: only check-single-authz-resolver was genuinely vacuous end to end (it went red by luck, because both DELEGATORS live under the same root, reporting "Delegator missing" for the wrong cause). The other two already failed, but named the wrong cause — startup-registry-verdict's `existsSync` accepted a file and then blamed an empty corpus, and driver-conformance answered a renamed `packages/spec/src/data` with five "CASE_SETS names X, which <file> no longer exports" errors that read as five deliberate deletions. Both now name the directory. The proofs are bidirectional and permanent, not one-offs in the PR body: each `--self-test` renames a root away mid-run and requires red naming that root, replaces one with a file and requires the not-a-directory verdict, dangles a symlink inside the walk and requires the error rather than a smaller corpus, then restores everything and requires green again. check-single-authz-resolver had no self-test and gained one covering both of its original invariants; `package.json` chains `--self-test &&` into `check:authz-resolver` to match the other ten `check:*` entries. Claude-Session: https://claude.ai/code/session_018iARDqtrhQgz6fVHDeDkbQ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 86552f2 commit d5eae53

5 files changed

Lines changed: 616 additions & 73 deletions
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
---
3+
4+
Tooling-only: three more `scripts/check-*.mjs` gates now fail, by name, when a declared scan root cannot be resolved (#4930). Releases nothing — no package changes.
5+
6+
Same shape as #4916/#4934, in three more scripts: a walker that opens with `try { readdirSync(dir) } catch { return out; }` (or `catch { return []; }`) cannot distinguish "this root is clean" from "this root was never opened", because both produce zero findings. Each script now resolves every declared root up front and fails with the root's name and reason ("does not exist" / "exists but is not a directory") before drawing any conclusion from the scan, and the swallows inside the walks are gone — an error mid-walk means the corpus was only partly read, which must not print as a clean pass either.
7+
8+
- `check-single-authz-resolver.mjs``SCAN_ROOTS` (`packages/`). The duplicate-resolver scan was genuinely vacuous on a dead root; the script only went red by luck, because both `DELEGATORS` happen to live under that same root, so it reported "Delegator missing" for the wrong cause. Move a delegator, or add a second scan root, and the vacuous scan goes fully silent.
9+
- `check-startup-registry-verdict.mjs` — the `packages/` scan root. The old `existsSync` guard accepted a *file* and then blamed the empty corpus; and one level in, a directory the walk could not stat shrank the corpus while `files.length` stayed non-zero, so the loud empty-corpus guard never fired.
10+
- `check-driver-conformance.mjs``DRIVERS_DIR`, `CASE_SETS_DIR`, and each driver's `src/`. DISCOVERED/CLASSIFIED did catch a dead root, but as a *consequence* naming the wrong cause: a renamed `packages/spec/src/data` printed five "CASE_SETS names X, which <file> no longer exports" errors, which reads as five deliberate deletions rather than one directory that moved.
11+
12+
No whitelist and no `optional: true` flag in any of the three, deliberately — every one of these roots is a git-tracked directory with tracked files, so no checkout that can run these gates at the repo root is legitimately missing one. An optional marker would be a supported way to silence the failure instead of fixing the rename: the empty `catch {}`, spelled politely.
13+
14+
Each proof is bidirectional and permanent rather than a one-off in the PR description: every script's `--self-test` now renames a root away mid-run and requires red naming that root (and not the survivors), replaces one with a file and requires the `not a directory` verdict, dangles a symlink inside the walk and requires the error rather than a smaller corpus, then restores everything and requires green again. `check-single-authz-resolver.mjs` had no self-test at all and gained one, with its two original invariants driven over a real temporary tree; `package.json` chains `--self-test &&` into `check:authz-resolver` to match the other ten `check:*` entries.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
"check:role-word": "node scripts/check-role-word.mjs",
3939
"check:adr-anchors": "node scripts/check-adr-anchors.mjs",
4040
"check:org-identifier": "node scripts/check-org-identifier.mjs",
41-
"check:authz-resolver": "node scripts/check-single-authz-resolver.mjs",
41+
"check:authz-resolver": "node scripts/check-single-authz-resolver.mjs --self-test && node scripts/check-single-authz-resolver.mjs",
4242
"check:slot-lookup": "node scripts/check-slot-lookup-ratchet.mjs",
4343
"check:service-providers": "node scripts/check-service-providers.mjs",
4444
"check:route-envelope": "node scripts/check-route-envelope.mjs --self-test && node scripts/check-route-envelope.mjs",

scripts/check-driver-conformance.mjs

Lines changed: 172 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,26 @@
7070
// Every entry was measured against `main`. To clear one: write the suite, then
7171
// delete the entry in the same PR. Deleting without the suite fails CONSUMED;
7272
// keeping the entry alongside the suite fails RECONCILED.
73+
//
74+
// ## Dead scan roots are a hard error (#4930)
75+
//
76+
// Both axes of the matrix are read off disk, from two declared directories:
77+
// DRIVERS_DIR and CASE_SETS_DIR. `listDir` used to be
78+
// `try { return readdirSync(dir); } catch { return []; }`, so a root that was
79+
// renamed, moved or made unreadable simply produced an empty axis. DISCOVERED
80+
// and CLASSIFIED do catch that today — but they catch it as a *consequence*,
81+
// and they name the wrong cause: a renamed `packages/spec/src/data` reports five
82+
// separate "CASE_SETS names X, which <file> no longer exports" errors, which
83+
// reads as five deliberate deletions rather than one directory that moved. The
84+
// author's next action follows the message, so the message has to be the cause.
85+
//
86+
// Both roots are therefore resolved before anything is discovered, and a dead
87+
// one fails BY NAME up front. The `listDir` swallow is gone with them: an error
88+
// during a walk means the corpus was only partly read, and partial evidence of
89+
// coverage is exactly the wrong thing to resolve in coverage's favour.
90+
// Deliberately no whitelist and no optional-root flag — see `assertRootsResolvable`.
7391

74-
import { mkdirSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs';
92+
import { mkdirSync, readdirSync, readFileSync, rmSync, statSync, symlinkSync, writeFileSync } from 'node:fs';
7593
import { join, dirname } from 'node:path';
7694
import { fileURLToPath } from 'node:url';
7795

@@ -144,13 +162,58 @@ const LEDGER = [];
144162

145163
// ── Discovery ───────────────────────────────────────────────────────────────
146164

147-
const listDir = (dir) => {
148-
try {
149-
return readdirSync(dir);
150-
} catch {
151-
return [];
165+
/** A declared scan root that could not be resolved to a directory. Carries the names. */
166+
class DeadRootError extends Error {
167+
constructor(dead) {
168+
super(`unresolvable scan root(s): ${dead.map((d) => `${d.root}${d.reason}`).join('; ')}`);
169+
this.name = 'DeadRootError';
170+
this.dead = dead;
171+
/** @type {string[]} just the root paths, for callers that only need to point. */
172+
this.roots = dead.map((d) => d.root);
152173
}
153-
};
174+
}
175+
176+
/**
177+
* Resolve every declared scan root before discovering anything; throw naming the
178+
* ones that are not directories.
179+
*
180+
* Deliberately no whitelist and no `optional: true` marker. `packages/plugins`,
181+
* `packages/spec/src/data` and every driver's `src/` are git-tracked directories
182+
* with tracked files in them, so any checkout that can run
183+
* `pnpm check:driver-conformance` has all of them. An optional marker "just in
184+
* case" would hand the next author a supported way to silence this failure
185+
* instead of fixing the rename — the empty `catch { return []; }` again, only
186+
* spelled politely. If a root ever does become legitimately absent, that is a real
187+
* decision: record it with its condition and a test, don't relax the check.
188+
*
189+
* @throws {DeadRootError}
190+
*/
191+
function assertRootsResolvable(roots) {
192+
const dead = [];
193+
for (const root of roots) {
194+
let st = null;
195+
try {
196+
st = statSync(root);
197+
} catch (err) {
198+
dead.push({
199+
root,
200+
reason: err?.code === 'ENOENT' ? 'does not exist' : `cannot be read (${err?.code ?? err})`,
201+
});
202+
continue;
203+
}
204+
if (!st.isDirectory()) dead.push({ root, reason: 'exists but is not a directory' });
205+
}
206+
if (dead.length) throw new DeadRootError(dead);
207+
}
208+
209+
/**
210+
* The entries of a directory the caller has already asserted is a scan root.
211+
*
212+
* No catch: an unresolvable root fails loudly in `assertRootsResolvable`, and an
213+
* error here means the axis was only partly read — which must not resolve in
214+
* coverage's favour (#4930).
215+
*/
216+
const listDir = (dir) => readdirSync(dir);
154217

155218
/** Driver packages, from disk — never a hardcoded list. */
156219
function discoverDrivers() {
@@ -201,18 +264,26 @@ function discoverCaseSets() {
201264
return found;
202265
}
203266

204-
/** Every `.ts` file under a directory, recursively. */
267+
/**
268+
* Every `.ts` file under a directory, recursively.
269+
*
270+
* A driver's `src/` is a scan root like the other two: "this driver does not run
271+
* the shared cases" must mean the files were read and the marker was absent, never
272+
* that the directory could not be opened. So it is asserted, and nothing in the
273+
* walk is swallowed (#4930).
274+
*/
205275
function walkTs(dir, out = []) {
206-
for (const entry of listDir(dir)) {
276+
assertRootsResolvable([dir]);
277+
walkTsInto(dir, out);
278+
return out;
279+
}
280+
281+
function walkTsInto(dir, out) {
282+
for (const entry of readdirSync(dir)) {
207283
if (entry === 'node_modules' || entry === 'dist') continue;
208284
const full = join(dir, entry);
209-
let s;
210-
try {
211-
s = statSync(full);
212-
} catch {
213-
continue;
214-
}
215-
if (s.isDirectory()) walkTs(full, out);
285+
const s = statSync(full);
286+
if (s.isDirectory()) walkTsInto(full, out);
216287
else if (entry.endsWith('.ts')) out.push(full);
217288
}
218289
return out;
@@ -243,6 +314,11 @@ function consumes(driverDir, marker) {
243314
// ── The run ─────────────────────────────────────────────────────────────────
244315

245316
function audit() {
317+
// Both axes come off disk, so both roots must resolve before a single cell of
318+
// the matrix is believed. Throws DeadRootError — `report()` turns it into a red
319+
// that names the directory rather than the five downstream symptoms (#4930).
320+
assertRootsResolvable([DRIVERS_DIR, CASE_SETS_DIR]);
321+
246322
const drivers = discoverDrivers();
247323
const errors = [];
248324
const rows = [];
@@ -311,8 +387,30 @@ function audit() {
311387
return { drivers, rows, errors };
312388
}
313389

390+
function reportDeadRoots(err) {
391+
console.error('\n x check-driver-conformance: declared scan root(s) do not resolve, so the matrix would\n' +
392+
' have been built from an axis nothing could read:\n');
393+
for (const d of err.dead) console.error(` ${d.root.startsWith(ROOT) ? d.root.slice(ROOT.length + 1) : d.root}${d.reason}`);
394+
console.error(
395+
'\n DRIVERS_DIR and CASE_SETS_DIR (scripts/check-driver-conformance.mjs) must both be' +
396+
'\n directories in the checkout. If one was renamed or moved, point the constant at it; if it' +
397+
'\n was deleted, that is a deliberate decision to record. Do NOT restore a tolerant skip: this' +
398+
'\n used to be `catch { return []; }`, and a dead root produced an empty axis whose downstream' +
399+
'\n errors named the wrong cause (#4930).\n',
400+
);
401+
}
402+
314403
function report() {
315-
const { drivers, rows, errors } = audit();
404+
let audited;
405+
try {
406+
audited = audit();
407+
} catch (err) {
408+
if (!(err instanceof DeadRootError)) throw err;
409+
reportDeadRoots(err);
410+
process.exit(1);
411+
return;
412+
}
413+
const { drivers, rows, errors } = audited;
316414

317415
const covered = rows.filter((r) => r.state === 'covered').length;
318416
const debt = rows.filter((r) => r.state === 'debt').length;
@@ -410,12 +508,68 @@ function selfTest() {
410508
expect('a discovery that found something is not', discoveredErrors(['driver-anything']).length === 0);
411509
expect('discovers driver packages from disk', discoverDrivers().length > 0);
412510

511+
// --- Reverse proof for the dead-root hard error (#4930), made permanent. ---
512+
// Everything above ran over roots that resolve, which proves nothing about a
513+
// gate whose failure mode is discovering an empty axis. So break a root the way
514+
// a rename breaks it, require red naming that root and not the survivor, then
515+
// restore it and require green again. Red-then-green, in the same run, every run.
516+
const tmpRoots = join(ROOT, 'node_modules', '.check-driver-conformance-selftest-roots');
517+
try {
518+
mkdirSync(join(tmpRoots, 'live'), { recursive: true });
519+
const missing = join(tmpRoots, 'renamed-away');
520+
let deadErr = null;
521+
try { assertRootsResolvable([join(tmpRoots, 'live'), missing]); } catch (err) { deadErr = err; }
522+
expect('a renamed scan root throws instead of yielding an empty axis', deadErr instanceof DeadRootError);
523+
expect('the failure names the dead root', deadErr?.roots?.join(',') === missing);
524+
expect('the failure does not blame the surviving root', !/live/.test(deadErr?.message ?? ''));
525+
expect('the failure says why', deadErr?.dead?.[0]?.reason === 'does not exist');
526+
527+
// A root that exists but is not a directory is dead in the same way: the old
528+
// `catch { return []; }` swallowed its ENOTDIR exactly as it swallowed ENOENT.
529+
const asFile = join(tmpRoots, 'a-file');
530+
writeFileSync(asFile, 'not a directory');
531+
let notDirErr = null;
532+
try { assertRootsResolvable([asFile]); } catch (err) { notDirErr = err; }
533+
expect('a scan root that is a file is dead too',
534+
notDirErr?.dead?.[0]?.reason === 'exists but is not a directory');
535+
536+
// An entry the walk cannot stat inside a driver's src/ is the same defect one
537+
// level in: `catch { continue; }` used to drop it, and a dropped file that
538+
// held the marker reads as "this driver does not run the case-set".
539+
mkdirSync(join(tmpRoots, 'pkg', 'src'), { recursive: true });
540+
writeFileSync(join(tmpRoots, 'pkg', 'src', 'a.ts'), 'export const a = 1;\n');
541+
expect('a readable src/ walks clean', walkTs(join(tmpRoots, 'pkg', 'src')).length === 1);
542+
symlinkSync(join(tmpRoots, 'no-such-target'), join(tmpRoots, 'pkg', 'src', 'dangling'));
543+
let partialErr = null;
544+
try { walkTs(join(tmpRoots, 'pkg', 'src')); } catch (err) { partialErr = err; }
545+
expect('an entry the walk cannot stat is an error, not a smaller corpus', partialErr?.code === 'ENOENT');
546+
rmSync(join(tmpRoots, 'pkg', 'src', 'dangling'));
547+
548+
// ...and roots that resolve are green, so the reds above were caused by the
549+
// broken roots and nothing else.
550+
let restored = null;
551+
try { assertRootsResolvable([join(tmpRoots, 'live'), join(tmpRoots, 'pkg', 'src')]); } catch (err) { restored = err; }
552+
expect('roots that resolve raise nothing', restored === null);
553+
expect('restoring the tree makes the walk green again', walkTs(join(tmpRoots, 'pkg', 'src')).length === 1);
554+
555+
// The real roots this gate runs against resolve — the assertion is wired in,
556+
// not merely defined.
557+
let realErr = null;
558+
try { assertRootsResolvable([DRIVERS_DIR, CASE_SETS_DIR]); } catch (err) { realErr = err; }
559+
expect('the real DRIVERS_DIR and CASE_SETS_DIR both resolve', realErr === null);
560+
} finally {
561+
rmSync(tmpRoots, { recursive: true, force: true });
562+
}
563+
413564
if (failures.length) {
414565
for (const f of failures) console.error(` x self-test: ${f}`);
415566
console.error(`\ncheck-driver-conformance --self-test: ${failures.length} failure(s).\n`);
416567
process.exit(1);
417568
}
418-
console.log('OK self-test: detects driven / unused / re-declared fixtures, and discovers both axes.');
569+
console.log(
570+
'OK self-test: detects driven / unused / re-declared fixtures, discovers both axes, and holds the '
571+
+ 'dead-root hard error (red when a scan root is renamed, green when restored).',
572+
);
419573
}
420574

421575
if (process.argv.includes('--self-test')) selfTest();

0 commit comments

Comments
 (0)