-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix(cua-driver): persist set_config to disk on Windows + Linux (config parity) #2034
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,22 @@ impl Default for DriverConfig { | |
| fn default() -> Self { Self { capture_mode: "som".into(), max_image_dimension: 1568 } } | ||
| } | ||
|
|
||
| /// Load `DriverConfig` from `~/.cua-driver/config.json`, falling back to | ||
| /// defaults for any missing/malformed keys. Called once at `ToolState` | ||
| /// construction (i.e. on every fresh `cua-driver call` process) so that a | ||
| /// prior `set_config capture_mode=vision` survives across stateless one-shot | ||
| /// invocations — matching the macOS daemon's startup load. See #2008. | ||
| pub fn load_driver_config() -> DriverConfig { | ||
| let mut cfg = DriverConfig::default(); | ||
| if let Some(v) = pip_preview::read_config_value("capture_mode").and_then(|v| v.as_str().map(str::to_owned)) { | ||
| cfg.capture_mode = v; | ||
| } | ||
| if let Some(v) = pip_preview::read_config_value("max_image_dimension").and_then(|v| v.as_u64()) { | ||
| if let Ok(v32) = u32::try_from(v) { cfg.max_image_dimension = v32; } | ||
| } | ||
| cfg | ||
| } | ||
|
|
||
| pub struct ResizeRegistry { | ||
| ratios: std::sync::Mutex<std::collections::HashMap<u32, f64>>, | ||
| } | ||
|
|
@@ -91,7 +107,7 @@ impl ToolState { | |
| resize_registry: Arc::new(ResizeRegistry::new()), | ||
| zoom_registry: Arc::new(ZoomRegistry::new()), | ||
| mouse_hold: std::sync::Mutex::new(Default::default()), | ||
| config: Arc::new(RwLock::new(DriverConfig::default())), | ||
| config: Arc::new(RwLock::new(load_driver_config())), | ||
| }) | ||
| } | ||
| } | ||
|
|
@@ -3410,11 +3426,23 @@ impl Tool for SetConfigTool { | |
| ) { | ||
| match key { | ||
| "capture_mode" => match val.as_str() { | ||
| Some(s) => { cfg.capture_mode = s.to_owned(); parts.push(format!("capture_mode={s}")); } | ||
| Some(s) => { | ||
| cfg.capture_mode = s.to_owned(); | ||
| if let Err(e) = pip_preview::write_config_key("capture_mode", Value::String(s.to_owned())) { | ||
| tracing::warn!("set_config: failed to persist capture_mode: {e}"); | ||
| } | ||
| parts.push(format!("capture_mode={s}")); | ||
| } | ||
| None => return ToolResult::error(format!("`capture_mode` must be a string, got {val}.")), | ||
| }, | ||
| "max_image_dimension" => match val.as_u64() { | ||
| Some(n) => { cfg.max_image_dimension = n as u32; parts.push(format!("max_image_dimension={n}")); } | ||
| Some(n) => { | ||
| cfg.max_image_dimension = n as u32; | ||
| if let Err(e) = pip_preview::write_config_key("max_image_dimension", Value::from(n)) { | ||
| tracing::warn!("set_config: failed to persist max_image_dimension: {e}"); | ||
| } | ||
| parts.push(format!("max_image_dimension={n}")); | ||
| } | ||
|
Comment on lines
+3429
to
+3445
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟡 Minor 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# capture_mode consumers / validation in cua-driver
rg -nP --type=rust -C3 'capture_mode' libs/cua-driver/rust/crates | rg -nP -C3 'match|enum|som|vision|ax|parse|validate'Repository: trycua/cua Length of output: 50367 🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE='libs/cua-driver/rust/crates/platform-linux/src/tools/impl_.rs'
# Map the relevant symbols first, then inspect the smallest useful ranges.
ast-grep outline "$FILE" --view expanded | sed -n '1,220p'
printf '\n--- load_driver_config / set_config ranges ---\n'
rg -n -C 4 'fn load_driver_config|capture_mode|max_image_dimension|set_config|DriverConfig' "$FILE"Repository: trycua/cua Length of output: 21516 🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE='libs/cua-driver/rust/crates/platform-linux/src/tools/impl_.rs'
# Show the relevant code around config load and set_config.
sed -n '150,220p' "$FILE"
printf '\n---\n'
sed -n '5000,5065p' "$FILE"Repository: trycua/cua Length of output: 3675 🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE='libs/cua-driver/rust/crates/platform-linux/src/tools/impl_.rs'
python3 - <<'PY'
from pathlib import Path
file = Path("libs/cua-driver/rust/crates/platform-linux/src/tools/impl_.rs")
text = file.read_text()
for needle in [
"pub fn load_driver_config()",
"\"capture_mode\" => match val.as_str()",
"\"max_image_dimension\" => match val.as_u64()",
]:
idx = text.find(needle)
print(f"== {needle} == {idx}")
if idx != -1:
start = max(0, text.rfind("\n", 0, idx-400))
end = text.find("\n", idx+900)
print(text[start:end])
print("\n---")
PYRepository: trycua/cua Length of output: 4447 🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE='libs/cua-driver/rust/crates/platform-linux/src/tools/impl_.rs'
echo '--- load_driver_config ---'
sed -n '169,190p' "$FILE"
echo
echo '--- set_config capture_mode / max_image_dimension ---'
sed -n '5030,5058p' "$FILE"
echo
echo '--- capture_mode consumers ---'
rg -n -C 2 'do_tree|do_shot|capture_mode !=|capture_mode ==' "$FILE"Repository: trycua/cua Length of output: 2424 🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE='libs/cua-driver/rust/crates/platform-linux/src/tools/impl_.rs'
# Narrowly inspect the exact file under review.
rg -n -C 3 'pub fn load_driver_config|capture_mode|max_image_dimension|set_config|do_tree|do_shot' "$FILE"Repository: trycua/cua Length of output: 10954 Validate config values before persisting.
🤖 Prompt for AI Agents |
||
| None => return ToolResult::error(format!("`max_image_dimension` must be an integer, got {val}.")), | ||
| }, | ||
| "experimental_pip" => match val.as_bool() { | ||
|
|
@@ -3447,11 +3475,17 @@ impl Tool for SetConfigTool { | |
| } | ||
| // Legacy per-field shape. | ||
| if let Some(mode) = args.opt_str("capture_mode") { | ||
| if let Err(e) = pip_preview::write_config_key("capture_mode", Value::String(mode.clone())) { | ||
| tracing::warn!("set_config: failed to persist capture_mode: {e}"); | ||
| } | ||
| parts.push(format!("capture_mode={mode}")); | ||
| cfg.capture_mode = mode; | ||
| } | ||
| if let Some(dim) = args.opt_u64("max_image_dimension") { | ||
| cfg.max_image_dimension = dim as u32; | ||
| if let Err(e) = pip_preview::write_config_key("max_image_dimension", Value::from(dim)) { | ||
| tracing::warn!("set_config: failed to persist max_image_dimension: {e}"); | ||
| } | ||
| parts.push(format!("max_image_dimension={dim}")); | ||
| } | ||
| if let Some(enabled) = args.get("experimental_pip").and_then(|v| v.as_bool()) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Treat empty
HOME/USERPROFILEas unset.Line 24 and Line 25 use
var_osdirectly, soHOME=""orUSERPROFILE=""becomes.cua-driver/config.jsonrelative to the current working directory instead of returningNone. That breaks the documented contract here and can persist config to the wrong location.Suggested fix
pub fn default_config_path() -> Option<std::path::PathBuf> { std::env::var_os("HOME") .or_else(|| std::env::var_os("USERPROFILE")) + .filter(|h| !h.is_empty()) .map(|h| std::path::PathBuf::from(h).join(".cua-driver").join("config.json")) }📝 Committable suggestion
🤖 Prompt for AI Agents