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.
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 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.
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>
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>
* 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>
Gregor Žunič