diff --git a/docs-site/docs/browser-collaboration.md b/docs-site/docs/browser-collaboration.md index 47bc53dd7..3d95c9b3d 100644 --- a/docs-site/docs/browser-collaboration.md +++ b/docs-site/docs/browser-collaboration.md @@ -80,9 +80,11 @@ It takes the current unowned tab or gets a fresh one. Subsequent calls stay bound to that tab even if the panel displays another one. An agent action brings its tab forward; a read alone does not. Another owner's -tab is marked held and cannot be activated or closed by that agent. Bindings -expire after ten idle minutes or when the tab closes. User panel interactions -are not restricted to a model's owner binding. +tab is marked held and cannot be activated or closed by that agent. A tab with +no owner is the user's and may hold a login they were asked to finish: an agent +closes it only after activating it, a switch the panel shows. Bindings expire +after ten idle minutes or when the tab closes. User panel interactions are not +restricted to a model's owner binding. This shares one process's browser, not every browser in the deployment. External ACP/CLI processes may have their own browser state. Do not promise @@ -98,6 +100,8 @@ that an external Claude Code session, for example, uses the panel's Chromium. Check the effective [permission mode](permissions.md): `full` skips ordinary ask-tier prompts. Site-scoped consent is broader than one button, and the approval may show a ref and site rather than a human-readable element label. +An action that would open a fresh tab has no site yet: its prompt names none, +and a session grant for it covers only an identical call. Disable tools through `tools.disabledTools` or explicit permission rules when they must not be available. diff --git a/docs-site/docs/browser-collaboration.zh.md b/docs-site/docs/browser-collaboration.zh.md index 1135a000c..5cb1d10a8 100644 --- a/docs-site/docs/browser-collaboration.zh.md +++ b/docs-site/docs/browser-collaboration.zh.md @@ -67,7 +67,9 @@ Owner 是当前进程内子 Agent 运行,否则是主对话。它会取得未 或新建标签页。后续调用绑定该页,即使面板显示另一页也不改变地址。 Agent 的动作会将自己的页切到前台,单纯读取不会。其他 owner 的页标为 held, -该 Agent 不能切换到它或关闭它。绑定在空闲十分钟后或标签页关闭时失效。 +该 Agent 不能切换到它或关闭它。没有 owner 的页属于用户,可能停在刚请用户完成的 +登录上:Agent 必须先切换到它(面板可见)才能关闭。绑定在空闲十分钟后或标签页 +关闭时失效。 用户面板交互不受模型 owner 绑定限制。 共享的是同一进程的浏览器,不是部署中的所有浏览器。外部 ACP/CLI 进程可能有 @@ -82,6 +84,8 @@ Agent 的动作会将自己的页切到前台,单纯读取不会。其他 owne 检查实际[权限模式](permissions.md):`full` 跳过普通 ask-tier 提示。 站点授权比单个按钮更宽;提示可能显示 ref 和站点,而不是元素的可读标签。 +会新开标签页的动作此时还没有站点:提示中不显示站点,对它的会话授权只覆盖参数 +完全相同的调用。 必须不可用的工具应通过 `tools.disabledTools` 或明确权限规则禁用。 导航接受 HTTP(S) 和空白页,拒绝危险 scheme 和字面 link-local 目标。 diff --git a/raven/agent/tools/browser.py b/raven/agent/tools/browser.py index 9abc6e3f7..86bc02da4 100644 --- a/raven/agent/tools/browser.py +++ b/raven/agent/tools/browser.py @@ -37,6 +37,13 @@ SNAPSHOT_TEXT_CHARS = 8_000 MAX_REFS_SHOWN = 120 +# How many owners' stamps ``_acted`` keeps, most recent first. The stamps only +# answer "did the reader touch the page since this owner last acted", which the +# owners acting lately can answer as well as all of them. The driver prunes an +# owner when it drops that owner's binding; the cap is for the owners it never +# drops, so one key per delegated run cannot accumulate for the process's life. +_ACTED_MAX = 512 + # Carried by browser_navigate alone. Every tool's description is paid for on # every turn of every conversation, and the one place the model decides whether # a page is its business at all is when it opens one. @@ -50,7 +57,10 @@ # person approves for a click is the site, not the ref id -- so the site the # call will land on is written into the parameters before the gate reads them. # ``raven.permissions.builtin.session_keys`` keys a browser grant on it, and -# the approval prompt shows it. +# the approval prompt shows it. The driver's ``url_for`` predicts that page by +# the rule ``_page_for`` then binds by. When the call would open a tab of its +# own, no site describes it yet and none is written: the grant is keyed on the +# call itself, which no later call to a real site can present. def _browser(): @@ -175,7 +185,10 @@ class _BrowserTool(Tool): timeout_seconds = 90.0 # The last time this owner acted, so a readback can say whether a hand - # other than the model's touched the browser in between. + # other than the model's touched the browser in between. Bounded, and + # pruned when the driver drops the matching tab binding: the key is a + # delegated run's uid, and a run that has ended never acts again, so an + # unbounded map grows one entry per run for the life of the process. _acted: dict[str, float] = {} @staticmethod @@ -188,12 +201,36 @@ def _owner(self) -> str: return current_owner() def _mark(self, owner: str) -> None: - _BrowserTool._acted[owner] = time.monotonic() + acted = _BrowserTool._acted + # Re-inserted rather than updated, so the dict's own order is the order + # owners last acted in and the cap can evict from the front: no sort, + # and no tie to break between two stamps one clock tick apart. + acted.pop(owner, None) + acted[owner] = time.monotonic() + self._bind_to_driver() + while len(acted) > _ACTED_MAX: + acted.pop(next(iter(acted))) def _touched(self, owner: str) -> bool: last = _BrowserTool._acted.get(owner) return last is not None and _browser().touched_since(last) + @classmethod + def _forget_owner(cls, owner: str) -> None: + cls._acted.pop(owner, None) + + def _bind_to_driver(self) -> None: + """Have the driver tell the stamp map when it drops an owner's binding. + + Called from ``_mark``, the only place that adds a stamp, so no owner + can hold one before the driver knows to prune it. Not at import: reaching for + the driver is what builds the process-wide browser, and importing this + module must not construct one as a side effect. The driver is the one + place that knows when an owner's binding ends, so the map is pruned on + that event rather than on a second clock of its own. + """ + _browser().on_owner_released = _BrowserTool._forget_owner + async def _readback(self, owner: str, state: dict[str, Any], *, acted: bool) -> ToolResult: """State plus a compact snapshot; the snapshot is skipped on an error so a failure reads as one and not as a page description.""" @@ -550,7 +587,8 @@ def description(self) -> str: return ( "List, open, switch or close tabs of the shared browser. Your calls always land on your own " "tab; a tab marked held is another agent's and cannot be taken. new opens a fresh tab (with " - "an optional url) and makes it yours; activate makes an unheld tab yours." + "an optional url) and makes it yours; activate makes an unheld tab yours. close takes only a " + "tab you hold: a tab with no mark at all is the user's, and closing it needs activate first." ) @property diff --git a/raven/browser/driver.py b/raven/browser/driver.py index 7b87a8012..8c31725e1 100644 --- a/raven/browser/driver.py +++ b/raven/browser/driver.py @@ -26,6 +26,7 @@ import asyncio import base64 +import re import shlex import sys import time @@ -217,6 +218,12 @@ def __init__(self) -> None: # moment two callers are most likely to collide. Separate from # `_s.lock`, which the navigation methods take after calling `_ensure`. self._launch_lock = asyncio.Lock() + # Told the name of every owner this driver stops binding to a page, so + # whatever a caller keeps per owner can drop its copy on the same event + # instead of on a second, independently drifting clock. Held here + # rather than in `_State`, which close() replaces: the store listening + # outlives any one browser session, as the launch lock above does. + self.on_owner_released: Any = None # ---- lifecycle --------------------------------------------------------- @@ -318,7 +325,20 @@ async def _launch_once(self) -> Any: await self.close() hint = f"{exc}" if "Executable doesn't exist" in hint or "playwright install" in hint: - raise BrowserUnavailableError(f"Chromium is not installed. Run: {CHROMIUM_INSTALL_HINT}") from None + # "Not installed" is the right reading only when nothing is on + # disk. The same error comes back when a browser is installed + # where this process does not look -- Playwright resolves its + # cache from HOME, which a sandboxed run or another account + # moves -- and then the install line sends the reader to fetch + # what they already have. Playwright's error names the path it + # tried, which is the one fact that tells the two apart. + looked = re.search(r"Executable doesn't exist at (.+)", hint) + where = f" ({looked.group(1).strip()})" if looked else "" + raise BrowserUnavailableError( + f"Chromium is not installed where this process looks for it{where}; if it is " + "installed elsewhere, set PLAYWRIGHT_BROWSERS_PATH to its ms-playwright directory. " + f"Otherwise run: {CHROMIUM_INSTALL_HINT}" + ) from None raise BrowserUnavailableError(f"could not start Chromium: {hint}") from None logger.info("browser: chromium started ({}x{})", w, h) return self._s.page @@ -485,9 +505,14 @@ def _owner_live(self, rec: _Owner, now: float) -> bool: closed = True return not closed and now - rec.seen < OWNER_IDLE_S - def _reap_owners(self, now: float) -> None: - for key in [k for k, rec in self._s.owners.items() if not self._owner_live(rec, now)]: - del self._s.owners[key] + def _reap_owners(self, now: float) -> list[str]: + """The owners whose bindings have just died. Removal is the caller's. + + Names rather than removal, because every way a binding ends has to be + announced in one place: ``_drop_owners`` does the removing and the + telling, so a reap and an explicit release cannot drift apart. + """ + return [k for k, rec in self._s.owners.items() if not self._owner_live(rec, now)] def owner_of(self, page: Any) -> str | None: """Which live owner holds this page, if any.""" @@ -497,13 +522,42 @@ def owner_of(self, page: Any) -> str | None: return key return None + def _landing(self, owner: str, now: float) -> Any | None: + """The page a call for ``owner`` lands on now, or None for a new tab. + + The one rule both questions about it go through: ``_page_for`` binds + the answer and ``url_for`` reports it without binding. The permission + gate keys an acting call on that report before the call runs, so the + site a person is asked about is only the site acted on if the same + rule picks the page both times. Only live bindings count, which is the + view ``_page_for`` acts on once it has reaped. + """ + rec = self._s.owners.get(owner) + if rec is not None and self._owner_live(rec, now): + return rec.page + active = self._s.page + held = {id(r.page) for k, r in self._s.owners.items() if k != owner and self._owner_live(r, now)} + if id(active) not in held: + return active + return None + def url_for(self, owner: str | None) -> str: - """Where an owner's tab is, without starting anything or rebinding.""" - if owner is not None: - rec = self._s.owners.get(owner) - if rec is not None and self._owner_live(rec, time.monotonic()): - return rec.page.url - return self.url + """Where a call for this owner would act, without starting or binding. + + The owner's own tab while it holds a live one; otherwise the page + ``_page_for`` would hand it -- the front tab, unless another live owner + holds that -- and empty when the call would have to open a tab of its + own, which no site describes yet, or when no browser is running and the + call would start one. A prediction, made before the call: + whatever moves in between (the reader switches tabs, another owner + takes the front one, the binding idles past ``OWNER_IDLE_S`` while a + person decides) moves the call with it. The reader, ``None``, reads + the front tab. + """ + if owner is None: + return self.url + page = self._landing(owner, time.monotonic()) + return page.url if page is not None else "" async def _page_for(self, owner: str | None, *, act: bool = True) -> Any: """The page a caller works on, binding an owner on its first call. @@ -518,18 +572,16 @@ async def _page_for(self, owner: str | None, *, act: bool = True) -> Any: if act: self._s.touched = now return active - self._reap_owners(now) + self._drop_owners(self._reap_owners(now)) rec = self._s.owners.get(owner) if rec is not None: rec.seen = now page = rec.page else: - held = {id(r.page) for k, r in self._s.owners.items() if k != owner} - if id(active) not in held: - page = active - elif len(self._pages()) >= MAX_TABS: - raise BrowserBusyError(f"tab limit reached ({MAX_TABS}); close one before opening another") - else: + page = self._landing(owner, now) + if page is None: + if len(self._pages()) >= MAX_TABS: + raise BrowserBusyError(f"tab limit reached ({MAX_TABS}); close one before opening another") self._s.spawning += 1 try: page = await self._s.context.new_page() @@ -559,7 +611,25 @@ def touched_since(self, when: float) -> bool: def release(self, owner: str) -> None: """Forget an owner's binding; its tab stays open for whoever claims it next.""" - self._s.owners.pop(owner, None) + self._drop_owners([owner]) + + def _drop_owners(self, owners: list[str]) -> None: + """Remove these bindings, and tell the listener about the ones there were. + + Whatever ends a binding -- a reap, an explicit release, the tab it was + on closing, the browser closing -- goes through here, so the listener + is told once per owner under one rule rather than at each call site. An + owner that was never bound is not announced: there is nothing to have + lost. + """ + gone = [owner for owner in owners if self._s.owners.pop(owner, None) is not None] + if self.on_owner_released is None: + return + for owner in gone: + try: + self.on_owner_released(owner) + except Exception as exc: # noqa: BLE001 - a listener must not take a binding drop down + logger.debug("browser: owner-release listener failed for {}: {}", owner, exc) # ---- tabs ---------------------------------------------------------------- @@ -670,8 +740,7 @@ async def tab_close(self, index: int, *, owner: str | None = None) -> dict[str, await victim.close() except Exception: pass - for key in [k for k, rec in self._s.owners.items() if rec.page is victim]: - del self._s.owners[key] + self._drop_owners([k for k, rec in self._s.owners.items() if rec.page is victim]) rest = self._pages() if not rest: await self.close() @@ -733,6 +802,11 @@ async def close(self) -> None: await s.playwright.stop() except Exception: pass + # Announced before the state is replaced: closing ends every binding it + # held, and a per-owner store keyed against them outlives the state -- + # the pop-out flow is close-and-relaunch, and a relaunch continues the + # same owners. A fresh `_State` would otherwise strand those keys. + self._drop_owners(list(s.owners)) self._s = _State() logger.info("browser: closed") diff --git a/raven/permissions/builtin.py b/raven/permissions/builtin.py index 80ce65589..54fa3201c 100644 --- a/raven/permissions/builtin.py +++ b/raven/permissions/builtin.py @@ -110,8 +110,11 @@ def session_keys(tool_name: str, params: dict[str, Any], ask_segments: tuple[str path names another file once the directory is rebound. The browser's acting tools share one key per site: what the human approved was letting the agent work on that site, and a click and the typing that follows it are one such - piece of work, not two. Every other tool keys the exact call -- a `path` - field on an unknown tool says nothing about what its other fields do. + piece of work, not two. One that names no site -- the page it acts on has + none yet: a tab it is about to open, or one still blank -- keys the exact + call, so its grant is never presented by a call to a real site. Every other + tool keys the exact call -- a `path` field on an unknown tool says nothing + about what its other fields do. """ if tool_name == "exec" and ask_segments: working_dir = params.get("working_dir") diff --git a/tests/_browser_cache.py b/tests/_browser_cache.py new file mode 100644 index 000000000..6245ab30b --- /dev/null +++ b/tests/_browser_cache.py @@ -0,0 +1,108 @@ +"""Where Playwright keeps the Chromium a real-page test needs. + +This suite redirects ``HOME`` at a per-test tmp dir (``tests/conftest.py``, +so no test reads the config of whoever is running it), and Playwright looks +its browsers up from the home directory -- so a real-page test has to aim the +look-up back at the login's own cache before it launches anything. + +The rule mirrored here is ``registryDirectory`` in playwright-core's registry, +read from the driver bundled with playwright 1.62: + +* ``PLAYWRIGHT_BROWSERS_PATH=0`` -- a hermetic install, inside the package; +* any other value of it -- that directory, a relative one taken from + ``INIT_CWD`` or the working directory; +* otherwise ``ms-playwright`` under a per-OS cache root: ``XDG_CACHE_HOME`` + or ``~/.cache`` on Linux, ``~/Library/Caches`` on macOS, ``LOCALAPPDATA`` or + ``~/AppData/Local`` on Windows. Playwright refuses every other platform. + +Only the home directory needs correcting: the two environment variables are +not sandboxed, so where one is set Playwright already finds the cache. +""" + +from __future__ import annotations + +import os +import sys + +try: + import pwd +except ImportError: # pragma: no cover - Windows has no pwd module + pwd = None # type: ignore[assignment] + + +def login_home() -> str: + """The login directory, ignoring a ``HOME`` a fixture has sandboxed. + + ``pwd`` rather than ``Path.home()``, which follows ``HOME`` -- the value + every caller here is trying to see past. Without a passwd entry there is + nothing to see past it with, and ``HOME`` is the answer. + """ + if pwd is not None: + try: + return pwd.getpwuid(os.getuid()).pw_dir + except KeyError: + pass + return os.path.expanduser("~") + + +def playwright_cache(home: str) -> str | None: + """The default browser cache for ``home``, or None where Playwright has none.""" + if sys.platform == "linux": + root = os.environ.get("XDG_CACHE_HOME") or os.path.join(home, ".cache") + elif sys.platform == "darwin": + root = os.path.join(home, "Library", "Caches") + elif sys.platform == "win32": + root = os.environ.get("LOCALAPPDATA") or os.path.join(home, "AppData", "Local") + else: + return None + return os.path.join(root, "ms-playwright") + + +def browsers_dir() -> str | None: + """The browser cache this process resolves to. Safe to call at import.""" + explicit = os.environ.get("PLAYWRIGHT_BROWSERS_PATH") + if explicit == "0": + try: + import playwright + except ImportError: + return None + return os.path.join(os.path.dirname(playwright.__file__), "driver", "package", ".local-browsers") + if explicit: + return os.path.abspath(os.path.join(os.environ.get("INIT_CWD") or os.getcwd(), explicit)) + return playwright_cache(login_home()) + + +def chromium_installed(where: str | None = None) -> bool: + """Whether a Chromium build sits in the browser cache. + + A directory entry rather than ``Browser.probe()``, which only answers + whether the ``playwright`` package imports: skipping on the package alone + turned a missing binary into failures. It does not check the revision + -- a cache holding only an older build still fails the launch, loudly, and + the driver's error then names the path it looked for. + """ + target = where or browsers_dir() + if target is None: + return False + try: + return any(name.startswith("chromium") for name in os.listdir(target)) + except OSError: + return False + + +def point_at_login_cache(monkeypatch) -> None: + """Fixture body: aim Playwright at the login's cache, unless already aimed.""" + if os.environ.get("PLAYWRIGHT_BROWSERS_PATH"): + return + cache = playwright_cache(login_home()) + if cache is not None: + monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", cache) + + +__all__ = [ + "browsers_dir", + "chromium_installed", + "login_home", + "playwright_cache", + "point_at_login_cache", +] diff --git a/tests/integration/test_browser_real_web.py b/tests/integration/test_browser_real_web.py index dcc969e21..965d15f13 100644 --- a/tests/integration/test_browser_real_web.py +++ b/tests/integration/test_browser_real_web.py @@ -12,15 +12,19 @@ from __future__ import annotations -import os -import pwd - import pytest from raven.browser import get_browser from raven.browser.driver import Browser +from tests._browser_cache import chromium_installed, point_at_login_cache -pytestmark = pytest.mark.skipif(not Browser.probe()[0], reason="browser extra not installed") +# Both halves matter: the package can import with no browser downloaded, and a +# skip that only asks about the package reports that as six failures rather +# than as a skip. The install line above is what the second half is about. +pytestmark = pytest.mark.skipif( + not Browser.probe()[0] or not chromium_installed(), + reason="playwright or its Chromium is not installed (see the module docstring)", +) @pytest.fixture @@ -136,9 +140,7 @@ async def test_chromium_starts_only_on_demand(browser) -> None: @pytest.fixture(autouse=True) -def _browsers_from_the_real_home(monkeypatch): +def _browsers_from_the_real_home(monkeypatch: pytest.MonkeyPatch) -> None: """The suite redirects HOME to a temp dir; playwright keeps its browsers under the real one. Point it there unless the caller already did.""" - if not os.environ.get("PLAYWRIGHT_BROWSERS_PATH"): - real_home = pwd.getpwuid(os.getuid()).pw_dir - monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", os.path.join(real_home, "Library", "Caches", "ms-playwright")) + point_at_login_cache(monkeypatch) diff --git a/tests/integration/test_browser_tools_real_web.py b/tests/integration/test_browser_tools_real_web.py index 215d85ee7..62b08591c 100644 --- a/tests/integration/test_browser_tools_real_web.py +++ b/tests/integration/test_browser_tools_real_web.py @@ -16,7 +16,6 @@ import http.server import os -import sys import threading from collections.abc import Iterator from pathlib import Path @@ -34,8 +33,15 @@ ) from raven.browser import get_browser from raven.browser.driver import Browser - -pytestmark = pytest.mark.skipif(not Browser.probe()[0], reason="browser extra not installed") +from tests._browser_cache import chromium_installed, point_at_login_cache + +# Both halves matter: the package can import with no browser downloaded, and a +# skip that only asks about the package reports that as failures rather than as +# a skip. +pytestmark = pytest.mark.skipif( + not Browser.probe()[0] or not chromium_installed(), + reason="playwright or its Chromium is not installed (see test_browser_real_web.py)", +) FORM = b"""Raven Form Test

Sign up

@@ -66,28 +72,6 @@ def log_message(self, *args: object) -> None: pass -try: - import pwd -except ImportError: # pragma: no cover - Windows has no pwd module - pwd = None # type: ignore[assignment] - - -def _playwright_cache(home: str) -> str: - """Where ``playwright install`` put the browsers, by platform. - - Playwright picks this directory by OS, and this fixture only has to name - the same one: hard-coded to macOS it sent a Linux run looking in - ``~/Library/Caches``, which the documented install never writes -- the - suite then reported Chromium missing and suggested an install that would - write somewhere else again. - """ - if sys.platform == "darwin": - return os.path.join(home, "Library", "Caches", "ms-playwright") - if sys.platform == "win32": - return os.path.join(home, "AppData", "Local", "ms-playwright") - return os.path.join(home, ".cache", "ms-playwright") - - @pytest.fixture(autouse=True) def _browsers_from_the_real_home(monkeypatch: pytest.MonkeyPatch) -> None: """Point Playwright at the human's cache rather than a sandboxed HOME. @@ -95,13 +79,7 @@ def _browsers_from_the_real_home(monkeypatch: pytest.MonkeyPatch) -> None: An explicit ``PLAYWRIGHT_BROWSERS_PATH`` always wins, so CI that installs browsers elsewhere is untouched. """ - if os.environ.get("PLAYWRIGHT_BROWSERS_PATH"): - return - try: - home = pwd.getpwuid(os.getuid()).pw_dir if pwd is not None else os.path.expanduser("~") - except (KeyError, AttributeError): - home = os.path.expanduser("~") - monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", _playwright_cache(home)) + point_at_login_cache(monkeypatch) @pytest.fixture diff --git a/tests/test_browser_cache.py b/tests/test_browser_cache.py new file mode 100644 index 000000000..bc8082d94 --- /dev/null +++ b/tests/test_browser_cache.py @@ -0,0 +1,125 @@ +"""The Playwright browser-cache look-up the real-page suites share. + +Those suites need the cache Playwright would use for the *login's* home: the +suite redirects ``HOME`` at a per-test tmp dir, and Playwright resolves the +cache from the home directory. Getting it wrong is quiet -- the run reports +Chromium missing and suggests an install that writes somewhere else again -- +so the cases below are the branches of Playwright's own rule, not a list of +the platforms someone happened to test on. +""" + +from __future__ import annotations + +import os +import sys + +import pytest + +from tests._browser_cache import ( + browsers_dir, + chromium_installed, + login_home, + playwright_cache, + point_at_login_cache, +) + + +@pytest.mark.parametrize( + ("platform", "env", "expected"), + [ + pytest.param("linux", {}, ("", ".cache", "ms-playwright"), id="linux"), + pytest.param("linux", {"XDG_CACHE_HOME": "/xdg"}, ("/xdg", "ms-playwright"), id="linux-xdg"), + pytest.param("darwin", {}, ("", "Library", "Caches", "ms-playwright"), id="darwin"), + pytest.param("win32", {}, ("", "AppData", "Local", "ms-playwright"), id="win32"), + pytest.param("win32", {"LOCALAPPDATA": "/lad"}, ("/lad", "ms-playwright"), id="win32-localappdata"), + pytest.param("sunos5", {}, None, id="a-platform-playwright-refuses"), + ], +) +def test_the_default_cache_follows_playwright_s_rule( + monkeypatch: pytest.MonkeyPatch, platform: str, env: dict[str, str], expected: tuple[str, ...] | None +) -> None: + monkeypatch.setattr(sys, "platform", platform) + for name in ("XDG_CACHE_HOME", "LOCALAPPDATA"): + monkeypatch.delenv(name, raising=False) + for name, value in env.items(): + monkeypatch.setenv(name, value) + + got = playwright_cache("/h") + + if expected is None: + assert got is None + else: + assert got == os.path.join(*("/h" if part == "" else part for part in expected)) + + +def test_an_explicit_cache_path_wins(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", "/elsewhere/ms-playwright") + + assert browsers_dir() == os.path.abspath("/elsewhere/ms-playwright") + + +def test_a_relative_cache_path_is_taken_from_init_cwd_or_the_working_directory( + monkeypatch: pytest.MonkeyPatch, tmp_path +) -> None: + monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", os.path.join("rel", "pw")) + monkeypatch.delenv("INIT_CWD", raising=False) + monkeypatch.chdir(tmp_path) + assert browsers_dir() == str(tmp_path / "rel" / "pw") + + monkeypatch.setenv("INIT_CWD", str(tmp_path / "init")) + assert browsers_dir() == str(tmp_path / "init" / "rel" / "pw") + + +def test_a_hermetic_install_is_looked_for_inside_the_package(monkeypatch: pytest.MonkeyPatch) -> None: + playwright = pytest.importorskip("playwright") + monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", "0") + + assert browsers_dir() == os.path.join( + os.path.dirname(playwright.__file__), "driver", "package", ".local-browsers" + ), "a literal '0' is a directory name to nothing but Playwright" + + +def test_the_login_home_sees_past_a_sandboxed_home(monkeypatch: pytest.MonkeyPatch) -> None: + pwd = pytest.importorskip("pwd") + try: + real = pwd.getpwuid(os.getuid()).pw_dir + except KeyError: + pytest.skip("this account has no passwd entry, so HOME is all there is to read") + monkeypatch.setenv("HOME", "/tmp/sandboxed") + + assert login_home() == real + + +def test_the_fixture_aims_playwright_at_the_login_cache_unless_already_aimed( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.delenv("PLAYWRIGHT_BROWSERS_PATH", raising=False) + point_at_login_cache(monkeypatch) + assert os.environ["PLAYWRIGHT_BROWSERS_PATH"] == playwright_cache(login_home()) + + monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", "/chosen") + point_at_login_cache(monkeypatch) + assert os.environ["PLAYWRIGHT_BROWSERS_PATH"] == "/chosen", "a caller's own choice is left alone" + + +def test_a_chromium_directory_is_what_counts_as_installed(tmp_path) -> None: + """The old skip asked ``Browser.probe()``, which only tests the package, + so a missing binary failed the real-page tests instead of skipping them.""" + cache = tmp_path / "ms-playwright" + cache.mkdir() + (cache / "ffmpeg-1011").mkdir() + assert chromium_installed(str(cache)) is False, "another download is not a browser" + + (cache / "chromium_headless_shell-1234").mkdir() + assert chromium_installed(str(cache)) is True + + assert chromium_installed(str(tmp_path / "absent")) is False + + +def test_a_platform_playwright_refuses_counts_as_not_installed(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(sys, "platform", "sunos5") + monkeypatch.delenv("PLAYWRIGHT_BROWSERS_PATH", raising=False) + + assert chromium_installed() is False + point_at_login_cache(monkeypatch) + assert "PLAYWRIGHT_BROWSERS_PATH" not in os.environ diff --git a/tests/test_browser_driver.py b/tests/test_browser_driver.py index 49dd1fddc..facaa785d 100644 --- a/tests/test_browser_driver.py +++ b/tests/test_browser_driver.py @@ -349,8 +349,15 @@ async def start(self) -> None: await b._ensure() why = str(exc.value) - assert "Chromium is not installed" in why - assert f"{shlex.quote(sys.executable)} -m playwright install chromium" in why + assert "Chromium is not installed where this process looks for it" in why + assert why.endswith(f"{shlex.quote(sys.executable)} -m playwright install chromium"), "the fix closes the line" + # The path Playwright searched, quoted back, and the knob that moves the + # search: without them the message reads as "not installed" whichever of + # the two it was, and a sandboxed HOME sends the reader to download a + # browser they already have. + assert "(/nowhere/chrome)" in why + assert "PLAYWRIGHT_BROWSERS_PATH" in why + assert "\n" not in why, "the panel shows this in one block, where a newline collapses to a space" # ── tabs ──────────────────────────────────────────────────────────────── @@ -380,6 +387,11 @@ class _FakeContext: def __init__(self, pages: list[Any]) -> None: self.pages = pages + async def new_page(self) -> _FakePage: + page = _FakePage("about:blank", f"tab{len(self.pages)}") + self.pages.append(page) + return page + def _with_pages(b: Browser, pages: list[_FakePage], active: int = 0) -> None: b._s.context = _FakeContext(pages) @@ -947,3 +959,100 @@ async def new_page(self) -> Any: assert b._s.page is page and page is not held assert streams == ["restream"], "an act fronts the new tab exactly once" + + +@pytest.mark.parametrize( + ("bindings", "lands_on"), + [ + pytest.param({}, "front", id="front-tab-is-the-readers"), + pytest.param({"run:other": ("front", 0)}, "new", id="front-tab-held-by-another-owner"), + pytest.param({"run:x": ("mine", 0)}, "mine", id="caller-holds-its-own-tab"), + pytest.param({"run:x": ("mine", "idle")}, "front", id="caller-idled-out-front-tab-free"), + pytest.param( + {"run:x": ("mine", "idle"), "run:other": ("front", 0)}, "new", id="caller-idled-out-front-tab-held" + ), + pytest.param({"run:x": ("mine", "closed")}, "front", id="callers-tab-was-closed"), + pytest.param({"run:other": ("front", "idle")}, "front", id="front-tab-held-by-an-idled-out-owner"), + ], +) +async def test_the_url_reported_for_an_owner_is_where_its_call_lands( + bindings: dict[str, tuple[str, Any]], lands_on: str +) -> None: + """The permission gate keys an acting call on ``url_for`` before the call + runs, so the site a person approves is the site acted on only if this + report and ``_page_for`` pick the same page. One case per branch of that + choice; a call that must open a tab of its own is reported empty, since no + site describes a page nobody has opened yet.""" + b = get_browser() + pages = {"mine": _FakePage("https://mine.test/"), "front": _FakePage("https://bank.test/")} + _driving(b, [pages["mine"], pages["front"]], active=1) + b._wire = lambda page: None # type: ignore[method-assign] + for owner, (name, age) in bindings.items(): + if age == "closed": + pages[name]._closed = True + seen = time.monotonic() - (driver_module.OWNER_IDLE_S + 1 if age == "idle" else 0) + b._s.owners[owner] = _Owner(pages[name], seen) + before = set(map(id, pages.values())) + + reported = b.url_for("run:x") + landed = await b._page_for("run:x") + + if lands_on == "new": + assert id(landed) not in before, "the call opened a tab of its own" + assert reported == "" + else: + assert landed is pages[lands_on] + assert reported == landed.url + + +async def test_a_binding_ends_names_the_owner_to_the_listener_however_it_ended() -> None: + """A per-owner store outside the driver is told on the same event the driver + drops the binding. Four endings, and the set is the driver's own removal + sites rather than a list: an explicit release, a reap, the owner's tab + closing, and the whole browser closing. An owner that was never bound is + not announced -- there is nothing to have lost.""" + b = get_browser() + first, second, third = (_FakePage(f"https://{n}.test/") for n in ("a", "b", "c")) + _driving(b, [first, second, third]) + b._wire = lambda page: None # type: ignore[method-assign] + seen: list[str] = [] + b.on_owner_released = seen.append + + b.release("never-bound") + assert seen == [], "nothing to announce" + + b._s.owners["run:a"] = _Owner(first, time.monotonic()) + b.release("run:a") + assert seen == ["run:a"] + + seen.clear() + b._s.owners["run:b"] = _Owner(second, time.monotonic() - driver_module.OWNER_IDLE_S - 1) + await b._page_for("run:c", act=False) + assert seen == ["run:b"], "a reaped binding is announced, and only it" + + seen.clear() + b._s.owners["run:c"] = _Owner(third, time.monotonic()) + await b.tab_close(2, owner=None) + assert seen == ["run:c"], "closing the tab an owner held announces it" + + seen.clear() + b._s.owners["run:d"] = _Owner(first, time.monotonic()) + await b.close() + assert seen == ["run:d"], "closing the browser ends the bindings it still held" + + +async def test_a_listener_that_raises_does_not_stop_the_binding_from_ending() -> None: + """The listener is a caller's bookkeeping; a bug in it must not leave the + driver holding a binding it has decided to drop.""" + b = get_browser() + page = _FakePage("https://a.test/") + _driving(b, [page]) + b._s.owners["run:a"] = _Owner(page, time.monotonic()) + + def explode(owner: str) -> None: + raise RuntimeError("listener is broken") + + b.on_owner_released = explode + b.release("run:a") + + assert "run:a" not in b._s.owners diff --git a/tests/test_browser_tools.py b/tests/test_browser_tools.py index e82d0c359..f04737198 100644 --- a/tests/test_browser_tools.py +++ b/tests/test_browser_tools.py @@ -164,6 +164,29 @@ def test_acting_tools_write_the_site_into_the_call(monkeypatch: pytest.MonkeyPat assert BrowserClickTool().cast_params({"ref": "ref_2"})["site"] == "shop.example.com" +def test_a_call_that_would_open_its_own_tab_carries_no_site(monkeypatch: pytest.MonkeyPatch) -> None: + """With the front tab another owner's, this owner's first call opens a tab + of its own, so no site describes where it acts yet. None is written -- not + the front tab's, and not one the model supplied -- and the grant is then + keyed on the call itself.""" + b = get_browser() + held = _FakePage("https://bank.test/login") + _running(b, [held]) + b._s.owners["session:parent"] = _Owner(held, time.monotonic()) + monkeypatch.setattr(tools_mod, "current_owner", lambda: "run:child") + + out = BrowserPressTool().cast_params({"key": "Enter", "site": "attacker.test"}) + + assert out == {"key": "Enter"}, "neither the panel's site nor the model's own" + from raven.permissions.builtin import BROWSER_SITE_KEYED_TOOLS, action_digest, session_keys + + assert "browser_press" in BROWSER_SITE_KEYED_TOOLS + assert session_keys("browser_press", out) == (action_digest("browser_press", out),), ( + "no site means the grant is this call's own, so a later click on bank.test " + "presents a different key and a site nobody approved cannot be banked" + ) + + def test_no_page_means_no_site() -> None: assert "site" not in BrowserTypeTool().cast_params({"text": "hi"}) @@ -746,3 +769,58 @@ def test_site_keyed_permission_set_matches_the_acting_tool_hierarchy() -> None: acting = {cls().name for cls in tools_mod._ActingTool.__subclasses__()} assert acting == set(BROWSER_SITE_KEYED_TOOLS) + + +def test_the_stamp_store_is_pruned_by_the_driver_once_it_holds_an_entry( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The store registers with the driver from the only place that adds a + stamp, so an owner that holds one is an owner the driver will prune -- and + not at import, where reaching for the driver would build the process-wide + browser. This drives the release a run's end would cause and asserts the + stamp is gone; an unregistered store would keep it for the life of the + process.""" + b = get_browser() + page = _FakePage("https://a.test/") + _running(b, [page]) + assert b.on_owner_released is None, "nothing has reached the driver yet" + + BrowserPressTool()._mark("run:r1") + b._s.owners["run:r1"] = _Owner(page, time.monotonic()) + + assert b.on_owner_released == tools_mod._BrowserTool._forget_owner + b.release("run:r1") + assert "run:r1" not in tools_mod._BrowserTool._acted + + +def test_the_stamp_store_keeps_the_owners_that_acted_most_recently(monkeypatch: pytest.MonkeyPatch) -> None: + """An owner the driver never drops is still evicted once the store is + full, least recent first, and acting again moves an owner to the back.""" + monkeypatch.setattr(tools_mod, "_ACTED_MAX", 3) + _running(get_browser(), [_FakePage("https://a.test/")]) + tool = BrowserPressTool() + + for owner in ("run:a", "run:b", "run:c"): + tool._mark(owner) + tool._mark("run:a") + tool._mark("run:d") + + assert list(tools_mod._BrowserTool._acted) == ["run:c", "run:a", "run:d"] + + +async def test_the_tabs_description_names_the_close_rule_the_driver_enforces() -> None: + """The driver refuses an owner's close of a tab nobody holds -- the + reader's, which may carry a login the model just asked them to finish -- + and the listing marks that tab with neither ``yours`` nor ``held``. The + description is what the model reads before it calls; without the rule + there, the refusal is the first it hears of it.""" + b = get_browser() + _running(b, [_FakePage("https://bank.test/")]) + + refused = await b.tab_close(0, owner="run:x") + + assert "activate it first" in refused["error"], "the driver's own recovery, which the description must match" + said = " ".join(BrowserTabsTool().description.split()) + assert "close takes only a tab you hold" in said + assert "a tab with no mark at all is the user's" in said + assert "closing it needs activate first" in said