From bc4869a03d734d529382cdfef3614d3bb855ae56 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 21 Sep 2026 17:43:09 -0700 Subject: [PATCH 1/3] test(vale): pin raw concatenation and front-matter scoping on the vendored binary --- .../cli/test/vale-vendor-contract.test.ts | 180 ++++++++++++++++++ 1 file changed, 180 insertions(+) diff --git a/packages/cli/test/vale-vendor-contract.test.ts b/packages/cli/test/vale-vendor-contract.test.ts index d7428d29..cff9c2dd 100644 --- a/packages/cli/test/vale-vendor-contract.test.ts +++ b/packages/cli/test/vale-vendor-contract.test.ts @@ -160,6 +160,17 @@ function lines( const hedge = (scope: string) => `extends: existence\nmessage: "hedge: %s"\nlevel: warning\nscope: '${scope}'\ntokens:\n - worth noting\n`; +/** + * An existence rule with one `tokens` or `raw` entry, `message: "%s"` so a + * finding's message is the match, and `extra` lines above the list. + */ +const existenceOver = ( + key: "tokens" | "raw", + pattern: string, + extra = "" +): string => + `extends: existence\nmessage: "%s"\nlevel: warning\n${extra}${key}:\n - ${pattern}\n`; + /** A word budget of 8 over whatever `scope` names. */ const budget = (scope: string) => `extends: metric\nmessage: "%s words"\nlevel: error\nscope: '${scope}'\nformula: words\ncondition: "> 8"\n`; @@ -671,6 +682,175 @@ withVale("Vale vendor contract", () => { }); }); + /** + * How an `existence` rule's `raw` list is read, and what `nonword` does and + * does not govern. Both from the 2026-09-21 dogfood notes (taskless/cli#360): + * a second `raw` entry added to a working rule never fired, and its fail + * fixture failed, with no error anywhere. + * + * Depended on by: the `verify` advisory in `schemas/vale-rule.ts` that + * warns on a multi-entry `raw`, and the recipe's guidance to write + * alternation as one `(a|b)` entry. If Vale ever started treating the + * entries as alternatives the advisory would be telling authors to fix a + * rule that works. + */ + describe("existence `raw` entries join into one pattern", () => { + /** Two entries, each a real pattern on its own. */ + const twoEntries = + 'extends: existence\nmessage: "%s"\nlevel: warning\nraw:\n' + + ' - "\\\\bstops being\\\\b"\n' + + ' - "[^.]{0,20}\\\\band becomes\\\\b"\n'; + /** The same two patterns as one entry, alternated. */ + const oneAlternation = + 'extends: existence\nmessage: "%s"\nlevel: warning\nraw:\n' + + ' - "(\\\\bstops being\\\\b|[^.]{0,20}\\\\band becomes\\\\b)"\n'; + const secondOnly = "It and becomes fun.\n"; + const firstOnly = "It stops being fun.\n"; + const both = "It stops being dull and becomes fun.\n"; + + it("never matches an entry on its own", () => { + // Vale joins `raw` with no separator (`strings.Join(rule.Raw, "")`), + // so the list is one regex whose pieces happen to be on separate + // lines. A line that matches only the second entry is silent, and so + // is one that matches only the first. + expect(lines(twoEntries, secondOnly).lines).toEqual([]); + expect(lines(twoEntries, firstOnly).lines).toEqual([]); + }); + + it("matches the entries in sequence, as the one regex they became", () => { + expect(lines(twoEntries, both).messages).toEqual([ + "stops being dull and becomes", + ]); + }); + + it("joins three the same way: the middle entry alone is silent", () => { + const three = + 'extends: existence\nmessage: "%s"\nlevel: warning\nraw:\n' + + ' - "alpha"\n - "bravo"\n - "charlie"\n'; + expect( + lines(three, "bravo alone. alphabravocharlie together.\n").messages + ).toEqual(["alphabravocharlie"]); + }); + + it("fires on either branch when the alternation is inside one entry", () => { + // The form the recipe teaches. Same two patterns, one entry. + expect(lines(oneAlternation, secondOnly).messages).toEqual([ + "It and becomes", + ]); + expect(lines(oneAlternation, firstOnly).messages).toEqual([ + "stops being", + ]); + }); + }); + + describe("`nonword` governs `tokens`, not `raw`", () => { + // Vale wraps the pattern in `\b…\b` only when the rule has `tokens` and + // `nonword` is unset, and `raw` is inserted verbatim either way. So a + // `tokens` entry that starts or ends on punctuation needs `nonword: true` + // to be reachable at all (`\b` needs a word character on one side), while + // a `raw` pattern edged on punctuation fires with or without it. The + // corpus row `field/existence+nonword` (`vale-corpus.ts`) proves the + // em dash token fires WITH the key; this is the other half, that it is + // silent without it, and that `raw` does not care. + // + // Depended on by: the recipe's `nonword` guidance, which must say which + // key the flag is for. Issue #360's worked example carries `nonword: + // true` beside a `raw` list, where it is harmless and does nothing. + const emDash = "This is a sentence — with an em dash.\n"; + const twist = "That is the twist. Yes.\n"; + const twistPattern = String.raw`"(The|That|This) is the twist\\."`; + const NONWORD = "nonword: true\n"; + + it("a punctuation-only token is silent until nonword: true", () => { + expect(lines(existenceOver("tokens", '"—"'), emDash).lines).toEqual([]); + expect( + lines(existenceOver("tokens", '"—"', NONWORD), emDash).messages + ).toEqual(["—"]); + }); + + it("a raw pattern edged on punctuation fires with or without it", () => { + expect(lines(existenceOver("raw", '"—"'), emDash).messages).toEqual([ + "—", + ]); + expect( + lines(existenceOver("raw", '"—"', NONWORD), emDash).messages + ).toEqual(["—"]); + expect(lines(existenceOver("raw", twistPattern), twist).messages).toEqual( + ["That is the twist."] + ); + expect( + lines(existenceOver("raw", twistPattern, NONWORD), twist).messages + ).toEqual(["That is the twist."]); + }); + }); + + /** + * Which scopes see YAML front matter. From the same dogfood notes + * (taskless/cli#361): a `scope: raw` casing rule flagged `target: taskless` + * in blog front matter, where lowercase is a machine key. + * + * Depended on by: the recipe's scope guidance, which must say that `raw` + * (and, measured here, the default scope) lint front matter as prose, and + * that `frontmatter.` is how a rule reaches one key and nothing else. + */ + describe("front matter under each scope", () => { + // `taskless` on line 2 (a front-matter value) and line 5 (body prose). + const document = + "---\ntarget: taskless\n---\n\nSee taskless in the body.\n"; + const at = (scope: string) => + lines( + existenceOver( + "tokens", + "taskless", + scope === "" ? "" : `scope: ${scope}\n` + ), + document + ).lines; + + it("`raw` lints a front-matter value as text", () => { + expect(at("raw")).toEqual([2, 5]); + }); + + it("so do `text` and the default scope", () => { + // The surprise is not confined to `raw`. A rule with no `scope` at all + // reads the front matter too, so the recipe cannot say "just drop + // `scope: raw`" as the fix. + expect(at("text")).toEqual([2, 5]); + expect(at("")).toEqual([2, 5]); + }); + + it("`paragraph` and `sentence` skip it", () => { + expect(at("paragraph")).toEqual([5]); + expect(at("sentence")).toEqual([5]); + }); + + it("`frontmatter.` reaches that key and never the body", () => { + expect(at("frontmatter.target")).toEqual([2]); + // A key the document does not have: silent, not an error. + expect(at("frontmatter.title")).toEqual([]); + }); + + it("`frontmatter` reaches the block and never the body", () => { + expect(at("frontmatter")).toEqual([2]); + }); + + it("`occurrence` with `min: 1` at `raw` can require a field", () => { + // The flip side, and the shape the recipe now recommends for "a draft + // needs a description". `(?m)` is what lets `^` and `$` anchor to a + // line inside the raw text. + const requireDescription = + 'extends: occurrence\nmessage: "needs a description"\nlevel: error\n' + + "scope: raw\nmin: 1\ntoken: '(?m)^description: .+$'\n"; + const without = "---\ntitle: x\n---\n\nBody.\n"; + const withField = + "---\ntitle: x\ndescription: a real one\n---\n\nBody.\n"; + expect(lines(requireDescription, without).lines).toEqual([1]); + expect(lines(requireDescription, withField).lines).toEqual([]); + // No front matter at all is the same shortfall, reported the same way. + expect(lines(requireDescription, "Body only.\n").lines).toEqual([1]); + }); + }); + /** * What 3.21.0 added that a rule under `.taskless/rules/vale/` can reach. * From 84d34596f20e6d34e29fc53255565e387d80e22c Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 21 Sep 2026 17:46:27 -0700 Subject: [PATCH 2/3] feat(verify): warn when a Vale rule's raw list has more than one entry --- .changeset/vale-raw-authoring.md | 5 ++ packages/cli/src/rules/inspect.ts | 19 +++-- packages/cli/src/schemas/vale-rule.ts | 64 +++++++++++++-- .../cli/test/vale-schema-contract.test.ts | 82 +++++++++++++++++++ .../cli/test/verify-test-commands.test.ts | 60 ++++++++++++++ 5 files changed, 216 insertions(+), 14 deletions(-) create mode 100644 .changeset/vale-raw-authoring.md diff --git a/.changeset/vale-raw-authoring.md b/.changeset/vale-raw-authoring.md new file mode 100644 index 00000000..7d21f986 --- /dev/null +++ b/.changeset/vale-raw-authoring.md @@ -0,0 +1,5 @@ +--- +"@taskless/cli": patch +--- + +`verify` warns when a Vale rule's `raw` list has more than one entry, since Vale concatenates them into one pattern rather than alternating them; the `create-vale-rule` recipe explains the `(a|b)` form. The warning rides on `notice` and does not fail the rule. diff --git a/packages/cli/src/rules/inspect.ts b/packages/cli/src/rules/inspect.ts index b8e61d80..d0996b6a 100644 --- a/packages/cli/src/rules/inspect.ts +++ b/packages/cli/src/rules/inspect.ts @@ -206,6 +206,11 @@ export async function verifyOneRule( } if (engine === "vale") { + // What is true but not invalid, from the style file and the config alike, + // rides on `notice`. Both schema layers speak in that register: the style + // layer about a `raw` list Vale will join into one pattern, the config + // layer about a repeated key or a `[*]` matcher. + const advisories: string[] = []; const stylePath = ruleFilePath(cwd, engine, ruleId); try { const style = await readYaml(stylePath); @@ -216,7 +221,9 @@ export async function verifyOneRule( // catches are not local — Vale reads one assembled config per run, so an // unknown `extends` or a foreign field takes down every other Vale // rule's findings rather than just this one's. - errors.push(...validateValeRule(ruleId, style).errors); + const schema = validateValeRule(ruleId, style); + errors.push(...schema.errors); + advisories.push(...schema.advisories); if (typeof style === "object" && style !== null) { const record = style as Record; @@ -246,10 +253,8 @@ export async function verifyOneRule( // Vale would accept and read as something other than what its author // wrote: a rule assignment above the first matcher, a matcher with no // breadcrumb, a key naming another rule. Each rejection is attributed to - // its `vale-config-*` constraint; what is true but not invalid rides on - // `notice`. + // its `vale-config-*` constraint. const violations: RuleViolation[] = []; - let notice: string | undefined; const configPath = ruleConfigPath(cwd, engine, ruleId); if (configPath !== undefined) { let config: string | undefined; @@ -269,9 +274,7 @@ export async function verifyOneRule( rejection.message ); } - if (verdict.advisories.length > 0) { - notice = verdict.advisories.join("\n"); - } + advisories.push(...verdict.advisories); } } @@ -284,7 +287,7 @@ export async function verifyOneRule( ok: errors.length === 0, errors, violations, - ...(notice === undefined ? {} : { notice }), + ...(advisories.length === 0 ? {} : { notice: advisories.join("\n") }), }; } diff --git a/packages/cli/src/schemas/vale-rule.ts b/packages/cli/src/schemas/vale-rule.ts index 5bd92d41..a9e66f01 100644 --- a/packages/cli/src/schemas/vale-rule.ts +++ b/packages/cli/src/schemas/vale-rule.ts @@ -812,8 +812,57 @@ export const valeRuleSchema = valeHeaderSchema .transform(canonicalKeys) .pipe(valeBodySchema); -/** What the schema layer concluded. Shared with the ast-grep path. */ -export type ValeSchemaResult = SchemaLayerResult; +/** + * What is true about a rule that does not make it invalid. + * + * The counterpart of `adviseValeRuleConfig` in `vale-config.ts`, and held to + * the same line: each entry has a legitimate reading, so none is a rejection. + * Said rather than refused, on `notice`. + * + * Read off the canonical keys, because `Raw:` is decoded case-insensitively + * like every non-literal field; and off the mapping whether or not the schema + * passed it, since an advisory about one field is as true beside a rejection + * of another as it is alone. + */ +function adviseValeRule(ruleId: string, data: unknown): string[] { + if (typeof data !== "object" || data === null || Array.isArray(data)) { + return []; + } + const rule = canonicalKeys(data as Record); + const advisories: string[] = []; + + // Vale joins an `existence` rule's `raw` list with no separator + // (`strings.Join(rule.Raw, "")`), so two entries are one regex, not two + // alternatives, and a line matching only the second entry is silent. That + // is a real authoring trap (taskless/cli#360: the second entry's fail + // fixture never fired, with no error anywhere) and also a real way to write + // a long pattern across lines, which is why it is a notice. Pinned on the + // binary in `vale-vendor-contract.test.ts`, "existence `raw` entries join + // into one pattern". + if ( + rule.extends === "existence" && + Array.isArray(rule.raw) && + rule.raw.length > 1 + ) { + advisories.push( + `${ruleId}: raw has ${String(rule.raw.length)} entries; Vale joins them into one pattern with no separator, so the second never matches on its own. Write one entry with (a|b) unless the join is intended.` + ); + } + return advisories; +} + +/** + * What the schema layer concluded, plus what it noticed. + * + * `valid` and `errors` are the shape shared with the ast-grep path; + * `advisories` is the Vale layer's own, since its checks are hand-authored + * against a measured binary and some of what they know is worth saying without + * being worth failing. + */ +export interface ValeSchemaResult extends SchemaLayerResult { + /** True things about the rule that do not make it invalid. */ + advisories: string[]; +} /** * Validate a parsed Vale style file structurally, before Vale is invoked. @@ -826,8 +875,11 @@ export function validateValeRule( ruleId: string, data: unknown ): ValeSchemaResult { - return schemaLayer( - valeRuleSchema.safeParse(data), - (issue) => `${ruleId}.yml: ${issue.message}` - ); + return { + ...schemaLayer( + valeRuleSchema.safeParse(data), + (issue) => `${ruleId}.yml: ${issue.message}` + ), + advisories: adviseValeRule(ruleId, data), + }; } diff --git a/packages/cli/test/vale-schema-contract.test.ts b/packages/cli/test/vale-schema-contract.test.ts index 5e0e2908..22780317 100644 --- a/packages/cli/test/vale-schema-contract.test.ts +++ b/packages/cli/test/vale-schema-contract.test.ts @@ -349,6 +349,88 @@ describe("the permissive checks stay permissive", () => { }); }); +/** + * What the schema says without failing the rule. + * + * The corpus has three verdicts and none of them is "accepted, with a note", + * so an advisory cannot be a row there; it is pinned here instead, beside the + * other tests that ask the schema rather than the binary. The behaviour the + * advisory describes is measured in `vale-vendor-contract.test.ts` + * ("existence `raw` entries join into one pattern"); this asks only that the + * schema says so, and only when it applies. + */ +describe("advisories are said, not refused", () => { + const RAW_TWO = + 'extends: existence\nmessage: "x"\nlevel: warning\nraw:\n - "\\\\bstops being\\\\b"\n - "\\\\band becomes\\\\b"\n'; + + it("warns on a raw list with more than one entry, and still accepts the rule", () => { + const { valid, errors, advisories } = validateValeRule( + "no-twist", + parseYaml(RAW_TWO) + ); + expect(valid).toBe(true); + expect(errors).toEqual([]); + expect(advisories).toEqual([ + "no-twist: raw has 2 entries; Vale joins them into one pattern with no separator, so the second never matches on its own. Write one entry with (a|b) unless the join is intended.", + ]); + }); + + it("counts the entries it saw", () => { + const { advisories } = validateValeRule( + "demo", + parseYaml('extends: existence\nmessage: "x"\nraw:\n - a\n - b\n - c\n') + ); + expect(advisories[0]).toContain("raw has 3 entries"); + }); + + it("is silent on a one-entry raw list", () => { + const { advisories } = validateValeRule( + "demo", + parseYaml( + 'extends: existence\nmessage: "x"\nraw:\n - "(\\\\bstops being\\\\b|\\\\band becomes\\\\b)"\n' + ) + ); + expect(advisories).toEqual([]); + }); + + it("is silent on a tokens list, which Vale does alternate", () => { + const { advisories } = validateValeRule( + "demo", + parseYaml('extends: existence\nmessage: "x"\ntokens:\n - a\n - b\n') + ); + expect(advisories).toEqual([]); + }); + + it("reads the key case-insensitively, as Vale decodes it", () => { + const { advisories } = validateValeRule( + "demo", + parseYaml('extends: existence\nmessage: "x"\nRaw:\n - a\n - b\n') + ); + expect(advisories).toHaveLength(1); + }); + + it("still speaks beside a rejection of another field", () => { + // An advisory about `raw` is no less true because `scope` is wrong, and an + // author fixing the one should hear about the other in the same pass. + const { valid, advisories } = validateValeRule( + "demo", + parseYaml( + 'extends: existence\nmessage: "x"\nscope: fenced\nraw:\n - a\n - b\n' + ) + ); + expect(valid).toBe(false); + expect(advisories).toHaveLength(1); + }); + + it("carries an empty list, never an absent one, when there is nothing to say", () => { + const { advisories } = validateValeRule( + "demo", + parseYaml('extends: existence\nmessage: "x"\ntokens: [a]\n') + ); + expect(advisories).toEqual([]); + }); +}); + // --- Helpers ----------------------------------------------------------------- /** diff --git a/packages/cli/test/verify-test-commands.test.ts b/packages/cli/test/verify-test-commands.test.ts index c86888df..6b078a3d 100644 --- a/packages/cli/test/verify-test-commands.test.ts +++ b/packages/cli/test/verify-test-commands.test.ts @@ -218,6 +218,66 @@ describe("verify checks components without requiring tests", () => { expect(rule?.notice).toMatch(/assigns no-simply\.no-simply again/); }); + // The style schema's own advisory, through the real CLI. + // `vale-schema-contract.test.ts` holds the advisory to its rule; this says + // it reaches `notice` on `verify` output and `--json`, joined with whatever + // the config layer had to say, and that a one-entry list draws nothing. + it("accepts a Vale rule whose raw has two entries, and says so on notice", async () => { + await valeRule("no-twist", { + config: "[*.md]\ntskl) rule = no-twist\nno-twist.no-twist = YES\n", + style: + 'extends: existence\nmessage: "Avoid the twist"\nlevel: warning\nraw:\n' + + ' - "\\\\bstops being\\\\b"\n - "\\\\band becomes\\\\b"\n', + }); + const json = await runCli(["verify", "-d", cwd, "--json"]); + expect(json.exitCode).toBe(0); + const rule = (JSON.parse(json.stdout) as Report).rules[0]; + expect(rule?.ok).toBe(true); + expect(rule?.errors).toEqual([]); + expect(rule?.notice).toBe( + "no-twist: raw has 2 entries; Vale joins them into one pattern with no separator, so the second never matches on its own. Write one entry with (a|b) unless the join is intended." + ); + + const text = await runCli(["verify", "-d", cwd]); + expect(text.exitCode).toBe(0); + expect(text.stdout).toContain("notice: no-twist: raw has 2 entries"); + }); + + it("says nothing about a one-entry raw list", async () => { + await valeRule("no-twist", { + config: "[*.md]\ntskl) rule = no-twist\nno-twist.no-twist = YES\n", + style: + 'extends: existence\nmessage: "Avoid the twist"\nlevel: warning\nraw:\n' + + ' - "(\\\\bstops being\\\\b|\\\\band becomes\\\\b)"\n', + }); + const json = await runCli(["verify", "-d", cwd, "--json"]); + expect(json.exitCode).toBe(0); + const rule = (JSON.parse(json.stdout) as Report).rules[0]; + expect(rule?.ok).toBe(true); + expect(rule?.notice).toBeUndefined(); + + const text = await runCli(["verify", "-d", cwd]); + expect(text.stdout).not.toContain("notice:"); + }); + + it("joins a raw advisory and a config advisory on one notice", async () => { + await valeRule("no-twist", { + config: + "[docs/**]\ntskl) rule = no-twist\nno-twist.no-twist = YES\n\n" + + "[*.md]\ntskl) rule = no-twist\nno-twist.no-twist = YES\nno-twist.no-twist = NO\n", + style: + 'extends: existence\nmessage: "Avoid the twist"\nlevel: warning\nraw:\n' + + " - a\n - b\n", + }); + const result = await runCli(["verify", "-d", cwd, "--json"]); + expect(result.exitCode).toBe(0); + const rule = (JSON.parse(result.stdout) as Report).rules[0]; + expect(rule?.notice?.split("\n")).toEqual([ + expect.stringContaining("raw has 2 entries"), + expect.stringContaining("assigns no-twist.no-twist again"), + ]); + }); + it("rejects a Vale config whose only YES a later NO overrides", async () => { await valeRule("no-simply", { config: `${SCOPED}no-simply.no-simply = NO\n`, From b31854d9630f89dbcbcf39d51357bb1d5b01851d Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 21 Sep 2026 21:25:49 -0700 Subject: [PATCH 3/3] fix(verify): prefix every line of a multi-line notice --- packages/cli/src/commands/verify.ts | 8 +++++++- packages/cli/test/verify-test-commands.test.ts | 12 ++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/commands/verify.ts b/packages/cli/src/commands/verify.ts index bc072493..acced10f 100644 --- a/packages/cli/src/commands/verify.ts +++ b/packages/cli/src/commands/verify.ts @@ -159,8 +159,14 @@ async function runOverPath(options: { // Printed even when the rule passed. A misplaced `.vale.ini` assignment // makes Vale exit zero having enabled nothing, so the clean line above // is exactly the moment the author needs to hear this. + // + // A notice is one advisory per line, joined with `\n` (a style-layer + // advisory beside a config-layer one, say). Every line gets the prefix, + // so the second reads as a notice rather than as an unlabelled stray. if ("notice" in result && result.notice !== undefined) { - console.log(` notice: ${result.notice}`); + for (const line of result.notice.split("\n")) { + console.log(` notice: ${line}`); + } } } // A rule that did not run is not among the rules tested. Counting it there diff --git a/packages/cli/test/verify-test-commands.test.ts b/packages/cli/test/verify-test-commands.test.ts index 6b078a3d..048cd4a1 100644 --- a/packages/cli/test/verify-test-commands.test.ts +++ b/packages/cli/test/verify-test-commands.test.ts @@ -276,6 +276,18 @@ describe("verify checks components without requiring tests", () => { expect.stringContaining("raw has 2 entries"), expect.stringContaining("assigns no-twist.no-twist again"), ]); + + // Text mode prefixes every line, so the second advisory is labelled too + // rather than trailing the first as an unindented stray. + const text = await runCli(["verify", "-d", cwd]); + const noticeLines = text.stdout + .split("\n") + .filter((line) => line.startsWith(" notice: ")); + expect(noticeLines).toEqual([ + expect.stringContaining("notice: no-twist: raw has 2 entries"), + expect.stringContaining("notice: no-twist/.vale.ini"), + ]); + expect(noticeLines[1]).toContain("assigns no-twist.no-twist again"); }); it("rejects a Vale config whose only YES a later NO overrides", async () => {