Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 26 additions & 8 deletions cmd/submit.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,21 +230,39 @@ 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)
// Push branches, then create/update each PR in stack order (bottom to top).
//
// Normally all active branches go up in a single atomic push. GitHub then
// receives every rewritten ref in one push event, which lets it synchronize
// each PR once (for its head ref) rather than once for its head and again
// for its base, and the remote never holds a partially-updated stack.
//
// When a modify is pending, the stack's shape may have changed (branches
// reordered, dropped, or folded), so PR bases on GitHub can be stale. In
// that case push branches one at a time and fix each PR's base before
// pushing the next, so no PR is compared against an outdated base.
sequentialPush := pendingModify
if sequentialPush {
cfg.Printf("Pushing to %s...", remote)
} else {
cfg.Printf("Pushing %d %s to %s...", len(activeBranches), plural(len(activeBranches), "branch", "branches"), remote)
if err := git.Push(remote, activeBranches, true, true); 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 {
if s.Branches[i].IsMerged() || s.Branches[i].IsQueued() {
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
if sequentialPush {
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
Expand Down
64 changes: 55 additions & 9 deletions cmd/submit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,11 +151,12 @@ func TestSubmit_CreatesPRsAndStack(t *testing.T) {

assert.NoError(t, err)

// Branches should be pushed (sequentially, one per branch)
require.Len(t, pushCalls, 2)
// All branches are pushed together in a single atomic, forced push
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)
Expand Down Expand Up @@ -346,6 +347,52 @@ func TestSubmit_PushFailure(t *testing.T) {
assert.Contains(t, output, "failed to push")
}

// TestSubmit_AtomicPushFailureCreatesNoPRs verifies that, without a pending
// modify, a rejected push aborts before any PR is created or updated.
func TestSubmit_AtomicPushFailureCreatesNoPRs(t *testing.T) {
s := stack.Stack{
Trunk: stack.BranchRef{Branch: "main"},
Branches: []stack.BranchRef{
{Branch: "b1"},
{Branch: "b2"},
{Branch: "b3"},
},
}

tmpDir := t.TempDir()
writeStackFile(t, tmpDir, s)

var pushCalls []pushCall
mock := newSubmitMock(tmpDir, "b1")
mock.PushFn = func(remote string, branches []string, force, atomic bool) error {
pushCalls = append(pushCalls, pushCall{remote, branches, force, atomic})
return fmt.Errorf("stale info for b2")
}

restore := git.SetOps(mock)
defer restore()

creations := 0
cfg, _, errR := config.NewTestConfig()
cfg.GitHubClientOverride = &github.MockClient{
CreatePRFn: func(_, _, _, _ string, _ bool) (*github.PullRequest, error) {
creations++
return &github.PullRequest{Number: 1}, nil
},
}
err := runSubmit(cfg, &submitOptions{auto: true})

cfg.Err.Close()
errOut, _ := io.ReadAll(errR)

assert.ErrorIs(t, err, ErrSilent)
assert.Contains(t, string(errOut), "failed to push")
require.Len(t, pushCalls, 1)
assert.Equal(t, []string{"b1", "b2", "b3"}, pushCalls[0].branches)
assert.True(t, pushCalls[0].atomic)
assert.Zero(t, creations, "no PR should be created after a failed push")
}

func TestSubmit_SkipsMergedBranches(t *testing.T) {
s := stack.Stack{
Trunk: stack.BranchRef{Branch: "main"},
Expand Down Expand Up @@ -489,9 +536,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")
Expand Down Expand Up @@ -2059,7 +2105,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))
Expand Down Expand Up @@ -2563,7 +2609,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)")
require.Equal(t, []string{"fetch", "push"}, callOrder, "expected fetch followed by a single push")
assert.Equal(t, "fetch", callOrder[0], "fetch must happen before any push")
}

Expand Down
2 changes: 1 addition & 1 deletion docs/src/content/docs/reference/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ gh stack submit [flags]
| `--open` | Create new PRs as ready for review instead of drafts, and mark existing PRs as ready for review |
| `--remote <name>` | Remote to push to (defaults to auto-detected remote) |

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.
Creates a Stacked PR for every branch in the stack, pushing branches to the remote. All active branches are pushed together in a single atomic push (with per-branch `--force-with-lease` checks), so either every branch is updated or none are. When finishing a pending `gh stack modify`, branches are instead pushed one at a time so each PR's base can be corrected before the next branch is 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.

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.

Expand Down
Loading