From a6f7c399e7ad4f76c9e14dc364e06cee7d2d5985 Mon Sep 17 00:00:00 2001 From: Shironex Date: Thu, 1 Oct 2026 21:58:39 +0200 Subject: [PATCH 1/2] fix(harness): return files only from ctx.glob Node's fs.globSync also returns matching directories, and vitest names its image snapshot directories .test.tsx, so a rule that globbed then read threw EISDIR. The files-only guarantee now lives in portableFileGlob, shared by the published ctx and the in-repo one, and is stated on IMetaCtx.glob. Closes #476 --- packages/harness/src/lint-meta/ctx.test.ts | 12 +++++++++ packages/harness/src/lint-meta/ctx.ts | 11 ++++---- packages/harness/src/lint-meta/glob.ts | 18 +++++++++++++ packages/harness/src/lint-meta/types.ts | 5 +++- tools/lint-meta/ctx.ts | 31 +++++----------------- 5 files changed, 47 insertions(+), 30 deletions(-) diff --git a/packages/harness/src/lint-meta/ctx.test.ts b/packages/harness/src/lint-meta/ctx.test.ts index a06f3be8..5bdd36c5 100644 --- a/packages/harness/src/lint-meta/ctx.test.ts +++ b/packages/harness/src/lint-meta/ctx.test.ts @@ -81,6 +81,18 @@ describe('createNodeCtx — real filesystem', () => { expect(ctx.glob('**/*.ts')).toEqual(['a.ts', 'src/b.ts', 'src/nested/c.ts']); }); + // #476: Node's globSync returns matching directories too, and vitest writes + // image snapshots into directories literally named `.test.tsx`. + test('glob returns files only, so every result is safe to read', () => { + mkdirSync(path.join(root, 'src', 'Button.test.tsx'), { recursive: true }); + writeFileSync(path.join(root, 'src', 'Button.test.tsx', 'shot.png'), ''); + writeFileSync(path.join(root, 'src', 'Real.test.tsx'), 'real'); + + expect(ctx.glob('src/*.tsx')).toEqual(['src/Real.test.tsx']); + expect(ctx.glob('src/*')).toEqual(['src/Real.test.tsx', 'src/b.ts', 'src/d.js']); + for (const rel of ctx.glob('**/*')) expect(ctx.read(rel)).not.toBeNull(); + }); + test('exec runs a command and captures output; it never throws', () => { const ok = ctx.exec('node -e "process.stdout.write(String(1+1))"'); expect(ok.code).toBe(0); diff --git a/packages/harness/src/lint-meta/ctx.ts b/packages/harness/src/lint-meta/ctx.ts index 011e76c9..e88f3ba5 100644 --- a/packages/harness/src/lint-meta/ctx.ts +++ b/packages/harness/src/lint-meta/ctx.ts @@ -4,9 +4,10 @@ * network), so it runs under plain `node` in a stranger's CI: * - `read` — `node:fs` + LF-normalization (line-based rules match CI), * - `exists`— `node:fs`, - * - `glob` — `portableGlob` (`glob.ts`): Node's `fs.globSync` under Node, and a - * walker with the same rules under Bun, whose `globSync` cannot see - * dot-directories (so `.github/workflows/*.yml` would match nothing), + * - `glob` — `portableFileGlob` (`glob.ts`): Node's `fs.globSync` under Node, and + * a walker with the same rules under Bun, whose `globSync` cannot see + * dot-directories (so `.github/workflows/*.yml` would match nothing). Files + * only: a matching directory is dropped, * - `exec` — `node:child_process` `spawnSync`, which NEVER throws. * * The shape mirrors `createFakeCtx` (the in-memory test double), so rules are @@ -16,7 +17,7 @@ import { spawnSync } from 'node:child_process'; import { existsSync, readFileSync } from 'node:fs'; import path from 'node:path'; -import { portableGlob } from './glob.js'; +import { portableFileGlob } from './glob.js'; import type { IMetaCtx } from './types.js'; /** Repo-relative path with forward slashes (the lint-meta internal convention). */ @@ -49,7 +50,7 @@ export function createNodeCtx(root: string): IMetaCtx { }, glob(pattern) { // Same cwd-relative, forward-slashed, sorted match set under Node and Bun. - return portableGlob(pattern, root); + return portableFileGlob(pattern, root); }, exec(cmd) { // spawnSync with `shell: true` mirrors the original `execSync` (a shell diff --git a/packages/harness/src/lint-meta/glob.ts b/packages/harness/src/lint-meta/glob.ts index a95727ad..6e2406b8 100644 --- a/packages/harness/src/lint-meta/glob.ts +++ b/packages/harness/src/lint-meta/glob.ts @@ -51,6 +51,24 @@ export function portableGlob(pattern: string, cwd: string): string[] { return [...new Set(matches)].sort(); } +/** + * {@link portableGlob}, files only. Node's `fs.globSync` returns matching + * DIRECTORIES as well as files, and a directory can carry a file-looking name + * (vitest writes image snapshots into directories literally named + * `.test.tsx`). A lint rule globs in order to read, so a directory in the + * result is never useful and always a latent `EISDIR`. A path that vanishes + * between the walk and the stat is dropped too. + */ +export function portableFileGlob(pattern: string, cwd: string): string[] { + return portableGlob(pattern, cwd).filter((rel) => { + try { + return statSync(path.join(cwd, rel)).isFile(); + } catch { + return false; + } + }); +} + /** A compiled pattern segment. */ type Segment = | { kind: 'globstar' } diff --git a/packages/harness/src/lint-meta/types.ts b/packages/harness/src/lint-meta/types.ts index 5743d7c3..0a3835c4 100644 --- a/packages/harness/src/lint-meta/types.ts +++ b/packages/harness/src/lint-meta/types.ts @@ -22,7 +22,10 @@ export interface IMetaCtx { read(rel: string): string | null; /** Whether a repo-relative path exists. */ exists(rel: string): boolean; - /** Glob repo-relative paths (cwd = {@link root}). */ + /** + * Glob repo-relative paths (cwd = {@link root}). FILES ONLY: a directory that + * matches the pattern is never returned, so every result is safe to `read`. + */ glob(pattern: string): string[]; /** Run a shell command at {@link root}; never throws. */ exec(cmd: string): { code: number; stdout: string; stderr: string }; diff --git a/tools/lint-meta/ctx.ts b/tools/lint-meta/ctx.ts index 83a6b046..12536f0b 100644 --- a/tools/lint-meta/ctx.ts +++ b/tools/lint-meta/ctx.ts @@ -4,19 +4,19 @@ * `root`. Split out of `cli.ts` so it can be exercised against a real tree in a * test (a fake ctx cannot show what the glob engine does on disk). * - * `glob` goes through `@noctcore/harness`'s `portableGlob` instead of Bun's + * `glob` goes through `@noctcore/harness`'s `portableFileGlob` instead of Bun's * `Glob`: Bun's globbing skips every dot-directory segment, even a literal one, * so `.github/workflows/*.yml` matched nothing and a rule globbing it would pass - * forever. `portableGlob` walks with Node's `fs.globSync` rules under Bun and + * forever. `portableFileGlob` walks with Node's `fs.globSync` rules under Bun and * throws on syntax it does not implement rather than matching nothing. It is * imported by relative path, like `tools/codegen` reads `packages/contracts`, * because the harness only publishes its type surface from the barrel. */ import { execSync } from 'node:child_process'; -import { existsSync, readFileSync, statSync } from 'node:fs'; +import { existsSync, readFileSync } from 'node:fs'; import path from 'node:path'; -import { portableGlob } from '../../packages/harness/src/lint-meta/glob.ts'; +import { portableFileGlob } from '../../packages/harness/src/lint-meta/glob.ts'; import { normalizeText, toPosixRel } from './paths'; import type { IMetaCtx } from './types'; @@ -32,27 +32,10 @@ export function createCtx(root: string): IMetaCtx { exists(rel) { return existsSync(path.join(root, toPosixRel(rel))); }, - /* - * Files only. `portableGlob` faithfully reproduces Node's `fs.globSync`, - * which returns matching DIRECTORIES as well as files; Bun's `Glob`, which - * this replaced, returned only files. That difference is not academic here: - * vitest writes its image snapshots into directories literally named - * `.test.tsx`, so `components/**\/*.tsx` matches 54 directories, and - * every rule that globs then reads threw `EISDIR` the moment the walker - * changed. - * - * A lint rule globs in order to read, so a directory in the result is never - * useful and always a latent crash. Filtering here keeps that guarantee for - * every rule rather than asking each one to remember. - */ + // Files only (see `portableFileGlob`): a rule globs in order to read, and a + // directory named like a file (`.test.tsx/` snapshot dirs) would EISDIR. glob(pattern) { - return portableGlob(pattern, root).filter((rel) => { - try { - return statSync(path.join(root, rel)).isFile(); - } catch { - return false; - } - }); + return portableFileGlob(pattern, root); }, exec(cmd) { try { From 59a60ede8baecb74a9e4f183e12accf2c1c4dbb6 Mon Sep 17 00:00:00 2001 From: Shironex Date: Thu, 1 Oct 2026 21:58:48 +0200 Subject: [PATCH 2/2] fix(harness): report a run where a rule threw as incomplete A rule that throws checked nothing, yet the summary still read "lint-meta: no violations". Both CLIs now end such a run with an INCOMPLETE line naming how many rules threw, and the in-repo CLI reds the build on any throw, not only for ciCritical rules. Closes #478 --- packages/harness/src/cli.ts | 5 +- packages/harness/src/lint-meta.test.ts | 17 +++++++ packages/harness/src/lint-meta/run.test.ts | 55 +++++++++++++++++++++- packages/harness/src/lint-meta/run.ts | 24 +++++++++- tools/lint-meta/README.md | 4 +- tools/lint-meta/cli.ts | 14 +++++- 6 files changed, 111 insertions(+), 8 deletions(-) diff --git a/packages/harness/src/cli.ts b/packages/harness/src/cli.ts index deb60b35..42dacb15 100644 --- a/packages/harness/src/cli.ts +++ b/packages/harness/src/cli.ts @@ -28,7 +28,7 @@ import { loadRegistry, type ModuleImporter, } from './lint-meta/registry.js'; -import { exitCodeFor, reportMetaOutcomes, runMetaRules } from './lint-meta/run.js'; +import { exitCodeFor, reportMetaOutcomes, runMetaRules, summaryLineFor } from './lint-meta/run.js'; import type { IMetaRule } from './lint-meta/types.js'; import { type FileReader,loadChecks, MANIFEST_RELATIVE_PATH } from './manifest.js'; import { emptyPass, fixInstruction, runChecks, type SpawnFn } from './run.js'; @@ -365,7 +365,8 @@ async function runLintMeta(parsed: ParsedArgs, io: CliIO): Promise { const report = reportMetaOutcomes(outcomes); for (const line of report.lines) io.stderr(line); - if (report.lines.length === 0) io.stdout('lint-meta: no violations'); + const summary = summaryLineFor(report); + if (summary !== null) (report.threwCount > 0 ? io.stderr : io.stdout)(summary); return exitCodeFor(report); } diff --git a/packages/harness/src/lint-meta.test.ts b/packages/harness/src/lint-meta.test.ts index b5dc3451..0e8c0c40 100644 --- a/packages/harness/src/lint-meta.test.ts +++ b/packages/harness/src/lint-meta.test.ts @@ -123,6 +123,22 @@ describe('runCli lint-meta — verdicts', () => { expect(h.err.join('\n')).toBe('[ERROR] no-todo (src/x.ts): found a TODO'); }); + test('a rule that throws reads as INCOMPLETE, never "no violations" (#478)', async () => { + const throwRule: IMetaRule = { + id: 'boom', + category: 'source-text', + description: 'always throws', + run: () => { + throw new Error('EISDIR: illegal operation on a directory, read'); + }, + }; + const h = harness({ present: [DEFAULT_REGISTRY], mod: { META_RULES: [passRule, throwRule] } }); + expect(await runCli(['lint-meta'], h.io)).toBe(1); + expect(h.out.join('\n')).not.toContain('no violations'); + expect(h.err.join('\n')).toContain('[ERROR] boom: rule threw'); + expect(h.err.join('\n')).toContain('lint-meta: INCOMPLETE, 1 of 2 rules threw'); + }); + test('a registry that fails to import reds the build (exit 1)', async () => { const h = harness({ present: [DEFAULT_REGISTRY], importThrows: new Error('boom') }); expect(await runCli(['lint-meta'], h.io)).toBe(1); @@ -173,6 +189,7 @@ describe('runCli lint-meta: async rules (#277)', () => { expect(h.err).toEqual([ '[ERROR] rejects: rule rejected — config did not resolve', '[ERROR] no-todo (src/x.ts): found a TODO', + 'lint-meta: INCOMPLETE, 1 of 2 rules threw and checked nothing (see above); 1 violation from the rest', ]); }); }); diff --git a/packages/harness/src/lint-meta/run.test.ts b/packages/harness/src/lint-meta/run.test.ts index 92c44371..4c81d28c 100644 --- a/packages/harness/src/lint-meta/run.test.ts +++ b/packages/harness/src/lint-meta/run.test.ts @@ -1,7 +1,7 @@ import { describe, expect, test } from 'bun:test'; import { createFakeCtx } from './create-fake-ctx.js'; -import { exitCodeFor, reportMetaOutcomes, runMetaRules } from './run.js'; +import { exitCodeFor, reportMetaOutcomes, runMetaRules, summaryLineFor } from './run.js'; import type { IMetaCtx, IMetaRule, IViolation } from './types.js'; const CTX: IMetaCtx = createFakeCtx({ files: { 'src/x.ts': 'contents' } }); @@ -21,7 +21,13 @@ describe('runMetaRules — capture, never abort', () => { test('a passing rule yields a clean outcome and no critical failure', async () => { const outcomes = await runMetaRules([rule({ id: 'ok', ciCritical: true, run: () => [] })], CTX); const report = reportMetaOutcomes(outcomes); - expect(report).toEqual({ criticalCount: 0, totalViolations: 0, lines: [] }); + expect(report).toEqual({ + criticalCount: 0, + totalViolations: 0, + lines: [], + ruleCount: 1, + threwCount: 0, + }); expect(exitCodeFor(report)).toBe(0); }); @@ -271,3 +277,48 @@ describe('runMetaRules: async rules (runAsync)', () => { expect(exitCodeFor(report)).toBe(0); }); }); + +describe('summaryLineFor: a run where a rule threw never reads as clean (#478)', () => { + const boom = rule({ + id: 'boom', + run: () => { + throw new Error('EISDIR'); + }, + }); + + test('a clean run says "no violations"', async () => { + const report = reportMetaOutcomes(await runMetaRules([rule({ id: 'ok', run: () => [] })], CTX)); + expect(summaryLineFor(report)).toBe('lint-meta: no violations'); + }); + + test('a throw with zero violations is INCOMPLETE, not clean', async () => { + const report = reportMetaOutcomes( + await runMetaRules([rule({ id: 'ok', run: () => [] }), boom], CTX), + ); + expect(report.totalViolations).toBe(0); + expect(summaryLineFor(report)).toBe( + 'lint-meta: INCOMPLETE, 1 of 2 rules threw and checked nothing (see above); 0 violations from the rest', + ); + }); + + test('a rule whose sync and async passes both fail counts once', async () => { + const both = rule({ + id: 'both', + run: () => { + throw new Error('sync'); + }, + runAsync: () => Promise.reject(new Error('async')), + }); + const report = reportMetaOutcomes(await runMetaRules([both], CTX)); + expect(report.criticalCount).toBe(2); + expect(report).toMatchObject({ ruleCount: 1, threwCount: 1 }); + expect(summaryLineFor(report)).toContain('1 of 1 rule threw'); + }); + + test('violations without a throw need no summary: the lines say it', async () => { + const report = reportMetaOutcomes( + await runMetaRules([rule({ id: 'soft', run: () => [violation({ rule: 'soft' })] })], CTX), + ); + expect(summaryLineFor(report)).toBeNull(); + }); +}); diff --git a/packages/harness/src/lint-meta/run.ts b/packages/harness/src/lint-meta/run.ts index d8f50a00..753a0d72 100644 --- a/packages/harness/src/lint-meta/run.ts +++ b/packages/harness/src/lint-meta/run.ts @@ -39,6 +39,10 @@ export interface MetaReport { totalViolations: number; /** The lines to print (violation + throw lines), in rule order. */ lines: string[]; + /** Distinct rules that ran at least one pass. */ + ruleCount: number; + /** Distinct rules with a pass that threw or rejected, so checked nothing. */ + threwCount: number; } function describeError(err: unknown): string { @@ -107,9 +111,13 @@ export function reportMetaOutcomes(outcomes: RuleOutcome[]): MetaReport { let criticalCount = 0; let totalViolations = 0; const lines: string[] = []; + const ran = new Set(); + const threw = new Set(); for (const outcome of outcomes) { + ran.add(outcome.id); if (outcome.threw !== null) { + threw.add(outcome.id); const verb = outcome.pass === 'async' ? 'rejected' : 'threw'; lines.push(`[ERROR] ${outcome.id}: rule ${verb} — ${outcome.threw}`); criticalCount += 1; @@ -123,7 +131,21 @@ export function reportMetaOutcomes(outcomes: RuleOutcome[]): MetaReport { } } - return { criticalCount, totalViolations, lines }; + return { criticalCount, totalViolations, lines, ruleCount: ran.size, threwCount: threw.size }; +} + +/** + * The closing line of a text run, or `null` when the violation lines already say + * it all. A run where any rule threw is INCOMPLETE and must never read as clean: + * that rule checked nothing, so "no violations" would be a claim nobody verified. + */ +export function summaryLineFor(report: MetaReport): string | null { + if (report.threwCount > 0) { + const rules = `${report.threwCount} of ${report.ruleCount} rule${report.ruleCount === 1 ? '' : 's'}`; + const rest = `${report.totalViolations} violation${report.totalViolations === 1 ? '' : 's'}`; + return `lint-meta: INCOMPLETE, ${rules} threw and checked nothing (see above); ${rest} from the rest`; + } + return report.totalViolations === 0 ? 'lint-meta: no violations' : null; } /** The process exit code a report implies: 1 on any critical failure, else 0. */ diff --git a/tools/lint-meta/README.md b/tools/lint-meta/README.md index e351a4e1..ff25ea83 100644 --- a/tools/lint-meta/README.md +++ b/tools/lint-meta/README.md @@ -21,7 +21,9 @@ Run it directly: bun run lint:meta # == bun run tools/lint-meta/cli.ts ``` -`lint-meta: no violations` on a clean tree means the gate is green. +`lint-meta: no violations` on a clean tree means the gate is green. A rule that +throws reds the build and ends the run with `lint-meta: INCOMPLETE, ...` instead: +that rule checked nothing, so the run never reads as clean. ### `--json` (machine-readable) diff --git a/tools/lint-meta/cli.ts b/tools/lint-meta/cli.ts index 78f9b8ca..63186075 100644 --- a/tools/lint-meta/cli.ts +++ b/tools/lint-meta/cli.ts @@ -68,11 +68,15 @@ if (process.argv.includes('--json')) { // Default: the human/CI text reporter — unchanged. let criticalCount = 0; let totalCount = 0; +let threwCount = 0; for (const { rule, outcome } of outcomes) { if (outcome.error !== null) { + // A rule that threw checked nothing, so it reds the build whether or not it + // is `ciCritical`: a broken guardrail is not a passing one. console.error(`[ERROR] ${rule.id}: rule threw — ${outcome.error}`); - criticalCount += rule.ciCritical ? 1 : 0; + threwCount += 1; + criticalCount += 1; continue; } for (const v of outcome.violations) { @@ -83,7 +87,13 @@ for (const { rule, outcome } of outcomes) { } } -if (totalCount === 0) { +// Never the bare "no violations" line when a rule threw: the run is incomplete. +if (threwCount > 0) { + console.error( + `lint-meta: INCOMPLETE, ${threwCount} of ${outcomes.length} rules threw and checked nothing (see above); ` + + `${totalCount} violation${totalCount === 1 ? '' : 's'} from the rest`, + ); +} else if (totalCount === 0) { console.log('lint-meta: no violations'); }