fix(cli): migration failures reach the CLI exit code

[wheels-bot]

Summary

Three reporting-honesty gaps let migrator failures exit 0, so a wheels migrate latest && … CI gate (or any script chaining on the exit code) proceeded as if the schema had moved. This is the migrate-side sibling of the seeder honesty fix in #2973/#2987, and the fix is scoped entirely to the CLI's interpretation of the /wheels/cli bridge response — the transaction/rollback behaviour was already correct and is untouched.

1. migrate latest|up|down — Migrator.migrateTo() folds a failed up()/down() step into its returned string (Error migrating to <version>.) instead of throwing, so the bridge reports success:true and parseCliResponse()'s success→exit-code mapping never trips. runMigration() now detects that signature for the schema-mutating actions via a new helper and throws MigrationError, so the failure reaches $?.
2. db reset --force — the migrate step's catch swallowed the refusal (e.g. ServerNotRunning) / failure with return "" (exit 0). It now rethrows, matching migrate latest and seed, which already exit non-zero on the same refusal.
3. migrate forget|pretend — server-side refusals (not found in the tracking table, matching local file exists, already applied, no matching file) came back success:false but printed red and returned "" (exit 0). They now throw. Informational dry-run output (missing <version> / missing --yes, which precede the server call) still exits 0.

The detection logic lives in two public $-prefixed helpers ($migrationOutputIndicatesFailure, $cliMigrationResponseFailed) so the CLI specs can unit-test it directly; the mcpHiddenTools() structural $-prefix sweep keeps them off the MCP tool surface.

Why CLI-side and not framework-side

The triage offered two paths: flip the bridge response success=false framework-side (preferred), or detect the failure CLI-side. I took the CLI-side path deliberately — it keeps the change out of the migration-execution path (vendor/wheels/migrator/**), which the bot's safety net flags as sensitive, while still satisfying every acceptance criterion. The bridge's success:true-with-error-in-message behaviour is left intact; the CLI now reads it honestly. A reviewer who prefers the framework-side flip can layer it on top without conflicting with this change.

Related Issue

Fixes #3081

Type of Change

• Bug fix
• New feature
• Enhancement to existing feature
• Documentation update
• Refactoring

Feature Completeness Checklist

• DCO sign-off — commit carries Signed-off-by: (git commit -s)
• Tests — cli/lucli/tests/specs/commands/MigrationExitCodeSpec.cfc (failing → passing): covers the swallowed-step detection (gap 1), the explicit-refusal mapping (gap 3), and db reset --force rethrow (gap 2)
• Framework Docs — left for bot-update-docs.yml
• AI Reference Docs — left for bot-update-docs.yml
• CLAUDE.md — left for bot-update-docs.yml
• Changelog fragment — changelog.d/3081-cli-migration-exit-codes.fixed.md
• Test runner passes — see note below

Test Plan

New spec asserts:

• $migrationOutputIndicatesFailure() → true for a swallowed failed up()/down() step (and the IrreversibleMigration shape), false for normal progress and the No pending migrations no-op.
• $cliMigrationResponseFailed() → true for success:true carrying a failed-step message (gap 1) and for an explicit success:false refusal (gap 3); false for a clean success.
• wheels db reset --force against a project with no bound server rethrows the ServerNotRunning refusal (exit non-zero) instead of swallowing it (gap 2). The spec neutralises the temp project's lucee.json port so this path is deterministic and never POSTs a real reset.

The spec fails before the patch by construction — the two helpers don't exist (gap 1/3) and dbReset's catch returns "" rather than rethrowing (gap 2) — and passes after.

Local run note: this PR's authoring sandbox could not execute the server-backed CLI suite (bash tools/test-cli-local.sh needs to bind a port, which the sandbox blocks; tools/test-local.sh runs only the core framework suite and additionally needs the wheels binary, which isn't installed here). The authoritative run is CI on this PR (/wheels/cli/tests). Reviewers running locally: bash tools/test-cli-local.sh (optionally WHEELS_CLI_TEST_STRICT=1).

[wheels-bot]

Wheels Bot — Docs updated

Added a doc commit to this PR:

• web/sites/guides/src/content/docs/v4-0-0/command-line-tools/wheels-commands/database.mdx — noted that migrate latest/up/down exits non-zero on failure, forget/pretend refusals exit non-zero, and db reset --force failures exit non-zero (cli: migration failures don't reach the CLI exit code — migrate latest/up/down exit 0 on a failed migration, db reset --force exits 0 on refusal, forget/pretend refusals exit 0 (migrate-side sibling of #2973) #3081 exit-code honesty fix)

[wheels-bot]

Wheels Bot — Reviewer

TL;DR: This PR closes three real exit-code honesty gaps in the migrate CLI surface (migrate latest|up|down swallowed-step detection, db reset --force rethrow, migrate forget|pretend refusal throws), with a failing-first spec, a changelog fragment, accurate guide updates, and a clean commit. I verified the failure-signature regex against both Migrator error labels, the MCP hiding sweep, and every throw-propagation path — no blocking findings. Verdict: comment — one test-isolation nit worth fixing (a one-liner), nothing that should hold the merge.

Correctness

All verified clean — citing the load-bearing checks so the approval trail is concrete:

• The regex covers both Migrator error labels. $migrationOutputIndicatesFailure (cli/lucli/Module.cfc:3811) uses Error migrating(\s+to)?\s+[0-9]+\.. The Migrator composes the line as #errorLabel# #version#. (vendor/wheels/Migrator.cfc:387) with errorLabel defaulting to "Error migrating to" (Migrator.cfc:351) and migrateIndividual() passing "Error migrating" (Migrator.cfc:198) — the optional (\s+to)? handles both, and the numeric anchor keeps progress lines (Migrating from 0 up to N.) from matching.
• The detection is correctly gated to mutating actions (mutatingAction && at Module.cfc:3877), so info/doctor output can never trip it.
• Every new throw actually reaches the exit code. migrate latest|up|down catches MigrationError and rethrows (Module.cfc:602-607); forget/pretend propagate uncaught (Module.cfc:615-618); dbReset rethrows the migrate step (Module.cfc:4126) and the seed step (runSeed at Module.cfc:4132) was already uncaught.
• One deliberate default-flip I checked and accept: the old runForgetOrPretend read parsed.success ?: false (missing key → red), while $cliMigrationResponseFailed reads success ?: true (Module.cfc:3823). This aligns with parseCliResponse's documented convention ("Returns the parsed struct on success: true (or no success key)", Module.cfc:6507), and the non-JSON fallback struct sets success: false explicitly (Module.cfc:3962), so no real producer hits the changed branch. No action needed.
• No existing spec asserts the old swallow behavior — DbCommandSpec's reset tests never pass --force (they short-circuit at the dry-run branch), and MigrateCommandSpec doesn't touch forget/pretend/reset.

One non-blocking observation, not a finding: signature-sniffing the message string means a successful migration whose echoed output happened to contain the literal error migrating to <digits>. (the match is reFindNoCase) would false-positive. That's contrived, and the PR body already documents the framework-side success=false flip as the layerable robust alternative — fine as-is.

Conventions

• The two helpers being public + $-prefixed is the documented cli/CLAUDE.md "public for specs" carve-out, and the mcpHiddenTools() structural sweep (Module.cfc:199-210) auto-hides them from the MCP tools/list — confirmed McpHiddenToolsSpec asserts this structurally (source-text checks), so it won't break on the additions.
• The regex avoids the (.+)-matches-newlines trap that cli/CLAUDE.md flags for reFind — \s+/[0-9]+ only.

Tests

MigrationExitCodeSpec.cfc mirrors the DbCommandSpec setup (same base class, TestHelper.scaffoldTempProject, mod.__arguments prior art from DeployCommandSpec), covers all three gaps, and fails before the patch by construction. One isolation gap:

• cli/lucli/tests/specs/commands/MigrationExitCodeSpec.cfc:37 — the lucee.json neutralization is incomplete; the .env channel is still open. The spec writes fileWrite(tempRoot & "/lucee.json", "{}") so detectServerPort() can't resolve the live CI test server (repo lucee.json carries "port": 8080, the very port tools/test-cli-local.sh binds). But TestHelper.scaffoldTempProject() also copies the repo-root .env into the temp project (cli/lucli/tests/TestHelper.cfc:85-91), and detectServerPort() consults it for PORT\s*=\s*(\d+) before refusing the common-port fallback (cli/lucli/Module.cfc:6250-6259). The committed .env has no PORT= line, so the gap-2 test is deterministic in CI today — but PORT in .env is an explicitly documented local config (the command's own hint text: "Set 'port' in lucee.json (or PORT in .env)"). A developer with that line and a running server gets a false test failure and a real makeBridgePost of migrateToLatest at their dev server — exactly what the spec's comment says it must never do ("we must never accidentally POST a real reset"). Fix is one line next to the existing neutralization:

  fileWrite(tempRoot & "/lucee.json", "{}");
  fileWrite(tempRoot & "/.env", "");

Docs

Accurate and verified against the code: database.mdx's "Refusals exit non-zero" matches the uncaught forget/pretend throws, "the command exits non-zero" matches the migrate() rethrow, and the db reset line is correct for both migrate (rethrow) and seed (uncaught) failures. Changelog fragment is correctly placed (changelog.d/3081-cli-migration-exit-codes.fixed.md, .fixed.md type) — no direct CHANGELOG.md edit. The PR checklist is honest about the sandbox test-run limitation.

Commits

Single commit 8f7cd0605 — fix(cli): … is a valid type/scope, subject under 100 chars, body explains the why, Signed-off-by matches the author email. Clean.

---

The CI run on this PR is the authoritative test execution (the author's sandbox note checks out — the spec needs a bound server). The .env nit above is worth folding in before or shortly after merge, but it doesn't block: the committed .env keeps CI deterministic.

[wheels-bot]

Wheels Bot — Reviewer

TL;DR: Re-review at the new head, whose only delta since the last review is the docs(web/guides) commit updating database.mdx. I independently re-verified the full diff rather than diffing against the prior review: the three exit-code honesty fixes are correct and fully propagated, the docs claims all match the code, and both commits are clean. Verdict: comment — the one test-isolation nit from the previous round (the spec's .env channel) is still open; it remains non-blocking because CI stays deterministic with the committed .env.

Correctness

All verified clean at this head — the load-bearing checks, re-done from the code rather than inherited from the prior review:

• Failure-signature regex covers both Migrator error labels. $migrationOutputIndicatesFailure (cli/lucli/Module.cfc:3811) uses Error migrating(\s+to)?\s+[0-9]+\.. The Migrator composes the line as #errorLabel# #version#. (vendor/wheels/Migrator.cfc:387), with errorLabel defaulting to "Error migrating to" (Migrator.cfc:351) and migrateIndividual() passing "Error migrating" (Migrator.cfc:198). The numeric anchor keeps progress lines (Migrating from 0 up to N.) and the No pending migrations... no-op from matching.
• Detection is gated to schema-mutating actions (mutatingAction && at Module.cfc:3877), so info/doctor output can never trip it.
• Every new throw reaches the exit code. migrate latest|up|down catches MigrationError and rethrows (Module.cfc:601-607); forget/pretend throws propagate uncaught from migrate() (Module.cfc:616-618); dbReset rethrows the migrate step (Module.cfc:4126) and the seed step (runSeed at Module.cfc:4132) was already uncaught. $requireRunningServer throws typed Wheels.ServerNotRunning (Module.cfc:6308), which Picocli maps to non-zero per its own docblock.
• The success ?: true default in $cliMigrationResponseFailed (Module.cfc:3823) flips the old parsed.success ?: false read, but it aligns with parseCliResponse's documented convention (no success key → success), and the non-JSON fallback sets success: false explicitly (Module.cfc:3962) — no real producer hits the changed branch, and even for a hypothetical missing-key producer the exit code is unchanged (0 before, 0 after; only the color differs). No action needed.

Cross-engine

The CLI runs on the bundled Lucee only; the one applicable trap is the (.+)-matches-newlines reFind gotcha from cli/CLAUDE.md, which the regex avoids (\s+/[0-9]+ only). The two new helpers being public + $-prefixed is the documented "public for specs" carve-out, and the mcpHiddenTools() structural sweep (Module.cfc:199-217) auto-hides them from the MCP tools/list.

Tests

MigrationExitCodeSpec.cfc mirrors the DbCommandSpec/MigrateCommandSpec setup exactly (same base class, scaffoldTempProject, vendor/wheels stub) and mod.__arguments follows DeployCommandSpec prior art. It fails before the patch by construction (the helpers don't exist; the old dbReset catch returned ""). One carried-over finding:

• cli/lucli/tests/specs/commands/MigrationExitCodeSpec.cfc:37 — the lucee.json neutralization is still incomplete; the .env channel remains open. Unchanged since the previous review on 8f7cd06. TestHelper.scaffoldTempProject() copies the repo-root .env into the temp project (cli/lucli/tests/TestHelper.cfc:85-91), and detectServerPort() consults it for PORT\s*=\s*(\d+) (cli/lucli/Module.cfc:6250-6259) before the requireProjectConfig refusal (Module.cfc:6265). The committed .env has no PORT= line, so CI is deterministic — but a developer with that documented local config ("Set 'port' in lucee.json (or PORT in .env)" is the command's own hint text) and a running server gets a false test failure and a real makeBridgePost of migrateToLatest at their dev server, which the spec's own comment says it must never do. Fix is still one line next to the existing neutralization:

  fileWrite(tempRoot & "/lucee.json", "{}");
  fileWrite(tempRoot & "/.env", "");

Docs

The new commit's database.mdx claims are all accurate against the code: "Refusals exit non-zero" for forget/pretend matches the uncaught throw at Module.cfc:3974; "the command exits non-zero, so a wheels migrate latest && ... CI gate does not proceed" matches the rethrow at Module.cfc:606; the db reset line is correct for both the migrate rethrow (Module.cfc:4126) and the already-uncaught seed step. Changelog fragment is correctly placed (changelog.d/3081-cli-migration-exit-codes.fixed.md, .fixed.md type); no direct CHANGELOG.md edit.

Commits

Two commits: 8f7cd0605 fix(cli): migration failures reach the CLI exit code and 6313e4349 docs(web/guides): note migration failure exit codes in database guide. Both use valid types, subjects under 100 chars, bodies explaining the why, and Signed-off-by matching the author email. Clean.

---

The CI run on this PR remains the authoritative test execution (the author's sandbox limitation note checks out). The .env one-liner is the only thing worth folding in — non-blocking, since the committed .env keeps CI deterministic.

[wheels-bot]

Wheels Bot — Reviewer

TL;DR: Third pass at head 8934555, whose only delta since the last review is the test(cli) commit reworking the gap-2 spec to drive db reset --force through the callerArgs path instead of the unreliable __arguments stash. I re-verified the full diff independently (not just the delta): the three exit-code honesty fixes are correct and fully propagated, the new spec mechanism is sound and matches DbCommandSpec prior art, and all three commits are clean. Verdict: comment — the .env test-isolation nit from the previous rounds remains open and remains non-blocking.

Correctness

All clean at this head — the load-bearing checks, re-done from the code:

• The delta commit fixes a real latent spec failure. The old form (mod.__arguments = ["reset", "--force"]; expect(() => mod.db()).toThrow()) set __arguments in the component's this scope, which structuredArgs()'s unscoped read (var raw = __arguments ?: [], cli/lucli/Module.cfc:110) never resolves — db() would have seen zero args, printed usage help, returned "", and the toThrow() expectation would have failed in the in-server suite. The new form is correct end-to-end: mod.db(arg1 = "reset", arg2 = "--force") hits the callerArgs branch (Module.cfc:112-114), toArgv() emits ["reset", "--force"] via numeric-sorted positional indices (services/ArgSpec.cfc:156-158), db() dispatches dbReset (Module.cfc:2818-2819), --force flips the guard (Module.cfc:4105), and runMigration("latest") → $requireRunningServer(requireProjectConfig = true) (Module.cfc:3830-3836) throws typed Wheels.ServerNotRunning (Module.cfc:6308-6313) because detectServerPort finds no port: the spec's lucee.json is {}, the copied repo .env has no PORT= match for the PORT\s*=\s*(\d+) probe (Module.cfc:6254), and requireProjectConfig=true refuses the common-port fallback (Module.cfc:6265-6267). The pinned throw type is exact, and the call form is the same one DbCommandSpec.cfc:33 already uses (expect(() => mod.db(arg1 = "invalid")).toThrow(type = "Wheels.InvalidArguments")).
• Failure-signature regex covers both Migrator error labels. $migrationOutputIndicatesFailure (Module.cfc:3811) uses Error migrating(\s+to)?\s+[0-9]+\.; $runMigrationStep composes the line as #arguments.errorLabel# #arguments.migration.version#. (vendor/wheels/Migrator.cfc:387) with the label defaulting to "Error migrating to" (Migrator.cfc:351) and migrateIndividual() passing "Error migrating" (Migrator.cfc:198). The numeric anchor keeps Migrating from 0 up to N. progress lines and the No pending migrations... no-op from matching, and the check is gated to mutating actions (mutatingAction &&, Module.cfc:3877) so info/doctor can never trip it.
• Every new throw reaches the exit code. migrate latest|up|down|info and doctor catch MigrationError and rethrow (Module.cfc:601-614); forget/pretend throws propagate uncaught from migrate() (Module.cfc:615-618) with the informational dry-run branches (missing <version> / missing --yes) still returning "" before the server call (Module.cfc:3920-3932); dbReset rethrows the migrate step (Module.cfc:4126) and the seed step (runSeed, Module.cfc:4132) was already uncaught.

Conventions

• The two helpers being public + $-prefixed is the documented cli/CLAUDE.md "public for specs" carve-out, and the mcpHiddenTools() structural sweep (Module.cfc:199-217) auto-discovers every $-prefixed public via getMetaData(this).functions, so neither helper can leak into the MCP tools/list.
• Spec base class (wheels.wheelstest.system.BaseSpec) and scaffold setup mirror DbCommandSpec/MigrateCommandSpec exactly.

Tests

One open nit, carried from the previous two rounds and still a one-liner:

• MigrationExitCodeSpec.cfc:36 neutralizes lucee.json but not the copied .env. scaffoldTempProject copies the repo root's .env into the temp project (cli/lucli/tests/TestHelper.cfc:85-91), and detectServerPort probes it for PORT= right after lucee.json (Module.cfc:6250-6259). The committed .env has no PORT= line, so CI is deterministic — but a developer whose local repo-root .env gains PORT=8080 (the port tools/test-cli-local.sh binds) would have the gap-2 spec resolve the live test server and POST a real migrateToLatest at it, which is exactly the accidental-reset hazard the lucee.json neutralization comment warns about. Suggested fix, same shape as the existing line: add fileWrite(tempRoot & "/.env", ""); next to the lucee.json write in beforeAll(). Non-blocking.

Docs

database.mdx claims all match the code at this head: migrate latest errors exit non-zero (Module.cfc:601-607), forget/pretend refusals exit non-zero (Module.cfc:3972-3978), and db reset migration/seed failures exit non-zero (Module.cfc:4126, 4132). Changelog fragment changelog.d/3081-cli-migration-exit-codes.fixed.md is present, correctly named, and accurate.

Commits

All three conform to commitlint.config.js: fix(cli): migration failures reach the CLI exit code, docs(web/guides): note migration failure exit codes in database guide, test(cli): drive the db reset refusal spec through the callerArgs path — valid types, headers under 100 chars, DCO sign-offs present. The test(cli) body explains the why (the this-scope vs unscoped-read mismatch) rather than restating the diff — exactly what the convention asks for.