fix(model): bypass nested cftransaction when migrator owns the outer (#2789)

CHANGELOG.md

@@ -33,6 +33,7 @@ All historical references to "CFWheels" in this changelog have been preserved fo
 
 ### Fixed
 
+- `model().create()` / `.update()` / `.deleteAll()` inside a migration's `up()` or `down()` no longer silently roll back. The Migrator wraps every `up()` / `down()` invocation in its own outer `cftransaction`, and Wheels Model's default `transaction="commit"` opened a nested `cftransaction` whose JDBC nested-transaction semantics differ per adapter — most acutely on MSSQL, where the inner commit didn't release the row and the eventual outer commit dropped it, leaving the user with orphan FK rows referencing IDs that never persisted. `Migrator.cfc` now sets `request.$wheelsTransactionWrapper = true` around each `up()` / `down()` / `migrateIndividual()` invocation and `Model::invokeWithTransaction()` reads that signal, treating the call as `alreadyopen` and skipping the nested `cftransaction` entirely. The flag is request-scope and cleared in both the success and error paths of every migrator code path, so it never leaks past the migration's outer transaction (#2789)
 - `wheels upgrade check` underscore-references opt-in advisory no longer contradicts the mixed-convention warning when `set(useUnderscoreReferenceColumns=true)` lives in an environment override file (e.g. `config/production/settings.cfm`) rather than the root `config/settings.cfm`. The pre-check guard in `cli/lucli/Module.cfc::runUpgradeCheck()` now walks all of `config/` recursively (matching advisory #2's `scanDir: "config"` scope) instead of reading only `config/settings.cfm`, so the opt-in advisory is suppressed whenever the flag is set anywhere in the config tree. Comment-stripping still runs per file so a commented-out `set(useUnderscoreReferenceColumns=true);` cannot satisfy the guard (Anti-Pattern #14) (#2808)
 - `wheels migrate latest` no longer takes a misleading "down" branch and silently no-ops when `wheels_migrator_versions` records a version whose migration file isn't in the current checkout (shared dev DB / peer migration not yet pulled). `Migrator.migrateTo()` now diffs the tracking table against `app/migrator/migrations/` via the new `$getOrphanVersions()` helper and branches on orphan-at-top before the existing direction check — applying pending local migrations with a clear warning when all DB versions above target are orphans, emitting "Nothing to do" naming target vs current when none are pending, and warning + letting the existing down loop continue in mixed cases (orphan rows skip naturally because the loop iterates files). `wheels migrate info` also now shows orphan rows with `[?] <version> ********** NO FILE **********` (Rails-style) and a footer explaining the cause (#2780)
 - `tools/test-local.sh` silently aborted with `EXIT=1` (no `/tmp/wheels-test-server.log` written, no diagnostic printed) on every install since the `lucli` → `wheels` rebrand window closed — i.e. anyone whose `~/.lucli/express/` directory never existed. Line 81 ran `LUCEE_LIB=$(find ~/.wheels/express ~/.lucli/express -path "*/lib/ext" -type d 2>/dev/null | head -1)` under `set -euo pipefail`; `find` exits non-zero whenever any path argument doesn't exist (stderr suppressed via `2>/dev/null`, but the exit status survives), `pipefail` propagated it through `head -1`, and the assignment tripped `set -e`. The cleanup trap then fired with no server to clean up, leaving the user staring at "Starting Wheels CLI server on port 8080…" with no further output. Dropped the now-dead `~/.lucli/express` fallback (the rename landed in 3.0 and recent CLI releases extract Lucee Express to `~/.wheels/express/` only) and added `|| true` for defense in depth so a missing directory (e.g. a truly fresh install before `wheels start` has ever run) leaves `LUCEE_LIB` empty and the downstream `[ -n "$LUCEE_LIB" ]` guard skips the JDBC pre-install cleanly (#2796)

vendor/wheels/Migrator.cfc

@@ -122,14 +122,18 @@ component output="false" extends="wheels.Global"{
 								if (application[local.appKey].writeMigratorSQLFiles) {
 									$writeMigrationFile(request.$wheelsMigrationSQLFile, "");
 								}
+								// Issue #2789: skip nested cftransaction when migrator's outer one owns commit/rollback.
+								request.$wheelsTransactionWrapper = true;
 								local.migration.cfc.down();
 								local.rv = local.rv & request.$wheelsMigrationOutput;
 								$removeVersionAsMigrated(local.migration.version);
 							} catch (any e) {
 								local.rv = local.rv & "Error migrating to #local.migration.version#.#Chr(13) & Chr(10)##e.message##Chr(13) & Chr(10)##e.detail##Chr(13) & Chr(10)#";
 								transaction action="rollback";
+								StructDelete(request, "$wheelsTransactionWrapper");
 								break;
 							}
+							StructDelete(request, "$wheelsTransactionWrapper");
 							transaction action="commit";
 						}
 					}
@@ -164,14 +168,18 @@ component output="false" extends="wheels.Global"{
 								if (application[local.appKey].writeMigratorSQLFiles) {
 									$writeMigrationFile(request.$wheelsMigrationSQLFile, "");
 								}
+								// Issue #2789: skip nested cftransaction when migrator's outer one owns commit/rollback.
+								request.$wheelsTransactionWrapper = true;
 								local.migration.cfc.up();
 								local.rv = local.rv & request.$wheelsMigrationOutput;
 								$setVersionAsMigrated(local.migration.version, local.migration.name);
 							} catch (any e) {
 								local.rv = local.rv & "Error migrating to #local.migration.version#.#Chr(13) & Chr(10)##e.message##Chr(13) & Chr(10)##e.detail##Chr(13) & Chr(10)#";
 								transaction action="rollback";
+								StructDelete(request, "$wheelsTransactionWrapper");
 								break;
 							}
+							StructDelete(request, "$wheelsTransactionWrapper");
 							transaction action="commit";
 						}
 					} else if (local.migration.version > arguments.version) {
@@ -225,13 +233,17 @@ component output="false" extends="wheels.Global"{
 				if (application[local.appKey].writeMigratorSQLFiles) {
 					$writeMigrationFile(request.$wheelsMigrationSQLFile, "");
 				}
+				// Issue #2789: skip nested cftransaction when migrator's outer one owns commit/rollback.
+				request.$wheelsTransactionWrapper = true;
 				local.migration.cfc.up();
 				local.rv = local.rv & request.$wheelsMigrationOutput;
 				$setVersionAsMigrated(local.migration.version, local.migration.name);
 			} catch (any e) {
 				local.rv = local.rv & "Error migrating #local.migration.version#.#Chr(13) & Chr(10)##e.message##Chr(13) & Chr(10)##e.detail##Chr(13) & Chr(10)#";
 				transaction action="rollback";
+				StructDelete(request, "$wheelsTransactionWrapper");
 			}
+			StructDelete(request, "$wheelsTransactionWrapper");
 			transaction action="commit";
 		}
 		return local.rv;

vendor/wheels/model/transactions.cfc

@@ -36,8 +36,15 @@ component {
 			request.wheels.transactions[local.connectionArgs] = false;
 		}
 
+		// Issue #2789: skip model-level cftransaction when an outer owner (e.g. migrator) wraps this call.
+		local.outerTransactionActive = (
+			StructKeyExists(request, "$wheelsTransactionWrapper")
+			&& IsBoolean(request.$wheelsTransactionWrapper)
+			&& request.$wheelsTransactionWrapper
+		);
+
 		// If a transaction is already marked as open, change the mode to "alreadyopen", otherwise open one.
-		if (request.wheels.transactions[local.connectionArgs]) {
+		if (local.outerTransactionActive || request.wheels.transactions[local.connectionArgs]) {
 			arguments.transaction = "alreadyopen";
 			local.closeTransaction = false;
 		} else {

vendor/wheels/tests/_assets/migrator/migrations_2789/001_seed_tag_via_model_create.cfc

@@ -0,0 +1,19 @@
+component extends="wheels.migrator.Migration" hint="issue ##2789 repro — seed a tag via Model.create() inside up()" {
+
+	function up() {
+		// Issue #2789 repro: model().create() inside up() must persist after the outer transaction commits.
+		model("Tag").create(name = "issue2789_via_model_create");
+
+		// Capture the flag's value during up() so the spec can assert the migrator set it.
+		request.$issue2789FlagDuringUp = (
+			StructKeyExists(request, "$wheelsTransactionWrapper")
+			&& request.$wheelsTransactionWrapper
+		);
+	}
+
+	function down() {
+		// Issue #2789: raw queryExecute avoids nested transaction; safe across all adapters.
+		removeRecord(table = "c_o_r_e_tags", where = "name = 'issue2789_via_model_create'");
+	}
+
+}

vendor/wheels/tests/specs/migrator/MigratorOuterTransactionSpec.cfc

@@ -0,0 +1,83 @@
+component extends="wheels.WheelsTest" {
+
+	include "helperFunctions.cfm";
+
+	function beforeAll() {
+		// Required by helperFunctions.cfm engine-detection helpers (e.g. isDbCompatible's adapter probe).
+		migration = CreateObject("component", "wheels.migrator.Migration").init();
+		// Separate migrations dir avoids collision with reconciliation spec's 001/002/003 fixtures.
+		migrator = CreateObject("component", "wheels.Migrator").init(
+			migratePath = "/wheels/tests/_assets/migrator/migrations_2789/",
+			sqlPath = "/wheels/tests/_assets/migrator/sql_2789/"
+		);
+	}
+
+	function run() {
+
+		var ctx = {isCockroachDB: CreateObject("component", "wheels.migrator.Migration").init().adapter.adapterName() == "CockroachDB"};
+
+		describe("Migrator sets $wheelsTransactionWrapper while up()/down() runs (issue ##2789)", () => {
+
+			beforeEach(() => {
+				deleteMigratorVersions(2);
+				$cleanSqlDirectory();
+				try {
+					queryExecute(
+						"DELETE FROM c_o_r_e_tags WHERE name = 'issue2789_via_model_create'",
+						{},
+						{ datasource = application.wheels.dataSourceName }
+					);
+				} catch (any e) {}
+				StructDelete(request, "$issue2789FlagDuringUp");
+				StructDelete(request, "$wheelsTransactionWrapper");
+			});
+
+			afterEach(() => {
+				deleteMigratorVersions(2);
+				$cleanSqlDirectory();
+				try {
+					queryExecute(
+						"DELETE FROM c_o_r_e_tags WHERE name = 'issue2789_via_model_create'",
+						{},
+						{ datasource = application.wheels.dataSourceName }
+					);
+				} catch (any e) {}
+				StructDelete(request, "$issue2789FlagDuringUp");
+				StructDelete(request, "$wheelsTransactionWrapper");
+			});
+
+			it("model.create() inside up() persists after the outer transaction commits", () => {
+				if (ctx.isCockroachDB) return;
+				migrator.migrateTo("001");
+				var found = queryExecute(
+					"SELECT id FROM c_o_r_e_tags WHERE name = 'issue2789_via_model_create'",
+					{},
+					{ datasource = application.wheels.dataSourceName }
+				);
+				expect(found.recordCount).toBe(1);
+			});
+
+			it("sets request.$wheelsTransactionWrapper for the duration of up()", () => {
+				if (ctx.isCockroachDB) return;
+				migrator.migrateTo("001");
+				expect(request.$issue2789FlagDuringUp).toBeTrue();
+			});
+
+			it("clears request.$wheelsTransactionWrapper after up() returns", () => {
+				if (ctx.isCockroachDB) return;
+				migrator.migrateTo("001");
+				expect(StructKeyExists(request, "$wheelsTransactionWrapper")).toBeFalse();
+			});
+
+			it("clears request.$wheelsTransactionWrapper after down() returns", () => {
+				if (ctx.isCockroachDB) return;
+				migrator.migrateTo("001");
+				migrator.migrateTo("0");
+				expect(StructKeyExists(request, "$wheelsTransactionWrapper")).toBeFalse();
+			});
+
+		});
+
+	}
+
+}

vendor/wheels/tests/specs/model/OuterTransactionSignalSpec.cfc

@@ -0,0 +1,90 @@
+component extends="wheels.WheelsTest" {
+
+	function run() {
+
+		g = application.wo;
+
+		describe("model invokeWithTransaction respects outer-transaction signal (issue ##2789)", () => {
+
+			beforeEach(() => {
+				// Defensive cleanup in case a prior failing run left the flag set.
+				if (StructKeyExists(request, "$wheelsTransactionWrapper")) {
+					StructDelete(request, "$wheelsTransactionWrapper");
+				}
+				queryExecute(
+					"DELETE FROM c_o_r_e_tags WHERE name IN ('outerSig_control', 'outerSig_signal', 'outerSig_signal_create', 'outerSig_signal_update', 'outerSig_signal_delete')",
+					{},
+					{ datasource = application.wheels.dataSourceName }
+				);
+			});
+
+			afterEach(() => {
+				if (StructKeyExists(request, "$wheelsTransactionWrapper")) {
+					StructDelete(request, "$wheelsTransactionWrapper");
+				}
+				queryExecute(
+					"DELETE FROM c_o_r_e_tags WHERE name IN ('outerSig_control', 'outerSig_signal', 'outerSig_signal_create', 'outerSig_signal_update', 'outerSig_signal_delete')",
+					{},
+					{ datasource = application.wheels.dataSourceName }
+				);
+			});
+
+			it("control: model.create(transaction='rollback') without the flag rolls the row back", () => {
+				// Baseline: proves the rollback path actually fires in this environment.
+				var beforeCount = g.model("tag").count();
+				g.model("tag").create(name = "outerSig_control", transaction = "rollback");
+				var afterCount = g.model("tag").count();
+				expect(afterCount).toBe(beforeCount);
+			});
+
+			it("treats invokeWithTransaction as 'alreadyopen' when request.$wheelsTransactionWrapper is set", () => {
+				// Issue #2789: signal set → model bypasses its own wrapper; INSERT persists despite transaction='rollback'.
+				var beforeCount = g.model("tag").count();
+				request.$wheelsTransactionWrapper = true;
+				try {
+					g.model("tag").create(name = "outerSig_signal", transaction = "rollback");
+				} finally {
+					StructDelete(request, "$wheelsTransactionWrapper");
+				}
+				var afterCount = g.model("tag").count();
+				expect(afterCount).toBe(beforeCount + 1);
+			});
+
+			it("also bypasses the wrapper for update via save()", () => {
+				// Issue #2789: same bypass applies to save()'s UPDATE path.
+				var tag = g.model("tag").create(name = "outerSig_signal_update");
+				request.$wheelsTransactionWrapper = true;
+				try {
+					tag.name = "outerSig_signal_update_modified";
+					tag.save(transaction = "rollback");
+				} finally {
+					StructDelete(request, "$wheelsTransactionWrapper");
+				}
+				var refetched = g.model("tag").findByKey(tag.id);
+				expect(refetched.name).toBe("outerSig_signal_update_modified");
+				// Cleanup: row name no longer matches the afterEach pattern.
+				queryExecute(
+					"DELETE FROM c_o_r_e_tags WHERE id = :id",
+					{ id = { value = tag.id, cfsqltype = "cf_sql_integer" } },
+					{ datasource = application.wheels.dataSourceName }
+				);
+			});
+
+			it("also bypasses the wrapper for deleteAll", () => {
+				// Issue #2789: same bypass applies to deleteAll's DELETE path.
+				g.model("tag").create(name = "outerSig_signal_delete");
+				request.$wheelsTransactionWrapper = true;
+				try {
+					g.model("tag").deleteAll(where = "name = 'outerSig_signal_delete'", transaction = "rollback");
+				} finally {
+					StructDelete(request, "$wheelsTransactionWrapper");
+				}
+				var remaining = g.model("tag").count(where = "name = 'outerSig_signal_delete'");
+				expect(remaining).toBe(0);
+			});
+
+		});
+
+	}
+
+}