diff --git a/crates/windows/src/system/hresult.rs b/crates/windows/src/system/hresult.rs new file mode 100644 index 0000000..cc45a55 --- /dev/null +++ b/crates/windows/src/system/hresult.rs @@ -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) +} diff --git a/crates/windows/src/system/mod.rs b/crates/windows/src/system/mod.rs index 6a3aeb4..499d232 100644 --- a/crates/windows/src/system/mod.rs +++ b/crates/windows/src/system/mod.rs @@ -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; diff --git a/crates/windows/src/system/permissions.rs b/crates/windows/src/system/permissions.rs index 1bf9ff6..4c43e5d 100644 --- a/crates/windows/src/system/permissions.rs +++ b/crates/windows/src/system/permissions.rs @@ -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()) } diff --git a/crates/windows/src/system/permissions_tests.rs b/crates/windows/src/system/permissions_tests.rs index e8491f4..620b8e9 100644 --- a/crates/windows/src/system/permissions_tests.rs +++ b/crates/windows/src/system/permissions_tests.rs @@ -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] diff --git a/crates/windows/src/tree/automation.rs b/crates/windows/src/tree/automation.rs index 964cddb..60916f1 100644 --- a/crates/windows/src/tree/automation.rs +++ b/crates/windows/src/tree/automation.rs @@ -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 { crate::system::permissions::ensure_budget(deadline)?; let client = automation_client()?; diff --git a/crates/windows/src/tree/cache_tests.rs b/crates/windows/src/tree/cache_tests.rs index cb564b2..475f642 100644 --- a/crates/windows/src/tree/cache_tests.rs +++ b/crates/windows/src/tree/cache_tests.rs @@ -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), diff --git a/crates/windows/src/tree/captures.rs b/crates/windows/src/tree/captures.rs index 88d3d3d..674d69a 100644 --- a/crates/windows/src/tree/captures.rs +++ b/crates/windows/src/tree/captures.rs @@ -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"] diff --git a/crates/windows/src/tree/element_properties.rs b/crates/windows/src/tree/element_properties.rs new file mode 100644 index 0000000..c1e6eb6 --- /dev/null +++ b/crates/windows/src/tree/element_properties.rs @@ -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, + available_actions: LocatorField>, + ) -> 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(), + } + } +} diff --git a/crates/windows/src/tree/fixture.rs b/crates/windows/src/tree/fixture.rs index 1cabf66..787a5fb 100644 --- a/crates/windows/src/tree/fixture.rs +++ b/crates/windows/src/tree/fixture.rs @@ -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, + host: Option>, } impl StalledFixture { pub(crate) fn create() -> Result { 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); } } diff --git a/crates/windows/src/tree/fixture_window.rs b/crates/windows/src/tree/fixture_window.rs index 715478a..50c8e0d 100644 --- a/crates/windows/src/tree/fixture_window.rs +++ b/crates/windows/src/tree/fixture_window.rs @@ -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>) { +pub(crate) fn stalled_window( + class_name: &str, + ready: Sender>, + stop: std::sync::Arc, +) { 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 WindowGeometry { diff --git a/crates/windows/src/tree/mod.rs b/crates/windows/src/tree/mod.rs index f57fb9b..eaf361e 100644 --- a/crates/windows/src/tree/mod.rs +++ b/crates/windows/src/tree/mod.rs @@ -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)] diff --git a/crates/windows/src/tree/properties.rs b/crates/windows/src/tree/properties.rs index 1ab6a87..5ec53c3 100644 --- a/crates/windows/src/tree/properties.rs +++ b/crates/windows/src/tree/properties.rs @@ -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 { - 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 { - match self { - Self::Known(PropertyValue::Flag(value)) => Some(*value), - _ => None, - } - } - - pub fn number(&self) -> Option { - match self { - Self::Known(PropertyValue::Number(value)) => Some(*value), - _ => None, - } - } - - pub fn bounds(&self) -> LocatorField { - 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, - available_actions: LocatorField>, - ) -> 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 = + 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()) + }) } } diff --git a/crates/windows/src/tree/properties_live_tests.rs b/crates/windows/src/tree/properties_live_tests.rs index 15da532..ebad043 100644 --- a/crates/windows/src/tree/properties_live_tests.rs +++ b/crates/windows/src/tree/properties_live_tests.rs @@ -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}" + ); + } +} diff --git a/crates/windows/src/tree/properties_tests.rs b/crates/windows/src/tree/properties_tests.rs index a58f8c8..309af79 100644 --- a/crates/windows/src/tree/properties_tests.rs +++ b/crates/windows/src/tree/properties_tests.rs @@ -1,4 +1,5 @@ use super::*; +use agent_desktop_core::{IdentifierKind, LocatorField, Rect}; fn text(value: &str) -> PropertyOutcome { PropertyOutcome::Known(PropertyValue::Text(value.into())) diff --git a/crates/windows/src/tree/property_ids_tests.rs b/crates/windows/src/tree/property_ids_tests.rs index 079f7ae..4cb873b 100644 --- a/crates/windows/src/tree/property_ids_tests.rs +++ b/crates/windows/src/tree/property_ids_tests.rs @@ -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::() + .is_ok_and(|value| (30_000..=30_999).contains(&value)) + }) +} diff --git a/crates/windows/src/tree/property_outcome.rs b/crates/windows/src/tree/property_outcome.rs new file mode 100644 index 0000000..04f17bf --- /dev/null +++ b/crates/windows/src/tree/property_outcome.rs @@ -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 { + 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 { + match self { + Self::Known(PropertyValue::Flag(value)) => Some(*value), + _ => None, + } + } + + pub fn number(&self) -> Option { + match self { + Self::Known(PropertyValue::Number(value)) => Some(*value), + _ => None, + } + } + + pub fn bounds(&self) -> LocatorField { + match self { + Self::Known(PropertyValue::Bounds(value)) => LocatorField::Known(*value), + Self::Known(_) => LocatorField::Unknown, + Self::Absent => LocatorField::Absent, + Self::Unknown => LocatorField::Unknown, + } + } +} diff --git a/crates/windows/src/tree/walker_enumerate.rs b/crates/windows/src/tree/walker_enumerate.rs index d60ad21..00b56bd 100644 --- a/crates/windows/src/tree/walker_enumerate.rs +++ b/crates/windows/src/tree/walker_enumerate.rs @@ -30,6 +30,7 @@ pub struct TreeWalk<'a, S: TreeSource> { stats: LocatorStats, complete: bool, failures: Vec, + 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; diff --git a/crates/windows/src/tree/walker_tests.rs b/crates/windows/src/tree/walker_tests.rs index d924409..f169a15 100644 --- a/crates/windows/src/tree/walker_tests.rs +++ b/crates/windows/src/tree/walker_tests.rs @@ -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 = (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" + ); +}