fix: enforce canonical state vocabulary and live is visible evidence

Add role/state vocabulary modules and move is --property visible onto live
bounds plus hidden/offscreen tokens so off-screen elements no longer pass.

Co-authored-by: Cursor <cursoragent@cursor.com>

crates/core/src/commands/is_check.rs

@@ -5,6 +5,7 @@ use crate::{
     element_state::ElementState,
     error::AppError,
     refs::RefEntry,
+    state::{self, CHECKED, DISABLED, EXPANDED, FOCUSED, VisibilityEvidence},
 };
 use serde_json::{Value, json};
 
@@ -22,7 +23,6 @@ pub enum IsProperty {
     Expanded,
 }
 
-/// State is read live when the platform supports it, then falls back to snapshot state.
 pub fn execute(
     args: IsArgs,
     adapter: &dyn PlatformAdapter,
@@ -30,8 +30,6 @@ pub fn execute(
 ) -> Result<Value, AppError> {
     let (entry, handle) =
         resolve_ref_with_context(&args.ref_id, args.snapshot_id.as_deref(), adapter, context)?;
-    let state = optional_live_read(adapter.get_live_state(handle.handle()))?
-        .unwrap_or_else(|| state_from_ref_entry(&entry));
 
     let prop_name = match args.property {
         IsProperty::Visible => "visible",
@@ -41,14 +39,46 @@ pub fn execute(
         IsProperty::Expanded => "expanded",
     };
 
-    let applicable = is_applicable(&args.property, &entry, &state);
+    let live_state = optional_live_read(adapter.get_live_state(handle.handle()))?;
+    let state = live_state
+        .clone()
+        .unwrap_or_else(|| state_from_ref_entry(&entry));
+    let states_from_live = live_state.is_some();
+    let live_bounds = optional_live_read(adapter.get_element_bounds(handle.handle()))?;
+    let visibility = VisibilityEvidence {
+        bounds: live_bounds.or(entry.bounds),
+        states: state.states.clone(),
+        bounds_from_live: live_bounds.is_some(),
+        states_from_live,
+    };
+
+    let applicable = match args.property {
+        IsProperty::Visible => visibility.applicable(),
+        IsProperty::Enabled | IsProperty::Focused => true,
+        IsProperty::Checked => {
+            crate::roles::is_toggleable_role(&entry.role)
+                || state::has_state(&state.states, CHECKED)
+                || crate::capability::contains_any(
+                    &entry.available_actions,
+                    crate::capability::CHECKED_APPLICABILITY,
+                )
+        }
+        IsProperty::Expanded => {
+            crate::roles::is_expandable_role(&entry.role)
+                || state::has_state(&state.states, EXPANDED)
+                || crate::capability::contains_any(
+                    &entry.available_actions,
+                    crate::capability::EXPANDED_APPLICABILITY,
+                )
+        }
+    };
 
     let result = match args.property {
-        IsProperty::Visible => !has_state(&state, "hidden"),
-        IsProperty::Enabled => !has_state(&state, "disabled"),
-        IsProperty::Checked => has_state(&state, "checked"),
-        IsProperty::Focused => has_state(&state, "focused"),
-        IsProperty::Expanded => has_state(&state, "expanded"),
+        IsProperty::Visible => visibility.result(),
+        IsProperty::Enabled => !state::has_state(&state.states, DISABLED),
+        IsProperty::Checked => state::has_state(&state.states, CHECKED),
+        IsProperty::Focused => state::has_state(&state.states, FOCUSED),
+        IsProperty::Expanded => state::has_state(&state.states, EXPANDED),
     };
 
     Ok(
@@ -64,32 +94,6 @@ fn state_from_ref_entry(entry: &RefEntry) -> ElementState {
     }
 }
 
-fn has_state(state: &ElementState, name: &str) -> bool {
-    state.states.iter().any(|s| s == name)
-}
-
-fn is_applicable(property: &IsProperty, entry: &RefEntry, state: &ElementState) -> bool {
-    match property {
-        IsProperty::Visible | IsProperty::Enabled | IsProperty::Focused => true,
-        IsProperty::Checked => {
-            crate::roles::is_toggleable_role(&entry.role)
-                || has_state(state, "checked")
-                || crate::capability::contains_any(
-                    &entry.available_actions,
-                    crate::capability::CHECKED_APPLICABILITY,
-                )
-        }
-        IsProperty::Expanded => {
-            crate::roles::is_expandable_role(&entry.role)
-                || has_state(state, "expanded")
-                || crate::capability::contains_any(
-                    &entry.available_actions,
-                    crate::capability::EXPANDED_APPLICABILITY,
-                )
-        }
-    }
-}
-
 #[cfg(test)]
 #[path = "is_check_tests.rs"]
 mod tests;

crates/core/src/commands/is_check_tests.rs

@@ -1,13 +1,40 @@
 use super::*;
 use crate::adapter::{ActionOps, InputOps, ObservationOps, SystemOps};
 use crate::{
-    adapter::NativeHandle, error::AdapterError, refs::RefMap, refs_store::RefStore,
-    refs_test_support::HomeGuard,
+    adapter::NativeHandle, error::AdapterError, node::Rect, refs::RefMap, refs_store::RefStore,
+    refs_test_support::HomeGuard, state,
 };
 use std::sync::Mutex;
 
 struct LiveStateAdapter {
     state: Mutex<Option<ElementState>>,
+    bounds: Mutex<Option<Rect>>,
+    bounds_supported: bool,
+    state_supported: bool,
+}
+
+impl LiveStateAdapter {
+    fn with_live(bounds: Rect, states: Vec<String>) -> Self {
+        Self {
+            state: Mutex::new(Some(ElementState {
+                role: "button".into(),
+                states,
+                value: None,
+            })),
+            bounds: Mutex::new(Some(bounds)),
+            bounds_supported: true,
+            state_supported: true,
+        }
+    }
+
+    fn without_live_support() -> Self {
+        Self {
+            state: Mutex::new(None),
+            bounds: Mutex::new(None),
+            bounds_supported: false,
+            state_supported: false,
+        }
+    }
 }
 
 impl ObservationOps for LiveStateAdapter {
@@ -16,8 +43,18 @@ impl ObservationOps for LiveStateAdapter {
     }
 
     fn get_live_state(&self, _handle: &NativeHandle) -> Result<Option<ElementState>, AdapterError> {
+        if !self.state_supported {
+            return Err(AdapterError::not_supported("get_live_state"));
+        }
         Ok(self.state.lock().unwrap().clone())
     }
+
+    fn get_element_bounds(&self, _handle: &NativeHandle) -> Result<Option<Rect>, AdapterError> {
+        if !self.bounds_supported {
+            return Err(AdapterError::not_supported("get_element_bounds"));
+        }
+        Ok(*self.bounds.lock().unwrap())
+    }
 }
 
 impl ActionOps for LiveStateAdapter {}
@@ -53,6 +90,128 @@ fn entry(states: Vec<String>, value: Option<&str>, actions: Vec<&str>) -> RefEnt
     }
 }
 
+fn visible_bounds() -> Rect {
+    Rect {
+        x: 0.0,
+        y: 0.0,
+        width: 10.0,
+        height: 10.0,
+    }
+}
+
+#[test]
+fn hidden_element_reports_not_visible() {
+    let _guard = HomeGuard::new();
+    let snapshot_id = save_entry(entry(vec![], None, vec![]));
+    let adapter = LiveStateAdapter::with_live(visible_bounds(), vec![state::HIDDEN.into()]);
+
+    let result = execute(
+        IsArgs {
+            ref_id: "@e1".into(),
+            snapshot_id: Some(snapshot_id),
+            property: IsProperty::Visible,
+        },
+        &adapter,
+        &CommandContext::default(),
+    )
+    .unwrap();
+
+    assert_eq!(result["result"], false);
+    assert_eq!(result["applicable"], true);
+}
+
+#[test]
+fn zero_sized_bounds_report_not_visible() {
+    let _guard = HomeGuard::new();
+    let snapshot_id = save_entry(entry(vec![], None, vec![]));
+    let adapter = LiveStateAdapter::with_live(
+        Rect {
+            x: 0.0,
+            y: 0.0,
+            width: 0.0,
+            height: 10.0,
+        },
+        vec![],
+    );
+
+    let result = execute(
+        IsArgs {
+            ref_id: "@e1".into(),
+            snapshot_id: Some(snapshot_id),
+            property: IsProperty::Visible,
+        },
+        &adapter,
+        &CommandContext::default(),
+    )
+    .unwrap();
+
+    assert_eq!(result["result"], false);
+    assert_eq!(result["applicable"], true);
+}
+
+#[test]
+fn offscreen_element_reports_not_visible() {
+    let _guard = HomeGuard::new();
+    let snapshot_id = save_entry(entry(vec![], None, vec![]));
+    let adapter = LiveStateAdapter::with_live(visible_bounds(), vec![state::OFFSCREEN.into()]);
+
+    let result = execute(
+        IsArgs {
+            ref_id: "@e1".into(),
+            snapshot_id: Some(snapshot_id),
+            property: IsProperty::Visible,
+        },
+        &adapter,
+        &CommandContext::default(),
+    )
+    .unwrap();
+
+    assert_eq!(result["result"], false);
+    assert_eq!(result["applicable"], true);
+}
+
+#[test]
+fn visible_element_with_live_evidence_reports_true() {
+    let _guard = HomeGuard::new();
+    let snapshot_id = save_entry(entry(vec![], None, vec![]));
+    let adapter = LiveStateAdapter::with_live(visible_bounds(), vec![]);
+
+    let result = execute(
+        IsArgs {
+            ref_id: "@e1".into(),
+            snapshot_id: Some(snapshot_id),
+            property: IsProperty::Visible,
+        },
+        &adapter,
+        &CommandContext::default(),
+    )
+    .unwrap();
+
+    assert_eq!(result["result"], true);
+    assert_eq!(result["applicable"], true);
+}
+
+#[test]
+fn visible_degrades_applicability_when_live_reads_unsupported() {
+    let _guard = HomeGuard::new();
+    let snapshot_id = save_entry(entry(vec![], None, vec![]));
+    let adapter = LiveStateAdapter::without_live_support();
+
+    let result = execute(
+        IsArgs {
+            ref_id: "@e1".into(),
+            snapshot_id: Some(snapshot_id),
+            property: IsProperty::Visible,
+        },
+        &adapter,
+        &CommandContext::default(),
+    )
+    .unwrap();
+
+    assert_eq!(result["result"], false);
+    assert_eq!(result["applicable"], false);
+}
+
 #[test]
 fn checked_uses_live_canonical_state() {
     let _guard = HomeGuard::new();
@@ -63,6 +222,9 @@ fn checked_uses_live_canonical_state() {
             states: vec!["checked".into()],
             value: Some("1".into()),
         })),
+        bounds: Mutex::new(None),
+        bounds_supported: false,
+        state_supported: true,
     };
 
     let result = execute(
@@ -86,6 +248,9 @@ fn checked_does_not_infer_platform_values_in_core() {
     let snapshot_id = save_entry(entry(vec![], Some("1"), vec!["Toggle"]));
     let adapter = LiveStateAdapter {
         state: Mutex::new(None),
+        bounds: Mutex::new(None),
+        bounds_supported: false,
+        state_supported: true,
     };
 
     let result = execute(
@@ -109,6 +274,9 @@ fn checked_falls_back_to_snapshot_state_when_live_state_is_missing() {
     let snapshot_id = save_entry(entry(vec!["checked".into()], None, vec!["Toggle"]));
     let adapter = LiveStateAdapter {
         state: Mutex::new(None),
+        bounds: Mutex::new(None),
+        bounds_supported: false,
+        state_supported: true,
     };
 
     let result = execute(
@@ -130,16 +298,10 @@ fn checked_falls_back_to_snapshot_state_when_live_state_is_missing() {
 fn basic_state_properties_use_live_state() {
     let _guard = HomeGuard::new();
     let snapshot_id = save_entry(entry(vec![], None, vec![]));
-    let adapter = LiveStateAdapter {
-        state: Mutex::new(Some(ElementState {
-            role: "button".into(),
-            states: vec!["focused".into(), "expanded".into()],
-            value: None,
-        })),
-    };
+    let adapter =
+        LiveStateAdapter::with_live(visible_bounds(), vec!["focused".into(), "expanded".into()]);
 
     for (property, expected) in [
-        (IsProperty::Visible, true),
         (IsProperty::Enabled, true),
         (IsProperty::Focused, true),
         (IsProperty::Expanded, true),
@@ -183,6 +345,9 @@ fn action_availability_makes_toggle_and_expand_applicable() {
     });
     let adapter = LiveStateAdapter {
         state: Mutex::new(None),
+        bounds: Mutex::new(None),
+        bounds_supported: false,
+        state_supported: true,
     };
 
     for property in [IsProperty::Checked, IsProperty::Expanded] {
@@ -200,3 +365,17 @@ fn action_availability_makes_toggle_and_expand_applicable() {
         assert_eq!(result["applicable"], true);
     }
 }
+
+#[test]
+fn state_vocabulary_conformance_guard() {
+    for token in [
+        state::FOCUSED,
+        state::DISABLED,
+        state::CHECKED,
+        state::EXPANDED,
+        state::HIDDEN,
+        state::OFFSCREEN,
+    ] {
+        state::assert_states_in_vocabulary(&[token.to_string()]);
+    }
+}

crates/core/src/commands/wait_predicate.rs

@@ -1,7 +1,7 @@
 use crate::{
     action::Action,
     action_request::ActionRequest,
-    actionability::{bounds_are_visible, states_are_enabled},
+    actionability::states_are_enabled,
     adapter::{NativeHandle, PlatformAdapter, optional_live_read},
     error::{AdapterError, AppError, ErrorCode},
     refs::RefEntry,
@@ -146,12 +146,23 @@ fn enabled(
 }
 
 fn visible(
-    entry: &RefEntry,
+    _entry: &RefEntry,
     handle: &NativeHandle,
     adapter: &dyn PlatformAdapter,
 ) -> Result<Value, AdapterError> {
-    let bounds = optional_live_read(adapter.get_element_bounds(handle))?.or(entry.bounds);
-    Ok(json!({ "visible": bounds_are_visible(bounds) }))
+    let live_bounds = optional_live_read(adapter.get_element_bounds(handle))?;
+    let live_state = optional_live_read(adapter.get_live_state(handle))?;
+    let states_from_live = live_state.is_some();
+    let evidence = crate::state::VisibilityEvidence {
+        bounds: live_bounds,
+        states: live_state.map(|state| state.states).unwrap_or_default(),
+        bounds_from_live: live_bounds.is_some(),
+        states_from_live,
+    };
+    Ok(json!({
+        "visible": evidence.result(),
+        "applicable": evidence.applicable(),
+    }))
 }
 
 fn actionable(

crates/core/src/lib.rs

@@ -29,6 +29,7 @@ pub mod refs_store;
 #[cfg(test)]
 mod refs_test_support;
 pub(crate) mod resolved_element;
+pub mod role;
 pub mod roles;
 pub mod screenshot_target;
 pub(crate) mod search_text;
@@ -36,6 +37,7 @@ pub mod session;
 pub mod snapshot;
 pub mod snapshot_ref;
 pub mod snapshot_surface;
+pub mod state;
 pub(crate) mod trace;
 pub(crate) mod trace_artifacts;
 pub mod trace_read;

crates/core/src/role.rs

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

crates/core/src/roles.rs

@@ -1,3 +1,5 @@
+use crate::role::Role;
+
 /// Interactive roles that receive refs during snapshot allocation.
 ///
 /// Each entry must be produced by at least one platform adapter's native-to-canonical
@@ -39,7 +41,7 @@ pub fn normalize_role_query(role: &str) -> String {
 
 /// Returns true when `role` is in [`INTERACTIVE_ROLES`].
 pub fn is_interactive_role(role: &str) -> bool {
-    INTERACTIVE_ROLES.contains(&role)
+    Role::parse(role).is_interactive()
 }
 
 /// Returns true for roles whose checked/unchecked state can be queried and set.

crates/core/src/state.rs

@@ -0,0 +1,133 @@
+use crate::node::Rect;
+
+pub const FOCUSED: &str = "focused";
+pub const DISABLED: &str = "disabled";
+pub const SECURE: &str = "secure";
+pub const EXPANDED: &str = "expanded";
+pub const CHECKED: &str = "checked";
+pub const SELECTED: &str = "selected";
+pub const HIDDEN: &str = "hidden";
+pub const BUSY: &str = "busy";
+pub const MODAL: &str = "modal";
+pub const REQUIRED: &str = "required";
+pub const INDETERMINATE: &str = "indeterminate";
+pub const PRESSED: &str = "pressed";
+pub const READONLY: &str = "readonly";
+pub const OFFSCREEN: &str = "offscreen";
+pub const INVALID: &str = "invalid";
+pub const MULTISELECTABLE: &str = "multiselectable";
+pub const HASPOPUP: &str = "haspopup";
+
+pub const STATE_VOCABULARY: &[&str] = &[
+    FOCUSED,
+    DISABLED,
+    SECURE,
+    EXPANDED,
+    CHECKED,
+    SELECTED,
+    HIDDEN,
+    BUSY,
+    MODAL,
+    REQUIRED,
+    INDETERMINATE,
+    PRESSED,
+    READONLY,
+    OFFSCREEN,
+    INVALID,
+    MULTISELECTABLE,
+    HASPOPUP,
+];
+
+pub fn has_state(states: &[String], token: &str) -> bool {
+    states.iter().any(|state| state == token)
+}
+
+pub fn assert_states_in_vocabulary(states: &[String]) {
+    for state in states {
+        assert!(
+            STATE_VOCABULARY.contains(&state.as_str()),
+            "state token '{state}' is not in STATE_VOCABULARY"
+        );
+    }
+}
+
+pub fn is_visible(bounds: Option<Rect>, states: &[String]) -> bool {
+    crate::actionability::bounds_are_visible(bounds)
+        && !has_state(states, HIDDEN)
+        && !has_state(states, OFFSCREEN)
+}
+
+pub struct VisibilityEvidence {
+    pub bounds: Option<Rect>,
+    pub states: Vec<String>,
+    pub bounds_from_live: bool,
+    pub states_from_live: bool,
+}
+
+impl VisibilityEvidence {
+    pub fn applicable(&self) -> bool {
+        self.bounds_from_live && self.states_from_live
+    }
+
+    pub fn result(&self) -> bool {
+        if !self.applicable() {
+            return false;
+        }
+        is_visible(self.bounds, &self.states)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::node::Rect;
+
+    #[test]
+    fn vocabulary_contains_seventeen_tokens() {
+        assert_eq!(STATE_VOCABULARY.len(), 17);
+    }
+
+    #[test]
+    fn hidden_element_is_not_visible() {
+        let bounds = Some(Rect {
+            x: 0.0,
+            y: 0.0,
+            width: 10.0,
+            height: 10.0,
+        });
+        assert!(!is_visible(bounds, &[HIDDEN.to_string()]));
+    }
+
+    #[test]
+    fn zero_sized_bounds_are_not_visible() {
+        let bounds = Some(Rect {
+            x: 0.0,
+            y: 0.0,
+            width: 0.0,
+            height: 10.0,
+        });
+        assert!(!is_visible(bounds, &[]));
+    }
+
+    #[test]
+    fn offscreen_element_is_not_visible() {
+        let bounds = Some(Rect {
+            x: 0.0,
+            y: 0.0,
+            width: 10.0,
+            height: 10.0,
+        });
+        assert!(!is_visible(bounds, &[OFFSCREEN.to_string()]));
+    }
+
+    #[test]
+    fn visible_element_with_live_evidence() {
+        let bounds = Some(Rect {
+            x: 0.0,
+            y: 0.0,
+            width: 10.0,
+            height: 10.0,
+        });
+        assert!(is_visible(bounds, &[]));
+    }
+}