Skip to content

Commit a7e5050

Browse files
Fix SSH control master cleanup on remote teardown (#2104)
* test: add SSH control master cleanup regressions * fix: close SSH control master on remote teardown * test: keep SSH workspace after child exit * fix: keep SSH workspace after child exit * fix: keep connecting SSH workspaces after child exit * test: add SSH child-exit demotion regression * fix: keep SSH workspace after connected shell exit * fix: address SSH cleanup review feedback * test: cover SSH cleanup without explicit controlpath * fix: clean up SSH control masters without explicit controlpath * test: cover remote detach cleanup edge cases * fix: preserve SSH sessions during remote detach --------- Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
1 parent 7cbd07e commit a7e5050

4 files changed

Lines changed: 433 additions & 2 deletions

File tree

Sources/TabManager.swift

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2851,14 +2851,27 @@ class TabManager: ObservableObject {
28512851
func closePanelAfterChildExited(tabId: UUID, surfaceId: UUID) {
28522852
guard let tab = tabs.first(where: { $0.id == tabId }) else { return }
28532853
guard tab.panels[surfaceId] != nil else { return }
2854+
let keepsRemoteWorkspaceOpen =
2855+
tab.panels.count <= 1 && tab.shouldDemoteWorkspaceAfterChildExit(surfaceId: surfaceId)
28542856

28552857
#if DEBUG
28562858
dlog(
28572859
"surface.close.childExited tab=\(tabId.uuidString.prefix(5)) " +
2858-
"surface=\(surfaceId.uuidString.prefix(5)) panels=\(tab.panels.count) workspaces=\(tabs.count)"
2860+
"surface=\(surfaceId.uuidString.prefix(5)) panels=\(tab.panels.count) workspaces=\(tabs.count) " +
2861+
"remoteWorkspace=\(tab.isRemoteWorkspace ? 1 : 0) keepRemote=\(keepsRemoteWorkspaceOpen ? 1 : 0)"
28592862
)
28602863
#endif
28612864

2865+
// Exiting the last SSH surface should demote the workspace back to a local one.
2866+
// Route through Workspace close handling so remote teardown and replacement-panel
2867+
// logic run before TabManager considers removing the workspace itself, including
2868+
// session-end paths where remote configuration was cleared before Ghostty delivered
2869+
// the child-exit callback.
2870+
if keepsRemoteWorkspaceOpen {
2871+
closeRuntimeSurface(tabId: tabId, surfaceId: surfaceId)
2872+
return
2873+
}
2874+
28622875
// Child-exit on the last panel should collapse the workspace, matching explicit close
28632876
// semantics (and close the window when it was the last workspace).
28642877
if tab.panels.count <= 1 {

Sources/Workspace.swift

Lines changed: 97 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5469,14 +5469,20 @@ final class Workspace: Identifiable, ObservableObject {
54695469
private var remoteLastDaemonErrorFingerprint: String?
54705470
private var remoteLastPortConflictFingerprint: String?
54715471
private var activeRemoteTerminalSurfaceIds: Set<UUID> = []
5472+
private var pendingRemoteTerminalChildExitSurfaceIds: Set<UUID> = []
54725473

54735474
private static let remoteErrorStatusKey = "remote.error"
54745475
private static let remotePortConflictStatusKey = "remote.port_conflicts"
5476+
private static let sshControlMasterCleanupQueue = DispatchQueue(
5477+
label: "com.cmux.remote-ssh.control-master-cleanup",
5478+
qos: .utility
5479+
)
54755480
private static let remoteHeartbeatDateFormatter: ISO8601DateFormatter = {
54765481
let formatter = ISO8601DateFormatter()
54775482
formatter.formatOptions = [.withInternetDateTime, .withFractionalSeconds]
54785483
return formatter
54795484
}()
5485+
nonisolated(unsafe) static var runSSHControlMasterCommandOverrideForTesting: (([String]) -> Void)?
54805486
private var panelShellActivityStates: [UUID: PanelShellActivityState] = [:]
54815487
/// PIDs associated with agent status entries (e.g. claude_code), keyed by status key.
54825488
/// Used for stale-session detection: if the PID is dead, the status entry is cleared.
@@ -6670,6 +6676,11 @@ final class Workspace: Identifiable, ObservableObject {
66706676
activeRemoteTerminalSurfaceIds.contains(panelId)
66716677
}
66726678

6679+
@MainActor
6680+
func shouldDemoteWorkspaceAfterChildExit(surfaceId: UUID) -> Bool {
6681+
isRemoteWorkspace || pendingRemoteTerminalChildExitSurfaceIds.contains(surfaceId)
6682+
}
6683+
66736684
var remoteDisplayTarget: String? {
66746685
remoteConfiguration?.displayTarget
66756686
}
@@ -6825,6 +6836,9 @@ final class Workspace: Identifiable, ObservableObject {
68256836
}
68266837

68276838
func disconnectRemoteConnection(clearConfiguration: Bool = false) {
6839+
let shouldCleanupControlMaster =
6840+
clearConfiguration && !isDetachingCloseTransaction && pendingDetachedSurfaces.isEmpty
6841+
let configurationForCleanup = shouldCleanupControlMaster ? remoteConfiguration : nil
68286842
let previousController = remoteSessionController
68296843
activeRemoteSessionControllerID = nil
68306844
remoteSessionController = nil
@@ -6851,10 +6865,13 @@ final class Workspace: Identifiable, ObservableObject {
68516865
applyRemoteProxyEndpointUpdate(nil)
68526866
applyBrowserRemoteWorkspaceStatusToPanels()
68536867
recomputeListeningPorts()
6868+
if let configurationForCleanup {
6869+
Self.requestSSHControlMasterCleanupIfNeeded(configuration: configurationForCleanup)
6870+
}
68546871
}
68556872

68566873
private func clearRemoteConfigurationIfWorkspaceBecameLocal() {
6857-
guard panels.isEmpty, remoteConfiguration != nil else { return }
6874+
guard !isDetachingCloseTransaction, panels.isEmpty, remoteConfiguration != nil else { return }
68586875
disconnectRemoteConnection(clearConfiguration: true)
68596876
}
68606877

@@ -6871,13 +6888,15 @@ final class Workspace: Identifiable, ObservableObject {
68716888
}
68726889

68736890
private func trackRemoteTerminalSurface(_ panelId: UUID) {
6891+
pendingRemoteTerminalChildExitSurfaceIds.remove(panelId)
68746892
guard activeRemoteTerminalSurfaceIds.insert(panelId).inserted else { return }
68756893
activeRemoteTerminalSessionCount = activeRemoteTerminalSurfaceIds.count
68766894
}
68776895

68786896
private func untrackRemoteTerminalSurface(_ panelId: UUID) {
68796897
guard activeRemoteTerminalSurfaceIds.remove(panelId) != nil else { return }
68806898
activeRemoteTerminalSessionCount = activeRemoteTerminalSurfaceIds.count
6899+
guard !isDetachingCloseTransaction else { return }
68816900
maybeDemoteRemoteWorkspaceAfterSSHSessionEnded()
68826901
}
68836902

@@ -6898,13 +6917,87 @@ final class Workspace: Identifiable, ObservableObject {
68986917
remoteConfiguration?.relayPort == relayPort else {
68996918
return
69006919
}
6920+
pendingRemoteTerminalChildExitSurfaceIds.insert(surfaceId)
69016921
untrackRemoteTerminalSurface(surfaceId)
69026922
}
69036923

69046924
func teardownRemoteConnection() {
69056925
disconnectRemoteConnection(clearConfiguration: true)
69066926
}
69076927

6928+
private static func requestSSHControlMasterCleanupIfNeeded(configuration: WorkspaceRemoteConfiguration) {
6929+
guard let arguments = sshControlMasterCleanupArguments(configuration: configuration) else { return }
6930+
if let override = runSSHControlMasterCommandOverrideForTesting {
6931+
override(arguments)
6932+
return
6933+
}
6934+
6935+
sshControlMasterCleanupQueue.async {
6936+
let process = Process()
6937+
process.executableURL = URL(fileURLWithPath: "/usr/bin/ssh")
6938+
process.arguments = arguments
6939+
process.standardInput = FileHandle.nullDevice
6940+
process.standardOutput = FileHandle.nullDevice
6941+
process.standardError = FileHandle.nullDevice
6942+
let exitSemaphore = DispatchSemaphore(value: 0)
6943+
process.terminationHandler = { _ in
6944+
exitSemaphore.signal()
6945+
}
6946+
6947+
do {
6948+
try process.run()
6949+
if exitSemaphore.wait(timeout: .now() + 5) == .timedOut {
6950+
if process.isRunning {
6951+
process.terminate()
6952+
}
6953+
_ = exitSemaphore.wait(timeout: .now() + 1)
6954+
}
6955+
} catch {
6956+
return
6957+
}
6958+
}
6959+
}
6960+
6961+
private static func sshControlMasterCleanupArguments(configuration: WorkspaceRemoteConfiguration) -> [String]? {
6962+
let sshOptions = normalizedSSHControlCleanupOptions(configuration.sshOptions)
6963+
var arguments: [String] = [
6964+
"-o", "BatchMode=yes",
6965+
"-o", "ControlMaster=no",
6966+
]
6967+
if let port = configuration.port {
6968+
arguments += ["-p", String(port)]
6969+
}
6970+
if let identityFile = configuration.identityFile?.trimmingCharacters(in: .whitespacesAndNewlines),
6971+
!identityFile.isEmpty {
6972+
arguments += ["-i", identityFile]
6973+
}
6974+
for option in sshOptions {
6975+
arguments += ["-o", option]
6976+
}
6977+
arguments += ["-O", "exit", configuration.destination]
6978+
return arguments
6979+
}
6980+
6981+
private static func normalizedSSHControlCleanupOptions(_ options: [String]) -> [String] {
6982+
let disallowedKeys: Set<String> = ["controlmaster", "controlpersist"]
6983+
return options.compactMap { option in
6984+
let trimmed = option.trimmingCharacters(in: .whitespacesAndNewlines)
6985+
guard !trimmed.isEmpty else { return nil }
6986+
guard let key = sshOptionKeyForControlCleanup(trimmed) else { return nil }
6987+
return disallowedKeys.contains(key) ? nil : trimmed
6988+
}
6989+
}
6990+
6991+
private static func sshOptionKeyForControlCleanup(_ option: String) -> String? {
6992+
let trimmed = option.trimmingCharacters(in: .whitespacesAndNewlines)
6993+
guard !trimmed.isEmpty else { return nil }
6994+
return trimmed
6995+
.split(whereSeparator: { $0 == "=" || $0.isWhitespace })
6996+
.first
6997+
.map(String.init)?
6998+
.lowercased()
6999+
}
7000+
69087001
func applyRemoteConnectionStateUpdate(
69097002
_ state: WorkspaceRemoteConnectionState,
69107003
detail: String?,
@@ -7688,6 +7781,7 @@ final class Workspace: Identifiable, ObservableObject {
76887781
panels.removeAll(keepingCapacity: false)
76897782
surfaceIdToPanelId.removeAll(keepingCapacity: false)
76907783
panelSubscriptions.removeAll(keepingCapacity: false)
7784+
pendingRemoteTerminalChildExitSurfaceIds.removeAll(keepingCapacity: false)
76917785
pruneSurfaceMetadata(validSurfaceIds: [])
76927786
restoredTerminalScrollbackByPanelId.removeAll(keepingCapacity: false)
76937787
terminalInheritanceFontPointsByPanelId.removeAll(keepingCapacity: false)
@@ -10087,6 +10181,7 @@ extension Workspace: BonsplitDelegate {
1008710181

1008810182
panels.removeValue(forKey: panelId)
1008910183
untrackRemoteTerminalSurface(panelId)
10184+
pendingRemoteTerminalChildExitSurfaceIds.remove(panelId)
1009010185
surfaceIdToPanelId.removeValue(forKey: tabId)
1009110186
panelDirectories.removeValue(forKey: panelId)
1009210187
panelGitBranches.removeValue(forKey: panelId)
@@ -10237,6 +10332,7 @@ extension Workspace: BonsplitDelegate {
1023710332
panels[panelId]?.close()
1023810333
panels.removeValue(forKey: panelId)
1023910334
untrackRemoteTerminalSurface(panelId)
10335+
pendingRemoteTerminalChildExitSurfaceIds.remove(panelId)
1024010336
panelDirectories.removeValue(forKey: panelId)
1024110337
panelGitBranches.removeValue(forKey: panelId)
1024210338
panelPullRequests.removeValue(forKey: panelId)

cmuxTests/TabManagerUnitTests.swift

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,89 @@ final class TabManagerChildExitCloseTests: XCTestCase {
162162
)
163163
}
164164

165+
func testChildExitOnLastRemotePanelKeepsWorkspaceAndDemotesToLocal() throws {
166+
let manager = TabManager()
167+
guard let workspace = manager.selectedWorkspace,
168+
let remotePanelId = workspace.focusedPanelId else {
169+
XCTFail("Expected selected workspace with focused panel")
170+
return
171+
}
172+
173+
workspace.configureRemoteConnection(
174+
WorkspaceRemoteConfiguration(
175+
destination: "cmux-macmini",
176+
port: nil,
177+
identityFile: nil,
178+
sshOptions: [],
179+
localProxyPort: nil,
180+
relayPort: 64015,
181+
relayID: String(repeating: "a", count: 16),
182+
relayToken: String(repeating: "b", count: 64),
183+
localSocketPath: "/tmp/cmux-debug-test.sock",
184+
terminalStartupCommand: "ssh cmux-macmini"
185+
),
186+
autoConnect: false
187+
)
188+
189+
XCTAssertTrue(workspace.isRemoteWorkspace)
190+
XCTAssertTrue(workspace.isRemoteTerminalSurface(remotePanelId))
191+
192+
manager.closePanelAfterChildExited(tabId: workspace.id, surfaceId: remotePanelId)
193+
drainMainQueue()
194+
drainMainQueue()
195+
196+
XCTAssertEqual(manager.tabs.count, 1)
197+
XCTAssertEqual(manager.selectedTabId, workspace.id)
198+
XCTAssertEqual(manager.tabs.first?.id, workspace.id)
199+
XCTAssertFalse(workspace.isRemoteWorkspace)
200+
XCTAssertNil(workspace.panels[remotePanelId])
201+
XCTAssertEqual(workspace.panels.count, 1)
202+
XCTAssertNotEqual(workspace.focusedPanelId, remotePanelId)
203+
XCTAssertEqual(workspace.activeRemoteTerminalSessionCount, 0)
204+
}
205+
206+
func testChildExitAfterRemoteSessionEndKeepsWorkspaceAndDemotesToLocal() throws {
207+
let manager = TabManager()
208+
guard let workspace = manager.selectedWorkspace,
209+
let remotePanelId = workspace.focusedPanelId else {
210+
XCTFail("Expected selected workspace with focused panel")
211+
return
212+
}
213+
214+
workspace.configureRemoteConnection(
215+
WorkspaceRemoteConfiguration(
216+
destination: "cmux-macmini",
217+
port: nil,
218+
identityFile: nil,
219+
sshOptions: [],
220+
localProxyPort: nil,
221+
relayPort: 64016,
222+
relayID: String(repeating: "a", count: 16),
223+
relayToken: String(repeating: "b", count: 64),
224+
localSocketPath: "/tmp/cmux-debug-test.sock",
225+
terminalStartupCommand: "ssh cmux-macmini"
226+
),
227+
autoConnect: false
228+
)
229+
230+
workspace.markRemoteTerminalSessionEnded(surfaceId: remotePanelId, relayPort: 64016)
231+
232+
XCTAssertFalse(workspace.isRemoteWorkspace)
233+
234+
manager.closePanelAfterChildExited(tabId: workspace.id, surfaceId: remotePanelId)
235+
drainMainQueue()
236+
drainMainQueue()
237+
238+
XCTAssertEqual(manager.tabs.count, 1)
239+
XCTAssertEqual(manager.selectedTabId, workspace.id)
240+
XCTAssertEqual(manager.tabs.first?.id, workspace.id)
241+
XCTAssertFalse(workspace.isRemoteWorkspace)
242+
XCTAssertNil(workspace.panels[remotePanelId])
243+
XCTAssertEqual(workspace.panels.count, 1)
244+
XCTAssertNotEqual(workspace.focusedPanelId, remotePanelId)
245+
XCTAssertEqual(workspace.activeRemoteTerminalSessionCount, 0)
246+
}
247+
165248
func testChildExitOnNonLastPanelClosesOnlyPanel() {
166249
let manager = TabManager()
167250
guard let workspace = manager.selectedWorkspace,

0 commit comments

Comments
 (0)