Merge remote changes

Resolved .beads/beads.jsonl conflict

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

cmd/bd/deletion_tracking_test.go

@@ -235,16 +235,17 @@ func TestDeletionWithLocalModification(t *testing.T) {
 		t.Fatalf("Failed to simulate remote deletion: %v", err)
 	}
 
-	// Try to merge - this should detect a conflict (modified locally, deleted remotely)
+	// Try to merge - deletion now wins over modification (bd-pq5k)
+	// This should succeed and delete the issue
 	_, err = merge3WayAndPruneDeletions(ctx, store, jsonlPath)
-	if err == nil {
-		t.Error("Expected merge conflict error, but got nil")
+	if err != nil {
+		t.Errorf("Expected merge to succeed (deletion wins), but got error: %v", err)
 	}
 
-	// The issue should still exist in the database (conflict not auto-resolved)
+	// The issue should be deleted (deletion wins over modification)
 	conflictIssue, err := store.GetIssue(ctx, "bd-conflict")
-	if err != nil || conflictIssue == nil {
-		t.Error("Issue should still exist after conflict")
+	if err == nil && conflictIssue != nil {
+		t.Error("Issue should be deleted after merge (deletion wins)")
 	}
 }
 
@@ -295,12 +296,13 @@ func TestComputeAcceptedDeletions(t *testing.T) {
 	}
 }
 
-// TestComputeAcceptedDeletions_LocallyModified tests that locally modified issues are not deleted
+// TestComputeAcceptedDeletions_LocallyModified tests that deletion wins even for locally modified issues (bd-pq5k)
 func TestComputeAcceptedDeletions_LocallyModified(t *testing.T) {
 	dir := t.TempDir()
 
-	basePath := filepath.Join(dir, "base.jsonl")
-	leftPath := filepath.Join(dir, "left.jsonl")
+	jsonlPath := filepath.Join(dir, "issues.jsonl")
+	sm := NewSnapshotManager(jsonlPath)
+	basePath, leftPath := sm.GetSnapshotPaths()
 	mergedPath := filepath.Join(dir, "merged.jsonl")
 
 	// Base has 2 issues
@@ -313,7 +315,7 @@ func TestComputeAcceptedDeletions_LocallyModified(t *testing.T) {
 {"id":"bd-2","title":"Modified locally"}
 `
 
-	// Merged has only bd-1 (bd-2 deleted remotely, but we modified it locally)
+	// Merged has only bd-1 (bd-2 deleted remotely, we modified it locally, but deletion wins per bd-pq5k)
 	mergedContent := `{"id":"bd-1","title":"Original 1"}
 `
 
@@ -327,16 +329,17 @@ func TestComputeAcceptedDeletions_LocallyModified(t *testing.T) {
 		t.Fatalf("Failed to write merged: %v", err)
 	}
 
-	jsonlPath := filepath.Join(dir, "issues.jsonl")
-	sm := NewSnapshotManager(jsonlPath)
 	deletions, err := sm.ComputeAcceptedDeletions(mergedPath)
 	if err != nil {
 		t.Fatalf("Failed to compute deletions: %v", err)
 	}
 
-	// bd-2 should NOT be in accepted deletions because it was modified locally
-	if len(deletions) != 0 {
-		t.Errorf("Expected 0 deletions (locally modified), got %d: %v", len(deletions), deletions)
+	// bd-pq5k: bd-2 SHOULD be in accepted deletions even though modified locally (deletion wins)
+	if len(deletions) != 1 {
+		t.Errorf("Expected 1 deletion (deletion wins over local modification), got %d: %v", len(deletions), deletions)
+	}
+	if len(deletions) == 1 && deletions[0] != "bd-2" {
+		t.Errorf("Expected deletion of bd-2, got %v", deletions)
 	}
 }
 

cmd/bd/snapshot_manager.go

@@ -232,21 +232,19 @@ func (sm *SnapshotManager) Initialize() error {
 // An issue is an "accepted deletion" if:
 // - It exists in base (last import)
 // - It does NOT exist in merged (after 3-way merge)
-// - It is unchanged in left (pre-pull export) compared to base
+//
+// Note (bd-pq5k): Deletion always wins over modification in the merge,
+// so if an issue is deleted in the merged result, we accept it regardless
+// of local changes.
 func (sm *SnapshotManager) ComputeAcceptedDeletions(mergedPath string) ([]string, error) {
-	basePath, leftPath := sm.getSnapshotPaths()
+	basePath, _ := sm.getSnapshotPaths()
 
-	// Build map of ID -> raw line for base and left
+	// Build map of ID -> raw line for base
 	baseIndex, err := sm.buildIDToLineMap(basePath)
 	if err != nil {
 		return nil, fmt.Errorf("failed to read base snapshot: %w", err)
 	}
 
-	leftIndex, err := sm.buildIDToLineMap(leftPath)
-	if err != nil {
-		return nil, fmt.Errorf("failed to read left snapshot: %w", err)
-	}
-
 	// Build set of IDs in merged result
 	mergedIDs, err := sm.buildIDSet(mergedPath)
 	if err != nil {
@@ -257,13 +255,13 @@ func (sm *SnapshotManager) ComputeAcceptedDeletions(mergedPath string) ([]string
 
 	// Find accepted deletions
 	var deletions []string
-	for id, baseLine := range baseIndex {
+	for id := range baseIndex {
 		// Issue in base but not in merged
 		if !mergedIDs[id] {
-			// Check if unchanged locally - try raw equality first, then semantic JSON comparison
-			if leftLine, existsInLeft := leftIndex[id]; existsInLeft && (leftLine == baseLine || sm.jsonEquals(leftLine, baseLine)) {
-				deletions = append(deletions, id)
-			}
+			// bd-pq5k: Deletion always wins over modification in 3-way merge
+			// If the merge resulted in deletion, accept it regardless of local changes
+			// The 3-way merge already determined that deletion should win
+			deletions = append(deletions, id)
 		}
 	}
 

internal/merge/merge.go

@@ -297,22 +297,14 @@ func merge3Way(base, left, right []Issue) ([]Issue, []string) {
 			}
 		} else if inBase && inLeft && !inRight {
 			// Deleted in right, maybe modified in left
-			if issuesEqual(baseIssue, leftIssue) {
-				// Deleted in right, unchanged in left - accept deletion
-				continue
-			} else {
-				// Modified in left, deleted in right - conflict
-				conflicts = append(conflicts, makeConflictWithBase(baseIssue.RawLine, leftIssue.RawLine, ""))
-			}
+			// RULE 2: deletion always wins over modification
+			// This is because deletion is an explicit action that should be preserved
+			continue
 		} else if inBase && !inLeft && inRight {
 			// Deleted in left, maybe modified in right
-			if issuesEqual(baseIssue, rightIssue) {
-				// Deleted in left, unchanged in right - accept deletion
-				continue
-			} else {
-				// Modified in right, deleted in left - conflict
-				conflicts = append(conflicts, makeConflictWithBase(baseIssue.RawLine, "", rightIssue.RawLine))
-			}
+			// RULE 2: deletion always wins over modification
+			// This is because deletion is an explicit action that should be preserved
+			continue
 		} else if !inBase && inLeft && !inRight {
 			// Added only in left
 			result = append(result, leftIssue)
@@ -341,8 +333,8 @@ func mergeIssue(base, left, right Issue) (Issue, string) {
 	// Merge notes
 	result.Notes = mergeField(base.Notes, left.Notes, right.Notes)
 
-	// Merge status
-	result.Status = mergeField(base.Status, left.Status, right.Status)
+	// Merge status - SPECIAL RULE: closed always wins over open
+	result.Status = mergeStatus(base.Status, left.Status, right.Status)
 
 	// Merge priority (as int)
 	if base.Priority == left.Priority && base.Priority != right.Priority {
@@ -362,8 +354,13 @@ func mergeIssue(base, left, right Issue) (Issue, string) {
 	// Merge updated_at - take the max
 	result.UpdatedAt = maxTime(left.UpdatedAt, right.UpdatedAt)
 
-	// Merge closed_at - take the max
-	result.ClosedAt = maxTime(left.ClosedAt, right.ClosedAt)
+	// Merge closed_at - only if status is closed
+	// This prevents invalid state (status=open with closed_at set)
+	if result.Status == "closed" {
+		result.ClosedAt = maxTime(left.ClosedAt, right.ClosedAt)
+	} else {
+		result.ClosedAt = ""
+	}
 
 	// Merge dependencies - combine and deduplicate
 	result.Dependencies = mergeDependencies(left.Dependencies, right.Dependencies)
@@ -376,6 +373,17 @@ func mergeIssue(base, left, right Issue) (Issue, string) {
 	return result, ""
 }
 
+func mergeStatus(base, left, right string) string {
+	// RULE 1: closed always wins over open
+	// This prevents the insane situation where issues never die
+	if left == "closed" || right == "closed" {
+		return "closed"
+	}
+
+	// Otherwise use standard 3-way merge
+	return mergeField(base, left, right)
+}
+
 func mergeField(base, left, right string) string {
 	if base == left && base != right {
 		return right

internal/merge/merge_test.go

@@ -7,6 +7,70 @@ import (
 	"testing"
 )
 
+// TestMergeStatus tests the status merging logic with special rules
+func TestMergeStatus(t *testing.T) {
+	tests := []struct {
+		name     string
+		base     string
+		left     string
+		right    string
+		expected string
+	}{
+		{
+			name:     "no changes",
+			base:     "open",
+			left:     "open",
+			right:    "open",
+			expected: "open",
+		},
+		{
+			name:     "left closed, right open - closed wins",
+			base:     "open",
+			left:     "closed",
+			right:    "open",
+			expected: "closed",
+		},
+		{
+			name:     "left open, right closed - closed wins",
+			base:     "open",
+			left:     "open",
+			right:    "closed",
+			expected: "closed",
+		},
+		{
+			name:     "both closed",
+			base:     "open",
+			left:     "closed",
+			right:    "closed",
+			expected: "closed",
+		},
+		{
+			name:     "base closed, left open, right open - open (standard merge)",
+			base:     "closed",
+			left:     "open",
+			right:    "open",
+			expected: "open",
+		},
+		{
+			name:     "base closed, left open, right closed - closed wins",
+			base:     "closed",
+			left:     "open",
+			right:    "closed",
+			expected: "closed",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := mergeStatus(tt.base, tt.left, tt.right)
+			if result != tt.expected {
+				t.Errorf("mergeStatus(%q, %q, %q) = %q, want %q",
+					tt.base, tt.left, tt.right, result, tt.expected)
+			}
+		})
+	}
+}
+
 // TestMergeField tests the basic field merging logic
 func TestMergeField(t *testing.T) {
 	tests := []struct {
@@ -475,7 +539,7 @@ func TestMerge3Way_Deletions(t *testing.T) {
 		}
 	})
 
-	t.Run("deleted in left, modified in right - conflict", func(t *testing.T) {
+	t.Run("deleted in left, modified in right - deletion wins", func(t *testing.T) {
 		base := []Issue{
 			{
 				ID:        "bd-abc123",
@@ -499,15 +563,15 @@ func TestMerge3Way_Deletions(t *testing.T) {
 		}
 
 		result, conflicts := merge3Way(base, left, right)
-		if len(conflicts) == 0 {
-			t.Error("expected conflict for delete vs modify")
+		if len(conflicts) != 0 {
+			t.Errorf("expected no conflicts, got %d", len(conflicts))
 		}
 		if len(result) != 0 {
-			t.Errorf("expected no merged issues with conflict, got %d", len(result))
+			t.Errorf("expected deletion to win (0 results), got %d", len(result))
 		}
 	})
 
-	t.Run("deleted in right, modified in left - conflict", func(t *testing.T) {
+	t.Run("deleted in right, modified in left - deletion wins", func(t *testing.T) {
 		base := []Issue{
 			{
 				ID:        "bd-abc123",
@@ -531,11 +595,11 @@ func TestMerge3Way_Deletions(t *testing.T) {
 		right := []Issue{} // Deleted in right
 
 		result, conflicts := merge3Way(base, left, right)
-		if len(conflicts) == 0 {
-			t.Error("expected conflict for modify vs delete")
+		if len(conflicts) != 0 {
+			t.Errorf("expected no conflicts, got %d", len(conflicts))
 		}
 		if len(result) != 0 {
-			t.Errorf("expected no merged issues with conflict, got %d", len(result))
+			t.Errorf("expected deletion to win (0 results), got %d", len(result))
 		}
 	})
 }
@@ -648,6 +712,61 @@ func TestMerge3Way_Additions(t *testing.T) {
 
 // TestMerge3Way_ResurrectionPrevention tests bd-hv01 regression
 func TestMerge3Way_ResurrectionPrevention(t *testing.T) {
+	t.Run("bd-pq5k: no invalid state (status=open with closed_at)", func(t *testing.T) {
+		// Simulate the broken merge case that was creating invalid data
+		// Base: issue is closed
+		base := []Issue{
+			{
+				ID:        "bd-test",
+				Title:     "Test issue",
+				Status:    "closed",
+				ClosedAt:  "2024-01-02T00:00:00Z",
+				CreatedAt: "2024-01-01T00:00:00Z",
+				UpdatedAt: "2024-01-02T00:00:00Z",
+				CreatedBy: "user1",
+				RawLine:   `{"id":"bd-test","title":"Test issue","status":"closed","closed_at":"2024-01-02T00:00:00Z","created_at":"2024-01-01T00:00:00Z","updated_at":"2024-01-02T00:00:00Z","created_by":"user1"}`,
+			},
+		}
+		// Left: still closed with closed_at
+		left := base
+		// Right: somehow got reopened but WITHOUT removing closed_at (the bug scenario)
+		right := []Issue{
+			{
+				ID:        "bd-test",
+				Title:     "Test issue",
+				Status:    "open", // reopened
+				ClosedAt:  "",     // correctly removed
+				CreatedAt: "2024-01-01T00:00:00Z",
+				UpdatedAt: "2024-01-03T00:00:00Z",
+				CreatedBy: "user1",
+				RawLine:   `{"id":"bd-test","title":"Test issue","status":"open","created_at":"2024-01-01T00:00:00Z","updated_at":"2024-01-03T00:00:00Z","created_by":"user1"}`,
+			},
+		}
+
+		result, conflicts := merge3Way(base, left, right)
+		if len(conflicts) != 0 {
+			t.Errorf("unexpected conflicts: %v", conflicts)
+		}
+		if len(result) != 1 {
+			t.Fatalf("expected 1 issue, got %d", len(result))
+		}
+
+		// CRITICAL: Status should be closed (closed wins over open)
+		if result[0].Status != "closed" {
+			t.Errorf("expected status 'closed', got %q", result[0].Status)
+		}
+
+		// CRITICAL: If status is closed, closed_at MUST be set
+		if result[0].Status == "closed" && result[0].ClosedAt == "" {
+			t.Error("INVALID STATE: status='closed' but closed_at is empty")
+		}
+
+		// CRITICAL: If status is open, closed_at MUST be empty
+		if result[0].Status == "open" && result[0].ClosedAt != "" {
+			t.Errorf("INVALID STATE: status='open' but closed_at='%s'", result[0].ClosedAt)
+		}
+	})
+
 	t.Run("bd-hv01 regression: closed issue not resurrected", func(t *testing.T) {
 		// Base: issue is open
 		base := []Issue{

internal/storage/sqlite/util.go

@@ -51,4 +51,15 @@ func IsUniqueConstraintError(err error) bool {
 	return strings.Contains(err.Error(), "UNIQUE constraint failed")
 }
 
+// IsForeignKeyConstraintError checks if an error is a FOREIGN KEY constraint violation
+// This can occur when importing issues that reference deleted issues (e.g., after merge)
+func IsForeignKeyConstraintError(err error) bool {
+	if err == nil {
+		return false
+	}
+	errStr := err.Error()
+	return strings.Contains(errStr, "FOREIGN KEY constraint failed") ||
+		strings.Contains(errStr, "foreign key constraint failed")
+}
+
 

internal/storage/sqlite/util_test.go

@@ -50,6 +50,54 @@ func TestIsUniqueConstraintError(t *testing.T) {
 	}
 }
 
+func TestIsForeignKeyConstraintError(t *testing.T) {
+	tests := []struct {
+		name     string
+		err      error
+		expected bool
+	}{
+		{
+			name:     "nil error",
+			err:      nil,
+			expected: false,
+		},
+		{
+			name:     "FOREIGN KEY constraint error (uppercase)",
+			err:      errors.New("FOREIGN KEY constraint failed"),
+			expected: true,
+		},
+		{
+			name:     "foreign key constraint error (lowercase)",
+			err:      errors.New("foreign key constraint failed"),
+			expected: true,
+		},
+		{
+			name:     "FOREIGN KEY with details",
+			err:      errors.New("FOREIGN KEY constraint failed: dependencies.depends_on_id"),
+			expected: true,
+		},
+		{
+			name:     "UNIQUE constraint error",
+			err:      errors.New("UNIQUE constraint failed: issues.id"),
+			expected: false,
+		},
+		{
+			name:     "other error",
+			err:      errors.New("some other database error"),
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := IsForeignKeyConstraintError(tt.err)
+			if result != tt.expected {
+				t.Errorf("IsForeignKeyConstraintError(%v) = %v, want %v", tt.err, result, tt.expected)
+			}
+		})
+	}
+}
+
 func TestExecInTransaction(t *testing.T) {
 	ctx := context.Background()
 	store := newTestStore(t, t.TempDir()+"/test.db")