diff --git a/openspec/changes/add-vale-rule-engine/.openspec.yaml b/openspec/changes/archive/2026-08-11-add-vale-rule-engine/.openspec.yaml similarity index 100% rename from openspec/changes/add-vale-rule-engine/.openspec.yaml rename to openspec/changes/archive/2026-08-11-add-vale-rule-engine/.openspec.yaml diff --git a/openspec/changes/add-vale-rule-engine/design.md b/openspec/changes/archive/2026-08-11-add-vale-rule-engine/design.md similarity index 100% rename from openspec/changes/add-vale-rule-engine/design.md rename to openspec/changes/archive/2026-08-11-add-vale-rule-engine/design.md diff --git a/openspec/changes/add-vale-rule-engine/proposal.md b/openspec/changes/archive/2026-08-11-add-vale-rule-engine/proposal.md similarity index 100% rename from openspec/changes/add-vale-rule-engine/proposal.md rename to openspec/changes/archive/2026-08-11-add-vale-rule-engine/proposal.md diff --git a/openspec/changes/add-vale-rule-engine/specs/cli-check/spec.md b/openspec/changes/archive/2026-08-11-add-vale-rule-engine/specs/cli-check/spec.md similarity index 100% rename from openspec/changes/add-vale-rule-engine/specs/cli-check/spec.md rename to openspec/changes/archive/2026-08-11-add-vale-rule-engine/specs/cli-check/spec.md diff --git a/openspec/changes/add-vale-rule-engine/specs/cli-help/spec.md b/openspec/changes/archive/2026-08-11-add-vale-rule-engine/specs/cli-help/spec.md similarity index 100% rename from openspec/changes/add-vale-rule-engine/specs/cli-help/spec.md rename to openspec/changes/archive/2026-08-11-add-vale-rule-engine/specs/cli-help/spec.md diff --git a/openspec/changes/add-vale-rule-engine/specs/cli-rule-format/spec.md b/openspec/changes/archive/2026-08-11-add-vale-rule-engine/specs/cli-rule-format/spec.md similarity index 100% rename from openspec/changes/add-vale-rule-engine/specs/cli-rule-format/spec.md rename to openspec/changes/archive/2026-08-11-add-vale-rule-engine/specs/cli-rule-format/spec.md diff --git a/openspec/changes/add-vale-rule-engine/specs/cli-rule-routing/spec.md b/openspec/changes/archive/2026-08-11-add-vale-rule-engine/specs/cli-rule-routing/spec.md similarity index 80% rename from openspec/changes/add-vale-rule-engine/specs/cli-rule-routing/spec.md rename to openspec/changes/archive/2026-08-11-add-vale-rule-engine/specs/cli-rule-routing/spec.md index d16df5bb..eacbdf15 100644 --- a/openspec/changes/add-vale-rule-engine/specs/cli-rule-routing/spec.md +++ b/openspec/changes/archive/2026-08-11-add-vale-rule-engine/specs/cli-rule-routing/spec.md @@ -52,14 +52,14 @@ Where code or diff context is available, the engine-selection topic SHALL weigh - **AND WHEN** the equivalent rule requires normalizing a captured value to match a declaration elsewhere - **THEN** it selects `runtime`, despite an identically phrased request -### Requirement: Ambiguity defaults to an engine known to be available +### Requirement: Ambiguity resolves to an engine known to be available -When no engine is clearly indicated, the engine-selection topic SHALL default to `sg` and record why the call was close. The topic SHALL state this as a property — the default names an engine known to be available — rather than as a bare fact about `sg`, so it stays correct wherever an engine can be unavailable or a route withheld. +When no engine is clearly indicated, the engine-selection topic SHALL direct the reader to choose an engine whose availability can be asserted in the situation at hand, and to give that availability as the reason for the call. The topic SHALL NOT name a fixed fallback engine. Both `sg` and `vale` ship as platform binaries, so either can be the missing one on an unsupported architecture or where an install was blocked; server-side the constraint is different again, `sg` being the only ungated route. A named default is wrong in whichever of those situations it failed to anticipate, which is why the requirement is stated as a property rather than as a fact about any one engine. -#### Scenario: Ambiguous request defaults to sg +#### Scenario: Ambiguous request resolves to an assertably available engine - **WHEN** the available context does not disambiguate which engine can enforce a rule -- **THEN** the topic selects `sg` and states the reasoning that made the call close +- **THEN** the topic selects an engine whose availability it can assert, and states that availability as the reasoning that made the call close #### Scenario: The default is never an unavailable engine diff --git a/openspec/changes/add-vale-rule-engine/specs/cli-vale-rule-engine/spec.md b/openspec/changes/archive/2026-08-11-add-vale-rule-engine/specs/cli-vale-rule-engine/spec.md similarity index 89% rename from openspec/changes/add-vale-rule-engine/specs/cli-vale-rule-engine/spec.md rename to openspec/changes/archive/2026-08-11-add-vale-rule-engine/specs/cli-vale-rule-engine/spec.md index 89a213be..896fff0a 100644 --- a/openspec/changes/add-vale-rule-engine/specs/cli-vale-rule-engine/spec.md +++ b/openspec/changes/archive/2026-08-11-add-vale-rule-engine/specs/cli-vale-rule-engine/spec.md @@ -85,6 +85,8 @@ When the `vale` binary cannot be found or invoked, the system SHALL report that The system SHALL verify a Vale rule from a `.taskless/vale/rule-tests//` subdirectory containing `pass/` and `fail/` fixture documents. Because verification is one-time (not per-check), the system SHALL **generate** an ephemeral `.vale.ini` at verify time (StylesPath plus only that rule enabled) rather than requiring a committed one — the subdirectory holds fixtures only. Verification SHALL assert that every `fail/` fixture produces at least one finding for the rule and every `pass/` fixture produces none (mirroring ast-grep's `invalid`/`valid`). +Both buckets SHALL hold at least one document before a rule can be reported as verified. A `fail/` fixture proves the rule fires; a `pass/` fixture proves it does not over-fire; either alone establishes half the claim. A rule populating only one bucket SHALL be reported as unverified rather than passing, and the report SHALL distinguish that half-written state from a rule carrying no fixtures at all — a rule with only `pass/` fixtures would otherwise pass trivially, on an empty set of expected failures, having never demonstrated that it fires. + #### Scenario: Fail fixture triggers, pass fixture does not - **WHEN** verify runs for a rule and generates an isolating `.vale.ini` enabling only that rule @@ -95,6 +97,12 @@ The system SHALL verify a Vale rule from a `.taskless/vale/rule-tests//` s - **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 + ### Requirement: Taskless breadcrumbs use a namespaced ignored key in the Vale config Any Taskless-owned breadcrumb the system records in `.vale.ini` SHALL use a `tskl) = ` key. The system SHALL NOT rely on Vale enforcing these keys; they are read only by Taskless tooling, and Vale's ini parser accepts and ignores them. Each Taskless-owned matcher SHALL carry a `tskl) rule = ` key naming its owning rule, so tooling can locate and update the right rule's matchers even when its scoping is split across multiple (possibly duplicate) matchers. diff --git a/openspec/changes/add-vale-rule-engine/tasks.md b/openspec/changes/archive/2026-08-11-add-vale-rule-engine/tasks.md similarity index 83% rename from openspec/changes/add-vale-rule-engine/tasks.md rename to openspec/changes/archive/2026-08-11-add-vale-rule-engine/tasks.md index ba369682..8bd6d88c 100644 --- a/openspec/changes/add-vale-rule-engine/tasks.md +++ b/openspec/changes/archive/2026-08-11-add-vale-rule-engine/tasks.md @@ -19,17 +19,17 @@ ## 3. Engine-selection knowledge topic -- [ ] 3.1 Author `packages/cli/src/help/.txt` covering: the three engine definitions (`sg` in-file syntax tree incl. relational correlation, `vale` prose/markup, `runtime` cross-file/graph/metadata/normalization), the reason-before-answer procedure, and a worked example table. The original seed (`tmp/SEED-engine-selection-prose.md`) no longer exists — rebuild the examples rather than treating the file as a prerequisite; the content requirements are stated here and in 3.2–3.4, which is the authority -- [ ] 3.2 State the ambiguity default as a property — the default names an engine known to be available — and note that `@ast-grep/cli` ships as a dependency while the Vale binary is external, so `sg` satisfies it locally -- [ ] 3.3 Carry the three boundary cases: prose-about-code vs structure, Vale is per-document (cross-document prose consistency is `runtime`), and `sg`/`vale` are both static-tier so trust tier is a separate axis -- [ ] 3.4 Keep the topic scoped to engine choice — no authoring-destination guidance, no tool-calling mechanics from the seed's source prompt -- [ ] 3.5 Register the topic in the help index and add `route`/`static` cross-references to it -- [ ] 3.6 Tests: the topic resolves via `taskless help` and appears in the index; the topic file matches the established recipe header/format convention -- [ ] 3.7 Add the topic to `TOPICS` in `packages/cli/src/prompts/index.ts`, in the **same unit as the topic file**. This is not optional bookkeeping: `TOPICS ∪ INTERNAL_TOPICS` must account for every canonical recipe on disk, and `test/prompts.test.ts` asserts that in both directions — so a recipe file added without a classification turns the suite red. `TOPICS` rather than `INTERNAL_TOPICS` per D7: the platform generator is the intended consumer, and exporting the entry is what lets it render the same text `taskless help` serves instead of keeping its own copy +- [x] 3.1 Author `packages/cli/src/help/.txt` covering: the three engine definitions (`sg` in-file syntax tree incl. relational correlation, `vale` prose/markup, `runtime` cross-file/graph/metadata/normalization), the reason-before-answer procedure, and a worked example table. The original seed (`tmp/SEED-engine-selection-prose.md`) no longer exists — rebuild the examples rather than treating the file as a prerequisite; the content requirements are stated here and in 3.2–3.4, which is the authority +- [x] 3.2 State the ambiguity default as a property — the default names an engine known to be available. Written per D7 rather than this line's parenthetical, which is stale: `add-vale-binary-packages` ships the Vale binary as an `optionalDependency` too, so both `sg` and `vale` are normally present locally. The property still binds on an unsupported architecture or a blocked install, and server-side `sg` remains the only ungated route +- [x] 3.3 Carry the three boundary cases: prose-about-code vs structure, Vale is per-document (cross-document prose consistency is `runtime`), and `sg`/`vale` are both static-tier so trust tier is a separate axis +- [x] 3.4 Keep the topic scoped to engine choice — no authoring-destination guidance, no tool-calling mechanics from the seed's source prompt +- [x] 3.5 Register the topic in the help index and add `route`/`static` cross-references to it +- [x] 3.6 Tests: the topic resolves via `taskless help` and appears in the index; the topic file matches the established recipe header/format convention +- [x] 3.7 Add the topic to `TOPICS` in `packages/cli/src/prompts/index.ts`, in the **same unit as the topic file**. This is not optional bookkeeping: `TOPICS ∪ INTERNAL_TOPICS` must account for every canonical recipe on disk, and `test/prompts.test.ts` asserts that in both directions — so a recipe file added without a classification turns the suite red. `TOPICS` rather than `INTERNAL_TOPICS` per D7: the platform generator is the intended consumer, and exporting the entry is what lets it render the same text `taskless help` serves instead of keeping its own copy > Export via `@taskless/cli/prompts` **is** part of this change (D7). `export-knowledge-prompts` landed first, so the one-line `TOPICS` entry falls here — see task 3.7. ## 4. Quality gates -- [ ] 4.1 `pnpm --filter @taskless/cli typecheck && lint && test` clean -- [ ] 4.2 With the Vale binary absent, confirm ast-grep and runtime results still return and only Vale reports unavailable +- [x] 4.1 `pnpm --filter @taskless/cli typecheck && lint && test` clean +- [x] 4.2 With the Vale binary absent, confirm ast-grep and runtime results still return and only Vale reports unavailable diff --git a/openspec/specs/cli-check/spec.md b/openspec/specs/cli-check/spec.md index 5f510b96..a76c0073 100644 --- a/openspec/specs/cli-check/spec.md +++ b/openspec/specs/cli-check/spec.md @@ -381,3 +381,17 @@ SHALL be the only way to execute runtime rules on an unverified path. - **WHEN** `--dangerously-run-scripts` and `--json` are both set - **THEN** stdout SHALL contain only the existing `{ success, results }` JSON shape - **AND** the unverified-execution warning SHALL NOT appear in stdout + +### Requirement: Check runs engines concurrently and merges their results + +`taskless check` SHALL run its per-engine executors concurrently and merge their `CheckResult`s into a single result set. A missing or unavailable engine SHALL NOT abort the others; its absence SHALL be reported while the remaining engines still produce results. + +#### Scenario: ast-grep and Vale run concurrently and merge + +- **WHEN** `.taskless/sg/` and `.taskless/vale/` both contain rules +- **THEN** the CLI runs both engines concurrently and returns one merged result set whose findings count toward the exit code identically + +#### Scenario: One engine unavailable, others proceed + +- **WHEN** the `vale` binary is unavailable but `.taskless/sg/` has rules +- **THEN** the CLI reports the Vale engine as unavailable and still returns ast-grep results diff --git a/openspec/specs/cli-help/spec.md b/openspec/specs/cli-help/spec.md index f9fb6190..ff82a10b 100644 --- a/openspec/specs/cli-help/spec.md +++ b/openspec/specs/cli-help/spec.md @@ -186,6 +186,29 @@ consistent with the existing `help_` telemetry convention. - **THEN** the help command SHALL capture the corresponding `help_` intent event with the topic name +### Requirement: The engine-selection topic is registered in the help system + +The help system SHALL register the engine-selection recipe as an embedded help topic, retrievable via `taskless help ` and listed in the help index, consistent with the existing topic embedding and format requirements. + +#### Scenario: Engine-selection topic resolves + +- **WHEN** `taskless help` is run for the engine-selection topic +- **THEN** the recipe text SHALL be returned and an unknown-topic error SHALL NOT be raised + +#### Scenario: Engine-selection topic appears in the index + +- **WHEN** `taskless help` is run with no arguments +- **THEN** the topic index SHALL include the engine-selection topic so an agent can discover it + +### Requirement: Routing recipes reference engine selection + +The `route` and `static` recipes SHALL reference the engine-selection topic so an agent following the local authoring flow applies the same engine test the service applies, rather than assuming ast-grep. + +#### Scenario: Local flow reaches engine selection + +- **WHEN** an agent follows `route` to a destination that authors a Taskless rule +- **THEN** the recipe directs it to the engine-selection topic before the rule is authored + ## Goal diff --git a/openspec/specs/cli-rule-format/spec.md b/openspec/specs/cli-rule-format/spec.md index 364a6007..9eca5e46 100644 --- a/openspec/specs/cli-rule-format/spec.md +++ b/openspec/specs/cli-rule-format/spec.md @@ -118,3 +118,12 @@ When `taskless.json`'s `version` exceeds the highest migration the installed CLI - **WHEN** the same condition holds and `--allow-version-mismatches` is set - **THEN** the CLI proceeds without applying migrations + +### Requirement: Vale styles live under the rules StyleName + +The system SHALL place Vale styles under `.taskless/vale/rules/` so that `rules` is Vale's StyleName, with `.vale.ini` configured `StylesPath = .` and `BasedOnStyles = rules`. The Vale check identifier `rules.` SHALL be normalized to `ruleId = ` in results. + +#### Scenario: Style resolution and identity + +- **WHEN** a Vale style exists at `.taskless/vale/rules/no-simply.yml` +- **THEN** Vale loads it as `rules.no-simply`, and the CLI reports its findings with `ruleId` `no-simply` diff --git a/openspec/specs/cli-rule-routing/spec.md b/openspec/specs/cli-rule-routing/spec.md index 27cca600..cebcbd9f 100644 --- a/openspec/specs/cli-rule-routing/spec.md +++ b/openspec/specs/cli-rule-routing/spec.md @@ -207,3 +207,69 @@ SHALL NOT itself decide static versus runtime. - **WHEN** the service returns a generated rule via `remote` - **THEN** the written files and paths SHALL match the shape produced by the local `static` path + +### Requirement: An engine-selection topic states which engine can enforce a rule + +The CLI SHALL provide a knowledge topic that decides, for a requested rule, **which engine can enforce it** — `sg`, `vale`, or `runtime` — valued as the engine's on-disk directory name. The topic SHALL define each engine by the information a rule fundamentally needs: + +- **`sg`** — expressible as a pattern over a single file's syntax tree, including correlation between constructs within that same file via relational operators. +- **`vale`** — the target is prose or markup content rather than code structure. +- **`runtime`** — needs information no single file's syntax tree contains: cross-file consistency, import or call graph, comparison against a non-code file, file metadata, or values requiring normalization a static pattern cannot express. + +The topic SHALL instruct that the decision follow from what the rule fundamentally needs rather than how the request was phrased, and that the reasoning be stated before the engine is named. + +#### Scenario: Engine named for a single-file structural rule + +- **WHEN** the topic is applied to a request expressible as a pattern over one file's syntax tree +- **THEN** it selects `sg` + +#### Scenario: Engine named for a prose rule + +- **WHEN** the topic is applied to a request targeting prose or markup content +- **THEN** it selects `vale` + +#### Scenario: Engine named for a cross-file rule + +- **WHEN** the topic is applied to a request requiring information beyond a single file's syntax tree +- **THEN** it selects `runtime` + +### Requirement: Engine selection is a separate axis from authoring destination + +The engine-selection topic SHALL decide only which engine enforces a rule, and SHALL NOT decide where the rule is authored — that remains the `route` topic's concern. Locally the two compose, `route` first and engine selection second. + +The topic SHALL NOT describe login, reconciliation, or signing as inputs to the engine choice: `sg` and `vale` are both static-tier, and only `runtime` carries those concerns, so trust tier is a distinct axis from engine selection. + +#### Scenario: Topic stays clear of authoring destination + +- **WHEN** the engine-selection topic is applied +- **THEN** it names an engine and does not select among `existing`, `static`, or `remote` authoring destinations + +#### Scenario: Trust tier is not an engine-selection input + +- **WHEN** the topic distinguishes `sg` from `vale` +- **THEN** it does so on the prose-versus-structure axis, not on any auth, reconcile, or signing property, since both are static-tier + +### Requirement: Available code context outranks the phrasing of the request + +Where code or diff context is available, the engine-selection topic SHALL weigh the concrete syntactic form present in the repository above the wording of the request, since the same request routes differently depending on the form the code actually takes. + +#### Scenario: Concrete form changes the engine + +- **WHEN** a rule is statically correlatable in the form the repository actually contains +- **THEN** the topic selects `sg` +- **AND WHEN** the equivalent rule requires normalizing a captured value to match a declaration elsewhere +- **THEN** it selects `runtime`, despite an identically phrased request + +### Requirement: Ambiguity resolves to an engine known to be available + +When no engine is clearly indicated, the engine-selection topic SHALL direct the reader to choose an engine whose availability can be asserted in the situation at hand, and to give that availability as the reason for the call. The topic SHALL NOT name a fixed fallback engine. Both `sg` and `vale` ship as platform binaries, so either can be the missing one on an unsupported architecture or where an install was blocked; server-side the constraint is different again, `sg` being the only ungated route. A named default is wrong in whichever of those situations it failed to anticipate, which is why the requirement is stated as a property rather than as a fact about any one engine. + +#### Scenario: Ambiguous request resolves to an assertably available engine + +- **WHEN** the available context does not disambiguate which engine can enforce a rule +- **THEN** the topic selects an engine whose availability it can assert, and states that availability as the reasoning that made the call close + +#### Scenario: The default is never an unavailable engine + +- **WHEN** an engine is unavailable in the current environment, such as the Vale binary being absent +- **THEN** the ambiguity default SHALL NOT name it diff --git a/openspec/specs/cli-vale-rule-engine/spec.md b/openspec/specs/cli-vale-rule-engine/spec.md new file mode 100644 index 00000000..f87026c1 --- /dev/null +++ b/openspec/specs/cli-vale-rule-engine/spec.md @@ -0,0 +1,124 @@ +# cli-vale-rule-engine Specification + +## Purpose + +TBD - created by archiving change add-vale-rule-engine. Update Purpose after archive. + +## Requirements + +### Requirement: Vale runs in the static tier without reconciliation or signing + +The system SHALL treat Vale as a static-tier engine — always run, with no server reconciliation or signature verification. Vale's `script` checks execute in a sandbox that exposes only pure-computation modules (`text`/`math`/`fmt`) with no host access, so a Vale rule is inert data equivalent in trust to a static ast-grep rule. + +#### Scenario: Vale runs when anonymous + +- **WHEN** the CLI runs a check while logged out or anonymous +- **THEN** Vale rules are executed the same as ast-grep static rules, with no reconcile or signing step + +### Requirement: Vale check executes against the committed config over the target paths + +The system SHALL run `vale --config .taskless/vale/.vale.ini --output=JSON --no-exit` over the resolved target paths, reading the committed config and styles as-is. The `.vale.ini` SHALL set `MinAlertLevel = suggestion` so that every finding surfaces to the client for normalization and filtering. + +#### Scenario: Check runs Vale via the committed config + +- **WHEN** the CLI runs a check and `.taskless/vale/` contains rules +- **THEN** it invokes Vale with `--config .taskless/vale/.vale.ini` over the target paths and parses the JSON output + +#### Scenario: No Vale rules present + +- **WHEN** `.taskless/vale/rules/` is empty +- **THEN** the CLI does not invoke Vale and produces no Vale findings + +### Requirement: Per-rule scoping is expressed via Vale config matchers + +The system SHALL express a Vale rule's scope through `.vale.ini` **matchers** — `[]` sections. Include is `rules. = YES`, exclude is `rules. = NO`. + +Precedence is **positional**, and the system SHALL order matchers accordingly rather than relying on a disable to win on its own. Measured against Vale 3.17.1: + +- Where two matchers both match a file, the **last** one wins for that rule. +- Where the same key is assigned twice inside one matcher — including across duplicate `[]` sections, which Vale merges — the **first** assignment wins. + +A disable therefore SHALL be declared **after** the enable it narrows. Duplicate `[]` matchers SHALL be treated as merged, and a rule's scope SHALL NOT be expressed as a repeated assignment of the same key within one glob, since the later assignment is discarded. + +#### Scenario: Duplicate matchers merge + +- **WHEN** two `[*.md]` matchers each enable a different rule +- **THEN** both rules run on a matching `.md` file (Vale merges the matchers) + +#### Scenario: Include scopes a rule to a path + +- **WHEN** a rule is enabled only under `[marketing/**]` +- **THEN** the rule produces findings in `marketing/` files and none in `api/` files + +#### Scenario: A later matcher overrides an earlier one + +- **WHEN** a rule is enabled under `[marketing/**]` and then disabled under `[marketing/legacy/**]` +- **THEN** the rule fires in `marketing/` but not in `marketing/legacy/` + +#### Scenario: Declaration order is significant + +- **WHEN** the same two matchers are declared in the opposite order — `[marketing/legacy/**]` disabling first, `[marketing/**]` enabling second +- **THEN** the rule fires in `marketing/legacy/` as well, because the later enable wins; a disable does not take precedence on its own + +### Requirement: Vale findings map to the scanner-agnostic CheckResult + +The system SHALL map each Vale finding to a `CheckResult` with `source` `"vale"` and `ruleId` equal to the Vale check name with its `rules.` prefix stripped. Severity SHALL be normalized `error → error`, `warning → warning`, `suggestion → hint`. The system SHALL map `message` from `Message`, `note` from `Description`/`Link`, `range` from `Line`/`Span`, `matchedText` from `Match`, and `fix` from `Action` only when the action is populated. + +#### Scenario: Finding maps to CheckResult + +- **WHEN** Vale reports a finding `{Check: "rules.no-simply", Severity: "warning", Line: 3, Span: [1,7], Message: "Avoid 'simply'", Match: "simply"}` in `docs/a.md` +- **THEN** the CLI emits a `CheckResult` with `source` `"vale"`, `ruleId` `"no-simply"`, `severity` `"warning"`, `message` `"Avoid 'simply'"`, `file` `"docs/a.md"`, and a `range` derived from line 3 / span 1–7 + +### Requirement: A Vale check is bounded by a subprocess timeout + +The system SHALL bound each Vale invocation with a timeout and, on expiry, terminate the process and report the timeout rather than hanging. + +#### Scenario: Runaway Vale invocation is terminated + +- **WHEN** a Vale invocation exceeds its timeout +- **THEN** the CLI terminates the process and reports a timeout for the Vale engine without hanging the overall check + +### Requirement: A missing Vale binary is reported without failing other engines + +When the `vale` binary cannot be found or invoked, the system SHALL report that the Vale engine is unavailable and continue running other engines, rather than aborting the entire check. + +#### Scenario: Vale binary absent + +- **WHEN** `.taskless/vale/` has rules but the `vale` binary is not installed +- **THEN** the CLI reports the Vale engine as unavailable with an actionable message and still returns ast-grep results + +### Requirement: Vale rules are verified with per-rule fixture subdirectories + +The system SHALL verify a Vale rule from a `.taskless/vale/rule-tests//` subdirectory containing `pass/` and `fail/` fixture documents. Because verification is one-time (not per-check), the system SHALL **generate** an ephemeral `.vale.ini` at verify time (StylesPath plus only that rule enabled) rather than requiring a committed one — the subdirectory holds fixtures only. Verification SHALL assert that every `fail/` fixture produces at least one finding for the rule and every `pass/` fixture produces none (mirroring ast-grep's `invalid`/`valid`). + +Both buckets SHALL hold at least one document before a rule can be reported as verified. A `fail/` fixture proves the rule fires; a `pass/` fixture proves it does not over-fire; either alone establishes half the claim. A rule populating only one bucket SHALL be reported as unverified rather than passing, and the report SHALL distinguish that half-written state from a rule carrying no fixtures at all — a rule with only `pass/` fixtures would otherwise pass trivially, on an empty set of expected failures, having never demonstrated that it fires. + +#### Scenario: Fail fixture triggers, pass fixture does not + +- **WHEN** verify runs for a rule and generates an isolating `.vale.ini` enabling only that rule +- **THEN** verification passes because every `fail/` fixture yields a finding and every `pass/` fixture yields none + +#### 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 + +### Requirement: Taskless breadcrumbs use a namespaced ignored key in the Vale config + +Any Taskless-owned breadcrumb the system records in `.vale.ini` SHALL use a `tskl) = ` key. The system SHALL NOT rely on Vale enforcing these keys; they are read only by Taskless tooling, and Vale's ini parser accepts and ignores them. Each Taskless-owned matcher SHALL carry a `tskl) rule = ` key naming its owning rule, so tooling can locate and update the right rule's matchers even when its scoping is split across multiple (possibly duplicate) matchers. + +#### Scenario: Breadcrumb key is ignored by Vale + +- **WHEN** `.vale.ini` contains a `tskl) rule = no-simply` key +- **THEN** Vale runs normally, ignoring the key, and Taskless tooling can read it back + +#### Scenario: Canonical id locates a rule's matchers + +- **WHEN** a rule's scoping spans several matchers each tagged `tskl) rule = no-simply` +- **THEN** tooling can find every matcher owned by `no-simply` by its `tskl) rule` id rather than by glob diff --git a/packages/cli/src/commands/help.ts b/packages/cli/src/commands/help.ts index 1156f244..d89e4c9c 100644 --- a/packages/cli/src/commands/help.ts +++ b/packages/cli/src/commands/help.ts @@ -18,6 +18,7 @@ const RECIPE_TOPICS: ReadonlyArray<[string, string]> = [ ["existing", "Author a rule in a linter the repo already uses"], ["static", "Author a local ast-grep rule on this machine (no login)"], ["remote", "Generate a rule via the Taskless service (login)"], + ["engine-selection", "Decide which engine enforces a rule (sg/vale/runtime)"], ]; async function unwrap(resolvable: Resolvable): Promise { diff --git a/packages/cli/src/help/engine-selection.txt b/packages/cli/src/help/engine-selection.txt new file mode 100644 index 00000000..15ffed6d --- /dev/null +++ b/packages/cli/src/help/engine-selection.txt @@ -0,0 +1,113 @@ +# Topic: engine-selection (CLI v%(CLI_VERSION)s / topic v1) + +## Goal +Decide WHICH ENGINE enforces a rule: `sg` (ast-grep), `vale`, or +`runtime`. This is a question about the shape of the evidence the rule +needs, not about where the rule gets authored and not about how much +the rule is trusted. + +## Preconditions +- A rule intent stated in words ("no `eval`", "don't say 'simply'", + "every exported symbol must appear in the changelog"). +- No auth required. Engine choice is a property of the rule, not of the + user's session. + +## The three engines + +**`sg` — one file's syntax tree.** +Code structure, decided by parsing a single file: a call, an import, a +JSX attribute, a type annotation. Relational correlation counts as +in-file — "a `useEffect` whose dependency array omits a value used in +its body" is still one file's tree, and `sg` handles it. If the rule +can be settled by looking at exactly one file's structure, it is `sg`. + +**`vale` — prose and markup.** +Natural language in documents and in the prose parts of code: docs, +README, comments, commit-message bodies. Word choice, phrasing, +terminology, capitalization, banned or preferred vocabulary. Vale reads +a document, not a syntax tree. + +**`runtime` — everything that needs more than one file, or needs the +world.** +Cross-file and graph questions ("this exported symbol is unused across +the repo"), metadata not in any file's text (git history, ownership, +build outputs), and normalization a parser cannot express (resolving +aliases, following a config chain). If answering the rule requires +holding two files at once, or looking outside the files, it is +`runtime`. + +## Steps + +1. **State the evidence the rule needs, before naming an engine.** + Write one sentence: "to decide this, you must look at ___." Do this + first, every time. Naming an engine and then justifying it is how + prose-about-code ends up in `sg` and cross-file questions end up in + `vale`. + +2. **Match the evidence to an engine.** + - one file's structure → `sg` + - a document's words → `vale` + - more than one file, or anything outside the files → `runtime` + +3. **Check the boundary cases below.** Most wrong answers are one of + the three. + +4. **If it is still ambiguous, choose an engine you know is + available.** State that as the reason. Both `sg` and `vale` ship as + platform binaries with the CLI, so both are normally present; on an + unsupported architecture, or where an install was blocked, the + fallback may find nothing. Server-side the same conclusion follows + from a different cause: `sg` is the only ungated route. Prefer the + engine whose availability you can actually assert in the situation + you are in, rather than defaulting to a name. + +## Boundary cases + +**Prose about code is still prose.** +"Comments must not say 'obviously'" is `vale` — the evidence is the +words. "Every exported function has a doc comment" is `sg` — the +evidence is whether a node exists above a declaration. Ask what you +would have to read to decide, not what the subject matter is. + +**Vale sees one document at a time.** +Vale has no cross-document view. "This term is spelled consistently +ACROSS the docs directory" is `runtime`, not `vale`, even though it is +entirely about prose. Per-document prose is `vale`; prose consistency +between documents is a graph question. + +**Engine is not trust tier.** +`sg` and `vale` are both static-tier: inert data, always run, no login, +no reconcile, no signing. Only `runtime` involves verification, because +only `runtime` executes code. "Static vs runtime" is a different axis +from "which engine", and conflating them is what makes a prose rule +look like it needs login. + +## Worked examples + +| Rule intent | Evidence needed | Engine | +| -------------------------------------------------------- | ---------------------------------------- | --------- | +| No `eval(...)` anywhere | one file's call expressions | `sg` | +| `useEffect` deps must include what the body reads | one file's tree, correlated within it | `sg` | +| Don't write "simply" or "just" in docs | a document's words | `vale` | +| Comments must not say "obviously" | a document's words (comments are prose) | `vale` | +| Headings use sentence case | a document's markup | `vale` | +| Exported symbols must be used somewhere in the repo | every file, correlated | `runtime` | +| Product name spelled the same across all docs | many documents, compared | `runtime` | +| Files changed in the last release need a changelog entry | git metadata, not file contents | `runtime` | +| Imports must resolve through the tsconfig path aliases | config chain resolution, outside the file| `runtime` | + +## Important Notes + +- This topic decides the engine and nothing else. Where a rule is + authored — in a linter the repo already uses, locally, or via the + service — is `route`, and it is a separate decision made earlier. + Locally the two compose: route first, then engine. +- A rule that seems to need two engines is usually two rules. Split it + and let each half pick its own engine, rather than forcing one engine + to cover evidence it cannot see. + +## See Also + +- `taskless help route` — decide WHERE to author a rule (a different question) +- `taskless help static` — author a local ast-grep rule (no login) +- `taskless help check` — run every engine over the repo diff --git a/packages/cli/src/help/route.txt b/packages/cli/src/help/route.txt index 363abc03..b39ac71d 100644 --- a/packages/cli/src/help/route.txt +++ b/packages/cli/src/help/route.txt @@ -93,3 +93,5 @@ failed and the user confirms. - `taskless help existing` — author in a linter the repo already uses - `taskless help static` — author a local ast-grep rule (no login) - `taskless help remote` — generate via the Taskless service (login) +- `taskless help engine-selection` — decide WHICH engine enforces the rule + (the next decision once the destination is settled) diff --git a/packages/cli/src/help/static.txt b/packages/cli/src/help/static.txt index c4cf9cd6..a67f6b26 100644 --- a/packages/cli/src/help/static.txt +++ b/packages/cli/src/help/static.txt @@ -71,4 +71,6 @@ writes, so `check`, `improve`, and `verify` treat them identically. - `taskless help route` — re-decide the destination - `taskless help remote` — generate via the Taskless service (login) +- `taskless help engine-selection` — confirm `sg` is the right engine for + this rule, rather than `vale` or `runtime` - `taskless help check` — validate the new rule against the codebase diff --git a/packages/cli/src/prompts/index.ts b/packages/cli/src/prompts/index.ts index 4ae5d4f2..f7e10175 100644 --- a/packages/cli/src/prompts/index.ts +++ b/packages/cli/src/prompts/index.ts @@ -27,8 +27,16 @@ import { getRecipe, type RecipeOptions } from "./recipes.js"; * The list starts at what a consumer has actually asked for and grows on * demand. `static` is the canonical on-disk rule shape, the one topic the * generator's decision router can use server-side. + * + * `engine-selection` is exported for the same consumer and the same reason + * (D7 of `add-vale-rule-engine`). Choosing an engine is a decision the service + * already makes — today as a binary `static | runtime` classifier that predates + * Vale, and is therefore now wrong. Exporting the topic is what lets the + * generator render the same guidance `taskless help` serves instead of keeping + * a second copy free to drift, which is the divergence this module exists to + * prevent. */ -export const TOPICS = ["static"] as const; +export const TOPICS = ["static", "engine-selection"] as const; /** * Recipes deliberately withheld from the export, recorded so they stay visible diff --git a/packages/cli/test/help-extensions.test.ts b/packages/cli/test/help-extensions.test.ts index 10688669..eca96b91 100644 --- a/packages/cli/test/help-extensions.test.ts +++ b/packages/cli/test/help-extensions.test.ts @@ -62,7 +62,13 @@ describe("taskless help (no args)", () => { it("lists the routing recipe topics under Authoring recipes", async () => { const result = await runCli(["help", "-d", cwd]); expect(result.stdout).toContain("Authoring recipes:"); - for (const topic of ["route", "existing", "static", "remote"]) { + for (const topic of [ + "route", + "existing", + "static", + "remote", + "engine-selection", + ]) { expect(result.stdout).toContain(topic); } }); @@ -79,7 +85,7 @@ describe("taskless help ", () => { await rm(cwd, { recursive: true, force: true }); }); - it.each(["route", "existing", "static", "remote"])( + it.each(["route", "existing", "static", "remote", "engine-selection"])( "resolves the %s recipe without an unknown-topic error", async (topic) => { const result = await runCli(["help", topic, "-d", cwd]); @@ -211,3 +217,60 @@ describe("bare taskless (non-TTY) routes to help index", () => { expect(result.stdout).toContain("Topics:"); }); }); + +describe("taskless help engine-selection", () => { + let cwd: string; + + beforeEach(async () => { + cwd = await mkdtemp(join(tmpdir(), "taskless-help-engine-")); + }); + + afterEach(async () => { + await rm(cwd, { recursive: true, force: true }); + }); + + it("names all three engines and puts evidence before the answer", async () => { + const result = await runCli(["help", "engine-selection", "-d", cwd]); + expect(result.exitCode).toBe(0); + for (const engine of ["`sg`", "`vale`", "`runtime`"]) { + expect(result.stdout).toContain(engine); + } + expect(result.stdout).toContain("before naming an engine"); + }); + + it("carries the three boundary cases", async () => { + const result = await runCli(["help", "engine-selection", "-d", cwd]); + // Each is a wrong answer someone actually reaches for. + expect(result.stdout).toContain("Prose about code is still prose"); + expect(result.stdout).toContain("one document at a time"); + expect(result.stdout).toContain("Engine is not trust tier"); + }); + + it("states the ambiguity default as a property, not as `sg`", async () => { + const result = await runCli(["help", "engine-selection", "-d", cwd]); + // D7: "choose an engine you know is available" stays correct on an + // unsupported arch and server-side alike; naming `sg` outright would be + // false on the host where `sg` is the missing one. + expect(result.stdout).toContain("choose an engine you know is"); + expect(result.stdout).toContain("available"); + }); + + it("stays out of the authoring-destination decision", async () => { + const result = await runCli(["help", "engine-selection", "-d", cwd]); + // Scope guard (3.4): it may point at `route`, never re-decide it. + expect(result.stdout).toContain("is `route`, and it is a separate"); + }); + + it("follows the recipe header and section convention", async () => { + const result = await runCli(["help", "engine-selection", "-d", cwd]); + expect(result.stdout).toContain("# Topic: engine-selection"); + for (const section of [ + "## Goal", + "## Preconditions", + "## Steps", + "## See Also", + ]) { + expect(result.stdout).toContain(section); + } + }); +}); diff --git a/packages/cli/test/help-routing-telemetry.test.ts b/packages/cli/test/help-routing-telemetry.test.ts index ece49ffe..c3925b4b 100644 --- a/packages/cli/test/help-routing-telemetry.test.ts +++ b/packages/cli/test/help-routing-telemetry.test.ts @@ -37,7 +37,7 @@ describe("help routing topics emit cli_help intent telemetry", () => { logSpy.mockRestore(); }); - it.each(["route", "existing", "static", "remote"])( + it.each(["route", "existing", "static", "remote", "engine-selection"])( "captures cli_help for %s", async (topic) => { const command = createHelpCommand({}) as unknown as RunnableCommand; diff --git a/packages/cli/test/prompts.test.ts b/packages/cli/test/prompts.test.ts index 4ed60093..8c35a769 100644 --- a/packages/cli/test/prompts.test.ts +++ b/packages/cli/test/prompts.test.ts @@ -1,6 +1,6 @@ import { execFile } from "node:child_process"; import { readFile, readdir } from "node:fs/promises"; -import { dirname, resolve } from "node:path"; +import { resolve } from "node:path"; import { pathToFileURL } from "node:url"; import { promisify } from "node:util"; import { describe, expect, it } from "vitest"; @@ -235,7 +235,16 @@ describe("built prompts entry", () => { }); }); -/** Module specifiers a built chunk imports, static and dynamic. */ +/** + * Module specifiers a **source** file imports, static and dynamic. + * + * Scoped to hand-written TypeScript on purpose. A regex is sound over a module + * someone wrote and unsound over one a bundler generated: a built chunk embeds + * every recipe as a string literal, so prose containing `from "…"` reads as an + * import. The built graph is checked in the build instead — see + * `assert-prompts-graph` in `vite.config.ts`, which reads rollup's resolved + * `imports`/`dynamicImports` rather than guessing at them from text. + */ function importSpecifiers(source: string): string[] { const specifiers = new Set(); for (const match of source.matchAll(/\bfrom\s*["']([^"']+)["']/g)) { @@ -277,31 +286,10 @@ describe("prompts entry carries no CLI runtime", () => { } }); - it("never reaches the CLI entry or a host capability once built", async () => { - const seen = new Set(); - const queue = [distributionPromptsPath]; - - while (queue.length > 0) { - const file = queue.pop()!; - if (seen.has(file)) continue; - seen.add(file); - const source = await readFile(file, "utf8"); - for (const specifier of importSpecifiers(source)) { - if (specifier.startsWith(".")) { - queue.push(resolve(dirname(file), specifier)); - continue; - } - // A bare specifier here would be an unbundled runtime dependency; the - // lib build bundles everything except node builtins, so any survivor is - // a builtin the prompts graph has no business touching. - expect(specifier, `dist/prompts.js graph imports ${specifier}`).toBe( - "" - ); - } - } - - expect( - [...seen].map((file) => file.replace(`${distributionDirectory}/`, "")) - ).not.toContain("index.js"); - }); + // The built graph is NOT asserted here. `assert-prompts-graph` in + // `vite.config.ts` fails the build if the prompts entry reaches the CLI entry + // or imports anything external, so a bundle that leaks cannot be emitted in + // the first place — there is no artifact left for a test to inspect. This + // file keeps the source-level constraint, which is about what we wrote rather + // than about what the build produced. }); diff --git a/packages/cli/vite.config.ts b/packages/cli/vite.config.ts index 1f9a8cae..decdb460 100644 --- a/packages/cli/vite.config.ts +++ b/packages/cli/vite.config.ts @@ -168,13 +168,96 @@ function shebang(): Plugin { }; } +/** The entry chunk this build emitted under `name`, if it emitted one. */ +function findEntryChunk( + bundle: Rollup.OutputBundle, + name: string +): Rollup.OutputChunk | undefined { + return Object.values(bundle).find( + (chunk): chunk is Rollup.OutputChunk => + chunk.type === "chunk" && chunk.isEntry && chunk.name === name + ); +} + +/** + * Refuse to emit a prompts entry that drags the CLI runtime along with it. + * + * `@taskless/cli/prompts` is a library surface: an agent imports it to render + * recipe text, and it must not pull in the command layer or reach a host + * capability to do that. The graph is allowed to touch embedded text and the + * pure helpers it renders with, and nothing else. + * + * Two rules, both checked over the entry's transitive chunk graph: + * + * - **No external imports at all.** Everything but node builtins is bundled + * (see `rollupOptions.external`), so a bare specifier surviving here is a + * builtin — `node:fs`, `node:child_process` — and the render path has no + * business with any of them. + * - **Never reach the bin entry.** That chunk is the CLI itself. + * + * ENFORCED IN THE BUILD, DELIBERATELY, rather than asserted by a test over the + * artifact. A build that refuses to emit a leaking bundle makes the bad artifact + * unproducible; a test that inspects one afterwards only notices. It is also + * the difference between reading rollup's own resolved graph and reconstructing + * it: the test this replaces regex-scanned built JavaScript for `from "…"`, and + * a chunk embeds every recipe as a string literal, so the engine-selection + * recipe's `a different axis from "which engine"` was reported as an import. + * `imports`/`dynamicImports` below are the real thing and cannot be spoofed by + * prose. + */ +function assertPromptsGraph(): Plugin { + return { + name: "assert-prompts-graph", + generateBundle(_options, bundle) { + const entry = findEntryChunk(bundle, PROMPTS_ENTRY); + // Not an error: `build:self`/`build:dev` and any future single-entry + // build legitimately emit no prompts entry. Nothing to check, not a + // failure to check it. + if (entry === undefined) return; + + const binFile = findEntryChunk(bundle, BIN_ENTRY)?.fileName; + + const seen = new Set(); + const queue = [entry.fileName]; + while (queue.length > 0) { + // A chunk's file name, or — for anything rollup left external — the + // bare specifier itself, which is why the bundle lookup below can miss. + const imported = queue.pop()!; + if (seen.has(imported)) continue; + seen.add(imported); + + const chunk = bundle[imported]; + if (chunk === undefined || chunk.type !== "chunk") { + // Resolved to something outside the bundle: an external module. + this.error( + `prompts entry graph imports ${imported}; the render path must not ` + + `reach a host capability` + ); + } + if (imported === binFile) { + this.error( + `prompts entry graph reaches the CLI entry (${imported}); ` + + `importing @taskless/cli/prompts would load the command layer` + ); + } + queue.push(...chunk.imports, ...chunk.dynamicImports); + } + }, + }; +} + export default defineConfig({ define: { __VERSION__: JSON.stringify(pkg.version), __TASKLESS_CLI__: JSON.stringify(resolveCliInvocation()), __TASKLESS_CLI_NOTICE__: JSON.stringify(resolveCliNotice()), }, - plugins: [tsconfigPaths(), assertSkillVersions(), shebang()], + plugins: [ + tsconfigPaths(), + assertSkillVersions(), + shebang(), + assertPromptsGraph(), + ], build: { outDir: resolveOutDir(), lib: {