Fix modify --abort leaving a broken stack after a conflict

When `gh stack modify` hit a rebase or cherry-pick conflict it saved state
with phase "conflict" and told the user to run `gh stack modify --abort` to
restore. But runModifyAbort had no case for PhaseConflict, so it fell into the
default branch that merely printed "unexpected modify state phase" and deleted
the state file without unwinding. The in-flight rebase/cherry-pick stayed
active, branches were left partially rewritten, and the deleted state file also
made --continue impossible: the stack was stuck in limbo.

Fixes:

- runModifyAbort now unwinds on PhaseConflict (same recovery as PhaseApplying),
  aborting the in-progress operation, resetting branch tips to their pre-modify
  SHAs, restoring stack metadata, and clearing state.
- Unwind now also aborts an in-progress cherry-pick (fold-down conflicts), not
  just a rebase. Without this the restore checkouts would fail on the unmerged
  cherry-pick index.
- ContinueApply now records a subsequent cascade-rebase conflict as
  ConflictType "rebase" instead of leaving a stale "cherry_pick", so the next
  --continue calls RebaseContinue rather than failing in CherryPickContinue.

Adds coverage for the conflict-phase abort, pending-submit no-op abort, Unwind
aborting an active cherry-pick, and the cherry-pick to rebase ConflictType
transition.

Author: skarim

cmd/modify.go

@@ -219,8 +219,12 @@ func runModifyAbort(cfg *config.Config) error {
 	}
 
 	switch state.Phase {
-	case modify.PhaseApplying:
-		cfg.Printf("A modify session was interrupted during the apply phase")
+	case modify.PhaseApplying, modify.PhaseConflict:
+		if state.Phase == modify.PhaseConflict {
+			cfg.Printf("Aborting the modify and discarding in-progress conflict resolution")
+		} else {
+			cfg.Printf("A modify session was interrupted during the apply phase")
+		}
 		cfg.Printf("Restoring stack to pre-modify state...")
 		if err := modify.UnwindFromStateFile(cfg, gitDir); err != nil {
 			cfg.Errorf("recovery failed: %s", err)

cmd/modify_test.go

@@ -771,3 +771,132 @@ func TestCheckModifyStateGuard_UnknownPhase(t *testing.T) {
 	err := modify.CheckStateGuard(gitDir)
 	assert.NoError(t, err, "guard only blocks on 'applying' phase")
 }
+
+// ---------------------------------------------------------------------------
+// 6. runModifyAbort recovery
+// ---------------------------------------------------------------------------
+
+// Regression test: aborting a modify that stopped at a conflict must actually
+// unwind the stack (abort the in-flight rebase, reset branch tips to their
+// pre-modify SHAs, restore metadata, clear state) — not fall into a default
+// branch that merely deletes the state file and strands the user.
+func TestRunModifyAbort_ConflictPhase_Unwinds(t *testing.T) {
+	s := stack.Stack{
+		Trunk: stack.BranchRef{Branch: "main"},
+		Branches: []stack.BranchRef{
+			{Branch: "A"},
+			{Branch: "B"},
+		},
+	}
+
+	tmpDir := t.TempDir()
+	writeStackFile(t, tmpDir, s)
+
+	meta, err := json.Marshal(s)
+	require.NoError(t, err)
+
+	snapshot := modify.Snapshot{
+		Branches: []modify.BranchSnapshot{
+			{Name: "A", TipSHA: "sha-A-original", Position: 0},
+			{Name: "B", TipSHA: "sha-B-original", Position: 1},
+		},
+		StackMetadata: meta,
+	}
+
+	// The state ApplyPlan persists when a cascade rebase conflicts.
+	state := &modify.StateFile{
+		SchemaVersion:  1,
+		StackName:      "main",
+		StackIndex:     0,
+		Phase:          modify.PhaseConflict,
+		ConflictBranch: "B",
+		ConflictType:   "rebase",
+		Snapshot:       snapshot,
+	}
+	require.NoError(t, modify.SaveState(tmpDir, state))
+
+	var rebaseAborted bool
+	var resetCalls []struct{ branch, sha string }
+	current := ""
+	mock := &git.MockOps{
+		GitDirFn:                 func() (string, error) { return tmpDir, nil },
+		IsRebaseInProgressFn:     func() bool { return true },
+		IsCherryPickInProgressFn: func() bool { return false },
+		RebaseAbortFn:            func() error { rebaseAborted = true; return nil },
+		BranchExistsFn:           func(string) bool { return true },
+		CheckoutBranchFn:         func(name string) error { current = name; return nil },
+		ResetHardFn: func(sha string) error {
+			resetCalls = append(resetCalls, struct{ branch, sha string }{current, sha})
+			return nil
+		},
+		CreateBranchFn: func(string, string) error { return nil },
+	}
+	restore := git.SetOps(mock)
+	defer restore()
+
+	cfg, _, errR := config.NewTestConfig()
+
+	err = runModifyAbort(cfg)
+
+	cfg.Out.Close()
+	cfg.Err.Close()
+	out, _ := io.ReadAll(errR)
+	output := string(out)
+
+	require.NoError(t, err)
+
+	// The in-flight rebase must have been aborted.
+	assert.True(t, rebaseAborted, "in-progress rebase should be aborted during recovery")
+
+	// The state file must be cleared because recovery ran (not left dangling,
+	// and not deleted-without-unwind).
+	assert.False(t, modify.StateExists(tmpDir), "state file should be cleared after a successful abort")
+
+	// Branch tips must be reset to their pre-modify snapshot SHAs.
+	resetMap := map[string]string{}
+	for _, r := range resetCalls {
+		resetMap[r.branch] = r.sha
+	}
+	assert.Equal(t, "sha-A-original", resetMap["A"])
+	assert.Equal(t, "sha-B-original", resetMap["B"])
+
+	// It must not fall into the old default branch.
+	assert.NotContains(t, output, "unexpected modify state phase")
+	assert.Contains(t, output, "Restoring stack to pre-modify state")
+}
+
+// PendingSubmit abort is a no-op that guides the user to submit; it must not
+// try to unwind (the local changes already succeeded).
+func TestRunModifyAbort_PendingSubmit_NoUnwind(t *testing.T) {
+	tmpDir := t.TempDir()
+
+	state := &modify.StateFile{
+		SchemaVersion: 1,
+		StackName:     "main",
+		StackIndex:    0,
+		Phase:         modify.PhasePendingSubmit,
+	}
+	require.NoError(t, modify.SaveState(tmpDir, state))
+
+	var resetCalled bool
+	mock := &git.MockOps{
+		GitDirFn:    func() (string, error) { return tmpDir, nil },
+		ResetHardFn: func(string) error { resetCalled = true; return nil },
+	}
+	restore := git.SetOps(mock)
+	defer restore()
+
+	cfg, _, errR := config.NewTestConfig()
+
+	err := runModifyAbort(cfg)
+
+	cfg.Out.Close()
+	cfg.Err.Close()
+	out, _ := io.ReadAll(errR)
+	output := string(out)
+
+	require.NoError(t, err)
+	assert.False(t, resetCalled, "pending-submit abort must not unwind branches")
+	assert.True(t, modify.StateExists(tmpDir), "pending-submit state should be preserved")
+	assert.Contains(t, output, "gh stack submit")
+}

internal/modify/apply.go

@@ -397,7 +397,7 @@ func ApplyPlan(
 					shas[len(commits)-1-i] = c.SHA
 				}
 
-				git.CherryPickAbort()
+				git.CherryPickQuit()
 
 				if err := git.CherryPick(shas); err != nil {
 					conflict := &modifyview.ConflictInfo{Branch: foldBranch}
@@ -906,6 +906,12 @@ func ContinueApply(
 				}
 			}
 			state.ConflictBranch = branchName
+			// These remaining branches are always rebased via RebaseOnto, so
+			// the in-progress operation is a rebase. Update ConflictType in
+			// case the original conflict was a cherry-pick (fold-down) — a
+			// stale "cherry_pick" here would make the next --continue call
+			// CherryPickContinue and fail.
+			state.ConflictType = "rebase"
 			state.RemainingBranches = remaining
 			state.AffectsPRs = affectsPRs
 			_ = SaveState(gitDir, state)
@@ -977,10 +983,16 @@ func ContinueApply(
 // Unwind restores the stack to its pre-modify state using the snapshot.
 // stackIndex is the index of the stack in sf.Stacks at modify start time.
 func Unwind(cfg *config.Config, gitDir string, snapshot Snapshot, stackIndex int, sf *stack.StackFile, plan []Action) error {
-	// Abort any in-progress rebase
+	// Abort any in-progress rebase or cherry-pick so the working tree and
+	// index are clean before we restore branch tips. A fold-down conflict
+	// leaves an in-progress cherry-pick with an unmerged index; without
+	// aborting it first, the restore checkouts below would fail.
 	if git.IsRebaseInProgress() {
 		_ = git.RebaseAbort()
 	}
+	if git.IsCherryPickInProgress() {
+		_ = git.CherryPickAbort()
+	}
 
 	// Restore branch tips
 	snapshotNames := make(map[string]bool, len(snapshot.Branches))