Skip to content

fix(browser): keep CDP target stable across commands - #83274

Open
brandonwadepackard-cell wants to merge 1 commit into
NousResearch:mainfrom
brandonwadepackard-cell:agent/browser-cdp-target-stability
Open

fix(browser): keep CDP target stable across commands#83274
brandonwadepackard-cell wants to merge 1 commit into
NousResearch:mainfrom
brandonwadepackard-cell:agent/browser-cdp-target-stability

Conversation

@brandonwadepackard-cell

Copy link
Copy Markdown

What changed

  • --cdp sessions now run through a named agent-browser daemon: the daemon is connected once, and every subsequent command addresses it with --session NAME instead of re-attaching one-shot with --cdp <url> per command.
  • Each CDP session binds a dedicated labeled tab (tab new --label …) and re-selects it before every command, so Hermes commands stop following whatever Chrome target happens to be active externally.
  • Auxiliary daemon commands write stdout/stderr to owner-only (0600) temp files, redact secrets in error paths, and fail closed on invalid JSON.

Why

With one-shot --cdp attachment, agent-browser (>=0.13, observed through 0.26) can attach to the externally-active Chrome target at command time. A long-idle session — or a user switching tabs mid-task — makes consecutive Hermes browser commands land on different targets. Pinning a named daemon plus a dedicated labeled tab keeps the target stable across the whole command sequence.

Verification

scripts/run_tests.sh tests/tools/test_browser_cdp_override.py tests/tools/test_browser_headed_mode.py -q
2 files, 24 tests passed, 0 failed

Behavior-level tests cover: connect-once-then-command, live-daemon detection (no reconnect on reuse), dedicated-tab creation/reselection/close, and secret redaction in timeout/start-error paths.

Notes for review

If a Hermes process restarts while the named daemon survives, the dedicated-tab bookkeeping resets and the code issues a fresh tab new --label <name>; behavior with a pre-existing identical label follows agent-browser's label semantics. Flagging in case reviewers prefer an explicit select-then-create fallback there.

🤖 Generated with Claude Code

@brandonwadepackard-cell
brandonwadepackard-cell marked this pull request as ready for review August 10, 2026 15:32
@alt-glitch alt-glitch added type/bug Something isn't working tool/browser Browser automation (CDP, Playwright) P2 Medium — degraded but workaround exists labels Aug 10, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

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

fix(browser): keep CDP target stable across commands

  1. tools/browser_tool.py:2440–2455 (_agent_browser_daemon_is_live) — the liveness check only proves a PID exists, not that the daemon is bound to the expected cdp_url. If a session name is reused against a different CDP endpoint (or the OS recycles the PID), the check passes and subsequent commands silently target the wrong browser. Consider persisting the connected URL next to the pid file and comparing it on each check.
  2. tools/browser_tool.py:2696–2703 — there is no mutual exclusion around the one-shot connect: two concurrent CDP commands that both observe daemon_is_live == False will both run connect and race on the daemon. A per-session lock (or an idempotent connect-if-absent flag) would make this safe under parallel tool calls.
  3. tools/browser_tool.py:2746–2758close in CDP mode closes only the dedicated tab; the daemon stays connected to the remote CDP target, and later commands transparently recreate the tab. Please confirm daemon teardown happens at session end, otherwise the daemon lingers against the remote browser after the session is closed.
  4. Minor — in the new tests the builtins.open mock returns success_json for every read; it happens to work because the pid-file check uses Path.read_text, but the mock would mask real IO behavior if _run_browser_command later reads another file. Consider keying the mock on the expected path.

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.

3 participants