From e5fd26eb9e1992ef8e779d5d6661aa152fa408f1 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Tue, 3 Mar 2026 22:34:07 -0600 Subject: [PATCH] headed mode (#607) * headed mode * fixes * fixes * docs * fixes * fixes * fixes --- README.md | 4 +- cli/src/connection.rs | 53 ++----- cli/src/main.rs | 2 + cli/src/native/actions.rs | 24 ++- cli/src/native/cdp/chrome.rs | 229 +++++++++++++++++++++++++--- cli/src/native/policy.rs | 5 +- cli/src/output.rs | 2 +- cli/src/test_utils.rs | 49 ++++++ docs/src/app/configuration/page.mdx | 2 + skills/agent-browser/SKILL.md | 2 + src/browser.ts | 2 +- src/daemon.ts | 6 +- 12 files changed, 308 insertions(+), 72 deletions(-) create mode 100644 cli/src/test_utils.rs diff --git a/README.md b/README.md index 8c8cb67..530ca62 100644 --- a/README.md +++ b/README.md @@ -484,7 +484,7 @@ This is useful for multimodal AI models that can reason about visual layout, unl | `--json` | JSON output (for agents) | | `--full, -f` | Full page screenshot | | `--annotate` | Annotated screenshot with numbered element labels (or `AGENT_BROWSER_ANNOTATE` env) | -| `--headed` | Show browser window (not headless) | +| `--headed` | Show browser window (not headless) (or `AGENT_BROWSER_HEADED` env) | | `--cdp ` | Connect via Chrome DevTools Protocol (port or WebSocket URL) | | `--auto-connect` | Auto-discover and connect to running Chrome (or `AGENT_BROWSER_AUTO_CONNECT` env) | | `--color-scheme ` | Color scheme: `dark`, `light`, `no-preference` (or `AGENT_BROWSER_COLOR_SCHEME` env) | @@ -658,6 +658,8 @@ agent-browser open example.com --headed This opens a visible browser window instead of running headless. +> **Note:** Browser extensions work in both headed and headless mode (Chrome's `--headless=new`). + ## Authenticated Sessions Use `--headers` to set HTTP headers for a specific origin, enabling authentication without login flows: diff --git a/cli/src/connection.rs b/cli/src/connection.rs index 882da0e..3d9b991 100644 --- a/cli/src/connection.rs +++ b/cli/src/connection.rs @@ -593,45 +593,14 @@ fn send_command_once(cmd: &Value, session: &str) -> Result { #[cfg(test)] mod tests { use super::*; - use std::sync::{Mutex, MutexGuard}; - - // Mutex to prevent parallel tests from interfering with env vars - static ENV_MUTEX: Mutex<()> = Mutex::new(()); - - /// RAII guard that locks env mutex and restores env vars on drop - struct EnvGuard<'a> { - _lock: MutexGuard<'a, ()>, - vars: Vec<(String, Option)>, - } - - impl<'a> EnvGuard<'a> { - fn new(var_names: &[&str]) -> Self { - let lock = ENV_MUTEX.lock().unwrap(); - let vars = var_names - .iter() - .map(|&name| (name.to_string(), env::var(name).ok())) - .collect(); - Self { _lock: lock, vars } - } - } - - impl Drop for EnvGuard<'_> { - fn drop(&mut self) { - for (name, value) in &self.vars { - match value { - Some(v) => env::set_var(name, v), - None => env::remove_var(name), - } - } - } - } + use crate::test_utils::EnvGuard; #[test] fn test_get_socket_dir_explicit_override() { let _guard = EnvGuard::new(&["AGENT_BROWSER_SOCKET_DIR", "XDG_RUNTIME_DIR"]); - env::set_var("AGENT_BROWSER_SOCKET_DIR", "/custom/socket/path"); - env::remove_var("XDG_RUNTIME_DIR"); + _guard.set("AGENT_BROWSER_SOCKET_DIR", "/custom/socket/path"); + _guard.remove("XDG_RUNTIME_DIR"); assert_eq!(get_socket_dir(), PathBuf::from("/custom/socket/path")); } @@ -640,8 +609,8 @@ mod tests { fn test_get_socket_dir_ignores_empty_socket_dir() { let _guard = EnvGuard::new(&["AGENT_BROWSER_SOCKET_DIR", "XDG_RUNTIME_DIR"]); - env::set_var("AGENT_BROWSER_SOCKET_DIR", ""); - env::remove_var("XDG_RUNTIME_DIR"); + _guard.set("AGENT_BROWSER_SOCKET_DIR", ""); + _guard.remove("XDG_RUNTIME_DIR"); assert!(get_socket_dir() .to_string_lossy() @@ -652,8 +621,8 @@ mod tests { fn test_get_socket_dir_xdg_runtime() { let _guard = EnvGuard::new(&["AGENT_BROWSER_SOCKET_DIR", "XDG_RUNTIME_DIR"]); - env::remove_var("AGENT_BROWSER_SOCKET_DIR"); - env::set_var("XDG_RUNTIME_DIR", "/run/user/1000"); + _guard.remove("AGENT_BROWSER_SOCKET_DIR"); + _guard.set("XDG_RUNTIME_DIR", "/run/user/1000"); assert_eq!( get_socket_dir(), @@ -665,8 +634,8 @@ mod tests { fn test_get_socket_dir_ignores_empty_xdg_runtime() { let _guard = EnvGuard::new(&["AGENT_BROWSER_SOCKET_DIR", "XDG_RUNTIME_DIR"]); - env::set_var("AGENT_BROWSER_SOCKET_DIR", ""); - env::set_var("XDG_RUNTIME_DIR", ""); + _guard.set("AGENT_BROWSER_SOCKET_DIR", ""); + _guard.set("XDG_RUNTIME_DIR", ""); assert!(get_socket_dir() .to_string_lossy() @@ -677,8 +646,8 @@ mod tests { fn test_get_socket_dir_home_fallback() { let _guard = EnvGuard::new(&["AGENT_BROWSER_SOCKET_DIR", "XDG_RUNTIME_DIR"]); - env::remove_var("AGENT_BROWSER_SOCKET_DIR"); - env::remove_var("XDG_RUNTIME_DIR"); + _guard.remove("AGENT_BROWSER_SOCKET_DIR"); + _guard.remove("XDG_RUNTIME_DIR"); let result = get_socket_dir(); assert!(result.to_string_lossy().ends_with(".agent-browser")); diff --git a/cli/src/main.rs b/cli/src/main.rs index 98695b5..f6476f5 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -5,6 +5,8 @@ mod flags; mod install; mod native; mod output; +#[cfg(test)] +mod test_utils; mod validation; use serde_json::json; diff --git a/cli/src/native/actions.rs b/cli/src/native/actions.rs index 34680ec..b1b032a 100644 --- a/cli/src/native/actions.rs +++ b/cli/src/native/actions.rs @@ -755,6 +755,13 @@ fn launch_options_from_env() -> LaunchOptions { .map(|v| v == "1" || v == "true") .unwrap_or(false); + let extensions: Option> = env::var("AGENT_BROWSER_EXTENSIONS").ok().map(|v| { + v.split([',', '\n']) + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) + .collect() + }); + LaunchOptions { headless: !headed, executable_path: env::var("AGENT_BROWSER_EXECUTABLE_PATH").ok(), @@ -772,12 +779,7 @@ fn launch_options_from_env() -> LaunchOptions { .collect() }) .unwrap_or_default(), - extensions: env::var("AGENT_BROWSER_EXTENSIONS").ok().map(|v| { - v.split([',', '\n']) - .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()) - .collect() - }), + extensions, storage_state: env::var("AGENT_BROWSER_STATE").ok(), user_agent: env::var("AGENT_BROWSER_USER_AGENT").ok(), ignore_https_errors: env::var("AGENT_BROWSER_IGNORE_HTTPS_ERRORS") @@ -866,6 +868,7 @@ async fn handle_launch(cmd: &Value, state: &mut DaemonState) -> Result Value { #[cfg(test)] mod tests { use super::*; + use crate::test_utils::EnvGuard; #[test] fn test_success_response_structure() { @@ -5160,6 +5164,14 @@ mod tests { assert!(!opts.allow_file_access); } + #[test] + fn test_launch_options_from_env_headed_flag() { + let _guard = EnvGuard::new(&["AGENT_BROWSER_HEADED"]); + _guard.set("AGENT_BROWSER_HEADED", "1"); + let opts = launch_options_from_env(); + assert!(!opts.headless, "AGENT_BROWSER_HEADED=1 should set headless=false"); + } + #[tokio::test] async fn test_execute_unknown_command() { let mut state = DaemonState::new(); diff --git a/cli/src/native/cdp/chrome.rs b/cli/src/native/cdp/chrome.rs index a0a3620..7a15e56 100644 --- a/cli/src/native/cdp/chrome.rs +++ b/cli/src/native/cdp/chrome.rs @@ -8,6 +8,7 @@ use super::types::BrowserVersionInfo; pub struct ChromeProcess { child: Child, pub ws_url: String, + temp_user_data_dir: Option, } impl ChromeProcess { @@ -20,6 +21,23 @@ impl ChromeProcess { impl Drop for ChromeProcess { fn drop(&mut self) { self.kill(); + if let Some(ref dir) = self.temp_user_data_dir { + for attempt in 0..3 { + match std::fs::remove_dir_all(dir) { + Ok(()) => break, + Err(_) if attempt < 2 => { + std::thread::sleep(Duration::from_millis(100)); + } + Err(e) => { + eprintln!( + "Warning: failed to clean up temp profile {}: {}", + dir.display(), + e + ); + } + } + } + } } } @@ -59,14 +77,12 @@ impl Default for LaunchOptions { } } -pub fn launch_chrome(options: &LaunchOptions) -> Result { - let chrome_path = match &options.executable_path { - Some(p) => PathBuf::from(p), - None => { - find_chrome().ok_or("Chrome not found. Install Chrome or use --executable-path.")? - } - }; +struct ChromeArgs { + args: Vec, + temp_user_data_dir: Option, +} +fn build_chrome_args(options: &LaunchOptions) -> Result { let mut args = vec![ "--remote-debugging-port=0".to_string(), "--no-first-run".to_string(), @@ -97,10 +113,18 @@ pub fn launch_chrome(options: &LaunchOptions) -> Result { args.push(format!("--proxy-bypass-list={}", bypass)); } - if let Some(ref profile) = options.profile { + let temp_user_data_dir = if let Some(ref profile) = options.profile { let expanded = expand_tilde(profile); args.push(format!("--user-data-dir={}", expanded)); - } + None + } else { + let dir = std::env::temp_dir() + .join(format!("agent-browser-chrome-{}", uuid::Uuid::new_v4())); + std::fs::create_dir_all(&dir) + .map_err(|e| format!("Failed to create temp profile dir: {}", e))?; + args.push(format!("--user-data-dir={}", dir.display())); + Some(dir) + }; if options.allow_file_access { args.push("--allow-file-access-from-files".to_string()); @@ -115,7 +139,6 @@ pub fn launch_chrome(options: &LaunchOptions) -> Result { } } - // Check if user args set window size (skip viewport override) let has_window_size = options .args .iter() @@ -131,23 +154,66 @@ pub fn launch_chrome(options: &LaunchOptions) -> Result { args.push("--no-sandbox".to_string()); } + Ok(ChromeArgs { + args, + temp_user_data_dir, + }) +} + +pub fn launch_chrome(options: &LaunchOptions) -> Result { + let chrome_path = match &options.executable_path { + Some(p) => PathBuf::from(p), + None => { + find_chrome().ok_or("Chrome not found. Install Chrome or use --executable-path.")? + } + }; + + let ChromeArgs { + args, + temp_user_data_dir, + } = build_chrome_args(options)?; + + let cleanup_temp_dir = |dir: &Option| { + if let Some(ref d) = dir { + let _ = std::fs::remove_dir_all(d); + } + }; + let mut child = Command::new(&chrome_path) .args(&args) .stdin(Stdio::null()) .stdout(Stdio::null()) .stderr(Stdio::piped()) .spawn() - .map_err(|e| format!("Failed to launch Chrome at {:?}: {}", chrome_path, e))?; + .map_err(|e| { + cleanup_temp_dir(&temp_user_data_dir); + format!("Failed to launch Chrome at {:?}: {}", chrome_path, e) + })?; let stderr = child .stderr .take() - .ok_or("Failed to capture Chrome stderr")?; + .ok_or_else(|| { + let _ = child.kill(); + cleanup_temp_dir(&temp_user_data_dir); + "Failed to capture Chrome stderr".to_string() + })?; let reader = BufReader::new(stderr); - let ws_url = wait_for_ws_url(reader)?; + let ws_url = match wait_for_ws_url(reader) { + Ok(url) => url, + Err(e) => { + let _ = child.kill(); + cleanup_temp_dir(&temp_user_data_dir); + return Err(e); + } + }; - Ok(ChromeProcess { child, ws_url }) + Ok(ChromeProcess { + child, + ws_url, + temp_user_data_dir, + }) } fn wait_for_ws_url(reader: BufReader) -> Result { @@ -559,6 +625,7 @@ fn expand_tilde(path: &str) -> String { #[cfg(test)] mod tests { use super::*; + use crate::test_utils::EnvGuard; #[test] fn test_find_chrome_returns_some_on_host() { @@ -626,10 +693,138 @@ mod tests { #[test] fn test_find_playwright_chromium_nonexistent() { - // With no Playwright cache, should return None - std::env::set_var("PLAYWRIGHT_BROWSERS_PATH", "/nonexistent/path"); + let _guard = EnvGuard::new(&["PLAYWRIGHT_BROWSERS_PATH"]); + _guard.set("PLAYWRIGHT_BROWSERS_PATH", "/nonexistent/path"); let result = find_playwright_chromium(); - std::env::remove_var("PLAYWRIGHT_BROWSERS_PATH"); assert!(result.is_none()); } + + #[test] + fn test_build_args_headless_includes_headless_flag() { + let opts = LaunchOptions { + headless: true, + ..Default::default() + }; + let result = build_chrome_args(&opts).unwrap(); + assert!(result.args.iter().any(|a| a == "--headless=new")); + assert!(result + .args + .iter() + .any(|a| a == "--window-size=1280,720")); + // Temp dir created when no profile + assert!(result.temp_user_data_dir.is_some()); + let dir = result.temp_user_data_dir.unwrap(); + assert!(dir.exists()); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn test_build_args_headed_no_headless_flag() { + let opts = LaunchOptions { + headless: false, + ..Default::default() + }; + let result = build_chrome_args(&opts).unwrap(); + assert!(!result.args.iter().any(|a| a.contains("--headless"))); + assert!(!result.args.iter().any(|a| a.starts_with("--window-size="))); + // Temp dir created when no profile + assert!(result.temp_user_data_dir.is_some()); + let dir = result.temp_user_data_dir.unwrap(); + assert!(dir.exists()); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn test_build_args_temp_user_data_dir_created() { + let opts = LaunchOptions::default(); + let result = build_chrome_args(&opts).unwrap(); + let dir = result.temp_user_data_dir.as_ref().unwrap(); + assert!(dir.exists()); + assert!(result + .args + .iter() + .any(|a| a.starts_with("--user-data-dir="))); + let _ = std::fs::remove_dir_all(dir); + } + + #[test] + fn test_build_args_profile_no_temp_dir() { + let opts = LaunchOptions { + profile: Some("/tmp/my-profile".to_string()), + ..Default::default() + }; + let result = build_chrome_args(&opts).unwrap(); + assert!(result.temp_user_data_dir.is_none()); + assert!(result + .args + .iter() + .any(|a| a == "--user-data-dir=/tmp/my-profile")); + } + + #[test] + fn test_build_args_custom_window_size_not_overridden() { + let opts = LaunchOptions { + headless: true, + args: vec!["--window-size=1920,1080".to_string()], + ..Default::default() + }; + let result = build_chrome_args(&opts).unwrap(); + assert!(!result + .args + .iter() + .any(|a| a == "--window-size=1280,720")); + assert!(result + .args + .iter() + .any(|a| a == "--window-size=1920,1080")); + if let Some(ref dir) = result.temp_user_data_dir { + let _ = std::fs::remove_dir_all(dir); + } + } + + #[test] + fn test_build_args_start_maximized_suppresses_default_window_size() { + let opts = LaunchOptions { + headless: true, + args: vec!["--start-maximized".to_string()], + ..Default::default() + }; + let result = build_chrome_args(&opts).unwrap(); + assert!(!result.args.iter().any(|a| a == "--window-size=1280,720")); + assert!(result.args.iter().any(|a| a == "--start-maximized")); + if let Some(ref dir) = result.temp_user_data_dir { + let _ = std::fs::remove_dir_all(dir); + } + } + + #[test] + fn test_chrome_process_drop_cleans_temp_dir() { + let dir = std::env::temp_dir().join(format!( + "agent-browser-chrome-drop-test-{}", + uuid::Uuid::new_v4() + )); + let _ = std::fs::create_dir_all(&dir); + assert!(dir.exists()); + + { + // Simulate a ChromeProcess with a temp dir but a dummy child. + // We can't actually spawn Chrome here, but we can verify the Drop + // logic by creating a small helper process. + let child = Command::new("echo") + .arg("test") + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .unwrap(); + let _process = ChromeProcess { + child, + ws_url: String::new(), + temp_user_data_dir: Some(dir.clone()), + }; + // _process dropped here + } + + assert!(!dir.exists(), "Temp dir should be cleaned up on drop"); + } } diff --git a/cli/src/native/policy.rs b/cli/src/native/policy.rs index bd49b5a..7758c8a 100644 --- a/cli/src/native/policy.rs +++ b/cli/src/native/policy.rs @@ -135,6 +135,7 @@ impl ActionPolicy { #[cfg(test)] mod tests { use super::*; + use crate::test_utils::EnvGuard; #[test] fn test_policy_allow_whitelist() { @@ -205,12 +206,12 @@ mod tests { #[test] fn test_confirm_actions_from_env() { - env::set_var("AGENT_BROWSER_CONFIRM_ACTIONS", "navigate,click,fill"); + let _guard = EnvGuard::new(&["AGENT_BROWSER_CONFIRM_ACTIONS"]); + _guard.set("AGENT_BROWSER_CONFIRM_ACTIONS", "navigate,click,fill"); let ca = ConfirmActions::from_env().unwrap(); assert!(ca.requires_confirmation("navigate")); assert!(ca.requires_confirmation("click")); assert!(ca.requires_confirmation("fill")); assert!(!ca.requires_confirmation("screenshot")); - env::remove_var("AGENT_BROWSER_CONFIRM_ACTIONS"); } } diff --git a/cli/src/output.rs b/cli/src/output.rs index 9f974c1..8b0a9e0 100644 --- a/cli/src/output.rs +++ b/cli/src/output.rs @@ -2436,7 +2436,7 @@ Options: --json JSON output --full, -f Full page screenshot --annotate Annotated screenshot with numbered labels and legend - --headed Show browser window (not headless) + --headed Show browser window (not headless) (or AGENT_BROWSER_HEADED env) --cdp Connect via CDP (Chrome DevTools Protocol) --auto-connect Auto-discover and connect to running Chrome --color-scheme Color scheme: dark, light, no-preference (or AGENT_BROWSER_COLOR_SCHEME) diff --git a/cli/src/test_utils.rs b/cli/src/test_utils.rs new file mode 100644 index 0000000..2517cfb --- /dev/null +++ b/cli/src/test_utils.rs @@ -0,0 +1,49 @@ +use std::sync::{Mutex, MutexGuard}; + +/// Global mutex shared across all test modules to prevent parallel tests from +/// interfering with each other when mutating environment variables. +pub static ENV_MUTEX: Mutex<()> = Mutex::new(()); + +/// RAII guard that locks [`ENV_MUTEX`] and restores environment variables on drop. +pub struct EnvGuard<'a> { + _lock: MutexGuard<'a, ()>, + vars: Vec<(String, Option)>, +} + +impl<'a> EnvGuard<'a> { + pub fn new(var_names: &[&str]) -> Self { + let lock = ENV_MUTEX.lock().unwrap(); + let vars = var_names + .iter() + .map(|&name| (name.to_string(), std::env::var(name).ok())) + .collect(); + Self { _lock: lock, vars } + } + + pub fn set(&self, name: &str, value: &str) { + debug_assert!( + self.vars.iter().any(|(n, _)| n == name), + "EnvGuard::set called with unregistered var: {name}" + ); + std::env::set_var(name, value); + } + + pub fn remove(&self, name: &str) { + debug_assert!( + self.vars.iter().any(|(n, _)| n == name), + "EnvGuard::remove called with unregistered var: {name}" + ); + std::env::remove_var(name); + } +} + +impl Drop for EnvGuard<'_> { + fn drop(&mut self) { + for (name, value) in &self.vars { + match value { + Some(v) => std::env::set_var(name, v), + None => std::env::remove_var(name), + } + } + } +} diff --git a/docs/src/app/configuration/page.mdx b/docs/src/app/configuration/page.mdx index c0e7465..53cc165 100644 --- a/docs/src/app/configuration/page.mdx +++ b/docs/src/app/configuration/page.mdx @@ -175,6 +175,8 @@ These environment variables configure additional daemon and runtime behavior: AGENT_BROWSER_SESSION_NAMEAuto-save/load state persistence name.(none) AGENT_BROWSER_STATE_EXPIRE_DAYSAuto-delete saved session states older than N days.30 AGENT_BROWSER_ENCRYPTION_KEY64-char hex key for AES-256-GCM session encryption.(none) + AGENT_BROWSER_EXTENSIONSComma-separated browser extension paths. Extensions work in both headed and headless mode.(none) + AGENT_BROWSER_HEADEDShow browser window instead of running headless (1 to enable).(disabled) AGENT_BROWSER_STREAM_PORTEnable WebSocket streaming on the specified port (e.g., 9223).(disabled) AGENT_BROWSER_IOS_DEVICEDefault iOS device name for the ios provider.(none) AGENT_BROWSER_IOS_UDIDDefault iOS device UDID for the ios provider.(none) diff --git a/skills/agent-browser/SKILL.md b/skills/agent-browser/SKILL.md index 0153ce4..8cd7b7a 100644 --- a/skills/agent-browser/SKILL.md +++ b/skills/agent-browser/SKILL.md @@ -229,6 +229,8 @@ agent-browser profiler start # Start Chrome DevTools profiling agent-browser profiler stop trace.json # Stop and save profile (path optional) ``` +Use `AGENT_BROWSER_HEADED=1` to enable headed mode via environment variable. Browser extensions work in both headed and headless mode. + ### Local Files (PDFs, HTML) ```bash diff --git a/src/browser.ts b/src/browser.ts index 73fe0d0..8c723e2 100644 --- a/src/browser.ts +++ b/src/browser.ts @@ -1355,7 +1355,7 @@ export class BrowserManager { context = await launcher.launchPersistentContext( path.join(os.tmpdir(), `agent-browser-ext-${session}`), { - headless: false, + headless: options.headless ?? true, executablePath: options.executablePath, args: allArgs, viewport, diff --git a/src/daemon.ts b/src/daemon.ts index c5faff4..220b2e9 100644 --- a/src/daemon.ts +++ b/src/daemon.ts @@ -432,7 +432,7 @@ export async function startDaemon(options?: { } else if (manager instanceof BrowserManager) { // Auto-launch desktop browser const extensions = process.env.AGENT_BROWSER_EXTENSIONS - ? process.env.AGENT_BROWSER_EXTENSIONS.split(',') + ? process.env.AGENT_BROWSER_EXTENSIONS.split(/[,\n]/) .map((p) => p.trim()) .filter(Boolean) : undefined; @@ -468,7 +468,9 @@ export async function startDaemon(options?: { await manager.launch({ id: 'auto', action: 'launch' as const, - headless: process.env.AGENT_BROWSER_HEADED !== '1', + headless: + process.env.AGENT_BROWSER_HEADED !== '1' && + process.env.AGENT_BROWSER_HEADED !== 'true', executablePath: process.env.AGENT_BROWSER_EXECUTABLE_PATH, extensions: extensions, profile: process.env.AGENT_BROWSER_PROFILE,