diff --git a/cmd/status.go b/cmd/status.go index ed59619..3b7098d 100644 --- a/cmd/status.go +++ b/cmd/status.go @@ -297,6 +297,8 @@ func detectSyncIssues(gitClient git.GitClient, stackBranches []stack.StackBranch fmt.Printf("Checking %d branch(es) for sync issues...\n", len(stackBranches)) } + baseBranch := stack.GetBaseBranch(gitClient) + // Check each stack branch for sync issues for i, branch := range stackBranches { progress(fmt.Sprintf("Checking branch %d/%d (%s)...", i+1, len(stackBranches), branch.Name)) @@ -327,8 +329,23 @@ func detectSyncIssues(gitClient git.GitClient, stackBranches []stack.StackBranch } else if verbose { fmt.Printf(" ✓ PR base matches configured parent\n") } - } else if verbose { - fmt.Printf(" No PR found for this branch\n") + } else { + // No PR found — check if branch appears merged via git history + if verbose { + fmt.Printf(" No PR found, checking git history for merge...\n") + } + merged, err := gitClient.IsAncestor(branch.Name, "origin/"+baseBranch) + if err == nil && merged { + if verbose { + fmt.Printf(" ✓ Branch appears merged into %s via git history\n", baseBranch) + } + issues = append(issues, fmt.Sprintf(" - Branch '%s' appears merged into %s (run '%s' to clean up)", ui.Branch(branch.Name), ui.Branch(baseBranch), ui.Command("stack prune"))) + continue // Skip other checks for merged branches + } else if err != nil && verbose { + fmt.Printf(" ⚠ Could not check if branch is ancestor: %v\n", err) + } else if verbose { + fmt.Printf(" No PR found for this branch\n") + } } // Check if branch is behind its parent (needs rebase) - always check this regardless of PR diff --git a/cmd/status_test.go b/cmd/status_test.go index bde6075..21a1620 100644 --- a/cmd/status_test.go +++ b/cmd/status_test.go @@ -118,6 +118,7 @@ func TestDetectSyncIssues(t *testing.T) { }, prCache: make(map[string]*github.PRInfo), setupMocks: func(mockGit *testutil.MockGitClient) { + mockGit.On("IsAncestor", "feature-a", "origin/main").Return(false, nil) mockGit.On("IsCommitsBehind", "feature-a", "main").Return(true, nil) mockGit.On("RemoteBranchExists", "feature-a").Return(false) }, @@ -130,11 +131,23 @@ func TestDetectSyncIssues(t *testing.T) { }, prCache: make(map[string]*github.PRInfo), setupMocks: func(mockGit *testutil.MockGitClient) { + mockGit.On("IsAncestor", "feature-a", "origin/main").Return(false, nil) mockGit.On("IsCommitsBehind", "feature-a", "main").Return(false, nil) mockGit.On("RemoteBranchExists", "feature-a").Return(false) }, expectedIssues: 0, }, + { + name: "branch merged via git history (no PR)", + stackBranches: []stack.StackBranch{ + {Name: "feature-a", Parent: "main"}, + }, + prCache: make(map[string]*github.PRInfo), + setupMocks: func(mockGit *testutil.MockGitClient) { + mockGit.On("IsAncestor", "feature-a", "origin/main").Return(true, nil) + }, + expectedIssues: 1, + }, } for _, tt := range tests { diff --git a/cmd/sync.go b/cmd/sync.go index 4e1d1a4..3ac1e81 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -508,6 +508,24 @@ func runSync(gitClient git.GitClient, githubClient github.GitHubClient, syncRemo } fmt.Println() continue + } else if _, exists := prCache[branch.Name]; !exists { + // No PR found - check if branch was merged via git history + remoteBase := syncRemote + "/" + baseBranch + if merged, err := gitClient.IsAncestor(branch.Name, remoteBase); err == nil && merged { + fmt.Printf("%s Skipping %s (merged into %s, detected via git history)...\n", progress, ui.Branch(branch.Name), ui.Branch(baseBranch)) + fmt.Printf(" Removing from stack tracking...\n") + configKey := fmt.Sprintf("branch.%s.stackparent", branch.Name) + if err := gitClient.UnsetConfig(configKey); err != nil { + fmt.Fprintf(os.Stderr, " Warning: failed to remove stack config: %v\n", err) + } else { + fmt.Printf(" %s Removed. You can delete this branch with: %s\n", ui.SuccessIcon(), ui.Command(fmt.Sprintf("git branch -d %s", branch.Name))) + } + if branch.Name == originalBranch { + originalBranchMerged = true + } + fmt.Println() + continue + } } fmt.Printf("\n%s %s\n", progress, ui.Branch(branch.Name)) @@ -515,11 +533,22 @@ func runSync(gitClient git.GitClient, githubClient github.GitHubClient, syncRemo // Check if parent PR is merged oldParent := "" // Track old parent for --onto rebase parentPR := prCache[branch.Parent] + parentMergedViaGit := false if parentPR != nil && parentPR.State == "MERGED" { fmt.Printf(" Parent PR #%d has been merged\n", parentPR.Number) + oldParent = branch.Parent + } else if parentPR == nil && branch.Parent != baseBranch { + // No PR found for parent - check if parent was merged via git history + remoteBase := syncRemote + "/" + baseBranch + if merged, err := gitClient.IsAncestor(branch.Parent, remoteBase); err == nil && merged { + fmt.Printf(" Parent %s appears merged into %s (detected via git history)\n", ui.Branch(branch.Parent), ui.Branch(baseBranch)) + oldParent = branch.Parent + parentMergedViaGit = true + } + } + if oldParent != "" { // Save old parent for --onto rebase - oldParent = branch.Parent // Update parent to grandparent grandparent := gitClient.GetConfig(fmt.Sprintf("branch.%s.stackparent", branch.Parent)) @@ -534,6 +563,16 @@ func runSync(gitClient git.GitClient, githubClient github.GitHubClient, syncRemo } else { branch.Parent = grandparent } + + // If parent was detected as merged via git (no PR), also remove it from stack tracking + if parentMergedViaGit { + parentConfigKey := fmt.Sprintf("branch.%s.stackparent", oldParent) + if err := gitClient.UnsetConfig(parentConfigKey); err != nil { + if git.Verbose { + fmt.Fprintf(os.Stderr, " Note: could not remove stack config for %s: %v\n", oldParent, err) + } + } + } } // Checkout the branch diff --git a/cmd/sync_test.go b/cmd/sync_test.go index 42c78ea..17bafee 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -44,6 +44,8 @@ func TestRunSyncBasic(t *testing.T) { // Parallel operations mockGit.On("FetchRemote", "origin").Return(nil) mockGH.On("GetPRsForBranches", mock.Anything).Return(make(map[string]*github.PRInfo)) + // Git-based merge detection (no branches are merged) + mockGit.On("IsAncestor", mock.Anything, mock.Anything).Return(false, nil).Maybe() // Check if any branches in the current stack are in worktrees mockGit.On("GetWorktreeBranches").Return(make(map[string]string), nil) mockGit.On("GetCurrentWorktreePath").Return("/Users/test/repo", nil) @@ -129,6 +131,8 @@ func TestRunSyncMergedParent(t *testing.T) { "feature-a": testutil.NewPRInfo(1, "MERGED", "main", "Feature A", "url"), } mockGH.On("GetPRsForBranches", mock.Anything).Return(prCache) + // Git-based merge detection fallback (for branches without PRs) + mockGit.On("IsAncestor", mock.Anything, mock.Anything).Return(false, nil).Maybe() mockGit.On("GetWorktreeBranches").Return(make(map[string]string), nil) mockGit.On("GetCurrentWorktreePath").Return("/Users/test/repo", nil) @@ -285,6 +289,8 @@ func TestRunSyncStashHandling(t *testing.T) { mockGit.On("FetchRemote", "origin").Return(nil) mockGH.On("GetPRsForBranches", mock.Anything).Return(make(map[string]*github.PRInfo)) + // Git-based merge detection (no branches are merged) + mockGit.On("IsAncestor", mock.Anything, mock.Anything).Return(false, nil).Maybe() mockGit.On("GetWorktreeBranches").Return(make(map[string]string), nil) mockGit.On("GetCurrentWorktreePath").Return("/Users/test/repo", nil) @@ -348,6 +354,8 @@ func TestRunSyncErrorHandling(t *testing.T) { mockGit.On("FetchRemote", "origin").Return(nil) mockGH.On("GetPRsForBranches", mock.Anything).Return(make(map[string]*github.PRInfo)) + // Git-based merge detection (no branches are merged) + mockGit.On("IsAncestor", mock.Anything, mock.Anything).Return(false, nil).Maybe() mockGit.On("GetWorktreeBranches").Return(make(map[string]string), nil) mockGit.On("GetCurrentWorktreePath").Return("/Users/test/repo", nil) @@ -402,6 +410,8 @@ func TestRunSyncErrorHandling(t *testing.T) { mockGit.On("FetchRemote", "origin").Return(nil) mockGH.On("GetPRsForBranches", mock.Anything).Return(make(map[string]*github.PRInfo)) + // Git-based merge detection (no branches are merged) + mockGit.On("IsAncestor", mock.Anything, mock.Anything).Return(false, nil).Maybe() mockGit.On("GetWorktreeBranches").Return(make(map[string]string), nil) mockGit.On("GetCurrentWorktreePath").Return("/Users/test/repo", nil) @@ -537,6 +547,8 @@ func TestRunSyncResume(t *testing.T) { mockGit.On("FetchRemote", "origin").Return(nil) mockGH.On("GetPRsForBranches", mock.Anything).Return(make(map[string]*github.PRInfo)) + // Git-based merge detection (no branches are merged) + mockGit.On("IsAncestor", mock.Anything, mock.Anything).Return(false, nil).Maybe() mockGit.On("GetWorktreeBranches").Return(make(map[string]string), nil) mockGit.On("GetCurrentWorktreePath").Return("/Users/test/repo", nil) @@ -606,6 +618,8 @@ func TestRunSyncResume(t *testing.T) { mockGit.On("FetchRemote", "origin").Return(nil) mockGH.On("GetPRsForBranches", mock.Anything).Return(make(map[string]*github.PRInfo)) + // Git-based merge detection (no branches are merged) + mockGit.On("IsAncestor", mock.Anything, mock.Anything).Return(false, nil).Maybe() mockGit.On("GetWorktreeBranches").Return(make(map[string]string), nil) mockGit.On("GetCurrentWorktreePath").Return("/Users/test/repo", nil) @@ -694,6 +708,8 @@ func TestRunSyncAutoConfiguresMissingStackparent(t *testing.T) { // Parallel operations mockGit.On("FetchRemote", "origin").Return(nil) mockGH.On("GetPRsForBranches", mock.Anything).Return(make(map[string]*github.PRInfo)) + // Git-based merge detection (no branches are merged) + mockGit.On("IsAncestor", mock.Anything, mock.Anything).Return(false, nil).Maybe() // Worktree checks mockGit.On("GetWorktreeBranches").Return(make(map[string]string), nil) @@ -774,6 +790,8 @@ func TestRunSyncNoUniqueCommits(t *testing.T) { // Parallel operations mockGit.On("FetchRemote", "origin").Return(nil) mockGH.On("GetPRsForBranches", mock.Anything).Return(make(map[string]*github.PRInfo)) + // Git-based merge detection (no branches are merged) + mockGit.On("IsAncestor", mock.Anything, mock.Anything).Return(false, nil).Maybe() // Check if any branches in the current stack are in worktrees mockGit.On("GetWorktreeBranches").Return(make(map[string]string), nil) mockGit.On("GetCurrentWorktreePath").Return("/Users/test/repo", nil) @@ -967,6 +985,8 @@ func TestRunSyncWithUpstreamRemote(t *testing.T) { // Also fetch origin since syncRemote != "origin" mockGit.On("Fetch").Return(nil) mockGH.On("GetPRsForBranches", mock.Anything).Return(make(map[string]*github.PRInfo)) + // Git-based merge detection (no branches are merged) + mockGit.On("IsAncestor", mock.Anything, mock.Anything).Return(false, nil).Maybe() // Worktree checks mockGit.On("GetWorktreeBranches").Return(make(map[string]string), nil) mockGit.On("GetCurrentWorktreePath").Return("/Users/test/repo", nil) @@ -1060,6 +1080,8 @@ func TestRunSyncSkipsWorktreeBranches(t *testing.T) { // Parallel operations mockGit.On("FetchRemote", "origin").Return(nil) mockGH.On("GetPRsForBranches", mock.Anything).Return(make(map[string]*github.PRInfo)) + // Git-based merge detection (no branches are merged) + mockGit.On("IsAncestor", mock.Anything, mock.Anything).Return(false, nil).Maybe() // feature-b is in another worktree mockGit.On("GetWorktreeBranches").Return(map[string]string{ @@ -1175,3 +1197,142 @@ func TestRunPostSyncInstall(t *testing.T) { assert.Equal(t, "pnpm", detected.command) }) } + +func TestRunSyncGitBasedMergeDetection(t *testing.T) { + testutil.SetupTest() + defer testutil.TeardownTest() + + t.Run("removes branch from stack when merged via git history (no PR)", func(t *testing.T) { + mockGit := new(testutil.MockGitClient) + mockGH := new(testutil.MockGitHubClient) + + // Setup: no existing sync state + mockGit.On("GetConfig", "stack.sync.stashed").Return("") + mockGit.On("GetConfig", "stack.sync.originalBranch").Return("") + mockGit.On("GetCurrentBranch").Return("feature-b", nil) + mockGit.On("SetConfig", "stack.sync.originalBranch", "feature-b").Return(nil) + mockGit.On("IsWorkingTreeClean").Return(true, nil) + mockGit.On("GetConfig", "branch.feature-b.stackparent").Return("feature-a") + mockGit.On("GetConfig", "stack.baseBranch").Return("").Maybe() + mockGit.On("GetDefaultBranch").Return("main").Maybe() + + stackParents := map[string]string{ + "feature-a": "main", + "feature-b": "feature-a", + } + mockGit.On("GetAllStackParents").Return(stackParents, nil).Maybe() + + // No PRs found at all + mockGit.On("FetchRemote", "origin").Return(nil) + mockGH.On("GetPRsForBranches", mock.Anything).Return(make(map[string]*github.PRInfo)) + + mockGit.On("GetWorktreeBranches").Return(make(map[string]string), nil) + mockGit.On("GetCurrentWorktreePath").Return("/Users/test/repo", nil) + mockGit.On("GetRemoteBranchesSet").Return(map[string]bool{ + "main": true, + "feature-a": true, + "feature-b": true, + }) + + // feature-a: no PR, but IsAncestor returns true → merged via git history + mockGit.On("IsAncestor", "feature-a", "origin/main").Return(true, nil) + // Remove feature-a from stack + mockGit.On("UnsetConfig", "branch.feature-a.stackparent").Return(nil) + + // feature-b: no PR, not merged via git + mockGit.On("IsAncestor", "feature-b", "origin/main").Return(false, nil) + + // feature-b's parent (feature-a) has no PR, parent merged via git + // IsAncestor("feature-a", "origin/main") already mocked above → true + + // Reparent feature-b from feature-a to main + mockGit.On("GetConfig", "branch.feature-a.stackparent").Return("main") + mockGit.On("SetConfig", "branch.feature-b.stackparent", "main").Return(nil) + // Also remove parent from stack tracking (called again for parent cleanup) + mockGit.On("UnsetConfig", "branch.feature-a.stackparent").Return(nil) + + // Process feature-b: checkout, rebase onto origin/main (new parent) + 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("FetchBranchFromRemote", "origin", "main").Return(nil) + // --onto rebase since parent was merged + mockGit.On("RebaseOnto", "origin/main", "feature-a", "feature-b").Return(nil) + mockGit.On("FetchBranch", "feature-b").Return(nil) + mockGit.On("PushWithExpectedRemote", "feature-b", "def456").Return(nil) + + // Return to original branch + mockGit.On("CheckoutBranch", "feature-b").Return(nil) + // Clean up sync state + mockGit.On("UnsetConfig", "stack.sync.stashed").Return(nil) + mockGit.On("UnsetConfig", "stack.sync.originalBranch").Return(nil) + mockGit.On("GetConfig", "stack.postSyncInstall").Return("false").Maybe() + + err := runSync(mockGit, mockGH, "origin") + + assert.NoError(t, err) + mockGit.AssertExpectations(t) + mockGH.AssertExpectations(t) + }) + + t.Run("branch not merged via git is processed normally", func(t *testing.T) { + mockGit := new(testutil.MockGitClient) + mockGH := new(testutil.MockGitHubClient) + + // Setup: no existing sync state + mockGit.On("GetConfig", "stack.sync.stashed").Return("") + mockGit.On("GetConfig", "stack.sync.originalBranch").Return("") + mockGit.On("GetCurrentBranch").Return("feature-a", nil) + mockGit.On("SetConfig", "stack.sync.originalBranch", "feature-a").Return(nil) + mockGit.On("IsWorkingTreeClean").Return(true, nil) + mockGit.On("GetConfig", "branch.feature-a.stackparent").Return("main") + mockGit.On("GetConfig", "stack.baseBranch").Return("").Maybe() + mockGit.On("GetDefaultBranch").Return("main").Maybe() + + stackParents := map[string]string{ + "feature-a": "main", + } + mockGit.On("GetAllStackParents").Return(stackParents, nil).Maybe() + + // No PRs found + mockGit.On("FetchRemote", "origin").Return(nil) + mockGH.On("GetPRsForBranches", mock.Anything).Return(make(map[string]*github.PRInfo)) + // Git-based merge detection (no branches are merged) + mockGit.On("IsAncestor", mock.Anything, mock.Anything).Return(false, nil).Maybe() + + mockGit.On("GetWorktreeBranches").Return(make(map[string]string), nil) + mockGit.On("GetCurrentWorktreePath").Return("/Users/test/repo", nil) + mockGit.On("GetRemoteBranchesSet").Return(map[string]bool{ + "main": true, + "feature-a": true, + }) + + // feature-a: no PR, NOT merged via git either + mockGit.On("IsAncestor", "feature-a", "origin/main").Return(false, nil) + + // Should process normally (no parent reparenting since parent is baseBranch) + 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("FetchBranchFromRemote", "origin", "main").Return(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) + + // Return to original branch + mockGit.On("CheckoutBranch", "feature-a").Return(nil) + // Clean up sync state + mockGit.On("UnsetConfig", "stack.sync.stashed").Return(nil) + mockGit.On("UnsetConfig", "stack.sync.originalBranch").Return(nil) + mockGit.On("GetConfig", "stack.postSyncInstall").Return("false").Maybe() + + err := runSync(mockGit, mockGH, "origin") + + assert.NoError(t, err) + mockGit.AssertExpectations(t) + mockGH.AssertExpectations(t) + }) +} diff --git a/internal/git/git.go b/internal/git/git.go index 2ffa30c..cb05bca 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -687,6 +687,25 @@ func (c *gitClient) ListWorktrees() ([]string, error) { return paths, nil } +// IsAncestor checks if commit is an ancestor of branch using git merge-base --is-ancestor. +// Returns true if commit is an ancestor, false if not. Other exit codes return an error. +func (c *gitClient) IsAncestor(commit, branch string) (bool, error) { + if Verbose { + fmt.Printf(" [git] merge-base --is-ancestor %s %s\n", commit, branch) + } + cmd := exec.Command("git", "merge-base", "--is-ancestor", commit, branch) + err := cmd.Run() + if err == nil { + return true, nil + } + if exitErr, ok := err.(*exec.ExitError); ok { + if exitErr.ExitCode() == 1 { + return false, nil + } + } + return false, fmt.Errorf("git merge-base --is-ancestor %s %s failed: %v", commit, branch, err) +} + // GetRemoteURL returns the URL of the specified remote (e.g., "origin") func (c *gitClient) GetRemoteURL(remoteName string) string { return c.runCmdMayFail("remote", "get-url", remoteName) diff --git a/internal/git/interface.go b/internal/git/interface.go index 32352dc..8b6520d 100644 --- a/internal/git/interface.go +++ b/internal/git/interface.go @@ -54,4 +54,5 @@ type GitClient interface { RemoveWorktree(path string) error ListWorktrees() ([]string, error) GetRemoteURL(remoteName string) string + IsAncestor(commit, branch string) (bool, error) } diff --git a/internal/testutil/mocks.go b/internal/testutil/mocks.go index 0eaec64..56c700f 100644 --- a/internal/testutil/mocks.go +++ b/internal/testutil/mocks.go @@ -276,6 +276,11 @@ func (m *MockGitClient) GetRemoteURL(remoteName string) string { return args.String(0) } +func (m *MockGitClient) IsAncestor(commit, branch string) (bool, error) { + args := m.Called(commit, branch) + return args.Bool(0), args.Error(1) +} + // MockGitHubClient is a mock implementation of github.GitHubClient for testing type MockGitHubClient struct { mock.Mock