A command task streams its output into a view from its own goroutine. To
track soft-wraps (so cursor-positioning escapes from a pager land on the
right line) the write path read the view's live InnerWidth, and the pty
setup read its InnerSize -- both off the UI thread, racing the UI thread
mutating the view's dimensions during layout.
Capture the width on the UI thread instead and hand it to the task: the
escape interpreter keeps a screenColMax it reads from, seeded in NewView
and refreshed per render via View.SetContentWidth (called from
newCmdTask/newPtyTask before the task's goroutine starts), and the pty
size is computed in the after-layout callback rather than in the task's
start func. The view's dimensions stay UI-thread-only; the task uses the
snapshot rather than reading them live.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When a task renders different content to a view (a new task key), the
view's scroll origin is reset to the top via onNewKey. That ran on the
task's own goroutine, racing the UI thread, which reads the origin
(OriginY) while laying out and drawing the view -- the single largest
source of view-render data races.
Give ViewBufferManager a bounce primitive (onUIThread) that runs a
function on the UI thread and waits for it, and reset the origin through
it. This is the first use of the primitive; subsequent commits route the
rest of the task's view mutations through it too, so that the view is
only ever touched on the UI thread. It runs as background work
(OnUIThreadAndWaitBackground) so rendering doesn't count towards the app
being busy, matching how the render's gocui task is already created.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The closures that render static content to a main view (newStringTask
and friends) ran on the ViewBufferManager's task goroutine, calling
SetViewContent/SetOrigin/ResetViewOrigin directly on the view. Those
touch view state (the line buffer, hover cells, the origin) that the UI
thread concurrently reads and mutates while laying out and drawing, so
they raced it -- e.g. a string task's SetContent clearing the view's
lines while the UI thread's CopyContent read them, or its SetOrigin
racing the layout's OriginY read.
Bounce the whole closure onto the UI thread instead, so the view is only
touched there. The bounce blocks (OnUIThreadAndWaitBackground) so the
task still completes only once the content has actually been rendered,
which the integration-test idle detection relies on; the background
variant keeps it from counting towards the app being busy, matching the
existing treatment of view rendering as work that must not block a repo
switch.
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>
The transient contexts (remoteBranches, subCommits, commitFiles) take
over the window of the context they are drilled into from, but until
then they carry a hardcoded initial window ("branches" or
"commits"). Under a gui.sidePanels config where those tabs aren't
their panel's first, no window of that name exists, leaving the
window-to-view map with entries for windows the layout never
produces. The previous commit made such entries harmless, but there's
no reason to have contexts point at nonexistent windows in the first
place; assign them the window hosting branches or commits instead,
which the config validation guarantees to exist.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
With gui.sidePanels, a panel's gocui window is named after its first
tab, so when branches is grouped behind, say, worktrees, there is no
window called "branches" at all. The transient contexts
(remoteBranches, subCommits, commitFiles) initially point at the
windows "branches" and "commits", and layout() showed their views
whenever the window-to-view map named them as their window's current
view — without checking that the window exists in the layout. Since
the map is seeded from the contexts themselves, a window that no
panel owns keeps naming a transient view as its current view, and
that view had just been parked at full screen size (the fallback for
views in unlaid-out windows), so it covered every side panel below it
in z-order.
Only show a transient view if its window actually received dimensions
in this layout.
Fixes#5823.
Co-Authored-By: Claude Fable 5 <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>
Running the integration tests in a loop under the race detector
eventually hung in demo/bisect. The goroutine dump shows the cycle: a
background worker's task.Done() held the task manager's mutex while
blocking on the unbuffered idle-listener channel send, and the test
runner goroutine — the only reader of that channel — was itself blocked
in NewTask on that same mutex, on its way to enqueueing a caption
render (SetCaption -> Render -> OnUIThread). Neither side could
proceed: the notification couldn't be delivered until the test
goroutine got the mutex, and the mutex couldn't be released until the
notification was delivered.
The root problem is that the busy-to-idle notification is a blocking
rendezvous performed while holding the mutex, so it needs the waiter's
cooperation at a moment where the waiter may legitimately need the
mutex first.
Make the notification fire-and-forget instead: WaitUntilIdle waits on a
condition variable and re-checks "is any task busy?" under the mutex,
and the busy-to-idle transition broadcasts, which never blocks. Waiting
is now level-triggered rather than edge-triggered, which is also more
robust: a wait can no longer be satisfied by a stale idle transition
produced by an unrelated background task, because the predicate is
evaluated against the current state. This relies on the previous commit
having made replayed input events carry their task from submission;
without that, the wait could return in the window where an event is in
flight but not yet picked up by the main loop.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Integration tests synchronize with lazygit through the task manager:
after submitting an input event, the test driver waits until the
program goes idle before asserting. But a submitted event only got its
task once the main loop picked it up from the events channel; while it
was still in flight (handed to the poller goroutine, or sitting in the
channel), no task existed for it, so the program could look idle even
though input was still pending.
The edge-triggered idle protocol mostly papers over this: each wait is
satisfied by the *next* busy-to-idle transition, which in practice is
the one produced by processing the submitted event. It only goes wrong
when some other task (e.g. a background refresh) completes in that
window, producing an edge the waiting test mistakes for its own — a
rare source of test flakes. The next commit replaces that protocol
with a level-triggered one, for which the window would be fatal rather
than rare: a wait falling into the gap would return immediately.
Close the gap by creating the task on the test goroutine before the
event is submitted, and carrying it through the poller into the main
loop, which uses it instead of creating its own. The new Replay*
methods own this invariant, and the replayed-events channels are no
longer exported, so tests can't submit an untracked event.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Resolve the pkg/gocui/gui.go conflict by keeping master's background-task
structure (Update/update(background), taskManager) and applying the
unbounded user-event queue on top — the same end state as if the fix had
been written on master directly.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Now that the queue is unbounded, its depth is a useful signal for
understanding how the event loop behaves under load — and we expect it
to look very different across builds (e.g. master, which carries the
bounce-state-updates-to-ui-thread work, versus the v0.63.0 release this
fix ships in). Track the deepest the queue has ever been and log an Info
line whenever that record is broken, so the numbers show up in the log
for later reasoning. The mark is session-wide and doesn't reset when the
queue drains.
gocui has no logger of its own, so it exposes the new depth through a
handler (matching the existing SetFocusHandler / SetOpenHyperlinkFunc
pattern) that the gui registers to log via its own logger.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
startBackgroundFetch assigned the field from its own goroutine, and
only after the initial fetch had completed, while the UI thread reads
it in triggerImmediateFetch on every repo switch, with no
synchronization.
Create the channel in startBackgroundRoutines instead, which runs on
the UI thread before the fetch goroutine is spawned; everything the UI
thread does afterwards is ordered after the write, so the read is
race-free without any locking. To make this possible, goEvery now
takes the retrigger channel as a parameter instead of creating and
returning it; callers that have no use for a retrigger channel pass
nil, and a nil channel in a select is simply never ready.
As a side effect, a repo switch that happens before the fetch loop has
started (during the intro popup or the initial fetch) now latches a
trigger and causes an immediate fetch once the loop is running, where
previously it was silently dropped.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Switching repos triggers an immediate background fetch by sending on
the goEvery retrigger channel. The send was blocking, but the goEvery
loop only receives between callbacks: while a fetch is in flight, it
waits for that fetch to finish before returning to its select. So a
repo switch that landed while a fetch was in flight would stall the UI
thread for the remainder of the fetch.
Worse, since worker refreshes capture state on the UI thread with a
blocking OnUIThreadAndWaitBackground call, the in-flight fetch's
post-fetch refresh can itself be waiting for the UI thread, turning
that stall into a deadlock cycle:
UI thread: switchTo -> triggerImmediateFetch, blocking send
goEvery loop: waiting for the in-flight fetch to finish
fetch worker: PostFetchRefresh -> RefreshFromWorker, waiting for
the UI thread
Make the send non-blocking, and give the channel a buffer of one so
that a trigger arriving while a fetch is in flight is latched rather
than dropped; that fetch is fetching the previous repo, so we still
need another one after it. The goEvery loop picks the trigger up as
soon as it returns to its select, and concurrent triggers coalesce.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The model<->view index conversions were derived from arrays that only
renderLines populated. That made them depend on the list having been
rendered (so a conversion before the first render ignored the non-model
items), and it made them go stale whenever the model changed after a
render: converting an index then returned a wrong result, and once the
model had grown past the last rendered length the conversion indexed a
too-short array and panicked (seen in cherry_pick under -race).
The conversion is a pure function of the current list length and the
current non-model items, and needs none of the rendered display strings.
Compute it directly and drop the cached arrays, so the result is always
consistent with the current model and no longer depends on rendering.
searchModelCommits converts every commit's index, and building the
non-model items can be O(len) mid-rebase, so it would now be quadratic;
snapshot the non-model items once via modelToViewIndexConverter instead
of rebuilding them per index.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ModelIndexToViewIndex and ViewIndexToModelIndex read conversion arrays
that only renderLines populates. So converting an index before the list
has been rendered ignores the non-model items (e.g. section headers) and
returns a wrong result; the same staleness makes a conversion after the
model has grown index a too-short array and panic (seen in cherry_pick
under -race).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reading more lines into a lazy-loaded view (e.g. a diff being scrolled)
never changes the window layout, and after the first screenful it
doesn't even change the visible content - the new lines land below the
viewport, so the only thing that changes on screen is the scrollbar
thumb. Yet each read triggered a full render: a layout pass plus a
redraw of every view. On a slow terminal that full-screen repaint on
every read is a big part of why scrolling through a not-yet-fully-read
diff stutters.
Route the task's refresh through a content-only render instead. It
skips the layout pass and only redraws the views whose content changed,
leaving tcell's cell-level dirty tracking to emit just the cells that
actually differ (in the steady state, the scrollbar column).
Co-Authored-By: Claude Opus 4.8 (1M context) <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>
LogAction and LogCommand are called from git worker goroutines (every
command a worker runs logs itself, and controllers log an action before
kicking off their worker), where they set the Extras view's Autoscroll
flag and append to GuiLog while the UI thread reads both when it lays out
and draws the view. Bounce the writes onto the UI thread instead.
Use the background variant so the bounce doesn't count towards lazygit
being busy: writing the command log is incidental display work, and a
foreground task would let an in-flight log write refuse a concurrent repo
switch (the same reason view-buffer renders and toasts are backgrounded).
Ordering between successive log calls is preserved by the bounce FIFO.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GetStatusString and HasStatus read the statuses slice without holding
the mutex that addStatus and removeStatus take when they mutate it. The
readers run on the spinner-render worker (which polls GetStatusString
every frame) while removeStatus fires from the waiting-status and
toast-expiry goroutines, so the unguarded reads race the concurrent
writes. Take the mutex in the readers too.
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.
CheckoutRef and ResetToRef set this flag from their worker goroutine
(to load fewer commits for speed) while the commits refresh reads it on
the UI thread in captureCommitsState to decide how many to load — a data
race. Make it an atomic.Bool so those writes are safe where they are,
rather than routing the flag through a refresh intent. Precedent:
Branch.BehindBaseBranch.
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.
