Skip to content

fix(browser): pin shared CDP sessions to tabs - #86879

Open
the3asic wants to merge 11 commits into
NousResearch:mainfrom
the3asic:feat/browser-cdp-pin-tab-20260815
Open

fix(browser): pin shared CDP sessions to tabs#86879
the3asic wants to merge 11 commits into
NousResearch:mainfrom
the3asic:feat/browser-cdp-pin-tab-20260815

Conversation

@the3asic

@the3asic the3asic commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Prevents high-level browser tasks from hijacking each other when multiple Hermes tasks share one Chrome/CDP endpoint and profile.

  • gives each high-level shared-CDP task a unique named agent-browser session with --cdp --pin-tab
  • binds Hermes' CDP supervisor to that exact pinned target
  • returns structured tab_gone instead of adopting another task's or user's page
  • separates page JavaScript exceptions from CDP transport/session loss; dispatched JavaScript is never replayed after an ambiguous or serialization failure
  • keeps shared external-CDP sessions alive during long model waits, then fails non-navigation calls closed after terminal cleanup instead of creating a replacement about:blank session
  • uses explicit lifecycle state, cleanup reasons, per-task operation locks, and generation fencing so commands, cleanup, session creation, and supervisor publication cannot reopen or leak a retired task
  • keeps provider-backed browser sessions separate from task-owned external CDP; provider cleanup does not depend on local pin-tab capability and failed provider closes retain retry ownership
  • reaps dead-owner targets only from persisted exact target metadata; malformed or unknown metadata fails closed instead of deleting the last ownership evidence

Why this approach?

agent-browser 0.34 provides the missing low-level primitive: persistent named-session-to-target binding plus strict --pin-tab. Hermes owns the lifecycle, publication fencing, supervisor routing, and cleanup guarantees around that primitive rather than adding a second target broker or an endpoint-wide lock.

The shared browser profile is still intentionally shared: cookies, storage, accounts, and rate limits are not isolated. The isolation boundary is page-target ownership.

Addresses the high-level browser path in #62338 and overlaps with the shared-CDP isolation work discussed in #83274, #75597, and #86924. This PR additionally covers Hermes high-level tool lifecycle, provider ownership, supervisor routing, exact-once eval behavior, cleanup/orphan handling, and idle continuity.

Runtime compatibility

  • ordinary non-CDP sessions: exact agent-browser@0.26.0, compatible with Hermes' Node.js 22.22+ baseline
  • shared-CDP pinning: Node.js 24+ and agent-browser >=0.34.0
  • capability-scoped lazy install: exact audited agent-browser@0.34.0; npm's release-age override applies only to that exact acquisition, not future 0.34.x releases
  • Node 22 fails before starting a pin-tab daemon or mutating the shared browser

Testing

Final local verification on commit f9de232401:

  • 538 passed, 8 skipped across tests/tools/test_browser_*.py
  • 44 passed across lifecycle, turn-finalizer, interrupt, and run-agent regression tests
  • 7 passed in managed runtime resolution tests (plus two pre-existing DeprecationWarnings)
  • Ruff, py_compile, and git diff --check pass
  • focused regressions cover JavaScript exception markers and exact-once dispatch, stale-session fallback, version/candidate selection, task lifecycle transitions, cleanup retry, provider ownership, concurrent creator disposal, supervisor start/stop fencing, SIGKILL orphan recovery, malformed target metadata, long idle waits, terminal cleanup, and failed restart navigation

Real isolated Chromium + agent-browser 0.34.0 acceptance covered:

  • two tasks received distinct pinned targets and each read only its own token
  • cleaning task A closed only A; task B remained attached and usable
  • task A then failed closed with browser_session_retired
  • a pre-existing user target was neither read nor closed
  • final cleanup removed every task-owned target while the persistent browser and pre-existing target survived

Node/runtime acquisition was also exercised with empty npm caches on the supported paths.

Scope

This intentionally does not add per-task ACLs to raw browser_cdp. Raw CDP remains a privileged browser-wide escape hatch and can address a caller-supplied target directly. The page-ownership guarantees above apply to the high-level browser_* task flow.

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/browser Browser automation (CDP, Playwright) P2 Medium — degraded but workaround exists labels Aug 15, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #83274 and #75597 address the same shared-CDP isolation outcome through different mechanisms (Hermes-side persistent/dedicated targets versus this agent-browser native named-session pinning). Maintainer selection or consolidation is needed; this is not a duplicate.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(browser): pin shared CDP sessions to tabs

A thorough, security-motivated change: per-task named sessions with --pin-tab, target-pinned supervisors, structured error kinds (never page-controlled text), and terminal task tombstones. The test suite (including a real CDP contract test) is excellent. Observations:

  1. tools/browser_tool.py _ensure_cdp_supervisor — the new guard if cdp_url and target_id is None: return makes the function a silent no-op for any shared-CDP session without a resolved pinned target. That is the correct anti-leak default, but it changes behavior for other callers of _ensure_cdp_supervisor(task_id) that previously relied on attaching to the endpoint's first page (e.g. dialog/frame supervision during a session that never resolves a pinned target via _pinned_cdp_target_id). Worth auditing remaining call sites so supervisor-backed features (dialog handling, console streaming) don't silently degrade for CDP sessions where pinning never succeeds — e.g. a tab list that returns no active page.
  2. _reap_orphaned_browser_sessions + _close_orphaned_pinned_target — a live pinned orphan whose exact close keeps failing is now retained indefinitely (continue with no age-based fallback). The PR deliberately trades hard-kill for exact close (good), but a wedged daemon that never answers could leak a page forever. Consider an age threshold after which a retained pinned orphan falls back to the old process-tree termination, so "fail closed" doesn't become "fail forever".
  3. _find_agent_browser(require_pin_tab=True) — capability failures are deliberately not cached, so every shared-CDP command that can't resolve the capability re-spawns agent-browser --version and node --version probes (up to 2 subprocesses per command). Correct for upgrade responsiveness, but on Node 22 installs every CDP command pays probe overhead before failing with pin_tab_unavailable. A short negative-TTL cache (e.g. re-probe at most once per N seconds) would keep both properties.
  4. SupervisorRegistry.get_or_start — the displaced supervisor is now stopped outside the lock (correct; no deadlock). One consequence: an in-flight evaluate_runtime on the displaced supervisor can hit the loop teardown mid-call and return an ambiguous cdp_evaluate_failed rather than replaying — acceptable, but the docstring on get_or_start could note that a target change invalidates in-flight evals for the old binding.
  5. browser_supervisor.py _attach_initial_page raises RuntimeError when the requested target_id is absent. If that happens inside start(), the supervisor thread dies and eval falls back to the CLI path, which correctly returns tab_gone — verified by tests. Minor: consider catching it in start() to mark the supervisor unavailable with the structured supervisor_unavailable kind instead of a bare RuntimeError (the _run reconnect loop's exception handling may already cover it — worth confirming the thread doesn't silently die).
  6. Docs are strong and honest about the shared-profile tradeoff. Minor: browser_console's early tab_gone return drops the other stream's content even when only one stream reports the gone binding — intentional (both streams share the binding), no change needed.

@the3asic
the3asic force-pushed the feat/browser-cdp-pin-tab-20260815 branch from f9de232 to 13e3e90 Compare August 16, 2026 09:14
@the3asic

Copy link
Copy Markdown
Contributor Author

The triage note above asks for maintainer selection among this PR, #83274,
and #75597. To help that decision, here is a side-by-side of what each
mechanism actually covers. All three address the same underlying failure
(two Hermes tasks sharing one CDP endpoint ending up on the same tab), but
the isolation boundary and lifecycle coverage differ.

#86879 (this PR) #83274 #75597 #86924 (merged)
Layer named agent-browser session with --cdp --pin-tab per task; Hermes owns lifecycle fencing named daemon + labeled tab re-selected before every command supervisor-owned page target via Target.createTarget, batch select+execute browser_use CLI preamble pins named browser_exec sessions to a fresh tab
Isolates built-in tools (browser_navigate/browser_cdp/…) n/a (browser_exec only)
Isolates browser_exec sessions n/a n/a n/a
Session survives long idle waits
Structured tab_gone instead of adopting another task's page partial (re-select per command) partial (owned target)
Terminal cleanup fencing (no session resurrection after cleanup, generation fencing, tombstones)
Exact-once JS eval semantics (no replay after ambiguous send)
Provider-backed sessions kept separate from task-owned CDP; failed closes retain retry ownership
Dead-owner reap only from persisted exact target metadata; malformed metadata fails closed
Test coverage 538 browser-tool tests + 44 lifecycle/turn-finalizer regression tests + real-CDP contract test 139 test lines added 477 test lines added 5 tests
Runtime requirement agent-browser >=0.34.0, Node 24+ for pin-tab path (exact-version audited install; Node 22 fails closed before touching the shared browser) stock daemon behavior stock supervisor stock CLI

Mechanism notes:

  • fix(browser): keep CDP target stable across commands #83274 re-selects a labeled tab before every command through a named
    daemon. That closes the command-time hijack window but keeps the per-command
    re-selection as the only guard — no lifecycle fencing, so a session that
    survives a cleanup or a long idle can still race.
  • fix(browser): give each CDP supervisor its own page target #75597 gives each CDP supervisor its own page target and folds
    select+execute into one batch request. Strong per-operation atomicity, but
    the ownership lives in the supervisor; there is no cross-cutting cleanup
    generation/tombstone, and JS dispatch semantics after transport ambiguity
    are not addressed.
  • fix(browser): pin named browser_exec sessions to their own tab on shared browsers #86924 (already merged) covers the browser_exec surface only —
    named CLI sessions pin to a tab they created. This PR covers the built-in
    high-level tool path, which is a different surface; the two are
    complementary, not competing.
  • fix(browser): pin shared CDP sessions to tabs #86879 (this PR) builds on agent-browser 0.34's native
    named-session-to-target binding and puts Hermes-side lifecycle guarantees
    around it: generation fencing, per-task operation locks, tombstoned cleanup,
    structured tab_gone, exact-once eval, idle continuity, provider/CDP
    ownership separation. It is the only one of the three that treats the
    session lifecycle end-to-end rather than the attach/execute moment.

This PR's regression suite includes an explicit two-tasks-on-one-endpoint
contract test (test_two_tasks_pin_distinct_sessions_on_same_cdp_endpoint),
which is the exact reproduction scenario in #69727 — two sessions, one headed
browser via CDP, each navigates independently. Fixes #69727 on the
built-in tool path.

For transparency: this code has been running on a production Hermes
deployment (multi-platform gateway with concurrent browser tasks over one
shared CDP Chrome) since Aug 16, under daily real workload — scheduled
samplers, interactive sessions, and admin tooling all go through the pinned
path. Issues found during that soak (lifecycle races, capability-probe
retry storms) are already folded back into the final commits here
(f24a569, 13e3e90, af8bf37).

Happy to rebase onto current main (mergeable state is currently clean;
the only overlapping upstream landing since our base is c99a45b "reap
leaked agent-browser daemons", whose semantics this PR's fencing subsumes
— will double-check at rebase time).

@the3asic

the3asic commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main at 22a0771bdd (resolves the dirty state from the last few days of landings).

Verification on the rebased head: 120 browser-tool tests passed across orphan-reaper / lifecycle-fail-closed / CDP-pinning / cleanup / CDP-override / agent-version suites; ruff, py_compile, Windows footgun checker, and diff-check clean. Two test adaptations were needed for changes that landed on main after our branch: the orphan reaper now requires a start-time fingerprint before terminating a daemon (tests mock get_process_start_time and age dirs past BROWSER_ORPHAN_GRACE_SECONDS, matching main's own conventions in 4cca38be86 / 90e916efc9), and the local-Chromium fast-fail gate now short-circuits commands on hosts without Chromium (the two terminal-cleanup race tests stub _is_local_mode so their fake Popen paths still execute).

Note on the Desktop E2E failure on this head: correction-session-switch.spec.ts:188 fails identically on plain main — run 33549336300 (push to main, 2026-09-01 19:24 UTC) hit the same assertion at the same line. This PR does not touch apps/desktop or any file exercised by that spec (changes are confined to tools/browser_tool.py, tools/browser_supervisor.py, run_agent.py, agent/chat_completion_helpers.py and their tests), so that failure is a pre-existing main flake, not a regression from this branch.

@the3asic
the3asic force-pushed the feat/browser-cdp-pin-tab-20260815 branch from 22a0771 to 8953184 Compare September 2, 2026 17:43
@the3asic
the3asic force-pushed the feat/browser-cdp-pin-tab-20260815 branch from 8953184 to b772a42 Compare September 4, 2026 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state tool/browser Browser automation (CDP, Playwright) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants