diff --git a/AGENTS.md b/AGENTS.md index e851ac8..8fa1463 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -65,3 +65,7 @@ Uses `go-gh` library (not subprocess calls to `gh`): ### Git Operations `internal/git` uses `safeexec.LookPath("git")` to find git securely (prevents PATH injection on Windows). The path is cached with `sync.Once`. + +## Generated Files + +- `CHANGELOG.md` is automatically generated by Release Please and should never be edited manually. diff --git a/README.md b/README.md index 16f9e04..a604bce 100644 --- a/README.md +++ b/README.md @@ -268,7 +268,7 @@ Restack, push, and create/update PRs for the entire stack. This is the primary workflow command. By default it processes **every tracked branch** in parent-before-child order. It performs three phases: 1. **Restack**: Rebase affected branches onto their parents -2. **Push**: Force-push all affected branches (using `--force-with-lease`) +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 3. **PR**: Create PRs for branches without them; update PR bases for existing PRs PRs targeting non-trunk branches are created as drafts. When a PR's base changes to trunk (after its parent merges), you'll be prompted to mark it ready for review. diff --git a/cmd/submit.go b/cmd/submit.go index 444ac84..409414d 100644 --- a/cmd/submit.go +++ b/cmd/submit.go @@ -285,6 +285,7 @@ func doSubmitPushAndPR(g *git.Git, cfg *config.Config, root *tree.Node, branches // Phase 2: Push branches that will participate in PRs (or all if --skip-prs). fmt.Println(s.Bold("\n=== Phase 2: Push ===")) + var toPush []string for _, b := range branches { var d *prDecision if !opts.PushOnly { @@ -298,12 +299,17 @@ func doSubmitPushAndPR(g *git.Git, cfg *config.Config, root *tree.Node, branches if opts.DryRun { fmt.Printf("%s Would push %s -> origin/%s (forced)\n", s.Muted("dry-run:"), s.Branch(b.Name), s.Branch(b.Name)) } else { - fmt.Printf("Pushing %s -> origin/%s (forced)... ", s.Branch(b.Name), s.Branch(b.Name)) - if err := g.Push(b.Name, true); err != nil { - fmt.Println(s.Error("failed")) - return fmt.Errorf("failed to push %s: %w", b.Name, err) - } - fmt.Println(s.Success("ok")) + toPush = append(toPush, b.Name) + } + } + if !opts.DryRun && len(toPush) > 0 { + styled := make([]string, len(toPush)) + for i, name := range toPush { + styled[i] = s.Branch(name) + } + fmt.Printf("Pushing %s to origin (force-with-lease, atomic)...\n", strings.Join(styled, ", ")) + if err := g.PushMany(toPush, true); err != nil { + return fmt.Errorf("failed to push branches [%s]: %w", strings.Join(toPush, ", "), err) } } diff --git a/e2e/submit_test.go b/e2e/submit_test.go index 3225d07..43db415 100644 --- a/e2e/submit_test.go +++ b/e2e/submit_test.go @@ -491,3 +491,44 @@ func TestSubmitFromTrunkFallback(t *testing.T) { t.Error("should not push trunk branch") } } + +// TestSubmitBatchPush verifies that a multi-branch submit advances all remote +// refs in one atomic push and emits the expected summary line. +func TestSubmitBatchPush(t *testing.T) { + env := NewTestEnvWithRemote(t) + env.MustRun("init") + + // Build a 3-branch stack: main -> feat-a -> feat-b -> feat-c + env.MustRun("create", "feat-a") + tipA := env.CreateCommit("a work") + + env.MustRun("create", "feat-b") + tipB := env.CreateCommit("b work") + + env.MustRun("create", "feat-c") + tipC := env.CreateCommit("c work") + + result := env.MustRun("submit", "--skip-prs", "--yes") + + // Output should mention the batch push summary + if !strings.Contains(result.Stdout, "Pushing") { + t.Error("expected batch push summary line in output") + } + if !strings.Contains(result.Stdout, "atomic") { + t.Error("expected 'atomic' in push summary line") + } + + // All three remote refs must match local tips + remoteA := env.GitRemote("rev-parse", "refs/heads/feat-a") + if remoteA != tipA { + t.Errorf("remote feat-a = %s, want %s", remoteA, tipA) + } + remoteB := env.GitRemote("rev-parse", "refs/heads/feat-b") + if remoteB != tipB { + t.Errorf("remote feat-b = %s, want %s", remoteB, tipB) + } + remoteC := env.GitRemote("rev-parse", "refs/heads/feat-c") + if remoteC != tipC { + t.Errorf("remote feat-c = %s, want %s", remoteC, tipC) + } +} diff --git a/internal/git/git.go b/internal/git/git.go index 17c9efe..58e3665 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -173,12 +173,31 @@ func (g *Git) Commit(message string) error { return g.runSilent("commit", "-m", message) } -// Push force-pushes a branch to origin with lease. +// Push pushes a branch to origin. When force is true, --force-with-lease is +// used; when false, the push is a normal fast-forward push (and will fail if +// the remote has diverged). func (g *Git) Push(branch string, force bool) error { - args := []string{"push", "origin", branch} + return g.PushMany([]string{branch}, force) +} + +// PushMany pushes multiple branches to origin in a single invocation. +// When force is true, --force-with-lease and --atomic are added so that the +// push is all-or-nothing: if any ref is rejected (e.g. lease conflict), none +// of the refs are updated on the remote. +// Returns nil immediately when branches is empty. +// +// All flags are placed before a `--` end-of-options marker so refspecs that +// happen to start with `-` are never misinterpreted as git options. +func (g *Git) PushMany(branches []string, force bool) error { + if len(branches) == 0 { + return nil + } + args := []string{"push"} if force { - args = append(args, "--force-with-lease") + args = append(args, "--force-with-lease", "--atomic") } + args = append(args, "origin", "--") + args = append(args, branches...) return g.runInteractive(args...) } diff --git a/internal/git/git_test.go b/internal/git/git_test.go index d68b2e9..165c950 100644 --- a/internal/git/git_test.go +++ b/internal/git/git_test.go @@ -904,3 +904,173 @@ func TestRebaseNoUpdateRefsPreservesBookmark(t *testing.T) { t.Errorf("expected bookmark to be preserved with --no-update-refs, but it moved from %s to %s", bookmarkBefore, bookmarkAfter) } } + +// setupRepoWithRemote creates a local repo + bare remote and returns +// (localDir, remoteDir, *Git, trunk). The trunk branch is pushed to the +// remote and the remote is set as "origin". +func setupRepoWithRemote(t *testing.T) (dir, remoteDir string, g *git.Git, trunk string) { + t.Helper() + dir = t.TempDir() + + run := func(args ...string) string { + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v failed: %v\n%s", args, err, out) + } + return strings.TrimSpace(string(out)) + } + + run("init", "-b", "main") + run("config", "user.email", "test@test.com") + run("config", "user.name", "Test") + os.WriteFile(filepath.Join(dir, "root"), []byte("root"), 0644) + run("add", ".") + run("commit", "-m", "root") + + remoteDir = t.TempDir() + exec.Command("git", "clone", "--bare", dir, remoteDir).Run() //nolint:errcheck + run("remote", "add", "origin", remoteDir) + run("push", "-u", "origin", "main") + + g = git.New(dir) + trunk = "main" + return dir, remoteDir, g, trunk +} + +// remoteRef returns the SHA that a ref points to on the bare remote, or "" on error. +func remoteRef(t *testing.T, remoteDir, branch string) string { + t.Helper() + out, err := exec.Command("git", "-C", remoteDir, "rev-parse", "refs/heads/"+branch).Output() + if err != nil { + return "" + } + return strings.TrimSpace(string(out)) +} + +func TestPushManyHappyPath(t *testing.T) { + dir, remoteDir, g, _ := setupRepoWithRemote(t) + + run := func(args ...string) { + cmd := exec.Command("git", args...) + cmd.Dir = dir + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v failed: %v\n%s", args, err, out) + } + } + + // Create feat-a + run("checkout", "-b", "feat-a") + os.WriteFile(filepath.Join(dir, "a"), []byte("a"), 0644) + run("add", ".") + run("commit", "-m", "a") + tipA, _ := g.GetTip("feat-a") + + // Create feat-b on top + run("checkout", "-b", "feat-b") + os.WriteFile(filepath.Join(dir, "b"), []byte("b"), 0644) + run("add", ".") + run("commit", "-m", "b") + tipB, _ := g.GetTip("feat-b") + + // Both branches are local-only at this point; PushMany should create them on remote. + if err := g.PushMany([]string{"feat-a", "feat-b"}, false); err != nil { + t.Fatalf("PushMany failed: %v", err) + } + + if got := remoteRef(t, remoteDir, "feat-a"); got != tipA { + t.Errorf("remote feat-a = %s, want %s", got, tipA) + } + if got := remoteRef(t, remoteDir, "feat-b"); got != tipB { + t.Errorf("remote feat-b = %s, want %s", got, tipB) + } +} + +func TestPushManyEmpty(t *testing.T) { + _, _, g, _ := setupRepoWithRemote(t) + // Should be a no-op with no error. + if err := g.PushMany(nil, true); err != nil { + t.Fatalf("PushMany(nil) returned unexpected error: %v", err) + } + if err := g.PushMany([]string{}, true); err != nil { + t.Fatalf("PushMany([]) returned unexpected error: %v", err) + } +} + +// TestPushManyAtomicRejection verifies that when one branch fails --force-with-lease, +// the --atomic flag prevents the other branch from being updated on the remote. +func TestPushManyAtomicRejection(t *testing.T) { + dir, remoteDir, g, _ := setupRepoWithRemote(t) + + run := func(args ...string) { + cmd := exec.Command("git", args...) + cmd.Dir = dir + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v failed: %v\n%s", args, err, out) + } + } + + // Create and push feat-a + run("checkout", "-b", "feat-a") + os.WriteFile(filepath.Join(dir, "a"), []byte("a"), 0644) + run("add", ".") + run("commit", "-m", "a") + run("push", "origin", "feat-a") + + // Create feat-b (will be pushed clean — no prior remote ref) + run("checkout", "main") + run("checkout", "-b", "feat-b") + os.WriteFile(filepath.Join(dir, "b"), []byte("b"), 0644) + run("add", ".") + run("commit", "-m", "b") + tipBLocal, _ := g.GetTip("feat-b") + run("push", "origin", "feat-b") + + // Diverge feat-a on the remote by committing & pushing through a temp + // clone. This advances refs/heads/feat-a on the bare remote out from + // under our local clone, so our next force-with-lease for feat-a will + // see a stale lease and be rejected. + tmp := t.TempDir() + cloneTmp := exec.Command("git", "clone", remoteDir, tmp) + if out, err := cloneTmp.CombinedOutput(); err != nil { + t.Fatalf("git clone (divergence) failed: %v\n%s", err, out) + } + tmpRun := func(args ...string) { + cmd := exec.Command("git", args...) + cmd.Dir = tmp + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v (divergence) failed: %v\n%s", args, err, out) + } + } + tmpRun("config", "user.email", "t@t.com") + tmpRun("config", "user.name", "T") + tmpRun("checkout", "feat-a") + os.WriteFile(filepath.Join(tmp, "remote-diverge"), []byte("x"), 0644) + tmpRun("add", ".") + tmpRun("commit", "-m", "diverge") + tmpRun("push", "origin", "feat-a") + + // Add a new commit to feat-b locally (so we're trying to advance it) + run("checkout", "feat-b") + os.WriteFile(filepath.Join(dir, "b2"), []byte("b2"), 0644) + run("add", ".") + run("commit", "-m", "b2") + + // PushMany should fail because feat-a's lease is broken. + err := g.PushMany([]string{"feat-a", "feat-b"}, true) + if err == nil { + t.Fatal("expected PushMany to fail due to lease rejection on feat-a, but it succeeded") + } + + // Due to --atomic, feat-b must remain at exactly its pre-push value on + // the remote. Assert strict equality (and fail loudly if the remote ref + // cannot be read) so a missing/blank rev-parse can't masquerade as success. + remoteB := remoteRef(t, remoteDir, "feat-b") + if remoteB == "" { + t.Fatal("could not read remote feat-b SHA after PushMany failure") + } + if remoteB != tipBLocal { + t.Errorf("remote feat-b = %s, want unchanged %s — --atomic did not hold", remoteB, tipBLocal) + } +}