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
This commit is contained in:
Steve Yegge
2025-11-02 15:27:59 -08:00
parent 8c5d2373bd
commit 55c722a3e3
8 changed files with 839 additions and 9 deletions

View File

@@ -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,

View File

@@ -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")
}
}

View File

@@ -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 (

View File

@@ -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,