From ec8d01ef4c7ef3ad5aec7c3d0f44ff6cc7f0cc01 Mon Sep 17 00:00:00 2001 From: leeguooooo Date: Fri, 12 Jun 2026 01:21:39 +0900 Subject: [PATCH] feat(cli): coordinate click + command aliases + clearer find error (issue #8.4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Field-report ergonomics fixes so agents stop wasting a round on a wrong guess: - Coordinate click is now first-class: `click `, `click ,`, and `click --coords ,` dispatch a raw viewport-point click (no element resolution), reusing the humanize trajectory + press dwell. Was previously only reachable via eval(elementFromPoint(...).click()). - Aliases: `tabs` (plural) → the `tab` subcommand tree; `get-text`/`get_text` → `get text `. - `find ` with a bare value (no locator keyword), e.g. `find "I'm not a robot" click`, now errors with the corrected command (`find text "I'm not a robot" click`) plus concrete examples, instead of a bare "Valid options: role, text, ..." list. Adds parse-layer regression tests for every form. --- cli/src/commands.rs | 144 +++++++++++++++++++++++++++++----- cli/src/native/actions.rs | 14 ++++ cli/src/native/interaction.rs | 14 ++++ 3 files changed, 154 insertions(+), 18 deletions(-) diff --git a/cli/src/commands.rs b/cli/src/commands.rs index 548ccea..6e75b11 100644 --- a/cli/src/commands.rs +++ b/cli/src/commands.rs @@ -375,12 +375,25 @@ fn parse_command_inner(args: &[String], flags: &Flags) -> Result { let new_tab = rest.contains(&"--new-tab"); + // Coordinate click as a first-class form (issue #8.4): when the only + // handle is a pixel position, no element/selector is needed. + // click e.g. click 449 320 + // click , e.g. click 449,320 + // click --coords , | --coords + let coord_args: Vec<&str> = rest + .iter() + .copied() + .filter(|a| *a != "--new-tab" && *a != "--coords") + .collect(); + if let Some((x, y)) = parse_coords(&coord_args) { + return Ok(json!({ "id": id, "action": "click", "x": x, "y": y })); + } let sel = rest .iter() .find(|arg| **arg != "--new-tab") .ok_or_else(|| ParseError::MissingArguments { context: "click".to_string(), - usage: "click [--new-tab]", + usage: "click | click | click --coords , [--new-tab]", })?; if new_tab { Ok(json!({ "id": id, "action": "click", "selector": sel, "newTab": true })) @@ -1208,6 +1221,15 @@ fn parse_command_inner(args: &[String], flags: &Flags) -> Result` — agents naturally + // guess `get-text` / `get_text` (issue #8.4). + "get-text" | "get_text" => { + let mut get_args: Vec<&str> = Vec::with_capacity(rest.len() + 1); + get_args.push("text"); + get_args.extend_from_slice(&rest); + parse_get(&get_args, &id) + } + // === Is (state checks) === "is" => parse_is(&rest, &id), @@ -1389,7 +1411,9 @@ fn parse_command_inner(args: &[String], flags: &Flags) -> Result { + // `tabs` (plural) is a natural guess for the `tab` subcommand tree — + // alias it so `tabs` / `tabs list` / `tabs new` all work (issue #8.4). + "tab" | "tabs" => { match rest.first().copied() { Some("new") => { // Accepted forms: @@ -2313,19 +2337,6 @@ fn parse_is(rest: &[&str], id: &str) -> Result { } fn parse_find(rest: &[&str], id: &str) -> Result { - const VALID: &[&str] = &[ - "role", - "text", - "label", - "placeholder", - "alt", - "title", - "testid", - "first", - "last", - "nth", - ]; - let locator = rest.first().ok_or_else(|| ParseError::MissingArguments { context: "find".to_string(), usage: "find [action] [text]", @@ -2495,13 +2506,37 @@ fn parse_find(rest: &[&str], id: &str) -> Result { } Ok(cmd) } - _ => Err(ParseError::UnknownSubcommand { - subcommand: locator.to_string(), - valid_options: VALID, + _ => Err(ParseError::InvalidValue { + // The user passed a value where a locator keyword was expected — the + // classic `find "I'm not a robot" click` mistake (issue #8.4). Lead + // with the corrected command using their own value, then the menu. + message: format!( + "`{loc}` is not a find locator. To match by visible text, name the locator:\n \ + agent-browser find text \"{loc}\" click\n\n\ + Locators: role, text, label, placeholder, alt, title, testid, first, last, nth\n\ + Examples:\n \ + agent-browser find text \"Sign in\" click\n \ + agent-browser find role button --name \"Submit\" click\n \ + agent-browser find label \"Email\" fill you@example.com", + loc = locator, + ), + usage: "find [action] [text]", }), } } +/// Parse a coordinate pair from `["449","320"]`, `["449,320"]`, or `["449, 320"]`. +/// Returns None if the args aren't a clean numeric pair (so callers fall back to +/// treating the argument as a selector). Used by first-class coordinate `click`. +fn parse_coords(args: &[&str]) -> Option<(f64, f64)> { + let (a, b) = match args { + [one] => one.split_once(',')?, + [a, b] => (*a, *b), + _ => return None, + }; + Some((a.trim().parse().ok()?, b.trim().parse().ok()?)) +} + fn parse_mouse(rest: &[&str], id: &str) -> Result { const VALID: &[&str] = &["move", "down", "up", "wheel"]; @@ -3550,6 +3585,79 @@ mod tests { assert_eq!(cmd["action"], "reload"); } + // === issue #8.4: CLI ergonomics === + + #[test] + fn test_click_coords_two_args() { + let cmd = parse_command(&args("click 449 320"), &default_flags()).unwrap(); + assert_eq!(cmd["action"], "click"); + assert_eq!(cmd["x"], 449.0); + assert_eq!(cmd["y"], 320.0); + assert!(cmd.get("selector").is_none()); + } + + #[test] + fn test_click_coords_comma() { + let cmd = parse_command(&args("click 449,320"), &default_flags()).unwrap(); + assert_eq!(cmd["x"], 449.0); + assert_eq!(cmd["y"], 320.0); + } + + #[test] + fn test_click_coords_flag() { + let cmd = parse_command(&args("click --coords 449,320"), &default_flags()).unwrap(); + assert_eq!(cmd["x"], 449.0); + assert_eq!(cmd["y"], 320.0); + } + + #[test] + fn test_click_selector_not_coords() { + let cmd = parse_command(&args("click button.submit"), &default_flags()).unwrap(); + assert_eq!(cmd["action"], "click"); + assert_eq!(cmd["selector"], "button.submit"); + assert!(cmd.get("x").is_none()); + } + + #[test] + fn test_tabs_alias_lists() { + assert_eq!( + parse_command(&args("tabs"), &default_flags()).unwrap()["action"], + "tab_list" + ); + assert_eq!( + parse_command(&args("tabs list"), &default_flags()).unwrap()["action"], + "tab_list" + ); + assert_eq!( + parse_command(&args("tabs new"), &default_flags()).unwrap()["action"], + "tab_new" + ); + } + + #[test] + fn test_get_text_hyphen_and_underscore_aliases() { + for verb in ["get-text", "get_text"] { + let cmd = parse_command(&args(&format!("{verb} .price")), &default_flags()).unwrap(); + assert_eq!(cmd["action"], "gettext", "{verb}"); + assert_eq!(cmd["selector"], ".price", "{verb}"); + } + } + + #[test] + fn test_find_bare_value_suggests_text_locator() { + // `find "I'm not a robot" click` — value where a locator keyword was + // expected. Error must steer to the corrected `find text ...` form. + let input: Vec = vec![ + "find".to_string(), + "I'm not a robot".to_string(), + "click".to_string(), + ]; + let err = parse_command(&input, &default_flags()).unwrap_err(); + let msg = err.format(); + assert!(msg.contains("find text"), "got: {msg}"); + assert!(msg.contains("I'm not a robot"), "got: {msg}"); + } + // === Core Actions === #[test] diff --git a/cli/src/native/actions.rs b/cli/src/native/actions.rs index b71bdfc..be92eb6 100644 --- a/cli/src/native/actions.rs +++ b/cli/src/native/actions.rs @@ -2986,6 +2986,20 @@ async fn handle_screenshot(cmd: &Value, state: &mut DaemonState) -> Result Result { + // First-class coordinate click (issue #8.4): click a raw viewport point with + // no element resolution. Parsed from `click ` / `click --coords x,y`. + if let (Some(x), Some(y)) = ( + cmd.get("x").and_then(|v| v.as_f64()), + cmd.get("y").and_then(|v| v.as_f64()), + ) { + let mgr = state.browser.as_ref().ok_or("Browser not launched")?; + let session_id = mgr.active_session_id()?.to_string(); + let button = cmd.get("button").and_then(|v| v.as_str()).unwrap_or("left"); + let click_count = cmd.get("clickCount").and_then(|v| v.as_i64()).unwrap_or(1) as i32; + interaction::click_at_point(&mgr.client, &session_id, x, y, button, click_count).await?; + return Ok(json!({ "clicked": { "x": x, "y": y } })); + } + let selector = cmd .get("selector") .and_then(|v| v.as_str()) diff --git a/cli/src/native/interaction.rs b/cli/src/native/interaction.rs index 8ccc5d0..f746d22 100644 --- a/cli/src/native/interaction.rs +++ b/cli/src/native/interaction.rs @@ -1143,6 +1143,20 @@ async fn wait_for_paint_settled(client: &CdpClient, session_id: &str) { .await; } +/// Click at a raw viewport coordinate, bypassing element/selector resolution +/// (issue #8.4 first-class coordinate click). Honors the humanize trajectory and +/// press dwell exactly like a selector click — it shares `dispatch_click`. +pub async fn click_at_point( + client: &CdpClient, + session_id: &str, + x: f64, + y: f64, + button: &str, + click_count: i32, +) -> Result<(), String> { + dispatch_click(client, session_id, x, y, button, click_count).await +} + async fn dispatch_click( client: &CdpClient, session_id: &str,