* fix: Windows auto-connect profiling (#835) Fix three interrelated bugs causing `--auto-connect` to fail on Windows, plus a UX issue where auto-connect hijacked existing tabs: 1. Stale DevToolsActivePort — add TCP port liveness check before returning M144+ WebSocket URL; remove stale files when port is dead. 2. Missing Windows error codes — add os error 10061 (WSAECONNREFUSED) and 10054 (WSAECONNRESET) to is_transient_error() so daemon startup races are retried on Windows. 3. --auto-connect not propagated to daemon — add auto_connect to DaemonOptions, set AGENT_BROWSER_AUTO_CONNECT env var via apply_daemon_env(), and guard the headed launch block so it doesn't send a second launch that overrides the auto-connect. 4. Auto-connect opens a fresh tab — after connecting to an existing Chrome, create a new about:blank tab and bring it to front so navigations don't hijack the user's existing tabs. Made-with: Cursor * fix: address review feedback — cargo fmt, shared helper, Windows tests - Run cargo fmt on is_port_reachable() formatting - Extract duplicated auto-connect-with-fresh-tab logic into connect_auto_with_fresh_tab() helper used by both handle_launch() and auto_launch() - Add unit tests for Windows WSAECONNREFUSED (os error 10061) and WSAECONNRESET (os error 10054) in is_transient_error() --------- Co-authored-by: ctate <366502+ctate@users.noreply.github.com>
This commit is contained in:
@@ -234,6 +234,7 @@ pub struct DaemonOptions<'a> {
|
|||||||
pub action_policy: Option<&'a str>,
|
pub action_policy: Option<&'a str>,
|
||||||
pub confirm_actions: Option<&'a str>,
|
pub confirm_actions: Option<&'a str>,
|
||||||
pub engine: Option<&'a str>,
|
pub engine: Option<&'a str>,
|
||||||
|
pub auto_connect: bool,
|
||||||
pub idle_timeout: Option<&'a str>,
|
pub idle_timeout: Option<&'a str>,
|
||||||
pub cdp: Option<&'a str>,
|
pub cdp: Option<&'a str>,
|
||||||
}
|
}
|
||||||
@@ -302,6 +303,9 @@ fn apply_daemon_env(cmd: &mut Command, session: &str, opts: &DaemonOptions) {
|
|||||||
if let Some(engine) = opts.engine {
|
if let Some(engine) = opts.engine {
|
||||||
cmd.env("AGENT_BROWSER_ENGINE", engine);
|
cmd.env("AGENT_BROWSER_ENGINE", engine);
|
||||||
}
|
}
|
||||||
|
if opts.auto_connect {
|
||||||
|
cmd.env("AGENT_BROWSER_AUTO_CONNECT", "1");
|
||||||
|
}
|
||||||
if let Some(idle) = opts.idle_timeout {
|
if let Some(idle) = opts.idle_timeout {
|
||||||
cmd.env("AGENT_BROWSER_IDLE_TIMEOUT_MS", idle);
|
cmd.env("AGENT_BROWSER_IDLE_TIMEOUT_MS", idle);
|
||||||
}
|
}
|
||||||
@@ -533,6 +537,8 @@ fn is_transient_error(error: &str) -> bool {
|
|||||||
|| error.contains("os error 2") // No such file or directory (socket gone)
|
|| error.contains("os error 2") // No such file or directory (socket gone)
|
||||||
|| error.contains("os error 61") // Connection refused (macOS)
|
|| error.contains("os error 61") // Connection refused (macOS)
|
||||||
|| error.contains("os error 111") // Connection refused (Linux)
|
|| error.contains("os error 111") // Connection refused (Linux)
|
||||||
|
|| error.contains("os error 10061") // Connection refused (Windows)
|
||||||
|
|| error.contains("os error 10054") // Connection reset by peer (Windows)
|
||||||
}
|
}
|
||||||
|
|
||||||
fn send_command_once(cmd: &Value, session: &str) -> Result<Response, String> {
|
fn send_command_once(cmd: &Value, session: &str) -> Result<Response, String> {
|
||||||
@@ -708,6 +714,20 @@ mod tests {
|
|||||||
));
|
));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_is_transient_error_connection_refused_windows() {
|
||||||
|
assert!(is_transient_error(
|
||||||
|
"Failed to connect: No connection could be made because the target machine actively refused it. (os error 10061)"
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_is_transient_error_connection_reset_windows() {
|
||||||
|
assert!(is_transient_error(
|
||||||
|
"Failed to send: An existing connection was forcibly closed by the remote host. (os error 10054)"
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_is_transient_error_non_transient() {
|
fn test_is_transient_error_non_transient() {
|
||||||
// These should NOT be considered transient
|
// These should NOT be considered transient
|
||||||
|
|||||||
@@ -316,6 +316,7 @@ fn main() {
|
|||||||
action_policy: flags.action_policy.as_deref(),
|
action_policy: flags.action_policy.as_deref(),
|
||||||
confirm_actions: flags.confirm_actions.as_deref(),
|
confirm_actions: flags.confirm_actions.as_deref(),
|
||||||
engine: flags.engine.as_deref(),
|
engine: flags.engine.as_deref(),
|
||||||
|
auto_connect: flags.auto_connect,
|
||||||
idle_timeout: flags.idle_timeout.as_deref(),
|
idle_timeout: flags.idle_timeout.as_deref(),
|
||||||
cdp: flags.cdp.as_deref(),
|
cdp: flags.cdp.as_deref(),
|
||||||
};
|
};
|
||||||
@@ -617,6 +618,7 @@ fn main() {
|
|||||||
|| !flags.extensions.is_empty())
|
|| !flags.extensions.is_empty())
|
||||||
&& flags.cdp.is_none()
|
&& flags.cdp.is_none()
|
||||||
&& flags.provider.is_none()
|
&& flags.provider.is_none()
|
||||||
|
&& !flags.auto_connect
|
||||||
{
|
{
|
||||||
let mut launch_cmd = json!({
|
let mut launch_cmd = json!({
|
||||||
"id": gen_id(),
|
"id": gen_id(),
|
||||||
|
|||||||
@@ -792,6 +792,19 @@ pub async fn execute_command(cmd: &Value, state: &mut DaemonState) -> Value {
|
|||||||
// Auto-launch
|
// Auto-launch
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
/// Connect to a running Chrome via auto-discovery and open a fresh tab so
|
||||||
|
/// subsequent navigations don't hijack the user's existing tabs.
|
||||||
|
async fn connect_auto_with_fresh_tab() -> Result<BrowserManager, String> {
|
||||||
|
let mut mgr = BrowserManager::connect_auto().await?;
|
||||||
|
mgr.tab_new(None).await?;
|
||||||
|
let session_id = mgr.active_session_id()?.to_string();
|
||||||
|
let _ = mgr
|
||||||
|
.client
|
||||||
|
.send_command("Page.bringToFront", None, Some(&session_id))
|
||||||
|
.await;
|
||||||
|
Ok(mgr)
|
||||||
|
}
|
||||||
|
|
||||||
async fn auto_launch(state: &mut DaemonState) -> Result<(), String> {
|
async fn auto_launch(state: &mut DaemonState) -> Result<(), String> {
|
||||||
let options = launch_options_from_env();
|
let options = launch_options_from_env();
|
||||||
let engine = env::var("AGENT_BROWSER_ENGINE").ok();
|
let engine = env::var("AGENT_BROWSER_ENGINE").ok();
|
||||||
@@ -806,8 +819,7 @@ async fn auto_launch(state: &mut DaemonState) -> Result<(), String> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if env::var("AGENT_BROWSER_AUTO_CONNECT").is_ok() {
|
if env::var("AGENT_BROWSER_AUTO_CONNECT").is_ok() {
|
||||||
let mgr = BrowserManager::connect_auto().await?;
|
state.browser = Some(connect_auto_with_fresh_tab().await?);
|
||||||
state.browser = Some(mgr);
|
|
||||||
state.subscribe_to_browser_events();
|
state.subscribe_to_browser_events();
|
||||||
state.update_stream_client().await;
|
state.update_stream_client().await;
|
||||||
try_auto_restore_state(state).await;
|
try_auto_restore_state(state).await;
|
||||||
@@ -971,7 +983,7 @@ async fn handle_launch(cmd: &Value, state: &mut DaemonState) -> Result<Value, St
|
|||||||
}
|
}
|
||||||
|
|
||||||
if auto_connect {
|
if auto_connect {
|
||||||
state.browser = Some(BrowserManager::connect_auto().await?);
|
state.browser = Some(connect_auto_with_fresh_tab().await?);
|
||||||
state.subscribe_to_browser_events();
|
state.subscribe_to_browser_events();
|
||||||
state.update_stream_client().await;
|
state.update_stream_client().await;
|
||||||
return Ok(json!({ "launched": true }));
|
return Ok(json!({ "launched": true }));
|
||||||
|
|||||||
@@ -453,9 +453,17 @@ pub async fn auto_connect_cdp() -> Result<String, String> {
|
|||||||
if let Ok(ws_url) = discover_cdp_url("127.0.0.1", port).await {
|
if let Ok(ws_url) = discover_cdp_url("127.0.0.1", port).await {
|
||||||
return Ok(ws_url);
|
return Ok(ws_url);
|
||||||
}
|
}
|
||||||
// M144+: direct WebSocket
|
// M144+: direct WebSocket — verify the port is actually listening
|
||||||
let ws_url = format!("ws://127.0.0.1:{}{}", port, ws_path);
|
// before returning, otherwise a stale DevToolsActivePort file
|
||||||
return Ok(ws_url);
|
// (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);
|
||||||
|
return Ok(ws_url);
|
||||||
|
}
|
||||||
|
// Port is dead — remove the stale file so future runs skip it.
|
||||||
|
let stale = dir.join("DevToolsActivePort");
|
||||||
|
let _ = std::fs::remove_file(&stale);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -469,6 +477,12 @@ pub async fn auto_connect_cdp() -> Result<String, String> {
|
|||||||
Err("No running Chrome instance found. Launch Chrome with --remote-debugging-port or use --cdp.".to_string())
|
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()
|
||||||
|
}
|
||||||
|
|
||||||
fn get_chrome_user_data_dirs() -> Vec<PathBuf> {
|
fn get_chrome_user_data_dirs() -> Vec<PathBuf> {
|
||||||
let mut dirs = Vec::new();
|
let mut dirs = Vec::new();
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user