fix(merge): preserve close_reason field during merge/sync (GH#891)
The close_reason and closed_by_session fields were being silently dropped during 3-way merge operations because the simplified Issue struct in internal/merge/merge.go was missing these fields. Changes: - Add CloseReason and ClosedBySession fields to merge.Issue struct - Implement merge logic that preserves these fields when status is closed - Use timestamp-based conflict resolution (later closed_at wins) - Clear close metadata when status becomes non-closed 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -48,10 +48,12 @@ type Issue struct {
|
|||||||
Status string `json:"status,omitempty"`
|
Status string `json:"status,omitempty"`
|
||||||
Priority int `json:"priority"` // No omitempty: 0 is valid (P0/critical)
|
Priority int `json:"priority"` // No omitempty: 0 is valid (P0/critical)
|
||||||
IssueType string `json:"issue_type,omitempty"`
|
IssueType string `json:"issue_type,omitempty"`
|
||||||
CreatedAt string `json:"created_at,omitempty"`
|
CreatedAt string `json:"created_at,omitempty"`
|
||||||
UpdatedAt string `json:"updated_at,omitempty"`
|
UpdatedAt string `json:"updated_at,omitempty"`
|
||||||
ClosedAt string `json:"closed_at,omitempty"`
|
ClosedAt string `json:"closed_at,omitempty"`
|
||||||
CreatedBy string `json:"created_by,omitempty"`
|
CloseReason string `json:"close_reason,omitempty"` // Reason provided when closing (GH#891)
|
||||||
|
ClosedBySession string `json:"closed_by_session,omitempty"` // Session that closed this issue (GH#891)
|
||||||
|
CreatedBy string `json:"created_by,omitempty"`
|
||||||
Dependencies []Dependency `json:"dependencies,omitempty"`
|
Dependencies []Dependency `json:"dependencies,omitempty"`
|
||||||
RawLine string `json:"-"` // Store original line for conflict output
|
RawLine string `json:"-"` // Store original line for conflict output
|
||||||
// Tombstone fields: inline soft-delete support for merge
|
// Tombstone fields: inline soft-delete support for merge
|
||||||
@@ -596,8 +598,23 @@ func mergeIssue(base, left, right Issue) (Issue, string) {
|
|||||||
// This prevents invalid state (status=open with closed_at set)
|
// This prevents invalid state (status=open with closed_at set)
|
||||||
if result.Status == StatusClosed {
|
if result.Status == StatusClosed {
|
||||||
result.ClosedAt = maxTime(left.ClosedAt, right.ClosedAt)
|
result.ClosedAt = maxTime(left.ClosedAt, right.ClosedAt)
|
||||||
|
// Merge close_reason and closed_by_session - use value from side with later closed_at (GH#891)
|
||||||
|
// This ensures we keep the most recent close action's metadata
|
||||||
|
if isTimeAfter(left.ClosedAt, right.ClosedAt) {
|
||||||
|
result.CloseReason = left.CloseReason
|
||||||
|
result.ClosedBySession = left.ClosedBySession
|
||||||
|
} else if right.ClosedAt != "" {
|
||||||
|
result.CloseReason = right.CloseReason
|
||||||
|
result.ClosedBySession = right.ClosedBySession
|
||||||
|
} else {
|
||||||
|
// Both empty or only left has value - prefer left
|
||||||
|
result.CloseReason = left.CloseReason
|
||||||
|
result.ClosedBySession = left.ClosedBySession
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
result.ClosedAt = ""
|
result.ClosedAt = ""
|
||||||
|
result.CloseReason = ""
|
||||||
|
result.ClosedBySession = ""
|
||||||
}
|
}
|
||||||
|
|
||||||
// Merge dependencies - proper 3-way merge where removals win
|
// Merge dependencies - proper 3-way merge where removals win
|
||||||
|
|||||||
@@ -2460,3 +2460,265 @@ func TestMerge3Way_DeterministicOutputOrder(t *testing.T) {
|
|||||||
}
|
}
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
// TestMerge3Way_CloseReasonPreservation tests that close_reason and closed_by_session
|
||||||
|
// are preserved during merge/sync operations (GH#891)
|
||||||
|
func TestMerge3Way_CloseReasonPreservation(t *testing.T) {
|
||||||
|
t.Run("close_reason preserved when both sides closed - later closed_at wins", func(t *testing.T) {
|
||||||
|
base := []Issue{
|
||||||
|
{
|
||||||
|
ID: "bd-close1",
|
||||||
|
Title: "Test Issue",
|
||||||
|
Status: "open",
|
||||||
|
CreatedAt: "2024-01-01T00:00:00Z",
|
||||||
|
CreatedBy: "user1",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
left := []Issue{
|
||||||
|
{
|
||||||
|
ID: "bd-close1",
|
||||||
|
Title: "Test Issue",
|
||||||
|
Status: "closed",
|
||||||
|
ClosedAt: "2024-01-02T00:00:00Z", // Earlier
|
||||||
|
CloseReason: "Fixed in commit abc",
|
||||||
|
ClosedBySession: "session-left",
|
||||||
|
CreatedAt: "2024-01-01T00:00:00Z",
|
||||||
|
CreatedBy: "user1",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
right := []Issue{
|
||||||
|
{
|
||||||
|
ID: "bd-close1",
|
||||||
|
Title: "Test Issue",
|
||||||
|
Status: "closed",
|
||||||
|
ClosedAt: "2024-01-03T00:00:00Z", // Later - should win
|
||||||
|
CloseReason: "Fixed in commit xyz",
|
||||||
|
ClosedBySession: "session-right",
|
||||||
|
CreatedAt: "2024-01-01T00:00:00Z",
|
||||||
|
CreatedBy: "user1",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
result, conflicts := merge3Way(base, left, right, false)
|
||||||
|
if len(conflicts) != 0 {
|
||||||
|
t.Errorf("expected no conflicts, got %d", len(conflicts))
|
||||||
|
}
|
||||||
|
if len(result) != 1 {
|
||||||
|
t.Fatalf("expected 1 merged issue, got %d", len(result))
|
||||||
|
}
|
||||||
|
// Right has later closed_at, so right's close_reason should win
|
||||||
|
if result[0].CloseReason != "Fixed in commit xyz" {
|
||||||
|
t.Errorf("expected close_reason 'Fixed in commit xyz', got %q", result[0].CloseReason)
|
||||||
|
}
|
||||||
|
if result[0].ClosedBySession != "session-right" {
|
||||||
|
t.Errorf("expected closed_by_session 'session-right', got %q", result[0].ClosedBySession)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("close_reason preserved when left has later closed_at", func(t *testing.T) {
|
||||||
|
base := []Issue{
|
||||||
|
{
|
||||||
|
ID: "bd-close2",
|
||||||
|
Title: "Test Issue",
|
||||||
|
Status: "open",
|
||||||
|
CreatedAt: "2024-01-01T00:00:00Z",
|
||||||
|
CreatedBy: "user1",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
left := []Issue{
|
||||||
|
{
|
||||||
|
ID: "bd-close2",
|
||||||
|
Title: "Test Issue",
|
||||||
|
Status: "closed",
|
||||||
|
ClosedAt: "2024-01-03T00:00:00Z", // Later - should win
|
||||||
|
CloseReason: "Resolved by PR #123",
|
||||||
|
ClosedBySession: "session-left",
|
||||||
|
CreatedAt: "2024-01-01T00:00:00Z",
|
||||||
|
CreatedBy: "user1",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
right := []Issue{
|
||||||
|
{
|
||||||
|
ID: "bd-close2",
|
||||||
|
Title: "Test Issue",
|
||||||
|
Status: "closed",
|
||||||
|
ClosedAt: "2024-01-02T00:00:00Z", // Earlier
|
||||||
|
CloseReason: "Duplicate",
|
||||||
|
ClosedBySession: "session-right",
|
||||||
|
CreatedAt: "2024-01-01T00:00:00Z",
|
||||||
|
CreatedBy: "user1",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
result, conflicts := merge3Way(base, left, right, false)
|
||||||
|
if len(conflicts) != 0 {
|
||||||
|
t.Errorf("expected no conflicts, got %d", len(conflicts))
|
||||||
|
}
|
||||||
|
if len(result) != 1 {
|
||||||
|
t.Fatalf("expected 1 merged issue, got %d", len(result))
|
||||||
|
}
|
||||||
|
// Left has later closed_at, so left's close_reason should win
|
||||||
|
if result[0].CloseReason != "Resolved by PR #123" {
|
||||||
|
t.Errorf("expected close_reason 'Resolved by PR #123', got %q", result[0].CloseReason)
|
||||||
|
}
|
||||||
|
if result[0].ClosedBySession != "session-left" {
|
||||||
|
t.Errorf("expected closed_by_session 'session-left', got %q", result[0].ClosedBySession)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("close_reason cleared when status becomes open", func(t *testing.T) {
|
||||||
|
base := []Issue{
|
||||||
|
{
|
||||||
|
ID: "bd-close3",
|
||||||
|
Title: "Test Issue",
|
||||||
|
Status: "closed",
|
||||||
|
ClosedAt: "2024-01-02T00:00:00Z",
|
||||||
|
CloseReason: "Fixed",
|
||||||
|
ClosedBySession: "session-old",
|
||||||
|
CreatedAt: "2024-01-01T00:00:00Z",
|
||||||
|
CreatedBy: "user1",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
left := []Issue{
|
||||||
|
{
|
||||||
|
ID: "bd-close3",
|
||||||
|
Title: "Test Issue",
|
||||||
|
Status: "open", // Reopened
|
||||||
|
ClosedAt: "",
|
||||||
|
CloseReason: "", // Should be cleared
|
||||||
|
ClosedBySession: "",
|
||||||
|
CreatedAt: "2024-01-01T00:00:00Z",
|
||||||
|
CreatedBy: "user1",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
right := []Issue{
|
||||||
|
{
|
||||||
|
ID: "bd-close3",
|
||||||
|
Title: "Test Issue",
|
||||||
|
Status: "open", // Both reopened
|
||||||
|
ClosedAt: "",
|
||||||
|
CloseReason: "",
|
||||||
|
ClosedBySession: "",
|
||||||
|
CreatedAt: "2024-01-01T00:00:00Z",
|
||||||
|
CreatedBy: "user1",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
result, conflicts := merge3Way(base, left, right, false)
|
||||||
|
if len(conflicts) != 0 {
|
||||||
|
t.Errorf("expected no conflicts, got %d", len(conflicts))
|
||||||
|
}
|
||||||
|
if len(result) != 1 {
|
||||||
|
t.Fatalf("expected 1 merged issue, got %d", len(result))
|
||||||
|
}
|
||||||
|
if result[0].Status != "open" {
|
||||||
|
t.Errorf("expected status 'open', got %q", result[0].Status)
|
||||||
|
}
|
||||||
|
if result[0].CloseReason != "" {
|
||||||
|
t.Errorf("expected empty close_reason when reopened, got %q", result[0].CloseReason)
|
||||||
|
}
|
||||||
|
if result[0].ClosedBySession != "" {
|
||||||
|
t.Errorf("expected empty closed_by_session when reopened, got %q", result[0].ClosedBySession)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("close_reason from single side preserved", func(t *testing.T) {
|
||||||
|
base := []Issue{
|
||||||
|
{
|
||||||
|
ID: "bd-close4",
|
||||||
|
Title: "Test Issue",
|
||||||
|
Status: "open",
|
||||||
|
CreatedAt: "2024-01-01T00:00:00Z",
|
||||||
|
CreatedBy: "user1",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
left := []Issue{
|
||||||
|
{
|
||||||
|
ID: "bd-close4",
|
||||||
|
Title: "Test Issue",
|
||||||
|
Status: "closed",
|
||||||
|
ClosedAt: "2024-01-02T00:00:00Z",
|
||||||
|
CloseReason: "Won't fix - by design",
|
||||||
|
ClosedBySession: "session-abc",
|
||||||
|
CreatedAt: "2024-01-01T00:00:00Z",
|
||||||
|
CreatedBy: "user1",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
right := []Issue{
|
||||||
|
{
|
||||||
|
ID: "bd-close4",
|
||||||
|
Title: "Test Issue",
|
||||||
|
Status: "open", // Still open on right
|
||||||
|
CreatedAt: "2024-01-01T00:00:00Z",
|
||||||
|
CreatedBy: "user1",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
result, conflicts := merge3Way(base, left, right, false)
|
||||||
|
if len(conflicts) != 0 {
|
||||||
|
t.Errorf("expected no conflicts, got %d", len(conflicts))
|
||||||
|
}
|
||||||
|
if len(result) != 1 {
|
||||||
|
t.Fatalf("expected 1 merged issue, got %d", len(result))
|
||||||
|
}
|
||||||
|
// Closed wins over open
|
||||||
|
if result[0].Status != "closed" {
|
||||||
|
t.Errorf("expected status 'closed', got %q", result[0].Status)
|
||||||
|
}
|
||||||
|
// Close reason from the closed side should be preserved
|
||||||
|
if result[0].CloseReason != "Won't fix - by design" {
|
||||||
|
t.Errorf("expected close_reason 'Won't fix - by design', got %q", result[0].CloseReason)
|
||||||
|
}
|
||||||
|
if result[0].ClosedBySession != "session-abc" {
|
||||||
|
t.Errorf("expected closed_by_session 'session-abc', got %q", result[0].ClosedBySession)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("close_reason survives round-trip through JSONL", func(t *testing.T) {
|
||||||
|
// This tests the full merge pipeline including JSON marshaling/unmarshaling
|
||||||
|
tmpDir := t.TempDir()
|
||||||
|
|
||||||
|
baseContent := `{"id":"bd-jsonl1","title":"Test Issue","status":"open","created_at":"2024-01-01T00:00:00Z","created_by":"user1"}`
|
||||||
|
leftContent := `{"id":"bd-jsonl1","title":"Test Issue","status":"closed","closed_at":"2024-01-02T00:00:00Z","close_reason":"Fixed in commit def456","closed_by_session":"session-jsonl","created_at":"2024-01-01T00:00:00Z","created_by":"user1"}`
|
||||||
|
rightContent := `{"id":"bd-jsonl1","title":"Test Issue","status":"open","created_at":"2024-01-01T00:00:00Z","created_by":"user1"}`
|
||||||
|
|
||||||
|
basePath := filepath.Join(tmpDir, "base.jsonl")
|
||||||
|
leftPath := filepath.Join(tmpDir, "left.jsonl")
|
||||||
|
rightPath := filepath.Join(tmpDir, "right.jsonl")
|
||||||
|
outputPath := filepath.Join(tmpDir, "output.jsonl")
|
||||||
|
|
||||||
|
if err := os.WriteFile(basePath, []byte(baseContent+"\n"), 0644); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if err := os.WriteFile(leftPath, []byte(leftContent+"\n"), 0644); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if err := os.WriteFile(rightPath, []byte(rightContent+"\n"), 0644); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := Merge3Way(outputPath, basePath, leftPath, rightPath, false); err != nil {
|
||||||
|
t.Fatalf("Merge3Way failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Read output and verify close_reason is preserved
|
||||||
|
outputData, err := os.ReadFile(outputPath)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
var outputIssue Issue
|
||||||
|
if err := json.Unmarshal(outputData[:len(outputData)-1], &outputIssue); err != nil {
|
||||||
|
t.Fatalf("failed to parse output: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if outputIssue.Status != "closed" {
|
||||||
|
t.Errorf("expected status 'closed', got %q", outputIssue.Status)
|
||||||
|
}
|
||||||
|
if outputIssue.CloseReason != "Fixed in commit def456" {
|
||||||
|
t.Errorf("expected close_reason 'Fixed in commit def456', got %q", outputIssue.CloseReason)
|
||||||
|
}
|
||||||
|
if outputIssue.ClosedBySession != "session-jsonl" {
|
||||||
|
t.Errorf("expected closed_by_session 'session-jsonl', got %q", outputIssue.ClosedBySession)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user