Skip to content

Commit 07afcbc

Browse files
committed
test(check): build the cross-engine collision after migration 9 runs
`check --rule <id>` selects every engine holding the id, and #385's test proved it by seeding `vale/no-eval` beside the fixture's `sg/no-eval`. Migration 9 now renames exactly that state, and `runCli` migrates on every invocation through `migrateFixture`, so the collision was renamed to `no-eval-sg`/`no-eval-vale` before `check` ever saw it: `--rule no-eval` exited `RULE_NOT_FOUND` and the test died reading `.map` of an undefined `results`. The migration invalidated the setup, not the behaviour. An id held by two engines still selects both, and a project can still reach that state — by hand, or by a merge landing a same-id rule under another engine — which is the case the new per-rule check in `verify` exists to catch. So the fixture is migrated first and the second engine's copy seeded after, with a comment naming migration 9 so the setup is not "simplified" back. Also names the consequence in the changeset: an id passed to `--rule` yesterday may not exist today, and that failure is `RULE_NOT_FOUND` rather than a quiet zero findings.
1 parent 06d32f7 commit 07afcbc

2 files changed

Lines changed: 20 additions & 1 deletion

File tree

‎.changeset/rule-id-uniqueness.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,6 @@ Two rules can no longer share an id across engines. `verify` fails a rule whose
1010

1111
**A `runtime` rule is never renamed** and keeps the bare id, so a collision between `runtime` and another engine moves only the other one. Runtime rules are the signed and blessed tier, and this keeps the upgrade clear of that machinery. Nothing is left colliding either way, because one engine can only hold one directory per id.
1212

13-
Everything the rename touches is inside the rule's own directory: the directory name, the rule file, its `id:` field, an sg rule's `.tests/` fixtures and their `id:` fields, and a Vale rule's `.vale.ini` breadcrumb and both segments of its `<id>.<id>` assignment. Every rename is printed — old path, new path, and each file rewritten — as is any runtime rule that kept its id, so you can see exactly what moved before committing it. Update any CI config, baseline file or suppression comment that names an old id.
13+
Everything the rename touches is inside the rule's own directory: the directory name, the rule file, its `id:` field, an sg rule's `.tests/` fixtures and their `id:` fields, and a Vale rule's `.vale.ini` breadcrumb and both segments of its `<id>.<id>` assignment. Every rename is printed — old path, new path, and each file rewritten — as is any runtime rule that kept its id, so you can see exactly what moved before committing it. Update any CI config, baseline file or suppression comment that names an old id — including `check --rule <id>`, which errors with `RULE_NOT_FOUND` rather than reporting zero findings when the id it names has been renamed out from under it.
1414

1515
`.taskless/rule-metadata/<id>.yml` is left where it is rather than following either rule, since a symmetric rename gives it no owner. In practice there is nothing there: this CLI has never written a sidecar, because the service does not return the metadata block they are written from.

‎packages/cli/test/check-rule-filter.test.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,25 @@ describe("check --rule", () => {
319319
// `rules delete` refuses an ambiguous id because deleting the wrong rule
320320
// is irreversible. Measuring is not, and an unfiltered `check` would have
321321
// run both, so both run and `source` tells them apart.
322+
323+
// MIGRATE FIRST, THEN BUILD THE COLLISION. Migration 9
324+
// (`0009-unique-rule-ids`) renames every id held by more than one engine,
325+
// so a collision seeded into the fixture before it runs is renamed to
326+
// `no-eval-sg`/`no-eval-vale` and `--rule no-eval` then names no rule at
327+
// all — `check` exits `RULE_NOT_FOUND` and this test dies in `triples()`
328+
// reading `.map` of an undefined `results`. `runCli` migrates on every
329+
// invocation via `migrateFixture`, so the migration has to happen here,
330+
// before the second engine's copy exists.
331+
//
332+
// That is not a trick to keep the old wording alive: it is the only way a
333+
// project can hold this state now. The migration clears the collisions
334+
// already on disk, and what remains is one created AFTER it ran — by
335+
// hand, or by a merge landing a same-id rule under another engine — which
336+
// is exactly the case the per-rule check in `verify` exists to catch.
337+
// `check` still has to measure both, and `rules/rule-filter.ts` says so.
338+
// Do not "simplify" this back into the `beforeEach`.
339+
await migrateFixture(["-d", project]);
340+
322341
const valeRule = join(project, ".taskless/rules/vale/no-eval");
323342
await mkdir(valeRule, { recursive: true });
324343
await writeFile(

0 commit comments

Comments
 (0)