refactor(cursor-overlay): extract rotate_toward into shared util crate - #1662
Merged
Conversation
`rotate_toward` (angle-easing primitive used by per-OS overlay render loops) was duplicated verbatim — 6 lines, identical — between platform-windows/src/overlay.rs and platform-linux/src/overlay.rs. Pulled it into a new `cursor-overlay/src/util.rs` module so both per-OS backends (and any future ones — Wayland-native, etc.) share the same math. Added 4 unit tests covering: - no-motion-when-aligned (current == desired) - clamping to max_step - short-way wrap (current=0.95π, desired=-0.95π) takes the +0.1π path not the -1.9π path - negative wrap (current=-0.95π, desired=0.95π) takes the -0.1π path This is the first pass at the "abstract visual components between OSes" work surfaced in the 2026-05-23 overnight journal. Render-loop functions (`render_frame`, `draw_default_arrow`) are LARGER and per-OS state shapes differ (Win: virt_w/h, Linux: scr_w/h) — those need a follow-up that unifies the state-name surface before they can be shared. Local-only; not pushed yet.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR extracts the ChangesShared rotation utility extraction
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 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 |
f-trycua
added a commit
that referenced
this pull request
May 23, 2026
…arnings cleanup (#1665) * fix(cua-driver-rs)(uninstall): converge uninstall.{ps1,sh} + UAC self-elevation Three changes that together let -AutoStart installs roll back cleanly: 1. uninstall.ps1 self-elevates via UAC when it detects an -AutoStart install. The cua-driver-serve task is registered at RunLevel=Highest (autostart.rs:127, since 2026-05-21 for UWP/AppContainer support), so the daemon spawned by it runs at High IL and a non-elevated process — even the same user who installed it — could neither terminate the daemon nor delete the task. Detection: schtasks /Query OR a running cua-driver.exe. Re-exec via Start-Process -Verb RunAs; supports both `-File` invocation and `irm | iex` (materialises body to tempfile). 2. uninstall.sh inlines what _uninstall-rust.sh used to do — single canonical script per shell, mirrors uninstall.ps1's shape. Same --experimental-rust / --backend=rust / --backend=swift flag set, same non-macOS Rust auto-select. _uninstall-rust.sh deleted. 3. cd-rust-cua-driver.yml drops the cp _uninstall-rust.sh release-upload line + updated comment. installation.mdx docs page reflects the new one-file-per-shell shape and the .ps1 self-elevation behaviour. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(cua-driver-rs)(install-local.ps1): UTF-8 read for post-install-hints The `Next steps:` block rendered with mojibake (`•` for `•`, `â€"` for `—`) because PowerShell 5.1's `Get-Content -Raw` defaults to Windows-1252 when the source file has no BOM. The .txt is UTF-8. Replaced `Get-Content -Raw` with `[System.IO.File]::ReadAllText(..., UTF8)`. Surgical fix at the one Windows reader; the other 3 installers (install.ps1 via Invoke-WebRequest HTTP-charset, _install-rust.sh / install-local.sh via sed on bytes) were never affected, so the .txt stays BOM-free. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore(cua-driver-rs): clean up 30 build warnings in -p cua-driver path `cargo build --release -p cua-driver` previously emitted 28 warnings on platform-windows + 3 on cua-driver. After this commit: zero. Mechanical groups: 1. Dead code (delete): - overlay.rs: unused `Duration` import; 2 stray `use Win32::*` lines (PR #1662 fallout from rotate_toward extraction); 2 unused `mut`; uia/mod.rs lines 173-174 dead writes to `counter`/`total` that are never read after the fallback block. - cli.rs: deleted unused `run_dump_docs` wrapper — `main.rs` only calls `run_dump_docs_with_type`, the wrapper had zero callers. 2. #[allow(dead_code)] on bundle.rs non-macOS/non-unix stubs — they exist for cross-platform API symmetry per the module header. 3. `let _ = ` on 10 Win32 teardown calls (ShowWindow, TranslateMessage, DeleteDC, UpdateLayeredWindow, DeleteObject) that return BOOL / Result. These are fire-and-forget at end-of-scope; the lint wants explicit ignore. 4. Module-scope #![allow(non_upper_case_globals)] in uia/windows_enum.rs — pattern-matches against `UIA_*ControlTypeId` constants from the `windows` crate (we can't rename external symbols). Mirrors the existing `#![allow(...)]` at overlay.rs:12. Cargo.lock picks up the 0.2.7 → 0.2.18 workspace-version-bump that shipped earlier this session. The `TypeTextCharsTool { state: _state }` rename + per-tool description thickening on platform-windows/tools/impl_.rs land in a follow-up commit alongside the agent-guidance refactor. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(cua-driver-rs): agent-guidance refactor — Skills/MCP-instructions/per-tool Three changes informed by a survey of how OSS computer-use tools push usage guidance to LLM agents (Playwright MCP, browser-use, Goose, Open Interpreter, Anthropic computer-use cookbook) cross-checked against the MCP spec + Claude Skills convention. Findings driving the changes: - Anthropic's documented Skills tip is "Keep SKILL.md under 500 lines; move detailed reference material to separate files" (code.claude.com/docs/en/skills). Our SKILL.md was 911 lines. - The MCP `instructions` field is spec'd as a hint that "MAY be added to the system prompt" — eager, every-turn cost. Community ceiling ~200 words. Goose templates its `instructions` per-OS (AX vs UIA vs AT-SPI) so the agent only sees the path that applies. - AGENT_INSTRUCTIONS reaches every MCP client uniformly (Hermes, Cursor, Copilot CLI, plus Claude-Skills-aware agents over MCP). SKILL.md reaches only the 4 agents with a Skills loader. So both surfaces should cross-reference each other rather than duplicate. Changes: 1. **SKILL.md split**: 911 lines → 493-line cross-platform entrypoint (snapshot invariant, CLI/MCP defaults, behavior matrix, canonical loop, pixel-click contract, common errors) + new MACOS.md (475 lines) carrying the no-foreground contract, AppleScript / `open` prohibitions, AXMenuBar navigation, SkyLight click dispatch, and the Apple-Events JS bridge. Existing WINDOWS.md (686), LINUX.md (87), WEB_APPS.md (477), RECORDING.md (120) untouched — they were already separated. (Sibling Swift driver skill at libs/cua-driver/Skills/cua-driver/SKILL.md is 887 lines — also over budget but out of scope for this Rust-port PR.) 2. **AGENT_INSTRUCTIONS in crates/mcp-server/src/protocol.rs**: - Converted from `const &str` to `fn() -> String` so it can be templated at compile time per host (`cfg!(target_os = ...)`). macOS clients see "AX (Accessibility)", Windows see "UIA (UI Automation)", Linux see "AT-SPI" — same pattern as Goose's ComputerController + Open Interpreter's `platform.system()`. - Added a closing line pointing skill-aware harnesses at SKILL.md + the relevant per-OS companion (e.g. MACOS.md on macOS). MCP clients without a skills loader (Hermes, Cursor) ignore the pointer; clients with one get a free description-match hint. - Tightened the workflow: 5 steps including re-snapshot, dropped the redundant list_apps step in favor of launch_app's `windows` array. Still under 200 words. 3. **Thicker per-tool descriptions** on click + screenshot (all 3 platforms): pushed "prefer element_index over pixel coords" guidance onto click (per Playwright's "this is better than screenshot" editorial pattern), and a pointer to get_window_state + max_image_dimension=1568 default onto screenshot. Per-call gating rather than per-session — Anthropic computer-use cookbook shape. Also includes the `TypeTextCharsTool { state: _state }` rename on platform-windows/tools/impl_.rs (the warnings-cleanup change that overlapped this file — the only field rename, struct still intentionally unregistered per the comment at impl_.rs:3991). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(cua-driver-rs): address CodeRabbit review on #1665 Six findings, all valid: 1. SKILL.md:114 — added `bash` language identifier to the canonical- workflow fenced code block (MD040 lint + syntax highlighting). 2. MACOS.md:373 — added `bash` identifier to the osascript activate fallback fence. 3. MACOS.md:414 — added `bash` identifier to the get_window_state-with-javascript example fence. 4. uninstall.sh:78-80 — `--backend=swift` was silently overridden by the auto-Rust-on-non-Darwin branch (the doc calls it an "explicit no-op default" but the code ignored the explicit signal). Added a BACKEND_EXPLICIT flag set by every `--backend=*`/`--experimental-rust` arm; the auto-Rust dispatch checks it and skips when the user pinned a backend. 5. uninstall.sh — `/Applications/CuaDriver.app` is shared between Swift and Rust on macOS (same bundle id `com.trycua.driver` since cua-driver-rs ≥ 0.2.4), so the prior Rust-branch logic could delete a Swift-only Mac's bundle, symlink, and Claude MCP registrations on a stray `uninstall.sh --experimental-rust`. Added a $RUST_INSTALL_PRESENT marker computed up-front from unambiguous Rust artifacts (HOME_DIR, legacy CuaDriverRs.app, LaunchAgent plist, systemd unit) and gated every shared-path removal on it: - CLI symlink case branch (split into "unambiguous Rust paths" and "shared path; require marker") - .app bundle removal (legacy unconditionally; canonical only when marker present) - Claude MCP scrub (the Python heredoc now reads RUST_INSTALL_PRESENT from env and gates the `/Applications/CuaDriver.app` match on it; unambiguous Rust anchors still match unconditionally) 6. protocol.rs:188-209 — converted AGENT_INSTRUCTIONS format! string to a raw string literal (r#"..."#). Drops the backslash line continuations + escaped backticks; identical runtime output, much easier to read. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <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
Pure refactor — no behavior change. Extracts the `rotate_toward` function (~10 LOC) from `platform-linux/src/overlay.rs` and `platform-windows/src/overlay.rs` into the shared `cursor-overlay` crate at `src/util.rs`, plus 4 unit tests covering the wraparound + clamp logic.
`rotate_toward` rotates a current angle toward a desired angle by at most `max_step` radians, picking the short way around. It was identically copy-pasted in both per-OS overlays; consolidating removes the duplication and unit-tests the modular-arithmetic edges.
Diff
Tests
`cargo test -p cursor-overlay --lib util::tests` → 4/4 passing (verified locally + on Windows VM).
The 4 cases:
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit