From 55c722a3e3b82634ff3d91f2488baf14c28cb0d6 Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Sun, 2 Nov 2025 15:27:59 -0800 Subject: [PATCH] Implement external_ref as primary matching key for import updates (bd-1022) - Add GetIssueByExternalRef() query function to storage interface and implementations - Update DetectCollisions() to prioritize external_ref matching over ID matching - Modify upsertIssues() to handle external_ref matches in import logic - Add index on external_ref column for performance - Add comprehensive tests for external_ref matching in both collision detection and import - Enables re-syncing from external systems (Jira, GitHub, Linear) without duplicates - Preserves local issues (no external_ref) from being overwritten --- internal/importer/external_ref_test.go | 368 +++++++++++++++++++ internal/importer/importer.go | 62 +++- internal/storage/memory/memory.go | 29 ++ internal/storage/sqlite/collision.go | 36 +- internal/storage/sqlite/external_ref_test.go | 281 ++++++++++++++ internal/storage/sqlite/schema.go | 1 + internal/storage/sqlite/sqlite.go | 70 ++++ internal/storage/storage.go | 1 + 8 files changed, 839 insertions(+), 9 deletions(-) create mode 100644 internal/importer/external_ref_test.go create mode 100644 internal/storage/sqlite/external_ref_test.go diff --git a/internal/importer/external_ref_test.go b/internal/importer/external_ref_test.go new file mode 100644 index 00000000..7fc64648 --- /dev/null +++ b/internal/importer/external_ref_test.go @@ -0,0 +1,368 @@ +package importer + +import ( + "context" + "path/filepath" + "testing" + "time" + + "github.com/steveyegge/beads/internal/storage/sqlite" + "github.com/steveyegge/beads/internal/types" +) + +func TestImportWithExternalRef(t *testing.T) { + ctx := context.Background() + tmpDir := t.TempDir() + dbPath := filepath.Join(tmpDir, "test.db") + + // Create database + store, err := sqlite.New(dbPath) + if err != nil { + t.Fatalf("Failed to create database: %v", err) + } + defer store.Close() + + // Set prefix + if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil { + t.Fatalf("Failed to set prefix: %v", err) + } + + // Create initial issue with external_ref + externalRef := "JIRA-100" + initial := &types.Issue{ + ID: "bd-test-1", + Title: "Initial title", + Description: "Initial description", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeBug, + ExternalRef: &externalRef, + CreatedAt: time.Now().Add(-2 * time.Hour), + UpdatedAt: time.Now().Add(-2 * time.Hour), + } + + err = store.CreateIssue(ctx, initial, "test") + if err != nil { + t.Fatalf("Failed to create initial issue: %v", err) + } + + // Import updated issue with same external_ref but different content + updated := &types.Issue{ + ID: "bd-test-1", // Same ID + Title: "Updated title from Jira", + Description: "Updated description from Jira", + Status: types.StatusInProgress, + Priority: 2, + IssueType: types.TypeBug, + ExternalRef: &externalRef, // Same external_ref + CreatedAt: initial.CreatedAt, + UpdatedAt: time.Now(), // Newer timestamp + } + + opts := Options{ + DryRun: false, + SkipUpdate: false, + SkipPrefixValidation: true, + } + + result, err := ImportIssues(ctx, dbPath, store, []*types.Issue{updated}, opts) + if err != nil { + t.Fatalf("ImportIssues failed: %v", err) + } + + // Should have updated 1 issue + if result.Updated != 1 { + t.Errorf("Expected 1 updated issue, got %d", result.Updated) + } + + if result.Created != 0 { + t.Errorf("Expected 0 created issues, got %d", result.Created) + } + + // Verify the update + issue, err := store.GetIssue(ctx, "bd-test-1") + if err != nil { + t.Fatalf("Failed to get issue: %v", err) + } + + if issue.Title != "Updated title from Jira" { + t.Errorf("Expected title 'Updated title from Jira', got '%s'", issue.Title) + } + + if issue.Status != types.StatusInProgress { + t.Errorf("Expected status in_progress, got %s", issue.Status) + } +} + +func TestImportWithExternalRefDifferentID(t *testing.T) { + ctx := context.Background() + tmpDir := t.TempDir() + dbPath := filepath.Join(tmpDir, "test.db") + + // Create database + store, err := sqlite.New(dbPath) + if err != nil { + t.Fatalf("Failed to create database: %v", err) + } + defer store.Close() + + // Set prefix + if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil { + t.Fatalf("Failed to set prefix: %v", err) + } + + // Create initial issue with external_ref + externalRef := "GH-200" + initial := &types.Issue{ + ID: "bd-old-id", + Title: "Initial title", + Description: "Initial description", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeFeature, + ExternalRef: &externalRef, + CreatedAt: time.Now().Add(-2 * time.Hour), + UpdatedAt: time.Now().Add(-2 * time.Hour), + } + + err = store.CreateIssue(ctx, initial, "test") + if err != nil { + t.Fatalf("Failed to create initial issue: %v", err) + } + + // Import issue with same external_ref but DIFFERENT ID + // This simulates re-syncing from GitHub where ID changed + updated := &types.Issue{ + ID: "bd-new-id", // Different ID + Title: "Updated title from GitHub", + Description: "Updated description from GitHub", + Status: types.StatusInProgress, + Priority: 2, + IssueType: types.TypeFeature, + ExternalRef: &externalRef, // Same external_ref + CreatedAt: initial.CreatedAt, + UpdatedAt: time.Now(), // Newer timestamp + } + + opts := Options{ + DryRun: false, + SkipUpdate: false, + SkipPrefixValidation: true, + } + + result, err := ImportIssues(ctx, dbPath, store, []*types.Issue{updated}, opts) + if err != nil { + t.Fatalf("ImportIssues failed: %v", err) + } + + // Should have updated the existing issue (matched by external_ref) + if result.Updated != 1 { + t.Errorf("Expected 1 updated issue, got %d", result.Updated) + } + + // Verify the old ID was updated (not deleted/recreated) + oldIssue, err := store.GetIssue(ctx, "bd-old-id") + if err != nil { + t.Fatalf("Failed to get issue by old ID: %v", err) + } + + if oldIssue == nil { + t.Fatal("Expected old ID to still exist and be updated") + } + + if oldIssue.Title != "Updated title from GitHub" { + t.Errorf("Expected title 'Updated title from GitHub', got '%s'", oldIssue.Title) + } + + // The new ID should NOT exist (we updated the existing one) + newIssue, err := store.GetIssue(ctx, "bd-new-id") + if err != nil { + t.Fatalf("Failed to check for new ID: %v", err) + } + + if newIssue != nil { + t.Error("Expected new ID to NOT be created, but it exists") + } +} + +func TestImportLocalIssueNotOverwrittenByExternalRef(t *testing.T) { + ctx := context.Background() + tmpDir := t.TempDir() + dbPath := filepath.Join(tmpDir, "test.db") + + // Create database + store, err := sqlite.New(dbPath) + if err != nil { + t.Fatalf("Failed to create database: %v", err) + } + defer store.Close() + + // Set prefix + if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil { + t.Fatalf("Failed to set prefix: %v", err) + } + + // Create local issue WITHOUT external_ref + local := &types.Issue{ + ID: "bd-local-1", + Title: "Local task", + Description: "Created locally", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + // No ExternalRef + CreatedAt: time.Now().Add(-2 * time.Hour), + UpdatedAt: time.Now().Add(-2 * time.Hour), + } + + err = store.CreateIssue(ctx, local, "test") + if err != nil { + t.Fatalf("Failed to create local issue: %v", err) + } + + // Import external issue with external_ref but different ID + externalRef := "JIRA-300" + external := &types.Issue{ + ID: "bd-external-1", + Title: "External issue", + Description: "From Jira", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeBug, + ExternalRef: &externalRef, + CreatedAt: time.Now().Add(-1 * time.Hour), + UpdatedAt: time.Now().Add(-1 * time.Hour), + } + + opts := Options{ + DryRun: false, + SkipUpdate: false, + SkipPrefixValidation: true, + } + + result, err := ImportIssues(ctx, dbPath, store, []*types.Issue{external}, opts) + if err != nil { + t.Fatalf("ImportIssues failed: %v", err) + } + + // Should create new issue (not overwrite local one) + if result.Created != 1 { + t.Errorf("Expected 1 created issue, got %d", result.Created) + } + + // Verify local issue still exists unchanged + localIssue, err := store.GetIssue(ctx, "bd-local-1") + if err != nil { + t.Fatalf("Failed to get local issue: %v", err) + } + + if localIssue == nil { + t.Fatal("Local issue was deleted!") + } + + if localIssue.Title != "Local task" { + t.Errorf("Local issue was modified! Title: %s", localIssue.Title) + } + + if localIssue.ExternalRef != nil { + t.Error("Local issue should not have external_ref") + } + + // Verify external issue was created + externalIssue, err := store.GetIssue(ctx, "bd-external-1") + if err != nil { + t.Fatalf("Failed to get external issue: %v", err) + } + + if externalIssue == nil { + t.Fatal("External issue was not created") + } + + if externalIssue.ExternalRef == nil || *externalIssue.ExternalRef != externalRef { + t.Error("External issue missing external_ref") + } +} + +func TestImportExternalRefTimestampCheck(t *testing.T) { + ctx := context.Background() + tmpDir := t.TempDir() + dbPath := filepath.Join(tmpDir, "test.db") + + // Create database + store, err := sqlite.New(dbPath) + if err != nil { + t.Fatalf("Failed to create database: %v", err) + } + defer store.Close() + + // Set prefix + if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil { + t.Fatalf("Failed to set prefix: %v", err) + } + + // Create issue with external_ref and recent timestamp + externalRef := "LINEAR-400" + recent := &types.Issue{ + ID: "bd-test-1", + Title: "Recent version", + Description: "Most recent", + Status: types.StatusInProgress, + Priority: 1, + IssueType: types.TypeBug, + ExternalRef: &externalRef, + CreatedAt: time.Now().Add(-1 * time.Hour), + UpdatedAt: time.Now(), // Recent + } + + err = store.CreateIssue(ctx, recent, "test") + if err != nil { + t.Fatalf("Failed to create recent issue: %v", err) + } + + // Try to import older version with same external_ref + older := &types.Issue{ + ID: "bd-test-1", + Title: "Older version", + Description: "Older", + Status: types.StatusOpen, + Priority: 2, + IssueType: types.TypeBug, + ExternalRef: &externalRef, + CreatedAt: time.Now().Add(-2 * time.Hour), + UpdatedAt: time.Now().Add(-2 * time.Hour), // Older + } + + opts := Options{ + DryRun: false, + SkipUpdate: false, + SkipPrefixValidation: true, + } + + result, err := ImportIssues(ctx, dbPath, store, []*types.Issue{older}, opts) + if err != nil { + t.Fatalf("ImportIssues failed: %v", err) + } + + // Should NOT update (incoming is older) + if result.Updated != 0 { + t.Errorf("Expected 0 updated issues (timestamp check), got %d", result.Updated) + } + + if result.Unchanged != 1 { + t.Errorf("Expected 1 unchanged issue, got %d", result.Unchanged) + } + + // Verify the issue was not changed + issue, err := store.GetIssue(ctx, "bd-test-1") + if err != nil { + t.Fatalf("Failed to get issue: %v", err) + } + + if issue.Title != "Recent version" { + t.Errorf("Issue was updated when it shouldn't be! Title: %s", issue.Title) + } + + if issue.Status != types.StatusInProgress { + t.Errorf("Issue status changed! Got %s", issue.Status) + } +} diff --git a/internal/importer/importer.go b/internal/importer/importer.go index 61b489b2..d16d7e12 100644 --- a/internal/importer/importer.go +++ b/internal/importer/importer.go @@ -373,6 +373,14 @@ func upsertIssues(ctx context.Context, sqliteStore *sqlite.SQLiteStorage, issues dbByHash := buildHashMap(dbIssues) dbByID := buildIDMap(dbIssues) + + // Build external_ref map for O(1) lookup + dbByExternalRef := make(map[string]*types.Issue) + for _, issue := range dbIssues { + if issue.ExternalRef != nil && *issue.ExternalRef != "" { + dbByExternalRef[*issue.ExternalRef] = issue + } + } // Track what we need to create var newIssues []*types.Issue @@ -392,8 +400,60 @@ func upsertIssues(ctx context.Context, sqliteStore *sqlite.SQLiteStorage, issues continue } seenHashes[hash] = true + + // Phase 0: Match by external_ref first (if present) + // This enables re-syncing from external systems (Jira, GitHub, Linear) + if incoming.ExternalRef != nil && *incoming.ExternalRef != "" { + if existing, found := dbByExternalRef[*incoming.ExternalRef]; found { + // Found match by external_ref - update the existing issue + if !opts.SkipUpdate { + // Check timestamps - only update if incoming is newer (bd-e55c) + if !incoming.UpdatedAt.After(existing.UpdatedAt) { + // Local version is newer or same - skip update + result.Unchanged++ + continue + } + + // Build updates map + updates := make(map[string]interface{}) + updates["title"] = incoming.Title + updates["description"] = incoming.Description + updates["status"] = incoming.Status + updates["priority"] = incoming.Priority + updates["issue_type"] = incoming.IssueType + updates["design"] = incoming.Design + updates["acceptance_criteria"] = incoming.AcceptanceCriteria + updates["notes"] = incoming.Notes + + if incoming.Assignee != "" { + updates["assignee"] = incoming.Assignee + } else { + updates["assignee"] = nil + } + + if incoming.ExternalRef != nil && *incoming.ExternalRef != "" { + updates["external_ref"] = *incoming.ExternalRef + } else { + updates["external_ref"] = nil + } + + // Only update if data actually changed + if IssueDataChanged(existing, updates) { + if err := sqliteStore.UpdateIssue(ctx, existing.ID, updates, "import"); err != nil { + return fmt.Errorf("error updating issue %s (matched by external_ref): %w", existing.ID, err) + } + result.Updated++ + } else { + result.Unchanged++ + } + } else { + result.Skipped++ + } + continue + } + } - // Phase 1: Match by content hash first + // Phase 1: Match by content hash if existing, found := dbByHash[hash]; found { // Same content exists if existing.ID == incoming.ID { diff --git a/internal/storage/memory/memory.go b/internal/storage/memory/memory.go index 4260ecce..c6dcc9a4 100644 --- a/internal/storage/memory/memory.go +++ b/internal/storage/memory/memory.go @@ -278,6 +278,35 @@ func (m *MemoryStorage) GetIssue(ctx context.Context, id string) (*types.Issue, return &issueCopy, nil } +// GetIssueByExternalRef retrieves an issue by external reference +func (m *MemoryStorage) GetIssueByExternalRef(ctx context.Context, externalRef string) (*types.Issue, error) { + m.mu.RLock() + defer m.mu.RUnlock() + + // Linear search through all issues to find match by external_ref + for _, issue := range m.issues { + if issue.ExternalRef != nil && *issue.ExternalRef == externalRef { + // Return a copy to avoid mutations + issueCopy := *issue + + // Attach dependencies + if deps, ok := m.dependencies[issue.ID]; ok { + issueCopy.Dependencies = deps + } + + // Attach labels + if labels, ok := m.labels[issue.ID]; ok { + issueCopy.Labels = labels + } + + return &issueCopy, nil + } + } + + // Not found + return nil, nil +} + // UpdateIssue updates fields on an issue func (m *MemoryStorage) UpdateIssue(ctx context.Context, id string, updates map[string]interface{}, actor string) error { m.mu.Lock() diff --git a/internal/storage/sqlite/collision.go b/internal/storage/sqlite/collision.go index bd4c9d0f..4e4c3133 100644 --- a/internal/storage/sqlite/collision.go +++ b/internal/storage/sqlite/collision.go @@ -35,9 +35,12 @@ type CollisionDetail struct { // DetectCollisions compares incoming JSONL issues against DB state // It distinguishes between: // 1. Exact match (idempotent) - ID and content are identical -// 2. ID match but different content (collision) - same ID, different fields +// 2. ID match but different content (collision/update) - same ID, different fields // 3. New issue - ID doesn't exist in DB -// 4. Rename detected - Different ID but same content (from prior remap) +// 4. External ref match - Different ID but same external_ref (update from external system) +// +// When an incoming issue has an external_ref, we match by external_ref first, +// then by ID. This enables re-syncing from external systems (Jira, GitHub, Linear). // // Returns a CollisionResult categorizing all incoming issues. func DetectCollisions(ctx context.Context, s *SQLiteStorage, incomingIssues []*types.Issue) (*CollisionResult, error) { @@ -56,21 +59,38 @@ func DetectCollisions(ctx context.Context, s *SQLiteStorage, incomingIssues []*t // Check each incoming issue for _, incoming := range incomingIssues { - existing, err := s.GetIssue(ctx, incoming.ID) - if err != nil || existing == nil { - // Issue doesn't exist in DB - it's new + var existing *types.Issue + var err error + + // If incoming issue has external_ref, try matching by external_ref first + if incoming.ExternalRef != nil && *incoming.ExternalRef != "" { + existing, err = s.GetIssueByExternalRef(ctx, *incoming.ExternalRef) + if err != nil { + return nil, fmt.Errorf("failed to lookup by external_ref: %w", err) + } + } + + // If no external_ref match, try matching by ID + if existing == nil { + existing, err = s.GetIssue(ctx, incoming.ID) + if err != nil { + return nil, fmt.Errorf("failed to lookup by ID: %w", err) + } + } + + // No match found - it's a new issue + if existing == nil { result.NewIssues = append(result.NewIssues, incoming.ID) continue } - // Issue ID exists - check if content matches + // Found a match - check if content matches conflictingFields := compareIssues(existing, incoming) if len(conflictingFields) == 0 { // Exact match - idempotent import result.ExactMatches = append(result.ExactMatches, incoming.ID) } else { - // Same ID, different content - collision - // With hash IDs, this shouldn't happen unless manually edited + // Same ID/external_ref, different content - collision (needs update) result.Collisions = append(result.Collisions, &CollisionDetail{ ID: incoming.ID, IncomingIssue: incoming, diff --git a/internal/storage/sqlite/external_ref_test.go b/internal/storage/sqlite/external_ref_test.go new file mode 100644 index 00000000..b5c037b4 --- /dev/null +++ b/internal/storage/sqlite/external_ref_test.go @@ -0,0 +1,281 @@ +package sqlite + +import ( + "context" + "testing" + "time" + + "github.com/steveyegge/beads/internal/types" +) + +func TestGetIssueByExternalRef(t *testing.T) { + ctx := context.Background() + s, cleanup := setupTestDB(t) + defer cleanup() + + // Create test issue with external_ref + externalRef := "JIRA-123" + issue := &types.Issue{ + ID: "bd-test-1", + Title: "Test issue", + Description: "Test description", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeBug, + ExternalRef: &externalRef, + } + + err := s.CreateIssue(ctx, issue, "test") + if err != nil { + t.Fatalf("Failed to create issue: %v", err) + } + + // Test: Find by external_ref + found, err := s.GetIssueByExternalRef(ctx, externalRef) + if err != nil { + t.Fatalf("GetIssueByExternalRef failed: %v", err) + } + + if found == nil { + t.Fatal("Expected to find issue by external_ref, got nil") + } + + if found.ID != issue.ID { + t.Errorf("Expected ID %s, got %s", issue.ID, found.ID) + } + + if found.ExternalRef == nil || *found.ExternalRef != externalRef { + t.Errorf("Expected external_ref %s, got %v", externalRef, found.ExternalRef) + } +} + +func TestGetIssueByExternalRefNotFound(t *testing.T) { + ctx := context.Background() + s, cleanup := setupTestDB(t) + defer cleanup() + + // Test: Search for non-existent external_ref + found, err := s.GetIssueByExternalRef(ctx, "NONEXISTENT-999") + if err != nil { + t.Fatalf("GetIssueByExternalRef failed: %v", err) + } + + if found != nil { + t.Errorf("Expected nil for non-existent external_ref, got %v", found) + } +} + +func TestDetectCollisionsWithExternalRef(t *testing.T) { + ctx := context.Background() + s, cleanup := setupTestDB(t) + defer cleanup() + + // Create existing issue with external_ref + externalRef := "JIRA-456" + existing := &types.Issue{ + ID: "bd-test-1", + Title: "Original title", + Description: "Original description", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeBug, + ExternalRef: &externalRef, + } + + err := s.CreateIssue(ctx, existing, "test") + if err != nil { + t.Fatalf("Failed to create existing issue: %v", err) + } + + // Incoming issue with same external_ref but different ID and content + incoming := &types.Issue{ + ID: "bd-test-2", // Different ID + Title: "Updated title", + Description: "Updated description", + Status: types.StatusInProgress, + Priority: 2, + IssueType: types.TypeBug, + ExternalRef: &externalRef, // Same external_ref + UpdatedAt: time.Now().Add(1 * time.Hour), // Newer timestamp + } + + // Test: Detect collision by external_ref + result, err := DetectCollisions(ctx, s, []*types.Issue{incoming}) + if err != nil { + t.Fatalf("DetectCollisions failed: %v", err) + } + + // Should detect as collision (update needed) + if len(result.Collisions) != 1 { + t.Fatalf("Expected 1 collision, got %d", len(result.Collisions)) + } + + collision := result.Collisions[0] + if collision.ExistingIssue.ID != existing.ID { + t.Errorf("Expected existing issue ID %s, got %s", existing.ID, collision.ExistingIssue.ID) + } + + if collision.IncomingIssue.ID != incoming.ID { + t.Errorf("Expected incoming issue ID %s, got %s", incoming.ID, collision.IncomingIssue.ID) + } + + // Should have conflicting fields + expectedConflicts := []string{"title", "description", "status", "priority"} + for _, field := range expectedConflicts { + found := false + for _, conflictField := range collision.ConflictingFields { + if conflictField == field { + found = true + break + } + } + if !found { + t.Errorf("Expected conflict on field %s, but not found in %v", field, collision.ConflictingFields) + } + } +} + +func TestDetectCollisionsExternalRefPriorityOverID(t *testing.T) { + ctx := context.Background() + s, cleanup := setupTestDB(t) + defer cleanup() + + // Create existing issue with external_ref + externalRef := "GH-789" + existing := &types.Issue{ + ID: "bd-test-1", + Title: "Original title", + Description: "Original description", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeFeature, + ExternalRef: &externalRef, + } + + err := s.CreateIssue(ctx, existing, "test") + if err != nil { + t.Fatalf("Failed to create existing issue: %v", err) + } + + // Create a second issue with a different ID and no external_ref + otherIssue := &types.Issue{ + ID: "bd-test-2", + Title: "Other issue", + Description: "Other description", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + + err = s.CreateIssue(ctx, otherIssue, "test") + if err != nil { + t.Fatalf("Failed to create other issue: %v", err) + } + + // Incoming issue with: + // - Same external_ref as bd-test-1 + // - Same ID as bd-test-2 + // This tests that external_ref matching takes priority over ID matching + incoming := &types.Issue{ + ID: "bd-test-2", // Matches otherIssue.ID + Title: "Updated from external system", + Description: "Updated description", + Status: types.StatusInProgress, + Priority: 2, + IssueType: types.TypeFeature, + ExternalRef: &externalRef, // Matches existing.ExternalRef + UpdatedAt: time.Now().Add(1 * time.Hour), + } + + // Test: DetectCollisions should match by external_ref first + result, err := DetectCollisions(ctx, s, []*types.Issue{incoming}) + if err != nil { + t.Fatalf("DetectCollisions failed: %v", err) + } + + // Should match by external_ref, not ID + if len(result.Collisions) != 1 { + t.Fatalf("Expected 1 collision, got %d", len(result.Collisions)) + } + + collision := result.Collisions[0] + + // The existing issue matched should be bd-test-1 (by external_ref), not bd-test-2 (by ID) + if collision.ExistingIssue.ID != existing.ID { + t.Errorf("Expected external_ref match with %s, but got %s", existing.ID, collision.ExistingIssue.ID) + } + + if collision.ExistingIssue.ExternalRef == nil || *collision.ExistingIssue.ExternalRef != externalRef { + t.Errorf("Expected matched issue to have external_ref %s", externalRef) + } +} + +func TestDetectCollisionsNoExternalRef(t *testing.T) { + ctx := context.Background() + s, cleanup := setupTestDB(t) + defer cleanup() + + // Create existing issue without external_ref + existing := &types.Issue{ + ID: "bd-test-1", + Title: "Local issue", + Description: "Local description", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + + err := s.CreateIssue(ctx, existing, "test") + if err != nil { + t.Fatalf("Failed to create existing issue: %v", err) + } + + // Incoming issue with same ID but no external_ref + incoming := &types.Issue{ + ID: "bd-test-1", + Title: "Updated local issue", + Description: "Updated description", + Status: types.StatusInProgress, + Priority: 2, + IssueType: types.TypeTask, + UpdatedAt: time.Now().Add(1 * time.Hour), + } + + // Test: Should still match by ID when no external_ref + result, err := DetectCollisions(ctx, s, []*types.Issue{incoming}) + if err != nil { + t.Fatalf("DetectCollisions failed: %v", err) + } + + if len(result.Collisions) != 1 { + t.Fatalf("Expected 1 collision, got %d", len(result.Collisions)) + } + + collision := result.Collisions[0] + if collision.ExistingIssue.ID != existing.ID { + t.Errorf("Expected ID match with %s, got %s", existing.ID, collision.ExistingIssue.ID) + } +} + +func TestExternalRefIndex(t *testing.T) { + ctx := context.Background() + s, cleanup := setupTestDB(t) + defer cleanup() + + // Verify that the external_ref index exists + var indexExists bool + err := s.db.QueryRowContext(ctx, ` + SELECT EXISTS( + SELECT 1 FROM sqlite_master + WHERE type='index' AND name='idx_issues_external_ref' + ) + `).Scan(&indexExists) + + if err != nil { + t.Fatalf("Failed to check for index: %v", err) + } + + if !indexExists { + t.Error("Expected idx_issues_external_ref index to exist") + } +} diff --git a/internal/storage/sqlite/schema.go b/internal/storage/sqlite/schema.go index 0b638dd5..cb0cf562 100644 --- a/internal/storage/sqlite/schema.go +++ b/internal/storage/sqlite/schema.go @@ -30,6 +30,7 @@ CREATE INDEX IF NOT EXISTS idx_issues_status ON issues(status); CREATE INDEX IF NOT EXISTS idx_issues_priority ON issues(priority); CREATE INDEX IF NOT EXISTS idx_issues_assignee ON issues(assignee); CREATE INDEX IF NOT EXISTS idx_issues_created_at ON issues(created_at); +CREATE INDEX IF NOT EXISTS idx_issues_external_ref ON issues(external_ref); -- Dependencies table CREATE TABLE IF NOT EXISTS dependencies ( diff --git a/internal/storage/sqlite/sqlite.go b/internal/storage/sqlite/sqlite.go index 25c57bba..7acc874a 100644 --- a/internal/storage/sqlite/sqlite.go +++ b/internal/storage/sqlite/sqlite.go @@ -286,6 +286,76 @@ func (s *SQLiteStorage) GetIssue(ctx context.Context, id string) (*types.Issue, return &issue, nil } +// GetIssueByExternalRef retrieves an issue by external reference +func (s *SQLiteStorage) GetIssueByExternalRef(ctx context.Context, externalRef string) (*types.Issue, error) { + var issue types.Issue + var closedAt sql.NullTime + var estimatedMinutes sql.NullInt64 + var assignee sql.NullString + var externalRefCol sql.NullString + var compactedAt sql.NullTime + var originalSize sql.NullInt64 + var contentHash sql.NullString + var compactedAtCommit sql.NullString + + err := s.db.QueryRowContext(ctx, ` + SELECT id, content_hash, title, description, design, acceptance_criteria, notes, + status, priority, issue_type, assignee, estimated_minutes, + created_at, updated_at, closed_at, external_ref, + compaction_level, compacted_at, compacted_at_commit, original_size + FROM issues + WHERE external_ref = ? + `, externalRef).Scan( + &issue.ID, &contentHash, &issue.Title, &issue.Description, &issue.Design, + &issue.AcceptanceCriteria, &issue.Notes, &issue.Status, + &issue.Priority, &issue.IssueType, &assignee, &estimatedMinutes, + &issue.CreatedAt, &issue.UpdatedAt, &closedAt, &externalRefCol, + &issue.CompactionLevel, &compactedAt, &compactedAtCommit, &originalSize, + ) + + if err == sql.ErrNoRows { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("failed to get issue by external_ref: %w", err) + } + + if contentHash.Valid { + issue.ContentHash = contentHash.String + } + if closedAt.Valid { + issue.ClosedAt = &closedAt.Time + } + if estimatedMinutes.Valid { + mins := int(estimatedMinutes.Int64) + issue.EstimatedMinutes = &mins + } + if assignee.Valid { + issue.Assignee = assignee.String + } + if externalRefCol.Valid { + issue.ExternalRef = &externalRefCol.String + } + if compactedAt.Valid { + issue.CompactedAt = &compactedAt.Time + } + if compactedAtCommit.Valid { + issue.CompactedAtCommit = &compactedAtCommit.String + } + if originalSize.Valid { + issue.OriginalSize = int(originalSize.Int64) + } + + // Fetch labels for this issue + labels, err := s.GetLabels(ctx, issue.ID) + if err != nil { + return nil, fmt.Errorf("failed to get labels: %w", err) + } + issue.Labels = labels + + return &issue, nil +} + // Allowed fields for update to prevent SQL injection var allowedUpdateFields = map[string]bool{ "status": true, diff --git a/internal/storage/storage.go b/internal/storage/storage.go index 3f43d806..c422ea3f 100644 --- a/internal/storage/storage.go +++ b/internal/storage/storage.go @@ -14,6 +14,7 @@ type Storage interface { CreateIssue(ctx context.Context, issue *types.Issue, actor string) error CreateIssues(ctx context.Context, issues []*types.Issue, actor string) error GetIssue(ctx context.Context, id string) (*types.Issue, error) + GetIssueByExternalRef(ctx context.Context, externalRef string) (*types.Issue, error) UpdateIssue(ctx context.Context, id string, updates map[string]interface{}, actor string) error CloseIssue(ctx context.Context, id string, reason string, actor string) error SearchIssues(ctx context.Context, query string, filter types.IssueFilter) ([]*types.Issue, error)