diff --git a/.changeset/vale-rule-engine.md b/.changeset/vale-rule-engine.md index 9079c3ce..62bf36b2 100644 --- a/.changeset/vale-rule-engine.md +++ b/.changeset/vale-rule-engine.md @@ -2,13 +2,33 @@ "@taskless/cli": minor --- -Add Vale as a second static-tier rule engine, and rename the agent-facing command. +Add Vale as a second static-tier rule engine, give every engine one rule layout, and rename the agent-facing command. `check` now dispatches by engine and runs ast-grep, Vale, and runtime rules concurrently, merging their findings into one result set. An unavailable Vale reports itself and the other engines still return. A Vale that times out or rejects its config fails the check rather than passing as a clean run. +**Every rule is now one directory**, `.taskless/rules///`, holding +the rule, any per-engine config, and its tests in `.tests/`. Writing a rule +means creating a directory and deleting one means `rm -rf`. Nothing outside it +is touched either way, so concurrent authors never collide on a shared file. + +Vale rules carry their own `.vale.ini` declaring which files they apply to. +The single config Vale reads is assembled from those per-rule files on each +run, gitignored, and regenerated, so hand edits to it have no effect. ast-grep +keeps its `files`/`ignores` inside the rule and needs no second file. + +**`rule verify` is replaced by two path-addressed commands.** `verify ` +checks that a rule has the components its engine requires and needs no tests, +so it works while you're still authoring. `test ` runs the rule's tests, +after running `verify` and stopping if that fails. Both take a rule directory, +an engine directory, or nothing at all for the whole project, and both report +one result per rule. Addressing by path rather than id removes the ambiguity +that arose when two engines held the same rule id. + +Projects on an older layout migrate automatically on the next command. + **BREAKING: `taskless help ` is now `taskless agent `.** The command is named for who reads it. Agents fetching a procedure are not asking for help, and the old name is gone rather than aliased. @@ -30,7 +50,8 @@ copies rather than a phrase it can reorder. The renames: `route` now applies the engine reasoning itself and names a concrete `create-*-rule` topic, so `engine-selection`, `static`, and `existing` are -removed rather than renamed. Their criterion is stated once, in `route`. +removed rather than renamed. Their criterion is stated once, in `route`. Every +authoring recipe is rewritten for the rule-directory layout. **BREAKING for `@taskless/cli/prompts` consumers.** `engine-selection` is no longer exported. `TOPICS` is now `create-sg-rule`, `create-vale-rule`, and diff --git a/.prettierignore b/.prettierignore index 76b675bf..c2a0f1df 100644 --- a/.prettierignore +++ b/.prettierignore @@ -10,3 +10,6 @@ __generated__ # Worktrees are second checkouts; formatting them would touch other branches worktrees/ + +# The demo project: deliberately-wrong source and prose fixtures. +example/ diff --git a/eslint.config.js b/eslint.config.js index f20473a3..113646db 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -28,6 +28,11 @@ export default tseslint.config( // Zero-dependency CommonJS workflow scripts (covered by their own // node:test suite); the app's TS/ESM-oriented rules don't apply. ".github/scripts/", + // The demo project. Its source is deliberately wrong — `example.cjs` + // calls `eval` so a rule has something to find — and its fixtures are + // prose written to be flagged. Linting it fails on content nobody wrote + // as source. `example-project.test.ts` is what keeps it honest. + "example/", ], }, eslint.configs.recommended, diff --git a/example/.taskless/.gitignore b/example/.taskless/.gitignore new file mode 100644 index 00000000..7ceacc38 --- /dev/null +++ b/example/.taskless/.gitignore @@ -0,0 +1,2 @@ +/.vale.ini +/.sgconfig.yml diff --git a/example/.taskless/rules/sg/no-eval/.tests/no-eval-20260814-test.yml b/example/.taskless/rules/sg/no-eval/.tests/no-eval-20260814-test.yml new file mode 100644 index 00000000..88566f2c --- /dev/null +++ b/example/.taskless/rules/sg/no-eval/.tests/no-eval-20260814-test.yml @@ -0,0 +1,7 @@ +id: no-eval +valid: + - "JSON.parse(raw)" + - "const evaluate = () => 1" +invalid: + - "eval(raw)" + - 'eval("(" + raw + ")")' diff --git a/example/.taskless/rules/sg/no-eval/no-eval.yml b/example/.taskless/rules/sg/no-eval/no-eval.yml new file mode 100644 index 00000000..dc546a9a --- /dev/null +++ b/example/.taskless/rules/sg/no-eval/no-eval.yml @@ -0,0 +1,9 @@ +id: no-eval +language: JavaScript +severity: error +message: Avoid eval. It executes whatever string it's handed. +note: | + `eval` runs arbitrary code with the caller's permissions. Parse the value + instead: `JSON.parse` for JSON, a real parser for anything else. +rule: + pattern: eval($$$ARGS) diff --git a/example/.taskless/rules/vale/no-simply/.tests/fail/hedged.md b/example/.taskless/rules/vale/no-simply/.tests/fail/hedged.md new file mode 100644 index 00000000..79b0d5df --- /dev/null +++ b/example/.taskless/rules/vale/no-simply/.tests/fail/hedged.md @@ -0,0 +1,3 @@ +You can simply drop a rule in. + +Just run the check. diff --git a/example/.taskless/rules/vale/no-simply/.tests/pass/direct.md b/example/.taskless/rules/vale/no-simply/.tests/pass/direct.md new file mode 100644 index 00000000..a9245121 --- /dev/null +++ b/example/.taskless/rules/vale/no-simply/.tests/pass/direct.md @@ -0,0 +1,3 @@ +Drop a rule in, then run the check. + +The adjustment took three releases. diff --git a/example/.taskless/rules/vale/no-simply/.vale.ini b/example/.taskless/rules/vale/no-simply/.vale.ini new file mode 100644 index 00000000..a123e1f2 --- /dev/null +++ b/example/.taskless/rules/vale/no-simply/.vale.ini @@ -0,0 +1,6 @@ +# Which files this rule applies to. Adding a rule edits nothing outside this +# directory. That's the point of the layout. +[*.{html,md}] +tskl) rule = no-simply +BasedOnStyles = +no-simply.no-simply = YES diff --git a/example/.taskless/rules/vale/no-simply/no-simply.yml b/example/.taskless/rules/vale/no-simply/no-simply.yml new file mode 100644 index 00000000..8b46bbc7 --- /dev/null +++ b/example/.taskless/rules/vale/no-simply/no-simply.yml @@ -0,0 +1,7 @@ +extends: existence +message: "Avoid '%s'. It tells the reader the work was easy." +level: warning +ignorecase: true +tokens: + - simply + - just diff --git a/example/.taskless/taskless.json b/example/.taskless/taskless.json new file mode 100644 index 00000000..ebf106f0 --- /dev/null +++ b/example/.taskless/taskless.json @@ -0,0 +1,3 @@ +{ + "version": 5 +} diff --git a/example/README.md b/example/README.md new file mode 100644 index 00000000..ffd48460 --- /dev/null +++ b/example/README.md @@ -0,0 +1,106 @@ +# A Taskless install, as it actually looks + +This is a small project with Taskless rules in it. Everything here is real: the +same layout you get after installing, so you can read it before you commit to +anything. + +Two rules, one per engine. + +## The files + +| Path | What it is | +| -------------- | ------------------------------------------------------- | +| `example.cjs` | A CommonJS module that calls `eval` on file contents | +| `example.html` | A page with a Title Case heading and some hedging prose | +| `.taskless/` | The rules. No build output, no cached state. | + +## What a rule looks like + +A rule is **one directory**. It holds everything that defines it. Adding a rule +means adding a directory. Removing one means removing that directory. No shared +file gets edited either way. + +``` +.taskless/rules/ + sg/no-eval/ + no-eval.yml the rule + .tests/no-eval-20260814-test.yml its test cases + vale/no-simply/ + no-simply.yml the rule + .vale.ini which files it applies to + .tests/fail/hedged.md prose it must flag + .tests/pass/direct.md prose it must leave alone +``` + +Two details in there need explaining. + +**`.tests/` is dot-prefixed on purpose.** ast-grep discovers rules by walking +the rules tree, and it reads every `.yml` it finds as a rule. A plain `tests/` +directory would make it parse the test files as rules and fail the whole scan. +A dot-directory gets skipped by that walk. The test runner still finds it. + +**Only Vale has a per-rule `.vale.ini`.** Vale can't express "which files does +this apply to" inside the rule file, because it rejects unknown keys. Scope +needs somewhere else to live. ast-grep puts its equivalent (`files`, `ignores`) +inside the rule, so an `sg` rule gets no second file. + +You won't find a project-wide `.vale.ini` or `sgconfig.yml` here. Both get +assembled from the per-rule configs when a check runs, and both are gitignored. +They're build output. + +## What `check` reports + +``` +$ npx @taskless/cli check + + example.cjs:7:10 + error[no-eval] Avoid eval. It executes whatever string it's handed. + > eval("(" + raw + ")") + note: `eval` runs arbitrary code with the caller's permissions. Parse the value +instead: `JSON.parse` for JSON, a real parser for anything else. + + + example.html:7:11 + warning[no-simply] Avoid 'simply'. It tells the reader the work was easy. + > simply + +2 issues (1 error, 1 warning) across 2 files +``` + +One finding from each engine, merged into one report. The exit code follows +severity, so this run exits 1 on the `error`. + +## Running it yourself, in this repo + +`npx @taskless/cli` fetches the published CLI. To run the one in this +checkout, build it first and then call it from here: + +``` +pnpm --filter @taskless/cli build # from the repo root +cd example +../packages/cli/dist/index.js check +``` + +Substitute that path for `npx @taskless/cli` in every command below. +Note that `check` writes the two assembled configs into `.taskless/`, so +expect them to appear after the first run. Both are gitignored. + +## Checking the rules themselves + +`check` runs rules against your code. Two other commands run against the rules: + +``` +$ npx @taskless/cli verify # are these rules well-formed? +$ npx @taskless/cli test # do they fire where they should, and only there? +``` + +Both take a path: a rule directory, an engine directory, or nothing at all for +everything. `test` runs `verify` first and stops if it fails. That way a broken +rule tells you what's broken. + +## This example is tested + +`packages/cli/test/example-project.test.ts` runs `check`, `verify`, and `test` +against this directory and asserts on what comes back. A demo that's drifted +from the layout it demonstrates is worse than no demo. If the layout changes +and this stops being true, the build fails. diff --git a/example/example.cjs b/example/example.cjs new file mode 100644 index 00000000..98ab4033 --- /dev/null +++ b/example/example.cjs @@ -0,0 +1,14 @@ +// A small CommonJS module with something the ast-grep rule has to say about. +const { readFileSync } = require("node:fs"); + +function loadConfig(path) { + const raw = readFileSync(path, "utf8"); + // `eval` on file contents is the pattern `no-eval` exists to catch. + return eval("(" + raw + ")"); +} + +function greet(name) { + return `Hello, ${name}`; +} + +module.exports = { loadConfig, greet }; diff --git a/example/example.html b/example/example.html new file mode 100644 index 00000000..86d9b44f --- /dev/null +++ b/example/example.html @@ -0,0 +1,11 @@ + +Taskless example + +

Getting Started With The Example

+ +

+ You can simply drop a rule into this project and run a check. The heading + above is Title Case, which the capitalization rule has an opinion about. +

+ +

Read the README for what each file is for.

diff --git a/openspec/changes/archive/2026-08-15-agent-command-and-vale-authoring/resume.md b/openspec/changes/archive/2026-08-15-agent-command-and-vale-authoring/resume.md index e3920023..531739c0 100644 --- a/openspec/changes/archive/2026-08-15-agent-command-and-vale-authoring/resume.md +++ b/openspec/changes/archive/2026-08-15-agent-command-and-vale-authoring/resume.md @@ -180,6 +180,37 @@ Also settled: pre-1.0, every backwards-incompatible change here is a **MINOR** b never MAJOR. The telemetry event stays `cli_help` (agent-call volume stays visible under the existing event). +## PR #103 is stacked above this one + +`openspec/self-contained-rules` (branch still named `openspec/self-contained-vale-rules`), +based on this branch. **Spec-only and green** — proposal, design, four spec deltas, tasks. +`openspec validate self-contained-rules --strict` passes. + +It unifies the rule layout across all three engines: one directory per rule at +`.taskless/rules///` holding the rule, any config that engine needs, and its +tests in `.tests/`. Plus path-addressed `verify`/`test` replacing `rule verify `, and +an `example/` project. + +**Implementation is approved and not started.** Follow `self-contained-rules/tasks.md`. +One caveat that is easy to miss: task 1.5 deletes the legacy read paths, and it exists +because `.taskless/rules/` is simultaneously the new root and the old +`LEGACY_RULES_DIRECTORY`. That was found by starting the refactor, not by writing the +proposal — see design D9. The partial `engines.ts` rewrite was reverted rather than +pushed, so the PR stays spec-only; regenerating it is mechanical from D1/D2 and the +task list, and worth writing *against* D9 rather than patching D9 in afterwards. + +Measured facts the implementation depends on, so nobody re-derives them: + +- ast-grep `ruleDirs` **recurses**; `tests/` and `__tests__/` inside a rule directory + hard-fail the scan; **`.tests/` is skipped**, and `sg test` still reads it via `testDir`. +- Vale resolves `/.yml` as check `.` only under a `StylesPath` naming its + parent — nothing at all under `StylesPath = .`. +- Vale rejects unknown keys in a style (`E201`), so scope cannot live in the style file. +- A `.yml` sidecar in a style directory is loaded as a rule and fails; `.vale.ini` and + `.tests/` in the same place are ignored. +- Migrations run before any read (`ensureTasklessDirectory`), which is why the legacy + paths are unreachable rather than merely stale. + ## Outside this PR - **#99** — migrate subprocess handling to execa (inventory and sequencing already written up) diff --git a/openspec/changes/archive/2026-08-15-self-contained-rules/.openspec.yaml b/openspec/changes/archive/2026-08-15-self-contained-rules/.openspec.yaml new file mode 100644 index 00000000..4af86417 --- /dev/null +++ b/openspec/changes/archive/2026-08-15-self-contained-rules/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-14 diff --git a/openspec/changes/archive/2026-08-15-self-contained-rules/design.md b/openspec/changes/archive/2026-08-15-self-contained-rules/design.md new file mode 100644 index 00000000..b157b63b --- /dev/null +++ b/openspec/changes/archive/2026-08-15-self-contained-rules/design.md @@ -0,0 +1,159 @@ +## Context + +`0004` partitioned `.taskless/` by engine: `sg/rules` + `sg/rule-tests`, `vale/rules` + `vale/rule-tests`, `runtime/rules` + `runtime/rule-tests`. Vale additionally carries a single committed `.vale.ini` where every rule declares its scope, with `tskl) = ` breadcrumbs so tooling can find which matchers belong to which rule. + +`agent-command-and-vale-authoring` then wrote the Vale authoring recipe and executed it five times against sandboxed agents with no repository access. Every silent failure those runs found was in the shared config, and the recipe grew a debug ladder whose first four rungs are all "did you edit the shared file correctly". + +Everything below rests on measurement against the bundled ast-grep 0.41.0 and Vale 3.17.1. + +**ast-grep** + +- `ruleDirs` **recurses**, and every `.yml`/`.yaml` beneath is parsed as a rule. A `tests/` directory inside a rule directory fails the whole run with `Fail to parse yaml as RuleConfig: missing field 'language'`. +- `__tests__/` fails the same way. A **dot-directory is skipped**: `.tests/` inside a rule directory leaves `sg scan` clean. +- `sg test` reads `testDir: /.tests` normally and writes snapshots to `/.tests/__snapshots__/`. + +**Vale** + +- `rules//.yml` resolves as check `.` under `StylesPath` pointing at the rules tree; under `StylesPath = .` it resolves to nothing. +- Unknown top-level keys in a style are rejected (`E201 has invalid keys`), so scope cannot ride inside the style file. +- A `.yml` sidecar inside a style directory is loaded as a rule and fails `E201` when the style is enabled wholesale; a non-`.yml` file is ignored. A `.tests/` directory is harmless even when it contains a `.yml`. + +**runtime** + +- Discovery reads the rule directory non-recursively for `*.yml`, so any subdirectory is already invisible to it. + +## Goals / Non-Goals + +**Goals:** + +- One directory per rule, the same shape for every engine, so "where is this rule" has one answer. +- No file written by more than one rule's author. +- A rule addressable by path, so `verify`/`test` need no id lookup and no ambiguity rule. +- Silent-disable failure modes removed by construction rather than documented. + +**Non-Goals:** + +- Supporting both layouts. There is one legal shape; `0005` moves projects to it. +- A writer for rule configs. The agent authors every committed file; assembly only concatenates. +- Changing what any engine can express. This is where files sit, not what rules do. + +## Decisions + +### D1 — One rule, one directory, every engine + +``` +.taskless/rules/sg/no-eval/ + no-eval.yml + .tests/no-eval-20260101-test.yml + +.taskless/rules/vale/no-simply/ + no-simply.yml + .vale.ini + .tests/pass/ok.md + .tests/fail/bad.md + +.taskless/rules/runtime/unused-exports/ + check.ts + captures/exported-symbol.yml + .tests/… +``` + +The engine is a path segment, so it is still read from position and never from content — the rule `dispatch` already follows. What changes is that a rule is now **one** path rather than two, which is what makes `verify ` and `test ` possible without an id lookup. + +_Alternative rejected:_ mirrored `rules/` and `tests/` trees. Uniform and requires no cleverness, but a rule is two paths again, which is the thing being fixed. + +### D2 — Tests live in `.tests/`, and the dot is load-bearing + +`.tests/` rather than `tests/` because ast-grep's `ruleDirs` recurses and parses every `.yml` beneath as a rule. Measured, `tests/` and `__tests__/` both hard-fail the scan; a dot-directory is skipped, and `sg test` still reads it when `testDir` names it. + +This is a dependency on undocumented behavior and should be recorded as one. Three things make it acceptable: + +- **The failure is loud.** If ast-grep stops skipping dot-directories, the scan fails with a parse error naming the file. It does not silently reinterpret a test as a rule, and it does not silently disable anything. +- **A test pins it.** A fixture with a rule directory containing `.tests/` asserts the scan stays clean, so the assumption is checked on every run rather than remembered. +- **The binary is version-pinned.** `@ast-grep/cli` and every platform package are pinned to an exact version (`0.41.0`), not a range, so this behavior cannot change under a project without a deliberate dependency bump. That bump is where the pinning test fires, which makes the discovery a migration task with a changelog to read rather than a mystery in someone's CI. + +The cost is real: dot-prefixing hides tests from a casual `ls`, and tests are the part of a rule most worth reading. + +_Alternative rejected:_ materialize a rules-only tree and point `ruleDirs` at it, keeping a plain `tests/`. It reaches the same authored layout with no undocumented dependency, and there is precedent — runtime already materializes `.taskless/.run/`. It was rejected for cost: a third assembly step, plus rule paths in ast-grep's own diagnostics pointing at a generated copy rather than the file the author edits. Worth revisiting if the dot-directory assumption ever breaks. + +### D3 — Per-rule configs, assembled per run, gitignored + +Vale accepts exactly one `--config`, and ast-grep one `sgconfig.yml`, so per-rule configuration has to reach a single file before either tool can be invoked. The committed source of truth is per-rule; the file handed to the tool is assembled and gitignored — the same treatment the ephemeral `sgconfig.yml` already receives. + +**Assembly order is a correctness constraint.** Vale's precedence is positional: across matchers the last wins, within one matcher the first assignment wins. Assembly SHALL therefore be deterministic — rules ordered by id, each rule's own matcher order preserved verbatim — or a rule's effective scope would depend on directory iteration order. + +A consequence worth stating: a rule cannot override another rule's matchers, because it cannot know its position. That is the coupling per-rule configs remove. + +_Alternative rejected:_ invoke the tool once per rule. Vale takes one `--config`, so N rules is N process spawns per check and N JSON payloads to merge. + +_Alternative rejected:_ commit the assembled file. It is the shared write-contended file again, arriving by a different route. + +### D4 — No per-rule config for ast-grep + +Vale needs a per-rule config because its scoping cannot live in the style file — measured, `E201`. ast-grep's scoping (`files`, `ignores`) lives *in* the rule, so the equivalent slot has nothing to hold. + +An empty `sg-config` per rule would be symmetry as decoration: a file every author must create, no author ever fills, and every reader must learn to ignore. The symmetry that does hold is one level up — both engines' project configs are assembled and gitignored. + +### D5 — `StylesPath` follows the layout + +`StylesPath` points at the Vale rules tree so each rule directory is a style, giving check name `.`. This is not a free choice: under `StylesPath = .` a nested rule file resolves to nothing at all. + +It reverses the note in `0004`, which calls `StylesPath = rules` wrong. That was correct **for the flat layout**, where pointing StylesPath at `rules/` makes each rule file a style directory with no rules in it and every check silently resolves to nothing. The same setting is right for one layout and silently wrong for the other, so the note must be rewritten rather than deleted — a future reader who finds it will otherwise "fix" it back. + +### D6 — `verify` and `test` are separate, and `verify` is a layer of `test` + +`verify ` answers "is this a well-formed rule"; `test ` answers "does it behave". They split because their preconditions differ: an agent mid-authoring has a rule and no tests yet, and needs the first before it can write the second. + +`test` runs `verify` first and stops on failure. Today the composition is backwards — fixture coverage short-circuits before Vale parses the rule, so a rule with an invalid `level` and a half-written fixture set reports `fixtures: "fail-only"` and never surfaces `'level' must be one of [suggestion warning error]`. The error the author needs is hidden behind the one they do not. + +### D7 — Paths, not ids + +A path names one thing; an id does not. The same id can exist under `sg` and `vale`, which is why the id-addressed command needed an ambiguity error at all. Removing the addressing scheme removes the error case. + +_Alternative rejected:_ keep `rule verify ` as an alias. It preserves the ambiguity case to save typing, in a command invoked from a recipe that can carry a path just as easily. + +### D9 — The legacy read paths are removed, not renamed + +`.taskless/rules/` is the new root. It is also `LEGACY_RULES_DIRECTORY`, the pre-`0004` flat location — the same string, now meaning something else. Found while implementing: the two collide exactly. + +The legacy read paths are removable rather than renameable because they are unreachable. `ensureTasklessDirectory` runs migrations before anything reads a rule, so `0004` has already moved `.taskless/rules/*.yml` to `sg/rules/`, and `0005` moves it again. A "legacy" lookup under the new layout would resolve `.taskless/rules/.yml` inside a tree whose real contents are `rules///` — reading the new root as if it were the old flat directory. + +So `LEGACY_RULES_DIRECTORY` and `LEGACY_RULE_TESTS_DIRECTORY` go, along with their readers in `verify.ts`, `detect/scan.ts`, and `commands/rules.ts`. Their error messages, which name the legacy path as a place a rule might be, go with them. + +_Alternative rejected:_ keep them under a new constant name. It preserves a fallback for a state migrations guarantee cannot exist, and the fallback would now point into the live tree. A stale read path that resolves to a real directory is worse than no fallback. + +### D8 — `captures/`, not `matchers/` + +Runtime's ast-grep capture rules move to `captures/`. "Matcher" now has a precise meaning in the Vale spec — a `[]` ini section — and one word for two unrelated concepts in one `.taskless/` tree is a cost paid at every future reading. + +### D10 — A `consistency` rule's id must be word characters only + +Vale compiles a `consistency` rule's own name into its pattern as a Go RE2 named capture group (`(?P…)`), and RE2 requires a group name to be word characters. Measured against Vale 3.17.1, an id containing `-` fails that file with `E201 … invalid group name`, and because Vale reads one config for the whole run it takes **every** Vale rule in the project down with it: 9 rules, 0 findings, one error. + +Found while re-running the recipe's own worked rules under this layout (task 5.6), where the id became the directory name and the check name at once. + +The layout makes this sharper rather than causing it, since the id is now three things at once, so it is caught in `verify` and stated in the recipe. Kebab-case remains correct for the other ten extension points; only `consistency` is constrained. + +## Risks / Trade-offs + +- **The dot-directory assumption is undocumented** → mitigated by a loud failure mode, a pinning test, and an exact version pin on the binary, so a change arrives with a deliberate upgrade rather than silently (D2). Materialization is the recorded fallback. +- **Assembly is a new failure surface** → a bug there disables rules silently, which is the failure this engine's design exists to prevent. Mitigated by asserting the assembled artifact byte-for-byte and asserting stability across runs. +- **Two migrations touch the same tree in one stack** → `0004` and `0005` are both unreleased and both in this stack, so every consumer runs them as one upgrade and never observes the intermediate layout. The risk is to this repository's own fixtures, which tests cover. +- **The recipe changes again** → `create-vale-rule` teaches the flat layout in detail and its nine worked rules were verified against it. The rule bodies are unaffected; only where the file sits and where scope is declared. The 2b harness re-runs against the new text. +- **Tests are less visible** → dot-prefixing hides the part of a rule most worth reading. `example/` exists partly to counteract this by showing a full rule directory in a place nothing hides. + +## Migration Plan + +`0005` layers on `0004`; neither is released, and both ship in this stack, so consumers run them as a single upgrade. + +0. Note that `0004` has already emptied `.taskless/rules/` by moving it to `sg/rules/`, so the new root is free before `0005` writes into it. `0005` SHALL assert this rather than assume it: a top-level `*.yml` still sitting in `.taskless/rules/` means `0004` did not complete, and writing engine directories around it would interleave two layouts in one tree. +1. Move `/rules/.yml` → `rules///.yml`; for runtime, `runtime/rules//` → `rules/runtime//` with its `*.yml` capture rules into `captures/`. +2. Move `/rule-tests/*` → `rules///.tests/`, preserving each engine's internal test shape. +3. Split the committed `vale/.vale.ini`: each matcher carrying `tskl) rule = ` moves into that rule's own `.vale.ini`. +4. A matcher with **no** `tskl) rule` breadcrumb cannot be attributed. Leave it and report it rather than guessing an owner or dropping it — an unattributable matcher is a user's hand edit, and discarding it silently changes what their check reports. +5. Delete the committed `vale/.vale.ini` and `sg/sgconfig.yml`; gitignore both assembled paths. +6. Content is preserved byte-for-byte throughout: runtime capture bytes determine server-side reconciliation hashes, so a rewrite would invalidate every signature. + +## Open Questions + +- None outstanding. diff --git a/openspec/changes/archive/2026-08-15-self-contained-rules/proposal.md b/openspec/changes/archive/2026-08-15-self-contained-rules/proposal.md new file mode 100644 index 00000000..167bc6b3 --- /dev/null +++ b/openspec/changes/archive/2026-08-15-self-contained-rules/proposal.md @@ -0,0 +1,48 @@ +## Why + +A rule is spread across locations today, and for Vale one of them is shared by every rule in the project. The style is `vale/rules/.yml`, the fixtures are `vale/rule-tests//`, and the scope — the part deciding whether the rule runs at all — is a matcher inside the single committed `vale/.vale.ini`. + +That shared file is where the failures are. Five sandboxed harness runs against `create-vale-rule` found silent failures in that step and nowhere else: an assignment above the first matcher, a glob that missed the fixture's extension, three names that had to agree with nothing reporting when they didn't. A single write-contended config is the wrong shape whether it is hundreds of agents or one agent and a year of rules. + +The same reasoning generalizes past Vale. `sg` and `runtime` rules are also split between a `rules/` tree and a parallel `rule-tests/` tree, so no engine has a single path that means "this rule". Fixing Vale alone would leave three layouts to reason about instead of one. + +Now, because no Vale rules exist yet and `0004` is unreleased. Once either is true in the field, this is a migration with users attached. + +## What Changes + +- **BREAKING** One directory per rule, identical across engines: `.taskless/rules///`, holding the rule, any config that engine requires, and its tests in `.tests/`. +- **BREAKING** A Vale rule carries its own `.vale.ini` with its matchers, exclude directives, and `tskl)` metadata. `check` assembles the run config from every rule's config and gitignores the result. +- **BREAKING** `sgconfig.yml` becomes assembled and gitignored too, pointing `ruleDirs` at the rules tree and `testConfigs` at each rule's `.tests/`. +- **BREAKING** `StylesPath` becomes `rules/vale`, which is what makes each rule directory a Vale *style*. Measured: `/.yml` resolves as check `.` under that StylesPath and resolves to nothing under `StylesPath = .`. +- **BREAKING** `rule verify ` is removed. An id does not name one thing — the same id can exist under two engines — so the id form needed an ambiguity error the path form does not. +- Two path-addressed commands: `verify ` checks a rule has its required components, `test ` runs its tests. Both accept a rule directory or any directory above it, and both run in the rule generation loop. +- `verify` becomes a prerequisite layer of `test`, so a malformed rule reports its own error instead of a fixture complaint. +- Runtime's capture rules move to `captures/`, freeing "matcher" to mean one thing — a Vale `[]` ini section. +- `create-vale-rule` is rewritten against the layout and its worked examples re-verified. +- A committed `example/` project — README, an HTML and a CommonJS file, and a `.taskless/` with one Vale rule and one ast-grep rule — so a reader can see an install rather than infer it from tests that build their own fixtures. + +## Capabilities + +### New Capabilities + +- `cli-rule-validation`: the path-addressed `verify` and `test` commands — how a path resolves to an engine, what each checks per engine, and how they compose in the generation loop. + +### Modified Capabilities + +- `cli-rule-format`: the canonical on-disk shape of every rule, the engine-per-directory rule, and the "committed native config, never generated" requirement that an assembled config now breaks. +- `cli-vale-rule-engine`: the rule is a directory with its own config; the run config is assembled and gitignored; `StylesPath` changes; matcher precedence must survive an assembly step. +- `cli-agent-authoring`: `create-vale-rule` teaches the layout and names `verify`/`test`. **This capability is introduced by `agent-command-and-vale-authoring` (PR #102) and is not yet in `openspec/specs/`**, so this delta modifies a requirement that exists only once #102 archives — expected for a stacked change. + +## Impact + +- `src/filesystem/migrations/0005-*` — layers on `0004` rather than replacing it. Both are unreleased and both are in this stack, so every consumer runs them as one upgrade and never observes the intermediate layout. +- `src/rules/engines.ts` — `ENGINE_LAYOUTS` becomes one rule-directory rule plus per-engine contents. +- `src/rules/vale/run.ts`, `verify.ts` — assembly; the isolating verify config derives from the rule's own. +- `src/rules/dispatch.ts` — `hasValeRules` looks for flat `*.yml` and would read a directory-shaped rule set as "no rules configured". +- `src/filesystem/sgconfig.ts` — assembled rather than committed. +- `src/rules/runtime/discover.ts` — capture rules move to `captures/`. +- `src/commands/rules.ts` — `rule verify` removed; new `verify` and `test` commands. +- `src/help/create-vale-rule.txt`, `verify-rule.txt`, and every recipe naming `rule verify`. +- `.taskless/.gitignore` — the two assembled configs. +- `example/` — new, plus the root tooling ignores that would otherwise walk its deliberately-wrong fixture prose. +- Reverses the `.vale.ini`-writer non-goal in `agent-command-and-vale-authoring/design.md`, which assumed a hand-authored shared config. diff --git a/openspec/changes/archive/2026-08-15-self-contained-rules/specs/cli-agent-authoring/spec.md b/openspec/changes/archive/2026-08-15-self-contained-rules/specs/cli-agent-authoring/spec.md new file mode 100644 index 00000000..37066d7a --- /dev/null +++ b/openspec/changes/archive/2026-08-15-self-contained-rules/specs/cli-agent-authoring/spec.md @@ -0,0 +1,54 @@ +## 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. `pass/` and `fail/` fixture documents under `.taskless/rules/vale//.tests/`. + +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 `verify` and then `test` 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. + +#### 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 `verify` and `test` 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** `verify` SHALL report it + +### Requirement: Authoring recipes write files rather than invoking a writer + +The `create-*-rule` recipes SHALL instruct the agent to write the rule, its configuration, and its fixtures directly. The CLI SHALL NOT provide a command that generates a Vale style file or authors a rule's matchers on the agent's behalf. + +Assembling the run config is not an exception to this. The agent authors every committed file, including the rule's own `.vale.ini`; assembly only concatenates what the agent wrote into the single file Vale's `--config` requires, and adds no scoping decision of its own. + +#### Scenario: No CLI writer for rule configuration + +- **WHEN** an agent authors a Vale rule +- **THEN** it writes that rule's `.vale.ini` itself +- **AND** the CLI SHALL NOT offer a subcommand that authors matchers + +#### Scenario: Assembly makes no scoping decisions + +- **WHEN** the run config is assembled +- **THEN** it SHALL contain only matchers the agent authored, in the order they were authored diff --git a/openspec/changes/archive/2026-08-15-self-contained-rules/specs/cli-rule-format/spec.md b/openspec/changes/archive/2026-08-15-self-contained-rules/specs/cli-rule-format/spec.md new file mode 100644 index 00000000..26be71e7 --- /dev/null +++ b/openspec/changes/archive/2026-08-15-self-contained-rules/specs/cli-rule-format/spec.md @@ -0,0 +1,118 @@ +## ADDED 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/-YYYYMMDD-test.yml` | +| `vale` | `.yml`, `.vale.ini`, `.tests/pass/*`, `.tests/fail/*` | +| `runtime` | `check.ts`, `captures/*.yml`, `.tests/…` | + +One directory per rule is what lets a rule be addressed, reviewed, moved, or deleted as a single thing, and it is what makes `verify ` and `test ` possible without an id lookup. + +Tests SHALL live in `.tests/`, dot-prefixed. This is not cosmetic: ast-grep's `ruleDirs` recurses and parses every `.yml` beneath it as a rule, so a plain `tests/` directory inside a rule directory fails the scan outright. Measured against ast-grep 0.41.0, a dot-directory is skipped by rule discovery while `sg test` still reads it when `testDir` names it. + +#### 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 test YAML 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 a test +- **THEN** it assembles `sgconfig.yml` with `ruleDirs` covering the rules tree and `testConfigs` covering each rule's `.tests/` +- **AND** the assembled file SHALL be gitignored + +#### 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 + +### Requirement: Runtime capture rules live in captures + +A runtime rule's ast-grep capture rules SHALL live in `captures/` inside the rule directory, and `check.ts` SHALL remain at the rule directory's root. + +The name is deliberate. "Matcher" denotes a Vale `[]` config section elsewhere in this system, and one word for two unrelated concepts in one tree is a cost paid at every future reading. + +#### Scenario: Capture rules are found in captures + +- **WHEN** the system discovers a runtime rule +- **THEN** it reads its capture rules from `captures/` +- **AND** it reads `check.ts` from the rule directory root + +### Requirement: A rule's canonical location is what verify and test address + +Each engine SHALL have one canonical on-disk location per rule — the rule directory — and that location SHALL be what `verify` and `test` accept as a path. A directory above it SHALL mean every rule beneath. + +#### Scenario: One address per rule + +- **WHEN** `verify` or `test` is given `.taskless/rules///` +- **THEN** it operates on exactly that rule +- **AND** the engine is determined from the path without reading the rule + +## REMOVED Requirements + +### Requirement: Rules are partitioned into per-engine directories + +**Reason**: Superseded by "Rules are one directory each, partitioned by engine". The old requirement placed a rule as a bare file under an engine directory; a rule is now a directory holding its own file, config, and tests. + +### Requirement: Each engine's committed native config is the source of truth + +**Reason**: Superseded by "Each engine's native config is the source of truth". The committed per-engine config is gone: configs are per-rule and the file each tool reads is assembled per run and gitignored. + +### Requirement: Vale styles live under the rules StyleName + +**Reason**: Superseded by "Vale styles live under a per-rule StyleName". `StylesPath` now points at the Vale rules tree, so each rule directory is its own style and a rule resolves as `.` rather than `rules.`. + +### Requirement: Both the legacy and engine-partitioned layouts are readable + +**Reason**: This change deletes the legacy read paths. Migrations run before any read, so an unmigrated tree cannot reach dispatch, and the legacy constant now names the same string as the new rules root — a stale read path resolving into the live tree is worse than none (design D9). diff --git a/openspec/changes/archive/2026-08-15-self-contained-rules/specs/cli-rule-validation/spec.md b/openspec/changes/archive/2026-08-15-self-contained-rules/specs/cli-rule-validation/spec.md new file mode 100644 index 00000000..f9d25126 --- /dev/null +++ b/openspec/changes/archive/2026-08-15-self-contained-rules/specs/cli-rule-validation/spec.md @@ -0,0 +1,83 @@ +## ADDED Requirements + +### Requirement: Rules are validated and tested by path, not by id + +The CLI SHALL provide `verify ` and `test `. Both SHALL accept a path to a rule's canonical location or to any directory above it, and SHALL resolve the owning engine from the path's position under `.taskless/rules//` rather than by parsing the file. + +An id does not name one thing. The same id can exist under `sg` and under `vale`, so an id-addressed command has to either guess or report an ambiguity; a path has neither problem. Resolving the engine from position — never from content — is the same rule dispatch follows, so a rule cannot be validated by one engine and executed by another. + +#### Scenario: A rule path resolves to its engine + +- **WHEN** `verify .taskless/rules/vale/no-simply` is run +- **THEN** the CLI SHALL validate it as a Vale rule + +#### Scenario: The same id under two engines is not ambiguous + +- **WHEN** `no-simply` exists under both `rules/sg/` and `rules/vale/` +- **THEN** each is addressed by its own path +- **AND** neither command SHALL require the user to disambiguate + +#### Scenario: A directory means everything beneath it + +- **WHEN** `verify .taskless/` is run +- **THEN** every rule beneath it SHALL be validated, each against its own engine +- **AND** the command SHALL report per-rule results rather than a single pass or fail + +#### Scenario: A path outside any engine's rules directory is rejected + +- **WHEN** a path resolves to no engine +- **THEN** the CLI SHALL exit non-zero naming the path, rather than guessing an engine + +### Requirement: Verify checks a rule's required components + +`verify` SHALL check that a rule has the components its engine requires and that they are well formed, and SHALL NOT require fixtures or test cases to exist. + +The two commands split because they have different preconditions. An agent part-way through authoring has a rule and no fixtures yet, and needs to know the rule itself is valid before it can write a meaningful test for it. + +Per engine, `verify` SHALL check: + +| Engine | Components | +|-----------|--------------------------------------------------------------------------------| +| `sg` | `.yml` against the ast-grep schema and the Taskless required fields | +| `vale` | `.yml` against Vale's own validation, and the rule's `.vale.ini` | +| `runtime` | `check.ts` present, and at least one capture rule under `captures/` | + +#### Scenario: A rule with no fixtures still verifies + +- **WHEN** `verify` runs against a rule whose fixture buckets are empty or absent +- **THEN** it SHALL report on the rule's components only +- **AND** the absence of fixtures SHALL NOT be a verify failure + +#### Scenario: A malformed rule reports its own error + +- **WHEN** a Vale style declares a `level` outside `suggestion`/`warning`/`error` +- **THEN** `verify` SHALL report that error, naming the field + +### Requirement: Test runs a rule's fixtures and runs verify first + +`test` SHALL execute a rule against its test material — ast-grep test cases, Vale `pass`/`fail` fixture buckets, or the runtime harness — and SHALL run `verify` first, stopping on a verify 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. + +#### Scenario: A malformed rule reports the malformation, not the fixtures + +- **WHEN** `test` 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: Vale fixtures are tested per bucket + +- **WHEN** `test` runs against a Vale rule +- **THEN** every `fail/` document SHALL produce at least one finding for that rule +- **AND** every `pass/` document SHALL produce none +- **AND** a rule populating only one bucket SHALL be reported as unverified rather than passing + +### Requirement: The generation loop runs verify and test + +The rule generation loop SHALL run `verify` and then `test` against a newly authored or newly delivered rule, and SHALL treat a failure of either as a rule that is not ready to report as complete. + +#### Scenario: A generated rule is checked before it is reported + +- **WHEN** a rule is authored locally or written by the service +- **THEN** the loop SHALL run `verify` and `test` against its path +- **AND** SHALL surface a failure rather than reporting the rule as written diff --git a/openspec/changes/archive/2026-08-15-self-contained-rules/specs/cli-vale-rule-engine/spec.md b/openspec/changes/archive/2026-08-15-self-contained-rules/specs/cli-vale-rule-engine/spec.md new file mode 100644 index 00000000..03fc0957 --- /dev/null +++ b/openspec/changes/archive/2026-08-15-self-contained-rules/specs/cli-vale-rule-engine/spec.md @@ -0,0 +1,137 @@ +## MODIFIED Requirements + +### Requirement: Taskless breadcrumbs use a namespaced ignored key in the Vale config + +Any Taskless-owned breadcrumb the system records in a Vale config SHALL use a `tskl) = ` key. The system SHALL NOT rely on Vale enforcing these keys; they are read only by Taskless tooling, and Vale's ini parser accepts and ignores them. + +With each rule owning its config, a matcher's owner is given by the directory it lives in, so the breadcrumb is no longer needed to locate a rule's matchers. It is retained to mark Taskless-owned matchers **within the assembled file**, where several rules' matchers are interleaved and provenance is otherwise lost. + +#### Scenario: Breadcrumb key is ignored by Vale + +- **WHEN** a config contains a `tskl) rule = no-simply` key +- **THEN** Vale runs normally, ignoring the key, and Taskless tooling can read it back + +#### Scenario: Provenance survives assembly + +- **WHEN** matchers from several rules are assembled into one run config +- **THEN** each SHALL carry the `tskl) rule` key naming the rule it came from + +### Requirement: Vale rules are verified with per-rule fixture subdirectories + +The system SHALL verify a Vale rule from a `.taskless/rules/vale//.tests/` subdirectory containing `pass/` and `fail/` fixture documents. Because verification isolates one rule, the system SHALL generate an ephemeral config enabling only that rule — derived from the rule's own config so that verification exercises the scope the rule actually declares. Verification SHALL assert that every `fail/` fixture produces at least one finding for the rule and every `pass/` fixture produces none (mirroring ast-grep's `invalid`/`valid`). + +#### 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 fails when a fail fixture does not trigger + +- **WHEN** a `fail/` fixture 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 + +## ADDED Requirements + +### Requirement: Vale check executes against an assembled run config over the target paths + +The system SHALL assemble a run config from the per-rule configs and run `vale --config --output=JSON --no-exit` over the resolved target paths. The assembled config SHALL set `StylesPath` naming the Vale rules tree and `MinAlertLevel = suggestion`, so that every finding surfaces to the client for normalization and filtering. + +The config is assembled rather than committed because it has no single author. Every rule contributes its own matchers, and a shared committed file is one every rule's author must edit correctly — which is where the engine's silent failures were found in practice. + +The assembled config SHALL be written where the run can read it and SHALL be gitignored. A generated file that is also committed drifts from its inputs and invites hand edits the next assembly discards. + +#### Scenario: Check runs Vale via the assembled config + +- **WHEN** the CLI runs a check and `.taskless/rules/vale/` contains rule directories +- **THEN** it assembles a run config from their per-rule configs and invokes Vale with it over the target paths + +#### Scenario: The assembled config is not a source file + +- **WHEN** the run config is written +- **THEN** it SHALL be ignored by version control +- **AND** editing it SHALL NOT change what a later check reports + +#### Scenario: No Vale rules present + +- **WHEN** `.taskless/rules/vale/` contains no rule directories +- **THEN** the CLI does not invoke Vale and produces no Vale findings + +### Requirement: Per-rule scoping is expressed in the rule's own Vale config + +The system SHALL express a Vale rule's scope through **matchers** — `[]` sections — declared in that rule's own `.taskless/rules/vale//.vale.ini`. Include is `. = YES`, exclude is `. = NO`. + +Precedence is **positional**, and the system SHALL order matchers accordingly rather than relying on a disable to win on its own. Measured against Vale 3.17.1: + +- Where two matchers both match a file, the **last** one wins for that rule. +- Where the same key is assigned twice inside one matcher — including across duplicate `[]` sections, which Vale merges — the **first** assignment wins. + +A disable therefore SHALL be declared **after** the enable it narrows, within the rule's own config. Because precedence is positional and the run config is assembled, **assembly SHALL be deterministic**: rules ordered by id, and each rule's own matcher order preserved verbatim. A non-deterministic assembly would make a rule's effective scope depend on directory iteration order. + +A rule SHALL NOT be able to override another rule's matchers. It cannot know its own position in the assembled file, and cross-rule overriding through a shared file is the coupling the per-rule layout removes. + +#### Scenario: A rule scopes itself + +- **WHEN** a rule's own config enables it under `[marketing/**]` +- **THEN** the rule produces findings in `marketing/` files and none in `api/` files + +#### Scenario: A rule narrows itself + +- **WHEN** a rule's config enables it under `[marketing/**]` and then disables it under `[marketing/legacy/**]` +- **THEN** the rule fires in `marketing/` but not in `marketing/legacy/` + +#### Scenario: Assembly order is stable + +- **WHEN** the same set of rules is assembled twice +- **THEN** the resulting config SHALL be byte-identical +- **AND** each rule's matchers SHALL appear in the order that rule declared them + +#### Scenario: Duplicate matchers merge + +- **WHEN** two rules each declare a `[*.md]` matcher +- **THEN** both rules run on a matching `.md` file (Vale merges the matchers) + +### 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 + +## REMOVED Requirements + +### Requirement: Vale check executes against the committed config over the target paths + +**Reason**: Superseded by "Vale check executes against an assembled run config over the target paths". There is no committed config: the file Vale reads is assembled from every rule's own config on each run and gitignored. + +### Requirement: Per-rule scoping is expressed via Vale config matchers + +**Reason**: Superseded by "Per-rule scoping is expressed in the rule's own Vale config". Matchers still express scope, but they live in the rule's directory rather than in one shared file every author edits. + +### Requirement: The scaffolded Vale config carries no section + +**Reason**: There is no scaffolded Vale config. `init` creates `.taskless/rules/vale/` and nothing else, because scope is declared per rule; verified against a fresh scaffold, which contains no `.ini` at all. A rule that declares no scope is now a `verify` error rather than a quiet no-op. diff --git a/openspec/changes/archive/2026-08-15-self-contained-rules/tasks.md b/openspec/changes/archive/2026-08-15-self-contained-rules/tasks.md new file mode 100644 index 00000000..ff048ae5 --- /dev/null +++ b/openspec/changes/archive/2026-08-15-self-contained-rules/tasks.md @@ -0,0 +1,80 @@ +# Tasks + +Delivery shape: **stacked, merging down**, on top of `agent-command-and-vale-authoring` (PR #102). The units are only correct together — a layout change without its migration, or a migration without the assemblers, ships a project whose rules silently stop running. Nothing reaches `main` until all of it does. + +`0005` layers on `0004`. Both are unreleased and both ship in this stack, so every consumer runs them as one upgrade and never observes the intermediate layout. + +## 1. The layout + +- [x] 1.1 `ENGINE_LAYOUTS` becomes one rule-directory rule (`rules///`) plus per-engine contents. Every path helper derives from it; no caller reconstructs a path by hand +- [x] 1.2 Pin the dot-directory assumption with a test: a rule directory containing `.tests/` with test YAML in it, asserting `sg scan` completes clean. This is the load-bearing undocumented behavior (design D2) and it must be checked on every run rather than remembered +- [x] 1.3 `hasValeRules` looks for flat `*.yml` under `vale/rules/` and would read a directory-shaped rule set as "no rules configured" — a silent skip of the whole engine. Fix it and add the test that would have caught it +- [x] 1.4 Runtime discovery reads capture rules from `captures/`; `check.ts` stays at the rule root +- [x] 1.5 **Delete `LEGACY_RULES_DIRECTORY` and `LEGACY_RULE_TESTS_DIRECTORY` and their readers** in `rules/verify.ts`, `detect/scan.ts`, and `commands/rules.ts`, including the error messages that name the legacy path as somewhere a rule might live. `.taskless/rules/` is now the new root and the legacy constant is the same string — a stale read path that resolves into the live tree is worse than none. Migrations run before any read (`ensureTasklessDirectory`), so the state they guard against cannot exist (design D9) + +## 2. Assembly + +- [x] 2.1 Assemble the Vale run config from every rule's `.vale.ini`. Deterministic: rules ordered by id, each rule's matchers verbatim in its own order, `StylesPath` and `MinAlertLevel` as the header +- [x] 2.2 Carry each matcher's `tskl) rule = ` breadcrumb through assembly. Provenance is otherwise lost the moment matchers interleave +- [x] 2.3 Assemble `sgconfig.yml`: `ruleDirs` over the rules tree, one `testConfigs` entry per rule's `.tests/` +- [x] 2.4 Gitignore both assembled configs. They are build artifacts; a committed generated file drifts and invites hand edits the next assembly discards +- [x] 2.5 Assert both assembled artifacts byte-for-byte from a known rule set, and assert stability across two runs. This is the new silent-failure surface — a bug here disables rules without saying so +- [x] 2.6 `runVale` and the ast-grep scan run against the assembled configs + +## 3. Migration `0005` + +- [x] 3.0 Assert `.taskless/rules/` holds no top-level `*.yml` before writing engine directories into it. `0004` empties it by moving it to `sg/rules/`, so a file still there means `0004` did not complete, and proceeding would interleave two layouts in one tree +- [x] 3.1 Move `/rules/.yml` → `rules///.yml`; runtime `runtime/rules//` → `rules/runtime//`, its `*.yml` into `captures/` +- [x] 3.2 Move `/rule-tests/*` → `rules///.tests/`, preserving each engine's internal test shape +- [x] 3.3 Split the committed `vale/.vale.ini` — each matcher carrying `tskl) rule = ` into that rule's own config +- [x] 3.4 A matcher with **no** `tskl) rule` breadcrumb cannot be attributed. Leave it and report it rather than guessing an owner or dropping it: an unattributable matcher is a user's hand edit, and discarding it silently changes what their check reports +- [x] 3.5 Delete the committed `vale/.vale.ini` and `sg/sgconfig.yml` +- [x] 3.6 Preserve content byte-for-byte. Runtime capture bytes determine server-side reconciliation hashes, so a rewrite invalidates every signature +- [x] 3.7 Rewrite the `StylesPath` docstring in `0004`. It states `StylesPath = rules` is wrong — true for the flat layout, exactly backwards now. It has to explain both or a future reader will "fix" it back +- [x] 3.8 Idempotent, and a no-op on an already-migrated tree + +## 4. `verify` and `test` + +- [x] 4.1 Resolve a path to an engine from its `` segment, never by parsing the file. A path outside the rules tree is an error naming the path +- [x] 4.2 A directory above a rule means every rule beneath it; report per-rule results rather than one pass/fail +- [x] 4.3 `verify `: ast-grep schema + Taskless required fields; Vale style validation + the rule's `.vale.ini`; runtime `check.ts` plus at least one capture. Tests are NOT required for verify to pass +- [x] 4.4 `test `: ast-grep test cases, Vale `.tests/` buckets, runtime harness. Runs `verify` first and stops on failure — today a malformed rule reports a fixture complaint while the real error goes unmentioned +- [x] 4.5 Delete `rule verify` and the id-based dispatch added in `bc09897`, including `rulefileOwners` and its ambiguity error. The path form has no ambiguity case +- [x] 4.6 Wire both into the rule generation loop +- [x] 4.7 Port `rule-verify-dispatch.test.ts` onto the path-addressed commands; delete the id-addressed tests + +## 5. Recipes + +- [x] 5.1 Rewrite `create-vale-rule.txt` for the layout: rule directory, its own config, `.tests/`, no shared file. The nine worked rules keep their bodies — only where the file sits and where scope is declared changes +- [x] 5.2 Replace the `.vale.ini` walkthrough. The current step teaches editing a shared config and carries the `W101`-outside-a-matcher warning; both describe a situation that no longer exists +- [x] 5.3 Step 6 names `verify` and `test` rather than reading `results[].ruleId` out of `check` +- [x] 5.4 Update `create-sg-rule.txt` for the rule-directory layout, `verify-rule.txt` for both commands, and sweep every recipe naming `rule verify` +- [x] 5.5 Re-run the 2b harness against the rewritten recipes — fresh agents, no repository access, the same three extension points. The recipes changed materially, so prior convergence does not carry over +- [x] 5.6 Re-verify the nine worked rules by extracting the YAML from the *rendered* recipe and executing it. What ships must be what was tested + +## 6. An example project at `/example` + +**Primarily so a person can see what a Taskless install looks like.** The tests cover behavior thoroughly, but they build their fixtures inside the test that reads them — so nothing in the repository shows the layout as a reader would encounter it. `example/` is that: a small, real project someone can open and understand in a minute. + +Its second job is to stop being wrong. A demo that drifts from the layout it demonstrates is worse than none, so a test runs `check` against it — the example rots loudly rather than quietly. It also counteracts `.tests/` being dot-hidden, by showing a complete rule directory somewhere nothing is hidden. + +- [x] 6.1 `example/README.md` — what this is, what each file is for, and what `check` reports against it. Written for someone who has never installed Taskless and wants to see the shape before they do +- [x] 6.2 `example/example.html` and `example/example.cjs` — the prose and the code the rules have something to say about. Small enough to read in one screen +- [x] 6.3 `example/.taskless/` with one Vale rule and one ast-grep rule, each in its canonical directory with its `.tests/` +- [x] 6.4 A test that runs `check` against `example/` and asserts on the findings. This is what keeps the demo honest: a layout change that breaks it fails the build instead of leaving a misleading example in the repo +- [x] 6.5 A test that runs `verify example/.taskless/rules/` and `test example/.taskless/rules/` — the directory form, which is also the CI form +- [x] 6.6 Add `example/` to the root tooling ignores that would otherwise walk it. Its `.tests/` hold deliberately wrong prose, and a root prettier or eslint pass reaching them fails on content nobody wrote as source + +## 7. Verify + +- [x] 7.1 `pnpm typecheck`, `pnpm lint`, `pnpm --filter @taskless/cli build`, `pnpm --filter @taskless/cli test` +- [x] 7.2 End-to-end: author two Vale rules with different globs, confirm each fires only in its own scope, and confirm deleting one directory leaves the other's scope untouched +- [x] 7.3 Confirm a hand-edited assembled config has no effect on the next check — it is regenerated +- [x] 7.4 Migrate a `0004`-shaped fixture through `0005` and confirm the rules still fire, the tests still run, and runtime signatures are unchanged +- [x] 7.5 `pnpm openspec validate --all --strict`. The `cli-agent-authoring` delta modifies a requirement #102 introduces, so this passes cleanly only once #102 archives +- [x] 7.6 Extend the changeset on the bottom of the stack: the rule layout, the removal of `rule verify`, the new `verify`/`test` commands +- [x] 7.7 Archive the change + +**Archive ordering, for the record.** `agent-command-and-vale-authoring` had to archive first: a change archives once on the tip and the gate wants `openspec/changes/` empty, and this change's `cli-agent-authoring` delta modifies a requirement that one introduces, so it needed a target in `specs/`. Both archives land on the tip PR. + +Note on 7.5: `cli-rules` and `cli-update-engine` fail `--strict` on `main` already and are unrelated to this stack (confirmed by diffing both specs against `origin/main` — this stack never touched them). `change/self-contained-rules` itself validates clean. diff --git a/openspec/specs/cli-agent-authoring/spec.md b/openspec/specs/cli-agent-authoring/spec.md index ad09bd03..00b8603a 100644 --- a/openspec/specs/cli-agent-authoring/spec.md +++ b/openspec/specs/cli-agent-authoring/spec.md @@ -23,38 +23,54 @@ A decision procedure that can produce an answer with no destination is incomplet 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 under `.taskless/vale/rules/.yml`. -2. A section in the committed `.taskless/vale/.vale.ini` scoping which files the rule applies to, enabling it as `rules. = YES`. -3. `pass/` and `fail/` fixture documents under `.taskless/vale/rule-tests//`. +1. A Vale style file at `.taskless/rules/vale//.yml`. +2. That rule's own `.taskless/rules/vale//.vale.ini`, declaring the matchers that scope it and enabling it as `. = YES`. +3. `pass/` and `fail/` fixture documents under `.taskless/rules/vale//.tests/`. -The recipe SHALL state that the scaffolded config carries no section, so the first rule authored in a project also authors the first scope. +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 `verify` and then `test` 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. #### Scenario: Authoring produces all three artifacts - **WHEN** the agent follows `create-vale-rule` -- **THEN** it writes the style file, a scoping section enabling the rule, and both fixture buckets +- **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 -#### Scenario: The recipe teaches the first section +- **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 -- **WHEN** a project's `.vale.ini` has no section yet -- **THEN** the recipe SHALL direct the agent to add one scoped to the files the rule is about, rather than assuming a section exists +#### 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 `verify` and `test` against the rule's path #### Scenario: An unscoped rule is not silently accepted -- **WHEN** the agent enables a rule without placing it inside a section -- **THEN** the recipe SHALL identify this as incomplete, because Vale ignores a rule assignment outside a section +- **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** `verify` SHALL report it ### Requirement: Authoring recipes write files rather than invoking a writer -The `create-*-rule` recipes SHALL instruct the agent to write the rule, its configuration, and its fixtures directly. The CLI SHALL NOT provide a command that generates a Vale style file or edits `.vale.ini` on the agent's behalf. +The `create-*-rule` recipes SHALL instruct the agent to write the rule, its configuration, and its fixtures directly. The CLI SHALL NOT provide a command that generates a Vale style file or authors a rule's matchers on the agent's behalf. -This matches how ast-grep rules are authored today: the agent writes the rule and its config entry, and construction belongs to the downstream generator rather than to the CLI. +Assembling the run config is not an exception to this. The agent authors every committed file, including the rule's own `.vale.ini`; assembly only concatenates what the agent wrote into the single file Vale's `--config` requires, and adds no scoping decision of its own. -#### Scenario: No CLI writer for Vale configuration +#### Scenario: No CLI writer for rule configuration - **WHEN** an agent authors a Vale rule -- **THEN** it edits `.vale.ini` itself -- **AND** the CLI SHALL NOT offer a subcommand that performs that edit +- **THEN** it writes that rule's `.vale.ini` itself +- **AND** the CLI SHALL NOT offer a subcommand that authors matchers + +#### Scenario: Assembly makes no scoping decisions + +- **WHEN** the run config is assembled +- **THEN** it SHALL contain only matchers the agent authored, in the order they were authored ### Requirement: The runtime authoring recipe is the logged-out path diff --git a/openspec/specs/cli-rule-format/spec.md b/openspec/specs/cli-rule-format/spec.md index 9eca5e46..3909f7b2 100644 --- a/openspec/specs/cli-rule-format/spec.md +++ b/openspec/specs/cli-rule-format/spec.md @@ -3,23 +3,7 @@ ## Purpose TBD - created by archiving change partition-rules-by-engine. Update Purpose after archive. - ## Requirements - -### Requirement: Rules are partitioned into per-engine directories - -The system SHALL store rules under a top-level engine directory `.taskless//`, each with a `rules/` directory and a `rule-tests/` directory. The `sg` engine SHALL use `sgconfig.yml`; the `vale` engine SHALL use `.vale.ini`; the `runtime` engine SHALL store each rule as a directory `rules//` (capture `*.yml` + `check.ts`) with fixtures under `rule-tests//`. - -#### Scenario: ast-grep engine directory - -- **WHEN** the CLI resolves `.taskless/` -- **THEN** ast-grep rules are found under `.taskless/sg/rules/`, the config is `.taskless/sg/sgconfig.yml`, and tests are under `.taskless/sg/rule-tests/` - -#### Scenario: Vale engine directory - -- **WHEN** the CLI resolves `.taskless/` -- **THEN** Vale styles are found under `.taskless/vale/rules/`, the config is `.taskless/vale/.vale.ini`, and tests are under `.taskless/vale/rule-tests/` - ### Requirement: A rule's engine is determined by its containing directory The system SHALL dispatch each rule to the engine named by its top-level `.taskless//` directory, and SHALL NOT parse a rule file to determine its engine. @@ -29,20 +13,6 @@ The system SHALL dispatch each rule to the engine named by its top-level `.taskl - **WHEN** a rule file exists at `.taskless/sg/rules/no-eval.yml` and another at `.taskless/vale/rules/no-simply.yml` - **THEN** the first is executed by ast-grep and the second by Vale, based solely on directory -### Requirement: Each engine's committed native config is the source of truth - -The system SHALL treat each engine's committed native config as the authoritative definition of its rules, their scoping, and their metadata. The system SHALL NOT require a separate Taskless sidecar or metadata file for a rule, and SHALL NOT generate an engine config at check time. - -#### Scenario: No sidecar or generated config - -- **WHEN** the CLI runs a check -- **THEN** it reads the committed `sg/sgconfig.yml` and `vale/.vale.ini` as-is, and neither writes nor generates an engine config - -#### Scenario: Native scoping is applied by the engine - -- **WHEN** an ast-grep rule declares native `files`/`ignores`, or a Vale `.vale.ini` declares per-rule include/exclude sections -- **THEN** the engine applies that scoping directly, with no Taskless-side rule transformation - ### Requirement: Migration preserves existing ast-grep rules by moving them under sg The migration to the engine-partitioned layout SHALL move the existing `.taskless/rules/`, `.taskless/rule-tests/`, and `.taskless/sgconfig.yml` under `.taskless/sg/` without editing file contents, relying on `sgconfig.yml`'s relative `ruleDirs: [rules]` remaining valid after the move. It SHALL scaffold `.taskless/vale/` and SHALL move `.taskless/runtime-rules/` to `.taskless/runtime/rules/` and `.taskless/runtime-rule-tests/` to `.taskless/runtime/rule-tests/` without editing file contents (preserving runtime capture-rule hashes). Every scaffolded directory that would otherwise be empty SHALL contain a `.gitkeep` file so the structure is tracked reliably. @@ -80,22 +50,6 @@ Absence of an engine and an **unrecognized** engine are distinct. If a payload i - **WHEN** a payload identifies an engine the installed CLI does not support - **THEN** ingest exits with an error naming the engine and directing the user to upgrade, and no rule file is written under any engine directory -### Requirement: Both the legacy and engine-partitioned layouts are readable - -The CLI SHALL dispatch rules found at the legacy `.taskless/rules/` path as ast-grep, in addition to `.taskless/sg/rules/`, so a checkout that has not yet been migrated — or a rule delivered by a service that still names the legacy location — is executed rather than ignored. - -This tolerance is what decouples the CLI's release from any consumer's: a producer may continue to use the pre-migration layout indefinitely and its rules keep running. - -#### Scenario: Unmigrated checkout still runs its rules - -- **WHEN** `check` runs against a `.taskless/` containing `rules/` but no `sg/` -- **THEN** those rules are dispatched to ast-grep and reported, not silently skipped - -#### Scenario: Both layouts present - -- **WHEN** rules exist under both `.taskless/rules/` and `.taskless/sg/rules/` -- **THEN** both are dispatched to ast-grep and their findings merged, with no duplicate reporting of the same rule - ### Requirement: Reconciliation survives the relayout The CLI SHALL report rule files to the reconcile endpoint at their post-migration repo-relative paths. Because the server joins reported files by content signature rather than by path, moving a rule without editing it SHALL NOT change its reconciled state. @@ -119,11 +73,102 @@ When `taskless.json`'s `version` exceeds the highest migration the installed CLI - **WHEN** the same condition holds and `--allow-version-mismatches` is set - **THEN** the CLI proceeds without applying migrations -### Requirement: Vale styles live under the rules StyleName +### 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/-YYYYMMDD-test.yml` | +| `vale` | `.yml`, `.vale.ini`, `.tests/pass/*`, `.tests/fail/*` | +| `runtime` | `check.ts`, `captures/*.yml`, `.tests/…` | + +One directory per rule is what lets a rule be addressed, reviewed, moved, or deleted as a single thing, and it is what makes `verify ` and `test ` possible without an id lookup. + +Tests SHALL live in `.tests/`, dot-prefixed. This is not cosmetic: ast-grep's `ruleDirs` recurses and parses every `.yml` beneath it as a rule, so a plain `tests/` directory inside a rule directory fails the scan outright. Measured against ast-grep 0.41.0, a dot-directory is skipped by rule discovery while `sg test` still reads it when `testDir` names it. + +#### 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 test YAML in it +- **THEN** a scan SHALL complete without attempting to parse those files as rules -The system SHALL place Vale styles under `.taskless/vale/rules/` so that `rules` is Vale's StyleName, with `.vale.ini` configured `StylesPath = .` and `BasedOnStyles = rules`. The Vale check identifier `rules.` SHALL be normalized to `ruleId = ` in results. +#### 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 a test +- **THEN** it assembles `sgconfig.yml` with `ruleDirs` covering the rules tree and `testConfigs` covering each rule's `.tests/` +- **AND** the assembled file SHALL be gitignored + +#### 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/vale/rules/no-simply.yml` -- **THEN** Vale loads it as `rules.no-simply`, and the CLI reports its findings with `ruleId` `no-simply` +- **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 + +### Requirement: Runtime capture rules live in captures + +A runtime rule's ast-grep capture rules SHALL live in `captures/` inside the rule directory, and `check.ts` SHALL remain at the rule directory's root. + +The name is deliberate. "Matcher" denotes a Vale `[]` config section elsewhere in this system, and one word for two unrelated concepts in one tree is a cost paid at every future reading. + +#### Scenario: Capture rules are found in captures + +- **WHEN** the system discovers a runtime rule +- **THEN** it reads its capture rules from `captures/` +- **AND** it reads `check.ts` from the rule directory root + +### Requirement: A rule's canonical location is what verify and test address + +Each engine SHALL have one canonical on-disk location per rule — the rule directory — and that location SHALL be what `verify` and `test` accept as a path. A directory above it SHALL mean every rule beneath. + +#### Scenario: One address per rule + +- **WHEN** `verify` or `test` is given `.taskless/rules///` +- **THEN** it operates on exactly that rule +- **AND** the engine is determined from the path without reading the rule + diff --git a/openspec/specs/cli-rule-validation/spec.md b/openspec/specs/cli-rule-validation/spec.md new file mode 100644 index 00000000..187276d6 --- /dev/null +++ b/openspec/specs/cli-rule-validation/spec.md @@ -0,0 +1,87 @@ +# cli-rule-validation Specification + +## Purpose +TBD - created by archiving change self-contained-rules. Update Purpose after archive. +## Requirements +### Requirement: Rules are validated and tested by path, not by id + +The CLI SHALL provide `verify ` and `test `. Both SHALL accept a path to a rule's canonical location or to any directory above it, and SHALL resolve the owning engine from the path's position under `.taskless/rules//` rather than by parsing the file. + +An id does not name one thing. The same id can exist under `sg` and under `vale`, so an id-addressed command has to either guess or report an ambiguity; a path has neither problem. Resolving the engine from position — never from content — is the same rule dispatch follows, so a rule cannot be validated by one engine and executed by another. + +#### Scenario: A rule path resolves to its engine + +- **WHEN** `verify .taskless/rules/vale/no-simply` is run +- **THEN** the CLI SHALL validate it as a Vale rule + +#### Scenario: The same id under two engines is not ambiguous + +- **WHEN** `no-simply` exists under both `rules/sg/` and `rules/vale/` +- **THEN** each is addressed by its own path +- **AND** neither command SHALL require the user to disambiguate + +#### Scenario: A directory means everything beneath it + +- **WHEN** `verify .taskless/` is run +- **THEN** every rule beneath it SHALL be validated, each against its own engine +- **AND** the command SHALL report per-rule results rather than a single pass or fail + +#### Scenario: A path outside any engine's rules directory is rejected + +- **WHEN** a path resolves to no engine +- **THEN** the CLI SHALL exit non-zero naming the path, rather than guessing an engine + +### Requirement: Verify checks a rule's required components + +`verify` SHALL check that a rule has the components its engine requires and that they are well formed, and SHALL NOT require fixtures or test cases to exist. + +The two commands split because they have different preconditions. An agent part-way through authoring has a rule and no fixtures yet, and needs to know the rule itself is valid before it can write a meaningful test for it. + +Per engine, `verify` SHALL check: + +| Engine | Components | +|-----------|--------------------------------------------------------------------------------| +| `sg` | `.yml` against the ast-grep schema and the Taskless required fields | +| `vale` | `.yml` against Vale's own validation, and the rule's `.vale.ini` | +| `runtime` | `check.ts` present, and at least one capture rule under `captures/` | + +#### Scenario: A rule with no fixtures still verifies + +- **WHEN** `verify` runs against a rule whose fixture buckets are empty or absent +- **THEN** it SHALL report on the rule's components only +- **AND** the absence of fixtures SHALL NOT be a verify failure + +#### Scenario: A malformed rule reports its own error + +- **WHEN** a Vale style declares a `level` outside `suggestion`/`warning`/`error` +- **THEN** `verify` SHALL report that error, naming the field + +### Requirement: Test runs a rule's fixtures and runs verify first + +`test` SHALL execute a rule against its test material — ast-grep test cases, Vale `pass`/`fail` fixture buckets, or the runtime harness — and SHALL run `verify` first, stopping on a verify 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. + +#### Scenario: A malformed rule reports the malformation, not the fixtures + +- **WHEN** `test` 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: Vale fixtures are tested per bucket + +- **WHEN** `test` runs against a Vale rule +- **THEN** every `fail/` document SHALL produce at least one finding for that rule +- **AND** every `pass/` document SHALL produce none +- **AND** a rule populating only one bucket SHALL be reported as unverified rather than passing + +### Requirement: The generation loop runs verify and test + +The rule generation loop SHALL run `verify` and then `test` against a newly authored or newly delivered rule, and SHALL treat a failure of either as a rule that is not ready to report as complete. + +#### Scenario: A generated rule is checked before it is reported + +- **WHEN** a rule is authored locally or written by the service +- **THEN** the loop SHALL run `verify` and `test` against its path +- **AND** SHALL surface a failure rather than reporting the rule as written + diff --git a/openspec/specs/cli-vale-rule-engine/spec.md b/openspec/specs/cli-vale-rule-engine/spec.md index 0c67b282..86ae1f0d 100644 --- a/openspec/specs/cli-vale-rule-engine/spec.md +++ b/openspec/specs/cli-vale-rule-engine/spec.md @@ -13,51 +13,6 @@ The system SHALL treat Vale as a static-tier engine — always run, with no serv - **WHEN** the CLI runs a check while logged out or anonymous - **THEN** Vale rules are executed the same as ast-grep static rules, with no reconcile or signing step -### Requirement: Vale check executes against the committed config over the target paths - -The system SHALL run `vale --config .taskless/vale/.vale.ini --output=JSON --no-exit` over the resolved target paths, reading the committed config and styles as-is. The `.vale.ini` SHALL set `MinAlertLevel = suggestion` so that every finding surfaces to the client for normalization and filtering. - -#### Scenario: Check runs Vale via the committed config - -- **WHEN** the CLI runs a check and `.taskless/vale/` contains rules -- **THEN** it invokes Vale with `--config .taskless/vale/.vale.ini` over the target paths and parses the JSON output - -#### Scenario: No Vale rules present - -- **WHEN** `.taskless/vale/rules/` is empty -- **THEN** the CLI does not invoke Vale and produces no Vale findings - -### Requirement: Per-rule scoping is expressed via Vale config matchers - -The system SHALL express a Vale rule's scope through `.vale.ini` **matchers** — `[]` sections. Include is `rules. = YES`, exclude is `rules. = NO`. - -Precedence is **positional**, and the system SHALL order matchers accordingly rather than relying on a disable to win on its own. Measured against Vale 3.17.1: - -- Where two matchers both match a file, the **last** one wins for that rule. -- Where the same key is assigned twice inside one matcher — including across duplicate `[]` sections, which Vale merges — the **first** assignment wins. - -A disable therefore SHALL be declared **after** the enable it narrows. Duplicate `[]` matchers SHALL be treated as merged, and a rule's scope SHALL NOT be expressed as a repeated assignment of the same key within one glob, since the later assignment is discarded. - -#### Scenario: Duplicate matchers merge - -- **WHEN** two `[*.md]` matchers each enable a different rule -- **THEN** both rules run on a matching `.md` file (Vale merges the matchers) - -#### Scenario: Include scopes a rule to a path - -- **WHEN** a rule is enabled only under `[marketing/**]` -- **THEN** the rule produces findings in `marketing/` files and none in `api/` files - -#### Scenario: A later matcher overrides an earlier one - -- **WHEN** a rule is enabled under `[marketing/**]` and then disabled under `[marketing/legacy/**]` -- **THEN** the rule fires in `marketing/` but not in `marketing/legacy/` - -#### Scenario: Declaration order is significant - -- **WHEN** the same two matchers are declared in the opposite order — `[marketing/legacy/**]` disabling first, `[marketing/**]` enabling second -- **THEN** the rule fires in `marketing/legacy/` as well, because the later enable wins; a disable does not take precedence on its own - ### Requirement: Vale findings map to the scanner-agnostic CheckResult The system SHALL map each Vale finding to a `CheckResult` with `source` `"vale"` and `ruleId` equal to the Vale check name with its `rules.` prefix stripped. Severity SHALL be normalized `error → error`, `warning → warning`, `suggestion → hint`. The system SHALL map `message` from `Message`, `note` from `Description`/`Link`, `range` from `Line`/`Span`, `matchedText` from `Match`, and `fix` from `Action` only when the action is populated. @@ -87,14 +42,12 @@ When the `vale` binary cannot be found or invoked, the system SHALL report that ### Requirement: Vale rules are verified with per-rule fixture subdirectories -The system SHALL verify a Vale rule from a `.taskless/vale/rule-tests//` subdirectory containing `pass/` and `fail/` fixture documents. Because verification is one-time (not per-check), the system SHALL **generate** an ephemeral `.vale.ini` at verify time (StylesPath plus only that rule enabled) rather than requiring a committed one — the subdirectory holds fixtures only. Verification SHALL assert that every `fail/` fixture produces at least one finding for the rule and every `pass/` fixture produces none (mirroring ast-grep's `invalid`/`valid`). - -Both buckets SHALL hold at least one document before a rule can be reported as verified. A `fail/` fixture proves the rule fires; a `pass/` fixture proves it does not over-fire; either alone establishes half the claim. A rule populating only one bucket SHALL be reported as unverified rather than passing, and the report SHALL distinguish that half-written state from a rule carrying no fixtures at all — a rule with only `pass/` fixtures would otherwise pass trivially, on an empty set of expected failures, having never demonstrated that it fires. +The system SHALL verify a Vale rule from a `.taskless/rules/vale//.tests/` subdirectory containing `pass/` and `fail/` fixture documents. Because verification isolates one rule, the system SHALL generate an ephemeral config enabling only that rule — derived from the rule's own config so that verification exercises the scope the rule actually declares. Verification SHALL assert that every `fail/` fixture produces at least one finding for the rule and every `pass/` fixture produces none (mirroring ast-grep's `invalid`/`valid`). -#### Scenario: Fail fixture triggers, pass fixture does not +#### Scenario: Verification isolates the rule under test -- **WHEN** verify runs for a rule and generates an isolating `.vale.ini` enabling only that rule -- **THEN** verification passes because every `fail/` fixture yields a finding and every `pass/` fixture yields none +- **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 fails when a fail fixture does not trigger @@ -109,34 +62,19 @@ Both buckets SHALL hold at least one document before a rule can be reported as v ### Requirement: Taskless breadcrumbs use a namespaced ignored key in the Vale config -Any Taskless-owned breadcrumb the system records in `.vale.ini` SHALL use a `tskl) = ` key. The system SHALL NOT rely on Vale enforcing these keys; they are read only by Taskless tooling, and Vale's ini parser accepts and ignores them. Each Taskless-owned matcher SHALL carry a `tskl) rule = ` key naming its owning rule, so tooling can locate and update the right rule's matchers even when its scoping is split across multiple (possibly duplicate) matchers. +Any Taskless-owned breadcrumb the system records in a Vale config SHALL use a `tskl) = ` key. The system SHALL NOT rely on Vale enforcing these keys; they are read only by Taskless tooling, and Vale's ini parser accepts and ignores them. + +With each rule owning its config, a matcher's owner is given by the directory it lives in, so the breadcrumb is no longer needed to locate a rule's matchers. It is retained to mark Taskless-owned matchers **within the assembled file**, where several rules' matchers are interleaved and provenance is otherwise lost. #### Scenario: Breadcrumb key is ignored by Vale -- **WHEN** `.vale.ini` contains a `tskl) rule = no-simply` key +- **WHEN** a config contains a `tskl) rule = no-simply` key - **THEN** Vale runs normally, ignoring the key, and Taskless tooling can read it back -#### Scenario: Canonical id locates a rule's matchers - -- **WHEN** a rule's scoping spans several matchers each tagged `tskl) rule = no-simply` -- **THEN** tooling can find every matcher owned by `no-simply` by its `tskl) rule` id rather than by glob +#### Scenario: Provenance survives assembly -### Requirement: The scaffolded Vale config carries no section - -The `.vale.ini` written when a project is scaffolded SHALL contain `StylesPath` and `MinAlertLevel` and no section. A project therefore lints nothing with Vale until someone scopes something deliberately. - -An unscoped `[*]` applies every enabled rule to every file the walk reaches, which makes the default the most aggressive scope available rather than the narrowest. Scope is the author's decision, and the scaffold SHALL NOT make it on their behalf. - -#### Scenario: A freshly scaffolded project reports nothing - -- **WHEN** `check` runs against a scaffolded project with a rule file present and no section added -- **THEN** Vale SHALL report no findings -- **AND** the run SHALL NOT be reported as an engine failure - -#### Scenario: Scope is added by the author - -- **WHEN** an author scopes a rule by adding a section -- **THEN** only files matching that section SHALL be subject to it +- **WHEN** matchers from several rules are assembled into one run config +- **THEN** each SHALL carry the `tskl) rule` key naming the rule it came from ### Requirement: Vale diagnostics on a successful run are surfaced as notices @@ -159,3 +97,87 @@ This is a precondition of the section-less scaffold rather than an independent i - **WHEN** Vale exits zero and writes nothing to stderr - **THEN** the CLI SHALL add no notice +### Requirement: Vale check executes against an assembled run config over the target paths + +The system SHALL assemble a run config from the per-rule configs and run `vale --config --output=JSON --no-exit` over the resolved target paths. The assembled config SHALL set `StylesPath` naming the Vale rules tree and `MinAlertLevel = suggestion`, so that every finding surfaces to the client for normalization and filtering. + +The config is assembled rather than committed because it has no single author. Every rule contributes its own matchers, and a shared committed file is one every rule's author must edit correctly — which is where the engine's silent failures were found in practice. + +The assembled config SHALL be written where the run can read it and SHALL be gitignored. A generated file that is also committed drifts from its inputs and invites hand edits the next assembly discards. + +#### Scenario: Check runs Vale via the assembled config + +- **WHEN** the CLI runs a check and `.taskless/rules/vale/` contains rule directories +- **THEN** it assembles a run config from their per-rule configs and invokes Vale with it over the target paths + +#### Scenario: The assembled config is not a source file + +- **WHEN** the run config is written +- **THEN** it SHALL be ignored by version control +- **AND** editing it SHALL NOT change what a later check reports + +#### Scenario: No Vale rules present + +- **WHEN** `.taskless/rules/vale/` contains no rule directories +- **THEN** the CLI does not invoke Vale and produces no Vale findings + +### Requirement: Per-rule scoping is expressed in the rule's own Vale config + +The system SHALL express a Vale rule's scope through **matchers** — `[]` sections — declared in that rule's own `.taskless/rules/vale//.vale.ini`. Include is `. = YES`, exclude is `. = NO`. + +Precedence is **positional**, and the system SHALL order matchers accordingly rather than relying on a disable to win on its own. Measured against Vale 3.17.1: + +- Where two matchers both match a file, the **last** one wins for that rule. +- Where the same key is assigned twice inside one matcher — including across duplicate `[]` sections, which Vale merges — the **first** assignment wins. + +A disable therefore SHALL be declared **after** the enable it narrows, within the rule's own config. Because precedence is positional and the run config is assembled, **assembly SHALL be deterministic**: rules ordered by id, and each rule's own matcher order preserved verbatim. A non-deterministic assembly would make a rule's effective scope depend on directory iteration order. + +A rule SHALL NOT be able to override another rule's matchers. It cannot know its own position in the assembled file, and cross-rule overriding through a shared file is the coupling the per-rule layout removes. + +#### Scenario: A rule scopes itself + +- **WHEN** a rule's own config enables it under `[marketing/**]` +- **THEN** the rule produces findings in `marketing/` files and none in `api/` files + +#### Scenario: A rule narrows itself + +- **WHEN** a rule's config enables it under `[marketing/**]` and then disables it under `[marketing/legacy/**]` +- **THEN** the rule fires in `marketing/` but not in `marketing/legacy/` + +#### Scenario: Assembly order is stable + +- **WHEN** the same set of rules is assembled twice +- **THEN** the resulting config SHALL be byte-identical +- **AND** each rule's matchers SHALL appear in the order that rule declared them + +#### Scenario: Duplicate matchers merge + +- **WHEN** two rules each declare a `[*.md]` matcher +- **THEN** both rules run on a matching `.md` file (Vale merges the matchers) + +### 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/packages/cli/src/commands/check.ts b/packages/cli/src/commands/check.ts index 40b19627..b8cfac20 100644 --- a/packages/cli/src/commands/check.ts +++ b/packages/cli/src/commands/check.ts @@ -3,11 +3,11 @@ import { stat } from "node:fs/promises"; import { defineCommand } from "citty"; import { hasValeRules, runEngines } from "../rules/dispatch"; +import { assembleEngineConfigs } from "../rules/assemble"; import { formatText } from "../util/format"; -import { resolveSgConfigPath } from "../filesystem/sgconfig"; import { ensureTasklessDirectory } from "../filesystem/directory"; import { - discoverAstGrepRuleSources, + listRuleIds, planEngineDispatch, } from "../rules/engines"; import { getTelemetry } from "../telemetry"; @@ -332,7 +332,7 @@ export const checkCommand = defineCommand({ // executor, so none of them can be assumed to contribute nothing. A // directory that is not a known engine is still ignored rather than // handed to someone's parser. - const astGrepSources = await discoverAstGrepRuleSources(cwd); + const astGrepRuleIds = await listRuleIds(cwd, "sg"); // Both halves matter: `executor` alone is read from the static layout // table and is therefore always `runtime-harness`, so gating on it only // would make this unconditionally true and the presence check decorative. @@ -347,13 +347,13 @@ export const checkCommand = defineCommand({ : []; // "No rules configured" has to mean *no engine* has any, not just these - // two: a project whose only rules live in `.taskless/vale/rules/` would + // two: a project whose only rules live in `.taskless/rules/vale/` would // otherwise return here and Vale would never be dispatched, which is a // silent skip of the engine the user actually configured. Asked last and // short-circuited, so the ordinary project with ast-grep or runtime rules // pays nothing and `runEngines` still owns the decision to spawn Vale. const noRuleFiles = - astGrepSources.length === 0 && + astGrepRuleIds.length === 0 && runtimeRules.length === 0 && !(await hasValeRules(cwd)); @@ -389,13 +389,14 @@ export const checkCommand = defineCommand({ // Every engine runs concurrently and merges into one result set. An // engine that cannot run reports a notice and the others still return. - const astGrepConfigPaths = await Promise.all( - astGrepSources.map((source) => resolveSgConfigPath(cwd, source)) - ); + // Assemble both engine configs from the per-rule tree. Each returns + // `undefined` when its engine has no rules, which dispatch reads as + // "nothing to run" rather than running an empty config. + const assembled = await assembleEngineConfigs(cwd); const dispatched = await runEngines({ cwd, paths: existingPaths, - astGrepConfigPaths, + astGrepConfigPath: assembled.sg, runtimeRules: plan.execute, runtimeTimeoutMs: parseTimeoutMs(args.timeout), }); diff --git a/packages/cli/src/commands/rules.ts b/packages/cli/src/commands/rules.ts index b1d8b477..a056db0e 100644 --- a/packages/cli/src/commands/rules.ts +++ b/packages/cli/src/commands/rules.ts @@ -5,9 +5,6 @@ import { defineCommand } from "citty"; import { ZodError } from "zod"; import { resolveIdentity } from "../auth/identity"; -import { verifyRule } from "../rules/verify"; -import { verifyValeRule } from "../rules/vale/verify"; -import { rulefileOwners, ruleFileLocation } from "../rules/owner"; import { submitRule, pollRuleStatus, iterateRule } from "../api/rules"; import { writeRuleFile, @@ -16,7 +13,7 @@ import { readRuleMetaFile, deleteRuleFiles, } from "../rules/files"; -import { ENGINE_LAYOUTS, LEGACY_RULES_DIRECTORY } from "../rules/engines"; +import { RULES_DIRECTORY } from "../rules/engines"; import { inputSchema as createInputSchema, outputSchema as createOutputSchema, @@ -26,10 +23,6 @@ import { outputSchema as improveOutputSchema, } from "../schemas/rules-improve"; import { outputSchema as metaOutputSchema } from "../schemas/rules-meta"; -import { - verifyOutputSchema, - valeVerifyOutputSchema, -} from "../schemas/rules-verify"; import { getTelemetry } from "../telemetry"; import { CLIError } from "../util/cli-error"; import { type CLIErrorCode, makeErrorEnvelope } from "../types/errors"; @@ -667,9 +660,7 @@ const deleteCommand = defineCommand({ } success = true; } else { - const message = - `Rule "${id}" not found in .taskless/${ENGINE_LAYOUTS.sg.rulesDirectory}/${id}.yml ` + - `or .taskless/${LEGACY_RULES_DIRECTORY}/${id}.yml`; + const message = `Rule "${id}" not found in .taskless/${RULES_DIRECTORY}/sg/${id}/`; if (args.json) { console.log( JSON.stringify(makeErrorEnvelope("RULE_NOT_FOUND", message)) @@ -688,227 +679,6 @@ const deleteCommand = defineCommand({ }, }); -/** - * Verify a Vale rule against its fixture buckets. - * - * Split out because the two engines answer different questions and their - * reports share no fields beyond `ruleId`/`success`. Vale's failure modes are - * about the *fixtures*: a bucket nobody populated proves nothing, and reporting - * that as a pass is how an unverified rule ships looking verified. - */ -async function verifyValeRuleCommand( - cwd: string, - ruleId: string, - json: boolean -): Promise { - let result; - try { - result = await verifyValeRule(cwd, ruleId); - } catch (error) { - // A nested fixture directory throws rather than being skipped: Vale lints - // the rule's whole tree, so a nested document is linted but never checked - // against a bucket, and silently ignoring it would let half a rule's - // fixtures go unverified while it reported a pass. - const message = error instanceof Error ? error.message : String(error); - if (json) { - console.log(JSON.stringify(makeErrorEnvelope("INVALID_INPUT", message))); - } else { - console.error(`Error: ${message}`); - } - process.exitCode = 1; - return; - } - - if ("outcome" in result) { - // Vale never ran. Not a verification result, and not a pass. - const { outcome } = result; - if (json) { - console.log( - JSON.stringify( - makeErrorEnvelope( - outcome.status === "unavailable" - ? "ENGINE_UNAVAILABLE" - : "SCAN_FAILED", - outcome.message - ) - ) - ); - } else { - console.error(`Error: ${outcome.message}`); - } - process.exitCode = 1; - return; - } - - if (json) { - console.log( - JSON.stringify( - valeVerifyOutputSchema.parse({ - engine: "vale", - success: result.passed, - ruleId: result.ruleId, - fixtures: result.fixtures, - missingFailures: result.missingFailures, - unexpectedFindings: result.unexpectedFindings, - }) - ) - ); - } else { - console.log(`Verifying Vale rule: ${result.ruleId}\n`); - - const coverage: Record = { - both: "✓ pass/ and fail/ both have documents", - "pass-only": "✗ no fail/ fixtures — the rule is never shown to fire", - "fail-only": - "✗ no pass/ fixtures — the rule is never shown to stay quiet", - none: "✗ no fixtures at all", - }; - console.log(`Fixtures: ${coverage[result.fixtures]}`); - - console.log( - `Fires: ${result.missingFailures.length === 0 ? "✓ every fail/ document was flagged" : "✗ some fail/ documents were not flagged"}` - ); - for (const file of result.missingFailures) { - console.log(` - ${file}`); - } - - console.log( - `Stays quiet: ${result.unexpectedFindings.length === 0 ? "✓ no pass/ document was flagged" : "✗ some pass/ documents were flagged"}` - ); - for (const file of result.unexpectedFindings) { - console.log(` - ${file}`); - } - - console.log( - `\nResult: ${result.passed ? "✓ All checks passed" : "✗ Verification failed"}` - ); - } - - if (!result.passed) { - process.exitCode = 1; - } -} - -const verifyCommand = defineCommand({ - meta: { - name: "verify", - description: - "Validate a rule and run its tests (ast-grep) or its fixtures (Vale)", - }, - args: { - dir: { - type: "string", - alias: "d", - description: "Working directory", - }, - json: { - type: "boolean", - description: "Output as JSON", - default: false, - }, - anonymous: { - type: "boolean", - description: "Accepted for compatibility; verify is purely local", - default: false, - }, - id: { - type: "positional", - description: "Rule ID to verify", - required: false, - }, - }, - async run({ args }) { - const cwd = resolve(args.dir ?? process.cwd()); - - if (!args.id) { - if (args.json) { - console.log( - JSON.stringify( - makeErrorEnvelope("INVALID_INPUT", "Rule ID is required.") - ) - ); - } else { - console.error( - "Error: Rule ID is required.\n Usage: taskless rule verify " - ); - } - process.exitCode = 1; - return; - } - - // Which engine owns the rule is decided by where its file sits, the same - // way `dispatch` decides it, so a rule cannot be verified by one engine and - // run by another. - const ruleId = args.id; - const owners = await rulefileOwners(cwd, ruleId); - - if (owners.length > 1) { - // Both engines hold this id. Verifying one silently would report on a - // file the user may not have meant, so name both and let them say which. - const message = - `Rule "${ruleId}" exists for more than one engine: ` + - owners.map((engine) => ruleFileLocation(engine, ruleId)).join(", ") + - ". Rename one so the id identifies a single rule."; - if (args.json) { - console.log( - JSON.stringify(makeErrorEnvelope("INVALID_INPUT", message)) - ); - } else { - console.error(`Error: ${message}`); - } - process.exitCode = 1; - return; - } - - if (owners[0] === "vale") { - await verifyValeRuleCommand(cwd, ruleId, args.json); - return; - } - - // No owner falls through to the ast-grep verifier, which already reports a - // missing rule file across its layers — including the legacy location. - const result = await verifyRule(cwd, ruleId); - - if (args.json) { - console.log(JSON.stringify(verifyOutputSchema.parse({ engine: "sg", ...result }))); - } else { - console.log(`Verifying rule: ${result.ruleId}\n`); - - // Layer 1 - console.log( - `Schema: ${result.schema.valid ? "✓ valid" : "✗ invalid"}` - ); - for (const error of result.schema.errors) { - console.log(` - ${error}`); - } - - // Layer 2 - console.log( - `Requirements: ${result.requirements.valid ? "✓ valid" : "✗ invalid"}` - ); - for (const error of result.requirements.errors) { - console.log(` - ${error}`); - } - - // Layer 3 - console.log( - `Tests: ${result.tests.valid ? "✓ passed" : "✗ failed"} (${String(result.tests.passed)} passed, ${String(result.tests.failed)} failed)` - ); - for (const error of result.tests.errors) { - console.log(` - ${error}`); - } - - console.log( - `\nResult: ${result.success ? "✓ All checks passed" : "✗ Verification failed"}` - ); - } - - if (!result.success) { - process.exitCode = 1; - } - }, -}); - export const ruleCommand = defineCommand({ meta: { name: "rule", @@ -919,6 +689,5 @@ export const ruleCommand = defineCommand({ improve: improveCommand, meta: metaCommand, delete: deleteCommand, - verify: verifyCommand, }, }); diff --git a/packages/cli/src/commands/verify.ts b/packages/cli/src/commands/verify.ts new file mode 100644 index 00000000..7b0a4203 --- /dev/null +++ b/packages/cli/src/commands/verify.ts @@ -0,0 +1,158 @@ +import { resolve } from "node:path"; + +import { defineCommand } from "citty"; + +import { ensureTasklessDirectory } from "../filesystem/directory"; +import { + testOneRule, + verifyOneRule, + type RuleTestResult, + type RuleVerification, +} from "../rules/inspect"; +import { + PathOutsideRulesError, + resolveRulePath, + RuleNotFoundError, +} from "../rules/resolve-path"; +import { makeErrorEnvelope } from "../types/errors"; + +/** + * The shared body of `verify` and `test`. + * + * Both take a path, resolve it to rules, and report per rule — they differ only + * in what they run against each. Keeping one implementation means the two can + * never disagree about what a path means, which is the property that makes + * `verify ` and `test ` interchangeable in a recipe. + */ +async function runOverPath(options: { + cwd: string; + target: string; + json: boolean; + /** What the command is called, for messages. */ + label: "verify" | "test"; + run: ( + cwd: string, + rule: { engine: "sg" | "vale" | "runtime"; ruleId: string } + ) => Promise; +}): Promise { + const { cwd, target, json, label, run } = options; + + await ensureTasklessDirectory(cwd); + + let rules; + try { + rules = await resolveRulePath(cwd, target); + } catch (error) { + if ( + error instanceof PathOutsideRulesError || + error instanceof RuleNotFoundError + ) { + const message = error.message; + if (json) { + console.log( + JSON.stringify(makeErrorEnvelope("INVALID_INPUT", message)) + ); + } else { + console.error(`Error: ${message}`); + } + process.exitCode = 1; + return; + } + throw error; + } + + if (rules.length === 0) { + // Not an error: an empty rules tree is the ordinary state of a project that + // has not written a rule yet, and failing here would make `verify` unusable + // in CI on a fresh install. + if (json) { + console.log(JSON.stringify({ ok: true, rules: [] })); + } else { + console.log(`No rules found under ${target}.`); + } + return; + } + + const results = []; + for (const rule of rules) { + results.push(await run(cwd, rule)); + } + + const failed = results.filter((result) => !result.ok); + + if (json) { + console.log(JSON.stringify({ ok: failed.length === 0, rules: results })); + } else { + for (const result of results) { + const mark = result.ok ? "✓" : "✗"; + console.log(`${mark} ${result.engine}/${result.ruleId}`); + for (const error of result.errors) { + console.log(` ${error}`); + } + } + console.log( + failed.length === 0 + ? `\n${String(results.length)} rule(s) ${label === "verify" ? "verified" : "tested"}.` + : `\n${String(failed.length)} of ${String(results.length)} rule(s) failed.` + ); + } + + if (failed.length > 0) process.exitCode = 1; +} + +const ruleTargetArguments = { + dir: { + type: "string", + alias: "d", + description: "Working directory", + }, + json: { + type: "boolean", + description: "Output as JSON", + default: false, + }, + path: { + type: "positional", + description: + "Rule directory, engine directory, or .taskless/rules for everything", + required: false, + }, +} as const; + +export const verifyCommand = defineCommand({ + meta: { + name: "verify", + description: "Check that a rule has the components its engine requires", + }, + args: ruleTargetArguments, + async run({ args }) { + const cwd = resolve(args.dir ?? process.cwd()); + await runOverPath({ + cwd, + // No path means the whole rules tree, which is what CI wants and what an + // author means when they ask "is everything here valid". + target: args.path ?? ".taskless/rules", + json: args.json, + label: "verify", + run: verifyOneRule, + }); + }, +}); + +export const testCommand = defineCommand({ + meta: { + name: "test", + description: "Run a rule's tests, after verifying the rule itself", + }, + args: ruleTargetArguments, + async run({ args }) { + const cwd = resolve(args.dir ?? process.cwd()); + await runOverPath({ + cwd, + target: args.path ?? ".taskless/rules", + json: args.json, + label: "test", + run: testOneRule, + }); + }, +}); diff --git a/packages/cli/src/detect/scan.ts b/packages/cli/src/detect/scan.ts index 56637a5f..0da166a0 100644 --- a/packages/cli/src/detect/scan.ts +++ b/packages/cli/src/detect/scan.ts @@ -4,7 +4,7 @@ import { resolve } from "node:path"; import { parse as parseToml } from "smol-toml"; -import { ENGINE_LAYOUTS, LEGACY_RULES_DIRECTORY } from "../rules/engines"; +import { RULES_DIRECTORY } from "../rules/engines"; export interface DetectedLinter { name: string; @@ -429,10 +429,7 @@ function detectRuleStyles( nodeManifests: NodeManifest[] ): RuleStyle[] { const ruleStyles: RuleStyle[] = []; - for (const source of [ - `.taskless/${ENGINE_LAYOUTS.sg.rulesDirectory}`, - `.taskless/${LEGACY_RULES_DIRECTORY}`, - ]) { + for (const source of [`.taskless/${RULES_DIRECTORY}/sg`]) { if (!existsSync(resolve(root, source))) continue; ruleStyles.push({ source, diff --git a/packages/cli/src/filesystem/migrate.ts b/packages/cli/src/filesystem/migrate.ts index f954fe27..77bf2c67 100644 --- a/packages/cli/src/filesystem/migrate.ts +++ b/packages/cli/src/filesystem/migrate.ts @@ -7,6 +7,7 @@ import init from "./migrations/0001-init"; import installMigration from "./migrations/0002-install"; import dropInstalledAt from "./migrations/0003-drop-installed-at"; import valeEngine from "./migrations/0004-vale-engine"; +import ruleDirectories from "./migrations/0005-rule-directories"; export interface TasklessInstallTarget { skills?: string[]; @@ -37,6 +38,7 @@ const migrations: Migrations = { "2": installMigration, "3": dropInstalledAt, "4": valeEngine, + "5": ruleDirectories, }; /** Global flag that downgrades a too-new scaffold from an error to a skip. */ diff --git a/packages/cli/src/filesystem/migrations/0004-vale-engine.ts b/packages/cli/src/filesystem/migrations/0004-vale-engine.ts index fe3c9b88..27873590 100644 --- a/packages/cli/src/filesystem/migrations/0004-vale-engine.ts +++ b/packages/cli/src/filesystem/migrations/0004-vale-engine.ts @@ -25,15 +25,20 @@ const SG_CONFIG_CONTENT = `ruleDirs:\n - rules\ntestConfigs:\n - testDir: rule /** * The scaffolded `.vale.ini`. * - * `StylesPath` is the engine directory, NOT `rules/`. Vale treats StylesPath as - * a directory *of styles*, so a rule at `vale/rules/no-simply.yml` is the - * `no-simply` rule of the `rules` style, and its check is `rules.no-simply` — - * which is the name `stripRulesPrefix` in `vale/map.ts` exists to undo, and the - * shape `verify.ts` generates. Pointing StylesPath at `rules/` instead makes + * `StylesPath` is the engine directory, NOT `rules/` — **for this layout**. + * Vale treats StylesPath as a directory *of styles*, so with rules flat in + * `vale/rules/`, `rules` is the style and a rule at `vale/rules/no-simply.yml` + * is the check `rules.no-simply`. Pointing StylesPath at `rules/` instead makes * that same file a style directory with no rules in it: every check resolves to * nothing, Vale reports `{}`, and a prose check passes clean with every rule - * silently disabled. Measured against the real binary, which is the only way - * this is visible — the layout is identical either way. + * silently disabled. + * + * **Do not carry that conclusion forward.** Migration `0005` moves each rule + * into its own directory, and there `StylesPath = rules/vale` is the *correct* + * setting and `.` is the one that resolves nothing — the exact reverse. + * StylesPath is a function of the layout, and the same value is right for one + * and silently wrong for the other. Both measured against the real binary, + * which is the only way either is visible: the files look identical. * * It carries **no section**, so a scaffolded project lints nothing until an * author scopes something deliberately. An unscoped `[*]` would apply every diff --git a/packages/cli/src/filesystem/migrations/0005-rule-directories.ts b/packages/cli/src/filesystem/migrations/0005-rule-directories.ts new file mode 100644 index 00000000..77070dca --- /dev/null +++ b/packages/cli/src/filesystem/migrations/0005-rule-directories.ts @@ -0,0 +1,350 @@ +import { mkdir, readdir, readFile, rename, rm, writeFile } from "node:fs/promises"; +import { dirname, join } from "node:path"; + +import type { Migration } from "../types"; +import { CLIError } from "../../util/cli-error"; +import { ENGINES, RULE_TESTS_DIRECTORY, RULES_DIRECTORY } from "../../rules/engines"; + +/** + * Where `0004` left each engine's rules and tests, relative to `.taskless/`. + * This migration reads from here and writes to `rules///`. + */ +const PRIOR_LAYOUT = { + sg: { rules: "sg/rules", tests: "sg/rule-tests" }, + vale: { rules: "vale/rules", tests: "vale/rule-tests" }, + runtime: { rules: "runtime/rules", tests: "runtime/rule-tests" }, +} as const; + +/** The committed configs `0004` wrote, which assembly replaces. */ +const PRIOR_CONFIGS = ["sg/sgconfig.yml", "vale/.vale.ini"] as const; + +/** Files the migration adds to `.taskless/.gitignore`. */ +const GENERATED_PATHS = ["/.vale.ini", "/.sgconfig.yml"] as const; + +/** Directory entries, or `[]` when the directory is not there. */ +async function entriesOf(directory: string) { + try { + return await readdir(directory, { withFileTypes: true }); + } catch { + return []; + } +} + +/** + * Move `source` to `destination`, creating the parent and preserving bytes. + * + * Content is never rewritten. Runtime capture bytes determine their + * server-side reconciliation hashes, so a reformat here would invalidate every + * signature — and the same guarantee is what lets this migration run against a + * project whose rules are already blessed. + */ +async function move(source: string, destination: string): Promise { + await mkdir(dirname(destination), { recursive: true }); + await rename(source, destination); +} + +/** + * Refuse to write engine directories into a `.taskless/rules/` that still holds + * loose rule files. + * + * `.taskless/rules/` is both this layout's root and the pre-`0004` flat + * location, so the two occupy the same path. `0004` empties it by moving it to + * `sg/rules/`; a `*.yml` still sitting there means `0004` did not complete, and + * creating `rules/sg/` around it would interleave two layouts in one tree with + * no way to tell them apart afterwards. + */ +async function assertRootIsFree(directory: string): Promise { + const root = join(directory, RULES_DIRECTORY); + const entries = await entriesOf(root); + const stray = entries + .filter((entry) => entry.isFile() && entry.name.endsWith(".yml")) + .map((entry) => entry.name); + if (stray.length === 0) return; + + throw new CLIError( + `Cannot create the rule directories: .taskless/${RULES_DIRECTORY}/ still contains ` + + `${stray.join(", ")} from the pre-migration layout. Migration 0004 moves those to ` + + `.taskless/sg/rules/; run it to completion first.`, + "SCAFFOLD_CONFLICT" + ); +} + +/** + * Move one engine's rules into per-rule directories. + * + * `sg` and `vale` hold a flat `.yml` per rule; `runtime` already holds a + * directory per rule, whose loose `*.yml` capture rules move down into + * `captures/`. + */ +async function moveEngineRules( + directory: string, + engine: (typeof ENGINES)[number] +): Promise { + const from = join(directory, PRIOR_LAYOUT[engine].rules); + + for (const entry of await entriesOf(from)) { + if (entry.name === ".gitkeep") continue; + + if (engine === "runtime") { + if (!entry.isDirectory()) continue; + const ruleDirectory = join(directory, RULES_DIRECTORY, engine, entry.name); + await move(join(from, entry.name), ruleDirectory); + + // Capture rules move under `captures/`; `check.ts` stays at the root. + const captures = join(ruleDirectory, "captures"); + for (const inner of await entriesOf(ruleDirectory)) { + if (!inner.isFile()) continue; + if (!inner.name.endsWith(".yml") && !inner.name.endsWith(".yaml")) { + continue; + } + await move( + join(ruleDirectory, inner.name), + join(captures, inner.name) + ); + } + continue; + } + + if (!entry.isFile() || !entry.name.endsWith(".yml")) continue; + const ruleId = entry.name.slice(0, -".yml".length); + await move( + join(from, entry.name), + join(directory, RULES_DIRECTORY, engine, ruleId, entry.name) + ); + } +} + +/** + * Move one engine's tests into each rule's `.tests/`. + * + * The two shapes differ and both are preserved as-is: `sg` names its tests + * `-YYYYMMDD-test.yml` in one flat directory, while `vale` keeps a + * `/` subdirectory of `pass/` and `fail/` documents. + */ +async function moveEngineTests( + directory: string, + engine: (typeof ENGINES)[number] +): Promise { + const from = join(directory, PRIOR_LAYOUT[engine].tests); + + for (const entry of await entriesOf(from)) { + if (entry.name === ".gitkeep") continue; + + if (entry.isDirectory()) { + // vale / runtime: a directory per rule. + await move( + join(from, entry.name), + join(directory, RULES_DIRECTORY, engine, entry.name, RULE_TESTS_DIRECTORY) + ); + continue; + } + + // sg: `-YYYYMMDD-test.yml`, so the id is everything before the first + // `-` that begins the timestamp suffix. + const match = /^(?.+?)-\d{8}-test\.ya?ml$/.exec(entry.name); + const ruleId = match?.groups?.id; + if (ruleId === undefined) continue; + await move( + join(from, entry.name), + join( + directory, + RULES_DIRECTORY, + engine, + ruleId, + RULE_TESTS_DIRECTORY, + entry.name + ) + ); + } +} + +/** + * Create `rules//` for every engine, tracked when empty. + * + * The scaffold is what tells an author where a rule goes, and what lets engine + * dispatch see that an engine exists at all. `0004` scaffolded its own layout + * and this migration prunes those directories, so without this a freshly + * migrated project would have no rules tree — every engine reporting "not + * present" and no obvious place to write the first rule. + */ +async function scaffoldEngineDirectories(directory: string): Promise { + for (const engine of ENGINES) { + const path = join(directory, RULES_DIRECTORY, engine); + await mkdir(path, { recursive: true }); + const entries = await entriesOf(path); + if (entries.length === 0) { + await writeFile(join(path, ".gitkeep"), "", "utf8"); + } + } +} + +/** Remove an engine's now-empty `0004` directories, leaving anything else. */ +async function pruneEmpty(directory: string, relativePath: string): Promise { + const path = join(directory, relativePath); + const entries = await entriesOf(path); + const remaining = entries.filter((entry) => entry.name !== ".gitkeep"); + if (remaining.length > 0) return; + await rm(path, { recursive: true, force: true }); +} + +/** + * Rewrite a matcher's assignments from the old check name to the new one. + * + * **This is the difference between a migrated rule that runs and one that is + * silently disabled.** A Vale check is named `