diff --git a/crates/core/src/action.rs b/crates/core/src/action.rs index 9ab2d4a..5faa923 100644 --- a/crates/core/src/action.rs +++ b/crates/core/src/action.rs @@ -28,6 +28,32 @@ pub enum Action { } impl Action { + pub fn headed_requirement(&self) -> crate::HeadedRequirement { + match self { + Self::Click + | Self::DoubleClick + | Self::RightClick + | Self::TripleClick + | Self::Scroll(_, _) + | Self::Hover + | Self::Drag(_) => crate::HeadedRequirement::FocusedWindowAndCursor, + Self::SetValue(_) + | Self::SetFocus + | Self::Expand + | Self::Collapse + | Self::Select(_) + | Self::Toggle + | Self::Check + | Self::Uncheck + | Self::ScrollTo + | Self::PressKey(_) + | Self::KeyDown(_) + | Self::KeyUp(_) + | Self::TypeText(_) + | Self::Clear => crate::HeadedRequirement::FocusedWindow, + } + } + pub fn name(&self) -> &'static str { match self { Self::Click => "click", diff --git a/crates/core/src/action_request.rs b/crates/core/src/action_request.rs index fb7a944..f476f1f 100644 --- a/crates/core/src/action_request.rs +++ b/crates/core/src/action_request.rs @@ -16,6 +16,14 @@ pub struct ActionRequest { } impl ActionRequest { + pub fn headed_requirement(&self) -> crate::HeadedRequirement { + if self.policy.is_headed() { + self.action.headed_requirement() + } else { + crate::HeadedRequirement::None + } + } + pub fn headless(action: Action) -> Self { Self { action, @@ -88,6 +96,22 @@ mod tests { assert_eq!(request.policy, InteractionPolicy::headless()); } + #[test] + fn headed_requirement_is_inert_until_the_caller_selects_headed_mode() { + assert_eq!( + ActionRequest::headless(Action::Click).headed_requirement(), + crate::HeadedRequirement::None + ); + assert_eq!( + ActionRequest::headed(Action::Click).headed_requirement(), + crate::HeadedRequirement::FocusedWindowAndCursor + ); + assert_eq!( + ActionRequest::headed(Action::TypeText("text".into())).headed_requirement(), + crate::HeadedRequirement::FocusedWindow + ); + } + #[test] fn focus_fallback_policy_never_moves_cursor() { let request = ActionRequest::focus_fallback(Action::Scroll(Direction::Down, 1)); diff --git a/crates/core/src/action_tests.rs b/crates/core/src/action_tests.rs index 8194060..bbc6dc6 100644 --- a/crates/core/src/action_tests.rs +++ b/crates/core/src/action_tests.rs @@ -40,6 +40,47 @@ fn action_names_do_not_include_payloads() { } } +#[test] +fn headed_requirements_distinguish_window_and_pointer_preconditions() { + for action in [ + Action::Click, + Action::DoubleClick, + Action::RightClick, + Action::TripleClick, + Action::Scroll(Direction::Down, 1), + Action::Hover, + Action::Drag(dummy_drag()), + ] { + assert_eq!( + action.headed_requirement(), + crate::HeadedRequirement::FocusedWindowAndCursor, + "{} must require focus plus cursor delivery", + action.name() + ); + } + for action in [ + Action::SetValue("v".into()), + Action::SetFocus, + Action::Expand, + Action::Collapse, + Action::Select("v".into()), + Action::Toggle, + Action::Check, + Action::Uncheck, + Action::ScrollTo, + Action::PressKey(dummy_key()), + Action::TypeText("v".into()), + Action::Clear, + ] { + assert_eq!( + action.headed_requirement(), + crate::HeadedRequirement::FocusedWindow, + "{} must require focus without pointer movement", + action.name() + ); + } +} + #[test] fn pure_ax_actions_base_policy_is_headless() { let headless = InteractionPolicy::headless(); diff --git a/crates/core/src/actionability/requirements.rs b/crates/core/src/actionability/requirements.rs index d6d1a69..7eda381 100644 --- a/crates/core/src/actionability/requirements.rs +++ b/crates/core/src/actionability/requirements.rs @@ -49,7 +49,7 @@ impl ActionabilityRequirements { if !self.receives_events { return PointerDelivery::NotApplicable; } - if policy.is_headed() { + if policy.is_headed() && action.headed_requirement().requires_cursor() { PointerDelivery::Physical } else if crate::capability::supports_direct_semantic_pointer_delivery( action, diff --git a/crates/core/src/adapter/mod.rs b/crates/core/src/adapter/mod.rs index 37b708d..6012387 100644 --- a/crates/core/src/adapter/mod.rs +++ b/crates/core/src/adapter/mod.rs @@ -12,7 +12,8 @@ pub(crate) use observation::optional_live_read; pub use system::SystemOps; #[cfg(test)] pub(crate) use test_support::{ - complete_live_observation, guarded_interaction_lease, live_identity, observed_tree, + complete_live_observation, exact_window_focus, guarded_interaction_lease, live_identity, + observed_tree, }; pub use crate::live_element::LiveElement; diff --git a/crates/core/src/adapter/system.rs b/crates/core/src/adapter/system.rs index 36feb2a..c6fc47c 100644 --- a/crates/core/src/adapter/system.rs +++ b/crates/core/src/adapter/system.rs @@ -48,6 +48,9 @@ pub trait SystemOps: Send + Sync { self.permission_report(lease.deadline()) } + /// Performs the platform-native focus/raise operation for the exact window. + /// Core decides when focus is required; implementations must return only + /// after that window is confirmed focused, or return an error. fn focus_window( &self, _win: &WindowInfo, diff --git a/crates/core/src/adapter/test_support.rs b/crates/core/src/adapter/test_support.rs index b8715fe..1b53ce5 100644 --- a/crates/core/src/adapter/test_support.rs +++ b/crates/core/src/adapter/test_support.rs @@ -92,6 +92,28 @@ macro_rules! guarded_interaction_lease { pub(crate) use guarded_interaction_lease; +macro_rules! exact_window_focus { + () => { + fn resolve_window_strict( + &self, + window: &$crate::WindowInfo, + _deadline: $crate::Deadline, + ) -> Result<$crate::WindowInfo, $crate::AdapterError> { + Ok(window.clone()) + } + + fn focus_window( + &self, + _window: &$crate::WindowInfo, + _lease: &$crate::InteractionLease, + ) -> Result<(), $crate::AdapterError> { + Ok(()) + } + }; +} + +pub(crate) use exact_window_focus; + pub(crate) fn live_identity(name: &str) -> crate::LiveIdentity { crate::LiveIdentity { name: crate::LocatorField::Known(name.into()), diff --git a/crates/core/src/commands/drag.rs b/crates/core/src/commands/drag.rs index 7d23c3f..a1ef25c 100644 --- a/crates/core/src/commands/drag.rs +++ b/crates/core/src/commands/drag.rs @@ -35,19 +35,19 @@ pub fn execute( xy: args.from_xy, snapshot_id: args.snapshot_id.as_deref(), missing_input_message: "Provide --from or --from-xy x,y", - focus_before_resolve: true, + headed_requirement: crate::HeadedRequirement::FocusedWindowAndCursor, }; let to_args = PointResolveArgs { ref_id: args.to_ref.as_deref(), xy: args.to_xy, snapshot_id: args.snapshot_id.as_deref(), missing_input_message: "Provide --to or --to-xy x,y", - focus_before_resolve: false, + headed_requirement: crate::HeadedRequirement::None, }; let initial_from = resolve_point_with_deadline(from_args, deadline, &lease, adapter, context)?; let from = resolve_point_with_deadline( PointResolveArgs { - focus_before_resolve: false, + headed_requirement: crate::HeadedRequirement::None, ..from_args }, deadline, diff --git a/crates/core/src/commands/execute_by_ref_tests.rs b/crates/core/src/commands/execute_by_ref_tests.rs index a8e781a..45773da 100644 --- a/crates/core/src/commands/execute_by_ref_tests.rs +++ b/crates/core/src/commands/execute_by_ref_tests.rs @@ -41,9 +41,9 @@ fn snapshot_with_ref(role: &str, available_actions: &[&str]) -> String { available_actions: available_actions.iter().map(|a| (*a).to_string()).collect(), }, source: crate::RefSource { - source_app: None, - source_window_id: None, - source_window_title: None, + source_app: Some("Fixture".into()), + source_window_id: Some("w-1".into()), + source_window_title: Some("Fixture".into()), source_window_bounds_hash: None, source_surface: crate::adapter::SnapshotSurface::Window, }, @@ -159,6 +159,7 @@ impl ActionOps for PolicyCaptureAdapter { impl InputOps for PolicyCaptureAdapter {} impl SystemOps for PolicyCaptureAdapter { crate::adapter::guarded_interaction_lease!(); + crate::adapter::exact_window_focus!(); } #[test] diff --git a/crates/core/src/commands/focus_window.rs b/crates/core/src/commands/focus_window.rs index 764ed64..471a0d5 100644 --- a/crates/core/src/commands/focus_window.rs +++ b/crates/core/src/commands/focus_window.rs @@ -1,14 +1,8 @@ use crate::{ - AdapterError, AppError, ErrorCode, WindowInfo, + AppError, adapter::{PlatformAdapter, WindowFilter}, }; use serde_json::{Value, json}; -use std::time::Duration; - -const FOCUS_SETTLE_TIMEOUT_MS: u64 = 750; -const FOCUS_POLL_INTERVAL_MS: u64 = 50; -const FOCUS_CONFIRMATIONS: u8 = 2; - pub struct FocusWindowArgs { pub window_id: Option, pub app: Option, @@ -48,89 +42,11 @@ pub fn execute(args: FocusWindowArgs, adapter: &dyn PlatformAdapter) -> Result, - deadline: crate::Deadline, -) -> Result { - wait_for_focused_window_with_poll_interval( - adapter, - window_id, - app.as_deref(), - Duration::from_millis(FOCUS_POLL_INTERVAL_MS), - deadline, - ) -} - -fn wait_for_focused_window_with_poll_interval( - adapter: &dyn PlatformAdapter, - window_id: &str, - app: Option<&str>, - poll_interval: Duration, - parent_deadline: crate::Deadline, -) -> Result { - let deadline = parent_deadline.capped(Duration::from_millis(FOCUS_SETTLE_TIMEOUT_MS)); - let mut confirmations = 0; - loop { - match observed_focused_window(adapter, app, deadline) { - Ok(Some(window)) if window.id == window_id => { - confirmations += 1; - if confirmations >= FOCUS_CONFIRMATIONS { - return Ok(window); - } - } - Ok(_) => { - confirmations = 0; - } - Err(AppError::Adapter(error)) if error.permits_retry_by_default() => { - confirmations = 0; - } - Err(error) => return Err(error), - } - - if deadline.is_expired() { - return Err(AppError::Adapter( - AdapterError::new( - ErrorCode::ActionFailed, - "Window focus did not settle on the requested window", - ) - .with_suggestion("Run 'list-windows' to refresh window IDs, then retry."), - )); - } - - if !poll_interval.is_zero() { - std::thread::sleep(poll_interval); - } - } -} - -fn observed_focused_window( - adapter: &dyn PlatformAdapter, - app: Option<&str>, - deadline: crate::Deadline, -) -> Result, AppError> { - match adapter.focused_window(deadline) { - Ok(window) => Ok(window), - Err(err) if err.code == ErrorCode::PlatformNotSupported => adapter - .list_windows( - &WindowFilter { - focused_only: true, - app: app.map(str::to_string), - }, - deadline, - ) - .map(|windows| windows.into_iter().next()) - .map_err(AppError::Adapter), - Err(err) => Err(AppError::Adapter(err)), - } -} - #[cfg(test)] #[path = "focus_window_tests.rs"] mod tests; diff --git a/crates/core/src/commands/focus_window_tests.rs b/crates/core/src/commands/focus_window_tests.rs index 253a1de..1118c7a 100644 --- a/crates/core/src/commands/focus_window_tests.rs +++ b/crates/core/src/commands/focus_window_tests.rs @@ -1,6 +1,8 @@ use super::*; use crate::adapter::{ActionOps, InputOps, ObservationOps, SystemOps}; +use crate::{AdapterError, WindowInfo}; use std::sync::Mutex; +use std::time::Duration; struct FocusAdapter { windows: Vec, @@ -157,7 +159,7 @@ fn focus_confirmation_resets_after_transient_wrong_window() { focused_window_supported: true, }; - let value = wait_for_focused_window_with_poll_interval( + let value = crate::window_focus::wait_for_focused_window_with_poll_interval( &adapter, &target.id, None, diff --git a/crates/core/src/commands/hover.rs b/crates/core/src/commands/hover.rs index 678e56d..e47e7f7 100644 --- a/crates/core/src/commands/hover.rs +++ b/crates/core/src/commands/hover.rs @@ -39,7 +39,7 @@ pub fn execute( xy: args.xy, snapshot_id: args.snapshot_id.as_deref(), missing_input_message: "Provide a ref (@e1) or --xy x,y", - focus_before_resolve: true, + headed_requirement: crate::HeadedRequirement::FocusedWindowAndCursor, }, deadline, &lease, diff --git a/crates/core/src/commands/point_resolve.rs b/crates/core/src/commands/point_resolve.rs index 175d32d..8b07e6a 100644 --- a/crates/core/src/commands/point_resolve.rs +++ b/crates/core/src/commands/point_resolve.rs @@ -2,7 +2,6 @@ use crate::{ AdapterError, AppError, Point, actionability, adapter::PlatformAdapter, commands::helpers::resolve_ref_with_context, context::CommandContext, }; -use serde_json::json; #[derive(Clone, Copy)] pub(crate) struct PointResolveArgs<'a> { @@ -10,7 +9,7 @@ pub(crate) struct PointResolveArgs<'a> { pub xy: Option<(f64, f64)>, pub snapshot_id: Option<&'a str>, pub missing_input_message: &'a str, - pub focus_before_resolve: bool, + pub headed_requirement: crate::HeadedRequirement, } pub(crate) struct ResolvedPoint { @@ -77,71 +76,8 @@ pub(crate) fn focus_for_physical_input( if !context.physical_input_policy().allow_focus_steal { return Ok(false); } - let process_instance = entry - .process - .process_instance - .clone() - .filter(|instance| !instance.is_empty()); - let Some(process_instance) = process_instance else { - return Ok(false); - }; - let window_id = entry - .source - .source_window_id - .clone() - .filter(|id| !id.is_empty()); - let Some(window_id) = window_id else { - return Ok(false); - }; - let expected = crate::WindowInfo { - id: window_id, - title: entry.source.source_window_title.clone().unwrap_or_default(), - app: entry.source.source_app.clone().unwrap_or_default(), - pid: entry.process.pid, - process_instance: Some(process_instance.clone()), - bounds: None, - state: crate::WindowState { - is_focused: false, - ..Default::default() - }, - }; - let live = match adapter.resolve_window_strict(&expected, lease.deadline()) { - Ok(live) => live, - Err(error) => return focus_failure(error), - }; - if live.id != expected.id - || live.pid != expected.pid - || live.process_instance.as_deref() != Some(process_instance.as_str()) - { - return Err(AdapterError::stale_ref( - "target source window belongs to a different process instance", - ) - .into()); - } - if let Err(error) = adapter.focus_window(&live, lease) { - return focus_failure(error); - } - let focused = true; - context.trace_lazy( - "input.focus_window", - || json!({ "pid": live.pid, "window_id": live.id, "ok": focused }), - )?; - Ok(focused) -} - -fn focus_failure(error: AdapterError) -> Result { - if matches!( - error.code, - crate::ErrorCode::PermDenied - | crate::ErrorCode::PolicyDenied - | crate::ErrorCode::StaleRef - | crate::ErrorCode::AmbiguousTarget - | crate::ErrorCode::InvalidArgs - | crate::ErrorCode::Internal - ) { - return Err(error.into()); - } - Ok(false) + crate::headed_focus::focus_entry_window(entry, adapter, context, lease)?; + Ok(true) } #[cfg(test)] diff --git a/crates/core/src/commands/point_resolve_tests.rs b/crates/core/src/commands/point_resolve_tests.rs index 0641a86..21fb699 100644 --- a/crates/core/src/commands/point_resolve_tests.rs +++ b/crates/core/src/commands/point_resolve_tests.rs @@ -121,7 +121,7 @@ fn ref_args(snapshot_id: &str) -> PointResolveArgs<'_> { xy: None, snapshot_id: Some(snapshot_id), missing_input_message: "Provide a ref (@e1) or --xy x,y", - focus_before_resolve: false, + headed_requirement: crate::HeadedRequirement::None, } } @@ -221,7 +221,7 @@ fn raw_xy_input_never_calls_hit_test() { xy: Some((5.0, 6.0)), snapshot_id: None, missing_input_message: "Provide a ref (@e1) or --xy x,y", - focus_before_resolve: false, + headed_requirement: crate::HeadedRequirement::None, }, &adapter, &CommandContext::default(), @@ -307,27 +307,27 @@ fn focus_test_lease() -> crate::InteractionLease { } #[test] -fn transient_window_resolution_failure_degrades_to_unfocused_input() { +fn transient_window_resolution_failure_blocks_headed_input() { let adapter = FocusFailureAdapter { resolve_error: Some(AdapterError::app_unresponsive("Fixture")), focus_error: None, focus_calls: AtomicU32::new(0), }; - let focused = focus_for_physical_input( + let error = focus_for_physical_input( Some(&focus_ref_entry()), &adapter, &CommandContext::default().with_headed(true), &focus_test_lease(), ) - .unwrap(); + .unwrap_err(); - assert!(!focused); + assert_eq!(error.code(), "APP_UNRESPONSIVE"); assert_eq!(adapter.focus_calls.load(Ordering::SeqCst), 0); } #[test] -fn transient_focus_failure_degrades_to_unfocused_input() { +fn transient_focus_failure_blocks_headed_input() { let adapter = FocusFailureAdapter { resolve_error: None, focus_error: Some(AdapterError::new( @@ -337,15 +337,15 @@ fn transient_focus_failure_degrades_to_unfocused_input() { focus_calls: AtomicU32::new(0), }; - let focused = focus_for_physical_input( + let error = focus_for_physical_input( Some(&focus_ref_entry()), &adapter, &CommandContext::default().with_headed(true), &focus_test_lease(), ) - .unwrap(); + .unwrap_err(); - assert!(!focused); + assert_eq!(error.code(), "ACTION_FAILED"); assert_eq!(adapter.focus_calls.load(Ordering::SeqCst), 1); } diff --git a/crates/core/src/commands/pointer_action.rs b/crates/core/src/commands/pointer_action.rs index 7e5296a..ee18f71 100644 --- a/crates/core/src/commands/pointer_action.rs +++ b/crates/core/src/commands/pointer_action.rs @@ -131,7 +131,7 @@ pub(crate) fn resolve_point_with_deadline<'a>( return resolve_point_from_ref_or_xy_with_context(args, adapter, context, deadline, lease); }; let entry = load_ref_entry(ref_id, args.snapshot_id, context)?; - let focused = if args.focus_before_resolve { + let focused = if args.headed_requirement.requires_focus() { crate::commands::point_resolve::focus_for_physical_input( Some(&entry), adapter, diff --git a/crates/core/src/commands/pointer_action_tests.rs b/crates/core/src/commands/pointer_action_tests.rs index da5b3c9..8efbff2 100644 --- a/crates/core/src/commands/pointer_action_tests.rs +++ b/crates/core/src/commands/pointer_action_tests.rs @@ -85,7 +85,7 @@ fn point_args(snapshot_id: &str) -> crate::commands::point_resolve::PointResolve xy: None, snapshot_id: Some(snapshot_id), missing_input_message: "target required", - focus_before_resolve: false, + headed_requirement: crate::HeadedRequirement::None, } } diff --git a/crates/core/src/commands/press.rs b/crates/core/src/commands/press.rs index 9189d57..258d3f8 100644 --- a/crates/core/src/commands/press.rs +++ b/crates/core/src/commands/press.rs @@ -30,12 +30,12 @@ pub fn execute( &expected, lease.deadline(), )?; - let result = adapter.press_key_for_app( - crate::commands::helpers::process_identity(&live)?, - &combo, - context.physical_input_policy(), - &lease, - )?; + let process = crate::commands::helpers::process_identity(&live)?; + if context.physical_input_policy().is_headed() { + crate::headed_focus::focus_process_window(process.clone(), adapter, context, &lease)?; + } + let result = + adapter.press_key_for_app(process, &combo, context.physical_input_policy(), &lease)?; return Ok(serde_json::to_value(result)?); } diff --git a/crates/core/src/commands/press_tests.rs b/crates/core/src/commands/press_tests.rs index 077534c..13ef95d 100644 --- a/crates/core/src/commands/press_tests.rs +++ b/crates/core/src/commands/press_tests.rs @@ -68,6 +68,25 @@ impl ObservationOps for CapturingAdapter { process_instance: Some("generation-1".into()), }]) } + + fn list_windows( + &self, + _filter: &crate::WindowFilter, + _deadline: crate::Deadline, + ) -> Result, AdapterError> { + Ok(vec![crate::WindowInfo { + id: "w-42".into(), + title: "Editor".into(), + app: "Editor".into(), + pid: crate::ProcessId::new(42), + process_instance: Some("generation-1".into()), + bounds: None, + state: crate::WindowState { + visible: Some(true), + ..Default::default() + }, + }]) + } } impl ActionOps for CapturingAdapter { @@ -86,6 +105,7 @@ impl InputOps for CapturingAdapter {} impl SystemOps for CapturingAdapter { crate::adapter::guarded_interaction_lease!(); + crate::adapter::exact_window_focus!(); fn press_key_for_app( &self, diff --git a/crates/core/src/commands/ref_policy_tests.rs b/crates/core/src/commands/ref_policy_tests.rs index 98e503d..f4c58c7 100644 --- a/crates/core/src/commands/ref_policy_tests.rs +++ b/crates/core/src/commands/ref_policy_tests.rs @@ -99,6 +99,7 @@ impl InputOps for RecordingAdapter {} impl SystemOps for RecordingAdapter { crate::adapter::guarded_interaction_lease!(); + crate::adapter::exact_window_focus!(); } fn snapshot_id() -> String { @@ -147,9 +148,9 @@ fn snapshot_id() -> String { ], }, source: crate::RefSource { - source_app: None, - source_window_id: None, - source_window_title: None, + source_app: Some("Fixture".into()), + source_window_id: Some("w-1".into()), + source_window_title: Some("Fixture".into()), source_window_bounds_hash: None, source_surface: crate::adapter::SnapshotSurface::Window, }, diff --git a/crates/core/src/headed_focus.rs b/crates/core/src/headed_focus.rs new file mode 100644 index 0000000..e932b41 --- /dev/null +++ b/crates/core/src/headed_focus.rs @@ -0,0 +1,72 @@ +use serde_json::json; + +use crate::{AdapterError, AppError, ErrorCode, PlatformAdapter, ProcessIdentity, WindowInfo}; + +pub(crate) fn focus_entry_window( + entry: &crate::RefEntry, + adapter: &dyn PlatformAdapter, + context: &crate::CommandContext, + lease: &crate::InteractionLease, +) -> Result { + let process_instance = required_text( + entry.process.process_instance.as_deref(), + "Headed input requires target process-instance identity", + )?; + let window_id = required_text( + entry.source.source_window_id.as_deref(), + "Headed input requires an exact source window id", + )?; + let expected = WindowInfo { + id: window_id.to_string(), + title: entry.source.source_window_title.clone().unwrap_or_default(), + app: entry.source.source_app.clone().unwrap_or_default(), + pid: entry.process.pid, + process_instance: Some(process_instance.to_string()), + bounds: None, + state: crate::WindowState::default(), + }; + focus_exact_window(&expected, adapter, context, lease) +} + +pub(crate) fn focus_process_window( + process: ProcessIdentity, + adapter: &dyn PlatformAdapter, + context: &crate::CommandContext, + lease: &crate::InteractionLease, +) -> Result { + let expected = + crate::window_lookup::find_window_for_process(process, adapter, lease.deadline())?; + focus_exact_window(&expected, adapter, context, lease) +} + +fn focus_exact_window( + expected: &WindowInfo, + adapter: &dyn PlatformAdapter, + context: &crate::CommandContext, + lease: &crate::InteractionLease, +) -> Result { + let live = adapter.resolve_window_strict(expected, lease.deadline())?; + if live.id != expected.id + || live.pid != expected.pid + || live.process_instance != expected.process_instance + { + return Err(AdapterError::stale_ref( + "Headed target window belongs to a different process instance", + ) + .into()); + } + adapter.focus_window(&live, lease)?; + context.trace_lazy( + "input.focus_window", + || json!({ "pid": live.pid, "window_id": live.id, "ok": true }), + )?; + Ok(live) +} + +fn required_text<'a>(value: Option<&'a str>, message: &str) -> Result<&'a str, AppError> { + value.filter(|value| !value.is_empty()).ok_or_else(|| { + AdapterError::new(ErrorCode::ActionNotSupported, message) + .with_details(json!({ "physical_delivery_started": false })) + .into() + }) +} diff --git a/crates/core/src/headed_requirement.rs b/crates/core/src/headed_requirement.rs new file mode 100644 index 0000000..da1e42c --- /dev/null +++ b/crates/core/src/headed_requirement.rs @@ -0,0 +1,19 @@ +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum HeadedRequirement { + None, + FocusedWindow, + FocusedWindowAndCursor, +} + +impl HeadedRequirement { + pub fn requires_focus(self) -> bool { + !matches!(self, Self::None) + } + + pub fn requires_cursor(self) -> bool { + matches!(self, Self::FocusedWindowAndCursor) + } +} diff --git a/crates/core/src/lib.rs b/crates/core/src/lib.rs index a66bd15..e635821 100644 --- a/crates/core/src/lib.rs +++ b/crates/core/src/lib.rs @@ -32,6 +32,8 @@ pub mod element_state; mod error_code; mod event_kind; mod file_lock; +mod headed_focus; +mod headed_requirement; pub mod hints; pub mod hit_test; mod identifier_kind; @@ -138,6 +140,7 @@ pub mod tree_options; mod ui_event; mod wait_budget; pub mod window_filter; +mod window_focus; mod window_info; mod window_lookup; mod window_op; @@ -170,6 +173,7 @@ pub use element_identifier::ElementIdentifier; pub use element_state::ElementState; pub use error_code::ErrorCode; pub use event_kind::EventKind; +pub use headed_requirement::HeadedRequirement; pub use hit_test::HitTestResult; pub use identifier_kind::IdentifierKind; pub use identity_predicate::IdentityPredicate; diff --git a/crates/core/src/ref_action_single.rs b/crates/core/src/ref_action_single.rs index 7b2135d..60d9782 100644 --- a/crates/core/src/ref_action_single.rs +++ b/crates/core/src/ref_action_single.rs @@ -14,5 +14,14 @@ pub(crate) fn execute_single_shot( &crate::InteractionLease, ) -> Result, ) -> Result { + if request.headed_requirement().requires_focus() { + crate::headed_focus::focus_entry_window( + context.entry, + context.adapter, + context.context, + lease, + ) + .map_err(crate::ref_action::into_adapter_error)?; + } dispatch(RefActionContext::new(context, deadline), request, lease).map_err(into_adapter_error) } diff --git a/crates/core/src/ref_action_tests.rs b/crates/core/src/ref_action_tests.rs index d4546e4..42b9459 100644 --- a/crates/core/src/ref_action_tests.rs +++ b/crates/core/src/ref_action_tests.rs @@ -64,6 +64,7 @@ impl InputOps for SuccessfulAdapter {} impl SystemOps for SuccessfulAdapter { crate::adapter::guarded_interaction_lease!(); + crate::adapter::exact_window_focus!(); } struct FailingAdapter { @@ -208,9 +209,9 @@ fn entry() -> RefEntry { available_actions: vec![capability::CLICK.into()], }, source: crate::RefSource { - source_app: None, - source_window_id: None, - source_window_title: None, + source_app: Some("Fixture".into()), + source_window_id: Some("w-42".into()), + source_window_title: Some("Fixture".into()), source_window_bounds_hash: None, source_surface: SnapshotSurface::Window, }, diff --git a/crates/core/src/ref_action_wait_tests.rs b/crates/core/src/ref_action_wait_tests.rs index aa4e1b8..7ce9c60 100644 --- a/crates/core/src/ref_action_wait_tests.rs +++ b/crates/core/src/ref_action_wait_tests.rs @@ -148,9 +148,9 @@ fn entry() -> RefEntry { available_actions: vec![capability::CLICK.into()], }, source: crate::RefSource { - source_app: None, - source_window_id: None, - source_window_title: None, + source_app: Some("Fixture".into()), + source_window_id: Some("w-1".into()), + source_window_title: Some("Fixture".into()), source_window_bounds_hash: None, source_surface: crate::snapshot_surface::SnapshotSurface::Window, }, @@ -331,6 +331,8 @@ impl ActionOps for DoubleCheckAdapter { impl InputOps for DoubleCheckAdapter {} impl SystemOps for DoubleCheckAdapter { + crate::adapter::exact_window_focus!(); + fn acquire_interaction_lease( &self, deadline: crate::Deadline, diff --git a/crates/core/src/window_focus.rs b/crates/core/src/window_focus.rs new file mode 100644 index 0000000..8df5b89 --- /dev/null +++ b/crates/core/src/window_focus.rs @@ -0,0 +1,80 @@ +use std::time::Duration; + +use crate::{AdapterError, AppError, ErrorCode, PlatformAdapter, WindowFilter, WindowInfo}; + +const FOCUS_SETTLE_TIMEOUT_MS: u64 = 750; +const FOCUS_POLL_INTERVAL_MS: u64 = 50; +const FOCUS_CONFIRMATIONS: u8 = 2; + +pub(crate) fn focus_and_confirm( + adapter: &dyn PlatformAdapter, + window: &WindowInfo, + lease: &crate::InteractionLease, +) -> Result { + adapter.focus_window(window, lease)?; + wait_for_focused_window_with_poll_interval( + adapter, + &window.id, + Some(&window.app), + Duration::from_millis(FOCUS_POLL_INTERVAL_MS), + lease.deadline(), + ) +} + +pub(crate) fn wait_for_focused_window_with_poll_interval( + adapter: &dyn PlatformAdapter, + window_id: &str, + app: Option<&str>, + poll_interval: Duration, + parent_deadline: crate::Deadline, +) -> Result { + let deadline = parent_deadline.capped(Duration::from_millis(FOCUS_SETTLE_TIMEOUT_MS)); + let mut confirmations = 0; + loop { + match observed_focused_window(adapter, app, deadline) { + Ok(Some(window)) if window.id == window_id => { + confirmations += 1; + if confirmations >= FOCUS_CONFIRMATIONS { + return Ok(window); + } + } + Ok(_) => confirmations = 0, + Err(AppError::Adapter(error)) if error.permits_retry_by_default() => { + confirmations = 0; + } + Err(error) => return Err(error), + } + if deadline.is_expired() { + return Err(AdapterError::new( + ErrorCode::ActionFailed, + "Window focus did not settle on the requested window", + ) + .with_suggestion("Run 'list-windows' to refresh window IDs, then retry.") + .into()); + } + if !poll_interval.is_zero() { + std::thread::sleep(poll_interval.min(deadline.remaining())); + } + } +} + +fn observed_focused_window( + adapter: &dyn PlatformAdapter, + app: Option<&str>, + deadline: crate::Deadline, +) -> Result, AppError> { + match adapter.focused_window(deadline) { + Ok(window) => Ok(window), + Err(err) if err.code == ErrorCode::PlatformNotSupported => adapter + .list_windows( + &WindowFilter { + focused_only: true, + app: app.map(str::to_string), + }, + deadline, + ) + .map(|windows| windows.into_iter().next()) + .map_err(AppError::Adapter), + Err(err) => Err(AppError::Adapter(err)), + } +} diff --git a/crates/macos/src/actions/chain.rs b/crates/macos/src/actions/chain.rs index 2fa227a..5ba2dee 100644 --- a/crates/macos/src/actions/chain.rs +++ b/crates/macos/src/actions/chain.rs @@ -59,9 +59,9 @@ mod imp { pub(crate) fn step_mechanism(step: &ChainStep) -> StepMechanism { match step { - ChainStep::CGClick { .. } - | ChainStep::CGDisclosureClick { .. } - | ChainStep::FocusThenClearByKeyboard => StepMechanism::PhysicalSynthetic, + ChainStep::CGClick { .. } | ChainStep::FocusThenClearByKeyboard => { + StepMechanism::PhysicalSynthetic + } _ => StepMechanism::SemanticApi, } } @@ -103,16 +103,13 @@ mod imp { ChainStep::FocusThenClearByKeyboard => "FocusThenClearByKeyboard", ChainStep::CustomWithDeadline { label, .. } => label, ChainStep::CGClick { .. } => "CGClick", - ChainStep::CGDisclosureClick { .. } => "CGDisclosureClick", } } pub(crate) fn step_allowed(step: &ChainStep, policy: InteractionPolicy) -> bool { !matches!( step, - ChainStep::CGClick { .. } - | ChainStep::CGDisclosureClick { .. } - | ChainStep::FocusThenClearByKeyboard + ChainStep::CGClick { .. } | ChainStep::FocusThenClearByKeyboard ) || policy.is_headed() } } diff --git a/crates/macos/src/actions/chain_defs.rs b/crates/macos/src/actions/chain_defs.rs index 5813115..d9396ef 100644 --- a/crates/macos/src/actions/chain_defs.rs +++ b/crates/macos/src/actions/chain_defs.rs @@ -53,25 +53,19 @@ mod imp { }; pub(crate) static EXPAND_CHAIN: ChainDef = ChainDef { - steps: &[ - ChainStep::CGDisclosureClick { expanded: true }, - ChainStep::CustomWithDeadline { - label: "expand_verified", - func: chain_disclosure_steps::press_to_expand, - }, - ], + steps: &[ChainStep::CustomWithDeadline { + label: "expand_verified", + func: chain_disclosure_steps::press_to_expand, + }], suggestion: "Target a control with a readable expandable state.", continue_after_unverified_delivery: false, }; pub(crate) static COLLAPSE_CHAIN: ChainDef = ChainDef { - steps: &[ - ChainStep::CGDisclosureClick { expanded: false }, - ChainStep::CustomWithDeadline { - label: "collapse_verified", - func: chain_disclosure_steps::press_to_collapse, - }, - ], + steps: &[ChainStep::CustomWithDeadline { + label: "collapse_verified", + func: chain_disclosure_steps::press_to_collapse, + }], suggestion: "Target a control with a readable expandable state.", continue_after_unverified_delivery: false, }; @@ -163,11 +157,11 @@ mod imp { use crate::actions::chain_step::ChainStep; #[test] - fn disclosure_chains_begin_with_a_headed_physical_fallback() { + fn disclosure_chains_use_verified_semantic_delivery() { for chain in [&EXPAND_CHAIN, &COLLAPSE_CHAIN] { assert!(matches!( chain.steps.first(), - Some(ChainStep::CGDisclosureClick { .. }) + Some(ChainStep::CustomWithDeadline { .. }) )); } } diff --git a/crates/macos/src/actions/chain_disclosure_steps.rs b/crates/macos/src/actions/chain_disclosure_steps.rs index f0e9b6f..21534c2 100644 --- a/crates/macos/src/actions/chain_disclosure_steps.rs +++ b/crates/macos/src/actions/chain_disclosure_steps.rs @@ -105,50 +105,10 @@ mod imp { if last_state == Some(!expanded) { Ok(DeliveryOutcome::NotDelivered) } else { - Err(AdapterError::new( - agent_desktop_core::ErrorCode::ActionFailed, - "Disclosure state became unreadable after native delivery", - ) - .with_details(serde_json::json!({ - "verification": "expanded_state_unknown_after_delivery", - }))) + Ok(DeliveryOutcome::DeliveredUnverified) } } - pub(crate) fn physical_fallback_allowed( - element: &AXElement, - expanded: bool, - deadline: Deadline, - ) -> Result { - Ok(disclosed_state(element, deadline)? == Some(!expanded)) - } - - pub(crate) fn verify_physical_delivery( - element: &AXElement, - expanded: bool, - deadline: Deadline, - ) -> Result { - let outcome = verify_disclosure(element, expanded, deadline).map_err(after_delivery)?; - require_verified_physical_delivery(outcome) - } - - fn require_verified_physical_delivery( - outcome: DeliveryOutcome, - ) -> Result { - if outcome == DeliveryOutcome::DeliveredVerified { - return Ok(outcome); - } - Err(after_delivery( - AdapterError::new( - agent_desktop_core::ErrorCode::ActionFailed, - "Physical disclosure click did not reach the requested state", - ) - .with_details(serde_json::json!({ - "verification": "expanded_state_not_observed_after_physical_click", - })), - )) - } - fn disclosed_state( element: &AXElement, deadline: Deadline, @@ -172,10 +132,7 @@ mod imp { #[cfg(test)] mod tests { - use super::{ - DeliveryOutcome, disclosure_plan, require_verified_physical_delivery, - unobserved_delivery, - }; + use super::{DeliveryOutcome, disclosure_plan, unobserved_delivery}; #[test] fn disclosure_plan_never_blindly_toggles_unknown_state() { @@ -188,32 +145,18 @@ mod imp { } #[test] - fn unobserved_delivery_distinguishes_no_effect_from_unknown_state() { + fn unobserved_delivery_preserves_honest_unknown_state() { assert_eq!( unobserved_delivery(Some(false), true).expect("known opposite state"), DeliveryOutcome::NotDelivered ); - assert!(unobserved_delivery(None, true).is_err()); - } - - #[test] - fn physical_fallback_requires_verified_requested_state() { assert_eq!( - require_verified_physical_delivery(DeliveryOutcome::DeliveredVerified) - .expect("verified delivery"), - DeliveryOutcome::DeliveredVerified - ); - let error = require_verified_physical_delivery(DeliveryOutcome::NotDelivered) - .expect_err("unverified physical click must fail closed"); - assert_eq!( - error.disposition, - agent_desktop_core::DeliverySemantics::delivered_unverified() + unobserved_delivery(None, true).expect("unreadable state after delivery"), + DeliveryOutcome::DeliveredUnverified ); } } } #[cfg(target_os = "macos")] -pub(crate) use imp::{ - physical_fallback_allowed, press_to_collapse, press_to_expand, verify_physical_delivery, -}; +pub(crate) use imp::{press_to_collapse, press_to_expand}; diff --git a/crates/macos/src/actions/chain_step.rs b/crates/macos/src/actions/chain_step.rs index 6b0574f..abcc321 100644 --- a/crates/macos/src/actions/chain_step.rs +++ b/crates/macos/src/actions/chain_step.rs @@ -22,7 +22,4 @@ pub(crate) enum ChainStep { button: MouseButton, count: u32, }, - CGDisclosureClick { - expanded: bool, - }, } diff --git a/crates/macos/src/actions/chain_step_exec.rs b/crates/macos/src/actions/chain_step_exec.rs index ccd1be8..7a1d4a0 100644 --- a/crates/macos/src/actions/chain_step_exec.rs +++ b/crates/macos/src/actions/chain_step_exec.rs @@ -86,24 +86,6 @@ mod imp { physical_click(el, button.clone(), *count, ctx, policy)?; Ok(DeliveryOutcome::DeliveredUnverified) } - ChainStep::CGDisclosureClick { expanded } => { - if !policy.allow_focus_steal || !policy.allow_cursor_move { - return Ok(DeliveryOutcome::NotDelivered); - } - if !crate::actions::chain_disclosure_steps::physical_fallback_allowed( - el, - *expanded, - ctx.deadline, - )? { - return Ok(DeliveryOutcome::NotDelivered); - } - physical_click(el, agent_desktop_core::MouseButton::Left, 1, ctx, policy)?; - crate::actions::chain_disclosure_steps::verify_physical_delivery( - el, - *expanded, - ctx.deadline, - ) - } } } diff --git a/crates/macos/src/actions/chain_tests.rs b/crates/macos/src/actions/chain_tests.rs index 02d9ef3..0cb0ec6 100644 --- a/crates/macos/src/actions/chain_tests.rs +++ b/crates/macos/src/actions/chain_tests.rs @@ -53,10 +53,6 @@ fn step_mechanism_tags_physical_for_cgclick_and_keyboard_clear() { }), StepMechanism::PhysicalSynthetic ); - assert_eq!( - step_mechanism(&ChainStep::CGDisclosureClick { expanded: true }), - StepMechanism::PhysicalSynthetic - ); assert_eq!( step_mechanism(&ChainStep::FocusThenClearByKeyboard), StepMechanism::PhysicalSynthetic diff --git a/crates/macos/src/actions/mod.rs b/crates/macos/src/actions/mod.rs index 5bce1ca..0e7de4d 100644 --- a/crates/macos/src/actions/mod.rs +++ b/crates/macos/src/actions/mod.rs @@ -16,6 +16,7 @@ pub(crate) mod dispatch; pub(crate) mod extras; mod physical_click; mod physical_keyboard; +mod physical_target; pub(crate) mod post_state; pub(crate) mod scroll; pub(crate) mod scroll_into_view; diff --git a/crates/macos/src/actions/physical_click.rs b/crates/macos/src/actions/physical_click.rs index 489ccad..56d97e1 100644 --- a/crates/macos/src/actions/physical_click.rs +++ b/crates/macos/src/actions/physical_click.rs @@ -23,24 +23,8 @@ pub(crate) fn click_via_bounds( policy, )); } - let pid = crate::system::app_ops::pid_from_element(element, deadline).ok_or_else(|| { - AdapterError::new( - ErrorCode::StaleRef, - "Physical click target no longer has an owning application", - ) - })?; - let identity = - crate::system::process_identity::ProcessIdentity::capture(pid)?.ok_or_else(|| { - AdapterError::new( - ErrorCode::StaleRef, - "Physical click target process exited before input preparation", - ) - })?; - let window = target_window(element, deadline)?; - crate::system::focus::ensure_app_focused(pid, deadline)?; - crate::system::window_ops::raise_window(&window, deadline)?; - crate::system::focus::verify_app_focused(pid, deadline)?; - crate::system::focus::verify_window_main(&window, deadline)?; + let prepared = + crate::actions::physical_target::PreparedPhysicalTarget::prepare(element, deadline)?; let read_deadline = crate::tree::locator_deadline::from_operation(deadline)?; let bounds = crate::tree::element_bounds::read_bounds_with_deadline(element, read_deadline)? .ok_or_else(|| { @@ -62,7 +46,7 @@ pub(crate) fn click_via_bounds( y = point.y, "AX action failed, falling back to CGEvent click" ); - let mut verify_target = || verify_delivery_target(element, &window, &point, identity, deadline); + let mut verify_target = || prepared.verify_pointer(element, &point, deadline); crate::input::mouse::synthesize_mouse_after( MouseEvent { kind: MouseEventKind::Click { count: click.count }, @@ -95,70 +79,6 @@ fn delivery_point( Ok(point) } -fn verify_delivery_target( - element: &AXElement, - window: &AXElement, - point: &Point, - identity: crate::system::process_identity::ProcessIdentity, - deadline: Deadline, -) -> Result<(), AdapterError> { - crate::system::focus::verify_app_focused(identity.pid(), deadline)?; - crate::system::focus::verify_window_main(window, deadline)?; - match crate::tree::hit_test::hit_test_ax_element(element, point.clone(), deadline)? { - agent_desktop_core::hit_test::HitTestResult::ReachesTarget => {} - agent_desktop_core::hit_test::HitTestResult::InterceptedBy { role, name, .. } => { - return Err(AdapterError::new( - ErrorCode::ActionFailed, - "Physical click point is intercepted by another accessibility element", - ) - .with_details(serde_json::json!({ - "physical_delivery_started": false, - "occluder_role": role, - "occluder_name": name, - }))); - } - agent_desktop_core::hit_test::HitTestResult::Unknown => { - return Err(AdapterError::new( - ErrorCode::ActionFailed, - "Physical click target could not be proven at the final input point", - ) - .with_details(serde_json::json!({ "physical_delivery_started": false }))); - } - } - if !identity.still_matches()? { - return Err(AdapterError::new( - ErrorCode::StaleRef, - "Physical click target process instance changed at input delivery", - ) - .with_details(serde_json::json!({ "physical_delivery_started": false }))); - } - Ok(()) -} - -fn target_window(element: &AXElement, deadline: Deadline) -> Result { - crate::tree::attributes::set_messaging_timeout(element, deadline)?; - let result = crate::tree::attributes::copy_element_attr_result(element, "AXWindow", deadline); - if deadline.is_expired() { - return Err(deadline.timeout_error()); - } - match result { - Ok(Some(window)) => Ok(window), - Ok(None) => Err(AdapterError::new( - ErrorCode::ActionFailed, - "Physical click target has no verified owning window", - ) - .with_details(serde_json::json!({ "physical_delivery_started": false }))), - Err(error) => Err(AdapterError::new( - ErrorCode::ActionFailed, - "Could not verify the physical click target window", - ) - .with_details(serde_json::json!({ - "ax_error": error, - "physical_delivery_started": false, - }))), - } -} - #[cfg(test)] #[path = "physical_click_tests.rs"] mod tests; diff --git a/crates/macos/src/actions/physical_keyboard.rs b/crates/macos/src/actions/physical_keyboard.rs index 4a32608..9d97001 100644 --- a/crates/macos/src/actions/physical_keyboard.rs +++ b/crates/macos/src/actions/physical_keyboard.rs @@ -72,10 +72,10 @@ fn prepare_target( "Keyboard target process exited before physical input preparation", ) })?; - crate::system::focus::ensure_app_focused(pid, deadline)?; if let Some(window) = target_window(element, deadline)? { - crate::system::window_ops::raise_window(&window, deadline)?; + crate::system::focus::verify_window_main(&window, deadline)?; } + crate::system::focus::verify_app_focused(pid, deadline)?; prepare(element, deadline)?; if !crate::actions::ax_helpers::ax_focus_or_err(element, deadline)? { return Err(AdapterError::new( diff --git a/crates/macos/src/actions/physical_target.rs b/crates/macos/src/actions/physical_target.rs new file mode 100644 index 0000000..09abf60 --- /dev/null +++ b/crates/macos/src/actions/physical_target.rs @@ -0,0 +1,93 @@ +use agent_desktop_core::{AdapterError, Deadline, ErrorCode, Point}; + +use crate::tree::AXElement; + +pub(crate) struct PreparedPhysicalTarget { + identity: crate::system::process_identity::ProcessIdentity, + window: AXElement, +} + +impl PreparedPhysicalTarget { + pub(crate) fn prepare(element: &AXElement, deadline: Deadline) -> Result { + let pid = crate::system::app_ops::pid_from_element(element, deadline).ok_or_else(|| { + AdapterError::new( + ErrorCode::StaleRef, + "Physical input target no longer has an owning application", + ) + })?; + let identity = + crate::system::process_identity::ProcessIdentity::capture(pid)?.ok_or_else(|| { + AdapterError::new( + ErrorCode::StaleRef, + "Physical input target process exited before input preparation", + ) + })?; + let window = target_window(element, deadline)?; + crate::system::focus::verify_app_focused(pid, deadline)?; + crate::system::focus::verify_window_main(&window, deadline)?; + Ok(Self { identity, window }) + } + + pub(crate) fn verify_pointer( + &self, + element: &AXElement, + point: &Point, + deadline: Deadline, + ) -> Result<(), AdapterError> { + crate::system::focus::verify_app_focused(self.identity.pid(), deadline)?; + crate::system::focus::verify_window_main(&self.window, deadline)?; + match crate::tree::hit_test::hit_test_ax_element(element, point.clone(), deadline)? { + agent_desktop_core::hit_test::HitTestResult::ReachesTarget => {} + agent_desktop_core::hit_test::HitTestResult::InterceptedBy { role, name, .. } => { + return Err(AdapterError::new( + ErrorCode::ActionFailed, + "Physical input point is intercepted by another accessibility element", + ) + .with_details(serde_json::json!({ + "physical_delivery_started": false, + "occluder_role": role, + "occluder_name": name, + }))); + } + agent_desktop_core::hit_test::HitTestResult::Unknown => { + return Err(AdapterError::new( + ErrorCode::ActionFailed, + "Physical input target could not be proven at the final input point", + ) + .with_details(serde_json::json!({ "physical_delivery_started": false }))); + } + } + if !self.identity.still_matches()? { + return Err(AdapterError::new( + ErrorCode::StaleRef, + "Physical input target process instance changed at input delivery", + ) + .with_details(serde_json::json!({ "physical_delivery_started": false }))); + } + Ok(()) + } +} + +fn target_window(element: &AXElement, deadline: Deadline) -> Result { + crate::tree::attributes::set_messaging_timeout(element, deadline)?; + let result = crate::tree::attributes::copy_element_attr_result(element, "AXWindow", deadline); + if deadline.is_expired() { + return Err(deadline.timeout_error()); + } + match result { + Ok(Some(window)) => Ok(window), + Ok(None) => Err(AdapterError::new( + ErrorCode::ActionFailed, + "Physical input target has no verified owning window", + ) + .with_details(serde_json::json!({ "physical_delivery_started": false }))), + Err(error) => Err(AdapterError::new( + ErrorCode::ActionFailed, + "Could not verify the physical input target window", + ) + .with_details(serde_json::json!({ + "ax_error": error, + "physical_delivery_started": false, + }))), + } +} diff --git a/crates/macos/src/actions/scroll.rs b/crates/macos/src/actions/scroll.rs index 2e34159..c735f70 100644 --- a/crates/macos/src/actions/scroll.rs +++ b/crates/macos/src/actions/scroll.rs @@ -271,31 +271,26 @@ fn physical_wheel( amount: u32, deadline: Deadline, ) -> Result<(), AdapterError> { - let pid = crate::system::app_ops::pid_from_element(target, deadline) - .filter(|pid| *pid > 0) - .ok_or_else(|| AdapterError::new(ErrorCode::StaleRef, "Scroll target has no owner"))?; - crate::system::focus::ensure_app_focused(pid, deadline)?; - let bounds = crate::tree::element_bounds::read_bounds_with_deadline( - target, - crate::actions::scroll_read::deadline_instant(deadline)?, - )? - .ok_or_else(|| AdapterError::new(ErrorCode::ActionFailed, "Scroll target has no bounds"))?; + let prepared = + crate::actions::physical_target::PreparedPhysicalTarget::prepare(target, deadline)?; + let bounds = + crate::tree::hit_test::visible_bounds_ax_element(target, deadline)?.ok_or_else(|| { + AdapterError::new( + ErrorCode::ActionFailed, + "Scroll target has no visible delivery area", + ) + })?; let (vertical, horizontal) = wheel_delta(direction, amount); - crate::system::focus::verify_app_focused(pid, deadline)?; - crate::input::mouse::synthesize_scroll_at( - agent_desktop_core::Point { - x: bounds.x + bounds.width / 2.0, - y: bounds.y + bounds.height / 2.0, - }, - (vertical, horizontal), - &[], - Some(pid), - deadline, - ) + let point = agent_desktop_core::Point { + x: bounds.x + bounds.width / 2.0, + y: bounds.y + bounds.height / 2.0, + }; + prepared.verify_pointer(target, &point, deadline)?; + crate::input::mouse_scroll::synthesize_scroll_at(point, (vertical, horizontal), &[], deadline) } fn wheel_delta(direction: &Direction, amount: u32) -> (i32, i32) { - let units = amount.saturating_mul(5).min(i32::MAX as u32) as i32; + let units = amount.min(i32::MAX as u32) as i32; match direction { Direction::Down => (-units, 0), Direction::Up => (units, 0), diff --git a/crates/macos/src/actions/scroll_read.rs b/crates/macos/src/actions/scroll_read.rs index 0af0b91..c32d7d0 100644 --- a/crates/macos/src/actions/scroll_read.rs +++ b/crates/macos/src/actions/scroll_read.rs @@ -1,5 +1,4 @@ use agent_desktop_core::{AdapterError, Deadline, ErrorCode}; -use std::time::Instant; use crate::tree::AXElement; @@ -86,16 +85,6 @@ pub(crate) fn number( .and_then(|number| number.to_f64())) } -pub(crate) fn deadline_instant(deadline: Deadline) -> Result { - let remaining = deadline.remaining(); - if remaining.is_zero() { - return Err(deadline.timeout_error()); - } - Instant::now() - .checked_add(remaining) - .ok_or_else(|| AdapterError::new(ErrorCode::InvalidArgs, "Deadline is out of range")) -} - fn prepare(element: &AXElement, deadline: Deadline) -> Result<(), AdapterError> { crate::tree::attributes::set_messaging_timeout(element, deadline) } diff --git a/crates/macos/src/actions/scroll_tests.rs b/crates/macos/src/actions/scroll_tests.rs index d72d301..51d23a5 100644 --- a/crates/macos/src/actions/scroll_tests.rs +++ b/crates/macos/src/actions/scroll_tests.rs @@ -37,8 +37,8 @@ fn optional_visibility_pre_step_preserves_terminal_failures() { #[test] fn horizontal_wheel_delta_matches_direction() { - assert_eq!(super::scroll_wheel_delta(&Direction::Right, 2), (0, 10)); - assert_eq!(super::scroll_wheel_delta(&Direction::Left, 2), (0, -10)); + assert_eq!(super::scroll_wheel_delta(&Direction::Right, 2), (0, 2)); + assert_eq!(super::scroll_wheel_delta(&Direction::Left, 2), (0, -2)); } #[test] diff --git a/crates/macos/src/input/mod.rs b/crates/macos/src/input/mod.rs index 16f9d40..8f32f47 100644 --- a/crates/macos/src/input/mod.rs +++ b/crates/macos/src/input/mod.rs @@ -11,4 +11,6 @@ pub(crate) mod mouse; mod mouse_drag; #[cfg(target_os = "macos")] mod mouse_drag_state; +pub(crate) mod mouse_move; +pub(crate) mod mouse_scroll; mod owned_object; diff --git a/crates/macos/src/input/mouse.rs b/crates/macos/src/input/mouse.rs index 6723188..d149171 100644 --- a/crates/macos/src/input/mouse.rs +++ b/crates/macos/src/input/mouse.rs @@ -9,7 +9,6 @@ mod imp { use super::*; use core_graphics::event::{ CGEvent, CGEventFlags, CGEventTapLocation, CGEventType, CGMouseButton, EventField, - ScrollEventUnit, }; use core_graphics::event_source::{CGEventSource, CGEventSourceStateID}; use core_graphics::geometry::CGPoint; @@ -52,7 +51,16 @@ mod imp { let cg_button = to_cg_button(&event.button); let flags = event_flags(&event.modifiers); match event.kind { - MouseEventKind::Move => post_move_events(point, cg_button, flags, deadline), + MouseEventKind::Move => { + let mut delivery = crate::delivery_tracker::DeliveryTracker::default(); + crate::input::mouse_move::post_move_events( + point, + cg_button, + flags, + deadline, + &mut delivery, + ) + } MouseEventKind::Down | MouseEventKind::Up => Err(standalone_state_error()), MouseEventKind::Click { count } => { agent_desktop_core::validate_mouse_click_count(count)?; @@ -66,13 +74,14 @@ mod imp { verify_target, ) } - MouseEventKind::Wheel { delta_x, delta_y } => synthesize_scroll_at( - event.point, - (wheel_lines_to_i32(delta_y)?, wheel_lines_to_i32(delta_x)?), - &event.modifiers, - None, - deadline, - ), + MouseEventKind::Wheel { delta_x, delta_y } => { + crate::input::mouse_scroll::synthesize_scroll_at( + event.point, + (wheel_lines_to_i32(delta_y)?, wheel_lines_to_i32(delta_x)?), + &event.modifiers, + deadline, + ) + } } } @@ -131,6 +140,13 @@ mod imp { let down_ty = down_type(button); let up_ty = up_type(button); let mut delivery = crate::delivery_tracker::DeliveryTracker::default(); + crate::input::mouse_move::post_move_events( + point, + cg_button, + flags, + deadline, + &mut delivery, + )?; for i in 1..=count { ensure_budget(deadline, delivery)?; let down = create_event(down_ty, point, cg_button, flags) @@ -205,36 +221,6 @@ mod imp { .map_err(|()| AdapterError::internal("Failed to create CGEventSource")) } - fn post_move_events( - point: CGPoint, - button: CGMouseButton, - flags: CGEventFlags, - deadline: Deadline, - ) -> Result<(), AdapterError> { - let mut delivery = crate::delivery_tracker::DeliveryTracker::default(); - let source = event_source().map_err(|error| delivery.annotate(error))?; - for position in [approach_point(point), point] { - ensure_budget(deadline, delivery)?; - let event = - create_event_with_source(&source, CGEventType::MouseMoved, position, button, flags) - .map_err(|error| delivery.annotate(error))?; - event.post(CGEventTapLocation::HID); - delivery.mark_delivered(); - } - ensure_budget(deadline, delivery) - } - - pub(crate) fn approach_point(point: CGPoint) -> CGPoint { - CGPoint::new( - if point.x > -999_999.0 { - point.x - 1.0 - } else { - point.x + 1.0 - }, - point.y, - ) - } - pub(crate) fn post_event_with_source( source: &CGEventSource, event: (CGEventType, CGPoint, CGMouseButton, CGEventFlags), @@ -274,34 +260,6 @@ mod imp { } } - pub fn synthesize_scroll_at( - point: Point, - delta: (i32, i32), - modifiers: &[Modifier], - target_pid: Option, - deadline: Deadline, - ) -> Result<(), AdapterError> { - validate_point(&point)?; - let mut delivery = crate::delivery_tracker::DeliveryTracker::default(); - ensure_budget(deadline, delivery)?; - let (dy, dx) = delta; - let (x, y) = (point.x, point.y); - tracing::debug!("mouse: scroll at ({x:.0},{y:.0}) dy={dy} dx={dx} modifiers={modifiers:?}"); - let source = event_source().map_err(|error| delivery.annotate(error))?; - let event = CGEvent::new_scroll_event(source, ScrollEventUnit::LINE, 2, dy, dx, 0) - .map_err(|()| AdapterError::internal("CGEvent::new_scroll_event failed")) - .map_err(|error| delivery.annotate(error))?; - event.set_location(CGPoint::new(x, y)); - event.set_flags(event_flags(modifiers)); - if let Some(pid) = target_pid { - event.post_to_pid(pid); - } else { - event.post(CGEventTapLocation::HID); - } - delivery.mark_delivered(); - ensure_budget(deadline, delivery) - } - pub(crate) fn sleep_bounded( deadline: Deadline, duration: std::time::Duration, @@ -352,19 +310,9 @@ mod imp { pub fn synthesize_drag(_params: DragParams, _deadline: Deadline) -> Result<(), AdapterError> { Err(AdapterError::not_supported("drag")) } - - pub fn synthesize_scroll_at( - _point: Point, - _delta: (i32, i32), - _modifiers: &[Modifier], - _target_pid: Option, - _deadline: Deadline, - ) -> Result<(), AdapterError> { - Err(AdapterError::not_supported("scroll")) - } } -pub(crate) use imp::{synthesize_mouse, synthesize_mouse_after, synthesize_scroll_at}; +pub(crate) use imp::{synthesize_mouse, synthesize_mouse_after}; #[cfg(target_os = "macos")] pub(crate) use crate::input::mouse_drag::synthesize_drag; @@ -378,9 +326,6 @@ pub(crate) use imp::{ sleep_bounded, validate_point, }; -#[cfg(all(test, target_os = "macos"))] -pub(crate) use imp::approach_point; - #[cfg(all(test, target_os = "macos", feature = "interactive-tests"))] pub(crate) use imp::{create_event, standalone_state_error, wheel_lines_to_i32}; diff --git a/crates/macos/src/input/mouse_move.rs b/crates/macos/src/input/mouse_move.rs new file mode 100644 index 0000000..b5fac82 --- /dev/null +++ b/crates/macos/src/input/mouse_move.rs @@ -0,0 +1,141 @@ +#[cfg(target_os = "macos")] +use agent_desktop_core::{AdapterError, Deadline, ErrorCode, Modifier, Point}; +#[cfg(target_os = "macos")] +use core_graphics::{ + event::{CGEvent, CGEventFlags, CGEventTapLocation, CGEventType, CGMouseButton}, + geometry::CGPoint, +}; + +#[cfg(target_os = "macos")] +pub(crate) fn post_move_events( + point: CGPoint, + button: CGMouseButton, + flags: CGEventFlags, + deadline: Deadline, + delivery: &mut crate::delivery_tracker::DeliveryTracker, +) -> Result<(), AdapterError> { + let source = crate::input::mouse::event_source().map_err(|error| delivery.annotate(error))?; + for position in [approach_point(point), point] { + crate::input::mouse::ensure_budget(deadline, *delivery)?; + let event = crate::input::mouse::create_event_with_source( + &source, + CGEventType::MouseMoved, + position, + button, + flags, + ) + .map_err(|error| delivery.annotate(error))?; + event.post(CGEventTapLocation::HID); + delivery.mark_delivered(); + crate::input::mouse::sleep_bounded( + deadline, + std::time::Duration::from_millis(10), + *delivery, + )?; + } + verify_position(source, point, deadline, delivery) +} + +#[cfg(target_os = "macos")] +fn verify_position( + source: core_graphics::event_source::CGEventSource, + requested: CGPoint, + deadline: Deadline, + delivery: &mut crate::delivery_tracker::DeliveryTracker, +) -> Result<(), AdapterError> { + let verification_end = std::time::Instant::now() + std::time::Duration::from_millis(100); + loop { + let observed = CGEvent::new(source.clone()) + .map_err(|()| AdapterError::internal("Failed to read the current pointer position")) + .map_err(|error| delivery.annotate(error))? + .location(); + if pointer_position_matches(observed, requested) { + return Ok(()); + } + if std::time::Instant::now() >= verification_end { + return Err(pointer_position_error(observed, requested, delivery)); + } + crate::input::mouse::sleep_bounded( + deadline, + std::time::Duration::from_millis(5), + *delivery, + )?; + } +} + +#[cfg(target_os = "macos")] +fn pointer_position_error( + observed: CGPoint, + requested: CGPoint, + delivery: &crate::delivery_tracker::DeliveryTracker, +) -> AdapterError { + delivery.annotate( + AdapterError::new( + ErrorCode::ActionFailed, + "Physical pointer did not reach the requested position", + ) + .with_details(serde_json::json!({ + "requested": { "x": requested.x, "y": requested.y }, + "observed": { "x": observed.x, "y": observed.y }, + })), + ) +} + +#[cfg(target_os = "macos")] +pub(crate) fn preposition_pointer( + point: &Point, + modifiers: &[Modifier], + deadline: Deadline, + delivery: &mut crate::delivery_tracker::DeliveryTracker, +) -> Result<(), AdapterError> { + post_move_events( + CGPoint::new(point.x, point.y), + CGMouseButton::Left, + crate::input::mouse::event_flags(modifiers), + deadline, + delivery, + ) +} + +#[cfg(target_os = "macos")] +fn approach_point(point: CGPoint) -> CGPoint { + CGPoint::new( + if point.x > -999_999.0 { + point.x - 1.0 + } else { + point.x + 1.0 + }, + point.y, + ) +} + +#[cfg(target_os = "macos")] +fn pointer_position_matches(observed: CGPoint, requested: CGPoint) -> bool { + (observed.x - requested.x).abs() <= 0.5 && (observed.y - requested.y).abs() <= 0.5 +} + +#[cfg(all(test, target_os = "macos"))] +mod tests { + use super::{approach_point, pointer_position_matches}; + use core_graphics::geometry::CGPoint; + + #[test] + fn approach_moves_one_point_before_the_exact_destination() { + let approach = approach_point(CGPoint::new(2065.0, 636.0)); + assert_eq!(approach.x, 2064.0); + assert_eq!(approach.y, 636.0); + } + + #[test] + fn verification_allows_subpixel_rounding_only() { + let requested = CGPoint::new(2065.0, 636.0); + assert!(pointer_position_matches( + CGPoint::new(2065.4, 635.6), + requested + )); + assert!(!pointer_position_matches( + CGPoint::new(2065.6, 636.0), + requested + )); + } +} diff --git a/crates/macos/src/input/mouse_scroll.rs b/crates/macos/src/input/mouse_scroll.rs new file mode 100644 index 0000000..b967226 --- /dev/null +++ b/crates/macos/src/input/mouse_scroll.rs @@ -0,0 +1,105 @@ +use agent_desktop_core::{AdapterError, Deadline, ErrorCode, Modifier, Point}; + +const MAX_LINES_PER_EVENT: i32 = 10; +const MAX_TOTAL_LINES: i32 = 1_000; + +#[cfg(target_os = "macos")] +pub(crate) fn synthesize_scroll_at( + point: Point, + delta: (i32, i32), + modifiers: &[Modifier], + deadline: Deadline, +) -> Result<(), AdapterError> { + use core_graphics::event::{CGEvent, CGEventTapLocation, ScrollEventUnit}; + use core_graphics::geometry::CGPoint; + + crate::input::mouse::validate_point(&point)?; + let chunks = scroll_chunks(delta)?; + let mut delivery = crate::delivery_tracker::DeliveryTracker::default(); + crate::input::mouse::ensure_budget(deadline, delivery)?; + crate::input::mouse_move::preposition_pointer(&point, modifiers, deadline, &mut delivery)?; + let source = crate::input::mouse::event_source().map_err(|error| delivery.annotate(error))?; + let flags = crate::input::mouse::event_flags(modifiers); + for (index, (dy, dx)) in chunks.iter().copied().enumerate() { + crate::input::mouse::ensure_budget(deadline, delivery)?; + tracing::debug!(x = point.x, y = point.y, dy, dx, "mouse: scroll chunk"); + let event = CGEvent::new_scroll_event(source.clone(), ScrollEventUnit::LINE, 2, dy, dx, 0) + .map_err(|()| AdapterError::internal("CGEvent::new_scroll_event failed")) + .map_err(|error| delivery.annotate(error))?; + event.set_location(CGPoint::new(point.x, point.y)); + event.set_flags(flags); + event.post(CGEventTapLocation::HID); + delivery.mark_delivered(); + if index + 1 < chunks.len() { + crate::input::mouse::sleep_bounded( + deadline, + std::time::Duration::from_millis(5), + delivery, + )?; + } + } + crate::input::mouse::ensure_budget(deadline, delivery) +} + +#[cfg(not(target_os = "macos"))] +pub(crate) fn synthesize_scroll_at( + _point: Point, + _delta: (i32, i32), + _modifiers: &[Modifier], + _deadline: Deadline, +) -> Result<(), AdapterError> { + Err(AdapterError::not_supported("scroll")) +} + +fn scroll_chunks(delta: (i32, i32)) -> Result, AdapterError> { + let (mut dy, mut dx) = delta; + if dy == 0 && dx == 0 { + return Err(AdapterError::new( + ErrorCode::InvalidArgs, + "Wheel delta must be non-zero", + )); + } + if dy.unsigned_abs() > MAX_TOTAL_LINES as u32 || dx.unsigned_abs() > MAX_TOTAL_LINES as u32 { + return Err(AdapterError::new( + ErrorCode::InvalidArgs, + "Wheel delta must be within -1000..=1000 lines per axis", + )); + } + let mut chunks = Vec::new(); + while dy != 0 || dx != 0 { + let next_y = dy.clamp(-MAX_LINES_PER_EVENT, MAX_LINES_PER_EVENT); + let next_x = dx.clamp(-MAX_LINES_PER_EVENT, MAX_LINES_PER_EVENT); + chunks.push((next_y, next_x)); + dy -= next_y; + dx -= next_x; + } + Ok(chunks) +} + +#[cfg(test)] +mod tests { + use super::{MAX_LINES_PER_EVENT, scroll_chunks}; + + #[test] + fn large_wheel_delta_is_split_into_apple_sized_signed_chunks() { + assert_eq!( + scroll_chunks((-25, 12)).expect("bounded wheel delta"), + vec![(-10, 10), (-10, 2), (-5, 0)] + ); + } + + #[test] + fn every_wheel_chunk_stays_within_the_native_event_range() { + let chunks = scroll_chunks((1_000, -1_000)).expect("maximum wheel delta"); + assert_eq!(chunks.len(), 100); + assert!(chunks.iter().all(|(dy, dx)| { + dy.abs() <= MAX_LINES_PER_EVENT && dx.abs() <= MAX_LINES_PER_EVENT + })); + } + + #[test] + fn zero_and_unbounded_wheel_deltas_are_rejected() { + assert!(scroll_chunks((0, 0)).is_err()); + assert!(scroll_chunks((1_001, 0)).is_err()); + } +} diff --git a/crates/macos/src/input/mouse_tests.rs b/crates/macos/src/input/mouse_tests.rs index b0a5eb1..28b10e6 100644 --- a/crates/macos/src/input/mouse_tests.rs +++ b/crates/macos/src/input/mouse_tests.rs @@ -1,6 +1,5 @@ -use super::{Modifier, approach_point, event_flags, standalone_state_error, wheel_lines_to_i32}; +use super::{Modifier, event_flags, standalone_state_error, wheel_lines_to_i32}; use core_graphics::event::CGEventFlags; -use core_graphics::geometry::CGPoint; #[test] fn standalone_mouse_state_is_rejected_without_emission() { @@ -76,14 +75,6 @@ fn wheel_line_conversion_rejects_non_finite_input() { assert!(wheel_lines_to_i32(f64::INFINITY).is_err()); } -#[test] -fn hover_approach_moves_one_point_before_the_exact_destination() { - let approach = approach_point(CGPoint::new(2065.0, 636.0)); - - assert_eq!(approach.x, 2064.0); - assert_eq!(approach.y, 636.0); -} - #[cfg(feature = "interactive-tests")] #[test] fn native_cg_event_contract_is_bounded() { diff --git a/crates/macos/src/system/key_dispatch.rs b/crates/macos/src/system/key_dispatch.rs index 9d433a6..108f1b1 100644 --- a/crates/macos/src/system/key_dispatch.rs +++ b/crates/macos/src/system/key_dispatch.rs @@ -47,13 +47,10 @@ pub(crate) fn press_for_app_impl( if policy.allow_focus_steal { crate::system::process_identity::require_core(&process)?; - crate::system::focus::ensure_app_focused(pid, deadline)?; + crate::system::focus::verify_app_focused(pid, deadline)?; } crate::system::process_identity::require_core(&process)?; require_focused_element(&app, deadline)?; - if policy.allow_focus_steal { - crate::system::focus::verify_app_focused(pid, deadline)?; - } crate::system::process_identity::require_core(&process)?; crate::input::keyboard::synthesize_key(combo, Some(pid), deadline)?; post_delivery_process_result(&process) diff --git a/crates/macos/src/tree/hit_test.rs b/crates/macos/src/tree/hit_test.rs index eae8cf8..d4a0a23 100644 --- a/crates/macos/src/tree/hit_test.rs +++ b/crates/macos/src/tree/hit_test.rs @@ -8,7 +8,7 @@ mod imp { AXUIElementCreateSystemWide, kAXApplicationRole, kAXErrorSuccess, kAXRoleAttribute, }; use agent_desktop_core::{ - AdapterError, Point, hit_test::HitTestResult, native_handle::NativeHandle, + AdapterError, Point, Rect, hit_test::HitTestResult, native_handle::NativeHandle, }; /// Hit-tests `point` in the system-wide accessibility hierarchy, then @@ -34,6 +34,14 @@ mod imp { hit_test_element(target, point, deadline) } + pub(crate) fn visible_bounds_ax_element( + target: &AXElement, + deadline: agent_desktop_core::Deadline, + ) -> Result, AdapterError> { + let deadline = crate::tree::locator_deadline::from_operation(deadline)?; + clipped_bounds(target, deadline) + } + fn hit_test_element( target: &AXElement, point: Point, @@ -222,6 +230,21 @@ mod imp { point: &Point, deadline: std::time::Instant, ) -> Result { + Ok(clipped_bounds(target, deadline)?.is_some_and(|bounds| { + point.x >= bounds.x + && point.y >= bounds.y + && point.x <= bounds.x + bounds.width + && point.y <= bounds.y + bounds.height + })) + } + + fn clipped_bounds( + target: &AXElement, + deadline: std::time::Instant, + ) -> Result, AdapterError> { + let Some(mut visible) = read_bounds_with_deadline(target, deadline)? else { + return Ok(None); + }; let mut current = target.clone(); let mut visited = Vec::new(); let mut usage = crate::tree::observation_usage::ObservationUsage::new( @@ -229,7 +252,7 @@ mod imp { ); for _ in 0..ABSOLUTE_MAX_DEPTH { if !remember_ancestor(&mut visited, ¤t) { - return Ok(false); + return Ok(None); } let role = match read_complete_text( ¤t, @@ -239,31 +262,41 @@ mod imp { "hit_test.clip_role", ) { Ok(role) => role, - Err(_) => return Ok(false), + Err(_) => return Ok(None), }; if ends_clipping_walk(role.as_deref()) { - return Ok(true); + return Ok(Some(visible)); } if role.as_deref().is_some_and(clips_descendants) { let Some(bounds) = read_bounds_with_deadline(¤t, deadline)? else { - return Ok(false); + return Ok(None); }; - if point.x < bounds.x - || point.y < bounds.y - || point.x > bounds.x + bounds.width - || point.y > bounds.y + bounds.height - { - return Ok(false); - } + let Some(intersection) = intersect_rects(visible, bounds) else { + return Ok(None); + }; + visible = intersection; } current = match crate::tree::resolve_ax_read::read_element(¤t, "AXParent", deadline) { Ok(Some(parent)) => parent, - Ok(None) => return Ok(true), - Err(_) => return Ok(false), + Ok(None) => return Ok(Some(visible)), + Err(_) => return Ok(None), }; } - Ok(false) + Ok(None) + } + + pub(super) fn intersect_rects(left: Rect, right: Rect) -> Option { + let x = left.x.max(right.x); + let y = left.y.max(right.y); + let right_edge = (left.x + left.width).min(right.x + right.width); + let bottom_edge = (left.y + left.height).min(right.y + right.height); + (right_edge > x && bottom_edge > y).then_some(Rect { + x, + y, + width: right_edge - x, + height: bottom_edge - y, + }) } fn clips_descendants(role: &str) -> bool { @@ -334,7 +367,7 @@ mod imp { pub(crate) use imp::hit_test_impl; #[cfg(target_os = "macos")] -pub(crate) use imp::hit_test_ax_element; +pub(crate) use imp::{hit_test_ax_element, visible_bounds_ax_element}; #[cfg(all(test, target_os = "macos"))] #[path = "hit_test_tests.rs"] diff --git a/crates/macos/src/tree/hit_test_tests.rs b/crates/macos/src/tree/hit_test_tests.rs index eb457b7..462a63c 100644 --- a/crates/macos/src/tree/hit_test_tests.rs +++ b/crates/macos/src/tree/hit_test_tests.rs @@ -1,10 +1,10 @@ use super::imp::{ - HitClassification, ax_point, classify_relation, ends_clipping_walk, needs_application_retry, - remember_ancestor, + HitClassification, ax_point, classify_relation, ends_clipping_walk, intersect_rects, + needs_application_retry, remember_ancestor, }; use crate::tree::AXElement; use accessibility_sys::AXUIElementCreateSystemWide; -use agent_desktop_core::Point; +use agent_desktop_core::{Point, Rect}; #[test] fn self_hit_reaches_target() { @@ -63,6 +63,32 @@ fn application_root_completes_the_clipping_walk_without_a_parent() { assert!(!ends_clipping_walk(None)); } +#[test] +fn clipped_bounds_keep_only_the_visible_part_of_a_partially_shown_target() { + let target = Rect { + x: 100.0, + y: 900.0, + width: 220.0, + height: 160.0, + }; + let viewport = Rect { + x: 0.0, + y: 0.0, + width: 1000.0, + height: 950.0, + }; + + assert_eq!( + intersect_rects(target, viewport), + Some(Rect { + x: 100.0, + y: 900.0, + width: 220.0, + height: 50.0, + }) + ); +} + #[test] fn ancestor_walk_retains_handles_and_detects_the_same_element() { let system = AXElement(unsafe { AXUIElementCreateSystemWide() }); diff --git a/crates/macos/src/tree/locator_deadline.rs b/crates/macos/src/tree/locator_deadline.rs index be77aa0..eb7f576 100644 --- a/crates/macos/src/tree/locator_deadline.rs +++ b/crates/macos/src/tree/locator_deadline.rs @@ -38,6 +38,7 @@ fn remaining_at(deadline: Instant, now: Instant) -> Result&2 exit 2 } +"$bin" --headed mouse-move --xy 20,20 >/dev/null 2>&1 || { + echo "fixture cursor precondition could not be established" >&2 + exit 2 +} guard_exec 10 1048576 open "$fixture_app" || { echo "fixture could not be opened" >&2 diff --git a/tests/e2e/scenarios-interaction.sh b/tests/e2e/scenarios-interaction.sh index d682044..422c161 100644 --- a/tests/e2e/scenarios-interaction.sh +++ b/tests/e2e/scenarios-interaction.sh @@ -117,15 +117,15 @@ assert "headed triple-click delivers a three-tap gesture" \ "$([ "$triple_after" = "triple-clicked" ] && echo 1 || echo 0)" \ "before=$triple_before after=$triple_after" -require_value hover_before hover-status -assert "hover baseline is clean" "$([ "$hover_before" != "hovered" ] && echo 1 || echo 0)" \ - "status=$hover_before" hover_preposition="$("$bin" --headed mouse-move --xy 20,20 2>&1)" "$bin" focus-window --app "$app" >/dev/null 2>&1 sleep 0.2 assert "hover precondition positions the pointer outside the target" \ "$([ "$(json_field "$hover_preposition" ok)" = "True" ] && echo 1 || echo 0)" \ "ok=$(json_field "$hover_preposition" ok) error=$(json_field "$hover_preposition" error.code)" +require_value hover_before hover-status +assert "hover baseline is clean after moving outside the target" \ + "$([ "$hover_before" != "hovered" ] && echo 1 || echo 0)" "status=$hover_before" hover_snapshot="$("$bin" snapshot --app "$app" --include-bounds --max-depth 30 2>/dev/null)" hover_xy="$(printf '%s' "$hover_snapshot" | python3 "$json_tool" tree hover-target center 2>/dev/null)" || hover_xy="" if [ -z "$hover_xy" ]; then diff --git a/tests/e2e/scenarios-surfaces.sh b/tests/e2e/scenarios-surfaces.sh index 6e76cac..ee6abed 100644 --- a/tests/e2e/scenarios-surfaces.sh +++ b/tests/e2e/scenarios-surfaces.sh @@ -43,6 +43,8 @@ assert "context-menu item action is independently observed" \ "before=$context_before after=$context_after" note "Menu bar enumeration" +"$bin" focus-window --app "$app" >/dev/null 2>&1 +sleep 0.2 menu_snapshot="$("$bin" snapshot --app "$app" --surface menubar --max-depth 5 2>/dev/null)" menu_items="$(printf '%s' "$menu_snapshot" | grep -o '"role":"menuitem"' | wc -l | tr -d ' ')" has_fixture_menu="$(printf '%s' "$menu_snapshot" | grep -q '"name":"Fixture"' && echo 1 || echo 0)" @@ -79,8 +81,12 @@ note "Disclosure collapse and expand" disclosed_present() { local out out="$("$bin" find --app "$app" --role statictext --name disclosed-content --first 2>/dev/null)" - [ "$(json_field "$out" ok)" = "True" ] && \ - [ -n "$(json_field "$out" data.match)" ] && echo 1 || echo 0 + if [ "$(json_field "$out" ok)" = "True" ] && [ -n "$(json_field "$out" data.match)" ]; then + echo 1 + return + fi + out="$("$bin" find --app "$app" --role statictext --name disclosure-section --first 2>/dev/null)" + [ "$(json_field "$out" ok)" = "True" ] && [ -n "$(json_field "$out" data.match)" ] && echo 1 || echo 0 } require_target disclosure disclosure disclosure-section initially_hidden="$(disclosed_present)" diff --git a/tests/e2e/scenarios-trace-performance.sh b/tests/e2e/scenarios-trace-performance.sh index fe93720..24c1461 100644 --- a/tests/e2e/scenarios-trace-performance.sh +++ b/tests/e2e/scenarios-trace-performance.sh @@ -2,7 +2,7 @@ note "Trace JSONL and secret redaction" trace_file="$(mktemp -t agentdesk-e2e-trace.XXXXXX)" cleanup_files+=("$trace_file") require_target trace_text textfield text-input -trace_type="$("$bin" --trace "$trace_file" type "$(target_ref "$trace_text")" \ +trace_type="$("$bin" --headed --trace "$trace_file" type "$(target_ref "$trace_text")" \ --snapshot "$(target_snapshot "$trace_text")" "sup3r-secret-trace" 2>&1)" sleep 0.2 trace_bytes="$(wc -c < "$trace_file" | tr -d ' ')" @@ -36,7 +36,7 @@ if [ -z "$trace_primary" ] || [ -z "$trace_text" ]; then fi trace_click="$("$bin" --session "$trace_session" click "$(target_ref "$trace_primary")" \ --snapshot "$(target_snapshot "$trace_primary")" 2>&1)" -trace_session_type="$("$bin" --session "$trace_session" type "$(target_ref "$trace_text")" \ +trace_session_type="$("$bin" --headed --session "$trace_session" type "$(target_ref "$trace_text")" \ --snapshot "$(target_snapshot "$trace_text")" trace-e2e 2>&1)" sleep 0.5 trace_show="$("$bin" --session "$trace_session" trace show --limit 0 2>/dev/null)" @@ -107,7 +107,7 @@ record_timing "click AX press" "$bin" click "$(target_ref "$perf_primary")" \ --snapshot "$(target_snapshot "$perf_primary")" record_timing "set text value" "$bin" set-value "$(target_ref "$perf_text")" \ --snapshot "$(target_snapshot "$perf_text")" perf-probe -record_timing "type text" "$bin" type "$(target_ref "$perf_text")" \ +record_timing "type text" "$bin" --headed type "$(target_ref "$perf_text")" \ --snapshot "$(target_snapshot "$perf_text")" perf awk -F'\t' '{printf " %-26s %9.1f ms\n",$1,$2; n++; s+=$2} diff --git a/tests/e2e/test_harness_contract.py b/tests/e2e/test_harness_contract.py index c1d9008..c2ed931 100644 --- a/tests/e2e/test_harness_contract.py +++ b/tests/e2e/test_harness_contract.py @@ -80,7 +80,10 @@ class HarnessContractTests(unittest.TestCase): self.assertIn('"$bin" --session "$trace_session" find', source) self.assertIn('trace_click="$("$bin" --session "$trace_session" click', source) - self.assertIn('trace_session_type="$("$bin" --session "$trace_session" type', source) + self.assertIn( + 'trace_session_type="$("$bin" --headed --session "$trace_session" type', + source, + ) def test_sheet_scenarios_scroll_the_button_before_clicking(self): fixtures = [