diff --git a/Sources/TabManager+SessionPersistence.swift b/Sources/TabManager+SessionPersistence.swift index 2bc889d2..f20ce2f6 100644 --- a/Sources/TabManager+SessionPersistence.swift +++ b/Sources/TabManager+SessionPersistence.swift @@ -13,8 +13,19 @@ extension TabManager { } func sessionSnapshot(includeScrollback: Bool) -> SessionTabManagerSnapshot { + // Only a *live* remote workspace is excluded -- a workspace the user disconnected from + // its remote host but kept using locally must not silently vanish from every snapshot + // for the rest of its life (see incident: three live local terminals lost on relaunch + // because `isRemoteWorkspace` alone stays true after disconnect). Whatever is skipped + // here is logged so the gap is never silent again. + for tab in tabs where tab.isLiveRemoteWorkspace { + dilog( + "session.snapshot", + "skipped workspace=\(tab.id.uuidString.prefix(8)) reason=remote panels=\(tab.panels.count)" + ) + } let restorableTabs = tabs - .filter { !$0.isRemoteWorkspace } + .filter { !$0.isLiveRemoteWorkspace } .prefix(SessionPersistencePolicy.maxWorkspacesPerWindow) let workspaceSnapshots = restorableTabs .map { $0.sessionSnapshot(includeScrollback: includeScrollback) } diff --git a/Sources/TerminalController+Telemetry.swift b/Sources/TerminalController+Telemetry.swift index 8e234622..6cfad04f 100644 --- a/Sources/TerminalController+Telemetry.swift +++ b/Sources/TerminalController+Telemetry.swift @@ -70,14 +70,17 @@ extension TerminalController { validSurfaceIds: validSurfaceIds ) guard let surfaceId, validSurfaceIds.contains(surfaceId) else { - if tab.isRemoteWorkspace, validSurfaceIds.isEmpty { + // `isLiveRemoteWorkspace`, not `isRemoteWorkspace`: a disconnected-but-configured + // workspace has no active remote session to buffer this report for, and its + // panels are ordinary local shells again -- route it like any local workspace. + if tab.isLiveRemoteWorkspace, validSurfaceIds.isEmpty { tab.rememberPendingRemoteSurfaceTTY(ttyName, requestedSurfaceId: requestedSurfaceId) } return } tab.surfaceTTYNames[surfaceId] = ttyName - if tab.isRemoteWorkspace { + if tab.isLiveRemoteWorkspace { tab.syncRemotePortScanTTYs() _ = tab.applyPendingRemoteSurfacePortKickIfNeeded(to: surfaceId) } else { @@ -127,7 +130,10 @@ extension TerminalController { validSurfaceIds: validSurfaceIds ) guard let surfaceId, validSurfaceIds.contains(surfaceId) else { - if tab.isRemoteWorkspace, validSurfaceIds.isEmpty { + // See v2SurfaceReportTTY above: `isLiveRemoteWorkspace`, not `isRemoteWorkspace` + // -- a disconnected-but-configured workspace has no active remote session to + // buffer this kick for. + if tab.isLiveRemoteWorkspace, validSurfaceIds.isEmpty { tab.rememberPendingRemoteSurfacePortKick( reason: reason, requestedSurfaceId: requestedSurfaceId @@ -136,7 +142,7 @@ extension TerminalController { return } - if tab.isRemoteWorkspace { + if tab.isLiveRemoteWorkspace { tab.kickRemotePortScan(panelId: surfaceId, reason: reason) } else { PortScanner.shared.kick(workspaceId: workspaceId, panelId: surfaceId) diff --git a/Sources/Workspace+Remote.swift b/Sources/Workspace+Remote.swift index 7ff2d529..894a3e91 100644 --- a/Sources/Workspace+Remote.swift +++ b/Sources/Workspace+Remote.swift @@ -16,6 +16,21 @@ extension Workspace { remoteConfiguration != nil } + /// Whether this workspace is a *live* remote session right now -- connected, connecting, or + /// erroring while still configured -- as opposed to merely having been configured for remote + /// at some point. `remoteConfiguration` intentionally survives a user-initiated disconnect + /// (see `disconnectRemoteConnection(clearConfiguration:)`, whose default and the sidebar's + /// disconnect action both pass `false`) so `reconnectRemoteConnection()` still has something + /// to reconnect to. That means `isRemoteWorkspace` alone is NOT a safe proxy for "this + /// workspace's panels currently live on a remote host" -- a disconnected-but-configured + /// workspace is, for every practical purpose (running shells, port telemetry, session + /// persistence), a local workspace again. Use this property anywhere that distinction + /// matters; use `isRemoteWorkspace` only for "has a remote destination configured" facts + /// (e.g. whether the sidebar should offer Reconnect). + var isLiveRemoteWorkspace: Bool { + isRemoteWorkspace && remoteConnectionState != .disconnected + } + @MainActor func isRemoteTerminalSurface(_ panelId: UUID) -> Bool { activeRemoteTerminalSurfaceIds.contains(panelId) diff --git a/programaTests/TabManagerSessionSnapshotTests.swift b/programaTests/TabManagerSessionSnapshotTests.swift index 8920f496..70bd8883 100644 --- a/programaTests/TabManagerSessionSnapshotTests.swift +++ b/programaTests/TabManagerSessionSnapshotTests.swift @@ -47,6 +47,12 @@ final class TabManagerSessionSnapshotTests: XCTestCase { XCTAssertNotNil(manager.selectedTabId) } + /// Only a *live* remote workspace is excluded from restore -- `remoteConfiguration` alone + /// isn't enough, since it deliberately survives a user-initiated disconnect so Reconnect + /// keeps working (see `Workspace.isLiveRemoteWorkspace`'s doc comment). This test therefore + /// simulates an actually-connected remote session; the disconnected case (which must persist + /// its local panels) is covered by + /// `WorkspaceRemoteConnectionTests.testDisconnectedRemoteWorkspacePersistsLocalPanelsInSessionSnapshot`. func testSessionSnapshotExcludesRemoteWorkspacesFromRestore() throws { let manager = TabManager() let remoteWorkspace = manager.addWorkspace(select: true) @@ -63,6 +69,7 @@ final class TabManagerSessionSnapshotTests: XCTestCase { terminalStartupCommand: "ssh cmux-macmini" ) remoteWorkspace.configureRemoteConnection(configuration, autoConnect: false) + remoteWorkspace.applyRemoteConnectionStateUpdate(.connected, detail: nil, target: "cmux-macmini") let paneId = try XCTUnwrap(remoteWorkspace.bonsplitController.allPaneIds.first) _ = remoteWorkspace.newBrowserSurface(inPane: paneId, url: URL(string: "http://localhost:3000"), focus: false) diff --git a/programaTests/WorkspaceRemoteConnectionTests.swift b/programaTests/WorkspaceRemoteConnectionTests.swift index 164018db..d6468654 100644 --- a/programaTests/WorkspaceRemoteConnectionTests.swift +++ b/programaTests/WorkspaceRemoteConnectionTests.swift @@ -426,6 +426,66 @@ final class WorkspaceRemoteConnectionTests: XCTestCase { XCTAssertEqual(workspace.remoteConnectionState, .disconnected) } + /// Regression for the "disconnect a remote workspace, keep using it locally, lose every + /// terminal in it at the next restart" incident: `remoteConfiguration` intentionally + /// survives a user-initiated disconnect (see `disconnectRemoteConnection`'s doc comment) so + /// `reconnectRemoteConnection()` keeps working, but that meant `isRemoteWorkspace` alone -- + /// config presence, not live connection state -- excluded the whole workspace, and every + /// local pane in it, from `sessionSnapshot(includeScrollback:)` forever after disconnect. + @MainActor + func testDisconnectedRemoteWorkspacePersistsLocalPanelsInSessionSnapshot() throws { + let manager = TabManager() + let remoteWorkspace = manager.addWorkspace(select: true) + let paneId = try XCTUnwrap(remoteWorkspace.bonsplitController.allPaneIds.first) + let config = WorkspaceRemoteConfiguration( + destination: "cmux-macmini", + port: nil, + identityFile: nil, + sshOptions: [], + localProxyPort: nil, + relayPort: 64034, + relayID: String(repeating: "a", count: 16), + relayToken: String(repeating: "b", count: 64), + localSocketPath: "/tmp/programa-debug-test.sock", + terminalStartupCommand: "ssh cmux-macmini" + ) + + // Configuring seeds the workspace's sole existing terminal panel as a tracked remote + // surface; two more splits created while still configured pick up the same tracking -- + // mirroring the incident's three live remote-tracked terminals. + remoteWorkspace.configureRemoteConnection(config, autoConnect: false) + let firstPanelId = try XCTUnwrap(remoteWorkspace.focusedTerminalPanel?.id) + let secondPanelId = try XCTUnwrap(remoteWorkspace.newTerminalSurface(inPane: paneId, focus: false)?.id) + let thirdPanelId = try XCTUnwrap(remoteWorkspace.newTerminalSurface(inPane: paneId, focus: false)?.id) + for panelId in [firstPanelId, secondPanelId, thirdPanelId] { + XCTAssertTrue(remoteWorkspace.isRemoteTerminalSurface(panelId)) + } + + remoteWorkspace.applyRemoteConnectionStateUpdate(.connected, detail: nil, target: "cmux-macmini") + XCTAssertEqual(remoteWorkspace.remoteConnectionState, .connected) + + // The user disconnects from the sidebar -- mirrors `TabItemView`'s disconnect action, + // which passes `clearConfiguration: false` so Reconnect keeps working. + remoteWorkspace.disconnectRemoteConnection(clearConfiguration: false) + XCTAssertTrue(remoteWorkspace.isRemoteWorkspace) + XCTAssertEqual(remoteWorkspace.remoteConnectionState, .disconnected) + for panelId in [firstPanelId, secondPanelId, thirdPanelId] { + XCTAssertFalse(remoteWorkspace.isRemoteTerminalSurface(panelId)) + } + + let snapshot = manager.sessionSnapshot(includeScrollback: false) + let restoredPanelIds = Set( + snapshot.workspaces + .first(where: { $0.panels.contains { $0.id == firstPanelId } })? + .panels + .map(\.id) ?? [] + ) + + XCTAssertTrue(restoredPanelIds.contains(firstPanelId)) + XCTAssertTrue(restoredPanelIds.contains(secondPanelId)) + XCTAssertTrue(restoredPanelIds.contains(thirdPanelId)) + } + @MainActor func testRemoteTerminalSessionEndRequestsControlMasterCleanupWhenWorkspaceDemotes() throws { let workspace = Workspace()