diff --git a/cli/src/native/actions.rs b/cli/src/native/actions.rs index 6b2511c..ddf7ae5 100644 --- a/cli/src/native/actions.rs +++ b/cli/src/native/actions.rs @@ -166,6 +166,30 @@ struct DrainedEvents { detached_iframe_sessions: Vec, } +/// Compute a hash of the [`LaunchOptions`] fields that require a browser +/// relaunch when changed (baked into the Chrome process at startup). +/// +/// Fields NOT hashed (adjustable at runtime via CDP without relaunch): +/// ignore_https_errors, color_scheme, download_path, storage_state +fn launch_hash(opts: &LaunchOptions) -> u64 { + use std::collections::hash_map::DefaultHasher; + use std::hash::{Hash, Hasher}; + + let mut h = DefaultHasher::new(); + opts.headless.hash(&mut h); + opts.extensions.hash(&mut h); + opts.profile.hash(&mut h); + opts.executable_path.hash(&mut h); + opts.args.hash(&mut h); + opts.proxy.hash(&mut h); + opts.proxy_bypass.hash(&mut h); + opts.proxy_username.hash(&mut h); + opts.proxy_password.hash(&mut h); + opts.user_agent.hash(&mut h); + opts.allow_file_access.hash(&mut h); + h.finish() +} + pub struct DaemonState { pub browser: Option, pub appium: Option, @@ -218,6 +242,8 @@ pub struct DaemonState { pub stream_client: Option>>>>, /// Stream server instance kept alive so the broadcast channel remains open. pub stream_server: Option>, + /// Hash of launch options used for the current browser, for relaunch detection. + launch_hash: Option, /// Browser engine name (e.g. "chrome", "lightpanda") for observability. pub engine: String, } @@ -267,6 +293,7 @@ impl DaemonState { ), stream_client: None, stream_server: None, + launch_hash: None, engine: env::var("AGENT_BROWSER_ENGINE").unwrap_or_else(|_| "chrome".to_string()), } } @@ -1523,9 +1550,12 @@ async fn auto_launch(state: &mut DaemonState) -> Result<(), String> { } } } + + let hash = launch_hash(&options); let mgr = BrowserManager::launch(options, engine.as_deref()).await?; state.reset_input_state(); state.browser = Some(mgr); + state.launch_hash = Some(hash); state.subscribe_to_browser_events(); state.start_fetch_handler(); state.start_dialog_handler(); @@ -1616,12 +1646,95 @@ async fn handle_launch(cmd: &Value, state: &mut DaemonState) -> Result> = + cmd.get("extensions").and_then(|v| v.as_array()).map(|arr| { + arr.iter() + .filter_map(|v| v.as_str().map(String::from)) + .collect() + }); + let storage_state = cmd.get("storageState").and_then(|v| v.as_str()); + + let launch_options = LaunchOptions { + headless, + executable_path: cmd + .get("executablePath") + .and_then(|v| v.as_str()) + .map(|s| s.to_string()) + .or_else(|| env::var("AGENT_BROWSER_EXECUTABLE_PATH").ok()), + proxy: cmd.get("proxy").and_then(|v| { + v.as_str().map(|s| s.to_string()).or_else(|| { + v.get("server") + .and_then(|s| s.as_str()) + .map(|s| s.to_string()) + }) + }), + proxy_bypass: cmd + .get("proxy") + .and_then(|v| v.get("bypass")) + .and_then(|v| v.as_str()) + .map(String::from), + proxy_username: cmd + .get("proxy") + .and_then(|v| v.get("username")) + .and_then(|v| v.as_str()) + .map(String::from) + .or_else(|| env::var("AGENT_BROWSER_PROXY_USERNAME").ok()), + proxy_password: cmd + .get("proxy") + .and_then(|v| v.get("password")) + .and_then(|v| v.as_str()) + .map(String::from) + .or_else(|| env::var("AGENT_BROWSER_PROXY_PASSWORD").ok()), + profile: cmd + .get("profile") + .and_then(|v| v.as_str()) + .map(|s| s.to_string()), + allow_file_access: cmd + .get("allowFileAccess") + .and_then(|v| v.as_bool()) + .unwrap_or(false), + args: cmd + .get("args") + .and_then(|v| v.as_array()) + .map(|arr| { + arr.iter() + .filter_map(|v| v.as_str().map(|s| s.to_string())) + .collect() + }) + .unwrap_or_default(), + extensions, + storage_state: storage_state.map(String::from), + user_agent: cmd + .get("userAgent") + .and_then(|v| v.as_str()) + .map(String::from), + ignore_https_errors: cmd + .get("ignoreHTTPSErrors") + .and_then(|v| v.as_bool()) + .unwrap_or(false), + color_scheme: cmd + .get("colorScheme") + .and_then(|v| v.as_str()) + .map(String::from), + download_path: cmd + .get("downloadPath") + .and_then(|v| v.as_str()) + .map(String::from), + }; + + let new_hash = launch_hash(&launch_options); + + // Hash comparison and fast process-exit check are evaluated before the + // async is_connection_alive to skip the expensive CDP liveness probe + // when a relaunch is already certain. let needs_relaunch = if let Some(ref mut mgr) = state.browser { let is_external = cdp_url.is_some() || cdp_port.is_some() || auto_connect; let was_external = mgr.is_cdp_connection(); - is_external != was_external || mgr.has_process_exited() || !mgr.is_connection_alive().await + let hash_changed = !is_external && state.launch_hash != Some(new_hash); + is_external != was_external + || hash_changed + || mgr.has_process_exited() + || !mgr.is_connection_alive().await } else { true }; @@ -1630,6 +1743,7 @@ async fn handle_launch(cmd: &Value, state: &mut DaemonState) -> Result Result> = - cmd.get("extensions").and_then(|v| v.as_array()).map(|arr| { - arr.iter() - .filter_map(|v| v.as_str().map(String::from)) - .collect() - }); - - let profile = cmd.get("profile").and_then(|v| v.as_str()); - let storage_state = cmd.get("storageState").and_then(|v| v.as_str()); - let allow_file_access = cmd - .get("allowFileAccess") - .and_then(|v| v.as_bool()) - .unwrap_or(false); - let executable_path: Option = cmd - .get("executablePath") - .and_then(|v| v.as_str()) - .map(String::from) - .or_else(|| std::env::var("AGENT_BROWSER_EXECUTABLE_PATH").ok()); let has_cdp = cdp_url.is_some() || cdp_port.is_some(); super::browser::validate_launch_options( - extensions.as_deref(), + launch_options.extensions.as_deref(), has_cdp, - profile, + launch_options.profile.as_deref(), storage_state, - allow_file_access, - executable_path.as_deref(), + launch_options.allow_file_access, + launch_options.executable_path.as_deref(), )?; if let Some(url) = cdp_url { @@ -1759,81 +1855,13 @@ async fn handle_launch(cmd: &Value, state: &mut DaemonState) -> Result Result Result { mgr.close().await?; } state.browser = None; + state.launch_hash = None; state.screencasting = false; state.reset_input_state(); state.update_stream_client().await; diff --git a/cli/src/native/e2e_tests.rs b/cli/src/native/e2e_tests.rs index c50a46f..b170cbf 100644 --- a/cli/src/native/e2e_tests.rs +++ b/cli/src/native/e2e_tests.rs @@ -3822,3 +3822,65 @@ async fn e2e_externally_opened_tab_detected() { let resp = execute_command(&json!({ "id": "99", "action": "close" }), &mut state).await; assert_success(&resp); } + +// --------------------------------------------------------------------------- +// Regression: issue #993 — launch options change must trigger relaunch +// --------------------------------------------------------------------------- + +/// When the browser is already running and a second launch command arrives with +/// different options (e.g., extensions added), the daemon must relaunch the +/// browser instead of silently reusing the old one. +/// +/// Before the fix, `handle_launch` only checked connection type and liveness, +/// so changed options like extensions were ignored and the old browser was reused. +#[tokio::test] +#[ignore] +async fn e2e_relaunch_on_options_change() { + let mut state = DaemonState::new(); + + // First launch — headless, no extensions. + let resp = execute_command( + &json!({ "id": "1", "action": "launch", "headless": true }), + &mut state, + ) + .await; + assert_success(&resp); + assert_eq!(get_data(&resp)["launched"], true); + assert!( + get_data(&resp).get("reused").is_none(), + "first launch must not be a reuse" + ); + + // Second launch — same options → should reuse. + let resp = execute_command( + &json!({ "id": "2", "action": "launch", "headless": true }), + &mut state, + ) + .await; + assert_success(&resp); + assert_eq!( + get_data(&resp)["reused"], + true, + "identical options must reuse the browser" + ); + + // Third launch — different options (extensions added) → must relaunch, not reuse. + let resp = execute_command( + &json!({ + "id": "3", + "action": "launch", + "headless": false, + "extensions": ["/tmp/fake-extension"] + }), + &mut state, + ) + .await; + assert_success(&resp); + assert!( + get_data(&resp).get("reused").is_none(), + "changed options must trigger a relaunch, not reuse (issue #993)" + ); + + let resp = execute_command(&json!({ "id": "99", "action": "close" }), &mut state).await; + assert_success(&resp); +}