diff --git a/cmd/checkout.go b/cmd/checkout.go index e0d2162..d66087c 100644 --- a/cmd/checkout.go +++ b/cmd/checkout.go @@ -92,7 +92,7 @@ func runCheckout(cfg *config.Config, opts *checkoutOptions) error { } else { // Non-numeric target — resolve against local stacks only var br *stack.BranchRef - s, br, err = resolvePR(sf, opts.target) + s, br, err = resolvePR(cfg, sf, opts.target) if err != nil { cfg.Errorf("%s", err) return ErrNotInStack @@ -194,10 +194,13 @@ func checkoutRemoteStack(cfg *config.Config, sf *stack.StackFile, gitDir string, // Determine trunk (base branch of the first PR) and the target branch trunk := prs[0].BaseRefName var targetBranch string + allMerged := true for _, pr := range prs { if pr.Number == prNumber { targetBranch = pr.HeadRefName - break + } + if !pr.Merged { + allMerged = false } } if targetBranch == "" { @@ -205,6 +208,12 @@ func checkoutRemoteStack(cfg *config.Config, sf *stack.StackFile, gitDir string, return nil, "", ErrAPIFailure } + if allMerged { + cfg.Infof("All PRs in this stack have been merged") + cfg.Printf("To start a new stack, use `%s`", cfg.ColorCyan("gh stack init")) + return nil, "", ErrSilent + } + remoteStackID := strconv.Itoa(remoteStack.ID) // Step 3: Check if the target branch is already in a local stack @@ -461,7 +470,9 @@ func importRemoteStack( } } - // Create local branches for each PR's head branch + // Create local branches for each PR's head branch. + // Skip merged PRs whose branches were deleted from the remote — + // these no longer exist upstream and can't be created locally. for _, pr := range prs { branch := pr.HeadRefName if git.BranchExists(branch) { @@ -469,6 +480,10 @@ func importRemoteStack( } remoteRef := remote + "/" + branch if err := git.CreateBranch(branch, remoteRef); err != nil { + if pr.Merged { + cfg.Infof("Skipping merged branch %s", branch) + continue + } cfg.Errorf("failed to pull branch %s from %s: %v", branch, remoteRef, err) return nil, ErrSilent } diff --git a/cmd/checkout_test.go b/cmd/checkout_test.go index 7ce18fb..61da63e 100644 --- a/cmd/checkout_test.go +++ b/cmd/checkout_test.go @@ -618,6 +618,107 @@ func TestCheckout_NumericTarget_ClosedMergedPR(t *testing.T) { assert.False(t, sf.Stacks[0].Branches[1].PullRequest.Merged) } +func TestCheckout_NumericTarget_MergedBranchDeletedFromRemote(t *testing.T) { + gitDir := t.TempDir() + var checkedOut string + + restore := git.SetOps(&git.MockOps{ + GitDirFn: func() (string, error) { return gitDir, nil }, + CurrentBranchFn: func() (string, error) { return "main", nil }, + BranchExistsFn: func(name string) bool { + return name == "main" + }, + FetchFn: func(remote string) error { return nil }, + CreateBranchFn: func(name, base string) error { + // Simulate merged branch deleted from remote: origin/feat-1 doesn't exist + if base == "origin/feat-1" { + return fmt.Errorf("failed to run git: fatal: not a valid object name: 'origin/feat-1'") + } + return nil + }, + SetUpstreamTrackingFn: func(branch, remote string) error { return nil }, + ResolveRemoteFn: func(branch string) (string, error) { + return "origin", nil + }, + CheckoutBranchFn: func(name string) error { + checkedOut = name + return nil + }, + RevParseFn: func(ref string) (string, error) { + return "abc123", nil + }, + RevParseMultiFn: func(refs []string) ([]string, error) { + shas := make([]string, len(refs)) + for i := range refs { + shas[i] = "abc123" + } + return shas, nil + }, + }) + defer restore() + + require.NoError(t, stack.Save(gitDir, &stack.StackFile{SchemaVersion: 1, Stacks: []stack.Stack{}})) + + cfg, outR, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{ + {ID: 60, PullRequests: []int{10, 11}}, + }, nil + }, + FindPRByNumberFn: func(number int) (*github.PullRequest, error) { + prs := map[int]*github.PullRequest{ + 10: {ID: "PR_10", Number: 10, HeadRefName: "feat-1", BaseRefName: "main", Merged: true, State: "MERGED", URL: "https://github.com/o/r/pull/10"}, + 11: {ID: "PR_11", Number: 11, HeadRefName: "feat-2", BaseRefName: "feat-1", State: "OPEN", URL: "https://github.com/o/r/pull/11"}, + } + return prs[number], nil + }, + } + + err := runCheckout(cfg, &checkoutOptions{target: "11"}) + output := collectOutput(cfg, outR, errR) + + require.NoError(t, err) + assert.Equal(t, "feat-2", checkedOut) + assert.Contains(t, output, "Skipping merged branch feat-1") + assert.Contains(t, output, "Imported stack with 2 branches") +} + +func TestCheckout_NumericTarget_AllPRsMerged(t *testing.T) { + gitDir := t.TempDir() + + restore := git.SetOps(&git.MockOps{ + GitDirFn: func() (string, error) { return gitDir, nil }, + CurrentBranchFn: func() (string, error) { return "main", nil }, + }) + defer restore() + + require.NoError(t, stack.Save(gitDir, &stack.StackFile{SchemaVersion: 1, Stacks: []stack.Stack{}})) + + cfg, outR, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{ + {ID: 70, PullRequests: []int{10, 11}}, + }, nil + }, + FindPRByNumberFn: func(number int) (*github.PullRequest, error) { + prs := map[int]*github.PullRequest{ + 10: {ID: "PR_10", Number: 10, HeadRefName: "feat-1", BaseRefName: "main", Merged: true, State: "MERGED", URL: "https://github.com/o/r/pull/10"}, + 11: {ID: "PR_11", Number: 11, HeadRefName: "feat-2", BaseRefName: "feat-1", Merged: true, State: "MERGED", URL: "https://github.com/o/r/pull/11"}, + } + return prs[number], nil + }, + } + + err := runCheckout(cfg, &checkoutOptions{target: "11"}) + output := collectOutput(cfg, outR, errR) + + assert.ErrorIs(t, err, ErrSilent) + assert.Contains(t, output, "All PRs in this stack have been merged") + assert.Contains(t, output, "gh stack init") +} + func TestCheckout_NumericTarget_APIError(t *testing.T) { gitDir := t.TempDir() restore := git.SetOps(&git.MockOps{ diff --git a/cmd/merge.go b/cmd/merge.go index b882551..8a298e2 100644 --- a/cmd/merge.go +++ b/cmd/merge.go @@ -49,7 +49,7 @@ func runMerge(cfg *config.Config, target string) error { // Resolve which branch to operate on. var br *stack.BranchRef if target != "" { - _, br, err = resolvePR(result.StackFile, target) + _, br, err = resolvePR(cfg, result.StackFile, target) if err != nil { cfg.Errorf("%s", err) return ErrNotInStack diff --git a/cmd/utils.go b/cmd/utils.go index 60cb0da..f6cf0cc 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -319,7 +319,7 @@ func activeBranchNames(s *stack.Stack) []string { // resolvePR resolves a user-provided target to a stack and branch using // waterfall logic: PR URL → PR number → branch name. -func resolvePR(sf *stack.StackFile, target string) (*stack.Stack, *stack.BranchRef, error) { +func resolvePR(cfg *config.Config, sf *stack.StackFile, target string) (*stack.Stack, *stack.BranchRef, error) { // Try parsing as a GitHub PR URL (e.g. https://github.com/owner/repo/pull/42). if prNumber, ok := parsePRURL(target); ok { s, b := sf.FindStackByPRNumber(prNumber) @@ -352,9 +352,9 @@ func resolvePR(sf *stack.StackFile, target string) (*stack.Stack, *stack.BranchR return nil, nil, fmt.Errorf( "no locally tracked stack found for %q\n"+ - "This command currently only works with stacks created locally.\n"+ - "Server-side stack discovery will be available in a future release.", + "To pull down a stack from remote, use the PR number: `%s`", target, + cfg.ColorCyan("gh stack checkout "), ) } diff --git a/cmd/utils_test.go b/cmd/utils_test.go index 0d0005f..8090132 100644 --- a/cmd/utils_test.go +++ b/cmd/utils_test.go @@ -156,7 +156,8 @@ func TestResolvePR_ByPRNumber(t *testing.T) { }, } - s, br, err := resolvePR(sf, "42") + cfg, _, _ := config.NewTestConfig() + s, br, err := resolvePR(cfg, sf, "42") assert.NoError(t, err) assert.Equal(t, "feat-1", br.Branch) assert.Equal(t, 42, br.PullRequest.Number) @@ -176,7 +177,8 @@ func TestResolvePR_ByPRURL(t *testing.T) { }, } - s, br, err := resolvePR(sf, "https://github.com/o/r/pull/42") + cfg, _, _ := config.NewTestConfig() + s, br, err := resolvePR(cfg, sf, "https://github.com/o/r/pull/42") assert.NoError(t, err) assert.Equal(t, "feat-1", br.Branch) assert.Equal(t, "main", s.Trunk.Branch) @@ -196,7 +198,8 @@ func TestResolvePR_ByBranchName(t *testing.T) { }, } - s, br, err := resolvePR(sf, "feat-2") + cfg, _, _ := config.NewTestConfig() + s, br, err := resolvePR(cfg, sf, "feat-2") assert.NoError(t, err) assert.Equal(t, "feat-2", br.Branch) assert.Equal(t, 43, br.PullRequest.Number) @@ -214,7 +217,8 @@ func TestResolvePR_NotFound(t *testing.T) { }, } - _, _, err := resolvePR(sf, "nonexistent") + cfg, _, _ := config.NewTestConfig() + _, _, err := resolvePR(cfg, sf, "nonexistent") assert.Error(t, err) assert.Contains(t, err.Error(), "no locally tracked stack found") } @@ -234,7 +238,8 @@ func TestResolvePR_URLPrecedesNumber(t *testing.T) { }, } - _, br, err := resolvePR(sf, "https://github.com/o/r/pull/99") + cfg, _, _ := config.NewTestConfig() + _, br, err := resolvePR(cfg, sf, "https://github.com/o/r/pull/99") assert.NoError(t, err) assert.Equal(t, 99, br.PullRequest.Number) }