Add manual SSH reconnect: in-pane 'r' prompt + Reconnect Pane menu - #7250
Conversation
- SSH startup wrapper: when the auto-reconnect loop gives up, the hold prompt now offers 'r' + Enter to re-run the connect loop (fresh attempt counter) instead of only closing the pane. Enter/q/EOF dismisses. - Terminal pane right-click menu: add 'Reconnect Pane' (remote panes only) that reconnects the focused surface via reconnectRemoteConnection(surfaceId:). - Localize 'Reconnect Pane' (en/ja/ko). - Tests: manual-retry re-enters connect loop; session-end runs once.
|
@0xJord4n is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
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:
📝 WalkthroughWalkthroughThe generated SSH startup script now loops on reconnect: successful SSH exits break immediately, while failures prompt for retry ( ChangesManual SSH pane reconnect
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant cmuxSwift as cmux.swift generated script
participant SSH
participant ReconnectHelper as cmux_ssh_remote_reconnect (RPC helper)
cmuxSwift->>SSH: start background SSH command
SSH-->>cmuxSwift: exit status
alt status == 0
cmuxSwift->>cmuxSwift: break session loop
else non-zero exit
cmuxSwift->>User: show manual reconnect prompt
alt user enters r/reconnect
cmuxSwift->>ReconnectHelper: cmux_ssh_remote_reconnect
ReconnectHelper->>ReconnectHelper: rpc workspace.remote.reconnect
cmuxSwift->>cmuxSwift: reset retry counter, re-enter outer loop
else user dismisses
cmuxSwift->>cmuxSwift: cmux_ssh_session_end teardown, exit
end
end
sequenceDiagram
participant User
participant GhosttyNSView
participant Workspace
participant TerminalPanel
User->>GhosttyNSView: right-click terminal
GhosttyNSView->>Workspace: remoteWorkspaceForCurrentSurface()
GhosttyNSView->>Workspace: canReconnectRemotePane(state)
GhosttyNSView->>TerminalPanel: remoteReconnectablePanel(in:)
GhosttyNSView-->>User: show "Reconnect Pane" menu item
User->>GhosttyNSView: select Reconnect Pane
GhosttyNSView->>Workspace: re-validate reconnect eligibility
GhosttyNSView->>TerminalPanel: send input "r\r"
Possibly related issues
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 inconclusive)
✅ Passed checks (23 passed)
✨ Finishing Touches🧪 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 |
Greptile SummaryThis PR adds a manual SSH reconnect mechanism with two entry points: an in-pane
Confidence Score: 5/5Safe to merge — the new reconnect paths are well-guarded, thoroughly tested, and localized across all supported locales. The outer session-loop restructure in the shell script, the reconnectRemoteConnection guard additions, and the remoteReconnectablePanel gate in the context-menu extension all work correctly and are validated by four new integration tests. Signal handling during read degrades to the same close-pane behavior as before this PR. The pendingRemoteTerminalChildExitSurfaceIds cleanup goes through trackRemoteTerminalSurface, which removes the ID as confirmed in the source. No correctness-critical paths are left unguarded. Sources/Workspace.swift — the new reconnectRemoteConnection one-liner is the only code that benefits from a second look, purely for future maintainability. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant PTY as Terminal PTY
participant Shell as SSH Shell Script
participant App as cmux App (Workspace)
participant SSH as ssh process
Shell->>SSH: "launch ssh <&0 &"
SSH-->>Shell: exit non-zero (non-retryable)
Shell->>App: cmux_ssh_session_end (RPC: ssh-session-end)
Note over App: surface → pendingRemoteTerminalChildExitSurfaceIds
Shell->>PTY: printf manual reconnect prompt
alt User presses r + Enter (in-pane)
PTY->>Shell: r\n via stdin read
Shell->>App: cmux_ssh_remote_reconnect (RPC: workspace.remote.reconnect)
App->>App: reconnectRemoteConnection(surfaceId:)
Shell->>Shell: "CMUX_SSH_SESSION_ENDED=0 && continue outer loop"
Shell->>SSH: launch ssh again (fresh inner loop)
SSH-->>Shell: exit 0 (success)
Shell->>App: cmux_ssh_session_end (2nd call)
else User presses Enter / EOF
PTY->>Shell: empty line or EOF
Shell->>Shell: break → exit $cmux_ssh_status
else Context-menu Reconnect Pane
App->>App: remoteReconnectablePanel() guards check
App->>PTY: panel.sendInput(r\r)
PTY->>Shell: r\n via stdin read
Note over Shell,App: same r+Enter path as above
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant PTY as Terminal PTY
participant Shell as SSH Shell Script
participant App as cmux App (Workspace)
participant SSH as ssh process
Shell->>SSH: "launch ssh <&0 &"
SSH-->>Shell: exit non-zero (non-retryable)
Shell->>App: cmux_ssh_session_end (RPC: ssh-session-end)
Note over App: surface → pendingRemoteTerminalChildExitSurfaceIds
Shell->>PTY: printf manual reconnect prompt
alt User presses r + Enter (in-pane)
PTY->>Shell: r\n via stdin read
Shell->>App: cmux_ssh_remote_reconnect (RPC: workspace.remote.reconnect)
App->>App: reconnectRemoteConnection(surfaceId:)
Shell->>Shell: "CMUX_SSH_SESSION_ENDED=0 && continue outer loop"
Shell->>SSH: launch ssh again (fresh inner loop)
SSH-->>Shell: exit 0 (success)
Shell->>App: cmux_ssh_session_end (2nd call)
else User presses Enter / EOF
PTY->>Shell: empty line or EOF
Shell->>Shell: break → exit $cmux_ssh_status
else Context-menu Reconnect Pane
App->>App: remoteReconnectablePanel() guards check
App->>PTY: panel.sendInput(r\r)
PTY->>Shell: r\n via stdin read
Note over Shell,App: same r+Enter path as above
end
Reviews (15): Last reviewed commit: "Qualify static test helpers in SSH manua..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
The outer session loop resets cmux_ssh_retry=0 unconditionally at the top of every iteration (including the first), so the pre-loop initializer was dead code introduced by the manual-reconnect change. Flagged in PR review.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Workspace.swift (1)
5892-5906: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPlaceholder/tracking side effects run before the new connecting/reconnecting guard, leaving state desynced on early return.
reconnectRemoteConnection(surfaceId:)removes the surface fromremoteDisconnectPlaceholderPanelIdsand callstrackRemoteTerminalSurface(_:)unconditionally (lines 5900-5903) before the guard at line 5904 checksremoteConnectionState. The PR's stated intent for widening this guard is to prevent reconnect during an active attempt (e.g. user clicks "Reconnect Pane" while an automatic retry is already.reconnecting/.connecting) — meaning this early-return path is now expected to be hit in real usage, not just as a defensive/unreachable check.When that happens, the surface is already unmarked as a disconnect placeholder and marked as a tracked/live remote terminal surface, even though
configureRemoteConnectionwas never called and no actual reconnect occurred. This leaves the pane's placeholder/tracking state out of sync with the real connection state.Move the guard to the top of the function so the placeholder/tracking mutation only happens when a reconnect will actually be attempted.
🐛 Proposed fix: check the guard before mutating placeholder/tracking state
func reconnectRemoteConnection(surfaceId: UUID? = nil) { + guard let configuration = remoteConfiguration, + remoteConnectionState != .connecting, + remoteConnectionState != .reconnecting else { return } let reconnectingPlaceholderSurfaceId = surfaceId.flatMap { candidate -> UUID? in guard remoteDisconnectPlaceholderPanelIds.contains(candidate), panels[candidate] is TerminalPanel else { return nil } return candidate } if let reconnectingPlaceholderSurfaceId { remoteDisconnectPlaceholderPanelIds.remove(reconnectingPlaceholderSurfaceId) trackRemoteTerminalSurface(reconnectingPlaceholderSurfaceId) } - guard let configuration = remoteConfiguration, remoteConnectionState != .connecting, remoteConnectionState != .reconnecting else { return } configureRemoteConnection(configuration, autoConnect: true) }🤖 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 5892 - 5906, In reconnectRemoteConnection(surfaceId:), the placeholder/tracking mutation happens before the new remoteConnectionState guard, so early returns can desync state. Move the guard that blocks .connecting and .reconnecting to the top of the function, before removing from remoteDisconnectPlaceholderPanelIds or calling trackRemoteTerminalSurface(_:), so those side effects only occur when configureRemoteConnection(_:autoConnect:) will actually run.
🤖 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 `@Sources/GhosttyNSView`+RemoteReconnect.swift:
- Around line 45-50: The reconnect action in reconnectRemotePane(_:) is
currently sending a bare Enter through remoteReconnectPlaceholderPanel, which
only follows the dismiss flow instead of manually reconnecting. Update this path
to invoke the same reconnect helper used elsewhere in the remote reconnect flow,
using the workspace/panel context already gathered, so the menu item actually
triggers pane reconnection rather than submitting an empty line.
---
Outside diff comments:
In `@Sources/Workspace.swift`:
- Around line 5892-5906: In reconnectRemoteConnection(surfaceId:), the
placeholder/tracking mutation happens before the new remoteConnectionState
guard, so early returns can desync state. Move the guard that blocks .connecting
and .reconnecting to the top of the function, before removing from
remoteDisconnectPlaceholderPanelIds or calling trackRemoteTerminalSurface(_:),
so those side effects only occur when configureRemoteConnection(_:autoConnect:)
will actually run.
🪄 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: 4e120679-4d67-41f0-8665-c05dd5aac56e
📒 Files selected for processing (9)
CLI/CMUXCLI+SSHReconnectPrompt.swiftCLI/cmux.swiftResources/Localizable.xcstringsSources/GhosttyNSView+RemoteReconnect.swiftSources/GhosttyTerminalView.swiftSources/Workspace.swiftcmux.xcodeproj/project.pbxprojcmuxTests/SSHStartupManualReconnectTests.swiftcmuxTests/SSHStartupSignalLifecycleTests.swift
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
| guard let workspace = remoteWorkspaceForCurrentSurface(), | ||
| canReconnectRemotePane(in: workspace), | ||
| let panel = remoteReconnectPlaceholderPanel(in: workspace) else { return } | ||
| panel.sendInput("\r") |
There was a problem hiding this comment.
" " (a bare Enter/carriage-return) to the panel. The shell's IFS= read -r _cmux_dismiss_key waits for a newline-terminated line; with ICRNL active, is translated to , so _cmux_dismiss_key is set to the empty string. The case in the shell script only matches r|R|retry|reconnect — an empty string falls through, hits the break, and the pane closes without reconnecting. The test correctly feeds "r " to reproduce the reconnect path; the menu item needs to send "r" followed by a line-ending to match. Using "r " triggers the same ICRNL translation (r + Enter) that the keyboard path uses.
| panel.sendInput("\r") | |
| panel.sendInput("r\r") |
There was a problem hiding this comment.
Fixed — the menu item now sends "r\r" (commit dc81c14 "Send reconnect input from pane menu"), which takes the same r + Enter path as typing in the pane. manualReconnectReentersConnectLoop covers the resulting reconnect re-entry end-to-end (fake ssh + fake CLI, asserting the second connect attempt and the session-end accounting).
Pin the deliberate mutation-before-guard semantics of reconnectRemoteConnection(surfaceId:): a valid ended pane is re-tracked even when the workspace-level reconfigure is skipped (workspace already connected, or a reconnect already in flight), because the pane's shell loop re-runs ssh regardless of which internal path the RPC took. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Re the CodeRabbit outside-diff finding on The ordering is deliberate. The pane's shell loop re-runs ssh regardless of which internal path the RPC took ( Invalid/unended surface ids still return before any mutation ( |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CLI/cmux.swift (1)
9829-9838: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winKeep the reconnect prompt open on reconnect RPC failure.
cmux_ssh_remote_reconnect()already returns success when it can’t even attempt a reconnect, so the only failure path here is an actualworkspace.remote.reconnecterror. In that case therbranch still falls through tobreak, closing the pane without telling the user the reconnect failed. Re-prompt instead.🤖 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 9829 - 9838, The reconnect prompt flow in the cmux SSH loop should stay open when `cmux_ssh_remote_reconnect()` fails on an actual `workspace.remote.reconnect` error. Update the `r|R|retry|reconnect` branch in the SSH reconnect prompt logic to detect a failed reconnect attempt and re-display the prompt instead of falling through to `break`, while keeping the existing success path that clears `CMUX_SSH_SESSION_ENDED`.
🤖 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.
Outside diff comments:
In `@CLI/cmux.swift`:
- Around line 9829-9838: The reconnect prompt flow in the cmux SSH loop should
stay open when `cmux_ssh_remote_reconnect()` fails on an actual
`workspace.remote.reconnect` error. Update the `r|R|retry|reconnect` branch in
the SSH reconnect prompt logic to detect a failed reconnect attempt and
re-display the prompt instead of falling through to `break`, while keeping the
existing success path that clears `CMUX_SSH_SESSION_ENDED`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 233468cf-bd68-4c62-8132-cd0581926e28
📒 Files selected for processing (8)
CLI/CMUXCLI+SSHReconnectPrompt.swiftCLI/cmux.swiftResources/Localizable.xcstringsSources/GhosttyNSView+RemoteReconnect.swiftSources/GhosttyTerminalView.swiftSources/Workspace.swiftcmux.xcodeproj/project.pbxprojcmuxTests/SSHStartupManualReconnectTests.swift
💤 Files with no reviewable changes (1)
- cmuxTests/SSHStartupManualReconnectTests.swift
WorkspaceRemoteConfiguration is a CmuxCore type; the suite referenced it without importing the package. Earlier PR CI runs were cancelled before this file ever compiled, so the first full app-host compile surfaced it across all four shards. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
writeShellFile and runProcess are private static helpers called from the instance-context @test func; Swift requires Self-qualification there. Latent since the suite predates any completed CI compile of this file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2 participants
What
Adds a way to manually reconnect a failed SSH pane, instead of only being able to close it.
Two entrypoints:
In-pane
rprompt — When the automatic reconnect loop gives up (or hits a non-retryable exit), the SSH hold prompt now offersr+ Enter to re-run the connect loop with a fresh attempt counter. Enter / q / EOF still dismisses and closes the pane as before."Reconnect Pane" context-menu item — The terminal pane's right-click menu gains a "Reconnect Pane" action (shown only for remote panes) that reconnects the focused surface via
reconnectRemoteConnection(surfaceId:).Why
Previously a dropped/paused/destroyed remote VM left the pane showing only "press Enter to close this pane" — the only recovery was to close and manually recreate it. This lets the user retry in place.
Changes
CLI/cmux.swift— wrap the reconnect loop + hold prompt in an outer session loop;rre-enters the connect loop.Sources/GhosttyTerminalView.swift— "Reconnect Pane" item inmenu(for:), gated on the surface belonging to a remote workspace.Resources/Localizable.xcstrings— localize "Reconnect Pane" (en / ja / ko).cmuxTests/SSHStartupSignalLifecycleTests.swift—testSSHStartupManualReconnectReentersConnectLoop(fake ssh fails once then succeeds;r\nre-enters the loop → status 0, 2 attempts, session-end once) plus updated banner-wording / session-end-once assertions.Testing
xcodebuild test(cmux-unitscheme): both SSH-startup lifecycle tests pass (2 tests, 0 failures).reload.sh; verified live —r+ Enter reconnects, and the "Reconnect Pane" menu item reconnects the focused pane.Notes
reconnectRemoteConnectionaction path (per cmux shared-behavior policy) — no duplicated reconnect logic per entrypoint.Localizable.xcstringsfor en/ja/ko (matching the existingterminalContextMenu.*sibling keys).Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds manual SSH reconnect so users can retry a failed remote pane without closing it. Press r then Enter in the pane or use the Reconnect Pane menu; only ended/inactive panes are eligible, and reconnects are blocked while the workspace is connecting or reconnecting.
New Features
Bug Fixes
CmuxCoreimport and qualified static helpers in SSH manual reconnect tests.Written for commit 7931e79. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes