Skip to content

fix(db): getDb() applies pending migrations on open (#202)#210

Merged
edheltzel merged 2 commits into
mainfrom
worktree-fix+getdb-applies-migrations-202
Jun 28, 2026
Merged

fix(db): getDb() applies pending migrations on open (#202)#210
edheltzel merged 2 commits into
mainfrom
worktree-fix+getdb-applies-migrations-202

Conversation

@edheltzel

Copy link
Copy Markdown
Owner

Closes #202.

Problem

getDb() (src/db/connection.ts, 68 callers — the shared hot open path) opened the DB and applied PRAGMAs but never called applyMigrations(). Only initDb() (run by recall init / the installer) migrated. So once new migrations landed in source, a live DB stayed pinned at whatever user_version it held the last time init ran, and every CLI/MCP/hook opening via getDb() silently used a stale schema.

Observed live (2026-06-25): a real DB sat at user_version=8 while source was 15 → the provenance column (ADR-0001/#42) was missing → memory_add writes failed silently.

Fix (Option 1 from the issue — runtime self-heal)

  • Extract initDb's idempotent schema bootstrap into ensureSchema() and call it from both initDb and getDb. getDb now brings a drifted DB current on open, exactly as initDb does.
  • The load-bearing ordering (CREATE_TABLES → applyMigrations → CREATE_INDEXES/FTS/VECTOR → guarded vec table) is preserved by construction — both paths run the same helper (DRY), so initDb's semantics are unchanged.

Must-investigate findings

1. Ordering safety. Migrations are mostly self-contained (7→8 creates its own indexes; 14→15 / 15→16 create their own FTS objects inline). The exceptions are migrations that depend on base tables created by CREATE_TABLES, not by the migration itself: 3→4 (extraction_*), 9→10 (dedup_lineage), and 15→16 (code_*). For a live v15 DB the code_* base tables don't exist, and 15→16's try/catch swallows their absence and only bumps the version — so calling applyMigrations alone from getDb would advance to v16 while leaving code_* missing (the version gate would then lie). Running the full ensureSchema sequence (i.e. CREATE_TABLES before applyMigrations, as initDb does) is what makes the self-heal complete. Regression-tested.

2. Concurrency. applyMigrations read user_version once before its loop. Now that every open can migrate, two processes can both observe version i and race. Added an under-lock re-read: after BEGIN IMMEDIATE takes the write lock (the loser waits out busy_timeout=5000), re-read user_version and skip the iteration if a concurrent migrator already advanced past inever double-apply. Each migration still runs in its own transaction.

3. Read-only / locked DB. getDb runs ensureSchema best-effort: on a read-only FS, or when the write lock can't be won within busy_timeout, the throw is swallowed and the DB is used as-is — no new crash for the 68 callers (incl. pure read paths) that work today. The DB stays at its current version; the next writable open retries. initDb still lets migration errors surface loudly (it's the explicit setup path).

Tests

  • getDb migrates a DB pinned at user_version = N-1 to MIGRATIONS.length on open (not just initDb).
  • getDb self-heals a base table a pending migration depends on — drops code_*, pins to N-1, asserts they're recreated (proves the full ordering runs in the open path; regresses the live v15 gap).
  • Concurrent migrators converge without double-applying.
  • Updated the repair legacy-DB test for the new contract: getDb now self-heals dedup_lineage on open, so repair sees a current schema (no crash; pending = 0).

Gate

  • bun run lint (tsc) ✅ · bun run build
  • bun test: 1195 pass / 10 fail. The 10 failures are pre-existing on origin/main (verified with this branch's changes stashed) and are entirely in the unrelated opencode/pi MCP-configure script (tests/opencode-integration.test.ts / tests/pi-*), not the DB layer. Not touched here — flagging separately.
  • Perf: cold getDb() incl. ensureSchema on an already-current DB ≈ 0.93 ms (idempotent DDL, once per process — same cost initDb already paid).

Out of scope (deliberate)

The recall doctor --fix check (issue option 3) and any Phase 2 / code_* / query-layer work.

getDb() opened the shared DB and applied PRAGMAs but never ran
applyMigrations — only initDb did. A live DB therefore stayed pinned at
whatever user_version it held when init last ran, and every CLI/MCP/hook
opening via getDb() silently used a stale schema (observed live: v8 vs
source v15 → memory_add INSERTing a missing provenance column → silent
write failures).

- Extract initDb's idempotent schema bootstrap into ensureSchema() and call
  it from both initDb and getDb. Preserves the load-bearing CREATE_TABLES →
  applyMigrations → CREATE_INDEXES/FTS/VECTOR ordering by construction (DRY).
- getDb runs ensureSchema best-effort: a read-only FS or lock-contended open
  swallows the error and proceeds, so no new throw for the 68 read callers.
- applyMigrations re-reads user_version under BEGIN IMMEDIATE before each
  migration, so a concurrent migrator can't double-apply.
- Tests: v(N-1) → current via getDb; code_* base-table self-heal; concurrent
  convergence. Updated the repair legacy-DB test for the new self-heal contract.
@edheltzel edheltzel marked this pull request as ready for review June 28, 2026 12:17
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.

harden: getDb() never applies pending migrations — live DB silently re-drifts after schema bumps (memory_add breaks)

1 participant