From b74abf0f7acd610cce7e7cd1d10ee61aa0837822 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Sun, 23 Aug 2026 21:29:52 -0700 Subject: [PATCH 1/4] fix(cli): stop one AsciiDoc file from disabling every Vale rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Vale supports AsciiDoc, reStructuredText, XML/DITA and MDX but parses none of them itself — it shells out to asciidoctor, rst2html, dita or mdx2vast, and the @taskless/vale-* packages ship the binary with none of those alongside it. Without the converter Vale does not skip the file: it writes one E100 to stderr, nothing at all to stdout, and exits 2. The abort is Vale's own and it is not scoped to the offending file, so every finding from every other file in the run was destroyed before serialization. Measured against the example project, one .adoc took a check reporting five Vale findings across four files down to zero. runVale now excludes the converter-dependent extensions from Vale's walk and names the skipped files in a notice that says which converter would put them back in scope. The tiers live in one table in rules/vale/formats.ts, measured against the pinned binary rather than transcribed — which is how .asc, a third AsciiDoc spelling absent from the bug report, got covered. A per-extension test re-measures every entry against the real Vale, so a version bump that moves a format between tiers fails there instead of silently turning the engine off again. Two details are load-bearing: Vale honours exactly one --glob and keeps the last, so the .taskless/ and format exclusions must travel as one negated alternation; and Vale matches a --glob against the basename only when the pattern contains no `/`, so a bare *.adoc branch inside that alternation stops matching docs/guide.adoc and the crash survives one directory down. Vale's error output is also decoded rather than forwarded, so a failure reads as a sentence naming the missing program. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Cyga14bww8rmazH2XrF8ms --- .changeset/vale-converter-formats.md | 38 +++ packages/cli/src/rules/vale/formats.ts | 287 ++++++++++++++++ packages/cli/src/rules/vale/run.ts | 112 +++++- packages/cli/test/vale-formats.test.ts | 337 +++++++++++++++++++ packages/cli/test/vale-orchestration.test.ts | 39 +++ 5 files changed, 795 insertions(+), 18 deletions(-) create mode 100644 .changeset/vale-converter-formats.md create mode 100644 packages/cli/src/rules/vale/formats.ts create mode 100644 packages/cli/test/vale-formats.test.ts diff --git a/.changeset/vale-converter-formats.md b/.changeset/vale-converter-formats.md new file mode 100644 index 00000000..5f347eaf --- /dev/null +++ b/.changeset/vale-converter-formats.md @@ -0,0 +1,38 @@ +--- +"@taskless/cli": patch +--- + +Stop one AsciiDoc file from disabling every Vale rule in the project. + +Vale supports AsciiDoc, reStructuredText, XML/DITA and MDX, but it parses none +of them by itself — it shells out to `asciidoctor`, `rst2html`, `dita` or +`mdx2vast`, and the `@taskless/vale-*` packages ship the binary with none of +those alongside it. On a host without the converter Vale does not skip the file: +it prints one `E100 [lintAdoc] Runtime error` on stderr, writes nothing at all to +stdout, and exits 2. The abort is Vale's own and it is not scoped to the file +that caused it, so every finding from every other file in the run was destroyed +before it was ever serialized. Measured against the example project, adding a +single `.adoc` took a check that reported five Vale findings across four files +down to zero — reported as a raw JSON blob among the results, and exiting 1 the +same way any ordinary failing check does. + +`runVale` now excludes the converter-dependent extensions from Vale's own walk, +so the rest of the project is checked normally and the skipped files are named +in a notice that says which converter would put them back in scope. The tiers +live in one table in `rules/vale/formats.ts`, measured against the pinned binary +rather than transcribed from documentation — which is how `.asc`, a third +AsciiDoc spelling that crashes identically and was not in the bug report, ended +up covered. A per-extension test re-measures every entry against the real Vale, +so a version bump that moves a format between tiers fails there instead of +silently turning the engine off again. + +Two details are load-bearing and were both wrong on the first attempt. Vale +honours exactly one `--glob` and keeps the last, so the `.taskless/` exclusion +and the format exclusions have to travel as one negated alternation or the first +is silently discarded. And Vale matches a `--glob` against the basename only +when the pattern contains no `/` — combined with `.taskless/**` the whole +expression goes path-wise, at which point a bare `*.adoc` stops matching +`docs/guide.adoc` and the crash survives one directory down from wherever it was +tested. Vale's error output is also decoded now rather than forwarded verbatim, +so a failure reads as a sentence naming the missing program instead of a +five-field JSON object. diff --git a/packages/cli/src/rules/vale/formats.ts b/packages/cli/src/rules/vale/formats.ts new file mode 100644 index 00000000..36072894 --- /dev/null +++ b/packages/cli/src/rules/vale/formats.ts @@ -0,0 +1,287 @@ +import { glob } from "node:fs/promises"; +import { basename, extname } from "node:path"; + +/** + * Taskless's own directory, as a project-relative path. + * + * Lives here rather than in `run.ts` because both the exclusion glob and the + * notice walk have to agree on what "ours, not the user's prose" means, and two + * copies of that string is how they stop agreeing. + */ +export const TASKLESS_DIRECTORY = ".taskless"; + +/** + * Which markup formats this build of Vale can actually parse. + * + * Vale supports AsciiDoc, reStructuredText, XML/DITA and (before 3.18) MDX + * upstream — but not on its own. For those formats it shells out to an external + * program to convert the source into something it can lint, and the + * `@taskless/vale-*` platform packages ship the Vale binary as pure payload + * with none of those programs alongside it. On a host without the converter the + * conversion fails, and Vale does not degrade: it prints one + * `E100 [lintAdoc] Runtime error` object on stderr, writes **nothing** to + * stdout, and exits 2. The abort is Vale's, not ours, and it is not scoped to + * the offending file — every finding from every other file in the same run is + * lost inside Vale before it is ever serialized. + * + * So a single `.adoc` anywhere a rule's matcher reaches turned the whole Vale + * engine off. `check` reported the crash, but as a raw JSON blob among the + * findings, and the exit code looked the same as any other failing check — so + * in a repo that already had an ast-grep finding, "every Vale rule stopped + * running" was indistinguishable from a normal red check. + * + * ## Asserting known support rather than dodging known breakage + * + * The tiers below are **measured against the pinned binary**, not read off + * Vale's documentation, and `vale-formats.test.ts` re-measures every entry + * against the real binary on every run. A format whose tier changes — or a + * converter Vale starts requiring for a format we currently call native — turns + * that test red before it can turn a user's check silently green. + * + * The reason the operative list is the *converter* tier rather than the native + * one deserves stating, because "allowlist what we know works" reads like it + * should be the other way round. Vale does not lint markup only. Anything it + * does not recognize as markup it reads as plain text or as source-code + * comments — `.py`, `.ts`, `.yml`, `.txt`, and files with no extension at all + * (`README`, `LICENSE`, `Makefile`) all get linted, and none of them can shell + * out. An allowlist expressed as "only hand Vale these extensions" would + * therefore have to enumerate every language Vale knows *and* would still drop + * every extensionless file — trading a loud crash for exactly the silent + * disabling this whole module exists to prevent, over a far larger set of + * files. + * + * The safe path and the unknown path are the same path here, which is what + * makes an exclusion honest rather than a denylist with a nice name: shelling + * out is a property of a short, closed set of markup formats, and an extension + * outside that set falls through to Vale's plain-text reader, which has no + * converter to be missing. `MARKUP_FORMAT_TIERS` is the assertion — it names + * what we measured and what we concluded — and the exclusion is derived from + * it, so the two cannot drift. + * + * **That property is pinned to the vendored binary, and a version bump is what + * breaks it.** "Unknown to us" is safe only while it also means "unknown to + * Vale": the moment Vale learns a format, it starts routing that extension to a + * parser, and if that parser shells out, an extension missing from this table is + * a crash rather than a plain-text read. Vale 3.18.0 is the live example — it + * added Typst, which converts through `typst2vast`, so upgrading the + * `@taskless/vale-*` packages without re-measuring would reintroduce exactly + * this bug under a new extension. Re-measure the table on every bump; the + * per-extension cases in `vale-formats.test.ts` are how. + * + * TODO(capabilities): `packages/cli/src/rules/capabilities.ts` is being built + * in parallel to hold pinned engine-capability constants, Vale format tiers + * among them. This table is the single place those tiers live today; move it + * there wholesale at integration rather than copying entries out of it. + */ +export type ValeFormatTier = + /** Vale parses it in-process. Safe to hand over. */ + | "native" + /** Vale shells out to a program we do not ship. Must not be handed over. */ + | "external-converter"; + +/** One markup extension, the tier we measured it in, and why. */ +export interface ValeFormatSupport { + tier: ValeFormatTier; + /** + * The program Vale invokes, for `external-converter` entries. Named in the + * user-facing notice so "skipped" comes with something to act on. + */ + converter?: string; +} + +/** + * Every markup extension Vale routes to a syntax-aware parser, tiered. + * + * Measured against `@taskless/vale-*` 3.17.1 by linting a one-line file per + * extension under a `[*]` matcher and recording whether Vale returned findings + * or aborted with `E100`. + * + * `.asc` is the entry worth pointing at: it is a third AsciiDoc spelling, it + * crashes exactly like `.adoc`, and it was not in the bug report. It is here + * because the tiers were measured rather than transcribed. + */ +export const MARKUP_FORMAT_TIERS: Readonly> = + { + ".md": { tier: "native" }, + ".markdown": { tier: "native" }, + ".mdown": { tier: "native" }, + ".mkdn": { tier: "native" }, + ".mkd": { tier: "native" }, + ".html": { tier: "native" }, + ".htm": { tier: "native" }, + ".xhtml": { tier: "native" }, + ".org": { tier: "native" }, + ".tex": { tier: "native" }, + ".rmd": { tier: "native" }, + ".adoc": { tier: "external-converter", converter: "asciidoctor" }, + ".asciidoc": { tier: "external-converter", converter: "asciidoctor" }, + ".asc": { tier: "external-converter", converter: "asciidoctor" }, + ".rst": { tier: "external-converter", converter: "rst2html" }, + ".rest": { tier: "external-converter", converter: "rst2html" }, + ".xml": { tier: "external-converter", converter: "an XSLT stylesheet" }, + ".dita": { tier: "external-converter", converter: "dita" }, + ".mdx": { tier: "external-converter", converter: "mdx2vast" }, + }; + +/** + * Extensions Vale must never be handed, lowercase, leading dot, sorted. + * + * Derived from the tier table rather than written out again, so adding a + * measured entry there is the whole change. + */ +export const CONVERTER_DEPENDENT_EXTENSIONS: readonly string[] = Object.entries( + MARKUP_FORMAT_TIERS +) + .filter(([, support]) => support.tier === "external-converter") + .map(([extension]) => extension) + .toSorted(); + +/** + * The converter Vale would need for `path`, or `undefined` if it needs none. + * + * Extension comparison is lowercased: `README.RST` is a reStructuredText file + * to Vale on a case-insensitive filesystem, and letting case decide would make + * the crash reappear on exactly one platform. + */ +export function converterFor(path: string): string | undefined { + return MARKUP_FORMAT_TIERS[extname(path).toLowerCase()]?.converter; +} + +/** + * Glob patterns, in Vale's dialect, that exclude the converter-dependent files. + * + * A globstar-prefixed `*.adoc` rather than a bare one, and this is not + * cosmetic. Vale matches a `--glob` against the file's **basename** when the + * pattern contains no `/`, and against its path when it does. Every pattern + * here is combined into one + * alternation with `.taskless/**`, which contains a `/` — so the whole + * expression is matched path-wise, and a bare `*.adoc` branch then stops + * matching `docs/guide.adoc`. Measured: that exact combination still crashed on + * a nested file while excluding the root-level one, which is the worst possible + * shape of bug — it looks fixed in the repository you tested it in. + */ +export function converterExclusionGlobs(): string[] { + return CONVERTER_DEPENDENT_EXTENSIONS.map((extension) => `**/*${extension}`); +} + +/** + * The single `--glob` expression for a run, or `undefined` when there is + * nothing to exclude. + * + * One expression because Vale accepts one `--glob` and the last one wins: + * passing two flags silently drops the first, so the exclusions have to be one + * negated alternation or they are not exclusions at all. + */ +export function buildValeGlob(patterns: string[]): string | undefined { + if (patterns.length === 0) return undefined; + return `--glob=!{${patterns.join(",")}}`; +} + +/** How many skipped paths a notice names before it summarizes the rest. */ +const NOTICE_SAMPLE_LIMIT = 5; + +/** Directories never worth walking to build a notice. */ +const UNWALKED_DIRECTORIES = new Set([ + "node_modules", + ".git", + TASKLESS_DIRECTORY, +]); + +/** + * Converter-dependent files inside the run's target set. + * + * This exists only so the skip can be *named*. The fix itself needs no file + * list — Vale's own walker does the excluding — but a fix whose entire user + * experience is "some findings are quietly not there" would be the bug again + * one layer down, so the notice has to say which files and which converter. + * + * Two things it deliberately does not do. It does not reconstruct Vale's walk: + * it asks a much narrower question (are there files with these eight + * extensions?) whose answer is a notice, never a finding, so being approximate + * costs a slightly vague message and nothing else. And it does not run when + * there is nothing to say — the common repository has no AsciiDoc at all, and + * `glob` over a pruned tree returning empty is the whole cost in that case. + * + * Known imprecision, and it is one-directional: Node's `glob` does not descend + * into dot-directories, so a `.github/adr/0001.adoc` is skipped by Vale and + * goes unnamed here. That under-reports a notice; it never suppresses a + * finding, and it never lets the crash back in. + */ +export async function findConverterDependentFiles( + cwd: string, + paths: string[] +): Promise { + const extensions = CONVERTER_DEPENDENT_EXTENSIONS.map((extension) => + extension.slice(1) + ).join(","); + + // An explicitly named file answers by its own name; only a directory needs + // walking. A whole-project run has one target, the project. + const named: string[] = []; + const roots: string[] = []; + if (paths.length === 0) { + roots.push("."); + } else { + for (const path of paths) { + if (converterFor(path) !== undefined) named.push(path); + roots.push(path); + } + } + + const found = new Set(named); + for (const root of roots) { + const prefix = root === "." || root === "" ? "" : `${root}/`; + try { + for await (const match of glob(`${prefix}**/*.{${extensions}}`, { + cwd, + exclude: (entry) => UNWALKED_DIRECTORIES.has(basename(String(entry))), + })) { + found.add(match); + } + } catch { + // A target that is not a directory, an unreadable subtree, a platform + // where `glob` rejects the pattern: all of them mean "no notice", never + // "no fix". The exclusion has already been applied by the time this runs. + } + } + + return [...found].toSorted(); +} + +/** + * The user-facing sentence for a set of skipped files, or `undefined` when + * nothing was skipped. + * + * Phrased as "this build cannot parse", not "Vale does not support": Vale + * supports every one of these formats, and telling a user otherwise sends them + * to the wrong project's issue tracker. It names the converter because that is + * the one thing they can act on — installing `asciidoctor` puts the files back + * in scope with no change on our side. + */ +export function skippedFilesNotice(files: string[]): string | undefined { + if (files.length === 0) return undefined; + + const converters = [ + ...new Set( + files.flatMap((file) => { + const converter = converterFor(file); + return converter === undefined ? [] : [converter]; + }) + ), + ].toSorted(); + + const sample = files.slice(0, NOTICE_SAMPLE_LIMIT); + const remainder = files.length - sample.length; + const listed = + remainder > 0 + ? `${sample.join(", ")} (and ${String(remainder)} more)` + : sample.join(", "); + + return ( + `Vale did not check ${String(files.length)} file(s): ${listed}. Vale ` + + `supports these formats, but parsing them needs an external converter ` + + `(${converters.join(", ")}) that this build does not ship. Install it and ` + + `put it on your PATH to have these files checked; every other file was ` + + `checked normally.` + ); +} diff --git a/packages/cli/src/rules/vale/run.ts b/packages/cli/src/rules/vale/run.ts index 0b7fd9a2..d9b2063b 100644 --- a/packages/cli/src/rules/vale/run.ts +++ b/packages/cli/src/rules/vale/run.ts @@ -7,11 +7,15 @@ import { ASSEMBLED_VALE_CONFIG } from "../engines"; import { buildPath } from "../scan"; import { findValeBinary, valeUnavailableMessage } from "./binary"; +import { + buildValeGlob, + converterExclusionGlobs, + findConverterDependentFiles, + skippedFilesNotice, + TASKLESS_DIRECTORY, +} from "./formats"; import { asValeConfigError, toValeCheckResults, type ValeOutput } from "./map"; -/** Taskless's own directory, as a project-relative path. */ -const TASKLESS_DIRECTORY = ".taskless"; - /** * The Vale config a run reads, relative to the project root. * @@ -83,6 +87,44 @@ export type ValeRunOutcome = | { status: "timeout"; blocking: true; message: string } | { status: "failed"; blocking: true; message: string }; +/** + * Vale's stderr, rendered as a sentence instead of a JSON blob. + * + * Vale reports its own errors as a one-object JSON document on stderr — + * `{Line, Path, Text, Code, Span}` with `Text` carrying embedded newlines. Piped + * straight into a failure message that lands amid a check's findings, it reads + * as a stack trace: the actionable half (`asciidoctor not found`) is four lines + * into a structure whose other four fields say nothing. Same reasoning as + * decoding ast-grep's stderr rather than forwarding bytes — the message is the + * only thing the user has to act on. + * + * Anything that is not that shape is returned untouched. A best-effort decoder + * that swallows what it cannot read would be worse than none. + */ +function describeValeStderr(stderr: string): string { + let parsed: unknown; + try { + parsed = JSON.parse(stderr); + } catch { + return stderr; + } + const error = asValeConfigError(parsed); + if (error === undefined) return stderr; + + const text = error.Text.split("\n") + .map((line) => line.trim()) + .filter((line) => line !== "") + .join(" "); + // The code stays in the message, and is prepended only when `Text` does not + // already open with it. Vale is inconsistent about that — `E100` repeats the + // code in its text and `E201` does not — and the code is what a user searches + // for, so losing it while "improving" the message would be a downgrade. + const code = text.startsWith(error.Code) ? "" : `${error.Code}: `; + return `${code}${text}${ + error.Path === undefined || error.Path === "" ? "" : ` in ${error.Path}` + }`; +} + export interface ValeRunOptions { /** Project root. Vale runs here, so the config's relative paths resolve. */ cwd: string; @@ -131,18 +173,42 @@ export async function runVale( const wholeProject = paths.length === 0; const targets = wholeProject ? ["."] : paths; - // Walking the whole project reaches `.taskless/` too, and Vale has no reason - // to know that directory is ours: with a rule enabled it reports findings in - // the rule configs and in the user's own rule definitions — prose - // complaints about the machinery, pointing at files nobody wrote as prose. - // Section globs do not help, since `.taskless/README.md` matches `[*.md]` as - // readily as any document. `--glob` filters which files are walked without - // touching which rules apply to them, so a user's scoping still decides that. + // Two exclusions reach Vale, and they have to travel together because Vale + // accepts exactly one `--glob` and the last one wins — pass two flags and the + // first is silently discarded, which is how an exclusion becomes a no-op that + // still looks applied on the command line. // - // Applied ONLY when we chose `.` ourselves. An explicit path is a request, - // and silently declining to check a file someone named would be worse than + // `.taskless/**` keeps Vale out of our own directory. Walking the whole + // project reaches it too, and Vale has no reason to know that directory is + // ours: with a rule enabled it reports findings in the rule configs and in the + // user's own rule definitions — prose complaints about the machinery, pointing + // at files nobody wrote as prose. Section globs do not help, since + // `.taskless/README.md` matches `[*.md]` as readily as any document. Applied + // ONLY when we chose `.` ourselves: an explicit path is a request, and + // silently declining to check a file someone named would be worse than // checking one they did not. - const exclude = wholeProject ? [`--glob=!${TASKLESS_DIRECTORY}/**`] : []; + // + // The converter-dependent formats are excluded on **every** run, named path or + // not, and that asymmetry is deliberate. Handing Vale one `.adoc` on a host + // with no `asciidoctor` does not check that file badly — it aborts the entire + // Vale process before any result is written, taking every other file's + // findings with it. Honouring the request would cost the user the rest of + // their check, so the request is declined and reported instead. See + // `formats.ts` for the tier table this is derived from. + const exclude = [ + ...(wholeProject ? [`${TASKLESS_DIRECTORY}/**`] : []), + ...converterExclusionGlobs(), + ]; + const globArgument = buildValeGlob(exclude); + const globFlags = globArgument === undefined ? [] : [globArgument]; + + // Asked before the run rather than inferred from it. Vale never reports what + // its walker declined to open, so once the glob has done its job the skipped + // files are unrecoverable from the output — and a fix whose only visible + // effect is that some findings are quietly missing is the bug it replaced. + const skipped = skippedFilesNotice( + await findConverterDependentFiles(options.cwd, paths) + ); // `--` separates flags from positional paths, so a path beginning with `-` // is not read as a flag. @@ -151,7 +217,7 @@ export async function runVale( configPath, "--output=JSON", "--no-exit", - ...exclude, + ...globFlags, "--", ...targets, ]; @@ -231,7 +297,9 @@ export async function runVale( settle({ status: "failed", blocking: true, - message: `Vale exited ${String(code)}${stderr === "" ? "" : `: ${stderr}`}`, + message: `Vale exited ${String(code)}${ + stderr === "" ? "" : `: ${describeValeStderr(stderr)}` + }`, }); return; } @@ -241,10 +309,18 @@ export async function runVale( // Attached to every `ok` path so a diagnostic cannot be dropped by which // branch happened to produce the (empty) results. const diagnostic = stderrChunks.join("").trim(); + // Both advisories share one field, so they are joined rather than one + // overwriting the other: a project can perfectly well have a section-less + // rule assignment *and* an AsciiDoc file, and dropping either message + // would be a silent skip wearing the other's clothes. + const advisories = [ + ...(skipped === undefined ? [] : [skipped]), + ...(diagnostic === "" + ? [] + : [`Vale reported while running: ${diagnostic}`]), + ]; const notice = - diagnostic === "" - ? {} - : { notice: `Vale reported while running: ${diagnostic}` }; + advisories.length === 0 ? {} : { notice: advisories.join("\n") }; const stdout = stdoutChunks.join("").trim(); if (stdout === "") { diff --git a/packages/cli/test/vale-formats.test.ts b/packages/cli/test/vale-formats.test.ts new file mode 100644 index 00000000..71ae3463 --- /dev/null +++ b/packages/cli/test/vale-formats.test.ts @@ -0,0 +1,337 @@ +import { spawnSync } from "node:child_process"; +import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterEach, describe, expect, it } from "vitest"; + +import { findValeBinary } from "../src/rules/vale/binary"; +import { + buildValeGlob, + CONVERTER_DEPENDENT_EXTENSIONS, + converterExclusionGlobs, + converterFor, + findConverterDependentFiles, + MARKUP_FORMAT_TIERS, + skippedFilesNotice, +} from "../src/rules/vale/formats"; +import { runVale } from "../src/rules/vale/run"; + +/** + * The format tiers, and the exclusion derived from them. + * + * Split the way `vale-vendor-contract.test.ts` splits: the cases that run the + * real binary assert what Vale *does* with each extension, and the rest assert + * what our code does given that. A Vale upgrade that moves a format between + * tiers should fail in the first group, naming the format, rather than + * resurfacing as an engine that stopped reporting. + */ + +const binary = findValeBinary().path; +const withVale = binary === undefined ? describe.skip : describe; + +const workspaces: string[] = []; +afterEach(() => { + for (const workspace of workspaces.splice(0)) { + rmSync(workspace, { recursive: true, force: true }); + } +}); + +const existenceRule = + "extends: existence\nmessage: \"Avoid 'simply'\"\nlevel: warning\ntokens:\n - simply\n"; + +/** + * A project whose single rule matches **every** file, which is the scoping that + * makes the crash reachable: Vale only routes a file to a parser when the + * configuration gives it a check to run, so a rule scoped `[*.md]` never asks + * for `asciidoctor` in the first place. + */ +function makeProject(documents: Record): string { + const cwd = mkdtempSync(join(tmpdir(), "vale-formats-")); + workspaces.push(cwd); + mkdirSync(join(cwd, ".taskless", "rules", "vale", "no-simply"), { + recursive: true, + }); + writeFileSync( + join(cwd, ".taskless", "rules", "vale", "no-simply", "no-simply.yml"), + existenceRule + ); + writeFileSync( + join(cwd, ".taskless", ".vale.ini"), + "StylesPath = rules/vale\nMinAlertLevel = suggestion\n\n[*]\nBasedOnStyles =\nno-simply.no-simply = YES\n" + ); + for (const [path, body] of Object.entries(documents)) { + const full = join(cwd, path); + mkdirSync(join(full, ".."), { recursive: true }); + writeFileSync(full, body); + } + return cwd; +} + +describe("the format tier table", () => { + it("derives the exclusion list from the tiers, with nothing hand-written", () => { + // One place, so the merge with `capabilities.ts` is mechanical: adding a + // measured entry to the table is the entire change, and no second list can + // fall behind it. + const expected = Object.entries(MARKUP_FORMAT_TIERS) + .filter(([, support]) => support.tier === "external-converter") + .map(([extension]) => extension) + .toSorted(); + expect([...CONVERTER_DEPENDENT_EXTENSIONS]).toEqual(expected); + // Every excluded format names the program a user would install. A skip the + // user cannot act on is only marginally better than a silent one. + for (const extension of CONVERTER_DEPENDENT_EXTENSIONS) { + expect(MARKUP_FORMAT_TIERS[extension]?.converter).toBeTruthy(); + } + }); + + it("covers all three AsciiDoc spellings", () => { + // `.asc` was not in the bug report and crashes identically. It is here + // because the tiers were measured rather than transcribed, and this case + // is what stops the next transcription from dropping it again. + for (const extension of [".adoc", ".asciidoc", ".asc"]) { + expect(converterFor(`guide${extension}`)).toBe("asciidoctor"); + } + }); + + it("reads the extension case-insensitively", () => { + // A case-insensitive filesystem hands Vale `README.RST` as + // reStructuredText. Letting case decide would put the crash back on macOS + // and Windows only. + expect(converterFor("docs/README.RST")).toBe("rst2html"); + }); + + it("treats an unmeasured extension as needing no converter", () => { + // The safe path and the unknown path are the same path: Vale reads an + // extension it does not recognize as plain text, which cannot shell out. + expect(converterFor("script.py")).toBeUndefined(); + expect(converterFor("Makefile")).toBeUndefined(); + expect(converterFor("notes.md")).toBeUndefined(); + }); +}); + +describe("the exclusion glob", () => { + it("anchors every pattern with **/ so nested files are excluded too", () => { + // Vale matches a `--glob` against the basename when the pattern contains no + // `/`, and against the path when it does. Combined with `.taskless/**` the + // whole expression goes path-wise, and a bare `*.adoc` branch then stops + // matching `docs/guide.adoc` — excluding the file you tested and not the + // one in the next directory down. + for (const pattern of converterExclusionGlobs()) { + expect(pattern.startsWith("**/*.")).toBe(true); + } + }); + + it("emits one negated alternation, because Vale honours only one --glob", () => { + expect(buildValeGlob([".taskless/**", "**/*.adoc"])).toBe( + "--glob=!{.taskless/**,**/*.adoc}" + ); + }); + + it("emits no flag when there is nothing to exclude", () => { + expect(buildValeGlob([])).toBeUndefined(); + }); +}); + +describe("the skipped-files notice", () => { + it("is absent when nothing was skipped", () => { + expect(skippedFilesNotice([])).toBeUndefined(); + }); + + it("names the files, the converter, and that the rest was checked", () => { + const notice = skippedFilesNotice(["docs/guide.adoc", "spec/api.rst"]); + expect(notice).toContain("docs/guide.adoc"); + expect(notice).toContain("spec/api.rst"); + expect(notice).toContain("asciidoctor"); + expect(notice).toContain("rst2html"); + expect(notice).toContain("every other file was checked normally"); + // Vale supports these formats; this build cannot parse them. Telling a user + // otherwise sends them to the wrong project's issue tracker. + expect(notice).toContain("Vale supports these formats"); + }); + + it("summarizes rather than printing an unbounded file list", () => { + const files = Array.from({ length: 9 }, (_, index) => `d${index}.adoc`); + const notice = skippedFilesNotice(files) ?? ""; + expect(notice).toContain("9 file(s)"); + expect(notice).toContain("and 4 more"); + }); +}); + +describe("finding converter-dependent files", () => { + it("finds them at the root and nested, and ignores everything else", async () => { + const cwd = makeProject({ + "a.md": "simply\n", + "d.adoc": "= T\n", + "docs/deep/f.adoc": "= T\n", + "docs/g.rst": "T\n", + "node_modules/pkg/vendor.adoc": "= T\n", + }); + expect(await findConverterDependentFiles(cwd, [])).toEqual([ + "d.adoc", + "docs/deep/f.adoc", + "docs/g.rst", + ]); + }); + + it("answers an explicitly named file from its own name", async () => { + const cwd = makeProject({ "a.md": "simply\n", "d.adoc": "= T\n" }); + expect(await findConverterDependentFiles(cwd, ["a.md", "d.adoc"])).toEqual([ + "d.adoc", + ]); + }); +}); + +withVale( + "runVale against the real binary, with converter-dependent files", + () => { + it("still reports every Markdown finding when an AsciiDoc file is present", async () => { + // The bug, as one case. Vale aborts the whole process on the first `E100` + // and writes nothing at all to stdout, so before the exclusion these three + // findings did not arrive late or partially — they never existed. + const cwd = makeProject({ + "a.md": "Just simply do it.\n", + "b.md": "You can simply run it.\n", + "c.md": "You simply go.\n", + "d.adoc": "= Title\n\nJust simply do it.\n", + "docs/nested.adoc": "= Title\n\nsimply\n", + }); + + const outcome = await runVale({ + cwd, + configPath: join(".taskless", ".vale.ini"), + }); + + expect(outcome.status).toBe("ok"); + if (outcome.status !== "ok") return; + expect(outcome.results.map((result) => result.file).toSorted()).toEqual([ + "a.md", + "b.md", + "c.md", + ]); + }); + + it("says which files it skipped, rather than dropping them silently", async () => { + // Silence is the bug. A run that quietly checks less than it was asked to + // is indistinguishable from a clean one, which is exactly how the engine + // got disabled without anyone noticing. + const cwd = makeProject({ + "a.md": "Just simply do it.\n", + "docs/nested.adoc": "= Title\n\nsimply\n", + }); + + const outcome = await runVale({ + cwd, + configPath: join(".taskless", ".vale.ini"), + }); + + expect(outcome.status).toBe("ok"); + if (outcome.status !== "ok") return; + expect(outcome.notice).toContain("docs/nested.adoc"); + expect(outcome.notice).toContain("asciidoctor"); + }); + + it("declines a converter-dependent file even when named explicitly", async () => { + // The one place we override an explicit request. Honouring it does not + // check that file badly — it aborts the process, so the request would cost + // the user the rest of their check. + const cwd = makeProject({ + "a.md": "Just simply do it.\n", + "d.adoc": "= Title\n\nsimply\n", + }); + + const outcome = await runVale({ + cwd, + paths: ["a.md", "d.adoc"], + configPath: join(".taskless", ".vale.ini"), + }); + + expect(outcome.status).toBe("ok"); + if (outcome.status !== "ok") return; + expect(outcome.results.every((result) => result.file === "a.md")).toBe( + true + ); + expect(outcome.results.length).toBeGreaterThan(0); + expect(outcome.notice).toContain("d.adoc"); + }); + + it("keeps out of .taskless/ while excluding converter formats", () => { + // The two exclusions have to travel in one `--glob`, because Vale keeps + // only the last one. This is the case that catches a future edit that adds + // a second flag and silently drops the first. + const cwd = makeProject({ + "a.md": "Just simply do it.\n", + "d.adoc": "= Title\n\nsimply\n", + }); + // The rule's own fixture directory: prose about the machinery, which a + // whole-project run must not report as a user's finding. + mkdirSync( + join(cwd, ".taskless", "rules", "vale", "no-simply", ".tests", "fail"), + { recursive: true } + ); + writeFileSync( + join( + cwd, + ".taskless", + "rules", + "vale", + "no-simply", + ".tests", + "fail", + "hedged.md" + ), + "Just simply do it.\n" + ); + + const result = spawnSync( + binary as string, + [ + "--config", + join(".taskless", ".vale.ini"), + "--output=JSON", + "--no-exit", + buildValeGlob([ + ".taskless/**", + ...converterExclusionGlobs(), + ]) as string, + "--", + ".", + ], + { cwd, encoding: "utf8" } + ); + + expect(result.status).toBe(0); + expect(result.stdout).not.toContain(".taskless"); + expect(result.stdout).toContain("a.md"); + }); + } +); + +withVale("Vale format tiers, measured against the pinned binary", () => { + // The assertion behind "assert known support". Every extension in the table + // is re-measured here, so a Vale release that starts or stops needing a + // converter for a format turns this red — naming the format — instead of + // either crashing a user's whole check or silently excluding a format Vale + // can now read perfectly well. + for (const [extension, support] of Object.entries(MARKUP_FORMAT_TIERS)) { + it(`${extension} is ${support.tier}`, () => { + const cwd = makeProject({ [`probe${extension}`]: "simply\n" }); + const result = spawnSync( + binary as string, + [ + "--config", + join(".taskless", ".vale.ini"), + "--output=JSON", + "--no-exit", + "--", + `probe${extension}`, + ], + { cwd, encoding: "utf8" } + ); + + const crashed = + result.status !== 0 && `${result.stderr}`.includes("E100"); + expect(crashed).toBe(support.tier === "external-converter"); + }); + } +}); diff --git a/packages/cli/test/vale-orchestration.test.ts b/packages/cli/test/vale-orchestration.test.ts index 80e703aa..122112c2 100644 --- a/packages/cli/test/vale-orchestration.test.ts +++ b/packages/cli/test/vale-orchestration.test.ts @@ -446,3 +446,42 @@ describe("an engine failure under --json", () => { expect(output.notices).toBeUndefined(); }); }); + +withVale("a repository containing a converter-dependent file", () => { + it("still reports its Markdown findings, and says what it skipped", async () => { + // End to end, through the built CLI, because the failure this covers was + // whole-run: one `.adoc` aborted Vale before it serialized anything, so + // every Markdown finding in the project disappeared while `check` still + // exited non-zero for an unrelated ast-grep finding — a dead engine + // wearing a normal red check. + const cwd = makeMixedProject(); + // Widen the rule past `[*.md]`. Vale only routes a file to a parser when + // the configuration gives it a check to run, so the crash is unreachable + // while every matcher is Markdown-only. + writeFileSync( + join(cwd, ".taskless", "rules", "vale", "no-simply", ".vale.ini"), + "[*]\ntskl) rule = no-simply\nBasedOnStyles =\nno-simply.no-simply = YES\n" + ); + writeFileSync(join(cwd, "guide.adoc"), "= Guide\n\nJust simply do it.\n"); + mkdirSync(join(cwd, "docs"), { recursive: true }); + writeFileSync(join(cwd, "docs", "api.rst"), "API\n===\n\nsimply\n"); + + const { stdout } = await runCli(["check", "-d", cwd, "--json"]); + const output = parseJson(stdout); + + // The findings survive... + expect( + output.results.some( + (result) => (result as { source?: string }).source === "vale" + ) + ).toBe(true); + // ...and it is a skip, not an engine failure. + expect(output.failures).toBeUndefined(); + // ...and the skip is said out loud, naming both the file and the fix. + const notices = (output.notices ?? []).join("\n"); + expect(notices).toContain("guide.adoc"); + expect(notices).toContain("docs/api.rst"); + expect(notices).toContain("asciidoctor"); + expect(notices).toContain("rst2html"); + }); +}); From fa917e24f461fc294421497ab863a1ba3a7a5d06 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Sun, 23 Aug 2026 22:09:48 -0700 Subject: [PATCH 2/4] fix(cli): reconcile the two Vale format tables into one measured record MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two branches measured Vale 3.17.1's format tiers independently and landed two tables that disagreed about six extensions. `VALE_FORMAT_TIERS` in `rules/capabilities.ts` is now the only one: a flat extension -> tier record where a converter-dependent row carries the program's own name in its tier (`converter:asciidoctor`), so adding an extension is one line in one file. `rules/vale/formats.ts` derives the exclusion list, the glob and the notice's converter name from it and keeps no copy; the recipes render their format lists from the same record. `capabilities.ts` is the home rather than `formats.ts` because the tiers have two consumers with incompatible constraints — the module that runs Vale, and `prompts/recipes.ts`, which must stay free of every host capability or `assert-prompts-graph` fails the build. It stays import-free. Every row was re-probed against the shipped binary, each tier by the property only that tier has, since ordinary prose fires in all three readable tiers: - `.asc` and `.rest` are converter-dependent (exit 2, `E100 [lintAdoc]` asciidoctor and `E100 [lintRST]` rst2html). Neither was in either table's converter tier before this. - `.tex` and `.rmd` are the plaintext fallback, not native: Vale lints `% simply in a comment` and `simply <- 1` inside a ```{r} chunk. - `.mkd` and `.mkdn` are the plaintext fallback too, found while re-measuring the rest — both lint straight through a fenced code block, an HTML comment and an indented block, where `.md`, `.markdown` and `.mdown` skip all three. Calling them markup was the more expensive error of the two directions: it would have promised a `scope:` that has nothing to act on. - `.ditamap` was probed and is plaintext, so it is not in the converter tier despite the `.dita` neighbour. `.xml` is "an XSLT transform", which is Vale's own wording ("no XSLT transform provided"). - All 35 comment extensions and the six remaining markup ones re-confirmed unchanged; `.mdown` was missing from the markup constant and is added. Tests: the per-extension probing lives in `vale-vendor-contract.test.ts` only, so a weaker fixture cannot overrule a stronger one — `vale-formats.test.ts` lost its bare-prose tier loop, which could not discriminate, and keeps the derivation and end-to-end cases. New there: a plaintext tier suite that asserts each listed extension lints the construct a parser would have skipped, a coverage case that every row of the table is reached by some probe, and a plaintext-is-converter-free case in `formats.ts`. MDX is now described as not supported *yet* in `create-vale-rule.txt` and `route.txt` — Vale 3.18.0 parses it natively and a CLI update carrying that Vale is expected to bring it, with no date promised. That release also adds a Typst converter, so the table warns that a version bump invalidates every row and names `.typ` as the known incoming case. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Cyga14bww8rmazH2XrF8ms --- .changeset/vale-converter-formats.md | 24 +- packages/cli/src/agent/create-vale-rule.txt | 9 + packages/cli/src/agent/route.txt | 8 +- packages/cli/src/prompts/recipes.ts | 2 + packages/cli/src/rules/capabilities.ts | 263 +++++++++++++----- packages/cli/src/rules/vale/formats.ts | 110 +++----- packages/cli/test/prompts.test.ts | 1 + .../cli/test/recipe-cross-references.test.ts | 11 + packages/cli/test/vale-formats.test.ts | 64 ++--- .../cli/test/vale-vendor-contract.test.ts | 78 +++++- 10 files changed, 377 insertions(+), 193 deletions(-) diff --git a/.changeset/vale-converter-formats.md b/.changeset/vale-converter-formats.md index 5f347eaf..0f0eafc7 100644 --- a/.changeset/vale-converter-formats.md +++ b/.changeset/vale-converter-formats.md @@ -19,12 +19,24 @@ same way any ordinary failing check does. `runVale` now excludes the converter-dependent extensions from Vale's own walk, so the rest of the project is checked normally and the skipped files are named in a notice that says which converter would put them back in scope. The tiers -live in one table in `rules/vale/formats.ts`, measured against the pinned binary -rather than transcribed from documentation — which is how `.asc`, a third -AsciiDoc spelling that crashes identically and was not in the bug report, ended -up covered. A per-extension test re-measures every entry against the real Vale, -so a version bump that moves a format between tiers fails there instead of -silently turning the engine off again. +live in one table in `rules/capabilities.ts` — the same record the agent recipes +render their format lists from — measured against the pinned binary rather than +transcribed from documentation. That is how `.asc` and `.rest`, a third AsciiDoc +spelling and a second reStructuredText one that crash identically and were in +neither bug report, ended up covered. Measurement also corrected four +extensions that a documentation reading had put in the wrong tier: `.tex`, +`.rmd`, `.mkd` and `.mkdn` are all read as plain text by this Vale, not parsed, +so excluding them would have dropped files Vale lints perfectly well. A +per-extension test re-measures every row against the real Vale — each tier by +the property only that tier has, since ordinary prose fires in all of them — so +a version bump that moves a format between tiers fails there instead of silently +turning the engine off again. + +The recipes now say MDX is not supported _yet_, rather than unsupported: Vale +3.18.0 parses it natively and a CLI update carrying that Vale is expected to +bring it. The same release adds a Typst converter, which will move `.typ` out of +the plaintext tier, so the table carries a standing instruction to re-measure +every row on a version bump. Two details are load-bearing and were both wrong on the first attempt. Vale honours exactly one `--glob` and keeps the last, so the `.taskless/` exclusion diff --git a/packages/cli/src/agent/create-vale-rule.txt b/packages/cli/src/agent/create-vale-rule.txt index 76bcb10e..1136fc33 100644 --- a/packages/cli/src/agent/create-vale-rule.txt +++ b/packages/cli/src/agent/create-vale-rule.txt @@ -314,6 +314,8 @@ it. parser, so the whole file is linted as prose: a rule matched to YAML flags key names and values, not just the comments. If that is not what the rule means, narrow the glob rather than accepting it. + These land here despite reading like markup, so a `scope:` value + has nothing to act on in them: %(VALE_PLAINTEXT_FORMATS)s - **cannot be read at all** — Vale supports these formats but shells out to an external converter to parse them, and this CLI ships none of those converters: @@ -326,6 +328,13 @@ it. matcher that takes `check` down the first time the repo grows an `.mdx` file. Never put one of those extensions in a glob. + **MDX is not supported yet.** Vale v%(VALE_VERSION)s reads `.mdx` only + through `mdx2vast`, which this CLI does not ship. Vale 3.18.0 parses + MDX natively, and a CLI update carrying that Vale is expected to + bring MDX support with it. Until then, scope the rule to `.md` and + say that `.mdx` is not supported yet rather than offering a matcher + that covers it. + 5. **Write the fixtures.** Two directories inside the rule, both flat. Vale lints the whole fixture tree, so a document nested a level deeper would be linted and never checked against either bucket, which diff --git a/packages/cli/src/agent/route.txt b/packages/cli/src/agent/route.txt index 597e9785..0cf9220a 100644 --- a/packages/cli/src/agent/route.txt +++ b/packages/cli/src/agent/route.txt @@ -122,7 +122,8 @@ answered together. `.sql` and every unnamed extension included. There is no parser, so the file is linted as one block of prose, and a Vale rule scoped to YAML flags the code as readily as the comments. That is - rarely what was asked for — say so before writing it. + rarely what was asked for — say so before writing it. These read + like markup and are not: %(VALE_PLAINTEXT_FORMATS)s - *cannot be read at all* — Vale supports these formats but shells out to an external converter to parse them, and this CLI ships none of those converters: @@ -134,6 +135,11 @@ answered together. unreported. A matcher written as `[*.{md,mdx}]` is not a wider `[*.md]` — it is a broken one. + **MDX is not supported yet.** Vale 3.18.0 parses MDX natively and + a CLI update carrying that Vale is expected to bring it; on + v%(VALE_VERSION)s, `.mdx` needs `mdx2vast` and this CLI ships + none. Tell the user MDX is not supported yet and scope to `.md`. + **A language on neither list does not route to runtime by default.** Check `create-legacy-rule` first: the repo may already run a linter that speaks it, and that linter's own dialect is a local destination diff --git a/packages/cli/src/prompts/recipes.ts b/packages/cli/src/prompts/recipes.ts index 29506259..73dc8e60 100644 --- a/packages/cli/src/prompts/recipes.ts +++ b/packages/cli/src/prompts/recipes.ts @@ -15,6 +15,7 @@ import { valeCommentList, valeConverterList, valeMarkupList, + valePlaintextList, } from "../rules/capabilities"; // Agent recipe files embedded at build time via Vite import.meta.glob. @@ -164,6 +165,7 @@ export function buildVariables( VALE_VERSION, VALE_MARKUP_FORMATS: valeMarkupList(), VALE_COMMENT_FORMATS: valeCommentList(), + VALE_PLAINTEXT_FORMATS: valePlaintextList(), VALE_CONVERTER_FORMATS: valeConverterList(), PACKAGE_MANAGER_DLX: options.packageManagerDlx ?? PACKAGE_MANAGER_DLX_MARKER, diff --git a/packages/cli/src/rules/capabilities.ts b/packages/cli/src/rules/capabilities.ts index 6655c8e7..96ae3777 100644 --- a/packages/cli/src/rules/capabilities.ts +++ b/packages/cli/src/rules/capabilities.ts @@ -98,79 +98,183 @@ export const AST_GREP_LANGUAGES = [ */ export const VALE_VERSION = "3.17.1"; +/** + * Which tier Vale routes an extension to. + * + * `converter:` carries the program's own name in the tier, so one + * table row states both the tier and the thing a user would install. The name + * is the one Vale prints in its `E100` text, because that is the string a + * reader will search for. + */ +export type ValeFormatTier = + /** Parsed in-process: the document is prose, its own syntax is skipped. */ + | "markup" + /** Parsed in-process: comment text is linted, the code body is invisible. */ + | "comment" + /** No parser: the whole file is linted as one block of prose. */ + | "plaintext" + /** Vale shells out to a program the `@taskless/vale-*` packages do not ship. */ + | `converter:${string}`; + +/** The `ValeFormatTier` prefix that marks a converter-dependent format. */ +const CONVERTER_TIER_PREFIX = "converter:"; + +/** + * EVERY MEASURED VALE EXTENSION, AND ITS TIER. THE ONLY TABLE. + * + * Adding an extension is one line here; every list, glob, notice and test below + * derives from this record, so there is no second place to keep in step. Two + * branches once measured this independently and produced two tables that + * disagreed about six extensions — that is what this single record exists to + * make impossible. + * + * MEASURED, NOT DOCUMENTED, AND MEASURED BY A DISCRIMINATING PROBE. Ordinary + * prose fires in all three readable tiers, so it can never separate them. Each + * tier is pinned by the property only that tier has, in + * `test/vale-vendor-contract.test.ts`: + * + * - **markup** — a construct only a real parser skips yields ZERO (a fenced + * code block, an Org `#` line, an HTML comment). + * - **comment** — the token in a comment yields one finding and the same token + * on a bare non-comment line yields ZERO. A type that fires on both is the + * plaintext fallback wearing a code extension. + * - **plaintext** — a bare line yields a finding. Listed only where the tier is + * surprising: `.tex`, `.rmd`, `.mkd` and `.mkdn` all look like markup and are + * not. Everything unnamed lands here too, which is why this tier does not + * need to be exhaustive. + * - **converter** — a non-zero exit whose output carries `E100` and the + * program's name. + * + * Measured spellings, not families. `.mdown` is native Markdown and `.mkd` and + * `.mkdn` are not; `.asc` is a third AsciiDoc spelling that crashes exactly + * like `.adoc`; `.ditamap` is plaintext while `.dita` needs `dita`. Case is + * part of the key — `.r` and `.R` were both measured, `.PY` was measured and is + * not comment-aware. Add a row only after probing it; the contract test refuses + * to take one on faith. + * + * A VERSION BUMP INVALIDATES THIS TABLE — RE-MEASURE THE WHOLE OF IT. The tiers + * are a property of {@link VALE_VERSION}'s binary, and the dangerous direction + * is a format Vale *learns*: an extension missing from this table is read as + * plain text today, but the moment Vale routes it to a converter the same + * omission is a crash that takes down every Vale rule in the run. Vale 3.18.0 + * is the known incoming case in both directions — it parses MDX natively, and + * it adds a Typst converter, so `.typ` moves from `plaintext` to + * `converter:typst2vast` on that bump while `.mdx` moves to `markup`. Neither + * happens on its own; re-probe every row. + */ +export const VALE_FORMAT_TIERS: Readonly> = { + // markup — parsed, the format's own constructs skipped + ".htm": "markup", + ".html": "markup", + ".markdown": "markup", + ".md": "markup", + ".mdown": "markup", + ".org": "markup", + ".xhtml": "markup", + // comment text only — the code body is invisible + ".c": "comment", + ".c++": "comment", + ".cc": "comment", + ".clj": "comment", + ".cpp": "comment", + ".cs": "comment", + ".css": "comment", + ".cxx": "comment", + ".go": "comment", + ".h": "comment", + ".h++": "comment", + ".hpp": "comment", + ".hs": "comment", + ".java": "comment", + ".jl": "comment", + ".js": "comment", + ".jsx": "comment", + ".less": "comment", + ".lua": "comment", + ".php": "comment", + ".pl": "comment", + ".pm": "comment", + ".proto": "comment", + ".ps1": "comment", + ".py": "comment", + ".pyw": "comment", + ".r": "comment", + ".R": "comment", + ".rb": "comment", + ".rs": "comment", + ".sass": "comment", + ".scala": "comment", + ".swift": "comment", + ".ts": "comment", + ".tsx": "comment", + // plaintext, and surprising about it — these look parsed and are not + ".mkd": "plaintext", + ".mkdn": "plaintext", + ".rmd": "plaintext", + ".tex": "plaintext", + ".typ": "plaintext", + // converter-dependent — Vale supports the format, we ship no converter + ".adoc": "converter:asciidoctor", + ".asc": "converter:asciidoctor", + ".asciidoc": "converter:asciidoctor", + ".dita": "converter:dita", + ".mdx": "converter:mdx2vast", + ".rest": "converter:rst2html", + ".rst": "converter:rst2html", + ".xml": "converter:xsltproc and an XSLT stylesheet", +}; + +/** Every extension in `tier`, in table order. */ +function extensionsInTier(tier: ValeFormatTier): string[] { + return Object.entries(VALE_FORMAT_TIERS) + .filter(([, entry]) => entry === tier) + .map(([extension]) => extension); +} + /** * Extensions Vale parses as markup: the whole document is prose, and the * format's own non-prose constructs are excluded. * - * MEASURED, NOT DOCUMENTED. Each entry was distinguished from the plaintext - * fallback by a construct only a real parser skips — a fenced code block for - * Markdown, a `#` line for Org, an HTML comment for the HTML family — because - * on ordinary prose a markup parse and a plaintext parse are indistinguishable. - * * The HTML entries carry a consequence worth stating to an author: prose * outside an element is not linted, so a bare sentence in a `.html` file yields * nothing. */ -export const VALE_MARKUP_EXTENSIONS = [ - ".htm", - ".html", - ".markdown", - ".md", - ".org", - ".xhtml", -] as const; +export const VALE_MARKUP_EXTENSIONS: readonly string[] = + extensionsInTier("markup"); /** * Extensions where Vale lints **comment text only** and ignores the code body. + */ +export const VALE_COMMENT_EXTENSIONS: readonly string[] = + extensionsInTier("comment"); + +/** + * Extensions measured into the plaintext fallback whose spelling suggests + * otherwise. * - * MEASURED BY THE NEGATIVE, which is the only test that separates this tier - * from the plaintext fallback: a token inside a comment yields a finding, and - * the same token on a bare non-comment line yields nothing. A file type that - * fires on both is plaintext, not comment-aware. + * Not exhaustive and not meant to be — every unnamed extension is plaintext + * too. These are the ones an author would reasonably assume were parsed, so + * they are worth naming in a recipe rather than leaving to "everything else". + */ +export const VALE_PLAINTEXT_EXTENSIONS: readonly string[] = + extensionsInTier("plaintext"); + +/** + * The converter each converter-dependent extension needs, keyed by extension. * - * Case-sensitive, and not closed over the obvious aliases. `.r` and `.R` are - * both here because both were measured; `.PY` was measured and is not - * comment-aware, and neither are `.hh`/`.hxx` despite `.h`/`.hpp` being. Add an - * entry only after probing it — the contract test below refuses to take one on - * faith. + * The lookup `rules/vale/formats.ts` uses to name a converter in the skip + * notice. Keys are lowercase because every measured converter format is; a + * caller comparing an extension off the filesystem must lowercase it first, or + * `README.RST` becomes a crash on case-insensitive platforms only. */ -export const VALE_COMMENT_EXTENSIONS = [ - ".c", - ".c++", - ".cc", - ".clj", - ".cpp", - ".cs", - ".css", - ".cxx", - ".go", - ".h", - ".h++", - ".hpp", - ".hs", - ".java", - ".jl", - ".js", - ".jsx", - ".less", - ".lua", - ".php", - ".pl", - ".pm", - ".proto", - ".ps1", - ".py", - ".pyw", - ".r", - ".R", - ".rb", - ".rs", - ".sass", - ".scala", - ".swift", - ".ts", - ".tsx", -] as const; +export const VALE_CONVERTER_BY_EXTENSION: Readonly> = + Object.fromEntries( + Object.entries(VALE_FORMAT_TIERS).flatMap(([extension, tier]) => + tier.startsWith(CONVERTER_TIER_PREFIX) + ? [[extension, tier.slice(CONVERTER_TIER_PREFIX.length)]] + : [] + ) + ); /** A format Vale supports upstream but cannot read without an external tool. */ export interface ValeConverterFormat { @@ -180,6 +284,22 @@ export interface ValeConverterFormat { converter: string; } +/** The converter-dependent extensions grouped by the program they need. */ +function groupByConverter(): ValeConverterFormat[] { + const groups = new Map(); + for (const [extension, converter] of Object.entries( + VALE_CONVERTER_BY_EXTENSION + )) { + const existing = groups.get(converter); + if (existing === undefined) groups.set(converter, [extension]); + else existing.push(extension); + } + return [...groups].map(([converter, extensions]) => ({ + converter, + extensions, + })); +} + /** * Formats that fail rather than lint, because Vale shells out to a converter * this CLI does not ship. @@ -195,18 +315,9 @@ export interface ValeConverterFormat { * caught by a rule's glob takes down the entire Vale pass, including every * other rule and every other file — so `[*.{md,mdx}]` is not a slightly wider * matcher than `[*.md]`, it is a broken one. - * - * VERSION-SENSITIVE. Vale 3.18.0 parses MDX natively, so a bump past it moves - * `.mdx` out of this list; that is a {@link VALE_VERSION} edit plus an entry - * removal here, and the contract test fails until both happen. */ -export const VALE_CONVERTER_DEPENDENT: readonly ValeConverterFormat[] = [ - { extensions: [".rst"], converter: "rst2html" }, - { extensions: [".adoc", ".asciidoc"], converter: "asciidoctor" }, - { extensions: [".xml"], converter: "xsltproc and an XSLT stylesheet" }, - { extensions: [".dita"], converter: "dita" }, - { extensions: [".mdx"], converter: "mdx2vast" }, -]; +export const VALE_CONVERTER_DEPENDENT: readonly ValeConverterFormat[] = + groupByConverter(); /** * Vale's checker tag per converter-dependent extension, from the `E100` text. @@ -235,7 +346,7 @@ export const VALE_CONVERTER_CHECKERS: Readonly> = { /** Every converter-dependent extension, flattened. */ export const VALE_CONVERTER_DEPENDENT_EXTENSIONS: readonly string[] = - VALE_CONVERTER_DEPENDENT.flatMap((format) => format.extensions); + Object.keys(VALE_CONVERTER_BY_EXTENSION); /** `Bash, C, Cpp, …` — the ast-grep language list as recipe prose. */ export function astGrepLanguageList(): string { @@ -252,6 +363,16 @@ export function valeCommentList(): string { return VALE_COMMENT_EXTENSIONS.join(", "); } +/** + * `.mkd, .mkdn, …` — the plaintext extensions worth naming, as recipe prose. + * + * Rendered rather than written into the recipe because the surprising cases are + * exactly the ones a hand-written list gets wrong. + */ +export function valePlaintextList(): string { + return VALE_PLAINTEXT_EXTENSIONS.join(", "); +} + /** * `.rst (needs rst2html), …` — Vale's converter-dependent formats as recipe * prose, each naming the tool whose absence is the actual failure. diff --git a/packages/cli/src/rules/vale/formats.ts b/packages/cli/src/rules/vale/formats.ts index 36072894..61924075 100644 --- a/packages/cli/src/rules/vale/formats.ts +++ b/packages/cli/src/rules/vale/formats.ts @@ -1,6 +1,11 @@ import { glob } from "node:fs/promises"; import { basename, extname } from "node:path"; +import { + VALE_CONVERTER_BY_EXTENSION, + VALE_CONVERTER_DEPENDENT_EXTENSIONS, +} from "../capabilities"; + /** * Taskless's own directory, as a project-relative path. * @@ -30,13 +35,28 @@ export const TASKLESS_DIRECTORY = ".taskless"; * in a repo that already had an ast-grep finding, "every Vale rule stopped * running" was indistinguishable from a normal red check. * + * ## The table lives in `rules/capabilities.ts` + * + * `VALE_FORMAT_TIERS` there is the single measured record of what Vale does + * with an extension, and everything in this module is derived from it — the + * exclusion list, the glob, and the converter named in the notice. Nothing here + * restates a tier, because a second copy is precisely how two independently + * measured tables came to disagree about six extensions. + * + * `capabilities.ts` is the home rather than this file because the same tiers + * are read by two consumers with incompatible constraints: this module, which + * runs Vale, and `src/prompts/recipes.ts`, which renders the tiers into the + * agent recipes and must stay free of every host capability (`node:fs` here + * would fail `assert-prompts-graph` at build time). Pure data satisfies both. + * * ## Asserting known support rather than dodging known breakage * - * The tiers below are **measured against the pinned binary**, not read off - * Vale's documentation, and `vale-formats.test.ts` re-measures every entry - * against the real binary on every run. A format whose tier changes — or a - * converter Vale starts requiring for a format we currently call native — turns - * that test red before it can turn a user's check silently green. + * The tiers are **measured against the pinned binary**, not read off Vale's + * documentation, and `vale-vendor-contract.test.ts` re-measures every entry + * against the real binary on every run, each tier by the property only that + * tier has. A format whose tier changes — or a converter Vale starts requiring + * for a format we currently call native — turns that test red before it can + * turn a user's check silently green. * * The reason the operative list is the *converter* tier rather than the native * one deserves stating, because "allowlist what we know works" reads like it @@ -54,74 +74,19 @@ export const TASKLESS_DIRECTORY = ".taskless"; * makes an exclusion honest rather than a denylist with a nice name: shelling * out is a property of a short, closed set of markup formats, and an extension * outside that set falls through to Vale's plain-text reader, which has no - * converter to be missing. `MARKUP_FORMAT_TIERS` is the assertion — it names - * what we measured and what we concluded — and the exclusion is derived from - * it, so the two cannot drift. + * converter to be missing. * * **That property is pinned to the vendored binary, and a version bump is what * breaks it.** "Unknown to us" is safe only while it also means "unknown to * Vale": the moment Vale learns a format, it starts routing that extension to a - * parser, and if that parser shells out, an extension missing from this table is + * parser, and if that parser shells out, an extension missing from the table is * a crash rather than a plain-text read. Vale 3.18.0 is the live example — it - * added Typst, which converts through `typst2vast`, so upgrading the - * `@taskless/vale-*` packages without re-measuring would reintroduce exactly - * this bug under a new extension. Re-measure the table on every bump; the - * per-extension cases in `vale-formats.test.ts` are how. - * - * TODO(capabilities): `packages/cli/src/rules/capabilities.ts` is being built - * in parallel to hold pinned engine-capability constants, Vale format tiers - * among them. This table is the single place those tiers live today; move it - * there wholesale at integration rather than copying entries out of it. - */ -export type ValeFormatTier = - /** Vale parses it in-process. Safe to hand over. */ - | "native" - /** Vale shells out to a program we do not ship. Must not be handed over. */ - | "external-converter"; - -/** One markup extension, the tier we measured it in, and why. */ -export interface ValeFormatSupport { - tier: ValeFormatTier; - /** - * The program Vale invokes, for `external-converter` entries. Named in the - * user-facing notice so "skipped" comes with something to act on. - */ - converter?: string; -} - -/** - * Every markup extension Vale routes to a syntax-aware parser, tiered. - * - * Measured against `@taskless/vale-*` 3.17.1 by linting a one-line file per - * extension under a `[*]` matcher and recording whether Vale returned findings - * or aborted with `E100`. - * - * `.asc` is the entry worth pointing at: it is a third AsciiDoc spelling, it - * crashes exactly like `.adoc`, and it was not in the bug report. It is here - * because the tiers were measured rather than transcribed. + * adds Typst, which converts through `typst2vast`, so `.typ` stops being the + * plaintext read it is today and upgrading the `@taskless/vale-*` packages + * without re-measuring would reintroduce exactly this bug under a new + * extension. Re-measure the whole table on every bump; the per-extension cases + * in `vale-vendor-contract.test.ts` are how. */ -export const MARKUP_FORMAT_TIERS: Readonly> = - { - ".md": { tier: "native" }, - ".markdown": { tier: "native" }, - ".mdown": { tier: "native" }, - ".mkdn": { tier: "native" }, - ".mkd": { tier: "native" }, - ".html": { tier: "native" }, - ".htm": { tier: "native" }, - ".xhtml": { tier: "native" }, - ".org": { tier: "native" }, - ".tex": { tier: "native" }, - ".rmd": { tier: "native" }, - ".adoc": { tier: "external-converter", converter: "asciidoctor" }, - ".asciidoc": { tier: "external-converter", converter: "asciidoctor" }, - ".asc": { tier: "external-converter", converter: "asciidoctor" }, - ".rst": { tier: "external-converter", converter: "rst2html" }, - ".rest": { tier: "external-converter", converter: "rst2html" }, - ".xml": { tier: "external-converter", converter: "an XSLT stylesheet" }, - ".dita": { tier: "external-converter", converter: "dita" }, - ".mdx": { tier: "external-converter", converter: "mdx2vast" }, - }; /** * Extensions Vale must never be handed, lowercase, leading dot, sorted. @@ -129,12 +94,9 @@ export const MARKUP_FORMAT_TIERS: Readonly> = * Derived from the tier table rather than written out again, so adding a * measured entry there is the whole change. */ -export const CONVERTER_DEPENDENT_EXTENSIONS: readonly string[] = Object.entries( - MARKUP_FORMAT_TIERS -) - .filter(([, support]) => support.tier === "external-converter") - .map(([extension]) => extension) - .toSorted(); +export const CONVERTER_DEPENDENT_EXTENSIONS: readonly string[] = [ + ...VALE_CONVERTER_DEPENDENT_EXTENSIONS, +].toSorted(); /** * The converter Vale would need for `path`, or `undefined` if it needs none. @@ -144,7 +106,7 @@ export const CONVERTER_DEPENDENT_EXTENSIONS: readonly string[] = Object.entries( * the crash reappear on exactly one platform. */ export function converterFor(path: string): string | undefined { - return MARKUP_FORMAT_TIERS[extname(path).toLowerCase()]?.converter; + return VALE_CONVERTER_BY_EXTENSION[extname(path).toLowerCase()]; } /** diff --git a/packages/cli/test/prompts.test.ts b/packages/cli/test/prompts.test.ts index 78e74222..6b406827 100644 --- a/packages/cli/test/prompts.test.ts +++ b/packages/cli/test/prompts.test.ts @@ -144,6 +144,7 @@ describe("the CLI invocation variable", () => { "VALE_COMMENT_FORMATS", "VALE_CONVERTER_FORMATS", "VALE_MARKUP_FORMATS", + "VALE_PLAINTEXT_FORMATS", "VALE_VERSION", ]); // INPUT_SCHEMA stays conditional on the placeholder being present. diff --git a/packages/cli/test/recipe-cross-references.test.ts b/packages/cli/test/recipe-cross-references.test.ts index 1fb28816..afc3c083 100644 --- a/packages/cli/test/recipe-cross-references.test.ts +++ b/packages/cli/test/recipe-cross-references.test.ts @@ -9,6 +9,7 @@ import { valeCommentList, valeConverterList, valeMarkupList, + valePlaintextList, } from "../src/rules/capabilities"; import { buildInvocation } from "../src/util/invocation"; @@ -375,9 +376,14 @@ describe("recipes state engine reach from the pinned versions", () => { expect(route).toContain(valeMarkupList()); expect(route).toContain(valeCommentList()); expect(route).toContain(valeConverterList()); + expect(route).toContain(valePlaintextList()); // The consequence, not just the list. A recipe that names `.mdx` without // saying it takes the whole pass down has not conveyed the hazard. expect(route).toContain("E100"); + // MDX is a "not yet", not a "never" — Vale 3.18.0 parses it natively. An + // agent told only that it is unreadable would tell a user MDX is + // unsupported, full stop. + expect(route).toContain("MDX is not supported yet"); }); it("repeats the reach where a Vale matcher is written", async () => { @@ -389,5 +395,10 @@ describe("recipes state engine reach from the pinned versions", () => { // a matcher. It may still cite it — as the counter-example it now is — so // this pins the warning rather than the absence of the string. expect(recipe).toContain("Never put one of those extensions in a glob."); + expect(recipe).toContain(valePlaintextList()); + expect(recipe).toContain("MDX is not supported yet"); + // No date. The bump is expected, not scheduled, and a recipe that implies + // otherwise is stale the moment it slips. + expect(recipe).not.toMatch(/\b20\d\d-\d\d\b/); }); }); diff --git a/packages/cli/test/vale-formats.test.ts b/packages/cli/test/vale-formats.test.ts index 71ae3463..35930b67 100644 --- a/packages/cli/test/vale-formats.test.ts +++ b/packages/cli/test/vale-formats.test.ts @@ -5,6 +5,7 @@ import { join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; +import { VALE_FORMAT_TIERS } from "../src/rules/capabilities"; import { findValeBinary } from "../src/rules/vale/binary"; import { buildValeGlob, @@ -12,19 +13,21 @@ import { converterExclusionGlobs, converterFor, findConverterDependentFiles, - MARKUP_FORMAT_TIERS, skippedFilesNotice, } from "../src/rules/vale/formats"; import { runVale } from "../src/rules/vale/run"; /** - * The format tiers, and the exclusion derived from them. + * The exclusion derived from the format tiers, and the run that uses it. * - * Split the way `vale-vendor-contract.test.ts` splits: the cases that run the - * real binary assert what Vale *does* with each extension, and the rest assert - * what our code does given that. A Vale upgrade that moves a format between - * tiers should fail in the first group, naming the format, rather than - * resurfacing as an engine that stopped reporting. + * WHAT VALE DOES WITH AN EXTENSION IS NOT ASSERTED HERE. `VALE_FORMAT_TIERS` + * lives in `src/rules/capabilities.ts` and every row of it is re-measured + * against the real binary in `vale-vendor-contract.test.ts` ("Vale engine + * capabilities"), each tier by the discriminating property only that tier has. + * This file asserts what our code does *given* those tiers, plus the end-to-end + * behaviour of a run that contains a converter-dependent file. Two files + * probing the same extension with different fixtures is how a weaker probe gets + * to overrule a stronger one, so the probing happens in exactly one of them. */ const binary = findValeBinary().path; @@ -73,15 +76,16 @@ describe("the format tier table", () => { // One place, so the merge with `capabilities.ts` is mechanical: adding a // measured entry to the table is the entire change, and no second list can // fall behind it. - const expected = Object.entries(MARKUP_FORMAT_TIERS) - .filter(([, support]) => support.tier === "external-converter") + const expected = Object.entries(VALE_FORMAT_TIERS) + .filter(([, tier]) => tier.startsWith("converter:")) .map(([extension]) => extension) .toSorted(); + expect(expected.length).toBeGreaterThan(0); expect([...CONVERTER_DEPENDENT_EXTENSIONS]).toEqual(expected); // Every excluded format names the program a user would install. A skip the // user cannot act on is only marginally better than a silent one. for (const extension of CONVERTER_DEPENDENT_EXTENSIONS) { - expect(MARKUP_FORMAT_TIERS[extension]?.converter).toBeTruthy(); + expect(converterFor(`doc${extension}`)).toBeTruthy(); } }); @@ -108,6 +112,17 @@ describe("the format tier table", () => { expect(converterFor("Makefile")).toBeUndefined(); expect(converterFor("notes.md")).toBeUndefined(); }); + + it("hands over the formats measured as plaintext, converter-free", () => { + // `.tex`, `.rmd`, `.mkd` and `.mkdn` all read as markup and are not — the + // first table to be written by hand put `.tex` and `.rmd` in the native + // tier. Being wrong about the tier is survivable; being wrong about needing + // a converter is not, because it excludes a file Vale would have linted + // perfectly well. This is that half of the claim. + for (const extension of [".tex", ".rmd", ".mkd", ".mkdn", ".typ"]) { + expect(converterFor(`doc${extension}`)).toBeUndefined(); + } + }); }); describe("the exclusion glob", () => { @@ -306,32 +321,3 @@ withVale( }); } ); - -withVale("Vale format tiers, measured against the pinned binary", () => { - // The assertion behind "assert known support". Every extension in the table - // is re-measured here, so a Vale release that starts or stops needing a - // converter for a format turns this red — naming the format — instead of - // either crashing a user's whole check or silently excluding a format Vale - // can now read perfectly well. - for (const [extension, support] of Object.entries(MARKUP_FORMAT_TIERS)) { - it(`${extension} is ${support.tier}`, () => { - const cwd = makeProject({ [`probe${extension}`]: "simply\n" }); - const result = spawnSync( - binary as string, - [ - "--config", - join(".taskless", ".vale.ini"), - "--output=JSON", - "--no-exit", - "--", - `probe${extension}`, - ], - { cwd, encoding: "utf8" } - ); - - const crashed = - result.status !== 0 && `${result.stderr}`.includes("E100"); - expect(crashed).toBe(support.tier === "external-converter"); - }); - } -}); diff --git a/packages/cli/test/vale-vendor-contract.test.ts b/packages/cli/test/vale-vendor-contract.test.ts index b9f88256..d22f4ae1 100644 --- a/packages/cli/test/vale-vendor-contract.test.ts +++ b/packages/cli/test/vale-vendor-contract.test.ts @@ -11,11 +11,14 @@ import { VALE_CONVERTER_CHECKERS, VALE_CONVERTER_DEPENDENT, VALE_CONVERTER_DEPENDENT_EXTENSIONS, + VALE_FORMAT_TIERS, VALE_MARKUP_EXTENSIONS, + VALE_PLAINTEXT_EXTENSIONS, VALE_VERSION, valeCommentList, valeConverterList, valeMarkupList, + valePlaintextList, } from "../src/rules/capabilities"; /** @@ -382,6 +385,11 @@ withVale("Vale vendor contract", () => { * - **comment-only** is separated from plaintext by the NEGATIVE — the same * token on a bare non-comment line must yield nothing. A file type that fires * on both is the plaintext fallback wearing a code extension. + * - **plaintext** is the tier that needs no separating — a bare line fires — + * but the entries listed in it do: each names a construct a parser WOULD have + * skipped, and the probe asserts Vale lints it. That is the assertion that + * `.tex` and `.rmd` are not markup, and it is the one the first hand-written + * table got backwards. * - **converter-dependent** is separated from everything by failing. * * See taskless/cli#151. @@ -441,6 +449,10 @@ withVale("Vale engine capabilities", () => { prose: "We simply do it.\n", skipped: "Fine.\n\n```\nsimply\n```\n", }, + ".mdown": { + prose: "We simply do it.\n", + skipped: "Fine.\n\n```\nsimply\n```\n", + }, ".org": { prose: "We simply do it.\n", skipped: "# simply\nFine.\n" }, ".htm": { prose: "

We simply do it.

\n", @@ -459,14 +471,32 @@ withVale("Vale engine capabilities", () => { it("reports the pinned version", () => { // VALE_VERSION is rendered beside the reach lists in route.txt and // create-vale-rule.txt, so it is the attribution for every claim below. - // It also gates one of them: Vale 3.18.0 parses MDX natively, and a bump - // past it must move `.mdx` out of VALE_CONVERTER_DEPENDENT. + // It also gates two of them, in opposite directions: Vale 3.18.0 parses MDX + // natively, so a bump past it moves `.mdx` from converter-dependent to + // markup — and the same release adds a Typst converter, so it moves `.typ` + // from plaintext to `converter:typst2vast`. A bump re-measures the whole + // table; these two are only the rows already known to move. const result = spawnSync(binary as string, ["--version"], { encoding: "utf8", }); expect(result.stdout.trim()).toBe(`vale version ${VALE_VERSION}`); }); + it("probes every row of VALE_FORMAT_TIERS", () => { + // The table is the claim and this is its coverage check: every extension in + // it is reached by one of the tier suites below, so a row cannot be added + // without being measured. Reconciling two independently written tables + // turned up six rows that disagreed, every one of them in a tier nothing + // probed. + const probed = [ + ...VALE_MARKUP_EXTENSIONS, + ...VALE_COMMENT_EXTENSIONS, + ...VALE_PLAINTEXT_EXTENSIONS, + ...VALE_CONVERTER_DEPENDENT.flatMap(({ extensions }) => extensions), + ].toSorted(); + expect(probed).toEqual(Object.keys(VALE_FORMAT_TIERS).toSorted()); + }); + it("covers every markup extension in VALE_MARKUP_EXTENSIONS", () => { // A fixture missing here would let an extension be added to the constant // without ever being probed, which is the drift the constant exists to @@ -502,6 +532,47 @@ withVale("Vale engine capabilities", () => { } ); + /** + * For each listed plaintext extension, the construct a parser for the format + * its spelling suggests would have skipped. + * + * Vale lints it, which is the whole finding: `.tex` is not TeX to Vale and + * `.rmd` is not R Markdown. The mirror image of the markup fixtures — same + * documents, opposite expectation. + */ + const PLAINTEXT_FIXTURES: Record = { + ".mkd": "Fine.\n\n```\nsimply\n```\n", + ".mkdn": "Fine.\n\n```\nsimply\n```\n", + ".rmd": "Fine.\n\n```{r}\nsimply <- 1\n```\n", + ".tex": "% simply in a comment\nFine.\n", + ".typ": "// simply\nFine.\n", + }; + + it("covers every extension in VALE_PLAINTEXT_EXTENSIONS", () => { + expect(Object.keys(PLAINTEXT_FIXTURES).toSorted()).toEqual( + [...VALE_PLAINTEXT_EXTENSIONS].toSorted() + ); + }); + + it.each([...VALE_PLAINTEXT_EXTENSIONS])( + "reads %s as plaintext, lint-through-syntax and all", + (extension) => { + // A bare line fires: the tier of last resort has no syntax to hide behind. + expect( + findings(`bare${extension}`, "simply\n"), + `${extension} ignored a bare line — it has a parser, and is not plaintext` + ).toHaveLength(1); + // And the format's own non-prose syntax fires too, which is what makes it + // plaintext rather than markup. `.mkdn` and `.mkd` are the cautionary + // pair: they read as Markdown spellings and Vale lints straight through a + // fenced code block in both. + expect( + findings(`syntax${extension}`, PLAINTEXT_FIXTURES[extension]!), + `${extension} skipped its own syntax — it is parsed, and belongs in a markup or comment tier` + ).toHaveLength(1); + } + ); + it("lints an unrecognized extension as whole-file prose", () => { // The fallback, stated as an assertion because it is a routing hazard // rather than a convenience: `.yml` has no parser, so a Vale rule scoped to @@ -563,6 +634,9 @@ withVale("Vale engine capabilities", () => { it("renders each list as recipe prose with no gaps", () => { expect(valeMarkupList().split(", ")).toEqual([...VALE_MARKUP_EXTENSIONS]); expect(valeCommentList().split(", ")).toEqual([...VALE_COMMENT_EXTENSIONS]); + expect(valePlaintextList().split(", ")).toEqual([ + ...VALE_PLAINTEXT_EXTENSIONS, + ]); for (const { extensions, converter } of VALE_CONVERTER_DEPENDENT) { expect(valeConverterList()).toContain( `${extensions.join("/")} (needs ${converter})` From 975d9facc9003fd07dac74bc4b3bf50a36a83296 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Sun, 23 Aug 2026 23:13:55 -0700 Subject: [PATCH 3/4] fix(cli): give .asc and .rest their Vale checker tags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reconciled table added `.asc` and `.rest` as converter-dependent, but VALE_CONVERTER_CHECKERS arrived from the branch below without them, so the per-extension probe had no tag to assert and the set-equality case failed. Measured against the shipped 3.17.1: `.asc` reports `[lintAdoc]` and `.rest` reports `[lintRST]`, matching their canonical spellings. Working as intended — the set-equality assertion exists so that an extension added to one list and not the other fails here rather than throwing on an undefined tag inside the probe. Also carries the `.xml` correction into the reconciled table, which is now the single source: it names `xsltproc and an XSLT stylesheet` rather than `an XSLT transform`, because the program alone does not make `.xml` lintable. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Cyga14bww8rmazH2XrF8ms --- packages/cli/src/rules/capabilities.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/cli/src/rules/capabilities.ts b/packages/cli/src/rules/capabilities.ts index 96ae3777..279c4935 100644 --- a/packages/cli/src/rules/capabilities.ts +++ b/packages/cli/src/rules/capabilities.ts @@ -337,8 +337,10 @@ export const VALE_CONVERTER_DEPENDENT: readonly ValeConverterFormat[] = */ export const VALE_CONVERTER_CHECKERS: Readonly> = { ".rst": "lintRST", + ".rest": "lintRST", ".adoc": "lintAdoc", ".asciidoc": "lintAdoc", + ".asc": "lintAdoc", ".xml": "lintXML", ".dita": "lintDITA", ".mdx": "lintMDX", From b6498f2f364ac86cb31eae085e03847491df2ac6 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Sun, 23 Aug 2026 23:42:03 -0700 Subject: [PATCH 4/4] fix(cli): state converter-backed formats as unsupported, not as needing a tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Maintainer decision: we do not offer support for any Vale format that parses through an external program. The skip notice used to end "Install it and put it on your PATH to have these files checked." That offered a path this build does not ship and does not test, and for `.xml` cannot deliver at all — an XSLT stylesheet is specific to the document, so no install makes it lintable. It also made behaviour host-dependent: macOS ships /usr/bin/xsltproc and typical Linux CI images do not, so the same repository checked differently depending on the machine. That split is what made this stack red in CI while green locally, which is the strongest argument for excluding unconditionally: a repository should check the same way everywhere. The exclusion mechanism is unchanged — it is what prevents the total-run crash. What changed is the promise. The programs are still named, as the reason rather than as a remedy, and the recipes now say plainly that installing them does not change the answer. The cost, accepted: a user who genuinely has asciidoctor installed loses `.adoc` checking that would have worked. Also reconciles the comment tier against Vale's own documentation, and sharpens the version-bump note now that the 3.18.0 picture is confirmed: `.mdx` gains a native parser and becomes supported, `.typ` gains one that shells out to typst2vast and so stays unsupported permanently, and MyST, Quarto and QDoc arrive needing no external program. All four require 3.18.0, so none is reachable from the pinned 3.17.1. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Cyga14bww8rmazH2XrF8ms --- .changeset/vale-converter-formats.md | 21 +++++++++++ packages/cli/src/agent/create-vale-rule.txt | 11 ++++-- packages/cli/src/agent/route.txt | 11 ++++-- packages/cli/src/rules/capabilities.ts | 36 ++++++++++++++++--- packages/cli/src/rules/vale/formats.ts | 17 +++++---- packages/cli/test/vale-formats.test.ts | 21 +++++++---- .../cli/test/vale-vendor-contract.test.ts | 13 +++++++ 7 files changed, 107 insertions(+), 23 deletions(-) diff --git a/.changeset/vale-converter-formats.md b/.changeset/vale-converter-formats.md index 0f0eafc7..2725d48d 100644 --- a/.changeset/vale-converter-formats.md +++ b/.changeset/vale-converter-formats.md @@ -48,3 +48,24 @@ expression goes path-wise, at which point a bare `*.adoc` stops matching tested. Vale's error output is also decoded now rather than forwarded verbatim, so a failure reads as a sentence naming the missing program instead of a five-field JSON object. + +**These formats are now stated as unsupported rather than as needing a tool.** +The notice used to end "Install it and put it on your PATH to have these files +checked", which offered a path this build does not ship, does not test, and for +`.xml` cannot deliver — an XSLT stylesheet is specific to the document, so no +install makes it lintable. It also made behaviour host-dependent: macOS ships +`/usr/bin/xsltproc` and typical Linux CI images do not, so the same repository +checked differently depending on the machine. The exclusion is unconditional +for that reason, and the programs are still named as the reason rather than as +a remedy. + +The comment tier was reconciled against Vale's own documentation at +docs.vale.sh/formats/code, which adds `.bsh`, `.csx`, `.pod`, `.py3` and `.sbt` +once measured. It also documents `.pyi`, `.qml` and `.scss` as comment-aware, +and on the pinned 3.17.1 a bare non-comment line in each of them lints — so they +stay in the plaintext tier. That divergence is the argument for probing rather +than transcribing: the docs describe the current Vale, this build pins an older +one, and copying the list would have shipped `.scss` as comment-aware and been +wrong. `.pod` is a reminder of how easily this is misread — it lints Perl +comments but not POD blocks, so probing it with `=head1` looks like no support +at all. diff --git a/packages/cli/src/agent/create-vale-rule.txt b/packages/cli/src/agent/create-vale-rule.txt index 1136fc33..7df1ceda 100644 --- a/packages/cli/src/agent/create-vale-rule.txt +++ b/packages/cli/src/agent/create-vale-rule.txt @@ -316,11 +316,16 @@ it. not what the rule means, narrow the glob rather than accepting it. These land here despite reading like markup, so a `scope:` value has nothing to act on in them: %(VALE_PLAINTEXT_FORMATS)s - - **cannot be read at all** — Vale supports these formats but shells - out to an external converter to parse them, and this CLI ships none - of those converters: + - **not supported** — Vale parses these only by shelling out to an + external program, and this build does not support any format that + needs one: %(VALE_CONVERTER_FORMATS)s + Do not tell the user to install the program. Taskless excludes these + files from the run whatever is installed, so that a repository + checks the same way on every machine; `.xml` could not work anyway, + since an XSLT stylesheet is specific to the document. + **A single unreadable file fails the whole Vale pass.** Vale exits 2 with an `E100` runtime error and abandons the run — `--no-exit` does not suppress it — so every other Vale rule over every other file goes diff --git a/packages/cli/src/agent/route.txt b/packages/cli/src/agent/route.txt index 0cf9220a..4832a3a4 100644 --- a/packages/cli/src/agent/route.txt +++ b/packages/cli/src/agent/route.txt @@ -124,11 +124,16 @@ answered together. scoped to YAML flags the code as readily as the comments. That is rarely what was asked for — say so before writing it. These read like markup and are not: %(VALE_PLAINTEXT_FORMATS)s - - *cannot be read at all* — Vale supports these formats but shells - out to an external converter to parse them, and this CLI ships - none of those converters: + - *not supported* — Vale parses these only by shelling out to an + external program, and this build does not support any format that + needs one: %(VALE_CONVERTER_FORMATS)s + Installing the program does not change this. Taskless excludes + these files from the Vale run whatever is on the machine, so that + a repository checks the same way everywhere rather than depending + on what a given host happens to have available. + **One such file fails the entire Vale pass, not just that file.** Vale exits 2 with an `E100` runtime error, `--no-exit` does not suppress it, and every other Vale rule over every other file goes diff --git a/packages/cli/src/rules/capabilities.ts b/packages/cli/src/rules/capabilities.ts index 279c4935..45079323 100644 --- a/packages/cli/src/rules/capabilities.ts +++ b/packages/cli/src/rules/capabilities.ts @@ -156,11 +156,24 @@ const CONVERTER_TIER_PREFIX = "converter:"; * are a property of {@link VALE_VERSION}'s binary, and the dangerous direction * is a format Vale *learns*: an extension missing from this table is read as * plain text today, but the moment Vale routes it to a converter the same - * omission is a crash that takes down every Vale rule in the run. Vale 3.18.0 - * is the known incoming case in both directions — it parses MDX natively, and - * it adds a Typst converter, so `.typ` moves from `plaintext` to - * `converter:typst2vast` on that bump while `.mdx` moves to `markup`. Neither - * happens on its own; re-probe every row. + * omission is a crash that takes down every Vale rule in the run. + * + * Vale 3.18.0 is the known incoming bump, and it moves rows in three different + * directions — which is why "re-measure" is not boilerplate here: + * + * - `.mdx` gains a native parser, so it moves `converter:mdx2vast` → `markup` + * and becomes supported. + * - `.typ` gains a parser that shells out to `typst2vast` + * (https://docs.vale.sh/formats/typst), so it moves `plaintext` → + * `converter:typst2vast`. That is the dangerous direction: today it is read + * as prose, and after the bump the same row would crash the run. It also + * stays unsupported permanently, since we do not support formats needing an + * external program. + * - MyST, Quarto and QDoc arrive with parsers needing no external program, so + * they become genuinely supportable and want `markup` rows once measured. + * + * All four are documented as requiring v3.18.0 or later, so none of them is + * reachable from {@link VALE_VERSION}. Re-probe every row on the bump. */ export const VALE_FORMAT_TIERS: Readonly> = { // markup — parsed, the format's own constructs skipped @@ -174,11 +187,13 @@ export const VALE_FORMAT_TIERS: Readonly> = { // comment text only — the code body is invisible ".c": "comment", ".c++": "comment", + ".bsh": "comment", ".cc": "comment", ".clj": "comment", ".cpp": "comment", ".cs": "comment", ".css": "comment", + ".csx": "comment", ".cxx": "comment", ".go": "comment", ".h": "comment", @@ -194,15 +209,18 @@ export const VALE_FORMAT_TIERS: Readonly> = { ".php": "comment", ".pl": "comment", ".pm": "comment", + ".pod": "comment", ".proto": "comment", ".ps1": "comment", ".py": "comment", + ".py3": "comment", ".pyw": "comment", ".r": "comment", ".R": "comment", ".rb": "comment", ".rs": "comment", ".sass": "comment", + ".sbt": "comment", ".scala": "comment", ".swift": "comment", ".ts": "comment", @@ -213,6 +231,14 @@ export const VALE_FORMAT_TIERS: Readonly> = { ".rmd": "plaintext", ".tex": "plaintext", ".typ": "plaintext", + // plaintext HERE, though Vale's own docs list them as comment-tier. The docs + // describe the CURRENT Vale; we pin 3.17.1. Measured on the pinned binary a + // bare non-comment line lints, which is the plaintext signature. Transcribing + // the docs would have shipped these as comment-tier and been wrong for this + // build — the case for probing rather than copying. + ".pyi": "plaintext", + ".qml": "plaintext", + ".scss": "plaintext", // converter-dependent — Vale supports the format, we ship no converter ".adoc": "converter:asciidoctor", ".asc": "converter:asciidoctor", diff --git a/packages/cli/src/rules/vale/formats.ts b/packages/cli/src/rules/vale/formats.ts index 61924075..3650376c 100644 --- a/packages/cli/src/rules/vale/formats.ts +++ b/packages/cli/src/rules/vale/formats.ts @@ -81,11 +81,16 @@ export const TASKLESS_DIRECTORY = ".taskless"; * Vale": the moment Vale learns a format, it starts routing that extension to a * parser, and if that parser shells out, an extension missing from the table is * a crash rather than a plain-text read. Vale 3.18.0 is the live example — it - * adds Typst, which converts through `typst2vast`, so `.typ` stops being the + * adds Typst, which parses through `typst2vast`, so `.typ` stops being the * plaintext read it is today and upgrading the `@taskless/vale-*` packages * without re-measuring would reintroduce exactly this bug under a new * extension. Re-measure the whole table on every bump; the per-extension cases * in `vale-vendor-contract.test.ts` are how. + * + * Note what the exclusion policy buys here. Because a format needing an + * external program is never supported, a newly-converter-backed extension has + * one correct destination rather than a judgement call, and the answer does not + * depend on what happens to be installed on the machine running `check`. */ /** @@ -240,10 +245,10 @@ export function skippedFilesNotice(files: string[]): string | undefined { : sample.join(", "); return ( - `Vale did not check ${String(files.length)} file(s): ${listed}. Vale ` + - `supports these formats, but parsing them needs an external converter ` + - `(${converters.join(", ")}) that this build does not ship. Install it and ` + - `put it on your PATH to have these files checked; every other file was ` + - `checked normally.` + `Vale did not check ${String(files.length)} file(s): ${listed}. These ` + + `formats are not supported by this build — Vale parses them only through ` + + `an external program (${converters.join(", ")}), which this build does ` + + `not ship and does not check for. Scope the rule to a supported format; ` + + `every other file was checked normally.` ); } diff --git a/packages/cli/test/vale-formats.test.ts b/packages/cli/test/vale-formats.test.ts index 35930b67..44310cb3 100644 --- a/packages/cli/test/vale-formats.test.ts +++ b/packages/cli/test/vale-formats.test.ts @@ -153,16 +153,25 @@ describe("the skipped-files notice", () => { expect(skippedFilesNotice([])).toBeUndefined(); }); - it("names the files, the converter, and that the rest was checked", () => { - const notice = skippedFilesNotice(["docs/guide.adoc", "spec/api.rst"]); + it("names the files and says the format is unsupported, not that a tool is missing", () => { + const notice = + skippedFilesNotice(["docs/guide.adoc", "spec/api.rst"]) ?? ""; expect(notice).toContain("docs/guide.adoc"); expect(notice).toContain("spec/api.rst"); + expect(notice).toContain("every other file was checked normally"); + // The programs are still named — they are the REASON, and a user reading + // "asciidoctor" understands what kind of gap this is. expect(notice).toContain("asciidoctor"); expect(notice).toContain("rst2html"); - expect(notice).toContain("every other file was checked normally"); - // Vale supports these formats; this build cannot parse them. Telling a user - // otherwise sends them to the wrong project's issue tracker. - expect(notice).toContain("Vale supports these formats"); + // But the notice must not read as an offer. We do not support any format + // that needs an external program, so telling a user to install one promises + // a path that is untested, and for `.xml` impossible — an XSLT stylesheet is + // specific to the document. It would also make behaviour host-dependent: + // macOS ships /usr/bin/xsltproc and Linux CI images do not, so the same + // repository would check differently per machine. + expect(notice).toContain("not supported by this build"); + expect(notice).not.toMatch(/install/i); + expect(notice).not.toMatch(/\bPATH\b/); }); it("summarizes rather than printing an unbounded file list", () => { diff --git a/packages/cli/test/vale-vendor-contract.test.ts b/packages/cli/test/vale-vendor-contract.test.ts index d22f4ae1..cd780969 100644 --- a/packages/cli/test/vale-vendor-contract.test.ts +++ b/packages/cli/test/vale-vendor-contract.test.ts @@ -395,13 +395,21 @@ withVale("Vale vendor contract", () => { * See taskless/cli#151. */ /** The comment syntax Vale must see through, per extension. */ +/** + * A correct comment for `extension`, which is the whole difficulty of this + * file: a wrong delimiter reads exactly like absent support. `.css` looks + * unsupported if fed `//` and is comment-aware with a block comment; `.pod` is + * linted as Perl, so a `#` comment fires while a `=head1` POD block does not. + */ function comment(extension: string): string { const HASH = new Set([ ".jl", ".pl", ".pm", ".ps1", + ".pod", ".py", + ".py3", ".pyw", ".r", ".R", @@ -546,6 +554,11 @@ withVale("Vale engine capabilities", () => { ".rmd": "Fine.\n\n```{r}\nsimply <- 1\n```\n", ".tex": "% simply in a comment\nFine.\n", ".typ": "// simply\nFine.\n", + // Documented by Vale as comment-aware, measured as plaintext on the pinned + // binary — the construct here is the comment a parser would have skipped. + ".pyi": "# simply\nFine.\n", + ".qml": "// simply\nFine.\n", + ".scss": "// simply\nFine.\n", }; it("covers every extension in VALE_PLAINTEXT_EXTENSIONS", () => {