diff --git a/README.md b/README.md index baa33d0..da524af 100644 --- a/README.md +++ b/README.md @@ -307,6 +307,42 @@ gh stack submit --auto gh stack submit --draft ``` +### `gh stack link` + +Link PRs into a stack on GitHub without local tracking. + +``` +gh stack link [flags] [...] +``` + +Creates or updates a stack on GitHub from branch names or PR numbers. This command does not store or modify any `gh stack` local tracking state. It is designed for users who manage branches with other tools locally (e.g., jj, Sapling, git-town) and want to simply open a stack of PRs. + +Arguments are provided in stack order (bottom to top). Branch arguments are automatically pushed to the remote before creating or looking up PRs. For branches that already have open PRs, those PRs are used. For branches without PRs, new PRs are created automatically with the correct base branch chaining. Existing PRs whose base branch doesn't match the expected chain are corrected automatically. + +If the PRs are not yet in a stack, a new stack is created. If some of the PRs are already in a stack, the existing stack is updated to include the new PRs. Existing PRs are never removed from a stack — the update is additive only. + +| Flag | Description | +|------|-------------| +| `--base ` | Base branch for the bottom of the stack (default: `main`) | +| `--draft` | Create new PRs as drafts | +| `--remote ` | Remote to push to (defaults to auto-detected remote) | + +**Examples:** + +```sh +# Link branches into a stack (pushes branches, creates PRs, creates stack) +gh stack link feature-auth feature-api feature-ui + +# Link existing PRs by number +gh stack link 10 20 30 + +# Add branches to an existing stack of PRs +gh stack link 42 43 feature-auth feature-ui + +# Use a different base branch and create PRs as drafts +gh stack link --base develop --draft feat-a feat-b feat-c +``` + ### `gh stack view` View the current stack. diff --git a/cmd/link.go b/cmd/link.go new file mode 100644 index 0000000..fdb9907 --- /dev/null +++ b/cmd/link.go @@ -0,0 +1,524 @@ +package cmd + +import ( + "errors" + "fmt" + "strconv" + + "github.com/cli/go-gh/v2/pkg/api" + "github.com/github/gh-stack/internal/config" + "github.com/github/gh-stack/internal/git" + "github.com/github/gh-stack/internal/github" + "github.com/spf13/cobra" +) + +type linkOptions struct { + base string + draft bool + remote string +} + +func LinkCmd(cfg *config.Config) *cobra.Command { + opts := &linkOptions{} + + cmd := &cobra.Command{ + Use: "link [...]", + Short: "Link PRs into a stack on GitHub without local tracking", + Long: `Create or update a stack on GitHub from branch names or PR numbers. + +This command does not rely on gh-stack local tracking state. It is +designed for users who manage branches with external tools (e.g. jj) +and want to use GitHub stacked PRs without adopting local stack +tracking. + +Arguments are provided in stack order (bottom to top). Each argument +can be a branch name or a PR number. For numeric arguments, the +command first checks if a PR with that number exists; if not, it +treats the argument as a branch name. + +Branch arguments are automatically pushed to the remote before +creating or looking up PRs. For branches that already have open PRs, +those PRs are used. For branches without PRs, new PRs are created +automatically with the correct base branch chaining. + +If the PRs are not yet in a stack, a new stack is created. If some of +the PRs are already in a stack, the existing stack is updated to include +the new PRs (existing PRs are never removed).`, + Args: cobra.MinimumNArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + return runLink(cfg, opts, args) + }, + } + + cmd.Flags().StringVar(&opts.base, "base", "main", "Base branch for the bottom of the stack") + cmd.Flags().BoolVar(&opts.draft, "draft", false, "Create new PRs as drafts") + cmd.Flags().StringVar(&opts.remote, "remote", "", "Remote to push to (defaults to auto-detected remote)") + + return cmd +} + +// resolvedArg holds the result of resolving a single CLI argument to a PR. +type resolvedArg struct { + branch string // head branch name + prNumber int // PR number + prURL string // PR URL (for display) + created bool // true if we created this PR (skip base-fix re-fetch) +} + +func runLink(cfg *config.Config, opts *linkOptions, args []string) error { + if err := validateArgs(args); err != nil { + cfg.Errorf("%s", err) + return ErrInvalidArgs + } + + client, err := cfg.GitHubClient() + if err != nil { + cfg.Errorf("failed to create GitHub client: %s", err) + return ErrAPIFailure + } + + // Phase 1: Push branch args to the remote so PRs can be found/created + if err := pushBranchArgs(cfg, opts, args); err != nil { + return err + } + + // Phase 2: Find existing PRs for all args (don't create yet) + found, err := findExistingPRs(cfg, client, args) + if err != nil { + return err + } + + // Phase 3: Pre-validate the stack — check that adding these PRs won't + // conflict with existing stacks before creating any new PRs. + // Also fetches stacks for reuse in the upsert phase. + knownPRNumbers := make([]int, 0, len(found)) + for _, r := range found { + if r != nil { + knownPRNumbers = append(knownPRNumbers, r.prNumber) + } + } + stacks, err := listStacksSafe(cfg, client) + if err != nil { + return err + } + if len(knownPRNumbers) > 0 { + if err := prevalidateStack(cfg, stacks, knownPRNumbers); err != nil { + return err + } + } + + // Phase 4: Create PRs for branches that don't have one yet + resolved, err := createMissingPRs(cfg, client, opts, args, found) + if err != nil { + return err + } + + // Phase 5: Fix base branches for existing PRs with wrong bases + fixBaseBranches(cfg, client, opts, resolved) + + // Phase 6: Upsert the stack (reuse stacks from phase 3) + prNumbers := make([]int, len(resolved)) + for i, r := range resolved { + prNumbers[i] = r.prNumber + } + + return upsertStack(cfg, client, stacks, prNumbers) +} + +// pushBranchArgs pushes all arguments that correspond to local branches +// to the remote. This ensures branches exist on the server before we try +// to create or look up PRs. Args that are pure PR numbers (not local +// branch names) are skipped. +func pushBranchArgs(cfg *config.Config, opts *linkOptions, args []string) error { + var branches []string + for _, arg := range args { + if git.BranchExists(arg) { + branches = append(branches, arg) + } + } + + if len(branches) == 0 { + return nil + } + + // Resolve the remote using the first branch as context + remote, err := pickRemote(cfg, branches[0], opts.remote) + if err != nil { + if !errors.Is(err, errInterrupt) { + cfg.Errorf("%s", err) + } + return ErrSilent + } + + cfg.Printf("Pushing %d %s to %s...", len(branches), plural(len(branches), "branch", "branches"), remote) + if err := git.Push(remote, branches, false, true); err != nil { + cfg.Errorf("failed to push branches: %s", err) + return ErrSilent + } + + return nil +} + +// validateArgs checks for duplicates in the arg list. +func validateArgs(args []string) error { + seen := make(map[string]bool, len(args)) + for _, arg := range args { + if seen[arg] { + return fmt.Errorf("duplicate argument: %q", arg) + } + seen[arg] = true + } + return nil +} + +// findExistingPRs looks up existing PRs for each arg without creating any. +// Returns a slice parallel to args where each entry is either a resolved PR +// or nil (meaning the branch has no PR yet and one needs to be created). +func findExistingPRs(cfg *config.Config, client github.ClientOps, args []string) ([]*resolvedArg, error) { + found := make([]*resolvedArg, len(args)) + + for i, arg := range args { + r, err := findExistingPR(cfg, client, arg) + if err != nil { + return nil, err + } + if r != nil { + // Check for duplicate PR numbers + for j := 0; j < i; j++ { + if found[j] != nil && found[j].prNumber == r.prNumber { + cfg.Errorf("arguments %q and %q resolve to the same PR #%d", found[j].branch, r.branch, r.prNumber) + return nil, ErrInvalidArgs + } + } + } + found[i] = r + } + + return found, nil +} + +// findExistingPR looks up an existing PR for a single arg. +// Returns nil if the arg is a branch with no open PR. +func findExistingPR(cfg *config.Config, client github.ClientOps, arg string) (*resolvedArg, error) { + // If numeric, try as PR number first + if n, err := strconv.Atoi(arg); err == nil && n > 0 { + pr, err := client.FindPRByNumber(n) + if err != nil { + cfg.Errorf("failed to look up PR #%d: %v", n, err) + return nil, ErrAPIFailure + } + if pr != nil { + return &resolvedArg{ + branch: pr.HeadRefName, + prNumber: pr.Number, + prURL: pr.URL, + }, nil + } + // PR doesn't exist — fall through to branch name lookup + } + + // Treat as branch name: look for an open PR + pr, err := client.FindPRForBranch(arg) + if err != nil { + cfg.Errorf("failed to look up PR for branch %s: %v", arg, err) + return nil, ErrAPIFailure + } + if pr != nil { + cfg.Printf("Found PR %s for branch %s", cfg.PRLink(pr.Number, pr.URL), arg) + return &resolvedArg{ + branch: arg, + prNumber: pr.Number, + prURL: pr.URL, + }, nil + } + + return nil, nil // needs PR creation +} + +// listStacksSafe fetches all stacks, handling the 404 "not enabled" case. +func listStacksSafe(cfg *config.Config, client github.ClientOps) ([]github.RemoteStack, error) { + stacks, err := client.ListStacks() + if err != nil { + var httpErr *api.HTTPError + if errors.As(err, &httpErr) && httpErr.StatusCode == 404 { + cfg.Warningf("Stacked PRs are not enabled for this repository") + return nil, ErrStacksUnavailable + } + cfg.Errorf("failed to list stacks: %v", err) + return nil, ErrAPIFailure + } + return stacks, nil +} + +// prevalidateStack checks whether the known PRs would conflict with +// existing stacks. This runs before creating new PRs so we can fail +// early without leaving orphaned PRs. +func prevalidateStack(cfg *config.Config, stacks []github.RemoteStack, knownPRNumbers []int) error { + matchedStack, err := findMatchingStack(stacks, knownPRNumbers) + if err != nil { + cfg.Errorf("%s", err) + return ErrDisambiguate + } + + if matchedStack != nil { + // Check that we won't be removing PRs from the existing stack. + // At this point we only have the known PR numbers (existing PRs). + // New PRs will be created later and added. Since new PRs can't + // match existing stack PRs (they don't exist yet), we just need + // to check that all existing stack PRs are in the known set. + knownSet := make(map[int]bool, len(knownPRNumbers)) + for _, n := range knownPRNumbers { + knownSet[n] = true + } + + var dropped []int + for _, n := range matchedStack.PullRequests { + if !knownSet[n] { + dropped = append(dropped, n) + } + } + + if len(dropped) > 0 { + cfg.Errorf("Cannot update stack: this would remove %s from the stack", + formatPRList(dropped)) + cfg.Printf("Current stack: %s", formatPRList(matchedStack.PullRequests)) + cfg.Printf("Include all existing PRs in the command to update the stack") + return ErrInvalidArgs + } + } + + return nil +} + +// createMissingPRs creates PRs for args that don't have one yet. +// Returns the fully resolved list with all args mapped to PRs. +func createMissingPRs(cfg *config.Config, client github.ClientOps, opts *linkOptions, args []string, found []*resolvedArg) ([]resolvedArg, error) { + resolved := make([]resolvedArg, len(args)) + + for i, arg := range args { + if found[i] != nil { + resolved[i] = *found[i] + continue + } + + // Determine the base branch for this PR + baseBranch := opts.base + if i > 0 { + baseBranch = resolved[i-1].branch + } + + title := humanize(arg) + body := generatePRBody("") + + newPR, err := client.CreatePR(baseBranch, arg, title, body, opts.draft) + if err != nil { + cfg.Errorf("failed to create PR for branch %s: %v", arg, err) + return nil, ErrAPIFailure + } + + cfg.Successf("Created PR %s for %s (base: %s)", cfg.PRLink(newPR.Number, newPR.URL), arg, baseBranch) + resolved[i] = resolvedArg{ + branch: arg, + prNumber: newPR.Number, + prURL: newPR.URL, + created: true, + } + } + + return resolved, nil +} + +// fixBaseBranches updates the base branch of existing PRs to match the +// expected stack chain. The first PR should have base = opts.base, +// each subsequent PR should have base = previous PR's head branch. +// Newly created PRs (created=true) are skipped since they already have +// the correct base from creation. +func fixBaseBranches(cfg *config.Config, client github.ClientOps, opts *linkOptions, resolved []resolvedArg) { + for i, r := range resolved { + if r.created { + continue + } + + expectedBase := opts.base + if i > 0 { + expectedBase = resolved[i-1].branch + } + + // Look up the PR to check its current base + pr, err := client.FindPRByNumber(r.prNumber) + if err != nil { + cfg.Warningf("could not verify base branch for PR %s: %v", + cfg.PRLink(r.prNumber, r.prURL), err) + continue + } + if pr == nil { + continue + } + + if pr.BaseRefName != expectedBase { + if err := client.UpdatePRBase(r.prNumber, expectedBase); err != nil { + cfg.Warningf("failed to update base branch for PR %s to %s: %s", + cfg.PRLink(r.prNumber, r.prURL), expectedBase, formatAPIError(err)) + } else { + cfg.Successf("Updated base branch for PR %s to %s", + cfg.PRLink(r.prNumber, r.prURL), expectedBase) + } + } + } +} + +// formatAPIError extracts a clean error message from an API error. +// For HTTP errors, returns just the status and message without the raw URL. +func formatAPIError(err error) string { + var httpErr *api.HTTPError + if errors.As(err, &httpErr) { + if httpErr.Message != "" { + return fmt.Sprintf("HTTP %d: %s", httpErr.StatusCode, httpErr.Message) + } + return fmt.Sprintf("HTTP %d", httpErr.StatusCode) + } + return err.Error() +} + +// upsertStack uses the pre-fetched stacks to create or update as needed. +func upsertStack(cfg *config.Config, client github.ClientOps, stacks []github.RemoteStack, prNumbers []int) error { + matchedStack, err := findMatchingStack(stacks, prNumbers) + if err != nil { + cfg.Errorf("%s", err) + return ErrDisambiguate + } + + if matchedStack == nil { + return createLink(cfg, client, prNumbers) + } + + return updateLink(cfg, client, matchedStack, prNumbers) +} + +// findMatchingStack finds a single stack that contains any of the given PR numbers. +// Returns nil if no stack matches. Returns an error if PRs span multiple stacks. +func findMatchingStack(stacks []github.RemoteStack, prNumbers []int) (*github.RemoteStack, error) { + prSet := make(map[int]bool, len(prNumbers)) + for _, n := range prNumbers { + prSet[n] = true + } + + var matched *github.RemoteStack + for i := range stacks { + for _, n := range stacks[i].PullRequests { + if prSet[n] { + if matched != nil && matched.ID != stacks[i].ID { + return nil, fmt.Errorf("PRs belong to multiple stacks — unstack them first, then re-link") + } + matched = &stacks[i] + break + } + } + } + + return matched, nil +} + +// createLink creates a new stack with the given PR numbers. +func createLink(cfg *config.Config, client github.ClientOps, prNumbers []int) error { + _, err := client.CreateStack(prNumbers) + if err != nil { + var httpErr *api.HTTPError + if errors.As(err, &httpErr) { + switch httpErr.StatusCode { + case 422: + cfg.Errorf("Cannot create stack: %s", httpErr.Message) + return ErrAPIFailure + case 404: + cfg.Warningf("Stacked PRs are not enabled for this repository") + return ErrStacksUnavailable + default: + cfg.Errorf("Failed to create stack (HTTP %d): %s", httpErr.StatusCode, httpErr.Message) + return ErrAPIFailure + } + } + cfg.Errorf("Failed to create stack: %v", err) + return ErrAPIFailure + } + + cfg.Successf("Created stack with %d PRs", len(prNumbers)) + return nil +} + +// updateLink updates an existing stack with the given PR numbers. +// The update is additive-only: it errors if any existing PRs would be removed. +func updateLink(cfg *config.Config, client github.ClientOps, existing *github.RemoteStack, prNumbers []int) error { + // Check if the input exactly matches the existing stack. + if slicesEqual(existing.PullRequests, prNumbers) { + cfg.Successf("Stack with %d PRs is already up to date", len(prNumbers)) + return nil + } + + // Check that no existing PRs would be removed (additive-only). + newSet := make(map[int]bool, len(prNumbers)) + for _, n := range prNumbers { + newSet[n] = true + } + + var dropped []int + for _, n := range existing.PullRequests { + if !newSet[n] { + dropped = append(dropped, n) + } + } + + if len(dropped) > 0 { + cfg.Errorf("Cannot update stack: this would remove %s from the stack", + formatPRList(dropped)) + cfg.Printf("Current stack: %s", formatPRList(existing.PullRequests)) + cfg.Printf("Include all existing PRs in the command to update the stack") + return ErrInvalidArgs + } + + stackID := strconv.Itoa(existing.ID) + if err := client.UpdateStack(stackID, prNumbers); err != nil { + var httpErr *api.HTTPError + if errors.As(err, &httpErr) { + switch httpErr.StatusCode { + case 404: + // Stack was deleted between list and update — try creating instead. + cfg.Warningf("Stack was deleted — creating a new one") + return createLink(cfg, client, prNumbers) + case 422: + cfg.Errorf("Cannot update stack: %s", httpErr.Message) + return ErrAPIFailure + default: + cfg.Errorf("Failed to update stack (HTTP %d): %s", httpErr.StatusCode, httpErr.Message) + return ErrAPIFailure + } + } + cfg.Errorf("Failed to update stack: %v", err) + return ErrAPIFailure + } + + cfg.Successf("Updated stack to %d PRs", len(prNumbers)) + return nil +} + +func slicesEqual(a, b []int) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + +func formatPRList(numbers []int) string { + if len(numbers) == 0 { + return "" + } + s := fmt.Sprintf("#%d", numbers[0]) + for _, n := range numbers[1:] { + s += fmt.Sprintf(", #%d", n) + } + return s +} diff --git a/cmd/link_test.go b/cmd/link_test.go new file mode 100644 index 0000000..48aaef9 --- /dev/null +++ b/cmd/link_test.go @@ -0,0 +1,1098 @@ +package cmd + +import ( + "fmt" + "io" + "testing" + + "github.com/cli/go-gh/v2/pkg/api" + "github.com/github/gh-stack/internal/config" + "github.com/github/gh-stack/internal/git" + "github.com/github/gh-stack/internal/github" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// newLinkGitMock creates a MockOps for link tests that involve branch args. +// BranchExists returns true for the given branches, Push is a no-op, +// and ResolveRemote returns "origin". +func newLinkGitMock(branches ...string) *git.MockOps { + branchSet := make(map[string]bool, len(branches)) + for _, b := range branches { + branchSet[b] = true + } + return &git.MockOps{ + BranchExistsFn: func(name string) bool { return branchSet[name] }, + PushFn: func(string, []string, bool, bool) error { return nil }, + ResolveRemoteFn: func(string) (string, error) { return "origin", nil }, + } +} + +// --- PR-number tests --- + +func TestLink_PRNumbers_CreateNewStack(t *testing.T) { + var createdPRs []int + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + return &github.PullRequest{ + Number: n, + HeadRefName: fmt.Sprintf("branch-%d", n), + BaseRefName: "main", + URL: fmt.Sprintf("https://github.com/o/r/pull/%d", n), + }, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{}, nil + }, + CreateStackFn: func(prNumbers []int) (int, error) { + createdPRs = prNumbers + return 42, nil + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"10", "20", "30"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Err.Close() + errOut, _ := io.ReadAll(errR) + output := string(errOut) + + assert.NoError(t, err) + assert.Equal(t, []int{10, 20, 30}, createdPRs) + assert.Contains(t, output, "Created stack with 3 PRs") +} + +func TestLink_PRNumbers_UpdateExistingStack(t *testing.T) { + var updatedID string + var updatedPRs []int + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + return &github.PullRequest{ + Number: n, + HeadRefName: fmt.Sprintf("branch-%d", n), + BaseRefName: "main", + URL: fmt.Sprintf("https://github.com/o/r/pull/%d", n), + }, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{ + {ID: 7, PullRequests: []int{10, 20}}, + }, nil + }, + UpdateStackFn: func(stackID string, prNumbers []int) error { + updatedID = stackID + updatedPRs = prNumbers + return nil + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"10", "20", "30"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Err.Close() + errOut, _ := io.ReadAll(errR) + output := string(errOut) + + assert.NoError(t, err) + assert.Equal(t, "7", updatedID) + assert.Equal(t, []int{10, 20, 30}, updatedPRs) + assert.Contains(t, output, "Updated stack to 3 PRs") +} + +func TestLink_PRNumbers_ExactMatch_NoOp(t *testing.T) { + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + return &github.PullRequest{ + Number: n, + HeadRefName: fmt.Sprintf("branch-%d", n), + BaseRefName: "main", + URL: fmt.Sprintf("https://github.com/o/r/pull/%d", n), + }, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{ + {ID: 7, PullRequests: []int{10, 20, 30}}, + }, nil + }, + UpdateStackFn: func(string, []int) error { + t.Fatal("UpdateStack should not be called for exact match") + return nil + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"10", "20", "30"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Err.Close() + errOut, _ := io.ReadAll(errR) + output := string(errOut) + + assert.NoError(t, err) + assert.Contains(t, output, "already up to date") +} + +func TestLink_PRNumbers_WouldRemovePRs(t *testing.T) { + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + return &github.PullRequest{ + Number: n, + HeadRefName: fmt.Sprintf("branch-%d", n), + BaseRefName: "main", + URL: fmt.Sprintf("https://github.com/o/r/pull/%d", n), + }, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{ + {ID: 7, PullRequests: []int{10, 20, 30}}, + }, nil + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"20", "30"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Err.Close() + errOut, _ := io.ReadAll(errR) + output := string(errOut) + + assert.ErrorIs(t, err, ErrInvalidArgs) + assert.Contains(t, output, "would remove") + assert.Contains(t, output, "#10") +} + +func TestLink_PRNumbers_MultipleStacks(t *testing.T) { + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + return &github.PullRequest{ + Number: n, + HeadRefName: fmt.Sprintf("branch-%d", n), + BaseRefName: "main", + }, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{ + {ID: 1, PullRequests: []int{10, 20}}, + {ID: 2, PullRequests: []int{30, 40}}, + }, nil + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"10", "30"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Err.Close() + errOut, _ := io.ReadAll(errR) + output := string(errOut) + + assert.ErrorIs(t, err, ErrDisambiguate) + assert.Contains(t, output, "multiple stacks") +} + +func TestLink_TooFewArgs(t *testing.T) { + cfg, _, _ := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{} + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"10"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + // cobra enforces MinimumNArgs(2) before RunE is called + assert.Error(t, err) +} + +func TestLink_DuplicateArgs(t *testing.T) { + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{} + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"feature-a", "feature-a"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Err.Close() + errOut, _ := io.ReadAll(errR) + output := string(errOut) + + assert.ErrorIs(t, err, ErrInvalidArgs) + assert.Contains(t, output, "duplicate argument") +} + +func TestLink_StacksUnavailable(t *testing.T) { + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + return &github.PullRequest{Number: n, HeadRefName: "b", BaseRefName: "main"}, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return nil, &api.HTTPError{StatusCode: 404, Message: "Not Found"} + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"10", "20"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Err.Close() + errOut, _ := io.ReadAll(errR) + output := string(errOut) + + assert.ErrorIs(t, err, ErrStacksUnavailable) + assert.Contains(t, output, "not enabled") +} + +func TestLink_Create422(t *testing.T) { + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + return &github.PullRequest{Number: n, HeadRefName: "b", BaseRefName: "main"}, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{}, nil + }, + CreateStackFn: func(prNumbers []int) (int, error) { + return 0, &api.HTTPError{ + StatusCode: 422, + Message: "Pull requests must form a stack", + } + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"10", "20"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Err.Close() + errOut, _ := io.ReadAll(errR) + output := string(errOut) + + assert.ErrorIs(t, err, ErrAPIFailure) + assert.Contains(t, output, "must form a stack") +} + +// --- Branch name tests --- + +func TestLink_BranchNames_AllHavePRs(t *testing.T) { + restore := git.SetOps(newLinkGitMock("feature-a", "feature-b")) + defer restore() + + var stackedPRs []int + prMap := map[string]*github.PullRequest{ + "feature-a": {Number: 10, HeadRefName: "feature-a", BaseRefName: "main", URL: "https://github.com/o/r/pull/10"}, + "feature-b": {Number: 20, HeadRefName: "feature-b", BaseRefName: "feature-a", URL: "https://github.com/o/r/pull/20"}, + } + + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRForBranchFn: func(branch string) (*github.PullRequest, error) { + return prMap[branch], nil + }, + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + for _, pr := range prMap { + if pr.Number == n { + return pr, nil + } + } + return nil, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{}, nil + }, + CreateStackFn: func(prNumbers []int) (int, error) { + stackedPRs = prNumbers + return 42, nil + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"feature-a", "feature-b"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Err.Close() + errOut, _ := io.ReadAll(errR) + output := string(errOut) + + assert.NoError(t, err) + assert.Equal(t, []int{10, 20}, stackedPRs) + assert.Contains(t, output, "Created stack with 2 PRs") +} + +func TestLink_BranchNames_CreatesMissingPRs(t *testing.T) { + restore := git.SetOps(newLinkGitMock("feature-a", "feature-b")) + defer restore() + + var createdPRs []struct{ base, head string } + var stackedPRs []int + + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRForBranchFn: func(branch string) (*github.PullRequest, error) { + if branch == "feature-a" { + return &github.PullRequest{ + Number: 10, HeadRefName: "feature-a", BaseRefName: "main", + URL: "https://github.com/o/r/pull/10", + }, nil + } + return nil, nil // feature-b has no PR + }, + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + if n == 10 { + return &github.PullRequest{ + Number: 10, HeadRefName: "feature-a", BaseRefName: "main", + URL: "https://github.com/o/r/pull/10", + }, nil + } + if n == 20 { + return &github.PullRequest{ + Number: 20, HeadRefName: "feature-b", BaseRefName: "feature-a", + URL: "https://github.com/o/r/pull/20", + }, nil + } + return nil, nil + }, + CreatePRFn: func(base, head, title, body string, draft bool) (*github.PullRequest, error) { + createdPRs = append(createdPRs, struct{ base, head string }{base, head}) + return &github.PullRequest{ + Number: 20, HeadRefName: head, BaseRefName: base, + URL: "https://github.com/o/r/pull/20", + }, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{}, nil + }, + CreateStackFn: func(prNumbers []int) (int, error) { + stackedPRs = prNumbers + return 42, nil + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"feature-a", "feature-b"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Err.Close() + errOut, _ := io.ReadAll(errR) + output := string(errOut) + + assert.NoError(t, err) + require.Len(t, createdPRs, 1) + assert.Equal(t, "feature-a", createdPRs[0].base) // base should chain to previous branch + assert.Equal(t, "feature-b", createdPRs[0].head) + assert.Equal(t, []int{10, 20}, stackedPRs) + assert.Contains(t, output, "Created PR") + assert.Contains(t, output, "Created stack with 2 PRs") +} + +func TestLink_BranchNames_AllNeedPRs(t *testing.T) { + restore := git.SetOps(newLinkGitMock("feat-a", "feat-b", "feat-c")) + defer restore() + + prCounter := 0 + var createdPRs []struct{ base, head string } + + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRForBranchFn: func(branch string) (*github.PullRequest, error) { + return nil, nil // no open PRs for any branch + }, + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + bases := map[int]string{1: "main", 2: "feat-a", 3: "feat-b"} + heads := map[int]string{1: "feat-a", 2: "feat-b", 3: "feat-c"} + if h, ok := heads[n]; ok { + return &github.PullRequest{ + Number: n, HeadRefName: h, BaseRefName: bases[n], + }, nil + } + return nil, nil + }, + CreatePRFn: func(base, head, title, body string, draft bool) (*github.PullRequest, error) { + prCounter++ + createdPRs = append(createdPRs, struct{ base, head string }{base, head}) + return &github.PullRequest{ + Number: prCounter, + HeadRefName: head, + BaseRefName: base, + URL: fmt.Sprintf("https://github.com/o/r/pull/%d", prCounter), + }, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{}, nil + }, + CreateStackFn: func(prNumbers []int) (int, error) { + return 42, nil + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"--base", "develop", "feat-a", "feat-b", "feat-c"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Err.Close() + errOut, _ := io.ReadAll(errR) + output := string(errOut) + + assert.NoError(t, err) + require.Len(t, createdPRs, 3) + // First PR base should be the --base flag value + assert.Equal(t, "develop", createdPRs[0].base) + assert.Equal(t, "feat-a", createdPRs[0].head) + // Second PR base should be previous branch + assert.Equal(t, "feat-a", createdPRs[1].base) + assert.Equal(t, "feat-b", createdPRs[1].head) + // Third PR base should be previous branch + assert.Equal(t, "feat-b", createdPRs[2].base) + assert.Equal(t, "feat-c", createdPRs[2].head) + assert.Contains(t, output, "Created stack with 3 PRs") +} + +func TestLink_BranchNames_DraftFlag(t *testing.T) { + restore := git.SetOps(newLinkGitMock("feat-a", "feat-b")) + defer restore() + + var createdDraft bool + prCounter := 0 + + cfg, _, _ := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRForBranchFn: func(branch string) (*github.PullRequest, error) { + return nil, nil + }, + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + heads := map[int]string{1: "feat-a", 2: "feat-b"} + bases := map[int]string{1: "main", 2: "feat-a"} + if h, ok := heads[n]; ok { + return &github.PullRequest{Number: n, HeadRefName: h, BaseRefName: bases[n]}, nil + } + return nil, nil + }, + CreatePRFn: func(base, head, title, body string, draft bool) (*github.PullRequest, error) { + createdDraft = draft + prCounter++ + return &github.PullRequest{ + Number: prCounter, HeadRefName: head, BaseRefName: base, + URL: fmt.Sprintf("https://github.com/o/r/pull/%d", prCounter), + }, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{}, nil + }, + CreateStackFn: func([]int) (int, error) { return 1, nil }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"--draft", "feat-a", "feat-b"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + assert.NoError(t, err) + assert.True(t, createdDraft, "PRs should be created as drafts when --draft is set") +} + +func TestLink_MixedArgs_PRNumberAndBranch(t *testing.T) { + restore := git.SetOps(newLinkGitMock("new-feature")) + defer restore() + + var stackedPRs []int + + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + if n == 42 { + return &github.PullRequest{ + Number: 42, HeadRefName: "existing-branch", BaseRefName: "main", + URL: "https://github.com/o/r/pull/42", + }, nil + } + return nil, nil + }, + FindPRForBranchFn: func(branch string) (*github.PullRequest, error) { + if branch == "new-feature" { + return &github.PullRequest{ + Number: 99, HeadRefName: "new-feature", BaseRefName: "existing-branch", + URL: "https://github.com/o/r/pull/99", + }, nil + } + return nil, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{}, nil + }, + CreateStackFn: func(prNumbers []int) (int, error) { + stackedPRs = prNumbers + return 42, nil + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"42", "new-feature"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Err.Close() + errOut, _ := io.ReadAll(errR) + output := string(errOut) + + assert.NoError(t, err) + assert.Equal(t, []int{42, 99}, stackedPRs) + assert.Contains(t, output, "Created stack with 2 PRs") +} + +func TestLink_NumericArg_PRNotFound_TreatedAsBranch(t *testing.T) { + // Numeric branches "123" and "456" exist locally + restore := git.SetOps(newLinkGitMock("123", "456")) + defer restore() + + var stackedPRs []int + + cfg, _, _ := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + return nil, nil // PR not found + }, + FindPRForBranchFn: func(branch string) (*github.PullRequest, error) { + // Treat "123" as a branch name + if branch == "123" { + return &github.PullRequest{ + Number: 50, HeadRefName: "123", BaseRefName: "main", + URL: "https://github.com/o/r/pull/50", + }, nil + } + if branch == "456" { + return &github.PullRequest{ + Number: 51, HeadRefName: "456", BaseRefName: "123", + URL: "https://github.com/o/r/pull/51", + }, nil + } + return nil, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{}, nil + }, + CreateStackFn: func(prNumbers []int) (int, error) { + stackedPRs = prNumbers + return 42, nil + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"123", "456"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + assert.NoError(t, err) + assert.Equal(t, []int{50, 51}, stackedPRs) +} + +func TestLink_FixesBaseBranches(t *testing.T) { + restore := git.SetOps(newLinkGitMock("feat-a", "feat-b")) + defer restore() + + var baseUpdates []struct { + number int + base string + } + + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRForBranchFn: func(branch string) (*github.PullRequest, error) { + switch branch { + case "feat-a": + return &github.PullRequest{ + Number: 10, HeadRefName: "feat-a", BaseRefName: "main", + URL: "https://github.com/o/r/pull/10", + }, nil + case "feat-b": + // This PR has the wrong base — should be feat-a, not main + return &github.PullRequest{ + Number: 20, HeadRefName: "feat-b", BaseRefName: "main", + URL: "https://github.com/o/r/pull/20", + }, nil + } + return nil, nil + }, + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + switch n { + case 10: + return &github.PullRequest{ + Number: 10, HeadRefName: "feat-a", BaseRefName: "main", + }, nil + case 20: + return &github.PullRequest{ + Number: 20, HeadRefName: "feat-b", BaseRefName: "main", + }, nil + } + return nil, nil + }, + UpdatePRBaseFn: func(number int, base string) error { + baseUpdates = append(baseUpdates, struct { + number int + base string + }{number, base}) + return nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{}, nil + }, + CreateStackFn: func([]int) (int, error) { return 42, nil }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"feat-a", "feat-b"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Err.Close() + errOut, _ := io.ReadAll(errR) + output := string(errOut) + + assert.NoError(t, err) + // PR #20's base should be updated from "main" to "feat-a" + require.Len(t, baseUpdates, 1) + assert.Equal(t, 20, baseUpdates[0].number) + assert.Equal(t, "feat-a", baseUpdates[0].base) + assert.Contains(t, output, "Updated base branch") +} + +func TestLink_DuplicateBranchResolvesToSamePR(t *testing.T) { + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRForBranchFn: func(branch string) (*github.PullRequest, error) { + return &github.PullRequest{ + Number: 10, HeadRefName: branch, BaseRefName: "main", + }, nil + }, + } + + // Different args that resolve to the same PR + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"feat-a", "feat-a"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Err.Close() + errOut, _ := io.ReadAll(errR) + output := string(errOut) + + assert.ErrorIs(t, err, ErrInvalidArgs) + assert.Contains(t, output, "duplicate argument") +} + +func TestLink_UpdateDeletedStack_FallsBackToCreate(t *testing.T) { + var created bool + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + return &github.PullRequest{Number: n, HeadRefName: "b", BaseRefName: "main"}, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{ + {ID: 7, PullRequests: []int{10}}, + }, nil + }, + UpdateStackFn: func(string, []int) error { + return &api.HTTPError{StatusCode: 404, Message: "Not Found"} + }, + CreateStackFn: func(prNumbers []int) (int, error) { + created = true + return 99, nil + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"10", "20"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Err.Close() + errOut, _ := io.ReadAll(errR) + output := string(errOut) + + assert.NoError(t, err) + assert.True(t, created) + assert.Contains(t, output, "Created stack with 2 PRs") +} + +func TestLink_PushesBranchesBeforeResolution(t *testing.T) { + var pushedBranches []string + var pushedRemote string + + restore := git.SetOps(&git.MockOps{ + BranchExistsFn: func(name string) bool { return name == "feat-a" || name == "feat-b" }, + ResolveRemoteFn: func(string) (string, error) { return "origin", nil }, + PushFn: func(remote string, branches []string, force, atomic bool) error { + pushedRemote = remote + pushedBranches = branches + return nil + }, + }) + defer restore() + + prCounter := 0 + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRForBranchFn: func(branch string) (*github.PullRequest, error) { + prCounter++ + return &github.PullRequest{ + Number: prCounter, HeadRefName: branch, BaseRefName: "main", + URL: fmt.Sprintf("https://github.com/o/r/pull/%d", prCounter), + }, nil + }, + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + return &github.PullRequest{Number: n, HeadRefName: fmt.Sprintf("b%d", n), BaseRefName: "main"}, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { return []github.RemoteStack{}, nil }, + CreateStackFn: func([]int) (int, error) { return 1, nil }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"feat-a", "feat-b"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Err.Close() + errOut, _ := io.ReadAll(errR) + output := string(errOut) + + assert.NoError(t, err) + assert.Equal(t, "origin", pushedRemote) + assert.Equal(t, []string{"feat-a", "feat-b"}, pushedBranches) + assert.Contains(t, output, "Pushing 2 branches") +} + +func TestLink_RemoteFlag(t *testing.T) { + var pushedRemote string + + restore := git.SetOps(&git.MockOps{ + BranchExistsFn: func(string) bool { return true }, + PushFn: func(remote string, branches []string, force, atomic bool) error { + pushedRemote = remote + return nil + }, + }) + defer restore() + + prCounter := 0 + cfg, _, _ := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRForBranchFn: func(branch string) (*github.PullRequest, error) { + prCounter++ + return &github.PullRequest{ + Number: prCounter, HeadRefName: branch, BaseRefName: "main", + URL: fmt.Sprintf("https://github.com/o/r/pull/%d", prCounter), + }, nil + }, + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + return &github.PullRequest{Number: n, HeadRefName: fmt.Sprintf("b%d", n), BaseRefName: "main"}, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { return []github.RemoteStack{}, nil }, + CreateStackFn: func([]int) (int, error) { return 1, nil }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"--remote", "upstream", "feat-a", "feat-b"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + assert.NoError(t, err) + assert.Equal(t, "upstream", pushedRemote) +} + +func TestLink_SkipsPushForPRNumbersOnly(t *testing.T) { + pushCalled := false + + restore := git.SetOps(&git.MockOps{ + BranchExistsFn: func(string) bool { return false }, // PR numbers aren't local branches + PushFn: func(string, []string, bool, bool) error { + pushCalled = true + return nil + }, + }) + defer restore() + + cfg, _, _ := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + return &github.PullRequest{Number: n, HeadRefName: "b", BaseRefName: "main"}, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { return []github.RemoteStack{}, nil }, + CreateStackFn: func([]int) (int, error) { return 1, nil }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"10", "20"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + assert.NoError(t, err) + assert.False(t, pushCalled, "push should not be called when all args are PR numbers") +} + +func TestLink_PrevalidatesBeforeCreatingPRs(t *testing.T) { + // Scenario: branch feat-b has an existing PR #106 in a stack with [104, 105, 106]. + // User runs: gh stack link feat-a feat-b + // feat-a has no PR yet, but the stack pre-validation should catch that + // #104 and #105 would be dropped — and fail BEFORE creating a PR for feat-a. + restore := git.SetOps(newLinkGitMock("feat-a", "feat-b")) + defer restore() + + prCreated := false + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRForBranchFn: func(branch string) (*github.PullRequest, error) { + if branch == "feat-b" { + return &github.PullRequest{ + Number: 106, HeadRefName: "feat-b", BaseRefName: "main", + URL: "https://github.com/o/r/pull/106", + }, nil + } + return nil, nil // feat-a has no PR + }, + CreatePRFn: func(base, head, title, body string, draft bool) (*github.PullRequest, error) { + prCreated = true + return &github.PullRequest{Number: 200, HeadRefName: head}, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{ + {ID: 7, PullRequests: []int{104, 105, 106}}, + }, nil + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"feat-a", "feat-b"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Err.Close() + errOut, _ := io.ReadAll(errR) + output := string(errOut) + + assert.ErrorIs(t, err, ErrInvalidArgs) + assert.False(t, prCreated, "should NOT create PRs before validating stack") + assert.Contains(t, output, "would remove") + assert.Contains(t, output, "#104") + assert.Contains(t, output, "#105") +} + +// --- Unit tests for helpers --- + +func TestFindMatchingStack(t *testing.T) { + tests := []struct { + name string + stacks []github.RemoteStack + prNumbers []int + wantID int + wantNil bool + wantErr bool + }{ + { + name: "no stacks", + stacks: []github.RemoteStack{}, + prNumbers: []int{10, 20}, + wantNil: true, + }, + { + name: "no match", + stacks: []github.RemoteStack{ + {ID: 1, PullRequests: []int{30, 40}}, + }, + prNumbers: []int{10, 20}, + wantNil: true, + }, + { + name: "single match", + stacks: []github.RemoteStack{ + {ID: 5, PullRequests: []int{10, 20}}, + }, + prNumbers: []int{10, 30}, + wantID: 5, + }, + { + name: "multiple matches", + stacks: []github.RemoteStack{ + {ID: 1, PullRequests: []int{10}}, + {ID: 2, PullRequests: []int{20}}, + }, + prNumbers: []int{10, 20}, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := findMatchingStack(tt.stacks, tt.prNumbers) + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + if tt.wantNil { + assert.Nil(t, got) + } else { + assert.Equal(t, tt.wantID, got.ID) + } + } + }) + } +} + +func TestFormatPRList(t *testing.T) { + assert.Equal(t, "#10", formatPRList([]int{10})) + assert.Equal(t, "#10, #20, #30", formatPRList([]int{10, 20, 30})) + assert.Equal(t, "", formatPRList([]int{})) +} + +func TestSlicesEqual(t *testing.T) { + assert.True(t, slicesEqual([]int{1, 2, 3}, []int{1, 2, 3})) + assert.False(t, slicesEqual([]int{1, 2, 3}, []int{1, 2})) + assert.False(t, slicesEqual([]int{1, 2}, []int{1, 3})) + assert.True(t, slicesEqual([]int{}, []int{})) +} + +func TestValidateArgs(t *testing.T) { + assert.NoError(t, validateArgs([]string{"a", "b", "c"})) + assert.NoError(t, validateArgs([]string{"10", "20"})) + assert.Error(t, validateArgs([]string{"a", "a"})) + assert.Error(t, validateArgs([]string{"10", "10"})) +} + +func TestFormatAPIError(t *testing.T) { + tests := []struct { + name string + err error + want string + }{ + { + name: "HTTP error with message", + err: &api.HTTPError{StatusCode: 422, Message: "Validation Failed"}, + want: "HTTP 422: Validation Failed", + }, + { + name: "HTTP error without message", + err: &api.HTTPError{StatusCode: 500}, + want: "HTTP 500", + }, + { + name: "non-HTTP error", + err: fmt.Errorf("network timeout"), + want: "network timeout", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, formatAPIError(tt.err)) + }) + } +} + +func TestLink_FindPRByNumber_ErrorIsFatal(t *testing.T) { + // When FindPRByNumber returns an error (not just nil), it should NOT + // silently fall through to branch-name lookup. + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + return nil, fmt.Errorf("network error") + }, + FindPRForBranchFn: func(branch string) (*github.PullRequest, error) { + t.Fatal("FindPRForBranch should NOT be called when FindPRByNumber errors") + return nil, nil + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"42", "43"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Err.Close() + errOut, _ := io.ReadAll(errR) + output := string(errOut) + + assert.ErrorIs(t, err, ErrAPIFailure) + assert.Contains(t, output, "failed to look up PR #42") +} + +func TestLink_SkipsBaseFix_ForNewlyCreatedPRs(t *testing.T) { + // When PRs are created by the command, fixBaseBranches should skip them + // (no re-fetch needed since they already have the correct base). + restore := git.SetOps(newLinkGitMock("feat-a", "feat-b")) + defer restore() + + findByNumberCalls := 0 + cfg, _, _ := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRForBranchFn: func(branch string) (*github.PullRequest, error) { + return nil, nil // no existing PRs + }, + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + findByNumberCalls++ + return nil, nil + }, + CreatePRFn: func(base, head, title, body string, draft bool) (*github.PullRequest, error) { + return &github.PullRequest{ + Number: 100, HeadRefName: head, BaseRefName: base, + URL: "https://github.com/o/r/pull/100", + }, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { return []github.RemoteStack{}, nil }, + CreateStackFn: func([]int) (int, error) { return 1, nil }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"feat-a", "feat-b"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + assert.NoError(t, err) + // FindPRByNumber is called during findExistingPRs (phase 2) for numeric + // args only, but NOT during fixBaseBranches for newly created PRs. + // Since "feat-a" and "feat-b" are not numeric, FindPRByNumber should + // not be called at all. + assert.Equal(t, 0, findByNumberCalls, "FindPRByNumber should not be called for newly created PRs") +} + +// Silence "imported and not used" for fmt in case test helpers use it. +var _ = fmt.Sprintf diff --git a/cmd/root.go b/cmd/root.go index 704fe1e..b29a23d 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -37,6 +37,7 @@ func RootCmd() *cobra.Command { root.AddCommand(SyncCmd(cfg)) root.AddCommand(UnstackCmd(cfg)) root.AddCommand(MergeCmd(cfg)) + root.AddCommand(LinkCmd(cfg)) // Helper commands root.AddCommand(ViewCmd(cfg)) diff --git a/docs/src/content/docs/faq.md b/docs/src/content/docs/faq.md index 38d7381..aa315e5 100644 --- a/docs/src/content/docs/faq.md +++ b/docs/src/content/docs/faq.md @@ -188,11 +188,12 @@ No. Stacked PRs are built on standard git branches and regular pull requests. Yo ### Will this work with a different tool for stacking? -Yes, you can continue to use your tool of choice (e.g., jj, Sapling, ghstack, git-town, etc.) to manage stacks locally and push up your branches to GitHub. +Yes, you can continue to use your tool of choice (e.g., jj, Sapling, git-town, etc.) to manage stacks locally and push up your branches to GitHub. Stacked PRs on GitHub are based on the standard pull request model — any tool that creates PRs with the correct base branches can work with them. The `gh stack` CLI is purpose-built for the GitHub experience, but other tools that manage branch chains should be compatible. -You can also use the GitHub CLI in conjunction with other tools to open your PRs as a stack: +You can also use the `gh stack link` command in conjunction with other tools to open your PRs as a stack: + ```bash # Create a stack of branches locally using jj @@ -208,7 +209,30 @@ jj new -m "third change" jj bookmark create change3 --revision @ # ... -# Use gh stack to submit a stack of PRs +# Push branches and link them as a stack on GitHub +# (creates PRs automatically if they don't exist) +gh stack link change1 change2 change3 +``` + +This doesn't create any local tracking and only hits the APIs to create Stacked PRs. + +If the provided branches already have open PRs, `link` will use them. If not, it creates PRs with the correct base branch chaining. + +To add more to the stack, run `link` again, but be sure to include the full list of PRs/branches in the stack: + +```bash +gh stack link 123 124 125 change4 change5 +``` + +You can also use `--base` to specify a different trunk branch and `--draft` to create PRs as drafts: + +```bash +gh stack link --base develop --draft change1 change2 change3 +``` + +Alternatively, if you want full local stack tracking (for commands like `rebase`, `sync`, and navigation), you can adopt existing branches to local tracking with `gh stack`: + +```bash gh stack init --adopt change1 change2 change3 gh stack submit ``` diff --git a/docs/src/content/docs/reference/cli.md b/docs/src/content/docs/reference/cli.md index 2a90504..f54dee9 100644 --- a/docs/src/content/docs/reference/cli.md +++ b/docs/src/content/docs/reference/cli.md @@ -312,6 +312,42 @@ gh stack unstack --local gh stack unstack feature-auth ``` +### `gh stack link` + +Link PRs into a stack on GitHub without local tracking. + +```sh +gh stack link [flags] [...] +``` + +Creates or updates a stack on GitHub from branch names or PR numbers. This command does not create or modify any `gh-stack` local tracking state. It is designed for users who manage branches with other tools locally (e.g., jj, Sapling, git-town) and want to simply open a stack of PRs. + +Arguments are provided in stack order (bottom to top). Branch arguments are automatically pushed to the remote before creating or looking up PRs. For branches that already have open PRs, those PRs are used. For branches without PRs, new PRs are created automatically with the correct base branch chaining. Existing PRs whose base branch doesn't match the expected chain are corrected automatically. + +If the PRs are not yet in a stack, a new stack is created. If some of the PRs are already in a stack, the existing stack is updated to include the new PRs. Existing PRs are never removed from a stack — the update is additive only. + +| Flag | Description | +|------|-------------| +| `--base ` | Base branch for the bottom of the stack (default: `main`) | +| `--draft` | Create new PRs as drafts | +| `--remote ` | Remote to push to (defaults to auto-detected remote) | + +**Examples:** + +```sh +# Link branches into a stack (pushes, creates PRs, creates stack) +gh stack link feature-auth feature-api feature-ui + +# Link existing PRs by number +gh stack link 10 20 30 + +# Add branches to an existing stack of PRs +gh stack link 42 43 feature-auth feature-ui + +# Use a different base branch and create PRs as drafts +gh stack link --base develop --draft feat-a feat-b feat-c +``` + --- ## Navigation diff --git a/skills/gh-stack/SKILL.md b/skills/gh-stack/SKILL.md index ad928a4..0bff246 100644 --- a/skills/gh-stack/SKILL.md +++ b/skills/gh-stack/SKILL.md @@ -56,11 +56,12 @@ git config remote.pushDefault origin # if multiple remotes exist (skips remo 2. **When a prefix is set, pass only the suffix to `add`.** `gh stack add auth` with prefix `feat` → `feat/auth`. Passing `feat/auth` creates `feat/feat/auth`. 3. **Always use `--auto` with `gh stack submit`** to auto-generate PR titles. Without `--auto`, `submit` prompts for a title for each new PR. 4. **Always use `--json` with `gh stack view`.** Without `--json`, the command launches an interactive TUI that cannot be operated by agents. There is no other appropriate flag — always pass `--json`. -5. **Use `--remote ` when multiple remotes are configured**, or pre-configure `git config remote.pushDefault origin`. Without this, `push`, `submit`, `sync`, and `checkout` trigger an interactive remote picker. +5. **Use `--remote ` when multiple remotes are configured**, or pre-configure `git config remote.pushDefault origin`. Without this, `push`, `submit`, `sync`, `link`, and `checkout` trigger an interactive remote picker. 6. **Avoid branches shared across multiple stacks.** If a branch belongs to multiple stacks, commands exit with code 6. Check out a non-shared branch first. 7. **Plan your stack layers by dependency order before writing code.** Foundational changes (models, APIs, shared utilities) go in lower branches; dependent changes (UI, consumers) go in higher branches. Think through the dependency chain before running `gh stack init`. 8. **Use standard `git add` and `git commit` for staging and committing.** This gives you full control over which changes go into each branch. The `-Am` shortcut is available but should not be the default approach—stacked PRs are most effective when each branch contains a deliberate, logical set of changes. 9. **Navigate down the stack when you need to change a lower layer.** If you're working on a frontend branch and realize you need API changes, don't hack around it at the current layer. Navigate to the appropriate branch (`gh stack down`, `gh stack checkout`, or `gh stack bottom`), make and commit the changes there, run `gh stack rebase --upstack`, then navigate back up to continue. +10. **Use `gh stack link` for external tool workflows.** When branches are managed by an external tool (jj, Sapling, etc.), use `gh stack link branch-a branch-b`. `link` does not rely on local tracking state and is intended for API-driven PR and stack management. Always provide at least 2 branch names or PR numbers. **Never do any of the following — each triggers an interactive prompt or TUI that will hang:** - ❌ `gh stack view` or `gh stack view --short` — always use `gh stack view --json` @@ -555,6 +556,54 @@ gh stack submit --auto --draft --- +### Link branches as a stack (no local tracking) — `gh stack link` + +Link PRs into a stack on GitHub without creating any local tracking state. This is the recommended approach if you are managing stacked branches with other tools (jj, Sapling, git-town) and want to simply create GitHub Stacked PRs via an API. + +``` +gh stack link [flags] [...] +``` + +```bash +# Link branches into a stack (pushes, creates PRs, creates stack) +gh stack link branch-a branch-b branch-c + +# Use a different base branch and create PRs as drafts +gh stack link --base develop --draft branch-a branch-b branch-c + +# Link existing PRs by number +gh stack link 10 20 30 + +# Add branches to an existing stack of PRs +gh stack link 42 43 feature-auth feature-ui +``` + +| Flag | Description | +|------|---------| +| `--base ` | Base branch for the bottom of the stack (default: `main`) | +| `--draft` | Create new PRs as drafts | +| `--remote ` | Remote to push to (use if multiple remotes exist) | + +**Behavior:** + +- Arguments are provided in stack order (bottom to top) +- Each argument can be a branch name or a PR number. Numeric arguments are tried as PR numbers first; if no PR with that number exists, the argument is treated as a branch name +- Branch arguments are pushed to the remote automatically (non-force, atomic) +- For branches without open PRs, new PRs are created with auto-generated titles and the correct base branch chaining (first branch uses `--base`, subsequent branches use the previous branch) +- Existing PRs whose base branch doesn't match the expected chain are corrected automatically +- If the PRs are not yet in a stack, a new stack is created. If some PRs are already in a stack, the stack is updated (additive only — existing PRs are never removed) +- Does **not** create or modify any local state + +**Output (stderr):** + +- `Pushing N branches to ...` +- `Found PR #N for branch ` for branches with existing PRs +- `Created PR #N for (base: )` for newly created PRs +- `Updated base branch for PR #N to ` when base branches are corrected +- `Created stack with N PRs` or `Updated stack to N PRs` + +--- + ### Sync the stack — `gh stack sync` Fetch, rebase, push, and sync PR state in a single command. This is the recommended command for routine synchronization.