Fix crash when creating a new workspace via snapshot-based state

Snapshot TabManager's published state (tabs, selectedTabId) at the start
of workspace creation so mutations don't bounce through Combine-backed
accessors mid-creation. Prevents crashes when adding a workspace via
Cmd+N, the new-workspace button, or any other creation path that
triggers re-entrant publishes.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
austinpower1258 2026-03-13 11:15:03 -07:00
parent e52959a528
commit c1998e347c
2 changed files with 141 additions and 20 deletions

View file

@ -754,6 +754,15 @@ class TabManager: ObservableObject {
private var pendingWorkspaceUnfocusTarget: (tabId: UUID, panelId: UUID)?
private var sidebarSelectedWorkspaceIds: Set<UUID> = []
var confirmCloseHandler: ((String, String, Bool) -> Bool)?
private struct WorkspaceCreationSnapshot {
let tabs: [Workspace]
let selectedTabId: UUID?
var selectedWorkspace: Workspace? {
guard let selectedTabId else { return nil }
return tabs.first(where: { $0.id == selectedTabId })
}
}
#if DEBUG
private var debugWorkspaceSwitchCounter: UInt64 = 0
private var debugWorkspaceSwitchId: UInt64 = 0
@ -927,26 +936,32 @@ class TabManager: ObservableObject {
placementOverride: NewWorkspacePlacement? = nil,
autoWelcomeIfNeeded: Bool = true
) -> Workspace {
sentryBreadcrumb("workspace.create", data: ["tabCount": tabs.count + 1])
// Snapshot current published state once so workspace creation doesn't repeatedly
// bounce through Combine-backed accessors while we're preparing the new workspace.
let snapshot = workspaceCreationSnapshot()
let nextTabCount = snapshot.tabs.count + 1
sentryBreadcrumb("workspace.create", data: ["tabCount": nextTabCount])
let explicitWorkingDirectory = normalizedWorkingDirectory(overrideWorkingDirectory)
let workingDirectory = explicitWorkingDirectory ?? preferredWorkingDirectoryForNewTab()
let inheritedConfig = inheritedTerminalConfigForNewWorkspace()
let workingDirectory = explicitWorkingDirectory ?? preferredWorkingDirectoryForNewTab(snapshot: snapshot)
let inheritedConfig = inheritedTerminalConfigForNewWorkspace(snapshot: snapshot)
let ordinal = Self.nextPortOrdinal
Self.nextPortOrdinal += 1
let newWorkspace = Workspace(
title: "Terminal \(tabs.count + 1)",
title: "Terminal \(nextTabCount)",
workingDirectory: workingDirectory,
portOrdinal: ordinal,
configTemplate: inheritedConfig
)
newWorkspace.owningTabManager = self
wireClosedBrowserTracking(for: newWorkspace)
let insertIndex = newTabInsertIndex(placementOverride: placementOverride)
if insertIndex >= 0 && insertIndex <= tabs.count {
tabs.insert(newWorkspace, at: insertIndex)
let insertIndex = newTabInsertIndex(snapshot: snapshot, placementOverride: placementOverride)
var updatedTabs = snapshot.tabs
if insertIndex >= 0 && insertIndex <= updatedTabs.count {
updatedTabs.insert(newWorkspace, at: insertIndex)
} else {
tabs.append(newWorkspace)
updatedTabs.append(newWorkspace)
}
tabs = updatedTabs
if let explicitWorkingDirectory,
let terminalPanel = newWorkspace.focusedTerminalPanel {
scheduleInitialWorkspaceGitMetadataRefresh(
@ -973,8 +988,8 @@ class TabManager: ObservableObject {
#if DEBUG
UITestRecorder.incrementInt("addTabInvocations")
UITestRecorder.record([
"tabCount": String(tabs.count),
"selectedTabId": select ? newWorkspace.id.uuidString : (selectedTabId?.uuidString ?? "")
"tabCount": String(updatedTabs.count),
"selectedTabId": select ? newWorkspace.id.uuidString : (snapshot.selectedTabId?.uuidString ?? "")
])
#endif
if autoWelcomeIfNeeded && select && !UserDefaults.standard.bool(forKey: WelcomeSettings.shownKey) {
@ -1202,7 +1217,20 @@ class TabManager: ObservableObject {
}
func terminalPanelForWorkspaceConfigInheritanceSource() -> TerminalPanel? {
guard let workspace = selectedWorkspace else { return nil }
terminalPanelForWorkspaceConfigInheritanceSource(snapshot: workspaceCreationSnapshot())
}
private func workspaceCreationSnapshot() -> WorkspaceCreationSnapshot {
WorkspaceCreationSnapshot(
tabs: tabs,
selectedTabId: selectedTabId
)
}
private func terminalPanelForWorkspaceConfigInheritanceSource(
snapshot: WorkspaceCreationSnapshot
) -> TerminalPanel? {
guard let workspace = snapshot.selectedWorkspace else { return nil }
if let focusedTerminal = workspace.focusedTerminalPanel {
return focusedTerminal
}
@ -1217,13 +1245,19 @@ class TabManager: ObservableObject {
}
private func inheritedTerminalConfigForNewWorkspace() -> ghostty_surface_config_s? {
if let sourceSurface = terminalPanelForWorkspaceConfigInheritanceSource()?.surface.surface {
inheritedTerminalConfigForNewWorkspace(snapshot: workspaceCreationSnapshot())
}
private func inheritedTerminalConfigForNewWorkspace(
snapshot: WorkspaceCreationSnapshot
) -> ghostty_surface_config_s? {
if let sourceSurface = terminalPanelForWorkspaceConfigInheritanceSource(snapshot: snapshot)?.surface.surface {
return cmuxInheritedSurfaceConfig(
sourceSurface: sourceSurface,
context: GHOSTTY_SURFACE_CONTEXT_TAB
)
}
if let fallbackFontPoints = selectedWorkspace?.lastRememberedTerminalFontPointsForConfigInheritance() {
if let fallbackFontPoints = snapshot.selectedWorkspace?.lastRememberedTerminalFontPointsForConfigInheritance() {
var config = ghostty_surface_config_new()
config.font_size = fallbackFontPoints
return config
@ -1239,24 +1273,36 @@ class TabManager: ObservableObject {
}
private func newTabInsertIndex(placementOverride: NewWorkspacePlacement? = nil) -> Int {
newTabInsertIndex(snapshot: workspaceCreationSnapshot(), placementOverride: placementOverride)
}
private func newTabInsertIndex(
snapshot: WorkspaceCreationSnapshot,
placementOverride: NewWorkspacePlacement? = nil
) -> Int {
let placement = placementOverride ?? WorkspacePlacementSettings.current()
let pinnedCount = tabs.filter { $0.isPinned }.count
let selectedIndex = selectedTabId.flatMap { tabId in
tabs.firstIndex(where: { $0.id == tabId })
let pinnedCount = snapshot.tabs.filter { $0.isPinned }.count
let selectedIndex = snapshot.selectedTabId.flatMap { tabId in
snapshot.tabs.firstIndex(where: { $0.id == tabId })
}
let selectedIsPinned = selectedIndex.map { tabs[$0].isPinned } ?? false
let selectedIsPinned = selectedIndex.map { snapshot.tabs[$0].isPinned } ?? false
return WorkspacePlacementSettings.insertionIndex(
placement: placement,
selectedIndex: selectedIndex,
selectedIsPinned: selectedIsPinned,
pinnedCount: pinnedCount,
totalCount: tabs.count
totalCount: snapshot.tabs.count
)
}
private func preferredWorkingDirectoryForNewTab() -> String? {
guard let selectedTabId,
let tab = tabs.first(where: { $0.id == selectedTabId }) else {
preferredWorkingDirectoryForNewTab(snapshot: workspaceCreationSnapshot())
}
private func preferredWorkingDirectoryForNewTab(
snapshot: WorkspaceCreationSnapshot
) -> String? {
guard let tab = snapshot.selectedWorkspace else {
return nil
}
let focusedDirectory = tab.focusedPanelId

View file

@ -277,6 +277,71 @@ final class AppDelegateShortcutRoutingTests: XCTestCase {
XCTAssertNil(appDelegate.tabManagerFor(windowId: orphanWindowId), "Orphaned context should be pruned after failed resolution")
}
func testCustomCmdTNewWorkspacePrunesOrphanedContextWithoutLiveWindow() {
guard let appDelegate = AppDelegate.shared else {
XCTFail("Expected AppDelegate.shared")
return
}
let existingWindowIds = mainWindowIds()
let orphanWindowId = UUID()
let orphanManager = TabManager()
let orphanSidebarState = SidebarState()
let orphanSidebarSelectionState = SidebarSelectionState()
autoreleasepool {
var orphanWindow: NSWindow? = NSWindow(
contentRect: NSRect(x: 0, y: 0, width: 320, height: 240),
styleMask: [.titled, .closable, .resizable],
backing: .buffered,
defer: false
)
orphanWindow?.identifier = NSUserInterfaceItemIdentifier("cmux.main.\(orphanWindowId.uuidString)")
appDelegate.registerMainWindow(
orphanWindow!,
windowId: orphanWindowId,
tabManager: orphanManager,
sidebarState: orphanSidebarState,
sidebarSelectionState: orphanSidebarSelectionState
)
orphanWindow = nil
}
RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05))
XCTAssertNil(appDelegate.mainWindow(for: orphanWindowId), "Test precondition: orphaned context should not have a live window")
let orphanCount = orphanManager.tabs.count
let remappedCmdT = StoredShortcut(key: "t", command: true, shift: false, option: false, control: false)
withTemporaryShortcut(action: .newTab, shortcut: remappedCmdT) {
guard let event = makeKeyDownEvent(
key: "t",
modifiers: [.command],
keyCode: 17, // kVK_ANSI_T
windowNumber: 0
) else {
XCTFail("Failed to construct remapped Cmd+T event")
return
}
#if DEBUG
XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: event))
#else
XCTFail("debugHandleCustomShortcut is only available in DEBUG")
#endif
RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05))
}
XCTAssertEqual(orphanManager.tabs.count, orphanCount, "Orphaned manager must not receive a new workspace from remapped Cmd+T")
XCTAssertNil(appDelegate.tabManagerFor(windowId: orphanWindowId), "Remapped Cmd+T should prune the orphaned context after failed resolution")
let createdWindowIds = mainWindowIds().subtracting(existingWindowIds)
for windowId in createdWindowIds {
closeWindow(withId: windowId)
}
}
func testCmdDigitRoutesToEventWindowWhenActiveManagerIsStale() {
guard let appDelegate = AppDelegate.shared else {
XCTFail("Expected AppDelegate.shared")
@ -2378,6 +2443,16 @@ final class AppDelegateShortcutRoutingTests: XCTestCase {
return NSApp.windows.first(where: { $0.identifier?.rawValue == identifier })
}
private func mainWindowIds() -> Set<UUID> {
Set(NSApp.windows.compactMap { window in
guard let raw = window.identifier?.rawValue,
raw.hasPrefix("cmux.main.") else {
return nil
}
return UUID(uuidString: String(raw.dropFirst("cmux.main.".count)))
})
}
private func closeWindow(withId windowId: UUID) {
guard let window = window(withId: windowId) else { return }
window.performClose(nil)