diff --git a/cmd/link.go b/cmd/link.go index c585cf1..86905bd 100644 --- a/cmd/link.go +++ b/cmd/link.go @@ -14,16 +14,17 @@ import ( ) type linkOptions struct { - base string - open bool - remote string + base string + open bool + remote string + baseChanged bool } func LinkCmd(cfg *config.Config) *cobra.Command { opts := &linkOptions{} cmd := &cobra.Command{ - Use: "link [...]", + Use: "link [...]", Short: "Link PRs into a stack on GitHub without local tracking", Long: `Create or update a stack on GitHub from branch names, PR numbers, or PR URLs. @@ -46,7 +47,15 @@ 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).`, +the new PRs (existing PRs are never removed). + +As a shortcut for growing an existing stack, pass a stack number as the +first argument (the number shown in the GitHub stack UI). The remaining +arguments are appended to the top of that stack, so you don't have to +re-list its current PRs. Arguments already in the stack are skipped; +arguments that belong to a different stack are rejected. Because stack and +PR numbers never overlap, a numeric first argument is treated as a stack +only when it matches an existing stack.`, Example: ` # Link branches into a stack (bottom to top) $ gh stack link auth-layer api-routes ui-components @@ -56,10 +65,14 @@ the new PRs (existing PRs are never removed).`, # Link existing PRs by URL $ gh stack link https://github.com/owner/repo/pull/41 https://github.com/owner/repo/pull/42 + # Add PRs to the top of an existing stack (7 is a stack number) + $ gh stack link 7 48 ui-polish + # Specify a custom base branch for stack $ gh stack link --base develop auth-layer api-routes`, Args: cobra.MinimumNArgs(2), RunE: func(cmd *cobra.Command, args []string) error { + opts.baseChanged = cmd.Flags().Changed("base") return runLink(cfg, opts, args) }, } @@ -92,32 +105,84 @@ func runLink(cfg *config.Config, opts *linkOptions, args []string) error { return ErrAPIFailure } - // Phase 1: Push branch args to the remote so PRs can be found/created - if err := pushBranchArgs(cfg, opts, args); err != nil { + // Fetch existing stacks up front. They are needed to detect whether the + // first argument names an existing stack (add mode) and, in the + // create/update path, to find the stack the PRs already belong to. + cfg.Printf("Checking existing stacks...") + stacks, err := listStacksSafe(cfg, client) + if err != nil { + return err + } + + // Detect "add mode": when the first argument is a bare stack number that + // matches an existing stack, the remaining arguments are appended to the + // top of that stack. Stack, PR, and issue numbers share one repo-scoped + // numberspace, so a number that names a stack never also names a PR. + targetStack, prArgs := detectAddMode(args, stacks) + + // Phase 1: Push branch args to the remote so PRs can be found/created. + if err := pushBranchArgs(cfg, opts, prArgs); err != nil { return err } - // Phase 2: Find existing PRs for all args (don't create yet) - cfg.Printf("Looking up PRs for %d %s...", len(args), plural(len(args), "branch", "branches")) - found, err := findExistingPRs(cfg, client, args) + // Phase 2: Find existing PRs for all PR args (don't create yet). + cfg.Printf("Looking up PRs for %d %s...", len(prArgs), plural(len(prArgs), "branch", "branches")) + found, err := findExistingPRs(cfg, client, prArgs) if err != nil { return err } - // Phase 2b: Fetch existing stacks first so eligibility validation and - // stack pre-validation can account for PRs that are already members of - // the target stack. The stacks are also reused in the upsert phase. + // Look up the repository's PR template (best-effort; skip if not in a repo). + var templateContent string + if repoRoot, tlErr := git.RootDir(); tlErr == nil { + templateContent = pr.FindTemplate(repoRoot) + } + + // Add mode: append the PR args to the top of the named stack. + if targetStack != nil { + return runLinkAdd(cfg, client, opts, targetStack, stacks, prArgs, found, templateContent) + } + + // Create/update mode: create a new stack or additively update the stack + // the PRs already belong to. + return runLinkCreateOrUpdate(cfg, client, opts, stacks, prArgs, found, templateContent) +} + +// detectAddMode reports whether the first argument names an existing stack, +// enabling "add mode" in which the remaining arguments are appended to the top +// of that stack. It returns the matched stack (or nil) and the PR arguments to +// resolve — args[1:] in add mode, or all args otherwise. +// +// Only a bare positive integer that isn't also a local branch name can name a +// stack. Stack, PR, and issue numbers share one repo-scoped numberspace (so a +// number never doubles as a PR), but branch names don't — a branch literally +// named like a stack number is kept as a branch. +func detectAddMode(args []string, stacks []github.RemoteStack) (*github.RemoteStack, []string) { + if len(args) < 2 { + return nil, args + } + n, err := strconv.Atoi(args[0]) + if err != nil || n <= 0 || git.BranchExists(args[0]) { + return nil, args + } + for i := range stacks { + if stacks[i].Number == n { + return &stacks[i], args[1:] + } + } + return nil, args +} + +// runLinkCreateOrUpdate creates a new stack from the resolved PR args, or +// additively updates the single stack the PRs already belong to. This is the +// original link behavior, where every PR in the stack must be listed. +func runLinkCreateOrUpdate(cfg *config.Config, client github.ClientOps, opts *linkOptions, stacks []github.RemoteStack, prArgs []string, found []*resolvedArg, templateContent string) error { knownPRNumbers := make([]int, 0, len(found)) for _, r := range found { if r != nil { knownPRNumbers = append(knownPRNumbers, r.prNumber) } } - cfg.Printf("Checking existing stacks...") - stacks, err := listStacksSafe(cfg, client) - if err != nil { - return err - } // Determine the stack these PRs already belong to (if any). PRs that are // already members of this stack are exempt from the eligibility checks @@ -135,22 +200,16 @@ func runLink(cfg *config.Config, opts *linkOptions, args []string) error { return err } - // Phase 3: Pre-validate the stack — check that adding these PRs won't - // drop existing PRs from the target stack before creating any new PRs, - // so we can fail early without leaving orphaned PRs. + // Pre-validate the stack — check that adding these PRs won't drop existing + // PRs from the target stack before creating any new PRs, so we can fail + // early without leaving orphaned PRs. if targetStack != nil { if err := prevalidateStack(cfg, targetStack, knownPRNumbers); err != nil { return err } } - // Look up the repository's PR template (best-effort; skip if not in a repo). - var templateContent string - if repoRoot, tlErr := git.RootDir(); tlErr == nil { - templateContent = pr.FindTemplate(repoRoot) - } - - // Phase 4: Create PRs for branches that don't have one yet + // Create PRs for branches that don't have one yet. needsCreation := 0 for _, r := range found { if r == nil { @@ -160,15 +219,15 @@ func runLink(cfg *config.Config, opts *linkOptions, args []string) error { if needsCreation > 0 { cfg.Printf("Creating %d %s...", needsCreation, plural(needsCreation, "PR", "PRs")) } - resolved, err := createMissingPRs(cfg, client, opts, args, found, templateContent) + resolved, err := createMissingPRs(cfg, client, opts, prArgs, found, templateContent, opts.base) if err != nil { return err } - // Phase 5: Fix base branches for existing PRs with wrong bases - fixBaseBranches(cfg, client, opts, resolved) + // Fix base branches for existing PRs with wrong bases. + fixBaseBranches(cfg, client, opts, resolved, opts.base) - // Phase 6: Upsert the stack (reuse stacks from phase 3) + // Upsert the stack (reuse the stacks fetched above). prNumbers := make([]int, len(resolved)) for i, r := range resolved { prNumbers[i] = r.prNumber @@ -177,6 +236,159 @@ func runLink(cfg *config.Config, opts *linkOptions, args []string) error { return upsertStack(cfg, client, stacks, prNumbers) } +// runLinkAdd appends the resolved PR args to the top of an existing stack. PRs +// already in the target stack are skipped (idempotent); PRs that belong to a +// different stack are rejected. Branch args without a PR get one created and +// chained on top of the stack's current top branch. +func runLinkAdd(cfg *config.Config, client github.ClientOps, opts *linkOptions, target *github.RemoteStack, stacks []github.RemoteStack, prArgs []string, found []*resolvedArg, templateContent string) error { + if opts.baseChanged { + cfg.Warningf("--base is ignored when adding to stack #%d (its base is fixed by the existing stack)", target.Number) + } + + inTarget := make(map[int]bool, len(target.PRNumbers())) + for _, n := range target.PRNumbers() { + inTarget[n] = true + } + + // Partition the args into those already in the target stack (skipped) and + // those to append. found is parallel to prArgs; a nil entry is a branch + // with no PR yet, which is always new. + var appendArgs []string + var appendFound []*resolvedArg + for i, arg := range prArgs { + r := found[i] + if r != nil && inTarget[r.prNumber] { + cfg.Infof("PR %s is already in stack #%d — skipping", + cfg.PRLink(r.prNumber, r.prURL), target.Number) + continue + } + appendArgs = append(appendArgs, arg) + appendFound = append(appendFound, r) + } + + // Enforce the one-stack constraint: none of the PRs being appended may + // belong to a different stack. + if err := ensureNotInOtherStack(cfg, stacks, target.Number, appendFound); err != nil { + return err + } + + if len(appendArgs) == 0 { + cfg.Successf("Stack #%d is already up to date", target.Number) + return nil + } + + // Validate eligibility of the PRs being appended. Target members were + // filtered out above, so every remaining PR faces the full checks. + if err := validatePREligibility(cfg, appendFound, nil); err != nil { + return err + } + + // New PRs chain on top of the stack's current top branch. The stack list + // response should carry per-PR head refs, but fall back to fetching the + // full stack if the top branch can't be resolved from the listed stack. + topBranch, err := stackTopBranch(target) + if err != nil { + if full, gerr := client.GetStack(target.Number); gerr == nil && full != nil { + topBranch, err = stackTopBranch(full) + } + if err != nil { + cfg.Errorf("%s", err) + return ErrAPIFailure + } + } + + needsCreation := 0 + for _, r := range appendFound { + if r == nil { + needsCreation++ + } + } + if needsCreation > 0 { + cfg.Printf("Creating %d %s...", needsCreation, plural(needsCreation, "PR", "PRs")) + } + resolved, err := createMissingPRs(cfg, client, opts, appendArgs, appendFound, templateContent, topBranch) + if err != nil { + return err + } + + // Correct base branches so the appended PRs chain on top of the stack. + fixBaseBranches(cfg, client, opts, resolved, topBranch) + + delta := make([]int, len(resolved)) + for i, r := range resolved { + delta[i] = r.prNumber + } + + return addToStack(cfg, client, target.Number, delta) +} + +// ensureNotInOtherStack verifies that none of the resolved PRs belong to a +// stack other than the target. PRs in no stack are allowed. Reports every +// offender before returning an error. +func ensureNotInOtherStack(cfg *config.Config, stacks []github.RemoteStack, targetNumber int, found []*resolvedArg) error { + owner := make(map[int]int) + for i := range stacks { + for _, n := range stacks[i].PRNumbers() { + owner[n] = stacks[i].Number + } + } + + invalid := 0 + for _, r := range found { + if r == nil { + continue + } + if sn, ok := owner[r.prNumber]; ok && sn != targetNumber { + cfg.Errorf("PR %s already belongs to stack #%d — unstack it first", + cfg.PRLink(r.prNumber, r.prURL), sn) + invalid++ + } + } + if invalid > 0 { + return ErrInvalidArgs + } + return nil +} + +// stackTopBranch returns the head branch of the pull request at the top of the +// stack — the base for the first PR appended on top of it. +func stackTopBranch(s *github.RemoteStack) (string, error) { + if len(s.PRDetails) == 0 { + return "", fmt.Errorf("stack #%d has no pull requests to append to", s.Number) + } + top := s.PRDetails[len(s.PRDetails)-1] + if top.Head.Ref == "" { + return "", fmt.Errorf("could not determine the top branch of stack #%d", s.Number) + } + return top.Head.Ref, nil +} + +// addToStack appends the delta PR numbers to the top of the target stack and +// reports the result, translating API errors into typed exit codes. +func addToStack(cfg *config.Config, client github.ClientOps, stackNumber int, delta []int) error { + if _, err := client.AddToStack(stackNumber, delta); err != nil { + var httpErr *api.HTTPError + if errors.As(err, &httpErr) { + switch httpErr.StatusCode { + case 404: + cfg.Errorf("Stack #%d no longer exists", stackNumber) + return ErrNotInStack + case 422: + cfg.Errorf("Cannot add to stack: %s", httpErr.Message) + return ErrAPIFailure + default: + cfg.Errorf("Failed to add to stack (HTTP %d): %s", httpErr.StatusCode, httpErr.Message) + return ErrAPIFailure + } + } + cfg.Errorf("Failed to add to stack: %v", err) + return ErrAPIFailure + } + + cfg.Successf("Added %d %s to stack #%d", len(delta), plural(len(delta), "PR", "PRs"), stackNumber) + return nil +} + // 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 @@ -410,8 +622,10 @@ func prevalidateStack(cfg *config.Config, matchedStack *github.RemoteStack, know } // createMissingPRs creates PRs for branches that don't have one yet. -// Returns the fully resolved list with all branches mapped to PRs. -func createMissingPRs(cfg *config.Config, client github.ClientOps, opts *linkOptions, args []string, found []*resolvedArg, templateContent string) ([]resolvedArg, error) { +// Returns the fully resolved list with all branches mapped to PRs. bottomBase +// is the base branch for the first PR in the chain; each subsequent PR bases +// off the previous PR's head branch. +func createMissingPRs(cfg *config.Config, client github.ClientOps, opts *linkOptions, args []string, found []*resolvedArg, templateContent, bottomBase string) ([]resolvedArg, error) { resolved := make([]resolvedArg, len(args)) for i, arg := range args { @@ -421,7 +635,7 @@ func createMissingPRs(cfg *config.Config, client github.ClientOps, opts *linkOpt } // Determine the base branch for this PR - baseBranch := opts.base + baseBranch := bottomBase if i > 0 { baseBranch = resolved[i-1].branch } @@ -448,17 +662,17 @@ func createMissingPRs(cfg *config.Config, client github.ClientOps, opts *linkOpt } // fixBaseBranches updates the base branch of existing PRs to match the -// expected stack chain. The first PR should have base = opts.base, +// expected stack chain. The first PR should have base = bottomBase, // 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) { +func fixBaseBranches(cfg *config.Config, client github.ClientOps, opts *linkOptions, resolved []resolvedArg, bottomBase string) { for i, r := range resolved { if r.created { continue } - expectedBase := opts.base + expectedBase := bottomBase if i > 0 { expectedBase = resolved[i-1].branch } diff --git a/cmd/link_test.go b/cmd/link_test.go index 15aeb23..d973961 100644 --- a/cmd/link_test.go +++ b/cmd/link_test.go @@ -1177,6 +1177,51 @@ func TestLink_NumericArg_PRNotFound_TreatedAsBranch(t *testing.T) { assert.Equal(t, []int{50, 51}, stackedPRs) } +func TestLink_NumericFirstArgIsLocalBranch_NotAddMode(t *testing.T) { + // Branch "123" exists locally and stack #123 also exists. The numeric + // branch name must win over add mode, so the args form a new stack rather + // than appending #456 to the unrelated stack #123. + restore := git.SetOps(newLinkGitMock("123", "456")) + defer restore() + + var createdPRs []int + cfg, _, _ := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRByNumberFn: func(int) (*github.PullRequest, error) { return nil, nil }, + FindPRForBranchFn: func(branch string) (*github.PullRequest, error) { + switch branch { + case "123": + return &github.PullRequest{Number: 50, HeadRefName: "123", BaseRefName: "main", URL: "https://github.com/o/r/pull/50"}, nil + case "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{ + linkRemoteStack(123, linkPR(90, "unrelated-a"), linkPR(91, "unrelated-b")), + }, nil + }, + AddToStackFn: func(int, []int) (*github.RemoteStack, error) { + t.Fatal("AddToStack must not be called: a numeric branch name should not trigger add mode") + return nil, nil + }, + CreateStackFn: func(prNumbers []int) (*github.RemoteStack, error) { + createdPRs = prNumbers + return &github.RemoteStack{ID: 42, Number: 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}, createdPRs) +} + func TestLink_FixesBaseBranches(t *testing.T) { restore := git.SetOps(newLinkGitMock("feat-a", "feat-b")) defer restore() @@ -1913,3 +1958,560 @@ func TestLink_MixedURLsAndNumbers(t *testing.T) { assert.Equal(t, []int{10, 20, 30}, createdPRs) assert.Contains(t, output, "Created stack with 3 PRs") } + +// --- add-mode tests (first arg is a stack number) --- + +// linkPR builds an open RemoteStackPR with the given number and head ref. +func linkPR(num int, head string) github.RemoteStackPR { + return github.RemoteStackPR{ + Number: num, + State: "open", + Head: github.RemoteStackPRHead{Ref: head}, + } +} + +// linkRemoteStack builds a RemoteStack with matching PullRequests and PRDetails +// (bottom to top) for add-mode tests, so stackTopBranch can resolve the top ref. +func linkRemoteStack(number int, details ...github.RemoteStackPR) github.RemoteStack { + rs := github.RemoteStack{ID: number, Number: number} + for _, d := range details { + rs.PullRequests = append(rs.PullRequests, d.Number) + rs.PRDetails = append(rs.PRDetails, d) + } + return rs +} + +func TestLink_AddMode_AppendsPRNumberToStack(t *testing.T) { + var addNumber int + var addPRs []int + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + return &github.PullRequest{ + Number: n, State: "OPEN", + HeadRefName: fmt.Sprintf("branch-%d", n), + BaseRefName: "branch-20", // already chained on the stack top + URL: fmt.Sprintf("https://github.com/o/r/pull/%d", n), + }, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{ + linkRemoteStack(7, linkPR(10, "branch-10"), linkPR(20, "branch-20")), + }, nil + }, + AddToStackFn: func(stackNumber int, prNumbers []int) (*github.RemoteStack, error) { + addNumber = stackNumber + addPRs = prNumbers + return &github.RemoteStack{ID: 7, Number: 7}, nil + }, + CreateStackFn: func([]int) (*github.RemoteStack, error) { + t.Fatal("CreateStack should not be called in add mode") + return nil, nil + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"7", "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, addNumber) + assert.Equal(t, []int{30}, addPRs) + assert.Contains(t, output, "Added 1 PR to stack #7") +} + +func TestLink_AddMode_CreatesPRForBranchOnTopOfStack(t *testing.T) { + restore := git.SetOps(newLinkGitMock("feature-c")) + defer restore() + + var created []struct{ base, head string } + var addPRs []int + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRForBranchFn: func(string) (*github.PullRequest, error) { return nil, nil }, + CreatePRFn: func(base, head, title, body string, draft bool) (*github.PullRequest, error) { + created = append(created, struct{ base, head string }{base, head}) + return &github.PullRequest{ + Number: 99, State: "OPEN", HeadRefName: head, BaseRefName: base, + URL: "https://github.com/o/r/pull/99", + }, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{ + linkRemoteStack(7, linkPR(10, "branch-10"), linkPR(20, "branch-20")), + }, nil + }, + AddToStackFn: func(stackNumber int, prNumbers []int) (*github.RemoteStack, error) { + addPRs = prNumbers + return &github.RemoteStack{ID: 7, Number: 7}, nil + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"7", "feature-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, created, 1) + assert.Equal(t, "branch-20", created[0].base) // chains on the stack top branch + assert.Equal(t, "feature-c", created[0].head) + assert.Equal(t, []int{99}, addPRs) + assert.Contains(t, output, "Added 1 PR to stack #7") +} + +func TestLink_AddMode_IdempotentWhenAllPresent(t *testing.T) { + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + return &github.PullRequest{ + Number: n, State: "OPEN", + HeadRefName: fmt.Sprintf("branch-%d", n), BaseRefName: "branch-10", + URL: fmt.Sprintf("https://github.com/o/r/pull/%d", n), + }, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{ + linkRemoteStack(7, linkPR(10, "branch-10"), linkPR(20, "branch-20")), + }, nil + }, + AddToStackFn: func(int, []int) (*github.RemoteStack, error) { + t.Fatal("AddToStack should not be called when nothing new to append") + return nil, nil + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"7", "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.Contains(t, output, "already in stack #7") + assert.Contains(t, output, "Stack #7 is already up to date") +} + +func TestLink_AddMode_SkipsPresentAppendsNew(t *testing.T) { + var addPRs []int + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + base := "branch-20" // new PR chains on the stack top + if n == 20 { + base = "branch-10" + } + return &github.PullRequest{ + Number: n, State: "OPEN", + HeadRefName: fmt.Sprintf("branch-%d", n), BaseRefName: base, + URL: fmt.Sprintf("https://github.com/o/r/pull/%d", n), + }, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{ + linkRemoteStack(7, linkPR(10, "branch-10"), linkPR(20, "branch-20")), + }, nil + }, + AddToStackFn: func(stackNumber int, prNumbers []int) (*github.RemoteStack, error) { + addPRs = prNumbers + return &github.RemoteStack{ID: 7, Number: 7}, nil + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"7", "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{30}, addPRs) + assert.Contains(t, output, "already in stack #7") + assert.Contains(t, output, "Added 1 PR to stack #7") +} + +func TestLink_AddMode_RejectsPRFromAnotherStack(t *testing.T) { + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + return &github.PullRequest{ + Number: n, State: "OPEN", + 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{ + linkRemoteStack(7, linkPR(10, "branch-10"), linkPR(20, "branch-20")), + linkRemoteStack(8, linkPR(30, "branch-30"), linkPR(40, "branch-40")), + }, nil + }, + AddToStackFn: func(int, []int) (*github.RemoteStack, error) { + t.Fatal("AddToStack should not be called when a PR is in another stack") + return nil, nil + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"7", "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, "already belongs to stack #8") +} + +func TestLink_AddMode_RejectsIneligibleNewPR(t *testing.T) { + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + pr := &github.PullRequest{ + Number: n, State: "OPEN", + HeadRefName: fmt.Sprintf("branch-%d", n), BaseRefName: "branch-20", + URL: fmt.Sprintf("https://github.com/o/r/pull/%d", n), + } + if n == 30 { + pr.MergeQueueEntry = &github.MergeQueueEntry{ID: "MQE_1"} + } + return pr, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{ + linkRemoteStack(7, linkPR(10, "branch-10"), linkPR(20, "branch-20")), + }, nil + }, + AddToStackFn: func(int, []int) (*github.RemoteStack, error) { + t.Fatal("AddToStack should not be called for an ineligible PR") + return nil, nil + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"7", "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, "cannot be added to a stack") + assert.Contains(t, output, "queued for merge") +} + +func TestLink_AddMode_ExemptsIneligibleExistingMember(t *testing.T) { + var addPRs []int + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + pr := &github.PullRequest{ + Number: n, State: "OPEN", + HeadRefName: fmt.Sprintf("branch-%d", n), BaseRefName: "branch-20", + URL: fmt.Sprintf("https://github.com/o/r/pull/%d", n), + } + if n == 20 { + // A queued PR that is already a stack member: it is skipped, + // so its ineligibility must not block appending PR 30. + pr.MergeQueueEntry = &github.MergeQueueEntry{ID: "MQE_1"} + pr.BaseRefName = "branch-10" + } + return pr, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{ + linkRemoteStack(7, linkPR(10, "branch-10"), linkPR(20, "branch-20")), + }, nil + }, + AddToStackFn: func(stackNumber int, prNumbers []int) (*github.RemoteStack, error) { + addPRs = prNumbers + return &github.RemoteStack{ID: 7, Number: 7}, nil + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"7", "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{30}, addPRs) + assert.Contains(t, output, "already in stack #7") +} + +func TestLink_NumericFirstArgNotAStack_UsesCreateMode(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, State: "OPEN", + 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) { + // A stack exists but its number (7) does not match arg[0] (10). + return []github.RemoteStack{ + linkRemoteStack(7, linkPR(50, "branch-50"), linkPR(60, "branch-60")), + }, nil + }, + CreateStackFn: func(prNumbers []int) (*github.RemoteStack, error) { + createdPRs = prNumbers + return &github.RemoteStack{ID: 42, Number: 42}, nil + }, + AddToStackFn: func(int, []int) (*github.RemoteStack, error) { + t.Fatal("AddToStack should not be called in create mode") + return nil, 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.Equal(t, []int{10, 20}, createdPRs) + assert.Contains(t, output, "Created stack with 2 PRs") +} + +func TestLink_AddMode_WarnsWhenBaseFlagSet(t *testing.T) { + var addPRs []int + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + return &github.PullRequest{ + Number: n, State: "OPEN", + HeadRefName: fmt.Sprintf("branch-%d", n), BaseRefName: "branch-20", + URL: fmt.Sprintf("https://github.com/o/r/pull/%d", n), + }, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{ + linkRemoteStack(7, linkPR(10, "branch-10"), linkPR(20, "branch-20")), + }, nil + }, + AddToStackFn: func(stackNumber int, prNumbers []int) (*github.RemoteStack, error) { + addPRs = prNumbers + return &github.RemoteStack{ID: 7, Number: 7}, nil + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"--base", "develop", "7", "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{30}, addPRs) + assert.Contains(t, output, "--base is ignored") +} + +func TestLink_AddMode_ChainsMultipleCreatedPRs(t *testing.T) { + restore := git.SetOps(newLinkGitMock("feat-c", "feat-d")) + defer restore() + + var created []struct{ base, head string } + var addPRs []int + prNum := 100 + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRForBranchFn: func(string) (*github.PullRequest, error) { return nil, nil }, + CreatePRFn: func(base, head, title, body string, draft bool) (*github.PullRequest, error) { + prNum++ + created = append(created, struct{ base, head string }{base, head}) + return &github.PullRequest{ + Number: prNum, State: "OPEN", HeadRefName: head, BaseRefName: base, + URL: fmt.Sprintf("https://github.com/o/r/pull/%d", prNum), + }, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{ + linkRemoteStack(7, linkPR(10, "branch-10"), linkPR(20, "branch-20")), + }, nil + }, + AddToStackFn: func(stackNumber int, prNumbers []int) (*github.RemoteStack, error) { + addPRs = prNumbers + return &github.RemoteStack{ID: 7, Number: 7}, nil + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"7", "feat-c", "feat-d"}) + 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, created, 2) + assert.Equal(t, "branch-20", created[0].base) // first new PR on the stack top + assert.Equal(t, "feat-c", created[0].head) + assert.Equal(t, "feat-c", created[1].base) // second new PR chains off the first + assert.Equal(t, "feat-d", created[1].head) + assert.Equal(t, []int{101, 102}, addPRs) + assert.Contains(t, output, "Added 2 PRs to stack #7") +} + +func TestLink_AddMode_AddToStack422(t *testing.T) { + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + return &github.PullRequest{ + Number: n, State: "OPEN", + HeadRefName: fmt.Sprintf("branch-%d", n), BaseRefName: "branch-20", + URL: fmt.Sprintf("https://github.com/o/r/pull/%d", n), + }, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{ + linkRemoteStack(7, linkPR(10, "branch-10"), linkPR(20, "branch-20")), + }, nil + }, + AddToStackFn: func(int, []int) (*github.RemoteStack, error) { + return nil, &api.HTTPError{StatusCode: 422, Message: "cannot append"} + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"7", "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, ErrAPIFailure) + assert.Contains(t, output, "Cannot add to stack") + assert.Contains(t, output, "cannot append") +} + +func TestLink_AddMode_AddToStack404_StackGone(t *testing.T) { + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + return &github.PullRequest{ + Number: n, State: "OPEN", + HeadRefName: fmt.Sprintf("branch-%d", n), BaseRefName: "branch-20", + URL: fmt.Sprintf("https://github.com/o/r/pull/%d", n), + }, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{ + linkRemoteStack(7, linkPR(10, "branch-10"), linkPR(20, "branch-20")), + }, nil + }, + AddToStackFn: func(int, []int) (*github.RemoteStack, error) { + return nil, &api.HTTPError{StatusCode: 404, Message: "Not Found"} + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"7", "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, ErrNotInStack) + assert.Contains(t, output, "no longer exists") +} + +func TestLink_AddMode_FetchesFullStackWhenListLacksHeadRefs(t *testing.T) { + var addPRs []int + var getStackCalls int + cfg, _, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + return &github.PullRequest{ + Number: n, State: "OPEN", + HeadRefName: fmt.Sprintf("branch-%d", n), BaseRefName: "branch-20", + URL: fmt.Sprintf("https://github.com/o/r/pull/%d", n), + }, nil + }, + ListStacksFn: func() ([]github.RemoteStack, error) { + // The list response carries PR numbers but omits per-PR head refs. + return []github.RemoteStack{{ + ID: 7, Number: 7, + PullRequests: []int{10, 20}, + PRDetails: []github.RemoteStackPR{ + {Number: 10, State: "open"}, + {Number: 20, State: "open"}, + }, + }}, nil + }, + GetStackFn: func(number int) (*github.RemoteStack, error) { + getStackCalls++ + s := linkRemoteStack(7, linkPR(10, "branch-10"), linkPR(20, "branch-20")) + return &s, nil + }, + AddToStackFn: func(stackNumber int, prNumbers []int) (*github.RemoteStack, error) { + addPRs = prNumbers + return &github.RemoteStack{ID: 7, Number: 7}, nil + }, + } + + cmd := LinkCmd(cfg) + cmd.SetArgs([]string{"7", "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, 1, getStackCalls) // fell back to fetch the full stack + assert.Equal(t, []int{30}, addPRs) + assert.Contains(t, output, "Added 1 PR to stack #7") +} diff --git a/docs/src/content/docs/faq.md b/docs/src/content/docs/faq.md index 744592b..890c520 100644 --- a/docs/src/content/docs/faq.md +++ b/docs/src/content/docs/faq.md @@ -255,7 +255,14 @@ This doesn't create any local tracking and only hits the APIs to create Stacked If the provided branches already have open PRs, `link` will use them. If not, it creates draft PRs by default 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: +To add more to the stack, pass the stack number (shown in the GitHub stack UI) as the first argument, followed by just the new PRs or branches — you no longer need to re-list the PRs already in the stack: + +```bash +# 42 is the stack number; change4 and change5 are appended to its top +gh stack link 42 change4 change5 +``` + +You can also pass the full list of PRs/branches (without a leading stack number) to create a stack or additively update an existing one: ```bash gh stack link 123 124 125 change4 change5 diff --git a/docs/src/content/docs/reference/cli.md b/docs/src/content/docs/reference/cli.md index f07f380..4c4be00 100644 --- a/docs/src/content/docs/reference/cli.md +++ b/docs/src/content/docs/reference/cli.md @@ -430,7 +430,7 @@ gh stack push --remote upstream Link PRs into a stack on GitHub without local tracking. ```sh -gh stack link [flags] [...] +gh stack link [flags] [...] ``` Creates or updates a stack on GitHub from branch names or PR numbers/URLs. 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. @@ -439,9 +439,11 @@ Arguments are provided in stack order (bottom to top). Branch arguments are auto 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. +To grow an existing stack without re-listing its PRs, pass a stack number (the number shown in the GitHub stack UI) as the first argument. The remaining arguments are appended to the top of that stack. Arguments already in the stack are skipped, and arguments that belong to a different stack are rejected. Because stack and PR numbers never overlap, a numeric first argument is treated as a stack only when it matches an existing stack — otherwise it is treated as a PR or branch. + | Flag | Description | |------|-------------| -| `--base ` | Base branch for the bottom of the stack (default: `main`) | +| `--base ` | Base branch for the bottom of the stack (default: `main`); ignored when adding to an existing stack | | `--open` | Mark new and existing PRs as ready for review | | `--remote ` | Remote to push to (defaults to auto-detected remote) | @@ -460,6 +462,10 @@ gh stack link https://github.com/owner/repo/pull/10 https://github.com/owner/rep # Add branches to an existing stack of PRs gh stack link 42 43 feature-auth feature-ui +# Append to the top of an existing stack by its stack number (no need to +# re-list the PRs already in stack 7) +gh stack link 7 48 feature-ui + # Use a different base branch and mark PRs as ready for review gh stack link --base develop --open feat-a feat-b feat-c ``` diff --git a/skills/gh-stack/SKILL.md b/skills/gh-stack/SKILL.md index 24ce604..30e45b2 100644 --- a/skills/gh-stack/SKILL.md +++ b/skills/gh-stack/SKILL.md @@ -61,7 +61,7 @@ git config remote.pushDefault origin # if multiple remotes exist (skips remo 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. +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. Provide at least two branches/PRs to create or update a stack, or a stack number followed by the new branches/PRs to append them to the top of an existing stack (e.g. `gh stack link 7 branch-c`). **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` @@ -573,7 +573,7 @@ gh stack submit --auto --open 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] [...] +gh stack link [flags] [...] ``` ```bash @@ -588,8 +588,14 @@ gh stack link 10 20 30 # Add branches to an existing stack of PRs gh stack link 42 43 feature-auth feature-ui + +# Append to the top of an existing stack by its stack number +# (7 is a stack number; only the new PRs/branches are listed) +gh stack link 7 48 feature-auth ``` +When the first argument is a stack number, the remaining arguments are appended to the top of that stack, so you don't have to re-list its current PRs. Arguments already in the stack are skipped; arguments in a different stack are rejected. A numeric first argument is treated as a stack only when it matches an existing stack — otherwise it is a PR or branch. + | Flag | Description | |------|---------| | `--base ` | Base branch for the bottom of the stack (default: `main`) |