From 3bed49dafb2798ef6657d3b1d8c4327252dfdfe5 Mon Sep 17 00:00:00 2001 From: Lahfir Date: Thu, 2 Jul 2026 19:43:43 -0700 Subject: [PATCH] feat: add typed delivery tier and verified flag to action steps Co-authored-by: Cursor --- crates/core/src/action_step.rs | 33 +++++++ crates/core/src/action_step_tests.rs | 47 ++++++++++ crates/core/src/lib.rs | 2 + crates/core/src/step_mechanism.rs | 8 ++ crates/ffi/cbindgen.toml | 5 ++ crates/ffi/include/agent_desktop.h | 25 +++++- crates/ffi/src/actions/result.rs | 58 ++++++++++-- crates/ffi/src/lib.rs | 2 +- crates/ffi/src/types/action_step.rs | 38 +++++++- crates/ffi/src/types/mod.rs | 2 +- crates/ffi/tests/c_abi_actions.rs | 15 ++++ crates/ffi/tests/c_abi_layout.rs | 19 +++- crates/ffi/tests/c_header_compile.rs | 4 + crates/macos/src/actions/chain.rs | 117 +++++++++++++++++++++++-- crates/macos/src/actions/chain_defs.rs | 23 +++-- crates/macos/src/actions/dispatch.rs | 4 +- 16 files changed, 377 insertions(+), 25 deletions(-) create mode 100644 crates/core/src/action_step_tests.rs create mode 100644 crates/core/src/step_mechanism.rs diff --git a/crates/core/src/action_step.rs b/crates/core/src/action_step.rs index f2c22490..6cede83a 100644 --- a/crates/core/src/action_step.rs +++ b/crates/core/src/action_step.rs @@ -1,10 +1,15 @@ use crate::action_step_outcome::ActionStepOutcome; +use crate::step_mechanism::StepMechanism; use serde::{Deserialize, Serialize}; #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ActionStep { label: String, pub outcome: ActionStepOutcome, + #[serde(skip_serializing_if = "Option::is_none", default)] + pub mechanism: Option, + #[serde(skip_serializing_if = "Option::is_none", default)] + pub verified: Option, } impl ActionStep { @@ -12,6 +17,8 @@ impl ActionStep { Self { label: label.to_string(), outcome: ActionStepOutcome::Attempted, + mechanism: None, + verified: None, } } @@ -19,6 +26,8 @@ impl ActionStep { Self { label: label.to_string(), outcome: ActionStepOutcome::Skipped, + mechanism: None, + verified: None, } } @@ -26,10 +35,34 @@ impl ActionStep { Self { label: label.to_string(), outcome: ActionStepOutcome::Succeeded, + mechanism: None, + verified: None, } } pub fn label(&self) -> &str { &self.label } + + pub fn mechanism(&self) -> Option { + self.mechanism + } + + pub fn verified(&self) -> Option { + self.verified + } + + pub fn with_mechanism(mut self, mechanism: StepMechanism) -> Self { + self.mechanism = Some(mechanism); + self + } + + pub fn with_verified(mut self, verified: bool) -> Self { + self.verified = Some(verified); + self + } } + +#[cfg(test)] +#[path = "action_step_tests.rs"] +mod tests; diff --git a/crates/core/src/action_step_tests.rs b/crates/core/src/action_step_tests.rs new file mode 100644 index 00000000..961b7824 --- /dev/null +++ b/crates/core/src/action_step_tests.rs @@ -0,0 +1,47 @@ +use super::ActionStep; +use crate::step_mechanism::StepMechanism; +use crate::trace_sanitize::sanitize_trace_value; +use serde_json::json; + +#[test] +fn legacy_action_step_json_round_trips_without_new_fields() { + let legacy = r#"{"label":"AXPress","outcome":"succeeded"}"#; + let step: ActionStep = serde_json::from_str(legacy).unwrap(); + assert_eq!(step.label(), "AXPress"); + assert!(step.mechanism().is_none()); + assert!(step.verified().is_none()); + let round_trip = serde_json::to_string(&step).unwrap(); + assert_eq!(round_trip, legacy); +} + +#[test] +fn action_step_serializes_mechanism_and_verified() { + let step = ActionStep::succeeded("verified_press") + .with_mechanism(StepMechanism::SemanticApi) + .with_verified(true); + let value = serde_json::to_value(&step).unwrap(); + assert_eq!(value["mechanism"], "semantic_api"); + assert_eq!(value["verified"], true); +} + +#[test] +fn action_step_omits_absent_mechanism_and_verified() { + let step = ActionStep::skipped("AXConfirm"); + let value = serde_json::to_value(&step).unwrap(); + assert!(value.get("mechanism").is_none()); + assert!(value.get("verified").is_none()); +} + +#[test] +fn trace_preserves_mechanism_and_verified() { + let value = sanitize_trace_value(json!({ + "steps": [{ + "label": "AXPress", + "outcome": "succeeded", + "mechanism": "semantic_api", + "verified": true + }] + })); + assert_eq!(value["steps"][0]["mechanism"], "semantic_api"); + assert_eq!(value["steps"][0]["verified"], true); +} diff --git a/crates/core/src/lib.rs b/crates/core/src/lib.rs index 9fb07f81..04311972 100644 --- a/crates/core/src/lib.rs +++ b/crates/core/src/lib.rs @@ -38,6 +38,7 @@ pub mod snapshot; pub mod snapshot_ref; pub mod snapshot_surface; pub mod state; +pub mod step_mechanism; pub(crate) mod trace; pub(crate) mod trace_artifacts; pub mod trace_read; @@ -69,4 +70,5 @@ pub use permission_report::PermissionReport; pub use permission_state::PermissionState; pub use refs::{RefEntry, RefMap}; pub use refs_store::RefStore; +pub use step_mechanism::StepMechanism; pub use trace_sanitize::sanitize_trace_value; diff --git a/crates/core/src/step_mechanism.rs b/crates/core/src/step_mechanism.rs new file mode 100644 index 00000000..07c009b9 --- /dev/null +++ b/crates/core/src/step_mechanism.rs @@ -0,0 +1,8 @@ +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum StepMechanism { + SemanticApi, + PhysicalSynthetic, +} diff --git a/crates/ffi/cbindgen.toml b/crates/ffi/cbindgen.toml index a8a3f94e..693896f2 100644 --- a/crates/ffi/cbindgen.toml +++ b/crates/ffi/cbindgen.toml @@ -68,6 +68,10 @@ _Static_assert(sizeof(AdActionStep) == AD_ACTION_STEP_SIZE, "AdActionStep ABI si _Static_assert(_Alignof(AdActionStep) == 8, "AdActionStep ABI alignment changed"); _Static_assert(offsetof(AdActionStep, label) == 0, "AdActionStep.label offset changed"); _Static_assert(offsetof(AdActionStep, outcome) == 8, "AdActionStep.outcome offset changed"); +_Static_assert(offsetof(AdActionStep, mechanism) == 16, "AdActionStep.mechanism offset changed"); +_Static_assert(offsetof(AdActionStep, has_mechanism) == 20, "AdActionStep.has_mechanism offset changed"); +_Static_assert(offsetof(AdActionStep, verified) == 21, "AdActionStep.verified offset changed"); +_Static_assert(offsetof(AdActionStep, has_verified) == 22, "AdActionStep.has_verified offset changed"); _Static_assert(sizeof(AdActionResult) == AD_ACTION_RESULT_SIZE, "AdActionResult ABI size changed"); _Static_assert(_Alignof(AdActionResult) == 8, "AdActionResult ABI alignment changed"); _Static_assert(offsetof(AdActionResult, action) == 0, "AdActionResult.action offset changed"); @@ -103,6 +107,7 @@ include = [ "AdPolicyKind", "AdScreenshotKind", "AdSnapshotSurface", + "AdStepMechanism", "AdWindowOpKind", ] diff --git a/crates/ffi/include/agent_desktop.h b/crates/ffi/include/agent_desktop.h index 39c13127..ca76f3b8 100644 --- a/crates/ffi/include/agent_desktop.h +++ b/crates/ffi/include/agent_desktop.h @@ -68,7 +68,7 @@ #define AD_ACTION_RESULT_SIZE 40 -#define AD_ACTION_STEP_SIZE 16 +#define AD_ACTION_STEP_SIZE 32 #define AD_DRAG_PARAMS_SIZE 48 @@ -287,6 +287,20 @@ typedef enum AdSnapshotSurface AdSnapshotSurface; typedef int32_t AdSnapshotSurface; #endif // __STDC_VERSION__ >= 202311L +enum AdStepMechanism +#if __STDC_VERSION__ >= 202311L + : int32_t +#endif // __STDC_VERSION__ >= 202311L + { + AD_STEP_MECHANISM_SEMANTIC_API = 1, + AD_STEP_MECHANISM_PHYSICAL_SYNTHETIC = 2, +}; +#if __STDC_VERSION__ >= 202311L +typedef enum AdStepMechanism AdStepMechanism; +#else +typedef int32_t AdStepMechanism; +#endif // __STDC_VERSION__ >= 202311L + enum AdWindowOpKind #if __STDC_VERSION__ >= 202311L : int32_t @@ -427,6 +441,11 @@ typedef struct AdElementState { typedef struct AdActionStep { const char *label; const char *outcome; + int32_t mechanism; + bool has_mechanism; + bool verified; + bool has_verified; + uint64_t _reserved; } AdActionStep; typedef struct AdActionResult { @@ -1818,6 +1837,10 @@ _Static_assert(sizeof(AdActionStep) == AD_ACTION_STEP_SIZE, "AdActionStep ABI si _Static_assert(_Alignof(AdActionStep) == 8, "AdActionStep ABI alignment changed"); _Static_assert(offsetof(AdActionStep, label) == 0, "AdActionStep.label offset changed"); _Static_assert(offsetof(AdActionStep, outcome) == 8, "AdActionStep.outcome offset changed"); +_Static_assert(offsetof(AdActionStep, mechanism) == 16, "AdActionStep.mechanism offset changed"); +_Static_assert(offsetof(AdActionStep, has_mechanism) == 20, "AdActionStep.has_mechanism offset changed"); +_Static_assert(offsetof(AdActionStep, verified) == 21, "AdActionStep.verified offset changed"); +_Static_assert(offsetof(AdActionStep, has_verified) == 22, "AdActionStep.has_verified offset changed"); _Static_assert(sizeof(AdActionResult) == AD_ACTION_RESULT_SIZE, "AdActionResult ABI size changed"); _Static_assert(_Alignof(AdActionResult) == 8, "AdActionResult ABI alignment changed"); _Static_assert(offsetof(AdActionResult, action) == 0, "AdActionResult.action offset changed"); diff --git a/crates/ffi/src/actions/result.rs b/crates/ffi/src/actions/result.rs index 1941b5eb..60a74422 100644 --- a/crates/ffi/src/actions/result.rs +++ b/crates/ffi/src/actions/result.rs @@ -1,7 +1,11 @@ use crate::convert::string::{free_c_string, opt_string_to_c, string_to_c_lossy}; -use crate::types::{AdActionResult, AdElementState, action_step::AdActionStep}; +use crate::types::{ + AdActionResult, AdElementState, + action_step::{AdActionStep, AdStepMechanism}, +}; use agent_desktop_core::action_result::ActionResult as CoreActionResult; use agent_desktop_core::action_step_outcome::ActionStepOutcome; +use agent_desktop_core::step_mechanism::StepMechanism; use std::ptr; pub(crate) fn action_result_to_c(r: &CoreActionResult) -> AdActionResult { @@ -84,9 +88,18 @@ fn action_steps_to_c(r: &CoreActionResult) -> *mut AdActionStep { let mut steps = r .steps .iter() - .map(|step| AdActionStep { - label: string_to_c_lossy(step.label()), - outcome: string_to_c_lossy(step_outcome_name(&step.outcome)), + .map(|step| { + let mechanism = step.mechanism().map(core_mechanism_to_c); + let verified = step.verified(); + AdActionStep { + label: string_to_c_lossy(step.label()), + outcome: string_to_c_lossy(step_outcome_name(&step.outcome)), + mechanism: mechanism.unwrap_or(AdStepMechanism::SemanticApi) as i32, + has_mechanism: mechanism.is_some(), + verified: verified.unwrap_or(false), + has_verified: verified.is_some(), + _reserved: 0, + } }) .collect::>(); steps.push(step_sentinel()); @@ -108,6 +121,18 @@ fn step_sentinel() -> AdActionStep { AdActionStep { label: ptr::null(), outcome: ptr::null(), + mechanism: 0, + has_mechanism: false, + verified: false, + has_verified: false, + _reserved: 0, + } +} + +fn core_mechanism_to_c(mechanism: StepMechanism) -> AdStepMechanism { + match mechanism { + StepMechanism::SemanticApi => AdStepMechanism::SemanticApi, + StepMechanism::PhysicalSynthetic => AdStepMechanism::PhysicalSynthetic, } } @@ -194,6 +219,11 @@ mod tests { AdActionStep { label: crate::convert::string::string_to_c_lossy("AXPress"), outcome: crate::convert::string::string_to_c_lossy("succeeded"), + mechanism: AdStepMechanism::SemanticApi as i32, + has_mechanism: true, + verified: false, + has_verified: false, + _reserved: 0, }, step_sentinel(), ] @@ -216,8 +246,11 @@ mod tests { #[test] fn action_result_to_c_preserves_steps() { let core_result = CoreActionResult::new("click").with_steps(vec![ - agent_desktop_core::action_step::ActionStep::attempted("AXScrollToVisible"), - agent_desktop_core::action_step::ActionStep::succeeded("AXPress"), + agent_desktop_core::action_step::ActionStep::attempted("AXScrollToVisible") + .with_mechanism(StepMechanism::SemanticApi), + agent_desktop_core::action_step::ActionStep::succeeded("AXPress") + .with_mechanism(StepMechanism::SemanticApi) + .with_verified(true), ]); let mut c_result = action_result_to_c(&core_result); @@ -233,6 +266,12 @@ mod tests { c_to_string((*c_result.steps.add(0)).outcome).as_deref(), Some("attempted") ); + assert!((*c_result.steps.add(0)).has_mechanism); + assert_eq!( + (*c_result.steps.add(0)).mechanism, + AdStepMechanism::SemanticApi as i32 + ); + assert!(!(*c_result.steps.add(0)).has_verified); assert_eq!( c_to_string((*c_result.steps.add(1)).label).as_deref(), Some("AXPress") @@ -241,6 +280,13 @@ mod tests { c_to_string((*c_result.steps.add(1)).outcome).as_deref(), Some("succeeded") ); + assert!((*c_result.steps.add(1)).has_mechanism); + assert_eq!( + (*c_result.steps.add(1)).mechanism, + AdStepMechanism::SemanticApi as i32 + ); + assert!((*c_result.steps.add(1)).has_verified); + assert!((*c_result.steps.add(1)).verified); } unsafe { ad_free_action_result(&mut c_result) }; diff --git a/crates/ffi/src/lib.rs b/crates/ffi/src/lib.rs index 4db892a1..43641731 100644 --- a/crates/ffi/src/lib.rs +++ b/crates/ffi/src/lib.rs @@ -69,7 +69,7 @@ pub use error::AdResult; pub use types::action::AdAction; pub use types::action_kind::AdActionKind; pub use types::action_result::AdActionResult; -pub use types::action_step::AdActionStep; +pub use types::action_step::{AdActionStep, AdStepMechanism}; pub use types::app_info::AdAppInfo; pub use types::app_list::AdAppList; pub use types::direction::AdDirection; diff --git a/crates/ffi/src/types/action_step.rs b/crates/ffi/src/types/action_step.rs index d11dc90b..ef218052 100644 --- a/crates/ffi/src/types/action_step.rs +++ b/crates/ffi/src/types/action_step.rs @@ -1,12 +1,24 @@ use std::os::raw::c_char; +#[repr(i32)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum AdStepMechanism { + SemanticApi = 1, + PhysicalSynthetic = 2, +} + #[repr(C)] pub struct AdActionStep { pub label: *const c_char, pub outcome: *const c_char, + pub mechanism: i32, + pub has_mechanism: bool, + pub verified: bool, + pub has_verified: bool, + pub _reserved: u64, } -pub const AD_ACTION_STEP_SIZE: usize = 16; +pub const AD_ACTION_STEP_SIZE: usize = 32; const _: () = assert!(std::mem::size_of::() == AD_ACTION_STEP_SIZE); @@ -14,3 +26,27 @@ const _: () = assert!(std::mem::size_of::() == AD_ACTION_STEP_SIZE pub extern "C" fn ad_action_step_size() -> usize { std::mem::size_of::() } + +#[cfg(test)] +mod tests { + use super::*; + use std::mem::{offset_of, size_of}; + + #[test] + fn discriminants_are_abi_stable() { + assert_eq!(AdStepMechanism::SemanticApi as i32, 1); + assert_eq!(AdStepMechanism::PhysicalSynthetic as i32, 2); + } + + #[test] + fn layout_matches_published_abi() { + assert_eq!(size_of::(), AD_ACTION_STEP_SIZE); + assert_eq!(offset_of!(AdActionStep, label), 0); + assert_eq!(offset_of!(AdActionStep, outcome), 8); + assert_eq!(offset_of!(AdActionStep, mechanism), 16); + assert_eq!(offset_of!(AdActionStep, has_mechanism), 20); + assert_eq!(offset_of!(AdActionStep, verified), 21); + assert_eq!(offset_of!(AdActionStep, has_verified), 22); + assert_eq!(offset_of!(AdActionStep, _reserved), 24); + } +} diff --git a/crates/ffi/src/types/mod.rs b/crates/ffi/src/types/mod.rs index 515c76f6..05a6293f 100644 --- a/crates/ffi/src/types/mod.rs +++ b/crates/ffi/src/types/mod.rs @@ -41,7 +41,7 @@ pub mod window_op_kind; pub use action::AdAction; pub use action_kind::AdActionKind; pub use action_result::AdActionResult; -pub use action_step::AdActionStep; +pub use action_step::{AdActionStep, AdStepMechanism}; pub use app_info::AdAppInfo; pub use app_list::AdAppList; pub use direction::AdDirection; diff --git a/crates/ffi/tests/c_abi_actions.rs b/crates/ffi/tests/c_abi_actions.rs index 4a9909a5..4a1936a8 100644 --- a/crates/ffi/tests/c_abi_actions.rs +++ b/crates/ffi/tests/c_abi_actions.rs @@ -157,14 +157,29 @@ fn free_action_result_releases_non_empty_steps_array() { AdActionStep { label: CString::new("AXScrollToVisible").unwrap().into_raw(), outcome: CString::new("attempted").unwrap().into_raw(), + mechanism: 1, + has_mechanism: true, + verified: false, + has_verified: false, + _reserved: 0, }, AdActionStep { label: CString::new("AXPress").unwrap().into_raw(), outcome: CString::new("succeeded").unwrap().into_raw(), + mechanism: 1, + has_mechanism: true, + verified: true, + has_verified: true, + _reserved: 0, }, AdActionStep { label: std::ptr::null(), outcome: std::ptr::null(), + mechanism: 0, + has_mechanism: false, + verified: false, + has_verified: false, + _reserved: 0, }, ] .into_boxed_slice(); diff --git a/crates/ffi/tests/c_abi_layout.rs b/crates/ffi/tests/c_abi_layout.rs index eec240f3..0bc68b9b 100644 --- a/crates/ffi/tests/c_abi_layout.rs +++ b/crates/ffi/tests/c_abi_layout.rs @@ -45,7 +45,7 @@ fn action_result_layout_is_guarded_for_c_consumers() { ); assert_eq!( agent_desktop_ffi::types::action_step::AD_ACTION_STEP_SIZE, - 16 + 32 ); assert_eq!( unsafe { common::ad_action_step_size() }, @@ -65,6 +65,23 @@ fn action_result_layout_is_guarded_for_c_consumers() { assert_eq!(offset_of!(AdActionResult, step_count), 32); assert_eq!(offset_of!(AdActionStep, label), 0); assert_eq!(offset_of!(AdActionStep, outcome), 8); + assert_eq!(offset_of!(AdActionStep, mechanism), 16); + assert_eq!(offset_of!(AdActionStep, has_mechanism), 20); + assert_eq!(offset_of!(AdActionStep, verified), 21); + assert_eq!(offset_of!(AdActionStep, has_verified), 22); + assert_eq!(offset_of!(AdActionStep, _reserved), 24); + + let copied = unsafe { + let step = MaybeUninit::::zeroed().assume_init(); + std::ptr::read(&step as *const AdActionStep) + }; + assert!(copied.label.is_null()); + assert!(copied.outcome.is_null()); + assert_eq!(copied.mechanism, 0); + assert!(!copied.has_mechanism); + assert!(!copied.verified); + assert!(!copied.has_verified); + assert_eq!(copied._reserved, 0); } #[test] diff --git a/crates/ffi/tests/c_header_compile.rs b/crates/ffi/tests/c_header_compile.rs index 2c8ded58..9746f87c 100644 --- a/crates/ffi/tests/c_header_compile.rs +++ b/crates/ffi/tests/c_header_compile.rs @@ -67,6 +67,10 @@ int main(void) { _Static_assert(offsetof(AdActionResult, steps) == 24, "AdActionResult.steps offset changed"); _Static_assert(offsetof(AdActionResult, step_count) == 32, "AdActionResult.step_count offset changed"); _Static_assert(offsetof(AdActionStep, outcome) == 8, "AdActionStep.outcome offset changed"); + _Static_assert(offsetof(AdActionStep, mechanism) == 16, "AdActionStep.mechanism offset changed"); + _Static_assert(offsetof(AdActionStep, has_mechanism) == 20, "AdActionStep.has_mechanism offset changed"); + _Static_assert(offsetof(AdActionStep, verified) == 21, "AdActionStep.verified offset changed"); + _Static_assert(offsetof(AdActionStep, has_verified) == 22, "AdActionStep.has_verified offset changed"); _Static_assert(AD_ELEMENT_STATE_SIZE == sizeof(AdElementState), "AdElementState size macro drifted"); (void)ad_action_step_size; (void)ad_ref_entry_size; diff --git a/crates/macos/src/actions/chain.rs b/crates/macos/src/actions/chain.rs index ebe33f52..b2a88bdf 100644 --- a/crates/macos/src/actions/chain.rs +++ b/crates/macos/src/actions/chain.rs @@ -1,4 +1,6 @@ +use agent_desktop_core::action_step_outcome::ActionStepOutcome; use agent_desktop_core::error::{AdapterError, ErrorCode}; +use agent_desktop_core::step_mechanism::StepMechanism; use agent_desktop_core::{action_step::ActionStep, interaction_policy::InteractionPolicy}; use crate::actions::discovery::ElementCaps; @@ -42,7 +44,10 @@ mod imp { if def.pre_scroll { tracing::debug!("chain: pre-scroll AXScrollToVisible"); ax_helpers::ensure_visible(el); - steps.push(ActionStep::attempted("AXScrollToVisible")); + steps.push( + ActionStep::attempted("AXScrollToVisible") + .with_mechanism(StepMechanism::SemanticApi), + ); } for (i, step) in def.steps.iter().enumerate() { @@ -53,11 +58,10 @@ mod imp { .iter() .find(|s| matches!(s, ChainStep::CGClick { .. })) { - let label = step_label(cg); if physical_click_permitted(policy) && execute_step(el, caps, cg, &ctx, policy)? { tracing::debug!("chain: CGClick fallback succeeded"); - steps.push(ActionStep::succeeded(label)); + steps.push(build_step(cg, ActionStepOutcome::Succeeded)); return Ok(steps); } } @@ -76,11 +80,11 @@ mod imp { let label = step_label(step); if execute_step(el, caps, step, &ctx, policy)? { tracing::debug!("chain: [{}/{}] {} -> success", i + 1, total, label); - steps.push(ActionStep::succeeded(label)); + steps.push(build_step(step, ActionStepOutcome::Succeeded)); return Ok(steps); } tracing::debug!("chain: [{}/{}] {} -> skip", i + 1, total, label); - steps.push(ActionStep::skipped(label)); + steps.push(build_step(step, ActionStepOutcome::Skipped)); } tracing::debug!("chain: all {total} steps exhausted"); @@ -90,6 +94,46 @@ mod imp { ) } + fn step_mechanism(step: &ChainStep) -> StepMechanism { + match step { + ChainStep::CGClick { .. } | ChainStep::FocusThenClearByKeyboard => { + StepMechanism::PhysicalSynthetic + } + _ => StepMechanism::SemanticApi, + } + } + + fn step_verifies_effect(step: &ChainStep) -> bool { + match step { + ChainStep::SetBool { .. } + | ChainStep::SetDynamic { .. } + | ChainStep::FocusThenSetDynamic { .. } + | ChainStep::IncrementToDynamic => true, + ChainStep::Custom { label, .. } => matches!( + *label, + "verified_press" | "value_relay" | "visible_in_scroll_context" + ), + ChainStep::CustomWithDeadline { label, .. } => { + matches!(*label, "expand_verified" | "collapse_verified") + } + _ => false, + } + } + + fn build_step(step: &ChainStep, outcome: ActionStepOutcome) -> ActionStep { + let label = step_label(step); + let mut built = match outcome { + ActionStepOutcome::Attempted => ActionStep::attempted(label), + ActionStepOutcome::Skipped => ActionStep::skipped(label), + ActionStepOutcome::Succeeded => ActionStep::succeeded(label), + }; + built = built.with_mechanism(step_mechanism(step)); + if matches!(outcome, ActionStepOutcome::Succeeded) && step_verifies_effect(step) { + built = built.with_verified(true); + } + built + } + fn step_label(step: &ChainStep) -> &'static str { match step { ChainStep::Action(name) => name, @@ -318,7 +362,10 @@ mod imp { #[cfg(test)] mod tests { - use super::finite_target; + use super::{ChainStep, build_step, finite_target, step_mechanism, step_verifies_effect}; + use agent_desktop_core::action::MouseButton; + use agent_desktop_core::action_step_outcome::ActionStepOutcome; + use agent_desktop_core::step_mechanism::StepMechanism; #[test] fn finite_target_rejects_non_finite_numbers() { @@ -328,6 +375,64 @@ mod imp { assert_eq!(finite_target("-inf"), None); assert_eq!(finite_target("not-a-number"), None); } + + #[test] + fn step_mechanism_tags_physical_for_cgclick_and_keyboard_clear() { + assert_eq!( + step_mechanism(&ChainStep::CGClick { + button: MouseButton::Left, + count: 1, + }), + StepMechanism::PhysicalSynthetic + ); + assert_eq!( + step_mechanism(&ChainStep::FocusThenClearByKeyboard), + StepMechanism::PhysicalSynthetic + ); + assert_eq!( + step_mechanism(&ChainStep::Action("AXPress")), + StepMechanism::SemanticApi + ); + } + + #[test] + fn step_verifies_effect_matches_verified_chain_steps() { + assert!(step_verifies_effect(&ChainStep::SetBool { + attr: "AXSelected", + value: true, + })); + assert!(step_verifies_effect(&ChainStep::Custom { + label: "verified_press", + func: |_| Ok(false), + })); + assert!(step_verifies_effect(&ChainStep::CustomWithDeadline { + label: "expand_verified", + func: |_, _| Ok(false), + })); + assert!(!step_verifies_effect(&ChainStep::Action("AXPress"))); + } + + #[test] + fn build_step_tags_mechanism_and_verified_on_success() { + let step = ChainStep::SetBool { + attr: "AXSelected", + value: true, + }; + let built = build_step(&step, ActionStepOutcome::Succeeded); + assert_eq!(built.mechanism(), Some(StepMechanism::SemanticApi)); + assert_eq!(built.verified(), Some(true)); + } + + #[test] + fn build_step_skipped_does_not_tag_verified() { + let step = ChainStep::SetBool { + attr: "AXSelected", + value: true, + }; + let built = build_step(&step, ActionStepOutcome::Skipped); + assert_eq!(built.mechanism(), Some(StepMechanism::SemanticApi)); + assert!(built.verified().is_none()); + } } } diff --git a/crates/macos/src/actions/chain_defs.rs b/crates/macos/src/actions/chain_defs.rs index c406cbb8..01679ffd 100644 --- a/crates/macos/src/actions/chain_defs.rs +++ b/crates/macos/src/actions/chain_defs.rs @@ -9,7 +9,10 @@ mod imp { chain_disclosure_steps, chain_menu_steps, chain_steps, }; use crate::tree::AXElement; - use agent_desktop_core::{action::MouseButton, interaction_policy::InteractionPolicy}; + use agent_desktop_core::{ + action::MouseButton, action_step::ActionStep, interaction_policy::InteractionPolicy, + step_mechanism::StepMechanism, + }; pub(crate) static CLICK_CHAIN: ChainDef = ChainDef { pre_scroll: true, @@ -183,11 +186,16 @@ mod imp { pub(crate) fn double_click( el: &AXElement, policy: InteractionPolicy, - ) -> Result<(), AdapterError> { + ) -> Result, AdapterError> { if ax_helpers::has_ax_action(el, "AXOpen") && ax_helpers::try_ax_action(el, "AXOpen") { - return Ok(()); + return Ok(vec![ + ActionStep::succeeded("AXOpen").with_mechanism(StepMechanism::SemanticApi), + ]); } - crate::actions::dispatch::click_via_bounds(el, MouseButton::Left, 2, policy) + crate::actions::dispatch::click_via_bounds(el, MouseButton::Left, 2, policy)?; + Ok(vec![ + ActionStep::succeeded("CGClick").with_mechanism(StepMechanism::PhysicalSynthetic), + ]) } /// Triple-click has no AX semantic equivalent on macOS and is therefore @@ -197,8 +205,11 @@ mod imp { pub(crate) fn triple_click( el: &AXElement, policy: InteractionPolicy, - ) -> Result<(), AdapterError> { - crate::actions::dispatch::click_via_bounds(el, MouseButton::Left, 3, policy) + ) -> Result, AdapterError> { + crate::actions::dispatch::click_via_bounds(el, MouseButton::Left, 3, policy)?; + Ok(vec![ + ActionStep::succeeded("CGClick").with_mechanism(StepMechanism::PhysicalSynthetic), + ]) } } diff --git a/crates/macos/src/actions/dispatch.rs b/crates/macos/src/actions/dispatch.rs index 9c8e8e48..4ba05d9b 100644 --- a/crates/macos/src/actions/dispatch.rs +++ b/crates/macos/src/actions/dispatch.rs @@ -88,7 +88,7 @@ mod imp { } Action::DoubleClick => { - chain_defs::double_click(el, request.policy)?; + steps.extend(chain_defs::double_click(el, request.policy)?); } Action::RightClick => { @@ -195,7 +195,7 @@ mod imp { } Action::TripleClick => { - chain_defs::triple_click(el, request.policy)?; + steps.extend(chain_defs::triple_click(el, request.policy)?); } Action::ScrollTo => {