fix(sqlitedb): verify the whole migration set at startup - #139
Merged
Conversation
RequireMigratedSchema asserted only that __schema_migrations was non-empty, which is true of every partially migrated database. The release flips the bin/release symlink before it runs the migrator, so for a few seconds the cron binaries resolve to a build newer than the schema: they started, then died mid-query on the first new column. An interrupted migrator or a hand-rolled deploy reaches the same state with no window at all. Compare the recorded filenames against the migrations the build embeds and refuse to start when any is missing, naming the first few. The service now fails closed with a sentence saying what to run, instead of failing open and reporting a missing column. The check is one-directional on purpose: a database carrying migrations this build does not know about is what a rollback to the previous artifact looks like, and refusing there would turn a rollback into an outage. An empty migration file is not counted as missing either — it applies nothing, so it is never recorded, and that is Migrator.Verify's complaint to make. The deploy ordering is left alone. Migrating before the flip would run the old binary against the new schema for the duration, which is only safe while every migration stays additive. Refs: #134 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U6jNuzK5PZrhDWBxB3t2gY
This was referenced Aug 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RequireMigratedSchemaasserted only that__schema_migrationswas non-empty — which istrue of every partially migrated database.
release.ymlflips thebin/releasesymlink before it starts thebeacon-migrateone-shot, so for a few seconds the cron binaries resolve to a build newer than the schema.
They started fine and then died mid-query on the first new column, which is a confusing way
to learn the migration has not run yet. #131 was the first column addition in a while:
anything touching
weather_user_citiesin that window would have failed onno such column: notify_state. An interrupted migrator or a hand-rolled deploy reaches the same state with nowindow at all.
What changed
The gate now compares the recorded filenames against the migrations the build embeds, and
refuses to start when any is missing — naming the first five and counting the rest, because a
fresh database is behind by all 33 and a startup line listing them buries the sentence that
says what to run. Callers pass
migrations.MigrationsFS;sqlitedbtakes anfs.FSand doesnot import the migrations package, so the layering holds.
Failing closed with "schema is behind this build: run cmd/migrator..." replaces failing open
and reporting a missing column.
One-directional on purpose. A database carrying migrations this build does not know about
is accepted: that is exactly what a rollback to the previous artifact looks like, and refusing
would turn a rollback into an outage. An empty migration file is not counted as missing
either — it applies nothing, so it is never recorded, and that is
Migrator.Verify'scomplaint to make rather than a reason to keep a service down.
What was rejected
Reordering the deploy so the migration runs before the symlink flip. It closes the same
window, but it runs the old binary against the new schema for the duration, which is
only safe while every migration stays additive — a weaker invariant than the one this change
relies on, and it protects nothing outside the deploy. The code change covers the interrupted
migrator and the hand-rolled deploy as well. Reversing this means moving the two lines in
release.yml; nothing here depends on the current order.Tests
Every migration recorded → nil. Unmigrated →
schema not initialised. Build ahead of thedatabase →
schema is behind this build, naming the file (verified to fail with thecomparison removed). Database ahead of the build → accepted. Empty migration file → not
counted as missing. Plus the message trimming, including that it does not scribble on the
caller's slice.
go vet,scripts/lint-checks.sh,golangci-lintclean; 41 packages green.Refs #134