From 007fd1b27f8d997db1f8d86762ded63c7f90d9f2 Mon Sep 17 00:00:00 2001
From: leeguooooo <guoli@zhihu.com>
Date: Sat, 9 May 2026 03:26:20 +0900
Subject: [PATCH] fix(refs): verify identity before using cached backendNodeId
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Closes the "click @e20 hits the sibling element" bug. Real-world
example: snapshot shows @e20=[button "Add post"] next to
@e17=[button "Post all"]. By the time you click @e20, React has
re-rendered — and React often re-uses the same <button> DOM node
across renders, just updating its accessible name. The cached
backendNodeId still resolves to a real, well-positioned node, so
the click lands cleanly. It just lands on what is now the "Post all"
button, silently submitting the entire thread instead of adding a
draft row.

Before every ref-based interaction (click / fill / type / hover /
select / drag — anything routing through resolve_element_center or
resolve_element_object_id), call Accessibility.getPartialAXTree for
the cached backendNodeId and check role + name still match the
snapshot entry. On mismatch, abort with an error that names both
labels:

  Ref @e20 no longer matches its snapshot. Was [button "Add post"],
  now [button "Post all"].
  ...Take a fresh snapshot, then re-target.

If the node is gone (CDP fails / no AX node), we silently fall
through to the existing "find by role+name" recovery path, so this
guard never makes a working flow worse.

Adds one CDP roundtrip per ref interaction (~5–20ms). Disable with
AGENT_BROWSER_VERIFY_REF=0 if you control the page lifecycle and
need the latency back.
---
 cli/src/native/element.rs | 93 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/cli/src/native/element.rs b/cli/src/native/element.rs
index 7b22ed9..3c997d2 100644
--- a/cli/src/native/element.rs
+++ b/cli/src/native/element.rs
@@ -163,6 +163,30 @@ pub async fn resolve_element_center(
 
         // Try cached backend_node_id first (fast path)
         if let Some(backend_node_id) = entry.backend_node_id {
+            // Identity check: React often re-uses the same DOM node when
+            // re-rendering — backendNodeId stays the same but accessibleName
+            // / role changes. Without this verification, `click @e20` (saved
+            // when the button said "Add post") happily clicks the *same*
+            // node that now says "Post all", silently submitting the thread.
+            //
+            // Set AGENT_BROWSER_VERIFY_REF=0 to skip (saves one CDP
+            // roundtrip per ref-based interaction; only safe if you know
+            // the page is static between snapshot and click).
+            if std::env::var("AGENT_BROWSER_VERIFY_REF").as_deref() != Ok("0") {
+                if let Err(e) = verify_ref_identity(
+                    client,
+                    effective_session_id,
+                    backend_node_id,
+                    &ref_id,
+                    &entry.role,
+                    &entry.name,
+                )
+                .await
+                {
+                    return Err(e);
+                }
+            }
+
             let result: Result<DomGetBoxModelResult, String> = client
                 .send_command_typed(
                     "DOM.getBoxModel",
@@ -230,6 +254,24 @@ pub async fn resolve_element_object_id(
 
         // Try cached backend_node_id first (fast path)
         if let Some(backend_node_id) = entry.backend_node_id {
+            // Same identity guard as resolve_element_center — see that
+            // function for why React DOM-node-reuse breaks ref-based
+            // interactions if we skip this.
+            if std::env::var("AGENT_BROWSER_VERIFY_REF").as_deref() != Ok("0") {
+                if let Err(e) = verify_ref_identity(
+                    client,
+                    effective_session_id,
+                    backend_node_id,
+                    &ref_id,
+                    &entry.role,
+                    &entry.name,
+                )
+                .await
+                {
+                    return Err(e);
+                }
+            }
+
             let result: Result<DomResolveNodeResult, String> = client
                 .send_command_typed(
                     "DOM.resolveNode",
@@ -333,6 +375,57 @@ fn resolve_frame_session<'a>(
         .unwrap_or(session_id)
 }
 
+/// Verify that the cached backendNodeId still has the same accessible role
+/// and name it had when the snapshot ran. Catches the case where React (or
+/// any reconciler) reused the DOM node for a different component instance
+/// — same physical node, different semantics.
+///
+/// On mismatch, returns an actionable error naming both the snapshot label
+/// and the current label so the agent can re-snapshot intelligently.
+/// On any CDP failure (e.g. node deleted), returns Ok(()) so the caller's
+/// existing fallback (`find_node_id_by_role_name`) takes over.
+async fn verify_ref_identity(
+    client: &CdpClient,
+    session_id: &str,
+    backend_node_id: i64,
+    ref_id: &str,
+    expected_role: &str,
+    expected_name: &str,
+) -> Result<(), String> {
+    let params = serde_json::json!({
+        "backendNodeId": backend_node_id,
+        "fetchRelatives": false,
+    });
+    let resp: Result<GetFullAXTreeResult, String> = client
+        .send_command_typed("Accessibility.getPartialAXTree", &params, Some(session_id))
+        .await;
+    let Ok(tree) = resp else {
+        // Node likely gone; let the box-model call fail and trigger fallback.
+        return Ok(());
+    };
+    // Find the AXNode for our backendNodeId. fetchRelatives=false still
+    // returns ancestors; the target node has the matching backendNodeId.
+    let Some(node) = tree
+        .nodes
+        .iter()
+        .find(|n| n.backend_d_o_m_node_id == Some(backend_node_id))
+    else {
+        return Ok(());
+    };
+    let actual_role = extract_ax_string(&node.role);
+    let actual_name = extract_ax_string(&node.name);
+    if actual_role == expected_role && actual_name == expected_name {
+        return Ok(());
+    }
+    Err(format!(
+        "Ref {} no longer matches its snapshot. Was [{} \"{}\"], now [{} \"{}\"].\n\
+         The DOM mutated between snapshot and interaction (typical with React/Vue \
+         reusing nodes during re-render). Take a fresh snapshot, then re-target.\n\
+         To bypass this guard set AGENT_BROWSER_VERIFY_REF=0.",
+        ref_id, expected_role, expected_name, actual_role, actual_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,