Conversation
The test drives the bundled CLI with `ssh --via tsh --no-focus` and asserts it opens a workspace running `tsh ssh` with no relay wiring (single workspace.create, no workspace.remote.configure, no OpenSSH-only options in initial_command). Without the implementation the CLI rejects the unknown --via flag and exits non-zero, so this commit is red on CI.
…links Adds an explicit, opt-in Teleport transport to cmux ssh. --via tsh (default ssh) opens a workspace whose terminal execs `tsh ssh <dest>` directly: interactive-only, with no cmux remote daemon (no relay, no cmuxd-remote, no workspace.remote.configure). tsh cannot honor the OpenSSH options the relay depends on (RemoteCommand, ControlMaster, SetEnv, LocalCommand, -i, -tt), so the emitted command is minimal: binary, -p, -A (when ForwardAgent), -o passthroughs, destination, and any -- remote args. Wired through every shared entrypoint: - CLI: --via <ssh|tsh> flag + runInteractiveTeleportSSH (CLI/cmux.swift), localized --via help text. - Deep links: via query param on the cmux scheme (cmux://ssh?...&via=tsh) and the standard ssh:// scheme, emitting --via tsh through CmuxSSHURLRequest's shared cliArguments path. New invalidTransport parse error (localized). - Web docs: --via flag row + via deep-link param (en/ja messages). The sidebar has no SSH-connect action to wire (sidebar.showSSH is a display-only toggle for already-remote workspaces). Tests: URL-scheme parsing (cmux + standard schemes, default, invalid value); the prior commit's CLI integration test goes green here. See docs/ssh-teleport.md for the tsh ssh -R relay investigation.
|
@4thel00z is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds Teleport ChangesTeleport tsh SSH Transport
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as cmux ssh
participant Parser as Argument Parser
participant Router as Transport Router
participant Teleport as runInteractiveTeleportSSH
participant RPC as workspace.create RPC
User->>CLI: cmux ssh user@host --via tsh
CLI->>Parser: parse --via tsh
Parser-->>CLI: SSHCommandOptions{transport: .teleport}
CLI->>Router: check transport value
Router->>Teleport: runInteractiveTeleportSSH(options)
Teleport->>RPC: workspace.create(ssh_command="tsh ssh user@host", transport="tsh")
RPC-->>Teleport: workspace ID + response
Teleport-->>User: print formatted success or JSON
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (18 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 |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds Teleport (tsh) as an alternative SSH transport across CLI and deep links, with docs/UI updates and tests to validate the new behavior.
Changes:
- Added
--via <ssh|tsh>tocmux ssh, implementing an interactive-onlytsh sshworkspace path. - Added
via=tshsupport in SSH deep links / URL parsing, including error handling and localization. - Updated web docs/messages and added test coverage for both CLI behavior and URL parsing.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| web/messages/ja.json | Adds Japanese UI strings for the new --via / deep-link via parameter. |
| web/messages/en.json | Adds English UI strings for the new --via / deep-link via parameter. |
| web/app/[locale]/docs/ssh/page.tsx | Documents the new --via flag and via deep-link parameter in the web docs table. |
| docs/ssh-teleport.md | Adds a new Teleport (tsh) SSH support doc describing interactive-only behavior and future relay ideas. |
| cmuxTests/VMSSHCommandTests.swift | Adds an integration regression test ensuring --via tsh creates an interactive workspace without relay RPCs. |
| cmuxTests/CmuxSSHURLRequestTests.swift | Adds unit tests for parsing via=tsh (and rejecting invalid values) in deep links. |
| Sources/CmuxSSHURLRequest.swift | Introduces transport parsing (via) and includes transport in generated CLI arguments. |
| Sources/AppDelegate+CmuxSSHURL.swift | Adds user-facing error string mapping for invalid via transport. |
| Resources/Localizable.xcstrings | Updates cmux ssh usage text to include --via and adds a new localized error key. |
| CLI/cmux.swift | Implements the --via flag, adds Teleport interactive execution path, and updates CLI help text. |
Comments suppressed due to low confidence (2)
CLI/cmux.swift:1
tsh sshagent forwarding is currently enabled based on parsingForwardAgentfromoptions.sshOptions. If agent forwarding is enabled via a non--opath (e.g., an explicit-A/--forward-agentflag or other internal resolution that yieldsoptions.agentSocketPath), this may fail to append-Aeven thoughSSH_AUTH_SOCKis set in the workspace env. Consider deriving the-Adecision from the resolved forwarding outcome (e.g.,options.agentSocketPath != nilor a dedicated boolean) instead of re-parsing SSH options.
Resources/Localizable.xcstrings:1- Several non-English localizations now include new
--viahelp text in English and are markedneeds_review. If these strings are user-facing, consider providing translations for the added--vialines (or keeping the prior translations and omitting the new lines until translated) to avoid mixed-language CLI help in localized builds.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+9
to
+14
| | Surface | How | | ||
| | --- | --- | | ||
| | CLI | `cmux ssh --via tsh user@node` (default `--via ssh`) | | ||
| | Deep link (cmux scheme) | `cmux://ssh?host=node&user=admin&via=tsh` | | ||
| | Deep link (standard) | `ssh://admin@node?via=tsh` | | ||
| | Web fallback | `https://cmux.com/deeplink/ssh?host=node&user=admin&via=tsh` | |
Comment on lines
189
to
+203
| let sshOptions: [String] | ||
| switch structuredSSHOptions(from: queryItems) { | ||
| case .success(let options): | ||
| sshOptions = options | ||
| case .failure(let error): | ||
| return .failure(error) | ||
| } | ||
|
|
||
| let transport: CmuxSSHURLTransport | ||
| switch parsedTransport(in: queryItems) { | ||
| case .success(let value): | ||
| transport = value | ||
| case .failure(let error): | ||
| return .failure(error) | ||
| } |
Comment on lines
+1015
to
+1019
| case .invalidTransport(let parameter): | ||
| return String( | ||
| format: String(localized: "dialog.sshURL.error.invalidTransport", defaultValue: "The SSH link included an invalid transport for parameter: %@. Use ssh or tsh."), | ||
| parameter | ||
| ) |
Contributor
Greptile SummaryAdds an opt-in Teleport (
Confidence Score: 4/5Safe to merge for the functional SSH transport changes; the localization gaps mean users on 18 non-English/non-Japanese locales will see English copy in the web docs and a potentially empty or English alert for invalid deep-link transport values. The Swift transport logic, URL parsing, and integration tests are all well-structured. The outstanding gap is localization: flagVia/deepLinkVia are absent from 18 web locale files, and the dialog.sshURL.error.invalidTransport alert string has only en/ja entries in the string catalog. These are real gaps for shipped user-facing surfaces — the web docs page and the deep-link error alert — not theoretical future issues. Resources/Localizable.xcstrings (invalidTransport key missing 18 locales; help-text blobs for 18 locales are needs_review with untranslated English --via copy) and all web/messages/*.json files other than en.json and ja.json (flagVia and deepLinkVia keys absent). Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["cmux ssh …"] --> B{--via flag}
DL["Deep link\n(cmux:// or ssh://)"] --> C["CmuxSSHURLRequest.parse\n(parsedTransport)"]
C -->|"via=tsh"| E["cliArguments → --via tsh"]
C -->|"via=ssh / absent"| F["cliArguments → (no --via)"]
E --> A
F --> A
B -->|"tsh / teleport"| G["runInteractiveTeleportSSH"]
B -->|"ssh / openssh (default)"| H["runSSH (OpenSSH relay pipeline)"]
G --> I{"--identity present?"}
I -->|yes| J["CLIError: fast-fail"]
I -->|no| K["teleportSSHCommandArguments\n(tsh ssh -p -A -o …)"]
K --> L["workspace.create\ninitial_command: exec tsh ssh …"]
H --> M["generateRemoteRelayPort\ncmuxd-remote bootstrap\nworkspace.remote.configure\n…"]
|
![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 `@CLI/cmux.swift`:
- Around line 7991-8006: The teleportSSHCommandArguments function currently
drops unsupported OpenSSH-only options like identity without warning, causing
the session to be created with different settings than requested. Add validation
logic at the start of the teleportSSHCommandArguments function to detect when
unsupported first-class OpenSSH options have been set in the SSHCommandOptions
parameter (such as identity), and throw an error or return a failure result
before attempting to build the command arguments. This ensures the function
fails fast rather than silently ignoring critical options that the user
explicitly requested.
🪄 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: 39352621-5304-45e8-9d73-e6e3899bf2ff
📒 Files selected for processing (10)
CLI/cmux.swiftResources/Localizable.xcstringsSources/AppDelegate+CmuxSSHURL.swiftSources/CmuxSSHURLRequest.swiftcmuxTests/CmuxSSHURLRequestTests.swiftcmuxTests/VMSSHCommandTests.swiftdocs/ssh-teleport.mdweb/app/[locale]/docs/ssh/page.tsxweb/messages/en.jsonweb/messages/ja.json
Per code review (CodeRabbit): teleportSSHCommandArguments forwarded only port, agent forwarding, -o passthroughs, destination, and trailing args, so a first-class OpenSSH option like --identity was silently discarded for the tsh transport — opening a session with a different auth path than requested. tsh authenticates via 'tsh login' certificates, so an identity file is meaningless. runInteractiveTeleportSSH now fails fast with a clear error before any RPC when --identity is set, rather than ignoring it. Adds a CLI regression test asserting the rejection sends no workspace.create. Other review findings were verified and intentionally skipped (forwarded -o options surface tsh's own error rather than being dropped; -A reflects explicit ForwardAgent intent; needs_review locale strings follow the existing help-text convention).
Author
|
Reviewed all CodeRabbit/Copilot findings against the current code. Fixed the one still-valid issue; skipping the rest with reasons. Fixed (
Verified & skipped
|
teamleaderleo
added
area: remote
This branch has not been 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
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
Adds an explicit, opt-in Teleport (
tsh) SSH transport tocmux ssh, exposed consistently across every shared entrypoint.--via tsh(defaultssh) opens a workspace whose terminalexecstsh ssh <dest>directly — interactive-only, with no cmux remote daemon (no relay, nocmuxd-remote, noworkspace.remote.configure).tshcannot honor the OpenSSH options the relay depends on (RemoteCommand,ControlMaster,SetEnv,LocalCommand,-i,-tt), so the emitted command is minimal: binary,-p,-A(when ForwardAgent),-opassthroughs, destination, and any--remote args. The OpenSSH path is left completely untouched.Entrypoints (one shared path)
cmux ssh --via <ssh|tsh> user@node+runInteractiveTeleportSSH(localized--viahelp).viaquery param on the cmux scheme (cmux://ssh?host=…&via=tsh) and standardssh://admin@node?via=tsh, emitting--via tshthroughCmuxSSHURLRequest.cliArguments. New localizedinvalidTransportparse error.--viaflag row +viadeep-link param (en/ja messages).sidebar.showSSHis display-only); noted, not silently built.Commits (red → green)
--viaflag.Testing
cmux-unitlocally: CLI integration test + 4 newCmuxSSHURLRequestTestspass (57 in that class, 0 failures).Notes / limitations
--via tshworkspaces.docs/ssh-teleport.mddocuments a futuretsh ssh -Rrelay path — the key open question is whether the target Teleport version/cluster supports remote (-R) forwarding (verify empirically first).Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds an opt‑in Teleport SSH transport to
cmux sshvia--via tshand deep links. It opens a workspace that execstsh sshfor an interactive session with no relay orcmuxd-remote, and now fails fast if--identityis passed.New Features
--via <ssh|tsh>(defaultssh).--via tshbuilds a minimaltsh sshcommand (-p,-Awhen requested,-opassthroughs, destination,--args); OpenSSH path is unchanged.via=tshincmux://sshandssh://URLs; invalid values show a localized error. All links funnel throughCmuxSSHURLRequest.cliArgumentswhich emits--via tsh.--viaandviaparam to SSH docs and web messages (en/ja); newdocs/ssh-teleport.mdexplains behavior and future-Rinvestigation.tsh(use oftsh ssh, noworkspace.remote.configure/relay); add regression test asserting--via tshrejects--identity.Bug Fixes
--via tshrejects--identitywith a clear error and performs no RPC, avoiding silent auth mismatches.Written for commit bf95a8b. Summary will update on new commits.
Summary by CodeRabbit
--via <ssh|tsh>(defaultssh) to route connections via OpenSSH or Teleport’stsh.viain SSH deep links.via/transport values with localized messaging.via/deep-link strings.ssh --via tshflows.