From 59b1b5877174b9196d071d3dbff6718a847a7e7c Mon Sep 17 00:00:00 2001 From: Jonatan Dahl Date: Wed, 21 Jan 2026 17:46:21 -0500 Subject: [PATCH 1/6] fix: include host in repo path for GitHub Enterprise remotes ParseRepoFromURL now returns HOST/OWNER/REPO for non-github.com hosts (e.g., ghe.spotify.net/owner/repo) so the gh CLI knows which host to use. Previously it only returned OWNER/REPO which defaulted to github.com. Co-Authored-By: Claude Opus 4.5 --- internal/github/github.go | 34 ++++++++++++++++++++++++---------- internal/github/github_test.go | 4 ++-- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/internal/github/github.go b/internal/github/github.go index d10285f..de66be6 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -36,12 +36,14 @@ func NewGitHubClient(repo string) GitHubClient { return &githubClient{repo: repo} } -// ParseRepoFromURL extracts OWNER/REPO from a git remote URL +// ParseRepoFromURL extracts HOST/OWNER/REPO or OWNER/REPO from a git remote URL +// For github.com, returns OWNER/REPO (gh CLI default) +// For other hosts (GHE), returns HOST/OWNER/REPO so gh CLI knows which host to use // Supports formats: -// - git@github.com:owner/repo.git -// - https://github.com/owner/repo.git -// - git@ghe.spotify.net:owner/repo.git -// - https://ghe.spotify.net/owner/repo +// - git@github.com:owner/repo.git -> owner/repo +// - https://github.com/owner/repo.git -> owner/repo +// - git@ghe.spotify.net:owner/repo.git -> ghe.spotify.net/owner/repo +// - https://ghe.spotify.net/owner/repo -> ghe.spotify.net/owner/repo func ParseRepoFromURL(remoteURL string) string { remoteURL = strings.TrimSpace(remoteURL) if remoteURL == "" { @@ -51,27 +53,39 @@ func ParseRepoFromURL(remoteURL string) string { // Remove .git suffix remoteURL = strings.TrimSuffix(remoteURL, ".git") + var host, path string + // Handle SSH format: git@host:owner/repo if strings.HasPrefix(remoteURL, "git@") { parts := strings.SplitN(remoteURL, ":", 2) if len(parts) == 2 { - return parts[1] + host = strings.TrimPrefix(parts[0], "git@") + path = parts[1] } } // Handle HTTPS format: https://host/owner/repo if strings.HasPrefix(remoteURL, "https://") || strings.HasPrefix(remoteURL, "http://") { - // Find the path after the host afterScheme := strings.TrimPrefix(remoteURL, "https://") afterScheme = strings.TrimPrefix(afterScheme, "http://") - // Split host from path slashIdx := strings.Index(afterScheme, "/") if slashIdx != -1 { - return afterScheme[slashIdx+1:] + host = afterScheme[:slashIdx] + path = afterScheme[slashIdx+1:] } } - return "" + if path == "" { + return "" + } + + // For github.com, just return OWNER/REPO (it's the default) + if host == "github.com" { + return path + } + + // For other hosts (GHE), return HOST/OWNER/REPO + return host + "/" + path } // runGH executes a gh CLI command and returns stdout diff --git a/internal/github/github_test.go b/internal/github/github_test.go index 1f4d15c..d7aa7e3 100644 --- a/internal/github/github_test.go +++ b/internal/github/github_test.go @@ -54,12 +54,12 @@ func TestParseRepoFromURL(t *testing.T) { { name: "GHE SSH format", url: "git@ghe.spotify.net:some-org/some-repo.git", - expected: "some-org/some-repo", + expected: "ghe.spotify.net/some-org/some-repo", }, { name: "GHE HTTPS format", url: "https://ghe.spotify.net/some-org/some-repo", - expected: "some-org/some-repo", + expected: "ghe.spotify.net/some-org/some-repo", }, { name: "empty string", From aba70e503da218b2386fa9b4a8a4440787568fc4 Mon Sep 17 00:00:00 2001 From: Jonatan Dahl Date: Thu, 22 Jan 2026 15:40:03 -0500 Subject: [PATCH 2/6] fix: fetch only open PRs to avoid 502 timeouts on large repos GetAllPRs was fetching all PRs (open, closed, merged) which caused 502 Bad Gateway errors on repos with thousands of PRs. - Changed to fetch only open PRs (--state open) - Added fallback to check individual branch PRs for stack branches not in the open PR cache, ensuring merged PR detection still works - Removed merged PR filtering (no longer needed with targeted fetches) - Added verbose output showing fetched PRs for debugging Co-Authored-By: Claude Opus 4.5 --- cmd/status.go | 146 ++++++++++++++++---------------------- cmd/status_test.go | 120 ++----------------------------- cmd/sync.go | 20 ++++++ internal/github/github.go | 27 ++++--- internal/stack/stack.go | 8 ++- 5 files changed, 103 insertions(+), 218 deletions(-) diff --git a/cmd/status.go b/cmd/status.go index 170c2cc..a4aeb7e 100644 --- a/cmd/status.go +++ b/cmd/status.go @@ -1,6 +1,7 @@ package cmd import ( + "bufio" "fmt" "os" "strings" @@ -74,6 +75,9 @@ func runStatus(gitClient git.GitClient, githubClient github.GitHubClient) error defer wg.Done() prCache, prErr = githubClient.GetAllPRs() if prErr != nil { + if verbose { + fmt.Printf(" [gh] Error fetching PRs: %v\n", prErr) + } // If fetching fails, fall back to empty cache prCache = make(map[string]*github.PRInfo) } @@ -113,6 +117,11 @@ func runStatus(gitClient git.GitClient, githubClient github.GitHubClient) error return fmt.Errorf("failed to build stack tree: %w", err) } + // If tree is nil, current branch is not in a stack + if tree == nil { + return nil // Will be handled after spinner + } + // Get ALL branch names in the tree (including intermediate branches without stackparent) allTreeBranches = getAllBranchNamesFromTree(tree) return nil @@ -129,13 +138,59 @@ func runStatus(gitClient git.GitClient, githubClient github.GitHubClient) error return nil } + // If tree is nil, current branch is not part of any stack + // Check this BEFORE waiting for PR fetch to avoid long delays + if tree == nil { + baseBranch := stack.GetBaseBranch(gitClient) + fmt.Printf("Current branch '%s' is not part of a stack.\n\n", currentBranch) + fmt.Printf("Add to stack with '%s' as parent? [Y/n] ", baseBranch) + + reader := bufio.NewReader(os.Stdin) + input, err := reader.ReadString('\n') + if err != nil { + return fmt.Errorf("failed to read input: %w", err) + } + + input = strings.TrimSpace(strings.ToLower(input)) + if input == "" || input == "y" || input == "yes" { + // Set the stackparent config + configKey := fmt.Sprintf("branch.%s.stackparent", currentBranch) + if err := gitClient.SetConfig(configKey, baseBranch); err != nil { + return fmt.Errorf("failed to set stack parent: %w", err) + } + fmt.Printf("✓ Added '%s' to stack with parent '%s'\n\n", currentBranch, baseBranch) + // Run status again to show the stack + return runStatus(gitClient, githubClient) + } + return nil + } + // Wait for PR fetch to complete (if running) if !noPR { wg.Wait() - } - // Filter out branches with merged PRs from the tree (but keep current branch) - tree = filterMergedBranches(tree, prCache, currentBranch) + // GetAllPRs only fetches open PRs (to avoid 502 timeouts on large repos). + // For branches in our stack that aren't in the cache, check individually + // to detect merged PRs that need special handling. + for _, branch := range stackBranches { + // Skip if already in cache (has open PR) + if _, exists := prCache[branch.Name]; exists { + continue + } + // Fetch PR info for this branch (might be merged or non-existent) + if pr, err := githubClient.GetPRForBranch(branch.Name); err == nil && pr != nil { + prCache[branch.Name] = pr + } + // Also check parent if not in cache and not base branch + if branch.Parent != stack.GetBaseBranch(gitClient) { + if _, exists := prCache[branch.Parent]; !exists { + if pr, err := githubClient.GetPRForBranch(branch.Parent); err == nil && pr != nil { + prCache[branch.Parent] = pr + } + } + } + } + } // Print the tree fmt.Println() @@ -198,44 +253,6 @@ func getAllBranchNamesFromTree(node *stack.TreeNode) []string { return result } -// filterMergedBranches removes branches with merged PRs from the tree, -// but only if they don't have children (to keep the stack structure visible) -// and they are not the current branch (always show where user is) -func filterMergedBranches(node *stack.TreeNode, prCache map[string]*github.PRInfo, currentBranch string) *stack.TreeNode { - if node == nil { - return nil - } - - // Filter children recursively first - var filteredChildren []*stack.TreeNode - for _, child := range node.Children { - // Recurse first to process all descendants - filtered := filterMergedBranches(child, prCache, currentBranch) - - // Only filter out merged branches if they have no children - // (i.e., they're leaf nodes) AND they're not the current branch - if pr, exists := prCache[child.Name]; exists && pr.State == "MERGED" { - // Always keep the current branch, even if merged - if child.Name == currentBranch { - filteredChildren = append(filteredChildren, filtered) - } else if filtered != nil && len(filtered.Children) > 0 { - // If this merged branch still has children after filtering, keep it - // so the stack structure remains visible - filteredChildren = append(filteredChildren, filtered) - } - // Otherwise skip this merged leaf branch - } else { - // Not merged, keep it - if filtered != nil { - filteredChildren = append(filteredChildren, filtered) - } - } - } - - node.Children = filteredChildren - return node -} - func printTree(gitClient git.GitClient, node *stack.TreeNode, prefix string, isLast bool, currentBranch string, prCache map[string]*github.PRInfo) { if node == nil { return @@ -280,15 +297,13 @@ func printTreeVertical(gitClient git.GitClient, node *stack.TreeNode, currentBra // syncIssuesResult holds the result of detectSyncIssues type syncIssuesResult struct { - issues []string - mergedBranches []string + issues []string } // detectSyncIssues checks if any branches are out of sync and returns the issues (doesn't print) // If skipFetch is true, assumes git fetch was already called (to avoid redundant network calls) func detectSyncIssues(gitClient git.GitClient, stackBranches []stack.StackBranch, prCache map[string]*github.PRInfo, progress spinner.ProgressFunc, skipFetch bool) (*syncIssuesResult, error) { var issues []string - var mergedBranches []string // Fetch once upfront to ensure we have latest remote refs (unless already done) if !skipFetch { @@ -311,27 +326,6 @@ func detectSyncIssues(gitClient git.GitClient, stackBranches []stack.StackBranch fmt.Printf("\n[%d/%d] Checking '%s' (parent: %s)\n", i+1, len(stackBranches), branch.Name, branch.Parent) } - // Track branches with merged PRs (for cleanup suggestion, not sync) - if pr, exists := prCache[branch.Name]; exists && pr.State == "MERGED" { - if verbose { - fmt.Printf(" ✓ Branch has merged PR #%d - marking for cleanup\n", pr.Number) - } - mergedBranches = append(mergedBranches, branch.Name) - continue // Don't check other sync issues for merged branches - } - - // Check if parent has a merged PR (child needs to be updated) - if branch.Parent != stack.GetBaseBranch(gitClient) { - if parentPR, exists := prCache[branch.Parent]; exists && parentPR.State == "MERGED" { - if verbose { - fmt.Printf(" ✗ Parent '%s' has merged PR #%d\n", branch.Parent, parentPR.Number) - } - issues = append(issues, fmt.Sprintf(" - Branch '%s' parent '%s' has a merged PR", branch.Name, branch.Parent)) - } else if verbose { - fmt.Printf(" ✓ Parent '%s' is not merged\n", branch.Parent) - } - } - // Check if PR base matches the configured parent (if PR exists) if pr, exists := prCache[branch.Name]; exists { if verbose { @@ -394,14 +388,12 @@ func detectSyncIssues(gitClient git.GitClient, stackBranches []stack.StackBranch } return &syncIssuesResult{ - issues: issues, - mergedBranches: mergedBranches, + issues: issues, }, nil } // printSyncIssues prints the sync issues result func printSyncIssues(result *syncIssuesResult) { - // If issues found, print warning if len(result.issues) > 0 { fmt.Println() fmt.Println("⚠ Stack out of sync detected:") @@ -410,26 +402,8 @@ func printSyncIssues(result *syncIssuesResult) { } fmt.Println() fmt.Println("Run 'stack sync' to rebase branches and update PR bases.") - - // Also mention merged branches if any - if len(result.mergedBranches) > 0 { - fmt.Println() - fmt.Printf("After syncing, clean up merged branches with 'stack prune': %s\n", strings.Join(result.mergedBranches, ", ")) - } - } else if len(result.mergedBranches) > 0 { - // Merged branches need cleanup via prune - fmt.Println() - fmt.Printf("⚠ Merged branches need cleanup: %s\n", strings.Join(result.mergedBranches, ", ")) - fmt.Println() - fmt.Println("Run 'stack prune' to remove merged branches.") } else { - // Everything is perfectly synced fmt.Println() fmt.Println("✓ Stack is perfectly synced! All branches are up to date.") } } - -// Helper to repeat a string n times -func repeatString(s string, n int) string { - return strings.Repeat(s, n) -} diff --git a/cmd/status_test.go b/cmd/status_test.go index 00d8f0c..3887b9b 100644 --- a/cmd/status_test.go +++ b/cmd/status_test.go @@ -74,97 +74,6 @@ func TestRunStatus(t *testing.T) { } } -func TestFilterMergedBranches(t *testing.T) { - testutil.SetupTest() - defer testutil.TeardownTest() - - tests := []struct { - name string - tree *stack.TreeNode - prCache map[string]*github.PRInfo - currentBranch string - expectFiltered bool - expectedBranches []string - }{ - { - name: "keep merged branch with children", - tree: &stack.TreeNode{ - Name: "main", - Children: []*stack.TreeNode{ - { - Name: "feature-a", - Children: []*stack.TreeNode{ - {Name: "feature-b", Children: nil}, - }, - }, - }, - }, - prCache: map[string]*github.PRInfo{ - "feature-a": testutil.NewPRInfo(1, "MERGED", "main", "Feature A", "url"), - "feature-b": testutil.NewPRInfo(2, "OPEN", "feature-a", "Feature B", "url"), - }, - currentBranch: "feature-b", - expectedBranches: []string{"main", "feature-a", "feature-b"}, // Keep feature-a because it has children - }, - { - name: "filter merged leaf branch", - tree: &stack.TreeNode{ - Name: "main", - Children: []*stack.TreeNode{ - { - Name: "feature-a", - Children: nil, - }, - }, - }, - prCache: map[string]*github.PRInfo{ - "feature-a": testutil.NewPRInfo(1, "MERGED", "main", "Feature A", "url"), - }, - currentBranch: "main", - expectedBranches: []string{"main"}, // Filter out feature-a because it's a merged leaf - }, - { - name: "keep current branch even if merged", - tree: &stack.TreeNode{ - Name: "main", - Children: []*stack.TreeNode{ - { - Name: "feature-a", - Children: nil, - }, - }, - }, - prCache: map[string]*github.PRInfo{ - "feature-a": testutil.NewPRInfo(1, "MERGED", "main", "Feature A", "url"), - }, - currentBranch: "feature-a", - expectedBranches: []string{"main", "feature-a"}, // Keep feature-a because it's current branch - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - filtered := filterMergedBranches(tt.tree, tt.prCache, tt.currentBranch) - - // Collect all branch names from filtered tree - var branches []string - var collectBranches func(*stack.TreeNode) - collectBranches = func(node *stack.TreeNode) { - if node == nil { - return - } - branches = append(branches, node.Name) - for _, child := range node.Children { - collectBranches(child) - } - } - collectBranches(filtered) - - assert.Equal(t, tt.expectedBranches, branches) - }) - } -} - func TestGetAllBranchNamesFromTree(t *testing.T) { testutil.SetupTest() defer testutil.TeardownTest() @@ -201,7 +110,6 @@ func TestDetectSyncIssues(t *testing.T) { prCache map[string]*github.PRInfo setupMocks func(*testutil.MockGitClient) expectedIssues int - expectedMerged int }{ { name: "branch behind parent", @@ -214,37 +122,18 @@ func TestDetectSyncIssues(t *testing.T) { mockGit.On("RemoteBranchExists", "feature-a").Return(false) }, expectedIssues: 1, - expectedMerged: 0, }, { - name: "branch with merged PR", + name: "branch up to date", stackBranches: []stack.StackBranch{ {Name: "feature-a", Parent: "main"}, }, - prCache: map[string]*github.PRInfo{ - "feature-a": testutil.NewPRInfo(1, "MERGED", "main", "Feature A", "url"), - }, + prCache: make(map[string]*github.PRInfo), setupMocks: func(mockGit *testutil.MockGitClient) { - // No calls expected for merged branches + mockGit.On("IsCommitsBehind", "feature-a", "main").Return(false, nil) + mockGit.On("RemoteBranchExists", "feature-a").Return(false) }, expectedIssues: 0, - expectedMerged: 1, - }, - { - name: "parent PR merged", - stackBranches: []stack.StackBranch{ - {Name: "feature-b", Parent: "feature-a"}, - }, - prCache: map[string]*github.PRInfo{ - "feature-a": testutil.NewPRInfo(1, "MERGED", "main", "Feature A", "url"), - }, - setupMocks: func(mockGit *testutil.MockGitClient) { - mockGit.On("GetDefaultBranch").Return("main") - mockGit.On("IsCommitsBehind", "feature-b", "feature-a").Return(false, nil) - mockGit.On("RemoteBranchExists", "feature-b").Return(false) - }, - expectedIssues: 1, // Issue because parent is merged - expectedMerged: 0, }, } @@ -263,7 +152,6 @@ func TestDetectSyncIssues(t *testing.T) { assert.NoError(t, err) assert.NotNil(t, result) assert.Len(t, result.issues, tt.expectedIssues, "Expected %d issues, got %d", tt.expectedIssues, len(result.issues)) - assert.Len(t, result.mergedBranches, tt.expectedMerged, "Expected %d merged branches, got %d", tt.expectedMerged, len(result.mergedBranches)) mockGit.AssertExpectations(t) }) diff --git a/cmd/sync.go b/cmd/sync.go index b2a9284..ae7b186 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -393,6 +393,26 @@ func runSync(gitClient git.GitClient, githubClient github.GitHubClient) error { prCache = make(map[string]*github.PRInfo) } + // GetAllPRs only fetches open PRs (to avoid 502 timeouts on large repos). + // For branches in our stack that aren't in the cache, check individually + // to detect merged PRs that need special handling. + for _, branch := range sorted { + // Skip if already in cache (has open PR) + if _, exists := prCache[branch.Name]; exists { + continue + } + // Fetch PR info for this branch (might be merged or non-existent) + if pr, err := githubClient.GetPRForBranch(branch.Name); err == nil && pr != nil { + prCache[branch.Name] = pr + } + // Also check parent if not in cache + if _, exists := prCache[branch.Parent]; !exists { + if pr, err := githubClient.GetPRForBranch(branch.Parent); err == nil && pr != nil { + prCache[branch.Parent] = pr + } + } + } + // Get all remote branches in one call (more efficient than checking each branch individually) remoteBranches := gitClient.GetRemoteBranchesSet() diff --git a/internal/github/github.go b/internal/github/github.go index de66be6..e7eaa96 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -141,10 +141,11 @@ func (c *githubClient) GetPRForBranch(branch string) (*PRInfo, error) { }, nil } -// GetAllPRs fetches all PRs for the repository in a single call +// GetAllPRs fetches all open PRs for the repository in a single call +// Only fetches open PRs to avoid timeouts on repos with many PRs func (c *githubClient) GetAllPRs() (map[string]*PRInfo, error) { - // Fetch all PRs (open, closed, and merged) in one call - output, err := c.runGH("pr", "list", "--state", "all", "--json", "number,state,headRefName,baseRefName,title,url,mergeStateStatus", "--limit", "1000") + // Fetch only open PRs - much faster and avoids 502 timeouts on large repos + output, err := c.runGH("pr", "list", "--state", "open", "--json", "number,state,headRefName,baseRefName,title,url,mergeStateStatus", "--limit", "500") if err != nil { return nil, fmt.Errorf("failed to list PRs: %w", err) } @@ -163,12 +164,17 @@ func (c *githubClient) GetAllPRs() (map[string]*PRInfo, error) { return nil, fmt.Errorf("failed to parse PR list: %w", err) } + if Verbose { + fmt.Printf(" [gh] Fetched %d PRs\n", len(prs)) + for _, pr := range prs { + fmt.Printf(" [gh] - %s (PR #%d, %s)\n", pr.HeadRefName, pr.Number, pr.State) + } + } + // Create a map of branch name -> PR info - // When multiple PRs exist for the same branch, prefer OPEN over closed/merged prMap := make(map[string]*PRInfo) for _, pr := range prs { - existing, exists := prMap[pr.HeadRefName] - prInfo := &PRInfo{ + prMap[pr.HeadRefName] = &PRInfo{ Number: pr.Number, State: pr.State, Base: pr.BaseRefName, @@ -176,15 +182,6 @@ func (c *githubClient) GetAllPRs() (map[string]*PRInfo, error) { URL: pr.URL, MergeStateStatus: pr.MergeStateStatus, } - - if !exists { - // No PR for this branch yet, add it - prMap[pr.HeadRefName] = prInfo - } else if pr.State == "OPEN" && existing.State != "OPEN" { - // New PR is open and existing is not - prefer the open one - prMap[pr.HeadRefName] = prInfo - } - // Otherwise keep the existing PR (first open PR wins, or first closed if no open) } return prMap, nil diff --git a/internal/stack/stack.go b/internal/stack/stack.go index 017f522..93f6bfb 100644 --- a/internal/stack/stack.go +++ b/internal/stack/stack.go @@ -64,6 +64,11 @@ func GetStackChain(gitClient git.GitClient, branch string) ([]string, error) { return nil, err } + // If current branch has no stackparent, it's not in a stack + if parents[branch] == "" { + return []string{}, nil + } + var chain []string current := branch seen := make(map[string]bool) @@ -191,7 +196,8 @@ func BuildStackTreeForBranch(gitClient git.GitClient, branchName string) (*TreeN } if len(chain) == 0 { - return nil, fmt.Errorf("no stack chain found for branch %s", branchName) + // Current branch is not in a stack + return nil, nil } // Build a set of branches in the chain for quick lookup From e34b014406a1a497f30acafd841e3fcbace49e1e Mon Sep 17 00:00:00 2001 From: Jonatan Dahl Date: Thu, 22 Jan 2026 16:55:59 -0500 Subject: [PATCH 3/6] fix: add GetPRForBranch mock expectations to sync tests Tests were failing because GetPRForBranch is now called for branches not in the PR cache (to detect merged PRs). Added mock expectations for these calls. Co-Authored-By: Claude Opus 4.5 --- cmd/sync_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/cmd/sync_test.go b/cmd/sync_test.go index 3f52931..1abc9c1 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -42,6 +42,10 @@ func TestRunSyncBasic(t *testing.T) { // Parallel operations mockGit.On("Fetch").Return(nil) mockGH.On("GetAllPRs").Return(make(map[string]*github.PRInfo), nil) + // GetPRForBranch is called for branches not in the cache (to detect merged PRs) + mockGH.On("GetPRForBranch", "feature-a").Return(nil, nil).Maybe() + mockGH.On("GetPRForBranch", "feature-b").Return(nil, nil).Maybe() + mockGH.On("GetPRForBranch", "main").Return(nil, 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) @@ -280,6 +284,9 @@ func TestRunSyncStashHandling(t *testing.T) { mockGit.On("Fetch").Return(nil) mockGH.On("GetAllPRs").Return(make(map[string]*github.PRInfo), nil) + // GetPRForBranch is called for branches not in the cache (to detect merged PRs) + mockGH.On("GetPRForBranch", "feature-a").Return(nil, nil).Maybe() + mockGH.On("GetPRForBranch", "main").Return(nil, nil).Maybe() mockGit.On("GetWorktreeBranches").Return(make(map[string]string), nil) mockGit.On("GetCurrentWorktreePath").Return("/Users/test/repo", nil) @@ -342,6 +349,9 @@ func TestRunSyncErrorHandling(t *testing.T) { mockGit.On("Fetch").Return(nil) mockGH.On("GetAllPRs").Return(make(map[string]*github.PRInfo), nil) + // GetPRForBranch is called for branches not in the cache (to detect merged PRs) + mockGH.On("GetPRForBranch", "feature-a").Return(nil, nil).Maybe() + mockGH.On("GetPRForBranch", "main").Return(nil, nil).Maybe() mockGit.On("GetWorktreeBranches").Return(make(map[string]string), nil) mockGit.On("GetCurrentWorktreePath").Return("/Users/test/repo", nil) @@ -396,6 +406,9 @@ func TestRunSyncErrorHandling(t *testing.T) { mockGit.On("Fetch").Return(nil) mockGH.On("GetAllPRs").Return(make(map[string]*github.PRInfo), nil) + // GetPRForBranch is called for branches not in the cache (to detect merged PRs) + mockGH.On("GetPRForBranch", "feature-a").Return(nil, nil).Maybe() + mockGH.On("GetPRForBranch", "main").Return(nil, nil).Maybe() mockGit.On("GetWorktreeBranches").Return(make(map[string]string), nil) mockGit.On("GetCurrentWorktreePath").Return("/Users/test/repo", nil) @@ -541,6 +554,9 @@ func TestRunSyncResume(t *testing.T) { mockGit.On("Fetch").Return(nil) mockGH.On("GetAllPRs").Return(make(map[string]*github.PRInfo), nil) + // GetPRForBranch is called for branches not in the cache (to detect merged PRs) + mockGH.On("GetPRForBranch", "feature-a").Return(nil, nil).Maybe() + mockGH.On("GetPRForBranch", "main").Return(nil, nil).Maybe() mockGit.On("GetWorktreeBranches").Return(make(map[string]string), nil) mockGit.On("GetCurrentWorktreePath").Return("/Users/test/repo", nil) @@ -608,6 +624,9 @@ func TestRunSyncResume(t *testing.T) { mockGit.On("Fetch").Return(nil) mockGH.On("GetAllPRs").Return(make(map[string]*github.PRInfo), nil) + // GetPRForBranch is called for branches not in the cache (to detect merged PRs) + mockGH.On("GetPRForBranch", "feature-a").Return(nil, nil).Maybe() + mockGH.On("GetPRForBranch", "main").Return(nil, nil).Maybe() mockGit.On("GetWorktreeBranches").Return(make(map[string]string), nil) mockGit.On("GetCurrentWorktreePath").Return("/Users/test/repo", nil) From 2680b4d28af99bd920a21c225a550b4891ca7763 Mon Sep 17 00:00:00 2001 From: Jonatan Dahl Date: Thu, 22 Jan 2026 17:00:48 -0500 Subject: [PATCH 4/6] fix: add missing GetPRForBranch mock in TestRunSyncMergedParent Co-Authored-By: Claude Opus 4.5 --- cmd/sync_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/sync_test.go b/cmd/sync_test.go index 1abc9c1..3cffd3b 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -130,6 +130,8 @@ func TestRunSyncMergedParent(t *testing.T) { "feature-a": testutil.NewPRInfo(1, "MERGED", "main", "Feature A", "url"), } mockGH.On("GetAllPRs").Return(prCache, nil) + // GetPRForBranch is called for branches not in the cache (to detect merged PRs) + mockGH.On("GetPRForBranch", "feature-b").Return(nil, nil).Maybe() mockGit.On("GetWorktreeBranches").Return(make(map[string]string), nil) mockGit.On("GetCurrentWorktreePath").Return("/Users/test/repo", nil) From 0a7c7b8934d2e83d6816c07b72c2982c720d94de Mon Sep 17 00:00:00 2001 From: Jonatan Dahl Date: Thu, 22 Jan 2026 17:04:25 -0500 Subject: [PATCH 5/6] fix: update TestRunSyncNoStackBranches mock expectations When there are no stack branches, code returns early before calling GetWorktreeBranches, GetRemoteBranchesSet, etc. Updated test to reflect the actual code path. Co-Authored-By: Claude Opus 4.5 --- cmd/sync_test.go | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/cmd/sync_test.go b/cmd/sync_test.go index 3cffd3b..feb09dd 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -490,25 +490,16 @@ func TestRunSyncNoStackBranches(t *testing.T) { stackParents := map[string]string{} mockGit.On("GetAllStackParents").Return(stackParents, nil).Maybe() - mockGit.On("Fetch").Return(nil) - mockGH.On("GetAllPRs").Return(make(map[string]*github.PRInfo), nil) + // When there are no stack branches, code returns early after parallel ops + // These are started but may not complete before early return + mockGit.On("Fetch").Return(nil).Maybe() + mockGH.On("GetAllPRs").Return(make(map[string]*github.PRInfo), 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, - }) - - mockGit.On("CheckoutBranch", "main").Return(nil) // Return to original branch - // Clean up sync state - mockGit.On("UnsetConfig", "stack.sync.stashed").Return(nil) - mockGit.On("UnsetConfig", "stack.sync.originalBranch").Return(nil) + // These calls don't happen when there are no stack branches (early return) err := runSync(mockGit, mockGH) assert.NoError(t, err) - mockGit.AssertExpectations(t) - mockGH.AssertExpectations(t) } func TestRunSyncResume(t *testing.T) { From dc056da73816e08f67fc4dc1daf5fa62d0b1ab80 Mon Sep 17 00:00:00 2001 From: Jonatan Dahl Date: Thu, 22 Jan 2026 16:31:59 -0500 Subject: [PATCH 6/6] fix: don't suggest rebase for merged PRs in status When a branch's PR is merged, status was incorrectly reporting "needs rebase" because the branch is behind master. Merged branches don't need any action - skip them in sync issue detection. Co-Authored-By: Claude Opus 4.5 --- cmd/status.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/cmd/status.go b/cmd/status.go index a4aeb7e..86b389f 100644 --- a/cmd/status.go +++ b/cmd/status.go @@ -326,6 +326,14 @@ func detectSyncIssues(gitClient git.GitClient, stackBranches []stack.StackBranch fmt.Printf("\n[%d/%d] Checking '%s' (parent: %s)\n", i+1, len(stackBranches), branch.Name, branch.Parent) } + // Skip branches with merged PRs - they don't need any sync action + if pr, exists := prCache[branch.Name]; exists && pr.State == "MERGED" { + if verbose { + fmt.Printf(" Skipping (PR is merged)\n") + } + continue + } + // Check if PR base matches the configured parent (if PR exists) if pr, exists := prCache[branch.Name]; exists { if verbose {