From d86a49ba3fc05bf20ec4a7e5f07bf3bcef0508d2 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 1 Jun 2026 09:18:20 +0200 Subject: [PATCH 01/11] When RecordCurrentDirectory fails, only log the error If we return the error here, we don't switch repos, but the chdir happened already, so this would be an inconsistent state (a lot of lazygit's code assumes that the current directory is always the worktree root). Only log the error; failing to record the current directory is not the end of the world. Also, it is very unlikely to happen; RecordCurrentDirectory only writes to a small file, and if this fails, then either there is filesystem corruption of the disk is full, and in both cases the user likely has much bigger problems. --- pkg/gui/controllers/helpers/repos_helper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/gui/controllers/helpers/repos_helper.go b/pkg/gui/controllers/helpers/repos_helper.go index 156c0ab30..4da15fa13 100644 --- a/pkg/gui/controllers/helpers/repos_helper.go +++ b/pkg/gui/controllers/helpers/repos_helper.go @@ -171,7 +171,7 @@ func (self *ReposHelper) DispatchSwitchTo(path string, errMsg string, contextKey } if err := self.recordDirectoryHelper.RecordCurrentDirectory(); err != nil { - return err + self.c.Log.Errorf("error recording current directory: %v", err) } self.c.Mutexes().RefreshingFilesMutex.Lock() From 685dfc87a4093d1aef05e366edb385f5be418e3d Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 28 May 2026 20:07:51 +0200 Subject: [PATCH 02/11] Cleanup: drop unneeded variable --- pkg/app/app.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/app/app.go b/pkg/app/app.go index 09b2236db..96dff31f6 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -262,7 +262,7 @@ func (app *App) setupRepo( os.Exit(0) } - if didOpenRepo := openRecentRepo(app); didOpenRepo { + if openRecentRepo(app) { return true, nil } From 26015561892aa43ae20fb3a99eae066dda883a9b Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 28 May 2026 09:53:30 +0200 Subject: [PATCH 03/11] Dedupe the recent-repos fallback in setupRepo The for-loop here was a verbatim copy of openRecentRepo, so call that instead. --- pkg/app/app.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/pkg/app/app.go b/pkg/app/app.go index 96dff31f6..9af0c46d7 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -239,12 +239,8 @@ func (app *App) setupRepo( } // check if we have a recent repo we can open - for _, repoDir := range app.Config.GetAppState().RecentRepos { - if isRepo, _ := isDirectoryAGitRepository(repoDir); isRepo { - if err := os.Chdir(repoDir); err == nil { - return true, nil - } - } + if openRecentRepo(app) { + return true, nil } fmt.Fprintln(os.Stderr, app.Tr.NoRecentRepositories) From bb8955f2de2db03abb964b2feeb1278590a6d02b Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 28 May 2026 10:07:00 +0200 Subject: [PATCH 04/11] Load direnv environment when switching repos MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a user opens a repo from the recent-repos menu or jumps between worktrees inside lazygit, only the env vars present at process startup reach subprocesses. That breaks pre-commit hooks and other tools whose dependencies are pulled in by a per-repo .envrc — users were left with read-only operations because the env their shell would normally load via direnv never made it into lazygit's git invocations. Shell out to `direnv export json` after each chdir and apply the JSON delta via os.Setenv/Unsetenv. direnv tracks the previous load in its own DIRENV_DIFF env var, so the delta also unloads vars from the old repo when entering one without a matching .envrc. If direnv isn't on PATH the call is a no-op, so users who don't use direnv pay nothing and users who do need no config to opt in. Any stderr direnv emits (loading messages, "blocked .envrc" errors, etc.) goes to the command log. The integration test puts a fake direnv on PATH and asserts that a value it exports reaches a custom command after switching repos. Wiring this up needed runner.go to support `{{actualPath}}` placeholders in ExtraEnvVars, mirroring the existing support for ExtraCmdArgs, so the test can prepend a fixture-relative directory to PATH. Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/app/app.go | 10 +++ pkg/commands/direnv/direnv.go | 62 ++++++++++++++++ pkg/commands/direnv/direnv_test.go | 43 ++++++++++++ pkg/gui/controllers/helpers/repos_helper.go | 15 +++- pkg/integration/components/runner.go | 6 +- .../misc/direnv_loaded_on_repo_switch.go | 68 ++++++++++++++++++ .../misc/direnv_unloads_on_blocked_envrc.go | 70 +++++++++++++++++++ pkg/integration/tests/test_list.go | 2 + 8 files changed, 274 insertions(+), 2 deletions(-) create mode 100644 pkg/commands/direnv/direnv.go create mode 100644 pkg/commands/direnv/direnv_test.go create mode 100644 pkg/integration/tests/misc/direnv_loaded_on_repo_switch.go create mode 100644 pkg/integration/tests/misc/direnv_unloads_on_blocked_envrc.go diff --git a/pkg/app/app.go b/pkg/app/app.go index 9af0c46d7..1f49a6a23 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -14,6 +14,7 @@ import ( "github.com/spf13/afero" appTypes "github.com/jesseduffield/lazygit/pkg/app/types" + "github.com/jesseduffield/lazygit/pkg/commands/direnv" "github.com/jesseduffield/lazygit/pkg/commands/git_commands" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/common" @@ -171,6 +172,15 @@ func openRecentRepo(app *App) bool { for _, repoDir := range app.Config.GetAppState().RecentRepos { if isRepo, _ := isDirectoryAGitRepository(repoDir); isRepo { if err := os.Chdir(repoDir); err == nil { + // The command log isn't up yet, so any direnv diagnostics + // only make it to the debug log here. + msg, derr := direnv.Load(app.OSCommand.Cmd) + if msg != "" { + app.Log.WithField("message", msg).Info("direnv") + } + if derr != nil { + app.Log.WithError(derr).Warn("direnv load failed") + } return true } } diff --git a/pkg/commands/direnv/direnv.go b/pkg/commands/direnv/direnv.go new file mode 100644 index 000000000..9d5d2d1e1 --- /dev/null +++ b/pkg/commands/direnv/direnv.go @@ -0,0 +1,62 @@ +package direnv + +import ( + "bytes" + "encoding/json" + "os" + "os/exec" + "strings" + + "github.com/jesseduffield/lazygit/pkg/commands/oscommands" +) + +// Load runs `direnv export json` for the current working directory and applies +// the resulting env-var delta to the current process. If direnv isn't on PATH, +// it's a no-op — users who don't use direnv pay nothing, and users who do need +// no config to opt in. +// +// direnv prints diagnostics to stderr ("direnv: loading .envrc", "direnv: +// error /path/.envrc is blocked", etc.); whatever it printed is returned in +// message so callers can surface it in their command log. +func Load(cmd oscommands.ICmdObjBuilder) (message string, err error) { + if _, lookupErr := exec.LookPath("direnv"); lookupErr != nil { + return "", nil + } + + stdout, stderr, runErr := cmd.New([]string{ + "direnv", "export", "json", + }).DontLog().RunWithOutputs() + message = strings.TrimRight(stderr, "\n") + + // Apply whatever delta direnv produced even if it exited non-zero. + // When the new dir's .envrc is blocked, direnv still emits a valid + // JSON delta on stdout that unloads vars from the previous dir; + // without applying it the old env would leak into the new repo. + delta, parseErr := parseDirenvExport([]byte(stdout)) + for k, v := range delta { + if v == nil { + _ = os.Unsetenv(k) + } else { + _ = os.Setenv(k, *v) + } + } + + // Prefer the runtime error (whose Error() text is direnv's stderr) + // over a parse error, since it's the more actionable signal. + if runErr != nil { + return message, runErr + } + return message, parseErr +} + +func parseDirenvExport(stdout []byte) (map[string]*string, error) { + trimmed := bytes.TrimSpace(stdout) + if len(trimmed) == 0 || bytes.Equal(trimmed, []byte("null")) { + return nil, nil + } + var delta map[string]*string + if err := json.Unmarshal(trimmed, &delta); err != nil { + return nil, err + } + return delta, nil +} diff --git a/pkg/commands/direnv/direnv_test.go b/pkg/commands/direnv/direnv_test.go new file mode 100644 index 000000000..69b102d4d --- /dev/null +++ b/pkg/commands/direnv/direnv_test.go @@ -0,0 +1,43 @@ +package direnv + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestParseDirenvExport(t *testing.T) { + hello := "hello" + empty := "" + + scenarios := []struct { + name string + input string + want map[string]*string + wantErr bool + }{ + {name: "empty stdout means no .envrc was loaded", input: "", want: nil}, + {name: "literal null from direnv means no delta", input: "null", want: nil}, + {name: "empty object means no delta", input: "{}", want: map[string]*string{}}, + {name: "string value is a set", input: `{"FOO":"hello"}`, want: map[string]*string{"FOO": &hello}}, + {name: "null value is an unset", input: `{"FOO":null}`, want: map[string]*string{"FOO": nil}}, + { + name: "set and unset can coexist", + input: `{"FOO":"hello","BAR":null,"BAZ":""}`, + want: map[string]*string{"FOO": &hello, "BAR": nil, "BAZ": &empty}, + }, + {name: "malformed JSON is an error", input: `{not json`, wantErr: true}, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + got, err := parseDirenvExport([]byte(s.input)) + if s.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, s.want, got) + } + }) + } +} diff --git a/pkg/gui/controllers/helpers/repos_helper.go b/pkg/gui/controllers/helpers/repos_helper.go index 4da15fa13..f8972b837 100644 --- a/pkg/gui/controllers/helpers/repos_helper.go +++ b/pkg/gui/controllers/helpers/repos_helper.go @@ -10,6 +10,7 @@ import ( appTypes "github.com/jesseduffield/lazygit/pkg/app/types" "github.com/jesseduffield/lazygit/pkg/commands" + "github.com/jesseduffield/lazygit/pkg/commands/direnv" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/env" "github.com/jesseduffield/lazygit/pkg/gocui" @@ -170,6 +171,14 @@ func (self *ReposHelper) DispatchSwitchTo(path string, errMsg string, contextKey return err } + direnvMsg, direnvErr := direnv.Load(self.c.OS().Cmd) + if direnvMsg != "" { + self.c.LogCommand(direnvMsg, false) + } + if direnvErr != nil { + self.c.Log.WithError(direnvErr).Warn("direnv load failed") + } + if err := self.recordDirectoryHelper.RecordCurrentDirectory(); err != nil { self.c.Log.Errorf("error recording current directory: %v", err) } @@ -177,6 +186,10 @@ func (self *ReposHelper) DispatchSwitchTo(path string, errMsg string, contextKey self.c.Mutexes().RefreshingFilesMutex.Lock() defer self.c.Mutexes().RefreshingFilesMutex.Unlock() - return self.onNewRepo(appTypes.StartArgs{}, contextKey) + if err := self.onNewRepo(appTypes.StartArgs{}, contextKey); err != nil { + return err + } + + return direnvErr }) } diff --git a/pkg/integration/components/runner.go b/pkg/integration/components/runner.go index 83ddfe66d..5640c3e70 100644 --- a/pkg/integration/components/runner.go +++ b/pkg/integration/components/runner.go @@ -246,7 +246,11 @@ func getLazygitCommand( cmdObj.AddEnvVars(fmt.Sprintf("GORACE=log_path=%s", raceDetectorLogsPath())) if test.ExtraEnvVars() != nil { for key, value := range test.ExtraEnvVars() { - cmdObj.AddEnvVars(fmt.Sprintf("%s=%s", key, value)) + resolvedValue := utils.ResolvePlaceholderString(value, map[string]string{ + "actualPath": paths.Actual(), + "actualRepoPath": paths.ActualRepo(), + }) + cmdObj.AddEnvVars(fmt.Sprintf("%s=%s", key, resolvedValue)) } } diff --git a/pkg/integration/tests/misc/direnv_loaded_on_repo_switch.go b/pkg/integration/tests/misc/direnv_loaded_on_repo_switch.go new file mode 100644 index 000000000..14470da88 --- /dev/null +++ b/pkg/integration/tests/misc/direnv_loaded_on_repo_switch.go @@ -0,0 +1,68 @@ +package misc + +import ( + "os" + "path/filepath" + + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +// Verifies that when the user switches repos from inside lazygit, env vars +// that direnv would load for the target repo are applied to subprocesses +// (custom commands, git hooks, etc.). The test puts a fake `direnv` binary +// on PATH so it works regardless of whether the host has real direnv +// installed. +var DirenvLoadedOnRepoSwitch = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Switching repos applies direnv-loaded env vars to subprocesses", + ExtraCmdArgs: []string{}, + ExtraEnvVars: map[string]string{ + // Prepend a dir under the test fixture to PATH so our fake direnv + // wins lookup. The placeholder is resolved at run time. + "PATH": "{{actualPath}}/bin:" + os.Getenv("PATH"), + }, + SetupConfig: func(cfg *config.AppConfig) { + otherRepo, _ := filepath.Abs("../other") + cfg.GetAppState().RecentRepos = []string{otherRepo} + cfg.GetUserConfig().CustomCommands = []config.CustomCommand{ + { + Key: config.Keybinding{"X"}, + Context: "files", + Command: `echo "VAR=$LG_DIRENV_TEST" > output.txt`, + }, + } + }, + SetupRepo: func(shell *Shell) { + shell.EmptyCommit("initial") + shell.CloneNonBare("other") + + // Fake direnv: echoes a fixed JSON delta on stdout (set + // LG_DIRENV_TEST) and a "loading" line on stderr, exactly as + // real direnv would after authorizing an .envrc. + shell.CreateFile("../bin/direnv", `#!/bin/sh +echo '{"LG_DIRENV_TEST":"from_direnv"}' +echo "direnv: loading .envrc" >&2 +`) + shell.MakeExecutable("../bin/direnv") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + // Switch to the "other" repo via the recent-repos menu. + t.GlobalPress(keys.Universal.OpenRecentRepos) + t.ExpectPopup().Menu().Title(Equals("Recent repositories")). + Lines( + Contains("other").IsSelected(), + Contains("Cancel"), + ). + Confirm() + + // Run the custom command; if direnv loading worked, $LG_DIRENV_TEST + // reaches the subprocess and ends up in output.txt. + t.Views().Files(). + Focus(). + Press(config.Keybinding{"X"}). + Lines( + Contains("output.txt").IsSelected(), + ) + t.Views().Main().Content(Contains("VAR=from_direnv")) + }, +}) diff --git a/pkg/integration/tests/misc/direnv_unloads_on_blocked_envrc.go b/pkg/integration/tests/misc/direnv_unloads_on_blocked_envrc.go new file mode 100644 index 000000000..ca7afe104 --- /dev/null +++ b/pkg/integration/tests/misc/direnv_unloads_on_blocked_envrc.go @@ -0,0 +1,70 @@ +package misc + +import ( + "os" + "path/filepath" + + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +// Real direnv exits non-zero when the destination .envrc isn't authorized, +// but it still emits a valid JSON delta on stdout that unloads vars from +// the previously-active .envrc. We have to apply that delta anyway, or the +// previous repo's env leaks into the new one. The fake direnv here mimics +// that behavior; the test also asserts that the user gets an error popup +// (the command log alone is easy to miss). +var DirenvUnloadsOnBlockedEnvrc = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Blocked .envrc unloads the previous repo's env and shows an error popup", + ExtraCmdArgs: []string{}, + ExtraEnvVars: map[string]string{ + "PATH": "{{actualPath}}/bin:" + os.Getenv("PATH"), + // Simulates a var that the previous repo's .envrc would have set. + "LG_DIRENV_TEST": "from_previous_repo", + }, + SetupConfig: func(cfg *config.AppConfig) { + otherRepo, _ := filepath.Abs("../other") + cfg.GetAppState().RecentRepos = []string{otherRepo} + cfg.GetUserConfig().CustomCommands = []config.CustomCommand{ + { + Key: config.Keybinding{"X"}, + Context: "files", + Command: `echo "VAR=[$LG_DIRENV_TEST]" > output.txt`, + }, + } + }, + SetupRepo: func(shell *Shell) { + shell.EmptyCommit("initial") + shell.CloneNonBare("other") + + shell.CreateFile("../bin/direnv", `#!/bin/sh +echo '{"LG_DIRENV_TEST":null}' +echo "direnv: error /repo/.envrc is blocked. Run 'direnv allow' to approve its content" >&2 +exit 1 +`) + shell.MakeExecutable("../bin/direnv") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.GlobalPress(keys.Universal.OpenRecentRepos) + t.ExpectPopup().Menu().Title(Equals("Recent repositories")). + Lines( + Contains("other").IsSelected(), + Contains("Cancel"), + ). + Confirm() + + t.ExpectPopup().Alert(). + Title(Equals("Error")). + Content(Contains("is blocked")). + Confirm() + + // If unload worked, $LG_DIRENV_TEST is empty in the custom command. + t.Views().Files(). + Focus(). + Press(config.Keybinding{"X"}). + Lines( + Contains("output.txt").IsSelected(), + ) + t.Views().Main().Content(Contains("VAR=[]")) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 2bf2837cd..3679099e0 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -334,6 +334,8 @@ var tests = []*components.IntegrationTest{ misc.ConfirmOnQuit, misc.CopyConfirmationMessageToClipboard, misc.CopyToClipboard, + misc.DirenvLoadedOnRepoSwitch, + misc.DirenvUnloadsOnBlockedEnvrc, misc.InitialOpen, misc.RecentReposOnLaunch, patch_building.Apply, From b76c1072ff38a9779fd796a781e79d0c7060dedf Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 1 Jun 2026 11:07:16 +0200 Subject: [PATCH 05/11] Offer direnv .envrc approval from inside lazygit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a user switches into a repo whose .envrc hasn't been approved with `direnv allow`, the previous behavior was to drop a "blocked" error popup and leave the user to fix it externally. That meant opening a terminal, running `direnv allow`, and then either restarting lazygit or switching repos and back to refresh the env — easy to get wrong, easy to forget. When `direnv export json` exits non-zero, follow up with `direnv status --json` to ask direnv whether the current directory has a not-yet- allowed .envrc, and if so, get its path. Then show a confirmation popup with the .envrc contents inline so the user can read what they're approving. Confirming runs `direnv allow ` and re-runs the load so the new env reaches subprocesses immediately; cancelling leaves the env unloaded (the same state as before this commit when direnv refused to load the .envrc). Using `direnv status --json` instead of parsing the "is blocked" stderr line means we rely on direnv's structured output rather than its human-readable error format, which is more stable across versions and avoids assumptions about output formatting. Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/app/app.go | 16 ++-- pkg/commands/direnv/direnv.go | 86 ++++++++++++++++-- pkg/commands/direnv/direnv_test.go | 45 ++++++++++ pkg/gui/controllers/helpers/repos_helper.go | 61 +++++++++++-- pkg/i18n/english.go | 4 + .../tests/misc/direnv_approves_envrc.go | 90 +++++++++++++++++++ .../misc/direnv_unloads_on_blocked_envrc.go | 36 ++++---- pkg/integration/tests/test_list.go | 1 + 8 files changed, 300 insertions(+), 39 deletions(-) create mode 100644 pkg/integration/tests/misc/direnv_approves_envrc.go diff --git a/pkg/app/app.go b/pkg/app/app.go index 1f49a6a23..15a3f327a 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -172,14 +172,16 @@ func openRecentRepo(app *App) bool { for _, repoDir := range app.Config.GetAppState().RecentRepos { if isRepo, _ := isDirectoryAGitRepository(repoDir); isRepo { if err := os.Chdir(repoDir); err == nil { - // The command log isn't up yet, so any direnv diagnostics - // only make it to the debug log here. - msg, derr := direnv.Load(app.OSCommand.Cmd) - if msg != "" { - app.Log.WithField("message", msg).Info("direnv") + // We're still in setup, before the gui exists, so we can't show the approval popup + // that DispatchSwitchTo offers for blocked .envrc files; just log and move on. + // Also, the logs only go to the debug log, not the Command Log, because that's not + // available yet, either. + result := direnv.Load(app.OSCommand.Cmd) + if result.Message != "" { + app.Log.WithField("message", result.Message).Info("direnv") } - if derr != nil { - app.Log.WithError(derr).Warn("direnv load failed") + if result.Err != nil { + app.Log.WithError(result.Err).Warn("direnv load failed") } return true } diff --git a/pkg/commands/direnv/direnv.go b/pkg/commands/direnv/direnv.go index 9d5d2d1e1..8e98785c2 100644 --- a/pkg/commands/direnv/direnv.go +++ b/pkg/commands/direnv/direnv.go @@ -10,23 +10,39 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/oscommands" ) +// LoadResult bundles everything callers might want to know about a direnv +// invocation. The env-var delta has already been applied to the process by +// the time Load returns. +type LoadResult struct { + // Message is whatever direnv printed to stderr — useful to log + // (success: "direnv: loading .envrc"; error: the error text). + Message string + + // Err is non-nil when direnv exited non-zero or its stdout could + // not be parsed. + Err error + + // Blocked is true when the target .envrc exists but hasn't been + // approved with `direnv allow` yet. EnvrcPath then holds the path + // direnv said was blocked, suitable for passing to Allow. + Blocked bool + EnvrcPath string +} + // Load runs `direnv export json` for the current working directory and applies // the resulting env-var delta to the current process. If direnv isn't on PATH, // it's a no-op — users who don't use direnv pay nothing, and users who do need // no config to opt in. -// -// direnv prints diagnostics to stderr ("direnv: loading .envrc", "direnv: -// error /path/.envrc is blocked", etc.); whatever it printed is returned in -// message so callers can surface it in their command log. -func Load(cmd oscommands.ICmdObjBuilder) (message string, err error) { +func Load(cmd oscommands.ICmdObjBuilder) LoadResult { if _, lookupErr := exec.LookPath("direnv"); lookupErr != nil { - return "", nil + return LoadResult{} } stdout, stderr, runErr := cmd.New([]string{ "direnv", "export", "json", }).DontLog().RunWithOutputs() - message = strings.TrimRight(stderr, "\n") + + result := LoadResult{Message: strings.TrimRight(stderr, "\n")} // Apply whatever delta direnv produced even if it exited non-zero. // When the new dir's .envrc is blocked, direnv still emits a valid @@ -44,9 +60,21 @@ func Load(cmd oscommands.ICmdObjBuilder) (message string, err error) { // Prefer the runtime error (whose Error() text is direnv's stderr) // over a parse error, since it's the more actionable signal. if runErr != nil { - return message, runErr + result.Err = runErr + if envrcPath := queryBlockedEnvrc(cmd); envrcPath != "" { + result.Blocked = true + result.EnvrcPath = envrcPath + } + } else { + result.Err = parseErr } - return message, parseErr + return result +} + +// Allow runs `direnv allow ` to approve a .envrc file so the next +// Load can read it. +func Allow(cmd oscommands.ICmdObjBuilder, envrcPath string) error { + return cmd.New([]string{"direnv", "allow", envrcPath}).DontLog().Run() } func parseDirenvExport(stdout []byte) (map[string]*string, error) { @@ -60,3 +88,43 @@ func parseDirenvExport(stdout []byte) (map[string]*string, error) { } return delta, nil } + +// queryBlockedEnvrc asks direnv (via `status --json`) whether the current +// directory has a found-but-not-yet-allowed .envrc, and returns its path +// if so. We use direnv's structured output rather than parsing the +// human-readable "is blocked" line because the status output is more +// stable across versions and locales. +func queryBlockedEnvrc(cmd oscommands.ICmdObjBuilder) string { + stdout, _, err := cmd.New([]string{ + "direnv", "status", "--json", + }).DontLog().RunWithOutputs() + if err != nil { + return "" + } + return parseDirenvStatus([]byte(stdout)) +} + +func parseDirenvStatus(stdout []byte) string { + var status struct { + State struct { + FoundRC *struct { + Allowed int `json:"allowed"` + Path string `json:"path"` + } `json:"foundRC"` + } `json:"state"` + } + if err := json.Unmarshal(stdout, &status); err != nil { + return "" + } + if status.State.FoundRC == nil { + return "" + } + // direnv's AllowStatus enum (`internal/cmd/rc.go`): 0=Allowed, + // 1=NotAllowed, 2=Denied. Only NotAllowed is something the user + // can approve; Denied means they already said no. + const notAllowed = 1 + if status.State.FoundRC.Allowed != notAllowed { + return "" + } + return status.State.FoundRC.Path +} diff --git a/pkg/commands/direnv/direnv_test.go b/pkg/commands/direnv/direnv_test.go index 69b102d4d..43fdbce88 100644 --- a/pkg/commands/direnv/direnv_test.go +++ b/pkg/commands/direnv/direnv_test.go @@ -41,3 +41,48 @@ func TestParseDirenvExport(t *testing.T) { }) } } + +func TestParseDirenvStatus(t *testing.T) { + scenarios := []struct { + name string + input string + want string + }{ + { + name: "no .envrc found", + input: `{"state":{"foundRC":null}}`, + want: "", + }, + { + name: "found and allowed (0)", + input: `{"state":{"foundRC":{"allowed":0,"path":"/repo/.envrc"}}}`, + want: "", + }, + { + name: "found but not allowed (1) — eligible for approval", + input: `{"state":{"foundRC":{"allowed":1,"path":"/repo/.envrc"}}}`, + want: "/repo/.envrc", + }, + { + name: "found but denied (2) — user already said no", + input: `{"state":{"foundRC":{"allowed":2,"path":"/repo/.envrc"}}}`, + want: "", + }, + { + name: "malformed JSON", + input: `{not json`, + want: "", + }, + { + name: "empty input", + input: "", + want: "", + }, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + assert.Equal(t, s.want, parseDirenvStatus([]byte(s.input))) + }) + } +} diff --git a/pkg/gui/controllers/helpers/repos_helper.go b/pkg/gui/controllers/helpers/repos_helper.go index f8972b837..bde1c47c6 100644 --- a/pkg/gui/controllers/helpers/repos_helper.go +++ b/pkg/gui/controllers/helpers/repos_helper.go @@ -171,13 +171,7 @@ func (self *ReposHelper) DispatchSwitchTo(path string, errMsg string, contextKey return err } - direnvMsg, direnvErr := direnv.Load(self.c.OS().Cmd) - if direnvMsg != "" { - self.c.LogCommand(direnvMsg, false) - } - if direnvErr != nil { - self.c.Log.WithError(direnvErr).Warn("direnv load failed") - } + direnvResult := self.logDirenvResult(direnv.Load(self.c.OS().Cmd)) if err := self.recordDirectoryHelper.RecordCurrentDirectory(); err != nil { self.c.Log.Errorf("error recording current directory: %v", err) @@ -190,6 +184,57 @@ func (self *ReposHelper) DispatchSwitchTo(path string, errMsg string, contextKey return err } - return direnvErr + if direnvResult.Blocked { + self.c.OnUIThread(func() error { + self.promptDirenvApproval(direnvResult.EnvrcPath) + return nil + }) + return nil + } + + return direnvResult.Err + }) +} + +// logDirenvResult writes whatever direnv emitted to the command log and the +// debug log; both happen for every load attempt regardless of outcome. +func (self *ReposHelper) logDirenvResult(result direnv.LoadResult) direnv.LoadResult { + if result.Message != "" { + self.c.LogCommand(result.Message, false) + } + if result.Err != nil { + self.c.Log.WithError(result.Err).Warn("direnv load failed") + } + return result +} + +// promptDirenvApproval shows the user the contents of an unapproved .envrc +// and offers to run `direnv allow` for them. On confirm, we approve the +// file and re-run Load so the new env reaches subprocesses; on cancel we +// leave the env as-is (the previous repo's vars are already unloaded by +// the initial Load call, which is the correct state). +func (self *ReposHelper) promptDirenvApproval(envrcPath string) { + content, err := os.ReadFile(envrcPath) + if err != nil { + self.c.Log.WithError(err).Warn("could not read .envrc for approval prompt") + return + } + + indented := " " + strings.ReplaceAll(strings.TrimRight(string(content), "\n"), "\n", "\n ") + prompt := utils.ResolvePlaceholderString(self.c.Tr.DirenvApprovalPrompt, map[string]string{ + "confirmKey": self.c.UserConfig().Keybinding.Universal.Confirm.String(), + "cancelKey": self.c.UserConfig().Keybinding.Universal.Return.String(), + "content": indented, + }) + + self.c.Confirm(types.ConfirmOpts{ + Title: self.c.Tr.DirenvApprovalTitle, + Prompt: prompt, + HandleConfirm: func() error { + if err := direnv.Allow(self.c.OS().Cmd, envrcPath); err != nil { + return err + } + return self.logDirenvResult(direnv.Load(self.c.OS().Cmd)).Err + }, }) } diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index b52c3f20e..5dcc80806 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -891,6 +891,8 @@ type TranslationSet struct { CreateWorktreeFromDetached string LcWorktree string ChangingDirectoryTo string + DirenvApprovalTitle string + DirenvApprovalPrompt string Name string Branch string Path string @@ -2012,6 +2014,8 @@ func EnglishTranslationSet() *TranslationSet { CreateWorktreeFromDetached: "Create worktree from {{.ref}} (detached)", LcWorktree: "worktree", ChangingDirectoryTo: "Changing directory to {{.path}}", + DirenvApprovalTitle: "Approve .envrc?", + DirenvApprovalPrompt: "Press {{.confirmKey}} to run 'direnv allow' and load the environment.\nPress {{.cancelKey}} to skip.\n\n{{.content}}", Name: "Name", Branch: "Branch", Path: "Path", diff --git a/pkg/integration/tests/misc/direnv_approves_envrc.go b/pkg/integration/tests/misc/direnv_approves_envrc.go new file mode 100644 index 000000000..60780ef19 --- /dev/null +++ b/pkg/integration/tests/misc/direnv_approves_envrc.go @@ -0,0 +1,90 @@ +package misc + +import ( + "os" + "path/filepath" + + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +// When the new repo's .envrc is blocked, lazygit offers the user a popup to +// approve it without leaving the app. Confirming runs `direnv allow` and +// re-runs the load so the env reaches subprocesses immediately. +var DirenvApprovesEnvrc = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Approving a blocked .envrc from the in-app popup loads its env", + ExtraCmdArgs: []string{}, + ExtraEnvVars: map[string]string{ + "PATH": "{{actualPath}}/bin:" + os.Getenv("PATH"), + }, + SetupConfig: func(cfg *config.AppConfig) { + otherRepo, _ := filepath.Abs("../other") + cfg.GetAppState().RecentRepos = []string{otherRepo} + cfg.GetUserConfig().CustomCommands = []config.CustomCommand{ + { + Key: config.Keybinding{"X"}, + Context: "files", + Command: `echo "VAR=$LG_DIRENV_TEST" > output.txt`, + }, + } + }, + SetupRepo: func(shell *Shell) { + shell.EmptyCommit("initial") + shell.CloneNonBare("other") + + shell.CreateFile("../other/.envrc", "export LG_DIRENV_TEST=approved_value\n") + + // Fake direnv that flips behavior once `direnv allow` runs. + // Before allow: export errors with the "blocked" signal, + // status reports allowed=1 (NotAllowed). + // On allow: create a sentinel and exit 0. + // After allow: export emits the loaded delta normally. + shell.CreateFile("../bin/direnv", `#!/bin/sh +SENTINEL="$(dirname "$0")/.approved" +case "$1 $2" in +"allow "*) + touch "$SENTINEL" + exit 0 + ;; +"export json") + if [ -f "$SENTINEL" ]; then + echo '{"LG_DIRENV_TEST":"approved_value"}' + echo "direnv: loading $PWD/.envrc" >&2 + else + echo '{"LG_DIRENV_TEST":null}' + echo "direnv: error $PWD/.envrc is blocked" >&2 + exit 1 + fi + ;; +"status --json") + if [ -f "$SENTINEL" ]; then + printf '{"state":{"foundRC":{"allowed":0,"path":"%s/.envrc"}}}\n' "$PWD" + else + printf '{"state":{"foundRC":{"allowed":1,"path":"%s/.envrc"}}}\n' "$PWD" + fi + ;; +esac +`) + shell.MakeExecutable("../bin/direnv") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.GlobalPress(keys.Universal.OpenRecentRepos) + t.ExpectPopup().Menu().Title(Equals("Recent repositories")). + Lines( + Contains("other").IsSelected(), + Contains("Cancel"), + ). + Confirm() + + t.ExpectPopup().Confirmation(). + Title(Equals("Approve .envrc?")). + Content(Contains("export LG_DIRENV_TEST=approved_value")). + Confirm() + + t.Views().Files(). + Focus(). + Press(config.Keybinding{"X"}). + NavigateToLine(Contains("output.txt")) + t.Views().Main().Content(Contains("VAR=approved_value")) + }, +}) diff --git a/pkg/integration/tests/misc/direnv_unloads_on_blocked_envrc.go b/pkg/integration/tests/misc/direnv_unloads_on_blocked_envrc.go index ca7afe104..541bc446a 100644 --- a/pkg/integration/tests/misc/direnv_unloads_on_blocked_envrc.go +++ b/pkg/integration/tests/misc/direnv_unloads_on_blocked_envrc.go @@ -11,11 +11,11 @@ import ( // Real direnv exits non-zero when the destination .envrc isn't authorized, // but it still emits a valid JSON delta on stdout that unloads vars from // the previously-active .envrc. We have to apply that delta anyway, or the -// previous repo's env leaks into the new one. The fake direnv here mimics -// that behavior; the test also asserts that the user gets an error popup -// (the command log alone is easy to miss). +// previous repo's env leaks into the new one. This test exercises the +// "skip approval" branch: the approval popup appears, the user cancels, +// and the previous repo's env is still gone. var DirenvUnloadsOnBlockedEnvrc = NewIntegrationTest(NewIntegrationTestArgs{ - Description: "Blocked .envrc unloads the previous repo's env and shows an error popup", + Description: "Blocked .envrc unloads the previous repo's env even if the user skips approval", ExtraCmdArgs: []string{}, ExtraEnvVars: map[string]string{ "PATH": "{{actualPath}}/bin:" + os.Getenv("PATH"), @@ -37,10 +37,19 @@ var DirenvUnloadsOnBlockedEnvrc = NewIntegrationTest(NewIntegrationTestArgs{ shell.EmptyCommit("initial") shell.CloneNonBare("other") + shell.CreateFile("../other/.envrc", "export LG_DIRENV_TEST=from_envrc\n") + shell.CreateFile("../bin/direnv", `#!/bin/sh -echo '{"LG_DIRENV_TEST":null}' -echo "direnv: error /repo/.envrc is blocked. Run 'direnv allow' to approve its content" >&2 -exit 1 +case "$1 $2" in +"export json") + echo '{"LG_DIRENV_TEST":null}' + echo "direnv: error $PWD/.envrc is blocked" >&2 + exit 1 + ;; +"status --json") + printf '{"state":{"foundRC":{"allowed":1,"path":"%s/.envrc"}}}\n' "$PWD" + ;; +esac `) shell.MakeExecutable("../bin/direnv") }, @@ -53,18 +62,15 @@ exit 1 ). Confirm() - t.ExpectPopup().Alert(). - Title(Equals("Error")). - Content(Contains("is blocked")). - Confirm() + t.ExpectPopup().Confirmation(). + Title(Equals("Approve .envrc?")). + Content(Contains("export LG_DIRENV_TEST=from_envrc")). + Cancel() - // If unload worked, $LG_DIRENV_TEST is empty in the custom command. t.Views().Files(). Focus(). Press(config.Keybinding{"X"}). - Lines( - Contains("output.txt").IsSelected(), - ) + NavigateToLine(Contains("output.txt")) t.Views().Main().Content(Contains("VAR=[]")) }, }) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 3679099e0..6f0032391 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -334,6 +334,7 @@ var tests = []*components.IntegrationTest{ misc.ConfirmOnQuit, misc.CopyConfirmationMessageToClipboard, misc.CopyToClipboard, + misc.DirenvApprovesEnvrc, misc.DirenvLoadedOnRepoSwitch, misc.DirenvUnloadsOnBlockedEnvrc, misc.InitialOpen, From c588c5507c60907654aec21043abbc4a1e75337b Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 2 Jun 2026 06:54:39 +0200 Subject: [PATCH 06/11] Add a test demonstrating that you can't unstage a dirty submodule When a submodule has both a new commit (which the parent repo can stage) and dirty working-tree content (which it can't), staging it lands on a "MM" status. Pressing space again should unstage it, but instead it tries to stage the dirty content over and over, so you can never get back to an unstaged state. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/integration/tests/submodule/stage.go | 51 ++++++++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 2 files changed, 52 insertions(+) create mode 100644 pkg/integration/tests/submodule/stage.go diff --git a/pkg/integration/tests/submodule/stage.go b/pkg/integration/tests/submodule/stage.go new file mode 100644 index 000000000..7303a0dbc --- /dev/null +++ b/pkg/integration/tests/submodule/stage.go @@ -0,0 +1,51 @@ +package submodule + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var Stage = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Stage and unstage a submodule that has both a new commit and dirty content. The new commit can be staged, but the dirty content can't, so unstaging must still work.", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.ShowFileTree = false + }, + SetupRepo: func(shell *Shell) { + shell.EmptyCommit("first commit") + shell.CloneIntoSubmodule("my_submodule_name", "my_submodule_path") + shell.GitAddAll() + shell.Commit("add submodule") + + // Give the submodule a new commit, which is a change that the parent + // repo can stage, as well as some dirty working-tree content, which + // the parent repo can never stage. This is what gets us a "MM" status + // once the new commit is staged. + shell.RunCommand([]string{"git", "-C", "my_submodule_path", "commit", "--allow-empty", "-m", "submodule commit"}) + shell.CreateFile("my_submodule_path/dirty_file", "dirty content") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Files().Focus(). + Lines( + Equals(" M my_submodule_path (submodule)").IsSelected(), + ). + // Staging the submodule stages the new commit, but the dirty + // content remains unstaged, leaving us at "MM". + PressPrimaryAction(). + Lines( + Equals("MM my_submodule_path (submodule)").IsSelected(), + ). + // Pressing again must unstage the submodule, taking us back to + // " M" rather than trying (and failing) to stage the dirty content. + PressPrimaryAction(). + /* EXPECTED: + Lines( + Equals(" M my_submodule_path (submodule)").IsSelected(), + ) + ACTUAL: */ + Lines( + Equals("MM my_submodule_path (submodule)").IsSelected(), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 6f0032391..7cf31d28a 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -426,6 +426,7 @@ var tests = []*components.IntegrationTest{ submodule.RemoveNested, submodule.Reset, submodule.ResetFolder, + submodule.Stage, sync.FetchAndAutoForwardBranchesAllBranches, sync.FetchAndAutoForwardBranchesAllBranchesCheckedOutInOtherWorktree, sync.FetchAndAutoForwardBranchesNone, From 66fe18dd593f7c3d7bf98fd4b5bda67b726eec5d Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 2 Jun 2026 11:58:38 +0200 Subject: [PATCH 07/11] Unify the stage/unstage decision for press and stage-all MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pressWithLock (acting on the selection) and toggleStagedAllWithLock (acting on the whole tree) each independently decided whether to stage or unstage, ran the optimistic update, and logged the action. That duplicated decision has already drifted: the tracked-files filter was added to press months before it was applied to stage-all, and fixes to one have repeatedly had to be chased into the other. Extract that shared decision into toggleStaged, leaving each caller to supply only the git commands it runs (per-path for the selection, bulk add -A / reset for the whole tree — the latter is required because the tree root node has an empty path, so a per-path stage wouldn't work). This is a pure refactor: the two callers' decisions were already equivalent, so behavior is unchanged. It exists so the next change to the staging logic only has to be made once. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/gui/controllers/files_controller.go | 190 ++++++++++++------------ 1 file changed, 95 insertions(+), 95 deletions(-) diff --git a/pkg/gui/controllers/files_controller.go b/pkg/gui/controllers/files_controller.go index 8ea4425e6..c03ff71ab 100644 --- a/pkg/gui/controllers/files_controller.go +++ b/pkg/gui/controllers/files_controller.go @@ -445,13 +445,23 @@ func (self *FilesController) optimisticChange(nodes []*filetree.FileNode, optimi return nil } -func (self *FilesController) pressWithLock(selectedNodes []*filetree.FileNode) error { - // Obtaining this lock because optimistic rendering requires us to mutate - // the files in our model. - self.c.Mutexes().RefreshingFilesMutex.Lock() - defer self.c.Mutexes().RefreshingFilesMutex.Unlock() - - for _, node := range selectedNodes { +// toggleStaged decides whether to stage or unstage the given nodes, updates the +// model optimistically, and then runs the matching git command via the supplied +// callbacks. press() (acting on the selection) and toggleStagedAll() (acting on +// the whole tree) share this; they differ only in the git commands they run, +// which is why those are passed in. +// +// If any node has unstaged changes we stage the nodes that have them (staging +// already-staged deleted files/folders would fail); otherwise we unstage all +// the nodes. +func (self *FilesController) toggleStaged( + nodes []*filetree.FileNode, + stageAction string, + unstageAction string, + stage func(unstagedNodes []*filetree.FileNode) error, + unstage func(nodes []*filetree.FileNode) error, +) error { + for _, node := range nodes { // if any files within have inline merge conflicts we can't stage or unstage, // or it'll end up with those >>>>>> lines actually staged if node.GetHasInlineMergeConflicts() { @@ -459,6 +469,35 @@ func (self *FilesController) pressWithLock(selectedNodes []*filetree.FileNode) e } } + nodes = normalisedSelectedNodes(nodes) + + unstagedNodes := filterNodesHaveUnstagedChanges(nodes) + + if len(unstagedNodes) > 0 { + self.c.LogAction(stageAction) + + if err := self.optimisticChange(unstagedNodes, self.optimisticStage); err != nil { + return err + } + + return stage(unstagedNodes) + } + + self.c.LogAction(unstageAction) + + if err := self.optimisticChange(nodes, self.optimisticUnstage); err != nil { + return err + } + + return unstage(nodes) +} + +func (self *FilesController) pressWithLock(selectedNodes []*filetree.FileNode) error { + // Obtaining this lock because optimistic rendering requires us to mutate + // the files in our model. + self.c.Mutexes().RefreshingFilesMutex.Lock() + defer self.c.Mutexes().RefreshingFilesMutex.Unlock() + // When filtering, expand directory nodes to individual visible file paths // so that only filtered files are staged/unstaged. toPaths := func(nodes []*filetree.FileNode) []string { @@ -477,63 +516,46 @@ func (self *FilesController) pressWithLock(selectedNodes []*filetree.FileNode) e }) } - selectedNodes = normalisedSelectedNodes(selectedNodes) - - // If any node has unstaged changes, we'll stage all the selected unstaged nodes (staging already staged deleted files/folders would fail). - // Otherwise, we unstage all the selected nodes. - unstagedSelectedNodes := filterNodesHaveUnstagedChanges(selectedNodes) - - if len(unstagedSelectedNodes) > 0 { + stage := func(unstagedNodes []*filetree.FileNode) error { var extraArgs []string - if self.context().GetStatusFilter() == filetree.DisplayTracked { extraArgs = []string{"-u"} } - self.c.LogAction(self.c.Tr.Actions.StageFile) - - if err := self.optimisticChange(unstagedSelectedNodes, self.optimisticStage); err != nil { - return err - } - - if err := self.c.Git().WorkingTree.StageFiles(toPaths(unstagedSelectedNodes), extraArgs); err != nil { - return err - } - } else { - self.c.LogAction(self.c.Tr.Actions.UnstageFile) - - if err := self.optimisticChange(selectedNodes, self.optimisticUnstage); err != nil { - return err - } - - if self.context().IsFiltering() { - // When filtering, only unstage visible files - if err := self.unstageFilteredFiles(selectedNodes); err != nil { - return err - } - } else { - // need to partition the paths into tracked and untracked (where we assume directories are tracked). Then we'll run the commands separately. - trackedNodes, untrackedNodes := utils.Partition(selectedNodes, func(node *filetree.FileNode) bool { - // We treat all directories as tracked. I'm not actually sure why we do this but - // it's been the existing behaviour for a while and nobody has complained - return !node.IsFile() || node.GetIsTracked() - }) - - if len(untrackedNodes) > 0 { - if err := self.c.Git().WorkingTree.UnstageUntrackedFiles(toPaths(untrackedNodes)); err != nil { - return err - } - } - - if len(trackedNodes) > 0 { - if err := self.c.Git().WorkingTree.UnstageTrackedFiles(toPaths(trackedNodes)); err != nil { - return err - } - } - } + return self.c.Git().WorkingTree.StageFiles(toPaths(unstagedNodes), extraArgs) } - return nil + unstage := func(nodes []*filetree.FileNode) error { + if self.context().IsFiltering() { + // When filtering, only unstage visible files + return self.unstageFilteredFiles(nodes) + } + + // need to partition the paths into tracked and untracked (where we assume directories are tracked). Then we'll run the commands separately. + trackedNodes, untrackedNodes := utils.Partition(nodes, func(node *filetree.FileNode) bool { + // We treat all directories as tracked. I'm not actually sure why we do this but + // it's been the existing behaviour for a while and nobody has complained + return !node.IsFile() || node.GetIsTracked() + }) + + if len(untrackedNodes) > 0 { + if err := self.c.Git().WorkingTree.UnstageUntrackedFiles(toPaths(untrackedNodes)); err != nil { + return err + } + } + + if len(trackedNodes) > 0 { + if err := self.c.Git().WorkingTree.UnstageTrackedFiles(toPaths(trackedNodes)); err != nil { + return err + } + } + + return nil + } + + return self.toggleStaged(selectedNodes, + self.c.Tr.Actions.StageFile, self.c.Tr.Actions.UnstageFile, + stage, unstage) } func (self *FilesController) press(nodes []*filetree.FileNode) error { @@ -721,19 +743,7 @@ func (self *FilesController) toggleStagedAllWithLock() error { root := self.context().FileTreeViewModel.GetRoot() - // if any files within have inline merge conflicts we can't stage or unstage, - // or it'll end up with those >>>>>> lines actually staged - if root.GetHasInlineMergeConflicts() { - return errors.New(self.c.Tr.ErrStageDirWithInlineMergeConflicts) - } - - if root.GetHasUnstagedChanges() { - self.c.LogAction(self.c.Tr.Actions.StageAllFiles) - - if err := self.optimisticChange([]*filetree.FileNode{root}, self.optimisticStage); err != nil { - return err - } - + stage := func(unstagedNodes []*filetree.FileNode) error { if self.context().IsFiltering() { // When filtering, only stage visible files var paths []string @@ -741,35 +751,25 @@ func (self *FilesController) toggleStagedAllWithLock() error { paths = append(paths, file.Path) return nil }) - if err := self.c.Git().WorkingTree.StageFiles(paths, nil); err != nil { - return err - } - } else { - onlyTrackedFiles := self.context().GetStatusFilter() == filetree.DisplayTracked - if err := self.c.Git().WorkingTree.StageAll(onlyTrackedFiles); err != nil { - return err - } - } - } else { - self.c.LogAction(self.c.Tr.Actions.UnstageAllFiles) - - if err := self.optimisticChange([]*filetree.FileNode{root}, self.optimisticUnstage); err != nil { - return err + return self.c.Git().WorkingTree.StageFiles(paths, nil) } - if self.context().IsFiltering() { - // When filtering, only unstage visible files - if err := self.unstageFilteredFiles([]*filetree.FileNode{root}); err != nil { - return err - } - } else { - if err := self.c.Git().WorkingTree.UnstageAll(); err != nil { - return err - } - } + onlyTrackedFiles := self.context().GetStatusFilter() == filetree.DisplayTracked + return self.c.Git().WorkingTree.StageAll(onlyTrackedFiles) } - return nil + unstage := func(nodes []*filetree.FileNode) error { + if self.context().IsFiltering() { + // When filtering, only unstage visible files + return self.unstageFilteredFiles(nodes) + } + + return self.c.Git().WorkingTree.UnstageAll() + } + + return self.toggleStaged([]*filetree.FileNode{root}, + self.c.Tr.Actions.StageAllFiles, self.c.Tr.Actions.UnstageAllFiles, + stage, unstage) } func (self *FilesController) unstageFiles(node *filetree.FileNode) error { From 3f0a7512f8ca251f17db757da52ced7d344f00ae Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 2 Jun 2026 12:09:09 +0200 Subject: [PATCH 08/11] Fix unstaging a submodule with dirty content The stage/unstage toggle decides what to do based on whether a node has unstaged changes: if it does, it stages; otherwise it unstages. For a submodule this breaks down, because dirty or untracked content inside the submodule always reports as an unstaged change in the parent repo but can never be staged from there. Once such a submodule's commit pointer is staged it sits at "MM", and every subsequent press keeps trying to stage the unstageable dirty content, so it can never be unstaged. Treat a submodule's unstaged change as stageable only when its commit isn't already staged, so that a staged submodule unstages on the next press regardless of leftover dirty content. Because the decision is now shared by press and stage-all, this fixes both at once. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/gui/controllers/files_controller.go | 26 +++++++++++++++++++++--- pkg/integration/tests/submodule/stage.go | 5 ----- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/pkg/gui/controllers/files_controller.go b/pkg/gui/controllers/files_controller.go index c03ff71ab..d8f883e40 100644 --- a/pkg/gui/controllers/files_controller.go +++ b/pkg/gui/controllers/files_controller.go @@ -471,7 +471,7 @@ func (self *FilesController) toggleStaged( nodes = normalisedSelectedNodes(nodes) - unstagedNodes := filterNodesHaveUnstagedChanges(nodes) + unstagedNodes := filterNodesHaveUnstagedChanges(nodes, self.c.Model().Submodules) if len(unstagedNodes) > 0 { self.c.LogAction(stageAction) @@ -1421,12 +1421,32 @@ func someNodesHaveStagedChanges(nodes []*filetree.FileNode) bool { return lo.SomeBy(nodes, (*filetree.FileNode).GetHasStagedChanges) } -func filterNodesHaveUnstagedChanges(nodes []*filetree.FileNode) []*filetree.FileNode { +func filterNodesHaveUnstagedChanges(nodes []*filetree.FileNode, submodules []*models.SubmoduleConfig) []*filetree.FileNode { return lo.Filter(nodes, func(node *filetree.FileNode, _ int) bool { - return node.GetHasUnstagedChanges() + return node.SomeFile(func(file *models.File) bool { + return fileHasStageableUnstagedChanges(file, submodules) + }) }) } +// For a submodule, the only thing the parent repo can stage is the +// commit-pointer change; dirty or untracked content within the submodule +// shows up as an unstaged change but can never be staged from the parent. So +// once the submodule's commit is staged (leaving it at e.g. "MM"), we mustn't +// treat the leftover unstaged change as stageable, or pressing space would +// keep trying to stage it instead of unstaging it. +func fileHasStageableUnstagedChanges(file *models.File, submodules []*models.SubmoduleConfig) bool { + if !file.HasUnstagedChanges { + return false + } + + if file.IsSubmodule(submodules) { + return !file.HasStagedChanges + } + + return true +} + func findSubmoduleNode(nodes []*filetree.FileNode, submodules []*models.SubmoduleConfig) *models.File { for _, node := range nodes { submoduleNode := node.FindFirstFileBy(func(f *models.File) bool { diff --git a/pkg/integration/tests/submodule/stage.go b/pkg/integration/tests/submodule/stage.go index 7303a0dbc..324d7690f 100644 --- a/pkg/integration/tests/submodule/stage.go +++ b/pkg/integration/tests/submodule/stage.go @@ -39,13 +39,8 @@ var Stage = NewIntegrationTest(NewIntegrationTestArgs{ // Pressing again must unstage the submodule, taking us back to // " M" rather than trying (and failing) to stage the dirty content. PressPrimaryAction(). - /* EXPECTED: Lines( Equals(" M my_submodule_path (submodule)").IsSelected(), ) - ACTUAL: */ - Lines( - Equals("MM my_submodule_path (submodule)").IsSelected(), - ) }, }) From c46c8744429c9dd29e95b62c8002f0875e635b62 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 2 Jun 2026 12:09:41 +0200 Subject: [PATCH 09/11] Also verify stage-all can unstage a dirty submodule Before the staging decision was unified, the stage (space) and stage-all (a) keybindings each made their own decision, so a fix to one wouldn't reach the other. Extend the test to drive the submodule through stage-all as well, guarding against that asymmetry coming back. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/integration/tests/submodule/stage.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/pkg/integration/tests/submodule/stage.go b/pkg/integration/tests/submodule/stage.go index 324d7690f..b8ef5e35f 100644 --- a/pkg/integration/tests/submodule/stage.go +++ b/pkg/integration/tests/submodule/stage.go @@ -6,7 +6,7 @@ import ( ) var Stage = NewIntegrationTest(NewIntegrationTestArgs{ - Description: "Stage and unstage a submodule that has both a new commit and dirty content. The new commit can be staged, but the dirty content can't, so unstaging must still work.", + Description: "Stage and unstage a submodule that has both a new commit and dirty content. The new commit can be staged, but the dirty content can't, so unstaging must still work; this must hold for both the stage (space) and stage-all (a) keybindings.", ExtraCmdArgs: []string{}, Skip: false, SetupConfig: func(config *config.AppConfig) { @@ -39,6 +39,18 @@ var Stage = NewIntegrationTest(NewIntegrationTestArgs{ // Pressing again must unstage the submodule, taking us back to // " M" rather than trying (and failing) to stage the dirty content. PressPrimaryAction(). + Lines( + Equals(" M my_submodule_path (submodule)").IsSelected(), + ). + // The same has to hold for the stage-all keybinding, which shares + // the same decision logic: it stages the new commit... + Press(keys.Files.ToggleStagedAll). + Lines( + Equals("MM my_submodule_path (submodule)").IsSelected(), + ). + // ...and then unstages it again rather than getting stuck on the + // dirty content. + Press(keys.Files.ToggleStagedAll). Lines( Equals(" M my_submodule_path (submodule)").IsSelected(), ) From 8b5cfb0425cb8be7dee7ea2607358a3570212fdd Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 2 Jun 2026 07:19:51 +0200 Subject: [PATCH 10/11] Optimistically render unstaging a dirty submodule This map only feeds the optimistic rendering that makes staging feel instant; it doesn't affect the eventual status, which git reports after the refresh. The "MM" entry can never be reached for a regular file: a file at "MM" has stageable unstaged changes, so pressing space stages it rather than unstaging, and the unstage path is where this map is used. The only thing that reaches the unstage path at "MM" is a submodule whose commit is staged on top of dirty content, so this entry exists purely to update that submodule instantly instead of waiting for the next git status. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/gui/controllers/files_controller.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/gui/controllers/files_controller.go b/pkg/gui/controllers/files_controller.go index d8f883e40..d4f38f0fb 100644 --- a/pkg/gui/controllers/files_controller.go +++ b/pkg/gui/controllers/files_controller.go @@ -387,6 +387,9 @@ var unstageStatusMap = map[string]string{ "A ": "??", "M ": " M", "D ": " D", + // A submodule with both a staged commit and unstageable dirty content; the + // staged commit gets unstaged, the dirty content stays. + "MM": " M", } func (self *FilesController) optimisticStage(file *models.File) bool { From 785c8a712cd239791ca1ee62595eb2c16a3f1657 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 2 Jun 2026 12:14:24 +0200 Subject: [PATCH 11/11] Explain when a submodule has nothing stageable A submodule that only has dirty or untracked content (no new commit) can't be staged from the parent repo, but it still shows up as having unstaged changes. Pressing stage on it therefore briefly flashed as staged and then reverted, without explaining why nothing was staged. Detect this case (via `git submodule status`, where a '+' prefix marks a stageable commit change) in the shared stage/unstage decision: if the only thing that looks stageable is such a submodule, don't try to stage it. Instead unstage if there's anything staged to unstage, so the toggle stays symmetric; otherwise show an error explaining that there's nothing to stage. Because the decision is shared, this covers both the stage (space) and stage-all (a) keybindings. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/commands/git_commands/submodule.go | 25 ++++++++ pkg/gui/controllers/files_controller.go | 57 ++++++++++++++++++- pkg/i18n/english.go | 2 + .../stage_all_with_dirty_submodule.go | 46 +++++++++++++++ .../tests/submodule/stage_dirty_only.go | 53 +++++++++++++++++ pkg/integration/tests/test_list.go | 2 + 6 files changed, 184 insertions(+), 1 deletion(-) create mode 100644 pkg/integration/tests/submodule/stage_all_with_dirty_submodule.go create mode 100644 pkg/integration/tests/submodule/stage_dirty_only.go diff --git a/pkg/commands/git_commands/submodule.go b/pkg/commands/git_commands/submodule.go index acb335e35..7a3cb687b 100644 --- a/pkg/commands/git_commands/submodule.go +++ b/pkg/commands/git_commands/submodule.go @@ -9,6 +9,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" + "github.com/samber/lo" ) // .gitmodules looks like this: @@ -86,6 +87,30 @@ func (self *SubmoduleCommands) GetConfigs(parentModule *models.SubmoduleConfig) return configs, nil } +// AnyHaveStageableChanges reports whether any of the given submodule paths has +// a checked-out commit that differs from the one recorded in the +// superproject's index, i.e. a change that `git add ` would actually +// stage. A submodule that only has dirty or untracked content (with no new +// commit) can't be staged from the superproject, so it won't be reported here. +func (self *SubmoduleCommands) AnyHaveStageableChanges(paths []string) (bool, error) { + if len(paths) == 0 { + return false, nil + } + + cmdArgs := NewGitCmd("submodule").Arg("status", "--").Arg(paths...).ToArgv() + output, err := self.cmd.New(cmdArgs).DontLog().RunWithOutput() + if err != nil { + return false, err + } + + // Each line looks like " ()". A '+' prefix + // means the checked-out commit differs from the index, i.e. there's a + // commit change to stage. + return lo.SomeBy(strings.Split(output, "\n"), func(line string) bool { + return strings.HasPrefix(line, "+") + }), nil +} + func (self *SubmoduleCommands) Stash(submodule *models.SubmoduleConfig) error { // if the path does not exist then it hasn't yet been initialized so we'll swallow the error // because the intention here is to have no dirty worktree state diff --git a/pkg/gui/controllers/files_controller.go b/pkg/gui/controllers/files_controller.go index d4f38f0fb..09f654e2b 100644 --- a/pkg/gui/controllers/files_controller.go +++ b/pkg/gui/controllers/files_controller.go @@ -476,7 +476,22 @@ func (self *FilesController) toggleStaged( unstagedNodes := filterNodesHaveUnstagedChanges(nodes, self.c.Model().Submodules) - if len(unstagedNodes) > 0 { + // Staging a submodule that only has dirty or untracked content (no new + // commit) is a no-op: the parent repo can't stage that content. When that's + // the only thing that looks stageable, don't stage; fall through to + // unstaging instead. That keeps the toggle symmetric (e.g. a fully-staged + // tree that also contains a dirty submodule still unstages on the next + // press) rather than getting stuck trying to stage the unstageable content. + shouldStage := len(unstagedNodes) > 0 + if shouldStage { + noOp, err := self.stagingWouldBeNoOp(unstagedNodes) + if err != nil { + return err + } + shouldStage = !noOp + } + + if shouldStage { self.c.LogAction(stageAction) if err := self.optimisticChange(unstagedNodes, self.optimisticStage); err != nil { @@ -486,6 +501,12 @@ func (self *FilesController) toggleStaged( return stage(unstagedNodes) } + // If there's nothing staged to unstage either, then the only thing we acted + // on was an unstageable submodule and nothing happened, so say why. + if !someNodesHaveStagedChanges(nodes) { + return errors.New(self.c.Tr.NothingToStageForSubmodule) + } + self.c.LogAction(unstageAction) if err := self.optimisticChange(nodes, self.optimisticUnstage); err != nil { @@ -1450,6 +1471,40 @@ func fileHasStageableUnstagedChanges(file *models.File, submodules []*models.Sub return true } +// stagingWouldBeNoOp reports whether staging the given nodes would have no +// visible effect, which happens when the only things being staged are +// submodules that have dirty or untracked content but no new commit: the +// parent repo can't stage that content. If a regular file (or a submodule with +// a stageable new commit) is among them, staging does something, so this +// returns false. +func (self *FilesController) stagingWouldBeNoOp(nodes []*filetree.FileNode) (bool, error) { + submodules := self.c.Model().Submodules + + var submodulePaths []string + hasOtherStageableChanges := false + for _, node := range nodes { + _ = node.ForEachFile(func(file *models.File) error { + if file.IsSubmodule(submodules) { + submodulePaths = append(submodulePaths, file.Path) + } else if file.HasUnstagedChanges { + hasOtherStageableChanges = true + } + return nil + }) + } + + if hasOtherStageableChanges || len(submodulePaths) == 0 { + return false, nil + } + + anyStageable, err := self.c.Git().Submodule.AnyHaveStageableChanges(submodulePaths) + if err != nil { + return false, err + } + + return !anyStageable, nil +} + func findSubmoduleNode(nodes []*filetree.FileNode, submodules []*models.SubmoduleConfig) *models.File { for _, node := range nodes { submoduleNode := node.FindFirstFileBy(func(f *models.File) bool { diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 5dcc80806..d112c0379 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -917,6 +917,7 @@ type TranslationSet struct { SelectedItemIsNotABranch string SelectedItemDoesNotHaveFiles string MultiSelectNotSupportedForSubmodules string + NothingToStageForSubmodule string CommandDoesNotSupportOpeningInEditor string CustomCommands string NoApplicableCommandsInThisContext string @@ -2038,6 +2039,7 @@ func EnglishTranslationSet() *TranslationSet { SelectedItemIsNotABranch: "Selected item is not a branch", SelectedItemDoesNotHaveFiles: "Selected item does not have files to view", MultiSelectNotSupportedForSubmodules: "Multiselection not supported for submodules", + NothingToStageForSubmodule: "Nothing to stage: the parent repo can only stage a new submodule commit, not the uncommitted changes inside a submodule. Commit inside the submodule first.", CommandDoesNotSupportOpeningInEditor: "This command doesn't support switching to the editor", CustomCommands: "Custom commands", NoApplicableCommandsInThisContext: "(No applicable commands in this context)", diff --git a/pkg/integration/tests/submodule/stage_all_with_dirty_submodule.go b/pkg/integration/tests/submodule/stage_all_with_dirty_submodule.go new file mode 100644 index 000000000..ca54a5970 --- /dev/null +++ b/pkg/integration/tests/submodule/stage_all_with_dirty_submodule.go @@ -0,0 +1,46 @@ +package submodule + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var StageAllWithDirtySubmodule = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "A submodule with only dirty content (which can't be staged) must not break the stage-all toggle: pressing it repeatedly should keep toggling the other files between staged and unstaged.", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.ShowFileTree = false + }, + SetupRepo: func(shell *Shell) { + shell.EmptyCommit("first commit") + shell.CloneIntoSubmodule("my_submodule_name", "my_submodule_path") + shell.GitAddAll() + shell.Commit("add submodule") + + // A submodule with dirty content but no new commit (can't be staged), + // alongside a regular file that can. + shell.CreateFile("my_submodule_path/dirty_file", "dirty content") + shell.CreateFile("regular_file", "content") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Files().Focus(). + Lines( + Equals(" M my_submodule_path (submodule)"), + Equals("?? regular_file"), + ). + // Stage all: the regular file gets staged; the submodule can't be. + Press(keys.Files.ToggleStagedAll). + Lines( + Equals(" M my_submodule_path (submodule)"), + Equals("A regular_file"), + ). + // Stage all again: nothing is stageable, but the regular file is + // staged, so this unstages it rather than erroring on the submodule. + Press(keys.Files.ToggleStagedAll). + Lines( + Equals(" M my_submodule_path (submodule)"), + Equals("?? regular_file"), + ) + }, +}) diff --git a/pkg/integration/tests/submodule/stage_dirty_only.go b/pkg/integration/tests/submodule/stage_dirty_only.go new file mode 100644 index 000000000..3ae20e677 --- /dev/null +++ b/pkg/integration/tests/submodule/stage_dirty_only.go @@ -0,0 +1,53 @@ +package submodule + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var StageDirtyOnly = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Pressing space on a submodule that only has dirty content (no new commit) can't stage anything, so we explain that with an error instead of silently doing nothing.", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.ShowFileTree = false + }, + SetupRepo: func(shell *Shell) { + shell.EmptyCommit("first commit") + shell.CloneIntoSubmodule("my_submodule_name", "my_submodule_path") + shell.GitAddAll() + shell.Commit("add submodule") + + // Dirty working-tree content, but no new commit: there's nothing the + // parent repo can stage. + shell.CreateFile("my_submodule_path/dirty_file", "dirty content") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Files().Focus(). + Lines( + Equals(" M my_submodule_path (submodule)").IsSelected(), + ). + PressPrimaryAction(). + Tap(func() { + t.ExpectPopup().Alert(). + Title(Equals("Error")). + Content(Contains("Nothing to stage")). + Confirm() + }). + // The status is unchanged: nothing got staged. + Lines( + Equals(" M my_submodule_path (submodule)").IsSelected(), + ). + // Pressing "stage all" must behave the same way. + Press(keys.Files.ToggleStagedAll). + Tap(func() { + t.ExpectPopup().Alert(). + Title(Equals("Error")). + Content(Contains("Nothing to stage")). + Confirm() + }). + Lines( + Equals(" M my_submodule_path (submodule)").IsSelected(), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 7cf31d28a..0f3a40634 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -427,6 +427,8 @@ var tests = []*components.IntegrationTest{ submodule.Reset, submodule.ResetFolder, submodule.Stage, + submodule.StageAllWithDirtySubmodule, + submodule.StageDirtyOnly, sync.FetchAndAutoForwardBranchesAllBranches, sync.FetchAndAutoForwardBranchesAllBranchesCheckedOutInOtherWorktree, sync.FetchAndAutoForwardBranchesNone,