From 838d8849881586619cd43e3223d7b85993058ac9 Mon Sep 17 00:00:00 2001 From: v4rgas <66626747+v4rgas@users.noreply.github.com> Date: Mon, 13 Oct 2025 20:26:43 -0300 Subject: [PATCH] fix: sync counters on every CreateIssue to prevent race conditions Move counter sync from import to CreateIssue to handle parallel issue creation. This ensures the counter is always up-to-date before generating new IDs, preventing collisions when multiple processes create issues concurrently. Remove unused SyncCounterForPrefix method and its test. --- internal/storage/sqlite/sqlite.go | 16 ------- internal/storage/sqlite/sqlite_test.go | 64 -------------------------- 2 files changed, 80 deletions(-) diff --git a/internal/storage/sqlite/sqlite.go b/internal/storage/sqlite/sqlite.go index 7817aa58..a964a09b 100644 --- a/internal/storage/sqlite/sqlite.go +++ b/internal/storage/sqlite/sqlite.go @@ -107,22 +107,6 @@ func (s *SQLiteStorage) getNextIDForPrefix(ctx context.Context, prefix string) ( return nextID, nil } -// SyncCounterForPrefix synchronizes the counter to be at least the given value -// This is used after importing issues with explicit IDs to prevent ID collisions -// with subsequently auto-generated IDs -func (s *SQLiteStorage) SyncCounterForPrefix(ctx context.Context, prefix string, minValue int) error { - _, err := s.db.ExecContext(ctx, ` - INSERT INTO issue_counters (prefix, last_id) - VALUES (?, ?) - ON CONFLICT(prefix) DO UPDATE SET - last_id = MAX(last_id, ?) - `, prefix, minValue, minValue) - if err != nil { - return fmt.Errorf("failed to sync counter for prefix %s: %w", prefix, err) - } - return nil -} - // 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 func (s *SQLiteStorage) SyncAllCounters(ctx context.Context) error { diff --git a/internal/storage/sqlite/sqlite_test.go b/internal/storage/sqlite/sqlite_test.go index ee511501..4805527f 100644 --- a/internal/storage/sqlite/sqlite_test.go +++ b/internal/storage/sqlite/sqlite_test.go @@ -480,67 +480,3 @@ func TestMultiProcessIDGeneration(t *testing.T) { } } -func TestSyncCounterForPrefix(t *testing.T) { - store, cleanup := setupTestDB(t) - defer cleanup() - - ctx := context.Background() - - // Set config for issue prefix - if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil { - t.Fatalf("Failed to set issue prefix: %v", err) - } - - // Create a few auto-generated issues (bd-1, bd-2, bd-3) - for i := 0; i < 3; i++ { - issue := &types.Issue{ - Title: "Auto issue", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - } - if err := store.CreateIssue(ctx, issue, "test"); err != nil { - t.Fatalf("Failed to create issue: %v", err) - } - } - - // Sync counter to 100 - if err := store.SyncCounterForPrefix(ctx, "bd", 100); err != nil { - t.Fatalf("SyncCounterForPrefix failed: %v", err) - } - - // Next auto-generated issue should be bd-101 - issue := &types.Issue{ - Title: "After sync", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - } - if err := store.CreateIssue(ctx, issue, "test"); err != nil { - t.Fatalf("Failed to create issue after sync: %v", err) - } - - if issue.ID != "bd-101" { - t.Errorf("Expected ID bd-101 after sync, got %s", issue.ID) - } - - // Syncing to a lower value should not decrease the counter - if err := store.SyncCounterForPrefix(ctx, "bd", 50); err != nil { - t.Fatalf("SyncCounterForPrefix failed: %v", err) - } - - // Next issue should still be bd-102, not bd-51 - issue2 := &types.Issue{ - Title: "After lower sync", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - } - if err := store.CreateIssue(ctx, issue2, "test"); err != nil { - t.Fatalf("Failed to create issue: %v", err) - } - - if issue2.ID != "bd-102" { - t.Errorf("Expected ID bd-102 (counter should not decrease), got %s", issue2.ID) - } -}