fix(computer-use): restore text-only observation and unblock the Enter path

src/apps/desktop/src/computer_use/macos_ax_dump.rs

@@ -431,6 +431,15 @@ pub(super) struct DumpOpts {
     pub max_depth: u32,
     pub max_nodes: usize,
     pub focus_window_only: bool,
+    /// Walk into menus that are currently closed. Off by default.
+    ///
+    /// A closed `AXMenu` still reports its whole item hierarchy, but every item
+    /// comes back collapsed at a zero-size off-screen frame — unclickable until
+    /// the menu is opened, and useless for addressing. They dominate the dump
+    /// anyway: observing a windowless app produced 188 nodes, 180 of them
+    /// closed menu items. `get_app_shortcuts` is the supported way to read menu
+    /// structure (and walks menus itself), so `get_app_state` stops at the menu.
+    pub include_closed_menus: bool,
 }
 
 impl Default for DumpOpts {
@@ -439,10 +448,28 @@ impl Default for DumpOpts {
             max_depth: 32,
             max_nodes: 4_000,
             focus_window_only: false,
+            include_closed_menus: false,
         }
     }
 }
 
+/// Whether a node is a menu container that is not currently open, and whose
+/// children are therefore off-screen and unclickable.
+///
+/// macOS gives an open menu's items real on-screen frames; a closed one leaves
+/// them at a zero-size origin. Size is the reliable signal here — `AXExpanded`
+/// is not exposed consistently by `AXMenu` across apps.
+fn is_closed_menu_container(role: &str, frame: Option<(f64, f64, f64, f64)>) -> bool {
+    if role != "AXMenu" {
+        return false;
+    }
+    match frame {
+        // No frame at all: treat as closed.
+        None => true,
+        Some((_, _, w, h)) => w < 1.0 || h < 1.0,
+    }
+}
+
 pub(super) fn dump_app_ax(pid: i32, opts: DumpOpts) -> BitFunResult<AppStateSnapshot> {
     let app = unsafe { AXUIElementCreateApplication(pid) };
     if app.is_null() {
@@ -501,6 +528,7 @@ pub(super) fn dump_app_ax(pid: i32, opts: DumpOpts) -> BitFunResult<AppStateSnap
         depth: 0,
     });
     let mut visited: usize = 0;
+    let mut pruned_menu_subtrees: usize = 0;
 
     while let Some(cur) = queue.pop_front() {
         if cur.depth > opts.max_depth || visited >= opts.max_nodes {
@@ -530,10 +558,13 @@ pub(super) fn dump_app_ax(pid: i32, opts: DumpOpts) -> BitFunResult<AppStateSnap
         let frame = unsafe { read_global_frame(cur.elem) };
         let actions = unsafe { read_action_names(cur.elem) };
 
+        let role = role.unwrap_or_default();
+        let is_closed_menu = is_closed_menu_container(&role, frame);
+
         nodes.push(AxNode {
             idx,
             parent_idx: cur.parent_idx,
-            role: role.unwrap_or_default(),
+            role,
             title,
             value,
             description,
@@ -552,6 +583,14 @@ pub(super) fn dump_app_ax(pid: i32, opts: DumpOpts) -> BitFunResult<AppStateSnap
         // Cache the retained ref so future actions can look it up.
         refs.push(AxRef(cur.elem));
 
+        // A closed menu is a leaf for our purposes: keep the container node so
+        // the model can see the menu exists (and `AXPress` it), but skip the
+        // subtree of unclickable zero-size items underneath.
+        if is_closed_menu && !opts.include_closed_menus {
+            pruned_menu_subtrees += 1;
+            continue;
+        }
+
         // Enqueue children — but DO NOT release `cur.elem`; the cache owns it.
         // At the application root (parent_idx is None), union `AXChildren`
         // with `AXWindows`. macOS only puts windows in `AXChildren` when the
@@ -600,7 +639,18 @@ pub(super) fn dump_app_ax(pid: i32, opts: DumpOpts) -> BitFunResult<AppStateSnap
         unsafe { ax_release(q.elem as CFTypeRef) };
     }
 
-    let tree_text = render_tree_text(&nodes);
+    let mut tree_text = render_tree_text(&nodes);
+    // Say that menus were skipped on purpose, and where to get them. Otherwise
+    // an agent that needs a menu command sees a bare `AXMenu` leaf and has no
+    // way to tell "pruned" from "this app has no menu items".
+    if pruned_menu_subtrees > 0 {
+        tree_text.push_str(&format!(
+            "\n[note] {} closed menu subtree(s) omitted — their items are off-screen and \
+unclickable until the menu opens. Use `get_app_shortcuts` for menu commands and their key \
+equivalents, or AXPress the menu first.\n",
+            pruned_menu_subtrees
+        ));
+    }
     let digest = compute_digest(&nodes);
     let captured_at_ms = SystemTime::now()
         .duration_since(UNIX_EPOCH)
@@ -887,6 +937,28 @@ mod tests {
         assert!(out.contains("actions=[AXShowMenu]"));
     }
 
+    #[test]
+    fn closed_menus_are_pruned_but_open_ones_are_kept() {
+        // A closed menu reports its items at a zero-size off-screen frame.
+        assert!(is_closed_menu_container(
+            "AXMenu",
+            Some((0.0, 982.0, 0.0, 0.0))
+        ));
+        assert!(is_closed_menu_container("AXMenu", None));
+        // An open menu has a real frame and must still be walked.
+        assert!(!is_closed_menu_container(
+            "AXMenu",
+            Some((100.0, 40.0, 220.0, 380.0))
+        ));
+        // Only menus are ever pruned — a zero-size button is still a node the
+        // model may need to reason about.
+        assert!(!is_closed_menu_container(
+            "AXButton",
+            Some((0.0, 0.0, 0.0, 0.0))
+        ));
+        assert!(!is_closed_menu_container("AXMenuItem", None));
+    }
+
     #[test]
     fn quote_clip_truncates_on_char_boundary() {
         let s = "中文字符测试abcdef";
@@ -903,6 +975,83 @@ mod tests {
         assert_ne!(d1, d2);
     }
 
+    /// Measure the closed-menu pruning against a real running app rather than
+    /// trusting the unit test's synthetic frames.
+    ///
+    /// Dumps the frontmost application twice — once with menus walked, once
+    /// with the default pruning — and reports both node counts. Requires
+    /// Accessibility permission and a GUI session, so it is `#[ignore]`d.
+    #[test]
+    #[ignore]
+    fn closed_menu_pruning_shrinks_a_real_app_dump() {
+        let pid = crate::computer_use::macos_bg_input::frontmost_pid_macos()
+            .expect("a GUI session has a frontmost app");
+
+        let with_menus = dump_app_ax(
+            pid,
+            DumpOpts {
+                include_closed_menus: true,
+                ..Default::default()
+            },
+        )
+        .expect("dump with menus");
+        let pruned = dump_app_ax(pid, DumpOpts::default()).expect("pruned dump");
+
+        // What `describe_screen` actually asks for: depth 8, focused window
+        // only. Reported alongside so the cost of the observe path is visible
+        // next to the cost of a full `get_app_state`.
+        let observe = dump_app_ax(
+            pid,
+            DumpOpts {
+                max_depth: 8,
+                focus_window_only: true,
+                ..Default::default()
+            },
+        )
+        .expect("describe_screen-shaped dump");
+
+        eprintln!(
+            "pid={pid}\n  full+menus:  {:>5} nodes, {:>7} bytes\n  full pruned: {:>5} nodes, {:>7} bytes\n  observe:     {:>5} nodes, {:>7} bytes",
+            with_menus.nodes.len(),
+            with_menus.tree_text.len(),
+            pruned.nodes.len(),
+            pruned.tree_text.len(),
+            observe.nodes.len(),
+            observe.tree_text.len(),
+        );
+        // Depth profile of the focused window. Run this when retuning
+        // `DESCRIBE_SCREEN_AX_DEPTH`: "actionable" (has AX actions and a real
+        // frame) is what the agent can actually click, and it is the column
+        // that matters — node count and bytes grow long after it plateaus.
+        for d in [8u32, 12, 16, 20, 24, 32] {
+            let s = dump_app_ax(
+                pid,
+                DumpOpts {
+                    max_depth: d,
+                    focus_window_only: true,
+                    ..Default::default()
+                },
+            )
+            .expect("depth dump");
+            let actionable = s
+                .nodes
+                .iter()
+                .filter(|n| !n.actions.is_empty() && n.frame_global.is_some())
+                .count();
+            eprintln!(
+                "  depth {:>2}: {:>5} nodes, {:>4} actionable, {:>7} bytes",
+                d,
+                s.nodes.len(),
+                actionable,
+                s.tree_text.len()
+            );
+        }
+        assert!(
+            pruned.nodes.len() <= with_menus.nodes.len(),
+            "pruning must never grow the tree"
+        );
+    }
+
     /// Smoke test: dump the AX tree of *this* test process. The test process
     /// usually has no AX windows of its own, so we only assert the call
     /// returns *something* (possibly an empty tree) without panicking and

src/apps/desktop/src/computer_use/macos_ax_ui.rs

@@ -48,24 +48,17 @@ unsafe extern "C" {
 const K_AX_VALUE_CGPOINT: u32 = 1;
 const K_AX_VALUE_CGSIZE: u32 = 2;
 
+/// Pid of the frontmost application, via `NSWorkspace.frontmostApplication`.
+///
+/// Reads in-process in microseconds. The previous implementation shelled out to
+/// `osascript` on every call — and this is on the hot path for
+/// `describe_screen` (which reaches it up to three times per call), so a single
+/// observation used to cost several hundred milliseconds of process spawns plus
+/// the risk of a System Events AppleEvent timeout.
 fn frontmost_pid() -> BitFunResult<i32> {
-    let out = std::process::Command::new("/usr/bin/osascript")
-        .args([
-            "-e",
-            "tell application \"System Events\" to get unix id of first process whose frontmost is true",
-        ])
-        .output()
-        .map_err(|e| BitFunError::tool(format!("osascript spawn: {}", e)))?;
-    if !out.status.success() {
-        return Err(BitFunError::tool(format!(
-            "osascript failed: {}",
-            String::from_utf8_lossy(&out.stderr)
-        )));
-    }
-    let s = String::from_utf8_lossy(&out.stdout);
-    s.trim()
-        .parse::<i32>()
-        .map_err(|_| BitFunError::tool("Could not parse frontmost process id.".to_string()))
+    crate::computer_use::macos_bg_input::frontmost_pid_macos().ok_or_else(|| {
+        BitFunError::tool("NSWorkspace reported no frontmost application.".to_string())
+    })
 }
 
 unsafe fn ax_release(v: CFTypeRef) {
@@ -525,8 +518,10 @@ unsafe fn is_ax_hidden(elem: AXUIElementRef) -> bool {
             return false; // No AXHidden attribute = not hidden
         };
         // AXHidden is a CFBoolean
-        let hidden =
-            std::ptr::eq(val, core_foundation::boolean::kCFBooleanTrue as *const c_void);
+        let hidden = std::ptr::eq(
+            val,
+            core_foundation::boolean::kCFBooleanTrue as *const c_void,
+        );
         ax_release(val);
         hidden
     }
@@ -1123,6 +1118,37 @@ pub(super) fn frontmost_window_bounds_global() -> BitFunResult<(i32, i32, u32, u
     window_bounds_global_for_pid(pid)
 }
 
+/// Number of windows the app currently owns, per the AX `AXWindows` attribute.
+///
+/// A launched-but-windowless app — common for Electron clients whose window was
+/// closed while the process kept running — is otherwise indistinguishable from a
+/// healthy launch: `open_app` reports `success: true` with a live pid while
+/// there is nothing on screen to act on. Returning the count lets `open_app`
+/// detect that case and re-open the app instead of leaving the agent to
+/// discover it by trial and error.
+///
+/// `None` means the AX handle could not be created at all (dead pid, no
+/// Accessibility trust); `Some(0)` means the app is alive with no windows.
+pub(super) fn window_count_for_pid(pid: i32) -> Option<usize> {
+    // SAFETY: `AXUIElementCreateApplication` accepts any pid and returns null
+    // rather than an invalid handle, which we check before use.
+    let app = unsafe { AXUIElementCreateApplication(pid) };
+    if app.is_null() {
+        return None;
+    }
+    // SAFETY: `app` is a live non-null AXUIElementRef we own. `ax_copy_attr`
+    // follows the CF *Copy* rule, so the returned array carries a +1 retain that
+    // `wrap_under_create_rule` takes over. `app` is released as soon as the
+    // attribute has been copied out of it, on both the Some and None paths.
+    unsafe {
+        let arr_ref = ax_copy_attr(app, "AXWindows");
+        ax_release(app as CFTypeRef);
+        let arr_ref = arr_ref?;
+        let arr = CFArray::<*const c_void>::wrap_under_create_rule(arr_ref as CFArrayRef);
+        Some(arr.len() as usize)
+    }
+}
+
 /// Bounds of the selected app's focused or main window in global screen coordinates.
 pub(super) fn window_bounds_global_for_pid(pid: i32) -> BitFunResult<(i32, i32, u32, u32)> {
     let app = unsafe { AXUIElementCreateApplication(pid) };