diff --git a/README.md b/README.md index f4d50feb..0ed597b6 100644 --- a/README.md +++ b/README.md @@ -269,6 +269,11 @@ agent-desktop batch '[ {"command": "type", "args": {"ref_id": "@e5", "snapshot": "", "text": "hello"}}, {"command": "press", "args": {"combo": "return"}} ]' --stop-on-error + +agent-desktop --session run-a batch '[ + {"command": "snapshot", "args": {"app": "Finder", "interactive_only": true}}, + {"command": "status", "session": "run-b", "args": {}} +]' ``` ### System @@ -354,10 +359,19 @@ Interactive roles that receive refs: `button`, `textfield`, `checkbox`, `link`, Static elements (labels, groups, containers) appear in the tree for context but have no ref. +Reliability contract: + +- `--session ` scopes the latest snapshot pointer and refmap to one caller. +- Ref actions use strict re-identification and return `STALE_REF` instead of acting on a changed target. +- Multiple plausible targets return `AMBIGUOUS_TARGET` instead of choosing arbitrarily. +- Actions run an actionability preflight before dispatch: visibility, enabled state, supported action, policy, and editability. +- `wait --element @e3 --predicate actionable` polls until the target can be acted on. +- `--trace ` appends JSONL diagnostics outside stdout; add `--trace-strict` to fail if trace writing fails. + Stale ref recovery: ``` -snapshot → act → STALE_REF? → snapshot again → retry +snapshot → act → STALE_REF or AMBIGUOUS_TARGET? → wait/snapshot again → retry with the new ref ``` ## Platform Support diff --git a/crates/core/src/action.rs b/crates/core/src/action.rs index a5ea3754..4af2d731 100644 --- a/crates/core/src/action.rs +++ b/crates/core/src/action.rs @@ -26,6 +26,34 @@ pub enum Action { Drag(DragParams), } +impl Action { + pub fn name(&self) -> &'static str { + match self { + Self::Click => "click", + Self::DoubleClick => "double-click", + Self::RightClick => "right-click", + Self::TripleClick => "triple-click", + Self::SetValue(_) => "set-value", + Self::SetFocus => "focus", + Self::Expand => "expand", + Self::Collapse => "collapse", + Self::Select(_) => "select", + Self::Toggle => "toggle", + Self::Check => "check", + Self::Uncheck => "uncheck", + Self::Scroll(_, _) => "scroll", + Self::ScrollTo => "scroll-to", + Self::PressKey(_) => "press", + Self::KeyDown(_) => "key-down", + Self::KeyUp(_) => "key-up", + Self::TypeText(_) => "type", + Self::Clear => "clear", + Self::Hover => "hover", + Self::Drag(_) => "drag", + } + } +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ActionRequest { pub action: Action, diff --git a/crates/core/src/commands/batch.rs b/crates/core/src/commands/batch.rs index c778e4b7..3a792ece 100644 --- a/crates/core/src/commands/batch.rs +++ b/crates/core/src/commands/batch.rs @@ -10,6 +10,7 @@ pub struct BatchArgs { #[derive(Debug, Deserialize)] pub struct BatchCommand { pub command: String, + pub session: Option, #[serde(default)] pub args: Value, } diff --git a/crates/core/src/commands/check.rs b/crates/core/src/commands/check.rs index e682d6b9..1e13f418 100644 --- a/crates/core/src/commands/check.rs +++ b/crates/core/src/commands/check.rs @@ -1,11 +1,21 @@ use crate::{ action::{Action, ActionRequest}, adapter::PlatformAdapter, - commands::helpers::{RefArgs, execute_ref_action}, + commands::helpers::{RefArgs, execute_ref_action_with_context}, + context::CommandContext, error::AppError, }; use serde_json::Value; -pub fn execute(args: RefArgs, adapter: &dyn PlatformAdapter) -> Result { - execute_ref_action(args, adapter, ActionRequest::headless(Action::Check)) +pub fn execute( + args: RefArgs, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { + execute_ref_action_with_context( + args, + adapter, + ActionRequest::headless(Action::Check), + context, + ) } diff --git a/crates/core/src/commands/clear.rs b/crates/core/src/commands/clear.rs index dee80ee8..e57cc927 100644 --- a/crates/core/src/commands/clear.rs +++ b/crates/core/src/commands/clear.rs @@ -1,11 +1,21 @@ use crate::{ action::{Action, ActionRequest}, adapter::PlatformAdapter, - commands::helpers::{RefArgs, execute_ref_action}, + commands::helpers::{RefArgs, execute_ref_action_with_context}, + context::CommandContext, error::AppError, }; use serde_json::Value; -pub fn execute(args: RefArgs, adapter: &dyn PlatformAdapter) -> Result { - execute_ref_action(args, adapter, ActionRequest::headless(Action::Clear)) +pub fn execute( + args: RefArgs, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { + execute_ref_action_with_context( + args, + adapter, + ActionRequest::headless(Action::Clear), + context, + ) } diff --git a/crates/core/src/commands/click.rs b/crates/core/src/commands/click.rs index 7e26f3c3..562bb438 100644 --- a/crates/core/src/commands/click.rs +++ b/crates/core/src/commands/click.rs @@ -1,11 +1,21 @@ use crate::{ action::{Action, ActionRequest}, adapter::PlatformAdapter, - commands::helpers::{RefArgs, execute_ref_action}, + commands::helpers::{RefArgs, execute_ref_action_with_context}, + context::CommandContext, error::AppError, }; use serde_json::Value; -pub fn execute(args: RefArgs, adapter: &dyn PlatformAdapter) -> Result { - execute_ref_action(args, adapter, ActionRequest::headless(Action::Click)) +pub fn execute( + args: RefArgs, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { + execute_ref_action_with_context( + args, + adapter, + ActionRequest::headless(Action::Click), + context, + ) } diff --git a/crates/core/src/commands/collapse.rs b/crates/core/src/commands/collapse.rs index e0a23188..7461d0b3 100644 --- a/crates/core/src/commands/collapse.rs +++ b/crates/core/src/commands/collapse.rs @@ -1,11 +1,21 @@ use crate::{ action::{Action, ActionRequest}, adapter::PlatformAdapter, - commands::helpers::{RefArgs, execute_ref_action}, + commands::helpers::{RefArgs, execute_ref_action_with_context}, + context::CommandContext, error::AppError, }; use serde_json::Value; -pub fn execute(args: RefArgs, adapter: &dyn PlatformAdapter) -> Result { - execute_ref_action(args, adapter, ActionRequest::headless(Action::Collapse)) +pub fn execute( + args: RefArgs, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { + execute_ref_action_with_context( + args, + adapter, + ActionRequest::headless(Action::Collapse), + context, + ) } diff --git a/crates/core/src/commands/double_click.rs b/crates/core/src/commands/double_click.rs index 1e70bad7..69766522 100644 --- a/crates/core/src/commands/double_click.rs +++ b/crates/core/src/commands/double_click.rs @@ -1,11 +1,21 @@ use crate::{ action::{Action, ActionRequest}, adapter::PlatformAdapter, - commands::helpers::{RefArgs, execute_ref_action}, + commands::helpers::{RefArgs, execute_ref_action_with_context}, + context::CommandContext, error::AppError, }; use serde_json::Value; -pub fn execute(args: RefArgs, adapter: &dyn PlatformAdapter) -> Result { - execute_ref_action(args, adapter, ActionRequest::headless(Action::DoubleClick)) +pub fn execute( + args: RefArgs, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { + execute_ref_action_with_context( + args, + adapter, + ActionRequest::headless(Action::DoubleClick), + context, + ) } diff --git a/crates/core/src/commands/drag.rs b/crates/core/src/commands/drag.rs index 718f242c..85eef8d0 100644 --- a/crates/core/src/commands/drag.rs +++ b/crates/core/src/commands/drag.rs @@ -1,7 +1,8 @@ use crate::{ action::{DragParams, Point}, adapter::PlatformAdapter, - commands::helpers::resolve_point_from_ref_or_xy, + commands::helpers::resolve_point_from_ref_or_xy_with_context, + context::CommandContext, error::AppError, }; use serde_json::{Value, json}; @@ -15,13 +16,18 @@ pub struct DragArgs { pub duration_ms: Option, } -pub fn execute(args: DragArgs, adapter: &dyn PlatformAdapter) -> Result { +pub fn execute( + args: DragArgs, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { let from = resolve_point( &args.from_ref, args.from_xy, "from", args.snapshot_id.as_deref(), adapter, + context, )?; let to = resolve_point( &args.to_ref, @@ -29,6 +35,7 @@ pub fn execute(args: DragArgs, adapter: &dyn PlatformAdapter) -> Result, adapter: &dyn PlatformAdapter, + context: &CommandContext, ) -> Result { - resolve_point_from_ref_or_xy( + resolve_point_from_ref_or_xy_with_context( ref_id.as_deref(), xy, snapshot_id, adapter, format!("Provide --{label} or --{label}-xy x,y"), + context, ) } diff --git a/crates/core/src/commands/expand.rs b/crates/core/src/commands/expand.rs index 4d4a65f4..90f284b1 100644 --- a/crates/core/src/commands/expand.rs +++ b/crates/core/src/commands/expand.rs @@ -1,11 +1,21 @@ use crate::{ action::{Action, ActionRequest}, adapter::PlatformAdapter, - commands::helpers::{RefArgs, execute_ref_action}, + commands::helpers::{RefArgs, execute_ref_action_with_context}, + context::CommandContext, error::AppError, }; use serde_json::Value; -pub fn execute(args: RefArgs, adapter: &dyn PlatformAdapter) -> Result { - execute_ref_action(args, adapter, ActionRequest::headless(Action::Expand)) +pub fn execute( + args: RefArgs, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { + execute_ref_action_with_context( + args, + adapter, + ActionRequest::headless(Action::Expand), + context, + ) } diff --git a/crates/core/src/commands/find.rs b/crates/core/src/commands/find.rs index 4102ea33..e2f7ae92 100644 --- a/crates/core/src/commands/find.rs +++ b/crates/core/src/commands/find.rs @@ -1,5 +1,6 @@ use crate::{ - adapter::PlatformAdapter, error::AppError, node::AccessibilityNode, search_text, snapshot, + adapter::PlatformAdapter, context::CommandContext, error::AppError, node::AccessibilityNode, + search_text, snapshot, }; use serde_json::{Value, json}; @@ -18,13 +19,17 @@ pub struct FindArgs { pub limit: Option, } -pub fn execute(args: FindArgs, adapter: &dyn PlatformAdapter) -> Result { +pub fn execute( + args: FindArgs, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { validate_find_mode(&args)?; let opts = crate::adapter::TreeOptions::default(); let result = if args.count { snapshot::build(adapter, &opts, args.app.as_deref(), None)? } else { - snapshot::run(adapter, &opts, args.app.as_deref(), None)? + snapshot::run_with_context(adapter, &opts, args.app.as_deref(), None, context)? }; let query = FindQuery::from_args(&args); diff --git a/crates/core/src/commands/focus.rs b/crates/core/src/commands/focus.rs index d4c59d77..63670201 100644 --- a/crates/core/src/commands/focus.rs +++ b/crates/core/src/commands/focus.rs @@ -1,11 +1,21 @@ use crate::{ action::{Action, ActionRequest}, adapter::PlatformAdapter, - commands::helpers::{RefArgs, execute_ref_action}, + commands::helpers::{RefArgs, execute_ref_action_with_context}, + context::CommandContext, error::AppError, }; use serde_json::Value; -pub fn execute(args: RefArgs, adapter: &dyn PlatformAdapter) -> Result { - execute_ref_action(args, adapter, ActionRequest::headless(Action::SetFocus)) +pub fn execute( + args: RefArgs, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { + execute_ref_action_with_context( + args, + adapter, + ActionRequest::headless(Action::SetFocus), + context, + ) } diff --git a/crates/core/src/commands/get.rs b/crates/core/src/commands/get.rs index f3aec97a..ed0ad603 100644 --- a/crates/core/src/commands/get.rs +++ b/crates/core/src/commands/get.rs @@ -1,4 +1,7 @@ -use crate::{adapter::PlatformAdapter, commands::helpers::resolve_ref, error::AppError}; +use crate::{ + adapter::PlatformAdapter, commands::helpers::resolve_ref_with_context, context::CommandContext, + error::AppError, +}; use serde_json::{Value, json}; pub struct GetArgs { @@ -16,8 +19,13 @@ pub enum GetProperty { States, } -pub fn execute(args: GetArgs, adapter: &dyn PlatformAdapter) -> Result { - let (entry, handle) = resolve_ref(&args.ref_id, args.snapshot_id.as_deref(), adapter)?; +pub fn execute( + args: GetArgs, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { + let (entry, handle) = + resolve_ref_with_context(&args.ref_id, args.snapshot_id.as_deref(), adapter, context)?; let value = match args.property { GetProperty::Role => json!(entry.role), diff --git a/crates/core/src/commands/helpers.rs b/crates/core/src/commands/helpers.rs index 89042bd8..afb64308 100644 --- a/crates/core/src/commands/helpers.rs +++ b/crates/core/src/commands/helpers.rs @@ -1,6 +1,7 @@ use crate::{ action::{ActionRequest, Point, WindowOp}, adapter::{PlatformAdapter, WindowFilter}, + context::CommandContext, error::AppError, node::WindowInfo, refs::{RefEntry, validate_ref_id}, @@ -19,13 +20,27 @@ pub struct RefArgs { pub snapshot_id: Option, } +#[cfg(test)] pub(crate) fn resolve_ref<'a>( ref_id: &str, snapshot_id: Option<&str>, adapter: &'a dyn PlatformAdapter, +) -> Result<(RefEntry, ResolvedElement<'a>), AppError> { + resolve_ref_with_context(ref_id, snapshot_id, adapter, &CommandContext::default()) +} + +pub(crate) fn resolve_ref_with_context<'a>( + ref_id: &str, + snapshot_id: Option<&str>, + adapter: &'a dyn PlatformAdapter, + context: &CommandContext, ) -> Result<(RefEntry, ResolvedElement<'a>), AppError> { validate_ref_id(ref_id)?; - let store = RefStore::new()?; + let store = RefStore::for_session(context.session_id.as_deref())?; + context.trace( + "ref.resolve.start", + json!({ "ref": ref_id, "snapshot_id": snapshot_id }), + )?; let refmap = store.load(snapshot_id).map_err(|e| { tracing::debug!("refmap load failed: {e}"); AppError::stale_ref(ref_id) @@ -41,8 +56,18 @@ pub(crate) fn resolve_ref<'a>( entry.role, entry.name.as_deref().unwrap_or("(none)") ); + context.trace( + "ref.resolve.entry", + json!({ + "ref": ref_id, + "pid": entry.pid, + "role": entry.role, + "name": entry.name + }), + )?; let handle = adapter.resolve_element_strict(&entry)?; tracing::debug!("resolve: {} resolved successfully", ref_id); + context.trace("ref.resolve.ok", json!({ "ref": ref_id }))?; Ok((entry, ResolvedElement::new(adapter, handle))) } @@ -69,26 +94,47 @@ pub(crate) fn resolve_app_pid( } } +#[cfg(test)] pub(crate) fn execute_ref_action( args: RefArgs, adapter: &dyn PlatformAdapter, request: ActionRequest, ) -> Result { - let (_entry, handle) = resolve_ref(&args.ref_id, args.snapshot_id.as_deref(), adapter)?; + execute_ref_action_with_context(args, adapter, request, &CommandContext::default()) +} + +pub(crate) fn execute_ref_action_with_context( + args: RefArgs, + adapter: &dyn PlatformAdapter, + request: ActionRequest, + context: &CommandContext, +) -> Result { + let (_entry, handle) = + resolve_ref_with_context(&args.ref_id, args.snapshot_id.as_deref(), adapter, context)?; + context.trace( + "actionability.check.start", + json!({ "ref": args.ref_id, "action": request.action.name() }), + )?; crate::actionability::check(&_entry, &request)?; + context.trace( + "actionability.check.ok", + json!({ "ref": args.ref_id, "action": request.action.name() }), + )?; let result = adapter.execute_action(handle.handle(), request)?; + context.trace("action.dispatch.ok", json!({ "ref": args.ref_id }))?; Ok(serde_json::to_value(result)?) } -pub(crate) fn resolve_point_from_ref_or_xy( +pub(crate) fn resolve_point_from_ref_or_xy_with_context( ref_id: Option<&str>, xy: Option<(f64, f64)>, snapshot_id: Option<&str>, adapter: &dyn PlatformAdapter, missing_input_message: impl Into, + context: &CommandContext, ) -> Result { if let Some(ref_id) = ref_id { - let (_entry, handle) = resolve_ref(ref_id, snapshot_id, adapter)?; + let (_entry, handle) = resolve_ref_with_context(ref_id, snapshot_id, adapter, context)?; let bounds = adapter .get_element_bounds(handle.handle())? .ok_or_else(|| AppError::invalid_input(format!("Element {ref_id} has no bounds")))?; diff --git a/crates/core/src/commands/hover.rs b/crates/core/src/commands/hover.rs index 8d0d3879..efe27f21 100644 --- a/crates/core/src/commands/hover.rs +++ b/crates/core/src/commands/hover.rs @@ -1,7 +1,8 @@ use crate::{ action::{MouseButton, MouseEvent, MouseEventKind, Point}, adapter::PlatformAdapter, - commands::helpers::resolve_point_from_ref_or_xy, + commands::helpers::resolve_point_from_ref_or_xy_with_context, + context::CommandContext, error::AppError, }; use serde_json::{Value, json}; @@ -13,8 +14,12 @@ pub struct HoverArgs { pub duration_ms: Option, } -pub fn execute(args: HoverArgs, adapter: &dyn PlatformAdapter) -> Result { - let point = resolve_hover_point(&args, adapter)?; +pub fn execute( + args: HoverArgs, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { + let point = resolve_hover_point(&args, adapter, context)?; adapter.mouse_event(MouseEvent { kind: MouseEventKind::Move, point: point.clone(), @@ -26,12 +31,17 @@ pub fn execute(args: HoverArgs, adapter: &dyn PlatformAdapter) -> Result Result { - resolve_point_from_ref_or_xy( +fn resolve_hover_point( + args: &HoverArgs, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { + resolve_point_from_ref_or_xy_with_context( args.ref_id.as_deref(), args.xy, args.snapshot_id.as_deref(), adapter, "Provide a ref (@e1) or --xy x,y", + context, ) } diff --git a/crates/core/src/commands/is_check.rs b/crates/core/src/commands/is_check.rs index 0b7bc6ed..63c462e7 100644 --- a/crates/core/src/commands/is_check.rs +++ b/crates/core/src/commands/is_check.rs @@ -1,6 +1,6 @@ use crate::{ - action::ElementState, adapter::PlatformAdapter, commands::helpers::resolve_ref, - error::AppError, refs::RefEntry, + action::ElementState, adapter::PlatformAdapter, commands::helpers::resolve_ref_with_context, + context::CommandContext, error::AppError, refs::RefEntry, }; use serde_json::{Value, json}; @@ -20,7 +20,16 @@ pub enum IsProperty { /// State is read live when the platform supports it, then falls back to snapshot state. pub fn execute(args: IsArgs, adapter: &dyn PlatformAdapter) -> Result { - let (entry, handle) = resolve_ref(&args.ref_id, args.snapshot_id.as_deref(), adapter)?; + execute_with_context(args, adapter, &CommandContext::default()) +} + +pub fn execute_with_context( + args: IsArgs, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { + let (entry, handle) = + resolve_ref_with_context(&args.ref_id, args.snapshot_id.as_deref(), adapter, context)?; let state = adapter .get_live_state(handle.handle()) .ok() diff --git a/crates/core/src/commands/mod.rs b/crates/core/src/commands/mod.rs index 99877a73..227ef397 100644 --- a/crates/core/src/commands/mod.rs +++ b/crates/core/src/commands/mod.rs @@ -53,6 +53,7 @@ pub mod type_text; pub mod uncheck; pub mod version; pub mod wait; +pub(crate) mod wait_latest_ref_cache; pub(crate) mod wait_predicate; #[cfg(test)] diff --git a/crates/core/src/commands/ref_policy_tests.rs b/crates/core/src/commands/ref_policy_tests.rs index d2d41c03..eaa565aa 100644 --- a/crates/core/src/commands/ref_policy_tests.rs +++ b/crates/core/src/commands/ref_policy_tests.rs @@ -5,6 +5,7 @@ use crate::{ check, clear, click, collapse, double_click, expand, focus, helpers::RefArgs, right_click, scroll, scroll_to, select, set_value, toggle, triple_click, type_text, uncheck, }, + context::CommandContext, error::AdapterError, refs::{RefEntry, RefMap}, refs_store::RefStore, @@ -98,23 +99,24 @@ fn default_ref_commands_are_headless() { let _guard = HomeGuard::new(); let snapshot_id = snapshot_id(); let adapter = RecordingAdapter::new(); + let context = CommandContext::default(); - click::execute(ref_args(&snapshot_id), &adapter).unwrap(); - double_click::execute(ref_args(&snapshot_id), &adapter).unwrap(); - triple_click::execute(ref_args(&snapshot_id), &adapter).unwrap(); + click::execute(ref_args(&snapshot_id), &adapter, &context).unwrap(); + double_click::execute(ref_args(&snapshot_id), &adapter, &context).unwrap(); + triple_click::execute(ref_args(&snapshot_id), &adapter, &context).unwrap(); let before_right_click = adapter.requests.lock().unwrap().len(); let _ = right_click::execute(ref_args(&snapshot_id), &adapter); assert_eq!( adapter.requests.lock().unwrap().len(), before_right_click + 1 ); - clear::execute(ref_args(&snapshot_id), &adapter).unwrap(); - toggle::execute(ref_args(&snapshot_id), &adapter).unwrap(); - check::execute(ref_args(&snapshot_id), &adapter).unwrap(); - uncheck::execute(ref_args(&snapshot_id), &adapter).unwrap(); - expand::execute(ref_args(&snapshot_id), &adapter).unwrap(); - collapse::execute(ref_args(&snapshot_id), &adapter).unwrap(); - scroll_to::execute(ref_args(&snapshot_id), &adapter).unwrap(); + clear::execute(ref_args(&snapshot_id), &adapter, &context).unwrap(); + toggle::execute(ref_args(&snapshot_id), &adapter, &context).unwrap(); + check::execute(ref_args(&snapshot_id), &adapter, &context).unwrap(); + uncheck::execute(ref_args(&snapshot_id), &adapter, &context).unwrap(); + expand::execute(ref_args(&snapshot_id), &adapter, &context).unwrap(); + collapse::execute(ref_args(&snapshot_id), &adapter, &context).unwrap(); + scroll_to::execute(ref_args(&snapshot_id), &adapter, &context).unwrap(); set_value::execute( set_value::SetValueArgs { ref_id: "@e1".into(), @@ -122,6 +124,7 @@ fn default_ref_commands_are_headless() { value: "value".into(), }, &adapter, + &context, ) .unwrap(); select::execute( @@ -131,6 +134,7 @@ fn default_ref_commands_are_headless() { value: "choice".into(), }, &adapter, + &context, ) .unwrap(); let before_type = adapter.requests.lock().unwrap().len(); @@ -141,6 +145,7 @@ fn default_ref_commands_are_headless() { text: "text".into(), }, &adapter, + &context, ) .unwrap(); let type_request = adapter.requests.lock().unwrap()[before_type].clone(); @@ -153,6 +158,7 @@ fn default_ref_commands_are_headless() { amount: 1, }, &adapter, + &context, ) .unwrap(); @@ -170,7 +176,7 @@ fn focus_command_is_explicit_headless_policy() { let snapshot_id = snapshot_id(); let adapter = RecordingAdapter::new(); - focus::execute(ref_args(&snapshot_id), &adapter).unwrap(); + focus::execute(ref_args(&snapshot_id), &adapter, &CommandContext::default()).unwrap(); let request = adapter.last_request(); assert!(matches!(request.action, Action::SetFocus)); diff --git a/crates/core/src/commands/right_click.rs b/crates/core/src/commands/right_click.rs index 087277f5..daf62fc9 100644 --- a/crates/core/src/commands/right_click.rs +++ b/crates/core/src/commands/right_click.rs @@ -1,7 +1,8 @@ use crate::{ action::{Action, ActionRequest}, adapter::{PlatformAdapter, SnapshotSurface, TreeOptions}, - commands::helpers::{RefArgs, find_window_for_pid, resolve_ref}, + commands::helpers::{RefArgs, find_window_for_pid, resolve_ref_with_context}, + context::CommandContext, error::AppError, refs::RefEntry, snapshot, @@ -9,10 +10,31 @@ use crate::{ use serde_json::{Value, json}; pub fn execute(args: RefArgs, adapter: &dyn PlatformAdapter) -> Result { - let (entry, handle) = resolve_ref(&args.ref_id, args.snapshot_id.as_deref(), adapter)?; + execute_with_context(args, adapter, &CommandContext::default()) +} + +pub fn execute_with_context( + args: RefArgs, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { + let (entry, handle) = + resolve_ref_with_context(&args.ref_id, args.snapshot_id.as_deref(), adapter, context)?; let request = ActionRequest::headless(Action::RightClick); + context.trace( + "actionability.check.start", + serde_json::json!({ "ref": args.ref_id, "action": request.action.name() }), + )?; crate::actionability::check(&entry, &request)?; + context.trace( + "actionability.check.ok", + serde_json::json!({ "ref": args.ref_id, "action": request.action.name() }), + )?; let result = adapter.execute_action(handle.handle(), request)?; + context.trace( + "action.dispatch.ok", + serde_json::json!({ "ref": args.ref_id }), + )?; let mut response = serde_json::to_value(&result)?; std::thread::sleep(std::time::Duration::from_millis(200)); @@ -23,7 +45,7 @@ pub fn execute(args: RefArgs, adapter: &dyn PlatformAdapter) -> Result match serde_json::to_value(&snap.tree) { Ok(menu_json) => { response["menu"] = menu_json; diff --git a/crates/core/src/commands/scroll.rs b/crates/core/src/commands/scroll.rs index b0c7ebd5..f735f927 100644 --- a/crates/core/src/commands/scroll.rs +++ b/crates/core/src/commands/scroll.rs @@ -1,7 +1,8 @@ use crate::{ action::{Action, ActionRequest, Direction}, adapter::PlatformAdapter, - commands::helpers::resolve_ref, + commands::helpers::resolve_ref_with_context, + context::CommandContext, error::AppError, }; use serde_json::Value; @@ -13,10 +14,27 @@ pub struct ScrollArgs { pub amount: u32, } -pub fn execute(args: ScrollArgs, adapter: &dyn PlatformAdapter) -> Result { - let (entry, handle) = resolve_ref(&args.ref_id, args.snapshot_id.as_deref(), adapter)?; +pub fn execute( + args: ScrollArgs, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { + let (entry, handle) = + resolve_ref_with_context(&args.ref_id, args.snapshot_id.as_deref(), adapter, context)?; let request = ActionRequest::headless(Action::Scroll(args.direction, args.amount)); + context.trace( + "actionability.check.start", + serde_json::json!({ "ref": args.ref_id, "action": request.action.name() }), + )?; crate::actionability::check(&entry, &request)?; + context.trace( + "actionability.check.ok", + serde_json::json!({ "ref": args.ref_id, "action": request.action.name() }), + )?; let result = adapter.execute_action(handle.handle(), request)?; + context.trace( + "action.dispatch.ok", + serde_json::json!({ "ref": args.ref_id }), + )?; Ok(serde_json::to_value(result)?) } diff --git a/crates/core/src/commands/scroll_to.rs b/crates/core/src/commands/scroll_to.rs index 38823ede..108f32c6 100644 --- a/crates/core/src/commands/scroll_to.rs +++ b/crates/core/src/commands/scroll_to.rs @@ -1,11 +1,21 @@ use crate::{ action::{Action, ActionRequest}, adapter::PlatformAdapter, - commands::helpers::{RefArgs, execute_ref_action}, + commands::helpers::{RefArgs, execute_ref_action_with_context}, + context::CommandContext, error::AppError, }; use serde_json::Value; -pub fn execute(args: RefArgs, adapter: &dyn PlatformAdapter) -> Result { - execute_ref_action(args, adapter, ActionRequest::headless(Action::ScrollTo)) +pub fn execute( + args: RefArgs, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { + execute_ref_action_with_context( + args, + adapter, + ActionRequest::headless(Action::ScrollTo), + context, + ) } diff --git a/crates/core/src/commands/select.rs b/crates/core/src/commands/select.rs index 39ac1c5e..a31c2aa4 100644 --- a/crates/core/src/commands/select.rs +++ b/crates/core/src/commands/select.rs @@ -1,7 +1,8 @@ use crate::{ action::{Action, ActionRequest}, adapter::PlatformAdapter, - commands::helpers::resolve_ref, + commands::helpers::resolve_ref_with_context, + context::CommandContext, error::AppError, }; use serde_json::Value; @@ -12,10 +13,27 @@ pub struct SelectArgs { pub value: String, } -pub fn execute(args: SelectArgs, adapter: &dyn PlatformAdapter) -> Result { - let (entry, handle) = resolve_ref(&args.ref_id, args.snapshot_id.as_deref(), adapter)?; +pub fn execute( + args: SelectArgs, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { + let (entry, handle) = + resolve_ref_with_context(&args.ref_id, args.snapshot_id.as_deref(), adapter, context)?; let request = ActionRequest::headless(Action::Select(args.value)); + context.trace( + "actionability.check.start", + serde_json::json!({ "ref": args.ref_id, "action": request.action.name() }), + )?; crate::actionability::check(&entry, &request)?; + context.trace( + "actionability.check.ok", + serde_json::json!({ "ref": args.ref_id, "action": request.action.name() }), + )?; let result = adapter.execute_action(handle.handle(), request)?; + context.trace( + "action.dispatch.ok", + serde_json::json!({ "ref": args.ref_id }), + )?; Ok(serde_json::to_value(result)?) } diff --git a/crates/core/src/commands/set_value.rs b/crates/core/src/commands/set_value.rs index db216761..68ec08b3 100644 --- a/crates/core/src/commands/set_value.rs +++ b/crates/core/src/commands/set_value.rs @@ -1,7 +1,8 @@ use crate::{ action::{Action, ActionRequest}, adapter::PlatformAdapter, - commands::helpers::resolve_ref, + commands::helpers::resolve_ref_with_context, + context::CommandContext, error::AppError, }; use serde_json::Value; @@ -12,10 +13,27 @@ pub struct SetValueArgs { pub value: String, } -pub fn execute(args: SetValueArgs, adapter: &dyn PlatformAdapter) -> Result { - let (entry, handle) = resolve_ref(&args.ref_id, args.snapshot_id.as_deref(), adapter)?; +pub fn execute( + args: SetValueArgs, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { + let (entry, handle) = + resolve_ref_with_context(&args.ref_id, args.snapshot_id.as_deref(), adapter, context)?; let request = ActionRequest::headless(Action::SetValue(args.value)); + context.trace( + "actionability.check.start", + serde_json::json!({ "ref": args.ref_id, "action": request.action.name() }), + )?; crate::actionability::check(&entry, &request)?; + context.trace( + "actionability.check.ok", + serde_json::json!({ "ref": args.ref_id, "action": request.action.name() }), + )?; let result = adapter.execute_action(handle.handle(), request)?; + context.trace( + "action.dispatch.ok", + serde_json::json!({ "ref": args.ref_id }), + )?; Ok(serde_json::to_value(result)?) } diff --git a/crates/core/src/commands/snapshot.rs b/crates/core/src/commands/snapshot.rs index 0a7b23b0..51a81f1b 100644 --- a/crates/core/src/commands/snapshot.rs +++ b/crates/core/src/commands/snapshot.rs @@ -1,5 +1,6 @@ use crate::{ adapter::{PlatformAdapter, SnapshotSurface}, + context::CommandContext, error::AppError, refs::validate_ref_id, snapshot, snapshot_ref, @@ -38,6 +39,14 @@ fn tree_options(args: &SnapshotArgs) -> crate::adapter::TreeOptions { } pub fn execute(args: SnapshotArgs, adapter: &dyn PlatformAdapter) -> Result { + execute_with_context(args, adapter, &CommandContext::default()) +} + +pub fn execute_with_context( + args: SnapshotArgs, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { tracing::debug!( "tree: snapshot app={:?} window_id={:?} max_depth={} interactive_only={} compact={}", args.app.as_deref().unwrap_or("(focused)"), @@ -56,19 +65,21 @@ pub fn execute(args: SnapshotArgs, adapter: &dyn PlatformAdapter) -> Result Result { pub fn execute_with_report( adapter: &dyn PlatformAdapter, report: &PermissionReport, +) -> Result { + execute_with_report_with_context(adapter, report, &CommandContext::default()) +} + +pub fn execute_with_report_with_context( + adapter: &dyn PlatformAdapter, + report: &PermissionReport, + context: &CommandContext, ) -> Result { let permissions = permissions::execute_with_report(PermissionsArgs { request: false }, adapter, report)?; - let store = RefStore::new().ok(); + let store = RefStore::for_session(context.session_id.as_deref()).ok(); let ref_count = store .as_ref() .and_then(|s| s.load_latest().ok()) diff --git a/crates/core/src/commands/toggle.rs b/crates/core/src/commands/toggle.rs index 531375b3..96e48607 100644 --- a/crates/core/src/commands/toggle.rs +++ b/crates/core/src/commands/toggle.rs @@ -1,11 +1,21 @@ use crate::{ action::{Action, ActionRequest}, adapter::PlatformAdapter, - commands::helpers::{RefArgs, execute_ref_action}, + commands::helpers::{RefArgs, execute_ref_action_with_context}, + context::CommandContext, error::AppError, }; use serde_json::Value; -pub fn execute(args: RefArgs, adapter: &dyn PlatformAdapter) -> Result { - execute_ref_action(args, adapter, ActionRequest::headless(Action::Toggle)) +pub fn execute( + args: RefArgs, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { + execute_ref_action_with_context( + args, + adapter, + ActionRequest::headless(Action::Toggle), + context, + ) } diff --git a/crates/core/src/commands/triple_click.rs b/crates/core/src/commands/triple_click.rs index 88ccc415..c94a6aa2 100644 --- a/crates/core/src/commands/triple_click.rs +++ b/crates/core/src/commands/triple_click.rs @@ -1,11 +1,21 @@ use crate::{ action::{Action, ActionRequest}, adapter::PlatformAdapter, - commands::helpers::{RefArgs, execute_ref_action}, + commands::helpers::{RefArgs, execute_ref_action_with_context}, + context::CommandContext, error::AppError, }; use serde_json::Value; -pub fn execute(args: RefArgs, adapter: &dyn PlatformAdapter) -> Result { - execute_ref_action(args, adapter, ActionRequest::headless(Action::TripleClick)) +pub fn execute( + args: RefArgs, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { + execute_ref_action_with_context( + args, + adapter, + ActionRequest::headless(Action::TripleClick), + context, + ) } diff --git a/crates/core/src/commands/type_text.rs b/crates/core/src/commands/type_text.rs index ad7a4db2..b7cd6b7a 100644 --- a/crates/core/src/commands/type_text.rs +++ b/crates/core/src/commands/type_text.rs @@ -1,7 +1,8 @@ use crate::{ action::{Action, ActionRequest}, adapter::PlatformAdapter, - commands::helpers::resolve_ref, + commands::helpers::resolve_ref_with_context, + context::CommandContext, error::AppError, }; use serde_json::Value; @@ -14,16 +15,33 @@ pub struct TypeArgs { pub text: String, } -pub fn execute(args: TypeArgs, adapter: &dyn PlatformAdapter) -> Result { +pub fn execute( + args: TypeArgs, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { if args.text.len() > MAX_TEXT_LEN { return Err(AppError::invalid_input(format!( "Text exceeds maximum length of {MAX_TEXT_LEN} characters" ))); } - let (entry, handle) = resolve_ref(&args.ref_id, args.snapshot_id.as_deref(), adapter)?; + let (entry, handle) = + resolve_ref_with_context(&args.ref_id, args.snapshot_id.as_deref(), adapter, context)?; let request = ActionRequest::focus_fallback(Action::TypeText(args.text)); + context.trace( + "actionability.check.start", + serde_json::json!({ "ref": args.ref_id, "action": request.action.name() }), + )?; crate::actionability::check(&entry, &request)?; + context.trace( + "actionability.check.ok", + serde_json::json!({ "ref": args.ref_id, "action": request.action.name() }), + )?; let result = adapter.execute_action(handle.handle(), request)?; + context.trace( + "action.dispatch.ok", + serde_json::json!({ "ref": args.ref_id }), + )?; Ok(serde_json::to_value(result)?) } diff --git a/crates/core/src/commands/uncheck.rs b/crates/core/src/commands/uncheck.rs index 44c85b25..73243128 100644 --- a/crates/core/src/commands/uncheck.rs +++ b/crates/core/src/commands/uncheck.rs @@ -1,11 +1,21 @@ use crate::{ action::{Action, ActionRequest}, adapter::PlatformAdapter, - commands::helpers::{RefArgs, execute_ref_action}, + commands::helpers::{RefArgs, execute_ref_action_with_context}, + context::CommandContext, error::AppError, }; use serde_json::Value; -pub fn execute(args: RefArgs, adapter: &dyn PlatformAdapter) -> Result { - execute_ref_action(args, adapter, ActionRequest::headless(Action::Uncheck)) +pub fn execute( + args: RefArgs, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { + execute_ref_action_with_context( + args, + adapter, + ActionRequest::headless(Action::Uncheck), + context, + ) } diff --git a/crates/core/src/commands/wait.rs b/crates/core/src/commands/wait.rs index b575ae4c..a236eaae 100644 --- a/crates/core/src/commands/wait.rs +++ b/crates/core/src/commands/wait.rs @@ -1,10 +1,11 @@ use crate::{ adapter::{PlatformAdapter, WindowFilter}, - commands::{helpers::resolve_app_pid, wait_predicate}, + commands::{helpers::resolve_app_pid, wait_latest_ref_cache::LatestRefCache, wait_predicate}, + context::CommandContext, error::{AppError, ErrorCode}, node::AccessibilityNode, notification::NotificationFilter, - refs::{RefMap, validate_ref_id}, + refs::validate_ref_id, refs_store::RefStore, search_text, snapshot, }; @@ -28,6 +29,14 @@ pub struct WaitArgs { } pub fn execute(args: WaitArgs, adapter: &dyn PlatformAdapter) -> Result { + execute_with_context(args, adapter, &CommandContext::default()) +} + +pub fn execute_with_context( + args: WaitArgs, + adapter: &dyn PlatformAdapter, + context: &CommandContext, +) -> Result { validate_wait_mode(&args)?; if let Some(ms) = args.ms { @@ -59,6 +68,7 @@ pub fn execute(args: WaitArgs, adapter: &dyn PlatformAdapter) -> Result Result Result { let start = Instant::now(); let timeout = Duration::from_millis(timeout_ms); - let store = RefStore::new()?; + let store = RefStore::for_session(context.session_id.as_deref())?; let fixed_refmap = match snapshot_id.as_deref() { Some(id) => Some(store.load_snapshot(id)?), None => None, @@ -190,54 +208,6 @@ fn wait_for_element( std::thread::sleep(remaining.min(Duration::from_millis(100))); } } - -struct LatestRefCache<'a> { - store: &'a RefStore, - snapshot_id: Option, - refmap: RefMap, - last_refresh: Instant, -} - -impl<'a> LatestRefCache<'a> { - fn new(store: &'a RefStore) -> Result { - let snapshot_id = store.latest_snapshot_id(); - let refmap = if let Some(id) = snapshot_id.as_deref() { - store.load_snapshot(id)? - } else { - store.load_latest()? - }; - Ok(Self { - store, - snapshot_id, - refmap, - last_refresh: Instant::now() - Duration::from_millis(500), - }) - } - - fn entry(&self, ref_id: &str) -> Option { - self.refmap.get(ref_id).cloned() - } - - fn refresh_if_due(&mut self) { - if self.last_refresh.elapsed() < Duration::from_millis(500) { - return; - } - self.last_refresh = Instant::now(); - if let Some(snapshot_id) = self.store.latest_snapshot_id() { - if self.snapshot_id.as_deref() == Some(snapshot_id.as_str()) { - return; - } - if let Ok(refmap) = self.store.load_snapshot(&snapshot_id) { - self.snapshot_id = Some(snapshot_id); - self.refmap = refmap; - } - } else if let Ok(refmap) = self.store.load_latest() { - self.refmap = refmap; - self.snapshot_id = self.store.latest_snapshot_id(); - } - } -} - fn wait_for_window( title: String, timeout_ms: u64, @@ -274,6 +244,7 @@ fn wait_for_text( app: Option, timeout_ms: u64, adapter: &dyn PlatformAdapter, + context: &CommandContext, ) -> Result { let start = Instant::now(); let timeout = Duration::from_millis(timeout_ms); @@ -288,7 +259,8 @@ fn wait_for_text( .map(|expected| matches.len() == expected) .unwrap_or_else(|| !matches.is_empty()); if matched { - let snapshot_id = RefStore::new()?.save_new_snapshot(&result.refmap)?; + let snapshot_id = RefStore::for_session(context.session_id.as_deref())? + .save_new_snapshot(&result.refmap)?; let elapsed = start.elapsed().as_millis(); let found = matches.first(); return Ok(json!({ diff --git a/crates/core/src/commands/wait_latest_ref_cache.rs b/crates/core/src/commands/wait_latest_ref_cache.rs new file mode 100644 index 00000000..68b48561 --- /dev/null +++ b/crates/core/src/commands/wait_latest_ref_cache.rs @@ -0,0 +1,49 @@ +use crate::{error::AppError, refs::RefMap, refs_store::RefStore}; +use std::time::{Duration, Instant}; + +pub(crate) struct LatestRefCache<'a> { + store: &'a RefStore, + pub(crate) snapshot_id: Option, + refmap: RefMap, + pub(crate) last_refresh: Instant, +} + +impl<'a> LatestRefCache<'a> { + pub(crate) fn new(store: &'a RefStore) -> Result { + let snapshot_id = store.latest_snapshot_id(); + let refmap = if let Some(id) = snapshot_id.as_deref() { + store.load_snapshot(id)? + } else { + store.load_latest()? + }; + Ok(Self { + store, + snapshot_id, + refmap, + last_refresh: Instant::now() - Duration::from_millis(500), + }) + } + + pub(crate) fn entry(&self, ref_id: &str) -> Option { + self.refmap.get(ref_id).cloned() + } + + pub(crate) fn refresh_if_due(&mut self) { + if self.last_refresh.elapsed() < Duration::from_millis(500) { + return; + } + self.last_refresh = Instant::now(); + if let Some(snapshot_id) = self.store.latest_snapshot_id() { + if self.snapshot_id.as_deref() == Some(snapshot_id.as_str()) { + return; + } + if let Ok(refmap) = self.store.load_snapshot(&snapshot_id) { + self.snapshot_id = Some(snapshot_id); + self.refmap = refmap; + } + } else if let Ok(refmap) = self.store.load_latest() { + self.refmap = refmap; + self.snapshot_id = self.store.latest_snapshot_id(); + } + } +} diff --git a/crates/core/src/commands/wait_tests.rs b/crates/core/src/commands/wait_tests.rs index 25b3c418..e0cb5539 100644 --- a/crates/core/src/commands/wait_tests.rs +++ b/crates/core/src/commands/wait_tests.rs @@ -86,6 +86,7 @@ fn snapshot_pinned_missing_ref_is_invalid_args() { wait_predicate::ElementPredicate::Exists, 1, &NoopAdapter, + &crate::context::CommandContext::default(), ) .unwrap_err(); @@ -185,6 +186,7 @@ fn element_wait_enabled_predicate_uses_live_state() { wait_predicate::ElementPredicate::Enabled, 1, &adapter, + &crate::context::CommandContext::default(), ) .unwrap(); @@ -208,6 +210,7 @@ fn element_wait_value_predicate_matches_live_value() { wait_predicate::ElementPredicate::Value("ready".into()), 1, &adapter, + &crate::context::CommandContext::default(), ) .unwrap(); diff --git a/crates/core/src/context.rs b/crates/core/src/context.rs new file mode 100644 index 00000000..1cb15104 --- /dev/null +++ b/crates/core/src/context.rs @@ -0,0 +1,130 @@ +use crate::{error::AppError, trace::TraceConfig}; +use serde_json::Value; +use std::path::PathBuf; + +#[derive(Debug, Clone, Default)] +pub struct CommandContext { + pub session_id: Option, + pub trace: TraceConfig, +} + +impl CommandContext { + pub fn new( + session_id: Option, + trace_path: Option, + trace_strict: bool, + ) -> Result { + if let Some(id) = session_id.as_deref() { + validate_session_id(id)?; + } + Ok(Self { + session_id, + trace: TraceConfig::new(trace_path, trace_strict)?, + }) + } + + pub fn for_batch_item(&self, session_id: Option) -> Result { + let session_id = session_id.or_else(|| self.session_id.clone()); + if let Some(id) = session_id.as_deref() { + validate_session_id(id)?; + } + Ok(Self { + session_id, + trace: self.trace.clone(), + }) + } + + pub fn trace(&self, event: &str, fields: Value) -> Result<(), AppError> { + self.trace.emit(event, fields) + } +} + +pub fn validate_session_id(id: &str) -> Result<(), AppError> { + let valid = !id.is_empty() + && id.len() <= 64 + && id + .bytes() + .all(|b| b.is_ascii_alphanumeric() || matches!(b, b'-' | b'_')); + if valid { + return Ok(()); + } + Err(AppError::invalid_input_with_suggestion( + "Session id must be 1-64 chars using letters, numbers, '-' or '_'", + "Use a short filesystem-safe session id such as run_1 or agent-a.", + )) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn accepts_filesystem_safe_session_ids() { + assert!(validate_session_id("agent-1_A").is_ok()); + } + + #[test] + fn rejects_path_like_session_ids() { + assert!(validate_session_id("../agent").is_err()); + assert!(validate_session_id("agent/a").is_err()); + } + + #[test] + fn trace_writes_jsonl_without_stdout_dependency() { + let path = std::env::temp_dir().join(format!( + "agent-desktop-trace-{}.jsonl", + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + let context = CommandContext::new(None, Some(path.clone()), false).unwrap(); + + context + .trace("ref.resolve.ok", serde_json::json!({ "ref": "@e1" })) + .unwrap(); + + let body = std::fs::read_to_string(&path).unwrap(); + let event: serde_json::Value = serde_json::from_str(body.trim()).unwrap(); + assert_eq!(event["event"], "ref.resolve.ok"); + assert_eq!(event["ref"], "@e1"); + assert!(event["ts_ms"].as_u64().is_some()); + let _ = std::fs::remove_file(path); + } + + #[test] + fn trace_write_failure_is_best_effort_unless_strict() { + let missing = std::env::temp_dir() + .join("agent-desktop-missing-dir") + .join("trace.jsonl"); + + let best_effort = CommandContext::new(None, Some(missing.clone()), false).unwrap(); + assert!(best_effort.trace("event", serde_json::json!({})).is_ok()); + + let strict = CommandContext::new(None, Some(missing), true).unwrap(); + assert!(strict.trace("event", serde_json::json!({})).is_err()); + } + + #[test] + fn trace_strict_requires_trace_path() { + let err = CommandContext::new(None, None, true).unwrap_err(); + assert_eq!(err.code(), "INVALID_ARGS"); + } + + #[test] + fn batch_item_inherits_or_overrides_session_without_trace_loss() { + let parent = CommandContext::new( + Some("parent".into()), + Some(std::env::temp_dir().join("agent-desktop-context-test.jsonl")), + false, + ) + .unwrap(); + + let inherited = parent.for_batch_item(None).unwrap(); + let overridden = parent.for_batch_item(Some("child".into())).unwrap(); + + assert_eq!(inherited.session_id.as_deref(), Some("parent")); + assert_eq!(overridden.session_id.as_deref(), Some("child")); + assert!(overridden.trace.path.is_some()); + } +} diff --git a/crates/core/src/lib.rs b/crates/core/src/lib.rs index 68b6ddc2..264d1622 100644 --- a/crates/core/src/lib.rs +++ b/crates/core/src/lib.rs @@ -2,6 +2,7 @@ pub mod action; pub mod actionability; pub mod adapter; pub mod commands; +pub mod context; pub mod error; pub mod hints; pub mod node; @@ -20,6 +21,7 @@ pub mod roles; pub(crate) mod search_text; pub mod snapshot; pub mod snapshot_ref; +pub mod trace; mod window_lookup; pub use action::{ @@ -30,6 +32,7 @@ pub use adapter::{ ImageBuffer, ImageFormat, NativeHandle, PlatformAdapter, ScreenshotTarget, TreeOptions, WindowFilter, }; +pub use context::CommandContext; pub use error::{AdapterError, AppError, ErrorCode}; pub use node::{AccessibilityNode, AppInfo, Rect, WindowInfo}; pub use notification::{NotificationFilter, NotificationInfo}; diff --git a/crates/core/src/refs_store.rs b/crates/core/src/refs_store.rs index fe4ea7d1..a122e1f8 100644 --- a/crates/core/src/refs_store.rs +++ b/crates/core/src/refs_store.rs @@ -1,4 +1,5 @@ use crate::{ + context::validate_session_id, error::{AdapterError, AppError}, refs::{RefMap, home_dir, new_snapshot_id, validate_snapshot_id, write_private_file}, refs_lock::RefStoreLock, @@ -16,8 +17,21 @@ pub struct RefStore { impl RefStore { pub fn new() -> Result { + Self::for_session(None) + } + + pub fn for_session(session_id: Option<&str>) -> Result { let home = home_dir().ok_or_else(|| AppError::Internal("HOME directory not found".into()))?; + if let Some(session_id) = session_id { + validate_session_id(session_id)?; + return Ok(Self { + base_dir: home + .join(".agent-desktop") + .join("sessions") + .join(session_id), + }); + } Ok(Self { base_dir: home.join(".agent-desktop"), }) diff --git a/crates/core/src/refs_tests.rs b/crates/core/src/refs_tests.rs index d32b906a..e620dab3 100644 --- a/crates/core/src/refs_tests.rs +++ b/crates/core/src/refs_tests.rs @@ -242,6 +242,49 @@ fn test_refstore_snapshot_roundtrip_and_latest_pointer() { assert_eq!(store.load(None).unwrap().len(), 1); } +#[test] +fn test_refstore_sessions_are_isolated_from_default_store() { + let _guard = HomeGuard::new(); + let default_store = RefStore::new().unwrap(); + let session_a = RefStore::for_session(Some("agent-a")).unwrap(); + let session_b = RefStore::for_session(Some("agent-b")).unwrap(); + + let mut default_map = RefMap::new(); + default_map.allocate(entry("button", Some("Default"))); + let default_id = default_store.save_new_snapshot(&default_map).unwrap(); + + let mut session_map = RefMap::new(); + session_map.allocate(entry("button", Some("Session A"))); + let session_id = session_a.save_new_snapshot(&session_map).unwrap(); + + assert_eq!(default_store.load(None).unwrap().len(), 1); + assert_eq!( + default_store + .load(Some(&default_id)) + .unwrap() + .get("@e1") + .unwrap() + .name + .as_deref(), + Some("Default") + ); + assert_eq!( + session_a + .load(Some(&session_id)) + .unwrap() + .get("@e1") + .unwrap() + .name + .as_deref(), + Some("Session A") + ); + assert!(session_b.load(None).is_err()); + assert_ne!( + default_store.latest_snapshot_id(), + session_a.latest_snapshot_id() + ); +} + #[test] fn test_save_existing_snapshot_does_not_promote_latest_pointer() { let _guard = HomeGuard::new(); diff --git a/crates/core/src/snapshot.rs b/crates/core/src/snapshot.rs index a78af3f0..2c53f5d1 100644 --- a/crates/core/src/snapshot.rs +++ b/crates/core/src/snapshot.rs @@ -1,5 +1,6 @@ use crate::{ adapter::{PlatformAdapter, SnapshotSurface, TreeOptions, WindowFilter}, + context::CommandContext, error::AppError, node::{AccessibilityNode, WindowInfo}, ref_alloc::{self, RefAllocConfig}, @@ -107,11 +108,31 @@ pub fn run( opts: &TreeOptions, app_name: Option<&str>, window_id: Option<&str>, +) -> Result { + run_with_context( + adapter, + opts, + app_name, + window_id, + &CommandContext::default(), + ) +} + +pub fn run_with_context( + adapter: &dyn PlatformAdapter, + opts: &TreeOptions, + app_name: Option<&str>, + window_id: Option<&str>, + context: &CommandContext, ) -> Result { let mut result = build(adapter, opts, app_name, window_id)?; - let store = RefStore::new()?; + let store = RefStore::for_session(context.session_id.as_deref())?; let snapshot_id = store.save_new_snapshot(&result.refmap)?; result.snapshot_id = Some(snapshot_id); + context.trace( + "snapshot.saved", + serde_json::json!({ "snapshot_id": result.snapshot_id, "ref_count": result.refmap.len() }), + )?; Ok(result) } @@ -120,6 +141,22 @@ pub fn append_surface_refs( pid: i32, source_app: Option<&str>, surface: SnapshotSurface, +) -> Result, AppError> { + append_surface_refs_with_context( + adapter, + pid, + source_app, + surface, + &CommandContext::default(), + ) +} + +pub fn append_surface_refs_with_context( + adapter: &dyn PlatformAdapter, + pid: i32, + source_app: Option<&str>, + surface: SnapshotSurface, + context: &CommandContext, ) -> Result, AppError> { let filter = WindowFilter { focused_only: false, @@ -135,7 +172,7 @@ pub fn append_surface_refs( ..Default::default() }; let raw_tree = adapter.get_tree(&window, &opts)?; - let store = RefStore::new()?; + let store = RefStore::for_session(context.session_id.as_deref())?; let mut refmap = store.load_latest()?; let config = RefAllocConfig { include_bounds: false, diff --git a/crates/core/src/snapshot_ref.rs b/crates/core/src/snapshot_ref.rs index 40d318b4..fe493e56 100644 --- a/crates/core/src/snapshot_ref.rs +++ b/crates/core/src/snapshot_ref.rs @@ -1,5 +1,6 @@ use crate::{ adapter::{PlatformAdapter, TreeOptions}, + context::CommandContext, error::AppError, node::WindowInfo, ref_alloc::{self, RefAllocConfig}, @@ -14,7 +15,23 @@ pub fn run_from_ref( root_ref_id: &str, snapshot_id: Option<&str>, ) -> Result { - let store = RefStore::new()?; + run_from_ref_with_context( + adapter, + opts, + root_ref_id, + snapshot_id, + &CommandContext::default(), + ) +} + +pub fn run_from_ref_with_context( + adapter: &dyn PlatformAdapter, + opts: &TreeOptions, + root_ref_id: &str, + snapshot_id: Option<&str>, + context: &CommandContext, +) -> Result { + let store = RefStore::for_session(context.session_id.as_deref())?; let mut refmap = store.load(snapshot_id)?; let active_snapshot_id = snapshot_id .map(str::to_string) @@ -58,6 +75,14 @@ pub fn run_from_ref( } else { Some(store.save_new_snapshot(&refmap)?) }; + context.trace( + "snapshot.root.saved", + serde_json::json!({ + "root_ref": root_ref_id, + "snapshot_id": saved_snapshot_id, + "ref_count": refmap.len() + }), + )?; let window = crate::window_lookup::find_window_for_pid(entry.pid, adapter).unwrap_or(WindowInfo { diff --git a/crates/core/src/trace.rs b/crates/core/src/trace.rs new file mode 100644 index 00000000..fce32ad2 --- /dev/null +++ b/crates/core/src/trace.rs @@ -0,0 +1,62 @@ +use crate::error::AppError; +use serde_json::{Map, Value, json}; +use std::path::{Path, PathBuf}; + +#[derive(Debug, Clone, Default)] +pub struct TraceConfig { + pub path: Option, + pub strict: bool, +} + +impl TraceConfig { + pub fn new(path: Option, strict: bool) -> Result { + if strict && path.is_none() { + return Err(AppError::invalid_input_with_suggestion( + "--trace-strict requires --trace", + "Provide --trace or remove --trace-strict.", + )); + } + Ok(Self { path, strict }) + } + + pub fn emit(&self, event: &str, fields: Value) -> Result<(), AppError> { + let Some(path) = self.path.as_deref() else { + return Ok(()); + }; + match write_event(path, event, fields) { + Ok(()) => Ok(()), + Err(err) if self.strict => Err(err), + Err(err) => { + tracing::warn!("trace write failed: {err}"); + Ok(()) + } + } + } +} + +fn write_event(path: &Path, event: &str, fields: Value) -> Result<(), AppError> { + let mut body = Map::new(); + body.insert("event".to_string(), json!(event)); + body.insert( + "ts_ms".to_string(), + json!( + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map_err(|err| AppError::Internal(err.to_string()))? + .as_millis() + ), + ); + if let Value::Object(fields) = fields { + for (key, value) in fields { + body.insert(key, value); + } + } + let mut file = std::fs::OpenOptions::new() + .create(true) + .append(true) + .open(path) + .map_err(AppError::from)?; + serde_json::to_writer(&mut file, &Value::Object(body))?; + use std::io::Write; + file.write_all(b"\n").map_err(AppError::from) +} diff --git a/crates/ffi/include/agent_desktop.h b/crates/ffi/include/agent_desktop.h index 853601cc..34213b0f 100644 --- a/crates/ffi/include/agent_desktop.h +++ b/crates/ffi/include/agent_desktop.h @@ -230,15 +230,6 @@ typedef struct AdActionResult { struct AdElementState *post_state; } AdActionResult; -typedef struct AdRefEntry { - int32_t pid; - const char *role; - const char *name; - const char *description; - uint64_t bounds_hash; - bool has_bounds_hash; -} AdRefEntry; - typedef struct AdRect { double x; double y; @@ -246,6 +237,30 @@ typedef struct AdRect { double height; } AdRect; +typedef struct AdRefEntry { + int32_t pid; + const char *role; + const char *name; + const char *value; + const char *description; + const char *const *states; + uintptr_t state_count; + const char *const *available_actions; + uintptr_t available_action_count; + struct AdRect bounds; + bool has_bounds; + uint64_t bounds_hash; + bool has_bounds_hash; + const char *source_app; + const char *source_window_id; + const char *source_window_title; + int32_t source_surface; + const char *root_ref; + bool path_is_absolute; + const uint32_t *path; + uintptr_t path_count; +} AdRefEntry; + typedef struct AdWindowInfo { const char *id; const char *title; @@ -400,6 +415,20 @@ AdResult ad_execute_action_with_policy(const struct AdAdapter *adapter, int32_t policy, struct AdActionResult *out); +/** + * # Safety + * + * `adapter` must be a non-null pointer returned by `ad_adapter_create`. + * `entry` must be a non-null pointer to a valid `AdRefEntry`. + * `action` must be a non-null pointer to a valid `AdAction`. + * `out` must be a non-null pointer to an `AdActionResult` to write the result into. + */ +AdResult ad_execute_ref_action_with_policy(const struct AdAdapter *adapter, + const struct AdRefEntry *entry, + const struct AdAction *action, + int32_t policy, + struct AdActionResult *out); + /** * Releases a handle previously returned by `ad_resolve_element` and * zeroes the caller's struct so accidentally calling this twice is diff --git a/crates/ffi/src/actions/execute.rs b/crates/ffi/src/actions/execute.rs index 1728a70c..b3b2e1ff 100644 --- a/crates/ffi/src/actions/execute.rs +++ b/crates/ffi/src/actions/execute.rs @@ -3,7 +3,7 @@ use crate::actions::conversion::action_from_c; use crate::actions::result::action_result_to_c; use crate::error::{self, AdResult}; use crate::ffi_try::trap_panic; -use crate::types::{AdAction, AdActionResult, AdNativeHandle, AdPolicyKind}; +use crate::types::{AdAction, AdActionResult, AdNativeHandle, AdPolicyKind, AdRefEntry}; use agent_desktop_core::{action::ActionRequest, adapter::NativeHandle}; /// # Safety @@ -41,9 +41,6 @@ pub unsafe extern "C" fn ad_execute_action_with_policy( trap_panic(|| unsafe { crate::pointer_guard::guard_non_null!(out, c"out is null"); *out = std::mem::zeroed(); - if let Err(rc) = crate::main_thread::require_main_thread() { - return rc; - } crate::pointer_guard::guard_non_null!(adapter, c"adapter is null"); crate::pointer_guard::guard_non_null!(handle, c"handle is null"); crate::pointer_guard::guard_non_null!(action, c"action is null"); @@ -89,6 +86,90 @@ pub unsafe extern "C" fn ad_execute_action_with_policy( }) } +/// # Safety +/// +/// `adapter` must be a non-null pointer returned by `ad_adapter_create`. +/// `entry` must be a non-null pointer to a valid `AdRefEntry`. +/// `action` must be a non-null pointer to a valid `AdAction`. +/// `out` must be a non-null pointer to an `AdActionResult` to write the result into. +#[unsafe(no_mangle)] +pub unsafe extern "C" fn ad_execute_ref_action_with_policy( + adapter: *const AdAdapter, + entry: *const AdRefEntry, + action: *const AdAction, + policy: i32, + out: *mut AdActionResult, +) -> AdResult { + trap_panic(|| unsafe { + crate::pointer_guard::guard_non_null!(out, c"out is null"); + *out = std::mem::zeroed(); + if let Err(rc) = crate::main_thread::require_main_thread() { + return rc; + } + crate::pointer_guard::guard_non_null!(adapter, c"adapter is null"); + crate::pointer_guard::guard_non_null!(entry, c"entry is null"); + crate::pointer_guard::guard_non_null!(action, c"action is null"); + let adapter = &*adapter; + let entry_ref = &*entry; + let core_entry = match crate::actions::resolve::core_ref_entry_from_ffi(entry_ref) { + Ok(entry) => entry, + Err(err) => { + error::set_last_error(&err); + return error::last_error_code(); + } + }; + let action_ref = &*action; + let core_action = match action_from_c(action_ref) { + Ok(action) => action, + Err(msg) => { + error::set_last_error(&agent_desktop_core::error::AdapterError::new( + agent_desktop_core::error::ErrorCode::InvalidArgs, + msg, + )); + return AdResult::ErrInvalidArgs; + } + }; + let Some(policy) = AdPolicyKind::from_c(policy) else { + error::set_last_error(&agent_desktop_core::error::AdapterError::new( + agent_desktop_core::error::ErrorCode::InvalidArgs, + "invalid policy kind discriminant", + )); + return AdResult::ErrInvalidArgs; + }; + let request = action_request(policy, core_action); + if let Err(err) = agent_desktop_core::actionability::check(&core_entry, &request) { + error::set_last_error(&err); + return error::last_error_code(); + } + if let Err(rc) = crate::main_thread::require_main_thread() { + return rc; + } + let native_handle = match adapter.inner.resolve_element_strict(&core_entry) { + Ok(handle) => handle, + Err(err) => { + error::set_last_error(&err); + return error::last_error_code(); + } + }; + let result = adapter.inner.execute_action(&native_handle, request); + let release = adapter.inner.release_handle(&native_handle); + match result { + Ok(result) => { + if let Err(err) = release { + error::set_last_error(&err); + return error::last_error_code(); + } + *out = action_result_to_c(&result); + AdResult::Ok + } + Err(err) => { + error::set_last_error(&err); + error::last_error_code() + } + } + }) +} + fn action_request( policy: AdPolicyKind, action: agent_desktop_core::action::Action, diff --git a/crates/ffi/src/actions/resolve.rs b/crates/ffi/src/actions/resolve.rs index 295358c0..d7b2bb09 100644 --- a/crates/ffi/src/actions/resolve.rs +++ b/crates/ffi/src/actions/resolve.rs @@ -34,7 +34,7 @@ pub unsafe extern "C" fn ad_resolve_element( if let Err(rc) = crate::main_thread::require_main_thread() { return rc; } - match adapter.inner.resolve_element(&core_entry) { + match adapter.inner.resolve_element_strict(&core_entry) { Ok(handle) => { let handle = ManuallyDrop::new(handle); (*out).ptr = handle.as_raw(); @@ -48,7 +48,7 @@ pub unsafe extern "C" fn ad_resolve_element( }) } -unsafe fn core_ref_entry_from_ffi( +pub(crate) unsafe fn core_ref_entry_from_ffi( entry: &AdRefEntry, ) -> Result { let role = unsafe { c_to_string(entry.role) }.ok_or_else(|| { @@ -58,30 +58,53 @@ unsafe fn core_ref_entry_from_ffi( ) })?; let name = unsafe { optional_string(entry.name, "name") }?; + let value = unsafe { optional_string(entry.value, "value") }?; let description = unsafe { optional_string(entry.description, "description") }?; + let states = unsafe { string_array(entry.states, entry.state_count, "states") }?; + let available_actions = unsafe { + string_array( + entry.available_actions, + entry.available_action_count, + "available_actions", + ) + }?; + let bounds = if entry.has_bounds { + Some(agent_desktop_core::node::Rect { + x: entry.bounds.x, + y: entry.bounds.y, + width: entry.bounds.width, + height: entry.bounds.height, + }) + } else { + None + }; let bounds_hash = if entry.has_bounds_hash { Some(entry.bounds_hash) } else { None }; + let source_surface = source_surface_from_c(entry.source_surface)?; + let path = unsafe { ref_path(entry.path, entry.path_count)? }; Ok(CoreRefEntry { pid: entry.pid, role, name, - value: None, + value, description, - states: vec![], - bounds: None, + states, + bounds, bounds_hash, - available_actions: vec![], - source_app: None, - source_window_id: None, - source_window_title: None, - source_surface: agent_desktop_core::adapter::SnapshotSurface::Window, - root_ref: None, - path_is_absolute: false, - path: smallvec::SmallVec::new(), + available_actions, + source_app: unsafe { optional_string(entry.source_app, "source_app") }?, + source_window_id: unsafe { optional_string(entry.source_window_id, "source_window_id") }?, + source_window_title: unsafe { + optional_string(entry.source_window_title, "source_window_title") + }?, + source_surface, + root_ref: unsafe { optional_string(entry.root_ref, "root_ref") }?, + path_is_absolute: entry.path_is_absolute, + path, }) } @@ -97,6 +120,75 @@ unsafe fn optional_string( }) } +unsafe fn string_array( + ptr: *const *const std::os::raw::c_char, + len: usize, + field: &str, +) -> Result, agent_desktop_core::error::AdapterError> { + if len == 0 { + return Ok(Vec::new()); + } + if ptr.is_null() { + return Err(agent_desktop_core::error::AdapterError::new( + agent_desktop_core::error::ErrorCode::InvalidArgs, + format!("{field} count is nonzero but pointer is null"), + )); + } + let items = unsafe { std::slice::from_raw_parts(ptr, len) }; + items + .iter() + .enumerate() + .map(|(index, item)| unsafe { + c_to_string(*item).ok_or_else(|| { + agent_desktop_core::error::AdapterError::new( + agent_desktop_core::error::ErrorCode::InvalidArgs, + format!("{field}[{index}] is null or invalid UTF-8"), + ) + }) + }) + .collect() +} + +unsafe fn ref_path( + ptr: *const u32, + len: usize, +) -> Result, agent_desktop_core::error::AdapterError> { + if len == 0 { + return Ok(smallvec::SmallVec::new()); + } + if ptr.is_null() { + return Err(agent_desktop_core::error::AdapterError::new( + agent_desktop_core::error::ErrorCode::InvalidArgs, + "path_count is nonzero but path pointer is null", + )); + } + let mut path = smallvec::SmallVec::new(); + path.extend( + unsafe { std::slice::from_raw_parts(ptr, len) } + .iter() + .map(|item| *item as usize), + ); + Ok(path) +} + +fn source_surface_from_c( + raw: i32, +) -> Result { + match raw { + 0 => Ok(agent_desktop_core::adapter::SnapshotSurface::Window), + 1 => Ok(agent_desktop_core::adapter::SnapshotSurface::Focused), + 2 => Ok(agent_desktop_core::adapter::SnapshotSurface::Menu), + 3 => Ok(agent_desktop_core::adapter::SnapshotSurface::Menubar), + 4 => Ok(agent_desktop_core::adapter::SnapshotSurface::Sheet), + 5 => Ok(agent_desktop_core::adapter::SnapshotSurface::Popover), + 6 => Ok(agent_desktop_core::adapter::SnapshotSurface::Alert), + _ => Err(agent_desktop_core::error::AdapterError::new( + agent_desktop_core::error::ErrorCode::InvalidArgs, + "invalid source_surface discriminant", + )), + } +} + #[cfg(test)] #[path = "resolve_tests.rs"] mod tests; diff --git a/crates/ffi/src/actions/resolve_tests.rs b/crates/ffi/src/actions/resolve_tests.rs index ef981ccc..17071e7e 100644 --- a/crates/ffi/src/actions/resolve_tests.rs +++ b/crates/ffi/src/actions/resolve_tests.rs @@ -2,44 +2,140 @@ use super::*; use agent_desktop_core::error::ErrorCode; use std::ffi::CString; +fn test_ref_entry() -> AdRefEntry { + AdRefEntry { + pid: 0, + role: std::ptr::null(), + name: std::ptr::null(), + value: std::ptr::null(), + description: std::ptr::null(), + states: std::ptr::null(), + state_count: 0, + available_actions: std::ptr::null(), + available_action_count: 0, + bounds: crate::types::AdRect { + x: 0.0, + y: 0.0, + width: 0.0, + height: 0.0, + }, + has_bounds: false, + bounds_hash: 0, + has_bounds_hash: false, + source_app: std::ptr::null(), + source_window_id: std::ptr::null(), + source_window_title: std::ptr::null(), + source_surface: 0, + root_ref: std::ptr::null(), + path_is_absolute: false, + path: std::ptr::null(), + path_count: 0, + } +} + #[test] fn ffi_ref_entry_preserves_description_identity() { let role = CString::new("button").unwrap(); let name = CString::new("Primary").unwrap(); + let value = CString::new("On").unwrap(); let description = CString::new("Insert Shape").unwrap(); - let entry = AdRefEntry { - pid: 42, - role: role.as_ptr(), - name: name.as_ptr(), - description: description.as_ptr(), - bounds_hash: 123, - has_bounds_hash: true, + let state = CString::new("focused").unwrap(); + let action = CString::new("Click").unwrap(); + let source_app = CString::new("Finder").unwrap(); + let window_id = CString::new("w-1").unwrap(); + let window_title = CString::new("Documents").unwrap(); + let root_ref = CString::new("@e1").unwrap(); + let states = [state.as_ptr()]; + let actions = [action.as_ptr()]; + let path = [1_u32, 2, 3]; + let mut entry = test_ref_entry(); + entry.pid = 42; + entry.role = role.as_ptr(); + entry.name = name.as_ptr(); + entry.value = value.as_ptr(); + entry.description = description.as_ptr(); + entry.states = states.as_ptr(); + entry.state_count = states.len(); + entry.available_actions = actions.as_ptr(); + entry.available_action_count = actions.len(); + entry.bounds = crate::types::AdRect { + x: 1.0, + y: 2.0, + width: 3.0, + height: 4.0, }; + entry.has_bounds = true; + entry.bounds_hash = 123; + entry.has_bounds_hash = true; + entry.source_app = source_app.as_ptr(); + entry.source_window_id = window_id.as_ptr(); + entry.source_window_title = window_title.as_ptr(); + entry.source_surface = 5; + entry.root_ref = root_ref.as_ptr(); + entry.path_is_absolute = true; + entry.path = path.as_ptr(); + entry.path_count = path.len(); let core_entry = unsafe { core_ref_entry_from_ffi(&entry) }.unwrap(); assert_eq!(core_entry.pid, 42); assert_eq!(core_entry.role, "button"); assert_eq!(core_entry.name.as_deref(), Some("Primary")); + assert_eq!(core_entry.value.as_deref(), Some("On")); assert_eq!(core_entry.description.as_deref(), Some("Insert Shape")); + assert_eq!(core_entry.states, ["focused"]); + assert_eq!(core_entry.available_actions, ["Click"]); + assert_eq!(core_entry.bounds.unwrap().width, 3.0); assert_eq!(core_entry.bounds_hash, Some(123)); + assert_eq!(core_entry.source_app.as_deref(), Some("Finder")); + assert_eq!(core_entry.source_window_id.as_deref(), Some("w-1")); + assert_eq!(core_entry.source_window_title.as_deref(), Some("Documents")); + assert_eq!( + core_entry.source_surface, + agent_desktop_core::adapter::SnapshotSurface::Popover + ); + assert_eq!(core_entry.root_ref.as_deref(), Some("@e1")); + assert!(core_entry.path_is_absolute); + assert_eq!(core_entry.path.as_slice(), &[1, 2, 3]); } #[test] fn ffi_ref_entry_rejects_invalid_description_identity() { let role = CString::new("button").unwrap(); let bad_description: [u8; 2] = [0xC3, 0x00]; - let entry = AdRefEntry { - pid: 42, - role: role.as_ptr(), - name: std::ptr::null(), - description: bad_description.as_ptr().cast(), - bounds_hash: 0, - has_bounds_hash: false, - }; + let mut entry = test_ref_entry(); + entry.pid = 42; + entry.role = role.as_ptr(); + entry.description = bad_description.as_ptr().cast(); let err = unsafe { core_ref_entry_from_ffi(&entry) }.unwrap_err(); assert_eq!(err.code, ErrorCode::InvalidArgs); assert_eq!(err.message, "description is not valid UTF-8"); } + +#[test] +fn ffi_ref_entry_rejects_invalid_array_pointer() { + let role = CString::new("button").unwrap(); + let mut entry = test_ref_entry(); + entry.role = role.as_ptr(); + entry.state_count = 1; + + let err = unsafe { core_ref_entry_from_ffi(&entry) }.unwrap_err(); + + assert_eq!(err.code, ErrorCode::InvalidArgs); + assert_eq!(err.message, "states count is nonzero but pointer is null"); +} + +#[test] +fn ffi_ref_entry_rejects_unknown_surface() { + let role = CString::new("button").unwrap(); + let mut entry = test_ref_entry(); + entry.role = role.as_ptr(); + entry.source_surface = 99; + + let err = unsafe { core_ref_entry_from_ffi(&entry) }.unwrap_err(); + + assert_eq!(err.code, ErrorCode::InvalidArgs); + assert_eq!(err.message, "invalid source_surface discriminant"); +} diff --git a/crates/ffi/src/types/ref_entry.rs b/crates/ffi/src/types/ref_entry.rs index dd4d3be5..03243f35 100644 --- a/crates/ffi/src/types/ref_entry.rs +++ b/crates/ffi/src/types/ref_entry.rs @@ -1,3 +1,4 @@ +use crate::types::AdRect; use std::os::raw::c_char; #[repr(C)] @@ -5,7 +6,22 @@ pub struct AdRefEntry { pub pid: i32, pub role: *const c_char, pub name: *const c_char, + pub value: *const c_char, pub description: *const c_char, + pub states: *const *const c_char, + pub state_count: usize, + pub available_actions: *const *const c_char, + pub available_action_count: usize, + pub bounds: AdRect, + pub has_bounds: bool, pub bounds_hash: u64, pub has_bounds_hash: bool, + pub source_app: *const c_char, + pub source_window_id: *const c_char, + pub source_window_title: *const c_char, + pub source_surface: i32, + pub root_ref: *const c_char, + pub path_is_absolute: bool, + pub path: *const u32, + pub path_count: usize, } diff --git a/crates/ffi/tests/c_abi_actions.rs b/crates/ffi/tests/c_abi_actions.rs index 7f5a59dc..7019f7b9 100644 --- a/crates/ffi/tests/c_abi_actions.rs +++ b/crates/ffi/tests/c_abi_actions.rs @@ -2,7 +2,8 @@ mod common; use common::{ AdActionResult, AdNativeHandle, AdPolicyKind, AdResult, ad_execute_action, - ad_execute_action_with_policy, default_action, with_adapter, + ad_execute_action_with_policy, ad_execute_ref_action_with_policy, default_action, + default_ref_entry, with_adapter, }; #[test] @@ -60,3 +61,27 @@ fn execute_action_rejects_null_handle_ptr() { )); }); } + +#[test] +fn execute_ref_action_applies_actionability_before_resolve() { + with_adapter(|adapter| unsafe { + let role = std::ffi::CString::new("button").unwrap(); + let mut entry = default_ref_entry(); + entry.role = role.as_ptr(); + let action = default_action(); + let mut out: AdActionResult = std::mem::zeroed(); + + let rc = ad_execute_ref_action_with_policy( + adapter, + &entry, + &action, + AdPolicyKind::Headless as i32, + &mut out, + ); + + assert!(matches!( + rc, + AdResult::ErrActionFailed | AdResult::ErrInternal + )); + }); +} diff --git a/crates/ffi/tests/c_abi_inputs.rs b/crates/ffi/tests/c_abi_inputs.rs index aa817918..764fcb3e 100644 --- a/crates/ffi/tests/c_abi_inputs.rs +++ b/crates/ffi/tests/c_abi_inputs.rs @@ -1,8 +1,8 @@ mod common; use common::{ - AdNativeHandle, AdRefEntry, AdResult, AdWindowInfo, AdWindowList, ad_launch_app, - ad_list_windows, ad_resolve_element, c_char, with_adapter, + AdNativeHandle, AdResult, AdWindowInfo, AdWindowList, ad_launch_app, ad_list_windows, + ad_resolve_element, c_char, default_ref_entry, with_adapter, }; #[test] @@ -38,14 +38,9 @@ fn resolve_element_rejects_invalid_utf8_name() { with_adapter(|adapter| unsafe { let role = std::ffi::CString::new("button").unwrap(); let bad_name: [u8; 2] = [0xC3, 0x00]; - let entry = AdRefEntry { - pid: 0, - role: role.as_ptr(), - name: bad_name.as_ptr() as *const c_char, - description: std::ptr::null(), - bounds_hash: 0, - has_bounds_hash: false, - }; + let mut entry = default_ref_entry(); + entry.role = role.as_ptr(); + entry.name = bad_name.as_ptr() as *const c_char; let mut out = AdNativeHandle { ptr: std::ptr::null(), }; @@ -60,14 +55,9 @@ fn resolve_element_rejects_invalid_utf8_description() { with_adapter(|adapter| unsafe { let role = std::ffi::CString::new("button").unwrap(); let bad_description: [u8; 2] = [0xC3, 0x00]; - let entry = AdRefEntry { - pid: 0, - role: role.as_ptr(), - name: std::ptr::null(), - description: bad_description.as_ptr() as *const c_char, - bounds_hash: 0, - has_bounds_hash: false, - }; + let mut entry = default_ref_entry(); + entry.role = role.as_ptr(); + entry.description = bad_description.as_ptr() as *const c_char; let mut out = AdNativeHandle { ptr: std::ptr::null(), }; diff --git a/crates/ffi/tests/common/mod.rs b/crates/ffi/tests/common/mod.rs index 5892d2be..b706d2cd 100644 --- a/crates/ffi/tests/common/mod.rs +++ b/crates/ffi/tests/common/mod.rs @@ -52,6 +52,13 @@ unsafe extern "C" { policy: i32, out: *mut AdActionResult, ) -> AdResult; + pub fn ad_execute_ref_action_with_policy( + adapter: *const AdAdapter, + entry: *const AdRefEntry, + action: *const AdAction, + policy: i32, + out: *mut AdActionResult, + ) -> AdResult; pub fn ad_find( adapter: *const AdAdapter, @@ -78,6 +85,37 @@ pub fn with_adapter(body: F) { } } +pub fn default_ref_entry() -> AdRefEntry { + AdRefEntry { + pid: 0, + role: std::ptr::null(), + name: std::ptr::null(), + value: std::ptr::null(), + description: std::ptr::null(), + states: std::ptr::null(), + state_count: 0, + available_actions: std::ptr::null(), + available_action_count: 0, + bounds: AdRect { + x: 0.0, + y: 0.0, + width: 0.0, + height: 0.0, + }, + has_bounds: false, + bounds_hash: 0, + has_bounds_hash: false, + source_app: std::ptr::null(), + source_window_id: std::ptr::null(), + source_window_title: std::ptr::null(), + source_surface: 0, + root_ref: std::ptr::null(), + path_is_absolute: false, + path: std::ptr::null(), + path_count: 0, + } +} + pub fn default_action() -> AdAction { AdAction { kind: 0, diff --git a/docs/solutions/best-practices/playwright-grade-desktop-reliability-2026-06-02.md b/docs/solutions/best-practices/playwright-grade-desktop-reliability-2026-06-02.md new file mode 100644 index 00000000..ebfdafbb --- /dev/null +++ b/docs/solutions/best-practices/playwright-grade-desktop-reliability-2026-06-02.md @@ -0,0 +1,62 @@ +--- +title: Playwright-grade desktop reliability contract +date: 2026-06-02 +category: best-practices +module: crates/core +problem_type: best_practice +component: reliability +severity: high +applies_when: + - Ref resolution, action dispatch, wait semantics, session scope, or FFI action paths change + - A platform adapter is added or modified + - A command moves from direct adapter calls to shared helpers +tags: + - reliability + - playwright + - refs + - actionability + - cross-platform +--- + +# Playwright-grade desktop reliability contract + +## Context + +Playwright is reliable because actions flow through a consistent ladder: +resolve a locator, prove it is actionable, wait when the state is changing, and +fail with structured recovery when the target is stale or ambiguous. Desktop +automation cannot copy browser semantics directly, but it can use the same +engineering shape. + +## Contract + +Ref actions must pass through the shared reliability path: + +1. Load the refmap from the caller's session. +2. Resolve the ref with strict platform identity checks. +3. Return `STALE_REF` when the old element no longer matches. +4. Return `AMBIGUOUS_TARGET` when multiple candidates match. +5. Run actionability checks before adapter dispatch. +6. Emit trace events only to the requested JSONL trace path, never stdout. + +## Cross-Platform Rule + +Core owns the contract; adapters own native evidence. Windows and Linux should +not fork CLI semantics. UIA and AT-SPI implementations must map their native +identity fields into the same `RefEntry` concepts: role, name, value, +description, state, bounds, supported actions, source surface, root ref, and +tree path. + +## Review Rule + +Any change to ref resolution or action dispatch must include tests for: + +- stale ref rejection +- ambiguous target rejection +- actionability failure before dispatch +- session isolation +- FFI parity when the behavior is exposed through C ABI + +If a platform needs a coordinate fallback, the fallback must be explicit and +lower confidence. Do not silently replace a failed semantic action with a pixel +click. diff --git a/skills/agent-desktop/SKILL.md b/skills/agent-desktop/SKILL.md index 8427a3c0..fde64611 100644 --- a/skills/agent-desktop/SKILL.md +++ b/skills/agent-desktop/SKILL.md @@ -91,6 +91,10 @@ Use **progressive skeleton traversal** as the default approach. It reduces token - `last_refmap.json` is only a latest-snapshot inspection artifact. The command path uses snapshot-scoped storage. - After any action that changes UI, re-drill the affected region or re-snapshot - **Scoped invalidation:** re-drilling `--root @e3` only replaces refs from @e3's previous drill — refs from other regions and the skeleton itself are preserved +- **Strict resolution:** stale refs return `STALE_REF`; duplicate plausible targets return `AMBIGUOUS_TARGET` instead of choosing arbitrarily. +- **Actionability:** ref actions check visibility, enabled state, supported action, policy, and editability before dispatch. +- **Sessions:** use `--session ` for concurrent or multi-agent runs; batch entries may override with `"session": "id"`. +- **Trace:** use `--trace ` for JSONL diagnostics outside stdout; add `--trace-strict` only when trace write failures should fail the command. ## JSON Output Contract @@ -111,6 +115,7 @@ Exit codes: `0` success, `1` structured error, `2` argument error. | `ACTION_FAILED` | AX action rejected | Try an explicit alternative command | | `ACTION_NOT_SUPPORTED` | Element can't do this | Use different command | | `STALE_REF` | Ref from old snapshot | Re-run snapshot | +| `AMBIGUOUS_TARGET` | Multiple elements matched the old ref identity | Re-run snapshot and choose a more specific ref | | `SNAPSHOT_NOT_FOUND` | Snapshot ID is missing or expired | Run `snapshot` again and use the returned ID | | `POLICY_DENIED` | A physical/headed path was blocked | Use an explicit mouse/focus/keyboard command if physical interaction is intended | | `WINDOW_NOT_FOUND` | No matching window | Check app name, use list-windows | @@ -206,6 +211,7 @@ agent-desktop clipboard-clear # Clear clipboard ``` agent-desktop wait 1000 # Pause 1 second agent-desktop wait --element @e5 --snapshot --timeout 5000 # Wait for element +agent-desktop wait --element @e5 --predicate actionable --timeout 5000 # Wait until actionable agent-desktop wait --window "Title" # Wait for window agent-desktop wait --text "Done" --app "App" # Wait for text agent-desktop wait --menu --app "App" # Wait for menu surface @@ -237,3 +243,5 @@ agent-desktop skills get desktop --full # Load this skill + all referenc 9. **Use surfaces for overlays.** `snapshot --surface menu` for menus, `--surface sheet` for dialogs. Never `--skeleton` for surfaces — they're already focused. 10. **Batch for performance.** Multiple commands in one invocation. 11. **Headless by default.** Ref actions use semantic AX paths and block silent focus stealing, cursor movement, keyboard synthesis, and pasteboard insertion. Use explicit `focus`, `press`, `hover`, `drag`, or `mouse-*` commands only when physical/headed interaction is intended. +12. **Use sessions for parallel work.** Add `--session ` when multiple agents or batches can run at once. +13. **Trace hard failures.** Add `--trace /tmp/agent-desktop.jsonl` when diagnosing stale, ambiguous, or actionability failures. diff --git a/skills/agent-desktop/references/commands-system.md b/skills/agent-desktop/references/commands-system.md index 35577d0d..e683dd46 100644 --- a/skills/agent-desktop/references/commands-system.md +++ b/skills/agent-desktop/references/commands-system.md @@ -201,6 +201,9 @@ Blocks until the menu surface is dismissed. | (positional) | | Milliseconds to pause | | `--element` | | Ref to wait for | | `--snapshot` | latest | Snapshot ID for `--element` waits | +| `--predicate` | exists | Element predicate: `exists`, `enabled`, `visible`, `actionable`, `value` | +| `--value` | | Expected text for `--predicate value` | +| `--count` | | Expected match count for `--text` waits | | `--window` | | Window title to wait for | | `--text` | | Text to wait for; with `--notification`, filters notification title/body | | `--menu` | false | Wait for menu surface to open | @@ -215,11 +218,14 @@ Blocks until the menu surface is dismissed. ```bash agent-desktop batch '[{"command":"click","args":{"ref_id":"@e1","snapshot":""}},{"command":"wait","args":{"ms":500}},{"command":"click","args":{"ref_id":"@e2","snapshot":""}}]' agent-desktop batch '[...]' --stop-on-error +agent-desktop --session run-a batch '[{"command":"status","session":"run-b","args":{}}]' ``` Execute multiple commands in sequence from a JSON array. Each entry has `command` (string) and `args` (object). Use `args`, not `params`. For ref-consuming commands, pass the output `snapshot_id` as the `snapshot` field. Batch uses the same typed `Commands` enum, command policy preflight, permission report, and dispatch path as the CLI. Unknown fields are rejected instead of being silently ignored. Nested `batch` is rejected. +Each entry may include `"session": "id"` beside `command` and `args`. If omitted, the entry inherits the top-level `--session`. Use per-entry sessions only when intentionally inspecting or coordinating separate agent runs. + | Flag | Default | Description | |------|---------|-------------| | `--stop-on-error` | false | Halt on first failed command | @@ -229,7 +235,8 @@ Batch uses the same typed `Commands` enum, command policy preflight, permission [ { "command": "click", "args": { "ref_id": "@e1", "snapshot": "" } }, { "command": "wait", "args": { "ms": 500 } }, - { "command": "type", "args": { "ref_id": "@e2", "snapshot": "", "text": "hello" } } + { "command": "type", "args": { "ref_id": "@e2", "snapshot": "", "text": "hello" } }, + { "command": "status", "session": "other-agent", "args": {} } ] ``` diff --git a/src/batch.rs b/src/batch.rs index 7c55f267..e8aabbd1 100644 --- a/src/batch.rs +++ b/src/batch.rs @@ -2,6 +2,7 @@ use agent_desktop_core::{ PermissionReport, adapter::PlatformAdapter, commands::batch::BatchCommand, + context::CommandContext, error::AppError, output::{ENVELOPE_VERSION, ErrorPayload}, }; @@ -19,15 +20,17 @@ pub(crate) fn execute( args: BatchArgs, adapter: &dyn PlatformAdapter, permission_report: &PermissionReport, + context: &CommandContext, ) -> Result { let commands = agent_desktop_core::commands::batch::parse_commands(&args.commands_json)?; let mut results = Vec::new(); for item in commands { let command = item.command.clone(); + let item_context = context.for_batch_item(item.session.clone())?; let result = parse_command(item).and_then(|typed| { crate::command_policy::preflight(&typed, permission_report)?; - crate::dispatch::dispatch(typed, adapter, permission_report) + crate::dispatch::dispatch(typed, adapter, permission_report, &item_context) }); let ok = result.is_ok(); results.push(batch_entry(&command, result)); diff --git a/src/batch_tests.rs b/src/batch_tests.rs index 2d6bbcca..f0868b9c 100644 --- a/src/batch_tests.rs +++ b/src/batch_tests.rs @@ -9,10 +9,21 @@ impl PlatformAdapter for NoopAdapter {} fn item(command: &str, args: Value) -> BatchCommand { BatchCommand { command: command.to_string(), + session: None, args, } } +#[test] +fn parses_optional_batch_session_scope() { + let commands = agent_desktop_core::commands::batch::parse_commands( + r#"[{"command":"status","session":"agent-a","args":{}}]"#, + ) + .unwrap(); + + assert_eq!(commands[0].session.as_deref(), Some("agent-a")); +} + #[test] fn parses_ref_command_into_cli_enum() { let command = @@ -63,7 +74,13 @@ fn stop_on_error_halts_after_first_failure() { stop_on_error: true, }; - let value = execute(args, &NoopAdapter, &PermissionReport::default()).unwrap(); + let value = execute( + args, + &NoopAdapter, + &PermissionReport::default(), + &agent_desktop_core::CommandContext::default(), + ) + .unwrap(); let results = value["results"].as_array().unwrap(); assert_eq!(results.len(), 1); diff --git a/src/cli.rs b/src/cli.rs index 223c5e0f..4524dbd6 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,4 +1,5 @@ use clap::{Parser, Subcommand}; +use std::path::PathBuf; use crate::cli_args::{ FindArgs, GetArgs, IsArgs, ListSurfacesArgs, RefArgs, ScreenshotArgs, SnapshotArgs, @@ -38,6 +39,23 @@ pub(crate) struct Cli { )] pub verbose: bool, + #[arg(long, global = true, help = "Scope ref snapshots to a session id")] + pub session: Option, + + #[arg( + long, + global = true, + help = "Append reliability trace JSONL to this path" + )] + pub trace: Option, + + #[arg( + long, + global = true, + help = "Fail the command if writing --trace fails" + )] + pub trace_strict: bool, + #[command(subcommand)] pub command: Option, } diff --git a/src/command_contract_tests.rs b/src/command_contract_tests.rs index 6332ffc2..f17cd545 100644 --- a/src/command_contract_tests.rs +++ b/src/command_contract_tests.rs @@ -3,7 +3,7 @@ use clap::CommandFactory; use std::collections::BTreeSet; use std::path::{Path, PathBuf}; -const NON_COMMAND_MODULES: &[&str] = &["helpers", "mod", "wait_predicate"]; +const NON_COMMAND_MODULES: &[&str] = &["helpers", "mod", "wait_latest_ref_cache", "wait_predicate"]; const COMMAND_SPECIFIC_TESTS: &[&str] = &[ "find", diff --git a/src/dispatch.rs b/src/dispatch.rs index 859b8b5c..13484017 100644 --- a/src/dispatch.rs +++ b/src/dispatch.rs @@ -9,6 +9,7 @@ use agent_desktop_core::{ resize_window, restore, right_click, screenshot, scroll, scroll_to, select, set_value, skills, snapshot, status, toggle, triple_click, type_text, uncheck, version, wait, }, + context::CommandContext, error::AppError, }; use serde_json::Value; @@ -24,10 +25,11 @@ pub(crate) fn dispatch( cmd: Commands, adapter: &dyn PlatformAdapter, permission_report: &PermissionReport, + context: &CommandContext, ) -> Result { tracing::debug!("dispatch: {}", cmd.name()); match cmd { - Commands::Snapshot(a) => snapshot::execute( + Commands::Snapshot(a) => snapshot::execute_with_context( snapshot::SnapshotArgs { app: a.app, window_id: a.window_id, @@ -41,6 +43,7 @@ pub(crate) fn dispatch( snapshot_id: a.snapshot, }, adapter, + context, ), Commands::Find(a) => find::execute( @@ -57,6 +60,7 @@ pub(crate) fn dispatch( limit: a.limit, }, adapter, + context, ), Commands::Screenshot(a) => screenshot::execute( @@ -75,21 +79,23 @@ pub(crate) fn dispatch( property: parse_get_property(&a.property)?, }, adapter, + context, ), - Commands::Is(a) => is_check::execute( + Commands::Is(a) => is_check::execute_with_context( is_check::IsArgs { ref_id: a.ref_id, snapshot_id: a.snapshot, property: parse_is_property(&a.property)?, }, adapter, + context, ), - Commands::Click(a) => click::execute(ref_args(a), adapter), - Commands::DoubleClick(a) => double_click::execute(ref_args(a), adapter), - Commands::TripleClick(a) => triple_click::execute(ref_args(a), adapter), - Commands::RightClick(a) => right_click::execute(ref_args(a), adapter), + Commands::Click(a) => click::execute(ref_args(a), adapter, context), + Commands::DoubleClick(a) => double_click::execute(ref_args(a), adapter, context), + Commands::TripleClick(a) => triple_click::execute(ref_args(a), adapter, context), + Commands::RightClick(a) => right_click::execute_with_context(ref_args(a), adapter, context), Commands::Type(a) => type_text::execute( type_text::TypeArgs { @@ -98,6 +104,7 @@ pub(crate) fn dispatch( text: a.text, }, adapter, + context, ), Commands::SetValue(a) => set_value::execute( @@ -107,16 +114,17 @@ pub(crate) fn dispatch( value: a.value, }, adapter, + context, ), - Commands::Clear(a) => clear::execute(ref_args(a), adapter), + Commands::Clear(a) => clear::execute(ref_args(a), adapter, context), - Commands::Focus(a) => focus::execute(ref_args(a), adapter), - Commands::Toggle(a) => toggle::execute(ref_args(a), adapter), - Commands::Check(a) => check::execute(ref_args(a), adapter), - Commands::Uncheck(a) => uncheck::execute(ref_args(a), adapter), - Commands::Expand(a) => expand::execute(ref_args(a), adapter), - Commands::Collapse(a) => collapse::execute(ref_args(a), adapter), + Commands::Focus(a) => focus::execute(ref_args(a), adapter, context), + Commands::Toggle(a) => toggle::execute(ref_args(a), adapter, context), + Commands::Check(a) => check::execute(ref_args(a), adapter, context), + Commands::Uncheck(a) => uncheck::execute(ref_args(a), adapter, context), + Commands::Expand(a) => expand::execute(ref_args(a), adapter, context), + Commands::Collapse(a) => collapse::execute(ref_args(a), adapter, context), Commands::Select(a) => select::execute( select::SelectArgs { @@ -125,6 +133,7 @@ pub(crate) fn dispatch( value: a.value, }, adapter, + context, ), Commands::Scroll(a) => scroll::execute( @@ -135,9 +144,10 @@ pub(crate) fn dispatch( amount: a.amount, }, adapter, + context, ), - Commands::ScrollTo(a) => scroll_to::execute(ref_args(a), adapter), + Commands::ScrollTo(a) => scroll_to::execute(ref_args(a), adapter, context), Commands::Press(a) => press::execute( press::PressArgs { @@ -161,6 +171,7 @@ pub(crate) fn dispatch( duration_ms: a.duration, }, adapter, + context, ), Commands::Drag(a) => drag::execute( @@ -173,6 +184,7 @@ pub(crate) fn dispatch( duration_ms: a.duration, }, adapter, + context, ), Commands::MouseMove(a) => { @@ -289,7 +301,7 @@ pub(crate) fn dispatch( Commands::ClipboardSet(a) => clipboard_set::execute(a.text, adapter), Commands::ClipboardClear => clipboard_clear::execute(adapter), - Commands::Wait(a) => wait::execute( + Commands::Wait(a) => wait::execute_with_context( wait::WaitArgs { ms: a.ms, element: a.element, @@ -306,9 +318,12 @@ pub(crate) fn dispatch( app: a.app, }, adapter, + context, ), - Commands::Status => status::execute_with_report(adapter, permission_report), + Commands::Status => { + status::execute_with_report_with_context(adapter, permission_report, context) + } Commands::Permissions(a) => permissions::execute_with_report( permissions::PermissionsArgs { request: a.request }, @@ -328,7 +343,7 @@ pub(crate) fn dispatch( }), }, - Commands::Batch(a) => crate::batch::execute(a, adapter, permission_report), + Commands::Batch(a) => crate::batch::execute(a, adapter, permission_report, context), } } diff --git a/src/help_after.txt b/src/help_after.txt index 73b6c8d7..8bdb4d8b 100644 --- a/src/help_after.txt +++ b/src/help_after.txt @@ -63,6 +63,8 @@ CLIPBOARD WAIT wait [ms] Pause for N milliseconds wait --element Block until element appears (--timeout ms) + wait --element --predicate actionable Block until element is actionable + wait --element --predicate value --value Block until value matches wait --element --snapshot Wait against a specific snapshot refmap wait --window Block until window appears wait --text <text> Block until text appears in app @@ -78,12 +80,19 @@ SYSTEM BATCH batch <json> Run commands from a JSON array (--stop-on-error) + batch items may set "session": "id" to override the inherited --session REF IDs snapshot assigns @e1, @e2, ... to interactive elements in depth-first order. Use a ref wherever <ref> appears. Refs are snapshot-scoped. Pass --snapshot <snapshot_id> to pin a command to a known snapshot; when omitted, ref commands use the latest saved snapshot. Run snapshot again after UI changes. + Use --session <id> to isolate latest snapshots for concurrent agents. + Ref actions use strict resolution: stale targets return STALE_REF; duplicate + plausible targets return AMBIGUOUS_TARGET instead of choosing arbitrarily. + Ref actions run actionability checks before dispatch. Use --trace <path> to + write JSONL diagnostics outside stdout; add --trace-strict to fail on trace + write errors. KEY COMBOS Single keys: return, escape, tab, space, delete, up, down, left, right @@ -93,12 +102,12 @@ KEY COMBOS EXAMPLES agent-desktop skills get desktop --full Load the primary skill (start here) - agent-desktop snapshot --app "System Settings" -i + agent-desktop --session task-1 snapshot --app "System Settings" -i agent-desktop find --role button --name "OK" agent-desktop find --role button --limit 20 - agent-desktop click @e5 - agent-desktop check @e3 - agent-desktop type @e2 "hello@example.com" + agent-desktop click @e5 --snapshot <snapshot_id> + agent-desktop check @e3 --snapshot <snapshot_id> + agent-desktop type @e2 --snapshot <snapshot_id> "hello@example.com" agent-desktop press cmd+z agent-desktop drag --from @e1 --to @e5 agent-desktop hover @e5 @@ -106,4 +115,17 @@ EXAMPLES agent-desktop resize-window --app TextEdit --width 800 --height 600 agent-desktop mouse-click --xy 500,300 agent-desktop wait --text "Loading complete" --app Safari --timeout 5000 + agent-desktop wait --element @e5 --predicate actionable --timeout 5000 + agent-desktop --session run-a --trace /tmp/ad.jsonl click @e5 agent-desktop batch '[{"command":"click","args":{"ref_id":"@e1","snapshot":"<snapshot_id>"}}]' + agent-desktop --session run-a batch '[{"command":"status","session":"run-b","args":{}}]' + +AGENT LOOP + 1. Use `skills get desktop --full` when you need detailed guidance. + 2. Use `--session <id>` when another agent or task may also run commands. + 3. Observe with `snapshot -i` or `snapshot --skeleton -i --compact`. + 4. Keep `snapshot_id`; pass it to every ref-consuming action. + 5. Use `wait --predicate actionable`, `wait --text`, or `wait --window` + instead of sleeping blindly. + 6. On STALE_REF or AMBIGUOUS_TARGET, snapshot again and choose a fresh ref. + 7. Add `--trace <path>` when diagnosing stale, ambiguous, or actionability failures. diff --git a/src/help_before.txt b/src/help_before.txt index e7b0f49a..1ad247b4 100644 --- a/src/help_before.txt +++ b/src/help_before.txt @@ -2,11 +2,11 @@ For AI agents — read this first: agent-desktop skills get desktop --full Bundled skill docs travel with the binary, so they always match this - exact version. They cover the snapshot/ref loop, JSON envelope contract, - error codes with recovery hints, and copy-paste patterns for forms, - menus, dialogs, and async waits. Specialized skills go deeper on macOS - internals (TCC, AX API, surfaces, Notification Center) and on embedding - agent-desktop through its C ABI. + exact version. They cover the snapshot/ref loop, session-scoped refs, + actionability, trace JSONL, error codes with recovery hints, and + copy-paste patterns for forms, menus, dialogs, and async waits. + Specialized skills go deeper on macOS internals (TCC, AX API, surfaces, + Notification Center) and on embedding agent-desktop through its C ABI. skills List bundled skills with summaries skills get desktop Primary guide — overview + the observe-act loop diff --git a/src/main.rs b/src/main.rs index 693f4e46..c0955a22 100644 --- a/src/main.rs +++ b/src/main.rs @@ -14,6 +14,7 @@ mod dispatch_parse; use agent_desktop_core::{ adapter::PlatformAdapter, + context::CommandContext, error::AppError, output::{ENVELOPE_VERSION, ErrorPayload, Response}, }; @@ -43,6 +44,13 @@ fn main() { }; init_tracing(cli.verbose); + let context = match CommandContext::new(cli.session, cli.trace, cli.trace_strict) { + Ok(context) => context, + Err(err) => { + finish("unknown", Err(err)); + return; + } + }; let cmd = match cli.command { Some(c) => c, @@ -75,11 +83,11 @@ fn main() { }; finish(cmd_name, result); } - cmd => run_with_adapter(cmd, cmd_name), + cmd => run_with_adapter(cmd, cmd_name, &context), } } -fn run_with_adapter(cmd: Commands, cmd_name: &str) { +fn run_with_adapter(cmd: Commands, cmd_name: &str, context: &CommandContext) { let adapter = build_adapter(); let report = adapter.permission_report(); if let Err(err) = command_policy::preflight(&cmd, &report) { @@ -87,7 +95,7 @@ fn run_with_adapter(cmd: Commands, cmd_name: &str) { return; } - let result = dispatch::dispatch(cmd, &adapter, &report); + let result = dispatch::dispatch(cmd, &adapter, &report, context); finish(cmd_name, result); } diff --git a/tests/conformance/README.md b/tests/conformance/README.md new file mode 100644 index 00000000..c66172e2 --- /dev/null +++ b/tests/conformance/README.md @@ -0,0 +1,38 @@ +# Adapter Reliability Conformance + +Every platform adapter must satisfy the same ref/action contract. The macOS +adapter is the first implementation, but the tests are written against +`PlatformAdapter` semantics so Windows UIA and Linux AT-SPI can reuse the same +expectations. + +## Required Gates + +| Area | Required behavior | +|------|-------------------| +| Snapshot refs | Refs are depth-first, snapshot-scoped, and persisted in the caller session | +| Strict resolve | A ref resolves only when identity still matches; stale refs return `STALE_REF` | +| Ambiguity | Multiple plausible matches return `AMBIGUOUS_TARGET`, never an arbitrary click | +| Actionability | Ref actions check visibility, enabled state, supported action, policy, and editability before dispatch | +| Wait recovery | `wait --element` can poll the latest session refmap when no snapshot is pinned | +| Session scope | `--session <id>` and batch item `session` never read or write another session's refmap | +| Trace | `--trace <path>` writes JSONL diagnostics outside stdout and is best-effort unless strict | +| FFI parity | FFI ref actions use strict resolve and actionability checks before adapter dispatch | + +## Platform Matrix + +| Fixture | macOS AX | Windows UIA | Linux AT-SPI | +|---------|:--------:|:-----------:|:------------:| +| Two identical buttons produce ambiguity | Required | Required | Required | +| Ref disappears after snapshot | Required | Required | Required | +| Disabled button blocks click before dispatch | Required | Required | Required | +| Text field supports value/type actionability | Required | Required | Required | +| Session A latest refmap is invisible to Session B | Required | Required | Required | +| Batch item session overrides inherited session | Required | Required | Required | +| FFI `AdRefEntry` preserves full identity envelope | Required | Required | Required | + +## Adding Windows or Linux + +Add adapter-specific integration fixtures, but keep expected errors and JSON +shapes identical. Prefer semantic platform actions first (`AXPress`, UIA +Invoke/Value/Selection patterns, AT-SPI actions). Coordinate input is a lower +confidence fallback and must remain explicit in policy or command choice.