feat(cua-driver-rs): updater delegates to canonical install script (Windows + Unix) - #1673
Conversation
…indows + Unix) `cua-driver update --apply` previously shelled out to `bash -c "curl ... install.sh | bash"` — broken on Windows (no bash by default) and not in sync with the canonical installer URL. Replace with a thin per-platform shim that invokes the documented install one-liner with `CUA_DRIVER_RS_VERSION` pinned to the version `version_check::fetch_latest_version` just resolved: - Unix: curl -fsSL https://.../install.sh | bash -s -- install --backend=rust - Windows: irm https://.../install.ps1 | iex Why delegate rather than reimplement in Rust? The install scripts already own everything an updater needs: - target-triple → asset name mapping - per-version dir layout (`packages/releases/<version>-<target>/`) - atomic upgrade — symlink retarget on Unix, NTFS junction flip on Windows. A running daemon survives the swap because the kernel keeps the old inode alive (Unix) or the junction flip doesn't touch the locked .exe (Windows). - GC of stale per-version dirs (`CUA_DRIVER_RS_KEEP_VERSIONS`) - PATH wiring Treating "update" as a pinned re-install keeps install + update reading from one source of truth. New asset names / new platforms / better GC strategies ship in the scripts and benefit both paths automatically. Post-install enhancements: - Detects a running daemon and tells the user to restart it (the atomic swap means the running process kept executing the old binary on disk; a `cua-driver stop && cua-driver serve` picks up the new one). - The "or reinstall directly:" hint now prints the platform-correct one-liner instead of the always-Unix `curl ... | bash`.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughThis PR refactors the ChangesInstaller script delegation via updater module
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@libs/cua-driver-rs/crates/cua-driver/src/cli.rs`:
- Around line 1127-1133: The restart hint currently prints a single line using
shell && which isn't supported in PowerShell 5.1; update the daemon_was_running
branch in cli.rs (the block that prints the restart instructions) to detect
Windows (e.g., cfg!(windows) or cfg!(target_os = "windows")) and, on Windows,
print two separate lines (" cua-driver stop" and " cua-driver serve") instead
of the combined "cua-driver stop && cua-driver serve", leaving the original
single-line message for non-Windows platforms.
In `@libs/cua-driver-rs/crates/cua-driver/src/updater.rs`:
- Around line 70-76: The pipeline using bash_cmd ("curl -fsSL
{CANONICAL_INSTALL_SH} | bash -s -- install --backend=rust") can hide curl
failures because pipefail is not set; update the invocation in
Command::new("bash") so the shell runs with pipefail (either by passing bash the
-o pipefail option before -c or by prefixing bash_cmd with "set -o pipefail;")
so that Command::new("bash").args([... "-c", &bash_cmd]).status() propagates
curl errors; change the args call that currently uses ["-c", &bash_cmd] to
include pipefail and keep VERSION_PIN_ENV usage the same.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8834c324-37bd-4b92-968a-a8fec076284a
📒 Files selected for processing (3)
libs/cua-driver-rs/crates/cua-driver/src/cli.rslibs/cua-driver-rs/crates/cua-driver/src/main.rslibs/cua-driver-rs/crates/cua-driver/src/updater.rs
| if daemon_was_running { | ||
| // The atomic swap (symlink retarget / junction flip) | ||
| // means the running daemon kept executing the old | ||
| // binary — restart picks up the new one. | ||
| println!(); | ||
| println!("A daemon was running before the install. Restart it to pick up the new binary:"); | ||
| println!(" cua-driver stop && cua-driver serve"); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Windows PowerShell 5.1 support the && command chaining operator, or is it only available in PowerShell 7+?
💡 Result:
Windows PowerShell 5.1 does not support the && pipeline chaining operator. The && (and ||) pipeline chain operators were added beginning in PowerShell 7. [1] Microsoft’s documentation for about_Pipeline_Chain_Operators states: “Beginning in PowerShell 7, PowerShell implements the && and || operators to conditionally chain pipelines.” [1] Therefore, Windows PowerShell 5.1 (the “Windows PowerShell” line) lacks && support and you must use other constructs (for example, -and between expressions) or upgrade to PowerShell 7+ to use &&. [1] Practical implication: in PowerShell 5.1, command1 && command2 is not valid pipeline chaining syntax; in PowerShell 7+, it conditionally runs command2 only if the left-hand pipeline succeeded. [1]
Citations:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant region around the reported line numbers
sed -n '1100,1160p' libs/cua-driver-rs/crates/cua-driver/src/cli.rs
# Also grab the function name + surrounding context for style consistency
rg -n "fn run_update_cmd|run_update_cmd" libs/cua-driver-rs/crates/cua-driver/src/cli.rsRepository: trycua/cua
Length of output: 2889
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find Windows PowerShell invocation in updater.rs
fd -a "updater.rs" libs/cua-driver-rs/crates/cua-driver/src | head -n 20
# Search for powershell usage in the updater module(s)
rg -n "powershell\.exe|powershell" libs/cua-driver-rs/crates/cua-driver/src
# Also confirm where the restart hint string appears (single source of truth)
rg -n "A daemon was running before the install\. Restart it" -S libs/cua-driver-rs/crates/cua-driver/src/cli.rsRepository: trycua/cua
Length of output: 1582
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show relevant parts of updater.rs around powershell invocation and install scripting
sed -n '1,140p' libs/cua-driver-rs/crates/cua-driver/src/updater.rs
# Find the function that runs the install script and print its body region
rg -n "fn run_install_script|run_install_script\\(" libs/cua-driver-rs/crates/cua-driver/src/updater.rs
sed -n '140,320p' libs/cua-driver-rs/crates/cua-driver/src/updater.rs
# Check if there are other user-facing command hints for Windows shell syntax
rg -n "A daemon was running|Restart it to pick up|manual_install_one_liner" libs/cua-driver-rs/crates/cua-driver/src/cli.rs
rg -n "manual_install_one_liner" libs/cua-driver-rs/crates/cua-driver/src/updater.rs libs/cua-driver-rs/crates/cua-driver/src/cli.rsRepository: trycua/cua
Length of output: 5182
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "&&" libs/cua-driver-rs/crates/cua-driver/src/cli.rs
rg -n "cfg\\(windows\\)|cfg\\(not\\(windows\\)\\)" libs/cua-driver-rs/crates/cua-driver/src/cli.rs | head -n 50Repository: trycua/cua
Length of output: 773
Make the restart hint Windows PowerShell 5.1-compatible.
cua-driver stop && cua-driver serve uses &&, which isn’t supported by Windows PowerShell 5.1 (it was added in PowerShell 7). Print two separate commands on Windows instead.
File: libs/cua-driver-rs/crates/cua-driver/src/cli.rs (around the restart message)
Suggested fix
if daemon_was_running {
println!();
println!("A daemon was running before the install. Restart it to pick up the new binary:");
- println!(" cua-driver stop && cua-driver serve");
+ #[cfg(windows)]
+ {
+ println!(" cua-driver stop");
+ println!(" cua-driver serve");
+ }
+ #[cfg(not(windows))]
+ {
+ println!(" cua-driver stop && cua-driver serve");
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if daemon_was_running { | |
| // The atomic swap (symlink retarget / junction flip) | |
| // means the running daemon kept executing the old | |
| // binary — restart picks up the new one. | |
| println!(); | |
| println!("A daemon was running before the install. Restart it to pick up the new binary:"); | |
| println!(" cua-driver stop && cua-driver serve"); | |
| if daemon_was_running { | |
| // The atomic swap (symlink retarget / junction flip) | |
| // means the running daemon kept executing the old | |
| // binary — restart picks up the new one. | |
| println!(); | |
| println!("A daemon was running before the install. Restart it to pick up the new binary:"); | |
| #[cfg(windows)] | |
| { | |
| println!(" cua-driver stop"); | |
| println!(" cua-driver serve"); | |
| } | |
| #[cfg(not(windows))] | |
| { | |
| println!(" cua-driver stop && cua-driver serve"); | |
| } |
🤖 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 `@libs/cua-driver-rs/crates/cua-driver/src/cli.rs` around lines 1127 - 1133,
The restart hint currently prints a single line using shell && which isn't
supported in PowerShell 5.1; update the daemon_was_running branch in cli.rs (the
block that prints the restart instructions) to detect Windows (e.g.,
cfg!(windows) or cfg!(target_os = "windows")) and, on Windows, print two
separate lines (" cua-driver stop" and " cua-driver serve") instead of the
combined "cua-driver stop && cua-driver serve", leaving the original single-line
message for non-Windows platforms.
| let bash_cmd = format!( | ||
| "curl -fsSL {CANONICAL_INSTALL_SH} | bash -s -- install --backend=rust" | ||
| ); | ||
| Command::new("bash") | ||
| .env(VERSION_PIN_ENV, version) | ||
| .args(["-c", &bash_cmd]) | ||
| .status() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf 'without pipefail:\n'
bash -c 'missing_cmd | bash -s -- install --backend=rust; printf "exit=%s\n" "$?"'
printf '\nwith pipefail:\n'
bash -o pipefail -c 'missing_cmd | bash -s -- install --backend=rust; printf "exit=%s\n" "$?"'Repository: trycua/cua
Length of output: 212
🏁 Script executed:
#!/bin/bash
set -eu
printf 'without pipefail:\n'
bash -c 'missing_cmd | bash -s -- install --backend=rust; printf "exit=%s\n" "$?"'
printf '\nwith pipefail:\n'
bash -o pipefail -c 'missing_cmd | bash -s -- install --backend=rust; printf "exit=%s\n" "$?"'Repository: trycua/cua
Length of output: 212
🏁 Script executed:
#!/bin/bash
set -eu
printf 'without pipefail:\n'
bash -c 'missing_cmd | bash -s -- install --backend=rust; printf "exit=%s\n" "$?"'
printf '\nwith pipefail:\n'
bash -o pipefail -c 'missing_cmd | bash -s -- install --backend=rust; printf "exit=%s\n" "$?"'Repository: trycua/cua
Length of output: 212
Propagate curl failures out of the Unix install pipeline (add pipefail).
The Unix path uses curl -fsSL ... | bash -s -- ... and currently returns the RHS bash status; without pipefail, failures in curl can be masked. Shell repro: missing_cmd | bash -s -- ... => exit 0, while bash -o pipefail -c 'missing_cmd | ...' => exit 127.
Suggested fix
- Command::new("bash")
- .env(VERSION_PIN_ENV, version)
- .args(["-c", &bash_cmd])
+ Command::new("bash")
+ .env(VERSION_PIN_ENV, version)
+ .args(["-o", "pipefail", "-c", &bash_cmd])
.status()🤖 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 `@libs/cua-driver-rs/crates/cua-driver/src/updater.rs` around lines 70 - 76,
The pipeline using bash_cmd ("curl -fsSL {CANONICAL_INSTALL_SH} | bash -s --
install --backend=rust") can hide curl failures because pipefail is not set;
update the invocation in Command::new("bash") so the shell runs with pipefail
(either by passing bash the -o pipefail option before -c or by prefixing
bash_cmd with "set -o pipefail;") so that Command::new("bash").args([... "-c",
&bash_cmd]).status() propagates curl errors; change the args call that currently
uses ["-c", &bash_cmd] to include pipefail and keep VERSION_PIN_ENV usage the
same.
1 participant
Summary
`cua-driver update --apply` previously shelled out to `bash -c "curl ... install.sh | bash"` — broken on Windows (no bash by default) and not in sync with the canonical installer URL.
This PR replaces the shell-out with a thin per-platform shim that invokes the documented one-liner with `CUA_DRIVER_RS_VERSION` pinned to the version `version_check::fetch_latest_version` just resolved.
Why delegate to the install script instead of reimplementing in Rust?
The install scripts already own all of the load-bearing logic:
Treating "update" as a pinned re-install keeps install + update reading from one source of truth. New asset names / new platforms / better GC strategies ship in the scripts and benefit both paths automatically.
Post-install enhancements
Code shape
New module `crates/cua-driver/src/updater.rs` (~95 lines):
`cli.rs::run_update_cmd` refactored to call into the module. ~60 lines of inline shell-out replaced with structured branches.
Test plan
Out of scope (follow-ups)
🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
updatecommand documentation to clarify version-fetching behavior and installer delegation.Bug Fixes