Skip to content

Fix #1983: avoid workspace creation crash after restore #1985

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
lawrencecchen merged 2 commits into from
Mar 25, 2026
+27 −12
Merged

Fix #1983: avoid workspace creation crash after restore #1985

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
b431b66
Fix workspace creation crash after restore
austinywang Mar 23, 2026
61e25a5
Merge remote-tracking branch 'origin/main' into issue-1983-newTabInse…
lawrencecchen 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
6 changes: 3 additions & 3 deletions Sources/PortScanner.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import Foundation
final class PortScanner: @unchecked Sendable {
static let shared = PortScanner()

/// Callback delivers `(workspaceId, panelId, ports)` on main thread.
var onPortsUpdated: ((_ workspaceId: UUID, _ panelId: UUID, _ ports: [Int]) -> Void)?
/// Callback delivers `(workspaceId, panelId, ports)` on the main actor.
var onPortsUpdated: (@MainActor (_ workspaceId: UUID, _ panelId: UUID, _ ports: [Int]) -> Void)?

// MARK: - State (all guarded by `queue`)

Expand Down Expand Up @@ -171,7 +171,7 @@ final class PortScanner: @unchecked Sendable {

private func deliverResults(_ results: [(PanelKey, [Int])]) {
guard let callback = onPortsUpdated else { return }
DispatchQueue.main.async {
Task { @MainActor in
for (key, ports) in results {
callback(key.workspaceId, key.panelId, ports)
}
Expand Down
19 changes: 18 additions & 1 deletion Sources/TabManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5163,8 +5163,19 @@ extension TabManager {
)
}

private func releaseRestoredAwayWorkspace(_ workspace: Workspace) {
// Session restore replaces the bootstrap workspace objects with freshly
// restored ones. Tear the old graph down after the atomic swap so late
// panel/socket callbacks cannot keep mutating hidden pre-restore state.
AppDelegate.shared?.notificationStore?.clearNotifications(forTabId: workspace.id)
workspace.teardownAllPanels()
workspace.teardownRemoteConnection()
workspace.owningTabManager = nil
Comment on lines +5166 to +5173
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 25, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Detach the old workspace before tearing its panels down.

Unlike closeWorkspace(_:), this helper runs after the old workspace has already been removed from tabs. Workspace can still call back into owningTabManager?.closeWorkspaceWithConfirmation(self) during panel teardown (see Sources/Workspace.swift, Lines 9747-9750), so leaving owningTabManager set through teardownAllPanels() lets restore cleanup re-enter TabManager against the restored tab list. That can prompt or close the wrong workspace/window. Nil the manager before teardown.

🛠️ Proposed fix 
 private func releaseRestoredAwayWorkspace(_ workspace: Workspace) {
     // Session restore replaces the bootstrap workspace objects with freshly
     // restored ones. Tear the old graph down after the atomic swap so late
     // panel/socket callbacks cannot keep mutating hidden pre-restore state.
     AppDelegate.shared?.notificationStore?.clearNotifications(forTabId: workspace.id)
+    workspace.owningTabManager = nil
     workspace.teardownAllPanels()
     workspace.teardownRemoteConnection()
-    workspace.owningTabManager = nil
 }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Sources/TabManager.swift` around lines 5166 - 5173, In
releaseRestoredAwayWorkspace, clear the workspace's owningTabManager before
tearing down panels or the remote connection to prevent callbacks (e.g.,
Workspace.closeWorkspaceWithConfirmation called during teardownAllPanels) from
re-entering TabManager against the restored tab list; specifically, set
workspace.owningTabManager = nil prior to calling workspace.teardownAllPanels()
and workspace.teardownRemoteConnection() in the releaseRestoredAwayWorkspace
helper.

}

func restoreSessionSnapshot(_ snapshot: SessionTabManagerSnapshot) {
for tab in tabs {
let previousTabs = tabs
for tab in previousTabs {
unwireClosedBrowserTracking(for: tab)
}
let existingProbeKeys = Set(workspaceGitProbeGenerationByKey.keys)
Expand Down Expand Up @@ -5228,6 +5239,12 @@ extension TabManager {
// never see an intermediate state with empty tabs or nil selection.
tabs = newTabs
selectedTabId = newSelectedId
let existingIds = Set(newTabs.map(\.id))
pruneBackgroundWorkspaceLoads(existingIds: existingIds)
sidebarSelectedWorkspaceIds.formIntersection(existingIds)
for workspace in previousTabs {
releaseRestoredAwayWorkspace(workspace)
}
for workspace in newTabs {
let terminalPanels = workspace.panels.values.compactMap { $0 as? TerminalPanel }
for terminalPanel in terminalPanels {
Expand Down
14 changes: 6 additions & 8 deletions Sources/TerminalController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1020,14 +1020,12 @@ class TerminalController {

// Wire batched port scanner results back to workspace state.
PortScanner.shared.onPortsUpdated = { [weak self] workspaceId, panelId, ports in
MainActor.assumeIsolated {
guard let self, let tabManager = self.tabManager else { return }
guard let workspace = tabManager.tabs.first(where: { $0.id == workspaceId }) else { return }
let validSurfaceIds = Set(workspace.panels.keys)
guard validSurfaceIds.contains(panelId) else { return }
workspace.surfaceListeningPorts[panelId] = ports.isEmpty ? nil : ports
workspace.recomputeListeningPorts()
}
guard let self, let tabManager = self.tabManager else { return }
guard let workspace = tabManager.tabs.first(where: { $0.id == workspaceId }) else { return }
Comment on lines 1022 to +1024
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 23, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Resolve port-scan updates by workspaceId, not the active-window tabManager.

Line 1023/Line 1024 still route through self.tabManager, which only tracks the active window. Port updates for workspaces in other windows will be dropped here, so their surfaceListeningPorts/listeningPorts can silently go stale.

Suggested fix 
-        PortScanner.shared.onPortsUpdated = { [weak self] workspaceId, panelId, ports in
-            guard let self, let tabManager = self.tabManager else { return }
-            guard let workspace = tabManager.tabs.first(where: { $0.id == workspaceId }) else { return }
+        PortScanner.shared.onPortsUpdated = { workspaceId, panelId, ports in
+            guard let workspace = AppDelegate.shared?.workspaceFor(tabId: workspaceId) else { return }
             let validSurfaceIds = Set(workspace.panels.keys)
             guard validSurfaceIds.contains(panelId) else { return }
             workspace.surfaceListeningPorts[panelId] = ports.isEmpty ? nil : ports
             workspace.recomputeListeningPorts()
         }

Based on learnings: sendPickedElementToTerminal(workspaceId:summary:) resolves the target workspace via AppDelegate.shared?.workspaceFor(tabId: workspaceId), which searches across all mainWindowContexts (not self.tabManager, which is active-window only).

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Sources/TerminalController.swift` around lines 1022 - 1024, The closure
assigned to PortScanner.shared.onPortsUpdated currently resolves the workspace
via self.tabManager (active-window only) which drops updates for workspaces in
other windows; change the resolution to use
AppDelegate.shared?.workspaceFor(tabId: workspaceId) (as used by
sendPickedElementToTerminal(workspaceId:summary:)) so the lookup searches across
all mainWindowContexts, and keep the existing weak self capture and early-return
behavior if workspace lookup fails; update any references to
tabManager.tabs.first(where:) to use the AppDelegate lookup to ensure
surfaceListeningPorts/listeningPorts are updated for workspaces in other
windows.

let validSurfaceIds = Set(workspace.panels.keys)
guard validSurfaceIds.contains(panelId) else { return }
workspace.surfaceListeningPorts[panelId] = ports.isEmpty ? nil : ports
workspace.recomputeListeningPorts()
}

// Accept connections in background thread
Expand Down
Loading