fix(migrator): handle orphan versions in shared dev databases (#2780)#2798
Conversation
When wheels_migrator_versions records a version whose migration file is not in the current checkout (shared dev DB / peer applied a migration whose file isn't yet in this branch), wheels migrate latest no longer takes a misleading "down" branch and silently no-ops. Changes: * Migrator.$getOrphanVersions() — diffs the tracking table against on- disk files and returns versions with no matching file, sorted ascending. * Migrator.migrateTo() — branches on "orphan-at-top" before the existing direction check. When every DB version above target is an orphan: emits a warning naming the orphans, then either applies pending local files (up branch) or prints a clear "Nothing to do" naming current vs target. When SOME above-target versions are legitimate down candidates and SOME are orphans, emits the warning but lets the existing down loop handle the rest (orphan rows skip naturally because the loop iterates files only). * Migrator.\$buildInfoOutput() — extracted info rendering so it is unit- testable without the HTTP dispatcher. Orphan rows render with a [?] marker and the literal "********** NO FILE **********" (Rails-style), with a footer explaining the cause. * public/views/cli.cfm info handler — delegates to \$buildInfoOutput(). * New spec OrphanDetectionSpec covers \$getOrphanVersions + the directional fix across five scenarios (clean state, single orphan, multiple orphans, nothing-to-do path, legitimate down still works). * New spec MigratorInfoSpec covers \$buildInfoOutput rendering. * Docs: new guides page at basics/shared-development-databases.mdx walks through what an orphan is, three resolution paths, and the recommendation to avoid shared dev DBs. AI-side reference at .ai/wheels/troubleshooting/shared-dev-databases.md. Note on local verification: a leaked JVM from another worktree is holding Tomcat's shutdown port (8081), blocking new wheels server starts on this machine. Tests are queued for CI to run across the full engine + DB matrix. Local TDD will run on next machine restart. Refs #2780 Signed-off-by: Peter Amiri <peter@alurium.com>
![wheels-bot[bot]](https://avatars.githubusercontent.com/in/3657588?s=60&v=4){kind=small}
There was a problem hiding this comment.
Wheels Bot - Reviewer A
The PR correctly diagnoses and fixes the orphan-version silent-no-op (issue 2780). The directional logic in migrateTo() and the new getOrphanVersions() helper are sound. Cross-engine patterns are clean: closures appear only as ArraySort comparators, no application-scope function members, no bracket-notation calls inside closures. DCO sign-off is present. Requesting changes for two reasons: missing test for the mixed-case orphan path, and explicitly deferred local verification (CLAUDE.md Required: Always for vendor/wheels/ changes).
CORRECTNESS
Missing test for mixed-case path (OrphanDetectionSpec.cfc)
Migrator.cfc lines 88-96 handle the case where some DB versions above target are orphans AND some have local files (legitimate down candidates). No it() block covers this path. The test should: apply 002, insert orphan 999, call migrateTo(001), assert the warning names the orphan, assert down-to-001 is in the output, and assert c_o_r_e_dropbears no longer exists after rollback.
Deferred local test run
The commit body states local TDD was deferred to CI. CLAUDE.md marks test-local.sh as Required: Always for any vendor/wheels/ change. Please run bash tools/test-local.sh migrator on a clean machine before merging.
TESTS
Double DB query (Migrator.cfc lines 47 and 65)
migrateTo() calls getOrphanVersions() which calls getVersionsPreviouslyMigrated() internally, then calls getVersionsPreviouslyMigrated() again directly to build dbVersionsAboveTarget. This is two DB round-trips per migrateTo() call even when there are no orphans. Suggested fix: fetch the applied-versions list once near the top of migrateTo(), pass it to getOrphanVersions(appliedList) as an optional param, and reuse it.
DOCS
No CHANGELOG entry
CHANGELOG.md has a live [Unreleased] Fixed section. The new orphan-warning behavior and [?] info output are user-visible changes that belong there.
Plan artifact (docs/superpowers/plans/2026-05-22-orphan-migration-detection.md)
A 1053-line agentic implementation plan was committed to the main tree. The same context lives in the PR body and commit message. Please drop this file before merging.
Nothing-to-do message wording (Migrator.cfc lines 78-81)
arguments.version is the target passed to migrateTo(), not necessarily the latest local migration. When called directly with an earlier version, the phrase Your latest local migration (X) is misleading. Consider Your target version (X) is below the database current version (Y).
COMMITS
Single commit fix(migrator): handle orphan versions in shared dev databases - type and scope correct, subject within 100 chars, DCO Signed-off-by present and matches commit author. No issues.
Wheels Bot — Reviewer B (round 1)A's review is substantively correct and the SycophancyNone detected. A explicitly verified cross-engine patterns (closure-only comparators in False positives
Missed issuesNone detected. The diff is focused (Migrator.cfc logic + two new spec files + cli.cfm delegation + doc pages), cross-engine patterns are clean, and A caught the only significant coverage gap. Verdict alignmentA's ConvergenceAligned, changes needed. Consensus: address four items before merge — (1) add a mixed-case test covering the path where orphan 999 sits above target 000 alongside local file 001 so the down branch runs with a warning, asserting the warning names the orphan and that 001 is rolled back; (2) delete |
- Reword "Your latest local migration" to "Your target version" in the orphan nothing-to-do message. arguments.version is the target, not necessarily the latest local file (Migrator.cfc). - Add mixed-case OrphanDetectionSpec covering orphan + legitimate down candidate above target: warning names the orphan, down branch still runs, c_o_r_e_dropbears is dropped after rollback. - Drop docs/superpowers/plans/2026-05-22-orphan-migration-detection.md (1053-line agentic plan duplicated in PR body / commit message). - Add CHANGELOG entry under [Unreleased] Fixed describing the orphan detection, three migrateTo branches, and the [?] info row. Non-blocking items left for follow-up: double getVersionsPreviouslyMigrated() fetch in migrateTo() (B disputed scope but underlying concern is valid), and the deferred local test-local.sh migrator run (sandbox lacks the wheels CLI binary; compat-matrix CI will validate on the new SHA). Refs #2780, #2798 Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Wheels Bot — Address Review (round 1)Applied consensus findings from Reviewer A and Reviewer B's convergence (round 1):
Skipped (non-blocking per B's convergence comment, deferred to follow-up):
The new commit will trigger a fresh Reviewer A run on the updated SHA. Convergence loop continues until reviewers align on |
There was a problem hiding this comment.
Wheels Bot - Reviewer A
TL;DR: This round addresses every blocking finding from the first review. The mixed-case orphan + legitimate-down test is present, the plan artifact is deleted, CHANGELOG entry is added, and the nothing-to-do wording is fixed. One docs nit remains (stale reference to the deleted plan file in the .ai/ doc), and the double $getVersionsPreviouslyMigrated() fetch is still in place but explicitly flagged as a non-blocking follow-up in the commit message. All cross-engine patterns are clean. Commenting (not requesting changes).
Correctness
All round-1 correctness issues resolved:
local.rv = ""is initialised atMigrator.cfc:28, so the&=compound assignments on lines 100/106/139/149/151 are safe across all engines.- Mixed-case path (orphan + legitimate-down candidate above target) is now covered by the new
it("warns about orphans and still runs the down branch when both are present above target")block inOrphanDetectionSpec.cfc. - Nothing-to-do wording fixed:
Migrator.cfc:78now reads"Nothing to do. Your target version ("—arguments.versionis the target passed tomigrateTo(), not necessarily the latest local file.
Docs
Stale reference in .ai/ doc. docs/superpowers/plans/2026-05-22-orphan-migration-detection.md was deleted in the second commit, but the agent reference doc still points to it.
.ai/wheels/troubleshooting/shared-dev-databases.md, line 47:
- Plan: `docs/superpowers/plans/2026-05-22-orphan-migration-detection.md`
The plan is gone. This line should be removed or replaced with a pointer to the PR. Minor, but the .ai/ docs are what the bot reads first, so stale paths accumulate confusion.
Performance (non-blocking, carry-over)
The double-fetch identified in round 1 is still present and noted in the commit message as a follow-up. Documenting for completeness:
In migrateTo():
getAvailableMigrations()at line 33 calls$getVersionsPreviouslyMigrated()(query 1).$getOrphanVersions()at line 47 calls$getVersionsPreviouslyMigrated()(query 2) andgetAvailableMigrations()(query 3 via the nested call).- The explicit
ListToArray($getVersionsPreviouslyMigrated())at line 65 inside theisOrphanAtTopbranch fires query 4.
For $buildInfoOutput(): getAvailableMigrations() at line 635, then $getOrphanVersions() at line 637 calls both again. Display-only path so impact is low, but the same structural issue exists. Caching the applied-versions list and threading it as an optional argument would eliminate the redundant queries.
Tests
Round-1 gap filled. The it() blocks in OrphanDetectionSpec.cfc now cover: empty match, single orphan, multiple orphans sorted ascending, sentinel exclusion, orphan-at-top does not down-migrate, pending local migrations apply, warning names the orphan, nothing-to-do for no pending, legitimate down still works, and the new mixed case. MigratorInfoSpec.cfc covers $buildInfoOutput() independently.
Commits
Both commits are well-formed. Types, scopes, and subjects conform to commitlint rules. DCO Signed-off-by is present and matches the commit author on both.
The 1053-line agentic plan file at docs/superpowers/plans/2026-05-22-orphan-migration-detection.md was deleted in commit ddac969 per Reviewer A/B convergence, but the AI-side reference doc still pointed to it. Replaced the dead link with a PR reference and tightened the follow-up section to describe the work in prose instead of pointing at plan files that may or may not exist. Round-2 Reviewer A nit. Refs #2780, #2798 Signed-off-by: Peter Amiri <peter@alurium.com>
Wheels Bot — Reviewer B (round 1)A's round-2 review is accurate and the stale doc-link finding is real. I verified the cited lines directly. Converging with one change needed: remove the stale plan reference from the new SycophancyNone detected. A confirmed the three round-1 blocking items are resolved (mixed-case test present, plan artifact deleted, CHANGELOG added), flagged the one remaining gap with a concrete file + line citation, and did not approve. False positives
Missed issuesNone detected. Cross-engine invariants checked:
Verdict alignmentA's ConvergenceAligned, changes needed. The single outstanding item is the stale plan reference in the new
The file it points to was deleted in the second commit of this PR. The rest of the doc is accurate. Optionally replace with a pointer to this PR number if a cross-reference is wanted. No other changes needed. Once this line is removed, the PR is review-clean. |
There was a problem hiding this comment.
Wheels Bot — Reviewer A
Round 3 addresses the only open finding from round 2 — the stale plan-file reference in .ai/wheels/troubleshooting/shared-dev-databases.md. The dead link to the deleted plan artifact is replaced with a PR reference, and the follow-up section is rewritten in prose that does not depend on plan files existing on disk. All round-1 and round-2 blocking findings have been resolved. Approving.
Docs
Round-2 nit resolved. .ai/wheels/troubleshooting/shared-dev-databases.md line 44:
- Plan: `docs/superpowers/plans/2026-05-22-orphan-migration-detection.md`
- Follow-up plans (separate PRs):
- Plan 2: `wheels migrate doctor` / `forget` / `pretend`
- Plan 3: schema enrichment of `wheels_migrator_versions`
is replaced with:
- PR #2798 (the fix)
...
- Follow-up work (separate PRs):
- `wheels migrate doctor` / `forget` / `pretend` for manual reconciliation
- Schema enrichment of `wheels_migrator_versions` (add `name` and `applied_at` columns)
Clean. All file references in the Related section now point to paths that exist in the tree.
One non-blocking observation: the PR body's "Follow-up" section still reads "Plans are checked in at docs/superpowers/plans/2026-05-22-orphan-migration-detection.md" — that file was deleted in round 2. PR descriptions are static after submission so this can't be fixed via a commit. Future readers will find the full context in the commit history. Not a merge blocker.
Performance (carry-over, non-blocking)
The double $getVersionsPreviouslyMigrated() fetch in Migrator.cfc noted in round 2 is still present and acknowledged in the commit message as a planned follow-up. No change here; no new findings.
Commits
docs(migrator): fix stale plan reference in shared-dev-databases.md — type docs, scope migrator, subject 54 chars, sentence-case, within the 100-char limit. Signed-off-by: Peter Amiri <peter@alurium.com> matches the commit author. All three commits in this PR carry valid DCO trailers.
Wheels Bot — Address Review (round 2)The round-2 converged-changes from Reviewer B ( That change was already applied in commit
No additional code changes were needed in this round. Reviewer A has already re-reviewed the current head ( |
Wheels Bot -- Reviewer B (round 1)A's round-3 review checks out. The one remaining open item from round 2 is verifiably addressed, the approval is backed by specific line citations, and no issues were glossed over. One minor factual error in a non-blocking note; the conclusion is sound regardless. SycophancyNone detected. A confirmed the round-2 stale-reference finding was resolved before approving, cited the actual before/after lines in the doc, and carried forward the non-blocking double-fetch note without inflating its severity. False positives
Missed issuesNone detected. The only diff in this SHA is the Verdict alignmentA's APPROVED is consistent with the findings: all four round-1 blocking items were addressed in the prior commit, the single round-2 blocking item is addressed in this commit, and no new issues surfaced. ConvergenceAligned, no changes needed. All blocking findings across three rounds are resolved. The PR is review-clean for this SHA. |
The cumulative state of #2798 (merged) and #2799 (this PR) added three new migrate subcommands plus the [?] orphan display in `migrate info`. PR-scope docs already covered the deep-dive page at basics/shared-development-databases.mdx, but the main migrations reference still said "four subcommands". Catching up: - basics/migrations.mdx now lists seven subcommands and points to shared-development-databases for the reconciliation deep dive. The `info` description mentions the [?] orphan row format. - CLAUDE.md's MCP/CLI table row now lists `doctor` under wheels_migrate and adds a separate "Migrator reconciliation" row for forget/pretend. - CLAUDE.md's Migrations & Seeding section gains a "Shared Dev DB Reconciliation" subsection summarising the new commands and pointing at the deep-dive docs. - CLAUDE.md's Reference Docs section adds the .ai/wheels/troubleshooting/shared-dev-databases.md link. Refs #2780, #2798, #2799 Signed-off-by: Peter Amiri <peter@alurium.com>
…#2799) * feat(migrator): doctor/forget/pretend reconciliation commands (#2780) Follow-up to #2798. Adds three new `wheels migrate` subcommands for manual reconciliation against the tracking table — the Flyway `validate` / `repair` / `SkipExecutingMigrations` analogues for Wheels. * `wheels migrate doctor` — single-command health report. Lists orphans, pending local migrations, and applied count. Pure read; never mutates. Built on Plan 1's `$getOrphanVersions()`. * `wheels migrate forget <version> --yes` — removes a single row from `wheels_migrator_versions` without running `down()`. Refuses if the version has a matching local file (use `migrate down` instead) or if the version isn't in the table. Idempotent. * `wheels migrate pretend <version> --yes` — inserts a row into `wheels_migrator_versions` without running `up()`. Refuses if already applied or if no local file matches. Both `forget` and `pretend` require explicit `--yes` to mutate; without it they print what would happen and exit. The CLI side wires through a new `runForgetOrPretend()` helper that handles the gating; the HTTP dispatcher in `cli.cfm` reads the version from `request.wheels.params.version`. New spec `MigratorReconciliationSpec.cfc` covers ~12 scenarios across the three methods: clean state, unhealthy with orphans, unhealthy with pending, refusal paths, and the no-mutation guarantee for invalid inputs. Docs: extended `basics/shared-development-databases.mdx` with the new commands (Option 2 reworked, Option 2b added, "Comprehensive diagnostic" section added). AI-side reference and CHANGELOG entry follow the same shape. Note on local verification: a leaked JVM from another worktree continues to hold Tomcat's shutdown port (8081); CI compat-matrix will validate across the full engine + DB matrix. Refs #2780, #2798 Signed-off-by: Peter Amiri <peter@alurium.com> * fix(migrator): address Reviewer A round-1 findings on #2799 - forgetVersion() now delegates to the existing private $removeVersionAsMigrated() helper instead of running an inline DELETE. The helper wraps the DELETE in the same request.$wheelsDebugSQL guard that $setVersionAsMigrated() uses, so forgetVersion() and pretendVersion() are now symmetric: both are no-ops in debug-mode request contexts instead of silently corrupting the tracking table. - runForgetOrPretend() now wraps the version argument with URLEncodedFormat() when building the reconcile URL. $sanitiseVersion() on the server side strips non-digits before SQL use (no SQL injection path), but raw URL-special characters (&, =, %) in the CLI argument could inject spurious query parameters before the sanitiser ran. Both items were flagged by Reviewer A on commit b781745. No new tests needed: existing MigratorReconciliationSpec covers the success and refusal paths for forgetVersion(), and the URL-encoding fix is on the CLI client side which is not exercised by core unit tests. Refs #2780, #2799 Signed-off-by: Peter Amiri <peter@alurium.com> * fix(cli): doctor output prints yellow when migrator is unhealthy Reviewer A flagged that runMigration("doctor") always printed in green regardless of the report's healthy flag. Green on an unhealthy result (orphans or pending migrations present) reads as "everything is fine" when actually attention is needed. The fix switches color to yellow when: - action == "doctor" - response includes a `healthy` key - healthy is false Other actions (latest/up/down/info) stay green on success — they either succeed or throw, so the binary mapping holds. Only `doctor` returns a structural "succeeded but unhealthy" state. Non-blocking per A's COMMENTED verdict, but a real UX improvement. Refs #2780, #2799 Signed-off-by: Peter Amiri <peter@alurium.com> * docs: cross-reference doctor/forget/pretend in main migration docs The cumulative state of #2798 (merged) and #2799 (this PR) added three new migrate subcommands plus the [?] orphan display in `migrate info`. PR-scope docs already covered the deep-dive page at basics/shared-development-databases.mdx, but the main migrations reference still said "four subcommands". Catching up: - basics/migrations.mdx now lists seven subcommands and points to shared-development-databases for the reconciliation deep dive. The `info` description mentions the [?] orphan row format. - CLAUDE.md's MCP/CLI table row now lists `doctor` under wheels_migrate and adds a separate "Migrator reconciliation" row for forget/pretend. - CLAUDE.md's Migrations & Seeding section gains a "Shared Dev DB Reconciliation" subsection summarising the new commands and pointing at the deep-dive docs. - CLAUDE.md's Reference Docs section adds the .ai/wheels/troubleshooting/shared-dev-databases.md link. Refs #2780, #2798, #2799 Signed-off-by: Peter Amiri <peter@alurium.com> * docs: replace stale schema_migrations references with wheels_migrator_versions Three carryover Rails table-name references in basics/migrations.mdx (lines 28, 36, 59) predate the c_o_r_e_* → wheels_* rename and don't match any code in vendor/wheels/, cli/, or app/. Reviewer A flagged them on PR 2799 round-4 review (f663b5a) as worth fixing since the file was already being touched; the surrounding edits in this PR already use the correct `wheels_migrator_versions` name (line 46). Other stale references in basics/seeding.mdx and the v4-0-1-snapshot/ copies are out of scope for this PR; left for a follow-up cleanup. Refs #2780, #2799 Signed-off-by: Peter Amiri <peter@alurium.com> --------- Signed-off-by: Peter Amiri <peter@alurium.com>
…action CI on Lucee 7 + SQLite revealed two correctness bugs in #2800: 1. \$setVersionAsMigrated called \$dbinfo(type="version") to detect SQLite-vs-other-engines before deciding whether to include applied_at in the INSERT. But \$setVersionAsMigrated runs INSIDE migrateTo's open JDBC transaction, and SQLite (and possibly other engines under concurrent load) can't service JDBC metadata calls inside an open transaction. Result: [SQLITE_ERROR] SQL error or missing database, the migration's transaction rolled back, and every test that applied a migration with the enriched schema active failed. Fix: cache the engine type on application[appKey].\$migratorDbType in \$ensureTrackingColumns() (which always runs outside transactions), then read it from the cache in \$setVersionAsMigrated(). No more \$dbinfo calls during inserts. 2. \$maybeEnsureTrackingColumns set the \$trackingColumnsEnsured cache flag whenever \$ensureTrackingColumns() didn't throw — even when the column probe failed (e.g. table didn't exist yet) and no ALTERs ran. Subsequent \$setVersionAsMigrated calls then saw the flag set and tried to INSERT with a `name` column that didn't actually exist on the table → SQL error → rolled back transaction. Fix: only set the cache flag when BOTH rv.hasName and rv.hasAppliedAt are true after \$ensureTrackingColumns() returns. Failures, missing tables, or partial-ALTER states leave the flag unset so subsequent calls retry. Together these failures broke six specs across migratorSpec, OrphanDetectionSpec, and SchemaEnrichmentSpec — all symptoms of the same two root causes. With the fix in place, \$setVersionAsMigrated is back to a single $query call with no metadata interrogation, and the enriched-INSERT path only fires when the schema is genuinely confirmed. Refs #2780, #2798, #2799, #2800 Signed-off-by: Peter Amiri <peter@alurium.com>
basics/migrations.mdx's one-line summary of `wheels migrate info` still described orphan rows in the legacy "********** NO FILE **********" format only. Plan 3 enriched that — orphans with populated name + applied_at columns now show as `[?] <version> <name> (applied <timestamp>)`, falling back to the legacy display for pre-enrichment rows. Mirrors the same sync that landed in Plan 2 (when the orphan format was first added). The deep-dive page at basics/shared-development-databases.mdx already shows both formats with sample output; this brings the main reference in line. Refs #2780, #2798, #2799, #2800 Signed-off-by: Peter Amiri <peter@alurium.com>
#2780) (#2800) * feat(migrator): enrich wheels_migrator_versions with name + applied_at (#2780) Final follow-up to #2798 + #2799. Adds two nullable columns to the tracking table so `wheels migrate info` and `wheels migrate doctor` can show *what* a peer applied and *when* — even for orphan rows whose migration file isn't in the local checkout. ## Schema change - `wheels_migrator_versions.name VARCHAR(255) NULL` — migration name derived from filename (e.g. `create_users`) - `wheels_migrator_versions.applied_at TIMESTAMP NULL DEFAULT CURRENT_TIMESTAMP` — when the migration was applied. SQLite gets TEXT with CFML-side `Now()` injection on insert because SQLite can't DEFAULT a column on ADD COLUMN. Both nullable, additive, backward compatible. Existing rows (pre-enrichment) stay NULL and display version-only in the info output. Going-forward-only — no backfill. ## Bootstrap `Migrator.$ensureTrackingColumns()` probes via `$dbinfo` and ALTERs the table per-engine when columns are missing. Idempotent (skip when already present). Per-engine SQL covers MySQL, PostgreSQL, SQLite, MSSQL, Oracle, H2, and CockroachDB — same pattern as the existing `renameSystemTables()` machinery at Migrator.cfc:932. Wrapped by `$maybeEnsureTrackingColumns(appKey)` which caches on `application[appKey].$trackingColumnsEnsured` so the ALTER runs once per app process, not on every migrator call. Non-fatal: if the ALTER fails (locked table, weird permissions), the flag stays unset and the legacy schema continues to work. Called from `$getVersionsPreviouslyMigrated()` after both the existing-table happy path and the bootstrap-creates-table path. ## Population `$setVersionAsMigrated(version, migrationName)` gains an optional `migrationName` arg. When non-empty AND the enriched-columns flag is set, includes `name` (and on SQLite, `applied_at`) in the INSERT. Other engines rely on the column's CURRENT_TIMESTAMP default for `applied_at`. Callers updated to pass the name: both up-loops in `migrateTo()`, `migrateIndividual()`, and `pretendVersion()` (captures the migration's name from `getAvailableMigrations()` during the local-file existence check). ## Display `$getOrphanVersionsWithMeta()` — new public helper. Returns `{version, name, appliedAt}` structs for each orphan. Falls back to bare structs when columns aren't ensured or the SELECT fails. `$buildInfoOutput()` and `cli.cfm`'s `doctor` case now render `[?] <version> <name> (applied <timestamp>)` when populated, with the legacy `[?] <version> ********** NO FILE **********` fallback for legacy NULL orphans. `doctor()` adds `orphansWithMeta` to its result struct alongside the existing `orphans: array of strings` (kept for backward-compat with `migrateTo()`'s direction logic). ## Docs - `web/sites/guides/.../basics/shared-development-databases.mdx`: updated `info` sample output to show the enriched format + a note explaining the legacy fallback. - `.ai/wheels/troubleshooting/shared-dev-databases.md`: new "Schema enrichment" section documenting the helpers and behavior. - `CLAUDE.md`: updated the `info` format line and added the tracking-table schema summary in the Shared Dev DB Reconciliation subsection. - `CHANGELOG.md`: entry under `[Unreleased] Changed`. ## Tests `SchemaEnrichmentSpec.cfc` covers three scenarios: - $ensureTrackingColumns adds both columns on first call - Idempotent re-run (added=[]) - Name column populated for newly applied migrations Existing specs (migratorSpec, OrphanDetectionSpec, MigratorInfoSpec, MigratorReconciliationSpec) all preserve backward compat — the new name column is opt-in (only written when caller passes it), and the new display logic falls back to legacy rendering for NULL metadata. ## Known follow-ups (non-blocking, separate PRs) - Double `$getVersionsPreviouslyMigrated()` fetch carry-over from reviewer notes on #2798 and #2799 — still pending. This PR doesn't worsen the situation; the cached `$trackingColumnsEnsured` flag means `$ensureTrackingColumns` only probes columns once per process. Refs #2780, #2798, #2799 Signed-off-by: Peter Amiri <peter@alurium.com> * fix(migrator): don't call \$dbinfo inside $setVersionAsMigrated transaction CI on Lucee 7 + SQLite revealed two correctness bugs in #2800: 1. \$setVersionAsMigrated called \$dbinfo(type="version") to detect SQLite-vs-other-engines before deciding whether to include applied_at in the INSERT. But \$setVersionAsMigrated runs INSIDE migrateTo's open JDBC transaction, and SQLite (and possibly other engines under concurrent load) can't service JDBC metadata calls inside an open transaction. Result: [SQLITE_ERROR] SQL error or missing database, the migration's transaction rolled back, and every test that applied a migration with the enriched schema active failed. Fix: cache the engine type on application[appKey].\$migratorDbType in \$ensureTrackingColumns() (which always runs outside transactions), then read it from the cache in \$setVersionAsMigrated(). No more \$dbinfo calls during inserts. 2. \$maybeEnsureTrackingColumns set the \$trackingColumnsEnsured cache flag whenever \$ensureTrackingColumns() didn't throw — even when the column probe failed (e.g. table didn't exist yet) and no ALTERs ran. Subsequent \$setVersionAsMigrated calls then saw the flag set and tried to INSERT with a `name` column that didn't actually exist on the table → SQL error → rolled back transaction. Fix: only set the cache flag when BOTH rv.hasName and rv.hasAppliedAt are true after \$ensureTrackingColumns() returns. Failures, missing tables, or partial-ALTER states leave the flag unset so subsequent calls retry. Together these failures broke six specs across migratorSpec, OrphanDetectionSpec, and SchemaEnrichmentSpec — all symptoms of the same two root causes. With the fix in place, \$setVersionAsMigrated is back to a single $query call with no metadata interrogation, and the enriched-INSERT path only fires when the schema is genuinely confirmed. Refs #2780, #2798, #2799, #2800 Signed-off-by: Peter Amiri <peter@alurium.com> * fix(migrator): address Reviewer A round-1 findings on #2800 C1 (correctness): The shared-dev-databases guide example showed "(applied ...)" timestamps on [x] rows, but the code in $buildInfoOutput only populates appliedAt for orphan rows (the DB isn't re-queried for applied_at on local-file rows). Aligning the guide example with what the code outputs — keep the timestamp display on the [?] orphan row where the schema enrichment actually shows through, drop it from the [x] rows. Surrounding prose already correctly scopes the enrichment to orphans. T1 (test quality): SchemaEnrichmentSpec's "populates the name column" test used expect(Len(rows.name) > 0).toBeTrue() which collapses to a boolean before the matcher sees it — on failure the error message is just "Expected [false] to be [true]" with no hint of the actual value. Replaced with expect(rows.name).notToBeEmpty() so failures include the actual name (or absence of one). T2 (coverage): Added a new "populates applied_at for newly applied migrations" spec that queries the applied_at value and asserts it parses as a date. Covers both code paths: the column-DEFAULT CURRENT_TIMESTAMP that fires on MySQL/Postgres/MSSQL/Oracle/H2 AND the CFML-side Now() that SQLite needs because it can't DEFAULT a TIMESTAMP on ADD COLUMN. T3 (lint): Added a comment block above the describe block explaining why CockroachDB is skipped — mirrors the existing pattern in migratorSpec, OrphanDetectionSpec, and MigratorInfoSpec (numeric-version test fixtures don't run cleanly against CockroachDB; compat-matrix.yml treats CockroachDB as soft-fail). The guard is intentional, not vestigial. Also clearing the new $migratorDbType app-scope cache in the beforeEach/afterEach alongside the existing $trackingColumnsEnsured cleanup, so each test starts from a clean state. A's performance note about $dbinfo in $setVersionAsMigrated is already addressed in commit bb4bd76 (caches the engine type on $migratorDbType to avoid the per-insert metadata round-trip). A was reviewing 695f830 and hadn't seen that commit yet. Refs #2780, #2800 Signed-off-by: Peter Amiri <peter@alurium.com> * docs: correct stale [x] timestamp claim in .ai schema-enrichment ref Reviewer B's round-1 convergence on #2800 caught a missed-by-A follow-on of the C1 finding: the .ai/wheels/troubleshooting/ shared-dev-databases.md doc's first display bullet read - `[x] <version> <name> (applied <timestamp>)` when populated but the code only renders the (applied ...) suffix on orphan rows. The MDX guide example was corrected in fdbbedc; this commit brings the .ai-side reference in line and adds a sentence explaining why $buildInfoOutput doesn't show applied_at for [x] rows (it would require re-querying the tracking table for every getAvailableMigrations result, which is a bigger change than this PR is taking on). Refs #2780, #2800 Signed-off-by: Peter Amiri <peter@alurium.com> * fix(migrator): populate \$migratorDbType cache before early-return (#2800 C2) Reviewer A round-2 caught a latent bug in \$ensureTrackingColumns: the early-return when both enriched columns are already present fired BEFORE the \$migratorDbType cache was populated. The cache is on application scope, so it gets cleared on every app restart / ?reload=true. On the second app start onward, the early-return fired, \$migratorDbType stayed empty, and \$setVersionAsMigrated's SQLite branch (which writes an explicit applied_at via CFML Now() because SQLite can't DEFAULT a TIMESTAMP on ADD COLUMN) was skipped. Result: SQLite's applied_at landed NULL on every migration applied after the first app start. Fix: populate the cache BEFORE the hasName/hasAppliedAt early-return, guarded by StructKeyExists so it fires at most once per app process. The existing ALTER path now reads from the cache instead of issuing its own \$dbinfo round-trip (still inside \$ensureTrackingColumns, which runs outside transactions — safe). Regression test: added "populates applied_at across app restarts (regression for round-2 C2)" that explicitly simulates the broken sequence: migrate 001, wipe both caches (mimicking app restart), migrate 002, assert applied_at is still a parseable date. On the unpatched code this would fail on SQLite (applied_at = NULL). Refs #2780, #2800 Signed-off-by: Peter Amiri <peter@alurium.com> * fix(migrator): refresh tracking-columns flag from probe on every call CI on Lucee 7 + SQLite still failed after the C2 patch (#2800 round 2). Same 6 failures: migrations rolled back with [SQLITE_ERROR] SQL error or missing dat[a] when applying 001. Root cause was a different stale- cache bug than C2 — the $trackingColumnsEnsured flag persisted across test runs but the underlying tracking table was dropped + recreated in each spec's beforeEach. $maybeEnsureTrackingColumns saw the flag set and skipped $ensureTrackingColumns; the freshly-bootstrapped table had only (version, core_level), but $setVersionAsMigrated still tried to INSERT against the missing `name` column → migration rolled back. Production analog: any operator-driven DROP / recreate of wheels_migrator_versions (rare but possible) would silently corrupt the next migrator run the same way. Fix: drop the skip-on-flag-set early return in $maybeEnsureTrackingColumns. Always call $ensureTrackingColumns; the helper is already idempotent and short-circuits when both columns are present (one $dbinfo column probe — cheap). Set the flag from the actual probe result on every call; clear it on probe failure or partial-ALTER outcome. The $migratorDbType cache from the C2 fix still avoids the per-INSERT $dbinfo round-trip, so the perf cost of removing the skip is one column probe per migrator call — well within the existing per-call cost. Regression test: "re-runs ALTER when the tracking table is dropped+recreated" sets up the exact failing scenario — applies 001 (sets cache), drops the tracking table, applies 001 again, and asserts the name column is populated. On the unpatched code the second migrateTo would error on the INSERT and roll back. Refs #2780, #2800 Signed-off-by: Peter Amiri <peter@alurium.com> * test(migrator): fix regression test setup — drop both tables The previous round's regression test for the stale-flag scenario left c_o_r_e_bunyips in place when dropping wheels_migrator_versions. On the second migrateTo("001") call, up()'s createTable("c_o_r_e_bunyips") failed because the table already existed, the transaction rolled back, and the expected INSERT row never landed (Expected [1] but received [0]). The bug under test was correctly fixed by the prior commit (2f71af2) — the failure was purely a test-setup miss. Drop both the tracking table AND c_o_r_e_bunyips so the second migrateTo can re-apply 001 from a clean schema. The cache-flag-stays-set scenario is preserved, which is what the regression test is actually verifying. Refs #2780, #2800 Signed-off-by: Peter Amiri <peter@alurium.com> * docs: sync main migrations.mdx with Plan 3 orphan display format basics/migrations.mdx's one-line summary of `wheels migrate info` still described orphan rows in the legacy "********** NO FILE **********" format only. Plan 3 enriched that — orphans with populated name + applied_at columns now show as `[?] <version> <name> (applied <timestamp>)`, falling back to the legacy display for pre-enrichment rows. Mirrors the same sync that landed in Plan 2 (when the orphan format was first added). The deep-dive page at basics/shared-development-databases.mdx already shows both formats with sample output; this brings the main reference in line. Refs #2780, #2798, #2799, #2800 Signed-off-by: Peter Amiri <peter@alurium.com> --------- Signed-off-by: Peter Amiri <peter@alurium.com>
…o sibling v4 docs (#2801) Carryover from PR #2799. Commit eacf4f6 fixed three stale references in v4-0-0/basics/migrations.mdx; the same family of references still appeared in three siblings and is fixed here in one pass: - v4-0-0/basics/seeding.mdx:26 - v4-0-1-snapshot/basics/migrations.mdx:28,36,49 - v4-0-1-snapshot/basics/seeding.mdx:26 The on-disk table has been wheels_migrator_versions since the c_o_r_e_* → wheels_* rename; no schema_migrations table exists anywhere in vendor/wheels/, cli/, or app/. The line-49 fix in v4-0-1-snapshot/basics/migrations.mdx also mirrors the "keys on the filename" → "keys on the timestamp prefix" wording fix from eacf4f6, since the tracking table stores only the timestamp version (e.g., 20260420143000), not the full filename. v3-0-0/command-line-tools/cli-guides/migrations.md:632-633 intentionally left as-is: v3 used a different table name pre-rename, so the references are historical and accurate. Verified: \`grep -rn schema_migrations web/sites/guides/src/content/docs/v4-0-0 web/sites/guides/src/content/docs/v4-0-1-snapshot\` now returns zero matches. Refs #2780, #2798, #2799 Signed-off-by: Peter Amiri <peter@alurium.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Summary
Resolves #2780. When
wheels_migrator_versionsrecords a version whosemigration file isn't in the current checkout (shared dev DB / peer
migration not yet pulled),
wheels migrate latestno longer takes amisleading "down" branch and silently no-ops.
What changed
Migrator.$getOrphanVersions()diffs the tracking tableagainst
app/migrator/migrations/and returns versions with nomatching file, sorted ascending.
Migrator.migrateTo()branches on orphan-at-topbefore the existing direction check. When all DB versions > target
are orphans, either applies pending local migrations (the up branch)
with a clear warning, OR emits "Nothing to do" naming current vs
target. Mixed case (some legitimate down candidates + some orphans)
emits the warning and lets the existing down loop continue — orphan
rows skip naturally because the loop iterates files only.
wheels migrate infoshows orphan rows with[?] <version> ********** NO FILE **********(Rails-style) and afooter explaining the cause. Rendering extracted into
Migrator.$buildInfoOutput()for unit-testability.web/sites/guides/.../basics/shared-development-databases.mdxcovering detection, three resolution paths, and the recommendation
to avoid shared dev DBs where possible. AI-side reference at
.ai/wheels/troubleshooting/shared-dev-databases.md.Behavior summary
Migrating from X down to Y.then silent no-opNote: database tracks version(s) X with no matching file...+Nothing to do.naming current vs targetMigrating from X down to Y.then partial down (orphan skipped silently)migrate infowith orphan presentCurrent version: Xbut X missing from list (self-contradictory)[?] X ********** NO FILE **********row plusorphan: 1summary + explanation footerTest plan
compat-matrix.ymlruns all new specs across Lucee 5/6/7,Adobe CF 2018/2021/2023/2025, BoxLang × SQLite/H2/MySQL/Postgres/
MSSQL/Oracle/CockroachDB
compat-matrix.ymlconfirms no regression in existingmigratorSpec.cfcthe tracking table + a new migration file with an earlier timestamp;
confirm
wheels migrate latestapplies the new file and emits thewarning (rather than the previous silent no-op)
wheels migrate infoagainst the same state shows theorphan with
[?]and aNO FILEannotationNote on local verification: a leaked JVM from another worktree is
holding Tomcat's shutdown port (8081), blocking new
wheels serverstarts on the dev machine. Local TDD was deferred to the next clean
restart; CI runs the full engine × DB matrix on this PR.
Follow-up
Two follow-up PRs are planned (independent of this one):
wheels migrate doctor/forget/pretendCLIcommands for manual reconciliation (Flyway
validate/repairequivalents).
wheels_migrator_versions—add
nameandapplied_atcolumns so orphan rows display thepeer's migration name and apply timestamp, not just the version.
Plans are checked in at
docs/superpowers/plans/2026-05-22-orphan-migration-detection.md.Refs #2780