From 5f033aedfdfbc75c3d74ea84032de2e491342d10 Mon Sep 17 00:00:00 2001 From: Francesco Bonacci Date: Sat, 23 May 2026 02:45:34 +0200 Subject: [PATCH 1/4] fix(cua-driver-rs): strip UTF-8 BOM from `cua-driver call` stdin payload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `read_stdin_json` was feeding the buffer directly to serde_json::from_str after trimming whitespace, but PowerShell 5.1's `Set-Content -Encoding utf8` silently prepends a UTF-8 BOM (`EF BB BF` / `U+FEFF`) to the file. When that file is fed to cua-driver via `Start-Process -RedirectStandardInput`, the BOM stays at the front of stdin, serde_json fails (a BOM isn't a valid JSON start character), and the call falls through to default args — producing surprising errors like "Missing required integer field pid" despite a valid-looking payload. Surfaced live during the 2026-05-23 overnight Windows dogfood: $ Set-Content -Path tmp.json -Value '{"pid":8608}' -Encoding utf8 $ cua-driver call hotkey < tmp.json Missing required integer field pid. After fix: $ cua-driver call hotkey < tmp.json ✅ Pressed ctrl+s on pid 8608 via PostMessage (Win32 target). Implementation: strip a single leading `'\u{feff}'` character from the trimmed buffer before handing to serde_json. The PS5.1 BOM is a 3-byte sequence in UTF-8 (`EF BB BF`) but a single char in str terms, so a `strip_prefix('\u{feff}')` is the right primitive. Tests in `stdin_bom_tests`: - BOM-prefixed payload parses successfully - Plain JSON unchanged (no-op when no BOM) Co-Authored-By: Claude Opus 4.7 --- .../crates/cua-driver/src/cli.rs | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/libs/cua-driver-rs/crates/cua-driver/src/cli.rs b/libs/cua-driver-rs/crates/cua-driver/src/cli.rs index 265238f593..24202d2bd4 100644 --- a/libs/cua-driver-rs/crates/cua-driver/src/cli.rs +++ b/libs/cua-driver-rs/crates/cua-driver/src/cli.rs @@ -1599,6 +1599,13 @@ pub fn run_doctor_cmd(json: bool) { /// Read JSON from stdin when stdin is a pipe (non-interactive). Returns `None` /// when stdin is a terminal or the input isn't valid JSON. /// Matches Swift's "If omitted, reads from stdin when stdin is a pipe." +/// +/// Strips a leading UTF-8 BOM (`U+FEFF`, bytes `EF BB BF`) before parsing. +/// Without this, payloads written via PowerShell 5.1's +/// `Set-Content -Encoding utf8` (which silently prepends a BOM) parse as +/// invalid JSON and the call falls through to default-args, producing +/// confusing "Missing required integer field" errors despite the caller +/// having sent a valid-looking payload. See the 2026-05-23 dogfood journal. fn read_stdin_json() -> Option { use std::io::{self, IsTerminal, Read}; let stdin = io::stdin(); @@ -1607,7 +1614,32 @@ fn read_stdin_json() -> Option { } let mut buf = String::new(); stdin.lock().read_to_string(&mut buf).ok()?; - serde_json::from_str(buf.trim()).ok() + let trimmed = buf.trim(); + // U+FEFF is one character (3 bytes UTF-8) — `str::strip_prefix` matches by + // chars, so a single `'\u{feff}'` is the right comparand. + let stripped = trimmed.strip_prefix('\u{feff}').unwrap_or(trimmed); + serde_json::from_str(stripped).ok() +} + +#[cfg(test)] +mod stdin_bom_tests { + /// Manual cross-check that the BOM-stripping logic round-trips correctly + /// without needing a real stdin pipe. + #[test] + fn strip_prefix_handles_utf8_bom() { + let with_bom = "\u{feff}{\"pid\":42}"; + let stripped = with_bom.strip_prefix('\u{feff}').unwrap_or(with_bom); + assert_eq!(stripped, "{\"pid\":42}"); + let v: serde_json::Value = serde_json::from_str(stripped).unwrap(); + assert_eq!(v["pid"], 42); + } + + #[test] + fn strip_prefix_no_op_when_no_bom() { + let plain = "{\"pid\":7}"; + let stripped = plain.strip_prefix('\u{feff}').unwrap_or(plain); + assert_eq!(stripped, plain); + } } /// Map a parsed [`Command`] to its canonical telemetry event name. From cf8f3ca91ac952f6f6e39ccd5da58e0e53025c08 Mon Sep 17 00:00:00 2001 From: Francesco Bonacci Date: Sat, 23 May 2026 03:31:17 +0200 Subject: [PATCH 2/4] fix(cua-driver-rs): merge image into structuredContent in daemon-forwarding path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cua-driver call screenshot` was silently dropping the image bytes when a daemon was running on the named pipe. The output looked like: $ cua-driver call screenshot {"format": "png", "height": 949, "width": 1512} — metadata only, no `screenshot_png_b64`. The image was 7.4 MB and the caller had no way to get it back. Root cause: the CLI in `run_call` has two code paths. The **in-process path** (no daemon listening) loops `result.content`, captures any Content::Image into `image_b64`, then merges that into the JSON-printed `structuredContent`: if let Some(sc) = &result.structured_content { let mut obj = sc.clone(); if out_path.is_none() { if let Some((b64, mime)) = image_b64 { ... insert("screenshot_png_b64", b64) ... } } println!("{pretty}"); } The **daemon-forwarding path** (daemon listening at default pipe) parsed the daemon's response, looped `content`, but only handled the image when `--screenshot-out-file` was set — otherwise the image was just thrown away: for item in content { if item.get("type") == Some("image") { if let Some(ref path) = screenshot_out_file { ... write to file ... } // ← else: silently drop } } if let Some(sc) = result.get("structuredContent") { let pretty = serde_json::to_string_pretty(sc)...; println!("{pretty}"); } Tonight on the cuademo VM (cua-driver-serve task registered, daemon at \\.\pipe\cua-driver listening), every `cua-driver call screenshot` invocation went through the daemon path and lost the image. From a non-daemon shell it worked fine. Fix: in the daemon-forwarding path, stash the image into `image_b64` during the content walk (when no out-path is set), then merge it into the structuredContent object before the pretty-print — mirroring the in-process path's behavior 1:1. Same final `screenshot_png_b64` + `screenshot_mime_type` keys. Empirical verification: before: cua-driver call screenshot | out-string → 61 chars after: cua-driver call screenshot | out-string → 7,654,802 chars Co-Authored-By: Claude Opus 4.7 --- .../crates/cua-driver/src/cli.rs | 35 +++++++++++++++---- 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/libs/cua-driver-rs/crates/cua-driver/src/cli.rs b/libs/cua-driver-rs/crates/cua-driver/src/cli.rs index 24202d2bd4..225c525d05 100644 --- a/libs/cua-driver-rs/crates/cua-driver/src/cli.rs +++ b/libs/cua-driver-rs/crates/cua-driver/src/cli.rs @@ -733,26 +733,49 @@ pub fn run_call( Ok(resp) => { if resp.ok { if let Some(result) = resp.result { - // Handle image write if --screenshot-out-file was given. + // Walk the content array once: pick up any Image + // payloads (either to write to --screenshot-out-file + // or to merge into structuredContent below). let mut printed = false; + let mut image_b64: Option<(String, String)> = None; if let Some(content) = result.get("content").and_then(|v| v.as_array()) { for item in content { if item.get("type").and_then(|v| v.as_str()) == Some("image") { - if let Some(ref path) = screenshot_out_file { - if let Some(b64) = item.get("data").and_then(|v| v.as_str()) { + let b64 = item.get("data").and_then(|v| v.as_str()).map(str::to_owned); + let mime = item.get("mimeType").and_then(|v| v.as_str()) + .unwrap_or("image/png").to_owned(); + if let Some(b64) = b64 { + if let Some(ref path) = screenshot_out_file { use base64::Engine as _; - match base64::engine::general_purpose::STANDARD.decode(b64) { + match base64::engine::general_purpose::STANDARD.decode(&b64) { Ok(bytes) => { let _ = std::fs::write(path, &bytes); } Err(e) => eprintln!("--screenshot-out-file: {e}"), } + } else { + // Stash for the structuredContent merge below. + image_b64 = Some((b64, mime)); } } } } } if let Some(sc) = result.get("structuredContent") { - let pretty = serde_json::to_string_pretty(sc) - .unwrap_or_else(|_| sc.to_string()); + // Merge image data into the structured payload + // (matches in-process behaviour at the bottom of + // this fn) so `cua-driver call screenshot` over + // the daemon socket still emits + // `screenshot_png_b64`. Previously this path + // dropped the image entirely when no + // --screenshot-out-file was given. + let mut obj = sc.clone(); + if let Some((b64, mime)) = image_b64 { + if let serde_json::Value::Object(ref mut map) = obj { + map.insert("screenshot_png_b64".into(), serde_json::Value::String(b64)); + map.insert("screenshot_mime_type".into(), serde_json::Value::String(mime)); + } + } + let pretty = serde_json::to_string_pretty(&obj) + .unwrap_or_else(|_| obj.to_string()); println!("{pretty}"); printed = true; } From d375e05351c57d6c3637c94787f15b8f491780e6 Mon Sep 17 00:00:00 2001 From: Francesco Bonacci Date: Sat, 23 May 2026 03:55:25 +0200 Subject: [PATCH 3/4] feat(cua-driver-rs): plumb --socket flag through cua-driver call + regression test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `--socket ` was a CLI-level flag (parsed in cli.rs line 142, listed in VALUE_FLAGS line 93) and threaded into `Command::Serve` / `Command::Stop` / `Command::Status` / `Command::Mcp` / `Command::Config` / `Command::Recording` — but **not into `Command::Call`**. That meant `cua-driver call --socket X screenshot` silently ignored the --socket override and used `default_socket_path()`. The flag worked for every long-running command but not for the per-call invocation. This made it impossible to write a daemon-forwarding regression test that hits a tempfile-socketed test daemon (the existing ServeDaemonTests pattern): the call always routed to the default socket, never to the test's `--socket /tmp/cua-test-XXX.sock` daemon. Changes: - `cli::Command::Call` gains `socket: Option` field - Both parse sites (line 249 + 310) thread `socket` through - `cli::run_call` takes a `socket_override: Option` parameter - When Some, skip the platform default + uia-worker-preferred resolution and route directly to that path - Both main.rs callsites pass the socket through Added a regression test for the 2026-05-23 screenshot fix: `test_call_screenshot_via_daemon_emits_b64` — starts a tempfile-socketed daemon, invokes `cua-driver call --socket X screenshot`, asserts that `screenshot_png_b64` appears in the JSON output. This catches the bug where the daemon-forwarding path in `run_call` was silently dropping the image (only printing `structuredContent` metadata: format / width / height) — a class of bug the in-process-only `test_call_screenshot_returns_b64_image` test couldn't catch because it never exercised the daemon code path. Co-Authored-By: Claude Opus 4.7 --- .../crates/cua-driver/src/cli.rs | 45 ++++++++++++++----- .../crates/cua-driver/src/main.rs | 8 ++-- .../tests/integration/test_cli.py | 41 +++++++++++++++++ 3 files changed, 79 insertions(+), 15 deletions(-) diff --git a/libs/cua-driver-rs/crates/cua-driver/src/cli.rs b/libs/cua-driver-rs/crates/cua-driver/src/cli.rs index 225c525d05..edc5794466 100644 --- a/libs/cua-driver-rs/crates/cua-driver/src/cli.rs +++ b/libs/cua-driver-rs/crates/cua-driver/src/cli.rs @@ -32,7 +32,19 @@ pub enum Command { }, ListTools, Describe(String), - Call { tool: String, json_args: Option, screenshot_out_file: Option }, + Call { + tool: String, + json_args: Option, + screenshot_out_file: Option, + /// Override the daemon socket/pipe path used by the in-process + /// forwarding fallback (matches `--socket` semantics for `serve` / + /// `status` / `stop`). Defaults to `serve::default_socket_path()` + /// when None — i.e. `cua-driver call X` looks for the user's + /// default-path daemon. Surfaced to make integration tests able + /// to spin up a tempfile-socketed daemon and route calls + /// through it. + socket: Option, + }, McpConfig { client: Option }, Serve { socket: Option, @@ -234,7 +246,7 @@ pub fn parse_command() -> Command { }, None => read_stdin_json(), }; - Command::Call { tool, json_args, screenshot_out_file } + Command::Call { tool, json_args, screenshot_out_file, socket: socket.clone() } } Some("telemetry") => { // Hidden — used by install.sh. Only supports `install-event` @@ -295,7 +307,7 @@ pub fn parse_command() -> Command { }, None => read_stdin_json(), }; - Command::Call { tool, json_args, screenshot_out_file } + Command::Call { tool, json_args, screenshot_out_file, socket: socket.clone() } } } } @@ -701,6 +713,7 @@ pub fn run_call( tool: &str, json_args: Option, screenshot_out_file: Option, + socket_override: Option, ) { // Daemon forwarding: if a daemon is listening, proxy the request // through it so AppStateEngine's element_index cache is shared. @@ -710,17 +723,27 @@ pub fn run_call( // like Calculator / modern Notepad / Settings. The regular daemon at // `\\.\pipe\cua-driver` is Medium integrity and gets ERROR_ACCESS_DENIED on // SendInput into AppContainer'd processes. See #1602. - #[cfg(target_os = "windows")] - let socket_path = { - let uia = crate::serve::default_uia_pipe_path(); - if crate::serve::is_daemon_listening(&uia) { - uia - } else { + // + // When `socket_override` is Some (i.e. caller passed `--socket `), + // route directly to that path and skip the platform default + uia worker + // search. Used by integration tests to drive a tempfile-socketed daemon. + let socket_path = if let Some(s) = socket_override { + s + } else { + #[cfg(target_os = "windows")] + { + let uia = crate::serve::default_uia_pipe_path(); + if crate::serve::is_daemon_listening(&uia) { + uia + } else { + crate::serve::default_socket_path() + } + } + #[cfg(not(target_os = "windows"))] + { crate::serve::default_socket_path() } }; - #[cfg(not(target_os = "windows"))] - let socket_path = crate::serve::default_socket_path(); if crate::serve::is_daemon_listening(&socket_path) { let args_for_daemon = json_args.clone() .unwrap_or(serde_json::Value::Object(serde_json::Map::new())); diff --git a/libs/cua-driver-rs/crates/cua-driver/src/main.rs b/libs/cua-driver-rs/crates/cua-driver/src/main.rs index 8f6632fc13..89d839dfb2 100644 --- a/libs/cua-driver-rs/crates/cua-driver/src/main.rs +++ b/libs/cua-driver-rs/crates/cua-driver/src/main.rs @@ -97,7 +97,7 @@ fn main() { cli::run_mcp_config(client.as_deref()); return; } - cli::Command::Call { tool, json_args, screenshot_out_file } => { + cli::Command::Call { tool, json_args, screenshot_out_file, socket } => { // Register callbacks (needed if the tool does screenshots/recording). mcp_server::recording::set_screenshot_fn(|window_id, pid| { if let Some(wid) = window_id { @@ -114,7 +114,7 @@ fn main() { }); let reg = Arc::new(platform_macos::register_tools()); reg.init_self_weak(); - cli::run_call(reg, &tool, json_args, screenshot_out_file); + cli::run_call(reg, &tool, json_args, screenshot_out_file, socket); return; } cli::Command::Serve { socket, no_permissions_gate } => { @@ -345,12 +345,12 @@ fn main() -> anyhow::Result<()> { cli::run_mcp_config(client.as_deref()); return Ok(()); } - cli::Command::Call { tool, json_args, screenshot_out_file } => { + cli::Command::Call { tool, json_args, screenshot_out_file, socket } => { let reg = Arc::new(build_registry_no_cursor()); reg.init_self_weak(); // run_call builds its own tokio runtime; must run on a fresh thread. std::thread::spawn(move || { - cli::run_call(reg, &tool, json_args, screenshot_out_file); + cli::run_call(reg, &tool, json_args, screenshot_out_file, socket); }).join().ok(); return Ok(()); } diff --git a/libs/cua-driver-rs/tests/integration/test_cli.py b/libs/cua-driver-rs/tests/integration/test_cli.py index 152e5c7a81..c713315d35 100644 --- a/libs/cua-driver-rs/tests/integration/test_cli.py +++ b/libs/cua-driver-rs/tests/integration/test_cli.py @@ -196,6 +196,47 @@ def test_serve_double_start_exits_1(self) -> None: _run([self.binary, "stop", "--socket", self._sock_file]) proc.wait(timeout=3) + @unittest.skipIf( + sys.platform == "win32", + "Windows: daemon spawned by subprocess.Popen runs in same session as " + "the test runner (typically Session 0 / non-interactive in CI); the " + "GDI BitBlt path fails with `The handle is invalid (0x80070006)` " + "because Session 0 has no graphics. The regression-guard behaviour " + "still verifies on macOS/Linux runners.", + ) + def test_call_screenshot_via_daemon_emits_b64(self) -> None: + """`cua-driver call screenshot` over the daemon socket must emit + `screenshot_png_b64` — same shape as the in-process path. Regression + guard for the 2026-05-23 fix where the daemon-forwarding path in + `run_call` was silently dropping the image bytes (only printing + structuredContent metadata: format / width / height). + """ + proc = self._start_daemon() + try: + r = _run( + [self.binary, "call", "--socket", self._sock_file, "screenshot"], + timeout=30, + ) + self.assertEqual(r.returncode, 0, f"stderr: {r.stderr}") + data = json.loads(r.stdout) + self.assertIn( + "screenshot_png_b64", data, + "daemon-forwarded screenshot dropped the image — " + "merge into structuredContent regressed", + ) + b64 = data["screenshot_png_b64"] + self.assertGreater( + len(b64), 100, + "screenshot base64 data seems too short — likely empty payload", + ) + self.assertIn( + "screenshot_mime_type", data, + "merge into structuredContent missed the mime-type key", + ) + finally: + _run([self.binary, "stop", "--socket", self._sock_file]) + proc.wait(timeout=3) + if __name__ == "__main__": unittest.main(verbosity=2) From fc4e43f74c189fd47506e8eb8a7d69124ba469cd Mon Sep 17 00:00:00 2001 From: Francesco Bonacci Date: Sat, 23 May 2026 16:04:44 +0200 Subject: [PATCH 4/4] fix(cua-driver-rs): surface --screenshot-out-file write errors in daemon path The daemon-forwarding branch was silently discarding the result of `std::fs::write`, so a failed write (e.g. permission denied, disk full, parent dir missing) would exit 0 with no indication. The in-process branch already reports these. This commit aligns the daemon branch with the same error-reporting shape, with separate messages for the base64 decode vs. file-write failure paths. Spotted by CodeRabbit on PR #1661. Co-Authored-By: Claude Opus 4.7 --- libs/cua-driver-rs/crates/cua-driver/src/cli.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/libs/cua-driver-rs/crates/cua-driver/src/cli.rs b/libs/cua-driver-rs/crates/cua-driver/src/cli.rs index edc5794466..faca5a7183 100644 --- a/libs/cua-driver-rs/crates/cua-driver/src/cli.rs +++ b/libs/cua-driver-rs/crates/cua-driver/src/cli.rs @@ -771,8 +771,14 @@ pub fn run_call( if let Some(ref path) = screenshot_out_file { use base64::Engine as _; match base64::engine::general_purpose::STANDARD.decode(&b64) { - Ok(bytes) => { let _ = std::fs::write(path, &bytes); } - Err(e) => eprintln!("--screenshot-out-file: {e}"), + Ok(bytes) => { + if let Err(e) = std::fs::write(path, &bytes) { + eprintln!("--screenshot-out-file: failed to write {path}: {e}"); + } + } + Err(e) => { + eprintln!("--screenshot-out-file: base64 decode failed: {e}"); + } } } else { // Stash for the structuredContent merge below.