diff --git a/Sources/TabManager.swift b/Sources/TabManager.swift index 2c4cadaf..4633225e 100644 --- a/Sources/TabManager.swift +++ b/Sources/TabManager.swift @@ -2851,14 +2851,27 @@ class TabManager: ObservableObject { func closePanelAfterChildExited(tabId: UUID, surfaceId: UUID) { guard let tab = tabs.first(where: { $0.id == tabId }) else { return } guard tab.panels[surfaceId] != nil else { return } + let keepsRemoteWorkspaceOpen = + tab.panels.count <= 1 && tab.shouldDemoteWorkspaceAfterChildExit(surfaceId: surfaceId) #if DEBUG dlog( "surface.close.childExited tab=\(tabId.uuidString.prefix(5)) " + - "surface=\(surfaceId.uuidString.prefix(5)) panels=\(tab.panels.count) workspaces=\(tabs.count)" + "surface=\(surfaceId.uuidString.prefix(5)) panels=\(tab.panels.count) workspaces=\(tabs.count) " + + "remoteWorkspace=\(tab.isRemoteWorkspace ? 1 : 0) keepRemote=\(keepsRemoteWorkspaceOpen ? 1 : 0)" ) #endif + // Exiting the last SSH surface should demote the workspace back to a local one. + // Route through Workspace close handling so remote teardown and replacement-panel + // logic run before TabManager considers removing the workspace itself, including + // session-end paths where remote configuration was cleared before Ghostty delivered + // the child-exit callback. + if keepsRemoteWorkspaceOpen { + closeRuntimeSurface(tabId: tabId, surfaceId: surfaceId) + return + } + // Child-exit on the last panel should collapse the workspace, matching explicit close // semantics (and close the window when it was the last workspace). if tab.panels.count <= 1 { diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index 82a7256f..6bfab1c7 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -5469,14 +5469,20 @@ final class Workspace: Identifiable, ObservableObject { private var remoteLastDaemonErrorFingerprint: String? private var remoteLastPortConflictFingerprint: String? private var activeRemoteTerminalSurfaceIds: Set = [] + private var pendingRemoteTerminalChildExitSurfaceIds: Set = [] private static let remoteErrorStatusKey = "remote.error" private static let remotePortConflictStatusKey = "remote.port_conflicts" + private static let sshControlMasterCleanupQueue = DispatchQueue( + label: "com.cmux.remote-ssh.control-master-cleanup", + qos: .utility + ) private static let remoteHeartbeatDateFormatter: ISO8601DateFormatter = { let formatter = ISO8601DateFormatter() formatter.formatOptions = [.withInternetDateTime, .withFractionalSeconds] return formatter }() + nonisolated(unsafe) static var runSSHControlMasterCommandOverrideForTesting: (([String]) -> Void)? private var panelShellActivityStates: [UUID: PanelShellActivityState] = [:] /// PIDs associated with agent status entries (e.g. claude_code), keyed by status key. /// Used for stale-session detection: if the PID is dead, the status entry is cleared. @@ -6670,6 +6676,11 @@ final class Workspace: Identifiable, ObservableObject { activeRemoteTerminalSurfaceIds.contains(panelId) } + @MainActor + func shouldDemoteWorkspaceAfterChildExit(surfaceId: UUID) -> Bool { + isRemoteWorkspace || pendingRemoteTerminalChildExitSurfaceIds.contains(surfaceId) + } + var remoteDisplayTarget: String? { remoteConfiguration?.displayTarget } @@ -6825,6 +6836,9 @@ final class Workspace: Identifiable, ObservableObject { } func disconnectRemoteConnection(clearConfiguration: Bool = false) { + let shouldCleanupControlMaster = + clearConfiguration && !isDetachingCloseTransaction && pendingDetachedSurfaces.isEmpty + let configurationForCleanup = shouldCleanupControlMaster ? remoteConfiguration : nil let previousController = remoteSessionController activeRemoteSessionControllerID = nil remoteSessionController = nil @@ -6851,10 +6865,13 @@ final class Workspace: Identifiable, ObservableObject { applyRemoteProxyEndpointUpdate(nil) applyBrowserRemoteWorkspaceStatusToPanels() recomputeListeningPorts() + if let configurationForCleanup { + Self.requestSSHControlMasterCleanupIfNeeded(configuration: configurationForCleanup) + } } private func clearRemoteConfigurationIfWorkspaceBecameLocal() { - guard panels.isEmpty, remoteConfiguration != nil else { return } + guard !isDetachingCloseTransaction, panels.isEmpty, remoteConfiguration != nil else { return } disconnectRemoteConnection(clearConfiguration: true) } @@ -6871,6 +6888,7 @@ final class Workspace: Identifiable, ObservableObject { } private func trackRemoteTerminalSurface(_ panelId: UUID) { + pendingRemoteTerminalChildExitSurfaceIds.remove(panelId) guard activeRemoteTerminalSurfaceIds.insert(panelId).inserted else { return } activeRemoteTerminalSessionCount = activeRemoteTerminalSurfaceIds.count } @@ -6878,6 +6896,7 @@ final class Workspace: Identifiable, ObservableObject { private func untrackRemoteTerminalSurface(_ panelId: UUID) { guard activeRemoteTerminalSurfaceIds.remove(panelId) != nil else { return } activeRemoteTerminalSessionCount = activeRemoteTerminalSurfaceIds.count + guard !isDetachingCloseTransaction else { return } maybeDemoteRemoteWorkspaceAfterSSHSessionEnded() } @@ -6898,6 +6917,7 @@ final class Workspace: Identifiable, ObservableObject { remoteConfiguration?.relayPort == relayPort else { return } + pendingRemoteTerminalChildExitSurfaceIds.insert(surfaceId) untrackRemoteTerminalSurface(surfaceId) } @@ -6905,6 +6925,79 @@ final class Workspace: Identifiable, ObservableObject { disconnectRemoteConnection(clearConfiguration: true) } + private static func requestSSHControlMasterCleanupIfNeeded(configuration: WorkspaceRemoteConfiguration) { + guard let arguments = sshControlMasterCleanupArguments(configuration: configuration) else { return } + if let override = runSSHControlMasterCommandOverrideForTesting { + override(arguments) + return + } + + sshControlMasterCleanupQueue.async { + let process = Process() + process.executableURL = URL(fileURLWithPath: "/usr/bin/ssh") + process.arguments = arguments + process.standardInput = FileHandle.nullDevice + process.standardOutput = FileHandle.nullDevice + process.standardError = FileHandle.nullDevice + let exitSemaphore = DispatchSemaphore(value: 0) + process.terminationHandler = { _ in + exitSemaphore.signal() + } + + do { + try process.run() + if exitSemaphore.wait(timeout: .now() + 5) == .timedOut { + if process.isRunning { + process.terminate() + } + _ = exitSemaphore.wait(timeout: .now() + 1) + } + } catch { + return + } + } + } + + private static func sshControlMasterCleanupArguments(configuration: WorkspaceRemoteConfiguration) -> [String]? { + let sshOptions = normalizedSSHControlCleanupOptions(configuration.sshOptions) + var arguments: [String] = [ + "-o", "BatchMode=yes", + "-o", "ControlMaster=no", + ] + if let port = configuration.port { + arguments += ["-p", String(port)] + } + if let identityFile = configuration.identityFile?.trimmingCharacters(in: .whitespacesAndNewlines), + !identityFile.isEmpty { + arguments += ["-i", identityFile] + } + for option in sshOptions { + arguments += ["-o", option] + } + arguments += ["-O", "exit", configuration.destination] + return arguments + } + + private static func normalizedSSHControlCleanupOptions(_ options: [String]) -> [String] { + let disallowedKeys: Set = ["controlmaster", "controlpersist"] + return options.compactMap { option in + let trimmed = option.trimmingCharacters(in: .whitespacesAndNewlines) + guard !trimmed.isEmpty else { return nil } + guard let key = sshOptionKeyForControlCleanup(trimmed) else { return nil } + return disallowedKeys.contains(key) ? nil : trimmed + } + } + + private static func sshOptionKeyForControlCleanup(_ option: String) -> String? { + let trimmed = option.trimmingCharacters(in: .whitespacesAndNewlines) + guard !trimmed.isEmpty else { return nil } + return trimmed + .split(whereSeparator: { $0 == "=" || $0.isWhitespace }) + .first + .map(String.init)? + .lowercased() + } + func applyRemoteConnectionStateUpdate( _ state: WorkspaceRemoteConnectionState, detail: String?, @@ -7688,6 +7781,7 @@ final class Workspace: Identifiable, ObservableObject { panels.removeAll(keepingCapacity: false) surfaceIdToPanelId.removeAll(keepingCapacity: false) panelSubscriptions.removeAll(keepingCapacity: false) + pendingRemoteTerminalChildExitSurfaceIds.removeAll(keepingCapacity: false) pruneSurfaceMetadata(validSurfaceIds: []) restoredTerminalScrollbackByPanelId.removeAll(keepingCapacity: false) terminalInheritanceFontPointsByPanelId.removeAll(keepingCapacity: false) @@ -10087,6 +10181,7 @@ extension Workspace: BonsplitDelegate { panels.removeValue(forKey: panelId) untrackRemoteTerminalSurface(panelId) + pendingRemoteTerminalChildExitSurfaceIds.remove(panelId) surfaceIdToPanelId.removeValue(forKey: tabId) panelDirectories.removeValue(forKey: panelId) panelGitBranches.removeValue(forKey: panelId) @@ -10237,6 +10332,7 @@ extension Workspace: BonsplitDelegate { panels[panelId]?.close() panels.removeValue(forKey: panelId) untrackRemoteTerminalSurface(panelId) + pendingRemoteTerminalChildExitSurfaceIds.remove(panelId) panelDirectories.removeValue(forKey: panelId) panelGitBranches.removeValue(forKey: panelId) panelPullRequests.removeValue(forKey: panelId) diff --git a/cmuxTests/TabManagerUnitTests.swift b/cmuxTests/TabManagerUnitTests.swift index d8ec097a..332c288f 100644 --- a/cmuxTests/TabManagerUnitTests.swift +++ b/cmuxTests/TabManagerUnitTests.swift @@ -162,6 +162,89 @@ final class TabManagerChildExitCloseTests: XCTestCase { ) } + func testChildExitOnLastRemotePanelKeepsWorkspaceAndDemotesToLocal() throws { + let manager = TabManager() + guard let workspace = manager.selectedWorkspace, + let remotePanelId = workspace.focusedPanelId else { + XCTFail("Expected selected workspace with focused panel") + return + } + + workspace.configureRemoteConnection( + WorkspaceRemoteConfiguration( + destination: "cmux-macmini", + port: nil, + identityFile: nil, + sshOptions: [], + localProxyPort: nil, + relayPort: 64015, + relayID: String(repeating: "a", count: 16), + relayToken: String(repeating: "b", count: 64), + localSocketPath: "/tmp/cmux-debug-test.sock", + terminalStartupCommand: "ssh cmux-macmini" + ), + autoConnect: false + ) + + XCTAssertTrue(workspace.isRemoteWorkspace) + XCTAssertTrue(workspace.isRemoteTerminalSurface(remotePanelId)) + + manager.closePanelAfterChildExited(tabId: workspace.id, surfaceId: remotePanelId) + drainMainQueue() + drainMainQueue() + + XCTAssertEqual(manager.tabs.count, 1) + XCTAssertEqual(manager.selectedTabId, workspace.id) + XCTAssertEqual(manager.tabs.first?.id, workspace.id) + XCTAssertFalse(workspace.isRemoteWorkspace) + XCTAssertNil(workspace.panels[remotePanelId]) + XCTAssertEqual(workspace.panels.count, 1) + XCTAssertNotEqual(workspace.focusedPanelId, remotePanelId) + XCTAssertEqual(workspace.activeRemoteTerminalSessionCount, 0) + } + + func testChildExitAfterRemoteSessionEndKeepsWorkspaceAndDemotesToLocal() throws { + let manager = TabManager() + guard let workspace = manager.selectedWorkspace, + let remotePanelId = workspace.focusedPanelId else { + XCTFail("Expected selected workspace with focused panel") + return + } + + workspace.configureRemoteConnection( + WorkspaceRemoteConfiguration( + destination: "cmux-macmini", + port: nil, + identityFile: nil, + sshOptions: [], + localProxyPort: nil, + relayPort: 64016, + relayID: String(repeating: "a", count: 16), + relayToken: String(repeating: "b", count: 64), + localSocketPath: "/tmp/cmux-debug-test.sock", + terminalStartupCommand: "ssh cmux-macmini" + ), + autoConnect: false + ) + + workspace.markRemoteTerminalSessionEnded(surfaceId: remotePanelId, relayPort: 64016) + + XCTAssertFalse(workspace.isRemoteWorkspace) + + manager.closePanelAfterChildExited(tabId: workspace.id, surfaceId: remotePanelId) + drainMainQueue() + drainMainQueue() + + XCTAssertEqual(manager.tabs.count, 1) + XCTAssertEqual(manager.selectedTabId, workspace.id) + XCTAssertEqual(manager.tabs.first?.id, workspace.id) + XCTAssertFalse(workspace.isRemoteWorkspace) + XCTAssertNil(workspace.panels[remotePanelId]) + XCTAssertEqual(workspace.panels.count, 1) + XCTAssertNotEqual(workspace.focusedPanelId, remotePanelId) + XCTAssertEqual(workspace.activeRemoteTerminalSessionCount, 0) + } + func testChildExitOnNonLastPanelClosesOnlyPanel() { let manager = TabManager() guard let workspace = manager.selectedWorkspace, diff --git a/cmuxTests/WorkspaceRemoteConnectionTests.swift b/cmuxTests/WorkspaceRemoteConnectionTests.swift index 47fcd120..6ff1dfc9 100644 --- a/cmuxTests/WorkspaceRemoteConnectionTests.swift +++ b/cmuxTests/WorkspaceRemoteConnectionTests.swift @@ -247,6 +247,245 @@ final class WorkspaceRemoteConnectionTests: XCTestCase { XCTAssertFalse(workspace.isRemoteTerminalSurface(panelID)) } + @MainActor + func testRemoteTerminalSessionEndRequestsControlMasterCleanupWhenWorkspaceDemotes() throws { + let workspace = Workspace() + let config = WorkspaceRemoteConfiguration( + destination: "cmux-macmini", + port: 2222, + identityFile: "/Users/test/.ssh/id_ed25519", + sshOptions: [ + "ControlMaster=auto", + "ControlPersist=600", + "ControlPath=/tmp/cmux-ssh-%C", + "StrictHostKeyChecking=accept-new", + ], + localProxyPort: nil, + relayPort: 64012, + relayID: String(repeating: "a", count: 16), + relayToken: String(repeating: "b", count: 64), + localSocketPath: "/tmp/cmux-debug-test.sock", + terminalStartupCommand: "ssh cmux-macmini" + ) + let cleanupRequested = expectation(description: "control master cleanup requested") + var capturedArguments: [String] = [] + + Workspace.runSSHControlMasterCommandOverrideForTesting = { arguments in + capturedArguments = arguments + cleanupRequested.fulfill() + } + defer { Workspace.runSSHControlMasterCommandOverrideForTesting = nil } + + workspace.configureRemoteConnection(config, autoConnect: false) + + let panelID = try XCTUnwrap(workspace.focusedTerminalPanel?.id) + workspace.markRemoteTerminalSessionEnded(surfaceId: panelID, relayPort: 64012) + + wait(for: [cleanupRequested], timeout: 1.0) + + XCTAssertFalse(workspace.isRemoteWorkspace) + XCTAssertEqual(workspace.activeRemoteTerminalSessionCount, 0) + XCTAssertEqual( + capturedArguments, + [ + "-o", "BatchMode=yes", + "-o", "ControlMaster=no", + "-p", "2222", + "-i", "/Users/test/.ssh/id_ed25519", + "-o", "ControlPath=/tmp/cmux-ssh-%C", + "-o", "StrictHostKeyChecking=accept-new", + "-O", "exit", + "cmux-macmini", + ] + ) + } + + @MainActor + func testTeardownRemoteConnectionRequestsControlMasterCleanupWhileStillConnecting() { + let workspace = Workspace() + let config = WorkspaceRemoteConfiguration( + destination: "cmux-macmini", + port: nil, + identityFile: nil, + sshOptions: [ + "ControlMaster=auto", + "ControlPersist=600", + "ControlPath=/tmp/cmux-ssh-%C", + ], + localProxyPort: nil, + relayPort: 64014, + relayID: String(repeating: "a", count: 16), + relayToken: String(repeating: "b", count: 64), + localSocketPath: "/tmp/cmux-debug-test.sock", + terminalStartupCommand: "ssh cmux-macmini" + ) + let cleanupRequested = expectation(description: "control master cleanup requested") + var capturedArguments: [String] = [] + + Workspace.runSSHControlMasterCommandOverrideForTesting = { arguments in + capturedArguments = arguments + cleanupRequested.fulfill() + } + defer { Workspace.runSSHControlMasterCommandOverrideForTesting = nil } + + workspace.configureRemoteConnection(config, autoConnect: false) + workspace.applyRemoteConnectionStateUpdate( + .connecting, + detail: "Connecting to cmux-macmini", + target: "cmux-macmini" + ) + + workspace.teardownRemoteConnection() + + wait(for: [cleanupRequested], timeout: 1.0) + + XCTAssertFalse(workspace.isRemoteWorkspace) + XCTAssertEqual( + capturedArguments, + [ + "-o", "BatchMode=yes", + "-o", "ControlMaster=no", + "-o", "ControlPath=/tmp/cmux-ssh-%C", + "-O", "exit", + "cmux-macmini", + ] + ) + } + + @MainActor + func testTeardownRemoteConnectionRequestsControlMasterCleanupWithoutExplicitControlPath() { + let workspace = Workspace() + let config = WorkspaceRemoteConfiguration( + destination: "cmux-macmini", + port: nil, + identityFile: nil, + sshOptions: [], + localProxyPort: nil, + relayPort: 64015, + relayID: String(repeating: "a", count: 16), + relayToken: String(repeating: "b", count: 64), + localSocketPath: "/tmp/cmux-debug-test.sock", + terminalStartupCommand: "ssh cmux-macmini" + ) + let cleanupRequested = expectation(description: "control master cleanup requested") + var capturedArguments: [String] = [] + + Workspace.runSSHControlMasterCommandOverrideForTesting = { arguments in + capturedArguments = arguments + cleanupRequested.fulfill() + } + defer { Workspace.runSSHControlMasterCommandOverrideForTesting = nil } + + workspace.configureRemoteConnection(config, autoConnect: false) + workspace.applyRemoteConnectionStateUpdate( + .connecting, + detail: "Connecting to cmux-macmini", + target: "cmux-macmini" + ) + + workspace.teardownRemoteConnection() + + wait(for: [cleanupRequested], timeout: 1.0) + + XCTAssertFalse(workspace.isRemoteWorkspace) + XCTAssertEqual( + capturedArguments, + [ + "-o", "BatchMode=yes", + "-o", "ControlMaster=no", + "-O", "exit", + "cmux-macmini", + ] + ) + } + + @MainActor + func testDetachLastRemoteSurfacePreservesRemoteSessionWithoutCleanup() throws { + let workspace = Workspace() + let config = WorkspaceRemoteConfiguration( + destination: "cmux-macmini", + port: nil, + identityFile: nil, + sshOptions: [ + "ControlMaster=auto", + "ControlPersist=600", + "ControlPath=/tmp/cmux-ssh-%C", + ], + localProxyPort: nil, + relayPort: 64016, + relayID: String(repeating: "a", count: 16), + relayToken: String(repeating: "b", count: 64), + localSocketPath: "/tmp/cmux-debug-test.sock", + terminalStartupCommand: "ssh cmux-macmini" + ) + let cleanupRequested = expectation(description: "control master cleanup requested") + cleanupRequested.isInverted = true + + Workspace.runSSHControlMasterCommandOverrideForTesting = { _ in + cleanupRequested.fulfill() + } + defer { Workspace.runSSHControlMasterCommandOverrideForTesting = nil } + + workspace.configureRemoteConnection(config, autoConnect: false) + + let paneID = try XCTUnwrap(workspace.bonsplitController.allPaneIds.first) + let panelID = try XCTUnwrap(workspace.focusedTerminalPanel?.id) + let detached = try XCTUnwrap(workspace.detachSurface(panelId: panelID)) + + wait(for: [cleanupRequested], timeout: 0.2) + + XCTAssertTrue(detached.isRemoteTerminal) + XCTAssertTrue(workspace.isRemoteWorkspace) + XCTAssertEqual(workspace.activeRemoteTerminalSessionCount, 0) + + let reattachedSurfaceID = workspace.attachDetachedSurface(detached, inPane: paneID, focus: false) + + XCTAssertNotNil(reattachedSurfaceID) + XCTAssertTrue(workspace.isRemoteWorkspace) + XCTAssertEqual(workspace.activeRemoteTerminalSessionCount, 1) + XCTAssertTrue(workspace.isRemoteTerminalSurface(detached.panelId)) + } + + @MainActor + func testRemoteTerminalSessionEndSkipsControlMasterCleanupWhenBrowserPanelsKeepWorkspaceRemote() throws { + let workspace = Workspace() + let paneID = try XCTUnwrap(workspace.bonsplitController.allPaneIds.first) + let initialTerminalID = try XCTUnwrap(workspace.focusedTerminalPanel?.id) + let config = WorkspaceRemoteConfiguration( + destination: "cmux-macmini", + port: nil, + identityFile: nil, + sshOptions: [ + "ControlMaster=auto", + "ControlPersist=600", + "ControlPath=/tmp/cmux-ssh-%C", + ], + localProxyPort: nil, + relayPort: 64013, + relayID: String(repeating: "a", count: 16), + relayToken: String(repeating: "b", count: 64), + localSocketPath: "/tmp/cmux-debug-test.sock", + terminalStartupCommand: "ssh cmux-macmini" + ) + let cleanupRequested = expectation(description: "control master cleanup requested") + cleanupRequested.isInverted = true + + Workspace.runSSHControlMasterCommandOverrideForTesting = { _ in + cleanupRequested.fulfill() + } + defer { Workspace.runSSHControlMasterCommandOverrideForTesting = nil } + + workspace.configureRemoteConnection(config, autoConnect: false) + _ = workspace.newBrowserSurface(inPane: paneID, url: URL(string: "https://example.com"), focus: false) + + workspace.markRemoteTerminalSessionEnded(surfaceId: initialTerminalID, relayPort: 64013) + + wait(for: [cleanupRequested], timeout: 0.2) + + XCTAssertTrue(workspace.isRemoteWorkspace) + XCTAssertEqual(workspace.activeRemoteTerminalSessionCount, 0) + } + func testRemoteDropPathUsesLowercasedExtensionAndProvidedUUID() throws { let fileURL = URL(fileURLWithPath: "/Users/test/Screen Shot.PNG") let uuid = try XCTUnwrap(UUID(uuidString: "12345678-1234-1234-1234-1234567890AB"))