diff --git a/BD-3S8-CHANGES.md b/BD-3S8-CHANGES.md new file mode 100644 index 00000000..64cb1d10 --- /dev/null +++ b/BD-3S8-CHANGES.md @@ -0,0 +1,190 @@ +# BD-3S8: Multi-Clone Sync Fix + +## Problem + +When multiple clones of a repository both commit to the `beads-sync` branch and one tries to pull, git's merge would fail due to diverged histories. This made multi-clone workflows unreliable. + +## Solution + +Replace git's commit-level merge with a content-based merge that handles divergence gracefully: + +1. **Fetch** (not pull) from remote +2. **Detect divergence** using `git rev-list --left-right --count` +3. **Extract JSONL** from merge base, local HEAD, and remote +4. **Merge content** using bd's 3-way merge algorithm +5. **Reset to remote's history** (adopt their commit graph) +6. **Commit merged content** on top + +This ensures sync never fails due to git merge conflicts - we handle merging at the JSONL content level where we have semantic understanding of the data. + +## Changes + +### `internal/syncbranch/worktree.go` + +**New functions:** +- `getDivergence()` - Detects how many commits local/remote are ahead/behind +- `performContentMerge()` - Extracts and merges JSONL content from base/local/remote +- `performDeletionsMerge()` - Merges deletions.jsonl by union (keeps all deletions) +- `extractJSONLFromCommit()` - Extracts file content from a specific git commit +- `copyJSONLToMainRepo()` - Refactored helper for copying JSONL files +- `preemptiveFetchAndFastForward()` - Reduces divergence by fetching before commit + +**Modified functions:** +- `PullFromSyncBranch()` - Now handles three cases: + - Already up-to-date: Remote has nothing new + - Fast-forward: Simple `--ff-only` merge + - **Diverged**: Content-based merge (the fix) +- `CommitToSyncBranch()` - Now fetches and fast-forwards before committing + +**Enhanced structs:** +- `PullResult` - Added `Merged` and `FastForwarded` fields + +### `cmd/bd/sync.go` + +- Updated output messages to show merge type (fast-forward vs merged divergent histories) + +### `internal/syncbranch/worktree_divergence_test.go` (new file) + +Test coverage for: +- `getDivergence()` - 4 scenarios +- `extractJSONLFromCommit()` - 3 scenarios +- `performContentMerge()` - 2 scenarios +- `performDeletionsMerge()` - 2 scenarios + +## How It Works + +``` +Clone A commits and pushes: origin/beads-sync = A -- B -- C +Clone B commits locally: local beads-sync = A -- B -- D + +When Clone B syncs: +1. Fetch: gets C from origin +2. Detect divergence: local ahead 1, remote ahead 1 +3. Find merge base: B +4. Extract: base=B's JSONL, local=D's JSONL, remote=C's JSONL +5. Content merge: merge JSONL using 3-way algorithm +6. Reset to origin: beads-sync = A -- B -- C +7. Commit merged: beads-sync = A -- B -- C -- M (merged content) +8. Push: no conflict, linear history +``` + +## Merge Rules + +The 3-way merge uses these rules (from `internal/merge/merge.go`): + +- **New issues**: Added from both sides +- **Deleted issues**: Deletion wins over modification +- **Modified issues**: Field-level merge + - `status`: "closed" always wins over "open" + - `updated_at`: Takes the max (latest) + - `closed_at`: Only set if status is "closed" + - `dependencies`: Union of both sides + - Other fields: Standard 3-way merge + +## Edge Cases Handled + +1. **Remote branch doesn't exist** - Nothing to pull, return early +2. **No common ancestor** - Use empty base for merge +3. **File doesn't exist in commit** - Use empty content +4. **Deletions.jsonl missing** - Non-fatal, skip deletion merge +5. **True conflicts** - Currently fails with error (manual resolution required) + +## Future Improvements + +### 1. Auto-Resolve All Conflicts (No Manual Resolution Required) + +Currently, true conflicts (both sides changed same field to different values) fail the sync. This should be changed to auto-resolve deterministically: + +| Field | Auto-Resolution Strategy | +|-------|-------------------------| +| `updated_at` | Already handled - takes max (latest) | +| `closed_at` | Already handled - takes max (latest) | +| `status` | Already handled - "closed" wins | +| `Priority` | Take higher priority (lower number = more urgent) | +| `IssueType` | Take left (local wins) | +| `Notes` | **Concatenate both** with separator (preserves all contributions) | +| `Title` | Take from side with latest `updated_at` on the issue | +| `Description` | Take from side with latest `updated_at` on the issue | + +With this strategy, **no conflicts ever require manual resolution** - there's always a deterministic auto-resolution. The merge driver becomes fully automatic. + +### 2. Auto-Push After Merge (Default Behavior) + +Users shouldn't need to review merge diffs on beads metadata. The goal is "one command that just works": + +``` +bd sync # Should handle everything, including push +``` + +**Proposed behavior:** +- After successful content merge, auto-push by default +- Only hold off on push when unsafe conditions detected + +**Safety checks before auto-push:** +1. No conflict markers in JSONL (shouldn't happen with full auto-resolve) +2. Issue count sanity check - didn't drop to zero unexpectedly +3. Reasonable deletion threshold - didn't delete > N% of issues in one sync + +**The deletions manifest problem:** +- In multi-clone environments, deletions from one clone propagate to others +- This is correct behavior, but can feel like "corruption" when unexpected +- Swarms legitimately close/delete all issues sometimes +- Hard to distinguish "swarm finished all work" from "corruption" + +**Proposed safeguards:** +- Track whether issues were *closed* (status change) vs *deleted* (removed from JSONL) +- Closing all issues = legitimate (swarm finished) +- Deleting all issues when there were many = suspicious, pause for confirmation +- Config option: `sync.auto_push` (default: true, can set to false for paranoid mode) + +**Integration with bd doctor:** +- `bd doctor --fix` should also run this recovery logic +- But `bd doctor` is for daily/upgrade maintenance, not inner loop +- `bd sync` must handle divergence recovery itself + +**The "one nuclear fix" philosophy:** +- `bd sync` should just work 99.9% of the time +- Auto-resolve all conflicts +- Auto-push when safe +- Only fail/pause when genuinely dangerous (mass deletion detected) + +### 3. V1 Implementation Plan + +Keep it simple for the first iteration: + +**Auto-push behavior:** +1. After successful content merge, auto-push by default +2. One safety check: if issue count dropped by >50% AND there were >5 issues before, log a warning but still push +3. Config option `sync.require_confirmation_on_mass_delete` (default: false) for paranoid users who want to be prompted + +**Rationale:** +- Logging gives forensics if something goes wrong +- Doesn't block the happy path (99.9% of syncs) +- Users who've been burned can enable confirmation mode +- We can tighten safeguards later based on real-world feedback + +**What "mass deletion" means:** +- Issues that **vanished** from `issues.jsonl` (not just closed) +- `status=closed` is fine - swarm finished legitimately +- Issues disappearing entirely is suspicious + +**Future safeguards (not v1):** +- Tombstone TTL: Ignore deletions older than N days +- Deletion rate limit: Pause if deletions.jsonl suddenly has 100+ new entries +- Protected issues: Certain issues can't be deleted via sync + +--- + +## Summary of Work Items + +1. **Already implemented (this PR):** + - Content-based merge for diverged histories + - Pre-emptive fetch before commit + - Deletions.jsonl merge + - Fast-forward detection + +2. **Still to implement:** + - Auto-resolve all field conflicts (no manual resolution) + - Auto-push after merge with safety check + - Mass deletion warning/logging + - Config option for confirmation mode diff --git a/cmd/bd/sync.go b/cmd/bd/sync.go index 3f9e6e30..209023b4 100644 --- a/cmd/bd/sync.go +++ b/cmd/bd/sync.go @@ -384,7 +384,14 @@ Use --merge to merge the sync branch back to main branch.`, os.Exit(1) } if pullResult.Pulled { - fmt.Printf("✓ Pulled from %s\n", syncBranchName) + if pullResult.Merged { + // bd-3s8 fix: divergent histories were merged at content level + fmt.Printf("✓ Merged divergent histories from %s\n", syncBranchName) + } else if pullResult.FastForwarded { + fmt.Printf("✓ Fast-forwarded from %s\n", syncBranchName) + } else { + fmt.Printf("✓ Pulled from %s\n", syncBranchName) + } } // JSONL is already copied to main repo by PullFromSyncBranch } else { diff --git a/internal/syncbranch/worktree.go b/internal/syncbranch/worktree.go index 753bbac1..c2f08c7f 100644 --- a/internal/syncbranch/worktree.go +++ b/internal/syncbranch/worktree.go @@ -6,10 +6,12 @@ import ( "os" "os/exec" "path/filepath" + "strconv" "strings" "time" "github.com/steveyegge/beads/internal/git" + "github.com/steveyegge/beads/internal/merge" ) // CommitResult contains information about a worktree commit operation @@ -22,14 +24,20 @@ type CommitResult struct { // PullResult contains information about a worktree pull operation type PullResult struct { - Pulled bool // True if pull was performed - Branch string // The sync branch name - JSONLPath string // Path to the synced JSONL in main repo + Pulled bool // True if pull was performed + Branch string // The sync branch name + JSONLPath string // Path to the synced JSONL in main repo + Merged bool // True if divergent histories were merged + FastForwarded bool // True if fast-forward was possible } // CommitToSyncBranch commits JSONL changes to the sync branch using a git worktree. // This allows committing to a different branch without changing the user's working directory. // +// IMPORTANT (bd-3s8 fix): Before committing, this function now performs a pre-emptive fetch +// and fast-forward if possible. This reduces the likelihood of divergence by ensuring we're +// building on top of the latest remote state when possible. +// // Parameters: // - ctx: Context for cancellation // - repoRoot: Path to the git repository root @@ -66,6 +74,17 @@ func CommitToSyncBranch(ctx context.Context, repoRoot, syncBranch, jsonlPath str } } + // Get remote name + remote := getRemoteForBranch(ctx, worktreePath, syncBranch) + + // Pre-emptive fetch and fast-forward (bd-3s8 fix) + // This reduces divergence by ensuring we commit on top of latest remote state + if err := preemptiveFetchAndFastForward(ctx, worktreePath, syncBranch, remote); err != nil { + // Non-fatal: if fetch fails (e.g., offline), we can still commit locally + // The divergence will be handled during the next pull + _ = err + } + // Convert absolute path to relative path from repo root jsonlRelPath, err := filepath.Rel(repoRoot, jsonlPath) if err != nil { @@ -118,9 +137,54 @@ func CommitToSyncBranch(ctx context.Context, repoRoot, syncBranch, jsonlPath str return result, nil } +// preemptiveFetchAndFastForward fetches from remote and fast-forwards if possible. +// This reduces divergence by keeping the local sync branch up-to-date before committing. +// Returns nil on success, or error if fetch/ff fails (caller should treat as non-fatal). +func preemptiveFetchAndFastForward(ctx context.Context, worktreePath, branch, remote string) error { + // Fetch from remote + fetchCmd := exec.CommandContext(ctx, "git", "-C", worktreePath, "fetch", remote, branch) + if output, err := fetchCmd.CombinedOutput(); err != nil { + // Check if remote branch doesn't exist yet (first sync) + if strings.Contains(string(output), "couldn't find remote ref") { + return nil // Not an error - remote branch doesn't exist yet + } + return fmt.Errorf("fetch failed: %w", err) + } + + // Check if we can fast-forward + localAhead, remoteAhead, err := getDivergence(ctx, worktreePath, branch, remote) + if err != nil { + return fmt.Errorf("divergence check failed: %w", err) + } + + // If remote has new commits and we have no local commits, fast-forward + if remoteAhead > 0 && localAhead == 0 { + mergeCmd := exec.CommandContext(ctx, "git", "-C", worktreePath, "merge", "--ff-only", + fmt.Sprintf("%s/%s", remote, branch)) + if output, err := mergeCmd.CombinedOutput(); err != nil { + return fmt.Errorf("fast-forward failed: %w\n%s", err, output) + } + } + + return nil +} + // PullFromSyncBranch pulls changes from the sync branch and copies JSONL to the main repo. // This fetches remote changes without affecting the user's working directory. // +// IMPORTANT (bd-3s8 fix): This function handles diverged histories gracefully by performing +// a content-based merge instead of relying on git's commit-level merge. When local and remote +// sync branches have diverged: +// 1. Fetch remote changes (don't pull) +// 2. Find the merge base +// 3. Extract JSONL from base, local, and remote +// 4. Perform 3-way content merge using bd's merge algorithm +// 5. Reset to remote's history (adopt remote commit graph) +// 6. Commit merged content on top +// +// This ensures sync never fails due to git merge conflicts, as we handle merging at the +// JSONL content level where we have semantic understanding of the data. +// // Parameters: // - ctx: Context for cancellation // - repoRoot: Path to the git repository root @@ -148,57 +212,346 @@ func PullFromSyncBranch(ctx context.Context, repoRoot, syncBranch, jsonlPath str // Get remote name remote := getRemoteForBranch(ctx, worktreePath, syncBranch) - // Pull in worktree - cmd := exec.CommandContext(ctx, "git", "-C", worktreePath, "pull", remote, syncBranch) - output, err := cmd.CombinedOutput() - if err != nil { - // Check if it's just "already up to date" or similar non-error - if strings.Contains(string(output), "Already up to date") { - result.Pulled = true - // Still copy JSONL in case worktree has changes we haven't synced - } else { - return nil, fmt.Errorf("git pull failed in worktree: %w\n%s", err, output) - } - } else { - result.Pulled = true - } - // Convert absolute path to relative path from repo root jsonlRelPath, err := filepath.Rel(repoRoot, jsonlPath) if err != nil { return nil, fmt.Errorf("failed to get relative JSONL path: %w", err) } - // Copy JSONL from worktree to main repo + // Step 1: Fetch from remote (don't pull - we handle merge ourselves) + fetchCmd := exec.CommandContext(ctx, "git", "-C", worktreePath, "fetch", remote, syncBranch) + if output, err := fetchCmd.CombinedOutput(); err != nil { + // Check if remote branch doesn't exist yet (first sync) + if strings.Contains(string(output), "couldn't find remote ref") { + // Remote branch doesn't exist - nothing to pull + result.Pulled = false + return result, nil + } + return nil, fmt.Errorf("git fetch failed in worktree: %w\n%s", err, output) + } + + // Step 2: Check for divergence + localAhead, remoteAhead, err := getDivergence(ctx, worktreePath, syncBranch, remote) + if err != nil { + return nil, fmt.Errorf("failed to check divergence: %w", err) + } + + // Case 1: Already up to date (remote has nothing new) + if remoteAhead == 0 { + result.Pulled = true + // Still copy JSONL in case worktree has uncommitted changes + if err := copyJSONLToMainRepo(worktreePath, jsonlRelPath, jsonlPath); err != nil { + return nil, err + } + return result, nil + } + + // Case 2: Can fast-forward (we have no local commits ahead of remote) + if localAhead == 0 { + // Simple fast-forward merge + mergeCmd := exec.CommandContext(ctx, "git", "-C", worktreePath, "merge", "--ff-only", + fmt.Sprintf("%s/%s", remote, syncBranch)) + if output, err := mergeCmd.CombinedOutput(); err != nil { + return nil, fmt.Errorf("git merge --ff-only failed: %w\n%s", err, output) + } + result.Pulled = true + result.FastForwarded = true + + // Copy JSONL to main repo + if err := copyJSONLToMainRepo(worktreePath, jsonlRelPath, jsonlPath); err != nil { + return nil, err + } + return result, nil + } + + // Case 3: DIVERGED - perform content-based merge (bd-3s8 fix) + // This is the key fix: instead of git merge (which can fail), we: + // 1. Extract JSONL content from base, local, and remote + // 2. Merge at content level using our 3-way merge algorithm + // 3. Reset to remote's commit history + // 4. Commit merged content on top + + mergedContent, err := performContentMerge(ctx, worktreePath, syncBranch, remote, jsonlRelPath) + if err != nil { + return nil, fmt.Errorf("content merge failed: %w", err) + } + + // Also merge deletions.jsonl if it exists + beadsRelDir := filepath.Dir(jsonlRelPath) + deletionsRelPath := filepath.Join(beadsRelDir, "deletions.jsonl") + mergedDeletions, deletionsErr := performDeletionsMerge(ctx, worktreePath, syncBranch, remote, deletionsRelPath) + // deletionsErr is non-fatal - file might not exist + + // Reset worktree to remote's history (adopt their commit graph) + resetCmd := exec.CommandContext(ctx, "git", "-C", worktreePath, "reset", "--hard", + fmt.Sprintf("%s/%s", remote, syncBranch)) + if output, err := resetCmd.CombinedOutput(); err != nil { + return nil, fmt.Errorf("git reset failed: %w\n%s", err, output) + } + + // Write merged content + worktreeJSONLPath := filepath.Join(worktreePath, jsonlRelPath) + if err := os.MkdirAll(filepath.Dir(worktreeJSONLPath), 0750); err != nil { + return nil, fmt.Errorf("failed to create directory: %w", err) + } + if err := os.WriteFile(worktreeJSONLPath, mergedContent, 0600); err != nil { + return nil, fmt.Errorf("failed to write merged JSONL: %w", err) + } + + // Write merged deletions if we have them + if deletionsErr == nil && len(mergedDeletions) > 0 { + deletionsPath := filepath.Join(worktreePath, deletionsRelPath) + if err := os.WriteFile(deletionsPath, mergedDeletions, 0600); err != nil { + // Non-fatal - deletions are supplementary + _ = err + } + } + + // Check if merge produced any changes from remote + hasChanges, err := hasChangesInWorktree(ctx, worktreePath, worktreeJSONLPath) + if err != nil { + return nil, fmt.Errorf("failed to check for changes: %w", err) + } + + // Commit merged content if there are changes + if hasChanges { + message := fmt.Sprintf("bd sync: merge divergent histories (%d local + %d remote commits)", + localAhead, remoteAhead) + if err := commitInWorktree(ctx, worktreePath, jsonlRelPath, message); err != nil { + return nil, fmt.Errorf("failed to commit merged content: %w", err) + } + } + + result.Pulled = true + result.Merged = true + + // Copy merged JSONL to main repo + if err := copyJSONLToMainRepo(worktreePath, jsonlRelPath, jsonlPath); err != nil { + return nil, err + } + + return result, nil +} + +// getDivergence returns how many commits local is ahead and behind remote. +// Returns (localAhead, remoteAhead, error) +func getDivergence(ctx context.Context, worktreePath, branch, remote string) (int, int, error) { + // Use rev-list to count commits in each direction + // --left-right --count gives us "local\tremote" + cmd := exec.CommandContext(ctx, "git", "-C", worktreePath, "rev-list", + "--left-right", "--count", + fmt.Sprintf("HEAD...%s/%s", remote, branch)) + output, err := cmd.Output() + if err != nil { + // If this fails, remote branch might not exist locally yet + // Check if it's a tracking issue + return 0, 0, fmt.Errorf("failed to get divergence: %w", err) + } + + // Parse "N\tM" format + parts := strings.Fields(strings.TrimSpace(string(output))) + if len(parts) != 2 { + return 0, 0, fmt.Errorf("unexpected rev-list output: %s", output) + } + + localAhead, err := strconv.Atoi(parts[0]) + if err != nil { + return 0, 0, fmt.Errorf("failed to parse local ahead count: %w", err) + } + + remoteAhead, err := strconv.Atoi(parts[1]) + if err != nil { + return 0, 0, fmt.Errorf("failed to parse remote ahead count: %w", err) + } + + return localAhead, remoteAhead, nil +} + +// performContentMerge extracts JSONL from base, local, and remote, then merges content. +// Returns the merged JSONL content. +func performContentMerge(ctx context.Context, worktreePath, branch, remote, jsonlRelPath string) ([]byte, error) { + // Find merge base + mergeBaseCmd := exec.CommandContext(ctx, "git", "-C", worktreePath, "merge-base", + "HEAD", fmt.Sprintf("%s/%s", remote, branch)) + mergeBaseOutput, err := mergeBaseCmd.Output() + if err != nil { + // No common ancestor - treat as empty base + mergeBaseOutput = nil + } + mergeBase := strings.TrimSpace(string(mergeBaseOutput)) + + // Create temp files for 3-way merge + tmpDir, err := os.MkdirTemp("", "bd-merge-*") + if err != nil { + return nil, fmt.Errorf("failed to create temp dir: %w", err) + } + defer os.RemoveAll(tmpDir) + + baseFile := filepath.Join(tmpDir, "base.jsonl") + localFile := filepath.Join(tmpDir, "local.jsonl") + remoteFile := filepath.Join(tmpDir, "remote.jsonl") + outputFile := filepath.Join(tmpDir, "merged.jsonl") + + // Extract base JSONL (may not exist if this is first divergence) + if mergeBase != "" { + baseContent, err := extractJSONLFromCommit(ctx, worktreePath, mergeBase, jsonlRelPath) + if err != nil { + // Base file might not exist in ancestor - use empty file + baseContent = []byte{} + } + if err := os.WriteFile(baseFile, baseContent, 0600); err != nil { + return nil, fmt.Errorf("failed to write base file: %w", err) + } + } else { + // No merge base - use empty file + if err := os.WriteFile(baseFile, []byte{}, 0600); err != nil { + return nil, fmt.Errorf("failed to write empty base file: %w", err) + } + } + + // Extract local JSONL (current HEAD in worktree) + localContent, err := extractJSONLFromCommit(ctx, worktreePath, "HEAD", jsonlRelPath) + if err != nil { + // Local file might not exist - use empty + localContent = []byte{} + } + if err := os.WriteFile(localFile, localContent, 0600); err != nil { + return nil, fmt.Errorf("failed to write local file: %w", err) + } + + // Extract remote JSONL + remoteRef := fmt.Sprintf("%s/%s", remote, branch) + remoteContent, err := extractJSONLFromCommit(ctx, worktreePath, remoteRef, jsonlRelPath) + if err != nil { + // Remote file might not exist - use empty + remoteContent = []byte{} + } + if err := os.WriteFile(remoteFile, remoteContent, 0600); err != nil { + return nil, fmt.Errorf("failed to write remote file: %w", err) + } + + // Perform 3-way merge using bd's merge algorithm + // The merge function writes to outputFile (first arg) and returns error if conflicts + err = merge.Merge3Way(outputFile, baseFile, localFile, remoteFile, false) + if err != nil { + // Check if it's a conflict error + if strings.Contains(err.Error(), "merge completed with") { + // There were conflicts - this is rare for JSONL since most fields can be + // auto-merged. When it happens, it means both sides changed the same field + // to different values. We fail here rather than writing corrupt JSONL. + return nil, fmt.Errorf("merge conflict: %w (manual resolution required)", err) + } + return nil, fmt.Errorf("3-way merge failed: %w", err) + } + + // Read merged result + mergedContent, err := os.ReadFile(outputFile) + if err != nil { + return nil, fmt.Errorf("failed to read merged file: %w", err) + } + + return mergedContent, nil +} + +// extractJSONLFromCommit extracts a file's content from a specific git commit. +func extractJSONLFromCommit(ctx context.Context, worktreePath, commit, filePath string) ([]byte, error) { + cmd := exec.CommandContext(ctx, "git", "-C", worktreePath, "show", + fmt.Sprintf("%s:%s", commit, filePath)) + output, err := cmd.Output() + if err != nil { + return nil, fmt.Errorf("failed to extract %s from %s: %w", filePath, commit, err) + } + return output, nil +} + +// performDeletionsMerge merges deletions.jsonl from local and remote. +// Deletions are merged by union - we keep all deletion records from both sides. +// This ensures that if either side deleted an issue, it stays deleted. +func performDeletionsMerge(ctx context.Context, worktreePath, branch, remote, deletionsRelPath string) ([]byte, error) { + // Extract local deletions + localDeletions, localErr := extractJSONLFromCommit(ctx, worktreePath, "HEAD", deletionsRelPath) + + // Extract remote deletions + remoteRef := fmt.Sprintf("%s/%s", remote, branch) + remoteDeletions, remoteErr := extractJSONLFromCommit(ctx, worktreePath, remoteRef, deletionsRelPath) + + // If neither exists, nothing to merge + if localErr != nil && remoteErr != nil { + return nil, fmt.Errorf("no deletions files to merge") + } + + // If only one exists, use that + if localErr != nil { + return remoteDeletions, nil + } + if remoteErr != nil { + return localDeletions, nil + } + + // Both exist - merge by taking union of lines (deduplicated) + // Each line in deletions.jsonl is a JSON object with an "id" field + seen := make(map[string]bool) + var merged []byte + + // Process local deletions + for _, line := range strings.Split(string(localDeletions), "\n") { + line = strings.TrimSpace(line) + if line == "" { + continue + } + if !seen[line] { + seen[line] = true + merged = append(merged, []byte(line+"\n")...) + } + } + + // Process remote deletions + for _, line := range strings.Split(string(remoteDeletions), "\n") { + line = strings.TrimSpace(line) + if line == "" { + continue + } + if !seen[line] { + seen[line] = true + merged = append(merged, []byte(line+"\n")...) + } + } + + return merged, nil +} + +// copyJSONLToMainRepo copies JSONL and related files from worktree to main repo. +func copyJSONLToMainRepo(worktreePath, jsonlRelPath, jsonlPath string) error { worktreeJSONLPath := filepath.Join(worktreePath, jsonlRelPath) // Check if worktree JSONL exists if _, err := os.Stat(worktreeJSONLPath); os.IsNotExist(err) { // No JSONL in worktree yet, nothing to sync - return result, nil + return nil } // Copy JSONL from worktree to main repo data, err := os.ReadFile(worktreeJSONLPath) if err != nil { - return nil, fmt.Errorf("failed to read worktree JSONL: %w", err) + return fmt.Errorf("failed to read worktree JSONL: %w", err) } - if err := os.WriteFile(jsonlPath, data, 0644); err != nil { - return nil, fmt.Errorf("failed to write main JSONL: %w", err) + if err := os.WriteFile(jsonlPath, data, 0600); err != nil { + return fmt.Errorf("failed to write main JSONL: %w", err) } // Also sync other beads files back (deletions.jsonl, metadata.json) beadsDir := filepath.Dir(jsonlPath) + worktreeBeadsDir := filepath.Dir(worktreeJSONLPath) for _, filename := range []string{"deletions.jsonl", "metadata.json"} { - worktreeSrcPath := filepath.Join(worktreePath, ".beads", filename) - if data, err := os.ReadFile(worktreeSrcPath); err == nil { + worktreeSrcPath := filepath.Join(worktreeBeadsDir, filename) + if fileData, err := os.ReadFile(worktreeSrcPath); err == nil { dstPath := filepath.Join(beadsDir, filename) - _ = os.WriteFile(dstPath, data, 0644) // Best effort + _ = os.WriteFile(dstPath, fileData, 0600) // Best effort, match JSONL permissions } } - return result, nil + return nil } // hasChangesInWorktree checks if there are uncommitted changes in the worktree diff --git a/internal/syncbranch/worktree_divergence_test.go b/internal/syncbranch/worktree_divergence_test.go new file mode 100644 index 00000000..d200deaa --- /dev/null +++ b/internal/syncbranch/worktree_divergence_test.go @@ -0,0 +1,460 @@ +package syncbranch + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// TestGetDivergence tests the divergence detection function +func TestGetDivergence(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + ctx := context.Background() + + t.Run("no divergence when synced", func(t *testing.T) { + // Create a test repo with a branch + repoDir := setupTestRepo(t) + defer os.RemoveAll(repoDir) + + // Create and checkout test branch + runGit(t, repoDir, "checkout", "-b", "test-branch") + writeFile(t, filepath.Join(repoDir, ".beads", "issues.jsonl"), `{"id":"test-1"}`) + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "initial commit") + + // Simulate remote by creating a local ref + runGit(t, repoDir, "update-ref", "refs/remotes/origin/test-branch", "HEAD") + + localAhead, remoteAhead, err := getDivergence(ctx, repoDir, "test-branch", "origin") + if err != nil { + t.Fatalf("getDivergence() error = %v", err) + } + if localAhead != 0 || remoteAhead != 0 { + t.Errorf("getDivergence() = (%d, %d), want (0, 0)", localAhead, remoteAhead) + } + }) + + t.Run("local ahead of remote", func(t *testing.T) { + repoDir := setupTestRepo(t) + defer os.RemoveAll(repoDir) + + runGit(t, repoDir, "checkout", "-b", "test-branch") + writeFile(t, filepath.Join(repoDir, ".beads", "issues.jsonl"), `{"id":"test-1"}`) + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "initial commit") + + // Set remote ref to current HEAD + runGit(t, repoDir, "update-ref", "refs/remotes/origin/test-branch", "HEAD") + + // Add more local commits + writeFile(t, filepath.Join(repoDir, ".beads", "issues.jsonl"), `{"id":"test-1"} +{"id":"test-2"}`) + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "second commit") + + localAhead, remoteAhead, err := getDivergence(ctx, repoDir, "test-branch", "origin") + if err != nil { + t.Fatalf("getDivergence() error = %v", err) + } + if localAhead != 1 || remoteAhead != 0 { + t.Errorf("getDivergence() = (%d, %d), want (1, 0)", localAhead, remoteAhead) + } + }) + + t.Run("remote ahead of local", func(t *testing.T) { + repoDir := setupTestRepo(t) + defer os.RemoveAll(repoDir) + + runGit(t, repoDir, "checkout", "-b", "test-branch") + writeFile(t, filepath.Join(repoDir, ".beads", "issues.jsonl"), `{"id":"test-1"}`) + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "initial commit") + + // Save current HEAD as "local" + localHead := strings.TrimSpace(getGitOutput(t, repoDir, "rev-parse", "HEAD")) + + // Create more commits and set as remote + writeFile(t, filepath.Join(repoDir, ".beads", "issues.jsonl"), `{"id":"test-1"} +{"id":"test-2"}`) + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "remote commit") + runGit(t, repoDir, "update-ref", "refs/remotes/origin/test-branch", "HEAD") + + // Reset local to previous commit + runGit(t, repoDir, "reset", "--hard", localHead) + + localAhead, remoteAhead, err := getDivergence(ctx, repoDir, "test-branch", "origin") + if err != nil { + t.Fatalf("getDivergence() error = %v", err) + } + if localAhead != 0 || remoteAhead != 1 { + t.Errorf("getDivergence() = (%d, %d), want (0, 1)", localAhead, remoteAhead) + } + }) + + t.Run("diverged histories", func(t *testing.T) { + repoDir := setupTestRepo(t) + defer os.RemoveAll(repoDir) + + runGit(t, repoDir, "checkout", "-b", "test-branch") + writeFile(t, filepath.Join(repoDir, ".beads", "issues.jsonl"), `{"id":"test-1"}`) + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "base commit") + + // Save base commit + baseCommit := strings.TrimSpace(getGitOutput(t, repoDir, "rev-parse", "HEAD")) + + // Create local commit + writeFile(t, filepath.Join(repoDir, ".beads", "issues.jsonl"), `{"id":"test-1"} +{"id":"local-2"}`) + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "local commit") + localHead := strings.TrimSpace(getGitOutput(t, repoDir, "rev-parse", "HEAD")) + + // Create remote commit from base + runGit(t, repoDir, "checkout", baseCommit) + writeFile(t, filepath.Join(repoDir, ".beads", "issues.jsonl"), `{"id":"test-1"} +{"id":"remote-2"}`) + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "remote commit") + runGit(t, repoDir, "update-ref", "refs/remotes/origin/test-branch", "HEAD") + + // Go back to local branch + runGit(t, repoDir, "checkout", "-B", "test-branch", localHead) + + localAhead, remoteAhead, err := getDivergence(ctx, repoDir, "test-branch", "origin") + if err != nil { + t.Fatalf("getDivergence() error = %v", err) + } + if localAhead != 1 || remoteAhead != 1 { + t.Errorf("getDivergence() = (%d, %d), want (1, 1)", localAhead, remoteAhead) + } + }) +} + +// TestExtractJSONLFromCommit tests extracting JSONL content from git commits +func TestExtractJSONLFromCommit(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + ctx := context.Background() + + t.Run("extracts file from HEAD", func(t *testing.T) { + repoDir := setupTestRepo(t) + defer os.RemoveAll(repoDir) + + content := `{"id":"test-1","title":"Test Issue"}` + writeFile(t, filepath.Join(repoDir, ".beads", "issues.jsonl"), content) + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "test commit") + + extracted, err := extractJSONLFromCommit(ctx, repoDir, "HEAD", ".beads/issues.jsonl") + if err != nil { + t.Fatalf("extractJSONLFromCommit() error = %v", err) + } + if strings.TrimSpace(string(extracted)) != content { + t.Errorf("extractJSONLFromCommit() = %q, want %q", extracted, content) + } + }) + + t.Run("extracts file from specific commit", func(t *testing.T) { + repoDir := setupTestRepo(t) + defer os.RemoveAll(repoDir) + + // First commit + content1 := `{"id":"test-1"}` + writeFile(t, filepath.Join(repoDir, ".beads", "issues.jsonl"), content1) + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "first commit") + firstCommit := strings.TrimSpace(getGitOutput(t, repoDir, "rev-parse", "HEAD")) + + // Second commit + content2 := `{"id":"test-1"} +{"id":"test-2"}` + writeFile(t, filepath.Join(repoDir, ".beads", "issues.jsonl"), content2) + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "second commit") + + // Extract from first commit + extracted, err := extractJSONLFromCommit(ctx, repoDir, firstCommit, ".beads/issues.jsonl") + if err != nil { + t.Fatalf("extractJSONLFromCommit() error = %v", err) + } + if strings.TrimSpace(string(extracted)) != content1 { + t.Errorf("extractJSONLFromCommit() = %q, want %q", extracted, content1) + } + }) + + t.Run("returns error for nonexistent file", func(t *testing.T) { + repoDir := setupTestRepo(t) + defer os.RemoveAll(repoDir) + + writeFile(t, filepath.Join(repoDir, "dummy.txt"), "dummy") + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "test commit") + + _, err := extractJSONLFromCommit(ctx, repoDir, "HEAD", ".beads/issues.jsonl") + if err == nil { + t.Error("extractJSONLFromCommit() expected error for nonexistent file") + } + }) +} + +// TestPerformContentMerge tests the content-based merge function +func TestPerformContentMerge(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + ctx := context.Background() + + t.Run("merges diverged content", func(t *testing.T) { + repoDir := setupTestRepo(t) + defer os.RemoveAll(repoDir) + + runGit(t, repoDir, "checkout", "-b", "test-branch") + + // Base content + baseContent := `{"id":"test-1","title":"Base","created_at":"2024-01-01T00:00:00Z","created_by":"user1"}` + writeFile(t, filepath.Join(repoDir, ".beads", "issues.jsonl"), baseContent) + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "base commit") + baseCommit := strings.TrimSpace(getGitOutput(t, repoDir, "rev-parse", "HEAD")) + + // Create local changes (add issue) + localContent := `{"id":"test-1","title":"Base","created_at":"2024-01-01T00:00:00Z","created_by":"user1"} +{"id":"local-1","title":"Local Issue","created_at":"2024-01-02T00:00:00Z","created_by":"user1"}` + writeFile(t, filepath.Join(repoDir, ".beads", "issues.jsonl"), localContent) + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "local commit") + localHead := strings.TrimSpace(getGitOutput(t, repoDir, "rev-parse", "HEAD")) + + // Create remote changes from base (add different issue) + runGit(t, repoDir, "checkout", baseCommit) + remoteContent := `{"id":"test-1","title":"Base","created_at":"2024-01-01T00:00:00Z","created_by":"user1"} +{"id":"remote-1","title":"Remote Issue","created_at":"2024-01-02T00:00:00Z","created_by":"user2"}` + writeFile(t, filepath.Join(repoDir, ".beads", "issues.jsonl"), remoteContent) + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "remote commit") + runGit(t, repoDir, "update-ref", "refs/remotes/origin/test-branch", "HEAD") + + // Go back to local + runGit(t, repoDir, "checkout", "-B", "test-branch", localHead) + + // Perform merge + merged, err := performContentMerge(ctx, repoDir, "test-branch", "origin", ".beads/issues.jsonl") + if err != nil { + t.Fatalf("performContentMerge() error = %v", err) + } + + // Check that merged content contains all three issues + mergedStr := string(merged) + if !strings.Contains(mergedStr, "test-1") { + t.Error("merged content missing base issue test-1") + } + if !strings.Contains(mergedStr, "local-1") { + t.Error("merged content missing local issue local-1") + } + if !strings.Contains(mergedStr, "remote-1") { + t.Error("merged content missing remote issue remote-1") + } + }) + + t.Run("handles deletion correctly", func(t *testing.T) { + repoDir := setupTestRepo(t) + defer os.RemoveAll(repoDir) + + runGit(t, repoDir, "checkout", "-b", "test-branch") + + // Base content with two issues + baseContent := `{"id":"test-1","title":"Issue 1","created_at":"2024-01-01T00:00:00Z","created_by":"user1"} +{"id":"test-2","title":"Issue 2","created_at":"2024-01-01T00:00:00Z","created_by":"user1"}` + writeFile(t, filepath.Join(repoDir, ".beads", "issues.jsonl"), baseContent) + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "base commit") + baseCommit := strings.TrimSpace(getGitOutput(t, repoDir, "rev-parse", "HEAD")) + + // Local keeps both + localHead := baseCommit + + // Remote deletes test-2 + runGit(t, repoDir, "checkout", baseCommit) + remoteContent := `{"id":"test-1","title":"Issue 1","created_at":"2024-01-01T00:00:00Z","created_by":"user1"}` + writeFile(t, filepath.Join(repoDir, ".beads", "issues.jsonl"), remoteContent) + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "remote delete") + runGit(t, repoDir, "update-ref", "refs/remotes/origin/test-branch", "HEAD") + + // Go back to local + runGit(t, repoDir, "checkout", "-B", "test-branch", localHead) + + // Perform merge + merged, err := performContentMerge(ctx, repoDir, "test-branch", "origin", ".beads/issues.jsonl") + if err != nil { + t.Fatalf("performContentMerge() error = %v", err) + } + + // Deletion should win - test-2 should be gone + mergedStr := string(merged) + if !strings.Contains(mergedStr, "test-1") { + t.Error("merged content missing issue test-1") + } + if strings.Contains(mergedStr, "test-2") { + t.Error("merged content still contains deleted issue test-2") + } + }) +} + +// TestPerformDeletionsMerge tests the deletions merge function +func TestPerformDeletionsMerge(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + ctx := context.Background() + + t.Run("merges deletions from both sides", func(t *testing.T) { + repoDir := setupTestRepo(t) + defer os.RemoveAll(repoDir) + + runGit(t, repoDir, "checkout", "-b", "test-branch") + + // Base: no deletions + writeFile(t, filepath.Join(repoDir, ".beads", "issues.jsonl"), `{"id":"test-1"}`) + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "base commit") + baseCommit := strings.TrimSpace(getGitOutput(t, repoDir, "rev-parse", "HEAD")) + + // Local: delete issue-A + writeFile(t, filepath.Join(repoDir, ".beads", "deletions.jsonl"), `{"id":"issue-A","deleted_at":"2024-01-01T00:00:00Z"}`) + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "local deletion") + localHead := strings.TrimSpace(getGitOutput(t, repoDir, "rev-parse", "HEAD")) + + // Remote: delete issue-B + runGit(t, repoDir, "checkout", baseCommit) + writeFile(t, filepath.Join(repoDir, ".beads", "deletions.jsonl"), `{"id":"issue-B","deleted_at":"2024-01-02T00:00:00Z"}`) + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "remote deletion") + runGit(t, repoDir, "update-ref", "refs/remotes/origin/test-branch", "HEAD") + + // Go back to local + runGit(t, repoDir, "checkout", "-B", "test-branch", localHead) + + // Perform merge + merged, err := performDeletionsMerge(ctx, repoDir, "test-branch", "origin", ".beads/deletions.jsonl") + if err != nil { + t.Fatalf("performDeletionsMerge() error = %v", err) + } + + // Both deletions should be present + mergedStr := string(merged) + if !strings.Contains(mergedStr, "issue-A") { + t.Error("merged deletions missing issue-A") + } + if !strings.Contains(mergedStr, "issue-B") { + t.Error("merged deletions missing issue-B") + } + }) + + t.Run("handles only local deletions", func(t *testing.T) { + repoDir := setupTestRepo(t) + defer os.RemoveAll(repoDir) + + runGit(t, repoDir, "checkout", "-b", "test-branch") + + // Base: no deletions + writeFile(t, filepath.Join(repoDir, ".beads", "issues.jsonl"), `{"id":"test-1"}`) + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "base commit") + baseCommit := strings.TrimSpace(getGitOutput(t, repoDir, "rev-parse", "HEAD")) + + // Local: has deletions + writeFile(t, filepath.Join(repoDir, ".beads", "deletions.jsonl"), `{"id":"issue-A"}`) + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "local deletion") + localHead := strings.TrimSpace(getGitOutput(t, repoDir, "rev-parse", "HEAD")) + + // Remote: no deletions file + runGit(t, repoDir, "checkout", baseCommit) + runGit(t, repoDir, "update-ref", "refs/remotes/origin/test-branch", "HEAD") + + // Go back to local + runGit(t, repoDir, "checkout", "-B", "test-branch", localHead) + + // Perform merge + merged, err := performDeletionsMerge(ctx, repoDir, "test-branch", "origin", ".beads/deletions.jsonl") + if err != nil { + t.Fatalf("performDeletionsMerge() error = %v", err) + } + + // Local deletions should be present + if !strings.Contains(string(merged), "issue-A") { + t.Error("merged deletions missing issue-A") + } + }) +} + +// Helper functions + +func setupTestRepo(t *testing.T) string { + t.Helper() + + tmpDir, err := os.MkdirTemp("", "bd-test-repo-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + + // Initialize git repo + runGit(t, tmpDir, "init") + runGit(t, tmpDir, "config", "user.email", "test@test.com") + runGit(t, tmpDir, "config", "user.name", "Test User") + + // Create .beads directory + beadsDir := filepath.Join(tmpDir, ".beads") + if err := os.MkdirAll(beadsDir, 0750); err != nil { + os.RemoveAll(tmpDir) + t.Fatalf("Failed to create .beads dir: %v", err) + } + + return tmpDir +} + +func runGit(t *testing.T, dir string, args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v failed: %v\n%s", args, err, output) + } +} + +func getGitOutput(t *testing.T, dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + output, err := cmd.Output() + if err != nil { + t.Fatalf("git %v failed: %v", args, err) + } + return string(output) +} + +func writeFile(t *testing.T, path, content string) { + t.Helper() + dir := filepath.Dir(path) + if err := os.MkdirAll(dir, 0750); err != nil { + t.Fatalf("Failed to create directory %s: %v", dir, err) + } + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatalf("Failed to write file %s: %v", path, err) + } +}