Coalesce repos:changed refetch so stale fetches can't leave removed projects as ghost rows (#7020)#7083
Conversation
…ed projects (stablyai#7020) Deleting a project group with "Remove contained projects" could leave the removed projects as stale, unusable sidebar rows until restart. `repos:changed` is broadcast once per persisted mutation, so the group delete plus each contained-project removal emit N+1 events. The renderer handled every event by firing three independent, unsequenced fetches (`fetchProjectGroups` / `fetchFolderWorkspaces` / `fetchRepos`). An early fetch — dispatched for the group-delete event before the project removals had persisted — could resolve last and overwrite the store, reintroducing the just-removed projects in memory even though persistence had dropped them. Restart "fixed" it only because startup reloads the final state. Route the repos:changed refresh through a serial coalescing runner (`createReposChangedRefetchRunner`) so refetches never overlap and a trailing run always executes after the final event, letting the store settle on the final persisted state. The generic coalescing primitive is promoted from the git-status domain to `lib/coalesced-poll-runner` since it now has two consumers. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
📝 WalkthroughWalkthroughChangesThis pull request introduces a coalescing runner for handling Sequence Diagram(s)Included above in the hidden review stack artifact. Related PRs: None identified. Suggested labels: refactor, hooks, tests Suggested reviewers: None identified. Poem 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/renderer/src/hooks/repos-changed-refetch.ts (1)
27-47: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSequential awaits vs
Promise.all: inconsistent and fragile on partial failure.The runtime-active branch awaits
fetchReposForAllHosts→fetchProjectGroupsForAllHosts→fetchFolderWorkspacesForAllHostssequentially, while the default branch runs the equivalent three fetches concurrently viaPromise.all. These three calls appear independent (no data passed between them), so the sequential version adds unnecessary latency (sum vs. max) and, if the first call rejects, the other two never even start in this cycle — leaving those slices stale until the nextrepos:changedevent. If there's no ordering dependency, unify both branches to run concurrently.♻️ Proposed fix
if (options.isRuntimeEnvironmentActive()) { // Why: the all-host sidebar still shows local repos while a runtime is // focused, so refresh every host's slice without dropping the // runtime-owned slices already shown. - await actions.fetchReposForAllHosts() - await actions.fetchProjectGroupsForAllHosts() - await actions.fetchFolderWorkspacesForAllHosts() + await Promise.all([ + actions.fetchReposForAllHosts(), + actions.fetchProjectGroupsForAllHosts(), + actions.fetchFolderWorkspacesForAllHosts() + ]) return }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0e900c86-018b-4eb8-a020-395f8a6b4925
📒 Files selected for processing (6)
src/renderer/src/components/right-sidebar/useGitStatusPolling.tssrc/renderer/src/hooks/repos-changed-refetch.test.tssrc/renderer/src/hooks/repos-changed-refetch.tssrc/renderer/src/hooks/useIpcEvents.tssrc/renderer/src/lib/coalesced-poll-runner.test.tssrc/renderer/src/lib/coalesced-poll-runner.ts
|
Thanks for this, @xianjianlf2 — really thoughtful work. Your root-cause writeup is spot-on, and the coalescing-runner approach is genuinely nice: it serializes the whole After evaluating both approaches for the same issue (#7020), we went with #7024, which has now merged and closed the issue. The deciding factors were scope and blast radius: #7024 is a surgical, store-level generation guard on That said, the broader-path coverage your PR provides — the runtime-active |
Summary
Fixes #7020.
Deleting a Project Group with "Remove contained projects" could leave the removed projects as stale, unusable rows in the sidebar until the app was restarted.
Root cause
repos:changedis broadcast once per persisted mutation, so a group delete that also removes N contained projects emits N+1 events. The renderer'srepos.onChangedhandler reacted to every event by firing three independent, unsequenced fetches (fetchProjectGroups/fetchFolderWorkspaces/fetchRepos, or their all-hosts variants).Because the fetches were unsequenced, an early fetch — dispatched for the group-delete event, before the per-project removals had persisted — could resolve last and overwrite the store with a snapshot that still contained the projects, reintroducing them in memory even though persistence had already dropped them. A restart "fixed" it only because startup reloads the final persisted state.
Fix
Route the
repos:changedrefresh through a serial coalescing runner (createReposChangedRefetchRunner, built on the existingcreateCoalescedPollRunner):The last event therefore always drives a refetch whose read happens at-or-after the final mutation, and serialization guarantees that refetch is the one that applies last. As a bonus, an N+1-event burst now performs ~2 refetch cycles instead of N+1.
The generic coalescing primitive is promoted from the git-status domain to
src/renderer/src/lib/coalesced-poll-runner.ts(with its test), since it now has two consumers;useGitStatusPollingis updated to import from the new location.Testing
repos-changed-refetch.test.tscovers: burst coalescing into a single trailing refetch, immediate run for an isolated event, the runtime-active all-hosts path, and dispose dropping a queued trailing run.coalesced-poll-runner,useGitStatusPolling, anduseIpcEventssuites pass unchanged.oxlint(incl. react-doctor) and webtsgotypecheck pass.🤖 Generated with Claude Code
X: @mark86202384100