From cf4c27d13de18a91873359e8c3ed394c06847342 Mon Sep 17 00:00:00 2001 From: leeguooooo Date: Wed, 10 Jun 2026 14:48:49 +0900 Subject: [PATCH] fix: resolve Hermes-found CLI bugs (wait --url, find role, invalid selector, polish) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - wait --url: the arg parser never read `--timeout`, so a non-matching pattern waited the large default and wedged the daemon. Parse it. Also: matching was a literal substring (`includes`) so globs never matched — convert `**`/`*`/`?` globs to an anchored regex. And `poll_until_true` now bounds each probe with a timeout and tolerates transient navigation errors, so a hung `Runtime.evaluate` can never block past the deadline (un-wedges the daemon). - find role [--name]: the query was `[role="X"], X`, which matches a literal tag / explicit attribute but NOT implicit-role elements — so `find role link` () and `find role heading` (

) never matched. Add a proper ARIA-role → implicit-element map and broaden accessible-name matching (aria-label/title/alt/value/text). - click on a syntactically-invalid selector returned `✓ Done`: querySelector throws, and Runtime.evaluate returned the thrown DOMException as an objectId that was clicked as if it were the element. Check exception_details → error. - output: a title-less page now prints `✓ ` instead of an empty title line. - docs(skill): tab refs are `t2`, not `2` (SKILL.md, electron). Verified live (isolated launch): wait --url glob matches instantly; non-matching honors --timeout (2s) and leaves the daemon responsive; find role link/heading match; invalid selector errors. Unit tests added for the glob + role map + parse. --- cli/Cargo.lock | 2 +- cli/Cargo.toml | 2 +- cli/src/commands.rs | 26 ++++- cli/src/native/actions.rs | 199 +++++++++++++++++++++++++++++------ cli/src/native/element.rs | 12 +++ cli/src/output.rs | 18 +++- package.json | 2 +- skill-data/core/SKILL.md | 11 +- skill-data/electron/SKILL.md | 6 +- 9 files changed, 230 insertions(+), 48 deletions(-) diff --git a/cli/Cargo.lock b/cli/Cargo.lock index 6c4a0f5..79d6f92 100644 --- a/cli/Cargo.lock +++ b/cli/Cargo.lock @@ -45,7 +45,7 @@ dependencies = [ [[package]] name = "agent-browser-stealth" -version = "0.27.0-fork.31" +version = "0.27.0-fork.32" dependencies = [ "aes-gcm", "async-trait", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index e65aa71..6f33fca 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "agent-browser-stealth" -version = "0.27.0-fork.31" +version = "0.27.0-fork.32" edition = "2021" description = "Fast browser automation CLI for AI agents" license = "Apache-2.0" diff --git a/cli/src/commands.rs b/cli/src/commands.rs index fd5e2e2..86fe8ac 100644 --- a/cli/src/commands.rs +++ b/cli/src/commands.rs @@ -540,7 +540,7 @@ fn parse_command_inner(args: &[String], flags: &Flags) -> Result { - // Check for --url flag: wait --url "**/dashboard" + // Check for --url flag: wait --url "**/dashboard" [--timeout ms] if let Some(idx) = rest.iter().position(|&s| s == "--url" || s == "-u") { let url = rest .get(idx + 1) @@ -548,7 +548,15 @@ fn parse_command_inner(args: &[String], flags: &Flags) -> Result", })?; - return Ok(json!({ "id": id, "action": "waitforurl", "url": url })); + let mut cmd = json!({ "id": id, "action": "waitforurl", "url": url }); + // Parse --timeout (without it the default applies — and a + // non-matching pattern would otherwise wait the full default). + if let Some(t_idx) = rest.iter().position(|&s| s == "--timeout") { + if let Some(ms) = rest.get(t_idx + 1).and_then(|s| s.parse::().ok()) { + cmd["timeout"] = json!(ms); + } + } + return Ok(cmd); } // Check for --load flag: wait --load networkidle @@ -3803,6 +3811,20 @@ mod tests { assert_eq!(cmd["url"], "**/dashboard"); } + #[test] + fn test_wait_url_with_timeout() { + // --timeout must be parsed for the --url path; without it a non-matching + // pattern waits the full default (and could wedge the daemon). + let cmd = parse_command( + &args("wait --url **/dashboard --timeout 3000"), + &default_flags(), + ) + .unwrap(); + assert_eq!(cmd["action"], "waitforurl"); + assert_eq!(cmd["url"], "**/dashboard"); + assert_eq!(cmd["timeout"], 3000); + } + #[test] fn test_wait_load() { let cmd = parse_command(&args("wait --load networkidle"), &default_flags()).unwrap(); diff --git a/cli/src/native/actions.rs b/cli/src/native/actions.rs index f2e7b37..cfa3f8e 100644 --- a/cli/src/native/actions.rs +++ b/cli/src/native/actions.rs @@ -3494,16 +3494,53 @@ async fn wait_for_selector( poll_until_true(client, session_id, &check_fn, timeout_ms).await } +/// Convert a URL glob (Playwright-style: `*` matches within a path segment, +/// `**` matches across segments, `?` matches one char) to an anchored regex. +fn url_glob_to_regex(glob: &str) -> String { + let mut re = String::from("^"); + let mut chars = glob.chars().peekable(); + while let Some(c) = chars.next() { + match c { + '*' => { + if chars.peek() == Some(&'*') { + chars.next(); + re.push_str(".*"); // ** — any chars incl. '/' + } else { + re.push_str("[^/]*"); // * — any chars except '/' + } + } + '?' => re.push('.'), + '.' | '+' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '\\' => { + re.push('\\'); + re.push(c); + } + _ => re.push(c), + } + } + re.push('$'); + re +} + async fn wait_for_url( client: &super::cdp::client::CdpClient, session_id: &str, pattern: &str, timeout_ms: u64, ) -> Result<(), String> { - let check_fn = format!( - "location.href.includes({})", - serde_json::to_string(pattern).unwrap_or_default() - ); + // A pattern with glob metacharacters is matched as a glob (the core skill + // documents `wait --url "**/dashboard"`); otherwise it's a plain substring + // so exact / partial URLs keep working. + let check_fn = if pattern.contains('*') || pattern.contains('?') { + format!( + "(()=>{{try{{return new RegExp({}).test(location.href)}}catch(e){{return false}}}})()", + serde_json::to_string(&url_glob_to_regex(pattern)).unwrap_or_default() + ) + } else { + format!( + "location.href.includes({})", + serde_json::to_string(pattern).unwrap_or_default() + ) + }; poll_until_true(client, session_id, &check_fn, timeout_ms).await } @@ -3539,8 +3576,19 @@ async fn poll_until_true( let deadline = tokio::time::Instant::now() + tokio::time::Duration::from_millis(timeout_ms); loop { - let result: super::cdp::types::EvaluateResult = client - .send_command_typed( + let remaining = deadline.saturating_duration_since(tokio::time::Instant::now()); + if remaining.is_zero() { + return Err(format!("Wait timed out after {}ms", timeout_ms)); + } + + // Bound each probe. A `Runtime.evaluate` issued while the page is + // navigating can hang (the execution context is being torn down); without + // a cap the `.await` would block past the deadline forever and wedge the + // daemon's request loop. Cap at the remaining budget (max 2s per probe). + let probe_cap = remaining.min(tokio::time::Duration::from_secs(2)); + let probe = tokio::time::timeout( + probe_cap, + client.send_command_typed::<_, super::cdp::types::EvaluateResult>( "Runtime.evaluate", &super::cdp::types::EvaluateParams { expression: expression.to_string(), @@ -3548,24 +3596,31 @@ async fn poll_until_true( await_promise: Some(true), }, Some(session_id), - ) - .await?; + ), + ) + .await; - if result - .result - .value - .as_ref() - .and_then(|v| v.as_bool()) - .unwrap_or(false) - { - return Ok(()); + // A probe that timed out or errored (e.g. the execution context was + // replaced mid-navigation) is transient — keep polling until the + // deadline rather than failing or hanging. + if let Ok(Ok(result)) = probe { + if result + .result + .value + .as_ref() + .and_then(|v| v.as_bool()) + .unwrap_or(false) + { + return Ok(()); + } } - if tokio::time::Instant::now() >= deadline { + let nap = tokio::time::Duration::from_millis(100) + .min(deadline.saturating_duration_since(tokio::time::Instant::now())); + if nap.is_zero() { return Err(format!("Wait timed out after {}ms", timeout_ms)); } - - tokio::time::sleep(tokio::time::Duration::from_millis(100)).await; + tokio::time::sleep(nap).await; } } @@ -6004,6 +6059,54 @@ async fn execute_subaction( } } +/// CSS selector that matches an ARIA `role` — both an explicit `role="X"` +/// attribute AND the HTML elements that carry that role *implicitly*. The naive +/// `[role="X"], X` form fails for every role whose implicit element has a +/// different tag than the role name (e.g. role `link` ⇒ ``, not ``; +/// role `heading` ⇒ `

`..`

`), which made `find role link/heading` never +/// match real elements. +fn role_to_query(role: &str) -> String { + let implicit = match role { + "link" => "a[href], area[href]", + "button" => "button, input[type=button], input[type=submit], input[type=reset], summary", + "heading" => "h1, h2, h3, h4, h5, h6", + "textbox" => { + "input[type=text], input[type=search], input[type=email], input[type=url], \ + input[type=tel], input[type=password], input:not([type]), textarea" + } + "searchbox" => "input[type=search]", + "checkbox" => "input[type=checkbox]", + "radio" => "input[type=radio]", + "combobox" => "select", + "listbox" => "select[multiple]", + "slider" => "input[type=range]", + "spinbutton" => "input[type=number]", + "img" => "img", + "list" => "ul, ol", + "listitem" => "li", + "table" => "table", + "row" => "tr", + "cell" | "gridcell" => "td", + "columnheader" | "rowheader" => "th", + "article" => "article", + "navigation" => "nav", + "main" => "main", + "banner" => "header", + "contentinfo" => "footer", + "complementary" => "aside", + "figure" => "figure", + "separator" => "hr", + "progressbar" => "progress", + "group" => "fieldset", + _ => "", + }; + if implicit.is_empty() { + format!("[role=\"{role}\"], {role}") + } else { + format!("[role=\"{role}\"], {implicit}") + } +} + fn build_role_selector(role: &str, name: Option<&str>, exact: bool) -> String { match name { Some(n) => { @@ -6024,27 +6127,25 @@ async fn handle_getbyrole(cmd: &Value, state: &mut DaemonState) -> Result {{ - const els = document.querySelectorAll('[role="{role}"], {role}'); + const els = document.querySelectorAll({selector}); for (const el of els) {{ + const __an = (el.getAttribute('aria-label') || el.getAttribute('title') + || el.getAttribute('alt') || el.value || el.textContent || '').trim(); if ({name_match}) {{ el.setAttribute('data-agent-browser-located', 'true'); return true; @@ -6052,7 +6153,7 @@ async fn handle_getbyrole(cmd: &Value, state: &mut DaemonState) -> Result, not ; headings are h1..h6 + assert_eq!( + role_to_query("link"), + "[role=\"link\"], a[href], area[href]" + ); + assert_eq!( + role_to_query("heading"), + "[role=\"heading\"], h1, h2, h3, h4, h5, h6" + ); + assert!(role_to_query("button").contains("button")); + // unknown/custom roles fall back to the attribute + literal tag + assert_eq!(role_to_query("tablist"), "[role=\"tablist\"], tablist"); + } + fn unique_socket_dir(label: &str) -> PathBuf { let nanos = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) diff --git a/cli/src/native/element.rs b/cli/src/native/element.rs index 900591b..4aec670 100644 --- a/cli/src/native/element.rs +++ b/cli/src/native/element.rs @@ -440,6 +440,18 @@ pub async fn resolve_element_object_id( ) .await?; + // A syntactically-invalid selector makes `document.querySelector` THROW. + // With returnByValue:false, Runtime.evaluate then returns the thrown + // DOMException as a remote object *with* an objectId — which would otherwise + // be mistaken for "the element" and silently no-op a `.click()` on it. Treat + // any thrown exception as a hard error so a typo'd selector fails loudly. + if let Some(ex) = result.exception_details { + return Err(format!( + "Invalid selector '{}': {}", + selector_or_ref, ex.text + )); + } + let object_id = result .result .object_id diff --git a/cli/src/output.rs b/cli/src/output.rs index 8ad63ca..bb88fc1 100644 --- a/cli/src/output.rs +++ b/cli/src/output.rs @@ -228,12 +228,20 @@ pub fn print_response_with_opts(resp: &Response, action: Option<&str>, opts: &Ou } // Navigation response if let Some(url) = data.get("url").and_then(|v| v.as_str()) { - if let Some(title) = data.get("title").and_then(|v| v.as_str()) { - println!("{} {}", color::success_indicator(), color::bold(title)); - println!(" {}", color::dim(url)); - return; + let title = data + .get("title") + .and_then(|v| v.as_str()) + .map(str::trim) + .filter(|t| !t.is_empty()); + match title { + Some(t) => { + println!("{} {}", color::success_indicator(), color::bold(t)); + println!(" {}", color::dim(url)); + } + // Title-less page: show the URL with the checkmark instead of an + // empty title line. + None => println!("{} {}", color::success_indicator(), color::dim(url)), } - println!("{}", url); return; } if let Some(cdp_url) = data.get("cdpUrl").and_then(|v| v.as_str()) { diff --git a/package.json b/package.json index 7a65650..72798d1 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "agent-browser-stealth", - "version": "0.27.0-fork.31", + "version": "0.27.0-fork.32", "description": "Browser automation CLI for AI agents — stealth fork with anti-detection", "type": "module", "packageManager": "pnpm@11.1.3", diff --git a/skill-data/core/SKILL.md b/skill-data/core/SKILL.md index 77d16ae..ae2102e 100644 --- a/skill-data/core/SKILL.md +++ b/skill-data/core/SKILL.md @@ -363,13 +363,14 @@ Pass `--hide-scrollbars false` when launching to keep native scrollbars visible. ```bash agent-browser tab # list open tabs (with stable tabId) agent-browser tab new https://docs... # open a new tab (and switch to it) -agent-browser tab 2 # switch to tab 2 -agent-browser tab close 2 # close tab 2 +agent-browser tab t2 # switch to tab t2 +agent-browser tab close t2 # close tab t2 ``` -Stable `tabId`s mean `tab 2` points at the same tab across commands even -when other tabs open or close. After switching, refs from a prior snapshot -on a different tab no longer apply — re-snapshot. +Tab ids are stable strings (`t1`, `t2`, …), never reused within a session, so +the same id keeps referring to the same tab across commands. Positional +integers are **not** accepted — use `t2`, not `2`. After switching, refs from a +prior snapshot on a different tab no longer apply — re-snapshot. ### Run multiple browsers in parallel diff --git a/skill-data/electron/SKILL.md b/skill-data/electron/SKILL.md index 1fc2cfd..f659f23 100644 --- a/skill-data/electron/SKILL.md +++ b/skill-data/electron/SKILL.md @@ -95,8 +95,8 @@ Electron apps often have multiple windows or webviews. Use tab commands to list # List all available targets (windows, webviews, etc.) agent-browser tab -# Switch to a specific tab by index -agent-browser tab 2 +# Switch to a specific tab by id (t1, t2, …; integers not accepted) +agent-browser tab t2 # Switch by URL pattern agent-browser tab --url "*settings*" @@ -117,7 +117,7 @@ agent-browser tab # 1: [webview] Embedded Content https://example.com/widget # Switch to a webview -agent-browser tab 1 +agent-browser tab t1 # Interact with the webview normally agent-browser snapshot -i