johno/beads
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(merge): proper 3-way merge for dependencies - removals win (bd-ndye)

Browse Source 
CRITICAL: Fixed dependency resurrection bug that caused removed/orphaned
dependencies to keep coming back after sync.

Root cause: mergeDependencies() was doing a union (additive only) and
completely ignored the base parameter. This meant any dependency present
in either left or right would be included, even if it was intentionally
removed.

Fix: Proper 3-way merge where REMOVALS ARE AUTHORITATIVE:
- If dep was in base and removed by left OR right → stays removed
- If dep wasn't in base and added by left OR right → included
- If dep was in base and both still have it → included

This fixes months of issues with orphaned parent-child relationships
being resurrected during git sync operations.

🤖 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-18 23:20:49 -08:00
parent 45bb38770a
commit 5ad1c80f7e
4 changed files with 130 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

92
internal/merge/merge.go
View File

@@ -575,8 +575,8 @@ func mergeIssue(base, left, right Issue) (Issue, string) {
result.ClosedAt = ""
}
// Merge dependencies - combine and deduplicate
result.Dependencies = mergeDependencies(left.Dependencies, right.Dependencies)
// Merge dependencies - proper 3-way merge where removals win (bd-ndye)
result.Dependencies = mergeDependencies(base.Dependencies, left.Dependencies, right.Dependencies)
// bd-1sn: If status became tombstone via mergeStatus safety fallback,
// copy tombstone fields from whichever side has them
@@ -792,23 +792,87 @@ func maxTime(t1, t2 string) string {
return t2
}
func mergeDependencies(left, right []Dependency) []Dependency {
seen := make(map[string]bool)
var result []Dependency
for _, dep := range left {
key := fmt.Sprintf("%s:%s:%s", dep.IssueID, dep.DependsOnID, dep.Type)
if !seen[key] {
seen[key] = true
result = append(result, dep)
}
// mergeDependencies performs a proper 3-way merge of dependencies (bd-ndye)
// Key principle: REMOVALS ARE AUTHORITATIVE
// - If dep was in base and removed by left OR right → exclude (removal wins)
// - If dep wasn't in base and added by left OR right → include
// - If dep was in base and both still have it → include
func mergeDependencies(base, left, right []Dependency) []Dependency {
// Build sets for O(1) lookup
depKey := func(dep Dependency) string {
return fmt.Sprintf("%s:%s:%s", dep.IssueID, dep.DependsOnID, dep.Type)
}
baseSet := make(map[string]bool)
for _, dep := range base {
baseSet[depKey(dep)] = true
}
leftSet := make(map[string]bool)
leftDeps := make(map[string]Dependency)
for _, dep := range left {
key := depKey(dep)
leftSet[key] = true
leftDeps[key] = dep
}
rightSet := make(map[string]bool)
rightDeps := make(map[string]Dependency)
for _, dep := range right {
key := fmt.Sprintf("%s:%s:%s", dep.IssueID, dep.DependsOnID, dep.Type)
key := depKey(dep)
rightSet[key] = true
rightDeps[key] = dep
}
// Collect all unique keys
allKeys := make(map[string]bool)
for k := range baseSet {
allKeys[k] = true
}
for k := range leftSet {
allKeys[k] = true
}
for k := range rightSet {
allKeys[k] = true
}
var result []Dependency
seen := make(map[string]bool)
for key := range allKeys {
inBase := baseSet[key]
inLeft := leftSet[key]
inRight := rightSet[key]
// 3-way merge logic:
if inBase {
// Was in base - check if either side removed it
if !inLeft {
// Left removed it → don't include (left wins)
continue
}
if !inRight {
// Right removed it → don't include (right wins)
continue
}
// Both still have it → include
} else {
// Wasn't in base - must have been added by left or right
if !inLeft && !inRight {
// Neither has it (shouldn't happen but handle gracefully)
continue
}
// At least one side added it → include
}
if !seen[key] {
seen[key] = true
result = append(result, dep)
// Prefer left's version of the dep (for any metadata differences)
if dep, ok := leftDeps[key]; ok {
result = append(result, dep)
} else if dep, ok := rightDeps[key]; ok {
result = append(result, dep)
}
}
}

60
internal/merge/merge_test.go
View File

@@ -128,22 +128,25 @@ func TestMergeField(t *testing.T) {
}
}
// TestMergeDependencies tests dependency union and deduplication
// TestMergeDependencies tests 3-way dependency merge with removal semantics (bd-ndye)
func TestMergeDependencies(t *testing.T) {
tests := []struct {
name string
base []Dependency
left []Dependency
right []Dependency
expected []Dependency
}{
{
name: "empty both sides",
name: "empty all sides",
base: []Dependency{},
left: []Dependency{},
right: []Dependency{},
expected: []Dependency{},
},
{
name: "only left has deps",
name: "left adds dep (not in base)",
base: []Dependency{},
left: []Dependency{
{IssueID: "bd-1", DependsOnID: "bd-2", Type: "blocks", CreatedAt: "2024-01-01T00:00:00Z"},
},
@@ -153,7 +156,8 @@ func TestMergeDependencies(t *testing.T) {
},
},
{
name: "only right has deps",
name: "right adds dep (not in base)",
base: []Dependency{},
left: []Dependency{},
right: []Dependency{
{IssueID: "bd-1", DependsOnID: "bd-3", Type: "related", CreatedAt: "2024-01-01T00:00:00Z"},
@@ -163,7 +167,8 @@ func TestMergeDependencies(t *testing.T) {
},
},
{
name: "union of different deps",
name: "both add different deps (not in base)",
base: []Dependency{},
left: []Dependency{
{IssueID: "bd-1", DependsOnID: "bd-2", Type: "blocks", CreatedAt: "2024-01-01T00:00:00Z"},
},
@@ -176,38 +181,61 @@ func TestMergeDependencies(t *testing.T) {
},
},
{
name: "deduplication of identical deps",
left: []Dependency{
name: "left removes dep from base - REMOVAL WINS",
base: []Dependency{
{IssueID: "bd-1", DependsOnID: "bd-2", Type: "blocks", CreatedAt: "2024-01-01T00:00:00Z"},
},
left: []Dependency{}, // Left removed it
right: []Dependency{
{IssueID: "bd-1", DependsOnID: "bd-2", Type: "blocks", CreatedAt: "2024-01-02T00:00:00Z"}, // Different timestamp but same logical dep
},
expected: []Dependency{
{IssueID: "bd-1", DependsOnID: "bd-2", Type: "blocks", CreatedAt: "2024-01-01T00:00:00Z"},
},
expected: []Dependency{}, // Should be empty - removal wins
},
{
name: "multiple deps with dedup",
name: "right removes dep from base - REMOVAL WINS",
base: []Dependency{
{IssueID: "bd-1", DependsOnID: "bd-2", Type: "blocks", CreatedAt: "2024-01-01T00:00:00Z"},
},
left: []Dependency{
{IssueID: "bd-1", DependsOnID: "bd-2", Type: "blocks", CreatedAt: "2024-01-01T00:00:00Z"},
},
right: []Dependency{}, // Right removed it
expected: []Dependency{}, // Should be empty - removal wins
},
{
name: "both keep dep from base",
base: []Dependency{
{IssueID: "bd-1", DependsOnID: "bd-2", Type: "blocks", CreatedAt: "2024-01-01T00:00:00Z"},
},
left: []Dependency{
{IssueID: "bd-1", DependsOnID: "bd-2", Type: "blocks", CreatedAt: "2024-01-01T00:00:00Z"},
{IssueID: "bd-1", DependsOnID: "bd-3", Type: "related", CreatedAt: "2024-01-01T00:00:00Z"},
},
right: []Dependency{
{IssueID: "bd-1", DependsOnID: "bd-2", Type: "blocks", CreatedAt: "2024-01-02T00:00:00Z"},
{IssueID: "bd-1", DependsOnID: "bd-4", Type: "blocks", CreatedAt: "2024-01-01T00:00:00Z"},
},
expected: []Dependency{
{IssueID: "bd-1", DependsOnID: "bd-2", Type: "blocks", CreatedAt: "2024-01-01T00:00:00Z"},
{IssueID: "bd-1", DependsOnID: "bd-3", Type: "related", CreatedAt: "2024-01-01T00:00:00Z"},
{IssueID: "bd-1", DependsOnID: "bd-4", Type: "blocks", CreatedAt: "2024-01-01T00:00:00Z"},
},
},
{
name: "complex: left removes one, right adds one",
base: []Dependency{
{IssueID: "bd-1", DependsOnID: "bd-2", Type: "blocks", CreatedAt: "2024-01-01T00:00:00Z"},
},
left: []Dependency{}, // Left removed bd-2
right: []Dependency{
{IssueID: "bd-1", DependsOnID: "bd-2", Type: "blocks", CreatedAt: "2024-01-01T00:00:00Z"},
{IssueID: "bd-1", DependsOnID: "bd-3", Type: "related", CreatedAt: "2024-01-01T00:00:00Z"}, // Right added bd-3
},
expected: []Dependency{
{IssueID: "bd-1", DependsOnID: "bd-3", Type: "related", CreatedAt: "2024-01-01T00:00:00Z"}, // Only the new one
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := mergeDependencies(tt.left, tt.right)
result := mergeDependencies(tt.base, tt.left, tt.right)
if len(result) != len(tt.expected) {
t.Errorf("mergeDependencies() returned %d deps, want %d", len(result), len(tt.expected))
return