fix(cua-driver-rs)(macos): run the agent-cursor overlay in the serve daemon - #1788
Closed
f-trycua wants to merge 1 commit into
Closed
fix(cua-driver-rs)(macos): run the agent-cursor overlay in the serve daemon#1788f-trycua wants to merge 1 commit into
f-trycua wants to merge 1 commit into
Conversation
…daemon The overlay NSWindow + AppKit render loop were only wired into the in-process `mcp` arm. In the daemon-proxy setup users actually run (`mcp` relaunches `open -n -g … serve` and proxies to it for correct TCC attribution), the DAEMON performs the clicks/AX presses but never inited or ran the overlay — its main thread parked in `serve_handle.join()`. So `set_agent_cursor_enabled` flipped registry flags and clicks sent OverlayCommands, but CMD_TX was never set → every cursor command was a silent no-op and the agent cursor never appeared (confirmed: daemon main thread in pthread_join, zero tiny_skia / SkyLight / NSWindow activity). Fix: the Serve arm now builds cursor_cfg, inits the overlay channel before spawning the serve thread, and — when the cursor is enabled — parks the main thread in `overlay::run_on_main_thread()` (mirrors the Mcp arm) instead of join. It self-guards on has_graphic_access() and falls back to join when the daemon has no Window Server session, so headless serving is unaffected. Verified: with the fix the serve daemon's main thread is now in __CFRunLoopRun / -[NSApplication run] with tiny_skia + SkyLight overlay render activity, and it still serves (list_windows over the socket works). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📝 WalkthroughWalkthroughOn macOS ChangesmacOS Serve Startup Sequence
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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: 1
🤖 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/rust/crates/cua-driver/src/main.rs`:
- Around line 336-346: The PiP branch currently returns early and skips
initializing/rendering the cursor overlay when both pip_cfg.enabled and
cursor_cfg.enabled are true; update the control flow so that when
cursor_cfg.enabled is true you always call
platform_macos::cursor::overlay::run_on_main_thread() (even if pip_cfg.enabled
is also true), then run platform_macos::pip::run_appkit_main_loop() as needed
and finally join the serve_handle (serve_handle.join()) so the overlay actually
renders in combined PiP+overlay mode; check the existing maybe_init_pip() usage
to preserve any initialization ordering.
🪄 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: f2e121c0-5a8f-40f2-ab87-488a0c1fe7ee
📒 Files selected for processing (1)
libs/cua-driver/rust/crates/cua-driver/src/main.rs
Comment on lines
336
to
+346
| if pip_cfg.enabled { | ||
| platform_macos::pip::run_appkit_main_loop(); | ||
| } else if cursor_cfg.enabled { | ||
| // Render the agent-cursor overlay: park the main thread in the | ||
| // AppKit run loop so the overlay NSWindow draws. `run_on_main_thread` | ||
| // self-guards on `has_graphic_access()` and returns immediately | ||
| // when the daemon has no Window Server session — fall through to | ||
| // join so the daemon still serves headless. The serve thread runs | ||
| // on its background thread regardless. | ||
| platform_macos::cursor::overlay::run_on_main_thread(); | ||
| let _ = serve_handle.join(); |
Contributor
There was a problem hiding this comment.
Don't let the PiP branch bypass the cursor overlay.
When PiP and the cursor overlay are both enabled, this ordering never calls platform_macos::cursor::overlay::run_on_main_thread(). In the same file, the mcp path still does maybe_init_pip() and then runs the overlay on the main thread, so serve now regresses the combined mode to "overlay initialized but never rendered."
Suggested shape
- if pip_cfg.enabled {
- platform_macos::pip::run_appkit_main_loop();
- } else if cursor_cfg.enabled {
+ if cursor_cfg.enabled {
// Render the agent-cursor overlay: park the main thread in the
// AppKit run loop so the overlay NSWindow draws. `run_on_main_thread`
// self-guards on `has_graphic_access()` and returns immediately
// when the daemon has no Window Server session — fall through to
// join so the daemon still serves headless. The serve thread runs
// on its background thread regardless.
platform_macos::cursor::overlay::run_on_main_thread();
let _ = serve_handle.join();
+ } else if pip_cfg.enabled {
+ platform_macos::pip::run_appkit_main_loop();
} else {
let _ = serve_handle.join();
}📝 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 pip_cfg.enabled { | |
| platform_macos::pip::run_appkit_main_loop(); | |
| } else if cursor_cfg.enabled { | |
| // Render the agent-cursor overlay: park the main thread in the | |
| // AppKit run loop so the overlay NSWindow draws. `run_on_main_thread` | |
| // self-guards on `has_graphic_access()` and returns immediately | |
| // when the daemon has no Window Server session — fall through to | |
| // join so the daemon still serves headless. The serve thread runs | |
| // on its background thread regardless. | |
| platform_macos::cursor::overlay::run_on_main_thread(); | |
| let _ = serve_handle.join(); | |
| if cursor_cfg.enabled { | |
| // Render the agent-cursor overlay: park the main thread in the | |
| // AppKit run loop so the overlay NSWindow draws. `run_on_main_thread` | |
| // self-guards on `has_graphic_access()` and returns immediately | |
| // when the daemon has no Window Server session — fall through to | |
| // join so the daemon still serves headless. The serve thread runs | |
| // on its background thread regardless. | |
| platform_macos::cursor::overlay::run_on_main_thread(); | |
| let _ = serve_handle.join(); | |
| } else if pip_cfg.enabled { | |
| platform_macos::pip::run_appkit_main_loop(); | |
| } else { | |
| let _ = serve_handle.join(); | |
| } |
🤖 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/rust/crates/cua-driver/src/main.rs` around lines 336 - 346,
The PiP branch currently returns early and skips initializing/rendering the
cursor overlay when both pip_cfg.enabled and cursor_cfg.enabled are true; update
the control flow so that when cursor_cfg.enabled is true you always call
platform_macos::cursor::overlay::run_on_main_thread() (even if pip_cfg.enabled
is also true), then run platform_macos::pip::run_appkit_main_loop() as needed
and finally join the serve_handle (serve_handle.join()) so the overlay actually
renders in combined PiP+overlay mode; check the existing maybe_init_pip() usage
to preserve any initialization ordering.
Collaborator
Author
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.
The bug (why the agent cursor was never visible)
The overlay NSWindow + AppKit render loop were wired into the in-process
mcparm but never theservedaemon arm. In the daemon-proxy setup everyone actually runs —cua-driver mcprelaunchesopen -n -g … serveand proxies to it for correct TCC attribution — the daemon does the clicking but parked its main thread inserve_handle.join()and never inited/ran the overlay. SoCMD_TXwas unset and everyOverlayCommandwas a silent no-op; the cursor could not render.Confirmed empirically on the running daemon: main thread in
pthread_join, zerotiny_skia/SkyLight/NSWindowactivity.Fix
The Serve arm now builds
cursor_cfg, inits the overlay channel before spawning the serve thread, and (when the cursor is enabled) parks main inoverlay::run_on_main_thread()— mirroring the Mcp arm — instead ofjoin. It self-guards onhas_graphic_access()and falls back tojoinwhen there's no Window Server session, so headless serving is unaffected. PiP path unchanged.Verified
After the fix the serve daemon's main thread runs
__CFRunLoopRun/-[NSApplication run]withtiny_skia+SkyLightoverlay activity, and still serves (list_windowsover the socket works). On-screen pixel confirmation is a manual VM eyeball (redo the Calculator AX run → the agent cursor now renders + glides).🤖 Generated with Claude Code
Summary by CodeRabbit