From 67dc63197742deea65372d68ef54d80ff6dae078 Mon Sep 17 00:00:00 2001 From: Daniel Hails Date: Thu, 16 Apr 2026 18:02:55 +0100 Subject: [PATCH] Consistent Tab IDs & Global Tag Targeting (#892) Introduces stable per-tab IDs and a global `--tab ` flag for scoping individual commands to a specific tab. Breaking change: response payloads for `tab_list`, `tab_new`, `tab_switch`, `tab_close`, and `window_new` now use `tabId` instead of `index`. `tab_close` returns `{tabId, closed: true}` instead of `{closed, activeIndex}`. `agent-browser tab ` now errors instead of silently listing tabs. Follow-up PR to land immediately after this fixes a compile error on the provider direct-page path, clears per-tab daemon state around scoped switches, and implements active-tab restoration so `--tab N` is non-intrusive as intended. --- cli/src/commands.rs | 84 ++++++--- cli/src/flags.rs | 33 ++++ cli/src/main.rs | 6 + cli/src/native/actions.rs | 37 ++-- cli/src/native/browser.rs | 55 +++++- cli/src/native/e2e_tests.rs | 337 +++++++++++++++++++++++++++++++++++- cli/src/output.rs | 111 +++++++++++- 7 files changed, 618 insertions(+), 45 deletions(-) diff --git a/cli/src/commands.rs b/cli/src/commands.rs index 274f997..8345c73 100644 --- a/cli/src/commands.rs +++ b/cli/src/commands.rs @@ -1010,28 +1010,35 @@ fn parse_command_inner(args: &[String], flags: &Flags) -> Result match rest.first().copied() { - Some("new") => { - let mut cmd = json!({ "id": id, "action": "tab_new" }); - if let Some(url) = rest.get(1) { - cmd["url"] = json!(url); + "tab" => { + const VALID: &[&str] = &["list", "new", "close", ""]; + match rest.first().copied() { + Some("new") => { + let mut cmd = json!({ "id": id, "action": "tab_new" }); + if let Some(url) = rest.get(1) { + cmd["url"] = json!(url); + } + Ok(cmd) } - Ok(cmd) - } - Some("list") => Ok(json!({ "id": id, "action": "tab_list" })), - Some("close") => { - let mut cmd = json!({ "id": id, "action": "tab_close" }); - if let Some(index) = rest.get(1).and_then(|s| s.parse::().ok()) { - cmd["index"] = json!(index); + Some("list") => Ok(json!({ "id": id, "action": "tab_list" })), + Some("close") => { + let mut cmd = json!({ "id": id, "action": "tab_close" }); + if let Some(tab_id) = rest.get(1).and_then(|s| s.parse::().ok()) { + cmd["tabId"] = json!(tab_id); + } + Ok(cmd) } - Ok(cmd) + Some(n) if n.parse::().is_ok() => { + let tab_id = n.parse::().expect("already checked parse succeeds"); + Ok(json!({ "id": id, "action": "tab_switch", "tabId": tab_id })) + } + Some(sub) => Err(ParseError::UnknownSubcommand { + subcommand: sub.to_string(), + valid_options: VALID, + }), + None => Ok(json!({ "id": id, "action": "tab_list" })), } - Some(n) if n.parse::().is_ok() => { - let index = n.parse::().expect("already checked parse succeeds"); - Ok(json!({ "id": id, "action": "tab_switch", "index": index })) - } - _ => Ok(json!({ "id": id, "action": "tab_list" })), - }, + } // === Window === "window" => { @@ -2335,6 +2342,7 @@ mod tests { fn default_flags() -> Flags { Flags { session: "test".to_string(), + tab: None, json: false, headed: false, debug: false, @@ -2847,7 +2855,7 @@ mod tests { fn test_tab_switch() { let cmd = parse_command(&args("tab 2"), &default_flags()).unwrap(); assert_eq!(cmd["action"], "tab_switch"); - assert_eq!(cmd["index"], 2); + assert_eq!(cmd["tabId"], 2); } #[test] @@ -2856,6 +2864,42 @@ mod tests { assert_eq!(cmd["action"], "tab_close"); } + #[test] + fn test_tab_close_with_id() { + let cmd = parse_command(&args("tab close 2"), &default_flags()).unwrap(); + assert_eq!(cmd["action"], "tab_close"); + assert_eq!(cmd["tabId"], 2); + } + + #[test] + fn test_tab_switch_sends_tab_id() { + let cmd = parse_command(&args("tab 2"), &default_flags()).unwrap(); + assert_eq!(cmd["tabId"], 2); + assert!(cmd.get("index").is_none()); + } + + #[test] + fn test_tab_close_sends_tab_id() { + let cmd = parse_command(&args("tab close 3"), &default_flags()).unwrap(); + assert_eq!(cmd["tabId"], 3); + assert!(cmd.get("index").is_none()); + } + + #[test] + fn test_tab_no_args_defaults_to_list() { + let cmd = parse_command(&args("tab"), &default_flags()).unwrap(); + assert_eq!(cmd["action"], "tab_list"); + } + + #[test] + fn test_tab_unknown_subcommand_errors() { + let result = parse_command(&args("tab select 3"), &default_flags()); + assert!( + result.is_err(), + "tab select should error, not silently fall through to tab_list" + ); + } + // === Network === #[test] diff --git a/cli/src/flags.rs b/cli/src/flags.rs index 6e517c9..924fc14 100644 --- a/cli/src/flags.rs +++ b/cli/src/flags.rs @@ -57,6 +57,7 @@ pub struct Config { pub json: Option, pub debug: Option, pub session: Option, + pub tab: Option, pub session_name: Option, pub executable_path: Option, pub extensions: Option>, @@ -98,6 +99,7 @@ impl Config { json: other.json.or(self.json), debug: other.debug.or(self.debug), session: other.session.or(self.session), + tab: other.tab.or(self.tab), session_name: other.session_name.or(self.session_name), executable_path: other.executable_path.or(self.executable_path), extensions: match (self.extensions, other.extensions) { @@ -196,6 +198,7 @@ fn parse_bool_arg(args: &[String], i: usize) -> (bool, bool) { fn extract_config_path(args: &[String]) -> Option> { const FLAGS_WITH_VALUE: &[&str] = &[ "--session", + "--tab", "--headers", "--executable-path", "--cdp", @@ -273,6 +276,7 @@ pub struct Flags { pub headed: bool, pub debug: bool, pub session: String, + pub tab: Option, pub headers: Option, pub executable_path: Option, pub cdp: Option, @@ -355,6 +359,7 @@ pub fn parse_flags(args: &[String]) -> Flags { .ok() .or(config.session) .unwrap_or_else(|| "default".to_string()), + tab: config.tab, headers: config.headers, executable_path: env::var("AGENT_BROWSER_EXECUTABLE_PATH") .ok() @@ -492,6 +497,12 @@ pub fn parse_flags(args: &[String]) -> Flags { i += 1; } } + "--tab" => { + if let Some(s) = args.get(i + 1) { + flags.tab = s.parse::().ok(); + i += 1; + } + } "--idle-timeout" => { if let Some(s) = args.get(i + 1) { match parse_idle_timeout(s) { @@ -775,6 +786,7 @@ pub fn clean_args(args: &[String]) -> Vec { // Global flags that always take a value (need to skip the next arg too) const GLOBAL_FLAGS_WITH_VALUE: &[&str] = &[ "--session", + "--tab", "--headers", "--executable-path", "--cdp", @@ -1441,4 +1453,25 @@ mod tests { let clean = clean_args(&input); assert_eq!(clean, vec!["open", "example.com"]); } + + // === Tab flag tests === + + #[test] + fn test_parse_tab_flag() { + let flags = parse_flags(&args("--tab 4 snapshot")); + assert_eq!(flags.tab, Some(4)); + } + + #[test] + fn test_clean_args_removes_tab_flag() { + let cleaned = clean_args(&args("--tab 4 snapshot")); + assert_eq!(cleaned, vec!["snapshot"]); + } + + #[test] + fn test_parse_tab_config() { + let json = r#"{"tab": 4}"#; + let config: Config = serde_json::from_str(json).unwrap(); + assert_eq!(config.tab, Some(4)); + } } diff --git a/cli/src/main.rs b/cli/src/main.rs index afc8cb4..c6eab4b 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -738,6 +738,12 @@ fn main() { } }; + if let Some(tab_id) = flags.tab { + if cmd.get("tabId").is_none() { + cmd["tabId"] = json!(tab_id); + } + } + // Handle --password-stdin for auth save if cmd.get("action").and_then(|v| v.as_str()) == Some("auth_save") { if cmd.get("password").is_some() { diff --git a/cli/src/native/actions.rs b/cli/src/native/actions.rs index 2cca7d3..63ac093 100644 --- a/cli/src/native/actions.rs +++ b/cli/src/native/actions.rs @@ -662,7 +662,9 @@ impl DaemonState { .await; } + let tab_id = mgr.assign_tab_id(); mgr.add_page(super::browser::PageInfo { + tab_id, target_id: te.target_info.target_id.clone(), session_id: attach.session_id, url: te.target_info.url.clone(), @@ -1273,6 +1275,20 @@ pub async fn execute_command(cmd: &Value, state: &mut DaemonState) -> Value { ); } + // Pre-dispatch: if tabId is set on a non-tab command, switch to that tab first + if !matches!( + action, + "tab_list" | "tab_new" | "tab_switch" | "tab_close" | "launch" | "close" + ) { + if let Some(tab_id) = cmd.get("tabId").and_then(|v| v.as_u64()) { + if let Some(ref mut mgr) = state.browser { + if let Err(e) = mgr.tab_switch_by_id(tab_id as u32).await { + return error_response(&id, &e); + } + } + } + } + let result = match action { "launch" => handle_launch(cmd, state).await, "navigate" => handle_navigate(cmd, state).await, @@ -3647,14 +3663,14 @@ async fn handle_tab_new(cmd: &Value, state: &mut DaemonState) -> Result Result { let mgr = state.browser.as_mut().ok_or("Browser not launched")?; - let index = cmd - .get("index") + let tab_id = cmd + .get("tabId") .and_then(|v| v.as_u64()) - .ok_or("Missing 'index' parameter")? as usize; + .ok_or("Missing 'tabId' parameter")? as u32; state.ref_map.clear(); state.iframe_sessions.clear(); state.active_frame_id = None; - let result = mgr.tab_switch(index).await?; + let result = mgr.tab_switch_by_id(tab_id).await?; if let Some(ref server) = state.stream_server { if let Ok(dims) = mgr @@ -3679,14 +3695,11 @@ async fn handle_tab_switch(cmd: &Value, state: &mut DaemonState) -> Result Result { let mgr = state.browser.as_mut().ok_or("Browser not launched")?; - let index = cmd - .get("index") - .and_then(|v| v.as_u64()) - .map(|i| i as usize); + let tab_id = cmd.get("tabId").and_then(|v| v.as_u64()).map(|i| i as u32); state.ref_map.clear(); state.iframe_sessions.clear(); state.active_frame_id = None; - mgr.tab_close(index).await + mgr.tab_close_by_id(tab_id).await } async fn handle_viewport(cmd: &Value, state: &mut DaemonState) -> Result { @@ -4065,7 +4078,9 @@ async fn handle_recording_start(cmd: &Value, state: &mut DaemonState) -> Result< } // Add page and switch to it + let tab_id = mgr.assign_tab_id(); mgr.add_page(super::browser::PageInfo { + tab_id, target_id: create_result.target_id, session_id: new_session_id.clone(), url: nav_url.clone(), @@ -5976,7 +5991,9 @@ async fn handle_window_new(cmd: &Value, state: &mut DaemonState) -> Result Result Result { diff --git a/cli/src/native/browser.rs b/cli/src/native/browser.rs index eafac54..4ab8fa0 100644 --- a/cli/src/native/browser.rs +++ b/cli/src/native/browser.rs @@ -158,6 +158,7 @@ pub fn to_ai_friendly_error(error: &str) -> String { #[derive(Debug, Clone)] pub struct PageInfo { + pub tab_id: u32, pub target_id: String, pub session_id: String, pub url: String, @@ -226,6 +227,7 @@ pub struct BrowserManager { pub ignore_https_errors: bool, /// Origins visited during this session, used by save_state to collect cross-origin localStorage. visited_origins: HashSet, + next_tab_id: u32, } const LIGHTPANDA_CDP_CONNECT_TIMEOUT: Duration = Duration::from_secs(5); @@ -297,6 +299,7 @@ impl BrowserManager { download_path: download_path.clone(), ignore_https_errors, visited_origins: HashSet::new(), + next_tab_id: 1, }; manager.discover_and_attach_targets().await?; manager @@ -385,6 +388,7 @@ impl BrowserManager { download_path: None, ignore_https_errors: false, visited_origins: HashSet::new(), + next_tab_id: 1, }; if direct_page { @@ -453,7 +457,10 @@ impl BrowserManager { ) .await?; + let tab_id = self.next_tab_id; + self.next_tab_id += 1; self.pages.push(PageInfo { + tab_id, target_id: result.target_id, session_id: attach_result.session_id.clone(), url: "about:blank".to_string(), @@ -476,7 +483,10 @@ impl BrowserManager { ) .await?; + let tab_id = self.next_tab_id; + self.next_tab_id += 1; self.pages.push(PageInfo { + tab_id, target_id: target.target_id.clone(), session_id: attach_result.session_id.clone(), url: target.url.clone(), @@ -821,7 +831,10 @@ impl BrowserManager { ) .await?; + let tab_id = self.next_tab_id; + self.next_tab_id += 1; self.pages.push(PageInfo { + tab_id, target_id: result.target_id, session_id: attach_result.session_id.clone(), url: "about:blank".to_string(), @@ -864,7 +877,7 @@ impl BrowserManager { .enumerate() .map(|(i, p)| { json!({ - "index": i, + "tabId": p.tab_id, "title": p.title, "url": p.url, "type": p.target_type, @@ -902,8 +915,11 @@ impl BrowserManager { self.enable_domains(&attach.session_id).await?; + let tab_id = self.next_tab_id; + self.next_tab_id += 1; let index = self.pages.len(); self.pages.push(PageInfo { + tab_id, target_id: result.target_id, session_id: attach.session_id, url: target_url.to_string(), @@ -912,7 +928,7 @@ impl BrowserManager { }); self.active_page_index = index; - Ok(json!({ "index": index, "url": target_url })) + Ok(json!({ "tabId": tab_id, "url": target_url, "total": self.pages.len() })) } pub async fn tab_switch(&mut self, index: usize) -> Result { @@ -942,7 +958,8 @@ impl BrowserManager { page.title = title.clone(); } - Ok(json!({ "index": index, "url": url, "title": title })) + let tab_id = self.pages[index].tab_id; + Ok(json!({ "tabId": tab_id, "url": url, "title": title })) } pub async fn tab_close(&mut self, index: Option) -> Result { @@ -958,6 +975,7 @@ impl BrowserManager { let page = self.pages.remove(target_index); self.update_active_page_after_removal(target_index); + let closed_tab_id = page.tab_id; let _ = self .client .send_command_typed::<_, Value>( @@ -972,7 +990,7 @@ impl BrowserManager { let session_id = self.pages[self.active_page_index].session_id.clone(); self.enable_domains(&session_id).await?; - Ok(json!({ "closed": target_index, "activeIndex": self.active_page_index })) + Ok(json!({ "tabId": closed_tab_id, "closed": true })) } // ----------------------------------------------------------------------- @@ -1213,6 +1231,34 @@ impl BrowserManager { .to_string()) } + pub async fn tab_switch_by_id(&mut self, tab_id: u32) -> Result { + let index = self + .pages + .iter() + .position(|p| p.tab_id == tab_id) + .ok_or_else(|| format!("Tab ID {} not found", tab_id))?; + self.tab_switch(index).await + } + + pub async fn tab_close_by_id(&mut self, tab_id: Option) -> Result { + let index = match tab_id { + Some(id) => Some( + self.pages + .iter() + .position(|p| p.tab_id == id) + .ok_or_else(|| format!("Tab ID {} not found", id))?, + ), + None => None, + }; + self.tab_close(index).await + } + + pub fn assign_tab_id(&mut self) -> u32 { + let id = self.next_tab_id; + self.next_tab_id += 1; + id + } + pub fn add_page(&mut self, page: PageInfo) { let index = self.pages.len(); self.pages.push(page); @@ -1393,6 +1439,7 @@ async fn initialize_lightpanda_manager( download_path: None, ignore_https_errors: false, visited_origins: HashSet::new(), + next_tab_id: 1, }; match discover_and_attach_lightpanda_targets(&mut manager, deadline).await { diff --git a/cli/src/native/e2e_tests.rs b/cli/src/native/e2e_tests.rs index 37e27eb..d648e4b 100644 --- a/cli/src/native/e2e_tests.rs +++ b/cli/src/native/e2e_tests.rs @@ -905,12 +905,13 @@ async fn e2e_tabs() { .await; assert_success(&resp); - // Tab list should show 1 tab + // Tab list should show 1 tab with tabId 1 let resp = execute_command(&json!({ "id": "3", "action": "tab_list" }), &mut state).await; assert_success(&resp); let tabs = get_data(&resp)["tabs"].as_array().unwrap(); assert_eq!(tabs.len(), 1); assert_eq!(tabs[0]["active"], true); + assert_eq!(tabs[0]["tabId"], 1, "First tab should have tabId 1"); // Open new tab let resp = execute_command( @@ -919,18 +920,21 @@ async fn e2e_tabs() { ) .await; assert_success(&resp); - assert_eq!(get_data(&resp)["index"], 1); + assert_eq!(get_data(&resp)["tabId"], 2, "New tab should have tabId 2"); + assert_eq!(get_data(&resp)["total"], 2); - // Tab list should show 2 tabs + // Tab list should show 2 tabs with distinct, incrementing tabIds let resp = execute_command(&json!({ "id": "5", "action": "tab_list" }), &mut state).await; assert_success(&resp); let tabs = get_data(&resp)["tabs"].as_array().unwrap(); assert_eq!(tabs.len(), 2); assert_eq!(tabs[1]["active"], true); + assert_eq!(tabs[0]["tabId"], 1, "First tab should keep tabId 1"); + assert_eq!(tabs[1]["tabId"], 2, "Second tab should have tabId 2"); // Switch to first tab let resp = execute_command( - &json!({ "id": "6", "action": "tab_switch", "index": 0 }), + &json!({ "id": "6", "action": "tab_switch", "tabId": 1 }), &mut state, ) .await; @@ -946,7 +950,7 @@ async fn e2e_tabs() { // Close second tab let resp = execute_command( - &json!({ "id": "8", "action": "tab_close", "index": 1 }), + &json!({ "id": "8", "action": "tab_close", "tabId": 2 }), &mut state, ) .await; @@ -962,6 +966,329 @@ async fn e2e_tabs() { assert_success(&resp); } +#[tokio::test] +#[ignore] +async fn e2e_tab_ids_not_reused() { + let mut state = DaemonState::new(); + + let resp = execute_command( + &json!({ "id": "1", "action": "launch", "headless": true }), + &mut state, + ) + .await; + assert_success(&resp); + + // First tab gets tabId 1 + let resp = execute_command(&json!({ "id": "2", "action": "tab_list" }), &mut state).await; + assert_success(&resp); + let tabs = get_data(&resp)["tabs"].as_array().unwrap(); + assert_eq!(tabs[0]["tabId"], 1); + + // Open tab 2 and tab 3 + let resp = execute_command( + &json!({ "id": "3", "action": "tab_new", "url": "data:text/html,

Tab 2

" }), + &mut state, + ) + .await; + assert_success(&resp); + assert_eq!(get_data(&resp)["tabId"], 2); + + let resp = execute_command( + &json!({ "id": "4", "action": "tab_new", "url": "data:text/html,

Tab 3

" }), + &mut state, + ) + .await; + assert_success(&resp); + assert_eq!(get_data(&resp)["tabId"], 3); + + // Close tab 2 + let resp = execute_command( + &json!({ "id": "5", "action": "tab_close", "tabId": 2 }), + &mut state, + ) + .await; + assert_success(&resp); + + // Open a new tab — should get tabId 4, NOT 2 + let resp = execute_command( + &json!({ "id": "6", "action": "tab_new", "url": "data:text/html,

Tab 4

" }), + &mut state, + ) + .await; + assert_success(&resp); + assert_eq!( + get_data(&resp)["tabId"], + 4, + "Tab IDs must not be reused after closing" + ); + + // Verify final state: tabs 1, 3, 4 + let resp = execute_command(&json!({ "id": "7", "action": "tab_list" }), &mut state).await; + assert_success(&resp); + let tabs = get_data(&resp)["tabs"].as_array().unwrap(); + assert_eq!(tabs.len(), 3); + let ids: Vec = tabs.iter().map(|t| t["tabId"].as_i64().unwrap()).collect(); + assert_eq!(ids, vec![1, 3, 4]); + + let resp = execute_command(&json!({ "id": "99", "action": "close" }), &mut state).await; + assert_success(&resp); +} + +#[tokio::test] +#[ignore] +async fn e2e_tab_global_targeting() { + let mut state = DaemonState::new(); + + let resp = execute_command( + &json!({ "id": "1", "action": "launch", "headless": true }), + &mut state, + ) + .await; + assert_success(&resp); + + // Navigate tab 1 + let resp = execute_command( + &json!({ "id": "2", "action": "navigate", "url": "data:text/html,

Page A

" }), + &mut state, + ) + .await; + assert_success(&resp); + + // Open tab 2 (becomes active) + let resp = execute_command( + &json!({ "id": "3", "action": "tab_new", "url": "data:text/html,

Page B

" }), + &mut state, + ) + .await; + assert_success(&resp); + assert_eq!(get_data(&resp)["tabId"], 2); + + // Use tabId to evaluate on tab 1 while tab 2 is active + // (simulates --tab 1 evaluate ...) + let resp = execute_command( + &json!({ "id": "4", "action": "evaluate", "tabId": 1, "script": "document.querySelector('h1').textContent" }), + &mut state, + ) + .await; + assert_success(&resp); + assert_eq!( + get_data(&resp)["result"], + "Page A", + "tabId should target tab 1 even though tab 2 was active" + ); + + // Verify tab 2 content is still accessible + let resp = execute_command( + &json!({ "id": "5", "action": "evaluate", "tabId": 2, "script": "document.querySelector('h1').textContent" }), + &mut state, + ) + .await; + assert_success(&resp); + assert_eq!(get_data(&resp)["result"], "Page B"); + + // Without tabId, should use the current active tab (now tab 1 from the switch) + let resp = execute_command( + &json!({ "id": "6", "action": "evaluate", "script": "document.querySelector('h1').textContent" }), + &mut state, + ) + .await; + assert_success(&resp); + // After targeting tab 1 then tab 2, active tab is now tab 2 + assert_eq!(get_data(&resp)["result"], "Page B"); + + let resp = execute_command(&json!({ "id": "99", "action": "close" }), &mut state).await; + assert_success(&resp); +} + +#[tokio::test] +#[ignore] +async fn e2e_tab_global_targeting_snapshot() { + let mut state = DaemonState::new(); + + let resp = execute_command( + &json!({ "id": "1", "action": "launch", "headless": true }), + &mut state, + ) + .await; + assert_success(&resp); + + // Navigate tab 1 + let resp = execute_command( + &json!({ "id": "2", "action": "navigate", "url": "data:text/html,

Page A

" }), + &mut state, + ) + .await; + assert_success(&resp); + + // Open tab 2 (becomes active) + let resp = execute_command( + &json!({ "id": "3", "action": "tab_new", "url": "data:text/html,

Page B

" }), + &mut state, + ) + .await; + assert_success(&resp); + assert_eq!(get_data(&resp)["tabId"], 2); + + // Snapshot tab 1 via tabId while tab 2 is active + let resp = execute_command( + &json!({ "id": "4", "action": "snapshot", "tabId": 1 }), + &mut state, + ) + .await; + assert_success(&resp); + let snapshot = get_data(&resp)["snapshot"].as_str().unwrap(); + assert!( + snapshot.contains("Page A"), + "Snapshot with tabId=1 should contain 'Page A', got: {}", + snapshot + ); + assert!( + !snapshot.contains("Page B"), + "Snapshot with tabId=1 should NOT contain 'Page B', got: {}", + snapshot + ); + + // Snapshot tab 2 via tabId + let resp = execute_command( + &json!({ "id": "5", "action": "snapshot", "tabId": 2 }), + &mut state, + ) + .await; + assert_success(&resp); + let snapshot = get_data(&resp)["snapshot"].as_str().unwrap(); + assert!( + snapshot.contains("Page B"), + "Snapshot with tabId=2 should contain 'Page B', got: {}", + snapshot + ); + assert!( + !snapshot.contains("Page A"), + "Snapshot with tabId=2 should NOT contain 'Page A', got: {}", + snapshot + ); + + // Snapshot without tabId should use the last-switched tab (tab 2) + let resp = execute_command(&json!({ "id": "6", "action": "snapshot" }), &mut state).await; + assert_success(&resp); + let snapshot = get_data(&resp)["snapshot"].as_str().unwrap(); + assert!( + snapshot.contains("Page B"), + "Snapshot without tabId should use active tab (Page B), got: {}", + snapshot + ); + + let resp = execute_command(&json!({ "id": "99", "action": "close" }), &mut state).await; + assert_success(&resp); +} + +#[tokio::test] +#[ignore] +async fn e2e_tab_global_targeting_snapshot_non_contiguous() { + // Reproduces the bug where --tab 3 snapshot shows tab 1's content + // when tab IDs are non-contiguous (e.g. tabs [1] and [3] after + // closing tab [2]). + let mut state = DaemonState::new(); + + let resp = execute_command( + &json!({ "id": "1", "action": "launch", "headless": true }), + &mut state, + ) + .await; + assert_success(&resp); + + // Navigate tab 1 to Page A + let resp = execute_command( + &json!({ "id": "2", "action": "navigate", "url": "data:text/html,

Page A

" }), + &mut state, + ) + .await; + assert_success(&resp); + + // Open tab 2 + let resp = execute_command( + &json!({ "id": "3", "action": "tab_new", "url": "data:text/html,

Page B

" }), + &mut state, + ) + .await; + assert_success(&resp); + assert_eq!(get_data(&resp)["tabId"], 2); + + // Open tab 3 + let resp = execute_command( + &json!({ "id": "4", "action": "tab_new", "url": "data:text/html,

Page C

" }), + &mut state, + ) + .await; + assert_success(&resp); + assert_eq!(get_data(&resp)["tabId"], 3); + + // Close tab 2 to create non-contiguous IDs: [1, 3] + let resp = execute_command( + &json!({ "id": "5", "action": "tab_close", "tabId": 2 }), + &mut state, + ) + .await; + assert_success(&resp); + + // Verify tab list shows [1] and [3] + let resp = execute_command(&json!({ "id": "6", "action": "tab_list" }), &mut state).await; + assert_success(&resp); + let tabs = get_data(&resp)["tabs"].as_array().unwrap(); + assert_eq!(tabs.len(), 2); + assert_eq!(tabs[0]["tabId"], 1); + assert_eq!(tabs[1]["tabId"], 3); + + // Switch active tab back to tab 1 + let resp = execute_command( + &json!({ "id": "7", "action": "tab_switch", "tabId": 1 }), + &mut state, + ) + .await; + assert_success(&resp); + + // Snapshot tab 3 via tabId while tab 1 is active + // (simulates: --tab 3 snapshot) + let resp = execute_command( + &json!({ "id": "8", "action": "snapshot", "tabId": 3 }), + &mut state, + ) + .await; + assert_success(&resp); + let snapshot = get_data(&resp)["snapshot"].as_str().unwrap(); + assert!( + snapshot.contains("Page C"), + "Snapshot with tabId=3 should contain 'Page C', got: {}", + snapshot + ); + assert!( + !snapshot.contains("Page A"), + "Snapshot with tabId=3 should NOT contain 'Page A', got: {}", + snapshot + ); + + // Snapshot tab 1 via tabId + let resp = execute_command( + &json!({ "id": "9", "action": "snapshot", "tabId": 1 }), + &mut state, + ) + .await; + assert_success(&resp); + let snapshot = get_data(&resp)["snapshot"].as_str().unwrap(); + assert!( + snapshot.contains("Page A"), + "Snapshot with tabId=1 should contain 'Page A', got: {}", + snapshot + ); + assert!( + !snapshot.contains("Page C"), + "Snapshot with tabId=1 should NOT contain 'Page C', got: {}", + snapshot + ); + + let resp = execute_command(&json!({ "id": "99", "action": "close" }), &mut state).await; + assert_success(&resp); +} + // --------------------------------------------------------------------------- // Element queries: isvisible, isenabled, gettext, getattribute // --------------------------------------------------------------------------- diff --git a/cli/src/output.rs b/cli/src/output.rs index f23301d..bac0c9f 100644 --- a/cli/src/output.rs +++ b/cli/src/output.rs @@ -406,7 +406,11 @@ pub fn print_response_with_opts(resp: &Response, action: Option<&str>, opts: &Ou } // Tabs if let Some(tabs) = data.get("tabs").and_then(|v| v.as_array()) { - for (i, tab) in tabs.iter().enumerate() { + for tab in tabs { + let tab_id = tab + .get("tabId") + .and_then(|v| v.as_i64()) + .unwrap_or_default(); let title = tab .get("title") .and_then(|v| v.as_str()) @@ -418,10 +422,47 @@ pub fn print_response_with_opts(resp: &Response, action: Option<&str>, opts: &Ou } else { " ".to_string() }; - println!("{} [{}] {} - {}", marker, i, title, url); + println!("{} [{}] {} - {}", marker, tab_id, title, url); } return; } + // Tab switch + if action == Some("tab_switch") { + if let Some(tab_id) = data.get("tabId").and_then(|v| v.as_i64()) { + if let Some(url) = data.get("url").and_then(|v| v.as_str()) { + println!( + "{} Switched to tab [{}] ({})", + color::success_indicator(), + tab_id, + url + ); + } else { + println!( + "{} Switched to tab [{}]", + color::success_indicator(), + tab_id + ); + } + return; + } + } + // New tab/window + if let Some(tab_id) = data.get("tabId").and_then(|v| v.as_i64()) { + if let Some(total) = data.get("total").and_then(|v| v.as_i64()) { + let label = match action { + Some("window_new") => "Window opened", + _ => "Tab opened", + }; + println!( + "{} {} [{}] ({} total)", + color::success_indicator(), + label, + tab_id, + total + ); + return; + } + } // Console logs if let Some(logs) = data.get("messages").and_then(|v| v.as_array()) { if opts.content_boundaries { @@ -562,7 +603,13 @@ pub fn print_response_with_opts(resp: &Response, action: Option<&str>, opts: &Ou // Closed (browser or tab) if data.get("closed").is_some() { let label = match action { - Some("tab_close") => "Tab closed", + Some("tab_close") => { + if let Some(closed_id) = data.get("tabId").and_then(|v| v.as_i64()) { + println!("{} Tab [{}] closed", color::success_indicator(), closed_id); + return; + } + "Tab closed" + } _ => "Browser closed", }; println!("{} {}", color::success_indicator(), label); @@ -1010,6 +1057,7 @@ Aliases: goto, navigate Global Options: --json Output as JSON --session Use specific session + --tab Target specific tab ID --headers Set HTTP headers (scoped to this origin) --headed Show browser window @@ -1033,6 +1081,7 @@ the browser's back button. Global Options: --json Output as JSON --session Use specific session + --tab Target specific tab ID Examples: agent-browser back @@ -1050,6 +1099,7 @@ the browser's forward button. Global Options: --json Output as JSON --session Use specific session + --tab Target specific tab ID Examples: agent-browser forward @@ -1067,6 +1117,7 @@ the browser's reload button. Global Options: --json Output as JSON --session Use specific session + --tab Target specific tab ID Examples: agent-browser reload @@ -1090,6 +1141,7 @@ Options: Global Options: --json Output as JSON --session Use specific session + --tab Target specific tab ID Examples: agent-browser click "#submit-button" @@ -1111,6 +1163,7 @@ or triggering double-click handlers. Global Options: --json Output as JSON --session Use specific session + --tab Target specific tab ID Examples: agent-browser dblclick "#editable-text" @@ -1129,6 +1182,7 @@ This replaces any existing content in the field. Global Options: --json Output as JSON --session Use specific session + --tab Target specific tab ID Examples: agent-browser fill "#email" "user@example.com" @@ -1148,6 +1202,7 @@ Unlike fill, this does not clear existing content first. Global Options: --json Output as JSON --session Use specific session + --tab Target specific tab ID Examples: agent-browser type "#search" "hello" @@ -1171,6 +1226,7 @@ triggering hover states or dropdown menus. Global Options: --json Output as JSON --session Use specific session + --tab Target specific tab ID Examples: agent-browser hover "#dropdown-trigger" @@ -1188,6 +1244,7 @@ Sets keyboard focus to the specified element. Global Options: --json Output as JSON --session Use specific session + --tab Target specific tab ID Examples: agent-browser focus "#input-field" @@ -1205,6 +1262,7 @@ Checks a checkbox element. If already checked, no action is taken. Global Options: --json Output as JSON --session Use specific session + --tab Target specific tab ID Examples: agent-browser check "#terms-checkbox" @@ -1222,6 +1280,7 @@ Unchecks a checkbox element. If already unchecked, no action is taken. Global Options: --json Output as JSON --session Use specific session + --tab Target specific tab ID Examples: agent-browser uncheck "#newsletter-opt-in" @@ -1239,6 +1298,7 @@ Selects one or more options in a