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

fix: ProcessState liveness classification and non-fatal enrichment

Browse source 
Reshape ProcessState to the plan's {Running, Exited{code}, Crashed,
Unresponsive} contract instead of the ad hoc {Responsive, Unresponsive,
Unknown}. macOS now does a kill(pid,0)-style liveness check before
probing AX at all, and only classifies Unresponsive after a second
consecutive kAXErrorCannotComplete, so one transient AX blip on a
healthy-but-busy app no longer hard-fails the action. The classification
threshold is isolated behind a pure, platform-independent classify() fn.

ensure_process_responsive was an unconditional preflight hard-gate that
could turn a would-succeed action into a failure and propagated a
transient probe error as the terminal error. Replace it with
enrich_with_process_state: best-effort, terminal-only enrichment that
runs exactly once (never per auto-wait tick), never converts a success
into a failure, attaches details.process_state on STALE_REF/APP_NOT_FOUND,
and only surfaces APP_UNRESPONSIVE when the process is genuinely
classified Unresponsive.

Add the missing retry_token_for_code arm for AppUnresponsive.
This commit is contained in:
Lahfir 2026-07-02 23:48:02 -07:00
parent d18b70ef93
commit 07db5ceafb
8 changed files with 756 additions and 189 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

170
crates/core/src/output.rs
View file

@ -59,6 +59,7 @@ fn retry_token_for_code(code: &ErrorCode) -> Option<String> {
Some("snapshot;execute_by_ref".to_owned())
}
ErrorCode::PolicyDenied => Some("escalate_policy".to_owned()),
ErrorCode::AppUnresponsive => Some("wait;execute_by_ref".to_owned()),
_ => None,
}
}
@ -100,170 +101,5 @@ impl ErrorPayload {
}
#[cfg(test)]
mod tests {
use super::*;
use crate::error::{AdapterError, ErrorCode};
use serde_json::json;
#[test]
fn app_error_payload_preserves_adapter_recovery_fields() {
let err = AppError::Adapter(
AdapterError::new(ErrorCode::ActionFailed, "not actionable")
.with_suggestion("wait and retry")
.with_platform_detail("native press action failed")
.with_details(json!({ "check": "visible" })),
);
let payload = ErrorPayload::from_app_error(&err);
assert_eq!(payload.code, "ACTION_FAILED");
assert_eq!(payload.message, "not actionable");
assert_eq!(payload.suggestion.as_deref(), Some("wait and retry"));
assert_eq!(
payload.platform_detail.as_deref(),
Some("native press action failed")
);
assert_eq!(payload.details, Some(json!({ "check": "visible" })));
assert_eq!(
payload.retry_command, None,
"ACTION_FAILED must not carry a retry token"
);
}
#[test]
fn stale_ref_payload_carries_snapshot_retry_token() {
let err = AppError::stale_ref("@e5");
let payload = ErrorPayload::from_app_error(&err);
assert_eq!(payload.code, "STALE_REF");
assert_eq!(
payload.retry_command.as_deref(),
Some("snapshot;execute_by_ref"),
"STALE_REF must carry the canonical retry token"
);
}
#[test]
fn snapshot_not_found_payload_carries_snapshot_retry_token() {
let err = AppError::Adapter(AdapterError::snapshot_not_found("snap-abc"));
let payload = ErrorPayload::from_app_error(&err);
assert_eq!(payload.code, "SNAPSHOT_NOT_FOUND");
assert_eq!(
payload.retry_command.as_deref(),
Some("snapshot;execute_by_ref"),
"SNAPSHOT_NOT_FOUND must carry the canonical retry token"
);
}
#[test]
fn policy_denied_payload_carries_escalate_policy_token() {
let err = AppError::Adapter(AdapterError::policy_denied("blocked by policy"));
let payload = ErrorPayload::from_app_error(&err);
assert_eq!(payload.code, "POLICY_DENIED");
assert_eq!(
payload.retry_command.as_deref(),
Some("escalate_policy"),
"POLICY_DENIED must carry the escalate_policy token, not a snapshot token"
);
}
#[test]
fn retry_command_absent_for_non_retryable_errors() {
for err in [
AppError::Adapter(AdapterError::new(ErrorCode::InvalidArgs, "bad input")),
AppError::Adapter(AdapterError::not_supported("method_x")),
AppError::Adapter(AdapterError::new(ErrorCode::ActionFailed, "failed")),
] {
let payload = ErrorPayload::from_app_error(&err);
assert!(
payload.retry_command.is_none(),
"non-retryable error {} must not carry a retry token",
payload.code
);
}
}
#[test]
fn ok_response_json_shape_has_version_ok_command_data_and_no_error_field() {
let resp = Response::ok("snapshot", json!({"app": "Finder"}));
let map: serde_json::Map<String, serde_json::Value> =
serde_json::from_value(serde_json::to_value(&resp).expect("serializable"))
.expect("map");
assert_eq!(map["version"].as_str(), Some("2.1"), "version must be 2.1");
assert_eq!(map["ok"].as_bool(), Some(true), "ok must be true");
assert_eq!(
map["command"].as_str(),
Some("snapshot"),
"command must match"
);
assert!(map.contains_key("data"), "ok response must have data field");
assert!(
!map.contains_key("error"),
"ok response must not serialize an error field (skip_serializing_if = is_none)"
);
}
#[test]
fn err_response_json_shape_has_version_ok_command_error_and_no_data_field() {
let payload =
ErrorPayload::new("STALE_REF", "ref @e1 is stale").with_suggestion("re-run snapshot");
let resp = Response::err("click", payload);
let map: serde_json::Map<String, serde_json::Value> =
serde_json::from_value(serde_json::to_value(&resp).expect("serializable"))
.expect("map");
assert_eq!(map["version"].as_str(), Some("2.1"), "version must be 2.1");
assert_eq!(map["ok"].as_bool(), Some(false), "ok must be false");
assert_eq!(map["command"].as_str(), Some("click"), "command must match");
assert!(
!map.contains_key("data"),
"err response must not serialize a data field (skip_serializing_if = is_none)"
);
assert!(
map.contains_key("error"),
"err response must have error field"
);
assert_eq!(
map["error"]["code"].as_str(),
Some("STALE_REF"),
"error code must round-trip"
);
assert_eq!(
map["error"]["message"].as_str(),
Some("ref @e1 is stale"),
"error message must round-trip"
);
assert_eq!(
map["error"]["suggestion"].as_str(),
Some("re-run snapshot"),
"suggestion must be present when set"
);
}
#[test]
fn err_response_omits_optional_error_subfields_when_absent() {
let payload = ErrorPayload::new("INTERNAL", "something broke");
let resp = Response::err("snapshot", payload);
let map: serde_json::Map<String, serde_json::Value> =
serde_json::from_value(serde_json::to_value(&resp).expect("serializable"))
.expect("map");
let error = map["error"].as_object().expect("error must be an object");
assert!(
!error.contains_key("suggestion"),
"absent suggestion must be omitted from JSON"
);
assert!(
!error.contains_key("retry_command"),
"absent retry_command must be omitted from JSON"
);
assert!(
!error.contains_key("platform_detail"),
"absent platform_detail must be omitted from JSON"
);
assert!(
!error.contains_key("details"),
"absent details must be omitted from JSON"
);
}
}
#[path = "output_tests.rs"]
mod tests;

174
crates/core/src/output_tests.rs Normal file
View file

@ -0,0 +1,174 @@
use super::*;
use crate::error::{AdapterError, ErrorCode};
use serde_json::json;
#[test]
fn app_error_payload_preserves_adapter_recovery_fields() {
let err = AppError::Adapter(
AdapterError::new(ErrorCode::ActionFailed, "not actionable")
.with_suggestion("wait and retry")
.with_platform_detail("native press action failed")
.with_details(json!({ "check": "visible" })),
);
let payload = ErrorPayload::from_app_error(&err);
assert_eq!(payload.code, "ACTION_FAILED");
assert_eq!(payload.message, "not actionable");
assert_eq!(payload.suggestion.as_deref(), Some("wait and retry"));
assert_eq!(
payload.platform_detail.as_deref(),
Some("native press action failed")
);
assert_eq!(payload.details, Some(json!({ "check": "visible" })));
assert_eq!(
payload.retry_command, None,
"ACTION_FAILED must not carry a retry token"
);
}
#[test]
fn stale_ref_payload_carries_snapshot_retry_token() {
let err = AppError::stale_ref("@e5");
let payload = ErrorPayload::from_app_error(&err);
assert_eq!(payload.code, "STALE_REF");
assert_eq!(
payload.retry_command.as_deref(),
Some("snapshot;execute_by_ref"),
"STALE_REF must carry the canonical retry token"
);
}
#[test]
fn snapshot_not_found_payload_carries_snapshot_retry_token() {
let err = AppError::Adapter(AdapterError::snapshot_not_found("snap-abc"));
let payload = ErrorPayload::from_app_error(&err);
assert_eq!(payload.code, "SNAPSHOT_NOT_FOUND");
assert_eq!(
payload.retry_command.as_deref(),
Some("snapshot;execute_by_ref"),
"SNAPSHOT_NOT_FOUND must carry the canonical retry token"
);
}
#[test]
fn policy_denied_payload_carries_escalate_policy_token() {
let err = AppError::Adapter(AdapterError::policy_denied("blocked by policy"));
let payload = ErrorPayload::from_app_error(&err);
assert_eq!(payload.code, "POLICY_DENIED");
assert_eq!(
payload.retry_command.as_deref(),
Some("escalate_policy"),
"POLICY_DENIED must carry the escalate_policy token, not a snapshot token"
);
}
#[test]
fn app_unresponsive_payload_carries_a_recovery_token() {
let err = AppError::Adapter(AdapterError::app_unresponsive("Finder"));
let payload = ErrorPayload::from_app_error(&err);
assert_eq!(payload.code, "APP_UNRESPONSIVE");
assert_eq!(
payload.retry_command.as_deref(),
Some("wait;execute_by_ref"),
"APP_UNRESPONSIVE must carry a sensible recovery token, not silently omit one"
);
}
#[test]
fn retry_command_absent_for_non_retryable_errors() {
for err in [
AppError::Adapter(AdapterError::new(ErrorCode::InvalidArgs, "bad input")),
AppError::Adapter(AdapterError::not_supported("method_x")),
AppError::Adapter(AdapterError::new(ErrorCode::ActionFailed, "failed")),
] {
let payload = ErrorPayload::from_app_error(&err);
assert!(
payload.retry_command.is_none(),
"non-retryable error {} must not carry a retry token",
payload.code
);
}
}
#[test]
fn ok_response_json_shape_has_version_ok_command_data_and_no_error_field() {
let resp = Response::ok("snapshot", json!({"app": "Finder"}));
let map: serde_json::Map<String, serde_json::Value> =
serde_json::from_value(serde_json::to_value(&resp).expect("serializable")).expect("map");
assert_eq!(map["version"].as_str(), Some("2.1"), "version must be 2.1");
assert_eq!(map["ok"].as_bool(), Some(true), "ok must be true");
assert_eq!(
map["command"].as_str(),
Some("snapshot"),
"command must match"
);
assert!(map.contains_key("data"), "ok response must have data field");
assert!(
!map.contains_key("error"),
"ok response must not serialize an error field (skip_serializing_if = is_none)"
);
}
#[test]
fn err_response_json_shape_has_version_ok_command_error_and_no_data_field() {
let payload =
ErrorPayload::new("STALE_REF", "ref @e1 is stale").with_suggestion("re-run snapshot");
let resp = Response::err("click", payload);
let map: serde_json::Map<String, serde_json::Value> =
serde_json::from_value(serde_json::to_value(&resp).expect("serializable")).expect("map");
assert_eq!(map["version"].as_str(), Some("2.1"), "version must be 2.1");
assert_eq!(map["ok"].as_bool(), Some(false), "ok must be false");
assert_eq!(map["command"].as_str(), Some("click"), "command must match");
assert!(
!map.contains_key("data"),
"err response must not serialize a data field (skip_serializing_if = is_none)"
);
assert!(
map.contains_key("error"),
"err response must have error field"
);
assert_eq!(
map["error"]["code"].as_str(),
Some("STALE_REF"),
"error code must round-trip"
);
assert_eq!(
map["error"]["message"].as_str(),
Some("ref @e1 is stale"),
"error message must round-trip"
);
assert_eq!(
map["error"]["suggestion"].as_str(),
Some("re-run snapshot"),
"suggestion must be present when set"
);
}
#[test]
fn err_response_omits_optional_error_subfields_when_absent() {
let payload = ErrorPayload::new("INTERNAL", "something broke");
let resp = Response::err("snapshot", payload);
let map: serde_json::Map<String, serde_json::Value> =
serde_json::from_value(serde_json::to_value(&resp).expect("serializable")).expect("map");
let error = map["error"].as_object().expect("error must be an object");
assert!(
!error.contains_key("suggestion"),
"absent suggestion must be omitted from JSON"
);
assert!(
!error.contains_key("retry_command"),
"absent retry_command must be omitted from JSON"
);
assert!(
!error.contains_key("platform_detail"),
"absent platform_detail must be omitted from JSON"
);
assert!(
!error.contains_key("details"),
"absent details must be omitted from JSON"
);
}

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

@ -1,9 +1,35 @@
use serde::{Deserialize, Serialize};
/// Liveness/responsiveness classification for a target process, per KTD8.
///
/// macOS can only emit `Running`, `Exited { code: None }`, and
/// `Unresponsive` — it has no way to read the exit code of a detached
/// process (apps launched via `open -g -a` are not children of this
/// process). `Crashed` stays in the contract for adapters with real crash
/// evidence (e.g. Windows `GetExitCodeProcess`).
#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)]
#[serde(rename_all = "snake_case")]
#[serde(tag = "state", rename_all = "snake_case")]
pub enum ProcessState {
Responsive,
Running,
Exited { code: Option<i32> },
Crashed { signal_or_code: i32 },
Unresponsive,
Unknown,
}
impl ProcessState {
/// Compact lowercase tag for best-effort `details.process_state`
/// enrichment on terminal errors — a short label, not the full
/// serialized shape (which carries `code`/`signal_or_code` payloads).
pub fn label(&self) -> &'static str {
match self {
ProcessState::Running => "running",
ProcessState::Exited { .. } => "exited",
ProcessState::Crashed { .. } => "crashed",
ProcessState::Unresponsive => "unresponsive",
}
}
}
#[cfg(test)]
#[path = "process_state_tests.rs"]
mod tests;

37
crates/core/src/process_state_tests.rs Normal file
View file

@ -0,0 +1,37 @@
use super::*;
#[test]
fn label_reports_short_lowercase_tag_per_variant() {
assert_eq!(ProcessState::Running.label(), "running");
assert_eq!(ProcessState::Exited { code: None }.label(), "exited");
assert_eq!(ProcessState::Exited { code: Some(1) }.label(), "exited");
assert_eq!(
ProcessState::Crashed { signal_or_code: 11 }.label(),
"crashed"
);
assert_eq!(ProcessState::Unresponsive.label(), "unresponsive");
}
#[test]
fn exited_serializes_with_optional_code_field() {
let value = serde_json::to_value(ProcessState::Exited { code: None }).expect("serializable");
assert_eq!(
value,
serde_json::json!({ "state": "exited", "code": null })
);
let value = serde_json::to_value(ProcessState::Exited { code: Some(9) }).expect("serializable");
assert_eq!(value, serde_json::json!({ "state": "exited", "code": 9 }));
}
#[test]
fn running_and_unresponsive_serialize_as_tag_only() {
assert_eq!(
serde_json::to_value(ProcessState::Running).expect("serializable"),
serde_json::json!({ "state": "running" })
);
assert_eq!(
serde_json::to_value(ProcessState::Unresponsive).expect("serializable"),
serde_json::json!({ "state": "unresponsive" })
);
}

49
crates/core/src/ref_action_wait.rs
View file

@ -10,20 +10,44 @@ use crate::{
use serde_json::{Value, json};
use std::time::{Duration, Instant};
fn ensure_process_responsive(
/// Best-effort enrichment of a terminal (caller-visible) error with process
/// liveness context. Runs exactly once, on the error `execute_with_auto_wait`
/// is about to return — never on an internal auto-wait retry tick — and
/// never converts a success into a failure: it only ever transforms an
/// already-failed `Result`. Only `STALE_REF`/`APP_NOT_FOUND` are enriched
/// (the codes that plausibly indicate a dead or hung target); a probe error
/// (including `PLATFORM_NOT_SUPPORTED` on adapters without this capability)
/// leaves the original error untouched.
fn enrich_with_process_state(
adapter: &dyn PlatformAdapter,
entry: &RefEntry,
) -> Result<(), AdapterError> {
let state = match adapter.process_state(entry.pid) {
Ok(state) => state,
Err(err) if err.code == ErrorCode::PlatformNotSupported => return Ok(()),
Err(err) => return Err(err),
err: AdapterError,
) -> AdapterError {
if !matches!(err.code, ErrorCode::StaleRef | ErrorCode::AppNotFound) {
return err;
}
let Ok(state) = adapter.process_state(entry.pid) else {
return err;
};
if state == crate::process_state::ProcessState::Unresponsive {
let app = entry.source_app.as_deref().unwrap_or("target application");
return Err(AdapterError::app_unresponsive(app));
return AdapterError::app_unresponsive(app);
}
Ok(())
attach_process_state_detail(err, state)
}
fn attach_process_state_detail(
err: AdapterError,
state: crate::process_state::ProcessState,
) -> AdapterError {
let mut details = err.details.clone().unwrap_or_else(|| json!({}));
match details.as_object_mut() {
Some(obj) => {
obj.insert("process_state".into(), json!(state.label()));
}
None => details = json!({ "process_state": state.label() }),
}
err.with_details(details)
}
fn trace_resolve_error(context: &CommandContext, ref_id: &str, err: &AdapterError) {
@ -56,9 +80,9 @@ pub(crate) fn execute_with_auto_wait(
ActionRequest,
) -> Result<ActionResult, crate::error::AppError>,
) -> Result<ActionResult, AdapterError> {
ensure_process_responsive(adapter, entry)?;
let Some(budget_ms) = request.timeout_ms else {
return execute_single_shot(adapter, entry, ref_id, context, request, dispatch);
return execute_single_shot(adapter, entry, ref_id, context, request, dispatch)
.map_err(|err| enrich_with_process_state(adapter, entry, err));
};
execute_poll_loop(
adapter,
@ -69,6 +93,7 @@ pub(crate) fn execute_with_auto_wait(
budget_from_ms(budget_ms),
dispatch,
)
.map_err(|err| enrich_with_process_state(adapter, entry, err))
}
fn execute_single_shot(
@ -229,3 +254,7 @@ fn actionability_timeout(last_report: Option<Value>) -> AdapterError {
#[cfg(test)]
#[path = "ref_action_wait_tests.rs"]
mod tests;
#[cfg(test)]
#[path = "ref_action_wait_process_state_tests.rs"]
mod process_state_tests;

304
crates/core/src/ref_action_wait_process_state_tests.rs Normal file
View file

@ -0,0 +1,304 @@
use super::*;
use crate::{
action::Action,
adapter::{ActionOps, InputOps, NativeHandle, ObservationOps, SystemOps},
capability,
error::{AdapterError, ErrorCode},
};
use std::sync::atomic::{AtomicU32, Ordering};
use std::time::Duration;
/// F23: `ensure_process_responsive` used to be an unconditional preflight
/// hard-gate that queried process liveness before every ref action, even
/// ones that would otherwise succeed, and turned a genuinely-Unresponsive
/// classification straight into the terminal error regardless of whether
/// the action itself failed. These tests cover the replacement contract:
/// terminal-only, best-effort enrichment that never touches a success.
fn entry() -> RefEntry {
RefEntry {
pid: 1,
role: "button".into(),
name: Some("Run".into()),
value: None,
description: None,
native_id: None,
states: vec![],
bounds: Some(crate::node::Rect {
x: 0.0,
y: 0.0,
width: 10.0,
height: 10.0,
}),
bounds_hash: Some(1),
available_actions: vec![capability::CLICK.into()],
source_app: None,
source_window_id: None,
source_window_title: None,
source_surface: crate::snapshot_surface::SnapshotSurface::Window,
root_ref: None,
path_is_absolute: false,
path: smallvec::SmallVec::new(),
}
}
fn request_with_timeout(timeout_ms: u64) -> ActionRequest {
ActionRequest::headless(Action::Click).with_timeout_ms(Some(timeout_ms))
}
struct UnresponsiveProcessAdapter {
probe_calls: AtomicU32,
}
impl ObservationOps for UnresponsiveProcessAdapter {
fn resolve_element_strict(&self, _entry: &RefEntry) -> Result<NativeHandle, AdapterError> {
Err(AdapterError::stale_ref("@e1"))
}
fn resolve_element_strict_with_timeout(
&self,
entry: &RefEntry,
_timeout: Duration,
) -> Result<NativeHandle, AdapterError> {
self.resolve_element_strict(entry)
}
}
impl ActionOps for UnresponsiveProcessAdapter {
fn execute_action(
&self,
_handle: &NativeHandle,
_request: ActionRequest,
) -> Result<crate::action_result::ActionResult, AdapterError> {
Ok(crate::action_result::ActionResult::new("click"))
}
}
impl InputOps for UnresponsiveProcessAdapter {}
impl SystemOps for UnresponsiveProcessAdapter {
fn process_state(&self, _pid: i32) -> Result<crate::process_state::ProcessState, AdapterError> {
self.probe_calls.fetch_add(1, Ordering::SeqCst);
Ok(crate::process_state::ProcessState::Unresponsive)
}
}
#[test]
fn terminal_stale_ref_against_unresponsive_process_surfaces_app_unresponsive() {
let adapter = UnresponsiveProcessAdapter {
probe_calls: AtomicU32::new(0),
};
let err = execute_with_auto_wait(
&adapter,
&entry(),
"@e1",
&CommandContext::default(),
ActionRequest::headless(Action::Click),
crate::ref_action::execute_resolved,
)
.unwrap_err();
assert_eq!(err.code, ErrorCode::AppUnresponsive);
assert!(
err.suggestion.is_some(),
"APP_UNRESPONSIVE must carry a recovery suggestion"
);
assert_eq!(
adapter.probe_calls.load(Ordering::SeqCst),
1,
"the liveness probe must run exactly once when building the terminal error"
);
}
struct ExitedProcessAdapter;
impl ObservationOps for ExitedProcessAdapter {
fn resolve_element_strict(&self, _entry: &RefEntry) -> Result<NativeHandle, AdapterError> {
Err(AdapterError::stale_ref("@e1"))
}
fn resolve_element_strict_with_timeout(
&self,
entry: &RefEntry,
_timeout: Duration,
) -> Result<NativeHandle, AdapterError> {
self.resolve_element_strict(entry)
}
}
impl ActionOps for ExitedProcessAdapter {
fn execute_action(
&self,
_handle: &NativeHandle,
_request: ActionRequest,
) -> Result<crate::action_result::ActionResult, AdapterError> {
Ok(crate::action_result::ActionResult::new("click"))
}
}
impl InputOps for ExitedProcessAdapter {}
impl SystemOps for ExitedProcessAdapter {
fn process_state(&self, _pid: i32) -> Result<crate::process_state::ProcessState, AdapterError> {
Ok(crate::process_state::ProcessState::Exited { code: None })
}
}
#[test]
fn terminal_stale_ref_against_exited_process_carries_process_state_detail() {
let err = execute_with_auto_wait(
&ExitedProcessAdapter,
&entry(),
"@e1",
&CommandContext::default(),
ActionRequest::headless(Action::Click),
crate::ref_action::execute_resolved,
)
.unwrap_err();
assert_eq!(
err.code,
ErrorCode::StaleRef,
"an Exited (not Unresponsive) classification must not replace the original error code"
);
assert_eq!(
err.details.as_ref().and_then(|d| d.get("process_state")),
Some(&serde_json::json!("exited")),
"STALE_REF against a dead pid must carry details.process_state = \"exited\""
);
}
struct SuccessWithUnresponsiveProbeAdapter {
probe_calls: AtomicU32,
}
impl ObservationOps for SuccessWithUnresponsiveProbeAdapter {
fn resolve_element_strict(&self, _entry: &RefEntry) -> Result<NativeHandle, AdapterError> {
Ok(NativeHandle::null())
}
fn resolve_element_strict_with_timeout(
&self,
entry: &RefEntry,
_timeout: Duration,
) -> Result<NativeHandle, AdapterError> {
self.resolve_element_strict(entry)
}
}
impl ActionOps for SuccessWithUnresponsiveProbeAdapter {
fn execute_action(
&self,
_handle: &NativeHandle,
_request: ActionRequest,
) -> Result<crate::action_result::ActionResult, AdapterError> {
Ok(crate::action_result::ActionResult::new("click"))
}
}
impl InputOps for SuccessWithUnresponsiveProbeAdapter {}
impl SystemOps for SuccessWithUnresponsiveProbeAdapter {
fn process_state(&self, _pid: i32) -> Result<crate::process_state::ProcessState, AdapterError> {
self.probe_calls.fetch_add(1, Ordering::SeqCst);
Ok(crate::process_state::ProcessState::Unresponsive)
}
}
#[test]
fn enrichment_never_converts_a_successful_action_into_a_failure() {
let adapter = SuccessWithUnresponsiveProbeAdapter {
probe_calls: AtomicU32::new(0),
};
let result = execute_with_auto_wait(
&adapter,
&entry(),
"@e1",
&CommandContext::default(),
ActionRequest::headless(Action::Click),
crate::ref_action::execute_resolved,
)
.unwrap();
assert_eq!(result.action, "click");
assert_eq!(
adapter.probe_calls.load(Ordering::SeqCst),
0,
"a successful action must never consult the liveness probe at all \
(this used to be an unconditional preflight hard-gate)"
);
}
struct TicksThenAppNotFoundAdapter {
resolve_calls: AtomicU32,
probe_calls: AtomicU32,
}
impl ObservationOps for TicksThenAppNotFoundAdapter {
fn resolve_element_strict(&self, _entry: &RefEntry) -> Result<NativeHandle, AdapterError> {
Ok(NativeHandle::null())
}
fn resolve_element_strict_with_timeout(
&self,
_entry: &RefEntry,
_timeout: Duration,
) -> Result<NativeHandle, AdapterError> {
let attempt = self.resolve_calls.fetch_add(1, Ordering::SeqCst) + 1;
if attempt < 3 {
Err(AdapterError::new(ErrorCode::StaleRef, "not yet"))
} else {
Err(AdapterError::new(ErrorCode::AppNotFound, "app gone"))
}
}
}
impl ActionOps for TicksThenAppNotFoundAdapter {
fn execute_action(
&self,
_handle: &NativeHandle,
_request: ActionRequest,
) -> Result<crate::action_result::ActionResult, AdapterError> {
Ok(crate::action_result::ActionResult::new("click"))
}
}
impl InputOps for TicksThenAppNotFoundAdapter {}
impl SystemOps for TicksThenAppNotFoundAdapter {
fn process_state(&self, _pid: i32) -> Result<crate::process_state::ProcessState, AdapterError> {
self.probe_calls.fetch_add(1, Ordering::SeqCst);
Ok(crate::process_state::ProcessState::Running)
}
}
#[test]
fn probe_call_count_is_independent_of_auto_wait_tick_count() {
let adapter = TicksThenAppNotFoundAdapter {
resolve_calls: AtomicU32::new(0),
probe_calls: AtomicU32::new(0),
};
let err = execute_with_auto_wait(
&adapter,
&entry(),
"@e1",
&CommandContext::default(),
request_with_timeout(5_000),
crate::ref_action::execute_resolved,
)
.unwrap_err();
assert_eq!(err.code, ErrorCode::AppNotFound);
assert!(
adapter.resolve_calls.load(Ordering::SeqCst) >= 3,
"the poll loop must have ticked multiple times before the terminal error"
);
assert_eq!(
adapter.probe_calls.load(Ordering::SeqCst),
1,
"the liveness probe must run exactly once at the terminal boundary, \
not once per auto-wait tick"
);
}

82
crates/macos/src/system/process_state.rs
View file

@ -1,26 +1,90 @@
use agent_desktop_core::error::AdapterError;
use agent_desktop_core::process_state::ProcessState;
/// Result of one AX responsiveness read, decoupled from the raw AXError so
/// `classify` (below) is testable without a live accessibility tree.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) enum AxProbeResult {
Responsive,
CannotComplete,
}
/// Pure classification: alive/dead + a probe closure in, `ProcessState` out.
/// Kept free of any platform API so the retry threshold (one transient
/// `CannotComplete` must not classify `Unresponsive`; two consecutive must)
/// is unit-testable on every host, not just macOS with a live AX tree.
pub(crate) fn classify(pid_alive: bool, mut probe: impl FnMut() -> AxProbeResult) -> ProcessState {
if !pid_alive {
return ProcessState::Exited { code: None };
}
match probe() {
AxProbeResult::Responsive => ProcessState::Running,
AxProbeResult::CannotComplete => match probe() {
AxProbeResult::Responsive => ProcessState::Running,
AxProbeResult::CannotComplete => ProcessState::Unresponsive,
},
}
}
#[cfg(target_os = "macos")]
pub fn process_state_impl(pid: i32) -> Result<ProcessState, AdapterError> {
use crate::tree::{copy_string_attr, element_for_pid};
use accessibility_sys::kAXRoleAttribute;
use crate::tree::element_for_pid;
Ok(classify(pid_is_alive(pid), || {
ax_probe(&element_for_pid(pid))
}))
}
/// `kill(pid, 0)`-style liveness check: signal 0 sends no actual signal, the
/// kernel only validates the target exists and is reachable. Mirrors the
/// convention already used by `system::force_close::signal_result`.
#[cfg(target_os = "macos")]
fn pid_is_alive(pid: i32) -> bool {
const POSIX_ESRCH: i32 = 3;
if pid <= 0 {
return Ok(ProcessState::Unknown);
return false;
}
let app = element_for_pid(pid);
unsafe extern "C" {
fn kill(pid: i32, sig: i32) -> i32;
}
if unsafe { kill(pid, 0) } == 0 {
return true;
}
std::io::Error::last_os_error().raw_os_error() != Some(POSIX_ESRCH)
}
#[cfg(target_os = "macos")]
fn ax_probe(app: &crate::tree::AXElement) -> AxProbeResult {
use accessibility_sys::{
AXUIElementCopyAttributeValue, kAXErrorCannotComplete, kAXErrorSuccess, kAXRoleAttribute,
};
use core_foundation::{
base::{CFType, CFTypeRef, TCFType},
string::CFString,
};
if app.0.is_null() {
return Ok(ProcessState::Unknown);
return AxProbeResult::Responsive;
}
if copy_string_attr(&app, kAXRoleAttribute).is_some() {
Ok(ProcessState::Responsive)
} else {
Ok(ProcessState::Unresponsive)
let cf_attr = CFString::new(kAXRoleAttribute);
let mut value: CFTypeRef = std::ptr::null_mut();
let err =
unsafe { AXUIElementCopyAttributeValue(app.0, cf_attr.as_concrete_TypeRef(), &mut value) };
if err == kAXErrorCannotComplete {
return AxProbeResult::CannotComplete;
}
if err == kAXErrorSuccess && !value.is_null() {
unsafe { CFType::wrap_under_create_rule(value) };
}
AxProbeResult::Responsive
}
#[cfg(not(target_os = "macos"))]
pub fn process_state_impl(_pid: i32) -> Result<ProcessState, AdapterError> {
Err(AdapterError::not_supported("process_state"))
}
#[cfg(test)]
#[path = "process_state_tests.rs"]
mod tests;

97
crates/macos/src/system/process_state_tests.rs Normal file
View file

@ -0,0 +1,97 @@
use super::*;
use agent_desktop_core::process_state::ProcessState;
use std::cell::Cell;
#[test]
fn dead_pid_classifies_exited_without_consulting_probe() {
let probe_calls = Cell::new(0);
let state = classify(false, || {
probe_calls.set(probe_calls.get() + 1);
AxProbeResult::Responsive
});
assert_eq!(state, ProcessState::Exited { code: None });
assert_eq!(
probe_calls.get(),
0,
"a dead pid must short-circuit before probing AX at all"
);
}
#[test]
fn single_transient_cannot_complete_does_not_classify_unresponsive() {
let calls = Cell::new(0);
let state = classify(true, || {
let n = calls.get() + 1;
calls.set(n);
if n == 1 {
AxProbeResult::CannotComplete
} else {
AxProbeResult::Responsive
}
});
assert_eq!(
state,
ProcessState::Running,
"one transient AX blip on a healthy-but-busy app must not hard-fail as Unresponsive"
);
assert_eq!(calls.get(), 2, "a transient failure retries exactly once");
}
#[test]
fn two_consecutive_cannot_complete_classifies_unresponsive() {
let calls = Cell::new(0);
let state = classify(true, || {
calls.set(calls.get() + 1);
AxProbeResult::CannotComplete
});
assert_eq!(state, ProcessState::Unresponsive);
assert_eq!(
calls.get(),
2,
"classification requires exactly a second consecutive CannotComplete, not more"
);
}
#[test]
fn immediately_responsive_pid_classifies_running_with_one_probe() {
let calls = Cell::new(0);
let state = classify(true, || {
calls.set(calls.get() + 1);
AxProbeResult::Responsive
});
assert_eq!(state, ProcessState::Running);
assert_eq!(calls.get(), 1);
}
#[cfg(target_os = "macos")]
#[test]
fn exited_child_process_is_classified_exited_with_no_code() {
let mut child = std::process::Command::new("/bin/echo")
.arg("hi")
.spawn()
.expect("spawn /bin/echo");
let pid = child.id() as i32;
child.wait().expect("wait for exit");
for _ in 0..50 {
if !pid_is_alive(pid) {
break;
}
std::thread::sleep(std::time::Duration::from_millis(20));
}
let state = process_state_impl(pid).expect("process_state_impl should not error");
assert_eq!(state, ProcessState::Exited { code: None });
}
#[cfg(target_os = "macos")]
#[test]
fn nonpositive_pid_is_never_alive() {
assert!(!pid_is_alive(0));
assert!(!pid_is_alive(-1));
}
#[cfg(target_os = "macos")]
#[test]
fn currently_running_pid_is_alive() {
assert!(pid_is_alive(std::process::id() as i32));
}