From f7a61443fa832d47bfec4bb9726a7cc52eb5f19f Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 3 Jul 2026 15:56:01 +0200 Subject: [PATCH] Bounce BRANCHES model updates onto the UI thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refreshBranches now loads the branches (and worktrees) on the worker and writes Model.Branches, the pull-requests map, Model.Worktrees, and the restored branch selection in an onUIThreadUnlessRepoChanged bounce. The selection restore and rebuildPullRequestsMap run in the bounce so they see the branches we just wrote; the LocalCommits re-render (for branch head visualization) moves into the same bounce. refreshStatus is adjusted to read the checked-out branch and the linked worktree name inside its bounce rather than on the worker: both derive from models (Branches, Worktrees) that are now written via bounces, so reading them on the worker would format the status from stale values — which showed up as the status line dropping the "(worktree)" suffix right after entering a submodule or switching worktrees. The git work (WorkingTreeState) stays on the worker. Two callers that read the branches model right after a SYNC branches refresh move their reads into Then: - BranchesHelper.PostFetchRefresh: AutoForwardBranches reads Model.Branches, so it runs in Then (preserving that a fetch error is still returned to the caller and that background auto-forward errors aren't surfaced as a popup). - BranchesController rename: the re-select-by-name loop runs in Then. RefreshingBranchesMutex is left in place for the mutex cleanup. Co-Authored-By: Claude Sonnet 5 --- pkg/gui/controllers/branches_controller.go | 23 ++++--- .../controllers/helpers/branches_helper.go | 27 ++++++-- pkg/gui/controllers/helpers/refresh_helper.go | 68 +++++++++++-------- 3 files changed, 74 insertions(+), 44 deletions(-) diff --git a/pkg/gui/controllers/branches_controller.go b/pkg/gui/controllers/branches_controller.go index 45f98e9c5..131d19439 100644 --- a/pkg/gui/controllers/branches_controller.go +++ b/pkg/gui/controllers/branches_controller.go @@ -783,20 +783,25 @@ func (self *BranchesController) rename(branch *models.Branch) error { return err } - // need to find where the branch is now so that we can re-select it. That means we need to refetch the branches synchronously and then find our branch + // need to find where the branch is now so that we can re-select it. That means we need to + // refetch the branches and then find our branch. The branches model update is bounced + // onto the UI thread, so the re-selection (which reads Model.Branches) has to run in + // Then; reading it inline here would see the previous model. self.c.Refresh(types.RefreshOptions{ Mode: types.SYNC, Scope: []types.RefreshableView{types.BRANCHES, types.WORKTREES}, + Then: func() error { + // now that we've got our stuff again we need to find that branch and reselect it. + for i, newBranch := range self.c.Model().Branches { + if newBranch.Name == newBranchName { + self.context().SetSelection(i) + self.context().HandleRender() + } + } + return nil + }, }) - // now that we've got our stuff again we need to find that branch and reselect it. - for i, newBranch := range self.c.Model().Branches { - if newBranch.Name == newBranchName { - self.context().SetSelection(i) - self.context().HandleRender() - } - } - return nil }, }) diff --git a/pkg/gui/controllers/helpers/branches_helper.go b/pkg/gui/controllers/helpers/branches_helper.go index 4283bd29a..e5c1a07e4 100644 --- a/pkg/gui/controllers/helpers/branches_helper.go +++ b/pkg/gui/controllers/helpers/branches_helper.go @@ -387,11 +387,28 @@ func (self *BranchesHelper) PostFetchRefresh(fetchErr error, background bool) er if self.c.UserConfig().Git.AutoForwardBranches != "none" { scope = append(scope, types.WORKTREES) } - self.c.Refresh(types.RefreshOptions{Scope: scope, Mode: types.SYNC, Background: background}) - if fetchErr != nil { - return fetchErr - } - return self.AutoForwardBranches() + // AutoForwardBranches reads Model.Branches, which the branches refresh writes + // via a bounce, so it has to run in Then rather than right after Refresh + // returns (where it would still see the previous branches). + self.c.Refresh(types.RefreshOptions{ + Scope: scope, + Mode: types.SYNC, + Background: background, + Then: func() error { + if fetchErr != nil { + return nil + } + err := self.AutoForwardBranches() + if background && err != nil { + // The background poller discards this return value, so surface + // the error in the log rather than as a popup for background work. + self.c.Log.Error(err) + return nil + } + return err + }, + }) + return fetchErr } func (self *BranchesHelper) AutoForwardBranches() error { diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index a86e91111..876ce0f30 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -742,6 +742,8 @@ func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, keepBranchSele self.c.Mutexes().RefreshingBranchesMutex.Lock() defer self.c.Mutexes().RefreshingBranchesMutex.Unlock() + generation := self.c.State().GetRepoGeneration() + branches, err := self.c.Git().Loaders.BranchLoader.Load( reflogCommits, self.c.Model().MainBranches, @@ -753,7 +755,7 @@ func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, keepBranchSele }) }, func() { - self.c.OnUIThread(func() error { + self.onUIThreadUnlessRepoChanged(generation, func() error { self.c.Contexts().Branches.HandleRender() self.refreshStatus() return nil @@ -765,38 +767,42 @@ func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, keepBranchSele prevSelectedBranch := self.c.Contexts().Branches.GetSelected() - self.c.Model().Branches = branches - self.rebuildPullRequestsMap() - + var worktrees []*models.Worktree if refreshWorktrees { - // TODO: this synchronous worker write goes away when refreshBranches is - // itself migrated to bouncing; for now it matches the rest of this - // not-yet-bounced function. - self.c.Model().Worktrees = self.loadWorktrees() - self.refreshView(self.c.Contexts().Worktrees) + worktrees = self.loadWorktrees() } - if !keepBranchSelectionIndex && prevSelectedBranch != nil { - self.searchHelper.ReApplyFilter(self.c.Contexts().Branches) + self.onUIThreadUnlessRepoChanged(generation, func() error { + self.c.Model().Branches = branches + // Rebuilding here (rather than on the worker) means the map is built from + // the branches we just wrote, on the UI thread. + self.rebuildPullRequestsMap() - _, idx, found := lo.FindIndexOf(self.c.Contexts().Branches.GetItems(), - func(b *models.Branch) bool { return b.Name == prevSelectedBranch.Name }) - if found { - self.c.Contexts().Branches.SetSelectedLineIdx(idx) + if refreshWorktrees { + self.c.Model().Worktrees = worktrees + self.refreshView(self.c.Contexts().Worktrees) } - } - self.refreshView(self.c.Contexts().Branches) + if !keepBranchSelectionIndex && prevSelectedBranch != nil { + self.searchHelper.ReApplyFilter(self.c.Contexts().Branches) - // Need to re-render the commits view because the visualization of local - // branch heads might have changed - self.c.OnUIThread(func() error { + _, idx, found := lo.FindIndexOf(self.c.Contexts().Branches.GetItems(), + func(b *models.Branch) bool { return b.Name == prevSelectedBranch.Name }) + if found { + self.c.Contexts().Branches.SetSelectedLineIdx(idx) + } + } + + // Need to re-render the commits view because the visualization of local + // branch heads might have changed self.c.Mutexes().LocalCommitsMutex.Lock() self.c.Contexts().LocalCommits.HandleRender() self.c.Mutexes().LocalCommitsMutex.Unlock() return nil }) + self.refreshView(self.c.Contexts().Branches) + self.refreshStatus() } @@ -1074,20 +1080,22 @@ func (self *RefreshHelper) refreshStatus() { generation := self.c.State().GetRepoGeneration() - currentBranch := self.refsHelper.GetCheckedOutRef() - if currentBranch == nil { - // need to wait for branches to refresh - return - } - workingTreeState := self.c.Git().Status.WorkingTreeState() - linkedWorktreeName := self.worktreeHelper.GetLinkedWorktreeName() - repoName := self.c.Git().RepoPaths.RepoName() - status := presentation.FormatStatus(repoName, currentBranch, types.ItemOperationNone, linkedWorktreeName, workingTreeState, self.c.Tr, self.c.UserConfig()) - self.onUIThreadUnlessRepoChanged(generation, func() error { + // Read the checked-out branch and the linked worktree name here on the UI + // thread: both derive from models (Branches, Worktrees) that their + // refreshes now write via bounces, so reading them on the worker would + // see stale values from before those bounces applied. + currentBranch := self.refsHelper.GetCheckedOutRef() + if currentBranch == nil { + // need to wait for branches to refresh + return nil + } + linkedWorktreeName := self.worktreeHelper.GetLinkedWorktreeName() + + status := presentation.FormatStatus(repoName, currentBranch, types.ItemOperationNone, linkedWorktreeName, workingTreeState, self.c.Tr, self.c.UserConfig()) self.c.SetViewContent(self.c.Views().Status, status) return nil })