Repository navigation
remote-tmux: recover interactive SSH auth when a ProxyCommand transport closes silently - #7020
Merged
Merged
Conversation
|
@ejc3 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:
📝 WalkthroughWalkthroughThis PR adds a new stderr classification predicate, ChangesInteractive SSH Retry Routing
Sequence Diagram(s)sequenceDiagram
participant RemoteTmuxController
participant RemoteTmuxSSHTransport
participant Host
RemoteTmuxController->>RemoteTmuxSSHTransport: indicatesInteractiveRetryWillHelp(stderr)
RemoteTmuxSSHTransport-->>RemoteTmuxController: true (auth-required or proxy-closed)
RemoteTmuxController->>Host: interactiveAuthInvocation()
🎯 2 (Simple) | ⏱️ ~15 minutes
🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 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 |
ejc3
marked this pull request as ready for review
June 28, 2026 03:38
Contributor
Greptile SummaryThis PR fixes a silent failure in the remote-tmux SSH discovery probe: when a
Confidence Score: 5/5Safe to merge — the change is narrow, the non-recoverable marker list is well-considered, and the existing The new predicate is anchored to a highly specific OpenSSH pipe-transport placeholder, suppressed by 17 non-recoverable markers (covering DNS, TCP timeout, refused, banner exchange, missing binary on all OSes), and composed cleanly with the existing auth predicate. All three retry-routing sites in the controller are updated. Test coverage spans silent positive cases, all explained-closure negatives, false-positive anchoring, and composed predicate behavior. No behavioral regressions are introduced at unmodified call sites. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["BatchMode=yes SSH probe\n(commandFailed thrown)"] --> B{indicatesInteractiveRetryWillHelp?}
B --> C{indicatesAuthRequired?}
C -- "Permission denied\nHost key mismatch\nMFA / Too many failures" --> D["✅ Interactive retry\n(route user to ssh in terminal)"]
C -- no --> E{indicatesProxyCommandTransportClosed?}
E --> F{"stderr contains\n'to/by UNKNOWN port 65535'?"}
F -- no --> G["❌ Hard failure\n(rethrow to caller)"]
F -- yes --> H{"stderr also contains\nnonRecoverableProxyMarker?"}
H -- "connect failed / open failed\nstdio forwarding failed\nConnection refused / timed out\nDNS errors / kex errors\nmissing binary / etc." --> G
H -- "no diagnostic marker\n(truly silent proxy closure)" --> D
%%{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"}}}%%
flowchart TD
A["BatchMode=yes SSH probe\n(commandFailed thrown)"] --> B{indicatesInteractiveRetryWillHelp?}
B --> C{indicatesAuthRequired?}
C -- "Permission denied\nHost key mismatch\nMFA / Too many failures" --> D["✅ Interactive retry\n(route user to ssh in terminal)"]
C -- no --> E{indicatesProxyCommandTransportClosed?}
E --> F{"stderr contains\n'to/by UNKNOWN port 65535'?"}
F -- no --> G["❌ Hard failure\n(rethrow to caller)"]
F -- yes --> H{"stderr also contains\nnonRecoverableProxyMarker?"}
H -- "connect failed / open failed\nstdio forwarding failed\nConnection refused / timed out\nDNS errors / kex errors\nmissing binary / etc." --> G
H -- "no diagnostic marker\n(truly silent proxy closure)" --> D
Reviews (10): Last reviewed commit: "remote-tmux: treat local ProxyCommand la..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?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: 1
🤖 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/RemoteTmuxSSHTransport.swift`:
- Around line 345-359: Add `ssh_exchange_identification:` to
`RemoteTmuxSSHTransport.nonRecoverableProxyMarkers` alongside
`kex_exchange_identification:` so `indicatesProxyCommandTransportClosed(_:)`
treats both banner-failure variants as non-recoverable. Keep the existing
proxy-closure detection path in `RemoteTmuxSSHTransport` unchanged otherwise,
and ensure the new marker matches the same stderr wording already covered by the
auth tests.
🪄 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: 0c8e9f17-faae-43b9-ab8b-3508fbcee8e2
📒 Files selected for processing (4)
Sources/RemoteTmuxController.swiftSources/RemoteTmuxError.swiftSources/RemoteTmuxSSHTransport.swiftcmuxTests/RemoteTmuxAuthTests.swift
This was referenced Jun 28, 2026
Contributor
Author
|
Addressed in 3a14766: added |
ejc3
marked this pull request as draft
June 29, 2026 08:33
ejc3
force-pushed
the
ejc3/remote-tmux-proxy-retry
branch
from
June 29, 2026 22:47
3a14766 to
52fea4c
Compare
ejc3
marked this pull request as ready for review
June 29, 2026 22:54
ejc3
force-pushed
the
ejc3/remote-tmux-proxy-retry
branch
from
June 29, 2026 23:12
52fea4c to
7474e94
Compare
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ejc3
force-pushed
the
ejc3/remote-tmux-proxy-retry
branch
from
July 1, 2026 01:04
e3f22e9 to
d30f2b1
Compare
Contributor
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
…etry recoverable A BatchMode=yes discovery probe through an ssh `ProxyCommand` whose own pre-handshake auth or 2FA leg silently aborts (no tty to prompt on) closes the proxy pipe before SSH emits any auth-failure string. Catch this stderr signature (OpenSSH's `to/by UNKNOWN port 65535` placeholder for pipe transports) and route it to the same interactive ssh retry already used for `Permission denied` / host-key TOFU / MFA. Introduces `indicatesProxyCommandTransportClosed` next to the existing `indicatesAuthRequired` and a composed `indicatesInteractiveRetryWillHelp` so the three RemoteTmuxController routing sites that previously each spelled out `indicatesAuthRequired` (mirrorHostInNewWindow's discovery catch, preflightControlAttach's catch arm, and authRequiredAttachArgv) now go through a single name — preventing the asymmetry where only one entrypoint would have gotten the new recovery and the others silently regressed.
… recoverable OpenSSH's `to/by UNKNOWN port 65535` placeholder is also emitted when a ProxyCommand / ProxyJump fails for reasons no interactive ssh retry can fix: target unreachable behind the jumphost, `nc` to a refused port, stdio forwarding teardown, target spoke no SSH on the negotiated port, DNS NXDOMAIN. Those failures stamp explicit diagnostic markers (`connect failed:`, `: open failed:`, `stdio forwarding failed`, `kex_exchange_identification:`, `Connection refused`, `No route to host`, etc.) into stderr alongside the placeholder; route them to the real error instead of bouncing the user through a futile interactive prompt. Tightens indicatesProxyCommandTransportClosed to require the placeholder AND no diagnostic marker before firing. Adds positive tests for the SILENT closures we still need to catch and negative tests for the EXPLAINED closures the predicate must now skip — including the precise stderr codex's review reproduced via `ssh -J nowhere.invalid` and `ssh -oProxyCommand='nc localhost 9'`.
…classification `xcodebuild test` against `cmuxTests/RemoteTmuxAuthTests` flagged that the `Could not resolve hostname …\nConnection closed by UNKNOWN port 65535` stderr slipped past the silent-closure check — OpenSSH wraps every `getaddrinfo` failure with that prefix (across macOS / Linux / Windows getaddrinfo strerrors) so the proxy DNS NXDOMAIN looked silent to the predicate. Adds `could not resolve hostname` to the non-recoverable marker list alongside the existing `name or service not known` / `temporary failure in name resolution` constants (which only cover the underlying getaddrinfo wording, not the OpenSSH wrapper).
…t-proxy-close Second codex review pass reproduced a remaining false positive: when a `ProxyCommand` uses `nc` and `nc` itself fails DNS, BSD/macOS netcat emits `nc: getaddrinfo: nodename nor servname provided, or not known` raw — OpenSSH's `Could not resolve hostname` wrapper only fires when OpenSSH does the resolution, not when an inferior `ProxyCommand` does. The resulting stderr has the proxy placeholder and no exclusion marker, so the predicate returned true and routed the user through a futile interactive retry. Adds `nodename nor servname provided` to the non-recoverable marker list and extends `doesNotClassifyExplainedProxyClosures` with the exact stderr codex reproduced via `ssh -o ProxyCommand='nc nonexistent.invalid 22'`.
…osures as non-recoverable Review follow-up: two more explained ProxyCommand/ProxyJump closures were slipping past the silent-closure check and routing the user through a futile interactive retry: - Linux TCP connect timeouts phrase it "Connection timed out" (only the BSD/macOS "Operation timed out" was covered), so an nc-based ProxyCommand timing out on Linux looked silent. - "ssh_exchange_identification:" banner-exchange closures (the inner target dropping the connection pre-auth: fail2ban, tcpwrappers, not-SSH-on-port) were only excluded when a second marker happened to co-occur. Adds both to nonRecoverableProxyMarkers with isolated negative tests (no co-present marker) so each is genuinely exercised.
ejc3
force-pushed
the
ejc3/remote-tmux-proxy-retry
branch
from
July 1, 2026 05:29
d30f2b1 to
037f8f8
Compare
…y-retry # Conflicts: # cmux.xcodeproj/project.pbxproj
A ProxyCommand that fails to launch (missing binary, bad path, wrong-arch executable) emits only the shell's diagnostic plus OpenSSH's UNKNOWN-port placeholder — verified on OpenSSH 10.2: no kex_exchange_identification line precedes it. The silent-closure classifier treated those as interactive-retry recoverable, hiding the actionable config error behind a retry that fails identically. Add the shell launch diagnostics (bash/zsh 'command not found', dash/busybox ': not found', 'no such file or directory', 'exec format error') to nonRecoverableProxyMarkers, with negative fixtures for each shell phrasing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This was referenced Jul 3, 2026
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
2 participants
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.
Summary
When cmux mirrors a remote host's tmux over
ssh tmux -CC, the first step is aBatchMode=yesdiscovery probe over the shared SSH ControlMaster. If the host is reached through ansshProxyCommand/ProxyJumpwhose own pre-handshake step (authentication, host-key, or a 2FA/MFA leg) needs a terminal, that step can't prompt underBatchMode, so the proxy closes the pipe before OpenSSH emits any auth-failure string. cmux saw only OpenSSH's pipe-transport placeholder (Connection closed by/to UNKNOWN port 65535) and treated it as a hard failure — so it never offered the interactive retry it already uses forPermission denied/ host-key TOFU / MFA, and the connection failed outright.Fix
Classify a silent
ProxyCommandtransport closure as interactive-retry-recoverable, so the existing "runsshin your terminal, then retry over the now-authenticated master" path kicks in.indicatesProxyCommandTransportClosedand a composedindicatesInteractiveRetryWillHelp, routed through all three discovery/attach sites so no entrypoint silently regresses.connect failed:,: open failed:,stdio forwarding failed,kex_exchange_identification:,Connection refused,No route to host,could not resolve hostname(OpenSSH's DNS wrapper), andnodename nor servname provided(BSD/macOSgetaddrinfoNXDOMAIN, e.g. anc-basedProxyCommand).Tests
cmuxTests/RemoteTmuxAuthTests— positive cases for the silent closures we must recover, and negative cases for every explained closure we must skip. 42 tests pass.Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Restores interactive SSH auth when a
ProxyCommand/ProxyJumpcloses the transport silently during BatchMode discovery. Adds a composed retry check and skips explained proxy failures (including DNS, timeouts, banner/pre-auth, port-forward errors, and local ProxyCommand launch errors) to avoid futile prompts.indicatesProxyCommandTransportClosedand composedindicatesInteractiveRetryWillHelp; all discovery/attach paths use the composed predicate.getaddrinfo; TCP timeouts on macOS/Linux;ssh_exchange_identification:/kex_exchange_identification:; shell launch errors like “command not found”, “: not found”, “no such file or directory”, “exec format error”).RemoteTmuxProxyTransportRetryTests) covering silent vs explained proxy closures, includingncDNS NXDOMAIN (Linux/BSD), Linux connect timeouts, banner-exchange closures, and shell launch failures.Written for commit 9d7c180. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Tests