diff --git a/.changeset/unified-rule-fixtures.md b/.changeset/unified-rule-fixtures.md new file mode 100644 index 00000000..9dc3f259 --- /dev/null +++ b/.changeset/unified-rule-fixtures.md @@ -0,0 +1,7 @@ +--- +"@taskless/cli": patch +--- + +Every rule's fixtures are now case directories under `tests/` for every engine: `tests/pass//` and `tests/fail//`. `taskless verify` runs each case through the engine's own check, with the case directory as its root, so ast-grep `files:`/`ignores:` and a Vale rule's `.vale.ini` matchers apply to fixtures the same way they apply to your code. A fixture that only passed because the old runner ignored scope now fails, and the failure says the scope excluded it. ast-grep `valid:`/`invalid:` test YAML and `sg test` are no longer used. + +Run `taskless init` once to migrate. Migration 12 converts `.tests/` into the new layout: each ast-grep snippet becomes its own case, at a path the rule's scope admits, and each Vale document becomes a case named for it. It leaves anything it cannot convert in place and names it. The scaffold version moves to 12, so an older pinned CLI refuses a migrated project with `SCAFFOLD_VERSION_MISMATCH` until it is upgraded. A rule served in the old `.tests/` layout is converted as it is written, so nothing depends on the service changing. The `reference.json` corpus moves to version 4. diff --git a/openspec/changes/unified-rule-fixtures/.openspec.yaml b/openspec/changes/unified-rule-fixtures/.openspec.yaml new file mode 100644 index 00000000..d58ba023 --- /dev/null +++ b/openspec/changes/unified-rule-fixtures/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-10-26 diff --git a/openspec/changes/unified-rule-fixtures/design.md b/openspec/changes/unified-rule-fixtures/design.md new file mode 100644 index 00000000..6a6aa8b5 --- /dev/null +++ b/openspec/changes/unified-rule-fixtures/design.md @@ -0,0 +1,137 @@ +## Context + +See proposal.md for the motivation. The current state, by engine: + +| Engine | Fixtures | Verdict | Same path as `check`? | +| ------- | ------------------------------------------ | -------------------------------------------------------------------------------------------------- | -------------------------------------- | +| runtime | `.tests/{pass,fail}//` directories | `executeRuntimeRule(caseDir, rule)` | yes | +| vale | `.tests/{pass,fail}/` flat files | `vale` under `buildIsolatingConfig`, which enables the rule under `[*]` | half: same binary, scope discarded | +| sg | `.tests/-test.yml` `valid:`/`invalid:` | `sg test` for the verdict, then `scan --stdin` per snippet for findings (`sg-fixture-findings.ts`) | no: two mechanisms, neither is `check` | + +`cli-vale-rule-engine` already requires that Vale verification "exercises the scope the rule actually declares". The `[*]` config does not, so part of this change brings the implementation into line with a requirement that was already standing. + +Measured against ast-grep **0.51.0**, the version `main` pins, in a scratch tree: + +| Setup | Result | +| ----------------------------------------------------------------------------------- | ------------------------------------------ | +| `scan -r rule.yml --no-ignore hidden .`, cwd = case, file at the `files:` glob path | fires | +| same, file at the case root | silent: `files:` filtered it | +| same, cwd = parent, case passed as a path | silent: globs resolve against the cwd | +| `--config /sgconfig.yml` with an absolute `ruleDirs`, cwd = case | fires: globs still resolve against the cwd | +| plain `tests/` inside a `ruleDirs` tree | `Error: Cannot parse rule …/tests/…` | +| `.tests/` inside a `ruleDirs` tree | skipped | + +And against Vale **3.24.0**, the pinned version, with `--config` in a separate directory: + +| Setup | Result | +| ---------------------------------------------- | ------------------------------------ | +| matcher `[docs/**/*.md]`, cwd = case, path `.` | only `docs/guide/x.md` fires | +| same, cwd = parent, case passed as a path | nothing fires | +| `[*]` or `[*.md]`, cwd = case | every `.md` fires | +| `[*]`, document under `.github/` | fires: Vale walks hidden directories | + +Both engines resolve scope against the working directory, so "cwd = case directory" means the same thing for each. + +## Goals / Non-Goals + +**Goals:** + +- One fixture reader and one judging loop for every engine. An engine contributes only "run my check over this root for this rule". +- Fixtures exercise scope. +- The committed rules tree is never handed to ast-grep directly, so what a rule directory may contain does not depend on undocumented discovery behavior. + +**Non-Goals:** + +- Asserting position, message, or count. Fire / stay quiet is the whole test. +- Linting fixture style (unchanged: recipe guidance, never `verify`). +- Fixing the stale `cli-check` requirement "Check subcommand executes ast-grep scan", which still describes a committed `.taskless/sg/sgconfig.yml` the code stopped reading several migrations ago. Restating it here would hide a separate correction inside this change. Track it as its own fix. + +## Decisions + +### D1. A case is judged by the engine's real check, cwd = case directory + +Each engine exposes `runCheck(rule, root) → findings` and nothing else. For ast-grep, that is `runSg` with `cwd: caseDir`, `configPath` pointing at a per-verification config in the run directory, and `ruleIds: [id]`. It is the same function `check` calls. For Vale, it is the Vale runner with cwd = case directory and a config built from the rule's own `.vale.ini` alone, with an absolute `StylesPath`. Not `assembleValeConfig({ ruleIds })`: that narrows what it writes but still validates every rule and refuses the whole assembly on any rejection, deliberately, for `check --rule`. Under verification that would let another rule's broken config fail this rule's cases. `validate` has already run on the rule under verification, so a one-rule config skips nothing. For runtime, it is `executeRuntimeRule(caseDir, rule)`, unchanged. + +The shared loop in `rules/fixtures.ts` reads the buckets, runs each case, applies fail ≥1 / pass 0, classifies coverage, and attaches `bucket` to each finding. + +_Alternatives:_ keep `sg test` for the verdict and adopt case directories only for findings. That keeps two mechanisms, which is the problem being fixed. A per-snippet `-r` stdin scan bypasses `files:` (measured), which defeats the goal. + +### D2. ast-grep reads rules from a rules-only tree in the run directory + +`assembleSgConfig` copies each rule's `.yml` into `.taskless/.sg-rules/.yml` beside the assembled `.sgconfig.yml`, and writes `ruleDirs` relative to it. `check` assembles against its snapshot, so for `check` the tree lands inside the run directory, which is per-run and swept (`cli-check`: "Each check run works in its own run directory"). For any other caller it is gitignored (migration 12 adds `/.sg-rules/`). Copied rather than symlinked, for Windows and because rule files are small. + +Verification writes its own one-rule config and tree in a temporary directory, with an absolute `ruleDirs`, so the tree holds only the rule under verification. That gives the per-rule isolation the current `--filter` / single-rule config provides, so one bad `language:` elsewhere cannot abort the case. + +_Alternatives:_ + +- One `-r` per rule in `check`: N full repository walks. Rejected. +- Keep `.tests/`: works today, but leaves the layout depending on undocumented discovery behavior that this repository pins with a vendor-contract test. It also leaves fixtures in a directory most tools hide. +- A multi-document `-r` file: one bad rule still aborts all, and it buys nothing over D2. + +`ast-grep-vendor-contract.test.ts` keeps one assertion that a plain `tests/` inside `ruleDirs` fails. It now documents why D2 exists, not a behavior we rely on. + +### D3. The unscoped diagnostic run + +When a `fail/` case is quiet, run it once more without scope: + +- **ast-grep:** a copy of the rule in the run directory with `files` and `ignores` removed, edited through the `yaml` document API that `sg-fixture-findings.ts` already uses, so formatting is irrelevant. +- **Vale:** today's `buildIsolatingConfig` (`[*]`), which survives exactly for this purpose. + +The diagnostic run only selects the message. It never changes the verdict. + +### D4. Migration 12 is a scaffold migration, and its converter is a shared module + +The layout is what the scaffold version records (see the header of `rule-migrate.ts`), so this is `filesystem/migrations/0012-*`, not a rule-content migration. The conversion lives in `rules/fixture-convert.ts` so that migration 12 and delivery (D6) share it: + +- **Language → extension:** a const table over the languages ast-grep 0.51.0 supports, keyed by ast-grep's own spelling. A vendor-contract test asserts the table covers the binary's language list, so a bump that adds a language fails there rather than in a user's migration. +- **Scope → path:** take the first `files:` glob that admits the file's own extension (a wildcard-only last segment such as `src/*` takes it; one naming another extension cannot hold the file), substitute `**/` → empty and `*` → the case name, and expand the first `{a,b}` alternative. Then check the result against `files` and `ignores` with `node:path`'s `matchesGlob`, which the Node floor (≥22.22) provides, so no glob dependency is added. Its dialect is close to ast-grep's `globset` but not identical, so the check catches derivation mistakes and is not a proof. The next `verify` is the proof, and D3 names the cause if they disagree. A rule with no `files:` gets `.` at the case root. Vale does the same with the rule's enabling matchers. If the result fails its own check, the rule is left unconverted and named. +- **Case names:** `-` in source order, continuing past every case already in `tests/` and across all of a rule's test files: a rule improved twice owns two dated `-YYYYMMDD-test.yml` files, and numbering each from one would put both files' first snippets at the same path. A Vale document is named by its stem, suffixed `-2`, `-3`… when another document already took it. Authors rename them to descriptions. The migration cannot invent one. + +Migration 12 cannot run `verify`: scaffold migrations spawn no engines. Checking each derived path against the scope's own patterns is what stands in for that. A converted rule that still fails surfaces on the next `verify`, with D3's message pointing at the cause. This walks back an earlier idea of "report verdict changes after migrating": doing that would need the old verifier alive alongside the new one. + +_Alternative:_ keep snippets in a single file per case named after the snippet's hash. Unreadable, and it buys nothing over ordinals. + +### D5. The Vale 3.24 `tests:` rule migration writes the new layout + +`rule-migrations/vale-3-24-0-tests-to-fixtures.ts` currently writes `.tests//`. It changes to write `tests///`, placing the document at a matcher-accepted path through the shared converter. Rule migrations run under `update --rules`, after scaffold migrations, so they always see the new layout. + +### D6. Delivery migrates an old-format response just in time + +Until the Cloud generator emits `tests///`, served v2 file sets carry fixtures under `.tests/`. Delivery treats `.tests/` as unsigned fixtures (as it does today), runs them through `fixture-convert.ts`, and writes the result under `tests/`. Fixtures are unsigned, so this changes no signed byte. Reconcile excludes both directories. + +This is a permanent reading of the old format, not a transition shim. The CLI depends on no change from Cloud: the service can keep emitting `.tests/` for as long as it likes, and every CLI from this release on writes `tests/`. A served set already in the new layout passes through untouched. So the conversion is a no-op for whichever side moves first, and there is no rollout window to sequence. + +### D7. Published surface + +- `RULE_TESTS_DIRECTORY` becomes `"tests"`. `FixtureLayout` narrows to `"case-directories"`, and every `ENGINE_LAYOUTS` entry carries it. The type stays (not deleted), because the published `fixtureLayout` field is what a consumer reads and a later layout change would move. +- `reference.json` publishes a case list for every engine and increments its own `version`. Per AGENTS.md, the artifact's version field is the compatibility signal, not the package version. + +### D8. A case the engine never read fails, and the engine says what it read + +Found in review, reproduced four ways: a fixture under a gitignored path, an empty file, a path the scope excludes, and a path ast-grep's glob dialect excludes while `node:path`'s admits (a single `*` crosses `/` in ast-grep). In each a `pass/` case passed on no evidence. So every case is asked whether the engine read any file in it, and one it did not fails whatever its bucket. + +The answer comes from the engine, never from a reimplementation of its walk: + +- **ast-grep:** every case scan passes `--inspect entity`, which writes one stderr line per walked file with the number of rules applied to it (0.51.0: `sg: entity|file|: language=,appliedRuleCount=`). A file with a count above zero was read. Free: no extra run. Pinned in the vendor contract. +- **Vale:** no equivalent output, so a case with no finding is linted once more with a probe rule installed under the rule's own name in a scratch StylesPath, under the same matchers: `existence`, raw scope, one token `(?s)\S.*`, so one alert per non-empty document. Measured on 3.24.0: matchers apply, empty files are skipped, `.gitignore` is not read. +- Either way, a case found unread is asked again without scope, to say whether the scope or the walk was the cause. + +Cost, measured on this repository: verify over 14 rules went from 2.2 s to 1.3 s on the same machine; the probe runs only for cases with no finding, and the noise between runs exceeds it. + +_Alternatives:_ checking each fixture path against the scope with `matchesGlob` (rejected: that is the glob dialect gap itself); a probe rule for ast-grep (rejected: an ast-grep rule must name node kinds, which differ per language); disabling ignore files for fixture scans (rejected: `check` honours them, so a fixture there tests a path `check` never reads). + +## Risks / Trade-offs + +- [A fixture path matches a project `.gitignore` pattern, so the walker skips it and the case is quiet] → Detected by D8: the case fails as never read, naming ignore files among the causes, rather than passing (a `pass/` case) or blaming the pattern (a `fail/` one). The shared fixture section also tells authors to avoid ignored directory names. +- [Authoring cost: scoped rules need path-mirrored fixtures] → Accepted. It is the price of testing scope, and the recipes teach it once. +- [Spawn cost: one process per case] → ast-grep starts in tens of milliseconds, and cases run concurrently with a small cap. Measure on this repository's rules during implementation and record it. +- [The scaffold bump makes a pinned older CLI refuse migrated projects] → Expected for a layout change. Call it out in the changeset body. +- [Derived glob paths are wrong for exotic globs] → Each derived path is checked against the same globs, and failures leave the rule untouched and named. Nothing is converted wrong in silence. + +## Migration Plan + +1. Land the stack on `main` with `gh stack merge` (see proposal, Delivery shape). The nightly publishes it. This repository's own `.taskless/rules/` migrate in the stack itself, so `pnpm cli check` keeps working throughout. +2. Release 0.13.4 when ready. Nothing is gated on Cloud (D6). +3. Tell Cloud about the new layout and the `reference.json` version through the collab artifact, so that they can move the generator when it suits them. No action is required of them. + +Rollback: revert the stack. Migration 12 is forward-only like every scaffold migration, but it only moves unsigned fixtures, so a project on a reverted CLI fails with `SCAFFOLD_VERSION_MISMATCH` rather than misreading its rules. diff --git a/openspec/changes/unified-rule-fixtures/proposal.md b/openspec/changes/unified-rule-fixtures/proposal.md new file mode 100644 index 00000000..d5cdab61 --- /dev/null +++ b/openspec/changes/unified-rule-fixtures/proposal.md @@ -0,0 +1,49 @@ +## Why + +Each engine tests its rules a different way. ast-grep reads `valid:`/`invalid:` snippets from a test YAML and judges them with `sg test`, then scans the same snippets a second time over stdin to recover the rendered messages. Vale lints flat fixture documents under a config that enables the rule everywhere (`[*]`). Runtime hands a case directory to the harness. None of the static paths is the one `check` runs. The stdin scan bypasses `files:`, and the Vale config throws away the rule's own matchers. So a rule whose scope never matches a real file passes `verify` and reports nothing in `check`: the silent-success failure this project keeps meeting. Vale 3.24 now ships its own in-rule test format too, and treating each engine's test format as authoritative would add a fourth layout. + +One layout for every engine, judged by the same invocation `check` makes, gives a single fixture loop and closes the scope gap. It also leaves less to go wrong, because the code that verifies a rule is the code that runs it. + +## What Changes + +- **BREAKING (layout):** a rule's fixtures move from `.tests/` to `tests/`, and every engine uses one shape: `tests/pass//` and `tests/fail//`, where a case is a directory named for what it shows. +- `verify` judges each case by running the engine's own check with the case directory as the root. A `fail/` case passes on at least one finding for the rule, and a `pass/` case passes on none. Scope applies: ast-grep `files:`/`ignores:` and the matchers in the rule's own `.vale.ini`. +- When a `fail/` case stays quiet, verify runs it again with scope removed. If it fires then, the error says the rule's scope excluded the fixture. +- **REMOVED:** `sg test` as the ast-grep verdict, and the stdin re-scan that collected ast-grep fixture findings. Findings now come from real files, so their paths and positions are native. +- ast-grep reads its rules from a rules-only tree assembled in the run directory, so `ruleDirs` never walks into `tests/`. Measured on 0.51.0, a plain `tests/` inside `ruleDirs` still fails the whole scan. +- Scaffold migration 12 converts existing fixtures. ast-grep snippets become case files, using a fixed language → extension table and a path that the rule's `files:` globs accept. Vale documents each become a case directory. Runtime cases are only renamed. Anything the migration cannot convert stays where it is, and the migration names it. +- The Vale 3.24 `tests:` → fixtures rule migration writes into the new layout. +- Rule delivery reads a service response in the old fixture format and migrates it just in time, through the same converter migration 12 uses, before anything is written. Fixtures carry no signature, so converting them changes no signed byte. The CLI therefore depends on no change from the Cloud service, now or later. +- Reconcile and delivery exclude `tests/` from signing the same way they exclude `.tests/` today. +- The authoring recipes share one fixture-writing section, which covers recreating the path a scoped rule expects. The per-engine recipes point at it. +- The conformance corpus and the published layout report one grouping, `case-directories`, for every engine. + +## Delivery shape + +**Stacked, merging forward with `gh stack merge`.** The directory rename, the runners that read it, migration 12 and delivery's conversion are only correct together, so they share one layer; a CLI that verifies `tests/` while delivery still writes `.tests/` would ship a broken state. Everything that is correct on its own goes in a layer below it: the rules-only ast-grep tree, the fixture converter and the engine behavior pins. Every layer is green on its own, because a stack merge is all-or-nothing and one red layer blocks the whole stack. The nightly carries it, and the release (0.13.4) follows when it is ready. Nothing waits on Cloud, because delivery migrates the old format just in time. One `patch` changeset sits on the bottom branch, since the package is pre-1.0. Its body tells consumers about the layout move. + +## Capabilities + +### New Capabilities + +- `cli-rule-fixtures`: the engine-independent fixture contract: the case-directory layout, judging a case through the engine's own check, scope applying to fixtures, the unscoped diagnosis, and what in `tests/` is refused. + +### Modified Capabilities + +- `cli-rule-validation`: verify's fixture scenarios move to case directories for every engine. The ast-grep-specific findings requirement is removed, because the unified runner covers it. +- `cli-vale-rule-engine`: Vale fixtures are case directories and are verified under the rule's real scope. The self-contained directory names `tests/`. +- `cli-runtime-rule-execution`: the fixture paths and exclusions name `tests/`. +- `cli-rule-format`: the rule directory table, the dot-prefix rationale (replaced by the rules-only tree), and the assembled ast-grep config. +- `cli-conformance-reference`: every engine publishes `case-directories` with an explicit case list. +- `cli-generated-rule-delivery`: unsigned fixtures live under `tests/`, and a response in the old fixture format is migrated just in time on write. +- `cli-rule-reconciliation`: `tests/` is the unsigned, unreported fixture directory. +- `cli-rules`: create and delete name `tests/`. +- `cli-agent-authoring`: the Vale recipe's fixture artifact, plus a shared fixture section every authoring recipe uses. +- `cli-taskless-bootstrap`: migration 12. + +## Impact + +- **Code:** `rules/layout.ts` (`RULE_TESTS_DIRECTORY`, `FixtureLayout`), `rules/verify.ts`, `rules/sg-fixture-findings.ts` (deleted), `rules/vale/verify.ts`, `rules/runtime/fixtures.ts`, `rules/fixtures.ts`, `rules/assemble.ts`, `rules/scan.ts`, `rules/deliver.ts`, `rules/reference.ts`, reconcile signing, `filesystem/migrations/` (new `0012`), `rules/rule-migrations/vale-3-24-0-tests-to-fixtures.ts`, the agent recipes under `src/agent/`. +- **Published surface:** `@taskless/cli/layout` (`RULE_TESTS_DIRECTORY`, `fixtureLayout`) and `reference.json` (`tests.grouping`). The Cloud generator builds payloads against both. +- **Cross-team:** none required. The service may keep emitting `.tests/` indefinitely, and delivery migrates it. Cloud is told about the new layout and corpus version so that it can move when it wants to. +- **Users:** the scaffold version bump makes a pinned older CLI refuse a migrated project with `SCAFFOLD_VERSION_MISMATCH`, which is what a layout change means. This repository's own `.taskless/rules/` migrate as part of the change. diff --git a/openspec/changes/unified-rule-fixtures/specs/cli-agent-authoring/spec.md b/openspec/changes/unified-rule-fixtures/specs/cli-agent-authoring/spec.md new file mode 100644 index 00000000..801ebd19 --- /dev/null +++ b/openspec/changes/unified-rule-fixtures/specs/cli-agent-authoring/spec.md @@ -0,0 +1,86 @@ +## MODIFIED Requirements + +### Requirement: The Vale authoring recipe covers rule, scope, and fixtures + +The `create-vale-rule` recipe SHALL instruct the agent to produce three artifacts, and SHALL state that a rule is incomplete without all three: + +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. 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. + +It SHALL direct the agent to check its work by running `validate` and then `verify` against the rule's directory path. + +The previous version of this requirement taught the agent to add a matcher to a single project-wide `.vale.ini`. Executing that recipe against sandboxed agents found every one of its silent failures in that step and nowhere else — an assignment above the first matcher, a glob that missed the fixture extension, three names that had to agree with nothing reporting when they didn't. The layout change removes the step rather than documenting it further. + +The recipe SHALL additionally carry the guidance below. Each item is a failure observed while authoring rules against this repository, and each produced a rule that passed `validate` and `verify` while reporting nothing: + +- **What each `scope` reaches**, as measured: `text` sees prose, `code` sees inline code spans, `[code, text]` sees both, and `raw` sees prose, inline code, and fenced blocks. `raw` subsumes the other two. +- **That `scope` is per-rule.** Vale assembles one config per run, which invites the assumption that scopes interact. They do not. +- **That a rule scoped to `raw` cannot be suppressed** by Vale's `` directive, because it reads the unparsed document. A rule about a command needs `raw`, so it trades away per-case exemption. +- **That a token made only of punctuation needs `nonword: true`**, because Vale wraps every token in word boundaries. +- **How to scope a rule out**, not only in: a second matcher assigning `. = NO`. +- **That a bare word finds senses you did not mean**, and that narrowing to a collocation is checked by writing the `pass/` fixture from the literal sense first. +- **That fixture design follows the rule's subject**: when the subject normally appears in code, the `fail/` fixture SHALL contain it inline, fenced, and in prose. +- **The `limit` and `vocab` common fields**, which the recipe's field table omits. + +#### Scenario: Authoring produces all three artifacts + +- **WHEN** the agent follows `create-vale-rule` +- **THEN** it writes the style file, the rule's own config with a scoping matcher, and both fixture buckets + +#### Scenario: No shared file is edited + +- **WHEN** the agent scopes a rule +- **THEN** it writes matchers into that rule's own config +- **AND** it SHALL NOT be directed to edit a project-wide Vale config + +#### Scenario: The recipe names the commands that check the work + +- **WHEN** the agent has written the three artifacts +- **THEN** the recipe SHALL direct it to run `validate` and `verify` against the rule's path + +#### Scenario: An unscoped rule is not silently accepted + +- **WHEN** the agent writes a style file without a matcher enabling it in the rule's own config +- **THEN** the recipe SHALL identify this as incomplete +- **AND** `validate` SHALL report it + +#### Scenario: A rule about a command reaches fenced blocks + +- **WHEN** the agent authors a rule whose subject is a command, flag, or package name +- **THEN** the recipe SHALL direct it to a scope that reaches fenced blocks +- **AND** the `fail/` fixture SHALL carry the subject inline, fenced, and in prose + +#### Scenario: A punctuation token is not left unable to match + +- **WHEN** the agent authors a rule whose token contains no word characters +- **THEN** the recipe SHALL direct it to set `nonword: true` + +## ADDED Requirements + +### 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 (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`. +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. + +**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/changes/unified-rule-fixtures/specs/cli-conformance-reference/spec.md b/openspec/changes/unified-rule-fixtures/specs/cli-conformance-reference/spec.md new file mode 100644 index 00000000..925d028b --- /dev/null +++ b/openspec/changes/unified-rule-fixtures/specs/cli-conformance-reference/spec.md @@ -0,0 +1,58 @@ +## REMOVED Requirements + +### Requirement: The corpus states how its fixtures group into cases + +**Reason**: Its scenarios publish three groupings (`case-directories`, `case-documents` and `ast-grep-test`), and two of them describe layouts that no longer exist. Under `cli-rule-fixtures` every engine's case is a directory. A MODIFIED block could not drop those scenarios, and keeping them under the old title would leave the spec saying a Vale case is one document. + +**Migration**: Replaced by "The corpus publishes every engine's fixtures as case directories" below. The `grouping` field stays, and its only value is `case-directories`. The corpus `version` increments, because `tests.files` paths change prefix and the `sg` entry gains a case list. + +## ADDED Requirements + +### 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/changes/unified-rule-fixtures/specs/cli-generated-rule-delivery/spec.md b/openspec/changes/unified-rule-fixtures/specs/cli-generated-rule-delivery/spec.md new file mode 100644 index 00000000..a2a73ebe --- /dev/null +++ b/openspec/changes/unified-rule-fixtures/specs/cli-generated-rule-delivery/spec.md @@ -0,0 +1,100 @@ +## MODIFIED Requirements + +### 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 +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` +- **THEN** the CLI SHALL refuse the rule naming the file +- **AND** SHALL write nothing for it + +#### Scenario: An unsigned file is refused + +- **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` +- **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 +create each served file's parent directories as needed, so a nested path such as +`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 +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. A rule served with several legacy test files SHALL keep every snippet: +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. + +#### Scenario: A local extra file does not survive + +- **WHEN** a rule directory holds `captures/extra.yml` and the served set does not +- **THEN** after the write `captures/extra.yml` SHALL NOT exist + +#### Scenario: Nested paths are written + +- **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 + +- **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: 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 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` +- **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//` +- **THEN** the CLI SHALL write them at those paths without conversion + +#### Scenario: A generated rule's revision is confirmed + +- **WHEN** `rule create` or `rule improve` fetches a produced rule whose served `revisionId` differs from the one the request reported +- **THEN** the CLI SHALL refuse that rule and write nothing for it diff --git a/openspec/changes/unified-rule-fixtures/specs/cli-rule-fixtures/spec.md b/openspec/changes/unified-rule-fixtures/specs/cli-rule-fixtures/spec.md new file mode 100644 index 00000000..928f2cc9 --- /dev/null +++ b/openspec/changes/unified-rule-fixtures/specs/cli-rule-fixtures/spec.md @@ -0,0 +1,143 @@ +## 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`. + +## ADDED 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. 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, 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 + +- **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: 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 +- **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 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 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. + +#### 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/changes/unified-rule-fixtures/specs/cli-rule-format/spec.md b/openspec/changes/unified-rule-fixtures/specs/cli-rule-format/spec.md new file mode 100644 index 00000000..cb05c34a --- /dev/null +++ b/openspec/changes/unified-rule-fixtures/specs/cli-rule-format/spec.md @@ -0,0 +1,79 @@ +## MODIFIED Requirements + +### 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/`. + +| 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/`, 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 + +- **WHEN** a rule is authored for any engine +- **THEN** everything defining it lives under one `.taskless/rules///` directory +- **AND** removing that directory removes the rule completely + +#### Scenario: Test files are not mistaken for rules + +- **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 + +- **WHEN** the system needs a rule's engine +- **THEN** it reads the `` path segment +- **AND** it SHALL NOT parse the rule file to determine it + +### Requirement: Each engine's native config is the source of truth + +The system SHALL treat each engine's native config as the authoritative definition of its rules, their scoping, and their metadata, and SHALL NOT require a separate Taskless sidecar or metadata file for a rule. + +Where an engine's configuration is per-rule, that per-rule file is the committed source of truth. Where the engine requires a single file at invocation — Vale accepts one `--config`, ast-grep one `sgconfig.yml` — the system SHALL assemble that file from the committed per-rule sources and SHALL gitignore the result. An assembled config is the engine's own native config, split along the boundary the engine's own scoping already has; it is not a Taskless sidecar. + +An engine SHALL NOT be given a per-rule config file it has nothing to put in. ast-grep expresses scoping (`files`, `ignores`) inside the rule itself, so it has no per-rule config; Vale cannot express scoping inside the style — measured, `E201 has invalid keys` — so it does. + +#### Scenario: Vale config is assembled from committed per-rule configs + +- **WHEN** the CLI runs a check +- **THEN** it reads each committed `rules/vale//.vale.ini` and assembles the config it hands to Vale +- **AND** the assembled file SHALL be gitignored + +#### Scenario: ast-grep config is assembled from the rule tree + +- **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 + +- **WHEN** an ast-grep rule is authored +- **THEN** no per-rule config file SHALL be required alongside it + +#### Scenario: Native scoping is applied by the engine + +- **WHEN** an ast-grep rule declares native `files`/`ignores`, or a Vale rule's config declares include/exclude matchers +- **THEN** the engine applies that scoping directly, with no Taskless-side rule transformation + +### Requirement: Vale styles live under a per-rule StyleName + +The system SHALL place each Vale rule in its own directory `.taskless/rules/vale//`, so that `` is Vale's StyleName, with the assembled config setting `StylesPath` to the Vale rules tree. The Vale check identifier `.` SHALL be normalized to `ruleId = ` in results. + +`StylesPath` follows the layout and cannot be chosen independently of it. Measured against Vale 3.17.1: a rule at `/.yml` resolves as check `.` under a StylesPath naming its parent, and resolves to nothing at all under `StylesPath = .`. The reverse held for the previous flat layout — the same setting is correct for one layout and silently wrong for the other. + +#### Scenario: Style resolution and identity + +- **WHEN** a Vale style exists at `.taskless/rules/vale/no-simply/no-simply.yml` +- **THEN** Vale loads it as `no-simply.no-simply`, and the CLI reports its findings with `ruleId` `no-simply` + +#### Scenario: A rule's tests are not loaded as styles + +- **WHEN** a Vale rule directory contains `tests/` +- **THEN** Vale SHALL NOT load anything under it as a rule diff --git a/openspec/changes/unified-rule-fixtures/specs/cli-rule-reconciliation/spec.md b/openspec/changes/unified-rule-fixtures/specs/cli-rule-reconciliation/spec.md new file mode 100644 index 00000000..956dfb97 --- /dev/null +++ b/openspec/changes/unified-rule-fixtures/specs/cli-rule-reconciliation/spec.md @@ -0,0 +1,95 @@ +## MODIFIED Requirements + +### Requirement: Canonical rule signature envelope + +The CLI SHALL represent a rule file's canonical signature as a single self-describing +string of the form `;h=;d=`. For algoVersion `1` this is +`1;h=sha-256;d=`, where `` is the digest as lowercase hexadecimal. The token +before the **first** `;` is the algoVersion and SHALL be read up to that one delimiter to +detect the version (and therefore the normalization procedure and hash algorithm) before +any `key=value` parameters are parsed. Signatures SHALL be compared as whole strings. + +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/` 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 +contract grow from "`check.ts` only" to "every non-fixture file" without a new algoVersion, +because no single signature's meaning changed. + +#### Scenario: Envelope is emitted for algoVersion 1 + +- **WHEN** the CLI computes a signature for a rule file's bytes using algoVersion 1 +- **THEN** the signature SHALL be the string `1;h=sha-256;d=` for that file's normalized bytes + +#### Scenario: Version is read before parameters + +- **WHEN** the CLI parses a signature string +- **THEN** it SHALL read the algoVersion as the substring before the first `;` +- **AND** SHALL NOT rely on the `key=value` parameter syntax to determine the version + +#### Scenario: Signatures compare as whole strings + +- **WHEN** the CLI compares two signatures for equality +- **THEN** it SHALL compare the full envelope strings, not the bare digests + +#### Scenario: A v1 signature covers exactly one file + +- **WHEN** the CLI computes an algoVersion-1 signature for a rule +- **THEN** that signature SHALL be over one file's bytes and over no other file + +#### Scenario: A v1 signature covers the engine's rule file + +- **WHEN** a served runtime rule carries its singular `signature` +- **THEN** that signature SHALL be over `check.ts` and SHALL equal the `check.ts` entry in `signatures` + +#### Scenario: Every non-fixture file of a rule is signed + +- **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/` or `.tests/` + +### Requirement: Reconcile reports every rule directory + +When `check` reconciles, the CLI SHALL send `POST /cli/api/v2/reconcile` with +`{ repositoryUrl, orgId?, rules }`, where `rules` holds exactly one +`{ 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/` 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. + +#### Scenario: Every engine is reported + +- **WHEN** `.taskless/rules/sg/`, `.taskless/rules/vale/`, and `.taskless/rules/runtime/` each hold rules +- **THEN** the request SHALL carry one entry per rule directory across all three engines + +#### Scenario: A rule's files are reported relative to its directory + +- **WHEN** runtime rule `no-env-leak-3fa9c21b` holds `check.ts` and `captures/env.yml` +- **THEN** its entry SHALL be `{ ruleId: "no-env-leak-3fa9c21b", files: [{ path: "check.ts", … }, { path: "captures/env.yml", … }] }` + +#### Scenario: Fixtures are not reported + +- **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 + +- **WHEN** a user runs `check --rule a` in a project holding rules `a` and `b` +- **THEN** the reconcile request SHALL report both `a` and `b` +- **AND** only `a` SHALL run diff --git a/openspec/changes/unified-rule-fixtures/specs/cli-rule-validation/spec.md b/openspec/changes/unified-rule-fixtures/specs/cli-rule-validation/spec.md new file mode 100644 index 00000000..8dbe9b95 --- /dev/null +++ b/openspec/changes/unified-rule-fixtures/specs/cli-rule-validation/spec.md @@ -0,0 +1,117 @@ +## MODIFIED Requirements + +### Requirement: Verify runs a rule's fixtures and runs validate first + +`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` 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. + +That is deliberately stricter than the policy `check` applies, not softer. `check` executes rules as a side effect of scanning a repository, so a blessed signature admits code the user never asked to run and a reconcile stands between the request and the execution. `verify` runs fixtures because the user asked for them, so the verb is the consent and the flag is the confirmation; a rule that `check` would run unflagged on a blessed signature still requires the flag here. Nothing executes under `verify` that would not have executed under the shared policy. + +A run refused for want of the flag SHALL be reported as not run, and SHALL be reported as neither a pass nor a failure: the rule is not defective, and no action available to its holder would make a failure green. The refusal SHALL name the flag, and SHALL NOT direct the reader to authenticate — authenticating cannot bless a rule that never left the working tree, so naming it would offer a fix that is not one. + +`verify` SHALL report, per rule, the findings its fixtures produced, as a `findings` array carrying the same finding shape `check --json` prints — `source`, `ruleId`, `severity`, `message`, `file`, `range`, `matchedText` and the optional `note` and `fix` — plus the `bucket` the fixture that produced it belongs to. + +The RENDERED message is the point, and it is what a verdict cannot carry. A rule whose message interpolates its captures can have its slots in the wrong order and still fire on every `fail/` fixture and stay quiet on every `pass/` one, so a boolean verdict reports it as a rule that passed. The only evidence that the message says what its author meant is the message as the engine rendered it, against material the author wrote, which `verify` already has in hand and discards. + +The array SHALL be present on every rule result and SHALL be empty rather than absent when there is nothing to report — including for an engine that does not yet surface its fixture findings, a rule whose validation failed before fixtures ran, and a run the execution policy refused. A consumer SHALL NOT have to distinguish "this rule produced no findings" from "this command does not report findings", because a key that is sometimes absent is one a reader learns to treat as optional, and the reading that follows is that its absence means zero. + +Fail-bucket findings SHALL be reported under `--json` on a passing run. They are the evidence, and a payload that carries the evidence only once the rule is already failing carries it at the one moment it is no longer needed. + +The human rendering SHALL stay a single line per passing rule. Under a FAILING rule it SHALL print the findings that bear on the failure, labelled by bucket, using the same renderer `check` prints findings with, so one finding does not read two ways depending on which command surfaced it. A pass-bucket finding SHALL be printed there: it is a fixture that wrongly fired, and naming the file says only that it happened, while the finding says what matched. + +#### Scenario: A malformed rule reports the malformation, not the fixtures + +- **WHEN** `verify` runs against a rule that is both invalid and missing a fixture bucket +- **THEN** it SHALL report the validation error +- **AND** it SHALL NOT report the fixture coverage as the failure + +#### Scenario: Another rule's config error does not fail this rule's fixtures + +- **WHEN** `verify` runs against one `sg` rule while a different `sg` rule in the project is one ast-grep refuses +- **THEN** the rule under verification SHALL be judged on its own fixtures alone +- **AND** its result SHALL NOT carry the other rule's error + +#### Scenario: Vale fixtures are tested per bucket + +- **WHEN** `verify` runs against a Vale rule +- **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** 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 + +#### 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 +- **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 + +#### Scenario: A runtime rule without the flag is reported as not run + +- **WHEN** `verify` runs against a runtime rule without `--dangerously-run-scripts` +- **THEN** the rule SHALL NOT be reported as passing +- **AND** the output SHALL say the fixtures did not run and why +- **AND** the output SHALL name `--dangerously-run-scripts` as what would run them +- **AND** the output SHALL NOT direct the reader to authenticate +- **AND** the rule SHALL NOT be counted among the rules verified +- **AND** the refusal alone SHALL NOT fail the command + +#### Scenario: Verifying a runtime rule reaches no network + +- **WHEN** `verify` runs against a runtime rule, with or without `--dangerously-run-scripts` +- **THEN** the CLI SHALL NOT request a token, resolve an organization, or reconcile +- **AND** the outcome SHALL NOT depend on authentication state, a git remote, or the availability of the rule service + +#### Scenario: A case that never reaches the check is reported as a fixture defect + +- **WHEN** a fixture case produces no narrow matches, so the check is never invoked +- **THEN** the CLI SHALL report that case as a fixture defect naming the case +- **AND** SHALL say the check did not run +- **AND** SHALL do so whether the case is in `pass/` or `fail/`, since a case that never invokes the check is evidence about the fixture rather than about the rule + +#### Scenario: A check that throws is distinguished from one that finds nothing + +- **WHEN** a runtime rule's check raises while running a fixture case +- **THEN** that SHALL be reported as the check failing, not as the case producing no findings + +#### Scenario: Verify reports the findings its fixtures produced + +- **WHEN** `verify --json` runs against a rule whose fixtures produced findings +- **THEN** each rule result SHALL carry a `findings` array +- **AND** each entry SHALL carry the rendered `message`, `file`, `range`, `matchedText` and `severity` that `check --json` reports for the same finding +- **AND** each entry SHALL name the bucket of the fixture that produced it +- **AND** the fail-bucket findings SHALL be reported even when the rule passed + +#### Scenario: The findings array is present and empty rather than absent + +- **WHEN** `verify --json` reports a rule that produced no findings, whose validation failed before its fixtures ran, whose run was refused, or whose engine does not surface fixture findings +- **THEN** the rule result SHALL still carry a `findings` array +- **AND** that array SHALL be empty + +#### Scenario: A failing rule prints the findings that bear on the failure + +- **WHEN** `verify` runs without `--json` and a rule fails because a `pass/` fixture 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 + +## REMOVED Requirements + +### Requirement: Verify reports the findings an ast-grep rule's fixtures produced + +**Reason**: This requirement existed because ast-grep fixtures were snippets inside a test YAML, judged by `sg test`, which cannot emit a finding. Their findings had to come from a second scan over stdin, mapped back to positions inside block scalars. Under `cli-rule-fixtures`, an ast-grep case is a directory of real files judged by the same scan `check` runs, so the finding that decides the verdict is the reported finding. It carries ast-grep's rendered message, and its `file` and position already point at the fixture. The general findings guarantees (the same shape as `check --json`, the bucket, fail-bucket findings on a passing run, an array that is empty rather than absent) stay in "Verify runs a rule's fixtures and runs validate first". The isolation and no-residue guarantees move to `cli-rule-fixtures`. + +**Migration**: Migration 12 converts every `valid:`/`invalid:` snippet into a case file. Consumers of `verify --json` see the same finding shape. `file` now names `tests///` instead of the test YAML. diff --git a/openspec/changes/unified-rule-fixtures/specs/cli-rules/spec.md b/openspec/changes/unified-rule-fixtures/specs/cli-rules/spec.md new file mode 100644 index 00000000..4459f943 --- /dev/null +++ b/openspec/changes/unified-rule-fixtures/specs/cli-rules/spec.md @@ -0,0 +1,59 @@ +## MODIFIED Requirements + +### 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.) + +#### 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/` +- **AND** it SHALL do so whether or not `--anonymous` was set + +### Requirement: Rules delete removes rule and test files + +`taskless rule delete ` SHALL remove the rule and everything that defines it. Under the rule-directory layout that is one directory, `.taskless/rules///`, which carries the rule, any per-engine config, and its tests. (Renamed; repathed.) Accepts `--anonymous` as a no-op. + +A rule id does not carry its engine, so the CLI SHALL **resolve** which engine directory holds `` rather than assuming one. A rule id is globally unique by construction, so at most one engine can hold it. When no engine holds the id, the CLI SHALL report not-found without naming an engine, because naming one would be a guess. + +#### 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/` +- **AND** no file belonging to that rule SHALL remain + +#### Scenario: Deleting a rule filed under any engine + +- **WHEN** a rule with id `` exists under `.taskless/rules/vale//` or `.taskless/rules/runtime//` +- **THEN** `taskless rule delete ` SHALL remove that directory +- **AND** SHALL NOT report not-found for a rule that is present on disk + +#### Scenario: Deleting an id no engine holds + +- **WHEN** no engine directory contains `` +- **THEN** the CLI SHALL report the rule was not found under `.taskless/rules/` +- **AND** the message SHALL NOT name a single engine's path + +### Requirement: Rule create supports anonymous local-only flow + +When `taskless rule create --anonymous` is invoked, the CLI SHALL execute the local-only rule-creation flow (previously implemented as the `taskless-create-rule-anonymous` skill body). The flow SHALL: + +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/` +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) +6. Return the same output format as the API-backed branch (paths to created files) + +#### Scenario: rule create --anonymous skips API + +- **WHEN** a user runs `taskless rule create --from req.json --anonymous` +- **THEN** the CLI SHALL NOT make any HTTP request to the Taskless API +- **AND** SHALL produce a rule file under `.taskless/rules/` + +#### Scenario: rule create --anonymous produces no metadata sidecar + +- **WHEN** `taskless rule create --anonymous` succeeds +- **THEN** no file under `.taskless/rule-metadata/` SHALL be written for the new rule diff --git a/openspec/changes/unified-rule-fixtures/specs/cli-runtime-rule-execution/spec.md b/openspec/changes/unified-rule-fixtures/specs/cli-runtime-rule-execution/spec.md new file mode 100644 index 00000000..fdd82e2d --- /dev/null +++ b/openspec/changes/unified-rule-fixtures/specs/cli-runtime-rule-execution/spec.md @@ -0,0 +1,114 @@ +## MODIFIED Requirements + +### Requirement: Runtime rules are directories recognized by metadata + +The CLI SHALL recognize a **runtime rule** as a directory under `.taskless/rules/runtime//` +containing a `captures/` directory of one or more ast-grep capture `*.yml` (one per capture rule) +and a single `check.ts`, its capture rules declaring `metadata.taskless.kind: runtime`. The rule's +check file SHALL be the `check.ts` in the rule directory. The CLI SHALL read **each capture rule's** +`metadata.taskless.match` to select that capture rule's ast-grep invocation mode; capture rules +within one runtime rule MAY mix modes (each is independent). + +A `match` value the build does not implement SHALL be **refused**, never coerced to a default. The +modes scan different things — `anchor` is a syntactic narrow, `broad` a whole-language enumerator — +so substituting one for another reinterprets the capture rather than degrading it, and the shortfall +would be reported as a clean pass. The refusal SHALL be per capture file, not per rule, so one +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 +executed by `check`. + +#### Scenario: A runtime directory entry is a runtime rule + +- **WHEN** `.taskless/rules/runtime//captures/` contains `*.yml` with `metadata.taskless.kind: runtime` and the directory has a `check.ts` +- **THEN** the CLI SHALL treat it as a runtime rule with `check.ts` as its check file and route it to the runtime harness + +#### Scenario: Rules under sg remain static + +- **WHEN** a rule file lives under `.taskless/rules/sg/` +- **THEN** the CLI SHALL treat it as a static rule and SHALL NOT route it to the runtime harness + +#### Scenario: An unimplemented match mode is refused + +- **WHEN** a capture rule declares a `match` value this build does not implement +- **THEN** the CLI SHALL NOT load that capture rule +- **AND** SHALL NOT treat it as `anchor` +- **AND** the rule's other capture rules SHALL still load + +#### Scenario: verify explains a refused capture + +- **WHEN** `taskless verify` inspects a rule holding a capture with an unimplemented `match` value +- **THEN** verification SHALL fail +- **AND** the error SHALL name the capture file, the offending value, and the modes this build implements + +### Requirement: A runtime rule has exactly one executable file + +The CLI SHALL refuse a runtime rule whose directory contains any module file other than +`check.ts`. `check.ts` is the only executable surface of a runtime rule and the only artifact +carrying a signature, so any other module would be code reachable from a blessed entry point +without itself being blessed — one relative import away, while tampering with it leaves +`check.ts` still matching its blessed digest. The generator commits to emitting exactly one; +this requirement makes the guarantee enforced rather than trusted. + +"Module file" SHALL cover every extension a check could import, not only `.ts`: the loader +transpiles TypeScript, but a `.js` sibling resolves just as readily. + +The search SHALL be recursive. "One import away" is not "one directory away" — a single +`import "./lib/helper.ts"` reaches an arbitrarily deep relative path in one hop — so a search +bounded to the rule root would let any nested directory carry unsigned code past the check while +`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 +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. + +#### Scenario: A helper module beside check.ts is refused + +- **WHEN** a runtime rule directory contains `check.ts` and any other module file, at any depth +- **THEN** the CLI SHALL refuse the rule and name the offending file +- **AND** SHALL NOT execute its `check.ts` + +#### Scenario: A TypeScript test fixture is not a stray module + +- **WHEN** a runtime rule carries a `.ts` file under its `tests/` directory +- **THEN** the CLI SHALL still discover and run the rule + +### 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 +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 +be an error naming the path, not an ignored entry. Buckets SHALL be read +independently, and a bucket that cannot be read SHALL be an error rather than an +empty bucket. + +**Rationale.** A check is a function over a root and its matches, reading +whatever files under that root it needs, so a case has to be a directory: a +runtime rule exists because its evidence spans more than one file, and a +one-file-per-case layout could not express the rules the tier is for. The +strictness on reading is the Vale runner's, for its reason — a swallowed +permission error on one bucket makes a two-sided rule look one-sided, and a +one-sided rule look complete. + +#### Scenario: A case directory is the harness root + +- **WHEN** a runtime rule's fixture case is executed +- **THEN** the check SHALL receive that case's directory as its `root` +- **AND** SHALL resolve the files it reads beneath that root + +#### Scenario: A loose file in a bucket is refused + +- **WHEN** a fixture bucket contains an entry that is not a directory +- **THEN** the CLI SHALL report an error naming that path +- **AND** SHALL NOT silently skip it + +#### Scenario: An unreadable bucket is not an empty bucket + +- **WHEN** a fixture bucket exists but cannot be read +- **THEN** the CLI SHALL report the failure +- **AND** SHALL NOT treat the bucket as holding no cases diff --git a/openspec/changes/unified-rule-fixtures/specs/cli-taskless-bootstrap/spec.md b/openspec/changes/unified-rule-fixtures/specs/cli-taskless-bootstrap/spec.md new file mode 100644 index 00000000..ca3198cc --- /dev/null +++ b/openspec/changes/unified-rule-fixtures/specs/cli-taskless-bootstrap/spec.md @@ -0,0 +1,67 @@ +## ADDED Requirements + +### 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 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; 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. + +**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 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 file fixed after an earlier run lands beside its cases + +- **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 + +- **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/changes/unified-rule-fixtures/specs/cli-vale-rule-engine/spec.md b/openspec/changes/unified-rule-fixtures/specs/cli-vale-rule-engine/spec.md new file mode 100644 index 00000000..73e7e30c --- /dev/null +++ b/openspec/changes/unified-rule-fixtures/specs/cli-vale-rule-engine/spec.md @@ -0,0 +1,55 @@ +## MODIFIED Requirements + +### Requirement: Vale rules are verified with per-rule fixture subdirectories + +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/` case for a rule produces no finding +- **THEN** verification reports a failure for that rule + +#### Scenario: A one-sided fixture set is not verified + +- **WHEN** a rule has `fail/` fixtures but no `pass/` fixtures, or `pass/` fixtures but no `fail/` +- **THEN** verification reports the rule as unverified rather than passing +- **AND** the result distinguishes a half-written fixture set from a rule with no fixtures at all + +### 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. + +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. + +The rule's config SHALL be named `.vale.ini` rather than carrying a `.yml` extension. Measured: a `.yml` file inside a style directory is loaded as a rule and fails `E201` when the style is enabled wholesale, while a non-`.yml` file in the same directory is ignored. + +Scope SHALL NOT be expressed inside the style file. Measured: Vale rejects unknown top-level keys in a rule with `E201 has invalid keys`. + +#### Scenario: A rule is complete in one directory + +- **WHEN** a rule directory contains its style and its config +- **THEN** the rule is fully defined without editing any shared file + +#### Scenario: The rule config is invisible to Vale's style loader + +- **WHEN** a rule directory contains `.vale.ini` beside its style +- **THEN** Vale SHALL NOT attempt to load it as a rule + +#### Scenario: Deleting a rule is deleting a directory + +- **WHEN** a rule directory is removed +- **THEN** no other rule's scope changes +- **AND** no shared file needs editing diff --git a/openspec/changes/unified-rule-fixtures/tasks.md b/openspec/changes/unified-rule-fixtures/tasks.md new file mode 100644 index 00000000..f6bc0a5a --- /dev/null +++ b/openspec/changes/unified-rule-fixtures/tasks.md @@ -0,0 +1,40 @@ +Stacked, merging forward with `gh stack merge`. The task groups describe the work; the branches group it so each layer is green on its own (see proposal, Delivery shape). Work through every group without pausing, committing once per group (AGENTS.md, OpenSpec Apply). + +## 1. Engine plumbing (bottom branch) + +- [x] 1.1 Pin the measured Vale 3.24.0 behavior (design, Context) in `vale-vendor-contract.test.ts`: with cwd = case and `[docs/**/*.md]`, `docs/guide/x.md` fires and `x.md` does not; the same tree passed from its parent fires nothing; a document under `.github/` is walked. Add the ast-grep equivalents to `ast-grep-vendor-contract.test.ts`. Verify both pass. +- [x] 1.2 Add `.changeset/*.md` (`patch`, `@taskless/cli`) describing the layout move, the scaffold bump to 12, and the `SCAFFOLD_VERSION_MISMATCH` a pinned older CLI will report. Verify `changeset.yml` reports it on the PR. +- [x] 1.3 Make `assembleSgConfig` copy each selected rule's `.yml` into a rules-only tree in the run directory and point `ruleDirs` at it (D2). Verify that `check` on this repository produces identical findings before and after (diff `pnpm cli check --json`), and that a test with a plain `tests/` directory inside an `sg` rule passes `check`. +- [x] 1.4 Keep the `ast-grep-vendor-contract.test.ts` assertion that a plain `tests/` inside `ruleDirs` fails, re-pointed to document why D2 exists. Verify it passes on 0.51.0. +- [x] 1.5 Set `RULE_TESTS_DIRECTORY = "tests"`, narrow `FixtureLayout` to `"case-directories"` for every `ENGINE_LAYOUTS` entry, and rewrite the layout comments. Verify `pnpm typecheck` and that `@taskless/cli/layout` still builds under the host-capability guard. +- [x] 1.6 Move the case-directory bucket reader from `runtime/fixtures.ts` into `rules/fixtures.ts` as the one reader for every engine, refusing loose bucket files and stray `tests/` entries. Verify with the existing loose-file/unreadable-bucket tests moved to cover all three engines. + +## 2. One verify loop (branch 2) + +- [x] 2.1 Add the shared judging loop to `rules/fixtures.ts` (fail ≥1 / pass 0, coverage, `bucket` on findings) over an engine-supplied `runCheck(rule, root)`. Verify with unit tests over a fake engine covering every coverage state. +- [x] 2.2 ast-grep `runCheck`: `runSg` with cwd = case, a one-rule config from 1.3, and `--rule `. Delete `sg-fixture-findings.ts`, the `sg test` path in `verify.ts`, and `test-fixture-findings.test.ts`. Verify that `verify.test.ts` covers a scoped rule firing at the glob path and staying quiet at the case root, and that another rule's bad `language:` does not reach the case. +- [x] 2.3 Vale `runCheck`: cwd = case, a config built from the rule's own `.vale.ini` alone (D1). Verify `vale-verify.test.ts` covers the matcher-scoped scenario from the `cli-vale-rule-engine` delta, and that another rule's refused `.vale.ini` does not fail this rule. +- [x] 2.4 Runtime `runCheck`: route `executeRuntimeRule` through the shared loop and keep the flag gating. Verify `runtime-fixture-runner.test.ts` passes unchanged apart from paths. +- [x] 2.5 The unscoped diagnostic run for a quiet `fail/` case (D3): ast-grep strips `files`/`ignores` from a copy in the run directory, and Vale uses `buildIsolatingConfig`. Verify both messages ("scope excluded" with globs/matchers named, and "did not match") in tests, and that the verdict is unchanged. +- [x] 2.6 Verify writes nothing under `.taskless/rules/` and leaves no run directory without `--preserve-logs`. Verify with a test that snapshots the tree before and after. +- [x] 2.7 Measure `pnpm cli verify` across this repository's rules and record the time per case in the PR, choosing the concurrency cap from it. Measured: 14 rules, about 100 cases, 2.2 s wall clock run sequentially, so no concurrency cap was added. + +## 3. Migration and delivery (branch 3) + +- [x] 3.1 Add `rules/fixture-convert.ts`: the language → extension table, scope → path derivation checked with `path.matchesGlob`, and ordinal case naming (D4). Add a vendor-contract assertion that the table covers every language ast-grep 0.51.0 accepts. Verify with unit tests over this repository's seven scoped/unscoped `sg` rules and a deliberately underivable glob. +- [x] 3.2 Add scaffold migration `0012`: sg snippets → cases, Vale documents → cases, runtime rename, never delete the unconverted, idempotent. Verify with migration tests for every scenario in the `cli-taskless-bootstrap` delta, including a fresh scaffold running 1–12. +- [x] 3.3 Point `rule-migrations/vale-3-24-0-tests-to-fixtures.ts` at the new layout (D5). Verify its existing tests with updated paths. +- [x] 3.4 Delivery: migrate an old-format response just in time. Treat `.tests/` as unsigned fixtures, convert them through `fixture-convert.ts` before writing, name anything unplaceable, and pass a new-layout set through unchanged (D6). Verify with `verify-delivery.test.ts` and deliver tests serving one v2 set of each shape. +- [x] 3.5 Reconcile and signing exclude `tests/` and `.tests/`. Verify with a reconcile test whose rule holds both and reports neither. +- [x] 3.6 Run migration 12 on this repository's `.taskless/rules/`, rename the ordinal cases to descriptions, and confirm every rule still passes `pnpm cli verify`. Fix any fixture that only passed because stdin bypassed `files:`, and list those fixes in the PR body. +- [x] 3.7 Review found that case ordinals restarted in each of a rule's legacy test files, so a rule with two dated files lost a fixture: migration 12 skipped the existing destination and then deleted the source. Claim case names across a rule's files (delivery too), count a destination as written only when it holds the same bytes, and prefer a derived path that keeps the file's extension. Verify with converter and migration tests for two dated files, a conflicting destination, and an interrupted rerun. +- [x] 3.8 A second review round, reproduced: a case the engine never read passed (gitignored path, empty file, out-of-scope path, ast-grep's `*` crossing `/`); delivery merged converted cases into existing ones and moved `sg test` snapshots into `tests/`; migration 12 could not convert a file fixed after an earlier run; `.DS_Store` failed a rule; an empty case passed; a leftover `.tests/` was never reported; a derived path could still change a file's extension; a signal-killed scan read as clean. Add D8 (ast-grep `--inspect entity`, a Vale probe rule) and fix the rest. Verify with tests for each reproduction and `pnpm cli verify` over this repository. + +## 4. Recipes, corpus, and archive (tip branch) + +- [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.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.