fix(db): don't mark migrations applied when non-optional statements fail (root cause of #61)#66
Open
RubyWolff27 wants to merge 1 commit into
Open
fix(db): don't mark migrations applied when non-optional statements fail (root cause of #61)#66RubyWolff27 wants to merge 1 commit into
RubyWolff27 wants to merge 1 commit into
Conversation
_apply_migrations recorded a migration in schema_migrations even when its statements hard-failed (only raising if ALL failed). A CREATE TABLE written in SQLite dialect (INTEGER PRIMARY KEY AUTOINCREMENT) fails on Postgres, the SAVEPOINT swallows it, and the migration is still marked applied — leaving a permanent silent gap. This is exactly why _ensure_critical_columns exists as a band-aid, and is the root cause of the missing `api_keys` table (issue #61: api-key creation 500s because the table never got created on the VPS Postgres while migration 107 was marked applied). Fix: classify per-statement failures. CREATE EXTENSION failures stay tolerated (e.g. pgvector not installed); any other failed statement is a "hard" failure. If a migration has hard failures (but not all-fail), commit what succeeded but do NOT record it as applied, so it retries on the next boot instead of going silently missing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Why
Issue #61 ("api-key creation 500s") had a deeper root cause than the route. The
api_keystable never existed on the VPS Postgres, yet migration107_create_api_keys.sqlwas recorded as applied (2026-05-26)._apply_migrationswraps each statement in aSAVEPOINTand only aborts if all statements fail; on partial failure it logs a warning and records the migration as applied anyway. So aCREATE TABLEwritten in SQLite dialect (INTEGER PRIMARY KEY AUTOINCREMENT,datetime('now')) fails on Postgres, gets swallowed, and the migration is marked done — a permanent silent gap. The existing_ensure_critical_columns()band-aid exists precisely because of this failure class; it just didn't cover theapi_keystable.Fix
Distinguish optional from hard statement failures:
CREATE EXTENSIONfailures stay tolerated (e.g. pgvector not installed) — unchanged.This fixes the whole class, not just
api_keys, with no change to the intentional pgvector tolerance.Note
The HTTP create-key route itself is fine on Postgres —
execute_insertalready usesRETURNING id. The only fault was the missing table. Theapi_keystable has already been created on prod manually (idempotent107_create_api_keys_pg.sql) and anrf_pk_…key minted for the Sage MCP integration; this PR prevents the silent-gap recurrence.Test
python -m py_compileclean; failure-classifier unit-checked (CREATE TABLE/INDEX/ALTER → hard; CREATE EXTENSION → optional).DATABASE_URL— deferred to CI.🤖 Generated with Claude Code