From 3c6a15f74f2f680644e855f5ef1f8522ea8c849a Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Mon, 26 Oct 2026 15:23:40 -0400 Subject: [PATCH 1/3] Default stack pushes to atomic --- README.md | 14 ++- cmd/link.go | 9 +- cmd/link_test.go | 2 + cmd/push.go | 12 +-- cmd/push_test.go | 62 +++++++++++++- cmd/submit.go | 31 ++++--- cmd/submit_test.go | 100 +++++++++++----------- cmd/sync.go | 10 ++- cmd/sync_test.go | 2 + docs/src/content/docs/guides/workflows.md | 2 +- docs/src/content/docs/reference/cli.md | 14 ++- internal/git/gitops_test.go | 43 ++++++++++ skills/gh-stack/references/commands.md | 20 +++-- 13 files changed, 228 insertions(+), 93 deletions(-) diff --git a/README.md b/README.md index 3ac6e84e..911a9666 100644 --- a/README.md +++ b/README.md @@ -369,7 +369,7 @@ Performs a synchronization of the entire stack: 2. **Reconcile the remote stack** — mirrors the GitHub stack locally. When PRs have been added to the stack on GitHub (the remote is ahead of your local stack), their branches are pulled down and appended to your local stack automatically. When the local and remote stacks have genuinely diverged (for example, you added a branch locally while different PRs were added to the stack on GitHub), you are prompted to resolve (see [Diverged stacks](#diverged-stacks) below). In a non-interactive terminal a divergence aborts the sync (nothing is pushed or updated). 3. **Fast-forward trunk** — fast-forwards the trunk branch to match the remote (skips if diverged). 4. **Cascade rebase** — rebases all stack branches onto their updated parents (only if trunk moved). If a conflict is detected, all branches are restored to their original state, and you are advised to run `gh stack rebase` to resolve conflicts interactively. -5. **Push** — pushes all branches (uses `--force-with-lease` if a rebase occurred). +5. **Push** — pushes all branches atomically by default (uses `--force-with-lease` if a rebase occurred). 6. **Sync PRs** — syncs PR state from GitHub and reports the status of each PR. 7. **Sync the stack** — links the stack's open PRs into a stack on GitHub, creating the remote stack object if it doesn't exist yet or updating it if it's partially formed. This only happens when two or more PRs exist; sync never opens PRs (use `gh stack submit` for that). 8. **Prune** — in interactive terminals, prompts to delete local branches for merged PRs. Use `--prune` to prune automatically. @@ -390,6 +390,7 @@ In a non-interactive terminal, a divergence aborts the sync (exit success) witho |------|-------------| | `--remote ` | Remote to fetch from and push to (defaults to auto-detected remote) | | `--prune` | Delete local branches for merged PRs | +| `--no-atomic` | Allow partial branch updates | **Examples:** @@ -408,11 +409,14 @@ Push active branches in the current stack to the remote. gh stack push [flags] ``` -Pushes every active branch (excluding merged and queued branches) in one `git push` using explicit per-branch `--force-with-lease` checks. The update is not atomic: branches whose leases pass may update even if another branch is rejected. Fix the rejected branch and rerun the command; branches already updated will be unchanged. This command does not create or update pull requests — use `gh stack submit` for that. +Pushes every active branch (excluding merged and queued branches) in one atomic `git push` using explicit per-branch `--force-with-lease` checks. All branch updates succeed or fail together. This command does not create or update pull requests — use `gh stack submit` for that. + +All pushing commands (`push`, `submit`, `sync`, and `link`) accept `--no-atomic` to allow partial branch updates. Set `GH_STACK_NO_ATOMIC=1` to opt out by default; any non-empty value disables atomic pushes. An explicit `--no-atomic=false` overrides the environment. Failed atomic pushes never automatically retry non-atomically. | Flag | Description | |------|-------------| | `--remote ` | Remote to push to (defaults to auto-detected remote) | +| `--no-atomic` | Allow partial branch updates | **Examples:** @@ -429,7 +433,7 @@ Push all branches and create/update PRs and the stack on GitHub. gh stack submit [flags] ``` -Creates a Stacked PR for every branch in the stack, pushing branches to the remote. +Pushes all active branches together atomically before creating or updating PRs. Atomicity applies to Git branch updates, not PR or stack API operations. After creating PRs, `submit` automatically creates a **Stack** on GitHub to link the PRs together. If the stack already exists on GitHub (e.g., from a previous submit), new PRs will be added to the top of the stack. @@ -446,6 +450,7 @@ In the editor, new PRs default to ready for review; flip any PR to draft with th | `--auto` | Skip the editor and use auto-generated PR titles | | `--open` | Mark new and existing PRs as ready for review | | `--remote ` | Remote to push to (defaults to auto-detected remote) | +| `--no-atomic` | Allow partial branch updates | **Examples:** @@ -465,7 +470,7 @@ gh stack link [flags] [...] Creates or updates a stack on GitHub from branch names or PR numbers/URLs. 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. +Arguments are provided in stack order (bottom to top). Branch arguments are automatically pushed atomically 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. @@ -474,6 +479,7 @@ If the PRs are not yet in a stack, a new stack is created. If some of the PRs ar | `--base ` | Base branch for the bottom of the stack (defaults to the repository's default branch) | | `--open` | Mark new and existing PRs as ready for review | | `--remote ` | Remote to push to (defaults to auto-detected remote) | +| `--no-atomic` | Allow partial branch updates | **Examples:** diff --git a/cmd/link.go b/cmd/link.go index 163099d6..b9997cd8 100644 --- a/cmd/link.go +++ b/cmd/link.go @@ -3,6 +3,7 @@ package cmd import ( "errors" "fmt" + "os" "strconv" "github.com/cli/go-gh/v2/pkg/api" @@ -17,6 +18,7 @@ type linkOptions struct { base string open bool remote string + noAtomic bool baseChanged bool } @@ -40,11 +42,13 @@ command first checks if a PR with that number exists; if not, it treats the argument as a branch name. PR URLs are always resolved as pull requests (never as branch names). -Branch arguments are automatically pushed to the remote before +Branch arguments are automatically pushed atomically 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. +Use --no-atomic to allow partial branch updates. + 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). @@ -80,6 +84,7 @@ only when it matches an existing stack.`, cmd.Flags().StringVar(&opts.base, "base", "", "Base branch for the bottom of the stack (defaults to the repository default branch)") cmd.Flags().BoolVar(&opts.open, "open", false, "Mark new and existing PRs as ready for review") cmd.Flags().StringVar(&opts.remote, "remote", "", "Remote to push to (defaults to auto-detected remote)") + cmd.Flags().BoolVar(&opts.noAtomic, "no-atomic", os.Getenv("GH_STACK_NO_ATOMIC") != "", "Disable atomic pushes (also set by GH_STACK_NO_ATOMIC)") return cmd } @@ -466,7 +471,7 @@ func pushBranchArgs(cfg *config.Config, opts *linkOptions, args []string) error } 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 { + if err := git.Push(remote, branches, false, !opts.noAtomic); err != nil { cfg.Errorf("failed to push branches: %s", err) return ErrSilent } diff --git a/cmd/link_test.go b/cmd/link_test.go index 310f9ba9..b5b79cc0 100644 --- a/cmd/link_test.go +++ b/cmd/link_test.go @@ -1689,6 +1689,8 @@ func TestLink_PushesBranchesBeforeResolution(t *testing.T) { PushFn: func(remote string, branches []string, force, atomic bool) error { pushedRemote = remote pushedBranches = branches + assert.False(t, force) + assert.True(t, atomic) return nil }, }) diff --git a/cmd/push.go b/cmd/push.go index 019e202a..9c4451e8 100644 --- a/cmd/push.go +++ b/cmd/push.go @@ -2,6 +2,7 @@ package cmd import ( "errors" + "os" "github.com/github/gh-stack/internal/config" "github.com/github/gh-stack/internal/git" @@ -10,7 +11,8 @@ import ( ) type pushOptions struct { - remote string + remote string + noAtomic bool } func PushCmd(cfg *config.Config) *cobra.Command { @@ -21,9 +23,8 @@ func PushCmd(cfg *config.Config) *cobra.Command { Short: "Push active branches in the current stack to the remote", Long: `Push active branches in the current stack to the remote. -Uses explicit per-branch --force-with-lease checks. Updates are not atomic: a -branch may update even if another branch is rejected. Fix the rejected branch -and run the command again; branches already updated will be unchanged. +Uses explicit per-branch --force-with-lease checks and atomic updates: all +branch updates succeed or fail together. Use --no-atomic to allow partial updates. Merged and queued branches are automatically skipped.`, Example: ` # Push active stack branches to the default remote $ gh stack push @@ -36,6 +37,7 @@ Merged and queued branches are automatically skipped.`, } cmd.Flags().StringVar(&opts.remote, "remote", "", "Remote to push to (defaults to auto-detected remote)") + cmd.Flags().BoolVar(&opts.noAtomic, "no-atomic", os.Getenv("GH_STACK_NO_ATOMIC") != "", "Disable atomic pushes (also set by GH_STACK_NO_ATOMIC)") return cmd } @@ -108,7 +110,7 @@ func runPush(cfg *config.Config, opts *pushOptions) error { // remote yet. _ = git.FetchBranches(remote, activeBranches) cfg.Printf("Pushing %d %s to %s...", len(activeBranches), plural(len(activeBranches), "branch", "branches"), remote) - if err := git.Push(remote, activeBranches, true, false); err != nil { + if err := git.Push(remote, activeBranches, true, !opts.noAtomic); err != nil { cfg.Errorf("failed to push: %s", err) return ErrSilent } diff --git a/cmd/push_test.go b/cmd/push_test.go index 15fa1875..1d85cb28 100644 --- a/cmd/push_test.go +++ b/cmd/push_test.go @@ -9,6 +9,7 @@ import ( "github.com/github/gh-stack/internal/git" "github.com/github/gh-stack/internal/github" "github.com/github/gh-stack/internal/stack" + "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -62,11 +63,70 @@ func TestPush_PushesAllBranches(t *testing.T) { assert.Equal(t, "origin", pushCalls[0].remote) assert.Equal(t, []string{"b1", "b2"}, pushCalls[0].branches) assert.True(t, pushCalls[0].force) - assert.False(t, pushCalls[0].atomic) + assert.True(t, pushCalls[0].atomic) assert.Contains(t, output, "Pushed 2 branches") assert.Contains(t, output, "gh stack submit", "should hint about submit when branches have no PRs") } +func TestPush_AtomicOptions(t *testing.T) { + for _, command := range []struct { + name string + newCmd func(*config.Config) *cobra.Command + args []string + force bool + wantErr error + }{ + {"push", PushCmd, nil, true, ErrSilent}, + {"submit", SubmitCmd, []string{"--auto"}, true, ErrSilent}, + {"sync", SyncCmd, nil, false, nil}, + {"link", LinkCmd, []string{"b1", "b2"}, false, ErrSilent}, + } { + for _, tc := range []struct { + name string + env string + args []string + atomic bool + }{ + {"default", "", nil, true}, + {"flag", "", []string{"--no-atomic"}, false}, + {"environment", "1", nil, false}, + {"flag overrides environment", "1", []string{"--no-atomic=false"}, true}, + } { + t.Run(command.name+"/"+tc.name, func(t *testing.T) { + t.Setenv("GH_STACK_NO_ATOMIC", tc.env) + dir := t.TempDir() + writeStackFile(t, dir, stack.Stack{ + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{{Branch: "b1"}, {Branch: "b2"}}, + }) + var pushes []pushCall + mock := newSyncMock(dir, "b1") + mock.RootDirFn = func() (string, error) { return dir, nil } + mock.PushFn = func(remote string, branches []string, force, atomic bool) error { + pushes = append(pushes, pushCall{remote, branches, force, atomic}) + return assert.AnError + } + restore := git.SetOps(mock) + defer restore() + + cfg, outR, errR := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{} + cmd := command.newCmd(cfg) + args := append([]string(nil), command.args...) + cmd.SetArgs(append(args, tc.args...)) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + _, output := commandOutput(t, cfg, outR, errR) + + require.ErrorIs(t, err, command.wantErr, output) + require.Len(t, pushes, 1, "failed pushes must not retry non-atomically") + assert.Equal(t, pushCall{"origin", []string{"b1", "b2"}, command.force, tc.atomic}, pushes[0]) + }) + } + } +} + func TestPush_NoSubmitHintWhenPRsExist(t *testing.T) { s := stack.Stack{ Trunk: stack.BranchRef{Branch: "main"}, diff --git a/cmd/submit.go b/cmd/submit.go index 2d90d192..a8e741b9 100644 --- a/cmd/submit.go +++ b/cmd/submit.go @@ -3,6 +3,7 @@ package cmd import ( "errors" "fmt" + "os" "slices" "strconv" "strings" @@ -22,9 +23,10 @@ import ( ) type submitOptions struct { - auto bool - open bool - remote string + auto bool + open bool + remote string + noAtomic bool } func SubmitCmd(cfg *config.Config) *cobra.Command { @@ -46,11 +48,13 @@ after deleting the stack) and you deselect every new PR, press Ctrl+B (or click the "STACK N PRs" button) to link the existing open PRs into a stack. This command performs several steps: - 1. Pushes all branches to the remote + 1. Pushes all active branches to the remote atomically 2. Creates new PRs for the included branches 3. Updates base branches for existing PRs 4. Creates or updates the stack on GitHub +Use --no-atomic to allow partial branch updates. PR updates are not atomic. + In the editor, new PRs default to ready for review; switch any to draft with the "CREATE AS" toggle. With --auto, new PRs are created as drafts unless you pass --open.`, @@ -70,6 +74,7 @@ In the editor, new PRs default to ready for review; switch any to draft with the cmd.Flags().BoolVar(&opts.auto, "auto", false, "Use auto-generated PR titles without prompting") cmd.Flags().BoolVar(&opts.open, "open", false, "Mark new and existing PRs as ready for review") cmd.Flags().StringVar(&opts.remote, "remote", "", "Remote to push to (defaults to auto-detected remote)") + cmd.Flags().BoolVar(&opts.noAtomic, "no-atomic", os.Getenv("GH_STACK_NO_ATOMIC") != "", "Disable atomic pushes (also set by GH_STACK_NO_ATOMIC)") return cmd } @@ -237,10 +242,14 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error { drafts = collected } - // Push each branch and create/update its PR in stack order (bottom to top). - // Sequential pushing ensures each branch's base is up-to-date on the - // remote before the next branch is pushed, preventing race conditions. - cfg.Printf("Pushing to %s...", remote) + if len(activeBranches) > 0 { + cfg.Printf("Pushing to %s...", remote) + if err := git.Push(remote, activeBranches, true, !opts.noAtomic); err != nil { + cfg.Errorf("failed to push: %s", err) + return ErrSilent + } + } + allPRsSubmitted := len(queued) == 0 submittedPRs := make(map[string]int, len(activeBranches)) for i, b := range s.Branches { @@ -248,12 +257,6 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error { continue } - // Push this branch - if err := git.Push(remote, []string{b.Branch}, true, false); err != nil { - cfg.Errorf("failed to push %s: %s", b.Branch, err) - return ErrSilent - } - // Find or create PR, and fix base if needed baseBranch := s.ActiveBaseBranch(b.Branch) submitted, err := ensurePR(cfg, client, s, i, baseBranch, opts, templateContent, drafts) diff --git a/cmd/submit_test.go b/cmd/submit_test.go index 62868390..44e56f69 100644 --- a/cmd/submit_test.go +++ b/cmd/submit_test.go @@ -7,6 +7,7 @@ import ( "net/url" "os" "path/filepath" + "slices" "testing" "github.com/cli/go-gh/v2/pkg/api" @@ -126,6 +127,7 @@ func TestSubmit_CreatesPRsAndStack(t *testing.T) { return nil, nil // No existing PR }, CreatePRFn: func(base, head, title, body string, draft bool) (*github.PullRequest, error) { + require.Len(t, pushCalls, 1, "all branches must be pushed before creating PRs") createdPRs = append(createdPRs, head) prCounter++ return &github.PullRequest{ @@ -151,11 +153,12 @@ func TestSubmit_CreatesPRsAndStack(t *testing.T) { assert.NoError(t, err) - // Branches should be pushed (sequentially, one per branch) - require.Len(t, pushCalls, 2) + // Branches should be pushed together before creating PRs. + require.Len(t, pushCalls, 1) assert.Equal(t, "origin", pushCalls[0].remote) - assert.Equal(t, []string{"b1"}, pushCalls[0].branches) - assert.Equal(t, []string{"b2"}, pushCalls[1].branches) + assert.Equal(t, []string{"b1", "b2"}, pushCalls[0].branches) + assert.True(t, pushCalls[0].force) + assert.True(t, pushCalls[0].atomic) // PRs should be created assert.Equal(t, []string{"b1", "b2"}, createdPRs) @@ -331,7 +334,12 @@ func TestSubmit_PushFailure(t *testing.T) { defer restore() cfg, _, errR := config.NewTestConfig() - cfg.GitHubClientOverride = &github.MockClient{} + cfg.GitHubClientOverride = &github.MockClient{ + CreatePRFn: func(string, string, string, string, bool) (*github.PullRequest, error) { + t.Fatal("must not create PRs after a failed push") + return nil, nil + }, + } cmd := SubmitCmd(cfg) cmd.SetArgs([]string{"--auto"}) cmd.SetOut(io.Discard) @@ -489,9 +497,8 @@ func TestSubmit_ForksWhenRemoteStackFullyMerged(t *testing.T) { require.NoError(t, err) // Only the new branches are pushed; merged ones are left behind. - require.Len(t, pushCalls, 2) - assert.Equal(t, []string{"b3"}, pushCalls[0].branches) - assert.Equal(t, []string{"b4"}, pushCalls[1].branches) + require.Len(t, pushCalls, 1) + assert.Equal(t, []string{"b3", "b4"}, pushCalls[0].branches) // Fork messaging. assert.Contains(t, output, "Every PR in this stack has already been merged") @@ -1869,16 +1876,18 @@ func newPendingSubmitState(priorStackID string) *modify.StateFile { func TestSubmit_PendingModifyCompletion(t *testing.T) { for _, tc := range []struct { - name string - branches int - newPR bool - failure string - priorDeleted bool - mergedBase bool - retry bool - mergeOnRetry bool + name string + branches int + newPR bool + failure string + priorDeleted bool + mergedBase bool + retry bool + mergeOnRetry bool + mergeAfterUnstack bool }{ {name: "one existing PR", branches: 1}, + {name: "last PR merged while unstacking", branches: 1, mergeAfterUnstack: true}, {name: "one new PR", branches: 1, newPR: true}, {name: "retry after old stack deletion", branches: 1, newPR: true, priorDeleted: true}, {name: "one PR failed push", branches: 1, failure: "push"}, @@ -1950,8 +1959,9 @@ func TestSubmit_PendingModifyCompletion(t *testing.T) { mock := newSubmitMock(dir, "b1") var pushes []string mock.PushFn = func(_ string, branches []string, _, _ bool) error { - pushes = append(pushes, branches[0]) - if tc.failure == "push" && branches[0] == target { + require.NotEmpty(t, branches) + pushes = append(pushes, branches...) + if tc.failure == "push" && slices.Contains(branches, target) { return assert.AnError } return nil @@ -2017,6 +2027,10 @@ func TestSubmit_PendingModifyCompletion(t *testing.T) { require.Equal(t, 7, number) unstackCalls++ oldDeleted = true + if tc.mergeAfterUnstack { + prs[target].Merged = true + prs[target].State = "MERGED" + } if tc.mergedBase { return &github.RemoteStack{ID: 42, Number: 7, PullRequests: []int{9}}, false, nil } @@ -2079,7 +2093,12 @@ func TestSubmit_PendingModifyCompletion(t *testing.T) { } else { require.NoError(t, err, output) assert.NoFileExists(t, modify.StatePath(dir)) - assert.Len(t, pushes, tc.branches) + if tc.mergeAfterUnstack { + assert.Empty(t, pushes) + assert.Zero(t, baseUpdates) + } else { + assert.Len(t, pushes, tc.branches) + } saved, loadErr := stack.Load(dir) require.NoError(t, loadErr) assert.Equal(t, s.BranchNames(), saved.Stacks[0].BranchNames()) @@ -2094,7 +2113,7 @@ func TestSubmit_PendingModifyCompletion(t *testing.T) { assert.Contains(t, output, "Changes from modify submitted") if tc.newPR { assert.Equal(t, 1, createdPRs) - } else { + } else if !tc.mergeAfterUnstack { assert.Equal(t, 1, baseUpdates) } } @@ -2163,7 +2182,7 @@ func TestSubmit_PRFailureWithoutPendingModifyRemainsBestEffort(t *testing.T) { require.NoError(t, runSubmit(cfg, &submitOptions{auto: true})) _, output := commandOutput(t, cfg, outR, errR) - assert.Equal(t, 2, pushes) + assert.Equal(t, 1, pushes) assert.Equal(t, 2, creations) assert.Contains(t, output, "failed to create PR for b2") assert.NoFileExists(t, modify.StatePath(dir)) @@ -2520,7 +2539,7 @@ func TestClearPendingModifyState_NoFile(t *testing.T) { assert.False(t, modify.StateExists(gitDir)) } -func TestSubmit_WithPendingModify_SequentialPush(t *testing.T) { +func TestSubmit_WithPendingModify_AtomicPush(t *testing.T) { s := stack.Stack{ ID: "42", Number: 7, @@ -2543,7 +2562,7 @@ func TestSubmit_WithPendingModify_SequentialPush(t *testing.T) { mock := newSubmitMock(tmpDir, "b1") mock.PushFn = func(remote string, branches []string, force, atomic bool) error { pushCalls = append(pushCalls, pushCall{remote, branches, force, atomic}) - callOrder = append(callOrder, fmt.Sprintf("push:%s", branches[0])) + callOrder = append(callOrder, "push") return nil } @@ -2616,36 +2635,15 @@ func TestSubmit_WithPendingModify_SequentialPush(t *testing.T) { // Unstack called with old stack number assert.Equal(t, 7, unstackedNumber) - // Push called per-branch (3 separate calls, not 1 atomic call) - require.Len(t, pushCalls, 3, "should push each branch individually") - assert.Equal(t, []string{"b1"}, pushCalls[0].branches) - assert.Equal(t, []string{"b2"}, pushCalls[1].branches) - assert.Equal(t, []string{"b3"}, pushCalls[2].branches) - for _, pc := range pushCalls { - assert.False(t, pc.atomic, "sequential push should not use atomic mode") - } + require.Len(t, pushCalls, 1) + assert.Equal(t, []string{"b1", "b2", "b3"}, pushCalls[0].branches) + assert.True(t, pushCalls[0].force) + assert.True(t, pushCalls[0].atomic) // CreateStack called with all 3 PRs assert.Equal(t, []int{10, 11, 12}, createdStackPRs) - // Verify ordering: unstack before push, push before create_stack - assert.True(t, len(callOrder) >= 5, "expected at least 5 calls, got %d: %v", len(callOrder), callOrder) - deleteIdx := -1 - firstPushIdx := -1 - createIdx := -1 - for i, c := range callOrder { - if c == "unstack:7" && deleteIdx == -1 { - deleteIdx = i - } - if c == "push:b1" && firstPushIdx == -1 { - firstPushIdx = i - } - if c == "create_stack" && createIdx == -1 { - createIdx = i - } - } - assert.Greater(t, firstPushIdx, deleteIdx, "delete should happen before push") - assert.Greater(t, createIdx, firstPushIdx, "create_stack should happen after push") + assert.Equal(t, []string{"unstack:7", "push", "create_stack"}, callOrder) // State file should be cleared assert.False(t, modify.StateExists(tmpDir), "modify state file should be cleared after success") @@ -2711,9 +2709,7 @@ func TestSubmit_FetchesBeforePush(t *testing.T) { assert.NoError(t, err) assert.Equal(t, []string{"b1", "b2"}, fetchedBranches, "should fetch active branches") - // fetch must come before all pushes - require.True(t, len(callOrder) >= 3, "expected at least 3 calls (fetch + 2 pushes)") - assert.Equal(t, "fetch", callOrder[0], "fetch must happen before any push") + assert.Equal(t, []string{"fetch", "push"}, callOrder) } func TestSubmit_UsesPRTemplate(t *testing.T) { diff --git a/cmd/sync.go b/cmd/sync.go index ab955065..b5ece467 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -3,6 +3,7 @@ package cmd import ( "errors" "fmt" + "os" "strings" "github.com/cli/go-gh/v2/pkg/prompter" @@ -18,6 +19,7 @@ type syncOptions struct { remote string prune bool acceptBaseChange bool + noAtomic bool } func SyncCmd(cfg *config.Config) *cobra.Command { @@ -36,11 +38,14 @@ This command performs a safe synchronization: resolve a divergence in an interactive terminal 3. Fast-forwards the trunk branch to match the remote 4. Cascade-rebases stack branches onto their updated parents - 5. Pushes all branches atomically (using --force-with-lease --atomic) + 5. Pushes all branches atomically by default, using --force-with-lease + after a rebase 6. Syncs PR state from GitHub 7. Links the stack's open PRs into a stack on GitHub (creating or updating the remote stack object) when two or more PRs exist +Use --no-atomic to allow partial branch updates. + If PRs have been added to the stack on GitHub, their branches are pulled down and appended to your local stack so it mirrors the remote. A clean "remote is ahead" update happens automatically without prompting. If the @@ -77,6 +82,7 @@ the first active branch in the stack, or the trunk if all are merged.`, cmd.Flags().StringVar(&opts.remote, "remote", "", "Remote to fetch from and push to (defaults to auto-detected remote)") cmd.Flags().BoolVar(&opts.prune, "prune", false, "Delete local branches for merged PRs") cmd.Flags().BoolVar(&opts.acceptBaseChange, "accept-base-change", false, "Rebase onto a retargeted base even if the old base did not merge") + cmd.Flags().BoolVar(&opts.noAtomic, "no-atomic", os.Getenv("GH_STACK_NO_ATOMIC") != "", "Disable atomic pushes (also set by GH_STACK_NO_ATOMIC)") return cmd } @@ -315,7 +321,7 @@ func runSync(cfg *config.Config, opts *syncOptions) error { // Without rebase, try a normal push first. force := rebased cfg.Printf("Pushing %d %s to %s...", len(branches), plural(len(branches), "branch", "branches"), remote) - if err := git.Push(remote, branches, force, true); err != nil { + if err := git.Push(remote, branches, force, !opts.noAtomic); err != nil { if !force { cfg.Warningf("Push failed — branches may need force push after rebase") cfg.Printf(" Run `%s` to push with --force-with-lease.", diff --git a/cmd/sync_test.go b/cmd/sync_test.go index 2682c761..5d36fc06 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -134,6 +134,7 @@ func TestSync_TrunkAlreadyUpToDate(t *testing.T) { // Push should happen without force require.Len(t, pushCalls, 1) assert.False(t, pushCalls[0].force, "push should not use force when no rebase occurred") + assert.True(t, pushCalls[0].atomic) } // TestSync_TrunkUpToDate_StackStale verifies that when trunk is already up to @@ -710,6 +711,7 @@ func TestSync_PushForceFlagDependsOnRebase(t *testing.T) { require.Len(t, pushCalls, 1, "exactly one push call expected") assert.Equal(t, tt.expectedForce, pushCalls[0].force, "force flag should be %v when trunkMoved=%v", tt.expectedForce, tt.trunkMoved) + assert.True(t, pushCalls[0].atomic) }) } } diff --git a/docs/src/content/docs/guides/workflows.md b/docs/src/content/docs/guides/workflows.md index 297b666d..70d277c7 100644 --- a/docs/src/content/docs/guides/workflows.md +++ b/docs/src/content/docs/guides/workflows.md @@ -354,7 +354,7 @@ gh stack push This is equivalent but distinct from updating your branch using a merge commit. The key difference is that after changing a lower branch, rebase maintains a linear commit history so the unique set of commits on each branch have clean diffs. -`gh stack push` then handles the force push with explicit per-branch `--force-with-lease` checks. The multi-branch push is not atomic: branches whose leases pass may update even if another branch is rejected. Fix the rejected branch and rerun the command; branches already updated will be unchanged. +`gh stack push` then handles the force push with explicit per-branch `--force-with-lease` checks. The multi-branch push is atomic by default: all branch updates succeed or fail together. Use `--no-atomic` or set `GH_STACK_NO_ATOMIC=1` to allow partial branch updates. For a simpler all-in-one flow, `gh stack sync` combines fetch, rebase, and push into a single command — useful when you just need to pull in the latest upstream changes: diff --git a/docs/src/content/docs/reference/cli.md b/docs/src/content/docs/reference/cli.md index b89bcfcd..a102989d 100644 --- a/docs/src/content/docs/reference/cli.md +++ b/docs/src/content/docs/reference/cli.md @@ -348,8 +348,9 @@ gh stack submit [flags] | `--auto` | Skip the editor and use auto-generated PR titles | | `--open` | Create new PRs as ready for review instead of drafts, and mark existing PRs as ready for review | | `--remote ` | Remote to push to (defaults to auto-detected remote) | +| `--no-atomic` | Allow partial branch updates | -Creates a Stacked PR for every branch in the stack, pushing branches to the remote. After creating PRs, `submit` automatically creates a **Stack** on GitHub to link the PRs together. If the stack already exists on GitHub (e.g., from a previous submit), new PRs are added to the existing stack. +Pushes all active branches together atomically before creating or updating PRs. After creating PRs, `submit` automatically creates a **Stack** on GitHub to link the PRs together. If the stack already exists on GitHub (e.g., from a previous submit), new PRs are added to the existing stack. Atomicity applies to Git branch updates, not PR or stack API operations. If every PR in the stack has already been merged, that stack is complete and can't be extended. In that case `submit` automatically starts a **new** stack rooted at the trunk for your unmerged branches and creates it on GitHub, leaving the merged stack untouched. @@ -385,6 +386,7 @@ gh stack sync [flags] | `--remote ` | Remote to fetch from and push to (defaults to auto-detected remote) | | `--prune` | Delete local branches for merged PRs | | `--accept-base-change` | Rebase onto a base GitHub retargeted the stack to, even if the old base did not merge | +| `--no-atomic` | Allow partial branch updates | Performs a synchronization of the entire stack: @@ -392,7 +394,7 @@ Performs a synchronization of the entire stack: 2. **Reconcile the remote stack** — mirrors the GitHub stack locally. When PRs have been added to the stack on GitHub (the remote is ahead of your local stack), their branches are pulled down and appended to your local stack automatically. When the local and remote stacks have genuinely diverged (for example, you added a branch locally while different PRs were added to the stack on GitHub), you are prompted to resolve (see **Diverged stacks** below). In a non-interactive terminal a divergence aborts the sync (nothing is pushed or updated). 3. **Fast-forward trunk** — fast-forwards the trunk branch to match the remote (skips if diverged). 4. **Cascade rebase** — rebases all stack branches onto their updated parents (only if trunk moved). If a conflict is detected, all branches are restored to their original state, and you are advised to run `gh stack rebase` to resolve conflicts interactively. -5. **Push** — pushes all branches (uses `--force-with-lease` if a rebase occurred). +5. **Push** — pushes all branches atomically by default (uses `--force-with-lease` if a rebase occurred). 6. **Sync PRs** — syncs PR state from GitHub and reports the status of each PR. 7. **Sync the stack** — links the stack's open PRs into a stack on GitHub, creating the remote stack object if it doesn't exist yet or updating it if it's partially formed. This only happens when two or more PRs exist; sync never opens PRs (use `gh stack submit` for that). 8. **Prune** — in interactive terminals, prompts to delete local branches for merged PRs. Use `--prune` to prune automatically. @@ -500,13 +502,15 @@ gh stack push [flags] | Flag | Description | |------|-------------| | `--remote ` | Remote to push to (defaults to auto-detected remote) | +| `--no-atomic` | Allow partial branch updates | -Pushes every active branch (excluding merged and queued branches) in one `git push` using explicit per-branch `--force-with-lease` checks. The update is not atomic: branches whose leases pass may update even if another branch is rejected. Fix the rejected branch and rerun the command; branches already updated will be unchanged. This command does not create or update pull requests — use `gh stack submit` for that. +Pushes every active branch (excluding merged and queued branches) in one atomic `git push` using explicit per-branch `--force-with-lease` checks. All branch updates succeed or fail together. Use `--no-atomic` to allow partial branch updates. Failed atomic pushes never automatically retry non-atomically. This command does not create or update pull requests — use `gh stack submit` for that. **Examples:** ```sh gh stack push +gh stack push --no-atomic gh stack push --remote upstream ``` @@ -523,10 +527,11 @@ gh stack link [flags] [...] | `--base ` | Base branch for the bottom of the stack (defaults to the repository's default branch); 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) | +| `--no-atomic` | Allow partial branch updates | 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. -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. +Arguments are provided in stack order (bottom to top). Branch arguments are automatically pushed atomically 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. @@ -757,6 +762,7 @@ gh stack feedback "Support for reordering branches" | Variable | Values | Description | |----------|--------|-------------| +| `GH_STACK_NO_ATOMIC` | Any non-empty value | Disables atomic pushes for `push`, `submit`, `sync`, and `link`, allowing partial branch updates. Set to `1` to opt out; an explicit `--no-atomic=false` overrides the environment. | | `GH_STACK_THEME` | `auto` (default), `light`, `dark` | Controls the color palette of the interactive screens (`submit`, `modify`, `view`) and all colored command output. Colors adapt to your terminal background automatically; set this to force the light or dark palette (and the matching canvas) when a terminal doesn't report its background (some SSH or `tmux` setups). | | `GH_STACK_BACKGROUND` | `github` (default), `terminal` | Controls whether the full-screen screens (`submit`, `modify`, `view`) paint GitHub's canvas (`#0d1117` on dark terminals, `#ffffff` on light ones) behind themselves. Painting needs a truecolor terminal. The terminal's colors are restored when the screen closes and while `$EDITOR` is open. Set to `terminal` to keep your terminal's own background and text colors. | | `GH_STACK_HYPERLINKS` | `0`, `1` | Disables or enables OSC 8 hyperlinks for PR and stack references when terminal detection is incorrect. Text output falls back to full URLs; compact TUIs keep plain labels. Does not control application-handled clicks in `view` and `submit`. | diff --git a/internal/git/gitops_test.go b/internal/git/gitops_test.go index 71b7edcf..cb1822c7 100644 --- a/internal/git/gitops_test.go +++ b/internal/git/gitops_test.go @@ -318,6 +318,49 @@ func TestIntegration_Push_RemoteAdvancedByOther(t *testing.T) { assert.NotEqual(t, otherSHA, finalRemoteSHA, "remote should have advanced past original other SHA") } +func TestIntegration_Push_AtomicLeaseRejection(t *testing.T) { + for _, tc := range []struct { + name string + atomic bool + }{ + {"atomic", true}, + {"non-atomic", false}, + } { + t.Run(tc.name, func(t *testing.T) { + bareDir, cloneDir := setupBareAndClone(t) + restore := withGitDir(t, cloneDir) + defer restore() + d := &defaultOps{} + + gitExec(t, cloneDir, "branch", "b1") + gitExec(t, cloneDir, "branch", "b2") + gitExec(t, cloneDir, "push", "origin", "b1", "b2") + require.NoError(t, d.FetchBranches("origin", []string{"b1", "b2"})) + beforeB1 := remoteBranchSHA(t, bareDir, "b1") + + gitExec(t, cloneDir, "checkout", "b1") + gitExec(t, cloneDir, "commit", "--allow-empty", "-m", "local update") + gitExec(t, cloneDir, "branch", "-f", "b2", "b1") + localB1 := gitExec(t, cloneDir, "rev-parse", "b1") + + otherClone := filepath.Join(t.TempDir(), "other") + gitExec(t, ".", "clone", bareDir, otherClone) + gitExec(t, otherClone, "checkout", "b2") + gitExec(t, otherClone, "commit", "--allow-empty", "-m", "competing update") + gitExec(t, otherClone, "push", "origin", "b2") + competingB2 := remoteBranchSHA(t, bareDir, "b2") + + require.Error(t, d.Push("origin", []string{"b1", "b2"}, true, tc.atomic)) + wantB1 := beforeB1 + if !tc.atomic { + wantB1 = localB1 + } + assert.Equal(t, wantB1, remoteBranchSHA(t, bareDir, "b1")) + assert.Equal(t, competingB2, remoteBranchSHA(t, bareDir, "b2")) + }) + } +} + // Test 4: Brand-new branch, absent on remote. // Push should create the branch via empty-expect lease. func TestIntegration_Push_NewBranchAbsentOnRemote(t *testing.T) { diff --git a/skills/gh-stack/references/commands.md b/skills/gh-stack/references/commands.md index 89ed80b0..54fd09a5 100644 --- a/skills/gh-stack/references/commands.md +++ b/skills/gh-stack/references/commands.md @@ -51,21 +51,25 @@ selects a non-default trunk. ## push -Pushes every active (non-merged, non-queued) branch in one multi-ref push with per-branch +Pushes every active (non-merged, non-queued) branch in one atomic multi-ref push with per-branch `--force-with-lease`. -**Not atomic.** Some branches may update while another is rejected. A rejection means that branch -moved on the remote; fix that branch and rerun — rerunning is safe and skips what already landed. +All branch updates succeed or fail together. `push`, `submit`, `sync`, and `link` accept +`--no-atomic` to allow partial updates. Any non-empty `GH_STACK_NO_ATOMIC` (for example, `1`) +opts out by default; `--no-atomic=false` overrides the environment. Failed atomic pushes never +automatically retry non-atomically. `push` never creates or updates pull requests. Use `submit` for that. ## submit -Pushes each active branch, then creates a PR for every branch that lacks one, basing it on the +Pushes all active branches together, then creates a PR for every branch that lacks one, basing it on the first non-merged ancestor, then links them into a Stack on GitHub. -- **Not atomic.** Branches are pushed sequentially with per-branch `--force-with-lease`. If a later - push is rejected, earlier pushes and PR updates stand. Fix the rejection and rerun the same command. +- **Atomic pushes by default.** One multi-ref push with per-branch `--force-with-lease` precedes + the bottom-to-top PR creation/update loop, even when opting out of atomicity. A failed push + stops before that loop. PR and stack API operations are not atomic; later API failures do not + undo the push. - **A fully merged stack cannot be extended.** When every PR in the current stack is already merged, `submit` forks the remaining unmerged branches into a **new** stack rooted at the trunk and creates it on GitHub, leaving the merged stack untouched. @@ -88,7 +92,7 @@ branches managed by another tool. Worktrees alone do not require `link`: local t exists.** In that case the remaining arguments are appended to the top of that stack and you do not re-list its current PRs: `gh stack link 7 feature-c`. Arguments already in the stack are skipped; arguments belonging to a different stack are rejected. -- Branch arguments are pushed automatically (non-force, atomic). Missing PRs are created with +- Branch arguments are pushed automatically (non-force, atomic by default). Missing PRs are created with auto-generated titles and correctly chained bases; existing PRs with a wrong base are corrected. - Stack membership is **additive only** — `link` never removes a PR from a stack. @@ -104,7 +108,7 @@ The routine command. Steps, in order: from its remote, or a branch no longer contains its expected parent. Merged PRs are handled automatically. On conflict, **all branches are restored** to their pre-rebase state and the command exits **3**. -5. **Push** all active branches, atomically. +5. **Push** all active branches, atomically by default. 6. **Refresh PR state** from GitHub. 7. **Sync the stack object** — link open PRs into a stack, additively. Only when two or more PRs exist. `sync` never opens PRs; that is `submit`. From 8c313e80b2393d026963191e4316cb48b4502850 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Mon, 26 Oct 2026 16:27:35 -0400 Subject: [PATCH 2/3] Protect reordered PRs during submit --- README.md | 2 +- cmd/link_test.go | 1 + cmd/push_test.go | 1 + cmd/submit.go | 36 ++++++++++++++++ cmd/submit_test.go | 60 ++++++++++++++++++++++---- cmd/sync_test.go | 2 + docs/src/content/docs/reference/cli.md | 2 +- 7 files changed, 93 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 911a9666..8cd3ee72 100644 --- a/README.md +++ b/README.md @@ -433,7 +433,7 @@ Push all branches and create/update PRs and the stack on GitHub. gh stack submit [flags] ``` -Pushes all active branches together atomically before creating or updating PRs. Atomicity applies to Git branch updates, not PR or stack API operations. +Pushes all active branches together atomically before creating PRs or setting their final bases. For pending reorders, at-risk PRs temporarily target the trunk while branches are pushed. Atomicity applies to Git branch updates, not PR or stack API operations. After creating PRs, `submit` automatically creates a **Stack** on GitHub to link the PRs together. If the stack already exists on GitHub (e.g., from a previous submit), new PRs will be added to the top of the stack. diff --git a/cmd/link_test.go b/cmd/link_test.go index b5b79cc0..d6528460 100644 --- a/cmd/link_test.go +++ b/cmd/link_test.go @@ -1679,6 +1679,7 @@ func TestLink_UpdateDeletedStack_FallsBackToCreate(t *testing.T) { } func TestLink_PushesBranchesBeforeResolution(t *testing.T) { + t.Setenv("GH_STACK_NO_ATOMIC", "") var pushedBranches []string var pushedRemote string diff --git a/cmd/push_test.go b/cmd/push_test.go index 1d85cb28..c4a7ac2b 100644 --- a/cmd/push_test.go +++ b/cmd/push_test.go @@ -25,6 +25,7 @@ func newPushMock(tmpDir string, currentBranch string) *git.MockOps { } func TestPush_PushesAllBranches(t *testing.T) { + t.Setenv("GH_STACK_NO_ATOMIC", "") s := stack.Stack{ Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{ diff --git a/cmd/submit.go b/cmd/submit.go index a8e741b9..564ffd96 100644 --- a/cmd/submit.go +++ b/cmd/submit.go @@ -242,6 +242,13 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error { drafts = collected } + if pendingModify { + if err := prepareReorderedPRs(cfg, client, s); err != nil { + cfg.Errorf("failed to prepare reordered PRs: %s", err) + return ErrSilent + } + } + if len(activeBranches) > 0 { cfg.Printf("Pushing to %s...", remote) if err := git.Push(remote, activeBranches, true, !opts.noAtomic); err != nil { @@ -307,6 +314,35 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error { return nil } +// Reversed PR relationships can cause indirect merges when the new heads are +// pushed. Use the unchanged trunk until ensurePR restores the intended bases. +func prepareReorderedPRs(cfg *config.Config, client github.ClientOps, s *stack.Stack) error { + for i, b := range s.Branches { + if b.IsSkipped() { + continue + } + pr, err := client.FindPRForBranch(b.Branch) + if err != nil { + return fmt.Errorf("looking up PR for %s: %w", b.Branch, err) + } + if pr == nil || s.IndexOf(pr.BaseRefName) <= i { + continue + } + if pr.IsAutoMergeEnabled() { + if err := client.DisableAutoMerge(pr.ID); err != nil { + return fmt.Errorf("disabling auto-merge for PR #%d: %w", pr.Number, err) + } + cfg.Warningf("Disabled auto-merge for PR %s (incompatible with stacked PRs)", + cfg.PRLink(pr.Number, pr.URL)) + } + if err := client.UpdatePRBase(pr.Number, s.Trunk.Branch); err != nil { + return fmt.Errorf("temporarily setting base of PR #%d to %s: %w", pr.Number, s.Trunk.Branch, err) + } + cfg.Successf("Temporarily set base of PR %s to %s", cfg.PRLink(pr.Number, pr.URL), s.Trunk.Branch) + } + return nil +} + // collectPRDrafts loads branch display data and runs the interactive submit TUI // so the user can choose which new branches become PRs and draft each one. It // returns the per-branch overrides, whether the user cancelled, and any error. diff --git a/cmd/submit_test.go b/cmd/submit_test.go index 44e56f69..8c2c50ff 100644 --- a/cmd/submit_test.go +++ b/cmd/submit_test.go @@ -94,6 +94,7 @@ func newSubmitMock(tmpDir string, currentBranch string) *git.MockOps { } func TestSubmit_CreatesPRsAndStack(t *testing.T) { + t.Setenv("GH_STACK_NO_ATOMIC", "") s := stack.Stack{ Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{ @@ -1885,6 +1886,8 @@ func TestSubmit_PendingModifyCompletion(t *testing.T) { retry bool mergeOnRetry bool mergeAfterUnstack bool + reorder bool + noAtomic bool }{ {name: "one existing PR", branches: 1}, {name: "last PR merged while unstacking", branches: 1, mergeAfterUnstack: true}, @@ -1901,6 +1904,10 @@ func TestSubmit_PendingModifyCompletion(t *testing.T) { {name: "one PR lost during final refresh", branches: 1, failure: "refresh"}, {name: "one PR after partial unstack", branches: 1, mergedBase: true}, {name: "multiple PRs complete", branches: 3}, + {name: "reordered PRs", branches: 3, reorder: true}, + {name: "reordered PRs non-atomic", branches: 3, reorder: true, noAtomic: true}, + {name: "reordered PRs preparation fails", branches: 3, reorder: true, failure: "prepare", retry: true}, + {name: "reordered PRs push fails", branches: 3, reorder: true, failure: "push", retry: true}, {name: "multiple PRs after partial unstack", branches: 3, mergedBase: true}, {name: "retry push after partial unstack", branches: 3, mergedBase: true, failure: "push", retry: true}, {name: "released PR merged before retry", branches: 3, mergedBase: true, failure: "push", retry: true, mergeOnRetry: true}, @@ -1933,14 +1940,14 @@ func TestSubmit_PendingModifyCompletion(t *testing.T) { if i > 0 { base = fmt.Sprintf("b%d", i) } - if name == target { + if name == target && !tc.reorder { base = "removed" } prs[name] = &github.PullRequest{ Number: 10 + i, ID: fmt.Sprintf("PR_%d", 10+i), State: "OPEN", HeadRefName: name, BaseRefName: base, } - if tc.failure == "auto-merge" && name == target { + if (tc.failure == "auto-merge" || tc.reorder) && name == target { prs[name].AutoMergeRequest = &github.AutoMergeRequest{EnabledAt: "2026-09-29T00:00:00Z"} } prs[name].IsDraft = tc.failure == "ready" && name == target @@ -1949,6 +1956,9 @@ func TestSubmit_PendingModifyCompletion(t *testing.T) { } s.Branches = append(s.Branches, br) } + if tc.reorder { + s.Branches[1], s.Branches[2] = s.Branches[2], s.Branches[1] + } writeStackFile(t, dir, s) pending := newPendingSubmitState("42") if tc.priorDeleted { @@ -1958,8 +1968,12 @@ func TestSubmit_PendingModifyCompletion(t *testing.T) { saveModifyState(t, dir, pending) mock := newSubmitMock(dir, "b1") var pushes []string - mock.PushFn = func(_ string, branches []string, _, _ bool) error { + mock.PushFn = func(_ string, branches []string, _, atomic bool) error { require.NotEmpty(t, branches) + if tc.reorder { + assert.Equal(t, "main", prs[target].BaseRefName, "detach the reversed PR base before pushing") + assert.Equal(t, !tc.noAtomic, atomic) + } pushes = append(pushes, branches...) if tc.failure == "push" && slices.Contains(branches, target) { return assert.AnError @@ -2050,11 +2064,20 @@ func TestSubmit_PendingModifyCompletion(t *testing.T) { }, UpdatePRBaseFn: func(number int, base string) error { baseUpdates++ - if tc.failure == "base" { + if tc.failure == "base" || tc.failure == "prepare" { return assert.AnError } - require.Equal(t, prs[target].Number, number) - prs[target].BaseRefName = base + if !tc.reorder { + require.Equal(t, prs[target].Number, number) + } + for _, pr := range prs { + if pr.Number == number { + if tc.reorder { + assert.False(t, pr.IsAutoMergeEnabled(), "disable auto-merge before changing bases") + } + pr.BaseRefName = base + } + } if tc.failure == "save" { var err error catalogLock, err = stack.Lock(dir) @@ -2062,7 +2085,14 @@ func TestSubmit_PendingModifyCompletion(t *testing.T) { } return nil }, - DisableAutoMergeFn: func(string) error { return assert.AnError }, + DisableAutoMergeFn: func(id string) error { + if tc.failure == "auto-merge" { + return assert.AnError + } + require.Equal(t, prs[target].ID, id) + prs[target].AutoMergeRequest = nil + return nil + }, MarkPRReadyForReviewFn: func(string) error { return assert.AnError }, CreateStackFn: func(numbers []int) (*github.RemoteStack, error) { require.GreaterOrEqual(t, len(numbers), 2) @@ -2077,7 +2107,7 @@ func TestSubmit_PendingModifyCompletion(t *testing.T) { cfg, outR, errR := config.NewTestConfig() cfg.GitHubClientOverride = client - err := runSubmit(cfg, &submitOptions{auto: true, open: tc.failure == "ready"}) + err := runSubmit(cfg, &submitOptions{auto: true, open: tc.failure == "ready", noAtomic: tc.noAtomic}) _, output := commandOutput(t, cfg, outR, errR) if tc.failure != "" { @@ -2090,6 +2120,9 @@ func TestSubmit_PendingModifyCompletion(t *testing.T) { assert.NotContains(t, output, "Stack recreated") assert.NotContains(t, output, "Changes from modify submitted") assert.Nil(t, replacement, "an incomplete modification must not create a partial replacement stack") + if tc.failure == "prepare" { + assert.Empty(t, pushes, "do not push unless all reversed bases were detached") + } } else { require.NoError(t, err, output) assert.NoFileExists(t, modify.StatePath(dir)) @@ -2113,6 +2146,8 @@ func TestSubmit_PendingModifyCompletion(t *testing.T) { assert.Contains(t, output, "Changes from modify submitted") if tc.newPR { assert.Equal(t, 1, createdPRs) + } else if tc.reorder { + assert.Equal(t, 3, baseUpdates) } else if !tc.mergeAfterUnstack { assert.Equal(t, 1, baseUpdates) } @@ -2129,7 +2164,7 @@ func TestSubmit_PendingModifyCompletion(t *testing.T) { pushStart := len(pushes) retryCfg, retryOut, retryErr := config.NewTestConfig() retryCfg.GitHubClientOverride = client - err := runSubmit(retryCfg, &submitOptions{auto: true}) + err := runSubmit(retryCfg, &submitOptions{auto: true, noAtomic: tc.noAtomic}) _, diagnostics := commandOutput(t, retryCfg, retryOut, retryErr) require.NoError(t, err, diagnostics) assert.NoFileExists(t, modify.StatePath(dir)) @@ -2153,6 +2188,12 @@ func TestSubmit_PendingModifyCompletion(t *testing.T) { } else { assert.Equal(t, 1, unstackCalls) } + if tc.reorder { + assert.Equal(t, "b1", prs["b3"].BaseRefName) + assert.Equal(t, "b3", prs["b2"].BaseRefName) + require.NotNil(t, replacement) + assert.Equal(t, []int{10, 12, 11}, replacement.PullRequests) + } }) } } @@ -2540,6 +2581,7 @@ func TestClearPendingModifyState_NoFile(t *testing.T) { } func TestSubmit_WithPendingModify_AtomicPush(t *testing.T) { + t.Setenv("GH_STACK_NO_ATOMIC", "") s := stack.Stack{ ID: "42", Number: 7, diff --git a/cmd/sync_test.go b/cmd/sync_test.go index 5d36fc06..063b7e91 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -76,6 +76,7 @@ func newSyncMock(tmpDir string, currentBranch string) *git.MockOps { // TestSync_TrunkAlreadyUpToDate verifies that when trunk and origin/trunk have // the same SHA, no rebase occurs and push is normal (not force). func TestSync_TrunkAlreadyUpToDate(t *testing.T) { + t.Setenv("GH_STACK_NO_ATOMIC", "") s := stack.Stack{ Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{ @@ -642,6 +643,7 @@ func TestSync_NoRebaseWhenTrunkDidntMove(t *testing.T) { // TestSync_PushForceFlagDependsOnRebase verifies that the force flag on Push // correlates with whether a rebase actually happened. func TestSync_PushForceFlagDependsOnRebase(t *testing.T) { + t.Setenv("GH_STACK_NO_ATOMIC", "") tests := []struct { name string trunkMoved bool diff --git a/docs/src/content/docs/reference/cli.md b/docs/src/content/docs/reference/cli.md index a102989d..acc8ec1b 100644 --- a/docs/src/content/docs/reference/cli.md +++ b/docs/src/content/docs/reference/cli.md @@ -350,7 +350,7 @@ gh stack submit [flags] | `--remote ` | Remote to push to (defaults to auto-detected remote) | | `--no-atomic` | Allow partial branch updates | -Pushes all active branches together atomically before creating or updating PRs. After creating PRs, `submit` automatically creates a **Stack** on GitHub to link the PRs together. If the stack already exists on GitHub (e.g., from a previous submit), new PRs are added to the existing stack. Atomicity applies to Git branch updates, not PR or stack API operations. +Pushes all active branches together atomically before creating PRs or setting their final bases. For pending reorders, at-risk PRs temporarily target the trunk while branches are pushed. After creating PRs, `submit` automatically creates a **Stack** on GitHub to link the PRs together. If the stack already exists on GitHub (e.g., from a previous submit), new PRs are added to the existing stack. Atomicity applies to Git branch updates, not PR or stack API operations. If every PR in the stack has already been merged, that stack is complete and can't be extended. In that case `submit` automatically starts a **new** stack rooted at the trunk for your unmerged branches and creates it on GitHub, leaving the merged stack untouched. From 2cf4bc96a23592a849df5e25760b90ecf075d5ba Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Mon, 26 Oct 2026 16:47:25 -0400 Subject: [PATCH 3/3] Fix retargeting hints after theme update --- cmd/checkout.go | 2 +- cmd/utils.go | 2 +- cmd/view.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/checkout.go b/cmd/checkout.go index 06c91b9f..521dfae6 100644 --- a/cmd/checkout.go +++ b/cmd/checkout.go @@ -477,7 +477,7 @@ func reconcileAndImportRemoteStack(cfg *config.Config, client github.ClientOps, } if base := remoteStack.Base.Ref; base != "" && localStack.Trunk.Branch != base { cfg.Warningf("GitHub retargeted this stack%s from %s to %s", linkedStackLabel(cfg, remoteStack.Number), localStack.Trunk.Branch, base) - cfg.Printf(" Run `%s` to rebase the stack onto %s", cfg.ColorCyan("gh stack sync"), base) + cfg.Printf(" Run `%s` to rebase the stack onto %s", cfg.ColorAccent("gh stack sync"), base) } else { cfg.Successf("Local stack matches remote%s", linkedStackLabel(cfg, remoteStack.Number)) } diff --git a/cmd/utils.go b/cmd/utils.go index af456dc0..db43aa93 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -1149,7 +1149,7 @@ func requireCurrentBase(cfg *config.Config, s *stack.Stack, remote string) error return ErrAPIFailure } if newBase != "" { - cfg.Errorf("run `%s` to rebase the stack onto %s before publishing", cfg.ColorCyan("gh stack sync"), newBase) + cfg.Errorf("run `%s` to rebase the stack onto %s before publishing", cfg.ColorAccent("gh stack sync"), newBase) return ErrSilent } return nil diff --git a/cmd/view.go b/cmd/view.go index 8fcd0c47..85093d15 100644 --- a/cmd/view.go +++ b/cmd/view.go @@ -143,7 +143,7 @@ func warnRetargeted(cfg *config.Config, s *stack.Stack) bool { remote, _ := git.ResolveRemote(s.Trunk.Branch) newBase, _ := retargetedBase(cfg, s, remote) if newBase != "" { - cfg.Printf(" Run `%s` to rebase the stack onto %s", cfg.ColorCyan("gh stack sync"), newBase) + cfg.Printf(" Run `%s` to rebase the stack onto %s", cfg.ColorAccent("gh stack sync"), newBase) } return newBase != "" }