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

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

Three reporting-honesty gaps let migrator failures exit 0, so a
`wheels migrate latest && ...` CI gate proceeded as if the schema moved.
The migrate-side sibling of the #2973/#2987 seeder honesty fix.

- migrate latest|up|down: migrateTo() folds a failed up()/down() step into
  its returned message ("Error migrating to <version>.") instead of
  throwing, so the /wheels/cli bridge reports success:true and the CLI
  printed the error inside the green success block at exit 0. runMigration
  now detects that signature for the schema-mutating actions and throws.
- db reset --force: the migrate step's catch swallowed the refusal
  (ServerNotRunning) / failure with return "" (exit 0). It now rethrows,
  matching migrate latest and seed.
- migrate forget|pretend: server-side refusals (not in 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) still
  exits 0.

Two public $-prefixed helpers carry the detection logic so the CLI specs
can unit-test it; the mcpHiddenTools() structural sweep keeps them off the
MCP surface.

Refs #3081

Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>

* docs(web/guides): note migration failure exit codes in database guide

migrate latest/up/down, db reset --force, and migrate forget/pretend
now exit non-zero on failure or refusal (#3081).

Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>

* test(cli): drive the db reset refusal spec through the callerArgs path

The gap-2 spec set mod.__arguments externally, which lands in the
component's this scope; structuredArgs()'s unscoped read resolves the
variables scope in the in-server suite, so db() saw zero args, printed
usage help, and returned without throwing. Switch to the
mod.db(arg1, arg2) callerArgs form — the same mechanism DbCommandSpec's
throwing spec uses — and pin the expected Wheels.ServerNotRunning type.

Signed-off-by: Peter Amiri <peter@alurium.com>

---------

Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: Peter Amiri <peter@alurium.com>

Author: 3 people

changelog.d/3081-cli-migration-exit-codes.fixed.md

@@ -0,0 +1 @@
+- CLI: migration failures now reach the exit code. A failed `up()`/`down()` step that `migrateTo()` folds into its output (e.g. `Error migrating to <version>.`) made `wheels migrate latest|up|down` print the error inside the green success block and still exit 0; `wheels db reset --force` exited 0 when it refused on a `ServerNotRunning` check; and `wheels migrate forget|pretend` refusals (not in tracking table, matching local file exists, already applied, no matching file) printed red but exited 0. All three now exit non-zero, so a `wheels migrate latest && …` CI gate no longer proceeds as if the schema moved. Informational dry-run output (missing `<version>` / missing `--yes`) still exits 0. The migrate-side sibling of the #2973/#2987 seeder honesty fix (#3081)

cli/lucli/Module.cfc

@@ -3941,6 +3941,39 @@ component extends="modules.BaseModule" {
 
 	// ── Migration Execution ──────────────────────────
 
+	/**
+	 * True when migrator output carries the failed-step signature that
+	 * Migrator.$runMigrationStep() emits — "Error migrating to <version>."
+	 * migrateTo() concatenates that line into its returned string instead of
+	 * throwing, so the /wheels/cli bridge reports success:true and the CLI's
+	 * parseCliResponse() success->exit-code mapping never trips. Detecting the
+	 * signature lets a failed up()/down() reach the exit code — the
+	 * migrate-side sibling of the #2973/#2987 seeder honesty fix (#3081).
+	 *
+	 * Anchored on the literal label plus a numeric version so normal progress
+	 * lines ("Migrating from 0 up to N.") never match. Public ONLY so the CLI
+	 * specs can reach it (cli/CLAUDE.md "public for specs" carve-out); the
+	 * mcpHiddenTools() structural $-prefix sweep keeps it off the MCP surface.
+	 */
+	public boolean function $migrationOutputIndicatesFailure(required string output) {
+		return reFindNoCase("Error migrating(\s+to)?\s+[0-9]+\.", arguments.output) > 0;
+	}
+
+	/**
+	 * True when a /wheels/cli migration-style response should map to a non-zero
+	 * CLI exit: either an explicit success:false (forget/pretend refusals such
+	 * as "not found in the tracking table" or "matching local file exists", or
+	 * a bridge-surfaced error) OR the subtler honesty gap where the bridge
+	 * reports success:true while migrateTo() folded a failed step into the
+	 * message. Public for specs; hidden from MCP via the structural sweep (#3081).
+	 */
+	public boolean function $cliMigrationResponseFailed(required struct response) {
+		if (!(arguments.response.success ?: true)) {
+			return true;
+		}
+		return $migrationOutputIndicatesFailure(arguments.response.message ?: "");
+	}
+
 	private string function runMigration(required string action) {
 		var serverPort = $requireRunningServer(
 			hints = [
@@ -3983,6 +4016,20 @@ component extends="modules.BaseModule" {
 		// the previous code silently treated it as success. See issue #2315.
 		var result = parseCliResponse(httpResult, "Migration #action#");
 
+		// Honesty gap (#3081): migrateTo() folds a failed up()/down() step into
+		// its returned message ("Error migrating to <version>.") instead of
+		// throwing, so the bridge reports success:true and parseCliResponse()
+		// above doesn't trip. Detect that signature for the schema-mutating
+		// actions so a failed migration reaches the exit code — the migrate-side
+		// sibling of the #2973/#2987 seeder honesty fix.
+		if (mutatingAction && $migrationOutputIndicatesFailure(result.message ?: "")) {
+			out(result.message ?: "", "red");
+			throw(
+				type    = "MigrationError",
+				message = "Migration #arguments.action# failed — a migration step reported an error (see output above)."
+			);
+		}
+
 		// For `doctor`, switch the output color to yellow when the report
 		// signals unhealthy state (orphans or pending migrations). Green
 		// on an unhealthy result reads as "everything's fine" when it
@@ -4061,14 +4108,24 @@ component extends="modules.BaseModule" {
 		}
 
 		var parsed = isJSON(httpResult) ? deserializeJSON(httpResult) : {success: false, message: "Invalid response"};
-		var success = parsed.success ?: false;
 		var msg = parsed.message ?: "";
 
-		if (success) {
-			out(msg, "green");
-		} else {
-			out(msg, "red");
+		// Honesty gap (#3081): forget/pretend refusals ("not found in the
+		// tracking table", "matching local file exists", "already applied",
+		// "no matching file") come back as success:false but previously printed
+		// red and returned "" — exit 0, indistinguishable from a real mutation
+		// in a script. Throw so the refusal reaches the exit code. The
+		// informational dry-run branches above (missing <version> / missing
+		// --yes) still return "" (exit 0) because they precede the server call.
+		if ($cliMigrationResponseFailed(parsed)) {
+			out(Len(msg) ? msg : "#verb# refused.", "red");
+			throw(
+				type    = "MigrationError",
+				message = Len(msg) ? msg : "#verb# refused — no change made."
+			);
 		}
+
+		out(msg, "green");
 		return "";
 	}
 
@@ -4209,7 +4266,12 @@ component extends="modules.BaseModule" {
 			runMigration("latest");
 		} catch (any e) {
 			out("Migration failed: #e.message#", "red");
-			return "";
+			// #3081: a refusal (e.g. ServerNotRunning) or a failed migration
+			// must reach the exit code — swallowing it (return "") made
+			// `db reset --force` report success while the schema never moved.
+			// `wheels migrate latest` and `wheels seed` already exit non-zero
+			// on the same refusal; this aligns `db reset` with them.
+			rethrow;
 		}
 
 		// Step 2: Seed (unless skipped)

cli/lucli/tests/specs/commands/MigrationExitCodeSpec.cfc

@@ -0,0 +1,138 @@
+/**
+ * Migration failures must reach the CLI exit code (issue #3081).
+ *
+ * The seeder got an honesty fix in #2973/#2987 (partial failure →
+ * success=false + non-zero exit). The migrate surface had the same class of
+ * bug: a failed up()/down() printed "Error migrating to <version>." yet
+ * exited 0, `db reset --force` exited 0 on a ServerNotRunning refusal, and
+ * forget/pretend refusals exited 0 — all indistinguishable from success in
+ * a `wheels migrate latest && ...` CI gate.
+ *
+ * These specs cover the CLI-side reporting-honesty seams: the failure
+ * detection helpers and the `db reset --force` rethrow. Transaction/rollback
+ * behaviour is already correct and is not exercised here.
+ *
+ * Module instantiation mirrors DbCommandSpec/MigrateCommandSpec — the CLI
+ * test runner loads the module against a scaffolded temp project; server-
+ * dependent paths throw "No running Wheels server detected" because the
+ * temp project has no bound server, which is exactly what gap 2 relies on.
+ */
+component extends="wheels.wheelstest.system.BaseSpec" {
+
+	function beforeAll() {
+		variables.testHelper = new cli.lucli.tests.TestHelper();
+		variables.tempRoot = testHelper.scaffoldTempProject(expandPath("/"));
+
+		// Create vendor/wheels stub
+		directoryCreate(tempRoot & "/vendor/wheels", true, true);
+
+		// scaffoldTempProject() copies the repo's lucee.json (which carries a
+		// `port`) into the temp project. Strip the port so detectServerPort()
+		// can never resolve a live server for this project — the gap-2 case
+		// must deterministically take the ServerNotRunning refusal path
+		// (requireProjectConfig=true refuses the common-port fallback), and we
+		// must never accidentally POST a real `reset` at a server that happens
+		// to be bound to the configured port.
+		fileWrite(tempRoot & "/lucee.json", "{}");
+
+		variables.mod = new cli.lucli.Module(cwd = variables.tempRoot);
+	}
+
+	function afterAll() {
+		testHelper.cleanupTempProject(variables.tempRoot);
+	}
+
+	function run() {
+
+		describe("migration failure → CLI exit code (##3081)", () => {
+
+			describe("$migrationOutputIndicatesFailure — swallowed-step detection (gap 1)", () => {
+
+				it("flags a failed up() step that migrateTo() folded into its return string", () => {
+					var output = "Migrating from 0 up to 20260101000000." & chr(10)
+						& "-------- 20260101000000_create_widgets --------" & chr(10)
+						& "Error migrating to 20260101000000." & chr(10)
+						& "[SQLITE_ERROR] SQL error or missing database (no such function: NOW)";
+					expect(mod.$migrationOutputIndicatesFailure(output)).toBeTrue();
+				});
+
+				it("flags an IrreversibleMigration down() failure", () => {
+					var output = "Migrating from 20260101000000 down to 0." & chr(10)
+						& "Error migrating to 20260101000000." & chr(10)
+						& "Cannot reverse this migration (IrreversibleMigration).";
+					expect(mod.$migrationOutputIndicatesFailure(output)).toBeTrue();
+				});
+
+				it("does not flag normal successful migration output", () => {
+					var output = "Migrating from 0 up to 20260101000000." & chr(10)
+						& "-------- 20260101000000_create_widgets --------" & chr(10)
+						& "CREATE TABLE widgets (id INTEGER PRIMARY KEY)";
+					expect(mod.$migrationOutputIndicatesFailure(output)).toBeFalse();
+				});
+
+				it("does not flag the no-op 'No pending migrations' message", () => {
+					expect(
+						mod.$migrationOutputIndicatesFailure("No pending migrations. Database is at version 20260101000000.")
+					).toBeFalse();
+				});
+
+			});
+
+			describe("$cliMigrationResponseFailed — bridge response honesty", () => {
+
+				it("treats success:true carrying a failed-step message as a failure (gap 1)", () => {
+					expect(
+						mod.$cliMigrationResponseFailed({
+							success: true,
+							message: "Error migrating to 20260101000000." & chr(10) & "[SQLITE_ERROR] no such function: NOW"
+						})
+					).toBeTrue();
+				});
+
+				it("treats an explicit success:false refusal as a failure (gap 3)", () => {
+					expect(
+						mod.$cliMigrationResponseFailed({
+							success: false,
+							message: "Version 20260101000000 was not found in the tracking table."
+						})
+					).toBeTrue();
+				});
+
+				it("treats a clean success response as not-failed", () => {
+					expect(
+						mod.$cliMigrationResponseFailed({
+							success: true,
+							message: "Migrating from 0 up to 20260101000000." & chr(10) & "CREATE TABLE widgets (...)"
+						})
+					).toBeFalse();
+				});
+
+			});
+
+			describe("db reset --force refusal honesty (gap 2)", () => {
+
+				it("rethrows the ServerNotRunning refusal instead of swallowing it (exit non-zero)", () => {
+					// No server is bound to the scaffolded temp project, so
+					// runMigration("latest") -> $requireRunningServer throws.
+					// The pre-fix dbReset catch printed red and returned ""
+					// (exit 0); the fix rethrows so the refusal reaches $?.
+					//
+					// arg1=/arg2= exercises the callerArgs path through
+					// structuredArgs() — the same mechanism DbCommandSpec's
+					// throwing spec uses. The instance-level `__arguments`
+					// stash is NOT reliable here: set externally it lands in
+					// the component's `this` scope, but structuredArgs()'s
+					// unscoped read resolves the variables scope in the
+					// in-server suite, so db() would see zero args and print
+					// usage help instead of dispatching reset.
+					expect(() => mod.db(arg1 = "reset", arg2 = "--force"))
+						.toThrow(type = "Wheels.ServerNotRunning");
+				});
+
+			});
+
+		});
+
+	}
+
+}

web/sites/guides/src/content/docs/v4-0-0/command-line-tools/wheels-commands/database.mdx

@@ -57,11 +57,11 @@ Single-command health report for the migration tracking table. Lists orphan trac
 
 ##### `forget`
 
-Deletes a stale tracking row: `wheels migrate forget <version> --yes`. Dry-run by default; `--yes` is required to mutate. Refuses if a matching local migration file exists, or if the version isn't in the tracking table.
+Deletes a stale tracking row: `wheels migrate forget <version> --yes`. Dry-run by default; `--yes` is required to mutate. Refuses if a matching local migration file exists, or if the version isn't in the tracking table. Refusals exit non-zero.
 
 ##### `pretend`
 
-Records a version as applied without running its `up()`: `wheels migrate pretend <version> --yes`. Dry-run by default; `--yes` is required to mutate. Refuses if the version is already applied or has no matching file.
+Records a version as applied without running its `up()`: `wheels migrate pretend <version> --yes`. Dry-run by default; `--yes` is required to mutate. Refuses if the version is already applied or has no matching file. Refusals exit non-zero.
 
 ##### `rename-system-tables`
 
@@ -73,7 +73,7 @@ Migrates legacy framework bookkeeping tables to their current `wheels_`-prefixed
 wheels migrate latest
 ```
 
-Prints `Running migration: latest...` in cyan, dispatches the run to the server, and reports completion with a message like `Migration latest completed.` in green. Migration errors are caught and reported as `Migration failed: <message>`.
+Prints `Running migration: latest...` in cyan, dispatches the run to the server, and reports completion with a message like `Migration latest completed.` in green. Migration errors are caught and reported as `Migration failed: <message>` — and the command exits non-zero, so a `wheels migrate latest && ...` CI gate does not proceed as if the schema moved.
 
 <Aside type="note" title="Generating migration files">
 `wheels migrate` only applies migrations — it does not create them. To stamp out a new migration file, use `wheels generate migration` (blank) or `wheels generate property` (add-column). See [Code Generation](../code-generation/).
@@ -142,7 +142,7 @@ Runs pending migrations and reseeds the database in one shot. Destructive in the
 wheels db reset --force
 ```
 
-Prints `Running migrations...`, dispatches `wheels migrate latest`, then prints `Running seeds...` and dispatches `wheels seed`. Ends with `Database reset complete.` in green.
+Prints `Running migrations...`, dispatches `wheels migrate latest`, then prints `Running seeds...` and dispatches `wheels seed`. Ends with `Database reset complete.` in green. A migration or seed failure exits non-zero so the error is visible in scripts and CI.
 
 ##### `status`