Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 9 additions & 2 deletions CLI/CMUXCLI+SSHConnectionSharing.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ extension CMUXCLI {
func resolvedUserSSHControlOptions(for options: SSHCommandOptions) -> [String]? {
guard let output = resolvedSSHConfigurationOutput(for: options) else { return nil }
return SSHConnectionSharingOptions()
.userConfiguredControlOptions(fromSSHConfigOutput: output)
.userConfiguredControlOptions(
fromSSHConfigOutput: output,
explicitOptions: options.sshOptions
)
}

func resolvedCmuxControlPathOptions(for options: SSHCommandOptions) -> [String] {
Expand Down Expand Up @@ -38,9 +41,13 @@ extension CMUXCLI {

func resolvedSSHConfigurationResult(
for options: SSHCommandOptions,
timeout: TimeInterval = 2
timeout: TimeInterval = 2,
configurationFile: String? = nil
) -> CLIProcessResult {
var arguments = ["-G"]
if let configurationFile {
arguments += ["-F", configurationFile]
}
if let port = options.port {
arguments += ["-p", String(port)]
}
Expand Down
117 changes: 85 additions & 32 deletions CLI/cmux.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4721,6 +4721,9 @@ struct CMUXCLI {
if normalizedCommand == "surface", commandArgs.first?.lowercased() == "resume" {
return false
}
if Self.commandDefersSocketConnectionUntilRequest(command: command, commandArgs: commandArgs) {
return false
}
return true
}

Expand Down Expand Up @@ -5244,6 +5247,10 @@ struct CMUXCLI {
)
try validateWorkspaceLoadingCommandBeforeSocket(command: command, commandArgs: commandArgs)
var client = SocketClient(path: resolvedSocketPath)
let defersSocketConnection = Self.commandDefersSocketConnectionUntilRequest(
Comment on lines +5250 to +5253

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Window focus defeats deferral

For vm or cloud dev, layout, and env commands with a window ID, the shared pre-dispatch path sends window.focus before entering the command handler. That request connects and authenticates immediately, so local planning and validation no longer finish first. If the socket is unavailable and the local arguments are invalid, the user receives a transport failure instead of the intended validation error. Exclude these deferred commands from pre-dispatch focusing or move focusing after local validation.

command: command,
commandArgs: commandArgs
)
let cursorHookSocketTimeout: TimeInterval? = isCursorShellHookCommand ? 0.35 : nil
let cursorHookDeadline: Date? = isCursorShellHookCommand
? Date.now.addingTimeInterval(3.0)
Expand All @@ -5257,20 +5264,24 @@ struct CMUXCLI {
]
)
}
cliTelemetry.breadcrumb(
"socket.connect.attempt",
data: [
"command": command,
"path": resolvedSocketPath
]
)
if !defersSocketConnection {
cliTelemetry.breadcrumb(
"socket.connect.attempt",
data: [
"command": command,
"path": resolvedSocketPath
]
)
}
do {
if let cursorHookDeadline {
try client.connect(deadline: cursorHookDeadline)
} else {
try client.connect()
if !defersSocketConnection {
if let cursorHookDeadline {
try client.connect(deadline: cursorHookDeadline)
} else {
try client.connect()
}
cliTelemetry.breadcrumb("socket.connect.success", data: ["path": resolvedSocketPath])
}
cliTelemetry.breadcrumb("socket.connect.success", data: ["path": resolvedSocketPath])
} catch {
cliTelemetry.breadcrumb("socket.connect.failure", data: ["path": resolvedSocketPath])
cliTelemetry.captureError(stage: "socket_connect", error: error)
Expand Down Expand Up @@ -5312,13 +5323,21 @@ struct CMUXCLI {
}
defer { client.close() }

try authenticateClientIfNeeded(
client,
explicitPassword: socketPasswordArg,
socketPath: resolvedSocketPath,
responseTimeout: cursorHookSocketTimeout,
deadline: cursorHookDeadline
)
if defersSocketConnection {
// send/sendV2 connects and authenticates immediately before the first request.
client.configureAuthentication(password: SocketPasswordResolver.resolve(
explicit: socketPasswordArg,
socketPath: resolvedSocketPath
))
} else {
try authenticateClientIfNeeded(
client,
explicitPassword: socketPasswordArg,
socketPath: resolvedSocketPath,
responseTimeout: cursorHookSocketTimeout,
deadline: cursorHookDeadline
)
}

let idFormat = try resolvedIDFormat(jsonOutput: jsonOutput, raw: idFormatArg)
// Workspace inspection JSON is a scripting boundary: keep stable UUIDs
Expand Down Expand Up @@ -8137,6 +8156,16 @@ struct CMUXCLI {
return FileManager.default.fileExists(atPath: resolvePath(arg))
}

/// These VM handlers finish local planning and validation before their first request.
private static func commandDefersSocketConnectionUntilRequest(
command: String,
commandArgs: [String]
) -> Bool {
guard command == "vm" || command == "cloud",
let subcommand = commandArgs.first?.lowercased() else { return false }
return ["dev", "layout", "env"].contains(subcommand)
}

/// Returns whether a command can reach its own dispatch path without a live
/// implicit socket. Commands that launch cmux or only touch local state must
/// validate their arguments before discovery reports a transport failure.
Expand All @@ -8145,7 +8174,8 @@ struct CMUXCLI {
commandArgs: [String],
environment: [String: String]
) -> Bool {
if commandCanLaunchAppWhenSocketUnavailable(command) {
if commandCanLaunchAppWhenSocketUnavailable(command)
|| Self.commandDefersSocketConnectionUntilRequest(command: command, commandArgs: commandArgs) {
return true
}

Expand Down Expand Up @@ -11512,6 +11542,19 @@ struct CMUXCLI {
)
let resolvedUserSSHConfiguration =
configurationResult.status == 0 ? configurationResult.stdout : nil
let resolvedOpenSSHDefaults: String?
if configurationResult.status == 0 {
let defaultConfigurationResult = resolvedSSHConfigurationResult(
for: sshOptions,
timeout: configurationTimeout,
configurationFile: "/dev/null"
)
resolvedOpenSSHDefaults = defaultConfigurationResult.status == 0
? defaultConfigurationResult.stdout
: nil
} else {
resolvedOpenSSHDefaults = nil
}
let fallsBackToOpenSSHInteractiveSession =
usesImplicitManagedInteractiveShell && resolvedUserSSHConfiguration == nil
let effectiveTerminalTransport: WorkspaceRemoteTerminalTransport =
Expand All @@ -11526,7 +11569,11 @@ struct CMUXCLI {
sshOptions.sshOptions = sharingOptions.mergingDefaults(
into: inputSSHOptions.sshOptions,
userConfiguredControlOptions: resolvedUserSSHConfiguration.flatMap {
sharingOptions.userConfiguredControlOptions(fromSSHConfigOutput: $0)
sharingOptions.userConfiguredControlOptions(
fromSSHConfigOutput: $0,
baselineSSHConfigOutput: resolvedOpenSSHDefaults,
explicitOptions: inputSSHOptions.sshOptions
)
}
)
if resolvedUserSSHConfiguration != nil {
Expand Down Expand Up @@ -13535,10 +13582,19 @@ struct CMUXCLI {
retryLimit: Int,
retryDelaySeconds: Double
) -> String {
let retryText = String(
localized: "cli.vm.sshInfo.retry.status",
defaultValue: "Retrying in \(Self.retryDelayLabel(retryDelaySeconds)) (\(Self.retryAttemptLabel(attempt: attempt, retryLimit: retryLimit)))."
)
let retryAttempt = Self.retryAttemptLabel(attempt: attempt, retryLimit: retryLimit)
let retryText: String
if retryDelaySeconds <= 0 {
retryText = String(
localized: "cli.vm.sshInfo.retry.nowStatus",
defaultValue: "Retrying now (attempt \(retryAttempt))."
)
} else {
retryText = String(
localized: "cli.vm.sshInfo.retry.status",
defaultValue: "Retrying in \(Self.retryDelayLabel(retryDelaySeconds))s (attempt \(retryAttempt))."
)
}
let errorText = String(describing: error)
if Self.isLocalCloudVMServiceUnreachable(errorText),
let url = Self.firstHTTPURL(in: errorText) {
Expand All @@ -13564,19 +13620,16 @@ struct CMUXCLI {

private static func retryAttemptLabel(attempt: Int, retryLimit: Int) -> String {
if retryLimit >= 86_400 {
return "attempt \(attempt)"
return "\(attempt)"
}
return "attempt \(attempt)/\(retryLimit)"
return "\(attempt)/\(retryLimit)"
}

private static func retryDelayLabel(_ seconds: Double) -> String {
if seconds <= 0 {
return String(localized: "cli.vm.sshInfo.retry.now", defaultValue: "now")
}
if seconds.rounded(.towardZero) == seconds {
return "\(Int(seconds))s"
return "\(Int(seconds))"
}
return String(format: "%.1fs", seconds)
return String(format: "%.1f", seconds)
}

private static func isLocalCloudVMServiceUnreachable(_ message: String) -> Bool {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,14 @@ public struct SSHConnectionSharingOptions: Sendable {
/// Adds sharing defaults while honoring effective control settings from
/// the user's SSH configuration.
///
/// Explicit caller options retain highest precedence. When the caller did
/// not provide any control option and `ssh -G` reported non-default
/// control settings, those effective values are carried forward instead
/// of installing cmux's socket.
/// Explicit caller options retain highest precedence per key. Independently
/// configured host control settings fill the remaining keys instead of
/// installing cmux's socket.
///
/// - Parameters:
/// - options: Explicit OpenSSH `-o` values.
/// - userConfiguredControlOptions: Effective custom values parsed by
/// ``userConfiguredControlOptions(fromSSHConfigOutput:)``.
/// ``userConfiguredControlOptions(fromSSHConfigOutput:explicitOptions:)``.
/// - Returns: Effective explicit options for native SSH commands.
public func mergingDefaults(
into options: [String],
Expand Down Expand Up @@ -136,24 +135,57 @@ public struct SSHConnectionSharingOptions: Sendable {
/// cmux control options are added.
/// - Returns: Effective custom `-o` values, or `nil` for OpenSSH defaults.
public func userConfiguredControlOptions(fromSSHConfigOutput output: String) -> [String]? {
var values: [String: String] = [:]
for line in output.split(whereSeparator: \.isNewline) {
let parts = line.split(maxSplits: 1, whereSeparator: \.isWhitespace)
guard parts.count == 2 else { continue }
let key = parts[0].lowercased()
guard ["controlmaster", "controlpath", "controlpersist"].contains(key) else {
continue
}
values[key] = parts[1].trimmingCharacters(in: .whitespacesAndNewlines)
}
userConfiguredControlOptions(fromSSHConfigOutput: output, explicitOptions: [])
}

/// Parses resolved host control settings with the explicit caller options
/// that were included in the `ssh -G` invocation.
///
/// Explicit values do not prove host customization: OpenSSH includes them in
/// its output and normalizes `ControlPersist=0` to `yes`. A custom value on
/// another control key still preserves the host's full effective settings,
/// with explicit options retaining precedence when merged.
///
/// - Parameters:
/// - output: Effective configuration reported by OpenSSH.
/// - explicitOptions: Caller-provided `-o` values included in that output.
/// - Returns: Effective custom host control settings, or `nil` for defaults.
public func userConfiguredControlOptions(
fromSSHConfigOutput output: String,
explicitOptions: [String]
) -> [String]? {
userConfiguredControlOptions(
fromSSHConfigOutput: output,
baselineSSHConfigOutput: nil,
explicitOptions: explicitOptions
)
}

/// Parses resolved host control settings against OpenSSH's built-in
/// defaults. Comparing with a `-F /dev/null` baseline distinguishes an
/// explicit host `ControlMaster=no` from the ordinary default `false`.
public func userConfiguredControlOptions(
fromSSHConfigOutput output: String,
baselineSSHConfigOutput: String?,
explicitOptions: [String]
) -> [String]? {
let values = controlConfigurationValues(fromSSHConfigOutput: output)
let baselineValues = baselineSSHConfigOutput.map(controlConfigurationValues(fromSSHConfigOutput:))

// Keep the fallback explicitly disabled if an OpenSSH version omits default-valued keys.
let controlMaster = values["controlmaster"] ?? "false"
let controlPath = values["controlpath"] ?? "none"
let controlPersist = values["controlpersist"] ?? "no"
let hasCustomValue = !isDisabled(controlMaster)
|| controlPath.lowercased() != "none"
|| !["no", "false", "off", "0"].contains(controlPersist.lowercased())
let resolver = SSHAgentSocketResolver()
let defaultValues = baselineValues ?? [
"controlmaster": "false",
"controlpath": "none",
"controlpersist": "no",
]
let hasCustomValue = ["controlmaster", "controlpath", "controlpersist"].contains { key in
guard !resolver.hasOptionKey(explicitOptions, key: key) else { return false }
return values[key]?.lowercased() != defaultValues[key]?.lowercased()
}
guard hasCustomValue else { return nil }
return [
"ControlMaster=\(controlMaster)",
Expand All @@ -162,6 +194,20 @@ public struct SSHConnectionSharingOptions: Sendable {
]
}

private func controlConfigurationValues(fromSSHConfigOutput output: String) -> [String: String] {
var values: [String: String] = [:]
for line in output.split(whereSeparator: \.isNewline) {
let parts = line.split(maxSplits: 1, whereSeparator: \.isWhitespace)
guard parts.count == 2 else { continue }
let key = parts[0].lowercased()
guard ["controlmaster", "controlpath", "controlpersist"].contains(key) else {
continue
}
values[key] = parts[1].trimmingCharacters(in: .whitespacesAndNewlines)
}
return values
}

/// Returns the configured `ControlPath` when it is one of cmux's native
/// SSH templates, including the older relay-port-scoped template so an
/// upgraded app can still clean up a socket it created.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,39 @@ struct SSHConnectionSharingOptionsTests {
).contains("ControlPath=/tmp/cmux-ssh-501-%C"))
}

@Test("An explicit host opt-out differs from OpenSSH defaults")
func detectsExplicitHostOptOutAgainstBaseline() {
let output = """
controlmaster false
controlpath none
controlpersist no
"""
let baseline = """
controlmaster false
controlpath none
controlpersist no
"""
#expect(options.userConfiguredControlOptions(
fromSSHConfigOutput: output,
baselineSSHConfigOutput: baseline,
explicitOptions: []
) == nil)
let configured = """
controlmaster no
controlpath none
controlpersist no
"""
#expect(options.userConfiguredControlOptions(
fromSSHConfigOutput: configured,
baselineSSHConfigOutput: baseline,
explicitOptions: []
) == [
"ControlMaster=no",
"ControlPath=none",
"ControlPersist=no",
])
}

@Test("Explicit CLI control options win per key over resolved ssh_config settings")
func explicitOptionsWinOverResolvedConfiguration() {
let configured = [
Expand Down
Loading
Loading