A single BH_TMP_DIR conflated two storage concerns with opposite
constraints: sock/port/pid files are bound by AF_UNIX sun_path (104
bytes on macOS, 108 on Linux) and must live in a short path; log files
and screenshots have no length limit and benefit from a deep, indexable,
persistent location. Forcing both into one dir means callers either
bury screenshots under /tmp or risk silently overrunning sun_path.
Decouple with a new BH_RUNTIME_DIR for sock/port/pid. BH_TMP_DIR keeps
log/screenshot duty. BH_RUNTIME_DIR falls back to BH_TMP_DIR, then to
/tmp on POSIX or tempfile.gettempdir() on Windows, so single-dir callers
keep working unchanged.
Calling Target.getTargetInfo from helpers can't work: the daemon strips
session_id for any Target.* method, so the call hit the browser-level
connection with no targetId, and Chrome silently returned info about the
*browser* target (empty url/title) instead of the attached page. New
current_tab meta uses self.target_id like connection_status already does.
Fixes#304.
install.md:58-59 documents BU_CDP_URL / BU_CDP_WS as overrides for local
Chrome discovery, but run.py:87-92's auto-bootstrap guard never checked
either env var. With BU_CDP_URL set plus BROWSER_USE_API_KEY and
BU_AUTOSPAWN (commonly set together for unrelated reasons -- profile sync,
headless CI fallback, parent agents managing their own session), no daemon,
and no local Chrome on 9222/9223, the guard fires and start_remote_daemon()
provisions a billed cloud browser. admin.py:471 then calls
ensure_daemon(env={"BU_CDP_WS": _cdp_ws_from_url(browser["cdpUrl"]), ...}),
overwriting the user's explicit endpoint with the cloud WebSocket. Net:
surprise cloud bill plus silent endpoint replacement.
Add _explicit_cdp_configured() next to _local_chrome_listening() and gate
the auto-bootstrap with `not _explicit_cdp_configured()`. Mirrors the
existing _is_local_chrome_mode helper at admin.py:159-161 that already
treats BU_CDP_WS as the "not local discovery" signal. #277's
fresh-headless-box behaviour (no explicit endpoint set) is preserved.
Tests: 8 new unit tests in tests/unit/test_run.py covering URL/WS variants,
both-set, empty-string, daemon-alive and local-Chrome short-circuit
robustness, and direct helper input/output. tests/unit/ goes from 66 to 74,
all green. Without this patch, exactly 5 of the new tests fail (the ones
targeting the guard + helper); the 3 short-circuit/empty edge cases pass
even unpatched because existing guards already cover those paths.
Refs #266, #277, #292.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
codex flagged that set_session was now performing up to four sequential
domain enables (5s timeout each) plus a 2s Network.disable, so a slow
or remote daemon could block the synchronous IPC reply for ~22s while
the helper's _send() socket has only a 5s read timeout. Old code only
awaited Page.enable (3s) on this path.
Three changes to keep the reply under the IPC deadline:
1. _enable_default_domains now awaits asyncio.gather over the four
Domain.enable coroutines instead of looping sequentially. Per-call
timeout reduced from 5s to 4s. Worst case is bounded by a single
CDP round trip rather than four.
2. set_session schedules the old-session Network.disable in the same
gather as the four enables on the new session — independent CDP
sessions, no ordering required for correctness (the consumer-side
filter in wait_for_network_idle is the actual correctness gate).
3. The 🟢 tab-marker title-prefix Runtime.evaluate is now fire-and-forget
via asyncio.create_task (+ _silent). It's purely cosmetic; agents
shouldn't wait on it.
Worst-case set_session reply time: max(2s disable, 4s parallel enables)
≈ 4s, comfortably under the 5s IPC timeout. Normal case on a remote
daemon drops from ~800ms (4 sequential round trips) to ~200ms (1 round
trip in parallel).
Two new tests in tests/unit/test_daemon.py use a fake CDP whose
send_raw blocks on an asyncio.Event, then assert the peak in-flight
call count:
- with a previous session: peak == 5 (1 disable + 4 enables)
- first attach: peak == 4 (4 enables, disable skipped)
Sequential await would peak at 1 on both. Full suite: 65 passed.
codex flagged a P2 follow-on to the just-added Network.enable: my fix
reliably enables Network on every fresh session, but old sessions
never get Network disabled (helpers.switch_tab attaches without
detaching), and wait_for_network_idle reads from the daemon's global
drain_events stream without filtering by session_id. Net effect: an
agent that visited a polling/SSE tab and switched away would observe
that background tab's traffic in a later wait, either timing out or
waiting on the wrong tab's requests.
Two narrow fixes:
1. helpers.wait_for_network_idle now captures the active session at
the start of the wait and skips events whose session_id doesn't
match. That's the consumer-side root-cause fix.
2. daemon set_session now calls Network.disable on the previous
session before enabling on the new one. Defense in depth — keeps
the daemon's event buffer from filling with background-tab noise
in the first place. Best-effort with its own 2s timeout; failure
doesn't abort the rest of the handler.
Tests added:
- tests/unit/test_helpers.py: events from a background session are
ignored; the active session can reach idle even when the
background session is busy.
- tests/unit/test_daemon.py: set_session disables Network on the old
session; first set_session call (no prior session) does not call
Network.disable.
Full suite: 63 passed (60 -> 63).
Prior to this change, the daemon's set_session meta-handler only called
Page.enable on the new CDP session. The initial-attach path enabled all
four of Page/DOM/Runtime/Network. set_session is what backs switch_tab()
and new_tab() in helpers.py, so any helper that depends on Network
events — most notably the wait_for_network_idle() that just landed in
PR #258 — silently stopped receiving events after a tab switch.
Refactored: extracted the domain-enable loop into a private
_enable_default_domains(session_id) helper used by both attach_first_page
and the set_session handler. Each domain is enabled with its own
timeout, and a single failure does not abort the others.
Also tightened the target_id fallback semantics: if a caller passes
target_id=None on set_session, the daemon keeps its existing target_id
rather than overwriting with None (preserves the existing 'or' fallback,
just covered by a test now).
Tests in new tests/unit/test_daemon.py drive Daemon.handle() directly
with a fake CDP client and assert:
- set_session enables all four default domains on the new session
(the regression — would fail against the old single-Page.enable code)
- target_id is preserved when caller passes None
- _enable_default_domains attempts every domain even when one raises
Identified via codex review (P1). Full suite: 60 passed.
codex flagged that the SIGTERM gate could never pass on Windows because
_process_start_time() returned None there, while ipc.identify() also
returns None during the slow-shutdown window (the daemon's serve()
tears down the IPC endpoint before stop_remote() runs its 15s-timeout
PATCH to api.browser-use.com). Combined: a Windows user with a remote
daemon would lose force-kill, leaving an orphan that may keep a billed
cloud browser alive.
Implemented the Windows branch via ctypes:
OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, pid)
GetProcessTimes -> creation FILETIME (100-ns intervals since 1601)
CloseHandle on the way out
The combined high/low DWORDs of the creation FILETIME serve as the
opaque fingerprint. PROCESS_QUERY_LIMITED_INFORMATION is sufficient
for GetProcessTimes (Vista+) and avoids needing PROCESS_QUERY_INFORMATION
which is more privilege-sensitive.
Also tightened the top guard from isinstance(pid, int) to
type(pid) is int — bool subclasses int in Python and was previously
slipping through (though the f-string formatting in the Linux branch
saved us by hitting /proc/True/stat which doesn't exist).
Test coverage: extended test_process_start_time_returns_stable_fingerprint_for_self
to also run on win32. Full suite: 75 passed locally on macOS.
Two more codex findings on ad39a95, both real:
1. _ipc.identify() accepted any positive int as pid, including values
too large for C pid_t (typically signed 32-bit). os.kill(huge_pid, 0)
then raises OverflowError, which propagates out of restart_daemon()
before its cleanup runs. Bounded the accepted range to 0 < pid <
2**31. Linux pid_max is also <2**22 in practice. Defense-in-depth:
added OverflowError to the except lists wrapping the os.kill calls
in restart_daemon, in case a different code path ever feeds it a
too-large pid.
2. The daemon's serve() tears down the IPC socket BEFORE the daemon
process actually exits — the daemon then runs slow cleanup work
(notably stop_remote()'s 15s-timeout PATCH to api.browser-use.com
to release a cloud browser). During that window identify() returns
None even though the process is still our daemon, so the strict
identify-only re-verification skipped SIGTERM and let the orphan
keep running. The next bh invocation would then spawn a new daemon
competing with the old one for the same Chrome.
Fix: snapshot a process-start-time fingerprint at the top of
restart_daemon, and accept it as a secondary identity signal in
the SIGTERM gate. Two reads returning the same fingerprint means
the PID still refers to the same process; a different fingerprint
means PID reuse, in which case SIGTERM is skipped (preserving the
protection from cubic's earlier finding). Implementation uses
/proc/<pid>/stat field 22 on Linux and ps -o lstart= on macOS;
on Windows / unsupported platforms the helper returns None and
restart_daemon falls back to the strict identify-only check.
Tests in tests/unit/test_admin.py and tests/unit/test_ipc.py:
- restart_daemon SIGTERMs via start-time match when socket is gone
(slow-shutdown recovery)
- restart_daemon skips SIGTERM when start-time has changed (PID
reuse during the wait window)
- _process_start_time returns a stable fingerprint for the current
process and None for invalid pids (None/0/negatives/non-int/dead)
- identify rejects oversized ints (covered via the bounded check)
Full suite: 75 passed.
Two additional codex findings on 6d412c9:
1. _ipc.identify() accepted any int as pid, including 0 and negatives.
On POSIX, os.kill(0, sig) signals every process in the calling
process group, and os.kill(-1, sig) signals every process the
caller can. A hostile or buggy daemon replying {pid: 0} or
{pid: -1} would have turned restart_daemon() into a process-group
kill. Restrict to pid > 0.
2. _ipc.ping() still did resp.get('pong') without a type check, so a
list/scalar/null reply would raise AttributeError. The previous
commit added that guard to identify() but left ping() bare. With
the new daemon_alive fallback in restart_daemon() that now calls
ping(), an unhandled raise here would abort restart before
cleanup ran. Mirrored the identify() guards: isinstance(resp, dict)
plus AttributeError in the except.
Tests in tests/unit/test_ipc.py now also cover:
- identify() rejects pid=0, pid=-1, pid=-42, pid=-99999
- ping() returns False for non-dict payloads (list/str/int/None)
- ping() requires pong is exactly True (rejects truthy non-True values)
Full suite: 71 passed.
Two related cubic/codex findings on _ipc.identify():
1. isinstance(pid, int) accepts bool (since bool subclasses int in
Python), so a hostile or buggy daemon replying {pid: True} would
yield PID 1 and os.kill(1, SIGTERM) would target init on POSIX.
Switched to type(pid) is int — strict same-type check, no subclass
surprises.
2. request() returns whatever JSON the daemon sent, which can be a
list, scalar, or None for a stale/hostile endpoint. resp.get(...)
would then raise AttributeError, propagating out of identify() and
crashing restart_daemon() before its cleanup runs. Added an
isinstance(resp, dict) guard and AttributeError to the except.
Tests in new tests/unit/test_ipc.py cover both rejections plus the
happy path, the missing-pid path (pre-upgrade daemon), pong=False,
and several non-dict shapes (list, str, int, None). Full suite: 67
passed.
Two issues raised on the initial commit:
1. Pre-upgrade daemons that don't include 'pid' in their ping reply made
identify() return None, which short-circuited the entire shutdown
path. The function would still delete the socket and pid file,
orphaning the still-running daemon. Fix: keep daemon_pid (verified
PID, used for signaling) separate from daemon_alive (any pong reply
counts, falls back to ipc.ping for the alive check). Shutdown IPC
is now sent whenever daemon_alive is true, regardless of whether
we got a verifiable PID; SIGTERM still requires a verifiable PID.
2. A single identify() at the top didn't prevent PID reuse during
the 15-second wait loop. If the daemon exited and the kernel
reused its PID before the loop timed out, SIGTERM would land on
the new owner. Fix: re-call identify() right before SIGTERM and
only signal if it still returns the same PID we've been waiting
on — any other state (None, different PID) means PID reuse is
possible and we skip the kill.
Two new tests in tests/unit/test_admin.py lock in both:
- test_restart_daemon_sends_shutdown_to_pre_upgrade_daemon_without_pid_in_ping
- test_restart_daemon_skips_sigterm_if_pid_was_reused_during_wait
Existing tests updated to also stub ipc.ping. Full suite: 61 passed.
The previous restart_daemon() read the daemon's PID from the
/tmp/<name>.pid file and could end up calling os.kill(pid, SIGTERM)
on whatever process now owned that PID — a real hazard whenever the
daemon had crashed without removing the file and the OS subsequently
reused the PID for an unrelated local process (browser, editor,
language server, ...). The 15-second polling loop made the issue
worse: any time os.kill(pid, 0) kept succeeding (because the new
owner stayed alive), the loop would fall through to SIGTERM.
Fix: ask the live daemon for its PID over the existing IPC ping
channel before signaling anything. The 'ping' meta-handler now
returns {pong: True, pid: os.getpid()}, and a new ipc.identify(name)
helper returns that PID or None if the daemon is unreachable.
restart_daemon() now signals only the PID returned by identify(),
never the contents of the pid file. If the daemon is unreachable we
just clean up the socket and pid file and return — we never escalate
to a kill-by-pid-file.
Adds two unit tests in tests/unit/test_admin.py:
- the unreachable-daemon path performs zero os.kill calls (the
exact regression: previously the stale pid file would still get
signaled);
- when identify() returns a different PID than the pid file, only
the identify-supplied PID is signaled.
Identified via codex code review (P0). All 59 unit tests pass.
Cross-checked AGENTS.md, README.md, SKILL.md, install.md, and profile-sync.md for accuracy. Tightened the sync_local_profile docstring and the chrome://inspect stderr message to match. Cloud-bootstrap test updated to set BU_AUTOSPAWN. All unit tests pass.
press_key emits a Input.dispatchKeyEvent('char', text='a') after every
single-character keyDown. With Cmd/Ctrl held, that char event makes Chrome
treat the input as a printable letter "a" rather than firing the
select-all shortcut, so the field never gets cleared (Backspace would
then delete the literal "a" and leave the original value untouched).
Dispatch the rawKeyDown/keyUp pair directly with the modifier set and no
text/char event. Tightened the test to assert (a) the modifier is on the
'a' event with the platform-correct value and (b) no char event with
text='a' is emitted.
The computed-style check measures the element itself, so an element nested
inside a display:none or visibility:hidden ancestor is reported as visible
(getComputedStyle returns the descendant's own non-none value, not the
inherited "is rendered" state). checkVisibility walks the ancestor chain
and is the right primitive on modern Chrome. Kept the per-element CSS
check as a fallback for older Chrome that lacks checkVisibility.
Three improvements to the cross-platform IPC layer, lifted from #104:
- meta:'ping' handshake replaces bare TCP connect in daemon_alive() and already_running(). A connect-only check on Windows can succeed against an unrelated process that grabbed our ephemeral port after a daemon crash; the ping/pong response confirms the listener is actually our daemon.
- Per-daemon random token (secrets.token_hex(32)) gates every request on Windows. AF_UNIX + chmod 600 is the boundary on POSIX, but TCP loopback has no chmod-equivalent; without a token any local process could connect and issue CDP commands.
- Atomic .port write (write .port.tmp, os.replace) so a concurrent reader never sees a half-written file.
Adds rohitdutt108 to VOUCHED.td.
Co-authored-by: Rohit Dutt <rohit.dutt@iyc.ishafoundation.org>
Domain skills auto-injected by goto_url() are community-contributed and quality varies; defaulting them off avoids polluting the average run while preserving the contribution loop. Set BH_DOMAIN_SKILLS=1 to opt in.
Cubic flagged that the original socket.create_connection probe matches any process on 9222/9223, not just Chrome. Mirror daemon.py's fallback by hitting /json/version, so a stale or unrelated listener does not skip the cloud bootstrap.
Drop the three boolean-table tests that mocked every collaborator and re-asserted the literal if-condition. Add a focused test for _local_chrome_listening that covers the false-positive case directly.
On headless servers (VPS, Docker) with no local Chrome, ensure_daemon() fires
before any user script runs and raises immediately — start_remote_daemon() can
never be reached from within a -c script.
Add a pre-check in main(): if no daemon is alive, Chrome is not listening on
known debugging ports (9222/9223), and BROWSER_USE_API_KEY is set, auto-
provision a Browser Use cloud browser before falling through to ensure_daemon().
_local_chrome_listening() probes ports 9222/9223 with a 0.3s timeout rather
than relying on _is_local_chrome_mode(), which only checks for absence of
BU_CDP_WS and would incorrectly trigger cloud bootstrap on a local machine
where Chrome is running but BROWSER_USE_API_KEY is also set (e.g. for profile
sync).
Fixes the behaviour reported in issues #181 and #183.
Tested on a headless Hostinger VPS running hermes-agent in Docker — browser-
harness -c '...' now works without any manual daemon setup when BROWSER_USE_API_KEY
is set.
When the browser dies but the daemon process keeps running, target_id and session stay cached on the Bridge. Pre-fix, connection_status swallowed the Target.getTargetInfo failure and returned the cached IDs with page=null, so admin counted the daemon as healthy. Now it returns {error: cdp_disconnected} and admin's existing error-check skips it. Also returns {error: not_attached} when target_id is unset.
When BH_TMP_DIR is set, the caller owns the directory and is expected to
give each daemon its own — so the bu-<NAME> filename prefix is redundant.
Dropping it cuts ~30 chars off the AF_UNIX path on POSIX, which matters
because sun_path is 104 bytes on macOS / 108 on Linux and a long BU_NAME
(e.g. an opencode-style session id) can blow past it.
Concretely the failing path on macOS for a 30-char session id under
~/.local/share/bcode/sessions/<sessionID>/bu-<sessionID>.sock was 117
bytes; with this change it becomes ~83 bytes.
When BH_TMP_DIR is unset, _TMP is the shared default (/tmp on POSIX,
gettempdir() on Windows) and the bu-<NAME> prefix is kept to disambiguate
multiple daemons sharing the dir. Backward compatible by default.
admin._daemon_endpoint_names: when BH_TMP_DIR is set, the dir is per-daemon
by construction, so glob discovery doesn't apply — just check whether our
local endpoint exists. Without BH_TMP_DIR, fall back to the existing
shared-dir glob.
Validation (_check on BU_NAME) still runs in both modes to catch garbage
names early.
Add browser_connections() / active_browser_connections() to admin, which probe
each daemon socket via a new connection_status meta message. The daemon tracks
target_id (set on attach and on set_session) and returns the live page title/URL
via Target.getTargetInfo. switch_tab now forwards target_id in set_session so the
daemon stays in sync after tab switches. run_doctor shows the count, each daemon
name, and the truncated active-page title and URL.
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace naive `"return " in expression` check with a character-level parser that
ignores strings, line comments, and block comments. Add `_decode_unserializable_js_value`
to handle NaN, ±Infinity, -0, and BigInt. Extract `_runtime_value` and
`_runtime_evaluate` to unify error handling across `js()` and `page_info()`.
Wrap Runtime.evaluate TimeoutErrors in RuntimeError with expression context.
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* refactor(tests): reorganize into tests/unit and tests/integration
Moves all root-level test_*.py files into a structured tests/ directory:
- tests/unit/ — admin, helpers (was test_screenshot), run
- tests/integration/ — js expression tests
- tests/conftest.py — shared fake_png pytest fixture, eliminating duplication
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* refactor: move to src layout, agent-workspace, and fix SKILL.md invocation format
- Move package to src/browser_harness/ and domain-skills/interaction-skills to agent-workspace/
- Fix all browser-harness <<'PY' heredoc examples in SKILL.md and run.py HELP string to use the correct -c '...' flag format (heredoc was never supported by the CLI)
- Update SKILL.md path references from domain-skills/ to agent-workspace/domain-skills/
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Laith Weinberger