From 9d602f4948928122c0cb402ef36d1b77d308b3de Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Mon, 22 Jun 2026 01:09:01 -0700 Subject: [PATCH 1/9] test: cover stale pane surface rebinding --- .../CmuxPanesTests/PaneTreeModelTests.swift | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Packages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift b/Packages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift index 43f91496f139..d525efe6c9fd 100644 --- a/Packages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift +++ b/Packages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift @@ -101,4 +101,20 @@ struct PaneTreeModelTests { #expect(model.panelId(forSurfaceId: TabID()) == nil) #expect(model.surfaceId(forPanelId: UUID()) == nil) } + + /// Rebinding one live panel to a new bonsplit surface must not leave the + /// old surface id resolving to the same panel. + @Test func rebindingPanelToNewSurfaceInvalidatesOldSurfaceMapping() { + let model = PaneTreeModel() + let oldTabId = TabID() + let newTabId = TabID() + let panelId = UUID() + + model.surfaceIdToPanelId[oldTabId] = panelId + model.surfaceIdToPanelId[newTabId] = panelId + + #expect(model.panelId(forSurfaceId: oldTabId) == nil) + #expect(model.panelId(forSurfaceId: newTabId) == panelId) + #expect(model.surfaceId(forPanelId: panelId) == newTabId) + } } From 3318f29259f9f241b4b62406542604e54c300502 Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Mon, 22 Jun 2026 01:10:24 -0700 Subject: [PATCH 2/9] fix: prevent stale pane surface rebinding --- .../CmuxPanes/Model/PaneTreeModel.swift | 65 +++++++++++++++++-- Sources/Workspace+PanelLifecycle.swift | 7 +- 2 files changed, 64 insertions(+), 8 deletions(-) diff --git a/Packages/macOS/CmuxPanes/Sources/CmuxPanes/Model/PaneTreeModel.swift b/Packages/macOS/CmuxPanes/Sources/CmuxPanes/Model/PaneTreeModel.swift index 953ea3b025d1..2d796fb5a26b 100644 --- a/Packages/macOS/CmuxPanes/Sources/CmuxPanes/Model/PaneTreeModel.swift +++ b/Packages/macOS/CmuxPanes/Sources/CmuxPanes/Model/PaneTreeModel.swift @@ -34,7 +34,14 @@ public final class PaneTreeModel { /// Mapping from bonsplit `TabID` (surface id) to the owning panel id /// (legacy `Workspace.surfaceIdToPanelId`). - public var surfaceIdToPanelId: [TabID: UUID] = [:] + /// + /// A panel can be mounted under only one bonsplit surface at a time. + /// Rebinding the same panel id to a new surface id removes any stale + /// surface entries for that panel so focus and input never resolve two + /// tabs to one live PTY. + public var surfaceIdToPanelId: [TabID: UUID] = [:] { + didSet { normalizeSurfaceMappings(preferredBy: oldValue) } + } /// Snapshot of the spatially ordered panel ids from the last geometry /// notification, used to gate `paneLayoutVersion` bumps to genuine @@ -44,6 +51,9 @@ public final class PaneTreeModel { @ObservationIgnored private weak var host: (any PaneTreeHosting)? + @ObservationIgnored + private var isNormalizingSurfaceMappings = false + /// Creates an empty model; the owning workspace attaches itself as host /// before the first mutation. public init() {} @@ -62,10 +72,57 @@ public final class PaneTreeModel { } /// Resolves the bonsplit surface id currently mapped to a panel id - /// (legacy `Workspace.surfaceIdFromPanelId`). When multiple surfaces map - /// to the same panel the match is dictionary-order arbitrary, exactly as - /// the legacy `first(where:)` lookup was. + /// (legacy `Workspace.surfaceIdFromPanelId`). public func surfaceId(forPanelId panelId: UUID) -> TabID? { surfaceIdToPanelId.first { $0.value == panelId }?.key } + + private func normalizeSurfaceMappings(preferredBy oldValue: [TabID: UUID]) { + guard !isNormalizingSurfaceMappings else { return } + let changedSurfaceIds = Set(surfaceIdToPanelId.compactMap { surfaceId, panelId in + oldValue[surfaceId] == panelId ? nil : surfaceId + }) + let normalized = Self.normalizedSurfaceMappings( + surfaceIdToPanelId, + preferredSurfaceIds: changedSurfaceIds + ) + guard normalized != surfaceIdToPanelId else { return } + + isNormalizingSurfaceMappings = true + defer { isNormalizingSurfaceMappings = false } + surfaceIdToPanelId = normalized + } + + private static func normalizedSurfaceMappings( + _ mappings: [TabID: UUID], + preferredSurfaceIds: Set + ) -> [TabID: UUID] { + let mappingsByPanelId = Dictionary(grouping: mappings) { $0.value } + var normalized = mappings + + for entries in mappingsByPanelId.values where entries.count > 1 { + guard let retainedSurfaceId = retainedSurfaceId( + from: entries, + preferredSurfaceIds: preferredSurfaceIds + ) else { + continue + } + for (surfaceId, _) in entries where surfaceId != retainedSurfaceId { + normalized.removeValue(forKey: surfaceId) + } + } + + return normalized + } + + private static func retainedSurfaceId( + from entries: [(key: TabID, value: UUID)], + preferredSurfaceIds: Set + ) -> TabID? { + let preferredEntries = entries.filter { preferredSurfaceIds.contains($0.key) } + let candidates = preferredEntries.isEmpty ? entries : preferredEntries + return candidates.max { lhs, rhs in + lhs.key.uuid.uuidString < rhs.key.uuid.uuidString + }?.key + } } diff --git a/Sources/Workspace+PanelLifecycle.swift b/Sources/Workspace+PanelLifecycle.swift index ba741d1bbf32..3742e3a3c1c7 100644 --- a/Sources/Workspace+PanelLifecycle.swift +++ b/Sources/Workspace+PanelLifecycle.swift @@ -335,10 +335,9 @@ extension Workspace { panels.removeValue(forKey: panelId) untrackRemoteTerminalSurface(panelId) pendingRemoteTerminalChildExitSurfaceIds.remove(panelId) - if let tabId { - surfaceIdToPanelId.removeValue(forKey: tabId) - } else { - surfaceIdToPanelId = surfaceIdToPanelId.filter { $0.value != panelId } + surfaceIdToPanelId = surfaceIdToPanelId.filter { surfaceId, mappedPanelId in + let isClosingSurface = tabId.map { surfaceId == $0 } ?? false + return mappedPanelId != panelId && !isClosingSurface } panelDirectories.removeValue(forKey: panelId) From 1f37a8462c63b5dad0c9a931c7db246569ca2f53 Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Mon, 22 Jun 2026 01:21:19 -0700 Subject: [PATCH 3/9] fix: use explicit pane surface binding --- .../CmuxPanes/Model/PaneTreeModel.swift | 82 ++++++------------- .../CmuxPanesTests/PaneTreeModelTests.swift | 8 +- Sources/Workspace+CustomSidebarPane.swift | 6 +- Sources/Workspace+PanelLifecycle.swift | 5 +- Sources/Workspace.swift | 71 +++++++++------- 5 files changed, 76 insertions(+), 96 deletions(-) diff --git a/Packages/macOS/CmuxPanes/Sources/CmuxPanes/Model/PaneTreeModel.swift b/Packages/macOS/CmuxPanes/Sources/CmuxPanes/Model/PaneTreeModel.swift index 2d796fb5a26b..41353cb79f56 100644 --- a/Packages/macOS/CmuxPanes/Sources/CmuxPanes/Model/PaneTreeModel.swift +++ b/Packages/macOS/CmuxPanes/Sources/CmuxPanes/Model/PaneTreeModel.swift @@ -39,9 +39,7 @@ public final class PaneTreeModel { /// Rebinding the same panel id to a new surface id removes any stale /// surface entries for that panel so focus and input never resolve two /// tabs to one live PTY. - public var surfaceIdToPanelId: [TabID: UUID] = [:] { - didSet { normalizeSurfaceMappings(preferredBy: oldValue) } - } + public private(set) var surfaceIdToPanelId: [TabID: UUID] = [:] /// Snapshot of the spatially ordered panel ids from the last geometry /// notification, used to gate `paneLayoutVersion` bumps to genuine @@ -51,9 +49,6 @@ public final class PaneTreeModel { @ObservationIgnored private weak var host: (any PaneTreeHosting)? - @ObservationIgnored - private var isNormalizingSurfaceMappings = false - /// Creates an empty model; the owning workspace attaches itself as host /// before the first mutation. public init() {} @@ -65,6 +60,32 @@ public final class PaneTreeModel { self.host = host } + /// Binds a bonsplit surface id to a panel id. + /// + /// The binding is exclusive by panel id: a live panel can be represented by + /// only one surface at a time, so rebinding the panel removes stale surface + /// entries before installing the new owner. + public func bindSurface(_ surfaceId: TabID, toPanelId panelId: UUID) { + var updatedMappings = surfaceIdToPanelId.filter { existingSurfaceId, existingPanelId in + existingSurfaceId == surfaceId || existingPanelId != panelId + } + updatedMappings[surfaceId] = panelId + surfaceIdToPanelId = updatedMappings + } + + /// Removes the mapping for one bonsplit surface id. + public func removeSurfaceMapping(forSurfaceId surfaceId: TabID) { + surfaceIdToPanelId.removeValue(forKey: surfaceId) + } + + /// Removes every mapping that can still resolve to a closed panel. + public func removeSurfaceMappings(forPanelId panelId: UUID, includingSurfaceId surfaceId: TabID? = nil) { + surfaceIdToPanelId = surfaceIdToPanelId.filter { existingSurfaceId, existingPanelId in + let isExplicitSurface = surfaceId.map { existingSurfaceId == $0 } ?? false + return existingPanelId != panelId && !isExplicitSurface + } + } + /// Resolves the owning panel id for a bonsplit surface id (legacy /// `Workspace.panelIdFromSurfaceId`). public func panelId(forSurfaceId surfaceId: TabID) -> UUID? { @@ -76,53 +97,4 @@ public final class PaneTreeModel { public func surfaceId(forPanelId panelId: UUID) -> TabID? { surfaceIdToPanelId.first { $0.value == panelId }?.key } - - private func normalizeSurfaceMappings(preferredBy oldValue: [TabID: UUID]) { - guard !isNormalizingSurfaceMappings else { return } - let changedSurfaceIds = Set(surfaceIdToPanelId.compactMap { surfaceId, panelId in - oldValue[surfaceId] == panelId ? nil : surfaceId - }) - let normalized = Self.normalizedSurfaceMappings( - surfaceIdToPanelId, - preferredSurfaceIds: changedSurfaceIds - ) - guard normalized != surfaceIdToPanelId else { return } - - isNormalizingSurfaceMappings = true - defer { isNormalizingSurfaceMappings = false } - surfaceIdToPanelId = normalized - } - - private static func normalizedSurfaceMappings( - _ mappings: [TabID: UUID], - preferredSurfaceIds: Set - ) -> [TabID: UUID] { - let mappingsByPanelId = Dictionary(grouping: mappings) { $0.value } - var normalized = mappings - - for entries in mappingsByPanelId.values where entries.count > 1 { - guard let retainedSurfaceId = retainedSurfaceId( - from: entries, - preferredSurfaceIds: preferredSurfaceIds - ) else { - continue - } - for (surfaceId, _) in entries where surfaceId != retainedSurfaceId { - normalized.removeValue(forKey: surfaceId) - } - } - - return normalized - } - - private static func retainedSurfaceId( - from entries: [(key: TabID, value: UUID)], - preferredSurfaceIds: Set - ) -> TabID? { - let preferredEntries = entries.filter { preferredSurfaceIds.contains($0.key) } - let candidates = preferredEntries.isEmpty ? entries : preferredEntries - return candidates.max { lhs, rhs in - lhs.key.uuid.uuidString < rhs.key.uuid.uuidString - }?.key - } } diff --git a/Packages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift b/Packages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift index d525efe6c9fd..6f3581b06e9d 100644 --- a/Packages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift +++ b/Packages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift @@ -81,7 +81,7 @@ struct PaneTreeModelTests { let model = PaneTreeModel() let tabId = TabID() let panelId = UUID() - model.surfaceIdToPanelId[tabId] = panelId + model.bindSurface(tabId, toPanelId: panelId) #expect(model.surfaceIdToPanelId[tabId] == panelId) model.lastOrderedPanelIds = [panelId] #expect(model.lastOrderedPanelIds == [panelId]) @@ -94,7 +94,7 @@ struct PaneTreeModelTests { let model = PaneTreeModel() let tabId = TabID() let panelId = UUID() - model.surfaceIdToPanelId[tabId] = panelId + model.bindSurface(tabId, toPanelId: panelId) #expect(model.panelId(forSurfaceId: tabId) == panelId) #expect(model.surfaceId(forPanelId: panelId) == tabId) @@ -110,8 +110,8 @@ struct PaneTreeModelTests { let newTabId = TabID() let panelId = UUID() - model.surfaceIdToPanelId[oldTabId] = panelId - model.surfaceIdToPanelId[newTabId] = panelId + model.bindSurface(oldTabId, toPanelId: panelId) + model.bindSurface(newTabId, toPanelId: panelId) #expect(model.panelId(forSurfaceId: oldTabId) == nil) #expect(model.panelId(forSurfaceId: newTabId) == panelId) diff --git a/Sources/Workspace+CustomSidebarPane.swift b/Sources/Workspace+CustomSidebarPane.swift index 794f77022e06..4de61d4ed386 100644 --- a/Sources/Workspace+CustomSidebarPane.swift +++ b/Sources/Workspace+CustomSidebarPane.swift @@ -100,7 +100,7 @@ extension Workspace { return nil } - surfaceIdToPanelId[newTabId] = customPanel.id + bindSurface(newTabId, toPanelId: customPanel.id) if let targetIndex { _ = bonsplitController.reorderTab(newTabId, toIndex: targetIndex) } @@ -149,7 +149,7 @@ extension Workspace { isLoading: false, isPinned: false ) - surfaceIdToPanelId[newTab.id] = customPanel.id + bindSurface(newTab.id, toPanelId: customPanel.id) let previousHostedView = focusedTerminalPanel?.hostedView isProgrammaticSplit = true @@ -162,7 +162,7 @@ extension Workspace { ) else { panels.removeValue(forKey: customPanel.id) panelTitles.removeValue(forKey: customPanel.id) - surfaceIdToPanelId.removeValue(forKey: newTab.id) + removeSurfaceMapping(forSurfaceId: newTab.id) return nil } diff --git a/Sources/Workspace+PanelLifecycle.swift b/Sources/Workspace+PanelLifecycle.swift index 3742e3a3c1c7..20d497eedca2 100644 --- a/Sources/Workspace+PanelLifecycle.swift +++ b/Sources/Workspace+PanelLifecycle.swift @@ -335,10 +335,7 @@ extension Workspace { panels.removeValue(forKey: panelId) untrackRemoteTerminalSurface(panelId) pendingRemoteTerminalChildExitSurfaceIds.remove(panelId) - surfaceIdToPanelId = surfaceIdToPanelId.filter { surfaceId, mappedPanelId in - let isClosingSurface = tabId.map { surfaceId == $0 } ?? false - return mappedPanelId != panelId && !isClosingSurface - } + removeSurfaceMappings(forPanelId: panelId, includingSurfaceId: tabId) panelDirectories.removeValue(forKey: panelId) panelGitBranches.removeValue(forKey: panelId) diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index fb1d8fc8aa90..31706847c693 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -3061,7 +3061,7 @@ final class Workspace: Identifiable, ObservableObject { isAudioPlaying: browserPanel.isPlayingAudio, isPinned: false ) { - surfaceIdToPanelId[tabId] = browserPanel.id + bindSurface(tabId, toPanelId: browserPanel.id) initialTabId = tabId } installBrowserPanelSubscription(browserPanel) @@ -3093,7 +3093,7 @@ final class Workspace: Identifiable, ObservableObject { isDirty: false, isPinned: false ) { - surfaceIdToPanelId[tabId] = terminalPanel.id + bindSurface(tabId, toPanelId: terminalPanel.id) initialTabId = tabId } } @@ -3280,8 +3280,19 @@ final class Workspace: Identifiable, ObservableObject { /// Mapping from bonsplit TabID (surface id) to the owning panel id; /// stored in the pane-tree sub-model. var surfaceIdToPanelId: [TabID: UUID] { - get { paneTree.surfaceIdToPanelId } - set { paneTree.surfaceIdToPanelId = newValue } + paneTree.surfaceIdToPanelId + } + + func bindSurface(_ surfaceId: TabID, toPanelId panelId: UUID) { + paneTree.bindSurface(surfaceId, toPanelId: panelId) + } + + func removeSurfaceMapping(forSurfaceId surfaceId: TabID) { + paneTree.removeSurfaceMapping(forSurfaceId: surfaceId) + } + + func removeSurfaceMappings(forPanelId panelId: UUID, includingSurfaceId surfaceId: TabID? = nil) { + paneTree.removeSurfaceMappings(forPanelId: panelId, includingSurfaceId: surfaceId) } /// Tab IDs that are allowed to close even if they would normally require confirmation. @@ -7004,7 +7015,7 @@ final class Workspace: Identifiable, ObservableObject { isDirty: newPanel.isDirty, isPinned: false ) - surfaceIdToPanelId[newTab.id] = newPanel.id + bindSurface(newTab.id, toPanelId: newPanel.id) let previousFocusedPanelId = focusedPanelId // Capture the source terminal's hosted view before bonsplit mutates focusedPaneId, @@ -7019,7 +7030,7 @@ final class Workspace: Identifiable, ObservableObject { panelTitles.removeValue(forKey: newPanel.id) remotePTYSessionIDsByPanelId.removeValue(forKey: newPanel.id) removeRemoteRelaySurfaceAliases(targeting: newPanel.id) - surfaceIdToPanelId.removeValue(forKey: newTab.id) + removeSurfaceMapping(forSurfaceId: newTab.id) if tracksRemoteTerminalSurface { untrackRemoteTerminalSurface(newPanel.id) } @@ -7259,7 +7270,7 @@ final class Workspace: Identifiable, ObservableObject { return nil } - surfaceIdToPanelId[newTabId] = newPanel.id + bindSurface(newTabId, toPanelId: newPanel.id) publishCmuxSurfaceCreated(newPanel.id, paneId: paneId, kind: "terminal", origin: "terminal_tab", focused: shouldFocusNewTab) // bonsplit's createTab may not reliably emit didSelectTab, and its internal selection @@ -7355,7 +7366,7 @@ final class Workspace: Identifiable, ObservableObject { panelTitles.removeValue(forKey: newPanel.id) return nil } - surfaceIdToPanelId[newTabId] = newPanel.id + bindSurface(newTabId, toPanelId: newPanel.id) if focus { bonsplitController.focusPane(paneId) } @@ -7523,7 +7534,7 @@ final class Workspace: Identifiable, ObservableObject { if wasPinned { pinnedPanelIds.insert(panelId) } - surfaceIdToPanelId[tabId] = panelId + bindSurface(tabId, toPanelId: panelId) seedTerminalInheritanceFontPoints(panelId: panelId, configTemplate: inheritedConfig) let resolvedTitle = resolvedPanelTitle(panelId: panelId, fallback: replacementPanel.displayTitle) @@ -7644,7 +7655,7 @@ final class Workspace: Identifiable, ObservableObject { isAudioPlaying: browserPanel.isPlayingAudio, isPinned: false ) - surfaceIdToPanelId[newTab.id] = browserPanel.id + bindSurface(newTab.id, toPanelId: browserPanel.id) let previousFocusedPanelId = focusedPanelId // Create the split with the browser tab already present. @@ -7652,7 +7663,7 @@ final class Workspace: Identifiable, ObservableObject { isProgrammaticSplit = true defer { isProgrammaticSplit = false } guard let newPaneId = bonsplitController.splitPane(paneId, orientation: orientation, withTab: newTab, insertFirst: insertFirst) else { - surfaceIdToPanelId.removeValue(forKey: newTab.id) + removeSurfaceMapping(forSurfaceId: newTab.id) panels.removeValue(forKey: browserPanel.id) panelTitles.removeValue(forKey: browserPanel.id) return nil @@ -7758,7 +7769,7 @@ final class Workspace: Identifiable, ObservableObject { return nil } - surfaceIdToPanelId[newTabId] = browserPanel.id + bindSurface(newTabId, toPanelId: browserPanel.id) setPreferredBrowserProfileID(browserPanel.profileID) // Keyboard/browser-open paths want "new tab at end" regardless of global new-tab placement. @@ -7823,7 +7834,7 @@ final class Workspace: Identifiable, ObservableObject { return nil } - surfaceIdToPanelId[newTabId] = extensionBrowserPanel.id + bindSurface(newTabId, toPanelId: extensionBrowserPanel.id) publishCmuxSurfaceCreated( extensionBrowserPanel.id, paneId: paneId, @@ -7907,13 +7918,13 @@ final class Workspace: Identifiable, ObservableObject { isLoading: false, isPinned: false ) - surfaceIdToPanelId[newTab.id] = markdownPanel.id + bindSurface(newTab.id, toPanelId: markdownPanel.id) let previousFocusedPanelId = focusedPanelId isProgrammaticSplit = true defer { isProgrammaticSplit = false } guard let newPaneId = bonsplitController.splitPane(paneId, orientation: orientation, withTab: newTab, insertFirst: insertFirst) else { - surfaceIdToPanelId.removeValue(forKey: newTab.id) + removeSurfaceMapping(forSurfaceId: newTab.id) panels.removeValue(forKey: markdownPanel.id) panelTitles.removeValue(forKey: markdownPanel.id) return nil @@ -7968,7 +7979,7 @@ final class Workspace: Identifiable, ObservableObject { return nil } - surfaceIdToPanelId[newTabId] = markdownPanel.id + bindSurface(newTabId, toPanelId: markdownPanel.id) if let targetIndex { _ = bonsplitController.reorderTab(newTabId, toIndex: targetIndex) } @@ -8020,7 +8031,7 @@ final class Workspace: Identifiable, ObservableObject { return nil } - surfaceIdToPanelId[newTabId] = projectPanel.id + bindSurface(newTabId, toPanelId: projectPanel.id) if let targetIndex { _ = bonsplitController.reorderTab(newTabId, toIndex: targetIndex) } @@ -8080,7 +8091,7 @@ final class Workspace: Identifiable, ObservableObject { isLoading: false, isPinned: false ) - surfaceIdToPanelId[newTab.id] = markdownPanel.id + bindSurface(newTab.id, toPanelId: markdownPanel.id) isProgrammaticSplit = true defer { isProgrammaticSplit = false } @@ -8092,7 +8103,7 @@ final class Workspace: Identifiable, ObservableObject { ) != nil else { panels.removeValue(forKey: markdownPanel.id) panelTitles.removeValue(forKey: markdownPanel.id) - surfaceIdToPanelId.removeValue(forKey: newTab.id) + removeSurfaceMapping(forSurfaceId: newTab.id) return nil } @@ -8178,7 +8189,7 @@ final class Workspace: Identifiable, ObservableObject { return nil } - surfaceIdToPanelId[newTabId] = filePreviewPanel.id + bindSurface(newTabId, toPanelId: filePreviewPanel.id) if let targetIndex { _ = bonsplitController.reorderTab(newTabId, toIndex: targetIndex) } @@ -8250,7 +8261,7 @@ final class Workspace: Identifiable, ObservableObject { return nil } - surfaceIdToPanelId[newTabId] = toolPanel.id + bindSurface(newTabId, toPanelId: toolPanel.id) if let targetIndex { _ = bonsplitController.reorderTab(newTabId, toIndex: targetIndex) } @@ -8309,7 +8320,7 @@ final class Workspace: Identifiable, ObservableObject { return nil } - surfaceIdToPanelId[newTabId] = agentPanel.id + bindSurface(newTabId, toPanelId: agentPanel.id) if let targetIndex { _ = bonsplitController.reorderTab(newTabId, toIndex: targetIndex) } @@ -8358,14 +8369,14 @@ final class Workspace: Identifiable, ObservableObject { isLoading: false, isPinned: false ) - surfaceIdToPanelId[newTab.id] = filePreviewPanel.id + bindSurface(newTab.id, toPanelId: filePreviewPanel.id) isProgrammaticSplit = true defer { isProgrammaticSplit = false } guard let newPaneId = bonsplitController.splitPane(paneId, orientation: orientation, withTab: newTab, insertFirst: insertFirst) else { panels.removeValue(forKey: filePreviewPanel.id) panelTitles.removeValue(forKey: filePreviewPanel.id) - surfaceIdToPanelId.removeValue(forKey: newTab.id) + removeSurfaceMapping(forSurfaceId: newTab.id) return nil } publishCmuxSplitCreated(newPaneId, sourcePaneId: paneId, orientation: orientation, surfaceId: filePreviewPanel.id, kind: "file_preview", origin: "file_preview_split", focused: true) @@ -9055,7 +9066,7 @@ final class Workspace: Identifiable, ObservableObject { return nil } - surfaceIdToPanelId[newTabId] = detached.panelId + bindSurface(newTabId, toPanelId: detached.panelId) panels[detached.panelId] = detached.panel if let terminalPanel = detached.panel as? TerminalPanel { terminalPanel.updateWorkspaceId(id) @@ -9792,7 +9803,7 @@ final class Workspace: Identifiable, ObservableObject { isDirty: newPanel.isDirty, isPinned: false ) { - surfaceIdToPanelId[newTabId] = newPanel.id + bindSurface(newTabId, toPanelId: newPanel.id) } return newPanel @@ -10872,14 +10883,14 @@ final class Workspace: Identifiable, ObservableObject { isDirty: newPanel.isDirty, isPinned: false ) - surfaceIdToPanelId[newTab.id] = newPanel.id + bindSurface(newTab.id, toPanelId: newPanel.id) isProgrammaticSplit = true defer { isProgrammaticSplit = false } guard let newPaneId = bonsplitController.splitPane(paneId, orientation: orientation, withTab: newTab, insertFirst: insertFirst) else { panels.removeValue(forKey: newPanel.id) panelTitles.removeValue(forKey: newPanel.id) - surfaceIdToPanelId.removeValue(forKey: newTab.id) + removeSurfaceMapping(forSurfaceId: newTab.id) if startupCommand != nil { untrackRemoteTerminalSurface(newPanel.id) } @@ -12331,7 +12342,7 @@ extension Workspace: BonsplitDelegate { panels[replacementPanel.id] = replacementPanel panelTitles[replacementPanel.id] = replacementPanel.displayTitle seedTerminalInheritanceFontPoints(panelId: replacementPanel.id, configTemplate: inheritedConfig) - surfaceIdToPanelId[replacementTab.id] = replacementPanel.id + bindSurface(replacementTab.id, toPanelId: replacementPanel.id) bonsplitController.updateTab( replacementTab.id, @@ -12415,7 +12426,7 @@ extension Workspace: BonsplitDelegate { return } - surfaceIdToPanelId[newTabId] = newPanel.id + bindSurface(newTabId, toPanelId: newPanel.id) normalizePinnedTabs(in: newPane) publishCmuxSplitCreated(newPane, sourcePaneId: originalPane, orientation: orientation, surfaceId: newPanel.id, kind: "terminal", origin: "ui_split", focused: true) #if DEBUG From 733b8d63168c343cb20273681230380fa606154d Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Mon, 22 Jun 2026 01:23:25 -0700 Subject: [PATCH 4/9] fix: keep rebound surface mappings during cleanup --- .../CmuxPanes/Model/PaneTreeModel.swift | 9 ++++++--- .../CmuxPanesTests/PaneTreeModelTests.swift | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/Packages/macOS/CmuxPanes/Sources/CmuxPanes/Model/PaneTreeModel.swift b/Packages/macOS/CmuxPanes/Sources/CmuxPanes/Model/PaneTreeModel.swift index 41353cb79f56..8a3553f4e11b 100644 --- a/Packages/macOS/CmuxPanes/Sources/CmuxPanes/Model/PaneTreeModel.swift +++ b/Packages/macOS/CmuxPanes/Sources/CmuxPanes/Model/PaneTreeModel.swift @@ -79,10 +79,13 @@ public final class PaneTreeModel { } /// Removes every mapping that can still resolve to a closed panel. + /// + /// When `surfaceId` is present, it is expected to still belong to + /// `panelId`; cleanup intentionally removes by panel id so a stale surface + /// id cannot delete a live binding that has already moved to another panel. public func removeSurfaceMappings(forPanelId panelId: UUID, includingSurfaceId surfaceId: TabID? = nil) { - surfaceIdToPanelId = surfaceIdToPanelId.filter { existingSurfaceId, existingPanelId in - let isExplicitSurface = surfaceId.map { existingSurfaceId == $0 } ?? false - return existingPanelId != panelId && !isExplicitSurface + surfaceIdToPanelId = surfaceIdToPanelId.filter { _, existingPanelId in + existingPanelId != panelId } } diff --git a/Packages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift b/Packages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift index 6f3581b06e9d..45bb08e42190 100644 --- a/Packages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift +++ b/Packages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift @@ -117,4 +117,23 @@ struct PaneTreeModelTests { #expect(model.panelId(forSurfaceId: newTabId) == panelId) #expect(model.surfaceId(forPanelId: panelId) == newTabId) } + + /// Close cleanup removes stale aliases for the closed panel without using + /// a stale tab id to drop a surface that has already moved to another panel. + @Test func closedPanelCleanupKeepsReboundSurfaceMapping() { + let model = PaneTreeModel() + let closedPanelTabId = TabID() + let reboundTabId = TabID() + let closedPanelId = UUID() + let livePanelId = UUID() + + model.bindSurface(closedPanelTabId, toPanelId: closedPanelId) + model.bindSurface(reboundTabId, toPanelId: livePanelId) + + model.removeSurfaceMappings(forPanelId: closedPanelId, includingSurfaceId: reboundTabId) + + #expect(model.panelId(forSurfaceId: closedPanelTabId) == nil) + #expect(model.panelId(forSurfaceId: reboundTabId) == livePanelId) + #expect(model.surfaceId(forPanelId: livePanelId) == reboundTabId) + } } From c4f0a3c5a0ae08227a450da4555472e2d24e5cd1 Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Mon, 22 Jun 2026 01:27:43 -0700 Subject: [PATCH 5/9] chore: refresh Swift file length budget --- .github/swift-file-length-budget.tsv | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/swift-file-length-budget.tsv b/.github/swift-file-length-budget.tsv index c44228842e47..abe5179a4b91 100644 --- a/.github/swift-file-length-budget.tsv +++ b/.github/swift-file-length-budget.tsv @@ -5,7 +5,7 @@ 17717 Sources/AppDelegate.swift 16112 Sources/ContentView.swift 13950 Sources/TerminalController.swift -12758 Sources/Workspace.swift +12769 Sources/Workspace.swift 12144 cmuxTests/AppDelegateShortcutRoutingTests.swift 11924 Sources/Panels/BrowserPanel.swift 11867 Sources/GhosttyTerminalView.swift @@ -20,7 +20,7 @@ 6084 Sources/TextBoxInput.swift 5915 cmuxTests/TerminalAndGhosttyTests.swift 5573 cmuxTests/BrowserConfigTests.swift -5566 Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift +5507 Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift 4487 Sources/Panels/FilePreviewPanel.swift 4478 Sources/cmuxApp.swift 4401 cmuxTests/BrowserPanelTests.swift @@ -123,9 +123,9 @@ 754 Sources/TerminalController+ControlWorkspaceContext.swift 752 cmuxUITests/CloseWorkspaceCmdDUITests.swift 738 Packages/macOS/CMUXProjectModel/Sources/CMUXProjectModel/XcodeProjectAdapter.swift -736 Packages/Shared/CmuxAuthRuntime/Sources/CmuxAuthRuntime/Coordinator/AuthCoordinator.swift 726 cmuxTests/CLICodexHookTimeoutRegressionTests.swift 722 Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationStore.swift +718 Packages/Shared/CmuxAuthRuntime/Sources/CmuxAuthRuntime/Coordinator/AuthCoordinator.swift 716 Sources/TaskManagerSnapshot.swift 715 Sources/AppleScriptSupport.swift 710 Sources/TerminalSSHSessionDetector.swift From 4da5ecaf4001b927f7e0a0963bbdfafe909d7c23 Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Mon, 22 Jun 2026 01:29:08 -0700 Subject: [PATCH 6/9] refactor: remove stale surface cleanup parameter --- .../CmuxPanes/Sources/CmuxPanes/Model/PaneTreeModel.swift | 6 +----- .../CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift | 2 +- Sources/Workspace+PanelLifecycle.swift | 2 +- Sources/Workspace.swift | 4 ++-- 4 files changed, 5 insertions(+), 9 deletions(-) diff --git a/Packages/macOS/CmuxPanes/Sources/CmuxPanes/Model/PaneTreeModel.swift b/Packages/macOS/CmuxPanes/Sources/CmuxPanes/Model/PaneTreeModel.swift index 8a3553f4e11b..3cfe03dca7ab 100644 --- a/Packages/macOS/CmuxPanes/Sources/CmuxPanes/Model/PaneTreeModel.swift +++ b/Packages/macOS/CmuxPanes/Sources/CmuxPanes/Model/PaneTreeModel.swift @@ -79,11 +79,7 @@ public final class PaneTreeModel { } /// Removes every mapping that can still resolve to a closed panel. - /// - /// When `surfaceId` is present, it is expected to still belong to - /// `panelId`; cleanup intentionally removes by panel id so a stale surface - /// id cannot delete a live binding that has already moved to another panel. - public func removeSurfaceMappings(forPanelId panelId: UUID, includingSurfaceId surfaceId: TabID? = nil) { + public func removeSurfaceMappings(forPanelId panelId: UUID) { surfaceIdToPanelId = surfaceIdToPanelId.filter { _, existingPanelId in existingPanelId != panelId } diff --git a/Packages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift b/Packages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift index 45bb08e42190..64db649e6c13 100644 --- a/Packages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift +++ b/Packages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift @@ -130,7 +130,7 @@ struct PaneTreeModelTests { model.bindSurface(closedPanelTabId, toPanelId: closedPanelId) model.bindSurface(reboundTabId, toPanelId: livePanelId) - model.removeSurfaceMappings(forPanelId: closedPanelId, includingSurfaceId: reboundTabId) + model.removeSurfaceMappings(forPanelId: closedPanelId) #expect(model.panelId(forSurfaceId: closedPanelTabId) == nil) #expect(model.panelId(forSurfaceId: reboundTabId) == livePanelId) diff --git a/Sources/Workspace+PanelLifecycle.swift b/Sources/Workspace+PanelLifecycle.swift index 20d497eedca2..874816dfd44c 100644 --- a/Sources/Workspace+PanelLifecycle.swift +++ b/Sources/Workspace+PanelLifecycle.swift @@ -335,7 +335,7 @@ extension Workspace { panels.removeValue(forKey: panelId) untrackRemoteTerminalSurface(panelId) pendingRemoteTerminalChildExitSurfaceIds.remove(panelId) - removeSurfaceMappings(forPanelId: panelId, includingSurfaceId: tabId) + removeSurfaceMappings(forPanelId: panelId) panelDirectories.removeValue(forKey: panelId) panelGitBranches.removeValue(forKey: panelId) diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index 31706847c693..455136260870 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -3291,8 +3291,8 @@ final class Workspace: Identifiable, ObservableObject { paneTree.removeSurfaceMapping(forSurfaceId: surfaceId) } - func removeSurfaceMappings(forPanelId panelId: UUID, includingSurfaceId surfaceId: TabID? = nil) { - paneTree.removeSurfaceMappings(forPanelId: panelId, includingSurfaceId: surfaceId) + func removeSurfaceMappings(forPanelId panelId: UUID) { + paneTree.removeSurfaceMappings(forPanelId: panelId) } /// Tab IDs that are allowed to close even if they would normally require confirmation. From 82307ccfb3dea989776b8a83b051facaa8414fd4 Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Mon, 22 Jun 2026 01:31:52 -0700 Subject: [PATCH 7/9] docs: document surface binding helpers --- .github/swift-file-length-budget.tsv | 2 +- Sources/Workspace.swift | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/swift-file-length-budget.tsv b/.github/swift-file-length-budget.tsv index abe5179a4b91..3b73792f07dd 100644 --- a/.github/swift-file-length-budget.tsv +++ b/.github/swift-file-length-budget.tsv @@ -5,7 +5,7 @@ 17717 Sources/AppDelegate.swift 16112 Sources/ContentView.swift 13950 Sources/TerminalController.swift -12769 Sources/Workspace.swift +12772 Sources/Workspace.swift 12144 cmuxTests/AppDelegateShortcutRoutingTests.swift 11924 Sources/Panels/BrowserPanel.swift 11867 Sources/GhosttyTerminalView.swift diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index 455136260870..e0d6b686a3cb 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -3283,14 +3283,17 @@ final class Workspace: Identifiable, ObservableObject { paneTree.surfaceIdToPanelId } + /// Registers a bonsplit surface as the active owner for a panel. func bindSurface(_ surfaceId: TabID, toPanelId panelId: UUID) { paneTree.bindSurface(surfaceId, toPanelId: panelId) } + /// Removes one bonsplit surface mapping. func removeSurfaceMapping(forSurfaceId surfaceId: TabID) { paneTree.removeSurfaceMapping(forSurfaceId: surfaceId) } + /// Removes every bonsplit surface mapping for a closed panel. func removeSurfaceMappings(forPanelId panelId: UUID) { paneTree.removeSurfaceMappings(forPanelId: panelId) } From 7be4b29f252526553e42fd89b565b68d7078bf2e Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Mon, 22 Jun 2026 01:34:57 -0700 Subject: [PATCH 8/9] fix: keep surface binding updates targeted --- .../CmuxPanes/Model/PaneTreeModel.swift | 30 ++++++++++++++----- .../CmuxPanesTests/PaneTreeModelTests.swift | 16 ++++++++++ 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/Packages/macOS/CmuxPanes/Sources/CmuxPanes/Model/PaneTreeModel.swift b/Packages/macOS/CmuxPanes/Sources/CmuxPanes/Model/PaneTreeModel.swift index 3cfe03dca7ab..3075179a2215 100644 --- a/Packages/macOS/CmuxPanes/Sources/CmuxPanes/Model/PaneTreeModel.swift +++ b/Packages/macOS/CmuxPanes/Sources/CmuxPanes/Model/PaneTreeModel.swift @@ -46,6 +46,10 @@ public final class PaneTreeModel { /// reorder events (legacy `Workspace.lastOrderedPanelIds`). public var lastOrderedPanelIds: [UUID] = [] + /// Reverse index for targeted panel lookups and stale-surface removal. + @ObservationIgnored + private var panelIdToSurfaceId: [UUID: TabID] = [:] + @ObservationIgnored private weak var host: (any PaneTreeHosting)? @@ -66,22 +70,32 @@ public final class PaneTreeModel { /// only one surface at a time, so rebinding the panel removes stale surface /// entries before installing the new owner. public func bindSurface(_ surfaceId: TabID, toPanelId panelId: UUID) { - var updatedMappings = surfaceIdToPanelId.filter { existingSurfaceId, existingPanelId in - existingSurfaceId == surfaceId || existingPanelId != panelId + if let previousSurfaceId = panelIdToSurfaceId[panelId], + previousSurfaceId != surfaceId { + surfaceIdToPanelId.removeValue(forKey: previousSurfaceId) + } + if let previousPanelId = surfaceIdToPanelId[surfaceId], + previousPanelId != panelId, + panelIdToSurfaceId[previousPanelId] == surfaceId { + panelIdToSurfaceId.removeValue(forKey: previousPanelId) } - updatedMappings[surfaceId] = panelId - surfaceIdToPanelId = updatedMappings + + surfaceIdToPanelId[surfaceId] = panelId + panelIdToSurfaceId[panelId] = surfaceId } /// Removes the mapping for one bonsplit surface id. public func removeSurfaceMapping(forSurfaceId surfaceId: TabID) { - surfaceIdToPanelId.removeValue(forKey: surfaceId) + if let panelId = surfaceIdToPanelId.removeValue(forKey: surfaceId), + panelIdToSurfaceId[panelId] == surfaceId { + panelIdToSurfaceId.removeValue(forKey: panelId) + } } /// Removes every mapping that can still resolve to a closed panel. public func removeSurfaceMappings(forPanelId panelId: UUID) { - surfaceIdToPanelId = surfaceIdToPanelId.filter { _, existingPanelId in - existingPanelId != panelId + if let surfaceId = panelIdToSurfaceId.removeValue(forKey: panelId) { + surfaceIdToPanelId.removeValue(forKey: surfaceId) } } @@ -94,6 +108,6 @@ public final class PaneTreeModel { /// Resolves the bonsplit surface id currently mapped to a panel id /// (legacy `Workspace.surfaceIdFromPanelId`). public func surfaceId(forPanelId panelId: UUID) -> TabID? { - surfaceIdToPanelId.first { $0.value == panelId }?.key + panelIdToSurfaceId[panelId] } } diff --git a/Packages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift b/Packages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift index 64db649e6c13..e91f49a6c7b9 100644 --- a/Packages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift +++ b/Packages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift @@ -118,6 +118,22 @@ struct PaneTreeModelTests { #expect(model.surfaceId(forPanelId: panelId) == newTabId) } + /// Reusing one bonsplit surface for a different panel must also clear the + /// old panel's reverse lookup. + @Test func rebindingSurfaceToNewPanelInvalidatesOldPanelMapping() { + let model = PaneTreeModel() + let tabId = TabID() + let oldPanelId = UUID() + let newPanelId = UUID() + + model.bindSurface(tabId, toPanelId: oldPanelId) + model.bindSurface(tabId, toPanelId: newPanelId) + + #expect(model.panelId(forSurfaceId: tabId) == newPanelId) + #expect(model.surfaceId(forPanelId: oldPanelId) == nil) + #expect(model.surfaceId(forPanelId: newPanelId) == tabId) + } + /// Close cleanup removes stale aliases for the closed panel without using /// a stale tab id to drop a surface that has already moved to another panel. @Test func closedPanelCleanupKeepsReboundSurfaceMapping() { From cc64cf7a0098c6ab8d548138cce1f6c25f0449d6 Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Mon, 22 Jun 2026 01:39:51 -0700 Subject: [PATCH 9/9] test: cover rebound surface cleanup path --- .../CmuxPanesTests/PaneTreeModelTests.swift | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/Packages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift b/Packages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift index e91f49a6c7b9..3c3e02e68b0f 100644 --- a/Packages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift +++ b/Packages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swift @@ -134,22 +134,34 @@ struct PaneTreeModelTests { #expect(model.surfaceId(forPanelId: newPanelId) == tabId) } + /// Close cleanup removes the surface owned by the closed panel. + @Test func closedPanelCleanupRemovesClosedSurfaceMapping() { + let model = PaneTreeModel() + let closedPanelTabId = TabID() + let closedPanelId = UUID() + + model.bindSurface(closedPanelTabId, toPanelId: closedPanelId) + model.removeSurfaceMappings(forPanelId: closedPanelId) + + #expect(model.panelId(forSurfaceId: closedPanelTabId) == nil) + #expect(model.surfaceId(forPanelId: closedPanelId) == nil) + } + /// Close cleanup removes stale aliases for the closed panel without using /// a stale tab id to drop a surface that has already moved to another panel. @Test func closedPanelCleanupKeepsReboundSurfaceMapping() { let model = PaneTreeModel() - let closedPanelTabId = TabID() let reboundTabId = TabID() let closedPanelId = UUID() let livePanelId = UUID() - model.bindSurface(closedPanelTabId, toPanelId: closedPanelId) + model.bindSurface(reboundTabId, toPanelId: closedPanelId) model.bindSurface(reboundTabId, toPanelId: livePanelId) model.removeSurfaceMappings(forPanelId: closedPanelId) - #expect(model.panelId(forSurfaceId: closedPanelTabId) == nil) #expect(model.panelId(forSurfaceId: reboundTabId) == livePanelId) + #expect(model.surfaceId(forPanelId: closedPanelId) == nil) #expect(model.surfaceId(forPanelId: livePanelId) == reboundTabId) } }