diff --git a/README.md b/README.md index f7f962e..0eecef4 100644 --- a/README.md +++ b/README.md @@ -193,7 +193,7 @@ agent-desktop list-surfaces --app Notes # list menus, sheets, popovers, agent-desktop click @s8f3k2p9:e3 # semantic AX-first click agent-desktop double-click @s8f3k2p9:e3 # AXOpen; physical double-click uses --headed mouse-click --count 2 agent-desktop triple-click @s8f3k2p9:e3 # POLICY_DENIED if physical input is disabled -agent-desktop right-click @s8f3k2p9:e3 # open verified context menu +agent-desktop right-click @s8f3k2p9:e3 # open context menu; inspect effect before retrying agent-desktop type @s8f3k2p9:e5 "hello world" # insert text into element agent-desktop set-value @s8f3k2p9:e5 "new value" # set value directly via AX agent-desktop clear @s8f3k2p9:e5 # clear element value diff --git a/crates/core/src/actionability/report.rs b/crates/core/src/actionability/report.rs index 066acdd..a0f0aed 100644 --- a/crates/core/src/actionability/report.rs +++ b/crates/core/src/actionability/report.rs @@ -34,8 +34,8 @@ impl ActionabilityReport { pub(crate) fn terminal_code(&self) -> Option { self.checks .iter() - .filter(|check| !matches!(check.status, ActionabilityStatus::Pass)) - .find_map(|check| check.terminal_code.clone()) + .find(|check| !matches!(check.status, ActionabilityStatus::Pass)) + .and_then(|check| check.terminal_code.clone()) } pub(crate) fn failure_reasons(&self) -> String { @@ -50,3 +50,34 @@ impl ActionabilityReport { .join(", ") } } + +#[cfg(test)] +mod tests { + use super::*; + + fn failed(check: &'static str, terminal_code: Option) -> ActionabilityCheck { + ActionabilityCheck { + check, + status: ActionabilityStatus::Fail, + reason: None, + occluder: None, + terminal_code, + hit_test: None, + stability: None, + } + } + + #[test] + fn retryable_failure_is_not_overridden_by_a_later_terminal_check() { + let report = ActionabilityReport::from_checks( + vec![ + failed("enabled", None), + failed("supported_action", Some(ErrorCode::PolicyDenied)), + ], + None, + PointerDelivery::NotApplicable, + ); + + assert_eq!(report.terminal_code(), None); + } +} diff --git a/crates/core/src/commands/wait_element.rs b/crates/core/src/commands/wait_element.rs index 096382f..c4d1d63 100644 --- a/crates/core/src/commands/wait_element.rs +++ b/crates/core/src/commands/wait_element.rs @@ -49,33 +49,44 @@ pub(crate) fn wait_for_element( return wait_timeout::element(ref_id, predicate, timeout_ms, last_observed); } ResolveAttemptOutcome::Resolved(handle) => { - let observed = wait_predicate::observe( + match wait_predicate::observe( &entry, &handle, &predicate, adapter, deadline, crate::actionability::StabilityExpectation::strict_hash(expected_bounds_hash), - ); - last_observed = observed.map_err(AppError::Adapter)?; - if let Some(observed) = last_observed - .get("observed_bounds_hash") - .and_then(Value::as_u64) - { - expected_bounds_hash = Some(observed); - } - if wait_predicate::satisfied(&predicate, &last_observed) { - let elapsed = start.elapsed().as_millis(); - return Ok(json!({ - "found": true, - "ref": ref_id, - "predicate": predicate.name(), - "observed": last_observed, - "elapsed_ms": elapsed - })); + ) { + Ok(observed) => { + last_observed = observed; + if let Some(observed) = last_observed + .get("observed_bounds_hash") + .and_then(Value::as_u64) + { + expected_bounds_hash = Some(observed); + } + if wait_predicate::satisfied(&predicate, &last_observed) { + let elapsed = start.elapsed().as_millis(); + return Ok(json!({ + "found": true, + "ref": ref_id, + "predicate": predicate.name(), + "observed": last_observed, + "elapsed_ms": elapsed + })); + } + } + Err(err) if is_retryable_wait_error(&err) => { + last_observed = json!({ + "error": err.code.as_str(), + "message": err.message, + "details": err.details + }); + } + Err(err) => return Err(AppError::Adapter(err)), } } - ResolveAttemptOutcome::Failed(err) if is_retryable_wait_resolution_error(&err) => { + ResolveAttemptOutcome::Failed(err) if is_retryable_wait_error(&err) => { last_observed = json!({ "error": err.code.as_str(), "message": err.message @@ -92,6 +103,6 @@ pub(crate) fn wait_for_element( } } -fn is_retryable_wait_resolution_error(error: &crate::AdapterError) -> bool { +fn is_retryable_wait_error(error: &crate::AdapterError) -> bool { error.is_explicitly_retryable() } diff --git a/crates/core/src/commands/wait_predicate_tests.rs b/crates/core/src/commands/wait_predicate_tests.rs index 10ee3a6..412a987 100644 --- a/crates/core/src/commands/wait_predicate_tests.rs +++ b/crates/core/src/commands/wait_predicate_tests.rs @@ -20,6 +20,7 @@ fn live_bounds() -> Option { struct FlippingPredicateAdapter { states: Mutex>>, + remaining_errors: Mutex, } impl ObservationOps for FlippingPredicateAdapter { @@ -36,6 +37,13 @@ impl ObservationOps for FlippingPredicateAdapter { _handle: &NativeHandle, _deadline: crate::Deadline, ) -> Result { + let mut remaining_errors = self.remaining_errors.lock().unwrap(); + if *remaining_errors > 0 { + *remaining_errors -= 1; + return Err(AdapterError::app_unresponsive("Fixture") + .with_details(serde_json::json!({ "retryable": true }))); + } + drop(remaining_errors); let states = self.states.lock().unwrap().pop().unwrap_or_default(); Ok(crate::LiveElement { identity: crate::adapter::live_identity("Run"), @@ -242,6 +250,7 @@ fn element_wait_actionable_retries_until_live_state_converges() { let snapshot_id = snapshot_with_disabled_ref(); let adapter = FlippingPredicateAdapter { states: Mutex::new(vec![vec![], vec!["disabled".into()]]), + remaining_errors: Mutex::new(0), }; let value = wait_for_element_test( @@ -260,6 +269,31 @@ fn element_wait_actionable_retries_until_live_state_converges() { assert_eq!(value["observed"]["actionable"], true); } +#[test] +fn element_wait_actionable_retries_transient_observation_errors() { + let _guard = HomeGuard::new(); + let snapshot_id = snapshot_with_disabled_ref(); + let adapter = FlippingPredicateAdapter { + states: Mutex::new(vec![vec![]]), + remaining_errors: Mutex::new(1), + }; + + let value = wait_for_element_test( + "@e1".into(), + Some(snapshot_id), + wait_predicate::ElementPredicate::Actionable( + crate::action_request::ActionRequest::headless(crate::action::Action::Click), + ), + 500, + &adapter, + &crate::context::CommandContext::default(), + ) + .unwrap(); + + assert_eq!(value["observed"]["actionable"], true); + assert_eq!(*adapter.remaining_errors.lock().unwrap(), 0); +} + #[test] fn element_wait_actionable_type_fails_on_uneditable_role() { let _guard = HomeGuard::new(); diff --git a/crates/macos/src/actions/chain_disclosure_steps.rs b/crates/macos/src/actions/chain_disclosure_steps.rs index a800dd8..15e0170 100644 --- a/crates/macos/src/actions/chain_disclosure_steps.rs +++ b/crates/macos/src/actions/chain_disclosure_steps.rs @@ -24,10 +24,9 @@ mod imp { expanded: bool, deadline: Deadline, ) -> Result { - let Some(current) = disclosed_state(element, deadline)? else { - return Ok(DeliveryOutcome::NotDelivered); - }; - if current == expanded { + let (satisfied, allow_toggle) = + disclosure_plan(disclosed_state(element, deadline)?, expanded); + if satisfied { return Ok(DeliveryOutcome::SatisfiedNoDelivery); } let action = if expanded { "AXExpand" } else { "AXCollapse" }; @@ -47,13 +46,19 @@ mod imp { return verify_disclosure(element, expanded, deadline).map_err(after_delivery); } } - prepare(element, deadline)?; - if crate::actions::ax_helpers::try_ax_action_or_err(element, "AXPress", deadline)? { - return verify_disclosure(element, expanded, deadline).map_err(after_delivery); + if allow_toggle { + prepare(element, deadline)?; + if crate::actions::ax_helpers::try_ax_action_or_err(element, "AXPress", deadline)? { + return verify_disclosure(element, expanded, deadline).map_err(after_delivery); + } } Ok(DeliveryOutcome::NotDelivered) } + fn disclosure_plan(current: Option, desired: bool) -> (bool, bool) { + (current == Some(desired), current == Some(!desired)) + } + fn verify_disclosure( element: &AXElement, expanded: bool, @@ -97,6 +102,21 @@ mod imp { delivery.mark_delivered(); delivery.annotate(error) } + + #[cfg(test)] + mod tests { + use super::disclosure_plan; + + #[test] + fn disclosure_plan_never_blindly_toggles_unknown_state() { + assert_eq!(disclosure_plan(Some(true), true), (true, false)); + assert_eq!(disclosure_plan(Some(false), true), (false, true)); + assert_eq!(disclosure_plan(None, true), (false, false)); + assert_eq!(disclosure_plan(Some(false), false), (true, false)); + assert_eq!(disclosure_plan(Some(true), false), (false, true)); + assert_eq!(disclosure_plan(None, false), (false, false)); + } + } } #[cfg(target_os = "macos")] diff --git a/crates/macos/src/notifications/actions.rs b/crates/macos/src/notifications/actions.rs index 86865f8..fb15244 100644 --- a/crates/macos/src/notifications/actions.rs +++ b/crates/macos/src/notifications/actions.rs @@ -18,7 +18,7 @@ pub fn dismiss_notification( ) -> Result { require_foreground_policy(policy)?; let session = NcSession::open(deadline)?; - let result = dismiss_impl(index, app_filter, identity, policy, deadline); + let result = dismiss_impl(index, app_filter, identity, policy, session.pid(), deadline); close_session(session, result) } @@ -29,7 +29,7 @@ pub fn dismiss_all( ) -> Result<(Vec, Vec), AdapterError> { require_foreground_policy(policy)?; let session = NcSession::open(deadline)?; - let result = dismiss_all_impl(app_filter, policy, deadline); + let result = dismiss_all_impl(app_filter, policy, session.pid(), deadline); close_session(session, result) } @@ -42,7 +42,7 @@ pub fn notification_action( ) -> Result { require_foreground_policy(policy)?; let session = NcSession::open(deadline)?; - let result = action_impl(index, identity, action_name, deadline); + let result = action_impl(index, identity, action_name, session.pid(), deadline); close_session(session, result) } @@ -52,10 +52,11 @@ fn dismiss_impl( app_filter: Option<&str>, identity: Option<&NotificationIdentity>, policy: InteractionPolicy, + pid: i32, deadline: Deadline, ) -> Result { let filter = build_filter(app_filter); - let entries = super::list::list_entries(&filter, deadline)?; + let entries = super::list::list_entries(&filter, pid, deadline)?; let entry = entries .iter() @@ -65,7 +66,7 @@ fn dismiss_impl( let info = entry.info.clone(); let matching_before = super::dismiss_verify::matching_count(&entries, entry); - dismiss_entry(entry, policy, &filter, matching_before, deadline)?; + dismiss_entry(entry, policy, &filter, matching_before, pid, deadline)?; Ok(info) } @@ -73,16 +74,17 @@ fn dismiss_impl( fn dismiss_all_impl( app_filter: Option<&str>, policy: InteractionPolicy, + pid: i32, deadline: Deadline, ) -> Result<(Vec, Vec), AdapterError> { let filter = build_filter(app_filter); - let entries = super::list::list_entries(&filter, deadline)?; + let entries = super::list::list_entries(&filter, pid, deadline)?; let mut dismissed = Vec::new(); let mut failures = Vec::new(); for original in entries.iter().rev() { - let current_entries = super::list::list_entries(&filter, deadline)?; + let current_entries = super::list::list_entries(&filter, pid, deadline)?; let Some(current) = current_entries .iter() .find(|entry| super::dismiss_verify::matches(original, entry)) @@ -94,7 +96,7 @@ fn dismiss_all_impl( continue; }; let matching_before = super::dismiss_verify::matching_count(¤t_entries, current); - match dismiss_entry(current, policy, &filter, matching_before, deadline) { + match dismiss_entry(current, policy, &filter, matching_before, pid, deadline) { Ok(()) => dismissed.push(original.info.clone()), Err(e) => failures.push(format!("#{}: {}", original.info.index, e)), } @@ -109,12 +111,13 @@ fn dismiss_entry( policy: InteractionPolicy, filter: &NotificationFilter, matching_before: usize, + pid: i32, deadline: Deadline, ) -> Result<(), AdapterError> { for action in ["AXDismiss", "AXRemoveFromParent"] { let outcome = crate::actions::ax_helpers::try_ax_action_or_err(&entry.element, action, deadline); - if strategy_verified(outcome, entry, filter, matching_before, deadline)? { + if strategy_verified(outcome, entry, filter, matching_before, pid, deadline)? { return Ok(()); } } @@ -125,7 +128,7 @@ fn dismiss_entry( )? .unwrap_or_default(); let pressed = try_dismiss_button(&children, deadline)?; - if strategy_verified(Ok(pressed), entry, filter, matching_before, deadline)? { + if strategy_verified(Ok(pressed), entry, filter, matching_before, pid, deadline)? { return Ok(()); } @@ -151,7 +154,7 @@ fn dismiss_entry( )? .unwrap_or_default(); let pressed = try_dismiss_button(&children, deadline)?; - if strategy_verified(Ok(pressed), entry, filter, matching_before, deadline)? { + if strategy_verified(Ok(pressed), entry, filter, matching_before, pid, deadline)? { return Ok(()); } @@ -203,10 +206,11 @@ fn action_impl( index: usize, identity: Option<&NotificationIdentity>, action_name: &str, + pid: i32, deadline: Deadline, ) -> Result { let filter = NotificationFilter::default(); - let entries = super::list::list_entries(&filter, deadline)?; + let entries = super::list::list_entries(&filter, pid, deadline)?; let entry = entries .into_iter() @@ -339,12 +343,13 @@ fn strategy_verified( entry: &super::list::NotificationEntry, filter: &NotificationFilter, matching_before: usize, + pid: i32, deadline: Deadline, ) -> Result { if !strategy_succeeded(outcome, deadline)? { return Ok(false); } - match super::dismiss_verify::disappeared(entry, filter, matching_before, deadline) { + match super::dismiss_verify::disappeared(entry, filter, matching_before, pid, deadline) { Ok(disappeared) => Ok(disappeared), Err(error) => { crate::notifications::read::tolerate_ax_strategy_error(error, deadline)?; @@ -366,6 +371,7 @@ fn dismiss_impl( _app_filter: Option<&str>, _identity: Option<&NotificationIdentity>, _policy: InteractionPolicy, + _pid: i32, _deadline: Deadline, ) -> Result { Err(AdapterError::not_supported("dismiss_notification")) @@ -375,6 +381,7 @@ fn dismiss_impl( fn dismiss_all_impl( _app_filter: Option<&str>, _policy: InteractionPolicy, + _pid: i32, _deadline: Deadline, ) -> Result<(Vec, Vec), AdapterError> { Err(AdapterError::not_supported("dismiss_all_notifications")) @@ -385,6 +392,7 @@ fn action_impl( _index: usize, _identity: Option<&NotificationIdentity>, _action_name: &str, + _pid: i32, _deadline: Deadline, ) -> Result { Err(AdapterError::not_supported("notification_action")) diff --git a/crates/macos/src/notifications/dismiss_verify.rs b/crates/macos/src/notifications/dismiss_verify.rs index a038b5e..f985f4b 100644 --- a/crates/macos/src/notifications/dismiss_verify.rs +++ b/crates/macos/src/notifications/dismiss_verify.rs @@ -15,12 +15,13 @@ pub(super) fn disappeared( original: &NotificationEntry, filter: &NotificationFilter, matching_before: usize, + pid: i32, deadline: Deadline, ) -> Result { let settle_deadline = deadline.capped(STRATEGY_SETTLE_TIME); let result = wait_with( || { - let current = super::list::list_entries(filter, settle_deadline)?; + let current = super::list::list_entries(filter, pid, settle_deadline)?; Ok(matching_count(¤t, original) >= matching_before) }, settle_deadline, diff --git a/crates/macos/src/notifications/list.rs b/crates/macos/src/notifications/list.rs index cf612b9..abff46f 100644 --- a/crates/macos/src/notifications/list.rs +++ b/crates/macos/src/notifications/list.rs @@ -7,16 +7,17 @@ pub fn list_notifications( deadline: Deadline, ) -> Result, AdapterError> { let session = NcSession::open(deadline)?; - let result = list_from_nc(filter, deadline); + let result = list_from_nc(filter, session.pid(), deadline); close_session(session, result) } #[cfg(target_os = "macos")] fn list_from_nc( filter: &NotificationFilter, + pid: i32, deadline: Deadline, ) -> Result, AdapterError> { - let entries = list_entries(filter, deadline)?; + let entries = list_entries(filter, pid, deadline)?; Ok(entries.into_iter().map(|e| e.info).collect()) } @@ -29,13 +30,11 @@ pub(super) struct NotificationEntry { #[cfg(target_os = "macos")] pub(super) fn list_entries( filter: &NotificationFilter, + pid: i32, deadline: Deadline, ) -> Result, AdapterError> { use crate::tree::element_for_pid; - let pid = super::nc_session::nc_pid(deadline) - .ok_or_else(|| AdapterError::internal("Notification Center process not found"))?; - let app = element_for_pid(pid); let windows = crate::notifications::read::children_for_attribute(&app, "AXWindows", deadline)?; if windows.is_empty() { @@ -81,6 +80,7 @@ pub(super) fn matches_filters( #[cfg(not(target_os = "macos"))] fn list_from_nc( _filter: &NotificationFilter, + _pid: i32, _deadline: Deadline, ) -> Result, AdapterError> { Err(AdapterError::not_supported("list_notifications")) diff --git a/crates/macos/src/notifications/nc_session.rs b/crates/macos/src/notifications/nc_session.rs index b3e4a60..e0a4792 100644 --- a/crates/macos/src/notifications/nc_session.rs +++ b/crates/macos/src/notifications/nc_session.rs @@ -16,6 +16,7 @@ pub(crate) fn close_session( } pub(crate) struct NcSession { + pid: i32, was_already_open: bool, previous_app: Option, closed: bool, @@ -25,12 +26,15 @@ pub(crate) struct NcSession { impl NcSession { pub(crate) fn open(deadline: Deadline) -> Result { let previous_app = frontmost_app(deadline); - let was_already_open = is_nc_open(deadline); - if !was_already_open { - open_nc(deadline)?; - wait_for_nc_ready(deadline)?; - } + let (was_already_open, pid) = match nc_pid(deadline)? { + Some(pid) if is_nc_open(pid, deadline) => (true, pid), + _ => { + open_nc(deadline)?; + (false, wait_for_nc_ready(deadline)?) + } + }; Ok(Self { + pid, was_already_open, previous_app, closed: false, @@ -38,6 +42,10 @@ impl NcSession { }) } + pub(crate) fn pid(&self) -> i32 { + self.pid + } + pub(crate) fn close(mut self) -> Result<(), AdapterError> { let close_result = if self.was_already_open { Ok(()) @@ -130,31 +138,36 @@ fn applescript_string(value: &str) -> String { fn reactivate_app(_name: &str, _deadline: Deadline) {} #[cfg(target_os = "macos")] -pub(super) fn nc_pid(deadline: Deadline) -> Option { +fn nc_pid(deadline: Deadline) -> Result, AdapterError> { let mut command = std::process::Command::new("/usr/bin/pgrep"); command.arg("-x").arg("NotificationCenter"); let output = crate::system::process::run_with_deadline( &mut command, "pgrep NotificationCenter", - operation_deadline(deadline, std::time::Duration::from_secs(1)).ok()?, - ) - .ok()?; - - String::from_utf8_lossy(&output.stdout) - .trim() - .lines() - .next() - .and_then(|line| line.trim().parse::().ok()) + operation_deadline(deadline, std::time::Duration::from_secs(1))?, + ); + nc_pid_from_output(output) } #[cfg(target_os = "macos")] -fn is_nc_open(deadline: Deadline) -> bool { +fn nc_pid_from_output( + output: Result, +) -> Result, AdapterError> { + let output = output?; + if !output.status.success() { + return Ok(None); + } + Ok(String::from_utf8_lossy(&output.stdout) + .trim() + .lines() + .next() + .and_then(|line| line.trim().parse::().ok())) +} + +#[cfg(target_os = "macos")] +fn is_nc_open(pid: i32, deadline: Deadline) -> bool { use crate::tree::element_for_pid; - let pid = match nc_pid(deadline) { - Some(p) => p, - None => return false, - }; let app = element_for_pid(pid); let windows = crate::notifications::read::children_for_attribute(&app, "AXWindows", deadline) .unwrap_or_default(); @@ -162,7 +175,12 @@ fn is_nc_open(deadline: Deadline) -> bool { } #[cfg(not(target_os = "macos"))] -fn is_nc_open(_deadline: Deadline) -> bool { +fn nc_pid(_deadline: Deadline) -> Result, AdapterError> { + Ok(None) +} + +#[cfg(not(target_os = "macos"))] +fn is_nc_open(_pid: i32, _deadline: Deadline) -> bool { false } @@ -203,7 +221,8 @@ fn close_nc(deadline: Deadline) -> Result<(), AdapterError> { #[cfg(all(test, target_os = "macos"))] mod tests { - use super::applescript_string; + use super::{applescript_string, nc_pid_from_output}; + use agent_desktop_core::AdapterError; #[test] fn applescript_string_escapes_quotes_and_backslashes() { @@ -220,6 +239,14 @@ mod tests { assert_eq!(applescript_string("a\\\nb"), r#""a\\b""#); assert_eq!(applescript_string("a\"b\nc"), r#""a\"bc""#); } + + #[test] + fn nc_pid_preserves_probe_errors() { + let error = nc_pid_from_output(Err(AdapterError::timeout("pid probe timed out"))) + .expect_err("timeout must not become process-not-found"); + + assert_eq!(error.code, agent_desktop_core::ErrorCode::Timeout); + } } #[cfg(not(target_os = "macos"))] @@ -228,12 +255,14 @@ fn close_nc(_deadline: Deadline) -> Result<(), AdapterError> { } #[cfg(target_os = "macos")] -fn wait_for_nc_ready(deadline: Deadline) -> Result<(), AdapterError> { +fn wait_for_nc_ready(deadline: Deadline) -> Result { let poll = std::time::Duration::from_millis(50); loop { - if is_nc_open(deadline) { - return Ok(()); + if let Some(pid) = nc_pid(deadline)? { + if is_nc_open(pid, deadline) { + return Ok(pid); + } } if deadline.is_expired() { return Err(AdapterError::timeout( @@ -245,7 +274,7 @@ fn wait_for_nc_ready(deadline: Deadline) -> Result<(), AdapterError> { } #[cfg(not(target_os = "macos"))] -fn wait_for_nc_ready(_deadline: Deadline) -> Result<(), AdapterError> { +fn wait_for_nc_ready(_deadline: Deadline) -> Result { Err(AdapterError::not_supported("wait_for_nc_ready")) } diff --git a/skills/agent-desktop/SKILL.md b/skills/agent-desktop/SKILL.md index bdae74b..5316468 100644 --- a/skills/agent-desktop/SKILL.md +++ b/skills/agent-desktop/SKILL.md @@ -153,7 +153,7 @@ agent-desktop list-surfaces --app "App" # Available surfaces agent-desktop click @e5 --snapshot # AX-first click, no cursor move by default agent-desktop double-click @s8f3k2p9:e3 # AXOpen; physical double-click uses --headed mouse-click --count 2 agent-desktop triple-click @s8f3k2p9:e2 # Physical triple-click uses mouse-click --count 3 -agent-desktop right-click @s8f3k2p9:e5 # Right-click; menu returned when verified +agent-desktop right-click @s8f3k2p9:e5 # Right-click; inspect the resulting menu/effect separately agent-desktop type @e2 --snapshot "hello" # Headless AX text insertion when supported agent-desktop set-value @s8f3k2p9:e2 "new value" # Set value directly agent-desktop clear @s8f3k2p9:e2 # Clear element value diff --git a/skills/agent-desktop/references/commands-interaction.md b/skills/agent-desktop/references/commands-interaction.md index 907df88..63a5740 100644 --- a/skills/agent-desktop/references/commands-interaction.md +++ b/skills/agent-desktop/references/commands-interaction.md @@ -94,7 +94,7 @@ Triple-click requires cursor/focus side effects and is blocked in headless mode; ```bash agent-desktop right-click @s8f3k2p9:e5 ``` -Performs a semantic right-click/context-menu action and includes `menu` plus `menu_snapshot_id` when a menu surface can be verified. If the right-click action succeeds but menu probing fails, the command still returns the action result with `menu_probe.ok: false` so callers do not retry and double-open context menus. Combo boxes and menu buttons expose menu-opening actions for their primary dropdown; use `select` for those controls, not `right-click`. Focus-stealing and coordinate right-click fallback are blocked in headless mode; pass `--headed` to allow them. +Performs a semantic right-click/context-menu action. On macOS, `AXShowMenu` can return `APP_UNRESPONSIVE` with `delivery: delivery_uncertain` and `retry: unsafe` after opening a modal context menu; inspect the resulting menu or target effect before deciding what to do, and never retry that outcome blindly. Combo boxes and menu buttons expose menu-opening actions for their primary dropdown; use `select` for those controls, not `right-click`. Focus-stealing and coordinate right-click fallback are blocked in headless mode; pass `--headed` to allow them. ## Text Input diff --git a/skills/agent-desktop/references/macos.md b/skills/agent-desktop/references/macos.md index 3a1db01..4f4da39 100644 --- a/skills/agent-desktop/references/macos.md +++ b/skills/agent-desktop/references/macos.md @@ -69,9 +69,9 @@ When you run `click @ref`, agent-desktop doesn't just do a simple click. It runs 11. **Ancestor activation** — try pressing ancestor elements 12. **Explicit physical path** — coordinate click only when the caller selected a policy that allows focus stealing and cursor movement -For `right-click`, AXShowMenu and related semantic menu paths include a `menu` tree only after a real menu surface appears. If the action succeeds but the menu probe cannot verify a surface, the command still returns success with `menu_probe.ok: false`; callers should inspect that field instead of retrying blindly. Combo boxes and menu buttons use the same AX menu mechanism for their primary dropdown; use `select` for those controls. Coordinate right-click is blocked by the default headless policy. +For `right-click`, `AXShowMenu` may enter modal menu tracking and return `kAXErrorCannotComplete` after the menu opened. The command reports this as `APP_UNRESPONSIVE` with `delivery: delivery_uncertain` and `retry: unsafe`; inspect the resulting menu or target effect instead of retrying blindly. Combo boxes and menu buttons use the same AX menu mechanism for their primary dropdown; use `select` for those controls. Coordinate right-click is blocked by the default headless policy. -Menu verification intentionally requires a closed-to-open transition. If a menu is already open for the target app, `right-click` and `select` refuse to treat that existing menu as proof of success; dismiss the old menu and retry. This avoids acting on a stale sibling menu opened by a prior command. +Menu verification for `select` intentionally requires a closed-to-open transition. If a menu is already open for the target app, dismiss it before selecting so an existing sibling menu is not treated as proof of success. The default activation-chain deadline is 10 seconds. Set `AGENT_DESKTOP_CHAIN_TIMEOUT_MS` to a positive millisecond value when diagnosing unusually slow AX targets; values are capped at 300000 ms. Menu verification waits use `AGENT_DESKTOP_MENU_TIMEOUT_MS` with a default of 750 ms and a 10000 ms cap. Toggle verification uses `AGENT_DESKTOP_TOGGLE_TIMEOUT_MS` with a default of 600 ms and a 10000 ms cap; changed toggle values must remain stable for `AGENT_DESKTOP_TOGGLE_STABLE_MS`, default 200 ms and cap 2000 ms. @@ -94,7 +94,7 @@ macOS apps can have multiple accessibility surfaces: |---------|-------------|-------------| | `window` | Main application window (default) | General UI interaction | | `focused` | Currently focused element's context | Inspecting active element | -| `menu` | Open dropdown or context menu | After `select`, verified `right-click`, or explicit menu trigger | +| `menu` | Open dropdown or context menu | After `select`, `right-click`, or an explicit menu trigger | | `menubar` | Application menu bar | Navigating File/Edit/View menus | | `sheet` | Modal sheet (Save dialog, etc.) | After triggering sheet dialogs | | `popover` | Popover/popup content | Inspecting tooltips, popovers | @@ -237,7 +237,7 @@ Large apps (Xcode, Safari with many tabs) can have deep trees. ### Context Menu Doesn't Appear -After `right-click @ref`, inspect `menu` first. If it is absent and `menu_probe.ok` is `false`: +After `right-click @ref`, inspect the open menu or the target effect. If macOS returns `APP_UNRESPONSIVE` for `AXShowMenu` with unsafe retry semantics: 1. The element may not support context menus 2. If the target is a combo box or menu button, use `select @ref "Option"` instead 3. Run `list-surfaces --app "App"` to confirm whether a menu surface exists diff --git a/tests/e2e/scenarios-surfaces.sh b/tests/e2e/scenarios-surfaces.sh index 44c6df4..e1c6ade 100644 --- a/tests/e2e/scenarios-surfaces.sh +++ b/tests/e2e/scenarios-surfaces.sh @@ -24,14 +24,20 @@ act_target "$context_target" scroll-to >/dev/null 2>&1 require_target context_target button context-target context_output="$(act_target "$context_target" right-click 2>&1)" sleep 0.5 -context_menu="$(json_field "$context_output" data.menu)" require_target context_choice menuitem context-choice act_target "$context_choice" click >/dev/null 2>&1 sleep 0.4 require_value context_after right-status -assert "right-click returns a verifiable context-menu snapshot" \ - "$([ "$(json_field "$context_output" ok)" = "True" ] && [ -n "$context_menu" ] && echo 1 || echo 0)" \ - "menu_snapshot=$(json_field "$context_output" data.menu_snapshot_id)" +context_ok="$(json_field "$context_output" ok)" +context_code="$(json_field "$context_output" error.code)" +context_operation="$(json_field "$context_output" error.details.operation)" +context_delivery="$(json_field "$context_output" error.disposition.delivery)" +context_retry="$(json_field "$context_output" error.disposition.retry)" +assert "right-click reports success or exact macOS modal delivery uncertainty" \ + "$([ "$context_ok" = "True" ] || \ + { [ "$context_code" = "APP_UNRESPONSIVE" ] && [ "$context_operation" = "AXShowMenu" ] && \ + [ "$context_delivery" = "delivery_uncertain" ] && [ "$context_retry" = "unsafe" ]; } && echo 1 || echo 0)" \ + "ok=$context_ok code=$context_code operation=$context_operation delivery=$context_delivery retry=$context_retry" assert "context-menu item action is independently observed" \ "$([ "$context_after" = "context-picked" ] && echo 1 || echo 0)" \ "before=$context_before after=$context_after" @@ -73,9 +79,17 @@ 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" ] && echo 1 || echo 0 + [ "$(json_field "$out" ok)" = "True" ] && \ + [ -n "$(json_field "$out" data.match)" ] && echo 1 || echo 0 } require_target disclosure disclosure disclosure-section +precondition_output="$(act_target "$disclosure" click 2>&1)" +sleep 0.4 +expanded_before="$(disclosed_present)" +assert "disclosure precondition is independently expanded" \ + "$([ "$(json_field "$precondition_output" ok)" = "True" ] && [ "$expanded_before" = "1" ] && echo 1 || echo 0)" \ + "content_present=$expanded_before error=$(json_field "$precondition_output" error.code)" +require_target disclosure disclosure disclosure-section collapse_output="$(act_target "$disclosure" collapse 2>&1)" sleep 0.4 collapsed_hidden="$(disclosed_present)" diff --git a/tests/e2e/scenarios-trace-performance.sh b/tests/e2e/scenarios-trace-performance.sh index 4d8d04f..901928c 100644 --- a/tests/e2e/scenarios-trace-performance.sh +++ b/tests/e2e/scenarios-trace-performance.sh @@ -23,12 +23,21 @@ if [ -z "$trace_session" ]; then abort_suite "trace session did not return a session_id" fi "$bin" --session "$trace_session" snapshot --app "$app" -i >/dev/null 2>&1 -require_target trace_primary button primary-button -"$bin" --session "$trace_session" click "$(target_ref "$trace_primary")" \ - --snapshot "$(target_snapshot "$trace_primary")" >/dev/null 2>&1 -require_target trace_text textfield text-input -"$bin" --session "$trace_session" type "$(target_ref "$trace_text")" \ - --snapshot "$(target_snapshot "$trace_text")" trace-e2e >/dev/null 2>&1 +trace_target() { + local role="$1" name="$2" output + output="$("$bin" --session "$trace_session" find --app "$app" \ + --role "$role" --name "$name" --first 2>/dev/null)" + printf '%s' "$output" | python3 "$json_tool" target 2>/dev/null +} +trace_primary="$(trace_target button primary-button)" +trace_text="$(trace_target textfield text-input)" +if [ -z "$trace_primary" ] || [ -z "$trace_text" ]; then + abort_suite "trace session targets are missing" +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")" \ + --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)" trace_events="$(printf '%s' "$trace_show" | python3 -c ' @@ -47,8 +56,10 @@ if [ -d "$trace_dir/screens" ]; then fi assert "trace show includes command and snapshot events" "$trace_events" \ "session=$trace_session" -assert "trace screenshot artifacts have PNG magic" "$png_magic" \ - "directory=$trace_dir/screens exists=$([ -d "$trace_dir/screens" ] && echo yes || echo no) pngs=$(find "$trace_dir/screens" -name '*.png' 2>/dev/null | wc -l | tr -d ' ')" +trace_artifacts_ok="$([ "$(json_field "$trace_click" ok)" = "True" ] && \ + [ "$(json_field "$trace_session_type" ok)" = "True" ] && [ "$png_magic" = "1" ] && echo 1 || echo 0)" +assert "trace screenshot artifacts have PNG magic" "$trace_artifacts_ok" \ + "click_ok=$(json_field "$trace_click" ok) type_ok=$(json_field "$trace_session_type" ok) directory=$trace_dir/screens exists=$([ -d "$trace_dir/screens" ] && echo yes || echo no) pngs=$(find "$trace_dir/screens" -name '*.png' 2>/dev/null | wc -l | tr -d ' ')" trace_html="$(mktemp -t agentdesk-e2e-trace-export.XXXXXX.html)" cleanup_files+=("$trace_html") diff --git a/tests/e2e/test_harness_contract.py b/tests/e2e/test_harness_contract.py index 22789f2..07ea9ea 100644 --- a/tests/e2e/test_harness_contract.py +++ b/tests/e2e/test_harness_contract.py @@ -27,7 +27,7 @@ class HarnessContractTests(unittest.TestCase): self.assertEqual(len(exits), 1, exits) self.assertEqual(exits[0][0], "lib.sh") - self.assertEqual(len(abort_calls), 10, abort_calls) + self.assertEqual(len(abort_calls), 11, abort_calls) source = (E2E_ROOT / "lib.sh").read_text() abort = re.search(r"abort_suite\(\) \{(?P.*?)\n\}", source, re.DOTALL) self.assertIsNotNone(abort) @@ -68,6 +68,13 @@ class HarnessContractTests(unittest.TestCase): self.assertIn('--role statictext --native-id "$name" --first', source) + def test_trace_artifact_actions_use_the_session_that_created_their_refs(self): + source = (E2E_ROOT / "scenarios-trace-performance.sh").read_text() + + 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) + def test_sheet_scenarios_scroll_the_button_before_clicking(self): fixtures = [ (