Skip to content

Ready: fix(tabs): stop event-discovered tabs from stealing the active tab#1426

Open
soichisumi wants to merge 1 commit into
vercel-labs:mainfrom
soichisumi:fix/no-active-steal-on-discovery
Open

Ready: fix(tabs): stop event-discovered tabs from stealing the active tab#1426
soichisumi wants to merge 1 commit into
vercel-labs:mainfrom
soichisumi:fix/no-active-steal-on-discovery

Conversation

@soichisumi

Copy link
Copy Markdown

Summary

  • Stop event-discovered (Target.targetCreated) tabs — e.g. a tab the human opens in a shared Chrome — from silently stealing the daemon's active tab.

Why

When the daemon is attached to a shared Chrome (e.g. over --cdp, or whenever a human / another client also drives the browser), every command first drains pending CDP target events. A Target.targetCreated for a tab someone else opens is registered via add_page(), which does self.active_page_index = index unconditionally — so a tab the human opens silently becomes the daemon's active tab, and the agent's next command (snapshot / click / fill) targets the wrong page. In a shared-Chrome / human-in-the-loop setup this is the dominant "my command went to the wrong tab" failure.

What

  • Split the activation decision out of add_page():
    • add_page() keeps activating (explicit user commands, recording setup).
    • add_page_without_activation() appends without moving the active pointer.
    • a pure active_page_index_after_add() helper encodes the rule.
  • The event-drain path (apply_drained_events, on Target.targetCreated) now uses add_page_without_activation().
  • Unit tests for the helper.

How

active_page_index_after_add(active, new_index, was_empty, activate) returns the new index only when the caller explicitly activates or the manager was empty (first/only page) — mirroring the existing active_page_index_after_removal helper so the rule is unit-testable without a live BrowserManager.

Discovered tabs still appear in tab list, but never steal active. Explicit new-tab paths are unaffected — tab new, window new, and click --new-tab activate via their own code paths (they do not go through the event drain).

Behavior change to note: a popup opened by page JS (window.open / target="_blank" without --new-tab) also arrives via the discovery path, so it is now append-only too — interact with it via tab switch. A future refinement could special-case popups whose openerId is the active target (CDP Target.targetCreated carries openerId), but that is not modeled today, and re-introducing "a new target always steals active" is exactly the bug this fixes.

Test plan

  • [auto] cargo test --bin agent-browseractive_page_index_after_add tests pass: first-page activates; discovered tab is append-only; explicit activation moves active.
  • [one-shot-e2e] N/A — reproducing the live race needs a second concurrent tab-opener driving the same shared Chrome; the decision logic is fully covered by the pure-function unit tests above.
  • [manual] Inspected the single rewired call site (apply_drained_events -> add_page_without_activation) and confirmed explicit new-tab paths still activate via their own code paths.

When the daemon is attached to a shared Chrome (e.g. over `--cdp`), every
command first drains pending CDP target events. A `Target.targetCreated` for a
tab the human (or another client) opens is registered via `add_page()`, which
unconditionally sets it active — so an externally opened tab silently steals
the agent's active tab and the next command targets the wrong page.

Split the activation decision out of `add_page()`:
- `add_page()` keeps activating (explicit user commands, recording setup).
- `add_page_without_activation()` appends without moving the active pointer.
- a pure `active_page_index_after_add()` helper makes the rule unit-testable
  (mirrors the existing `active_page_index_after_removal` helper).

The event-drain path now uses `add_page_without_activation()`, so discovered
tabs still appear in `tab list` but never steal active. Explicit new-tab paths
(`tab new`, `window new`, `click --new-tab`) are unaffected — they activate via
their own code paths. The first/only page still activates, so a non-empty
manager always has a valid active index.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

@soichisumi is attempting to deploy a commit to the Vercel Labs Team on Vercel.

A member of the Team first needs to authorize it.

@soichisumi soichisumi marked this pull request as ready for review June 8, 2026 05:56
@soichisumi soichisumi changed the title fix(tabs): stop event-discovered tabs from stealing the active tab Ready: fix(tabs): stop event-discovered tabs from stealing the active tab Jun 8, 2026
@soichisumi

soichisumi commented Jun 13, 2026

Copy link
Copy Markdown
Author

also this is needed 🙏
Cannot upgrade agent-browser wo these features #1425 #1426 #1427

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant