From d85ff17f40cc2ae467ded50ea93ab4183a0db7e5 Mon Sep 17 00:00:00 2001 From: v4rgas <66626747+v4rgas@users.noreply.github.com> Date: Mon, 13 Oct 2025 18:54:39 -0300 Subject: [PATCH 1/7] test: add failing test for multi-process ID generation race Add TestMultiProcessIDGeneration to reproduce the bug where multiple bd create processes fail with UNIQUE constraint errors when run simultaneously. Each goroutine opens a separate database connection to simulate independent processes. Test currently fails with 17/20 processes getting UNIQUE constraint errors, confirming the race condition in the in-memory ID counter. --- internal/storage/sqlite/sqlite_test.go | 90 +++++++++++++++++++++++++- 1 file changed, 89 insertions(+), 1 deletion(-) diff --git a/internal/storage/sqlite/sqlite_test.go b/internal/storage/sqlite/sqlite_test.go index 6bfe997e..2743934c 100644 --- a/internal/storage/sqlite/sqlite_test.go +++ b/internal/storage/sqlite/sqlite_test.go @@ -359,7 +359,7 @@ func TestConcurrentIDGeneration(t *testing.T) { results := make(chan result, numIssues) - // Create issues concurrently + // Create issues concurrently (goroutines, not processes) for i := 0; i < numIssues; i++ { go func(n int) { issue := &types.Issue{ @@ -391,3 +391,91 @@ func TestConcurrentIDGeneration(t *testing.T) { t.Errorf("Expected %d unique IDs, got %d", numIssues, len(ids)) } } + +// TestMultiProcessIDGeneration tests ID generation across multiple processes +// This test simulates the real-world scenario of multiple `bd create` commands +// running in parallel, which is what triggers the race condition. +func TestMultiProcessIDGeneration(t *testing.T) { + // Create temporary directory + tmpDir, err := os.MkdirTemp("", "beads-multiprocess-test-*") + if err != nil { + t.Fatalf("failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + dbPath := filepath.Join(tmpDir, "test.db") + + // Initialize database + store, err := New(dbPath) + if err != nil { + t.Fatalf("failed to create storage: %v", err) + } + store.Close() + + // Spawn multiple processes that each open the DB and create an issue + const numProcesses = 20 + type result struct { + id string + err error + } + + results := make(chan result, numProcesses) + + for i := 0; i < numProcesses; i++ { + go func(n int) { + // Each goroutine simulates a separate process by opening a new connection + procStore, err := New(dbPath) + if err != nil { + results <- result{err: err} + return + } + defer procStore.Close() + + ctx := context.Background() + issue := &types.Issue{ + Title: "Multi-process test", + Status: types.StatusOpen, + Priority: 2, + IssueType: types.TypeTask, + } + + err = procStore.CreateIssue(ctx, issue, "test-user") + results <- result{id: issue.ID, err: err} + }(i) + } + + // Collect results + ids := make(map[string]bool) + var errors []error + + for i := 0; i < numProcesses; i++ { + res := <-results + if res.err != nil { + errors = append(errors, res.err) + continue + } + if ids[res.id] { + t.Errorf("Duplicate ID generated: %s", res.id) + } + ids[res.id] = true + } + + // With the bug, we expect UNIQUE constraint errors + if len(errors) > 0 { + t.Logf("Got %d errors (expected with current implementation):", len(errors)) + for _, err := range errors { + t.Logf(" - %v", err) + } + } + + t.Logf("Successfully created %d unique issues out of %d attempts", len(ids), numProcesses) + + // After the fix, all should succeed + if len(ids) != numProcesses { + t.Errorf("Expected %d unique IDs, got %d", numProcesses, len(ids)) + } + + if len(errors) > 0 { + t.Errorf("Expected no errors, got %d", len(errors)) + } +} 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 2/7] 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 From 187d291647ad1a91dec399b08395164014e3157a Mon Sep 17 00:00:00 2001 From: v4rgas <66626747+v4rgas@users.noreply.github.com> Date: Mon, 13 Oct 2025 19:34:46 -0300 Subject: [PATCH 3/7] test: add failing test for ID counter sync after import TestImportCounterSyncAfterHighID demonstrates that importing an issue with a high explicit ID (bd-100) doesn't sync the auto-increment counter, causing the next auto-generated ID to be bd-4 instead of bd-101. This test currently fails and documents the expected behavior. --- cmd/bd/import_collision_test.go | 69 +++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/cmd/bd/import_collision_test.go b/cmd/bd/import_collision_test.go index 177740bb..80dfc6d9 100644 --- a/cmd/bd/import_collision_test.go +++ b/cmd/bd/import_collision_test.go @@ -968,3 +968,72 @@ func TestImportWithDependenciesInJSONL(t *testing.T) { t.Errorf("Dependency target = %s, want bd-1", deps[0].DependsOnID) } } + +func TestImportCounterSyncAfterHighID(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "bd-collision-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer func() { + if err := os.RemoveAll(tmpDir); err != nil { + t.Logf("Warning: cleanup failed: %v", err) + } + }() + + dbPath := filepath.Join(tmpDir, "test.db") + testStore, err := sqlite.New(dbPath) + if err != nil { + t.Fatalf("Failed to create storage: %v", err) + } + defer func() { + if err := testStore.Close(); err != nil { + t.Logf("Warning: failed to close store: %v", err) + } + }() + + ctx := context.Background() + + if err := testStore.SetConfig(ctx, "issue_prefix", "bd"); err != nil { + t.Fatalf("Failed to set issue prefix: %v", err) + } + + for i := 0; i < 3; i++ { + issue := &types.Issue{ + Title: fmt.Sprintf("Auto issue %d", i+1), + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + if err := testStore.CreateIssue(ctx, issue, "test"); err != nil { + t.Fatalf("Failed to create auto issue %d: %v", i+1, err) + } + } + + highIDIssue := &types.Issue{ + ID: "bd-100", + Title: "High ID issue", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + } + + if err := testStore.CreateIssue(ctx, highIDIssue, "import"); err != nil { + t.Fatalf("Failed to import high ID issue: %v", err) + } + + newIssue := &types.Issue{ + Title: "New issue after import", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + if err := testStore.CreateIssue(ctx, newIssue, "test"); err != nil { + t.Fatalf("Failed to create new issue: %v", err) + } + + if newIssue.ID != "bd-101" { + t.Errorf("Expected new issue to get ID bd-101, got %s", newIssue.ID) + } +} From 73f5acadfaf87559035c59b4696cdc4e8855e741 Mon Sep 17 00:00:00 2001 From: v4rgas <66626747+v4rgas@users.noreply.github.com> Date: Mon, 13 Oct 2025 19:56:34 -0300 Subject: [PATCH 4/7] fix: sync ID counters after import to prevent collisions When importing issues with explicit high IDs (e.g., bd-100), the issue_counters table wasn't being updated. This caused the next auto-generated issue to collide with existing IDs (bd-4 instead of bd-101). Changes: - Add SyncAllCounters() to scan all issues and update counters atomically - Add SyncCounterForPrefix() for granular counter synchronization - Call SyncAllCounters() in import command after creating issues - Add comprehensive tests for counter sync functionality - Update TestImportCounterSyncAfterHighID to verify fix The fix uses a single efficient SQL query to prevent ID collisions with subsequently auto-generated issues. --- cmd/bd/import.go | 7 +++ cmd/bd/import_collision_test.go | 7 +++ internal/storage/sqlite/sqlite.go | 16 +++++++ internal/storage/sqlite/sqlite_test.go | 65 ++++++++++++++++++++++++++ 4 files changed, 95 insertions(+) diff --git a/cmd/bd/import.go b/cmd/bd/import.go index 8731fa55..b1988efb 100644 --- a/cmd/bd/import.go +++ b/cmd/bd/import.go @@ -238,6 +238,13 @@ Behavior: } } + // Phase 4.5: Sync ID counters after importing issues with explicit IDs + // This prevents ID collisions with subsequently auto-generated issues + if err := sqliteStore.SyncAllCounters(ctx); err != nil { + fmt.Fprintf(os.Stderr, "Warning: failed to sync ID counters: %v\n", err) + // Don't exit - this is not fatal, just a warning + } + // Phase 5: Process dependencies // Do this after all issues are created to handle forward references var depsCreated, depsSkipped int diff --git a/cmd/bd/import_collision_test.go b/cmd/bd/import_collision_test.go index 80dfc6d9..c3537521 100644 --- a/cmd/bd/import_collision_test.go +++ b/cmd/bd/import_collision_test.go @@ -1023,6 +1023,13 @@ func TestImportCounterSyncAfterHighID(t *testing.T) { t.Fatalf("Failed to import high ID issue: %v", err) } + // Step 4: Sync counters after import (mimics import command behavior) + if err := testStore.SyncAllCounters(ctx); err != nil { + t.Fatalf("Failed to sync counters: %v", err) + } + + // Step 5: Create another auto-generated issue + // This should get bd-101 (counter should have synced to 100), not bd-4 newIssue := &types.Issue{ Title: "New issue after import", Status: types.StatusOpen, diff --git a/internal/storage/sqlite/sqlite.go b/internal/storage/sqlite/sqlite.go index a964a09b..7817aa58 100644 --- a/internal/storage/sqlite/sqlite.go +++ b/internal/storage/sqlite/sqlite.go @@ -107,6 +107,22 @@ 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 2743934c..ee511501 100644 --- a/internal/storage/sqlite/sqlite_test.go +++ b/internal/storage/sqlite/sqlite_test.go @@ -479,3 +479,68 @@ func TestMultiProcessIDGeneration(t *testing.T) { t.Errorf("Expected no errors, got %d", len(errors)) } } + +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) + } +} From 64c13c759bff8f11c14c11bdf09859ddf8ea2b22 Mon Sep 17 00:00:00 2001 From: v4rgas <66626747+v4rgas@users.noreply.github.com> Date: Mon, 13 Oct 2025 20:00:01 -0300 Subject: [PATCH 5/7] chore: restore .beads folder from main --- .beads/BUG-FOUND-getNextID.md | 96 +++++++++++ .beads/bd-9-child-issues.txt | 86 ++++++++++ .beads/bd-9-design.md | 303 ++++++++++++++++++++++++++++++++++ 3 files changed, 485 insertions(+) create mode 100644 .beads/BUG-FOUND-getNextID.md create mode 100644 .beads/bd-9-child-issues.txt create mode 100644 .beads/bd-9-design.md diff --git a/.beads/BUG-FOUND-getNextID.md b/.beads/BUG-FOUND-getNextID.md new file mode 100644 index 00000000..cf392a93 --- /dev/null +++ b/.beads/BUG-FOUND-getNextID.md @@ -0,0 +1,96 @@ +# 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 new file mode 100644 index 00000000..dd9a3e66 --- /dev/null +++ b/.beads/bd-9-child-issues.txt @@ -0,0 +1,86 @@ +# 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 new file mode 100644 index 00000000..2cda8f3c --- /dev/null +++ b/.beads/bd-9-design.md @@ -0,0 +1,303 @@ +# 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. From 367259168db062a31d35e5db0a19fae3586b8658 Mon Sep 17 00:00:00 2001 From: v4rgas <66626747+v4rgas@users.noreply.github.com> Date: Mon, 13 Oct 2025 20:13:44 -0300 Subject: [PATCH 6/7] =?UTF-8?q?fix:=20renumber=20phases=20in=20import.go?= =?UTF-8?q?=20(Phase=204.5=20=E2=86=92=20Phase=205,=20Phase=205=20?= =?UTF-8?q?=E2=86=92=20Phase=206)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- cmd/bd/import.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/bd/import.go b/cmd/bd/import.go index b1988efb..92f46b94 100644 --- a/cmd/bd/import.go +++ b/cmd/bd/import.go @@ -238,14 +238,14 @@ Behavior: } } - // Phase 4.5: Sync ID counters after importing issues with explicit IDs + // Phase 5: Sync ID counters after importing issues with explicit IDs // This prevents ID collisions with subsequently auto-generated issues if err := sqliteStore.SyncAllCounters(ctx); err != nil { fmt.Fprintf(os.Stderr, "Warning: failed to sync ID counters: %v\n", err) // Don't exit - this is not fatal, just a warning } - // Phase 5: Process dependencies + // Phase 6: Process dependencies // Do this after all issues are created to handle forward references var depsCreated, depsSkipped int for _, issue := range allIssues { 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 7/7] 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) - } -}