run fetch before push operations

skarim · skarim · commit 3099f2005300 · 2026-05-05T00:23:35.000-04:00
diff --git a/cmd/link.go b/cmd/link.go
@@ -161,6 +161,10 @@ func pushBranchArgs(cfg *config.Config, opts *linkOptions, args []string) error
 		return ErrSilent
 	}
 
+	if err := git.FetchBranches(remote, branches); err != nil {
+		cfg.Warningf("Failed to fetch branches from %s: %v", remote, err)
+	}
+
 	cfg.Printf("Pushing %d %s to %s...", len(branches), plural(len(branches), "branch", "branches"), remote)
 	if err := git.Push(remote, branches, false, true); err != nil {
 		cfg.Errorf("failed to push branches: %s", err)
diff --git a/cmd/link_test.go b/cmd/link_test.go
@@ -1096,3 +1096,59 @@ func TestLink_SkipsBaseFix_ForNewlyCreatedPRs(t *testing.T) {
 
 // Silence "imported and not used" for fmt in case test helpers use it.
 var _ = fmt.Sprintf
+
+func TestLink_FetchesBeforePush(t *testing.T) {
+	var callOrder []string
+	var fetchedBranches []string
+
+	mock := newLinkGitMock("feat-a", "feat-b")
+	mock.FetchBranchesFn = func(remote string, branches []string) error {
+		callOrder = append(callOrder, "fetch")
+		fetchedBranches = branches
+		assert.Equal(t, "origin", remote)
+		return nil
+	}
+	mock.PushFn = func(remote string, branches []string, force, atomic bool) error {
+		callOrder = append(callOrder, "push")
+		return nil
+	}
+
+	restore := git.SetOps(mock)
+	defer restore()
+
+	prNum := 0
+	cfg, _, errR := config.NewTestConfig()
+	cfg.GitHubClientOverride = &github.MockClient{
+		FindPRForBranchFn: func(branch string) (*github.PullRequest, error) {
+			prNum++
+			return &github.PullRequest{
+				Number:      prNum,
+				URL:         fmt.Sprintf("https://github.com/o/r/pull/%d", prNum),
+				BaseRefName: "main",
+				HeadRefName: branch,
+				State:       "OPEN",
+			}, nil
+		},
+		ListStacksFn: func() ([]github.RemoteStack, error) {
+			return []github.RemoteStack{}, nil
+		},
+		CreateStackFn: func(prNumbers []int) (int, error) {
+			return 42, nil
+		},
+	}
+
+	cmd := LinkCmd(cfg)
+	cmd.SetArgs([]string{"feat-a", "feat-b"})
+	cmd.SetOut(io.Discard)
+	cmd.SetErr(io.Discard)
+	err := cmd.Execute()
+
+	cfg.Err.Close()
+	_, _ = io.ReadAll(errR)
+
+	assert.NoError(t, err)
+	assert.Equal(t, []string{"feat-a", "feat-b"}, fetchedBranches, "should fetch pushed branches")
+	require.Len(t, callOrder, 2)
+	assert.Equal(t, "fetch", callOrder[0], "fetch must happen before push")
+	assert.Equal(t, "push", callOrder[1])
+}
diff --git a/cmd/push.go b/cmd/push.go
@@ -93,8 +93,11 @@ func runPush(cfg *config.Config, opts *pushOptions) error {
 		cfg.Printf("No active branches to push (all merged or queued)")
 		return nil
 	}
+	if err := git.FetchBranches(remote, activeBranches); err != nil {
+		cfg.Warningf("Failed to fetch branches from %s: %v", remote, err)
+	}
 	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 {
+	if err := git.Push(remote, activeBranches, true, false); err != nil {
 		cfg.Errorf("failed to push: %s", err)
 		return ErrSilent
 	}
diff --git a/cmd/push_test.go b/cmd/push_test.go
@@ -62,7 +62,7 @@ func TestPush_PushesAllBranches(t *testing.T) {
 	assert.Equal(t, "origin", pushCalls[0].remote)
 	assert.Equal(t, []string{"b1", "b2"}, pushCalls[0].branches)
 	assert.True(t, pushCalls[0].force)
-	assert.True(t, pushCalls[0].atomic)
+	assert.False(t, pushCalls[0].atomic)
 	assert.Contains(t, output, "Pushed 2 branches")
 	assert.Contains(t, output, "gh stack submit", "should hint about submit when branches have no PRs")
 }
@@ -182,6 +182,89 @@ func TestPush_PushFailure(t *testing.T) {
 	assert.Contains(t, output, "failed to push")
 }
 
+func TestPush_FetchesBeforePush(t *testing.T) {
+	s := stack.Stack{
+		Trunk: stack.BranchRef{Branch: "main"},
+		Branches: []stack.BranchRef{
+			{Branch: "b1"},
+			{Branch: "b2"},
+		},
+	}
+
+	tmpDir := t.TempDir()
+	writeStackFile(t, tmpDir, s)
+
+	var callOrder []string
+
+	mock := newPushMock(tmpDir, "b1")
+	mock.FetchBranchesFn = func(remote string, branches []string) error {
+		callOrder = append(callOrder, "fetch")
+		assert.Equal(t, "origin", remote)
+		assert.Equal(t, []string{"b1", "b2"}, branches)
+		return nil
+	}
+	mock.PushFn = func(remote string, branches []string, force, atomic bool) error {
+		callOrder = append(callOrder, "push")
+		return nil
+	}
+
+	restore := git.SetOps(mock)
+	defer restore()
+
+	cfg, _, errR := config.NewTestConfig()
+	cfg.GitHubClientOverride = &github.MockClient{}
+	cmd := PushCmd(cfg)
+	cmd.SetOut(io.Discard)
+	cmd.SetErr(io.Discard)
+	err := cmd.Execute()
+
+	cfg.Err.Close()
+	_, _ = io.ReadAll(errR)
+
+	assert.NoError(t, err)
+	assert.Equal(t, []string{"fetch", "push"}, callOrder, "fetch must happen before push")
+}
+
+func TestPush_FetchFailureIsNonFatal(t *testing.T) {
+	s := stack.Stack{
+		Trunk: stack.BranchRef{Branch: "main"},
+		Branches: []stack.BranchRef{
+			{Branch: "b1"},
+		},
+	}
+
+	tmpDir := t.TempDir()
+	writeStackFile(t, tmpDir, s)
+
+	pushCalled := false
+
+	mock := newPushMock(tmpDir, "b1")
+	mock.FetchBranchesFn = func(string, []string) error {
+		return fmt.Errorf("network error")
+	}
+	mock.PushFn = func(string, []string, bool, bool) error {
+		pushCalled = true
+		return nil
+	}
+
+	restore := git.SetOps(mock)
+	defer restore()
+
+	cfg, _, errR := config.NewTestConfig()
+	cfg.GitHubClientOverride = &github.MockClient{}
+	cmd := PushCmd(cfg)
+	cmd.SetOut(io.Discard)
+	cmd.SetErr(io.Discard)
+	err := cmd.Execute()
+
+	cfg.Err.Close()
+	errOut, _ := io.ReadAll(errR)
+
+	assert.NoError(t, err, "fetch failure should not abort push")
+	assert.True(t, pushCalled, "push should proceed after fetch failure")
+	assert.Contains(t, string(errOut), "Failed to fetch")
+}
+
 func TestPush_DoesNotCreatePRs(t *testing.T) {
 	s := stack.Stack{
 		Trunk: stack.BranchRef{Branch: "main"},
diff --git a/cmd/submit.go b/cmd/submit.go
@@ -129,7 +129,7 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error {
 		return nil
 	}
 
-	// If a modification is pending, delete the old remote stack first so that
+	// If a modification is pending, delete the old remote stack first so that
 	// PR base updates are allowed and force-pushes don't trigger auto-merges.
 	if stacksAvailable {
 		if err := handlePendingModify(cfg, client, s, gitDir); err != nil {
@@ -141,6 +141,13 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error {
 		}
 	}
 
+	// Fetch the active branches from the remote so that the local tracking refs
+	// are up-to-date before pushing. This ensures --force-with-lease has accurate
+	// remote state even in shallow clones.
+	if err := git.FetchBranches(remote, activeBranches); err != nil {
+		cfg.Warningf("Failed to fetch branches from %s: %v", remote, err)
+	}
+
 	// 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.
diff --git a/cmd/submit_test.go b/cmd/submit_test.go
@@ -1370,3 +1370,68 @@ func TestSubmit_WithPendingModify_SequentialPush(t *testing.T) {
 	// State file should be cleared
 	assert.False(t, modify.StateExists(tmpDir), "modify state file should be cleared after success")
 }
+
+func TestSubmit_FetchesBeforePush(t *testing.T) {
+	s := stack.Stack{
+		Trunk: stack.BranchRef{Branch: "main"},
+		Branches: []stack.BranchRef{
+			{Branch: "b1"},
+			{Branch: "b2"},
+		},
+	}
+
+	tmpDir := t.TempDir()
+	writeStackFile(t, tmpDir, s)
+
+	var callOrder []string
+	var fetchedBranches []string
+
+	mock := newSubmitMock(tmpDir, "b1")
+	mock.FetchBranchesFn = func(remote string, branches []string) error {
+		callOrder = append(callOrder, "fetch")
+		fetchedBranches = branches
+		assert.Equal(t, "origin", remote)
+		return nil
+	}
+	mock.PushFn = func(remote string, branches []string, force, atomic bool) error {
+		callOrder = append(callOrder, "push")
+		return nil
+	}
+
+	restore := git.SetOps(mock)
+	defer restore()
+
+	cfg, _, errR := config.NewTestConfig()
+	cfg.GitHubClientOverride = &github.MockClient{
+		FindPRForBranchFn: func(branch string) (*github.PullRequest, error) {
+			return &github.PullRequest{
+				Number:      1,
+				URL:         "https://github.com/o/r/pull/1",
+				BaseRefName: "main",
+				HeadRefName: branch,
+				State:       "OPEN",
+			}, nil
+		},
+		ListStacksFn: func() ([]github.RemoteStack, error) {
+			return []github.RemoteStack{}, nil
+		},
+		CreateStackFn: func(prNumbers []int) (int, error) {
+			return 42, nil
+		},
+	}
+
+	cmd := SubmitCmd(cfg)
+	cmd.SetArgs([]string{"--auto"})
+	cmd.SetOut(io.Discard)
+	cmd.SetErr(io.Discard)
+	err := cmd.Execute()
+
+	cfg.Err.Close()
+	_, _ = io.ReadAll(errR)
+
+	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)")
+	assert.Equal(t, "fetch", callOrder[0], "fetch must happen before any push")
+}
diff --git a/internal/git/git.go b/internal/git/git.go
@@ -124,6 +124,12 @@ func Fetch(remote string) error {
 	return ops.Fetch(remote)
 }
 
+// FetchBranches fetches specific branches from a remote,
+// updating their tracking refs.
+func FetchBranches(remote string, branches []string) error {
+	return ops.FetchBranches(remote, branches)
+}
+
 // DefaultBranch returns the HEAD branch from origin.
 func DefaultBranch() (string, error) {
 	return ops.DefaultBranch()
diff --git a/internal/git/gitops.go b/internal/git/gitops.go
@@ -21,6 +21,7 @@ type Ops interface {
 	BranchExists(name string) bool
 	CheckoutBranch(name string) error
 	Fetch(remote string) error
+	FetchBranches(remote string, branches []string) error
 	DefaultBranch() (string, error)
 	CreateBranch(name, base string) error
 	Push(remote string, branches []string, force, atomic bool) error
@@ -106,6 +107,15 @@ func (d *defaultOps) Fetch(remote string) error {
 	return client.Fetch(context.Background(), remote, "")
 }
 
+func (d *defaultOps) FetchBranches(remote string, branches []string) error {
+	if len(branches) == 0 {
+		return nil
+	}
+	args := []string{"fetch", remote}
+	args = append(args, branches...)
+	return runSilent(args...)
+}
+
 func (d *defaultOps) DefaultBranch() (string, error) {
 	ref, err := run("symbolic-ref", "refs/remotes/origin/HEAD")
 	if err != nil {
diff --git a/internal/git/mock_ops.go b/internal/git/mock_ops.go
@@ -9,6 +9,7 @@ type MockOps struct {
 	BranchExistsFn        func(string) bool
 	CheckoutBranchFn      func(string) error
 	FetchFn               func(string) error
+	FetchBranchesFn       func(string, []string) error
 	DefaultBranchFn       func() (string, error)
 	CreateBranchFn        func(string, string) error
 	PushFn                func(string, []string, bool, bool) error
@@ -87,6 +88,13 @@ func (m *MockOps) Fetch(remote string) error {
 	return nil
 }
 
+func (m *MockOps) FetchBranches(remote string, branches []string) error {
+	if m.FetchBranchesFn != nil {
+		return m.FetchBranchesFn(remote, branches)
+	}
+	return nil
+}
+
 func (m *MockOps) DefaultBranch() (string, error) {
 	if m.DefaultBranchFn != nil {
 		return m.DefaultBranchFn()

Original file line number	Diff line number	Diff line change
`@@ -93,8 +93,11 @@ func runPush(cfg config.Config, opts pushOptions) error {`
`93`	`93`	`cfg.Printf("No active branches to push (all merged or queued)")`
`94`	`94`	`return nil`
`95`	`95`	`}`
	`96`	`+ if err := git.FetchBranches(remote, activeBranches); err != nil {`
	`97`	`+ cfg.Warningf("Failed to fetch branches from %s: %v", remote, err)`
	`98`	`+ }`
`96`	`99`	`cfg.Printf("Pushing %d %s to %s...", len(activeBranches), plural(len(activeBranches), "branch", "branches"), remote)`
`97`		`- if err := git.Push(remote, activeBranches, true, true); err != nil {`
	`100`	`+ if err := git.Push(remote, activeBranches, true, false); err != nil {`
`98`	`101`	`cfg.Errorf("failed to push: %s", err)`
`99`	`102`	`return ErrSilent`
`100`	`103`	`}`