diff --git a/cli/src/commands.rs b/cli/src/commands.rs index 5c1bd50..465b8d2 100644 --- a/cli/src/commands.rs +++ b/cli/src/commands.rs @@ -370,6 +370,12 @@ fn parse_command_inner(args: &[String], flags: &Flags) -> Result Result Result Result { - let mgr = state.browser.as_ref().ok_or("Browser not launched")?; +async fn handle_tab_list(cmd: &Value, state: &mut DaemonState) -> Result { + let mgr = state.browser.as_mut().ok_or("Browser not launched")?; + // Re-sync with the live browser so the list reflects tabs opened by other + // sessions or re-attached after a cross-process nav, and drops gone ones + // (issue #21). Best-effort: a stale list still beats erroring the command. + mgr.resync_targets().await.ok(); let tabs = mgr.tab_list(); // Echo `full` so the formatter prints untruncated URLs (issue #19). if cmd.get("full").and_then(|v| v.as_bool()).unwrap_or(false) { @@ -4394,9 +4412,20 @@ async fn handle_tab_switch(cmd: &Value, state: &mut DaemonState) -> Result` or a label)")?; - let tab_ref = super::browser::TabRef::parse(tab_ref_str)?; - let tab_id = mgr.resolve_tab_ref(&tab_ref)?; + .ok_or("Missing 'tabId' parameter (expected `t`, a label, or a targetId)")?; + // Re-sync first so a tab opened by another session, or one that re-attached + // after a cross-process nav, is adoptable from here (issue #21). + mgr.resync_targets().await.ok(); + // A CDP `targetId` (shown in `tab list`) is stable across sessions, so accept + // it directly for adopting a specific pre-existing tab — falling back to the + // per-session `t` / label form. + let tab_id = match mgr.tab_id_for_target(tab_ref_str) { + Some(id) => id, + None => { + let tab_ref = super::browser::TabRef::parse(tab_ref_str)?; + mgr.resolve_tab_ref(&tab_ref)? + } + }; state.ref_map.clear(); state.iframe_sessions.clear(); state.active_frame_id = None; diff --git a/cli/src/native/browser.rs b/cli/src/native/browser.rs index 1693b7b..65c3c12 100644 --- a/cli/src/native/browser.rs +++ b/cli/src/native/browser.rs @@ -106,6 +106,18 @@ pub(crate) fn should_track_target(target: &TargetInfo) -> bool { && (target.url.is_empty() || !is_internal_chrome_target(&target.url)) } +/// Origin + path of a URL, dropping the query string and fragment, for +/// `--reuse-tab` matching. SPA/SSO URLs carry volatile `?client_id=…&state=…` +/// and `#/route` parts, so two opens of the "same" page rarely match +/// byte-for-byte; comparing origin+path lands the reuse on the right tab. +/// Returns the input unchanged if it doesn't parse as a URL. +fn normalize_url_for_match(url: &str) -> String { + match url::Url::parse(url) { + Ok(u) => format!("{}{}", u.origin().ascii_serialization(), u.path()), + Err(_) => url.to_string(), + } +} + fn update_page_target_info_in_pages(pages: &mut [PageInfo], target: &TargetInfo) -> bool { if let Some(page) = pages.iter_mut().find(|p| p.target_id == target.target_id) { page.url = target.url.clone(); @@ -1184,22 +1196,168 @@ impl BrowserManager { } pub fn tab_list(&self) -> Vec { + let active = self.resolved_active_index(); self.pages .iter() .enumerate() .map(|(i, p)| { json!({ "tabId": format_tab_id(p.tab_id), + // Stable CDP target id. Unlike `t` (per-session, reassigned + // each connect) this is the same handle across every session + // attached to the relayed Chrome, so it's how you adopt a + // specific pre-existing tab from another session (issue #21). + "targetId": p.target_id, "label": p.label, "title": p.title, "url": p.url, "type": p.target_type, - "active": i == self.active_page_index, + "active": i == active, }) }) .collect() } + /// Stable `tab_id` for a page identified by its CDP `targetId`, if tracked. + /// Lets callers adopt a tab by the cross-session-stable target id. + pub fn tab_id_for_target(&self, target_id: &str) -> Option { + self.pages + .iter() + .find(|p| p.target_id == target_id) + .map(|p| p.tab_id) + } + + /// Re-pull the live target set and reconcile `self.pages`: adopt tabs that + /// appeared since connect (another session's tab, or one that just + /// re-attached after a cross-process nav), refresh url/title on known tabs, + /// and drop tabs that are gone (clearing phantom rows). Never steals focus — + /// 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). + pub async fn resync_targets(&mut self) -> Result<(), String> { + self.client + .send_command_typed::<_, Value>( + "Target.setDiscoverTargets", + &SetDiscoverTargetsParams { discover: true }, + None, + ) + .await?; + let result: GetTargetsResult = self + .client + .send_command_typed("Target.getTargets", &json!({}), None) + .await?; + let live: Vec = result + .target_infos + .into_iter() + .filter(should_track_target) + .collect(); + let live_ids: HashSet = live.iter().map(|t| t.target_id.clone()).collect(); + + for target in &live { + if self.update_page_target_info(target) { + continue; + } + // A target this session hasn't tracked yet — attach and add it in the + // background so it's listable/adoptable without stealing the active tab. + let attach_result: AttachToTargetResult = match self + .client + .send_command_typed( + "Target.attachToTarget", + &AttachToTargetParams { + target_id: target.target_id.clone(), + flatten: true, + }, + None, + ) + .await + { + Ok(r) => r, + // The tab may have closed between getTargets and attach, or be a + // restricted page — skip it rather than failing the whole resync. + Err(_) => continue, + }; + let tab_id = self.assign_tab_id(); + self.add_background_page(PageInfo { + tab_id, + label: None, + target_id: target.target_id.clone(), + session_id: attach_result.session_id.clone(), + url: target.url.clone(), + title: target.title.clone(), + target_type: target.target_type.clone(), + }); + let _ = self.enable_domains(&attach_result.session_id).await; + } + + // Drop tabs that no longer exist so `tab list` doesn't show phantom rows. + let gone: Vec = self + .pages + .iter() + .map(|p| p.target_id.clone()) + .filter(|tid| !live_ids.contains(tid)) + .collect(); + for tid in gone { + self.remove_page_by_target_id(&tid); + } + + // Refresh url/title from each live tab. The relay only stamps target_info + // on attach, so after a navigation its cached url/title go stale (or stay + // blank for a tab attached at about:blank) — which made `tab list` show + // blank rows you couldn't tell apart, defeating the point of listing them + // to pick a tab to adopt (issue #21). `Target.getTargetInfo` is a plain + // CDP read (no Runtime fingerprint), one cheap call per tab. + let sessions: Vec<(usize, String)> = self + .pages + .iter() + .enumerate() + .map(|(i, p)| (i, p.session_id.clone())) + .collect(); + for (i, sid) in sessions { + if sid.is_empty() { + continue; + } + if let Ok(resp) = self + .client + .send_command("Target.getTargetInfo", None, Some(&sid)) + .await + { + if let Some(ti) = resp.get("targetInfo") { + if let Some(page) = self.pages.get_mut(i) { + if let Some(u) = ti.get("url").and_then(|v| v.as_str()) { + if !u.is_empty() { + page.url = u.to_string(); + } + } + if let Some(t) = ti.get("title").and_then(|v| v.as_str()) { + page.title = t.to_string(); + } + } + } + } + } + Ok(()) + } + + /// If `--reuse-tab` and a tracked tab already shows `url`, switch to it + /// (without reloading, so any in-page state survives) and return its info. + /// Returns `None` when no tab matches and the caller should navigate/create. + /// Matches on exact URL or the same origin+path (ignoring query/fragment) so + /// a re-`open` of a stable entry URL lands on the existing tab instead of + /// piling up duplicates (issue #21). + pub async fn reuse_tab_for_url(&mut self, url: &str) -> Result, String> { + self.resync_targets().await.ok(); + let want = normalize_url_for_match(url); + let tab_id = self + .pages + .iter() + .find(|p| !want.is_empty() && (p.url == url || normalize_url_for_match(&p.url) == want)) + .map(|p| p.tab_id); + match tab_id { + Some(id) => Ok(Some(self.tab_switch_by_id(id).await?)), + None => Ok(None), + } + } + /// Resolve a user-supplied `TabRef` (either `t` or a label) to the /// stable numeric `tab_id`. Returns a teaching error for unknown tabs. pub fn resolve_tab_ref(&self, tab_ref: &TabRef) -> Result { @@ -2217,6 +2375,34 @@ mod tests { } } + // --- issue #21: --reuse-tab URL matching ignores query/fragment --- + + #[test] + fn normalize_url_match_strips_query_and_fragment() { + // Two opens of the "same" SSO page differ only in volatile query/hash — + // they must normalize equal so --reuse-tab lands on the existing tab. + let a = normalize_url_for_match( + "https://login.account.rakuten.com/sso/authorize?client_id=x&state=abc#/sign_in", + ); + let b = normalize_url_for_match( + "https://login.account.rakuten.com/sso/authorize?client_id=y&state=zzz#/forgot", + ); + assert_eq!(a, b); + assert_eq!(a, "https://login.account.rakuten.com/sso/authorize"); + } + + #[test] + fn normalize_url_match_distinguishes_different_paths() { + let cart = normalize_url_for_match("https://cart.step.rakuten.co.jp/cart"); + let order = normalize_url_for_match("https://cart.step.rakuten.co.jp/order"); + assert_ne!(cart, order); + } + + #[test] + fn normalize_url_match_passes_through_unparseable() { + assert_eq!(normalize_url_for_match("not a url"), "not a url"); + } + // --- issue #14: a pinned target must keep commands on the right tab --- #[test] diff --git a/cli/src/output.rs b/cli/src/output.rs index 957b27b..db6ba4a 100644 --- a/cli/src/output.rs +++ b/cli/src/output.rs @@ -512,6 +512,14 @@ pub fn print_response_with_opts(resp: &Response, action: Option<&str>, opts: &Ou } else { println!("{} [{}] {} - {}", marker, tab_id, title, url); } + // `--full` also surfaces the stable cross-session CDP targetId so + // a stranded tab can be adopted from another session via + // `tab ` (issue #21). + if full { + if let Some(target_id) = tab.get("targetId").and_then(|v| v.as_str()) { + println!(" {}", color::dim(&format!("target: {}", target_id))); + } + } } return; } @@ -3116,7 +3124,12 @@ Storage: storage Manage web storage Tabs: - tab [new|list|close|] Manage tabs + tab [new|list|close|] Manage tabs ( = t, a label, or a CDP targetId) + tab list --full Full URLs + stable cross-session targetId per tab + tab Adopt a specific tab (incl. another session's) by its + stable targetId, no reload — preserves in-page state + open --reuse-tab Reuse an existing tab on that URL instead of spawning + a duplicate (matches origin+path; preserves state) Diff: diff snapshot Compare current vs last snapshot diff --git a/skill-data/core/SKILL.md b/skill-data/core/SKILL.md index b22aa25..060c0e1 100644 --- a/skill-data/core/SKILL.md +++ b/skill-data/core/SKILL.md @@ -501,12 +501,26 @@ the same browser's existing targets, so a second session's first `open` can navigate a sibling's tab. For concurrent agents on one real Chrome, use the extension (each with a distinct `--session`), not raw `--cdp`. -Because each session owns its own tab group, **one session cannot read another -session's tabs** — a fresh session's `tab list` shows only its own (empty) group, -not the tab the first session opened. So if a session's handle dies (e.g. a tab -navigates across render processes), recover *that* session — reload, re-`open` -the URL, or `daemon restart` — rather than opening a second session to read the -first one's tab. There's no "settle in session A, attach session B to read it". +Each session owns its own tab group and assigns its own `t` indices (the same +physical tab is `t8` in one session, `t1` in another), so `t` is **not** a +stable cross-session handle. To reach a *specific* tab from another session — e.g. +a tab that was filled in a session whose handle later died — use the **stable CDP +`targetId`**: + +```bash +chrome-use tab list --full --session B # re-syncs live tabs; prints `target: ` per row +chrome-use tab --session B # adopt that exact tab, NO reload (state preserved) +``` + +`tab list` re-discovers the live tab set on every call, so a fresh session sees +tabs other sessions opened (and re-attached ones), not just its own. Adopting by +`targetId` lands session B on the stranded tab without reloading it, so a +half-filled form survives. Still, the simplest recovery for a session whose own +tab died is to recover *that* session (reload / re-`open` / `daemon restart`). + +To avoid piling up duplicate tabs when you re-`open` the same entry URL on +rebind, pass **`--reuse-tab`**: if a tab already shows that URL (matched by +origin+path), it switches to it instead of spawning a new one. ### Reset stuck daemon state