From c5340b0936b4267741af01ebde02271604538fd5 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 26 Oct 2026 09:18:03 -0700 Subject: [PATCH 1/6] docs(agent): one fixture layout block for every authoring recipe, and archive the change --- .../.openspec.yaml | 0 .../design.md | 0 .../proposal.md | 0 .../specs/cli-agent-authoring/spec.md | 0 .../specs/cli-conformance-reference/spec.md | 0 .../specs/cli-generated-rule-delivery/spec.md | 0 .../specs/cli-rule-fixtures/spec.md | 0 .../specs/cli-rule-format/spec.md | 0 .../specs/cli-rule-reconciliation/spec.md | 0 .../specs/cli-rule-validation/spec.md | 0 .../specs/cli-rules/spec.md | 0 .../specs/cli-runtime-rule-execution/spec.md | 0 .../specs/cli-taskless-bootstrap/spec.md | 0 .../specs/cli-vale-rule-engine/spec.md | 0 .../tasks.md | 0 openspec/specs/cli-agent-authoring/spec.md | 26 ++++- .../specs/cli-conformance-reference/spec.md | 95 ++++++++-------- .../specs/cli-generated-rule-delivery/spec.md | 43 +++++-- openspec/specs/cli-rule-fixtures/spec.md | 98 ++++++++++++++++ openspec/specs/cli-rule-format/spec.md | 25 ++-- .../specs/cli-rule-reconciliation/spec.md | 18 ++- openspec/specs/cli-rule-validation/spec.md | 67 ++--------- openspec/specs/cli-rules/spec.md | 6 +- .../specs/cli-runtime-rule-execution/spec.md | 8 +- openspec/specs/cli-taskless-bootstrap/spec.md | 48 ++++++++ openspec/specs/cli-vale-rule-engine/spec.md | 14 ++- packages/cli/src/agent/create-remote-rule.md | 4 +- packages/cli/src/agent/create-runtime-rule.md | 25 ++-- packages/cli/src/agent/create-sg-rule.md | 51 ++++----- packages/cli/src/agent/create-vale-rule.md | 107 ++++++------------ packages/cli/src/agent/delete-rule.md | 6 +- .../cli/src/agent/improve-rule.anonymous.md | 16 +-- packages/cli/src/agent/improve-rule.md | 16 +-- packages/cli/src/agent/update.md | 34 +++++- packages/cli/src/agent/validate-rule.md | 10 +- packages/cli/src/prompts/recipes.ts | 42 +++++++ packages/cli/test/prompts.test.ts | 1 + .../cli/test/recipe-cross-references.test.ts | 18 +-- 38 files changed, 489 insertions(+), 289 deletions(-) rename openspec/changes/{unified-rule-fixtures => archive/2026-10-26-unified-rule-fixtures}/.openspec.yaml (100%) rename openspec/changes/{unified-rule-fixtures => archive/2026-10-26-unified-rule-fixtures}/design.md (100%) rename openspec/changes/{unified-rule-fixtures => archive/2026-10-26-unified-rule-fixtures}/proposal.md (100%) rename openspec/changes/{unified-rule-fixtures => archive/2026-10-26-unified-rule-fixtures}/specs/cli-agent-authoring/spec.md (100%) rename openspec/changes/{unified-rule-fixtures => archive/2026-10-26-unified-rule-fixtures}/specs/cli-conformance-reference/spec.md (100%) rename openspec/changes/{unified-rule-fixtures => archive/2026-10-26-unified-rule-fixtures}/specs/cli-generated-rule-delivery/spec.md (100%) rename openspec/changes/{unified-rule-fixtures => archive/2026-10-26-unified-rule-fixtures}/specs/cli-rule-fixtures/spec.md (100%) rename openspec/changes/{unified-rule-fixtures => archive/2026-10-26-unified-rule-fixtures}/specs/cli-rule-format/spec.md (100%) rename openspec/changes/{unified-rule-fixtures => archive/2026-10-26-unified-rule-fixtures}/specs/cli-rule-reconciliation/spec.md (100%) rename openspec/changes/{unified-rule-fixtures => archive/2026-10-26-unified-rule-fixtures}/specs/cli-rule-validation/spec.md (100%) rename openspec/changes/{unified-rule-fixtures => archive/2026-10-26-unified-rule-fixtures}/specs/cli-rules/spec.md (100%) rename openspec/changes/{unified-rule-fixtures => archive/2026-10-26-unified-rule-fixtures}/specs/cli-runtime-rule-execution/spec.md (100%) rename openspec/changes/{unified-rule-fixtures => archive/2026-10-26-unified-rule-fixtures}/specs/cli-taskless-bootstrap/spec.md (100%) rename openspec/changes/{unified-rule-fixtures => archive/2026-10-26-unified-rule-fixtures}/specs/cli-vale-rule-engine/spec.md (100%) rename openspec/changes/{unified-rule-fixtures => archive/2026-10-26-unified-rule-fixtures}/tasks.md (100%) create mode 100644 openspec/specs/cli-rule-fixtures/spec.md diff --git a/openspec/changes/unified-rule-fixtures/.openspec.yaml b/openspec/changes/archive/2026-10-26-unified-rule-fixtures/.openspec.yaml similarity index 100% rename from openspec/changes/unified-rule-fixtures/.openspec.yaml rename to openspec/changes/archive/2026-10-26-unified-rule-fixtures/.openspec.yaml diff --git a/openspec/changes/unified-rule-fixtures/design.md b/openspec/changes/archive/2026-10-26-unified-rule-fixtures/design.md similarity index 100% rename from openspec/changes/unified-rule-fixtures/design.md rename to openspec/changes/archive/2026-10-26-unified-rule-fixtures/design.md diff --git a/openspec/changes/unified-rule-fixtures/proposal.md b/openspec/changes/archive/2026-10-26-unified-rule-fixtures/proposal.md similarity index 100% rename from openspec/changes/unified-rule-fixtures/proposal.md rename to openspec/changes/archive/2026-10-26-unified-rule-fixtures/proposal.md diff --git a/openspec/changes/unified-rule-fixtures/specs/cli-agent-authoring/spec.md b/openspec/changes/archive/2026-10-26-unified-rule-fixtures/specs/cli-agent-authoring/spec.md similarity index 100% rename from openspec/changes/unified-rule-fixtures/specs/cli-agent-authoring/spec.md rename to openspec/changes/archive/2026-10-26-unified-rule-fixtures/specs/cli-agent-authoring/spec.md diff --git a/openspec/changes/unified-rule-fixtures/specs/cli-conformance-reference/spec.md b/openspec/changes/archive/2026-10-26-unified-rule-fixtures/specs/cli-conformance-reference/spec.md similarity index 100% rename from openspec/changes/unified-rule-fixtures/specs/cli-conformance-reference/spec.md rename to openspec/changes/archive/2026-10-26-unified-rule-fixtures/specs/cli-conformance-reference/spec.md diff --git a/openspec/changes/unified-rule-fixtures/specs/cli-generated-rule-delivery/spec.md b/openspec/changes/archive/2026-10-26-unified-rule-fixtures/specs/cli-generated-rule-delivery/spec.md similarity index 100% rename from openspec/changes/unified-rule-fixtures/specs/cli-generated-rule-delivery/spec.md rename to openspec/changes/archive/2026-10-26-unified-rule-fixtures/specs/cli-generated-rule-delivery/spec.md diff --git a/openspec/changes/unified-rule-fixtures/specs/cli-rule-fixtures/spec.md b/openspec/changes/archive/2026-10-26-unified-rule-fixtures/specs/cli-rule-fixtures/spec.md similarity index 100% rename from openspec/changes/unified-rule-fixtures/specs/cli-rule-fixtures/spec.md rename to openspec/changes/archive/2026-10-26-unified-rule-fixtures/specs/cli-rule-fixtures/spec.md diff --git a/openspec/changes/unified-rule-fixtures/specs/cli-rule-format/spec.md b/openspec/changes/archive/2026-10-26-unified-rule-fixtures/specs/cli-rule-format/spec.md similarity index 100% rename from openspec/changes/unified-rule-fixtures/specs/cli-rule-format/spec.md rename to openspec/changes/archive/2026-10-26-unified-rule-fixtures/specs/cli-rule-format/spec.md diff --git a/openspec/changes/unified-rule-fixtures/specs/cli-rule-reconciliation/spec.md b/openspec/changes/archive/2026-10-26-unified-rule-fixtures/specs/cli-rule-reconciliation/spec.md similarity index 100% rename from openspec/changes/unified-rule-fixtures/specs/cli-rule-reconciliation/spec.md rename to openspec/changes/archive/2026-10-26-unified-rule-fixtures/specs/cli-rule-reconciliation/spec.md diff --git a/openspec/changes/unified-rule-fixtures/specs/cli-rule-validation/spec.md b/openspec/changes/archive/2026-10-26-unified-rule-fixtures/specs/cli-rule-validation/spec.md similarity index 100% rename from openspec/changes/unified-rule-fixtures/specs/cli-rule-validation/spec.md rename to openspec/changes/archive/2026-10-26-unified-rule-fixtures/specs/cli-rule-validation/spec.md diff --git a/openspec/changes/unified-rule-fixtures/specs/cli-rules/spec.md b/openspec/changes/archive/2026-10-26-unified-rule-fixtures/specs/cli-rules/spec.md similarity index 100% rename from openspec/changes/unified-rule-fixtures/specs/cli-rules/spec.md rename to openspec/changes/archive/2026-10-26-unified-rule-fixtures/specs/cli-rules/spec.md diff --git a/openspec/changes/unified-rule-fixtures/specs/cli-runtime-rule-execution/spec.md b/openspec/changes/archive/2026-10-26-unified-rule-fixtures/specs/cli-runtime-rule-execution/spec.md similarity index 100% rename from openspec/changes/unified-rule-fixtures/specs/cli-runtime-rule-execution/spec.md rename to openspec/changes/archive/2026-10-26-unified-rule-fixtures/specs/cli-runtime-rule-execution/spec.md diff --git a/openspec/changes/unified-rule-fixtures/specs/cli-taskless-bootstrap/spec.md b/openspec/changes/archive/2026-10-26-unified-rule-fixtures/specs/cli-taskless-bootstrap/spec.md similarity index 100% rename from openspec/changes/unified-rule-fixtures/specs/cli-taskless-bootstrap/spec.md rename to openspec/changes/archive/2026-10-26-unified-rule-fixtures/specs/cli-taskless-bootstrap/spec.md diff --git a/openspec/changes/unified-rule-fixtures/specs/cli-vale-rule-engine/spec.md b/openspec/changes/archive/2026-10-26-unified-rule-fixtures/specs/cli-vale-rule-engine/spec.md similarity index 100% rename from openspec/changes/unified-rule-fixtures/specs/cli-vale-rule-engine/spec.md rename to openspec/changes/archive/2026-10-26-unified-rule-fixtures/specs/cli-vale-rule-engine/spec.md diff --git a/openspec/changes/unified-rule-fixtures/tasks.md b/openspec/changes/archive/2026-10-26-unified-rule-fixtures/tasks.md similarity index 100% rename from openspec/changes/unified-rule-fixtures/tasks.md rename to openspec/changes/archive/2026-10-26-unified-rule-fixtures/tasks.md diff --git a/openspec/specs/cli-agent-authoring/spec.md b/openspec/specs/cli-agent-authoring/spec.md index afad256a..bdae887d 100644 --- a/openspec/specs/cli-agent-authoring/spec.md +++ b/openspec/specs/cli-agent-authoring/spec.md @@ -28,7 +28,7 @@ The `create-vale-rule` recipe SHALL instruct the agent to produce three artifact 1. A Vale style file at `.taskless/rules/vale//.yml`. 2. That rule's own `.taskless/rules/vale//.vale.ini`, declaring the matchers that scope it and enabling it as `. = YES`. -3. `pass/` and `fail/` fixture documents under `.taskless/rules/vale//.tests/`. +3. Fixture case directories under `.taskless/rules/vale//tests/pass/` and `tests/fail/`, laid out as the shared fixture section describes. The recipe SHALL state that scope is declared in the rule's own config, that no shared file is edited, and that the project-wide config is assembled rather than authored. @@ -241,3 +241,27 @@ Enforcing this is the recipe's job. `verify` SHALL NOT lint fixtures and SHALL N - **WHEN** `verify` runs a rule whose `pass` fixture would fail the repository's linter - **THEN** `verify` SHALL report on the rule's behavior alone, and SHALL NOT fail the rule over that fixture's style + +### Requirement: Authoring recipes share one fixture section + +Every recipe in which the agent writes or extends fixtures (`create-sg-rule`, `create-vale-rule`, `create-runtime-rule`, `improve-rule` and `validate-rule`) SHALL direct the agent to one shared fixture section and SHALL NOT carry its own copy of the layout. The shared section SHALL state: + +1. The layout: `tests/pass//` and `tests/fail//`, with the case directory named for what the case shows. +2. That a case is judged by the engine's own check with the case directory as the root, so a fixture for a scoped rule places its files at a path the scope accepts. For example, an ast-grep rule scoped to `.github/workflows/*.yml` needs `tests/fail//.github/workflows/ci.yml`. +3. That a `fail/` case needs one finding and a `pass/` case none, and that nothing about position, message, or count is asserted. +4. What a "scope excluded this case" failure means, and that the fix is the fixture's path or the scope, not the pattern. + +A per-engine recipe SHALL keep only what is that engine's own, such as Vale's scope reach and the fact that a runtime case is the check's root. + +**Rationale.** The fixture contract is now one contract. Three copies of it would drift the way the three fixture readers did, and the path rule for scoped rules is the one new thing authors have to learn. It belongs where every recipe reaches it. + +#### Scenario: Every authoring recipe reaches the same fixture guidance + +- **WHEN** the agent follows any recipe that writes fixtures +- **THEN** the recipe SHALL direct it to the shared fixture section +- **AND** SHALL NOT restate the layout in its own words + +#### Scenario: A scoped rule's fixture path is taught + +- **WHEN** the agent writes fixtures for a rule that declares `files:` or Vale matchers +- **THEN** the shared section SHALL direct it to place each case's files at a path the scope accepts diff --git a/openspec/specs/cli-conformance-reference/spec.md b/openspec/specs/cli-conformance-reference/spec.md index a4025a34..f5f51aa8 100644 --- a/openspec/specs/cli-conformance-reference/spec.md +++ b/openspec/specs/cli-conformance-reference/spec.md @@ -33,52 +33,6 @@ behaving correctly, so the bump is the only signal the corpus owes it. - **THEN** the files a generator must produce SHALL be carried apart from the cases both sides' rules must satisfy - **AND** no file SHALL appear in both -### Requirement: The corpus states how its fixtures group into cases - -Each corpus entry's `tests` SHALL carry a `grouping` field naming how its fixture -files group into cases, the flat list of fixture files with their contents, and -— where the grouping is the CLI's own — an explicit list of cases. - -A consumer SHALL be able to recover which files belong to which case by reading -published fields alone. Deriving it from path prefixes requires knowing that a -`runtime` case is a directory and a `vale` case is a document, which is a fact -about the CLI's layout that every consumer would otherwise transcribe, and a -transcribed fact goes stale silently when the layout changes. - -Each case SHALL name the `bucket` it asserts (`pass` or `fail`), its own `name`, -the `path` the engine is pointed at, and the fixture files it holds. Case files -SHALL be named by path rather than by repeated content, and every such path -SHALL resolve to an entry in the same `tests.files` list. - -`bucket` SHALL be spelled `pass`/`fail`, matching the directories the CLI reads, -rather than an individual engine's vocabulary. - -#### Scenario: A runtime case is a directory of files - -- **WHEN** a consumer reads the `runtime` entry -- **THEN** `grouping` SHALL be `case-directories` -- **AND** each case SHALL name the directory the check is handed as its root -- **AND** a case whose evidence spans two files SHALL list both - -#### Scenario: A vale case is one document - -- **WHEN** a consumer reads the `vale` entry -- **THEN** `grouping` SHALL be `case-documents` -- **AND** each case SHALL name the document and hold exactly that one file - -#### Scenario: An ast-grep entry says its grouping is not the CLI's - -- **WHEN** a consumer reads the `sg` entry -- **THEN** `grouping` SHALL be `ast-grep-test` -- **AND** the ast-grep test file SHALL be carried whole -- **AND** no case list SHALL be published, because the grouping inside that file belongs to ast-grep's own documented schema rather than to the CLI - -#### Scenario: Every case file resolves - -- **WHEN** the corpus is generated -- **THEN** every path a case names SHALL appear in that entry's `tests.files` -- **AND** every file in `tests.files` SHALL belong to exactly one case, where a case list is published - ### Requirement: The corpus states where a rule lives The corpus SHALL publish the rule layout: the root every rule hangs off, the @@ -130,3 +84,52 @@ disagreement about the subject the rule addresses. - **WHEN** the commands an `enforcedBy` value names are renamed, so an existing value would mean a different command - **THEN** the corpus `version` SHALL be incremented - **AND** a consumer reading the new version SHALL find only the current command names among the `enforcedBy` values + +### Requirement: The corpus publishes every engine's fixtures as case directories + +Each corpus entry's `tests` SHALL carry a `grouping` field naming how its fixture +files group into cases, the flat list of fixture files with their contents, and +an explicit list of cases. + +A consumer SHALL be able to recover which files belong to which case by reading +published fields alone, so that the case layout stays a published fact and is +not transcribed by every consumer. A transcribed fact goes stale silently when +the layout changes, and this change is such a change. + +Every engine SHALL publish `grouping` as `case-directories` with an explicit case +list. The field stays even though it now has one value: a consumer asserts on +it, and it is what a later layout change would move. The `case-documents` and +`ast-grep-test` values SHALL NOT be published, because no engine uses them. + +Each case SHALL name the `bucket` it asserts (`pass` or `fail`), its own `name`, +the `path` the engine is pointed at, and the fixture files it holds. Case files +SHALL be named by path rather than by repeated content, and every such path +SHALL resolve to an entry in the same `tests.files` list. + +`bucket` SHALL be spelled `pass`/`fail`, matching the directories the CLI reads, +rather than an individual engine's vocabulary. + +#### Scenario: Every engine publishes case directories + +- **WHEN** a consumer reads the `sg`, `vale`, or `runtime` entry +- **THEN** `grouping` SHALL be `case-directories` +- **AND** each case SHALL name the directory the engine is handed as its root +- **AND** a case whose evidence spans two files SHALL list both + +#### Scenario: No engine-native test file is published + +- **WHEN** a consumer reads the `sg` entry +- **THEN** its `tests.files` SHALL hold no ast-grep test YAML +- **AND** every file SHALL belong to a published case + +#### Scenario: A single-document case is still a directory + +- **WHEN** a consumer reads the `vale` entry and a case holds a single document +- **THEN** the case SHALL name its directory as its `path` +- **AND** SHALL list that one document as its file + +#### Scenario: Every case file resolves + +- **WHEN** the corpus is generated +- **THEN** every path a case names SHALL appear in that entry's `tests.files` +- **AND** every file in `tests.files` SHALL belong to exactly one case diff --git a/openspec/specs/cli-generated-rule-delivery/spec.md b/openspec/specs/cli-generated-rule-delivery/spec.md index c980d6bb..3ce1da11 100644 --- a/openspec/specs/cli-generated-rule-delivery/spec.md +++ b/openspec/specs/cli-generated-rule-delivery/spec.md @@ -81,11 +81,15 @@ When the CLI writes a runtime rule from a response whose `entitlement.runtimeSig ### Requirement: A delivered file set is verified against its signatures Before writing any file of a served rule, the CLI SHALL verify that every signature names a -delivered file, that every delivered file outside `.tests/` has exactly one signature, and that +delivered file, that every delivered file outside `tests/` has exactly one signature, and that the algoVersion-1 `canonicalHash` of each such file's content equals its signature. For a runtime rule it SHALL also verify that the singular `signature` equals the `check.ts` entry. Any failure SHALL refuse the whole rule and write nothing. +A served file under the legacy `.tests/` directory SHALL be treated as a fixture for this check, +like a file under `tests/`. A service that has not yet moved to the new layout still serves its +fixtures there, unsigned, and refusing them as unsigned files would refuse every rule it serves. + #### Scenario: A corrupted file is refused - **WHEN** a served file's content does not hash to its entry in `signatures` @@ -94,23 +98,37 @@ Any failure SHALL refuse the whole rule and write nothing. #### Scenario: An unsigned file is refused -- **WHEN** a served file outside `.tests/` has no entry in `signatures` +- **WHEN** a served file outside `tests/` has no entry in `signatures` - **THEN** the CLI SHALL refuse the rule #### Scenario: Fixtures need no signature -- **WHEN** a served rule carries files under `.tests/` that have no entry in `signatures` +- **WHEN** a served rule carries files under `tests/` that have no entry in `signatures` - **THEN** the CLI SHALL accept them +#### Scenario: Legacy fixtures need no signature either + +- **WHEN** a served rule carries files under `.tests/` that have no entry in `signatures` +- **THEN** the CLI SHALL accept them as fixtures + ### Requirement: A delivered rule replaces its directory After verification, the CLI SHALL write a served rule so that its directory holds exactly the served files: every file in the directory that the served set does not contain SHALL be removed, -including under `.tests/`, since a served set always carries the rule's fixtures. The CLI SHALL +including under `tests/`, since a served set always carries the rule's fixtures. The CLI SHALL create each served file's parent directories as needed, so a nested path such as -`captures/env.yml` or `.tests/fail/case.ts` is written wherever it lands. When a stale file cannot be removed, the CLI SHALL say that the served +`captures/env.yml` or `tests/fail/case.ts` is written wherever it lands. When a stale file cannot be removed, the CLI SHALL say that the served bytes were written and name each entry it could not remove. +A served set whose fixtures are in the old format, under `.tests/`, SHALL be migrated just in +time: converted the same way migration 12 converts a local rule, written in the `tests/` +layout, and never written under `.tests/`. Fixtures carry no signature, so the conversion +changes no signed byte, and the CLI never writes a layout its own `verify` does not read. This +is a permanent reading of the old format, not a transition step. The CLI SHALL NOT depend on +the service emitting the new layout, and a served set already in the new layout SHALL be +written unchanged. Fixtures the converter cannot place SHALL be written under +`tests/` as served, and the delivery SHALL say so, naming them. + #### Scenario: A local extra file does not survive - **WHEN** a rule directory holds `captures/extra.yml` and the served set does not @@ -118,8 +136,19 @@ bytes were written and name each entry it could not remove. #### Scenario: Nested paths are written -- **WHEN** a served set carries `.tests/fail/case.ts` and the rule directory does not exist -- **THEN** the CLI SHALL create `.tests/fail/` and write the file +- **WHEN** a served set carries `tests/fail/case.ts` and the rule directory does not exist +- **THEN** the CLI SHALL create `tests/fail/` and write the file + +#### Scenario: Legacy fixture paths are written in the new layout + +- **WHEN** a served `sg` rule carries its fixtures as `.tests/-test.yml` with `valid:` and `invalid:` snippets +- **THEN** the CLI SHALL write them as case directories under `tests/pass/` and `tests/fail/` +- **AND** the rule directory SHALL NOT contain `.tests/` afterwards + +#### Scenario: A served set in the new layout is written as served + +- **WHEN** a served set carries its fixtures under `tests/pass//` and `tests/fail//` +- **THEN** the CLI SHALL write them at those paths without conversion #### Scenario: A generated rule's revision is confirmed diff --git a/openspec/specs/cli-rule-fixtures/spec.md b/openspec/specs/cli-rule-fixtures/spec.md new file mode 100644 index 00000000..cb17ea08 --- /dev/null +++ b/openspec/specs/cli-rule-fixtures/spec.md @@ -0,0 +1,98 @@ +# cli-rule-fixtures Specification + +## Purpose + +One fixture contract for every rule engine: where a rule's test cases live, what a case is, and how one is judged. Verifying a rule runs the same check that scanning a repository runs, so a rule that passes its fixtures behaves the same way in `check`. + +## Requirements + +### Requirement: A rule's fixtures are case directories under tests/pass and tests/fail + +Every rule, whatever its engine, SHALL keep its fixtures at `tests/pass//` and `tests/fail//` inside its rule directory. A case SHALL be a directory, and its name SHALL be the case's description. A case MAY hold any number of files at any depth. + +Each bucket SHALL be read one level deep. An entry directly under `pass/` or `fail/` that is not a directory SHALL be an error naming the path, not an ignored entry. An entry directly under `tests/` other than `pass/` and `fail/` SHALL be an error naming the path. Buckets SHALL be read independently, and a bucket that exists but cannot be read SHALL be an error rather than an empty bucket. + +**Rationale.** One shape for every engine is what lets one loop verify every rule. A case is a directory rather than a document because the engines that scope by path judge a file by where it sits, so a case has to be able to place a file at the path the rule expects. A runtime rule's evidence also spans files. The strict reads keep a fixture that was written but never ran from passing in silence: a skipped loose file is a fixture nobody mentioned, and a swallowed permission error makes a two-sided rule look one-sided and a one-sided rule look complete. An engine-native test file left in `tests/`, such as an ast-grep test YAML, is refused for the same reason. It would otherwise sit beside the cases and never run. + +#### Scenario: A case is a directory named for what it shows + +- **WHEN** a rule holds `tests/fail/npx-latest/.github/workflows/ci.yml` +- **THEN** `npx-latest` SHALL be one `fail` case +- **AND** every file beneath it SHALL belong to that case + +#### Scenario: A loose file in a bucket is refused + +- **WHEN** `tests/pass/` contains a file directly rather than inside a case directory +- **THEN** verification SHALL report an error naming that path +- **AND** SHALL NOT skip it in silence + +#### Scenario: A stray entry in tests is refused + +- **WHEN** `tests/` contains an entry other than `pass/` and `fail/`, such as `-test.yml` +- **THEN** verification SHALL report an error naming that path + +#### Scenario: An unreadable bucket is not an empty bucket + +- **WHEN** a bucket exists but cannot be read +- **THEN** verification SHALL report the failure +- **AND** SHALL NOT treat the bucket as holding no cases + +### Requirement: A fixture case is judged by the engine's own check with the case as its root + +Verification SHALL judge each case by running the rule's engine the way `check` runs it, over the case directory as the project root: the same executable, the same walk, and the same rule config, narrowed to the rule under verification. A `fail/` case SHALL pass when it produces at least one finding for that rule. A `pass/` case SHALL pass when it produces none. Findings SHALL NOT be counted against line, column, message, or number. A rule's test is whether it fires and whether it stays quiet. + +The rule's scope SHALL apply: an ast-grep rule's `files:` and `ignores:`, and the matchers in a Vale rule's own `.vale.ini`. A fixture for a scoped rule therefore places its files at paths the scope accepts. + +Each case SHALL be judged in isolation from every other rule in the project. A rule the engine refuses elsewhere in the project SHALL NOT affect the verdict or the findings of the rule under verification. + +Verification SHALL write nothing to the rules tree. Whatever it writes to run the engine SHALL live in a temporary directory outside the project and SHALL be gone when the run ends. + +The findings a case produces SHALL be the findings `verify` reports for it, in the shape `check --json` prints, with `file` naming the fixture file as a project-relative path. + +**Rationale.** A fixture is evidence about the rule as it will run. Every earlier fixture path diverged from `check` in some way. ast-grep snippets were judged by `sg test` and scanned over stdin, which bypasses `files:`. Vale fixtures ran under a config that enabled the rule everywhere. In both, a rule whose scope never matched a real file passed verification and reported nothing in `check`, and no part of the system could see it. Running the check itself over a smaller root removes the divergence instead of documenting it. It also leaves one thing to keep working instead of two. + +#### Scenario: A scoped ast-grep rule's fixture sits at a path the scope accepts + +- **WHEN** an ast-grep rule declares `files: [".github/workflows/*.yml"]` and a `fail/` case holds the offending step at `.github/workflows/ci.yml` +- **THEN** the case SHALL produce a finding and pass + +#### Scenario: A fixture outside the rule's scope does not fire + +- **WHEN** the same rule's `fail/` case holds the offending step at `ci.yml` in the case root +- **THEN** the case SHALL produce no finding +- **AND** verification SHALL report the case as failing + +#### Scenario: A pass case that fires fails the rule + +- **WHEN** a `pass/` case produces a finding for the rule +- **THEN** verification SHALL fail the rule +- **AND** SHALL report the finding under the `pass` bucket, naming the fixture file + +#### Scenario: Another rule's error does not reach this rule + +- **WHEN** a rule is verified while a different rule of the same engine is one the engine refuses +- **THEN** the rule under verification SHALL be judged on its own cases alone + +#### Scenario: Verification leaves nothing behind + +- **WHEN** verification of any rule finishes, whether it passes or fails +- **THEN** nothing SHALL have been written under `.taskless/rules/` +- **AND** nothing SHALL remain that a later `check` could report against + +### Requirement: A quiet fail case is diagnosed against the rule's scope + +When a `fail/` case of an ast-grep or Vale rule produces no finding, verification SHALL run that case once more with the rule's scope removed. If that run produces a finding, the failure SHALL say that the rule's scope excluded the case and name the scope that did it. Otherwise it SHALL say the rule did not match. The diagnostic run SHALL NOT change the verdict. The case still fails. + +**Rationale.** Applying scope to fixtures adds a new way for a correct rule to fail: a fixture at a path the scope does not accept. "Did not fire" points at the pattern, and an author edits the wrong thing. One extra run separates the two causes, and it costs nothing on a passing rule. + +#### Scenario: A glob that excludes the fixture is named + +- **WHEN** a `fail/` case is quiet under the rule's scope and fires with the scope removed +- **THEN** the failure SHALL say the scope excluded the case +- **AND** SHALL name the `files:` globs or Vale matchers in effect + +#### Scenario: A pattern that never matches is reported as such + +- **WHEN** a `fail/` case is quiet both with and without the rule's scope +- **THEN** the failure SHALL say the rule did not match the case +- **AND** SHALL NOT blame the scope diff --git a/openspec/specs/cli-rule-format/spec.md b/openspec/specs/cli-rule-format/spec.md index 552c09f3..a4168ff7 100644 --- a/openspec/specs/cli-rule-format/spec.md +++ b/openspec/specs/cli-rule-format/spec.md @@ -57,17 +57,17 @@ When `taskless.json`'s `version` exceeds the highest migration the installed CLI ### Requirement: Rules are one directory each, partitioned by engine -The system SHALL store every rule as a directory at `.taskless/rules///`, holding the rule, any config that engine requires, and its tests under `.tests/`. +The system SHALL store every rule as a directory at `.taskless/rules///`, holding the rule, any config that engine requires, and its tests under `tests/`. -| Engine | Rule directory contents | -| --------- | --------------------------------------------------------- | -| `sg` | `.yml`, `.tests/-YYYYMMDD-test.yml` | -| `vale` | `.yml`, `.vale.ini`, `.tests/pass/*`, `.tests/fail/*` | -| `runtime` | `check.ts`, `captures/*.yml`, `.tests/…` | +| Engine | Rule directory contents | +| --------- | -------------------------------------------------------------------------- | +| `sg` | `.yml`, `tests/pass//…`, `tests/fail//…` | +| `vale` | `.yml`, `.vale.ini`, `tests/pass//…`, `tests/fail//…` | +| `runtime` | `check.ts`, `captures/*.yml`, `tests/pass//…`, `tests/fail//…` | One directory per rule is what lets a rule be addressed, reviewed, moved, or deleted as a single thing, and it is what makes `verify ` and `test ` possible without an id lookup. -Tests SHALL live in `.tests/`, dot-prefixed. This is not cosmetic: ast-grep's `ruleDirs` recurses and parses every `.yml` beneath it as a rule, so a plain `tests/` directory inside a rule directory fails the scan outright. Measured against ast-grep 0.41.0, a dot-directory is skipped by rule discovery while `sg test` still reads it when `testDir` names it. +Tests SHALL live in `tests/`, with no dot prefix, laid out as `cli-rule-fixtures` describes. The previous dot prefix worked around ast-grep's rule discovery: `ruleDirs` recurses and parses every `.yml` beneath it as a rule, and a plain `tests/` inside a rule directory fails the whole scan. Re-measured on 0.51.0, that is still true. So the engine is never pointed at the committed rules tree. It reads its rules from a rules-only tree the CLI assembles (see "Each engine's native config is the source of truth"), and what a rule directory may contain stops depending on undocumented engine behavior. A visible `tests/` is also where an author or an agent looks first. A dot directory is hidden in most listings, and fixtures written beside it were common enough to need a migration of their own (`0010`). #### Scenario: A rule is one path @@ -77,7 +77,7 @@ Tests SHALL live in `.tests/`, dot-prefixed. This is not cosmetic: ast-grep's `r #### Scenario: Test files are not mistaken for rules -- **WHEN** an ast-grep rule directory contains `.tests/` with test YAML in it +- **WHEN** an ast-grep rule directory contains `tests/` with `.yml` fixture files in it - **THEN** a scan SHALL complete without attempting to parse those files as rules #### Scenario: The engine is read from the path @@ -102,9 +102,10 @@ An engine SHALL NOT be given a per-rule config file it has nothing to put in. as #### Scenario: ast-grep config is assembled from the rule tree -- **WHEN** the CLI runs a check or a test -- **THEN** it assembles `sgconfig.yml` with `ruleDirs` covering the rules tree and `testConfigs` covering each rule's `.tests/` -- **AND** the assembled file SHALL be gitignored +- **WHEN** the CLI runs a check or verifies a rule +- **THEN** it assembles a rules-only tree holding each selected rule's `.yml` and nothing else from its directory +- **AND** it assembles `sgconfig.yml` with `ruleDirs` naming that tree +- **AND** both SHALL live in the run directory or be gitignored, and SHALL NOT be written under `.taskless/rules/` #### Scenario: No empty per-rule config is required @@ -129,7 +130,7 @@ The system SHALL place each Vale rule in its own directory `.taskless/rules/vale #### Scenario: A rule's tests are not loaded as styles -- **WHEN** a Vale rule directory contains `.tests/` +- **WHEN** a Vale rule directory contains `tests/` - **THEN** Vale SHALL NOT load anything under it as a rule ### Requirement: Runtime capture rules live in captures diff --git a/openspec/specs/cli-rule-reconciliation/spec.md b/openspec/specs/cli-rule-reconciliation/spec.md index cad10a30..96c8b2cf 100644 --- a/openspec/specs/cli-rule-reconciliation/spec.md +++ b/openspec/specs/cli-rule-reconciliation/spec.md @@ -18,7 +18,11 @@ any `key=value` parameters are parsed. Signatures SHALL be compared as whole str The algoVersion SHALL also determine **what one signature covers**. A signature at algoVersion `1` covers exactly one file. **Which** files of a rule are signed is set by the reconcile contract, not by the envelope: every file in the rule's directory except those -under `.tests/`, each with its own signature. +under `tests/` or the legacy `.tests/`, each with its own signature. + +`.tests/` stays excluded beside `tests/` because migration 12 leaves in place any fixture it +cannot convert, and a leftover fixture reported as a rule file would make an issued rule read as +edited over a file that was never signed. **Rationale.** Coverage per signature is a property of the signature scheme; the set of signed files is a property of the rule contract. Keeping them apart is what lets the rule @@ -55,7 +59,7 @@ because no single signature's meaning changed. - **WHEN** the CLI signs a rule directory for reconcile - **THEN** it SHALL compute one signature per file in the directory -- **AND** SHALL NOT sign any file under `.tests/` +- **AND** SHALL NOT sign any file under `tests/` or `.tests/` ### Requirement: Signature normalization procedure (algoVersion 1) @@ -184,7 +188,7 @@ When `check` reconciles, the CLI SHALL send `POST /cli/api/v2/reconcile` with `{ ruleId, files: [{ path, signature }] }` for **every** rule directory under `.taskless/rules//`, of every engine, whatever `--rule` selects to run. `ruleId` SHALL be the directory name. `files` SHALL list every regular file under the directory, -recursively, except files under `.tests/` and the operating-system metadata files +recursively, except files under `tests/` or the legacy `.tests/` and the operating-system metadata files `.DS_Store`, `Thumbs.db`, and `desktop.ini`. `path` SHALL be relative to the rule directory with `/` separators on every platform, and `signature` SHALL be the full algoVersion-1 envelope of the snapshot's bytes for that file. @@ -201,8 +205,14 @@ envelope of the snapshot's bytes for that file. #### Scenario: Fixtures are not reported -- **WHEN** a rule directory holds files under `.tests/` +- **WHEN** a rule directory holds files under `tests/` +- **THEN** no reported path SHALL begin with `tests/` + +#### Scenario: Unconverted legacy fixtures are not reported + +- **WHEN** a rule directory still holds files under `.tests/` that migration 12 could not convert - **THEN** no reported path SHALL begin with `.tests/` +- **AND** the rule SHALL NOT read as edited because of them #### Scenario: --rule does not narrow the report diff --git a/openspec/specs/cli-rule-validation/spec.md b/openspec/specs/cli-rule-validation/spec.md index 08843827..3bff754b 100644 --- a/openspec/specs/cli-rule-validation/spec.md +++ b/openspec/specs/cli-rule-validation/spec.md @@ -203,64 +203,13 @@ A notice SHALL be reported on a rule that passed as well as on one that failed, - **WHEN** `validate --json` reports a rule that drew no advisory - **THEN** that rule's `notices` SHALL be present and empty -### Requirement: Verify reports the findings an ast-grep rule's fixtures produced - -`verify` SHALL report, for an ast-grep rule whose fixtures produced matches, the findings those fixtures produced, in the same shape the other engines report theirs. A consumer SHALL NOT be able to tell which engine produced a finding except by its `source`. - -The rendered `message` is what this exists for. `ast-grep test` decides only whether each `invalid:` snippet fired and each `valid:` one stayed quiet, and a rule whose message interpolates its metavariables can have the slots in the wrong order while satisfying both — so the verdict cannot see the defect and the message is the only thing that can. - -The findings SHALL be gathered independently of the verdict and SHALL NOT change it. `ast-grep test` remains what decides whether the rule passed; a failure to gather findings SHALL be reported as a rule with no findings rather than as a rule that failed. - -A finding SHALL name the fixture FILE that declares the snippet, as a project-relative path, not the temporary or synthetic name any scanning mechanism used internally. Where the snippet is a literal block scalar, the position SHALL be the snippet's real position in that file, so the author can open it directly. - -#### Scenario: The rendered message is reported, not just the verdict - -- **WHEN** `verify --json` runs against an ast-grep rule whose `invalid:` snippet matched -- **THEN** the rule result SHALL carry a finding for that snippet -- **AND** the finding's `message` SHALL be the message as ast-grep rendered it, with the rule's metavariables already interpolated -- **AND** a rule whose message names the same metavariables in the other order SHALL produce a different `message`, so the two cannot both pass - -#### Scenario: A finding points back into the fixture file - -- **WHEN** an ast-grep fixture snippet produces a finding -- **THEN** the finding's `file` SHALL be the project-relative path of the test YAML declaring the snippet -- **AND** SHALL NOT be a temporary path or ast-grep's name for a stream -- **AND** where the snippet is a literal block scalar, the finding's line and column SHALL be its position in that file rather than its position within the snippet - -#### Scenario: Both buckets are reported, and named in ast-grep's vocabulary - -- **WHEN** `verify --json` reports an ast-grep rule's findings -- **THEN** a finding from an `invalid:` snippet SHALL carry the `fail` bucket -- **AND** a finding from a `valid:` snippet SHALL carry the `pass` bucket -- **AND** the `fail` bucket SHALL be reported even when the rule passed - -#### Scenario: A valid snippet that wrongly fired is printed without --json - -- **WHEN** `verify` runs without `--json` and an ast-grep rule fails because a `valid:` snippet fired -- **THEN** the offending findings SHALL be printed under that rule, labelled by bucket -- **AND** they SHALL be rendered the way `check` renders a finding -- **AND** a rule that passed SHALL still print one line and no findings - -#### Scenario: A language ast-grep cannot parse degrades to no findings - -- **WHEN** an ast-grep rule declares a `language:` the vendored binary does not recognise -- **THEN** `verify` SHALL report that rule as carrying an empty `findings` array -- **AND** SHALL NOT fail to produce a report -- **AND** the failure SHALL NOT suppress any other rule's findings - -#### Scenario: Collecting findings writes nothing to disk - -- **WHEN** `verify` collects an ast-grep rule's fixture findings, whether the rule passes or fails -- **THEN** no file SHALL be left behind in the project tree or the temporary directory -- **AND** nothing SHALL be written that a later `check` could report against - ### Requirement: Verify runs a rule's fixtures and runs validate first -`verify` SHALL execute a rule against its test material — ast-grep test cases, Vale `pass`/`fail` fixture buckets, or the runtime harness — and SHALL run `validate` first, stopping on a validate failure without running the fixtures. +`verify` SHALL execute a rule against its fixture cases, laid out and judged as `cli-rule-fixtures` describes for every engine, and SHALL run `validate` first, stopping on a validate failure without running the fixtures. Ordering is the point. When a rule is both malformed and under-fixtured, the fixture complaint is the less useful of the two errors and is the one that surfaces first if the checks run in the other order — so the author is told their fixtures are incomplete while the reason the rule could never have run goes unmentioned. -A rule that populates only one bucket has proved only half of what a rule claims, whatever its engine. An engine SHALL NOT be trusted to report this itself: `ast-grep test` reports an empty `invalid:` bucket as `1 passed; 0 failed` and exits zero, so a rule that has never matched anything is indistinguishable from one that passed. +A rule that populates only one bucket has proved only half of what a rule claims, whatever its engine. An engine SHALL NOT be trusted to report this itself: `ast-grep test` reported an empty `invalid:` bucket as `1 passed; 0 failed` and exited zero, so a rule that had never matched anything was indistinguishable from one that passed. Coverage is therefore counted by the CLI over case directories, never read from an engine's summary. A runtime rule's fixtures execute code, so they SHALL run only when `--dangerously-run-scripts` is passed, and under no other mechanism. `verify` SHALL NOT consult the rule service to decide this, and SHALL make no network request in the course of running fixtures. @@ -293,23 +242,23 @@ The human rendering SHALL stay a single line per passing rule. Under a FAILING r #### Scenario: Vale fixtures are tested per bucket - **WHEN** `verify` runs against a Vale rule -- **THEN** every `fail/` document SHALL produce at least one finding for that rule -- **AND** every `pass/` document SHALL produce none +- **THEN** every case directory under `tests/fail/` SHALL produce at least one finding for that rule +- **AND** every case directory under `tests/pass/` SHALL produce none - **AND** a rule populating only one bucket SHALL be reported as unverified rather than passing #### Scenario: ast-grep fixtures are counted per bucket - **WHEN** `verify` runs against an ast-grep rule -- **THEN** the `valid:` and `invalid:` entries SHALL be counted across every `-test.yml` file the rule owns +- **THEN** every case directory under `tests/fail/` SHALL produce at least one finding for that rule +- **AND** every case directory under `tests/pass/` SHALL produce none - **AND** a rule populating only one bucket SHALL be reported as unverified rather than passing - **AND** a rule whose buckets are all empty or absent SHALL be reported as unverified rather than passing -- **AND** a green `ast-grep test` run SHALL NOT on its own be sufficient to report the rule as passing #### Scenario: Runtime fixtures are run per case - **WHEN** `verify` runs against a runtime rule with `--dangerously-run-scripts` -- **THEN** each directory under `.tests/fail/` SHALL be passed to the check as its `root` and SHALL produce at least one finding -- **AND** each directory under `.tests/pass/` SHALL be passed as its `root` and SHALL produce none +- **THEN** each directory under `tests/fail/` SHALL be passed to the check as its `root` and SHALL produce at least one finding +- **AND** each directory under `tests/pass/` SHALL be passed as its `root` and SHALL produce none - **AND** a rule populating only one bucket SHALL be reported as unverified rather than passing - **AND** a rule holding no fixture cases at all SHALL be reported as unverified rather than passing diff --git a/openspec/specs/cli-rules/spec.md b/openspec/specs/cli-rules/spec.md index 02b2ab32..a3f09993 100644 --- a/openspec/specs/cli-rules/spec.md +++ b/openspec/specs/cli-rules/spec.md @@ -118,12 +118,12 @@ response's `error` as given. ### Requirement: Rules create writes test files to disk -`taskless rule create` SHALL write generated test files into the rule's own directory, at `.taskless/rules/sg//.tests/`, regardless of whether `--anonymous` was set. (Renamed; strengthened; repathed for the rule-directory layout.) +`taskless rule create` SHALL write generated test files into the rule's own directory, at `.taskless/rules/sg//tests/`, regardless of whether `--anonymous` was set. (Renamed; strengthened; repathed for the rule-directory layout.) #### Scenario: Tests land inside the rule they cover - **WHEN** `taskless rule create` generates test cases for rule `` -- **THEN** the CLI SHALL write them under `.taskless/rules/sg//.tests/` +- **THEN** the CLI SHALL write them under `.taskless/rules/sg//tests/` - **AND** it SHALL do so whether or not `--anonymous` was set ### Requirement: Rules create outputs results @@ -319,7 +319,7 @@ A rule id does not carry its engine, so the CLI SHALL **resolve** which engine d #### Scenario: Deleting a rule removes its whole directory - **WHEN** a user runs `taskless rule delete no-eval` -- **THEN** the CLI SHALL remove the rule's directory including its `.tests/` +- **THEN** the CLI SHALL remove the rule's directory including its `tests/` - **AND** no file belonging to that rule SHALL remain #### Scenario: Deleting a rule filed under any engine diff --git a/openspec/specs/cli-runtime-rule-execution/spec.md b/openspec/specs/cli-runtime-rule-execution/spec.md index 269004e0..d3a140a8 100644 --- a/openspec/specs/cli-runtime-rule-execution/spec.md +++ b/openspec/specs/cli-runtime-rule-execution/spec.md @@ -22,7 +22,7 @@ would be reported as a clean pass. The refusal SHALL be per capture file, not pe unimplemented mode does not remove a rule's other narrows. Rule files under `.taskless/rules/sg/` SHALL continue to be treated as static ast-grep rules, -not runtime rules. A rule's `.tests/` directory holds verification fixtures and SHALL NOT be +not runtime rules. A rule's `tests/` directory holds verification fixtures and SHALL NOT be executed by `check`. #### Scenario: A runtime directory entry is a runtime rule @@ -184,7 +184,7 @@ bounded to the rule root would let any nested directory carry unsigned code past `check.ts` still matched its blessed signature. The search SHALL also consider only files, so a directory whose name ends in a module extension is not reported as one. -The rule's `.tests/` directory SHALL be excluded from this search. A runtime check reads real +The rule's `tests/` directory SHALL be excluded from this search. A runtime check reads real files under a root, so a fixture that is itself TypeScript is the normal case rather than a smuggled helper, and refusing it would break correct rules — a worse failure than the one being prevented. @@ -197,7 +197,7 @@ prevented. #### Scenario: A TypeScript test fixture is not a stray module -- **WHEN** a runtime rule carries a `.ts` file under its `.tests/` directory +- **WHEN** a runtime rule carries a `.ts` file under its `tests/` directory - **THEN** the CLI SHALL still discover and run the rule ### Requirement: Declared versions are read @@ -216,7 +216,7 @@ executed against the current one. ### Requirement: A runtime rule's fixtures are executed through the harness The CLI SHALL execute a runtime rule against the fixture cases it holds under -`.tests/pass/` and `.tests/fail/`. A case SHALL be a **directory**, and that +`tests/pass/` and `tests/fail/`. A case SHALL be a **directory**, and that directory SHALL be the `root` the harness passes to the check. Each bucket SHALL be read one level deep. An entry that is not a directory SHALL diff --git a/openspec/specs/cli-taskless-bootstrap/spec.md b/openspec/specs/cli-taskless-bootstrap/spec.md index df1469e7..9a9d9800 100644 --- a/openspec/specs/cli-taskless-bootstrap/spec.md +++ b/openspec/specs/cli-taskless-bootstrap/spec.md @@ -465,3 +465,51 @@ For a build that installs a launcher, `.taskless/README.md` SHALL list that pack - **WHEN** migration `"1"` runs in a `nightly` build - **THEN** the first usage example SHALL be `./.taskless/bin/cli-nightly.mjs check` - **AND** the README SHALL NOT name `./.taskless/bin/cli.mjs` + +### Requirement: Migration 12 moves every rule's fixtures into tests case directories + +Migration `12` SHALL convert each rule's fixtures from `.tests/` to the `tests/pass//` and `tests/fail//` layout, for every engine: + +| Engine | From | To | +| --------- | ------------------------------------------------------------------- | ----------------------------------------------------------------------------------------- | +| `sg` | each `valid:` / `invalid:` snippet of each `.tests/*.yml` test file | one case per snippet under `tests/pass/` / `tests/fail/`, holding the snippet as one file | +| `vale` | each document `.tests//` | `tests///` | +| `runtime` | `.tests///…` | `tests///…`, unchanged | + +An ast-grep snippet's file extension SHALL come from a fixed table keyed on the rule's `language:`, covering every language the vendored ast-grep supports. A converted file's path within its case SHALL be one the rule's scope accepts (`files:` and `ignores:` for ast-grep, the enabling matchers for Vale), derived from the scope's own patterns and checked against them. A converted case SHALL be named by bucket and ordinal in source order, since a snippet carries no description to name it by. + +The migration SHALL NOT delete what it did not convert. A test file it cannot parse, a snippet whose language has no table entry, or a scope it cannot derive an accepted path from SHALL be left in place under `.tests/`. The migration SHALL name each one, and SHALL remove `.tests/` only once it is empty. A destination that already exists SHALL keep its bytes. The migration SHALL be idempotent. + +It SHALL NOT modify any shipped migration. Migrations `5`, `9` and `10` still move fixtures into `.tests/`, and `12` then moves them on, so a fresh scaffold and an old one end in the same state by running `1` through `12` in order. + +**Rationale.** A layout is what the scaffold version records, so this is a scaffold migration and not a rule-content migration. Fixtures carry no signature, so moving or rewriting them changes no signed byte, and the migration is as safe for an issued rule as for a hand-written one. Deriving a path the scope accepts is what keeps a converted fixture meaningful once scope applies. A snippet that used to pass only because stdin bypassed `files:` would otherwise become a quiet `fail/` case the moment it was migrated. + +#### Scenario: ast-grep snippets become case directories + +- **WHEN** `taskless.json` records version 11 and `.taskless/rules/sg/no-eval/.tests/no-eval-test.yml` holds two `valid:` and one `invalid:` snippet for a `language: TypeScript` rule +- **THEN** migration 12 SHALL run +- **AND** `tests/pass/` SHALL hold two cases and `tests/fail/` one, each with one `.ts` file holding its snippet +- **AND** `.tests/` SHALL NOT exist afterwards +- **AND** `taskless.json` SHALL record version 12 + +#### Scenario: A scoped rule's converted fixture sits where its scope looks + +- **WHEN** an ast-grep rule declares `files: [".github/workflows/*.yml"]` and its test YAML holds an `invalid:` snippet +- **THEN** the converted case SHALL hold that snippet at a path matching `.github/workflows/*.yml` + +#### Scenario: What cannot be converted stays and is named + +- **WHEN** a rule's scope admits no path the migration can derive and check +- **THEN** that rule's fixtures SHALL remain under `.tests/` +- **AND** the migration SHALL name the rule and why +- **AND** every other rule SHALL still be migrated + +#### Scenario: Runtime cases are renamed only + +- **WHEN** a runtime rule holds `.tests/fail/two-files/a.ts` and `.tests/fail/two-files/b.ts` +- **THEN** after migration 12 they SHALL be at `tests/fail/two-files/a.ts` and `tests/fail/two-files/b.ts` with identical bytes + +#### Scenario: Migration 12 is idempotent + +- **WHEN** migration 12 runs on a project it has already migrated +- **THEN** no file SHALL change diff --git a/openspec/specs/cli-vale-rule-engine/spec.md b/openspec/specs/cli-vale-rule-engine/spec.md index 37708e3c..afdd7eab 100644 --- a/openspec/specs/cli-vale-rule-engine/spec.md +++ b/openspec/specs/cli-vale-rule-engine/spec.md @@ -44,16 +44,24 @@ When the `vale` binary cannot be found or invoked, the system SHALL report that ### Requirement: Vale rules are verified with per-rule fixture subdirectories -The system SHALL verify a Vale rule from a `.taskless/rules/vale//.tests/` subdirectory containing `pass/` and `fail/` fixture documents. Because verification isolates one rule, the system SHALL generate an ephemeral config enabling only that rule — derived from the rule's own config so that verification exercises the scope the rule actually declares. Verification SHALL assert that every `fail/` fixture produces at least one finding for the rule and every `pass/` fixture produces none (mirroring ast-grep's `invalid`/`valid`). +The system SHALL verify a Vale rule from the case directories under `.taskless/rules/vale//tests/pass/` and `tests/fail/`, as `cli-rule-fixtures` describes. Because verification isolates one rule, the system SHALL generate an ephemeral config assembled from that rule's own `.vale.ini` alone, so verification exercises the scope the rule actually declares. Verification SHALL assert that every `fail/` case produces at least one finding for the rule and every `pass/` case produces none. + +The config SHALL be the rule's own matchers, not a blanket `[*]` enabling the rule everywhere. This requirement always said verification exercises the declared scope, and the implementation did not: it enabled the rule under `[*]`, so a rule whose matchers matched nothing real passed verification and stayed silent in `check`. The blanket config is still the right tool for one thing, the unscoped re-run that diagnoses a quiet `fail/` case. #### Scenario: Verification isolates the rule under test - **WHEN** verify runs for a rule and generates a config enabling only that rule - **THEN** findings from other rules SHALL NOT affect its result +#### Scenario: Verification applies the rule's own matchers + +- **WHEN** a rule's `.vale.ini` enables it only under `[docs/**/*.md]`, and a `fail/` case holds the offending text at `notes.md` +- **THEN** that case SHALL produce no finding and verification SHALL fail +- **AND** the same text at `docs/guide/notes.md` in the case SHALL produce a finding + #### Scenario: Verification fails when a fail fixture does not trigger -- **WHEN** a `fail/` fixture for a rule produces no finding +- **WHEN** a `fail/` case for a rule produces no finding - **THEN** verification reports a failure for that rule #### Scenario: A one-sided fixture set is not verified @@ -190,7 +198,7 @@ A rule's config SHALL NOT carry `BasedOnStyles`, with any value. Measured agains ### Requirement: A Vale rule is a self-contained directory -The system SHALL store a Vale rule as a directory `.taskless/rules/vale//` containing its style file `.yml`, its own `.vale.ini`, and its fixtures under `.tests/`. No file outside that directory SHALL be required to define or verify the rule. +The system SHALL store a Vale rule as a directory `.taskless/rules/vale//` containing its style file `.yml`, its own `.vale.ini`, and its fixtures under `tests/`. No file outside that directory SHALL be required to define or verify the rule. Self-containment is what removes the engine's silent-failure class. A rule can be added, reviewed, moved, or deleted as one directory, and no two authors write the same file. diff --git a/packages/cli/src/agent/create-remote-rule.md b/packages/cli/src/agent/create-remote-rule.md index 088ee2fb..aaf192f7 100644 --- a/packages/cli/src/agent/create-remote-rule.md +++ b/packages/cli/src/agent/create-remote-rule.md @@ -1,4 +1,4 @@ -# Topic: create-remote-rule (CLI v%(CLI_VERSION)s / topic v8) +# Topic: create-remote-rule (CLI v%(CLI_VERSION)s / topic v9) %(ONE_COMMAND_PER_CALL)s @@ -151,7 +151,7 @@ Two ways to legitimately be here: **Read `notices` if it is present.** It is an optional array of advisory messages about a delivery that was written anyway. A rule - arriving with no `.tests/` fixtures is the one to expect. That rule works, but + arriving with no `tests/` fixtures is the one to expect. That rule works, but nothing exercises it: `verify` reports a pass over zero cases, which reads exactly like a rule proven to work. Surface any notice to the user rather than reporting an unqualified success. Under `--json` diff --git a/packages/cli/src/agent/create-runtime-rule.md b/packages/cli/src/agent/create-runtime-rule.md index 73834a15..5f60cbc7 100644 --- a/packages/cli/src/agent/create-runtime-rule.md +++ b/packages/cli/src/agent/create-runtime-rule.md @@ -1,4 +1,4 @@ -# Topic: create-runtime-rule (CLI v%(CLI_VERSION)s / topic v6) +# Topic: create-runtime-rule (CLI v%(CLI_VERSION)s / topic v7) %(ONE_COMMAND_PER_CALL)s @@ -33,22 +33,13 @@ kinds of file: |------------------|---------------------------------------------------------------------------| | `captures/*.yml` | ast-grep capture rules that narrow which files the check looks at | | `check.ts` | a module whose default export receives those matches and returns findings | -| `.tests/` | fixtures, in `pass/` and `fail/` buckets | +| `tests/` | fixtures, one directory per case under `pass/` and `fail/` | -A fixture case is a DIRECTORY, not a file: `.tests/pass/case-1/`, and its -path is the `root` the harness hands the check, with every `match.file` -relative to it. So a case may hold as many files as the case needs. - -That is not a detail. A runtime rule exists because its evidence spans -more than one file, so a layout allowing one file per case could not -express the rules the tier is for. `pass/case-1/src/a.ts` alongside -`pass/case-1/src/b.ts` is the ordinary shape, not the exotic one. - -`pass`/`fail` rather than `valid`/`invalid`, matching Vale's buckets. -`valid`/`invalid` are key names inside an ast-grep test YAML and are -right for `sg` for one reason only: that file is handed to `sg test` -verbatim. Nothing hands a runtime fixture to ast-grep, so borrowing the -word here would make it mean two things. +A case directory is the `root` the harness hands the check, with every +`match.file` relative to it. That matters more here than for any other +engine: a runtime rule exists because its evidence spans more than one +file, so `pass/declared/src/config.ts` alongside `pass/declared/.env` is +the ordinary shape, not the exotic one. `validate` fails a runtime rule with no capture rule in `captures/`, because `check.ts` would then never be invoked. @@ -133,6 +124,8 @@ not run, names the flag, and exits 0. That is neither a pass nor a failure, and it is deliberately not a failure: nothing the holder of an unblessed rule can do would make a failure green. +%(FIXTURE_LAYOUT)s + %(FIXTURE_SOURCES)s Both buckets have to be populated for the rule to pass, and every case diff --git a/packages/cli/src/agent/create-sg-rule.md b/packages/cli/src/agent/create-sg-rule.md index be01790d..1c6a84d2 100644 --- a/packages/cli/src/agent/create-sg-rule.md +++ b/packages/cli/src/agent/create-sg-rule.md @@ -1,4 +1,4 @@ -# Topic: create-sg-rule (CLI v%(CLI_VERSION)s / topic v11) +# Topic: create-sg-rule (CLI v%(CLI_VERSION)s / topic v12) %(ONE_COMMAND_PER_CALL)s @@ -9,7 +9,7 @@ If that is not the kind of check you need, re-run `%(TASKLESS_CLI)s agent route` and follow its decision rather than adapting this recipe. ## Goal -Produce a verified ast-grep rule and its test file, locally, without +Produce a verified ast-grep rule and its test cases, locally, without contacting the Taskless service. You derive the rule yourself, write it in the canonical on-disk shape, and check it with `validate` and `verify` in a feedback loop. The files you produce match exactly what the @@ -30,16 +30,11 @@ two rules never collide. ``` .taskless/rules/sg/no-eval/ - no-eval.yml the rule - .tests/no-eval-YYYYMMDD-test.yml its test cases + no-eval.yml the rule + tests/fail/eval-of-payload/handler.ts a case it must fire on + tests/pass/json-parse/handler.ts a case it must stay quiet on ``` -**The leading dot on `.tests/` is required.** ast-grep walks the rules -tree and parses every `.yml` it reaches as a rule, so a plain `tests/` -directory makes it read your test file as a rule and fail the scan for -the whole project. A dot-directory is skipped by that walk, and the -test runner still finds it. Do not rename it. - Unlike Vale, an `sg` rule needs no second config file. It carries its own `files` and `ignores` inside the rule, so the rule file is the whole rule. @@ -104,7 +99,7 @@ whole rule. 5. **Check the rule against Quirks.** Read the **Quirks** section below against what you just wrote, before writing any tests. Most entries there get past `validate` and then either fail the whole scan or match - less than they should, and a test file written from the same + less than they should, and a test case written from the same assumption passes anyway. The table says which ones `validate` does catch. Look again at: - a `$$$` next to a comma (the separator trap), @@ -113,28 +108,31 @@ whole rule. `await f(...)`, - a `follows` or `precedes`, or a `not` that reuses a metavariable. -6. **Write the tests.** Write - `.taskless/rules/sg//.tests/-YYYYMMDD-test.yml` with the matching - `id` field plus `valid` and `invalid` arrays, at least two of each. - The `id` must match the rule's `id` so ast-grep test filtering pairs - them. These paths and this shape are the same ones the service - writes; do not invent a different layout. +6. **Write the tests.** Write at least two cases in each bucket under + `.taskless/rules/sg//tests/`. These paths are the same ones the + service writes; do not invent a different layout. + + %(FIXTURE_LAYOUT)s + + Each case file takes the extension ast-grep parses as the rule's + `language:` (`.ts` for `TypeScript`, `.tsx` for `Tsx`, `.yml` for + `Yaml`), and sits at a path the rule's `files:` admits and its + `ignores:` does not. %(FIXTURE_SOURCES)s - **Both arrays must be non-empty, and `verify` fails the rule if either - one is.** An empty `invalid:` is not a neutral starting point to fill - in later: ast-grep reports `1 passed; 0 failed` and exits zero over - no cases at all, so a rule that matches nothing anywhere looks - exactly like a rule that works. The `invalid:` bucket is the only - thing that demonstrates the rule can fire. + **Both buckets must hold a case, and `verify` fails the rule if either + one is empty.** An empty `fail/` is not a neutral starting point to + fill in later: it is the only thing that demonstrates the rule can + fire, and a rule that matches nothing anywhere looks exactly like a + rule that works without it. Where the rule has an arity boundary (the separator trap under Quirks), put a case on each side of it. A pattern that starts at two arguments when it was meant to start at one passes a test suite whose fixtures all have two. Where the rule keys on a callee that the repository awaits with a type argument (`await f(...)`), put that - form among the `invalid:` cases. Write it only for such a callee: an + form among the `fail/` cases. Write it only for such a callee: an awaited generic call to a function the repository never awaits is not code it would accept, and the case would teach the wrong shape. @@ -145,8 +143,8 @@ whole rule. %(TASKLESS_CLI)s verify .taskless/rules/sg/ --json ``` `validate` asks whether the rule is well-formed: the YAML matches the - ast-grep schema and every Taskless-required field is present. It - needs no test file, so run it the moment the rule exists. `verify` runs + ast-grep schema and every Taskless-required field is present. Run it + the moment the rule exists. `verify` runs the cases, after running `validate` and stopping if that fails, so a malformed rule reports the malformation rather than a test complaint. @@ -163,6 +161,7 @@ whole rule. | the YAML doesn't match the schema | check field types against the upstream schema | | a Taskless-required field is missing | add `id`/`language`/`severity`/`message`/`rule` | | a case didn't behave as expected | fix the rule pattern OR the test case | +| the rule's scope excluded a case | move the fixture to a path `files:` admits | A `regex` without an accompanying `kind` fails validation, the two always travel together. diff --git a/packages/cli/src/agent/create-vale-rule.md b/packages/cli/src/agent/create-vale-rule.md index 7de5a844..f86f05c0 100644 --- a/packages/cli/src/agent/create-vale-rule.md +++ b/packages/cli/src/agent/create-vale-rule.md @@ -1,4 +1,4 @@ -# Topic: create-vale-rule (CLI v%(CLI_VERSION)s / topic v18) +# Topic: create-vale-rule (CLI v%(CLI_VERSION)s / topic v19) %(ONE_COMMAND_PER_CALL)s @@ -26,10 +26,10 @@ touched either way, so two agents authoring two rules never collide. ``` .taskless/rules/vale/no-simply/ - no-simply.yml the style: what the rule looks for - .vale.ini the scope: which files it applies to - .tests/fail/bad.md prose it must flag - .tests/pass/ok.md prose it must leave alone + no-simply.yml the style: what the rule looks for + .vale.ini the scope: which files it applies to + tests/fail/simply-in-setup/README.md prose it must flag + tests/pass/simple-noun/README.md prose it must leave alone ``` The rule is **incomplete until the style and the config both exist**, @@ -845,25 +845,15 @@ it. `text.class.` tail, because the set of component names is the author's, not Vale's. -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 - `verify` rejects by name rather than skipping. Keep both one level - deep: +5. **Write the fixtures.** Cases go under + `.taskless/rules/vale//tests/`, one directory per case. - ``` - .taskless/rules/vale//.tests/pass/ok.md # rule must stay quiet - .taskless/rules/vale//.tests/fail/bad.md # rule must fire - ``` + %(FIXTURE_LAYOUT)s - **The leading dot on `.tests/` is required.** ast-grep walks the - rules tree and parses every `.yml` it reaches as a rule, and a plain - `tests/` directory fails that scan for the whole project. A - dot-directory is skipped. Do not rename it. - - Give the fixtures an extension your matcher's glob matches. A `.txt` - fixture under a `[*.md]` matcher is never linted, so the `fail/` - document silently passes. + For a Vale rule, "a path the scope admits" means a path one of the + rule's own `.vale.ini` matchers enables it for. A rule enabled under + `[docs/**/*.md]` reads `tests/fail//docs/guide.md`; a `.txt` + fixture under a `[*.md]` matcher is never linted. %(FIXTURE_SOURCES)s @@ -897,14 +887,13 @@ it. three. If the fixture fires on the prose line and not on the other two, the answer is `scope: raw`. See step 2 for what that costs. - **Fixtures run under a config that isolates this rule, so a green - `verify` is not evidence the rule reaches any real file.** `verify` - generates its own `.vale.ini` pointing at the fixture directory and - enabling only `.`; your rule's own matcher globs are not - consulted. So a glob of `packages/cli/src/**/*.ts` that matches - nothing in the repository still produces a rule that validates, passes - `verify`, and reports forever. The only check for that is a real - `check` over a real file: + **A green `verify` shows your matchers admit the fixture's path, not + that any real file sits at such a path.** Each case is linted under + the rule's own matchers, so a glob that misses the fixture fails + `verify`. A glob of `packages/cli/src/**/*.ts` in a repository whose + source lives elsewhere still admits a fixture written to match it, and + then reports nothing forever. The check for that is a real `check` + over a real file: ``` %(TASKLESS_CLI)s check --json @@ -1002,40 +991,12 @@ it. reported one entry per rule. `.taskless/rules/vale` covers every Vale rule; no argument at all covers the project. - **`verify` answers pass-or-fail and never shows you the finding**, so - it cannot tell you that a `substitution` message renders its two - `%%s` slots in the wrong order: the rule fires, the fixture is - satisfied, and `verify` reports `ok`. To read the rendered message, - the line numbers and the matched text, name the bucket to `check`: - - ``` - %(TASKLESS_CLI)s check .taskless/rules/vale//.tests/fail --json - ``` - - **That works because a path you name is honored.** `.taskless/` is - excluded from the *whole-project* walk only, so a bare `check` over - the project reports nothing from anyone's fixtures while the command - above reports every finding in that bucket. Measured on this build: - a whole-project `check` returned no result under `.taskless/`, and - the same rule's `fail/` bucket named explicitly returned its - findings with the message text rendered. - - **If that command returns `results: []` for a rule whose `verify` is - green, suspect the rule's own config before the pattern.** A - `[.taskless/**]` matcher setting `. = NO` turns the bucket - off for exactly this invocation, which is the one shape that - reproduces "`verify` says the fixture fired, `check` on the same - fixture says nothing". Measured: adding that matcher to a working - rule left `verify` at `ok: true` and emptied `results`. The tell is - in the same envelope, as a notice reading `matcher [.taskless/**] - is unnecessary`. `validate` reports it too, as an advisory. Delete - the matcher; step 4 explains why no rule needs one. - - Read `results` there. Ignore `success` and the exit code: `success` - says the run worked rather than that the fixture behaved, and the - exit code follows severity, so a `level: error` rule exits 1 on - `fail/` while a `warning` rule exits 0 and both are correct. `verify` - answers pass-or-fail; `check` shows you the finding. + **`verify --json` shows you every finding the cases produced,** each + with its rendered message, line, matched text and bucket, the + `fail/` findings included on a rule that passed. That is how to catch + a `substitution` message that renders its two `%%s` slots in the + wrong order: the rule fires, the case passes, and only the message + says something is off. Read `findings` in the rule's entry. **An empty `results` is a clean pass only when nothing else in the envelope says otherwise.** Two things can leave a file unchecked @@ -1058,8 +1019,8 @@ it. Every other file's findings are reported normally, and no rule of yours ran over this one. If it is a fixture, `verify` shows the same - gap from the other side: a `fail/` fixture Vale cannot parse - reports as `fail fixture did not fire`, and a `pass/` fixture it + gap from the other side: a `fail/` case Vale cannot parse + reports as `fail case did not fire`, and a `pass/` case it cannot parse stays green, since a rule that never ran cannot fire. Fix the front matter rather than the pattern. @@ -1092,22 +1053,20 @@ it. with the rule set to `NO`, meant to keep a bare run quiet over fixtures that hold violations on purpose. It does not stay confined to the invocation it was written for: a whole-project `check` skips - `.taskless/` without its help, and on a fixture bucket you name it - is the one thing acting, which is how it empties the `check` above - while `verify` stays green. `validate` reports it as an advisory (step 4 + `.taskless/` without its help, and on a path under `.taskless/` you + name it is the one thing acting. `validate` reports it as an advisory (step 4 lists it). A rule needs the matchers for the files it is about and no more. When a `fail/` document does not fire, work down this list before touching the pattern. The cause is usually further up: - - Read the finding first, with the `check` on the `fail/` bucket - above. What the run saw is cheaper than any guess about why it - saw nothing, and it separates "no finding" from "a finding whose - message is wrong". + - Read what `verify` said first. "The rule's scope excluded it" + means the case fires with the scope removed, so the matchers and + the fixture's path disagree and the pattern is fine. - Does the rule have a `.vale.ini` at all? - Is the assignment underneath a `[…]` matcher? - Is it spelled `.`, both halves the same? - - Does the matcher's glob match the fixture's extension? + - Does a matcher's glob match the fixture's path inside its case? - Only then: does the pattern actually match the text? A `pass/` document that fires means the pattern is too broad. Look diff --git a/packages/cli/src/agent/delete-rule.md b/packages/cli/src/agent/delete-rule.md index a1d127da..4908b1d8 100644 --- a/packages/cli/src/agent/delete-rule.md +++ b/packages/cli/src/agent/delete-rule.md @@ -1,9 +1,9 @@ -# Topic: delete-rule (CLI v%(CLI_VERSION)s / topic v5) +# Topic: delete-rule (CLI v%(CLI_VERSION)s / topic v6) %(ONE_COMMAND_PER_CALL)s ## Goal -Remove a rule and its associated test files from `.taskless/`. Does not +Remove a rule and its test cases from `.taskless/`. Does not contact the Taskless API; purely a local filesystem operation. `rule delete` takes a bare id, not a path, and resolves which engine @@ -34,7 +34,7 @@ picking one. The CLI removes: - `.taskless/rules///`: the whole directory, which holds - the rule, its `.tests/`, and any per-rule config + the rule, its `tests/`, and any per-rule config - `.taskless/rule-metadata/.yml`, a sidecar the CLI never writes. The removal is there for a directory a person created by hand; expect it to be absent. diff --git a/packages/cli/src/agent/improve-rule.anonymous.md b/packages/cli/src/agent/improve-rule.anonymous.md index 55cec21c..bfd5b7ea 100644 --- a/packages/cli/src/agent/improve-rule.anonymous.md +++ b/packages/cli/src/agent/improve-rule.anonymous.md @@ -1,4 +1,4 @@ -# Topic: improve-rule (anonymous) (CLI v%(CLI_VERSION)s / topic v5) +# Topic: improve-rule (anonymous) (CLI v%(CLI_VERSION)s / topic v6) %(ONE_COMMAND_PER_CALL)s @@ -16,8 +16,7 @@ check it with `validate` and `verify` in a feedback loop. 1. **Identify the target rule.** If the user named one, use it. Otherwise, list `.taskless/rules/sg/` and ask which one. Read the - rule file and any test file under - `.taskless/rules/sg//.tests/`. + rule file and its cases under `.taskless/rules/sg//tests/`. 2. **Gather improvement guidance.** Ask: - Are there false positives (cases currently flagged but @@ -38,9 +37,12 @@ check it with `validate` and `verify` in a feedback loop. `language`, and `severity` unless the user explicitly asked to change them. -5. **Update test cases.** Add the new false-positive examples to the - `valid:` list and the new false-negative examples to the - `invalid:` list in `.taskless/rules/sg//.tests/-*.yml`. +5. **Update test cases.** Add each new false-positive example as a case + under `.taskless/rules/sg//tests/pass/` and each new + false-negative example as a case under `tests/fail/`, one directory + per case, named for what it shows. + + %(FIXTURE_LAYOUT)s %(FIXTURE_SOURCES)s @@ -62,7 +64,7 @@ check it with `validate` and `verify` in a feedback loop. 7. **Report results.** Show: - The updated rule file path - - The updated test file path + - The cases added or changed under `tests/` - A diff-style summary of what changed Suggest fetching `%(TASKLESS_CLI)s agent check` to validate against the broader codebase. diff --git a/packages/cli/src/agent/improve-rule.md b/packages/cli/src/agent/improve-rule.md index 5808353a..273b8c6c 100644 --- a/packages/cli/src/agent/improve-rule.md +++ b/packages/cli/src/agent/improve-rule.md @@ -1,4 +1,4 @@ -# Topic: improve-rule (CLI v%(CLI_VERSION)s / topic v10) +# Topic: improve-rule (CLI v%(CLI_VERSION)s / topic v11) %(ONE_COMMAND_PER_CALL)s @@ -90,21 +90,21 @@ If the user wants the local-only flow (no API call), fetch 9. **Clean up.** Delete `.taskless/.tmp-improve-request.json` regardless of success or failure. -10. **Report results.** The CLI overwrites the rule file (and its - test file) with the updated version. Show the file paths and a +10. **Report results.** The CLI overwrites the rule directory, its + `tests/` included, with the updated version. Show the file paths and a summary of what changed. Suggest fetching `%(TASKLESS_CLI)s agent check` to validate. **Read `notices` if it is present.** It is an optional array of advisory messages about a delivery that was written anyway. A rule - arriving with no `.tests/` fixtures is the one to expect. Surface any notice + arriving with no `tests/` fixtures is the one to expect. Surface any notice rather than reporting an unqualified success. Under `--json` these do not go to stderr, so the envelope is the only place they appear. - **Confirm both fixture buckets are still non-empty.** `verify` fails - an ast-grep rule whose `valid:` or `invalid:` array is empty, and - narrowing a pattern to kill a false positive is exactly how - `invalid:` ends up with nothing the rule still matches. Run + **Confirm both fixture buckets still hold a case the rule behaves + on.** `verify` fails a rule whose `tests/pass/` or `tests/fail/` is + empty, and narrowing a pattern to kill a false positive is exactly + how `tests/fail/` ends up with nothing the rule still matches. Run ``` %(TASKLESS_CLI)s verify .taskless/rules/sg/ --json ``` diff --git a/packages/cli/src/agent/update.md b/packages/cli/src/agent/update.md index ca49a3d7..0589c61a 100644 --- a/packages/cli/src/agent/update.md +++ b/packages/cli/src/agent/update.md @@ -1,4 +1,4 @@ -# Topic: update (CLI v%(CLI_VERSION)s / topic v20) +# Topic: update (CLI v%(CLI_VERSION)s / topic v21) %(ONE_COMMAND_PER_CALL)s @@ -583,6 +583,38 @@ running CLI is not the version `.taskless/taskless.json` records, and are already doing what clears both: `init` moved the record to this CLI, step 4 moves the pins, and step 5 records the engines. +### Migrating to 0.13.4 + +**A rule's fixtures are case directories under `tests/` now, for every +engine, and `init` moves them for you.** They used to live in `.tests/`, +in three shapes: ast-grep's `valid:`/`invalid:` test YAML, Vale's loose +documents, and runtime's case directories. Each rule now keeps +`tests/pass//` and `tests/fail//`, and `verify` runs each +case through the engine's own check with the case as its root, so the +rule's `files:` globs or `.vale.ini` matchers apply to its fixtures the +way they apply to real code. A rule whose scope matched no real file +used to pass `verify` and report nothing in `check`; now its fixtures +fail, saying the scope excluded them. + +Migration 12, which `%(TASKLESS_CLI)s init` applies, writes each ast-grep +snippet as its own case and moves each Vale document into a case named +for it, each at a path the rule's scope admits, and renames runtime +cases. It leaves anything it cannot convert under `.tests/` and names +it. Fixtures carry no signature, so no issued rule is touched. + +What needs you, after `init`: + +- **Run `%(TASKLESS_CLI)s verify`.** A case that now fails because the + rule's scope excluded it is a fixture that only ever passed because + the old runner ignored scope. Move it to a path the scope admits, or + widen the scope if the rule was meant to reach it. +- **Rename the converted cases.** ast-grep snippets carry no + description, so they arrive named `pass-01`, `fail-01` and so on. Name + each directory for what it shows. +- **Anything left under `.tests/`** is named in the migration's output + with the reason. Move those files by hand into case directories, then + delete `.tests/`. + ## Errors With `--json`, `--rules` failures emit `{ ok: false, code, message }`: diff --git a/packages/cli/src/agent/validate-rule.md b/packages/cli/src/agent/validate-rule.md index 2a1f3753..e1350608 100644 --- a/packages/cli/src/agent/validate-rule.md +++ b/packages/cli/src/agent/validate-rule.md @@ -1,4 +1,4 @@ -# Topic: validate-rule (CLI v%(CLI_VERSION)s / topic v7) +# Topic: validate-rule (CLI v%(CLI_VERSION)s / topic v8) %(ONE_COMMAND_PER_CALL)s @@ -43,9 +43,11 @@ What each engine is checked for: | engine | `validate` checks | `verify` runs | |-----------|--------------------------------------------------------------------------------------------------------------------------------------|------------------------------------------------------------| -| `sg` | ast-grep schema, plus `id`/`language`/`severity`/`message`/`rule`, `regex` accompanied by `kind`, no top-level `strictness`, then ast-grep loads the rule on its own | the `valid`/`invalid` cases in `.tests/` | -| `vale` | style parses, `extends` and `message` present, `level` in vocabulary, and the rule's `.vale.ini` enables `.` under a matcher | the `.tests/pass/` and `.tests/fail/` buckets | -| `runtime` | `check.ts` present, at least one capture rule in `captures/` | each directory under `.tests/pass/` and `.tests/fail/`, as the check's `root` | +| `sg` | ast-grep schema, plus `id`/`language`/`severity`/`message`/`rule`, `regex` accompanied by `kind`, no top-level `strictness`, then ast-grep loads the rule on its own | each case under `tests/pass/` and `tests/fail/`, scanned as its own root | +| `vale` | style parses, `extends` and `message` present, `level` in vocabulary, and the rule's `.vale.ini` enables `.` under a matcher | each case under `tests/pass/` and `tests/fail/`, linted as its own root under the rule's matchers | +| `runtime` | `check.ts` present, at least one capture rule in `captures/` | each case under `tests/pass/` and `tests/fail/`, as the check's `root` | + +%(FIXTURE_LAYOUT)s A runtime rule's fixtures execute its `check.ts`, so they run only when you pass `--dangerously-run-scripts`. Nothing else runs them: `verify` diff --git a/packages/cli/src/prompts/recipes.ts b/packages/cli/src/prompts/recipes.ts index 00484b10..21f12a52 100644 --- a/packages/cli/src/prompts/recipes.ts +++ b/packages/cli/src/prompts/recipes.ts @@ -422,6 +422,47 @@ the construct is written, real ones follow how it is actually written. twin only on the rule's axis.`; } +/** + * How a rule's fixtures are laid out and judged, shared by every recipe that + * has the agent write or extend them. + * + * One block for every engine, because the contract is one contract: every + * engine's case is a directory, judged by that engine's own check with the + * case as its root. Three recipe-local copies of it would drift the way the + * three fixture readers did before they became one. The new thing for an + * author is the path rule for a scoped rule, so it is stated here, where every + * recipe reaches it. Each recipe keeps what is its engine's own beside it. + */ +function fixtureLayoutBlock(): string { + return `**How fixtures are laid out and judged.** Every rule, whatever its +engine, keeps its cases beside it in \`tests/\`: + +\`\`\` +/tests/pass//… the rule must stay quiet here +/tests/fail//… the rule must fire here +\`\`\` + +- **A case is a directory**, named for what it shows + (\`tests/fail/npx-latest/\`, not \`tests/fail/case-1/\`). It holds as many + files as the case needs. A file directly in \`pass/\` or \`fail/\`, or + anything in \`tests/\` besides those two, is refused by \`verify\`. +- **Each case is run like a small repository.** \`verify\` runs the + engine's own check with the case directory as the project root, so the + rule's scope applies to the case exactly as it applies to real code. A + rule scoped to some paths needs its fixture at such a path inside the + case: a rule with \`files: [".github/workflows/*.yml"]\` reads + \`tests/fail//.github/workflows/ci.yml\`, and a bare + \`tests/fail//ci.yml\` is outside its scope. +- **Firing is the whole test.** A \`fail/\` case passes when the rule reports + at least one finding in it, and a \`pass/\` case when it reports none. + Nothing about the line, the column, the message or the count is + asserted. Both buckets need at least one case. +- **"The rule's scope excluded it"** means a \`fail/\` case fires once its + scope is removed, so the pattern is fine and the fixture's path is not. + Move the fixture to a path the scope admits, or widen the scope. Do not + edit the pattern.`; +} + /** * Indent every line after the first, so a multi-line block substituted into an * indented `%(KEY)s` is still the markdown unit it replaces. @@ -769,6 +810,7 @@ export function buildVariables( " " ), // Substituted into a numbered fixture step, so indented to its body. + FIXTURE_LAYOUT: indentBlock(fixtureLayoutBlock(), " "), FIXTURE_SOURCES: indentBlock( fixtureSourcesBlock( options.mechanics === false ? undefined : resolveInvocation(options) diff --git a/packages/cli/test/prompts.test.ts b/packages/cli/test/prompts.test.ts index 14b46ac9..39712991 100644 --- a/packages/cli/test/prompts.test.ts +++ b/packages/cli/test/prompts.test.ts @@ -214,6 +214,7 @@ describe("the CLI invocation variable", () => { "AST_GREP_VERSION", "CLI_VERSION", "DETECT_EVIDENCE", + "FIXTURE_LAYOUT", "FIXTURE_SOURCES", "HOST_TOOLS", "LEGACY_LINTERS", diff --git a/packages/cli/test/recipe-cross-references.test.ts b/packages/cli/test/recipe-cross-references.test.ts index 967450b9..e723dcce 100644 --- a/packages/cli/test/recipe-cross-references.test.ts +++ b/packages/cli/test/recipe-cross-references.test.ts @@ -436,15 +436,15 @@ describe("recipes state engine reach from the pinned versions", () => { expect(recipe).toContain("A hyphen is a boundary"); }); - it("explains why a named fixture bucket is reachable by check", async () => { - // `check` on a `fail/` bucket is the only way to read a rendered `%s` - // message, and it works: `.taskless/` is excluded from the whole-project - // walk only. taskless/cli#370 read an empty `results` as proof the - // command could never work, so the recipe now says which exclusion is - // which, and names the rule-local matcher that really does empty it. + it("sends the author to verify --json for a rendered message", async () => { + // A `substitution` rule can render its `%s` slots in the wrong order and + // still pass every case, so the rendered message is the only evidence. + // It used to be read by running `check` on a fixture bucket, which no + // longer applies the rule's matchers the way `verify` does now that each + // case is run as its own root. `verify --json` reports every finding, + // the `fail/` ones included on a passing rule. const recipe = await rendered("create-vale-rule.md"); - expect(recipe).toContain(".tests/fail --json"); - expect(recipe).toContain("a path you name is honored"); - expect(recipe).toContain("matcher [.taskless/**]"); + expect(recipe).toContain("`verify --json` shows you every finding"); + expect(recipe).not.toContain(".tests/"); }); }); From b77a26002b0d38443a0916368b2449f6e1878f74 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 26 Oct 2026 11:23:23 -0700 Subject: [PATCH 2/6] docs(openspec): re-archive unified-rule-fixtures with the review fixes Standing specs pick up the corrected deltas: delivery's case-directory example and its two new scenarios, migration 12's cross-file case naming and source-removal rule, cli-rules' repathed anonymous flow, and the recipe name the shared fixture section lists. --- openspec/specs/cli-agent-authoring/spec.md | 2 +- .../specs/cli-generated-rule-delivery/spec.md | 24 +++++++++++++++---- openspec/specs/cli-rules/spec.md | 2 +- openspec/specs/cli-taskless-bootstrap/spec.md | 17 +++++++++++-- 4 files changed, 36 insertions(+), 9 deletions(-) diff --git a/openspec/specs/cli-agent-authoring/spec.md b/openspec/specs/cli-agent-authoring/spec.md index bdae887d..d157f050 100644 --- a/openspec/specs/cli-agent-authoring/spec.md +++ b/openspec/specs/cli-agent-authoring/spec.md @@ -244,7 +244,7 @@ Enforcing this is the recipe's job. `verify` SHALL NOT lint fixtures and SHALL N ### Requirement: Authoring recipes share one fixture section -Every recipe in which the agent writes or extends fixtures (`create-sg-rule`, `create-vale-rule`, `create-runtime-rule`, `improve-rule` and `validate-rule`) SHALL direct the agent to one shared fixture section and SHALL NOT carry its own copy of the layout. The shared section SHALL state: +Every recipe in which the agent writes or extends fixtures (`create-sg-rule`, `create-vale-rule`, `create-runtime-rule`, `improve-rule (anonymous)` and `validate-rule`) SHALL direct the agent to one shared fixture section and SHALL NOT carry its own copy of the layout. The shared section SHALL state: 1. The layout: `tests/pass//` and `tests/fail//`, with the case directory named for what the case shows. 2. That a case is judged by the engine's own check with the case directory as the root, so a fixture for a scoped rule places its files at a path the scope accepts. For example, an ast-grep rule scoped to `.github/workflows/*.yml` needs `tests/fail//.github/workflows/ci.yml`. diff --git a/openspec/specs/cli-generated-rule-delivery/spec.md b/openspec/specs/cli-generated-rule-delivery/spec.md index 3ce1da11..e1175f6c 100644 --- a/openspec/specs/cli-generated-rule-delivery/spec.md +++ b/openspec/specs/cli-generated-rule-delivery/spec.md @@ -117,7 +117,7 @@ After verification, the CLI SHALL write a served rule so that its directory hold served files: every file in the directory that the served set does not contain SHALL be removed, including under `tests/`, since a served set always carries the rule's fixtures. The CLI SHALL create each served file's parent directories as needed, so a nested path such as -`captures/env.yml` or `tests/fail/case.ts` is written wherever it lands. When a stale file cannot be removed, the CLI SHALL say that the served +`captures/env.yml` or `tests/fail/eval-call/case.ts` is written wherever it lands. When a stale file cannot be removed, the CLI SHALL say that the served bytes were written and name each entry it could not remove. A served set whose fixtures are in the old format, under `.tests/`, SHALL be migrated just in @@ -126,8 +126,10 @@ layout, and never written under `.tests/`. Fixtures carry no signature, so the c changes no signed byte, and the CLI never writes a layout its own `verify` does not read. This is a permanent reading of the old format, not a transition step. The CLI SHALL NOT depend on the service emitting the new layout, and a served set already in the new layout SHALL be -written unchanged. Fixtures the converter cannot place SHALL be written under -`tests/` as served, and the delivery SHALL say so, naming them. +written unchanged. A rule served with several legacy test files SHALL keep every snippet: +case names SHALL NOT collide across them. Fixtures the converter cannot place SHALL be written +under `tests/` at their served path below `.tests/`, and the delivery SHALL say so, naming them. +`verify` then refuses them by path, so an unplaced fixture is never mistaken for a passing one. #### Scenario: A local extra file does not survive @@ -136,8 +138,8 @@ written unchanged. Fixtures the converter cannot place SHALL be written under #### Scenario: Nested paths are written -- **WHEN** a served set carries `tests/fail/case.ts` and the rule directory does not exist -- **THEN** the CLI SHALL create `tests/fail/` and write the file +- **WHEN** a served set carries `tests/fail/eval-call/case.ts` and the rule directory does not exist +- **THEN** the CLI SHALL create `tests/fail/eval-call/` and write the file #### Scenario: Legacy fixture paths are written in the new layout @@ -145,6 +147,18 @@ written unchanged. Fixtures the converter cannot place SHALL be written under - **THEN** the CLI SHALL write them as case directories under `tests/pass/` and `tests/fail/` - **AND** the rule directory SHALL NOT contain `.tests/` afterwards +#### Scenario: Several legacy test files keep every snippet + +- **WHEN** a served `sg` rule carries two legacy test files, each with one `invalid:` snippet +- **THEN** the CLI SHALL write two `fail/` cases, one per snippet + +#### Scenario: A fixture the converter cannot place is written and named + +- **WHEN** a served `sg` rule's `files:` and `ignores:` admit no path the converter can derive for its `.tests/-test.yml` +- **THEN** the CLI SHALL write that file at `tests/-test.yml`, not under `.tests/` +- **AND** the delivery SHALL warn, naming the file and why +- **AND** the next `verify` of the rule SHALL report `tests/-test.yml` as a stray entry + #### Scenario: A served set in the new layout is written as served - **WHEN** a served set carries its fixtures under `tests/pass//` and `tests/fail//` diff --git a/openspec/specs/cli-rules/spec.md b/openspec/specs/cli-rules/spec.md index a3f09993..7d2c7673 100644 --- a/openspec/specs/cli-rules/spec.md +++ b/openspec/specs/cli-rules/spec.md @@ -414,7 +414,7 @@ When `taskless rule create --anonymous` is invoked, the CLI SHALL execute the lo 1. NOT submit any request to the Taskless API 2. Generate the ast-grep rule using local logic (Claude SDK, agent-driven generation, or whatever the migrated implementation prefers — see design.md) 3. Write the rule file to `.taskless/rules/sg//.yml` -4. Write any generated test files into that rule's own directory, under `.taskless/rules/sg//.tests/` +4. Write any generated test files into that rule's own directory, under `.taskless/rules/sg//tests/` 5. NOT write a metadata sidecar (neither does the API-backed branch: the service never populates the `meta` block a sidecar would be written from, so no branch of `rule create` has ever written one) diff --git a/openspec/specs/cli-taskless-bootstrap/spec.md b/openspec/specs/cli-taskless-bootstrap/spec.md index 9a9d9800..5ec62460 100644 --- a/openspec/specs/cli-taskless-bootstrap/spec.md +++ b/openspec/specs/cli-taskless-bootstrap/spec.md @@ -476,9 +476,9 @@ Migration `12` SHALL convert each rule's fixtures from `.tests/` to the `tests/p | `vale` | each document `.tests//` | `tests///` | | `runtime` | `.tests///…` | `tests///…`, unchanged | -An ast-grep snippet's file extension SHALL come from a fixed table keyed on the rule's `language:`, covering every language the vendored ast-grep supports. A converted file's path within its case SHALL be one the rule's scope accepts (`files:` and `ignores:` for ast-grep, the enabling matchers for Vale), derived from the scope's own patterns and checked against them. A converted case SHALL be named by bucket and ordinal in source order, since a snippet carries no description to name it by. +An ast-grep snippet's file extension SHALL come from a fixed table keyed on the rule's `language:`, covering every language the vendored ast-grep supports. A converted file's path within its case SHALL be one the rule's scope accepts (`files:` and `ignores:` for ast-grep, the enabling matchers for Vale), derived from the scope's own patterns and checked against them. Where the scope offers more than one path, one keeping the file's own extension SHALL be preferred, since both engines choose how to read a file by its extension. A converted case SHALL be named by bucket and ordinal in source order, since a snippet carries no description to name it by. Ordinals SHALL continue across all of a rule's test files, read in name order, and two Vale documents sharing a stem SHALL get distinct case names, so no converted case lands on another. -The migration SHALL NOT delete what it did not convert. A test file it cannot parse, a snippet whose language has no table entry, or a scope it cannot derive an accepted path from SHALL be left in place under `.tests/`. The migration SHALL name each one, and SHALL remove `.tests/` only once it is empty. A destination that already exists SHALL keep its bytes. The migration SHALL be idempotent. +The migration SHALL NOT delete what it did not convert. A test file it cannot parse, a snippet whose language has no table entry, or a scope it cannot derive an accepted path from SHALL be left in place under `.tests/`. The migration SHALL name each one, and SHALL remove `.tests/` only once it is empty. A destination that already exists SHALL keep its bytes. A source SHALL be removed only once every file it produced is on disk: a destination already holding the same bytes counts, and one holding other bytes SHALL leave the source in place and named. The migration SHALL be idempotent. It SHALL NOT modify any shipped migration. Migrations `5`, `9` and `10` still move fixtures into `.tests/`, and `12` then moves them on, so a fresh scaffold and an old one end in the same state by running `1` through `12` in order. @@ -492,6 +492,19 @@ It SHALL NOT modify any shipped migration. Migrations `5`, `9` and `10` still mo - **AND** `.tests/` SHALL NOT exist afterwards - **AND** `taskless.json` SHALL record version 12 +#### Scenario: A rule with several test files keeps every snippet + +- **WHEN** an ast-grep rule's `.tests/` holds `no-eval-20260101-test.yml` and `no-eval-20260301-test.yml`, each with one `invalid:` snippet +- **THEN** `tests/fail/` SHALL hold `fail-01` with the first file's snippet and `fail-02` with the second's +- **AND** both test files SHALL be gone + +#### Scenario: A destination holding other bytes keeps its source + +- **WHEN** a snippet converts to a path that already holds different content +- **THEN** that content SHALL be unchanged +- **AND** the test file SHALL remain under `.tests/` +- **AND** the migration SHALL name the path + #### Scenario: A scoped rule's converted fixture sits where its scope looks - **WHEN** an ast-grep rule declares `files: [".github/workflows/*.yml"]` and its test YAML holds an `invalid:` snippet From d71124e1e5828417b60c984b58160d95021d5aae Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 26 Oct 2026 11:57:43 -0700 Subject: [PATCH 3/6] docs(agent): teach that every fixture case must be read, and indent the shared blocks From the follow-up review round: - The shared fixture section says a case the rule never read fails, what the two causes are, that the project's .gitignore applies inside a case, and that an empty case is refused. - validate-rule and create-runtime-rule put FIXTURE_LAYOUT and FIXTURE_SOURCES at column 0, so the rendered block's first line sat flush and its body three spaces in. A rendered-output test pins it. - update.md's Vale 3.24.0 section named .tests/; the 0.13.4 entry covers never-read cases and a leftover .tests/ failing verify. --- packages/cli/src/agent/create-runtime-rule.md | 4 +-- packages/cli/src/agent/update.md | 22 +++++++++------- packages/cli/src/agent/validate-rule.md | 2 +- packages/cli/src/prompts/recipes.ts | 12 +++++++-- .../cli/test/recipe-cross-references.test.ts | 26 +++++++++++++++++++ 5 files changed, 52 insertions(+), 14 deletions(-) diff --git a/packages/cli/src/agent/create-runtime-rule.md b/packages/cli/src/agent/create-runtime-rule.md index 5f60cbc7..342e697f 100644 --- a/packages/cli/src/agent/create-runtime-rule.md +++ b/packages/cli/src/agent/create-runtime-rule.md @@ -124,9 +124,9 @@ not run, names the flag, and exits 0. That is neither a pass nor a failure, and it is deliberately not a failure: nothing the holder of an unblessed rule can do would make a failure green. -%(FIXTURE_LAYOUT)s + %(FIXTURE_LAYOUT)s -%(FIXTURE_SOURCES)s + %(FIXTURE_SOURCES)s Both buckets have to be populated for the rule to pass, and every case has to match at least one capture. A case the captures do not match diff --git a/packages/cli/src/agent/update.md b/packages/cli/src/agent/update.md index 0589c61a..9b6169ca 100644 --- a/packages/cli/src/agent/update.md +++ b/packages/cli/src/agent/update.md @@ -558,15 +558,16 @@ whose front matter satisfies it. `validate` accepts it. **A Vale rule's `tests:` cases move into its fixtures, and `update --rules` moves them for you.** Vale 3.24.0 ships `vale test`, which runs cases written in a rule's `tests:` list. Taskless never runs it: -`verify` judges a Vale rule by the documents under `.tests/pass` and -`.tests/fail`. So `validate` and `verify` now reject the key, which +`verify` judges a Vale rule by its fixture cases under `tests/pass/` and +`tests/fail/`. So `validate` and `verify` now reject the key, which 0.12.0 had started accepting, rather than let a rule look tested by cases nothing runs. `check` still runs such a rule. -Do not move them by hand. Step 5 writes each case's `input` as a -document named for the case, under `.tests/fail` when the case expects -an alert and `.tests/pass` when it expects none, with the case's -`format` as its extension. It then deletes the `tests:` block, leaves +Do not move them by hand. Step 5 writes each case's `input` as one +document in a case directory named for the case, under `tests/fail/` +when the case expects an alert and `tests/pass/` when it expects none, +with the case's `format` as its extension, at a path the rule's +`.vale.ini` matchers admit. It then deletes the `tests:` block, leaves everything else in the rule as it was, never overwrites a fixture, and names each file it wrote. No rule Taskless issued carries `tests:`, so it never touches a signed rule. A rule with a case that reads through a @@ -607,13 +608,16 @@ What needs you, after `init`: - **Run `%(TASKLESS_CLI)s verify`.** A case that now fails because the rule's scope excluded it is a fixture that only ever passed because the old runner ignored scope. Move it to a path the scope admits, or - widen the scope if the rule was meant to reach it. + widen the scope if the rule was meant to reach it. A case the rule + never read at all, in either bucket, now fails too: a pass case used + to pass on no evidence when its file sat outside the scope, under an + ignored path, or was empty. - **Rename the converted cases.** ast-grep snippets carry no description, so they arrive named `pass-01`, `fail-01` and so on. Name each directory for what it shows. - **Anything left under `.tests/`** is named in the migration's output - with the reason. Move those files by hand into case directories, then - delete `.tests/`. + with the reason, and `verify` fails the rule until it is gone. Move + those files by hand into case directories, then delete `.tests/`. ## Errors diff --git a/packages/cli/src/agent/validate-rule.md b/packages/cli/src/agent/validate-rule.md index e1350608..07c17ae9 100644 --- a/packages/cli/src/agent/validate-rule.md +++ b/packages/cli/src/agent/validate-rule.md @@ -47,7 +47,7 @@ What each engine is checked for: | `vale` | style parses, `extends` and `message` present, `level` in vocabulary, and the rule's `.vale.ini` enables `.` under a matcher | each case under `tests/pass/` and `tests/fail/`, linted as its own root under the rule's matchers | | `runtime` | `check.ts` present, at least one capture rule in `captures/` | each case under `tests/pass/` and `tests/fail/`, as the check's `root` | -%(FIXTURE_LAYOUT)s + %(FIXTURE_LAYOUT)s A runtime rule's fixtures execute its `check.ts`, so they run only when you pass `--dangerously-run-scripts`. Nothing else runs them: `verify` diff --git a/packages/cli/src/prompts/recipes.ts b/packages/cli/src/prompts/recipes.ts index 21f12a52..65e1693c 100644 --- a/packages/cli/src/prompts/recipes.ts +++ b/packages/cli/src/prompts/recipes.ts @@ -445,18 +445,26 @@ engine, keeps its cases beside it in \`tests/\`: - **A case is a directory**, named for what it shows (\`tests/fail/npx-latest/\`, not \`tests/fail/case-1/\`). It holds as many files as the case needs. A file directly in \`pass/\` or \`fail/\`, or - anything in \`tests/\` besides those two, is refused by \`verify\`. + anything in \`tests/\` besides those two, is refused by \`verify\`, and + so is a case with no file in it. - **Each case is run like a small repository.** \`verify\` runs the engine's own check with the case directory as the project root, so the rule's scope applies to the case exactly as it applies to real code. A rule scoped to some paths needs its fixture at such a path inside the case: a rule with \`files: [".github/workflows/*.yml"]\` reads \`tests/fail//.github/workflows/ci.yml\`, and a bare - \`tests/fail//ci.yml\` is outside its scope. + \`tests/fail//ci.yml\` is outside its scope. The project's + \`.gitignore\` applies inside a case too, so keep fixtures out of ignored + directory names such as \`dist/\` or \`build/\`. - **Firing is the whole test.** A \`fail/\` case passes when the rule reports at least one finding in it, and a \`pass/\` case when it reports none. Nothing about the line, the column, the message or the count is asserted. Both buckets need at least one case. +- **Every case must be read.** A case in which the rule read no file fails + in either bucket, because it shows nothing: \`verify\` says the case "was + never read". Either the scope excluded every file in it (move the file to + a path the scope admits), or the engine walked none of them (an ignored + path, an empty file, or an extension the engine does not read). - **"The rule's scope excluded it"** means a \`fail/\` case fires once its scope is removed, so the pattern is fine and the fixture's path is not. Move the fixture to a path the scope admits, or widen the scope. Do not diff --git a/packages/cli/test/recipe-cross-references.test.ts b/packages/cli/test/recipe-cross-references.test.ts index e723dcce..b61c991f 100644 --- a/packages/cli/test/recipe-cross-references.test.ts +++ b/packages/cli/test/recipe-cross-references.test.ts @@ -448,3 +448,29 @@ describe("recipes state engine reach from the pinned versions", () => { expect(recipe).not.toContain(".tests/"); }); }); + +/** How many spaces `line` starts with. */ +function indentOf(line: string): number | undefined { + return /^ */.exec(line)?.[0].length; +} + +describe("a substituted block keeps one indent", () => { + it("indents the fixture layout block's first line like the rest", async () => { + // Block variables indent every line after the first by three spaces, so + // a placeholder at any other column renders a block whose first line is + // flush and whose body is indented: two markdown units where the source + // meant one. + const offenders: string[] = []; + for (const [file, rendered] of await renderedRecipes()) { + const lines = rendered.split("\n"); + const index = lines.findIndex((line) => + line.includes("**How fixtures are laid out and judged.**") + ); + if (index === -1) continue; + if (indentOf(lines[index] ?? "") !== indentOf(lines[index + 1] ?? "")) { + offenders.push(file); + } + } + expect(offenders).toEqual([]); + }); +}); From 439fc6d94aaf2db5319023b17a601fb6b96a74e9 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 26 Oct 2026 11:59:39 -0700 Subject: [PATCH 4/6] docs(openspec): re-archive unified-rule-fixtures after the second review round --- openspec/specs/cli-agent-authoring/spec.md | 1 + .../specs/cli-generated-rule-delivery/spec.md | 10 +++- openspec/specs/cli-rule-fixtures/spec.md | 53 +++++++++++++++++-- openspec/specs/cli-taskless-bootstrap/spec.md | 19 ++++--- 4 files changed, 72 insertions(+), 11 deletions(-) diff --git a/openspec/specs/cli-agent-authoring/spec.md b/openspec/specs/cli-agent-authoring/spec.md index d157f050..ab7d89fb 100644 --- a/openspec/specs/cli-agent-authoring/spec.md +++ b/openspec/specs/cli-agent-authoring/spec.md @@ -250,6 +250,7 @@ Every recipe in which the agent writes or extends fixtures (`create-sg-rule`, `c 2. That a case is judged by the engine's own check with the case directory as the root, so a fixture for a scoped rule places its files at a path the scope accepts. For example, an ast-grep rule scoped to `.github/workflows/*.yml` needs `tests/fail//.github/workflows/ci.yml`. 3. That a `fail/` case needs one finding and a `pass/` case none, and that nothing about position, message, or count is asserted. 4. What a "scope excluded this case" failure means, and that the fix is the fixture's path or the scope, not the pattern. +5. That every case must be read: a case the rule read no file in fails in either bucket, the project's `.gitignore` applies inside a case, and a case with no file is refused. A per-engine recipe SHALL keep only what is that engine's own, such as Vale's scope reach and the fact that a runtime case is the check's root. diff --git a/openspec/specs/cli-generated-rule-delivery/spec.md b/openspec/specs/cli-generated-rule-delivery/spec.md index e1175f6c..80f01e21 100644 --- a/openspec/specs/cli-generated-rule-delivery/spec.md +++ b/openspec/specs/cli-generated-rule-delivery/spec.md @@ -127,7 +127,9 @@ changes no signed byte, and the CLI never writes a layout its own `verify` does is a permanent reading of the old format, not a transition step. The CLI SHALL NOT depend on the service emitting the new layout, and a served set already in the new layout SHALL be written unchanged. A rule served with several legacy test files SHALL keep every snippet: -case names SHALL NOT collide across them. Fixtures the converter cannot place SHALL be written +case names SHALL NOT collide across them, nor with a case the served set already carries +under `tests/`, nor with the name a fixture moved as served keeps. `sg test`'s +`__snapshots__/` SHALL be dropped and named. Fixtures the converter cannot place SHALL be written under `tests/` at their served path below `.tests/`, and the delivery SHALL say so, naming them. `verify` then refuses them by path, so an unplaced fixture is never mistaken for a passing one. @@ -152,6 +154,12 @@ under `tests/` at their served path below `.tests/`, and the delivery SHALL say - **WHEN** a served `sg` rule carries two legacy test files, each with one `invalid:` snippet - **THEN** the CLI SHALL write two `fail/` cases, one per snippet +#### Scenario: A served set in both layouts keeps its cases apart + +- **WHEN** a served `sg` rule carries `tests/fail/fail-01/` and a legacy test file with one `invalid:` snippet +- **THEN** the CLI SHALL write the converted snippet as `tests/fail/fail-02/` +- **AND** `tests/fail/fail-01/` SHALL hold only what was served there + #### Scenario: A fixture the converter cannot place is written and named - **WHEN** a served `sg` rule's `files:` and `ignores:` admit no path the converter can derive for its `.tests/-test.yml` diff --git a/openspec/specs/cli-rule-fixtures/spec.md b/openspec/specs/cli-rule-fixtures/spec.md index cb17ea08..821faaab 100644 --- a/openspec/specs/cli-rule-fixtures/spec.md +++ b/openspec/specs/cli-rule-fixtures/spec.md @@ -10,9 +10,9 @@ One fixture contract for every rule engine: where a rule's test cases live, what Every rule, whatever its engine, SHALL keep its fixtures at `tests/pass//` and `tests/fail//` inside its rule directory. A case SHALL be a directory, and its name SHALL be the case's description. A case MAY hold any number of files at any depth. -Each bucket SHALL be read one level deep. An entry directly under `pass/` or `fail/` that is not a directory SHALL be an error naming the path, not an ignored entry. An entry directly under `tests/` other than `pass/` and `fail/` SHALL be an error naming the path. Buckets SHALL be read independently, and a bucket that exists but cannot be read SHALL be an error rather than an empty bucket. +Each bucket SHALL be read one level deep. An entry directly under `pass/` or `fail/` that is not a directory SHALL be an error naming the path, not an ignored entry. An entry directly under `tests/` other than `pass/` and `fail/` SHALL be an error naming the path. A case directory that holds no file at any depth SHALL be an error naming it. A legacy `.tests/` beside the rule that still holds fixtures SHALL be an error naming it. Operating-system metadata (`.DS_Store`, `Thumbs.db`, `desktop.ini`) and `.gitkeep` SHALL be skipped wherever fixtures are read, and SHALL NOT count as a case's file. Buckets SHALL be read independently, and a bucket that exists but cannot be read SHALL be an error rather than an empty bucket. -**Rationale.** One shape for every engine is what lets one loop verify every rule. A case is a directory rather than a document because the engines that scope by path judge a file by where it sits, so a case has to be able to place a file at the path the rule expects. A runtime rule's evidence also spans files. The strict reads keep a fixture that was written but never ran from passing in silence: a skipped loose file is a fixture nobody mentioned, and a swallowed permission error makes a two-sided rule look one-sided and a one-sided rule look complete. An engine-native test file left in `tests/`, such as an ast-grep test YAML, is refused for the same reason. It would otherwise sit beside the cases and never run. +**Rationale.** One shape for every engine is what lets one loop verify every rule. A case is a directory rather than a document because the engines that scope by path judge a file by where it sits, so a case has to be able to place a file at the path the rule expects. A runtime rule's evidence also spans files. The strict reads keep a fixture that was written but never ran from passing in silence: a skipped loose file is a fixture nobody mentioned, and a swallowed permission error makes a two-sided rule look one-sided and a one-sided rule look complete. An engine-native test file left in `tests/`, such as an ast-grep test YAML, is refused for the same reason. It would otherwise sit beside the cases and never run, and so would a leftover `.tests/`. An empty case tests nothing, and git does not keep an empty directory, so it would vanish from every clone. Metadata a file manager writes is no author's fixture, and refusing it would fail a rule over a `.DS_Store`. #### Scenario: A case is a directory named for what it shows @@ -31,6 +31,21 @@ Each bucket SHALL be read one level deep. An entry directly under `pass/` or `fa - **WHEN** `tests/` contains an entry other than `pass/` and `fail/`, such as `-test.yml` - **THEN** verification SHALL report an error naming that path +#### Scenario: A case with no file is refused + +- **WHEN** `tests/pass/clean/` exists and holds no file, or only `.gitkeep` +- **THEN** verification SHALL report an error naming that case + +#### Scenario: A leftover .tests is refused + +- **WHEN** a rule has valid `tests/` cases and a `.tests/` still holding a fixture +- **THEN** verification SHALL report an error naming `.tests/` + +#### Scenario: Operating-system metadata is skipped + +- **WHEN** a rule's `tests/` and `tests/pass/` each hold a `.DS_Store` +- **THEN** verification SHALL judge the rule on its cases as if neither were there + #### Scenario: An unreadable bucket is not an empty bucket - **WHEN** a bucket exists but cannot be read @@ -79,9 +94,41 @@ The findings a case produces SHALL be the findings `verify` reports for it, in t - **THEN** nothing SHALL have been written under `.taskless/rules/` - **AND** nothing SHALL remain that a later `check` could report against +### Requirement: A case the engine never read fails + +Verification SHALL establish, for every case, whether the engine read at least one file in it under the rule's scope. A case in which it read none SHALL fail, whichever bucket it is in, and SHALL be reported as never read. The failure SHALL say which cause it was: the rule's scope excluded every file in the case, decided by asking again with the scope removed; or the walk reached no file even then, because the project's ignore files cover them, they are empty, or none is in a language or format the engine reads. + +Whether a file was read SHALL be asked of the engine itself, not decided by reimplementing its walk or its glob matching: for ast-grep, the files its `--inspect entity` output reports a rule was applied to; for Vale, a probe rule that reports once on any non-empty document, run under the same matchers. A case that produced a finding was read. + +A scan that ends without an exit code, killed by a signal, SHALL be a failure of the run, never a scan that found nothing. + +**Rationale.** A `pass/` case passes on no finding, and a case the engine never read produces no finding, so a pass case under a gitignored path, holding an empty file, or excluded by the scope passed on no evidence at all. A `fail/` case in the same position blamed the pattern. Reproduced in each form during review, including through a glob dialect difference: in ast-grep a single `*` crosses `/`, in `node:path`'s `matchesGlob` it does not, so any check that re-derived the walk would have agreed the file was read when ast-grep never opened it. Asking the engine leaves no second implementation to disagree with it. + +#### Scenario: A pass case outside the scope fails as never read + +- **WHEN** an ast-grep rule declares `files: [".github/workflows/*.yml"]` and a `pass/` case holds only `ci.yml` at its root +- **THEN** the case SHALL fail +- **AND** the failure SHALL say the case was never read because the scope excluded every file in it + +#### Scenario: A case under an ignored path fails as never read + +- **WHEN** the project's `.gitignore` lists `dist/` and an ast-grep rule's `pass/` case holds its only file at `dist/a.ts` +- **THEN** the case SHALL fail as never read +- **AND** the failure SHALL name the project's ignore files among the causes + +#### Scenario: A case whose only file is empty fails as never read + +- **WHEN** a `pass/` case holds one empty file +- **THEN** the case SHALL fail as never read + +#### Scenario: The engine decides what was read + +- **WHEN** an ast-grep rule declares `files: ["**/*.ts"]` and `ignores: ["*01.ts"]`, and a `pass/` case holds `src/pass-01.ts` +- **THEN** the case SHALL fail as never read, because ast-grep's `*` matches across `/` and the ignore applies + ### Requirement: A quiet fail case is diagnosed against the rule's scope -When a `fail/` case of an ast-grep or Vale rule produces no finding, verification SHALL run that case once more with the rule's scope removed. If that run produces a finding, the failure SHALL say that the rule's scope excluded the case and name the scope that did it. Otherwise it SHALL say the rule did not match. The diagnostic run SHALL NOT change the verdict. The case still fails. +When a `fail/` case of an ast-grep or Vale rule was read and produced no finding, verification SHALL run that case once more with the rule's scope removed. If that run produces a finding, the failure SHALL say that the rule's scope excluded the case and name the scope that did it. Otherwise it SHALL say the rule did not match. The diagnostic run SHALL NOT change the verdict. The case still fails. **Rationale.** Applying scope to fixtures adds a new way for a correct rule to fail: a fixture at a path the scope does not accept. "Did not fire" points at the pattern, and an author edits the wrong thing. One extra run separates the two causes, and it costs nothing on a passing rule. diff --git a/openspec/specs/cli-taskless-bootstrap/spec.md b/openspec/specs/cli-taskless-bootstrap/spec.md index 5ec62460..ce644c45 100644 --- a/openspec/specs/cli-taskless-bootstrap/spec.md +++ b/openspec/specs/cli-taskless-bootstrap/spec.md @@ -476,9 +476,9 @@ Migration `12` SHALL convert each rule's fixtures from `.tests/` to the `tests/p | `vale` | each document `.tests//` | `tests///` | | `runtime` | `.tests///…` | `tests///…`, unchanged | -An ast-grep snippet's file extension SHALL come from a fixed table keyed on the rule's `language:`, covering every language the vendored ast-grep supports. A converted file's path within its case SHALL be one the rule's scope accepts (`files:` and `ignores:` for ast-grep, the enabling matchers for Vale), derived from the scope's own patterns and checked against them. Where the scope offers more than one path, one keeping the file's own extension SHALL be preferred, since both engines choose how to read a file by its extension. A converted case SHALL be named by bucket and ordinal in source order, since a snippet carries no description to name it by. Ordinals SHALL continue across all of a rule's test files, read in name order, and two Vale documents sharing a stem SHALL get distinct case names, so no converted case lands on another. +An ast-grep snippet's file extension SHALL come from a fixed table keyed on the rule's `language:`, covering every language the vendored ast-grep supports. A converted file's path within its case SHALL be one the rule's scope accepts (`files:` and `ignores:` for ast-grep, the enabling matchers for Vale), derived from the scope's own patterns and checked against them. A converted file SHALL keep its own extension, since both engines choose how to read a file by its extension; a scope that admits no path with that extension leaves the fixture unconverted and named. A converted case SHALL be named by bucket and ordinal in source order, since a snippet carries no description to name it by. Ordinals SHALL continue across all of a rule's test files, read in name order, and past every case already in `tests/`, and two Vale documents sharing a stem SHALL get distinct case names, so no converted case lands on another. -The migration SHALL NOT delete what it did not convert. A test file it cannot parse, a snippet whose language has no table entry, or a scope it cannot derive an accepted path from SHALL be left in place under `.tests/`. The migration SHALL name each one, and SHALL remove `.tests/` only once it is empty. A destination that already exists SHALL keep its bytes. A source SHALL be removed only once every file it produced is on disk: a destination already holding the same bytes counts, and one holding other bytes SHALL leave the source in place and named. The migration SHALL be idempotent. +The migration SHALL NOT delete what it did not convert. A test file it cannot parse, a snippet whose language has no table entry, or a scope it cannot derive an accepted path from SHALL be left in place under `.tests/`. The migration SHALL name each one, and SHALL remove `.tests/` only once it is empty. A destination that already exists SHALL keep its bytes. A source SHALL be removed only once every file it produced is on disk; one holding other bytes SHALL leave the source in place and named. A source whose conversion is already on disk byte for byte, from a run interrupted before it removed the source, SHALL only be removed. ast-grep's `__snapshots__/` under `.tests/` SHALL be removed, since it records the output of `sg test`, which nothing runs. The migration SHALL be idempotent. It SHALL NOT modify any shipped migration. Migrations `5`, `9` and `10` still move fixtures into `.tests/`, and `12` then moves them on, so a fresh scaffold and an old one end in the same state by running `1` through `12` in order. @@ -498,12 +498,17 @@ It SHALL NOT modify any shipped migration. Migrations `5`, `9` and `10` still mo - **THEN** `tests/fail/` SHALL hold `fail-01` with the first file's snippet and `fail-02` with the second's - **AND** both test files SHALL be gone -#### Scenario: A destination holding other bytes keeps its source +#### Scenario: A file fixed after an earlier run lands beside its cases -- **WHEN** a snippet converts to a path that already holds different content -- **THEN** that content SHALL be unchanged -- **AND** the test file SHALL remain under `.tests/` -- **AND** the migration SHALL name the path +- **WHEN** an earlier run converted a rule's good test file to `tests/fail/fail-01/` and left an unparseable one, and its author has since fixed it +- **THEN** the next run SHALL convert the fixed file to `fail-02` +- **AND** `fail-01` SHALL be unchanged + +#### Scenario: An interrupted run is finished, not duplicated + +- **WHEN** a test file's conversion is on disk byte for byte but the test file is still under `.tests/` +- **THEN** the migration SHALL remove the test file +- **AND** SHALL write no second copy of its cases #### Scenario: A scoped rule's converted fixture sits where its scope looks From 87e1b68fb687782ecd2f6db5e11ac3b0f5a571d0 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 26 Oct 2026 12:15:16 -0700 Subject: [PATCH 5/6] docs(openspec): record the Cloud notice for unified-rule-fixtures (taskless/taskless#398) --- .../changes/archive/2026-10-26-unified-rule-fixtures/tasks.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openspec/changes/archive/2026-10-26-unified-rule-fixtures/tasks.md b/openspec/changes/archive/2026-10-26-unified-rule-fixtures/tasks.md index f6bc0a5a..8a07cdcf 100644 --- a/openspec/changes/archive/2026-10-26-unified-rule-fixtures/tasks.md +++ b/openspec/changes/archive/2026-10-26-unified-rule-fixtures/tasks.md @@ -35,6 +35,6 @@ Stacked, merging forward with `gh stack merge`. The task groups describe the wor - [x] 4.1 Write the shared fixture section and point `create-sg-rule`, `create-vale-rule`, `create-runtime-rule`, `improve-rule` and `validate-rule` at it, removing their per-engine layout copies. Pass explicit paths to prettier, never `src/` (memory: prompt parity). Verify the prompt parity tests and `pnpm lint`. - [x] 4.2 `reference.json`: a case list for every engine, `grouping: case-directories`, and an incremented corpus `version`. Verify `reference.test.ts` covers every scenario in the `cli-conformance-reference` delta. - [x] 4.3 Update `.taskless/rules` READMEs, CONTRIBUTING and any doc naming `.tests/`. Verify `grep -rn '\.tests/'` outside migrations, the delivery JIT conversion and the CHANGELOG returns nothing unexpected. -- [ ] 4.4 Note the new layout and corpus version in the CLI collab artifact as information for Cloud, stating that no action is required because delivery migrates the old format just in time (memory: cloud collab artifacts). Verify the artifact reads back with the note. +- [x] 4.4 Tell Cloud about the new layout and corpus version. Filed as taskless/taskless#398 instead of the collab artifact: delivery migrates the old format just in time, so nothing breaks when the CLI ships, and Cloud follows by keying the new format on CLI >= 0.13.4 and validating against the nightly. - [x] 4.5 Run `pnpm typecheck` and `pnpm lint` across the stack tip. Archive the change on the tip (`pnpm openspec archive unified-rule-fixtures -y`), confirming every prior scenario survives per AGENTS.md. - [ ] 4.6 Land the stack with `gh stack merge`, after review. From 5eae0fabf94ad319fc925f6ba16aa257947e9e45 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 26 Oct 2026 13:11:46 -0700 Subject: [PATCH 6/6] docs(openspec): drop the landing task, which no merged tree can record --- .../changes/archive/2026-10-26-unified-rule-fixtures/tasks.md | 1 - 1 file changed, 1 deletion(-) diff --git a/openspec/changes/archive/2026-10-26-unified-rule-fixtures/tasks.md b/openspec/changes/archive/2026-10-26-unified-rule-fixtures/tasks.md index 8a07cdcf..575c58e1 100644 --- a/openspec/changes/archive/2026-10-26-unified-rule-fixtures/tasks.md +++ b/openspec/changes/archive/2026-10-26-unified-rule-fixtures/tasks.md @@ -37,4 +37,3 @@ Stacked, merging forward with `gh stack merge`. The task groups describe the wor - [x] 4.3 Update `.taskless/rules` READMEs, CONTRIBUTING and any doc naming `.tests/`. Verify `grep -rn '\.tests/'` outside migrations, the delivery JIT conversion and the CHANGELOG returns nothing unexpected. - [x] 4.4 Tell Cloud about the new layout and corpus version. Filed as taskless/taskless#398 instead of the collab artifact: delivery migrates the old format just in time, so nothing breaks when the CLI ships, and Cloud follows by keying the new format on CLI >= 0.13.4 and validating against the nightly. - [x] 4.5 Run `pnpm typecheck` and `pnpm lint` across the stack tip. Archive the change on the tip (`pnpm openspec archive unified-rule-fixtures -y`), confirming every prior scenario survives per AGENTS.md. -- [ ] 4.6 Land the stack with `gh stack merge`, after review.