From f9632c7df4d3e89836f15011963d844f822db273 Mon Sep 17 00:00:00 2001
From: jedarden <github@jedarden.com>
Date: Mon, 6 Apr 2026 11:02:08 -0400
Subject: [PATCH] fix: resolve deadlock in database migration engine

Fixed a deadlock in the Migrator where Pending() and Migrate() methods
called CurrentVersion() while holding the lock, but CurrentVersion()
also tried to acquire the same lock. Created internal currentVersionLocked()
method for lock-safe access.

Also fixed unused imports in migrations.go and fixed NOT NULL constraint
in TestMigrateFromV1 test.

All tests pass including:
- TestMigrateIdempotent: running migrations on already-migrated DB is no-op
- TestMigrateFromV1: migrating from v1 to current version
- TestMigrationRollback: failed migrations roll back cleanly
- TestPreMigrationBackup: backups created before schema changes
- TestPendingMigrations: pending migrations correctly identified
- TestCurrentVersion: version queries work for all DB states
- TestBackupPruning: old backups pruned correctly
- TestOpenDBFullSequence: full 7-phase startup sequence

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 mothership/internal/db/migrate.go      | 9 +++++++--
 mothership/internal/db/migrate_test.go | 2 +-
 mothership/internal/db/migrations.go   | 2 --
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/mothership/internal/db/migrate.go b/mothership/internal/db/migrate.go
index 6a3c904..2aba06b 100644
--- a/mothership/internal/db/migrate.go
+++ b/mothership/internal/db/migrate.go
@@ -83,7 +83,12 @@ func (m *Migrator) Register(migrations ...Migration) {
 func (m *Migrator) CurrentVersion(ctx context.Context) (int, error) {
 	m.mu.Lock()
 	defer m.mu.Unlock()
+	return m.currentVersionLocked(ctx)
+}
 
+// currentVersionLocked returns the current schema version without acquiring the lock.
+// The caller must hold m.mu.
+func (m *Migrator) currentVersionLocked(ctx context.Context) (int, error) {
 	// Check if schema_migrations table exists
 	var tableName string
 	err := m.db.QueryRowContext(ctx,
@@ -114,7 +119,7 @@ func (m *Migrator) Pending(ctx context.Context) ([]Migration, error) {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
-	current, err := m.CurrentVersion(ctx)
+	current, err := m.currentVersionLocked(ctx)
 	if err != nil {
 		return nil, err
 	}
@@ -135,7 +140,7 @@ func (m *Migrator) Migrate(ctx context.Context) error {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
-	current, err := m.CurrentVersion(ctx)
+	current, err := m.currentVersionLocked(ctx)
 	if err != nil {
 		return fmt.Errorf("get current version: %w", err)
 	}
diff --git a/mothership/internal/db/migrate_test.go b/mothership/internal/db/migrate_test.go
index 1178b5a..9c87fa1 100644
--- a/mothership/internal/db/migrate_test.go
+++ b/mothership/internal/db/migrate_test.go
@@ -130,7 +130,7 @@ func TestMigrateFromV1(t *testing.T) {
 			updated_at INTEGER NOT NULL
 		);
 
-		INSERT INTO auth (id, install_secret) VALUES (1, X'0000000000000000000000000000000000000000000000000000000000000000');
+		INSERT INTO auth (id, install_secret, updated_at) VALUES (1, X'0000000000000000000000000000000000000000000000000000000000000000', strftime('%s', 'now') * 1000);
 
 		INSERT INTO schema_migrations (version, applied_at, description)
 		VALUES (1, strftime('%s', 'now') * 1000, 'initial schema');
diff --git a/mothership/internal/db/migrations.go b/mothership/internal/db/migrations.go
index 87cf5e8..10c50b2 100644
--- a/mothership/internal/db/migrations.go
+++ b/mothership/internal/db/migrations.go
@@ -3,8 +3,6 @@ package db
 
 import (
 	"database/sql"
-	"fmt"
-	"time"
 )
 
 // AllMigrations returns the complete list of schema migrations in order.