Repository navigation
Add Rust cloud CLI entrypoint - #4601
lawrencecchen wants to merge 11 commits into
Conversation
|
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.
|
|
Important Review skippedWe couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting You can disable this status message by setting the Use the checkbox below for a quick retry:
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 introduces ChangesCloud CLI Implementation and Integration
Sequence DiagramsequenceDiagram
participant User
participant CMux as cmux (Swift)
participant Cloud as cmux-cloud (Rust)
participant Socket as UnixSocketDaemon
User->>CMux: cmux cloud new --image <image>
CMux->>CMux: resolve cmux-cloud executable path & env
CMux->>Cloud: execv(cmux-cloud, env: CMUX_CLOUD_SOCKET_PATH, CMUX_CLOUD_SOCKET_PASSWORD, CMUX_CLOUD_JSON, CMUX_CLOUD_WINDOW, ...)
Cloud->>Cloud: parse argv, CloudContext::from_env
Cloud->>Socket: connect (with retry & timeouts)
Socket->>Cloud: v2 RPC responses / auth probe results
Cloud->>Socket: v2 RPC request: vm.create / vm.exec / vm.list
Cloud->>Cloud: persist idempotency record (when applicable)
Cloud->>CMux: execv parent cmux vm (when delegating interactive flows)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (3 errors, 1 warning)
✅ Passed checks (13 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryAdds a Rust-backed
Confidence Score: 5/5Safe to merge; the new Rust binary, Swift routing, and build integration are all well-structured and the previously flagged issues have been addressed. The exec/signal semantics, password-in-argv, and remote exit-code issues raised in earlier rounds are all fixed. The Cargo.lock dependencies (zmij, serde_core) are legitimate crates. The only finding is that 18 non-Japanese locales in the new xcstrings entries carry English copy marked as 'translated' instead of real translations — a localization quality issue rather than a functional defect. Resources/Localizable.xcstrings — the two new string keys should have actual translations for all supported locales before shipping to non-English users. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Swift as cmux (Swift)
participant Rust as cmux-cloud (Rust)
participant Socket as cmux daemon
User->>Swift: cmux cloud subcommand
Swift->>Swift: isRelaySocketPath()?
alt relay socket
Swift->>Socket: "vm.* (existing Swift path)"
Socket-->>Swift: response
Swift-->>User: output
else local socket
Swift->>Rust: "execv(cmux-cloud, env: CMUX_CLOUD_*)"
Note over Swift,Rust: process image replaced
alt ls / new / rm / exec / ssh-info
Rust->>Socket: v2 RPC (vm.list / vm.create / etc.)
Socket-->>Rust: response
Rust-->>User: output
else shell / attach / ssh
Rust->>Swift: exec(cmux vm shell id)
Note over Rust,Swift: process image replaced
Swift->>Socket: vm.shell / attach
Socket-->>User: interactive session
end
opt cloud new non-detach
Note over Rust,Swift: sets CMUX_CLOUD_CLEAR_IDEMPOTENCY_*
Swift->>Swift: clearVMCreateIdempotencyFromCloudEnvironment()
end
end
Reviews (6): Last reviewed commit: "fix: retry cloud cli socket connect" | 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 `@CLI/cmux.swift`:
- Around line 2650-2655: The loop over candidates incorrectly treats directories
with traverse permission as executables; update the check in the candidates
iteration (the code using variable candidate and path and calling
fileManager.isExecutableFile(atPath:)) to first call
fileManager.fileExists(atPath:isDirectory:) (or equivalent) and ensure
isDirectory == false before calling isExecutableFile(atPath:), returning the
path only if it is not a directory and is executable.
🪄 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: 37a817f1-dd8e-4e11-8981-dfc93880bf38
⛔ Files ignored due to path filters (1)
Native/CloudCLI/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.github/workflows/nightly.yml.github/workflows/release.ymlCLI/cmux.swiftNative/CloudCLI/Cargo.tomlNative/CloudCLI/src/main.rscmux.xcodeproj/project.pbxprojdocs/cli-contract.mdscripts/build-cloud-cli.sh
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?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`:
- Line 2698: The CLI error messages in CLI/cmux.swift (e.g., the user-facing
strings produced by the CLIError cases around the CLIError enum/throw sites
at/near the symbols mentioned on lines ~2698 and ~2735) are hardcoded English;
update those messages to use Swift's localized API (for example
String(localized:defaultValue:)) instead of raw literals and add matching
entries to the string catalog, or if localization is intentionally exempt, add a
short documented exemption comment near the CLIError declaration; ensure every
user-facing string in the CLIError cases uses the localized API with a
corresponding catalog key.
🪄 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: 5ef0623d-4821-4ab3-a274-e49483e56815
📒 Files selected for processing (3)
CLI/cmux.swiftNative/CloudCLI/src/main.rscmux.xcodeproj/project.pbxproj
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)
Native/CloudCLI/src/main.rs (1)
418-425:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftClear the idempotency record after a successful interactive handoff.
This path never clears the persisted key on success.
exec_parent_vm()callsCommand::exec(), so once the delegatedshellstarts, control never returns here to remove the record. A secondcmux cloud newwith the same image/provider inside the 10-minute TTL will therefore reuse the previous VM instead of creating a new one. Add a success-path cleanup mechanism here, or hand cleanup responsibility to the delegated flow.Also applies to: 583-589
🤖 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 `@Native/CloudCLI/src/main.rs` around lines 418 - 425, The idempotency key persists because exec_parent_vm() calls Command::exec() and never returns, so clear the persisted idempotency record before handing off to the delegated shell: add a call to the same cleanup helper used to create the key (e.g. remove_idempotency_record(...) / clear_idempotency_key(...)) right before exec_parent_vm(ctx, &vm_args) in the block that constructs vm_args (using id and target_window), and mirror the same pre-exec cleanup in the other delegated handoff path that also calls exec_parent_vm (the similar block later in the file). Ensure the cleanup call succeeds (log/ignore errors) before calling exec_parent_vm so the persisted key is not reused by subsequent runs.
🤖 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 `@Native/CloudCLI/src/main.rs`:
- Around line 418-425: The idempotency key persists because exec_parent_vm()
calls Command::exec() and never returns, so clear the persisted idempotency
record before handing off to the delegated shell: add a call to the same cleanup
helper used to create the key (e.g. remove_idempotency_record(...) /
clear_idempotency_key(...)) right before exec_parent_vm(ctx, &vm_args) in the
block that constructs vm_args (using id and target_window), and mirror the same
pre-exec cleanup in the other delegated handoff path that also calls
exec_parent_vm (the similar block later in the file). Ensure the cleanup call
succeeds (log/ignore errors) before calling exec_parent_vm so the persisted key
is not reused by subsequent runs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5bb43e91-64eb-4f51-a4d1-981cdebd2461
📒 Files selected for processing (2)
CLI/cmux.swiftNative/CloudCLI/src/main.rs
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0cc0c60. Configure here.
|
All contributors have signed the CLA ✍️ ✅ |
|
Mac fleet instructions for head JOB_JSON=$(~/.local/bin/cmux-ci submit --kind cmux --command 'CMUX_FLEET_BUILD_TAG=pr-4601-214ed4a6 /Users/Shared/cmux-build-fleet/recipes/cmux.sh https://github.com/manaflow-ai/cmux.git 214ed4a6357ea651b98952a464a83cc3009efb8f' --artifact artifacts/cmux.app.zip --workspace https://github.com/manaflow-ai/cmux/pull/4601 --source-digest 214ed4a6357ea651b98952a464a83cc3009efb8f --cache-key cmux:pr-4601 --min-free-bytes 268435456000 --label cmux --label ram48)
JOB_ID=$(python3 -c 'import json,sys; print(json.load(sys.stdin)["id"])' <<<"$JOB_JSON")
~/.local/bin/cmux-ci wait "$JOB_ID" --receipt artifacts/fleet/$JOB_ID.json
~/.local/bin/cmux-ci publish-hq "$JOB_ID"Use an existing campaign job ID if one is already posted; do not submit a duplicate. A wait timeout leaves the remote job running. Published results will include an exact-head artifact link and timing/disk receipt. This recipe validates the macOS app only, not iOS or tests. Never use maclease or put credentials in a PR comment. |
|
Automatic catch-up: I tried to catch this branch up with
Nothing was pushed. Merge Automatic catch-up will not try this head again; a new push or |
|
Automatic catch-up: I tried to catch this branch up with
Nothing was pushed. Merge Automatic catch-up will not try this head again; a new push or |
teamleaderleo
added
S2: major
This branch was successfully deployed
2 participants
Summary
cmux-cloudhelper for thecmux cloudcommand.cmuxlauncher exec the bundled helper forcmux cloudwhile keepingcmux vmas the compatibility command.cmux-cloudin Xcode, nightly, and release flows.Testing
cargo fmt --manifest-path Native/CloudCLI/Cargo.toml --checkcargo test --manifest-path Native/CloudCLI/Cargo.toml./scripts/build-cloud-cli.sh./scripts/reload.sh --tag cloudclicmux cloud --help,cmux cloud new --provider badCMUX_CLI_BIN=... python3 tests/test_cli_contract_help.pyIssues
cmux cloudNeed help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Introduces a new bundled Rust executable and changes the
cloudCLI dispatch path toexecvinto it, which can affect core VM workflows and release/nightly packaging if the helper build/lookup/env plumbing is wrong.Overview
Makes
cmux clouda Rust-backed entrypoint by adding a bundledcmux-cloudhelper (newNative/CloudCLIcrate) and teaching the Swift launcher toexecvinto it for non-relay sockets, passing socket/password/window/id-format context viaCMUX_CLOUD_*env.Keeps
cmux vmas a compatibility alias, updates help text anddocs/cli-contract.md, and adds localized error strings for missing/failed helper launches.Adds an Xcode build phase plus
scripts/build-cloud-cli.shto build/sign a universal (arm64+x86_64)cmux-cloudbinary, and extends nightly/release CI to verify the helper’s architectures alongside existing binaries.Reviewed by Cursor Bugbot for commit 4a2ff4f. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
cmux cloudnow uses the bundled Rustcmux-cloudhelper for local sockets instead of the Swift VM path; relay sockets keep the Swift path.cloudis now the primary command, whilevmremains a compatibility alias.Details
rm,exec, andssh-info, retries transient socket failures, focuses the requested window, and redacts socket credentials from errors.CMUX_CLOUD_*;CMUX_CLOUD_CLI_PATHcan override the bundled executable.cloud newremains idempotent, supports--detach, and clears its idempotency record after attaching.Build and docs
scripts/build-cloud-cli.shbuild and bundle a signed universalcmux-cloudbinary.arm64andx86_64slices, and the CLI contract documents the new dispatch behavior.Written for commit 214ed4a. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Chores
Localization