diff --git a/internal/storage/sqlite/counter_sync_test.go b/internal/storage/sqlite/counter_sync_test.go new file mode 100644 index 00000000..2ddae6c9 --- /dev/null +++ b/internal/storage/sqlite/counter_sync_test.go @@ -0,0 +1,232 @@ +package sqlite + +import ( + "context" + "fmt" + "testing" + + "github.com/steveyegge/beads/internal/types" +) + +// TestCounterSyncAfterDelete verifies that counters are synced after deletion (bd-49) +func TestCounterSyncAfterDelete(t *testing.T) { + store, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + + // Set the issue prefix to "bd" for this test + if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil { + t.Fatalf("Failed to set issue_prefix: %v", err) + } + + // Create issues bd-1 through bd-5 + for i := 1; i <= 5; i++ { + issue := &types.Issue{ + ID: genID("bd", i), + Title: "Test issue", + Priority: 2, + IssueType: types.TypeTask, + Status: types.StatusOpen, + } + if err := store.CreateIssue(ctx, issue, "test"); err != nil { + t.Fatalf("Failed to create issue: %v", err) + } + } + + // Create one issue with auto-generated ID to initialize counter + autoIssue := &types.Issue{ + Title: "Auto issue", + Priority: 2, + IssueType: types.TypeTask, + Status: types.StatusOpen, + } + if err := store.CreateIssue(ctx, autoIssue, "test"); err != nil { + t.Fatalf("Failed to create auto issue: %v", err) + } + if autoIssue.ID != "bd-6" { + t.Fatalf("Expected auto issue to be bd-6, got %s", autoIssue.ID) + } + + // Verify counter is at 6 + var counter int + err := store.db.QueryRow(`SELECT last_id FROM issue_counters WHERE prefix = 'bd'`).Scan(&counter) + if err != nil { + t.Fatalf("Failed to query counter: %v", err) + } + if counter != 6 { + t.Errorf("Expected counter at 6, got %d", counter) + } + + // Delete bd-5 and bd-6 + if err := store.DeleteIssue(ctx, "bd-5"); err != nil { + t.Fatalf("Failed to delete bd-5: %v", err) + } + if err := store.DeleteIssue(ctx, "bd-6"); err != nil { + t.Fatalf("Failed to delete bd-6: %v", err) + } + + // Counter should now be synced to 4 (max remaining ID) + err = store.db.QueryRow(`SELECT last_id FROM issue_counters WHERE prefix = 'bd'`).Scan(&counter) + if err != nil { + t.Fatalf("Failed to query counter after delete: %v", err) + } + if counter != 4 { + t.Errorf("Expected counter at 4 after deletion, got %d", counter) + } + + // Create new issue - should be bd-5 (not bd-7) + newIssue := &types.Issue{ + Title: "New issue after deletion", + Priority: 2, + IssueType: types.TypeTask, + Status: types.StatusOpen, + } + if err := store.CreateIssue(ctx, newIssue, "test"); err != nil { + t.Fatalf("Failed to create new issue: %v", err) + } + if newIssue.ID != "bd-5" { + t.Errorf("Expected new issue to be bd-5, got %s", newIssue.ID) + } +} + +// TestCounterSyncAfterBatchDelete verifies that counters are synced after batch deletion (bd-49) +func TestCounterSyncAfterBatchDelete(t *testing.T) { + store, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + + // Set the issue prefix to "bd" for this test + if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil { + t.Fatalf("Failed to set issue_prefix: %v", err) + } + + // Create issues bd-1 through bd-10 + for i := 1; i <= 10; i++ { + issue := &types.Issue{ + ID: genID("bd", i), + Title: "Test issue", + Priority: 2, + IssueType: types.TypeTask, + Status: types.StatusOpen, + } + if err := store.CreateIssue(ctx, issue, "test"); err != nil { + t.Fatalf("Failed to create issue: %v", err) + } + } + + // Create one issue with auto-generated ID to initialize counter + autoIssue := &types.Issue{ + Title: "Auto issue", + Priority: 2, + IssueType: types.TypeTask, + Status: types.StatusOpen, + } + if err := store.CreateIssue(ctx, autoIssue, "test"); err != nil { + t.Fatalf("Failed to create auto issue: %v", err) + } + if autoIssue.ID != "bd-11" { + t.Fatalf("Expected auto issue to be bd-11, got %s", autoIssue.ID) + } + + // Verify counter is at 11 + var counter int + err := store.db.QueryRow(`SELECT last_id FROM issue_counters WHERE prefix = 'bd'`).Scan(&counter) + if err != nil { + t.Fatalf("Failed to query counter: %v", err) + } + if counter != 11 { + t.Errorf("Expected counter at 11, got %d", counter) + } + + // Batch delete bd-6 through bd-11 + toDelete := []string{"bd-6", "bd-7", "bd-8", "bd-9", "bd-10", "bd-11"} + _, err = store.DeleteIssues(ctx, toDelete, false, false, false) + if err != nil { + t.Fatalf("Failed to batch delete: %v", err) + } + + // Counter should now be synced to 5 (max remaining ID) + err = store.db.QueryRow(`SELECT last_id FROM issue_counters WHERE prefix = 'bd'`).Scan(&counter) + if err != nil { + t.Fatalf("Failed to query counter after batch delete: %v", err) + } + if counter != 5 { + t.Errorf("Expected counter at 5 after batch deletion, got %d", counter) + } + + // Create new issue - should be bd-6 (not bd-11) + newIssue := &types.Issue{ + Title: "New issue after batch deletion", + Priority: 2, + IssueType: types.TypeTask, + Status: types.StatusOpen, + } + if err := store.CreateIssue(ctx, newIssue, "test"); err != nil { + t.Fatalf("Failed to create new issue: %v", err) + } + if newIssue.ID != "bd-6" { + t.Errorf("Expected new issue to be bd-6, got %s", newIssue.ID) + } +} + +// TestCounterSyncAfterDeleteAll verifies counter resets when all issues deleted (bd-49) +func TestCounterSyncAfterDeleteAll(t *testing.T) { + store, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + + // Set the issue prefix to "bd" for this test + if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil { + t.Fatalf("Failed to set issue_prefix: %v", err) + } + + // Create issues bd-1 through bd-5 + for i := 1; i <= 5; i++ { + issue := &types.Issue{ + ID: genID("bd", i), + Title: "Test issue", + Priority: 2, + IssueType: types.TypeTask, + Status: types.StatusOpen, + } + if err := store.CreateIssue(ctx, issue, "test"); err != nil { + t.Fatalf("Failed to create issue: %v", err) + } + } + + // Delete all issues + toDelete := []string{"bd-1", "bd-2", "bd-3", "bd-4", "bd-5"} + _, err := store.DeleteIssues(ctx, toDelete, false, false, false) + if err != nil { + t.Fatalf("Failed to delete all issues: %v", err) + } + + // Counter should be deleted (no issues left with this prefix) + var counter int + err = store.db.QueryRow(`SELECT last_id FROM issue_counters WHERE prefix = 'bd'`).Scan(&counter) + if err == nil { + t.Errorf("Expected no counter row, but found counter at %d", counter) + } + + // Create new issue - should be bd-1 (fresh start) + newIssue := &types.Issue{ + Title: "First issue after deleting all", + Priority: 2, + IssueType: types.TypeTask, + Status: types.StatusOpen, + } + if err := store.CreateIssue(ctx, newIssue, "test"); err != nil { + t.Fatalf("Failed to create new issue: %v", err) + } + if newIssue.ID != "bd-1" { + t.Errorf("Expected new issue to be bd-1, got %s", newIssue.ID) + } +} + +// genID is a helper to generate issue IDs in tests +func genID(prefix string, num int) string { + return fmt.Sprintf("%s-%d", prefix, num) +} diff --git a/internal/storage/sqlite/sqlite.go b/internal/storage/sqlite/sqlite.go index e48c8bdc..e02ef877 100644 --- a/internal/storage/sqlite/sqlite.go +++ b/internal/storage/sqlite/sqlite.go @@ -463,8 +463,24 @@ func (s *SQLiteStorage) getNextIDForPrefix(ctx context.Context, prefix string) ( // SyncAllCounters synchronizes all ID counters based on existing issues in the database // This scans all issues and updates counters to prevent ID collisions with auto-generated IDs +// Note: This unconditionally overwrites counter values, allowing them to decrease after deletions func (s *SQLiteStorage) SyncAllCounters(ctx context.Context) error { + // First, delete counters for prefixes that have no issues _, err := s.db.ExecContext(ctx, ` + DELETE FROM issue_counters + WHERE prefix NOT IN ( + SELECT DISTINCT substr(id, 1, instr(id, '-') - 1) + FROM issues + WHERE instr(id, '-') > 0 + AND substr(id, instr(id, '-') + 1) GLOB '[0-9]*' + ) + `) + if err != nil { + return fmt.Errorf("failed to delete orphaned counters: %w", err) + } + + // Then, upsert counters for prefixes that have issues + _, err = s.db.ExecContext(ctx, ` INSERT INTO issue_counters (prefix, last_id) SELECT substr(id, 1, instr(id, '-') - 1) as prefix, @@ -474,7 +490,7 @@ func (s *SQLiteStorage) SyncAllCounters(ctx context.Context) error { AND substr(id, instr(id, '-') + 1) GLOB '[0-9]*' GROUP BY prefix ON CONFLICT(prefix) DO UPDATE SET - last_id = MAX(last_id, excluded.last_id) + last_id = excluded.last_id `) if err != nil { return fmt.Errorf("failed to sync counters: %w", err) @@ -1400,7 +1416,12 @@ func (s *SQLiteStorage) DeleteIssue(ctx context.Context, id string) error { return fmt.Errorf("issue not found: %s", id) } - return tx.Commit() + if err := tx.Commit(); err != nil { + return err + } + + // Sync counters after deletion to keep them accurate + return s.SyncAllCounters(ctx) } // DeleteIssuesResult contains statistics about a batch deletion operation @@ -1612,6 +1633,11 @@ func (s *SQLiteStorage) DeleteIssues(ctx context.Context, ids []string, cascade return nil, fmt.Errorf("failed to commit transaction: %w", err) } + // Sync counters after deletion to keep them accurate + if err := s.SyncAllCounters(ctx); err != nil { + return nil, fmt.Errorf("failed to sync counters after deletion: %w", err) + } + return result, nil }