Merge pull request #3846 from bobleer/bob/fix-computer-use-web-text-focus

fix(computer-use): give web text fields real keyboard focus on macOS

Author: bobleer

src/crates/execution/tool-contracts/src/computer_use_geometry.rs

@@ -129,7 +129,7 @@ impl CaptureGeometry {
             && y.checked_add(height)
                 .is_some_and(|bottom| bottom <= top + content_height);
         if !inside {
-            return Err("[INVALID_COORDINATES] Region must be a nonempty rectangle inside the captured image content".into());
+            return Err(format!("[INVALID_COORDINATES] Region must be a nonempty rectangle inside the captured image content: requested x={x} y={y} width={width} height={height}; content is x={left} y={top} width={content_width} height={content_height}"));
         }
         let scale_x = self.bounds[2] / f64::from(content_width);
         let scale_y = self.bounds[3] / f64::from(content_height);

src/crates/services/computer-use/AGENTS.md

@@ -150,6 +150,18 @@ does nothing (WebKit). Select the existing directed-keyboard route before
 submission for these proxies; native controls retain AX insertion at the current
 selection. Never retry a submitted text write based only on unchanged AX state.
 
+Text controls inside an `AXWebArea` need a real directed click to become the
+keyboard destination. AXPress and AXFocused succeed there but move only DOM
+focus: the window's first responder stays elsewhere and later directed keys are
+lost while still reporting `submitted`. Route them through the pointer path and
+directed keyboard, without app-name rules. Verified on Electron; native controls
+keep the AX path. A field scrolled out of view is an error; web
+content covering the center cannot be detected, because Chromium's synchronous
+AX hit test ignores stacking. A later focus request on a field that a real click
+already focused sends no click, which preserves its caret and selection; other
+pointer or semantic input, shortcuts that can move focus and focus-loss events
+end that state.
+
 The directed-input route can change a target application's internal
 focus without changing the human foreground application. Its window-addressed
 focus pair at local `(-8,-8)` must remain outside content; the requested gesture

src/crates/services/computer-use/src/computer_use/desktop_host/ax_orchestration.rs

@@ -166,7 +166,8 @@ fn resolve_macos_node_click(
     let target = crate::computer_use::macos_ax_dump::retained_cached_target(pid, idx)
         .ok_or_else(|| OpenBitFunError::tool(format!("AX_NODE_STALE: idx={idx} is not in the observed app snapshot; observe the app again")))?;
     crate::computer_use::macos_ax_dump::validate_bound_target(pid, target.reference())?;
-    if semantic {
+    let web_text_field = semantic && target.is_web_text_input();
+    if semantic && !web_text_field {
         match crate::computer_use::macos_ax_write::try_ax_press(target.reference()) {
             crate::computer_use::macos_ax_write::AxWriteOutcome::Ok => return Ok(None),
             // kAXErrorActionUnsupported guarantees no semantic action was sent.
@@ -181,9 +182,61 @@ fn resolve_macos_node_click(
             "AX_NODE_STALE: idx={idx} no longer has a readable frame; observe the app again"
         ))
     })?;
+    if web_text_field {
+        require_visible_web_text_field(pid, idx, frame)?;
+    }
     observed_node_center(frame).map(Some)
 }
 
+/// A web text field takes a real click instead of AXPress, and that click
+/// needs a visible point. Chromium clips a scrolled-away node's frame to the
+/// viewport edge, leaving under one pixel. Web content can still cover the
+/// point: its synchronous AX hit test ignores stacking, so it cannot tell.
+#[cfg(target_os = "macos")]
+fn require_visible_web_text_field(
+    pid: i32,
+    idx: u32,
+    (field_left, field_top, field_width, field_height): (f64, f64, f64, f64),
+) -> OpenBitFunResult<()> {
+    let (x, y) = (
+        field_left + field_width / 2.0,
+        field_top + field_height / 2.0,
+    );
+    let window =
+        crate::computer_use::macos_capture::bound_window_id(pid).map_err(OpenBitFunError::tool)?;
+    let [left, top, width, height] = crate::computer_use::macos_capture::window_bounds(pid, window)
+        .map_err(OpenBitFunError::tool)?;
+    if field_width < 1.0
+        || field_height < 1.0
+        || x < left
+        || y < top
+        || x >= left + width
+        || y >= top + height
+    {
+        return Err(OpenBitFunError::tool(format!("TARGET_COORDINATES_OUTSIDE_WINDOW: Text field idx={idx} is not visible in the window; no click was sent. Scroll it into view and observe again")));
+    }
+    Ok(())
+}
+
+/// Remember the field a real click just focused, so a later focus request on
+/// it does not click again and lose the caret or selection.
+#[cfg(target_os = "macos")]
+fn remember_web_text_click(pid: i32, target: &ClickTarget, x: f64, y: f64) -> OpenBitFunResult<()> {
+    let element = match target {
+        ClickTarget::NodeIdx { idx } => {
+            crate::computer_use::macos_ax_dump::retained_cached_target(pid, *idx)
+        }
+        ClickTarget::ImageXy { .. } => {
+            crate::computer_use::macos_ax_dump::retained_target_at_point(pid, x, y)?
+        }
+        _ => None,
+    };
+    if let Some(element) = element.filter(|element| element.is_web_text_input()) {
+        crate::computer_use::macos_input_focus::note_web_text_click(pid, element);
+    }
+    Ok(())
+}
+
 #[cfg(any(test, target_os = "macos"))]
 fn observed_node_center(
     (x, y, width, height): (f64, f64, f64, f64),
@@ -548,6 +601,18 @@ impl NativeComputerUseHost {
                 })
                 .await
                 .map_err(|e| OpenBitFunError::tool(e.to_string()))??;
+                if is_plain_activation_click(
+                    &params.mouse_button,
+                    params.click_count,
+                    &params.modifier_keys,
+                ) {
+                    let target = params.target.clone();
+                    // Read-only bookkeeping: without it the next focus request clicks again.
+                    let _ = crate::computer_use::control_session::spawn_blocking(move || {
+                        macos::catch_objc_local(|| remember_web_text_click(pid, &target, x, y))
+                    })
+                    .await;
+                }
             }
             let settle_ms = params.wait_ms_after.unwrap_or(0);
             if settle_ms > 0 {
@@ -679,6 +744,11 @@ impl NativeComputerUseHost {
             } else { None };
             let Some(element) = element else { return Ok(false); };
             crate::computer_use::macos_ax_dump::validate_bound_target(pid, element.reference())?;
+            if element.is_web_text_input() {
+                // Text goes through the directed keyboard. Focus needs a real
+                // click unless an earlier click already gave this field focus.
+                return Ok(insertion.is_none() && crate::computer_use::macos_input_focus::holds_web_text_focus(pid, &element));
+            }
             if !element.is_text_input() { return Ok(false); }
             if let Some(text) = insertion.as_deref() {
                 return crate::computer_use::macos_ax_write::insert_selected_text(element.reference(), text);

src/crates/services/computer-use/src/computer_use/desktop_host/observation.rs

@@ -1,6 +1,8 @@
 //! Observation lifecycle shared by AX, UIA and AT-SPI providers.
 use super::*;
 
+const UNPAINTED_WINDOW_NOTE: &str = "[note] WINDOW_NOT_REPAINTED: The window image is identical to the previous observation although the accessibility tree changed. The application may not have repainted this window, which is common for covered Chromium and Electron windows, or the change may not be visible. Use the tree for the current state; the image does not show whether the input took effect.\n";
+
 impl NativeComputerUseHost {
     pub(crate) async fn get_app_state_inner(
         &self,
@@ -28,6 +30,21 @@ impl NativeComputerUseHost {
                 .record_application(pid)
                 .map_err(OpenBitFunError::tool)?,
         );
+        if let Some(shot) = snapshot.screenshot.as_ref() {
+            let pixels = {
+                use std::hash::{Hash, Hasher};
+                let mut hasher = std::collections::hash_map::DefaultHasher::new();
+                shot.bytes.hash(&mut hasher);
+                hasher.finish()
+            };
+            if state
+                .observations
+                .record_frame(pid, pixels, &snapshot.digest)
+            {
+                // First, because long trees are clipped at the end for the model.
+                snapshot.tree_text.insert_str(0, UNPAINTED_WINDOW_NOTE);
+            }
+        }
         // Native node indices were rebuilt, including same-content observations.
         #[cfg(any(target_os = "macos", target_os = "windows"))]
         {

src/crates/services/computer-use/src/computer_use/desktop_host/pointer_input.rs

@@ -718,6 +718,9 @@ impl NativeComputerUseHost {
     }
 
     pub(super) async fn mouse_click_impl(&self, button: &str) -> OpenBitFunResult<()> {
+        // A global press can move focus away from a field clicked earlier.
+        #[cfg(target_os = "macos")]
+        crate::computer_use::macos_input_focus::forget_web_text_focus();
         debug!("computer_use: mouse_click button={}", button);
         ComputerUseHost::computer_use_guard_click_allowed(self)?;
         self.mouse_click_at_current_pointer(button).await
@@ -732,6 +735,9 @@ impl NativeComputerUseHost {
     }
 
     pub(super) async fn mouse_down_impl(&self, button: &str) -> OpenBitFunResult<()> {
+        // A global press can move focus away from a field clicked earlier.
+        #[cfg(target_os = "macos")]
+        crate::computer_use::macos_input_focus::forget_web_text_focus();
         #[cfg(target_os = "linux")]
         if let Some(session) =
             crate::computer_use::linux_control::session().map_err(OpenBitFunError::tool)?

src/crates/services/computer-use/src/computer_use/macos_ax_dump.rs

@@ -180,6 +180,18 @@ impl RetainedCachedTarget {
         )
     }
 
+    /// A text control rendered inside a web area. AXPress and AXFocused move
+    /// only the page's DOM focus there; the window's first responder stays
+    /// put, so directed keys vanish while the call reports success.
+    pub(crate) fn is_web_text_input(&self) -> bool {
+        (self.is_text_input() || self.role().as_deref() == Some("AXComboBox"))
+            && unsafe { has_web_area_ancestor(self.0 .0) }
+    }
+
+    pub(crate) fn same_element(&self, other: AxRef) -> bool {
+        !other.0.is_null() && unsafe { CFEqual(self.0 .0 as CFTypeRef, other.0 as CFTypeRef) != 0 }
+    }
+
     /// Only controls whose primary activation is independent of the exact
     /// point can replace a plain coordinate click with AXPress. Text, sliders,
     /// containers and custom canvases retain their coordinate semantics.
@@ -236,6 +248,33 @@ pub(crate) fn validate_bound_target(pid: i32, target: AxRef) -> OpenBitFunResult
         .map_err(OpenBitFunError::tool)
 }
 
+const WEB_AREA_ANCESTOR_LIMIT: usize = 64;
+
+/// Whether a parent chain reaches a web area within the depth bound. An
+/// unreadable parent ends the chain and counts as native content.
+fn role_chain_reaches_web_area(mut next_role: impl FnMut() -> Option<String>) -> bool {
+    (0..WEB_AREA_ANCESTOR_LIMIT)
+        .map_while(|_| next_role())
+        .any(|role| role == "AXWebArea")
+}
+
+unsafe fn has_web_area_ancestor(element: AXUIElementRef) -> bool {
+    let mut current: Option<CFTypeRef> = None;
+    let found = role_chain_reaches_web_area(|| unsafe {
+        let child = current.map_or(element, |owned| owned as AXUIElementRef);
+        let parent = ax_copy_attr(child, "AXParent");
+        if let Some(owned) = current.take() {
+            ax_release(owned);
+        }
+        current = parent;
+        read_cf_string_attr(current? as AXUIElementRef, "AXRole")
+    });
+    if let Some(owned) = current {
+        unsafe { ax_release(owned) };
+    }
+    found
+}
+
 pub(crate) fn element_window_id(target: AxRef) -> Option<u32> {
     unsafe {
         super::macos_ax_ui::ax_window_id(target.0)
@@ -1141,7 +1180,7 @@ fn render_tree_text(nodes: &[AxNode]) -> String {
         }
         if let Some(v) = &n.value {
             if !v.is_empty() {
-                out.push_str(&format!(" value={}", quote_clip(v, 120)));
+                out.push_str(&format!(" value={}", quote_clip(v, value_clip_chars(n))));
             }
         }
         if let Some(id) = &n.identifier {
@@ -1178,16 +1217,21 @@ fn render_tree_text(nodes: &[AxNode]) -> String {
         }
         match n.expanded {
             Some(true) => out.push_str(" [expanded]"),
-            Some(false) => out.push_str(" [collapsed]"),
-            None => {}
+            Some(false) if expansion_is_meaningful(&n.role) => out.push_str(" [collapsed]"),
+            _ => {}
         }
         // Surface non-trivial AX actions inline so the model can pick
         // AXShowMenu / AXIncrement / AXDecrement etc. without re-querying.
         let extra: Vec<&str> = n
             .actions
             .iter()
             .map(String::as_str)
-            .filter(|a| !matches!(*a, "AXPress" | "AXShowAlternateUI" | "AXShowDefaultUI"))
+            .filter(|a| {
+                !matches!(
+                    *a,
+                    "AXPress" | "AXShowAlternateUI" | "AXShowDefaultUI" | "AXScrollToVisible"
+                )
+            })
             .collect();
         if !extra.is_empty() {
             out.push_str(&format!(" actions=[{}]", extra.join(",")));
@@ -1202,6 +1246,38 @@ fn render_tree_text(nodes: &[AxNode]) -> String {
     out
 }
 
+/// The caller verifies a draft against the focused field before sending it,
+/// so that one value keeps enough text to read to the end.
+fn value_clip_chars(node: &AxNode) -> usize {
+    const VALUE_CHARS: usize = 120;
+    const FOCUSED_TEXT_VALUE_CHARS: usize = 4000;
+    if node.focused
+        && matches!(
+            node.role.as_str(),
+            "AXTextField" | "AXTextArea" | "AXSearchField"
+        )
+    {
+        FOCUSED_TEXT_VALUE_CHARS
+    } else {
+        VALUE_CHARS
+    }
+}
+
+/// Chromium reports `AXExpanded = false` on nearly every web node. Rendering
+/// it for structural roles only adds noise; the digest still covers the state.
+fn expansion_is_meaningful(role: &str) -> bool {
+    !matches!(
+        role,
+        "AXGroup"
+            | "AXStaticText"
+            | "AXImage"
+            | "AXWebArea"
+            | "AXHeading"
+            | "AXTextField"
+            | "AXTextArea"
+    )
+}
+
 /// Compose a Codex-style label: prefer humanised role description, fall
 /// back to `role + (subrole)`.
 fn format_label(n: &AxNode) -> String {
@@ -1233,6 +1309,52 @@ fn quote_clip(s: &str, max: usize) -> String {
 mod tests {
     use super::*;
 
+    #[test]
+    fn web_area_ancestor_decides_the_text_focus_route() {
+        let walk = |roles: &[&str]| {
+            let mut roles = roles.iter().map(|role| role.to_string());
+            role_chain_reaches_web_area(|| roles.next())
+        };
+        assert!(walk(&["AXGroup", "AXGroup", "AXWebArea", "AXGroup"]));
+        assert!(!walk(&["AXGroup", "AXWindow", "AXApplication"]));
+        assert!(!walk(&[]));
+        let mut steps = 0;
+        assert!(!role_chain_reaches_web_area(|| {
+            steps += 1;
+            Some("AXGroup".to_string())
+        }));
+        assert_eq!(steps, WEB_AREA_ANCESTOR_LIMIT);
+    }
+
+    #[test]
+    fn only_the_focused_text_field_keeps_its_full_value() {
+        let text = "\u{4e2d}".repeat(300);
+        let node = |role: &str, focused: bool| AxNode {
+            role: role.to_string(),
+            focused,
+            value: Some(text.clone()),
+            ..n(0, None, role, None)
+        };
+        let rendered = |focused| render_tree_text(&[node("AXTextArea", focused)]);
+        assert!(rendered(true).contains(&format!("value=\"{text}\"")));
+        assert!(rendered(false).contains('\u{2026}'));
+        assert!(render_tree_text(&[node("AXStaticText", true)]).contains('\u{2026}'));
+    }
+
+    #[test]
+    fn structural_collapsed_state_and_scroll_action_are_not_rendered() {
+        let node = |role: &str| AxNode {
+            role: role.to_string(),
+            expanded: Some(false),
+            actions: vec!["AXShowMenu".to_string(), "AXScrollToVisible".to_string()],
+            ..n(0, None, role, None)
+        };
+        let group = render_tree_text(&[node("AXGroup")]);
+        assert!(!group.contains("[collapsed]"));
+        assert!(group.contains("actions=[AXShowMenu]"));
+        assert!(render_tree_text(&[node("AXComboBox")]).contains("[collapsed]"));
+    }
+
     #[test]
     fn optional_point_hit_read_failure_preserves_directed_input_route() {
         assert!(optional_point_hit_available(0).unwrap());

src/crates/services/computer-use/src/computer_use/macos_ax_write.rs

@@ -61,6 +61,8 @@ fn can_write(target: AxRef) -> bool {
 }
 
 fn note_dispatched_input(target: AxRef) {
+    // A semantic action can move DOM focus without any pointer input.
+    super::macos_input_focus::forget_web_text_focus();
     let mut pid = 0;
     if unsafe { AXUIElementGetPid(target.0, &mut pid) } == 0 {
         super::macos_capture::note_input(pid);