Creepso/agent-desktop
1
0
Fork 0
mirror of https://github.com/lahfir/agent-desktop.git synced 2026-08-19 05:25:43 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix: close reliability review gaps

Browse source
This commit is contained in:
Lahfir 2026-06-17 11:22:05 -07:00
parent e0ba3d7d69
commit 1f8e283119
25 changed files with 498 additions and 187 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
crates/core/src/commands/find.rs
View file

@ -188,10 +188,11 @@ fn search_tree(
.map(String::from)
.unwrap_or_else(|| format!("(unnamed {})", node.role));
matches.push(json!({
"ref": node.ref_id,
"ref_id": node.ref_id,
"role": node.role,
"name": display_name,
"value": node.value,
"states": node.states,
"interactive": interactive,
"path": path.clone()
}));

19
crates/core/src/commands/find_tests.rs
View file

@ -32,6 +32,25 @@ fn display_name_prefers_value_before_description() {
assert_eq!(matches[0]["name"], "current value");
}
#[test]
fn search_tree_match_uses_ref_id_contract_and_includes_states() {
let mut root = node(Some("Save"), None, None);
root.states = vec!["enabled".into()];
let query = FindQuery {
role: None,
name: None,
value: None,
text: None,
};
let mut matches = Vec::new();
search_tree(&root, &query, &mut Vec::new(), &mut matches, None);
assert_eq!(matches[0]["ref_id"], "@e1");
assert!(matches[0].get("ref").is_none());
assert_eq!(matches[0]["states"], serde_json::json!(["enabled"]));
}
#[test]
fn search_tree_matches_text_across_fields() {
let root = node(None, Some("Primary"), Some("Secondary"));

24
crates/core/src/commands/wait_latest_ref_cache.rs
View file

@ -4,23 +4,23 @@ use std::time::{Duration, Instant};
pub(crate) struct LatestRefCache<'a> {
store: &'a RefStore,
snapshot_id: Option<String>,
missing_latest_snapshot_id: Option<String>,
refmap: RefMap,
last_refresh: Instant,
}
impl<'a> LatestRefCache<'a> {
pub(crate) fn new(store: &'a RefStore) -> Result<Self, AppError> {
let snapshot_id = store.latest_snapshot_id();
let mut snapshot_id = store.latest_snapshot_id();
let refmap = if let Some(id) = snapshot_id.as_deref() {
store.load_snapshot(id)?
} else {
store.load_latest()?
let refmap = store.load_latest()?;
snapshot_id = store.latest_snapshot_id();
refmap
};
Ok(Self {
store,
snapshot_id,
missing_latest_snapshot_id: None,
refmap,
last_refresh: Instant::now() - Duration::from_millis(500),
})
@ -35,17 +35,9 @@ impl<'a> LatestRefCache<'a> {
return Ok(());
}
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 Ok(());
}
if self.missing_latest_snapshot_id.as_deref() == Some(snapshot_id.as_str()) {
return Ok(());
}
match self.store.load_snapshot(&snapshot_id) {
if let Some(snapshot_id) = self.snapshot_id.as_deref() {
match self.store.load_snapshot(snapshot_id) {
Ok(refmap) => {
self.snapshot_id = Some(snapshot_id);
self.missing_latest_snapshot_id = None;
self.refmap = refmap;
Ok(())
}
@ -53,9 +45,6 @@ impl<'a> LatestRefCache<'a> {
tracing::warn!(
"latest snapshot {snapshot_id} unreadable during wait refresh: {err}"
);
if err.code() == "SNAPSHOT_NOT_FOUND" {
self.missing_latest_snapshot_id = Some(snapshot_id);
}
Ok(())
}
}
@ -64,7 +53,6 @@ impl<'a> LatestRefCache<'a> {
Ok(refmap) => {
self.refmap = refmap;
self.snapshot_id = self.store.latest_snapshot_id();
self.missing_latest_snapshot_id = None;
Ok(())
}
Err(err) => {

29
crates/core/src/commands/wait_latest_ref_cache_tests.rs
View file

@ -37,7 +37,7 @@ fn refmap_with_ref(pid: i32, name: Option<&str>) -> RefMap {
}
#[test]
fn latest_ref_cache_picks_up_newer_snapshot_after_refresh() {
fn latest_ref_cache_pins_starting_snapshot_when_latest_advances() {
let _guard = HomeGuard::new();
let first_id = save_ref(1, Some("First"));
let store = RefStore::new().unwrap();
@ -51,8 +51,12 @@ fn latest_ref_cache_picks_up_newer_snapshot_after_refresh() {
cache.last_refresh = std::time::Instant::now() - std::time::Duration::from_secs(2);
cache.refresh_if_due().unwrap();
assert_eq!(cache.snapshot_id.as_deref(), Some(second_id.as_str()));
assert!(cache.entry("@e1").is_some());
assert_eq!(cache.snapshot_id.as_deref(), Some(first_id.as_str()));
assert_eq!(
store.latest_snapshot_id().as_deref(),
Some(second_id.as_str())
);
assert_eq!(cache.entry("@e1").unwrap().pid, 1);
}
#[test]
@ -75,7 +79,7 @@ fn latest_ref_cache_debounces_consecutive_refreshes() {
}
#[test]
fn latest_ref_cache_keeps_last_good_map_when_new_latest_snapshot_disappears() {
fn latest_ref_cache_keeps_last_good_map_when_pinned_snapshot_disappears() {
let _guard = HomeGuard::new();
let first_id = save_ref(1, Some("First"));
let store = RefStore::new().unwrap();
@ -83,27 +87,22 @@ fn latest_ref_cache_keeps_last_good_map_when_new_latest_snapshot_disappears() {
let mut cache = LatestRefCache::new(&store).unwrap();
assert_eq!(cache.snapshot_id.as_deref(), Some(first_id.as_str()));
let second_id = save_ref(2, Some("Second"));
let home = crate::refs::home_dir().unwrap();
let snapshot_dir = home
.join(".agent-desktop")
.join("snapshots")
.join(&second_id);
.join(&first_id);
std::fs::remove_dir_all(snapshot_dir).unwrap();
cache.last_refresh = std::time::Instant::now() - std::time::Duration::from_secs(2);
cache.refresh_if_due().unwrap();
assert_eq!(cache.snapshot_id.as_deref(), Some(first_id.as_str()));
assert_eq!(
cache.missing_latest_snapshot_id.as_deref(),
Some(second_id.as_str())
);
assert_eq!(cache.entry("@e1").unwrap().pid, 1);
}
#[test]
fn latest_ref_cache_retries_unreadable_latest_snapshot_after_refresh_error() {
fn latest_ref_cache_retries_unreadable_pinned_snapshot_after_refresh_error() {
let _guard = HomeGuard::new();
let first_id = save_ref(1, Some("First"));
let store = RefStore::new().unwrap();
@ -111,12 +110,11 @@ fn latest_ref_cache_retries_unreadable_latest_snapshot_after_refresh_error() {
let mut cache = LatestRefCache::new(&store).unwrap();
assert_eq!(cache.snapshot_id.as_deref(), Some(first_id.as_str()));
let second_id = save_ref(2, Some("Second"));
let refmap_path = crate::refs::home_dir()
.unwrap()
.join(".agent-desktop")
.join("snapshots")
.join(&second_id)
.join(&first_id)
.join("refmap.json");
std::fs::write(&refmap_path, b"{not-json").unwrap();
@ -124,15 +122,14 @@ fn latest_ref_cache_retries_unreadable_latest_snapshot_after_refresh_error() {
cache.refresh_if_due().unwrap();
assert_eq!(cache.snapshot_id.as_deref(), Some(first_id.as_str()));
assert!(cache.missing_latest_snapshot_id.is_none());
assert_eq!(cache.entry("@e1").unwrap().pid, 1);
store
.save_snapshot(&second_id, &refmap_with_ref(3, Some("Recovered")))
.save_snapshot(&first_id, &refmap_with_ref(3, Some("Recovered")))
.unwrap();
cache.last_refresh = std::time::Instant::now() - std::time::Duration::from_secs(2);
cache.refresh_if_due().unwrap();
assert_eq!(cache.snapshot_id.as_deref(), Some(second_id.as_str()));
assert_eq!(cache.snapshot_id.as_deref(), Some(first_id.as_str()));
assert_eq!(cache.entry("@e1").unwrap().pid, 3);
}

62
crates/core/src/refs_store_prune.rs
View file

@ -7,28 +7,40 @@ impl RefStore {
/// the temp write and the atomic rename. Runs under the store write lock;
/// the age threshold keeps any in-flight write from another process safe.
pub(crate) fn remove_tmp_files_older_than(&self, max_age: std::time::Duration) {
for dir in [self.base_dir.clone(), self.snapshots_dir()] {
let Ok(entries) = std::fs::read_dir(dir) else {
self.remove_tmp_files_in_dir(&self.base_dir, max_age);
let snapshots_dir = self.snapshots_dir();
self.remove_tmp_files_in_dir(&snapshots_dir, max_age);
let Ok(entries) = std::fs::read_dir(snapshots_dir) else {
return;
};
for entry in entries.flatten() {
if entry.file_type().is_ok_and(|kind| kind.is_dir()) {
self.remove_tmp_files_in_dir(&entry.path(), max_age);
}
}
}
fn remove_tmp_files_in_dir(&self, dir: &std::path::Path, max_age: std::time::Duration) {
let Ok(entries) = std::fs::read_dir(dir) else {
return;
};
for entry in entries.flatten() {
let path = entry.path();
if path.extension().is_none_or(|ext| ext != "tmp") {
continue;
};
for entry in entries.flatten() {
let path = entry.path();
if path.extension().is_none_or(|ext| ext != "tmp") {
continue;
}
let is_plain_file = entry.file_type().is_ok_and(|kind| kind.is_file());
if !is_plain_file {
continue;
}
let stale = entry
.metadata()
.ok()
.and_then(|metadata| metadata.modified().ok())
.and_then(|modified| modified.elapsed().ok())
.is_some_and(|age| age >= max_age);
if stale {
let _ = std::fs::remove_file(&path);
}
}
let is_plain_file = entry.file_type().is_ok_and(|kind| kind.is_file());
if !is_plain_file {
continue;
}
let stale = entry
.metadata()
.ok()
.and_then(|metadata| metadata.modified().ok())
.and_then(|modified| modified.elapsed().ok())
.is_some_and(|age| age >= max_age);
if stale {
let _ = std::fs::remove_file(&path);
}
}
}
@ -61,10 +73,14 @@ impl RefStore {
return Ok(());
}
snapshots.sort_by_key(|(modified, id, _)| (*modified, id.clone()));
let remove_count = snapshots.len() - MAX_SAVED_SNAPSHOTS;
for (_, id, path) in snapshots.into_iter().take(remove_count) {
let mut remove_count = snapshots.len() - MAX_SAVED_SNAPSHOTS;
for (_, id, path) in snapshots {
if remove_count == 0 {
break;
}
if id != latest_id {
let _ = std::fs::remove_dir_all(path);
remove_count -= 1;
}
}
Ok(())

32
crates/core/src/refs_store_tests.rs
View file

@ -327,13 +327,16 @@ fn stale_tmp_files_are_swept_and_fresh_ones_kept() {
let snapshot_id = store.save_new_snapshot(&map_with("Send")).unwrap();
let base_tmp = store.base_dir.join("latest_snapshot_id.tmp");
let snapshot_tmp = store.snapshots_dir().join("dead.tmp");
let refmap_tmp = store.snapshots_dir().join(&snapshot_id).join("refmap.tmp");
std::fs::write(&base_tmp, b"orphan").unwrap();
std::fs::write(&snapshot_tmp, b"orphan").unwrap();
std::fs::write(&refmap_tmp, b"orphan").unwrap();
store.remove_tmp_files_older_than(std::time::Duration::ZERO);
assert!(!base_tmp.exists());
assert!(!snapshot_tmp.exists());
assert!(!refmap_tmp.exists());
assert!(store.snapshot_path(&snapshot_id).is_file());
std::fs::write(&base_tmp, b"fresh").unwrap();
@ -342,6 +345,35 @@ fn stale_tmp_files_are_swept_and_fresh_ones_kept() {
assert!(base_tmp.exists());
}
#[test]
fn save_new_snapshot_prunes_old_snapshots_without_removing_latest() {
let _guard = HomeGuard::new();
let store = RefStore::new().unwrap();
let first_id = store.save_new_snapshot(&map_with("First")).unwrap();
std::thread::sleep(std::time::Duration::from_millis(5));
let mut latest_id = first_id.clone();
for i in 0..=MAX_SAVED_SNAPSHOTS {
latest_id = store
.save_new_snapshot(&map_with(&format!("Snapshot {i}")))
.unwrap();
}
let count = std::fs::read_dir(store.snapshots_dir())
.unwrap()
.filter_map(Result::ok)
.filter(|entry| entry.file_type().is_ok_and(|kind| kind.is_dir()))
.count();
assert!(count <= MAX_SAVED_SNAPSHOTS);
assert!(store.snapshot_path(&latest_id).is_file());
assert!(!store.snapshot_path(&first_id).exists());
assert_eq!(
store.latest_snapshot_id().as_deref(),
Some(latest_id.as_str())
);
}
#[test]
fn save_existing_recreates_snapshot_pruned_from_every_store() {
let _guard = HomeGuard::new();

19
crates/ffi/include/agent_desktop.h
View file

@ -221,6 +221,7 @@ typedef struct AdDragParams {
#define AD_DRAG_PARAMS_SIZE (sizeof(AdDragParams))
#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L
_Static_assert(sizeof(AdDragParams) == 48, "AdDragParams ABI size changed");
_Static_assert(_Alignof(AdDragParams) == 8, "AdDragParams ABI alignment changed");
#endif
uintptr_t ad_drag_params_size(void);
@ -251,6 +252,7 @@ typedef struct AdAction {
#define AD_ACTION_SIZE (sizeof(AdAction))
#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L
_Static_assert(sizeof(AdAction) == 96, "AdAction ABI size changed");
_Static_assert(_Alignof(AdAction) == 8, "AdAction ABI alignment changed");
#endif
uintptr_t ad_action_size(void);
@ -262,12 +264,28 @@ typedef struct AdElementState {
const char *value;
} AdElementState;
#define AD_ELEMENT_STATE_SIZE (sizeof(AdElementState))
#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L
_Static_assert(sizeof(AdElementState) == 32, "AdElementState ABI size changed");
_Static_assert(_Alignof(AdElementState) == 8, "AdElementState ABI alignment changed");
#endif
uintptr_t ad_element_state_size(void);
typedef struct AdActionResult {
const char *action;
const char *ref_id;
struct AdElementState *post_state;
} AdActionResult;
#define AD_ACTION_RESULT_SIZE (sizeof(AdActionResult))
#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L
_Static_assert(sizeof(AdActionResult) == 24, "AdActionResult ABI size changed");
_Static_assert(_Alignof(AdActionResult) == 8, "AdActionResult ABI alignment changed");
#endif
uintptr_t ad_action_result_size(void);
typedef struct AdRect {
double x;
double y;
@ -302,6 +320,7 @@ typedef struct AdRefEntry {
#define AD_REF_ENTRY_SIZE (sizeof(AdRefEntry))
#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L
_Static_assert(sizeof(AdRefEntry) == 192, "AdRefEntry ABI size changed");
_Static_assert(_Alignof(AdRefEntry) == 8, "AdRefEntry ABI alignment changed");
#endif
/*

69
crates/ffi/src/actions/execute.rs
View file

@ -4,7 +4,7 @@ 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, AdRefEntry};
use agent_desktop_core::{action_request::ActionRequest, adapter::NativeHandle};
use agent_desktop_core::{action::Action, action_request::ActionRequest, adapter::NativeHandle};
/// # Safety
///
@ -56,24 +56,14 @@ pub unsafe extern "C" fn ad_execute_action_with_policy(
));
return AdResult::ErrInvalidArgs;
}
let action_ref = &*action;
let core_action = match action_from_c(action_ref) {
Ok(a) => a,
Err(msg) => {
error::set_last_error(&agent_desktop_core::error::AdapterError::new(
agent_desktop_core::error::ErrorCode::InvalidArgs,
msg,
));
return AdResult::ErrInvalidArgs;
}
let core_action = match decode_action(&*action) {
Ok(action) => action,
Err(result) => return result,
};
let native_handle = NativeHandle::from_ptr(handle_ref.ptr);
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 policy = match decode_policy(policy) {
Ok(policy) => policy,
Err(result) => return result,
};
let request = action_request(policy, core_action);
match adapter.inner.execute_action(&native_handle, request) {
@ -121,23 +111,13 @@ pub unsafe extern "C" fn ad_execute_ref_action_with_policy(
return error::last_error_code();
}
};
let action_ref = &*action;
let core_action = match action_from_c(action_ref) {
let core_action = match decode_action(&*action) {
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;
}
Err(result) => return result,
};
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 policy = match decode_policy(policy) {
Ok(policy) => policy,
Err(result) => return result,
};
let request = action_request(policy, core_action);
match agent_desktop_core::ref_action::execute_entry(
@ -157,10 +137,27 @@ pub unsafe extern "C" fn ad_execute_ref_action_with_policy(
})
}
fn action_request(
policy: AdPolicyKind,
action: agent_desktop_core::action::Action,
) -> ActionRequest {
fn decode_action(action: &AdAction) -> Result<Action, AdResult> {
unsafe { action_from_c(action) }.map_err(|msg| {
error::set_last_error(&agent_desktop_core::error::AdapterError::new(
agent_desktop_core::error::ErrorCode::InvalidArgs,
msg,
));
AdResult::ErrInvalidArgs
})
}
fn decode_policy(policy: i32) -> Result<AdPolicyKind, AdResult> {
AdPolicyKind::from_c(policy).ok_or_else(|| {
error::set_last_error(&agent_desktop_core::error::AdapterError::new(
agent_desktop_core::error::ErrorCode::InvalidArgs,
"invalid policy kind discriminant",
));
AdResult::ErrInvalidArgs
})
}
fn action_request(policy: AdPolicyKind, action: Action) -> ActionRequest {
match policy {
AdPolicyKind::Headless => ActionRequest::headless(action),
AdPolicyKind::FocusFallback => ActionRequest::focus_fallback(action),

52
crates/ffi/src/actions/result.rs
View file

@ -3,6 +3,8 @@ use crate::types::{AdActionResult, AdElementState};
use agent_desktop_core::action_result::ActionResult as CoreActionResult;
use std::ptr;
const MAX_STATE_STRINGS_TO_FREE: usize = 1024;
pub(crate) fn action_result_to_c(r: &CoreActionResult) -> AdActionResult {
let action = string_to_c_lossy(&r.action);
let ref_id = opt_string_to_c(r.ref_id.as_deref());
@ -15,8 +17,9 @@ pub(crate) fn action_result_to_c(r: &CoreActionResult) -> AdActionResult {
let states = if state.states.is_empty() {
ptr::null_mut()
} else {
let ptrs: Vec<*mut std::os::raw::c_char> =
let mut ptrs: Vec<*mut std::os::raw::c_char> =
state.states.iter().map(|s| string_to_c_lossy(s)).collect();
ptrs.push(ptr::null_mut());
let mut boxed = ptrs.into_boxed_slice();
let raw = boxed.as_mut_ptr();
std::mem::forget(boxed);
@ -55,16 +58,8 @@ pub unsafe extern "C" fn ad_free_action_result(result: *mut AdActionResult) {
let state = &mut *r.post_state;
free_c_string(state.role as *mut _);
free_c_string(state.value as *mut _);
if !state.states.is_null() && state.state_count > 0 {
let slice =
std::slice::from_raw_parts_mut(state.states, state.state_count as usize);
for ptr in slice.iter() {
free_c_string(*ptr);
}
drop(Box::from_raw(std::ptr::slice_from_raw_parts_mut(
state.states,
state.state_count as usize,
)));
if !state.states.is_null() {
free_state_array(state.states);
}
drop(Box::from_raw(r.post_state));
r.post_state = ptr::null_mut();
@ -74,6 +69,20 @@ pub unsafe extern "C" fn ad_free_action_result(result: *mut AdActionResult) {
})
}
unsafe fn free_state_array(states: *mut *mut std::os::raw::c_char) {
unsafe {
let mut len = 0;
while len < MAX_STATE_STRINGS_TO_FREE && !(*states.add(len)).is_null() {
free_c_string(*states.add(len));
len += 1;
}
drop(Box::from_raw(std::ptr::slice_from_raw_parts_mut(
states,
len + 1,
)));
}
}
#[cfg(test)]
mod tests {
use super::*;
@ -106,6 +115,27 @@ mod tests {
unsafe { ad_free_action_result(&mut c_result) };
}
#[test]
fn free_action_result_ignores_mutated_state_count() {
let core_result = CoreActionResult {
action: "click".to_owned(),
ref_id: None,
post_state: Some(ElementState {
role: "button".to_owned(),
states: vec!["focused".to_owned()],
value: None,
}),
steps: Vec::new(),
};
let mut c_result = action_result_to_c(&core_result);
unsafe {
(*c_result.post_state).state_count = u32::MAX;
ad_free_action_result(&mut c_result);
}
assert!(c_result.post_state.is_null());
}
#[test]
fn test_free_null_action_result() {
unsafe { ad_free_action_result(ptr::null_mut()) };

27
crates/ffi/src/tree/flatten.rs
View file

@ -44,6 +44,7 @@ pub(crate) fn flatten_tree(root: &AccessibilityNode) -> AdNodeTree {
}
let count = flat.len() as u32;
flat.push(sentinel_node());
let nodes = if flat.is_empty() {
ptr::null_mut()
} else {
@ -55,6 +56,29 @@ pub(crate) fn flatten_tree(root: &AccessibilityNode) -> AdNodeTree {
AdNodeTree { nodes, count }
}
fn sentinel_node() -> AdNode {
AdNode {
ref_id: ptr::null(),
role: ptr::null(),
name: ptr::null(),
value: ptr::null(),
description: ptr::null(),
hint: ptr::null(),
states: ptr::null_mut(),
state_count: 0,
bounds: AdRect {
x: 0.0,
y: 0.0,
width: 0.0,
height: 0.0,
},
has_bounds: false,
parent_index: -1,
child_start: 0,
child_count: 0,
}
}
fn count_nodes(node: &AccessibilityNode) -> usize {
let mut total: usize = 0;
let mut queue: VecDeque<&AccessibilityNode> = VecDeque::new();
@ -103,8 +127,9 @@ fn strings_to_c_array(strings: &[String]) -> (*mut *mut c_char, u32) {
if strings.is_empty() {
return (ptr::null_mut(), 0);
}
let ptrs: Vec<*mut c_char> = strings.iter().map(|s| string_to_c_lossy(s)).collect();
let mut ptrs: Vec<*mut c_char> = strings.iter().map(|s| string_to_c_lossy(s)).collect();
let count = ptrs.len() as u32;
ptrs.push(ptr::null_mut());
let mut boxed = ptrs.into_boxed_slice();
let ptr = boxed.as_mut_ptr();
std::mem::forget(boxed);

78
crates/ffi/src/tree/free.rs
View file

@ -3,18 +3,22 @@ use crate::types::{AdNode, AdNodeTree};
use std::os::raw::c_char;
use std::ptr;
unsafe fn free_c_string_array(arr: *mut *mut c_char, count: u32) {
const MAX_NODE_STATE_STRINGS_TO_FREE: usize = 1024;
const MAX_TREE_NODES_TO_FREE: usize = 1_000_000;
unsafe fn free_c_string_array(arr: *mut *mut c_char) {
unsafe {
if arr.is_null() {
return;
}
let slice = std::slice::from_raw_parts_mut(arr, count as usize);
for p in slice.iter_mut() {
free_c_string(*p);
let mut len = 0;
while len < MAX_NODE_STATE_STRINGS_TO_FREE && !(*arr.add(len)).is_null() {
free_c_string(*arr.add(len));
len += 1;
}
drop(Box::from_raw(std::ptr::slice_from_raw_parts_mut(
arr,
count as usize,
len + 1,
)));
}
}
@ -27,7 +31,7 @@ unsafe fn free_node_fields(node: &mut AdNode) {
free_c_string(node.value as *mut c_char);
free_c_string(node.description as *mut c_char);
free_c_string(node.hint as *mut c_char);
free_c_string_array(node.states, node.state_count);
free_c_string_array(node.states);
node.ref_id = ptr::null();
node.role = ptr::null();
node.name = ptr::null();
@ -52,19 +56,30 @@ pub unsafe extern "C" fn ad_free_tree(tree: *mut AdNodeTree) {
if tree.nodes.is_null() {
return;
}
let nodes = std::slice::from_raw_parts_mut(tree.nodes, tree.count as usize);
let node_count = sentinel_node_count(tree.nodes);
let nodes = std::slice::from_raw_parts_mut(tree.nodes, node_count);
for node in nodes.iter_mut() {
free_node_fields(node);
}
drop(Box::from_raw(std::ptr::slice_from_raw_parts_mut(
tree.nodes,
tree.count as usize,
node_count + 1,
)));
tree.nodes = ptr::null_mut();
tree.count = 0;
})
}
unsafe fn sentinel_node_count(nodes: *mut AdNode) -> usize {
unsafe {
let mut count = 0;
while count < MAX_TREE_NODES_TO_FREE && !(*nodes.add(count)).role.is_null() {
count += 1;
}
count
}
}
#[cfg(test)]
mod tests {
use super::*;
@ -73,4 +88,51 @@ mod tests {
fn test_free_null_tree_is_noop() {
unsafe { ad_free_tree(std::ptr::null_mut()) };
}
#[test]
fn free_tree_ignores_mutated_node_state_count() {
let root = agent_desktop_core::node::AccessibilityNode {
ref_id: None,
role: "button".into(),
name: None,
value: None,
description: None,
hint: None,
states: vec!["focused".into()],
available_actions: vec![],
bounds: None,
children: vec![],
children_count: None,
};
let mut tree = crate::tree::flatten::flatten_tree(&root);
unsafe {
(*tree.nodes).state_count = u32::MAX;
ad_free_tree(&mut tree);
}
assert!(tree.nodes.is_null());
}
#[test]
fn free_tree_ignores_mutated_tree_count() {
let root = agent_desktop_core::node::AccessibilityNode {
ref_id: None,
role: "button".into(),
name: None,
value: None,
description: None,
hint: None,
states: vec![],
available_actions: vec![],
bounds: None,
children: vec![],
children_count: None,
};
let mut tree = crate::tree::flatten::flatten_tree(&root);
tree.count = u32::MAX;
unsafe { ad_free_tree(&mut tree) };
assert!(tree.nodes.is_null());
assert_eq!(tree.count, 0);
}
}

9
crates/ffi/src/types/action_result.rs
View file

@ -7,3 +7,12 @@ pub struct AdActionResult {
pub ref_id: *const c_char,
pub post_state: *mut AdElementState,
}
pub const AD_ACTION_RESULT_SIZE: usize = 24;
const _: () = assert!(std::mem::size_of::<AdActionResult>() == AD_ACTION_RESULT_SIZE);
#[unsafe(no_mangle)]
pub extern "C" fn ad_action_result_size() -> usize {
std::mem::size_of::<AdActionResult>()
}

9
crates/ffi/src/types/element_state.rs
View file

@ -7,3 +7,12 @@ pub struct AdElementState {
pub state_count: u32,
pub value: *const c_char,
}
pub const AD_ELEMENT_STATE_SIZE: usize = 32;
const _: () = assert!(std::mem::size_of::<AdElementState>() == AD_ELEMENT_STATE_SIZE);
#[unsafe(no_mangle)]
pub extern "C" fn ad_element_state_size() -> usize {
std::mem::size_of::<AdElementState>()
}

32
crates/ffi/tests/c_abi_layout.rs
View file

@ -1,6 +1,6 @@
mod common;
use common::{AdAction, AdPoint, AdRect, AdRefEntry};
use common::{AdAction, AdActionResult, AdElementState, AdPoint, AdRect, AdRefEntry};
use std::mem::{MaybeUninit, align_of, offset_of, size_of};
#[test]
@ -31,6 +31,36 @@ fn action_layout_is_guarded_for_c_consumers() {
assert_eq!(copied.drag.drop_delay_ms, 0);
}
#[test]
fn action_result_layout_is_guarded_for_c_consumers() {
assert_eq!(
agent_desktop_ffi::types::action_result::AD_ACTION_RESULT_SIZE,
24
);
assert_eq!(
unsafe { common::ad_action_result_size() },
agent_desktop_ffi::types::action_result::AD_ACTION_RESULT_SIZE
);
assert_eq!(size_of::<AdActionResult>(), 24);
assert_eq!(align_of::<AdActionResult>(), align_of::<usize>());
assert_eq!(offset_of!(AdActionResult, action), 0);
}
#[test]
fn element_state_layout_is_guarded_for_c_consumers() {
assert_eq!(
agent_desktop_ffi::types::element_state::AD_ELEMENT_STATE_SIZE,
32
);
assert_eq!(
unsafe { common::ad_element_state_size() },
agent_desktop_ffi::types::element_state::AD_ELEMENT_STATE_SIZE
);
assert_eq!(size_of::<AdElementState>(), 32);
assert_eq!(align_of::<AdElementState>(), align_of::<usize>());
assert_eq!(offset_of!(AdElementState, role), 0);
}
#[test]
fn rect_and_point_layouts_are_memcpyable() {
let rect = AdRect {

8
crates/ffi/tests/common/mod.rs
View file

@ -3,9 +3,9 @@
pub use agent_desktop_ffi::error::AdResult;
pub use agent_desktop_ffi::{
AdAction, AdActionResult, AdAdapter, AdAppList, AdDirection, AdDragParams, AdFindQuery,
AdKeyCombo, AdNativeHandle, AdPoint, AdPolicyKind, AdRect, AdRefEntry, AdScrollParams,
AdWindowInfo, AdWindowList,
AdAction, AdActionResult, AdAdapter, AdAppList, AdDirection, AdDragParams, AdElementState,
AdFindQuery, AdKeyCombo, AdNativeHandle, AdPoint, AdPolicyKind, AdRect, AdRefEntry,
AdScrollParams, AdWindowInfo, AdWindowList,
};
pub use std::ffi::CStr;
pub use std::os::raw::c_char;
@ -13,6 +13,8 @@ pub use std::os::raw::c_char;
unsafe extern "C" {
pub fn ad_ref_entry_size() -> usize;
pub fn ad_action_size() -> usize;
pub fn ad_action_result_size() -> usize;
pub fn ad_element_state_size() -> usize;
pub fn ad_adapter_create() -> *mut AdAdapter;
pub fn ad_adapter_destroy(adapter: *mut AdAdapter);

54
crates/macos/src/actions/chain_steps.rs
View file

@ -248,30 +248,13 @@ mod imp {
}
fn set_container_selection(candidate: &AXElement, attr: &str) -> bool {
use accessibility_sys::{AXUIElementSetAttributeValue, kAXErrorSuccess};
use core_foundation::{
array::CFArray,
base::{CFRetain, CFType, CFTypeRef, TCFType},
string::CFString,
};
let Some(container) = crate::tree::copy_element_attr(candidate, "AXParent") else {
return false;
};
if !ax_helpers::is_attr_settable(&container, attr) {
return false;
}
unsafe { CFRetain(candidate.0 as CFTypeRef) };
let candidate_cf = unsafe { CFType::wrap_under_create_rule(candidate.0 as CFTypeRef) };
let selected = CFArray::from_CFTypes(&[candidate_cf]);
let cf_attr = CFString::new(attr);
let err = unsafe {
AXUIElementSetAttributeValue(
container.0,
cf_attr.as_concrete_TypeRef(),
selected.as_CFTypeRef(),
)
};
err == kAXErrorSuccess
set_single_element_selection(&container, candidate, attr)
}
fn container_selection_contains(candidate: &AXElement, attr: &str) -> bool {
@ -289,12 +272,7 @@ mod imp {
el: &AXElement,
_caps: &ElementCaps,
) -> Result<bool, AdapterError> {
use accessibility_sys::{AXUIElementSetAttributeValue, kAXErrorSuccess, kAXRoleAttribute};
use core_foundation::{
array::CFArray,
base::{CFRetain, CFType, CFTypeRef, TCFType},
string::CFString,
};
use accessibility_sys::kAXRoleAttribute;
let Some(parent) = crate::tree::copy_element_attr(el, "AXParent") else {
return Ok(false);
};
@ -307,18 +285,32 @@ mod imp {
if !ax_helpers::is_attr_settable(&parent, "AXSelectedRows") {
return Ok(false);
}
unsafe { CFRetain(el.0 as CFTypeRef) };
let el_cf = unsafe { CFType::wrap_under_create_rule(el.0 as CFTypeRef) };
let arr = CFArray::from_CFTypes(&[el_cf]);
let cf_attr = CFString::new("AXSelectedRows");
Ok(set_single_element_selection(&parent, el, "AXSelectedRows"))
}
fn set_single_element_selection(
container: &AXElement,
element: &AXElement,
attr: &str,
) -> bool {
use accessibility_sys::{AXUIElementSetAttributeValue, kAXErrorSuccess};
use core_foundation::{
array::CFArray,
base::{CFRetain, CFType, CFTypeRef, TCFType},
string::CFString,
};
unsafe { CFRetain(element.0 as CFTypeRef) };
let element_cf = unsafe { CFType::wrap_under_create_rule(element.0 as CFTypeRef) };
let selected = CFArray::from_CFTypes(&[element_cf]);
let cf_attr = CFString::new(attr);
let err = unsafe {
AXUIElementSetAttributeValue(
parent.0,
container.0,
cf_attr.as_concrete_TypeRef(),
arr.as_CFTypeRef(),
selected.as_CFTypeRef(),
)
};
Ok(err == kAXErrorSuccess)
err == kAXErrorSuccess
}
pub(crate) fn try_custom_actions(

21
crates/macos/src/notifications/actions.rs
View file

@ -12,8 +12,7 @@ pub fn dismiss_notification(
) -> Result<NotificationInfo, AdapterError> {
let session = NcSession::open()?;
let result = dismiss_impl(index, app_filter);
session.close()?;
result
close_session(session, result)
}
pub fn dismiss_all(
@ -21,8 +20,7 @@ pub fn dismiss_all(
) -> Result<(Vec<NotificationInfo>, Vec<String>), AdapterError> {
let session = NcSession::open()?;
let result = dismiss_all_impl(app_filter);
session.close()?;
result
close_session(session, result)
}
pub fn notification_action(
@ -32,8 +30,19 @@ pub fn notification_action(
) -> Result<ActionResult, AdapterError> {
let session = NcSession::open()?;
let result = action_impl(index, identity, action_name);
session.close()?;
result
close_session(session, result)
}
fn close_session<T>(
session: NcSession,
result: Result<T, AdapterError>,
) -> Result<T, AdapterError> {
let close_result = session.close();
match (result, close_result) {
(Ok(value), Ok(())) => Ok(value),
(Ok(_), Err(err)) => Err(err),
(Err(err), _) => Err(err),
}
}
#[cfg(target_os = "macos")]

13
crates/macos/src/system/app_ops.rs
View file

@ -237,7 +237,18 @@ end tell"#
);
let mut command = Command::new("/usr/bin/osascript");
command.arg("-e").arg(script);
crate::system::process::run_with_timeout(&mut command, "osascript", QUIT_TIMEOUT)?;
let output =
crate::system::process::run_with_timeout(&mut command, "osascript", QUIT_TIMEOUT)?;
if !output.status.success() {
return Err(AdapterError::new(
ErrorCode::ActionFailed,
format!("Failed to request graceful quit for app '{id}'"),
)
.with_platform_detail(String::from_utf8_lossy(&output.stderr).trim().to_string())
.with_suggestion(
"Use 'list-apps' to verify the app name, or retry with --force.",
));
}
}
}
Ok(())

24
crates/macos/src/tree/builder.rs
View file

@ -22,10 +22,14 @@ pub fn window_element_for(pid: i32, win_title: &str) -> AXElement {
let app = element_for_pid(pid);
if let Some(windows) = copy_ax_array(&app, kAXWindowsAttribute) {
let mut first_candidate = None;
let mut child_candidate = None;
let mut partial_candidate = None;
for win in &windows {
if !is_window_candidate(win) {
continue;
}
first_candidate.get_or_insert_with(|| win.clone());
let title = copy_string_attr(win, kAXTitleAttribute);
if title
.as_deref()
@ -33,26 +37,18 @@ pub fn window_element_for(pid: i32, win_title: &str) -> AXElement {
{
return win.clone();
}
}
for win in &windows {
if !is_window_candidate(win) {
continue;
}
let title = copy_string_attr(win, kAXTitleAttribute);
if title
.as_deref()
.is_some_and(|title| window_titles_are_partial_match(title, win_title))
{
return win.clone();
partial_candidate.get_or_insert_with(|| win.clone());
}
if child_candidate.is_none() && count_children(win, None) > 0 {
child_candidate = Some(win.clone());
}
}
for win in &windows {
if is_window_candidate(win) && count_children(win, None) > 0 {
return win.clone();
}
}
if let Some(first) = windows.into_iter().find(is_window_candidate) {
return first;
if let Some(candidate) = partial_candidate.or(child_candidate).or(first_candidate) {
return candidate;
}
}

3
crates/macos/src/tree/resolve_classify.rs
View file

@ -43,6 +43,9 @@ fn classify_ambiguous_candidates(
.filter(|candidate| verified_bounds_match(candidate, entry))
.cloned()
.collect();
if entry.bounds_hash.is_some() && bounds_matches.is_empty() {
return Err(AdapterError::element_not_found("element"));
}
if bounds_matches.len() == 1 {
return retained_handle(bounds_matches.remove(0));
}

21
crates/macos/src/tree/resolve_search.rs
View file

@ -57,7 +57,6 @@ fn element_at_path(
let mut seen = FxHashSet::default();
for idx in path {
ensure_before_deadline(deadline)?;
set_messaging_timeout(&current, remaining_before_deadline(deadline)?);
let ax_role = copy_string_attr(&current, accessibility_sys::kAXRoleAttribute);
let children = resolve_children(&current, ax_role.as_deref(), deadline, &mut seen)?;
let Some(child) = children.get(*idx) else {
@ -128,10 +127,11 @@ fn collect_elements_recursive(
}
let ax_role = copy_string_attr(el, kAXRoleAttribute);
let normalized = crate::tree::roles::normalized_role_for_element(el, ax_role.as_deref());
let (normalized, promoted_label) =
crate::tree::roles::normalized_role_and_label(el, ax_role.as_deref());
if normalized == context.entry.role
&& element_matches_path_entry_with_role(el, context.entry, ax_role.as_deref())
&& element_identity_matches(el, context.entry, promoted_label)
&& context.seen_matches.push_clone(context.matches, el)
&& should_stop_collecting(context.matches.len(), context.entry)
{
@ -158,7 +158,9 @@ fn collect_elements_recursive(
#[cfg(target_os = "macos")]
fn element_matches_entry(el: &AXElement, entry: &RefEntry) -> bool {
let ax_role = copy_string_attr(el, accessibility_sys::kAXRoleAttribute);
element_matches_path_entry_with_role(el, entry, ax_role.as_deref())
let (normalized, promoted_label) =
crate::tree::roles::normalized_role_and_label(el, ax_role.as_deref());
normalized == entry.role && element_identity_matches(el, entry, promoted_label)
}
pub(super) fn should_stop_collecting(match_count: usize, entry: &RefEntry) -> bool {
@ -170,16 +172,11 @@ fn should_prune_for_resolution(el: &AXElement, entry: &RefEntry, depth: u8) -> b
}
#[cfg(target_os = "macos")]
fn element_matches_path_entry_with_role(
fn element_identity_matches(
el: &AXElement,
entry: &RefEntry,
ax_role: Option<&str>,
promoted_label: Option<String>,
) -> bool {
let (normalized, promoted_label) = crate::tree::roles::normalized_role_and_label(el, ax_role);
if normalized != entry.role {
return false;
}
let elem_name = promoted_label.or_else(|| resolve_element_name(el));
let elem_value = crate::tree::copy_value_typed(el);
let elem_description = copy_string_attr(el, accessibility_sys::kAXDescriptionAttribute);
@ -199,10 +196,10 @@ fn resolve_children(
seen: &mut FxHashSet<usize>,
) -> Result<Vec<AXElement>, AdapterError> {
seen.clear();
set_messaging_timeout(el, remaining_before_deadline(deadline)?);
let mut result = Vec::new();
for attr in child_attributes(ax_role) {
ensure_before_deadline(deadline)?;
set_messaging_timeout(el, remaining_before_deadline(deadline)?);
if let Some(children) = copy_ax_array(el, attr) {
for child in children {
if seen.insert(child.0 as usize) {

17
crates/macos/src/tree/resolve_tests.rs
View file

@ -195,6 +195,23 @@ fn ambiguous_candidate_classification_reports_structured_details() {
assert_eq!(details["source_window_id"], "w-42");
}
#[test]
fn multiple_identity_candidates_without_bounds_match_are_stale_not_ambiguous() {
let err = match classify_candidates(
vec![
AXElement(std::ptr::null_mut()),
AXElement(std::ptr::null_mut()),
],
&entry(Some(42), Some("w-42"), Some("Documents"), None),
true,
) {
Ok(_) => panic!("expected stale moved target"),
Err(err) => err,
};
assert_eq!(err.code, ErrorCode::ElementNotFound);
}
#[test]
fn single_meaningful_identity_candidate_resolves_after_bounds_change() {
let _handle = classify_candidates(

2
skills/agent-desktop/references/commands-interaction.md
View file

@ -59,7 +59,7 @@ Triple-click requires cursor/focus side effects and is blocked in headless mode;
```bash
agent-desktop right-click @e5
```
Performs a semantic right-click/context-menu action and includes the menu tree when a menu surface can be verified. If the right-click action succeeds but menu probing fails, the command still returns the action result with `menu_probe.ok: false` so callers do not retry and double-open context menus. Combo boxes and menu buttons expose menu-opening actions for their primary dropdown; use `select` for those controls, not `right-click`. Focus-stealing and coordinate right-click fallback are blocked in headless mode; pass `--headed` to allow them.
Performs a semantic right-click/context-menu action and includes `menu` plus `menu_snapshot_id` when a menu surface can be verified. If the right-click action succeeds but menu probing fails, the command still returns the action result with `menu_probe.ok: false` so callers do not retry and double-open context menus. Combo boxes and menu buttons expose menu-opening actions for their primary dropdown; use `select` for those controls, not `right-click`. Focus-stealing and coordinate right-click fallback are blocked in headless mode; pass `--headed` to allow them.
## Text Input

53
src/dispatch/notifications.rs
View file

@ -24,7 +24,7 @@ pub(crate) fn dispatch_notification(
),
Commands::DismissNotification(a) => dismiss_notification::execute(
dismiss_notification::DismissNotificationArgs {
index: a.index as usize,
index: notification_index(a.index)?,
app: a.app,
},
adapter,
@ -35,7 +35,7 @@ pub(crate) fn dispatch_notification(
),
Commands::NotificationAction(a) => notification_action::execute(
notification_action::NotificationActionArgs {
index: a.index as usize,
index: notification_index(a.index)?,
action: a.action,
expected_app: a.expected_app,
expected_title: a.expected_title,
@ -50,3 +50,52 @@ pub(crate) fn dispatch_notification(
)),
}
}
fn notification_index(index: u64) -> Result<usize, AppError> {
if index == 0 {
return Err(AppError::invalid_input(
"Notification index is 1-based and must be greater than zero",
));
}
usize::try_from(index).map_err(|_| AppError::invalid_input("Notification index is too large"))
}
#[cfg(test)]
mod tests {
use super::*;
use crate::cli_args::notifications::{DismissNotificationCliArgs, NotificationActionCliArgs};
struct NoopAdapter;
impl PlatformAdapter for NoopAdapter {}
#[test]
fn dismiss_notification_rejects_zero_index_before_adapter() {
let err = dispatch_notification(
Commands::DismissNotification(DismissNotificationCliArgs {
index: 0,
app: None,
}),
&NoopAdapter,
)
.unwrap_err();
assert_eq!(err.code(), "INVALID_ARGS");
}
#[test]
fn notification_action_rejects_zero_index_before_adapter() {
let err = dispatch_notification(
Commands::NotificationAction(NotificationActionCliArgs {
index: 0,
action: "Reply".into(),
expected_app: None,
expected_title: None,
}),
&NoopAdapter,
)
.unwrap_err();
assert_eq!(err.code(), "INVALID_ARGS");
}
}

5
tests/e2e/run.sh
View file

@ -36,7 +36,7 @@ except Exception: print(''); sys.exit()
try: print(eval('d'+sys.argv[1]))
except Exception: print('')" "$1" 2>/dev/null; }
resolve() { "$bin" find --app "$app" --role "$1" --name "$2" --first 2>/dev/null | field "['data']['match']['ref']"; }
resolve() { "$bin" find --app "$app" --role "$1" --name "$2" --first 2>/dev/null | field "['data']['match']['ref_id']"; }
read_value() { "$bin" find --app "$app" --role statictext --name "$1" --first 2>/dev/null | field "['data']['match']['value']"; }
running() { "$bin" list-apps 2>/dev/null | python3 -c "import json,sys;print(any(a['name']=='$app' for a in json.load(sys.stdin)['data']['apps']))" 2>/dev/null; }
@ -69,6 +69,7 @@ interaction_suite() {
na="$(printf '%s' "$ca" | grep -oE '[0-9]+$' || true)"; na="${na:-0}"
assert "[$MODE] click incremented counter" "$([ "$na" -gt "$nb" ] && echo 1 || echo 0)" \
"click-status before='$cb' after='$ca'"
act clear "$(resolve textfield text-input)" >/dev/null 2>&1; sleep 0.2
verify "type sets field" text-echo "typed-$MODE" type "$(resolve textfield text-input)" "typed-$MODE"
verify "set-value sets field" text-echo "set-$MODE" set-value "$(resolve textfield text-input)" "set-$MODE"
verify "clear empties field" text-echo "" clear "$(resolve textfield text-input)"
@ -136,7 +137,7 @@ done
note "find vocabulary (observed resolution)"
tf="$(resolve textfield text-input)"
assert "find textfield by name" "$([ -n "$tf" ] && echo 1 || echo 0)" "resolved ref='$tf'"
ta="$("$bin" find --app "$app" --role textarea --name text-input --first 2>/dev/null | field "['data']['match']['ref']")"
ta="$("$bin" find --app "$app" --role textarea --name text-input --first 2>/dev/null | field "['data']['match']['ref_id']")"
assert "textarea alias -> textfield" "$([ -n "$ta" ] && echo 1 || echo 0)" "alias resolved ref='$ta'"
hint="$("$bin" find --app "$app" --role navbar 2>/dev/null | field "['data']['roles_present']")"
assert "absent role returns roles_present hint" "$([ -n "$hint" ] && echo 1 || echo 0)" "roles_present=${hint:0:60}..."