fix(cua-driver-rs)(macos): surface display-asleep state in capture errors and action results - #2104
fix(cua-driver-rs)(macos): surface display-asleep state in capture errors and action results#2104zqchris wants to merge 1 commit into
Conversation
…rors and action results
When the main display is asleep (idle sleep / lid closed), every
screencapture fails and posted CGEvents may never render — but the
driver reported opaque errors ('screencapture failed for window N',
'No content produced') and cheerful action results ('Posted click …
confirm via screenshot'), while get_window_state's escalation even
told the agent to act off a screenshot that silently failed to
capture. Agents burn their whole budget re-deriving coordinates and
re-posting clicks into the void with no signal that the display is
the problem.
Observed in a real session: an agent drove an Electron app while the
user stepped away; the display slept mid-run, get_window_state
degraded to 'No content produced', zoom failed for every window, and
click kept returning success.
Fix (macOS):
- new display_state module: CGDisplayIsAsleep(CGMainDisplayID()) FFI
+ shared hint/suffix strings, unit-tested via a pure helper
- capture.rs: window/display screencapture failures append the
asleep hint when the display is asleep
- get_window_state: NEW screenshot_error field (structured + text
content) so a missing screenshot is never silent; the
'No content produced' double-failure error carries the asleep hint
- click / double_click / drag / type_text / press_key / hotkey /
scroll success texts append an explicit '😴 Main display is
ASLEEP … do NOT re-derive coordinates' warning while asleep
Verified live against a sleeping Retina display: screenshot_error =
'screencapture failed for window 17554: the main display is asleep,
so macOS cannot capture windows. Wake it (user presence or
`caffeinate -u -t 1`) and retry'. All 113 platform-macos tests pass.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@zqchris is attempting to deploy a commit to the Cua Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds a macOS ChangesAsleep display detection and messaging
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
libs/cua-driver/rust/crates/platform-macos/src/tools/get_window_state.rs (1)
270-278: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winFinal error discards the captured
screenshot_errordetail.
screenshot_erroris populated specifically to surface why the screenshot failed (permissions, WindowServer refusal, etc.), but thecontent.is_empty()branch never includes it — only the generic message plus, conditionally, the asleep hint. The actual root cause captured a few lines earlier is thrown away right when it would matter most (total capture failure).♻️ Proposed fix to include the captured detail
if content.is_empty() { + let detail = screenshot_error + .as_deref() + .map(|e| format!(" (screenshot error: {e})")) + .unwrap_or_default(); if crate::display_state::main_display_asleep() { return ToolResult::error(format!( - "No content produced (neither AX tree nor screenshot succeeded) — {}", - crate::display_state::ASLEEP_CAPTURE_HINT + "No content produced (neither AX tree nor screenshot succeeded){} — {}", + detail, crate::display_state::ASLEEP_CAPTURE_HINT )); } - return ToolResult::error("No content produced (neither AX tree nor screenshot succeeded)"); + return ToolResult::error(format!( + "No content produced (neither AX tree nor screenshot succeeded){}", + detail + )); }🤖 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/platform-macos/src/tools/get_window_state.rs` around lines 270 - 278, The final fallback in get_window_state currently drops the captured screenshot_error, so total capture failures lose the real root cause. Update the content.is_empty() error path in get_window_state to include the stored screenshot_error detail alongside the existing generic message, while still preserving the asleep-case hint from display_state::ASLEEP_CAPTURE_HINT when main_display_asleep() is true. Keep the fix scoped to the existing screenshot capture/result assembly logic so the final ToolResult::error surfaces the actual failure reason.libs/cua-driver/rust/crates/platform-macos/src/display_state.rs (1)
15-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse
core-graphics's existing bindings instead of hand-rolled FFI.
core-graphics0.24.0 is already a dependency in this crate and publicly exportsCGMainDisplayIDandCGDisplayIsAsleepfromcore_graphics::displaywith matching signatures. Re-declaring theseextern "C"symbols here duplicates vetted bindings and risks subtle drift (e.g., the declared return typeu32here vs. upstream'sboolean_t).♻️ Suggested refactor
-use std::os::raw::c_uint; - -extern "C" { - fn CGMainDisplayID() -> c_uint; - fn CGDisplayIsAsleep(display: c_uint) -> u32; -} +use core_graphics::display::{CGDisplayIsAsleep, CGMainDisplayID}; pub fn main_display_asleep() -> bool { unsafe { CGDisplayIsAsleep(CGMainDisplayID()) != 0 } }🤖 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/platform-macos/src/display_state.rs` around lines 15 - 20, The display sleep helpers are duplicating FFI bindings that already exist in core_graphics::display. Update display_state.rs to stop declaring CGMainDisplayID and CGDisplayIsAsleep in the local extern block and instead use the existing core-graphics bindings directly from core_graphics::display, keeping the same call sites in display_state logic but relying on the upstream signatures to avoid drift.libs/cua-driver/rust/crates/platform-macos/src/capture.rs (1)
29-37: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated asleep-check-and-bail logic across two functions.
Both
screenshot_window_bytesandscreenshot_display_bytesrepeat the same "checkmain_display_asleep(), then bail withASLEEP_CAPTURE_HINT" pattern. Extracting a small helper (e.g., indisplay_state.rs) would avoid drift if the hint format changes later.♻️ Suggested helper
// in display_state.rs pub fn asleep_aware_bail(context: &str) -> String { if main_display_asleep() { format!("{context}: {ASLEEP_CAPTURE_HINT}") } else { context.to_string() } }- if !status.success() { - if crate::display_state::main_display_asleep() { - anyhow::bail!( - "screencapture failed for window {window_id}: {}", - crate::display_state::ASLEEP_CAPTURE_HINT - ); - } - anyhow::bail!("screencapture failed for window {window_id}"); - } + if !status.success() { + anyhow::bail!(crate::display_state::asleep_aware_bail( + &format!("screencapture failed for window {window_id}") + )); + }Also applies to: 67-75
🤖 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/platform-macos/src/capture.rs` around lines 29 - 37, The asleep-check-and-bail pattern is duplicated in both screenshot capture paths, so consolidate it into a shared helper instead of repeating the `main_display_asleep()` branch in `screenshot_window_bytes` and `screenshot_display_bytes`. Add a small utility in `display_state` (for example an `asleep_aware_bail` helper) that returns the appropriate error message using `ASLEEP_CAPTURE_HINT`, then call that helper from the capture functions so both window and display failures stay consistent.
🤖 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/platform-macos/src/tools/double_click.rs`:
- Around line 250-253: The AXOpen fast-path in ax_double_click() is still
returning a plain success message without the display-asleep hint. Update the
AXOpen success string to append crate::display_state::asleep_suffix(), matching
the existing double-click success path in double_click() so openable-element
clicks also surface the asleep warning.
---
Nitpick comments:
In `@libs/cua-driver/rust/crates/platform-macos/src/capture.rs`:
- Around line 29-37: The asleep-check-and-bail pattern is duplicated in both
screenshot capture paths, so consolidate it into a shared helper instead of
repeating the `main_display_asleep()` branch in `screenshot_window_bytes` and
`screenshot_display_bytes`. Add a small utility in `display_state` (for example
an `asleep_aware_bail` helper) that returns the appropriate error message using
`ASLEEP_CAPTURE_HINT`, then call that helper from the capture functions so both
window and display failures stay consistent.
In `@libs/cua-driver/rust/crates/platform-macos/src/display_state.rs`:
- Around line 15-20: The display sleep helpers are duplicating FFI bindings that
already exist in core_graphics::display. Update display_state.rs to stop
declaring CGMainDisplayID and CGDisplayIsAsleep in the local extern block and
instead use the existing core-graphics bindings directly from
core_graphics::display, keeping the same call sites in display_state logic but
relying on the upstream signatures to avoid drift.
In `@libs/cua-driver/rust/crates/platform-macos/src/tools/get_window_state.rs`:
- Around line 270-278: The final fallback in get_window_state currently drops
the captured screenshot_error, so total capture failures lose the real root
cause. Update the content.is_empty() error path in get_window_state to include
the stored screenshot_error detail alongside the existing generic message, while
still preserving the asleep-case hint from display_state::ASLEEP_CAPTURE_HINT
when main_display_asleep() is true. Keep the fix scoped to the existing
screenshot capture/result assembly logic so the final ToolResult::error surfaces
the actual failure reason.
🪄 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: 3559dd32-3c97-429f-b315-c62bd5a46a5d
📒 Files selected for processing (11)
libs/cua-driver/rust/crates/platform-macos/src/capture.rslibs/cua-driver/rust/crates/platform-macos/src/display_state.rslibs/cua-driver/rust/crates/platform-macos/src/lib.rslibs/cua-driver/rust/crates/platform-macos/src/tools/click.rslibs/cua-driver/rust/crates/platform-macos/src/tools/double_click.rslibs/cua-driver/rust/crates/platform-macos/src/tools/drag.rslibs/cua-driver/rust/crates/platform-macos/src/tools/get_window_state.rslibs/cua-driver/rust/crates/platform-macos/src/tools/hotkey.rslibs/cua-driver/rust/crates/platform-macos/src/tools/press_key.rslibs/cua-driver/rust/crates/platform-macos/src/tools/scroll.rslibs/cua-driver/rust/crates/platform-macos/src/tools/type_text.rs
| Ok(format!( | ||
| "✅ Double-clicked element [{idx}] at ({cx:.1}, {cy:.1}).{}", | ||
| crate::display_state::asleep_suffix() | ||
| )) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Extend the asleep hint to the AXOpen fast-path.
ax_double_click() still returns a plain AXOpen performed... string, so successful openable-element double-clicks won't include the new display-asleep warning.
Diff
if err == kAXErrorSuccess {
- return Ok(format!("AXOpen performed on element [{idx}]."));
+ return Ok(format!(
+ "AXOpen performed on element [{idx}].{}",
+ crate::display_state::asleep_suffix()
+ ));
}🤖 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/platform-macos/src/tools/double_click.rs` around
lines 250 - 253, The AXOpen fast-path in ax_double_click() is still returning a
plain success message without the display-asleep hint. Update the AXOpen success
string to append crate::display_state::asleep_suffix(), matching the existing
double-click success path in double_click() so openable-element clicks also
surface the asleep warning.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56ddf518a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| changes.result_suffix() | ||
| not driver-verified — confirm via screenshot).{}{}", | ||
| changes.result_suffix(), | ||
| crate::display_state::asleep_suffix() |
There was a problem hiding this comment.
Adding the suffix here only covers the x/y CGEvent success path; the default click({pid, window_id, element_index}) AX branch still returns ToolResult::text(msg) after perform_ax_click without appending display_state::asleep_suffix(). When the display is asleep, the primary element-indexed click therefore reports a normal success with no sleep marker, so callers can keep retrying or re-deriving coordinates despite this commit's new warning contract.
Useful? React with 👍 / 👎.
|
Closing this one — no longer pursuing it on my side. Feel free to pick up the diff if the display-asleep surfacing is still wanted. |
1 participant
Problem
When the macOS main display is asleep (idle sleep, lid closed), the driver goes blind but never says so:
screencapturefails for every window →get_window_state/zoom/debug_image_outreturn opaque errors:screencapture failed for window N,No content produced (neither AX tree nor screenshot succeeded).tracing::warnonly) — and theescalationblock even tells the agent to "act by pixel (x,y) off the screenshot in this response" when no screenshot is in the response.✅ Posted click … confirm via screenshot) — but the app may never render the event, and there is no screenshot to confirm with.An agent with no display-asleep signal does the worst possible thing: it assumes its coordinates are wrong, re-derives them, and keeps re-posting clicks into the void until its budget is gone.
Real-world repro (how we hit this): an agent was driving an Electron app while the user stepped away; the display slept mid-session.
get_window_statedegraded to "No content produced",zoomfailed for every window on the system, andclickkept reporting success. The agent spent the rest of the run second-guessing perfectly correct coordinates.Fix (macOS only)
New
display_statemodule —CGDisplayIsAsleep(CGMainDisplayID())FFI + shared hint/suffix strings; suffix logic split into a pure helper with unit tests.capture.rs— window/display screencapture failures append the asleep hint when the display is asleep (checked only on the failure path; zero happy-path overhead).get_window_state— newscreenshot_errorfield (structured + text content) so a missing screenshot is never silent; theNo content produceddouble-failure error carries the asleep hint.Action tools (
click,double_click,drag,type_text,press_key,hotkey,scroll) — success texts append an explicit warning while asleep:Verification
All 113
platform-macostests pass; 2 new unit tests for the suffix helper.Live-tested against a genuinely sleeping Retina display (built daemon on a temp socket):
Awake path unchanged (suffix is
"", capture errors identical to before).Possible follow-up (not in this PR)
A structured
display_asleep: trueflag on action results would make the state machine-checkable in structured-only consumers (the CLIcalloutput prints structured only); left out to keep this change text-first and minimal.🤖 Generated with Claude Code
Summary by CodeRabbit