Skip to content

Commit b3ab7e0

Browse files
skarimCopilot
andcommitted
Heal stacks whose recorded base was already corrupted
The previous commit stops `gh stack push` from recording a base the branch does not contain, but every stack that has already been through that path still carries a bad value on disk. Those stacks would keep hitting the conflict on their next rebase, because neither the parent's current tip nor the recorded base is a boundary the branch actually has. `resolveOntoOldBase` now also considers `git merge-base --fork-point`, which reads the parent's reflog and so still finds where the branch diverged after the parent was amended, rebased, or force-pushed — exactly the record the stack file lost. It is only a candidate: the ancestry check still gates it, and a fresh clone or an expired reflog simply falls through to the merge bases as before. Verified on a real stack whose metadata had been corrupted by the previous build: the rebase now completes, replaying one commit instead of two, and the recorded bases are genuine ancestors again afterwards. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f5959297-80fd-4732-aeae-aa9a4b6a7755
1 parent 1775047 commit b3ab7e0

5 files changed

Lines changed: 60 additions & 1 deletion

File tree

cmd/onto_base_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,38 @@ func TestResolveOntoOldBase(t *testing.T) {
9292
assert.Equal(t, "trunk", got)
9393
})
9494

95+
// Stacks written by an older version recorded the parent's amended tip as
96+
// the child's base, so the metadata is no more usable than the recorded
97+
// tip. git's fork-point reads the parent's reflog and still finds the real
98+
// boundary, which lets those stacks heal on their next rebase.
99+
t.Run("uses the fork point when the metadata base is also stale", func(t *testing.T) {
100+
restore := git.SetOps(&git.MockOps{
101+
IsAncestorFn: ancestryMock(history),
102+
MergeBaseFn: func(a, b string) (string, error) { return "trunk", nil },
103+
MergeBaseForkPointFn: func(ref, branch string) (string, error) { return "a-old", nil },
104+
})
105+
defer restore()
106+
107+
// Both the recorded tip and the metadata base are the amended commit.
108+
got := resolveOntoOldBase("a-new", "a-new", "b1", "b2")
109+
assert.Equal(t, "a-old", got,
110+
"the fork point is the only remaining record of where b2 diverged")
111+
})
112+
113+
t.Run("ignores an unavailable fork point", func(t *testing.T) {
114+
restore := git.SetOps(&git.MockOps{
115+
IsAncestorFn: ancestryMock(history),
116+
MergeBaseFn: func(a, b string) (string, error) { return "trunk", nil },
117+
MergeBaseForkPointFn: func(ref, branch string) (string, error) {
118+
return "", errors.New("no fork point found")
119+
},
120+
})
121+
defer restore()
122+
123+
got := resolveOntoOldBase("a-new", "", "b1", "b2")
124+
assert.Equal(t, "trunk", got)
125+
})
126+
95127
t.Run("returns the recorded value when nothing is usable", func(t *testing.T) {
96128
restore := git.SetOps(&git.MockOps{
97129
IsAncestorFn: func(string, string) (bool, error) { return false, nil },

cmd/utils.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1240,7 +1240,10 @@ type cascadeRebaseResult struct {
12401240
// 2. metadataBase — the parent tip this branch was last stacked on, from the
12411241
// stack file. Correct when the parent was amended, reordered, or
12421242
// squash-merged since.
1243-
// 3. merge-base(recordedOldBase, branch) / merge-base(newBase, branch).
1243+
// 3. merge-base --fork-point, which reads the parent's reflog and so still
1244+
// finds the right boundary when the stack file itself is stale — for
1245+
// example a stack whose metadata was written by an older version.
1246+
// 4. merge-base(recordedOldBase, branch) / merge-base(newBase, branch).
12441247
//
12451248
// Returns recordedOldBase unchanged when nothing better can be determined.
12461249
func resolveOntoOldBase(recordedOldBase, metadataBase, newBase, branch string) string {
@@ -1257,6 +1260,9 @@ func resolveOntoOldBase(recordedOldBase, metadataBase, newBase, branch string) s
12571260
}
12581261

12591262
candidates := []string{metadataBase}
1263+
if fp, err := git.MergeBaseForkPoint(newBase, branch); err == nil && fp != "" {
1264+
candidates = append(candidates, fp)
1265+
}
12601266
if mb, err := git.MergeBase(recordedOldBase, branch); err == nil {
12611267
candidates = append(candidates, mb)
12621268
}

internal/git/git.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,14 @@ func MergeBase(a, b string) (string, error) {
354354
return ops.MergeBase(a, b)
355355
}
356356

357+
// MergeBaseForkPoint returns the commit at which branch forked off ref, using
358+
// ref's reflog to see through rewrites of ref itself (an amend, a rebase, a
359+
// force-push). Returns an error when no fork point can be determined, which is
360+
// normal in a fresh clone or once the reflog has expired.
361+
func MergeBaseForkPoint(ref, branch string) (string, error) {
362+
return ops.MergeBaseForkPoint(ref, branch)
363+
}
364+
357365
// Log returns recent commits for the given branch.
358366
func Log(ref string, maxCount int) ([]CommitInfo, error) {
359367
return ops.Log(ref, maxCount)

internal/git/gitops.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ type Ops interface {
5959
RevParse(ref string) (string, error)
6060
RevParseMulti(refs []string) ([]string, error)
6161
MergeBase(a, b string) (string, error)
62+
MergeBaseForkPoint(ref, branch string) (string, error)
6263
Log(ref string, maxCount int) ([]CommitInfo, error)
6364
LogRange(base, head string) ([]CommitInfo, error)
6465
DiffStatRange(base, head string) (additions, deletions int, err error)
@@ -442,6 +443,10 @@ func (d *defaultOps) MergeBase(a, b string) (string, error) {
442443
return run("merge-base", a, b)
443444
}
444445

446+
func (d *defaultOps) MergeBaseForkPoint(ref, branch string) (string, error) {
447+
return run("merge-base", "--fork-point", ref, branch)
448+
}
449+
445450
func (d *defaultOps) Log(ref string, maxCount int) ([]CommitInfo, error) {
446451
format := "%H\t%s\t%at"
447452
output, err := run("log", ref, "--format="+format, "-n", strconv.Itoa(maxCount))

internal/git/mock_ops.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ type MockOps struct {
3535
RevParseFn func(string) (string, error)
3636
RevParseMultiFn func([]string) ([]string, error)
3737
MergeBaseFn func(string, string) (string, error)
38+
MergeBaseForkPointFn func(string, string) (string, error)
3839
LogFn func(string, int) ([]CommitInfo, error)
3940
LogRangeFn func(string, string) ([]CommitInfo, error)
4041
DiffStatRangeFn func(string, string) (int, int, error)
@@ -280,6 +281,13 @@ func (m *MockOps) MergeBase(a, b string) (string, error) {
280281
return "", nil
281282
}
282283

284+
func (m *MockOps) MergeBaseForkPoint(ref, branch string) (string, error) {
285+
if m.MergeBaseForkPointFn != nil {
286+
return m.MergeBaseForkPointFn(ref, branch)
287+
}
288+
return "", nil
289+
}
290+
283291
func (m *MockOps) Log(ref string, maxCount int) ([]CommitInfo, error) {
284292
if m.LogFn != nil {
285293
return m.LogFn(ref, maxCount)

0 commit comments

Comments
 (0)