From c07f43e242cfabdb26afb0343711456581b94b52 Mon Sep 17 00:00:00 2001
From: Chris Tate <chris@ctate.dev>
Date: Sat, 14 Mar 2026 20:54:08 -0500
Subject: [PATCH] fix: re-query accessibility tree when backend_node_id is
 stale (#806)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fix: gracefully fall back to role/name lookup when backend_node_id is stale

When the DOM changes between snapshot and click (common with SPAs and
dynamic UIs), the stored backend_node_id becomes invalid. Previously,
DOM.getBoxModel and DOM.resolveNode failures propagated as hard errors,
bypassing the role/name fallback path entirely. Now these failures are
caught and the code falls through to a JS-based element lookup.

Also adds resolve_object_id_by_role_name so that resolve_element_object_id
has a fallback for ref-based lookups (previously it had none), and
improves the role matching JS to correctly map implicit ARIA roles
(e.g. <input type="submit"> → "button", <a href> → "link").

Closes #805

* test: add e2e regression test for stale ref click fallback (#805)

Verifies that clicking a ref whose backend_node_id has become stale
(because the DOM was replaced by JavaScript) falls back to role/name
lookup instead of failing with "Could not compute box model".

* fix: use accessibility tree for stale ref fallback instead of JS heuristic

Replace the hand-rolled JS role/name matching (getImplicitRole,
getAccessibleName) with a re-query of Accessibility.getFullAXTree —
the same data source that built the ref map during snapshot. This
guarantees role/name matching is identical to what was stored,
preventing silent wrong-element clicks from name computation
divergence (e.g. aria-labelledby, <label for>, alt text).

Matches v0.19.0 (Playwright) behavior where getByRole always
re-queried the live accessibility tree.

---------

Co-authored-by: ctate <366502+ctate@users.noreply.github.com>
---
 cli/src/native/e2e_tests.rs |  92 ++++++++++++++++++++++
 cli/src/native/element.rs   | 147 ++++++++++++++++++++++++------------
 2 files changed, 189 insertions(+), 50 deletions(-)
diff --git a/cli/src/native/e2e_tests.rs b/cli/src/native/e2e_tests.rs
index 1238104..609afee 100644
--- a/cli/src/native/e2e_tests.rs
+++ b/cli/src/native/e2e_tests.rs
@@ -1711,3 +1711,95 @@ async fn e2e_inspect() {
     let resp = execute_command(&json!({ "id": "99", "action": "close" }), &mut state).await;
     assert_success(&resp);
 }
+
+// ---------------------------------------------------------------------------
+// Stale ref fallback (#805): clicking a ref after the DOM has been replaced
+// should fall back to role/name lookup instead of failing.
+// ---------------------------------------------------------------------------
+
+#[tokio::test]
+#[ignore]
+async fn e2e_click_stale_ref_falls_back_to_role_name() {
+    let mut state = DaemonState::new();
+
+    let resp = execute_command(
+        &json!({ "id": "1", "action": "launch", "headless": true }),
+        &mut state,
+    )
+    .await;
+    assert_success(&resp);
+
+    // Navigate to a page with a button that replaces the DOM when clicked.
+    let html = r#"data:text/html,<body>
+        <div id="c">
+            <button onclick="
+                var c = document.getElementById('c');
+                c.innerHTML = '';
+                var b = document.createElement('button');
+                b.textContent = 'Target';
+                b.onclick = function() { document.title = 'clicked'; };
+                c.appendChild(b);
+                document.title = 'replaced';
+            ">Replace</button>
+            <button>Target</button>
+        </div>
+    </body>"#;
+
+    let resp = execute_command(
+        &json!({ "id": "2", "action": "navigate", "url": html }),
+        &mut state,
+    )
+    .await;
+    assert_success(&resp);
+
+    // Snapshot to populate the ref_map with backend_node_ids.
+    let resp = execute_command(&json!({ "id": "3", "action": "snapshot" }), &mut state).await;
+    assert_success(&resp);
+    let snapshot = get_data(&resp)["snapshot"].as_str().unwrap();
+    assert!(
+        snapshot.contains("Replace"),
+        "Snapshot should contain Replace button"
+    );
+    assert!(
+        snapshot.contains("Target"),
+        "Snapshot should contain Target button"
+    );
+
+    // Click "Replace" — this removes all DOM nodes and recreates them,
+    // making the backend_node_id for "Target" stale.
+    let resp = execute_command(
+        &json!({ "id": "4", "action": "click", "selector": "e1" }),
+        &mut state,
+    )
+    .await;
+    assert_success(&resp);
+    tokio::time::sleep(tokio::time::Duration::from_millis(200)).await;
+
+    // Verify the DOM was actually replaced.
+    let resp = execute_command(&json!({ "id": "5", "action": "title" }), &mut state).await;
+    assert_success(&resp);
+    assert_eq!(get_data(&resp)["title"], "replaced");
+
+    // Now click the stale "Target" ref. Before the fix this returned:
+    //   "CDP error (DOM.getBoxModel): Could not compute box model."
+    // After the fix it falls back to role/name lookup and succeeds.
+    let resp = execute_command(
+        &json!({ "id": "6", "action": "click", "selector": "e2" }),
+        &mut state,
+    )
+    .await;
+    assert_success(&resp);
+    tokio::time::sleep(tokio::time::Duration::from_millis(200)).await;
+
+    // Verify the fallback click hit the right (recreated) button.
+    let resp = execute_command(&json!({ "id": "7", "action": "title" }), &mut state).await;
+    assert_success(&resp);
+    assert_eq!(
+        get_data(&resp)["title"],
+        "clicked",
+        "Stale ref should have been resolved via role/name fallback"
+    );
+
+    let resp = execute_command(&json!({ "id": "99", "action": "close" }), &mut state).await;
+    assert_success(&resp);
+}
diff --git a/cli/src/native/element.rs b/cli/src/native/element.rs
index b21ef00..35718d6 100644
--- a/cli/src/native/element.rs
+++ b/cli/src/native/element.rs
@@ -138,8 +138,9 @@ pub async fn resolve_element_center(
             .get(&ref_id)
             .ok_or_else(|| format!("Unknown ref: {}", ref_id))?;
 
+        // Try cached backend_node_id first (fast path)
         if let Some(backend_node_id) = entry.backend_node_id {
-            let result: DomGetBoxModelResult = client
+            let result: Result<DomGetBoxModelResult, String> = client
                 .send_command_typed(
                     "DOM.getBoxModel",
                     &DomGetBoxModelParams {
@@ -149,13 +150,30 @@ pub async fn resolve_element_center(
                     },
                     Some(session_id),
                 )
-                .await?;
+                .await;
 
-            return Ok(box_model_center(&result.model));
+            if let Ok(r) = result {
+                return Ok(box_model_center(&r.model));
+            }
+            // backend_node_id is stale; re-query the accessibility tree below
         }
 
-        // Fallback: use role/name to find via JS
-        return resolve_by_role_name(client, session_id, &entry.role, &entry.name, entry.nth).await;
+        // Fallback: re-query the accessibility tree to find a fresh node by role/name
+        let fresh_id =
+            find_node_id_by_role_name(client, session_id, &entry.role, &entry.name, entry.nth)
+                .await?;
+        let result: DomGetBoxModelResult = client
+            .send_command_typed(
+                "DOM.getBoxModel",
+                &DomGetBoxModelParams {
+                    backend_node_id: Some(fresh_id),
+                    node_id: None,
+                    object_id: None,
+                },
+                Some(session_id),
+            )
+            .await?;
+        return Ok(box_model_center(&result.model));
     }
 
     // CSS selector
@@ -173,8 +191,9 @@ pub async fn resolve_element_object_id(
             .get(&ref_id)
             .ok_or_else(|| format!("Unknown ref: {}", ref_id))?;
 
+        // Try cached backend_node_id first (fast path)
         if let Some(backend_node_id) = entry.backend_node_id {
-            let result: DomResolveNodeResult = client
+            let result: Result<DomResolveNodeResult, String> = client
                 .send_command_typed(
                     "DOM.resolveNode",
                     &DomResolveNodeParams {
@@ -184,13 +203,35 @@ pub async fn resolve_element_object_id(
                     },
                     Some(session_id),
                 )
-                .await?;
+                .await;
 
-            return result
-                .object
-                .object_id
-                .ok_or_else(|| format!("No objectId for ref {}", ref_id));
+            if let Ok(r) = result {
+                if let Some(oid) = r.object.object_id {
+                    return Ok(oid);
+                }
+            }
+            // backend_node_id is stale; re-query the accessibility tree below
         }
+
+        // Fallback: re-query the accessibility tree to find a fresh node by role/name
+        let fresh_id =
+            find_node_id_by_role_name(client, session_id, &entry.role, &entry.name, entry.nth)
+                .await?;
+        let result: DomResolveNodeResult = client
+            .send_command_typed(
+                "DOM.resolveNode",
+                &DomResolveNodeParams {
+                    backend_node_id: Some(fresh_id),
+                    node_id: None,
+                    object_group: Some("agent-browser".to_string()),
+                },
+                Some(session_id),
+            )
+            .await?;
+        return result
+            .object
+            .object_id
+            .ok_or_else(|| format!("No objectId for ref {}", ref_id));
     }
 
     // CSS selector fallback
@@ -216,56 +257,62 @@ pub async fn resolve_element_object_id(
         .ok_or_else(|| format!("Element not found: {}", selector_or_ref))
 }
 
-async fn resolve_by_role_name(
+/// Re-query the accessibility tree to find a node matching role+name+nth,
+/// returning its fresh backendDOMNodeId. This uses the same data source
+/// (Accessibility.getFullAXTree) that built the ref map during snapshot,
+/// so role/name matching is guaranteed to be consistent.
+async fn find_node_id_by_role_name(
     client: &CdpClient,
     session_id: &str,
     role: &str,
     name: &str,
     nth: Option<usize>,
-) -> Result<(f64, f64), String> {
-    let nth_index = nth.unwrap_or(0);
-    let js = format!(
-        r#"(() => {{
-            const walker = document.createTreeWalker(document.body, NodeFilter.SHOW_ELEMENT);
-            const matches = [];
-            let node;
-            while (node = walker.nextNode()) {{
-                const r = node.getAttribute('role') || node.tagName.toLowerCase();
-                const n = node.getAttribute('aria-label') || node.textContent.trim().slice(0, 100);
-                if (r === {role} && n === {name}) matches.push(node);
-            }}
-            const el = matches[{nth}];
-            if (!el) return null;
-            const rect = el.getBoundingClientRect();
-            return {{ x: rect.x + rect.width / 2, y: rect.y + rect.height / 2 }};
-        }})()"#,
-        role = serde_json::to_string(role).unwrap_or_default(),
-        name = serde_json::to_string(name).unwrap_or_default(),
-        nth = nth_index,
-    );
-
-    let result: EvaluateResult = client
+) -> Result<i64, String> {
+    let ax_tree: GetFullAXTreeResult = client
         .send_command_typed(
-            "Runtime.evaluate",
-            &EvaluateParams {
-                expression: js,
-                return_by_value: Some(true),
-                await_promise: Some(false),
-            },
+            "Accessibility.getFullAXTree",
+            &serde_json::json!({}),
             Some(session_id),
         )
         .await?;
 
-    let val = result.result.value.unwrap_or(Value::Null);
-    let x = val.get("x").and_then(|v| v.as_f64());
-    let y = val.get("y").and_then(|v| v.as_f64());
+    let nth_index = nth.unwrap_or(0);
+    let mut match_count: usize = 0;
 
-    match (x, y) {
-        (Some(x), Some(y)) => Ok((x, y)),
-        _ => Err(format!(
-            "Could not locate element with role={} name={}",
-            role, name
-        )),
+    for node in &ax_tree.nodes {
+        if node.ignored.unwrap_or(false) {
+            continue;
+        }
+        let node_role = extract_ax_string(&node.role);
+        let node_name = extract_ax_string(&node.name);
+        if node_role == role && node_name == name {
+            if match_count == nth_index {
+                return node.backend_d_o_m_node_id.ok_or_else(|| {
+                    format!(
+                        "AX node has no backendDOMNodeId for role={} name={}",
+                        role, name
+                    )
+                });
+            }
+            match_count += 1;
+        }
+    }
+
+    Err(format!(
+        "Could not locate element with role={} name={}",
+        role, name
+    ))
+}
+
+fn extract_ax_string(value: &Option<AXValue>) -> String {
+    match value {
+        Some(v) => match &v.value {
+            Some(Value::String(s)) => s.clone(),
+            Some(Value::Number(n)) => n.to_string(),
+            Some(Value::Bool(b)) => b.to_string(),
+            _ => String::new(),
+        },
+        None => String::new(),
     }
 }
 
