Skip to content

Commit 36b2784

Browse files
committed
Add atomic push option to push and sync commands
Expose --atomic for multi-ref pushes through the shared Git push infrastructure. Keep push non-atomic by default and preserve sync's existing atomic default with --atomic=false available as an opt-out. Document the command behavior, audit link and submit push paths, and add unit and integration coverage for atomic rejection semantics.
1 parent 14fc42e commit 36b2784

7 files changed

Lines changed: 168 additions & 8 deletions

File tree

cmd/link_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1591,13 +1591,17 @@ func TestLink_UpdateDeletedStack_FallsBackToCreate(t *testing.T) {
15911591
func TestLink_PushesBranchesBeforeResolution(t *testing.T) {
15921592
var pushedBranches []string
15931593
var pushedRemote string
1594+
var pushedForce bool
1595+
var pushedAtomic bool
15941596

15951597
restore := git.SetOps(&git.MockOps{
15961598
BranchExistsFn: func(name string) bool { return name == "feat-a" || name == "feat-b" },
15971599
ResolveRemoteFn: func(string) (string, error) { return "origin", nil },
15981600
PushFn: func(remote string, branches []string, force, atomic bool) error {
15991601
pushedRemote = remote
16001602
pushedBranches = branches
1603+
pushedForce = force
1604+
pushedAtomic = atomic
16011605
return nil
16021606
},
16031607
})
@@ -1633,6 +1637,8 @@ func TestLink_PushesBranchesBeforeResolution(t *testing.T) {
16331637
assert.NoError(t, err)
16341638
assert.Equal(t, "origin", pushedRemote)
16351639
assert.Equal(t, []string{"feat-a", "feat-b"}, pushedBranches)
1640+
assert.False(t, pushedForce)
1641+
assert.True(t, pushedAtomic)
16361642
assert.Contains(t, output, "Pushing 2 branches")
16371643
}
16381644

cmd/push.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212

1313
type pushOptions struct {
1414
remote string
15+
atomic bool
1516
}
1617

1718
func PushCmd(cfg *config.Config) *cobra.Command {
@@ -22,13 +23,16 @@ func PushCmd(cfg *config.Config) *cobra.Command {
2223
Short: "Push active branches in the current stack to the remote",
2324
Long: `Push active branches in the current stack to the remote.
2425
25-
Uses explicit per-branch --force-with-lease checks. Updates are not atomic: a
26-
branch may update even if another branch is rejected. Fix the rejected branch
27-
and run the command again; branches already updated will be unchanged.
26+
Uses explicit per-branch --force-with-lease checks. By default, updates are not
27+
atomic: a branch may update even if another branch is rejected. Use --atomic
28+
to require all branch updates to succeed or fail together.
2829
Merged and queued branches are automatically skipped.`,
2930
Example: ` # Push active stack branches to the default remote
3031
$ gh stack push
3132
33+
# Push all active branches atomically
34+
$ gh stack push --atomic
35+
3236
# Push to a specific remote
3337
$ gh stack push --remote upstream`,
3438
RunE: func(cmd *cobra.Command, args []string) error {
@@ -37,6 +41,7 @@ Merged and queued branches are automatically skipped.`,
3741
}
3842

3943
cmd.Flags().StringVar(&opts.remote, "remote", "", "Remote to push to (defaults to auto-detected remote)")
44+
cmd.Flags().BoolVar(&opts.atomic, "atomic", false, "Require all branch updates to succeed or fail together (default: disabled)")
4045

4146
return cmd
4247
}
@@ -107,7 +112,7 @@ func runPush(cfg *config.Config, opts *pushOptions) error {
107112
// remote yet.
108113
_ = git.FetchBranches(remote, activeBranches)
109114
cfg.Printf("Pushing %d %s to %s...", len(activeBranches), plural(len(activeBranches), "branch", "branches"), remote)
110-
if err := git.Push(remote, activeBranches, true, false); err != nil {
115+
if err := git.Push(remote, activeBranches, true, opts.atomic); err != nil {
111116
cfg.Errorf("failed to push: %s", err)
112117
return ErrSilent
113118
}

cmd/push_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,42 @@ func TestPush_PushesAllBranches(t *testing.T) {
6767
assert.Contains(t, output, "gh stack submit", "should hint about submit when branches have no PRs")
6868
}
6969

70+
func TestPush_Atomic(t *testing.T) {
71+
s := stack.Stack{
72+
Trunk: stack.BranchRef{Branch: "main"},
73+
Branches: []stack.BranchRef{
74+
{Branch: "b1"},
75+
{Branch: "b2"},
76+
},
77+
}
78+
79+
tmpDir := t.TempDir()
80+
writeStackFile(t, tmpDir, s)
81+
82+
var pushCalls []pushCall
83+
mock := newPushMock(tmpDir, "b1")
84+
mock.PushFn = func(remote string, branches []string, force, atomic bool) error {
85+
pushCalls = append(pushCalls, pushCall{remote, branches, force, atomic})
86+
return nil
87+
}
88+
89+
restore := git.SetOps(mock)
90+
defer restore()
91+
92+
cfg, _, _ := config.NewTestConfig()
93+
cfg.GitHubClientOverride = &github.MockClient{}
94+
cmd := PushCmd(cfg)
95+
cmd.SetArgs([]string{"--atomic"})
96+
cmd.SetOut(io.Discard)
97+
cmd.SetErr(io.Discard)
98+
99+
require.NoError(t, cmd.Execute())
100+
require.Len(t, pushCalls, 1)
101+
assert.Equal(t, []string{"b1", "b2"}, pushCalls[0].branches)
102+
assert.True(t, pushCalls[0].force)
103+
assert.True(t, pushCalls[0].atomic)
104+
}
105+
70106
func TestPush_NoSubmitHintWhenPRsExist(t *testing.T) {
71107
s := stack.Stack{
72108
Trunk: stack.BranchRef{Branch: "main"},

cmd/sync.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
type syncOptions struct {
1717
remote string
1818
prune bool
19+
atomic bool
1920
}
2021

2122
func SyncCmd(cfg *config.Config) *cobra.Command {
@@ -34,11 +35,14 @@ This command performs a safe synchronization:
3435
resolve a divergence in an interactive terminal
3536
3. Fast-forwards the trunk branch to match the remote
3637
4. Cascade-rebases stack branches onto their updated parents
37-
5. Pushes all branches atomically (using --force-with-lease --atomic)
38+
5. Pushes all branches atomically by default
3839
6. Syncs PR state from GitHub
3940
7. Links the stack's open PRs into a stack on GitHub (creating or updating
4041
the remote stack object) when two or more PRs exist
4142
43+
Atomic sync uses --atomic and, after a rebase, --force-with-lease. Use
44+
--atomic=false to allow partial branch updates.
45+
4246
If PRs have been added to the stack on GitHub, their branches are pulled
4347
down and appended to your local stack so it mirrors the remote. A clean
4448
"remote is ahead" update happens automatically without prompting. If the
@@ -70,6 +74,7 @@ the first active branch in the stack, or the trunk if all are merged.`,
7074

7175
cmd.Flags().StringVar(&opts.remote, "remote", "", "Remote to fetch from and push to (defaults to auto-detected remote)")
7276
cmd.Flags().BoolVar(&opts.prune, "prune", false, "Delete local branches for merged PRs")
77+
cmd.Flags().BoolVar(&opts.atomic, "atomic", true, "Require all branch updates to succeed or fail together")
7378

7479
return cmd
7580
}
@@ -239,7 +244,7 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
239244
// Without rebase, try a normal push first.
240245
force := rebased
241246
cfg.Printf("Pushing %d %s to %s...", len(branches), plural(len(branches), "branch", "branches"), remote)
242-
if err := git.Push(remote, branches, force, true); err != nil {
247+
if err := git.Push(remote, branches, force, opts.atomic); err != nil {
243248
if !force {
244249
cfg.Warningf("Push failed — branches may need force push after rebase")
245250
cfg.Printf(" Run `%s` to push with --force-with-lease.",

cmd/sync_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,54 @@ func TestSync_TrunkAlreadyUpToDate(t *testing.T) {
107107
// Push should happen without force
108108
require.Len(t, pushCalls, 1)
109109
assert.False(t, pushCalls[0].force, "push should not use force when no rebase occurred")
110+
assert.True(t, pushCalls[0].atomic, "sync should push atomically by default")
111+
}
112+
113+
func TestSync_AtomicFlag(t *testing.T) {
114+
tests := []struct {
115+
name string
116+
args []string
117+
wantAtomic bool
118+
}{
119+
{name: "explicit atomic", args: []string{"--atomic"}, wantAtomic: true},
120+
{name: "atomic disabled", args: []string{"--atomic=false"}, wantAtomic: false},
121+
}
122+
123+
for _, tt := range tests {
124+
t.Run(tt.name, func(t *testing.T) {
125+
s := stack.Stack{
126+
Trunk: stack.BranchRef{Branch: "main"},
127+
Branches: []stack.BranchRef{
128+
{Branch: "b1"},
129+
{Branch: "b2"},
130+
},
131+
}
132+
133+
tmpDir := t.TempDir()
134+
writeStackFile(t, tmpDir, s)
135+
136+
var pushCalls []pushCall
137+
mock := newSyncMock(tmpDir, "b1")
138+
mock.PushFn = func(remote string, branches []string, force, atomic bool) error {
139+
pushCalls = append(pushCalls, pushCall{remote, branches, force, atomic})
140+
return nil
141+
}
142+
143+
restore := git.SetOps(mock)
144+
defer restore()
145+
146+
cfg, _, _ := config.NewTestConfig()
147+
cmd := SyncCmd(cfg)
148+
cmd.SetArgs(tt.args)
149+
cmd.SetOut(io.Discard)
150+
cmd.SetErr(io.Discard)
151+
152+
require.NoError(t, cmd.Execute())
153+
require.Len(t, pushCalls, 1)
154+
assert.Equal(t, []string{"b1", "b2"}, pushCalls[0].branches)
155+
assert.Equal(t, tt.wantAtomic, pushCalls[0].atomic)
156+
})
157+
}
110158
}
111159

112160
// TestSync_TrunkUpToDate_StackStale verifies that when trunk is already up to
@@ -251,6 +299,7 @@ func TestSync_TrunkFastForward_TriggersRebase(t *testing.T) {
251299

252300
cfg, _, errR := config.NewTestConfig()
253301
cmd := SyncCmd(cfg)
302+
cmd.SetArgs([]string{"--atomic=false"})
254303
cmd.SetOut(io.Discard)
255304
cmd.SetErr(io.Discard)
256305
err := cmd.Execute()
@@ -273,6 +322,7 @@ func TestSync_TrunkFastForward_TriggersRebase(t *testing.T) {
273322

274323
// Push should use force-with-lease after rebase
275324
require.Len(t, pushCalls, 1)
325+
assert.False(t, pushCalls[0].atomic, "atomic option should apply to force pushes")
276326
assert.True(t, pushCalls[0].force, "push should use force-with-lease after rebase")
277327
}
278328

docs/src/content/docs/reference/cli.md

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,7 @@ gh stack sync [flags]
304304

305305
| Flag | Description |
306306
|------|-------------|
307+
| `--atomic` | Require all branch updates to succeed or fail together (enabled by default; use `--atomic=false` to disable) |
307308
| `--remote <name>` | Remote to fetch from and push to (defaults to auto-detected remote) |
308309
| `--prune` | Delete local branches for merged PRs |
309310

@@ -313,7 +314,7 @@ Performs a synchronization of the entire stack:
313314
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).
314315
3. **Fast-forward trunk** — fast-forwards the trunk branch to match the remote (skips if diverged).
315316
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.
316-
5. **Push** — pushes all branches (uses `--force-with-lease` if a rebase occurred).
317+
5. **Push** — pushes all branches atomically by default (uses `--force-with-lease` if a rebase occurred). Use `--atomic=false` to allow branches whose updates succeed to proceed when another branch is rejected.
317318
6. **Sync PRs** — syncs PR state from GitHub and reports the status of each PR.
318319
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).
319320
8. **Prune** — in interactive terminals, prompts to delete local branches for merged PRs. Use `--prune` to prune automatically.
@@ -335,6 +336,9 @@ In a non-interactive terminal, a divergence aborts the sync (exit success) witho
335336
```sh
336337
gh stack sync
337338

339+
# Explicitly allow partial branch updates
340+
gh stack sync --atomic=false
341+
338342
# Sync and automatically prune merged branches
339343
gh stack sync --prune
340344
```
@@ -402,14 +406,16 @@ gh stack push [flags]
402406

403407
| Flag | Description |
404408
|------|-------------|
409+
| `--atomic` | Require all branch updates to succeed or fail together (disabled by default) |
405410
| `--remote <name>` | Remote to push to (defaults to auto-detected remote) |
406411

407-
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 `git push` using explicit per-branch `--force-with-lease` checks. By default, the update is not atomic: branches whose leases pass may update even if another branch is rejected. Use `--atomic` to make the multi-ref push all-or-nothing. This command does not create or update pull requests — use `gh stack submit` for that.
408413

409414
**Examples:**
410415

411416
```sh
412417
gh stack push
418+
gh stack push --atomic
413419
gh stack push --remote upstream
414420
```
415421

internal/git/gitops_test.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,58 @@ func TestIntegration_Push_RemoteAdvancedByOther(t *testing.T) {
240240
assert.NotEqual(t, otherSHA, finalRemoteSHA, "remote should have advanced past original other SHA")
241241
}
242242

243+
func TestIntegration_Push_AtomicRejectsAllRefs(t *testing.T) {
244+
bareDir, cloneDir := setupBareAndClone(t)
245+
restore := withGitDir(t, cloneDir)
246+
defer restore()
247+
248+
d := &defaultOps{}
249+
250+
gitExec(t, cloneDir, "checkout", "-b", "b1")
251+
writeFile(t, cloneDir, "b1.txt", "v1")
252+
gitExec(t, cloneDir, "add", ".")
253+
gitExec(t, cloneDir, "commit", "-m", "b1 initial")
254+
gitExec(t, cloneDir, "push", "origin", "b1")
255+
256+
gitExec(t, cloneDir, "checkout", "-b", "b2")
257+
writeFile(t, cloneDir, "b2.txt", "v1")
258+
gitExec(t, cloneDir, "add", ".")
259+
gitExec(t, cloneDir, "commit", "-m", "b2 initial")
260+
gitExec(t, cloneDir, "push", "origin", "b2")
261+
262+
require.NoError(t, d.FetchBranches("origin", []string{"b1", "b2"}))
263+
remoteB1Before := remoteBranchSHA(t, bareDir, "b1")
264+
265+
gitExec(t, cloneDir, "checkout", "b1")
266+
writeFile(t, cloneDir, "b1.txt", "local update")
267+
gitExec(t, cloneDir, "add", ".")
268+
gitExec(t, cloneDir, "commit", "-m", "b1 local update")
269+
localB1 := gitExec(t, cloneDir, "rev-parse", "b1")
270+
require.NotEqual(t, remoteB1Before, localB1)
271+
272+
gitExec(t, cloneDir, "checkout", "b2")
273+
writeFile(t, cloneDir, "b2.txt", "local update")
274+
gitExec(t, cloneDir, "add", ".")
275+
gitExec(t, cloneDir, "commit", "-m", "b2 local update")
276+
277+
otherClone := filepath.Join(t.TempDir(), "other")
278+
gitExec(t, ".", "clone", bareDir, otherClone)
279+
gitExec(t, otherClone, "checkout", "b2")
280+
writeFile(t, otherClone, "b2.txt", "competing update")
281+
gitExec(t, otherClone, "add", ".")
282+
gitExec(t, otherClone, "commit", "-m", "b2 competing update")
283+
gitExec(t, otherClone, "push", "origin", "b2")
284+
competingB2 := remoteBranchSHA(t, bareDir, "b2")
285+
286+
err := d.Push("origin", []string{"b1", "b2"}, true, true)
287+
require.Error(t, err, "atomic push should fail when one branch has a stale lease")
288+
289+
assert.Equal(t, remoteB1Before, remoteBranchSHA(t, bareDir, "b1"),
290+
"valid branch must not update when another ref is rejected")
291+
assert.Equal(t, competingB2, remoteBranchSHA(t, bareDir, "b2"),
292+
"rejected branch must preserve the competing remote update")
293+
}
294+
243295
// Test 4: Brand-new branch, absent on remote.
244296
// Push should create the branch via empty-expect lease.
245297
func TestIntegration_Push_NewBranchAbsentOnRemote(t *testing.T) {

0 commit comments

Comments
 (0)