fix(bot-screen): agent attaches to a human-started dock Browser instead of dying on the profile singleton
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 <user-data-dir>/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 <port> 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)
This commit is contained in:
@@ -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 <port>``; 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"]
|
||||
|
||||
@@ -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 ``<user-data-dir>/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/<ppid>/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()))
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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():
|
||||
|
||||
Reference in New Issue
Block a user