Skip to content

Coalesce and retry SSH PTY resize delivery (#6306) #6320

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
austinywang wants to merge 9 commits into from
Closed

Coalesce and retry SSH PTY resize delivery (#6306) #6320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b6a9a7f
Coalesce and retry SSH PTY resize delivery
austinywang Jun 17, 2026
24efe76
Add SSH PTY resize retry regression test
austinywang Jun 17, 2026
a3639e0
Move resize retry test to its own file for file-length budget
austinywang Jun 17, 2026
ead1e17
Merge remote-tracking branch 'origin/main' into issue-6306
austinywang Jun 17, 2026
9bd29ba
Tie SSH PTY resize retries to attach lifecycle (cancel safely)
austinywang Jun 17, 2026
1ba42af
Trim comment to keep cmux.swift within file-length budget
austinywang Jun 17, 2026
35c948a
Only retry SSH PTY resize after in-sync protocol rejections
austinywang Jun 17, 2026
8aa33ef
Move SSHPTYResizeCoordinator into its own file
austinywang Jun 17, 2026
a929814
Merge remote-tracking branch 'origin/main' into issue-6306
austinywang Jun 17, 2026
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
257 changes: 257 additions & 0 deletions CLI/SSHPTYResizeCoordinator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,257 @@
import Foundation

/// A resize send failed at the transport layer (timeout / socket error) before a
/// complete response was read. The shared control socket may now hold a pending
/// late reply, so the resize must not be retried on it. Terminal for the
/// coordinator's retry loop.
struct SSHPTYResizeTransportError: Error {}

/// The remote/app rejected the resize with a complete response (`ok:false` or an
/// unparseable but fully-consumed line). The socket is in sync, so the retry loop
/// may safely re-send the latest size — this is the stale/blocked remote path
/// from issue #6306.
struct SSHPTYResizeProtocolRejection: Error {
let response: String
}

/// Coalesces and retries SSH-PTY resize delivery (`workspace.remote.pty_resize`).
///
/// A SIGWINCH burst during a split/divider drag would previously fire one
/// best-effort `try?` send per event, silently dropping any send that raced a
/// stale or blocked remote-session control path. Output kept flowing, so the
/// terminal looked alive while the remote PTY/TUI never received the new size —
/// and only a manual workspace reconnect restored it (issue #6306).
///
/// This coordinator collapses rapid resize events into a single delivery of the
/// newest size and, when a delivery fails, retries the latest size with a
/// bounded backoff instead of dropping it.
///
/// The coalescing/retry state (`pendingSize`, `lastDeliveredSize`,
/// `deliveryScheduled`, `retryCount`) is touched only from
/// `noteResize`/`deliver`, which in production run on a single serial dispatch
/// queue (the signal source's target queue), so that state needs no locking.
///
/// `cancel()` is the exception: it must be callable synchronously from the
/// attach teardown thread (bridge EOF / `defer`) so an already-scheduled retry
/// cannot fire after teardown and send a `workspace.remote.pty_resize` on the
/// shared socket once the session is gone. The `cancelled` flag is therefore
/// guarded by `stateLock` and observed by every scheduling/delivery decision; a
/// retry block that has not yet started bails as soon as `cancel()` returns.
///
/// The `scheduleAfter` and `send` seams are injected so the coalescing/retry
/// state machine can be driven deterministically in tests without real time or
/// a live socket.
final class SSHPTYResizeCoordinator {
/// Schedules `block` to run after `delay`. Production wires this to the
/// signal source's serial queue via `asyncAfter`.
typealias Scheduler = (_ delay: DispatchTimeInterval, _ block: @escaping () -> Void) -> Void

private let scheduleAfter: Scheduler
private let send: (_ cols: Int, _ rows: Int) throws -> Void
private let sizeProvider: () -> (cols: Int, rows: Int)
private let log: (String) -> Void

private var pendingSize: (cols: Int, rows: Int)?
private var lastDeliveredSize: (cols: Int, rows: Int)?
private var deliveryScheduled = false
private var retryCount = 0

// `cancelled` and `signalSource` are reachable from both the serial queue
// and the teardown thread, so they are guarded by `stateLock`.
private let stateLock = NSLock()
private var cancelled = false
private var signalSource: DispatchSourceSignal?

let coalesceDelay: DispatchTimeInterval
let maxRetries: Int

init(
sizeProvider: @escaping () -> (cols: Int, rows: Int),
send: @escaping (_ cols: Int, _ rows: Int) throws -> Void,
scheduleAfter: @escaping Scheduler,
log: @escaping (String) -> Void = { _ in },
coalesceDelay: DispatchTimeInterval = .milliseconds(20),
maxRetries: Int = 6
) {
self.sizeProvider = sizeProvider
self.send = send
self.scheduleAfter = scheduleAfter
self.log = log
self.coalesceDelay = coalesceDelay
self.maxRetries = maxRetries
}

/// Production convenience initializer wiring the send to a `SocketClient`
/// (serialized by `socketLock`) and scheduling on `queue`.
///
/// The send is issued at the raw `send(command:)` layer rather than through
/// `sendV2` so the coordinator can tell a clean protocol rejection apart from
/// a transport failure. `send(command:)` only returns once it has consumed a
/// complete response line, so a thrown error (timeout / socket error) means
/// the shared control socket may still have a pending late reply. Retrying on
/// that socket could misattribute the late reply to a later request (the
/// control protocol does not match response ids), so transport failures are
/// surfaced as `SSHPTYResizeTransportError` and not retried. A returned line
/// means the socket is back in sync, so an `ok:false` rejection is safe to
/// retry — which is the stale/blocked remote path from issue #6306.
convenience init(
client: SocketClient,
baseParams: [String: Any],
socketLock: NSLock,
queue: DispatchQueue,
sizeProvider: @escaping () -> (cols: Int, rows: Int),
log: @escaping (String) -> Void
) {
self.init(
sizeProvider: sizeProvider,
send: { cols, rows in
var params = baseParams
params["cols"] = cols
params["rows"] = rows
let request: [String: Any] = [
"id": UUID().uuidString,
"method": "workspace.remote.pty_resize",
"params": params,
]
guard let requestData = try? JSONSerialization.data(withJSONObject: request),
let requestLine = String(data: requestData, encoding: .utf8) else {
// Encoding can't fail for our own payload; if it somehow
// does, the socket was never touched — terminal, no retry.
throw SSHPTYResizeTransportError()
}

let raw: String
socketLock.lock()
do {
raw = try client.send(command: requestLine)
socketLock.unlock()
} catch {
socketLock.unlock()
throw SSHPTYResizeTransportError()
}

// A complete response line was consumed, so the socket is in
// sync. ok:true → delivered; anything else is a protocol-level
// rejection that is safe to retry on the same socket.
if let data = raw.data(using: .utf8),
let response = try? JSONSerialization.jsonObject(with: data) as? [String: Any],
let ok = response["ok"] as? Bool, ok {
return
}
throw SSHPTYResizeProtocolRejection(response: raw)
},
scheduleAfter: { delay, block in
queue.asyncAfter(deadline: .now() + delay, execute: block)
},
log: log
)
}

/// Owns the SIGWINCH source so `cancel()` can tear it down synchronously.
/// Wired once by `startSSHPTYResizeSource` right after the source is built.
func bindSignalSource(_ source: DispatchSourceSignal) {
stateLock.lock()
let alreadyCancelled = cancelled
if !alreadyCancelled {
signalSource = source
}
stateLock.unlock()
// If cancellation already raced ahead of binding, don't keep a live
// source around.
if alreadyCancelled {
source.cancel()
}
}

private var isCancelled: Bool {
stateLock.lock()
defer { stateLock.unlock() }
return cancelled
}

/// Record a new terminal size and schedule a coalesced delivery.
/// Invoked from the signal source event handler, which runs on the queue.
func noteResize() {
guard !isCancelled else { return }
let size = sizeProvider()
guard size.cols > 0, size.rows > 0 else { return }
pendingSize = size
// A fresh, user-driven resize resets the retry budget so we keep trying
// to deliver the newest size even after a prior burst gave up.
retryCount = 0
scheduleDelivery(after: coalesceDelay)
}

/// Stop further delivery. Safe to call synchronously from any thread; once it
/// returns, no not-yet-started retry will send another resize. Idempotent.
func cancel() {
stateLock.lock()
let already = cancelled
cancelled = true
let source = signalSource
signalSource = nil
stateLock.unlock()
if !already {
source?.cancel()
}
}

private func scheduleDelivery(after delay: DispatchTimeInterval) {
guard !isCancelled, !deliveryScheduled else { return }
deliveryScheduled = true
scheduleAfter(delay) { [weak self] in
self?.deliver()
}
}

private func deliver() {
deliveryScheduled = false
guard !isCancelled, let size = pendingSize else { return }
// The newest size already reached the remote; nothing to do.
if let last = lastDeliveredSize, last == size {
pendingSize = nil
retryCount = 0
return
}

let delivered: Bool
do {
try send(size.cols, size.rows)
delivered = true
} catch is SSHPTYResizeTransportError {
// The control socket may be desynced after a transport failure;
// retrying could misread a late reply as a later request's result.
// Drop this attempt but keep pendingSize so a fresh SIGWINCH retries
// once the socket is healthy again.
log("ssh-pty resize transport failure (\(size.cols)x\(size.rows)); not retrying on possibly-desynced socket")
retryCount = 0
return
} catch {
delivered = false
log("ssh-pty resize delivery failed (\(size.cols)x\(size.rows), attempt \(retryCount + 1)): \(error)")
}

if delivered {
lastDeliveredSize = size
retryCount = 0
// A newer size may have arrived while the send was in flight.
if let newest = pendingSize, newest != size {
scheduleDelivery(after: coalesceDelay)
} else {
pendingSize = nil
}
return
}

// Retry the latest size with a bounded exponential backoff. Keeping
// pendingSize set means a give-up still recovers on the next SIGWINCH.
if retryCount < maxRetries {
retryCount += 1
let backoffMs = min(1000, 50 * (1 << min(retryCount - 1, 4)))
scheduleDelivery(after: .milliseconds(backoffMs))
} else {
log("ssh-pty resize giving up after \(maxRetries) attempts (\(size.cols)x\(size.rows)); awaiting next resize")
retryCount = 0
}
}
}
54 changes: 27 additions & 27 deletions CLI/cmux.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11610,7 +11610,7 @@ struct CMUXCLI {

let rawMode = TerminalRawMode()
defer { rawMode?.restore() }
let resizeSource = startSSHPTYResizeSource(
let resizeCoordinator = startSSHPTYResizeSource(
client: client,
workspaceId: workspaceId,
surfaceID: surfaceID,
Expand All @@ -11619,7 +11619,7 @@ struct CMUXCLI {
attachmentToken: attachmentToken,
socketLock: controlSocketLock
)
defer { resizeSource.cancel() }
defer { resizeCoordinator.cancel() }

DispatchQueue.global(qos: .userInteractive).async {
var buffer = [UInt8](repeating: 0, count: 8192)
Expand Down Expand Up @@ -11648,7 +11648,7 @@ struct CMUXCLI {
if count > 0 {
FileHandle.standardOutput.write(Data(outputBuffer.prefix(count)))
} else if count == 0 {
resizeSource.cancel()
resizeCoordinator.cancel()
try handleSSHPTYBridgeEOF(
client: client,
workspaceId: workspaceId,
Expand All @@ -11661,7 +11661,7 @@ struct CMUXCLI {
return
} else if errno != EINTR {
if sshPTYBridgeReadErrorIsEOF(errno) {
resizeSource.cancel()
resizeCoordinator.cancel()
try handleSSHPTYBridgeEOF(
client: client,
workspaceId: workspaceId,
Expand Down Expand Up @@ -11906,32 +11906,32 @@ struct CMUXCLI {
attachmentID: String,
attachmentToken: String,
socketLock: NSLock
) -> DispatchSourceSignal {
) -> SSHPTYResizeCoordinator {
signal(SIGWINCH, SIG_IGN)
let source = DispatchSource.makeSignalSource(
signal: SIGWINCH,
queue: DispatchQueue(label: "com.cmux.ssh-pty.resize")
)
source.setEventHandler {
let size = self.currentCLITerminalSize()
socketLock.lock()
defer { socketLock.unlock() }
var params: [String: Any] = [
"workspace_id": workspaceId,
"session_id": sessionID,
"attachment_id": attachmentID,
"attachment_token": attachmentToken,
"cols": size.cols,
"rows": size.rows,
]
if let surfaceID {
params["surface_id"] = surfaceID
params["allow_moved_surface"] = true
}
_ = try? client.sendV2(method: "workspace.remote.pty_resize", params: params)
let queue = DispatchQueue(label: "com.cmux.ssh-pty.resize")
var baseParams: [String: Any] = [
"workspace_id": workspaceId,
"session_id": sessionID,
"attachment_id": attachmentID,
"attachment_token": attachmentToken,
]
if let surfaceID {
baseParams["surface_id"] = surfaceID
baseParams["allow_moved_surface"] = true
}
let coordinator = SSHPTYResizeCoordinator(
client: client,
baseParams: baseParams,
socketLock: socketLock,
queue: queue,
sizeProvider: { self.currentCLITerminalSize() },
log: { self.cliDebugLog($0) }
)
let source = DispatchSource.makeSignalSource(signal: SIGWINCH, queue: queue)
source.setEventHandler { coordinator.noteResize() }
coordinator.bindSignalSource(source) // lets cancel() tear down retries synchronously
source.resume()
return source
return coordinator
}

private func connectLoopbackTCP(host: String, port: Int) throws -> Int32 {
Expand Down
8 changes: 8 additions & 0 deletions cmux.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -768,6 +768,8 @@
C510C1E00000000000000002 /* SocketOperationTelemetry.swift in Sources */ = {isa = PBXBuildFile; fileRef = C510C1E00000000000000001 /* SocketOperationTelemetry.swift */; };
A5001230 /* Sparkle in Frameworks */ = {isa = PBXBuildFile; productRef = A5001231 /* Sparkle */; };
E30780000000000000000012 /* SSHPTYAttachStartupCommandBuilder.swift in Sources */ = {isa = PBXBuildFile; fileRef = E30780000000000000000011 /* SSHPTYAttachStartupCommandBuilder.swift */; };
A6306000000000000000C001 /* SSHPTYResizeCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = A6306000000000000000C002 /* SSHPTYResizeCoordinator.swift */; };
A6306000000000000000F003 /* SSHPTYResizeRetryIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A6306000000000000000F004 /* SSHPTYResizeRetryIntegrationTests.swift */; };
F6355600A1B2C3D4E5F60718 /* SSHStartupSignalLifecycleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F6355601A1B2C3D4E5F60718 /* SSHStartupSignalLifecycleTests.swift */; };
F20F85FC5900550685FA33AD /* StackAuth in Frameworks */ = {isa = PBXBuildFile; productRef = A8BD195031FC4B82B4354297 /* StackAuth */; };
D35B71010000000000000001 /* StartupBreadcrumbLog.swift in Sources */ = {isa = PBXBuildFile; fileRef = D35B71010000000000000002 /* StartupBreadcrumbLog.swift */; };
Expand Down Expand Up @@ -1705,6 +1707,8 @@
F8000001A1B2C3D4E5F60718 /* SocketControlPasswordStoreTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SocketControlPasswordStoreTests.swift; sourceTree = "<group>"; };
C510C1E00000000000000001 /* SocketOperationTelemetry.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SocketOperationTelemetry.swift; sourceTree = "<group>"; };
E30780000000000000000011 /* SSHPTYAttachStartupCommandBuilder.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SSHPTYAttachStartupCommandBuilder.swift; sourceTree = "<group>"; };
A6306000000000000000C002 /* SSHPTYResizeCoordinator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SSHPTYResizeCoordinator.swift; sourceTree = "<group>"; };
A6306000000000000000F004 /* SSHPTYResizeRetryIntegrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SSHPTYResizeRetryIntegrationTests.swift; sourceTree = "<group>"; };
F6355601A1B2C3D4E5F60718 /* SSHStartupSignalLifecycleTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SSHStartupSignalLifecycleTests.swift; sourceTree = "<group>"; };
D35B71010000000000000002 /* StartupBreadcrumbLog.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = App/StartupBreadcrumbLog.swift; sourceTree = "<group>"; };
D7AB00000000000000B040 /* SupersededPhoneDismissBuffer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SupersededPhoneDismissBuffer.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2715,6 +2719,7 @@
C510C1E00000000000000001 /* SocketOperationTelemetry.swift */,
B9000030A1B2C3D4E5F60719 /* cmux_open.swift */,
B9000040A1B2C3D4E5F60719 /* CMUXCLI+SSHCommandSupport.swift */,
A6306000000000000000C002 /* SSHPTYResizeCoordinator.swift */,
D7AB0000000000000000000E /* CMUXCLI+MoveTabToNewWorkspace.swift */,
B9000047A1B2C3D4E5F60719 /* CMUXCLI+ExecutableResolution.swift */,
B9000045A1B2C3D4E5F60719 /* CMUXCLI+TmuxCompatSupport.swift */,
Expand Down Expand Up @@ -2802,6 +2807,7 @@
F6100001A1B2C3D4E5F60718 /* WorkspaceRemoteConnectionTests.swift */,
F6357301A1B2C3D4E5F60718 /* WorkspaceRemoteReconnectPolicyTests.swift */,
F6355601A1B2C3D4E5F60718 /* SSHStartupSignalLifecycleTests.swift */,
A6306000000000000000F004 /* SSHPTYResizeRetryIntegrationTests.swift */,
F6120001A1B2C3D4E5F60718 /* WorkspaceSSHFishShellTests.swift */,
F7000001A1B2C3D4E5F60718 /* WorkspaceContentViewVisibilityTests.swift */,
F8000001A1B2C3D4E5F60718 /* SocketControlPasswordStoreTests.swift */,
Expand Down Expand Up @@ -4035,6 +4041,7 @@
B9000027A1B2C3D4E5F60719 /* RemoteRelayZshBootstrap.swift in Sources */,
5E2701020000000000000003 /* SentryEventScrubber.swift in Sources */,
C510C1E00000000000000002 /* SocketOperationTelemetry.swift in Sources */,
A6306000000000000000C001 /* SSHPTYResizeCoordinator.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand Down Expand Up @@ -4284,6 +4291,7 @@
C0DE56010000000000000001 /* SidebarWorkspaceSelectionAnchorPolicyTests.swift in Sources */,
62270F3DCECB4787D789CCE3 /* SidebarWorkspaceSnapshotRefreshPolicyTests.swift in Sources */,
F8000000A1B2C3D4E5F60718 /* SocketControlPasswordStoreTests.swift in Sources */,
A6306000000000000000F003 /* SSHPTYResizeRetryIntegrationTests.swift in Sources */,
F6355600A1B2C3D4E5F60718 /* SSHStartupSignalLifecycleTests.swift in Sources */,
2BB56A710BB1FC50367E5BCF /* TabManagerSessionSnapshotTests.swift in Sources */,
B6BF3DC98DB1495E57900199 /* TabManagerUnitTests.swift in Sources */,
Expand Down
Loading
Loading