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

fix: harden core foundation per whole-branch review

Browse source 
Remediate the validated findings from the whole-branch review. Each was
triaged, validated against the real code, fixed on a disjoint file set, then
confirmed by an adversarial code-read review and the full gate (fmt, clippy
-D warnings, 985 unit tests, core isolation, 1.6MB binary, e2e 72/0). The
ref.resolve trace restoration was additionally verified on a real app (Finder
hover under a traced session emits start/entry/ok again).

- signals: propagate app-enumeration failures in the signal baseline instead of
  swallowing them with unwrap_or_default, mirroring list_windows_impl
- image: validate the PNG signature and IHDR chunk in one shared core parser
  (parse_png_dimensions), dedup three hand-rolled copies, and reject non-PNG
  --image input instead of tagging arbitrary bytes as a zero-dimensioned PNG
- roles: delete the dead Role enum and inline is_interactive_role as a direct
  INTERACTIVE_ROLES membership check, keeping the vocabulary-contract test
- helpers: cap each ref-resolve attempt to the wait deadline through the single
  traced resolver, so budgeted hover/drag can no longer spend the whole budget
  on one slow attempt and once again emit ref.resolve.* trace events
- actionability: run the hit_test occlusion gate even when get_live_element is
  unsupported, so occlusion is checked on hit_test-capable adapters
- launch: share the app-identifier traversal guard across both launch paths
- window_resolve: fail closed when the window-title fallback is ambiguous
  (resolve only when exactly one window matches the title)
- main: surface stdout write/flush failures from the JSON envelope instead of
  reporting success, exiting with a distinct code on a broken pipe
- cli_args: add a one-to-one Surface<->SnapshotSurface parity tripwire test
This commit is contained in:
Lahfir 2026-07-04 00:01:56 -07:00
parent eb9f7c0f06
commit adda4c977e
18 changed files with 330 additions and 219 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

50
crates/core/src/actionability/mod.rs
View file

@ -34,37 +34,35 @@ pub fn check_live(
request: &ActionRequest,
) -> Result<ActionabilityReport, AdapterError> {
let mut observed = entry.clone();
let live = match adapter.get_live_element(handle) {
Ok(live) => live,
match adapter.get_live_element(handle) {
Ok(live) => {
if live_element_is_stale(&live) {
return Err(AdapterError::new(
ErrorCode::StaleRef,
"Resolved element no longer exposes live accessibility state",
)
.with_suggestion(
"Re-run a snapshot to obtain fresh refs, then retry with the new ref (CLI: snapshot; FFI: ad_snapshot then ad_execute_by_ref with the returned snapshot_id)",
));
}
if let Some(state) = live.state {
observed.role = state.role;
observed.states = state.states;
observed.value = state.value.or(observed.value);
}
observed.bounds = live.bounds;
if let Some(actions) = live.available_actions
&& !actions.is_empty()
{
observed.available_actions = actions;
}
}
Err(err)
if matches!(
err.code,
ErrorCode::PlatformNotSupported | ErrorCode::ActionNotSupported
) =>
{
return check_with_stability(entry.bounds_hash, &observed, request, None);
}
) => {}
Err(err) => return Err(err),
};
if live_element_is_stale(&live) {
return Err(AdapterError::new(
ErrorCode::StaleRef,
"Resolved element no longer exposes live accessibility state",
)
.with_suggestion(
"Re-run a snapshot to obtain fresh refs, then retry with the new ref (CLI: snapshot; FFI: ad_snapshot then ad_execute_by_ref with the returned snapshot_id)",
));
}
if let Some(state) = live.state {
observed.role = state.role;
observed.states = state.states;
observed.value = state.value.or(observed.value);
}
observed.bounds = live.bounds;
if let Some(actions) = live.available_actions
&& !actions.is_empty()
{
observed.available_actions = actions;
}
check_with_stability(
entry.bounds_hash,

15
crates/core/src/commands/clipboard_set.rs
View file

@ -2,7 +2,7 @@ use crate::{
adapter::PlatformAdapter,
clipboard_content::ClipboardContent,
error::AppError,
image_buffer::{ImageBuffer, ImageFormat},
image_buffer::{ImageBuffer, ImageFormat, parse_png_dimensions},
};
use serde_json::{Value, json};
use std::path::PathBuf;
@ -28,7 +28,9 @@ fn build_content(args: ClipboardSetArgs) -> Result<ClipboardContent, AppError> {
}
if let Some(path) = args.image {
let data = std::fs::read(&path)?;
let (width, height) = png_dimensions(&data);
let Some((width, height)) = parse_png_dimensions(&data) else {
return Err(AppError::invalid_input("--image file is not a valid PNG"));
};
return Ok(ClipboardContent::Image(ImageBuffer {
data,
format: ImageFormat::Png,
@ -60,15 +62,6 @@ fn validate_file_urls(urls: &[String]) -> Result<Vec<String>, AppError> {
Ok(urls.to_vec())
}
fn png_dimensions(data: &[u8]) -> (u32, u32) {
if data.len() < 24 {
return (0, 0);
}
let w = u32::from_be_bytes([data[16], data[17], data[18], data[19]]);
let h = u32::from_be_bytes([data[20], data[21], data[22], data[23]]);
(w, h)
}
#[cfg(test)]
#[path = "clipboard_set_tests.rs"]
mod tests;

2
crates/core/src/commands/clipboard_set_tests.rs
View file

@ -126,6 +126,8 @@ fn image_flag_reads_bytes_and_dimensions_from_file() {
std::process::id()
));
let mut bytes = vec![0u8; 24];
bytes[0..8].copy_from_slice(&[0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]);
bytes[12..16].copy_from_slice(b"IHDR");
bytes[16..20].copy_from_slice(&11u32.to_be_bytes());
bytes[20..24].copy_from_slice(&8u32.to_be_bytes());
std::fs::write(&path, &bytes).unwrap();

77
crates/core/src/commands/helpers.rs
View file

@ -33,6 +33,24 @@ pub(crate) fn resolve_ref_with_context<'a>(
snapshot_id: Option<&str>,
adapter: &'a dyn PlatformAdapter,
context: &CommandContext,
) -> Result<(RefEntry, ResolvedElement<'a>), AppError> {
resolve_ref_within_deadline(ref_id, snapshot_id, None, adapter, context)
}
/// Single owner of ref resolution and its `ref.resolve.start/entry/ok/error`
/// tracing. With `deadline: None` the strict resolve is uncapped (the
/// `get`/`is`/single-shot pointer path); with `Some(deadline)` each resolve is
/// capped to the remaining budget via
/// [`crate::ref_action_wait::resolve_within_deadline`] so a ref-addressed
/// pointer wait (`hover`, `drag`) cannot spend its whole budget on one slow
/// attempt. Both modes emit identical telemetry, so budgeted and single-shot
/// resolution trace the same way.
fn resolve_ref_within_deadline<'a>(
ref_id: &str,
snapshot_id: Option<&str>,
deadline: Option<std::time::Instant>,
adapter: &'a dyn PlatformAdapter,
context: &CommandContext,
) -> Result<(RefEntry, ResolvedElement<'a>), AppError> {
validate_ref_id(ref_id)?;
let store = RefStore::for_session(context.session_id())?;
@ -80,7 +98,7 @@ pub(crate) fn resolve_ref_with_context<'a>(
"name": entry.name
})
})?;
let handle = adapter.resolve_element_strict(&entry).inspect_err(|err| {
let handle = resolve_handle_within_deadline(adapter, &entry, deadline).inspect_err(|err| {
let _ = context.trace_lazy("ref.resolve.error", || {
json!({
"ref": ref_id,
@ -96,6 +114,26 @@ pub(crate) fn resolve_ref_with_context<'a>(
Ok((entry, ResolvedElement::new(adapter, handle)))
}
/// Performs the strict resolve for [`resolve_ref_within_deadline`], capping the
/// attempt to `deadline` when one is supplied and surfacing an exhausted budget
/// as a `TIMEOUT`, or resolving uncapped when it is not.
fn resolve_handle_within_deadline(
adapter: &dyn PlatformAdapter,
entry: &RefEntry,
deadline: Option<std::time::Instant>,
) -> Result<crate::adapter::NativeHandle, crate::error::AdapterError> {
let Some(deadline) = deadline else {
return adapter.resolve_element_strict(entry);
};
match crate::ref_action_wait::resolve_within_deadline(adapter, entry, deadline) {
crate::ref_action_wait::ResolveAttemptOutcome::Resolved(handle) => Ok(handle),
crate::ref_action_wait::ResolveAttemptOutcome::Failed(err) => Err(err),
crate::ref_action_wait::ResolveAttemptOutcome::DeadlinePassed => Err(
crate::error::AdapterError::timeout("Target did not resolve within the wait budget"),
),
}
}
pub(crate) fn resolve_app_pid(
app: Option<&str>,
adapter: &dyn PlatformAdapter,
@ -254,13 +292,44 @@ pub(crate) fn resolve_window_for_app(
window_lookup::find_window_for_pid(pid, adapter)
}
/// Resolves the ref underlying a wait-budgeted point resolution through the
/// shared [`resolve_ref_within_deadline`] (so the capped pointer-wait path
/// emits the same `ref.resolve.*` telemetry as every other ref resolution and
/// each attempt is capped to what remains of `deadline`), then completes the
/// same bounds/actionability steps [`point_resolve::resolve_point_from_ref_or_xy_with_context`]
/// performs for its ref branch.
fn resolve_point_from_ref_capped(
ref_id: &str,
snapshot_id: Option<&str>,
deadline: std::time::Instant,
adapter: &dyn PlatformAdapter,
context: &CommandContext,
) -> Result<crate::commands::point_resolve::ResolvedPoint, AppError> {
let (entry, resolved) =
resolve_ref_within_deadline(ref_id, snapshot_id, Some(deadline), adapter, context)?;
let bounds = adapter
.get_element_bounds(resolved.handle())?
.ok_or_else(|| AppError::invalid_input(format!("Element {ref_id} has no bounds")))?;
let point = crate::action::Point {
x: bounds.x + bounds.width / 2.0,
y: bounds.y + bounds.height / 2.0,
};
crate::actionability::require_receives_events(resolved.handle(), point.clone(), adapter)?;
Ok(crate::commands::point_resolve::ResolvedPoint {
point,
pid: Some(entry.pid),
})
}
/// Resolves a ref-or-xy point the same way [`point_resolve`] does, but retries
/// transient resolve failures (`STALE_REF`, `AMBIGUOUS_TARGET`, `TIMEOUT`)
/// within `timeout_ms` — the auto-wait budget for ref-addressed pointer
/// commands (`hover`, `drag`) that resolve coordinates rather than dispatching
/// through [`execute_ref_action_with_context`]. Coordinate-only input (no
/// `ref_id`) and `timeout_ms: None` (`--timeout-ms 0`) both make exactly one
/// attempt.
/// attempt. When a ref and a budget are both present, each retry's resolve is
/// capped to the remaining budget via [`resolve_point_from_ref_capped`] so a
/// single slow resolve cannot exhaust the whole wait window.
pub(crate) fn resolve_point_with_wait<'a>(
ref_id: Option<&'a str>,
xy: Option<(f64, f64)>,
@ -286,12 +355,12 @@ pub(crate) fn resolve_point_with_wait<'a>(
context,
)
};
let Some(budget_ms) = timeout_ms.filter(|_| ref_id.is_some()) else {
let (Some(budget_ms), Some(ref_id)) = (timeout_ms, ref_id) else {
return attempt();
};
let deadline = std::time::Instant::now() + crate::ref_action_wait::budget_from_ms(budget_ms);
loop {
match attempt() {
match resolve_point_from_ref_capped(ref_id, snapshot_id, deadline, adapter, context) {
Ok(point) => return Ok(point),
Err(err) => {
let remaining = deadline.saturating_duration_since(std::time::Instant::now());

64
crates/core/src/image_buffer.rs
View file

@ -21,3 +21,67 @@ impl ImageFormat {
}
}
}
const PNG_SIGNATURE: [u8; 8] = [0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A];
const PNG_IHDR_CHUNK_TYPE: &[u8; 4] = b"IHDR";
/// Reads width/height from a PNG's `IHDR` chunk, validating the 8-byte PNG
/// signature and the `IHDR` chunk type before trusting the byte offsets.
/// Returns `None` for anything that is not a well-formed PNG header,
/// including truncated buffers.
pub fn parse_png_dimensions(data: &[u8]) -> Option<(u32, u32)> {
if data.len() < 24 {
return None;
}
if data[0..8] != PNG_SIGNATURE {
return None;
}
if &data[12..16] != PNG_IHDR_CHUNK_TYPE {
return None;
}
let width = u32::from_be_bytes([data[16], data[17], data[18], data[19]]);
let height = u32::from_be_bytes([data[20], data[21], data[22], data[23]]);
Some((width, height))
}
#[cfg(test)]
mod tests {
use super::*;
fn valid_png_header(width: u32, height: u32) -> Vec<u8> {
let mut bytes = PNG_SIGNATURE.to_vec();
bytes.extend_from_slice(&[0, 0, 0, 13]);
bytes.extend_from_slice(PNG_IHDR_CHUNK_TYPE);
bytes.extend_from_slice(&width.to_be_bytes());
bytes.extend_from_slice(&height.to_be_bytes());
bytes
}
#[test]
fn parses_dimensions_from_valid_png_header() {
let bytes = valid_png_header(640, 480);
assert_eq!(parse_png_dimensions(&bytes), Some((640, 480)));
}
#[test]
fn rejects_undersized_buffer() {
assert_eq!(parse_png_dimensions(&[1, 2, 3]), None);
}
#[test]
fn rejects_buffer_with_wrong_signature() {
let mut bytes = valid_png_header(640, 480);
bytes[0] = 0x00;
assert_eq!(parse_png_dimensions(&bytes), None);
}
#[test]
fn rejects_buffer_with_wrong_chunk_type() {
let mut bytes = valid_png_header(640, 480);
bytes[12..16].copy_from_slice(b"IDAT");
assert_eq!(parse_png_dimensions(&bytes), None);
}
}

2
crates/core/src/lib.rs
View file

@ -37,7 +37,6 @@ pub mod refs_store;
#[cfg(test)]
mod refs_test_support;
pub(crate) mod resolved_element;
pub mod role;
pub mod roles;
pub mod screenshot_target;
pub(crate) mod search_text;
@ -74,6 +73,7 @@ pub use display_info::DisplayInfo;
pub use element_state::ElementState;
pub use error::{AdapterError, AppError, ErrorCode};
pub use hit_test::HitTestResult;
pub use image_buffer::parse_png_dimensions;
pub use interaction_policy::InteractionPolicy;
pub use node::{AccessibilityNode, AppInfo, Rect, WindowInfo};
pub use notification::{NotificationFilter, NotificationInfo};

121
crates/core/src/role.rs
View file

@ -1,121 +0,0 @@
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub enum Role {
Button,
Cell,
Checkbox,
Colorwell,
Combobox,
Disclosure,
Dockitem,
Group,
Image,
Incrementor,
Link,
List,
Menubutton,
Menuitem,
Radiobutton,
Scrollarea,
Slider,
Statictext,
Switch,
Tab,
Table,
Textfield,
Treeitem,
Unknown,
}
impl Role {
pub fn as_str(self) -> &'static str {
match self {
Self::Button => "button",
Self::Cell => "cell",
Self::Checkbox => "checkbox",
Self::Colorwell => "colorwell",
Self::Combobox => "combobox",
Self::Disclosure => "disclosure",
Self::Dockitem => "dockitem",
Self::Group => "group",
Self::Image => "image",
Self::Incrementor => "incrementor",
Self::Link => "link",
Self::List => "list",
Self::Menubutton => "menubutton",
Self::Menuitem => "menuitem",
Self::Radiobutton => "radiobutton",
Self::Scrollarea => "scrollarea",
Self::Slider => "slider",
Self::Statictext => "statictext",
Self::Switch => "switch",
Self::Tab => "tab",
Self::Table => "table",
Self::Textfield => "textfield",
Self::Treeitem => "treeitem",
Self::Unknown => "unknown",
}
}
pub fn parse(role: &str) -> Self {
role.parse().unwrap_or(Self::Unknown)
}
}
impl std::str::FromStr for Role {
type Err = std::convert::Infallible;
fn from_str(role: &str) -> Result<Self, Self::Err> {
Ok(match role.trim().to_ascii_lowercase().as_str() {
"button" => Self::Button,
"cell" => Self::Cell,
"checkbox" => Self::Checkbox,
"colorwell" => Self::Colorwell,
"combobox" => Self::Combobox,
"disclosure" => Self::Disclosure,
"dockitem" => Self::Dockitem,
"group" => Self::Group,
"image" => Self::Image,
"incrementor" => Self::Incrementor,
"link" => Self::Link,
"list" => Self::List,
"menubutton" => Self::Menubutton,
"menuitem" => Self::Menuitem,
"radiobutton" => Self::Radiobutton,
"scrollarea" => Self::Scrollarea,
"slider" => Self::Slider,
"statictext" => Self::Statictext,
"switch" => Self::Switch,
"tab" => Self::Tab,
"table" => Self::Table,
"textfield" => Self::Textfield,
"treeitem" => Self::Treeitem,
_ => Self::Unknown,
})
}
}
impl Role {
pub fn is_interactive(self) -> bool {
crate::roles::INTERACTIVE_ROLES.contains(&self.as_str())
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn from_str_unknown_for_bogus_role() {
assert_eq!(Role::parse("bogus"), Role::Unknown);
}
#[test]
fn is_interactive_matches_interactive_roles_list() {
for role in crate::roles::INTERACTIVE_ROLES {
assert!(
Role::parse(role).is_interactive(),
"{role} should be interactive"
);
}
}
}

12
crates/core/src/roles.rs
View file

@ -1,5 +1,3 @@
use crate::role::Role;
/// Interactive roles that receive refs during snapshot allocation.
///
/// Each entry must be produced by at least one platform adapter's native-to-canonical
@ -41,7 +39,8 @@ pub fn normalize_role_query(role: &str) -> String {
/// Returns true when `role` is in [`INTERACTIVE_ROLES`].
pub fn is_interactive_role(role: &str) -> bool {
Role::parse(role).is_interactive()
let normalized = role.trim().to_ascii_lowercase();
INTERACTIVE_ROLES.contains(&normalized.as_str())
}
/// Returns true for roles whose checked/unchecked state can be queried and set.
@ -64,6 +63,13 @@ pub fn is_mutable_value_role(role: &str) -> bool {
mod tests {
use super::*;
#[test]
fn is_interactive_role_matches_interactive_roles_list() {
for role in INTERACTIVE_ROLES {
assert!(is_interactive_role(role), "{role} should be interactive");
}
}
#[test]
fn interactive_roles_are_sorted_and_unique() {
let mut sorted = INTERACTIVE_ROLES.to_vec();

14
crates/macos/src/input/clipboard_rich.rs
View file

@ -1,3 +1,4 @@
use agent_desktop_core::image_buffer::parse_png_dimensions;
use core_foundation::base::TCFType;
use core_foundation::string::CFString;
use core_foundation::url::CFURL;
@ -17,16 +18,11 @@ unsafe extern "C" {
static NSPasteboardTypeFileURL: Id;
}
/// Reads the width/height stored in a PNG's `IHDR` chunk directly from the
/// byte buffer, mirroring `macos::system::screenshot::png_dimensions` — no
/// image-decoding dependency needed for two big-endian `u32` reads.
/// Reads the width/height stored in a PNG's `IHDR` chunk via the shared core
/// helper, defaulting to `(0, 0)` for read-path data that fails validation
/// rather than rejecting an already-received clipboard payload.
pub(crate) fn png_dimensions(data: &[u8]) -> (u32, u32) {
if data.len() < 24 {
return (0, 0);
}
let w = u32::from_be_bytes([data[16], data[17], data[18], data[19]]);
let h = u32::from_be_bytes([data[20], data[21], data[22], data[23]]);
(w, h)
parse_png_dimensions(data).unwrap_or((0, 0))
}
pub(crate) fn read_image(pb: Id) -> Option<Vec<u8>> {

1
crates/macos/src/input/clipboard_rich_tests.rs
View file

@ -3,6 +3,7 @@ use super::*;
fn fake_png(width: u32, height: u32) -> Vec<u8> {
let mut bytes = vec![0u8; 24];
bytes[0..8].copy_from_slice(&[0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]);
bytes[12..16].copy_from_slice(b"IHDR");
bytes[16..20].copy_from_slice(&width.to_be_bytes());
bytes[20..24].copy_from_slice(&height.to_be_bytes());
bytes

1
crates/macos/src/input/clipboard_tests.rs
View file

@ -28,6 +28,7 @@ impl Drop for RestoreGuard {
fn fake_png(width: u32, height: u32) -> Vec<u8> {
let mut bytes = vec![0u8; 24];
bytes[0..8].copy_from_slice(&[0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]);
bytes[12..16].copy_from_slice(b"IHDR");
bytes[16..20].copy_from_slice(&width.to_be_bytes());
bytes[20..24].copy_from_slice(&height.to_be_bytes());
bytes

25
crates/macos/src/system/launch.rs
View file

@ -17,6 +17,8 @@ pub fn launch_app_with_options_impl(
const OPEN_TIMEOUT: Duration = Duration::from_secs(5);
validate_app_identifier(id)?;
let filter = WindowFilter {
focused_only: false,
app: Some(id.to_string()),
@ -92,13 +94,7 @@ pub fn launch_app_impl(id: &str, timeout_ms: u64) -> Result<WindowInfo, AdapterE
const OPEN_TIMEOUT: Duration = Duration::from_secs(5);
if id.contains("..") || id.starts_with('/') {
return Err(AdapterError::new(
ErrorCode::InvalidArgs,
format!("Invalid app identifier: '{id}'"),
)
.with_suggestion("Use an app name like 'Safari' or bundle ID like 'com.apple.Safari'."));
}
validate_app_identifier(id)?;
let filter = WindowFilter {
focused_only: false,
@ -144,6 +140,21 @@ pub fn launch_app_impl(_id: &str, _timeout_ms: u64) -> Result<WindowInfo, Adapte
Err(AdapterError::not_supported("launch_app"))
}
/// Rejects app identifiers that look like filesystem traversal attempts
/// (`..` segments or an absolute path) rather than a bare app name or
/// bundle ID, so both launch entrypoints refuse the same inputs.
#[cfg(target_os = "macos")]
fn validate_app_identifier(id: &str) -> Result<(), AdapterError> {
if id.contains("..") || id.starts_with('/') {
return Err(AdapterError::new(
ErrorCode::InvalidArgs,
format!("Invalid app identifier: '{id}'"),
)
.with_suggestion("Use an app name like 'Safari' or bundle ID like 'com.apple.Safari'."));
}
Ok(())
}
#[cfg(target_os = "macos")]
fn open_app_args(id: &str) -> [&str; 3] {
["-g", "-a", id]

12
crates/macos/src/system/screenshot.rs
View file

@ -1,6 +1,7 @@
use agent_desktop_core::{
adapter::{ImageBuffer, ImageFormat},
error::{AdapterError, ErrorCode},
image_buffer::parse_png_dimensions,
};
#[cfg(target_os = "macos")]
@ -116,7 +117,7 @@ mod imp {
fn read_png(path: &Path) -> Result<ImageBuffer, AdapterError> {
let data = std::fs::read(path)
.map_err(|e| AdapterError::internal(format!("read screenshot: {e}")))?;
let (width, height) = png_dimensions(&data);
let (width, height) = parse_png_dimensions(&data).unwrap_or((0, 0));
Ok(ImageBuffer {
data,
format: ImageFormat::Png,
@ -152,15 +153,6 @@ mod imp {
AdapterError::internal("screencapture exited with error").with_platform_detail(detail)
}
fn png_dimensions(data: &[u8]) -> (u32, u32) {
if data.len() < 24 {
return (0, 0);
}
let w = u32::from_be_bytes([data[16], data[17], data[18], data[19]]);
let h = u32::from_be_bytes([data[20], data[21], data[22], data[23]]);
(w, h)
}
fn find_cg_window_id_for_pid(pid: i32) -> Option<u32> {
let mut best_id: Option<u32> = None;
let mut best_area: f64 = 0.0;

14
crates/macos/src/system/signals.rs
View file

@ -29,7 +29,7 @@ pub fn capture_signal_baseline_impl(filter: &SignalFilter) -> Result<SignalBasel
focused_only: false,
app: filter.app.clone(),
})?;
let apps = matching_apps(filter);
let apps = matching_apps(filter)?;
let surfaces = surfaces_for_apps(filter, &apps);
Ok(SignalBaseline {
windows,
@ -46,18 +46,18 @@ pub fn capture_signal_baseline_impl(
}
#[cfg(target_os = "macos")]
fn matching_apps(filter: &SignalFilter) -> Vec<AppInfo> {
let all = crate::system::app_list::list_apps_impl().unwrap_or_default();
fn matching_apps(filter: &SignalFilter) -> Result<Vec<AppInfo>, AdapterError> {
let all = crate::system::app_list::list_apps_impl()?;
if let Some(name) = &filter.app {
return all
return Ok(all
.into_iter()
.filter(|app| app.name.eq_ignore_ascii_case(name))
.collect();
.collect());
}
if let Some(pid) = filter.pid {
return all.into_iter().filter(|app| app.pid == pid).collect();
return Ok(all.into_iter().filter(|app| app.pid == pid).collect());
}
all
Ok(all)
}
/// Surfaces (sheets, alerts, popovers, open menus) are only inspected for

2
crates/macos/src/system/signals_tests.rs
View file

@ -19,7 +19,7 @@ fn matching_apps_filters_by_name_case_insensitively() {
app: Some("Definitely-Not-A-Real-App-xyz123".into()),
pid: None,
};
let apps = matching_apps(&filter);
let apps = matching_apps(&filter).expect("app enumeration must succeed on the test host");
assert!(
apps.is_empty(),
"an app name that matches no running process must yield no apps"

26
crates/macos/src/system/window_resolve.rs
View file

@ -73,6 +73,18 @@ fn window_info_from_record(win: &WindowInfo, record: &WindowRecord) -> WindowInf
}
}
/// Matches the CGWindow-verified `window_number` against the app's live
/// `AXWindow` elements.
///
/// The CGWindow record was already verified (pid + title) by
/// `locate_verified_record`, so a total bridge miss means
/// `_AXUIElementGetWindow` transiently failed (busy/hung app, benign race),
/// not that the window is gone. In that case, fall back to the AX window
/// matching the verified title before reporting `WINDOW_NOT_FOUND` — but only
/// when the title identifies exactly one live `AXWindow`. If zero or two-plus
/// windows share that title, the correct target cannot be determined safely,
/// so this returns `None` and lets the caller surface `WINDOW_NOT_FOUND`
/// rather than risk acting on the wrong same-titled window.
fn ax_window_element_for_number(
pid: i32,
window_number: i64,
@ -88,16 +100,16 @@ fn ax_window_element_for_number(
return Some(window.clone());
}
}
// The CGWindow record was already verified (pid + title) by
// locate_verified_record, so a total bridge miss means
// _AXUIElementGetWindow transiently failed (busy/hung app, benign race),
// not that the window is gone. Fall back to the AX window matching the
// verified title before reporting WINDOW_NOT_FOUND.
let title = fallback_title?;
windows.into_iter().find(|window| {
let mut matches = windows.into_iter().filter(|window| {
copy_string_attr(window, "AXRole").as_deref() == Some("AXWindow")
&& copy_string_attr(window, "AXTitle").as_deref() == Some(title)
})
});
let single_match = matches.next()?;
match matches.next() {
Some(_) => None,
None => Some(single_match),
}
}
/// Bridges an accessibility window element to its CoreGraphics window number

73
src/cli_args/mod_tests.rs
View file

@ -1,4 +1,77 @@
use super::*;
use agent_desktop_core::adapter::SnapshotSurface;
use clap::ValueEnum;
/// The exhaustive set of `SnapshotSurface` values the CLI's `Surface` enum is
/// meant to expose. `SnapshotSurface` is `#[non_exhaustive]`, so a new variant
/// added to it will not force `Surface::to_core`'s match arms (which match on
/// the local `Surface`, not on `SnapshotSurface`) to change — this list is the
/// independent tripwire that must be updated by hand alongside any new
/// `Surface`/`to_core` arm.
const EXPECTED_CORE_SURFACES: &[SnapshotSurface] = &[
SnapshotSurface::Window,
SnapshotSurface::Focused,
SnapshotSurface::Menu,
SnapshotSurface::Menubar,
SnapshotSurface::Sheet,
SnapshotSurface::Popover,
SnapshotSurface::Alert,
];
/// Keeps the CLI `Surface` enum and the hand-maintained `EXPECTED_CORE_SURFACES`
/// tripwire in one-to-one correspondence: every `Surface` variant (enumerated
/// via its `ValueEnum` derive, so this side stays exhaustive automatically) maps
/// through `.to_core()` to a distinct `SnapshotSurface`, the tripwire lists each
/// of those exactly once, and the two sets match. An inconsistent hand-edit — a
/// `to_core` arm that collides with another, or the tripwire updated without a
/// matching `Surface`/`to_core` arm (or the reverse) — fails here. The one gap it
/// cannot close: because `SnapshotSurface` is `#[non_exhaustive]` with no
/// reflection, a new core variant added with no CLI-side edit keeps both sides
/// their current size and passes — the by-hand tripwire update is the only
/// checkpoint for that case (see `EXPECTED_CORE_SURFACES`).
#[test]
fn surface_to_core_maps_one_to_one_onto_expected_core_surfaces() {
let cli_variants = Surface::value_variants();
assert_eq!(
cli_variants.len(),
EXPECTED_CORE_SURFACES.len(),
"Surface has {} variants but EXPECTED_CORE_SURFACES has {} — \
update EXPECTED_CORE_SURFACES (and Surface::to_core) to match",
cli_variants.len(),
EXPECTED_CORE_SURFACES.len()
);
for (i, a) in EXPECTED_CORE_SURFACES.iter().enumerate() {
for (j, b) in EXPECTED_CORE_SURFACES.iter().enumerate() {
assert!(
i == j || a != b,
"EXPECTED_CORE_SURFACES lists SnapshotSurface::{a:?} more than once — \
each expected core surface must appear exactly once"
);
}
}
let mapped: Vec<SnapshotSurface> = cli_variants.iter().map(Surface::to_core).collect();
for (i, a) in mapped.iter().enumerate() {
for (j, b) in mapped.iter().enumerate() {
assert!(
i == j || a != b,
"Surface::{:?} and Surface::{:?} both map to the same \
SnapshotSurface::{a:?} via to_core — mapping must be distinct",
cli_variants[i],
cli_variants[j]
);
}
}
for expected in EXPECTED_CORE_SURFACES {
assert!(
mapped.contains(expected),
"no Surface variant maps to SnapshotSurface::{expected:?} via to_core — \
a Surface arm is missing for this core surface"
);
}
}
/// God-object regression: `FindArgs` used to carry 14 flat fields; the
/// match-criteria and result-shaping groups now live in

38
src/main.rs
View file

@ -23,6 +23,8 @@ use cli::{Cli, Commands};
use cli_args::skills::SkillsAction;
use std::io::{BufWriter, Write};
const EXIT_STDOUT_WRITE_FAILURE: i32 = 74;
const WAIT_SUPPORTED: &[&str] = &[
"snapshot",
"click",
@ -55,10 +57,12 @@ fn main() {
}
let msg = e.to_string();
let first_line = msg.lines().next().unwrap_or("parse error");
emit_response(&Response::err(
if let Err(write_err) = emit_response(&Response::err(
"unknown",
ErrorPayload::new("INVALID_ARGS", first_line),
));
)) {
exit_after_emit_failure(write_err);
}
std::process::exit(2);
}
};
@ -165,20 +169,32 @@ fn run_with_adapter(cmd: Commands, cmd_name: &str, context: &CommandContext) {
fn finish(cmd_name: &str, result: Result<serde_json::Value, agent_desktop_core::error::AppError>) {
match result {
Ok(data) => {
emit_response(&Response::ok(cmd_name, data));
if let Err(write_err) = emit_response(&Response::ok(cmd_name, data)) {
exit_after_emit_failure(write_err);
}
std::process::exit(0);
}
Err(e) => {
emit_response(&Response::err(
if let Err(write_err) = emit_response(&Response::err(
cmd_name,
agent_desktop_core::ErrorPayload::from_app_error(&e),
));
)) {
exit_after_emit_failure(write_err);
}
std::process::exit(1);
}
}
}
fn emit_response(response: &Response) {
/// Reports a stdout write failure on stderr and terminates with a distinct
/// non-zero exit code, so a broken pipe or full disk is never mistaken for a
/// successful command outcome.
fn exit_after_emit_failure(write_err: std::io::Error) -> ! {
eprintln!("agent-desktop: failed to write response to stdout: {write_err}");
std::process::exit(EXIT_STDOUT_WRITE_FAILURE);
}
fn emit_response(response: &Response) -> std::io::Result<()> {
match serde_json::to_value(response) {
Ok(value) => emit_json(&value),
Err(err) => emit_json(&serde_json::json!({
@ -193,14 +209,12 @@ fn emit_response(response: &Response) {
}
}
fn emit_json(value: &serde_json::Value) {
fn emit_json(value: &serde_json::Value) -> std::io::Result<()> {
let stdout = std::io::stdout();
let mut writer = BufWriter::new(stdout.lock());
if serde_json::to_writer(&mut writer, value).is_err() {
return;
}
let _ = writer.write_all(b"\n");
let _ = writer.flush();
serde_json::to_writer(&mut writer, value).map_err(std::io::Error::other)?;
writer.write_all(b"\n")?;
writer.flush()
}
fn build_adapter() -> impl agent_desktop_core::adapter::PlatformAdapter {