diff --git a/internal/storage/sqlite/migration_invariants.go b/internal/storage/sqlite/migration_invariants.go index b1e3b105..34baf56a 100644 --- a/internal/storage/sqlite/migration_invariants.go +++ b/internal/storage/sqlite/migration_invariants.go @@ -205,3 +205,46 @@ func GetInvariantNames() []string { sort.Strings(names) return names } + +// CleanOrphanedRefs removes orphaned dependencies and labels that reference non-existent issues. +// This runs BEFORE migrations to prevent the chicken-and-egg problem where: +// 1. bd doctor --fix tries to open the database +// 2. Opening triggers migrations with invariant checks +// 3. Invariant check fails due to orphaned refs from prior tombstone deletion +// 4. Fix never runs because database won't open +// +// Returns counts of cleaned items for logging. +func CleanOrphanedRefs(db *sql.DB) (deps int, labels int, err error) { + // Clean orphaned dependencies (issue_id not in issues) + result, err := db.Exec(` + DELETE FROM dependencies + WHERE NOT EXISTS (SELECT 1 FROM issues WHERE id = issue_id) + `) + if err != nil { + return 0, 0, fmt.Errorf("failed to clean orphaned dependencies (issue_id): %w", err) + } + depsIssue, _ := result.RowsAffected() + + // Clean orphaned dependencies (depends_on_id not in issues, excluding external refs) + result, err = db.Exec(` + DELETE FROM dependencies + WHERE NOT EXISTS (SELECT 1 FROM issues WHERE id = depends_on_id) + AND depends_on_id NOT LIKE 'external:%' + `) + if err != nil { + return 0, 0, fmt.Errorf("failed to clean orphaned dependencies (depends_on_id): %w", err) + } + depsDependsOn, _ := result.RowsAffected() + + // Clean orphaned labels (issue_id not in issues) + result, err = db.Exec(` + DELETE FROM labels + WHERE NOT EXISTS (SELECT 1 FROM issues WHERE id = issue_id) + `) + if err != nil { + return 0, 0, fmt.Errorf("failed to clean orphaned labels: %w", err) + } + labelsCount, _ := result.RowsAffected() + + return int(depsIssue + depsDependsOn), int(labelsCount), nil +} diff --git a/internal/storage/sqlite/migration_invariants_test.go b/internal/storage/sqlite/migration_invariants_test.go index bf600764..7b201146 100644 --- a/internal/storage/sqlite/migration_invariants_test.go +++ b/internal/storage/sqlite/migration_invariants_test.go @@ -248,6 +248,111 @@ func TestGetInvariantNames(t *testing.T) { } } +func TestCleanOrphanedRefs(t *testing.T) { + db := setupInvariantTestDB(t) + defer db.Close() + + // Create a valid issue + _, err := db.Exec(`INSERT INTO issues (id, title) VALUES ('test-1', 'Test Issue')`) + if err != nil { + t.Fatalf("failed to insert test issue: %v", err) + } + + // Create valid dependency and label + _, err = db.Exec(`INSERT INTO dependencies (issue_id, depends_on_id, created_by) VALUES ('test-1', 'test-1', 'test')`) + if err != nil { + t.Fatalf("failed to insert valid dependency: %v", err) + } + + _, err = db.Exec(`INSERT INTO labels (issue_id, label) VALUES ('test-1', 'valid-label')`) + if err != nil { + t.Fatalf("failed to insert valid label: %v", err) + } + + // Disable FK constraints to create orphaned refs + _, err = db.Exec(`PRAGMA foreign_keys = OFF`) + if err != nil { + t.Fatalf("failed to disable foreign keys: %v", err) + } + + // Create orphaned dependency (issue_id not in issues) + _, err = db.Exec(`INSERT INTO dependencies (issue_id, depends_on_id, created_by) VALUES ('orphan-1', 'test-1', 'test')`) + if err != nil { + t.Fatalf("failed to insert orphaned dependency (issue_id): %v", err) + } + + // Create orphaned dependency (depends_on_id not in issues) + _, err = db.Exec(`INSERT INTO dependencies (issue_id, depends_on_id, created_by) VALUES ('test-1', 'orphan-2', 'test')`) + if err != nil { + t.Fatalf("failed to insert orphaned dependency (depends_on_id): %v", err) + } + + // Create external dependency (should NOT be cleaned) + _, err = db.Exec(`INSERT INTO dependencies (issue_id, depends_on_id, created_by) VALUES ('test-1', 'external:proj:cap', 'test')`) + if err != nil { + t.Fatalf("failed to insert external dependency: %v", err) + } + + // Create orphaned label + _, err = db.Exec(`INSERT INTO labels (issue_id, label) VALUES ('orphan-3', 'orphan-label')`) + if err != nil { + t.Fatalf("failed to insert orphaned label: %v", err) + } + + _, err = db.Exec(`PRAGMA foreign_keys = ON`) + if err != nil { + t.Fatalf("failed to enable foreign keys: %v", err) + } + + // Verify we have orphaned refs (checkForeignKeys should fail) + err = checkForeignKeys(db, &Snapshot{}) + if err == nil { + t.Error("expected checkForeignKeys to fail with orphaned refs") + } + + // Run cleanup + deps, labels, err := CleanOrphanedRefs(db) + if err != nil { + t.Fatalf("CleanOrphanedRefs failed: %v", err) + } + + // Should have cleaned 2 orphaned deps (orphan-1 and orphan-2, not external) + if deps != 2 { + t.Errorf("expected 2 orphaned deps cleaned, got %d", deps) + } + + // Should have cleaned 1 orphaned label + if labels != 1 { + t.Errorf("expected 1 orphaned label cleaned, got %d", labels) + } + + // Verify checkForeignKeys now passes + err = checkForeignKeys(db, &Snapshot{}) + if err != nil { + t.Errorf("expected checkForeignKeys to pass after cleanup, got: %v", err) + } + + // Verify valid data is still present + var depCount, labelCount int + err = db.QueryRow(`SELECT COUNT(*) FROM dependencies`).Scan(&depCount) + if err != nil { + t.Fatalf("failed to count dependencies: %v", err) + } + // Should have 2: valid self-ref + external ref + if depCount != 2 { + t.Errorf("expected 2 dependencies remaining, got %d", depCount) + } + + err = db.QueryRow(`SELECT COUNT(*) FROM labels`).Scan(&labelCount) + if err != nil { + t.Fatalf("failed to count labels: %v", err) + } + // Should have 1: valid label + if labelCount != 1 { + t.Errorf("expected 1 label remaining, got %d", labelCount) + } +} + // setupInvariantTestDB creates an in-memory test database with schema func setupInvariantTestDB(t *testing.T) *sql.DB { t.Helper() diff --git a/internal/storage/sqlite/migrations.go b/internal/storage/sqlite/migrations.go index ceba5a79..bf2e13d6 100644 --- a/internal/storage/sqlite/migrations.go +++ b/internal/storage/sqlite/migrations.go @@ -142,6 +142,13 @@ func RunMigrations(db *sql.DB) error { } }() + // Pre-migration cleanup: remove orphaned refs that would fail invariant checks. + // This prevents the chicken-and-egg problem where the database can't open + // due to orphans left behind by tombstone deletion (see bd-eko4). + if _, _, err := CleanOrphanedRefs(db); err != nil { + return fmt.Errorf("pre-migration orphan cleanup failed: %w", err) + } + snapshot, err := captureSnapshot(db) if err != nil { return fmt.Errorf("failed to capture pre-migration snapshot: %w", err)