diff --git a/.changeset/reference-fixture-cases.md b/.changeset/reference-fixture-cases.md new file mode 100644 index 00000000..5cdef2b6 --- /dev/null +++ b/.changeset/reference-fixture-cases.md @@ -0,0 +1,13 @@ +--- +"@taskless/cli": minor +--- + +`@taskless/cli/reference.json` now states how its fixtures group into cases, and names the tree its paths are relative to. `version` is `2`. + +`tests` was a flat `{ path, content }[]`, so recovering which files belong to which case meant knowing that a `runtime` case is a directory, a `vale` case is a document, and an `sg` rule's cases are `valid:`/`invalid:` keys inside one ast-grep test file. That is a fact about this repository, and every consumer transcribed it. It cost the Cloud eval team a rule that failed the fixtures shipped beside it: their request format carried one anonymous blob per case, so the two-file `runtime` case could not be expressed and the rule was graded against half of itself. + +`tests` is now an object carrying a `grouping` discriminant, the same file list, and — for the groupings the CLI itself defines — the cases, each naming the path a runner is handed and the files it holds. `sg` publishes `grouping: "ast-grep-test"` and no cases, because that grouping lives inside ast-grep's own documented schema rather than ours. + +A new top-level `layout` block publishes the rule tree — `.taskless/rules/{engine}/{id}`, which file is the rule for each engine, where its config and captures go — and each entry carries its own resolved `directory` and `ruleFile`. Without it the corpus published paths relative to a root it never named, so a consumer materializing a rule had to assume where the CLI looks. Every value is generated from the table the CLI dispatches on, so it cannot describe a layout the CLI does not implement. + +`@taskless/cli/layout` gains `TASKLESS_DIRECTORY` and each engine's `fixtureLayout`. diff --git a/openspec/changes/reference-fixture-cases/design.md b/openspec/changes/reference-fixture-cases/design.md new file mode 100644 index 00000000..e66f56cd --- /dev/null +++ b/openspec/changes/reference-fixture-cases/design.md @@ -0,0 +1,176 @@ +# Design + +## The shape + +`ReferenceRule.tests` goes from `Array<{ path, content }>` to: + +```jsonc +// runtime — a case is a directory the check is given as its root +"tests": { + "grouping": "case-directories", + "files": [ + { "path": ".tests/pass/declared/src/config.ts", "content": "…" }, + { "path": ".tests/pass/declared/.env", "content": "…" }, + { "path": ".tests/fail/undeclared/src/config.ts", "content": "…" }, + { "path": ".tests/fail/undeclared/.env", "content": "…" } + ], + "cases": [ + { "bucket": "pass", "name": "declared", "path": ".tests/pass/declared", + "files": [".tests/pass/declared/src/config.ts", ".tests/pass/declared/.env"] }, + { "bucket": "fail", "name": "undeclared", "path": ".tests/fail/undeclared", + "files": [".tests/fail/undeclared/src/config.ts", ".tests/fail/undeclared/.env"] } + ] +} + +// vale — a case is one document +"tests": { + "grouping": "case-documents", + "files": [ { "path": ".tests/pass/README.md", "content": "…" }, … ], + "cases": [ + { "bucket": "pass", "name": "README.md", "path": ".tests/pass/README.md", + "files": [".tests/pass/README.md"] }, … + ] +} + +// sg — grouping is ast-grep's, inside the file +"tests": { + "grouping": "ast-grep-test", + "files": [ { "path": ".tests/no-eval-call-test.yml", "content": "…" } ] +} +``` + +Three decisions are worth the words. + +**`cases[].files` names paths rather than repeating content.** Content lives in +`tests.files`, once. Two copies of a fixture's bytes in one document is a thing +that can disagree with itself, and the case that a consumer would then have to +decide which copy is authoritative is not one worth creating. Every entry +resolves against `tests.files`, and a test asserts it. + +**`cases[].path` is what the runner is handed, and its type differs by engine +because the engines differ.** For `runtime` it is a directory: the harness calls +`executeRuntimeRule(root, rule)` with exactly that path, so a case's files are +the tree beneath it. For `vale` it is the document itself, because Vale lints +files and a bucket is one directory deep. `grouping` is what tells a consumer +which it is looking at, which is the whole point of publishing it. + +**`bucket` is `pass`/`fail`, matching the directories.** Not ast-grep's +`valid`/`invalid`: those are keys in a file we are deliberately not restating, +and the corpus should use the vocabulary of the layout it is describing. The +cloud team has already adopted `pass`/`fail` on their side. + +## Where the rule goes + +The corpus publishes paths relative to a rule directory, and until now it never +said what that directory is. A consumer materializing a rule — ours, to run +`taskless verify` against, or its own generated answer to the same prompt, which +has to go somewhere the CLI will look — had to assume `.taskless/rules///`. +Nothing in the file states it. + +```jsonc +"layout": { + "rulesRoot": ".taskless/rules", + "ruleDirectory": ".taskless/rules/{engine}/{id}", + "testsDirectory": ".tests", + "engines": { + "sg": { "ruleFile": "{id}.yml", "ruleConfigFile": null, "capturesDirectory": null }, + "vale": { "ruleFile": "{id}.yml", "ruleConfigFile": ".vale.ini", "capturesDirectory": null }, + "runtime": { "ruleFile": "check.ts", "ruleConfigFile": null, "capturesDirectory": "captures" } + } +} +``` + +and on each entry, the same values already resolved: + +```jsonc +{ "engine": "runtime", "id": "env-keys-declared", + "directory": ".taskless/rules/runtime/env-keys-declared", + "ruleFile": "check.ts", … } +``` + +**Both, and not one or the other.** The resolved `directory` needs no +substitution and cannot be got wrong, which is what a consumer wants for the +three rules in front of it. The template is what a consumer needs for a rule +that is not in the corpus at all — the one it just generated — and stating only +the resolved paths would leave three examples from which the pattern has to be +inferred, which is the same guess in a smaller costume. + +**`ruleFile` resolved per entry answers a question the file lists cannot.** +`rule` carries `check.ts` and `captures/env-read.yml` as a flat list with no +indication which is the rule and which is supporting material. That was +assumable while the corpus held one rule per engine and unassumable the moment +it does not. + +**`{engine}` and `{id}` are the only placeholders, and they are literal.** Not a +path DSL. `ruleFile` differs by engine as a function of the id — `{id}.yml` for +`sg` and `vale`, a constant `check.ts` for `runtime`, because a runtime rule is +a program rather than a document — and that difference is precisely the thing +worth publishing rather than describing. + +**Everything is generated from `ENGINE_LAYOUTS`**, the table the CLI itself +dispatches on, so the block cannot describe a layout the CLI does not +implement. `@taskless/cli/layout` already publishes that table as a module and +was created for this exact complaint from the Cloud Generator team; a JSON +consumer should not have to import a bundle to learn what its paths are +relative to. + +`.taskless` is the one value with nowhere to come from. It is a literal in about +fifteen places behind four constants that do not know about each other +(`CANONICAL_DIR` in `install/install.ts` and `install/canonical.ts`, +`TASKLESS_DIR` in `install/state.ts`, `TASKLESS_DIRECTORY` in `rules/scan.ts` +and `rules/vale/formats.ts`). One moves into `rules/layout.ts` beside the rest +of the table and `rulesRoot` reads it. Converging the other fourteen is a +tidy-up, and doing it inside a contract change would hide the contract change +inside a rename. + +## Alternatives rejected + +**Keep `tests` flat and add a sibling `cases` field.** Purely additive, so v1 +consumers keep working. Rejected because it leaves the grouping stated twice — +once implicitly in the path prefixes, once explicitly — and the artifact would +have no way to say which is authoritative when they disagree. The consumer told +us a version bump is a signal they handle, so the cost of the clean shape is one +they have already priced. + +**Case-relative file paths (`src/config.ts`, `.env`), as the issue sketched.** +Rejected because it loses where the file lives in our tree, and `vale`'s case is +a document rather than a directory, so there is nothing coherent for its one +file to be relative _to_. Publishing `path` and rule-relative file paths gives a +consumer both readings: strip the published prefix and you have the case-relative +form, with no layout knowledge involved. + +**Synthesise `sg` cases from `valid:`/`invalid:`.** Rejected for now — see the +proposal. The YAML is what `ast-grep test` consumes, so extracting snippets +serves no run anybody makes. + +## Constraint ids on verify output + +`violations` is added alongside `errors` rather than replacing it: + +```jsonc +{ + "engine": "sg", + "ruleId": "no-eval-call", + "ok": false, + "errors": ["id: \"mismatched\" does not match the rule's directory …"], + "violations": [ + { + "constraintId": "sg-id-matches-directory", + "message": "id: \"mismatched\" does not match the rule's directory …", + }, + ], +} +``` + +**The message is repeated rather than joined by index.** An index join between +two arrays is a contract nobody can see, and it breaks the first time an error +is filtered or reordered on either side. Repeating the string makes the pairing +explicit and lets a consumer ignore `errors` entirely. + +**Every `violations` message also appears in `errors`.** `errors` remains the +complete list; `violations` is the attributable subset. A consumer reading only +`errors` sees exactly what it sees today, which is what makes this additive. + +Internally the producers return `{ message, constraintId? }` and the JSON +boundary projects both fields out of it. Carrying two parallel arrays through +the layers is how they would drift. diff --git a/openspec/changes/reference-fixture-cases/proposal.md b/openspec/changes/reference-fixture-cases/proposal.md new file mode 100644 index 00000000..b9e99bab --- /dev/null +++ b/openspec/changes/reference-fixture-cases/proposal.md @@ -0,0 +1,122 @@ +## Why + +The cloud team grades rule generation against `@taskless/cli/reference.json` +(#263). They hit a defect our shape made hard to see, and the fix changes how +they model fixtures everywhere. + +Their request format carried a rule's cases as anonymous code strings — one +blob per case, no filename, no siblings. Generating from our +`runtime/env-keys-declared` prompt therefore produced a rule that **failed the +fixtures we ship beside it**, because our case is a two-file tree: + +``` +.tests/pass/declared/src/config.ts .tests/fail/undeclared/src/config.ts +.tests/pass/declared/.env .tests/fail/undeclared/.env +``` + +Reading one file to judge another is the entire reason that rule is `runtime` +rather than `sg`. A single-file case cannot express it, so nothing was declared +and the passing case flagged too. + +**Their shape is theirs to fix. Ours is what let it go unnoticed.** +`reference.json` publishes `tests` as `Array<{ path, content }>`, and recovering +which files belong to which case requires knowing our layout: + +| engine | a case is | +| --------- | ------------------------------------------------------------------------------------- | +| `runtime` | a **directory** under `pass/` or `fail/` — many files | +| `vale` | a **file** under `pass/` or `fail/` — one document | +| `sg` | not a directory layout at all: `valid:`/`invalid:` keys inside one ast-grep test YAML | + +That table is in our code three times (`rules/runtime/fixtures.ts`, +`rules/vale/verify.ts`, `rules/verify.ts`) and nowhere in the artifact that is +supposed to be authoritative about it. Every consumer transcribes it, and a +transcribed fact goes stale silently — which is the same failure mode +`RULE_CONSTRAINTS` was published to close, one field over. + +The corpus has no spec at all today. It was added across three `chore` commits +and is now an external contract with another team, which is the wrong order. + +## What Changes + +**`tests` states its own grouping.** It becomes an object carrying a `grouping` +discriminant, the flat file list it carries today, and — where the grouping is +_ours_ — an explicit list of cases. A consumer branches on `grouping`, a published +field, instead of on `engine` plus knowledge it copied from us. + +**`sg` keeps its ast-grep test YAML, and says so.** Its grouping lives inside +that file in ast-grep's own documented schema. Restating it here would move a +fact out of the upstream schema that owns it and into a copy we would then have +to keep true. `grouping: "ast-grep-test"` names that, so a consumer knows the +absence of `cases` is a statement rather than a gap. + +**The corpus states where a rule lives.** A top-level `layout` block carries the +tree — `.taskless/rules///`, which file _is_ the rule for each +engine, where its config and captures go, what the tests directory is called — +and each entry carries its own resolved `directory` and `ruleFile`. Every value +is derived from `ENGINE_LAYOUTS`, the table the CLI itself dispatches on. + +Without it the corpus publishes paths relative to a root it never names, so a +consumer materializing a rule — ours to run, or their own generated answer to +the same prompt — has to assume where the CLI looks. That assumption is not +recoverable from anything in the file. `@taskless/cli/layout` publishes the +table already and exists for exactly this reason, but it is a JavaScript module, +and a consumer reading a JSON artifact should not have to import a bundle to +find out what the artifact's paths are relative to. + +**`.taskless` gets one home.** It is currently a string literal in about fifteen +places behind four separate constants (`CANONICAL_DIR` twice, `TASKLESS_DIR`, +`TASKLESS_DIRECTORY` twice), so there is no single value the corpus could be +generated from. `TASKLESS_DIRECTORY` moves into `rules/layout.ts` beside the +rest of the table and `rulesRoot` reads it. The other call sites are left alone; +converging them is a tidy-up that does not belong in a contract change. + +**`version` goes to 2.** The consumer asserts it on load and stops rather than +interpret a shape it does not understand, which is the correct behaviour and +the reason a bump is enough of a signal. + +**`verify` and `test` name the constraint a rejection violated.** Each rule +result gains `violations`, pairing a `constraintId` from the published +`constraints[]` with the message that reports it. `errors` is unchanged and +still carries every message, so nothing that reads it today breaks. Without +this, mapping a rejection back to the rationale we already wrote is a text +match on our wording, which rots the first time we rephrase an error. + +**The output shapes become importable.** `@taskless/cli/schemas` publishes the +zod schemas for `verify --json` and `test --json`, and the constraint type. A +consumer parsing our JSON currently hand-writes the interface for it, which is +transcription of exactly the kind this issue is about, one field over from the +layout table. Data only: no filesystem, no spawn, no command tree. + +The CLI stays the execution surface. No `verify()` or `test()` function is +exported. Both spawn a vendored platform binary, so anywhere a function call +could run them `npx @taskless/cli verify --json` runs too — the export would buy +a call site, not a capability, while making internal signatures public in the +same release that changes them. + +## Delivery shape + +**Stacked, merging forward — two PRs.** Each is independently safe in +production and neither leaves the other half-migrated: + +1. `reference.json` v2. Self-contained: the artifact, its generator, its spec. +2. `violations` on `verify`/`test` output. Purely additive to a JSON envelope, + and useful whether or not (1) has landed. +3. `@taskless/cli/schemas`. A new export, and it goes last on purpose: it + publishes the envelope (2) changes, so shipping it first would publish a + shape and then immediately amend it. + +The changeset goes on PR 1 and each PR above extends it. + +## What this does not do + +**It does not extract `sg` snippets into cases.** Running an sg rule against +these cases means handing the YAML to `ast-grep test`, which consumes the file +as it stands; a snippet list would be a second representation of something no +consumer has to read. Reconsider if someone actually needs the snippets apart +from the file. + +**It does not attach a constraint id to every error.** Only the seven entries in +`RULE_CONSTRAINTS` have one. An error with no constraint behind it gets no id +rather than a plausible-looking one, because a wrong attribution sends someone +to the wrong rationale, which is worse than sending them to none. diff --git a/openspec/changes/reference-fixture-cases/specs/cli-conformance-reference/spec.md b/openspec/changes/reference-fixture-cases/specs/cli-conformance-reference/spec.md new file mode 100644 index 00000000..342a6966 --- /dev/null +++ b/openspec/changes/reference-fixture-cases/specs/cli-conformance-reference/spec.md @@ -0,0 +1,120 @@ +## ADDED Requirements + +### Requirement: The CLI publishes a conformance corpus + +The CLI SHALL publish `assets/reference.json` as the package export +`@taskless/cli/reference.json`, carrying a `version`, the `protocol` the corpus +exists to support, the `constraints` `verify` and `test` enforce beyond an +engine's own schema, and one entry per shipped demonstration rule. + +The corpus SHALL be built from the shipped rules rather than restating them, so +what another team grades against and what the CLI writes into a project cannot +come to describe different things. + +`version` SHALL be incremented whenever a consumer would have to change code to +keep reading the artifact. A consumer that asserts the version and stops is +behaving correctly, so the bump is the only signal the corpus owes it. + +#### Scenario: The corpus is reachable as a published export + +- **WHEN** the package is installed from the registry +- **THEN** `assets/reference.json` SHALL be among the published files +- **AND** the export map SHALL name `./reference.json` + +#### Scenario: The corpus separates the rule from its cases + +- **WHEN** a consumer reads a corpus entry +- **THEN** the files a generator must produce SHALL be carried apart from the cases both sides' rules must satisfy +- **AND** no file SHALL appear in both + +### Requirement: The corpus states how its fixtures group into cases + +Each corpus entry's `tests` SHALL carry a `grouping` field naming how its fixture +files group into cases, the flat list of fixture files with their contents, and +— where the grouping is the CLI's own — an explicit list of cases. + +A consumer SHALL be able to recover which files belong to which case by reading +published fields alone. Deriving it from path prefixes requires knowing that a +`runtime` case is a directory and a `vale` case is a document, which is a fact +about the CLI's layout that every consumer would otherwise transcribe, and a +transcribed fact goes stale silently when the layout changes. + +Each case SHALL name the `bucket` it asserts (`pass` or `fail`), its own `name`, +the `path` the engine is pointed at, and the fixture files it holds. Case files +SHALL be named by path rather than by repeated content, and every such path +SHALL resolve to an entry in the same `tests.files` list. + +`bucket` SHALL be spelled `pass`/`fail`, matching the directories the CLI reads, +rather than an individual engine's vocabulary. + +#### Scenario: A runtime case is a directory of files + +- **WHEN** a consumer reads the `runtime` entry +- **THEN** `grouping` SHALL be `case-directories` +- **AND** each case SHALL name the directory the check is handed as its root +- **AND** a case whose evidence spans two files SHALL list both + +#### Scenario: A vale case is one document + +- **WHEN** a consumer reads the `vale` entry +- **THEN** `grouping` SHALL be `case-documents` +- **AND** each case SHALL name the document and hold exactly that one file + +#### Scenario: An ast-grep entry says its grouping is not the CLI's + +- **WHEN** a consumer reads the `sg` entry +- **THEN** `grouping` SHALL be `ast-grep-test` +- **AND** the ast-grep test file SHALL be carried whole +- **AND** no case list SHALL be published, because the grouping inside that file belongs to ast-grep's own documented schema rather than to the CLI + +#### Scenario: Every case file resolves + +- **WHEN** the corpus is generated +- **THEN** every path a case names SHALL appear in that entry's `tests.files` +- **AND** every file in `tests.files` SHALL belong to exactly one case, where a case list is published + +### Requirement: The corpus states where a rule lives + +The corpus SHALL publish the rule layout: the root every rule hangs off, the +directory pattern a rule of a given engine and id occupies, the tests directory +name, and per engine which file is the rule, which is its engine config, and +where its capture rules live. + +Each entry SHALL additionally carry its own resolved rule directory and rule +file, so a consumer materializing the three rules in the corpus performs no +substitution at all, while a consumer placing a rule the corpus does not contain +has the pattern to place it by. + +Every value SHALL be derived from the same table the CLI dispatches on, so the +corpus cannot describe a layout the CLI does not implement. + +The corpus SHALL NOT publish file paths relative to a root it does not name. A +consumer that has to assume where the CLI looks is transcribing a fact about the +CLI, which is the failure this corpus exists to remove. + +#### Scenario: A consumer places a rule the corpus does not contain + +- **WHEN** a consumer generates its own rule for an engine and id +- **THEN** the corpus SHALL state the directory pattern that rule occupies +- **AND** it SHALL state which file inside that directory is the rule for that engine + +#### Scenario: The published layout matches the table the CLI dispatches on + +- **WHEN** the corpus is generated +- **THEN** the published layout SHALL agree with the CLI's own engine layout table +- **AND** each entry's resolved directory SHALL be the directory the CLI resolves for that engine and id + +### Requirement: The corpus publishes what verify enforces beyond the engine + +The corpus SHALL carry every entry in `RULE_CONSTRAINTS`, each with a stable +`id`, the `engine` it applies to, which command enforces it, a summary, and the +rationale behind it. + +Published because a generator that never reads the CLI's recipes cannot know +these, and because without the list a refusal is indistinguishable from a +disagreement about the subject the rule addresses. + +#### Scenario: A constraint says which command enforces it + +- **WHEN** a consumer plans the order of an evaluation +- **THEN** each constraint SHALL state whether `verify` or `test` refuses the rule diff --git a/openspec/changes/reference-fixture-cases/specs/cli-layout-export/spec.md b/openspec/changes/reference-fixture-cases/specs/cli-layout-export/spec.md new file mode 100644 index 00000000..cb09a8c7 --- /dev/null +++ b/openspec/changes/reference-fixture-cases/specs/cli-layout-export/spec.md @@ -0,0 +1,33 @@ +## ADDED Requirements + +### Requirement: The shapes a consumer parses are published as data + +The CLI SHALL publish the schemas describing its `--json` output as an +importable module, so that a consumer parsing that output validates against the +schema the CLI emits from rather than transcribing it into a hand-written type. + +This is the layout table's argument applied to the other half of the contract. A +consumer of `verify --json` today writes its own interface for the envelope, and +a hand-written interface is a copy that nothing checks: the CLI can add a field, +change a field's meaning, or rename an error code, and the copy stays confidently +wrong until something downstream misbehaves. + +The published module SHALL carry no host capability. A schema is data, and a +consumer wanting to know the shape of the CLI's output SHALL NOT thereby acquire +a dependency on the filesystem, a process spawn, or the command tree. + +The CLI SHALL remain the execution surface. Publishing the shapes does not +publish the operations: a consumer runs `taskless verify --json` and parses the +result, and no function that performs verification is exported. + +#### Scenario: A consumer parses verify output against the published schema + +- **WHEN** a consumer runs `taskless verify --json` and parses its output +- **THEN** the schema that output was produced against SHALL be importable +- **AND** parsing SHALL fail loudly on output the schema does not describe, rather than silently yielding a partially-typed value + +#### Scenario: The schema module reaches no host capability + +- **WHEN** the package is built +- **THEN** the schema entry's import graph SHALL reach no filesystem, process, or network capability +- **AND** it SHALL NOT reach the CLI entry diff --git a/openspec/changes/reference-fixture-cases/specs/cli-rule-validation/spec.md b/openspec/changes/reference-fixture-cases/specs/cli-rule-validation/spec.md new file mode 100644 index 00000000..85f3364e --- /dev/null +++ b/openspec/changes/reference-fixture-cases/specs/cli-rule-validation/spec.md @@ -0,0 +1,36 @@ +## ADDED Requirements + +### Requirement: A rejection names the constraint it violated + +`verify --json` and `test --json` SHALL report, per rule, the constraints a +rejection violated, pairing a `constraintId` drawn from the published +`RULE_CONSTRAINTS` with the message that reports it. + +The existing `errors` array SHALL continue to carry every failure message, +including those that are attributable. A consumer reading only `errors` SHALL +see what it sees today, so this is additive. + +An error with no constraint behind it SHALL NOT be given one. A wrong +attribution sends a reader to a rationale that does not describe their failure, +which is worse than sending them to none. + +A consumer SHALL NOT have to match on message text to recover the constraint. +Text matching rots the first time a message is rephrased, and rephrasing an +error message is not a breaking change. + +#### Scenario: A mismatched rule id is attributed + +- **WHEN** `verify --json` refuses a rule whose `id:` does not match its directory +- **THEN** the rule's result SHALL carry a violation with `constraintId` `sg-id-matches-directory` +- **AND** the violation's message SHALL also appear in `errors` + +#### Scenario: An unattributable failure carries no id + +- **WHEN** `verify --json` reports a failure that no published constraint describes +- **THEN** the message SHALL appear in `errors` +- **AND** no violation SHALL be reported for it + +#### Scenario: A passing rule reports no violations + +- **WHEN** `verify --json` accepts a rule +- **THEN** its violations SHALL be empty diff --git a/openspec/changes/reference-fixture-cases/tasks.md b/openspec/changes/reference-fixture-cases/tasks.md new file mode 100644 index 00000000..1c9cb8b7 --- /dev/null +++ b/openspec/changes/reference-fixture-cases/tasks.md @@ -0,0 +1,57 @@ +## 1. Corpus shape (PR 1) + +- [ ] 1.1 Add `ReferenceTests` to `src/rules/reference.ts`: a `grouping` + discriminant, `files`, and an optional `cases` list. +- [ ] 1.2 Move `TASKLESS_DIRECTORY` into `src/rules/layout.ts` and have + `rulesRoot` read it, so the corpus has one value to generate from. Leave + the other literals alone. +- [ ] 1.3 Publish the top-level `layout` block from `ENGINE_LAYOUTS`, and give + each entry its resolved `directory` and `ruleFile`. +- [ ] 1.4 Group `runtime` fixtures into case directories and `vale` fixtures into + case documents, from the manifest's `testPaths`. `sg` publishes + `grouping: "ast-grep-test"` and no cases. +- [ ] 1.5 Bump `REFERENCE_VERSION` to 2 and say in the docstring what changed. +- [ ] 1.6 Regenerate `assets/reference.json` (`pnpm --filter @taskless/cli reference`). +- [ ] 1.7 Extend `test/reference.test.ts`: every case path resolves into + `tests.files`; every fixture file belongs to exactly one case where cases + are published; `runtime`'s two-file case is carried as one case; `sg` + publishes no cases; the version is 2. +- [ ] 1.8 Assert the published `layout` agrees with `ENGINE_LAYOUTS` and that + each entry's `directory` is what `ruleDirectory` returns for it, so the + block cannot drift from the table the CLI dispatches on. +- [ ] 1.9 Changeset (minor — a published contract changed shape). + +## 2. Constraint ids on rejections (PR 2) + +- [ ] 2.1 Introduce the internal `{ message, constraintId? }` error shape and + thread it through the sg verify layers. +- [ ] 2.2 Attribute each of the seven `RULE_CONSTRAINTS` entries at the site + that raises it. Leave everything else unattributed. +- [ ] 2.3 Add `violations` to `src/schemas/verify-test.ts` and project it at the + JSON boundary, leaving `errors` unchanged. +- [ ] 2.4 Extend `test/constraints.test.ts` — it already builds a rule that + violates each entry keyed on `id`, so assert the reported `constraintId` + there rather than in a new file. +- [ ] 2.5 Assert an unattributable failure carries no violation, and that a + passing rule reports none. +- [ ] 2.6 Extend the changeset from PR 1 rather than adding a second. + +## 3. Published output schemas (PR 3) + +- [ ] 3.1 Add `src/schemas/index.ts` as the entry, re-exporting the `verify`/ + `test` envelope, the sg and vale verify-output schemas, and the constraint + type. Direct re-exports, no barrel of internals. +- [ ] 3.2 Wire it into `ENTRY_SOURCES` and `LIBRARY_ENTRIES` in `vite.config.ts`, + and into `exports` and `files` in `package.json`. The build's export + classification fails on an entry in neither list, so this is checked. +- [ ] 3.3 Confirm the graph reaches no host capability — the build asserts it, so + this is running the build, not writing a test that re-derives it. +- [ ] 3.4 Extend the changeset. + +## 4. Close out + +- [ ] 4.1 `pnpm typecheck`, `pnpm lint`, `pnpm --filter @taskless/cli test`. +- [ ] 4.2 Reply on #263 with the shape as shipped, and the two things + deliberately not done (sg snippets, per-error ids beyond the seven, and + any exported `verify()`/`test()`). +- [ ] 4.3 Archive on the tip PR. diff --git a/packages/cli/assets/reference.json b/packages/cli/assets/reference.json index 77ce2130..8af6d21a 100644 --- a/packages/cli/assets/reference.json +++ b/packages/cli/assets/reference.json @@ -1,5 +1,5 @@ { - "version": 1, + "version": 2, "protocol": [ "Generate a rule from `prompt`, using your own pipeline.", "Run `taskless verify` over what you generated. It enforces constraints beyond the engine's own schema, listed in `constraints` below, so a rule the engine executes correctly can still be refused. A rule that fails here is not deliverable however well it behaves, and every later step would be measuring the wrong thing. Check `enforcedBy` before concluding anything: some constraints are only decided once the fixtures run.", @@ -7,6 +7,31 @@ "Run your generated rule against `tests` here. A failure means your rule and ours disagree about the subject, and `tests` is the arbiter.", "Run the rule in `rule` here against your cases. A failure means your cases and ours disagree, which is worth as much as the previous step and is the one nobody runs." ], + "layout": { + "rulesRoot": ".taskless/rules", + "ruleDirectory": ".taskless/rules/{engine}/{id}", + "testsDirectory": ".tests", + "engines": { + "sg": { + "ruleFile": "{id}.yml", + "ruleConfigFile": null, + "capturesDirectory": null, + "fixtureLayout": "ast-grep-test" + }, + "vale": { + "ruleFile": "{id}.yml", + "ruleConfigFile": ".vale.ini", + "capturesDirectory": null, + "fixtureLayout": "case-documents" + }, + "runtime": { + "ruleFile": "check.ts", + "ruleConfigFile": null, + "capturesDirectory": "captures", + "fixtureLayout": "case-directories" + } + } + }, "constraints": [ { "id": "sg-id-matches-directory", @@ -62,6 +87,8 @@ { "engine": "sg", "id": "no-eval-call", + "directory": ".taskless/rules/sg/no-eval-call", + "ruleFile": "no-eval-call.yml", "prompt": "Create a rule that flags any call to `eval()` in TypeScript.\n\nExecuting a string at runtime is an injection risk, and it defeats every static\nanalysis the project runs: nothing downstream can see what the code will do.\nPrefer a parser, a lookup table, or an explicit dispatch.\n\nDeciding this needs only the expression itself. No other file has to be read,\nand no state has to be resolved.\n\nCode that should NOT be flagged:\n\n```ts\nconst parsed = JSON.parse(payload);\n```\n\nCode that SHOULD be flagged:\n\n```ts\nconst result = eval(payload);\n```\n", "rule": [ { @@ -69,16 +96,21 @@ "content": "id: no-eval-call\nlanguage: TypeScript\nseverity: error\nmessage: \"`eval` executes arbitrary code at runtime; use a parser or a lookup table instead.\"\nnote: |-\n This is an example rule, shipped with the Taskless CLI to demonstrate the\n `sg` tier. It was not written for this repository.\n\n The evidence is one expression in one file, which is what makes this an `sg`\n rule rather than a runtime one: no other file has to be read to decide it.\nrule:\n pattern: eval($ARG)\n" } ], - "tests": [ - { - "path": ".tests/no-eval-call-test.yml", - "content": "id: no-eval-call\nvalid:\n - const parsed = JSON.parse(payload);\ninvalid:\n - const result = eval(payload);\n" - } - ] + "tests": { + "grouping": "ast-grep-test", + "files": [ + { + "path": ".tests/no-eval-call-test.yml", + "content": "id: no-eval-call\nvalid:\n - const parsed = JSON.parse(payload);\ninvalid:\n - const result = eval(payload);\n" + } + ] + } }, { "engine": "vale", "id": "prefer-use-over-utilize", + "directory": ".taskless/rules/vale/prefer-use-over-utilize", + "ruleFile": "prefer-use-over-utilize.yml", "prompt": "Create a rule that flags the word \"utilize\" in markdown documentation and\nsuggests \"use\" instead.\n\n\"Utilize\" is longer than \"use\" and means the same thing in nearly every\nsentence a reader will meet. It should match case-insensitively and cover the\ninflected forms.\n\nThis is a prose rule. It applies to markdown, and it must not look at source\ncode.\n\nProse that should NOT be flagged:\n\n```md\nUse the installer to write the config file.\n```\n\nProse that SHOULD be flagged:\n\n```md\nUtilize the installer to write the config file.\n```\n", "rule": [ { @@ -90,20 +122,39 @@ "content": "# Scoped to markdown, which is where prose rules belong. A demonstration rule\n# should never widen its own scope into a project's source.\n[**/*.md]\nBasedOnStyles =\nprefer-use-over-utilize.prefer-use-over-utilize = YES\n" } ], - "tests": [ - { - "path": ".tests/pass/README.md", - "content": "# Setup\n\nUse the installer to write the config file.\n" - }, - { - "path": ".tests/fail/README.md", - "content": "# Setup\n\nUtilize the installer to write the config file.\n" - } - ] + "tests": { + "grouping": "case-documents", + "files": [ + { + "path": ".tests/pass/README.md", + "content": "# Setup\n\nUse the installer to write the config file.\n" + }, + { + "path": ".tests/fail/README.md", + "content": "# Setup\n\nUtilize the installer to write the config file.\n" + } + ], + "cases": [ + { + "bucket": "pass", + "name": "README.md", + "path": ".tests/pass/README.md", + "files": [".tests/pass/README.md"] + }, + { + "bucket": "fail", + "name": "README.md", + "path": ".tests/fail/README.md", + "files": [".tests/fail/README.md"] + } + ] + } }, { "engine": "runtime", "id": "env-keys-declared", + "directory": ".taskless/rules/runtime/env-keys-declared", + "ruleFile": "check.ts", "prompt": "Create a rule that requires every environment variable read through\n`process.env` in a JavaScript or TypeScript file to have a matching key\ndeclared in the `.env` file at the repository root.\n\nReading a variable that is never declared is the failure worth catching: it is\n`undefined` at runtime rather than an error, so the program continues with a\nmissing value and fails somewhere else entirely.\n\nDeciding this needs both files at once — the read and the declaration — so it\ncannot be answered by matching a pattern within a single file.\n\nA repository that should NOT be flagged:\n\n```ts\n// src/config.ts\nexport const apiUrl = process.env.API_URL;\n```\n\n```\n# .env\nAPI_URL=https://api.example.com\n```\n\nA repository that SHOULD be flagged, on the second line only:\n\n```ts\n// src/config.ts\nexport const apiUrl = process.env.API_URL;\nexport const apiKey = process.env.API_KEY;\n```\n\n```\n# .env\nAPI_URL=https://api.example.com\n```\n", "rule": [ { @@ -115,24 +166,47 @@ "content": "# The syntactic narrow. It finds every `process.env.X` read; it cannot know\n# whether X is declared, because that answer lives in a different file. Deciding\n# is `check.ts`'s job, which is what makes this a runtime rule rather than an\n# `sg` one.\nid: env-read\nlanguage: TypeScript\nrule:\n pattern: process.env.$VAR\nmetadata:\n taskless:\n version: 1\n kind: runtime\n name: env-read\n check: check.ts\n match: anchor\n" } ], - "tests": [ - { - "path": ".tests/pass/declared/src/config.ts", - "content": "// API_URL is read here and declared in .env, so this case is clean.\nexport const apiUrl = process.env.API_URL;\n" - }, - { - "path": ".tests/pass/declared/.env", - "content": "API_URL=https://api.example.com\n" - }, - { - "path": ".tests/fail/undeclared/src/config.ts", - "content": "// API_URL is declared; API_KEY is not, so the rule fires on the second.\nexport const apiUrl = process.env.API_URL;\nexport const apiKey = process.env.API_KEY;\n" - }, - { - "path": ".tests/fail/undeclared/.env", - "content": "API_URL=https://api.example.com\n" - } - ], + "tests": { + "grouping": "case-directories", + "files": [ + { + "path": ".tests/pass/declared/src/config.ts", + "content": "// API_URL is read here and declared in .env, so this case is clean.\nexport const apiUrl = process.env.API_URL;\n" + }, + { + "path": ".tests/pass/declared/.env", + "content": "API_URL=https://api.example.com\n" + }, + { + "path": ".tests/fail/undeclared/src/config.ts", + "content": "// API_URL is declared; API_KEY is not, so the rule fires on the second.\nexport const apiUrl = process.env.API_URL;\nexport const apiKey = process.env.API_KEY;\n" + }, + { + "path": ".tests/fail/undeclared/.env", + "content": "API_URL=https://api.example.com\n" + } + ], + "cases": [ + { + "bucket": "pass", + "name": "declared", + "path": ".tests/pass/declared", + "files": [ + ".tests/pass/declared/src/config.ts", + ".tests/pass/declared/.env" + ] + }, + { + "bucket": "fail", + "name": "undeclared", + "path": ".tests/fail/undeclared", + "files": [ + ".tests/fail/undeclared/src/config.ts", + ".tests/fail/undeclared/.env" + ] + } + ] + }, "signature": "1;h=sha-256;d=0000000000000000000000000000000000000000000000000000000000000000" } ] diff --git a/packages/cli/src/layout/index.ts b/packages/cli/src/layout/index.ts index c415bb86..144e720a 100644 --- a/packages/cli/src/layout/index.ts +++ b/packages/cli/src/layout/index.ts @@ -40,8 +40,10 @@ export { ENGINE_LAYOUTS, RULES_DIRECTORY, RULE_TESTS_DIRECTORY, + TASKLESS_DIRECTORY, isKnownEngine, type EngineExecutor, type EngineLayout, type EngineName, + type FixtureLayout, } from "../rules/layout.js"; diff --git a/packages/cli/src/rules/engines.ts b/packages/cli/src/rules/engines.ts index da5e71a6..9137e0bd 100644 --- a/packages/cli/src/rules/engines.ts +++ b/packages/cli/src/rules/engines.ts @@ -9,13 +9,14 @@ import { isKnownEngine, RULE_TESTS_DIRECTORY, RULES_DIRECTORY, + TASKLESS_DIRECTORY, type EngineExecutor, type EngineName, } from "./layout"; /** `.taskless/rules`, the root every rule lives under. */ export function rulesRoot(cwd: string): string { - return join(cwd, ".taskless", RULES_DIRECTORY); + return join(cwd, TASKLESS_DIRECTORY, RULES_DIRECTORY); } /** `.taskless/rules/`, the directory holding that engine's rules. */ diff --git a/packages/cli/src/rules/layout.ts b/packages/cli/src/rules/layout.ts index 5c5f7288..617dd64c 100644 --- a/packages/cli/src/rules/layout.ts +++ b/packages/cli/src/rules/layout.ts @@ -27,6 +27,32 @@ export const ENGINES = ["sg", "vale", "runtime"] as const; export type EngineName = (typeof ENGINES)[number]; +/** + * How a rule's fixtures group into cases. + * + * Stated here rather than left implicit in each engine's fixture reader, which + * is where it lived: `runtime/fixtures.ts` rejects a loose file because a case + * is a directory, `vale/verify.ts` rejects a nested directory because a case is + * a document, and `verify.ts` counts ast-grep's own `valid:`/`invalid:` keys. + * The fact was recoverable only by reading three rejections, so every consumer + * transcribed it — which is what published this table in the first place. + * + * Each value is pinned beside the rejection that implements it, rather than in + * a test of its own: `runtime-fixtures.test.ts` asserts `case-directories` + * where it proves a loose file is refused, `vale-verify.test.ts` asserts + * `case-documents` where it proves a nested directory is refused, and + * `reference.test.ts` asserts `ast-grep-test` publishes no cases. A reader + * changing a reader's mind about its layout meets the declaration in the same + * test, which a separate file would not have achieved. + */ +export type FixtureLayout = + /** A case is a directory under `pass/` or `fail/`, handed to the check as its root. */ + | "case-directories" + /** A case is one document under `pass/` or `fail/`. */ + | "case-documents" + /** Not a directory layout: `valid:`/`invalid:` keys inside one ast-grep test file. */ + | "ast-grep-test"; + /** How a rule reaches execution, or `null` when this CLI has no executor yet. */ export type EngineExecutor = | "ast-grep" @@ -55,6 +81,8 @@ export interface EngineLayout { ruleConfigFile: string | undefined; /** Subdirectory holding ast-grep capture rules, for engines that use them. */ capturesDirectory: string | undefined; + /** How this engine's fixtures group into cases. See {@link FixtureLayout}. */ + fixtureLayout: FixtureLayout; executor: EngineExecutor; } @@ -66,6 +94,24 @@ export interface EngineLayout { */ export const RULES_DIRECTORY = "rules"; +/** + * The directory the whole tree hangs off, relative to the project root. + * + * Here rather than beside its callers because the conformance corpus publishes + * `.taskless/rules///` as a path a consumer can act on, and a + * published path needs one value to be generated from. + * + * It is not yet the only copy. The literal appears in about fifteen places + * behind four constants that do not know about each other — `CANONICAL_DIR` in + * `install/install.ts` and `install/canonical.ts`, `TASKLESS_DIR` in + * `install/state.ts`, `TASKLESS_DIRECTORY` in `rules/scan.ts` and + * `rules/vale/formats.ts`. `rulesRoot` reads this one, so what the corpus + * publishes is what the rule commands resolve. Converging the rest is a + * tidy-up, and doing it inside a contract change would hide the contract + * change inside a rename. + */ +export const TASKLESS_DIRECTORY = ".taskless"; + /** * A rule's tests, relative to its rule directory. **The dot is load-bearing.** * @@ -95,6 +141,7 @@ export const ENGINE_LAYOUTS = { ruleFile: (ruleId: string) => `${ruleId}.yml`, ruleConfigFile: undefined, capturesDirectory: undefined, + fixtureLayout: "ast-grep-test", executor: "ast-grep", }, vale: { @@ -102,6 +149,7 @@ export const ENGINE_LAYOUTS = { ruleFile: (ruleId: string) => `${ruleId}.yml`, ruleConfigFile: ".vale.ini", capturesDirectory: undefined, + fixtureLayout: "case-documents", executor: "vale-runner", }, runtime: { @@ -112,6 +160,10 @@ export const ENGINE_LAYOUTS = { // config section elsewhere in this tree, and one word for two unrelated // concepts is a cost paid at every future reading. capturesDirectory: "captures", + // Directories, not documents. A runtime rule exists because its evidence + // spans more than one file, so a one-file case could not express the rules + // this tier is for. + fixtureLayout: "case-directories", executor: "runtime-harness", }, } satisfies Record; diff --git a/packages/cli/src/rules/reference.ts b/packages/cli/src/rules/reference.ts index 35989246..4b7c8e99 100644 --- a/packages/cli/src/rules/reference.ts +++ b/packages/cli/src/rules/reference.ts @@ -1,5 +1,13 @@ import type { DeliveredFile } from "./deliver"; -import type { EngineName } from "./layout"; +import { + ENGINE_LAYOUTS, + ENGINES, + RULE_TESTS_DIRECTORY, + RULES_DIRECTORY, + TASKLESS_DIRECTORY, + type EngineName, + type FixtureLayout, +} from "./layout"; import { RULE_CONSTRAINTS, type RuleConstraint } from "./constraints"; /** @@ -23,8 +31,18 @@ import { RULE_CONSTRAINTS, type RuleConstraint } from "./constraints"; */ export const REFERENCE_SIGNATURE = `1;h=sha-256;d=${"0".repeat(64)}`; -/** Bumped when a consumer would have to change code to keep reading this. */ -export const REFERENCE_VERSION = 1; +/** + * Bumped when a consumer would have to change code to keep reading this. + * + * 2 — `tests` went from a flat `{ path, content }[]` to an object stating how + * its files group into cases, and a `layout` block was added naming the tree + * every path in this document is relative to. Both were facts a consumer could + * previously only get by transcribing them out of this repository. + * + * A consumer that asserts this and stops is behaving correctly, which is what + * makes the bump a sufficient signal on its own. + */ +export const REFERENCE_VERSION = 2; /** * The check this corpus exists to make possible, stated in the artifact itself. @@ -52,23 +70,139 @@ export const REFERENCE_PROTOCOL = [ "Run the rule in `rule` here against your cases. A failure means your cases and ours disagree, which is worth as much as the previous step and is the one nobody runs.", ]; +/** A fixture file, and the rule-relative path it is written to. */ +export interface ReferenceFile { + path: string; + content: string; +} + +/** Which side of the claim a case asserts. */ +export type FixtureBucket = "pass" | "fail"; + +/** + * One fixture case: the unit a runner is pointed at, and the files it holds. + * + * `path` is what the engine is handed, and its kind differs by engine because + * the engines differ — a directory for `runtime`, whose check is called with + * that path as its root; the document itself for `vale`, which lints files. The + * enclosing {@link ReferenceTests.grouping} is what tells a reader which of the + * two it is looking at, and publishing that is this shape's entire purpose. + * + * `files` names paths rather than repeating content. The bytes live once, in + * {@link ReferenceTests.files}. Two copies of a fixture in one document is a + * thing that can disagree with itself, and nothing in the artifact could then + * say which copy is authoritative. + */ +export interface ReferenceCase { + bucket: FixtureBucket; + /** The case's own name — a directory name, or a document's filename. */ + name: string; + /** Rule-relative, and what the runner is given. */ + path: string; + /** Rule-relative paths, each resolving to an entry in `tests.files`. */ + files: string[]; +} + +/** + * A rule's held-out cases, with the grouping stated rather than implied. + * + * The flat `{ path, content }[]` this replaces required a consumer to know that + * a `runtime` case is a directory and a `vale` case is a document before it + * could tell which files belong together. That fact lives in this repository, + * so every consumer transcribed it, and a transcribed fact goes stale silently + * when the layout changes. It cost the Cloud eval team a rule that failed the + * fixtures shipped beside it (#263): their request format carried one anonymous + * blob per case, so the two-file `runtime` case could not be expressed and the + * rule was graded against half of itself. + * + * `cases` is absent for `ast-grep-test`, and the absence is a statement. That + * grouping lives inside ast-grep's own test file, in a schema ast-grep + * documents and owns; restating it here would move a fact out of the schema + * that owns it and into a copy this repository would then have to keep true. + */ +export interface ReferenceTests { + grouping: FixtureLayout; + files: ReferenceFile[]; + /** Present for every grouping the CLI itself defines. */ + cases?: ReferenceCase[]; +} + /** One rule as this corpus carries it. */ export interface ReferenceRule { engine: EngineName; id: string; + /** + * Where this rule's directory goes, relative to the project root. + * + * Resolved rather than left to the consumer to assemble from + * {@link ReferenceLayout.ruleDirectory}. Every other path in this entry is + * relative to it, and a corpus that publishes relative paths without naming + * the root is asking a reader to guess where the CLI looks. + */ + directory: string; + /** + * Which file in `rule` *is* the rule, as opposed to its config or its + * capture rules. + * + * `rule` is a flat list: `check.ts` and `captures/env-read.yml` are + * indistinguishable in it. That was assumable while the corpus held one rule + * per engine, and stops being assumable the moment it does not. + */ + ruleFile: string; /** The generation request, as a caller would phrase it. */ prompt: string; - /** What a generator must produce. Paths are relative to the rule directory. */ - rule: { path: string; content: string }[]; + /** What a generator must produce. Paths are relative to `directory`. */ + rule: ReferenceFile[]; /** The held-out cases both sides' rules must satisfy. */ - tests: { path: string; content: string }[]; + tests: ReferenceTests; /** Present only where execution is gated on it. See the constant's note. */ signature?: string; } +/** One engine's slot in the layout table, as the corpus publishes it. */ +export interface ReferenceEngineLayout { + /** `{id}` where the name follows the rule id, a constant where it does not. */ + ruleFile: string; + ruleConfigFile: string | null; + capturesDirectory: string | null; + fixtureLayout: FixtureLayout; +} + +/** + * The tree every path in this document is relative to. + * + * Published because the corpus is otherwise a set of paths with no stated root, + * and a consumer materializing a rule — one of ours to run `taskless verify` + * against, or its own generated answer to the same prompt, which has to land + * somewhere the CLI will look — would have to assume where that is. + * + * Both the template and each entry's resolved {@link ReferenceRule.directory} + * are published, deliberately. The resolved path needs no substitution and + * covers the rules in this file; the template covers the rule a consumer just + * generated, which is not in this file at all. Publishing only the resolved + * paths would leave three examples from which the pattern has to be inferred, + * which is the same guess in a smaller costume. + * + * `{engine}` and `{id}` are literal placeholders and the only two. This is not + * a path DSL: `ruleFile` differs by engine as a function of the id — `{id}.yml` + * for the document engines, a constant `check.ts` for `runtime`, because a + * runtime rule is a program — and that difference is the thing worth + * publishing as data rather than describing in prose. + * + * Generated from {@link ENGINE_LAYOUTS}, the table the CLI itself dispatches + * on, so this cannot describe a layout the CLI does not implement. + */ +export interface ReferenceLayout { + rulesRoot: string; + ruleDirectory: string; + testsDirectory: string; + engines: Record; +} + export interface Reference { version: number; protocol: string[]; + layout: ReferenceLayout; /** * What `verify` and `test` enforce beyond the engine's own schema. * @@ -90,9 +224,130 @@ export interface ReferenceInput { testFiles: readonly DeliveredFile[]; } -const plain = (files: readonly DeliveredFile[]) => +/** + * The placeholder a template stands the rule id in for. + * + * `ENGINE_LAYOUTS[engine].ruleFile` is a function of the rule id, so calling it + * with this recovers the template it applies — `{id}.yml` where the name + * follows the id, `check.ts` where it does not. Derived rather than transcribed: + * an engine that changes how it names its rule file changes what is published + * here, with nothing to keep in step. + */ +const ID_PLACEHOLDER = "{id}"; + +const RULES_ROOT = `${TASKLESS_DIRECTORY}/${RULES_DIRECTORY}`; + +/** The rule directory for an engine and id, as a consumer must spell it. */ +function referenceRuleDirectory(engine: string, ruleId: string): string { + return `${RULES_ROOT}/${engine}/${ruleId}`; +} + +const REFERENCE_LAYOUT: ReferenceLayout = { + rulesRoot: RULES_ROOT, + ruleDirectory: referenceRuleDirectory("{engine}", ID_PLACEHOLDER), + testsDirectory: RULE_TESTS_DIRECTORY, + engines: Object.fromEntries( + ENGINES.map((engine) => { + const layout = ENGINE_LAYOUTS[engine]; + return [ + engine, + { + ruleFile: layout.ruleFile(ID_PLACEHOLDER), + // `null` rather than an absent key: a consumer reading JSON cannot + // tell a field this engine does not have from a field the corpus + // forgot to write. + ruleConfigFile: layout.ruleConfigFile ?? null, + capturesDirectory: layout.capturesDirectory ?? null, + fixtureLayout: layout.fixtureLayout, + }, + ]; + }) + ) as Record, +}; + +const plain = (files: readonly DeliveredFile[]): ReferenceFile[] => files.map((file) => ({ path: file.path, content: file.content })); +/** + * The bucket and remaining segments of a fixture path, or a thrown explanation. + * + * Every fixture lives under `.tests//…`. A path that does not is not a + * case this corpus can group, and the generator REFUSES rather than emitting a + * corpus whose grouping is a guess — a mis-grouped case is exactly the defect + * publishing the grouping exists to prevent, and it would be published as fact. + */ +function fixtureSegments( + ruleId: string, + path: string +): { bucket: FixtureBucket; rest: string[] } { + const segments = path.split("/"); + const [tests, bucket, ...rest] = segments; + if ( + tests !== RULE_TESTS_DIRECTORY || + (bucket !== "pass" && bucket !== "fail") + ) + throw new Error( + `${ruleId}: fixture ${path} is not under ${RULE_TESTS_DIRECTORY}/pass/ ` + + `or ${RULE_TESTS_DIRECTORY}/fail/, so the corpus cannot say which case ` + + `it belongs to.` + ); + if (rest.length === 0) + throw new Error( + `${ruleId}: fixture ${path} is the bucket directory itself, not a case.` + ); + return { bucket, rest }; +} + +/** + * Group a rule's fixture files into cases, by the layout its engine declares. + * + * Keyed on `fixtureLayout` rather than on the engine name, so this reads the + * same fact the fixture runners implement instead of restating it. A fourth + * engine states its layout in the table and is grouped here without a change. + */ +function groupCases( + engine: EngineName, + ruleId: string, + files: ReferenceFile[] +): ReferenceCase[] | undefined { + const grouping = ENGINE_LAYOUTS[engine].fixtureLayout; + if (grouping === "ast-grep-test") return undefined; + + const cases = new Map(); + for (const file of files) { + const { bucket, rest } = fixtureSegments(ruleId, file.path); + // A document engine's case IS the file; a directory engine's case is the + // first segment below the bucket, and everything deeper is its tree. + const name = rest[0] ?? ""; + if (grouping === "case-documents" && rest.length > 1) + throw new Error( + `${ruleId}: ${file.path} is nested, but a ${engine} case is one ` + + `document. Vale lints the rule's whole directory, so a nested ` + + `fixture is linted and never attributed to a case.` + ); + // The opposite rejection, and it is the one `bucketCases` already makes + // when the runner reads these buckets off disk. Publishing a bare file as + // a one-file "case" would emit a corpus indistinguishable in shape from a + // valid `case-documents` one, that `taskless test` throws on. + if (grouping === "case-directories" && rest.length === 1) + throw new Error( + `${ruleId}: ${file.path} is a bare file, but a ${engine} case is a ` + + `directory. The check is handed the case directory as its root and ` + + `reads the files it needs beneath it, so a bare file in ${bucket}/ ` + + `has no root to be and would never run.` + ); + + const path = `${RULE_TESTS_DIRECTORY}/${bucket}/${name}`; + const existing = cases.get(path); + if (existing === undefined) { + cases.set(path, { bucket, name, path, files: [file.path] }); + } else { + existing.files.push(file.path); + } + } + return [...cases.values()]; +} + /** * The shipped demonstration rules as a conformance corpus. * @@ -110,14 +365,27 @@ export function buildReference(rules: readonly ReferenceInput[]): Reference { return { version: REFERENCE_VERSION, protocol: REFERENCE_PROTOCOL, + layout: REFERENCE_LAYOUT, constraints: [...RULE_CONSTRAINTS], - rules: rules.map((rule) => ({ - engine: rule.engine, - id: rule.ruleId, - prompt: rule.prompt, - rule: plain(rule.ruleFiles), - tests: plain(rule.testFiles), - ...(rule.engine === "runtime" ? { signature: REFERENCE_SIGNATURE } : {}), - })), + rules: rules.map((rule) => { + const files = plain(rule.testFiles); + const cases = groupCases(rule.engine, rule.ruleId, files); + return { + engine: rule.engine, + id: rule.ruleId, + directory: referenceRuleDirectory(rule.engine, rule.ruleId), + ruleFile: ENGINE_LAYOUTS[rule.engine].ruleFile(rule.ruleId), + prompt: rule.prompt, + rule: plain(rule.ruleFiles), + tests: { + grouping: ENGINE_LAYOUTS[rule.engine].fixtureLayout, + files, + ...(cases === undefined ? {} : { cases }), + }, + ...(rule.engine === "runtime" + ? { signature: REFERENCE_SIGNATURE } + : {}), + }; + }), }; } diff --git a/packages/cli/test/layout.test.ts b/packages/cli/test/layout.test.ts index adf7e610..3d129626 100644 --- a/packages/cli/test/layout.test.ts +++ b/packages/cli/test/layout.test.ts @@ -40,6 +40,11 @@ const PUBLIC_EXPORTS = [ "ENGINE_LAYOUTS", "RULES_DIRECTORY", "RULE_TESTS_DIRECTORY", + // Widened deliberately. The conformance corpus publishes + // `.taskless/rules///` as a path a consumer acts on, so the + // directory the tree hangs off is part of the layout a consumer needs rather + // than an internal of the module that happens to hold it. + "TASKLESS_DIRECTORY", "isKnownEngine", ] as const; diff --git a/packages/cli/test/reference.test.ts b/packages/cli/test/reference.test.ts index 51806dc7..6ccb30c5 100644 --- a/packages/cli/test/reference.test.ts +++ b/packages/cli/test/reference.test.ts @@ -1,12 +1,18 @@ import { readFile } from "node:fs/promises"; -import { join } from "node:path"; +import { join, sep } from "node:path"; import { describe, expect, it } from "vitest"; import { parseSignature } from "../src/rules/rule-hash"; -import { buildReference, type Reference } from "../src/rules/reference"; +import { + buildReference, + type Reference, + type ReferenceInput, +} from "../src/rules/reference"; import { RULE_CONSTRAINTS } from "../src/rules/constraints"; import { DEMO_MANIFESTS, writtenPaths } from "../src/rules/demo/manifest"; +import { ENGINE_LAYOUTS, ENGINES, type EngineName } from "../src/rules/layout"; +import { ruleDirectory } from "../src/rules/engines"; import { DEMO_RULES } from "../src/rules/demo/rule"; /** @@ -117,15 +123,16 @@ describe("the demo reference payload", () => { rule.rule.length, `${rule.id} carries no rule files` ).toBeGreaterThan(0); - expect(rule.tests.length, `${rule.id} carries no cases`).toBeGreaterThan( - 0 - ); + expect( + rule.tests.files.length, + `${rule.id} carries no cases` + ).toBeGreaterThan(0); // If a case leaked into `rule`, "run their rule against our cases" would // hand over our answers along with the question, and the cross would // measure nothing. const rulePaths = new Set(rule.rule.map((file) => file.path)); - for (const test of rule.tests) { + for (const test of rule.tests.files) { expect(rulePaths.has(test.path), `${test.path} is in both halves`).toBe( false ); @@ -137,6 +144,110 @@ describe("the demo reference payload", () => { } }); + it("says how its fixtures group, so nobody re-derives it from paths", async () => { + const reference = await readReference(); + + // The defect this shape exists to prevent, stated as the assertion that + // would have caught it. The Cloud eval team's request format carried one + // anonymous blob per case, so this two-file case arrived as two one-file + // cases and the rule was graded against half of itself (#263). + const runtime = ruleFor(reference, "runtime"); + expect(runtime.tests.grouping).toBe("case-directories"); + const failing = runtime.tests.cases?.filter((one) => one.bucket === "fail"); + expect(failing).toHaveLength(1); + expect(failing?.[0]?.path).toBe(".tests/fail/undeclared"); + expect(failing?.[0]?.files.toSorted()).toEqual([ + ".tests/fail/undeclared/.env", + ".tests/fail/undeclared/src/config.ts", + ]); + + // A vale case is the document, not the bucket it sits in. + const vale = ruleFor(reference, "vale"); + expect(vale.tests.grouping).toBe("case-documents"); + for (const one of vale.tests.cases ?? []) { + expect(one.files).toEqual([one.path]); + } + }); + + it("publishes no cases for ast-grep, and that absence is the statement", async () => { + const reference = await readReference(); + const sg = ruleFor(reference, "sg"); + + // Not an omission. The grouping is inside ast-grep's own test file, in a + // schema ast-grep documents and owns. Restating it here would be a copy + // this repository would then have to keep true. + expect(sg.tests.grouping).toBe("ast-grep-test"); + expect(sg.tests.cases).toBeUndefined(); + expect(sg.tests.files).toHaveLength(1); + }); + + it("names every case file exactly once, and names no file it does not carry", async () => { + const reference = await readReference(); + for (const rule of reference.rules) { + if (rule.tests.cases === undefined) continue; + const carried = new Set(rule.tests.files.map((file) => file.path)); + const named = rule.tests.cases.flatMap((one) => one.files); + + // A case naming a path the corpus does not carry is a case a consumer + // cannot materialize, and it would read as a fixture we forgot to ship. + for (const path of named) { + expect(carried.has(path), `${rule.id}: case names absent ${path}`).toBe( + true + ); + } + // And the other direction: a fixture in no case is one that silently + // never runs, which is the failure the runners exist to catch. + expect(named.toSorted()).toEqual([...carried].toSorted()); + } + }); + + it("names the tree its paths are relative to", async () => { + const reference = await readReference(); + + // Without this the corpus is a set of paths with no stated root, and a + // consumer materializing a rule has to assume where the CLI looks. + expect(reference.layout.rulesRoot).toBe(".taskless/rules"); + expect(reference.layout.ruleDirectory).toBe( + ".taskless/rules/{engine}/{id}" + ); + expect(reference.layout.testsDirectory).toBe(".tests"); + + // Generated from the table the CLI dispatches on, so it cannot describe a + // layout the CLI does not implement. + for (const engine of ENGINES) { + const published = reference.layout.engines[engine]; + const layout = ENGINE_LAYOUTS[engine]; + expect(published.ruleFile).toBe(layout.ruleFile("{id}")); + expect(published.ruleConfigFile).toBe(layout.ruleConfigFile ?? null); + expect(published.capturesDirectory).toBe( + layout.capturesDirectory ?? null + ); + expect(published.fixtureLayout).toBe(layout.fixtureLayout); + } + }); + + it("resolves each rule's directory the way the CLI resolves it", async () => { + const reference = await readReference(); + for (const rule of reference.rules) { + // Against the resolver the commands use, not against a second copy of + // the pattern -- a template and a published path that agree with each + // other but not with `verify` would pass a weaker test. + expect(ruleDirectory("", rule.engine, rule.id).split(sep).join("/")).toBe( + rule.directory + ); + expect(rule.ruleFile).toBe(ENGINE_LAYOUTS[rule.engine].ruleFile(rule.id)); + expect(rule.rule.map((file) => file.path)).toContain(rule.ruleFile); + } + }); + + it("states a version a consumer can refuse on", async () => { + const reference = await readReference(); + // 2 is `tests` becoming an object and `layout` arriving. A consumer that + // asserts this and stops is behaving correctly, which is what makes the + // bump a sufficient signal on its own. + expect(reference.version).toBe(2); + }); + it("carries the prompt each rule answers", async () => { const reference = await readReference(); for (const rule of reference.rules) { @@ -180,3 +291,56 @@ describe("the reference payload is reachable by another team", () => { ); }); }); + +/** + * One rule's worth of input, with only the fixture paths varying. + * + * Hand-built rather than mutated out of `DEMO_MANIFESTS`, so a test that + * asserts a refusal cannot also alter what the real corpus publishes. + */ +function refusalInput(engine: EngineName, testPaths: string[]): ReferenceInput { + return { + engine, + ruleId: "example-rule", + prompt: "A prompt, unused by the grouping.", + ruleFiles: [ + { path: ENGINE_LAYOUTS[engine].ruleFile("example-rule"), content: "" }, + ], + testFiles: testPaths.map((path) => ({ path, content: "" })), + }; +} + +describe("the corpus refuses fixtures it cannot group", () => { + it("rejects a bare file where a runtime case must be a directory", () => { + // The generator held a weaker copy of the invariant `bucketCases` enforces + // when it reads these buckets off disk. Without this, the corpus publishes + // a one-file "directory" case that `taskless test` throws on. + expect(() => + buildReference([refusalInput("runtime", [".tests/fail/oops.ts"])]) + ).toThrow(/has no root to be/); + }); + + it("rejects a nested fixture where a vale case is one document", () => { + // Vale lints the rule's whole directory, so a nested fixture is linted and + // never attributed to a case — it would be graded, invisibly. + expect(() => + buildReference([refusalInput("vale", [".tests/pass/deep/README.md"])]) + ).toThrow(/is nested/); + }); + + it("rejects a fixture that is under neither bucket", () => { + // Nothing decides which side of the claim it asserts, so grouping it at + // all would be a guess published as fact. + expect(() => + buildReference([refusalInput("vale", [".tests/README.md"])]) + ).toThrow(/is not under/); + }); + + it("rejects a fixture path that is the bucket directory itself", () => { + // A bucket is where cases live, not a case. Accepting it would emit a case + // with an empty name whose path collides with the bucket. + expect(() => + buildReference([refusalInput("vale", [".tests/pass"])]) + ).toThrow(/is the bucket directory itself/); + }); +}); diff --git a/packages/cli/test/runtime-fixtures.test.ts b/packages/cli/test/runtime-fixtures.test.ts index 9edda9a3..03adcb4c 100644 --- a/packages/cli/test/runtime-fixtures.test.ts +++ b/packages/cli/test/runtime-fixtures.test.ts @@ -5,6 +5,7 @@ import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { readRuntimeFixtures } from "../src/rules/runtime/fixtures"; +import { ENGINE_LAYOUTS } from "../src/rules/layout"; /** * Reading a runtime rule's fixture cases. @@ -98,6 +99,12 @@ describe("reading runtime fixture cases", () => { // Skipping it would leave an author with a fixture that never ran and that // nothing mentioned, which is the failure this tier keeps producing. await expect(readRuntimeFixtures(cwd, RULE)).rejects.toThrow(/loose\.ts/); + + // This rejection IS the declared layout, and the conformance corpus + // publishes that declaration as fact. The two are asserted together so a + // reader changing either meets the other, rather than leaving the corpus + // telling consumers a case is a directory while the reader accepts files. + expect(ENGINE_LAYOUTS.runtime.fixtureLayout).toBe("case-directories"); }); it("does not read an unreadable bucket as an empty one", async () => { diff --git a/packages/cli/test/vale-verify.test.ts b/packages/cli/test/vale-verify.test.ts index b695ee7d..d9e9be30 100644 --- a/packages/cli/test/vale-verify.test.ts +++ b/packages/cli/test/vale-verify.test.ts @@ -11,6 +11,7 @@ import { join } from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; import { findValeBinary } from "../src/rules/vale/binary"; +import { ENGINE_LAYOUTS } from "../src/rules/layout"; import { buildIsolatingConfig, discoverValeRuleTests, @@ -218,6 +219,12 @@ describe("fixture buckets are flat", () => { await expect(verifyValeRule(cwd, "no-simply")).rejects.toThrow( /fixture buckets are flat/i ); + + // This rejection IS the declared layout, and the conformance corpus + // publishes that declaration as fact. Asserted here so a reader changing + // either meets the other, rather than leaving the corpus telling consumers + // a case is one document while the reader collects trees. + expect(ENGINE_LAYOUTS.vale.fixtureLayout).toBe("case-documents"); }); });