diff --git a/README.md b/README.md index 658eec2..069c544 100644 --- a/README.md +++ b/README.md @@ -290,22 +290,30 @@ agent-browser network har stop [output.har] # Stop and save HAR (temp path if ### Tabs & Windows ```bash -agent-browser tab # List tabs (shows stable `tabId` for each) -agent-browser tab new [url] # New tab (optionally with URL) -agent-browser tab # Switch to tab by id -agent-browser tab close [id] # Close tab by id (defaults to active tab) -agent-browser window new # New window +agent-browser tab # List tabs (shows `tabId` and optional label) +agent-browser tab new [url] # New tab (optionally with URL) +agent-browser tab new --label docs [url] # New tab with a user-assigned label +agent-browser tab |label> # Switch to a tab by id or label +agent-browser tab close [t|label] # Close a tab (defaults to active) +agent-browser window new # New window ``` -Tab IDs are stable and never reused within a session, so agents can keep -referring to the same tab across commands even if other tabs are opened or -closed in between. To run a single command against a specific tab without -changing the active tab, use the global `--tab ` flag: +Tab ids are stable strings of the form `t1`, `t2`, `t3`. They're never reused +within a session, so scripts and agents can keep referring to the same tab +even after other tabs are opened or closed. Positional integers like `tab 2` +are **not** accepted; the `t` prefix disambiguates handles from indices and +mirrors the `@e1` convention used for element refs. + +You can also assign a memorable label (`docs`, `app`, `admin`) and use it +interchangeably with the id. Labels are never auto-generated and never +rewritten on navigation — they're yours to name and keep: ```bash -agent-browser tab new https://docs.example.com # opens and activates tab 2 -agent-browser --tab 1 snapshot # peek at tab 1 (tab 2 stays active) -agent-browser click "#submit" # runs on tab 2 as expected +agent-browser tab new --label docs https://docs.example.com +agent-browser tab docs # switch to the docs tab +agent-browser snapshot # populate refs for docs +agent-browser click @e3 # click uses docs's refs +agent-browser tab close docs # close by label ``` ### Frames @@ -621,7 +629,6 @@ This is useful for multimodal AI models that can reason about visual layout, unl |--------|-------------| | `--session ` | Use isolated session (or `AGENT_BROWSER_SESSION` env) | | `--session-name ` | Auto-save/restore session state (or `AGENT_BROWSER_SESSION_NAME` env) | -| `--tab ` | Target a specific tab by stable `tabId` for this command only; the active tab is restored afterward | | `--profile ` | Chrome profile name or persistent directory path (or `AGENT_BROWSER_PROFILE` env) | | `--state ` | Load storage state from JSON file (or `AGENT_BROWSER_STATE` env) | | `--headers ` | Set HTTP headers scoped to the URL's origin | diff --git a/agent-browser.schema.json b/agent-browser.schema.json index ac92818..48e38ba 100644 --- a/agent-browser.schema.json +++ b/agent-browser.schema.json @@ -24,11 +24,6 @@ "type": "string", "description": "Auto-save/load state persistence name." }, - "tab": { - "type": "integer", - "minimum": 1, - "description": "Target a specific tab by stable tabId for this command only. The active tab is restored afterward." - }, "executablePath": { "type": "string", "description": "Path to a custom browser executable." diff --git a/cli/src/commands.rs b/cli/src/commands.rs index 8345c73..5858637 100644 --- a/cli/src/commands.rs +++ b/cli/src/commands.rs @@ -1011,31 +1011,51 @@ fn parse_command_inner(args: &[String], flags: &Flags) -> Result { - const VALID: &[&str] = &["list", "new", "close", ""]; match rest.first().copied() { Some("new") => { + // Accepted forms: + // tab new [url] + // tab new --label [url] + // tab new [url] --label let mut cmd = json!({ "id": id, "action": "tab_new" }); - if let Some(url) = rest.get(1) { - cmd["url"] = json!(url); + let mut i = 1; + while i < rest.len() { + match rest[i] { + "--label" => { + let name = rest.get(i + 1).ok_or(ParseError::MissingArguments { + context: "tab new --label".to_string(), + usage: "tab new --label [url]", + })?; + cmd["label"] = json!(name); + i += 2; + } + other if !other.starts_with("--") && cmd.get("url").is_none() => { + cmd["url"] = json!(other); + i += 1; + } + other => { + return Err(ParseError::UnknownSubcommand { + subcommand: other.to_string(), + valid_options: &["--label", ""], + }); + } + } } 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(tab_id) = rest.get(1).and_then(|s| s.parse::().ok()) { - cmd["tabId"] = json!(tab_id); + if let Some(tab_ref) = rest.get(1) { + cmd["tabId"] = json!(tab_ref); } 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, - }), + Some(tab_ref) => Ok(json!({ + "id": id, + "action": "tab_switch", + "tabId": tab_ref, + })), None => Ok(json!({ "id": id, "action": "tab_list" })), } } @@ -2342,7 +2362,6 @@ mod tests { fn default_flags() -> Flags { Flags { session: "test".to_string(), - tab: None, json: false, headed: false, debug: false, @@ -2852,10 +2871,17 @@ mod tests { } #[test] - fn test_tab_switch() { - let cmd = parse_command(&args("tab 2"), &default_flags()).unwrap(); + fn test_tab_switch_by_id() { + let cmd = parse_command(&args("tab t2"), &default_flags()).unwrap(); assert_eq!(cmd["action"], "tab_switch"); - assert_eq!(cmd["tabId"], 2); + assert_eq!(cmd["tabId"], "t2"); + } + + #[test] + fn test_tab_switch_by_label() { + let cmd = parse_command(&args("tab docs"), &default_flags()).unwrap(); + assert_eq!(cmd["action"], "tab_switch"); + assert_eq!(cmd["tabId"], "docs"); } #[test] @@ -2866,23 +2892,57 @@ mod tests { #[test] fn test_tab_close_with_id() { - let cmd = parse_command(&args("tab close 2"), &default_flags()).unwrap(); + let cmd = parse_command(&args("tab close t2"), &default_flags()).unwrap(); assert_eq!(cmd["action"], "tab_close"); - assert_eq!(cmd["tabId"], 2); + assert_eq!(cmd["tabId"], "t2"); } #[test] - fn test_tab_switch_sends_tab_id() { - let cmd = parse_command(&args("tab 2"), &default_flags()).unwrap(); - assert_eq!(cmd["tabId"], 2); + fn test_tab_close_with_label() { + let cmd = parse_command(&args("tab close docs"), &default_flags()).unwrap(); + assert_eq!(cmd["action"], "tab_close"); + assert_eq!(cmd["tabId"], "docs"); + } + + #[test] + fn test_tab_sends_string_tab_id() { + let cmd = parse_command(&args("tab t2"), &default_flags()).unwrap(); + assert!( + cmd["tabId"].is_string(), + "tabId must be a string, got: {:?}", + cmd["tabId"] + ); 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()); + fn test_tab_new_with_label() { + let cmd = parse_command(&args("tab new --label docs"), &default_flags()).unwrap(); + assert_eq!(cmd["action"], "tab_new"); + assert_eq!(cmd["label"], "docs"); + } + + #[test] + fn test_tab_new_with_label_and_url() { + let cmd = parse_command( + &args("tab new --label docs https://docs.example.com"), + &default_flags(), + ) + .unwrap(); + assert_eq!(cmd["action"], "tab_new"); + assert_eq!(cmd["label"], "docs"); + assert_eq!(cmd["url"], "https://docs.example.com"); + } + + #[test] + fn test_tab_new_with_url_then_label() { + let cmd = parse_command( + &args("tab new https://docs.example.com --label docs"), + &default_flags(), + ) + .unwrap(); + assert_eq!(cmd["url"], "https://docs.example.com"); + assert_eq!(cmd["label"], "docs"); } #[test] @@ -2892,14 +2952,30 @@ mod tests { } #[test] - fn test_tab_unknown_subcommand_errors() { - let result = parse_command(&args("tab select 3"), &default_flags()); + fn test_tab_unknown_flag_errors() { + // Unknown flags on `tab new` must error instead of being silently + // dropped. This protects against typos like `--labl` or `--new-tab`. + let result = parse_command( + &args("tab new --unknown-flag https://example.com"), + &default_flags(), + ); assert!( result.is_err(), - "tab select should error, not silently fall through to tab_list" + "tab new with an unknown flag must error, got: {:?}", + result ); } + #[test] + fn test_tab_non_keyword_treated_as_ref() { + // After the shift to `t`/label ids, non-keyword tokens (`select`, + // `docs`, etc.) are valid label refs; `tab ` routes to + // tab_switch and the runtime decides whether the label exists. + let cmd = parse_command(&args("tab select"), &default_flags()).unwrap(); + assert_eq!(cmd["action"], "tab_switch"); + assert_eq!(cmd["tabId"], "select"); + } + // === Network === #[test] diff --git a/cli/src/flags.rs b/cli/src/flags.rs index 924fc14..6e517c9 100644 --- a/cli/src/flags.rs +++ b/cli/src/flags.rs @@ -57,7 +57,6 @@ 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>, @@ -99,7 +98,6 @@ 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) { @@ -198,7 +196,6 @@ 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", @@ -276,7 +273,6 @@ 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, @@ -359,7 +355,6 @@ 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() @@ -497,12 +492,6 @@ 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) { @@ -786,7 +775,6 @@ 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", @@ -1453,25 +1441,4 @@ 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 c6eab4b..afc8cb4 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -738,12 +738,6 @@ 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 73e75e6..0d42768 100644 --- a/cli/src/native/actions.rs +++ b/cli/src/native/actions.rs @@ -665,6 +665,7 @@ impl DaemonState { let tab_id = mgr.assign_tab_id(); mgr.add_page(super::browser::PageInfo { tab_id, + label: None, target_id: te.target_info.target_id.clone(), session_id: attach.session_id, url: te.target_info.url.clone(), @@ -1275,44 +1276,6 @@ pub async fn execute_command(cmd: &Value, state: &mut DaemonState) -> Value { ); } - // Pre-dispatch: if `tabId` is set on a non-tab command, temporarily switch - // to that tab for the duration of the command and restore the original - // active tab after. This lets `--tab N ` target a specific tab - // without stealing the user's active-tab context. - // - // We save the current tab's stable `tab_id` (not its array index) so a - // tab close during the scoped command doesn't leave us restoring to a - // shifted position. If the saved tab was closed, we skip the restore. - let restore_tab_id: Option = if !matches!( - action, - "tab_list" | "tab_new" | "tab_switch" | "tab_close" | "launch" | "close" - ) { - if let Some(target_tab_id) = cmd.get("tabId").and_then(|v| v.as_u64()) { - let target_tab_id = target_tab_id as u32; - let current_tab_id = state.browser.as_ref().and_then(|mgr| mgr.active_tab_id()); - if current_tab_id == Some(target_tab_id) { - // Already on the target tab; nothing to do. - None - } else { - // Clear per-tab daemon state before switching so refs from the - // outer tab can't resolve against the target tab's DOM. - state.ref_map.clear(); - state.iframe_sessions.clear(); - state.active_frame_id = None; - if let Some(ref mut mgr) = state.browser { - if let Err(e) = mgr.tab_switch_by_id(target_tab_id).await { - return error_response(&id, &e); - } - } - current_tab_id - } - } else { - None - } - } else { - None - }; - let result = match action { "launch" => handle_launch(cmd, state).await, "navigate" => handle_navigate(cmd, state).await, @@ -1471,24 +1434,6 @@ pub async fn execute_command(cmd: &Value, state: &mut DaemonState) -> Value { _ => Err(format!("Not yet implemented: {}", action)), }; - // Post-dispatch: if we temporarily switched tabs for this command, restore - // the original active tab. Skip silently if the tab no longer exists (e.g. - // the scoped command closed it). - if let Some(restore_tab_id) = restore_tab_id { - let still_exists = state - .browser - .as_ref() - .is_some_and(|mgr| mgr.has_tab_id(restore_tab_id)); - if still_exists { - state.ref_map.clear(); - state.iframe_sessions.clear(); - state.active_frame_id = None; - if let Some(ref mut mgr) = state.browser { - let _ = mgr.tab_switch_by_id(restore_tab_id).await; - } - } - } - let mut resp = match result { Ok(data) => success_response(&id, data), Err(e) => error_response(&id, &super::browser::to_ai_friendly_error(&e)), @@ -1545,7 +1490,7 @@ pub async fn execute_command(cmd: &Value, state: &mut DaemonState) -> Value { /// subsequent navigations don't hijack the user's existing tabs. async fn connect_auto_with_fresh_tab() -> Result { let mut mgr = BrowserManager::connect_auto().await?; - mgr.tab_new(None).await?; + mgr.tab_new(None, None).await?; let session_id = mgr.active_session_id()?.to_string(); let _ = mgr .client @@ -2648,7 +2593,7 @@ async fn handle_click(cmd: &Value, state: &mut DaemonState) -> Result Result { async fn handle_tab_new(cmd: &Value, state: &mut DaemonState) -> Result { let mgr = state.browser.as_mut().ok_or("Browser not launched")?; let url = cmd.get("url").and_then(|v| v.as_str()); + let label = cmd.get("label").and_then(|v| v.as_str()); state.ref_map.clear(); state.iframe_sessions.clear(); state.active_frame_id = None; - mgr.tab_new(url).await + mgr.tab_new(url, label).await } async fn handle_tab_switch(cmd: &Value, state: &mut DaemonState) -> Result { let mgr = state.browser.as_mut().ok_or("Browser not launched")?; - let tab_id = cmd + let tab_ref_str = cmd .get("tabId") - .and_then(|v| v.as_u64()) - .ok_or("Missing 'tabId' parameter")? as u32; + .and_then(|v| v.as_str()) + .ok_or("Missing 'tabId' parameter (expected `t` or a label)")?; + let tab_ref = super::browser::TabRef::parse(tab_ref_str)?; + let tab_id = mgr.resolve_tab_ref(&tab_ref)?; state.ref_map.clear(); state.iframe_sessions.clear(); state.active_frame_id = None; @@ -3737,7 +3685,13 @@ async fn handle_tab_switch(cmd: &Value, state: &mut DaemonState) -> Result Result { let mgr = state.browser.as_mut().ok_or("Browser not launched")?; - let tab_id = cmd.get("tabId").and_then(|v| v.as_u64()).map(|i| i as u32); + let tab_id = match cmd.get("tabId").and_then(|v| v.as_str()) { + Some(s) => { + let tab_ref = super::browser::TabRef::parse(s)?; + Some(mgr.resolve_tab_ref(&tab_ref)?) + } + None => None, + }; state.ref_map.clear(); state.iframe_sessions.clear(); state.active_frame_id = None; @@ -4123,6 +4077,7 @@ async fn handle_recording_start(cmd: &Value, state: &mut DaemonState) -> Result< let tab_id = mgr.assign_tab_id(); mgr.add_page(super::browser::PageInfo { tab_id, + label: None, target_id: create_result.target_id, session_id: new_session_id.clone(), url: nav_url.clone(), @@ -6036,6 +5991,7 @@ 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 87eb3de..abaa5db 100644 --- a/cli/src/native/browser.rs +++ b/cli/src/native/browser.rs @@ -159,6 +159,12 @@ pub fn to_ai_friendly_error(error: &str) -> String { #[derive(Debug, Clone)] pub struct PageInfo { pub tab_id: u32, + /// Optional user-assigned label (e.g. "docs", "app"). Set via + /// `tab new --label `. Labels are agent-assigned and never + /// auto-generated, never rewritten on navigation, and unique within a + /// session. Agents use labels instead of `t` for readable multi-tab + /// workflows. + pub label: Option, pub target_id: String, pub session_id: String, pub url: String, @@ -166,6 +172,77 @@ pub struct PageInfo { pub target_type: String, // "page" or "webview" } +/// Canonical string form of a stable tab id: `t1`, `t2`, ... The `t` prefix +/// disambiguates stable ids from positional indices (which the CLI no longer +/// accepts) and matches the `@e` convention used for element refs. +pub fn format_tab_id(tab_id: u32) -> String { + format!("t{}", tab_id) +} + +/// A tab reference as parsed from CLI/JSON input. Either a stable id like +/// `t2` or a user-assigned label like `docs`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum TabRef { + Id(u32), + Label(String), +} + +impl TabRef { + /// Parse a user-supplied string tab reference. Rejects bare integers + /// with a teaching error so agents and scripts don't silently confuse + /// stable ids with positional indices. + pub fn parse(input: &str) -> Result { + let input = input.trim(); + if input.is_empty() { + return Err("Empty tab reference; expected `t` (e.g. `t2`) or a label".to_string()); + } + if let Some(digits) = input.strip_prefix('t').or_else(|| input.strip_prefix('T')) { + if !digits.is_empty() && digits.chars().all(|c| c.is_ascii_digit()) { + let id: u32 = digits.parse().map_err(|_| { + format!( + "Tab id `{}` out of range; ids are incrementing positive integers", + input + ) + })?; + if id == 0 { + return Err(format!( + "Tab id `{}` is invalid; tab ids start at t1", + input + )); + } + return Ok(TabRef::Id(id)); + } + } + if input.chars().all(|c| c.is_ascii_digit()) { + return Err(format!( + "Expected a tab id like `t{}` or a label; positional integers are not accepted \ + (run `agent-browser tab` to list stable tab ids)", + input + )); + } + if !is_valid_label(input) { + return Err(format!( + "Invalid tab label `{}`; labels must start with a letter and contain only \ + letters, digits, `-`, and `_`", + input + )); + } + Ok(TabRef::Label(input.to_string())) + } +} + +/// Labels must look like identifiers: start with a letter, contain only +/// letters/digits/dashes/underscores. This keeps them distinguishable from +/// `t` ids at a glance and safe to pass through shells without quoting. +pub fn is_valid_label(s: &str) -> bool { + let mut chars = s.chars(); + match chars.next() { + Some(c) if c.is_ascii_alphabetic() => {} + _ => return false, + } + chars.all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_') +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum WaitUntil { Load, @@ -395,6 +472,7 @@ impl BrowserManager { let tab_id = manager.assign_tab_id(); manager.pages.push(PageInfo { tab_id, + label: None, target_id: "provider-page".to_string(), session_id: String::new(), url: String::new(), @@ -463,6 +541,7 @@ impl BrowserManager { self.next_tab_id += 1; self.pages.push(PageInfo { tab_id, + label: None, target_id: result.target_id, session_id: attach_result.session_id.clone(), url: "about:blank".to_string(), @@ -489,6 +568,7 @@ impl BrowserManager { self.next_tab_id += 1; self.pages.push(PageInfo { tab_id, + label: None, target_id: target.target_id.clone(), session_id: attach_result.session_id.clone(), url: target.url.clone(), @@ -837,6 +917,7 @@ impl BrowserManager { self.next_tab_id += 1; self.pages.push(PageInfo { tab_id, + label: None, target_id: result.target_id, session_id: attach_result.session_id.clone(), url: "about:blank".to_string(), @@ -879,7 +960,8 @@ impl BrowserManager { .enumerate() .map(|(i, p)| { json!({ - "tabId": p.tab_id, + "tabId": format_tab_id(p.tab_id), + "label": p.label, "title": p.title, "url": p.url, "type": p.target_type, @@ -889,7 +971,61 @@ impl BrowserManager { .collect() } - pub async fn tab_new(&mut self, url: Option<&str>) -> Result { + /// 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 { + match tab_ref { + TabRef::Id(id) => { + if self.has_tab_id(*id) { + Ok(*id) + } else { + Err(format!( + "Tab {} not found; run `agent-browser tab` to list open tabs", + format_tab_id(*id) + )) + } + } + TabRef::Label(name) => self + .pages + .iter() + .find(|p| p.label.as_deref() == Some(name.as_str())) + .map(|p| p.tab_id) + .ok_or_else(|| { + format!( + "No tab with label `{}`; run `agent-browser tab` to list open tabs", + name + ) + }), + } + } + + /// Returns true iff a tab already carries the given label. + pub fn has_label(&self, label: &str) -> bool { + self.pages.iter().any(|p| p.label.as_deref() == Some(label)) + } + + pub async fn tab_new( + &mut self, + url: Option<&str>, + label: Option<&str>, + ) -> Result { + if let Some(label) = label { + if !is_valid_label(label) { + return Err(format!( + "Invalid tab label `{}`; labels must start with a letter and contain only \ + letters, digits, `-`, and `_`", + label + )); + } + if self.has_label(label) { + return Err(format!( + "Label `{}` is already used by another tab; labels must be unique within a \ + session", + label + )); + } + } + let target_url = url.unwrap_or("about:blank"); let result: CreateTargetResult = self @@ -920,8 +1056,10 @@ impl BrowserManager { let tab_id = self.next_tab_id; self.next_tab_id += 1; let index = self.pages.len(); + let label = label.map(|s| s.to_string()); self.pages.push(PageInfo { tab_id, + label: label.clone(), target_id: result.target_id, session_id: attach.session_id, url: target_url.to_string(), @@ -930,7 +1068,12 @@ impl BrowserManager { }); self.active_page_index = index; - Ok(json!({ "tabId": tab_id, "url": target_url, "total": self.pages.len() })) + Ok(json!({ + "tabId": format_tab_id(tab_id), + "label": label, + "url": target_url, + "total": self.pages.len(), + })) } pub async fn tab_switch(&mut self, index: usize) -> Result { @@ -960,8 +1103,13 @@ impl BrowserManager { page.title = title.clone(); } - let tab_id = self.pages[index].tab_id; - Ok(json!({ "tabId": tab_id, "url": url, "title": title })) + let page = &self.pages[index]; + Ok(json!({ + "tabId": format_tab_id(page.tab_id), + "label": page.label, + "url": url, + "title": title, + })) } pub async fn tab_close(&mut self, index: Option) -> Result { @@ -978,6 +1126,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 closed_label = page.label.clone(); let _ = self .client .send_command_typed::<_, Value>( @@ -992,7 +1141,11 @@ impl BrowserManager { let session_id = self.pages[self.active_page_index].session_id.clone(); self.enable_domains(&session_id).await?; - Ok(json!({ "tabId": closed_tab_id, "closed": true })) + Ok(json!({ + "tabId": format_tab_id(closed_tab_id), + "label": closed_label, + "closed": true, + })) } // ----------------------------------------------------------------------- @@ -1557,6 +1710,75 @@ mod tests { use super::*; use tokio::time::sleep; + #[test] + fn test_format_tab_id() { + assert_eq!(format_tab_id(1), "t1"); + assert_eq!(format_tab_id(42), "t42"); + } + + #[test] + fn test_parse_tab_ref_id() { + assert_eq!(TabRef::parse("t1"), Ok(TabRef::Id(1))); + assert_eq!(TabRef::parse("t42"), Ok(TabRef::Id(42))); + assert_eq!(TabRef::parse("T7"), Ok(TabRef::Id(7))); + } + + #[test] + fn test_parse_tab_ref_label() { + assert_eq!(TabRef::parse("docs"), Ok(TabRef::Label("docs".to_string()))); + assert_eq!( + TabRef::parse("app-2"), + Ok(TabRef::Label("app-2".to_string())) + ); + assert_eq!( + TabRef::parse("my_tab"), + Ok(TabRef::Label("my_tab".to_string())) + ); + } + + #[test] + fn test_parse_tab_ref_rejects_bare_integer() { + let err = TabRef::parse("2").unwrap_err(); + assert!( + err.contains("positional integers are not accepted"), + "error should teach the user to use `t`: {}", + err + ); + assert!(err.contains("t2")); + } + + #[test] + fn test_parse_tab_ref_rejects_empty() { + assert!(TabRef::parse("").is_err()); + assert!(TabRef::parse(" ").is_err()); + } + + #[test] + fn test_parse_tab_ref_rejects_zero() { + let err = TabRef::parse("t0").unwrap_err(); + assert!(err.contains("start at t1")); + } + + #[test] + fn test_parse_tab_ref_rejects_invalid_label() { + assert!(TabRef::parse("2docs").is_err()); + assert!(TabRef::parse("-docs").is_err()); + assert!(TabRef::parse("docs!").is_err()); + assert!(TabRef::parse("docs space").is_err()); + } + + #[test] + fn test_is_valid_label() { + assert!(is_valid_label("docs")); + assert!(is_valid_label("Docs")); + assert!(is_valid_label("app-2")); + assert!(is_valid_label("my_tab")); + assert!(!is_valid_label("")); + assert!(!is_valid_label("2docs")); + assert!(!is_valid_label("-docs")); + assert!(!is_valid_label("docs!")); + } + #[test] fn test_should_track_popup_target_with_empty_url() { let target = TargetInfo { @@ -1589,6 +1811,7 @@ mod tests { fn test_update_page_target_info_in_pages_updates_existing_page() { let mut pages = vec![PageInfo { tab_id: 1, + label: None, target_id: "popup-1".to_string(), session_id: "session-1".to_string(), url: String::new(), diff --git a/cli/src/native/e2e_tests.rs b/cli/src/native/e2e_tests.rs index 7bfb622..21b0d7a 100644 --- a/cli/src/native/e2e_tests.rs +++ b/cli/src/native/e2e_tests.rs @@ -911,7 +911,7 @@ async fn e2e_tabs() { 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"); + assert_eq!(tabs[0]["tabId"], "t1", "First tab should have tabId t1"); // Open new tab let resp = execute_command( @@ -920,7 +920,11 @@ async fn e2e_tabs() { ) .await; assert_success(&resp); - assert_eq!(get_data(&resp)["tabId"], 2, "New tab should have tabId 2"); + assert_eq!( + get_data(&resp)["tabId"], + "t2", + "New tab should have tabId t2" + ); assert_eq!(get_data(&resp)["total"], 2); // Tab list should show 2 tabs with distinct, incrementing tabIds @@ -929,12 +933,12 @@ async fn e2e_tabs() { 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"); + assert_eq!(tabs[0]["tabId"], "t1", "First tab should keep tabId t1"); + assert_eq!(tabs[1]["tabId"], "t2", "Second tab should have tabId t2"); // Switch to first tab let resp = execute_command( - &json!({ "id": "6", "action": "tab_switch", "tabId": 1 }), + &json!({ "id": "6", "action": "tab_switch", "tabId": "t1" }), &mut state, ) .await; @@ -950,7 +954,7 @@ async fn e2e_tabs() { // Close second tab let resp = execute_command( - &json!({ "id": "8", "action": "tab_close", "tabId": 2 }), + &json!({ "id": "8", "action": "tab_close", "tabId": "t2" }), &mut state, ) .await; @@ -982,7 +986,7 @@ async fn e2e_tab_ids_not_reused() { 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); + assert_eq!(tabs[0]["tabId"], "t1"); // Open tab 2 and tab 3 let resp = execute_command( @@ -991,7 +995,7 @@ async fn e2e_tab_ids_not_reused() { ) .await; assert_success(&resp); - assert_eq!(get_data(&resp)["tabId"], 2); + assert_eq!(get_data(&resp)["tabId"], "t2"); let resp = execute_command( &json!({ "id": "4", "action": "tab_new", "url": "data:text/html,

Tab 3

" }), @@ -999,11 +1003,11 @@ async fn e2e_tab_ids_not_reused() { ) .await; assert_success(&resp); - assert_eq!(get_data(&resp)["tabId"], 3); + assert_eq!(get_data(&resp)["tabId"], "t3"); // Close tab 2 let resp = execute_command( - &json!({ "id": "5", "action": "tab_close", "tabId": 2 }), + &json!({ "id": "5", "action": "tab_close", "tabId": "t2" }), &mut state, ) .await; @@ -1018,406 +1022,31 @@ async fn e2e_tab_ids_not_reused() { assert_success(&resp); assert_eq!( get_data(&resp)["tabId"], - 4, + "t4", "Tab IDs must not be reused after closing" ); - // Verify final state: tabs 1, 3, 4 + // Verify final state: tabs t1, t3, t4 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 ids: Vec = tabs + .iter() + .map(|t| t["tabId"].as_str().unwrap().to_string()) + .collect(); + assert_eq!(ids, vec!["t1", "t3", "t4"]); let resp = execute_command(&json!({ "id": "99", "action": "close" }), &mut state).await; assert_success(&resp); } +/// `tab_close` with an explicit `tabId` must close that tab regardless of +/// whether it's active, and leave the remaining tab active without leaking +/// per-tab state (refs, iframe sessions, frame id) from the closed tab. #[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); - // Active tab was never changed by the scoped `tabId` commands - // (restoration semantics), so it's still tab 2 from the earlier `tab_new`. - 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 uses the still-active tab 2 (restoration - // semantics: scoped commands don't change the active tab). - 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); -} - -// --------------------------------------------------------------------------- -// `--tab` / `tabId` scoped-command regression tests -// --------------------------------------------------------------------------- - -/// `tabId`-scoped commands must clear `state.ref_map`, `state.iframe_sessions`, -/// and `state.active_frame_id` when they temporarily switch tabs, otherwise -/// refs from the outer tab would resolve against the scoped tab's DOM. -#[tokio::test] -#[ignore] -async fn e2e_tab_scoped_command_clears_state_on_switch() { - let mut state = DaemonState::new(); - - let resp = execute_command( - &json!({ "id": "1", "action": "launch", "headless": true }), - &mut state, - ) - .await; - assert_success(&resp); - - let resp = execute_command( - &json!({ "id": "2", "action": "navigate", "url": "data:text/html," }), - &mut state, - ) - .await; - assert_success(&resp); - - let resp = execute_command( - &json!({ "id": "3", "action": "tab_new", "url": "data:text/html,

Beta

" }), - &mut state, - ) - .await; - assert_success(&resp); - assert_eq!(get_data(&resp)["tabId"], 2); - - let resp = execute_command( - &json!({ "id": "4", "action": "tab_switch", "tabId": 1 }), - &mut state, - ) - .await; - assert_success(&resp); - - let resp = execute_command(&json!({ "id": "5", "action": "snapshot" }), &mut state).await; - assert_success(&resp); - assert!( - state.ref_map.get("e1").is_some(), - "snapshot should populate @e1 on tab 1" - ); - - // Run a tabId-scoped command. The pre-dispatch must clear per-tab state - // so nothing can leak into the scoped tab's context. - let resp = execute_command( - &json!({ "id": "6", "action": "title", "tabId": 2 }), - &mut state, - ) - .await; - assert_success(&resp); - - assert!( - state.ref_map.get("e1").is_none(), - "ref_map must be cleared when a tabId-scoped command switches tabs, \ - but @e1 is still present: {:?}", - state.ref_map.get("e1") - ); - assert!(state.iframe_sessions.is_empty()); - assert!(state.active_frame_id.is_none()); - - let resp = execute_command(&json!({ "id": "99", "action": "close" }), &mut state).await; - assert_success(&resp); -} - -/// `tabId`-scoped commands must restore the original active tab afterward so -/// `--tab N` is a non-intrusive peek that doesn't change the user's context. -#[tokio::test] -#[ignore] -async fn e2e_tab_scoped_command_restores_active_tab() { - let mut state = DaemonState::new(); - - let resp = execute_command( - &json!({ "id": "1", "action": "launch", "headless": true }), - &mut state, - ) - .await; - assert_success(&resp); - - let resp = execute_command( - &json!({ "id": "2", "action": "navigate", "url": "data:text/html,A" }), - &mut state, - ) - .await; - assert_success(&resp); - - let resp = execute_command( - &json!({ "id": "3", "action": "tab_new", "url": "data:text/html,B" }), - &mut state, - ) - .await; - assert_success(&resp); - assert_eq!(get_data(&resp)["tabId"], 2); - - // Active tab is 2. Peek at tab 1 with a scoped command. - let resp = execute_command( - &json!({ "id": "4", "action": "title", "tabId": 1 }), - &mut state, - ) - .await; - assert_success(&resp); - assert_eq!(get_data(&resp)["title"], "A", "tabId should route to tab 1"); - - // No tabId: must reflect the originally active tab (tab 2). - let resp = execute_command(&json!({ "id": "5", "action": "title" }), &mut state).await; - assert_success(&resp); - assert_eq!( - get_data(&resp)["title"], - "B", - "active tab should be restored to tab 2 after the scoped command; \ - got the scoped tab's title instead" - ); - - let resp = execute_command(&json!({ "id": "99", "action": "close" }), &mut state).await; - assert_success(&resp); -} - -/// Restoration must be skipped (without error) if the scoped command closes -/// the tab that was active before the switch. -#[tokio::test] -#[ignore] -async fn e2e_tab_scoped_command_handles_outer_tab_closed() { +async fn e2e_tab_close_with_tab_id_closes_active_tab() { let mut state = DaemonState::new(); let resp = execute_command( @@ -1441,11 +1070,8 @@ async fn e2e_tab_scoped_command_handles_outer_tab_closed() { .await; assert_success(&resp); - // Active tab is 2. Peek at tab 1 with a command that also closes tab 2. - // The restoration path must not error when it discovers tab 2 is gone; - // we treat "outer tab vanished" as an implicit accept of the scoped tab. let resp = execute_command( - &json!({ "id": "4", "action": "tab_close", "tabId": 2 }), + &json!({ "id": "4", "action": "tab_close", "tabId": "t2" }), &mut state, ) .await; @@ -1454,6 +1080,174 @@ async fn e2e_tab_scoped_command_handles_outer_tab_closed() { let resp = execute_command(&json!({ "id": "5", "action": "title" }), &mut state).await; assert_success(&resp); assert_eq!(get_data(&resp)["title"], "A"); + assert!(state.ref_map.get("e1").is_none()); + assert!(state.iframe_sessions.is_empty()); + assert!(state.active_frame_id.is_none()); + + let resp = execute_command(&json!({ "id": "99", "action": "close" }), &mut state).await; + assert_success(&resp); +} + +/// Tabs can be opened with a user-assigned label and then addressed by that +/// label anywhere a `t` id is accepted (switch, close, and JSON `tabId` +/// on `tab_switch` / `tab_close`). Labels are the agent-friendly way to +/// write multi-tab workflows without memorizing ids. +#[tokio::test] +#[ignore] +async fn e2e_tab_new_with_label_can_be_switched_and_closed() { + let mut state = DaemonState::new(); + + let resp = execute_command( + &json!({ "id": "1", "action": "launch", "headless": true }), + &mut state, + ) + .await; + assert_success(&resp); + + let resp = execute_command( + &json!({ "id": "2", "action": "navigate", "url": "data:text/html,Home" }), + &mut state, + ) + .await; + assert_success(&resp); + + // Open a labeled tab and verify the response echoes the label and a + // `t` style tabId. + let resp = execute_command( + &json!({ + "id": "3", + "action": "tab_new", + "url": "data:text/html,Docs", + "label": "docs", + }), + &mut state, + ) + .await; + assert_success(&resp); + assert_eq!(get_data(&resp)["tabId"], "t2"); + assert_eq!(get_data(&resp)["label"], "docs"); + + // tab_list exposes the label alongside the id. + let resp = execute_command(&json!({ "id": "4", "action": "tab_list" }), &mut state).await; + assert_success(&resp); + let tabs = get_data(&resp)["tabs"].as_array().unwrap(); + let docs = tabs + .iter() + .find(|t| t["tabId"] == "t2") + .expect("docs tab should be present"); + assert_eq!(docs["label"], "docs"); + + // tab_switch accepts the label. + let resp = execute_command( + &json!({ "id": "5", "action": "tab_switch", "tabId": "t1" }), + &mut state, + ) + .await; + assert_success(&resp); + assert_eq!(state.browser.as_ref().unwrap().active_tab_id(), Some(1)); + + let resp = execute_command( + &json!({ "id": "6", "action": "tab_switch", "tabId": "docs" }), + &mut state, + ) + .await; + assert_success(&resp); + assert_eq!(state.browser.as_ref().unwrap().active_tab_id(), Some(2)); + + // Once switched, the active tab is the labeled one and normal commands + // work against it. + let resp = execute_command(&json!({ "id": "7", "action": "title" }), &mut state).await; + assert_success(&resp); + assert_eq!(get_data(&resp)["title"], "Docs"); + + // tab_close accepts the label. + let resp = execute_command( + &json!({ "id": "8", "action": "tab_close", "tabId": "docs" }), + &mut state, + ) + .await; + assert_success(&resp); + assert_eq!(get_data(&resp)["label"], "docs"); + + let resp = execute_command(&json!({ "id": "99", "action": "close" }), &mut state).await; + assert_success(&resp); +} + +/// Duplicate labels must be rejected so agents can treat a label as a unique +/// handle. The first tab keeps the label; the second tab's creation errors. +#[tokio::test] +#[ignore] +async fn e2e_tab_new_with_duplicate_label_errors() { + let mut state = DaemonState::new(); + + let resp = execute_command( + &json!({ "id": "1", "action": "launch", "headless": true }), + &mut state, + ) + .await; + assert_success(&resp); + + let resp = execute_command( + &json!({ "id": "2", "action": "tab_new", "url": "about:blank", "label": "docs" }), + &mut state, + ) + .await; + assert_success(&resp); + + let resp = execute_command( + &json!({ "id": "3", "action": "tab_new", "url": "about:blank", "label": "docs" }), + &mut state, + ) + .await; + assert_eq!( + resp.get("success").and_then(|v| v.as_bool()), + Some(false), + "duplicate label should error: {}", + serde_json::to_string_pretty(&resp).unwrap_or_default() + ); + let err = resp.get("error").and_then(|v| v.as_str()).unwrap_or(""); + assert!( + err.contains("already used"), + "error should explain the collision: {}", + err + ); + + let resp = execute_command(&json!({ "id": "99", "action": "close" }), &mut state).await; + assert_success(&resp); +} + +/// Positional integers passed as `tabId` on tab-switch / tab-close must be +/// rejected by the daemon-layer parser, not silently coerced. The error +/// should teach the user the correct form (`t`). +#[tokio::test] +#[ignore] +async fn e2e_tab_switch_rejects_bare_integer() { + let mut state = DaemonState::new(); + + let resp = execute_command( + &json!({ "id": "1", "action": "launch", "headless": true }), + &mut state, + ) + .await; + assert_success(&resp); + + let resp = execute_command( + &json!({ "id": "2", "action": "tab_switch", "tabId": "2" }), + &mut state, + ) + .await; + assert_eq!( + resp.get("success").and_then(|v| v.as_bool()), + Some(false), + "bare integer tabId on tab_switch should error: {}", + serde_json::to_string_pretty(&resp).unwrap_or_default() + ); + let err = resp.get("error").and_then(|v| v.as_str()).unwrap_or(""); + assert!( + err.contains("t2") && err.contains("positional integers"), + "error should teach `t` convention: {}", + err + ); let resp = execute_command(&json!({ "id": "99", "action": "close" }), &mut state).await; assert_success(&resp); diff --git a/cli/src/output.rs b/cli/src/output.rs index bac0c9f..59e5f27 100644 --- a/cli/src/output.rs +++ b/cli/src/output.rs @@ -407,10 +407,8 @@ 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 tab in tabs { - let tab_id = tab - .get("tabId") - .and_then(|v| v.as_i64()) - .unwrap_or_default(); + let tab_id = tab.get("tabId").and_then(|v| v.as_str()).unwrap_or("?"); + let tab_label = tab.get("label").and_then(|v| v.as_str()); let title = tab .get("title") .and_then(|v| v.as_str()) @@ -422,13 +420,17 @@ pub fn print_response_with_opts(resp: &Response, action: Option<&str>, opts: &Ou } else { " ".to_string() }; - println!("{} [{}] {} - {}", marker, tab_id, title, url); + if let Some(label) = tab_label { + println!("{} [{}] {} {} - {}", marker, tab_id, label, title, url); + } else { + 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(tab_id) = data.get("tabId").and_then(|v| v.as_str()) { if let Some(url) = data.get("url").and_then(|v| v.as_str()) { println!( "{} Switched to tab [{}] ({})", @@ -447,19 +449,31 @@ pub fn print_response_with_opts(resp: &Response, action: Option<&str>, opts: &Ou } } // New tab/window - if let Some(tab_id) = data.get("tabId").and_then(|v| v.as_i64()) { + if let Some(tab_id) = data.get("tabId").and_then(|v| v.as_str()) { if let Some(total) = data.get("total").and_then(|v| v.as_i64()) { - let label = match action { + let label_noun = match action { Some("window_new") => "Window opened", _ => "Tab opened", }; - println!( - "{} {} [{}] ({} total)", - color::success_indicator(), - label, - tab_id, - total - ); + let tab_label = data.get("label").and_then(|v| v.as_str()); + if let Some(lbl) = tab_label { + println!( + "{} {} [{}] {} ({} total)", + color::success_indicator(), + label_noun, + tab_id, + lbl, + total + ); + } else { + println!( + "{} {} [{}] ({} total)", + color::success_indicator(), + label_noun, + tab_id, + total + ); + } return; } } @@ -604,7 +618,7 @@ pub fn print_response_with_opts(resp: &Response, action: Option<&str>, opts: &Ou if data.get("closed").is_some() { let label = match action { Some("tab_close") => { - if let Some(closed_id) = data.get("tabId").and_then(|v| v.as_i64()) { + if let Some(closed_id) = data.get("tabId").and_then(|v| v.as_str()) { println!("{} Tab [{}] closed", color::success_indicator(), closed_id); return; } @@ -1057,7 +1071,6 @@ 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 @@ -1081,7 +1094,6 @@ the browser's back button. Global Options: --json Output as JSON --session Use specific session - --tab Target specific tab ID Examples: agent-browser back @@ -1099,7 +1111,6 @@ the browser's forward button. Global Options: --json Output as JSON --session Use specific session - --tab Target specific tab ID Examples: agent-browser forward @@ -1117,7 +1128,6 @@ the browser's reload button. Global Options: --json Output as JSON --session Use specific session - --tab Target specific tab ID Examples: agent-browser reload @@ -1141,7 +1151,6 @@ Options: Global Options: --json Output as JSON --session Use specific session - --tab Target specific tab ID Examples: agent-browser click "#submit-button" @@ -1163,7 +1172,6 @@ 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" @@ -1182,7 +1190,6 @@ 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" @@ -1202,7 +1209,6 @@ 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" @@ -1226,7 +1232,6 @@ 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" @@ -1244,7 +1249,6 @@ 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" @@ -1262,7 +1266,6 @@ 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" @@ -1280,7 +1283,6 @@ 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" @@ -1298,7 +1300,6 @@ Selects one or more options in a