Skip to content

Commit 9902191

Browse files
committed
ensure upstack rebase checks immediate predecessor
1 parent 40ae3e0 commit 9902191

File tree

2 files changed

+74
-0
lines changed

2 files changed

+74
-0
lines changed

cmd/rebase.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,19 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
203203
needsOnto := false
204204
var ontoOldBase string
205205

206+
// Get --onto state from merged branches below the rebase range.
207+
// Ensures that when --upstack excludes merged branches, we still check
208+
// the immediate predecessor for a merged PR and use --onto if needed.
209+
if startIdx > 0 {
210+
prev := s.Branches[startIdx-1]
211+
if prev.IsMerged() {
212+
if sha, ok := originalRefs[prev.Branch]; ok {
213+
needsOnto = true
214+
ontoOldBase = sha
215+
}
216+
}
217+
}
218+
206219
for i, br := range branchesToRebase {
207220
var base string
208221
absIdx := startIdx + i

cmd/rebase_test.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,67 @@ func TestRebase_UpstackOnly(t *testing.T) {
495495
assert.Equal(t, "b2", allRebaseCalls[1].newBase, "b3 should be rebased onto b2")
496496
}
497497

498+
// TestRebase_UpstackWithMergedBranchBelow verifies that --upstack pre-seeds
499+
// --onto state when a merged branch exists immediately below the rebase range.
500+
func TestRebase_UpstackWithMergedBranchBelow(t *testing.T) {
501+
s := stack.Stack{
502+
Trunk: stack.BranchRef{Branch: "main"},
503+
Branches: []stack.BranchRef{
504+
{Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 10, Merged: true}},
505+
{Branch: "b2"},
506+
{Branch: "b3"},
507+
},
508+
}
509+
510+
tmpDir := t.TempDir()
511+
writeStackFile(t, tmpDir, s)
512+
513+
var allRebaseCalls []rebaseCall
514+
var currentCheckedOut string
515+
516+
mock := newRebaseMock(tmpDir, "b2")
517+
mock.CheckoutBranchFn = func(name string) error {
518+
currentCheckedOut = name
519+
return nil
520+
}
521+
mock.BranchExistsFn = func(name string) bool { return true }
522+
mock.RebaseFn = func(base string) error {
523+
allRebaseCalls = append(allRebaseCalls, rebaseCall{newBase: base, oldBase: "", branch: currentCheckedOut})
524+
return nil
525+
}
526+
mock.RebaseOntoFn = func(newBase, oldBase, branch string) error {
527+
allRebaseCalls = append(allRebaseCalls, rebaseCall{newBase, oldBase, branch})
528+
return nil
529+
}
530+
531+
restore := git.SetOps(mock)
532+
defer restore()
533+
534+
cfg, _, _ := config.NewTestConfig()
535+
cmd := RebaseCmd(cfg)
536+
cmd.SetArgs([]string{"--upstack"})
537+
cmd.SetOut(io.Discard)
538+
cmd.SetErr(io.Discard)
539+
err := cmd.Execute()
540+
541+
cfg.Out.Close()
542+
cfg.Err.Close()
543+
544+
assert.NoError(t, err)
545+
// b2 is at index 1, upstack = [b2, b3]. b1 is merged below.
546+
// b2 should use --onto because b1 was merged.
547+
require.Len(t, allRebaseCalls, 2, "upstack should rebase b2 and b3")
548+
549+
// b2: --onto rebase with b1's old SHA as old base
550+
assert.Equal(t, "main", allRebaseCalls[0].newBase, "b2 should be rebased onto main (first non-merged ancestor)")
551+
assert.Equal(t, "sha-b1", allRebaseCalls[0].oldBase, "b2 should use b1's original SHA as old base")
552+
assert.Equal(t, "b2", allRebaseCalls[0].branch, "b2 should be the branch being rebased")
553+
554+
// b3: --onto continues to propagate
555+
assert.Equal(t, "b2", allRebaseCalls[1].newBase, "b3 should be rebased onto b2")
556+
assert.NotEmpty(t, allRebaseCalls[1].oldBase, "b3 should also use --onto")
557+
}
558+
498559
// TestRebase_SkipsMergedBranches verifies that merged branches are skipped
499560
// with an appropriate message.
500561
func TestRebase_SkipsMergedBranches(t *testing.T) {

0 commit comments

Comments
 (0)