bugfixes for rebase, sync, and view

README.md

@@ -197,7 +197,7 @@ Pull from remote and do a cascading rebase across the stack.
 gh stack rebase [flags] [branch]
 ```
 
-Fetches the latest changes from `origin`, then ensures each branch in the stack has the tip of the previous layer in its commit history. Rebases branches in order from trunk upward. If a branch's PR has been squash-merged, the rebase automatically switches to `--onto` mode to correctly replay commits on top of the merge target.
+Fetches the latest changes from `origin`, then ensures each branch in the stack has the tip of the previous layer in its commit history. Rebases branches in order from trunk upward. If a branch's PR has been merged, the rebase automatically switches to `--onto` mode to correctly replay commits on top of the merge target.
 
 If a rebase conflict occurs, the operation pauses and prints the conflicted files with line numbers. Resolve the conflicts, stage with `git add`, and continue with `--continue`. To undo the entire rebase, use `--abort` to restore all branches to their pre-rebase state.
 

cmd/rebase.go

@@ -135,6 +135,9 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
 		}
 	}
 
+	// Fast-forward stack branches that are behind their remote tracking branch.
+	fastForwardBranches(cfg, s, remote, currentBranch)
+
 	cfg.Printf("Stack detected: %s", s.DisplayChain())
 
 	currentIdx := s.IndexOf(currentBranch)
@@ -169,17 +172,23 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
 	// Sync PR state before rebase so we can detect merged PRs.
 	syncStackPRs(cfg, s)
 
-	branchNames := make([]string, len(s.Branches))
-	for i, b := range s.Branches {
-		branchNames[i] = b.Branch
+	branchNames := make([]string, 0, len(s.Branches))
+	for _, b := range s.Branches {
+		// Merged branches that no longer exist locally have no ref to
+		// resolve. They are always skipped during rebase, but we must
+		// also exclude them here to avoid a rev-parse error.
+		if b.IsMerged() && !git.BranchExists(b.Branch) {
+			continue
+		}
+		branchNames = append(branchNames, b.Branch)
 	}
 	originalRefs, err := git.RevParseMap(branchNames)
 	if err != nil {
 		cfg.Errorf("failed to resolve branch SHAs: %s", err)
 		return ErrSilent
 	}
 
-	// Track --onto rebase state for squash-merged branches.
+	// Track --onto rebase state for merged branches.
 	needsOnto := false
 	var ontoOldBase string
 
@@ -192,7 +201,7 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
 			base = s.Branches[absIdx-1].Branch
 		}
 
-		// Skip branches whose PRs have already been merged (e.g. via squash).
+		// Skip branches whose PRs have already been merged.
 		// Record state so subsequent branches can use --onto rebase.
 		if br.IsMerged() {
 			ontoOldBase = originalRefs[br.Branch]
@@ -243,7 +252,7 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
 				return ErrConflict
 			}
 
-			cfg.Successf("Rebased %s onto %s (squash-merge detected)", br.Branch, newBase)
+			cfg.Successf("Rebased %s onto %s (adjusted for merged PR)", br.Branch, newBase)
 			// Keep --onto mode; update old base for the next branch.
 			ontoOldBase = originalRefs[br.Branch]
 		} else {
@@ -441,7 +450,7 @@ func continueRebase(cfg *config.Config, gitDir string) error {
 				return ErrConflict
 			}
 
-			cfg.Successf("Rebased %s onto %s (squash-merge detected)", branchName, newBase)
+			cfg.Successf("Rebased %s onto %s (adjusted for merged PR)", branchName, newBase)
 			state.OntoOldBase = state.OriginalRefs[branchName]
 		} else {
 			var rebaseErr error

cmd/rebase_test.go

@@ -5,6 +5,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/github/gh-stack/internal/config"
@@ -34,7 +35,13 @@ func newRebaseMock(tmpDir string, currentBranch string) *git.MockOps {
 	return &git.MockOps{
 		GitDirFn:        func() (string, error) { return tmpDir, nil },
 		CurrentBranchFn: func() (string, error) { return currentBranch, nil },
-		RevParseFn:       func(ref string) (string, error) { return "sha-" + ref, nil },
+		RevParseFn: func(ref string) (string, error) {
+			// Default: origin/<branch> returns same SHA as <branch> (no FF needed)
+			if strings.HasPrefix(ref, "origin/") {
+				return "sha-" + strings.TrimPrefix(ref, "origin/"), nil
+			}
+			return "sha-" + ref, nil
+		},
 		IsAncestorFn:    func(a, d string) (bool, error) { return true, nil },
 		FetchFn:         func(string) error { return nil },
 		EnableRerereFn:  func() error { return nil },
@@ -99,10 +106,10 @@ func TestRebase_CascadeRebase(t *testing.T) {
 	assert.Contains(t, output, "rebased locally")
 }
 
-// TestRebase_SquashMergedBranch_UsesOnto verifies that when b1 has a merged PR,
+// TestRebase_MergedBranch_UsesOnto verifies that when b1 has a merged PR,
 // it is skipped and b2 uses RebaseOnto with trunk as newBase and b1's original
 // SHA as oldBase. b3 also uses --onto (propagation).
-func TestRebase_SquashMergedBranch_UsesOnto(t *testing.T) {
+func TestRebase_MergedBranch_UsesOnto(t *testing.T) {
 	s := stack.Stack{
 		Trunk: stack.BranchRef{Branch: "main"},
 		Branches: []stack.BranchRef{
@@ -126,6 +133,7 @@ func TestRebase_SquashMergedBranch_UsesOnto(t *testing.T) {
 	}
 
 	mock := newRebaseMock(tmpDir, "b2")
+	mock.BranchExistsFn = func(name string) bool { return true }
 	mock.RevParseFn = func(ref string) (string, error) {
 		if sha, ok := branchSHAs[ref]; ok {
 			return sha, nil
@@ -163,7 +171,7 @@ func TestRebase_SquashMergedBranch_UsesOnto(t *testing.T) {
 }
 
 // TestRebase_OntoPropagatesToSubsequentBranches verifies that when multiple
-// branches are squash-merged, --onto propagates correctly through the chain.
+// branches are merged, --onto propagates correctly through the chain.
 func TestRebase_OntoPropagatesToSubsequentBranches(t *testing.T) {
 	s := stack.Stack{
 		Trunk: stack.BranchRef{Branch: "main"},
@@ -190,6 +198,7 @@ func TestRebase_OntoPropagatesToSubsequentBranches(t *testing.T) {
 	}
 
 	mock := newRebaseMock(tmpDir, "b3")
+	mock.BranchExistsFn = func(name string) bool { return true }
 	mock.RevParseFn = func(ref string) (string, error) {
 		if sha, ok := branchSHAs[ref]; ok {
 			return sha, nil
@@ -642,7 +651,7 @@ func TestRebase_Continue_RebasesRemainingBranches(t *testing.T) {
 }
 
 // TestRebase_Continue_OntoMode verifies the --continue path when UseOnto is
-// set (squash-merged branches upstream). With no remaining branches, only
+// set (merged branches upstream). With no remaining branches, only
 // RebaseContinue runs and the state is cleaned up.
 func TestRebase_Continue_OntoMode(t *testing.T) {
 	s := stack.Stack{
@@ -848,3 +857,245 @@ func TestRebase_Abort_WithActiveRebase(t *testing.T) {
 	// Should return to original branch
 	assert.Contains(t, checkouts, "b1", "should checkout original branch at end")
 }
+
+// TestRebase_FastForwardsBranchFromRemote verifies that when origin/b1 is ahead
+// of local b1 (someone pushed a new commit), the branch is fast-forwarded before
+// the cascade rebase so downstream branches include the new commits.
+func TestRebase_FastForwardsBranchFromRemote(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 allRebaseCalls []rebaseCall
+	var updateBranchRefCalls []struct{ branch, sha string }
+
+	mock := newRebaseMock(tmpDir, "b2")
+	// b1 is behind origin/b1 (remote has new commit)
+	mock.RevParseFn = func(ref string) (string, error) {
+		if ref == "b1" {
+			return "b1-local-sha", nil
+		}
+		if ref == "origin/b1" {
+			return "b1-remote-sha", nil
+		}
+		// trunk and origin/trunk same — trunk already up to date
+		if ref == "main" || ref == "origin/main" {
+			return "main-sha", nil
+		}
+		if strings.HasPrefix(ref, "origin/") {
+			return "sha-" + strings.TrimPrefix(ref, "origin/"), nil
+		}
+		return "sha-" + ref, nil
+	}
+	mock.IsAncestorFn = func(a, d string) (bool, error) {
+		// b1-local is ancestor of b1-remote → can fast-forward
+		if a == "b1-local-sha" && d == "b1-remote-sha" {
+			return true, nil
+		}
+		return false, nil
+	}
+	mock.UpdateBranchRefFn = func(branch, sha string) error {
+		updateBranchRefCalls = append(updateBranchRefCalls, struct{ branch, sha string }{branch, sha})
+		return nil
+	}
+	mock.CheckoutBranchFn = func(string) error { return nil }
+	mock.RebaseFn = func(base string) error {
+		allRebaseCalls = append(allRebaseCalls, rebaseCall{newBase: base})
+		return nil
+	}
+	mock.RebaseOntoFn = func(newBase, oldBase, branch string) error {
+		allRebaseCalls = append(allRebaseCalls, rebaseCall{newBase, oldBase, branch})
+		return nil
+	}
+
+	restore := git.SetOps(mock)
+	defer restore()
+
+	cfg, _, errR := config.NewTestConfig()
+	cmd := RebaseCmd(cfg)
+	cmd.SetOut(io.Discard)
+	cmd.SetErr(io.Discard)
+	err := cmd.Execute()
+
+	cfg.Err.Close()
+	errOut, _ := io.ReadAll(errR)
+	output := string(errOut)
+
+	assert.NoError(t, err)
+	// b1 should be fast-forwarded to remote SHA
+	require.Len(t, updateBranchRefCalls, 1, "should fast-forward b1 via UpdateBranchRef")
+	assert.Equal(t, "b1", updateBranchRefCalls[0].branch)
+	assert.Equal(t, "b1-remote-sha", updateBranchRefCalls[0].sha)
+
+	assert.Contains(t, output, "Fast-forwarded b1")
+
+	// Cascade rebase should still occur
+	assert.NotEmpty(t, allRebaseCalls, "cascade rebase should still happen")
+}
+
+// TestRebase_BranchAlreadyUpToDate_NoFF verifies that when a branch's local
+// and remote SHAs match, no fast-forward occurs.
+func TestRebase_BranchAlreadyUpToDate_NoFF(t *testing.T) {
+	s := stack.Stack{
+		Trunk: stack.BranchRef{Branch: "main"},
+		Branches: []stack.BranchRef{
+			{Branch: "b1"},
+		},
+	}
+
+	tmpDir := t.TempDir()
+	writeStackFile(t, tmpDir, s)
+
+	var updateBranchRefCalls int
+	var mergeFFCalls int
+
+	mock := newRebaseMock(tmpDir, "b1")
+	// Same SHA for b1 and origin/b1 — already up to date (default mock handles this)
+	mock.UpdateBranchRefFn = func(string, string) error {
+		updateBranchRefCalls++
+		return nil
+	}
+	mock.MergeFFFn = func(string) error {
+		mergeFFCalls++
+		return nil
+	}
+	mock.CheckoutBranchFn = func(string) error { return nil }
+	mock.RebaseFn = func(string) error { return nil }
+
+	restore := git.SetOps(mock)
+	defer restore()
+
+	cfg, _, _ := config.NewTestConfig()
+	cmd := RebaseCmd(cfg)
+	cmd.SetOut(io.Discard)
+	cmd.SetErr(io.Discard)
+	err := cmd.Execute()
+
+	cfg.Out.Close()
+	cfg.Err.Close()
+
+	assert.NoError(t, err)
+	assert.Equal(t, 0, updateBranchRefCalls, "no UpdateBranchRef for branches already up to date")
+	assert.Equal(t, 0, mergeFFCalls, "no MergeFF for branches already up to date")
+}
+
+// TestRebase_BranchDiverged_NoFF verifies that when local and remote branches
+// have diverged (e.g., after a previous local rebase), no fast-forward occurs.
+func TestRebase_BranchDiverged_NoFF(t *testing.T) {
+	s := stack.Stack{
+		Trunk: stack.BranchRef{Branch: "main"},
+		Branches: []stack.BranchRef{
+			{Branch: "b1"},
+		},
+	}
+
+	tmpDir := t.TempDir()
+	writeStackFile(t, tmpDir, s)
+
+	var updateBranchRefCalls int
+
+	mock := newRebaseMock(tmpDir, "b1")
+	// Different SHAs for b1 and origin/b1
+	mock.RevParseFn = func(ref string) (string, error) {
+		if ref == "b1" {
+			return "b1-local-sha", nil
+		}
+		if ref == "origin/b1" {
+			return "b1-remote-sha", nil
+		}
+		if ref == "main" || ref == "origin/main" {
+			return "main-sha", nil
+		}
+		return "sha-" + ref, nil
+	}
+	// Neither is ancestor of the other — diverged
+	mock.IsAncestorFn = func(a, d string) (bool, error) {
+		return false, nil
+	}
+	mock.UpdateBranchRefFn = func(string, string) error {
+		updateBranchRefCalls++
+		return nil
+	}
+	mock.CheckoutBranchFn = func(string) error { return nil }
+	mock.RebaseFn = func(string) error { return nil }
+
+	restore := git.SetOps(mock)
+	defer restore()
+
+	cfg, _, _ := config.NewTestConfig()
+	cmd := RebaseCmd(cfg)
+	cmd.SetOut(io.Discard)
+	cmd.SetErr(io.Discard)
+	err := cmd.Execute()
+
+	cfg.Out.Close()
+	cfg.Err.Close()
+
+	assert.NoError(t, err)
+	assert.Equal(t, 0, updateBranchRefCalls, "no FF when branches have diverged")
+}
+
+func TestRebase_SkipsMergedBranchesNotExistingLocally(t *testing.T) {
+	// Simulates a stack where b1 is merged and its branch was auto-deleted
+	// from the remote, so it doesn't exist locally.
+	s := stack.Stack{
+		Trunk: stack.BranchRef{Branch: "main"},
+		Branches: []stack.BranchRef{
+			{Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 42, Merged: true}},
+			{Branch: "b2"},
+		},
+	}
+
+	tmpDir := t.TempDir()
+	writeStackFile(t, tmpDir, s)
+
+	var rebaseCalls []rebaseCall
+
+	mock := newRebaseMock(tmpDir, "b2")
+	mock.BranchExistsFn = func(name string) bool {
+		// b1 does not exist locally (deleted from remote after merge)
+		return name != "b1"
+	}
+	mock.RevParseMultiFn = func(refs []string) ([]string, error) {
+		// Only resolve refs that exist — b1 should not be in the list
+		shas := make([]string, len(refs))
+		for i, r := range refs {
+			if r == "b1" {
+				t.Fatalf("RevParseMulti should not be called with non-existent branch b1")
+			}
+			shas[i] = "sha-" + r
+		}
+		return shas, nil
+	}
+	mock.RebaseOntoFn = func(newBase, oldBase, branch string) error {
+		rebaseCalls = append(rebaseCalls, rebaseCall{newBase, oldBase, branch})
+		return nil
+	}
+
+	restore := git.SetOps(mock)
+	defer restore()
+
+	cfg, _, errR := config.NewTestConfig()
+	cmd := RebaseCmd(cfg)
+	cmd.SetOut(io.Discard)
+	cmd.SetErr(io.Discard)
+	err := cmd.Execute()
+
+	cfg.Err.Close()
+	errOut, _ := io.ReadAll(errR)
+	output := string(errOut)
+
+	assert.NoError(t, err)
+	assert.Contains(t, output, "Skipping b1")
+
+	// Only b2 should be rebased
+	require.Len(t, rebaseCalls, 1)
+	assert.Equal(t, "b2", rebaseCalls[0].branch)
+}