Creepso/agent-desktop
1
0
Fork 0
mirror of https://github.com/lahfir/agent-desktop.git synced 2026-07-27 01:22:16 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

refactor: trim reliability branch dead code

Browse source
This commit is contained in:
Lahfir 2026-06-17 13:34:58 -07:00
parent 0bc3a69e22
commit 5e4a09b4ce
12 changed files with 71 additions and 175 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
crates/core/src/action_result.rs
View file

@ -5,8 +5,6 @@ use serde::{Deserialize, Serialize};
pub struct ActionResult {
pub action: String,
#[serde(skip_serializing_if = "Option::is_none")]
pub ref_id: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub post_state: Option<ElementState>,
#[serde(skip_serializing_if = "Vec::is_empty", default)]
pub steps: Vec<ActionStep>,
@ -16,17 +14,11 @@ impl ActionResult {
pub fn new(action: impl Into<String>) -> Self {
Self {
action: action.into(),
ref_id: None,
post_state: None,
steps: Vec::new(),
}
}
pub fn with_ref(mut self, ref_id: impl Into<String>) -> Self {
self.ref_id = Some(ref_id.into());
self
}
pub fn with_state(mut self, state: ElementState) -> Self {
self.post_state = Some(state);
self

4
crates/core/src/action_step.rs
View file

@ -8,10 +8,6 @@ pub struct ActionStep {
}
impl ActionStep {
pub fn label(&self) -> &str {
&self.label
}
pub fn attempted(label: &'static str) -> Self {
Self {
label: label.to_string(),

25
crates/core/src/capability.rs
View file

@ -45,7 +45,17 @@ pub fn for_action(action: &Action) -> &'static [&'static str] {
}
pub fn defaults_for_role(role: &str) -> Vec<String> {
role_default_slice(role)
let capabilities: &[&str] = match role {
"button" | "link" | "menuitem" | "tab" | "radiobutton" => &[CLICK],
"textfield" | "incrementor" => &[CLICK, SET_VALUE, SET_FOCUS],
"checkbox" => &[CLICK, TOGGLE],
"combobox" => &[CLICK, SELECT],
"treeitem" => &[CLICK, EXPAND, COLLAPSE],
"slider" => &[SET_VALUE],
"cell" => &[CLICK],
_ => &[CLICK],
};
capabilities
.iter()
.map(|capability| (*capability).to_string())
.collect()
@ -61,19 +71,6 @@ pub fn contains_any(actions: &[String], capabilities: &[&str]) -> bool {
.any(|capability| contains(actions, capability))
}
fn role_default_slice(role: &str) -> &'static [&'static str] {
match role {
"button" | "link" | "menuitem" | "tab" | "radiobutton" => &[CLICK],
"textfield" | "incrementor" => &[CLICK, SET_VALUE, SET_FOCUS],
"checkbox" => &[CLICK, TOGGLE],
"combobox" => &[CLICK, SELECT],
"treeitem" => &[CLICK, EXPAND, COLLAPSE],
"slider" => &[SET_VALUE],
"cell" => &[CLICK],
_ => &[CLICK],
}
}
#[cfg(test)]
mod tests {
use super::*;

22
crates/core/src/context.rs
View file

@ -3,7 +3,7 @@ use crate::{
interaction_policy::InteractionPolicy, trace::TraceConfig,
};
use serde_json::Value;
use std::path::{Path, PathBuf};
use std::path::PathBuf;
#[derive(Debug, Clone, Default)]
pub struct CommandContext {
@ -87,10 +87,6 @@ impl CommandContext {
pub fn session_id(&self) -> Option<&str> {
self.session_id.as_deref()
}
pub fn trace_path(&self) -> Option<&Path> {
self.trace.path()
}
}
pub fn validate_session_id(id: &str) -> Result<(), AppError> {
@ -279,18 +275,20 @@ mod tests {
#[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 path = std::env::temp_dir().join("agent-desktop-context-test.jsonl");
let _ = std::fs::remove_file(&path);
let parent = CommandContext::new(Some("parent".into()), Some(path.clone()), 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(), Some("parent"));
assert_eq!(overridden.session_id(), Some("child"));
assert!(overridden.trace_path().is_some());
overridden
.trace("batch.child", serde_json::json!({ "ok": true }))
.unwrap();
let body = std::fs::read_to_string(&path).unwrap();
assert!(body.contains("batch.child"));
let _ = std::fs::remove_file(path);
}
}

11
crates/core/src/trace.rs
View file

@ -7,7 +7,6 @@ const MAX_TRACE_FILE_BYTES: u64 = 64 * 1024 * 1024;
#[derive(Debug, Clone, Default)]
pub struct TraceConfig {
path: Option<PathBuf>,
strict: bool,
writer: Option<Arc<Mutex<std::fs::File>>>,
}
@ -32,11 +31,7 @@ impl TraceConfig {
},
None => None,
};
Ok(Self {
path,
strict,
writer,
})
Ok(Self { strict, writer })
}
pub fn emit(&self, event: &str, fields: Value) -> Result<(), AppError> {
@ -60,10 +55,6 @@ impl TraceConfig {
}
}
}
pub fn path(&self) -> Option<&Path> {
self.path.as_deref()
}
}
fn open_trace_file(path: &Path) -> Result<std::fs::File, AppError> {

70
crates/ffi/src/actions/resolve.rs
View file

@ -61,12 +61,22 @@ pub(crate) 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_LIMIT) }?;
let states = unsafe {
string_array(
entry.states,
entry.state_count,
"states",
"AD_MAX_REF_STATES",
crate::types::ref_entry::AD_MAX_REF_STATES,
)
}?;
let available_actions = unsafe {
string_array(
entry.available_actions,
entry.available_action_count,
&ACTIONS_LIMIT,
"available_actions",
"AD_MAX_REF_ACTIONS",
crate::types::ref_entry::AD_MAX_REF_ACTIONS,
)
}?;
let bounds = if entry.has_bounds {
@ -121,51 +131,23 @@ unsafe fn optional_string(
})
}
/// Per-field cap for a counted array crossing the C boundary. `constant`
/// names the header macro so the error message tells callers which
/// published limit they exceeded.
struct ArrayLimit {
field: &'static str,
constant: &'static str,
max: usize,
}
const STATES_LIMIT: ArrayLimit = ArrayLimit {
field: "states",
constant: "AD_MAX_REF_STATES",
max: crate::types::ref_entry::AD_MAX_REF_STATES,
};
const ACTIONS_LIMIT: ArrayLimit = ArrayLimit {
field: "available_actions",
constant: "AD_MAX_REF_ACTIONS",
max: crate::types::ref_entry::AD_MAX_REF_ACTIONS,
};
const PATH_LIMIT: ArrayLimit = ArrayLimit {
field: "path",
constant: "AD_MAX_REF_PATH_DEPTH",
max: crate::types::ref_entry::AD_MAX_REF_PATH_DEPTH,
};
fn check_array_len(
len: usize,
is_null: bool,
limit: &ArrayLimit,
field: &str,
constant: &str,
max: usize,
) -> Result<(), agent_desktop_core::error::AdapterError> {
if len > limit.max {
if len > max {
return Err(agent_desktop_core::error::AdapterError::new(
agent_desktop_core::error::ErrorCode::InvalidArgs,
format!(
"{} count {len} exceeds {} ({})",
limit.field, limit.constant, limit.max
),
format!("{field} count {len} exceeds {constant} ({max})"),
));
}
if is_null {
return Err(agent_desktop_core::error::AdapterError::new(
agent_desktop_core::error::ErrorCode::InvalidArgs,
format!("{} count is nonzero but pointer is null", limit.field),
format!("{field} count is nonzero but pointer is null"),
));
}
Ok(())
@ -174,18 +156,20 @@ fn check_array_len(
unsafe fn string_array(
ptr: *const *const std::os::raw::c_char,
len: usize,
limit: &ArrayLimit,
field: &str,
constant: &str,
max: usize,
) -> Result<Vec<String>, agent_desktop_core::error::AdapterError> {
if len == 0 {
return Ok(Vec::new());
}
check_array_len(len, ptr.is_null(), limit)?;
check_array_len(len, ptr.is_null(), field, constant, max)?;
let items = unsafe { std::slice::from_raw_parts(ptr, len) };
items
.iter()
.enumerate()
.map(|(index, item)| {
let element = format!("{}[{index}]", limit.field);
let element = format!("{field}[{index}]");
unsafe { optional_string(*item, &element) }?.ok_or_else(|| {
agent_desktop_core::error::AdapterError::new(
agent_desktop_core::error::ErrorCode::InvalidArgs,
@ -203,7 +187,13 @@ unsafe fn ref_path(
if len == 0 {
return Ok(smallvec::SmallVec::new());
}
check_array_len(len, ptr.is_null(), &PATH_LIMIT)?;
check_array_len(
len,
ptr.is_null(),
"path",
"AD_MAX_REF_PATH_DEPTH",
crate::types::ref_entry::AD_MAX_REF_PATH_DEPTH,
)?;
let mut path = smallvec::SmallVec::new();
path.extend(
unsafe { std::slice::from_raw_parts(ptr, len) }

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

@ -7,7 +7,6 @@ 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());
let post_state = match &r.post_state {
None => ptr::null_mut(),
Some(state) => {
@ -36,7 +35,7 @@ pub(crate) fn action_result_to_c(r: &CoreActionResult) -> AdActionResult {
};
AdActionResult {
action,
ref_id,
ref_id: ptr::null(),
post_state,
}
}
@ -93,7 +92,6 @@ mod tests {
fn test_action_result_to_c_with_state() {
let core_result = CoreActionResult {
action: "click".to_owned(),
ref_id: Some("@e3".to_owned()),
post_state: Some(ElementState {
role: "button".to_owned(),
states: vec!["focused".to_owned(), "enabled".to_owned()],
@ -104,7 +102,7 @@ mod tests {
let c_result = action_result_to_c(&core_result);
unsafe {
assert_eq!(c_to_string(c_result.action).as_deref(), Some("click"));
assert_eq!(c_to_string(c_result.ref_id).as_deref(), Some("@e3"));
assert!(c_result.ref_id.is_null());
assert!(!c_result.post_state.is_null());
let state = &*c_result.post_state;
assert_eq!(c_to_string(state.role).as_deref(), Some("button"));

16
crates/ffi/src/error.rs
View file

@ -3,7 +3,6 @@ use std::cell::RefCell;
use std::ffi::{CStr, CString, c_char};
const fn error_code_variant_count() -> usize {
let mut count = 0;
let variants = [
ErrorCode::PermDenied,
ErrorCode::ElementNotFound,
@ -21,12 +20,7 @@ const fn error_code_variant_count() -> usize {
ErrorCode::PolicyDenied,
ErrorCode::Internal,
];
let mut i = 0;
while i < variants.len() {
count += 1;
i += 1;
}
count
variants.len()
}
const fn ad_result_error_variant_count() -> usize {
@ -47,13 +41,7 @@ const fn ad_result_error_variant_count() -> usize {
AdResult::ErrSnapshotNotFound,
AdResult::ErrPolicyDenied,
];
let mut count = 0;
let mut i = 0;
while i < variants.len() {
count += 1;
i += 1;
}
count
variants.len()
}
const _: () = assert!(

5
crates/macos/src/actions/chain.rs
View file

@ -27,7 +27,10 @@ mod imp {
let deadline = ctx
.deadline
.unwrap_or_else(|| Instant::now() + chain_timeout());
let ctx = ctx.with_deadline(deadline);
let ctx = ChainContext {
dynamic_value: ctx.dynamic_value,
deadline: Some(deadline),
};
let total = def.steps.len();
let mut steps = Vec::new();

33
crates/macos/src/actions/chain_context.rs
View file

@ -2,36 +2,3 @@ pub(crate) struct ChainContext<'a> {
pub(crate) dynamic_value: Option<&'a str>,
pub(crate) deadline: Option<std::time::Instant>,
}
impl<'a> ChainContext<'a> {
/// Pins the chain's resolved deadline so every step — notably the
/// `IncrementToDynamic` loop — observes the same budget the chain
/// enforces between steps. Callers construct contexts with
/// `deadline: None`; the chain owns resolving that into an instant.
pub(crate) fn with_deadline(&self, deadline: std::time::Instant) -> ChainContext<'a> {
ChainContext {
dynamic_value: self.dynamic_value,
deadline: Some(deadline),
}
}
}
#[cfg(test)]
mod tests {
use super::ChainContext;
use std::time::{Duration, Instant};
#[test]
fn with_deadline_pins_the_instant_and_keeps_the_dynamic_value() {
let base = ChainContext {
dynamic_value: Some("42"),
deadline: None,
};
let deadline = Instant::now() + Duration::from_secs(1);
let effective = base.with_deadline(deadline);
assert_eq!(effective.dynamic_value, Some("42"));
assert_eq!(effective.deadline, Some(deadline));
}
}

40
crates/macos/src/actions/chain_disclosure_steps.rs
View file

@ -24,12 +24,6 @@ mod imp {
press_toggle_disclosure(el, false, chain_deadline)
}
enum Settle {
Confirmed,
Failed,
DeadlineExpired,
}
/// Tries the semantic action / settable attribute, then a press. Each is
/// confirmed against the disclosed state; an action that succeeds at the AX
/// layer but does not move the control is not counted. A settle wait that
@ -52,42 +46,25 @@ mod imp {
};
if ax_helpers::has_ax_action(el, action) {
let _ = ax_helpers::try_ax_action_retried_or_err(el, action)?;
if settled_or_deadline(el, want_expanded, chain_deadline)? {
if disclosure_settled(el, want_expanded, chain_deadline)? {
return Ok(true);
}
}
if ax_helpers::is_attr_settable(el, "AXExpanded") {
let _ = ax_helpers::set_ax_bool_or_err(el, "AXExpanded", want_expanded)?;
if settled_or_deadline(el, want_expanded, chain_deadline)? {
if disclosure_settled(el, want_expanded, chain_deadline)? {
return Ok(true);
}
}
if ax_helpers::has_ax_action(el, "AXPress")
&& ax_helpers::try_ax_action_retried_or_err(el, "AXPress")?
&& settled_or_deadline(el, want_expanded, chain_deadline)?
&& disclosure_settled(el, want_expanded, chain_deadline)?
{
return Ok(true);
}
Ok(false)
}
fn settled_or_deadline(
el: &AXElement,
want_expanded: bool,
chain_deadline: Option<std::time::Instant>,
) -> Result<bool, AdapterError> {
match disclosure_settled(el, want_expanded, chain_deadline) {
Settle::Confirmed => Ok(true),
Settle::Failed => Ok(false),
Settle::DeadlineExpired => {
Err(crate::actions::chain_verify::disclosure_deadline_error(
want_expanded,
disclosed_state(el),
))
}
}
}
/// Polls for the disclosed state instead of a fixed settle sleep: fast UIs
/// confirm on the first read, while animated disclosures get up to the
/// settle budget. The budget is capped to the chain's remaining deadline;
@ -98,7 +75,7 @@ mod imp {
el: &AXElement,
want_expanded: bool,
chain_deadline: Option<std::time::Instant>,
) -> Settle {
) -> Result<bool, AdapterError> {
use std::time::{Duration, Instant};
const POLL_INTERVAL: Duration = Duration::from_millis(20);
@ -109,14 +86,17 @@ mod imp {
let truncated = deadline < budget_end;
loop {
if disclosed_state(el) == Some(want_expanded) {
return Settle::Confirmed;
return Ok(true);
}
let now = Instant::now();
if now >= deadline {
return if truncated {
Settle::DeadlineExpired
Err(crate::actions::chain_verify::disclosure_deadline_error(
want_expanded,
disclosed_state(el),
))
} else {
Settle::Failed
Ok(false)
};
}
std::thread::sleep(POLL_INTERVAL.min(deadline - now));

6
crates/macos/src/system/force_close.rs
View file

@ -23,7 +23,7 @@ pub(crate) fn terminate_app(id: &str, pids: &[i32], timeout: Duration) -> Result
fn signal_pids(id: &str, pids: &[i32], signal: Signal) -> Result<(), AdapterError> {
for &pid in pids {
send_signal(pid, signal).map_err(|detail| {
signal_result(pid, signal).map(|_| ()).map_err(|detail| {
AdapterError::new(
ErrorCode::ActionFailed,
format!("Failed to {} app '{id}' pid {pid}", signal.verb()),
@ -77,10 +77,6 @@ fn child_process_is_running(pid: i32) -> Option<bool> {
}
}
fn send_signal(pid: i32, signal: Signal) -> Result<(), String> {
signal_result(pid, signal).map(|_| ())
}
fn signal_result(pid: i32, signal: Signal) -> Result<bool, String> {
const POSIX_ESRCH: i32 = 3;