From a5787b1467cab54550db7e19ab8c675d46d776b8 Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Fri, 13 Feb 2026 17:24:09 -0800 Subject: [PATCH] fix(submit): submit the entire stack by default `gh stack submit` now processes all tracked branches (the entire stack) in parent-before-child order instead of only the current branch and its descendants. This ensures parent branches are pushed to the remote before their children need them as PR bases, preventing the cryptic `HTTP 422: PullRequest.base is invalid` error. - Add `--from` flag to limit scope to a subtree (`--from=branch`) or restore old behavior (`--from` with no value = current branch) - Improve error message when PR creation fails due to missing base branch on the remote - Document `--from`, `-y/--yes`, and `-w/--web` flags in README Closes #38 --- README.md | 33 ++++--- cmd/submit.go | 94 ++++++++++++++----- cmd/submit_internal_test.go | 44 +++++++++ e2e/submit_test.go | 176 +++++++++++++++++++++++++++++++++++- 4 files changed, 313 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index f6e87cf..91685d4 100644 --- a/README.md +++ b/README.md @@ -114,13 +114,19 @@ What it _won't_ do is push back up to the remote; see the [next section](#creati ### Creating & Updating PRs -To create PRs for the `feature-auth` and `feature-auth-tests` branches, execute this from the `feature-auth` branch: +To create PRs for the entire stack, run from any branch: ```bash gh stack submit ``` -Whenever you need to push these branches again, or update the PRs, you can run `gh stack submit` again. +This pushes every branch in the stack (in parent-to-child order) and creates or updates their PRs. You can run it again whenever you need to push changes or update PRs. + +To submit only the current branch and its descendants (the old default), use `--from`: + +```bash +gh stack submit --from +``` > [!TIP] > @@ -254,26 +260,31 @@ The PR itself is not affected; this only removes the local tracking. ### submit -Restack, push, and create/update PRs for current branch and descendants. +Restack, push, and create/update PRs for the entire stack. -This is the primary workflow command. It performs three phases: +This is the primary workflow command. By default it processes **every tracked branch** in parent-before-child order. It performs three phases: -1. **Restack**: Rebase current branch and descendants onto their parents +1. **Restack**: Rebase affected branches onto their parents 2. **Push**: Force-push all affected branches (using `--force-with-lease`) 3. **PR**: Create PRs for branches without them; update PR bases for existing PRs PRs targeting non-trunk branches are created as drafts. When a PR's base changes to trunk (after its parent merges), you'll be prompted to mark it ready for review. +Use `--from` to limit the scope to a subtree instead of the full stack. A bare `--from` (no value) starts from the current branch, preserving the pre-v0.x behavior. + If a rebase conflict occurs, resolve it and run `gh stack continue`. #### submit Flags -| Flag | Description | -| ---------------- | ----------------------------------------------- | -| `--dry-run` | Show what would happen without doing it | -| `--current-only` | Only submit the current branch, not descendants | -| `--update-only` | Only update existing PRs, don't create new ones | -| `--push-only` | Skip PR creation/update, only restack and push | +| Flag | Description | +| ---------------- | ------------------------------------------------------------------------ | +| `--dry-run` | Show what would happen without doing it | +| `--from [branch]` | Submit from this branch toward leaves (bare `--from` = current branch) | +| `--current-only` | Only submit the current branch, not descendants | +| `--update-only` | Only update existing PRs, don't create new ones | +| `--push-only` | Skip PR creation/update, only restack and push | +| `-y, --yes` | Skip interactive prompts; use auto-generated PR title/description | +| `-w, --web` | Open created/updated PRs in web browser | ### restack diff --git a/cmd/submit.go b/cmd/submit.go index 126a357..7901503 100644 --- a/cmd/submit.go +++ b/cmd/submit.go @@ -20,12 +20,15 @@ import ( var submitCmd = &cobra.Command{ Use: "submit", - Short: "Restack, push, and create/update PRs for current branch and descendants", - Long: `Submit rebases the current branch and its descendants onto their parents, -pushes all affected branches, and creates or updates pull requests. + Short: "Restack, push, and create/update PRs for the entire stack", + Long: `Submit rebases, pushes, and creates or updates pull requests for all +branches in the stack. -This is the typical workflow command after making changes in a stack: -1. Restack: rebase current branch + descendants onto their parents +By default, submit processes every tracked branch (the entire stack) in +parent-before-child order. Use --from to limit the scope to a subtree. + +Phases: +1. Restack: rebase affected branches onto their parents 2. Push: force-push all affected branches (with --force-with-lease) 3. PR: create PRs for branches without them, update PR bases for those that have them @@ -40,6 +43,7 @@ var ( submitPushOnlyFlag bool submitYesFlag bool submitWebFlag bool + submitFromFlag string ) func init() { @@ -49,6 +53,8 @@ func init() { submitCmd.Flags().BoolVar(&submitPushOnlyFlag, "push-only", false, "skip PR creation/update, only restack and push") submitCmd.Flags().BoolVarP(&submitYesFlag, "yes", "y", false, "skip interactive prompts and use auto-generated title/description for PRs") submitCmd.Flags().BoolVarP(&submitWebFlag, "web", "w", false, "open created/updated PRs in web browser") + submitCmd.Flags().StringVar(&submitFromFlag, "from", "", "submit from this branch toward leaves (default: entire stack; bare --from = current branch)") + submitCmd.Flags().Lookup("from").NoOptDefVal = "HEAD" rootCmd.AddCommand(submitCmd) } @@ -62,6 +68,9 @@ func runSubmit(cmd *cobra.Command, args []string) error { if submitPushOnlyFlag && submitWebFlag { return fmt.Errorf("--push-only and --web cannot be used together: --push-only skips all PR operations") } + if submitFromFlag != "" && submitCurrentOnlyFlag { + return fmt.Errorf("--from and --current-only cannot be used together") + } cwd, err := os.Getwd() if err != nil { @@ -96,27 +105,52 @@ func runSubmit(cmd *cobra.Command, args []string) error { return err } - node := tree.FindNode(root, currentBranch) - if node == nil { - return fmt.Errorf("branch %q is not tracked in the stack\n\nTo add it, run:\n gh stack adopt %s # to stack on %s\n gh stack adopt -p # to stack on a different branch", currentBranch, trunk, trunk) - } - - // Collect branches to submit (current + descendants, but never trunk) + // Collect branches to submit. + // + // --current-only: only the current branch (no descendants, no ancestors). + // --from (bare): current branch + descendants (old default behavior). + // --from=: that branch + descendants. + // Default: entire stack (all trunk descendants). var branches []*tree.Node - if currentBranch == trunk { - // On trunk: only submit descendants, not trunk itself - if submitCurrentOnlyFlag { + if submitCurrentOnlyFlag { + // --current-only: submit only the current checked-out branch + if currentBranch == trunk { return fmt.Errorf("cannot submit trunk branch %q; switch to a stack branch or remove --current-only", trunk) } - branches = tree.GetDescendants(node) - if len(branches) == 0 { - return fmt.Errorf("no stack branches to submit; trunk %q has no descendants", trunk) + node := tree.FindNode(root, currentBranch) + if node == nil { + return fmt.Errorf("branch %q is not tracked in the stack\n\nTo add it, run:\n gh stack adopt %s # to stack on %s\n gh stack adopt -p # to stack on a different branch", currentBranch, trunk, trunk) } - } else { - // On a stack branch: submit it and optionally its descendants branches = append(branches, node) - if !submitCurrentOnlyFlag { - branches = append(branches, tree.GetDescendants(node)...) + } else { + // Determine the starting node for branch collection + var startNode *tree.Node + if submitFromFlag == "HEAD" { + // --from without value: resolve to current branch (old behavior) + startNode = tree.FindNode(root, currentBranch) + if startNode == nil { + return fmt.Errorf("branch %q is not tracked in the stack\n\nTo add it, run:\n gh stack adopt %s # to stack on %s\n gh stack adopt -p # to stack on a different branch", currentBranch, trunk, trunk) + } + } else if submitFromFlag != "" && submitFromFlag != trunk { + // --from=: use specified branch + startNode = tree.FindNode(root, submitFromFlag) + if startNode == nil { + return fmt.Errorf("branch %q is not tracked in the stack", submitFromFlag) + } + } else { + // Default (no --from, or --from=): entire stack + startNode = root + } + + // Collect branches from start node (never include trunk itself) + if startNode == root { + branches = tree.GetDescendants(root) + if len(branches) == 0 { + return fmt.Errorf("no stack branches to submit; trunk %q has no descendants", trunk) + } + } else { + branches = append(branches, startNode) + branches = append(branches, tree.GetDescendants(startNode)...) } } @@ -324,6 +358,12 @@ func createPRForBranch(g *git.Git, ghClient *github.Client, cfg *config.Config, prNum, adoptErr := adoptExistingPR(ghClient, cfg, root, branch, base, trunk, remoteBranches, s) return prNum, true, adoptErr } + // Detect missing base branch on remote and provide an actionable message + if isBaseBranchInvalidError(err) { + return 0, false, fmt.Errorf( + "base branch %q does not exist on the remote; push it first or run 'gh stack submit' to push the entire stack: %w", + base, err) + } return 0, false, err } @@ -524,6 +564,18 @@ func promptMarkPRReady(ghClient *github.Client, prNumber int, branch, trunk stri } } +// isBaseBranchInvalidError returns true if the error indicates that the PR base +// branch does not exist on the remote (GitHub returns HTTP 422 with +// "PullRequest.base is invalid" in this case). +func isBaseBranchInvalidError(err error) bool { + if err == nil { + return false + } + msg := err.Error() + return strings.Contains(msg, "PullRequest.base is invalid") || + strings.Contains(msg, "base is invalid") +} + // generatePRBody creates a PR description from the commits between base and head. // For a single commit: returns the commit body. // For multiple commits: returns each commit as a markdown section. diff --git a/cmd/submit_internal_test.go b/cmd/submit_internal_test.go index 5cd21d8..9ad8f9f 100644 --- a/cmd/submit_internal_test.go +++ b/cmd/submit_internal_test.go @@ -7,6 +7,7 @@ package cmd import ( + "fmt" "testing" ) @@ -216,3 +217,46 @@ func TestIsHorizontalRule(t *testing.T) { } } } + +func TestIsBaseBranchInvalidError(t *testing.T) { + tests := []struct { + name string + err error + want bool + }{ + { + name: "nil error", + err: nil, + want: false, + }, + { + name: "unrelated error", + err: fmt.Errorf("network timeout"), + want: false, + }, + { + name: "exact GitHub 422 error", + err: fmt.Errorf("failed to create PR: HTTP 422: Validation Failed (https://api.github.com/repos/owner/repo/pulls)\nPullRequest.base is invalid"), + want: true, + }, + { + name: "short form", + err: fmt.Errorf("base is invalid"), + want: true, + }, + { + name: "wrapped error", + err: fmt.Errorf("something went wrong: %w", fmt.Errorf("PullRequest.base is invalid")), + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := isBaseBranchInvalidError(tt.err) + if got != tt.want { + t.Errorf("isBaseBranchInvalidError() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/e2e/submit_test.go b/e2e/submit_test.go index 7ff84d6..74c2dfb 100644 --- a/e2e/submit_test.go +++ b/e2e/submit_test.go @@ -175,11 +175,30 @@ func TestSubmitRejectsUntrackedBranch(t *testing.T) { env.Git("checkout", "-b", "untracked-branch") env.CreateCommit("work") - // Submit should fail + // Default submit (entire stack) should fail because there are no stack branches result := env.Run("submit", "--dry-run") if result.Success() { - t.Error("expected submit to fail on untracked branch") + t.Error("expected submit to fail with no stack branches") + } + if !strings.Contains(result.Stderr, "no stack branches") { + t.Errorf("expected error about no stack branches, got: %s", result.Stderr) + } +} + +func TestSubmitFromRejectsUntrackedBranch(t *testing.T) { + env := NewTestEnvWithRemote(t) + env.MustRun("init") + + // Create an untracked branch + env.Git("checkout", "-b", "untracked-branch") + env.CreateCommit("work") + + // Bare --from on untracked branch should report it's not tracked + result := env.Run("submit", "--dry-run", "--from") + + if result.Success() { + t.Error("expected submit --from to fail on untracked branch") } if !strings.Contains(result.Stderr, "not tracked") { t.Errorf("expected error about untracked branch, got: %s", result.Stderr) @@ -319,3 +338,156 @@ func TestSubmitPushOnlyWithWebFails(t *testing.T) { t.Errorf("expected error about conflicting flags, got: %s", result.Stderr) } } + +func TestSubmitEntireStackDryRun(t *testing.T) { + env := NewTestEnvWithRemote(t) + env.MustRun("init") + + // Create 3-level stack: main -> feat-a -> feat-b -> feat-c + env.MustRun("create", "feat-a") + env.CreateCommit("a work") + + env.MustRun("create", "feat-b") + env.CreateCommit("b work") + + env.MustRun("create", "feat-c") + env.CreateCommit("c work") + + // Checkout middle branch + env.Git("checkout", "feat-b") + + // Submit without --from should process the entire stack + result := env.MustRun("submit", "--dry-run") + + output := result.Stdout + if !strings.Contains(output, "Would push feat-a") { + t.Error("expected feat-a (ancestor) in push output") + } + if !strings.Contains(output, "Would push feat-b") { + t.Error("expected feat-b in push output") + } + if !strings.Contains(output, "Would push feat-c") { + t.Error("expected feat-c (descendant) in push output") + } +} + +func TestSubmitFromFlagDryRun(t *testing.T) { + env := NewTestEnvWithRemote(t) + env.MustRun("init") + + // Create 3-level stack: main -> feat-a -> feat-b -> feat-c + env.MustRun("create", "feat-a") + env.CreateCommit("a work") + + env.MustRun("create", "feat-b") + env.CreateCommit("b work") + + env.MustRun("create", "feat-c") + env.CreateCommit("c work") + + // Stay on feat-c, but use --from=feat-a + result := env.MustRun("submit", "--dry-run", "--from=feat-a") + + output := result.Stdout + if !strings.Contains(output, "Would push feat-a") { + t.Error("expected feat-a in push output") + } + if !strings.Contains(output, "Would push feat-b") { + t.Error("expected feat-b in push output") + } + if !strings.Contains(output, "Would push feat-c") { + t.Error("expected feat-c in push output") + } +} + +func TestSubmitFromFlagCurrentBranchDryRun(t *testing.T) { + env := NewTestEnvWithRemote(t) + env.MustRun("init") + + // Create 3-level stack: main -> feat-a -> feat-b -> feat-c + env.MustRun("create", "feat-a") + env.CreateCommit("a work") + + env.MustRun("create", "feat-b") + env.CreateCommit("b work") + + env.MustRun("create", "feat-c") + env.CreateCommit("c work") + + // Checkout middle branch + env.Git("checkout", "feat-b") + + // Bare --from should use current branch (feat-b) + descendants only + result := env.MustRun("submit", "--dry-run", "--from") + + output := result.Stdout + if strings.Contains(output, "Would push feat-a") { + t.Error("feat-a should NOT be in output with bare --from on feat-b") + } + if !strings.Contains(output, "Would push feat-b") { + t.Error("expected feat-b in push output") + } + if !strings.Contains(output, "Would push feat-c") { + t.Error("expected feat-c in push output") + } +} + +func TestSubmitFromFlagUntrackedBranch(t *testing.T) { + env := NewTestEnvWithRemote(t) + env.MustRun("init") + + env.MustRun("create", "feat-a") + env.CreateCommit("a work") + + result := env.Run("submit", "--dry-run", "--from=nonexistent") + + if result.Success() { + t.Error("expected --from=nonexistent to fail") + } + if !strings.Contains(result.Stderr, "not tracked") { + t.Errorf("expected error about untracked branch, got: %s", result.Stderr) + } +} + +func TestSubmitFromAndCurrentOnlyMutualExclusion(t *testing.T) { + env := NewTestEnvWithRemote(t) + env.MustRun("init") + + env.MustRun("create", "feat-a") + env.CreateCommit("a work") + + result := env.Run("submit", "--dry-run", "--from=feat-a", "--current-only") + + if result.Success() { + t.Error("expected --from and --current-only to fail") + } + if !strings.Contains(result.Stderr, "--from and --current-only cannot be used together") { + t.Errorf("expected error about conflicting flags, got: %s", result.Stderr) + } +} + +func TestSubmitFromTrunkFallback(t *testing.T) { + env := NewTestEnvWithRemote(t) + env.MustRun("init") + + // Create stack: main -> feat-a -> feat-b + env.MustRun("create", "feat-a") + env.CreateCommit("a work") + + env.MustRun("create", "feat-b") + env.CreateCommit("b work") + + // --from=main should behave like default (entire stack) + result := env.MustRun("submit", "--dry-run", "--from=main") + + output := result.Stdout + if !strings.Contains(output, "Would push feat-a") { + t.Error("expected feat-a in push output") + } + if !strings.Contains(output, "Would push feat-b") { + t.Error("expected feat-b in push output") + } + if strings.Contains(output, "Would push main") { + t.Error("should not push trunk branch") + } +}