fix: add ID-based fallback matching to prevent ghost resurrection (bd-ncwo)

Root cause: The merge driver matched issues by IssueKey (ID+CreatedAt+CreatedBy).
When timestamp precision differed (e.g., with/without nanoseconds), the same
issue was treated as two different issues, causing both tombstone and closed
versions to appear in the merge result.

Fix: Added ID-based fallback matching in merge3WayWithTTL. When key-based
matching fails but the same ID exists in the other side, use that for merging.
Also track processed IDs to prevent duplicates.

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

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Steve Yegge
2025-12-16 22:35:29 -08:00
parent 0ce039429d
commit fcd29f5ea4
2 changed files with 201 additions and 2 deletions

View File

@@ -298,7 +298,7 @@ func merge3Way(base, left, right []Issue) ([]Issue, []string) {
// merge3WayWithTTL performs a 3-way merge with configurable tombstone TTL.
// This is the core merge function that handles tombstone semantics.
func merge3WayWithTTL(base, left, right []Issue, ttl time.Duration) ([]Issue, []string) {
// Build maps for quick lookup
// Build maps for quick lookup by IssueKey
baseMap := make(map[IssueKey]Issue)
for _, issue := range base {
baseMap[makeKey(issue)] = issue
@@ -314,8 +314,22 @@ func merge3WayWithTTL(base, left, right []Issue, ttl time.Duration) ([]Issue, []
rightMap[makeKey(issue)] = issue
}
// Track which issues we've processed
// bd-ncwo: Also build ID-based maps for fallback matching
// This handles cases where the same issue has slightly different CreatedAt/CreatedBy
// (e.g., due to timestamp precision differences between systems)
leftByID := make(map[string]Issue)
for _, issue := range left {
leftByID[issue.ID] = issue
}
rightByID := make(map[string]Issue)
for _, issue := range right {
rightByID[issue.ID] = issue
}
// Track which issues we've processed (by both key and ID)
processed := make(map[IssueKey]bool)
processedIDs := make(map[string]bool) // bd-ncwo: track processed IDs to avoid duplicates
var result []Issue
var conflicts []string
@@ -341,6 +355,43 @@ func merge3WayWithTTL(base, left, right []Issue, ttl time.Duration) ([]Issue, []
leftIssue, inLeft := leftMap[key]
rightIssue, inRight := rightMap[key]
// bd-ncwo: ID-based fallback matching for tombstone preservation
// If key doesn't match but same ID exists in the other side, use that
if !inLeft && inRight {
if fallback, found := leftByID[rightIssue.ID]; found {
leftIssue = fallback
inLeft = true
// Mark the fallback's key as processed to avoid duplicate
processed[makeKey(fallback)] = true
}
}
if !inRight && inLeft {
if fallback, found := rightByID[leftIssue.ID]; found {
rightIssue = fallback
inRight = true
// Mark the fallback's key as processed to avoid duplicate
processed[makeKey(fallback)] = true
}
}
// bd-ncwo: Check if we've already processed this ID (via a different key)
currentID := key.ID
if currentID == "" {
if inLeft {
currentID = leftIssue.ID
} else if inRight {
currentID = rightIssue.ID
} else if inBase {
currentID = baseIssue.ID
}
}
if currentID != "" && processedIDs[currentID] {
continue
}
if currentID != "" {
processedIDs[currentID] = true
}
// Determine tombstone status
leftTombstone := inLeft && IsTombstone(leftIssue)
rightTombstone := inRight && IsTombstone(rightIssue)

View File

@@ -2202,3 +2202,151 @@ func TestMerge3Way_TombstoneBaseBothLiveResurrection(t *testing.T) {
}
})
}
// TestMerge3Way_TombstoneVsLiveTimestampPrecisionMismatch tests bd-ncwo:
// When the same issue has different CreatedAt timestamp precision (e.g., with/without nanoseconds),
// the tombstone should still win over the live version.
func TestMerge3Way_TombstoneVsLiveTimestampPrecisionMismatch(t *testing.T) {
// This test simulates the ghost resurrection bug where timestamp precision
// differences caused the same issue to be treated as two different issues.
// The key fix (bd-ncwo) adds ID-based fallback matching when keys don't match.
t.Run("tombstone wins despite different CreatedAt precision", func(t *testing.T) {
// Base: issue with status=closed
baseIssue := Issue{
ID: "bd-ghost1",
Title: "Original title",
Status: "closed",
Priority: 2,
CreatedAt: "2024-01-01T00:00:00Z", // No fractional seconds
UpdatedAt: "2024-01-10T00:00:00Z",
CreatedBy: "user1",
}
// Left: tombstone with DIFFERENT timestamp precision (has microseconds)
tombstone := Issue{
ID: "bd-ghost1",
Title: "(deleted)",
Status: StatusTombstone,
Priority: 2,
CreatedAt: "2024-01-01T00:00:00.000000Z", // WITH fractional seconds
UpdatedAt: "2024-01-15T00:00:00Z",
CreatedBy: "user1",
DeletedAt: time.Now().Add(-24 * time.Hour).Format(time.RFC3339),
DeletedBy: "user2",
DeleteReason: "Duplicate issue",
}
// Right: same closed issue (same precision as base)
closedIssue := Issue{
ID: "bd-ghost1",
Title: "Original title",
Status: "closed",
Priority: 2,
CreatedAt: "2024-01-01T00:00:00Z", // No fractional seconds
UpdatedAt: "2024-01-12T00:00:00Z",
CreatedBy: "user1",
}
base := []Issue{baseIssue}
left := []Issue{tombstone}
right := []Issue{closedIssue}
result, conflicts := merge3Way(base, left, right)
if len(conflicts) != 0 {
t.Errorf("unexpected conflicts: %v", conflicts)
}
// CRITICAL: Should have exactly 1 issue, not 2 (no duplicates)
if len(result) != 1 {
t.Fatalf("expected 1 issue (no duplicates), got %d - this suggests ID-based matching failed", len(result))
}
// Tombstone should win over closed
if result[0].Status != StatusTombstone {
t.Errorf("expected tombstone to win, got status %q", result[0].Status)
}
if result[0].DeletedBy != "user2" {
t.Errorf("expected tombstone fields preserved, got DeletedBy %q", result[0].DeletedBy)
}
})
t.Run("tombstone wins with CreatedBy mismatch", func(t *testing.T) {
// Test case where CreatedBy differs (e.g., empty vs populated)
tombstone := Issue{
ID: "bd-ghost2",
Title: "(deleted)",
Status: StatusTombstone,
Priority: 2,
CreatedAt: "2024-01-01T00:00:00Z",
CreatedBy: "", // Empty CreatedBy
DeletedAt: time.Now().Add(-24 * time.Hour).Format(time.RFC3339),
DeletedBy: "user2",
DeleteReason: "Cleanup",
}
closedIssue := Issue{
ID: "bd-ghost2",
Title: "Original title",
Status: "closed",
Priority: 2,
CreatedAt: "2024-01-01T00:00:00Z",
CreatedBy: "user1", // Non-empty CreatedBy
}
base := []Issue{}
left := []Issue{tombstone}
right := []Issue{closedIssue}
result, conflicts := merge3Way(base, left, right)
if len(conflicts) != 0 {
t.Errorf("unexpected conflicts: %v", conflicts)
}
// Should have exactly 1 issue
if len(result) != 1 {
t.Fatalf("expected 1 issue (no duplicates), got %d", len(result))
}
// Tombstone should win
if result[0].Status != StatusTombstone {
t.Errorf("expected tombstone to win despite CreatedBy mismatch, got status %q", result[0].Status)
}
})
t.Run("no duplicates when both have same ID but different keys", func(t *testing.T) {
// Ensure we don't create duplicate entries
liveLeft := Issue{
ID: "bd-ghost3",
Title: "Left version",
Status: "open",
CreatedAt: "2024-01-01T00:00:00.123456Z", // With nanoseconds
CreatedBy: "user1",
}
liveRight := Issue{
ID: "bd-ghost3",
Title: "Right version",
Status: "in_progress",
CreatedAt: "2024-01-01T00:00:00Z", // Without nanoseconds
CreatedBy: "user1",
}
base := []Issue{}
left := []Issue{liveLeft}
right := []Issue{liveRight}
result, conflicts := merge3Way(base, left, right)
if len(conflicts) != 0 {
t.Errorf("unexpected conflicts: %v", conflicts)
}
// CRITICAL: Should have exactly 1 issue, not 2
if len(result) != 1 {
t.Fatalf("expected 1 issue (no duplicates for same ID), got %d", len(result))
}
})
}