Skip to content

Commit 2250f2b

Browse files
authored
feat: batch push branches (#125)
* feat: batch push branches This changes the behavior of `gh stack submit` so that it pushes all branches at once, atomically. This improves performance. * docs(git): clarify Push doc comment The Push doc comment said "force-pushes ... with lease" but the function takes a `force` flag and only adds `--force-with-lease` when true. Update the comment to describe both branches honestly. Per PR #125 review. * fix(git): guard PushMany against dash-prefixed branch names Insert a `--` end-of-options marker between the flag list and the refspec list so a branch starting with `-` is never parsed as a git option. Per PR #125 review. * style(submit): format branch names in push summary line Other submit output ("Skipping push for ...") formats branch names via `s.Branch(...)`; the batched push summary should do the same so colors and emphasis stay consistent across the phase. Per PR #125 review. * fix(submit): include refs in batched push error message The previous `push failed: %w` wrapping dropped the list of branches being pushed, making failures harder to diagnose at a glance. Include the branch names alongside git's underlying error. Per PR #125 review. * docs(git): correct setupRepoWithRemote return-value comment Helper returns four values (localDir, remoteDir, *Git, trunk) but the comment only listed three. Sync the comment with the signature. Per PR #125 review. * test(git): drop dead pre-divergence block in atomic-rejection test The first block wrote a file into the bare remote and ran `update-ref ... HEAD` with all errors ignored. It did nothing useful — the temp-clone push that follows is what actually creates the divergence — and was confusing. Remove it and tighten the remaining setup so errors are surfaced via `t.Fatalf` instead of being swallowed. Per PR #125 review. * test(git): assert strict feat-b equality after atomic rejection The previous check (`remoteB != tipBNew`) could pass for the wrong reason — e.g. `rev-parse` returning `""` on error would silently satisfy it. Use the captured pre-push tip (`tipBLocal`) for an exact equality check, and fail loudly if the remote SHA can't be read at all. Per PR #125 review.
1 parent 5b73344 commit 2250f2b

6 files changed

Lines changed: 250 additions & 10 deletions

File tree

AGENTS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,3 +65,7 @@ Uses `go-gh` library (not subprocess calls to `gh`):
6565
### Git Operations
6666

6767
`internal/git` uses `safeexec.LookPath("git")` to find git securely (prevents PATH injection on Windows). The path is cached with `sync.Once`.
68+
69+
## Generated Files
70+
71+
- `CHANGELOG.md` is automatically generated by Release Please and should never be edited manually.

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ Restack, push, and create/update PRs for the entire stack.
268268
This is the primary workflow command. By default it processes **every tracked branch** in parent-before-child order. It performs three phases:
269269

270270
1. **Restack**: Rebase affected branches onto their parents
271-
2. **Push**: Force-push all affected branches (using `--force-with-lease`)
271+
2. **Push**: Force-push all affected branches in a single atomic `git push` (`--force-with-lease --atomic`); if any ref is rejected the push fails immediately and no refs are updated
272272
3. **PR**: Create PRs for branches without them; update PR bases for existing PRs
273273

274274
PRs targeting non-trunk branches are created as drafts. When a PR's base changes to trunk (after its parent merges), you'll be prompted to mark it ready for review.

cmd/submit.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,7 @@ func doSubmitPushAndPR(g *git.Git, cfg *config.Config, root *tree.Node, branches
285285

286286
// Phase 2: Push branches that will participate in PRs (or all if --skip-prs).
287287
fmt.Println(s.Bold("\n=== Phase 2: Push ==="))
288+
var toPush []string
288289
for _, b := range branches {
289290
var d *prDecision
290291
if !opts.PushOnly {
@@ -298,12 +299,17 @@ func doSubmitPushAndPR(g *git.Git, cfg *config.Config, root *tree.Node, branches
298299
if opts.DryRun {
299300
fmt.Printf("%s Would push %s -> origin/%s (forced)\n", s.Muted("dry-run:"), s.Branch(b.Name), s.Branch(b.Name))
300301
} else {
301-
fmt.Printf("Pushing %s -> origin/%s (forced)... ", s.Branch(b.Name), s.Branch(b.Name))
302-
if err := g.Push(b.Name, true); err != nil {
303-
fmt.Println(s.Error("failed"))
304-
return fmt.Errorf("failed to push %s: %w", b.Name, err)
305-
}
306-
fmt.Println(s.Success("ok"))
302+
toPush = append(toPush, b.Name)
303+
}
304+
}
305+
if !opts.DryRun && len(toPush) > 0 {
306+
styled := make([]string, len(toPush))
307+
for i, name := range toPush {
308+
styled[i] = s.Branch(name)
309+
}
310+
fmt.Printf("Pushing %s to origin (force-with-lease, atomic)...\n", strings.Join(styled, ", "))
311+
if err := g.PushMany(toPush, true); err != nil {
312+
return fmt.Errorf("failed to push branches [%s]: %w", strings.Join(toPush, ", "), err)
307313
}
308314
}
309315

e2e/submit_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,3 +491,44 @@ func TestSubmitFromTrunkFallback(t *testing.T) {
491491
t.Error("should not push trunk branch")
492492
}
493493
}
494+
495+
// TestSubmitBatchPush verifies that a multi-branch submit advances all remote
496+
// refs in one atomic push and emits the expected summary line.
497+
func TestSubmitBatchPush(t *testing.T) {
498+
env := NewTestEnvWithRemote(t)
499+
env.MustRun("init")
500+
501+
// Build a 3-branch stack: main -> feat-a -> feat-b -> feat-c
502+
env.MustRun("create", "feat-a")
503+
tipA := env.CreateCommit("a work")
504+
505+
env.MustRun("create", "feat-b")
506+
tipB := env.CreateCommit("b work")
507+
508+
env.MustRun("create", "feat-c")
509+
tipC := env.CreateCommit("c work")
510+
511+
result := env.MustRun("submit", "--skip-prs", "--yes")
512+
513+
// Output should mention the batch push summary
514+
if !strings.Contains(result.Stdout, "Pushing") {
515+
t.Error("expected batch push summary line in output")
516+
}
517+
if !strings.Contains(result.Stdout, "atomic") {
518+
t.Error("expected 'atomic' in push summary line")
519+
}
520+
521+
// All three remote refs must match local tips
522+
remoteA := env.GitRemote("rev-parse", "refs/heads/feat-a")
523+
if remoteA != tipA {
524+
t.Errorf("remote feat-a = %s, want %s", remoteA, tipA)
525+
}
526+
remoteB := env.GitRemote("rev-parse", "refs/heads/feat-b")
527+
if remoteB != tipB {
528+
t.Errorf("remote feat-b = %s, want %s", remoteB, tipB)
529+
}
530+
remoteC := env.GitRemote("rev-parse", "refs/heads/feat-c")
531+
if remoteC != tipC {
532+
t.Errorf("remote feat-c = %s, want %s", remoteC, tipC)
533+
}
534+
}

internal/git/git.go

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -173,12 +173,31 @@ func (g *Git) Commit(message string) error {
173173
return g.runSilent("commit", "-m", message)
174174
}
175175

176-
// Push force-pushes a branch to origin with lease.
176+
// Push pushes a branch to origin. When force is true, --force-with-lease is
177+
// used; when false, the push is a normal fast-forward push (and will fail if
178+
// the remote has diverged).
177179
func (g *Git) Push(branch string, force bool) error {
178-
args := []string{"push", "origin", branch}
180+
return g.PushMany([]string{branch}, force)
181+
}
182+
183+
// PushMany pushes multiple branches to origin in a single invocation.
184+
// When force is true, --force-with-lease and --atomic are added so that the
185+
// push is all-or-nothing: if any ref is rejected (e.g. lease conflict), none
186+
// of the refs are updated on the remote.
187+
// Returns nil immediately when branches is empty.
188+
//
189+
// All flags are placed before a `--` end-of-options marker so refspecs that
190+
// happen to start with `-` are never misinterpreted as git options.
191+
func (g *Git) PushMany(branches []string, force bool) error {
192+
if len(branches) == 0 {
193+
return nil
194+
}
195+
args := []string{"push"}
179196
if force {
180-
args = append(args, "--force-with-lease")
197+
args = append(args, "--force-with-lease", "--atomic")
181198
}
199+
args = append(args, "origin", "--")
200+
args = append(args, branches...)
182201
return g.runInteractive(args...)
183202
}
184203

internal/git/git_test.go

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -904,3 +904,173 @@ func TestRebaseNoUpdateRefsPreservesBookmark(t *testing.T) {
904904
t.Errorf("expected bookmark to be preserved with --no-update-refs, but it moved from %s to %s", bookmarkBefore, bookmarkAfter)
905905
}
906906
}
907+
908+
// setupRepoWithRemote creates a local repo + bare remote and returns
909+
// (localDir, remoteDir, *Git, trunk). The trunk branch is pushed to the
910+
// remote and the remote is set as "origin".
911+
func setupRepoWithRemote(t *testing.T) (dir, remoteDir string, g *git.Git, trunk string) {
912+
t.Helper()
913+
dir = t.TempDir()
914+
915+
run := func(args ...string) string {
916+
cmd := exec.Command("git", args...)
917+
cmd.Dir = dir
918+
out, err := cmd.CombinedOutput()
919+
if err != nil {
920+
t.Fatalf("git %v failed: %v\n%s", args, err, out)
921+
}
922+
return strings.TrimSpace(string(out))
923+
}
924+
925+
run("init", "-b", "main")
926+
run("config", "user.email", "test@test.com")
927+
run("config", "user.name", "Test")
928+
os.WriteFile(filepath.Join(dir, "root"), []byte("root"), 0644)
929+
run("add", ".")
930+
run("commit", "-m", "root")
931+
932+
remoteDir = t.TempDir()
933+
exec.Command("git", "clone", "--bare", dir, remoteDir).Run() //nolint:errcheck
934+
run("remote", "add", "origin", remoteDir)
935+
run("push", "-u", "origin", "main")
936+
937+
g = git.New(dir)
938+
trunk = "main"
939+
return dir, remoteDir, g, trunk
940+
}
941+
942+
// remoteRef returns the SHA that a ref points to on the bare remote, or "" on error.
943+
func remoteRef(t *testing.T, remoteDir, branch string) string {
944+
t.Helper()
945+
out, err := exec.Command("git", "-C", remoteDir, "rev-parse", "refs/heads/"+branch).Output()
946+
if err != nil {
947+
return ""
948+
}
949+
return strings.TrimSpace(string(out))
950+
}
951+
952+
func TestPushManyHappyPath(t *testing.T) {
953+
dir, remoteDir, g, _ := setupRepoWithRemote(t)
954+
955+
run := func(args ...string) {
956+
cmd := exec.Command("git", args...)
957+
cmd.Dir = dir
958+
if out, err := cmd.CombinedOutput(); err != nil {
959+
t.Fatalf("git %v failed: %v\n%s", args, err, out)
960+
}
961+
}
962+
963+
// Create feat-a
964+
run("checkout", "-b", "feat-a")
965+
os.WriteFile(filepath.Join(dir, "a"), []byte("a"), 0644)
966+
run("add", ".")
967+
run("commit", "-m", "a")
968+
tipA, _ := g.GetTip("feat-a")
969+
970+
// Create feat-b on top
971+
run("checkout", "-b", "feat-b")
972+
os.WriteFile(filepath.Join(dir, "b"), []byte("b"), 0644)
973+
run("add", ".")
974+
run("commit", "-m", "b")
975+
tipB, _ := g.GetTip("feat-b")
976+
977+
// Both branches are local-only at this point; PushMany should create them on remote.
978+
if err := g.PushMany([]string{"feat-a", "feat-b"}, false); err != nil {
979+
t.Fatalf("PushMany failed: %v", err)
980+
}
981+
982+
if got := remoteRef(t, remoteDir, "feat-a"); got != tipA {
983+
t.Errorf("remote feat-a = %s, want %s", got, tipA)
984+
}
985+
if got := remoteRef(t, remoteDir, "feat-b"); got != tipB {
986+
t.Errorf("remote feat-b = %s, want %s", got, tipB)
987+
}
988+
}
989+
990+
func TestPushManyEmpty(t *testing.T) {
991+
_, _, g, _ := setupRepoWithRemote(t)
992+
// Should be a no-op with no error.
993+
if err := g.PushMany(nil, true); err != nil {
994+
t.Fatalf("PushMany(nil) returned unexpected error: %v", err)
995+
}
996+
if err := g.PushMany([]string{}, true); err != nil {
997+
t.Fatalf("PushMany([]) returned unexpected error: %v", err)
998+
}
999+
}
1000+
1001+
// TestPushManyAtomicRejection verifies that when one branch fails --force-with-lease,
1002+
// the --atomic flag prevents the other branch from being updated on the remote.
1003+
func TestPushManyAtomicRejection(t *testing.T) {
1004+
dir, remoteDir, g, _ := setupRepoWithRemote(t)
1005+
1006+
run := func(args ...string) {
1007+
cmd := exec.Command("git", args...)
1008+
cmd.Dir = dir
1009+
if out, err := cmd.CombinedOutput(); err != nil {
1010+
t.Fatalf("git %v failed: %v\n%s", args, err, out)
1011+
}
1012+
}
1013+
1014+
// Create and push feat-a
1015+
run("checkout", "-b", "feat-a")
1016+
os.WriteFile(filepath.Join(dir, "a"), []byte("a"), 0644)
1017+
run("add", ".")
1018+
run("commit", "-m", "a")
1019+
run("push", "origin", "feat-a")
1020+
1021+
// Create feat-b (will be pushed clean — no prior remote ref)
1022+
run("checkout", "main")
1023+
run("checkout", "-b", "feat-b")
1024+
os.WriteFile(filepath.Join(dir, "b"), []byte("b"), 0644)
1025+
run("add", ".")
1026+
run("commit", "-m", "b")
1027+
tipBLocal, _ := g.GetTip("feat-b")
1028+
run("push", "origin", "feat-b")
1029+
1030+
// Diverge feat-a on the remote by committing & pushing through a temp
1031+
// clone. This advances refs/heads/feat-a on the bare remote out from
1032+
// under our local clone, so our next force-with-lease for feat-a will
1033+
// see a stale lease and be rejected.
1034+
tmp := t.TempDir()
1035+
cloneTmp := exec.Command("git", "clone", remoteDir, tmp)
1036+
if out, err := cloneTmp.CombinedOutput(); err != nil {
1037+
t.Fatalf("git clone (divergence) failed: %v\n%s", err, out)
1038+
}
1039+
tmpRun := func(args ...string) {
1040+
cmd := exec.Command("git", args...)
1041+
cmd.Dir = tmp
1042+
if out, err := cmd.CombinedOutput(); err != nil {
1043+
t.Fatalf("git %v (divergence) failed: %v\n%s", args, err, out)
1044+
}
1045+
}
1046+
tmpRun("config", "user.email", "t@t.com")
1047+
tmpRun("config", "user.name", "T")
1048+
tmpRun("checkout", "feat-a")
1049+
os.WriteFile(filepath.Join(tmp, "remote-diverge"), []byte("x"), 0644)
1050+
tmpRun("add", ".")
1051+
tmpRun("commit", "-m", "diverge")
1052+
tmpRun("push", "origin", "feat-a")
1053+
1054+
// Add a new commit to feat-b locally (so we're trying to advance it)
1055+
run("checkout", "feat-b")
1056+
os.WriteFile(filepath.Join(dir, "b2"), []byte("b2"), 0644)
1057+
run("add", ".")
1058+
run("commit", "-m", "b2")
1059+
1060+
// PushMany should fail because feat-a's lease is broken.
1061+
err := g.PushMany([]string{"feat-a", "feat-b"}, true)
1062+
if err == nil {
1063+
t.Fatal("expected PushMany to fail due to lease rejection on feat-a, but it succeeded")
1064+
}
1065+
1066+
// Due to --atomic, feat-b must remain at exactly its pre-push value on
1067+
// the remote. Assert strict equality (and fail loudly if the remote ref
1068+
// cannot be read) so a missing/blank rev-parse can't masquerade as success.
1069+
remoteB := remoteRef(t, remoteDir, "feat-b")
1070+
if remoteB == "" {
1071+
t.Fatal("could not read remote feat-b SHA after PushMany failure")
1072+
}
1073+
if remoteB != tipBLocal {
1074+
t.Errorf("remote feat-b = %s, want unchanged %s — --atomic did not hold", remoteB, tipBLocal)
1075+
}
1076+
}

0 commit comments

Comments
 (0)