Merge pull request #2133 from manaflow-ai/issue-2131-cmd-n-crash-regression
Fix Cmd+N crash from stale workspace creation snapshots
This commit is contained in:
commit
11a841e020
2 changed files with 123 additions and 47 deletions
|
|
@ -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
|
||||
|
|
@ -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
|
||||
|
|
@ -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
|
||||
|
||||
|
|
@ -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
|
||||
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.
|
||||
|
|
@ -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 {
|
||||
|
|
@ -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
|
||||
}
|
||||
|
|
@ -2184,13 +2209,13 @@ 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(
|
||||
|
|
@ -2198,7 +2223,7 @@ class TabManager: ObservableObject {
|
|||
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
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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?,
|
||||
|
|
@ -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()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue