From 2a8305fbd37e18eabbefa55aa3ed155b23b99171 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Sun, 1 Nov 2026 16:11:07 -0800 Subject: [PATCH 1/2] feat(verify): report every fixture case in verify --json Each rule gains cases: [{ name, expected, ok, message?, files? }] for sg, vale and runtime, built from each runner's own per-case verdict. A failing case's message is produced by the same function as its errors line, and errors is unchanged. --- .changeset/verify-json-cases.md | 5 + packages/cli/src/rules/fixtures.ts | 176 ++++++++++++++---- packages/cli/src/rules/inspect.ts | 27 ++- .../cli/src/rules/runtime/run-fixtures.ts | 72 +++++-- packages/cli/src/rules/vale/verify.ts | 10 + packages/cli/src/rules/verify.ts | 11 ++ packages/cli/src/schemas/rule-check.ts | 46 +++++ .../cli/test/runtime-fixture-runner.test.ts | 63 +++++++ packages/cli/test/vale-verify.test.ts | 5 + .../cli/test/validate-verify-commands.test.ts | 84 +++++++++ 10 files changed, 451 insertions(+), 48 deletions(-) create mode 100644 .changeset/verify-json-cases.md diff --git a/.changeset/verify-json-cases.md b/.changeset/verify-json-cases.md new file mode 100644 index 00000000..12d45377 --- /dev/null +++ b/.changeset/verify-json-cases.md @@ -0,0 +1,5 @@ +--- +"@taskless/cli": patch +--- + +`taskless verify --json` now reports every fixture case under each rule's new `cases` array, for `sg`, `vale` and `runtime` alike: `{ name, expected, ok, message?, files? }`, where `name` is the case's path under `tests/` (such as `fail/cast`). A failing case carries the same line `errors` prints for it as `message`, and its files as `files` (project-relative paths, the spelling `findings[].file` uses; a file over 64 KiB or one that is not text is left out). The entries are built from each runner's own per-case verdict, so a consumer no longer has to parse `errors` to learn which case failed. The field is additive: `errors` is unchanged, and `cases` is always present, empty when no case ran (for `validate`, a refused runtime run, or a rule that failed before its fixtures ran). It is also declared in the schema `@taskless/cli/schemas` publishes. diff --git a/packages/cli/src/rules/fixtures.ts b/packages/cli/src/rules/fixtures.ts index 50f2520d..c21e8b00 100644 --- a/packages/cli/src/rules/fixtures.ts +++ b/packages/cli/src/rules/fixtures.ts @@ -1,5 +1,5 @@ import type { Dirent } from "node:fs"; -import { readdir } from "node:fs/promises"; +import { readdir, readFile, stat } from "node:fs/promises"; import { join, posix, relative, sep } from "node:path"; import { ruleDirectory, ruleTestsDirectory } from "./engines"; @@ -291,48 +291,158 @@ export function casePassed(outcome: CaseOutcome): boolean { } /** - * One error line per case that did not come out as its bucket asserts. + * Why one case did not come out as its bucket asserts, or `undefined` when it + * did. * * `scope` names what was in effect, in the rule's own terms (the `files:` * globs, or the Vale matchers), so a "scope excluded" message points at the * exact text an author would edit. + * + * The single source of a case's failure wording: `errors` and `cases[].message` + * are both built from it, so the two can never describe one case differently. */ +export function describeCaseFailure( + cwd: string, + outcome: CaseOutcome, + scope: string | undefined +): string | undefined { + if (casePassed(outcome)) return undefined; + const path = projectRelative(cwd, outcome.fixture.root); + if (outcome.unread === "scope-excluded") { + return ( + `${outcome.fixture.bucket} case was never read, because the rule's ` + + `scope excluded every file in it: ${path}/. No file sits at a path ` + + `admitted by ${scope ?? "the scope"}. Move the fixture to such a path.` + ); + } + if (outcome.unread === "not-walked") { + return ( + `${outcome.fixture.bucket} case was never read: ${path}/. The engine ` + + `walked no file in it, even with the scope removed: the project's ` + + `.gitignore or .ignore covers them, they are empty, or none is in a ` + + `language or format the engine reads. Rename the path or the file.` + ); + } + if (outcome.fixture.bucket === "pass") { + return `pass case wrongly fired: ${path}/`; + } + if (outcome.quiet === "scope-excluded") { + return ( + `fail case did not fire because the rule's scope excluded it: ${path}/. ` + + `It fires with the scope removed, so the pattern matches, and no ` + + `file in the case sits at a path admitted by ${scope ?? "the scope"}. ` + + `Move the fixture to such a path, or widen the scope.` + ); + } + return `fail case did not fire: ${path}/ (the rule did not match it)`; +} + +/** One error line per case that did not come out as its bucket asserts. */ export function describeCaseFailures( cwd: string, outcomes: readonly CaseOutcome[], scope: string | undefined ): string[] { - const errors: string[] = []; - for (const outcome of outcomes) { - if (casePassed(outcome)) continue; - const path = projectRelative(cwd, outcome.fixture.root); - if (outcome.unread === "scope-excluded") { - errors.push( - `${outcome.fixture.bucket} case was never read, because the rule's ` + - `scope excluded every file in it: ${path}/. No file sits at a path ` + - `admitted by ${scope ?? "the scope"}. Move the fixture to such a path.` - ); - } else if (outcome.unread === "not-walked") { - errors.push( - `${outcome.fixture.bucket} case was never read: ${path}/. The engine ` + - `walked no file in it, even with the scope removed: the project's ` + - `.gitignore or .ignore covers them, they are empty, or none is in a ` + - `language or format the engine reads. Rename the path or the file.` - ); - } else if (outcome.fixture.bucket === "pass") { - errors.push(`pass case wrongly fired: ${path}/`); - } else if (outcome.quiet === "scope-excluded") { - errors.push( - `fail case did not fire because the rule's scope excluded it: ${path}/. ` + - `It fires with the scope removed, so the pattern matches, and no ` + - `file in the case sits at a path admitted by ${scope ?? "the scope"}. ` + - `Move the fixture to such a path, or widen the scope.` - ); - } else { - errors.push( - `fail case did not fire: ${path}/ (the rule did not match it)` - ); + return outcomes.flatMap((outcome) => { + const message = describeCaseFailure(cwd, outcome, scope); + return message === undefined ? [] : [message]; + }); +} + +/** One file of a fixture case, as `verify --json` reports it. */ +export interface FixtureCaseFile { + /** Project-relative, `/`-separated: the same spelling as `findings[].file`. */ + path: string; + content: string; +} + +/** + * One fixture case's verdict, as `verify --json` reports it under `cases`. + * + * Structured so a consumer never has to recover a case from `errors`, whose + * wording is free to change. Built from the outcome each runner already + * judged, never from the error text. + */ +export interface FixtureCaseResult { + /** `/`, the case's path under `tests/`, e.g. `fail/cast`. */ + name: string; + /** The bucket's assertion: `fail` should make the rule fire, `pass` should not. */ + expected: FixtureBucket; + /** Whether the case came out as its bucket asserts. */ + ok: boolean; + /** Why it did not, when `ok` is false: the same line `errors` carries. */ + message?: string; + /** The case's files, on a failing case only. See {@link readCaseFiles}. */ + files?: FixtureCaseFile[]; +} + +/** Larger than any fixture worth reading in a report; such a file is left out. */ +const CASE_FILE_LIMIT_BYTES = 64 * 1024; + +/** + * A case's files, for a report reader who wants to see what failed. + * + * Text only and bounded: a file over {@link CASE_FILE_LIMIT_BYTES} or holding + * a NUL byte is left out rather than truncated, since a truncated fixture + * misrepresents the case it came from. Operating-system metadata is skipped + * for the reason {@link NOT_FIXTURES} gives. Never rejects: the files are + * context for a verdict already reached, so a file that cannot be read is + * left out rather than turning a report into a failure. + */ +export async function readCaseFiles( + cwd: string, + root: string +): Promise { + let entries: Dirent[]; + try { + entries = await readdir(root, { recursive: true, withFileTypes: true }); + } catch { + return []; + } + const files: FixtureCaseFile[] = []; + for (const entry of entries) { + if (entry.isDirectory() || NOT_FIXTURES.has(entry.name)) continue; + const absolute = join(entry.parentPath, entry.name); + try { + const { size } = await stat(absolute); + if (size > CASE_FILE_LIMIT_BYTES) continue; + const content = await readFile(absolute, "utf8"); + if (content.includes("\0")) continue; + files.push({ path: projectRelative(cwd, absolute), content }); + } catch { + continue; } } - return errors; + return files.toSorted((a, b) => a.path.localeCompare(b.path)); +} + +/** A case's verdict, with its files attached when it failed. */ +export async function caseResult( + cwd: string, + fixture: FixtureCase, + message: string | undefined +): Promise { + const result: FixtureCaseResult = { + name: `${fixture.bucket}/${fixture.name}`, + expected: fixture.bucket, + ok: message === undefined, + }; + if (message !== undefined) { + result.message = message; + result.files = await readCaseFiles(cwd, fixture.root); + } + return result; +} + +/** Every case's verdict, for the two engines whose check is a scan. */ +export function describeCaseResults( + cwd: string, + outcomes: readonly CaseOutcome[], + scope: string | undefined +): Promise { + return Promise.all( + outcomes.map((outcome) => + caseResult(cwd, outcome.fixture, describeCaseFailure(cwd, outcome, scope)) + ) + ); } diff --git a/packages/cli/src/rules/inspect.ts b/packages/cli/src/rules/inspect.ts index 9e83d877..18e35ca7 100644 --- a/packages/cli/src/rules/inspect.ts +++ b/packages/cli/src/rules/inspect.ts @@ -8,7 +8,11 @@ import { ruleDirectory, ruleFilePath, } from "./engines"; -import { describeCoverageShortfall, type FixtureFinding } from "./fixtures"; +import { + describeCoverageShortfall, + type FixtureCaseResult, + type FixtureFinding, +} from "./fixtures"; import { describeEmptySgScope } from "./sg-scope"; import { type EngineName } from "./layout"; import { @@ -132,6 +136,14 @@ export interface RuleVerification { * only ever produced by the runs that passed. */ findings: FixtureFinding[]; + /** + * Every fixture case's verdict, one entry per case. + * + * Empty, never absent, whenever the cases did not run, for `findings`' reason. + * Built from the outcome each runner judged rather than from `errors`, whose + * wording a consumer must not have to parse to learn which case failed. + */ + cases: FixtureCaseResult[]; } /** What `verify` needs beyond a rule, all of it about the runtime engine. */ @@ -441,7 +453,7 @@ export async function verifyOneRule( // call, which would run every case twice for one answer. const validation = await withIdCollision(cwd, ruleId, verdict); if (!validation.ok) { - return { ...validation, ran: false, findings: [] }; + return { ...validation, ran: false, findings: [], cases: [] }; } // A rule that populated only one bucket has proved only half of what a // rule claims, and the CLI says so: coverage is counted over case @@ -468,12 +480,13 @@ export async function verifyOneRule( // The findings that decided the verdict, as the scan reported them: // rendered messages, and `file` naming the fixture in its case. findings: result.tests.findings, + cases: result.tests.cases, }; } const validation = await validateOneRule(cwd, rule); if (!validation.ok) { - return { ...validation, ran: false, findings: [] }; + return { ...validation, ran: false, findings: [], cases: [] }; } if (engine === "vale") { @@ -488,6 +501,7 @@ export async function verifyOneRule( ran: false, notices: [], findings: [], + cases: [], }; } const errors: string[] = []; @@ -503,6 +517,7 @@ export async function verifyOneRule( ran: true, notices: result.notices, findings: result.findings, + cases: result.cases, }; } @@ -542,6 +557,7 @@ export async function verifyOneRule( violations: [], ran: false, findings: [], + cases: [], refused: reason, notices: [], }; @@ -567,6 +583,7 @@ export async function verifyOneRule( violations: [], ran: false, findings: [], + cases: [], refused: reason, notices: [], }; @@ -589,6 +606,7 @@ export async function verifyOneRule( ran: false, notices: [], findings: [], + cases: [], }; } @@ -596,7 +614,7 @@ export async function verifyOneRule( // run anything, and a warning about code that never executed is noise. options.onRuntimeWarning?.(RUN_SCRIPTS_WARNING); - const report = await runRuntimeFixtures(runtimeRule, fixtures); + const report = await runRuntimeFixtures(cwd, runtimeRule, fixtures); return { engine, ruleId, @@ -606,5 +624,6 @@ export async function verifyOneRule( ran: true, notices: [], findings: report.findings, + cases: report.cases, }; } diff --git a/packages/cli/src/rules/runtime/run-fixtures.ts b/packages/cli/src/rules/runtime/run-fixtures.ts index f71b1878..658c07ad 100644 --- a/packages/cli/src/rules/runtime/run-fixtures.ts +++ b/packages/cli/src/rules/runtime/run-fixtures.ts @@ -1,4 +1,9 @@ -import { describeCoverageShortfall, type FixtureFinding } from "../fixtures"; +import { + caseResult, + describeCoverageShortfall, + type FixtureCaseResult, + type FixtureFinding, +} from "../fixtures"; import type { RuntimeRule } from "./discover"; import { executeRuntimeRuleDetailed } from "./harness"; import type { RuntimeRunOptions } from "./harness"; @@ -57,6 +62,14 @@ export interface RuntimeFixtureReport { * crash to the rule — the exact confusion `checkFailures` exists to prevent. */ findings: FixtureFinding[]; + /** + * Every case's verdict, in the order the cases ran. + * + * The four lists above are the same information grouped by failure kind; + * this is it grouped by case, with the line `describeFixtureReport` prints + * for that case as its message. + */ + cases: FixtureCaseResult[]; } /** `fail/case-1` — the bucket is half the identity of a case. */ @@ -75,8 +88,12 @@ function label(fixtureCase: RuntimeFixtureCase): string { * * Cases run in sequence, matching `executeRuntimeRules`, so `tsx` worker * startup stays predictable. + * + * `cwd` is the project root, and is used only to name a failing case's files + * project-relative in `cases`, the spelling `findings[].file` uses. */ export async function runRuntimeFixtures( + cwd: string, rule: RuntimeRule, fixtures: RuntimeFixtures, options: RuntimeRunOptions = {} @@ -86,6 +103,10 @@ export async function runRuntimeFixtures( const neverInvoked: string[] = []; const checkFailures: FixtureCheckFailure[] = []; const findings: FixtureFinding[] = []; + const cases: FixtureCaseResult[] = []; + const record = async (fixtureCase: RuntimeFixtureCase, message?: string) => { + cases.push(await caseResult(cwd, fixtureCase, message)); + }; for (const fixtureCase of fixtures.cases) { const execution = await executeRuntimeRuleDetailed( @@ -98,15 +119,18 @@ export async function runRuntimeFixtures( // both arrive as one error-severity finding, so reading the findings alone // would count a crashed check as a `fail/` case that fired correctly. if (execution.failure !== undefined) { - checkFailures.push({ + const failure = { name: label(fixtureCase), message: execution.failure, - }); + }; + checkFailures.push(failure); + await record(fixtureCase, describeCheckFailure(failure)); continue; } if (!execution.invoked) { neverInvoked.push(label(fixtureCase)); + await record(fixtureCase, describeNeverInvoked(label(fixtureCase))); continue; } @@ -119,11 +143,17 @@ export async function runRuntimeFixtures( } if (fixtureCase.bucket === "fail") { - if (execution.findings.length === 0) + if (execution.findings.length === 0) { missingFailures.push(label(fixtureCase)); + await record(fixtureCase, describeMissingFailure(label(fixtureCase))); + continue; + } } else if (execution.findings.length > 0) { unexpectedFindings.push(label(fixtureCase)); + await record(fixtureCase, describeUnexpectedFinding(label(fixtureCase))); + continue; } + await record(fixtureCase); } return { @@ -142,9 +172,32 @@ export async function runRuntimeFixtures( neverInvoked, checkFailures, findings, + cases, }; } +// One line per kind of case failure. Shared by `describeFixtureReport` and the +// per-case `message`, so the two can never describe one case differently. + +function describeMissingFailure(name: string): string { + return `fail fixture did not fire: ${name}`; +} + +function describeUnexpectedFinding(name: string): string { + return `pass fixture wrongly fired: ${name}`; +} + +function describeNeverInvoked(name: string): string { + return ( + `fixture case matched no captures, so check.ts never ran: ${name}. ` + + `The case is the defect, not the check: add code the rule's captures match.` + ); +} + +function describeCheckFailure(failure: FixtureCheckFailure): string { + return `check failed on ${failure.name}: ${failure.message}`; +} + /** * The report as the lines `verify` prints under the rule. * @@ -162,19 +215,16 @@ export function describeFixtureReport( const shortfall = describeCoverageShortfall(ruleId, report.coverage); if (shortfall !== undefined) errors.push(shortfall); for (const name of report.missingFailures) { - errors.push(`fail fixture did not fire: ${name}`); + errors.push(describeMissingFailure(name)); } for (const name of report.unexpectedFindings) { - errors.push(`pass fixture wrongly fired: ${name}`); + errors.push(describeUnexpectedFinding(name)); } for (const name of report.neverInvoked) { - errors.push( - `fixture case matched no captures, so check.ts never ran: ${name}. ` + - `The case is the defect, not the check: add code the rule's captures match.` - ); + errors.push(describeNeverInvoked(name)); } for (const failure of report.checkFailures) { - errors.push(`check failed on ${failure.name}: ${failure.message}`); + errors.push(describeCheckFailure(failure)); } return errors; diff --git a/packages/cli/src/rules/vale/verify.ts b/packages/cli/src/rules/vale/verify.ts index c80aec26..efaae5cd 100644 --- a/packages/cli/src/rules/vale/verify.ts +++ b/packages/cli/src/rules/vale/verify.ts @@ -9,6 +9,8 @@ import { bucketEntries, casePassed, describeCaseFailures, + describeCaseResults, + type FixtureCaseResult, projectRelative, readRuleFixtures, type CaseOutcome, @@ -115,6 +117,8 @@ export interface ValeRuleVerification { unexpectedFindings: string[]; /** One line per case that did not come out as its bucket asserts, ready to print. */ errors: string[]; + /** Every case's verdict, built from the same outcomes as `errors`. */ + cases: FixtureCaseResult[]; /** * Every finding the fixtures produced, each tagged with its bucket. * @@ -252,6 +256,7 @@ export async function verifyValeRule( missingFailures: [], unexpectedFindings: [], errors, + cases: [], findings: [], fixtures, notices: [], @@ -378,6 +383,11 @@ export async function verifyValeRule( outcomes, "the rule's .vale.ini matchers" ), + cases: await describeCaseResults( + cwd, + outcomes, + "the rule's .vale.ini matchers" + ), findings: outcomes.flatMap((outcome) => outcome.findings), fixtures: read.coverage, notices: [...notices], diff --git a/packages/cli/src/rules/verify.ts b/packages/cli/src/rules/verify.ts index 56e561a4..2e644786 100644 --- a/packages/cli/src/rules/verify.ts +++ b/packages/cli/src/rules/verify.ts @@ -11,7 +11,9 @@ import { pathPrefixed, schemaLayer } from "../schemas/layer"; import { casePassed, describeCaseFailures, + describeCaseResults, readRuleFixtures, + type FixtureCaseResult, type FixtureCoverage, type FixtureFinding, } from "./fixtures"; @@ -87,6 +89,8 @@ export interface TestLayerResult extends LayerResult { fixtures: FixtureCoverage; /** Every finding the cases produced, each tagged with its bucket. */ findings: FixtureFinding[]; + /** Every case's verdict. Empty when the cases did not run. */ + cases: FixtureCaseResult[]; } export interface VerifyResult { @@ -423,6 +427,7 @@ async function runTests(cwd: string, ruleId: string): Promise { failed: 0, fixtures: run.coverage, findings, + cases: [], }; } const errors = describeCaseFailures(cwd, run.outcomes, run.scope); @@ -434,6 +439,7 @@ async function runTests(cwd: string, ruleId: string): Promise { failed: run.outcomes.length - passed, fixtures: run.coverage, findings, + cases: await describeCaseResults(cwd, run.outcomes, run.scope), }; } @@ -454,6 +460,7 @@ async function runTestLayer( failed: 0, fixtures: "none", findings: [], + cases: [], }; } if (hasTestFile) return runTests(cwd, ruleId); @@ -464,6 +471,7 @@ async function runTestLayer( failed: 0, fixtures: "none", findings: [], + cases: [], }; } @@ -502,6 +510,7 @@ export async function verifyRule( failed: 0, fixtures: "none", findings: [], + cases: [], }, }; } @@ -540,6 +549,7 @@ export async function verifyRule( failed: 0, fixtures: "none", findings: [], + cases: [], }, }; } @@ -564,6 +574,7 @@ export async function verifyRule( failed: 0, fixtures: "none", findings: [], + cases: [], }, }; } diff --git a/packages/cli/src/schemas/rule-check.ts b/packages/cli/src/schemas/rule-check.ts index 4ff3420b..ae4733cf 100644 --- a/packages/cli/src/schemas/rule-check.ts +++ b/packages/cli/src/schemas/rule-check.ts @@ -51,6 +51,46 @@ export const fixtureFindingSchema = z.object({ ), }); +/** + * One fixture case's verdict. + * + * What a consumer reads to learn WHICH case failed without parsing `errors`, + * whose wording is the CLI's to change. Built from the outcome each engine's + * runner judged, never recovered from the error text. + */ +export const fixtureCaseSchema = z.object({ + name: z + .string() + .describe( + "The case's path under the rule's tests/, as `/`, e.g. `fail/cast`" + ), + expected: z + .enum(["pass", "fail"]) + .describe( + "The bucket's assertion: `fail` should make the rule fire, `pass` should leave it quiet" + ), + ok: z.boolean().describe("Whether the case came out as its bucket asserts"), + message: z + .string() + .optional() + .describe( + "Why it did not, present only when `ok` is false. The same line `errors` carries for this case" + ), + files: z + .array( + z.object({ + path: z + .string() + .describe("Project-relative, the same spelling as `findings[].file`"), + content: z.string(), + }) + ) + .optional() + .describe( + "The case's files, present only when `ok` is false. A file over 64 KiB, or one that is not text, is left out rather than truncated" + ), +}); + const ruleResultSchema = z.object({ engine: z.enum(["sg", "vale", "runtime"]), ruleId: z.string(), @@ -97,6 +137,12 @@ const ruleResultSchema = z.object({ .describe( "The findings this rule's fixtures produced, each tagged with its bucket. ALWAYS PRESENT, empty rather than absent — for a rule that produced nothing, for one whose verification failed before its fixtures ran, for a refused run, for `validate`, which runs no fixtures at all, and for an engine whose fixture findings are not surfaced yet. A key that is sometimes absent is one a reader learns to treat as optional, and reads absence as zero. The `fail` bucket is reported on a passing run too: it is the evidence that the rendered messages say what their author meant, and a payload carrying that only once the rule is already failing carries it at the one moment it is no longer needed" ), + cases: z + .array(fixtureCaseSchema) + .default([]) + .describe( + "`verify` only: one entry per fixture case, failing or not, for every engine. ALWAYS PRESENT, empty rather than absent whenever the cases did not run — for `validate`, a refused run, and a rule whose verification failed before its fixtures ran — for `findings`' reason. Additive: `errors` still carries every failure line" + ), }); /** Output schema for `taskless validate --json` and `taskless verify --json`. */ diff --git a/packages/cli/test/runtime-fixture-runner.test.ts b/packages/cli/test/runtime-fixture-runner.test.ts index 4cb98efe..7a72fd4a 100644 --- a/packages/cli/test/runtime-fixture-runner.test.ts +++ b/packages/cli/test/runtime-fixture-runner.test.ts @@ -48,6 +48,13 @@ interface Report { errors: string[]; ran?: boolean; refused?: string; + cases: { + name: string; + expected: "pass" | "fail"; + ok: boolean; + message?: string; + files?: { path: string; content: string }[]; + }[]; }[]; } @@ -332,6 +339,62 @@ describe("fixture directions", () => { }); }); +/** + * `cases`: the per-case verdict a consumer reads instead of parsing `errors`. + * Asserted against the runner's own judgement, so a case's `message` must be + * the line `errors` carries for it, and the files must be the case's own. + */ +describe("per-case results", () => { + it("reports every case, with the failing one's reason and files", async () => { + await writeRule(FLAGS_EVERY_MATCH); + await writeCase("fail", "uses-eval"); + await writeCase("pass", "literal-eval"); + + const report = await testRuleJson("--dangerously-run-scripts"); + const rule = report.rules[0]; + + expect(rule?.cases).toEqual([ + { name: "fail/uses-eval", expected: "fail", ok: true }, + { + name: "pass/literal-eval", + expected: "pass", + ok: false, + message: "pass fixture wrongly fired: pass/literal-eval", + files: [ + { + path: `.taskless/rules/runtime/${RULE}/tests/pass/literal-eval/sample.ts`, + content: 'eval("1 + 1");\n', + }, + ], + }, + ]); + expect(rule?.errors).toContain(rule?.cases[1]?.message); + }); + + it("reports a case that never reached the check as failed", async () => { + await writeRule(); + await writeCase("fail", "unmatched", { matching: false }); + await writeCase("pass", "literal-eval"); + + const report = await testRuleJson("--dangerously-run-scripts"); + const unmatched = report.rules[0]?.cases.find( + (entry) => entry.name === "fail/unmatched" + ); + expect(unmatched?.ok).toBe(false); + expect(unmatched?.message).toContain("check.ts never ran"); + }); + + it("is empty, never absent, when the fixtures did not run", async () => { + await writeRule(); + await writeCase("fail", "uses-eval"); + await writeCase("pass", "literal-eval"); + + const report = await testRuleJson(); + expect(report.rules[0]?.ran).toBe(false); + expect(report.rules[0]?.cases).toEqual([]); + }); +}); + /** * 2.4 / 2.5 — a case that never reached the check. * diff --git a/packages/cli/test/vale-verify.test.ts b/packages/cli/test/vale-verify.test.ts index 6c725fa2..97780154 100644 --- a/packages/cli/test/vale-verify.test.ts +++ b/packages/cli/test/vale-verify.test.ts @@ -261,6 +261,11 @@ withVale("verifyValeRule", () => { missingFailures: [], unexpectedFindings: [], errors: [], + cases: [ + { name: "fail/a", expected: "fail", ok: true }, + { name: "fail/b", expected: "fail", ok: true }, + { name: "pass/c", expected: "pass", ok: true }, + ], fixtures: "both", notices: [], findings: [ diff --git a/packages/cli/test/validate-verify-commands.test.ts b/packages/cli/test/validate-verify-commands.test.ts index f933ddad..e3c350e8 100644 --- a/packages/cli/test/validate-verify-commands.test.ts +++ b/packages/cli/test/validate-verify-commands.test.ts @@ -37,6 +37,13 @@ interface Report { errors: string[]; violations: { constraintId: string; message: string }[]; notices: string[]; + cases: { + name: string; + expected: "pass" | "fail"; + ok: boolean; + message?: string; + files?: { path: string; content: string }[]; + }[]; }[]; } @@ -402,6 +409,83 @@ withVale("verify runs validate first", () => { }); }); +/** + * `cases` on `verify --json`: one entry per fixture case, built from the + * outcome the runner judged. The failing case's `message` is the line `errors` + * carries for it, so a consumer never has to parse `errors` to name the case. + */ +describe("verify reports every sg case", () => { + it("names the failing case, why, and what it held", async () => { + await sgRule("no-eval", "no-eval"); + const directory = join(cwd, ".taskless", "rules", "sg", "no-eval"); + await writeFile( + join(directory, "tests", "fail", "eval-call", "sample.ts"), + "evaluate(x);\n" + ); + + const result = await runCli(["verify", "-d", cwd, "--json"]); + expect(result.exitCode).not.toBe(0); + const rule = (JSON.parse(result.stdout) as Report).rules[0]; + const path = ".taskless/rules/sg/no-eval/tests/fail/eval-call"; + expect(rule?.cases).toEqual([ + { + name: "fail/eval-call", + expected: "fail", + ok: false, + message: `fail case did not fire: ${path}/ (the rule did not match it)`, + files: [{ path: `${path}/sample.ts`, content: "evaluate(x);\n" }], + }, + { name: "pass/plain", expected: "pass", ok: true }, + ]); + expect(rule?.errors).toContain(rule?.cases[0]?.message); + }); + + it("reports passing cases on a passing rule", async () => { + await sgRule("no-eval", "no-eval"); + const result = await runCli(["verify", "-d", cwd, "--json"]); + expect(result.exitCode).toBe(0); + expect((JSON.parse(result.stdout) as Report).rules[0]?.cases).toEqual([ + { name: "fail/eval-call", expected: "fail", ok: true }, + { name: "pass/plain", expected: "pass", ok: true }, + ]); + }); + + it("carries an empty list from validate, which runs no cases", async () => { + await sgRule("no-eval", "no-eval"); + const result = await runCli(["validate", "-d", cwd, "--json"]); + expect((JSON.parse(result.stdout) as Report).rules[0]?.cases).toEqual([]); + }); +}); + +withVale("verify reports every Vale case", () => { + it("names the pass case that fired, why, and what it held", async () => { + const directory = await valeRule("no-simply", { + config: SCOPED, + fixtures: true, + }); + await writeFile( + join(directory, "tests", "pass", "plain", "ok.md"), + "You simply do it.\n" + ); + + const result = await runCli(["verify", "-d", cwd, "--json"]); + expect(result.exitCode).not.toBe(0); + const rule = (JSON.parse(result.stdout) as Report).rules[0]; + const path = ".taskless/rules/vale/no-simply/tests/pass/plain"; + expect(rule?.cases).toEqual([ + { name: "fail/simply", expected: "fail", ok: true }, + { + name: "pass/plain", + expected: "pass", + ok: false, + message: `pass case wrongly fired: ${path}/`, + files: [{ path: `${path}/ok.md`, content: "You simply do it.\n" }], + }, + ]); + expect(rule?.errors).toContain(rule?.cases[1]?.message); + }); +}); + /** * The schema layer, through the real CLI. * From e8f235de57cf2af9dd487ef048e67f864a4b5483 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Sun, 1 Nov 2026 16:32:48 -0800 Subject: [PATCH 2/2] fix(verify): leave out case files that are not valid UTF-8, and pin both exclusions readCaseFiles decoded with readFile(..., "utf8"), which substitutes U+FFFD for invalid bytes, so a binary fixture without a NUL byte reached cases[].files mangled instead of being left out as documented. Decode strictly (BOM preserved) and cover the size boundary, NUL, invalid UTF-8 and files-only-on-failure in a unit test. --- packages/cli/src/rules/fixtures.ts | 21 ++++-- packages/cli/test/case-files.test.ts | 99 ++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 5 deletions(-) create mode 100644 packages/cli/test/case-files.test.ts diff --git a/packages/cli/src/rules/fixtures.ts b/packages/cli/src/rules/fixtures.ts index c21e8b00..111d241c 100644 --- a/packages/cli/src/rules/fixtures.ts +++ b/packages/cli/src/rules/fixtures.ts @@ -379,12 +379,22 @@ export interface FixtureCaseResult { /** Larger than any fixture worth reading in a report; such a file is left out. */ const CASE_FILE_LIMIT_BYTES = 64 * 1024; +/** + * Strict, so bytes that are not UTF-8 throw instead of decoding to U+FFFD, and + * BOM-preserving, so a file that decodes comes back exactly as it is on disk. + */ +const CASE_FILE_DECODER = new TextDecoder("utf-8", { + fatal: true, + ignoreBOM: true, +}); + /** * A case's files, for a report reader who wants to see what failed. * - * Text only and bounded: a file over {@link CASE_FILE_LIMIT_BYTES} or holding - * a NUL byte is left out rather than truncated, since a truncated fixture - * misrepresents the case it came from. Operating-system metadata is skipped + * Text only and bounded: a file over {@link CASE_FILE_LIMIT_BYTES}, holding + * a NUL byte, or not valid UTF-8 is left out rather than truncated or decoded + * with replacement characters, since either misrepresents the case it came + * from. Operating-system metadata is skipped * for the reason {@link NOT_FIXTURES} gives. Never rejects: the files are * context for a verdict already reached, so a file that cannot be read is * left out rather than turning a report into a failure. @@ -406,8 +416,9 @@ export async function readCaseFiles( try { const { size } = await stat(absolute); if (size > CASE_FILE_LIMIT_BYTES) continue; - const content = await readFile(absolute, "utf8"); - if (content.includes("\0")) continue; + const bytes = await readFile(absolute); + if (bytes.includes(0)) continue; + const content = CASE_FILE_DECODER.decode(bytes); files.push({ path: projectRelative(cwd, absolute), content }); } catch { continue; diff --git a/packages/cli/test/case-files.test.ts b/packages/cli/test/case-files.test.ts new file mode 100644 index 00000000..189b2c77 --- /dev/null +++ b/packages/cli/test/case-files.test.ts @@ -0,0 +1,99 @@ +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { caseResult, readCaseFiles } from "../src/rules/fixtures"; + +/** + * The files a failing case carries in `verify --json`. + * + * The integration suites only ever see small text fixtures, so the two + * exclusions (size and "not text") are pinned here directly: a regression in + * either would otherwise pass silently. + */ + +const LIMIT = 64 * 1024; + +let cwd: string; +let root: string; + +beforeEach(async () => { + cwd = await mkdtemp(join(tmpdir(), "taskless-case-files-")); + root = join(cwd, ".taskless", "rules", "demo", "tests", "fail", "case"); + await mkdir(root, { recursive: true }); +}); + +afterEach(async () => { + await rm(cwd, { recursive: true, force: true }); +}); + +describe("readCaseFiles", () => { + it("keeps a file at exactly the size limit and drops one a byte over", async () => { + await writeFile(join(root, "at-limit.txt"), "a".repeat(LIMIT)); + await writeFile(join(root, "over-limit.txt"), "a".repeat(LIMIT + 1)); + + const files = await readCaseFiles(cwd, root); + + expect(files.map((file) => file.path)).toEqual([ + ".taskless/rules/demo/tests/fail/case/at-limit.txt", + ]); + }); + + it("drops a file holding a NUL byte", async () => { + await writeFile(join(root, "nul.bin"), Buffer.from("text\0more")); + await writeFile(join(root, "kept.ts"), "const a = 1;\n"); + + const files = await readCaseFiles(cwd, root); + + expect(files).toEqual([ + { + path: ".taskless/rules/demo/tests/fail/case/kept.ts", + content: "const a = 1;\n", + }, + ]); + }); + + it("drops a file that is not valid UTF-8 rather than decoding it lossily", async () => { + await writeFile( + join(root, "latin1.txt"), + Buffer.from([0x63, 0x61, 0xff, 0x65]) + ); + + expect(await readCaseFiles(cwd, root)).toEqual([]); + }); + + it("returns a file's content byte for byte, BOM included", async () => { + await writeFile(join(root, "bom.ts"), "const é = 1;\n"); + + const files = await readCaseFiles(cwd, root); + + expect(files[0]?.content).toBe("const é = 1;\n"); + }); +}); + +function fixture() { + return { bucket: "fail" as const, name: "case", root }; +} + +describe("caseResult", () => { + it("attaches files only to a failing case", async () => { + await writeFile(join(root, "a.ts"), "x\n"); + + // eslint-disable-next-line unicorn/no-useless-undefined -- the verdict is a required parameter, and undefined is what "passed" means + const passing = await caseResult(cwd, fixture(), undefined); + const failing = await caseResult(cwd, fixture(), "fail/case: no finding"); + + expect(passing).toEqual({ name: "fail/case", expected: "fail", ok: true }); + expect(failing).toEqual({ + name: "fail/case", + expected: "fail", + ok: false, + message: "fail/case: no finding", + files: [ + { path: ".taskless/rules/demo/tests/fail/case/a.ts", content: "x\n" }, + ], + }); + }); +});