Repository navigation
Add --no-focus flag to cmux ssh - #2227
Conversation
When cmux ssh is used from a script, workspace.select is called immediately after workspace.remote.configure, stealing the user's active workspace focus before SSH is established. Add --no-focus flag (consistent with break-pane/join-pane) to skip the workspace.select call so the caller's workspace retains focus. The caller can then redirect to the new workspace later via cmux select-workspace. Addresses cmux ssh case of #140; complementary to #1418.
- Fix existing test that constructs SSHCommandOptions directly (add noFocus: false to the initializer call) - Make parseSSHCommandOptions internal so it's accessible from tests - Add testParseSSHCommandOptionsNoFocusFlag covering: - flag sets noFocus=true, destination parses correctly - absent flag defaults noFocus=false - combined with --name and --port
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CMUX CLI
participant Remote as workspace.remote.configure
participant Selector as workspace.select
CLI->>Remote: create/configure remote workspace (includes sshOptions)
Remote-->>CLI: remote workspace created
alt sshOptions.noFocus == false
CLI->>Selector: workspace.select(new remote)
Selector-->>CLI: selection confirmed
else sshOptions.noFocus == true
CLI-->>CLI: skip workspace.select
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Greptile SummaryThis PR adds a Confidence Score: 5/5Safe to merge — the only finding is a non-breaking style issue (dropped The feature is a straightforward additive flag with correct default behavior. The single P2 comment (access modifier widening) has zero runtime impact and can be addressed in a trivial follow-up. All existing No files require special attention beyond the single-line access modifier fix in Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant cmux CLI
participant SocketClient
participant cmux App
User->>cmux CLI: cmux ssh user@host [--no-focus]
cmux CLI->>cmux CLI: parseSSHCommandOptions()<br/>sets noFocus=true/false
cmux CLI->>SocketClient: workspace.create
SocketClient->>cmux App: workspace.create
cmux App-->>SocketClient: workspaceId
SocketClient-->>cmux CLI: workspaceId
cmux CLI->>SocketClient: workspace.remote.configure
SocketClient->>cmux App: workspace.remote.configure
cmux App-->>SocketClient: configuredPayload
SocketClient-->>cmux CLI: configuredPayload
alt --no-focus NOT passed (default)
cmux CLI->>SocketClient: workspace.select
SocketClient->>cmux App: workspace.select (focus switches)
else --no-focus passed
cmux CLI->>cmux CLI: skip workspace.select (focus preserved)
end
cmux CLI->>User: OK workspace=... target=... state=...
Reviews (1): Last reviewed commit: "CLI: add test coverage for --no-focus fl..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?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
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)
6409-6427:⚠️ Potential issue | 🟡 MinorTop-level
cmux --helpis still stale forssh.This subcommand help is updated, but Line 12620 in
CMUXCLI.usage()still shows the old synopsis, so the new flag is invisible from the main help output.📝 Proposed doc fix
- ssh <destination> [--name <title>] [--port <n>] [--identity <path>] [--ssh-option <opt>] [-- <remote-command-args>] + ssh <destination> [--name <title>] [--port <n>] [--identity <path>] [--ssh-option <opt>] [--no-focus] [-- <remote-command-args>]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLI/cmux.swift` around lines 6409 - 6427, Update the top-level help text in CMUXCLI.usage() to match the revised ssh subcommand synopsis and flags: replace the stale "ssh" usage entry with the new synopsis "cmux ssh <destination> [flags] [-- <remote-command-args>]" and include the flags --name, --port, --identity, --ssh-option, and --no-focus (and the brief descriptions) so the main `cmux --help` shows the same options as the ssh subcommand help; look for the "ssh" case or entry in CMUXCLI.usage() and make the text consistent with the ssh help block in cmux.swift.
🧹 Nitpick comments (1)
CLI/cmux.swift (1)
3811-3899: Please add a runtime test for the--no-focusbranch.The parser coverage is useful, but
tests_v2/test_ssh_remote_cli_metadata.pystill only asserts that the new workspace becomes selected. There is no end-to-end assertion thatcmux ssh ... --no-focusleavescurrent_workspace()unchanged.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLI/cmux.swift` around lines 3811 - 3899, The test suite is missing an end-to-end runtime test that verifies the --no-focus flag leaves the selected workspace unchanged; add a new test in tests_v2/test_ssh_remote_cli_metadata.py that records the current_workspace() before running the CLI, invokes the ssh command (using the same helpers already used in the file) with the --no-focus flag and a destination, and then asserts current_workspace() is identical after the call; target the behavior produced by parseSSHCommandOptions (function name) by passing --no-focus and ensure the created SSHCommandOptions leads to no change in selection (use existing CLI invocation helpers and current_workspace() accessor in the test).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLI/cmux.swift`:
- Around line 3763-3768: The code currently calls
client.sendV2("workspace.select", params: selectParams) unless
sshOptions.noFocus is true, which makes focus the default; change this to make
focus opt-in by flipping the flag semantics: add or use an explicit
sshOptions.focus boolean (or invert sshOptions.noFocus to focus) and only call
client.sendV2(method: "workspace.select", params: selectParams) when that
explicit focus flag is true; update the option parsing/CLI flag for `cmux ssh`
to provide a `--focus` (default false) or invert the existing flag behavior, and
adjust any docs/tests that mention --no-focus accordingly so workspace.select is
only invoked when the user explicitly requests focus.
---
Outside diff comments:
In `@CLI/cmux.swift`:
- Around line 6409-6427: Update the top-level help text in CMUXCLI.usage() to
match the revised ssh subcommand synopsis and flags: replace the stale "ssh"
usage entry with the new synopsis "cmux ssh <destination> [flags] [--
<remote-command-args>]" and include the flags --name, --port, --identity,
--ssh-option, and --no-focus (and the brief descriptions) so the main `cmux
--help` shows the same options as the ssh subcommand help; look for the "ssh"
case or entry in CMUXCLI.usage() and make the text consistent with the ssh help
block in cmux.swift.
---
Nitpick comments:
In `@CLI/cmux.swift`:
- Around line 3811-3899: The test suite is missing an end-to-end runtime test
that verifies the --no-focus flag leaves the selected workspace unchanged; add a
new test in tests_v2/test_ssh_remote_cli_metadata.py that records the
current_workspace() before running the CLI, invokes the ssh command (using the
same helpers already used in the file) with the --no-focus flag and a
destination, and then asserts current_workspace() is identical after the call;
target the behavior produced by parseSSHCommandOptions (function name) by
passing --no-focus and ensure the created SSHCommandOptions leads to no change
in selection (use existing CLI invocation helpers and current_workspace()
accessor in the test).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| // `cmux ssh` is an explicit "open this remote workspace now" action, | ||
| // so we intentionally select the newly created workspace after wiring | ||
| // up the remote connection. | ||
| _ = try client.sendV2(method: "workspace.select", params: selectParams) | ||
| // up the remote connection — unless --no-focus is passed. | ||
| if !sshOptions.noFocus { | ||
| _ = try client.sendV2(method: "workspace.select", params: selectParams) | ||
| } |
There was a problem hiding this comment.
Make focus opt-in here, not opt-out.
cmux ssh still calls workspace.select by default, so scripted SSH workspace creation continues to steal selection unless callers know to pass --no-focus. For a non-focus command, the safe behavior should be the default and any focus change should be explicit.
Based on learnings: Socket/CLI commands must not steal macOS app focus; only explicit focus-intent commands may mutate in-app focus/selection.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLI/cmux.swift` around lines 3763 - 3768, The code currently calls
client.sendV2("workspace.select", params: selectParams) unless
sshOptions.noFocus is true, which makes focus the default; change this to make
focus opt-in by flipping the flag semantics: add or use an explicit
sshOptions.focus boolean (or invert sshOptions.noFocus to focus) and only call
client.sendV2(method: "workspace.select", params: selectParams) when that
explicit focus flag is true; update the option parsing/CLI flag for `cmux ssh`
to provide a `--focus` (default false) or invert the existing flag behavior, and
adjust any docs/tests that mention --no-focus accordingly so workspace.select is
only invoked when the user explicitly requests focus.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLI/cmux.swift`:
- Line 6421: The top-level help synopsis is missing the new --no-focus flag
added to the ssh subcommand; update the help text generator so the synopsis for
the ssh subcommand matches subcommandUsage("ssh"). Locate the top-level
usage/summary renderer (the help printing function or constant that builds the
main synopsis string in CLI/cmux.swift) and add the --no-focus flag to the ssh
entry so that both cmux help and cmux ssh --help show identical options.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
♻️ Duplicate comments (1)
CLI/cmux.swift (1)
3763-3768:⚠️ Potential issue | 🟠 MajorMake SSH workspace selection explicit, not default.
cmux sshstill callsworkspace.selectby default (Line 3767), so focus/selection changes happen unless callers know to pass--no-focus. This still violates the non-focus-command policy.♻️ Proposed direction
- // ... unless --no-focus is passed. - if !sshOptions.noFocus { + // Only mutate selection when explicitly requested. + if sshOptions.focus { _ = try client.sendV2(method: "workspace.select", params: selectParams) }Then parse and document an explicit
--focusflag (default false), and treat--no-focusas legacy compatibility if needed.Based on learnings: Socket/CLI commands must not steal macOS app focus; only explicit focus-intent commands may mutate in-app focus/selection (
window.focus,workspace.select/next/previous/last,surface.focus,pane.focus/last, etc.), and non-focus commands should preserve current user focus context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLI/cmux.swift` around lines 3763 - 3768, The CLI currently calls client.sendV2(method: "workspace.select", ...) whenever !sshOptions.noFocus, which still mutates app focus by default; change this to only call workspace.select when an explicit --focus flag is passed (e.g., add sshOptions.focus: Bool default false and use if sshOptions.focus { _ = try client.sendV2(method: "workspace.select", params: selectParams) }); keep --no-focus as a legacy alias mapping to focus = false if you must, and update the CLI flag parsing and help/docs to document the new --focus explicit intent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@CLI/cmux.swift`:
- Around line 3763-3768: The CLI currently calls client.sendV2(method:
"workspace.select", ...) whenever !sshOptions.noFocus, which still mutates app
focus by default; change this to only call workspace.select when an explicit
--focus flag is passed (e.g., add sshOptions.focus: Bool default false and use
if sshOptions.focus { _ = try client.sendV2(method: "workspace.select", params:
selectParams) }); keep --no-focus as a legacy alias mapping to focus = false if
you must, and update the CLI flag parsing and help/docs to document the new
--focus explicit intent.
* CLI: add --no-focus flag to cmux ssh When cmux ssh is used from a script, workspace.select is called immediately after workspace.remote.configure, stealing the user's active workspace focus before SSH is established. Add --no-focus flag (consistent with break-pane/join-pane) to skip the workspace.select call so the caller's workspace retains focus. The caller can then redirect to the new workspace later via cmux select-workspace. Addresses cmux ssh case of manaflow-ai#140; complementary to manaflow-ai#1418. * CLI: add test coverage for --no-focus flag parsing - Fix existing test that constructs SSHCommandOptions directly (add noFocus: false to the initializer call) - Make parseSSHCommandOptions internal so it's accessible from tests - Add testParseSSHCommandOptionsNoFocusFlag covering: - flag sets noFocus=true, destination parses correctly - absent flag defaults noFocus=false - combined with --name and --port * Update CLI/cmux.swift Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * Add --no-focus to top-level ssh help synopsis --------- Co-authored-by: Łukasz Majcher <lukasz.majcher@samsara.com> Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
This branch was successfully deployed
2 participants
Summary
Add
--no-focusflag tocmux sshso scripted/background SSH workspace creation does not steal the user's active workspace focus.Cherry-picked from #1942 by @maucher, rebased onto main with conflict resolution.
Changes
SSHCommandOptions: addnoFocus: BoolfieldparseSSHCommandOptions: parse--no-focusflagrunSSH: guardworkspace.selectcall withif !sshOptions.noFocus--no-focusflagTest plan
cmux ssh <host> --no-focuscreates SSH workspace without switching focuscmux ssh <host>(without flag) still switches focus as before--no-focusalongside other flags (--name,--port) parses correctlySummary by cubic
Adds a
--no-focusflag tocmux sshso scripts can create SSH workspaces without switching the active workspace. Default behavior remains unchanged.--no-focusintoSSHCommandOptions.noFocus.workspace.selectwhen--no-focusis set.sshsynopsis to include the flag.Written for commit 66e132d. Summary will update on new commits.
Summary by CodeRabbit
New Features
--no-focusoption to the SSH command so users can create/configure a remote workspace without automatically selecting or focusing it.Documentation
--no-focusoption and its effect.