Don't repeat a continue or skip that a lock error stopped partway

If a lock stops "git rebase --skip" after git has applied the changes
of the next commit but before it has committed them, running it again
discards those staged changes, and the commit is lost. Something
similar happens with "git cherry-pick --continue" and "--skip" (and
the same for revert). If the lock stops them at the next commit, git
doesn't reschedule that commit, and running them again skips it.

For a rebase, handle continue and skip like the commands that start a
rebase. Run the command again if the lock stopped it before it changed
the state of the rebase, continue if git rescheduled the todo that
failed, and give up otherwise. For a cherry-pick or revert, run the
command again if it didn't change the todos in .git/sequencer, and
give up otherwise.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

pkg/commands/git_commands/lock_error_recovery.go

@@ -1,6 +1,7 @@
 package git_commands
 
 import (
+	"bytes"
 	"os"
 	"path/filepath"
 	"strings"
@@ -88,6 +89,30 @@ func abortSequenceAfterLockError(
 	})
 }
 
+// retryContinueOrSkipAfterLockError sets the lock-error recovery (see
+// CmdObj.RecoverFromLockErrorWith) of cmdObj, a command that continues or skips
+// a step of an operation in progress, such as a cherry-pick or revert.
+// progressFile is the file in which git records how far the operation has got.
+// Call this function right before running the command, since it records the
+// contents of progressFile at that time.
+//
+// If a lock stops the command before it changes progressFile, we run the
+// command again. Otherwise it has got past the step that it continued or
+// skipped, and the lock stopped it at the next one. Unlike a rebase, git doesn't
+// reschedule that step (see abortSequenceAfterLockError), and running the
+// command again could skip it. So we give up and leave the operation to the
+// user.
+func retryContinueOrSkipAfterLockError(cmdObj *oscommands.CmdObj, progressFile string) *oscommands.CmdObj {
+	progressBefore, _ := os.ReadFile(progressFile)
+	return cmdObj.RecoverFromLockErrorWith(func() *oscommands.CmdObj {
+		progress, _ := os.ReadFile(progressFile)
+		if bytes.Equal(progress, progressBefore) {
+			return cmdObj
+		}
+		return nil
+	})
+}
+
 // rebaseState returns a snapshot of the rebase that is in progress in gitDir,
 // made of the files in which git records how far it has got. It's empty if no
 // rebase is in progress.

pkg/commands/git_commands/lock_error_recovery_test.go

@@ -175,3 +175,55 @@ func TestAbortSequenceAfterLockError(t *testing.T) {
 		})
 	}
 }
+
+func TestRetryContinueOrSkipAfterLockError(t *testing.T) {
+	newCmdObj := func(args ...string) *oscommands.CmdObj {
+		return oscommands.NewDummyCmdObjBuilder(nil).New(args)
+	}
+
+	writeTodos := func(gitDir string, todos string) {
+		dir := filepath.Join(gitDir, "sequencer")
+		assert.NoError(t, os.MkdirAll(dir, 0o755))
+		assert.NoError(t, os.WriteFile(filepath.Join(dir, "todo"), []byte(todos), 0o644))
+	}
+
+	t.Run("runs the command again if it didn't change the todos", func(t *testing.T) {
+		gitDir := t.TempDir()
+		writeTodos(gitDir, "pick 1234567 one\npick 2345678 two\n")
+		cmdObj := newCmdObj("git", "cherry-pick", "--continue")
+
+		retryContinueOrSkipAfterLockError(cmdObj, filepath.Join(gitDir, "sequencer", "todo"))
+
+		assert.Same(t, cmdObj, cmdObj.GetLockErrorRecovery()())
+	})
+
+	t.Run("runs the command again for a cherry-pick of a single commit", func(t *testing.T) {
+		cmdObj := newCmdObj("git", "cherry-pick", "--continue")
+
+		retryContinueOrSkipAfterLockError(cmdObj, filepath.Join(t.TempDir(), "sequencer", "todo"))
+
+		assert.Same(t, cmdObj, cmdObj.GetLockErrorRecovery()())
+	})
+
+	t.Run("gives up if it changed the todos", func(t *testing.T) {
+		gitDir := t.TempDir()
+		writeTodos(gitDir, "pick 1234567 one\npick 2345678 two\n")
+		cmdObj := newCmdObj("git", "cherry-pick", "--continue")
+
+		retryContinueOrSkipAfterLockError(cmdObj, filepath.Join(gitDir, "sequencer", "todo"))
+		writeTodos(gitDir, "pick 2345678 two\n")
+
+		assert.Nil(t, cmdObj.GetLockErrorRecovery()())
+	})
+
+	t.Run("gives up if the cherry-pick is gone", func(t *testing.T) {
+		gitDir := t.TempDir()
+		writeTodos(gitDir, "pick 1234567 one\n")
+		cmdObj := newCmdObj("git", "cherry-pick", "--continue")
+
+		retryContinueOrSkipAfterLockError(cmdObj, filepath.Join(gitDir, "sequencer", "todo"))
+		assert.NoError(t, os.RemoveAll(filepath.Join(gitDir, "sequencer")))
+
+		assert.Nil(t, cmdObj.GetLockErrorRecovery()())
+	})
+}

pkg/commands/git_commands/rebase.go

@@ -501,7 +501,18 @@ func (self *RebaseCommands) GenericMergeOrRebaseAction(commandType string, comma
 		}
 	}
 
-	err := skipEditor(self.GenericMergeOrRebaseActionCmdObj(commandType, command)).Run()
+	cmdObj := skipEditor(self.GenericMergeOrRebaseActionCmdObj(commandType, command))
+	if command == "continue" || command == "skip" {
+		switch commandType {
+		case "rebase":
+			cmdObj = self.resumeAfterLockError(cmdObj)
+		case "cherry-pick", "revert":
+			cmdObj = retryContinueOrSkipAfterLockError(cmdObj,
+				filepath.Join(self.repoPaths.WorktreeGitDirPath(), "sequencer", "todo"))
+		}
+	}
+
+	err := cmdObj.Run()
 	if err != nil {
 		undoRepeatingTodo()
 		if !strings.Contains(err.Error(), "no rebase in progress") {
@@ -524,8 +535,8 @@ func (self *RebaseCommands) GenericMergeOrRebaseAction(commandType string, comma
 	return nil
 }
 
-// resumeAfterLockError makes cmdObj, a command that starts a rebase, resume
-// the rebase if a lock error stops it partway through; see
+// resumeAfterLockError makes cmdObj, a command that starts or continues a
+// rebase, resume the rebase if a lock error stops it partway through; see
 // resumeRebaseAfterLockError.
 func (self *RebaseCommands) resumeAfterLockError(cmdObj *oscommands.CmdObj) *oscommands.CmdObj {
 	return resumeRebaseAfterLockError(cmdObj, self.repoPaths.WorktreeGitDirPath(), self.config.GetCoreCommentChar(),

pkg/integration/tests/interactive_rebase/lock_error_partway_through_skip.go

@@ -55,7 +55,6 @@ var LockErrorPartwayThroughSkip = NewIntegrationTest(NewIntegrationTestArgs{
 			Select(Contains("skip")).
 			Confirm()
 
-		/* EXPECTED:
 		t.ExpectPopup().Alert().
 			Title(Equals("Error")).
 			Content(Contains("index.lock': File exists.")).
@@ -65,12 +64,5 @@ var LockErrorPartwayThroughSkip = NewIntegrationTest(NewIntegrationTestArgs{
 			Lines(
 				Contains("A").Contains("file-y"),
 			)
-		ACTUAL: */
-		t.Views().Commits().
-			Lines(
-				Contains("add file-z"),
-				Contains("edited commit"),
-				Contains("base"),
-			)
 	},
 })