From 830838bd6921d222e36d3411032e5208d6fae8b9 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 9 Jun 2026 11:14:20 -0700 Subject: [PATCH 1/4] Remove sidebar row-ids preference aggregation that fed the layout livelock The workspace sidebar published SidebarWorkspaceRowIdsPreferenceKey from every row and group header, reduced it sidebar-wide on every layout pass, and wrote the aggregated set into @State from onPreferenceChange to gate selected-workspace scroll-into-view on "row is laid out". During lazy layout churn (scroll, tab switch) that preference value changes on every pass, so each pass scheduled another transaction from inside GraphHost.flushTransactions; once the cycle stopped converging the main thread never returned to the run loop (100% CPU, unbounded autorelease growth, CLI dispatch_sync deadlock). ScrollViewProxy.scrollTo resolves .id() values in lazy containers without the row being realized, and unknown ids are a no-op, so the laid-out gate is unnecessary: scroll pending requests unconditionally and delete the preference key, the per-row emitters, the sidebar-wide onPreferenceChange, and the laidOutWorkspaceRowIds state. Same class-elimination as #5325 did for the sibling SidebarWorkspaceRowFramePreferenceKey (drop-target anchors), which no longer appears in hang samples since it shipped. Issue: https://github.com/manaflow-ai/cmux/issues/2586 Co-Authored-By: Claude Fable 5 --- Sources/ContentView.swift | 30 +++++++------------ .../VerticalTabsSidebar+WorkspaceGroups.swift | 1 - 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index 98e428feb631..47d52203287b 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -10585,7 +10585,6 @@ struct VerticalTabsSidebar: View { // row sitting behind the open menu. See `SidebarShortcutHintFreezePolicy`. @State private var frozenShortcutHintsTabId: UUID? @State private var frozenShortcutHintsValue: Bool = false - @State private var laidOutWorkspaceRowIds: Set = [] @State private var pendingSelectedWorkspaceScrollId: UUID? @State private var collapsedExtensionSidebarSectionIds: Set = [] @State private var extensionSidebarWorktreeCreationInFlightSectionIds: Set = [] @@ -10876,14 +10875,18 @@ struct VerticalTabsSidebar: View { flushPendingSelectedWorkspaceScroll(proxy) } - private func flushPendingSelectedWorkspaceScroll( - _ proxy: ScrollViewProxy, - laidOutWorkspaceRowIds: Set? = nil - ) { + private func flushPendingSelectedWorkspaceScroll(_ proxy: ScrollViewProxy) { guard let selectedWorkspaceId = pendingSelectedWorkspaceScrollId else { return } - let rowIds = laidOutWorkspaceRowIds ?? self.laidOutWorkspaceRowIds - guard rowIds.contains(selectedWorkspaceId) else { return } + // Scroll unconditionally: ScrollViewProxy resolves `.id(_:)` values in + // lazy containers without requiring the row to be realized, and an + // unknown id is a harmless no-op. The previous design gated this on a + // per-row "laid-out row ids" PreferenceKey aggregated across every + // sidebar row; that preference fed `@State` writes from inside the + // layout/preference update cycle and was the surviving cmux-owned edge + // in the sidebar layout livelock + // (https://github.com/manaflow-ai/cmux/issues/2586). + // // No anchor means SwiftUI scrolls the minimum needed to reveal the row. proxy.scrollTo(selectedWorkspaceId) pendingSelectedWorkspaceScrollId = nil @@ -11236,10 +11239,6 @@ struct VerticalTabsSidebar: View { lastSidebarSelectionIndex = index } } - .onPreferenceChange(SidebarWorkspaceRowIdsPreferenceKey.self) { rowIds in - laidOutWorkspaceRowIds = rowIds - flushPendingSelectedWorkspaceScroll(scrollProxy, laidOutWorkspaceRowIds: rowIds) - } } } } @@ -12625,7 +12624,6 @@ struct VerticalTabsSidebar: View { .equatable() .id(tab.id) .accessibilityIdentifier("sidebarWorkspace.\(tab.id.uuidString)") - .preference(key: SidebarWorkspaceRowIdsPreferenceKey.self, value: Set([tab.id])) row .sidebarWorkspaceFrameAnchor(id: tab.id, isEnabled: shouldCollectWorkspaceDropTargets) @@ -12638,14 +12636,6 @@ struct VerticalTabsSidebar: View { } } -struct SidebarWorkspaceRowIdsPreferenceKey: PreferenceKey { - static let defaultValue: Set = [] - - static func reduce(value: inout Set, nextValue: () -> Set) { - value.formUnion(nextValue()) - } -} - struct SidebarWorkspaceFrameAnchorModifier: ViewModifier { let id: UUID let isEnabled: Bool diff --git a/Sources/VerticalTabsSidebar+WorkspaceGroups.swift b/Sources/VerticalTabsSidebar+WorkspaceGroups.swift index e11fb6b8f6b4..1adbb75b1345 100644 --- a/Sources/VerticalTabsSidebar+WorkspaceGroups.swift +++ b/Sources/VerticalTabsSidebar+WorkspaceGroups.swift @@ -156,7 +156,6 @@ extension VerticalTabsSidebar { .equatable() .id(group.anchorWorkspaceId) .accessibilityIdentifier("sidebarWorkspaceGroup.\(group.id.uuidString)") - .preference(key: SidebarWorkspaceRowIdsPreferenceKey.self, value: Set([group.anchorWorkspaceId])) header .sidebarWorkspaceFrameAnchor( From 68adaa7b03996fcaf0c11fde92dd607d4c6635e4 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 9 Jun 2026 11:24:27 -0700 Subject: [PATCH 2/4] Scroll to the group header when the selected workspace is in a collapsed group Codex review: a member of a collapsed group renders no row, so scrollTo with its UUID is a no-op, and clearing the pending request dropped the old deferred-scroll-on-expand behavior. Resolve the scroll target from render-context data instead: hidden members target their group header (anchor workspace id), which is always present. Never decided from what the lazy layout realized, so it cannot re-enter the preference/layout feedback cycle. Co-Authored-By: Claude Fable 5 --- Sources/ContentView.swift | 41 ++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index 47d52203287b..8c74799c83e3 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -10864,18 +10864,24 @@ struct VerticalTabsSidebar: View { return KeyboardShortcutSettings.shortcut(for: .selectWorkspaceByNumber) } - private func requestSelectedWorkspaceScroll(_ proxy: ScrollViewProxy, workspaceIds: [UUID]) { + private func requestSelectedWorkspaceScroll( + _ proxy: ScrollViewProxy, + renderContext: WorkspaceListRenderContext + ) { guard let selectedWorkspaceId = tabManager.selectedTabId, - workspaceIds.contains(selectedWorkspaceId) else { + renderContext.workspaceIds.contains(selectedWorkspaceId) else { pendingSelectedWorkspaceScrollId = nil return } pendingSelectedWorkspaceScrollId = selectedWorkspaceId - flushPendingSelectedWorkspaceScroll(proxy) + flushPendingSelectedWorkspaceScroll(proxy, renderContext: renderContext) } - private func flushPendingSelectedWorkspaceScroll(_ proxy: ScrollViewProxy) { + private func flushPendingSelectedWorkspaceScroll( + _ proxy: ScrollViewProxy, + renderContext: WorkspaceListRenderContext + ) { guard let selectedWorkspaceId = pendingSelectedWorkspaceScrollId else { return } // Scroll unconditionally: ScrollViewProxy resolves `.id(_:)` values in @@ -10888,10 +10894,27 @@ struct VerticalTabsSidebar: View { // (https://github.com/manaflow-ai/cmux/issues/2586). // // No anchor means SwiftUI scrolls the minimum needed to reveal the row. - proxy.scrollTo(selectedWorkspaceId) + proxy.scrollTo(sidebarScrollTargetId(for: selectedWorkspaceId, renderContext: renderContext)) pendingSelectedWorkspaceScrollId = nil } + /// A member of a collapsed group has no row of its own, so its UUID is not + /// a scrollable `.id` and `scrollTo` would no-op. Target the group header + /// (which carries the anchor workspace id) so the scroll still lands where + /// the workspace lives. Decided purely from render-context data, never + /// from what the lazy layout happens to have realized. + private func sidebarScrollTargetId( + for workspaceId: UUID, + renderContext: WorkspaceListRenderContext + ) -> UUID { + guard let tab = renderContext.workspaceById[workspaceId], + let groupId = tab.groupId, + let group = renderContext.workspaceGroupById[groupId], + group.isCollapsed + else { return workspaceId } + return group.anchorWorkspaceId + } + private func shouldRequestSelectedWorkspaceScrollAfterWorkspaceIdsChange( from oldWorkspaceIds: [UUID], to newWorkspaceIds: [UUID] @@ -11176,20 +11199,20 @@ struct VerticalTabsSidebar: View { .background(Color.clear) .modifier(ClearScrollBackground()) .onAppear { - requestSelectedWorkspaceScroll(scrollProxy, workspaceIds: renderContext.workspaceIds) + requestSelectedWorkspaceScroll(scrollProxy, renderContext: renderContext) } .onChange(of: tabManager.selectedTabId) { _, _ in - requestSelectedWorkspaceScroll(scrollProxy, workspaceIds: renderContext.workspaceIds) + requestSelectedWorkspaceScroll(scrollProxy, renderContext: renderContext) } .onChange(of: renderContext.workspaceIds) { oldWorkspaceIds, newWorkspaceIds in guard shouldRequestSelectedWorkspaceScrollAfterWorkspaceIdsChange( from: oldWorkspaceIds, to: newWorkspaceIds ) else { - flushPendingSelectedWorkspaceScroll(scrollProxy) + flushPendingSelectedWorkspaceScroll(scrollProxy, renderContext: renderContext) return } - requestSelectedWorkspaceScroll(scrollProxy, workspaceIds: newWorkspaceIds) + requestSelectedWorkspaceScroll(scrollProxy, renderContext: renderContext) } .onReceive(NotificationCenter.default.publisher(for: .workspaceOrderDidChange)) { notification in requestSelectedWorkspaceScrollAfterWorkspaceOrderChange(notification) From 57eb3781690d0a2588dd1de24524c1829b2e1a44 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 9 Jun 2026 11:28:36 -0700 Subject: [PATCH 3/4] Move collapsed-group scroll target into SidebarSelectedWorkspaceScrollPolicy Keeps ContentView.swift under the Swift file length budget (workflow-guard-tests) and makes the collapsed-group fallback unit testable. Adds coverage: no group / expanded group target the workspace itself, collapsed group targets the group anchor. Co-Authored-By: Claude Fable 5 --- Sources/ContentView.swift | 36 +++++---------- Sources/Sidebar/SidebarState.swift | 13 ++++++ ...rWorkspaceSnapshotRefreshPolicyTests.swift | 45 +++++++++++++++++++ 3 files changed, 69 insertions(+), 25 deletions(-) diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index 8c74799c83e3..a6abb00c08c7 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -10887,34 +10887,20 @@ struct VerticalTabsSidebar: View { // Scroll unconditionally: ScrollViewProxy resolves `.id(_:)` values in // lazy containers without requiring the row to be realized, and an // unknown id is a harmless no-op. The previous design gated this on a - // per-row "laid-out row ids" PreferenceKey aggregated across every - // sidebar row; that preference fed `@State` writes from inside the - // layout/preference update cycle and was the surviving cmux-owned edge - // in the sidebar layout livelock - // (https://github.com/manaflow-ai/cmux/issues/2586). - // - // No anchor means SwiftUI scrolls the minimum needed to reveal the row. - proxy.scrollTo(sidebarScrollTargetId(for: selectedWorkspaceId, renderContext: renderContext)) + // per-row "laid-out row ids" PreferenceKey whose sidebar-wide reduce + // fed `@State` writes from inside the layout/preference update cycle, + // the cmux-owned edge in the sidebar layout livelock + // (https://github.com/manaflow-ai/cmux/issues/2586). No anchor means + // SwiftUI scrolls the minimum needed to reveal the row. + let group = renderContext.workspaceById[selectedWorkspaceId]?.groupId + .flatMap { renderContext.workspaceGroupById[$0] } + proxy.scrollTo(SidebarSelectedWorkspaceScrollPolicy.scrollTargetWorkspaceId( + selectedWorkspaceId: selectedWorkspaceId, + group: group + )) pendingSelectedWorkspaceScrollId = nil } - /// A member of a collapsed group has no row of its own, so its UUID is not - /// a scrollable `.id` and `scrollTo` would no-op. Target the group header - /// (which carries the anchor workspace id) so the scroll still lands where - /// the workspace lives. Decided purely from render-context data, never - /// from what the lazy layout happens to have realized. - private func sidebarScrollTargetId( - for workspaceId: UUID, - renderContext: WorkspaceListRenderContext - ) -> UUID { - guard let tab = renderContext.workspaceById[workspaceId], - let groupId = tab.groupId, - let group = renderContext.workspaceGroupById[groupId], - group.isCollapsed - else { return workspaceId } - return group.anchorWorkspaceId - } - private func shouldRequestSelectedWorkspaceScrollAfterWorkspaceIdsChange( from oldWorkspaceIds: [UUID], to newWorkspaceIds: [UUID] diff --git a/Sources/Sidebar/SidebarState.swift b/Sources/Sidebar/SidebarState.swift index 8672c01bb07c..dc4f842b1b0e 100644 --- a/Sources/Sidebar/SidebarState.swift +++ b/Sources/Sidebar/SidebarState.swift @@ -74,4 +74,17 @@ enum SidebarSelectedWorkspaceScrollPolicy { return true } + + /// A member of a collapsed group has no sidebar row of its own, so its + /// UUID is not a scrollable `.id` and `scrollTo` would no-op. Target the + /// group header (which carries the anchor workspace id) so the scroll + /// still lands where the workspace lives. Decided purely from model data, + /// never from what the lazy layout happens to have realized. + static func scrollTargetWorkspaceId( + selectedWorkspaceId: UUID, + group: WorkspaceGroup? + ) -> UUID { + guard let group, group.isCollapsed else { return selectedWorkspaceId } + return group.anchorWorkspaceId + } } diff --git a/cmuxTests/SidebarWorkspaceSnapshotRefreshPolicyTests.swift b/cmuxTests/SidebarWorkspaceSnapshotRefreshPolicyTests.swift index 3bf9715051d4..174330600b14 100644 --- a/cmuxTests/SidebarWorkspaceSnapshotRefreshPolicyTests.swift +++ b/cmuxTests/SidebarWorkspaceSnapshotRefreshPolicyTests.swift @@ -206,6 +206,51 @@ final class SidebarSelectedWorkspaceScrollPolicyTests: XCTestCase { ) ) } + + func testScrollTargetIsSelfWithoutGroup() { + let workspaceId = UUID() + XCTAssertEqual( + SidebarSelectedWorkspaceScrollPolicy.scrollTargetWorkspaceId( + selectedWorkspaceId: workspaceId, + group: nil + ), + workspaceId + ) + } + + func testScrollTargetIsSelfInExpandedGroup() { + let workspaceId = UUID() + XCTAssertEqual( + SidebarSelectedWorkspaceScrollPolicy.scrollTargetWorkspaceId( + selectedWorkspaceId: workspaceId, + group: makeGroup(isCollapsed: false, anchorWorkspaceId: UUID()) + ), + workspaceId + ) + } + + func testScrollTargetIsGroupAnchorWhenGroupIsCollapsed() { + let anchorId = UUID() + XCTAssertEqual( + SidebarSelectedWorkspaceScrollPolicy.scrollTargetWorkspaceId( + selectedWorkspaceId: UUID(), + group: makeGroup(isCollapsed: true, anchorWorkspaceId: anchorId) + ), + anchorId + ) + } + + private func makeGroup(isCollapsed: Bool, anchorWorkspaceId: UUID) -> WorkspaceGroup { + WorkspaceGroup( + id: UUID(), + name: "group", + isCollapsed: isCollapsed, + isPinned: false, + anchorWorkspaceId: anchorWorkspaceId, + customColor: nil, + iconSymbol: nil + ) + } } final class SidebarWorkspaceRowInteractionStateTests: XCTestCase { From 5bab011ac60e4b71d6d48abec1e0ac93cdd8872c Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 9 Jun 2026 11:31:18 -0700 Subject: [PATCH 4/4] Import Foundation in SidebarState for UUID Co-Authored-By: Claude Fable 5 --- Sources/Sidebar/SidebarState.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/Sidebar/SidebarState.swift b/Sources/Sidebar/SidebarState.swift index dc4f842b1b0e..e065d1e8e87e 100644 --- a/Sources/Sidebar/SidebarState.swift +++ b/Sources/Sidebar/SidebarState.swift @@ -1,5 +1,6 @@ import Combine import CoreGraphics +import Foundation final class SidebarState: ObservableObject { @Published var isVisible: Bool