fix(scripts): refuse an unreadable package instead of dropping it, and refuse the repo root as a package

scripts/check-test-glob-coverage.mjs

@@ -72,12 +72,31 @@
 import { readFileSync, readdirSync, existsSync, statSync } from 'node:fs';
 import { join, dirname, relative } from 'node:path';
 import { fileURLToPath } from 'node:url';
+import { listWorkspacePackages } from './lib/list-workspace-packages.mjs';
+
+// Deliberately a literal here rather than imported from the shared walk.
+// `scripts/lib/ci-path-coverage.mjs`'s `deriveInputs` reads only THIS file's own
+// source text and does not follow imports, so these two strings are what put
+// `packages/` and `apps/` into the CI-path-coverage census for this gate. Move
+// them into the lib and the ratchet silently stops checking that this gate can
+// be triggered by the paths it reads. (#3347)
+const PACKAGE_PARENTS = ['packages', 'apps'];
 
 const SCRIPT_DIR = dirname(fileURLToPath(import.meta.url));
 
+// Declared before any module-evaluation-time caller of `fail`. A class sits in
+// the temporal dead zone until its own line, so with this further down a `fail`
+// during module eval threw ReferenceError instead of FailError.
+export class FailError extends Error {}
+
 const rootFlagIdx = process.argv.indexOf('--root');
 if (rootFlagIdx !== -1 && !process.argv[rootFlagIdx + 1]) {
-  fail('--root requires a directory argument');
+  // console.error + exit, NOT `fail`, matching check-test-wiring.mjs. This runs
+  // during module evaluation, so a throw here escapes the entry point's
+  // try/catch at the bottom of the file and prints a raw stack on top of the
+  // message - which is precisely what this gate's own tests assert against.
+  console.error('\ncheck-test-glob-coverage: --root requires a directory argument\n');
+  process.exit(1);
 }
 const ROOT = rootFlagIdx === -1 ? join(SCRIPT_DIR, '..') : resolveArg(process.argv[rootFlagIdx + 1]);
 
@@ -91,11 +110,8 @@ export function fail(message) {
   throw new FailError(message);
 }
 
-export class FailError extends Error {}
-
 const TEST_FILE_RE = /\.(test|spec)\.(ts|tsx|mts|js|mjs)$/;
 const SKIP_DIRS = new Set(['node_modules', 'dist', 'pkg', 'build', 'coverage', '.turbo', 'generated']);
-const PACKAGE_PARENTS = ['packages', 'apps'];
 
 /**
  * Lower bound on how many packages must actually get audited when this runs
@@ -228,9 +244,37 @@ const VITEST_CONFIG_NAMES = [
  * file by construction, nothing to check".
  */
 function resolveVitestGlobs(pkgDir, testLooking) {
+  // Deliberately `existsSync`, and deliberately NOT `existsOrThrow` from
+  // ./lib/exists-or-throw.mjs, which this file's package walk uses via
+  // listWorkspacePackages. Measured, both ways, on a chmod-000 config:
+  // `statSync` SUCCEEDS on an unreadable file (the mode bits gate open(2), not
+  // stat(2)), so existsOrThrow returns true here and the failure lands on the
+  // read below either way. Swapping it in was an inert guard - identical exit
+  // code and identical message with and without it.
+  //
+  // There is no silent shrink on this path to begin with: the read throws and
+  // the gate exits 1. The only defect was the SHAPE of that exit, a raw
+  // EACCES stack instead of this gate's own message, so that is all the
+  // try/catch below fixes. It buys diagnostics, not fail-closed - the path was
+  // already closed.
   const configName = VITEST_CONFIG_NAMES.find((n) => existsSync(join(pkgDir, n)));
   if (!configName) return null;
-  const source = readFileSync(join(pkgDir, configName), 'utf8');
+  const configPath = join(pkgDir, configName);
+  let source;
+  try {
+    source = readFileSync(configPath, 'utf8');
+  } catch (err) {
+    fail(
+      `cannot read vitest config ${configPath}: ${err.code || err.message}. ` +
+        'Its include: globs decide which of this package\'s test files count as ' +
+        'reached, so the audit cannot answer for this package without it.',
+    );
+    // Same rethrow the two refusals in lib/list-workspace-packages.mjs carry.
+    // Without it a `fail` that returns leaves `source` undefined and
+    // parseViteInclude throws a TypeError on it: closed, but with the
+    // diagnosis destroyed, which is the outcome this branch exists to stop.
+    throw err;
+  }
   const includes = parseViteInclude(source);
   if (includes === null) return null;
   if (includes.length === 0) {
@@ -302,67 +346,8 @@ export function auditPackage(pkgDir, pkgJson) {
   return { testLooking, matched, missed };
 }
 
-/**
- * Does this path exist? Throws if the answer is UNKNOWABLE.
- *
- * `existsSync` answers false for every failure, including EACCES, so an
- * unreadable directory is indistinguishable from an absent one. That is the
- * same "absence reads as success" defect this gate was fixed for -- one stage
- * earlier, in DISCOVERY rather than in the walk. Measured before this change:
- * a `chmod 000` package carrying a real test script reported
- * `OK (1 packages audited, 0 unrun test files)` and exit 0, with the locked
- * package silently missing from the count.
- */
-function existsOrThrow(path, what) {
-  try {
-    statSync(path);
-    return true;
-  } catch (err) {
-    if (err.code === 'ENOENT') return false;
-    fail(
-      `cannot read ${what} ${path}: ${err.code || err.message}. ` +
-        'Refusing to treat an unreadable path as an absent one -- that is how a ' +
-        'package drops out of the audit without anyone noticing.',
-    );
-  }
-}
-
-export function listPackages(root, seenParents = []) {
-  const out = [];
-  for (const parent of PACKAGE_PARENTS) {
-    const parentDir = join(root, parent);
-    if (!existsOrThrow(parentDir, 'package parent')) continue;
-    seenParents.push(parent);
-    for (const name of readdirSync(parentDir).sort()) {
-      // A dotfile is not a candidate package and never was: pnpm-workspace.yaml
-      // globs `packages/*` and `apps/*`, and a bare `*` does not match a
-      // leading dot, so no dotted entry can ever be a workspace package. macOS
-      // drops a `.DS_Store` FILE into any directory Finder has opened, and
-      // statting `.DS_Store/package.json` raises ENOTDIR — which existsOrThrow
-      // below refuses, correctly and by design, failing the whole Lint lane on
-      // a local-only file. The fix is to stop offering a dotfile as a
-      // candidate, NOT to soften that refusal: every entry that could
-      // plausibly be a package still goes through existsOrThrow unchanged.
-      // Same skip walk() already applies one stage later. (#3350)
-      if (name.startsWith('.')) continue;
-      const pkgDir = join(parentDir, name);
-      const pkgJsonPath = join(pkgDir, 'package.json');
-      if (!existsOrThrow(pkgJsonPath, 'package manifest')) continue;
-      let pkgJson;
-      try {
-        pkgJson = JSON.parse(readFileSync(pkgJsonPath, 'utf8'));
-      } catch (err) {
-        fail(`${pkgJsonPath} is not valid JSON: ${err.message}`);
-      }
-      out.push({ rel: `${parent}/${name}`, dir: pkgDir, pkgJson });
-    }
-  }
-  return out;
-}
-
 function main() {
-  const seenParents = [];
-  const packages = listPackages(ROOT, seenParents);
+  const { packages, seenParents } = listWorkspacePackages(ROOT, fail, PACKAGE_PARENTS);
 
   // Anti-vacuity, structural: true of the real repo AND of every synthetic
   // fixture tree the regression harness builds, so it costs the harness

scripts/check-test-glob-coverage.test.mjs

@@ -31,7 +31,7 @@
 
 import { test } from 'node:test';
 import assert from 'node:assert/strict';
-import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs';
+import { mkdtempSync, mkdirSync, writeFileSync, rmSync, chmodSync } from 'node:fs';
 import { spawnSync } from 'node:child_process';
 import { tmpdir } from 'node:os';
 import { join, dirname } from 'node:path';
@@ -346,3 +346,58 @@ test('parseViteInclude: reads the first top-level include array, ignoring a nest
 test('parseViteInclude: returns null when there is no include key (vitest default applies)', () => {
   assert.equal(parseViteInclude('export default defineConfig({ test: {} });'), null);
 });
+
+test('an unreadable vitest config is reported by the gate, not as a raw stack', (t) => {
+  // Windows chmod does not remove read permission, and root ignores the mode
+  // bits, so on either the config stays readable and the case cannot be built.
+  if (process.platform === 'win32') return t.skip('chmod does not gate reads on Windows');
+  if (process.getuid?.() === 0) return t.skip('root reads a 000 file regardless of mode');
+
+  const files = {
+    'packages/fixture/package.json': pkgJson('vitest run'),
+    'packages/fixture/src/a.test.ts': 'test("a", () => {})',
+    'packages/fixture/vitest.config.ts': 'export default { test: { include: ["src/**/*.test.ts"] } }',
+  };
+  const dir = writeTree(files);
+  const config = join(dir, 'packages/fixture/vitest.config.ts');
+
+  try {
+    // BOTH directions. Readable first, so a fixture that fails for some
+    // unrelated reason cannot be mistaken for the refusal firing. Inside the
+    // try, or a failure here leaks the tree instead of cleaning up.
+    const readable = runOn(dir);
+    assert.equal(readable.status, 0, `readable config should audit cleanly:\n${readable.out}`);
+
+    chmodSync(config, 0o000);
+    const locked = runOn(dir);
+    assert.equal(locked.status, 1, 'an unreadable config must fail the gate');
+    assert.match(locked.out, /cannot read vitest config/);
+    // The point of the change. It already exited 1 before; what it did NOT do
+    // was say why, and an uncaught readFileSync stack sends the reader into
+    // node internals instead of at their own file mode.
+    //
+    // Matched on the payload rather than a `at readFileSync (node:fs` frame:
+    // V8 names the frame after the call form, so the frame spelling is voided
+    // by a change to how this gate imports fs. See the sibling in
+    // check-test-wiring.test.mjs, which was vacuous for that exact reason.
+    assert.doesNotMatch(locked.out, /permission denied, open/, 'must not surface a raw node error');
+  } finally {
+    chmodSync(config, 0o644);
+    rmSync(dir, { recursive: true, force: true });
+  }
+});
+
+test('--root with no argument is refused cleanly, without a raw stack', () => {
+  // This runs during MODULE EVALUATION, before the entry point's try/catch
+  // exists, so a `fail` here escapes and prints a stack on top of the message.
+  // It did exactly that, two ways at once: `class FailError` was declared below
+  // `fail`, so the throw was a ReferenceError from the temporal dead zone.
+  // check-test-wiring.test.mjs has always had this case; this file did not,
+  // which is why a gate whose own tests forbid raw stacks was printing one.
+  const r = spawnSync(process.execPath, [CHECKER, '--root'], { encoding: 'utf8' });
+  const out = `${r.stdout}${r.stderr}`;
+  assert.equal(r.status, 1, 'a missing --root argument must fail the gate');
+  assert.match(out, /--root requires a directory argument/);
+  assert.doesNotMatch(out, /ReferenceError/, 'must not die in the temporal dead zone');
+  assert.doesNotMatch(out, /\n\s+at /, 'must not surface a raw node stack');
+});

scripts/check-test-wiring.mjs

@@ -10,7 +10,7 @@
  * silently skips it and the suite never runs in CI (this happened to
  * @ifc-lite/ifcx and @ifc-lite/renderer — 13 test files dark for months).
  *
- * Part 2 — the same absence one directory over. `PACKAGE_DIRS` is
+ * Part 2 — the same absence one directory over. `PACKAGE_PARENTS` is
  * `packages` + `apps`, and `scripts/` is neither — yet `scripts/` is where
  * this repo keeps its gates. PR #3062 shipped a gate script AND its test
  * with no workflow step, no package.json script and no turbo task, and
@@ -84,6 +84,15 @@ import { readdirSync, readFileSync, existsSync, statSync } from 'node:fs';
 import { join, dirname, relative } from 'node:path';
 import { fileURLToPath } from 'node:url';
 import { stripYamlComments } from './lib/server-bin-targets-parse.mjs';
+import { listWorkspacePackages } from './lib/list-workspace-packages.mjs';
+
+// Deliberately a literal here rather than imported from the shared walk.
+// `scripts/lib/ci-path-coverage.mjs`'s `deriveInputs` reads only THIS file's own
+// source text and does not follow imports, so these two strings are what put
+// `packages/` and `apps/` into the CI-path-coverage census for this gate. Move
+// them into the lib and the ratchet silently stops checking that this gate can
+// be triggered by the paths it reads. (#3347)
+const PACKAGE_PARENTS = ['packages', 'apps'];
 
 const SCRIPT_DIR = dirname(fileURLToPath(import.meta.url));
 
@@ -95,7 +104,6 @@ export function fail(message) {
   throw new FailError(message);
 }
 
-const PACKAGE_DIRS = ['packages', 'apps'];
 const TEST_FILE_RE = /\.(test|spec)\.(ts|tsx|mts|js|mjs)$/;
 const SKIP_DIRS = new Set(['node_modules', 'dist', 'pkg', 'build', 'coverage', '.turbo']);
 
@@ -140,50 +148,34 @@ function findTestFiles(dir, found = []) {
 }
 
 /* ------------------------------------------------------------------ *
- * Part 1: packages/ and apps/ (unchanged behaviour)                    *
+ * Part 1: packages/ and apps/ — and the package discovery Part 2 shares  *
  * ------------------------------------------------------------------ */
 
 export function auditPackages(root) {
   const offenders = [];
-  let examined = 0;
-  let parentsSeen = 0;
-
-  for (const parent of PACKAGE_DIRS) {
-    const parentDir = join(root, parent);
-    if (!existsSync(parentDir)) continue;
-    parentsSeen++;
-    for (const name of readdirSync(parentDir)) {
-      const pkgDir = join(parentDir, name);
-      const pkgJsonPath = join(pkgDir, 'package.json');
-      if (!existsSync(pkgJsonPath)) continue;
-      examined++;
-      let pkgJson;
-      try {
-        pkgJson = JSON.parse(readFileSync(pkgJsonPath, 'utf-8'));
-      } catch (err) {
-        fail(`${pkgJsonPath} is not valid JSON: ${err.message}`);
-      }
-      if (pkgJson.scripts?.test) continue;
-      const testFiles = findTestFiles(pkgDir);
-      if (testFiles.length > 0) {
-        offenders.push({
-          name: pkgJson.name ?? `${parent}/${name}`,
-          example: relative(root, testFiles[0]).split('\\').join('/'),
-        });
-      }
+  const { packages, seenParents } = listWorkspacePackages(root, fail, PACKAGE_PARENTS);
+
+  for (const { rel, dir, pkgJson } of packages) {
+    if (pkgJson.scripts?.test) continue;
+    const testFiles = findTestFiles(dir);
+    if (testFiles.length > 0) {
+      offenders.push({
+        name: pkgJson.name ?? rel,
+        example: relative(root, testFiles[0]).split('\\').join('/'),
+      });
     }
   }
 
   // Anti-vacuity: "0 offenders" must mean "looked and found none", never
   // "looked in the wrong tree". Both of these are silent greens otherwise.
-  if (parentsSeen === 0) {
-    fail(`no search root found: none of ${PACKAGE_DIRS.map((d) => `${root}/${d}`).join(', ')} exists`);
+  if (seenParents.length === 0) {
+    fail(`no search root found: none of ${PACKAGE_PARENTS.map((d) => `${root}/${d}`).join(', ')} exists`);
   }
-  if (examined === 0) {
-    fail(`found no package.json under ${PACKAGE_DIRS.join('/ or ')}/ in ${root} — the package scan cannot be trusted`);
+  if (packages.length === 0) {
+    fail(`found no package.json under ${PACKAGE_PARENTS.join('/ or ')}/ in ${root} — the package scan cannot be trusted`);
   }
 
-  return { offenders, examined };
+  return { offenders, examined: packages.length };
 }
 
 /* ------------------------------------------------------------------ *
@@ -202,6 +194,7 @@ export function readWorkflows(root) {
       source = readFileSync(join(dir, name), 'utf8');
     } catch (err) {
       fail(`${join(dir, name)} could not be read: ${err.message}`);
+      throw err;
     }
     return { name, text: stripYamlComments(source) };
   });
@@ -299,23 +292,19 @@ export function reachableTaskNames(sources) {
   return tasks;
 }
 
-/** `{ [pkgRelPath]: scripts }` for every workspace package under packages/ and apps/. */
+/**
+ * `{ [pkgRelPath]: scripts }` for every workspace package under packages/ and
+ * apps/. A PROJECTION of the same `listWorkspacePackages` function the audit
+ * uses: one discovery FUNCTION with two readers. Still called once per reader,
+ * so it is a second walk of the same tree at run time. That is deliberate and
+ * cheap at this size (two parents, ~50 entries); the thing worth keeping is
+ * that both readers now agree on WHAT a package is, not that they share a pass.
+ */
 export function readWorkspaceScripts(root) {
-  const out = [];
-  for (const parent of PACKAGE_DIRS) {
-    const parentDir = join(root, parent);
-    if (!existsSync(parentDir)) continue;
-    for (const name of readdirSync(parentDir).sort()) {
-      const pkgJsonPath = join(parentDir, name, 'package.json');
-      if (!existsSync(pkgJsonPath)) continue;
-      try {
-        out.push({ rel: `${parent}/${name}`, scripts: JSON.parse(readFileSync(pkgJsonPath, 'utf8')).scripts ?? {} });
-      } catch (err) {
-        fail(`${pkgJsonPath} is not valid JSON: ${err.message}`);
-      }
-    }
-  }
-  return out;
+  return listWorkspacePackages(root, fail, PACKAGE_PARENTS).packages.map(({ rel, pkgJson }) => ({
+    rel,
+    scripts: pkgJson.scripts ?? {},
+  }));
 }
 
 /**
@@ -548,6 +537,7 @@ export function auditGateScripts(root, workflows, pkgScripts) {
       source = readFileSync(join(root, rel), 'utf8');
     } catch (err) {
       fail(`${join(root, rel)} could not be read: ${err.message}`);
+      throw err;
     }
     const reason = unwiredReason(source);
     if (reason === null) {
@@ -624,6 +614,7 @@ export function audit(root) {
     pkgScripts = JSON.parse(readFileSync(pkgJsonPath, 'utf8')).scripts ?? {};
   } catch (err) {
     fail(`${pkgJsonPath} is not valid JSON: ${err.message}`);
+    throw err;
   }
   const workflows = readWorkflows(root);
   return {