From 558fd2c9d349b597932483c3b81c66a3c1c21281 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 6 Jul 2026 14:46:32 +0200 Subject: [PATCH] Route merge/rebase result handling to the right refresh entry point MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CheckMergeOrRebaseWithRefreshOptions refreshes after a merge/rebase step, and until now always via the UI-thread Refresh. Most of its callers are on a worker (the WithWaitingStatus/WithInlineStatus merge, squash-merge, rebase, pull, amend, drop, and patch-move handlers), so that refresh reads the commits scope off the UI thread — the race the previous commit addresses for everything else. Split it: the default is for worker callers and refreshes via RefreshFromWorker; a new CheckMergeOrRebaseWithRefreshOptionsFromUIThread is for the handlers that run the step synchronously on the UI thread (WithWaitingStatusSync, kept sync so rapid key presses batch): move up/down, revert, squash-fixups, cherry-pick paste, and patch-discard. The two share a private impl carrying which thread the caller is on, and the auto-skip recursion (genericMergeCommandImpl for an empty commit) threads it through so the follow-up step refreshes on the same thread. The merge-and-commit refresh in SquashMergeCommitted, also on a worker, moves to RefreshFromWorker to match. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../controllers/helpers/cherry_pick_helper.go | 2 +- .../helpers/merge_and_rebase_helper.go | 66 ++++++++++++++----- .../controllers/local_commits_controller.go | 8 +-- .../controllers/patch_building_controller.go | 2 +- 4 files changed, 56 insertions(+), 22 deletions(-) diff --git a/pkg/gui/controllers/helpers/cherry_pick_helper.go b/pkg/gui/controllers/helpers/cherry_pick_helper.go index e2fe46545..673f657f5 100644 --- a/pkg/gui/controllers/helpers/cherry_pick_helper.go +++ b/pkg/gui/controllers/helpers/cherry_pick_helper.go @@ -95,7 +95,7 @@ func (self *CherryPickHelper) Paste() error { cherryPickedCommits := self.getData().CherryPickedCommits result := self.c.Git().Rebase.CherryPickCommits(cherryPickedCommits) - err := self.rebaseHelper.CheckMergeOrRebaseWithRefreshOptions(result, types.RefreshOptions{Mode: types.SYNC}) + err := self.rebaseHelper.CheckMergeOrRebaseWithRefreshOptionsFromUIThread(result, types.RefreshOptions{Mode: types.SYNC}) if err != nil { return result } diff --git a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go index 51488a922..6adf84712 100644 --- a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go +++ b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go @@ -79,7 +79,9 @@ func (self *MergeAndRebaseHelper) ContinueRebase() error { } func (self *MergeAndRebaseHelper) genericMergeCommand(command string) error { - return self.genericMergeCommandImpl(command, true) + // The menu/prompt/confirm handlers that reach here run on the UI thread and + // spin up a worker (via the waiting status below) to do the actual work. + return self.genericMergeCommandImpl(command, true, false) } // genericMergeCommandImpl runs a merge/rebase continue/skip/abort and handles @@ -87,10 +89,12 @@ func (self *MergeAndRebaseHelper) genericMergeCommand(command string) error { // non-subprocess path runs on a worker with a waiting status. // // showWaitingStatus is false only for the recursive auto-skip in -// CheckMergeOrRebaseWithRefreshOptions: that call already runs on the caller's -// thread (the worker of the enclosing waiting status, or the UI thread for the -// synchronous callers), so it must not spin up a second one. -func (self *MergeAndRebaseHelper) genericMergeCommandImpl(command string, showWaitingStatus bool) error { +// checkMergeOrRebaseImpl: that call already runs on the caller's thread (the +// worker of the enclosing waiting status, or the UI thread for the synchronous +// callers), so it must not spin up a second one. calledFromWorker says which of +// those two the body runs on, so the post-action refresh picks Refresh vs +// RefreshFromWorker correctly. +func (self *MergeAndRebaseHelper) genericMergeCommandImpl(command string, showWaitingStatus bool, calledFromWorker bool) error { status := self.c.Git().Status.WorkingTreeState() if status.None() { @@ -128,29 +132,30 @@ func (self *MergeAndRebaseHelper) genericMergeCommandImpl(command string, showWa if needsSubprocess { // TODO: see if we should be calling more of the code from self.Git.Rebase.GenericMergeOrRebaseAction success, err := self.c.RunSubprocess(self.c.Git().Rebase.GenericMergeOrRebaseActionCmdObj(commandType, command)) - self.c.Refresh(types.RefreshOptions{ + self.refreshAfterMergeOrRebase(types.RefreshOptions{ Mode: types.ASYNC, CommitSelection: commitSelectionAfterMerge(success && selectHeadCommitOnSuccess), - }) + }, calledFromWorker) self.RecordWhetherMergeOrRebaseStartedInLazygit() return err } - runAction := func() error { + runAction := func(calledFromWorker bool) error { result := self.c.Git().Rebase.GenericMergeOrRebaseAction(commandType, command) - return self.CheckMergeOrRebaseWithRefreshOptions(result, + return self.checkMergeOrRebaseImpl(result, types.RefreshOptions{ Mode: types.ASYNC, CommitSelection: commitSelectionAfterMerge(result == nil && selectHeadCommitOnSuccess), - }) + }, calledFromWorker) } if showWaitingStatus { return self.c.WithWaitingStatus(status.Title(self.c.Tr), func(gocui.Task) error { - return runAction() + // The waiting status ran runAction on a worker. + return runAction(true) }) } - return runAction() + return runAction(calledFromWorker) } // commitSelectionAfterMerge maps whether a merge/rebase/pull created a new @@ -205,17 +210,34 @@ func (self *MergeAndRebaseHelper) RecordWhetherMergeOrRebaseStartedInLazygit() { self.c.Git().Status.WorkingTreeState().Any()) } +// CheckMergeOrRebaseWithRefreshOptions handles the result of a merge/rebase +// step and refreshes. It's for callers running on a worker (the +// WithWaitingStatus / WithInlineStatus handlers), which is the large majority; +// UI-thread callers use CheckMergeOrRebaseWithRefreshOptionsFromUIThread. func (self *MergeAndRebaseHelper) CheckMergeOrRebaseWithRefreshOptions(result error, refreshOptions types.RefreshOptions) error { - self.c.Refresh(refreshOptions) + return self.checkMergeOrRebaseImpl(result, refreshOptions, true) +} + +// CheckMergeOrRebaseWithRefreshOptionsFromUIThread is like +// CheckMergeOrRebaseWithRefreshOptions, but for the callers that run the +// merge/rebase synchronously on the UI thread (the WithWaitingStatusSync +// move/revert/squash-fixups/cherry-pick-paste/patch-discard handlers, kept sync +// so rapid key presses batch) rather than on a worker. +func (self *MergeAndRebaseHelper) CheckMergeOrRebaseWithRefreshOptionsFromUIThread(result error, refreshOptions types.RefreshOptions) error { + return self.checkMergeOrRebaseImpl(result, refreshOptions, false) +} + +func (self *MergeAndRebaseHelper) checkMergeOrRebaseImpl(result error, refreshOptions types.RefreshOptions, calledFromWorker bool) error { + self.refreshAfterMergeOrRebase(refreshOptions, calledFromWorker) self.RecordWhetherMergeOrRebaseStartedInLazygit() if result == nil { return nil } else if strings.Contains(result.Error(), "No changes - did you forget to use") { - return self.genericMergeCommandImpl(REBASE_OPTION_SKIP, false) + return self.genericMergeCommandImpl(REBASE_OPTION_SKIP, false, calledFromWorker) } else if strings.Contains(result.Error(), "The previous cherry-pick is now empty") { - return self.genericMergeCommandImpl(REBASE_OPTION_SKIP, false) + return self.genericMergeCommandImpl(REBASE_OPTION_SKIP, false, calledFromWorker) } else if strings.Contains(result.Error(), "No rebase in progress?") { // assume in this case that we're already done return nil @@ -223,6 +245,18 @@ func (self *MergeAndRebaseHelper) CheckMergeOrRebaseWithRefreshOptions(result er return self.CheckForConflicts(result) } +// refreshAfterMergeOrRebase issues the post-action refresh on the entry point +// that matches the thread the merge/rebase ran on: RefreshFromWorker for the +// worker callers, Refresh for the ones that stayed synchronously on the UI +// thread. +func (self *MergeAndRebaseHelper) refreshAfterMergeOrRebase(refreshOptions types.RefreshOptions, calledFromWorker bool) { + if calledFromWorker { + self.c.RefreshFromWorker(refreshOptions) + } else { + self.c.Refresh(refreshOptions) + } +} + func (self *MergeAndRebaseHelper) CheckMergeOrRebase(result error) error { return self.CheckMergeOrRebaseWithRefreshOptions(result, types.RefreshOptions{Mode: types.ASYNC}) } @@ -628,7 +662,7 @@ func (self *MergeAndRebaseHelper) SquashMergeCommitted(refName, checkedOutBranch if err != nil { return err } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.c.RefreshFromWorker(types.RefreshOptions{Mode: types.ASYNC}) return nil }) } diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 04c7fc290..12244d983 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -741,7 +741,7 @@ func (self *LocalCommitsController) moveDown(selectedCommits []*models.Commit, s self.context().MoveSelection(1) self.context().HandleFocus(types.OnFocusOpts{ScrollSelectionIntoView: true}) } - return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions( + return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptionsFromUIThread( err, types.RefreshOptions{Mode: types.SYNC, CommitSelection: types.KeepCommitSelectionIndex}) }) } @@ -769,7 +769,7 @@ func (self *LocalCommitsController) moveUp(selectedCommits []*models.Commit, sta self.context().MoveSelection(-1) self.context().HandleFocus(types.OnFocusOpts{ScrollSelectionIntoView: true}) } - return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions( + return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptionsFromUIThread( err, types.RefreshOptions{Mode: types.SYNC, CommitSelection: types.KeepCommitSelectionIndex}) }) } @@ -927,7 +927,7 @@ func (self *LocalCommitsController) revert(commits []*models.Commit, start, end } result := self.c.Git().Commit.Revert(hashes, isMerge) - if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions(result, types.RefreshOptions{Mode: types.SYNC}); err != nil { + if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptionsFromUIThread(result, types.RefreshOptions{Mode: types.SYNC}); err != nil { return err } @@ -1127,7 +1127,7 @@ func (self *LocalCommitsController) squashFixupsImpl(commit *models.Commit, reba self.c.LogAction(self.c.Tr.Actions.SquashAllAboveFixupCommits) err := self.c.Git().Rebase.SquashAllAboveFixupCommits(commit) self.context().MoveSelectedLine(-selectionOffset) - return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions( + return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptionsFromUIThread( err, types.RefreshOptions{Mode: types.SYNC}) }) } diff --git a/pkg/gui/controllers/patch_building_controller.go b/pkg/gui/controllers/patch_building_controller.go index 5e4a17169..d596c2ead 100644 --- a/pkg/gui/controllers/patch_building_controller.go +++ b/pkg/gui/controllers/patch_building_controller.go @@ -228,7 +228,7 @@ func (self *PatchBuildingController) discardSelectionFromCommit() error { self.c.LogAction(self.c.Tr.Actions.RemovePatchFromCommit) err := self.c.Git().Patch.DeletePatchesFromCommit(self.c.Model().Commits, commitIndex) self.c.Helpers().PatchBuilding.Escape() - return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions( + return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptionsFromUIThread( err, types.RefreshOptions{Mode: types.SYNC}) }) }