fix(tabs): correct --tab scoped commands and un-break provider direct-page path (#1249)
* fix(tabs): initialize tab_id on missing PageInfo sites PR #892 added a required `tab_id: u32` field to `PageInfo` but missed two initializer sites, which broke the build on the PR branch. CI never caught this because the external-contributor workflow status was `action_required` and never ran. - `cli/src/native/browser.rs:395` — the `direct_page` branch of `connect_cdp_inner` used by the cloud providers (Browserbase, Browserless, Browser Use, Kernel, AgentCore). Use `assign_tab_id()` to get a fresh id. - `cli/src/native/browser.rs:1580` — a unit test initializer. Use `tab_id: 1` since the test doesn't exercise id assignment. * feat(tabs): restore active tab and clear per-tab state for scoped --tab Follow-up on PR #892's `--tab <id>` flag. The original implementation called `tab_switch_by_id` directly from the pre-dispatch block in `execute_command` but didn't touch the daemon's per-tab state, and never restored the previously-active tab. Two concrete issues this fixes: 1. `state.ref_map`, `state.iframe_sessions`, and `state.active_frame_id` were left intact across the pre-dispatch switch, so `--tab N click @e1` would try to resolve `@e1` against the scoped tab's DOM using a backend-node id from the outer tab. In practice the click handler's role+name fallback hid this as "element not found" errors, but on pages where both tabs have similarly-labelled elements it could click the wrong one. 2. The PR description promised scoped routing would "restore the previous active tab", but the implementation permanently switched. `--tab 3 snapshot` would leave tab 3 as the active tab even after the command returned, surprising subsequent non-scoped commands. This change: - Saves the current tab's stable `tab_id` (not its array index, which would shift if the scoped command closed other tabs) before switching. - Clears per-tab daemon state before the switch so refs/iframes/frame context can't leak between tabs. - After the action runs, restores the original active tab (also via stable id) unless that tab was closed during the scoped command, in which case we leave the scoped tab active. - Adds `BrowserManager::active_tab_id()` and `has_tab_id()` accessors to support the above without exposing the internal `pages` vector. * test(tabs): regression tests for scoped --tab state clearing and restoration Three new `#[ignore]` e2e tests pinning the fixed behavior: - `e2e_tab_scoped_command_clears_state_on_switch` — populates `ref_map` on tab 1, runs a `tabId: 2`-scoped command, asserts `ref_map`, `iframe_sessions`, and `active_frame_id` are all cleared. - `e2e_tab_scoped_command_restores_active_tab` — sets up two tabs, runs a scoped command against the non-active one, asserts a subsequent unscoped command reflects the originally-active tab. - `e2e_tab_scoped_command_handles_outer_tab_closed` — runs a scoped `tab_close` that kills the outer tab itself, asserts no error and the scoped tab becomes active. Also updates two misleading comments in the PR's existing `e2e_tab_global_targeting*` tests to reflect restoration semantics; the assertions themselves were already consistent with restoration. * docs(tabs): document stable tab IDs and --tab scoped-command flag Per AGENTS.md, changes that users or agents would need to know about must land in every doc surface. Fills the gaps PR #892 left: - `README.md` — new `--tab <id>` row in the Options table, rewrite the tab command examples to use `<id>` instead of `<n>`, add a paragraph explaining stable tab IDs and `--tab` peek semantics. - `docs/src/app/commands/page.mdx` — same command-example rewrite plus a new "Stable tab IDs and `--tab`" subsection. - `docs/src/app/configuration/page.mdx` — add `tab` row to the config options table so JSON config users can discover it. - `agent-browser.schema.json` — add `tab` property with description, matching the config schema. - `skills/agent-browser/references/commands.md` — same command-example rewrite plus a short paragraph for agents on when to use `--tab`.
This commit is contained in:
Chris Tate
@@ -290,13 +290,24 @@ agent-browser network har stop [output.har] # Stop and save HAR (temp path if
|
||||
### Tabs & Windows
|
||||
|
||||
```bash
|
||||
agent-browser tab # List tabs
|
||||
agent-browser tab # List tabs (shows stable `tabId` for each)
|
||||
agent-browser tab new [url] # New tab (optionally with URL)
|
||||
agent-browser tab <n> # Switch to tab n
|
||||
agent-browser tab close [n] # Close tab
|
||||
agent-browser tab <id> # Switch to tab by id
|
||||
agent-browser tab close [id] # Close tab by id (defaults to active tab)
|
||||
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 <id>` flag:
|
||||
|
||||
```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
|
||||
```
|
||||
|
||||
### Frames
|
||||
|
||||
```bash
|
||||
@@ -610,6 +621,7 @@ This is useful for multimodal AI models that can reason about visual layout, unl
|
||||
|--------|-------------|
|
||||
| `--session <name>` | Use isolated session (or `AGENT_BROWSER_SESSION` env) |
|
||||
| `--session-name <name>` | Auto-save/restore session state (or `AGENT_BROWSER_SESSION_NAME` env) |
|
||||
| `--tab <id>` | Target a specific tab by stable `tabId` for this command only; the active tab is restored afterward |
|
||||
| `--profile <name\|path>` | Chrome profile name or persistent directory path (or `AGENT_BROWSER_PROFILE` env) |
|
||||
| `--state <path>` | Load storage state from JSON file (or `AGENT_BROWSER_STATE` env) |
|
||||
| `--headers <json>` | Set HTTP headers scoped to the URL's origin |
|
||||
|
||||
@@ -24,6 +24,11 @@
|
||||
"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."
|
||||
|
||||
@@ -1275,19 +1275,43 @@ 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!(
|
||||
// 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 <cmd>` 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<u32> = 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);
|
||||
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,
|
||||
@@ -1447,6 +1471,24 @@ 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)),
|
||||
|
||||
@@ -392,7 +392,9 @@ impl BrowserManager {
|
||||
};
|
||||
|
||||
if direct_page {
|
||||
let tab_id = manager.assign_tab_id();
|
||||
manager.pages.push(PageInfo {
|
||||
tab_id,
|
||||
target_id: "provider-page".to_string(),
|
||||
session_id: String::new(),
|
||||
url: String::new(),
|
||||
@@ -1284,6 +1286,16 @@ impl BrowserManager {
|
||||
self.pages.len()
|
||||
}
|
||||
|
||||
/// Returns the stable `tab_id` of the currently active page, if any.
|
||||
pub fn active_tab_id(&self) -> Option<u32> {
|
||||
self.pages.get(self.active_page_index).map(|p| p.tab_id)
|
||||
}
|
||||
|
||||
/// Returns true if a tab with the given stable `tab_id` is still open.
|
||||
pub fn has_tab_id(&self, tab_id: u32) -> bool {
|
||||
self.pages.iter().any(|p| p.tab_id == tab_id)
|
||||
}
|
||||
|
||||
pub fn pages_list(&self) -> Vec<PageInfo> {
|
||||
self.pages.clone()
|
||||
}
|
||||
@@ -1576,6 +1588,7 @@ mod tests {
|
||||
#[test]
|
||||
fn test_update_page_target_info_in_pages_updates_existing_page() {
|
||||
let mut pages = vec![PageInfo {
|
||||
tab_id: 1,
|
||||
target_id: "popup-1".to_string(),
|
||||
session_id: "session-1".to_string(),
|
||||
url: String::new(),
|
||||
|
||||
+172
-2
@@ -1093,7 +1093,8 @@ async fn e2e_tab_global_targeting() {
|
||||
)
|
||||
.await;
|
||||
assert_success(&resp);
|
||||
// After targeting tab 1 then tab 2, active tab is now tab 2
|
||||
// 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;
|
||||
@@ -1167,7 +1168,8 @@ async fn e2e_tab_global_targeting_snapshot() {
|
||||
snapshot
|
||||
);
|
||||
|
||||
// Snapshot without tabId should use the last-switched tab (tab 2)
|
||||
// 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();
|
||||
@@ -1289,6 +1291,174 @@ async fn e2e_tab_global_targeting_snapshot_non_contiguous() {
|
||||
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,<button>Alpha</button>" }),
|
||||
&mut state,
|
||||
)
|
||||
.await;
|
||||
assert_success(&resp);
|
||||
|
||||
let resp = execute_command(
|
||||
&json!({ "id": "3", "action": "tab_new", "url": "data:text/html,<p>Beta</p>" }),
|
||||
&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,<title>A</title>" }),
|
||||
&mut state,
|
||||
)
|
||||
.await;
|
||||
assert_success(&resp);
|
||||
|
||||
let resp = execute_command(
|
||||
&json!({ "id": "3", "action": "tab_new", "url": "data:text/html,<title>B</title>" }),
|
||||
&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() {
|
||||
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,<title>A</title>" }),
|
||||
&mut state,
|
||||
)
|
||||
.await;
|
||||
assert_success(&resp);
|
||||
|
||||
let resp = execute_command(
|
||||
&json!({ "id": "3", "action": "tab_new", "url": "data:text/html,<title>B</title>" }),
|
||||
&mut state,
|
||||
)
|
||||
.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 }),
|
||||
&mut state,
|
||||
)
|
||||
.await;
|
||||
assert_success(&resp);
|
||||
|
||||
let resp = execute_command(&json!({ "id": "5", "action": "title" }), &mut state).await;
|
||||
assert_success(&resp);
|
||||
assert_eq!(get_data(&resp)["title"], "A");
|
||||
|
||||
let resp = execute_command(&json!({ "id": "99", "action": "close" }), &mut state).await;
|
||||
assert_success(&resp);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Element queries: isvisible, isenabled, gettext, getattribute
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@@ -188,16 +188,29 @@ agent-browser network har stop [output.har] # Stop and save HAR (temp path if
|
||||
## Tabs & frames
|
||||
|
||||
```bash
|
||||
agent-browser tab # List tabs
|
||||
agent-browser tab # List tabs (each row includes a stable tabId)
|
||||
agent-browser tab new [url] # New tab
|
||||
agent-browser tab <n> # Switch to tab
|
||||
agent-browser tab close [n] # Close tab
|
||||
agent-browser tab <id> # Switch to tab by tabId
|
||||
agent-browser tab close [id] # Close tab by tabId (defaults to active)
|
||||
agent-browser window new # Open new browser window
|
||||
agent-browser frame <sel> # Switch to iframe by CSS selector
|
||||
agent-browser frame @e3 # Switch to iframe by element ref
|
||||
agent-browser frame main # Back to main frame
|
||||
```
|
||||
|
||||
### Stable tab IDs and `--tab`
|
||||
|
||||
Tab IDs are assigned on creation and never reused within a session, so a given
|
||||
`tabId` keeps pointing at the same tab even when other tabs are opened or
|
||||
closed. The global `--tab <id>` flag runs a single command against a specific
|
||||
tab without changing the active tab:
|
||||
|
||||
```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
|
||||
```
|
||||
|
||||
### Iframe support
|
||||
|
||||
Iframes are detected automatically during snapshots. `Iframe` nodes are resolved and their content is
|
||||
|
||||
@@ -63,6 +63,7 @@ Every CLI flag can be set in the config file using its camelCase equivalent:
|
||||
<tr><td><code>debug</code></td><td><code>--debug</code></td><td>boolean</td></tr>
|
||||
<tr><td><code>session</code></td><td><code>--session</code></td><td>string</td></tr>
|
||||
<tr><td><code>sessionName</code></td><td><code>--session-name</code></td><td>string</td></tr>
|
||||
<tr><td><code>tab</code></td><td><code>--tab</code></td><td>number</td></tr>
|
||||
<tr><td><code>executablePath</code></td><td><code>--executable-path</code></td><td>string</td></tr>
|
||||
<tr><td><code>extensions</code></td><td><code>--extension</code></td><td>string[]</td></tr>
|
||||
<tr><td><code>profile</code></td><td><code>--profile</code></td><td>string</td></tr>
|
||||
|
||||
@@ -166,14 +166,24 @@ agent-browser network requests --filter api # Filter requests
|
||||
## Tabs and Windows
|
||||
|
||||
```bash
|
||||
agent-browser tab # List tabs
|
||||
agent-browser tab # List tabs (each row includes a stable tabId)
|
||||
agent-browser tab new [url] # New tab
|
||||
agent-browser tab 2 # Switch to tab by index
|
||||
agent-browser tab 2 # Switch to tab by tabId
|
||||
agent-browser tab close # Close current tab
|
||||
agent-browser tab close 2 # Close tab by index
|
||||
agent-browser tab close 2 # Close tab by tabId
|
||||
agent-browser window new # New window
|
||||
```
|
||||
|
||||
Tab IDs are stable and never reused within a session, so the same `tabId` keeps
|
||||
referring to the same tab even when other tabs are opened or closed. For a
|
||||
non-intrusive peek at another tab, use the global `--tab <id>` flag; the
|
||||
active tab is restored after the command:
|
||||
|
||||
```bash
|
||||
agent-browser --tab 1 snapshot # snapshot tab 1 without switching
|
||||
agent-browser --tab 3 click @e1 # click @e1 on tab 3 and return
|
||||
```
|
||||
|
||||
## Frames
|
||||
|
||||
```bash
|
||||
|
||||
Reference in New Issue
Block a user