fix: make migration 9 crash-resilient and stop it double-suffixing fixtures

[thecodedrift]

Migration 9 renames rule directories whose id is held by more than one engine. Two defects, found while verifying that the nine scaffold migrations are idempotent on #395. Idempotency itself holds — repeated complete runs converge, measured by tree snapshot across a double run. What did not hold is atomicity.

Defect 1 — an interrupted run could never be repaired

A collision is one directory name appearing under two or more engines (rules/id-uniqueness.ts:29,62-66), and the migration returns early when there are none (0009:112). The rename resolved the collision by renaming the rule directory first, then chased the files inside it.

So a run that died in between — Ctrl-C, a full disk, an editor holding a file open — left sg/no-eval-sg/ containing no-eval.yml with id: no-eval and fixtures still under the no-eval- prefix. That tree is collision-free, so every later run returned at :112 having found nothing to do. verify reported the rule as broken and no amount of re-running fixed it.

The shape chosen: the directory rename is the commit point

Of the three options weighed, this one is the cheapest and the only one that needs no new state on disk:

Option	Why not
Detect the half-migrated state on entry	Needs a second, independent definition of "half-migrated" that has to stay in step with every future field the rename touches.
Stage into a scratch directory and commit with one rename	Moves the crash window rather than closing it: a crash after the move to staging makes the rule invisible until a recovery path that must itself be written and tested finds it again.
Make the directory rename the last operation	The commit point already exists — rename(2) on a directory is atomic — and it is already the operation that clears the collision.

Every edit a rule needs now happens inside the rule's old directory, and the directory rename runs last. Because that rename is also what clears the collision, the existing collisions.length === 0 gate becomes a sound resume signal rather than merely a no-op check: a rule interrupted before its commit still collides and is enumerated again; one interrupted after its commit is already whole. That is why no entry-time detection is needed.

Each inner step is written to tolerate having already run — a rule file whose old name is gone but whose new name is present still gets its id: rewritten (the old early return read that as "no rule file" and left the id behind) — and every file rewrite is committed by renaming a temporary sibling, so an interrupted write cannot truncate a rule file. The temporary name is derived from the target rather than randomized, so a crash between the write and the rename leaves one predictable path the next run overwrites and consumes.

The residual windows, stated plainly

"Fully atomic on a filesystem" would be false, so here is what remains:

1. Between the first edit and the commit, the directory name disagrees with the files inside it. The tree is repairable, not correct: verify reports it, and the next migration run finishes it. The scaffold version is written only after every migration returns, so the next taskless command re-runs this by itself.
2. A symmetric collision interrupted between its two halves loses its symmetry. Where sg and vale both hold an id, a complete run moves both. If the run dies after the first commit, the half that finished keeps its suffix and the half that never started keeps the bare id — the collision it would have been renamed for is gone. The tree is collision-free and every rule is internally consistent; only the symmetry is lost. Called out in the changeset so a user can rename it if they want the pair to match.
3. Power loss between rename(2) returning and the directory entry reaching disk is not addressed, and no user-space rename dance would address it. It is a fsync question, not an ordering one.

Defect 2 — the fixture predicate matched its own output

entry.startsWith(${from}-) at 0009:276-277 accepted every name the loop produced, since the replacement is ${from}-sg. Re-running could not trigger it (the directory rename threw ENOENT first), but a fixture a human had named no-eval-sg-basic-test.yml before the migration ran came out as no-eval-sg-sg-basic-test.yml on the first run. It is also the step a resumed run repeats, so once defect 1 is fixed it stops being cosmetic.

A name already carrying the target prefix is now never renamed — it is where it belongs either way — and only its id: field follows. That half still matters: ast-grep attributes cases by the id: inside the file, so a stale one reads as a rule that shipped no cases.

Migration 9 has never shipped, so there is no migration 10

$ npm view @taskless/cli dist-tags
{ latest: '0.11.2' }                      # v0.11.2 tagged 2026-09-19

$ git log -1 --format='%h %ad' --date=short d41576f
d41576f 2026-09-22                        # the commit that added 0009

$ git merge-base --is-ancestor d41576f v0.11.2; echo $?
1                                         # not an ancestor

No user has ever run it, so there is no already-migrated tree in the wild and nothing for a repair migration to repair. Its behaviour changes in place.

Tests

The centrepiece is a crash-and-resume case, injected at a named rename destination rather than at the Nth call, so each case says which step it interrupts and stays readable as the number of writes changes. Three points: the sg rule-file rename, the sg fixture rename, and the sg directory commit. Each asserts the interrupted rule still collides, then runs the migration again and requires the directory name, rule file name, fixture names and every id: field to agree — plus the vale half, which the resuming run renames.

Measured against the pre-fix source, with only 0009-unique-rule-ids.ts reverted:

× resumes to a correct end state after crashing at the sg rule file rename
  → expected [] to have a length of 1 but got +0
× resumes to a correct end state after crashing at the sg fixture rename
  → expected [] to have a length of 1 but got +0
✓ resumes to a correct end state after crashing at the sg directory commit
× does not double-suffix a fixture already named <id>-sg-...
  → expected [ Array(2) ] to deeply equal [ Array(2) ]

Tests  3 failed | 22 passed (25)

+0 is the point: the old ordering had already cleared the collision, so there was nothing left for a re-run to find. The third point passes against both, since under the old ordering it was the first operation; it is kept as a guard on the commit-point semantics.

No existing assertion was weakened. The double-run snapshot idempotency case at rule-id-uniqueness.test.ts:339, which compares size and mtime across the whole tree, still passes unchanged.

OpenSpec

cli-taskless-bootstrap gains one ADDED requirement, "Migration 9 resumes after an interrupted run", with three scenarios. Nothing is MODIFIED: the standing requirement already says every .tests/<id>-*-test.yml ends at the new prefix with its id: rewritten, which a fixture already at that prefix satisfies without being renamed again.

Archive dry-run, diffing both lists before and after:

BEFORE reqs=11 scenarios=44
AFTER  reqs=12 scenarios=47
requirement diff: + Migration 9 resumes after an interrupted run
scenario diff:    + A fixture already at the target prefix is not suffixed twice
                  + A run interrupted after a rule's directory rename leaves that rule whole
                  + A run interrupted before a rule's directory rename is resumed

Purely additive, nothing dropped. This is a single-PR change with no follow-up, so it is the tip and the change is archived here.

Gates

pnpm build, pnpm typecheck, pnpm lint (No issues found.) and pnpm test (102 files, 1702 tests, all passing).

Changeset: one file, patch — the migration has never been in a released version, so no consumer crosses this boundary.

[thecodedrift]

@claude /review

[claude]

Claude finished @thecodedrift's task in 5m 20s —— View job

---

Review complete

Review mode: incremental — read 0 prior review thread(s) before reviewing. .prior-review.json contains no prior reviewThreads or reviews, and its only comments entry is the @claude /review trigger itself — so this is the first review of this PR; the whole diff was assessed fresh.

• Read .prior-review.json
• Fetch PR diff and metadata (gh pr diff 400, gh pr view)
• Review packages/cli/src/filesystem/migrations/0009-unique-rule-ids.ts (full file, not just the hunks)
• Review packages/cli/test/rule-id-uniqueness.test.ts
• Review OpenSpec + changeset files
• Post findings

Summary

This is a careful fix, and the reasoning in the PR body checks out against the code: making the directory rename the last operation is what turns the existing collisions.length === 0 gate into a sound resume signal, since findRuleIdCollisions keys purely off directory names (packages/cli/src/rules/id-uniqueness.ts), not file contents. I traced all three named crash points (no-eval-sg.yml, the fixture rename, and the directory commit) by hand against the new ordering in renameRule / renameRuleFile / renameSgFixtures, and each resumes to a consistent state, matching what the tests assert. The fixture double-suffix fix (checking entry.startsWith(${to}-) before ${from}-}) is also correct — since to is always ${from}-<engine>, an entry already at the target prefix necessarily also matches the old predicate, so ordering the checks that way is required, not just tidier.

One finding, posted inline on packages/cli/src/filesystem/migrations/0009-unique-rule-ids.ts:305-308: the "finished an interrupted rename" branch in renameRuleFile — the exact code path the docblock and changeset call out as fixing the old early-return bug — is never actually exercised by the new crash-resume tests. The harness injects a crash by matching the basename of a rename() destination, but that branch's own commit (rewriteIdField → writeFileAtomically → rename(temporary, toFile)) targets the same basename as the outer file-rename it follows, so the mock always intercepts the outer rename first and the "renamed but id: not yet committed" state can never be constructed through crashAtRenameTo. The same structural gap applies to renameSgFixtures's rename-then-rewrite pair. The logic reads correctly by inspection, but a regression narrowly in that window wouldn't be caught by anything in this suite.

Nothing else stood out: the atomic-write-via-temp-sibling pattern (writeFileAtomically) is sound and doesn't collide with the directory scans (.tests/ fixture listing filters on -test.yml, so *.tskl-0009.tmp is invisible to it), the OpenSpec delta is purely ADDED (no MODIFIED block, so the archive-rewrite pitfall in this repo's CLAUDE.md doesn't apply here), and the changeset's patch bump matches the repo's 0.y.z convention since migration 9 has never shipped.
· branch fix/migration-9-atomicity

[thecodedrift]

Re: @claude[bot] — "Claude finished @thecodedrift's task in 5m 20s —— Review complete…"
#400 (comment)

The one finding was correct and is fixed in e31a6ad: the crash harness matched a rename by destination basename only, so rename(fromFile, toFile) and writeFileAtomically's rename(temporary, toFile) were indistinguishable and the window between them — the state renameRuleFile's resume branch repairs — was structurally unreachable, as was renameSgFixtures' equivalent. Renames are now matchable by source as well, which separates them; two crash points were added for those windows plus a hand-built case independent of the temp-file name. Reverting either repair now fails all four cases (suite 25 -> 28, all passing).

Also rebased onto 28cc7b9; no conflicts, and the cli-taskless-bootstrap spec verified by title set against main (11 -> 12 requirements, 44 -> 47 scenarios, nothing dropped, and no drift in the other 29 spec files).

— AI Coding Agent