From c9c4828387eeb3b688fde0d32dcc89facc23a84a Mon Sep 17 00:00:00 2001 From: pyr0ball Date: Sun, 5 Apr 2026 22:23:29 -0700 Subject: [PATCH] fix: make migration runner resilient to partial-failure recovery SQLite's executescript() auto-commits each DDL statement individually. If a migration crashes mid-run, prior ALTER TABLE statements are already committed but the migration is never recorded as applied. On restart, the runner re-runs the same file and hits 'duplicate column name' on already-applied statements, breaking subsequent startups permanently. Replace executescript() with per-statement execute() calls. 'Duplicate column name' OperationalErrors are caught and logged as warnings so the migration can complete and be marked as done. All other errors still propagate normally. --- circuitforge_core/db/migrations.py | 53 ++++++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/circuitforge_core/db/migrations.py b/circuitforge_core/db/migrations.py index f3b3cac..a11640c 100644 --- a/circuitforge_core/db/migrations.py +++ b/circuitforge_core/db/migrations.py @@ -4,12 +4,22 @@ Applies *.sql files from migrations_dir in filename order. Tracks applied migrations in a _migrations table — safe to call multiple times. """ from __future__ import annotations +import logging import sqlite3 from pathlib import Path +_log = logging.getLogger(__name__) + def run_migrations(conn: sqlite3.Connection, migrations_dir: Path) -> None: - """Apply any unapplied *.sql migrations from migrations_dir.""" + """Apply any unapplied *.sql migrations from migrations_dir. + + Resilient to partial-failure recovery: if a migration previously failed + mid-run (e.g. a crash after some ALTER TABLE statements auto-committed), + "duplicate column name" errors on re-run are silently skipped so the + migration can complete and be marked as applied. All other errors still + propagate. + """ conn.execute( "CREATE TABLE IF NOT EXISTS _migrations " "(name TEXT PRIMARY KEY, applied_at TEXT DEFAULT CURRENT_TIMESTAMP)" @@ -22,8 +32,47 @@ def run_migrations(conn: sqlite3.Connection, migrations_dir: Path) -> None: for sql_file in sql_files: if sql_file.name in applied: continue - conn.executescript(sql_file.read_text()) + + _run_script(conn, sql_file) + # OR IGNORE: safe if two Store() calls race on the same DB — second writer # just skips the insert rather than raising UNIQUE constraint failed. conn.execute("INSERT OR IGNORE INTO _migrations (name) VALUES (?)", (sql_file.name,)) conn.commit() + + +def _run_script(conn: sqlite3.Connection, sql_file: Path) -> None: + """Execute a SQL migration file, statement by statement. + + Splits on ';' so that individual DDL statements can be skipped on + "duplicate column name" errors (partial-failure recovery) without + silencing real errors. Empty statements and pure-comment chunks are + skipped automatically. + """ + text = sql_file.read_text() + + # Split into individual statements. This is a simple heuristic — + # semicolons inside string literals would confuse it, but migration files + # should never contain such strings. + for raw in text.split(";"): + stmt = raw.strip() + if not stmt or stmt.startswith("--"): + continue + # Strip inline leading comments (block of -- lines before the SQL). + lines = [l for l in stmt.splitlines() if not l.strip().startswith("--")] + stmt = "\n".join(lines).strip() + if not stmt: + continue + + try: + conn.execute(stmt) + except sqlite3.OperationalError as exc: + if "duplicate column name" in str(exc).lower(): + _log.warning( + "Migration %s: skipping already-present column (%s) — " + "partial-failure recovery", + sql_file.name, + exc, + ) + else: + raise