fix(cua-driver-rs): macOS install 404 + config set not persisting to MCP sessions - #1518
Merged
Merged
Conversation
…sessions **Install script 404 on Apple Silicon / Intel** The script built the tarball URL as `darwin-arm64-binary.tar.gz` but the release only publishes a single `darwin-universal-binary.tar.gz` (lipo'd arm64 + x86_64). Fix: collapse all `darwin-*` LABEL values to the universal binary name; Linux per-arch tarballs (`linux-x86_64-binary`) are unchanged. **`config set` not visible in MCP sessions** `cua-driver config set capture_mode vision` wrote to `~/.cua-driver/config.json` but MCP sessions always initialised `DriverConfig` from defaults, so every fresh `mcp` invocation reverted to `capture_mode=som`. Two changes: 1. `ToolState::default()` now calls `load_driver_config()` which reads `~/.cua-driver/config.json` at startup, so CLI `config set` changes carry over automatically into every subsequent MCP session. 2. The `set_config` MCP tool now also writes through to disk via `write_driver_config_key()`, so config changes made inside an MCP session (e.g. via `tools/call set_config`) are likewise persisted for future sessions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds persistent JSON configuration storage to the macOS MCP driver and updates the installer to fetch universal-binary releases. Driver settings like capture mode and image dimensions now persist across MCP sessions via ChangesConfiguration persistence for macOS MCP driver
Installer macOS universal binary support
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 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 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/platform-macos/src/tools/mod.rs`:
- Around line 155-168: The function write_driver_config_key currently swallows
all I/O and JSON errors; change it to return a Result (e.g., Result<(),
anyhow::Error> or std::result::Result<(), Box<dyn std::error::Error>>) and
propagate errors instead of ignoring them: use config_file_path() to get the
path, attempt reading (std::fs::read_to_string) and parsing
(serde_json::from_str) with ? to surface errors, create parent dir with
std::fs::create_dir_all? and write the file with std::fs::write? (avoid
unwrap_or_default and ignored results), and update any callers to handle the
returned Result from write_driver_config_key; keep the same JSON key mutation
(json[key] = value.clone()) and use serde_json::to_string_pretty? so
serialization errors also propagate.
- Around line 147-149: The code reads max_image_dimension from json and casts a
u64 to u32 which can silently truncate; change the conversion to a fallible
check (e.g. use u32::try_from or TryFrom::try_from on the value returned by
json.get("max_image_dimension").and_then(|v| v.as_u64())) and handle the Err
case instead of blindly casting: if conversion succeeds set
cfg.max_image_dimension, otherwise return or propagate a config parse error (or
log and skip) so oversized values are detected rather than truncated.
In `@libs/cua-driver-rs/crates/platform-macos/src/tools/set_config.rs`:
- Around line 54-56: Validate that the decoded dim value fits within u32 before
casting: when reading args.get("max_image_dimension").and_then(|v| v.as_u64()),
check dim <= u64::from(u32::MAX) and only then assign cfg.max_image_dimension =
dim as u32 and call write_driver_config_key("max_image_dimension", …); if it
exceeds u32::MAX, return or propagate a clear error (or skip/update with a
bounded value) rather than performing a silent truncating cast.
🪄 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: 30c40ee9-bfc1-4e0a-bfa4-19d1fb39efef
📒 Files selected for processing (3)
libs/cua-driver-rs/crates/platform-macos/src/tools/mod.rslibs/cua-driver-rs/crates/platform-macos/src/tools/set_config.rslibs/cua-driver-rs/scripts/install.sh
Comment on lines
+147
to
+149
| if let Some(v) = json.get("max_image_dimension").and_then(|v| v.as_u64()) { | ||
| cfg.max_image_dimension = v as u32; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify all potentially truncating conversions around config dimension handling.
rg -nP 'max_image_dimension|as_u64\(\)|as u32|u32::try_from' libs/cua-driver-rs/crates/platform-macos/src/toolsRepository: trycua/cua
Length of output: 8153
Add bounds check on max_image_dimension cast to prevent silent truncation.
At lines 147–149, the cast from u64 to u32 can silently truncate large values without validation. Use a fallible conversion to ensure the loaded config value fits within the valid range.
Suggested fix
if let Some(v) = json.get("max_image_dimension").and_then(|v| v.as_u64()) {
- cfg.max_image_dimension = v as u32;
+ if let Ok(v32) = u32::try_from(v) {
+ cfg.max_image_dimension = v32;
+ }
}📝 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.
Suggested change
| if let Some(v) = json.get("max_image_dimension").and_then(|v| v.as_u64()) { | |
| cfg.max_image_dimension = v as u32; | |
| } | |
| if let Some(v) = json.get("max_image_dimension").and_then(|v| v.as_u64()) { | |
| if let Ok(v32) = u32::try_from(v) { | |
| cfg.max_image_dimension = v32; | |
| } | |
| } |
🤖 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/platform-macos/src/tools/mod.rs` around lines 147 -
149, The code reads max_image_dimension from json and casts a u64 to u32 which
can silently truncate; change the conversion to a fallible check (e.g. use
u32::try_from or TryFrom::try_from on the value returned by
json.get("max_image_dimension").and_then(|v| v.as_u64())) and handle the Err
case instead of blindly casting: if conversion succeeds set
cfg.max_image_dimension, otherwise return or propagate a config parse error (or
log and skip) so oversized values are detected rather than truncated.
Comment on lines
+155
to
+168
| pub fn write_driver_config_key(key: &str, value: &serde_json::Value) { | ||
| let path = config_file_path(); | ||
| let mut json: serde_json::Value = path | ||
| .exists() | ||
| .then(|| std::fs::read_to_string(&path).ok()) | ||
| .flatten() | ||
| .and_then(|t| serde_json::from_str(&t).ok()) | ||
| .unwrap_or_else(|| serde_json::json!({})); | ||
| json[key] = value.clone(); | ||
| if let Some(parent) = path.parent() { | ||
| let _ = std::fs::create_dir_all(parent); | ||
| } | ||
| let _ = std::fs::write(&path, serde_json::to_string_pretty(&json).unwrap_or_default()); | ||
| } |
Contributor
There was a problem hiding this comment.
Do not swallow config persistence failures.
At Line 155–168, all I/O/JSON errors are ignored, so callers can report success while disk persistence actually failed.
💡 Suggested fix
-pub fn write_driver_config_key(key: &str, value: &serde_json::Value) {
+pub fn write_driver_config_key(
+ key: &str,
+ value: &serde_json::Value,
+) -> Result<(), String> {
let path = config_file_path();
let mut json: serde_json::Value = path
.exists()
.then(|| std::fs::read_to_string(&path).ok())
.flatten()
.and_then(|t| serde_json::from_str(&t).ok())
.unwrap_or_else(|| serde_json::json!({}));
json[key] = value.clone();
if let Some(parent) = path.parent() {
- let _ = std::fs::create_dir_all(parent);
+ std::fs::create_dir_all(parent).map_err(|e| e.to_string())?;
}
- let _ = std::fs::write(&path, serde_json::to_string_pretty(&json).unwrap_or_default());
+ let body = serde_json::to_string_pretty(&json).map_err(|e| e.to_string())?;
+ std::fs::write(&path, body).map_err(|e| e.to_string())?;
+ Ok(())
}🤖 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/platform-macos/src/tools/mod.rs` around lines 155 -
168, The function write_driver_config_key currently swallows all I/O and JSON
errors; change it to return a Result (e.g., Result<(), anyhow::Error> or
std::result::Result<(), Box<dyn std::error::Error>>) and propagate errors
instead of ignoring them: use config_file_path() to get the path, attempt
reading (std::fs::read_to_string) and parsing (serde_json::from_str) with ? to
surface errors, create parent dir with std::fs::create_dir_all? and write the
file with std::fs::write? (avoid unwrap_or_default and ignored results), and
update any callers to handle the returned Result from write_driver_config_key;
keep the same JSON key mutation (json[key] = value.clone()) and use
serde_json::to_string_pretty? so serialization errors also propagate.
Comment on lines
+54
to
+56
| if let Some(dim) = args.get("max_image_dimension").and_then(|v| v.as_u64()) { | ||
| cfg.max_image_dimension = dim as u32; | ||
| write_driver_config_key("max_image_dimension", &Value::Number(dim.into())); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify all current max_image_dimension conversion points and schema constraints.
rg -nP 'max_image_dimension|as_u64\(\)|as u32|u32::try_from|input_schema' libs/cua-driver-rs/crates/platform-macos/src/toolsRepository: trycua/cua
Length of output: 11627
🏁 Script executed:
cat -n libs/cua-driver-rs/crates/platform-macos/src/tools/set_config.rs | head -65Repository: trycua/cua
Length of output: 2634
🏁 Script executed:
rg -A 3 'ToolResult::' libs/cua-driver-rs/crates/platform-macos/src/tools/*.rs | head -40Repository: trycua/cua
Length of output: 4011
Add range validation for max_image_dimension before casting to u32.
At lines 54–56, dim as u32 silently truncates for values > 4,294,967,295. The input schema (line 30–33) defines max_image_dimension as an unconstrained integer, so validation must occur at the code level.
💡 Suggested fix
if let Some(dim) = args.get("max_image_dimension").and_then(|v| v.as_u64()) {
- cfg.max_image_dimension = dim as u32;
+ let dim_u32 = match u32::try_from(dim) {
+ Ok(v) => v,
+ Err(_) => return ToolResult::error("max_image_dimension must be <= 4294967295"),
+ };
+ cfg.max_image_dimension = dim_u32;
write_driver_config_key("max_image_dimension", &Value::Number(dim.into()));
}🤖 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/platform-macos/src/tools/set_config.rs` around
lines 54 - 56, Validate that the decoded dim value fits within u32 before
casting: when reading args.get("max_image_dimension").and_then(|v| v.as_u64()),
check dim <= u64::from(u32::MAX) and only then assign cfg.max_image_dimension =
dim as u32 and call write_driver_config_key("max_image_dimension", …); if it
exceeds u32::MAX, return or propagate a clear error (or skip/update with a
bounded value) rather than performing a silent truncating cast.
- Use `u32::try_from` instead of `as u32` cast in both `load_driver_config` and `set_config` invoke to prevent silent truncation of oversized max_image_dimension values; return an error to the caller when out-of-range - `write_driver_config_key` now returns `Result<(), String>` instead of swallowing I/O and JSON errors; callers log warnings on failure via `tracing::warn!` rather than silently succeeding Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This was referenced May 16, 2026
5 tasks
This was referenced Jun 26, 2026
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
1 participant
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
Two bugs reported after the cua-driver-rs beta drop:
1. Install script 404 on Apple Silicon (and Intel)
The script built the download URL as
cua-driver-rs-{ver}-darwin-arm64-binary.tar.gzbut the release only publishesdarwin-universal-binary.tar.gz(a single lipo'd arm64+x86_64 binary). Per-arch bare-binary tarballs don't exist for macOS.Fix: Collapse all
darwin-*labels to the universal tarball name in acaseblock; Linux per-arch tarballs (linux-x86_64-binary) are unchanged.2.
config set capture_mode visionnot visible in MCP sessionscua-driver config set capture_mode visionpersisted to~/.cua-driver/config.json, but everycua-driver mcpinvocation initialisedDriverConfigfrom hardcoded defaults (som), silently ignoring the file. MCP clients that opted intovisionvia the CLI were still paying the full AX-tree walk cost every turn.Fix (two parts):
ToolState::default()now callsload_driver_config()which reads~/.cua-driver/config.jsonat startup — CLIconfig setchanges carry over into every subsequent MCP session automatically.set_configMCP tool now also writes through to disk (write_driver_config_key), so config changes made inside an MCP session persist for future sessions too.Test plan
curlthe install script on Apple Silicon — should downloaddarwin-universal-binary.tar.gz, not 404cua-driver config set capture_mode vision→ startcua-driver mcp→tools/call get_config→capture_modeshould bevisiontools/call set_config {"capture_mode":"vision"}→ restart mcp →get_config→ stillvisionCUA_DRIVER_RS_VERSION=0.1.3 ./install.shstill works on Linux🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores