fix: address PR #93 third-pass review findings

Triage + validate (the review's line numbers were bogus diff-offset
artifacts, so every finding was checked against the real code) then fix the
real ones. 4 findings were false-positives (#2 core-internal fn, #5
VisibilityEvidence not in the actionability gate, #15 supertrait forward-design,
#20 as_str duplication), 2 were deferred as disproportionate (see below).
Verified by an adversarial code-read review + full gate: fmt, clippy
-D warnings, 1006 lib + 7 ABI tests, core isolation, 1.6MB, e2e 72/0, FFI
header regenerated with matching offset asserts.

- scroll gate: maybe_scroll_into_view now reads live state/bounds
  (fail-open to the snapshot entry on capability-less adapters) so an element
  that scrolled off-screen after the snapshot is still scrolled into view
- auto-wait: is_permanent_error now treats ErrorCode::Internal as permanent so
  an internal dispatch error fails fast instead of retrying the whole budget
- FFI header: document ad_execute_by_ref's 5000ms auto-wait default (and the
  timeout=0 single-shot escape hatch); correct the stale envelope version
  2.0 -> 2.1; add the 22 per-field offsetof static asserts for AdRefEntry
- skill docs: document --modifiers on the mouse commands, the receives_events
  occlusion check + occluder detail, the implicit scroll-into-view preflight,
  and --timeout-ms — and correct the actionability section to reflect that
  hover/drag run only the receives_events check (not the full battery) and
  fail fast with ACTION_FAILED rather than polling to TIMEOUT
- adapter: /// note that get_clipboard/set_clipboard were removed pre-1.0 for
  the typed content methods (C ABI unaffected)
- tests: StepMechanism serde roundtrip, ActionRequest legacy-no-timeout_ms
  deserialization, execute_by_ref unit tests; dedup the byte-identical
  StaleThenOkAdapter retry-counter into stale_retry_test_support

Deferred (disproportionate blast radius, tracked as follow-up issues):
process-state PID-reuse start-time corroboration (needs plumbing through the
already-large RefEntry + snapshot + platform adapter); a stub_ops! macro +
84-site test-adapter retrofit (large mechanical churn on the deliberate U0
four-trait split).

crates/core/src/action_request.rs

@@ -65,4 +65,22 @@ mod tests {
         assert!(request.policy.allow_focus_steal);
         assert!(!request.policy.allow_cursor_move);
     }
+
+    /// Regression coverage: `ActionRequest.timeout_ms` must stay
+    /// `#[serde(default)]` so a legacy payload recorded before `timeout_ms`
+    /// existed (or any FFI/batch caller that omits the key) still
+    /// deserializes instead of erroring out.
+    #[test]
+    fn action_request_json_without_timeout_ms_key_deserializes_to_none() {
+        let request: ActionRequest = serde_json::from_value(serde_json::json!({
+            "action": "Click",
+            "policy": {
+                "allow_focus_steal": false,
+                "allow_cursor_move": false,
+            },
+        }))
+        .unwrap();
+
+        assert_eq!(request.timeout_ms, None);
+    }
 }

crates/core/src/adapter/input.rs

@@ -4,6 +4,9 @@ use crate::{
     error::AdapterError,
 };
 
+/// `get_clipboard`/`set_clipboard` were removed pre-1.0 in favor of
+/// `get_clipboard_content`/`set_clipboard_content`; the C ABI
+/// (`ad_get_clipboard`/`ad_set_clipboard`) is unaffected.
 pub trait InputOps: Send + Sync {
     fn mouse_event(&self, _event: MouseEvent) -> Result<(), AdapterError> {
         Err(AdapterError::not_supported("mouse_event"))

crates/core/src/commands/drag_tests.rs

@@ -4,7 +4,8 @@ use crate::{
     action::DragParams,
     adapter::NativeHandle,
     capability,
-    error::{AdapterError, ErrorCode},
+    commands::stale_retry_test_support::StaleRetryCounter,
+    error::AdapterError,
     hit_test::HitTestResult,
     node::Rect,
     refs::{RefEntry, RefMap},
@@ -12,7 +13,6 @@ use crate::{
     refs_test_support::HomeGuard,
 };
 use std::sync::Mutex;
-use std::sync::atomic::{AtomicU32, Ordering};
 
 struct DragCaptureAdapter {
     captured: Mutex<Option<DragParams>>,
@@ -226,26 +226,20 @@ fn headed_xy_drag_never_steals_focus() {
 }
 
 struct StaleThenOkAdapter {
-    resolve_calls: AtomicU32,
-    fail_until: u32,
+    retry: StaleRetryCounter,
 }
 
 impl StaleThenOkAdapter {
     fn new(fail_until: u32) -> Self {
         Self {
-            resolve_calls: AtomicU32::new(0),
-            fail_until,
+            retry: StaleRetryCounter::new(fail_until),
         }
     }
 }
 
 impl ObservationOps for StaleThenOkAdapter {
     fn resolve_element_strict(&self, _entry: &RefEntry) -> Result<NativeHandle, AdapterError> {
-        let n = self.resolve_calls.fetch_add(1, Ordering::SeqCst) + 1;
-        if n <= self.fail_until {
-            return Err(AdapterError::new(ErrorCode::StaleRef, "not yet resolvable"));
-        }
-        Ok(NativeHandle::null())
+        self.retry.attempt()
     }
 
     fn get_element_bounds(&self, _handle: &NativeHandle) -> Result<Option<Rect>, AdapterError> {
@@ -295,7 +289,7 @@ fn transient_stale_ref_retries_then_succeeds_when_timeout_wired() {
     .unwrap();
 
     assert_eq!(value["dragged"], true);
-    assert!(adapter.resolve_calls.load(Ordering::SeqCst) >= 3);
+    assert!(adapter.retry.calls() >= 3);
 }
 
 struct OccludedFromAdapter {
@@ -387,5 +381,5 @@ fn timeout_none_makes_exactly_one_resolve_attempt() {
     .unwrap_err();
 
     assert_eq!(err.code(), "STALE_REF");
-    assert_eq!(adapter.resolve_calls.load(Ordering::SeqCst), 1);
+    assert_eq!(adapter.retry.calls(), 1);
 }

crates/core/src/commands/execute_by_ref.rs

@@ -76,3 +76,7 @@ pub fn execute_with_timeout(
         context,
     )
 }
+
+#[cfg(test)]
+#[path = "execute_by_ref_tests.rs"]
+mod tests;

crates/core/src/commands/execute_by_ref_tests.rs

@@ -0,0 +1,227 @@
+use super::*;
+use crate::adapter::{ActionOps, InputOps, ObservationOps, SystemOps};
+use crate::{
+    action::{Action, KeyCombo},
+    action_request::ActionRequest,
+    action_result::ActionResult,
+    adapter::NativeHandle,
+    error::{AdapterError, ErrorCode},
+    refs::{RefEntry, RefMap},
+    refs_store::RefStore,
+    refs_test_support::HomeGuard,
+};
+use std::sync::Mutex;
+use std::sync::atomic::{AtomicU32, Ordering};
+
+fn snapshot_with_ref(role: &str, available_actions: &[&str]) -> String {
+    let mut refmap = RefMap::new();
+    refmap.allocate(RefEntry {
+        pid: 1,
+        role: role.into(),
+        name: Some("Target".into()),
+        value: None,
+        description: None,
+        native_id: None,
+        states: vec![],
+        bounds: None,
+        bounds_hash: None,
+        available_actions: available_actions.iter().map(|a| (*a).to_string()).collect(),
+        source_app: None,
+        source_window_id: None,
+        source_window_title: None,
+        source_surface: crate::adapter::SnapshotSurface::Window,
+        root_ref: None,
+        path_is_absolute: false,
+        path: smallvec::SmallVec::new(),
+    });
+    RefStore::new().unwrap().save_new_snapshot(&refmap).unwrap()
+}
+
+struct StaleThenOkAdapter {
+    resolve_calls: AtomicU32,
+    fail_until: u32,
+}
+
+impl StaleThenOkAdapter {
+    fn new(fail_until: u32) -> Self {
+        Self {
+            resolve_calls: AtomicU32::new(0),
+            fail_until,
+        }
+    }
+}
+
+impl ObservationOps for StaleThenOkAdapter {
+    fn resolve_element_strict(&self, _entry: &RefEntry) -> Result<NativeHandle, AdapterError> {
+        let n = self.resolve_calls.fetch_add(1, Ordering::SeqCst) + 1;
+        if n <= self.fail_until {
+            return Err(AdapterError::new(ErrorCode::StaleRef, "not yet resolvable"));
+        }
+        Ok(NativeHandle::null())
+    }
+}
+
+impl ActionOps for StaleThenOkAdapter {
+    fn execute_action(
+        &self,
+        _handle: &NativeHandle,
+        _request: ActionRequest,
+    ) -> Result<ActionResult, AdapterError> {
+        Ok(ActionResult::new("click"))
+    }
+}
+
+impl InputOps for StaleThenOkAdapter {}
+impl SystemOps for StaleThenOkAdapter {}
+
+struct PolicyCaptureAdapter {
+    captured: Mutex<Option<ActionRequest>>,
+}
+
+impl PolicyCaptureAdapter {
+    fn new() -> Self {
+        Self {
+            captured: Mutex::new(None),
+        }
+    }
+}
+
+impl ObservationOps for PolicyCaptureAdapter {
+    fn resolve_element_strict(&self, _entry: &RefEntry) -> Result<NativeHandle, AdapterError> {
+        Ok(NativeHandle::null())
+    }
+}
+
+impl ActionOps for PolicyCaptureAdapter {
+    fn execute_action(
+        &self,
+        _handle: &NativeHandle,
+        request: ActionRequest,
+    ) -> Result<ActionResult, AdapterError> {
+        let name = request.action.name().to_string();
+        *self.captured.lock().unwrap() = Some(request);
+        Ok(ActionResult::new(name))
+    }
+}
+
+impl InputOps for PolicyCaptureAdapter {}
+impl SystemOps for PolicyCaptureAdapter {}
+
+#[test]
+fn default_action_timeout_ms_is_five_seconds() {
+    assert_eq!(DEFAULT_ACTION_TIMEOUT_MS, 5000);
+}
+
+/// `execute` must forward `DEFAULT_ACTION_TIMEOUT_MS` (not `None` or `0`) into
+/// the ref-action retry budget, exactly as a direct
+/// `execute_with_timeout(args, DEFAULT_ACTION_TIMEOUT_MS, ..)` call would.
+/// A transient `STALE_REF` that clears within the budget must therefore be
+/// retried until it resolves rather than surfacing on the first attempt.
+#[test]
+fn execute_forwards_default_timeout_and_retries_transient_stale_ref() {
+    let _guard = HomeGuard::new();
+    let snapshot_id = snapshot_with_ref("button", &["Click"]);
+    let adapter = StaleThenOkAdapter::new(2);
+
+    let value = execute(
+        ExecuteByRefArgs {
+            ref_id: "@e1",
+            snapshot_id: Some(&snapshot_id),
+            action: Action::Click,
+            caller_policy: InteractionPolicy::headless(),
+        },
+        &adapter,
+        &CommandContext::default(),
+    )
+    .unwrap();
+
+    assert_eq!(value["action"], "click");
+    assert!(adapter.resolve_calls.load(Ordering::SeqCst) >= 3);
+}
+
+/// `execute_with_timeout` must run `normalize_action_timeout_ms` on the raw
+/// timeout it is given, so a `0` (the CLI's "no retry budget" sentinel)
+/// collapses to `None` and the ref-action pipeline makes exactly one resolve
+/// attempt instead of silently retrying anyway.
+#[test]
+fn execute_with_timeout_zero_normalizes_to_single_attempt() {
+    let _guard = HomeGuard::new();
+    let snapshot_id = snapshot_with_ref("button", &["Click"]);
+    let adapter = StaleThenOkAdapter::new(1);
+
+    let err = execute_with_timeout(
+        ExecuteByRefArgs {
+            ref_id: "@e1",
+            snapshot_id: Some(&snapshot_id),
+            action: Action::Click,
+            caller_policy: InteractionPolicy::headless(),
+        },
+        0,
+        &adapter,
+        &CommandContext::default(),
+    )
+    .unwrap_err();
+
+    assert_eq!(err.code(), "STALE_REF");
+    assert_eq!(adapter.resolve_calls.load(Ordering::SeqCst), 1);
+}
+
+/// The effective policy is a join, not a passthrough of `caller_policy`: a
+/// caller supplying `headless` for an action whose CLI base is
+/// `focus_fallback` (here `PressKey`) must still end up with
+/// `allow_focus_steal = true`, because the join can only elevate the caller
+/// above the action's base, never downgrade below it.
+#[test]
+fn effective_policy_never_drops_below_action_base() {
+    let _guard = HomeGuard::new();
+    let snapshot_id = snapshot_with_ref("textfield", &["PressKey"]);
+    let adapter = PolicyCaptureAdapter::new();
+
+    execute(
+        ExecuteByRefArgs {
+            ref_id: "@e1",
+            snapshot_id: Some(&snapshot_id),
+            action: Action::PressKey(KeyCombo {
+                key: "A".into(),
+                modifiers: vec![],
+            }),
+            caller_policy: InteractionPolicy::headless(),
+        },
+        &adapter,
+        &CommandContext::default(),
+    )
+    .unwrap();
+
+    let captured = adapter.captured.lock().unwrap();
+    let policy = captured.as_ref().unwrap().policy;
+    assert!(policy.allow_focus_steal);
+    assert!(!policy.allow_cursor_move);
+}
+
+/// Symmetric case: a caller-supplied policy more permissive than the action's
+/// base must be honored, not clamped back down to the base. `Click`'s base is
+/// `headless` (both flags false); a caller passing `headed` must see both
+/// flags come through as `true` on the dispatched request.
+#[test]
+fn effective_policy_honors_caller_policy_above_action_base() {
+    let _guard = HomeGuard::new();
+    let snapshot_id = snapshot_with_ref("button", &["Click"]);
+    let adapter = PolicyCaptureAdapter::new();
+
+    execute(
+        ExecuteByRefArgs {
+            ref_id: "@e1",
+            snapshot_id: Some(&snapshot_id),
+            action: Action::Click,
+            caller_policy: InteractionPolicy::headed(),
+        },
+        &adapter,
+        &CommandContext::default(),
+    )
+    .unwrap();
+
+    let captured = adapter.captured.lock().unwrap();
+    let policy = captured.as_ref().unwrap().policy;
+    assert!(policy.allow_focus_steal);
+    assert!(policy.allow_cursor_move);
+}

crates/core/src/commands/hover_tests.rs

@@ -3,7 +3,8 @@ use crate::adapter::{ActionOps, InputOps, ObservationOps, SystemOps};
 use crate::{
     adapter::NativeHandle,
     capability,
-    error::{AdapterError, ErrorCode},
+    commands::stale_retry_test_support::StaleRetryCounter,
+    error::AdapterError,
     hit_test::HitTestResult,
     node::Rect,
     refs::{RefEntry, RefMap},
@@ -11,7 +12,6 @@ use crate::{
     refs_test_support::HomeGuard,
 };
 use std::sync::Mutex;
-use std::sync::atomic::{AtomicU32, Ordering};
 
 struct HoverCaptureAdapter {
     moved_to: Mutex<Option<MouseEvent>>,
@@ -147,26 +147,20 @@ fn headed_xy_hover_never_steals_focus() {
 }
 
 struct StaleThenOkAdapter {
-    resolve_calls: AtomicU32,
-    fail_until: u32,
+    retry: StaleRetryCounter,
 }
 
 impl StaleThenOkAdapter {
     fn new(fail_until: u32) -> Self {
         Self {
-            resolve_calls: AtomicU32::new(0),
-            fail_until,
+            retry: StaleRetryCounter::new(fail_until),
         }
     }
 }
 
 impl ObservationOps for StaleThenOkAdapter {
     fn resolve_element_strict(&self, _entry: &RefEntry) -> Result<NativeHandle, AdapterError> {
-        let n = self.resolve_calls.fetch_add(1, Ordering::SeqCst) + 1;
-        if n <= self.fail_until {
-            return Err(AdapterError::new(ErrorCode::StaleRef, "not yet resolvable"));
-        }
-        Ok(NativeHandle::null())
+        self.retry.attempt()
     }
 
     fn get_element_bounds(&self, _handle: &NativeHandle) -> Result<Option<Rect>, AdapterError> {
@@ -213,7 +207,7 @@ fn transient_stale_ref_retries_then_succeeds_when_timeout_wired() {
     .unwrap();
 
     assert_eq!(value["hovered"], true);
-    assert!(adapter.resolve_calls.load(Ordering::SeqCst) >= 3);
+    assert!(adapter.retry.calls() >= 3);
 }
 
 struct OccludedTargetAdapter {
@@ -302,5 +296,5 @@ fn timeout_none_makes_exactly_one_resolve_attempt() {
     .unwrap_err();
 
     assert_eq!(err.code(), "STALE_REF");
-    assert_eq!(adapter.resolve_calls.load(Ordering::SeqCst), 1);
+    assert_eq!(adapter.retry.calls(), 1);
 }

crates/core/src/commands/mod.rs

@@ -74,3 +74,6 @@ pub(crate) mod wait_timeout;
 
 #[cfg(test)]
 mod ref_policy_tests;
+
+#[cfg(test)]
+mod stale_retry_test_support;

crates/core/src/commands/scroll_tests.rs

@@ -5,34 +5,28 @@ use crate::{
     action_request::ActionRequest,
     action_result::ActionResult,
     adapter::NativeHandle,
-    error::{AdapterError, ErrorCode},
+    commands::stale_retry_test_support::StaleRetryCounter,
+    error::AdapterError,
     refs::{RefEntry, RefMap},
     refs_store::RefStore,
     refs_test_support::HomeGuard,
 };
-use std::sync::atomic::{AtomicU32, Ordering};
 
 struct StaleThenOkAdapter {
-    resolve_calls: AtomicU32,
-    fail_until: u32,
+    retry: StaleRetryCounter,
 }
 
 impl StaleThenOkAdapter {
     fn new(fail_until: u32) -> Self {
         Self {
-            resolve_calls: AtomicU32::new(0),
-            fail_until,
+            retry: StaleRetryCounter::new(fail_until),
         }
     }
 }
 
 impl ObservationOps for StaleThenOkAdapter {
     fn resolve_element_strict(&self, _entry: &RefEntry) -> Result<NativeHandle, AdapterError> {
-        let n = self.resolve_calls.fetch_add(1, Ordering::SeqCst) + 1;
-        if n <= self.fail_until {
-            return Err(AdapterError::new(ErrorCode::StaleRef, "not yet resolvable"));
-        }
-        Ok(NativeHandle::null())
+        self.retry.attempt()
     }
 }
 
@@ -97,7 +91,7 @@ fn transient_stale_ref_retries_then_succeeds_when_timeout_wired() {
     .unwrap();
 
     assert_eq!(value["action"], "scroll");
-    assert!(adapter.resolve_calls.load(Ordering::SeqCst) >= 3);
+    assert!(adapter.retry.calls() >= 3);
 }
 
 #[test]
@@ -120,5 +114,5 @@ fn timeout_none_makes_exactly_one_resolve_attempt() {
     .unwrap_err();
 
     assert_eq!(err.code(), "STALE_REF");
-    assert_eq!(adapter.resolve_calls.load(Ordering::SeqCst), 1);
+    assert_eq!(adapter.retry.calls(), 1);
 }

crates/core/src/commands/select_tests.rs

@@ -4,34 +4,28 @@ use crate::{
     action_request::ActionRequest,
     action_result::ActionResult,
     adapter::NativeHandle,
-    error::{AdapterError, ErrorCode},
+    commands::stale_retry_test_support::StaleRetryCounter,
+    error::AdapterError,
     refs::{RefEntry, RefMap},
     refs_store::RefStore,
     refs_test_support::HomeGuard,
 };
-use std::sync::atomic::{AtomicU32, Ordering};
 
 struct StaleThenOkAdapter {
-    resolve_calls: AtomicU32,
-    fail_until: u32,
+    retry: StaleRetryCounter,
 }
 
 impl StaleThenOkAdapter {
     fn new(fail_until: u32) -> Self {
         Self {
-            resolve_calls: AtomicU32::new(0),
-            fail_until,
+            retry: StaleRetryCounter::new(fail_until),
         }
     }
 }
 
 impl ObservationOps for StaleThenOkAdapter {
     fn resolve_element_strict(&self, _entry: &RefEntry) -> Result<NativeHandle, AdapterError> {
-        let n = self.resolve_calls.fetch_add(1, Ordering::SeqCst) + 1;
-        if n <= self.fail_until {
-            return Err(AdapterError::new(ErrorCode::StaleRef, "not yet resolvable"));
-        }
-        Ok(NativeHandle::null())
+        self.retry.attempt()
     }
 }
 
@@ -95,7 +89,7 @@ fn transient_stale_ref_retries_then_succeeds_when_timeout_wired() {
     .unwrap();
 
     assert_eq!(value["action"], "select");
-    assert!(adapter.resolve_calls.load(Ordering::SeqCst) >= 3);
+    assert!(adapter.retry.calls() >= 3);
 }
 
 #[test]
@@ -117,5 +111,5 @@ fn timeout_none_makes_exactly_one_resolve_attempt() {
     .unwrap_err();
 
     assert_eq!(err.code(), "STALE_REF");
-    assert_eq!(adapter.resolve_calls.load(Ordering::SeqCst), 1);
+    assert_eq!(adapter.retry.calls(), 1);
 }

crates/core/src/commands/set_value_tests.rs

@@ -4,34 +4,28 @@ use crate::{
     action_request::ActionRequest,
     action_result::ActionResult,
     adapter::NativeHandle,
-    error::{AdapterError, ErrorCode},
+    commands::stale_retry_test_support::StaleRetryCounter,
+    error::AdapterError,
     refs::{RefEntry, RefMap},
     refs_store::RefStore,
     refs_test_support::HomeGuard,
 };
-use std::sync::atomic::{AtomicU32, Ordering};
 
 struct StaleThenOkAdapter {
-    resolve_calls: AtomicU32,
-    fail_until: u32,
+    retry: StaleRetryCounter,
 }
 
 impl StaleThenOkAdapter {
     fn new(fail_until: u32) -> Self {
         Self {
-            resolve_calls: AtomicU32::new(0),
-            fail_until,
+            retry: StaleRetryCounter::new(fail_until),
         }
     }
 }
 
 impl ObservationOps for StaleThenOkAdapter {
     fn resolve_element_strict(&self, _entry: &RefEntry) -> Result<NativeHandle, AdapterError> {
-        let n = self.resolve_calls.fetch_add(1, Ordering::SeqCst) + 1;
-        if n <= self.fail_until {
-            return Err(AdapterError::new(ErrorCode::StaleRef, "not yet resolvable"));
-        }
-        Ok(NativeHandle::null())
+        self.retry.attempt()
     }
 }
 
@@ -95,7 +89,7 @@ fn transient_stale_ref_retries_then_succeeds_when_timeout_wired() {
     .unwrap();
 
     assert_eq!(value["action"], "set_value");
-    assert!(adapter.resolve_calls.load(Ordering::SeqCst) >= 3);
+    assert!(adapter.retry.calls() >= 3);
 }
 
 #[test]
@@ -117,5 +111,5 @@ fn timeout_none_makes_exactly_one_resolve_attempt() {
     .unwrap_err();
 
     assert_eq!(err.code(), "STALE_REF");
-    assert_eq!(adapter.resolve_calls.load(Ordering::SeqCst), 1);
+    assert_eq!(adapter.retry.calls(), 1);
 }

crates/core/src/commands/stale_retry_test_support.rs

@@ -0,0 +1,31 @@
+use crate::{
+    adapter::NativeHandle,
+    error::{AdapterError, ErrorCode},
+};
+use std::sync::atomic::{AtomicU32, Ordering};
+
+pub(crate) struct StaleRetryCounter {
+    calls: AtomicU32,
+    fail_until: u32,
+}
+
+impl StaleRetryCounter {
+    pub(crate) fn new(fail_until: u32) -> Self {
+        Self {
+            calls: AtomicU32::new(0),
+            fail_until,
+        }
+    }
+
+    pub(crate) fn attempt(&self) -> Result<NativeHandle, AdapterError> {
+        let n = self.calls.fetch_add(1, Ordering::SeqCst) + 1;
+        if n <= self.fail_until {
+            return Err(AdapterError::new(ErrorCode::StaleRef, "not yet resolvable"));
+        }
+        Ok(NativeHandle::null())
+    }
+
+    pub(crate) fn calls(&self) -> u32 {
+        self.calls.load(Ordering::SeqCst)
+    }
+}

crates/core/src/commands/type_text_tests.rs

@@ -4,34 +4,28 @@ use crate::{
     action_request::ActionRequest,
     action_result::ActionResult,
     adapter::NativeHandle,
-    error::{AdapterError, ErrorCode},
+    commands::stale_retry_test_support::StaleRetryCounter,
+    error::AdapterError,
     refs::{RefEntry, RefMap},
     refs_store::RefStore,
     refs_test_support::HomeGuard,
 };
-use std::sync::atomic::{AtomicU32, Ordering};
 
 struct StaleThenOkAdapter {
-    resolve_calls: AtomicU32,
-    fail_until: u32,
+    retry: StaleRetryCounter,
 }
 
 impl StaleThenOkAdapter {
     fn new(fail_until: u32) -> Self {
         Self {
-            resolve_calls: AtomicU32::new(0),
-            fail_until,
+            retry: StaleRetryCounter::new(fail_until),
         }
     }
 }
 
 impl ObservationOps for StaleThenOkAdapter {
     fn resolve_element_strict(&self, _entry: &RefEntry) -> Result<NativeHandle, AdapterError> {
-        let n = self.resolve_calls.fetch_add(1, Ordering::SeqCst) + 1;
-        if n <= self.fail_until {
-            return Err(AdapterError::new(ErrorCode::StaleRef, "not yet resolvable"));
-        }
-        Ok(NativeHandle::null())
+        self.retry.attempt()
     }
 }
 
@@ -96,7 +90,7 @@ fn transient_stale_ref_retries_then_succeeds_when_timeout_wired() {
     .unwrap();
 
     assert_eq!(value["action"], "type_text");
-    assert!(adapter.resolve_calls.load(Ordering::SeqCst) >= 3);
+    assert!(adapter.retry.calls() >= 3);
 }
 
 #[test]
@@ -118,5 +112,5 @@ fn timeout_none_makes_exactly_one_resolve_attempt() {
     .unwrap_err();
 
     assert_eq!(err.code(), "STALE_REF");
-    assert_eq!(adapter.resolve_calls.load(Ordering::SeqCst), 1);
+    assert_eq!(adapter.retry.calls(), 1);
 }

crates/core/src/ref_action_wait.rs

@@ -249,6 +249,7 @@ fn is_permanent_error(code: &ErrorCode) -> bool {
             | ErrorCode::InvalidArgs
             | ErrorCode::PolicyDenied
             | ErrorCode::AppUnresponsive
+            | ErrorCode::Internal
     )
 }
 
@@ -267,11 +268,17 @@ fn maybe_scroll_into_view(
     if !request.action.requires_scroll_into_view() {
         return;
     }
-    let needs_scroll = crate::state::has_state(&ctx.entry.states, crate::state::OFFSCREEN)
-        || ctx
-            .entry
-            .bounds
-            .is_none_or(|bounds| bounds.width <= 0.0 || bounds.height <= 0.0);
+    let live_states = crate::adapter::optional_live_read(ctx.adapter.get_live_state(handle))
+        .ok()
+        .flatten()
+        .map(|state| state.states);
+    let states = live_states.as_ref().unwrap_or(&ctx.entry.states);
+    let live_bounds = crate::adapter::optional_live_read(ctx.adapter.get_element_bounds(handle))
+        .ok()
+        .flatten();
+    let bounds = live_bounds.or(ctx.entry.bounds);
+    let needs_scroll = crate::state::has_state(states, crate::state::OFFSCREEN)
+        || bounds.is_none_or(|bounds| bounds.width <= 0.0 || bounds.height <= 0.0);
     if !needs_scroll {
         return;
     }

crates/core/src/step_mechanism.rs

@@ -6,3 +6,37 @@ pub enum StepMechanism {
     SemanticApi,
     PhysicalSynthetic,
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use serde_json::json;
+
+    #[test]
+    fn semantic_api_serializes_to_snake_case() {
+        assert_eq!(
+            serde_json::to_value(StepMechanism::SemanticApi).unwrap(),
+            json!("semantic_api")
+        );
+    }
+
+    #[test]
+    fn physical_synthetic_serializes_to_snake_case() {
+        assert_eq!(
+            serde_json::to_value(StepMechanism::PhysicalSynthetic).unwrap(),
+            json!("physical_synthetic")
+        );
+    }
+
+    #[test]
+    fn semantic_api_round_trips_from_value() {
+        let value: StepMechanism = serde_json::from_value(json!("semantic_api")).unwrap();
+        assert_eq!(value, StepMechanism::SemanticApi);
+    }
+
+    #[test]
+    fn physical_synthetic_round_trips_from_value() {
+        let value: StepMechanism = serde_json::from_value(json!("physical_synthetic")).unwrap();
+        assert_eq!(value, StepMechanism::PhysicalSynthetic);
+    }
+}

crates/ffi/cbindgen.toml

@@ -81,6 +81,28 @@ _Static_assert(offsetof(AdActionResult, steps) == 24, "AdActionResult.steps offs
 _Static_assert(offsetof(AdActionResult, step_count) == 32, "AdActionResult.step_count offset changed");
 _Static_assert(sizeof(AdRefEntry) == AD_REF_ENTRY_SIZE, "AdRefEntry ABI size changed");
 _Static_assert(_Alignof(AdRefEntry) == 8, "AdRefEntry ABI alignment changed");
+_Static_assert(offsetof(AdRefEntry, pid) == 0, "AdRefEntry.pid offset changed");
+_Static_assert(offsetof(AdRefEntry, role) == 8, "AdRefEntry.role offset changed");
+_Static_assert(offsetof(AdRefEntry, name) == 16, "AdRefEntry.name offset changed");
+_Static_assert(offsetof(AdRefEntry, value) == 24, "AdRefEntry.value offset changed");
+_Static_assert(offsetof(AdRefEntry, description) == 32, "AdRefEntry.description offset changed");
+_Static_assert(offsetof(AdRefEntry, states) == 40, "AdRefEntry.states offset changed");
+_Static_assert(offsetof(AdRefEntry, state_count) == 48, "AdRefEntry.state_count offset changed");
+_Static_assert(offsetof(AdRefEntry, available_actions) == 56, "AdRefEntry.available_actions offset changed");
+_Static_assert(offsetof(AdRefEntry, available_action_count) == 64, "AdRefEntry.available_action_count offset changed");
+_Static_assert(offsetof(AdRefEntry, bounds) == 72, "AdRefEntry.bounds offset changed");
+_Static_assert(offsetof(AdRefEntry, has_bounds) == 104, "AdRefEntry.has_bounds offset changed");
+_Static_assert(offsetof(AdRefEntry, bounds_hash) == 112, "AdRefEntry.bounds_hash offset changed");
+_Static_assert(offsetof(AdRefEntry, has_bounds_hash) == 120, "AdRefEntry.has_bounds_hash offset changed");
+_Static_assert(offsetof(AdRefEntry, source_app) == 128, "AdRefEntry.source_app offset changed");
+_Static_assert(offsetof(AdRefEntry, source_window_id) == 136, "AdRefEntry.source_window_id offset changed");
+_Static_assert(offsetof(AdRefEntry, source_window_title) == 144, "AdRefEntry.source_window_title offset changed");
+_Static_assert(offsetof(AdRefEntry, source_surface) == 152, "AdRefEntry.source_surface offset changed");
+_Static_assert(offsetof(AdRefEntry, root_ref) == 160, "AdRefEntry.root_ref offset changed");
+_Static_assert(offsetof(AdRefEntry, path_is_absolute) == 168, "AdRefEntry.path_is_absolute offset changed");
+_Static_assert(offsetof(AdRefEntry, path) == 176, "AdRefEntry.path offset changed");
+_Static_assert(offsetof(AdRefEntry, path_count) == 184, "AdRefEntry.path_count offset changed");
+_Static_assert(offsetof(AdRefEntry, native_id) == 192, "AdRefEntry.native_id offset changed");
 _Static_assert(sizeof(struct AdWaitArgs) == AD_WAIT_ARGS_SIZE, "AdWaitArgs ABI size drift");
 _Static_assert(_Alignof(struct AdWaitArgs) == 8, "AdWaitArgs ABI alignment changed");
 #endif /* __STDC_VERSION__ >= 201112L */

crates/ffi/codegen_templates/execute_by_ref.rs.in

@@ -25,6 +25,11 @@
 /// accepts the action's own CLI base (so `TypeText` still uses
 /// `focus_fallback`). `Headed (2)` opts in to cursor-based fallbacks.
 ///
+/// Uses a fixed 5000ms auto-wait budget (`DEFAULT_ACTION_TIMEOUT_MS`) before
+/// the actionability preflight, matching the CLI default. Call
+/// `ad_execute_by_ref_timeout` with an explicit `timeout_ms` (0 = single-shot,
+/// no auto-wait) to control this.
+///
 /// On success `*out` is set to a NUL-terminated JSON envelope (command
 /// `"execute_by_ref"`); free with `ad_free_string`. On guard or decode
 /// failure (invalid args before the command runs) `*out` remains null.

crates/ffi/codegen_templates/snapshot.rs.in

@@ -4,7 +4,7 @@
 /// to disk, and writes the JSON envelope into `*out`.
 ///
 /// The JSON shape matches `agent-desktop snapshot`:
-/// `{"version":"2.0","ok":true,"command":"snapshot","data":{"app":"...","window":{...},"ref_count":N,"snapshot_id":"...","tree":{...}}}`.
+/// `{"version":"2.1","ok":true,"command":"snapshot","data":{"app":"...","window":{...},"ref_count":N,"snapshot_id":"...","tree":{...}}}`.
 ///
 /// **`*out` ownership and error behaviour:**
 /// - On success (`AD_RESULT_OK`): `*out` is a heap-allocated JSON string with `"ok":true`.

crates/ffi/include/agent_desktop.h

@@ -983,6 +983,11 @@ void ad_app_list_free(struct AdAppList *list);
  * accepts the action's own CLI base (so `TypeText` still uses
  * `focus_fallback`). `Headed (2)` opts in to cursor-based fallbacks.
  *
+ * Uses a fixed 5000ms auto-wait budget (`DEFAULT_ACTION_TIMEOUT_MS`) before
+ * the actionability preflight, matching the CLI default. Call
+ * `ad_execute_by_ref_timeout` with an explicit `timeout_ms` (0 = single-shot,
+ * no auto-wait) to control this.
+ *
  * On success `*out` is set to a NUL-terminated JSON envelope (command
  * `"execute_by_ref"`); free with `ad_free_string`. On guard or decode
  * failure (invalid args before the command runs) `*out` remains null.
@@ -1040,7 +1045,7 @@ AdResult ad_execute_by_ref_timeout(const struct AdAdapter *adapter,
  * to disk, and writes the JSON envelope into `*out`.
  *
  * The JSON shape matches `agent-desktop snapshot`:
- * `{"version":"2.0","ok":true,"command":"snapshot","data":{"app":"...","window":{...},"ref_count":N,"snapshot_id":"...","tree":{...}}}`.
+ * `{"version":"2.1","ok":true,"command":"snapshot","data":{"app":"...","window":{...},"ref_count":N,"snapshot_id":"...","tree":{...}}}`.
  *
  * **`*out` ownership and error behaviour:**
  * - On success (`AD_RESULT_OK`): `*out` is a heap-allocated JSON string with `"ok":true`.
@@ -1745,7 +1750,7 @@ void ad_free_tree(struct AdNodeTree *tree);
  * **Observe–act agents** that need `@e` refs and refmap persistence should
  * call `ad_snapshot` instead. `ad_snapshot` runs the full snapshot pipeline
  * (ref allocation, refmap write to disk, JSON envelope with
- * `{"version":"2.0","ok":true,...}`) and is the correct starting point for
+ * `{"version":"2.1","ok":true,...}`) and is the correct starting point for
  * any workflow that drives subsequent ref-based actions via
  * `ad_execute_by_ref` (with an `AdAction`).
  *
@@ -1897,6 +1902,28 @@ _Static_assert(offsetof(AdActionResult, steps) == 24, "AdActionResult.steps offs
 _Static_assert(offsetof(AdActionResult, step_count) == 32, "AdActionResult.step_count offset changed");
 _Static_assert(sizeof(AdRefEntry) == AD_REF_ENTRY_SIZE, "AdRefEntry ABI size changed");
 _Static_assert(_Alignof(AdRefEntry) == 8, "AdRefEntry ABI alignment changed");
+_Static_assert(offsetof(AdRefEntry, pid) == 0, "AdRefEntry.pid offset changed");
+_Static_assert(offsetof(AdRefEntry, role) == 8, "AdRefEntry.role offset changed");
+_Static_assert(offsetof(AdRefEntry, name) == 16, "AdRefEntry.name offset changed");
+_Static_assert(offsetof(AdRefEntry, value) == 24, "AdRefEntry.value offset changed");
+_Static_assert(offsetof(AdRefEntry, description) == 32, "AdRefEntry.description offset changed");
+_Static_assert(offsetof(AdRefEntry, states) == 40, "AdRefEntry.states offset changed");
+_Static_assert(offsetof(AdRefEntry, state_count) == 48, "AdRefEntry.state_count offset changed");
+_Static_assert(offsetof(AdRefEntry, available_actions) == 56, "AdRefEntry.available_actions offset changed");
+_Static_assert(offsetof(AdRefEntry, available_action_count) == 64, "AdRefEntry.available_action_count offset changed");
+_Static_assert(offsetof(AdRefEntry, bounds) == 72, "AdRefEntry.bounds offset changed");
+_Static_assert(offsetof(AdRefEntry, has_bounds) == 104, "AdRefEntry.has_bounds offset changed");
+_Static_assert(offsetof(AdRefEntry, bounds_hash) == 112, "AdRefEntry.bounds_hash offset changed");
+_Static_assert(offsetof(AdRefEntry, has_bounds_hash) == 120, "AdRefEntry.has_bounds_hash offset changed");
+_Static_assert(offsetof(AdRefEntry, source_app) == 128, "AdRefEntry.source_app offset changed");
+_Static_assert(offsetof(AdRefEntry, source_window_id) == 136, "AdRefEntry.source_window_id offset changed");
+_Static_assert(offsetof(AdRefEntry, source_window_title) == 144, "AdRefEntry.source_window_title offset changed");
+_Static_assert(offsetof(AdRefEntry, source_surface) == 152, "AdRefEntry.source_surface offset changed");
+_Static_assert(offsetof(AdRefEntry, root_ref) == 160, "AdRefEntry.root_ref offset changed");
+_Static_assert(offsetof(AdRefEntry, path_is_absolute) == 168, "AdRefEntry.path_is_absolute offset changed");
+_Static_assert(offsetof(AdRefEntry, path) == 176, "AdRefEntry.path offset changed");
+_Static_assert(offsetof(AdRefEntry, path_count) == 184, "AdRefEntry.path_count offset changed");
+_Static_assert(offsetof(AdRefEntry, native_id) == 192, "AdRefEntry.native_id offset changed");
 _Static_assert(sizeof(struct AdWaitArgs) == AD_WAIT_ARGS_SIZE, "AdWaitArgs ABI size drift");
 _Static_assert(_Alignof(struct AdWaitArgs) == 8, "AdWaitArgs ABI alignment changed");
 #endif /* __STDC_VERSION__ >= 201112L */

crates/ffi/src/commands/generated.rs

@@ -50,6 +50,11 @@ use std::ptr;
 /// accepts the action's own CLI base (so `TypeText` still uses
 /// `focus_fallback`). `Headed (2)` opts in to cursor-based fallbacks.
 ///
+/// Uses a fixed 5000ms auto-wait budget (`DEFAULT_ACTION_TIMEOUT_MS`) before
+/// the actionability preflight, matching the CLI default. Call
+/// `ad_execute_by_ref_timeout` with an explicit `timeout_ms` (0 = single-shot,
+/// no auto-wait) to control this.
+///
 /// On success `*out` is set to a NUL-terminated JSON envelope (command
 /// `"execute_by_ref"`); free with `ad_free_string`. On guard or decode
 /// failure (invalid args before the command runs) `*out` remains null.
@@ -268,7 +273,7 @@ pub unsafe extern "C" fn ad_execute_by_ref_timeout(
 /// to disk, and writes the JSON envelope into `*out`.
 ///
 /// The JSON shape matches `agent-desktop snapshot`:
-/// `{"version":"2.0","ok":true,"command":"snapshot","data":{"app":"...","window":{...},"ref_count":N,"snapshot_id":"...","tree":{...}}}`.
+/// `{"version":"2.1","ok":true,"command":"snapshot","data":{"app":"...","window":{...},"ref_count":N,"snapshot_id":"...","tree":{...}}}`.
 ///
 /// **`*out` ownership and error behaviour:**
 /// - On success (`AD_RESULT_OK`): `*out` is a heap-allocated JSON string with `"ok":true`.

crates/ffi/src/tree/get.rs

@@ -36,7 +36,7 @@ use std::ptr;
 /// **Observe–act agents** that need `@e` refs and refmap persistence should
 /// call `ad_snapshot` instead. `ad_snapshot` runs the full snapshot pipeline
 /// (ref allocation, refmap write to disk, JSON envelope with
-/// `{"version":"2.0","ok":true,...}`) and is the correct starting point for
+/// `{"version":"2.1","ok":true,...}`) and is the correct starting point for
 /// any workflow that drives subsequent ref-based actions via
 /// `ad_execute_by_ref` (with an `AdAction`).
 ///

skills/agent-desktop/SKILL.md

@@ -93,7 +93,7 @@ Use **progressive skeleton traversal** as the default approach. It reduces token
 - After any action that changes UI, re-drill the affected region or re-snapshot
 - **Scoped invalidation:** re-drilling `--root @e3` only replaces refs from @e3's previous drill — refs from other regions and the skeleton itself are preserved
 - **Strict resolution:** stale refs return `STALE_REF`; duplicate plausible targets return `AMBIGUOUS_TARGET` instead of choosing arbitrarily.
-- **Actionability:** ref actions check live visibility, stability, enabled state, supported action, policy, and editability before dispatch.
+- **Actionability:** dispatch ref actions (`click`, `type`, `set-value`, `toggle`, etc.) check live visibility, stability, enabled state, supported action, policy, and editability before dispatch — and the four click variants also run a hit-test occlusion check — polling until actionable or `TIMEOUT`. `hover`/`drag` instead resolve a target point and run **only** the hit-test occlusion (`receives_events`) check, failing fast with `ACTION_FAILED` if occluded.
 - **Headless vs headed:** ref actions are headless by default (AX-only, no cursor) and fail closed when only a physical gesture would work. `type` uses a focus-fallback base policy because typing needs focus but never moves the cursor. Pass the global `--headed` flag to permit cursor movement and focus stealing so physical fallbacks can complete; the AX path is still tried first, so `--headed` never regresses headless-capable elements. Raw cursor commands (`hover`, `drag`, `mouse-*`) are physical and require `--headed`; keyboard commands (`press`, `key-down`, `key-up`) are explicit low-level input.
 - **Sessions and tracing:** run `session start` once per agent run to create a manifest with `trace: on` (default). Use `session start --screenshots` when you need replay artifacts (`artifacts: full`): pre/post-action PNGs and refmap copies under the session trace directory (sensitive — treat exports like screenshots). Subsequent commands record JSONL automatically to per-process segments under `~/.agent-desktop/sessions/<id>/trace/<pid>-<procTs>.jsonl` — no `--trace` on every call. Read traces back with `trace show` (bounded JSON for agents) or `trace export` (single-file HTML for humans). A session owns both its trace and its latest-snapshot namespace; activating it (via pointer, env, or flag) relocates implicit "latest" to that session. Explicit `--snapshot <id>` still resolves cross-session. **`--session <id>` alone** (no manifest from `session start`) selects only the snapshot namespace — existing callers see no surprise trace files. **`--trace <path>`** still overrides to one atomic file for CI or one-offs. Activation precedence: `--session` > `AGENT_DESKTOP_SESSION` > `~/.agent-desktop/current_session` (written only by `session start`). Concurrent independent agents set `AGENT_DESKTOP_SESSION` per process; the pointer is a single-active-session convenience. Multi-agent shared sessions: each agent acts on the `snapshot_id` from its own `snapshot` call — implicit latest is not a cross-agent guarantee. Run `status` to see `session_id` and `tracing`. Trace lines include `ts_ms`, monotonic per-process `seq`, and redacted sensitive fields (`text`, `value`, `expected`, `name`, `username`, `description`, `label`, `query`, `secret`, `token`, `password`, `title`, `url`, `help`, `placeholder` → `{ "redacted": true }`). `--trace-strict` fails on trace setup and pre-action writes; post-action success traces are best-effort.
 
@@ -106,6 +106,8 @@ Every command returns a JSON envelope on stdout:
 
 The `error` object may also carry an optional `details` object (e.g. the actionability report on an actionability failure, candidate summaries on `AMBIGUOUS_TARGET`, or the last observed state on a `wait` `TIMEOUT`). Parse errors leniently — `details` and future fields are additive, so do not reject responses with unknown keys.
 
+An actionability failure on a hit-test action (`click`, `double-click`, `right-click`, `triple-click`, `hover`, `drag`) can carry a `receives_events` check with `reason: "occluded by <role>"` plus a structured `occluder: { "role", "name", "bounds" }` — another element is on top of the target. Bring the target window/element to the front (or dismiss the occluder) rather than blind-retrying; see `references/commands-interaction.md` for the full check list.
+
 Exit codes: `0` success, `1` structured error, `2` argument error.
 
 ### Error Codes

skills/agent-desktop/references/commands-interaction.md

@@ -59,7 +59,13 @@ All ref-based interaction commands accept `--snapshot <snapshot_id>`. Omit it fo
 
 Success responses for ref actions include a `steps` array when the activation chain recorded attempts: each entry is `{ "label": "AXPress", "outcome": "attempted" | "skipped" | "succeeded" }` in execution order, showing which activation path produced the result.
 
-When the actionability preflight blocks an action, the error envelope carries the full report in `error.details`: `{ "actionable": false, "checks": [ { "name": "...", "status": "...", "reason": "..." } ] }`. Check names are `visible`, `stable`, `enabled`, `supported_action`, `policy`, and `editable`; statuses are `pass`, `fail`, and `unknown`. Use the failing check's `reason` to pick recovery: `wait --element <ref> --predicate actionable`, a fresh snapshot, or `--headed` when a `policy` check failed and a physical gesture is intended.
+When the actionability preflight blocks an action, the error envelope carries the full report in `error.details`: `{ "actionable": false, "checks": [ { "name": "...", "status": "...", "reason": "..." } ] }`. Check names are `visible`, `stable`, `enabled`, `supported_action`, `policy`, `editable`, and `receives_events`; statuses are `pass`, `fail`, and `unknown`. The dispatch actions that activate an element (`click`, `double-click`, `right-click`, `triple-click`, `type`, `set-value`, `select`, `toggle`, `check`, `uncheck`, `expand`, `collapse`, `clear`, `focus`, `scroll`, `scroll-to`) run the `visible`/`stable`/`enabled`/`supported_action`/`policy`/`editable` battery; the four hit-test variants (`click`, `double-click`, `right-click`, `triple-click`) additionally run `receives_events`. `hover` and `drag` are different — they resolve a target point and run **only** the `receives_events` occlusion check, so their report's `checks` array holds that single entry (no `visible`/`enabled`/etc.). Use the failing check's `reason` to pick recovery: `wait --element <ref> --predicate actionable`, a fresh snapshot, or `--headed` when a `policy` check failed and a physical gesture is intended.
+
+**`receives_events` failures.** When a hit test at the target's center point lands on a different element, `receives_events` fails with `reason: "occluded by <role>"` and a structured `occluder` object on that check: `{ "role", "name", "bounds" }` (the element that actually received the hit, when it can be identified). The target's own bounds have not changed — something else is now on top of them. Recovery is to bring the target's window or element to the front (or dismiss whatever is covering it), then retry; blind-retrying without changing z-order will fail the same way again.
+
+Every ref-resolving action accepts `--timeout-ms` (default `5000`), but it budgets different things. For the dispatch actions (`click`, `double-click`, `triple-click`, `right-click`, `clear`, `focus`, `toggle`, `check`, `uncheck`, `expand`, `collapse`, `scroll-to`, `type`, `set-value`, `select`, `scroll`) it is the actionability-wait budget: they poll roughly every 100ms until the target becomes actionable, then fail with `TIMEOUT` once the budget is exhausted. For `hover` and `drag` it budgets the ref-*resolution* retry instead — only `STALE_REF`/`AMBIGUOUS_TARGET`/`TIMEOUT` are retried within the budget; a `receives_events` occlusion failure on hover/drag is returned immediately as `ACTION_FAILED`, not polled to `TIMEOUT`.
+
+**Implicit scroll-into-view.** Before acting, every ref action other than `scroll`, `scroll-to`, `hover`, and `drag` automatically attempts to bring an offscreen or zero-bounds target into view (macOS: `AXScrollToVisible`) before dispatching the action. This is best-effort and silent — it has no separate error code, and a failed attempt does not block the action itself, it just proceeds without having scrolled. Use the standalone `scroll-to` command when you need an explicit, verifiable scroll instead of relying on this implicit step.
 
 ## Click Actions
 
@@ -174,6 +180,7 @@ agent-desktop scroll @e1 --direction right --amount 2
 |------|---------|-------------|
 | `--direction` | down | `up`, `down`, `left`, `right` |
 | `--amount` | 3 | Number of scroll units |
+| `--timeout-ms` | 5000 | Actionability wait budget in ms before failing with `TIMEOUT` |
 
 Uses AX scroll actions, scroll bars, and state-setting paths. If those are unavailable, the command returns a structured error instead of stealing focus or sending wheel events.
 
@@ -246,6 +253,7 @@ agent-desktop --headed drag --from @e1 --to @e5 --drop-delay 800
 | `--to-xy` | Destination coordinates as `x,y` |
 | `--duration` | Drag duration in milliseconds (movement from source to destination) |
 | `--drop-delay` | Milliseconds to hold over the destination before releasing; default 500 |
+| `--timeout-ms` | Actionability wait budget in ms before failing with `TIMEOUT`; default 5000 |
 
 Can mix ref and coordinate sources (e.g., `--from @e1 --to-xy 400,500`).
 
@@ -271,6 +279,7 @@ agent-desktop --headed mouse-click --xy 500,300 --count 2
 | `--xy` | (required) | Coordinates as `x,y` |
 | `--button` | left | `left`, `right`, `middle` |
 | `--count` | 1 | Number of clicks |
+| `--modifiers` | | Held modifiers: `shift`, `cmd`, `ctrl`, `alt` (repeatable); held during the click |
 
 ### mouse-down / mouse-up
 ```bash
@@ -283,6 +292,7 @@ Low-level press/release for custom drag or hold interactions.
 |------|---------|-------------|
 | `--xy` | (required) | Coordinates as `x,y` |
 | `--button` | left | `left`, `right`, `middle` |
+| `--modifiers` | | Held modifiers: `shift`, `cmd`, `ctrl`, `alt` (repeatable); held during the mouse event |
 
 ### mouse-wheel
 ```bash