refactor(cua-driver-rs): cross-platform dedup audit — image_utils + Spring + ArgsExt (#A + #1b + #C) - #1670
Merged
Merged
Conversation
Pure-image-processing helpers that lived as near-identical copies in
`platform-{macos,windows,linux}/src/capture.rs`. Each platform's
capture.rs still owns its native screenshot primitive (CGImage on
macOS / BitBlt+PrintWindow on Windows / XGetImage + ImageMagick
`import` on Linux). Everything DOWNSTREAM of "I have RGBA pixels" —
PNG encoding, JPEG encoding, downscaling to a max long edge, drawing
a crosshair, reading width/height from an IHDR — now lives here.
Functions extracted:
- png_bytes_to_jpeg(png_bytes, quality)
- resize_png_if_needed(png_bytes, max_dim)
- write_crosshair_png(png_bytes, cx, cy, path)
- crosshair_png_bytes(png_bytes, cx, cy)
- png_dimensions(data)
- encode_rgba_to_png(rgba, w, h)
- encode_bgra_to_png(bgra, w, h)
8 unit tests cover round-trip dimensions, resize semantics
(no-op when fits, no-op when max_dim=0, downscale to long edge),
JPEG signature, crosshair shape, and BGRA→RGBA channel swap.
Per-platform capture.rs swaps follow in subsequent commits.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…r::image_utils
Replace the file-local PNG/JPEG/resize/crosshair helpers in each
platform's capture.rs with thin re-exports of the shared
`mcp_server::image_utils::*` introduced in the previous commit.
The three platforms previously carried near-identical copies of:
- png_bytes_to_jpeg
- resize_png_if_needed
- crosshair_png_bytes / write_crosshair_png (macOS-only path)
- png_dimensions / png_dimensions_pub
Plus Windows + Linux each carried a hand-rolled
`write_uncompressed_png` + `write_png_chunk` + `zlib_store` +
`adler32` + `crc32_ieee` (~110 lines per platform) to convert raw RGBA
bytes from BitBlt / XGetImage to PNG. All of that is replaced by
`mcp_server::image_utils::encode_rgba_to_png` /
`encode_bgra_to_png` which go through the `image` crate's PNG encoder
— already a workspace dep, produces ~5x smaller files than the
uncompressed-store path that the hand-rolled code emitted.
Each platform's capture.rs keeps:
- screenshot_window_bytes / screenshot_display_bytes (native:
CGImage / BitBlt+PrintWindow / XGetImage)
- screenshot_window / screenshot_display wrapper that returns
base64+dimensions
- public re-export wrappers that call into mcp_server::image_utils
so existing callers (`tools/*.rs`) keep compiling without churn
Build: clean (0 warnings) on x86_64-pc-windows-msvc.
Tests: 32/32 platform-windows pass, 28/28 mcp_protocol_test pass,
8/8 new image_utils unit tests pass.
Diffstat: +89 / -488 lines across the three platform capture.rs files.
macOS and Linux compile-checks not run on this VM — reviewer should
`cargo check -p platform-macos` / `-p platform-linux` to confirm.
Structurally the substitutions are uniform: every public function in
the platform crate now delegates to the same `mcp_server::image_utils`
function with the same arguments.
Co-Authored-By: Claude Opus 4.7 <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 consolidates image manipulation utilities—PNG/JPEG encoding, resizing, crosshair drawing, dimension parsing—from three platform-specific capture modules (Linux, macOS, Windows) into a new shared ChangesCross-platform image utilities consolidation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 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 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 |
…t (dedup audit #1b)
The 4-field `struct Spring { ox, oy, vx, vy }` was duplicated
verbatim across all three platform `overlay.rs` files (with
`#[derive(Clone, Copy)]` and identical field names/types). Moved
to `cursor_overlay::Spring` (re-exported from `lib.rs` alongside
`MotionConfig`) and each platform crate now imports the shared
type via `use cursor_overlay::Spring;`.
Fields are now `pub` (were private when the struct was per-module);
the access pattern stays identical because Spring is now imported
into the same scope where it was previously declared.
Tiny extraction — about 30 lines removed total — but proves the
extraction pattern for the larger overlay dedup (PR #B in the audit
doc: RenderState + tick + apply_command + render_frame +
draw_default_arrow, ~1800 lines across the 3 platforms).
Build: clean (0 warnings) on Windows. Tests: 4/4 cursor-overlay,
8/8 image_utils, 32/32 platform-windows.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Document at `libs/cua-driver-rs/docs/dedup-audit.md` enumerates
duplication candidates across platform-{macos,windows,linux}/src,
ranks by ROI vs refactoring risk, and outlines three follow-up PRs:
- PR #A (this branch) — image_utils + Spring extraction (~430 lines)
- PR #B — overlay.rs RenderState + render pipeline (~1800 lines, deferred)
- PR #C — mcp-server::tool_args helper trait (~600 lines, deferred)
Also captures the explicit list of things NOT to dedupe (per-platform
tool descriptions, FFI bindings, AppsFolder enumerations) and the
existing shared crates' scope (mcp-server, cursor-overlay).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/cua-driver-rs/crates/platform-linux/src/capture.rs (1)
27-36:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCompilation error:
png_dimensionsfunction no longer exists.Line 30 calls
png_dimensions(&bytes), but the local function was removed as part of the dedup audit (per comments at lines 94-99). This will fail to compile on Linux.The PR summary notes Linux compile-checks were not run on the Windows VM—this confirms the issue.
🐛 Proposed fix
// Try `import -window <xid> png:-` (ImageMagick). if let Ok(bytes) = capture_via_import(xid) { - let (w, h) = png_dimensions(&bytes)?; + let (w, h) = mcp_server::image_utils::png_dimensions(&bytes)?; return Ok((BASE64.encode(&bytes), w, h)); }🤖 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-linux/src/capture.rs` around lines 27 - 36, The call to the removed helper png_dimensions in screenshot_window causes a compile error; update screenshot_window to derive the PNG width/height from the returned bytes instead of calling png_dimensions: after capture_via_import(xid) returns bytes, decode the PNG header (e.g. use the image crate or the PNG decoder API such as image::codecs::png::PngDecoder::new(&bytes[..]) or image::load_from_memory) to obtain (w,h), then return Ok((BASE64.encode(&bytes), w, h)); keep capture_via_import, BASE64.encode, and the existing fallback capture_via_xgetimage(xid) unchanged.
🧹 Nitpick comments (1)
libs/cua-driver-rs/crates/mcp-server/src/image_utils.rs (1)
257-263: ⚖️ Poor tradeoffConsider optimizing BGRA→RGBA conversion to avoid double allocation.
Line 258 creates a full copy with
to_vec(), then swaps in place. This could be optimized to swap channels during the copy in a single pass, reducing allocations.However, for a deduplication refactor focused on correctness, the current implementation is acceptable.
♻️ Optional optimization
pub fn encode_bgra_to_png(bgra: &[u8], w: u32, h: u32) -> Result<Vec<u8>> { - let mut rgba = bgra.to_vec(); - for px in rgba.chunks_exact_mut(4) { - px.swap(0, 2); // B ↔ R - } + let rgba: Vec<u8> = bgra + .chunks_exact(4) + .flat_map(|px| [px[2], px[1], px[0], px[3]]) // B,G,R,A → R,G,B,A + .collect(); encode_rgba_to_png(&rgba, w, h) }🤖 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/mcp-server/src/image_utils.rs` around lines 257 - 263, The current encode_bgra_to_png creates a full copy via bgra.to_vec() then swaps channels in-place, causing an extra allocation; change encode_bgra_to_png to perform a single-pass conversion: allocate a Vec<u8> with capacity bgra.len() and iterate bgra.chunks_exact(4), pushing bytes in RGBA order (i.e., push B->R, G, R->B, A swapped per pixel) into the new buffer, then call encode_rgba_to_png(&new_buf, w, h); reference: function encode_bgra_to_png and encode_rgba_to_png.
🤖 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/mcp-server/src/image_utils.rs`:
- Around line 102-116: The tilde-expansion in write_crosshair_png can produce an
unexpected root path when neither HOME nor USERPROFILE is set; change the logic
that builds `path` so that if `path` starts with '~' and both
`std::env::var("HOME")` and `std::env::var("USERPROFILE")` are missing/empty you
return a Err (propagate a clear error) instead of using an empty string (or
alternatively preserve the original `path` with the tilde); update the branch
that currently calls `unwrap_or_default()` to explicitly check for
presence/non-empty values and return an error from write_crosshair_png with a
helpful message referencing the missing env vars when expansion cannot be
performed.
---
Outside diff comments:
In `@libs/cua-driver-rs/crates/platform-linux/src/capture.rs`:
- Around line 27-36: The call to the removed helper png_dimensions in
screenshot_window causes a compile error; update screenshot_window to derive the
PNG width/height from the returned bytes instead of calling png_dimensions:
after capture_via_import(xid) returns bytes, decode the PNG header (e.g. use the
image crate or the PNG decoder API such as
image::codecs::png::PngDecoder::new(&bytes[..]) or image::load_from_memory) to
obtain (w,h), then return Ok((BASE64.encode(&bytes), w, h)); keep
capture_via_import, BASE64.encode, and the existing fallback
capture_via_xgetimage(xid) unchanged.
---
Nitpick comments:
In `@libs/cua-driver-rs/crates/mcp-server/src/image_utils.rs`:
- Around line 257-263: The current encode_bgra_to_png creates a full copy via
bgra.to_vec() then swaps channels in-place, causing an extra allocation; change
encode_bgra_to_png to perform a single-pass conversion: allocate a Vec<u8> with
capacity bgra.len() and iterate bgra.chunks_exact(4), pushing bytes in RGBA
order (i.e., push B->R, G, R->B, A swapped per pixel) into the new buffer, then
call encode_rgba_to_png(&new_buf, w, h); reference: function encode_bgra_to_png
and encode_rgba_to_png.
🪄 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: caf5eae9-8c0c-4261-a6c3-17096cf50c9e
⛔ Files ignored due to path filters (1)
libs/cua-driver-rs/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
libs/cua-driver-rs/crates/mcp-server/Cargo.tomllibs/cua-driver-rs/crates/mcp-server/src/image_utils.rslibs/cua-driver-rs/crates/mcp-server/src/lib.rslibs/cua-driver-rs/crates/platform-linux/src/capture.rslibs/cua-driver-rs/crates/platform-macos/src/capture.rslibs/cua-driver-rs/crates/platform-windows/src/capture.rs
Comment on lines
+102
to
+116
| pub fn write_crosshair_png(png_bytes: &[u8], cx: f64, cy: f64, path: &str) -> Result<()> { | ||
| let mut img = decode_png_to_rgba8(png_bytes)?; | ||
| draw_crosshair(&mut img, cx, cy); | ||
|
|
||
| let path = if let Some(rest) = path.strip_prefix('~') { | ||
| let home = std::env::var("HOME") | ||
| .or_else(|_| std::env::var("USERPROFILE")) | ||
| .unwrap_or_default(); | ||
| format!("{home}{rest}") | ||
| } else { | ||
| path.to_owned() | ||
| }; | ||
| img.save_with_format(&path, ImageFormat::Png)?; | ||
| Ok(()) | ||
| } |
Contributor
There was a problem hiding this comment.
Consider edge case when HOME/USERPROFILE env vars are unset.
Lines 107-109: if neither HOME nor USERPROFILE is set, unwrap_or_default() returns an empty string. For input like "~/foo.png", this produces "/foo.png" (a root path) rather than failing or preserving the tilde. This edge case is unlikely in practice but could cause unexpected behavior.
Consider failing explicitly or documenting this behavior if it's intentional.
🛡️ Suggested defensive fix
let path = if let Some(rest) = path.strip_prefix('~') {
let home = std::env::var("HOME")
.or_else(|_| std::env::var("USERPROFILE"))
- .unwrap_or_default();
+ .unwrap_or_else(|_| {
+ // If no home dir is set, keep the tilde path as-is
+ return path.to_owned();
+ });
format!("{home}{rest}")
} else {
path.to_owned()
};Or alternatively, fail explicitly:
let home = std::env::var("HOME")
.or_else(|_| std::env::var("USERPROFILE"))
- .unwrap_or_default();
+ .map_err(|_| anyhow!("Cannot expand ~: HOME/USERPROFILE not set"))?;🤖 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/mcp-server/src/image_utils.rs` around lines 102 -
116, The tilde-expansion in write_crosshair_png can produce an unexpected root
path when neither HOME nor USERPROFILE is set; change the logic that builds
`path` so that if `path` starts with '~' and both `std::env::var("HOME")` and
`std::env::var("USERPROFILE")` are missing/empty you return a Err (propagate a
clear error) instead of using an empty string (or alternatively preserve the
original `path` with the tilde); update the branch that currently calls
`unwrap_or_default()` to explicitly check for presence/non-empty values and
return an error from write_crosshair_png with a helpful message referencing the
missing env vars when expansion cannot be performed.
…up audit #C)
Adds the shared `ArgsExt` trait on `serde_json::Value` that every
platform crate uses to pull pid / window_id / element_index / etc.
out of inbound MCP tool args. Replaces ~200 hand-written
`match args.get("name").and_then(|v| v.as_X())` blocks with one
consistent surface.
Accessor families:
- `require_*` — bails with `ToolResult::error` on missing/wrong type;
narrowing casts (i64→i32, u64→u32) go through `try_from` so
out-of-range JSON numbers surface as an actionable range error
instead of silently truncating. Matches the CodeRabbit fix landed
on PR #1666's page tool.
- `opt_i32`/`opt_u32` — Result<Option<T>> with range check.
- `opt_*` (u64/i64/f64/str/bool) — plain Option<T> for callers with
defaults handled elsewhere.
- `*_or` — default-fallback variants (the most common pattern).
- `str_array` — drains an array of strings, skipping non-strings.
Error wording is canonical: `"Missing required {kind} field: {name}"`
so MCP clients can pattern-match. Per-tool error strings with custom
helper text (`get_window_state`'s window_id helper, `kill_app`'s
range message, page.rs's CodeRabbit-vetted wording) are preserved
as-is.
15 unit tests cover happy path, missing-field, wrong-type, and
out-of-range cases. `cargo test -p mcp-server` green.
See `libs/cua-driver-rs/docs/dedup-audit.md` for the audit trail.
…/linux tools (dedup audit #C) Threads the new `mcp_server::tool_args::ArgsExt` trait through every tool's `invoke()`. Replaces ~200 hand-written args.get/.and_then chains with consistent typed accessors. Per-platform refactor counts: - platform-windows/tools/impl_.rs — 15 tools refactored - platform-macos/tools/* — 20 of 26 files - platform-linux/tools/impl_.rs — 84 of 93 patterns Skipped (preserved verbatim per audit rules): - `kill_app`/`debug_window_info` — custom range + bespoke wording - `get_window_state` window_id branch — helper text directs to list_windows - `hotkey` keys-array — needs raw `as_array()` for inline filter - `set_agent_cursor_style` gradient_colors/bloom_color — per-element hex validation distinct from `str_array` silent-skip - page.rs — CodeRabbit-vetted per-action wording from PR #1666 Bonus correctness wins (uncovered while refactoring): - Linux `get_window_state` and `scroll` were silently truncating u64→u32 for pid. `require_u32` now range-checks. cargo build/test green on Windows (target available locally). macOS and Linux targets aren't installed on this Windows host — covered by CI on those platforms. Roughly -125 lines net across the three platforms.
CodeRabbit review on PR #1670 commit 921dcdc (image_utils routing) flagged two issues: 1. **CRITICAL** — Linux compile error. `platform-linux/src/capture.rs:30` still called the local `png_dimensions()` after the dedup extraction removed it. Route through `mcp_server::image_utils::png_dimensions` like the other two platforms. (CI on Windows hadn't caught this because the Linux target isn't installed on the VM.) 2. **Edge case** — `write_crosshair_png` tilde-expansion silently produced `/foo` instead of `~/foo` when both `HOME` and `USERPROFILE` are missing/empty. Now bails with a clear error referencing the missing env vars instead of writing to root. Skipped: nitpick to fold `bgra.to_vec() + swap` into a single-pass flat_map. CodeRabbit itself marked it `⚖️ Poor tradeoff` — not worth churning the BGRA path for a microbench.
f-trycua
changed the title
…latform wrappers (dedup audit #2) Extracts the locked-HashMap plumbing shared by all three platform element caches into a generic `ElementCacheCore<K, S>` in mcp-server. Each platform keeps its own: - `CacheKey` (i32 pid + u32 window_id on macOS, u32 pid + u64 hwnd on Windows, u32 pid + u64 xid on Linux) - `CachedSnapshot` with its native Drop impl (CFRelease for AX, COM Release for UIA, none for AT-SPI) - Specialised accessors (`get_element_ptr`, `get_element_center` on Windows, `get_element_key` on Linux) What moved: HashMap+Mutex insert/lookup/count. ~85 lines net. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…nder pipeline (dedup audit #B) Lifts ~950 lines of duplicated render state and animation logic out of the three platform `overlay.rs` files into the shared `cursor-overlay` crate. The three platforms previously held byte-near-identical copies of the animation state, tick physics, OverlayCommand dispatch, bloom + arrow rasteriser, and palette/gradient/bloom-override plumbing — maintaining them in three places meant every cursor change had to land three times and stay in sync. What moved to `cursor-overlay::render_state`: - `RenderStateCore` — the platform-agnostic animation fields (`cfg`, `palette`, `motion`, `pos`, `heading`, `path`, `dist`, `spring`, `spring_tgt`, `click_t`, `shape`, `visible`, `idle_secs`, `idle_alpha`, `pinned_wid`, `gradient_colors`, `bloom_override`). - `RenderStateCore::tick_motion(dt)` — speed-profile + spring physics + click pulse + idle fade using runtime `MotionConfig` (Windows / Linux). - `RenderStateCore::tick_swift_constants(dt) -> bool` — the macOS variant that uses the hardcoded Swift reference constants (peakSpeed=900, springK=400, overshoot=0.8) and returns whether the path just ended (so the caller can fire its arrival oneshot). - `RenderStateCore::apply_command_base(cmd, snap_move, snap_click)` — the 8 OverlayCommand match arms. Two booleans select the macOS-only sentinel-snap behaviour for MoveTo + ClickPulse. - `render_frame(core, w, h, origin_x, origin_y, focus_rect)` — the tiny-skia bloom + click-pulse + arrow paint, parametrised by pixmap dimensions and an origin offset (Windows passes virt_x/y; macOS + Linux pass 0,0). An optional `FocusRect` is drawn on top — only macOS supplies one. - `draw_default_arrow(...)` — gradient arrow rasteriser, now with the `gradient_override` argument all three platforms wanted. What stays per-platform: - macOS (`platform-macos/src/cursor/overlay.rs`): AppKit window + GCD render thread + `dispatch_set_layer_contents` (CGImage) + the focus_rect/focus_rect_t state (macOS-only post-arrival element highlight) + the win_w/win_h NSScreen dims. - Windows (`platform-windows/src/overlay.rs`): Win32 message loop + `UpdateLayeredWindow` (BGRA DIB) + virt_x/y/w/h virtual-screen geometry + last_tick wall-clock stamp for the WM_TIMER dt. - Linux (`platform-linux/src/overlay.rs`): X11 override-redirect window + XPutImage (BGRA ZPixmap) + scr_w/scr_h. Each platform's RenderState is now a thin wrapper that holds `core: cursor_overlay::RenderStateCore` plus its platform-specific extras. `tick` and `apply_command` forward to the shared core, with macOS layering its focus-rect fade on top of the shared tick and intercepting ShowFocusRect in apply_command. Behaviour: byte-identical. The two `tick` variants preserve the existing per-platform constants exactly (macOS still uses the Swift hardcoded values; Windows/Linux still use the runtime MotionConfig). The smootherstep speed profile `30·u²·(1-u)²/1.875` and `16·u²·(1-u)²` are algebraically equivalent (both peak at 1.0 at u=0.5); macOS keeps the 30/1.875 form for parity with the Swift ref. Net diff: 4 files changed, +132 / -1077 in the platform files plus +734 in the new shared module = -213 net lines, and from now on animation tweaks land in one place instead of three. Verified on Windows: `cargo build --release -p cua-driver` clean (0 warnings, 0 errors). `cargo test --release -p cursor-overlay -p platform-windows -p mcp-server` all green. macOS + Linux not compile-checked locally (only x86_64-pc-windows-msvc target installed); CI on those platforms will catch any issues. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…s got wrong
All five ranked candidates shipped on this branch:
- #A image_utils ✓ (close to estimate)
- #1b Spring ✓
- #B render_state ✓ (less line-savings than estimated; tick was NOT
byte-for-byte identical)
- #C tool_args ✓ (less line-savings; trait body costs add up)
- #2 element_cache ✓ (+14 lines; audit was optimistic)
Logged the three places the audit estimates were off so future audits
can calibrate against this.
5 tasks
f-trycua
added a commit
that referenced
this pull request
Jun 30, 2026
…is the only implementation The Rust implementation has been the active, cross-platform (macOS/Linux/Windows) backend and the default everywhere; the Swift macOS-only backend is dead weight. Per decision: keep the cua-driver-rs-v* release tags and retire the old cua-driver (Swift) release flow, pointing the public installer at Rust. Deleted: - libs/cua-driver/swift/ — the entire Swift source tree (~11k files). - The Swift CI/CD: .github/workflows/ci-swift-cua-driver.yml + cd-swift-cua-driver.yml (the old cua-driver-v* release flow). Public `curl|bash` survives: install.sh already defaults to Rust and execs _install-rust.sh, whose baked version is maintained by cd-rust-cua-driver.yml (cua-driver-rs-v* tags). - Swift-only scripts: _install-local-swift.sh, build-app.sh, scripts/test.sh, scripts/build/build-release-notarized.sh, top-level scripts/CuaDriver.entitlements (Rust uses rust/scripts/CuaDriver.entitlements), and the defunct root Package.swift. - libs/cua-driver/rust/docs/dedup-audit.md — completed audit, shipped via #1670. Excised the --backend=swift branches from the shared installers (install.sh 486→120, install-local.sh, uninstall.sh −340 lines, _install-common.sh); they go Rust-only and keep --backend=swift as an accepted no-op for back-compat. Verified: bash -n clean on every edited script; install.sh execs _install-rust.sh on both the on-disk and curl paths; cd-rust codesign uses the surviving rust/scripts/CuaDriver.entitlements (working-directory: libs/cua-driver/rust); cd-swift-lume uses lume's own build scripts (untouched); the Rust Cargo workspace has zero Swift references and still builds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
Three of the dedup-audit refactors bundled into one PR.
capture.rsimage helpers →mcp_server::image_utilsSpringphysics struct →cursor_overlaymcp_server::tool_args::ArgsExttrait + adopt across all platformsRenderState+ render pipelineAudit doc:
libs/cua-driver-rs/docs/dedup-audit.md.#A — image_utils extraction
Three platforms previously carried near-identical copies of:
png_bytes_to_jpegresize_png_if_neededcrosshair_png_byteswrite_crosshair_pngpng_dimensions/png_dimensions_pubwrite_uncompressed_png+write_png_chunk+zlib_store+adler32+crc32_ieeeAll consolidated in
crates/mcp-server/src/image_utils.rs. Each platform'scapture.rskeeps its native screenshot primitive (CGImage / BitBlt+PrintWindow / XGetImage + ImageMagickimport) plus thin pub re-export wrappers so existing callers keep compiling.The hand-rolled Windows + Linux PNG encoder produced ~5× larger files than
imagecrate's encoder (uncompressed-store mode). Bonus: the new path produces smaller files for free, sinceimageis already a workspace dep.#1b — Spring physics dedup
Spring { ox, oy, vx, vy }and its tick/apply impl appeared verbatim in all threeoverlay.rsfiles. Lifted tocursor_overlay::Spring.#C —
ArgsExttraitEvery tool's
invoke()did the same dance to pullpid/window_id/element_index/ etc. out of the inbound JSONValue, with subtly different error wording across ~200 sites. New trait atmcp_server::tool_args::ArgsExtwith three accessor families:require_*— bails withToolResult::errorif missing/wrong type. Narrowing casts (i64→i32, u64→u32) go throughtry_from, so out-of-range JSON numbers surface as actionable errors instead of silently truncating. (This was the CodeRabbit fix from PR feat(cua-driver-rs): unify browser_eval + page into one cross-platform tool #1666; now uniform.)opt_*—Option<T>for callers with defaults handled elsewhere.*_or— default-fallback variants (most common pattern).str_array— drains an array of strings, skipping non-strings.Canonical error format:
"Missing required {kind} field: {name}".Adoption counts:
platform-windows/tools/impl_.rs— 15 toolsplatform-macos/tools/*— 20 of 26 filesplatform-linux/tools/impl_.rs— 84 of 93args.getsitesPreserved verbatim (per audit rules):
kill_app/debug_window_info— custom range validation + bespoke wordingget_window_statewindow_idbranch — directs callers tolist_windowshotkeykeysarray — needs rawas_array()for inline filterset_agent_cursor_stylegradient_colors/bloom_color— per-element hex validation distinct fromstr_array's silent-skippage.rs— CodeRabbit-vetted per-action wording from PR feat(cua-driver-rs): unify browser_eval + page into one cross-platform tool #1666Bonus correctness wins (surfaced while refactoring):
get_window_stateandscrollwere silently truncating u64→u32 for pid.require_u32now range-checks.Tests
mcp_server::image_utils::tests— all passmcp_server::tool_args::tests— all passcargo test -p mcp-server— 23/23 passcargo test -p platform-windows— 32/32 passcargo test -p cua-driver --test mcp_protocol_test— 28/28 passCodeRabbit feedback already addressed (commit
02775bfe)platform-linux/src/capture.rs:30still called the localpng_dimensions()after extraction; now routed throughmcp_server::image_utils::png_dimensions.write_crosshair_pngtilde-expansion now bails with a clear error when bothHOMEandUSERPROFILEare missing/empty (previously silently became/foo).bgra.to_vec() + swapinto single-pass flat_map; CodeRabbit itself marked it⚖️ Poor tradeoff.Test plan
cargo build --release -p cua-driver— 0 warnings)cargo check -p platform-macosto verify macOS substitutions compilecargo check -p platform-linuxto verify Linux substitutions compile🤖 Generated with Claude Code