diff --git a/libs/cua-driver/rust/crates/cua-driver-core/src/element_token.rs b/libs/cua-driver/rust/crates/cua-driver-core/src/element_token.rs index 062caff68f..63e50a062c 100644 --- a/libs/cua-driver/rust/crates/cua-driver-core/src/element_token.rs +++ b/libs/cua-driver/rust/crates/cua-driver-core/src/element_token.rs @@ -1,199 +1,284 @@ -//! Opaque per-snapshot element tokens (Surface 6). +//! Opaque element capabilities issued by `get_window_state`. //! -//! ## Why this exists -//! -//! Today consumers (Hermes wrapper, Codex, Claude Code) treat the bare -//! 1-based `element_index` returned by `get_window_state` as valid until -//! the next snapshot — but there's no formal validity contract. If -//! cua-driver ever changes its internal indexing the silent failure mode -//! is a misclick: the integer still parses, the AX path still resolves -//! *something*, and the user lands on the wrong button. -//! -//! Surface 6 adds an opaque token alongside the integer index whose -//! validity is **explicit** and **invalidated cheaply** when the next -//! snapshot supersedes the previous one for the same (pid, window_id). -//! -//! ## Token format -//! -//! Chosen for "smallest to implement", per the Surface 6 plan: -//! -//! ```text -//! s{snapshot_id_hex}:{element_index} -//! ``` -//! -//! - `snapshot_id_hex` is a lowercase 4-hex-char prefix of a process- -//! global u32 snapshot counter (`AtomicU32`). 4 chars gives 16 bits of -//! namespace — collisions are statistically impossible inside the -//! 8-entry-per-pid LRU window we keep, and the prefix stays human-eyeball -//! friendly in logs. -//! - `element_index` is the same `usize` already returned in -//! `structuredContent.elements[].element_index`. Keeping it in plain -//! sight in the token means a server-side log line like -//! `element_token=s7a3f:42` is debug-grep-able without a side-table. -//! -//! Tokens are 8–12 chars (`"s0001:0"` up to `"sffff:999"`). Well within -//! the 8–16 char budget the Surface 6 plan called out. -//! -//! ## Validity contract -//! -//! - Snapshot IDs are minted in `register_snapshot` (called by every -//! platform's `get_window_state` implementation immediately after the -//! AX/UIA/AT-SPI walk lands in the per-platform element cache). -//! - A snapshot is valid until either (a) the LRU evicts it, or (b) a -//! newer snapshot for the same `pid` pushes it past the LRU cap of -//! [`LRU_CAP_PER_PID`]. -//! - Resolving a stale token returns the explicit error string -//! [`STALE_TOKEN_ERROR`] — consumers MUST treat that as "re-snapshot -//! and retry", never as "click failed". -//! -//! The LRU is **per-pid**, not global. Two snapshots from different pids -//! never collide even when their numeric counter happens to wrap (which -//! it won't in practice — u32 wraps after 4 billion calls). +//! Tokens contain no pid, window, generation, index, or node identity. Each +//! token is a UUID v4 capability minted by the current registry instance, and +//! the registry stores the complete binding server-side. A token from a prior +//! daemon process is therefore unknown after restart instead of aliasing a new +//! snapshot whose counters happen to repeat. use std::collections::HashMap; use std::sync::atomic::{AtomicU32, Ordering}; -use std::sync::Mutex; -use std::sync::OnceLock; - -/// LRU cap of valid snapshots retained per pid. Past this point the -/// oldest entry for the pid is evicted and its tokens go stale. -/// -/// Chosen at 8: enough for an agent that re-snapshots once per turn over -/// a multi-window session (open Slack, open Safari, swap to Cursor, …) -/// before recycling; small enough that memory pressure is irrelevant. -/// Matches the "e.g. 8 most recent" suggestion in the Surface 6 plan. -pub const LRU_CAP_PER_PID: usize = 8; +use std::sync::{Mutex, OnceLock}; +use uuid::Uuid; -/// Sentinel string returned by [`TokenRegistry::resolve`] when the token -/// parses but the snapshot it references has been invalidated. Consumers -/// (Hermes/Codex/Claude Code) MUST surface this as a re-snapshot-and-retry -/// signal, not a silent misclick. +pub const LRU_CAP_PER_PID: usize = 8; pub const STALE_TOKEN_ERROR: &str = "element_token is stale; call get_window_state again to refresh"; -/// One valid snapshot retained in the per-pid LRU. -#[derive(Debug, Clone, Copy)] +pub const TOKEN_INVALID_CODE: &str = "element_token_invalid"; +pub const TOKEN_UNKNOWN_CODE: &str = "element_token_unknown"; +pub const TOKEN_PID_MISMATCH_CODE: &str = "element_token_pid_mismatch"; +pub const TOKEN_WINDOW_MISMATCH_CODE: &str = "element_token_window_mismatch"; +pub const TOKEN_INDEX_MISMATCH_CODE: &str = "element_token_index_mismatch"; +pub const TOKEN_STALE_GENERATION_CODE: &str = "element_token_stale_generation"; +pub const TOKEN_IDENTITY_MISMATCH_CODE: &str = "element_token_identity_mismatch"; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct StableTokenError { + pub code: &'static str, + pub message: String, +} + +impl StableTokenError { + pub fn new(code: &'static str, message: impl Into) -> Self { + Self { + code, + message: message.into(), + } + } + + pub fn into_tool_result(self) -> crate::protocol::ToolResult { + crate::protocol::ToolResult::error(self.message.clone()).with_structured( + serde_json::json!({ + "code": self.code, + "message": self.message, + }), + ) + } + + pub fn stale_generation(generation: u32, pid: i32, window_id: u32) -> Self { + Self::new( + TOKEN_STALE_GENERATION_CODE, + format!( + "element_token generation {generation} is stale for pid={pid} \ + window_id={window_id}; call get_window_state again" + ), + ) + } + + pub fn identity_mismatch(element_index: usize) -> Self { + Self::new( + TOKEN_IDENTITY_MISMATCH_CODE, + format!( + "element_token AX node identity no longer matches element_index \ + {element_index}; call get_window_state again" + ), + ) + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct StableTokenBinding { + pub pid: i32, + pub window_id: u32, + pub generation: u32, + pub element_index: usize, + pub node_identity: u64, +} + +#[derive(Debug, Clone)] +struct TokenBinding { + pid: i32, + window_id: u32, + generation: u32, + element_index: usize, + node_identity: Option, +} + +#[derive(Debug)] struct SnapshotEntry { - /// Monotonic, process-global id assigned by [`mint_snapshot_id`]. - snapshot_id: u32, - /// The window the snapshot was taken against. Resolution returns - /// this so tools can verify the caller's `window_id` arg matches — - /// a token-only call doesn't have to pass window_id at all. + generation: u32, window_id: u32, - /// Maximum element_index that was assigned in this snapshot. The - /// resolver rejects out-of-range tokens up-front instead of waiting - /// for the per-platform cache to NPE. - max_element_index: usize, + tokens: Vec, +} + +#[derive(Default)] +struct RegistryState { + by_pid: HashMap>, + by_token: HashMap, } -/// Process-global token registry. Thread-safe; tools resolve from any -/// task via the shared [`global`] accessor. -/// -/// The data model is a `HashMap>` where each -/// pid's vec is the LRU (newest at the back). Vec instead of VecDeque -/// because the cap is tiny (8) and walks are linear either way. pub struct TokenRegistry { - by_pid: Mutex>>, + state: Mutex, + next_generation: AtomicU32, } impl TokenRegistry { fn new() -> Self { Self { - by_pid: Mutex::new(HashMap::new()), + state: Mutex::new(RegistryState::default()), + next_generation: AtomicU32::new(1), } } - /// Record a fresh snapshot for `pid` / `window_id`. Returns the - /// minted snapshot id so the caller can embed it in the per-element - /// token strings emitted alongside `element_index` in the structured - /// `elements` array. - /// - /// `element_count` is the number of actionable elements in the - /// snapshot (the count of nodes that received an `element_index`). - /// Used for up-front range checks on `resolve`. - /// - /// Side effect: if this pid already has [`LRU_CAP_PER_PID`] snapshots - /// in its lane, the oldest is evicted and any token that referenced - /// it becomes stale — that's the contract. pub fn register_snapshot(&self, pid: i32, window_id: u32, element_count: usize) -> u32 { - // Truncate to the 16-bit space the token format actually - // surfaces. The full u32 still increments monotonically — we - // just don't widen the on-the-wire token namespace beyond what - // the 4-hex-char prefix can carry. Round-trip property: - // `resolve(format_token(id, idx))` always finds the entry. - let id = mint_snapshot_id() & 0xffff; - let mut by_pid = self.by_pid.lock().unwrap(); - let lane = by_pid.entry(pid).or_default(); - lane.push(SnapshotEntry { - snapshot_id: id, - window_id, - max_element_index: element_count.saturating_sub(1), - }); - // Evict oldest. The loop guards against pre-existing over-cap - // state from a previous version of the binary; in steady state - // this fires exactly once per call. - while lane.len() > LRU_CAP_PER_PID { - lane.remove(0); + self.register_snapshot_inner(pid, window_id, element_count, HashMap::new()) + } + + pub fn register_snapshot_with_identities( + &self, + pid: i32, + window_id: u32, + element_count: usize, + nodes: impl IntoIterator, + ) -> u32 { + self.register_snapshot_inner(pid, window_id, element_count, nodes.into_iter().collect()) + } + + fn register_snapshot_inner( + &self, + pid: i32, + window_id: u32, + element_count: usize, + node_identities: HashMap, + ) -> u32 { + let generation = self.next_generation.fetch_add(1, Ordering::Relaxed); + let mut state = self.state.lock().unwrap(); + let mut tokens = Vec::with_capacity(element_count); + + for element_index in 0..element_count { + let token = mint_token(&state.by_token); + state.by_token.insert( + token.clone(), + TokenBinding { + pid, + window_id, + generation, + element_index, + node_identity: node_identities.get(&element_index).copied(), + }, + ); + tokens.push(token); } - id - } - - /// Resolve `token` against the LRU for `pid`. On success returns - /// `(window_id, element_index)` — the same pair the caller would - /// have passed as `(window_id, element_index)` integers. On failure - /// returns one of: - /// - /// - `"element_token has invalid format"` — couldn't parse the - /// `s{hex}:{idx}` shape. - /// - [`STALE_TOKEN_ERROR`] — parsed, but the snapshot id is no - /// longer in the pid's LRU (either evicted or never registered). - /// - `"element_token element_index out of range"` — the index in - /// the token is past the max recorded for the snapshot. - pub fn resolve(&self, pid: i32, token: &str) -> Result<(u32, usize), String> { - let (sid, idx) = - parse_token(token).ok_or_else(|| "element_token has invalid format".to_string())?; - let by_pid = self.by_pid.lock().unwrap(); - let lane = by_pid + + let evicted = { + let lane = state.by_pid.entry(pid).or_default(); + lane.push(SnapshotEntry { + generation, + window_id, + tokens, + }); + let mut evicted = Vec::new(); + while lane.len() > LRU_CAP_PER_PID { + evicted.push(lane.remove(0)); + } + evicted + }; + for snapshot in evicted { + for token in snapshot.tokens { + state.by_token.remove(&token); + } + } + generation + } + + pub fn token_for(&self, generation: u32, element_index: usize) -> Option { + let state = self.state.lock().unwrap(); + state + .by_pid + .values() + .flat_map(|lane| lane.iter()) + .find(|snapshot| snapshot.generation == generation) + .and_then(|snapshot| snapshot.tokens.get(element_index)) + .cloned() + } + + pub fn resolve_stable( + &self, + pid: i32, + args_window_id: Option, + args_element_index: Option, + token: &str, + ) -> Result { + validate_token_shape(token)?; + let state = self.state.lock().unwrap(); + let binding = state.by_token.get(token).ok_or_else(|| { + StableTokenError::new( + TOKEN_UNKNOWN_CODE, + "element_token is unknown to this cua-driver process", + ) + })?; + validate_binding(binding, pid, args_window_id, args_element_index)?; + + let current_generation = state + .by_pid .get(&pid) - .ok_or_else(|| STALE_TOKEN_ERROR.to_string())?; - let entry = lane - .iter() - .find(|e| e.snapshot_id == sid) - .ok_or_else(|| STALE_TOKEN_ERROR.to_string())?; - if idx > entry.max_element_index { - return Err(format!( - "element_token element_index {idx} out of range (snapshot had {} elements)", - entry.max_element_index + 1 + .and_then(|lane| { + lane.iter() + .rev() + .find(|snapshot| snapshot.window_id == binding.window_id) + }) + .map(|snapshot| snapshot.generation); + if current_generation != Some(binding.generation) { + return Err(StableTokenError::stale_generation( + binding.generation, + pid, + binding.window_id, )); } - Ok((entry.window_id, idx)) + let node_identity = binding.node_identity.ok_or_else(|| { + StableTokenError::new( + TOKEN_UNKNOWN_CODE, + format!( + "element_token has no AX node identity for element_index={} \ + in generation={}", + binding.element_index, binding.generation + ), + ) + })?; + Ok(StableTokenBinding { + pid: binding.pid, + window_id: binding.window_id, + generation: binding.generation, + element_index: binding.element_index, + node_identity, + }) + } + + pub fn resolve(&self, pid: i32, token: &str) -> Result<(u32, usize), String> { + validate_token_shape(token).map_err(|error| error.message)?; + let state = self.state.lock().unwrap(); + let binding = state + .by_token + .get(token) + .ok_or_else(|| STALE_TOKEN_ERROR.to_string())?; + if binding.pid != pid { + return Err(STALE_TOKEN_ERROR.to_string()); + } + let current_generation = state + .by_pid + .get(&pid) + .and_then(|lane| { + lane.iter() + .rev() + .find(|snapshot| snapshot.window_id == binding.window_id) + }) + .map(|snapshot| snapshot.generation); + if current_generation != Some(binding.generation) { + return Err(STALE_TOKEN_ERROR.to_string()); + } + Ok((binding.window_id, binding.element_index)) } - /// Build the canonical token string for `snapshot_id` / `element_index`. - /// Pure helper, mirrors [`format_token`] but lives on the registry so - /// callers don't have to import the free function. #[allow(dead_code)] - pub fn format(snapshot_id: u32, element_index: usize) -> String { - format_token(snapshot_id, element_index) + pub fn format(generation: u32, element_index: usize) -> String { + token_for(generation, element_index) } - /// Test-only: snapshot count for a pid. Used by the LRU-eviction - /// unit test to assert the cap was honoured. #[cfg(test)] fn snapshot_count(&self, pid: i32) -> usize { - self.by_pid + self.state .lock() .unwrap() + .by_pid .get(&pid) - .map(|v| v.len()) + .map(Vec::len) .unwrap_or(0) } - /// Test-only: clear all state. Lets parallel unit tests start clean - /// without relying on the global counter being at a specific value. #[cfg(test)] fn clear(&self) { - self.by_pid.lock().unwrap().clear(); + *self.state.lock().unwrap() = RegistryState::default(); } } @@ -203,461 +288,378 @@ impl Default for TokenRegistry { } } -/// Process-global counter for snapshot ids. Monotonically increasing — -/// even after eviction we never reuse an id during the process lifetime -/// (u32 wraps after 4 billion calls, well past any realistic agent run). -static SNAPSHOT_COUNTER: AtomicU32 = AtomicU32::new(1); - -/// Mint a fresh snapshot id. `1`-based so `"s0000:..."` is never a -/// legitimate token — makes "uninitialised default" bugs in client code -/// pop on the first call instead of accidentally aliasing a real -/// snapshot. -fn mint_snapshot_id() -> u32 { - // `Relaxed` is fine: the only invariant we need is uniqueness of the - // returned value, which `fetch_add` provides on its own. No happens- - // before edge with the Mutex below — the lock provides that. - SNAPSHOT_COUNTER.fetch_add(1, Ordering::Relaxed) +fn mint_token(existing: &HashMap) -> String { + loop { + let token = format!("e_{}", Uuid::new_v4().simple()); + if !existing.contains_key(&token) { + return token; + } + } } -/// Format `(snapshot_id, element_index)` as the canonical token string. -/// 4-hex-char snapshot prefix means tokens stay under 12 chars even -/// with 4-digit indices. -/// -/// Snapshot ids are masked to 16 bits by [`TokenRegistry::register_snapshot`] -/// before storage so the round trip `resolve(format_token(id, idx))` -/// closes cleanly without truncation drift. Collision chance inside the -/// 8-entry LRU window is 8/65536 ≈ 0.01%; the registry treats the -/// `(pid, snapshot_id)` pair as the lookup key so a same-bits collision -/// across pids never aliases. -pub fn format_token(snapshot_id: u32, element_index: usize) -> String { - let short = snapshot_id & 0xffff; - format!("s{short:04x}:{element_index}") +fn validate_token_shape(token: &str) -> Result<(), StableTokenError> { + let raw = token.strip_prefix("e_").ok_or_else(|| { + StableTokenError::new(TOKEN_INVALID_CODE, "element_token has invalid format") + })?; + let uuid = Uuid::parse_str(raw).map_err(|_| { + StableTokenError::new(TOKEN_INVALID_CODE, "element_token has invalid format") + })?; + if uuid.get_version_num() != 4 || uuid.get_variant() != uuid::Variant::RFC4122 { + return Err(StableTokenError::new( + TOKEN_INVALID_CODE, + "element_token has invalid format", + )); + } + Ok(()) } -/// Parse a canonical token string into `(snapshot_id, element_index)`. -/// Returns `None` on any shape error (unknown prefix, missing colon, -/// non-hex, non-decimal). The token strings are produced by -/// [`format_token`] only — consumers MUST treat the format as opaque -/// and never construct one by hand. -fn parse_token(token: &str) -> Option<(u32, usize)> { - let body = token.strip_prefix('s')?; - let (hex, idx) = body.split_once(':')?; - if hex.len() != 4 { - return None; - } - let sid = u32::from_str_radix(hex, 16).ok()?; - let idx = idx.parse::().ok()?; - Some((sid, idx)) +fn validate_binding( + binding: &TokenBinding, + pid: i32, + args_window_id: Option, + args_element_index: Option, +) -> Result<(), StableTokenError> { + if binding.pid != pid { + return Err(StableTokenError::new( + TOKEN_PID_MISMATCH_CODE, + format!( + "element_token belongs to pid={}, not requested pid={pid}", + binding.pid + ), + )); + } + if let Some(window_id) = args_window_id { + if binding.window_id != window_id { + return Err(StableTokenError::new( + TOKEN_WINDOW_MISMATCH_CODE, + format!( + "element_token belongs to window_id={}, not requested window_id={window_id}", + binding.window_id + ), + )); + } + } + if let Some(element_index) = args_element_index { + if binding.element_index != element_index { + return Err(StableTokenError::new( + TOKEN_INDEX_MISMATCH_CODE, + format!( + "element_token identifies element_index={}, not requested \ + element_index={element_index}", + binding.element_index + ), + )); + } + } + Ok(()) } -/// Process-global handle to the token registry. Used by every platform's -/// `get_window_state` (to register a fresh snapshot) and every element- -/// targeting tool (to resolve a passed-in token). pub fn global() -> &'static TokenRegistry { - static REG: OnceLock = OnceLock::new(); - REG.get_or_init(TokenRegistry::new) + static REGISTRY: OnceLock = OnceLock::new(); + REGISTRY.get_or_init(TokenRegistry::new) +} + +pub fn token_for(generation: u32, element_index: usize) -> String { + global() + .token_for(generation, element_index) + .expect("token requested for an unregistered snapshot element") } -/// Build an `(snapshot_id, element_index)` token by minting both halves -/// from the current globals. Convenience for the per-platform -/// `build_elements_array` paths that already iterate over actionable -/// nodes and want a token per row. -/// -/// `snapshot_id` is the value returned by [`TokenRegistry::register_snapshot`] -/// for the current `get_window_state` call. Pass the same id for every -/// element in one snapshot — the token registry already tracks them as -/// a group keyed by that id. -pub fn token_for(snapshot_id: u32, element_index: usize) -> String { - format_token(snapshot_id, element_index) +pub fn snapshot_id(generation: u32) -> String { + format!("s{generation:08x}") +} + +pub fn add_snapshot_metadata(structured: &mut serde_json::Value, generation: u32) { + structured["snapshot_id"] = serde_json::json!(snapshot_id(generation)); + structured["generation"] = serde_json::json!(generation); +} + +pub fn format_token(generation: u32, element_index: usize) -> String { + token_for(generation, element_index) } -/// Result of dispatching the `element_token` ↔ `element_index` precedence -/// rule on a tool call's args. Returned by [`resolve_element_args`]. #[derive(Debug, Clone)] pub enum ResolvedElement { - /// Neither `element_token` nor `element_index` was supplied — the - /// tool should fall through to its non-element addressing mode - /// (typically pixel `x, y`) or error. None, - /// Resolved to `(window_id, element_index)`. The `window_id` may be - /// `None` when the caller supplied only `element_index` without a - /// `window_id` (legacy back-compat for tools that already handled - /// that case); when the caller supplied a token, `window_id` is - /// always the one the snapshot was taken against. Element { window_id: Option, element_index: usize, - /// True when the caller supplied a token and we resolved - /// through the registry — informational, used by tools that - /// want to log "via token" in the success summary. via_token: bool, }, } -/// Apply the Surface 6 precedence rule for tool args that accept both -/// `element_index` and `element_token`. Returns either a stale/format -/// error (already wrapped as a `ToolResult::error`) or the resolved -/// `(window_id, element_index)` pair. -/// -/// Rule: -/// - **Neither**: returns [`ResolvedElement::None`]. The tool decides -/// whether to error or fall through to a pixel path. -/// - **Only `element_index`**: legacy behaviour, unchanged. Returns -/// `Element { window_id: , element_index, via_token: false }`. -/// - **Only `element_token`**: resolves through the registry. On stale -/// or malformed token, returns an error. On success returns -/// `Element { window_id: Some(), element_index, via_token: true }`. -/// - **Both supplied**: `element_token` takes precedence; the resolver -/// verifies it matches `element_index` and logs a warning on -/// disagreement (the integer is treated as advisory once a token is -/// present). On stale or malformed token, returns an error — the -/// integer is NOT used as a fallback (Surface 6 plan: "token wins"). -/// -/// `args_window_id` is the `window_id` arg the caller already pulled -/// off the JSON via the existing `args.opt_u64("window_id")`. Passing -/// it in here lets the helper keep that lookup in one place per tool -/// rather than duplicating it. pub fn resolve_element_args( pid: i32, args_element_index: Option, args_element_token: Option<&str>, args_window_id: Option, - tool_name: &str, + _tool_name: &str, ) -> Result { match (args_element_index, args_element_token) { (None, None) => Ok(ResolvedElement::None), - (Some(idx), None) => Ok(ResolvedElement::Element { + (Some(element_index), None) => Ok(ResolvedElement::Element { window_id: args_window_id, - element_index: idx, + element_index, via_token: false, }), - (idx_opt, Some(tok)) => { - // Token wins. Resolve through the registry; bail on stale - // or malformed without falling back to the integer. - let (wid, idx) = global() - .resolve(pid, tok) + (element_index, Some(token)) => { + let (window_id, resolved_index) = global() + .resolve(pid, token) .map_err(crate::protocol::ToolResult::error)?; - if let Some(int_idx) = idx_opt { - if int_idx != idx { - // Disagreement is non-fatal — token wins, but we - // log so the consumer can debug. Use eprintln so - // the daemon's stderr captures it (the recording - // path doesn't see this). - eprintln!( - "[cua-driver-rs] {tool_name}: element_token / element_index \ - disagree (token={tok} → idx={idx}, arg element_index={int_idx}); \ - token wins." - ); + if let Some(element_index) = element_index { + if element_index != resolved_index { + return Err(StableTokenError::new( + TOKEN_INDEX_MISMATCH_CODE, + format!( + "element_token identifies element_index={resolved_index}, not \ + requested element_index={element_index}" + ), + ) + .into_tool_result()); + } + } + if let Some(args_window_id) = args_window_id { + if args_window_id != window_id { + return Err(StableTokenError::new( + TOKEN_WINDOW_MISMATCH_CODE, + format!( + "element_token belongs to window_id={window_id}, not requested \ + window_id={args_window_id}" + ), + ) + .into_tool_result()); } } Ok(ResolvedElement::Element { - window_id: Some(wid), - element_index: idx, + window_id: Some(window_id), + element_index: resolved_index, via_token: true, }) } } } -// ── Tests ───────────────────────────────────────────────────────────────── - #[cfg(test)] mod tests { use super::*; - fn fresh_registry() -> TokenRegistry { - TokenRegistry::new() + fn token(registry: &TokenRegistry, generation: u32, index: usize) -> String { + registry.token_for(generation, index).unwrap() } #[test] - fn token_round_trips_through_format_then_parse() { - // Use a low-bit id that survives the 16-bit truncation in - // format_token, so we can compare format → parse without losing - // information. - let token = format_token(0x1234, 42); - assert_eq!(token, "s1234:42"); - let (sid, idx) = parse_token(&token).expect("parse_token should accept its own output"); - assert_eq!(sid, 0x1234); - assert_eq!(idx, 42); + fn tokens_are_uuid_v4_opaque_capabilities() { + let registry = TokenRegistry::new(); + let generation = registry.register_snapshot(10, 20, 2); + let first = token(®istry, generation, 0); + let second = token(®istry, generation, 1); + assert_ne!(first, second); + validate_token_shape(&first).unwrap(); + assert!(!first.contains("10")); + assert!(!first.contains(":0")); + assert!(!first.contains(&format!("{generation:08x}"))); } #[test] - fn token_format_pads_to_four_hex_chars() { - // Small ids must still have a 4-char prefix so the parser's - // length check passes. Surface 6's stated format is "8-16 chars"; - // we sit comfortably inside that. - let token = format_token(1, 0); - assert_eq!(token, "s0001:0"); - let token2 = format_token(0, 999); - assert_eq!(token2, "s0000:999"); - } - - #[test] - fn parse_rejects_unknown_prefix_or_shape() { - assert!(parse_token("").is_none()); - assert!(parse_token("x1234:42").is_none(), "wrong prefix"); - assert!(parse_token("s1234").is_none(), "missing colon"); - assert!(parse_token("s12345:42").is_none(), "hex too long"); - assert!(parse_token("s123:42").is_none(), "hex too short"); - assert!(parse_token("szzzz:42").is_none(), "non-hex"); - assert!(parse_token("s1234:abc").is_none(), "non-decimal index"); + fn empty_snapshot_preserves_cross_platform_metadata_schema_without_element_token() { + let registry = TokenRegistry::new(); + let generation = registry.register_snapshot(10, 20, 0); + assert_eq!(registry.token_for(generation, 0), None); + + let mut structured = serde_json::json!({}); + add_snapshot_metadata(&mut structured, generation); + assert_eq!( + structured["snapshot_id"], + serde_json::json!(format!("s{generation:08x}")) + ); + assert!(structured["snapshot_id"].is_string()); + assert_eq!(structured["generation"], serde_json::json!(generation)); + assert!(structured["generation"].is_number()); } #[test] - fn register_then_resolve_returns_window_and_index() { - let reg = fresh_registry(); - let pid = 100; - let snapshot_id = reg.register_snapshot(pid, 42, /* element_count */ 5); - let token = format_token(snapshot_id, 3); - let (wid, idx) = reg.resolve(pid, &token).expect("fresh token must resolve"); - assert_eq!(wid, 42); - assert_eq!(idx, 3); + fn registry_instances_issue_unique_tokens_and_reject_cross_restart_replay() { + let before_restart = TokenRegistry::new(); + let old_generation = before_restart.register_snapshot(10, 20, 1); + let old_token = token(&before_restart, old_generation, 0); + + let after_restart = TokenRegistry::new(); + let new_generation = after_restart.register_snapshot(10, 20, 1); + let new_token = token(&after_restart, new_generation, 0); + + assert_ne!(old_token, new_token); + assert_eq!( + after_restart + .resolve_stable(10, Some(20), Some(0), &old_token) + .unwrap_err() + .code, + TOKEN_UNKNOWN_CODE + ); } #[test] - fn resolve_with_unknown_pid_returns_stale_error() { - // `STALE_TOKEN_ERROR` is the contract string consumers grep for. - let reg = fresh_registry(); - let token = format_token(0x1234, 0); - let err = reg.resolve(/* pid = */ 999, &token).unwrap_err(); - assert_eq!(err, STALE_TOKEN_ERROR); + fn token_tamper_fails_closed() { + let registry = TokenRegistry::new(); + let generation = registry.register_snapshot_with_identities(10, 20, 1, [(0, 0xfeed)]); + let original = token(®istry, generation, 0); + let mut tampered = original.into_bytes(); + let last = tampered.last_mut().unwrap(); + *last = if *last == b'a' { b'b' } else { b'a' }; + let tampered = String::from_utf8(tampered).unwrap(); + assert_eq!( + registry + .resolve_stable(10, Some(20), Some(0), &tampered) + .unwrap_err() + .code, + TOKEN_UNKNOWN_CODE + ); } #[test] - fn resolve_with_bad_format_returns_invalid_error() { - let reg = fresh_registry(); - // Pre-register a snapshot so we know the failure isn't from an - // empty registry — the format check must run before the lane - // lookup so callers get the more useful error. - reg.register_snapshot(10, 1, 1); - let err = reg.resolve(10, "garbage").unwrap_err(); - assert!(err.contains("invalid format"), "got: {err}"); + fn stable_token_binds_pid_window_generation_index_and_identity() { + let registry = TokenRegistry::new(); + let generation = registry.register_snapshot_with_identities(10, 20, 5, [(4, 0xfeed)]); + let binding = registry + .resolve_stable(10, Some(20), Some(4), &token(®istry, generation, 4)) + .unwrap(); + assert_eq!( + binding, + StableTokenBinding { + pid: 10, + window_id: 20, + generation, + element_index: 4, + node_identity: 0xfeed, + } + ); } #[test] - fn out_of_range_index_returns_actionable_error() { - let reg = fresh_registry(); - let pid = 11; - let snapshot_id = reg.register_snapshot(pid, 1, /* element_count */ 3); - // Snapshot has indices 0..2 — 7 is past the end. - let token = format_token(snapshot_id, 7); - let err = reg.resolve(pid, &token).unwrap_err(); - assert!(err.contains("out of range"), "got: {err}"); + fn pid_window_and_index_forgery_fail_closed() { + let registry = TokenRegistry::new(); + let generation = registry.register_snapshot_with_identities(10, 20, 3, [(2, 9)]); + let token = token(®istry, generation, 2); + assert_eq!( + registry + .resolve_stable(11, Some(20), Some(2), &token) + .unwrap_err() + .code, + TOKEN_PID_MISMATCH_CODE + ); + assert_eq!( + registry + .resolve_stable(10, Some(21), Some(2), &token) + .unwrap_err() + .code, + TOKEN_WINDOW_MISMATCH_CODE + ); + assert_eq!( + registry + .resolve_stable(10, Some(20), Some(1), &token) + .unwrap_err() + .code, + TOKEN_INDEX_MISMATCH_CODE + ); } #[test] - fn next_snapshot_for_same_pid_keeps_old_until_lru_evicts() { - // The contract is "previous snapshot is invalidated when a NEW - // snapshot runs for the pid" — but we hold an LRU of size - // LRU_CAP_PER_PID, so callers get a small grace window of recent - // snapshots, not strictly the most recent one. This is what the - // Surface 6 plan describes ("cap at e.g. 8 most recent"). - let reg = fresh_registry(); - let pid = 12; - let s1 = reg.register_snapshot(pid, 1, 5); - let s2 = reg.register_snapshot(pid, 1, 5); - // Both should still resolve. - let _ = reg - .resolve(pid, &format_token(s1, 0)) - .expect("s1 still in LRU"); - let _ = reg.resolve(pid, &format_token(s2, 0)).expect("s2 fresh"); + fn newer_snapshot_makes_stable_token_stale() { + let registry = TokenRegistry::new(); + let generation = registry.register_snapshot_with_identities(10, 20, 1, [(0, 7)]); + let stale = token(®istry, generation, 0); + registry.register_snapshot_with_identities(10, 20, 1, [(0, 8)]); + assert_eq!( + registry + .resolve_stable(10, Some(20), Some(0), &stale) + .unwrap_err() + .code, + TOKEN_STALE_GENERATION_CODE + ); + assert_eq!( + registry.resolve(10, &stale).unwrap_err(), + STALE_TOKEN_ERROR, + "generic token consumers must not resolve an index from a superseded snapshot" + ); } #[test] - fn lru_eviction_invalidates_oldest_snapshot() { - let reg = fresh_registry(); - let pid = 13; - // Fill the LRU. - let oldest = reg.register_snapshot(pid, 1, 5); + fn lru_eviction_removes_capabilities() { + let registry = TokenRegistry::new(); + let generation = registry.register_snapshot(13, 1, 1); + let oldest = token(®istry, generation, 0); for _ in 0..LRU_CAP_PER_PID { - // Push LRU_CAP_PER_PID more, which evicts `oldest`. - let _ = reg.register_snapshot(pid, 1, 5); + registry.register_snapshot(13, 1, 1); } - // Lane size must respect the cap. - assert_eq!(reg.snapshot_count(pid), LRU_CAP_PER_PID); - // Oldest must be stale now. - let err = reg.resolve(pid, &format_token(oldest, 0)).unwrap_err(); - assert_eq!(err, STALE_TOKEN_ERROR); + assert_eq!(registry.snapshot_count(13), LRU_CAP_PER_PID); + assert_eq!( + registry.resolve(13, &oldest).unwrap_err(), + STALE_TOKEN_ERROR + ); } #[test] - fn tokens_in_different_pids_dont_collide() { - // Same snapshot counter values across pids must resolve back to - // each pid's own window_id, never the other's. This is the - // per-pid lane property the registry promises. - let reg = fresh_registry(); - let s_a = reg.register_snapshot(/* pid = */ 100, /* window_id = */ 11, 3); - let s_b = reg.register_snapshot(/* pid = */ 200, /* window_id = */ 22, 3); - let token_a = format_token(s_a, 0); - let token_b = format_token(s_b, 0); - // Cross-pid attempts must NOT resolve to the other pid's window. - assert_eq!(reg.resolve(100, &token_a).unwrap().0, 11); - assert_eq!(reg.resolve(200, &token_b).unwrap().0, 22); - // Attempting to use pid A's token under pid B must fail stale. - let err = reg.resolve(200, &token_a).unwrap_err(); - assert_eq!(err, STALE_TOKEN_ERROR); + fn malformed_token_is_distinct_from_unknown_token() { + let registry = TokenRegistry::new(); + assert_eq!( + registry + .resolve_stable(1, None, None, "s00000001:0") + .unwrap_err() + .code, + TOKEN_INVALID_CODE + ); } #[test] - fn global_registry_is_shared_across_calls() { - // Smoke test that `global()` returns the same instance every - // call. We don't depend on cross-test isolation here — the - // assertion is structural, not value-based. - let reg_a = global(); - let reg_b = global(); - assert!(std::ptr::eq(reg_a, reg_b)); + fn clear_removes_all_capabilities() { + let registry = TokenRegistry::new(); + let generation = registry.register_snapshot(1, 1, 1); + let token = token(®istry, generation, 0); + registry.clear(); + assert_eq!(registry.snapshot_count(1), 0); + assert_eq!(registry.resolve(1, &token).unwrap_err(), STALE_TOKEN_ERROR); } #[test] - fn stale_token_returns_explicit_error_not_silent_misclick() { - // Surface 6 hard constraint: we must NEVER silently re-map a - // stale token to "some index" — the consumer has to see the - // error string and re-snapshot. - let reg = fresh_registry(); - let pid = 14; - let s1 = reg.register_snapshot(pid, 1, 5); - // Evict by pushing LRU_CAP_PER_PID newer snapshots. - for _ in 0..LRU_CAP_PER_PID { - let _ = reg.register_snapshot(pid, 1, 5); - } - let err = reg.resolve(pid, &format_token(s1, 2)).unwrap_err(); - assert_eq!(err, STALE_TOKEN_ERROR); + fn global_registry_is_shared_across_calls() { + assert!(std::ptr::eq(global(), global())); } - #[test] - fn clear_then_register_starts_clean() { - let reg = fresh_registry(); - let _ = reg.register_snapshot(1, 1, 1); - reg.clear(); - assert_eq!(reg.snapshot_count(1), 0); - } - - // ── resolve_element_args precedence rule ───────────────────────── - // - // These cover the Surface 6 dispatch contract: - // - // - element_index_alone_still_works - // - element_token_alone_resolves_to_same_action - // - both_provided_token_wins_disagree_warns - // - // The "stale" and "different pids" surfaces are already covered by - // the registry-level tests above; resolve_element_args is just the - // thin precedence layer on top. - #[test] fn element_index_alone_still_works() { - // Surface 6 backward-compat regression guard: tools that only - // see element_index keep returning the same shape. - let resolved = resolve_element_args( - /* pid = */ 1, - /* element_index = */ Some(7), - /* element_token = */ None, - /* window_id = */ Some(99), - "click", - ) - .expect("element_index-only must succeed"); - match resolved { + let resolved = resolve_element_args(1, Some(7), None, Some(99), "click").unwrap(); + assert!(matches!( + resolved, ResolvedElement::Element { - window_id, - element_index, - via_token, - } => { - assert_eq!(window_id, Some(99)); - assert_eq!(element_index, 7); - assert!( - !via_token, - "element_index-only path must NOT report via_token" - ); + window_id: Some(99), + element_index: 7, + via_token: false, } - _ => panic!("expected Element, got {resolved:?}"), - } + )); } #[test] - fn element_token_alone_resolves_to_same_action() { - // Register a snapshot in the GLOBAL registry (resolve_element_args - // uses `global()`), then resolve the token through the same path - // the tool would use. - let reg = global(); - // Use a pid unlikely to collide with other tests. + fn global_token_resolves_and_index_forgery_errors() { let pid = 0x7fff_0001_i32; - let snapshot_id = reg.register_snapshot(pid, /* window_id = */ 555, 4); - let token = format_token(snapshot_id, 2); - let resolved = resolve_element_args( - pid, - None, - Some(&token), - // window_id arg intentionally omitted — the token carries it. - None, - "click", - ) - .expect("token-only must succeed"); - match resolved { + let generation = global().register_snapshot(pid, 555, 4); + let token = token_for(generation, 2); + let resolved = resolve_element_args(pid, None, Some(&token), None, "click").unwrap(); + assert!(matches!( + resolved, ResolvedElement::Element { - window_id, - element_index, - via_token, - } => { - assert_eq!(window_id, Some(555), "window_id comes from the snapshot"); - assert_eq!(element_index, 2); - assert!(via_token, "token path must report via_token=true"); - } - _ => panic!("expected Element, got {resolved:?}"), - } - } - - #[test] - fn both_provided_token_wins_disagree_warns() { - // Both args supplied with disagreeing indices — token wins, no - // error returned. We can't assert the stderr line content from - // a unit test, but we CAN assert the returned indices come from - // the token, not the integer. - let reg = global(); - let pid = 0x7fff_0002_i32; - let snapshot_id = reg.register_snapshot(pid, 777, 5); - let token = format_token(snapshot_id, 3); - let resolved = resolve_element_args( - pid, - Some(99), // disagrees with token (which says idx 3) - Some(&token), - None, - "click", - ) - .expect("disagreement still resolves; token wins"); - match resolved { - ResolvedElement::Element { - window_id, - element_index, - via_token, - } => { - assert_eq!(window_id, Some(777)); - assert_eq!(element_index, 3, "token's idx wins over the integer arg"); - assert!(via_token); + window_id: Some(555), + element_index: 2, + via_token: true, } - _ => panic!("expected Element, got {resolved:?}"), - } - } - - #[test] - fn token_only_stale_returns_error_not_silent_fallback_to_integer() { - // Surface 6 hard constraint: a stale token MUST NOT fall back - // to the integer — that would silently misclick. - let pid = 0x7fff_0003_i32; - // Token references a snapshot that was never registered → stale. - let token = format_token(0xdead, 0); - let err = resolve_element_args(pid, Some(0), Some(&token), Some(1), "click").unwrap_err(); - // ToolResult::error wraps the message in a Content::Text — the - // assertion uses the protocol-level error_text accessor. - assert!( - err.is_error.unwrap_or(false), - "stale token must return an error ToolResult" - ); + )); + assert!(resolve_element_args(pid, Some(3), Some(&token), Some(555), "click").is_err()); } #[test] - fn neither_returns_none() { - let resolved = resolve_element_args(1, None, None, None, "click") - .expect("neither arg returns None, not error"); - assert!(matches!(resolved, ResolvedElement::None)); + fn stable_token_errors_expose_structured_code_and_message() { + let result = StableTokenError::new(TOKEN_UNKNOWN_CODE, "unknown token").into_tool_result(); + assert_eq!(result.is_error, Some(true)); + let structured = result.structured_content.unwrap(); + assert_eq!(structured["code"], TOKEN_UNKNOWN_CODE); + assert_eq!(structured["message"], "unknown token"); } } diff --git a/libs/cua-driver/rust/crates/cua-driver-core/src/tool_schema.rs b/libs/cua-driver/rust/crates/cua-driver-core/src/tool_schema.rs index 7501e83350..2aa8a36422 100644 --- a/libs/cua-driver/rust/crates/cua-driver-core/src/tool_schema.rs +++ b/libs/cua-driver/rust/crates/cua-driver-core/src/tool_schema.rs @@ -98,9 +98,11 @@ pub fn element_token_schema() -> Value { json!({ "type": "string", "description": "Opaque per-snapshot element handle from \ - `structuredContent.elements[].element_token`. Takes precedence over \ - element_index when both are supplied. Returns an explicit \"stale\" \ - error once a newer snapshot supersedes it — re-snapshot in that case." + `structuredContent.elements[].element_token`. On macOS click/set_value \ + it is strictly bound to pid, window_id, generation, element_index, and \ + AX node identity. If element_index or window_id are also supplied they \ + must match. Unknown, cross-target, stale-generation, or identity-mismatch \ + tokens fail closed; call get_window_state again after a stale error." }) } diff --git a/libs/cua-driver/rust/crates/platform-linux/src/tools/impl_.rs b/libs/cua-driver/rust/crates/platform-linux/src/tools/impl_.rs index fb790a9377..29748b2c65 100644 --- a/libs/cua-driver/rust/crates/platform-linux/src/tools/impl_.rs +++ b/libs/cua-driver/rust/crates/platform-linux/src/tools/impl_.rs @@ -743,10 +743,10 @@ impl Tool for GetWindowStateTool { .collect(); structured["elements"] = json!(elements); // Surface 6: snapshot id mirror for debug correlation. - structured["snapshot_id"] = - json!(cua_driver_core::element_token::token_for(snapshot_id, 0) - .trim_end_matches(":0") - .to_string()); + cua_driver_core::element_token::add_snapshot_metadata( + &mut structured, + snapshot_id, + ); structured["_note"] = json!( "Prefer `elements` — `tree_markdown` will continue to work \ but new fields will only be added to the structured side. \ diff --git a/libs/cua-driver/rust/crates/platform-macos/src/ax/cache.rs b/libs/cua-driver/rust/crates/platform-macos/src/ax/cache.rs index e6a7a36eff..3716521800 100644 --- a/libs/cua-driver/rust/crates/platform-macos/src/ax/cache.rs +++ b/libs/cua-driver/rust/crates/platform-macos/src/ax/cache.rs @@ -61,21 +61,35 @@ pub struct CacheKey { /// Cached snapshot for one (pid, window_id) pair. pub struct CachedSnapshot { - /// element_index → raw AXUIElementRef pointer (retained, as usize for Send). - pub elements: Vec, + pub generation: u32, + /// element_index → raw AXUIElementRef pointer plus exact node identity. + pub elements: Vec, +} + +pub struct CachedElement { + pub ptr: usize, + pub node_identity: u64, } impl Drop for CachedSnapshot { fn drop(&mut self) { // Release the extra CFRetain that walk_element added for each cached ptr. - for ptr in &self.elements { - if *ptr != 0 { - unsafe { CFRelease(*ptr as AXUIElementRef as CFTypeRef) }; + for element in &self.elements { + if element.ptr != 0 { + unsafe { CFRelease(element.ptr as AXUIElementRef as CFTypeRef) }; } } } } +pub struct ValidatedElement { + pub element: RetainedElement, + pub window_id: u32, + pub element_index: usize, + pub generation: u32, + pub node_identity: u64, +} + /// Global element cache. pub struct ElementCache { core: ElementCacheCore, @@ -90,13 +104,31 @@ impl ElementCache { /// Replace the snapshot for (pid, window_id) with the nodes from a fresh walk. pub fn update(&self, pid: i32, window_id: u32, nodes: &[AXNode]) { - let elements: Vec = nodes + self.update_with_generation(pid, window_id, 0, nodes); + } + + pub fn update_with_generation( + &self, + pid: i32, + window_id: u32, + generation: u32, + nodes: &[AXNode], + ) { + let elements: Vec = nodes .iter() .filter(|n| n.element_index.is_some()) - .map(|n| n.element_ptr) + .map(|n| CachedElement { + ptr: n.element_ptr, + node_identity: n.node_identity, + }) .collect(); - self.core - .insert(CacheKey { pid, window_id }, CachedSnapshot { elements }); + self.core.insert( + CacheKey { pid, window_id }, + CachedSnapshot { + generation, + elements, + }, + ); } /// Look up + `CFRetain` the element for `element_index` in (pid, window_id), @@ -114,7 +146,7 @@ impl ElementCache { ) -> Option { self.core .with_snapshot(&CacheKey { pid, window_id }, |s| { - let ptr = s.elements.get(element_index).copied()?; + let ptr = s.elements.get(element_index)?.ptr; if ptr != 0 { // Safety: still inside `with_snapshot`'s lock, so the // snapshot (and thus this CFTypeRef) is alive right now. @@ -125,6 +157,73 @@ impl ElementCache { .flatten() } + pub fn resolve_token( + &self, + pid: i32, + args_window_id: Option, + args_element_index: Option, + token: &str, + ) -> Result { + let binding = cua_driver_core::element_token::global().resolve_stable( + pid, + args_window_id, + args_element_index, + token, + )?; + self.core + .with_snapshot( + &CacheKey { + pid, + window_id: binding.window_id, + }, + |snapshot| { + if snapshot.generation != binding.generation { + return Err( + cua_driver_core::element_token::StableTokenError::stale_generation( + binding.generation, + pid, + binding.window_id, + ), + ); + } + let cached = snapshot + .elements + .get(binding.element_index) + .ok_or_else(|| { + cua_driver_core::element_token::StableTokenError::identity_mismatch( + binding.element_index, + ) + })?; + if cached.node_identity != binding.node_identity { + return Err( + cua_driver_core::element_token::StableTokenError::identity_mismatch( + binding.element_index, + ), + ); + } + if cached.ptr != 0 { + unsafe { CFRetain(cached.ptr as AXUIElementRef as CFTypeRef) }; + } + Ok(ValidatedElement { + element: RetainedElement(cached.ptr), + window_id: binding.window_id, + element_index: binding.element_index, + generation: binding.generation, + node_identity: binding.node_identity, + }) + }, + ) + .unwrap_or_else(|| { + Err( + cua_driver_core::element_token::StableTokenError::stale_generation( + binding.generation, + pid, + binding.window_id, + ), + ) + }) + } + /// Number of indexed elements for (pid, window_id), or 0 if not cached. pub fn element_count(&self, pid: i32, window_id: u32) -> usize { self.core @@ -159,6 +258,7 @@ mod tests { help: None, actions: Vec::new(), element_ptr: ptr, + node_identity: ptr as u64, depth: 0, parent_element_index: None, frame: None, @@ -230,4 +330,122 @@ mod tests { cache.update(1, 2, &[]); assert!(cache.get_element_retained(1, 2, 5).is_none()); } + + #[test] + fn stable_token_resolves_only_exact_cached_ax_identity() { + let s = CFString::new("cua-driver-stable-token-exact-identity"); + let ptr = s.as_concrete_TypeRef() as usize; + unsafe { CFRetain(ptr as CFTypeRef) }; + let pid = 0x6afe_0001; + let window_id = 77; + let generation = cua_driver_core::element_token::global() + .register_snapshot_with_identities(pid, window_id, 1, [(0, ptr as u64)]); + let cache = ElementCache::new(); + cache.update_with_generation(pid, window_id, generation, &[node_with_ptr(ptr)]); + + let token = cua_driver_core::element_token::token_for(generation, 0); + let resolved = cache + .resolve_token(pid, Some(window_id), Some(0), &token) + .expect("exact identity resolves"); + assert_eq!(resolved.element.as_ptr(), ptr); + assert_eq!(resolved.node_identity, ptr as u64); + } + + #[test] + fn stable_token_fails_closed_on_identity_mismatch() { + let s = CFString::new("cua-driver-stable-token-identity-mismatch"); + let ptr = s.as_concrete_TypeRef() as usize; + unsafe { CFRetain(ptr as CFTypeRef) }; + let pid = 0x6afe_0002; + let window_id = 78; + let generation = cua_driver_core::element_token::global() + .register_snapshot_with_identities( + pid, + window_id, + 1, + [(0, (ptr as u64).wrapping_add(1))], + ); + let cache = ElementCache::new(); + cache.update_with_generation(pid, window_id, generation, &[node_with_ptr(ptr)]); + + let token = cua_driver_core::element_token::token_for(generation, 0); + let error = cache + .resolve_token(pid, Some(window_id), Some(0), &token) + .err() + .expect("identity mismatch must fail"); + assert_eq!( + error.code, + cua_driver_core::element_token::TOKEN_IDENTITY_MISMATCH_CODE + ); + } + + #[test] + fn stable_token_fails_closed_on_cache_generation_mismatch() { + let s = CFString::new("cua-driver-stable-token-generation-mismatch"); + let ptr = s.as_concrete_TypeRef() as usize; + unsafe { CFRetain(ptr as CFTypeRef) }; + let pid = 0x6afe_0003; + let window_id = 79; + let generation = cua_driver_core::element_token::global() + .register_snapshot_with_identities(pid, window_id, 1, [(0, ptr as u64)]); + let cache = ElementCache::new(); + cache.update_with_generation( + pid, + window_id, + generation.wrapping_add(1), + &[node_with_ptr(ptr)], + ); + + let token = cua_driver_core::element_token::token_for(generation, 0); + let error = cache + .resolve_token(pid, Some(window_id), Some(0), &token) + .err() + .expect("generation mismatch must fail"); + assert_eq!( + error.code, + cua_driver_core::element_token::TOKEN_STALE_GENERATION_CODE + ); + } + + #[test] + fn superseded_snapshot_same_index_replacement_fails_closed() { + let first = CFString::new("cua-driver-stable-token-first-node"); + let second = CFString::new("cua-driver-stable-token-replacement-node"); + let first_ptr = first.as_concrete_TypeRef() as usize; + let second_ptr = second.as_concrete_TypeRef() as usize; + unsafe { + CFRetain(first_ptr as CFTypeRef); + CFRetain(second_ptr as CFTypeRef); + } + let pid = 0x6afe_0004; + let window_id = 80; + let first_generation = cua_driver_core::element_token::global() + .register_snapshot_with_identities(pid, window_id, 1, [(0, first_ptr as u64)]); + let cache = ElementCache::new(); + cache.update_with_generation( + pid, + window_id, + first_generation, + &[node_with_ptr(first_ptr)], + ); + let stale_token = cua_driver_core::element_token::token_for(first_generation, 0); + + let second_generation = cua_driver_core::element_token::global() + .register_snapshot_with_identities(pid, window_id, 1, [(0, second_ptr as u64)]); + cache.update_with_generation( + pid, + window_id, + second_generation, + &[node_with_ptr(second_ptr)], + ); + + let error = cache + .resolve_token(pid, Some(window_id), Some(0), &stale_token) + .err() + .expect("old token must not resolve replacement node at the same index"); + assert_eq!( + error.code, + cua_driver_core::element_token::TOKEN_STALE_GENERATION_CODE + ); + } } diff --git a/libs/cua-driver/rust/crates/platform-macos/src/ax/tree.rs b/libs/cua-driver/rust/crates/platform-macos/src/ax/tree.rs index fe1c053b9f..6293f10359 100644 --- a/libs/cua-driver/rust/crates/platform-macos/src/ax/tree.rs +++ b/libs/cua-driver/rust/crates/platform-macos/src/ax/tree.rs @@ -79,6 +79,9 @@ pub struct AXNode { pub actions: Vec, /// The raw AXUIElementRef pointer value, for caching. pub element_ptr: usize, + /// Snapshot-local identity of the exact AXUIElementRef represented by this + /// node. Tokens bind this value in addition to pid/window/generation/index. + pub node_identity: u64, /// Depth in the rendered markdown tree (matches the indent level used in /// `tree_markdown`). Layout containers AXScrollArea/AXGroup collapse so /// children share the parent's depth. @@ -453,6 +456,7 @@ unsafe fn walk_element( help: help.clone(), actions: actions.clone(), element_ptr, + node_identity: element_ptr as u64, depth, parent_element_index: parent_index, frame, @@ -486,6 +490,7 @@ unsafe fn walk_element( help: help.clone(), actions: vec![], element_ptr, + node_identity: element_ptr as u64, depth, parent_element_index: parent_index, frame, diff --git a/libs/cua-driver/rust/crates/platform-macos/src/browser/consent_ui.rs b/libs/cua-driver/rust/crates/platform-macos/src/browser/consent_ui.rs index 52703cbd76..f98999cd39 100644 --- a/libs/cua-driver/rust/crates/platform-macos/src/browser/consent_ui.rs +++ b/libs/cua-driver/rust/crates/platform-macos/src/browser/consent_ui.rs @@ -264,6 +264,7 @@ mod tests { help: None, actions: actions.iter().map(|value| (*value).to_owned()).collect(), element_ptr: 7, + node_identity: 7, depth, parent_element_index: None, frame: None, diff --git a/libs/cua-driver/rust/crates/platform-macos/src/browser/setup_ui.rs b/libs/cua-driver/rust/crates/platform-macos/src/browser/setup_ui.rs index 5ad17f63ce..9ab80768e2 100644 --- a/libs/cua-driver/rust/crates/platform-macos/src/browser/setup_ui.rs +++ b/libs/cua-driver/rust/crates/platform-macos/src/browser/setup_ui.rs @@ -990,6 +990,7 @@ mod tests { help: None, actions: actions.iter().map(|value| (*value).to_owned()).collect(), element_ptr: 7, + node_identity: 7, depth: 0, parent_element_index: None, frame: None, diff --git a/libs/cua-driver/rust/crates/platform-macos/src/input/ax_actions.rs b/libs/cua-driver/rust/crates/platform-macos/src/input/ax_actions.rs index 6e5ca47190..12a9c4fc9f 100644 --- a/libs/cua-driver/rust/crates/platform-macos/src/input/ax_actions.rs +++ b/libs/cua-driver/rust/crates/platform-macos/src/input/ax_actions.rs @@ -29,10 +29,21 @@ fn map_action(action: &str) -> &'static str { /// Set AXFocused=true on an element (for pre-focusing before key press). pub fn focus_element(element_ptr: usize) -> anyhow::Result<()> { let err = unsafe { set_bool_attr_true(element_ptr as AXUIElementRef, "AXFocused") }; + focus_result(err, false) +} + +pub fn focus_element_strict(element_ptr: usize) -> anyhow::Result<()> { + let err = unsafe { set_bool_attr_true(element_ptr as AXUIElementRef, "AXFocused") }; + focus_result(err, true) +} + +fn focus_result(err: i32, strict: bool) -> anyhow::Result<()> { if err == kAXErrorSuccess { Ok(()) + } else if strict { + anyhow::bail!("AXSetAttribute(AXFocused) failed with error {err}") } else { - // Focus errors are often benign (element doesn't support focus). + // Focus errors are often benign for legacy index targeting. tracing::warn!("AXSetAttribute(AXFocused) returned {err}"); Ok(()) } @@ -47,3 +58,15 @@ pub fn set_ax_value(element_ptr: usize, value: &str) -> anyhow::Result<()> { anyhow::bail!("AXUIElementSetAttributeValue(AXValue) failed with error {err}") } } + +#[cfg(test)] +mod tests { + use super::focus_result; + + #[test] + fn token_focus_miss_fails_closed_while_legacy_focus_remains_best_effort() { + let focus_miss = -25205; + assert!(focus_result(focus_miss, true).is_err()); + assert!(focus_result(focus_miss, false).is_ok()); + } +} diff --git a/libs/cua-driver/rust/crates/platform-macos/src/tools/click.rs b/libs/cua-driver/rust/crates/platform-macos/src/tools/click.rs index 4d4726ca04..6b24775bbc 100644 --- a/libs/cua-driver/rust/crates/platform-macos/src/tools/click.rs +++ b/libs/cua-driver/rust/crates/platform-macos/src/tools/click.rs @@ -69,8 +69,8 @@ fn def() -> &'static ToolDef { field is fully back-compat — omit it and you get the legacy left-click behaviour. \ Pixel path: routes through the CGEvent left/right/middle mouse-button primitives. \ AX path: \"right\" maps to AXShowMenu (same surface as the dedicated `right_click` \ - tool); \"middle\" has no AX equivalent and falls back to a pixel middle-click at the \ - element's center.\n\ + tool). \"middle\" has no AX equivalent; token-targeted middle clicks fail closed, \ + while legacy element_index targeting retains its pixel-center fallback.\n\ action: press (default), show_menu, pick, confirm, cancel, open.\n\ from_zoom: set true after a zoom call to auto-translate zoom-image pixel \ coordinates to full-window space." @@ -88,14 +88,14 @@ fn def() -> &'static ToolDef { "pid": { "type": "integer", "description": "Target process ID." }, "window_id": { "type": "integer", "description": "Target window ID. Required for element_index. Optional when element_token is supplied (the token carries it)." }, "element_index": { "type": "integer", "description": "Element index from last get_window_state. REQUIRES `pid` and `window_id` to be passed alongside it — element_index alone (no pid) fails fast with \"Missing required integer field: pid\"; it is not a silent no-op." }, - "element_token": { "type": "string", "description": "Opaque per-snapshot element handle from `structuredContent.elements[].element_token` of the last get_window_state. Takes precedence over element_index when both supplied. Returns an explicit \"stale\" error if the snapshot has been superseded — re-snapshot in that case." }, + "element_token": { "type": "string", "description": "Opaque stable AX node handle from `structuredContent.elements[].element_token` of the last get_window_state. Strictly bound to pid, window_id, generation, element_index, and AX node identity. If window_id or element_index are also supplied they must match. Unknown, cross-target, stale-generation, or identity-mismatch tokens fail closed." }, "x": { "type": "number", "description": "X in screenshot pixels, read straight off the image you were handed — no scaling math needed. With pid+window_id (capture_scope=window): window-local pixels from the get_window_state PNG (top-left origin). Windowless (no pid/window_id, capture_scope=desktop): pixels from the get_desktop_state PNG (the native full-display image). Either way, the pixel you read IS the pixel that gets clicked; the driver undoes the Retina backing scale + any downscale internally." }, "y": { "type": "number", "description": "Y in screenshot pixels (see x). Window-local from get_window_state, or full-display from get_desktop_state under capture_scope=desktop." }, "action": { "type": "string", "description": "AX action: press, show_menu, pick, confirm, cancel, open." }, "button": { "type": "string", "enum": ["left", "right", "middle"], - "description": "Mouse button. Default: \"left\" — omit for legacy left-click behaviour. Pixel path uses the matching CGEvent primitive; AX path maps \"right\" to AXShowMenu and falls back to a pixel middle-click at the element's center for \"middle\"." + "description": "Mouse button. Default: \"left\" — omit for legacy left-click behaviour. Pixel path uses the matching CGEvent primitive; AX path maps \"right\" to AXShowMenu. Token-targeted \"middle\" fails closed because AX has no middle-click action; pass x,y explicitly for a pixel middle-click." }, "count": { "type": "integer", "description": "Click count (pixel path only). Default 1." }, "modifier": { @@ -258,31 +258,29 @@ impl Tool for ClickTool { // the calling session's cursor, not the shared "default" one. let cursor_key = super::cursor_tools::resolve_cursor_key(&args); - // Surface 6: resolve element_token / element_index precedence - // BEFORE the pixel-path fallback. Token wins on disagreement; a - // stale token returns an explicit error instead of silently - // falling back to the integer (Surface 6 hard constraint). let element_token_arg = args.opt_str("element_token"); + let token_targeted = element_token_arg.is_some(); let window_id_arg = args.opt_u64("window_id").map(|v| v as u32); let element_index_arg = args.opt_u64("element_index").map(|v| v as usize); - let resolved = match cua_driver_core::element_token::resolve_element_args( - pid, - element_index_arg, - element_token_arg.as_deref(), - window_id_arg, - "click", - ) { - Ok(r) => r, - Err(e) => return e, - }; - let (element_index, window_id, _via_token) = match resolved { - cua_driver_core::element_token::ResolvedElement::None => (None, window_id_arg, false), - cua_driver_core::element_token::ResolvedElement::Element { - window_id: wid, - element_index: idx, - via_token, - } => (Some(idx), wid, via_token), - }; + let (element_index, window_id, token_guard, token_generation) = + if let Some(token) = element_token_arg.as_deref() { + match self.state.element_cache.resolve_token( + pid, + window_id_arg, + element_index_arg, + token, + ) { + Ok(validated) => ( + Some(validated.element_index), + Some(validated.window_id), + Some(validated.element), + Some(validated.generation), + ), + Err(error) => return error.into_tool_result(), + } + } else { + (element_index_arg, window_id_arg, None, None) + }; let x = args .opt_f64("x") .or_else(|| args.opt_i64("x").map(|i| i as f64)); @@ -292,9 +290,8 @@ impl Tool for ClickTool { let action = args.str_or("action", "press"); // Surface 5: optional `button` arg, default "left" preserves legacy behaviour. // Pixel path: routes to left/right/middle CGEvent primitives. - // AX path: "right" delegates to AXShowMenu (same surface as right_click); - // "middle" has no AX equivalent and falls back to a pixel middle-click - // at the element's screen-space center. + // AX path: "right" delegates to AXShowMenu. Token-targeted middle + // clicks fail closed; legacy index targeting keeps its pixel fallback. let button_str = args.str_or("button", "left").to_lowercase(); // delivery_mode: per-call ladder rung. Foreground briefly activates the // target for both AX and pixel paths, then restores the prior app. @@ -322,14 +319,17 @@ impl Tool for ClickTool { // concurrent get_window_state on the same (pid, window_id) while // this click is mid-flight (use-after-free → daemon crash). The // guard lives to the end of this method, past the AX action below. - let element_guard = match self.state.element_cache.get_element_retained(pid, wid, idx) { - Some(e) => e, - None => { - return ToolResult::error(format!( - "Element index {idx} not found in cache for pid={pid} window_id={wid}. \ - Call get_window_state first." - )) - } + let element_guard = match token_guard { + Some(element) => element, + None => match self.state.element_cache.get_element_retained(pid, wid, idx) { + Some(e) => e, + None => { + return ToolResult::error(format!( + "Element index {idx} not found in cache for pid={pid} window_id={wid}. \ + Call get_window_state first." + )) + } + }, }; let element_ptr = element_guard.as_ptr(); @@ -352,11 +352,12 @@ impl Tool for ClickTool { .ok() .flatten(); - // Surface 5: button=middle on the AX path has no AX equivalent. - // Fall back to a pixel middle-click at the element's screen-space center - // so the request still produces a real middle-button event (browser tab - // close, autoscroll, etc.). If we can't resolve a center, error rather - // than silently degrade to AXPress. + if let Some(error) = token_middle_click_error(token_targeted, &button_str) { + return error; + } + + // Legacy element_index targeting retains the explicit pixel-center + // middle-click behavior. Capability targeting never reaches here. if button_str == "middle" { let (cx, cy) = match center { Some(c) => c, @@ -493,6 +494,11 @@ impl Tool for ClickTool { "verified": false, "effect": if suspected_noop { "suspected_noop" } else { "unverifiable" }, }); + if let Some(generation) = token_generation { + structured["element_token"] = + serde_json::json!(element_token_arg.as_deref()); + structured["generation"] = serde_json::json!(generation); + } if suspected_noop { structured["escalation"] = serde_json::json!({ "recommended": "px", @@ -825,6 +831,20 @@ impl Tool for ClickTool { } } +fn token_middle_click_error(token_targeted: bool, button: &str) -> Option { + (token_targeted && button == "middle").then(|| { + ToolResult::error( + "click(button=middle) with element_token is unsupported because AX has no \ + middle-click action; refusing implicit CGEvent/pixel fallback. Pass x and y \ + explicitly to request a pixel middle-click.", + ) + .with_structured(serde_json::json!({ + "code": "element_token_middle_click_unsupported", + "path": "ax", + })) + }) +} + // ── AX click implementation (blocking) ─────────────────────────────────────── /// Returns `(summary_text, needs_webkit_delay, suspected_noop)`. @@ -1031,4 +1051,15 @@ mod tests { assert_eq!(s, v); } } + + #[test] + fn token_targeted_middle_click_fails_closed_before_pixel_dispatch() { + let result = token_middle_click_error(true, "middle") + .expect("token-targeted middle click must be rejected"); + assert_eq!(result.is_error, Some(true)); + let structured = result.structured_content.expect("structured error"); + assert_eq!(structured["code"], "element_token_middle_click_unsupported"); + assert!(token_middle_click_error(false, "middle").is_none()); + assert!(token_middle_click_error(true, "left").is_none()); + } } diff --git a/libs/cua-driver/rust/crates/platform-macos/src/tools/double_click.rs b/libs/cua-driver/rust/crates/platform-macos/src/tools/double_click.rs index f9fdafc0be..ea97f39b0e 100644 --- a/libs/cua-driver/rust/crates/platform-macos/src/tools/double_click.rs +++ b/libs/cua-driver/rust/crates/platform-macos/src/tools/double_click.rs @@ -78,30 +78,28 @@ impl Tool for DoubleClickTool { let element_token_arg = args.opt_str("element_token"); let window_id_arg = args.opt_u64("window_id").map(|v| v as u32); let element_index_arg = args.opt_u64("element_index").map(|v| v as usize); - let resolved = match cua_driver_core::element_token::resolve_element_args( + let resolved = match super::resolve_element_target( + &self.state, pid, + window_id_arg, element_index_arg, element_token_arg.as_deref(), - window_id_arg, - "double_click", ) { Ok(r) => r, Err(e) => return e, }; - let (element_index, window_id) = match resolved { - cua_driver_core::element_token::ResolvedElement::None => (None, window_id_arg), - cua_driver_core::element_token::ResolvedElement::Element { - window_id: wid, - element_index: idx, - via_token: _, - } => (Some(idx), wid), - }; + let token_targeted = resolved.via_token; + let element_index = resolved.element_index; + let window_id = resolved.window_id; + let token_guard = resolved.retained; // ── AX element path ────────────────────────────────────────────────── if let (Some(idx), Some(wid)) = (element_index, window_id) { // Retain out of the cache so a concurrent get_window_state can't // free the element mid-action (use-after-free → daemon crash). - let element_guard = match self.state.element_cache.get_element_retained(pid, wid, idx) { + let element_guard = match token_guard + .or_else(|| self.state.element_cache.get_element_retained(pid, wid, idx)) + { Some(e) => e, None => { return ToolResult::error(format!( @@ -115,7 +113,14 @@ impl Tool for DoubleClickTool { // so its ClickPulse lands on THIS session's cursor, not "default". let ck = cursor_key.clone(); let result = tokio::task::spawn_blocking(move || { - ax_double_click(pid, wid, element_ptr, idx, &ck) + ax_double_click( + pid, + wid, + element_ptr, + idx, + &ck, + super::allow_pixel_fallback(token_targeted), + ) }) .await; @@ -260,6 +265,7 @@ fn ax_double_click( element_ptr: usize, idx: usize, cursor_key: &str, + allow_pixel_fallback: bool, ) -> anyhow::Result { let element = element_ptr as AXUIElementRef; @@ -274,6 +280,11 @@ fn ax_double_click( "AXOpen returned {err} for element [{idx}], falling back to pixel double-click" ); } + if !allow_pixel_fallback { + anyhow::bail!( + "token-targeted double_click requires AXOpen; refusing pixel fallback for exact-node capability" + ); + } // Resolve screen center and fall back to pixel double-click. let (cx, cy) = unsafe { element_screen_center(element) } diff --git a/libs/cua-driver/rust/crates/platform-macos/src/tools/get_window_state.rs b/libs/cua-driver/rust/crates/platform-macos/src/tools/get_window_state.rs index 025f66eb8b..97518f9f81 100644 --- a/libs/cua-driver/rust/crates/platform-macos/src/tools/get_window_state.rs +++ b/libs/cua-driver/rust/crates/platform-macos/src/tools/get_window_state.rs @@ -188,9 +188,26 @@ impl Tool for GetWindowStateTool { } }; - // Update element cache. + let elem_count_for_snapshot = tree_result + .as_ref() + .map(|r| r.nodes.iter().filter(|n| n.element_index.is_some()).count()) + .unwrap_or(0); + let stable_nodes = tree_result + .as_ref() + .into_iter() + .flat_map(|r| r.nodes.iter()) + .filter_map(|node| node.element_index.map(|idx| (idx, node.node_identity))); + let snapshot_id = cua_driver_core::element_token::global() + .register_snapshot_with_identities( + pid, + window_id, + elem_count_for_snapshot, + stable_nodes, + ); if let Some(ref r) = tree_result { - self.state.element_cache.update(pid, window_id, &r.nodes); + self.state + .element_cache + .update_with_generation(pid, window_id, snapshot_id, &r.nodes); } // Capture the screenshot and deliver it alongside the tree — the @@ -293,16 +310,6 @@ impl Tool for GetWindowStateTool { // generated even when the walk returned no elements so consumers // calling `get_window_state` and then immediately re-snapshotting // get a clean LRU step every time. - let elem_count_for_snapshot = tree_result - .as_ref() - .map(|r| r.nodes.iter().filter(|n| n.element_index.is_some()).count()) - .unwrap_or(0); - let snapshot_id = cua_driver_core::element_token::global().register_snapshot( - pid, - window_id, - elem_count_for_snapshot, - ); - // Build the structured `elements` array — one entry per actionable // node, matching the order (and indices) of the markdown rendering. // This is the preferred consumption path; `tree_markdown` is kept @@ -320,18 +327,12 @@ impl Tool for GetWindowStateTool { "element_count": element_count, "tree_markdown": tree_md, "elements": elements_json, - // Surface 6: an opaque snapshot identifier consumers can log - // alongside the per-element tokens for debug correlation. - // Same value embedded in every `element_token` emitted in - // `elements[]` above. Additive — old consumers ignore it. - "snapshot_id": cua_driver_core::element_token::token_for(snapshot_id, 0) - .trim_end_matches(":0") - .to_string(), "_note": "Prefer `elements` — `tree_markdown` will continue to work \ but new fields will only be added to the structured side. \ Issue #22865: use `max_elements` / `max_depth` to bound the \ AX walk on apps with very large trees." }); + cua_driver_core::element_token::add_snapshot_metadata(&mut structured, snapshot_id); // Best-effort-background ladder, rung (2): an AX walk that ran but found // zero actionable elements is NOT a clean snapshot — the window may be a // non-AX surface (canvas/WebGL) or its tree wasn't ready (Chromium needs @@ -395,6 +396,15 @@ impl Tool for GetWindowStateTool { pub(crate) fn build_elements_array_with_token( nodes: &[crate::ax::tree::AXNode], snapshot_id: u32, +) -> Vec { + build_elements_array_inner(nodes, |idx| { + Some(cua_driver_core::element_token::token_for(snapshot_id, idx)) + }) +} + +fn build_elements_array_inner( + nodes: &[crate::ax::tree::AXNode], + mut token_for_index: impl FnMut(usize) -> Option, ) -> Vec { nodes .iter() @@ -414,15 +424,12 @@ pub(crate) fn build_elements_array_with_token( .map(|[x, y, w, h]| serde_json::json!({ "x": x, "y": y, "w": w, "h": h })); let mut entry = serde_json::json!({ "element_index": idx, - // Surface 6: opaque token paired to the integer index. - // Tools accept either; the token has explicit validity - // (invalidated when the next snapshot supersedes this - // one in the per-pid LRU). See cua-driver-core's - // `element_token` module. - "element_token": cua_driver_core::element_token::token_for(snapshot_id, idx), "role": node.role, "depth": node.depth, }); + if let Some(token) = token_for_index(idx) { + entry["element_token"] = serde_json::Value::String(token); + } if let Some(label) = label { entry["label"] = serde_json::Value::String(label); } @@ -482,18 +489,7 @@ pub(crate) fn build_elements_array_with_token( /// `build_elements_array_with_token`. #[allow(dead_code)] pub(crate) fn build_elements_array(nodes: &[crate::ax::tree::AXNode]) -> Vec { - // Use a snapshot_id of 0 only to satisfy the signature; tokens - // built from id=0 are not registered and would fail the registry's - // stale check — but since this entry point is only kept for - // pre-existing callers (none in production after Surface 6), it - // strips the token field after rendering. - let mut out = build_elements_array_with_token(nodes, 0); - for entry in &mut out { - if let Some(obj) = entry.as_object_mut() { - obj.remove("element_token"); - } - } - out + build_elements_array_inner(nodes, |_| None) } #[cfg(test)] @@ -519,6 +515,7 @@ mod tests { help: None, actions: vec![], element_ptr: 0, + node_identity: idx.unwrap_or(usize::MAX) as u64, depth, parent_element_index: parent, frame, @@ -758,8 +755,14 @@ mod tests { .get("element_token") .and_then(|v| v.as_str()) .expect("element_token must be a string"); - assert!(tok.starts_with('s'), "token must use the 's' prefix: {tok}"); - assert!(tok.contains(':'), "token must be `s{{hex}}:{{idx}}`: {tok}"); + assert!( + tok.starts_with("e_") && tok.len() == 34, + "token must be an opaque UUID v4 capability: {tok}" + ); + assert!( + !tok.contains(':'), + "token must not expose generation or index fields: {tok}" + ); } // Each token must resolve through the registry to the same // (window_id, element_index) the integer field reports. diff --git a/libs/cua-driver/rust/crates/platform-macos/src/tools/mod.rs b/libs/cua-driver/rust/crates/platform-macos/src/tools/mod.rs index 19c4dfa63d..0e4212f59f 100644 --- a/libs/cua-driver/rust/crates/platform-macos/src/tools/mod.rs +++ b/libs/cua-driver/rust/crates/platform-macos/src/tools/mod.rs @@ -38,7 +38,59 @@ use cua_driver_core::tool::ToolRegistry; use std::collections::HashMap; use std::sync::Arc; -use crate::{ax::cache::ElementCache, cursor::state::CursorRegistry}; +use crate::{ + ax::cache::{ElementCache, RetainedElement}, + cursor::state::CursorRegistry, +}; + +pub(crate) struct ResolvedElementTarget { + pub element_index: Option, + pub window_id: Option, + pub retained: Option, + pub via_token: bool, +} + +pub(crate) fn resolve_element_target( + state: &ToolState, + pid: i32, + window_id: Option, + element_index: Option, + element_token: Option<&str>, +) -> Result { + if let Some(token) = element_token { + let validated = state + .element_cache + .resolve_token(pid, window_id, element_index, token) + .map_err(|error| error.into_tool_result())?; + return Ok(ResolvedElementTarget { + element_index: Some(validated.element_index), + window_id: Some(validated.window_id), + retained: Some(validated.element), + via_token: true, + }); + } + Ok(ResolvedElementTarget { + element_index, + window_id, + retained: None, + via_token: false, + }) +} + +pub(crate) fn allow_pixel_fallback(via_token: bool) -> bool { + !via_token +} + +#[cfg(test)] +mod stable_token_policy_tests { + use super::allow_pixel_fallback; + + #[test] + fn exact_node_capabilities_never_allow_pixel_fallback() { + assert!(!allow_pixel_fallback(true)); + assert!(allow_pixel_fallback(false)); + } +} /// Per-process zoom context — stores the padded crop origin and resize scale /// from the most recent `zoom` call, so `click(from_zoom=true)` can translate diff --git a/libs/cua-driver/rust/crates/platform-macos/src/tools/press_key.rs b/libs/cua-driver/rust/crates/platform-macos/src/tools/press_key.rs index 6649bdf921..d0efce25af 100644 --- a/libs/cua-driver/rust/crates/platform-macos/src/tools/press_key.rs +++ b/libs/cua-driver/rust/crates/platform-macos/src/tools/press_key.rs @@ -121,24 +121,20 @@ impl Tool for PressKeyTool { let element_token_arg = args.opt_str("element_token"); let window_id_arg = args.opt_u64("window_id").map(|v| v as u32); let element_index_arg = args.opt_u64("element_index").map(|v| v as usize); - let resolved = match cua_driver_core::element_token::resolve_element_args( + let resolved = match super::resolve_element_target( + &self.state, pid, + window_id_arg, element_index_arg, element_token_arg.as_deref(), - window_id_arg, - "press_key", ) { Ok(r) => r, Err(e) => return e, }; - let (element_index, window_id) = match resolved { - cua_driver_core::element_token::ResolvedElement::None => (None, window_id_arg), - cua_driver_core::element_token::ResolvedElement::Element { - window_id: wid, - element_index: idx, - via_token: _, - } => (Some(idx), wid), - }; + let token_targeted = resolved.via_token; + let element_index = resolved.element_index; + let window_id = resolved.window_id; + let token_guard = resolved.retained; // Remap "+" / "plus" → "=" + Shift (same physical key on US layout). let key = if key_raw == "+" || key_raw == "plus" { @@ -200,10 +196,17 @@ impl Tool for PressKeyTool { // the element before the suppressed focus below dereferences it // (use-after-free → daemon crash). Guard lives to method end. let pre_focus_guard = if let (Some(idx), Some(wid)) = (element_index, window_id) { - self.state.element_cache.get_element_retained(pid, wid, idx) + token_guard.or_else(|| self.state.element_cache.get_element_retained(pid, wid, idx)) } else { None }; + if let Some(idx) = element_index { + if pre_focus_guard.is_none() { + return ToolResult::error(format!( + "Element index {idx} not found. Call get_window_state first." + )); + } + } let pre_focus_ptr: Option = pre_focus_guard.as_ref().map(|g| g.as_ptr()); // ── Focus-suppression wrap (Swift WindowChangeDetector + FocusGuard) ── @@ -225,10 +228,19 @@ impl Tool for PressKeyTool { // Pre-focus the element under suppression so its // side-effects are captured by the snapshot + lease. if let Some(element_ptr) = pre_focus_ptr { - let _ = tokio::task::spawn_blocking(move || { - crate::input::ax_actions::focus_element(element_ptr) + match tokio::task::spawn_blocking(move || { + if token_targeted { + crate::input::ax_actions::focus_element_strict(element_ptr) + } else { + crate::input::ax_actions::focus_element(element_ptr) + } }) - .await; + .await + { + Ok(Ok(())) => {} + Ok(Err(error)) => return Ok(Err(error)), + Err(error) => return Err(error), + } tokio::time::sleep(std::time::Duration::from_millis(30)).await; } diff --git a/libs/cua-driver/rust/crates/platform-macos/src/tools/right_click.rs b/libs/cua-driver/rust/crates/platform-macos/src/tools/right_click.rs index 338cda20d9..f599f5c849 100644 --- a/libs/cua-driver/rust/crates/platform-macos/src/tools/right_click.rs +++ b/libs/cua-driver/rust/crates/platform-macos/src/tools/right_click.rs @@ -104,24 +104,20 @@ impl Tool for RightClickTool { let element_token_arg = args.opt_str("element_token"); let window_id_arg = args.opt_u64("window_id").map(|v| v as u32); let element_index_arg = args.opt_u64("element_index").map(|v| v as usize); - let resolved = match cua_driver_core::element_token::resolve_element_args( + let resolved = match super::resolve_element_target( + &self.state, pid, + window_id_arg, element_index_arg, element_token_arg.as_deref(), - window_id_arg, - "right_click", ) { Ok(r) => r, Err(e) => return e, }; - let (element_index, window_id) = match resolved { - cua_driver_core::element_token::ResolvedElement::None => (None, window_id_arg), - cua_driver_core::element_token::ResolvedElement::Element { - window_id: wid, - element_index: idx, - via_token: _, - } => (Some(idx), wid), - }; + let token_targeted = resolved.via_token; + let element_index = resolved.element_index; + let window_id = resolved.window_id; + let token_guard = resolved.retained; let x = args.opt_f64("x"); let y = args.opt_f64("y"); let has_xy = x.is_some() && y.is_some(); @@ -147,7 +143,9 @@ impl Tool for RightClickTool { if let (Some(idx), Some(wid)) = (element_index, window_id) { // Retain out of the cache so a concurrent get_window_state can't // free the element mid-action (use-after-free → daemon crash). - let element_guard = match self.state.element_cache.get_element_retained(pid, wid, idx) { + let element_guard = match token_guard + .or_else(|| self.state.element_cache.get_element_retained(pid, wid, idx)) + { Some(e) => e, None => { return ToolResult::error(format!( @@ -157,8 +155,16 @@ impl Tool for RightClickTool { }; let element_ptr = element_guard.as_ptr(); - let result = - tokio::task::spawn_blocking(move || ax_show_menu(element_ptr, idx, pid, wid)).await; + let result = tokio::task::spawn_blocking(move || { + ax_show_menu( + element_ptr, + idx, + pid, + wid, + super::allow_pixel_fallback(token_targeted), + ) + }) + .await; return match result { Ok(Ok(msg)) => ToolResult::text(msg), @@ -284,7 +290,13 @@ impl Tool for RightClickTool { // ── Blocking AX path ───────────────────────────────────────────────────────── -fn ax_show_menu(element_ptr: usize, idx: usize, pid: i32, wid: u32) -> anyhow::Result { +fn ax_show_menu( + element_ptr: usize, + idx: usize, + pid: i32, + wid: u32, + allow_pixel_fallback: bool, +) -> anyhow::Result { let element = element_ptr as AXUIElementRef; let role = unsafe { copy_string_attr(element, "AXRole") }.unwrap_or_default(); @@ -312,6 +324,11 @@ fn ax_show_menu(element_ptr: usize, idx: usize, pid: i32, wid: u32) -> anyhow::R // rather than erroring out. tracing::debug!("AXShowMenu returned {err} for [{idx}]; falling back to pixel right-click"); } + if !allow_pixel_fallback { + anyhow::bail!( + "token-targeted right_click requires AXShowMenu; refusing pixel fallback for exact-node capability" + ); + } // Pixel right-click at the element's screen-space center. let (cx, cy) = diff --git a/libs/cua-driver/rust/crates/platform-macos/src/tools/scroll.rs b/libs/cua-driver/rust/crates/platform-macos/src/tools/scroll.rs index 29e9ba2f04..25a313ff2b 100644 --- a/libs/cua-driver/rust/crates/platform-macos/src/tools/scroll.rs +++ b/libs/cua-driver/rust/crates/platform-macos/src/tools/scroll.rs @@ -181,24 +181,20 @@ impl Tool for ScrollTool { let element_token_arg = args.opt_str("element_token"); let window_id_arg = args.opt_u64("window_id").map(|v| v as u32); let element_index_arg = args.opt_u64("element_index").map(|v| v as usize); - let resolved = match cua_driver_core::element_token::resolve_element_args( + let resolved = match super::resolve_element_target( + &self.state, pid, + window_id_arg, element_index_arg, element_token_arg.as_deref(), - window_id_arg, - "scroll", ) { Ok(r) => r, Err(e) => return e, }; - let (element_index, window_id) = match resolved { - cua_driver_core::element_token::ResolvedElement::None => (None, window_id_arg), - cua_driver_core::element_token::ResolvedElement::Element { - window_id: wid, - element_index: idx, - via_token: _, - } => (Some(idx), wid), - }; + let token_targeted = resolved.via_token; + let element_index = resolved.element_index; + let window_id = resolved.window_id; + let token_guard = resolved.retained; // Resolve the pre-focus element pointer (if requested) outside // the suppression closure — only the focus_element() write itself @@ -207,7 +203,7 @@ impl Tool for ScrollTool { // the element before the suppressed focus below dereferences it // (use-after-free → daemon crash). Guard lives to method end. let pre_focus_guard = if let (Some(idx), Some(wid)) = (element_index, window_id) { - self.state.element_cache.get_element_retained(pid, wid, idx) + token_guard.or_else(|| self.state.element_cache.get_element_retained(pid, wid, idx)) } else { None }; @@ -229,17 +225,14 @@ impl Tool for ScrollTool { // AXScrollArea parent. Pressing those controls is a true // background-safe scroll: no activation, z-order change, or cursor move. if matches!(direction.as_str(), "up" | "down") { - if let (Some(index), Some(wid)) = (element_index, window_id) { - let native_element_guard = self - .state - .element_cache - .get_element_retained(pid, wid, index); + if let (Some(_index), Some(wid)) = (element_index, window_id) { + let native_element_ptr = pre_focus_ptr; let direction_for_ax = direction.clone(); let by_for_ax = by.clone(); let foreground = delivery_mode.is_foreground(); let ax_result = tokio::task::spawn_blocking(move || -> anyhow::Result<(bool, bool)> { - let Some(element_guard) = native_element_guard else { + let Some(element_ptr) = native_element_ptr else { return Ok((false, false)); }; if foreground { @@ -250,7 +243,7 @@ impl Tool for ScrollTool { || { delivered = unsafe { scroll_native_text_area( - element_guard.as_ptr() as AXUIElementRef, + element_ptr as AXUIElementRef, &direction_for_ax, &by_for_ax, amount, @@ -265,7 +258,7 @@ impl Tool for ScrollTool { Ok(( unsafe { scroll_native_text_area( - element_guard.as_ptr() as AXUIElementRef, + element_ptr as AXUIElementRef, &direction_for_ax, &by_for_ax, amount, @@ -297,6 +290,12 @@ impl Tool for ScrollTool { } } } + if token_targeted { + return ToolResult::error( + "token-targeted scroll requires a native AX scroll action; refusing \ + CGEvent wheel or keystroke fallback for an exact-node capability.", + ); + } // ── Targeted wheel path ───────────────────────────────────────────── // A target — element (preferred) OR window-local x,y — routes the scroll diff --git a/libs/cua-driver/rust/crates/platform-macos/src/tools/set_value.rs b/libs/cua-driver/rust/crates/platform-macos/src/tools/set_value.rs index b1c6de9226..3e8c27cb37 100644 --- a/libs/cua-driver/rust/crates/platform-macos/src/tools/set_value.rs +++ b/libs/cua-driver/rust/crates/platform-macos/src/tools/set_value.rs @@ -58,7 +58,11 @@ fn def() -> &'static ToolDef { date pickers, native text fields that expose settable AXValue).\n\ \n\ For free-form text entry into web inputs, prefer `type_text_chars` \ - which synthesises key events — AXValue writes are ignored by WebKit." + which synthesises key events — AXValue writes are ignored by WebKit.\n\ + \n\ + Success returns `changed`, `verified`, and `readback_value` when \ + AXValue is readable. Repeating the same value is an idempotent \ + success with `changed:false`." .into(), input_schema: serde_json::json!({ "type": "object", @@ -71,7 +75,7 @@ fn def() -> &'static ToolDef { "description": "CGWindowID for the window whose get_window_state produced the element_index. Required when element_index is used; optional when element_token is supplied (the token carries it)." }, "element_index": { "type": "integer", "description": "Element index from last get_window_state. Must be supplied unless element_token is provided. REQUIRES `pid` and `window_id` to be passed alongside it — element_index alone (no pid) fails fast with \"Missing required integer field: pid\"; it is not a silent no-op." }, - "element_token": { "type": "string", "description": "Opaque per-snapshot element handle from `structuredContent.elements[].element_token`. Takes precedence over element_index when both supplied. Returns an explicit \"stale\" error if the snapshot has been superseded." }, + "element_token": { "type": "string", "description": "Opaque stable AX node handle from `structuredContent.elements[].element_token`. Strictly bound to pid, window_id, generation, element_index, and AX node identity. If window_id or element_index are also supplied they must match. Unknown, cross-target, stale-generation, or identity-mismatch tokens fail closed." }, "value": { "type": "string", "description": "New value. AX will coerce to the element's native type." @@ -103,59 +107,58 @@ impl Tool for SetValueTool { Err(e) => return e, }; - // Surface 6: element_token / element_index precedence. Neither - // is now schema-required so the resolver can centralize the - // "missing addressing" error message. let element_token_arg = args.opt_str("element_token"); let window_id_arg = args.opt_u64("window_id").map(|v| v as u32); let element_index_arg = args.opt_u64("element_index").map(|v| v as usize); - let resolved = match cua_driver_core::element_token::resolve_element_args( - pid, - element_index_arg, - element_token_arg.as_deref(), - window_id_arg, - "set_value", - ) { - Ok(r) => r, - Err(e) => return e, - }; - let (element_index, window_id) = match resolved { - cua_driver_core::element_token::ResolvedElement::None => { + let (element_index, window_id, token_guard, token_generation) = + if let Some(token) = element_token_arg.as_deref() { + match self.state.element_cache.resolve_token( + pid, + window_id_arg, + element_index_arg, + token, + ) { + Ok(validated) => ( + validated.element_index, + validated.window_id, + Some(validated.element), + Some(validated.generation), + ), + Err(error) => return error.into_tool_result(), + } + } else if let Some(element_index) = element_index_arg { + let Some(window_id) = window_id_arg else { + return ToolResult::error( + "set_value requires window_id when element_index is used \ + (omit only when supplying element_token, which carries it).", + ); + }; + (element_index, window_id, None, None) + } else { return ToolResult::error( "set_value requires element_index (+ window_id) or element_token to \ address the target element.", - ) - } - cua_driver_core::element_token::ResolvedElement::Element { - window_id: Some(wid), - element_index: idx, - via_token: _, - } => (idx, wid), - cua_driver_core::element_token::ResolvedElement::Element { - window_id: None, .. - } => { - return ToolResult::error( - "set_value requires window_id when element_index is used \ - (omit only when supplying element_token, which carries it).", - ) - } - }; + ); + }; // Retain out of the cache so a concurrent get_window_state can't free // the element mid-action (use-after-free → daemon crash). Guard lives // to the end of this method, past the AX write below. let element_guard = - match self - .state - .element_cache - .get_element_retained(pid, window_id, element_index) - { - Some(e) => e, - None => { - return ToolResult::error(format!( - "Element index {element_index} not found. Call get_window_state first." - )) - } + match token_guard { + Some(element) => element, + None => match self.state.element_cache.get_element_retained( + pid, + window_id, + element_index, + ) { + Some(e) => e, + None => { + return ToolResult::error(format!( + "Element index {element_index} not found. Call get_window_state first." + )) + } + }, }; let element_ptr = element_guard.as_ptr(); @@ -182,9 +185,24 @@ impl Tool for SetValueTool { let changes = snapshot.detect_async().await; match result { - Ok(Ok(mut msg)) => { - msg.push_str(&changes.result_suffix()); - ToolResult::text(msg) + Ok(Ok(mut outcome)) => { + outcome.message.push_str(&changes.result_suffix()); + let mut structured = serde_json::json!({ + "path": "ax", + "element_index": element_index, + "window_id": window_id, + "changed": outcome.changed, + "verified": outcome.verified, + }); + if let Some(readback) = outcome.readback_value { + structured["value"] = serde_json::json!(readback); + structured["readback_value"] = structured["value"].clone(); + } + if let Some(generation) = token_generation { + structured["element_token"] = serde_json::json!(element_token_arg.as_deref()); + structured["generation"] = serde_json::json!(generation); + } + ToolResult::text(outcome.message).with_structured(structured) } Ok(Err(e)) => ToolResult::error(format!("set_value failed: {e}")), Err(e) => ToolResult::error(format!("Task error: {e}")), @@ -194,19 +212,40 @@ impl Tool for SetValueTool { // ── Blocking implementation (runs on spawn_blocking thread) ───────────────── +struct SetValueOutcome { + message: String, + readback_value: Option, + changed: bool, + verified: bool, +} + fn set_value_blocking( element_ptr: usize, element_index: usize, pid: i32, value: &str, -) -> anyhow::Result { +) -> anyhow::Result { let element = element_ptr as AXUIElementRef; let role = unsafe { copy_string_attr(element, "AXRole") }.unwrap_or_default(); + let before = read_ax_value(element); + if before + .as_deref() + .is_some_and(|current| values_equivalent(current, value)) + { + return Ok(SetValueOutcome { + message: format!( + "✅ AXValue on [{element_index}] {role} already equals the requested value." + ), + readback_value: before, + changed: false, + verified: true, + }); + } - if role == "AXPopUpButton" { + let message = if role == "AXPopUpButton" { let element_title = unsafe { copy_string_attr(element, "AXTitle") }.unwrap_or_default(); - select_popup_option(element, element_index, pid, value, &element_title) + select_popup_option(element, element_index, pid, value, &element_title)? } else { // Default path: write AXValue directly. Numeric controls (AXSlider / // AXStepper) reject a CFString with -25201 and need a CFNumber; text @@ -229,20 +268,51 @@ fn set_value_blocking( None => unsafe { set_string_attr(element, "AXValue", value) }, }; if err == kAXErrorSuccess { - Ok(format!("✅ Set AXValue on [{element_index}] {role}.")) + format!("✅ Set AXValue on [{element_index}] {role}.") } else if let Some(target) = numeric_target { // Both direct writes failed for a numeric target — fall back to // stepping the control via AXIncrement / AXDecrement actions. if step_to_value(element, target) { - Ok(format!( + format!( "✅ Set AXValue on [{element_index}] {role} via AXIncrement/AXDecrement stepping." - )) + ) } else { anyhow::bail!("AXUIElementSetAttributeValue(AXValue) failed with error {err}") } } else { anyhow::bail!("AXUIElementSetAttributeValue(AXValue) failed with error {err}") } + }; + let readback_value = read_ax_value(element); + let verified = readback_value + .as_deref() + .is_some_and(|current| values_equivalent(current, value)); + Ok(SetValueOutcome { + message, + readback_value, + changed: true, + verified, + }) +} + +fn read_ax_value(element: AXUIElementRef) -> Option { + unsafe { copy_string_attr(element, "AXValue") } + .or_else(|| unsafe { copy_number_attr(element, "AXValue") }.map(|value| value.to_string())) +} + +fn values_equivalent(current: &str, requested: &str) -> bool { + if current == requested { + return true; + } + match ( + current.trim().parse::(), + requested.trim().parse::(), + ) { + (Ok(current), Ok(requested)) => { + (current - requested).abs() + <= f64::EPSILON * current.abs().max(requested.abs()).max(1.0) + } + _ => false, } } @@ -498,3 +568,17 @@ fn hex_digit(n: u8) -> char { _ => '0', } } + +#[cfg(test)] +mod tests { + use super::values_equivalent; + + #[test] + fn value_equivalence_is_idempotent_for_text_and_numeric_representations() { + assert!(values_equivalent("ready", "ready")); + assert!(!values_equivalent("ready", "Ready")); + assert!(values_equivalent("1", "1.0")); + assert!(values_equivalent("0.5", "0.5000000000000000")); + assert!(!values_equivalent("1", "2")); + } +} diff --git a/libs/cua-driver/rust/crates/platform-macos/src/tools/type_text.rs b/libs/cua-driver/rust/crates/platform-macos/src/tools/type_text.rs index 514fb9ff52..2e89039f38 100644 --- a/libs/cua-driver/rust/crates/platform-macos/src/tools/type_text.rs +++ b/libs/cua-driver/rust/crates/platform-macos/src/tools/type_text.rs @@ -178,24 +178,20 @@ impl Tool for TypeTextTool { let element_token_arg = args.opt_str("element_token"); let window_id_arg = args.opt_u64("window_id").map(|v| v as u32); let element_index_arg = args.opt_u64("element_index").map(|v| v as usize); - let resolved = match cua_driver_core::element_token::resolve_element_args( + let resolved = match super::resolve_element_target( + &self.state, pid, + window_id_arg, element_index_arg, element_token_arg.as_deref(), - window_id_arg, - "type_text", ) { Ok(r) => r, Err(e) => return e, }; - let (element_index, window_id) = match resolved { - cua_driver_core::element_token::ResolvedElement::None => (None, window_id_arg), - cua_driver_core::element_token::ResolvedElement::Element { - window_id: wid, - element_index: idx, - via_token: _, - } => (Some(idx), wid), - }; + let token_targeted = resolved.via_token; + let element_index = resolved.element_index; + let window_id = resolved.window_id; + let token_guard = resolved.retained; let delay_ms = args.u64_or("delay_ms", 30); let delivery_mode = super::DeliveryMode::parse(args.opt_str("delivery_mode").as_deref()); @@ -248,7 +244,9 @@ impl Tool for TypeTextTool { // the blocking type below dereferences it (use-after-free → daemon // crash). The guard lives to method end, past type_text_blocking. let element_guard = if let (Some(idx), Some(wid)) = (element_index, window_id) { - match self.state.element_cache.get_element_retained(pid, wid, idx) { + match token_guard + .or_else(|| self.state.element_cache.get_element_retained(pid, wid, idx)) + { Some(e) => Some((e, idx)), None => { return ToolResult::error(format!( @@ -295,6 +293,7 @@ impl Tool for TypeTextTool { is_terminal_target, delivery_mode, window_id, + token_targeted, ) }) .await @@ -685,11 +684,37 @@ fn type_text_blocking( is_terminal_target: bool, delivery_mode: super::DeliveryMode, window_id: Option, + token_targeted: bool, ) -> anyhow::Result { // Original field value before any rung drives read-back verification only. // An unreadable value is not evidence that the field is empty. let before = read_axvalue(pid, element_ptr_and_idx); + if token_targeted { + let (ptr, idx) = element_ptr_and_idx + .ok_or_else(|| anyhow::anyhow!("token-targeted type_text lost its AX element"))?; + crate::input::ax_actions::focus_element_strict(ptr)?; + let element = ptr as AXUIElementRef; + let role = unsafe { copy_string_attr(element, "AXRole") }.unwrap_or_default(); + let title = unsafe { copy_string_attr(element, "AXTitle") }.unwrap_or_default(); + let err = unsafe { set_string_attr(element, "AXSelectedText", text) }; + let after = unsafe { copy_string_attr(element, "AXValue") }; + if err != kAXErrorSuccess || !verify_typed(before.as_deref(), after.as_deref(), text) { + anyhow::bail!( + "token-targeted type_text could not verify AXSelectedText on exact element" + ); + } + let idx_str = idx.map(|i| format!(" [{i}]")).unwrap_or_default(); + return Ok(TypeTextOutcome { + detail: format!(" into{idx_str} {role} \"{title}\""), + path: PATH_AX, + verified: true, + // The AXSelectedText write above was read back and verified, so the + // whole text landed — same contract as the focused-element AX rung. + delivered_chars: Some(text.chars().count()), + }); + } + // --- Foreground rung: explicit agent request (skip AX/background ladder). --- if delivery_mode.is_foreground() { // Settle between front+focus and the first keystroke — see the @@ -857,6 +882,7 @@ mod tests { /*is_terminal_target=*/ true, super::super::DeliveryMode::Background, None, + false, ); // We don't care whether r is Ok or Err — what matters is that // calling it with is_terminal_target=true is safe and never diff --git a/libs/cua-driver/rust/crates/platform-macos/src/tools/type_text_chars.rs b/libs/cua-driver/rust/crates/platform-macos/src/tools/type_text_chars.rs index 5edbb1c8e2..f36ecc7d03 100644 --- a/libs/cua-driver/rust/crates/platform-macos/src/tools/type_text_chars.rs +++ b/libs/cua-driver/rust/crates/platform-macos/src/tools/type_text_chars.rs @@ -72,25 +72,27 @@ impl Tool for TypeTextCharsTool { let element_token_arg = args.opt_str("element_token"); let window_id_arg = args.opt_u64("window_id").map(|v| v as u32); let element_index_arg = args.opt_u64("element_index").map(|v| v as usize); - let resolved = match cua_driver_core::element_token::resolve_element_args( + let resolved = match super::resolve_element_target( + &self.state, pid, + window_id_arg, element_index_arg, element_token_arg.as_deref(), - window_id_arg, - "type_text_chars", ) { Ok(r) => r, Err(e) => return e, }; - let (element_index, window_id) = match resolved { - cua_driver_core::element_token::ResolvedElement::None => (None, window_id_arg), - cua_driver_core::element_token::ResolvedElement::Element { - window_id: wid, - element_index: idx, - via_token: _, - } => (Some(idx), wid), - }; + let token_targeted = resolved.via_token; + let element_index = resolved.element_index; + let window_id = resolved.window_id; + let token_guard = resolved.retained; let type_chars_only = args.bool_or("type_chars_only", false); + if token_targeted && type_chars_only { + return ToolResult::error( + "type_chars_only cannot be used with element_token because it would discard \ + the exact-node focus binding.", + ); + } // Pre-focus element if requested. if !type_chars_only { @@ -98,17 +100,36 @@ impl Tool for TypeTextCharsTool { // Retain so a concurrent get_window_state can't free the element // during the focus call (use-after-free → daemon crash). The // guard outlives the awaited spawn_blocking below. - if let Some(element_guard) = - self.state.element_cache.get_element_retained(pid, wid, idx) + let element_guard = match token_guard + .or_else(|| self.state.element_cache.get_element_retained(pid, wid, idx)) { - let element_ptr = element_guard.as_ptr(); - let _ = tokio::task::spawn_blocking(move || { + Some(element) => element, + None => { + return ToolResult::error(format!( + "Element index {idx} not found. Call get_window_state first." + )) + } + }; + let element_ptr = element_guard.as_ptr(); + let focus = tokio::task::spawn_blocking(move || { + if token_targeted { + crate::input::ax_actions::focus_element_strict(element_ptr) + } else { crate::input::ax_actions::focus_element(element_ptr) - }) - .await; - drop(element_guard); - tokio::time::sleep(std::time::Duration::from_millis(50)).await; + } + }) + .await; + match focus { + Ok(Ok(())) => {} + Ok(Err(error)) => { + return ToolResult::error(format!( + "type_text_chars exact-node focus failed: {error}" + )) + } + Err(error) => return ToolResult::error(format!("Focus task failed: {error}")), } + drop(element_guard); + tokio::time::sleep(std::time::Duration::from_millis(50)).await; } } diff --git a/libs/cua-driver/rust/crates/platform-windows/src/tools/impl_.rs b/libs/cua-driver/rust/crates/platform-windows/src/tools/impl_.rs index 1e59dbe411..3cc6e8151b 100644 --- a/libs/cua-driver/rust/crates/platform-windows/src/tools/impl_.rs +++ b/libs/cua-driver/rust/crates/platform-windows/src/tools/impl_.rs @@ -1095,10 +1095,10 @@ impl Tool for GetWindowStateTool { .collect(); structured["elements"] = json!(elements); // Surface 6: snapshot id mirror for debug correlation. - structured["snapshot_id"] = - json!(cua_driver_core::element_token::token_for(snapshot_id, 0) - .trim_end_matches(":0") - .to_string()); + cua_driver_core::element_token::add_snapshot_metadata( + &mut structured, + snapshot_id, + ); structured["_note"] = json!( "Prefer `elements` — `tree_markdown` will continue to work \ but new fields will only be added to the structured side. \