From 49410fc95394d1a97a90c00611f995a04e980acf Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 13 May 2026 09:54:47 +0200 Subject: [PATCH 1/6] Some additions to AGENTS.md --- AGENTS.md | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 476d6e03a..1d7688b6c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -106,13 +106,54 @@ the buggy one, so the file compiles and the test passes against unfixed code. In the fix commit, remove the comment markers and delete the `ACTUAL` line. Don't explain the pattern in commit messages. +The fix commit must be _exactly_ "delete the markers and delete the `ACTUAL` +line" — no other edits. That means `EXPECTED` and `ACTUAL` have to be drop-in +replacements for each other at the same syntactic position. If you can't write +them that way (e.g. one is `.IsEmpty()` and the other is `.Lines(...)`), +restructure the surrounding code until you can — usually by putting the +comment block between two adjacent chained calls, so both forms are just the +next method in the chain: + +```go +t.Views().Files(). + Focus(). + /* EXPECTED: + IsEmpty() + ACTUAL: */ + Lines( + Equals("D file03.txt"), + ) +``` + +If you find yourself reaching for a local variable so that both forms can be +expressed against the same receiver, the structure isn't right yet — go back +and fix it instead of papering over it with a binding. + Use this pattern only where it makes sense; don't apply it by default. +## Integration test conventions + +Don't bind views to local variables. Always chain method calls directly from +`t.Views().()`. Patterns like `filesView := t.Views().Files().Focus()` +followed by `filesView.Lines(...)` are not how tests in this repo are written; +keep the call site fluent. + ## Use stretchr/testify for assertions Prefer `assert.Equal` (and friends) over hand-rolled `if` checks. The failure messages are more useful and the intent is clearer at a glance. +## Don't present "live with the bug" as an option + +When you're investigating a defect and laying out fix options for the user, +"accept the race / leave it as-is / document it and move on" is not one of +them. A known race condition, data corruption, or correctness violation is a +bug that needs a real fix, not a tradeoff. Even if the failure rate is low, +even if the window is tiny, even if no current code path appears to hit it — +present actual fixes. If a real fix is genuinely out of reach (e.g. it +requires API changes you can't make), say so plainly; don't dress "no fix" +up as a viable option in a numbered list alongside real ones. + ## Don't search outside the working tree Never run `find` (or similar) from `/` or other paths outside the project. All From 7d1d90ae4d319f8f1bb54f99e46c2bae20ff66e8 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 21 May 2026 11:54:18 +0200 Subject: [PATCH 2/6] Preserve empty Worktrees slice when worktree list fails to load If `git worktree list` fails, we want the Worktrees model to fall back to an empty slice so callers iterating over it stay correct. The error branch was setting it to `[]`, but the line below unconditionally overwrote it with the nil `worktrees` value from the failed call. Use an else branch so the empty-slice fallback actually sticks. Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/gui/controllers/helpers/refresh_helper.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 2922b4403..d27b38feb 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -724,9 +724,9 @@ func (self *RefreshHelper) loadWorktrees() { if err != nil { self.c.Log.Error(err) self.c.Model().Worktrees = []*models.Worktree{} + } else { + self.c.Model().Worktrees = worktrees } - - self.c.Model().Worktrees = worktrees } func (self *RefreshHelper) refreshWorktrees() { From 77652863c4c152fce1d2556db15c5ce3b9702bdf Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 21 May 2026 11:54:45 +0200 Subject: [PATCH 3/6] Refresh pull requests after a manual fetch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The background fetch path already includes PULL_REQUESTS in its post-fetch refresh scope, but the manual fetch from the files view doesn't. As far as I can tell that's an oversight from when PULL_REQUESTS was added — there's no reason the two paths should differ. Align them so both refresh PRs after fetching. This also sets up the next commit to extract a shared helper for the post-fetch refresh. Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/gui/controllers/files_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/gui/controllers/files_controller.go b/pkg/gui/controllers/files_controller.go index e75a5ee3f..1513a324e 100644 --- a/pkg/gui/controllers/files_controller.go +++ b/pkg/gui/controllers/files_controller.go @@ -1348,7 +1348,7 @@ func (self *FilesController) fetch() error { return errors.New(self.c.Tr.PassUnameWrong) } - self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.BRANCHES, types.COMMITS, types.REMOTES, types.TAGS}, Mode: types.SYNC}) + self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.BRANCHES, types.COMMITS, types.REMOTES, types.TAGS, types.PULL_REQUESTS}, Mode: types.SYNC}) if err == nil { err = self.c.Helpers().BranchesHelper.AutoForwardBranches() From f032ee8b0f36747b4edfeab178e82e2a4d32c643 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 21 May 2026 11:57:02 +0200 Subject: [PATCH 4/6] Extract BranchesHelper.PostFetchRefresh to unify the two fetch paths The post-fetch logic was duplicated in `backgroundFetch` and the manual fetch handler: refresh a fixed set of views, then auto-forward branches if the fetch succeeded. The two had already drifted on the refresh scope; folding them into a single helper makes the duplication go away and prevents it from drifting again. Pass the fetch error through so we preserve the previous behaviour of refreshing unconditionally but only auto-forwarding on success. Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/gui/background.go | 8 +------- pkg/gui/controllers/files_controller.go | 8 +------- pkg/gui/controllers/helpers/branches_helper.go | 13 +++++++++++++ 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/pkg/gui/background.go b/pkg/gui/background.go index cedc6a78c..8795b49aa 100644 --- a/pkg/gui/background.go +++ b/pkg/gui/background.go @@ -155,13 +155,7 @@ func (self *BackgroundRoutineMgr) goEvery(interval time.Duration, stop chan stru func (self *BackgroundRoutineMgr) backgroundFetch() (err error) { err = self.gui.git.Sync.FetchBackground() - self.gui.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.BRANCHES, types.COMMITS, types.REMOTES, types.TAGS, types.PULL_REQUESTS}, Mode: types.SYNC}) - - if err == nil { - err = self.gui.helpers.BranchesHelper.AutoForwardBranches() - } - - return err + return self.gui.helpers.BranchesHelper.PostFetchRefresh(err) } func (self *BackgroundRoutineMgr) triggerImmediateFetch() { diff --git a/pkg/gui/controllers/files_controller.go b/pkg/gui/controllers/files_controller.go index 1513a324e..c8d50d54d 100644 --- a/pkg/gui/controllers/files_controller.go +++ b/pkg/gui/controllers/files_controller.go @@ -1348,13 +1348,7 @@ func (self *FilesController) fetch() error { return errors.New(self.c.Tr.PassUnameWrong) } - self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.BRANCHES, types.COMMITS, types.REMOTES, types.TAGS, types.PULL_REQUESTS}, Mode: types.SYNC}) - - if err == nil { - err = self.c.Helpers().BranchesHelper.AutoForwardBranches() - } - - return err + return self.c.Helpers().BranchesHelper.PostFetchRefresh(err) }) } diff --git a/pkg/gui/controllers/helpers/branches_helper.go b/pkg/gui/controllers/helpers/branches_helper.go index c3f4242bf..a53bf2181 100644 --- a/pkg/gui/controllers/helpers/branches_helper.go +++ b/pkg/gui/controllers/helpers/branches_helper.go @@ -285,6 +285,19 @@ func (self *BranchesHelper) deleteRemoteBranches(remoteBranches []*models.Remote return nil } +func (self *BranchesHelper) PostFetchRefresh(fetchErr error) error { + self.c.Refresh(types.RefreshOptions{ + Scope: []types.RefreshableView{ + types.BRANCHES, types.COMMITS, types.REMOTES, types.TAGS, types.PULL_REQUESTS, + }, + Mode: types.SYNC, + }) + if fetchErr != nil { + return fetchErr + } + return self.AutoForwardBranches() +} + func (self *BranchesHelper) AutoForwardBranches() error { if self.c.UserConfig().Git.AutoForwardBranches == "none" { return nil From 532cce4873737a5d32c68d48a83bab098b124cb4 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 21 May 2026 12:30:36 +0200 Subject: [PATCH 5/6] Add test for auto-forwarding a branch checked out in another worktree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the linked worktree's branch is changed externally — by another shell, by another tool, or by git running outside lazygit — lazygit's worktrees model goes stale. The next post-fetch auto-forward then doesn't realise the branch is now checked out elsewhere, and advances its ref behind the worktree's back. The worktree's HEAD then resolves to a commit its index/working tree haven't been updated to, and the user sees that diff as the inverse of what was fetched — files appearing as pending changes that they didn't make. The test sets up a linked worktree initially on a side branch, then externally checks out master in it before pressing fetch. Two EXPECTED/ACTUAL pairs capture the symptoms: the branches view shows master as `✓` rather than `↓1`, and switching to the linked worktree shows master's would-be incoming file as a pending deletion against HEAD. Co-Authored-By: Claude Opus 4.7 (1M context) --- ...d_branches_worktree_added_after_startup.go | 68 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 2 files changed, 69 insertions(+) create mode 100644 pkg/integration/tests/sync/fetch_and_auto_forward_branches_worktree_added_after_startup.go diff --git a/pkg/integration/tests/sync/fetch_and_auto_forward_branches_worktree_added_after_startup.go b/pkg/integration/tests/sync/fetch_and_auto_forward_branches_worktree_added_after_startup.go new file mode 100644 index 000000000..61cb0fd9f --- /dev/null +++ b/pkg/integration/tests/sync/fetch_and_auto_forward_branches_worktree_added_after_startup.go @@ -0,0 +1,68 @@ +package sync + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var FetchAndAutoForwardBranchesWorktreeAddedAfterStartup = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Auto-forward skips a main branch that was externally checked out in a linked worktree after lazygit started", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Git.AutoForwardBranches = "onlyMainBranches" + config.GetUserConfig().Git.LocalBranchSortOrder = "alphabetical" + }, + SetupRepo: func(shell *Shell) { + shell.CreateNCommits(3) + shell.NewBranch("feature") + shell.NewBranch("wt-branch") + shell.CloneIntoRemote("origin") + shell.SetBranchUpstream("master", "origin/master") + shell.SetBranchUpstream("feature", "origin/feature") + shell.Checkout("master") + shell.HardReset("HEAD^") + shell.Checkout("feature") + shell.AddWorktreeCheckout("wt-branch", "../linked-worktree") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Branches(). + Lines( + Contains("feature").IsSelected(), + Contains("master ↓1").DoesNotContain("↑"), + Contains("wt-branch (worktree linked-worktree)"), + ) + + // Switch the linked worktree to master externally. + t.Shell().RunCommand([]string{"git", "-C", "../linked-worktree", "checkout", "master"}) + + t.Views().Files(). + IsFocused(). + Press(keys.Files.Fetch) + + t.Views().Branches(). + Lines( + Contains("feature").IsSelected(), + /* EXPECTED: + Contains("master (worktree linked-worktree) ↓1"), + Contains("wt-branch").DoesNotContain("worktree"), + ACTUAL: */ + Contains("master ✓"), + Contains("wt-branch (worktree linked-worktree)"), + ) + + t.Views().Worktrees(). + Focus(). + NavigateToLine(Contains("linked-worktree")). + PressPrimaryAction() + + t.Views().Files(). + Focus(). + /* EXPECTED: + IsEmpty() + ACTUAL: */ + Lines( + Equals("D file03.txt"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 4b96c4d42..fdccb08bf 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -428,6 +428,7 @@ var tests = []*components.IntegrationTest{ sync.FetchAndAutoForwardBranchesAllBranchesCheckedOutInOtherWorktree, sync.FetchAndAutoForwardBranchesNone, sync.FetchAndAutoForwardBranchesOnlyMainBranches, + sync.FetchAndAutoForwardBranchesWorktreeAddedAfterStartup, sync.FetchPrune, sync.FetchWhenSortedByDate, sync.ForcePush, From 4f6cdedb1e5ea8c13d0ccbf57d772a387ef5728f Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 21 May 2026 12:31:09 +0200 Subject: [PATCH 6/6] Refresh worktrees before auto-forwarding branches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AutoForwardBranches relies on the worktree model to skip any branch that's currently checked out in another worktree (so we don't update its ref behind the worktree's back). The post-fetch refresh wasn't including the worktrees scope, so any external change to the worktree list between lazygit's startup and the fetch — a `git worktree add`, a `git checkout` in a linked worktree, a branch rename — left the in-memory model stale and the skip check returned false negatives. Add WORKTREES to the post-fetch refresh scope when auto-forwarding is enabled. We gate on the config so users with auto-forward disabled don't pay for an extra `git worktree list` plus per-worktree rev-parse on every fetch tick. Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/gui/controllers/helpers/branches_helper.go | 14 ++++++++------ ...orward_branches_worktree_added_after_startup.go | 9 --------- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/pkg/gui/controllers/helpers/branches_helper.go b/pkg/gui/controllers/helpers/branches_helper.go index a53bf2181..8af447f79 100644 --- a/pkg/gui/controllers/helpers/branches_helper.go +++ b/pkg/gui/controllers/helpers/branches_helper.go @@ -286,12 +286,14 @@ func (self *BranchesHelper) deleteRemoteBranches(remoteBranches []*models.Remote } func (self *BranchesHelper) PostFetchRefresh(fetchErr error) error { - self.c.Refresh(types.RefreshOptions{ - Scope: []types.RefreshableView{ - types.BRANCHES, types.COMMITS, types.REMOTES, types.TAGS, types.PULL_REQUESTS, - }, - Mode: types.SYNC, - }) + scope := []types.RefreshableView{ + types.BRANCHES, types.COMMITS, types.REMOTES, types.TAGS, types.PULL_REQUESTS, + } + // AutoForwardBranches needs a fresh worktree model to skip branches that are checked out elsewhere. + if self.c.UserConfig().Git.AutoForwardBranches != "none" { + scope = append(scope, types.WORKTREES) + } + self.c.Refresh(types.RefreshOptions{Scope: scope, Mode: types.SYNC}) if fetchErr != nil { return fetchErr } diff --git a/pkg/integration/tests/sync/fetch_and_auto_forward_branches_worktree_added_after_startup.go b/pkg/integration/tests/sync/fetch_and_auto_forward_branches_worktree_added_after_startup.go index 61cb0fd9f..bee14276a 100644 --- a/pkg/integration/tests/sync/fetch_and_auto_forward_branches_worktree_added_after_startup.go +++ b/pkg/integration/tests/sync/fetch_and_auto_forward_branches_worktree_added_after_startup.go @@ -43,12 +43,8 @@ var FetchAndAutoForwardBranchesWorktreeAddedAfterStartup = NewIntegrationTest(Ne t.Views().Branches(). Lines( Contains("feature").IsSelected(), - /* EXPECTED: Contains("master (worktree linked-worktree) ↓1"), Contains("wt-branch").DoesNotContain("worktree"), - ACTUAL: */ - Contains("master ✓"), - Contains("wt-branch (worktree linked-worktree)"), ) t.Views().Worktrees(). @@ -58,11 +54,6 @@ var FetchAndAutoForwardBranchesWorktreeAddedAfterStartup = NewIntegrationTest(Ne t.Views().Files(). Focus(). - /* EXPECTED: IsEmpty() - ACTUAL: */ - Lines( - Equals("D file03.txt"), - ) }, })