Skip to content

Fix Cmd+N crash from stale workspace creation snapshots #2133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
austinywang merged 2 commits into from
Mar 25, 2026
Merged

Fix Cmd+N crash from stale workspace creation snapshots #2133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ad2c65e
Add regression test for Cmd+N snapshot close race
austinywang Mar 25, 2026
17e8bb1
Fix Cmd+N crash from stale workspace creation snapshots
austinywang Mar 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 74 additions & 47 deletions Sources/TabManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -800,15 +800,24 @@ 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?
private struct WorkspaceCreationTabSnapshot {
let id: UUID
let isPinned: Bool

var selectedWorkspace: Workspace? {
guard let selectedTabId else { return nil }
return tabs.first(where: { $0.id == selectedTabId })
@MainActor
init(workspace: Workspace) {
self.id = workspace.id
self.isPinned = workspace.isPinned
}
}

private struct WorkspaceCreationSnapshot {
let tabs: [WorkspaceCreationTabSnapshot]
let selectedTabId: UUID?
let selectedTabWasPinned: Bool
let preferredWorkingDirectory: String?
let inheritedTerminalConfig: ghostty_surface_config_s?
}
private var agentPIDSweepTimer: DispatchSourceTimer?
private var workspaceGitMetadataPollTimer: DispatchSourceTimer?
#if DEBUG
Expand Down Expand Up @@ -1160,6 +1169,9 @@ class TabManager: ObservableObject {
)
}

/// Test seam for mutating live workspace state after the creation snapshot is captured.
func didCaptureWorkspaceCreationSnapshot() {}

#if DEBUG
private func maybeMutateSelectionDuringWorkspaceCreationForDev(
snapshot: WorkspaceCreationSnapshot
Expand All @@ -1173,14 +1185,15 @@ class TabManager: ObservableObject {
}()
guard isEnabled,
let selectedTabId = snapshot.selectedTabId,
let target = snapshot.tabs.first(where: { $0.id != selectedTabId }) else {
let targetId = snapshot.tabs.lazy.map(\.id).first(where: { $0 != selectedTabId }),
tabs.contains(where: { $0.id == targetId }) else {
return
}
dlog(
"workspace.create.devSelectionMutation from=\(selectedTabId.uuidString.prefix(5)) " +
"to=\(target.id.uuidString.prefix(5))"
"to=\(targetId.uuidString.prefix(5))"
)
self.selectedTabId = target.id
self.selectedTabId = targetId
}
#endif

Expand All @@ -1197,14 +1210,15 @@ class TabManager: ObservableObject {
// 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()
didCaptureWorkspaceCreationSnapshot()
#if DEBUG
maybeMutateSelectionDuringWorkspaceCreationForDev(snapshot: snapshot)
#endif
let nextTabCount = snapshot.tabs.count + 1

@greptile-apps greptile-apps Bot Mar 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Stale tab count used for title and Sentry breadcrumb

nextTabCount is derived from snapshot.tabs.count, which reflects the tab count at snapshot capture time. In the exact race this PR is fixing — where a workspace is closed between snapshot and insertion — nextTabCount will be one higher than the actual live count, resulting in a workspace titled "Terminal N" while only N-1 tabs exist. The Sentry breadcrumb on the next line also reports the inflated count.

Since insertion now operates on tabs (live), the title and breadcrumb could use the live count at that point for accuracy:

Suggested change
let nextTabCount = snapshot.tabs.count + 1
let nextTabCount = tabs.count + 1

This is cosmetic-only, but it would prevent misleading Sentry breadcrumbs in the post-snapshot-close race.

sentryBreadcrumb("workspace.create", data: ["tabCount": nextTabCount])
let explicitWorkingDirectory = normalizedWorkingDirectory(overrideWorkingDirectory)
let workingDirectory = explicitWorkingDirectory ?? preferredWorkingDirectoryForNewTab(snapshot: snapshot)
let inheritedConfig = inheritedTerminalConfigForNewWorkspace(snapshot: snapshot)
let workingDirectory = explicitWorkingDirectory ?? snapshot.preferredWorkingDirectory
let inheritedConfig = snapshot.inheritedTerminalConfig
// Resolve placement against the pre-creation snapshot before Workspace init
// boots terminal state. The ssh/new-workspace path can otherwise crash while
// reading @Published placement state from existing workspaces mid-creation.
Expand All @@ -1224,7 +1238,9 @@ class TabManager: ObservableObject {
if eagerLoadTerminal && !select {
requestBackgroundWorkspaceLoad(for: newWorkspace.id)
}
var updatedTabs = snapshot.tabs
// Apply insertion to the current live array so post-snapshot closes/reorders
// are preserved instead of reintroducing stale workspace instances.
var updatedTabs = tabs
if insertIndex >= 0 && insertIndex <= updatedTabs.count {
updatedTabs.insert(newWorkspace, at: insertIndex)
} else {
Expand Down Expand Up @@ -2156,20 +2172,29 @@ class TabManager: ObservableObject {
}

func terminalPanelForWorkspaceConfigInheritanceSource() -> TerminalPanel? {
terminalPanelForWorkspaceConfigInheritanceSource(snapshot: workspaceCreationSnapshot())
terminalPanelForWorkspaceConfigInheritanceSource(workspace: selectedWorkspace)
}

private func workspaceCreationSnapshot() -> WorkspaceCreationSnapshot {
WorkspaceCreationSnapshot(
tabs: tabs,
selectedTabId: selectedTabId
let currentTabs = tabs
let currentSelectedTabId = selectedTabId
let selectedWorkspace = currentSelectedTabId.flatMap { selectedTabId in
currentTabs.first(where: { $0.id == selectedTabId })
}

return WorkspaceCreationSnapshot(
tabs: currentTabs.map { WorkspaceCreationTabSnapshot(workspace: $0) },
selectedTabId: currentSelectedTabId,
selectedTabWasPinned: selectedWorkspace?.isPinned ?? false,
preferredWorkingDirectory: preferredWorkingDirectoryForNewTab(workspace: selectedWorkspace),
inheritedTerminalConfig: inheritedTerminalConfigForNewWorkspace(workspace: selectedWorkspace)
)
}

private func terminalPanelForWorkspaceConfigInheritanceSource(
snapshot: WorkspaceCreationSnapshot
workspace: Workspace?
) -> TerminalPanel? {
guard let workspace = snapshot.selectedWorkspace else { return nil }
guard let workspace else { return nil }
if let focusedTerminal = workspace.focusedTerminalPanel {
return focusedTerminal
}
Expand All @@ -2184,21 +2209,21 @@ class TabManager: ObservableObject {
}

private func inheritedTerminalConfigForNewWorkspace() -> ghostty_surface_config_s? {
inheritedTerminalConfigForNewWorkspace(snapshot: workspaceCreationSnapshot())
inheritedTerminalConfigForNewWorkspace(workspace: selectedWorkspace)
}

private func inheritedTerminalConfigForNewWorkspace(
snapshot: WorkspaceCreationSnapshot
workspace: Workspace?
) -> ghostty_surface_config_s? {
if let panel = terminalPanelForWorkspaceConfigInheritanceSource(snapshot: snapshot),
if let panel = terminalPanelForWorkspaceConfigInheritanceSource(workspace: workspace),
panel.surface.hasLiveSurface,
let sourceSurface = panel.surface.surface {
return cmuxInheritedSurfaceConfig(
sourceSurface: sourceSurface,
context: GHOSTTY_SURFACE_CONTEXT_TAB
)
}
if let fallbackFontPoints = snapshot.selectedWorkspace?.lastRememberedTerminalFontPointsForConfigInheritance() {
if let fallbackFontPoints = workspace?.lastRememberedTerminalFontPointsForConfigInheritance() {
var config = ghostty_surface_config_new()
config.font_size = fallbackFontPoints
return config
Expand All @@ -2222,44 +2247,46 @@ class TabManager: ObservableObject {
placementOverride: NewWorkspacePlacement? = nil
) -> Int {
let placement = placementOverride ?? WorkspacePlacementSettings.current()
let tabs = snapshot.tabs
var pinnedCount = 0
var selectedIndex: Int?
var selectedIsPinned = false
let selectedTabId = snapshot.selectedTabId

for (index, tab) in tabs.enumerated() {
let liveTabs = tabs.map { WorkspaceCreationTabSnapshot(workspace: $0) }
let pinnedCount = liveTabs.reduce(into: 0) { partial, tab in
if tab.isPinned {
pinnedCount += 1
}
if selectedIndex == nil, tab.id == selectedTabId {
selectedIndex = index
selectedIsPinned = tab.isPinned
partial += 1
}
}

return WorkspacePlacementSettings.insertionIndex(
placement: placement,
selectedIndex: selectedIndex,
selectedIsPinned: selectedIsPinned,
pinnedCount: pinnedCount,
totalCount: tabs.count
)
switch placement {
case .top:
return pinnedCount
case .end:
return liveTabs.count
case .afterCurrent:
if let selectedTabId = snapshot.selectedTabId,
let selectedIndex = liveTabs.firstIndex(where: { $0.id == selectedTabId }) {
return WorkspacePlacementSettings.insertionIndex(
placement: placement,
selectedIndex: selectedIndex,
selectedIsPinned: snapshot.selectedTabWasPinned,
pinnedCount: pinnedCount,
totalCount: liveTabs.count
)
}
return snapshot.selectedTabWasPinned ? pinnedCount : liveTabs.count
}
}

private func preferredWorkingDirectoryForNewTab() -> String? {
preferredWorkingDirectoryForNewTab(snapshot: workspaceCreationSnapshot())
preferredWorkingDirectoryForNewTab(workspace: selectedWorkspace)
}

private func preferredWorkingDirectoryForNewTab(
snapshot: WorkspaceCreationSnapshot
workspace: Workspace?
) -> String? {
guard let tab = snapshot.selectedWorkspace else {
guard let workspace else {
return nil
}
let focusedDirectory = tab.focusedPanelId
.flatMap { tab.panelDirectories[$0] }
let candidate = focusedDirectory ?? tab.currentDirectory
let focusedDirectory = workspace.focusedPanelId
.flatMap { workspace.panelDirectories[$0] }
let candidate = focusedDirectory ?? workspace.currentDirectory
let normalized = normalizeDirectory(candidate)
let trimmed = normalized.trimmingCharacters(in: .whitespacesAndNewlines)
return trimmed.isEmpty ? nil : normalized
Expand Down
49 changes: 49 additions & 0 deletions cmuxTests/WorkspaceUnitTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,13 @@ final class WorkspacePlacementSettingsTests: XCTestCase {
@MainActor
final class WorkspaceCreationPlacementTests: XCTestCase {
private final class SnapshotMutatingTabManager: TabManager {
var afterCaptureWorkspaceCreationSnapshot: (() -> Void)?
var beforeCreateWorkspace: (() -> Void)?

override func didCaptureWorkspaceCreationSnapshot() {
afterCaptureWorkspaceCreationSnapshot?()
}

override func makeWorkspaceForCreation(
title: String,
workingDirectory: String?,
Expand Down Expand Up @@ -399,6 +404,50 @@ final class WorkspaceCreationPlacementTests: XCTestCase {
XCTAssertEqual(manager.selectedTabId, inserted.id)
}

func testAddWorkspaceAfterCurrentDoesNotReinsertClosedWorkspaceCapturedInSnapshot() {
let manager = SnapshotMutatingTabManager()
guard let first = manager.tabs.first else {
XCTFail("Expected initial workspace")
return
}

let second = manager.addWorkspace()
let third = manager.addWorkspace()
manager.selectWorkspace(third)

manager.afterCaptureWorkspaceCreationSnapshot = {
manager.closeWorkspace(second)
}

let inserted = manager.addWorkspace(placementOverride: .afterCurrent)

XCTAssertEqual(manager.tabs.map(\.id), [first.id, third.id, inserted.id])
XCTAssertFalse(manager.tabs.contains(where: { $0.id == second.id }))
XCTAssertEqual(manager.selectedTabId, inserted.id)
}

func testAddWorkspaceSurvivesSelectedWorkspaceClosingAfterSnapshot() {
let manager = SnapshotMutatingTabManager()
guard let first = manager.tabs.first else {
XCTFail("Expected initial workspace")
return
}

let second = manager.addWorkspace()
let third = manager.addWorkspace()
manager.selectWorkspace(third)

manager.afterCaptureWorkspaceCreationSnapshot = {
manager.closeWorkspace(third)
}

let inserted = manager.addWorkspace(placementOverride: .afterCurrent)

XCTAssertEqual(manager.tabs.map(\.id), [first.id, second.id, inserted.id])
XCTAssertFalse(manager.tabs.contains(where: { $0.id == third.id }))
XCTAssertEqual(manager.selectedTabId, inserted.id)
}

private func makeManagerWithThreeWorkspaces() -> TabManager {
let manager = TabManager()
_ = manager.addWorkspace()
Expand Down
Loading