From f5b82dc64d7e60c69a1b252cbf9a986333121f15 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Thu, 13 Aug 2026 18:58:58 -0700 Subject: [PATCH 01/16] docs(openspec): propose self-contained Vale rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Vale rule is currently three locations, one of which every rule in the project shares. That shared .vale.ini is where all five harness runs found silent failures — 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. A single write-contended config is the wrong shape at any scale. Each rule becomes a directory holding its style and its own .vale.ini; check assembles the run config from them and gitignores it. Measured against Vale 3.17.1: rules//.yml resolves as check . under StylesPath = rules and resolves to nothing under StylesPath = ., Vale rejects unknown keys in a style so scope cannot ride along inside it, and a .yml sidecar in a style directory is loaded as a rule and fails E201 while a .vale.ini in the same place is ignored. Also replaces rule verify with path-addressed verify and test. An id does not name one thing — the same id can exist under two engines, which is why the id form needed an ambiguity error at all. Adds an example/ project so a reader can see an install rather than infer it from tests that build their own fixtures. Stacked on #102 and merging down: a layout change without its migration ships a project whose rules silently stop running. --- .../self-contained-vale-rules/.openspec.yaml | 2 + .../self-contained-vale-rules/design.md | 103 +++++++++++++++ .../self-contained-vale-rules/proposal.md | 44 +++++++ .../specs/cli-agent-authoring/spec.md | 54 ++++++++ .../specs/cli-rule-format/spec.md | 61 +++++++++ .../specs/cli-rule-validation/spec.md | 83 ++++++++++++ .../specs/cli-vale-rule-engine/spec.md | 123 ++++++++++++++++++ .../self-contained-vale-rules/tasks.md | 66 ++++++++++ 8 files changed, 536 insertions(+) create mode 100644 openspec/changes/self-contained-vale-rules/.openspec.yaml create mode 100644 openspec/changes/self-contained-vale-rules/design.md create mode 100644 openspec/changes/self-contained-vale-rules/proposal.md create mode 100644 openspec/changes/self-contained-vale-rules/specs/cli-agent-authoring/spec.md create mode 100644 openspec/changes/self-contained-vale-rules/specs/cli-rule-format/spec.md create mode 100644 openspec/changes/self-contained-vale-rules/specs/cli-rule-validation/spec.md create mode 100644 openspec/changes/self-contained-vale-rules/specs/cli-vale-rule-engine/spec.md create mode 100644 openspec/changes/self-contained-vale-rules/tasks.md diff --git a/openspec/changes/self-contained-vale-rules/.openspec.yaml b/openspec/changes/self-contained-vale-rules/.openspec.yaml new file mode 100644 index 0000000..4af8641 --- /dev/null +++ b/openspec/changes/self-contained-vale-rules/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-14 diff --git a/openspec/changes/self-contained-vale-rules/design.md b/openspec/changes/self-contained-vale-rules/design.md new file mode 100644 index 0000000..c1f4773 --- /dev/null +++ b/openspec/changes/self-contained-vale-rules/design.md @@ -0,0 +1,103 @@ +## Context + +`add-vale-rule-engine` established the Vale engine with a single committed `.taskless/vale/.vale.ini`: matchers express scope, precedence is positional, and `tskl) = ` breadcrumb keys tag each Taskless-owned matcher with the rule that owns it. That breadcrumb exists specifically so tooling can find a rule's matchers "even when its scoping is split across multiple matchers" — a problem that only exists because every rule shares one file. + +`agent-command-and-vale-authoring` then wrote the authoring recipe and executed it five times against sandboxed agents with no repository access. Every silent failure the runs found was in that shared file: an assignment above the first matcher (ignored with `W101` on stderr, exit 0), a glob that did not match the fixture extension, three names that must agree with nothing reporting when they don't. The recipe grew a debug ladder whose first four rungs are all "did you edit the shared config correctly". + +The measurements this design rests on, taken against the bundled Vale 3.17.1: + +- `rules//.yml` resolves as check `.` under `StylesPath = rules`. Under `StylesPath = .` it resolves to nothing. +- Vale rejects unknown top-level keys in a rule file (`E201 has invalid keys: 'taskless'`), so per-rule scope cannot ride along inside the style. +- Vale's ini parser accepts and ignores `tskl)` keys. +- A `.yml` sidecar inside a style directory is parsed as a rule and fails `E201` when the style is enabled wholesale; a non-`.yml` file in the same place is ignored. A per-rule `.vale.ini` is therefore safe where a per-rule `.yml` would not be. + +## Goals / Non-Goals + +**Goals:** + +- A Vale rule is one directory, complete on its own: style, scope, metadata, and nothing shared. +- No file is edited by more than one rule's author. +- A rule is addressable by path, so `verify` and `test` need no id lookup and no ambiguity rule. +- The silent-disable failure modes are removed by construction, not documented. + +**Non-Goals:** + +- Changing how ast-grep or runtime rules are laid out. `verify`/`test` become path-addressed for all three, but only Vale's on-disk shape moves. +- A GUI or interactive editor for matchers. The agent writes the per-rule `.vale.ini`, as it writes the style file. +- Supporting both layouts. There is one legal shape; the migration moves projects to it. + +## Decisions + +### D1 — A Vale rule is a directory containing its style and its own `.vale.ini` + +``` +.taskless/vale/rules/no-simply/ + no-simply.yml # extends, message, level, scope, match, exceptions + .vale.ini # this rule's matchers, excludes, and tskl) metadata +``` + +The deciding argument is write contention, not tidiness. A shared config is a file every agent must edit and no agent owns, and it is where every silent failure was actually found. Splitting it means an agent writes only files it created, and a rule can be added, reviewed, or deleted as one directory. + +It also makes the `tskl) rule = ` breadcrumb largely redundant for *locating* matchers — the directory answers that — though it stays as the marker of Taskless ownership within an assembled file. + +`.vale.ini` rather than a `.yml` sidecar is load-bearing: measured, a `.yml` file inside a style directory is parsed as a rule and fails `E201` when the style is enabled wholesale. The ini extension is invisible to Vale's style loader. + +_Alternative rejected:_ keep one shared config and lock or serialize writes. It preserves the failure modes and adds coordination to paper over them. + +_Alternative rejected:_ put scope in the style file. Measured impossible — Vale rejects unknown keys. + +### D2 — `StylesPath = rules` + +Required by D1: it is what makes each rule directory a style, giving check name `.`. Under `StylesPath = .` a nested rule file resolves to nothing at all. + +This reverses the note in migration `0004`, which calls `StylesPath = rules` wrong. That note was correct **for the flat layout** — with rules directly under `rules/`, 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 wrong for the other, which is why the note must be rewritten rather than deleted. + +A side benefit: `rule-tests/` stops being a sibling style directory. Under `StylesPath = .` it sits beside `rules/` as something Vale would treat as a style; under `StylesPath = rules` it is outside StylesPath entirely. + +### D3 — The run config is assembled at check time and gitignored + +`check` reads every `rules//.vale.ini` and writes one `.taskless/vale/.vale.ini` for the run. It is a build artifact, not a source file, and it is gitignored — the same treatment the ephemeral `sgconfig.yml` already receives, and the same reason: a generated file that is also committed drifts from its inputs and invites hand edits that the next generation discards. + +**Assembly order is a correctness constraint, not a formatting choice.** The spec's precedence rule is positional: across matchers the last wins, and within one matcher the first assignment wins. So assembly SHALL be deterministic — rules ordered by id, each rule's own matcher order preserved verbatim. A non-deterministic assembly would make a rule's effective scope depend on directory iteration order, which is the kind of bug that reproduces on one machine and not another. + +This means a rule cannot express "override another rule's matcher", since it cannot know its position. That is a real loss and an acceptable one: cross-rule overriding through a shared file is exactly the coupling D1 removes. + +_Alternative rejected:_ invoke Vale once per rule. Vale takes one `--config`, so N rules means N process spawns on every check, and findings would have to be merged from N JSON payloads. + +_Alternative rejected:_ commit the assembled file. It would be the shared, write-contended file again, arriving by a different route. + +### D4 — `verify` and `test` are separate commands, and `verify` is a layer of `test` + +`verify ` answers "is this a well-formed rule" — required components present, schema satisfied. `test ` answers "does it behave" — ast-grep test cases, Vale `pass`/`fail` fixtures, the runtime harness. + +They split because they have different preconditions. `verify` needs only the rule; `test` needs fixtures that may not exist yet. An agent mid-authoring wants the first before it can satisfy the second, and CI wants both. + +`test` runs `verify` first and stops on failure. Today the composition is backwards: fixture coverage short-circuits before Vale ever 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 don't. + +### D5 — 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-based dispatch had to carry an ambiguity error at all. Deleting the addressing scheme deletes the error case. + +The engine is resolved from the path's position under `.taskless//rules/`, the same way `dispatch` resolves it — never by parsing the file. A path that is a directory means everything beneath it, so `verify .taskless/` is the CI form and `verify .taskless/vale/rules/no-simply` is the single-rule form. + +_Alternative rejected:_ keep `rule verify ` as an alias. It preserves the ambiguity case for the convenience of a shorter argument, in a command agents invoke from a recipe that can just as easily carry a path. + +## Risks / Trade-offs + +- **The recipe changes again** → `create-vale-rule` currently teaches the flat layout in detail, and its nine worked examples were verified against it. The examples' rule bodies are unaffected — only where the file sits and where scope is declared. The harness (`agent-command-and-vale-authoring/tasks.md` 2b) re-runs against the new text, so this is bounded work with an existing verification loop. +- **Assembly is a new failure surface** → a bug there disables rules silently, which is the failure this whole engine's design exists to prevent. Mitigated by asserting on the assembled artifact's content, and by the existing stderr-notice path that surfaces Vale's `W101` when an assignment lands outside a matcher. +- **Cross-rule matcher overrides become impossible** → intended (D3), but worth stating: a project that wants "enable everywhere, disable in `legacy/`" must express both matchers within the owning rule's own config. +- **Two unreleased migrations in a row touch the same tree** → `0004` is unreleased, so no project in the field has the old scaffold, and the new migration is a no-op for anyone who never ran it. The risk is to this repo's own fixtures, which are covered by tests. + +## Migration Plan + +No user data is at stake: migration `0004` is unreleased and no Vale rules exist in the field. The new migration is written for this repository's own fixtures and for anyone running a pre-release build. + +1. Move each flat `vale/rules/.yml` to `vale/rules//.yml`. +2. Split the committed `vale/.vale.ini`: each matcher carrying `tskl) rule = ` moves to that rule's directory; `StylesPath`/`MinAlertLevel` become assembly defaults. +3. Delete the committed `vale/.vale.ini` and add it to `.taskless/.gitignore`. +4. A matcher with no `tskl) rule` breadcrumb cannot be attributed to a rule. Leave it in place and report it rather than guessing an owner or dropping it — an unattributable matcher is a user's hand edit, and silently discarding it would change what their check reports. + +## Open Questions + +- None outstanding. diff --git a/openspec/changes/self-contained-vale-rules/proposal.md b/openspec/changes/self-contained-vale-rules/proposal.md new file mode 100644 index 0000000..405c4d1 --- /dev/null +++ b/openspec/changes/self-contained-vale-rules/proposal.md @@ -0,0 +1,44 @@ +## Why + +A Vale rule is currently spread across three locations, one of which every rule in the project shares. The style file is `vale/rules/.yml`, the fixtures are `vale/rule-tests//`, and the scope — the part that decides whether the rule runs at all — is a matcher inside the single committed `vale/.vale.ini`. + +That shared file is the problem. Every agent authoring a rule has to edit it correctly, in a file every other rule also occupies, and the failure is silent: an assignment outside a matcher is ignored with a warning on stderr, a glob that misses the fixture's extension lints nothing, and a rule whose three names disagree simply never runs. Five sandboxed harness runs against `create-vale-rule` found silent failures in exactly this step and nowhere else. At any scale — hundreds of agents, or one agent and a year of rules — a single write-contended config is the wrong shape. + +Making each rule self-contained removes the class rather than documenting it. It also makes a rule addressable as one path, which is what lets `verify` and `test` take a path instead of an id. + +Now, because no Vale rules exist yet. Once they do, this is a migration. + +## What Changes + +- **BREAKING** A Vale rule becomes a directory, `.taskless/vale/rules//`, holding its style file `.yml` and its own `.vale.ini` carrying that rule's matchers, exclude directives, and `tskl)` metadata. +- **BREAKING** `.taskless/vale/.vale.ini` stops being committed. `check` assembles it from the per-rule configs at run time, and it is gitignored — the same treatment the ephemeral `sgconfig.yml` already gets. +- **BREAKING** `StylesPath` becomes `rules`, which is what makes each rule directory a Vale *style*. Measured: `rules//.yml` resolves as check `.` under `StylesPath = rules`, and resolves to nothing under `StylesPath = .`. +- **BREAKING** `rule verify ` is removed, along with its id-based engine dispatch. An id is not a unique address — the same id can exist under `sg` and `vale` — so addressing by id required an ambiguity error that the path form does not need. +- Two new path-addressed commands: `verify ` checks a rule has its required components, `test ` runs its fixtures. Both accept a file or a directory; a directory means everything beneath it. Both run as part of the rule generation loop. +- `verify` becomes a prerequisite layer of `test`, so a malformed rule reports its own error instead of a fixture complaint. Today a rule with a bad `level` and an incomplete fixture set reports `fixtures: "fail-only"` and never surfaces `'level' must be one of [suggestion warning error]`. +- `create-vale-rule` is rewritten against the new layout, and its worked examples re-verified against it. +- A committed example project at `/example` — a README, an HTML and a CommonJS file, and a `.taskless/` holding one Vale rule and one ast-grep rule with their fixtures. It is a demo of a correct layout and a smoke test for `check`, `verify`, and `test`, exercised by a test so it cannot rot silently. + +## Capabilities + +### New Capabilities + +- `cli-rule-validation`: the path-addressed `verify` and `test` commands — how a path resolves to an engine, what each command checks per engine, and how they compose in the generation loop. + +### Modified Capabilities + +- `cli-vale-rule-engine`: the rule is a directory with its own config; the run config is assembled and gitignored rather than committed; `StylesPath` changes; matcher precedence now has to be preserved across an assembly step rather than within one authored file. +- `cli-rule-format`: the canonical on-disk shape of a Vale rule, and the "committed native config, never generated" rule that a Vale run config now breaks. +- `cli-agent-authoring`: `create-vale-rule` teaches the new 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 only exists once #102 archives — expected for a stacked change, and the reason `openspec validate` will flag it until then. + +## Impact + +- `src/filesystem/migrations/` — a new migration moving any flat `vale/rules/.yml` into `vale/rules//.yml`, and retiring the committed `vale/.vale.ini`. Migration `0004` is unreleased, so no project in the field carries the old scaffold. +- `src/rules/vale/run.ts` — assembles the run config instead of reading a committed one. +- `src/rules/vale/verify.ts` — a rule's config is now its own; the isolating config used for verification is built from it rather than invented. +- `src/rules/dispatch.ts` — `hasValeRules` currently looks for flat `*.yml` and would report a directory-shaped rule set as "no rules configured". +- `src/commands/rules.ts` — `rule verify` removed; `src/commands/` gains `verify` and `test`. +- `src/help/create-vale-rule.txt` — rewritten; `verify-rule.txt` and every recipe naming `rule verify`. +- `.taskless/.gitignore` — the assembled config. +- `/example/` — new, plus the root tooling ignores (prettier, eslint) that would otherwise walk its deliberately-wrong fixture prose. +- Reverses the `.vale.ini`-writer non-goal recorded in `agent-command-and-vale-authoring/design.md`, which assumed a hand-authored shared config. diff --git a/openspec/changes/self-contained-vale-rules/specs/cli-agent-authoring/spec.md b/openspec/changes/self-contained-vale-rules/specs/cli-agent-authoring/spec.md new file mode 100644 index 0000000..3efc1d7 --- /dev/null +++ b/openspec/changes/self-contained-vale-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/vale/rules//.yml`. +2. That rule's own `.taskless/vale/rules//.vale.ini`, declaring the matchers that scope it and enabling it as `. = YES`. +3. `pass/` and `fail/` fixture documents under `.taskless/vale/rule-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-vale-rules/specs/cli-rule-format/spec.md b/openspec/changes/self-contained-vale-rules/specs/cli-rule-format/spec.md new file mode 100644 index 0000000..9b2275f --- /dev/null +++ b/openspec/changes/self-contained-vale-rules/specs/cli-rule-format/spec.md @@ -0,0 +1,61 @@ +## MODIFIED Requirements + +### 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 that config is **committed**, the system SHALL read it as-is and SHALL NOT generate it at check time. `sg/sgconfig.yml` is committed and read as-is. + +Vale is the exception, and deliberately. Its scoping is per-rule but its config is per-run: Vale accepts exactly one `--config`, so a project's rules have to reach a single file before Vale can be invoked. The committed source of truth is therefore each rule's own `.vale.ini`, and the file handed to Vale is assembled from them. This is not a Taskless sidecar — it is the engine's own native config, split along the boundary the engine's scoping already has. + +#### Scenario: ast-grep config is read as committed + +- **WHEN** the CLI runs a check +- **THEN** it reads the committed `sg/sgconfig.yml` as-is, and neither writes nor generates it + +#### Scenario: Vale config is assembled from committed per-rule configs + +- **WHEN** the CLI runs a check +- **THEN** it reads each committed `vale/rules//.vale.ini` and assembles the config it hands to Vale +- **AND** the assembled file SHALL be gitignored + +#### 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/vale/rules//`, so that `` is Vale's StyleName, with the assembled config setting `StylesPath = rules`. 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 `rules//.yml` resolves as check `.` under `StylesPath = rules`, and resolves to nothing at all under `StylesPath = .`. The reverse held for the previous flat layout, where `StylesPath = .` made `rules` the StyleName and `StylesPath = rules` resolved nothing — 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/no-simply.yml` +- **THEN** Vale loads it as `no-simply.no-simply`, and the CLI reports its findings with `ruleId` `no-simply` + +#### Scenario: The rule-tests tree is outside StylesPath + +- **WHEN** `StylesPath` is `rules` +- **THEN** `.taskless/vale/rule-tests/` SHALL NOT be loaded as a style directory + +## ADDED Requirements + +### Requirement: A rule's canonical location is what verify and test address + +Each engine SHALL have one canonical on-disk location per rule, and that location SHALL be what `verify` and `test` accept as a path. + +| Engine | Canonical location | Shape | +|-----------|-------------------------------------------|-----------| +| `sg` | `.taskless/sg/rules/.yml` | file | +| `vale` | `.taskless/vale/rules//` | directory | +| `runtime` | `.taskless/runtime/rules//` | directory | + +A rule's engine SHALL be resolved from where its path sits, never from its contents, which is the rule the check dispatcher already follows. + +#### Scenario: Each engine has one canonical rule location + +- **WHEN** a rule is authored for any engine +- **THEN** it is written at that engine's canonical location +- **AND** a command given that path can determine the engine without reading the file diff --git a/openspec/changes/self-contained-vale-rules/specs/cli-rule-validation/spec.md b/openspec/changes/self-contained-vale-rules/specs/cli-rule-validation/spec.md new file mode 100644 index 0000000..f76b01f --- /dev/null +++ b/openspec/changes/self-contained-vale-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/vale/rules/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 `sg/rules/` and `vale/rules/` +- **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` | the rule file against the ast-grep schema and the Taskless required fields | +| `vale` | the style file against Vale's own validation, and the rule's `.vale.ini` | +| `runtime` | the rule directory holds `check.ts` and at least one capture rule | + +#### 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-vale-rules/specs/cli-vale-rule-engine/spec.md b/openspec/changes/self-contained-vale-rules/specs/cli-vale-rule-engine/spec.md new file mode 100644 index 0000000..1b97ee3 --- /dev/null +++ b/openspec/changes/self-contained-vale-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 = rules` 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/vale/rules/` 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/vale/rules/` 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/vale/rules//.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/vale/rule-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/vale/rules//` containing its style file `.yml` and its own `.vale.ini`. No file outside that directory, other than the rule's fixtures, SHALL be required to define 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-vale-rules/tasks.md b/openspec/changes/self-contained-vale-rules/tasks.md new file mode 100644 index 0000000..00010c3 --- /dev/null +++ b/openspec/changes/self-contained-vale-rules/tasks.md @@ -0,0 +1,66 @@ +# 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 assembler, ships a project whose Vale rules silently stop running. Nothing reaches `main` until all of it does. + +## 1. The rule directory and its config + +- [ ] 1.1 Teach `ENGINE_LAYOUTS.vale` that a rule is a directory. Add the canonical-location helper the whole change leans on: given a rule id, its directory, its style file, and its config path +- [ ] 1.2 `hasValeRules` currently looks for flat `*.yml` directly under `vale/rules/` and would report a directory-shaped rule set as "no rules configured" — a silent skip of the whole engine. Fix it first, and add the test that would have caught it +- [ ] 1.3 Read a rule's own `.vale.ini`. Preserve matcher order verbatim: precedence is positional, so reordering silently changes scope + +## 2. Assembly + +- [ ] 2.1 Assemble a run config from every rule's config. Deterministic: rules ordered by id, each rule's matchers in the order it declared them, `StylesPath = rules` and `MinAlertLevel = suggestion` as the header +- [ ] 2.2 Carry each matcher's `tskl) rule = ` breadcrumb into the assembled file. Provenance is otherwise lost the moment matchers interleave +- [ ] 2.3 Write it where the run can read it and add it to `.taskless/.gitignore`. It is a build artifact; a committed generated file drifts and invites hand edits the next assembly discards +- [ ] 2.4 Assert the assembled artifact 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 +- [ ] 2.5 `runVale` runs against the assembled config rather than a committed one + +## 3. Migration + +- [ ] 3.1 New migration: move each flat `vale/rules/.yml` to `vale/rules//.yml` +- [ ] 3.2 Split the committed `vale/.vale.ini` — each matcher carrying `tskl) rule = ` moves to that rule's own config +- [ ] 3.3 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 +- [ ] 3.4 Delete the committed `vale/.vale.ini`; gitignore the assembled path +- [ ] 3.5 Rewrite the `StylesPath` docstring in `0004`. It currently states `StylesPath = rules` is wrong, which was true for the flat layout and is now exactly backwards — the note has to explain both layouts or it will be read as a bug + +## 4. `verify` and `test` + +- [ ] 4.1 Resolve a path to an engine by position under `.taskless//rules/`, never by parsing the file. A path outside any engine's rules directory is an error naming the path +- [ ] 4.2 A directory path means every rule beneath it; report per-rule results rather than one pass/fail +- [ ] 4.3 `verify `: ast-grep schema + Taskless required fields; Vale style validation + the rule's own config; runtime `check.ts` plus at least one capture rule. Fixtures are NOT required for verify to pass +- [ ] 4.4 `test `: ast-grep test cases, Vale fixture buckets, runtime harness. Runs `verify` first and stops on failure — today a malformed rule reports a fixture complaint while the actual error goes unmentioned +- [ ] 4.5 Delete `rule verify` and the id-based dispatch added in `bc09897`, including `rulefileOwners` and the ambiguity error. The path form has no ambiguity case to report +- [ ] 4.6 Wire both into the rule generation loop +- [ ] 4.7 Port the tests from `rule-verify-dispatch.test.ts` onto the path-addressed commands and delete the id-addressed ones + +## 5. Recipes + +- [ ] 5.1 Rewrite `create-vale-rule.txt` for the layout: rule directory, its own config, no shared file. The nine worked rules keep their bodies — only where the file sits and where scope is declared changes +- [ ] 5.2 Replace the `.vale.ini` walkthrough. The current step 4 teaches editing a shared config and carries the `W101`-outside-a-matcher warning; both describe a situation that no longer exists +- [ ] 5.3 Step 6 names `verify` and `test` rather than reading `results[].ruleId` out of `check` +- [ ] 5.4 Update `verify-rule.txt` for both commands; sweep every recipe naming `rule verify` +- [ ] 5.5 Re-run the 2b harness against the rewritten recipe — fresh agents, no repository access, the same three extension points. The recipe changed materially, so its prior convergence does not carry over +- [ ] 5.6 Re-verify the nine worked rules by extracting the YAML from the *rendered* recipe and executing it, as before. 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. + +- [ ] 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 +- [ ] 6.2 `example/example.html` and `example/example.cjs` — the prose and the code the rules have something to say about. Keep both small enough to read in one screen +- [ ] 6.3 `example/.taskless/` with one Vale rule and one ast-grep rule, each in its canonical location, each with its fixtures. The Vale rule exercises the new directory layout end to end +- [ ] 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 +- [ ] 6.5 A test that runs `verify example/.taskless/` and `test example/.taskless/` — the directory form, which is also the CI form +- [ ] 6.6 Add `example/` to the root tooling ignores that would otherwise walk it. `.taskless/` inside it holds rule YAML and fixture documents that are deliberately wrong prose, and a root-level prettier or eslint pass reaching them fails on content nobody wrote as source + +## 7. Verify + +- [ ] 7.1 `pnpm typecheck`, `pnpm lint`, `pnpm --filter @taskless/cli build`, `pnpm --filter @taskless/cli test` +- [ ] 7.2 End-to-end against a scaffolded project: author two rules with different globs, confirm each fires only in its own scope, and confirm deleting one directory leaves the other's scope untouched +- [ ] 7.3 Confirm a hand-edited assembled config has no effect on the next check — it is regenerated +- [ ] 7.4 `pnpm openspec validate --all --strict`. Note that the `cli-agent-authoring` delta modifies a requirement #102 introduces, so this only passes cleanly once #102 archives +- [ ] 7.5 Extend the changeset on the bottom of the stack with this change's breaks: the Vale rule layout, the removal of `rule verify`, and the new `verify`/`test` commands +- [ ] 7.6 Archive the change From 321fc1eb14cd0d8cb8a0835b7614d6231ad7c943 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Thu, 13 Aug 2026 19:20:24 -0700 Subject: [PATCH 02/16] docs(openspec): unify the rule layout across all three engines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Widens the change from Vale to every engine, and renames it to match. One directory per rule, identical shape everywhere: `.taskless/rules///` holding the rule, any config that engine requires, and its tests in `.tests/`. A rule becomes one path rather than two, which is what makes `verify ` and `test ` work without an id lookup. The dot on `.tests/` is load-bearing and measured. ast-grep's ruleDirs recurses and parses every .yml beneath as a rule, so a plain `tests/` directory fails the scan with "missing field 'language'"; `__tests__/` fails the same way; a dot-directory is skipped, and `sg test` still reads it when testDir names it. Vale is unaffected either way — a `.tests/` inside a style directory is harmless even containing a .yml. That is a dependency on undocumented behavior, recorded as one in D2 with two mitigations: the failure is loud (a parse error naming the file, not a silently reinterpreted test), and a test pins it. The rejected alternative — materializing a rules-only tree for ast-grep — is written down as the fallback if the assumption ever breaks. Also drops the per-rule sg config: ast-grep expresses scoping inside the rule, so the slot would be an empty file every author creates, no author fills, and every reader learns to ignore. And renames runtime's capture rules to `captures/`, since "matcher" now means a Vale glob section in this same tree. 0005 layers on 0004; both are unreleased and both ship in this stack, so consumers run them as one upgrade and never see the intermediate layout. --- .../.openspec.yaml | 0 .../changes/self-contained-rules/design.md | 139 ++++++++++++++++++ .../changes/self-contained-rules/proposal.md | 48 ++++++ .../specs/cli-agent-authoring/spec.md | 6 +- .../specs/cli-rule-format/spec.md | 102 +++++++++++++ .../specs/cli-rule-validation/spec.md | 16 +- .../specs/cli-vale-rule-engine/spec.md | 12 +- .../changes/self-contained-rules/tasks.md | 74 ++++++++++ .../self-contained-vale-rules/design.md | 103 ------------- .../self-contained-vale-rules/proposal.md | 44 ------ .../specs/cli-rule-format/spec.md | 61 -------- .../self-contained-vale-rules/tasks.md | 66 --------- 12 files changed, 380 insertions(+), 291 deletions(-) rename openspec/changes/{self-contained-vale-rules => self-contained-rules}/.openspec.yaml (100%) create mode 100644 openspec/changes/self-contained-rules/design.md create mode 100644 openspec/changes/self-contained-rules/proposal.md rename openspec/changes/{self-contained-vale-rules => self-contained-rules}/specs/cli-agent-authoring/spec.md (93%) create mode 100644 openspec/changes/self-contained-rules/specs/cli-rule-format/spec.md rename openspec/changes/{self-contained-vale-rules => self-contained-rules}/specs/cli-rule-validation/spec.md (88%) rename openspec/changes/{self-contained-vale-rules => self-contained-rules}/specs/cli-vale-rule-engine/spec.md (85%) create mode 100644 openspec/changes/self-contained-rules/tasks.md delete mode 100644 openspec/changes/self-contained-vale-rules/design.md delete mode 100644 openspec/changes/self-contained-vale-rules/proposal.md delete mode 100644 openspec/changes/self-contained-vale-rules/specs/cli-rule-format/spec.md delete mode 100644 openspec/changes/self-contained-vale-rules/tasks.md diff --git a/openspec/changes/self-contained-vale-rules/.openspec.yaml b/openspec/changes/self-contained-rules/.openspec.yaml similarity index 100% rename from openspec/changes/self-contained-vale-rules/.openspec.yaml rename to openspec/changes/self-contained-rules/.openspec.yaml diff --git a/openspec/changes/self-contained-rules/design.md b/openspec/changes/self-contained-rules/design.md new file mode 100644 index 0000000..06eecb6 --- /dev/null +++ b/openspec/changes/self-contained-rules/design.md @@ -0,0 +1,139 @@ +## 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. Two 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 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. + +### 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. + +## Risks / Trade-offs + +- **The dot-directory assumption is undocumented** → mitigated by a loud failure mode and a pinning test (D2), with materialization as the known 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. + +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 0000000..167bc6b --- /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-vale-rules/specs/cli-agent-authoring/spec.md b/openspec/changes/self-contained-rules/specs/cli-agent-authoring/spec.md similarity index 93% rename from openspec/changes/self-contained-vale-rules/specs/cli-agent-authoring/spec.md rename to openspec/changes/self-contained-rules/specs/cli-agent-authoring/spec.md index 3efc1d7..37066d7 100644 --- a/openspec/changes/self-contained-vale-rules/specs/cli-agent-authoring/spec.md +++ b/openspec/changes/self-contained-rules/specs/cli-agent-authoring/spec.md @@ -4,9 +4,9 @@ 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/vale/rules//.yml`. -2. That rule's own `.taskless/vale/rules//.vale.ini`, declaring the matchers that scope it and enabling it as `. = 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 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. 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 0000000..dc8b2a1 --- /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-vale-rules/specs/cli-rule-validation/spec.md b/openspec/changes/self-contained-rules/specs/cli-rule-validation/spec.md similarity index 88% rename from openspec/changes/self-contained-vale-rules/specs/cli-rule-validation/spec.md rename to openspec/changes/self-contained-rules/specs/cli-rule-validation/spec.md index f76b01f..f9d2512 100644 --- a/openspec/changes/self-contained-vale-rules/specs/cli-rule-validation/spec.md +++ b/openspec/changes/self-contained-rules/specs/cli-rule-validation/spec.md @@ -2,18 +2,18 @@ ### 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. +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/vale/rules/no-simply` is run +- **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 `sg/rules/` and `vale/rules/` +- **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 @@ -36,11 +36,11 @@ The two commands split because they have different preconditions. An agent part- Per engine, `verify` SHALL check: -| Engine | Components | -|-----------|----------------------------------------------------------------------------| -| `sg` | the rule file against the ast-grep schema and the Taskless required fields | -| `vale` | the style file against Vale's own validation, and the rule's `.vale.ini` | -| `runtime` | the rule directory holds `check.ts` and at least one capture rule | +| 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 diff --git a/openspec/changes/self-contained-vale-rules/specs/cli-vale-rule-engine/spec.md b/openspec/changes/self-contained-rules/specs/cli-vale-rule-engine/spec.md similarity index 85% rename from openspec/changes/self-contained-vale-rules/specs/cli-vale-rule-engine/spec.md rename to openspec/changes/self-contained-rules/specs/cli-vale-rule-engine/spec.md index 1b97ee3..07b5f39 100644 --- a/openspec/changes/self-contained-vale-rules/specs/cli-vale-rule-engine/spec.md +++ b/openspec/changes/self-contained-rules/specs/cli-vale-rule-engine/spec.md @@ -2,7 +2,7 @@ ### 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 = rules` and `MinAlertLevel = suggestion`, so that every finding surfaces to the client for normalization and filtering. +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. @@ -10,7 +10,7 @@ The assembled config SHALL be written where the run can read it and SHALL be git #### Scenario: Check runs Vale via the assembled config -- **WHEN** the CLI runs a check and `.taskless/vale/rules/` contains rule directories +- **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 @@ -21,12 +21,12 @@ The assembled config SHALL be written where the run can read it and SHALL be git #### Scenario: No Vale rules present -- **WHEN** `.taskless/vale/rules/` contains no rule directories +- **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/vale/rules//.vale.ini`. Include is `. = YES`, exclude is `. = NO`. +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: @@ -76,7 +76,7 @@ With each rule owning its config, a matcher's owner is given by the directory it ### 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 isolates one rule, the system SHALL generate an ephemeral config enabling only that rule — derived from the rule's own config so that verification exercises the scope the rule actually declares. Verification SHALL assert that every `fail/` fixture produces at least one finding for the rule and every `pass/` fixture produces none (mirroring ast-grep's `invalid`/`valid`). +The system SHALL verify a Vale rule from 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 @@ -98,7 +98,7 @@ The system SHALL verify a Vale rule from a `.taskless/vale/rule-tests//` s ### Requirement: A Vale rule is a self-contained directory -The system SHALL store a Vale rule as a directory `.taskless/vale/rules//` containing its style file `.yml` and its own `.vale.ini`. No file outside that directory, other than the rule's fixtures, SHALL be required to define the rule. +The system SHALL store a Vale rule as a directory `.taskless/rules/vale//` containing its style file `.yml`, its own `.vale.ini`, and its fixtures under `.tests/`. No file outside that directory SHALL be required to define or verify the rule. Self-containment is what removes the engine's silent-failure class. A rule can be added, reviewed, moved, or deleted as one directory, and no two authors write the same file. diff --git a/openspec/changes/self-contained-rules/tasks.md b/openspec/changes/self-contained-rules/tasks.md new file mode 100644 index 0000000..df2def6 --- /dev/null +++ b/openspec/changes/self-contained-rules/tasks.md @@ -0,0 +1,74 @@ +# 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 + +- [ ] 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 +- [ ] 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 +- [ ] 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 +- [ ] 1.4 Runtime discovery reads capture rules from `captures/`; `check.ts` stays at the rule root + +## 2. Assembly + +- [ ] 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 +- [ ] 2.2 Carry each matcher's `tskl) rule = ` breadcrumb through assembly. Provenance is otherwise lost the moment matchers interleave +- [ ] 2.3 Assemble `sgconfig.yml`: `ruleDirs` over the rules tree, one `testConfigs` entry per rule's `.tests/` +- [ ] 2.4 Gitignore both assembled configs. They are build artifacts; a committed generated file drifts and invites hand edits the next assembly discards +- [ ] 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 +- [ ] 2.6 `runVale` and the ast-grep scan run against the assembled configs + +## 3. Migration `0005` + +- [ ] 3.1 Move `/rules/.yml` → `rules///.yml`; runtime `runtime/rules//` → `rules/runtime//`, its `*.yml` into `captures/` +- [ ] 3.2 Move `/rule-tests/*` → `rules///.tests/`, preserving each engine's internal test shape +- [ ] 3.3 Split the committed `vale/.vale.ini` — each matcher carrying `tskl) rule = ` into that rule's own config +- [ ] 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 +- [ ] 3.5 Delete the committed `vale/.vale.ini` and `sg/sgconfig.yml` +- [ ] 3.6 Preserve content byte-for-byte. Runtime capture bytes determine server-side reconciliation hashes, so a rewrite invalidates every signature +- [ ] 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 +- [ ] 3.8 Idempotent, and a no-op on an already-migrated tree + +## 4. `verify` and `test` + +- [ ] 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 +- [ ] 4.2 A directory above a rule means every rule beneath it; report per-rule results rather than one pass/fail +- [ ] 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 +- [ ] 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 +- [ ] 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 +- [ ] 4.6 Wire both into the rule generation loop +- [ ] 4.7 Port `rule-verify-dispatch.test.ts` onto the path-addressed commands; delete the id-addressed tests + +## 5. Recipes + +- [ ] 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 +- [ ] 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 +- [ ] 5.3 Step 6 names `verify` and `test` rather than reading `results[].ruleId` out of `check` +- [ ] 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` +- [ ] 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 +- [ ] 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. + +- [ ] 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 +- [ ] 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 +- [ ] 6.3 `example/.taskless/` with one Vale rule and one ast-grep rule, each in its canonical directory with its `.tests/` +- [ ] 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 +- [ ] 6.5 A test that runs `verify example/.taskless/rules/` and `test example/.taskless/rules/` — the directory form, which is also the CI form +- [ ] 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 + +- [ ] 7.1 `pnpm typecheck`, `pnpm lint`, `pnpm --filter @taskless/cli build`, `pnpm --filter @taskless/cli test` +- [ ] 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 +- [ ] 7.3 Confirm a hand-edited assembled config has no effect on the next check — it is regenerated +- [ ] 7.4 Migrate a `0004`-shaped fixture through `0005` and confirm the rules still fire, the tests still run, and runtime signatures are unchanged +- [ ] 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 +- [ ] 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 diff --git a/openspec/changes/self-contained-vale-rules/design.md b/openspec/changes/self-contained-vale-rules/design.md deleted file mode 100644 index c1f4773..0000000 --- a/openspec/changes/self-contained-vale-rules/design.md +++ /dev/null @@ -1,103 +0,0 @@ -## Context - -`add-vale-rule-engine` established the Vale engine with a single committed `.taskless/vale/.vale.ini`: matchers express scope, precedence is positional, and `tskl) = ` breadcrumb keys tag each Taskless-owned matcher with the rule that owns it. That breadcrumb exists specifically so tooling can find a rule's matchers "even when its scoping is split across multiple matchers" — a problem that only exists because every rule shares one file. - -`agent-command-and-vale-authoring` then wrote the authoring recipe and executed it five times against sandboxed agents with no repository access. Every silent failure the runs found was in that shared file: an assignment above the first matcher (ignored with `W101` on stderr, exit 0), a glob that did not match the fixture extension, three names that must agree with nothing reporting when they don't. The recipe grew a debug ladder whose first four rungs are all "did you edit the shared config correctly". - -The measurements this design rests on, taken against the bundled Vale 3.17.1: - -- `rules//.yml` resolves as check `.` under `StylesPath = rules`. Under `StylesPath = .` it resolves to nothing. -- Vale rejects unknown top-level keys in a rule file (`E201 has invalid keys: 'taskless'`), so per-rule scope cannot ride along inside the style. -- Vale's ini parser accepts and ignores `tskl)` keys. -- A `.yml` sidecar inside a style directory is parsed as a rule and fails `E201` when the style is enabled wholesale; a non-`.yml` file in the same place is ignored. A per-rule `.vale.ini` is therefore safe where a per-rule `.yml` would not be. - -## Goals / Non-Goals - -**Goals:** - -- A Vale rule is one directory, complete on its own: style, scope, metadata, and nothing shared. -- No file is edited by more than one rule's author. -- A rule is addressable by path, so `verify` and `test` need no id lookup and no ambiguity rule. -- The silent-disable failure modes are removed by construction, not documented. - -**Non-Goals:** - -- Changing how ast-grep or runtime rules are laid out. `verify`/`test` become path-addressed for all three, but only Vale's on-disk shape moves. -- A GUI or interactive editor for matchers. The agent writes the per-rule `.vale.ini`, as it writes the style file. -- Supporting both layouts. There is one legal shape; the migration moves projects to it. - -## Decisions - -### D1 — A Vale rule is a directory containing its style and its own `.vale.ini` - -``` -.taskless/vale/rules/no-simply/ - no-simply.yml # extends, message, level, scope, match, exceptions - .vale.ini # this rule's matchers, excludes, and tskl) metadata -``` - -The deciding argument is write contention, not tidiness. A shared config is a file every agent must edit and no agent owns, and it is where every silent failure was actually found. Splitting it means an agent writes only files it created, and a rule can be added, reviewed, or deleted as one directory. - -It also makes the `tskl) rule = ` breadcrumb largely redundant for *locating* matchers — the directory answers that — though it stays as the marker of Taskless ownership within an assembled file. - -`.vale.ini` rather than a `.yml` sidecar is load-bearing: measured, a `.yml` file inside a style directory is parsed as a rule and fails `E201` when the style is enabled wholesale. The ini extension is invisible to Vale's style loader. - -_Alternative rejected:_ keep one shared config and lock or serialize writes. It preserves the failure modes and adds coordination to paper over them. - -_Alternative rejected:_ put scope in the style file. Measured impossible — Vale rejects unknown keys. - -### D2 — `StylesPath = rules` - -Required by D1: it is what makes each rule directory a style, giving check name `.`. Under `StylesPath = .` a nested rule file resolves to nothing at all. - -This reverses the note in migration `0004`, which calls `StylesPath = rules` wrong. That note was correct **for the flat layout** — with rules directly under `rules/`, 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 wrong for the other, which is why the note must be rewritten rather than deleted. - -A side benefit: `rule-tests/` stops being a sibling style directory. Under `StylesPath = .` it sits beside `rules/` as something Vale would treat as a style; under `StylesPath = rules` it is outside StylesPath entirely. - -### D3 — The run config is assembled at check time and gitignored - -`check` reads every `rules//.vale.ini` and writes one `.taskless/vale/.vale.ini` for the run. It is a build artifact, not a source file, and it is gitignored — the same treatment the ephemeral `sgconfig.yml` already receives, and the same reason: a generated file that is also committed drifts from its inputs and invites hand edits that the next generation discards. - -**Assembly order is a correctness constraint, not a formatting choice.** The spec's precedence rule is positional: across matchers the last wins, and within one matcher the first assignment wins. So assembly SHALL be deterministic — rules ordered by id, each rule's own matcher order preserved verbatim. A non-deterministic assembly would make a rule's effective scope depend on directory iteration order, which is the kind of bug that reproduces on one machine and not another. - -This means a rule cannot express "override another rule's matcher", since it cannot know its position. That is a real loss and an acceptable one: cross-rule overriding through a shared file is exactly the coupling D1 removes. - -_Alternative rejected:_ invoke Vale once per rule. Vale takes one `--config`, so N rules means N process spawns on every check, and findings would have to be merged from N JSON payloads. - -_Alternative rejected:_ commit the assembled file. It would be the shared, write-contended file again, arriving by a different route. - -### D4 — `verify` and `test` are separate commands, and `verify` is a layer of `test` - -`verify ` answers "is this a well-formed rule" — required components present, schema satisfied. `test ` answers "does it behave" — ast-grep test cases, Vale `pass`/`fail` fixtures, the runtime harness. - -They split because they have different preconditions. `verify` needs only the rule; `test` needs fixtures that may not exist yet. An agent mid-authoring wants the first before it can satisfy the second, and CI wants both. - -`test` runs `verify` first and stops on failure. Today the composition is backwards: fixture coverage short-circuits before Vale ever 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 don't. - -### D5 — 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-based dispatch had to carry an ambiguity error at all. Deleting the addressing scheme deletes the error case. - -The engine is resolved from the path's position under `.taskless//rules/`, the same way `dispatch` resolves it — never by parsing the file. A path that is a directory means everything beneath it, so `verify .taskless/` is the CI form and `verify .taskless/vale/rules/no-simply` is the single-rule form. - -_Alternative rejected:_ keep `rule verify ` as an alias. It preserves the ambiguity case for the convenience of a shorter argument, in a command agents invoke from a recipe that can just as easily carry a path. - -## Risks / Trade-offs - -- **The recipe changes again** → `create-vale-rule` currently teaches the flat layout in detail, and its nine worked examples were verified against it. The examples' rule bodies are unaffected — only where the file sits and where scope is declared. The harness (`agent-command-and-vale-authoring/tasks.md` 2b) re-runs against the new text, so this is bounded work with an existing verification loop. -- **Assembly is a new failure surface** → a bug there disables rules silently, which is the failure this whole engine's design exists to prevent. Mitigated by asserting on the assembled artifact's content, and by the existing stderr-notice path that surfaces Vale's `W101` when an assignment lands outside a matcher. -- **Cross-rule matcher overrides become impossible** → intended (D3), but worth stating: a project that wants "enable everywhere, disable in `legacy/`" must express both matchers within the owning rule's own config. -- **Two unreleased migrations in a row touch the same tree** → `0004` is unreleased, so no project in the field has the old scaffold, and the new migration is a no-op for anyone who never ran it. The risk is to this repo's own fixtures, which are covered by tests. - -## Migration Plan - -No user data is at stake: migration `0004` is unreleased and no Vale rules exist in the field. The new migration is written for this repository's own fixtures and for anyone running a pre-release build. - -1. Move each flat `vale/rules/.yml` to `vale/rules//.yml`. -2. Split the committed `vale/.vale.ini`: each matcher carrying `tskl) rule = ` moves to that rule's directory; `StylesPath`/`MinAlertLevel` become assembly defaults. -3. Delete the committed `vale/.vale.ini` and add it to `.taskless/.gitignore`. -4. A matcher with no `tskl) rule` breadcrumb cannot be attributed to a rule. Leave it in place and report it rather than guessing an owner or dropping it — an unattributable matcher is a user's hand edit, and silently discarding it would change what their check reports. - -## Open Questions - -- None outstanding. diff --git a/openspec/changes/self-contained-vale-rules/proposal.md b/openspec/changes/self-contained-vale-rules/proposal.md deleted file mode 100644 index 405c4d1..0000000 --- a/openspec/changes/self-contained-vale-rules/proposal.md +++ /dev/null @@ -1,44 +0,0 @@ -## Why - -A Vale rule is currently spread across three locations, one of which every rule in the project shares. The style file is `vale/rules/.yml`, the fixtures are `vale/rule-tests//`, and the scope — the part that decides whether the rule runs at all — is a matcher inside the single committed `vale/.vale.ini`. - -That shared file is the problem. Every agent authoring a rule has to edit it correctly, in a file every other rule also occupies, and the failure is silent: an assignment outside a matcher is ignored with a warning on stderr, a glob that misses the fixture's extension lints nothing, and a rule whose three names disagree simply never runs. Five sandboxed harness runs against `create-vale-rule` found silent failures in exactly this step and nowhere else. At any scale — hundreds of agents, or one agent and a year of rules — a single write-contended config is the wrong shape. - -Making each rule self-contained removes the class rather than documenting it. It also makes a rule addressable as one path, which is what lets `verify` and `test` take a path instead of an id. - -Now, because no Vale rules exist yet. Once they do, this is a migration. - -## What Changes - -- **BREAKING** A Vale rule becomes a directory, `.taskless/vale/rules//`, holding its style file `.yml` and its own `.vale.ini` carrying that rule's matchers, exclude directives, and `tskl)` metadata. -- **BREAKING** `.taskless/vale/.vale.ini` stops being committed. `check` assembles it from the per-rule configs at run time, and it is gitignored — the same treatment the ephemeral `sgconfig.yml` already gets. -- **BREAKING** `StylesPath` becomes `rules`, which is what makes each rule directory a Vale *style*. Measured: `rules//.yml` resolves as check `.` under `StylesPath = rules`, and resolves to nothing under `StylesPath = .`. -- **BREAKING** `rule verify ` is removed, along with its id-based engine dispatch. An id is not a unique address — the same id can exist under `sg` and `vale` — so addressing by id required an ambiguity error that the path form does not need. -- Two new path-addressed commands: `verify ` checks a rule has its required components, `test ` runs its fixtures. Both accept a file or a directory; a directory means everything beneath it. Both run as part of the rule generation loop. -- `verify` becomes a prerequisite layer of `test`, so a malformed rule reports its own error instead of a fixture complaint. Today a rule with a bad `level` and an incomplete fixture set reports `fixtures: "fail-only"` and never surfaces `'level' must be one of [suggestion warning error]`. -- `create-vale-rule` is rewritten against the new layout, and its worked examples re-verified against it. -- A committed example project at `/example` — a README, an HTML and a CommonJS file, and a `.taskless/` holding one Vale rule and one ast-grep rule with their fixtures. It is a demo of a correct layout and a smoke test for `check`, `verify`, and `test`, exercised by a test so it cannot rot silently. - -## Capabilities - -### New Capabilities - -- `cli-rule-validation`: the path-addressed `verify` and `test` commands — how a path resolves to an engine, what each command checks per engine, and how they compose in the generation loop. - -### Modified Capabilities - -- `cli-vale-rule-engine`: the rule is a directory with its own config; the run config is assembled and gitignored rather than committed; `StylesPath` changes; matcher precedence now has to be preserved across an assembly step rather than within one authored file. -- `cli-rule-format`: the canonical on-disk shape of a Vale rule, and the "committed native config, never generated" rule that a Vale run config now breaks. -- `cli-agent-authoring`: `create-vale-rule` teaches the new 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 only exists once #102 archives — expected for a stacked change, and the reason `openspec validate` will flag it until then. - -## Impact - -- `src/filesystem/migrations/` — a new migration moving any flat `vale/rules/.yml` into `vale/rules//.yml`, and retiring the committed `vale/.vale.ini`. Migration `0004` is unreleased, so no project in the field carries the old scaffold. -- `src/rules/vale/run.ts` — assembles the run config instead of reading a committed one. -- `src/rules/vale/verify.ts` — a rule's config is now its own; the isolating config used for verification is built from it rather than invented. -- `src/rules/dispatch.ts` — `hasValeRules` currently looks for flat `*.yml` and would report a directory-shaped rule set as "no rules configured". -- `src/commands/rules.ts` — `rule verify` removed; `src/commands/` gains `verify` and `test`. -- `src/help/create-vale-rule.txt` — rewritten; `verify-rule.txt` and every recipe naming `rule verify`. -- `.taskless/.gitignore` — the assembled config. -- `/example/` — new, plus the root tooling ignores (prettier, eslint) that would otherwise walk its deliberately-wrong fixture prose. -- Reverses the `.vale.ini`-writer non-goal recorded in `agent-command-and-vale-authoring/design.md`, which assumed a hand-authored shared config. diff --git a/openspec/changes/self-contained-vale-rules/specs/cli-rule-format/spec.md b/openspec/changes/self-contained-vale-rules/specs/cli-rule-format/spec.md deleted file mode 100644 index 9b2275f..0000000 --- a/openspec/changes/self-contained-vale-rules/specs/cli-rule-format/spec.md +++ /dev/null @@ -1,61 +0,0 @@ -## MODIFIED Requirements - -### 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 that config is **committed**, the system SHALL read it as-is and SHALL NOT generate it at check time. `sg/sgconfig.yml` is committed and read as-is. - -Vale is the exception, and deliberately. Its scoping is per-rule but its config is per-run: Vale accepts exactly one `--config`, so a project's rules have to reach a single file before Vale can be invoked. The committed source of truth is therefore each rule's own `.vale.ini`, and the file handed to Vale is assembled from them. This is not a Taskless sidecar — it is the engine's own native config, split along the boundary the engine's scoping already has. - -#### Scenario: ast-grep config is read as committed - -- **WHEN** the CLI runs a check -- **THEN** it reads the committed `sg/sgconfig.yml` as-is, and neither writes nor generates it - -#### Scenario: Vale config is assembled from committed per-rule configs - -- **WHEN** the CLI runs a check -- **THEN** it reads each committed `vale/rules//.vale.ini` and assembles the config it hands to Vale -- **AND** the assembled file SHALL be gitignored - -#### 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/vale/rules//`, so that `` is Vale's StyleName, with the assembled config setting `StylesPath = rules`. 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 `rules//.yml` resolves as check `.` under `StylesPath = rules`, and resolves to nothing at all under `StylesPath = .`. The reverse held for the previous flat layout, where `StylesPath = .` made `rules` the StyleName and `StylesPath = rules` resolved nothing — 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/no-simply.yml` -- **THEN** Vale loads it as `no-simply.no-simply`, and the CLI reports its findings with `ruleId` `no-simply` - -#### Scenario: The rule-tests tree is outside StylesPath - -- **WHEN** `StylesPath` is `rules` -- **THEN** `.taskless/vale/rule-tests/` SHALL NOT be loaded as a style directory - -## ADDED Requirements - -### Requirement: A rule's canonical location is what verify and test address - -Each engine SHALL have one canonical on-disk location per rule, and that location SHALL be what `verify` and `test` accept as a path. - -| Engine | Canonical location | Shape | -|-----------|-------------------------------------------|-----------| -| `sg` | `.taskless/sg/rules/.yml` | file | -| `vale` | `.taskless/vale/rules//` | directory | -| `runtime` | `.taskless/runtime/rules//` | directory | - -A rule's engine SHALL be resolved from where its path sits, never from its contents, which is the rule the check dispatcher already follows. - -#### Scenario: Each engine has one canonical rule location - -- **WHEN** a rule is authored for any engine -- **THEN** it is written at that engine's canonical location -- **AND** a command given that path can determine the engine without reading the file diff --git a/openspec/changes/self-contained-vale-rules/tasks.md b/openspec/changes/self-contained-vale-rules/tasks.md deleted file mode 100644 index 00010c3..0000000 --- a/openspec/changes/self-contained-vale-rules/tasks.md +++ /dev/null @@ -1,66 +0,0 @@ -# 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 assembler, ships a project whose Vale rules silently stop running. Nothing reaches `main` until all of it does. - -## 1. The rule directory and its config - -- [ ] 1.1 Teach `ENGINE_LAYOUTS.vale` that a rule is a directory. Add the canonical-location helper the whole change leans on: given a rule id, its directory, its style file, and its config path -- [ ] 1.2 `hasValeRules` currently looks for flat `*.yml` directly under `vale/rules/` and would report a directory-shaped rule set as "no rules configured" — a silent skip of the whole engine. Fix it first, and add the test that would have caught it -- [ ] 1.3 Read a rule's own `.vale.ini`. Preserve matcher order verbatim: precedence is positional, so reordering silently changes scope - -## 2. Assembly - -- [ ] 2.1 Assemble a run config from every rule's config. Deterministic: rules ordered by id, each rule's matchers in the order it declared them, `StylesPath = rules` and `MinAlertLevel = suggestion` as the header -- [ ] 2.2 Carry each matcher's `tskl) rule = ` breadcrumb into the assembled file. Provenance is otherwise lost the moment matchers interleave -- [ ] 2.3 Write it where the run can read it and add it to `.taskless/.gitignore`. It is a build artifact; a committed generated file drifts and invites hand edits the next assembly discards -- [ ] 2.4 Assert the assembled artifact 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 -- [ ] 2.5 `runVale` runs against the assembled config rather than a committed one - -## 3. Migration - -- [ ] 3.1 New migration: move each flat `vale/rules/.yml` to `vale/rules//.yml` -- [ ] 3.2 Split the committed `vale/.vale.ini` — each matcher carrying `tskl) rule = ` moves to that rule's own config -- [ ] 3.3 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 -- [ ] 3.4 Delete the committed `vale/.vale.ini`; gitignore the assembled path -- [ ] 3.5 Rewrite the `StylesPath` docstring in `0004`. It currently states `StylesPath = rules` is wrong, which was true for the flat layout and is now exactly backwards — the note has to explain both layouts or it will be read as a bug - -## 4. `verify` and `test` - -- [ ] 4.1 Resolve a path to an engine by position under `.taskless//rules/`, never by parsing the file. A path outside any engine's rules directory is an error naming the path -- [ ] 4.2 A directory path means every rule beneath it; report per-rule results rather than one pass/fail -- [ ] 4.3 `verify `: ast-grep schema + Taskless required fields; Vale style validation + the rule's own config; runtime `check.ts` plus at least one capture rule. Fixtures are NOT required for verify to pass -- [ ] 4.4 `test `: ast-grep test cases, Vale fixture buckets, runtime harness. Runs `verify` first and stops on failure — today a malformed rule reports a fixture complaint while the actual error goes unmentioned -- [ ] 4.5 Delete `rule verify` and the id-based dispatch added in `bc09897`, including `rulefileOwners` and the ambiguity error. The path form has no ambiguity case to report -- [ ] 4.6 Wire both into the rule generation loop -- [ ] 4.7 Port the tests from `rule-verify-dispatch.test.ts` onto the path-addressed commands and delete the id-addressed ones - -## 5. Recipes - -- [ ] 5.1 Rewrite `create-vale-rule.txt` for the layout: rule directory, its own config, no shared file. The nine worked rules keep their bodies — only where the file sits and where scope is declared changes -- [ ] 5.2 Replace the `.vale.ini` walkthrough. The current step 4 teaches editing a shared config and carries the `W101`-outside-a-matcher warning; both describe a situation that no longer exists -- [ ] 5.3 Step 6 names `verify` and `test` rather than reading `results[].ruleId` out of `check` -- [ ] 5.4 Update `verify-rule.txt` for both commands; sweep every recipe naming `rule verify` -- [ ] 5.5 Re-run the 2b harness against the rewritten recipe — fresh agents, no repository access, the same three extension points. The recipe changed materially, so its prior convergence does not carry over -- [ ] 5.6 Re-verify the nine worked rules by extracting the YAML from the *rendered* recipe and executing it, as before. 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. - -- [ ] 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 -- [ ] 6.2 `example/example.html` and `example/example.cjs` — the prose and the code the rules have something to say about. Keep both small enough to read in one screen -- [ ] 6.3 `example/.taskless/` with one Vale rule and one ast-grep rule, each in its canonical location, each with its fixtures. The Vale rule exercises the new directory layout end to end -- [ ] 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 -- [ ] 6.5 A test that runs `verify example/.taskless/` and `test example/.taskless/` — the directory form, which is also the CI form -- [ ] 6.6 Add `example/` to the root tooling ignores that would otherwise walk it. `.taskless/` inside it holds rule YAML and fixture documents that are deliberately wrong prose, and a root-level prettier or eslint pass reaching them fails on content nobody wrote as source - -## 7. Verify - -- [ ] 7.1 `pnpm typecheck`, `pnpm lint`, `pnpm --filter @taskless/cli build`, `pnpm --filter @taskless/cli test` -- [ ] 7.2 End-to-end against a scaffolded project: author two rules with different globs, confirm each fires only in its own scope, and confirm deleting one directory leaves the other's scope untouched -- [ ] 7.3 Confirm a hand-edited assembled config has no effect on the next check — it is regenerated -- [ ] 7.4 `pnpm openspec validate --all --strict`. Note that the `cli-agent-authoring` delta modifies a requirement #102 introduces, so this only passes cleanly once #102 archives -- [ ] 7.5 Extend the changeset on the bottom of the stack with this change's breaks: the Vale rule layout, the removal of `rule verify`, and the new `verify`/`test` commands -- [ ] 7.6 Archive the change From 83f180205b42642bcadaba5a9f7bb8b95ece0405 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Thu, 13 Aug 2026 20:10:44 -0700 Subject: [PATCH 03/16] docs(openspec): record the legacy-path collision found while implementing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Started the layout refactor and hit something the proposal missed: `.taskless/rules/` is the new root and is also LEGACY_RULES_DIRECTORY, the pre-0004 flat location. Same string, different meaning. The legacy read paths turn out to be 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. Under the new layout a legacy lookup would resolve .taskless/rules/.yml inside a tree whose real contents are rules/// — reading the new root as though it were the old flat directory. A stale read path that resolves into the live tree is worse than no fallback. Also adds the migration precondition that follows: 0005 asserts the new root holds no top-level *.yml before writing engine directories into it, since a file still there means 0004 did not complete. Reverting the partial engines.ts refactor so this PR stays spec-only and green rather than carrying a tree with twelve broken callers. --- openspec/changes/self-contained-rules/design.md | 16 ++++++++++++++-- openspec/changes/self-contained-rules/tasks.md | 2 ++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/openspec/changes/self-contained-rules/design.md b/openspec/changes/self-contained-rules/design.md index 06eecb6..c103779 100644 --- a/openspec/changes/self-contained-rules/design.md +++ b/openspec/changes/self-contained-rules/design.md @@ -66,10 +66,11 @@ _Alternative rejected:_ mirrored `rules/` and `tests/` trees. Uniform and requir `.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. Two things make it acceptable: +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. @@ -111,13 +112,23 @@ A path names one thing; an id does not. The same id can exist under `sg` and `va _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. ## Risks / Trade-offs -- **The dot-directory assumption is undocumented** → mitigated by a loud failure mode and a pinning test (D2), with materialization as the known fallback. +- **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. @@ -127,6 +138,7 @@ Runtime's ast-grep capture rules move to `captures/`. "Matcher" now has a precis `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`. diff --git a/openspec/changes/self-contained-rules/tasks.md b/openspec/changes/self-contained-rules/tasks.md index df2def6..c77ea7c 100644 --- a/openspec/changes/self-contained-rules/tasks.md +++ b/openspec/changes/self-contained-rules/tasks.md @@ -10,6 +10,7 @@ Delivery shape: **stacked, merging down**, on top of `agent-command-and-vale-aut - [ ] 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 - [ ] 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 - [ ] 1.4 Runtime discovery reads capture rules from `captures/`; `check.ts` stays at the rule root +- [ ] 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 @@ -22,6 +23,7 @@ Delivery shape: **stacked, merging down**, on top of `agent-command-and-vale-aut ## 3. Migration `0005` +- [ ] 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 - [ ] 3.1 Move `/rules/.yml` → `rules///.yml`; runtime `runtime/rules//` → `rules/runtime//`, its `*.yml` into `captures/` - [ ] 3.2 Move `/rule-tests/*` → `rules///.tests/`, preserving each engine's internal test shape - [ ] 3.3 Split the committed `vale/.vale.ini` — each matcher carrying `tskl) rule = ` into that rule's own config From b953f9f7707d104e1755ea456ad8a672450f095a Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Thu, 13 Aug 2026 20:11:20 -0700 Subject: [PATCH 04/16] docs(openspec): carry #103's state into the resume notes --- .../resume.md | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/openspec/changes/agent-command-and-vale-authoring/resume.md b/openspec/changes/agent-command-and-vale-authoring/resume.md index e392002..531739c 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) From d06413c55523241316f4f228a0a5704cf066a2a5 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Thu, 13 Aug 2026 21:03:00 -0700 Subject: [PATCH 05/16] feat(cli): one directory per rule, with assembled engine configs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Groups 1-3 of self-contained-rules, plus the `rule verify` removal. ENGINE_LAYOUTS now describes a rule *directory* rather than parallel rules/ and rule-tests/ trees, and every path derives from it. Tests live in `.tests/` — the dot is load-bearing, since ast-grep's ruleDirs recurses and parses every .yml beneath as a rule, and the constant carries that measurement plus the fallback if it ever breaks. Both engine configs are now assembled per run and gitignored. Assembly is deterministic — rules sorted by id, each rule's matcher order verbatim — because Vale's precedence is positional and a config built in directory-iteration order would give a rule a different effective scope per machine. Migration 0005 moves everything and rewrites nothing: capture bytes determine reconciliation hashes. It asserts `.taskless/rules/` holds no loose .yml first, since that path is both the new root and the pre-0004 flat location. Vale matchers split by their `tskl) rule` breadcrumb; one without a breadcrumb is left in place and reported rather than guessed at or dropped, because it is a user's hand edit. Deleted with their layouts: the legacy read paths (unreachable — the migrations run before any read), filesystem/sgconfig.ts (assembly replaces it), rules/owner.ts and `rule verify` (an id does not name one rule; the path form has no ambiguity case). 0004's StylesPath docstring now explains both layouts. It said `StylesPath = rules` is wrong, which was true flat and is exactly backwards here — a bare contradiction invites a future reader to revert it. Typecheck and build clean. Tests still assert the old layout; they are next. --- packages/cli/src/commands/check.ts | 19 +- packages/cli/src/commands/rules.ts | 235 +------------ packages/cli/src/detect/scan.ts | 7 +- packages/cli/src/filesystem/migrate.ts | 2 + .../filesystem/migrations/0004-vale-engine.ts | 19 +- .../migrations/0005-rule-directories.ts | 305 +++++++++++++++++ packages/cli/src/filesystem/sgconfig.ts | 92 ----- packages/cli/src/rules/assemble.ts | 189 ++++++++++ packages/cli/src/rules/dispatch.ts | 45 +-- packages/cli/src/rules/engines.ts | 323 +++++++++--------- packages/cli/src/rules/files.ts | 87 ++--- packages/cli/src/rules/owner.ts | 73 ---- packages/cli/src/rules/runtime/discover.ts | 13 +- packages/cli/src/rules/scan.ts | 4 +- packages/cli/src/rules/vale/run.ts | 31 +- packages/cli/src/rules/vale/verify.ts | 26 +- packages/cli/src/rules/verify.ts | 97 ++---- packages/cli/test/sgconfig.test.ts | 124 ------- packages/cli/test/vale-orchestration.test.ts | 17 +- 19 files changed, 819 insertions(+), 889 deletions(-) create mode 100644 packages/cli/src/filesystem/migrations/0005-rule-directories.ts delete mode 100644 packages/cli/src/filesystem/sgconfig.ts create mode 100644 packages/cli/src/rules/assemble.ts delete mode 100644 packages/cli/src/rules/owner.ts delete mode 100644 packages/cli/test/sgconfig.test.ts diff --git a/packages/cli/src/commands/check.ts b/packages/cli/src/commands/check.ts index 40b1962..b8cfac2 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 b1d8b47..a056db0 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/detect/scan.ts b/packages/cli/src/detect/scan.ts index 56637a5..0da166a 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 f954fe2..77bf2c6 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 fe3c9b8..2787359 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 0000000..607aad0 --- /dev/null +++ b/packages/cli/src/filesystem/migrations/0005-rule-directories.ts @@ -0,0 +1,305 @@ +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 stray = (await entriesOf(root)) + .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 + ) + ); + } +} + +/** 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 remaining = (await entriesOf(path)).filter( + (entry) => entry.name !== ".gitkeep" + ); + if (remaining.length > 0) return; + await rm(path, { recursive: true, force: true }); +} + +/** + * Split the committed `vale/.vale.ini` into per-rule configs. + * + * Each matcher carrying a `tskl) rule = ` breadcrumb belongs to that rule. + * A matcher **without** one cannot be attributed: it is a user's hand edit, and + * guessing an owner or dropping it would silently change what their check + * reports. Those are left in place and reported, so the file stays on disk with + * only the unattributable part remaining. + */ +async function splitValeConfig(directory: string): Promise { + const configPath = join(directory, "vale", ".vale.ini"); + let source: string; + try { + source = await readFile(configPath, "utf8"); + } catch { + return []; + } + + const lines = source.split("\n"); + const blocks: Array<{ ruleId?: string; lines: string[] }> = []; + let current: { ruleId?: string; lines: string[] } | undefined; + + for (const line of lines) { + if (line.trimStart().startsWith("[")) { + if (current !== undefined) blocks.push(current); + current = { lines: [line] }; + continue; + } + if (current === undefined) continue; // header: StylesPath / MinAlertLevel + current.lines.push(line); + const breadcrumb = /^\s*tskl\)\s*rule\s*=\s*(?\S+)/.exec(line); + if (breadcrumb?.groups?.id !== undefined) { + current.ruleId = breadcrumb.groups.id; + } + } + if (current !== undefined) blocks.push(current); + + const byRule = new Map(); + const orphans: string[] = []; + for (const block of blocks) { + const body = block.lines.join("\n").trimEnd(); + if (body === "") continue; + if (block.ruleId === undefined) { + orphans.push(body); + continue; + } + byRule.set(block.ruleId, [...(byRule.get(block.ruleId) ?? []), body]); + } + + for (const [ruleId, bodies] of byRule) { + const target = join( + directory, + RULES_DIRECTORY, + "vale", + ruleId, + ".vale.ini" + ); + await mkdir(dirname(target), { recursive: true }); + await writeFile(target, `${bodies.join("\n\n")}\n`, "utf8"); + } + + if (orphans.length === 0) { + await rm(configPath, { force: true }); + return []; + } + + // Keep the unattributable matchers where the user put them, and say so. + await writeFile(configPath, `${orphans.join("\n\n")}\n`, "utf8"); + return orphans; +} + +/** Append the generated config paths to `.taskless/.gitignore`. */ +async function ignoreGeneratedConfigs(directory: string): Promise { + const gitignorePath = join(directory, ".gitignore"); + let existing = ""; + try { + existing = await readFile(gitignorePath, "utf8"); + } catch { + // 0001 writes one; if it is missing there is nothing to preserve. + } + const lines = existing.split("\n"); + const present = new Set(lines.map((line) => line.trim())); + const additions = GENERATED_PATHS.filter((path) => !present.has(path)); + if (additions.length === 0) return; + const next = [...lines.filter((line) => line !== ""), ...additions, ""]; + await writeFile(gitignorePath, next.join("\n"), "utf8"); +} + +/** + * Migration 5 — one directory per rule. + * + * `0004` partitioned by engine but kept rules and tests in separate trees, and + * kept Vale's scope in one committed config every rule shared. This collapses + * both: `.taskless/rules///` holds the rule, its own config where + * the engine needs one, and its tests in `.tests/`. + * + * The dot on `.tests/` is load-bearing — ast-grep's `ruleDirs` recurses and + * would parse a plain `tests/` directory's YAML as rules, failing the scan. + * + * Content is moved, never rewritten, so runtime reconciliation signatures + * survive. Idempotent: a tree already in the new shape has nothing to move. + */ +const migration: Migration = async (directory) => { + await assertRootIsFree(directory); + + for (const engine of ENGINES) { + await moveEngineRules(directory, engine); + await moveEngineTests(directory, engine); + } + + const orphans = await splitValeConfig(directory); + + for (const engine of ENGINES) { + await pruneEmpty(directory, PRIOR_LAYOUT[engine].rules); + await pruneEmpty(directory, PRIOR_LAYOUT[engine].tests); + await pruneEmpty(directory, engine); + } + + for (const config of PRIOR_CONFIGS) { + if (config === "vale/.vale.ini" && orphans.length > 0) continue; + await rm(join(directory, config), { force: true }); + } + + await ignoreGeneratedConfigs(directory); + + if (orphans.length > 0) { + console.error( + `Notice: .taskless/vale/.vale.ini still contains ${String(orphans.length)} ` + + `matcher(s) with no \`tskl) rule\` breadcrumb, so they could not be ` + + `attributed to a rule. They were left in place — move them into the ` + + `owning rule's .vale.ini under .taskless/${RULES_DIRECTORY}/vale/.` + ); + } +}; + +export default migration; diff --git a/packages/cli/src/filesystem/sgconfig.ts b/packages/cli/src/filesystem/sgconfig.ts deleted file mode 100644 index 99b8ca6..0000000 --- a/packages/cli/src/filesystem/sgconfig.ts +++ /dev/null @@ -1,92 +0,0 @@ -import { writeFile } from "node:fs/promises"; -import { join } from "node:path"; - -import { ensureTasklessDirectory } from "./directory"; -import { COMMITTED_SG_CONFIG, ENGINE_LAYOUTS } from "../rules/engines"; - -/** Build sgconfig contents pointing `ruleDirs` at the given directory. */ -function sgConfigContent( - rulesDirectory: string, - testDirectory: string -): string { - return `ruleDirs:\n - ${rulesDirectory}\ntestConfigs:\n - testDir: ${testDirectory}\n`; -} - -export interface SgConfigOptions { - /** - * Directory (relative to `.taskless/`) that ast-grep should load rules from. - * Defaults to `sg/rules`, the engine-partitioned location. Callers pass the - * legacy `rules` when scanning an unmigrated tree, and reconciliation points - * this at the ephemeral run directory so only the server-blessed run set is - * evaluated. - */ - rulesDirectory?: string; - /** - * Directory (relative to `.taskless/`) holding that rule set's tests. - * Defaults to `sg/rule-tests`. Only `sg test` reads it. - */ - testDirectory?: string; -} - -/** - * Generate an ephemeral `sgconfig.yml` in `.taskless/` for ast-grep. - * Runs migrations and ensures the directory structure is up-to-date. - * - * The `sg` engine no longer goes through here — it reads its committed - * `sg/sgconfig.yml` ({@link COMMITTED_SG_CONFIG}). What remains is exactly one - * caller: {@link resolveSgConfigPath} generating a config for the pre-migration - * `.taskless/rules/` layout, which has no committed config of its own. - * - * The runtime narrow is NOT a second caller, despite looking like one — it - * writes its own `sgconfig.yml` into the materialized run directory - * (`rules/runtime/narrow.ts`) rather than coming through here. - */ -export async function generateSgConfig( - cwd: string, - options: SgConfigOptions = {} -): Promise { - await ensureTasklessDirectory(cwd); - await writeFile( - join(cwd, ".taskless", EPHEMERAL_SG_CONFIG_FILE), - sgConfigContent( - options.rulesDirectory ?? ENGINE_LAYOUTS.sg.rulesDirectory, - options.testDirectory ?? ENGINE_LAYOUTS.sg.ruleTestsDirectory - ), - "utf8" - ); -} - -/** Filename of the generated config, inside `.taskless/` (git-ignored). */ -const EPHEMERAL_SG_CONFIG_FILE = "sgconfig.yml"; - -/** The generated config's path relative to the project root. */ -export const EPHEMERAL_SG_CONFIG = `.taskless/${EPHEMERAL_SG_CONFIG_FILE}`; - -/** A rule set to point ast-grep at, as returned by engine discovery. */ -export interface SgConfigSource { - /** Rules directory, relative to `.taskless/`. */ - rulesDirectory: string; - /** Rule-tests directory, relative to `.taskless/`. */ - ruleTestsDirectory: string; - /** Whether this is the pre-migration layout. */ - legacy: boolean; -} - -/** - * The `--config` path to run this rule set with, relative to the project root. - * - * The engine-partitioned source resolves to its committed config and nothing is - * written. Only the legacy layout — which by definition predates that config — - * still needs one generated for it. - */ -export async function resolveSgConfigPath( - cwd: string, - source: SgConfigSource -): Promise { - if (!source.legacy) return COMMITTED_SG_CONFIG; - await generateSgConfig(cwd, { - rulesDirectory: source.rulesDirectory, - testDirectory: source.ruleTestsDirectory, - }); - return EPHEMERAL_SG_CONFIG; -} diff --git a/packages/cli/src/rules/assemble.ts b/packages/cli/src/rules/assemble.ts new file mode 100644 index 0000000..0334b01 --- /dev/null +++ b/packages/cli/src/rules/assemble.ts @@ -0,0 +1,189 @@ +import { mkdir, readFile, writeFile } from "node:fs/promises"; +import { dirname, join, posix, relative, sep } from "node:path"; + +import { + ASSEMBLED_SG_CONFIG, + ASSEMBLED_VALE_CONFIG, + listRuleIds, + RULE_TESTS_DIRECTORY, + RULES_DIRECTORY, + ruleConfigPath, + ruleTestsDirectory, +} from "./engines"; + +/** + * Assembling the per-rule configs into the single file each tool accepts. + * + * Vale takes exactly one `--config` and ast-grep one `sgconfig.yml`, so + * per-rule configuration has to reach one file before either can be invoked. + * The committed source of truth stays per-rule; what the tool reads is + * generated here and gitignored. + * + * **Determinism is a correctness constraint, not tidiness.** Vale's matcher + * precedence is positional — across matchers the last wins, within one matcher + * the first assignment wins — so a config assembled in directory-iteration + * order would give a rule a different effective scope depending on the machine + * it ran on. Rules are therefore emitted in sorted id order, and each rule's own + * matcher order is preserved verbatim. + * + * A consequence worth naming: a rule cannot override another rule's matchers, + * because it cannot know its own position in the assembled file. That coupling + * is exactly what per-rule configs remove. + */ + +/** Path within `.taskless/`, in the POSIX form both tools' configs expect. */ +function tasklessRelative(...segments: string[]): string { + return segments.join(posix.sep); +} + +/** Normalize a filesystem path to POSIX separators for config output. */ +function toPosix(path: string): string { + return path.split(sep).join(posix.sep); +} + +/** + * The header every assembled Vale config carries. + * + * `StylesPath` points at the Vale rules tree so each rule directory is a style, + * which is what makes a rule at `/.yml` resolve as check `.`. + * Measured: under `StylesPath = .` that same file resolves to nothing at all. + * + * `MinAlertLevel = suggestion` so every finding reaches the client, which + * filters and normalizes rather than relying on Vale to decide what matters. + */ +function valeHeader(): string { + return [ + `StylesPath = ${tasklessRelative(RULES_DIRECTORY, "vale")}`, + "MinAlertLevel = suggestion", + "", + ].join("\n"); +} + +/** + * Read a rule's own config, dropping a leading `StylesPath`/`MinAlertLevel` if + * an author copied one in. + * + * Those two are properties of the run, not of a rule, and a per-rule copy would + * either be redundant or silently fight the header. Everything else — matchers, + * assignments, `tskl)` breadcrumbs, comments — is carried through **verbatim**, + * because matcher order inside a rule is the author's expression of precedence. + */ +function ruleConfigBody(source: string): string { + const lines = source.split("\n"); + const kept: string[] = []; + let seenSection = false; + for (const line of lines) { + if (line.trimStart().startsWith("[")) seenSection = true; + if (!seenSection) { + const key = line.split("=")[0]?.trim().toLowerCase(); + if (key === "stylespath" || key === "minalertlevel") continue; + } + kept.push(line); + } + return kept.join("\n").trim(); +} + +/** A rule's assembled block, tagged so its provenance survives interleaving. */ +function valeRuleBlock(ruleId: string, body: string): string { + return [`# tskl) rule = ${ruleId}`, body, ""].join("\n"); +} + +/** + * Assemble `.taskless/.vale.ini` from every Vale rule's own config. + * + * Returns the config path relative to the project root, or `undefined` when no + * Vale rule declares any config — there is nothing to run, and writing an empty + * config would invite Vale to lint the project against no rules and report a + * clean pass. + */ +export async function assembleValeConfig( + cwd: string +): Promise { + const ruleIds = await listRuleIds(cwd, "vale"); + const blocks: string[] = []; + + for (const ruleId of ruleIds) { + const configPath = ruleConfigPath(cwd, "vale", ruleId); + if (configPath === undefined) continue; + let source: string; + try { + source = await readFile(configPath, "utf8"); + } catch { + // A rule with no config of its own declares no scope, so it is enabled + // nowhere. That is the author's omission to fix, not ours to guess at — + // `verify` reports it. + continue; + } + const body = ruleConfigBody(source); + if (body === "") continue; + blocks.push(valeRuleBlock(ruleId, body)); + } + + if (blocks.length === 0) return undefined; + + const contents = [valeHeader(), ...blocks].join("\n"); + const target = join(cwd, ASSEMBLED_VALE_CONFIG); + await mkdir(dirname(target), { recursive: true }); + await writeFile(target, contents, "utf8"); + return ASSEMBLED_VALE_CONFIG; +} + +/** + * Assemble `.taskless/.sgconfig.yml`. + * + * `ruleDirs` names the ast-grep rules tree, which ast-grep walks recursively — + * that recursion is why tests live in `.tests/` rather than `tests/`, since + * every `.yml` it reaches is parsed as a rule. + * + * `testConfigs` gets one entry per rule, because each rule keeps its tests + * inside its own directory. Sorted with the rule ids, so the file is stable. + */ +export async function assembleSgConfig( + cwd: string +): Promise { + const ruleIds = await listRuleIds(cwd, "sg"); + if (ruleIds.length === 0) return undefined; + + const rulesDirectory = tasklessRelative(RULES_DIRECTORY, "sg"); + const testDirectories = ruleIds.map((ruleId) => + toPosix(relative(join(cwd, ".taskless"), ruleTestsDirectory(cwd, "sg", ruleId))) + ); + + const contents = [ + "ruleDirs:", + ` - ${rulesDirectory}`, + "testConfigs:", + ...testDirectories.map((directory) => ` - testDir: ${directory}`), + "", + ].join("\n"); + + const target = join(cwd, ASSEMBLED_SG_CONFIG); + await mkdir(dirname(target), { recursive: true }); + await writeFile(target, contents, "utf8"); + return ASSEMBLED_SG_CONFIG; +} + +/** Both assembled configs, for a run that needs whichever engines are present. */ +export interface AssembledConfigs { + /** `--config` for Vale, or `undefined` when no Vale rule is configured. */ + vale: string | undefined; + /** `-c` for ast-grep, or `undefined` when there are no ast-grep rules. */ + sg: string | undefined; +} + +export async function assembleEngineConfigs( + cwd: string +): Promise { + const [vale, sg] = await Promise.all([ + assembleValeConfig(cwd), + assembleSgConfig(cwd), + ]); + return { vale, sg }; +} + +/** Exported for the assembly tests, which assert on the artifact directly. */ +export const ASSEMBLY_INTERNALS = { + ruleConfigBody, + valeHeader, + RULE_TESTS_DIRECTORY, +}; diff --git a/packages/cli/src/rules/dispatch.ts b/packages/cli/src/rules/dispatch.ts index ec6eb6c..1d30e84 100644 --- a/packages/cli/src/rules/dispatch.ts +++ b/packages/cli/src/rules/dispatch.ts @@ -1,8 +1,6 @@ import { readdir } from "node:fs/promises"; -import { join } from "node:path"; - import type { CheckResult } from "../types/check"; -import { dedupeFindings, ENGINE_LAYOUTS, type EngineName } from "./engines"; +import { engineRulesDirectory, type EngineName } from "./engines"; import { executeRuntimeRules } from "./runtime/harness"; import type { RuntimeRule } from "./runtime/discover"; import { runAstGrepScan } from "./scan"; @@ -29,10 +27,15 @@ const ABSENT_DIRECTORY_CODES = new Set(["ENOENT", "ENOTDIR"]); */ export async function hasValeRules(cwd: string): Promise { try { - const entries = await readdir( - join(cwd, ".taskless", ENGINE_LAYOUTS.vale.rulesDirectory) - ); - return entries.some((entry) => entry.endsWith(".yml")); + // A rule is a *directory* now, so this counts directories rather than + // `*.yml` files. Looking for loose `.yml` here is how the whole engine + // would read as "no rules configured" against a correctly laid-out + // project — a silent skip, which is the failure this gate exists to make + // impossible. + const entries = await readdir(engineRulesDirectory(cwd, "vale"), { + withFileTypes: true, + }); + return entries.some((entry) => entry.isDirectory()); } catch (error) { const code = (error as NodeJS.ErrnoException).code; if (code !== undefined && ABSENT_DIRECTORY_CODES.has(code)) return false; @@ -74,10 +77,11 @@ export interface DispatchOptions { */ paths: string[]; /** - * One `--config` path per ast-grep rule source, already resolved. The source - * each was derived from is the caller's concern; dispatch only runs configs. + * The assembled ast-grep `--config` path, or `undefined` when the project has + * no ast-grep rules. One config, because assembly produced one — dispatch + * runs it and does not know how it was built. */ - astGrepConfigPaths: string[]; + astGrepConfigPath: string | undefined; /** Runtime rules that survived planning. Empty means the harness is skipped. */ runtimeRules: RuntimeRule[]; runtimeTimeoutMs?: number; @@ -110,23 +114,22 @@ export interface DispatchResult { } /** - * ast-grep over every source, deduped. + * ast-grep over the assembled config. * - * `sg/rules/` and the legacy `.taskless/rules/` are scanned separately, so a - * rule present in both reports twice; the finding is its own identity, so - * identical matches collapse. + * One scan, because there is one rule tree. The previous layout scanned the + * engine directory and the legacy directory separately and deduplicated the + * overlap; with a single tree there is no overlap to collapse. */ async function runAstGrepEngine( options: DispatchOptions ): Promise { - const results: CheckResult[] = []; - for (const configPath of options.astGrepConfigPaths) { - const scan = await runAstGrepScan(options.cwd, options.paths, { - configPath, - }); - results.push(...scan.results); + if (options.astGrepConfigPath === undefined) { + return { engine: "sg", results: [] }; } - return { engine: "sg", results: dedupeFindings(results) }; + const scan = await runAstGrepScan(options.cwd, options.paths, { + configPath: options.astGrepConfigPath, + }); + return { engine: "sg", results: scan.results }; } /** diff --git a/packages/cli/src/rules/engines.ts b/packages/cli/src/rules/engines.ts index 82ce211..d8151aa 100644 --- a/packages/cli/src/rules/engines.ts +++ b/packages/cli/src/rules/engines.ts @@ -1,13 +1,12 @@ import { readdir } from "node:fs/promises"; import { join } from "node:path"; -import type { CheckResult } from "../types/check"; import { CLIError } from "../util/cli-error"; /** - * Engines this CLI knows. The directory name under `.taskless/` **is** the - * engine: dispatch reads the path and never parses a rule file to decide who - * owns it. + * Engines this CLI knows. The directory name under `.taskless/rules/` **is** + * the engine: dispatch reads the path and never parses a rule file to decide + * who owns it. */ export const ENGINES = ["sg", "vale", "runtime"] as const; @@ -22,86 +21,181 @@ export type EngineExecutor = export interface EngineLayout { engine: EngineName; - /** Rules directory, relative to `.taskless/`. */ - rulesDirectory: string; - /** Rule-tests directory, relative to `.taskless/`. */ - ruleTestsDirectory: string; - /** The engine's native config, relative to `.taskless/`. */ - configFile: string | undefined; + /** + * The file inside a rule directory that *is* the rule, as a function of the + * rule id. `sg` and `vale` name it after the rule; `runtime` always calls it + * `check.ts`, because the rule is a program rather than a document. + */ + ruleFile: (ruleId: string) => string; + /** + * The engine's per-rule config file, or `undefined` where the engine has + * nothing to put in one. + * + * Only Vale has one, and not for symmetry: Vale cannot express a rule's scope + * inside the style file — measured, it rejects unknown keys with `E201` — so + * scope needs somewhere else to live. ast-grep carries `files`/`ignores` + * inside the rule itself, so an `sg` per-rule config would be a file every + * author creates, no author fills, and every reader learns to ignore. + */ + ruleConfigFile: string | undefined; + /** Subdirectory holding ast-grep capture rules, for engines that use them. */ + capturesDirectory: string | undefined; executor: EngineExecutor; } +/** + * Everything defining a rule lives in one directory, + * `.taskless/rules///`, the same shape for every engine. A rule is + * therefore one path — which is what lets `verify` and `test` take a path + * instead of an id, and what makes deleting a rule an `rm -rf` of one thing. + */ +export const RULES_DIRECTORY = "rules"; + +/** + * A rule's tests, relative to its rule directory. **The dot is load-bearing.** + * + * 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 entire scan + * with `Fail to parse yaml as RuleConfig: missing field 'language'`. Measured + * against the pinned ast-grep 0.41.0: `tests/` and `__tests__/` both hard-fail, + * a dot-directory is skipped by rule discovery, and `sg test` still reads it + * when `testDir` names it. + * + * That is undocumented behavior, and three things make depending on it + * acceptable. The failure is loud — a parse error naming the file, never a test + * silently reinterpreted as a rule. `engine-layout.test.ts` pins it, so it is + * checked on every run rather than remembered. And the binary is pinned to an + * exact version, so it cannot change without a deliberate bump, which is + * exactly where that test fires. + * + * If it ever does break, the recorded fallback is to materialize a rules-only + * tree for ast-grep and point `ruleDirs` at that (design D2). + */ +export const RULE_TESTS_DIRECTORY = ".tests"; + export const ENGINE_LAYOUTS = { sg: { engine: "sg", - rulesDirectory: "sg/rules", - ruleTestsDirectory: "sg/rule-tests", - configFile: "sg/sgconfig.yml", + ruleFile: (ruleId: string) => `${ruleId}.yml`, + ruleConfigFile: undefined, + capturesDirectory: undefined, executor: "ast-grep", }, vale: { engine: "vale", - rulesDirectory: "vale/rules", - ruleTestsDirectory: "vale/rule-tests", - configFile: "vale/.vale.ini", + ruleFile: (ruleId: string) => `${ruleId}.yml`, + ruleConfigFile: ".vale.ini", + capturesDirectory: undefined, executor: "vale-runner", }, runtime: { engine: "runtime", - rulesDirectory: "runtime/rules", - ruleTestsDirectory: "runtime/rule-tests", - configFile: undefined, + ruleFile: () => "check.ts", + ruleConfigFile: undefined, + // `captures/` rather than `matchers/`: "matcher" denotes a Vale `[]` + // config section elsewhere in this tree, and one word for two unrelated + // concepts is a cost paid at every future reading. + capturesDirectory: "captures", executor: "runtime-harness", }, } satisfies Record; -/** - * The committed ast-grep config, relative to the project root. It is authored - * and persisted, never generated at check time: its `ruleDirs`/`testConfigs` - * are relative to the config file, so it needs no rewriting to stay valid. - * - * Declared here beside the layout it derives from. `engines.ts` imports nothing - * of ours but the error type, so both the filesystem and rules layers can reach - * this constant without either pulling in the other's machinery. - */ -export const COMMITTED_SG_CONFIG = `.taskless/${ENGINE_LAYOUTS.sg.configFile}`; +/** `.taskless/rules`, the root every rule lives under. */ +export function rulesRoot(cwd: string): string { + return join(cwd, ".taskless", RULES_DIRECTORY); +} -/** - * The pre-`0004` ast-grep locations. Still dispatched as ast-grep so an - * unmigrated checkout — or a producer that keeps naming the old path — runs - * rather than being silently ignored. - */ -export const LEGACY_RULES_DIRECTORY = "rules"; -export const LEGACY_RULE_TESTS_DIRECTORY = "rule-tests"; +/** `.taskless/rules/`, the directory holding that engine's rules. */ +export function engineRulesDirectory(cwd: string, engine: EngineName): string { + return join(rulesRoot(cwd), engine); +} -export function isKnownEngine(value: string): value is EngineName { - return (ENGINES as readonly string[]).includes(value); +/** `.taskless/rules//` — the one path that means "this rule". */ +export function ruleDirectory( + cwd: string, + engine: EngineName, + ruleId: string +): string { + return join(engineRulesDirectory(cwd, engine), ruleId); +} + +/** The file inside a rule directory that is the rule itself. */ +export function ruleFilePath( + cwd: string, + engine: EngineName, + ruleId: string +): string { + return join( + ruleDirectory(cwd, engine, ruleId), + ENGINE_LAYOUTS[engine].ruleFile(ruleId) + ); +} + +/** A rule's tests directory. */ +export function ruleTestsDirectory( + cwd: string, + engine: EngineName, + ruleId: string +): string { + return join(ruleDirectory(cwd, engine, ruleId), RULE_TESTS_DIRECTORY); +} + +/** A rule's own engine config, for the one engine that has one. */ +export function ruleConfigPath( + cwd: string, + engine: EngineName, + ruleId: string +): string | undefined { + const configFile = ENGINE_LAYOUTS[engine].ruleConfigFile; + if (configFile === undefined) return undefined; + return join(ruleDirectory(cwd, engine, ruleId), configFile); +} + +/** A runtime rule's capture-rule directory. */ +export function ruleCapturesDirectory( + cwd: string, + engine: EngineName, + ruleId: string +): string | undefined { + const capturesDirectory = ENGINE_LAYOUTS[engine].capturesDirectory; + if (capturesDirectory === undefined) return undefined; + return join(ruleDirectory(cwd, engine, ruleId), capturesDirectory); } -/** One engine directory's disposition for this run. */ +/** + * The assembled engine configs, relative to the project root. + * + * Both are **generated per run and 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. Committing that file + * would recreate the shared, write-contended config this layout exists to + * remove, by a different route. + */ +export const ASSEMBLED_VALE_CONFIG = ".taskless/.vale.ini"; +export const ASSEMBLED_SG_CONFIG = ".taskless/.sgconfig.yml"; + +/** One engine's disposition for this run. */ export interface EngineDispatch { engine: EngineName; - /** Whether `.taskless//` exists on disk. */ + /** Whether `.taskless/rules//` exists on disk. */ present: boolean; executor: EngineExecutor; } -/** Directory entries of `.taskless/`, or `[]` when it does not exist. */ -async function readTasklessEntries(cwd: string): Promise> { +/** Directory names directly under `directory`, or `[]` when it is not there. */ +async function subdirectories(directory: string): Promise { try { - const entries = await readdir(join(cwd, ".taskless"), { - withFileTypes: true, - }); - return new Set( - entries.filter((entry) => entry.isDirectory()).map((entry) => entry.name) - ); + const entries = await readdir(directory, { withFileTypes: true }); + return entries + .filter((entry) => entry.isDirectory()) + .map((entry) => entry.name); } catch { - return new Set(); + return []; } } /** - * Resolve which engine directories are present under `.taskless/`. + * Resolve which engine directories are present under `.taskless/rules/`. * * Only known engines are returned. A directory this CLI does not recognize is * ignored — never guessed at, never handed to another engine's parser — so a @@ -111,7 +205,7 @@ async function readTasklessEntries(cwd: string): Promise> { export async function planEngineDispatch( cwd: string ): Promise { - const directories = await readTasklessEntries(cwd); + const directories = new Set(await subdirectories(rulesRoot(cwd))); return ENGINES.map((engine) => ({ engine, present: directories.has(engine), @@ -119,112 +213,23 @@ export async function planEngineDispatch( })); } -/** Rule ids (filename stems) of the `*.yml` files directly in `directory`. */ -async function listRuleIds(directory: string): Promise { - try { - const entries = await readdir(directory); - return entries - .filter((entry) => entry.endsWith(".yml")) - .map((entry) => entry.slice(0, -".yml".length)); - } catch { - return []; - } -} - -/** A directory of ast-grep rule files, and where its tests live. */ -export interface AstGrepRuleSource { - /** Rules directory, relative to `.taskless/`. */ - rulesDirectory: string; - /** Rule-tests directory, relative to `.taskless/`. */ - ruleTestsDirectory: string; - /** Absolute path to the rules directory. */ - absoluteRulesDirectory: string; - /** Rule ids found in the directory. */ - ruleIds: string[]; - /** Whether this is the pre-`0004` location. */ - legacy: boolean; -} - /** - * Every directory whose rules ast-grep should run: the `sg` engine directory - * and, when it still holds rules, the legacy `.taskless/rules/`. + * Rule ids for one engine — the directory names under `.taskless/rules/`. * - * A source with no rule files is omitted, so a scaffolded-but-empty `sg/rules/` - * costs nothing. The `sg` source comes first; callers de-duplicate findings - * ({@link dedupeFindings}) rather than dropping a source, since a rule id can - * legitimately exist in only one of the two. - */ -export async function discoverAstGrepRuleSources( - cwd: string -): Promise { - const dispatch = await planEngineDispatch(cwd); - const sgPresent = - dispatch.find((entry) => entry.engine === "sg")?.present === true; - - const candidates: Array> = []; - if (sgPresent) { - const layout = ENGINE_LAYOUTS.sg; - candidates.push({ - rulesDirectory: layout.rulesDirectory, - ruleTestsDirectory: layout.ruleTestsDirectory, - absoluteRulesDirectory: join(cwd, ".taskless", layout.rulesDirectory), - legacy: false, - }); - } - candidates.push({ - rulesDirectory: LEGACY_RULES_DIRECTORY, - ruleTestsDirectory: LEGACY_RULE_TESTS_DIRECTORY, - absoluteRulesDirectory: join(cwd, ".taskless", LEGACY_RULES_DIRECTORY), - legacy: true, - }); - - const sources: AstGrepRuleSource[] = []; - for (const candidate of candidates) { - const ruleIds = await listRuleIds(candidate.absoluteRulesDirectory); - if (ruleIds.length === 0) continue; - sources.push({ ...candidate, ruleIds }); - } - return sources; -} - -/** - * Collapse findings that describe the same match. Scanning both `sg/rules/` and - * the legacy `rules/` means a rule present in both reports twice; the finding - * itself is the identity, so an identical match from either source is reported - * once. - */ -export function dedupeFindings(results: CheckResult[]): CheckResult[] { - const seen = new Set(); - const unique: CheckResult[] = []; - for (const result of results) { - const key = [ - result.ruleId, - result.file, - result.range.start.line, - result.range.start.column, - result.range.end.line, - result.range.end.column, - result.message, - ].join("\u0000"); - if (seen.has(key)) continue; - seen.add(key); - unique.push(result); - } - return unique; -} - -/** - * Candidate locations of a single ast-grep rule file, in resolution order: - * the `sg` engine directory first, the legacy path second. + * A rule is a directory, so this lists directories rather than file stems. A + * stray *file* in an engine directory is not a rule and is skipped: the only + * thing that makes something a rule is being a directory in the right place. + * + * Sorted, because assembly order is a correctness constraint. Vale's matcher + * precedence is positional, so a config assembled in directory-iteration order + * would give a rule a different effective scope on different machines. */ -export function astGrepRuleFileCandidates( +export async function listRuleIds( cwd: string, - ruleId: string -): string[] { - return [ - join(cwd, ".taskless", ENGINE_LAYOUTS.sg.rulesDirectory, `${ruleId}.yml`), - join(cwd, ".taskless", LEGACY_RULES_DIRECTORY, `${ruleId}.yml`), - ]; + engine: EngineName +): Promise { + const ids = await subdirectories(engineRulesDirectory(cwd, engine)); + return ids.toSorted((a, b) => a.localeCompare(b)); } /** @@ -234,8 +239,8 @@ export function astGrepRuleFileCandidates( * documents `rules[].content` as an ast-grep rule definition — so a payload * that identifies no engine **is** ast-grep. That default is permanent, not a * migration window: published CLIs keep receiving engine-less payloads, and it - * files a delivered rule exactly where migration `0004` puts the same rule - * already on disk. + * files a delivered rule exactly where the migrations put the same rule already + * on disk. * * Absence and an unrecognized value are different. An engine this CLI does not * know means the payload is newer than the CLI; defaulting it to `sg` would @@ -261,10 +266,6 @@ export function resolveIngestEngine(payload: unknown): EngineName { return declared; } -/** Candidate rule-test directories, in the same resolution order. */ -export function astGrepRuleTestDirectories(cwd: string): string[] { - return [ - join(cwd, ".taskless", ENGINE_LAYOUTS.sg.ruleTestsDirectory), - join(cwd, ".taskless", LEGACY_RULE_TESTS_DIRECTORY), - ]; +export function isKnownEngine(value: string): value is EngineName { + return (ENGINES as readonly string[]).includes(value); } diff --git a/packages/cli/src/rules/files.ts b/packages/cli/src/rules/files.ts index bd19467..a3cdb42 100644 --- a/packages/cli/src/rules/files.ts +++ b/packages/cli/src/rules/files.ts @@ -1,4 +1,4 @@ -import { mkdir, readdir, readFile, rm, writeFile } from "node:fs/promises"; +import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; import { basename, join, resolve } from "node:path"; import { parse, stringify } from "yaml"; @@ -6,16 +6,16 @@ import { parse, stringify } from "yaml"; import { ensureTasklessDirectory } from "../filesystem/directory"; import type { GeneratedRule, RuleMetadata } from "../api/rules"; import { - ENGINE_LAYOUTS, - astGrepRuleFileCandidates, - astGrepRuleTestDirectories, resolveIngestEngine, + ruleDirectory, + ruleFilePath, + ruleTestsDirectory, } from "./engines"; import { isValidRuleId } from "./validate-id"; /** - * Write a generated rule's content into the engine directory its payload - * identifies — `.taskless/sg/rules/{kebab-id}.yml` for the engine-less payloads + * Write a generated rule's content into its own rule directory — + * `.taskless/rules/sg/{kebab-id}/{kebab-id}.yml` for the engine-less payloads * the API delivers today (see {@link resolveIngestEngine}). */ export async function writeRuleFile( @@ -29,20 +29,15 @@ export async function writeRuleFile( // must write nothing at all. const engine = resolveIngestEngine(rule); await ensureTasklessDirectory(cwd); - const directory = join( - cwd, - ".taskless", - ENGINE_LAYOUTS[engine].rulesDirectory - ); - await mkdir(directory, { recursive: true }); - const filePath = join(directory, `${rule.id}.yml`); + await mkdir(ruleDirectory(cwd, engine, rule.id), { recursive: true }); + const filePath = ruleFilePath(cwd, engine, rule.id); await writeFile(filePath, stringify(rule.content, { lineWidth: 0 }), "utf8"); return filePath; } /** - * Write a rule's test cases to that engine's rule-tests directory — - * `.taskless/sg/rule-tests/{kebab-id}-{timestamp}-test.yml` by default. + * Write a rule's test cases inside its own rule directory — + * `.taskless/rules/sg/{kebab-id}/.tests/{kebab-id}-{timestamp}-test.yml`. */ export async function writeRuleTestFile( cwd: string, @@ -54,11 +49,7 @@ export async function writeRuleTestFile( } const engine = resolveIngestEngine(rule); await ensureTasklessDirectory(cwd); - const directory = join( - cwd, - ".taskless", - ENGINE_LAYOUTS[engine].ruleTestsDirectory - ); + const directory = ruleTestsDirectory(cwd, engine, rule.id); await mkdir(directory, { recursive: true }); const filePath = join(directory, `${rule.id}-${timestamp}-test.yml`); const content = { @@ -123,51 +114,15 @@ export async function deleteRuleFiles( if (!isValidRuleId(id)) { return false; } - // Delete from every layout the CLI dispatches, so a rule that still lives at - // the legacy path is removed rather than reported as missing. - let ruleExisted = false; - for (const ruleFilePath of astGrepRuleFileCandidates(cwd, id)) { - try { - await rm(ruleFilePath); - ruleExisted = true; - } catch { - // Not in this layout — try the next. - } - } - if (!ruleExisted) return false; - - // Remove matching test files - for (const testDirectory of astGrepRuleTestDirectories(cwd)) { - try { - const entries = await readdir(testDirectory); - const matchingTests = entries.filter( - (f) => f.startsWith(`${id}-`) && f.endsWith("-test.yml") - ); - await Promise.all( - matchingTests.map((f) => - rm(join(testDirectory, f)).catch((error: NodeJS.ErrnoException) => { - if (error.code !== "ENOENT") { - console.error( - `Warning: failed to remove test file ${f}: ${error.message}` - ); - } - }) - ) - ); - } catch (error) { - if ( - !( - error && - typeof error === "object" && - "code" in error && - (error as NodeJS.ErrnoException).code === "ENOENT" - ) - ) { - console.error( - `Warning: failed to clean up test files: ${(error as Error).message}` - ); - } - } + // A rule is one directory, so deleting it is removing that directory. Its + // tests live inside, which is the point of the layout: there is no second + // place to remember, and no way to leave a rule half-deleted. + const directory = ruleDirectory(cwd, "sg", id); + try { + await rm(directory, { recursive: true }); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return false; + throw error; } // Remove matching metadata file @@ -189,5 +144,5 @@ export async function deleteRuleFiles( } } - return ruleExisted; + return true; } diff --git a/packages/cli/src/rules/owner.ts b/packages/cli/src/rules/owner.ts deleted file mode 100644 index 68509ae..0000000 --- a/packages/cli/src/rules/owner.ts +++ /dev/null @@ -1,73 +0,0 @@ -import { stat } from "node:fs/promises"; -import { join } from "node:path"; - -import { - astGrepRuleFileCandidates, - ENGINE_LAYOUTS, - type EngineName, -} from "./engines"; - -/** - * Which engines have a rule file for `ruleId`. - * - * Ownership is decided by **where the file is**, never by parsing it — the same - * rule `dispatch` follows, so a rule cannot be verified by one engine and run - * by another. `.taskless/vale/rules/.yml` is Vale's; the ast-grep - * candidates (including the pre-`0004` location) are `sg`'s. - * - * Returns every match rather than a single winner. Two engines holding the same - * id is a real state — a project that authored `no-simply` under both — and - * picking one silently would verify a file the user was not asking about. The - * caller reports the ambiguity and names both paths. - * - * A missing directory or a path whose ancestor is a file both read as "not - * here"; anything else is a real IO problem and propagates. - */ -export async function rulefileOwners( - cwd: string, - ruleId: string -): Promise { - const candidates: Array<[EngineName, string[]]> = [ - ["sg", astGrepRuleFileCandidates(cwd, ruleId)], - [ - "vale", - [ - join( - cwd, - ".taskless", - ENGINE_LAYOUTS.vale.rulesDirectory, - `${ruleId}.yml` - ), - ], - ], - ]; - - const owners: EngineName[] = []; - for (const [engine, paths] of candidates) { - for (const path of paths) { - if (await isFile(path)) { - owners.push(engine); - break; - } - } - } - return owners; -} - -/** Whether `path` is an existing regular file. */ -async function isFile(path: string): Promise { - try { - const stats = await stat(path); - return stats.isFile(); - } catch (error) { - const { code } = error as NodeJS.ErrnoException; - if (code === "ENOENT" || code === "ENOTDIR") return false; - throw error; - } -} - -/** Where a rule of each engine lives, for an error message that can be acted on. */ -export function ruleFileLocation(engine: EngineName, ruleId: string): string { - const layout = ENGINE_LAYOUTS[engine]; - return `.taskless/${layout.rulesDirectory}/${ruleId}.yml`; -} diff --git a/packages/cli/src/rules/runtime/discover.ts b/packages/cli/src/rules/runtime/discover.ts index 0ecde0a..bc4325c 100644 --- a/packages/cli/src/rules/runtime/discover.ts +++ b/packages/cli/src/rules/runtime/discover.ts @@ -4,15 +4,15 @@ import { join } from "node:path"; import { parse } from "yaml"; import type { CaptureRule, MatchMode } from "../../types/runtime-rule"; -import { ENGINE_LAYOUTS } from "../engines"; +import { RULES_DIRECTORY } from "../engines"; /** * Directory (relative to `.taskless/`) that holds runtime rules — the * `runtime` engine's own directory, so this tracks the engine layout rather - * than repeating it. Migration `0004` moved the tree here from - * `runtime-rules/` without touching a byte, so signatures are unaffected. + * than repeating it. Migrations `0004` and `0005` moved the tree here without + * touching a byte, so signatures are unaffected. */ -export const RUNTIME_RULES_DIR = ENGINE_LAYOUTS.runtime.rulesDirectory; +export const RUNTIME_RULES_DIR = join(RULES_DIRECTORY, "runtime"); /** A parsed capture `*.yml` of a runtime rule, with the fields the harness needs. */ export interface LoadedCaptureRule { @@ -128,7 +128,10 @@ export async function discoverRuntimeRulesIn( for (const entry of sorted) { if (!entry.isDirectory()) continue; const directory = join(root, entry.name); - const captureRules = await loadCaptureRules(directory); + // Capture rules live in `captures/`, not at the rule root. The name avoids + // "matcher", which denotes a Vale `[]` config section elsewhere in + // this same tree. + const captureRules = await loadCaptureRules(join(directory, "captures")); if (captureRules.length === 0) continue; // not a runtime rule // The check file is always `check.ts` inside the rule directory (per spec). diff --git a/packages/cli/src/rules/scan.ts b/packages/cli/src/rules/scan.ts index 107d507..8f0847d 100644 --- a/packages/cli/src/rules/scan.ts +++ b/packages/cli/src/rules/scan.ts @@ -5,7 +5,7 @@ import { fileURLToPath } from "node:url"; import type { AstGrepMatch } from "../types/check"; import { toCheckResult, type CheckResult } from "../types/check"; -import { COMMITTED_SG_CONFIG } from "./engines"; +import { ASSEMBLED_SG_CONFIG } from "./engines"; import { isPlatformBinary, pathCommandName, @@ -138,7 +138,7 @@ export async function runAstGrepScan( const argv = [ "scan", "--config", - options.configPath ?? COMMITTED_SG_CONFIG, + options.configPath ?? ASSEMBLED_SG_CONFIG, "--json=stream", ...(paths.length > 0 ? ["--", ...paths] : []), ]; diff --git a/packages/cli/src/rules/vale/run.ts b/packages/cli/src/rules/vale/run.ts index 7c1fb09..67cf932 100644 --- a/packages/cli/src/rules/vale/run.ts +++ b/packages/cli/src/rules/vale/run.ts @@ -3,7 +3,7 @@ import { join } from "node:path"; import { StringDecoder } from "node:string_decoder"; import type { CheckResult } from "../../types/check"; -import { ENGINE_LAYOUTS } from "../engines"; +import { ASSEMBLED_VALE_CONFIG } from "../engines"; import { buildPath } from "../scan"; import { findValeBinary, valeUnavailableMessage } from "./binary"; import { asValeConfigError, toValeCheckResults, type ValeOutput } from "./map"; @@ -11,8 +11,14 @@ import { asValeConfigError, toValeCheckResults, type ValeOutput } from "./map"; /** Taskless's own directory, as a project-relative path. */ const TASKLESS_DIRECTORY = ".taskless"; -/** The committed Vale config, relative to the project root. */ -export const COMMITTED_VALE_CONFIG = `${TASKLESS_DIRECTORY}/${ENGINE_LAYOUTS.vale.configFile}`; +/** + * The Vale config a run reads, relative to the project root. + * + * Assembled from every rule's own `.vale.ini` rather than committed — see + * `rules/assemble.ts`. Vale accepts exactly one `--config`, so per-rule + * configuration has to reach one file before it can be invoked. + */ +export { ASSEMBLED_VALE_CONFIG }; /** * How long a single Vale invocation may run before it is killed. @@ -77,25 +83,26 @@ export type ValeRunOutcome = | { status: "failed"; blocking: true; message: string }; export interface ValeRunOptions { - /** Project root. Vale runs here, so its config paths resolve as committed. */ + /** Project root. Vale runs here, so the config's relative paths resolve. */ cwd: string; /** Target paths, relative to `cwd`. Empty means Vale's own default set. */ paths?: string[]; - /** Config path relative to `cwd`. Defaults to the committed engine config. */ + /** Config path relative to `cwd`. Defaults to the assembled run config. */ configPath?: string; timeoutMs?: number; } /** - * Run Vale over `paths` using the committed config, and map what it reports. + * Run Vale over `paths` using the assembled run config, and map what it reports. * * `--no-exit` is what makes the exit code readable: without it Vale exits * non-zero merely because it found something, which is indistinguishable from * failing to run. With it, a non-zero exit means Vale itself failed. * - * The config is read as committed rather than generated per run — it is the - * source of truth for scoping (matchers), and rewriting it at check time would - * mean the file a user edits is not the file that executes. + * The config is assembled from each rule's own `.vale.ini` rather than read + * from one committed file. The per-rule configs remain the source of truth for + * scoping, so the matchers a user edits are exactly the matchers that execute — + * assembly concatenates them in a deterministic order and adds nothing. */ export async function runVale( options: ValeRunOptions @@ -109,7 +116,7 @@ export async function runVale( }; } - const configPath = options.configPath ?? COMMITTED_VALE_CONFIG; + const configPath = options.configPath ?? ASSEMBLED_VALE_CONFIG; const paths = options.paths ?? []; const timeoutMs = options.timeoutMs ?? VALE_TIMEOUT_MS; @@ -125,7 +132,7 @@ export async function runVale( // Walking the whole project reaches `.taskless/` too, and Vale has no reason // to know that directory is ours: with a rule enabled it reports findings in - // the committed `.vale.ini` and in the user's own rule definitions — prose + // the rule configs and in the user's own rule definitions — prose // complaints about the machinery, pointing at files nobody wrote as prose. // Section globs do not help, since `.taskless/README.md` matches `[*.md]` as // readily as any document. `--glob` filters which files are walked without @@ -291,5 +298,5 @@ export async function runVale( /** Absolute path of the committed Vale config for `cwd`. */ export function valeConfigPath(cwd: string): string { - return join(cwd, COMMITTED_VALE_CONFIG); + return join(cwd, ASSEMBLED_VALE_CONFIG); } diff --git a/packages/cli/src/rules/vale/verify.ts b/packages/cli/src/rules/vale/verify.ts index a00c844..e8833a2 100644 --- a/packages/cli/src/rules/vale/verify.ts +++ b/packages/cli/src/rules/vale/verify.ts @@ -3,13 +3,12 @@ import { readdir } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, posix, relative, resolve, sep } from "node:path"; -import { ENGINE_LAYOUTS } from "../engines"; +import { listRuleIds, ruleTestsDirectory } from "../engines"; import { runVale, type ValeRunOutcome } from "./run"; /** Where a rule's fixtures live, relative to the project root. */ -export function valeRuleTestsDirectory(cwd: string, ruleId?: string): string { - const base = join(cwd, ".taskless", ENGINE_LAYOUTS.vale.ruleTestsDirectory); - return ruleId === undefined ? base : join(base, ruleId); +export function valeRuleTestsDirectory(cwd: string, ruleId: string): string { + return ruleTestsDirectory(cwd, "vale", ruleId); } /** The styles root Vale resolves `rules.` against. */ @@ -184,13 +183,20 @@ export type ValeVerifyOutcome = | { status: "unavailable"; message: string } | { status: "failed"; message: string }; -/** Rule ids that have a `rule-tests//` directory. */ +/** + * Rule ids that have a populated `.tests/` directory. + * + * Walks the rule directories rather than a shared tests root: a rule's tests + * live inside the rule, so "has tests" is a question about the rule directory. + */ export async function discoverValeRuleTests(cwd: string): Promise { - const entries = await directoryEntries(valeRuleTestsDirectory(cwd)); - return entries - .filter((entry) => entry.isDirectory()) - .map((entry) => entry.name) - .toSorted(); + const ruleIds = await listRuleIds(cwd, "vale"); + const withTests: string[] = []; + for (const ruleId of ruleIds) { + const entries = await directoryEntries(valeRuleTestsDirectory(cwd, ruleId)); + if (entries.length > 0) withTests.push(ruleId); + } + return withTests; } /** diff --git a/packages/cli/src/rules/verify.ts b/packages/cli/src/rules/verify.ts index c07c470..6689474 100644 --- a/packages/cli/src/rules/verify.ts +++ b/packages/cli/src/rules/verify.ts @@ -9,16 +9,12 @@ import { findRegexWithoutKind, } from "../schemas/ast-grep-rule"; import { ensureTasklessDirectory } from "../filesystem/directory"; +import { assembleSgConfig } from "./assemble"; import { - resolveSgConfigPath, - type SgConfigSource, -} from "../filesystem/sgconfig"; -import { - astGrepRuleFileCandidates, - astGrepRuleTestDirectories, - ENGINE_LAYOUTS, - LEGACY_RULES_DIRECTORY, - LEGACY_RULE_TESTS_DIRECTORY, + RULE_TESTS_DIRECTORY, + RULES_DIRECTORY, + ruleFilePath, + ruleTestsDirectory, } from "./engines"; import { findSgBinary, buildPath } from "./scan"; import astGrepJsonSchema from "../generated/ast-grep-rule-schema.json"; @@ -139,28 +135,21 @@ async function validateRequirements( } } - // Check a test file exists, in either layout the CLI dispatches. + // A rule's tests live inside the rule directory, so there is one place to + // look and no resolution order to get wrong. let hasTestFile = false; - for (const testDirectory of astGrepRuleTestDirectories(cwd)) { - try { - const entries = await readdir(testDirectory); - if ( - entries.some( - (f) => f.startsWith(`${ruleId}-`) && f.endsWith("-test.yml") - ) - ) { - hasTestFile = true; - break; - } - } catch { - // directory doesn't exist - } + try { + const entries = await readdir(ruleTestsDirectory(cwd, "sg", ruleId)); + hasTestFile = entries.some( + (f) => f.startsWith(`${ruleId}-`) && f.endsWith("-test.yml") + ); + } catch { + // No tests directory — hasTestFile stays false. } if (!hasTestFile) { errors.push( `No test file found for rule "${ruleId}" in ` + - `.taskless/${ENGINE_LAYOUTS.sg.ruleTestsDirectory}/ or ` + - `.taskless/${LEGACY_RULE_TESTS_DIRECTORY}/` + `.taskless/${RULES_DIRECTORY}/sg/${ruleId}/${RULE_TESTS_DIRECTORY}/` ); } @@ -171,13 +160,20 @@ async function validateRequirements( async function runTests( cwd: string, - ruleId: string, - layout: SgConfigSource + ruleId: string ): Promise { - // Point ast-grep at the layout the rule was actually resolved from: the - // committed `sg/sgconfig.yml` over `sg/rule-tests/`, or a generated config - // when the rule still lives at the pre-migration path. - const configPath = await resolveSgConfigPath(cwd, layout); + // Assembly names every rule's `.tests/` as its own `testConfigs` entry, so + // the filter below selects a rule whose tests ast-grep already knows how to + // find. + const configPath = await assembleSgConfig(cwd); + if (configPath === undefined) { + return { + valid: false, + errors: ["No ast-grep rules are present, so no tests could be run."], + passed: 0, + failed: 0, + }; + } const sgBinary = findSgBinary(); @@ -268,34 +264,16 @@ export async function verifyRule( }; } - // Settle the layout before resolving anything: the migration moves rules - // between the two candidate paths, so resolving first and migrating later - // would point ast-grep at a directory the migration has just emptied. + // Settle the layout before resolving anything: the migrations move rules, so + // resolving first and migrating later would read a directory the migration + // has just emptied. await ensureTasklessDirectory(cwd); - // Resolve the rule from the engine directory first, then the legacy path, - // and remember which layout won so the test run points ast-grep at it. - const candidates = astGrepRuleFileCandidates(cwd, ruleId); let ruleContent: string | undefined; - let layout: SgConfigSource = { - rulesDirectory: ENGINE_LAYOUTS.sg.rulesDirectory, - ruleTestsDirectory: ENGINE_LAYOUTS.sg.ruleTestsDirectory, - legacy: false, - }; - for (const [index, candidate] of candidates.entries()) { - try { - ruleContent = await readFile(candidate, "utf8"); - if (index > 0) { - layout = { - rulesDirectory: LEGACY_RULES_DIRECTORY, - ruleTestsDirectory: LEGACY_RULE_TESTS_DIRECTORY, - legacy: true, - }; - } - break; - } catch { - // Not in this layout — try the next. - } + try { + ruleContent = await readFile(ruleFilePath(cwd, "sg", ruleId), "utf8"); + } catch { + // Reported below as a missing rule file. } if (ruleContent === undefined) { @@ -305,8 +283,7 @@ export async function verifyRule( schema: { valid: false, errors: [ - `Rule file not found: .taskless/${ENGINE_LAYOUTS.sg.rulesDirectory}/${ruleId}.yml ` + - `or .taskless/${LEGACY_RULES_DIRECTORY}/${ruleId}.yml`, + `Rule file not found: .taskless/${RULES_DIRECTORY}/sg/${ruleId}/${ruleId}.yml`, ], }, requirements: { @@ -359,7 +336,7 @@ export async function verifyRule( // Layer 3 — only if test file exists (Layer 2 checks this) const testResult = requirementsResult.hasTestFile - ? await runTests(cwd, ruleId, layout) + ? await runTests(cwd, ruleId) : { valid: false, errors: ["Skipped: no test file found"], diff --git a/packages/cli/test/sgconfig.test.ts b/packages/cli/test/sgconfig.test.ts deleted file mode 100644 index 8907c68..0000000 --- a/packages/cli/test/sgconfig.test.ts +++ /dev/null @@ -1,124 +0,0 @@ -import { mkdtemp, rm, readFile, mkdir, writeFile } from "node:fs/promises"; -import { join } from "node:path"; -import { tmpdir } from "node:os"; -import { describe, expect, it, beforeEach, afterEach } from "vitest"; - -import { generateSgConfig } from "../src/filesystem/sgconfig"; -import { addToGitignore } from "../src/filesystem/gitignore"; - -describe("generateSgConfig", () => { - let temporaryDirectory: string; - - beforeEach(async () => { - temporaryDirectory = await mkdtemp(join(tmpdir(), "taskless-sgconfig-")); - }); - - afterEach(async () => { - await rm(temporaryDirectory, { recursive: true, force: true }); - }); - - it("writes sgconfig.yml pointing at the sg engine directory by default", async () => { - await generateSgConfig(temporaryDirectory); - - const content = await readFile( - join(temporaryDirectory, ".taskless", "sgconfig.yml"), - "utf8" - ); - expect(content).toContain("ruleDirs:"); - expect(content).toContain("- sg/rules"); - expect(content).toContain("testConfigs:"); - expect(content).toContain("testDir: sg/rule-tests"); - }); - - it("accepts the legacy layout for an unmigrated rule set", async () => { - await generateSgConfig(temporaryDirectory, { - rulesDirectory: "rules", - testDirectory: "rule-tests", - }); - - const content = await readFile( - join(temporaryDirectory, ".taskless", "sgconfig.yml"), - "utf8" - ); - expect(content).toContain("- rules"); - expect(content).toContain("testDir: rule-tests"); - }); - - it("creates .taskless/.gitignore with required entries", async () => { - await generateSgConfig(temporaryDirectory); - - const gitignore = await readFile( - join(temporaryDirectory, ".taskless", ".gitignore"), - "utf8" - ); - expect(gitignore).toContain(".env.local.json"); - expect(gitignore).toContain("sgconfig.yml"); - }); - - it("is idempotent — running twice does not duplicate gitignore entries", async () => { - await generateSgConfig(temporaryDirectory); - await generateSgConfig(temporaryDirectory); - - const gitignore = await readFile( - join(temporaryDirectory, ".taskless", ".gitignore"), - "utf8" - ); - const envCount = gitignore - .split("\n") - .filter((line) => line.trim() === ".env.local.json").length; - expect(envCount).toBe(1); - }); -}); - -describe("addToGitignore", () => { - let temporaryDirectory: string; - - beforeEach(async () => { - temporaryDirectory = await mkdtemp(join(tmpdir(), "taskless-gitignore-")); - }); - - afterEach(async () => { - await rm(temporaryDirectory, { recursive: true, force: true }); - }); - - it("creates .taskless/.gitignore when .taskless/ does not exist", async () => { - await addToGitignore(temporaryDirectory, [ - ".env.local.json", - "sgconfig.yml", - ]); - - const content = await readFile( - join(temporaryDirectory, ".taskless", ".gitignore"), - "utf8" - ); - expect(content).toContain(".env.local.json"); - expect(content).toContain("sgconfig.yml"); - }); - - it("preserves existing entries and appends missing ones", async () => { - await mkdir(join(temporaryDirectory, ".taskless"), { recursive: true }); - await writeFile( - join(temporaryDirectory, ".taskless", ".gitignore"), - "custom-file.txt\n.env.local.json\n", - "utf8" - ); - - await addToGitignore(temporaryDirectory, [ - ".env.local.json", - "sgconfig.yml", - ]); - - const content = await readFile( - join(temporaryDirectory, ".taskless", ".gitignore"), - "utf8" - ); - expect(content).toContain("custom-file.txt"); - expect(content).toContain(".env.local.json"); - expect(content).toContain("sgconfig.yml"); - // .env.local.json should not be duplicated - const envCount = content - .split("\n") - .filter((line) => line.trim() === ".env.local.json").length; - expect(envCount).toBe(1); - }); -}); diff --git a/packages/cli/test/vale-orchestration.test.ts b/packages/cli/test/vale-orchestration.test.ts index e1a21f4..0b6dd24 100644 --- a/packages/cli/test/vale-orchestration.test.ts +++ b/packages/cli/test/vale-orchestration.test.ts @@ -77,7 +77,6 @@ function makeMixedProject(options?: { } /** The committed config `makeMixedProject` writes, as `check` would resolve it. */ -const sgConfigPaths = [".taskless/sg/sgconfig.yml"]; describe("hasValeRules", () => { it("is false for a scaffolded-but-empty rules directory", async () => { @@ -121,7 +120,7 @@ describe("exit code carried on the dispatch result", () => { const dispatched = await runEngines({ cwd, paths: ["app.js"], - astGrepConfigPaths: sgConfigPaths, + astGrepConfigPath: undefined, runtimeRules: [], }); expect(dispatched.results.length).toBeGreaterThan(0); @@ -133,7 +132,7 @@ describe("exit code carried on the dispatch result", () => { const dispatched = await runEngines({ cwd, paths: ["app.js"], - astGrepConfigPaths: sgConfigPaths, + astGrepConfigPath: undefined, runtimeRules: [], }); expect( @@ -147,7 +146,7 @@ describe("exit code carried on the dispatch result", () => { const dispatched = await runEngines({ cwd, paths: ["doc.md"], // the sg rule is javascript-only, so nothing matches - astGrepConfigPaths: sgConfigPaths, + astGrepConfigPath: undefined, runtimeRules: [], }); expect(dispatched.results).toEqual([]); @@ -162,7 +161,7 @@ withVale("runEngines over a mixed corpus", () => { const dispatched = await runEngines({ cwd, paths: ["app.js", "doc.md"], - astGrepConfigPaths: sgConfigPaths, + astGrepConfigPath: undefined, runtimeRules: [], }); @@ -179,7 +178,7 @@ withVale("runEngines over a mixed corpus", () => { const dispatched = await runEngines({ cwd, paths: ["app.js", "doc.md"], - astGrepConfigPaths: sgConfigPaths, + astGrepConfigPath: undefined, runtimeRules: [], }); expect(dispatched.results.every((result) => result.source !== "vale")).toBe( @@ -203,7 +202,7 @@ describe("runEngines when Vale is unavailable", () => { const dispatched = await runEngines({ cwd, paths: ["app.js", "doc.md"], - astGrepConfigPaths: sgConfigPaths, + astGrepConfigPath: undefined, runtimeRules: [], }); @@ -255,7 +254,7 @@ describe("runEngines when Vale is unavailable", () => { const dispatched = await runEngines({ cwd, paths: ["doc.md"], - astGrepConfigPaths: sgConfigPaths, + astGrepConfigPath: undefined, runtimeRules: [], }); @@ -282,7 +281,7 @@ describe("runEngines when Vale is unavailable", () => { const dispatched = await runEngines({ cwd, paths: ["app.js", "doc.md"], - astGrepConfigPaths: sgConfigPaths, + astGrepConfigPath: undefined, runtimeRules: [], }); From e1a5fd4fb5f27221c0ec524ecbe9558dad3dc0f6 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Thu, 13 Aug 2026 21:12:26 -0700 Subject: [PATCH 06/16] fix(cli): retarget Vale check names when 0005 splits the config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Caught by migrating a real 0004 project and running check: ast-grep kept reporting, Vale went silent. A Vale check is named