From 1b3d41e579980b02981f212ac65f86ecdaa50289 Mon Sep 17 00:00:00 2001 From: leeguooooo Date: Sat, 9 May 2026 10:04:14 +0900 Subject: [PATCH] fix(timeout): cap defensive CDP guards so click can't hang multi-minute MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported: a single `click @ref` could hang 5+ minutes, with multiple queued click invocations adding up to 7+ minutes — worst case 30s timeout × 3 CDP calls × N parallel processes: - verify_ref_identity (Accessibility.getPartialAXTree) → default 30s - resolveNode / getBoxModel → default 30s - wait_for_paint_settled (Runtime.evaluate awaitPromise) → default 30s The latter two are best-effort defenses added in fork.3-5 to fix SPA race / DOM-reuse bugs. They should never block a real click for 30s — the unguarded code path was always faster than the guarded path-that-hangs. - verify_ref_identity capped at 1s (skips check on timeout) - wait_for_paint_settled capped at 500ms (skips wait on timeout) Both skip-on-timeout intentionally: the worst case is the click behaves like fork.2 (race-prone but fast), which is strictly better than the user pkilling stuck processes. Also rewrites the misleading "Chrome 144+ chrome://inspect tip" in the auto-connect failure message — the toggle exposes target discovery only, not the /json/version HTTP API the auto-connect flow expects (verified by user: lsof shows :9222 listening but curl /json/version returns 404). --- cli/src/native/actions.rs | 10 ++++++---- cli/src/native/element.rs | 20 +++++++++++++++++--- cli/src/native/interaction.rs | 16 ++++++++++++---- 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/cli/src/native/actions.rs b/cli/src/native/actions.rs index d8b49ba..60014ba 100644 --- a/cli/src/native/actions.rs +++ b/cli/src/native/actions.rs @@ -1607,8 +1607,9 @@ async fn auto_launch(state: &mut DaemonState) -> Result<(), String> { To let agent-browser work with your existing Chrome (recommended):\n\ {}\n\n\ Or start a standalone browser with: agent-browser --launch open \n\n\ - Tip: On Chrome 144+, you can enable CDP without restarting:\n\ - Open chrome://inspect/#remote-debugging and toggle it on.", + Note: chrome://inspect/#remote-debugging only enables remote *target discovery* — \ + it does NOT expose the standard CDP HTTP API on /json/version. \ + A full restart with --remote-debugging-port= is required.", chrome_relaunch_hint(), )); } @@ -2135,8 +2136,9 @@ async fn handle_launch(cmd: &Value, state: &mut DaemonState) -> Result\n\n\ - Tip: On Chrome 144+, you can enable CDP without restarting:\n\ - Open chrome://inspect/#remote-debugging and toggle it on.", + Note: chrome://inspect/#remote-debugging only enables remote *target discovery* — \ + it does NOT expose the standard CDP HTTP API on /json/version. \ + A full restart with --remote-debugging-port= is required.", chrome_relaunch_hint(), )); } diff --git a/cli/src/native/element.rs b/cli/src/native/element.rs index 3c997d2..bbb5316 100644 --- a/cli/src/native/element.rs +++ b/cli/src/native/element.rs @@ -396,9 +396,23 @@ async fn verify_ref_identity( "backendNodeId": backend_node_id, "fetchRelatives": false, }); - let resp: Result = client - .send_command_typed("Accessibility.getPartialAXTree", ¶ms, Some(session_id)) - .await; + // Tight 1s timeout: this is a defensive guard, not a critical path. + // The default 30s CDP timeout was the dominant factor in the + // "click hangs 5+ minutes" report — three CDP calls (verify + + // resolveNode + paint-settle) at 30s each, multiplied by parallel + // click invocations queueing on the daemon, totalled multi-minute + // user-visible hangs. Cap our own helper so a stuck AX query + // doesn't make `click` worse than the no-guard version was. + let resp: Result = match tokio::time::timeout( + std::time::Duration::from_secs(1), + client.send_command_typed("Accessibility.getPartialAXTree", ¶ms, Some(session_id)), + ) + .await + { + Ok(r) => r, + // Timeout: skip identity verification rather than block the click. + Err(_) => return Ok(()), + }; let Ok(tree) = resp else { // Node likely gone; let the box-model call fail and trigger fallback. return Ok(()); diff --git a/cli/src/native/interaction.rs b/cli/src/native/interaction.rs index 3375ace..26758e6 100644 --- a/cli/src/native/interaction.rs +++ b/cli/src/native/interaction.rs @@ -903,8 +903,15 @@ async fn wait_for_paint_settled(client: &CdpClient, session_id: &str) { requestAnimationFrame(() => \ requestAnimationFrame(() => \ queueMicrotask(() => resolve(true)))))"; - let _ = client - .send_command_typed::<_, Value>( + // Tight 500ms timeout. RAF normally fires at 16ms, two RAFs total ~33ms. + // If the tab is hidden / throttled / page is doing something pathological + // and RAF doesn't fire in 500ms, we'd rather return now than stall the + // user's click. Without this cap, a stuck RAF inherited the default 30s + // CDP timeout and was the main contributor to the "click hangs 5+ min" + // user report. + let _ = tokio::time::timeout( + std::time::Duration::from_millis(500), + client.send_command_typed::<_, Value>( "Runtime.evaluate", &EvaluateParams { expression: script.to_string(), @@ -912,8 +919,9 @@ async fn wait_for_paint_settled(client: &CdpClient, session_id: &str) { await_promise: Some(true), }, Some(session_id), - ) - .await; + ), + ) + .await; } async fn dispatch_click(