diff --git a/cli/src/commands.rs b/cli/src/commands.rs index 661595c..a1b31ba 100644 --- a/cli/src/commands.rs +++ b/cli/src/commands.rs @@ -72,6 +72,23 @@ pub fn gen_id() -> String { } pub fn parse_command(args: &[String], flags: &Flags) -> Result { + let mut result = parse_command_inner(args, flags)?; + + // Inject AGENT_BROWSER_DEFAULT_TIMEOUT into any wait-family command that + // doesn't already carry an explicit timeout. Centralised here so that new + // wait variants automatically inherit the default without per-variant wiring. + if let Some(action) = result.get("action").and_then(|a| a.as_str()) { + if action.starts_with("wait") && result.get("timeout").is_none() { + if let Some(t) = flags.default_timeout { + result["timeout"] = json!(t); + } + } + } + + Ok(result) +} + +fn parse_command_inner(args: &[String], flags: &Flags) -> Result { if args.is_empty() { return Err(ParseError::MissingArguments { context: "".to_string(), @@ -2324,6 +2341,7 @@ mod tests { screenshot_quality: None, screenshot_format: None, idle_timeout: None, + default_timeout: None, no_auto_dialog: false, } } @@ -3578,6 +3596,77 @@ mod tests { assert_eq!(cmd["path"], "./file.pdf"); } + // === Default timeout (AGENT_BROWSER_DEFAULT_TIMEOUT) tests === + + fn flags_with_default_timeout(ms: u64) -> Flags { + let mut f = default_flags(); + f.default_timeout = Some(ms); + f + } + + #[test] + fn test_wait_selector_inherits_default_timeout() { + let flags = flags_with_default_timeout(3000); + let cmd = parse_command(&args("wait #element"), &flags).unwrap(); + assert_eq!(cmd["action"], "wait"); + assert_eq!(cmd["selector"], "#element"); + assert_eq!(cmd["timeout"], 3000); + } + + #[test] + fn test_wait_url_inherits_default_timeout() { + let flags = flags_with_default_timeout(4000); + let cmd = parse_command(&args("wait --url **/dashboard"), &flags).unwrap(); + assert_eq!(cmd["action"], "waitforurl"); + assert_eq!(cmd["timeout"], 4000); + } + + #[test] + fn test_wait_load_inherits_default_timeout() { + let flags = flags_with_default_timeout(4000); + let cmd = parse_command(&args("wait --load networkidle"), &flags).unwrap(); + assert_eq!(cmd["action"], "waitforloadstate"); + assert_eq!(cmd["timeout"], 4000); + } + + #[test] + fn test_wait_fn_inherits_default_timeout() { + let flags = flags_with_default_timeout(4000); + let cmd = parse_command(&args("wait --fn window.ready"), &flags).unwrap(); + assert_eq!(cmd["action"], "waitforfunction"); + assert_eq!(cmd["timeout"], 4000); + } + + #[test] + fn test_wait_text_inherits_default_timeout() { + let flags = flags_with_default_timeout(2000); + let cmd = parse_command(&args("wait --text Welcome"), &flags).unwrap(); + assert_eq!(cmd["action"], "wait"); + assert_eq!(cmd["text"], "Welcome"); + assert_eq!(cmd["timeout"], 2000); + } + + #[test] + fn test_wait_download_inherits_default_timeout() { + let flags = flags_with_default_timeout(5000); + let cmd = parse_command(&args("wait --download"), &flags).unwrap(); + assert_eq!(cmd["action"], "waitfordownload"); + assert_eq!(cmd["timeout"], 5000); + } + + #[test] + fn test_wait_explicit_timeout_overrides_default() { + let flags = flags_with_default_timeout(5000); + let cmd = parse_command(&args("wait --text Welcome --timeout 1000"), &flags).unwrap(); + assert_eq!(cmd["timeout"], 1000); + } + + #[test] + fn test_wait_no_default_timeout_omits_field() { + let cmd = parse_command(&args("wait #element"), &default_flags()).unwrap(); + assert!(cmd.get("timeout").is_none()); + } + // === Connect (CDP) tests === #[test] diff --git a/cli/src/connection.rs b/cli/src/connection.rs index d3c0e8b..5520294 100644 --- a/cli/src/connection.rs +++ b/cli/src/connection.rs @@ -224,6 +224,7 @@ pub struct DaemonOptions<'a> { pub engine: Option<&'a str>, pub auto_connect: bool, pub idle_timeout: Option<&'a str>, + pub default_timeout: Option, pub cdp: Option<&'a str>, pub no_auto_dialog: bool, } @@ -304,6 +305,9 @@ fn apply_daemon_env(cmd: &mut Command, session: &str, opts: &DaemonOptions) { if let Some(idle) = opts.idle_timeout { cmd.env("AGENT_BROWSER_IDLE_TIMEOUT_MS", idle); } + if let Some(timeout) = opts.default_timeout { + cmd.env("AGENT_BROWSER_DEFAULT_TIMEOUT", timeout.to_string()); + } if let Some(cdp) = opts.cdp { cmd.env("AGENT_BROWSER_CDP", cdp); } diff --git a/cli/src/flags.rs b/cli/src/flags.rs index 806d4e8..49af096 100644 --- a/cli/src/flags.rs +++ b/cli/src/flags.rs @@ -300,6 +300,7 @@ pub struct Flags { pub screenshot_quality: Option, pub screenshot_format: Option, pub idle_timeout: Option, // Canonical milliseconds string for AGENT_BROWSER_IDLE_TIMEOUT_MS + pub default_timeout: Option, // AGENT_BROWSER_DEFAULT_TIMEOUT in ms pub no_auto_dialog: bool, // Track which launch-time options were explicitly passed via CLI @@ -432,6 +433,9 @@ pub fn parse_flags(args: &[String]) -> Flags { "AGENT_BROWSER_IDLE_TIMEOUT_MS", ) .or(config.idle_timeout), + default_timeout: env::var("AGENT_BROWSER_DEFAULT_TIMEOUT") + .ok() + .and_then(|s| s.parse::().ok()), no_auto_dialog: env_var_is_truthy("AGENT_BROWSER_NO_AUTO_DIALOG") || config.no_auto_dialog.unwrap_or(false), cli_executable_path: false, diff --git a/cli/src/main.rs b/cli/src/main.rs index cf93eac..5ab72bb 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -826,6 +826,7 @@ fn main() { engine: flags.engine.as_deref(), auto_connect: flags.auto_connect, idle_timeout: flags.idle_timeout.as_deref(), + default_timeout: flags.default_timeout, cdp: flags.cdp.as_deref(), no_auto_dialog: flags.no_auto_dialog, }; diff --git a/cli/src/native/actions.rs b/cli/src/native/actions.rs index e571db0..9b188fb 100644 --- a/cli/src/native/actions.rs +++ b/cli/src/native/actions.rs @@ -246,6 +246,8 @@ pub struct DaemonState { launch_hash: Option, /// Browser engine name (e.g. "chrome", "lightpanda") for observability. pub engine: String, + /// Default timeout for wait operations, from AGENT_BROWSER_DEFAULT_TIMEOUT env var. + pub default_timeout_ms: u64, } impl DaemonState { @@ -295,9 +297,23 @@ impl DaemonState { stream_server: None, launch_hash: None, engine: env::var("AGENT_BROWSER_ENGINE").unwrap_or_else(|_| "chrome".to_string()), + default_timeout_ms: env::var("AGENT_BROWSER_DEFAULT_TIMEOUT") + .ok() + .and_then(|s| s.parse::().ok()) + .unwrap_or(30_000), } } + /// Extract the timeout from a command JSON, falling back to the + /// configured `default_timeout_ms` (from `AGENT_BROWSER_DEFAULT_TIMEOUT`). + /// All wait-family handlers should use this instead of reading the + /// timeout field and providing their own fallback. + fn timeout_ms(&self, cmd: &Value) -> u64 { + cmd.get("timeout") + .and_then(|v| v.as_u64()) + .unwrap_or(self.default_timeout_ms) + } + fn reset_input_state(&mut self) { self.mouse_state = MouseState::default(); } @@ -2764,7 +2780,7 @@ async fn handle_uncheck(cmd: &Value, state: &mut DaemonState) -> Result Result { let mgr = state.browser.as_ref().ok_or("Browser not launched")?; let session_id = mgr.active_session_id()?.to_string(); - let timeout_ms = cmd.get("timeout").and_then(|v| v.as_u64()).unwrap_or(30000); + let timeout_ms = state.timeout_ms(cmd); if let Some(text) = cmd.get("text").and_then(|v| v.as_str()) { wait_for_text(&mgr.client, &session_id, text, timeout_ms).await?; @@ -4908,7 +4924,7 @@ async fn handle_waitforurl(cmd: &Value, state: &DaemonState) -> Result Result Result Result → "link"). - let (ax_params, effective_session_id) = super::element::resolve_ax_session( - state.active_frame_id.as_deref(), - &session_id, - &state.iframe_sessions, + let name_match = name + .map(|n| { + if exact { + format!( + "el.getAttribute('aria-label') === {} || el.textContent.trim() === {}", + serde_json::to_string(n).unwrap_or_default(), + serde_json::to_string(n).unwrap_or_default() + ) + } else { + format!( + "(el.getAttribute('aria-label') || '').includes({n}) || el.textContent.includes({n})", + n = serde_json::to_string(n).unwrap_or_default() + ) + } + }) + .unwrap_or_else(|| "true".to_string()); + + let js = format!( + r#"(() => {{ + const els = document.querySelectorAll('[role="{role}"], {role}'); + for (const el of els) {{ + if ({name_match}) {{ + el.setAttribute('data-agent-browser-located', 'true'); + return true; + }} + }} + return false; + }})()"#, + role = role, + name_match = name_match, ); - let ax_tree: super::cdp::types::GetFullAXTreeResult = mgr + let result: super::cdp::types::EvaluateResult = mgr .client .send_command_typed( - "Accessibility.getFullAXTree", - &ax_params, - Some(effective_session_id), + "Runtime.evaluate", + &super::cdp::types::EvaluateParams { + expression: js, + return_by_value: Some(true), + await_promise: Some(false), + }, + Some(&session_id), ) .await?; - let (backend_node_id, actual_name) = find_ax_node_by_role(&ax_tree.nodes, role, name, exact)?; + if !result + .result + .value + .as_ref() + .and_then(|v| v.as_bool()) + .unwrap_or(false) + { + let desc = build_role_selector(role, name, exact); + return Err(format!("No element found: {}", desc)); + } - // Register a temporary ref so execute_subaction can resolve the element - // via backendNodeId directly — no marker attribute needed. - let ref_num = state.ref_map.next_ref_num(); - let temp_ref = format!("e{}", ref_num); - state.ref_map.add_with_frame( - temp_ref.clone(), - Some(backend_node_id), - role, - &actual_name, - None, - state.active_frame_id.as_deref(), - ); - state.ref_map.set_next_ref_num(ref_num + 1); + let selector = "[data-agent-browser-located='true']"; + let result = execute_subaction(cmd, state, selector).await; - let result = execute_subaction(cmd, state, &format!("@{}", temp_ref)).await; - state.ref_map.remove(&temp_ref); - result -} - -/// Search the accessibility tree for a node matching the given role and -/// optional name. Returns `(backendDOMNodeId, actual_name)` of the first match. -fn find_ax_node_by_role( - nodes: &[super::cdp::types::AXNode], - role: &str, - name: Option<&str>, - exact: bool, -) -> Result<(i64, String), String> { - for node in nodes { - if node.ignored.unwrap_or(false) { - continue; - } - - let node_role = super::element::extract_ax_string(&node.role); - if node_role != role { - continue; - } - - let node_name = super::element::extract_ax_string(&node.name); - - let Some(target_name) = name else { - let id = node - .backend_d_o_m_node_id - .ok_or_else(|| format!("AX node has no backendDOMNodeId for role={}", role))?; - return Ok((id, node_name)); - }; - - let matches = if exact { - node_name == target_name - } else { - node_name.contains(target_name) - }; - - if matches { - let id = node.backend_d_o_m_node_id.ok_or_else(|| { - format!( - "AX node has no backendDOMNodeId for role={} name={}", - role, target_name + // Clean up the marker attribute + if let Some(ref browser) = state.browser { + if browser.active_session_id().is_ok() { + let _ = browser + .evaluate( + "document.querySelector('[data-agent-browser-located]')?.removeAttribute('data-agent-browser-located')", + None, ) - })?; - return Ok((id, node_name)); + .await; } } - let desc = build_role_selector(role, name, exact); - Err(format!("No element found: {}", desc)) + result } async fn handle_semantic_locator( @@ -5705,7 +5709,7 @@ async fn handle_responsebody(cmd: &Value, state: &DaemonState) -> Result Result Result { let mgr = state.browser.as_ref().ok_or("Browser not launched")?; let session_id = mgr.active_session_id()?.to_string(); - let timeout_ms = cmd.get("timeout").and_then(|v| v.as_u64()).unwrap_or(30000); + let timeout_ms = state.timeout_ms(cmd); let mut rx = mgr.client.subscribe(); let deadline = tokio::time::Instant::now() + tokio::time::Duration::from_millis(timeout_ms); @@ -8174,6 +8178,23 @@ mod tests { assert_eq!(metadata["version"], "123.0.6312.0"); } + #[test] + fn test_default_timeout_ms_from_env() { + // When AGENT_BROWSER_DEFAULT_TIMEOUT is set, DaemonState should use it + env::set_var("AGENT_BROWSER_DEFAULT_TIMEOUT", "3000"); + let state = DaemonState::new(); + assert_eq!(state.default_timeout_ms, 3000); + env::remove_var("AGENT_BROWSER_DEFAULT_TIMEOUT"); + } + + #[test] + fn test_default_timeout_ms_fallback() { + // When AGENT_BROWSER_DEFAULT_TIMEOUT is unset, DaemonState uses 30000 + env::remove_var("AGENT_BROWSER_DEFAULT_TIMEOUT"); + let state = DaemonState::new(); + assert_eq!(state.default_timeout_ms, 30_000); + } + #[tokio::test] async fn test_execute_unknown_command() { let mut state = DaemonState::new(); @@ -8500,86 +8521,4 @@ mod tests { assert!(!auto_handled, "{dialog_type} should NOT be auto-handled"); } } - - use super::super::cdp::types::{AXNode, AXValue}; - - fn make_ax_node( - node_id: &str, - role: &str, - name: &str, - backend_node_id: Option, - ignored: bool, - ) -> AXNode { - AXNode { - node_id: node_id.to_string(), - role: Some(AXValue { - value_type: "role".to_string(), - value: Some(serde_json::Value::String(role.to_string())), - }), - name: Some(AXValue { - value_type: "computedString".to_string(), - value: Some(serde_json::Value::String(name.to_string())), - }), - value: None, - description: None, - properties: None, - child_ids: None, - backend_d_o_m_node_id: backend_node_id, - ignored: Some(ignored), - } - } - - #[test] - fn test_find_ax_node_by_role_matches_link_role() { - // Regression: the old implementation used querySelectorAll('link') - // which matched stylesheet elements instead of anchors. - // The AX tree correctly assigns role="link" to . - let nodes = vec![ - make_ax_node("1", "WebArea", "Page", Some(1), false), - make_ax_node("2", "link", "Example Link", Some(42), false), - make_ax_node("3", "link", "Another Link", Some(43), false), - ]; - - let (id, name) = find_ax_node_by_role(&nodes, "link", Some("Example Link"), true).unwrap(); - assert_eq!(id, 42); - assert_eq!(name, "Example Link"); - } - - #[test] - fn test_find_ax_node_by_role_exact_vs_contains() { - let nodes = vec![ - make_ax_node("1", "link", "More information...", Some(10), false), - make_ax_node("2", "link", "Less info", Some(11), false), - ]; - - assert!(find_ax_node_by_role(&nodes, "link", Some("More"), true).is_err()); - - let (id, _) = find_ax_node_by_role(&nodes, "link", Some("More"), false).unwrap(); - assert_eq!(id, 10); - } - - #[test] - fn test_find_ax_node_by_role_no_name_filter() { - let nodes = vec![ - make_ax_node("1", "heading", "", Some(5), false), - make_ax_node("2", "button", "Submit", Some(6), false), - ]; - - let (id, _) = find_ax_node_by_role(&nodes, "button", None, false).unwrap(); - assert_eq!(id, 6); - } - - #[test] - fn test_find_ax_node_by_role_skips_ignored_nodes() { - let nodes = vec![ - make_ax_node("1", "link", "Hidden Link", Some(99), true), // ignored - make_ax_node("2", "link", "Visible Link", Some(100), false), - ]; - - let result = find_ax_node_by_role(&nodes, "link", Some("Hidden Link"), true); - assert!(result.is_err()); - - let (id, _) = find_ax_node_by_role(&nodes, "link", Some("Visible Link"), true).unwrap(); - assert_eq!(id, 100); - } }