Keep the recorded base when a parent moves out of band

Amending a commit on a lower branch, pushing, and then rebasing replayed the
old version of that commit into every branch above it — a conflict at best, a
duplicated commit at worst (#250, #193). The same defect made a squash-merged
bottom PR conflict against the branches above its direct child (#309).

Root cause: `updateBaseSHAs` unconditionally recorded the parent's *current*
tip as each child's base, even when the child had never been rebased onto it.
`gh stack push` calls it, so the recipe "amend, push, rebase" corrupted the
metadata before the rebase ever ran:

    after init:  b2.base = <b1 original tip>   correct
    after push:  b2.base = <b1 amended tip>    b2 does not contain this

`branches[].base` is the `git rebase --onto <newBase> <upstream>` boundary for
the next cascade, so recording a commit the branch does not contain makes git
fall back to a merge base and replay the parent's superseded commits.

`updateBaseSHAs` now only advances a base when the parent's tip really is in
the branch's history, so the record keeps describing where the branch actually
sits. `Head` is still always updated — it is the branch's own tip.

`cascadeRebase` gains `resolveOntoOldBase`, which picks the latest boundary the
branch genuinely contains: the parent's current tip, else the recorded metadata
base, else a merge base. It replaces the ad-hoc staleness guard that existed
only on the merged-PR path, and now covers the plain path too, which had none.

Also fixes a regression from the previous commit: a trunk that was never pushed
(a stack based on a local integration branch) was treated like a trunk that had
been deleted upstream and failed outright. The two are now distinguished by
whether the trunk has an upstream configured — a tracked trunk that has
disappeared is still a hard error, an untracked one is used as-is.

Verified against real git: amend at the bottom, in the middle, and at two depths
at once; extra commits; a dropped commit force-pushed; squash-merge with three
branches above it; and each combined with a trunk that moved, diverged, or was
locked by another worktree.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f5959297-80fd-4732-aeae-aa9a4b6a7755

Author: skarim

cmd/onto_base_test.go

@@ -0,0 +1,211 @@
+package cmd
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/github/gh-stack/internal/git"
+	"github.com/github/gh-stack/internal/stack"
+	"github.com/stretchr/testify/assert"
+)
+
+// ancestryMock builds an IsAncestor func from an explicit linear history per
+// branch: history[branch] lists the SHAs that branch contains, oldest first.
+func ancestryMock(history map[string][]string) func(string, string) (bool, error) {
+	index := func(list []string, sha string) int {
+		for i, s := range list {
+			if s == sha {
+				return i
+			}
+		}
+		return -1
+	}
+	return func(ancestor, descendant string) (bool, error) {
+		// descendant may be a branch name or a SHA within some branch.
+		for branch, list := range history {
+			if branch != descendant && index(list, descendant) < 0 {
+				continue
+			}
+			end := len(list)
+			if i := index(list, descendant); i >= 0 {
+				end = i + 1
+			}
+			if index(list[:end], ancestor) >= 0 {
+				return true, nil
+			}
+		}
+		return false, nil
+	}
+}
+
+func TestResolveOntoOldBase(t *testing.T) {
+	// b2 contains: trunk -> a-old -> b2-own
+	// The parent (b1) has since been amended to a-new, which b2 does NOT have.
+	history := map[string][]string{
+		"b2": {"trunk", "a-old", "b2-own"},
+		"b1": {"trunk", "a-new"},
+	}
+
+	t.Run("recorded upstream is used when the branch contains it", func(t *testing.T) {
+		restore := git.SetOps(&git.MockOps{IsAncestorFn: ancestryMock(history)})
+		defer restore()
+
+		got := resolveOntoOldBase("a-old", "a-old", "b1", "b2")
+		assert.Equal(t, "a-old", got)
+	})
+
+	// The #250 case: the parent was amended, so its current tip is not in the
+	// branch's history. Falling back to a merge base would replay the parent's
+	// superseded commit; the recorded metadata base is the correct boundary.
+	t.Run("falls back to the metadata base when the parent was amended", func(t *testing.T) {
+		restore := git.SetOps(&git.MockOps{
+			IsAncestorFn: ancestryMock(history),
+			MergeBaseFn:  func(a, b string) (string, error) { return "trunk", nil },
+		})
+		defer restore()
+
+		got := resolveOntoOldBase("a-new", "a-old", "b1", "b2")
+		assert.Equal(t, "a-old", got,
+			"should replay only b2's own commits, not the parent's superseded one")
+	})
+
+	t.Run("prefers the latest usable boundary", func(t *testing.T) {
+		restore := git.SetOps(&git.MockOps{
+			IsAncestorFn: ancestryMock(history),
+			MergeBaseFn:  func(a, b string) (string, error) { return "trunk", nil },
+		})
+		defer restore()
+
+		// Both "trunk" and "a-old" are ancestors of b2; a-old replays fewer commits.
+		got := resolveOntoOldBase("a-new", "a-old", "b1", "b2")
+		assert.Equal(t, "a-old", got)
+	})
+
+	t.Run("falls back to a merge base when no metadata base is recorded", func(t *testing.T) {
+		restore := git.SetOps(&git.MockOps{
+			IsAncestorFn: ancestryMock(history),
+			MergeBaseFn:  func(a, b string) (string, error) { return "trunk", nil },
+		})
+		defer restore()
+
+		got := resolveOntoOldBase("a-new", "", "b1", "b2")
+		assert.Equal(t, "trunk", got)
+	})
+
+	t.Run("returns the recorded value when nothing is usable", func(t *testing.T) {
+		restore := git.SetOps(&git.MockOps{
+			IsAncestorFn: func(string, string) (bool, error) { return false, nil },
+			MergeBaseFn:  func(string, string) (string, error) { return "", errors.New("no merge base") },
+		})
+		defer restore()
+
+		got := resolveOntoOldBase("a-new", "unrelated", "b1", "b2")
+		assert.Equal(t, "a-new", got)
+	})
+
+	t.Run("ignores an unusable metadata base", func(t *testing.T) {
+		restore := git.SetOps(&git.MockOps{
+			IsAncestorFn: ancestryMock(history),
+			MergeBaseFn:  func(a, b string) (string, error) { return "trunk", nil },
+		})
+		defer restore()
+
+		// A metadata base from an unrelated history must not be trusted.
+		got := resolveOntoOldBase("a-new", "bogus", "b1", "b2")
+		assert.Equal(t, "trunk", got)
+	})
+}
+
+func TestUpdateBaseSHAs(t *testing.T) {
+	newStack := func(b2Base string) *stack.Stack {
+		return &stack.Stack{
+			Trunk: stack.BranchRef{Branch: "main"},
+			Branches: []stack.BranchRef{
+				{Branch: "b1"},
+				{Branch: "b2", Base: b2Base},
+			},
+		}
+	}
+
+	t.Run("advances the base when the branch really is stacked on the parent", func(t *testing.T) {
+		restore := git.SetOps(&git.MockOps{
+			RevParseFn:   func(ref string) (string, error) { return "sha-" + ref, nil },
+			IsAncestorFn: func(string, string) (bool, error) { return true, nil },
+		})
+		defer restore()
+
+		s := newStack("stale-base")
+		updateBaseSHAs(s)
+		assert.Equal(t, "sha-b1", s.Branches[1].Base)
+		assert.Equal(t, "sha-b2", s.Branches[1].Head)
+	})
+
+	// The root cause of #250: `gh stack push` recorded the parent's amended tip
+	// as the child's base even though the child had not been rebased onto it,
+	// which made the next cascade replay the parent's superseded commits.
+	t.Run("keeps the recorded base when the parent moved out of band", func(t *testing.T) {
+		restore := git.SetOps(&git.MockOps{
+			RevParseFn: func(ref string) (string, error) { return "sha-" + ref, nil },
+			IsAncestorFn: func(ancestor, descendant string) (bool, error) {
+				// b2 does not contain b1's amended tip.
+				if ancestor == "sha-b1" && descendant == "b2" {
+					return false, nil
+				}
+				return true, nil
+			},
+		})
+		defer restore()
+
+		s := newStack("b1-original-tip")
+		updateBaseSHAs(s)
+		assert.Equal(t, "b1-original-tip", s.Branches[1].Base,
+			"the base must keep describing where b2 actually sits")
+		assert.Equal(t, "sha-b2", s.Branches[1].Head, "head is always the branch's own tip")
+	})
+
+	t.Run("records a base when none was known yet", func(t *testing.T) {
+		restore := git.SetOps(&git.MockOps{
+			RevParseFn: func(ref string) (string, error) { return "sha-" + ref, nil },
+			IsAncestorFn: func(string, string) (bool, error) {
+				return false, nil
+			},
+		})
+		defer restore()
+
+		s := newStack("")
+		updateBaseSHAs(s)
+		assert.Equal(t, "sha-b1", s.Branches[1].Base,
+			"with nothing recorded there is no truth to preserve")
+	})
+
+	t.Run("keeps the recorded base when ancestry cannot be determined", func(t *testing.T) {
+		restore := git.SetOps(&git.MockOps{
+			RevParseFn:   func(ref string) (string, error) { return "sha-" + ref, nil },
+			IsAncestorFn: func(string, string) (bool, error) { return false, errors.New("boom") },
+		})
+		defer restore()
+
+		s := newStack("known-base")
+		updateBaseSHAs(s)
+		assert.Equal(t, "known-base", s.Branches[1].Base)
+	})
+
+	t.Run("skips merged branches", func(t *testing.T) {
+		restore := git.SetOps(&git.MockOps{
+			RevParseFn:   func(ref string) (string, error) { return "sha-" + ref, nil },
+			IsAncestorFn: func(string, string) (bool, error) { return true, nil },
+		})
+		defer restore()
+
+		s := &stack.Stack{
+			Trunk: stack.BranchRef{Branch: "main"},
+			Branches: []stack.BranchRef{
+				{Branch: "b1", Base: "frozen", PullRequest: &stack.PullRequestRef{Number: 1, Merged: true}},
+				{Branch: "b2"},
+			},
+		}
+		updateBaseSHAs(s)
+		assert.Equal(t, "frozen", s.Branches[0].Base, "a merged branch's base is left alone")
+		assert.Equal(t, "sha-main", s.Branches[1].Base, "b2 is measured against trunk")
+	})
+}

cmd/rebase_test.go

@@ -44,9 +44,9 @@ func newRebaseMock(tmpDir string, currentBranch string) *git.MockOps {
 			}
 			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 },
+		IsAncestorFn:         func(a, d string) (bool, error) { return true, nil },
+		FetchFn:              func(string) error { return nil },
+		EnableRerereFn:       func() error { return nil },
 		IsRebaseInProgressFn: func() bool { return false },
 	}
 }

cmd/stale_trunk_test.go

@@ -303,9 +303,10 @@ func TestRebase_RemoteQualifiedTrunk_IsNormalized(t *testing.T) {
 	assert.NotContains(t, string(out), "origin/origin/main")
 }
 
-// Issue #225 shape: the trunk branch was merged and deleted on the remote, so
-// there is nothing to bring the stack up to date with. That must be reported
-// rather than silently rebasing onto a stale local trunk.
+// Issue #225 shape: the trunk branch was tracked on the remote and has since
+// been merged and deleted, so there is nothing to bring the stack up to date
+// with. That must be reported rather than silently rebasing onto a stale local
+// trunk.
 func TestRebase_TrunkMissingOnRemote_Fails(t *testing.T) {
 	tmpDir := t.TempDir()
 	writeStackFile(t, tmpDir, twoBranchStack())
@@ -314,6 +315,7 @@ func TestRebase_TrunkMissingOnRemote_Fails(t *testing.T) {
 
 	mock := newRebaseMock(tmpDir, "b2")
 	mock.BranchExistsFn = func(string) bool { return true }
+	mock.UpstreamRemoteFn = func(string) (string, error) { return "origin", nil }
 	mock.RevParseFn = func(ref string) (string, error) {
 		if ref == "origin/main" {
 			return "", errors.New("unknown revision")
@@ -338,10 +340,57 @@ func TestRebase_TrunkMissingOnRemote_Fails(t *testing.T) {
 
 	assert.ErrorIs(t, err, ErrSilent)
 	assert.Zero(t, rebaseCalls, "nothing should be rebased onto a stale trunk")
-	assert.Contains(t, output, "does not exist on origin")
+	assert.Contains(t, output, "no longer exists on origin")
 	assert.Contains(t, output, "--no-trunk")
 }
 
+// A trunk that was never pushed is a local integration branch, not an orphaned
+// stack: the cascade must still run against it.
+func TestRebase_LocalOnlyTrunk_StillRebases(t *testing.T) {
+	s := twoBranchStack()
+	s.Trunk.Branch = "integration"
+
+	tmpDir := t.TempDir()
+	writeStackFile(t, tmpDir, s)
+
+	var rebaseBases []string
+
+	mock := newRebaseMock(tmpDir, "b2")
+	mock.BranchExistsFn = func(string) bool { return true }
+	mock.UpstreamRemoteFn = func(string) (string, error) { return "", nil }
+	mock.RevParseFn = func(ref string) (string, error) {
+		if strings.HasPrefix(ref, "origin/") {
+			return "", errors.New("unknown revision")
+		}
+		return "sha-" + ref, nil
+	}
+	mock.CheckoutBranchFn = func(string) error { return nil }
+	mock.RebaseFn = func(base string, _ git.RebaseOpts) error {
+		rebaseBases = append(rebaseBases, base)
+		return nil
+	}
+	mock.RebaseOntoFn = func(newBase, _, _ string, _ git.RebaseOpts) error {
+		rebaseBases = append(rebaseBases, newBase)
+		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()
+	out, _ := io.ReadAll(errR)
+
+	assert.NoError(t, err)
+	assert.Equal(t, []string{"integration", "b1"}, rebaseBases)
+	assert.Contains(t, string(out), "only exists locally")
+}
+
 // --no-trunk still works without a reachable remote trunk.
 func TestRebase_NoTrunk_SkipsTrunkResolution(t *testing.T) {
 	tmpDir := t.TempDir()

cmd/trunk_target_test.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	"errors"
 	"io"
+	"strings"
 	"testing"
 
 	"github.com/github/gh-stack/internal/config"
@@ -223,9 +224,11 @@ func TestResolveTrunkTarget(t *testing.T) {
 		assert.False(t, target.Detached)
 	})
 
-	// Issue #225 shape: the trunk branch was merged and deleted on the remote.
-	t.Run("fails when the trunk no longer exists on the remote", func(t *testing.T) {
+	// Issue #225 shape: the trunk branch was tracked on the remote and has since
+	// been merged and deleted, orphaning the stack.
+	t.Run("fails when a tracked trunk no longer exists on the remote", func(t *testing.T) {
 		mock := trunkTargetMock("local", "")
+		mock.UpstreamRemoteFn = func(string) (string, error) { return "origin", nil }
 		restore := git.SetOps(mock)
 		defer restore()
 
@@ -237,10 +240,55 @@ func TestResolveTrunkTarget(t *testing.T) {
 
 		cfg.Err.Close()
 		out, _ := io.ReadAll(errR)
-		assert.Contains(t, string(out), "does not exist on origin")
+		assert.Contains(t, string(out), "no longer exists on origin")
 		assert.Contains(t, string(out), "--no-trunk")
 	})
 
+	// A stack based on a local integration branch that was never pushed: the
+	// local branch is the source of truth, so the cascade must still run.
+	t.Run("uses a local-only trunk that was never pushed", func(t *testing.T) {
+		restore := git.SetOps(&git.MockOps{
+			BranchExistsFn:   func(string) bool { return true },
+			UpstreamRemoteFn: func(string) (string, error) { return "", nil },
+			RevParseFn: func(ref string) (string, error) {
+				if strings.HasPrefix(ref, "origin/") {
+					return "", errors.New("unknown revision")
+				}
+				return "sha-" + ref, nil
+			},
+		})
+		defer restore()
+
+		cfg, _, errR := config.NewTestConfig()
+		s := &stack.Stack{Trunk: stack.BranchRef{Branch: "integration"}}
+
+		target, err := resolveTrunkTarget(cfg, s, "origin", "b1")
+		require.NoError(t, err)
+		assert.Equal(t, "integration", target.Ref)
+		assert.False(t, target.Detached)
+
+		cfg.Err.Close()
+		out, _ := io.ReadAll(errR)
+		assert.Contains(t, string(out), "only exists locally")
+	})
+
+	t.Run("fails when the trunk exists neither locally nor remotely", func(t *testing.T) {
+		mock := trunkTargetMock("local", "")
+		mock.BranchExistsFn = func(string) bool { return false }
+		restore := git.SetOps(mock)
+		defer restore()
+
+		cfg, _, errR := config.NewTestConfig()
+		s := &stack.Stack{Trunk: stack.BranchRef{Branch: "main"}}
+
+		_, err := resolveTrunkTarget(cfg, s, "origin", "b1")
+		assert.ErrorIs(t, err, ErrSilent)
+
+		cfg.Err.Close()
+		out, _ := io.ReadAll(errR)
+		assert.Contains(t, string(out), "neither locally nor on origin")
+	})
+
 	// Issue #176: `gh stack init --base origin/main` records a remote-qualified
 	// trunk, which used to be re-qualified into "origin/origin/main".
 	t.Run("normalizes a remote-qualified trunk", func(t *testing.T) {