From 9ab8753b48cfceec61047c444c4c3c7df946e667 Mon Sep 17 00:00:00 2001 From: leeguooooo Date: Mon, 15 Jun 2026 11:13:34 +0900 Subject: [PATCH] feat(click): report (and optionally --follow) a tab opened by a click (#24-A) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A click on a target=_blank link / window.open opened a new tab, but the active tab stayed put, so the post-click snapshot showed the OLD page — looking exactly like the click failed. On the relay the new tab is discovered only via getTargets (the relay doesn't push target events to the daemon), so it went unsurfaced. handle_click now snapshots tracked targets before the click and, after, runs a lightweight BrowserManager::adopt_newly_opened (one getTargets, attaches only the new target — far cheaper than a full resync) to detect a freshly-opened tab. It's reported as openedTab {tabId,url,title} in the response (and a '→ opened new tab [tN] ' hint in text mode). Default keeps focus on the current tab (so multi-tab flows aren't hijacked, per #7/#8.1); 'click --follow' switches to the new tab. Verified live: clicking a _blank link prints '→ opened new tab [t13] https://example.org/'. Completes the #24 friction items (B/C/D shipped in 770708b). --- cli/src/commands.rs | 34 ++++++++++++++++++---- cli/src/native/actions.rs | 30 ++++++++++++++++++- cli/src/native/browser.rs | 61 +++++++++++++++++++++++++++++++++++++++ cli/src/output.rs | 23 +++++++++++++++ 4 files changed, 141 insertions(+), 7 deletions(-) diff --git a/cli/src/commands.rs b/cli/src/commands.rs index d2667d1..dbac782 100644 --- a/cli/src/commands.rs +++ b/cli/src/commands.rs @@ -424,6 +424,9 @@ fn parse_command_inner(args: &[String], flags: &Flags) -> Result { let new_tab = rest.contains(&"--new-tab"); + // `--follow`: if the click opens a new tab, switch the active tab to + // it (default reports the opened tab but stays put) (issue #24-A). + let follow = rest.contains(&"--follow"); // 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 @@ -432,23 +435,27 @@ fn parse_command_inner(args: &[String], flags: &Flags) -> Result = rest .iter() .copied() - .filter(|a| *a != "--new-tab" && *a != "--coords") + .filter(|a| !a.starts_with("--")) .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") + .find(|arg| !arg.starts_with("--")) .ok_or_else(|| ParseError::MissingArguments { context: "click".to_string(), - usage: "click | click | click --coords , [--new-tab]", + usage: + "click | click | click --coords , [--new-tab] [--follow]", })?; + let mut cmd = json!({ "id": id, "action": "click", "selector": sel }); if new_tab { - Ok(json!({ "id": id, "action": "click", "selector": sel, "newTab": true })) - } else { - Ok(json!({ "id": id, "action": "click", "selector": sel })) + cmd["newTab"] = json!(true); } + if follow { + cmd["follow"] = json!(true); + } + Ok(cmd) } "dblclick" => { let sel = rest.first().ok_or_else(|| ParseError::MissingArguments { @@ -3784,6 +3791,21 @@ mod tests { assert!(cmd.get("x").is_none()); } + #[test] + fn test_click_follow_flag() { + // `--follow` sets the flag; the selector is still found even with the flag + // before it (issue #24-A). + let cmd = parse_command(&args("click @e5 --follow"), &default_flags()).unwrap(); + assert_eq!(cmd["selector"], "@e5"); + assert_eq!(cmd["follow"], true); + let cmd2 = parse_command(&args("click --follow @e5"), &default_flags()).unwrap(); + assert_eq!(cmd2["selector"], "@e5"); + assert_eq!(cmd2["follow"], true); + // Absent by default. + let plain = parse_command(&args("click @e5"), &default_flags()).unwrap(); + assert!(plain.get("follow").is_none()); + } + #[test] fn test_tabs_alias_lists() { assert_eq!( diff --git a/cli/src/native/actions.rs b/cli/src/native/actions.rs index dbe84ee..68147ac 100644 --- a/cli/src/native/actions.rs +++ b/cli/src/native/actions.rs @@ -3116,6 +3116,15 @@ async fn handle_click(cmd: &Value, state: &mut DaemonState) -> Result = + mgr.pages_list().into_iter().map(|p| p.target_id).collect(); interaction::click( &mgr.client, @@ -3128,7 +3137,26 @@ async fn handle_click(cmd: &Value, state: &mut DaemonState) -> Result Result { diff --git a/cli/src/native/browser.rs b/cli/src/native/browser.rs index d697220..d308b8f 100644 --- a/cli/src/native/browser.rs +++ b/cli/src/native/browser.rs @@ -1279,6 +1279,67 @@ impl BrowserManager { /// the active tab is preserved, and re-pinned if it was pruned. Powers a live /// `tab list` and adopt-by-targetId so a fresh session can reach a stranded, /// still-filled tab without reloading it (issue #21). + /// Detect targets that appeared since the `before` set (e.g. a click that + /// opened a new tab via a `target=_blank` link or `window.open`), attach + + /// track each in the background, and return the first newly-opened page. + /// + /// Lighter than [`resync_targets`] — one `getTargets` and work only on the + /// new targets, no whole-tab url/title refresh — so it's cheap enough to run + /// after every click. The new tab is added in the background (never steals + /// the active tab, per #7/#8.1); the caller surfaces it so the agent knows a + /// tab opened instead of seeing the old page (issue #24-A). + pub async fn adopt_newly_opened(&mut self, before: &HashSet) -> Option { + let result: GetTargetsResult = self + .client + .send_command_typed("Target.getTargets", &json!({}), None) + .await + .ok()?; + let live: Vec = result + .target_infos + .into_iter() + .filter(should_track_target) + .collect(); + let mut opened: Option = None; + for target in &live { + if before.contains(&target.target_id) + || self.pages.iter().any(|p| p.target_id == target.target_id) + { + continue; + } + let attach: AttachToTargetResult = match self + .client + .send_command_typed( + "Target.attachToTarget", + &AttachToTargetParams { + target_id: target.target_id.clone(), + flatten: true, + }, + None, + ) + .await + { + Ok(r) => r, + Err(_) => continue, + }; + let tab_id = self.assign_tab_id(); + let page = PageInfo { + tab_id, + label: None, + target_id: target.target_id.clone(), + session_id: attach.session_id.clone(), + url: target.url.clone(), + title: target.title.clone(), + target_type: target.target_type.clone(), + }; + self.add_background_page(page.clone()); + let _ = self.enable_domains(&attach.session_id).await; + if opened.is_none() { + opened = Some(page); + } + } + opened + } + pub async fn resync_targets(&mut self) -> Result<(), String> { self.client .send_command_typed::<_, Value>( diff --git a/cli/src/output.rs b/cli/src/output.rs index bb59fc2..be08aaa 100644 --- a/cli/src/output.rs +++ b/cli/src/output.rs @@ -186,6 +186,29 @@ pub fn print_response_with_opts(resp: &Response, action: Option<&str>, opts: &Ou } if let Some(data) = &resp.data { + // A click that opened a new tab: surface it so the agent doesn't read the + // unchanged old page as a failed click (issue #24-A). + if let Some(opened) = data.get("openedTab") { + let tid = opened.get("tabId").and_then(|v| v.as_str()).unwrap_or("?"); + let url = opened.get("url").and_then(|v| v.as_str()).unwrap_or(""); + let followed = data + .get("followed") + .and_then(|v| v.as_bool()) + .unwrap_or(false); + let verb = if followed { + "switched to new tab" + } else { + "opened new tab" + }; + eprintln!( + "{} {} [{}] {}", + color::cyan("→"), + verb, + tid, + color::dim(url) + ); + } + // Dialog status response if action == Some("dialog") { if let Some(has_dialog) = data.get("hasDialog").and_then(|v| v.as_bool()) {