diff --git a/internal/git/worktree.go b/internal/git/worktree.go index a825a0a2..b028bcef 100644 --- a/internal/git/worktree.go +++ b/internal/git/worktree.go @@ -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") diff --git a/internal/git/worktree_test.go b/internal/git/worktree_test.go index 05f6835a..056fbfea 100644 --- a/internal/git/worktree_test.go +++ b/internal/git/worktree_test.go @@ -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) + } + }) + } +}