Skip to content

fix(tools): stand down the own-tab preamble when browser-harness isolates natively - #91528

Open
liuhao1024 wants to merge 2 commits into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-91522
Open

fix(tools): stand down the own-tab preamble when browser-harness isolates natively#91528
liuhao1024 wants to merge 2 commits into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-91522

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Stops browser_exec from creating a redundant second tab for every named session on current browser-harness releases. Hermes' _OWN_TAB_PREAMBLE (#86924) pinned named sessions to their own tab because the stock harness attached named daemons to the first shared page; one day later browser-harness 0.1.9 shipped native named-daemon tab isolation (browser-use/browser-harness#618). With both layers active, the harness creates its dedicated tab AND the preamble creates a second target — one extra Chrome renderer (~8-11 Linux cgroup tasks) per named session, materially accelerating PID-cgroup exhaustion under parallel sessions (#91522).

The fix detects harness ≥ 0.1.9 inside the preamble itself (leading-numeric parse of importlib.metadata.version("browser-harness")) and returns before creating any target. The check lives in the preamble rather than in Hermes because the browser-harness package is installed in the CLI's environment, not necessarily in the gateway process. Pre-0.1.9 harnesses keep the compatibility pin; an unresolvable version falls back to the pin (pre-fix behavior is safe); private provider browsers keep skipping the preamble entirely, as before.

Related Issue

Fixes #91522

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tools/browser_use_cli.py: Add _NATIVE_TAB_ISOLATION_SINCE = (0, 1, 9) (with a comment tying it to the embedded gate) and the _parse_version_prefix helper documenting the leading-numeric parse algorithm. Embed the version gate at the top of _OWN_TAB_PREAMBLE::_hermes_ensure_own_tab: on harness ≥ 0.1.9, return before any Target.createTarget; on parse failure, fall through to the pin.
  • tests/tools/test_browser_use_cli.py: In TestOwnTabPreamble, add test_preamble_stands_down_on_native_tab_isolation (AST structural assertion: the gate's return precedes the createTarget call, and the embedded threshold matches the module constant) and test_version_prefix_parser (pure parser coverage: 0.1.9rc1 → gate, 0.1.8 → no gate, short/pre-release forms).

How to Test

  1. With browser-use 0.13.8+ (browser-harness 0.1.9+), start a fresh named browser_exec session against a shared CDP Chrome and diff http://127.0.0.1:9222/json/list before/after
  2. Before the fix two page targets are created for one named session; after the fix exactly one is created and the named session's isolated current tab is preserved (matches the issue's patched live E2E: one target, isolation intact, target removed on idle cleanup)
  3. With a pre-0.1.9 browser-harness, the compatibility pin still runs (Observed result: test_version_prefix_parser asserts 0.1.8 < threshold)
  4. scripts/run_tests.sh tests/tools/test_browser_use_cli.py -q → should pass except the pre-existing environment-dependent timeout test (Observed result: 93 passed, 1 failed — test_timeout_returns_actionable_error fails identically on unpatched main)

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)

…ates natively

Hermes' _OWN_TAB_PREAMBLE (NousResearch#86924) pinned named browser_exec sessions to
their own tab because the stock harness attached named daemons to the
first shared page. One day later, browser-harness 0.1.9 shipped native
named-daemon tab isolation (browser-use/browser-harness#618), so with
current releases both layers run: the harness creates its dedicated tab
and the preamble creates a second target — a redundant Chrome renderer
(~8-11 cgroup tasks) per named session, accelerating PID-cgroup
exhaustion under parallel sessions (NousResearch#91522).

Detect harness >= 0.1.9 inside the preamble (leading-numeric version
parse of importlib.metadata) and return before creating any target.
Pre-0.1.9 harnesses keep the compatibility pin; unresolvable versions
fall back to the pin; private provider browsers keep skipping the
preamble entirely.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists tool/browser Browser automation (CDP, Playwright) labels Aug 21, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

  1. tests/tools/test_browser_use_cli.py:548-552 — The docstring claims "The embedded threshold matches the module-level constant so the two can't drift apart silently", but the assertion only checks _NATIVE_TAB_ISOLATION_SINCE == (0, 1, 9) against another hard-coded literal; nothing extracts the tuple embedded inside _OWN_TAB_PREAMBLE. Why it matters: editing the preamble's (0, 1, 9) to e.g. (0, 2, 0) while leaving the constant passes CI, reintroducing the redundant-tab bug or silently disabling the pin for 0.1.x users. Suggestion: AST-parse the preamble's Compare node and assert its tuple equals bu_cli._NATIVE_TAB_ISOLATION_SINCE.

  2. tests/tools/test_browser_use_cli.py:534 — The comment says "Walk statements in source order", but ast.walk is breadth-first, not source order; the gate-return-is-first assumption only holds by accident of nesting depth (both returns sit at the same depth today, ordered by their parent try-blocks). Why it matters: a small restructure (e.g., wrapping the gate in an extra block) flips which return is found first and the test starts asserting the wrong pair. Suggestion: collect the line numbers of all Return/cdp(...) candidates and compare min(return lines) < create_target_line, or filter returns to those inside the version-gate Try.

  3. tools/browser_use_cli.py:66-79 (_hermes_ensure_own_tab) — importlib.metadata.version() runs on every preamble execution even when it stands down, and distribution metadata lookups scan sys.path (single-digit milliseconds, sometimes worse on cold caches). Why it matters: the preamble is injected per session/exec, so this adds avoidable latency on every named-session start for 0.1.9+ users who get a no-op 99% of the time. Suggestion: memoize the verdict in a stable location available across execs (e.g., an env var like BU_TAB_GATE=stand-down set alongside the existing marker file, checked before the metadata lookup).

  4. tools/browser_use_cli.py:41-51 — Positive/nit: falling back to running the pin when the version is unresolvable (except Exception) is the right safe default, and duplicating the parser inline is justified since the preamble executes in the harness sandbox; the module-level _parse_version_prefix mirror plus test_version_prefix_parser keeps the algorithms honest. No change needed — noting for reviewers that the two implementations must stay textually in sync (see item 1 for enforcing the threshold half of that invariant).

@liuhao1024

Copy link
Copy Markdown
Contributor Author

Thanks — points 1 and 2 addressed in 6d7777b:

  • Threshold pin (point 1): the test now AST-extracts the >= (0, 1, 9) tuple from the preamble's version gate (Compare with GtE and an all-constant tuple comparator) and asserts it equals _NATIVE_TAB_ISOLATION_SINCE — editing the preamble's literal without the constant (or vice versa) now fails the test.
  • BFS-ordering assumption (point 2): the test collects the line numbers of every Return and asserts min(return_lines) < create_target_line; no reliance on ast.walk traversal order, and the comment now states the walk is breadth-first.
  • Per-exec metadata lookup (point 3): agree it's avoidable work on the stand-down path, but memoizing across execs needs a protocol (env-var marker set alongside the pid-keyed marker file, or similar) — that's an interface decision I'd rather leave to maintainers than bolt on here. The lookup is bounded and only runs for named sessions.
  • Point 4: acknowledged, thanks — the AST pin from point 1 is exactly the enforcement half.

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

Labels

P2 Medium — degraded but workaround exists 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.

browser_exec creates a redundant tab with browser-harness 0.1.9+

3 participants