marketing-shibata50/cmux
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix SSH control master cleanup on remote teardown (#2104)

Browse source 
* 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>
This commit is contained in:
Lawrence Chen 2026-03-25 00:01:39 -07:00 • committed by GitHub
parent 7cbd07e8cb
commit a7e5050552
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 433 additions and 2 deletions
Download patch file Download diff file Expand all files Collapse all files

15
Sources/TabManager.swift
View file

@ -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 {

98
Sources/Workspace.swift
View file

@ -5469,14 +5469,20 @@ final class Workspace: Identifiable, ObservableObject {
private var remoteLastDaemonErrorFingerprint: String?
private var remoteLastPortConflictFingerprint: String?
private var activeRemoteTerminalSurfaceIds: Set<UUID> = []
private var pendingRemoteTerminalChildExitSurfaceIds: Set<UUID> = []
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<String> = ["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)

83
cmuxTests/TabManagerUnitTests.swift
View file

@ -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,

239
cmuxTests/WorkspaceRemoteConnectionTests.swift
View file

@ -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"))