diff --git a/.changeset/vale-config-schema.md b/.changeset/vale-config-schema.md new file mode 100644 index 00000000..ecd422cb --- /dev/null +++ b/.changeset/vale-config-schema.md @@ -0,0 +1,5 @@ +--- +"@taskless/cli": patch +--- + +`verify` validates a Vale rule's `.vale.ini` against a schema and names the constraint each rejection violates. The config is parsed into an ordered structure and checked there: an assignment above the first matcher, a matcher without its `tskl) rule` breadcrumb, a key naming another rule, a value other than `YES`/`NO`, a non-empty `BasedOnStyles`, a config with no matcher or no `YES`, and a `NO` matcher that precedes every `YES` are each rejected under a `vale-config-*` constraint that `verify --json` reports in `violations[]` and `reference.json` publishes. A repeated key, a `[*]` matcher, and a `.taskless/**` matcher are reported as a notice without failing the rule. `check` is unchanged. diff --git a/openspec/changes/vale-config-schema/.openspec.yaml b/openspec/changes/vale-config-schema/.openspec.yaml new file mode 100644 index 00000000..563fab5a --- /dev/null +++ b/openspec/changes/vale-config-schema/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-09-21 diff --git a/openspec/changes/vale-config-schema/proposal.md b/openspec/changes/vale-config-schema/proposal.md new file mode 100644 index 00000000..90cab837 --- /dev/null +++ b/openspec/changes/vale-config-schema/proposal.md @@ -0,0 +1,48 @@ +## Why + +A per-rule `.taskless/rules/vale//.vale.ini` is the one rule input nothing validates. The style YAML goes through `schemas/vale-rule.ts` and the pinned corpus; the config is carried into the assembled run config as verbatim text. `assemble.ts` strips a copied-in `StylesPath`/`MinAlertLevel` with `line.split("=")[0]` and recovers matchers with `/^\[(.+)\]$/`; `verify` checks that the file contains a `[` and the substring `.`. Nothing checks that a matcher carries the `tskl) rule` breadcrumb, that a value is `YES` or `NO`, that an assignment names this rule and not another one, or that the run-level keys stayed out. The spec says a rule SHALL NOT be able to override another rule's matchers, and nothing enforces it. + +The gap is visible in a dogfood repository (eleven voice rules over ~1,100 markdown files, notes dated 2026-09-21): a fourth `.taskless/**` matcher copied into every rule to keep fixtures quiet under a bare `vale` run that `check` never performs, a duplicated key whose meaning flipped when Vale 3.21.0 moved repeated assignments from first-wins to last-wins, and matchers whose ordering only a person who has read the scoping spec can tell is backwards. Each of these is a config that Vale accepts and that does something other than what its author wrote, and the engine's design exists to remove exactly that class of silent failure. + +## What Changes + +- **Every per-rule config is parsed into an AST** with `@jedmao/ini-parser` (`resolve: false`, `delimiter: /=/`; blank lines, which the parser reports as empty unnamed sections, are dropped). The AST is lossless and ordered, so every check below is a refinement over the file's own sequence and nothing is re-derived from text. The parser was chosen by measurement against four alternatives: the `ini` lineage nests section names on `.` and loses source order, both fatal for a file whose sections are globs and whose order is its precedence; `iniparser` drops the `tskl) rule` key; `config-ini-parser` truncates it to `rule` and is GPL-3.0. Recorded in taskless/cli#359. +- **A zod schema over that AST**, `schemas/vale-config.ts`, keyed by the rule's directory id, rejects: any root-level property (`StylesPath`, `MinAlertLevel`, or anything else placed above the first matcher, which Vale reports as `W101` and ignores); a matcher without `tskl) rule = `; an assignment key other than `.`; a value other than `YES`/`NO`; a non-empty `BasedOnStyles`; a file with no matcher; a file that never assigns `YES`; a `NO` matcher that precedes every `YES` (with `BasedOnStyles` empty a rule is off until a `YES`, so such a `NO` is dead or overridden in every config — refusing it turns away nothing an author meant). It reports, without rejecting: a key assigned twice inside one matcher; a `[*]` matcher; a `.taskless/**` matcher. Each advisory has a legitimate reading, which is what separates the two lists. +- **`verify` runs the schema** in place of its two substring checks, and the rejections are attributable: the first Vale entries in `RULE_CONSTRAINTS`, so `verify --json` pairs each with a `constraintId` the way it already does for `sg`. Advisories reach the reader on the rule's `notice`. +- **Assembly runs the schema and refuses the Vale run** when any rule's config is rejected, naming the rule and the line. This is the same treatment a malformed ast-grep rule already gets: the engine that owns the bad file reports a failure that reaches the exit code, and the other engines still run. A rule left out with a soft notice would verify, run, and report nothing, which is the failure being closed. Advisories go to the check's `notices`. +- **Assembly stays byte concatenation.** The header, then per rule a breadcrumb comment and the source file verbatim. The schema's rejection of run-level keys removes the only string edit assembly performs today, and `sections` is read from the AST's section names. No serializer is introduced; the parser's `toString()` is measured not round-trip safe and is never called. +- **The `create-vale-rule` recipe** says the config is schema-checked and what each rejection and advisory means, and its `.vale.ini` guidance drops the `.taskless/**` matcher. The `update` ledger records that a previously tolerated config now refuses the Vale run. + +Nothing here is **BREAKING**, and the bump is `patch`. Slice 1 only makes `verify` stricter. Slice 2 makes `check` refuse a config it used to run to a zero exit, which looks like the "must react" case the bump guidance reserves higher bumps for, and is not: `0.y.z` is the first gate, and every config the schema refuses was already being misread by Vale — a rule enabled nowhere (`W101`), a rule silently overriding a neighbour, a disable the following enable cancelled. The release surfaces a defect the consumer already had rather than introducing one, the same call as dropping the 128 KB guard in 0.11.3, where files that were silently skipped are linted again. The changeset and the `update` ledger say what to do: run `verify`, which names the line. + +## Capabilities + +### New Capabilities + +None. + +### Modified Capabilities + +- `cli-vale-rule-engine`: a new requirement that a rule's config is validated against a schema before it is assembled, and that a rejected config refuses the Vale run; "Per-rule scoping is expressed in the rule's own Vale config" gains the statement that the schema enforces the cross-rule prohibition and the disable-after-enable ordering it already requires; "Vale diagnostics on a successful run are surfaced as notices" is re-pointed at diagnostics the schema cannot foresee, since its motivating case (a top-level assignment) is now rejected before Vale runs. +- `cli-rule-validation`: "Verify checks a rule's required components" gains scenarios for a rejected config; "A rejection names the constraint it violated" gains the Vale constraint ids. + +## Impact + +- New dependency: `@jedmao/ini-parser@0.2.4` (MIT, zero dependencies, ~300 lines, last published 2022). Pinned exactly. If it ever needs a change the file is vendored, not replaced with a parser of our own. +- `packages/cli/src/schemas/vale-config.ts` (new): parse and validate; exports the AST type, the rejection list with constraint ids, and the advisory list. +- `packages/cli/src/rules/constraints.ts`: first `vale` entries, `enforcedBy: "verify"` and one for assembly. +- `packages/cli/src/rules/inspect.ts`: `verifyOneRule` for `vale` calls the schema; the two substring checks and their messages become schema rejections. `violations` is no longer always empty for Vale. +- `packages/cli/src/rules/assemble.ts`: `ruleConfigBody` and `sectionPatternsOf` go; `assembleValeConfig` parses, validates, refuses on rejection, and concatenates the verbatim source. Its return carries the advisories. +- `packages/cli/src/rules/dispatch.ts`: a refused assembly becomes the Vale engine's `failure`; advisories join `notices`. +- `packages/cli/src/rules/vale/verify.ts`: the `buildIsolatingConfig` docblock still says a repeat inside one matcher is discarded, which stopped being true at 3.21.0; corrected in passing. +- `packages/cli/src/agent/create-vale-rule.md` (topic bump), `packages/cli/src/agent/update.md` (0.11.3 ledger line). +- Tests: schema unit tests over an ini fixture set (one file per rejection and advisory, plus the dogfood config shape), `verify --json` attribution for Vale, assembly refusal and the byte-identical assembly scenario re-pinned through the new path, and a vendor-contract case that the parser and Vale agree on which lines are matchers for the globs the corpus uses (`[*.md]`, `[docs/**/*.md]`, a character class like `[docs/[a-z]*.md]`). + +## Delivery shape + +**Stacked, merging forward, two PRs.** Each is safe on `main` alone: + +1. Parser, schema, constraints, and `verify`. Stricter `verify` output and nothing else; `check` is untouched. The changeset lives here. +2. Assembly refusal, dispatch wiring, recipe, ledger, and the archive. Extends the changeset. + +Ordering them this way means an author whose config would be refused hears it from `verify` in a release before `check` starts refusing it, which is the direction the existing "silent disable" guidance already pushes people. diff --git a/openspec/changes/vale-config-schema/specs/cli-rule-validation/spec.md b/openspec/changes/vale-config-schema/specs/cli-rule-validation/spec.md new file mode 100644 index 00000000..bfdb4ff5 --- /dev/null +++ b/openspec/changes/vale-config-schema/specs/cli-rule-validation/spec.md @@ -0,0 +1,112 @@ +## MODIFIED Requirements + +### Requirement: Verify checks a rule's required components + +`verify` SHALL check that a rule has the components its engine requires and that they are well formed, and SHALL NOT require fixtures or test cases to exist. + +The two commands split because they have different preconditions. An agent part-way through authoring has a rule and no fixtures yet, and needs to know the rule itself is valid before it can write a meaningful test for it. + +Per engine, `verify` SHALL check: + +| Engine | Components | +| --------- | ------------------------------------------------------------------------------------------------------------------------------ | +| `sg` | `.yml` against the ast-grep schema and the Taskless required fields | +| `vale` | `.yml` against the Vale rule schema and the Taskless required fields, and the rule's `.vale.ini` against the config schema | +| `runtime` | `check.ts` present, and at least one capture rule under `captures/` | + +The `vale` row previously read "against Vale's own validation." Measured against the pinned 3.18.0 binary, that covers less than it claims: `level: bananas` is reported, while `extends: nonsense` and `scope: fenced` both verify clean and produce a rule that matches nothing. Vale validates a rule when it _runs_ one, and it runs one field at a time — so a name it does not recognize is not an error, it is a check that never fires. Schema validation is therefore its own layer for `vale`, as it already is for `sg`. + +#### Scenario: A rule with no fixtures still verifies + +- **WHEN** `verify` runs against a rule whose fixture buckets are empty or absent +- **THEN** it SHALL report on the rule's components only +- **AND** the absence of fixtures SHALL NOT be a verify failure + +#### Scenario: A malformed rule reports its own error + +- **WHEN** a Vale style declares a `level` outside `suggestion`/`warning`/`error` +- **THEN** `verify` SHALL report that error, naming the field + +#### Scenario: An unrecognized extension point is rejected + +- **WHEN** a Vale style declares an `extends` that is not one of Vale's check types +- **THEN** `verify` SHALL report it, naming the field and the accepted values +- **AND** it SHALL NOT report the rule as valid + +#### Scenario: An unrecognized scope is rejected + +- **WHEN** a Vale style declares a `scope` that is not one of Vale's scope values +- **THEN** `verify` SHALL report it, naming the field +- **AND** a scope using the `~` negation or `&` chaining syntax over recognized values SHALL be accepted + +#### Scenario: A field belonging to another check type is rejected + +- **WHEN** a Vale style declares a field its `extends` does not accept, such as `tokens` on an `occurrence` check +- **THEN** `verify` SHALL report it before Vale is invoked + +The failure it prevents is not a local one: Vale reports this as `E201: has invalid keys` and reads one assembled config per run, so a single rule with a stray field suppresses every other Vale rule's findings. + +#### Scenario: A rule config that never enables the rule is rejected + +- **WHEN** a Vale rule's `.vale.ini` declares matchers but no `. = YES` +- **THEN** `verify` SHALL report that the rule is present but off, naming the file + +#### Scenario: A rule config that assigns a foreign key is rejected + +- **WHEN** a Vale rule's `.vale.ini` assigns a `