fix(chat-select): bounded non-blocking reads and shared composer finder

docker/tools/chat-select.py

@@ -20,6 +20,7 @@
 import json
 import os
 import re
+import select
 import shutil
 
 # ── Per-build constants ──────────────────────────────────────────────────────
@@ -230,26 +231,59 @@ def write_js(path, content):
 """
 
 
+def read_lines_until(proc, timeout, stop_on=None):
+    """Read output lines from proc.stdout until stop_on appears, EOF, or the
+    wall-clock deadline expires. Returns the lines read (rstripped).
+
+    A plain proc.stdout.readline() blocks indefinitely when the child stays
+    alive but silent (e.g. a frida hook that never fires), defeating any
+    time-based loop guard — that hang is what used to make `wx send` time out.
+    This reads the raw fd non-blocking behind select(), so the deadline holds
+    even for partial lines, and select() can never miss data stranded in a
+    Python-level buffer (we own the only buffer, kept on the proc object so
+    consecutive calls on the same proc don't lose bytes read past stop_on).
+    """
+    fd = proc.stdout.fileno()
+    os.set_blocking(fd, False)
+    deadline = time.time() + timeout
+    buf = getattr(proc, "_read_buf", b"")
+    lines = []
+    try:
+        while True:
+            while b"\n" in buf:
+                raw, buf = buf.split(b"\n", 1)
+                line = raw.decode("utf-8", errors="replace").rstrip()
+                lines.append(line)
+                if stop_on and stop_on in line:
+                    return lines
+            remaining = deadline - time.time()
+            if remaining <= 0:
+                return lines  # deadline expired
+            ready, _, _ = select.select([fd], [], [], remaining)
+            if not ready:
+                return lines  # deadline expired waiting for output
+            try:
+                chunk = os.read(fd, 65536)
+            except BlockingIOError:
+                continue
+            except OSError:
+                return lines
+            if not chunk:
+                return lines  # EOF
+            buf += chunk
+    finally:
+        proc._read_buf = buf
+
+
 def run_frida_script(pid, script_path, timeout=30, stop_on="SCRIPT_DONE"):
     """Run a frida script, return output lines."""
     proc = subprocess.Popen(
         [FRIDA_BIN, "-p", pid, "-l", script_path, "--runtime=v8", "-q"],
         stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
         stdin=subprocess.PIPE, text=True, bufsize=1,
     )
-    lines = []
-    start = time.time()
     try:
-        while time.time() - start < timeout:
-            line = proc.stdout.readline()
-            if not line:
-                break
-            line = line.rstrip()
-            lines.append(line)
-            if stop_on and stop_on in line:
-                break
-    except Exception:
-        pass
+        lines = read_lines_until(proc, timeout, stop_on=stop_on)
     finally:
         try:
             proc.stdin.close()
@@ -271,13 +305,7 @@ def run_frida_bg(pid, script_path):
         stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
         stdin=subprocess.PIPE, text=True, bufsize=1,
     )
-    start = time.time()
-    while time.time() - start < 10:
-        line = proc.stdout.readline()
-        if not line:
-            break
-        if "READY" in line:
-            break
+    read_lines_until(proc, 10, stop_on="READY")
     return proc
 
 
@@ -554,17 +582,10 @@ def select_by_index(pid, profile, target_index, click_coords, vector_base, vecto
                                  timeout=5, capture_output=True, text=True)
     log(f"[chat-select] Click result: {click_result.stdout.strip()}")
 
-    # Read output looking for DETACHED confirmation (hook fires once then detaches)
-    lines = []
-    start = time.time()
-    while time.time() - start < 5:
-        line = proc.stdout.readline()
-        if not line:
-            break
-        line = line.rstrip()
-        lines.append(line)
-        if "DETACHED" in line:
-            break
+    # Read output looking for DETACHED confirmation (hook fires once then
+    # detaches). Bounded read: if the click did not produce a selectSession
+    # call the hook never fires and we give up after the deadline.
+    lines = read_lines_until(proc, 5, stop_on="DETACHED")
 
     kill_frida(proc)
 
@@ -633,9 +654,16 @@ def main():
     target_index = sessions[target]
     log(f"[chat-select] Target: {target} -> index {target_index}")
 
-    # Current-selection skip: if not forced and target already selected, skip
-    if not force and current_sel and current_sel == target:
-        log(f"[chat-select] Target already selected (current_sel={current_sel}), skipping")
+    # Already-selected short-circuit: if the target chat is ALREADY the current
+    # selection, the right-hand pane is already showing it — there is nothing
+    # to do. This holds even when force=True: clicking the already-selected
+    # chat list item does NOT trigger a selectSession() call, so the Frida
+    # hook never fires and select_by_index() would wait out its deadline and
+    # report a false "Hook did not fire" failure. current_sel is freshly read
+    # from WeChat's current-session pointer on every invocation, so there is
+    # no stale skip decision for force to override.
+    if current_sel == target:
+        log(f"[chat-select] Target already selected (current_sel={current_sel}), skipping (force={force})")
         result_json(True, username=target, index=target_index, skipped=True)
 
     # Find click coordinates: use --click-xy if provided, else fall back to a11y

packages/agent-server-rust/src/ia/helpers.rs

@@ -36,6 +36,98 @@ pub fn get_bounds_center(bounds: &Bounds) -> (f64, f64) {
     )
 }
 
+/// Check whether an a11y node carries the given state (e.g. "FOCUSED",
+/// "DISABLED", "EDITABLE").
+pub fn node_has_state(node: &A11yNode, state: &str) -> bool {
+    node.states
+        .as_ref()
+        .map(|s| s.iter().any(|st| st == state))
+        .unwrap_or(false)
+}
+
+/// The main application frame is named "Weixin" or "WeChat" depending on
+/// build/locale (the in-repo fixtures use "WeChat"; chat.rs matches both
+/// names for the nav button for the same reason).
+fn is_main_frame(node: &A11yNode) -> bool {
+    node.role == "frame" && (node.name == "Weixin" || node.name == "WeChat")
+}
+
+/// A candidate composer: the editable text input plus its sibling Send(S) button.
+struct ComposerPair<'a> {
+    edit: &'a A11yNode,
+    send: &'a A11yNode,
+    /// True if this pair lives under the main application frame
+    /// (as opposed to a detached/ghost chat frame leftover in the a11y tree).
+    in_main_frame: bool,
+}
+
+/// Find the composer (editable + Send button) to operate on.
+///
+/// WeChat's accessibility tree can contain *multiple* edit+send pairs:
+/// the live main-window composer plus stale "ghost" frames left behind by
+/// chats that were previously detached into separate windows. A naive
+/// depth-first "take the first pair" grabs the wrong (ghost) composer, whose
+/// input never receives text and whose Send stays DISABLED forever, causing
+/// plans to loop and ultimately fail with "No action selected".
+///
+/// To be robust we collect every candidate pair, then rank them so the
+/// genuinely active composer wins:
+///   1. editable currently FOCUSED          (strongest signal)
+///   2. Send button NOT disabled            (composer already has text)
+///   3. pair under the main application frame (not a ghost/detached frame)
+/// The first pair (DFS order) breaks any remaining ties.
+pub fn find_edit_and_send_button(a11y: &A11yNode) -> Option<(&A11yNode, &A11yNode)> {
+    let mut candidates: Vec<ComposerPair> = Vec::new();
+    collect_edit_send_pairs(a11y, false, &mut candidates);
+
+    // Rank lexicographically; `false` sorts before `true`, so each criterion
+    // is written as "false = preferred". DFS index breaks ties.
+    candidates
+        .iter()
+        .enumerate()
+        .min_by_key(|(idx, c)| {
+            (
+                !node_has_state(c.edit, "FOCUSED"),
+                node_has_state(c.send, "DISABLED"),
+                !c.in_main_frame,
+                *idx,
+            )
+        })
+        .map(|(_, c)| (c.edit, c.send))
+}
+
+/// Recursively collect all edit+send composer pairs, tracking whether each
+/// pair is inside the main application frame.
+fn collect_edit_send_pairs<'a>(
+    node: &'a A11yNode,
+    in_main_frame: bool,
+    out: &mut Vec<ComposerPair<'a>>,
+) {
+    // Once we enter the main frame, everything below it is in-main.
+    let in_main_frame = in_main_frame || is_main_frame(node);
+
+    if let Some(children) = &node.children {
+        let send_btn = children
+            .iter()
+            .find(|c| c.role == "push-button" && c.name == "Send(S)");
+        let edit_node = children
+            .iter()
+            .find(|c| c.role == "text" && node_has_state(c, "EDITABLE"));
+
+        if let (Some(edit), Some(send)) = (edit_node, send_btn) {
+            out.push(ComposerPair {
+                edit,
+                send,
+                in_main_frame,
+            });
+        }
+
+        for child in children {
+            collect_edit_send_pairs(child, in_main_frame, out);
+        }
+    }
+}
+
 /// Extract a FrameHint from an a11y frame node.
 pub fn frame_hint_from_node(node: &A11yNode) -> Option<FrameHint> {
     let bounds = node.bounds.clone()?;

packages/agent-server-rust/src/plans/chat_open.rs

@@ -1,5 +1,6 @@
 use super::Plan;
 use crate::ia::actions;
+use crate::ia::helpers::find_edit_and_send_button;
 use crate::ia::selectors::{query_selector, query_selector_all};
 use crate::ia::types::*;
 use crate::tools::chat_select::{open_chat, OpenChatResult};
@@ -24,36 +25,9 @@ pub enum ChatOpenPhase {
 }
 
 fn find_edit_area(a11y: &A11yNode) -> Option<&A11yNode> {
-    find_edit_near_send(a11y)
-}
-
-fn find_edit_near_send(node: &A11yNode) -> Option<&A11yNode> {
-    if let Some(children) = &node.children {
-        let has_send = children.iter().any(|c| {
-            c.role == "push-button" && c.name == "Send(S)"
-        });
-        let edit_node = children.iter().find(|c| {
-            c.role == "text"
-                && c.states
-                    .as_ref()
-                    .map(|s| s.iter().any(|st| st == "EDITABLE"))
-                    .unwrap_or(false)
-        });
-
-        if has_send {
-            if let Some(edit) = edit_node {
-                return Some(edit);
-            }
-        }
-
-        // Recurse
-        for child in children {
-            if let Some(result) = find_edit_near_send(child) {
-                return Some(result);
-            }
-        }
-    }
-    None
+    // Ghost-frame-aware: ranks all edit+send pairs so a stale detached-chat
+    // composer is not picked over the live one (see ia::helpers).
+    find_edit_and_send_button(a11y).map(|(edit, _)| edit)
 }
 
 #[async_trait::async_trait]

packages/agent-server-rust/src/plans/send_message.rs

@@ -1,5 +1,6 @@
 use super::Plan;
 use crate::ia::actions;
+use crate::ia::helpers::{find_edit_and_send_button, node_has_state};
 use crate::ia::selectors::query_selector;
 use crate::ia::types::*;
 use crate::tools::chat_select::{open_chat, OpenChatResult};
@@ -29,49 +30,6 @@ pub struct SendMessagePlanState {
     pub confirm_attempts: u32,
 }
 
-fn find_edit_and_send_button(a11y: &A11yNode) -> Option<(&A11yNode, &A11yNode)> {
-    let send_btn = query_selector(a11y, r#"push-button[name="Send(S)"]"#)?;
-    // Find sibling EDITABLE text node via parent
-    // Since we don't have parent refs in the tree-based approach,
-    // we search the tree for the pattern
-    find_edit_near_send(a11y, send_btn)
-}
-
-fn find_edit_near_send<'a>(
-    root: &'a A11yNode,
-    _send_btn: &A11yNode,
-) -> Option<(&'a A11yNode, &'a A11yNode)> {
-    // Walk tree looking for a parent that has both an EDITABLE text and Send(S) button
-    find_edit_send_pair(root)
-}
-
-fn find_edit_send_pair(node: &A11yNode) -> Option<(&A11yNode, &A11yNode)> {
-    if let Some(children) = &node.children {
-        let send_btn = children.iter().find(|c| {
-            c.role == "push-button" && c.name == "Send(S)"
-        });
-        let edit_node = children.iter().find(|c| {
-            c.role == "text"
-                && c.states
-                    .as_ref()
-                    .map(|s| s.iter().any(|st| st == "EDITABLE"))
-                    .unwrap_or(false)
-        });
-
-        if let (Some(edit), Some(send)) = (edit_node, send_btn) {
-            return Some((edit, send));
-        }
-
-        // Recurse
-        for child in children {
-            if let Some(result) = find_edit_send_pair(child) {
-                return Some(result);
-            }
-        }
-    }
-    None
-}
-
 #[async_trait::async_trait]
 impl Plan for SendMessagePlan {
     type PlanState = SendMessagePlanState;
@@ -125,7 +83,11 @@ impl Plan for SendMessagePlan {
                         ))
                     });
 
-                    let force = main_state_id == Some("chat");
+                    // Always run chat-select, even in state "chat_open": the open
+                    // chat may not be the target. This is cheap — chat-select
+                    // itself short-circuits when the target is already the
+                    // current selection.
+                    let force = true;
                     let result = open_chat(&params.chat_id, force, click_xy).await;
 
                     if !result.ok {
@@ -158,13 +120,7 @@ impl Plan for SendMessagePlan {
 
                     plan_state.phase = SendMessagePhase::Inputting;
 
-                    let is_focused = edit_node
-                        .states
-                        .as_ref()
-                        .map(|s| s.iter().any(|st| st == "FOCUSED"))
-                        .unwrap_or(false);
-
-                    if is_focused {
+                    if node_has_state(edit_node, "FOCUSED") {
                         continue;
                     }
 
@@ -236,13 +192,7 @@ impl Plan for SendMessagePlan {
                         None => return None,
                     };
 
-                    let is_disabled = send_btn
-                        .states
-                        .as_ref()
-                        .map(|s| s.iter().any(|st| st == "DISABLED"))
-                        .unwrap_or(false);
-
-                    if is_disabled {
+                    if node_has_state(send_btn, "DISABLED") {
                         plan_state.phase = SendMessagePhase::Done;
                         return Some(SelectedAction {
                             action: actions::wait_short(),