Skip to content

Commit 525ab29

Browse files
javoireclaude
andauthored
fix: sync rebases branches with no unique commits (#63)
When GetUniqueCommitsByPatch returns 0 commits, sync was returning early without performing a rebase. This caused branches to remain behind origin/master even though the output claimed success. A branch can have no unique patches (e.g., all changes already merged, or only merge commits which git cherry ignores) but still need rebasing to incorporate new commits from the target. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent ef35227 commit 525ab29

2 files changed

Lines changed: 72 additions & 3 deletions

File tree

cmd/sync.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -625,12 +625,14 @@ func runSync(gitClient git.GitClient, githubClient github.GitHubClient) error {
625625
return gitClient.Rebase(rebaseTarget)
626626
}
627627

628-
// If no unique commits, branch is up-to-date
628+
// If no unique commits by patch comparison, just do a simple rebase.
629+
// The branch might still need to incorporate new commits from the target.
630+
// Rebase will be a no-op if truly up-to-date.
629631
if len(uniqueCommits) == 0 {
630632
if git.Verbose {
631-
fmt.Printf(" Branch is up-to-date with %s (no unique patches)\n", rebaseTarget)
633+
fmt.Printf(" No unique patches found, rebasing to incorporate target updates\n")
632634
}
633-
return nil
635+
return gitClient.Rebase(rebaseTarget)
634636
}
635637

636638
if git.Verbose {

cmd/sync_test.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -758,6 +758,73 @@ func TestRunSyncAutoConfiguresMissingStackparent(t *testing.T) {
758758
})
759759
}
760760

761+
func TestRunSyncNoUniqueCommits(t *testing.T) {
762+
testutil.SetupTest()
763+
defer testutil.TeardownTest()
764+
765+
t.Run("rebases even when no unique commits by patch", func(t *testing.T) {
766+
// This tests that sync still rebases when GetUniqueCommitsByPatch returns 0 commits.
767+
// A branch may have no unique patches but still be behind origin/master
768+
// (e.g., branch with only merge commits, or branch whose changes are already in master).
769+
mockGit := new(testutil.MockGitClient)
770+
mockGH := new(testutil.MockGitHubClient)
771+
772+
// Setup: Check for existing sync state (none)
773+
mockGit.On("GetConfig", "stack.sync.stashed").Return("")
774+
mockGit.On("GetConfig", "stack.sync.originalBranch").Return("")
775+
// Setup: Get current branch
776+
mockGit.On("GetCurrentBranch").Return("feature-a", nil)
777+
// Save original branch state
778+
mockGit.On("SetConfig", "stack.sync.originalBranch", "feature-a").Return(nil)
779+
// Check working tree
780+
mockGit.On("IsWorkingTreeClean").Return(true, nil)
781+
// Get base branch
782+
mockGit.On("GetConfig", "branch.feature-a.stackparent").Return("main")
783+
mockGit.On("GetConfig", "stack.baseBranch").Return("").Maybe()
784+
mockGit.On("GetDefaultBranch").Return("main").Maybe()
785+
// Get stack chain
786+
stackParents := map[string]string{
787+
"feature-a": "main",
788+
}
789+
mockGit.On("GetAllStackParents").Return(stackParents, nil).Maybe()
790+
// Parallel operations
791+
mockGit.On("Fetch").Return(nil)
792+
mockGH.On("GetAllPRs").Return(make(map[string]*github.PRInfo), nil)
793+
mockGH.On("GetPRForBranch", "feature-a").Return(nil, nil).Maybe()
794+
mockGH.On("GetPRForBranch", "main").Return(nil, nil).Maybe()
795+
// Check if any branches in the current stack are in worktrees
796+
mockGit.On("GetWorktreeBranches").Return(make(map[string]string), nil)
797+
mockGit.On("GetCurrentWorktreePath").Return("/Users/test/repo", nil)
798+
// Get remote branches
799+
mockGit.On("GetRemoteBranchesSet").Return(map[string]bool{
800+
"main": true,
801+
"feature-a": true,
802+
})
803+
// Process feature-a
804+
mockGit.On("CheckoutBranch", "feature-a").Return(nil)
805+
mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil)
806+
mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil)
807+
mockGit.On("FetchBranch", "main").Return(nil)
808+
// GetUniqueCommitsByPatch returns empty slice - no unique commits
809+
// But Rebase should STILL be called to incorporate target updates
810+
mockGit.On("GetUniqueCommitsByPatch", "origin/main", "feature-a").Return([]string{}, nil)
811+
mockGit.On("Rebase", "origin/main").Return(nil)
812+
mockGit.On("FetchBranch", "feature-a").Return(nil)
813+
mockGit.On("PushWithExpectedRemote", "feature-a", "abc123").Return(nil)
814+
// Return to original branch
815+
mockGit.On("CheckoutBranch", "feature-a").Return(nil)
816+
// Clean up sync state
817+
mockGit.On("UnsetConfig", "stack.sync.stashed").Return(nil)
818+
mockGit.On("UnsetConfig", "stack.sync.originalBranch").Return(nil)
819+
820+
err := runSync(mockGit, mockGH)
821+
822+
assert.NoError(t, err)
823+
mockGit.AssertExpectations(t)
824+
mockGH.AssertExpectations(t)
825+
})
826+
}
827+
761828
func TestRunSyncAbort(t *testing.T) {
762829
testutil.SetupTest()
763830
defer testutil.TeardownTest()

0 commit comments

Comments
 (0)