fix: move expensive operations off UI event loop and fix stale preview pane#253
fix: move expensive operations off UI event loop and fix stale preview pane#253miamachine wants to merge 6 commits intosmtg-ai:mainfrom
Conversation
The tickUpdateMetadataMessage handler was running expensive git and tmux operations synchronously on the Bubble Tea event loop. For large repos (e.g. 225K files / 30GB), each tick spawned `git add -N .` and `git diff` for every active instance sequentially — taking 10+ seconds and blocking all keypress processing during that time. Move the per-instance metadata work (HasUpdated + diff computation) into a tea.Cmd that runs in a background goroutine, and parallelize across instances using a WaitGroup. Results are sent back as a message and applied on the main goroutine to avoid data races with View. This reduces UI blocking from ~13s (5 instances × ~2.6s serial) to near-zero, since the event loop returns immediately and the I/O runs concurrently. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
|
Thank you for your contribution! Please sign the CLA before we can merge your pull request. You can sign the CLA by just posting a comment following the below format. I have read the CLA Document and I hereby sign the CLA 0 out of 3 committers have signed the CLA. |
|
I have read the CLA Document and I hereby sign the CLA (code was co-written with Claude, and this human reviewed the changes thoroughly and tested manually as noted above -- for more context, I've been using this tool with an extremely large monolith and the update loop was making the tool practically unusable for my needs) |
|
recheck |
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
mufeez-amjad
left a comment
There was a problem hiding this comment.
@miamachine Thanks for the PR!
I think you can simplify this – this should work with just one self-chaining Cmd:
func tickUpdateMetadataCmd(instances []*session.Instance) tea.Cmd {
return func() tea.Msg {
time.Sleep(500 * time.Millisecond)
// filter active, spawn goroutines, wg.Wait()
return metadataUpdateDoneMsg{results: results}
}
}Run instance.Start(true) as an async tea.Cmd instead of blocking the bubbletea Update handler. This keeps the UI responsive during git worktree setup, tmux session creation, and trust screen detection (which can take 30-60+ seconds, especially with CrowdStrike). Also fixes a bug where newInstanceFinalizer() was called twice per session creation, double-counting the repo in the multi-repo display. Co-Authored-By: Claude <noreply@anthropic.com>
After pressing Ctrl+Q to detach from an attached tmux session, the preview pane was not updated to reflect the current instance state. The onDismiss callback set m.state = stateDefault but never called m.instanceChanged(), so the preview stayed frozen until the next tick happened to fire. This matches the pattern used by every other showHelpScreen callback (checkout, kill, etc.). Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
|
Similar PR: #249 |
Per reviewer suggestion: merge tickUpdateMetadataCmd and runMetadataUpdateCmd into one self-chaining function. The sleep, filtering, parallel goroutines, and wg.Wait all happen in a single Cmd closure that returns metadataUpdateDoneMsg directly. This eliminates the intermediate tickUpdateMetadataMessage type, the metadataUpdating guard flag, and the two-step dispatch, making the intent clearer and the control flow simpler. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
done @mufeez-amjad !
merged the sleep + filtering + goroutines + wg.Wait() all into a single self-chaining tickUpdateMetadataCmd(instances []*session.Instance) tea.Cmd. This eliminates the intermediate tickUpdateMetadataMessage type, the metadataUpdating guard flag, and the two-step dispatch. Much cleaner — thanks for the suggestion.
resolves conflicts by: - adopting upstream's instanceStartedMsg for async start (already merged) - keeping our self-chaining tickUpdateMetadataCmd with parallel goroutines - adding upstream's CheckAndHandleTrustPrompt to metadata goroutine - merging upstream's terminal tab feature - removing duplicate Loading case in list.go Co-Authored-By: Claude <noreply@anthropic.com>
|
recheck |
|
re: CLA idk why but my username is weird -- @miaemyle and @miamachine point to the same user though not sure why only the former shows up (which might be an issue for the bot to approve this PR) |
Summary
Three related fixes that improve Claude Squad UI responsiveness and fix stale preview state:
1. Move metadata updates off the UI event loop
tickUpdateMetadataMessagehandler runsgit add -N .+git difffor every active instance synchronously on the Bubble Tea event loop. On large repos (225K files / 30GB), each instance takes ~2.6s, so 5 instances blocks all input processing for ~13 seconds per tick.HasUpdated+ diff computation) into atea.Cmdthat runs in a background goroutine, and parallelize across instances with async.WaitGroup. Results are sent back as ametadataUpdateDoneMsgand applied on the main goroutine to avoid data races withView.metadataUpdatingflag prevents overlapping ticks from spawning concurrent updates.2. Move session creation off the UI event loop
instance.Start(true)(git worktree setup, tmux session creation, trust screen detection) blocks the bubbleteaUpdatehandler for 30-60+ seconds, especially with CrowdStrike.instance.Start(true)as an asynctea.Cmdwith aLoadingstatus for visual feedback (spinner). Guards prevent user interaction with not-yet-started instances.newInstanceFinalizer()was called twice per session creation, double-counting the repo in the multi-repo display.3. Refresh preview pane after Ctrl+Q detach
onDismisscallback setm.state = stateDefaultbut never calledm.instanceChanged(), so the preview pane stayed frozen showing stale content from before the attach.m.instanceChanged()call after detach, matching the pattern used by every othershowHelpScreencallback (checkout, kill, etc.).Impact
Test plan
go build ./...compiles cleanlycswith 5+ instances on a large repo, verify input is responsive🤖 Generated with Claude Code