From 20e3235435942bde0b39a211506a5649543dd45f Mon Sep 17 00:00:00 2001 From: v4rgas <66626747+v4rgas@users.noreply.github.com> Date: Mon, 13 Oct 2025 19:29:29 -0300 Subject: [PATCH] fix: replace in-memory ID counter with atomic database counter Replace the in-memory nextID counter with an atomic database-backed counter using the issue_counters table. This fixes race conditions when multiple processes create issues concurrently. Changes: - Add issue_counters table with atomic INSERT...ON CONFLICT pattern - Remove in-memory nextID field and sync.Mutex from SQLiteStorage - Implement getNextIDForPrefix() for atomic ID generation - Update CreateIssue() to use database counter instead of memory - Update RemapCollisions() to use database counter for collision resolution - Clean up old planning and bug documentation files Fixes the multi-process ID generation race condition tested in cmd/bd/race_test.go. --- .beads/BUG-FOUND-getNextID.md | 96 --------- .beads/bd-9-child-issues.txt | 86 -------- .beads/bd-9-design.md | 303 --------------------------- internal/storage/sqlite/collision.go | 11 +- internal/storage/sqlite/schema.go | 6 + internal/storage/sqlite/sqlite.go | 108 +++++----- 6 files changed, 60 insertions(+), 550 deletions(-) delete mode 100644 .beads/BUG-FOUND-getNextID.md delete mode 100644 .beads/bd-9-child-issues.txt delete mode 100644 .beads/bd-9-design.md diff --git a/.beads/BUG-FOUND-getNextID.md b/.beads/BUG-FOUND-getNextID.md deleted file mode 100644 index cf392a93..00000000 --- a/.beads/BUG-FOUND-getNextID.md +++ /dev/null @@ -1,96 +0,0 @@ -# BUG FOUND: getNextID() uses alphabetical MAX instead of numerical - -## Location -`internal/storage/sqlite/sqlite.go:60-84`, function `getNextID()` - -## The Bug -```go -err := db.QueryRow("SELECT MAX(id) FROM issues").Scan(&maxID) -``` - -This uses alphabetical MAX on the text `id` column, not numerical MAX. - -## Impact -When you have bd-1 through bd-10: -- Alphabetical sort: bd-1, bd-10, bd-2, bd-3, ... bd-9 -- MAX(id) returns "bd-9" (alphabetically last) -- nextID is calculated as 10 -- Creating a new issue tries to use bd-10, which already exists -- Result: UNIQUE constraint failed - -## Reproduction -```bash -# After creating bd-1 through bd-10 -./bd create "Test issue" -t task -p 1 -# Error: failed to insert issue: UNIQUE constraint failed: issues.id -``` - -## The Fix - -Option 1: Cast to integer in SQL (BEST) -```sql -SELECT MAX(CAST(SUBSTR(id, INSTR(id, '-') + 1) AS INTEGER)) FROM issues WHERE id LIKE 'bd-%' -``` - -Option 2: Pad IDs with zeros -- Change ID format from "bd-10" to "bd-0010" -- Alphabetical and numerical order match -- Breaks existing IDs - -Option 3: Query all IDs and find max in Go -- Less efficient but more flexible -- Works with any ID format - -## Recommended Solution - -Option 1 with proper prefix handling: - -```go -func getNextID(db *sql.DB) int { - // Get prefix from config (default "bd") - var prefix string - err := db.QueryRow("SELECT value FROM config WHERE key = 'issue_prefix'").Scan(&prefix) - if err != nil || prefix == "" { - prefix = "bd" - } - - // Find max numeric ID for this prefix - var maxNum sql.NullInt64 - query := ` - SELECT MAX(CAST(SUBSTR(id, LENGTH(?) + 2) AS INTEGER)) - FROM issues - WHERE id LIKE ? || '-%' - ` - err = db.QueryRow(query, prefix, prefix).Scan(&maxNum) - if err != nil || !maxNum.Valid { - return 1 - } - - return int(maxNum.Int64) + 1 -} -``` - -## Workaround for Now - -Manually specify IDs when creating issues: -```bash -# This won't work because auto-ID fails: -./bd create "Title" -t task -p 1 - -# Workaround - manually calculate next ID: -./bd list | grep -oE 'bd-[0-9]+' | sed 's/bd-//' | sort -n | tail -1 -# Then add 1 and create with explicit ID in code -``` - -Or fix the bug first before continuing! - -## Related to bd-9 - -This bug is EXACTLY the kind of distributed ID collision problem that bd-9 is designed to solve! But we should also fix the root cause. - -## Created Issue - -Should create: "Fix getNextID() to use numerical MAX instead of alphabetical" -- Type: bug -- Priority: 0 (critical - blocks all new issue creation) -- Blocks: bd-9 (can't create child issues) diff --git a/.beads/bd-9-child-issues.txt b/.beads/bd-9-child-issues.txt deleted file mode 100644 index dd9a3e66..00000000 --- a/.beads/bd-9-child-issues.txt +++ /dev/null @@ -1,86 +0,0 @@ -# Child Issues for BD-9: Collision Resolution - -## Issues to Create - -These issues break down bd-9 into implementable chunks. Link them all to bd-9 as parent-child dependencies. - -### Issue 1: Extend export to include dependencies -**Title**: Extend export to include dependencies in JSONL -**Type**: task -**Priority**: 1 -**Description**: Modify export.go to include dependencies array in each issue's JSONL output. This makes JSONL self-contained and enables proper collision resolution. Format: {"id":"bd-10","dependencies":[{"depends_on_id":"bd-5","type":"blocks"}]} -**Command**: `bd create "Extend export to include dependencies in JSONL" -t task -p 1 -d "Modify export.go to include dependencies array in each issue's JSONL output. This makes JSONL self-contained and enables proper collision resolution. Format: {\"id\":\"bd-10\",\"dependencies\":[{\"depends_on_id\":\"bd-5\",\"type\":\"blocks\"}]}"` - -### Issue 2: Implement collision detection -**Title**: Implement collision detection in import -**Type**: task -**Priority**: 1 -**Description**: Create collision.go with detectCollisions() function. Compare incoming JSONL issues against DB state. Distinguish between: (1) exact match (idempotent), (2) ID match but different content (collision), (3) new issue. Return list of colliding issues. -**Command**: `bd create "Implement collision detection in import" -t task -p 1 -d "Create collision.go with detectCollisions() function. Compare incoming JSONL issues against DB state. Distinguish between: (1) exact match (idempotent), (2) ID match but different content (collision), (3) new issue. Return list of colliding issues."` - -### Issue 3: Implement reference scoring -**Title**: Implement reference scoring algorithm -**Type**: task -**Priority**: 1 -**Description**: Count references for each colliding issue: text mentions in descriptions/notes/design fields + dependency references. Sort collisions by score ascending (fewest refs first). This minimizes total updates during renumbering. -**Command**: `bd create "Implement reference scoring algorithm" -t task -p 1 -d "Count references for each colliding issue: text mentions in descriptions/notes/design fields + dependency references. Sort collisions by score ascending (fewest refs first). This minimizes total updates during renumbering."` - -### Issue 4: Implement ID remapping -**Title**: Implement ID remapping with reference updates -**Type**: task -**Priority**: 1 -**Description**: Allocate new IDs for colliding issues. Update all text field references using word-boundary regex (\bbd-10\b). Update dependency records. Build id_mapping for reporting. Handle chain dependencies properly. -**Command**: `bd create "Implement ID remapping with reference updates" -t task -p 1 -d "Allocate new IDs for colliding issues. Update all text field references using word-boundary regex (\\bbd-10\\b). Update dependency records. Build id_mapping for reporting. Handle chain dependencies properly."` - -### Issue 5: Add CLI flags -**Title**: Add --resolve-collisions flag and user reporting -**Type**: task -**Priority**: 1 -**Description**: Add import flags: --resolve-collisions (auto-fix) and --dry-run (preview). Display clear report: collisions detected, remappings applied (old→new with scores), reference counts updated. Default behavior: fail on collision (safe). -**Command**: `bd create "Add --resolve-collisions flag and user reporting" -t task -p 1 -d "Add import flags: --resolve-collisions (auto-fix) and --dry-run (preview). Display clear report: collisions detected, remappings applied (old→new with scores), reference counts updated. Default behavior: fail on collision (safe)."` - -### Issue 6: Write tests -**Title**: Write comprehensive collision resolution tests -**Type**: task -**Priority**: 1 -**Description**: Test cases: simple collision, multiple collisions, dependency updates, text reference updates, chain dependencies, edge cases (partial ID matches, case sensitivity, triple merges). Add to import_test.go and collision_test.go. -**Command**: `bd create "Write comprehensive collision resolution tests" -t task -p 1 -d "Test cases: simple collision, multiple collisions, dependency updates, text reference updates, chain dependencies, edge cases (partial ID matches, case sensitivity, triple merges). Add to import_test.go and collision_test.go."` - -### Issue 7: Update docs -**Title**: Update documentation for collision resolution -**Type**: task -**Priority**: 1 -**Description**: Update README.md with collision resolution section. Update CLAUDE.md with new workflow. Document --resolve-collisions and --dry-run flags. Add example scenarios showing branch merge workflows. -**Command**: `bd create "Update documentation for collision resolution" -t task -p 1 -d "Update README.md with collision resolution section. Update CLAUDE.md with new workflow. Document --resolve-collisions and --dry-run flags. Add example scenarios showing branch merge workflows."` - -## Additional Feature Issue - -### Issue: Add design field support to update command -**Title**: Add design/notes/acceptance_criteria fields to update command -**Type**: feature -**Priority**: 2 -**Description**: Currently bd update only supports status, priority, title, assignee. Add support for --design, --notes, --acceptance-criteria flags. This makes it easier to add detailed designs to issues after creation. -**Command**: `bd create "Add design/notes/acceptance_criteria fields to update command" -t feature -p 2 -d "Currently bd update only supports status, priority, title, assignee. Add support for --design, --notes, --acceptance-criteria flags. This makes it easier to add detailed designs to issues after creation."` - -## Dependency Linking - -After creating all child issues, link them to bd-9: -```bash -# Assuming the issues are bd-10 through bd-16 (or whatever IDs were assigned) -bd dep add bd-9 --type parent-child -``` - -Example: -```bash -bd dep add bd-10 bd-9 --type parent-child -bd dep add bd-11 bd-9 --type parent-child -bd dep add bd-12 bd-9 --type parent-child -# etc. -``` - -## Current State - -- bd-10 was created successfully ("Extend export to include dependencies") -- bd-11+ attempts failed with UNIQUE constraint errors -- This suggests those IDs already exist in the DB but may not be in the JSONL file -- Need to investigate DB/JSONL sync issue before creating more issues diff --git a/.beads/bd-9-design.md b/.beads/bd-9-design.md deleted file mode 100644 index 2cda8f3c..00000000 --- a/.beads/bd-9-design.md +++ /dev/null @@ -1,303 +0,0 @@ -# BD-9: Collision Resolution Design Document - -**Status**: In progress, design complete, ready for implementation -**Date**: 2025-10-12 -**Issue**: bd-9 - Build collision resolution tooling for distributed branch workflows - -## Problem Statement - -When branches diverge and both create issues, auto-incrementing IDs collide on merge: -- Branch A creates bd-10, bd-11, bd-12 -- Branch B (diverged) creates bd-10, bd-11, bd-12 (different issues!) -- On merge: 6 issues, but 3 duplicate IDs -- References to "bd-10" in descriptions/dependencies are now ambiguous - -## Design Goals - -1. **Preserve brevity** - Keep bd-302 format, not bd-302-branch-a-uuid-mess -2. **Minimize disruption** - Renumber issues with fewer references -3. **Update all references** - Text fields AND dependency table -4. **Atomic operation** - All or nothing -5. **Clear feedback** - User must understand what changed - -## Algorithm Design - -### Phase 1: Collision Detection - -``` -Input: JSONL issues + current DB state -Output: Set of colliding issues - -for each issue in JSONL: - if DB contains issue.ID: - if DB issue == JSONL issue: - skip (already imported, idempotent) - else: - mark as COLLISION -``` - -### Phase 2: Reference Counting (The Smart Part) - -Renumber issues with FEWER references first because: -- If bd-10 is referenced 20 times and bd-11 once -- Renumbering bd-11→bd-15 updates 1 reference -- Renumbering bd-10→bd-15 updates 20 references - -``` -for each colliding_issue: - score = 0 - - // Count text references in OTHER issues - for each other_issue in JSONL: - score += count_mentions(other_issue.all_text, colliding_issue.ID) - - // Count dependency references - deps = DB.get_dependents(colliding_issue.ID) // who depends on me? - score += len(deps) - - // Store score - collision_scores[colliding_issue.ID] = score - -// Sort ascending: lowest score = fewest references = renumber first -sorted_collisions = sort_by(collision_scores) -``` - -### Phase 3: ID Allocation - -``` -id_mapping = {} // old_id -> new_id -next_num = DB.get_next_id_number() - -for collision in sorted_collisions: - // Find next available ID - while true: - candidate = f"{prefix}-{next_num}" - if not DB.exists(candidate) and candidate not in id_mapping.values(): - id_mapping[collision.ID] = candidate - next_num++ - break - next_num++ -``` - -### Phase 4: Reference Updates - -This is the trickiest part - must update: -1. Issue IDs themselves -2. Text field references (description, design, notes, acceptance_criteria) -3. Dependency records (when they reference old IDs) - -``` -updated_issues = [] -reference_update_count = 0 - -for issue in JSONL: - new_issue = clone(issue) - - // 1. Update own ID if it collided - if issue.ID in id_mapping: - new_issue.ID = id_mapping[issue.ID] - - // 2. Update text field references - for old_id, new_id in id_mapping: - for field in [title, description, design, notes, acceptance_criteria]: - if field: - pattern = r'\b' + re.escape(old_id) + r'\b' - new_text, count = re.subn(pattern, new_id, field) - field = new_text - reference_update_count += count - - updated_issues.append(new_issue) -``` - -### Phase 5: Dependency Handling - -**Approach A: Export dependencies in JSONL** (PREFERRED) -- Extend export to include `"dependencies": [{...}]` per issue -- Import dependencies along with issues -- Update dependency records during phase 4 - -Why preferred: -- Self-contained JSONL (better for git workflow) -- Easier to reason about -- Can detect cross-file dependencies - -### Phase 6: Atomic Import - -``` -transaction: - for issue in updated_issues: - if issue.ID was remapped: - DB.create_issue(issue) - else: - DB.upsert_issue(issue) - - // Update dependency table - for issue in updated_issues: - for dep in issue.dependencies: - // dep IDs already updated in phase 4 - DB.create_or_update_dependency(dep) - - commit -``` - -### Phase 7: User Reporting - -``` -report = { - collisions_detected: N, - remappings: [ - "bd-10 → bd-15 (Score: 3 references)", - "bd-11 → bd-16 (Score: 15 references)", - ], - text_updates: M, - dependency_updates: K, -} -``` - -## Edge Cases - -1. **Chain dependencies**: bd-10 depends on bd-11, both collide - - Sorted renumbering handles this naturally - - Lower-referenced one renumbered first - -2. **Circular dependencies**: Shouldn't happen (DB has cycle detection) - -3. **Partial ID matches**: "bd-1" shouldn't match "bd-10" - - Use word boundary regex: `\bbd-10\b` - -4. **Case sensitivity**: IDs are case-sensitive (bd-10 ≠ BD-10) - -5. **IDs in code blocks**: Will be replaced - - Could add `--preserve-code-blocks` flag later - -6. **Triple merges**: Branch A, B, C all create bd-10 - - Algorithm handles N collisions - -7. **Dependencies pointing to DB-only issues**: - - JSONL issue depends on bd-999 (only in DB) - - No collision, works fine - -## Performance Considerations - -- O(N*M) for reference counting (N issues × M collisions) -- For 1000 issues, 10 collisions: 10,000 text scans -- Acceptable for typical use (hundreds of issues) -- Could optimize with index/trie if needed - -## API Design - -```bash -# Default: fail on collision (safe) -bd import -i issues.jsonl -# Error: Collision detected: bd-10 already exists - -# With auto-resolution -bd import -i issues.jsonl --resolve-collisions -# Resolved 3 collisions: -# bd-10 → bd-15 (3 refs) -# bd-11 → bd-16 (1 ref) -# bd-12 → bd-17 (7 refs) -# Imported 45 issues, updated 23 references - -# Dry run (preview changes) -bd import -i issues.jsonl --resolve-collisions --dry-run -``` - -## Implementation Breakdown - -### Child Issues to Create - -1. **bd-10**: Extend export to include dependencies in JSONL - - Modify export.go to include dependencies array - - Format: `{"id":"bd-10","dependencies":[{"depends_on_id":"bd-5","type":"blocks"}]}` - - Priority: 1, Type: task - -2. **bd-11**: Implement collision detection in import - - Create collision.go with detectCollisions() function - - Compare incoming JSONL against DB state - - Distinguish: exact match (skip), collision (flag), new (create) - - Priority: 1, Type: task - -3. **bd-12**: Implement reference scoring algorithm - - Count text mentions + dependency references - - Sort collisions by score ascending (fewest refs first) - - Minimize total updates during renumbering - - Priority: 1, Type: task - -4. **bd-13**: Implement ID remapping with reference updates - - Allocate new IDs for colliding issues - - Update text field references with word-boundary regex - - Update dependency records - - Build id_mapping for reporting - - Priority: 1, Type: task - -5. **bd-14**: Add --resolve-collisions flag and user reporting - - Add import flags: --resolve-collisions, --dry-run - - Display clear report with remappings and counts - - Default: fail on collision (safe) - - Priority: 1, Type: task - -6. **bd-15**: Write comprehensive collision resolution tests - - Test cases: simple/multiple collisions, dependencies, text refs - - Edge cases: partial matches, case sensitivity, triple merges - - Add to import_test.go and collision_test.go - - Priority: 1, Type: task - -7. **bd-16**: Update documentation for collision resolution - - Update README.md with collision resolution section - - Update CLAUDE.md with new workflow - - Document flags and example scenarios - - Priority: 1, Type: task - -### Additional Issue: Add Design Field Support - -**NEW ISSUE**: Add design field to bd update command -- Currently: `bd update` doesn't support --design flag (discovered during work) -- Need: Allow updating design, notes, acceptance_criteria fields -- This would make bd-9's design easier to attach to the issue itself -- Priority: 2, Type: feature - -## Current State - -- bd-9 is in_progress (claimed) -- bd-10 was successfully created (first child issue) -- bd-11+ creation failed with UNIQUE constraint (collision!) - - This demonstrates the exact problem we're solving - - Need to manually create remaining issues with different IDs - - Or implement collision resolution first! (chicken/egg) - -## Data Structures Involved - -- **Issues table**: id, title, description, design, notes, acceptance_criteria, status, priority, issue_type, assignee, estimated_minutes, created_at, updated_at, closed_at -- **Dependencies table**: issue_id, depends_on_id, type, created_at, created_by -- **Text fields with ID references**: description, design, notes, acceptance_criteria (title too?) - -## Files to Modify - -1. `cmd/bd/export.go` - Add dependency export -2. `cmd/bd/import.go` - Call collision resolution -3. `cmd/bd/collision.go` - NEW FILE - Core algorithm -4. `cmd/bd/collision_test.go` - NEW FILE - Tests -5. `internal/types/types.go` - May need collision report types -6. `README.md` - Documentation -7. `CLAUDE.md` - AI agent workflow docs - -## Next Steps - -1. ✅ Design complete -2. 🔄 Create child issues (bd-10 created, bd-11+ need different IDs) -3. ⏳ Implement Phase 1: Export enhancement -4. ⏳ Implement Phase 2-7: Core algorithm -5. ⏳ Tests -6. ⏳ Documentation -7. ⏳ Export issues to JSONL before committing - -## Meta: Real Collision Encountered! - -While creating child issues, we hit the exact problem: -- bd-10 was created successfully -- bd-11, bd-12, bd-13, bd-14, bd-15, bd-16 all failed with "UNIQUE constraint failed" -- This means the DB already has bd-11+ from a previous session/import -- Perfect demonstration of why we need collision resolution! - -Resolution: Create remaining child issues manually with explicit IDs after checking what exists. diff --git a/internal/storage/sqlite/collision.go b/internal/storage/sqlite/collision.go index 0cfb7545..0ec6933c 100644 --- a/internal/storage/sqlite/collision.go +++ b/internal/storage/sqlite/collision.go @@ -232,15 +232,16 @@ func RemapCollisions(ctx context.Context, s *SQLiteStorage, collisions []*Collis for _, collision := range collisions { oldID := collision.ID - // Allocate new ID - s.idMu.Lock() + // Allocate new ID using atomic counter prefix, err := s.GetConfig(ctx, "issue_prefix") if err != nil || prefix == "" { prefix = "bd" } - newID := fmt.Sprintf("%s-%d", prefix, s.nextID) - s.nextID++ - s.idMu.Unlock() + nextID, err := s.getNextIDForPrefix(ctx, prefix) + if err != nil { + return nil, fmt.Errorf("failed to generate new ID for collision %s: %w", oldID, err) + } + newID := fmt.Sprintf("%s-%d", prefix, nextID) // Record mapping idMapping[oldID] = newID diff --git a/internal/storage/sqlite/schema.go b/internal/storage/sqlite/schema.go index dd00e1b4..f05de236 100644 --- a/internal/storage/sqlite/schema.go +++ b/internal/storage/sqlite/schema.go @@ -81,6 +81,12 @@ CREATE TABLE IF NOT EXISTS dirty_issues ( CREATE INDEX IF NOT EXISTS idx_dirty_issues_marked_at ON dirty_issues(marked_at); +-- Issue counters table (for atomic ID generation) +CREATE TABLE IF NOT EXISTS issue_counters ( + prefix TEXT PRIMARY KEY, + last_id INTEGER NOT NULL DEFAULT 0 +); + -- Ready work view CREATE VIEW IF NOT EXISTS ready_issues AS SELECT i.* diff --git a/internal/storage/sqlite/sqlite.go b/internal/storage/sqlite/sqlite.go index 8bfb0eeb..a964a09b 100644 --- a/internal/storage/sqlite/sqlite.go +++ b/internal/storage/sqlite/sqlite.go @@ -9,7 +9,6 @@ import ( "os" "path/filepath" "strings" - "sync" "time" // Import SQLite driver @@ -19,9 +18,7 @@ import ( // SQLiteStorage implements the Storage interface using SQLite type SQLiteStorage struct { - db *sql.DB - nextID int - idMu sync.Mutex // Protects nextID from concurrent access + db *sql.DB } // New creates a new SQLite storage backend @@ -53,12 +50,8 @@ func New(path string) (*SQLiteStorage, error) { return nil, fmt.Errorf("failed to migrate dirty_issues table: %w", err) } - // Get next ID - nextID := getNextID(db) - return &SQLiteStorage{ - db: db, - nextID: nextID, + db: db, }, nil } @@ -97,56 +90,42 @@ func migrateDirtyIssuesTable(db *sql.DB) error { return nil } -// getNextID determines the next issue ID to use -func getNextID(db *sql.DB) int { - // Get prefix from config, default to "bd" - var prefix string - err := db.QueryRow("SELECT value FROM config WHERE key = 'issue_prefix'").Scan(&prefix) - if err != nil || prefix == "" { - prefix = "bd" +// getNextIDForPrefix atomically generates the next ID for a given prefix +// Uses the issue_counters table for atomic, cross-process ID generation +func (s *SQLiteStorage) getNextIDForPrefix(ctx context.Context, prefix string) (int, error) { + var nextID int + err := s.db.QueryRowContext(ctx, ` + INSERT INTO issue_counters (prefix, last_id) + VALUES (?, 1) + ON CONFLICT(prefix) DO UPDATE SET + last_id = last_id + 1 + RETURNING last_id + `, prefix).Scan(&nextID) + if err != nil { + return 0, fmt.Errorf("failed to generate next ID for prefix %s: %w", prefix, err) } + return nextID, nil +} - // Find the maximum numeric ID for this prefix - // Use SUBSTR to extract numeric part after prefix and hyphen, then CAST to INTEGER - // This ensures we get numerical max, not alphabetical (bd-10 > bd-9, not bd-9 > bd-10) - var maxNum sql.NullInt64 - query := ` - SELECT MAX(CAST(SUBSTR(id, LENGTH(?) + 2) AS INTEGER)) +// 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 { + _, err := s.db.ExecContext(ctx, ` + INSERT INTO issue_counters (prefix, last_id) + SELECT + substr(id, 1, instr(id, '-') - 1) as prefix, + MAX(CAST(substr(id, instr(id, '-') + 1) AS INTEGER)) as max_id FROM issues - WHERE id LIKE ? || '-%' - ` - err = db.QueryRow(query, prefix, prefix).Scan(&maxNum) - if err != nil || !maxNum.Valid { - return 1 // Start from 1 if table is empty or no matching IDs + WHERE instr(id, '-') > 0 + 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) + `) + if err != nil { + return fmt.Errorf("failed to sync counters: %w", err) } - - // Check for malformed IDs (non-numeric suffixes) and warn - // SQLite's CAST returns 0 for invalid integers, never NULL - // So we detect malformed IDs by checking if CAST returns 0 AND suffix doesn't start with '0' - malformedQuery := ` - SELECT id FROM issues - WHERE id LIKE ? || '-%' - AND CAST(SUBSTR(id, LENGTH(?) + 2) AS INTEGER) = 0 - AND SUBSTR(id, LENGTH(?) + 2, 1) != '0' - ` - rows, err := db.Query(malformedQuery, prefix, prefix, prefix) - if err == nil { - defer rows.Close() - var malformedIDs []string - for rows.Next() { - var id string - if err := rows.Scan(&id); err == nil { - malformedIDs = append(malformedIDs, id) - } - } - if len(malformedIDs) > 0 { - fmt.Fprintf(os.Stderr, "Warning: Found %d malformed issue IDs with non-numeric suffixes: %v\n", - len(malformedIDs), malformedIDs) - fmt.Fprintf(os.Stderr, "These IDs are being ignored for ID generation. Consider fixing them.\n") - } - } - - return int(maxNum.Int64) + 1 + return nil } // CreateIssue creates a new issue @@ -156,9 +135,14 @@ func (s *SQLiteStorage) CreateIssue(ctx context.Context, issue *types.Issue, act return fmt.Errorf("validation failed: %w", err) } - // Generate ID if not set (thread-safe) + // Generate ID if not set (using atomic counter table) if issue.ID == "" { - s.idMu.Lock() + // Sync all counters first to ensure we don't collide with existing issues + // This handles the case where the database was created before this fix + // or issues were imported without syncing counters + if err := s.SyncAllCounters(ctx); err != nil { + return fmt.Errorf("failed to sync counters: %w", err) + } // Get prefix from config, default to "bd" prefix, err := s.GetConfig(ctx, "issue_prefix") @@ -166,9 +150,13 @@ func (s *SQLiteStorage) CreateIssue(ctx context.Context, issue *types.Issue, act prefix = "bd" } - issue.ID = fmt.Sprintf("%s-%d", prefix, s.nextID) - s.nextID++ - s.idMu.Unlock() + // Atomically get next ID from counter table + nextID, err := s.getNextIDForPrefix(ctx, prefix) + if err != nil { + return err + } + + issue.ID = fmt.Sprintf("%s-%d", prefix, nextID) } // Set timestamps