Unify the git command lock-retry loops

RunWithOutput and RunWithOutputs each carried their own near-identical
copy of the index.lock retry loop. Extract the loop into a single
retryOnLockError helper so the retry policy lives in one place, ahead of
changing that policy. Behavior is unchanged; the added tests characterize
it (success and non-lock errors run once, a lock error in the output is
retried).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

pkg/commands/git_cmd_obj_runner.go

@@ -33,11 +33,30 @@ 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.innerRunner.RunWithOutput(cmdObj.Clone())
+	})
+}
+
+func (self *gitCmdObjRunner) RunWithOutputs(cmdObj *oscommands.CmdObj) (string, string, error) {
+	var stdout, stderr string
+	_, err := self.retryOnLockError(func() (string, error) {
+		var runErr error
+		stdout, stderr, runErr = self.innerRunner.RunWithOutputs(cmdObj.Clone())
+		return stdout + stderr, runErr
+	})
+	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
+// 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) {
 	var output string
 	var err error
 	for range RetryCount {
-		newCmdObj := cmdObj.Clone()
-		output, err = self.innerRunner.RunWithOutput(newCmdObj)
+		output, err = run()
 
 		if err == nil || !isRetryableError(output) {
 			return output, err
@@ -51,25 +70,6 @@ func (self *gitCmdObjRunner) RunWithOutput(cmdObj *oscommands.CmdObj) (string, e
 	return output, err
 }
 
-func (self *gitCmdObjRunner) RunWithOutputs(cmdObj *oscommands.CmdObj) (string, string, error) {
-	var stdout, stderr string
-	var err error
-	for range RetryCount {
-		newCmdObj := cmdObj.Clone()
-		stdout, stderr, err = self.innerRunner.RunWithOutputs(newCmdObj)
-
-		if err == nil || !isRetryableError(stdout+stderr) {
-			return stdout, stderr, err
-		}
-
-		// if we have an error based on a lock, we should wait a bit and then retry
-		self.log.Warn("lock error prevented command from running. Retrying command after a small wait")
-		time.Sleep(WaitTime)
-	}
-
-	return stdout, stderr, err
-}
-
 // Retry logic not implemented here, but these commands typically don't need to obtain a lock.
 func (self *gitCmdObjRunner) RunAndProcessLines(cmdObj *oscommands.CmdObj, onLine func(line string) (bool, error)) error {
 	return self.innerRunner.RunAndProcessLines(cmdObj, onLine)

pkg/commands/git_cmd_obj_runner_test.go

@@ -0,0 +1,92 @@
+package commands
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
+	"github.com/jesseduffield/lazygit/pkg/utils"
+	"github.com/stretchr/testify/assert"
+)
+
+type runnerResult struct {
+	output string
+	err    error
+}
+
+// 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.
+type scriptedRunner struct {
+	results []runnerResult
+	calls   int
+}
+
+func (self *scriptedRunner) next() (string, error) {
+	result := self.results[self.calls]
+	self.calls++
+	return result.output, result.err
+}
+
+func (self *scriptedRunner) Run(*oscommands.CmdObj) error {
+	_, err := self.next()
+	return err
+}
+
+func (self *scriptedRunner) RunWithOutput(*oscommands.CmdObj) (string, error) {
+	return self.next()
+}
+
+func (self *scriptedRunner) RunWithOutputs(*oscommands.CmdObj) (string, string, error) {
+	output, err := self.next()
+	return output, "", err
+}
+
+func (self *scriptedRunner) RunAndProcessLines(*oscommands.CmdObj, func(string) (bool, error)) error {
+	panic("not implemented")
+}
+
+func newTestRunner(inner *scriptedRunner) *gitCmdObjRunner {
+	return &gitCmdObjRunner{
+		log:         utils.NewDummyLog(),
+		innerRunner: inner,
+	}
+}
+
+// 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"})
+}
+
+func TestRunWithOutputReturnsSuccessWithoutRetrying(t *testing.T) {
+	inner := &scriptedRunner{results: []runnerResult{{output: "done", err: nil}}}
+
+	output, err := newTestRunner(inner).RunWithOutput(dummyCmdObj())
+
+	assert.NoError(t, err)
+	assert.Equal(t, "done", output)
+	assert.Equal(t, 1, inner.calls)
+}
+
+func TestRunWithOutputDoesNotRetryNonLockError(t *testing.T) {
+	inner := &scriptedRunner{results: []runnerResult{{output: "boom", err: errors.New("boom")}}}
+
+	_, err := newTestRunner(inner).RunWithOutput(dummyCmdObj())
+
+	assert.Error(t, err)
+	assert.Equal(t, 1, inner.calls)
+}
+
+func TestRunWithOutputRetriesWhenLockErrorIsInOutput(t *testing.T) {
+	inner := &scriptedRunner{results: []runnerResult{
+		{output: "fatal: Unable to create '/repo/.git/index.lock': File exists.", err: errors.New("exit status 128")},
+		{output: "done", err: nil},
+	}}
+
+	output, err := newTestRunner(inner).RunWithOutput(dummyCmdObj())
+
+	assert.NoError(t, err)
+	assert.Equal(t, "done", output)
+	assert.Equal(t, 2, inner.calls)
+}