fix(tools): browser_exec and computer_use caches are namespaced by the served profile
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).
This commit is contained in:
@@ -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()
|
||||
@@ -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]:
|
||||
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user