From 9b58f099d61487c7014744291cbea511eab2533a Mon Sep 17 00:00:00 2001 From: Lahfir Date: Wed, 10 Jun 2026 16:17:22 -0700 Subject: [PATCH] fix: gate the ref-gesture focus raise on the interaction policy Ref-addressed hover/drag raised the target app unconditionally, violating the headless no-implicit-focus-steal contract. The point resolver now returns the owning pid instead of focusing, and commands decide: headless never raises, --headed raises once (drag focuses only the from-app, fixing the cross-app double-focus). Responses report focused:true so multi-app agents can detect the frontmost change. --- crates/core/src/commands/drag.rs | 16 ++- crates/core/src/commands/drag_tests.rs | 96 +++++++++++-- crates/core/src/commands/helpers.rs | 46 +++++- crates/core/src/commands/hover.rs | 24 +++- crates/core/src/commands/hover_tests.rs | 134 ++++++++++++++++++ crates/core/src/context.rs | 19 ++- ...-gesture-headless-capability-2026-06-10.md | 2 +- .../references/commands-interaction.md | 4 + 8 files changed, 312 insertions(+), 29 deletions(-) create mode 100644 crates/core/src/commands/hover_tests.rs diff --git a/crates/core/src/commands/drag.rs b/crates/core/src/commands/drag.rs index b4dd2ef5..a04b77d1 100644 --- a/crates/core/src/commands/drag.rs +++ b/crates/core/src/commands/drag.rs @@ -1,7 +1,9 @@ use crate::{ action::DragParams, adapter::PlatformAdapter, - commands::helpers::{PointResolveArgs, resolve_point_from_ref_or_xy_with_context}, + commands::helpers::{ + PointResolveArgs, focus_for_physical_input, resolve_point_from_ref_or_xy_with_context, + }, context::CommandContext, error::AppError, }; @@ -42,21 +44,25 @@ pub fn execute( adapter, context, )?; + let focused = focus_for_physical_input(from.pid, adapter, context)?; let params = DragParams { - from: from.clone(), - to: to.clone(), + from: from.point.clone(), + to: to.point.clone(), duration_ms: args.duration_ms, drop_delay_ms: args.drop_delay_ms, }; adapter.drag(params)?; let mut response = json!({ "dragged": true, - "from": { "x": from.x, "y": from.y }, - "to": { "x": to.x, "y": to.y } + "from": { "x": from.point.x, "y": from.point.y }, + "to": { "x": to.point.x, "y": to.point.y } }); if let Some(drop_delay_ms) = args.drop_delay_ms { response["drop_delay_ms"] = json!(drop_delay_ms); } + if focused { + response["focused"] = json!(true); + } Ok(response) } diff --git a/crates/core/src/commands/drag_tests.rs b/crates/core/src/commands/drag_tests.rs index 90e0a07f..09c07ada 100644 --- a/crates/core/src/commands/drag_tests.rs +++ b/crates/core/src/commands/drag_tests.rs @@ -12,12 +12,14 @@ use std::sync::Mutex; struct DragCaptureAdapter { captured: Mutex>, + focused_pids: Mutex>, } impl DragCaptureAdapter { fn new() -> Self { Self { captured: Mutex::new(None), + focused_pids: Mutex::new(Vec::new()), } } } @@ -36,6 +38,11 @@ impl PlatformAdapter for DragCaptureAdapter { })) } + fn focus_app(&self, pid: i32) -> Result<(), AdapterError> { + self.focused_pids.lock().unwrap().push(pid); + Ok(()) + } + fn drag(&self, params: DragParams) -> Result<(), AdapterError> { *self.captured.lock().unwrap() = Some(params); Ok(()) @@ -79,13 +86,9 @@ fn drop_delay_omitted_uses_adapter_default_and_no_response_field() { assert_eq!(captured.drop_delay_ms, None); } -#[test] -fn drag_resolves_ref_bounds_to_center_point() { - let _guard = HomeGuard::new(); - let store = RefStore::new().unwrap(); - let mut refmap = RefMap::new(); - refmap.allocate(RefEntry { - pid: 1, +fn ref_entry(pid: i32) -> RefEntry { + RefEntry { + pid, role: "button".into(), name: Some("Item".into()), value: None, @@ -101,7 +104,35 @@ fn drag_resolves_ref_bounds_to_center_point() { root_ref: None, path_is_absolute: false, path: smallvec::SmallVec::new(), - }); + } +} + +fn cross_app_snapshot() -> String { + let store = RefStore::new().unwrap(); + let mut refmap = RefMap::new(); + refmap.allocate(ref_entry(1)); + refmap.allocate(ref_entry(2)); + store.save_new_snapshot(&refmap).unwrap() +} + +fn cross_app_args(snapshot_id: String) -> DragArgs { + DragArgs { + from_ref: Some("@e1".into()), + from_xy: None, + to_ref: Some("@e2".into()), + to_xy: None, + snapshot_id: Some(snapshot_id), + duration_ms: None, + drop_delay_ms: None, + } +} + +#[test] +fn drag_resolves_ref_bounds_to_center_point() { + let _guard = HomeGuard::new(); + let store = RefStore::new().unwrap(); + let mut refmap = RefMap::new(); + refmap.allocate(ref_entry(1)); let snapshot_id = store.save_new_snapshot(&refmap).unwrap(); let adapter = DragCaptureAdapter::new(); @@ -119,3 +150,52 @@ fn drag_resolves_ref_bounds_to_center_point() { let captured = adapter.captured.lock().unwrap().clone().unwrap(); assert_eq!((captured.from.x, captured.from.y), (30.0, 50.0)); } + +#[test] +fn headless_ref_drag_never_steals_focus() { + let _guard = HomeGuard::new(); + let snapshot_id = cross_app_snapshot(); + let adapter = DragCaptureAdapter::new(); + + let value = execute( + cross_app_args(snapshot_id), + &adapter, + &CommandContext::default(), + ) + .unwrap(); + + assert!(adapter.focused_pids.lock().unwrap().is_empty()); + assert!(value.get("focused").is_none()); +} + +#[test] +fn headed_ref_drag_focuses_only_the_from_app_once() { + let _guard = HomeGuard::new(); + let snapshot_id = cross_app_snapshot(); + let adapter = DragCaptureAdapter::new(); + + let value = execute( + cross_app_args(snapshot_id), + &adapter, + &CommandContext::default().with_headed(true), + ) + .unwrap(); + + assert_eq!(*adapter.focused_pids.lock().unwrap(), vec![1]); + assert_eq!(value["focused"], true); +} + +#[test] +fn headed_xy_drag_never_steals_focus() { + let adapter = DragCaptureAdapter::new(); + + let value = execute( + xy_args(None), + &adapter, + &CommandContext::default().with_headed(true), + ) + .unwrap(); + + assert!(adapter.focused_pids.lock().unwrap().is_empty()); + assert!(value.get("focused").is_none()); +} diff --git a/crates/core/src/commands/helpers.rs b/crates/core/src/commands/helpers.rs index 3d3e6c81..9dc8aab5 100644 --- a/crates/core/src/commands/helpers.rs +++ b/crates/core/src/commands/helpers.rs @@ -29,6 +29,11 @@ pub(crate) struct PointResolveArgs<'a> { pub missing_input_message: &'a str, } +pub(crate) struct ResolvedPoint { + pub point: Point, + pub pid: Option, +} + pub(crate) fn resolve_ref_with_context<'a>( ref_id: &str, snapshot_id: Option<&str>, @@ -161,24 +166,53 @@ pub(crate) fn resolve_point_from_ref_or_xy_with_context( args: PointResolveArgs<'_>, adapter: &dyn PlatformAdapter, context: &CommandContext, -) -> Result { +) -> Result { if let Some(ref_id) = args.ref_id { let (entry, handle) = resolve_ref_with_context(ref_id, args.snapshot_id, adapter, context)?; let bounds = adapter .get_element_bounds(handle.handle())? .ok_or_else(|| AppError::invalid_input(format!("Element {ref_id} has no bounds")))?; - let _ = adapter.focus_app(entry.pid); - return Ok(Point { - x: bounds.x + bounds.width / 2.0, - y: bounds.y + bounds.height / 2.0, + return Ok(ResolvedPoint { + point: Point { + x: bounds.x + bounds.width / 2.0, + y: bounds.y + bounds.height / 2.0, + }, + pid: Some(entry.pid), }); } if let Some((x, y)) = args.xy { - return Ok(Point { x, y }); + return Ok(ResolvedPoint { + point: Point { x, y }, + pid: None, + }); } Err(AppError::invalid_input(args.missing_input_message)) } +/// Raises the app that owns a ref-resolved point before physical input is +/// synthesized, so the events land on its window instead of whatever happens +/// to be frontmost. Headless never steals focus (`--headed` opts in), and a +/// raise failure downgrades to un-focused input rather than erroring. +pub(crate) fn focus_for_physical_input( + pid: Option, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { + let Some(pid) = pid else { return Ok(false) }; + if !context.physical_input_policy().allow_focus_steal { + return Ok(false); + } + let focused = match adapter.focus_app(pid) { + Ok(()) => true, + Err(err) => { + tracing::debug!("focus before physical input failed for pid {pid}: {err}"); + false + } + }; + context.trace_lazy("input.focus_app", || json!({ "pid": pid, "ok": focused }))?; + Ok(focused) +} + pub(crate) fn window_op_command( args: AppArgs, adapter: &dyn PlatformAdapter, diff --git a/crates/core/src/commands/hover.rs b/crates/core/src/commands/hover.rs index b4a9687c..65bbe337 100644 --- a/crates/core/src/commands/hover.rs +++ b/crates/core/src/commands/hover.rs @@ -1,7 +1,10 @@ use crate::{ - action::{MouseButton, MouseEvent, MouseEventKind, Point}, + action::{MouseButton, MouseEvent, MouseEventKind}, adapter::PlatformAdapter, - commands::helpers::{PointResolveArgs, resolve_point_from_ref_or_xy_with_context}, + commands::helpers::{ + PointResolveArgs, ResolvedPoint, focus_for_physical_input, + resolve_point_from_ref_or_xy_with_context, + }, context::CommandContext, error::AppError, }; @@ -19,23 +22,28 @@ pub fn execute( adapter: &dyn PlatformAdapter, context: &CommandContext, ) -> Result { - let point = resolve_hover_point(&args, adapter, context)?; + let resolved = resolve_hover_point(&args, adapter, context)?; + let focused = focus_for_physical_input(resolved.pid, adapter, context)?; adapter.mouse_event(MouseEvent { kind: MouseEventKind::Move, - point: point.clone(), + point: resolved.point.clone(), button: MouseButton::Left, })?; if let Some(ms) = args.duration_ms { std::thread::sleep(std::time::Duration::from_millis(ms)); } - Ok(json!({ "hovered": true, "x": point.x, "y": point.y })) + let mut response = json!({ "hovered": true, "x": resolved.point.x, "y": resolved.point.y }); + if focused { + response["focused"] = json!(true); + } + Ok(response) } fn resolve_hover_point( args: &HoverArgs, adapter: &dyn PlatformAdapter, context: &CommandContext, -) -> Result { +) -> Result { resolve_point_from_ref_or_xy_with_context( PointResolveArgs { ref_id: args.ref_id.as_deref(), @@ -47,3 +55,7 @@ fn resolve_hover_point( context, ) } + +#[cfg(test)] +#[path = "hover_tests.rs"] +mod tests; diff --git a/crates/core/src/commands/hover_tests.rs b/crates/core/src/commands/hover_tests.rs new file mode 100644 index 00000000..9a59837f --- /dev/null +++ b/crates/core/src/commands/hover_tests.rs @@ -0,0 +1,134 @@ +use super::*; +use crate::{ + adapter::NativeHandle, + error::AdapterError, + node::Rect, + refs::{RefEntry, RefMap}, + refs_store::RefStore, + refs_test_support::HomeGuard, +}; +use std::sync::Mutex; + +struct HoverCaptureAdapter { + moved_to: Mutex>, + focused_pids: Mutex>, +} + +impl HoverCaptureAdapter { + fn new() -> Self { + Self { + moved_to: Mutex::new(None), + focused_pids: Mutex::new(Vec::new()), + } + } +} + +impl PlatformAdapter for HoverCaptureAdapter { + fn resolve_element_strict(&self, _entry: &RefEntry) -> Result { + Ok(NativeHandle::null()) + } + + fn get_element_bounds(&self, _handle: &NativeHandle) -> Result, AdapterError> { + Ok(Some(Rect { + x: 100.0, + y: 200.0, + width: 20.0, + height: 10.0, + })) + } + + fn focus_app(&self, pid: i32) -> Result<(), AdapterError> { + self.focused_pids.lock().unwrap().push(pid); + Ok(()) + } + + fn mouse_event(&self, event: MouseEvent) -> Result<(), AdapterError> { + *self.moved_to.lock().unwrap() = Some(event); + Ok(()) + } +} + +fn ref_snapshot(pid: i32) -> String { + let store = RefStore::new().unwrap(); + let mut refmap = RefMap::new(); + refmap.allocate(RefEntry { + pid, + role: "button".into(), + name: Some("Target".into()), + value: None, + description: None, + states: vec![], + bounds: None, + bounds_hash: None, + available_actions: vec!["Click".into()], + source_app: None, + source_window_id: None, + source_window_title: None, + source_surface: crate::adapter::SnapshotSurface::Window, + root_ref: None, + path_is_absolute: false, + path: smallvec::SmallVec::new(), + }); + store.save_new_snapshot(&refmap).unwrap() +} + +fn ref_args(snapshot_id: String) -> HoverArgs { + HoverArgs { + ref_id: Some("@e1".into()), + snapshot_id: Some(snapshot_id), + xy: None, + duration_ms: None, + } +} + +#[test] +fn headless_ref_hover_never_steals_focus() { + let _guard = HomeGuard::new(); + let snapshot_id = ref_snapshot(42); + let adapter = HoverCaptureAdapter::new(); + + let value = execute(ref_args(snapshot_id), &adapter, &CommandContext::default()).unwrap(); + + assert_eq!(value["hovered"], true); + assert!(adapter.focused_pids.lock().unwrap().is_empty()); + assert!(value.get("focused").is_none()); +} + +#[test] +fn headed_ref_hover_focuses_target_app_once() { + let _guard = HomeGuard::new(); + let snapshot_id = ref_snapshot(42); + let adapter = HoverCaptureAdapter::new(); + + let value = execute( + ref_args(snapshot_id), + &adapter, + &CommandContext::default().with_headed(true), + ) + .unwrap(); + + assert_eq!(*adapter.focused_pids.lock().unwrap(), vec![42]); + assert_eq!(value["focused"], true); + assert_eq!(value["x"], 110.0); + assert_eq!(value["y"], 205.0); +} + +#[test] +fn headed_xy_hover_never_steals_focus() { + let adapter = HoverCaptureAdapter::new(); + + let value = execute( + HoverArgs { + ref_id: None, + snapshot_id: None, + xy: Some((5.0, 6.0)), + duration_ms: None, + }, + &adapter, + &CommandContext::default().with_headed(true), + ) + .unwrap(); + + assert!(adapter.focused_pids.lock().unwrap().is_empty()); + assert!(value.get("focused").is_none()); +} diff --git a/crates/core/src/context.rs b/crates/core/src/context.rs index fe75a8ec..a4f25895 100644 --- a/crates/core/src/context.rs +++ b/crates/core/src/context.rs @@ -43,12 +43,25 @@ impl CommandContext { /// because typing requires focus). `--headed` upgrades any base to `headed`, /// unlocking the cursor/OS-input fallbacks instead of failing closed. pub fn request(&self, action: Action, base: InteractionPolicy) -> ActionRequest { - let policy = if self.headed { + ActionRequest { + action, + policy: self.policy_with_base(base), + } + } + + /// Policy for the raw physical-input commands (hover, drag, mouse-*). + /// They have no semantic action chain, so the base is fully headless: + /// a ref-addressed gesture may not steal focus unless `--headed` opts in. + pub fn physical_input_policy(&self) -> InteractionPolicy { + self.policy_with_base(InteractionPolicy::headless()) + } + + fn policy_with_base(&self, base: InteractionPolicy) -> InteractionPolicy { + if self.headed { InteractionPolicy::headed() } else { base - }; - ActionRequest { action, policy } + } } pub fn for_batch_item(&self, session_id: Option) -> Result { diff --git a/docs/solutions/best-practices/macos-gesture-headless-capability-2026-06-10.md b/docs/solutions/best-practices/macos-gesture-headless-capability-2026-06-10.md index a8399d94..c816ff13 100644 --- a/docs/solutions/best-practices/macos-gesture-headless-capability-2026-06-10.md +++ b/docs/solutions/best-practices/macos-gesture-headless-capability-2026-06-10.md @@ -44,7 +44,7 @@ The answer is **per-gesture and per-platform**, because a gesture is headless-ca 2. **A new platform that exposes a headless path lights it up automatically — adapter-only change.** If a future Windows (UIA) or Linux (AT-SPI) adapter has a headless action for `double-click`/`triple-click`, it maps the `Action` there and the command succeeds headlessly on that platform with **zero change to the command or core**. The `InteractionPolicy` flows through the request; each adapter honors it per its own capabilities. The agent just sees success (or `POLICY_DENIED` → retry `--headed`) — it never needs to know the platform. -3. **`hover`/`drag`/`mouse-*` are modeled as raw cursor gestures, not semantic `Action`s** (they call `adapter.mouse_event`/`adapter.drag` with coordinates). They stay physical on every platform by design, because hovering/dragging *are* cursor operations universally. A semantic drag (AX reorder) would be a *new* `Action`, not a change to `drag`. +3. **`hover`/`drag`/`mouse-*` are modeled as raw cursor gestures, not semantic `Action`s** (they call `adapter.mouse_event`/`adapter.drag` with coordinates). They stay physical on every platform by design, because hovering/dragging *are* cursor operations universally. A semantic drag (AX reorder) would be a *new* `Action`, not a change to `drag`. When a gesture is ref-addressed, the target app is raised frontmost first **only under `--headed`** (`focus_for_physical_input`, gated on `InteractionPolicy::allow_focus_steal`; the response reports `"focused": true`). Headless gestures never change the frontmost app, and `--xy` input never focuses — the caller owns the target there. 4. **`POLICY_DENIED` on a headless gesture is correct, not a bug** — it is the fail-closed signal that the headless AX path is unavailable and the caller must opt into `--headed`. Never widen the default policy to make it disappear. diff --git a/skills/agent-desktop/references/commands-interaction.md b/skills/agent-desktop/references/commands-interaction.md index 8b086509..067310a8 100644 --- a/skills/agent-desktop/references/commands-interaction.md +++ b/skills/agent-desktop/references/commands-interaction.md @@ -197,6 +197,8 @@ agent-desktop hover @e5 --duration 2000 Moves cursor to element center or absolute coordinates. Optional `--duration` holds position for N ms. This is an explicit cursor-moving command. +With `--headed`, a ref-addressed hover raises the target app frontmost before moving the cursor, and the response includes `"focused": true` when the raise happened. Headless (default) never changes the frontmost app. `--xy` input never focuses — the caller owns the target there. + ### drag ```bash agent-desktop drag --from @e1 --to @e5 @@ -216,6 +218,8 @@ agent-desktop drag --from @e1 --to @e5 --drop-delay 800 Can mix ref and coordinate sources (e.g., `--from @e1 --to-xy 400,500`). +With `--headed`, a ref-addressed `--from` raises the source app frontmost before the mouse-down (the destination app is never pre-focused — raising it could cover the source point), and the response includes `"focused": true` when the raise happened. Headless (default) never changes the frontmost app; coordinate-only drags never focus. + macOS drop targets often need the dragged item to dwell over them before they register as the drop destination — too short and the gesture lands as a drag with no drop. The default 500ms dwell suits most targets; raise `--drop-delay` (e.g. 800–1200) for sluggish destinations like list reorders or cross-window drops. The dwell posts continuous drag events over the destination so it stays highlighted, rather than a dead pause. ### mouse-move