We don't want callers to need any additional logic, so pass in the
translation set so that the function can decide what static text to
return. This allows us to get rid of the CurrentPagerUsesGitConfigDiff
method which is in the way for the refactoring we're about to do.
Moving commits runs a rebase, which can take a while. Instead of
letting the drop indicator vanish the moment the button is released,
keep it in place and turn it into a "moving commits here" spinner once
the move takes longer than a short grace period, so that quick moves
stay free of flicker. The indicator is cleared when the post-move
refresh lands.
While a commit drag is in progress, escape now aborts it: the drag
state and the drop indicator are discarded and the mouse capture is
released, so nothing happens when the button is eventually released.
Otherwise escape keeps its normal meaning.
Reuse the drag autoscroller for commit drags. Scrolling stops once the
insertion point reaches the end of the allowed range in the scroll
direction, so during a rebase the view doesn't keep scrolling once the
last insertion position among the todos has been reached.
Pressing the left button on the current selection now starts a drag
that moves the selected commits, both in the normal commits view and
for todos during an interactive rebase. A press anywhere else falls
through to the usual click handling, so dragging from an unselected
line still creates a range selection, and releasing without having
moved collapses the selection to the pressed commit like a plain click
would.
While dragging, the insertion point follows the pointer: rows below
the dragged block insert after the pointed-at commit, rows above it
insert before it, and during a rebase the destination is limited to
the contiguous block of movable todos around the selection. gocui
moves the view cursor along with the pointer, so each drag event moves
it back to keep the original selection highlighted.
The move happens on release. The model may have been refreshed during
the drag, so the dragged commits are located again by their identity
(hash, subject, todo action); if they no longer form a unique
contiguous block, the drop is ignored rather than guessing.
Let the todo-move primitives take a distance instead of hardcoding a
single row, by iterating the one-row move in memory. Dropping a commit
several rows away thus rewrites the todo file once and, outside of an
interactive rebase, runs a single rebase rather than one per row.
Merge the up/down variants of the move commands into one
direction-parameterized implementation. Dragging commits is about to
need moves over arbitrary distances, which we don't want to build twice.
Give the list views the same edge autoscroll during drag selection
that the staging view already has; the new mouse-release binding stops
the autoscroll when the drag ends.
Dragging with the left button held now extends the selection from the
pressed line, exactly like moving with shift+up/down does. We use the
non-sticky flavor so that the range collapses on the next plain cursor
movement, again matching the keyboard behavior.
The binding is only registered for contexts that support range selection
in the first place; dragging in other lists continues to do nothing.
When the pointer reaches the edge of the view during a drag (or leaves
the view entirely, which mouse capture makes possible), keep scrolling
and extending the selection: slowly on the innermost edge row, faster
on the outermost row, and very fast beyond. Scrolling starts after a
short delay so that a drag merely passing near the edge doesn't scroll.
When the view loses focus mid-drag (e.g. because a popup appeared),
cancel the autoscroll and the mouse capture.
The root item's path is ".", and the path of a file at top level is
"./file". When using GetPath, this gives us "." and "file",
respectively, and isDescendentOfSelectedCommitFileNodes would return
false for these.
Working with the internal paths (i.e. without stripping the leading
"./") fixes this.
There is no known breakage that is caused by this, that's why I'm not
adding an integration test that demonstrates a bug.
Equivalent to the change that was made to isDescendentOfSelectedNodes in
files_controller.go in 302b621b68.
It never changes inside this function, so there's no need to recompute
it with every loop iteration.
Equivalent to the change that was made to isDescendentOfSelectedNodes in
files_controller.go in d0c6e27fee.
This fixes the problem described in the previous commit; we no longer
capture the selection at the start of the refresh. There's no reason to
do that (we don't do it for branches either). It is enough to capture
the selection in the final bounce, before we assign the new model slice.
Popping or dropping a stash shifts the indices of the entries below it,
and renaming re-creates the stash at the top, shifting all the others.
The stash model is only rebuilt by the refresh, which finishes in the
background, so acting on the next entry in quick succession — pressing
the key, confirming the popup, and pressing again right away — reads the
stale pre-operation indices and targets the wrong stash. Note that the
confirmation popup is no protection here: the race starts when the
confirm handler runs, and the next keypress can easily beat the refresh.
Use RefreshBlockingInput so a quick follow-up keypress is buffered and
replayed once the refreshed stash list is in place.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Moving a todo rewrites the todo file and advances the selection
synchronously, but the commits model is only rebuilt by the refresh. A
second press arriving before that grabs the swapped-with todo from the
stale model at the advanced index and moves it back, so holding the key
to move a todo several slots misbehaved. Use RefreshBlockingInput so the
second press is buffered and replayed once the moved todo list is in
place.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A refresh from the UI thread returns immediately and applies its model
and view updates as queued UI-thread callbacks. A key pressed before
those have run is handled against the stale, pre-refresh state. For most
keys that's harmless, but some handlers turn that state into git
commands: pressing space twice in quick succession in the staging panel
builds the second patch from the already-applied diff and fails with
'patch does not apply', because the refresh after the first press is
what moves the selection to the next stageable hunk.
Notably, this is not just a regression of the recent change that made
UI-thread refreshes non-blocking; the window was merely much narrower
before. A blocking refresh parked the UI thread while the scopes'
bounces were queued, and the event loop drains pending keyboard input
with priority over queued user events, so a key pressed during the
blocked window still beat the queued state updates. The guarantee that
the next keypress sees post-refresh state had already ended when the
scopes' state updates moved from worker-side mutex-guarded writes to
UI-thread bounces.
Fix it with the input-blocking mechanism we already use for commit
surgery, exposed as a new RefreshBlockingInput entry point: it begins
blocking events synchronously in the calling handler, and ends the
block from a callback that the finishing step queues behind the
refresh's own updates. Keys pressed while the refresh is in flight are
buffered and replayed, in order, against the fully refreshed state;
since a replayed key's handler re-enters this same path, a burst of
keypresses applies sequentially, each one seeing the previous one's
refresh. Unlike the old blocking refreshes, this doesn't freeze the UI
thread: rendering, spinners, resizing, and mouse scrolling keep working
while input is withheld.
Blocking input is opt-in per call site rather than the default for all
UI-thread refreshes, because most refreshes (the focus-in and startup
refreshes, say) don't produce state that the next keypress depends on,
and blocking on them would delay typing for no reason. It should also be
limited to quick, narrow-scoped refreshes: a full refresh, or any scope
that pulls in COMMITS, can take very long in large repos and should
usually not hold up input.
The staging panel's stage/discard/edit-hunk refreshes use it now.
A few comments still reasoned in terms of SYNC vs ASYNC refreshes, a
distinction that no longer exists: sync vs async is now derived from the
calling thread. Restate them in terms of the current mechanisms
(RefreshFromWorker blocking its worker, model updates being enqueued on
the UI thread) without changing any behavior.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Checking out a remote branch that has no local counterpart creates the
local branch, refreshes, and then checks it out. The refresh exists so
that CheckoutRef finds the new branch in the model and attaches an inline
status to the branch item instead of showing a global waiting status. But
since UI-thread refreshes stopped blocking, the checkout started before
the refreshed branches had landed in the model, so the lookup failed and
we always got the waiting status. Run the checkout from the refresh's
Then, which is queued behind the model update.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PostFetchRefresh's refresh is the only background refresh carrying a
Then callback, and Then callbacks are not generation-guarded: when the
background fetch's refresh crossed a repo switch, the callback still
ran — in the new repo — and auto-forwarded the new repo's branches
because the old repo's fetch had completed. That was harmless in
practice (the update-ref call compares against the expected old value,
and it only does what the next fetch's auto-forward would do anyway),
but mutating refs in a repo whose fetch never happened is not an action
the user took. Skip the auto-forward when the repo generation changed
since the fetch started.
The generation is captured by the fetch's callers before the fetch
runs, not by PostFetchRefresh itself: the background fetch doesn't
block repo switching and is a network call, so by the time
PostFetchRefresh runs a switch may already have happened — a capture
there (or the one the refresh itself takes) would compare against the
new repo's generation and let the auto-forward through. For the manual
fetch the capture point makes no difference, since a foreground
operation blocks repo switching for its entire duration.
This deliberately guards only this call site rather than making Then
callbacks generation-guarded in general: a Then is an arbitrary
callback, and whether it is safe to skip on a repo switch is a decision
for the author of the call site.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The refresh workers read a few files at paths relative to the process
working directory: the submodule config read of .gitmodules, the files
refresh's check for conflict markers, and the submodule stash's
existence check. Git commands are pinned to the repo their instance was
created for, but these Go file reads still followed the cwd, so a
background refresh crossing a repo switch would read the new repo's
files while computing data for the old one. Join them with the worktree
root of the instance they belong to. (Most git-state file reads —
working tree state, rebase todos, bisect info — already resolve
against RepoPaths and need no change.)
This also fixes the submodule stash's existence check for nested
submodules: it stat'ed submodule.Path, which is relative to the parent
module, against the repo root — now it uses the submodule's full path,
matching the stash command right below it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The refreshes on focus-in, right after a repo switch, and after
returning from a subprocess are full foreground refreshes, so their
tasks kept Busy() true for as long as the slowest scope took — and any
switch attempt in that window was refused with the "can't switch"
toast. The focus-in one is particularly annoying: focusing lazygit is
often precisely what the user does in order to switch repos, and right
after regaining focus is when a refresh takes longest.
Blocking the switch bought nothing there. The refusal exists for user
operations, whose follow-up work (e.g. a Then callback reading the
model) isn't covered by the switch-safety guards; but these refreshes
merely reload state, and a refresh by itself is now switch-safe: its
git commands run against the repo it was started for, and the
generation guard drops its updates when the repo changed.
We can't just mark them Background, because that flag also decides
whether the files refresh lets git take optional locks to persist its
refreshed stat cache — worth doing for an attended refresh, and the
focus-in refresh (typically running right after external changes) is
the case that profits most. So split the two meanings: a new
DontBlockRepoSwitch option dispatches the refresh's tasks as background
tasks (excluded from Busy()) while keeping the attended optional-locks
behavior. Combining it with Then panics, since Then is not
generation-guarded.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
An error returned from a gocui worker is shown to the user in an error
popup. For the branch loader's behind-counts worker that used to be the
"no such ref" popup when a background refresh crossed a repo switch:
the old repo's main branch didn't exist in the new repo. The previous
commits fix that scenario properly — the command now runs against the
repo the refresh was started for — but a stale worker can still fail
legitimately, most plausibly because that repo was deleted after
switching away from it (e.g. removing a worktree). Its results are
dropped anyway, so log the error instead of alarming the user about a
repo they already left.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A background refresh's model writes are dropped by the generation guard
when the repo is switched mid-flight, but its git commands kept running
— and because the refresh read the live git instance at each step, any
command issued after the switch ran against the new repo. Now that git
commands are pinned to the directory of the instance they were built
from, capture the instance once when the refresh starts and run every
scope's git work through it, so a switch-crossing refresh keeps
addressing the repo it was started for.
The instance is captured together with the repo generation, on the UI
thread (where repo switches run), so the pair can't straddle a switch:
an old instance paired with the new generation would compute data from
the old repo and write it into the new repo's model unguarded.
This also removes the refresh workers' unsynchronized reads of the live
instance pointer, which raced its reassignment on the UI thread when a
background refresh crossed a repo switch (foreground refreshes can't
cross one: they keep Busy() true, which refuses the switch).
Two reads keyed app-state by the live instance's repo path on a worker
and now use the captured instance, fixing which repo they file under
when crossing a switch: the pull-request cache, and the "user dismissed
the base-remote prompt" flag. The base-remote menu's handlers keep
reading the live instance: a switch dismisses any open popup, so they
can't run against the wrong repo (and the OnPress body runs under a
foreground task, which blocks switching anyway).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
When the user picks a base remote in the "select remote repository"
prompt, we called setGithubPullRequests directly, bypassing the refresh
machinery — which meant hand-rolling the refresh env that call needs
(with a comment explaining why), and fetching against the branches
captured when the prompt was created. Issue a PULL_REQUESTS-scoped
refresh instead: it re-reads branches and remotes (both fast even in
large repos), fetches against those fresh values, and gets the refresh
machinery's guarantees without any special-casing. The config write is
re-read by the refresh from git config, so it is guaranteed to be
picked up.
The waiting status now covers the config write and the branches/remotes
reload, while the GitHub request itself continues as a background task
— which is how every other pull-request fetch behaves.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Each status used to start a spinner render loop of its own, running on
a worker that inherited the foreground/background flavor of the
status's owner, and exiting only once the entire status stack was
empty. That shape had a real bug: a foreground operation's loop could
be kept alive by someone else's status. Finish a quick operation with
a waiting status while a background fetch's "Fetching..." status is
still showing, and the operation's render loop — a foreground worker
task — keeps ticking until the fetch ends. Busy() stays true for that
whole time, so repo switching is refused even though nothing is in
flight anymore; with a fetch hanging on a slow network, that means
minutes. The shape was also wasteful: overlapping statuses were each
drawn by their own loop (plus a duplicate whenever a task was paused
and resumed while another status was showing), all redundantly
redrawing the same top status.
Replace the per-status loops with a single loop owned by the status
stack as a whole: whoever shows the first status starts it, and it
exits after drawing a final empty frame once the last status is
removed. The claim/release methods on StatusManager keep the loop
flag's transitions atomic with the stack under the one mutex, so a
status added while the loop is about to exit starts a fresh loop
instead of going unrendered.
The loop always runs as a background task now: rendering issues no
git commands, so it never needs to block repo switching, and a
foreground operation's busy-ness is already carried by its own worker
task. This is what fixes the bug above, and it retires the need to
thread a foreground/background flag through the waiting-status
helpers altogether.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every full refresh includes the PULL_REQUESTS scope, and the worker it
spawns inherited the refresh's foreground/background flag. Full
foreground refreshes happen at startup, after switching repos or
worktrees, and when the terminal regains focus, so the GitHub API
request ran as a foreground task there, keeping Busy() true until it
completed. On a healthy network that's a few hundred milliseconds and
nobody notices; on a very slow one the request can stall for minutes,
and every attempt to switch repos in that window was refused with
"Can't switch repositories while an operation is in progress" even
though lazygit looked completely idle. (The request has no visible
status; at most, a background fetch hanging on the same bad network was
showing its "Fetching..." spinner, pointing the blame at the wrong
operation.)
The switch-safety guard only needs to wait for operations whose
remaining git commands would run against the wrong repo after a switch.
The pull-request fetch runs no git commands at all, and its model
writes are dropped when the repo generation has changed in the
meantime, so there is no reason for it to block switching.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
hasExecTodos reads Model().Commits. genericMergeCommandImpl evaluates it
when deciding whether to use a subprocess, and on the recursive auto-skip
path that runs on a worker -- so the read raced the UI thread. Bounce it
onto the UI thread there, keyed off the calledFromWorker flag the function
already carries (on the UI-thread entry path the read stays inline).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The file-path suggestions trie is rebuilt asynchronously and then read by
the suggestions search, which runs on an AsyncHandler worker. It lived in
Model().FilesTrie, so that worker read the (UI-thread-only) model. Move
it to an atomic pointer on the SuggestionsHelper instead: it's the only
place that uses it, the helper is recreated per repo (so the cache still
resets on a repo switch), and an atomic pointer is safe to store from the
build and load from the search worker.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The "merge conflicts" refresh scope ran on a worker like the others, but
unlike them it does UI work rather than git work: RefreshMergeState reads
the current context and renders (or escapes) the merge-conflicts view.
Reading the context manager and rendering from a worker races the UI
thread. Bounce it onto the UI thread with onUIThreadUnlessRepoChanged,
exactly as the staging and patch-building scopes already do.
Running on the UI thread also lets EscapeMerge push the files context
directly instead of deferring the push to a separate UI task; it only
needs to drop the merge-conflicts mutex first, because the push
renders the newly focused file, which can take the mutex again. The
deferred push could lose a race against the same refresh's prompt to
continue the rebase/merge: if the prompt opened between
RefreshMergeState and the deferred push, the push declined to cover
the popup and was dropped, so closing the prompt landed the user in
the emptied merge conflicts view.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Dropping a range of stashes ran a refresh after each drop. A refresh
issued from the UI thread does its git work on a worker and applies the
model update in the background, so firing one per iteration let the
workers race: an earlier drop's refresh (which read a stash list that
still contained a later-dropped entry) could apply its result last,
leaving the stash view showing an entry that git had already removed.
Refresh once, after all the drops, so a single worker reads the final
stash list. The indices are captured up front and dropped highest-first,
so the remaining lower indices stay valid without an intervening
refresh. It's also cheaper: one `git stash list` instead of one per
entry.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The patch-building scope ran RefreshPatchBuildingPanel directly on the
refresh worker, where it read the commit-files selection and set the patch
view's origin off the UI thread — the latter raced the UI thread's draw.
Bounce it onto the UI thread, exactly as the staging panel just above
already does, guarded on the generation so a repo switch drops it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PopupMutex guarded CurrentPopupOpts against a popup being created on a
worker goroutine while the UI thread deactivated it, or reset it on a
repo switch. Now that popup and menu creation is bounced onto the UI
thread, every access to CurrentPopupOpts — create, deactivate, and the
reset-on-switch (which already runs on the UI thread) — happens on the
one goroutine, so the mutex protects nothing.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fetching pull requests can take a long time, and we don't want to delay
the refresh by it; in particular, for a WithWaitingStatusBlockingInput
we want the UI thread to be unblocked again while pull requests are
still fetching in the background. This is similar to how we fetch the
behind values for branches in BranchLoader; this will update the UI
without much flicker when done, and doesn't have to block anything.
The two branches of the `refresh` closure ran the scope function
identically; they differed only in that the UI-thread path registered
each scope as its own gocui task while the worker/demo path used a bare
goroutine (and only the latter logged per-scope timing).
Those per-scope tasks were redundant. performRefresh always runs under a
task that stays busy until the wg.Wait in waitAndFinalize joins every
scope goroutine: the calling worker's own task when called from a worker,
or the waitAndFinalize worker task when called from the UI thread — and
that task is created (busy) before the triggering event's task goes Done,
so there is no window in which nothing is busy. Repo-switch safety and the
integration-test idle signal are therefore already covered without giving
each scope its own task.
Collapsing to the single goroutine path also means the timing log now
fires for UI-thread refreshes too, not just worker ones.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Nothing calls it anymore now that the commit-surgery operations run on a
worker with input blocked. Remove the helper, its bespoke synchronous
spinner loop (renderAppStatusSync/setAppStatusContent), the popup-handler
plumbing, and the interface method.
That loop was also the only thing suppressing the yellow "Rebasing" mode
indicator (and its reset button) while lazygit drives a rebase itself.
Move that suppression to WithWaitingStatusBlockingInput so it applies to
every input-blocking commit-surgery op — including the ones that already
ran on a worker (edit, drop, and so on) and previously let the indicator
flash on mid-operation. It's cleared after the refresh, so an operation
that legitimately leaves a rebase in progress still shows the mode.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
With the last synchronous commit-surgery callers moved to workers,
nothing runs CheckMergeOrRebase on the UI thread anymore, so
CheckMergeOrRebaseWithRefreshOptionsFromUIThread has no callers. Remove
it and fold the shared checkMergeOrRebaseImpl back into
CheckMergeOrRebaseWithRefreshOptions, which is now always on a worker.
The runAction closure loses its calledFromWorker parameter for the same
reason.
genericMergeCommandImpl keeps its calledFromWorker flag: the
merge/rebase-continue subprocess path still runs on the UI thread when
invoked straight from the menu, and on a worker for the recursive
auto-skip.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
edit, quick-start rebase, drop, reword, squash, fixup, amend
(including the amend-attribute author operations) and
discard-file-from-commit all run a rebase on a worker. A key pressed
while one is in flight could act on a stale commit or todo — pressing e
to start an interactive rebase, then up+d before it finishes, is the
motivating example. Switch them from WithWaitingStatus to
WithWaitingStatusBlockingInput so input is held and replayed against the
post-operation state, matching the commit-surgery ops that were already
sync.
Left alone: the custom-patch move/delete/pull-into-commit rebases (no
need to block input while building and applying a patch), the
loading-more-commits and patch-building toggle spinners (no rebase to
disrupt), and fetches and other non-surgery operations where blocking
navigation would only get in the way.
Move, revert, squash-fixups, create-fixup and cherry-pick paste ran
their rebase synchronously on the UI thread via WithWaitingStatusSync,
which froze the UI for the duration but kept the user from disrupting the
operation with a stray keypress. Switch them to
WithWaitingStatusBlockingInput so the git work runs on a worker — the UI
keeps rendering and the spinner animates — while input stays blocked for
the whole operation, as before.
discard-patch-from-commit also moves off WithWaitingStatusSync, but as a
plain WithWaitingStatus: it's a custom-patch command, and those don't
block input.
The bodies now follow the worker conventions: model state they need is
captured on the UI thread before dispatching, self.c.Refresh becomes
RefreshFromWorker, and CheckMergeOrRebase uses the worker variant. An
operation that moves the selection does so in the refresh's Then, so it
lands in the same frame as the refreshed commit list; squash sets it as
an absolute index there, because the shorter list would clamp a relative
move.
It reads the selected index and the commits and branches models to
decide where to move the fixup commit. Take those as parameters,
captured on the UI thread by the callers, so the function can run its
rebase on a worker without reading the model there. No behavior change;
the callers still run on the UI thread for now.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bracket gocui's BeginBlockingEvents/EndBlockingEvents around a
worker operation that shows a waiting status. The block is begun
synchronously on the UI thread, before the operation is dispatched to a
worker, so no keypress can slip through in between; it ends via
OnUIThread once the operation and its refresh have applied their UI
updates, so the replayed keys act on the refreshed state.
This composes what the retiring WithWaitingStatusSync did — show a
status and block input — but on a worker, so the UI keeps rendering
(spinner animates, model updates land) instead of freezing. Callers
follow in subsequent commits.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
With sync vs async now derived from the calling thread, the Mode field
and its SYNC/ASYNC constants no longer carry any information: Refresh is
always async, RefreshFromWorker always sync. Drop the field, the type,
and the Mode argument at every call site, and reduce the debug log's
mode name to a plain sync/async derived from calledFromWorker.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Whether a refresh should block or run in the background was controlled
by the Mode field, but that always lined up with the calling thread: a
UI-thread Refresh must not block the UI, while a RefreshFromWorker runs
on a worker where blocking is exactly what we want. Now that Then and
BatchUIUpdates work regardless of that choice, drop Mode from the
decision and key it off calledFromWorker instead:
- Refresh (UI thread) runs its scopes and the finishing step (wait,
batch flush, Then) on workers, so the caller returns immediately —
what ASYNC used to mean.
- RefreshFromWorker runs them on the calling worker, blocking it until
everything is done — what SYNC used to mean.
Demos keep taking the blocking, inline path so everything still lands in
one deterministic frame.
In practice this flips the handful of RefreshFromWorker calls that
passed ASYNC — they now block their worker until the refresh finishes,
keeping the waiting-status spinner up until the UI actually updates —
and the many UI-thread refreshes that defaulted to SYNC, which no longer
freeze the UI thread while the git work runs. Mode now only feeds the
log line; the next commit removes it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Creating a branch checks it out, and checking out a distant ref (a tag
or a commit far from HEAD) can take a noticeable while. NewBranch ran
that synchronously in the prompt's confirm handler, on the UI thread, so
the UI froze — no spinner, no repaint — until it finished.
Move the branch creation (and the autostash path) onto a worker with a
waiting status, mirroring CheckoutRef, and refresh from the worker so
the UI thread stays live and the spinner keeps animating.
Push the branches context from the refresh's Then rather than up front:
the refresh already batches its UI updates, so switching panels there
lands the switch in the same frame as the refreshed branch list instead
of flashing the pre-refresh list while the checkout is still running.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Then, and BatchUIUpdates, previously only worked for a SYNC refresh: the
calling goroutine blocked in wg.Wait until every scope had finished, and
only then flushed the batch and ran Then. An ASYNC refresh had no such
join point — it dispatched each scope onto its own worker and returned
right away — so Then was forbidden (it would have run before the scopes
finished) and a batch would never be drained.
Give the async path a join of its own. Both paths now register their
scopes in the WaitGroup, and the finishing work — wg.Wait, the batch
flush, and Then — moves into a closure. A SYNC refresh runs it inline as
before; an ASYNC refresh dispatches it to a worker, so the caller still
returns immediately but the batch and Then run once every scope is done.
Besides lifting the restriction, this makes SYNC and ASYNC differ only
in whether the finishing work blocks the caller, which is what lets a
later commit drop the mode entirely and key the choice off the calling
thread instead.
There is no f() function any more, so a variable named "f runs on"
doesn't make sense. And we also don't need it any more; it used to be
necessary when its meaning was not exactly the same as
`!calledFromWorker`, but also included the BLOCK_UI case, but that has
changed several commits ago.
This was useful when there was a BLOCK_UI mode where f() was called
differently, but now we no longer need it. I'm making this change as a
separate commit because folding it into the previous one (which would
conceptually have made sense) would have made that diff unreadable
because of the indentation change.
The variable `fRunsOnUIThread` and its comment no longer make sense now;
we'll clean this up next.
The diff is best viewed with --ignore-all-space.
BLOCK_UI ran the whole refresh on the UI thread and parked it in a
wg.Wait for the duration, so the UI (and its spinner) froze while the
git work ran. Blocking the UI was never the point — the point was to
apply all the scopes' updates in one frame instead of a per-scope
cascade — and if we genuinely wanted to block input it should span the
whole operation, not just its refresh, which needs a gocui-level
mechanism we don't have.
So drop the mode and add a BatchUIUpdates option that achieves the
"one frame" effect without blocking: each scope's UI-thread bounce is
collected into a shared refreshBounceBatch during the refresh, and once
every scope has finished they're all applied inside a single OnUIThread
task. gocui drains every queued event before it redraws, so one task
means one repaint. The refresh itself now runs SYNC — on a worker when
issued from one (checkout, move-to-new-branch, the rebase-edit result
handling), so the UI thread stays live and the spinner keeps animating.
The batch needs a mutex because the scopes add concurrently from their
worker goroutines, and a closed flag so that any bounces enqueued after
the flush starts — the nested ones a flushed bounce produces in turn,
e.g. scrolling the selection into view — are dispatched immediately as
ordinary follow-ups rather than collected into a batch that nothing
will drain.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When the last conflict of a file is resolved, a files refresh both
offers to continue the rebase/merge (if we started it ourselves) and,
via its merge-conflicts scope, escapes from the merge conflicts view
back to the files context. The two race: the prompt is bounced onto
the UI thread by the files worker, while the escape's context push is
queued separately by EscapeMerge, and it deliberately refuses to push
the files context over a popup. So if the prompt opens first, the
escape does nothing, and closing the prompt lands the user in the
stale merge conflicts view — usually already emptied by the escape's
state reset — instead of the files panel. No later refresh rescues
this.
Fix this by escaping from the merge conflicts view right before
opening the prompt. This runs on the UI thread and doesn't hold the
merge conflicts mutex, so it can reset the state and push the files
context synchronously; whichever side runs first, the prompt now
always opens on top of the files context, and EscapeMerge's guarded
push still does nothing only when that's the right thing to do.
This is a timing race with no deterministic regression test; it
showed up as a rare flake in tests that cancel the continue prompt
(e.g. commit/amend_when_there_are_conflicts_and_continue) when
looping the integration tests under the race detector.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
When scrolling a lazy-loaded view (a diff in the main view, the command
log, etc.), we top up the view's line buffer by reading more lines from
the still-running task. This was driven by asking the task to read a
fixed number of *additional* lines on every scroll event, which had two
problems:
- It was decoupled from the scroll position. Scrolling down, back up,
and down again re-read lines that had already been read, so the buffer
crept towards the end of the input regardless of where the user
actually scrolled.
- A single wheel notch only bought a single notch worth of runway, so
fast scrolling constantly outran the reader and had to wait for the
next read (and re-render) on every notch.
Make ReadLines take an absolute target total instead of a delta: the
task tracks how many lines it has read and only reads the shortfall, so
requests are idempotent. Callers now ask to fill the viewport at the
current scroll position plus a few screenfuls of read-ahead, which gives
scrolling enough runway to stay smooth.
The four call sites all wanted the same "fill this view" computation, so
consolidate them into a single ReadLinesToFillView helper.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The model-update bounces already drop themselves when the repo is
switched mid-refresh (onUIThreadUnlessRepoChanged), but three bounces
that touch the UI without writing the model did not: refreshView's
render, the staging-panel refresh, and the stale continue-rebase prompt
dismissal. All three ran unconditionally on the UI thread, so a
background refresh in flight across a repo switch could render the old
repo's data (through a context object belonging to the now-replaced
context tree), or pop the new repo's popup based on the old repo's
prompt state.
Route them through onUIThreadUnlessRepoChanged too, so they're dropped
alongside the model writes they accompany. This also fixes the dismiss
bounce using the raw foreground OnUIThread, which ignored the background
flag every other bounce in a background refresh respects.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Every refresh scope needs two ambient values to bounce its model and
view updates back to the UI thread safely: the background flag (which
picks the dispatch variant that doesn't count towards lazygit being
busy) and the repo generation that guards the bounce against a repo
switch. These were threaded separately — background as a parameter on
every refreshXxx function, generation re-read from the model inside each
one. Bundle them into a single refreshEnv passed through instead, so the
guard has a home to grow into (the next commit needs the generation in
refreshView, which currently has no access to it).
Capturing the generation once, at the start of the refresh, is also more
correct than the previous per-function re-read. The baseline should
reflect the repo whose inputs the refresh snapshotted (all captured up
front on the UI thread), not whenever each scope's worker happens to
wake. With the per-function read, a background refresh whose worker woke
after a repo switch would read the new generation and let its bounce
through, writing data computed from the old repo's inputs into the new
repo; capturing up front makes that bounce drop instead.
No behavior change for foreground refreshes, where the UI thread is held
for the whole refresh and the generation can't move under it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The commit's gpg onSuccess runs on a worker when the command output is
streamed, so its ClearPreservedCommitMessage wrote commit-message
context state off the UI thread. Bounce that write through OnUIThread.
GetFilePathSuggestionsFunc builds the trie on a worker (the slow
AllRepoFiles walk) and then assigned Model().FilesTrie and refreshed the
suggestions panel from there, racing the UI thread that reads the trie.
Keep the build on the worker but bounce just the model assignment and
the refresh through OnUIThread.
The discard handler cancelled the commit-files range selection from its
WithWaitingStatus worker. Bounce it through OnUIThread, keeping it after
the successful CheckMergeOrRebase as before.
The three branch-delete handlers and the two worktree-removal
continuations collapsed the Branches/RemoteBranches range selection from
their worker goroutine, racing the UI thread. Wrap each collapse in
OnUIThread, keeping it in the same spot relative to the refresh (FIFO
preserves the collapse-then-refresh order the name-restore depends on).
The pull-patch-into-new-commit handlers closed the commit-message panel
and, on success, pushed the local-commits context from inside the
WithWaitingStatus worker. Close the panel in OnConfirm before
dispatching (UI thread), and bounce the post-rebase context push through
OnUIThread, keeping it on the success path.
The three rebase-onto menu items read Modes().MarkedBaseCommit.GetHash()
(a bare string field) and, on success, cleared it via
ResetMarkedBaseCommit and pushed the commits context — all from the
WithWaitingStatus worker, racing the UI thread. Read the marked base
hash before dispatching, and bounce the post-rebase reset and context
push through OnUIThread, still guarded by the success check so they
don't run on the conflict path.
interactiveRebaseWithFlag and dropMergeCommit ran inside the
WithWaitingStatus worker but read Model().Commits and wrote the
selection (SetSelection(startIdx)) there, racing the UI thread. Thread
the commits slice in from each caller, and hoist the pre-rebase
selection into a UI-thread helper (selectRebaseResultCommit) called
before dispatching — squash/fixup unconditionally, drop only on the
non-merge path, matching the previous action guard.
ResetToRef ran on a worker and wrote the local-commits and reflog
selection directly (SetSelection(0) on both) before its refresh, racing
the UI thread. Fold those into the refresh's selection intents:
SelectHeadCommit for the commits (after a reset HEAD is the top commit,
and mid-interactive-rebase it correctly picks the real head over the
first todo entry) and SelectTopReflogCommit for the reflog. The
now-atomic SetLimitCommits stays where it is.
discard reads Model().Commits and the selected commit index from its
WithWaitingStatus worker; read them in HandleConfirm instead.
toggleForPatch reads the commit-files ref name from the worker, and its
startPatchBuilder call reads the context's canRebase and diff range from
there too. Capture the ref name and run startPatchBuilder in
HandleConfirm before dispatching; PatchBuilder.Start only assigns
fields, so moving it off the worker changes no timing.
discard still collapses the range selection from the worker; that write
is a separate concern, left for a follow-up.
ResetSubmodule and fastForward each call a helper that reads the model
from inside their worker: FileForSubmodule reads Model().Files and
worktreeForBranch reads Model().Worktrees, racing the UI thread's model
writes. Hoist both lookups above the worker dispatch.
The two move helpers run inside the WithWaitingStatus worker that
withNewBranchNamePrompt dispatches to, but read Model().Files/Submodules
(to decide whether to auto-stash) and Model().Commits (the unpushed
commits to cherry-pick off the base branch) from there, racing the UI
thread's model writes. Compute mustStash — needed by both paths — at the
top, and the unpushed commits in the off-of-main menu item, on the UI
thread, and pass them into the helpers.
handleReword, amendTo, and the reset/set/add-co-author handlers pass
Model().Commits (and the selected line index) to a git rebase from
inside the WithWaitingStatus worker, racing the UI thread's model
writes. Read them on the UI thread before dispatching.
The author handlers index the full commit list by absolute start/end, so
the range sub-slice withItemsRange hands amendAttribute is not what they
need; capture the full Model().Commits there and thread it through.
These handlers dispatch their rebase to a worker via WithWaitingStatus
but read Model().Commits (and, for move-to-selected-commit, the selected
line index) from inside that worker, racing the UI thread's model
writes. Read them on the UI thread before dispatching and close over the
results.
getPatchCommitIndex stays as-is: moving its call out of the worker makes
its own Model().Commits read UI-thread-bound too, so the identical copy
in patch_building_controller.go needs no matching signature change.
The two pull-patch-into-new-commit handlers still push a context and
close the commit-message panel from the worker; those writes are a
separate concern, left for a follow-up.
With every scope's worker reads now captured on the UI thread and every
worker caller on RefreshFromWorker, the debug entry-point assertion no longer
needs to be scoped to the commits refresh. Move it to the top of
performRefresh so it guards every refresh regardless of which scopes it
touches, and drop the per-scope gate.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The remaining refresh scopes each still read model, context, and mode state
directly on their worker, racing the UI thread — the same class of race the
commits refresh had:
- files reads Model.Files (to detect resolved conflicts and drive the
auto-stage) and the Files context's ForceShowUntracked;
- reflog reads the existing reflog slices (for the incremental fetch),
Model.HashPool and the filtering path/author;
- branches reads Model.MainBranches and the previous branches (for the
BehindBaseBranch carry-over);
- stash reads the filtering path.
Gather each scope's inputs into an immutable snapshot on the UI thread (via
captureOnUIThread) before dispatching the git work, and have the refresh
compute from the snapshot — for branches, threaded through both the immediate
and the recency-sorted startup loads, which share one snapshot (the
BehindBaseBranch carry-over is identical either way). Status, tags and
worktrees read nothing UI-owned, so they're left alone.
For the snapshots to actually run on the UI thread, the worker callers that
reach these scopes must announce themselves: convert the submodule
operations, the submodule stash-and-reset, and the background files poller
to RefreshFromWorker.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
These four refreshes each read model, context, and mode state directly on
their worker — the same class of race the commits refresh had:
- remotes reads the selected remote (Contexts().Remotes.GetSelected), needed
to keep the remote-branches selection valid;
- sub-commits reads the SubCommits ref/limit/divergence, the filtering
path/author, and Model.MainBranches/HashPool;
- commit-files reads the diff endpoints (CommitFiles from/to and the diffing
args);
- rebase-commits reads Model.HashPool/Commits.
Give each the same treatment as commits: gather its inputs into an
immutable snapshot on the UI thread (via captureOnUIThread, inline for a
UI-thread refresh, hopped for a worker one) before dispatching the git work,
and have the refresh compute from the snapshot. The commit-files re-init
inside the commits refresh captures its endpoints in the bounce, right after
ReInit sets them, before dispatching to the worker.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Now that every commits-reaching refresh issued from a worker goes through
RefreshFromWorker, guard the choice: in debug builds, panic if a refresh was
issued from the UI thread as RefreshFromWorker or from a worker as Refresh.
The caller's own goroutine is recorded at the top of performRefresh, before
a BLOCK_UI refresh dispatches onto the UI thread, so the check holds for
every mode rather than being fooled by BLOCK_UI. It's scoped to the commits
refresh for now, the only converted scope; once the rest are converted the
guard can move up to cover every refresh unconditionally.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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) <noreply@anthropic.com>
A commits refresh does its git work on a worker and then reads the model,
the contexts, and the modes for that work directly from there:
LocalCommits.GetSelectionRangeAndMode/GetLimitCommits/GetShowWholeGitGraph,
Model.Commits/MainBranches/HashPool, the filtering path/author. Those are
owned by the UI thread, which is concurrently running the cursor and render
code, so the reads race it — the dominant, confirmed source of the
commits-scope flakes (the startup ClampSelection vs GetSelectionRangeAndMode
race, for one).
Gather them into an immutable capturedCommitState on the UI thread, before
the git work is dispatched, and have refreshCommitsWithLimit compute from
that snapshot. UI-thread callers capture inline; worker callers can't (a
SYNC/BLOCK_UI refresh parks the UI thread at wg.Wait, so hopping from a
scope sub-worker would deadlock), so the capture is lifted out of the scope
worker into the refresh orchestration, and worker callers announce
themselves with a new RefreshFromWorker entry point that hops the capture to
the UI thread and blocks for it (OnUIThreadAndWait). BLOCK_UI runs the whole
refresh on the UI thread regardless of the caller, so it captures inline
too.
Every refresh issued from a worker that reaches the commits (or branches,
which pulls in commits) scope is converted: the fast-forward, branch/tag
delete, worktree remove/detach, push, reword-via-rebase, author edits,
custom-command, hard-reset-with-autostash, reset-to-ref, fetch-and-checkout,
gpg-stream, post-fetch, and external-change-poller refreshes, plus the
branch checkout and move-commits-to-new-branch refreshes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The PR fetch needs the current branches (for their upstreams) and
remotes to know what to query. It read them from Model().Branches /
Model().Remotes on its own worker, after waiting on branchesAndRemotesWg
for the branches and remotes refreshes to finish.
That wait no longer guarantees fresh data: those refreshes now write the
model in a bounce onto the UI thread, and Done() fires before the bounce
has been processed. So the fetch read the pre-refresh lists — most
visibly, checking out a branch that has a PR wouldn't show that PR until
the next refresh, because the fetch queried the old branch set.
Have refreshBranches / refreshReflogAndBranches / refreshRemotes return
what they loaded, stash it in locals in Refresh, and hand it to the
fetch. The wait on branchesAndRemotesWg orders the fetch after both
loads have stored their slices, so it fetches against exactly the
branches and remotes that were just loaded, with no model read on the
worker. The previous commit guarantees both are always in scope when
pull requests are, so no fallback is needed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The pull-request fetch queries GitHub for the tracking branches'
upstreams against the configured remotes. It therefore depends on the
branches and remotes being up to date; a refresh that asks for pull
requests but not for those (e.g. checking out a branch) would fetch
against a stale branch/remote list — for instance missing the PR of the
branch just checked out.
Expand the scope so pull requests always co-refresh branches and
remotes. This also sets up the next commit to hand the freshly-loaded
branches and remotes straight to the fetch, instead of reading them
back from the model (which, now that those writes are bounced onto the
UI thread, would be stale on the worker).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Operations that check something out (checkout, create branch, move
commits to a new branch, fetch-and-checkout) selected the newly
checked-out branch by calling SelectFirstBranchAndFirstCommit() before
the refresh and passing KeepBranchSelectionIndex so the refresh wouldn't
override it. That set the selection directly, usually from a worker
goroutine (WithWaitingStatus/WithInlineStatus). Now that the refresh's
own selection write is bounced onto the UI thread, the two writes could
land in either order, and under load the refresh's "restore the
previously-selected branch" write would win — leaving the old branch
selected instead of the new one (flaky
move_commits_to_new_branch_from_base_branch).
Replace it with declarative selection intents applied inside the
refresh's own bounce, so the selection is set on the UI thread and
atomically with the list write (no off-thread write, and no BLOCK_UI
needed to avoid a flicker):
- BranchSelection: SelectCheckedOutBranch selects the checked-out branch
(top of the list). The default, KeepBranchSelectionByName, restores
the previously-selected branch by name as before. This replaces the
KeepBranchSelectionIndex bool.
- CommitSelection: SelectHeadCommit (already existed) for the commit.
- SelectTopReflogCommit selects the top reflog entry, since a checkout
adds a new entry there (reflog/checkout relies on this).
SelectFirstBranchAndFirstCommit is gone. The previously-selected branch
is now read at the top of the branches bounce, before the list is
overwritten, so that read moves onto the UI thread too.
fetchAndCheckout's refresh changes from ASYNC to SYNC so its
post-refresh focus switch can run in Then on the UI thread; SYNC keeps
the inline fetch spinner spinning (only BLOCK_UI would freeze it).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A toast keeps a foreground spinner task alive for its whole lifetime
(~2-4s): showing one calls renderAppStatus, whose OnWorker loop runs
until the status string clears. With the repo-switch guard in place that
made the guard's own "can't switch, operation in progress" toast keep
Busy() true, so the next escape/switch was refused until the toast
faded — you had to wait it out.
Render toasts in the background, like view-buffer content: a toast is a
transient notification, not lazygit driving an operation, so a switch
during one is fine. A real operation that shows a toast still keeps its
own foreground task busy independently.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Switching repos reassigns gui.git and the process cwd; doing it while a
foreground git operation (rebase/commit/push/…) is mid-flight would run
that operation's remaining commands against the wrong repo. The same
applies while the refresh an operation triggers is still settling: its
model writes are generation-guarded, but the client-side Then/OnUIThread
callbacks that run after it aren't, and shouldn't run against a repo that
changed underneath them.
Refuse the switch (with a toast) whenever gocui reports a busy foreground
task. DispatchSwitchTo carries the guard for the simple callers. The
callers that do work before the switch check up front instead, so a
refused switch doesn't leave that work half-done: worktree creation
checks before creating (its own waiting-status spinner would otherwise
make the query busy and refuse its own switch); submodule-enter and the
recent-repos menu check before mutating the repo-path stack (pushing /
clearing it); and escape-to-parent (SwitchToParentRepo) checks before
popping it, so a refusal doesn't consume the entry and strand the user
with nowhere to escape back to. All then call the unguarded switchTo,
which is safe because their own operation is complete by then.
For the busy query to be usable as a repo-switch guard it has to be
false while the ongoing background routines run, or a switch would be
refused every time a background fetch or files refresh happened to be in
flight. Mark that work as background so it's excluded from the query.
The background routine dispatch in goEvery becomes OnWorkerBackground,
and the auto-fetch waiting status renders its spinner through the
background variants. Within a refresh, the background flag (which
Refresh already carries as options.Background, and which the files path
already threaded) is now threaded through every place that enqueues a
task: the async scope workers, the model-write bounces
(onUIThreadUnlessRepoChanged), refreshView, the staging bounce, the
Then dispatch, and the branch-loader's behind-count worker. Two
single-caller chains reached by a background files refresh get the flag
too: MergeConflictsHelper.EscapeMerge and BranchesHelper.
AutoForwardBranches (whose follow-up refresh must stay background when
triggered by the background fetch).
Nothing gates on the busy query yet, so this is behavior-preserving;
background tasks still count as busy for the test idle-listener, which
looks at every task regardless of the background flag.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
DispatchSwitchTo wrapped its whole body in WithWaitingStatus, so the
switch ran on a worker: it chdirs, reassigns gui.git, and swaps gui.State
(in resetState), all of which the UI thread also reads. The generation
guard prevents the refresh-in-flight logical corruption but not this
pointer data race on gui.State.
Run the switch synchronously on the UI thread instead. Every caller is
already a UI-thread handler except NewWorktreeCheckout, which must create
the worktree (git work) on a worker first; it now dispatches only the
switch via OnUIThread. The heavy data loading still happens
asynchronously via the refresh that onNewRepo triggers, so the
synchronous part is small (a couple of git rev-parse plus direnv).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This removes the last refresh mutex. RefreshingBranchesMutex wasn't
guarding a data race (Branch.BehindBaseBranch is atomic, and every model
write is now bounced onto the UI thread); it was serializing the two
branch loads that race at the INITIAL startup stage — an immediate one
sorted without the reflog, and an async one that loads the reflog and
sorts by recency — so that the recency-sorted write landed last and won.
That serialization was never a real guarantee, only "very likely": it
relied on the immediate load acquiring the lock before the async load,
which had to load the reflog first.
Instead, each branch load takes a monotonically increasing sequence
number, and its bounce drops the write if a later-started load has
already applied. Combined with the preceding commit (immediate load runs
before the async one is spawned), this is an actual guarantee: the
immediate non-recency load always has a lower sequence than its recency
async partner, so the highest sequence number is always held by a
recency-sorted load, and highest-wins converges on recency ordering —
even if more refreshes fire during the INITIAL window, since each
refresh's async out-sequences its own immediate.
The guard also subsumes what the mutex gave post-startup: a slow, stale
refresh's bounce can no longer clobber a newer refresh's branches.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
At the INITIAL startup stage two branch refreshes happen: an immediate
one sorted by whatever reflog we have (empty, so not by recency), and an
async one that loads the reflog first and re-sorts by recency. Until now
the async one was spawned first and the immediate one ran afterwards;
this inverts that so the immediate refresh runs before the async one is
spawned.
With RefreshingBranchesMutex still in place this is behavior-preserving
(the mutex serializes the two either way). It's a preparatory step for
replacing that mutex with a branch-load sequence guard: running the
immediate refresh first establishes a happens-before relation between
the two loads' sequence numbers, so the recency-sorted one is guaranteed
the higher sequence.
This also lets refreshReflogCommitsConsideringStartup fold into
refreshReflogAndBranches, whose two-phase logic is now all in one place.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Now that every refresh scope writes its model updates on the UI thread
via onUIThreadUnlessRepoChanged, the per-scope mutexes that used to
serialize concurrent worker-goroutine access are redundant:
Model().Commits, .SubCommits, .Authors, the status view content, and
.PullRequests/.PullRequestsMap are all now written only on the UI
thread, and their readers already ran there. setSubCommits only existed
to take the lock, so it's inlined to match refreshSubCommitsWithLimit,
which writes Model().SubCommits directly.
The worker phases still *read* some of these fields (the commit
selection range, MergeRebasingCommits), but those reads race a
concurrent refresh's bounced write regardless of the mutex — the write
happens in the bounce, outside the locked region — so the mutex never
protected them. That residual read race belongs to the broader -race
effort, not to these locks.
RefreshingBranchesMutex is deliberately kept. It is load-bearing for a
reason unrelated to data races: at the INITIAL startup stage two
refreshBranches run concurrently — an immediate one with an empty
reflog (non-recency order) and an async one with the freshly-loaded
reflog (recency order). The mutex serializes them so the recency write's
bounce is enqueued last and wins. Without it the stale non-recency write
can land last, reordering the branches list (caught by the recency-sort
e2e tests: cherry_pick/*, branch/rebase_*).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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 <noreply@anthropic.com>
refreshCommitsWithLimit now loads the commits, working-tree state and
bisect info on the worker and writes them all — Model.Commits,
Model.BisectInfo, Model.WorkingTreeStateAtLastCommitRefresh,
Model.CheckedOutBranch, the authors, and the restored commit selection —
in a single onUIThreadUnlessRepoChanged bounce. The selection restore
(SelectHeadCommit / KeepCommitSelectionByHash) has to run in the bounce
because it reads the freshly-loaded commits; the FocusLine scroll is
enqueued from within the bounce so it still runs after refreshView's
re-render, as before.
refForLog no longer writes Model.BisectInfo as a side effect; it returns
the bisect info it read, and the bounce writes it, keeping that model
write on the UI thread. No caller reads Model.BisectInfo synchronously
after a refresh (the bisect controller reads Git().Bisect.GetInfo()
directly), so this is safe.
refreshCommitsAndCommitFiles's post-refresh re-init of the commit files
context depends on that restored selection, so it reads the selection in
a bounce and dispatches the commit-files git work back to a worker.
LocalCommitsMutex / AuthorsMutex are left in place for the mutex cleanup.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refreshGithubPullRequests and setGithubPullRequests now do their network
work on the worker and write Model.PullRequests / PullRequestsMap in an
onUIThreadUnlessRepoChanged bounce (the "no github remotes" and "no base
remote" early-returns clear them the same way). rebuildPullRequestsMap
moves into the bounce so the map is built from Model.Branches and
Model.Remotes as they stand on the UI thread — after those scopes'
refreshes have applied their own bounces — rather than from whatever the
worker happened to see.
The remaining worker-side reads of Model.Branches (to pick which upstream
branches to query) are the same not-yet-addressed worker-read race that
applies to the other bounced scopes.
RefreshingPullRequestsMutex is left in place for the mutex cleanup.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refreshStatus computes the status line on the calling goroutine (as
before) but now writes it to the status view in an
onUIThreadUnlessRepoChanged bounce rather than calling SetViewContent
directly from the worker. RefreshingStatusMutex is left in place for now;
it only guards the compute phase between concurrent callers and comes out
in the mutex cleanup.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refreshReflogCommits now does the git fetch on the worker and computes
the new ReflogCommits / FilteredReflogCommits values (still reading the
existing slices for the incremental prepend), then writes them in an
onUIThreadUnlessRepoChanged bounce. The freshly-computed reflog is still
returned for the branches load, so recency sorting is unaffected by the
write now landing on the UI thread (see the previous commit).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
BranchLoader.Load reads the reflog commits to sort branches by recency.
Today it reads them straight from Model.ReflogCommits, which works
because in the recency path the reflog refresh writes that field
synchronously just before the branches refresh reads it (same goroutine,
sequential).
An upcoming commit bounces the reflog model write onto the UI thread, at
which point Model.ReflogCommits wouldn't be updated yet when branches
runs — branches would sort by the previous refresh's reflog. To decouple
the branches load from *when* that write lands, pass the reflog commits
to refreshBranches explicitly: refreshReflogCommits now returns the
commits it loaded, and the recency path hands them straight to
refreshBranches. The non-recency path (branches and reflog run
concurrently, as before) keeps passing Model.ReflogCommits.
Pure refactor: behavior is identical, since the value passed is exactly
what Load read from the model before.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refreshRemotes now loads the remotes on the worker and writes
Model.Remotes, rebuilds the pull-requests map, and updates the selected
remote's RemoteBranches inside an onUIThreadUnlessRepoChanged bounce.
RemotesController.addAndCheckoutRemote read Model.Remotes right after its
SYNC REMOTES refresh to select the newly-added remote; since that write
now bounces, the selection (and the follow-up fetch) move into Then so
they run against the post-refresh model rather than the stale one.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refreshSubCommitsWithLimit now loads the sub-commits on the worker and
writes Model.SubCommits (and folds their authors into Model.Authors via
RefreshAuthors) inside an onUIThreadUnlessRepoChanged bounce.
SubCommitsMutex and AuthorsMutex are left in place: the former is shared
with setSubCommits, the latter with the commits refresh's RefreshAuthors
call, so both come out only once those other writers are on the UI thread
too.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refreshRebaseCommits now computes the merged rebasing commits and working
tree state on the worker and writes Model.Commits /
WorkingTreeStateAtLastCommitRefresh in an onUIThreadUnlessRepoChanged
bounce. LocalCommitsMutex is left in place for now; it's shared with the
commits and branches refreshes and comes out once they're all bounced.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refreshWorktrees now writes Model.Worktrees in an
onUIThreadUnlessRepoChanged bounce. loadWorktrees becomes a pure loader
that returns the worktrees instead of writing them, since it's shared
with refreshBranches; refreshWorktrees bounces the result, and the
branches call site writes it directly for now (that write moves into
refreshBranches's own bounce when that scope is migrated).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refreshStashEntries now loads the stash entries on the worker and writes
Model.StashEntries in an onUIThreadUnlessRepoChanged bounce.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refreshTags now captures the repo generation, loads the tags on the
worker, and writes Model.Tags in an onUIThreadUnlessRepoChanged bounce
rather than directly from the worker goroutine.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refreshCommitFilesContext now enqueues the Model.CommitFiles write and
CommitFileTreeViewModel.SetTree() call via OnUIThread, instead of running
them directly on the worker goroutine that drives async refreshes. This is
what makes moving SwitchToDiffFilesController's post-refresh work into Then
(previous commit) actually necessary, rather than just future-proofing.
Same repo-switch hazard as the FILES bounce, closed the same way: it
captures the repo generation before the git work and bounces through
onUIThreadUnlessRepoChanged, so the write is dropped if the user switched
repos while it was in flight.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
