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

refactor: act on the independent review of sub-phase 2.2

Browse source 
Ten findings triaged; seven were well founded and are fixed, three were not
and are answered in the PR rather than changed.

The stalest thing in the diff was a doc comment. root_from_hwnd still claimed
a target stopping between the probe and the call "still blocks" and that
bounding it was a hang guard 2.2 does not own. Both stopped being true when
the CUIAutomation8 connection timeout landed; a comment understating a
guarantee misleads 2.4 exactly as badly as one overstating it.

The failure cap dropped faults silently past eight. The count now travels on
the last reported error, so a consumer cannot read a systemic failure as a
local one, and a test asserts reported plus suppressed equals the total.

The not-supported sentinel was fetched through FFI on every property of every
node; it is a documented process singleton, so it is fetched once per thread.

The HRESULT table existed twice, in permissions.rs and automation.rs, so a
code added to one read as unnamed in the other. One table now, in
system/hresult.rs.

An inverted BoundingRectangle produced negative width and height. It is
degenerate, not negative-sized, and now collapses to zero extent - the same
shape A14-8 measured on a minimized top level.

The stalled fixture parked a thread for 120 s per test. It polls a stop flag
instead, which is still not pumping, and teardown joins it in milliseconds.

The property-id grep matched the substring "300", so a timeout constant or a
prose mention would have failed it. It now matches whole five-digit tokens in
UIA's 30000-30999 block, and was observed catching a planted literal.

Two coverage gaps closed: the cached-equals-uncached test covered three of ten
properties and now covers all ten, and the plan's U5 redaction scenario now
has the live test it asked for - a forced read failure on the control whose
text is the marker, asserting the marker reaches no error.

Also splits the property domain types per the one-type-per-file rule and
narrows cache, walker_enumerate and captures to the visibility they need.
This commit is contained in:
Lahfir 2026-07-28 18:42:31 -06:00
parent 55141fe2fa
commit eb32b03f99
18 changed files with 479 additions and 273 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

73
crates/windows/src/system/hresult.rs Normal file
View file

@ -0,0 +1,73 @@
//! The one place a COM status code is named.
//!
//! Both the permission probe and the UI Automation tree path classify and
//! format HRESULTs. Holding two tables meant adding a code in one and reading
//! it as unnamed in the other, so the table lives here and both import it.
pub(crate) const S_OK: i32 = 0;
pub(crate) const E_NOINTERFACE: i32 = 0x8000_4002_u32 as i32;
pub(crate) const E_POINTER: i32 = 0x8000_4003_u32 as i32;
pub(crate) const E_FAIL: i32 = 0x8000_4005_u32 as i32;
pub(crate) const E_ACCESSDENIED: i32 = 0x8007_0005_u32 as i32;
pub(crate) const E_INVALIDARG: i32 = 0x8007_0057_u32 as i32;
pub(crate) const CO_E_NOTINITIALIZED: i32 = 0x8004_01F0_u32 as i32;
pub(crate) const RPC_E_SERVERFAULT: i32 = 0x8001_0105_u32 as i32;
pub(crate) const RPC_E_DISCONNECTED: i32 = 0x8001_0108_u32 as i32;
pub(crate) const RPC_S_SERVER_UNAVAILABLE: i32 = 0x8007_06BA_u32 as i32;
pub(crate) const RPC_S_CALL_FAILED: i32 = 0x8007_06BE_u32 as i32;
pub(crate) const UIA_E_ELEMENTNOTENABLED: i32 = 0x8004_0200_u32 as i32;
pub(crate) const UIA_E_ELEMENTNOTAVAILABLE: i32 = 0x8004_0201_u32 as i32;
pub(crate) const UIA_E_NOCLICKABLEPOINT: i32 = 0x8004_0202_u32 as i32;
pub(crate) const UIA_E_PROXYASSEMBLYNOTLOADED: i32 = 0x8004_0203_u32 as i32;
pub(crate) const UIA_E_NOTSUPPORTED: i32 = 0x8004_0204_u32 as i32;
pub(crate) const UIA_E_TIMEOUT: i32 = 0x8013_1505_u32 as i32;
pub(crate) const UIA_E_INVALIDOPERATION: i32 = 0x8013_1509_u32 as i32;
/// Renders an HRESULT for `platform_detail`.
///
/// Shape only: a code and, where one is known, its symbol and meaning. No
/// entry derives from an observed application.
pub(crate) fn com_hresult_detail(hresult: i32) -> String {
let code = hresult as u32;
match com_hresult_symbol(hresult) {
Some((symbol, meaning)) => format!("COM HRESULT 0x{code:08X} ({symbol}: {meaning})"),
None => format!("COM HRESULT 0x{code:08X}"),
}
}
/// Names the HRESULTs this crate's COM paths can raise.
///
/// An unlisted code formats as a bare hexadecimal value rather than being
/// guessed at.
pub(crate) fn com_hresult_symbol(hresult: i32) -> Option<(&'static str, &'static str)> {
let symbol = match hresult {
E_ACCESSDENIED => ("E_ACCESSDENIED", "Access is denied"),
E_NOINTERFACE => ("E_NOINTERFACE", "No such interface supported"),
E_POINTER => ("E_POINTER", "Invalid pointer"),
E_FAIL => ("E_FAIL", "Unspecified failure"),
E_INVALIDARG => ("E_INVALIDARG", "One or more arguments are invalid"),
CO_E_NOTINITIALIZED => ("CO_E_NOTINITIALIZED", "COM has not been initialized"),
RPC_E_SERVERFAULT => ("RPC_E_SERVERFAULT", "The server raised an exception"),
RPC_E_DISCONNECTED => ("RPC_E_DISCONNECTED", "The object invoked has disconnected"),
RPC_S_SERVER_UNAVAILABLE => ("RPC_S_SERVER_UNAVAILABLE", "The RPC server is unavailable"),
RPC_S_CALL_FAILED => ("RPC_S_CALL_FAILED", "The remote procedure call failed"),
UIA_E_ELEMENTNOTENABLED => ("UIA_E_ELEMENTNOTENABLED", "The element is not enabled"),
UIA_E_ELEMENTNOTAVAILABLE => ("UIA_E_ELEMENTNOTAVAILABLE", "The element is not available"),
UIA_E_NOCLICKABLEPOINT => (
"UIA_E_NOCLICKABLEPOINT",
"The element has no clickable point",
),
UIA_E_PROXYASSEMBLYNOTLOADED => (
"UIA_E_PROXYASSEMBLYNOTLOADED",
"The proxy assembly could not be loaded",
),
UIA_E_NOTSUPPORTED => (
"UIA_E_NOTSUPPORTED",
"The requested operation is unsupported",
),
UIA_E_TIMEOUT => ("UIA_E_TIMEOUT", "The operation timed out"),
UIA_E_INVALIDOPERATION => ("UIA_E_INVALIDOPERATION", "The operation is not valid"),
_ => return None,
};
Some(symbol)
}

1
crates/windows/src/system/mod.rs
View file

@ -1,6 +1,7 @@
mod adapter;
pub(crate) mod com_runtime;
pub(crate) mod dpi;
pub(crate) mod hresult;
pub(crate) mod permissions;
#[cfg(target_os = "windows")]
pub(crate) mod private_file;

65
crates/windows/src/system/permissions.rs
View file

@ -2,24 +2,8 @@ use agent_desktop_core::{AdapterError, Deadline, PermissionReport, PermissionSta
const ACCESSIBILITY_SUGGESTION: &str = "Run agent-desktop in an interactive desktop session as a user allowed to use the UI Automation COM runtime; restricted tokens and AppContainer processes are denied UIA access.";
const S_OK: i32 = 0;
const E_ACCESSDENIED: i32 = 0x8007_0005_u32 as i32;
const E_NOINTERFACE: i32 = 0x8000_4002_u32 as i32;
const E_POINTER: i32 = 0x8000_4003_u32 as i32;
const E_FAIL: i32 = 0x8000_4005_u32 as i32;
const E_INVALIDARG: i32 = 0x8007_0057_u32 as i32;
const CO_E_NOTINITIALIZED: i32 = 0x8004_01F0_u32 as i32;
const RPC_E_SERVERFAULT: i32 = 0x8001_0105_u32 as i32;
const RPC_E_DISCONNECTED: i32 = 0x8001_0108_u32 as i32;
const RPC_S_SERVER_UNAVAILABLE: i32 = 0x8007_06BA_u32 as i32;
const RPC_S_CALL_FAILED: i32 = 0x8007_06BE_u32 as i32;
const UIA_E_ELEMENTNOTENABLED: i32 = 0x8004_0200_u32 as i32;
const UIA_E_ELEMENTNOTAVAILABLE: i32 = 0x8004_0201_u32 as i32;
const UIA_E_NOCLICKABLEPOINT: i32 = 0x8004_0202_u32 as i32;
const UIA_E_PROXYASSEMBLYNOTLOADED: i32 = 0x8004_0203_u32 as i32;
const UIA_E_NOTSUPPORTED: i32 = 0x8004_0204_u32 as i32;
const UIA_E_TIMEOUT: i32 = 0x8013_1505_u32 as i32;
const UIA_E_INVALIDOPERATION: i32 = 0x8013_1509_u32 as i32;
pub(crate) use crate::system::hresult::com_hresult_detail;
use crate::system::hresult::{E_ACCESSDENIED, S_OK};
#[cfg(target_os = "windows")]
mod imp {
@ -146,51 +130,6 @@ pub(crate) fn uia_access_denied_error(hresult: i32) -> AdapterError {
.with_platform_detail(com_hresult_detail(hresult))
}
pub(crate) fn com_hresult_detail(hresult: i32) -> String {
let code = hresult as u32;
match com_hresult_symbol(hresult) {
Some((symbol, meaning)) => format!("COM HRESULT 0x{code:08X} ({symbol}: {meaning})"),
None => format!("COM HRESULT 0x{code:08X}"),
}
}
/// Names the HRESULTs the UI Automation client path can raise, so
/// `platform_detail` carries a symbol rather than a bare hexadecimal code.
///
/// The table is shape only: no entry derives from an observed application.
pub(crate) fn com_hresult_symbol(hresult: i32) -> Option<(&'static str, &'static str)> {
let symbol = match hresult {
E_ACCESSDENIED => ("E_ACCESSDENIED", "Access is denied"),
E_NOINTERFACE => ("E_NOINTERFACE", "No such interface supported"),
E_POINTER => ("E_POINTER", "Invalid pointer"),
E_FAIL => ("E_FAIL", "Unspecified failure"),
E_INVALIDARG => ("E_INVALIDARG", "One or more arguments are invalid"),
CO_E_NOTINITIALIZED => ("CO_E_NOTINITIALIZED", "COM has not been initialized"),
RPC_E_SERVERFAULT => ("RPC_E_SERVERFAULT", "The server raised an exception"),
RPC_E_DISCONNECTED => ("RPC_E_DISCONNECTED", "The object invoked has disconnected"),
RPC_S_SERVER_UNAVAILABLE => ("RPC_S_SERVER_UNAVAILABLE", "The RPC server is unavailable"),
RPC_S_CALL_FAILED => ("RPC_S_CALL_FAILED", "The remote procedure call failed"),
UIA_E_ELEMENTNOTENABLED => ("UIA_E_ELEMENTNOTENABLED", "The element is not enabled"),
UIA_E_ELEMENTNOTAVAILABLE => ("UIA_E_ELEMENTNOTAVAILABLE", "The element is not available"),
UIA_E_NOCLICKABLEPOINT => (
"UIA_E_NOCLICKABLEPOINT",
"The element has no clickable point",
),
UIA_E_PROXYASSEMBLYNOTLOADED => (
"UIA_E_PROXYASSEMBLYNOTLOADED",
"The proxy assembly could not be loaded",
),
UIA_E_NOTSUPPORTED => (
"UIA_E_NOTSUPPORTED",
"The requested operation is unsupported",
),
UIA_E_TIMEOUT => ("UIA_E_TIMEOUT", "The operation timed out"),
UIA_E_INVALIDOPERATION => ("UIA_E_INVALIDOPERATION", "The operation is not valid"),
_ => return None,
};
Some(symbol)
}
fn screen_recording_report_state() -> PermissionState {
map_capture_availability(imp::probe_capture_availability())
}

2
crates/windows/src/system/permissions_tests.rs
View file

@ -66,7 +66,7 @@ fn unnamed_hresults_format_without_inventing_a_name() {
com_hresult_detail(0x8007_0002_u32 as i32),
"COM HRESULT 0x80070002"
);
assert!(com_hresult_symbol(0x8007_0002_u32 as i32).is_none());
assert!(crate::system::hresult::com_hresult_symbol(0x8007_0002_u32 as i32).is_none());
}
#[test]

33
crates/windows/src/tree/automation.rs
View file

@ -1,6 +1,12 @@
use agent_desktop_core::{AdapterError, Deadline, ErrorCode};
use crate::system::permissions::{com_hresult_detail, ensure_budget};
use crate::system::hresult::{
CO_E_NOTINITIALIZED, E_ACCESSDENIED, E_INVALIDARG, E_POINTER, RPC_E_DISCONNECTED,
RPC_E_SERVERFAULT, RPC_S_CALL_FAILED, RPC_S_SERVER_UNAVAILABLE, UIA_E_ELEMENTNOTAVAILABLE,
UIA_E_ELEMENTNOTENABLED, UIA_E_INVALIDOPERATION, UIA_E_NOTSUPPORTED, UIA_E_TIMEOUT,
com_hresult_detail,
};
use crate::system::permissions::ensure_budget;
pub const ERR_NONE: i32 = 0;
pub const ERR_NOTFOUND: i32 = 1;
@ -13,20 +19,6 @@ pub const ERR_INVALID_OBJECT: i32 = 7;
pub const ERR_ALREADY_RUNNING: i32 = 8;
pub const ERR_INVALID_ARG: i32 = 9;
const E_ACCESSDENIED: i32 = 0x8007_0005_u32 as i32;
const E_POINTER: i32 = 0x8000_4003_u32 as i32;
const E_INVALIDARG: i32 = 0x8007_0057_u32 as i32;
const CO_E_NOTINITIALIZED: i32 = 0x8004_01F0_u32 as i32;
const RPC_E_SERVERFAULT: i32 = 0x8001_0105_u32 as i32;
const RPC_E_DISCONNECTED: i32 = 0x8001_0108_u32 as i32;
const RPC_S_SERVER_UNAVAILABLE: i32 = 0x8007_06BA_u32 as i32;
const RPC_S_CALL_FAILED: i32 = 0x8007_06BE_u32 as i32;
const UIA_E_ELEMENTNOTENABLED: i32 = 0x8004_0200_u32 as i32;
const UIA_E_ELEMENTNOTAVAILABLE: i32 = 0x8004_0201_u32 as i32;
const UIA_E_NOTSUPPORTED: i32 = 0x8004_0204_u32 as i32;
const UIA_E_TIMEOUT: i32 = 0x8013_1505_u32 as i32;
const UIA_E_INVALIDOPERATION: i32 = 0x8013_1509_u32 as i32;
const COM_UNINITIALIZED_SUGGESTION: &str =
"Join the calling thread to the COM multithreaded apartment before observing the desktop";
@ -264,10 +256,13 @@ mod imp {
/// put a bound on exactly this question. A target that is already hung
/// becomes a structured `APP_UNRESPONSIVE` instead of an indefinite block.
///
/// This is a mitigation, not a guarantee: a target that stops pumping in
/// the window between the probe and the call still blocks. Bounding that
/// needs the call issued on a thread this sub-phase can abandon, which is
/// a hang guard 2.2 does not own.
/// The probe is the fast, precise answer, not the safety net. A target
/// that answers it and then stops dispatching cannot be caught by any
/// preflight, so the bound that actually holds is the client's own
/// `ConnectionTimeout` (see `create_bounded_client`): that call returns
/// `UIA_E_TIMEOUT` rather than blocking. The probe exists because it turns
/// an already-hung target into a clearer error, sooner, than waiting the
/// connection timeout out.
pub fn root_from_hwnd(hwnd: isize, deadline: Deadline) -> Result<UIAElement, AdapterError> {
crate::system::permissions::ensure_budget(deadline)?;
let client = automation_client()?;

6
crates/windows/src/tree/cache_tests.rs
View file

@ -158,11 +158,7 @@ mod windows_only {
for child in &children {
let (cached, _) = read_cached(child);
let (live, _) = read_live(child);
for property in [
TreeProperty::ClassName,
TreeProperty::Name,
TreeProperty::Value,
] {
for property in TreeProperty::WALK_SET {
assert_eq!(
cached.get(property),
live.get(property),

2
crates/windows/src/tree/captures.rs
View file

@ -13,7 +13,7 @@
//! window handle or a user path is a defect regardless of what tree it holds.
/// The committed dev-box captures, by the target variant each records.
pub const CAPTURE_FILES: [&str; 2] = ["notepad-com.json", "explorer-com.json"];
const CAPTURE_FILES: [&str; 2] = ["notepad-com.json", "explorer-com.json"];
#[cfg(test)]
#[path = "captures_tests.rs"]

96
crates/windows/src/tree/element_properties.rs Normal file
View file

@ -0,0 +1,96 @@
use agent_desktop_core::{
ElementIdentifier, IdentifierEvidence, IdentifierKind, LocatorEvidence, LocatorField,
LocatorRefEvidence,
};
use super::property_ids::TreeProperty;
use super::property_outcome::{PropertyOutcome, PropertyValue};
/// Every property read for one element, already gated on `IsPassword`.
#[derive(Debug, Clone, Default)]
pub struct ElementProperties {
entries: Vec<(TreeProperty, PropertyOutcome)>,
secure: bool,
}
impl ElementProperties {
pub fn from_reads(reads: Vec<(TreeProperty, PropertyOutcome)>) -> Self {
let secure = reads
.iter()
.find(|(property, _)| *property == TreeProperty::IsPassword)
.and_then(|(_, outcome)| outcome.flag())
.unwrap_or(false);
let entries = reads
.into_iter()
.map(|(property, outcome)| {
if secure && property.is_value_bearing() {
(property, PropertyOutcome::Absent)
} else {
(property, outcome)
}
})
.collect();
Self { entries, secure }
}
pub fn is_secure(&self) -> bool {
self.secure
}
pub fn get(&self, property: TreeProperty) -> PropertyOutcome {
self.entries
.iter()
.find(|(candidate, _)| *candidate == property)
.map(|(_, outcome)| outcome.clone())
.unwrap_or(PropertyOutcome::Unknown)
}
/// Projects the read set onto the evidence slot shape core consumes, so
/// 2.4 needs no translation layer.
///
/// `role` and `available_actions` come from the 2.3 seams and are
/// deliberately `Unknown` until 2.3 fills them; `states` likewise.
/// `identifiers` uses `IdentifierEvidence::typed`, because
/// `IdentifierEvidence::new` stamps every value `Unknown` and would void
/// the ref downstream in `refs_validate.rs`.
pub fn into_locator_evidence(
self,
role: LocatorField<String>,
available_actions: LocatorField<Vec<String>>,
) -> LocatorEvidence {
let name = self.get(TreeProperty::Name).text();
let value = self.get(TreeProperty::Value).text();
let description = self.get(TreeProperty::HelpText).text();
let bounds = self.get(TreeProperty::BoundingRectangle).bounds();
LocatorEvidence {
role,
name,
description,
value,
identifiers: self.identifier_evidence(),
states: LocatorField::Unknown,
ref_evidence: LocatorRefEvidence {
bounds,
available_actions,
},
}
}
fn identifier_evidence(&self) -> IdentifierEvidence {
let automation_id = self.get(TreeProperty::AutomationId);
match automation_id {
PropertyOutcome::Known(PropertyValue::Text(value)) if !value.trim().is_empty() => {
IdentifierEvidence::typed(
[ElementIdentifier {
kind: IdentifierKind::AutomationId,
value,
}],
Some(0),
true,
)
}
PropertyOutcome::Known(_) | PropertyOutcome::Absent => IdentifierEvidence::absent(),
PropertyOutcome::Unknown => IdentifierEvidence::unknown(),
}
}
}

34
crates/windows/src/tree/fixture.rs
View file

@ -1,5 +1,7 @@
use std::io::{BufRead, BufReader};
use std::process::{Child, Command, Stdio};
use std::sync::Arc;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::mpsc::{Sender, channel};
use std::thread::{JoinHandle, spawn};
use std::time::Duration;
@ -203,26 +205,40 @@ impl Drop for LocalFixture {
/// A window whose thread owns it but never dispatches its messages.
///
/// Deliberately leaks its thread: the thread is sleeping and cannot be joined
/// without waiting it out, which is the whole point of the fixture. It is
/// bounded by the sleep, so a test run cannot retain it indefinitely.
/// The host thread cannot be joined the ordinary way - joining means waiting
/// for it to finish, and finishing means it stopped owning the window this
/// fixture exists to keep stalled. Instead it is told to stop and polls for
/// that signal, so teardown ends it in milliseconds rather than leaving a
/// parked thread per test.
pub(crate) struct StalledFixture {
handle: isize,
class_name: String,
stop: Arc<AtomicBool>,
host: Option<JoinHandle<()>>,
}
impl StalledFixture {
pub(crate) fn create() -> Result<Self, String> {
let class_name = fixture_window::unique_class_name();
let stop = Arc::new(AtomicBool::new(false));
let (sender, receiver) = channel();
spawn({
let host = spawn({
let class_name = class_name.clone();
move || fixture_window::stalled_window(&class_name, sender)
let stop = stop.clone();
move || fixture_window::stalled_window(&class_name, sender, stop)
});
match receiver.recv_timeout(READY_TIMEOUT) {
Ok(Ok(handle)) => Ok(Self { handle, class_name }),
Ok(Ok(handle)) => Ok(Self {
handle,
class_name,
stop,
host: Some(host),
}),
Ok(Err(error)) => Err(error),
Err(_) => Err(String::from("the stalled window never became ready")),
Err(_) => {
stop.store(true, Ordering::SeqCst);
Err(String::from("the stalled window never became ready"))
}
}
}
@ -233,6 +249,10 @@ impl StalledFixture {
impl Drop for StalledFixture {
fn drop(&mut self) {
self.stop.store(true, Ordering::SeqCst);
if let Some(host) = self.host.take() {
let _ = host.join();
}
fixture_window::unregister_class(&self.class_name);
}
}

18
crates/windows/src/tree/fixture_window.rs
View file

@ -271,6 +271,13 @@ pub(crate) fn destroy_window(handle: isize) {
unsafe { DestroyWindow(handle as *mut c_void) };
}
/// How often a stalled host looks for its stop signal.
///
/// Sleeping is not pumping - the queue is still never serviced, so the window
/// stays stalled for a `SendMessage` - but it lets teardown end the thread in
/// milliseconds instead of leaving one parked per test for two minutes.
const STALL_POLL: std::time::Duration = std::time::Duration::from_millis(25);
/// Creates a window and then deliberately never pumps its queue.
///
/// The 2.2 plan records "whether a non-pumping target produces a clean timeout
@ -278,7 +285,11 @@ pub(crate) fn destroy_window(handle: isize) {
/// can: `CreateWindowExW` dispatches `WM_CREATE` inline, so a thread can own a
/// live window and then stop dispatching. That makes the resolver's pump probe
/// testable instead of assumed.
pub(crate) fn stalled_window(class_name: &str, ready: Sender<Result<isize, String>>) {
pub(crate) fn stalled_window(
class_name: &str,
ready: Sender<Result<isize, String>>,
stop: std::sync::Arc<std::sync::atomic::AtomicBool>,
) {
if let Err(error) = register_class(class_name) {
let _ = ready.send(Err(error));
return;
@ -307,7 +318,10 @@ pub(crate) fn stalled_window(class_name: &str, ready: Sender<Result<isize, Strin
}
unsafe { ShowWindow(window, SW_SHOWNOACTIVATE) };
let _ = ready.send(Ok(window as isize));
std::thread::sleep(std::time::Duration::from_secs(120));
while !stop.load(std::sync::atomic::Ordering::SeqCst) {
std::thread::sleep(STALL_POLL);
}
unsafe { DestroyWindow(window) };
}
pub(crate) fn geometry(handle: isize) -> WindowGeometry {

10
crates/windows/src/tree/mod.rs
View file

@ -1,11 +1,15 @@
pub mod automation;
pub mod cache;
pub mod captures;
pub(crate) mod cache;
#[cfg(test)]
mod captures;
pub mod element;
pub mod element_properties;
pub mod properties;
pub mod property_ids;
pub mod property_outcome;
pub mod walker;
pub mod walker_enumerate;
pub(crate) mod walker_enumerate;
pub mod walker_source;
#[cfg(test)]

194
crates/windows/src/tree/properties.rs
View file

@ -1,163 +1,9 @@
use agent_desktop_core::{
AdapterError, ElementIdentifier, IdentifierEvidence, IdentifierKind, LocatorEvidence,
LocatorField, LocatorRefEvidence, Rect,
};
use agent_desktop_core::AdapterError;
use super::property_ids::TreeProperty;
/// Longest string this sub-phase will carry into evidence.
///
/// A value past the bound is `Unknown` rather than a truncated `Known`: a
/// prefix that is presented as exact identity evidence would make 2.5's
/// re-identification match the wrong element.
pub const MAX_EVIDENCE_CHARS: usize = 2_048;
/// One property read, in the three states core's `LocatorField` distinguishes.
///
/// UI Automation has no per-property error channel. macOS gets a parallel
/// array where an absent slot is `kCFNull` and a failed slot carries its own
/// error; UIA has neither, so this type is built by hand from the
/// not-supported sentinel, the variant tag, and the call's own result.
#[derive(Debug, Clone, PartialEq)]
pub enum PropertyOutcome {
/// The provider answered with a value.
Known(PropertyValue),
/// The provider answered, and does not implement this property.
Absent,
/// The read failed, or its answer cannot be trusted as identity evidence.
Unknown,
}
#[derive(Debug, Clone, PartialEq)]
pub enum PropertyValue {
Text(String),
Flag(bool),
Number(i32),
Bounds(Rect),
}
impl PropertyOutcome {
pub fn text(&self) -> LocatorField<String> {
match self {
Self::Known(PropertyValue::Text(value)) => LocatorField::Known(value.clone()),
Self::Known(_) => LocatorField::Unknown,
Self::Absent => LocatorField::Absent,
Self::Unknown => LocatorField::Unknown,
}
}
pub fn flag(&self) -> Option<bool> {
match self {
Self::Known(PropertyValue::Flag(value)) => Some(*value),
_ => None,
}
}
pub fn number(&self) -> Option<i32> {
match self {
Self::Known(PropertyValue::Number(value)) => Some(*value),
_ => None,
}
}
pub fn bounds(&self) -> LocatorField<Rect> {
match self {
Self::Known(PropertyValue::Bounds(value)) => LocatorField::Known(*value),
Self::Known(_) => LocatorField::Unknown,
Self::Absent => LocatorField::Absent,
Self::Unknown => LocatorField::Unknown,
}
}
}
/// Every property read for one element, already gated on `IsPassword`.
#[derive(Debug, Clone, Default)]
pub struct ElementProperties {
entries: Vec<(TreeProperty, PropertyOutcome)>,
secure: bool,
}
impl ElementProperties {
pub fn from_reads(reads: Vec<(TreeProperty, PropertyOutcome)>) -> Self {
let secure = reads
.iter()
.find(|(property, _)| *property == TreeProperty::IsPassword)
.and_then(|(_, outcome)| outcome.flag())
.unwrap_or(false);
let entries = reads
.into_iter()
.map(|(property, outcome)| {
if secure && property.is_value_bearing() {
(property, PropertyOutcome::Absent)
} else {
(property, outcome)
}
})
.collect();
Self { entries, secure }
}
pub fn is_secure(&self) -> bool {
self.secure
}
pub fn get(&self, property: TreeProperty) -> PropertyOutcome {
self.entries
.iter()
.find(|(candidate, _)| *candidate == property)
.map(|(_, outcome)| outcome.clone())
.unwrap_or(PropertyOutcome::Unknown)
}
/// Projects the read set onto the evidence slot shape core consumes, so
/// 2.4 needs no translation layer.
///
/// `role` and `available_actions` come from the 2.3 seams and are
/// deliberately `Unknown` until 2.3 fills them; `states` likewise.
/// `identifiers` uses `IdentifierEvidence::typed`, because
/// `IdentifierEvidence::new` stamps every value `Unknown` and would void
/// the ref downstream in `refs_validate.rs`.
pub fn into_locator_evidence(
self,
role: LocatorField<String>,
available_actions: LocatorField<Vec<String>>,
) -> LocatorEvidence {
let name = self.get(TreeProperty::Name).text();
let value = self.get(TreeProperty::Value).text();
let description = self.get(TreeProperty::HelpText).text();
let bounds = self.get(TreeProperty::BoundingRectangle).bounds();
LocatorEvidence {
role,
name,
description,
value,
identifiers: self.identifier_evidence(),
states: LocatorField::Unknown,
ref_evidence: LocatorRefEvidence {
bounds,
available_actions,
},
}
}
fn identifier_evidence(&self) -> IdentifierEvidence {
let automation_id = self.get(TreeProperty::AutomationId);
match automation_id {
PropertyOutcome::Known(PropertyValue::Text(value)) if !value.trim().is_empty() => {
IdentifierEvidence::typed(
[ElementIdentifier {
kind: IdentifierKind::AutomationId,
value,
}],
Some(0),
true,
)
}
PropertyOutcome::Known(_) | PropertyOutcome::Absent => IdentifierEvidence::absent(),
PropertyOutcome::Unknown => IdentifierEvidence::unknown(),
}
}
}
pub use super::element_properties::ElementProperties;
pub use super::property_outcome::{MAX_EVIDENCE_CHARS, PropertyOutcome, PropertyValue};
/// Bounds a string read and reports whether it survived intact.
///
@ -193,7 +39,7 @@ mod imp {
use uiautomation::Error as UiaError;
use uiautomation::variants::{Value, Variant};
use windows::Win32::UI::Accessibility::UiaGetReservedNotSupportedValue;
use windows::core::Interface;
use windows::core::{IUnknown, Interface};
/// Reads the walk property set from one element, live.
///
@ -279,12 +125,20 @@ mod imp {
(ElementProperties::from_reads(reads), errors)
}
/// Converts a UI Automation rectangle into core's.
///
/// An inverted rectangle - `right` left of `left` - is degenerate, not
/// negative-sized. It collapses to zero extent, which is the same shape a
/// minimized top-level window reports (A14-8), rather than travelling
/// downstream as a negative width that no consumer expects.
fn rect_outcome(rectangle: uiautomation::types::Rect) -> PropertyOutcome {
let width = rectangle.get_right() - rectangle.get_left();
let height = rectangle.get_bottom() - rectangle.get_top();
PropertyOutcome::Known(PropertyValue::Bounds(Rect {
x: f64::from(rectangle.get_left()),
y: f64::from(rectangle.get_top()),
width: f64::from(rectangle.get_right() - rectangle.get_left()),
height: f64::from(rectangle.get_bottom() - rectangle.get_top()),
width: f64::from(width.max(0)),
height: f64::from(height.max(0)),
}))
}
@ -314,14 +168,26 @@ mod imp {
}
}
thread_local! {
/// The not-supported sentinel, fetched once per thread.
///
/// UI Automation documents it as a process singleton, so its address
/// is stable and the comparison below is a pointer test. Fetching it
/// per property per element turned a constant into an FFI call on the
/// hottest path in the walk.
static NOT_SUPPORTED: Option<IUnknown> =
unsafe { UiaGetReservedNotSupportedValue() }.ok();
}
fn is_not_supported(variant: &Variant) -> bool {
let Ok(Value::UNKNOWN(candidate)) = variant.get_value() else {
return false;
};
let Ok(sentinel) = (unsafe { UiaGetReservedNotSupportedValue() }) else {
return false;
};
candidate.as_raw() == sentinel.as_raw()
NOT_SUPPORTED.with(|sentinel| {
sentinel
.as_ref()
.is_some_and(|sentinel| candidate.as_raw() == sentinel.as_raw())
})
}
}

53
crates/windows/src/tree/properties_live_tests.rs
View file

@ -167,3 +167,56 @@ fn a_read_that_genuinely_fails_classifies_unknown_and_never_absent() {
PropertyOutcome::Absent
);
}
/// The plan's U5 scenario, against a live provider rather than a synthetic
/// error: a failed read on a control whose text *is* a unique marker must
/// produce an error carrying none of it.
///
/// The failure is forced deterministically with an empty cache request, so
/// the element is real, its text is real, and the read genuinely cannot be
/// answered. `ref_action.rs` clones message and details into session JSONL
/// and the trace HTML export, so a leak here is persisted, not transient.
#[test]
fn a_failed_read_on_a_marker_bearing_control_leaks_none_of_it() {
bootstrap();
let fixture = HostedFixture::spawn().expect("the fixture host starts");
let client = crate::tree::automation::automation_client().expect("a UIA client");
let request = client
.create_cache_request()
.expect("an empty cache request builds");
request
.set_element_mode(uiautomation::types::ElementMode::Full)
.expect("the element mode is settable");
let marked = walk_children(fixture.handle())
.into_iter()
.find(|child| {
matches!(
read_live(child).0.get(TreeProperty::Value),
PropertyOutcome::Known(PropertyValue::Text(ref value))
if value.contains(CONTENT_MARKER)
)
})
.expect("the fixture exposes a control carrying the marker");
let uncacheable = marked
.0
.build_updated_cache(&request)
.map(crate::tree::element::UIAElement::from)
.expect("the same element with an empty cache");
let (_, errors) = read_cached(&uncacheable);
assert!(!errors.is_empty(), "the read must genuinely have failed");
for error in errors {
let rendered = format!(
"{}|{}|{}",
error.message,
error.platform_detail.unwrap_or_default(),
serde_json::to_string(&error.details).unwrap_or_default()
);
assert!(
!rendered.contains(CONTENT_MARKER),
"a failed read leaked the control's text: {rendered}"
);
}
}

1
crates/windows/src/tree/properties_tests.rs
View file

@ -1,4 +1,5 @@
use super::*;
use agent_desktop_core::{IdentifierKind, LocatorField, Rect};
fn text(value: &str) -> PropertyOutcome {
PropertyOutcome::Known(PropertyValue::Text(value.into()))

39
crates/windows/src/tree/property_ids_tests.rs
View file

@ -67,18 +67,43 @@ fn every_property_resolves_through_the_crate_generated_constants() {
/// A2-5 measured that UIA property ids are build-specific and named 2.2 as
/// the place a hand-written table would fail silently, so the source must
/// contain no bare property-id integer.
///
/// Matched as a whole token in the range UIA actually uses, not as the
/// substring "300": a prose mention of 300 milliseconds, or a `30_000` ms
/// constant, is not a property id and must not fail this.
#[test]
fn no_property_id_integer_appears_in_this_module() {
for source in [
include_str!("property_ids.rs"),
include_str!("properties.rs"),
include_str!("cache.rs"),
for (name, source) in [
("property_ids.rs", include_str!("property_ids.rs")),
("properties.rs", include_str!("properties.rs")),
("cache.rs", include_str!("cache.rs")),
] {
for line in source.lines() {
for (number, line) in source.lines().enumerate() {
let trimmed = line.trim_start();
if trimmed.starts_with("///") || trimmed.starts_with("//!") {
continue;
}
assert!(
!line.contains("300") || line.trim_start().starts_with("///"),
"a UIA property id literal appeared in: {line}"
!contains_property_id_literal(line),
"{name}:{} carries a UIA property-id literal: {line}",
number + 1
);
}
}
}
/// Reports whether a line contains a bare integer in UIA's property-id range.
///
/// A token is a candidate only when it is a whole number, five digits long,
/// and between 30000 and 30999 - the block UIA allocates property ids from.
fn contains_property_id_literal(line: &str) -> bool {
line.split(|character: char| !(character.is_ascii_digit() || character == '_'))
.filter(|token| !token.is_empty())
.map(|token| token.replace('_', ""))
.any(|token| {
token.len() == 5
&& token
.parse::<u32>()
.is_ok_and(|value| (30_000..=30_999).contains(&value))
})
}

66
crates/windows/src/tree/property_outcome.rs Normal file
View file

@ -0,0 +1,66 @@
use agent_desktop_core::{LocatorField, Rect};
/// Longest string this sub-phase will carry into evidence.
///
/// A value past the bound is `Unknown` rather than a truncated `Known`: a
/// prefix that is presented as exact identity evidence would make 2.5's
/// re-identification match the wrong element.
pub const MAX_EVIDENCE_CHARS: usize = 2_048;
/// One property read, in the three states core's `LocatorField` distinguishes.
///
/// UI Automation has no per-property error channel. macOS gets a parallel
/// array where an absent slot is `kCFNull` and a failed slot carries its own
/// error; UIA has neither, so this type is built by hand from the
/// not-supported sentinel, the variant tag, and the call's own result.
#[derive(Debug, Clone, PartialEq)]
pub enum PropertyOutcome {
/// The provider answered with a value.
Known(PropertyValue),
/// The provider answered, and does not implement this property.
Absent,
/// The read failed, or its answer cannot be trusted as identity evidence.
Unknown,
}
#[derive(Debug, Clone, PartialEq)]
pub enum PropertyValue {
Text(String),
Flag(bool),
Number(i32),
Bounds(Rect),
}
impl PropertyOutcome {
pub fn text(&self) -> LocatorField<String> {
match self {
Self::Known(PropertyValue::Text(value)) => LocatorField::Known(value.clone()),
Self::Known(_) => LocatorField::Unknown,
Self::Absent => LocatorField::Absent,
Self::Unknown => LocatorField::Unknown,
}
}
pub fn flag(&self) -> Option<bool> {
match self {
Self::Known(PropertyValue::Flag(value)) => Some(*value),
_ => None,
}
}
pub fn number(&self) -> Option<i32> {
match self {
Self::Known(PropertyValue::Number(value)) => Some(*value),
_ => None,
}
}
pub fn bounds(&self) -> LocatorField<Rect> {
match self {
Self::Known(PropertyValue::Bounds(value)) => LocatorField::Known(*value),
Self::Known(_) => LocatorField::Unknown,
Self::Absent => LocatorField::Absent,
Self::Unknown => LocatorField::Unknown,
}
}
}

26
crates/windows/src/tree/walker_enumerate.rs
View file

@ -30,6 +30,7 @@ pub struct TreeWalk<'a, S: TreeSource> {
stats: LocatorStats,
complete: bool,
failures: Vec<AdapterError>,
suppressed_failures: u32,
}
impl<'a, S: TreeSource> TreeWalk<'a, S> {
@ -41,6 +42,7 @@ impl<'a, S: TreeSource> TreeWalk<'a, S> {
stats: LocatorStats::default(),
complete: true,
failures: Vec::new(),
suppressed_failures: 0,
}
}
@ -49,6 +51,7 @@ impl<'a, S: TreeSource> TreeWalk<'a, S> {
/// Zero on every exit from a completed `visit`, including the exits taken
/// when enumeration faults. A missed removal is the bug this guard ships
/// with most easily, so it is observable rather than implied.
#[cfg(test)]
pub fn ancestor_depth(&self) -> usize {
self.ancestors.len()
}
@ -88,7 +91,11 @@ impl<'a, S: TreeSource> TreeWalk<'a, S> {
self.ancestors.len()
)));
}
Ok((self.stats, self.complete, self.failures))
let mut failures = self.failures;
if let Some(last) = failures.last_mut() {
annotate_suppressed(last, self.suppressed_failures);
}
Ok((self.stats, self.complete, failures))
}
fn visit_entered(
@ -222,6 +229,7 @@ impl<'a, S: TreeSource> TreeWalk<'a, S> {
fn record(&mut self, failure: UiaFailure, axis: &str, raw_depth: u8, child_index: usize) {
self.stats.reads.health.cannot_complete += 1;
if self.failures.len() >= MAX_REPORTED_FAILURES {
self.suppressed_failures += 1;
return;
}
self.failures
@ -262,6 +270,22 @@ fn enumeration_error(
}))
}
/// Names how many further faults the cap dropped.
///
/// The cap keeps one pathological target from flooding a trace segment, but a
/// consumer that sees eight errors and no count would read a systemic failure
/// as a local one. The number travels; the dropped errors do not.
fn annotate_suppressed(error: &mut AdapterError, suppressed: u32) {
if suppressed == 0 {
return;
}
let mut details = error.details.take().unwrap_or_else(|| json!({}));
if let Some(map) = details.as_object_mut() {
map.insert("suppressed_failures".into(), json!(suppressed));
}
error.details = Some(details);
}
/// Records whether every native predecessor of this edge was retained.
fn retained_edge_certainty(prefix_certain: &mut bool, retained: bool) -> bool {
let edge_certain = *prefix_certain;

33
crates/windows/src/tree/walker_tests.rs
View file

@ -258,3 +258,36 @@ fn the_complete_case_projects_and_the_incomplete_case_is_refused() {
.is_err()
);
}
/// The failure cap keeps one pathological target from flooding a trace
/// segment, but a consumer that sees the cap and no count would read a
/// systemic failure as a local one. The count travels even though the dropped
/// errors do not.
#[test]
fn faults_beyond_the_reporting_cap_are_counted_rather_than_vanishing() {
let children: Vec<i32> = (2..=40).collect();
let mut fake = FakeTree::default().with_children(1, &children);
for child in &children {
fake = fake.faulting_on_first_child(*child);
}
let outcome = walk(&fake, budget(10));
assert!(!outcome.tree.is_complete());
let suppressed = outcome
.failures
.last()
.and_then(|error| error.details.as_ref())
.and_then(|details| details.get("suppressed_failures"))
.and_then(serde_json::Value::as_u64)
.unwrap_or_default();
assert!(
suppressed > 0,
"a walk that dropped faults must say how many"
);
assert_eq!(
outcome.stats.reads.health.cannot_complete,
suppressed + outcome.failures.len() as u64,
"the counted total must account for every fault, reported or not"
);
}