-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat(cua-driver-rs)(recording): native ScreenCaptureKit on macOS + app_state.json/click.png regressions #1720
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
9d1844a
eb4bf5e
1cfdd2b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -18,7 +18,7 @@ use std::time::Instant; | |||||||||||||||||||
| use serde_json::Value; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| use crate::cursor_sampler::CursorSampler; | ||||||||||||||||||||
| use crate::video::{VideoMetadata, VideoRecorder}; | ||||||||||||||||||||
| use crate::video::{self, VideoBackend, VideoMetadata}; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // ── Platform screenshot callback ───────────────────────────────────────────── | ||||||||||||||||||||
| // | ||||||||||||||||||||
|
|
@@ -49,6 +49,35 @@ pub fn set_click_marker_fn(f: impl Fn(&[u8], f64, f64) -> Option<Vec<u8>> + Send | |||||||||||||||||||
| let _ = CLICK_MARKER_FN.set(Box::new(f)); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // ── Platform AX-snapshot callback ──────────────────────────────────────────── | ||||||||||||||||||||
| // | ||||||||||||||||||||
| // Takes (window_id, pid) and returns JSON bytes for `app_state.json` (the | ||||||||||||||||||||
| // post-action AX/UIA snapshot), or None if no snapshot is available on this | ||||||||||||||||||||
| // platform. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| type AxSnapshotFnBox = Box<dyn Fn(Option<u64>, Option<i64>) -> Option<Vec<u8>> + Send + Sync>; | ||||||||||||||||||||
| static AX_SNAPSHOT_FN: OnceLock<AxSnapshotFnBox> = OnceLock::new(); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /// Register the platform-specific AX/UIA snapshot callback. Call once at startup. | ||||||||||||||||||||
| pub fn set_ax_snapshot_fn(f: impl Fn(Option<u64>, Option<i64>) -> Option<Vec<u8>> + Send + Sync + 'static) { | ||||||||||||||||||||
| let _ = AX_SNAPSHOT_FN.set(Box::new(f)); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // ── Platform element-bounds callback ───────────────────────────────────────── | ||||||||||||||||||||
| // | ||||||||||||||||||||
| // Resolves an element_index to its center point in window-local screenshot | ||||||||||||||||||||
| // pixels (the same coordinate space as the existing `(cx, cy)` arg to | ||||||||||||||||||||
| // `CLICK_MARKER_FN`). Used so click.png is also written on element-indexed | ||||||||||||||||||||
| // clicks, not just pixel-addressed ones. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| type ElementBoundsFnBox = Box<dyn Fn(u64, i64, u32) -> Option<(f64, f64)> + Send + Sync>; | ||||||||||||||||||||
| static ELEMENT_BOUNDS_FN: OnceLock<ElementBoundsFnBox> = OnceLock::new(); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /// Register the platform-specific element-bounds resolver. Args: (window_id, pid, element_index). | ||||||||||||||||||||
| pub fn set_element_bounds_fn(f: impl Fn(u64, i64, u32) -> Option<(f64, f64)> + Send + Sync + 'static) { | ||||||||||||||||||||
| let _ = ELEMENT_BOUNDS_FN.set(Box::new(f)); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /// Persistent recording session state (singleton per process). | ||||||||||||||||||||
| pub struct RecordingSession { | ||||||||||||||||||||
| inner: Mutex<RecordingInner>, | ||||||||||||||||||||
|
|
@@ -63,9 +92,10 @@ struct RecordingInner { | |||||||||||||||||||
| /// matches the action-timeline anchor in `action.json`. | ||||||||||||||||||||
| session_monotonic_start: Option<Instant>, | ||||||||||||||||||||
| last_error: Option<String>, | ||||||||||||||||||||
| /// Live ffmpeg subprocess when video capture is active. Recreated | ||||||||||||||||||||
| /// per session. | ||||||||||||||||||||
| video: Option<VideoRecorder>, | ||||||||||||||||||||
| /// Live video backend when capture is active. Recreated per | ||||||||||||||||||||
| /// session. The concrete type is platform-determined (SCKit on | ||||||||||||||||||||
| /// macOS, ffmpeg subprocess elsewhere). | ||||||||||||||||||||
| video: Option<Box<dyn VideoBackend>>, | ||||||||||||||||||||
| /// Recorded after `stop()` until the next start — exposed in | ||||||||||||||||||||
| /// `current_state()` so callers can read the finalized video info | ||||||||||||||||||||
| /// after stopping. | ||||||||||||||||||||
|
|
@@ -144,7 +174,7 @@ impl RecordingSession { | |||||||||||||||||||
| let mut video_error: Option<String> = None; | ||||||||||||||||||||
| if record_video { | ||||||||||||||||||||
| let path = dir.join("recording.mp4"); | ||||||||||||||||||||
| match VideoRecorder::start(&path) { | ||||||||||||||||||||
| match video::start_video(&path) { | ||||||||||||||||||||
| Ok(rec) => { | ||||||||||||||||||||
| inner.video = Some(rec); | ||||||||||||||||||||
| video_present = true; | ||||||||||||||||||||
|
|
@@ -330,14 +360,23 @@ fn write_turn( | |||||||||||||||||||
| // Extract window_id and pid from args for screenshot capture. | ||||||||||||||||||||
| let window_id = args.opt_u64("window_id"); | ||||||||||||||||||||
| let pid = args.opt_i64("pid"); | ||||||||||||||||||||
| let element_index = args.opt_u64("element_index"); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Extract click point for click-family tools. | ||||||||||||||||||||
| // Extract click point for click-family tools. Falls back to the | ||||||||||||||||||||
| // platform element_index → window-local-pixels resolver when the call | ||||||||||||||||||||
| // used `element_index` instead of explicit `x, y`, so click.png is | ||||||||||||||||||||
| // written for AX-indexed clicks too. | ||||||||||||||||||||
| let click_point: Option<(f64, f64)> = if matches!( | ||||||||||||||||||||
| tool_name, "click" | "double_click" | "right_click" | ||||||||||||||||||||
| ) { | ||||||||||||||||||||
| match (args.opt_f64("x"), args.opt_f64("y")) { | ||||||||||||||||||||
| (Some(x), Some(y)) => Some((x, y)), | ||||||||||||||||||||
| _ => None, | ||||||||||||||||||||
| _ => match (window_id, pid, element_index, ELEMENT_BOUNDS_FN.get()) { | ||||||||||||||||||||
| (Some(wid), Some(p), Some(idx), Some(f)) => { | ||||||||||||||||||||
| u32::try_from(idx).ok().and_then(|idx32| f(wid, p, idx32)) | ||||||||||||||||||||
| } | ||||||||||||||||||||
| _ => None, | ||||||||||||||||||||
|
Comment on lines
+374
to
+378
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate
Proposed fix- _ => match (window_id, pid, element_index, ELEMENT_BOUNDS_FN.get()) {
- (Some(wid), Some(p), Some(idx), Some(f)) => f(wid, p, idx as u32),
- _ => None,
- },
+ _ => match (window_id, pid, element_index, ELEMENT_BOUNDS_FN.get()) {
+ (Some(wid), Some(p), Some(idx), Some(f)) => {
+ u32::try_from(idx).ok().and_then(|idx32| f(wid, p, idx32))
+ }
+ _ => None,
+ },📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||
| }, | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } else { | ||||||||||||||||||||
| None | ||||||||||||||||||||
|
|
@@ -356,6 +395,14 @@ fn write_turn( | |||||||||||||||||||
| } | ||||||||||||||||||||
| write_json_atomic(&turn_dir.join("action.json"), &payload)?; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Post-action AX/UIA snapshot — omitted on platforms that don't expose | ||||||||||||||||||||
| // a cheap snapshot helper (today: Linux ATSPI). | ||||||||||||||||||||
| if let Some(ax_fn) = AX_SNAPSHOT_FN.get() { | ||||||||||||||||||||
| if let Some(json_bytes) = ax_fn(window_id, pid) { | ||||||||||||||||||||
| let _ = std::fs::write(turn_dir.join("app_state.json"), &json_bytes); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Capture screenshot if a callback is registered. | ||||||||||||||||||||
| if let Some(screenshot_fn) = SCREENSHOT_FN.get() { | ||||||||||||||||||||
| if let Some(png_bytes) = screenshot_fn(window_id, pid) { | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -69,12 +69,18 @@ impl Tool for StartRecordingTool { | |
| Turn folders are named `turn-00001/`, `turn-00002/`, etc. Turn \ | ||
| numbering restarts at 1 each time recording is (re-)started.\n\n\ | ||
| **Video is on by default** — the main display is captured to \ | ||
| `<output_dir>/recording.mp4` (H.264 / yuv420p / 30 fps) via an ffmpeg \ | ||
| subprocess for the lifetime of the session. Pass `record_video: false` \ | ||
| to opt out. Requires ffmpeg on PATH (winget install Gyan.FFmpeg / \ | ||
| brew install ffmpeg / apt install ffmpeg); when ffmpeg is missing the \ | ||
| per-turn capture (screenshots + action.json) still runs and the \ | ||
| session's `last_error` field carries the ffmpeg-not-found message.\n\n\ | ||
| `<output_dir>/recording.mp4` (H.264 / 30 fps) for the lifetime of \ | ||
| the session. Pass `record_video: false` to opt out.\n\n\ | ||
| **macOS uses native ScreenCaptureKit** (in-process SCStream + \ | ||
| SCRecordingOutput) so video inherits cua-driver's own Screen \ | ||
| Recording grant — no extra TCC prompt, no ffmpeg subprocess. \ | ||
| Requires macOS 15.0+.\n\n\ | ||
| **Windows + Linux use an ffmpeg subprocess** (`gdigrab` / \ | ||
| `x11grab` + libx264). Requires ffmpeg on PATH (winget install \ | ||
| Gyan.FFmpeg / apt install ffmpeg); when ffmpeg is missing or \ | ||
| fails on startup the per-turn capture (screenshots + \ | ||
| action.json) still runs and the session's `last_error` field \ | ||
| carries the diagnostic.\n\n\ | ||
|
Comment on lines
+72
to
+83
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Align Line 74-Line 77 correctly says macOS uses ScreenCaptureKit, but Line 98-Line 100 still says ffmpeg is required. That contradiction can mislead clients that surface schema descriptions. Suggested doc fix "record_video": {
"type": "boolean",
- "description": "Capture the main display to <output_dir>/recording.mp4. \
- Default: true. Set to false to record only the per-turn \
- screenshots + JSON. Requires ffmpeg on PATH."
+ "description": "Capture the main display to <output_dir>/recording.mp4. \
+ Default: true. Set to false to record only the per-turn \
+ screenshots + JSON. On macOS this uses native \
+ ScreenCaptureKit; on Windows/Linux it requires ffmpeg on PATH."
}Also applies to: 96-101 🤖 Prompt for AI Agents |
||
| State persists for the life of the daemon / MCP session; a restart \ | ||
| resets to disabled with no on-disk state. Call `stop_recording` to \ | ||
| disable + finalize the mp4.".into(), | ||
|
|
@@ -91,7 +97,9 @@ impl Tool for StartRecordingTool { | |
| "type": "boolean", | ||
| "description": "Capture the main display to <output_dir>/recording.mp4. \ | ||
| Default: true. Set to false to record only the per-turn \ | ||
| screenshots + JSON. Requires ffmpeg on PATH." | ||
| screenshots + JSON. On macOS this uses native \ | ||
| ScreenCaptureKit (no extra TCC prompt, macOS 15.0+); on \ | ||
| Windows + Linux it requires ffmpeg on PATH." | ||
| } | ||
| }, | ||
| "additionalProperties": false | ||
|
|
@@ -114,11 +122,17 @@ impl Tool for StartRecordingTool { | |
| match self.session.start(output_dir.as_deref().unwrap(), record_video) { | ||
| Ok(()) => { | ||
| let state = self.session.current_state(); | ||
| // When the caller asked for video and it failed (e.g. macOS | ||
| // ffmpeg TCC prompt deadlock), surface the actual error | ||
| // prominently — the per-turn capture still runs, but the | ||
| // caller deserves to know the mp4 won't materialize. | ||
| let video_failed = record_video && !state.video_active; | ||
| let video_note = if record_video && state.video_active { | ||
| " (video → recording.mp4)" | ||
| } else if record_video && !state.video_active { | ||
| " (video requested but ffmpeg not available — see last_error)" | ||
| } else { "" }; | ||
| " (video → recording.mp4)".to_string() | ||
| } else if video_failed { | ||
| let err = state.last_error.clone().unwrap_or_else(|| "unknown".into()); | ||
| format!("\n\n⚠️ Video capture failed (per-turn JSON+screenshot still running):\n{err}") | ||
| } else { String::new() }; | ||
| let msg = format!("✅ Recording started -> {}{}", | ||
| state.output_dir.as_deref().unwrap_or("?"), | ||
| video_note); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolve the platform-support contradiction in this doc.
Line 33-Line 38 now documents Windows/Linux recording via ffmpeg, but the header at Line 3-Line 8 still says recording is macOS-only. Please make those sections consistent.
Suggested doc fix
📝 Committable suggestion
🤖 Prompt for AI Agents