diff --git a/README.md b/README.md index 4cdcec9..a4faa6b 100644 --- a/README.md +++ b/README.md @@ -158,7 +158,7 @@ A bare number is interpreted first as a stack or PR number (repo-scoped identifi When a remote stack is referenced, the command fetches the stack on GitHub, pulls the branches, and sets up the stack locally. If the stack already exists locally and matches, it switches to the branch. If the local and remote stacks have different compositions, you'll be prompted to resolve the conflict. -When a branch name is provided, the command resolves it against locally tracked stacks only. +When a branch name is provided, the command checks locally tracked stacks first. If the branch is not tracked locally, it looks for the branch on remote stacks and pulls down the matching stack. If more than one stack matches, use a stack or PR number to choose one explicitly. When run without arguments in an interactive terminal, first checks whether the current branch belongs to a stack on remote that is not tracked locally, and offers to check it out. If there is no unique match or you decline, it opens a searchable picker listing every stack available to you — both the stacks tracked locally and the stacks that exist only on GitHub. Each row shows the stack number, its bottom and top branch, base branch, a status bar summarizing how many of its pull requests are merged, open, closed, or not yet pushed, and whether the stack is available locally or only on the remote. Filter with the All / Local / Remote tabs or type `/` to search; fully merged stacks are omitted. Selecting a remote-only stack clones it locally before switching to it. @@ -174,7 +174,7 @@ gh stack checkout 42 # Check out a stack by PR URL gh stack checkout https://github.com/owner/repo/pull/42 -# Check out a stack by branch name (local only) +# Check out a stack by branch name gh stack checkout feature-auth # Interactive — pick from all available stacks (local and remote) diff --git a/cmd/checkout.go b/cmd/checkout.go index b5fbe73..c15f2d6 100644 --- a/cmd/checkout.go +++ b/cmd/checkout.go @@ -41,8 +41,9 @@ GitHub API to discover the stack, fetches the branches, and sets up the stack locally. If the stack already exists locally and matches, it simply switches to the branch. -When a branch name is provided, the command resolves it against -locally tracked stacks only. +When a branch name is provided, the command first checks locally tracked +stacks. If the branch is not tracked locally, it looks for the branch on +remote stacks and pulls down the matching stack. When run without arguments, first checks whether the current branch belongs to a stack on remote that is not tracked locally, and offers to check @@ -79,7 +80,7 @@ omitted.`, // runCheckout resolves a stack and checks out the target branch. // For numeric targets, it tries local lookup first, then falls back to // the GitHub API to discover remote stacks, then tries as a branch name. -// Non-numeric targets use local resolution only. +// Branch names resolve locally first and then against stacks on GitHub. func runCheckout(cfg *config.Config, opts *checkoutOptions) error { gitDir, err := git.GitDir() if err != nil { @@ -125,14 +126,23 @@ func runCheckout(cfg *config.Config, opts *checkoutOptions) error { return err } } else { - // Non-numeric target — resolve against local stacks only + // Non-numeric target — resolve locally before checking GitHub. var br *stack.BranchRef s, br, err = resolvePR(cfg, sf, opts.target) - if err != nil { - cfg.Errorf("%s", err) - return ErrNotInStack + if err == nil { + targetBranch = br.Branch + } else { + s, targetBranch, err = checkoutRemoteStackByBranch(cfg, sf, gitDir, opts.target) + if errors.Is(err, errRemoteBranchNotFound) { + cfg.Errorf("no local or remote stack found for %q", opts.target) + cfg.Printf("Try a stack or PR number with `%s`", + cfg.ColorCyan("gh stack checkout ")) + return ErrNotInStack + } + if err != nil { + return err + } } - targetBranch = br.Branch } currentBranch, _ := git.CurrentBranch() @@ -188,7 +198,7 @@ func resolveNumericTarget(cfg *config.Config, sf *stack.StackFile, gitDir string // attempt — the user might have a numeric branch name. remoteErr := err - // 4. Fall back to branch name lookup (handles numeric branch names). + // 4. Fall back to local branch name lookup (handles numeric branch names). stacks := sf.FindAllStacksForBranch(raw) if len(stacks) > 0 { s := stacks[0] @@ -202,11 +212,77 @@ func resolveNumericTarget(cfg *config.Config, sf *stack.StackFile, gitDir string } } + // Only a definitive "not in a stack" result can be reinterpreted as a + // remote branch name. Preserve API, conflict, and other actionable errors. + if !errors.Is(remoteErr, ErrNotInStack) { + return nil, "", remoteErr + } + + // Finally, try the numeric input as a branch in a stack on GitHub. + s, targetBranch, err = checkoutRemoteStackByBranch(cfg, sf, gitDir, raw) + if err == nil { + return s, targetBranch, nil + } + if !errors.Is(err, errRemoteBranchNotFound) { + return nil, "", err + } + // Nothing worked — return the remote error which has the most // informative message for a numeric input + cfg.Errorf("PR #%d is not part of a stack on GitHub", number) return nil, "", remoteErr } +var errRemoteBranchNotFound = errors.New("remote branch not found in a stack") + +// checkoutRemoteStackByBranch finds the unique active stack on GitHub that +// contains branch, then imports it through the existing PR checkout path. +func checkoutRemoteStackByBranch(cfg *config.Config, sf *stack.StackFile, gitDir, branch string) (*stack.Stack, string, error) { + client, err := cfg.GitHubClient() + if err != nil { + cfg.Errorf("failed to create GitHub client: %s", err) + return nil, "", ErrAPIFailure + } + + remoteStacks, err := client.ListStacks() + if err != nil { + var httpErr *api.HTTPError + if errors.As(err, &httpErr) && httpErr.StatusCode == 404 { + warnStacksUnavailable(cfg) + return nil, "", ErrStacksUnavailable + } + cfg.Errorf("failed to list stacks: %v", err) + return nil, "", ErrAPIFailure + } + + matches := matchingRemoteStacksForBranch(remoteStacks, branch) + if len(matches) == 0 { + return nil, "", errRemoteBranchNotFound + } + if len(matches) > 1 { + stackNumbers := make([]string, len(matches)) + for i, match := range matches { + stackNumbers[i] = strconv.Itoa(match.Number) + } + cfg.Errorf("branch %q belongs to multiple stacks on GitHub (%s)", + branch, strings.Join(stackNumbers, ", ")) + cfg.Printf("Use `%s` with a stack or PR number to choose one", + cfg.ColorCyan("gh stack checkout ")) + return nil, "", ErrDisambiguate + } + + for _, pr := range matches[0].PRDetails { + if pr.Head.Ref == branch && pr.Number > 0 { + s, targetBranch, err := checkoutRemoteStack(cfg, sf, gitDir, pr.Number) + if errors.Is(err, ErrNotInStack) { + cfg.Errorf("PR #%d is not part of a stack on GitHub", pr.Number) + } + return s, targetBranch, err + } + } + return nil, "", errRemoteBranchNotFound +} + // checkoutRemoteStack discovers a stack from GitHub for the given PR number, // reconciles it with any local state, and returns the resolved stack and // target branch name. The stack file is saved before returning. @@ -224,13 +300,12 @@ func checkoutRemoteStack(cfg *config.Config, sf *stack.StackFile, gitDir string, var httpErr *api.HTTPError if errors.As(err, &httpErr) && httpErr.StatusCode == 404 { warnStacksUnavailable(cfg) - return nil, "", ErrAPIFailure + return nil, "", ErrStacksUnavailable } cfg.Errorf("failed to list stacks: %v", err) return nil, "", ErrAPIFailure } if remoteStack == nil { - cfg.Errorf("PR #%d is not part of a stack on GitHub", prNumber) return nil, "", ErrNotInStack } diff --git a/cmd/checkout_test.go b/cmd/checkout_test.go index 1cfb808..f82c1f8 100644 --- a/cmd/checkout_test.go +++ b/cmd/checkout_test.go @@ -35,6 +35,12 @@ func TestCheckout_ByBranchName(t *testing.T) { }) cfg, outR, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + ListStacksFn: func() ([]github.RemoteStack, error) { + t.Fatal("local branch lookup should not call GitHub") + return nil, nil + }, + } err := runCheckout(cfg, &checkoutOptions{target: "b2"}) output := collectOutput(cfg, outR, errR) @@ -43,6 +49,101 @@ func TestCheckout_ByBranchName(t *testing.T) { assert.Contains(t, output, "Switched to b2") } +func TestCheckout_ByRemoteBranchName(t *testing.T) { + tests := []struct { + name string + target string + }{ + {name: "named branch", target: "feature"}, + {name: "numeric branch", target: "999"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gitDir := t.TempDir() + var checkedOut string + var createdBranches []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(string) error { return nil }, + CreateBranchFn: func(name, _ string) error { + createdBranches = append(createdBranches, name) + return nil + }, + SetUpstreamTrackingFn: func(string, string) error { return nil }, + ResolveRemoteFn: func(string) (string, error) { return "origin", nil }, + CheckoutBranchFn: func(name string) error { + checkedOut = name + return nil + }, + RevParseFn: func(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{}})) + + remoteStack := &github.RemoteStack{ + ID: 42, + Number: 7, + PullRequests: []int{10, 11, 12}, + PRDetails: []github.RemoteStackPR{ + {Number: 10, State: "open", Head: github.RemoteStackPRHead{Ref: "base-layer"}}, + {Number: 11, State: "open", Head: github.RemoteStackPRHead{Ref: tt.target}}, + {Number: 12, State: "open", Head: github.RemoteStackPRHead{Ref: "top-layer"}}, + }, + } + var lookedUpPRs []int + cfg, outR, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{*remoteStack}, nil + }, + FindStackForPRFn: func(number int) (*github.RemoteStack, error) { + lookedUpPRs = append(lookedUpPRs, number) + if number == 11 { + return remoteStack, nil + } + return nil, nil + }, + FindPRByNumberFn: func(number int) (*github.PullRequest, error) { + prs := map[int]*github.PullRequest{ + 10: {ID: "PR_10", Number: 10, HeadRefName: "base-layer", BaseRefName: "main"}, + 11: {ID: "PR_11", Number: 11, HeadRefName: tt.target, BaseRefName: "base-layer"}, + 12: {ID: "PR_12", Number: 12, HeadRefName: "top-layer", BaseRefName: tt.target}, + } + return prs[number], nil + }, + } + + err := runCheckout(cfg, &checkoutOptions{target: tt.target}) + output := collectOutput(cfg, outR, errR) + + require.NoError(t, err) + assert.Contains(t, lookedUpPRs, 11) + assert.Equal(t, []string{"base-layer", tt.target, "top-layer"}, createdBranches) + assert.Equal(t, tt.target, checkedOut, "should check out the requested layer, not the top branch") + assert.Contains(t, output, "Imported stack with 3 branches") + assert.Contains(t, output, "Switched to "+tt.target) + assert.NotContains(t, output, "not part of a stack") + + sf, loadErr := stack.Load(gitDir) + require.NoError(t, loadErr) + require.Len(t, sf.Stacks, 1) + assert.Equal(t, []string{"base-layer", tt.target, "top-layer"}, sf.Stacks[0].BranchNames()) + }) + } +} + func TestCheckout_ByPRNumber_Local(t *testing.T) { // When a PR number exists locally, no API call should be made gitDir := t.TempDir() @@ -138,11 +239,112 @@ func TestCheckout_BranchNotFound(t *testing.T) { }) cfg, outR, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + ListStacksFn: func() ([]github.RemoteStack, error) { + return nil, nil + }, + } err := runCheckout(cfg, &checkoutOptions{target: "nonexistent"}) output := collectOutput(cfg, outR, errR) assert.ErrorIs(t, err, ErrNotInStack) - assert.Contains(t, output, "no locally tracked stack found") + assert.Contains(t, output, `no local or remote stack found for "nonexistent"`) +} + +func TestCheckout_RemoteBranchName_MultipleStacks(t *testing.T) { + gitDir := t.TempDir() + checkoutCalled := false + restore := git.SetOps(&git.MockOps{ + GitDirFn: func() (string, error) { return gitDir, nil }, + CurrentBranchFn: func() (string, error) { return "main", nil }, + CheckoutBranchFn: func(string) error { + checkoutCalled = true + return 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{ + { + Number: 7, + PRDetails: []github.RemoteStackPR{ + {Number: 10, State: "open", Head: github.RemoteStackPRHead{Ref: "feature"}}, + }, + }, + { + Number: 8, + PRDetails: []github.RemoteStackPR{ + {Number: 11, State: "open", Head: github.RemoteStackPRHead{Ref: "feature"}}, + }, + }, + }, nil + }, + FindStackForPRFn: func(int) (*github.RemoteStack, error) { + t.Fatal("ambiguous branch should not select a stack") + return nil, nil + }, + } + + err := runCheckout(cfg, &checkoutOptions{target: "feature"}) + output := collectOutput(cfg, outR, errR) + + assert.ErrorIs(t, err, ErrDisambiguate) + assert.False(t, checkoutCalled) + assert.Contains(t, output, `branch "feature" belongs to multiple stacks on GitHub (7, 8)`) + assert.Contains(t, output, "stack or PR number") +} + +func TestCheckout_RemoteBranchName_APIError(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 nil, fmt.Errorf("network error") + }, + } + + err := runCheckout(cfg, &checkoutOptions{target: "feature"}) + output := collectOutput(cfg, outR, errR) + + assert.ErrorIs(t, err, ErrAPIFailure) + assert.Contains(t, output, "failed to list stacks: network error") +} + +func TestCheckout_RemoteBranchName_StacksUnavailable(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 nil, &api.HTTPError{StatusCode: 404, Message: "Not Found"} + }, + } + + err := runCheckout(cfg, &checkoutOptions{target: "feature"}) + output := collectOutput(cfg, outR, errR) + + assert.ErrorIs(t, err, ErrStacksUnavailable) + assert.Contains(t, output, "not enabled") } // --- Remote checkout tests (numeric target, local miss → API fallback) --- @@ -168,7 +370,7 @@ func TestCheckout_NumericTarget_StacksNotAvailable(t *testing.T) { err := runCheckout(cfg, &checkoutOptions{target: "123"}) output := collectOutput(cfg, outR, errR) - assert.ErrorIs(t, err, ErrAPIFailure) + assert.ErrorIs(t, err, ErrStacksUnavailable) assert.Contains(t, output, "not enabled") } @@ -613,6 +815,7 @@ func TestCheckout_NumericTarget_FallbackToBranchName(t *testing.T) { require.NoError(t, err) assert.Equal(t, "999", checkedOut) assert.Contains(t, output, "Switched to 999") + assert.NotContains(t, output, "not part of a stack") } func TestCheckout_NumericTarget_CompositionMismatch_NonInteractive(t *testing.T) { diff --git a/docs/src/content/docs/reference/cli.md b/docs/src/content/docs/reference/cli.md index d7e6d22..2438df7 100644 --- a/docs/src/content/docs/reference/cli.md +++ b/docs/src/content/docs/reference/cli.md @@ -133,7 +133,7 @@ A bare number is interpreted first as a stack or PR number (repo-scoped identifi When a remote stack is referenced, the command fetches the stack on GitHub, pulls the branches, and sets up the stack locally. If the stack already exists locally and matches, it switches to the branch. If the local and remote stacks have different compositions, you'll be prompted to resolve the conflict. -When a branch name is provided, the command resolves it against locally tracked stacks only. +When a branch name is provided, the command checks locally tracked stacks first. If the branch is not tracked locally, it looks for the branch on remote stacks and pulls down the matching stack. If more than one stack matches, use a stack or PR number to choose one explicitly. When run without arguments in an interactive terminal, first checks whether the current branch belongs to a stack on remote that is not tracked locally, and offers to check it out. If there is no unique match or you decline, it opens a searchable picker listing every stack available to you — both the stacks tracked locally and the stacks that exist only on GitHub. Each row shows the stack number, its bottom and top branch, base branch, a status bar summarizing how many of its pull requests are merged, open, closed, or not yet pushed, and whether the stack is available locally or only on the remote. Filter with the All / Local / Remote tabs or type `/` to search; fully merged stacks are omitted. Selecting a remote-only stack clones it locally before switching to it. @@ -149,7 +149,7 @@ gh stack checkout 42 # Check out a stack by PR URL gh stack checkout https://github.com/owner/repo/pull/42 -# Check out a stack by branch name (local only) +# Check out a stack by branch name gh stack checkout feature-auth # Interactive — pick from all available stacks (local and remote) diff --git a/skills/gh-stack/SKILL.md b/skills/gh-stack/SKILL.md index 96aab64..bb75496 100644 --- a/skills/gh-stack/SKILL.md +++ b/skills/gh-stack/SKILL.md @@ -165,8 +165,6 @@ an ancestor of the branch. - There is no non-interactive reorder or removal. Errors may suggest `gh stack modify`, but it is TUI-only — restructure with `unstack` then `init` instead. - PR titles and bodies are auto-generated. Use `gh pr edit` afterwards to change them. -- `checkout ` resolves against local stacks only. Use a stack or PR number to pull a - stack down from GitHub. ## More detail diff --git a/skills/gh-stack/references/commands.md b/skills/gh-stack/references/commands.md index 46e5833..fcb219a 100644 --- a/skills/gh-stack/references/commands.md +++ b/skills/gh-stack/references/commands.md @@ -136,8 +136,6 @@ Accepts a stack number, PR number, PR URL, or branch name. - A bare number resolves as a **stack number first**, then a PR number, then a branch name. - Stack numbers, PR numbers, and PR URLs fetch from GitHub, pull the branches down, and set the stack up locally. -- A **branch name resolves against locally tracked stacks only** and never contacts GitHub. Use a - stack or PR number to pull a stack that is not tracked locally. - If a local stack already exists over those branches with a different composition, `checkout` cannot be forced past it. Run `gh stack unstack --local` first, then retry. - `checkout` has no flags. It relies on `remote.pushDefault` when several remotes exist.