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

fix: preserve safe window title fallback

Browse source
This commit is contained in:
Lahfir 2026-06-04 08:52:25 -07:00
parent d281d5dac5
commit cd3b0ed29d
3 changed files with 113 additions and 27 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

30
crates/macos/src/tree/resolve.rs
View file

@ -1,5 +1,5 @@
use agent_desktop_core::{
adapter::{NativeHandle, SnapshotSurface},
adapter::NativeHandle,
error::{AdapterError, ErrorCode},
refs::RefEntry,
};
@ -15,7 +15,9 @@ use super::resolve_deadline::{
ensure_before_deadline, remaining_before_deadline, sleep_before_retry,
};
use super::resolve_identity::{has_meaningful_identity, identity_matches};
use super::resolve_roots::{candidate_roots, path_candidate_roots, source_window_number};
use super::resolve_roots::{
candidate_roots, path_candidate_roots, source_window_number, source_window_scope_required,
};
#[cfg(target_os = "macos")]
pub fn resolve_element_impl(entry: &RefEntry) -> Result<NativeHandle, AdapterError> {
@ -32,7 +34,8 @@ pub fn resolve_element_with_timeout(
for attempt in 0..attempts {
if can_use_path_fast_path(entry) {
let path_roots = path_candidate_roots(entry, deadline)?;
match find_entry_by_path(&path_roots, entry, deadline) {
let scope_verified = path_roots.scope_verified;
match find_entry_by_path(&path_roots.roots, entry, scope_verified, deadline) {
Ok(handle) => {
return Ok(handle);
}
@ -53,7 +56,8 @@ pub fn resolve_element_with_timeout(
continue;
}
let roots = candidate_roots(entry, deadline)?;
match find_entry_in_roots(&roots, entry, resolve_depth, deadline) {
let scope_verified = roots.scope_verified;
match find_entry_in_roots(&roots.roots, entry, resolve_depth, scope_verified, deadline) {
Ok(handle) => {
return Ok(handle);
}
@ -95,7 +99,7 @@ fn requires_scoped_path_resolution(entry: &RefEntry) -> bool {
(entry.root_ref.is_none() || entry.path_is_absolute)
&& entry.bounds_hash.is_none()
&& !entry.path.is_empty()
&& source_window_number(entry).is_some()
&& source_window_scope_required(entry)
}
#[cfg(target_os = "macos")]
@ -107,6 +111,7 @@ fn can_use_broad_search(entry: &RefEntry) -> bool {
fn find_entry_by_path(
roots: &[AXElement],
entry: &RefEntry,
source_window_verified: bool,
deadline: Instant,
) -> Result<NativeHandle, AdapterError> {
if entry.path.is_empty() {
@ -129,7 +134,7 @@ fn find_entry_by_path(
}
}
classify_candidates(matches, entry)
classify_candidates(matches, entry, source_window_verified)
}
#[cfg(target_os = "macos")]
@ -157,6 +162,7 @@ fn find_entry_in_roots(
roots: &[AXElement],
entry: &RefEntry,
resolve_depth: u8,
source_window_verified: bool,
deadline: Instant,
) -> Result<NativeHandle, AdapterError> {
let mut matches = Vec::new();
@ -176,21 +182,20 @@ fn find_entry_in_roots(
};
collect_elements_recursive(root, 0, &mut context)?;
}
classify_candidates(matches, entry)
classify_candidates(matches, entry, source_window_verified)
}
#[cfg(target_os = "macos")]
fn classify_candidates(
mut matches: Vec<AXElement>,
entry: &RefEntry,
source_window_verified: bool,
) -> Result<NativeHandle, AdapterError> {
match matches.len() {
0 => Err(AdapterError::element_not_found("element")),
1 => {
let candidate = matches.remove(0);
if source_window_scope_verifies_lone_match(entry)
|| verified_bounds_match(&candidate, entry)
{
if source_window_verified || verified_bounds_match(&candidate, entry) {
retained_handle(candidate)
} else {
Err(AdapterError::element_not_found("element"))
@ -200,11 +205,6 @@ fn classify_candidates(
}
}
#[cfg(target_os = "macos")]
fn source_window_scope_verifies_lone_match(entry: &RefEntry) -> bool {
matches!(entry.source_surface, SnapshotSurface::Window) && source_window_number(entry).is_some()
}
#[cfg(target_os = "macos")]
fn verified_bounds_match(candidate: &AXElement, entry: &RefEntry) -> bool {
entry.bounds_hash.is_some() && bounds_match(candidate, entry)

59
crates/macos/src/tree/resolve_roots.rs
View file

@ -9,26 +9,34 @@ use super::element::element_for_pid;
use super::element_dedupe::ElementDedupe;
use super::resolve_deadline::{ensure_before_deadline, remaining_before_deadline};
#[cfg(target_os = "macos")]
pub(super) struct CandidateRoots {
pub roots: Vec<AXElement>,
pub scope_verified: bool,
}
#[cfg(target_os = "macos")]
pub(super) fn path_candidate_roots(
entry: &RefEntry,
deadline: Instant,
) -> Result<Vec<AXElement>, AdapterError> {
) -> Result<CandidateRoots, AdapterError> {
if entry.bounds_hash.is_some() {
return candidate_roots(entry, deadline);
}
Ok(scoped_surface_root(entry, deadline)?.into_iter().collect())
let roots: Vec<_> = scoped_surface_root(entry, deadline)?.into_iter().collect();
Ok(CandidateRoots {
scope_verified: source_window_scope_required(entry) && !roots.is_empty(),
roots,
})
}
#[cfg(target_os = "macos")]
pub(super) fn candidate_roots(
entry: &RefEntry,
deadline: Instant,
) -> Result<Vec<AXElement>, AdapterError> {
) -> Result<CandidateRoots, AdapterError> {
if source_window_scope_required(entry) {
return Ok(exact_source_window_number_root(entry, deadline)?
.into_iter()
.collect());
return source_window_scoped_roots(entry, deadline);
}
let root = element_for_pid(entry.pid);
@ -61,7 +69,35 @@ pub(super) fn candidate_roots(
if roots.is_empty() {
roots.push(root);
}
Ok(roots)
Ok(CandidateRoots {
roots,
scope_verified: false,
})
}
#[cfg(target_os = "macos")]
fn source_window_scoped_roots(
entry: &RefEntry,
deadline: Instant,
) -> Result<CandidateRoots, AdapterError> {
if let Some(window) = exact_source_window_number_root(entry, deadline)? {
return Ok(CandidateRoots {
roots: vec![window],
scope_verified: true,
});
}
if source_window_title_fallback_allowed(entry) {
return Ok(CandidateRoots {
roots: exact_source_window_root(entry, deadline)?
.into_iter()
.collect(),
scope_verified: false,
});
}
Ok(CandidateRoots {
roots: Vec::new(),
scope_verified: false,
})
}
#[cfg(target_os = "macos")]
@ -132,10 +168,17 @@ fn exact_source_window_root(
}
#[cfg(target_os = "macos")]
fn source_window_scope_required(entry: &RefEntry) -> bool {
pub(super) fn source_window_scope_required(entry: &RefEntry) -> bool {
matches!(entry.source_surface, SnapshotSurface::Window) && source_window_number(entry).is_some()
}
#[cfg(target_os = "macos")]
pub(super) fn source_window_title_fallback_allowed(entry: &RefEntry) -> bool {
source_window_scope_required(entry)
&& entry.bounds_hash.is_some()
&& entry.source_window_title.is_some()
}
#[cfg(target_os = "macos")]
pub(super) fn source_window_number(entry: &RefEntry) -> Option<i64> {
entry

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

@ -1,5 +1,6 @@
use super::*;
use crate::tree::resolve_roots::source_window_number;
use crate::tree::resolve_roots::{source_window_number, source_window_title_fallback_allowed};
use agent_desktop_core::adapter::SnapshotSurface;
fn entry(
bounds_hash: Option<u64>,
@ -20,7 +21,7 @@ fn entry(
source_app: None,
source_window_id: source_window_id.map(String::from),
source_window_title: source_window_title.map(String::from),
source_surface: agent_desktop_core::adapter::SnapshotSurface::Window,
source_surface: SnapshotSurface::Window,
root_ref: root_ref.map(String::from),
path_is_absolute: false,
path: smallvec::smallvec![0, 1],
@ -155,6 +156,7 @@ fn expired_deadline_fails_before_path_resolution_reads() {
let err = match find_entry_by_path(
&[],
&entry(Some(42), Some("w-42"), Some("Documents"), None),
false,
std::time::Instant::now(),
) {
Ok(_) => panic!("expected timeout"),
@ -172,6 +174,7 @@ fn ambiguous_candidate_classification_reports_structured_details() {
AXElement(std::ptr::null_mut()),
],
&entry(None, Some("w-42"), Some("Documents"), None),
true,
) {
Ok(_) => panic!("expected ambiguous target"),
Err(err) => err,
@ -189,15 +192,17 @@ fn single_meaningful_identity_candidate_resolves_after_bounds_change() {
let _handle = classify_candidates(
vec![AXElement(std::ptr::null_mut())],
&entry(None, Some("w-42"), Some("Documents"), None),
true,
)
.expect("unique identity should resolve within the verified source window");
}
#[test]
fn single_identity_candidate_without_window_or_bounds_fails_closed() {
fn cross_window_replacement_without_verified_source_window_fails_closed() {
let err = match classify_candidates(
vec![AXElement(std::ptr::null_mut())],
&entry(None, None, Some("Documents"), None),
&entry(None, Some("w-42"), Some("Documents"), None),
false,
) {
Ok(_) => panic!("expected stale candidate to fail closed"),
Err(err) => err,
@ -206,6 +211,44 @@ fn single_identity_candidate_without_window_or_bounds_fails_closed() {
assert_eq!(err.code, ErrorCode::ElementNotFound);
}
#[test]
fn non_window_identity_candidate_without_bounds_fails_closed() {
let mut menu_entry = entry(None, None, None, None);
menu_entry.source_surface = SnapshotSurface::Menu;
let err = match classify_candidates(vec![AXElement(std::ptr::null_mut())], &menu_entry, false) {
Ok(_) => panic!("expected stale candidate to fail closed"),
Err(err) => err,
};
assert_eq!(err.code, ErrorCode::ElementNotFound);
}
#[test]
fn source_window_title_fallback_requires_bounds_hash() {
assert!(source_window_title_fallback_allowed(&entry(
Some(42),
Some("w-10"),
Some("Documents"),
None
)));
assert!(!source_window_title_fallback_allowed(&entry(
None,
Some("w-10"),
Some("Documents"),
None
)));
assert!(!source_window_title_fallback_allowed(&entry(
Some(42),
Some("w-10"),
None,
None
)));
let mut menu_entry = entry(Some(42), Some("w-10"), Some("Documents"), None);
menu_entry.source_surface = SnapshotSurface::Menu;
assert!(!source_window_title_fallback_allowed(&menu_entry));
}
#[test]
fn bounds_hash_keeps_collecting_to_disambiguate_identity_matches() {
assert!(!should_stop_collecting(