fix(vale): make the Vale 3.24.0 upgrade land green, and refuse a rule's tests: key

Vale 3.24.0 warns W101 about every key it does not recognise inside a
matcher, including the `tskl) rule` breadcrumb. That printed one notice
per matcher on every check, and the warning ahead of a fatal error's
JSON broke the per-file narrowing from #300. Assembly now
comments the breadcrumb out of the config Vale reads, and runVale peels
Vale's warning lines off before parsing its error, one notice each.

The converter E100 dropped its `[lintAdoc]` checker tag, so the
contract asserts the format Vale now names instead. New 3.24.0 pins
cover the W101, warning-then-error order, C# tool directives, a
non-breaking space after a link, and files no rule runs on.

`validate` and `verify` now reject a Vale rule's `tests:` key: Taskless
verifies Vale rules by their .tests/pass and .tests/fail documents and
never runs `vale test`.

Author: theCodeDrift

.changeset/vale-3-24-0.md

@@ -3,3 +3,9 @@
 ---
 
 Update the bundled Vale to 3.24.0.
+
+Vale 3.24.0 warns about any key it does not recognise inside a matcher, including the `tskl) rule` breadcrumb every Vale rule config carries. The CLI now comments the breadcrumb out of the config it assembles for Vale, so `check` no longer prints one notice per matcher, and a file Vale cannot parse is again set aside with its own finding instead of failing the whole Vale run. Vale's warnings now arrive as one notice each.
+
+Two findings change for existing rules: C# `// ReSharper disable` and `// NOSONAR` comments are no longer linted, and a word after a link and a non-breaking space is now matched. `update` (topic v18) walks both.
+
+`validate` and `verify` now reject a Vale rule's `tests:` key. Vale 3.24.0 runs those cases with `vale test`, which Taskless never calls; a Vale rule is verified by its `.tests/pass` and `.tests/fail` documents.

packages/cli/src/agent/update.md

@@ -497,8 +497,9 @@ loaded it without complaint.
 
 ### Migrating to 0.13.3
 
-The vendored ast-grep moves from 0.45.3 to 0.51.0. Nothing here needs
-work by hand.
+The vendored ast-grep moves from 0.45.3 to 0.51.0, and Vale from 3.23.0
+to 3.24.0. Nothing here needs work by hand except a Vale rule carrying
+a `tests:` key, described at the end.
 
 **A Haskell type synonym is `type_synonym` now, not `type_synomym`, and
 `update --rules` renames it for you.** ast-grep's Haskell grammar moved
@@ -526,6 +527,44 @@ files that was routed to the runtime tier, or not written because
 ast-grep could not parse Zig, can now be an sg rule. No existing rule
 changes.
 
+Two things in Vale 3.24.0 change which findings an existing rule
+reports, each measured against both binaries on the same fixture.
+
+**A C# tool directive is no longer read.** A `// ReSharper disable`,
+`// ReSharper restore` or `// NOSONAR` line in a `.cs` file was linted
+as an ordinary comment, so a rule over comments fired on a suppression
+list. It is now skipped, as 3.23.0 already did for `//nolint`, `# noqa`
+and `eslint-disable`. Findings disappear; none appear.
+
+**A word after a link and a non-breaking space is matched.** Through
+3.23.0 a U+00A0 or U+202F after inline markup was dropped, fusing the
+link text and the next word, so a token there went unreported. It is
+now a word break, and a rule over that word starts reporting.
+
+What the CLI absorbed, so you do not have to: Vale 3.24.0 warns `W101`
+about every key it does not recognise inside a matcher, which includes
+the `tskl) rule` breadcrumb every rule config carries. Keep writing the
+breadcrumb; the CLI comments it out of the file it hands Vale, so no
+notice appears. A file no rule runs on is no longer parsed, which
+`check` already approximated by setting converter-dependent files
+aside, so nothing changes there.
+
+Additions a rule config can use: a matcher may carry a condition,
+`[*.md if .Meta.draft]`, which applies it only to the matched files
+whose front matter satisfies it. `validate` accepts it.
+
+**A rule's `tests:` key is refused.** Vale 3.24.0 ships `vale test`,
+which runs cases written in a rule's `tests:` list. Taskless never runs
+it: `verify` judges a Vale rule by the documents under `.tests/pass`
+and `.tests/fail`. So `validate` and `verify` now reject the key, which
+0.12.0 had started accepting, rather than let a rule look tested by
+cases nothing runs. `check` still runs such a rule. Search
+`.taskless/rules/vale/*/*.yml` for a top-level `tests:`; for each case,
+write its `input` as a document under `.tests/fail` if the case expects
+the rule to fire, or `.tests/pass` if it expects no alert, named with
+the case's `format` as its extension (`.md` when it has none). Then
+delete the key and run `verify` on the rule.
+
 ## Errors
 
 With `--json`, `--rules` failures emit `{ ok: false, code, message }`:

packages/cli/src/rules/assemble.ts

@@ -83,25 +83,45 @@ function valeHeader(): string {
 }
 
 /**
- * A rule's assembled block: its breadcrumb comment, then its config verbatim.
+ * A rule's assembled block: its breadcrumb comment, then its config verbatim
+ * except that each breadcrumb is commented out.
  *
  * The config is the generator's structured input and is never re-serialized.
  * The schema has already read it as an AST, for validation and for the matcher
  * list; what Vale reads is the file's own bytes, so matcher order, comments and
  * spacing survive exactly as the author wrote them. Matcher order inside a rule
  * is the author's expression of precedence, so anything short of verbatim
- * would be a semantic edit. The only addition is a newline after a source that
+ * would be a semantic edit. The one addition is a newline after a source that
  * lacks one, so the next rule's breadcrumb starts its own line.
  *
+ * The one edit is `# ` in front of each `tskl) rule` line, at the lines the
+ * schema located (`breadcrumbLines`), so nothing here searches the text for
+ * it. That is not a semantic edit: Vale read the key as a rule named
+ * `tskl) rule` switched off, which is nothing. What changed in 3.24.0 is that
+ * it says so, `W101 'tskl) rule' isn't a section option`, once per matcher on
+ * every run, and that line on stderr in front of a real error's JSON is what
+ * kept a file Vale cannot parse from being narrowed out of the run
+ * (taskless/cli#300). Commenting rather than deleting keeps every line where
+ * the author put it, so a line number Vale reports is still the author's.
+ *
  * This is also why the schema, not this module, turns away a copied-in
  * `StylesPath` or `MinAlertLevel`. Assembly used to strip those two by
  * splitting each line on `=`; that was the one string edit it performed, and
  * it meant the file Vale read was not the file the author wrote. A run-level
  * key above the first matcher is now a rejection, so there is nothing left to
  * strip.
  */
-function valeRuleBlock(ruleId: string, source: string): string {
-  const terminated = source.endsWith("\n") ? source : `${source}\n`;
+function valeRuleBlock(
+  ruleId: string,
+  source: string,
+  breadcrumbLines: readonly number[]
+): string {
+  const lines = source.split("\n");
+  for (const line of breadcrumbLines) {
+    lines[line - 1] = `# ${lines[line - 1] ?? ""}`;
+  }
+  const commented = lines.join("\n");
+  const terminated = commented.endsWith("\n") ? commented : `${commented}\n`;
   return `# tskl) rule = ${ruleId}\n${terminated}`;
 }
 
@@ -243,7 +263,7 @@ export async function assembleValeConfig(
     if (selected !== undefined && !selected.has(ruleId)) continue;
     for (const pattern of verdict.sections) sections.add(pattern);
     advisories.push(...verdict.advisories);
-    blocks.push(valeRuleBlock(ruleId, source));
+    blocks.push(valeRuleBlock(ruleId, source, verdict.breadcrumbLines));
   }
 
   if (refusals.length > 0) return { status: "refused", refusals };

packages/cli/src/rules/capabilities.ts

@@ -569,30 +569,34 @@ export const VALE_CONVERTER_DEPENDENT: readonly ValeConverterFormat[] =
   groupByConverter();
 
 /**
- * Vale's checker tag per converter-dependent extension, from the `E100` text.
- *
- * This is the host-independent half of the failure. The prose after the tag is
- * not: `.xml` reports `xsltproc not found` where the program is absent and
- * `no XSLT transform provided` where it is present, and the two are split by
- * platform — macOS ships `/usr/bin/xsltproc`, the Linux CI image does not. A
- * contract test that matched on the program name therefore passed locally and
- * failed in CI, which is how this list came to exist.
+ * The format Vale names per converter-dependent extension, from the `E100` text.
+ *
+ * Since 3.24.0 the error reads `asciidoctor not found; it's needed to read
+ * AsciiDoc`, and the format at the end is the host-independent half. Through
+ * 3.23.0 that half was a checker tag, `E100 [lintAdoc] Runtime error`, which
+ * 3.24.0's typed errors dropped. The program name is not host-independent:
+ * `.xml` reports `xsltproc not found` where the program is absent and `no XSLT
+ * transform provided` where it is present, and the two are split by platform —
+ * macOS ships `/usr/bin/xsltproc`, the Linux CI image does not. A contract
+ * test that matched on the program name therefore passed locally and failed in
+ * CI, which is how this list came to exist.
  *
  * `.xml` is also the one entry whose converter is not sufficient on its own. An
  * XSLT transform is document-specific, so there is no default to ship and
  * installing `xsltproc` does not make `.xml` lintable — unlike `asciidoctor`,
  * which genuinely fixes `.adoc`. That is why its `converter` names the
- * stylesheet as well as the program.
+ * stylesheet as well as the program, and why the stylesheet message, which
+ * names no format, is the one case the contract test accepts in its place.
  */
-export const VALE_CONVERTER_CHECKERS: Readonly<Record<string, string>> = {
-  ".rst": "lintRST",
-  ".rest": "lintRST",
-  ".adoc": "lintAdoc",
-  ".asciidoc": "lintAdoc",
-  ".asc": "lintAdoc",
-  ".xml": "lintXML",
-  ".dita": "lintDITA",
-  ".typ": "lintTypst",
+export const VALE_CONVERTER_READERS: Readonly<Record<string, string>> = {
+  ".rst": "reStructuredText",
+  ".rest": "reStructuredText",
+  ".adoc": "AsciiDoc",
+  ".asciidoc": "AsciiDoc",
+  ".asc": "AsciiDoc",
+  ".xml": "XML",
+  ".dita": "DITA",
+  ".typ": "Typst",
 };
 
 /** Every converter-dependent extension, flattened. */

packages/cli/src/rules/vale/formats.ts

@@ -24,9 +24,9 @@ export const TASKLESS_DIRECTORY = ".taskless";
  * 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
+ * conversion fails, and Vale does not degrade: it prints one `E100` object
+ * (`asciidoctor not found; it's needed to read AsciiDoc`) 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.
  *
@@ -108,8 +108,8 @@ export const CONVERTER_DEPENDENT_EXTENSIONS: readonly string[] = [
  * The converter Vale would need for `path`, or `undefined` if it needs none.
  *
  * Extension comparison is **case-sensitive, because Vale's own routing is**.
- * Measured against the pinned binary: `docs/guide.adoc` exits 2 with
- * `E100 [lintAdoc]`, while `docs/guide.ADOC` and `docs/guide.AdOc` are read as
+ * Measured against the pinned binary: `docs/guide.adoc` exits 2 with an
+ * `E100`, while `docs/guide.ADOC` and `docs/guide.AdOc` are read as
  * plain text and exit 0 with findings — including when the uppercase spelling
  * names a lowercase file on a case-insensitive filesystem, since Vale routes on
  * the path string it was handed, not on the name the disk holds.

packages/cli/src/rules/vale/run.ts

@@ -129,6 +129,39 @@ function parseValeConfigError(stderr: string): ValeConfigError | undefined {
   }
 }
 
+/** A line Vale's `core.Warn` wrote: its code, a space, then the message. */
+const VALE_WARNING_LINE = /^W\d{3} /;
+
+/**
+ * Vale's stderr, split into the warnings it opens with and what follows.
+ *
+ * Vale writes each warning as its own `W101 <message>` line as it meets it,
+ * and a fatal error as one JSON object after them, so a run can produce both:
+ * since 3.24.0 a misspelled level (`<id>.<id> = maybe`) warns and the run then
+ * dies on a document it cannot parse. Parsing the whole stream as JSON failed
+ * on the warning, which threw away the error's `Path` and with it the
+ * narrowing that keeps one unparseable file from taking every other file's
+ * findings down (taskless/cli#300).
+ *
+ * Only the leading run of warning lines is peeled off. Anything after the
+ * first line that is not one is returned whole as `rest`, so a JSON object
+ * whose `Text` happens to contain such a line is never split.
+ */
+function splitValeWarnings(stderr: string): {
+  warnings: string[];
+  rest: string;
+} {
+  const lines = stderr.split("\n");
+  const firstOther = lines.findIndex(
+    (line) => !VALE_WARNING_LINE.test(line.trim())
+  );
+  const count = firstOther === -1 ? lines.length : firstOther;
+  return {
+    warnings: lines.slice(0, count).map((line) => line.trim()),
+    rest: lines.slice(count).join("\n").trim(),
+  };
+}
+
 /**
  * Vale's stderr, rendered as a sentence instead of a JSON blob.
  *
@@ -390,13 +423,20 @@ async function spawnVale(
 
       // With --no-exit, a non-zero code is Vale failing, not Vale finding.
       if (code !== null && code !== 0) {
-        const stderr = stderrChunks.join("").trim();
-        const configError = parseValeConfigError(stderr);
+        const { warnings, rest } = splitValeWarnings(
+          stderrChunks.join("").trim()
+        );
+        const configError = parseValeConfigError(rest);
+        // The error first, because it is why the run stopped; the warnings
+        // after it, because one may be the reason (a rule switched off by a
+        // misspelled level).
+        const said = [
+          ...(rest === "" ? [] : [describeValeStderr(rest, configError)]),
+          ...warnings,
+        ].join(" ");
         settle({
           status: "failed",
-          message: `Vale exited ${String(code)}${
-            stderr === "" ? "" : `: ${describeValeStderr(stderr, configError)}`
-          }`,
+          message: `Vale exited ${String(code)}${said === "" ? "" : `: ${said}`}`,
           ...(configError === undefined ? {} : { configError }),
         });
         return;
@@ -406,16 +446,20 @@ async function spawnVale(
       // otherwise succeeded — the `W101` ignored-assignment warning above all.
       // 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 ride the same field as separate elements, rather than
+      const { warnings, rest } = splitValeWarnings(
+        stderrChunks.join("").trim()
+      );
+      // Every advisory rides the same field as its own element, 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.
+      // One element per warning for the same reason: each `W101` line is its
+      // own message, and Vale since 3.24.0 writes one per misread key.
       const notices = collectNotices([
         skipped,
-        diagnostic === ""
-          ? undefined
-          : `Vale reported while running: ${diagnostic}`,
+        ...[...warnings, rest]
+          .filter((said) => said !== "")
+          .map((said) => `Vale reported while running: ${said}`),
       ]);
 
       const stdout = stdoutChunks.join("").trim();

packages/cli/src/schemas/vale-config.ts

@@ -503,6 +503,18 @@ export interface ValeRuleConfigVerdict {
   advisories: string[];
   /** Every matcher's pattern, in source order, exactly as the header spells it. */
   sections: string[];
+  /**
+   * The 1-based line of every breadcrumb that could be located, in source
+   * order, so assembly can hide them from Vale without re-reading the text.
+   *
+   * Since Vale 3.24.0 a key Vale does not recognise inside a matcher is said
+   * aloud, `W101 'tskl) rule' isn't a section option`, once per matcher on
+   * every run. The breadcrumb is ours and Vale never needed it, so the file
+   * Vale reads carries it as a comment instead. A breadcrumb whose line could
+   * not be located is left as written: Vale then warns about it, which is
+   * noise, and the alternative of guessing a line is a wrong edit.
+   */
+  breadcrumbLines: number[];
 }
 
 /**
@@ -545,5 +557,12 @@ export function validateValeRuleConfig(
     sections: ast.sections
       .map((section) => section.name)
       .filter((name): name is string => name !== undefined),
+    breadcrumbLines: ast.sections.flatMap((section) =>
+      properties(section).flatMap((property) =>
+        property.key === VALE_BREADCRUMB_KEY && property.line !== undefined
+          ? [property.line]
+          : []
+      )
+    ),
   };
 }

packages/cli/src/schemas/vale-rule.ts

@@ -819,6 +819,36 @@ function actionMessages(
   ];
 }
 
+/**
+ * A rule's own `tests:` cases, refused as policy rather than as vocabulary.
+ *
+ * Vale reads the key: it is a common field of every check, measured accepted
+ * since 3.22.0 and announced with `vale test` in 3.24.0, and the generated
+ * vocabulary records it as one. Taskless refuses it anyway. `verify` judges a
+ * Vale rule by the documents under `.tests/pass` and `.tests/fail`, which are
+ * whole files in any format the rule reaches, and it never runs `vale test`. A
+ * rule carrying cases there would read as tested while nothing ran them, and
+ * a second place to write a rule's cases is a second place for them to drift.
+ *
+ * `test/vale-corpus.ts` carries this as a counted divergence, so the
+ * disagreement with the binary stays visible.
+ */
+function testsMessages(
+  rule: Record<string, unknown>
+): { path: PropertyKey[]; message: string }[] {
+  if (!("tests" in rule)) return [];
+  return [
+    {
+      path: ["tests"],
+      message:
+        "'tests' holds cases for `vale test`, which Taskless never runs. " +
+        "A Vale rule is verified by its fixtures: write each case as a " +
+        "document under .tests/pass (the rule must stay quiet) or " +
+        ".tests/fail (the rule must fire), then delete the key.",
+    },
+  ];
+}
+
 /**
  * The union's members, spelled out, and the guard that keeps them honest.
  *
@@ -924,6 +954,7 @@ const valeBodySchema = z
       ...fatalShapeMessages(rule),
       ...swapBackreferenceMessages(rule),
       ...actionMessages(rule),
+      ...testsMessages(rule),
     ]) {
       context.issues.push({ code: "custom", input: rule, path, message });
     }

packages/cli/test/assemble.test.ts

@@ -201,15 +201,20 @@ describe("Vale config assembly", () => {
 
   // The config is the generator's structured input and is never re-serialized:
   // the schema reads it as an AST, Vale reads the author's own bytes.
-  it("writes an accepted config verbatim under its breadcrumb", async () => {
+  // Verbatim except the breadcrumbs, which are commented out in place: Vale
+  // 3.24.0 warns W101 about an unknown key in a matcher, once per matcher on
+  // every run. In place, so every other line keeps its number.
+  it("writes an accepted config verbatim under its breadcrumb, breadcrumbs commented", async () => {
     const source =
-      "# scope\n[docs/**]\ntskl) rule = no-simply\n\nno-simply.no-simply = YES   \n";
+      "# scope\n[docs/**]\ntskl) rule = no-simply\n\nno-simply.no-simply = YES   \n\n[*.md]\n  tskl) rule = no-simply\r\nno-simply.no-simply = NO\n";
     await valeRule("no-simply", source);
     const assembled = await assembleValeConfig(cwd);
     expect(assembled?.status).toBe("ok");
     if (assembled?.status !== "ok") return;
     const contents = await readFile(join(cwd, assembled.path), "utf8");
-    expect(contents).toContain(`# tskl) rule = no-simply\n${source}`);
+    expect(contents).toContain(
+      "# tskl) rule = no-simply\n# scope\n[docs/**]\n# tskl) rule = no-simply\n\nno-simply.no-simply = YES   \n\n[*.md]\n#   tskl) rule = no-simply\r\nno-simply.no-simply = NO\n"
+    );
   });
 
   // What the schema notes without refusing rides on the result, so `check` can