From 4486e8d149827ecafd3a60a25d16fe32d69b422e Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 7 Nov 2026 09:30:39 +0100 Subject: [PATCH 01/15] Extract the check for a rescheduled rebase command into a helper We are going to need it for recovering from lock errors during a rebase. Co-Authored-By: Claude Opus 5.5 --- pkg/commands/git_commands/commit_loader.go | 33 +-------------------- pkg/utils/rebase_todo.go | 34 ++++++++++++++++++++++ 2 files changed, 35 insertions(+), 32 deletions(-) diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index 987f2394e..b4a938880 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -430,38 +430,7 @@ func (self *CommitLoader) getConflictedCommitImpl(hashPool *utils.StringPool, to return nil } - // In certain cases, git reschedules commands that failed. One example is if - // a patch would overwrite an untracked file (another one is an "exec" that - // failed, but we don't care about that here because we dealt with exec - // already above). To detect this, compare the last command of the "done" - // file against the first command of "git-rebase-todo"; if they are the - // same, the command was rescheduled. - if len(doneTodos) > 0 && len(todos) > 0 && doneTodos[len(doneTodos)-1] == todos[0] { - // Command was rescheduled, no need to display it - return nil - } - - // Older versions of git have a bug whereby, if a command is rescheduled, - // the last successful command is appended to the "done" file again. To - // detect this, we need to compare the second-to-last done entry against the - // first todo entry, and also compare the last done entry against the - // last-but-two done entry; this latter check is needed for the following - // case: - // pick A - // exec make test - // pick B - // exec make test - // If pick B fails with conflicts, then the "done" file contains - // pick A - // exec make test - // pick B - // and git-rebase-todo contains - // exec make test - // Without the last condition we would erroneously treat this as the exec - // command being rescheduled, so we wouldn't display our fake entry for - // "pick B". - if len(doneTodos) >= 3 && len(todos) > 0 && doneTodos[len(doneTodos)-2] == todos[0] && - doneTodos[len(doneTodos)-1] == doneTodos[len(doneTodos)-3] { + if utils.LastDoneTodoWasRescheduled(doneTodos, todos) { // Command was rescheduled, no need to display it return nil } diff --git a/pkg/utils/rebase_todo.go b/pkg/utils/rebase_todo.go index 577b3da07..f6fe5cb32 100644 --- a/pkg/utils/rebase_todo.go +++ b/pkg/utils/rebase_todo.go @@ -68,6 +68,40 @@ func findTodo(todos []todo.Todo, todoToFind Todo) (int, bool) { return idx, ok } +// LastDoneTodoWasRescheduled tells whether git put the last command of +// rebase-merge/done back into git-rebase-todo, given the parsed contents of +// these two files. In certain cases, git reschedules commands that failed. One +// example is if a patch would overwrite an untracked file; another one is an +// "exec" that failed. +func LastDoneTodoWasRescheduled(doneTodos []todo.Todo, todos []todo.Todo) bool { + // If the last command of the "done" file is the same as the first command + // of "git-rebase-todo", the command was rescheduled. + if len(doneTodos) > 0 && len(todos) > 0 && doneTodos[len(doneTodos)-1] == todos[0] { + return true + } + + // Older versions of git have a bug whereby, if a command is rescheduled, + // the last successful command is appended to the "done" file again. To + // detect this, we need to compare the second-to-last done entry against the + // first todo entry, and also compare the last done entry against the + // last-but-two done entry; this latter check is needed for the following + // case: + // pick A + // exec make test + // pick B + // exec make test + // If pick B fails with conflicts, then the "done" file contains + // pick A + // exec make test + // pick B + // and git-rebase-todo contains + // exec make test + // Without the last condition we would erroneously treat this as the exec + // command being rescheduled. + return len(doneTodos) >= 3 && len(todos) > 0 && doneTodos[len(doneTodos)-2] == todos[0] && + doneTodos[len(doneTodos)-1] == doneTodos[len(doneTodos)-3] +} + func ReadRebaseTodoFile(fileName string, commentChar byte) ([]todo.Todo, error) { f, err := os.Open(fileName) if err != nil { From 3a538fc2a8e2e5cba5dc2e88dcaa3ba1d08e04cc Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 7 Nov 2026 09:33:12 +0100 Subject: [PATCH 02/15] Let a git command decide how to recover from a lock error The git command runner runs a command again if it failed because another git process held a lock. This is wrong for a command that starts an operation of several steps, such as a rebase or a cherry-pick of several commits, if the lock stopped it partway through. Running it again then fails because the operation is already in progress. Add a hook that such a command can use to tell the runner what to do instead. The following commits use it. Co-Authored-By: Claude Opus 5.5 --- pkg/commands/git_cmd_obj_runner.go | 24 ++++-- pkg/commands/git_cmd_obj_runner_test.go | 97 ++++++++++++++++++++++--- pkg/commands/oscommands/cmd_obj.go | 24 ++++++ 3 files changed, 129 insertions(+), 16 deletions(-) diff --git a/pkg/commands/git_cmd_obj_runner.go b/pkg/commands/git_cmd_obj_runner.go index 8112b0f30..4f6bd37ec 100644 --- a/pkg/commands/git_cmd_obj_runner.go +++ b/pkg/commands/git_cmd_obj_runner.go @@ -55,14 +55,14 @@ func (self *gitCmdObjRunner) Run(cmdObj *oscommands.CmdObj) error { } func (self *gitCmdObjRunner) RunWithOutput(cmdObj *oscommands.CmdObj) (string, error) { - return self.retryOnLockError(func() (string, error) { + return self.retryOnLockError(cmdObj, func(cmdObj *oscommands.CmdObj) (string, error) { return self.innerRunner.RunWithOutput(cmdObj.Clone()) }) } func (self *gitCmdObjRunner) RunWithOutputs(cmdObj *oscommands.CmdObj) (string, string, error) { var stdout, stderr string - _, err := self.retryOnLockError(func() (string, error) { + _, err := self.retryOnLockError(cmdObj, func(cmdObj *oscommands.CmdObj) (string, error) { var runErr error stdout, stderr, runErr = self.innerRunner.RunWithOutputs(cmdObj.Clone()) return stdout + stderr, runErr @@ -70,23 +70,33 @@ func (self *gitCmdObjRunner) RunWithOutputs(cmdObj *oscommands.CmdObj) (string, return stdout, stderr, err } -// retryOnLockError runs the given function, retrying if it fails with a -// transient lock error (see isRetryableError). The string returned by run is +// retryOnLockError runs the given command with the given function, trying +// again if it fails with a transient lock error (see isRetryableError). If the +// command has a lock-error recovery (see CmdObj.RecoverFromLockErrorWith), the +// recovery decides which command to run next. The string returned by run is // the command output we inspect to classify the failure. We clone the command // for each attempt (inside run) because an *exec.Cmd can only be run once. -func (self *gitCmdObjRunner) retryOnLockError(run func() (string, error)) (string, error) { +func (self *gitCmdObjRunner) retryOnLockError(cmdObj *oscommands.CmdObj, run func(*oscommands.CmdObj) (string, error)) (string, error) { delay := self.initialRetryDelay var output string var err error for attempt := range maxRetries { - output, err = run() + output, err = run(cmdObj) if err == nil || !isRetryableError(output, err) { break } + if recovery := cmdObj.GetLockErrorRecovery(); recovery != nil { + cmdObj = recovery() + if cmdObj == nil { + self.log.Warn("lock error stopped command partway through; giving up") + break + } + } + if attempt < maxRetries-1 { - self.log.Warnf("lock error prevented command from running; retrying in %s", delay) + self.log.Warnf("lock error prevented command from completing; running %q in %s", cmdObj.ToString(), delay) time.Sleep(delay) delay *= 2 } diff --git a/pkg/commands/git_cmd_obj_runner_test.go b/pkg/commands/git_cmd_obj_runner_test.go index bf938da54..cc6496bde 100644 --- a/pkg/commands/git_cmd_obj_runner_test.go +++ b/pkg/commands/git_cmd_obj_runner_test.go @@ -16,29 +16,32 @@ type runnerResult struct { // scriptedRunner is an ICmdObjRunner stub that returns a preconfigured result // for each successive call, letting us drive the retry loop deterministically. -// It counts calls so tests can assert whether a command was retried. +// It counts calls and records the commands it ran, so tests can assert whether +// and how a command was retried. type scriptedRunner struct { results []runnerResult calls int + ran []string } -func (self *scriptedRunner) next() (string, error) { +func (self *scriptedRunner) next(cmdObj *oscommands.CmdObj) (string, error) { result := self.results[self.calls] self.calls++ + self.ran = append(self.ran, cmdObj.ToString()) return result.output, result.err } -func (self *scriptedRunner) Run(*oscommands.CmdObj) error { - _, err := self.next() +func (self *scriptedRunner) Run(cmdObj *oscommands.CmdObj) error { + _, err := self.next(cmdObj) return err } -func (self *scriptedRunner) RunWithOutput(*oscommands.CmdObj) (string, error) { - return self.next() +func (self *scriptedRunner) RunWithOutput(cmdObj *oscommands.CmdObj) (string, error) { + return self.next(cmdObj) } -func (self *scriptedRunner) RunWithOutputs(*oscommands.CmdObj) (string, string, error) { - output, err := self.next() +func (self *scriptedRunner) RunWithOutputs(cmdObj *oscommands.CmdObj) (string, string, error) { + output, err := self.next(cmdObj) return output, "", err } @@ -58,9 +61,15 @@ func newTestRunner(inner *scriptedRunner) *gitCmdObjRunner { // dummyCmdObj returns a throwaway command; only its clonability matters, since // the scriptedRunner ignores it and returns preconfigured results. func dummyCmdObj() *oscommands.CmdObj { - return oscommands.NewDummyCmdObjBuilder(nil).New([]string{"git", "status"}) + return newCmdObj("git", "status") } +func newCmdObj(args ...string) *oscommands.CmdObj { + return oscommands.NewDummyCmdObjBuilder(nil).New(args) +} + +var errLock = errors.New("fatal: Unable to create '/repo/.git/index.lock': File exists.") + func TestRunWithOutputReturnsSuccessWithoutRetrying(t *testing.T) { inner := &scriptedRunner{results: []runnerResult{{output: "done", err: nil}}} @@ -135,3 +144,73 @@ func TestRunWithOutputRetriesLockErrorInLinkedWorktree(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 2, inner.calls) } + +func TestRunWithOutputRunsTheCommandReturnedByTheLockErrorRecovery(t *testing.T) { + inner := &scriptedRunner{results: []runnerResult{ + {err: errLock}, + {err: errLock}, + {output: "done", err: nil}, + }} + continueCmdObj := newCmdObj("git", "rebase", "--continue") + cmdObj := newCmdObj("git", "rebase", "--interactive", "main") + cmdObj.RecoverFromLockErrorWith(func() *oscommands.CmdObj { return continueCmdObj }) + + output, err := newTestRunner(inner).RunWithOutput(cmdObj) + + assert.NoError(t, err) + assert.Equal(t, "done", output) + // After switching to the continue command, the runner uses that command's + // recovery; it has none, so the runner runs it again. + assert.Equal(t, []string{ + "git rebase --interactive main", + "git rebase --continue", + "git rebase --continue", + }, inner.ran) +} + +func TestRunWithOutputGivesUpWhenTheLockErrorRecoveryReturnsNil(t *testing.T) { + inner := &scriptedRunner{results: []runnerResult{{err: errLock}}} + cmdObj := dummyCmdObj() + cmdObj.RecoverFromLockErrorWith(func() *oscommands.CmdObj { return nil }) + + _, err := newTestRunner(inner).RunWithOutput(cmdObj) + + assert.Equal(t, errLock, err) + assert.Equal(t, 1, inner.calls) +} + +func TestRunWithOutputCallsTheLockErrorRecoveryAfterTheLastAttempt(t *testing.T) { + results := make([]runnerResult, maxRetries) + for i := range results { + results[i] = runnerResult{err: errLock} + } + inner := &scriptedRunner{results: results} + recoveryCalls := 0 + cmdObj := dummyCmdObj() + cmdObj.RecoverFromLockErrorWith(func() *oscommands.CmdObj { + recoveryCalls++ + return cmdObj + }) + + _, err := newTestRunner(inner).RunWithOutput(cmdObj) + + assert.Error(t, err) + assert.Equal(t, maxRetries, inner.calls) + assert.Equal(t, maxRetries, recoveryCalls) +} + +func TestRunWithOutputDoesNotCallTheLockErrorRecoveryForOtherErrors(t *testing.T) { + inner := &scriptedRunner{results: []runnerResult{{output: "boom", err: errors.New("boom")}}} + recoveryCalls := 0 + cmdObj := dummyCmdObj() + cmdObj.RecoverFromLockErrorWith(func() *oscommands.CmdObj { + recoveryCalls++ + return cmdObj + }) + + _, err := newTestRunner(inner).RunWithOutput(cmdObj) + + assert.Error(t, err) + assert.Equal(t, 1, inner.calls) + assert.Equal(t, 0, recoveryCalls) +} diff --git a/pkg/commands/oscommands/cmd_obj.go b/pkg/commands/oscommands/cmd_obj.go index 2f7e66f7f..dd38fd384 100644 --- a/pkg/commands/oscommands/cmd_obj.go +++ b/pkg/commands/oscommands/cmd_obj.go @@ -37,6 +37,9 @@ type CmdObj struct { // can be set so that we don't run certain commands simultaneously mutex *deadlock.Mutex + + // see RecoverFromLockErrorWith() + lockErrorRecovery func() *CmdObj } type CredentialStrategy int @@ -201,6 +204,27 @@ func (self *CmdObj) WithMutex(mutex *deadlock.Mutex) *CmdObj { return self } +// RecoverFromLockErrorWith sets what the git command runner does when this +// command fails because another git process holds a lock (see +// gitCmdObjRunner.retryOnLockError). Without it, the runner runs the command +// again. That is wrong for a command that starts an operation of several +// steps, such as a rebase, if the lock only stopped it partway through. +// +// The runner calls recovery after each failed attempt, including the last +// one, so that recovery can also clean up after the command. It returns the +// command to run next: this one to run it again, a different one (for +// example, one that resumes the operation), or nil to give up and return the +// error. +func (self *CmdObj) RecoverFromLockErrorWith(recovery func() *CmdObj) *CmdObj { + self.lockErrorRecovery = recovery + + return self +} + +func (self *CmdObj) GetLockErrorRecovery() func() *CmdObj { + return self.lockErrorRecovery +} + // runs the command and returns an error if any func (self *CmdObj) Run() error { return self.runner.Run(self) From 3dcad25058edd4a7f8f23dce0f03d23e491309c9 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 7 Nov 2026 09:37:02 +0100 Subject: [PATCH 03/15] Turn runSkipEditorCommand into a function that only sets up the editor The next commit needs a "git rebase --continue" command with this setup that the git command runner runs, rather than one that we run directly. Co-Authored-By: Claude Opus 5.5 --- pkg/commands/git_commands/rebase.go | 11 ++++++----- pkg/commands/git_commands/rebase_test.go | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index c79105c1f..be0485e96 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -408,7 +408,7 @@ func (self *RebaseCommands) SquashAllAboveFixupCommits(commit *models.Commit) er Arg("--interactive", "--rebase-merges", "--autostash", "--autosquash", hashOrRoot). ToArgv() - return self.runSkipEditorCommand(self.cmd.New(cmdArgs)) + return skipEditor(self.cmd.New(cmdArgs)).Run() } // BeginInteractiveRebaseForCommit starts an interactive rebase to edit the current @@ -491,7 +491,7 @@ func (self *RebaseCommands) AbortRebase() error { // GenericMerge takes a commandType of "merge" or "rebase" and a command of "abort", "skip" or "continue" // By default we skip the editor in the case where a commit will be made func (self *RebaseCommands) GenericMergeOrRebaseAction(commandType string, command string) error { - err := self.runSkipEditorCommand(self.GenericMergeOrRebaseActionCmdObj(commandType, command)) + err := skipEditor(self.GenericMergeOrRebaseActionCmdObj(commandType, command)).Run() if err != nil { if !strings.Contains(err.Error(), "no rebase in progress") { return err @@ -513,7 +513,9 @@ func (self *RebaseCommands) GenericMergeOrRebaseAction(commandType string, comma return nil } -func (self *RebaseCommands) runSkipEditorCommand(cmdObj *oscommands.CmdObj) error { +// skipEditor makes git use lazygit as the editor for the given command; lazygit +// then exits right away, so git uses the file as it is. +func skipEditor(cmdObj *oscommands.CmdObj) *oscommands.CmdObj { instruction := daemon.NewExitImmediatelyInstruction() lazyGitPath := oscommands.GetLazygitPath() return cmdObj. @@ -523,8 +525,7 @@ func (self *RebaseCommands) runSkipEditorCommand(cmdObj *oscommands.CmdObj) erro "EDITOR="+lazyGitPath, "VISUAL="+lazyGitPath, ). - AddEnvVars(daemon.ToEnvVars(instruction)...). - Run() + AddEnvVars(daemon.ToEnvVars(instruction)...) } // DiscardOldFileChanges discards changes to a file from an old commit diff --git a/pkg/commands/git_commands/rebase_test.go b/pkg/commands/git_commands/rebase_test.go index 46f1fcc1c..fa47e668e 100644 --- a/pkg/commands/git_commands/rebase_test.go +++ b/pkg/commands/git_commands/rebase_test.go @@ -89,7 +89,7 @@ func TestRebaseSkipEditorCommand(t *testing.T) { return true }, "", nil) instance := buildRebaseCommands(commonDeps{runner: runner}) - err := instance.runSkipEditorCommand(instance.cmd.New(cmdArgs)) + err := skipEditor(instance.cmd.New(cmdArgs)).Run() assert.NoError(t, err) runner.CheckForMissingCalls() } From 21bbcc957a94f84c77a73025540db6ad9c855556 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 7 Nov 2026 09:35:32 +0100 Subject: [PATCH 04/15] Add a test for a lock error partway through a rebase If another git process takes index.lock while a rebase is replaying commits (for example an IDE running "git status" in the background), git stops the rebase and reschedules the command that failed. Lazygit then runs the whole "git rebase --interactive" command again. This fails with "It seems that there is already a rebase-merge directory", and the user is left with a rebase that stopped halfway. Co-Authored-By: Claude Opus 5.5 --- .../lock_error_partway_through_rebase.go | 59 +++++++++++++++++++ pkg/integration/tests/shared/index_lock.go | 32 ++++++++++ pkg/integration/tests/test_list.go | 1 + 3 files changed, 92 insertions(+) create mode 100644 pkg/integration/tests/interactive_rebase/lock_error_partway_through_rebase.go create mode 100644 pkg/integration/tests/shared/index_lock.go diff --git a/pkg/integration/tests/interactive_rebase/lock_error_partway_through_rebase.go b/pkg/integration/tests/interactive_rebase/lock_error_partway_through_rebase.go new file mode 100644 index 000000000..0d0b9cf92 --- /dev/null +++ b/pkg/integration/tests/interactive_rebase/lock_error_partway_through_rebase.go @@ -0,0 +1,59 @@ +package interactive_rebase + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" + "github.com/jesseduffield/lazygit/pkg/integration/tests/shared" +) + +var LockErrorPartwayThroughRebase = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Drops a commit while another git process takes index.lock partway through the rebase", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateNCommits(4) + // The rebase makes a commit when it picks commit-03; the lock then + // stops it when it tries to pick commit-04. + shared.HoldIndexLockAfterNextCommit(shell) + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("commit-04").IsSelected(), + Contains("commit-03"), + Contains("commit-02"), + Contains("commit-01"), + ). + NavigateToLine(Contains("commit-02")). + Press(keys.Universal.Remove). + Tap(func() { + t.ExpectPopup().Confirmation(). + Title(Equals("Drop commit")). + Content(Contains("Are you sure you want to drop the selected commit(s)?")). + Confirm() + }). + /* EXPECTED: + IsFocused(). + Lines( + Contains("commit-04"), + Contains("commit-03"), + Contains("commit-01"), + ) + ACTUAL: */ + Tap(func() { + t.ExpectPopup().Alert(). + Title(Equals("Error")). + Content(Contains("It seems that there is already a rebase-merge directory")). + Confirm() + }). + Lines( + Contains("─── Pending rebase todos"), + Contains("pick").Contains("commit-04"), + Contains("─── Commits"), + Contains("commit-03"), + Contains("commit-01"), + ) + }, +}) diff --git a/pkg/integration/tests/shared/index_lock.go b/pkg/integration/tests/shared/index_lock.go new file mode 100644 index 000000000..29e513f6f --- /dev/null +++ b/pkg/integration/tests/shared/index_lock.go @@ -0,0 +1,32 @@ +package shared + +import ( + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +// holdIndexLockScript is the part of a git hook that takes index.lock the +// first time the hook runs, and holds it until the git command that ran the +// hook has exited. This simulates another git process (for example an IDE +// running "git status" in the background) that takes the lock while a command +// of several steps, such as a rebase, is partway through. +const holdIndexLockScript = `if [ ! -e .git/index-lock-taken ]; then + touch .git/index-lock-taken .git/index.lock + ( + # If lazygit runs the command in a pty (as it does for a pull), we + # get SIGHUP when the command exits + trap '' HUP + while kill -0 $PPID 2>/dev/null; do sleep 0.01; done + rm -f .git/index.lock + ) >/dev/null 2>&1 Date: Sat, 7 Nov 2026 09:40:00 +0100 Subject: [PATCH 05/15] Resume a rebase that a lock error stopped partway through If another git process takes index.lock while a rebase is replaying commits, git stops the rebase and reschedules the todo that failed. The git command runner then ran the whole "git rebase --interactive" command again. This failed because the rebase was already in progress, and the user was left with a rebase that stopped halfway. Continue the rebase with "git rebase --continue" instead; this runs the rescheduled todo again. Only do this if the todo files show that git rescheduled it. In some cases git doesn't, for example if the lock stops a pick after it has applied the changes but before it has committed them. Continuing is not safe in that case, so give up and show the error. If the lock stops the command before the rebase starts, git cleans up after itself, and we run the command again as before. Co-Authored-By: Claude Opus 5.5 --- .../git_commands/lock_error_recovery.go | 92 +++++++++++++++ .../git_commands/lock_error_recovery_test.go | 108 ++++++++++++++++++ pkg/commands/git_commands/rebase.go | 14 ++- .../lock_error_partway_through_rebase.go | 15 --- 4 files changed, 212 insertions(+), 17 deletions(-) create mode 100644 pkg/commands/git_commands/lock_error_recovery.go create mode 100644 pkg/commands/git_commands/lock_error_recovery_test.go diff --git a/pkg/commands/git_commands/lock_error_recovery.go b/pkg/commands/git_commands/lock_error_recovery.go new file mode 100644 index 000000000..8925a65a4 --- /dev/null +++ b/pkg/commands/git_commands/lock_error_recovery.go @@ -0,0 +1,92 @@ +package git_commands + +import ( + "os" + "path/filepath" + "strings" + + "github.com/jesseduffield/lazygit/pkg/commands/oscommands" + "github.com/jesseduffield/lazygit/pkg/utils" +) + +// resumeRebaseAfterLockError sets the lock-error recovery (see +// CmdObj.RecoverFromLockErrorWith) of cmdObj, a command that starts or +// continues a rebase in the worktree whose git dir is gitDir. Call it right +// before running the command, since it records the state of the rebase at +// that time. +// +// If a lock stops the command before it changes the state of the rebase, we +// run the command again. This includes a command that hasn't started the +// rebase yet, since git cleans up after itself in that case. If a lock stops +// the rebase partway through, git normally reschedules the todo that failed, +// and the command made by newContinueCmdObj ("git rebase --continue") runs it +// again. In some cases git doesn't reschedule it, for example if the lock +// stops a pick after it has applied the changes but before it has committed +// them. Continuing could then lose the commit, so we give up and leave the +// rebase to the user. +func resumeRebaseAfterLockError( + cmdObj *oscommands.CmdObj, + gitDir string, + commentChar byte, + newContinueCmdObj func() *oscommands.CmdObj, +) *oscommands.CmdObj { + var recoverFrom func(cmdObj *oscommands.CmdObj, stateBefore string) *oscommands.CmdObj + recoverFrom = func(cmdObj *oscommands.CmdObj, stateBefore string) *oscommands.CmdObj { + return cmdObj.RecoverFromLockErrorWith(func() *oscommands.CmdObj { + state := rebaseState(gitDir) + if state == stateBefore { + return cmdObj + } + + if !wasLastRebaseTodoRescheduled(gitDir, commentChar) { + return nil + } + + return recoverFrom(newContinueCmdObj(), state) + }) + } + + return recoverFrom(cmdObj, rebaseState(gitDir)) +} + +// 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. +func rebaseState(gitDir string) string { + var state strings.Builder + for _, dir := range []string{"rebase-merge", "rebase-apply"} { + if dirExists(filepath.Join(gitDir, dir)) { + state.WriteString(dir + "\n") + } + } + for _, file := range []string{"rebase-merge/done", "rebase-merge/git-rebase-todo", "rebase-apply/next"} { + if content, err := os.ReadFile(filepath.Join(gitDir, file)); err == nil { + state.WriteString(file + "\n") + state.Write(content) + } + } + return state.String() +} + +// wasLastRebaseTodoRescheduled tells whether git put the todo that it executed +// last back into the todo list of the rebase that is in progress in gitDir. +// It's false if no rebase is in progress, or if the rebase doesn't use the +// rebase-merge directory; that kind of rebase never reschedules anything. +func wasLastRebaseTodoRescheduled(gitDir string, commentChar byte) bool { + doneTodos, err := utils.ReadRebaseTodoFile(filepath.Join(gitDir, "rebase-merge", "done"), commentChar) + if err != nil { + return false + } + + todos, err := utils.ReadRebaseTodoFile(filepath.Join(gitDir, "rebase-merge", "git-rebase-todo"), commentChar) + if err != nil { + return false + } + + return utils.LastDoneTodoWasRescheduled(doneTodos, todos) +} + +func dirExists(path string) bool { + info, err := os.Stat(path) + return err == nil && info.IsDir() +} diff --git a/pkg/commands/git_commands/lock_error_recovery_test.go b/pkg/commands/git_commands/lock_error_recovery_test.go new file mode 100644 index 000000000..1fe967983 --- /dev/null +++ b/pkg/commands/git_commands/lock_error_recovery_test.go @@ -0,0 +1,108 @@ +package git_commands + +import ( + "os" + "path/filepath" + "testing" + + "github.com/jesseduffield/lazygit/pkg/commands/oscommands" + "github.com/stretchr/testify/assert" +) + +func TestResumeRebaseAfterLockError(t *testing.T) { + newCmdObj := func(args ...string) *oscommands.CmdObj { + return oscommands.NewDummyCmdObjBuilder(nil).New(args) + } + + writeRebaseMergeFiles := func(gitDir string, done string, todo string) { + dir := filepath.Join(gitDir, "rebase-merge") + assert.NoError(t, os.MkdirAll(dir, 0o755)) + assert.NoError(t, os.WriteFile(filepath.Join(dir, "done"), []byte(done), 0o644)) + assert.NoError(t, os.WriteFile(filepath.Join(dir, "git-rebase-todo"), []byte(todo), 0o644)) + } + + // Sets up the recovery for cmdObj, then calls runUntilLockError to do what + // the command did before the lock stopped it, and returns the command that + // the recovery wants to run next. + recoverFromLockError := func(gitDir string, cmdObj *oscommands.CmdObj, runUntilLockError func()) *oscommands.CmdObj { + resumeRebaseAfterLockError(cmdObj, gitDir, '#', func() *oscommands.CmdObj { + return newCmdObj("git", "rebase", "--continue") + }) + runUntilLockError() + return cmdObj.GetLockErrorRecovery()() + } + + t.Run("runs the command again if it didn't start the rebase", func(t *testing.T) { + cmdObj := newCmdObj("git", "rebase", "--interactive", "main") + + assert.Same(t, cmdObj, recoverFromLockError(t.TempDir(), cmdObj, func() {})) + }) + + t.Run("runs the command again if it didn't change the rebase", func(t *testing.T) { + gitDir := t.TempDir() + writeRebaseMergeFiles(gitDir, + "pick 1234567 one\n", + "pick 2345678 two\n") + cmdObj := newCmdObj("git", "rebase", "--continue") + + assert.Same(t, cmdObj, recoverFromLockError(gitDir, cmdObj, func() {})) + }) + + t.Run("continues the rebase if git rescheduled the todo that failed", func(t *testing.T) { + gitDir := t.TempDir() + + next := recoverFromLockError(gitDir, newCmdObj("git", "rebase", "--interactive", "main"), func() { + writeRebaseMergeFiles(gitDir, + "pick 1234567 one\npick 2345678 two\n", + "pick 2345678 two\npick 3456789 three\n") + }) + + assert.Equal(t, "git rebase --continue", next.ToString()) + + // If continuing fails before it changes the rebase, we run it again + assert.Same(t, next, next.GetLockErrorRecovery()()) + + // If continuing fails after it has changed the rebase, and git + // rescheduled the todo that failed, we continue again + writeRebaseMergeFiles(gitDir, + "pick 1234567 one\npick 2345678 two\npick 2345678 two\npick 3456789 three\n", + "pick 3456789 three\n") + nextAfterContinuing := next.GetLockErrorRecovery()() + assert.Equal(t, "git rebase --continue", nextAfterContinuing.ToString()) + assert.NotSame(t, next, nextAfterContinuing) + + // If the rebase is gone after continuing failed, we give up + assert.NoError(t, os.RemoveAll(filepath.Join(gitDir, "rebase-merge"))) + assert.Nil(t, nextAfterContinuing.GetLockErrorRecovery()()) + }) + + t.Run("continues the rebase if an older git version rescheduled the todo that failed", func(t *testing.T) { + gitDir := t.TempDir() + + next := recoverFromLockError(gitDir, newCmdObj("git", "rebase", "--interactive", "main"), func() { + writeRebaseMergeFiles(gitDir, + "pick 1234567 one\npick 2345678 two\npick 1234567 one\n", + "pick 2345678 two\npick 3456789 three\n") + }) + + assert.Equal(t, "git rebase --continue", next.ToString()) + }) + + t.Run("gives up if git didn't reschedule the todo that failed", func(t *testing.T) { + gitDir := t.TempDir() + + assert.Nil(t, recoverFromLockError(gitDir, newCmdObj("git", "rebase", "--interactive", "main"), func() { + writeRebaseMergeFiles(gitDir, + "pick 1234567 one\npick 2345678 two\n", + "pick 3456789 three\n") + })) + }) + + t.Run("gives up if a rebase without a rebase-merge directory is in progress", func(t *testing.T) { + gitDir := t.TempDir() + + assert.Nil(t, recoverFromLockError(gitDir, newCmdObj("git", "pull"), func() { + assert.NoError(t, os.MkdirAll(filepath.Join(gitDir, "rebase-apply"), 0o755)) + })) + }) +} diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index be0485e96..bce778472 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -256,7 +256,7 @@ func (self *RebaseCommands) PrepareInteractiveRebaseCommand(opts PrepareInteract cmdObj.AddEnvVars("GIT_EDITOR=" + ex) } - return cmdObj + return self.resumeAfterLockError(cmdObj) } // GitRebaseEditTodo runs "git rebase --edit-todo", saving the given todosFileContent to the file @@ -408,7 +408,7 @@ func (self *RebaseCommands) SquashAllAboveFixupCommits(commit *models.Commit) er Arg("--interactive", "--rebase-merges", "--autostash", "--autosquash", hashOrRoot). ToArgv() - return skipEditor(self.cmd.New(cmdArgs)).Run() + return self.resumeAfterLockError(skipEditor(self.cmd.New(cmdArgs))).Run() } // BeginInteractiveRebaseForCommit starts an interactive rebase to edit the current @@ -513,6 +513,16 @@ 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 +// resumeRebaseAfterLockError. +func (self *RebaseCommands) resumeAfterLockError(cmdObj *oscommands.CmdObj) *oscommands.CmdObj { + return resumeRebaseAfterLockError(cmdObj, self.repoPaths.WorktreeGitDirPath(), self.config.GetCoreCommentChar(), + func() *oscommands.CmdObj { + return skipEditor(self.GenericMergeOrRebaseActionCmdObj("rebase", "continue")) + }) +} + // skipEditor makes git use lazygit as the editor for the given command; lazygit // then exits right away, so git uses the file as it is. func skipEditor(cmdObj *oscommands.CmdObj) *oscommands.CmdObj { diff --git a/pkg/integration/tests/interactive_rebase/lock_error_partway_through_rebase.go b/pkg/integration/tests/interactive_rebase/lock_error_partway_through_rebase.go index 0d0b9cf92..d5d697dfc 100644 --- a/pkg/integration/tests/interactive_rebase/lock_error_partway_through_rebase.go +++ b/pkg/integration/tests/interactive_rebase/lock_error_partway_through_rebase.go @@ -34,26 +34,11 @@ var LockErrorPartwayThroughRebase = NewIntegrationTest(NewIntegrationTestArgs{ Content(Contains("Are you sure you want to drop the selected commit(s)?")). Confirm() }). - /* EXPECTED: IsFocused(). Lines( Contains("commit-04"), Contains("commit-03"), Contains("commit-01"), ) - ACTUAL: */ - Tap(func() { - t.ExpectPopup().Alert(). - Title(Equals("Error")). - Content(Contains("It seems that there is already a rebase-merge directory")). - Confirm() - }). - Lines( - Contains("─── Pending rebase todos"), - Contains("pick").Contains("commit-04"), - Contains("─── Commits"), - Contains("commit-03"), - Contains("commit-01"), - ) }, }) From 3fb06d9095fa254d73886b19b667a1e23aa36e19 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 7 Nov 2026 09:41:20 +0100 Subject: [PATCH 06/15] Add a test for a lock error partway through a pull with rebase This has the same problem as an interactive rebase. If pull.rebase is set and a lock stops the rebase partway through, the git command runner runs "git pull" again. This fails with "You are not currently on a branch" because HEAD is detached during the rebase, and the rebase is left stopped halfway. Co-Authored-By: Claude Opus 5.5 --- ..._error_partway_through_pull_with_rebase.go | 80 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 2 files changed, 81 insertions(+) create mode 100644 pkg/integration/tests/sync/lock_error_partway_through_pull_with_rebase.go diff --git a/pkg/integration/tests/sync/lock_error_partway_through_pull_with_rebase.go b/pkg/integration/tests/sync/lock_error_partway_through_pull_with_rebase.go new file mode 100644 index 000000000..273f3e9fd --- /dev/null +++ b/pkg/integration/tests/sync/lock_error_partway_through_pull_with_rebase.go @@ -0,0 +1,80 @@ +package sync + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" + "github.com/jesseduffield/lazygit/pkg/integration/tests/shared" +) + +var LockErrorPartwayThroughPullWithRebase = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Pull with a rebase strategy while another git process takes index.lock partway through the rebase", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file", "content1") + shell.Commit("one") + shell.UpdateFileAndAdd("file", "content2") + shell.Commit("two") + shell.CreateFileAndAdd("file3", "content3") + shell.Commit("three") + + shell.CloneIntoRemote("origin") + + shell.SetBranchUpstream("master", "origin/master") + + shell.HardReset("HEAD^^") + shell.CreateFileAndAdd("file4", "content4") + shell.Commit("four") + shell.CreateFileAndAdd("file5", "content5") + shell.Commit("five") + + shell.SetConfig("pull.rebase", "true") + + // The rebase makes a commit when it picks "four"; the lock then stops + // it when it tries to pick "five". + shared.HoldIndexLockAfterNextCommit(shell) + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Lines( + Contains("five"), + Contains("four"), + Contains("one"), + ) + + t.Views().Status().Content(Equals("↓2↑2 repo → master")) + + t.Views().Files(). + IsFocused(). + Press(keys.Universal.Pull) + + /* EXPECTED: + t.Views().Status().Content(Equals("↑2 repo → master")) + + t.Views().Commits(). + Lines( + Contains("five"), + Contains("four"), + Contains("three"), + Contains("two"), + Contains("one"), + ) + ACTUAL: */ + t.ExpectPopup().Alert(). + Title(Equals("Error")). + Content(Contains("You are not currently on a branch.")). + Confirm() + + t.Views().Commits(). + Lines( + Contains("─── Pending rebase todos"), + Contains("pick").Contains("five"), + Contains("─── Commits"), + Contains("four"), + Contains("three"), + Contains("two"), + Contains("one"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 319966489..ddbfa63bc 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -644,6 +644,7 @@ var tests = []*components.IntegrationTest{ sync.ForcePushMultipleUpstream, sync.ForcePushRemoteBranchNotStoredLocally, sync.ForcePushTriangular, + sync.LockErrorPartwayThroughPullWithRebase, sync.Pull, sync.PullAndSetUpstream, sync.PullMerge, From 6dc34a62d225ab11659017144b473e519c90f2a7 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 7 Nov 2026 09:43:25 +0100 Subject: [PATCH 07/15] Resume a pull whose rebase a lock error stopped partway through If pull.rebase is set and a lock stops the rebase partway through, the git command runner ran "git pull" again. This failed with "You are not currently on a branch", and the rebase was left stopped halfway. Handle this the same way as for an interactive rebase. Continue the rebase if git rescheduled the todo that failed, give up if it didn't, and run the pull again if the lock stopped it before the rebase started. Co-Authored-By: Claude Opus 5.5 --- pkg/commands/git_commands/sync.go | 14 +++++++++++++- ...ck_error_partway_through_pull_with_rebase.go | 17 ----------------- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/pkg/commands/git_commands/sync.go b/pkg/commands/git_commands/sync.go index 603e5b8eb..59f052db8 100644 --- a/pkg/commands/git_commands/sync.go +++ b/pkg/commands/git_commands/sync.go @@ -111,7 +111,19 @@ func (self *SyncCommands) Pull(task gocui.Task, opts PullOptions) error { // setting GIT_SEQUENCE_EDITOR to ':' as a way of skipping it, in case the user // has 'pull.rebase = interactive' configured. - return self.cmd.New(cmdArgs).AddEnvVars("GIT_SEQUENCE_EDITOR=:").PromptOnCredentialRequest(task).Run() + cmdObj := self.cmd.New(cmdArgs).AddEnvVars("GIT_SEQUENCE_EDITOR=:").PromptOnCredentialRequest(task) + + // With pull.rebase set, a lock error can stop the pull partway through its + // rebase. + gitDir := lo.Ternary(opts.WorktreeGitDir != "", opts.WorktreeGitDir, self.repoPaths.WorktreeGitDirPath()) + return resumeRebaseAfterLockError(cmdObj, gitDir, self.config.GetCoreCommentChar(), func() *oscommands.CmdObj { + continueArgs := NewGitCmd("rebase"). + Arg("--continue"). + GitDirIf(opts.WorktreeGitDir != "", opts.WorktreeGitDir). + WorktreePathIf(opts.WorktreePath != "", opts.WorktreePath). + ToArgv() + return skipEditor(self.cmd.New(continueArgs)) + }).Run() } // Fetches the given branches of the given remote, updating their diff --git a/pkg/integration/tests/sync/lock_error_partway_through_pull_with_rebase.go b/pkg/integration/tests/sync/lock_error_partway_through_pull_with_rebase.go index 273f3e9fd..3135dd8b2 100644 --- a/pkg/integration/tests/sync/lock_error_partway_through_pull_with_rebase.go +++ b/pkg/integration/tests/sync/lock_error_partway_through_pull_with_rebase.go @@ -49,7 +49,6 @@ var LockErrorPartwayThroughPullWithRebase = NewIntegrationTest(NewIntegrationTes IsFocused(). Press(keys.Universal.Pull) - /* EXPECTED: t.Views().Status().Content(Equals("↑2 repo → master")) t.Views().Commits(). @@ -60,21 +59,5 @@ var LockErrorPartwayThroughPullWithRebase = NewIntegrationTest(NewIntegrationTes Contains("two"), Contains("one"), ) - ACTUAL: */ - t.ExpectPopup().Alert(). - Title(Equals("Error")). - Content(Contains("You are not currently on a branch.")). - Confirm() - - t.Views().Commits(). - Lines( - Contains("─── Pending rebase todos"), - Contains("pick").Contains("five"), - Contains("─── Commits"), - Contains("four"), - Contains("three"), - Contains("two"), - Contains("one"), - ) }, }) From 6614d4d8f87dc74ccd9b1505636994fd9f83123c Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 7 Nov 2026 09:46:25 +0100 Subject: [PATCH 08/15] Add a test for a cherry-pick that stops before it applies a commit If a cherry-pick of several commits stops before git starts to apply one of them, for example because the commit would overwrite an untracked file, lazygit doesn't show the cherry-pick as in progress. Git doesn't write CHERRY_PICK_HEAD in that case, and this is the only thing we look at. The user has no way to continue or abort the cherry-pick from lazygit, and the next cherry-pick fails with "cherry-pick is already in progress". Co-Authored-By: Claude Opus 5.5 --- .../cherry_pick_stopped_by_untracked_file.go | 108 ++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 2 files changed, 109 insertions(+) create mode 100644 pkg/integration/tests/cherry_pick/cherry_pick_stopped_by_untracked_file.go diff --git a/pkg/integration/tests/cherry_pick/cherry_pick_stopped_by_untracked_file.go b/pkg/integration/tests/cherry_pick/cherry_pick_stopped_by_untracked_file.go new file mode 100644 index 000000000..54e7dca0b --- /dev/null +++ b/pkg/integration/tests/cherry_pick/cherry_pick_stopped_by_untracked_file.go @@ -0,0 +1,108 @@ +package cherry_pick + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var CherryPickStoppedByUntrackedFile = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Cherry pick two commits; an untracked file stops the second one before git starts to apply it; continue after removing the file", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Git.LocalBranchSortOrder = "recency" + }, + SetupRepo: func(shell *Shell) { + shell. + CreateFileAndAdd("file", "original\n"). + Commit("base"). + NewBranch("source"). + CreateFileAndAdd("file1", "one\n"). + Commit("one"). + CreateFileAndAdd("file2", "two\n"). + Commit("two"). + Checkout("master"). + CreateFile("file2", "untracked\n") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Branches(). + Focus(). + Lines( + Contains("master"), + Contains("source"), + ). + SelectNextItem(). + PressEnter() + + t.Views().SubCommits(). + IsFocused(). + Lines( + Contains("two").IsSelected(), + Contains("one"), + Contains("base"), + ). + Press(keys.Commits.CherryPickCopy). + SelectNextItem(). + Press(keys.Commits.CherryPickCopy) + + t.Views().Information().Content(Contains("2 commits copied")) + + t.Views().Commits(). + Focus(). + Lines( + Contains("base").IsSelected(), + ). + Press(keys.Commits.PasteCommits). + Tap(func() { + t.ExpectPopup().Alert(). + Title(Equals("Cherry-pick")). + Content(Contains("Are you sure you want to cherry-pick the 2 copied commit(s) onto this branch?")). + Confirm() + }). + Tap(func() { + t.ExpectPopup().Alert(). + Title(Equals("Error")). + Content(Contains("The following untracked working tree files would be overwritten by merge")). + Confirm() + }). + /* EXPECTED: + Lines( + Contains("─── Pending cherry-picks"), + Contains("pick").DoesNotContain("CONFLICT").Contains("two"), + Contains("─── Commits"), + Contains("one"), + Contains("base"), + ) + ACTUAL: */ + Lines( + Contains("one"), + Contains("base"), + ) + + t.Views().Status(). + /* EXPECTED: + Content(Equals("(cherry-picking) repo → master")) + ACTUAL: */ + Content(Equals("repo → master")) + + t.Shell().DeleteFile("file2") + + t.GlobalPress(keys.Universal.CreateRebaseOptionsMenu) + /* EXPECTED: + t.ExpectPopup().Menu(). + Title(Equals("Cherry-pick options")). + Select(Contains("continue")). + Confirm() + + t.Views().Status().Content(Equals("repo → master")) + + t.Views().Commits(). + Lines( + Contains("two"), + Contains("one"), + Contains("base"), + ) + ACTUAL: */ + t.ExpectToast(Equals("Disabled: You are currently neither rebasing nor merging")) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index ddbfa63bc..6da04eae1 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -115,6 +115,7 @@ var tests = []*components.IntegrationTest{ cherry_pick.CherryPickMerge, cherry_pick.CherryPickRange, cherry_pick.CherryPickRangeAfterPaste, + cherry_pick.CherryPickStoppedByUntrackedFile, commit.AddCoAuthor, commit.AddCoAuthorRange, commit.AddCoAuthorWhileCommitting, From 4e24ef025b24a146a39f78d3a32322eb7527411e Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 7 Nov 2026 09:48:08 +0100 Subject: [PATCH 09/15] Show a cherry-pick or revert as in progress if only .git/sequencer exists If a cherry-pick or revert of several commits stops before git starts to apply one of them, git doesn't write CHERRY_PICK_HEAD or REVERT_HEAD. It only leaves the todos behind in .git/sequencer. Look at the first todo there, like git status does. If it is a "pick" or "revert", consider a cherry-pick or revert to be in progress. Once we show such an operation, two more things need care. The same state occurs if the user resolves a conflict and commits the result themselves. Git tells the two cases apart by whether HEAD has moved since it last tried to apply a commit, so do the same. Mark the commit that git stopped at as conflicted only if CHERRY_PICK_HEAD or REVERT_HEAD exists, and don't show it as pending if the user has committed it. Also, "git cherry-pick --continue" skips the commit that git stopped at, even if git never applied it. Before continuing in that case, put a copy of its todo in front of it, so that git skips the copy instead. Co-Authored-By: Claude Opus 5.5 --- pkg/commands/git_commands/commit_loader.go | 11 +- pkg/commands/git_commands/rebase.go | 11 ++ pkg/commands/git_commands/sequencer.go | 101 ++++++++++++ pkg/commands/git_commands/sequencer_test.go | 145 ++++++++++++++++++ pkg/commands/git_commands/status.go | 31 +++- pkg/commands/git_commands/status_test.go | 53 +++++++ ...cherry_pick_conflicts_committed_by_user.go | 84 ++++++++++ .../cherry_pick_stopped_by_untracked_file.go | 12 -- pkg/integration/tests/test_list.go | 1 + 9 files changed, 434 insertions(+), 15 deletions(-) create mode 100644 pkg/commands/git_commands/sequencer.go create mode 100644 pkg/commands/git_commands/sequencer_test.go create mode 100644 pkg/integration/tests/cherry_pick/cherry_pick_conflicts_committed_by_user.go diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index b4a938880..c0b6cf580 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -259,8 +259,15 @@ func (self *CommitLoader) getHydratedSequencerCommits(hashPool *utils.StringPool commits := self.getSequencerCommits(hashPool) if len(commits) > 0 { // If we have any commits in .git/sequencer/todo, then the last one of - // those is the conflicting one. - commits[len(commits)-1].Status = models.StatusConflicted + // those is the one that git stopped at. + switch getCurrentSequencerTodoState(self.cmd, self.repoPaths.WorktreeGitDirPath()) { + case sequencerTodoStarted: + commits[len(commits)-1].Status = models.StatusConflicted + case sequencerTodoNotStarted: + // It is still pending like the other ones + case sequencerTodoCommitted: + commits = commits[:len(commits)-1] + } } else { // For single-commit cherry-picks and reverts, git apparently doesn't // use the sequencer; in that case, CHERRY_PICK_HEAD or REVERT_HEAD is diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index bce778472..6098f9493 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -491,8 +491,19 @@ func (self *RebaseCommands) AbortRebase() error { // GenericMerge takes a commandType of "merge" or "rebase" and a command of "abort", "skip" or "continue" // By default we skip the editor in the case where a commit will be made func (self *RebaseCommands) GenericMergeOrRebaseAction(commandType string, command string) error { + undoRepeatingTodo := func() {} + if command == "continue" && (commandType == "cherry-pick" || commandType == "revert") { + var err error + undoRepeatingTodo, err = repeatCurrentSequencerTodo( + self.cmd, self.repoPaths.WorktreeGitDirPath(), self.config.GetCoreCommentChar()) + if err != nil { + return err + } + } + err := skipEditor(self.GenericMergeOrRebaseActionCmdObj(commandType, command)).Run() if err != nil { + undoRepeatingTodo() if !strings.Contains(err.Error(), "no rebase in progress") { return err } diff --git a/pkg/commands/git_commands/sequencer.go b/pkg/commands/git_commands/sequencer.go new file mode 100644 index 000000000..b216cc21e --- /dev/null +++ b/pkg/commands/git_commands/sequencer.go @@ -0,0 +1,101 @@ +package git_commands + +import ( + "bytes" + "os" + "path/filepath" + "strings" + + "github.com/jesseduffield/lazygit/pkg/commands/oscommands" + "github.com/samber/lo" +) + +// sequencerTodoState tells what has happened so far with the todo that a +// cherry-pick or revert of several commits stopped at. This is the first todo +// in .git/sequencer/todo. +type sequencerTodoState int + +const ( + // Git started to apply the todo, and stopped because of conflicts (or + // because it couldn't commit the result). CHERRY_PICK_HEAD or REVERT_HEAD + // exists, and continuing commits the changes. + sequencerTodoStarted sequencerTodoState = iota + // Git stopped before it started to apply the todo, for example because the + // commit would overwrite an untracked file. + sequencerTodoNotStarted + // The user has committed the result of the todo themselves, for example + // after resolving its conflicts. Continuing skips it. + sequencerTodoCommitted +) + +// getCurrentSequencerTodoState returns the state of the todo that the +// cherry-pick or revert of several commits that is in progress in gitDir +// stopped at. +// +// Without CHERRY_PICK_HEAD or REVERT_HEAD, we can tell whether the user has +// committed the todo by whether HEAD has moved since git last tried to apply a +// commit, which git records in .git/sequencer/abort-safety. Git does the same +// when it decides whether "--skip" is possible (see rollback_is_safe in git's +// sequencer.c). +func getCurrentSequencerTodoState(cmd oscommands.ICmdObjBuilder, gitDir string) sequencerTodoState { + for _, file := range []string{"CHERRY_PICK_HEAD", "REVERT_HEAD"} { + if _, err := os.Stat(filepath.Join(gitDir, file)); err == nil { + return sequencerTodoStarted + } + } + + // If the file doesn't exist, git compares HEAD to the null hash; an empty + // string does the same for us + abortSafety, _ := os.ReadFile(filepath.Join(gitDir, "sequencer", "abort-safety")) + + // If HEAD is unborn, this fails and returns an empty string + head, _ := cmd.New(NewGitCmd("rev-parse").Arg("--verify", "--quiet", "HEAD").ToArgv()).DontLog().RunWithOutput() + + if strings.TrimSpace(head) == strings.TrimSpace(string(abortSafety)) { + return sequencerTodoNotStarted + } + return sequencerTodoCommitted +} + +// repeatCurrentSequencerTodo prepares the cherry-pick or revert of several +// commits that is in progress in gitDir for "--continue". If git stopped it +// before it started to apply the current todo, "--continue" would skip that +// todo, because git can't tell it apart from a todo that the user has +// committed themselves. So we put a copy of the todo in front of it, and git +// skips the copy instead. +// +// The returned function removes the copy again if git hasn't got as far as +// skipping it, for example because it refused to continue. +func repeatCurrentSequencerTodo(cmd oscommands.ICmdObjBuilder, gitDir string, commentChar byte) (func(), error) { + noop := func() {} + + todoPath := filepath.Join(gitDir, "sequencer", "todo") + content, err := os.ReadFile(todoPath) + if err != nil { + // No cherry-pick or revert of several commits is in progress + return noop, nil + } + + if getCurrentSequencerTodoState(cmd, gitDir) != sequencerTodoNotStarted { + return noop, nil + } + + currentTodo, found := lo.Find(strings.Split(string(content), "\n"), func(line string) bool { + line = strings.TrimSpace(line) + return line != "" && line[0] != commentChar + }) + if !found { + return noop, nil + } + + newContent := append([]byte(currentTodo+"\n"), content...) + if err := os.WriteFile(todoPath, newContent, 0o644); err != nil { + return nil, err + } + + return func() { + if currentContent, err := os.ReadFile(todoPath); err == nil && bytes.Equal(currentContent, newContent) { + _ = os.WriteFile(todoPath, content, 0o644) + } + }, nil +} diff --git a/pkg/commands/git_commands/sequencer_test.go b/pkg/commands/git_commands/sequencer_test.go new file mode 100644 index 000000000..0ee54e3bb --- /dev/null +++ b/pkg/commands/git_commands/sequencer_test.go @@ -0,0 +1,145 @@ +package git_commands + +import ( + "errors" + "os" + "path/filepath" + "testing" + + "github.com/jesseduffield/lazygit/pkg/commands/oscommands" + "github.com/stretchr/testify/assert" +) + +func TestGetCurrentSequencerTodoState(t *testing.T) { + revParseArgs := []string{"rev-parse", "--verify", "--quiet", "HEAD"} + + scenarios := []struct { + testName string + files map[string]string + runner *oscommands.FakeCmdObjRunner + expectedState sequencerTodoState + }{ + { + testName: "CHERRY_PICK_HEAD exists", + files: map[string]string{"CHERRY_PICK_HEAD": "1234567\n"}, + runner: oscommands.NewFakeRunner(t), + expectedState: sequencerTodoStarted, + }, + { + testName: "REVERT_HEAD exists", + files: map[string]string{"REVERT_HEAD": "1234567\n"}, + runner: oscommands.NewFakeRunner(t), + expectedState: sequencerTodoStarted, + }, + { + testName: "HEAD hasn't moved since git tried to apply a commit", + files: map[string]string{"sequencer/abort-safety": "1234567\n"}, + runner: oscommands.NewFakeRunner(t).ExpectGitArgs(revParseArgs, "1234567\n", nil), + expectedState: sequencerTodoNotStarted, + }, + { + testName: "HEAD has moved since git tried to apply a commit", + files: map[string]string{"sequencer/abort-safety": "1234567\n"}, + runner: oscommands.NewFakeRunner(t).ExpectGitArgs(revParseArgs, "2345678\n", nil), + expectedState: sequencerTodoCommitted, + }, + { + testName: "HEAD is unborn, and there is no abort-safety file", + files: map[string]string{}, + runner: oscommands.NewFakeRunner(t).ExpectGitArgs(revParseArgs, "", errors.New("error")), + expectedState: sequencerTodoNotStarted, + }, + } + + for _, s := range scenarios { + t.Run(s.testName, func(t *testing.T) { + gitDir := t.TempDir() + for path, content := range s.files { + fullPath := filepath.Join(gitDir, path) + assert.NoError(t, os.MkdirAll(filepath.Dir(fullPath), 0o755)) + assert.NoError(t, os.WriteFile(fullPath, []byte(content), 0o644)) + } + + state := getCurrentSequencerTodoState(oscommands.NewDummyCmdObjBuilder(s.runner), gitDir) + + assert.Equal(t, s.expectedState, state) + s.runner.CheckForMissingCalls() + }) + } +} + +func TestRepeatCurrentSequencerTodo(t *testing.T) { + revParseArgs := []string{"rev-parse", "--verify", "--quiet", "HEAD"} + todos := "pick 1234567 one\npick 2345678 two\n" + repeatedTodos := "pick 1234567 one\npick 1234567 one\npick 2345678 two\n" + + setUp := func(t *testing.T, files map[string]string) string { + t.Helper() + gitDir := t.TempDir() + for path, content := range files { + fullPath := filepath.Join(gitDir, path) + assert.NoError(t, os.MkdirAll(filepath.Dir(fullPath), 0o755)) + assert.NoError(t, os.WriteFile(fullPath, []byte(content), 0o644)) + } + return gitDir + } + + readTodos := func(t *testing.T, gitDir string) string { + t.Helper() + content, err := os.ReadFile(filepath.Join(gitDir, "sequencer", "todo")) + assert.NoError(t, err) + return string(content) + } + + t.Run("no cherry-pick or revert of several commits in progress", func(t *testing.T) { + gitDir := setUp(t, map[string]string{}) + runner := oscommands.NewFakeRunner(t) + + undo, err := repeatCurrentSequencerTodo(oscommands.NewDummyCmdObjBuilder(runner), gitDir, '#') + + assert.NoError(t, err) + undo() + assert.NoFileExists(t, filepath.Join(gitDir, "sequencer", "todo")) + runner.CheckForMissingCalls() + }) + + t.Run("git started to apply the current todo", func(t *testing.T) { + gitDir := setUp(t, map[string]string{"sequencer/todo": todos, "CHERRY_PICK_HEAD": "1234567\n"}) + runner := oscommands.NewFakeRunner(t) + + _, err := repeatCurrentSequencerTodo(oscommands.NewDummyCmdObjBuilder(runner), gitDir, '#') + + assert.NoError(t, err) + assert.Equal(t, todos, readTodos(t, gitDir)) + runner.CheckForMissingCalls() + }) + + t.Run("git stopped before it started to apply the current todo", func(t *testing.T) { + gitDir := setUp(t, map[string]string{"sequencer/todo": todos, "sequencer/abort-safety": "abcdef0\n"}) + runner := oscommands.NewFakeRunner(t).ExpectGitArgs(revParseArgs, "abcdef0\n", nil) + + undo, err := repeatCurrentSequencerTodo(oscommands.NewDummyCmdObjBuilder(runner), gitDir, '#') + + assert.NoError(t, err) + assert.Equal(t, repeatedTodos, readTodos(t, gitDir)) + + // If git didn't touch the todos, undo restores them + undo() + assert.Equal(t, todos, readTodos(t, gitDir)) + runner.CheckForMissingCalls() + }) + + t.Run("undo leaves the todos alone if git has changed them", func(t *testing.T) { + gitDir := setUp(t, map[string]string{"sequencer/todo": todos, "sequencer/abort-safety": "abcdef0\n"}) + runner := oscommands.NewFakeRunner(t).ExpectGitArgs(revParseArgs, "abcdef0\n", nil) + + undo, err := repeatCurrentSequencerTodo(oscommands.NewDummyCmdObjBuilder(runner), gitDir, '#') + assert.NoError(t, err) + gitsTodos := "pick 2345678 two\n" + assert.NoError(t, os.WriteFile(filepath.Join(gitDir, "sequencer", "todo"), []byte(gitsTodos), 0o644)) + + undo() + assert.Equal(t, gitsTodos, readTodos(t, gitDir)) + runner.CheckForMissingCalls() + }) +} diff --git a/pkg/commands/git_commands/status.go b/pkg/commands/git_commands/status.go index d9120d87d..cdec190c4 100644 --- a/pkg/commands/git_commands/status.go +++ b/pkg/commands/git_commands/status.go @@ -7,7 +7,9 @@ import ( "time" "github.com/jesseduffield/lazygit/pkg/commands/models" + "github.com/jesseduffield/lazygit/pkg/utils" "github.com/spf13/afero" + "github.com/stefanhaller/git-todo-parser/todo" ) type StatusCommands struct { @@ -49,6 +51,14 @@ func (self *StatusCommands) IsInMergeState() (bool, error) { } func (self *StatusCommands) IsInCherryPick() (bool, error) { + hasCherryPickHead, err := self.hasCherryPickHead() + if err != nil || hasCherryPickHead { + return hasCherryPickHead, err + } + return self.sequencerCommand() == todo.Pick, nil +} + +func (self *StatusCommands) hasCherryPickHead() (bool, error) { exists, err := self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "CHERRY_PICK_HEAD")) if err != nil || !exists { return exists, err @@ -81,7 +91,26 @@ func (self *StatusCommands) IsInCherryPick() (bool, error) { } func (self *StatusCommands) IsInRevert() (bool, error) { - return self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "REVERT_HEAD")) + hasRevertHead, err := self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "REVERT_HEAD")) + if err != nil || hasRevertHead { + return hasRevertHead, err + } + return self.sequencerCommand() == todo.Revert, nil +} + +// sequencerCommand returns the command of the first todo of a cherry-pick or +// revert of several commits that is in progress, or 0 if there is none. Git +// keeps the todos of such an operation in .git/sequencer. If it stops before it +// starts to apply a commit (for example because the commit would overwrite an +// untracked file), it doesn't write CHERRY_PICK_HEAD or REVERT_HEAD. Like git +// status, we still consider the cherry-pick or revert to be in progress then. +func (self *StatusCommands) sequencerCommand() todo.TodoCommand { + todos, err := utils.ReadRebaseTodoFile( + filepath.Join(self.repoPaths.WorktreeGitDirPath(), "sequencer", "todo"), self.config.GetCoreCommentChar()) + if err != nil || len(todos) == 0 { + return 0 + } + return todos[0].Command } // RefsSnapshot returns a string fingerprint of the current state of local diff --git a/pkg/commands/git_commands/status_test.go b/pkg/commands/git_commands/status_test.go index dc6b2d558..0ec71123a 100644 --- a/pkg/commands/git_commands/status_test.go +++ b/pkg/commands/git_commands/status_test.go @@ -1,6 +1,8 @@ package git_commands import ( + "os" + "path/filepath" "testing" "github.com/go-errors/errors" @@ -89,3 +91,54 @@ func TestStatusRefsSnapshot(t *testing.T) { }) } } + +func TestStatusWorkingTreeStateOfCherryPickOrRevert(t *testing.T) { + scenarios := []struct { + testName string + files map[string]string + expectedCherryPicking bool + expectedReverting bool + }{ + { + testName: "nothing in progress", + files: map[string]string{}, + }, + { + testName: "CHERRY_PICK_HEAD", + files: map[string]string{"CHERRY_PICK_HEAD": "1234567\n"}, + expectedCherryPicking: true, + }, + { + testName: "REVERT_HEAD", + files: map[string]string{"REVERT_HEAD": "1234567\n"}, + expectedReverting: true, + }, + { + testName: "cherry-pick of several commits that stopped before applying one", + files: map[string]string{"sequencer/todo": "pick 1234567 one\npick 2345678 two\n"}, + expectedCherryPicking: true, + }, + { + testName: "revert of several commits that stopped before applying one", + files: map[string]string{"sequencer/todo": "revert 1234567 one\nrevert 2345678 two\n"}, + expectedReverting: true, + }, + } + + for _, s := range scenarios { + t.Run(s.testName, func(t *testing.T) { + repoDir := t.TempDir() + for path, content := range s.files { + fullPath := filepath.Join(repoDir, ".git", path) + assert.NoError(t, os.MkdirAll(filepath.Dir(fullPath), 0o755)) + assert.NoError(t, os.WriteFile(fullPath, []byte(content), 0o644)) + } + instance := buildStatusCommands(commonDeps{repoPaths: MockRepoPaths(repoDir)}) + + state := instance.WorkingTreeState() + + assert.Equal(t, s.expectedCherryPicking, state.CherryPicking) + assert.Equal(t, s.expectedReverting, state.Reverting) + }) + } +} diff --git a/pkg/integration/tests/cherry_pick/cherry_pick_conflicts_committed_by_user.go b/pkg/integration/tests/cherry_pick/cherry_pick_conflicts_committed_by_user.go new file mode 100644 index 000000000..be986de05 --- /dev/null +++ b/pkg/integration/tests/cherry_pick/cherry_pick_conflicts_committed_by_user.go @@ -0,0 +1,84 @@ +package cherry_pick + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" + "github.com/jesseduffield/lazygit/pkg/integration/tests/shared" +) + +var CherryPickConflictsCommittedByUser = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Cherry pick two commits, the first of which conflicts; the user commits the resolution themselves before continuing", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Git.LocalBranchSortOrder = "recency" + }, + SetupRepo: func(shell *Shell) { + shared.MergeConflictsSetup(shell) + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Branches(). + Focus(). + Lines( + Contains("first-change-branch"), + Contains("second-change-branch"), + Contains("original-branch"), + ). + SelectNextItem(). + PressEnter() + + t.Views().SubCommits(). + IsFocused(). + TopLines( + Contains("second-change-branch unrelated change"), + Contains("second change"), + ). + Press(keys.Commits.CherryPickCopy). + SelectNextItem(). + Press(keys.Commits.CherryPickCopy) + + t.Views().Information().Content(Contains("2 commits copied")) + + t.Views().Commits(). + Focus(). + TopLines( + Contains("first change").IsSelected(), + ). + Press(keys.Commits.PasteCommits) + + t.ExpectPopup().Alert(). + Title(Equals("Cherry-pick")). + Content(Contains("Are you sure you want to cherry-pick the 2 copied commit(s) onto this branch?")). + Confirm() + + t.Common().AcknowledgeConflicts() + + t.Shell(). + UpdateFileAndAdd("file", shared.SecondChangeFileContent). + RunCommand([]string{"git", "commit", "--no-edit"}) + + t.GlobalPress(keys.Universal.Refresh) + + t.Views().Status().Content(Equals("(cherry-picking) repo → first-change-branch")) + + t.Views().Commits(). + TopLines( + Contains("─── Pending cherry-picks"), + Contains("pick").DoesNotContain("CONFLICT").Contains("second-change-branch unrelated change"), + Contains("─── Commits"), + Contains("second change"), + Contains("first change"), + ) + + t.Common().ContinueOnConflictsResolved("cherry-pick") + + t.Views().Status().Content(Equals("repo → first-change-branch")) + + t.Views().Commits(). + TopLines( + Contains("second-change-branch unrelated change"), + Contains("second change"), + Contains("first change"), + ) + }, +}) diff --git a/pkg/integration/tests/cherry_pick/cherry_pick_stopped_by_untracked_file.go b/pkg/integration/tests/cherry_pick/cherry_pick_stopped_by_untracked_file.go index 54e7dca0b..40c148cf6 100644 --- a/pkg/integration/tests/cherry_pick/cherry_pick_stopped_by_untracked_file.go +++ b/pkg/integration/tests/cherry_pick/cherry_pick_stopped_by_untracked_file.go @@ -65,7 +65,6 @@ var CherryPickStoppedByUntrackedFile = NewIntegrationTest(NewIntegrationTestArgs Content(Contains("The following untracked working tree files would be overwritten by merge")). Confirm() }). - /* EXPECTED: Lines( Contains("─── Pending cherry-picks"), Contains("pick").DoesNotContain("CONFLICT").Contains("two"), @@ -73,22 +72,13 @@ var CherryPickStoppedByUntrackedFile = NewIntegrationTest(NewIntegrationTestArgs Contains("one"), Contains("base"), ) - ACTUAL: */ - Lines( - Contains("one"), - Contains("base"), - ) t.Views().Status(). - /* EXPECTED: Content(Equals("(cherry-picking) repo → master")) - ACTUAL: */ - Content(Equals("repo → master")) t.Shell().DeleteFile("file2") t.GlobalPress(keys.Universal.CreateRebaseOptionsMenu) - /* EXPECTED: t.ExpectPopup().Menu(). Title(Equals("Cherry-pick options")). Select(Contains("continue")). @@ -102,7 +92,5 @@ var CherryPickStoppedByUntrackedFile = NewIntegrationTest(NewIntegrationTestArgs Contains("one"), Contains("base"), ) - ACTUAL: */ - t.ExpectToast(Equals("Disabled: You are currently neither rebasing nor merging")) }, }) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 6da04eae1..501e9a86b 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -110,6 +110,7 @@ var tests = []*components.IntegrationTest{ cherry_pick.CherryPickAcrossWorktrees, cherry_pick.CherryPickCommitThatBecomesEmpty, cherry_pick.CherryPickConflicts, + cherry_pick.CherryPickConflictsCommittedByUser, cherry_pick.CherryPickConflictsEmptyCommitAfterResolving, cherry_pick.CherryPickDuringRebase, cherry_pick.CherryPickMerge, From 3ecf3ae4cb1f814402d5ced0ac6340c6cf277226 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 7 Nov 2026 09:49:44 +0100 Subject: [PATCH 10/15] Add a test for a lock error partway through a cherry-pick If a lock stops a cherry-pick of several commits partway through, the git command runner runs the same "git cherry-pick" command again. This fails with "cherry-pick is already in progress", and the user is left with a cherry-pick that stopped halfway. Unlike a rebase, git doesn't reschedule the commit that failed, so continuing the cherry-pick would skip that commit. Co-Authored-By: Claude Opus 5.5 --- .../lock_error_partway_through_cherry_pick.go | 87 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 2 files changed, 88 insertions(+) create mode 100644 pkg/integration/tests/cherry_pick/lock_error_partway_through_cherry_pick.go diff --git a/pkg/integration/tests/cherry_pick/lock_error_partway_through_cherry_pick.go b/pkg/integration/tests/cherry_pick/lock_error_partway_through_cherry_pick.go new file mode 100644 index 000000000..b3329ab9a --- /dev/null +++ b/pkg/integration/tests/cherry_pick/lock_error_partway_through_cherry_pick.go @@ -0,0 +1,87 @@ +package cherry_pick + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" + "github.com/jesseduffield/lazygit/pkg/integration/tests/shared" +) + +var LockErrorPartwayThroughCherryPick = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Cherry pick two commits while another git process takes index.lock partway through", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Git.LocalBranchSortOrder = "recency" + }, + SetupRepo: func(shell *Shell) { + shell. + CreateFileAndAdd("file", "original\n"). + Commit("base"). + NewBranch("source"). + CreateFileAndAdd("file1", "one\n"). + Commit("one"). + CreateFileAndAdd("file2", "two\n"). + Commit("two"). + Checkout("master") + + // The cherry-pick commits "one"; the lock then stops it when it tries + // to pick "two". + shared.HoldIndexLockAfterNextCommit(shell) + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Branches(). + Focus(). + Lines( + Contains("master"), + Contains("source"), + ). + SelectNextItem(). + PressEnter() + + t.Views().SubCommits(). + IsFocused(). + Lines( + Contains("two").IsSelected(), + Contains("one"), + Contains("base"), + ). + Press(keys.Commits.CherryPickCopy). + SelectNextItem(). + Press(keys.Commits.CherryPickCopy) + + t.Views().Information().Content(Contains("2 commits copied")) + + t.Views().Commits(). + Focus(). + Lines( + Contains("base").IsSelected(), + ). + Press(keys.Commits.PasteCommits). + Tap(func() { + t.ExpectPopup().Alert(). + Title(Equals("Cherry-pick")). + Content(Contains("Are you sure you want to cherry-pick the 2 copied commit(s) onto this branch?")). + Confirm() + }). + /* EXPECTED: + Lines( + Contains("two"), + Contains("one"), + Contains("base"), + ) + ACTUAL: */ + Tap(func() { + t.ExpectPopup().Alert(). + Title(Equals("Error")). + Content(Contains("cherry-pick is already in progress")). + Confirm() + }). + Lines( + Contains("─── Pending cherry-picks"), + Contains("pick").Contains("two"), + Contains("─── Commits"), + Contains("one"), + Contains("base"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 501e9a86b..21aec9a55 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -117,6 +117,7 @@ var tests = []*components.IntegrationTest{ cherry_pick.CherryPickRange, cherry_pick.CherryPickRangeAfterPaste, cherry_pick.CherryPickStoppedByUntrackedFile, + cherry_pick.LockErrorPartwayThroughCherryPick, commit.AddCoAuthor, commit.AddCoAuthorRange, commit.AddCoAuthorWhileCommitting, From d4b11b8cfdb46d23d20b6f46a24339dacc9b18fc Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 7 Nov 2026 09:52:18 +0100 Subject: [PATCH 11/15] Abort and redo a cherry-pick or revert that a lock error stopped If a lock stops a cherry-pick or revert of several commits partway through, the git command runner ran the same command again. This failed with "cherry-pick is already in progress", and the user was left with an operation that stopped halfway. Continuing it would skip the commit that failed, because git doesn't reschedule it the way a rebase does. Abort the operation instead, and then run the command again. Also abort when the runner gives up, so that the user doesn't end up with the stopped operation. Only do this for several commits. For a single commit, git doesn't use .git/sequencer, and a cherry-pick of one commit may even run while a cherry-pick of several commits is in progress. If .git/sequencer exists after a single cherry-pick failed, it belongs to that other operation, and we must not abort it. Co-Authored-By: Claude Opus 5.5 --- pkg/commands/git_commands/commit.go | 5 +- .../git_commands/lock_error_recovery.go | 39 +++++++++++ .../git_commands/lock_error_recovery_test.go | 69 +++++++++++++++++++ pkg/commands/git_commands/rebase.go | 5 +- .../lock_error_partway_through_cherry_pick.go | 15 ---- 5 files changed, 116 insertions(+), 17 deletions(-) diff --git a/pkg/commands/git_commands/commit.go b/pkg/commands/git_commands/commit.go index e46fecd84..3d61a3e11 100644 --- a/pkg/commands/git_commands/commit.go +++ b/pkg/commands/git_commands/commit.go @@ -275,7 +275,10 @@ func (self *CommitCommands) Revert(hashes []string, isMerge bool) error { Arg(hashes...). ToArgv() - return self.cmd.New(cmdArgs).Run() + return abortSequenceAfterLockError(self.cmd.New(cmdArgs), self.repoPaths.WorktreeGitDirPath(), len(hashes), + func() *oscommands.CmdObj { + return self.cmd.New(NewGitCmd("revert").Arg("--abort").ToArgv()) + }).Run() } // CreateFixupCommit creates a commit that fixes up a previous commit diff --git a/pkg/commands/git_commands/lock_error_recovery.go b/pkg/commands/git_commands/lock_error_recovery.go index 8925a65a4..c0f1243dc 100644 --- a/pkg/commands/git_commands/lock_error_recovery.go +++ b/pkg/commands/git_commands/lock_error_recovery.go @@ -49,6 +49,45 @@ func resumeRebaseAfterLockError( return recoverFrom(cmdObj, rebaseState(gitDir)) } +// abortSequenceAfterLockError sets the lock-error recovery (see +// CmdObj.RecoverFromLockErrorWith) of cmdObj, a cherry-pick or revert of +// numCommits commits in the worktree whose git dir is gitDir. +// +// If a lock stops a cherry-pick or revert of several commits after it has +// applied some of them, git leaves the operation in progress in +// .git/sequencer. Unlike a rebase, it doesn't reschedule the commit that +// failed, and continuing the operation would skip that commit. So we abort the +// operation with the command made by newAbortCmdObj, and run the command +// again. The runner calls the recovery after its last attempt too, so we +// also abort when it gives up, and the user isn't left with an operation that +// stopped halfway. +func abortSequenceAfterLockError( + cmdObj *oscommands.CmdObj, + gitDir string, + numCommits int, + newAbortCmdObj func() *oscommands.CmdObj, +) *oscommands.CmdObj { + // For a single commit, git doesn't use .git/sequencer. If a lock stops it, + // it has either not changed anything yet, or it has staged the changes, and + // running it again fails with an error. Git also doesn't check whether + // another operation is in progress in .git/sequencer, so we must not abort + // that one. + if numCommits < 2 { + return cmdObj + } + + return cmdObj.RecoverFromLockErrorWith(func() *oscommands.CmdObj { + // Git refuses to start a cherry-pick or revert of several commits while + // one is in progress, so if there is one now, our command started it. + if dirExists(filepath.Join(gitDir, "sequencer")) { + if err := newAbortCmdObj().Run(); err != nil { + return nil + } + } + return cmdObj + }) +} + // 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. diff --git a/pkg/commands/git_commands/lock_error_recovery_test.go b/pkg/commands/git_commands/lock_error_recovery_test.go index 1fe967983..e48d55a26 100644 --- a/pkg/commands/git_commands/lock_error_recovery_test.go +++ b/pkg/commands/git_commands/lock_error_recovery_test.go @@ -1,6 +1,7 @@ package git_commands import ( + "errors" "os" "path/filepath" "testing" @@ -106,3 +107,71 @@ func TestResumeRebaseAfterLockError(t *testing.T) { })) }) } + +func TestAbortSequenceAfterLockError(t *testing.T) { + scenarios := []struct { + testName string + numCommits int + sequencerExists bool + runner *oscommands.FakeCmdObjRunner + expectRecovery bool + expectRunningAgain bool + }{ + { + testName: "single commit", + numCommits: 1, + runner: oscommands.NewFakeRunner(t), + expectRecovery: false, + }, + { + testName: "the lock stopped the command before it started", + numCommits: 2, + runner: oscommands.NewFakeRunner(t), + expectRecovery: true, + expectRunningAgain: true, + }, + { + testName: "the lock stopped the command partway through", + numCommits: 2, + sequencerExists: true, + runner: oscommands.NewFakeRunner(t). + ExpectGitArgs([]string{"cherry-pick", "--abort"}, "", nil), + expectRecovery: true, + expectRunningAgain: true, + }, + { + testName: "aborting fails", + numCommits: 2, + sequencerExists: true, + runner: oscommands.NewFakeRunner(t). + ExpectGitArgs([]string{"cherry-pick", "--abort"}, "", errors.New("error")), + expectRecovery: true, + expectRunningAgain: false, + }, + } + + for _, s := range scenarios { + t.Run(s.testName, func(t *testing.T) { + gitDir := t.TempDir() + if s.sequencerExists { + assert.NoError(t, os.MkdirAll(filepath.Join(gitDir, "sequencer"), 0o755)) + } + builder := oscommands.NewDummyCmdObjBuilder(s.runner) + cmdObj := builder.New([]string{"git", "cherry-pick", "1234567", "2345678"}) + + abortSequenceAfterLockError(cmdObj, gitDir, s.numCommits, func() *oscommands.CmdObj { + return builder.New([]string{"git", "cherry-pick", "--abort"}) + }) + + recovery := cmdObj.GetLockErrorRecovery() + if !s.expectRecovery { + assert.Nil(t, recovery) + } else if s.expectRunningAgain { + assert.Same(t, cmdObj, recovery()) + } else { + assert.Nil(t, recovery()) + } + s.runner.CheckForMissingCalls() + }) + } +} diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index 6098f9493..b60d30a94 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -598,7 +598,10 @@ func (self *RebaseCommands) CherryPickCommits(commits []*models.Commit) error { Arg(lo.Reverse(lo.Map(commits, func(c *models.Commit, _ int) string { return c.Hash() }))...). ToArgv() - return self.cmd.New(cmdArgs).Run() + return abortSequenceAfterLockError(self.cmd.New(cmdArgs), self.repoPaths.WorktreeGitDirPath(), len(commits), + func() *oscommands.CmdObj { + return self.cmd.New(NewGitCmd("cherry-pick").Arg("--abort").ToArgv()) + }).Run() } func (self *RebaseCommands) DropMergeCommit(commits []*models.Commit, commitIndex int) error { diff --git a/pkg/integration/tests/cherry_pick/lock_error_partway_through_cherry_pick.go b/pkg/integration/tests/cherry_pick/lock_error_partway_through_cherry_pick.go index b3329ab9a..d1ae75715 100644 --- a/pkg/integration/tests/cherry_pick/lock_error_partway_through_cherry_pick.go +++ b/pkg/integration/tests/cherry_pick/lock_error_partway_through_cherry_pick.go @@ -63,25 +63,10 @@ var LockErrorPartwayThroughCherryPick = NewIntegrationTest(NewIntegrationTestArg Content(Contains("Are you sure you want to cherry-pick the 2 copied commit(s) onto this branch?")). Confirm() }). - /* EXPECTED: Lines( Contains("two"), Contains("one"), Contains("base"), ) - ACTUAL: */ - Tap(func() { - t.ExpectPopup().Alert(). - Title(Equals("Error")). - Content(Contains("cherry-pick is already in progress")). - Confirm() - }). - Lines( - Contains("─── Pending cherry-picks"), - Contains("pick").Contains("two"), - Contains("─── Commits"), - Contains("one"), - Contains("base"), - ) }, }) From 4c18daf933b8c8ff2bec4ed6bc37c5a1db89a501 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 7 Nov 2026 10:19:50 +0100 Subject: [PATCH 12/15] Add a test for a lock error partway through skipping in a rebase If a lock stops "git rebase --skip" after git has applied the changes of the next commit but before it has committed them, git doesn't reschedule that commit. The changes stay staged. The git command runner then runs "git rebase --skip" again, and this discards the staged changes, so the commit is silently lost. Co-Authored-By: Claude Opus 5.5 --- .../lock_error_partway_through_skip.go | 76 +++++++++++++++++++ pkg/integration/tests/shared/index_lock.go | 12 +++ pkg/integration/tests/test_list.go | 1 + 3 files changed, 89 insertions(+) create mode 100644 pkg/integration/tests/interactive_rebase/lock_error_partway_through_skip.go diff --git a/pkg/integration/tests/interactive_rebase/lock_error_partway_through_skip.go b/pkg/integration/tests/interactive_rebase/lock_error_partway_through_skip.go new file mode 100644 index 000000000..c2865aff1 --- /dev/null +++ b/pkg/integration/tests/interactive_rebase/lock_error_partway_through_skip.go @@ -0,0 +1,76 @@ +package interactive_rebase + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" + "github.com/jesseduffield/lazygit/pkg/integration/tests/shared" +) + +var LockErrorPartwayThroughSkip = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Skip in a rebase while another git process takes index.lock before the next commit is committed", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell. + CreateFileAndAdd("file", "original\n"). + Commit("base"). + UpdateFileAndAdd("file", "changed\n"). + Commit("commit to edit"). + CreateFileAndAdd("file-y", "y\n"). + Commit("add file-y"). + CreateFileAndAdd("file-z", "z\n"). + Commit("add file-z") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("add file-z").IsSelected(), + Contains("add file-y"), + Contains("commit to edit"), + Contains("base"), + ). + NavigateToLine(Contains("commit to edit")). + Press(keys.Universal.Edit). + Lines( + Contains("─── Pending rebase todos"), + Contains("pick").Contains("add file-z"), + Contains("pick").Contains("add file-y"), + Contains("─── Commits"), + Contains("commit to edit").IsSelected(), + Contains("base"), + ) + + // After amending the commit we stopped at, git has to make a new commit + // when it picks "add file-y", rather than fast-forwarding to it. The + // lock then stops it after it has applied the changes, but before it + // has committed them. + t.Shell().RunCommand([]string{"git", "commit", "--amend", "-m", "edited commit"}) + shared.HoldIndexLockBeforeNextPickIsCommitted(t.Shell()) + + t.GlobalPress(keys.Universal.CreateRebaseOptionsMenu) + t.ExpectPopup().Menu(). + Title(Equals("Rebase options")). + Select(Contains("skip")). + Confirm() + + /* EXPECTED: + t.ExpectPopup().Alert(). + Title(Equals("Error")). + Content(Contains("index.lock': File exists.")). + Confirm() + + t.Views().Files(). + Lines( + Contains("A").Contains("file-y"), + ) + ACTUAL: */ + t.Views().Commits(). + Lines( + Contains("add file-z"), + Contains("edited commit"), + Contains("base"), + ) + }, +}) diff --git a/pkg/integration/tests/shared/index_lock.go b/pkg/integration/tests/shared/index_lock.go index 29e513f6f..c8414813f 100644 --- a/pkg/integration/tests/shared/index_lock.go +++ b/pkg/integration/tests/shared/index_lock.go @@ -30,3 +30,15 @@ func HoldIndexLockAfterNextCommit(shell *Shell) { shell.CreateFile(".git/hooks/post-commit", "#!/bin/sh\n"+holdIndexLockScript) shell.MakeExecutable(".git/hooks/post-commit") } + +// HoldIndexLockBeforeNextPickIsCommitted installs a reference-transaction hook +// that takes index.lock (see holdIndexLockScript) the next time a rebase or +// cherry-pick has applied the changes of a commit, but hasn't committed them +// yet. This is when git creates CHERRY_PICK_HEAD. +func HoldIndexLockBeforeNextPickIsCommitted(shell *Shell) { + shell.CreateFile(".git/hooks/reference-transaction", `#!/bin/sh +[ "$1" = committed ] || exit 0 +awk '$3 == "CHERRY_PICK_HEAD" && $2 !~ /^0+$/ { found = 1 } END { exit !found }' || exit 0 +`+holdIndexLockScript) + shell.MakeExecutable(".git/hooks/reference-transaction") +} diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 21aec9a55..9dc3e4ff4 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -362,6 +362,7 @@ var tests = []*components.IntegrationTest{ interactive_rebase.InteractiveRebaseOfCopiedBranch, interactive_rebase.InteractiveRebaseWithConflictForEditCommand, interactive_rebase.LockErrorPartwayThroughRebase, + interactive_rebase.LockErrorPartwayThroughSkip, interactive_rebase.MidRebaseRangeSelect, interactive_rebase.Move, interactive_rebase.MoveAcrossBranchBoundaryOutsideRebase, From fcfe2e309f40db8ac4882174660092227e280ea9 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 7 Nov 2026 10:21:53 +0100 Subject: [PATCH 13/15] 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 --- .../git_commands/lock_error_recovery.go | 25 +++++++++ .../git_commands/lock_error_recovery_test.go | 52 +++++++++++++++++++ pkg/commands/git_commands/rebase.go | 17 ++++-- .../lock_error_partway_through_skip.go | 8 --- 4 files changed, 91 insertions(+), 11 deletions(-) diff --git a/pkg/commands/git_commands/lock_error_recovery.go b/pkg/commands/git_commands/lock_error_recovery.go index c0f1243dc..7a0834f95 100644 --- a/pkg/commands/git_commands/lock_error_recovery.go +++ b/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. diff --git a/pkg/commands/git_commands/lock_error_recovery_test.go b/pkg/commands/git_commands/lock_error_recovery_test.go index e48d55a26..3b3eff1ba 100644 --- a/pkg/commands/git_commands/lock_error_recovery_test.go +++ b/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()()) + }) +} diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index b60d30a94..e166687a9 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/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(), diff --git a/pkg/integration/tests/interactive_rebase/lock_error_partway_through_skip.go b/pkg/integration/tests/interactive_rebase/lock_error_partway_through_skip.go index c2865aff1..992225336 100644 --- a/pkg/integration/tests/interactive_rebase/lock_error_partway_through_skip.go +++ b/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"), - ) }, }) From de10b170c95d5e24b839848c4d7eeb5c4457fe40 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 7 Nov 2026 11:13:24 +0100 Subject: [PATCH 14/15] Add a test for a stopped rebase being shown as a cherry-pick If git can't take the index lock to commit the changes of a pick during a rebase, it exits without recording that the rebase stopped. It leaves CHERRY_PICK_HEAD behind, but no rebase-merge/stopped-sha. Lazygit only recognizes CHERRY_PICK_HEAD as part of a rebase if stopped-sha matches it, so it shows the rebase as a cherry-pick. Its "Cherry-pick options" then run cherry-pick commands during the rebase. Continuing happens to work, but takes two steps. Skipping or aborting discards the changes of the pick, and continuing the rebase afterwards loses that commit. Co-Authored-By: Claude Opus 5.5 --- .../lock_error_partway_through_skip.go | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/pkg/integration/tests/interactive_rebase/lock_error_partway_through_skip.go b/pkg/integration/tests/interactive_rebase/lock_error_partway_through_skip.go index 992225336..c1d0bfdd3 100644 --- a/pkg/integration/tests/interactive_rebase/lock_error_partway_through_skip.go +++ b/pkg/integration/tests/interactive_rebase/lock_error_partway_through_skip.go @@ -64,5 +64,29 @@ var LockErrorPartwayThroughSkip = NewIntegrationTest(NewIntegrationTestArgs{ Lines( Contains("A").Contains("file-y"), ) + + t.Views().Status().Content(Contains("(cherry-picking)")) + + t.GlobalPress(keys.Universal.CreateRebaseOptionsMenu) + t.ExpectPopup().Menu(). + Title(Equals("Cherry-pick options")). + Select(Contains("continue")). + Confirm() + + t.Views().Status().Content(Contains("(rebasing)")) + + t.GlobalPress(keys.Universal.CreateRebaseOptionsMenu) + t.ExpectPopup().Menu(). + Title(Equals("Rebase options")). + Select(Contains("continue")). + Confirm() + + t.Views().Commits(). + Lines( + Contains("add file-z"), + Contains("add file-y"), + Contains("edited commit"), + Contains("base"), + ) }, }) From 4d56db828caff35dda545844f015a7d98b2ed92c Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 7 Nov 2026 11:16:16 +0100 Subject: [PATCH 15/15] Recognize a rebase that couldn't commit a pick, and continue it If git can't take the index lock to commit the changes of a pick during a rebase, it exits without writing rebase-merge/stopped-sha. Lazygit then showed the rebase as a cherry-pick, because it only recognized CHERRY_PICK_HEAD as part of a rebase if stopped-sha matched it. Also accept CHERRY_PICK_HEAD as part of the rebase if it matches the last todo in rebase-merge/done, the pick that git was executing. With this, continuing runs "git rebase --continue". In this state it refuses to commit the staged changes of the pick, because git didn't write the message file that it writes when a pick conflicts. So commit them first with "git cherry-pick --continue". This uses the author and message of the pick, like the rebase would have. Co-Authored-By: Claude Opus 5.5 --- pkg/commands/git_commands/rebase.go | 19 +++++ pkg/commands/git_commands/status.go | 61 +++++++++----- pkg/commands/git_commands/status_test.go | 80 +++++++++++++++++++ .../lock_error_partway_through_skip.go | 8 -- 4 files changed, 140 insertions(+), 28 deletions(-) diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index e166687a9..c5e289e49 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -501,6 +501,12 @@ func (self *RebaseCommands) GenericMergeOrRebaseAction(commandType string, comma } } + if commandType == "rebase" && command == "continue" { + if err := self.commitPickThatRebaseStoppedAt(); err != nil { + return err + } + } + cmdObj := skipEditor(self.GenericMergeOrRebaseActionCmdObj(commandType, command)) if command == "continue" || command == "skip" { switch commandType { @@ -535,6 +541,19 @@ func (self *RebaseCommands) GenericMergeOrRebaseAction(commandType string, comma return nil } +// commitPickThatRebaseStoppedAt commits the changes of a pick if the rebase +// stopped after it applied them, but before it committed them (see +// rebaseStoppedBeforeCommittingPick). "git rebase --continue" refuses to commit +// them in that case. "git cherry-pick --continue" commits them with the author +// and message of the pick, like the rebase would have. +func (self *RebaseCommands) commitPickThatRebaseStoppedAt() error { + if !rebaseStoppedBeforeCommittingPick(self.repoPaths.WorktreeGitDirPath(), self.config.GetCoreCommentChar()) { + return nil + } + + return skipEditor(self.GenericMergeOrRebaseActionCmdObj("cherry-pick", "continue")).Run() +} + // resumeAfterLockError makes cmdObj, a command that starts or continues a // rebase, resume the rebase if a lock error stops it partway through; see // resumeRebaseAfterLockError. diff --git a/pkg/commands/git_commands/status.go b/pkg/commands/git_commands/status.go index cdec190c4..f6b1fd460 100644 --- a/pkg/commands/git_commands/status.go +++ b/pkg/commands/git_commands/status.go @@ -59,35 +59,56 @@ func (self *StatusCommands) IsInCherryPick() (bool, error) { } func (self *StatusCommands) hasCherryPickHead() (bool, error) { - exists, err := self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "CHERRY_PICK_HEAD")) + gitDir := self.repoPaths.WorktreeGitDirPath() + exists, err := self.os.FileExists(filepath.Join(gitDir, "CHERRY_PICK_HEAD")) if err != nil || !exists { return exists, err } - // Sometimes, CHERRY_PICK_HEAD is present during rebases even if no - // cherry-pick is in progress. I suppose this is because rebase used to be - // implemented as a series of cherry-picks, so this could be remnants of - // code that is shared between cherry-pick and rebase, or something. The way - // to tell if this is the case is to check for the presence of the - // stopped-sha file, which records the sha of the last pick that was - // executed before the rebase stopped, and seeing if the sha in that file is - // the same as the one in CHERRY_PICK_HEAD. - cherryPickHead, err := os.ReadFile(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "CHERRY_PICK_HEAD")) + cherryPickHead, err := os.ReadFile(filepath.Join(gitDir, "CHERRY_PICK_HEAD")) if err != nil { return false, err } - stoppedSha, err := os.ReadFile(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge", "stopped-sha")) + return !cherryPickHeadBelongsToRebase(gitDir, strings.TrimSpace(string(cherryPickHead)), self.config.GetCoreCommentChar()), nil +} + +// cherryPickHeadBelongsToRebase tells whether CHERRY_PICK_HEAD, whose value is +// cherryPickHead, belongs to a pick of the rebase that is in progress in +// gitDir, rather than to a cherry-pick. A rebase writes CHERRY_PICK_HEAD when it +// has applied the changes of a pick, and removes it when it has committed them. +// So it is still there if the rebase stopped because the pick conflicted; git +// records the pick in rebase-merge/stopped-sha then. It is also still there if +// git couldn't commit the changes, for example because it couldn't take the +// index lock. Git exits without writing stopped-sha in that case, but the pick +// is the last line of rebase-merge/done. +func cherryPickHeadBelongsToRebase(gitDir string, cherryPickHead string, commentChar byte) bool { + // The hashes in these files may be abbreviated + isPrefixOfCherryPickHead := func(hash string) bool { + return hash != "" && strings.HasPrefix(cherryPickHead, hash) + } + + stoppedSha, err := os.ReadFile(filepath.Join(gitDir, "rebase-merge", "stopped-sha")) + if err == nil && isPrefixOfCherryPickHead(strings.TrimSpace(string(stoppedSha))) { + return true + } + + doneTodos, err := utils.ReadRebaseTodoFile(filepath.Join(gitDir, "rebase-merge", "done"), commentChar) + return err == nil && len(doneTodos) > 0 && isPrefixOfCherryPickHead(doneTodos[len(doneTodos)-1].Commit) +} + +// rebaseStoppedBeforeCommittingPick tells whether the rebase that is in +// progress in gitDir has applied the changes of a pick, but exited before it +// could commit them (see cherryPickHeadBelongsToRebase). "git rebase +// --continue" refuses to commit the changes in that case, because git didn't +// write the message file that it writes when a pick conflicts. +func rebaseStoppedBeforeCommittingPick(gitDir string, commentChar byte) bool { + cherryPickHead, err := os.ReadFile(filepath.Join(gitDir, "CHERRY_PICK_HEAD")) if err != nil { - // If we get an error we assume the file doesn't exist - return true, nil + return false } - cherryPickHeadStr := strings.TrimSpace(string(cherryPickHead)) - stoppedShaStr := strings.TrimSpace(string(stoppedSha)) - // Need to use HasPrefix here because the cherry-pick HEAD is a full sha1, - // but stopped-sha is an abbreviated sha1 - if strings.HasPrefix(cherryPickHeadStr, stoppedShaStr) { - return false, nil + if _, err := os.Stat(filepath.Join(gitDir, "rebase-merge", "message")); err == nil { + return false } - return true, nil + return cherryPickHeadBelongsToRebase(gitDir, strings.TrimSpace(string(cherryPickHead)), commentChar) } func (self *StatusCommands) IsInRevert() (bool, error) { diff --git a/pkg/commands/git_commands/status_test.go b/pkg/commands/git_commands/status_test.go index 0ec71123a..45987f912 100644 --- a/pkg/commands/git_commands/status_test.go +++ b/pkg/commands/git_commands/status_test.go @@ -113,6 +113,31 @@ func TestStatusWorkingTreeStateOfCherryPickOrRevert(t *testing.T) { files: map[string]string{"REVERT_HEAD": "1234567\n"}, expectedReverting: true, }, + { + testName: "CHERRY_PICK_HEAD of a rebase that stopped because of conflicts", + files: map[string]string{ + "CHERRY_PICK_HEAD": "2345678abcdef\n", + "rebase-merge/stopped-sha": "2345678\n", + "rebase-merge/done": "pick 1234567abcdef one\npick 2345678abcdef two\n", + }, + expectedCherryPicking: false, + }, + { + testName: "CHERRY_PICK_HEAD of a rebase that couldn't commit a pick", + files: map[string]string{ + "CHERRY_PICK_HEAD": "2345678abcdef\n", + "rebase-merge/done": "pick 1234567abcdef one\npick 2345678abcdef two\n", + }, + expectedCherryPicking: false, + }, + { + testName: "cherry-pick while a rebase is stopped", + files: map[string]string{ + "CHERRY_PICK_HEAD": "3456789abcdef\n", + "rebase-merge/done": "pick 1234567abcdef one\nedit 2345678abcdef two\n", + }, + expectedCherryPicking: true, + }, { testName: "cherry-pick of several commits that stopped before applying one", files: map[string]string{"sequencer/todo": "pick 1234567 one\npick 2345678 two\n"}, @@ -142,3 +167,58 @@ func TestStatusWorkingTreeStateOfCherryPickOrRevert(t *testing.T) { }) } } + +func TestRebaseStoppedBeforeCommittingPick(t *testing.T) { + scenarios := []struct { + testName string + files map[string]string + expected bool + }{ + { + testName: "the rebase couldn't commit a pick", + files: map[string]string{ + "CHERRY_PICK_HEAD": "2345678abcdef\n", + "rebase-merge/done": "pick 1234567abcdef one\npick 2345678abcdef two\n", + }, + expected: true, + }, + { + testName: "the rebase stopped because a pick conflicted", + files: map[string]string{ + "CHERRY_PICK_HEAD": "2345678abcdef\n", + "rebase-merge/stopped-sha": "2345678\n", + "rebase-merge/message": "two\n", + "rebase-merge/done": "pick 1234567abcdef one\npick 2345678abcdef two\n", + }, + expected: false, + }, + { + testName: "no CHERRY_PICK_HEAD", + files: map[string]string{ + "rebase-merge/done": "pick 1234567abcdef one\nedit 2345678abcdef two\n", + }, + expected: false, + }, + { + testName: "CHERRY_PICK_HEAD of a cherry-pick while a rebase is stopped", + files: map[string]string{ + "CHERRY_PICK_HEAD": "3456789abcdef\n", + "rebase-merge/done": "pick 1234567abcdef one\nedit 2345678abcdef two\n", + }, + expected: false, + }, + } + + for _, s := range scenarios { + t.Run(s.testName, func(t *testing.T) { + gitDir := t.TempDir() + for path, content := range s.files { + fullPath := filepath.Join(gitDir, path) + assert.NoError(t, os.MkdirAll(filepath.Dir(fullPath), 0o755)) + assert.NoError(t, os.WriteFile(fullPath, []byte(content), 0o644)) + } + + assert.Equal(t, s.expected, rebaseStoppedBeforeCommittingPick(gitDir, '#')) + }) + } +} diff --git a/pkg/integration/tests/interactive_rebase/lock_error_partway_through_skip.go b/pkg/integration/tests/interactive_rebase/lock_error_partway_through_skip.go index c1d0bfdd3..f78bd9300 100644 --- a/pkg/integration/tests/interactive_rebase/lock_error_partway_through_skip.go +++ b/pkg/integration/tests/interactive_rebase/lock_error_partway_through_skip.go @@ -65,14 +65,6 @@ var LockErrorPartwayThroughSkip = NewIntegrationTest(NewIntegrationTestArgs{ Contains("A").Contains("file-y"), ) - t.Views().Status().Content(Contains("(cherry-picking)")) - - t.GlobalPress(keys.Universal.CreateRebaseOptionsMenu) - t.ExpectPopup().Menu(). - Title(Equals("Cherry-pick options")). - Select(Contains("continue")). - Confirm() - t.Views().Status().Content(Contains("(rebasing)")) t.GlobalPress(keys.Universal.CreateRebaseOptionsMenu)