Skip to content

Commit 2a49e67

Browse files
authored
Default to atomic git pushes (#596)
* Default stack pushes to atomic * Protect reordered PRs during submit * Fix retargeting hints after theme update
1 parent 3f8c957 commit 2a49e67

16 files changed

Lines changed: 322 additions & 105 deletions

File tree

‎README.md‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ Performs a synchronization of the entire stack:
369369
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).
370370
3. **Fast-forward trunk** — fast-forwards the trunk branch to match the remote (skips if diverged).
371371
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.
372-
5. **Push** — pushes all branches (uses `--force-with-lease` if a rebase occurred).
372+
5. **Push** — pushes all branches atomically by default (uses `--force-with-lease` if a rebase occurred).
373373
6. **Sync PRs** — syncs PR state from GitHub and reports the status of each PR.
374374
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).
375375
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
390390
|------|-------------|
391391
| `--remote <name>` | Remote to fetch from and push to (defaults to auto-detected remote) |
392392
| `--prune` | Delete local branches for merged PRs |
393+
| `--no-atomic` | Allow partial branch updates |
393394

394395
**Examples:**
395396

@@ -408,11 +409,14 @@ Push active branches in the current stack to the remote.
408409
gh stack push [flags]
409410
```
410411

411-
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.
412+
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.
413+
414+
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.
412415

413416
| Flag | Description |
414417
|------|-------------|
415418
| `--remote <name>` | Remote to push to (defaults to auto-detected remote) |
419+
| `--no-atomic` | Allow partial branch updates |
416420

417421
**Examples:**
418422

@@ -429,7 +433,7 @@ Push all branches and create/update PRs and the stack on GitHub.
429433
gh stack submit [flags]
430434
```
431435

432-
Creates a Stacked PR for every branch in the stack, pushing branches to the remote.
436+
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.
433437

434438
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.
435439

@@ -446,6 +450,7 @@ In the editor, new PRs default to ready for review; flip any PR to draft with th
446450
| `--auto` | Skip the editor and use auto-generated PR titles |
447451
| `--open` | Mark new and existing PRs as ready for review |
448452
| `--remote <name>` | Remote to push to (defaults to auto-detected remote) |
453+
| `--no-atomic` | Allow partial branch updates |
449454

450455
**Examples:**
451456

@@ -465,7 +470,7 @@ gh stack link [flags] <branch-or-pr> <branch-or-pr> [...]
465470

466471
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.
467472

468-
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.
473+
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.
469474

470475
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.
471476

@@ -474,6 +479,7 @@ If the PRs are not yet in a stack, a new stack is created. If some of the PRs ar
474479
| `--base <branch>` | Base branch for the bottom of the stack (defaults to the repository's default branch) |
475480
| `--open` | Mark new and existing PRs as ready for review |
476481
| `--remote <name>` | Remote to push to (defaults to auto-detected remote) |
482+
| `--no-atomic` | Allow partial branch updates |
477483

478484
**Examples:**
479485

‎cmd/checkout.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -477,7 +477,7 @@ func reconcileAndImportRemoteStack(cfg *config.Config, client github.ClientOps,
477477
}
478478
if base := remoteStack.Base.Ref; base != "" && localStack.Trunk.Branch != base {
479479
cfg.Warningf("GitHub retargeted this stack%s from %s to %s", linkedStackLabel(cfg, remoteStack.Number), localStack.Trunk.Branch, base)
480-
cfg.Printf(" Run `%s` to rebase the stack onto %s", cfg.ColorCyan("gh stack sync"), base)
480+
cfg.Printf(" Run `%s` to rebase the stack onto %s", cfg.ColorAccent("gh stack sync"), base)
481481
} else {
482482
cfg.Successf("Local stack matches remote%s", linkedStackLabel(cfg, remoteStack.Number))
483483
}

‎cmd/link.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package cmd
33
import (
44
"errors"
55
"fmt"
6+
"os"
67
"strconv"
78

89
"github.com/cli/go-gh/v2/pkg/api"
@@ -17,6 +18,7 @@ type linkOptions struct {
1718
base string
1819
open bool
1920
remote string
21+
noAtomic bool
2022
baseChanged bool
2123
}
2224

@@ -40,11 +42,13 @@ command first checks if a PR with that number exists; if not, it
4042
treats the argument as a branch name. PR URLs are always resolved
4143
as pull requests (never as branch names).
4244
43-
Branch arguments are automatically pushed to the remote before
45+
Branch arguments are automatically pushed atomically to the remote before
4446
creating or looking up PRs. For branches that already have open PRs,
4547
those PRs are used. For branches without PRs, new PRs are created
4648
automatically with the correct base branch chaining.
4749
50+
Use --no-atomic to allow partial branch updates.
51+
4852
If the PRs are not yet in a stack, a new stack is created. If some of
4953
the PRs are already in a stack, the existing stack is updated to include
5054
the new PRs (existing PRs are never removed).
@@ -80,6 +84,7 @@ only when it matches an existing stack.`,
8084
cmd.Flags().StringVar(&opts.base, "base", "", "Base branch for the bottom of the stack (defaults to the repository default branch)")
8185
cmd.Flags().BoolVar(&opts.open, "open", false, "Mark new and existing PRs as ready for review")
8286
cmd.Flags().StringVar(&opts.remote, "remote", "", "Remote to push to (defaults to auto-detected remote)")
87+
cmd.Flags().BoolVar(&opts.noAtomic, "no-atomic", os.Getenv("GH_STACK_NO_ATOMIC") != "", "Disable atomic pushes (also set by GH_STACK_NO_ATOMIC)")
8388

8489
return cmd
8590
}
@@ -466,7 +471,7 @@ func pushBranchArgs(cfg *config.Config, opts *linkOptions, args []string) error
466471
}
467472

468473
cfg.Printf("Pushing %d %s to %s...", len(branches), plural(len(branches), "branch", "branches"), remote)
469-
if err := git.Push(remote, branches, false, true); err != nil {
474+
if err := git.Push(remote, branches, false, !opts.noAtomic); err != nil {
470475
cfg.Errorf("failed to push branches: %s", err)
471476
return ErrSilent
472477
}

‎cmd/link_test.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1679,6 +1679,7 @@ func TestLink_UpdateDeletedStack_FallsBackToCreate(t *testing.T) {
16791679
}
16801680

16811681
func TestLink_PushesBranchesBeforeResolution(t *testing.T) {
1682+
t.Setenv("GH_STACK_NO_ATOMIC", "")
16821683
var pushedBranches []string
16831684
var pushedRemote string
16841685

@@ -1689,6 +1690,8 @@ func TestLink_PushesBranchesBeforeResolution(t *testing.T) {
16891690
PushFn: func(remote string, branches []string, force, atomic bool) error {
16901691
pushedRemote = remote
16911692
pushedBranches = branches
1693+
assert.False(t, force)
1694+
assert.True(t, atomic)
16921695
return nil
16931696
},
16941697
})

‎cmd/push.go‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package cmd
22

33
import (
44
"errors"
5+
"os"
56

67
"github.com/github/gh-stack/internal/config"
78
"github.com/github/gh-stack/internal/git"
@@ -10,7 +11,8 @@ import (
1011
)
1112

1213
type pushOptions struct {
13-
remote string
14+
remote string
15+
noAtomic bool
1416
}
1517

1618
func PushCmd(cfg *config.Config) *cobra.Command {
@@ -21,9 +23,8 @@ func PushCmd(cfg *config.Config) *cobra.Command {
2123
Short: "Push active branches in the current stack to the remote",
2224
Long: `Push active branches in the current stack to the remote.
2325
24-
Uses explicit per-branch --force-with-lease checks. Updates are not atomic: a
25-
branch may update even if another branch is rejected. Fix the rejected branch
26-
and run the command again; branches already updated will be unchanged.
26+
Uses explicit per-branch --force-with-lease checks and atomic updates: all
27+
branch updates succeed or fail together. Use --no-atomic to allow partial updates.
2728
Merged and queued branches are automatically skipped.`,
2829
Example: ` # Push active stack branches to the default remote
2930
$ gh stack push
@@ -36,6 +37,7 @@ Merged and queued branches are automatically skipped.`,
3637
}
3738

3839
cmd.Flags().StringVar(&opts.remote, "remote", "", "Remote to push to (defaults to auto-detected remote)")
40+
cmd.Flags().BoolVar(&opts.noAtomic, "no-atomic", os.Getenv("GH_STACK_NO_ATOMIC") != "", "Disable atomic pushes (also set by GH_STACK_NO_ATOMIC)")
3941

4042
return cmd
4143
}
@@ -108,7 +110,7 @@ func runPush(cfg *config.Config, opts *pushOptions) error {
108110
// remote yet.
109111
_ = git.FetchBranches(remote, activeBranches)
110112
cfg.Printf("Pushing %d %s to %s...", len(activeBranches), plural(len(activeBranches), "branch", "branches"), remote)
111-
if err := git.Push(remote, activeBranches, true, false); err != nil {
113+
if err := git.Push(remote, activeBranches, true, !opts.noAtomic); err != nil {
112114
cfg.Errorf("failed to push: %s", err)
113115
return ErrSilent
114116
}

‎cmd/push_test.go‎

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/github/gh-stack/internal/git"
1010
"github.com/github/gh-stack/internal/github"
1111
"github.com/github/gh-stack/internal/stack"
12+
"github.com/spf13/cobra"
1213
"github.com/stretchr/testify/assert"
1314
"github.com/stretchr/testify/require"
1415
)
@@ -24,6 +25,7 @@ func newPushMock(tmpDir string, currentBranch string) *git.MockOps {
2425
}
2526

2627
func TestPush_PushesAllBranches(t *testing.T) {
28+
t.Setenv("GH_STACK_NO_ATOMIC", "")
2729
s := stack.Stack{
2830
Trunk: stack.BranchRef{Branch: "main"},
2931
Branches: []stack.BranchRef{
@@ -62,11 +64,70 @@ func TestPush_PushesAllBranches(t *testing.T) {
6264
assert.Equal(t, "origin", pushCalls[0].remote)
6365
assert.Equal(t, []string{"b1", "b2"}, pushCalls[0].branches)
6466
assert.True(t, pushCalls[0].force)
65-
assert.False(t, pushCalls[0].atomic)
67+
assert.True(t, pushCalls[0].atomic)
6668
assert.Contains(t, output, "Pushed 2 branches")
6769
assert.Contains(t, output, "gh stack submit", "should hint about submit when branches have no PRs")
6870
}
6971

72+
func TestPush_AtomicOptions(t *testing.T) {
73+
for _, command := range []struct {
74+
name string
75+
newCmd func(*config.Config) *cobra.Command
76+
args []string
77+
force bool
78+
wantErr error
79+
}{
80+
{"push", PushCmd, nil, true, ErrSilent},
81+
{"submit", SubmitCmd, []string{"--auto"}, true, ErrSilent},
82+
{"sync", SyncCmd, nil, false, nil},
83+
{"link", LinkCmd, []string{"b1", "b2"}, false, ErrSilent},
84+
} {
85+
for _, tc := range []struct {
86+
name string
87+
env string
88+
args []string
89+
atomic bool
90+
}{
91+
{"default", "", nil, true},
92+
{"flag", "", []string{"--no-atomic"}, false},
93+
{"environment", "1", nil, false},
94+
{"flag overrides environment", "1", []string{"--no-atomic=false"}, true},
95+
} {
96+
t.Run(command.name+"/"+tc.name, func(t *testing.T) {
97+
t.Setenv("GH_STACK_NO_ATOMIC", tc.env)
98+
dir := t.TempDir()
99+
writeStackFile(t, dir, stack.Stack{
100+
Trunk: stack.BranchRef{Branch: "main"},
101+
Branches: []stack.BranchRef{{Branch: "b1"}, {Branch: "b2"}},
102+
})
103+
var pushes []pushCall
104+
mock := newSyncMock(dir, "b1")
105+
mock.RootDirFn = func() (string, error) { return dir, nil }
106+
mock.PushFn = func(remote string, branches []string, force, atomic bool) error {
107+
pushes = append(pushes, pushCall{remote, branches, force, atomic})
108+
return assert.AnError
109+
}
110+
restore := git.SetOps(mock)
111+
defer restore()
112+
113+
cfg, outR, errR := config.NewTestConfig()
114+
cfg.GitHubClientOverride = &github.MockClient{}
115+
cmd := command.newCmd(cfg)
116+
args := append([]string(nil), command.args...)
117+
cmd.SetArgs(append(args, tc.args...))
118+
cmd.SetOut(io.Discard)
119+
cmd.SetErr(io.Discard)
120+
err := cmd.Execute()
121+
_, output := commandOutput(t, cfg, outR, errR)
122+
123+
require.ErrorIs(t, err, command.wantErr, output)
124+
require.Len(t, pushes, 1, "failed pushes must not retry non-atomically")
125+
assert.Equal(t, pushCall{"origin", []string{"b1", "b2"}, command.force, tc.atomic}, pushes[0])
126+
})
127+
}
128+
}
129+
}
130+
70131
func TestPush_NoSubmitHintWhenPRsExist(t *testing.T) {
71132
s := stack.Stack{
72133
Trunk: stack.BranchRef{Branch: "main"},

‎cmd/submit.go‎

Lines changed: 53 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package cmd
33
import (
44
"errors"
55
"fmt"
6+
"os"
67
"slices"
78
"strconv"
89
"strings"
@@ -22,9 +23,10 @@ import (
2223
)
2324

2425
type submitOptions struct {
25-
auto bool
26-
open bool
27-
remote string
26+
auto bool
27+
open bool
28+
remote string
29+
noAtomic bool
2830
}
2931

3032
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
4648
the "STACK N PRs" button) to link the existing open PRs into a stack.
4749
4850
This command performs several steps:
49-
1. Pushes all branches to the remote
51+
1. Pushes all active branches to the remote atomically
5052
2. Creates new PRs for the included branches
5153
3. Updates base branches for existing PRs
5254
4. Creates or updates the stack on GitHub
5355
56+
Use --no-atomic to allow partial branch updates. PR updates are not atomic.
57+
5458
In the editor, new PRs default to ready for review; switch any to draft with the
5559
"CREATE AS" toggle. With --auto, new PRs are created as drafts unless you pass
5660
--open.`,
@@ -70,6 +74,7 @@ In the editor, new PRs default to ready for review; switch any to draft with the
7074
cmd.Flags().BoolVar(&opts.auto, "auto", false, "Use auto-generated PR titles without prompting")
7175
cmd.Flags().BoolVar(&opts.open, "open", false, "Mark new and existing PRs as ready for review")
7276
cmd.Flags().StringVar(&opts.remote, "remote", "", "Remote to push to (defaults to auto-detected remote)")
77+
cmd.Flags().BoolVar(&opts.noAtomic, "no-atomic", os.Getenv("GH_STACK_NO_ATOMIC") != "", "Disable atomic pushes (also set by GH_STACK_NO_ATOMIC)")
7378

7479
return cmd
7580
}
@@ -237,23 +242,28 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error {
237242
drafts = collected
238243
}
239244

240-
// Push each branch and create/update its PR in stack order (bottom to top).
241-
// Sequential pushing ensures each branch's base is up-to-date on the
242-
// remote before the next branch is pushed, preventing race conditions.
243-
cfg.Printf("Pushing to %s...", remote)
245+
if pendingModify {
246+
if err := prepareReorderedPRs(cfg, client, s); err != nil {
247+
cfg.Errorf("failed to prepare reordered PRs: %s", err)
248+
return ErrSilent
249+
}
250+
}
251+
252+
if len(activeBranches) > 0 {
253+
cfg.Printf("Pushing to %s...", remote)
254+
if err := git.Push(remote, activeBranches, true, !opts.noAtomic); err != nil {
255+
cfg.Errorf("failed to push: %s", err)
256+
return ErrSilent
257+
}
258+
}
259+
244260
allPRsSubmitted := len(queued) == 0
245261
submittedPRs := make(map[string]int, len(activeBranches))
246262
for i, b := range s.Branches {
247263
if s.Branches[i].IsMerged() || s.Branches[i].IsQueued() {
248264
continue
249265
}
250266

251-
// Push this branch
252-
if err := git.Push(remote, []string{b.Branch}, true, false); err != nil {
253-
cfg.Errorf("failed to push %s: %s", b.Branch, err)
254-
return ErrSilent
255-
}
256-
257267
// Find or create PR, and fix base if needed
258268
baseBranch := s.ActiveBaseBranch(b.Branch)
259269
submitted, err := ensurePR(cfg, client, s, i, baseBranch, opts, templateContent, drafts)
@@ -304,6 +314,35 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error {
304314
return nil
305315
}
306316

317+
// Reversed PR relationships can cause indirect merges when the new heads are
318+
// pushed. Use the unchanged trunk until ensurePR restores the intended bases.
319+
func prepareReorderedPRs(cfg *config.Config, client github.ClientOps, s *stack.Stack) error {
320+
for i, b := range s.Branches {
321+
if b.IsSkipped() {
322+
continue
323+
}
324+
pr, err := client.FindPRForBranch(b.Branch)
325+
if err != nil {
326+
return fmt.Errorf("looking up PR for %s: %w", b.Branch, err)
327+
}
328+
if pr == nil || s.IndexOf(pr.BaseRefName) <= i {
329+
continue
330+
}
331+
if pr.IsAutoMergeEnabled() {
332+
if err := client.DisableAutoMerge(pr.ID); err != nil {
333+
return fmt.Errorf("disabling auto-merge for PR #%d: %w", pr.Number, err)
334+
}
335+
cfg.Warningf("Disabled auto-merge for PR %s (incompatible with stacked PRs)",
336+
cfg.PRLink(pr.Number, pr.URL))
337+
}
338+
if err := client.UpdatePRBase(pr.Number, s.Trunk.Branch); err != nil {
339+
return fmt.Errorf("temporarily setting base of PR #%d to %s: %w", pr.Number, s.Trunk.Branch, err)
340+
}
341+
cfg.Successf("Temporarily set base of PR %s to %s", cfg.PRLink(pr.Number, pr.URL), s.Trunk.Branch)
342+
}
343+
return nil
344+
}
345+
307346
// collectPRDrafts loads branch display data and runs the interactive submit TUI
308347
// so the user can choose which new branches become PRs and draft each one. It
309348
// returns the per-branch overrides, whether the user cancelled, and any error.

0 commit comments

Comments
 (0)