Repository navigation
Fix stale remote connected state after proxy disconnect - #4513
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds WebSocket keepalives at CLI and RPC levels, a reconnect-capable terminal placeholder on remote-session end, preserves remote workspace state as disconnected (not local), updates UI/localization from “SSH” → “Remote”, and extends tests to validate the new disconnect/reconnect flows. ChangesRemote Disconnect Detection and Reconnect Flow
Sequence DiagramsequenceDiagram
participant UI as ContentView/Sidebar
participant RPC as WorkspaceRemoteDaemonRPCClient
participant CLI as VMPtyWebSocketBridge
participant WS as URLSessionWebSocketTask
UI->>RPC: observe remoteStatusPayload
RPC->>WS: open transport
RPC->>RPC: start RPC keepalive timer
CLI->>CLI: start bridge keepalive timer (on receive loop)
loop periodic keepalive
CLI->>WS: send ping
alt pong received
WS-->>CLI: pong
CLI-->>RPC: confirm alive
else timeout or error
CLI->>WS: cancel WebSocket (goingAway)
CLI->>RPC: mark bridge stopped
RPC->>UI: update sidebar to disconnected
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning, 3 inconclusive)
✅ Passed checks (11 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Greptile SummaryThis PR fixes a stale "Connected" remote state that persisted after a VM PTY or daemon WebSocket died without tearing down the workspace configuration. The fix wires terminal child-exit, pane/tab close, and daemon WebSocket keepalive failures into a unified
Confidence Score: 5/5Safe to merge; the core state-machine logic is well-tested and the behavioural changes are all intentional and verified. The only finding is a style-level note about using No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Daemon WebSocket close / keepalive timeout] --> D
B[VM-PTY WebSocket ping failure] --> D
C[Remote terminal child exit\nTabManager.closePanelAfterChildExited] --> E
F[Tab / pane close\nWorkspace+PanelLifecycle] --> E
E[markRemoteTerminalSessionClosingIfLast\nor markRemoteTerminalSessionEnded] --> D
D[disconnectRemoteConnectionAfterTerminalExit\nclearConfiguration: false] --> G
G{SSH transport?}
G -- Yes, relayPort set --> H[requestSSHControlMasterCleanup]
G -- WebSocket / no relay --> I[skip cleanup]
H --> J[remoteConnectionState = .disconnected\nremoteConfiguration preserved]
I --> J
J --> K[createReplacementTerminalPanel\nwith remoteDisconnectPlaceholderScript]
K --> L[Press Enter to reconnect placeholder]
L --> M{User presses Enter}
M -- reconnect RPC available --> N[workspace.remote.reconnect\nconfigureRemoteConnection autoConnect=true]
M -- reconnect unavailable --> O[reconnectUnavailableHint banner]
N --> P[remoteConnectionState = .connected]
Reviews (19): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Resources/Localizable.xcstrings (1)
91185-91383:⚠️ Potential issue | 🟠 MajorFix incomplete locale coverage in
Resources/Localizable.xcstrings.These
sidebar.remote.*entries (sidebar.remote.subtitleFallback,sidebar.remote.help.connected/connecting/reconnecting/error/errorWithDetail/disconnected/targetFallback) only includelocalizationsforen,ja,uk, andko, butResources/Localizable.xcstringsalso supports:ar,bs,da,de,es,fr,it,km,nb,pl,pt-BR,ru,th,tr,zh-Hans,zh-Hant. Add translated values for every supported locale for these updated keys.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Resources/Localizable.xcstrings` around lines 91185 - 91383, Several sidebar.remote localization keys (sidebar.remote.subtitleFallback, sidebar.remote.help.connected, sidebar.remote.help.connecting, sidebar.remote.help.reconnecting, sidebar.remote.help.error, sidebar.remote.help.errorWithDetail, sidebar.remote.help.disconnected, sidebar.remote.help.targetFallback) only have en/ja/uk/ko entries; add missing translations for all supported locales (ar, bs, da, de, es, fr, it, km, nb, pl, pt-BR, ru, th, tr, zh-Hans, zh-Hant) so each key’s "localizations" object includes those locales with a stringUnit { state: "translated", value: "<translated text>" }, preserving extractionState: "manual" and matching placeholder patterns (e.g., %@ and %@: %@) from the en strings.Sources/Workspace.swift (1)
12646-12665:⚠️ Potential issue | 🟠 Major | ⚡ Quick winForce
waitAfterCommandon the disconnect placeholder panel.This replacement panel gets an
initialCommand, but unlike the other remote startup paths it never flipswaitAfterCommand. When the placeholder script exits—or after itexecs a reconnect command that later terminates—Ghostty can still fall through to a local shell.Proposed fix
- let inheritedConfig = inheritedTerminalConfig( + var replacementConfig = inheritedTerminalConfig( preferredPanelId: focusedPanelId, inPane: bonsplitController.focusedPaneId ) let pendingRemoteDisconnect = pendingRemoteDisconnectReplacement pendingRemoteDisconnectReplacement = nil let replacementInitialCommand: String? = pendingRemoteDisconnect.map { Self.remoteDisconnectPlaceholderScript( target: $0.target, reconnectCommand: $0.reconnectCommand ) } + if replacementInitialCommand != nil { + var config = replacementConfig ?? CmuxSurfaceConfigTemplate() + config.waitAfterCommand = true + replacementConfig = config + } let newPanel = TerminalPanel( workspaceId: id, context: GHOSTTY_SURFACE_CONTEXT_TAB, - configTemplate: inheritedConfig, + configTemplate: replacementConfig, portOrdinal: portOrdinal, initialCommand: replacementInitialCommand )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/Workspace.swift` around lines 12646 - 12665, The replacement terminal panel created in createReplacementTerminalPanel() sets an initialCommand (via pendingRemoteDisconnect -> remoteDisconnectPlaceholderScript) but doesn't set waitAfterCommand, so the shell can fall through after the placeholder exits; update the TerminalPanel initialization for newPanel to force waitAfterCommand = true (or set the equivalent property on newPanel) so the placeholder script blocks heredity until reconnect, ensuring the panel does not fall through to a local shell after the placeholder exits.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLI/cmux.swift`:
- Around line 7948-7973: The keepalive logic in sendKeepalive() can take nearly
2× the interval to detect a lost connection because detection relies on the next
timer tick seeing keepaliveInFlight; change the flow so that when task.sendPing
is invoked you immediately schedule a per-ping timeout (instead of waiting for
the next keepaliveTimer tick) that will trigger the same failure path if
keepaliveInFlight remains true after the configured interval. Concretely, in
sendKeepalive() (referencing keepaliveInFlight, keepaliveTimer, task.sendPing,
markStopped, and task.cancel) set keepaliveInFlight = true, start/schedule a
one-shot Dispatch/Timer timeout for the desired interval that checks
keepaliveInFlight and calls debugEvent?("websocket.keepalive.timeout"),
markStopped(), and task.cancel(...) if still in-flight, and clear that per-ping
timeout when the sendPing completion runs and resets keepaliveInFlight.
In `@Resources/Localizable.xcstrings`:
- Around line 91095-91123: The localization key
remote.status.terminalDisconnected is missing entries for several supported
locales; add entries for zh-Hans, zh-Hant, de, es, fr, it, da, pl, ru, bs, ar,
nb, pt-BR, th, and tr in the Resources/Localizable.xcstrings block for that key,
each with "stringUnit": { "state": "new", "value": "Remote terminal session
disconnected" } so they follow the catalog's convention (use the exact key
remote.status.terminalDisconnected to locate the block and append the missing
locale objects).
In `@Sources/Workspace.swift`:
- Around line 14585-14588: The code currently drops the disconnect sentinel by
setting pendingRemoteDisconnectReplacement = nil when a remote terminal exits,
which allows clearRemoteConfigurationIfWorkspaceBecameLocal() to demote the
workspace later; instead remove this unconditional nil assignment and ensure
pendingRemoteDisconnectReplacement is only cleared in the explicit
remote-reconnect/clear code paths (e.g., in the workspace reconnection handler
and wherever remote configuration is explicitly cleared). Locate the assignment
to pendingRemoteDisconnectReplacement and delete it, and add/ensure logic in the
methods that handle explicit remote workspace clearing or successful
reconnection to clear pendingRemoteDisconnectReplacement so the sentinel
persists until an explicit action.
- Around line 12616-12628: The current block always execs
cmux_disconnect_reconnect_command even if the workspace.remote.reconnect RPC
fails; modify the logic around
cmux_reconnect_cli/cmux_reconnect_socket/CMUX_WORKSPACE_ID so you run the RPC
(workspace.remote.reconnect) and check its exit status (or output) and only exec
/bin/sh -lc "$cmux_disconnect_reconnect_command" when that RPC call succeeded;
if the RPC failed or was unavailable, skip the exec (or surface an error) so the
PTY isn't re-launched out-of-sync. Ensure you reference and gate on the existing
symbols cmux_disconnect_reconnect_command, cmux_reconnect_cli,
cmux_reconnect_socket and CMUX_WORKSPACE_ID and use the RPC command name
workspace.remote.reconnect to determine success before exec.
---
Outside diff comments:
In `@Resources/Localizable.xcstrings`:
- Around line 91185-91383: Several sidebar.remote localization keys
(sidebar.remote.subtitleFallback, sidebar.remote.help.connected,
sidebar.remote.help.connecting, sidebar.remote.help.reconnecting,
sidebar.remote.help.error, sidebar.remote.help.errorWithDetail,
sidebar.remote.help.disconnected, sidebar.remote.help.targetFallback) only have
en/ja/uk/ko entries; add missing translations for all supported locales (ar, bs,
da, de, es, fr, it, km, nb, pl, pt-BR, ru, th, tr, zh-Hans, zh-Hant) so each
key’s "localizations" object includes those locales with a stringUnit { state:
"translated", value: "<translated text>" }, preserving extractionState: "manual"
and matching placeholder patterns (e.g., %@ and %@: %@) from the en strings.
In `@Sources/Workspace.swift`:
- Around line 12646-12665: The replacement terminal panel created in
createReplacementTerminalPanel() sets an initialCommand (via
pendingRemoteDisconnect -> remoteDisconnectPlaceholderScript) but doesn't set
waitAfterCommand, so the shell can fall through after the placeholder exits;
update the TerminalPanel initialization for newPanel to force waitAfterCommand =
true (or set the equivalent property on newPanel) so the placeholder script
blocks heredity until reconnect, ensuring the panel does not fall through to a
local shell after the placeholder exits.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8970634c-7db4-4229-9610-1cb430fdae60
📒 Files selected for processing (7)
CLI/cmux.swiftResources/Localizable.xcstringsSources/ContentView.swiftSources/TabManager.swiftSources/Workspace.swiftcmuxTests/TabManagerUnitTests.swiftcmuxTests/WorkspaceRemoteConnectionTests.swift
👮 Files not reviewed due to content moderation or server errors (1)
- Sources/ContentView.swift
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="CLI/cmux.swift">
<violation number="1" location="CLI/cmux.swift:7941">
P2: `stopKeepalive` uses `sendQueue.sync`, which can block shutdown if the send queue is busy in a blocking send path. Use async cancellation to avoid deadlocking teardown.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
CLI/cmux.swift (1)
7963-7972:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep the existing ping timeout armed while a ping is still outstanding.
At Line 7963 the next keepalive tick cancels and replaces the in-flight timeout before it has a chance to fire. Because the repeating timer and the
asyncAftertimeout share the same deadline onsendQueue, the timer callback can run first, which pushes disconnect detection out by another interval and recreates the stale-connected window this change is trying to remove.🐛 Suggested fix
- keepaliveTimeoutWorkItem?.cancel() - keepaliveInFlight = true + if keepaliveInFlight { + return + } + keepaliveInFlight = true let timeoutWorkItem = DispatchWorkItem { [weak self] in guard let self, !self.isStopped, self.keepaliveInFlight else { return } self.debugEvent?("websocket.keepalive.timeout") self.markStopped() self.task?.cancel(with: .goingAway, reason: nil) } + keepaliveTimeoutWorkItem?.cancel() keepaliveTimeoutWorkItem = timeoutWorkItem sendQueue.asyncAfter(deadline: .now() + Self.keepaliveInterval, execute: timeoutWorkItem)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CLI/cmux.swift` around lines 7963 - 7972, The keepalive timeout is being cancelled and replaced even when a ping is still outstanding, which prevents the original timeout from firing; update the logic around keepaliveTimeoutWorkItem/keepaliveInFlight so you do not cancel or replace an existing timeout while a ping is in flight (i.e., only create/set a new DispatchWorkItem and call sendQueue.asyncAfter when keepaliveInFlight is false or keepaliveTimeoutWorkItem is nil/finished); keep the existing timeout active until it fires or the in-flight state is cleared and reference the existing keepaliveTimeoutWorkItem, keepaliveInFlight, DispatchWorkItem, sendQueue.asyncAfter and Self.keepaliveInterval symbols to locate and modify the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Resources/Localizable.xcstrings`:
- Around line 91729-91756: Update the "sidebar.remote.subtitle" string entry to
remove the stale "SSH • %@" wording and use a generic remote label (e.g.,
"Remote • %@"), and add localized stringUnit entries for every locale supported
in this Localizable.xcstrings catalog so the touched key has full locale
coverage; ensure each localization has a translated state and appropriate
localized value for the new generic wording while keeping extractionState as
needed.
---
Duplicate comments:
In `@CLI/cmux.swift`:
- Around line 7963-7972: The keepalive timeout is being cancelled and replaced
even when a ping is still outstanding, which prevents the original timeout from
firing; update the logic around keepaliveTimeoutWorkItem/keepaliveInFlight so
you do not cancel or replace an existing timeout while a ping is in flight
(i.e., only create/set a new DispatchWorkItem and call sendQueue.asyncAfter when
keepaliveInFlight is false or keepaliveTimeoutWorkItem is nil/finished); keep
the existing timeout active until it fires or the in-flight state is cleared and
reference the existing keepaliveTimeoutWorkItem, keepaliveInFlight,
DispatchWorkItem, sendQueue.asyncAfter and Self.keepaliveInterval symbols to
locate and modify the code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6ceb45db-44b2-41d4-b8a6-98029c5440b6
📒 Files selected for processing (4)
CLI/cmux.swiftResources/Localizable.xcstringsSources/Workspace.swiftcmuxTests/WorkspaceRemoteConnectionTests.swift
👮 Files not reviewed due to content moderation or server errors (1)
- Sources/Workspace.swift
There was a problem hiding this comment.
3 issues found across 7 files
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
…xy-stale-connected
…xy-stale-connected # Conflicts: # Sources/Workspace.swift # cmuxTests/WorkspaceRemoteConnectionTests.swift
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ca8cc16. Configure here.
austinywang
dismissed stale reviews from coderabbitai[bot] and coderabbitai[bot]
June 4, 2026 12:45
Stale bot review: all review threads resolved and current CodeRabbit check is green on latest head 3c1f1e1.
…xy-stale-connected
…xy-stale-connected
…xy-stale-connected # Conflicts: # .github/swift-file-length-budget.tsv
…xy-stale-connected # Conflicts: # .github/swift-file-length-budget.tsv
hhsw2015
pushed a commit
to hhsw2015/cmux
that referenced
this pull request
Jun 15, 2026
Strategy: -X theirs + cherry-pick fork features back Upstream changes pulled in (highlights): - Stagger restored terminal surface spawns (manaflow-ai#6149) - Configurable Dock max width (manaflow-ai#4385) - Polish canvas minimap navigation (manaflow-ai#6105) - Opt-in AI auto-naming workspaces (manaflow-ai#6071) - Dissolve namespace-enums into value types (manaflow-ai#6126) - Fix stale remote connected state after proxy disconnect (manaflow-ai#4513) - Surface browser Safari (manaflow-ai#6113) - Fix terminal top-row mouse (manaflow-ai#4391) - iOS Shift key support (manaflow-ai#6104) Fork-only features verified intact post-merge: - cmux_term socket handlers: surface.snapshot, screen_text, screen_hash, wait_for_text, wait_for_idle, wait_for_screen_change, wait_for_kind, wait_for_cursor, tui_probe, expect, screen_region (6 handler funcs) - agent-bus: notification.create with $bus dispatch - TerminalSurface.visibleSnapshot() + processHasExited() helpers - skills/cmux-terminal-control/* (13 Python lib files + ORCHESTRATOR_TEMPLATE.md) - HerdrWorkspaceSync + CustomTitleSource.herdrInbound Adapter changes (fork-side): - Drop InternalImportsByDefault from CmuxFeedback/CmuxFeedbackUI packages (default-arg parameter types remained internal-only across modules, blocking FeedbackComposerBridge() construction from cmux app target) - Strip .rawValue from SurfaceKind cases (became plain String) - Add herdrInbound to CustomTitleSource enum - Delete shadowing local enums in ContentView (CommandPaletteOverlayPromotionPolicy, ExtensionSidebarBrowserStackDropPlanner, SidebarDragFailsafePolicy, SidebarDragLifecycleNotification, SidebarMarkdownRenderer, SidebarOutsideDropResetPolicy, SidebarShortcutHintFreezePolicy, SidebarTrailingAccessoryWidthPolicy, SidebarWorkspaceSelectionSyncPolicy, ShortcutHintDebugSettings, ShortcutHintModifierPolicy, FeedbackComposerBridge, FeedbackComposerBridgeError, ExtensionSidebarBrowserStackDropRow, SidebarTabDropIndicatorPredicate, SidebarDropEdge, SidebarDropIndicator) - Delete local TerminalSurfaceClaudeCommandShim (use package's via typealias) - Delete local TerminalSurfaceRuntimeTeardownCoordinator + Request stubs (use package's enqueueRuntimeTeardown public API directly) - Adapt static→instance: CmuxGhosttyConfigSettingEditor.X → ().X, CmuxApplicationSupportDirectories.userDirectories → init(env:).userDirectories - newTerminalSurface/TerminalPanel: thread externalIo: parameter for herdr - Add createReplacementTerminalPanel(in:) overload - Add attachToViewForInputDemand/requestInputDemandSurfaceStartIfNeeded stubs - TerminalSurfaceRuntimeFilesystem.live(): wrap installClaudeCommandShim sync→async - AppDelegate.sortedMainWindowContextsForSessionSnapshot: add includeQuickTerminal param - Wire BackgroundSessionStore + BackgroundSessionsSidebarSection + SidebarSectionDivider in pbxproj (PBXFileReference + PBXBuildFile entries were missing post-merge) - Add SidebarSectionDivider stub View (was referenced but never created) Rebuilt clean against /tmp/cmux-p52 derivedData.
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4509
Data-flow map
cmux vm .../ remote launch configures aWorkspaceRemoteConfigurationon the workspace.Workspace.configureRemoteConnectionstores that config, seeds the active remote terminal surface, and startsWorkspaceRemoteSessionControllerwhen there is a daemon/proxy transport to observe.Before this change, the sidebar read
Workspace.remoteConnectionState, but that state was mostly driven by initial proxy/daemon startup success. The terminal lifecycle was separate: when the remote PTY process died,TabManager.closePanelAfterChildExitedclosed the panel andWorkspace.createReplacementTerminalPanelspawned a local shell banner fallback. That let the visible terminal become local whileremoteConnectionStatestayedconnected.Now the shared signal is:
Repro Used
I could not create a real Freestyle VM in this dev app because the tagged build reported
Not signed inforcmux auth status. I used the equivalent forced-disconnect path in the running tagged app:CMUX_SKIP_ZIG_BUILD=1 ./scripts/reload.sh --tag issue-4509-remote-proxy-stale-connected --launch.workspace.remote.configureas a WebSocket remote target.workspace.remote.statusreportedstate=connected,connected=true,active_terminal_sessions=1.exitto the tracked terminal surface to simulate the remote PTY child dying.workspace.remote.statusreportedstate=disconnected,connected=false,active_terminal_sessions=0, and detailRemote terminal session disconnected.austinwang@mac ~ %prompt.Architectural Change
TabManager.closePanelAfterChildExitednow notifies the workspace when a tracked remote terminal child exits, so terminal process death is an input to the remote state machine.Workspace.markRemoteTerminalSessionEndednow handles WebSocket/VM remote terminals that do not have an SSH relay port, transitions the workspace to disconnected, preserves the remote configuration for reconnect, and clears active remote terminal session state.Press Enter to reconnect...; it optionally invokesworkspace.remote.reconnectand then reruns the original remote startup command.connectedfor WebSocket VM workspaces; that preservation is limited to SSH proxy-only retry cases while the SSH terminal is still alive.vm-pty-connectbridge now send 5-second WebSocket pings so dead sleep/wake connections are detected without waiting for user input.Disconnectedafter a remote PTY death.Screenshots
Captured from the tagged dev app during the forced-disconnect verification:
Connected:/tmp/cmux-issue-4509-final-before.pngDisconnectedand the terminal shows the reconnect placeholder:/tmp/cmux-issue-4509-final-after.pngTest Approach
connectedwithin a bounded time.Focused verification run:
CMUX_SKIP_ZIG_BUILD=1 ./scripts/test-unit.sh \ -only-testing:cmuxTests/WorkspaceRemoteConnectionTests/testWebSocketRemoteTerminalEndLeavesConnectedStateWithinBoundedTime \ -only-testing:cmuxTests/WorkspaceRemoteConnectionTests/testWebSocketDaemonTransportErrorClearsConnectedSidebarState \ -only-testing:cmuxTests/TabManagerChildExitCloseTests/testChildExitOnLastRemotePanelKeepsWorkspaceDisconnected \ testResult: passed.
Also ran:
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Changes core remote workspace lifecycle, terminal replacement, and WebSocket teardown paths that users rely on for SSH/VM sessions; well-covered by tests but behavior shifts are broad.
Overview
Fixes stale Connected remote UI when the VM/SSH PTY or daemon WebSocket dies without tearing down the workspace config.
Remote state machine: Terminal child exit, pane/tab close, and relay callbacks now drive
markRemoteTerminalSessionEnded, which clears active PTY tracking, sets disconnected (with a stable detail string), and can still run SSH control-master cleanup. WebSocket/VM paths no longer require a relay port; “stay connected” on proxy-only errors is limited to SSH while a live SSH terminal remains.Terminal UX: Replacing the last panel no longer execs a local login shell. It launches a disconnect placeholder that shows a banner and Press Enter to reconnect (optional
workspace.remote.reconnectRPC + original startup command). Closing the last remote tab/pane preserves/refreshes that placeholder.Liveness: 5s WebSocket pings with timeout teardown were added to
VMPtyWebSocketBridgeand the workspace daemon RPC client so sleep/wake dead connections surface as disconnects.UI/copy: Sidebar remote row shows for any configured remote workspace (
isRemoteWorkspace), with SSH-specific strings broadened to Remote and expandedLocalizable.xcstringsentries for status/help/disconnect banners.Tests: Child-exit and remote-connection tests now expect remote-configured + disconnected instead of demotion to local; new WebSocket session-end and daemon error coverage.
Reviewed by Cursor Bugbot for commit ca8cc16. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes #4509 by clearing stale “Connected” when the VM/SSH PTY or daemon WebSocket drops. The workspace stays Remote, shows Disconnected, and terminals show a reconnect prompt that persists across reconnects and tab/pane closes.
Bug Fixes
Workspace.remoteConnectionStatefrom daemon WebSocket close/keepalive timeouts, VM‑PTY ping failures, terminal child exits (tracked/untracked), and manual tab/pane/terminal close.New Features
VMPtyWebSocketBridgeto detect dead connections quickly.workspace.remote.reconnectand re‑runs the startup command; the RPC validates and accepts an optionalsurface_idto reconnect in place, and the placeholder is consumed once to avoid duplicates.Written for commit 736ed37. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Tests