From b02e485a37e60a9a564d8cf57a4f82ec06bddd4f Mon Sep 17 00:00:00 2001 From: "jin.2" Date: Thu, 16 Apr 2026 07:50:11 +0900 Subject: [PATCH] fix: prefer DevToolsActivePort websocket path over HTTP discovery in --auto-connect (#1218) * fix: prefer DevToolsActivePort websocket path over HTTP discovery in --auto-connect Reverses the discovery order in `auto_connect_cdp()` so the exact WebSocket path from DevToolsActivePort is tried first, falling back to legacy HTTP endpoints (`/json/version`, `/json/list`) only when the direct path fails. This eliminates the duplicate remote-debugging permission prompts caused by unnecessary HTTP probes on Chrome M144+. Also adds `verify_ws_endpoint()` to validate the WebSocket URL is a live CDP server before returning it, preventing stale URLs from being handed to callers. Fixes #1210 Fixes #1206 * chore: remove unrelated issue references from test comment * style: apply rustfmt --------- Co-authored-by: hyunjinee --- cli/src/native/cdp/chrome.rs | 159 ++++++++++++++++++++++++++++++++--- 1 file changed, 145 insertions(+), 14 deletions(-) diff --git a/cli/src/native/cdp/chrome.rs b/cli/src/native/cdp/chrome.rs index 6f5e40d..facdf61 100644 --- a/cli/src/native/cdp/chrome.rs +++ b/cli/src/native/cdp/chrome.rs @@ -654,16 +654,7 @@ pub async fn auto_connect_cdp() -> Result { for dir in &user_data_dirs { if let Some((port, ws_path)) = read_devtools_active_port(dir) { - // Try HTTP endpoint first (pre-M144) - if let Ok(ws_url) = discover_cdp_url("127.0.0.1", port, None).await { - return Ok(ws_url); - } - // M144+: direct WebSocket — verify the port is actually listening - // before returning, otherwise a stale DevToolsActivePort file - // (left behind after Chrome exits/crashes) produces a confusing - // "connection refused" error instead of falling through. - if is_port_reachable(port) { - let ws_url = format!("ws://127.0.0.1:{}{}", port, ws_path); + if let Ok(ws_url) = resolve_cdp_from_active_port(port, &ws_path).await { return Ok(ws_url); } // Port is dead — remove the stale file so future runs skip it. @@ -682,10 +673,54 @@ pub async fn auto_connect_cdp() -> Result { Err("No running Chrome instance found. Launch Chrome with --remote-debugging-port or use --cdp.".to_string()) } -fn is_port_reachable(port: u16) -> bool { - use std::net::TcpStream; - let addr = format!("127.0.0.1:{}", port); - TcpStream::connect_timeout(&addr.parse().unwrap(), Duration::from_millis(500)).is_ok() +/// Resolve a CDP WebSocket URL from a DevToolsActivePort entry. +/// +/// Tries the exact WebSocket path from DevToolsActivePort first (single +/// prompt on M144+), then falls back to legacy HTTP discovery for older +/// Chrome versions. This order avoids triggering duplicate remote-debugging +/// permission prompts (#1210, #1206). +async fn resolve_cdp_from_active_port(port: u16, ws_path: &str) -> Result { + let ws_url = format!("ws://127.0.0.1:{}{}", port, ws_path); + if verify_ws_endpoint(&ws_url).await { + return Ok(ws_url); + } + + // Pre-M144 fallback: HTTP endpoints (/json/version, /json/list, etc.) + if let Ok(ws_url) = discover_cdp_url("127.0.0.1", port, None).await { + return Ok(ws_url); + } + + Err(format!( + "Cannot connect to Chrome on port {}: both direct WebSocket and HTTP discovery failed", + port + )) +} + +/// Verify that a WebSocket endpoint is a live CDP server by sending +/// `Browser.getVersion` and checking for a valid response. +async fn verify_ws_endpoint(ws_url: &str) -> bool { + use futures_util::{SinkExt, StreamExt}; + use tokio_tungstenite::tungstenite::Message; + + let timeout = Duration::from_secs(2); + let result = tokio::time::timeout(timeout, async { + let (mut ws, _) = tokio_tungstenite::connect_async(ws_url).await.ok()?; + let cmd = r#"{"id":1,"method":"Browser.getVersion"}"#; + ws.send(Message::Text(cmd.into())).await.ok()?; + while let Some(Ok(msg)) = ws.next().await { + if let Message::Text(text) = msg { + if let Ok(v) = serde_json::from_str::(&text) { + if v.get("id").and_then(|id| id.as_u64()) == Some(1) { + let _ = ws.close(None).await; + return Some(()); + } + } + } + } + None + }) + .await; + matches!(result, Ok(Some(()))) } /// Returns the default Chrome user-data directory paths for the current platform. @@ -1826,4 +1861,100 @@ mod tests { "profile path should keep keychain flags" ); } + + // ------------------------------------------------------------------- + // auto_connect_cdp discovery-order tests (#1210, #1206) + // ------------------------------------------------------------------- + + /// When DevToolsActivePort provides a ws_path and the port is reachable, + /// `resolve_cdp_from_active_port` should return the exact ws_path URL + /// WITHOUT calling HTTP discovery first. + #[tokio::test] + async fn test_resolve_cdp_from_active_port_prefers_ws_path() { + use futures_util::{SinkExt, StreamExt}; + use tokio_tungstenite::tungstenite::Message as WsMsg; + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let port = listener.local_addr().unwrap().port(); + let ws_path = "/devtools/browser/test-uuid-1234".to_string(); + + let server = tokio::spawn(async move { + // accept: verify_ws_endpoint() WebSocket handshake + let (stream, _) = listener.accept().await.unwrap(); + let mut ws = tokio_tungstenite::accept_async(stream).await.unwrap(); + if let Some(Ok(WsMsg::Text(text))) = ws.next().await { + let req: serde_json::Value = serde_json::from_str(&text).unwrap(); + let id = req.get("id").unwrap(); + let reply = format!( + r#"{{"id":{},"result":{{"protocolVersion":"1.3","product":"Chrome/147"}}}}"#, + id + ); + ws.send(WsMsg::Text(reply)).await.unwrap(); + } + let _ = ws.close(None).await; + }); + + let result = resolve_cdp_from_active_port(port, &ws_path).await; + assert!(result.is_ok(), "should succeed: {:?}", result); + let url = result.unwrap(); + assert!( + url.contains("test-uuid-1234"), + "should use exact ws_path from DevToolsActivePort, got: {}", + url + ); + assert_eq!(url, format!("ws://127.0.0.1:{}{}", port, ws_path)); + server.await.unwrap(); + } + + /// When the exact ws_path connection fails, `resolve_cdp_from_active_port` + /// should fall back to HTTP discovery. + #[tokio::test] + async fn test_resolve_cdp_from_active_port_falls_back_to_http_discovery() { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let port = listener.local_addr().unwrap().port(); + + let server = tokio::spawn(async move { + // 1st accept: verify_ws_endpoint() ws_path probe — reject (just close) + let (s1, _) = listener.accept().await.unwrap(); + drop(s1); + + // 2nd accept: HTTP /json/version from discover_cdp_url() + let (mut s2, _) = listener.accept().await.unwrap(); + let mut buf = [0u8; 2048]; + let _ = s2.read(&mut buf).await; + let body = format!( + r#"{{"webSocketDebuggerUrl":"ws://127.0.0.1:{}/devtools/browser/fallback-uuid"}}"#, + port + ); + let resp = format!( + "HTTP/1.1 200 OK\r\nContent-Length: {}\r\nContent-Type: application/json\r\n\r\n{}", + body.len(), + body + ); + s2.write_all(resp.as_bytes()).await.unwrap(); + }); + + let result = resolve_cdp_from_active_port(port, "/devtools/browser/nonexistent-uuid").await; + assert!(result.is_ok(), "should fall back to HTTP: {:?}", result); + let url = result.unwrap(); + assert!( + url.contains("fallback-uuid"), + "should use HTTP discovery fallback, got: {}", + url + ); + server.await.unwrap(); + } + + /// When neither ws_path nor HTTP discovery works, return an error. + #[tokio::test] + async fn test_resolve_cdp_from_active_port_both_fail() { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let port = listener.local_addr().unwrap().port(); + drop(listener); + + let result = resolve_cdp_from_active_port(port, "/devtools/browser/dead").await; + assert!(result.is_err(), "should fail when nothing is listening"); + } }