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 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 e75a5ee3f..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}, 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..8af447f79 100644 --- a/pkg/gui/controllers/helpers/branches_helper.go +++ b/pkg/gui/controllers/helpers/branches_helper.go @@ -285,6 +285,21 @@ func (self *BranchesHelper) deleteRemoteBranches(remoteBranches []*models.Remote return nil } +func (self *BranchesHelper) PostFetchRefresh(fetchErr error) error { + 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 + } + return self.AutoForwardBranches() +} + func (self *BranchesHelper) AutoForwardBranches() error { if self.c.UserConfig().Git.AutoForwardBranches == "none" { return nil 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() { 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..bee14276a --- /dev/null +++ b/pkg/integration/tests/sync/fetch_and_auto_forward_branches_worktree_added_after_startup.go @@ -0,0 +1,59 @@ +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(), + Contains("master (worktree linked-worktree) ↓1"), + Contains("wt-branch").DoesNotContain("worktree"), + ) + + t.Views().Worktrees(). + Focus(). + NavigateToLine(Contains("linked-worktree")). + PressPrimaryAction() + + t.Views().Files(). + Focus(). + IsEmpty() + }, +}) 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,