Skip to content

fix(deploy): script pending migrations from the first unapplied one - #113

Merged
ChristopherHoffman merged 1 commit into
mainfrom
fix/deploy-migration-script-out-of-order
Sep 4, 2026
Merged

ChristopherHoffman merged 1 commit into
mainfrom
fix/deploy-migration-script-out-of-order

Conversation

@ChristopherHoffman

Copy link
Copy Markdown
Contributor

The bug

The migration-script artifact — what a reviewer reads before approving the production environment gate — chose its starting point with:

[.[] | select(.applied == true)] | last

That assumes applied migrations form a prefix of the id-ordered list. They don't. A PR that branches early can merge after a sibling, leaving a pending migration with a lower id than migrations already in the database.

#106 merged after #107/#108 and did exactly that:

20260826034417_AddFilamentImage             applied
20260827122935_AddProjectDateOverrides      NOT applied   <- lower id, later merge
20260829105948_AddUniqueUserSettingIndex    applied
20260829110203_AddPushNotificationUser...   applied
20260829110351_AddDeviceTokens              applied

"Last applied" resolved to AddDeviceTokens, and scripting forward from there produced an empty file.

The deploy itself was correct. efbundle applies all pending migrations regardless of ordering, so AddProjectDateOverrides was applied. What was lost is review: the approval gate showed an empty script for a run that was about to add two columns.

The fix

Script from the migration before the first unapplied one. --idempotent already guards every statement, so already-applied migrations caught in that range are no-ops.

history before after
this case (0827 pending, 0829 applied) 20260829110351 → empty script 20260826034417 → includes both AddColumns
normal, pending at the tail last applied same migration, same output
nothing pending last applied → empty artifact none → -- No pending migrations.
fresh database step errors 0 → full script

The last two were previously wrong as well: an empty artifact is indistinguishable from this bug, and the hard error fired on an empty result rather than on what it was meant to catch — an unreadable migration list, i.e. the connection failing behind || true. That guard now keys off the list itself.

Verification

The step's selection logic was exercised against the four shapes in the table above. It has not run in CI yet — deploy.yml only triggers on a v* tag, so the first real proof is the next release's migration-script artifact. Worth a closer read than usual for that reason.

Not addressed

20260827122935_AddProjectDateOverrides.Designer.cs carries a pre-push-notification snapshot (no DeviceToken), which is inherent to the out-of-order merge. It's harmless — EF scaffolds new migrations from PrintLogContextModelSnapshot.cs, which is correct, which is also why CI's has-pending-model-changes stayed green. It would only matter to a migrations remove reaching back past it.

The migration-script artifact -- the thing a reviewer reads before approving
the production environment gate -- picked its starting point with
`[.[] | select(.applied == true)] | last`, which assumes applied migrations
form a prefix of the id-ordered list.

They do not. A PR that branches early can merge after a sibling, leaving a
pending migration with a LOWER id than migrations already in the database.
#106 merged after #107/#108 and did exactly that: 20260827_AddProjectDateOverrides
sat unapplied below three applied 20260829 migrations, so "last applied"
resolved to the newest id, and scripting forward from there produced an empty
file. The efbundle applies all pending migrations regardless of ordering, so
the deploy itself was correct -- but the approval gate showed nothing while two
columns were about to be added.

Script from the migration *before the first unapplied one* instead. --idempotent
already guards every statement, so already-applied migrations caught in that
range are no-ops.

Two edge cases the old query got wrong are now handled explicitly: nothing
pending writes a comment rather than an empty artifact, and a database with no
migrations applied yields EF's "0" marker instead of failing the step. The
hard error is now scoped to what it was meant to catch -- an unreadable
migration list, i.e. the connection failing behind `|| true`.
@ChristopherHoffman
ChristopherHoffman merged commit 6aea852 into main Sep 4, 2026
2 of 3 checks passed
@ChristopherHoffman
ChristopherHoffman deleted the fix/deploy-migration-script-out-of-order branch September 4, 2026 01:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant