Consistent Tab IDs & Global Tag Targeting (#892)

Introduces stable per-tab IDs and a global `--tab <id>` 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 <unknown>` 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.
This commit is contained in:
Daniel Hails
2026-04-16 12:02:55 -05:00
committed by GitHub
parent c691b269cb
commit 67dc631977
7 changed files with 618 additions and 45 deletions
+27 -10
View File
@@ -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<Value, S
async fn handle_tab_switch(cmd: &Value, state: &mut DaemonState) -> Result<Value, String> {
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<Value
async fn handle_tab_close(cmd: &Value, state: &mut DaemonState) -> Result<Value, String> {
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<Value, String> {
@@ -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<Value
)
.await?;
let tab_id = mgr.assign_tab_id();
mgr.add_page(super::browser::PageInfo {
tab_id,
target_id: create_result.target_id,
session_id: attach.session_id,
url: "about:blank".to_string(),
@@ -6004,7 +6021,7 @@ async fn handle_window_new(cmd: &Value, state: &mut DaemonState) -> Result<Value
let total = mgr.page_count();
state.ref_map.clear();
Ok(json!({ "index": total - 1, "total": total }))
Ok(json!({ "tabId": tab_id, "total": total }))
}
async fn handle_diff_screenshot(cmd: &Value, state: &DaemonState) -> Result<Value, String> {
+51 -4
View File
@@ -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<String>,
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<Value, String> {
@@ -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<usize>) -> Result<Value, String> {
@@ -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<Value, String> {
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<u32>) -> Result<Value, String> {
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 {
+332 -5
View File
@@ -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,<h1>Tab 2</h1>" }),
&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,<h1>Tab 3</h1>" }),
&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,<h1>Tab 4</h1>" }),
&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<i64> = 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,<h1>Page A</h1>" }),
&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,<h1>Page B</h1>" }),
&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,<h1>Page A</h1>" }),
&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,<h1>Page B</h1>" }),
&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,<h1>Page A</h1>" }),
&mut state,
)
.await;
assert_success(&resp);
// Open tab 2
let resp = execute_command(
&json!({ "id": "3", "action": "tab_new", "url": "data:text/html,<h1>Page B</h1>" }),
&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,<h1>Page C</h1>" }),
&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
// ---------------------------------------------------------------------------