diff --git a/.changeset/vale-rule-engine.md b/.changeset/vale-rule-engine.md index 11f1e297..a988c0e0 100644 --- a/.changeset/vale-rule-engine.md +++ b/.changeset/vale-rule-engine.md @@ -2,16 +2,32 @@ "@taskless/cli": minor --- -Add Vale as a second static-tier rule engine. +Add Vale as a second static-tier rule engine, and give every engine one rule layout. -`check` now dispatches by engine directory and runs ast-grep, Vale, and runtime -rules concurrently, merging their findings into one result set. Vale rules live -in `.taskless/vale/` and execute against the committed `.vale.ini`; an -unavailable Vale reports itself and the other engines still return, while a Vale -that times out or rejects its config fails the check rather than passing as a -clean run. Vale rules are verified from `rule-tests//pass|fail` fixtures -against a generated per-rule config. +`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. -Adds the `engine-selection` knowledge topic — which engine enforces a given -rule, and why — available from `taskless help engine-selection` and exported -through `@taskless/cli/prompts`. +**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. + +Agent recipes are rewritten for the layout, and `taskless agent route` now +carries the engine-selection reasoning that used to be its own topic. + +Projects on an older layout migrate automatically on the next command. 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..2020fc7a --- /dev/null +++ b/example/README.md @@ -0,0 +1,88 @@ +# 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 + ")") + + 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`. + +## 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/agent-command-and-vale-authoring/resume.md b/openspec/changes/agent-command-and-vale-authoring/resume.md index e3920023..531739c0 100644 --- a/openspec/changes/agent-command-and-vale-authoring/resume.md +++ b/openspec/changes/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/self-contained-rules/.openspec.yaml b/openspec/changes/self-contained-rules/.openspec.yaml new file mode 100644 index 00000000..4af86417 --- /dev/null +++ b/openspec/changes/self-contained-rules/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-14 diff --git a/openspec/changes/self-contained-rules/design.md b/openspec/changes/self-contained-rules/design.md new file mode 100644 index 00000000..b157b63b --- /dev/null +++ b/openspec/changes/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/self-contained-rules/proposal.md b/openspec/changes/self-contained-rules/proposal.md new file mode 100644 index 00000000..167bc6b3 --- /dev/null +++ b/openspec/changes/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/self-contained-rules/specs/cli-agent-authoring/spec.md b/openspec/changes/self-contained-rules/specs/cli-agent-authoring/spec.md new file mode 100644 index 00000000..37066d7a --- /dev/null +++ b/openspec/changes/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/self-contained-rules/specs/cli-rule-format/spec.md b/openspec/changes/self-contained-rules/specs/cli-rule-format/spec.md new file mode 100644 index 00000000..dc8b2a11 --- /dev/null +++ b/openspec/changes/self-contained-rules/specs/cli-rule-format/spec.md @@ -0,0 +1,102 @@ +## MODIFIED Requirements + +### Requirement: Rules are one directory each, partitioned by engine + +The system SHALL store every rule as a directory at `.taskless/rules///`, holding the rule, any config that engine requires, and its tests under `.tests/`. + +| Engine | Rule directory contents | +|-----------|--------------------------------------------------------------| +| `sg` | `.yml`, `.tests/-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 + +## ADDED Requirements + +### 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/changes/self-contained-rules/specs/cli-rule-validation/spec.md b/openspec/changes/self-contained-rules/specs/cli-rule-validation/spec.md new file mode 100644 index 00000000..f9d25126 --- /dev/null +++ b/openspec/changes/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/self-contained-rules/specs/cli-vale-rule-engine/spec.md b/openspec/changes/self-contained-rules/specs/cli-vale-rule-engine/spec.md new file mode 100644 index 00000000..07b5f39a --- /dev/null +++ b/openspec/changes/self-contained-rules/specs/cli-vale-rule-engine/spec.md @@ -0,0 +1,123 @@ +## MODIFIED 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: 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: A Vale rule is a self-contained directory + +The system SHALL store a Vale rule as a directory `.taskless/rules/vale//` containing its style file `.yml`, its own `.vale.ini`, and its fixtures under `.tests/`. No file outside that directory SHALL be required to define or verify the rule. + +Self-containment is what removes the engine's silent-failure class. A rule can be added, reviewed, moved, or deleted as one directory, and no two authors write the same file. + +The rule's config SHALL be named `.vale.ini` rather than carrying a `.yml` extension. Measured: a `.yml` file inside a style directory is loaded as a rule and fails `E201` when the style is enabled wholesale, while a non-`.yml` file in the same directory is ignored. + +Scope SHALL NOT be expressed inside the style file. Measured: Vale rejects unknown top-level keys in a rule with `E201 has invalid keys`. + +#### Scenario: A rule is complete in one directory + +- **WHEN** a rule directory contains its style and its config +- **THEN** the rule is fully defined without editing any shared file + +#### Scenario: The rule config is invisible to Vale's style loader + +- **WHEN** a rule directory contains `.vale.ini` beside its style +- **THEN** Vale SHALL NOT attempt to load it as a rule + +#### Scenario: Deleting a rule is deleting a directory + +- **WHEN** a rule directory is removed +- **THEN** no other rule's scope changes +- **AND** no shared file needs editing diff --git a/openspec/changes/self-contained-rules/tasks.md b/openspec/changes/self-contained-rules/tasks.md new file mode 100644 index 00000000..07b0e5ee --- /dev/null +++ b/openspec/changes/self-contained-rules/tasks.md @@ -0,0 +1,84 @@ +# 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 +- [ ] 7.7 Archive the change + +**7.7 is blocked on #102, and cannot be unblocked from here.** A change archives once, on whichever PR is the tip, and the gate requires `openspec/changes/` to hold no unarchived directory at all. `agent-command-and-vale-authoring` still has 14 open tasks (groups 3, 4, 5, 6 — the `taskless help` → `taskless agent` cross-reference sweep, the `TOPICS` export surface, and its own verification), so its directory has to stay. Archiving it early would delete the spec deltas for work that has not been done. + +The ordering is also one-way for a second reason already noted in 7.5: this change's `cli-agent-authoring` delta modifies a requirement #102 introduces, so #102 has to archive first for that delta to have a target. + +So: finish #102's groups 3–6, archive `agent-command-and-vale-authoring`, then archive this change. 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/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 `