From 0f7740ae1a95c3e6d12497abb51cd2f352ed0c2f Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Sun, 23 Aug 2026 21:24:47 -0700 Subject: [PATCH 1/4] feat(cli): state each local engine's reach in the routing recipe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `route` chose an engine from the shape of the evidence alone and said nothing about language reach, so a rule over a GitHub Actions workflow was escalated to `create-runtime-rule` — which needs a login — because nothing stated that ast-grep parses YAML. It does; `Yaml` is one of the 26 languages the pinned `@ast-grep/cli@0.41.0` lists in `sg run -h`. Nothing in the repo could have answered that. The vendored ast-grep schema types `$defs.Language` as a bare string with no enum, `verify` never validates a rule's `language`, and `detect --json` reports the repository's own languages in a different vocabulary. Vale self-reports nothing at all, so its reach was measured by probing the shipped 3.17.1 binary tier by tier. A new `src/rules/capabilities.ts` carries both engines' reach as pure data pinned to the version it came from, and `buildVariables` renders it into `route.txt` (topic v2) and `create-vale-rule.txt` (topic v3) as `%(...)s` substitutions rather than prose typed into a `.txt`. Two vendor-contract blocks pin the constants to the binaries, so an engine bump that changes what a binary parses fails a test instead of leaving a confident wrong sentence in front of an agent. Also names Vale's converter-dependent formats and what they cost: Vale supports reStructuredText, AsciiDoc, XML, DITA and MDX by shelling out to a converter this CLI does not ship, and one such file exits 2 with an `E100` and abandons the whole run. `create-vale-rule` had been offering `[*.{md,mdx}]` as its example of widening a matcher. Refs #151 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Cyga14bww8rmazH2XrF8ms --- .changeset/engine-reach-in-route.md | 29 +++ .../changes/engine-reach-in-route/proposal.md | 94 +++++++ .../specs/cli-rule-routing/spec.md | 65 +++++ .../changes/engine-reach-in-route/tasks.md | 30 +++ packages/cli/src/agent/create-vale-rule.txt | 38 ++- packages/cli/src/agent/route.txt | 52 +++- packages/cli/src/prompts/recipes.ts | 20 ++ packages/cli/src/rules/capabilities.ts | 234 ++++++++++++++++++ .../cli/test/ast-grep-vendor-contract.test.ts | 94 +++++++ packages/cli/test/prompts.test.ts | 17 +- .../cli/test/recipe-cross-references.test.ts | 74 ++++++ .../cli/test/vale-vendor-contract.test.ts | 192 ++++++++++++++ 12 files changed, 934 insertions(+), 5 deletions(-) create mode 100644 .changeset/engine-reach-in-route.md create mode 100644 openspec/changes/engine-reach-in-route/proposal.md create mode 100644 openspec/changes/engine-reach-in-route/specs/cli-rule-routing/spec.md create mode 100644 openspec/changes/engine-reach-in-route/tasks.md create mode 100644 packages/cli/src/rules/capabilities.ts diff --git a/.changeset/engine-reach-in-route.md b/.changeset/engine-reach-in-route.md new file mode 100644 index 00000000..43b069c7 --- /dev/null +++ b/.changeset/engine-reach-in-route.md @@ -0,0 +1,29 @@ +--- +"@taskless/cli": patch +--- + +Tell the routing recipe what the local engines can actually read. + +`route` chose between `sg`, `vale`, and the runtime tier on the shape of the +evidence alone, and had nothing to say about language reach — so a rule over a +GitHub Actions workflow was escalated to `create-runtime-rule`, which needs a +login, because nothing stated that ast-grep parses YAML. It does. Nothing in +the repository could have said so either: the vendored ast-grep schema types +`language` as a bare string with no enum, `verify` never checks the field, and +`detect --json` reports the repo's own languages in a different vocabulary. +Vale self-reports nothing at all. + +`route` now states both engines' reach, and `create-vale-rule` repeats Vale's +where a matcher is written. Both read the lists from constants pinned to the +engine versions this CLI ships, rather than from prose typed into the recipe — +an engine bump that changes what a binary parses now fails a vendor-contract +test instead of leaving a confident, wrong sentence in front of an agent. +Vale's reach was measured by probing the shipped binary, tier by tier, since it +publishes no capability listing. + +The Vale half carries a hazard worth naming on its own. Vale supports +reStructuredText, AsciiDoc, XML, DITA, and MDX by shelling out to an external +converter, and this CLI ships none of them — so one such file caught by a +rule's glob exits 2 with an `E100` and abandons the whole run, silencing every +other Vale rule over every other file. `create-vale-rule` had been offering +`[*.{md,mdx}]` as its example of widening a matcher. diff --git a/openspec/changes/engine-reach-in-route/proposal.md b/openspec/changes/engine-reach-in-route/proposal.md new file mode 100644 index 00000000..b1a6df19 --- /dev/null +++ b/openspec/changes/engine-reach-in-route/proposal.md @@ -0,0 +1,94 @@ +## Why + +`route.txt` tells an agent to pick an engine and gives it no way to know +what either local engine can read. The engine table is entirely about +evidence _shape_ — one file's tree, a document's words, more than one +file — and says nothing about language reach. The closest it comes is an +Important Note asking the agent to "choose an engine whose availability +you can assert," which asks for an assertion the recipe never equips it +to make. + +The reported case is concrete: two GitHub Actions workflow rules were +routed to `create-runtime-rule`, which requires a login, because nothing +said whether ast-grep parses YAML. It does — `Yaml` is one of the 26 +languages the pinned `@ast-grep/cli@0.41.0` lists in `sg run -h`. + +Nothing in the repository could have answered the question. +`src/generated/ast-grep-rule-schema.json` types `$defs.Language` as a +bare string with no enum; `verify` never validates a rule's `language`, +so any spelling passes locally and fails inside the binary; and +`detect --json` reports the _repository's_ languages in a different +vocabulary (`C++` where ast-grep says `Cpp`). Vale self-reports nothing +at all. The binaries are the only authority. + +## What Changes + +- **A new `packages/cli/src/rules/capabilities.ts`** carries each local + engine's reach as pure data, pinned to the engine version it was taken + from. Pure is load-bearing: `src/prompts/recipes.ts` is a Worker-safe + surface and `assert-prompts-graph` fails the build if the prompts + chunk reaches a host capability, so the constants cannot be read from + a binary or a file at render time. +- **The recipes state reach through `%(…)s` variables**, resolved from + those constants rather than transcribed into the `.txt`. Transcribed + lists would drift on the next engine bump with nothing to catch them, + and a stale claim about what an engine can read is worse than the + silence it replaces — an agent acts on it. +- **`route.txt` (topic v1 → v2)** gains a reach block in step 4: what + `sg` parses, that an Actions workflow is `Yaml` and `Yaml` is on the + list, Vale's markup / comments-only / plaintext-fallback tiers, and + the converter-dependent formats Vale cannot read at all. It also + states that a language on neither list does **not** route to runtime by + default — `create-legacy-rule` is checked first, because the repo's own + linter may already speak it. +- **`create-vale-rule.txt` (topic v2 → v3)** repeats the reach where a + matcher is actually authored, and stops offering `[*.{md,mdx}]` as the + worked example of widening a glob. That example is a trap: one `.mdx` + file fails the entire Vale pass. +- **Two vendor-contract test blocks** pin the constants to the binaries. + ast-grep is asserted by set-equality against `sg run -h`; Vale, which + self-reports nothing, is probe-measured, with each tier separated by a + construct that distinguishes it from the fallback. + +**Delivery is a single PR.** Constants, recipe text, spec delta, and +tests are one reviewable diff, and no slice of it is independently +useful — the constants exist only to be rendered, and the recipe text is +wrong without them. + +## Capabilities + +### Modified Capabilities + +- `cli-rule-routing`: the `route` recipe states each local engine's + language reach, derived from the pinned engine versions rather than + transcribed into the recipe text. + +## Impact + +- **Added**: `packages/cli/src/rules/capabilities.ts` — pure data, no + imports at all. +- **Modified**: `packages/cli/src/prompts/recipes.ts` — six new entries in + `buildVariables`, all build-time constants. Compatible with the + `TASKLESS_CLI` rule that a recipe variable is an argument and never an + ambient read: nothing here touches `process`. +- **Modified**: `packages/cli/src/agent/route.txt` (topic v1 → v2) and + `packages/cli/src/agent/create-vale-rule.txt` (topic v2 → v3). +- **Modified**: `packages/cli/test/ast-grep-vendor-contract.test.ts` and + `packages/cli/test/vale-vendor-contract.test.ts` each gain a separate + top-level `describe` for the capability constants; + `packages/cli/test/recipe-cross-references.test.ts` gains a block + asserting the constants reach rendered text; + `packages/cli/test/prompts.test.ts` has its `buildVariables` key list + and its prompts-import allowlist extended. +- **Out of scope**: bumping Vale past 3.18.0, where MDX parses natively + and `.mdx` leaves the converter-dependent list. The constants are + shaped so that bump is a one-file edit. +- **Out of scope**: a runtime guard that catches an `E100` before it + takes down a `check`. This change makes the hazard visible at authoring + time; it does not defend against one already committed. +- **Out of scope**: extending `detect --json` with an `engines` block. + `openspec/specs/cli-detect/spec.md` frames `detect` as an offline scan + of the _repository_; engine reach is knowable at build time and does + not need a third subprocess turn in a recipe already spending two. + +**Tracking:** taskless/cli#151 diff --git a/openspec/changes/engine-reach-in-route/specs/cli-rule-routing/spec.md b/openspec/changes/engine-reach-in-route/specs/cli-rule-routing/spec.md new file mode 100644 index 00000000..fbb15a5d --- /dev/null +++ b/openspec/changes/engine-reach-in-route/specs/cli-rule-routing/spec.md @@ -0,0 +1,65 @@ +## ADDED Requirements + +### Requirement: Route states each local engine's reach from the pinned engine versions + +The `route` recipe SHALL state what each local engine can actually read, so +that an agent choosing between `create-sg-rule`, `create-vale-rule`, and the +runtime destinations is not left to guess whether an engine parses the language +in front of it. + +The recipe SHALL state, for the ast-grep engine, the set of languages the +pinned ast-grep release parses, spelled as a rule's `language:` field must +spell them. It SHALL state, for the Vale engine, that Vale treats a file +according to its extension in tiers — markup, comment text only, and a +plaintext fallback that lints an unparsed file as whole-file prose — and that a +set of formats fails rather than lints because Vale delegates their parse to an +external converter this CLI does not ship. The recipe SHALL state that such a +failure aborts the entire Vale pass rather than skipping the offending file. + +These statements SHALL be **derived from the pinned engine versions rather +than transcribed into the recipe text**. The recipe SHALL carry substitution +markers resolved at render time from constants that name the engine version +they were taken from, and those constants SHALL be pinned to the engines' +observable behaviour by tests that invoke the engine binaries. A transcribed +list would go stale on the next engine bump with nothing to detect it, and a +stale claim about engine reach is more harmful than no claim, because an agent +acts on it. + +The recipe SHALL NOT treat a language absent from both engines' reach as +automatically requiring the runtime tier. It SHALL direct the agent to consider +`create-legacy-rule` first, since a linter the repository already runs may +cover the language, and that destination requires no login. + +The recipe SHALL distinguish an engine's reach from an engine's availability. +Reach is a property of the pinned engine version; availability is a property of +the host on which the CLI is running, and a language within reach is still +unusable where the engine's platform binary did not resolve. + +#### Scenario: Route names ast-grep's languages + +- **WHEN** the rendered `route` recipe is read +- **THEN** it SHALL name the languages the pinned ast-grep release parses, including `Yaml` +- **AND** an agent SHALL be able to conclude from it that a rule over a GitHub Actions workflow is expressible as an `sg` rule + +#### Scenario: Route names Vale's tiers and its unreadable formats + +- **WHEN** the rendered `route` recipe is read +- **THEN** it SHALL name the extensions Vale parses as markup, the extensions where Vale lints comment text only, and the plaintext fallback that applies to everything else +- **AND** it SHALL name the formats whose parse Vale delegates to an absent external converter +- **AND** it SHALL state that one such file fails the whole Vale pass rather than only itself + +#### Scenario: The reach statements carry no unresolved marker + +- **WHEN** the `route` recipe is rendered +- **THEN** no `%(…)s` substitution marker SHALL remain in the text an agent receives + +#### Scenario: A bumped engine cannot leave the recipe stale + +- **WHEN** an engine binary is upgraded to a version whose reach differs from the constants the recipe renders +- **THEN** a vendor-contract test that invokes the engine SHALL fail +- **AND** the recipe SHALL NOT be able to state the superseded reach without that failure + +#### Scenario: An unreachable language is not routed to runtime by default + +- **WHEN** the rule's language appears in neither engine's reach +- **THEN** `route` SHALL direct the agent to consider `create-legacy-rule` before escalating to a runtime destination diff --git a/openspec/changes/engine-reach-in-route/tasks.md b/openspec/changes/engine-reach-in-route/tasks.md new file mode 100644 index 00000000..76bd207b --- /dev/null +++ b/openspec/changes/engine-reach-in-route/tasks.md @@ -0,0 +1,30 @@ +Delivery shape: **single PR**. Constants, recipe text, spec delta, and tests are one diff — the constants exist only to be rendered, and the recipe text is wrong without them, so no slice lands independently. + +## 1. Constants + +- [x] 1.1 Add `packages/cli/src/rules/capabilities.ts` with the ast-grep language list verbatim from `sg run -h` at the pinned `0.41.0`, and the pinned version beside it +- [x] 1.2 Probe the shipped Vale 3.17.1 binary for its tiers rather than trusting any prior table, and record markup, comment-only, and converter-dependent extensions with the converter each failure names +- [x] 1.3 Keep the module free of imports entirely, so `assert-prompts-graph` still passes when `src/prompts/recipes.ts` reaches it +- [x] 1.4 Add the render helpers that turn each list into recipe prose, so a recipe never joins an array itself + +## 2. Recipes + +- [x] 2.1 Extend `buildVariables` with `AST_GREP_VERSION`, `AST_GREP_LANGUAGES`, `VALE_VERSION`, `VALE_MARKUP_FORMATS`, `VALE_COMMENT_FORMATS`, and `VALE_CONVERTER_FORMATS` +- [x] 2.2 Bump `route.txt` to topic v2 and add the reach block to step 4, after the engine table's sharpening bullets and before the worked examples +- [x] 2.3 State in `route.txt` that a language on neither list is not automatically runtime, and that `create-legacy-rule` is checked first +- [x] 2.4 Separate reach from availability in `route.txt`'s Important Notes — the existing "availability you can assert" bullet is about the binary resolving on this host, which the new lists do not answer +- [x] 2.5 Bump `create-vale-rule.txt` to topic v3, repeat the reach at the matcher step, and replace the `[*.{md,mdx}]` widening example + +## 3. Tests + +- [x] 3.1 Add an `ast-grep engine capabilities` block parsing `Supported languages are: [...]` and asserting set-equality, with `Yaml` named explicitly so the issue's answer is an assertion rather than an implication +- [x] 3.2 Add a `Vale engine capabilities` block that probes each tier with a discriminating fixture — a construct only a parser skips for markup, a bare non-comment line for comment-only, a non-zero exit with `E100` for converter-dependent +- [x] 3.3 Assert in `recipe-cross-references.test.ts` that no rendered recipe leaks a `%(…)s` marker and that rendered `route.txt` carries the lists +- [x] 3.4 Extend the `buildVariables` key list and the prompts import allowlist in `prompts.test.ts` + +## 4. Verification + +- [x] 4.1 `pnpm openspec validate engine-reach-in-route --strict` +- [x] 4.2 `pnpm --filter @taskless/cli build` — the prompts-graph plugin is the real check that `capabilities.ts` stayed pure +- [x] 4.3 `pnpm typecheck`, `pnpm lint`, and `pnpm --filter @taskless/cli test` +- [x] 4.4 Add the changeset diff --git a/packages/cli/src/agent/create-vale-rule.txt b/packages/cli/src/agent/create-vale-rule.txt index 4522e83c..76bcb10e 100644 --- a/packages/cli/src/agent/create-vale-rule.txt +++ b/packages/cli/src/agent/create-vale-rule.txt @@ -1,4 +1,4 @@ -# Topic: create-vale-rule (CLI v%(CLI_VERSION)s / topic v2) +# Topic: create-vale-rule (CLI v%(CLI_VERSION)s / topic v3) ## You are here This is `create-vale-rule`. It helps you write a Vale rule: a check over @@ -271,8 +271,10 @@ it. - `[*.md]` is a **matcher**: a glob over paths, deciding which files this rule sees. Match it to the files the rule is actually about, - such as `[*.{md,mdx}]` or `[docs/**/*.md]`. A rule can declare - several matchers if it needs to. + such as `[*.{md,markdown}]` or `[docs/**/*.md]`. A rule can declare + several matchers if it needs to. Before you widen a glob, check the + reach table below — what Vale does to a file it cannot parse is not + "nothing". - `tskl) rule = ` is a breadcrumb Taskless reads to attribute the matcher back to this rule after assembly interleaves every rule's matchers into one file. Vale parses the key and ignores it. Write it @@ -294,6 +296,36 @@ it. `[…]` line belongs to no matcher, and Vale ignores it after warning on stderr. + **What a matcher's glob is allowed to catch.** Vale (v%(VALE_VERSION)s) + treats a file one of four ways, decided by extension. The lists are + rendered from the pinned Vale version, not written out here, so they + track the shipped binary. + + - **markup** — the document is prose and the format's own non-prose + constructs are skipped. This is the tier every `scope:` value + assumes; `scope: heading` has nothing to find outside it: + %(VALE_MARKUP_FORMATS)s + - **comment text only** — the comments are linted and the code body + is invisible, which is exactly right for "comments must not say + 'obviously'": + %(VALE_COMMENT_FORMATS)s + - **plaintext fallback** — everything else, `.yml` `.toml` `.sh` + `.sql` and every extension not named above included. There is no + 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. + - **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: + %(VALE_CONVERTER_FORMATS)s + + **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 + unreported. `[*.{md,mdx}]` is not a slightly wider `[*.md]`; it is a + matcher that takes `check` down the first time the repo grows an + `.mdx` file. Never put one of those extensions in a glob. + 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 2af9e39a..8ad6ebae 100644 --- a/packages/cli/src/agent/route.txt +++ b/packages/cli/src/agent/route.txt @@ -1,4 +1,4 @@ -# Topic: route (CLI v%(CLI_VERSION)s / topic v1) +# Topic: route (CLI v%(CLI_VERSION)s / topic v2) ## Goal Turn "write me a rule that…" into one command to run. This is the front @@ -90,11 +90,57 @@ answered together. different axis from "which engine", and conflating them is what makes a prose rule look like it needs an account. + **What each local engine can actually read.** Both lists are rendered + from the pinned engine versions rather than written out here, so they + cannot go stale against the binaries: if one looks wrong, an engine + was bumped and a vendor-contract test is already red. + + - **ast-grep (v%(AST_GREP_VERSION)s) parses:** %(AST_GREP_LANGUAGES)s. + + Those spellings are ast-grep's own and go into a rule's `language:` + field verbatim. Nothing local validates that field, so a plausible + `C++` or `yaml` passes `verify` and fails inside the binary. + + **A GitHub Actions workflow is `Yaml`, and `Yaml` is on that list.** + A rule about `.github/workflows/*.yml` is an `sg` rule; sending it + to runtime spends a login on a check that builds here. + + - **Vale (v%(VALE_VERSION)s) reads three tiers, and hard-fails on a fourth.** + The tier is decided by the file's extension: + + - *markup* — the whole document is prose, and the format's own + non-prose constructs are skipped: + %(VALE_MARKUP_FORMATS)s + - *comments only* — the comment text is linted and the code body is + invisible: + %(VALE_COMMENT_FORMATS)s + - *plaintext fallback* — everything else, `.yml` `.toml` `.sh` + `.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. + - *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: + %(VALE_CONVERTER_FORMATS)s + + **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 + unreported. A matcher written as `[*.{md,mdx}]` is not a wider + `[*.md]` — it is a broken one. + + **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 + with no login. Escalate only once nothing local can see the evidence. + Worked examples: | Rule intent | Evidence needed | Destination | |----------------------------------------------------------|-------------------------------------------|--------------------| | No `eval(...)` anywhere | one file's call expressions | `create-sg-rule` | +| Actions workflows must pin actions to a SHA | one workflow file's tree (`Yaml`) | `create-sg-rule` | | `useEffect` deps must include what the body reads | one file's tree, correlated within it | `create-sg-rule` | | Don't write "simply" or "just" in docs | a document's words | `create-vale-rule` | | Comments must not say "obviously" | a document's words (comments are prose) | `create-vale-rule` | @@ -166,6 +212,10 @@ will work. form that correlates statically, and to runtime when the equivalent check would have to normalize a captured value against a declaration elsewhere. +- **Engine reach and engine availability are two different questions.** + Step 4's lists say what each engine can *parse*; they say nothing + about whether the binary resolved on this host. A language on the list + is still unusable where the platform build is missing. - **When it stays ambiguous, choose an engine whose availability you can assert, and say that is why.** There is no fixed fallback. `sg` and `vale` both ship as platform binaries, so either can be the missing one diff --git a/packages/cli/src/prompts/recipes.ts b/packages/cli/src/prompts/recipes.ts index a5579466..29506259 100644 --- a/packages/cli/src/prompts/recipes.ts +++ b/packages/cli/src/prompts/recipes.ts @@ -8,6 +8,14 @@ import { } from "../util/invocation"; import { inputSchema as ruleCreateInputSchema } from "../schemas/rules-create"; import { inputSchema as ruleImproveInputSchema } from "../schemas/rules-improve"; +import { + AST_GREP_VERSION, + VALE_VERSION, + astGrepLanguageList, + valeCommentList, + valeConverterList, + valeMarkupList, +} from "../rules/capabilities"; // Agent recipe files embedded at build time via Vite import.meta.glob. // Filename convention: .txt for the canonical recipe and @@ -145,6 +153,18 @@ export function buildVariables( ): Record { const variables: Record = { CLI_VERSION: __VERSION__, + // Engine reach, from the pinned engine versions rather than transcribed + // into a recipe. A `.txt` carrying these lists by hand would go stale on + // the next binary bump with nothing to catch it, and stale prose about + // what an engine can read is worse than the silence it replaced — an agent + // acts on it. `src/rules/capabilities.ts` is the single place a bump edits, + // and the two vendor-contract tests fail until it agrees with the binary. + AST_GREP_VERSION, + AST_GREP_LANGUAGES: astGrepLanguageList(), + VALE_VERSION, + VALE_MARKUP_FORMATS: valeMarkupList(), + VALE_COMMENT_FORMATS: valeCommentList(), + VALE_CONVERTER_FORMATS: valeConverterList(), PACKAGE_MANAGER_DLX: options.packageManagerDlx ?? PACKAGE_MANAGER_DLX_MARKER, // Three steps, in descending order of how much the resolver actually diff --git a/packages/cli/src/rules/capabilities.ts b/packages/cli/src/rules/capabilities.ts new file mode 100644 index 00000000..f050810a --- /dev/null +++ b/packages/cli/src/rules/capabilities.ts @@ -0,0 +1,234 @@ +/** + * What the two local engines can actually read. + * + * PURE DATA, DELIBERATELY. This module is imported by `src/prompts/recipes.ts`, + * which is a Worker-safe library surface — no citty, no telemetry, no + * filesystem, no network — and `assert-prompts-graph` in `vite.config.ts` + * fails the build if the prompts chunk's graph reaches a host capability. So + * nothing here may read `package.json`, spawn a binary, or import a node + * builtin. The values are transcribed once, here, and pinned by tests that do + * spawn the binaries. + * + * THE BINARIES ARE THE ONLY AUTHORITY, and neither of them can be asked at + * render time: + * + * - `src/generated/ast-grep-rule-schema.json` types `$defs.Language` as a bare + * string with no enum, and `verify` never validates a rule's `language`, so + * any spelling passes our own checks and fails only inside ast-grep. + * - `detect --json` reports the *repository's* languages in a different + * vocabulary — `C++` where ast-grep says `Cpp` — and says nothing about what + * an engine can parse. + * - Vale self-reports nothing at all. Its reach was measured by probing the + * shipped binary, which is why the Vale constants below carry a probe-shaped + * contract test rather than a parsed capability listing. + * + * BUMPING AN ENGINE IS A ONE-FILE EDIT. Change the version constant and the + * list beside it; `test/ast-grep-vendor-contract.test.ts` and + * `test/vale-vendor-contract.test.ts` fail until the two agree, which is the + * whole point of transcribing rather than describing. See taskless/cli#151 for + * the routing miss this exists to prevent: two GitHub Actions workflow rules + * were escalated to `runtime` because nothing said ast-grep parses YAML. + */ + +/** + * The ast-grep release pinned in `packages/cli/package.json`, both for + * `@ast-grep/cli` and for every `@ast-grep/cli-` optional dependency. + * + * Pinned against the binary by `test/ast-grep-vendor-contract.test.ts` + * ("engine capabilities" → "reports the pinned version"). + */ +export const AST_GREP_VERSION = "0.41.0"; + +/** + * Every language ast-grep can parse, verbatim from + * `sg run -h` → `Supported languages are: [...]` at + * {@link AST_GREP_VERSION}. + * + * SPELLINGS ARE ast-grep's, NOT ours and not `detect`'s. `Cpp`, `CSharp`, + * `JavaScript`, `Tsx` — a rule's `language:` field is handed to ast-grep + * unchanged, and `verify` does not check it, so a plausible-looking `C++` or + * `yaml` reaches the binary and fails there. + * + * Pinned by set-equality against the binary in + * `test/ast-grep-vendor-contract.test.ts`, so a version bump that adds or drops + * a language fails there rather than silently narrowing what the router + * believes is buildable locally. + */ +export const AST_GREP_LANGUAGES = [ + "Bash", + "C", + "Cpp", + "CSharp", + "Css", + "Elixir", + "Go", + "Haskell", + "Hcl", + "Html", + "Java", + "JavaScript", + "Json", + "Kotlin", + "Lua", + "Nix", + "Php", + "Python", + "Ruby", + "Rust", + "Scala", + "Solidity", + "Swift", + "Tsx", + "TypeScript", + "Yaml", +] as const; + +/** + * The Vale release carried by the `@taskless/vale-` packages pinned + * in `packages/cli/package.json`. Their npm versions append a build stamp + * (`3.17.1-20260810052605`); this is the version Vale itself reports. + * + * Pinned against the binary by `test/vale-vendor-contract.test.ts` + * ("engine capabilities" → "reports the pinned version"). + */ +export const VALE_VERSION = "3.17.1"; + +/** + * 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; + +/** + * Extensions where Vale lints **comment text only** and ignores the code body. + * + * 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. + * + * 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. + */ +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; + +/** A format Vale supports upstream but cannot read without an external tool. */ +export interface ValeConverterFormat { + /** Extensions Vale routes through this converter. */ + extensions: readonly string[]; + /** The executable or artifact Vale looks for, named in its own E100 text. */ + converter: string; +} + +/** + * Formats that fail rather than lint, because Vale shells out to a converter + * this CLI does not ship. + * + * SAY THIS ACCURATELY: Vale supports these formats. What is missing is the + * external program it delegates the parse to. The failure is environmental, and + * describing it as "Vale does not support reStructuredText" sends an author + * looking for the wrong fix. + * + * The blast radius is what makes this worth surfacing at routing time rather + * than at authoring time: Vale exits 2 with an `E100` runtime error and + * abandons the run, and `--no-exit` does not suppress it. One `.mdx` file + * 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: "an XSLT transform" }, + { extensions: [".dita"], converter: "dita" }, + { extensions: [".mdx"], converter: "mdx2vast" }, +]; + +/** Every converter-dependent extension, flattened. */ +export const VALE_CONVERTER_DEPENDENT_EXTENSIONS: readonly string[] = + VALE_CONVERTER_DEPENDENT.flatMap((format) => format.extensions); + +/** `Bash, C, Cpp, …` — the ast-grep language list as recipe prose. */ +export function astGrepLanguageList(): string { + return AST_GREP_LANGUAGES.join(", "); +} + +/** `.htm, .html, …` — Vale's markup extensions as recipe prose. */ +export function valeMarkupList(): string { + return VALE_MARKUP_EXTENSIONS.join(", "); +} + +/** `.c, .c++, …` — Vale's comment-only extensions as recipe prose. */ +export function valeCommentList(): string { + return VALE_COMMENT_EXTENSIONS.join(", "); +} + +/** + * `.rst (needs rst2html), …` — Vale's converter-dependent formats as recipe + * prose, each naming the tool whose absence is the actual failure. + */ +export function valeConverterList(): string { + return VALE_CONVERTER_DEPENDENT.map( + ({ extensions, converter }) => + `${extensions.join("/")} (needs ${converter})` + ).join(", "); +} diff --git a/packages/cli/test/ast-grep-vendor-contract.test.ts b/packages/cli/test/ast-grep-vendor-contract.test.ts index f896e489..2b69d678 100644 --- a/packages/cli/test/ast-grep-vendor-contract.test.ts +++ b/packages/cli/test/ast-grep-vendor-contract.test.ts @@ -6,6 +6,11 @@ import { dirname, join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { assembleSgConfig } from "../src/rules/assemble"; +import { + AST_GREP_LANGUAGES, + AST_GREP_VERSION, + astGrepLanguageList, +} from "../src/rules/capabilities"; import { ruleDirectory, ruleTestsDirectory } from "../src/rules/engines"; import { buildPath, findSgBinary } from "../src/rules/scan"; @@ -547,3 +552,92 @@ withSg("ast-grep vendor contract", () => { }); }); }); + +/** + * The language reach `src/rules/capabilities.ts` publishes, pinned to the + * binary that actually parses. + * + * Kept in its own top-level block rather than folded into the contract suite + * above: those cases are about how ast-grep *behaves* when we drive it, and + * these are about a constant we transcribed from it. A bump that adds or drops + * a language fails here, which is the entire reason the list is a constant + * instead of prose inside `route.txt` — transcribed prose in a `.txt` has + * nothing to go red, and a stale claim about what an engine can read is worse + * than the silence it replaced, because an agent acts on it. + * + * See taskless/cli#151. + */ +/** + * The bracketed list from `sg run -h`, which is the only place ast-grep + * enumerates this. Not derived from anything we generate: the vendored + * `src/generated/ast-grep-rule-schema.json` types `$defs.Language` as a bare + * string with no enum, and `verify` never validates a rule's `language`, so our + * own artifacts cannot answer the question. + */ +function reportedLanguages(): string[] { + const help = spawnSync(binary as string, ["run", "-h"], { + encoding: "utf8", + }); + expect(help.status).toBe(0); + const listed = /Supported languages are:\s*\[([^\]]*)\]/.exec( + `${help.stdout}${help.stderr}` + ); + expect( + listed, + "`sg run -h` no longer prints a bracketed language list" + ).not.toBeNull(); + return (listed?.[1] ?? "") + .split(",") + .map((name) => name.trim()) + .filter((name) => name !== ""); +} + +withSg("ast-grep engine capabilities", () => { + it("reports the pinned version", () => { + // AST_GREP_VERSION is what route.txt renders next to the language list, so + // an agent reading "ast-grep (v0.41.0) parses: …" is being told which + // binary the claim came from. A bump that updates package.json and forgets + // the constant makes that attribution a lie. + const result = spawnSync(binary as string, ["--version"], { + encoding: "utf8", + }); + expect(result.stdout.trim()).toBe(`ast-grep ${AST_GREP_VERSION}`); + }); + + it("parses exactly the languages AST_GREP_LANGUAGES claims", () => { + // Set equality in BOTH directions on purpose. A missing language narrows + // what `route` believes is buildable locally and escalates rules to the + // runtime tier for no reason; an extra one sends an agent to write a rule + // whose `language:` the binary then rejects. + expect([...reportedLanguages()].toSorted()).toEqual( + [...AST_GREP_LANGUAGES].toSorted() + ); + }); + + it("lists Yaml, so a GitHub Actions workflow is an sg rule", () => { + // The named case from taskless/cli#151: two Actions workflow rules were + // routed to `runtime` — which needs a login — because nothing in the + // recipe said ast-grep parses YAML. Asserted by name rather than left to + // fall out of the set-equality above, so the answer to that issue cannot + // regress into an implication nobody rereads. + expect(reportedLanguages()).toContain("Yaml"); + expect(AST_GREP_LANGUAGES).toContain("Yaml"); + }); + + it("spells languages the way a rule's `language:` field must", () => { + // ast-grep's vocabulary is not `detect --json`'s: `detect` reports the + // repository's languages as `C++`, and a rule that copies that spelling + // through fails inside the binary rather than in `verify`. + expect(AST_GREP_LANGUAGES).toContain("Cpp"); + expect(AST_GREP_LANGUAGES).not.toContain("C++"); + expect(AST_GREP_LANGUAGES).not.toContain("yaml"); + }); + + it("renders the list as recipe prose with no gaps", () => { + const rendered = astGrepLanguageList(); + for (const language of AST_GREP_LANGUAGES) { + expect(rendered).toContain(language); + } + expect(rendered.split(", ")).toHaveLength(AST_GREP_LANGUAGES.length); + }); +}); diff --git a/packages/cli/test/prompts.test.ts b/packages/cli/test/prompts.test.ts index 6a056753..78e74222 100644 --- a/packages/cli/test/prompts.test.ts +++ b/packages/cli/test/prompts.test.ts @@ -136,9 +136,15 @@ describe("the CLI invocation variable", () => { it("is provided on every render, alongside the other markers", () => { const table = buildVariables("", "any-topic"); expect(Object.keys(table).toSorted()).toEqual([ + "AST_GREP_LANGUAGES", + "AST_GREP_VERSION", "CLI_VERSION", "PACKAGE_MANAGER_DLX", "TASKLESS_CLI", + "VALE_COMMENT_FORMATS", + "VALE_CONVERTER_FORMATS", + "VALE_MARKUP_FORMATS", + "VALE_VERSION", ]); // INPUT_SCHEMA stays conditional on the placeholder being present. expect(buildVariables("%(INPUT_SCHEMA)s", "improve-rule")).toHaveProperty( @@ -433,11 +439,20 @@ function importSpecifiers(source: string): string[] { describe("prompts entry carries no CLI runtime", () => { // Everything the render path is allowed to reach: embedded text, the two leaf - // Zod schemas, the invocation rewrite, and the templating library. + // Zod schemas, the invocation rewrite, the engine capability constants, and + // the templating library. + // + // `../rules/capabilities` earns its place by being pure data — the reach of + // the pinned `sg` and Vale binaries, transcribed rather than probed at render + // time. It sits under `src/rules/` beside its subject rather than in + // `src/prompts/`, but it imports nothing at all, so it cannot drag the + // command layer in behind it. The vendor-contract tests, which do spawn the + // binaries, are what keep it honest; see taskless/cli#151. const ALLOWED_SOURCE_IMPORTS = new Set([ "sprintf-js", "zod", "../util/invocation", + "../rules/capabilities", "../schemas/rules-create", "../schemas/rules-improve", "./recipes.js", diff --git a/packages/cli/test/recipe-cross-references.test.ts b/packages/cli/test/recipe-cross-references.test.ts index a4e5a497..1fb28816 100644 --- a/packages/cli/test/recipe-cross-references.test.ts +++ b/packages/cli/test/recipe-cross-references.test.ts @@ -4,6 +4,12 @@ import { describe, expect, it } from "vitest"; import { SUBCOMMAND_NAMES } from "../src/commands/names"; import { getRecipe } from "../src/prompts/recipes"; +import { + astGrepLanguageList, + valeCommentList, + valeConverterList, + valeMarkupList, +} from "../src/rules/capabilities"; import { buildInvocation } from "../src/util/invocation"; /** @@ -317,3 +323,71 @@ describe("recipes defer the CLI invocation to the renderer", () => { } }); }); + +/** + * `route` is the only recipe that states what each local engine can read, and + * it states it through `%(…)s` variables resolved from + * `src/rules/capabilities.ts` rather than by transcribing two lists into the + * `.txt`. A transcription would go stale on the next engine bump with nothing + * to catch it, and a stale claim about engine reach is worse than the silence + * it replaced — an agent acts on it and escalates a buildable rule to the + * runtime tier, which needs a login. That is taskless/cli#151. + * + * The vendor-contract tests pin the constants to the binaries; these pin that + * the constants reach the text an agent is handed. + */ +/** One rendered recipe, or a failure naming the topic rather than `undefined`. */ +async function rendered(file: string): Promise { + const all = await renderedRecipes(); + const found = all.find(([name]) => name === file); + expect(found, `no rendered recipe for ${file}`).toBeDefined(); + return found![1]; +} + +describe("recipes state engine reach from the pinned versions", () => { + // Same shape as "renders a real invocation into every recipe that names the + // CLI": a placeholder that survives rendering reaches the agent as literal + // `%(AST_GREP_LANGUAGES)s`, which reads as a variable it is supposed to fill + // in — the worst possible failure for a recipe whose job is to state facts. + it("leaves no unsubstituted marker in any rendered recipe", async () => { + const leaked: string[] = []; + for (const [file, text] of await renderedRecipes()) { + for (const match of text.matchAll(/%\([A-Z_]+\)s/g)) { + leaked.push(`${file}: ${match[0]}`); + } + } + expect(leaked).toEqual([]); + }); + + it("names ast-grep's languages in rendered route.txt", async () => { + const route = await rendered("route.txt"); + // Yaml is the issue's own question — an Actions workflow is YAML, and + // nothing told the agent ast-grep parses it. TypeScript is the control: a + // language nobody doubts, so a route.txt that lost the list entirely fails + // here too rather than passing on a lucky substring. + expect(route).toContain("Yaml"); + expect(route).toContain("TypeScript"); + expect(route).toContain(astGrepLanguageList()); + }); + + it("names Vale's three readable tiers and its unreadable one", async () => { + const route = await rendered("route.txt"); + expect(route).toContain(valeMarkupList()); + expect(route).toContain(valeCommentList()); + expect(route).toContain(valeConverterList()); + // 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"); + }); + + it("repeats the reach where a Vale matcher is written", async () => { + // create-vale-rule.txt is where a glob is authored, which is the only + // place the converter-dependent extensions can actually do damage. + const recipe = await rendered("create-vale-rule.txt"); + expect(recipe).toContain(valeConverterList()); + // The recipe used to offer `[*.{md,mdx}]` as a worked example of widening + // 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."); + }); +}); diff --git a/packages/cli/test/vale-vendor-contract.test.ts b/packages/cli/test/vale-vendor-contract.test.ts index 0266d4c4..f95af77e 100644 --- a/packages/cli/test/vale-vendor-contract.test.ts +++ b/packages/cli/test/vale-vendor-contract.test.ts @@ -6,6 +6,15 @@ import { join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { findValeBinary } from "../src/rules/vale/binary"; +import { + VALE_COMMENT_EXTENSIONS, + VALE_CONVERTER_DEPENDENT, + VALE_MARKUP_EXTENSIONS, + VALE_VERSION, + valeCommentList, + valeConverterList, + valeMarkupList, +} from "../src/rules/capabilities"; /** * Vale's observable behaviour, pinned. @@ -351,3 +360,186 @@ withVale("Vale vendor contract", () => { expect(parsed["doc.md"]).toHaveLength(1); }); }); + +/** + * The reach `src/rules/capabilities.ts` publishes, pinned by probe. + * + * A separate top-level block from the contract suite above, because it asserts + * something different: those cases pin how Vale behaves when we drive it, these + * pin a constant we transcribed *from* it. + * + * PROBE-MEASURED, BECAUSE VALE SELF-REPORTS NOTHING. There is no `--list-formats` + * and no capability listing anywhere in the binary, so the only way to know + * which tier an extension lands in is to lint a file and look at what came + * back. Each tier therefore has its own discriminating fixture rather than a + * shared one — on ordinary prose all three tiers are indistinguishable, which + * is exactly how a wrong claim would survive a lazier test: + * + * - **markup** is separated from plaintext by a construct only a real parser + * skips (a fenced code block, an Org `#` line, an HTML comment). + * - **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. + * - **converter-dependent** is separated from everything by failing. + * + * See taskless/cli#151. + */ +/** The comment syntax Vale must see through, per extension. */ +function comment(extension: string): string { + const HASH = new Set([ + ".jl", + ".pl", + ".pm", + ".ps1", + ".py", + ".pyw", + ".r", + ".R", + ".rb", + ]); + if (HASH.has(extension)) return "# simply\n"; + if (extension === ".css") return "/* simply */\n"; + if (extension === ".hs" || extension === ".lua") return "-- simply\n"; + if (extension === ".clj") return "; simply\n"; + return "// simply\n"; +} + +withVale("Vale engine capabilities", () => { + /** A project whose single rule applies to every extension. */ + const anyExtension = (documents: Record) => + project( + `${header}\n[*]\nrules.no-simply = YES\n`, + { "no-simply": existence("simply") }, + documents + ); + + /** Findings Vale reports for one document, under `--no-exit`. */ + function findings(name: string, body: string): unknown[] { + const cwd = anyExtension({ [name]: body }); + const result = runRaw(cwd, [name], ["--no-exit"]); + expect(result.status, `${name}: ${result.stderr}`).toBe(0); + const parsed = JSON.parse(result.stdout || "{}") as Record< + string, + unknown[] + >; + return parsed[name] ?? []; + } + + /** + * Prose Vale must find in a markup document, plus a construct it must skip. + * The second half is the whole test: without it, `.md` and `.zzz` behave + * identically and the markup tier asserts nothing. + */ + const MARKUP_FIXTURES: Record = { + ".md": { + prose: "We simply do it.\n", + skipped: "Fine.\n\n```\nsimply\n```\n", + }, + ".markdown": { + 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", + skipped: "\n", + }, + ".html": { + prose: "

We simply do it.

\n", + skipped: "\n", + }, + ".xhtml": { + prose: "

We simply do it.

\n", + skipped: "\n", + }, + }; + + 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. + const result = spawnSync(binary as string, ["--version"], { + encoding: "utf8", + }); + expect(result.stdout.trim()).toBe(`vale version ${VALE_VERSION}`); + }); + + 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 + // prevent. + expect(Object.keys(MARKUP_FIXTURES).toSorted()).toEqual( + [...VALE_MARKUP_EXTENSIONS].toSorted() + ); + }); + + it.each([...VALE_MARKUP_EXTENSIONS])( + "parses %s as markup: prose is linted, the format's own syntax is not", + (extension) => { + const fixture = MARKUP_FIXTURES[extension]!; + expect(findings(`doc${extension}`, fixture.prose)).toHaveLength(1); + expect( + findings(`skip${extension}`, fixture.skipped), + `${extension} linted a construct a real parser would skip — it is the plaintext fallback, not markup` + ).toHaveLength(0); + } + ); + + it.each([...VALE_COMMENT_EXTENSIONS])( + "lints comment text but not the code body in %s", + (extension) => { + expect( + findings(`c${extension}`, comment(extension)), + `${extension} did not lint its comment text` + ).toHaveLength(1); + expect( + findings(`b${extension}`, "simply\n"), + `${extension} linted a bare non-comment line — it is the plaintext fallback, not comment-aware` + ).toHaveLength(0); + } + ); + + 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 + // YAML flags key names and values, not only the comments. Both lines below + // are findings, and neither is a comment. + expect(findings("workflow.yml", "name: simply\n")).toHaveLength(1); + expect(findings("script.sh", "simply\n")).toHaveLength(1); + expect(findings("unknown.zzz", "simply\n")).toHaveLength(1); + }); + + it.each( + VALE_CONVERTER_DEPENDENT.flatMap(({ extensions, converter }) => + extensions.map((extension) => [extension, converter] as const) + ) + )("fails the whole run on %s, needing %s", (extension, converter) => { + // The blast radius is the point. Vale exits 2 and abandons the RUN, not + // the file — `--no-exit` does not suppress it — so one such file caught + // by any rule's glob silences every other Vale rule over every other + // file. That is why route.txt and create-vale-rule.txt both say never to + // put these extensions in a matcher. + const cwd = anyExtension({ + [`doc${extension}`]: "We simply do it.\n", + }); + const result = runRaw(cwd, [`doc${extension}`], ["--no-exit"]); + expect(result.status, `${extension} no longer fails`).not.toBe(0); + const output = `${result.stdout}${result.stderr}`; + expect(output).toContain("E100"); + // The converter name is what makes the failure actionable, and what + // keeps the recipes honest about this being a missing external tool + // rather than absent support in Vale. + expect(output).toContain(converter.replace(/^an /, "").split(" ")[0]!); + }); + + it("renders each list as recipe prose with no gaps", () => { + expect(valeMarkupList().split(", ")).toEqual([...VALE_MARKUP_EXTENSIONS]); + expect(valeCommentList().split(", ")).toEqual([...VALE_COMMENT_EXTENSIONS]); + for (const { extensions, converter } of VALE_CONVERTER_DEPENDENT) { + expect(valeConverterList()).toContain( + `${extensions.join("/")} (needs ${converter})` + ); + } + }); +}); From ee044819ac12f91db68d5df3ff9a6966bdaac39b Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Sun, 23 Aug 2026 22:12:12 -0700 Subject: [PATCH 2/4] docs(cli): correct what an off-list language spelling actually does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The recipe and the constants both claimed a plausible `C++` or `yaml` passes `verify` and fails inside the binary. Measured against the pinned 0.41.0: `C++` and `cpp` are accepted aliases that resolve to Cpp, so an off-list spelling is not reliably an error at all. The two failures that are real, and neither is caught locally: a name ast-grep does not recognize (`C#` for `CSharp`) aborts config parsing, so every other rule goes unreported alongside it; and a valid name for the wrong parser (`TypeScript` over a `.tsx` tree) reports nothing and reads as a clean codebase. The advice to copy from the list is unchanged — the reason for it is now the accurate one. The assertions in the vendor contract were already correct: they pin the canonical list, not what the binary rejects. Their comment said otherwise and now says what they actually cover. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Cyga14bww8rmazH2XrF8ms --- packages/cli/src/agent/route.txt | 8 ++++++-- packages/cli/src/rules/capabilities.ts | 9 +++++++-- packages/cli/test/ast-grep-vendor-contract.test.ts | 8 ++++++-- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/agent/route.txt b/packages/cli/src/agent/route.txt index 8ad6ebae..597e9785 100644 --- a/packages/cli/src/agent/route.txt +++ b/packages/cli/src/agent/route.txt @@ -98,8 +98,12 @@ answered together. - **ast-grep (v%(AST_GREP_VERSION)s) parses:** %(AST_GREP_LANGUAGES)s. Those spellings are ast-grep's own and go into a rule's `language:` - field verbatim. Nothing local validates that field, so a plausible - `C++` or `yaml` passes `verify` and fails inside the binary. + field verbatim. Nothing local validates that field — the vendored + schema types it as a bare string with no enum — so the first thing + with an opinion is the binary. It accepts some off-list aliases + (`C++` and `cpp` both reach the Cpp parser), but a name it does not + know at all, like `C#` for `CSharp`, aborts config parsing and + takes every other rule's report down with it. Copy from the list. **A GitHub Actions workflow is `Yaml`, and `Yaml` is on that list.** A rule about `.github/workflows/*.yml` is an `sg` rule; sending it diff --git a/packages/cli/src/rules/capabilities.ts b/packages/cli/src/rules/capabilities.ts index f050810a..6c3444ff 100644 --- a/packages/cli/src/rules/capabilities.ts +++ b/packages/cli/src/rules/capabilities.ts @@ -46,8 +46,13 @@ export const AST_GREP_VERSION = "0.41.0"; * * SPELLINGS ARE ast-grep's, NOT ours and not `detect`'s. `Cpp`, `CSharp`, * `JavaScript`, `Tsx` — a rule's `language:` field is handed to ast-grep - * unchanged, and `verify` does not check it, so a plausible-looking `C++` or - * `yaml` reaches the binary and fails there. + * unchanged and `verify` does not check it, so the binary is the first thing + * with an opinion. MEASURED at 0.41.0: it accepts some off-list aliases + * (`C++` and `cpp` both resolve to Cpp), so an off-list spelling is not + * reliably an error. The two real failures are a name ast-grep does not know + * at all (`C#`), which aborts config parsing so every rule goes unreported, + * and a valid name for the wrong parser (`TypeScript` over `.tsx`), which + * reports nothing and reads as a clean codebase. Neither is caught locally. * * Pinned by set-equality against the binary in * `test/ast-grep-vendor-contract.test.ts`, so a version bump that adds or drops diff --git a/packages/cli/test/ast-grep-vendor-contract.test.ts b/packages/cli/test/ast-grep-vendor-contract.test.ts index 2b69d678..2f197309 100644 --- a/packages/cli/test/ast-grep-vendor-contract.test.ts +++ b/packages/cli/test/ast-grep-vendor-contract.test.ts @@ -626,8 +626,12 @@ withSg("ast-grep engine capabilities", () => { it("spells languages the way a rule's `language:` field must", () => { // ast-grep's vocabulary is not `detect --json`'s: `detect` reports the - // repository's languages as `C++`, and a rule that copies that spelling - // through fails inside the binary rather than in `verify`. + // repository's languages as `C++`. These assertions are about the + // canonical list, NOT about what the binary rejects — measured at 0.41.0, + // `C++` and `cpp` are accepted aliases for Cpp. The recipe tells authors + // to copy from this list because an unrecognized name (`C#`) aborts the + // whole scan and a wrong-parser name reports nothing; see `the language + // field` suite for the cases that pin those two failures. expect(AST_GREP_LANGUAGES).toContain("Cpp"); expect(AST_GREP_LANGUAGES).not.toContain("C++"); expect(AST_GREP_LANGUAGES).not.toContain("yaml"); From 10300fb8b761b9307ff3b8cbca8427a53e4a9879 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Sun, 23 Aug 2026 23:11:49 -0700 Subject: [PATCH 3/4] fix(cli): pin Vale's converter failure by checker tag, not by host-dependent prose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The vendor contract asserted that Vale's `E100` text contains the converter name, deriving the expected token from our own prose with `converter.replace(/^an /, "").split(" ")[0]`. For `.xml` that yielded "XSLT", which matched locally and failed in CI — the only failure across all three stacked PRs. The cause is that `.xml` fails two different ways. Vale reports `xsltproc not found` where the program is absent and `no XSLT transform provided` where it is present, and the split is by platform: macOS ships /usr/bin/xsltproc, the Linux runner image does not. So the assertion was reading host state through a vendor string while presenting itself as a vendor contract. It now asserts the checker tag — `[lintXML]`, `[lintAdoc]`, `[lintRST]`, `[lintDITA]`, `[lintMDX]` — which is identical on both hosts, from a new VALE_CONVERTER_CHECKERS map held to set-equality with the extension list. The actionability claim the probe used to make moved to its own case, asserted against our own data where no binary is involved. Corrects the `.xml` entry while here. It is the one format 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, where installing asciidoctor genuinely fixes `.adoc`. It now names the stylesheet as well as the program, and that reaches the routing recipe. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Cyga14bww8rmazH2XrF8ms --- .changeset/engine-reach-in-route.md | 10 +++++ packages/cli/src/rules/capabilities.ts | 27 +++++++++++++- .../cli/test/vale-vendor-contract.test.ts | 37 ++++++++++++++++--- 3 files changed, 68 insertions(+), 6 deletions(-) diff --git a/.changeset/engine-reach-in-route.md b/.changeset/engine-reach-in-route.md index 43b069c7..aee7be4f 100644 --- a/.changeset/engine-reach-in-route.md +++ b/.changeset/engine-reach-in-route.md @@ -27,3 +27,13 @@ converter, and this CLI ships none of them — so one such file caught by a rule's glob exits 2 with an `E100` and abandons the whole run, silencing every other Vale rule over every other file. `create-vale-rule` had been offering `[*.{md,mdx}]` as its example of widening a matcher. + +`.xml` is the one entry where naming the converter is not enough. It needs +`xsltproc` **and** an XSLT stylesheet, and a stylesheet is document-specific, so +there is nothing to ship and installing the program does not make `.xml` +lintable — unlike `asciidoctor`, which genuinely fixes `.adoc`. Vale says so +differently depending on the host, too: `xsltproc not found` where the program is +absent, `no XSLT transform provided` where it is present, and macOS ships +`/usr/bin/xsltproc` while a typical Linux CI image does not. The contract test +now asserts Vale's checker tag, which is the same everywhere, rather than a +substring of the converter name. diff --git a/packages/cli/src/rules/capabilities.ts b/packages/cli/src/rules/capabilities.ts index 6c3444ff..6655c8e7 100644 --- a/packages/cli/src/rules/capabilities.ts +++ b/packages/cli/src/rules/capabilities.ts @@ -203,11 +203,36 @@ export interface ValeConverterFormat { export const VALE_CONVERTER_DEPENDENT: readonly ValeConverterFormat[] = [ { extensions: [".rst"], converter: "rst2html" }, { extensions: [".adoc", ".asciidoc"], converter: "asciidoctor" }, - { extensions: [".xml"], converter: "an XSLT transform" }, + { extensions: [".xml"], converter: "xsltproc and an XSLT stylesheet" }, { extensions: [".dita"], converter: "dita" }, { extensions: [".mdx"], converter: "mdx2vast" }, ]; +/** + * 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. + * + * `.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. + */ +export const VALE_CONVERTER_CHECKERS: Readonly> = { + ".rst": "lintRST", + ".adoc": "lintAdoc", + ".asciidoc": "lintAdoc", + ".xml": "lintXML", + ".dita": "lintDITA", + ".mdx": "lintMDX", +}; + /** Every converter-dependent extension, flattened. */ export const VALE_CONVERTER_DEPENDENT_EXTENSIONS: readonly string[] = VALE_CONVERTER_DEPENDENT.flatMap((format) => format.extensions); diff --git a/packages/cli/test/vale-vendor-contract.test.ts b/packages/cli/test/vale-vendor-contract.test.ts index f95af77e..b9f88256 100644 --- a/packages/cli/test/vale-vendor-contract.test.ts +++ b/packages/cli/test/vale-vendor-contract.test.ts @@ -8,7 +8,9 @@ import { afterEach, describe, expect, it } from "vitest"; import { findValeBinary } from "../src/rules/vale/binary"; import { VALE_COMMENT_EXTENSIONS, + VALE_CONVERTER_CHECKERS, VALE_CONVERTER_DEPENDENT, + VALE_CONVERTER_DEPENDENT_EXTENSIONS, VALE_MARKUP_EXTENSIONS, VALE_VERSION, valeCommentList, @@ -514,7 +516,7 @@ withVale("Vale engine capabilities", () => { VALE_CONVERTER_DEPENDENT.flatMap(({ extensions, converter }) => extensions.map((extension) => [extension, converter] as const) ) - )("fails the whole run on %s, needing %s", (extension, converter) => { + )("fails the whole run on %s, needing %s", (extension) => { // The blast radius is the point. Vale exits 2 and abandons the RUN, not // the file — `--no-exit` does not suppress it — so one such file caught // by any rule's glob silences every other Vale rule over every other @@ -527,10 +529,35 @@ withVale("Vale engine capabilities", () => { expect(result.status, `${extension} no longer fails`).not.toBe(0); const output = `${result.stdout}${result.stderr}`; expect(output).toContain("E100"); - // The converter name is what makes the failure actionable, and what - // keeps the recipes honest about this being a missing external tool - // rather than absent support in Vale. - expect(output).toContain(converter.replace(/^an /, "").split(" ")[0]!); + // Assert the checker tag, not the prose after it. The tag is the same + // everywhere; the prose is not — `.xml` says `xsltproc not found` where + // the program is absent and `no XSLT transform provided` where it is + // present, and macOS ships `/usr/bin/xsltproc` while the Linux CI image + // does not. Deriving the expectation from our own `converter` string + // (`replace(/^an /, "").split(" ")[0]`) matched `XSLT` locally and failed + // in CI on exactly that split — a host-dependent assertion dressed up as a + // vendor contract. `converter` is still asserted, one level up, against + // our own data where no binary is involved. + expect(output).toContain(`[${VALE_CONVERTER_CHECKERS[extension]!}]`); + }); + + it("names a checker for every converter-dependent extension", () => { + // Set-equality, so an extension added to one and not the other fails here + // rather than throwing on an undefined tag inside the probe above. + expect(Object.keys(VALE_CONVERTER_CHECKERS).toSorted()).toEqual( + [...VALE_CONVERTER_DEPENDENT_EXTENSIONS].toSorted() + ); + }); + + it("names something installable for every converter-dependent format", () => { + // The actionability claim the probe used to make, asserted against our own + // data instead of against a vendor string that varies by host. `.xml` is + // deliberately allowed to name two things: the program AND the stylesheet, + // because installing the program alone does not make `.xml` lintable. + for (const { extensions, converter } of VALE_CONVERTER_DEPENDENT) { + expect(converter, `${extensions.join("/")} names no tool`).not.toBe(""); + expect(converter).toMatch(/^[a-z]/); + } }); it("renders each list as recipe prose with no gaps", () => { From b1eaaa70ceba6b2fed58f740e5e519698ffc4f6a Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Sun, 23 Aug 2026 23:24:48 -0700 Subject: [PATCH 4/4] test(cli): pin the language-field failures beside the recipe that claims them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A comment here pointed at a `the language field` suite that did not exist on this branch — it was added one PR up the stack, so a maintainer following the pointer found nothing and might have assumed the C#-abort and wrong-parser behaviours were covered. Reported in review, and correct. Moved down rather than the comment softened. `route.txt` makes both claims on this branch, and the house rule is that an engine claim in a recipe is quoted from a pinned binary rather than described, so the claim and its pin belong in the same change. Adds a third case while here, pinning the fact this branch's own history got wrong: `C++` and `cpp` are ACCEPTED aliases resolving to Cpp, so an off-list spelling is not reliably an error. Without it the suite reads as "off-list spellings fail", which is the claim that had to be corrected. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Cyga14bww8rmazH2XrF8ms --- .../cli/test/ast-grep-vendor-contract.test.ts | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/packages/cli/test/ast-grep-vendor-contract.test.ts b/packages/cli/test/ast-grep-vendor-contract.test.ts index 2f197309..c72d9e50 100644 --- a/packages/cli/test/ast-grep-vendor-contract.test.ts +++ b/packages/cli/test/ast-grep-vendor-contract.test.ts @@ -153,6 +153,10 @@ const test = (cwd: string, ruleId: string) => /** One eval call, at line 0 column 10 of `src/a.ts`. */ const evalSource = { "src/a.ts": 'const x = eval("1");\n' }; +/** `rule("no-eval")` with its `language:` swapped for another spelling. */ +const atLanguage = (language: string) => + rule("no-eval").replace("language: TypeScript", `language: ${language}`); + /** Exit status of scanning one finding declared at `severity`. */ const statusAt = (severity: string) => scan( @@ -484,6 +488,65 @@ withSg("ast-grep vendor contract", () => { }); }); + /** + * How a wrong `language:` fails — the two shapes `route.txt` warns about. + * + * Nothing of ours catches either one first: the vendored + * `src/generated/ast-grep-rule-schema.json` types `$defs.Language` as a bare + * string with no enum, and `verify` never reads the field. So the binary's + * response IS the contract, and a recipe telling an author what to expect is + * quoting it. These live beside the recipe that makes the claim, so the claim + * and its pin land in the same change. + */ + describe("the language field", () => { + it("fails the whole scan on a spelling it does not recognize", () => { + // `C#` is the plausible wrong spelling of `CSharp`, and getting it wrong + // is not a rule that quietly matches nothing: ast-grep cannot parse the + // config, so every OTHER rule in the project goes unreported too. The + // error names the enum, which is what an author sees. + const result = scan( + project({ rules: { "no-eval": atLanguage("C#") }, sources: evalSource }) + ); + expect(result.status).toBeGreaterThan(1); + expect(result.stderr).toContain("SgLang"); + }); + + it("accepts off-list aliases, so an off-list spelling is not an error", () => { + // The half that corrects an earlier claim in this branch's own history: + // `C++` and `cpp` are NOT rejected, they resolve to Cpp. The recipe tells + // authors to copy from the canonical list for the other two reasons, not + // because every off-list spelling fails. + const sources = { + "src/a.cpp": "int main(){int simply=1;return simply;}\n", + }; + for (const spelling of ["Cpp", "C++", "cpp"]) { + const result = scan( + project({ rules: { "no-eval": atLanguage(spelling) }, sources }) + ); + expect(result.status, `${spelling} was rejected`).not.toBeGreaterThan( + 1 + ); + } + }); + + it("treats Tsx and TypeScript as different parsers, not aliases", () => { + // The quiet half of the same field, and the reason the recipe names this + // pair specifically. `TypeScript` over a `.tsx` tree exits clean with no + // findings, which is indistinguishable from a codebase with nothing to + // flag — the rule looks written and proves nothing. + const sources = { "src/a.tsx": "const el =
{eval(x)}
;\n" }; + const asTypeScript = scan( + project({ rules: { "no-eval": atLanguage("TypeScript") }, sources }) + ); + expect(asTypeScript.status).toBe(0); + expect(asTypeScript.stdout.trim()).toBe(""); + expect( + scan(project({ rules: { "no-eval": atLanguage("Tsx") }, sources })) + .stdout + ).toContain("eval(x)"); + }); + }); + /** * Relocated from `engine-layout.test.ts`, which existed only for these two. *