feat: detect polluted branch history and guide manual cleanup

cmd/sync.go

@@ -98,22 +98,37 @@ func runSync(gitClient git.GitClient, githubClient github.GitHubClient) error {
 	hasSavedState := savedStashed == "true" || savedOriginalBranch != ""
 
 	if syncAbort {
-		// Aborting an interrupted sync
-		if !hasSavedState {
+		// Check if there's actually anything to abort
+		hasCherryPick := gitClient.IsCherryPickInProgress()
+		hasRebase := gitClient.IsRebaseInProgress()
+
+		if !hasSavedState && !hasCherryPick && !hasRebase {
 			return fmt.Errorf("no interrupted sync to abort\n\nUse 'stack sync' to start a new sync")
 		}
 
 		fmt.Println("Aborting sync and cleaning up...")
 		fmt.Println()
 
-		// Try to abort rebase if one is in progress
-		// It's okay if this fails - there might not be an active rebase
-		if err := gitClient.AbortRebase(); err != nil {
-			if git.Verbose {
-				fmt.Fprintf(os.Stderr, "Note: no rebase to abort (already resolved or not started)\n")
+		// Abort cherry-pick if one is in progress
+		if hasCherryPick {
+			if err := gitClient.AbortCherryPick(); err != nil {
+				fmt.Fprintf(os.Stderr, "Warning: failed to abort cherry-pick: %v\n", err)
+			} else {
+				fmt.Println("✓ Aborted cherry-pick")
 			}
-		} else {
-			fmt.Println("✓ Aborted rebase")
+		} else if git.Verbose {
+			fmt.Fprintf(os.Stderr, "Note: no cherry-pick in progress\n")
+		}
+
+		// Abort rebase if one is in progress
+		if hasRebase {
+			if err := gitClient.AbortRebase(); err != nil {
+				fmt.Fprintf(os.Stderr, "Warning: failed to abort rebase: %v\n", err)
+			} else {
+				fmt.Println("✓ Aborted rebase")
+			}
+		} else if git.Verbose {
+			fmt.Fprintf(os.Stderr, "Note: no rebase in progress\n")
 		}
 
 		// Restore stashed changes if any
@@ -509,7 +524,84 @@ func runSync(gitClient git.GitClient, githubClient github.GitHubClient) error {
 					fmt.Printf("  Using --onto to handle squash merge (excluding commits from %s)\n", oldParent)
 					return gitClient.RebaseOnto(rebaseTarget, oldParent, branch.Name)
 				}
-				return gitClient.Rebase(rebaseTarget)
+
+				// Get unique commits in this branch by comparing patch content (not just SHAs)
+				// This detects duplicate changes even if commits were rebased with different SHAs
+				uniqueCommits, err := gitClient.GetUniqueCommitsByPatch(rebaseTarget, branch.Name)
+				if err != nil {
+					// If we can't get unique commits, fall back to regular rebase
+					if git.Verbose {
+						fmt.Printf("  Could not get unique commits by patch, using regular rebase: %v\n", err)
+					}
+					return gitClient.Rebase(rebaseTarget)
+				}
+
+				// If no unique commits, branch is up-to-date
+				if len(uniqueCommits) == 0 {
+					if git.Verbose {
+						fmt.Printf("  Branch is up-to-date with %s (no unique patches)\n", rebaseTarget)
+					}
+					return nil
+				}
+
+				if git.Verbose {
+					fmt.Printf("  Found %d unique commit(s) by patch comparison\n", len(uniqueCommits))
+				}
+
+				// Get merge-base to understand the history
+				mergeBase, err := gitClient.GetMergeBase(branch.Name, rebaseTarget)
+				if err != nil {
+					// If we can't find merge-base, fall back to regular rebase
+					if git.Verbose {
+						fmt.Printf("  Could not find merge-base, using regular rebase: %v\n", err)
+					}
+					return gitClient.Rebase(rebaseTarget)
+				}
+
+				rebaseTargetHash, err := gitClient.GetCommitHash(rebaseTarget)
+				if err == nil && mergeBase == rebaseTargetHash {
+					// Parent hasn't changed since we branched, regular rebase is fine
+					return gitClient.Rebase(rebaseTarget)
+				}
+
+				// Count commits from merge-base to current branch (total commits in branch history)
+				allCommits, err := gitClient.GetUniqueCommits(mergeBase, branch.Name)
+				if err == nil && len(allCommits) > len(uniqueCommits)*2 {
+					// Branch has polluted history: many more commits than unique patches
+					// This usually means branch diverged from parent's history (e.g., based on old backup)
+					rebaseConflict = true
+					fmt.Fprintf(os.Stderr, "\n")
+					fmt.Fprintf(os.Stderr, "⚠ Detected polluted branch history:\n")
+					fmt.Fprintf(os.Stderr, "  - %d commits in branch history\n", len(allCommits))
+					fmt.Fprintf(os.Stderr, "  - Only %d unique patch(es)\n", len(uniqueCommits))
+					fmt.Fprintf(os.Stderr, "\n")
+					fmt.Fprintf(os.Stderr, "This usually means your branch diverged from the parent's history.\n")
+					fmt.Fprintf(os.Stderr, "Rebasing may result in many conflicts.\n")
+					fmt.Fprintf(os.Stderr, "\n")
+					fmt.Fprintf(os.Stderr, "Recommended: Rebuild branch manually with cherry-pick:\n")
+					fmt.Fprintf(os.Stderr, "  1. git checkout %s\n", branch.Parent)
+					fmt.Fprintf(os.Stderr, "  2. git checkout -b %s-clean\n", branch.Name)
+					for i, commit := range uniqueCommits {
+						if i < 5 { // Show first 5 commits
+							fmt.Fprintf(os.Stderr, "  3. git cherry-pick %s\n", commit[:8])
+						}
+					}
+					if len(uniqueCommits) > 5 {
+						fmt.Fprintf(os.Stderr, "     ... (%d more commits)\n", len(uniqueCommits)-5)
+					}
+					fmt.Fprintf(os.Stderr, "  4. git branch -D %s\n", branch.Name)
+					fmt.Fprintf(os.Stderr, "  5. git branch -m %s\n", branch.Name)
+					fmt.Fprintf(os.Stderr, "  6. git push --force-with-lease\n")
+					fmt.Fprintf(os.Stderr, "\n")
+					return fmt.Errorf("branch history is polluted, manual cleanup recommended")
+				}
+
+				// Use --onto to only replay commits unique to this branch
+				// This prevents conflicts from duplicate commits when parent was rebased
+				if git.Verbose {
+					fmt.Printf("  Using --onto with merge-base %s to handle rebased parent\n", mergeBase[:8])
+				}
+				return gitClient.RebaseOnto(rebaseTarget, mergeBase, branch.Name)
 			},
 		); err != nil {
 			rebaseConflict = true

cmd/sync_test.go

@@ -55,13 +55,23 @@ func TestRunSyncBasic(t *testing.T) {
 		mockGit.On("CheckoutBranch", "feature-a").Return(nil)
 		mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil)
 		mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil)
+		// Patch-based unique commit detection
+		mockGit.On("GetUniqueCommitsByPatch", "origin/main", "feature-a").Return([]string{"abc123"}, nil)
+		mockGit.On("GetMergeBase", "feature-a", "origin/main").Return("main123", nil)
+		mockGit.On("GetCommitHash", "origin/main").Return("main123", nil)
+		// Falls through to regular rebase since merge-base == parent
 		mockGit.On("Rebase", "origin/main").Return(nil)
 		mockGit.On("FetchBranch", "feature-a").Return(nil)
 		mockGit.On("PushWithExpectedRemote", "feature-a", "abc123").Return(nil)
 		// Process feature-b
 		mockGit.On("CheckoutBranch", "feature-b").Return(nil)
 		mockGit.On("GetCommitHash", "feature-b").Return("def456", nil)
 		mockGit.On("GetCommitHash", "origin/feature-b").Return("def456", nil)
+		// Patch-based unique commit detection
+		mockGit.On("GetUniqueCommitsByPatch", "feature-a", "feature-b").Return([]string{"def456"}, nil)
+		mockGit.On("GetMergeBase", "feature-b", "feature-a").Return("abc123", nil)
+		mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil)
+		// Falls through to regular rebase since merge-base == parent
 		mockGit.On("Rebase", "feature-a").Return(nil)
 		mockGit.On("FetchBranch", "feature-b").Return(nil)
 		mockGit.On("PushWithExpectedRemote", "feature-b", "def456").Return(nil)
@@ -197,6 +207,9 @@ func TestRunSyncUpdatePRBase(t *testing.T) {
 		mockGit.On("CheckoutBranch", "feature-a").Return(nil)
 		mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil)
 		mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil)
+		mockGit.On("GetUniqueCommitsByPatch", "origin/main", "feature-a").Return([]string{"abc123"}, nil)
+		mockGit.On("GetMergeBase", "feature-a", "origin/main").Return("main123", nil)
+		mockGit.On("GetCommitHash", "origin/main").Return("main123", nil)
 		mockGit.On("Rebase", "origin/main").Return(nil)
 		mockGit.On("FetchBranch", "feature-a").Return(nil)
 		mockGit.On("PushWithExpectedRemote", "feature-a", "abc123").Return(nil)
@@ -205,6 +218,9 @@ func TestRunSyncUpdatePRBase(t *testing.T) {
 		mockGit.On("CheckoutBranch", "feature-b").Return(nil)
 		mockGit.On("GetCommitHash", "feature-b").Return("def456", nil)
 		mockGit.On("GetCommitHash", "origin/feature-b").Return("def456", nil)
+		mockGit.On("GetUniqueCommitsByPatch", "feature-a", "feature-b").Return([]string{"def456"}, nil)
+		mockGit.On("GetMergeBase", "feature-b", "feature-a").Return("abc123", nil)
+		mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil)
 		mockGit.On("Rebase", "feature-a").Return(nil)
 		mockGit.On("FetchBranch", "feature-b").Return(nil)
 		mockGit.On("PushWithExpectedRemote", "feature-b", "def456").Return(nil)
@@ -271,6 +287,9 @@ func TestRunSyncStashHandling(t *testing.T) {
 		mockGit.On("CheckoutBranch", "feature-a").Return(nil)
 		mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil)
 		mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil)
+		mockGit.On("GetUniqueCommitsByPatch", "origin/main", "feature-a").Return([]string{"abc123"}, nil)
+		mockGit.On("GetMergeBase", "feature-a", "origin/main").Return("main123", nil)
+		mockGit.On("GetCommitHash", "origin/main").Return("main123", nil)
 		mockGit.On("Rebase", "origin/main").Return(nil)
 		mockGit.On("FetchBranch", "feature-a").Return(nil)
 		mockGit.On("PushWithExpectedRemote", "feature-a", "abc123").Return(nil)
@@ -328,6 +347,9 @@ func TestRunSyncErrorHandling(t *testing.T) {
 		mockGit.On("CheckoutBranch", "feature-a").Return(nil)
 		mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil)
 		mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil)
+		mockGit.On("GetUniqueCommitsByPatch", "origin/main", "feature-a").Return([]string{"abc123"}, nil)
+		mockGit.On("GetMergeBase", "feature-a", "origin/main").Return("main123", nil)
+		mockGit.On("GetCommitHash", "origin/main").Return("main123", nil)
 		// Rebase fails
 		mockGit.On("Rebase", "origin/main").Return(fmt.Errorf("rebase conflict"))
 		// Note: StashPop is NOT called because rebaseConflict=true
@@ -378,6 +400,9 @@ func TestRunSyncErrorHandling(t *testing.T) {
 		mockGit.On("CheckoutBranch", "feature-a").Return(nil)
 		mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil)
 		mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil)
+		mockGit.On("GetUniqueCommitsByPatch", "origin/main", "feature-a").Return([]string{"abc123"}, nil)
+		mockGit.On("GetMergeBase", "feature-a", "origin/main").Return("main123", nil)
+		mockGit.On("GetCommitHash", "origin/main").Return("main123", nil)
 		// Rebase fails - stash should NOT be popped (preserved for --resume)
 		mockGit.On("Rebase", "origin/main").Return(fmt.Errorf("rebase conflict"))
 		// Note: StashPop is NOT called because rebaseConflict=true
@@ -394,7 +419,7 @@ func TestFilterMergedBranchesForSync(t *testing.T) {
 	// Test the filterMergedBranchesForSync function
 	// This is a simple unit test for the tree filtering logic
 	prCache := map[string]*github.PRInfo{
-		"merged-leaf": testutil.NewPRInfo(1, "MERGED", "main", "Merged Leaf", "url"),
+		"merged-leaf":   testutil.NewPRInfo(1, "MERGED", "main", "Merged Leaf", "url"),
 		"merged-parent": testutil.NewPRInfo(2, "MERGED", "main", "Merged Parent", "url"),
 	}
 
@@ -520,6 +545,9 @@ func TestRunSyncResume(t *testing.T) {
 		mockGit.On("CheckoutBranch", "feature-a").Return(nil)
 		mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil)
 		mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil)
+		mockGit.On("GetUniqueCommitsByPatch", "origin/main", "feature-a").Return([]string{"abc123"}, nil)
+		mockGit.On("GetMergeBase", "feature-a", "origin/main").Return("main123", nil)
+		mockGit.On("GetCommitHash", "origin/main").Return("main123", nil)
 		mockGit.On("Rebase", "origin/main").Return(nil)
 		mockGit.On("FetchBranch", "feature-a").Return(nil)
 		mockGit.On("PushWithExpectedRemote", "feature-a", "abc123").Return(nil)
@@ -579,6 +607,9 @@ func TestRunSyncResume(t *testing.T) {
 		mockGit.On("CheckoutBranch", "feature-a").Return(nil)
 		mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil)
 		mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil)
+		mockGit.On("GetUniqueCommitsByPatch", "origin/main", "feature-a").Return([]string{"abc123"}, nil)
+		mockGit.On("GetMergeBase", "feature-a", "origin/main").Return("main123", nil)
+		mockGit.On("GetCommitHash", "origin/main").Return("main123", nil)
 		mockGit.On("Rebase", "origin/main").Return(nil)
 		mockGit.On("FetchBranch", "feature-a").Return(nil)
 		mockGit.On("PushWithExpectedRemote", "feature-a", "abc123").Return(nil)
@@ -609,6 +640,10 @@ func TestRunSyncAbort(t *testing.T) {
 		mockGit.On("GetConfig", "stack.sync.stashed").Return("")
 		mockGit.On("GetConfig", "stack.sync.originalBranch").Return("")
 
+		// No rebase or cherry-pick in progress
+		mockGit.On("IsCherryPickInProgress").Return(false)
+		mockGit.On("IsRebaseInProgress").Return(false)
+
 		// Set abort flag
 		syncAbort = true
 		defer func() { syncAbort = false }()
@@ -627,6 +662,10 @@ func TestRunSyncAbort(t *testing.T) {
 		mockGit.On("GetConfig", "stack.sync.stashed").Return("true")
 		mockGit.On("GetConfig", "stack.sync.originalBranch").Return("feature-a")
 
+		// Rebase in progress (to trigger AbortRebase)
+		mockGit.On("IsCherryPickInProgress").Return(false)
+		mockGit.On("IsRebaseInProgress").Return(true)
+
 		// Set abort flag
 		syncAbort = true
 		defer func() { syncAbort = false }()
@@ -657,6 +696,10 @@ func TestRunSyncAbort(t *testing.T) {
 		mockGit.On("GetConfig", "stack.sync.stashed").Return("")
 		mockGit.On("GetConfig", "stack.sync.originalBranch").Return("feature-a")
 
+		// Rebase in progress (to trigger AbortRebase)
+		mockGit.On("IsCherryPickInProgress").Return(false)
+		mockGit.On("IsRebaseInProgress").Return(true)
+
 		// Set abort flag
 		syncAbort = true
 		defer func() { syncAbort = false }()
@@ -686,11 +729,15 @@ func TestRunSyncAbort(t *testing.T) {
 		mockGit.On("GetConfig", "stack.sync.stashed").Return("")
 		mockGit.On("GetConfig", "stack.sync.originalBranch").Return("feature-a")
 
+		// Rebase in progress (to trigger AbortRebase)
+		mockGit.On("IsCherryPickInProgress").Return(false)
+		mockGit.On("IsRebaseInProgress").Return(true)
+
 		// Set abort flag
 		syncAbort = true
 		defer func() { syncAbort = false }()
 
-		// Abort rebase fails (no rebase in progress)
+		// Abort rebase fails (simulated failure)
 		mockGit.On("AbortRebase").Return(fmt.Errorf("no rebase in progress"))
 		// Return to original branch
 		mockGit.On("GetCurrentBranch").Return("feature-a", nil)

internal/git/git.go

@@ -300,6 +300,20 @@ func (c *gitClient) GetRemoteBranchesSet() map[string]bool {
 	return branches
 }
 
+// IsRebaseInProgress checks if a rebase is currently in progress
+func (c *gitClient) IsRebaseInProgress() bool {
+	// Git creates REBASE_HEAD during rebase (both regular and interactive)
+	_, err := c.runCmd("rev-parse", "--verify", "REBASE_HEAD")
+	return err == nil
+}
+
+// IsCherryPickInProgress checks if a cherry-pick is currently in progress
+func (c *gitClient) IsCherryPickInProgress() bool {
+	// Git creates .git/CHERRY_PICK_HEAD during cherry-pick
+	_, err := c.runCmd("rev-parse", "--verify", "CHERRY_PICK_HEAD")
+	return err == nil
+}
+
 // AbortRebase aborts an in-progress rebase
 func (c *gitClient) AbortRebase() error {
 	if DryRun {
@@ -310,6 +324,16 @@ func (c *gitClient) AbortRebase() error {
 	return err
 }
 
+// AbortCherryPick aborts an in-progress cherry-pick
+func (c *gitClient) AbortCherryPick() error {
+	if DryRun {
+		fmt.Printf("  [DRY RUN] git cherry-pick --abort\n")
+		return nil
+	}
+	_, err := c.runCmd("cherry-pick", "--abort")
+	return err
+}
+
 // ResetToRemote resets the current branch to match the remote branch exactly
 func (c *gitClient) ResetToRemote(branch string) error {
 	remoteBranch := "origin/" + branch
@@ -331,6 +355,68 @@ func (c *gitClient) GetCommitHash(ref string) (string, error) {
 	return c.runCmd("rev-parse", ref)
 }
 
+// GetUniqueCommits returns the list of commits in branch that are not in base
+// Returns commit hashes in reverse chronological order (newest first)
+func (c *gitClient) GetUniqueCommits(base, branch string) ([]string, error) {
+	output, err := c.runCmd("rev-list", "--reverse", base+".."+branch)
+	if err != nil {
+		return nil, err
+	}
+	if output == "" {
+		return []string{}, nil
+	}
+	return strings.Split(output, "\n"), nil
+}
+
+// GetUniqueCommitsByPatch returns commits in branch that are not in base by comparing patch content
+// This uses git-cherry which compares patch IDs rather than commit SHAs, so it detects
+// duplicate changes even if commits were rebased (different SHAs but same content)
+// Returns commit hashes of truly unique patches
+func (c *gitClient) GetUniqueCommitsByPatch(base, branch string) ([]string, error) {
+	// git cherry outputs: "+ <sha>" for unique commits, "- <sha>" for duplicates
+	output, err := c.runCmd("cherry", base, branch)
+	if err != nil {
+		return nil, err
+	}
+	if output == "" {
+		return []string{}, nil
+	}
+
+	var uniqueCommits []string
+	lines := strings.Split(output, "\n")
+	for _, line := range lines {
+		line = strings.TrimSpace(line)
+		if strings.HasPrefix(line, "+") {
+			// Extract SHA (format is "+ <sha>")
+			parts := strings.Fields(line)
+			if len(parts) >= 2 {
+				uniqueCommits = append(uniqueCommits, parts[1])
+			}
+		}
+	}
+	return uniqueCommits, nil
+}
+
+// CherryPick cherry-picks a commit onto the current branch
+func (c *gitClient) CherryPick(commit string) error {
+	if DryRun {
+		fmt.Printf("  [DRY RUN] git cherry-pick %s\n", commit)
+		return nil
+	}
+	_, err := c.runCmd("cherry-pick", commit)
+	return err
+}
+
+// ResetHard resets the current branch to a ref
+func (c *gitClient) ResetHard(ref string) error {
+	if DryRun {
+		fmt.Printf("  [DRY RUN] git reset --hard %s\n", ref)
+		return nil
+	}
+	_, err := c.runCmd("reset", "--hard", ref)
+	return err
+}
+
 // Stash stashes the current changes
 func (c *gitClient) Stash(message string) error {
 	if DryRun {