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

fix(sync): merge instead of overwrite in SyncJSONLToWorktree (bd-52q)

Browse Source 
When syncing JSONL to worktree, if the worktree has more issues than
local, merge them instead of blindly overwriting. This prevents fresh
clones from accidentally deleting remote issues when they sync with
fewer issues than the sync branch.

Root cause of GitHub #464: A fresh clone with sync-branch configured
would start with an empty database (since JSONL is on sync-branch, not
HEAD). When syncing, the local 1-issue JSONL would overwrite the
remotes 10-issue JSONL, and the subsequent 3-way merge would see this
as local deleted 9 issues causing deletion to win.

The fix compares issue counts and triggers a merge when local has fewer
issues than the worktree (remote). Uses 3-way merge with empty base to
combine both sets of issues.

Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Steve Yegge
2025-12-05 14:24:47 -08:00
parent 8fa680a880
commit 26b8013908
2 changed files with 267 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

117
internal/git/worktree.go
View File

@@ -1,11 +1,14 @@
package git
import (
"bufio"
"fmt"
"os"
"os/exec"
"path/filepath"
"strings"
"github.com/steveyegge/beads/internal/merge"
)
// WorktreeManager handles git worktree lifecycle for separate beads branches
@@ -140,11 +143,14 @@ func (wm *WorktreeManager) CheckWorktreeHealth(worktreePath string) error {
return nil
}
// SyncJSONLToWorktree copies the JSONL file from main repo to worktree
// SyncJSONLToWorktree syncs the JSONL file from main repo to worktree.
// If the worktree has issues that the local repo doesn't have, it merges them
// instead of overwriting. This prevents data loss when a fresh clone syncs
// with fewer issues than the remote. (bd-52q fix for GitHub #464)
func (wm *WorktreeManager) SyncJSONLToWorktree(worktreePath, jsonlRelPath string) error {
// Source: main repo JSONL
srcPath := filepath.Join(wm.repoPath, jsonlRelPath)
// Destination: worktree JSONL
dstPath := filepath.Join(worktreePath, jsonlRelPath)
@@ -155,19 +161,118 @@ func (wm *WorktreeManager) SyncJSONLToWorktree(worktreePath, jsonlRelPath string
}
// Read source file
data, err := os.ReadFile(srcPath) // #nosec G304 - controlled path from config
srcData, err := os.ReadFile(srcPath) // #nosec G304 - controlled path from config
if err != nil {
return fmt.Errorf("failed to read source JSONL: %w", err)
}
// Write to destination
if err := os.WriteFile(dstPath, data, 0644); err != nil { // #nosec G306 - JSONL needs to be readable
return fmt.Errorf("failed to write destination JSONL: %w", err)
// Check if destination exists and has content
dstData, dstErr := os.ReadFile(dstPath) // #nosec G304 - controlled path
if dstErr != nil || len(dstData) == 0 {
// Destination doesn't exist or is empty - just copy
if err := os.WriteFile(dstPath, srcData, 0644); err != nil { // #nosec G306 - JSONL needs to be readable
return fmt.Errorf("failed to write destination JSONL: %w", err)
}
return nil
}
// Count issues in both files
srcCount := countJSONLIssues(srcData)
dstCount := countJSONLIssues(dstData)
// If source has same or more issues, just copy (source is authoritative)
if srcCount >= dstCount {
if err := os.WriteFile(dstPath, srcData, 0644); err != nil { // #nosec G306 - JSONL needs to be readable
return fmt.Errorf("failed to write destination JSONL: %w", err)
}
return nil
}
// Source has fewer issues than destination - this indicates the local repo
// doesn't have all the issues from the sync branch. Merge instead of overwrite.
// (bd-52q: This prevents fresh clones from accidentally deleting remote issues)
mergedData, err := wm.mergeJSONLFiles(srcData, dstData)
if err != nil {
// If merge fails, fall back to copy behavior but log warning
// This shouldn't happen but ensures we don't break existing behavior
fmt.Fprintf(os.Stderr, "Warning: JSONL merge failed (%v), falling back to overwrite\n", err)
if writeErr := os.WriteFile(dstPath, srcData, 0644); writeErr != nil { // #nosec G306
return fmt.Errorf("failed to write destination JSONL: %w", writeErr)
}
return nil
}
if err := os.WriteFile(dstPath, mergedData, 0644); err != nil { // #nosec G306 - JSONL needs to be readable
return fmt.Errorf("failed to write merged JSONL: %w", err)
}
return nil
}
// countJSONLIssues counts the number of valid JSON lines in JSONL data
func countJSONLIssues(data []byte) int {
count := 0
scanner := bufio.NewScanner(strings.NewReader(string(data)))
for scanner.Scan() {
line := strings.TrimSpace(scanner.Text())
if line != "" && strings.HasPrefix(line, "{") {
count++
}
}
return count
}
// mergeJSONLFiles merges two JSONL files using 3-way merge with empty base.
// This combines issues from both files, with the source (local) taking precedence
// for issues that exist in both.
func (wm *WorktreeManager) mergeJSONLFiles(srcData, dstData []byte) ([]byte, error) {
// Create temp files for merge
tmpDir, err := os.MkdirTemp("", "bd-worktree-merge-*")
if err != nil {
return nil, fmt.Errorf("failed to create temp dir: %w", err)
}
defer func() { _ = os.RemoveAll(tmpDir) }()
baseFile := filepath.Join(tmpDir, "base.jsonl")
leftFile := filepath.Join(tmpDir, "left.jsonl") // source (local)
rightFile := filepath.Join(tmpDir, "right.jsonl") // destination (worktree)
outputFile := filepath.Join(tmpDir, "merged.jsonl")
// Empty base - treat this as both sides adding issues
if err := os.WriteFile(baseFile, []byte{}, 0600); err != nil {
return nil, fmt.Errorf("failed to write base file: %w", err)
}
// Source (local) is "left" - takes precedence for conflicts
if err := os.WriteFile(leftFile, srcData, 0600); err != nil {
return nil, fmt.Errorf("failed to write left file: %w", err)
}
// Destination (worktree) is "right"
if err := os.WriteFile(rightFile, dstData, 0600); err != nil {
return nil, fmt.Errorf("failed to write right file: %w", err)
}
// Perform 3-way merge
err = merge.Merge3Way(outputFile, baseFile, leftFile, rightFile, false)
if err != nil {
// Check if it's just a conflict warning (merge still produced output)
if !strings.Contains(err.Error(), "merge completed with") {
return nil, fmt.Errorf("3-way merge failed: %w", err)
}
// Conflicts are auto-resolved, continue
}
// Read merged result
mergedData, err := os.ReadFile(outputFile) // #nosec G304 - temp file we created
if err != nil {
return nil, fmt.Errorf("failed to read merged file: %w", err)
}
return mergedData, nil
}
// isValidWorktree checks if the path is a valid git worktree
func (wm *WorktreeManager) isValidWorktree(worktreePath string) (bool, error) {
cmd := exec.Command("git", "worktree", "list", "--porcelain")

156
internal/git/worktree_test.go
View File

@@ -589,3 +589,159 @@ func TestConfigureSparseCheckoutErrors(t *testing.T) {
}
})
}
// TestSyncJSONLToWorktreeMerge tests the merge behavior when worktree has more issues
// than the local repo (bd-52q fix for GitHub #464)
func TestSyncJSONLToWorktreeMerge(t *testing.T) {
repoPath, cleanup := setupTestRepo(t)
defer cleanup()
wm := NewWorktreeManager(repoPath)
worktreePath := filepath.Join(t.TempDir(), "beads-worktree")
// Create worktree
if err := wm.CreateBeadsWorktree("beads-metadata", worktreePath); err != nil {
t.Fatalf("CreateBeadsWorktree failed: %v", err)
}
t.Run("merges when worktree has more issues than local", func(t *testing.T) {
// Set up: worktree has 3 issues (simulating remote state)
worktreeJSONL := filepath.Join(worktreePath, ".beads", "issues.jsonl")
worktreeData := `{"id":"bd-001","title":"Issue 1","status":"open","created_at":"2025-01-01T00:00:00Z","created_by":"user1"}
{"id":"bd-002","title":"Issue 2","status":"open","created_at":"2025-01-01T00:00:01Z","created_by":"user1"}
{"id":"bd-003","title":"Issue 3","status":"open","created_at":"2025-01-01T00:00:02Z","created_by":"user1"}
`
if err := os.WriteFile(worktreeJSONL, []byte(worktreeData), 0644); err != nil {
t.Fatalf("Failed to write worktree JSONL: %v", err)
}
// Local has only 1 issue (simulating fresh clone that hasn't synced)
mainJSONL := filepath.Join(repoPath, ".beads", "issues.jsonl")
mainData := `{"id":"bd-004","title":"New Issue","status":"open","created_at":"2025-01-02T00:00:00Z","created_by":"user2"}
`
if err := os.WriteFile(mainJSONL, []byte(mainData), 0644); err != nil {
t.Fatalf("Failed to write main JSONL: %v", err)
}
// Sync should MERGE, not overwrite
if err := wm.SyncJSONLToWorktree(worktreePath, ".beads/issues.jsonl"); err != nil {
t.Fatalf("SyncJSONLToWorktree failed: %v", err)
}
// Read the result
resultData, err := os.ReadFile(worktreeJSONL)
if err != nil {
t.Fatalf("Failed to read result JSONL: %v", err)
}
// Should have all 4 issues (3 from worktree + 1 from local)
resultCount := countJSONLIssues(resultData)
if resultCount != 4 {
t.Errorf("Expected 4 issues after merge, got %d\nContent:\n%s", resultCount, string(resultData))
}
// Verify specific issues are present
resultStr := string(resultData)
for _, id := range []string{"bd-001", "bd-002", "bd-003", "bd-004"} {
if !strings.Contains(resultStr, id) {
t.Errorf("Expected issue %s to be in merged result", id)
}
}
})
t.Run("overwrites when local has same or more issues", func(t *testing.T) {
// Set up: worktree has 2 issues
worktreeJSONL := filepath.Join(worktreePath, ".beads", "issues.jsonl")
worktreeData := `{"id":"bd-010","title":"Old 1","status":"open","created_at":"2025-01-01T00:00:00Z","created_by":"user1"}
{"id":"bd-011","title":"Old 2","status":"open","created_at":"2025-01-01T00:00:01Z","created_by":"user1"}
`
if err := os.WriteFile(worktreeJSONL, []byte(worktreeData), 0644); err != nil {
t.Fatalf("Failed to write worktree JSONL: %v", err)
}
// Local has 3 issues (more than worktree)
mainJSONL := filepath.Join(repoPath, ".beads", "issues.jsonl")
mainData := `{"id":"bd-020","title":"New 1","status":"open","created_at":"2025-01-02T00:00:00Z","created_by":"user2"}
{"id":"bd-021","title":"New 2","status":"open","created_at":"2025-01-02T00:00:01Z","created_by":"user2"}
{"id":"bd-022","title":"New 3","status":"open","created_at":"2025-01-02T00:00:02Z","created_by":"user2"}
`
if err := os.WriteFile(mainJSONL, []byte(mainData), 0644); err != nil {
t.Fatalf("Failed to write main JSONL: %v", err)
}
// Sync should OVERWRITE (local is authoritative when it has more)
if err := wm.SyncJSONLToWorktree(worktreePath, ".beads/issues.jsonl"); err != nil {
t.Fatalf("SyncJSONLToWorktree failed: %v", err)
}
// Read the result
resultData, err := os.ReadFile(worktreeJSONL)
if err != nil {
t.Fatalf("Failed to read result JSONL: %v", err)
}
// Should have exactly 3 issues (from local)
resultCount := countJSONLIssues(resultData)
if resultCount != 3 {
t.Errorf("Expected 3 issues after overwrite, got %d", resultCount)
}
// Should have local issues, not worktree issues
resultStr := string(resultData)
if strings.Contains(resultStr, "bd-010") || strings.Contains(resultStr, "bd-011") {
t.Error("Old worktree issues should have been overwritten")
}
if !strings.Contains(resultStr, "bd-020") || !strings.Contains(resultStr, "bd-021") || !strings.Contains(resultStr, "bd-022") {
t.Error("New local issues should be present")
}
})
}
func TestCountJSONLIssues(t *testing.T) {
tests := []struct {
name string
data string
expected int
}{
{
name: "empty file",
data: "",
expected: 0,
},
{
name: "single issue",
data: `{"id":"bd-001","title":"Test"}`,
expected: 1,
},
{
name: "multiple issues",
data: `{"id":"bd-001","title":"Test 1"}
{"id":"bd-002","title":"Test 2"}
{"id":"bd-003","title":"Test 3"}`,
expected: 3,
},
{
name: "with blank lines",
data: `{"id":"bd-001","title":"Test 1"}
{"id":"bd-002","title":"Test 2"}
`,
expected: 2,
},
{
name: "non-JSON lines ignored",
data: "# comment\n{\"id\":\"bd-001\"}\nnot json",
expected: 1,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
count := countJSONLIssues([]byte(tt.data))
if count != tt.expected {
t.Errorf("countJSONLIssues() = %d, want %d", count, tt.expected)
}
})
}
}