fix: Add pre-migration orphan cleanup to avoid chicken-and-egg failure (bd-eko4)

When the database has orphaned foreign key references (dependencies or labels
pointing to non-existent issues), the migration invariant check would fail,
preventing the database from opening. This created a chicken-and-egg problem:

1. bd doctor --fix tries to open the database
2. Opening triggers migrations with invariant checks
3. Invariant check fails due to orphaned refs
4. Fix never runs because database won't open

The fix adds CleanOrphanedRefs() that runs BEFORE captureSnapshot() in
RunMigrations. This automatically cleans up orphaned dependencies and labels
(preserving external:* refs), allowing the database to open normally.

Added test coverage for the cleanup function.
This commit is contained in:
Steve Yegge
2025-12-29 13:46:47 -08:00
parent 819208b8c1
commit 64d5f20b90
3 changed files with 155 additions and 0 deletions

View File

@@ -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
}

View File

@@ -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()

View File

@@ -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)