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") + } +}