What happens
run_pending_migrations decides whether a migration has already run by looking only at its version number:
let already: bool = sqlx::query(
"SELECT EXISTS(SELECT 1 FROM schema_migrations WHERE version = $1) AS applied",
)
.bind(m.version)
...
if already {
continue;
}
crates/gitlawb-node/src/db/mod.rs:381-393 on main (bfc44f9). The name column is written when a migration is applied and never read back for comparison. So when two branches independently claim the same version number, whichever merges second has its DDL skipped in full, migrate() returns Ok, and the startup log reports a healthy schema. The node then 500s on every statement naming the object that migration was supposed to create.
This is live right now: three open PRs claim version 28 (#386, #384, #327), and #333 and #347 both claim 30 and 31.
Reproduction
Against origin/main at bfc44f9, with a real Postgres. Roll a fresh database back to a pre-v26 schema, record version 26 under a name from another branch, and migrate:
sqlx::query("ALTER TABLE pin_repair_sweep DROP COLUMN discovery_cursor_created_at").execute(&db.pool).await.unwrap();
sqlx::query("ALTER TABLE pin_repair_sweep DROP COLUMN discovery_cursor_id").execute(&db.pool).await.unwrap();
sqlx::query("DELETE FROM schema_migrations WHERE version = 26").execute(&db.pool).await.unwrap();
sqlx::query("INSERT INTO schema_migrations (version, name, applied_at) VALUES ($1, $2, $3)")
.bind(26i32).bind("some_other_branch_v26").bind("2026-08-31T00:00:00Z")
.execute(&db.pool).await.unwrap();
db.migrate().await.expect("migrate() must not error");
migrate() returns Ok, and discovery_cursor_created_at does not exist:
REPRO: migrate() returned Ok but v26's column is absent; the DDL was skipped
because version 26 was already recorded under the name some_other_branch_v26
Control, same setup but with no row at version 26 at all: migrate() reapplies the DDL and the column comes back, so the skip is caused by the recorded version and not by a failed ALTER.
Suggested fix
Read the recorded name and refuse to start rather than skipping silently. I ran this against the suite:
let recorded: Option<String> =
sqlx::query_scalar("SELECT name FROM schema_migrations WHERE version = $1")
.bind(m.version)
.fetch_optional(&self.pool)
.await?;
if let Some(recorded_name) = recorded {
if recorded_name != m.name {
anyhow::bail!(
"migration v{} is recorded as \"{}\" but this build carries \"{}\": \
two branches claimed the same version and one body was never applied. \
Renumber the newer migration rather than starting on this schema.",
m.version, recorded_name, m.name
);
}
continue;
}
With that in place the reproduction above aborts with the mismatch named instead of returning Ok, and the rest of the suite is unaffected: 13/13 migration tests and 96/96 db:: tests pass, including the legacy-install backfill and the existing upgrade-path tests. It is a startup abort, so an operator who somehow has a genuinely diverged database gets a loud failure instead of a node serving 500s, which is the tradeoff I would want here.
Severity
P2. It takes two branches claiming a version to trigger, so it is not reachable by an untrusted caller, but the failure mode is a production node running on a schema it believes is complete, with nothing in the log to say otherwise. The version-number collision it depends on is a normal consequence of parallel branches and is already present across five open PRs.
What happens
run_pending_migrationsdecides whether a migration has already run by looking only at its version number:crates/gitlawb-node/src/db/mod.rs:381-393onmain(bfc44f9). Thenamecolumn is written when a migration is applied and never read back for comparison. So when two branches independently claim the same version number, whichever merges second has its DDL skipped in full,migrate()returnsOk, and the startup log reports a healthy schema. The node then 500s on every statement naming the object that migration was supposed to create.This is live right now: three open PRs claim version 28 (#386, #384, #327), and #333 and #347 both claim 30 and 31.
Reproduction
Against
origin/mainat bfc44f9, with a real Postgres. Roll a fresh database back to a pre-v26 schema, record version 26 under a name from another branch, and migrate:migrate()returnsOk, anddiscovery_cursor_created_atdoes not exist:Control, same setup but with no row at version 26 at all:
migrate()reapplies the DDL and the column comes back, so the skip is caused by the recorded version and not by a failedALTER.Suggested fix
Read the recorded name and refuse to start rather than skipping silently. I ran this against the suite:
With that in place the reproduction above aborts with the mismatch named instead of returning
Ok, and the rest of the suite is unaffected: 13/13 migration tests and 96/96db::tests pass, including the legacy-install backfill and the existing upgrade-path tests. It is a startup abort, so an operator who somehow has a genuinely diverged database gets a loud failure instead of a node serving 500s, which is the tradeoff I would want here.Severity
P2. It takes two branches claiming a version to trigger, so it is not reachable by an untrusted caller, but the failure mode is a production node running on a schema it believes is complete, with nothing in the log to say otherwise. The version-number collision it depends on is a normal consequence of parallel branches and is already present across five open PRs.