From 2f495f676769b0de6da9abc1ecdbf036ec577d48 Mon Sep 17 00:00:00 2001 From: Matthew Raymer Date: Wed, 17 Sep 2025 06:52:43 +0000 Subject: [PATCH] feat: minimal stabilization of migration 004 with atomic execution - Single SQL source: Define MIG_004_SQL constant to eliminate duplicate SQL definitions - Atomic execution: Add BEGIN IMMEDIATE/COMMIT/ROLLBACK around migration execution - Name-only check: Skip migrations already recorded in migrations table - Guarded operations: Replace table-wide cleanups with conditional UPDATE/DELETE Changes: - migration.ts: Extract migration 004 SQL into MIG_004_SQL constant - migration.ts: Use guarded DELETE/UPDATE to prevent accidental data loss - migrationService.ts: Wrap migration execution in explicit transactions - migrationService.ts: Reorder checks to prioritize name-only skipping Benefits: - Prevents partial migration failures from corrupting database state - Eliminates SQL duplication and maintenance overhead - Maintains existing APIs and logging behavior - Reduces risk of data loss during migration execution Test results: All migration tests passing, ID generation working correctly --- src/db-sql/migration.ts | 88 +++++++++++++++++--------------- src/services/migrationService.ts | 76 +++++++++++++++------------ 2 files changed, 91 insertions(+), 73 deletions(-) diff --git a/src/db-sql/migration.ts b/src/db-sql/migration.ts index 7e9acb5d..05b99da9 100644 --- a/src/db-sql/migration.ts +++ b/src/db-sql/migration.ts @@ -31,6 +31,52 @@ import { logger } from "@/utils/logger"; const randomBytes = crypto.getRandomValues(new Uint8Array(32)); const secretBase64 = arrayBufferToBase64(randomBytes.buffer); +// Single source of truth for migration 004 SQL +const MIG_004_SQL = ` + -- Migration 004: active_identity_management (CONSOLIDATED) + -- Combines original migrations 004, 005, and 006 into single atomic operation + -- CRITICAL SECURITY: Uses ON DELETE RESTRICT constraint from the start + -- Assumes master code deployed with migration 003 (hasBackedUpSeed) + + -- Enable foreign key constraints for data integrity + PRAGMA foreign_keys = ON; + + -- Add UNIQUE constraint to accounts.did for foreign key support + CREATE UNIQUE INDEX IF NOT EXISTS idx_accounts_did_unique ON accounts(did); + + -- Create active_identity table with SECURE constraint (ON DELETE RESTRICT) + -- This prevents accidental account deletion - critical security feature + CREATE TABLE IF NOT EXISTS active_identity ( + id INTEGER PRIMARY KEY CHECK (id = 1), + activeDid TEXT REFERENCES accounts(did) ON DELETE RESTRICT, + lastUpdated TEXT NOT NULL DEFAULT (datetime('now')) + ); + + -- Add performance indexes + CREATE UNIQUE INDEX IF NOT EXISTS idx_active_identity_single_record ON active_identity(id); + + -- Seed singleton row (only if not already exists) + INSERT INTO active_identity (id, activeDid, lastUpdated) + SELECT 1, NULL, datetime('now') + WHERE NOT EXISTS (SELECT 1 FROM active_identity WHERE id = 1); + + -- MIGRATE EXISTING DATA: Copy activeDid from settings to active_identity + -- This prevents data loss when migration runs on existing databases + UPDATE active_identity + SET activeDid = (SELECT activeDid FROM settings WHERE id = 1), + lastUpdated = datetime('now') + WHERE id = 1 + AND EXISTS (SELECT 1 FROM settings WHERE id = 1 AND activeDid IS NOT NULL AND activeDid != ''); + + -- CLEANUP: Remove orphaned settings records and clear legacy activeDid values + -- This completes the migration from settings-based to table-based active identity + -- Use guarded operations to prevent accidental data loss + DELETE FROM settings WHERE accountDid IS NULL AND id != 1; + UPDATE settings SET activeDid = NULL WHERE id = 1 AND EXISTS ( + SELECT 1 FROM active_identity WHERE id = 1 AND activeDid IS NOT NULL + ); +`; + // Each migration can include multiple SQL statements (with semicolons) const MIGRATIONS = [ { @@ -136,47 +182,7 @@ const MIGRATIONS = [ }, { name: "004_active_identity_management", - sql: ` - -- Migration 004: active_identity_management (CONSOLIDATED) - -- Combines original migrations 004, 005, and 006 into single atomic operation - -- CRITICAL SECURITY: Uses ON DELETE RESTRICT constraint from the start - -- Assumes master code deployed with migration 003 (hasBackedUpSeed) - - -- Enable foreign key constraints for data integrity - PRAGMA foreign_keys = ON; - - -- Add UNIQUE constraint to accounts.did for foreign key support - CREATE UNIQUE INDEX IF NOT EXISTS idx_accounts_did_unique ON accounts(did); - - -- Create active_identity table with SECURE constraint (ON DELETE RESTRICT) - -- This prevents accidental account deletion - critical security feature - CREATE TABLE IF NOT EXISTS active_identity ( - id INTEGER PRIMARY KEY CHECK (id = 1), - activeDid TEXT REFERENCES accounts(did) ON DELETE RESTRICT, - lastUpdated TEXT NOT NULL DEFAULT (datetime('now')) - ); - - -- Add performance indexes - CREATE UNIQUE INDEX IF NOT EXISTS idx_active_identity_single_record ON active_identity(id); - - -- Seed singleton row (only if not already exists) - INSERT INTO active_identity (id, activeDid, lastUpdated) - SELECT 1, NULL, datetime('now') - WHERE NOT EXISTS (SELECT 1 FROM active_identity WHERE id = 1); - - -- MIGRATE EXISTING DATA: Copy activeDid from settings to active_identity - -- This prevents data loss when migration runs on existing databases - UPDATE active_identity - SET activeDid = (SELECT activeDid FROM settings WHERE id = 1), - lastUpdated = datetime('now') - WHERE id = 1 - AND EXISTS (SELECT 1 FROM settings WHERE id = 1 AND activeDid IS NOT NULL AND activeDid != ''); - - -- CLEANUP: Remove orphaned settings records and clear legacy activeDid values - -- This completes the migration from settings-based to table-based active identity - DELETE FROM settings WHERE accountDid IS NULL; - UPDATE settings SET activeDid = NULL; - `, + sql: MIG_004_SQL, }, ]; diff --git a/src/services/migrationService.ts b/src/services/migrationService.ts index 14a8f2ca..107dc311 100644 --- a/src/services/migrationService.ts +++ b/src/services/migrationService.ts @@ -649,15 +649,15 @@ export async function runMigrations( // Check 1: Is it recorded as applied in migrations table? const isRecordedAsApplied = appliedMigrations.has(migration.name); - // Check 2: Does the schema already exist in the database? - const isSchemaPresent = await isSchemaAlreadyPresent(migration, sqlQuery); - - // Skip if already recorded as applied + // Skip if already recorded as applied (name-only check) if (isRecordedAsApplied) { skippedCount++; continue; } + // Check 2: Does the schema already exist in the database? + const isSchemaPresent = await isSchemaAlreadyPresent(migration, sqlQuery); + // Handle case where schema exists but isn't recorded if (isSchemaPresent) { try { @@ -687,46 +687,58 @@ export async function runMigrations( } try { - // Execute the migration SQL as single atomic operation + // Execute the migration SQL as single atomic operation with transaction if (isDevelopment) { migrationLog(`🔧 [Migration] Executing SQL for: ${migration.name}`); migrationLog(`🔧 [Migration] SQL content: ${migration.sql}`); } - const execResult = await sqlExec(migration.sql); + // Begin transaction for atomic execution + await sqlExec("BEGIN IMMEDIATE"); + + try { + const execResult = await sqlExec(migration.sql); - if (isDevelopment) { - migrationLog( - `🔧 [Migration] SQL execution result: ${JSON.stringify(execResult)}`, - ); - } + if (isDevelopment) { + migrationLog( + `🔧 [Migration] SQL execution result: ${JSON.stringify(execResult)}`, + ); + } - // Validate the migration was applied correctly (only in development) - if (isDevelopment) { - const validation = await validateMigrationApplication( - migration, - sqlQuery, - ); - if (!validation.isValid) { - logger.warn( - `⚠️ [Migration] Validation failed for ${migration.name}:`, - validation.errors, + // Validate the migration was applied correctly (only in development) + if (isDevelopment) { + const validation = await validateMigrationApplication( + migration, + sqlQuery, ); + if (!validation.isValid) { + logger.warn( + `⚠️ [Migration] Validation failed for ${migration.name}:`, + validation.errors, + ); + } } - } - // Record that the migration was applied - await sqlExec("INSERT INTO migrations (name) VALUES (?)", [ - migration.name, - ]); + // Record that the migration was applied + await sqlExec("INSERT INTO migrations (name) VALUES (?)", [ + migration.name, + ]); + + // Commit transaction + await sqlExec("COMMIT"); - // Only log success in development - if (isDevelopment) { - migrationLog( - `🎉 [Migration] Successfully applied: ${migration.name}`, - ); + // Only log success in development + if (isDevelopment) { + migrationLog( + `🎉 [Migration] Successfully applied: ${migration.name}`, + ); + } + appliedCount++; + } catch (error) { + // Rollback transaction on any error + await sqlExec("ROLLBACK"); + throw error; } - appliedCount++; } catch (error) { logger.error(`❌ [Migration] Error applying ${migration.name}:`, error);