Skip to content

Commit 9421bd6

Browse files
authored
Fix for replaying amended parent commits during rebase (#333)
* Avoid replaying amended parent commits Preserve a branch's last valid base when its parent is rewritten, and only use verified ancestor commits as rebase boundaries. Recover previously corrupted metadata from the parent reflog when possible, otherwise stop safely instead of replaying superseded parent commits. * Record adopted branch merge bases Store the actual common ancestor when adding an existing branch so cascade rebases replay only that branch's unique commits while retaining the amended-parent safety guard. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 30bcbf2a-5ef1-4bdb-a0fc-618294ae8ded --------- Copilot-Session: 30bcbf2a-5ef1-4bdb-a0fc-618294ae8ded
1 parent a5cae7b commit 9421bd6

9 files changed

Lines changed: 479 additions & 34 deletions

File tree

cmd/add.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,14 @@ func runAdd(cfg *config.Config, opts *addOptions, args []string) error {
169169
// If the branch already exists in git but is not part of any stack,
170170
// adopt it instead of erroring. This mirrors the init command's behavior.
171171
adopted := git.BranchExists(branchName)
172+
var adoptedBase string
173+
if adopted {
174+
adoptedBase, err = git.MergeBase(currentBranch, branchName)
175+
if err != nil {
176+
cfg.Errorf("failed to determine the common base of %s and %s: %s", currentBranch, branchName, err)
177+
return ErrSilent
178+
}
179+
}
172180

173181
// Stage changes before creating the branch so we can fail early if
174182
// there's nothing to commit (avoids leaving an empty orphan branch).
@@ -191,9 +199,12 @@ func runAdd(cfg *config.Config, opts *addOptions, args []string) error {
191199
return ErrSilent
192200
}
193201

194-
base, err := git.RevParse(currentBranch)
195-
if err != nil {
196-
cfg.Warningf("could not resolve base SHA for %s: %s", currentBranch, err)
202+
base := adoptedBase
203+
if !adopted {
204+
base, err = git.RevParse(currentBranch)
205+
if err != nil {
206+
cfg.Warningf("could not resolve base SHA for %s: %s", currentBranch, err)
207+
}
197208
}
198209
s.Branches = append(s.Branches, stack.BranchRef{Branch: branchName, Base: base})
199210

cmd/add_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,11 @@ func TestAdd_AdoptsExistingBranch(t *testing.T) {
446446
GitDirFn: func() (string, error) { return gitDir, nil },
447447
CurrentBranchFn: func() (string, error) { return "b1", nil },
448448
BranchExistsFn: func(name string) bool { return name == "existing-branch" },
449+
MergeBaseFn: func(parent, branch string) (string, error) {
450+
assert.Equal(t, "b1", parent)
451+
assert.Equal(t, "existing-branch", branch)
452+
return "common-base", nil
453+
},
449454
CreateBranchFn: func(name, base string) error {
450455
createBranchCalled = true
451456
return nil
@@ -472,6 +477,8 @@ func TestAdd_AdoptsExistingBranch(t *testing.T) {
472477
require.NoError(t, err)
473478
names := sf.Stacks[0].BranchNames()
474479
assert.Equal(t, "existing-branch", names[len(names)-1], "adopted branch appended to stack")
480+
assert.Equal(t, "common-base", sf.Stacks[0].Branches[len(sf.Stacks[0].Branches)-1].Base,
481+
"adopted branch should record the actual common ancestor")
475482
}
476483

477484
func TestAdd_RejectsExistingBranchInStack(t *testing.T) {
@@ -520,6 +527,7 @@ func TestAdd_AdoptsExistingBranchWithCommit(t *testing.T) {
520527
GitDirFn: func() (string, error) { return gitDir, nil },
521528
CurrentBranchFn: func() (string, error) { return "b1", nil },
522529
BranchExistsFn: func(name string) bool { return name == "existing-branch" },
530+
MergeBaseFn: func(string, string) (string, error) { return "common-base", nil },
523531
RevParseMultiFn: func(refs []string) ([]string, error) {
524532
return []string{"aaa", "bbb"}, nil // different SHAs = branch has commits
525533
},
@@ -548,3 +556,36 @@ func TestAdd_AdoptsExistingBranchWithCommit(t *testing.T) {
548556
assert.True(t, commitCalled, "Commit should be called on the adopted branch")
549557
assert.Contains(t, output, "Adopted")
550558
}
559+
560+
func TestAdd_AdoptExistingBranchWithoutCommonBaseFails(t *testing.T) {
561+
gitDir := t.TempDir()
562+
saveStack(t, gitDir, stack.Stack{
563+
Trunk: stack.BranchRef{Branch: "main"},
564+
Branches: []stack.BranchRef{{Branch: "b1"}},
565+
})
566+
567+
checkedOut := false
568+
restore := git.SetOps(&git.MockOps{
569+
GitDirFn: func() (string, error) { return gitDir, nil },
570+
CurrentBranchFn: func() (string, error) { return "b1", nil },
571+
BranchExistsFn: func(name string) bool { return name == "unrelated" },
572+
MergeBaseFn: func(string, string) (string, error) { return "", assert.AnError },
573+
CheckoutBranchFn: func(string) error {
574+
checkedOut = true
575+
return nil
576+
},
577+
})
578+
defer restore()
579+
580+
cfg, outR, errR := config.NewTestConfig()
581+
err := runAdd(cfg, &addOptions{}, []string{"unrelated"})
582+
output := collectOutput(cfg, outR, errR)
583+
584+
assert.ErrorIs(t, err, ErrSilent)
585+
assert.False(t, checkedOut)
586+
assert.Contains(t, output, "failed to determine the common base")
587+
588+
sf, loadErr := stack.Load(gitDir)
589+
require.NoError(t, loadErr)
590+
assert.Equal(t, []string{"b1"}, sf.Stacks[0].BranchNames())
591+
}

0 commit comments

Comments
 (0)