Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/vale-config-schema.md
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 2 additions & 0 deletions openspec/changes/vale-config-schema/.openspec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-09-21
48 changes: 48 additions & 0 deletions openspec/changes/vale-config-schema/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
## Why

A per-rule `.taskless/rules/vale/<id>/.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 `<id>.<id>`. 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 = <id>`; an assignment key other than `<id>.<id>`; 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.
112 changes: 112 additions & 0 deletions openspec/changes/vale-config-schema/specs/cli-rule-validation/spec.md
Original file line number Diff line number Diff line change
@@ -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` | `<id>.yml` against the ast-grep schema and the Taskless required fields |
| `vale` | `<id>.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 `<id>.<id> = 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 `<style>.<check>` key naming a different rule
- **THEN** `verify` SHALL report it, naming the key and the line
- **AND** it SHALL NOT report the rule as valid

#### Scenario: A rule config advisory does not fail verify

- **WHEN** a Vale rule's `.vale.ini` assigns the same key twice inside one matcher, and another matcher still enables the rule
- **THEN** `verify` SHALL report the rule as valid
- **AND** the repeat SHALL be printed as a notice on that rule

#### Scenario: A rule config whose only enable is overridden is rejected

- **WHEN** a Vale rule's `.vale.ini` assigns `<id>.<id> = YES` and then `<id>.<id> = NO` in its only matcher
- **THEN** `verify` SHALL report that the rule is present but off, under `vale-config-enabled-somewhere`
- **AND** the repeat SHALL still be printed as a notice on that rule

### Requirement: A rejection names the constraint it violated

`verify --json` and `test --json` SHALL report, per rule, the constraints a
rejection violated, pairing a `constraintId` drawn from the published
`RULE_CONSTRAINTS` with the message that reports it. Vale config rejections are
attributable in the same way, under `vale-config-*` constraint ids.

The existing `errors` array SHALL continue to carry every failure message,
including those that are attributable. A consumer reading only `errors` SHALL
see what it sees today, so this is additive.

An error with no constraint behind it SHALL NOT be given one. A wrong
attribution sends a reader to a rationale that does not describe their failure,
which is worse than sending them to none.

A consumer SHALL NOT have to match on message text to recover the constraint.
Text matching rots the first time a message is rephrased, and rephrasing an
error message is not a breaking change.

#### Scenario: A mismatched rule id is attributed

- **WHEN** `verify --json` refuses a rule whose `id:` does not match its directory
- **THEN** the rule's result SHALL carry a violation with `constraintId` `sg-id-matches-directory`
- **AND** the violation's message SHALL also appear in `errors`

#### Scenario: An unattributable failure carries no id

- **WHEN** `verify --json` reports a failure that no published constraint describes
- **THEN** the message SHALL appear in `errors`
- **AND** no violation SHALL be reported for it

#### Scenario: A passing rule reports no violations

- **WHEN** `verify --json` accepts a rule
- **THEN** its violations SHALL be empty

#### Scenario: A Vale config rejection is attributed

- **WHEN** `verify --json` rejects a Vale rule whose config assigns a key naming another rule
- **THEN** the rule's result SHALL carry a violation with `constraintId` `vale-config-own-key-only`
- **AND** the violation's message SHALL also appear in `errors`
Loading
Loading