fix(migrate): report a failing migration once, naming the migration

[thecodedrift]

runMigrations logged a failing migration and then rethrew the original error, so whoever owned the surface printed the same text a second time — once with the migration number, once without. The longer and more useful the refusal, the worse it read.

The issue's motivating case (migration 9's rule-id collision) no longer refuses; #388 made it rename in place. The bug is still reachable through migration 4's SCAFFOLD_CONFLICT refusal, and through any migration that throws at all, so this is fixed as generic hygiene rather than for one migration.

Before

A .taskless/ at schema version 3 with a file where an engine directory belongs, then init:

Migrating .taskless/ from schema version 3 to 9...
Migration 4 failed: Cannot partition .taskless/ by engine: .taskless/sg is a file, but must be a directory. Move or delete it, then run the command again.
Error: Cannot partition .taskless/ by engine: .taskless/sg is a file, but must be a directory. Move or delete it, then run the command again.
exit=1

Not specific to coded errors — a file at .taskless/rules reaches a bare mkdir and doubles the same way:

Migrating .taskless/ from schema version 3 to 9...
Migration 4 failed: EEXIST: file already exists, mkdir '…/.taskless/sg/rules'
EEXIST: file already exists, mkdir '…/.taskless/sg/rules'
exit=1

After

The prefix rides on the rethrown error, so there is one string and one printer:

Migrating .taskless/ from schema version 3 to 9...
Error: Migration 4 failed: Cannot partition .taskless/ by engine: .taskless/sg is a file, but must be a directory. Move or delete it, then run the command again.
exit=1

Migrating .taskless/ from schema version 3 to 9...
Migration 4 failed: EEXIST: file already exists, mkdir '…/.taskless/sg/rules'
exit=1

The migration number is kept on both branches, including the unexpected fault. Losing it on precisely the errors we understand least is the wrong trade, and a migration's own message names paths and never itself — nothing else says which migration refused.

A CLIError is rewrapped with its original code (SCAFFOLD_CONFLICT) and reported flag, because code is what telemetry attributes on and re-coding would flatten a deliberate refusal into the same bucket as a crash. Anything else stays a plain Error with the original as cause, so it still classifies as INTERNAL_ERROR exactly as before.

Second user-visible fix: init --json now emits an error envelope

Folded in because the issue's own requirement — the migration number reaching a consumer — is unsatisfiable without it. When a migration threw, init --json wrote nothing at all to stdout and put prose on stderr, leaving a machine consumer with only the exit code. Its sibling update has carried makeErrorEnvelope all along; this is the same shape, not a second one.

$ taskless init --json    # stdout, previously empty
{"ok":false,"code":"SCAFFOLD_CONFLICT","message":"Migration 4 failed: Cannot partition .taskless/ by engine: .taskless/sg is a file, but must be a directory. Move or delete it, then run the command again."}
exit=1

check --json and verify --json never reach a failing migration (requireCurrentSchema walls them first with SCAFFOLD_MIGRATION_REQUIRED), so init and the wizard were the only surfaces where this was visible. The wizard already deduped via cancel(error.message) and now shows the migration number there too.

What is deliberately unchanged

• The manifest stamp in the same catch (write at last successful version so completed migrations do not re-run) is byte-identical. The diff there is 4 deleted lines above and below it and nothing else.
• Both branches end in an unconditional throw. The failure mode opposite to doubling is swallowing, which would exit 0 and let init report success over a half-migrated tree.
• No spec change. No OpenSpec requirement describes migration-failure output, and this is a presentation fix rather than a new contract; adding an ADDED requirement to pin one line of stderr formatting would put a spec where a test belongs.

Tests

New packages/cli/test/migration-failure.test.ts, spawning the built CLI. It asserts exit code 1, then that Cannot partition / EEXIST occurs exactly once and that the occurrence names Migration 4, plus the --json envelope for both the coded and uncoded cases. It spawns rather than unit-testing runMigrations because the bug lives in the seam between the two layers, and because only a real process has the exit code that guards against swallowing.

pnpm build, pnpm typecheck, pnpm lint, pnpm test (103 files / 1702 tests) all pass.

Fixes #389

[thecodedrift]

@claude /review

[claude]

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

---

Review complete

Review mode: incremental — read 0 prior review thread(s) before reviewing. .prior-review.json contains no reviewThreads, reviews, or (non-trigger) comments — only the @claude /review trigger comment itself. Treating this as the first review of this PR: the whole diff was assessed from scratch, nothing to mark as already-addressed.

• Read prior review data
• Fetch PR diff and metadata
• Review packages/cli/src/filesystem/migrate.ts
• Review packages/cli/src/commands/init.ts
• Review new test file
• Review changeset
• Post final review

Summary

The core fix is sound: folding the Migration N failed: prefix into the rethrown error (instead of console.error-ing it and then rethrowing the original) genuinely produces one string, one printer, on every path I traced (index.ts top-level handler, the wizard's cancel(error.message), and the new init --json envelope). code/reported are threaded through correctly for the common cases, and the new spawn-based test does cover the two motivating shapes (coded SCAFFOLD_CONFLICT, uncoded EEXIST) for both init and init --json.

I found one correctness issue worth fixing before merge, plus two lower-severity latent gaps in the same "don't lose or double the message" spirit the PR is about — posted inline:

1. packages/cli/src/filesystem/migrate.ts (lines 332-365) — The prefixed rethrow was moved to the end of the catch block, after the manifest re-stamp logic (readRawManifest/writeRawManifest, ~lines 314-330), which still has two unchanged throw points of its own. Previously the unconditional console.error at the top of the catch meant the original migration failure always reached stderr even if the re-stamp subsequently threw a different error. Now, if the re-stamp throws first (reachable whenever a later pending migration fails, e.g. migration 4 succeeds and migration 5 throws, then a concurrent-lock/permission/disk-full error hits the re-stamp), the original migration failure — message, migration number, and CLIError code — is discarded entirely; only the unrelated stamp error propagates. The new test suite doesn't exercise this because both scenarios fail on the first pending migration, where the re-stamp branch (v > version + 1) never runs.
2. packages/cli/src/filesystem/migrate.ts (lines 352-356) — reported: error.reported is carried onto the newly-prefixed error. No migration sets reported: true today, but if one ever throws via a fail()-style helper that already printed, the top-level handler would suppress the prefixed rethrow while the original (unprefixed) print already happened — losing the migration number, which is the entire point of this fix.
3. packages/cli/src/commands/init.ts (lines 88-104) — The new --json catch doesn't check error.reported before printing an envelope, so a future already-reported CLIError surfacing through runNonInteractive would double-print. Also a minor reuse note: writeJsonError(code, message) (already used this way in commands/verify.ts) could replace the inline console.log(JSON.stringify(makeErrorEnvelope(...))).

None of these affect the PR's stated motivating cases (migration 4's coded/uncoded failure on the very first pending migration), which is why the tests pass; #1 is the one I'd actually want addressed, since it reintroduces a worse failure mode (total silence) than the one being fixed, in a case one step removed from the one under test.

[thecodedrift]

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

All three inline findings are answered and resolved on their threads; no code changed. The short version: finding 1 rests on migrations being unsafe to re-run, and they are not. openspec/specs/cli-taskless-bootstrap/spec.md:11 requires each migration to be "an idempotent async function" (scenario at :36), so the version stamp is an optimization — a failed stamp means migrations re-run, which is a no-op — and a stamp failure (ENOSPC/EACCES/lock) is a distinct fault whose own error is the more actionable report.

Verified against the code for all nine, not against the docblocks, and then measured: a version-3 fixture with the full pre-0004 legacy tree migrated to 9, re-stamped to 3 and re-run so 4-9 genuinely execute over the migrated tree — four passes, each exit=0, sha256-per-file snapshot byte-identical every time; same for a version-8 fixture with a cross-engine no-eval collision, stable at no-eval-sg/no-eval-vale. Findings 2 and 3 are dormant branches (nothing sets reported: true on these paths today) and are answered on their threads.

— AI Coding Agent