From 7a55ef4b6b003c2c1329149fbbbfceeb74283eed Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sat, 12 Sep 2026 17:29:45 -0700 Subject: [PATCH] fix(bot-screen): agent attaches to a human-started dock Browser instead of dying on the profile singleton MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dock's Browser launched raw Chromium on the shared user-data-dir with no automation endpoint. When the human opened it first and handed back, agent-browser's own launch was forwarded into their instance by Chromium's ProcessSingleton and exited 21 without a DevToolsActivePort, so every browser_navigate failed until the human closed their window. The reverse order worked, which is why it slipped through. - The dock command carries --remote-debugging-port=0, so a human-started instance advertises a port in /DevToolsActivePort (browser.dock_command; runtime.start reads it). - browser.running_instance_cdp_port() trusts that file only when SingletonLock's pid is alive AND the port accepts a connection (both files outlive a closed Chromium), and never for the instance the calling agent-browser session launched itself: handing that daemon --cdp makes it treat the launch as a config change, close its browser and attach to the port that just died with it (seen live). - The local argv builder appends --cdp to the --session launch when such an instance exists, so the same daemon (and its snapshot refs) drives the human's window. Live, HERMES_HOME=/tmp/bs-f-home on this host: human-first — dock instance up, navigate x2 succeeded, one Chromium main process (same pid) throughout; agent-first — navigate, dock click, navigate x2 succeeded, one process throughout. Before the fix human-first returned "Chrome exited early (exit code: 21) ... Failed to create SingletonLock". (cherry picked from commit d732fad0af005ffd38eca153dd29c0f7e9dd6bc7) --- tests/tools/test_bot_desktop_browser.py | 67 ++++++++++++++++++++++++- tools/bot_desktop/browser.py | 65 +++++++++++++++++++++++- tools/bot_desktop/runtime.py | 5 +- tools/browser_tool_session.py | 15 ++++++ 4 files changed, 147 insertions(+), 5 deletions(-) diff --git a/tests/tools/test_bot_desktop_browser.py b/tests/tools/test_bot_desktop_browser.py index b3bb6ce9e8..2cfdc5523b 100644 --- a/tests/tools/test_bot_desktop_browser.py +++ b/tests/tools/test_bot_desktop_browser.py @@ -1,7 +1,12 @@ -"""The dock's Browser and agent-browser resolve to one identity: same executable, same user-data-dir.""" +"""The dock's Browser and agent-browser resolve to one identity: same executable, same user-data-dir — +and when a human opened that browser first, the agent attaches to it instead of launching a second one +(Chromium's profile singleton would forward the launch and kill it without a DevTools endpoint).""" from __future__ import annotations +import os +import socket + from tools.bot_desktop import browser, runtime @@ -24,3 +29,63 @@ def test_user_pinned_profile_wins(tmp_path, monkeypatch): monkeypatch.setenv("AGENT_BROWSER_PROFILE", str(tmp_path / "mine")) monkeypatch.setattr(runtime, "state_dir", lambda: tmp_path / "bot-desktop") assert browser.profile_dir() == tmp_path / "mine" + + +def test_dock_browser_advertises_a_devtools_port(): + """A human-started instance must be attachable, or the agent can never drive it afterwards.""" + assert "--remote-debugging-port=" in browser.dock_command("/opt/chrome", "/p/dir").split()[2] + + +def _fake_running_instance(user_data_dir, pid: int, port: int) -> None: + (user_data_dir / "DevToolsActivePort").write_text(f"{port}\n/devtools/browser/abc\n", encoding="utf-8") + os.symlink(f"host-{pid}", user_data_dir / "SingletonLock") + + +def test_running_instance_port_requires_live_pid_and_open_port(tmp_path): + listener = socket.socket() + listener.bind(("127.0.0.1", 0)) + listener.listen(1) + port = listener.getsockname()[1] + try: + _fake_running_instance(tmp_path, os.getpid(), port) + assert browser.running_instance_cdp_port(str(tmp_path)) == port + + # Both files outlive a closed Chromium: a dead pid must not be trusted. + os.unlink(tmp_path / "SingletonLock") + os.symlink("host-2147483000", tmp_path / "SingletonLock") + assert browser.running_instance_cdp_port(str(tmp_path)) is None + finally: + listener.close() + # Live pid, port no longer accepting: still not attachable. + os.unlink(tmp_path / "SingletonLock") + os.symlink(f"host-{os.getpid()}", tmp_path / "SingletonLock") + assert browser.running_instance_cdp_port(str(tmp_path)) is None + assert browser.running_instance_cdp_port(str(tmp_path / "missing")) is None + + +def test_agent_attaches_to_human_started_browser(monkeypatch): + """With a live dock instance on the shared profile the local argv carries ``--cdp ``; without one + it stays a plain ``--session`` launch.""" + from tools import browser_tool_session as session + + monkeypatch.setattr(runtime, "published_env", lambda: {"DISPLAY": ":37"}) + monkeypatch.setattr(session._cloud, "_get_browser_engine", lambda: "auto") + monkeypatch.setattr(session._cloud, "_is_headed_mode", lambda: False) + monkeypatch.setattr(session, "_agent_browser_argv", lambda cmd: [cmd]) + argvs: list = [] + + def spawn(task_id, session_info, cmd_parts, *rest): + argvs.append(cmd_parts) + return {"success": True} + + monkeypatch.setattr(session, "_spawn_and_collect", spawn) + monkeypatch.setattr(session._lp, "_lightpanda_fallback_reason", lambda *a: None) + info = {"session_name": "h_abc", "cdp_url": None} + + monkeypatch.setattr(browser, "running_instance_cdp_port", lambda d, **kw: 41234) + session._run_browser_command_unfenced("t", "open", ["https://x"], 10, None, "agent-browser", info) + assert argvs[-1][:5] == ["agent-browser", "--session", "h_abc", "--cdp", "41234"] + + monkeypatch.setattr(browser, "running_instance_cdp_port", lambda d, **kw: None) + session._run_browser_command_unfenced("t", "open", ["https://x"], 10, None, "agent-browser", info) + assert "--cdp" not in argvs[-1] and argvs[-1][:3] == ["agent-browser", "--session", "h_abc"] diff --git a/tools/bot_desktop/browser.py b/tools/bot_desktop/browser.py index a3fd382e07..08d7489ee3 100644 --- a/tools/bot_desktop/browser.py +++ b/tools/bot_desktop/browser.py @@ -3,7 +3,10 @@ The agent drives Chromium through agent-browser; a human who takes over clicks the dock's Browser icon. Both must be THE SAME browser — same binary, same ``--user-data-dir`` — or the human logs in to a jar the bot never sees. Chromium's singleton makes a second launch on the same user-data-dir -open a window in the running instance, which is exactly the hand-over we want. +open a window in the running instance, which is exactly the hand-over we want — in ONE direction. When the +human's dock instance is already up, agent-browser's own launch is forwarded to it and dies without a +DevTools endpoint, so the dock exposes a debugging port and the agent ATTACHES to it (see +:func:`running_instance_cdp_port`) instead of launching. """ from __future__ import annotations @@ -11,6 +14,7 @@ from __future__ import annotations import glob import os import shutil +import socket from pathlib import Path from typing import Optional, Tuple @@ -49,6 +53,65 @@ def dock_launch() -> Optional[Tuple[str, str]]: return (exe, str(profile_dir())) if exe else None +def dock_command(exe: str, user_data_dir: str) -> str: + """Shell line the dock's Browser icon runs. ``--remote-debugging-port=0`` makes a human-started + instance attachable (Chromium writes the chosen port to ``/DevToolsActivePort``); + first-run / default-browser dialogs would sit between the human and the bot's tabs.""" + return f"{exe} --user-data-dir={user_data_dir} --remote-debugging-port=0 --no-first-run --no-default-browser-check" + + +def running_instance_cdp_port(user_data_dir: str, *, exclude_session: Optional[str] = None) -> Optional[int]: + """DevTools port of a Chromium currently running on ``user_data_dir``, or ``None``. + + Both files outlive a crashed or closed Chromium: ``SingletonLock`` is a symlink to ``host-pid`` and + ``DevToolsActivePort`` keeps the last port, so the pid must be alive AND the port must accept a + connection before it is trusted. An instance agent-browser launched for ``exclude_session`` itself is + reported as ``None``: its daemon already owns that browser, and handing it ``--cdp`` would make it + close the browser as a config change and then attach to the port that just died with it. + """ + try: + with open(os.path.join(user_data_dir, "DevToolsActivePort"), encoding="utf-8") as fh: + port_line = fh.readline().strip() + target = os.readlink(os.path.join(user_data_dir, "SingletonLock")) + except OSError: + return None + _host, _, pid_text = target.rpartition("-") + if not (port_line.isdigit() and pid_text.isdigit()) or not _pid_alive(int(pid_text)): + return None + if exclude_session and _launched_by_session(int(pid_text)) == exclude_session: + return None + port = int(port_line) + try: + with socket.create_connection(("127.0.0.1", port), timeout=0.5): + pass + except OSError: + return None + return port + + +def _launched_by_session(chromium_pid: int) -> Optional[str]: + """``AGENT_BROWSER_SESSION`` of the agent-browser daemon that spawned ``chromium_pid``, or ``None`` + for a human-started (dock) instance. Chromium itself gets a scrubbed environment, so the daemon's + ``/proc//environ`` is the marker (Linux-only, same user).""" + try: + with open(f"/proc/{chromium_pid}/status", encoding="utf-8") as fh: + ppid = next((int(line.split()[1]) for line in fh if line.startswith("PPid:")), 0) + with open(f"/proc/{ppid}/environ", "rb") as fh: + raw = fh.read() + except (OSError, ValueError): + return None + for item in raw.split(b"\0"): + key, sep, value = item.partition(b"=") + if sep and key == b"AGENT_BROWSER_SESSION": + return value.decode("utf-8", "replace") or None + return None + + +def _pid_alive(pid: int) -> bool: + import psutil + return psutil.pid_exists(pid) + + def env_for_agent(env: dict) -> dict: """Pin agent-browser to the screen's browser identity unless the user pinned their own.""" env.setdefault("AGENT_BROWSER_PROFILE", str(profile_dir())) diff --git a/tools/bot_desktop/runtime.py b/tools/bot_desktop/runtime.py index 16e4569bfb..a9ce1923f0 100644 --- a/tools/bot_desktop/runtime.py +++ b/tools/bot_desktop/runtime.py @@ -327,10 +327,9 @@ def _spawn_and_wait(sd: Path, num: int, wait_seconds: float) -> DesktopStatus: "HERMES_BD_CONFIG_HOME": str(sd / "xdg"), "HERMES_BD_GEOMETRY": geometry(), }) - from tools.bot_desktop.browser import dock_launch + from tools.bot_desktop.browser import dock_command, dock_launch if (browser := dock_launch()) is not None: - # first-run / default-browser dialogs would sit between the human and the bot's tabs - child_env["HERMES_BD_BROWSER_EXEC"] = f"{browser[0]} --user-data-dir={browser[1]} --no-first-run --no-default-browser-check" + child_env["HERMES_BD_BROWSER_EXEC"] = dock_command(*browser) # Truncated per start: the log is a diagnostic for THIS launch, and nothing rotates it otherwise. log = open(sd / "launcher.log", "wb") # noqa: SIM115 — handed to the child, closed by it proc = subprocess.Popen( # windows-footgun: ok — Linux-only runtime (is_supported_host) diff --git a/tools/browser_tool_session.py b/tools/browser_tool_session.py index 5744915923..e762d19107 100644 --- a/tools/browser_tool_session.py +++ b/tools/browser_tool_session.py @@ -604,6 +604,15 @@ def _shares_bot_desktop_browser(session_info: Dict[str, Any]) -> bool: return bool(_bd_runtime.published_env().get("DISPLAY")) or _bd_lease.human_holds() +def _bot_desktop_attach_port(session_info: Dict[str, Any]) -> Optional[int]: + """DevTools port of a human-started Chromium on the Bot Desktop's shared profile, else ``None``.""" + if not _shares_bot_desktop_browser(session_info): + return None + from tools.bot_desktop import browser as _bd_browser + return _bd_browser.running_instance_cdp_port(str(_bd_browser.profile_dir()), + exclude_session=session_info["session_name"]) + + def _run_browser_command_unfenced(task_id: str, command: str, args: List[str], timeout: int, _engine_override: Optional[str], browser_cmd, session_info: Dict[str, Any]) -> Dict[str, Any]: # Cleanup stops the supervisor before closing the backend; keep it stopped. @@ -619,6 +628,12 @@ def _run_browser_command_unfenced(task_id: str, command: str, args: List[str], t backend_args = ["--cdp", session_info["cdp_url"]] else: backend_args = ["--session", session_info["session_name"]] + if (bd_port := _bot_desktop_attach_port(session_info)) is not None: + # A Chromium already runs on the Bot Desktop's shared profile (the human clicked the dock's + # Browser first): a launch would be forwarded into it by Chromium's singleton and die without + # a DevTools endpoint, so the session's daemon attaches to the port it advertises instead. + # Same daemon (keyed by --session) either way, so snapshot refs stay valid across commands. + backend_args += ["--cdp", str(bd_port)] if _cloud._is_headed_mode(): backend_args.append("--headed") if engine != "auto" and not _bt._is_camofox_mode():