Skip to content

Commit c1a8cbe

Browse files
committed
Make cherry-pick conflicts recoverable via --continue
Previously, cherry-pick conflicts during fold-down operations could only be resolved with --abort. Now they save full conflict state (phase, conflict type, fold branch/target, remaining branches) to the state file, enabling recovery via 'gh stack modify --continue'. Changes: - Add ConflictType field to StateFile (rebase or cherry_pick) - Add FoldBranch/FoldTarget fields for cherry-pick context - Add CherryPickContinue to git package (cherry-pick --continue) - Save cherry-pick conflict state in ApplyPlan with remaining branches - ContinueApply handles both rebase and cherry-pick conflicts - Unified conflict messaging in cmd/modify.go (both types show --continue) - Updated test to verify cherry-pick conflict state is saved correctly
1 parent 7f33691 commit c1a8cbe

7 files changed

Lines changed: 159 additions & 36 deletions

File tree

cmd/modify.go

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -123,26 +123,15 @@ func runModify(cfg *config.Config) error {
123123
isCherryPick := applyErr != nil && strings.Contains(applyErr.Error(), "cherry-pick")
124124
if isCherryPick {
125125
cfg.Warningf("Cherry-pick conflict folding %s", conflict.Branch)
126-
if len(conflict.ConflictedFiles) > 0 {
127-
cfg.Printf("")
128-
cfg.Printf("%s", cfg.ColorBold("Conflicted files:"))
129-
for _, f := range conflict.ConflictedFiles {
130-
cfg.Printf(" %s %s", cfg.ColorWarning("C"), f)
131-
}
132-
}
133-
cfg.Printf("")
134-
cfg.Printf("Cherry-pick conflicts cannot be continued with `--continue`.")
135-
cfg.Printf("Restore the stack to its pre-modify state with `%s`",
136-
cfg.ColorCyan("gh stack modify --abort"))
137126
} else {
138127
cfg.Warningf("Rebasing %s — conflict", conflict.Branch)
128+
}
139129

140-
printConflictDetailsWithContinue(cfg, conflict.Branch, "gh stack modify --continue")
141-
cfg.Printf("")
130+
printConflictDetailsWithContinue(cfg, conflict.Branch, "gh stack modify --continue")
131+
cfg.Printf("")
142132

143-
cfg.Printf("Or restore the stack to its pre-modify state with `%s`",
144-
cfg.ColorCyan("gh stack modify --abort"))
145-
}
133+
cfg.Printf("Or restore the stack to its pre-modify state with `%s`",
134+
cfg.ColorCyan("gh stack modify --abort"))
146135
return ErrConflict
147136
}
148137

internal/git/git.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,11 @@ func CherryPickAbort() {
368368
_ = ops.CherryPickAbort()
369369
}
370370

371+
// CherryPickContinue continues an in-progress cherry-pick after conflicts are resolved.
372+
func CherryPickContinue() error {
373+
return ops.CherryPickContinue()
374+
}
375+
371376
// HasUncommittedChanges returns true if the working tree has uncommitted changes.
372377
func HasUncommittedChanges() (bool, error) {
373378
return ops.HasUncommittedChanges()

internal/git/gitops.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ type Ops interface {
5959
RenameBranch(oldName, newName string) error
6060
CherryPick(commits []string) error
6161
CherryPickAbort() error
62+
CherryPickContinue() error
6263
HasUncommittedChanges() (bool, error)
6364
LogMerges(base, head string) ([]CommitInfo, error)
6465
}
@@ -513,6 +514,12 @@ func (d *defaultOps) CherryPickAbort() error {
513514
return runSilent("cherry-pick", "--quit")
514515
}
515516

517+
func (d *defaultOps) CherryPickContinue() error {
518+
cmd := exec.Command("git", "cherry-pick", "--continue")
519+
cmd.Env = append(os.Environ(), "GIT_EDITOR=true")
520+
return cmd.Run()
521+
}
522+
516523
func (d *defaultOps) HasUncommittedChanges() (bool, error) {
517524
out, err := run("status", "--porcelain")
518525
if err != nil {

internal/git/mock_ops.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,10 @@ func (m *MockOps) CherryPickAbort() error {
359359
return nil
360360
}
361361

362+
func (m *MockOps) CherryPickContinue() error {
363+
return nil
364+
}
365+
362366
func (m *MockOps) HasUncommittedChanges() (bool, error) {
363367
if m.HasUncommittedChangesFn != nil {
364368
return m.HasUncommittedChangesFn()

internal/modify/apply.go

Lines changed: 56 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,35 @@ func ApplyPlan(
281281
if files, ferr := git.ConflictedFiles(); ferr == nil {
282282
conflict.ConflictedFiles = files
283283
}
284+
285+
// Compute remaining branches for cascading rebase after cherry-pick resumes.
286+
// Since folds happen before cascading rebase (Step 5), all non-merged, non-folded
287+
// branches need rebasing.
288+
remaining := make([]string, 0)
289+
for _, br := range s.Branches {
290+
if !br.IsMerged() && br.Branch != foldBranch {
291+
remaining = append(remaining, br.Branch)
292+
}
293+
}
294+
295+
// Save conflict state so --continue can resume the cherry-pick
296+
stateFile.Phase = PhaseConflict
297+
stateFile.ConflictBranch = foldBranch
298+
stateFile.ConflictType = "cherry_pick"
299+
stateFile.FoldBranch = foldBranch
300+
stateFile.FoldTarget = targetBranch
301+
stateFile.RemainingBranches = remaining
302+
stateFile.OriginalBranch = currentBranch
303+
stateFile.OriginalRefs = originalParentTips
304+
if saveErr := SaveState(gitDir, stateFile); saveErr != nil {
305+
cfg.Warningf("failed to save conflict state: %v", saveErr)
306+
}
307+
308+
// Save stack metadata so far
309+
if saveErr := stack.SaveWithLock(gitDir, sf, lock); saveErr != nil {
310+
cfg.Warningf("failed to save stack metadata: %v", saveErr)
311+
}
312+
284313
return nil, conflict, fmt.Errorf("cherry-pick conflict folding %s into %s", foldBranch, targetBranch)
285314
}
286315

@@ -446,6 +475,7 @@ func ApplyPlan(
446475
}
447476
stateFile.Phase = PhaseConflict
448477
stateFile.ConflictBranch = b.Branch
478+
stateFile.ConflictType = "rebase"
449479
stateFile.RemainingBranches = remaining
450480
stateFile.OriginalBranch = currentBranch
451481
stateFile.OriginalRefs = originalParentTips
@@ -454,7 +484,9 @@ func ApplyPlan(
454484
}
455485

456486
// Save stack metadata so far (renames, folds, drops already applied)
457-
_ = stack.SaveWithLock(gitDir, sf, lock)
487+
if saveErr := stack.SaveWithLock(gitDir, sf, lock); saveErr != nil {
488+
cfg.Warningf("failed to save stack metadata: %v", saveErr)
489+
}
458490

459491
return nil, conflict, fmt.Errorf("rebase conflict on %s", b.Branch)
460492
}
@@ -519,26 +551,36 @@ func ContinueApply(
519551
}
520552
defer lock.Unlock()
521553

522-
// Find the stack
554+
// Find the stack using the saved index for reliable identification.
523555
var s *stack.Stack
524-
for i := range sf.Stacks {
525-
if sf.Stacks[i].Trunk.Branch == state.StackName {
526-
s = &sf.Stacks[i]
527-
break
528-
}
556+
if state.StackIndex >= 0 && state.StackIndex < len(sf.Stacks) {
557+
s = &sf.Stacks[state.StackIndex]
529558
}
530559
if s == nil {
531-
return fmt.Errorf("stack %q not found", state.StackName)
560+
return fmt.Errorf("stack at index %d not found (stack file may have changed)", state.StackIndex)
532561
}
533562

534-
// Finish the in-progress git rebase
535-
if git.IsRebaseInProgress() {
536-
if err := git.RebaseContinue(); err != nil {
537-
return fmt.Errorf("rebase continue failed — resolve remaining conflicts and try again: %w", err)
563+
// Finish the in-progress git operation (rebase or cherry-pick)
564+
if state.ConflictType == "cherry_pick" {
565+
if err := git.CherryPickContinue(); err != nil {
566+
return fmt.Errorf("cherry-pick continue failed — resolve remaining conflicts and try again: %w", err)
538567
}
539-
}
568+
cfg.Successf("Folded %s into %s", state.FoldBranch, state.FoldTarget)
540569

541-
cfg.Successf("Rebased %s", state.ConflictBranch)
570+
// Remove the folded branch from stack metadata
571+
foldIdx := s.IndexOf(state.FoldBranch)
572+
if foldIdx >= 0 && foldIdx < len(s.Branches) {
573+
s.Branches = append(s.Branches[:foldIdx], s.Branches[foldIdx+1:]...)
574+
}
575+
} else {
576+
// Rebase conflict
577+
if git.IsRebaseInProgress() {
578+
if err := git.RebaseContinue(); err != nil {
579+
return fmt.Errorf("rebase continue failed — resolve remaining conflicts and try again: %w", err)
580+
}
581+
}
582+
cfg.Successf("Rebased %s", state.ConflictBranch)
583+
}
542584

543585
// Continue cascading rebase for remaining branches
544586
for _, branchName := range state.RemainingBranches {

internal/modify/apply_test.go

Lines changed: 77 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -751,15 +751,86 @@ func TestApplyPlan_ConflictDuringCherryPick(t *testing.T) {
751751
require.NotNil(t, conflict)
752752
assert.Equal(t, "B", conflict.Branch)
753753

754-
// Cherry-pick conflicts return a conflict immediately (no "conflict" state for continue)
754+
// Cherry-pick conflicts now save state for --continue recovery
755755
state, loadErr := LoadState(gitDir)
756756
require.NoError(t, loadErr)
757-
// The state file is created with "applying" phase before the conflict,
758-
// but cherry-pick conflicts don't update it to "conflict"
759-
if state != nil {
760-
assert.NotEqual(t, "conflict", state.Phase,
761-
"cherry-pick conflicts should not set phase to 'conflict'")
757+
require.NotNil(t, state)
758+
assert.Equal(t, PhaseConflict, state.Phase)
759+
assert.Equal(t, "cherry_pick", state.ConflictType)
760+
assert.Equal(t, "B", state.FoldBranch)
761+
assert.Equal(t, "A", state.FoldTarget)
762+
assert.Equal(t, "A", state.OriginalBranch)
763+
assert.Contains(t, state.RemainingBranches, "A")
764+
}
765+
766+
// ─── ContinueApply: Multi-Stack Finds Correct Stack ─────────────────────────
767+
768+
func TestContinueApply_MultiStackFindsCorrectStack(t *testing.T) {
769+
// When multiple stacks share the same trunk, ContinueApply should use
770+
// StackIndex to find the right stack, not just trunk name matching.
771+
gitDir := t.TempDir()
772+
773+
// Stack 0: main <- X (a different stack)
774+
// Stack 1: main <- A <- B <- C (the one being modified)
775+
sf := &stack.StackFile{
776+
SchemaVersion: 1,
777+
Stacks: []stack.Stack{
778+
{
779+
Trunk: stack.BranchRef{Branch: "main"},
780+
Branches: []stack.BranchRef{{Branch: "X"}},
781+
},
782+
{
783+
Trunk: stack.BranchRef{Branch: "main"},
784+
Branches: []stack.BranchRef{
785+
{Branch: "A"},
786+
{Branch: "B"},
787+
{Branch: "C"},
788+
},
789+
},
790+
},
791+
}
792+
data, err := json.MarshalIndent(sf, "", " ")
793+
require.NoError(t, err)
794+
require.NoError(t, os.WriteFile(filepath.Join(gitDir, "gh-stack"), data, 0644))
795+
796+
// Create a state file pointing at Stack 1 (index 1)
797+
state := &StateFile{
798+
SchemaVersion: 1,
799+
StackName: "main",
800+
StackIndex: 1, // The correct stack is at index 1
801+
Phase: PhaseConflict,
802+
ConflictBranch: "A",
803+
ConflictType: "rebase",
804+
RemainingBranches: []string{"B", "C"},
805+
OriginalRefs: map[string]string{"B": "sha-A", "C": "sha-B"},
806+
}
807+
require.NoError(t, SaveState(gitDir, state))
808+
809+
mock := newApplyMock(gitDir, map[string]string{
810+
"main": "sha-main", "A": "sha-A", "B": "sha-B", "C": "sha-C",
811+
})
812+
mock.IsRebaseInProgressFn = func() bool { return true }
813+
mock.RebaseContinueFn = func() error { return nil }
814+
815+
var rebasedBranches []string
816+
mock.RebaseOntoFn = func(newBase, oldBase, branch string) error {
817+
rebasedBranches = append(rebasedBranches, branch)
818+
return nil
762819
}
820+
821+
restore := git.SetOps(mock)
822+
defer restore()
823+
824+
cfg, _, _ := config.NewTestConfig()
825+
defer cfg.Out.Close()
826+
defer cfg.Err.Close()
827+
828+
err = ContinueApply(cfg, gitDir, noopUpdateBaseSHAs)
829+
require.NoError(t, err)
830+
831+
// B and C should have been found and processed (not "no longer in stack")
832+
assert.Contains(t, rebasedBranches, "B", "B should be rebased")
833+
assert.Contains(t, rebasedBranches, "C", "C should be rebased")
763834
}
764835

765836
// ─── Unwind ──────────────────────────────────────────────────────────────────

internal/modify/state.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,14 @@ type StateFile struct {
3131

3232
// Conflict state — populated when phase is "conflict"
3333
ConflictBranch string `json:"conflict_branch,omitempty"`
34+
ConflictType string `json:"conflict_type,omitempty"` // "rebase" or "cherry_pick"
3435
RemainingBranches []string `json:"remaining_branches,omitempty"`
3536
OriginalBranch string `json:"original_branch,omitempty"`
3637
OriginalRefs map[string]string `json:"original_refs,omitempty"`
38+
39+
// Cherry-pick conflict context — which fold was in progress
40+
FoldBranch string `json:"fold_branch,omitempty"` // branch being folded
41+
FoldTarget string `json:"fold_target,omitempty"` // branch receiving the cherry-pick
3742
}
3843

3944
// Snapshot captures the pre-modify state for unwind/recovery.

0 commit comments

Comments
 (0)