From 71cebc63488c017635994bd92d5a5c98f08acd84 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sun, 13 Sep 2026 13:52:00 -0700 Subject: [PATCH] fix(tools): browser_exec and computer_use caches are namespaced by the served profile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both process-global caches were keyed by the caller's session/task id alone, so under gateway.multiplex_profiles two profiles using the same id — a shared `browser_exec session=` name, or two Hermes sessions whose screens report the same DISPLAY — resolved to the FIRST profile's cloud browser / cua-driver, and a command issued in one bot's chat could act on another bot's screen. The key now carries the routed profile's home key whenever a served-profile scope is active (`get_hermes_home_override()` set), the same shape `tools/approval.py::_baseline_key` and the camofox/cloud caches already use; outside a scope every key is byte-identical to before. The computer_use lookup, install and release paths all go through one `_scoped_sid`, so a release under profile B never stops profile A's driver; approval-bypass state keeps the bare session id. Fixes #110032 (report by @wolfyy970, from @vandaimer's manual test on #108914). --- ...browser_computer_use_profile_cache_keys.py | 91 +++++++++++++++++++ tools/browser_use_cli.py | 14 ++- tools/computer_use/tool.py | 15 ++- 3 files changed, 115 insertions(+), 5 deletions(-) create mode 100644 tests/tools/test_browser_computer_use_profile_cache_keys.py diff --git a/tests/tools/test_browser_computer_use_profile_cache_keys.py b/tests/tools/test_browser_computer_use_profile_cache_keys.py new file mode 100644 index 0000000000..8e87b02cd7 --- /dev/null +++ b/tests/tools/test_browser_computer_use_profile_cache_keys.py @@ -0,0 +1,91 @@ +"""Browser-exec and computer_use backend caches are namespaced by the served profile. + +Regression for #110032: both process-global caches were keyed by the caller's session/task id +alone, so under gateway.multiplex_profiles two profiles using the same id (``"default"``, a shared +named browser session, a matching DISPLAY) resolved to the FIRST profile's browser / cua-driver. +Outside a served-profile scope every key stays byte-identical to the legacy shape.""" + +from __future__ import annotations + +import pytest + +from hermes_constants import reset_hermes_home_override, set_hermes_home_override + + +@pytest.fixture +def two_homes(tmp_path): + a = tmp_path / "profiles" / "a" + b = tmp_path / "profiles" / "b" + a.mkdir(parents=True) + b.mkdir(parents=True) + return a, b + + +def _under(home): + return set_hermes_home_override(str(home)) + + +def test_browser_exec_cache_key_differs_per_served_profile_and_is_legacy_when_unscoped(two_homes): + import tools.browser_use_cli as bu + + a, b = two_homes + assert bu._backend_cache_key("t1", "work") == "bu-named-work" + assert bu._backend_cache_key(None) == "browser-exec-default" + tok = _under(a) + try: + key_a = bu._backend_cache_key("t1", "work") + finally: + reset_hermes_home_override(tok) + tok = _under(b) + try: + key_b = bu._backend_cache_key("t1", "work") + key_b_again = bu._backend_cache_key("t1", "work") + finally: + reset_hermes_home_override(tok) + assert key_a != key_b and key_b == key_b_again + assert key_a.startswith("bu-named-work") and key_b.startswith("bu-named-work") + + +def test_computer_use_backend_not_shared_across_profiles_and_release_finds_it(two_homes, monkeypatch): + import tools.computer_use.tool as cu + + a, b = two_homes + created = [] + + class _Backend: + def __init__(self): + self.stopped = False + created.append(self) + + def start(self): + pass + + def stop(self): + self.stopped = True + + monkeypatch.setattr(cu, "_new_backend", lambda mode: _Backend()) + monkeypatch.setattr(cu, "_cua_permission_mode", lambda sid: "standard") + with cu._backend_lock: + cu._backends.clear(), cu._backend_call_locks.clear(), cu._backend_permission_modes.clear() + + tok = _under(a) + try: + backend_a = cu._get_backend("shared") + assert cu._get_backend("shared") is backend_a + finally: + reset_hermes_home_override(tok) + tok = _under(b) + try: + backend_b = cu._get_backend("shared") + assert backend_b is not backend_a + assert cu.release_computer_use_session("shared") is True # releases B's, not A's + assert backend_b.stopped and not backend_a.stopped + finally: + reset_hermes_home_override(tok) + tok = _under(a) + try: + assert cu._get_backend("shared") is backend_a # A's entry survived B's release + finally: + reset_hermes_home_override(tok) + with cu._backend_lock: + cu._backends.clear(), cu._backend_call_locks.clear(), cu._backend_permission_modes.clear() diff --git a/tools/browser_use_cli.py b/tools/browser_use_cli.py index 99fddb994a..a52ac1b0bb 100644 --- a/tools/browser_use_cli.py +++ b/tools/browser_use_cli.py @@ -356,9 +356,19 @@ def _native_screenshot_result(result: Dict[str, Any], path: str) -> Optional[Dic return None +def _served_profile_tag() -> str: + """``""`` outside a served-profile scope (every legacy key stays byte-identical); under a + multiplexed turn, the routed profile's home key — one profile's browser must never be handed + to another that happens to use the same session name or task id (#110032).""" + from hermes_constants import get_hermes_home_override, hermes_home_key + return "" if get_hermes_home_override() is None else hermes_home_key() + + def _backend_cache_key(task_id: Optional[str], session_name: str = "") -> str: - """Session-cache key for a backend browser: named sessions get their own.""" - return f"bu-named-{session_name}" if session_name else (task_id or "browser-exec-default") + """Session-cache key for a backend browser: named sessions get their own; served profiles get their own.""" + key = f"bu-named-{session_name}" if session_name else (task_id or "browser-exec-default") + tag = _served_profile_tag() + return f"{key}@{tag}" if tag else key def _resolve_lightpanda_cdp(env: dict, task_id: Optional[str], session_name: str = "") -> Optional[str]: diff --git a/tools/computer_use/tool.py b/tools/computer_use/tool.py index d55de0f15b..c293e66bdb 100644 --- a/tools/computer_use/tool.py +++ b/tools/computer_use/tool.py @@ -156,12 +156,21 @@ def _stop_backend(backend: ComputerUseBackend, call_lock: Optional[threading.RLo except Exception as e: on_error(e) -def _get_backend(session_id: str = "") -> ComputerUseBackend: +def _scoped_sid(session_id: str) -> str: + """Cache key for one Hermes session's backend. Outside a served-profile scope it is the bare id + (legacy keys byte-identical); under a multiplexed turn the routed profile's home key is appended + so two profiles that share a session id (or a DISPLAY) never share one cua-driver (#110032). + Every cache path — lookup, install, release — goes through this, so release finds what lookup made.""" + from hermes_constants import get_hermes_home_override, hermes_home_key sid = str(session_id or "") + return sid if get_hermes_home_override() is None else f"{sid}@{hermes_home_key()}" + +def _get_backend(session_id: str = "") -> ComputerUseBackend: + bare_sid, sid = str(session_id or ""), _scoped_sid(session_id) while True: with _backend_lock: # Mode resolved under the cache lock; YOLO mutation never holds the approval lock while releasing it. - permission_mode = _cua_permission_mode(sid) + permission_mode = _cua_permission_mode(bare_sid) # approval state is keyed by the Hermes session id if sid == "" and _backend is not None and sid not in _backends: _install_backend(sid, _backend, permission_mode) # fold the injection hook into the cache if (cached := _backends.get(sid)) is None: @@ -178,7 +187,7 @@ def release_computer_use_session(session_id: str) -> bool: """Release one session-owned backend (lifecycle seam for hosts/plugins); idempotent, True iff one was released. Cache entries are removed BEFORE stopping so new lookups cannot retain the stale target/ref namespace. Approval grants are not touched here: they live in the shared store and die with ``tools.approval.clear_session``.""" - sid = str(session_id or "") + sid = _scoped_sid(session_id) with _backend_lock: backend, call_lock = _detach_locked(sid) if backend is None: