Remove sidebar row-ids preference aggregation feeding the layout livelock (#2586)

[lawrencecchen]

Summary

• Removes SidebarWorkspaceRowIdsPreferenceKey entirely: the per-row/per-group-header .preference emitters, the sidebar-wide .onPreferenceChange aggregation, and the laidOutWorkspaceRowIds @State.
• Selected-workspace scroll-into-view now calls ScrollViewProxy.scrollTo unconditionally for pending requests. scrollTo resolves .id() values in lazy containers without the target row being realized, and unknown ids are a harmless no-op, so the "wait until the row is laid out" gate was unnecessary.

Why

This is the surviving cmux-owned edge in the #2586 sidebar layout livelock. Fresh evidence from today (0.64.14-nightly, macOS 26.5, 36 workspaces, no groups — well below the 100-workspace scale discussed earlier): scrolling the sidebar pegged the main thread at 100% inside a single GraphHost.flushTransactions() that never returned to the run loop. Two sample captures minutes apart show the identical signature:

• ~99% of main-thread samples inside one NSHostingView.beginTransaction run-loop observer callback, looping runTransaction → LazySubviewPlacements.placeSubviews over the sidebar ForEach.
• SidebarWorkspaceRowIdsPreferenceKey.reduce → Set.formUnion in the hot loop (the Fix sidebar workspace frame collection virtualization #5325-gated SidebarWorkspaceRowFramePreferenceKey is absent, consistent with vlechemin's 0.64.13 table).
• The refill side of the loop is visible too: LazyLayoutCacheItem.AllItemsPhaseMutation → LazyLayoutViewCache.updateItemPhases → propagate_dirty, plus LazyLayoutViewCache.signalPrefetch → NSHostingView.requestUpdate queuing the next transaction.
• Because the observer callback never returns, the autorelease pool never drains: RSS grew ~10 MB/s to 6.1 GB before the process was killed (heap: 4.3M NSCompositeAppearance, 8.6M NSMutableDictionary).

Mechanism: the preference value is the set of currently-realized lazy rows, which changes on essentially every layout pass while scrolling. Each change re-runs the N-row reduce and fires onPreferenceChange, which writes @State from inside the update cycle, scheduling another transaction inside the same flushTransactions loop. Once lazy phase churn keeps the realized-row set oscillating, the cycle stops converging and the main thread never escapes.

This is the same class-elimination #5325 applied to the sibling drop-target anchor preference: delete the steady-state per-row preference traffic instead of patching timing. With this PR the workspace sidebar publishes no per-row preferences at rest at all.

How this is principled vs hacky

Principled: it deletes the feedback mechanism rather than debouncing or gating it, and the replacement (scrollTo on lazy-container ids) is the API's intended use. Residual risk: if some macOS build fails to resolve an unrealized row id, scroll-into-view for far-off-screen rows would silently no-op — cmuxUITests/WorkspaceSidebarScrollUITests (Cmd+1 from the bottom of a 20-workspace overflow list) covers exactly that path on CI.

Testing

• WorkspaceSidebarScrollUITests via test-e2e.yml (covers selected-row scroll-into-view incl. off-screen Cmd+1 and Move-to-Top): run pending below.
• A deterministic automated repro of the livelock itself is not practical — the non-convergence is timing/AttributeGraph-state dependent and has resisted deterministic repro across every report in the issue thread. The meaningful verification is the existing scroll behavior suite plus dogfood under the reporter conditions (select tab + scroll).
• Tagged build rowids for dogfood.

Issues

• Related: Nightly freezes: sidebar LazyVStack layout loop pegs main thread at 100% CPU, deadlocks CLI #2586 (do not auto-close; secondary CLI dispatch_sync deadlock and any non-cmux-owned residual churn remain tracked there)

---

^{Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.}

---

Note

Medium Risk
Changes lazy-sidebar scroll behavior (no longer gated on realized rows); collapsed-group targeting is new but model-driven, with UI tests covering off-screen scroll paths.

Overview
Removes the sidebar layout feedback loop that caused main-thread livelock (#2586) by deleting SidebarWorkspaceRowIdsPreferenceKey, per-row/group .preference emitters, the sidebar .onPreferenceChange handler, and @State laidOutWorkspaceRowIds.

Selected-workspace scroll-into-view no longer waits for lazy rows to report as laid out. flushPendingSelectedWorkspaceScroll always calls ScrollViewProxy.scrollTo on an id resolved from model data via SidebarSelectedWorkspaceScrollPolicy.scrollTargetWorkspaceId (selected workspace, or the group header anchor when the group is collapsed and the member row has no .id). Scroll helpers now take WorkspaceListRenderContext instead of raw id lists or preference-derived row sets.

Unit tests cover the new scroll-target mapping for ungrouped, expanded-group, and collapsed-group cases.

^{Reviewed by Cursor Bugbot for commit 5bab011. Bugbot is set up for automated code reviews on this repo. Configure here.}

---

Summary by cubic

Removes the sidebar row-id preference aggregation and scroll gate to break the #2586 layout livelock. Scrolling now always targets the selected workspace (or its group header when collapsed) via ScrollViewProxy.scrollTo.

• Bug Fixes
  • Removed the row-id preference pipeline and state: SidebarWorkspaceRowIdsPreferenceKey, per-row/group .preferences, sidebar .onPreferenceChange, and laidOutWorkspaceRowIds.
  • Selected-workspace scroll now unconditionally calls scrollTo with a policy-resolved id; lazy id resolution handles unrealized rows and unknown ids as a no-op. Collapsed-group selections map to the group header via SidebarSelectedWorkspaceScrollPolicy.scrollTargetWorkspaceId; tests cover no-group/expanded/collapsed cases.
  • Imported Foundation in SidebarState for UUID.

^{Written for commit 5bab011. Summary will update on new commits.}

Summary by CodeRabbit

• Refactor
  • Improved workspace sidebar scrolling: selection now scrolls immediately and more reliably, with more consistent handling when navigating into collapsed groups so the correct header is brought into view.
• Tests
  • Added unit tests covering scroll behavior for open/closed group scenarios to ensure consistent navigation.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
cmux	Ready	Preview, Comment	Jun 9, 2026 6:33pm
cmux-staging	Building	Preview, Comment	Jun 9, 2026 6:33pm

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR removes PreferenceKey-based layout tracking for sidebar workspace rows and instead tracks a single pendingSelectedWorkspaceScrollId; scrolling now occurs immediately and unconditionally via ScrollViewProxy, with collapsed-group redirects handled by a computed scroll target.

Changes

Workspace scrolling refactoring

Layer / File(s)	Summary
Scroll state and logic refactoring Sources/ContentView.swift	pendingSelectedWorkspaceScrollId replaces laid-out-ID tracking; request/flush logic updated to accept WorkspaceListRenderContext and call flushPendingSelectedWorkspaceScroll(proxy, renderContext:), scrolling unconditionally to the computed target.
Event wiring for selection and workspace list Sources/ContentView.swift	Selection changes and renderContext.workspaceIds changes call requestSelectedWorkspaceScroll(...); when requests are suppressed the pending scroll is flushed using the provided renderContext rather than waiting for preference updates.
Preference key infrastructure removal Sources/ContentView.swift, Sources/VerticalTabsSidebar+WorkspaceGroups.swift	Removed .onPreferenceChange(SidebarWorkspaceRowIdsPreferenceKey.self), per-row .preference(...) emissions, the SidebarWorkspaceRowIdsPreferenceKey type, and the group header .preference(...) modifier; sidebarWorkspaceFrameAnchor usage remains.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant ContentView
  participant ScrollViewProxy
  participant GroupHeaderAnchor

  User->>ContentView: change selected workspace
  ContentView->>ContentView: set pendingSelectedWorkspaceScrollId
  ContentView->>ScrollViewProxy: flushPendingSelectedWorkspaceScroll(scrollTarget)
  alt workspace belongs to collapsed group
    ScrollViewProxy->>GroupHeaderAnchor: scrollTo(groupAnchorId)
  else normal workspace
    ScrollViewProxy->>ScrollViewProxy: scrollTo(workspaceId)
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

• Ari4ka

Poem

A rabbit nudges the sidebar bright,
One pending ID, it hops to light.
No scattered prefs to track the trail,
A tidy scroll, a simple tale. 🐰✨

---

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (2 errors, 1 warning)

Check name	Status	Explanation	Resolution
Cmux Swift Blocking Runtime	❌ Error	PR introduces Task.sleep (150ms), DispatchQueue.asyncAfter delays, and NSLock in production code—all prohibited by swift-blocking-runtime.md rules.	Replace Task.sleep with proper signal/callback, DispatchQueue.asyncAfter with state transitions, and NSLock with MainActor-isolated state.
Cmux Algorithmic Complexity	❌ Error	requestSelectedWorkspaceScroll() uses O(n) Array.contains() on workspaceIds (n~1000) per tab selection event instead of O(1) tabIndexById[id] != nil.	Replace renderContext.workspaceIds.contains(selectedWorkspaceId) with renderContext.tabIndexById[selectedWorkspaceId] != nil for O(1) hot-path lookup.
Docstring Coverage	⚠️ Warning	Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (18 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: removing the sidebar row-ids preference aggregation that caused the layout livelock.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation	✅ Passed	New pure helper function with Sendable parameters (UUID, WorkspaceGroup?); no implicit MainActor issues, background context access, or mutable reference types without isolation.
Cmux Expensive Synchronous Load	✅ Passed	PR refactors scrolling with lightweight UI operations. No expensive loaders or disk I/O added to interactive paths per swift-expensive-sync-load.md rule.
Cmux Cache Substitution Correctness	✅ Passed	Scroll operations use fresh renderContext data from TabManager's @Published properties built each render, applied to transient UI hints only, not persisted values.
Cmux No Hacky Sleeps	✅ Passed	All PR changes are Swift files; check applies only to TypeScript, JavaScript, shell, and build/runtime scripts. Swift timing covered by separate check.
Cmux Swift Concurrency	✅ Passed	PR removes preference aggregation feedback loop causing livelock; introduces no new Dispatch queues, Combine app state, completion handlers, or Tasks.
Cmux Swift @Concurrent	✅ Passed	PR adds only synchronous, UI-bound Swift code: scrollTargetWorkspaceId helper and scroll-management functions in ContentView. No async, @concurrent, or nonisolated async work.
Cmux Swift File And Package Boundaries	✅ Passed	PR touches existing files only: ContentView -1 net line, SidebarState +14 to 105-line file, no new oversized files, no mixed responsibilities, proper unit test coverage.
Cmux Swift Logging	✅ Passed	PR adds one NSLog call in Sources/ContentView.swift guarded by #if DEBUG, which is an explicitly allowed case per swift-logging.md. No other logging violations in the modified files.
Cmux User-Facing Error Privacy	✅ Passed	PR contains no new user-facing error messages, alerts, or sensitive information; changes are internal implementation refactoring with developer-only comments.
Cmux Full Internationalization	✅ Passed	PR contains no new user-facing text or localization changes; only internal refactoring of scrolling logic and test code.
Cmux Swiftui State Layout	✅ Passed	PR removes preference key aggregation causing livelock, adds @State only in main sidebar container with event-handler mutations; no new @Published, render mutations, or row-level store refs.
Cmux Architecture Rethink	✅ Passed	Removes problematic preference observer pattern causing livelock. Replaces with single @State and direct scroll calls. No timing repairs, polling, locks, or split lifecycle ownership introduced.
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	No NSWindow, NSPanel, NSWindowController, or SwiftUI Window code added. PR modifies sidebar scrolling logic only (preference-key removal, @State approach, helper function). Check does not apply.
Cmux Source Artifacts	✅ Passed	All changed files are hand-written source code and unit tests in legitimate directories (Sources/, cmuxTests/). No artifact patterns, generated code markers, or prohibited directories detected.
Description check	✅ Passed	The PR description comprehensively covers the summary (what/why), testing approach, and context with related issue links, but lacks a demo video and incomplete checklist items.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue-2586-sidebar-rowids-livelock

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 830838bd69

ℹ️ 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".

[chatgpt-codex-connector]

Keep pending scrolls for hidden group children

When the selected workspace is a non-anchor child of a collapsed group (for example after the non-focus socket/CLI collapse path, which intentionally does not change selection), renderContext.workspaceIds still contains that UUID even though SidebarWorkspaceRenderItem.renderItems does not render a view with that .id. scrollTo is therefore an unknown-id no-op, but this now clears pendingSelectedWorkspaceScrollId; expanding the group later does not change workspaceIds or selectedTabId, so the selected row is never scrolled into view. Previously the pending request survived until the row preference appeared after expansion.

Useful? React with 👍 / 👎.

[lawrencecchen]

Addressed in 68adaa7: hidden members of a collapsed group now resolve their scroll target to the group header (anchor workspace id) from render-context data, so the scroll lands where the workspace lives instead of silently no-oping. Deliberately not restoring the deferred-scroll-on-expand: that required tracking what the lazy layout realized, which is the feedback edge this PR removes.

[greptile-apps]

Greptile Summary

This PR removes SidebarWorkspaceRowIdsPreferenceKey entirely to eliminate the feedback loop that caused the sidebar layout livelock (#2586): per-row .preference emitters → sidebar-wide onPreferenceChange → @State write during layout → another transaction → repeat. Scroll-into-view now calls ScrollViewProxy.scrollTo unconditionally, resolving IDs in lazy containers without requiring rows to be realized, with unknown IDs harmlessly no-oping.

• Deleted: SidebarWorkspaceRowIdsPreferenceKey, per-row and group-header .preference(key:value:) calls, laidOutWorkspaceRowIds @State, and the onPreferenceChange scroll-flush gate in VerticalTabsSidebar.
• Added: SidebarSelectedWorkspaceScrollPolicy.scrollTargetWorkspaceId — a pure, model-based function that redirects a collapsed-group selection to the group header's anchorWorkspaceId; all three cases are covered by new unit tests.
• Flush paths: requestSelectedWorkspaceScroll now flushes immediately; the workspace-order-change notification path still relies on onChange(of: renderContext.workspaceIds) as its implicit flush trigger (noted in a prior review comment as a documentation gap, unchanged by this PR).

Confidence Score: 5/5

Safe to merge: the change deletes a well-identified feedback loop rather than patching it, the replacement API is the platform's intended mechanism, and the collapsed-group scroll target is decided from model data that is always present.

The deletion of SidebarWorkspaceRowIdsPreferenceKey and its surrounding @State write removes the exact mechanism driving the livelock, and the new unconditional scrollTo path is simpler with no new state or observers introduced. scrollTargetWorkspaceId is pure and deterministic; the three unit tests cover the behaviorally distinct cases. The only pre-existing gap — the implicit coupling between the workspace-order-change notification path and onChange(workspaceIds) as its flush trigger — was already noted in a prior review and is unchanged by this PR.

No files require special attention; all changes are deletions or small, self-contained additions.

Important Files Changed

Filename	Overview
Sources/ContentView.swift	Removes laidOutWorkspaceRowIds state, SidebarWorkspaceRowIdsPreferenceKey struct, all .preference emitters, and the onPreferenceChange flush gate; scroll helpers now accept WorkspaceListRenderContext and flush unconditionally. Logic is correct and flush paths remain intact.
Sources/Sidebar/SidebarState.swift	Adds scrollTargetWorkspaceId to SidebarSelectedWorkspaceScrollPolicy — a pure, model-driven function that maps a collapsed-group workspace selection to the group header's anchor ID. Implementation is clean and well-documented.
Sources/VerticalTabsSidebar+WorkspaceGroups.swift	Removes the single .preference(key: SidebarWorkspaceRowIdsPreferenceKey.self, value: Set([group.anchorWorkspaceId])) call from the group header view — a clean deletion with no other changes.
cmuxTests/SidebarWorkspaceSnapshotRefreshPolicyTests.swift	Adds three unit tests covering no-group, expanded-group, and collapsed-group cases for scrollTargetWorkspaceId; uses a makeGroup factory helper. Tests are correct and cover the behaviorally important collapsed case.

_{Reviews (3): Last reviewed commit: "Import Foundation in SidebarState for UU..." | Re-trigger Greptile}