From 775ebc9e44828e7672590df8e04b033c20fbc201 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 7 Jul 2026 17:16:45 +0200 Subject: [PATCH] Re-render to clear an inline status when its operation finishes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Operations that show an inline status ("Pushing", "Fast-forwarding", "Fetching", …) removed it by relying on the async refresh they trigger to redraw the view after the item operation had been cleared. That ordering was never guaranteed: the item operation is cleared on the worker once the operation's function returns, while the refresh redraws the item from the UI thread whenever its (asynchronous) git work happens to finish. If the refresh redrew before the clear, the status was left on screen with no later redraw to remove it, so the branch (or tag/remote) stayed stuck showing e.g. "Pushing" indefinitely even though the operation had completed. This is timing-dependent, which is why it surfaced as rare, hard-to-reproduce reports and as flaky CI failures. Fix it by re-rendering in stop() right after clearing the operation, and by making these refreshes synchronous rather than async. Because a synchronous refresh has already updated the model and queued its own redraw by the time stop() runs, and UI-thread callbacks run in order, the redraw we queue here runs last and draws the up-to-date model with the status removed. An async refresh couldn't give that guarantee: its model update might not have landed yet, so the redraw could briefly flash the pre-operation status. Pull refreshes through the shared CheckMergeOrRebaseAndSelectHeadCommit, so that helper becomes synchronous too; its only other caller, RegularMerge, thereby also refreshes synchronously, which is fine: a synchronous on-worker refresh is what we want anyway. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/gui/controllers/branches_controller.go | 4 +-- .../helpers/inline_status_helper.go | 28 ++++++++----------- .../helpers/merge_and_rebase_helper.go | 2 +- pkg/gui/controllers/remotes_controller.go | 2 +- pkg/gui/controllers/sync_controller.go | 2 +- pkg/gui/controllers/tags_controller.go | 4 +-- 6 files changed, 19 insertions(+), 23 deletions(-) diff --git a/pkg/gui/controllers/branches_controller.go b/pkg/gui/controllers/branches_controller.go index 27bef4b66..45f98e9c5 100644 --- a/pkg/gui/controllers/branches_controller.go +++ b/pkg/gui/controllers/branches_controller.go @@ -734,7 +734,7 @@ func (self *BranchesController) fastForward(branch *models.Branch) error { WorktreePath: worktreePath, }, ) - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.Refresh(types.RefreshOptions{Mode: types.SYNC}) return err } @@ -743,7 +743,7 @@ func (self *BranchesController) fastForward(branch *models.Branch) error { err := self.c.Git().Sync.FastForward( task, branch.Name, branch.UpstreamRemote, branch.UpstreamBranch, ) - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.BRANCHES}}) + self.c.Refresh(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.BRANCHES}}) return err }) } diff --git a/pkg/gui/controllers/helpers/inline_status_helper.go b/pkg/gui/controllers/helpers/inline_status_helper.go index 0902c5bf2..f2ca7ec17 100644 --- a/pkg/gui/controllers/helpers/inline_status_helper.go +++ b/pkg/gui/controllers/helpers/inline_status_helper.go @@ -138,22 +138,18 @@ func (self *InlineStatusHelper) stop(opts InlineStatusOpts) { self.c.State().ClearItemOperation(opts.Item) - // When recording a demo we need to re-render the context again here to - // remove the inline status. In normal usage we don't want to do this - // because in the case of pushing a branch this would first reveal the ↑3↓7 - // status from before the push for a brief moment, to be replaced by a green - // checkmark a moment later when the async refresh is done. This looks - // jarring, so normally we rely on the async refresh to redraw with the - // status removed. (In some rare cases, where there's no refresh at all, we - // need to redraw manually in the controller; see TagsController.push() for - // an example.) - // - // In demos, however, we turn all async refreshes into sync ones, because - // this looks better in demos. In this case the refresh happens while the - // status is still set, so we need to render again after removing it. - if self.c.InDemo() { - self.renderContext(opts.ContextKey) - } + // Re-render the context to remove the inline status now that the operation + // finished. Any refresh it triggered must be synchronous, not async: by the + // time we get here a synchronous refresh has already updated the model and + // queued its own re-render, and since UI-thread callbacks run in order, the + // render we queue here runs after it and draws the up-to-date model without + // the inline status. An async refresh might not have updated the model yet, + // so this render could briefly show the stale, pre-operation model: when + // pushing a branch, for example, it would flash the old ↑3↓7 ahead/behind + // counts for a moment before the refresh replaced them with a green + // checkmark. (Operations that don't refresh at all are fine too: there's + // nothing stale to show, so this just drops the status.) + self.renderContext(opts.ContextKey) } func (self *InlineStatusHelper) renderContext(contextKey types.ContextKey) { diff --git a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go index 4b25d1a62..847b89893 100644 --- a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go +++ b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go @@ -233,7 +233,7 @@ func (self *MergeAndRebaseHelper) CheckMergeOrRebase(result error) error { // before the refresh. func (self *MergeAndRebaseHelper) CheckMergeOrRebaseAndSelectHeadCommit(result error) error { return self.CheckMergeOrRebaseWithRefreshOptions(result, - types.RefreshOptions{Mode: types.ASYNC, CommitSelection: commitSelectionAfterMerge(result == nil)}) + types.RefreshOptions{Mode: types.SYNC, CommitSelection: commitSelectionAfterMerge(result == nil)}) } func (self *MergeAndRebaseHelper) CheckForConflicts(result error) error { diff --git a/pkg/gui/controllers/remotes_controller.go b/pkg/gui/controllers/remotes_controller.go index dd5a171e4..f7b16e228 100644 --- a/pkg/gui/controllers/remotes_controller.go +++ b/pkg/gui/controllers/remotes_controller.go @@ -367,7 +367,7 @@ func (self *RemotesController) fetchAndCheckout(remote *models.Remote, branchNam } refreshOptions := types.RefreshOptions{ Scope: []types.RefreshableView{types.BRANCHES, types.REMOTES}, - Mode: types.ASYNC, + Mode: types.SYNC, } if branchName != "" { err = self.c.Git().Branch.New(branchName, remote.Name+"/"+branchName) diff --git a/pkg/gui/controllers/sync_controller.go b/pkg/gui/controllers/sync_controller.go index f1b794e97..0f754eb49 100644 --- a/pkg/gui/controllers/sync_controller.go +++ b/pkg/gui/controllers/sync_controller.go @@ -229,7 +229,7 @@ func (self *SyncController) pushAux(currentBranch *models.Branch, opts pushOpts) } return err } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.Refresh(types.RefreshOptions{Mode: types.SYNC}) return nil }) } diff --git a/pkg/gui/controllers/tags_controller.go b/pkg/gui/controllers/tags_controller.go index 879a73628..3cb5e0450 100644 --- a/pkg/gui/controllers/tags_controller.go +++ b/pkg/gui/controllers/tags_controller.go @@ -210,7 +210,7 @@ func (self *TagsController) remoteDelete(tag *models.Tag) error { return err } self.c.Toast(self.c.Tr.RemoteTagDeletedMessage) - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.COMMITS, types.TAGS}}) + self.c.Refresh(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.COMMITS, types.TAGS}}) return nil }) }, @@ -264,7 +264,7 @@ func (self *TagsController) localAndRemoteDelete(tag *models.Tag) error { if err := self.c.Git().Tag.LocalDelete(tag.Name); err != nil { return err } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: []types.RefreshableView{types.COMMITS, types.TAGS}}) + self.c.Refresh(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.COMMITS, types.TAGS}}) return nil }) },