From 3d032f9e88b5cadb9e4c6bfbdaac9b493b94bbba Mon Sep 17 00:00:00 2001 From: leeguooooo Date: Mon, 1 Jun 2026 11:36:00 +0900 Subject: [PATCH] fix(connect): stop remote-debugging consent storm on transient liveness timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The daemon re-validates the CDP connection before every browsing command via is_connection_alive() (Browser.getVersion, 3s timeout). It treated any timeout-or-error as "dead" and tore the connection down + reconnected. For an externally-attached browser (the stealth fork's default — the user's real Chrome), a timed-out probe is almost always Chrome being briefly busy or showing the Chrome 136+ "Allow remote debugging?" consent modal, which blocks CDP responses until the user clicks Allow. Tearing the already-consented connection down forces a reconnect that re-pops the consent prompt — repeated on every command this becomes an endless prompt loop, and the close + multiple new /devtools/browser WS probes storm Chrome into a freeze. Fix: distinguish the probe outcome. - Responded -> alive - TransportError -> dead (WS closed/reset; user closing Chrome lands here too, so zombie-socket detection is preserved) - TimedOut -> alive for an external attach (don't tear down a consented connection on transient slowness); dead for a browser we launched ourselves (a real hang worth reconnecting, and no consent modal in play). Extracted the verdict into a pure connection_alive_from_probe() with unit tests covering all outcomes. No behavior change for locally-launched browsers. --- cli/src/native/browser.rs | 89 +++++++++++++++++++++++++++++++++++---- 1 file changed, 80 insertions(+), 9 deletions(-) diff --git a/cli/src/native/browser.rs b/cli/src/native/browser.rs index 8e6e1be..fcdf55f 100644 --- a/cli/src/native/browser.rs +++ b/cli/src/native/browser.rs @@ -311,6 +311,42 @@ const LIGHTPANDA_CDP_CONNECT_TIMEOUT: Duration = Duration::from_secs(5); const LIGHTPANDA_CDP_CONNECT_POLL_INTERVAL: Duration = Duration::from_millis(100); const LIGHTPANDA_TARGET_INIT_TIMEOUT: Duration = Duration::from_secs(10); +/// Outcome of a single `Browser.getVersion` liveness probe. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum LivenessProbe { + /// Chrome answered — the connection is definitely alive. + Responded, + /// The CDP transport errored (WebSocket closed/reset) — the socket is gone. + TransportError, + /// The probe timed out with no response. + TimedOut, +} + +/// Decide whether a CDP connection should be considered alive from one probe. +/// +/// The subtle case is [`LivenessProbe::TimedOut`]. For a browser we launched +/// ourselves (`is_external_attach == false`) a hung CDP socket is a real +/// problem and the daemon should reconnect. But for an *externally attached* +/// browser — the stealth fork's default, where we attach to the user's real +/// Chrome — a slow/no response is almost always Chrome being briefly busy or, +/// critically, showing the Chrome 136+ "Allow remote debugging?" consent modal, +/// which blocks CDP responses until the user clicks Allow. +/// +/// Treating that timeout as "dead" tears down the already-consented connection +/// and forces a reconnect, which re-pops the consent prompt; repeated on every +/// command it produces an endless prompt loop and a connection storm that can +/// freeze Chrome. So for external attaches we keep the connection alive on +/// timeout. A genuinely dead external socket instead surfaces as +/// [`LivenessProbe::TransportError`] (and Chrome being closed by the user is a +/// transport error, not a timeout), so zombie-socket detection is preserved. +fn connection_alive_from_probe(probe: LivenessProbe, is_external_attach: bool) -> bool { + match probe { + LivenessProbe::Responded => true, + LivenessProbe::TransportError => false, + LivenessProbe::TimedOut => is_external_attach, + } +} + impl BrowserManager { pub async fn launch(options: LaunchOptions, engine: Option<&str>) -> Result { let engine = engine.unwrap_or("chrome"); @@ -829,21 +865,27 @@ impl BrowserManager { self.default_timeout_ms } - /// Checks if the CDP connection is alive by sending a simple command. - /// Returns false if the command times out or fails. + /// Checks if the CDP connection is alive by sending a `Browser.getVersion` + /// probe. See [`connection_alive_from_probe`] for how the outcome maps to a + /// liveness verdict — in particular why a timeout does NOT tear down an + /// externally-attached browser. pub async fn is_connection_alive(&self) -> bool { let timeout = tokio::time::Duration::from_secs(3); - let result = tokio::time::timeout( + let probe = match tokio::time::timeout( timeout, self.client .send_command_no_params("Browser.getVersion", None), ) - .await; - - match result { - Ok(Ok(_)) => true, - Ok(Err(_)) | Err(_) => false, - } + .await + { + Ok(Ok(_)) => LivenessProbe::Responded, + Ok(Err(_)) => LivenessProbe::TransportError, + Err(_) => LivenessProbe::TimedOut, + }; + // No child process => we attached to an external browser (the user's + // real Chrome — the stealth fork's default). + let is_external_attach = self.browser_process.is_none(); + connection_alive_from_probe(probe, is_external_attach) } /// Non-blocking check whether the locally-launched browser process has exited @@ -1728,6 +1770,35 @@ mod tests { assert_eq!(format_tab_id(42), "t42"); } + #[test] + fn liveness_responded_is_alive_for_both_kinds() { + assert!(connection_alive_from_probe(LivenessProbe::Responded, true)); + assert!(connection_alive_from_probe(LivenessProbe::Responded, false)); + } + + #[test] + fn liveness_transport_error_is_dead_for_both_kinds() { + // A closed/reset WebSocket is a genuine death — reconnect in both cases. + assert!(!connection_alive_from_probe(LivenessProbe::TransportError, true)); + assert!(!connection_alive_from_probe(LivenessProbe::TransportError, false)); + } + + #[test] + fn liveness_timeout_keeps_external_attach_alive() { + // Regression guard for the remote-debugging consent storm: a timed-out + // probe must NOT tear down an externally-attached browser, otherwise the + // daemon reconnects and re-pops Chrome's "Allow remote debugging?" modal + // on every command (endless prompts + browser freeze). + assert!(connection_alive_from_probe(LivenessProbe::TimedOut, true)); + } + + #[test] + fn liveness_timeout_marks_launched_browser_dead() { + // A browser we launched that stops responding is a real problem worth a + // reconnect (and has no consent modal to worry about). + assert!(!connection_alive_from_probe(LivenessProbe::TimedOut, false)); + } + #[test] fn test_parse_tab_ref_id() { assert_eq!(TabRef::parse("t1"), Ok(TabRef::Id(1)));