Repository navigation
Fix sidebar Running/Needs-input pills attaching to wrong workspace on cold load (#7519) - #8062
andresespinosapc wants to merge 2 commits into
Conversation
…anaflow-ai#7519) TabItemView memoizes its per-row sidebar snapshot (statusEntries pills and activeCodingAgentCount) in workspaceSnapshotStorage / workspaceSnapshotScratch, guarded only by SidebarWorkspaceSnapshotBuilder.PresentationKey. That key was built from global presentation settings alone, with nothing identifying the workspace, so every row's key compared equal. The sidebar list is a LazyVStack (enforced by check-sidebar-lazy-layout.py). On cold load TabManager.restoreSessionSnapshot replaces the whole tabs array with workspaces that all get fresh UUIDs. When SwiftUI recycles a row's @State across that identity change, the memo returns the previously bound workspace's cached snapshot and the presentation-key guard cannot tell it belongs to a different workspace, so a neighbour's "Running" / "Needs input" pills render on the wrong tile. Include workspaceId (tab.id) in PresentationKey so a snapshot built for one workspace can never satisfy the cache guard of another; a recycled row then falls through to makeWorkspaceSnapshot() and rebuilds from its own tab. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
@andresespinosapc is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe snapshot presentation key now includes the workspace UUID from the tab. Tests cover workspace-specific key equality, cached snapshot matching, and prevention of cross-workspace reuse after row recycling. ChangesWorkspace snapshot cache identity
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 25✅ Passed checks (25 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR prevents sidebar status pills from appearing on the wrong workspace after row reuse. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (4): Last reviewed commit: "Add regression test for #7519 cold-load ..." | Re-trigger Greptile |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
@andresespinosapc I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 241,631 of the 240,000 allowed lines of code this month. Reviews resume on 1 August 2026 (in 18 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews. To help optimise your usage, you can tune cubic to get the most out of your usage limits:
|
|
✅ Action performedReview finished.
|
|
To use Codex here, create a Codex account and connect to github. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b939e5f62
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Assert PresentationKey distinguishes workspaces, and that a snapshot memoized for one workspace fails the cache guard of another (the recycled-row state) so its pills cannot be served on a neighbouring tile. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
0b939e5 to
8ab8ea9
Compare
|
Thank you for this! You had the cache-identity fix first; main now passes workspace-specific snapshots from a parent-owned, workspace-ID-keyed cache and removes the row-local memo in 0bffb5f :) |
Summary
What changed?
SidebarWorkspaceSnapshotBuilder.PresentationKeynow includes the workspace identity (workspaceId: tab.id). This key guardsTabItemView's per-row snapshot memo (workspaceSnapshotStorage/workspaceSnapshotScratch).Why? Fixes #7519 — on cold app load the sidebar Running / Needs input status pills attach to the wrong workspaces (a neighbour's pill renders on the wrong tile; selecting the tile shows a different state than the pill claims).
Root cause:
TabItemViewmemoizes its per-row sidebar snapshot — which carries the pill data (metadataEntries=statusEntries, andactiveCodingAgentCount) — and serves the cached value whenever the currentPresentationKeymatches the stored one:But
PresentationKeywas built from global presentation settings only (showsAgentActivity,visibleAuxiliaryDetails, …) with nothing identifying the workspace, so every row's key compared equal. The sidebar list is aLazyVStack(enforced byscripts/check-sidebar-lazy-layout.py). On cold load,TabManager.restoreSessionSnapshotreplaces the entiretabsarray with workspaces that all receive fresh UUIDs. When SwiftUI recycles a row's@Stateacross that identity change, the memo returns the previously bound workspace's cached snapshot, and the presentation-key guard cannot detect that it belongs to a different workspace — so the neighbour's pills render on the wrong tile until an observation-driven refresh happens to correct it.The data layer is not at fault:
statusEntries/agentLifecycleStatesByPanelIdare cleared on restore and re-seeded by UUID-resolved events (verified via a trace of the restore + hook-resolution paths). The mis-attribution is purely in the render-layer memo.The fix makes the cache correct-by-construction: a snapshot built for workspace A can never satisfy workspace B's cache guard, so a recycled row falls through to
makeWorkspaceSnapshot()and rebuilds from its owntab.Two commits:
Sources/ContentView.swift)cmuxTests/SidebarWorkspaceSnapshotRefreshPolicyTests.swift)Testing
cmux-unitscheme): addedpresentationKeyDistinguishesWorkspacesandmemoizedSnapshotIsNotServedForADifferentWorkspacetoSidebarWorkspaceSnapshotRefreshPolicyTests(already wired into the pbxproj). Both pass compiled into the realcmuxmodule:xcodebuild -project cmux.xcodeproj -scheme cmux-unit -configuration Debug -destination 'platform=macOS' -only-testing:cmuxTests/SidebarWorkspaceSnapshotRefreshPolicyTests CMUX_SKIP_ZIG_BUILD=1 testworkspaceId— reproduces the bug before the fix and confirms the cache miss after.Manual repro (matches the report): launch the app, shrink the window so the sidebar scrolls (forces
LazyVStackrow recycling), create ~10–12 workspaces (a couple with nested cwds, e.g.~/tmp/repro/elasticand~/tmp/repro/elastic/kiriver), put severalclaudesessions into distinct states (Running / Needs input / idle),Cmd-Q, and cold-relaunch. Compare each tile's pill against the pane's real state.Demo Video
LazyVStack@Staterecycling (timing-dependent). Deterministic coverage is provided by the unit test above; can add a screen recording of the manual repro on request.Review Trigger (Copy/Paste as PR comment)
Checklist
Related (not addressed here)
While tracing this I found two adjacent issues worth separate follow-up:
SessionIndexStore.swift:418(filteredEntriesForCurrentScope) uses a cwd prefix match with no longest-prefix / nearest-workspace ownership, so a parent workspace (~/projects/elastic) absorbs a nested child's (~/projects/elastic/kiriver) sessions in the Sessions panel. This is the nested-cwd pattern noted in the issue, but on a different surface than the sidebar pills.RestorableAgentSession.entry(workspaceId:panelId:)falls back to a panel-id-only key on cold load (workspace UUIDs are regenerated); safe while panel ids are unique, but fragile if a panel has moved workspaces.Fixes #7519
🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Fixes sidebar "Running"/"Needs input" pills attaching to the wrong workspace on cold launch by scoping snapshot caching per workspace. We now include the workspace ID in the snapshot
PresentationKeyso memoized snapshots are never reused across workspaces. Fixes #7519.workspaceId(tab.id) toSidebarWorkspaceSnapshotBuilder.PresentationKeyand pass it fromTabItemView.LazyVStackrows miss the cache and rebuild from their owntab.SidebarWorkspaceSnapshotRefreshPolicyTeststo verify keys differ per workspace and prevent serving another workspace’s snapshot.Written for commit 8ab8ea9. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests