From d4d42a14928b30f2eea531aa042e55d116ded1a5 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Wed, 11 Nov 2026 16:31:19 -0800 Subject: [PATCH 1/3] feat(browser): the action log credits Playwright MCP's commands to the browser_* call they serve (FD-5503) browser/v1 gains calls/begin and calls/end, which a driver sends around an agent's tool call it serves through a CDP user's connections. The relay credits that user's commands to the call open at the time, or to every one open when calls overlap, as for, and the tab events they cause carry it too. The call's entry takes the tab its commands acted on and keeps what it typed only once each typing command of its own found no password field focused. The page driver brackets each browser_* call so. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 +- packages/agentenv-protocol/README.md | 14 ++- .../agentenv_protocol/website_browser/cdp.py | 79 ++++++++++-- .../website_browser/environment.py | 20 +++ .../agentenv_protocol/website_browser/log.py | 26 ++-- .../tests/test_browser_cdp.py | 116 +++++++++++++++++- .../tests/test_browser_log.py | 27 +++- src/agent_env/drivers/README.md | 2 +- src/agent_env/drivers/envs/page/driver.py | 33 ++++- .../drivers/envs/page/playwright_mcp.py | 15 ++- .../computer/browser_computer_test.py | 19 ++- tst/unit/drivers/page_driver_test.py | 53 +++++++- 12 files changed, 372 insertions(+), 34 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e9909751c..ec528b66c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -139,7 +139,7 @@ and deserialized through a registry. | `agents/` | The `agents` bundle: the agents agent-env ships, one folder each under `agents/agents/`, with a smoke task each under `agents/tasks/`. Its Claude Code, `@local/agentenv-framework/agents/claude-code`, is the default agent. An agent's Python packages are its folder's `requirements.txt`, installed in its image, never in agent-env's own environment. A run that pulls one of its agents reads the whole bundle, so `tst/unit/bundle/shipped_bundles_test.py` parses and checks every shipped bundle. | | `computers/` | The `computers` bundle: the base computers agent-env ships as `computer` artifacts, `ubuntu-24.04` (XFCE on Xvfb, supervisord as the session user, settings as `AGENTENV_SETTING_`), and `omarchy-4.5`, a disk alone built on the composing machine from Omarchy's own VM image (its `[forms.disk]` `source`) by `disk/omarchy/`'s recipe, its services systemd user units of the graphical session that start through `with-seed`, booted by the launcher as a `linux-desktop` guest (`env/envs/computer/utils/omarchy_disk.py`, on `thin_disk.py`, which Windows' disks use too), aliased through `[artifacts]`. It has no tasks either. `disk/linux/` is the Linux disk form's recipe: `boot/`, whose Dockerfile makes a disk form (a GPT disk of an EFI system partition holding one UKI, Ubuntu's kernel with a busybox initramfs whose init reads a NoCloud seed and `switch_root`s into the image's entrypoint, and an ext4 root filesystem that is a container image's, plus `/etc/agentenv/vm/image.env` from its OCI config), `tools/`, the disk tools that make it unprivileged on the builder's own platform (`mkfs.ext4 -d`), and `launcher/`, the QEMU launcher that boots one under KVM. A base's `[forms.disk]` with no image is made from its container form; the composer makes each arch's disk from its composed container, keeping the base disk's EFI system partition, keyed by the container's key, the base disk and the recipe; gate 3 probes it where a provider boots disks natively (`disk_image_platforms()`), and a deploy's first boot with KVM probes one left `unverified`; a deploy naming `form = "disk"` boots it through `create_vm(image=..., env=...)`, its settings in the seed. | | `browsers/` | The `browsers` bundle: the website browsers agent-env ships, today `chromium`, a `computer_app` env (Chrome for Testing at the build Playwright pins, for amd64 and arm64, set up for unattended use by managed policies in `/etc/opt/chrome_for_testing/policies/managed`, opened at boot on its `start_urls`, its profile its data). All of it, its pin, install step, policies and command, is in `envs/chromium/chromium.py`; `tst/integration/computer/chromium_app_test.py` runs it. Like `drivers`, it has no tasks. | -| `drivers/` | The `drivers` bundle: the computer drivers agent-env ships as `computer_driver` envs, the reference driver (X11 on Linux, recording through `x11grab`) aliased `reference`, the page driver aliased `page`, a browser computer's, whose screen is the browser's active tab and which serves Playwright MCP's `browser_*` tools beside `computer_*` from a Playwright MCP it runs attached to the browser over a CDP address the relay issues it (`browser_evaluate` and `browser_run_code_unsafe` hidden by default; `scripts/upgrade_playwright_mcp.py` pins its version), and trycua's cua-driver aliased `cua-driver` (its daemon from the pinned PyPI wheel, its own instructions, tools and skill resources served to agents through `@server_instructions`, `forwarded_tools` and `@resource`, its tools beside `computer_*`, `computer/v1` on its desktop-scope calls, its skill pack leading the computer skill; `tst/integration/computer/cua_driver_test.py` runs it). It has no tasks: a bundle with neither tasks nor evals is refused unless its `aliases.toml` names something for other bundles to use. The shipped `desktop` example composes it with the `ubuntu-24.04` base into a computer env, which `tst/integration/computer/` deploys, probes, records and has an agent use. | +| `drivers/` | The `drivers` bundle: the computer drivers agent-env ships as `computer_driver` envs, the reference driver (X11 on Linux, recording through `x11grab`) aliased `reference`, the page driver aliased `page`, a browser computer's, whose screen is the browser's active tab and which serves Playwright MCP's `browser_*` tools beside `computer_*` from a Playwright MCP it runs attached to the browser over a CDP address the relay issues it (`browser_evaluate` and `browser_run_code_unsafe` hidden by default; each call an entry in the browser's log, begun and ended with `browser/v1` `calls/begin` and `calls/end`, that its CDP commands name as `for`; `scripts/upgrade_playwright_mcp.py` pins its version), and trycua's cua-driver aliased `cua-driver` (its daemon from the pinned PyPI wheel, its own instructions, tools and skill resources served to agents through `@server_instructions`, `forwarded_tools` and `@resource`, its tools beside `computer_*`, `computer/v1` on its desktop-scope calls, its skill pack leading the computer skill; `tst/integration/computer/cua_driver_test.py` runs it). It has no tasks: a bundle with neither tasks nor evals is refused unless its `aliases.toml` names something for other bundles to use. The shipped `desktop` example composes it with the `ubuntu-24.04` base into a computer env, which `tst/integration/computer/` deploys, probes, records and has an agent use. | | `examples/` | The example bundles agent-env ships (`hello`, and `echo-mcp`, which needs Docker, and `desktop`, a computer env of `ubuntu-24.04` and `reference` with a probe task and an agent task). Tests run them, but they are examples for users, not fixtures. Every shipped bundle is registered under `agent_env.bundles` in `pyproject.toml`, ` = ""` shipping `src///`. | ### Training helpers diff --git a/packages/agentenv-protocol/README.md b/packages/agentenv-protocol/README.md index 89fd05516..5cb46e842 100644 --- a/packages/agentenv-protocol/README.md +++ b/packages/agentenv-protocol/README.md @@ -1013,7 +1013,12 @@ are the profile's template. The base class adds: that needs the one before takes it first, with `snapshot_env`. - **An action** is a CDP command through the relay (`via: "cdp"`, `who` the connection's user, its `connection` number, `method`, and the tab its session is on), an op a driver makes for an agent's MCP tool call (`via: "mcp"`, - `who` the caller's role, `tool`, `call`, `op`), or an acting op agentenv makes itself (`via: "agentenv"`). Each + `who` the caller's role, `tool`, `call`, `op`), an agent's MCP tool call a driver serves through a CDP user's + connections, such as Playwright MCP's (`via: "mcp"`, `who`, `tool`, `call`, with no `op`, between `calls/begin` + and `calls/end`), or an acting op agentenv makes itself (`via: "agentenv"`). A CDP command of a user with such a + call open names it, `for` its entry, or `for` a list of entries when several are open; the call's entry takes + the tab its commands acted on, and keeps what it typed only once each typing command of its own found no + password field focused. Each carries `tab`, `url`, `ok` and, failed, `error`, and is marked `acts` or not by its method or op (`log.acts`, `ACTING_OPS`): input, navigation, tab commands, `Runtime.evaluate` and `callFunctionOn`, DOM writes, faked responses, and viewport and cookie changes act; the rest read. An acting entry keeps its `params` @@ -1021,14 +1026,17 @@ are the profile's template. The base class adds: as its length; cookie and header values; the values of URL query keys that name a credential; a request's body; and each string past 2 KB cut, with its length and sha256. A reading entry keeps none. - **A tab event** (`opened`, `navigated`, `closed`) says what caused it: `by: "command"` with the `entry`, `via` and - `who` of the command that asked for it, matched exactly by the target a new tab's command answers with and the + `who` of the command that asked for it, and its `for`, matched exactly by the target a new tab's command answers with and the loader a navigation's does; `by: "agentenv"`; `exit` or `stop`; or `by: "page"`, with the `after` of the latest acting entry on its tab (its opener's, for a popup), for a click that follows a link, a redirect or a page script. - A driver passes an agent's tool call to the browser in the `AgentEnv-Tool-Call` header (`tool_call_header`), with the caller's `AgentEnv-Role`; the page driver sends it on every call it makes for a tool. `data/get` writes the log beside `browser-state.json` as `browser-log.jsonl`, which `data/add` passes over. - **`browser/v1`** on its card: `chromium` (its `version`), `clock: "real"` and, alone, `window`; `log {after}` - (GET), framework-only, its log's lines after `after`; and + (GET), framework-only, its log's lines after `after`; `calls/begin {user, acts?, within_s?}`, framework-only, + sent with the tool-call header, which begins the entry of an agent's tool call that `user`'s CDP connections + serve and answers its `entry`, and `calls/end {entry, ok, error?}`, which ends it, a call not ended within + `within_s` (600 s by default) written as never said to have ended; and `record` (GET), framework-only, which returns that record with `launches`, the app server's log of each launch (with its cause: `boot`, `server_restart`, `launch`, `data/add` or `data/reset`), each stop it made and each exit it didn't, with its exit `code` or the `signal` that ended it, since the computer booted. diff --git a/packages/agentenv-protocol/src/agentenv_protocol/website_browser/cdp.py b/packages/agentenv-protocol/src/agentenv_protocol/website_browser/cdp.py index 2795fcf84..92f9483b1 100644 --- a/packages/agentenv-protocol/src/agentenv_protocol/website_browser/cdp.py +++ b/packages/agentenv-protocol/src/agentenv_protocol/website_browser/cdp.py @@ -18,7 +18,8 @@ from starlette.requests import Request from starlette.websockets import WebSocket, WebSocketDisconnect -from .log import ERROR_CHARS, KEY_FIELDS, TYPING_METHODS, BrowserLog, acts, redact_text, redacted_params +from .log import (ERROR_CHARS, KEY_FIELDS, TYPING_METHODS, BrowserLog, acts, redact_text, redacted_args, + redacted_params) if TYPE_CHECKING: from .control import BrowserControl @@ -213,13 +214,35 @@ def __init__(self, websocket: WebSocket, user: str, session: str, record: dict, self.closing = False +class _Call: + """An agent's tool call a driver serves through one CDP user's connections, open from its ``calls/begin`` to its + ``calls/end``: its entry; the tab its commands last acted on, else the one they last read; whether a password field + had focus for its typing, None until a typing command of its own is checked; and when it's given up on.""" + + def __init__(self, begun: dict, user: str, call: dict) -> None: + self.begun, self.user, self.tool, self.args = begun, user, call["tool"], call["args"] + self.tab: Optional[str] = None + self.acted = False + self.password: Optional[bool] = None + self.expiry: Optional[asyncio.TimerHandle] = None + + def credit(self, target: Optional[str], acting: bool, typed: bool, password: bool, alone: bool) -> None: + """One of its user's commands credited to it, ``alone`` or with other calls open: the tab it was on, and for + one that typed, whether a password field had focus there, taken as so when other calls shared it.""" + if target and alone and (acting or not self.acted): + self.tab, self.acted = target, self.acted or acting + if typed: + self.password = True if password or not alone else bool(self.password) + + class Relay: """Each WebSocket connection given its own browser-level session over ``devtools``, its commands sent on it past the guard and the answers and events of its sessions sent back, so several clients share one browser; and a record of who connected when, and what the guard did to their commands. ``dialogs_answered`` says agentenv answers the browser's dialogs itself. With a ``log``, each command is an entry in it, on the tab its session is on, and ``control``, agentenv's own connection, tells its tab's URL, whether a password field has focus there, and what - causes the tab events a command brings.""" + causes the tab events a command brings. An agent's tool call a driver serves through a CDP user's connections, + such as Playwright MCP's, is an entry too, which that user's commands are credited to while it's open.""" def __init__(self, devtools: DevToolsPipe, *, dialogs_answered: bool = False, log: Optional[BrowserLog] = None, control: Optional[BrowserControl] = None) -> None: @@ -227,8 +250,37 @@ def __init__(self, devtools: DevToolsPipe, *, dialogs_answered: bool = False, lo self.connections: list[dict] = [] self._by_session: dict[str, _Connection] = {} self._open: set[_Connection] = set() + #: The tool calls open now, by their entry's sequence number. + self.calls: dict[int, _Call] = {} devtools.listen(self._event) + def begin_call(self, user: str, who: str, call: dict, *, acts: bool, within_s: float) -> int: + """``who``'s tool ``call``, which a driver serves through ``user``'s connections, begun as an entry in the log: + each of ``user``'s commands is credited to it until ``end_call`` ends it, or ``within_s`` passes and it's + written as never said to have ended. Its entry's sequence number.""" + begun = self.log.begin({"via": "mcp", "who": who, "tool": call["tool"], "call": call["call"], "acts": acts}) + opened = self.calls[begun["seq"]] = _Call(begun, user, call) + opened.expiry = asyncio.get_running_loop().call_later( + within_s, self.end_call, begun["seq"], False, f"its driver didn't say it ended within {within_s:g} s") + return begun["seq"] + + def end_call(self, seq: int, ok: bool, error: Optional[str] = None) -> bool: + """The call begun as entry ``seq`` ended: its entry written with whether it worked and why not, the tab its + commands acted on and that tab's URL, and, acting, its arguments, what it typed kept only once every command of + its own that typed found no password field focused. False when no such call is open.""" + call = self.calls.pop(seq, None) + if call is None: + return False + call.expiry.cancel() + fields: dict[str, Any] = {"tab": call.tab, "url": self.control.url_of(call.tab) + if self.control is not None and call.tab else None, "ok": ok} + if error: + fields["error"] = redact_text(error)[:ERROR_CHARS] + if call.begun["action"]["acts"]: + fields["args"] = redacted_args(call.tool, call.args, password=call.password is not False) + self.log.finish(call.begun, **fields) + return True + async def serve(self, websocket: WebSocket, user: str) -> None: """``websocket``, a client of ``user``'s, relayed over a browser-level session of its own until either ends.""" try: @@ -317,7 +369,8 @@ async def _command(self, connection: _Connection, message: Any) -> None: def _begin(self, connection: _Connection, session: str, method: str, params: dict) -> Optional[_Logged]: """``connection``'s command, begun in the log, if there is one, on the tab ``session`` is on, or for a browser - command the tab it names; what it causes made known to the control.""" + command the tab it names, ``for`` the tool calls of its user's open now; what it causes made known to the + control.""" if self.log is None: return None target = connection.targets.get(session) if session != connection.session else None @@ -325,9 +378,12 @@ def _begin(self, connection: _Connection, session: str, method: str, params: dic target = params["targetId"] acting = acts(method) url = self.control.url_of(target) if self.control is not None and target else None - begun = self.log.begin({"via": "cdp", "who": connection.user, "connection": connection.number, - "method": method, "acts": acting, "tab": target, "url": url}) - return _Logged(self, begun, method, params, target, acting) + crediting = [call for call in self.calls.values() if call.user == connection.user] + action = {"via": "cdp", "who": connection.user, "connection": connection.number, "method": method, + "acts": acting, "tab": target, "url": url} + if crediting: + action["for"] = crediting[0].begun["seq"] if len(crediting) == 1 else [c.begun["seq"] for c in crediting] + return _Logged(self, self.log.begin(action), method, params, target, acting, crediting) @staticmethod def _guarded(connection: _Connection, method: str, verdict: Verdict) -> None: @@ -398,11 +454,12 @@ class _Logged: and its answer.""" def __init__(self, relay: Relay, begun: dict, method: str, params: dict, target: Optional[str], - acting: bool) -> None: - self.relay, self.begun, self.method, self.params, self.target, self.acting = \ - relay, begun, method, params, target, acting + acting: bool, calls: list[_Call]) -> None: + self.relay, self.begun, self.method, self.params, self.target, self.acting, self.calls = \ + relay, begun, method, params, target, acting, calls control = relay.control - self._cause = cause = {"seq": begun["seq"], "via": "cdp", "who": begun["action"]["who"]} + self._cause = cause = {"seq": begun["seq"], "via": "cdp", "who": begun["action"]["who"], + **({"for": begun["action"]["for"]} if "for" in begun["action"] else {})} self._landed: Optional[asyncio.Future] = None self._checks_password = self.password = False if control is None or not acting: @@ -453,6 +510,8 @@ def finish(self, answer: dict, guarded: Optional[str]) -> None: fields["tab"] = self.target = opened if self.target and self.relay.control is not None and self.begun["action"].get("url") is None: fields["url"] = self.relay.control.url_of(self.target) + for call in self.calls: + call.credit(self.target, self.acting, self._checks_password, self.password, alone=len(self.calls) == 1) self.relay.log.finish(self.begun, **fields) diff --git a/packages/agentenv-protocol/src/agentenv_protocol/website_browser/environment.py b/packages/agentenv-protocol/src/agentenv_protocol/website_browser/environment.py index 40e852d82..b301c50fc 100644 --- a/packages/agentenv-protocol/src/agentenv_protocol/website_browser/environment.py +++ b/packages/agentenv-protocol/src/agentenv_protocol/website_browser/environment.py @@ -352,6 +352,26 @@ async def browser_sites_set(self, sites: list[dict]) -> dict: written.replace(SITES) return {} + @extension(BROWSER, op="calls/begin") + async def browser_calls_begin(self, user: str, request: Request, acts: bool = True, within_s: float = 600.0) -> dict: + """An agent's tool call, from the tool-call header this is sent with, which a driver serves through ``user``'s + CDP connections, begun as an entry in the log, by the caller's role: ``user``'s commands are credited to it + until ``calls/end`` ends it, or ``within_s`` passes and it's written as never said to have ended. Its entry's + sequence number, ``entry``.""" + call = read_tool_call(request.headers.get(TOOL_CALL_HEADER)) + if call is None: + raise InvalidArgument(f"calls/begin is sent with the tool call it begins, in its {TOOL_CALL_HEADER} header") + who = normalize_role(request.headers.get(ROLE_HEADER)) or DEFAULT_ROLE + return {"entry": self.relay.begin_call(user, who, call, acts=acts, within_s=within_s)} + + @extension(BROWSER, op="calls/end") + async def browser_calls_end(self, entry: int, ok: bool, error: Optional[str] = None) -> dict: + """The tool call ``calls/begin`` began as ``entry`` ended, with whether it worked and, if not, why: its entry + written, with the tab its commands acted on.""" + if not self.relay.end_call(entry, ok, error): + raise InvalidArgument(f"no tool call is open as entry {entry}") + return {} + @extension(BROWSER, op="cdp") async def browser_cdp(self, user: str, request: Request, origin: Optional[str] = None) -> dict: """``user``'s CDP address once the browser's ready, the same one each time it's asked for, as the server that diff --git a/packages/agentenv-protocol/src/agentenv_protocol/website_browser/log.py b/packages/agentenv-protocol/src/agentenv_protocol/website_browser/log.py index 34e88e778..f86f62f95 100644 --- a/packages/agentenv-protocol/src/agentenv_protocol/website_browser/log.py +++ b/packages/agentenv-protocol/src/agentenv_protocol/website_browser/log.py @@ -75,8 +75,9 @@ ACTING_OPS = frozenset({"page/input", "navigate", "history", "tabs/open", "tabs/activate", "tabs/close", "dialog/answer", "files/choose", "cookies/set", "cookies/delete", "local_storage/set", "local_storage/clear", "sites/set"}) -#: The ``browser/v1`` ops an agent's calls are never logged for: the log itself, and agentenv's own bookkeeping. -UNLOGGED_OPS = frozenset({"log", "record"}) +#: The ``browser/v1`` ops an agent's calls are never logged for: the log itself, and agentenv's own bookkeeping, a +#: driver's calls that begin and end a tool call's own entry among them. +UNLOGGED_OPS = frozenset({"log", "record", "calls/begin", "calls/end"}) #: The CDP methods and ops whose text is typed into the page. TYPING_METHODS = frozenset({"Input.insertText", "Input.dispatchKeyEvent", "Input.imeSetComposition"}) #: What a key event says of the key it types, all of it dropped when a password field has focus. @@ -92,8 +93,10 @@ URL_FIELDS = frozenset({"url", "urls", "href", "referrer", "documentURL"}) #: The fields a page's content or a response's body is in, which an entry withholds, keeping their length and sha256. BODY_FIELDS = frozenset({"body", "postData", "html", "rawResponse"}) -#: A tool call's arguments that say what is typed, withheld while a password field has focus. -TYPED_ARGS = frozenset({"text", "keys"}) +#: A tool call's arguments that say what is typed, withheld while a password field has focus: text, keys, or a key. +TYPED_ARGS = frozenset({"text", "keys", "key"}) +#: A tool call's argument that lists form fields, each typed into as its ``value`` says. +FIELDS_ARG = "fields" _WORDS = re.compile(r"[A-Z]?[a-z0-9]+|[A-Z]+(?![a-z])") #: A URL inside free text, such as an error message. _URL_IN_TEXT = re.compile(r"[A-Za-z][A-Za-z0-9+.-]*://[^\s\"'<>]+") @@ -240,10 +243,15 @@ def redacted_op(op: str, params: dict, *, password: bool = False) -> dict: def redacted_args(tool: str, args: Mapping[str, Any], *, password: bool = False) -> dict: """An MCP tool call's arguments as its entry keeps them: URLs as ``redact_url`` keeps them, and what it types, its - text or its keys in whatever shape the header carried them, as its length while a password field has focus.""" + text, its keys or its form fields' values, in whatever shape the header carried them, as its length while a + password field has focus.""" kept = _urls_and_bodies(args) if password: kept = {key: {"length": _length(value)} if key in TYPED_ARGS else value for key, value in kept.items()} + if isinstance(kept.get(FIELDS_ARG), list): + kept[FIELDS_ARG] = [{**field, "value": {"length": _length(field["value"])}} + if isinstance(field, Mapping) and "value" in field else field + for field in kept[FIELDS_ARG]] return cut(kept) @@ -449,14 +457,16 @@ def _scan(path: Path) -> tuple[int, bool, dict[int, tuple[int, int]], int]: def caused(cause: Optional[dict], log: BrowserLog, tab: Optional[str]) -> dict: - """What a tab event says caused it: the command it was, by its entry, ``via`` and ``who``; agentenv itself; or, - with no ``cause``, the page, after the latest acting entry on ``tab`` if there is one.""" + """What a tab event says caused it: the command it was, by its entry, ``via`` and ``who``, and the tool call it + served ``for`` when it was credited to one; agentenv itself; or, with no ``cause``, the page, after the latest + acting entry on ``tab`` if there is one.""" if cause is None: after = log.last_acting.get(tab) if tab else None return {"by": "page", **({"after": after} if after else {})} if cause.get("via") == "agentenv": return {"by": "agentenv"} - return {"by": "command", "entry": cause["seq"], "via": cause["via"], "who": cause.get("who")} + return {"by": "command", "entry": cause["seq"], "via": cause["via"], "who": cause.get("who"), + **({"for": cause["for"]} if cause.get("for") is not None else {})} def log_time() -> str: diff --git a/packages/agentenv-protocol/tests/test_browser_cdp.py b/packages/agentenv-protocol/tests/test_browser_cdp.py index 0ea2c4e9e..88584e1f6 100644 --- a/packages/agentenv-protocol/tests/test_browser_cdp.py +++ b/packages/agentenv-protocol/tests/test_browser_cdp.py @@ -282,18 +282,19 @@ async def test_a_client_that_falls_too_far_behind_is_closed_and_what_waited_for_ class Control: - """agentenv's control as the relay asks it: the URL of tab T1, a password field focused wherever it's asked, and - the causes of tab events; each check made with how many commands ``pipe`` had been sent by then.""" + """agentenv's control as the relay asks it: the URL of tab T1, a password field focused wherever it's asked unless + ``focused`` says not, and the causes of tab events; each check made with how many commands ``pipe`` had been sent + by then.""" def __init__(self, pipe=None) -> None: - self.causes, self.asked, self.pipe = Causes(), [], pipe + self.causes, self.asked, self.pipe, self.focused = Causes(), [], pipe, True def url_of(self, target): return "https://a.test/page" if target == "T1" else None async def password_focused(self, target) -> bool: self.asked.append((target, len(self.pipe.sent) if self.pipe is not None else None)) - return True + return self.focused @pytest.mark.asyncio @@ -332,6 +333,90 @@ async def test_every_relayed_command_is_an_entry_on_its_sessions_tab_an_acting_o assert [target for target, _ in control.asked] == ["T1"] +async def _attached(socket) -> str: + return (await _ask(socket, {"id": 0, "method": "Target.attachToTarget", + "params": {"targetId": "T1", "flatten": True}}))["result"]["sessionId"] + + +def _call(tool: str, call: str, args: dict) -> dict: + return {"tool": tool, "call": call, "args": args} + + +@pytest.mark.asyncio +async def test_a_users_commands_are_credited_to_its_open_tool_call_or_to_every_one_open_when_calls_overlap(tmp_path): + log, control = BrowserLog(tmp_path / "browser-chromium.jsonl"), Control() + async with _relay(log=log, control=control) as (pipe, relay, base): + async with connect(f"{base}/playwright-mcp") as mcp, connect(f"{base}/ada") as ada: + child = await _attached(mcp) + click = relay.begin_call("playwright-mcp", "agent", _call("browser_click", "c1", {"target": "e3"}), + acts=True, within_s=60) + await _ask(mcp, {"id": 1, "method": "Input.dispatchMouseEvent", "params": {"type": "mousePressed"}, + "sessionId": child}) + await _ask(ada, {"id": 1, "method": "Browser.getVersion"}) + snapshot = relay.begin_call("playwright-mcp", "agent", _call("browser_snapshot", "c2", {}), acts=False, + within_s=60) + await _ask(mcp, {"id": 2, "method": "DOM.getDocument", "sessionId": child}) + ended = relay.end_call(click, True), relay.end_call(snapshot, False, "Playwright MCP ended during it") + await _ask(mcp, {"id": 3, "method": "DOM.getDocument", "sessionId": child}) + again = relay.end_call(click, True) + await asyncio.sleep(0.05) + + mine = {"via": "cdp", "who": "playwright-mcp", "connection": 1, "tab": "T1", "url": "https://a.test/page"} + assert [line["action"] for line in log.read()["lines"]] == [ + {**mine, "method": "Target.attachToTarget", "acts": False, "ok": True}, + {"via": "mcp", "who": "agent", "tool": "browser_click", "call": "c1", "acts": True, "tab": "T1", + "url": "https://a.test/page", "ok": True, "args": {"target": "e3"}}, + {**mine, "method": "Input.dispatchMouseEvent", "acts": True, "for": click, "ok": True, + "params": {"type": "mousePressed"}}, + {"via": "cdp", "who": "ada", "connection": 2, "method": "Browser.getVersion", "acts": False, "tab": None, + "url": None, "ok": True}, + {"via": "mcp", "who": "agent", "tool": "browser_snapshot", "call": "c2", "acts": False, "tab": None, + "url": None, "ok": False, "error": "Playwright MCP ended during it"}, + {**mine, "method": "DOM.getDocument", "acts": False, "for": [click, snapshot], "ok": True}, + {**mine, "method": "DOM.getDocument", "acts": False, "ok": True}, + ] + assert ended == (True, True) and again is False and not relay.calls + + +@pytest.mark.asyncio +async def test_a_tool_calls_typed_text_is_kept_only_once_its_own_typing_found_no_password_field(tmp_path): + log, control = BrowserLog(tmp_path / "browser-chromium.jsonl"), Control() + async with _relay(log=log, control=control) as (pipe, relay, base): + async with connect(f"{base}/playwright-mcp") as mcp: + child = await _attached(mcp) + for n, (focused, types) in enumerate([(False, True), (True, True), (False, False)], start=1): + control.focused = focused + entry = relay.begin_call("playwright-mcp", "agent", + _call("browser_type", f"c{n}", {"target": "e3", "text": "hunter2"}), + acts=True, within_s=60) + if types: + await _ask(mcp, {"id": n, "method": "Input.insertText", "params": {"text": "hunter2"}, + "sessionId": child}) + relay.end_call(entry, True) + await asyncio.sleep(0.05) + + typed = {line["action"]["call"]: line["action"]["args"]["text"] for line in log.read()["lines"] + if line["action"]["via"] == "mcp"} + assert typed == {"c1": "hunter2", "c2": {"length": 7}, "c3": {"length": 7}} + + +@pytest.mark.asyncio +async def test_a_tool_call_never_said_to_end_is_written_so_once_its_bound_passes_and_is_credited_no_more(tmp_path): + log = BrowserLog(tmp_path / "browser-chromium.jsonl") + async with _relay(log=log) as (pipe, relay, base): + async with connect(f"{base}/playwright-mcp") as mcp: + entry = relay.begin_call("playwright-mcp", "agent", _call("browser_click", "c1", {}), acts=True, + within_s=0.05) + await asyncio.sleep(0.1) + await _ask(mcp, {"id": 1, "method": "Browser.getVersion"}) + ended = relay.end_call(entry, True) + await asyncio.sleep(0.05) + + call, command = [line["action"] for line in log.read()["lines"]] + assert (call["ok"], call["error"]) == (False, "its driver didn't say it ended within 0.05 s") + assert "for" not in command and ended is False + + @pytest.mark.asyncio async def test_a_command_is_checked_for_a_password_before_it_is_sent_and_logged_though_its_client_leaves(tmp_path): log = BrowserLog(tmp_path / "browser-chromium.jsonl") @@ -417,6 +502,29 @@ def tool(name: str, call: str, args: dict) -> dict: assert read["last"] == 4 and not read["more"] +@pytest.mark.asyncio +async def test_calls_begin_and_end_write_a_tool_calls_entry_from_its_header_and_are_no_entries_themselves( + monkeypatch, tmp_path): + (tmp_path / "logs").mkdir() + browser = _browser(monkeypatch, tmp_path) + asgi = browser.served_asgi(browser.create_app().streamable_http_app()) + url = "https://a.test/?token=t" + called = {TOOL_CALL_HEADER: tool_call_header("browser_navigate", "c1", {"url": url}), "AgentEnv-Role": "ada"} + + async with httpx.AsyncClient(transport=httpx.ASGITransport(app=asgi), base_url="http://app") as http: + begun = await http.post("/agentenv/ext/browser/calls/begin", json={"user": "playwright-mcp"}, headers=called) + ended = await http.post("/agentenv/ext/browser/calls/end", json={"entry": begun.json()["entry"], "ok": True}) + headerless = await http.post("/agentenv/ext/browser/calls/begin", json={"user": "playwright-mcp"}) + unknown = await http.post("/agentenv/ext/browser/calls/end", json={"entry": 99, "ok": True}) + read = (await http.get("/agentenv/ext/browser/log", params={"after": 0})).json() + + assert [line["action"] for line in read["lines"]] == [ + {"via": "mcp", "who": "ada", "tool": "browser_navigate", "call": "c1", "acts": True, "tab": None, "url": None, + "ok": True, "args": {"url": f"https://a.test/?token={REDACTED}"}}] + assert ended.status_code == 200 and headerless.status_code == 400 and unknown.status_code == 400 + assert "AgentEnv-Tool-Call" in headerless.json()["error"]["message"] + + @pytest.mark.asyncio async def test_a_commands_wait_for_its_password_check_lets_none_after_it_be_sent_first(tmp_path): log = BrowserLog(tmp_path / "browser-chromium.jsonl") diff --git a/packages/agentenv-protocol/tests/test_browser_log.py b/packages/agentenv-protocol/tests/test_browser_log.py index 11cff2fb3..de32876a0 100644 --- a/packages/agentenv-protocol/tests/test_browser_log.py +++ b/packages/agentenv-protocol/tests/test_browser_log.py @@ -13,8 +13,9 @@ import pytest from agentenv_protocol.website_browser import log as browser_log -from agentenv_protocol.website_browser.log import (ACTOR, CUT_CHARS, REDACTED, BrowserLog, acts, cut, op_acts, read_tool_call, redact_text, - redact_url, redacted_args, redacted_op, redacted_params, tool_call_header) +from agentenv_protocol.website_browser.log import (ACTOR, CUT_CHARS, REDACTED, BrowserLog, acts, caused, cut, op_acts, read_tool_call, + redact_text, redact_url, redacted_args, redacted_op, redacted_params, + tool_call_header) from agentenv_protocol.computer.errors import InvalidArgument from agentenv_protocol.website_browser.control import BrowserControl @@ -90,6 +91,28 @@ def test_typing_while_a_password_field_has_focus_keeps_only_how_much_was_typed() {"cookies": [{"name": "a", "value": REDACTED}]} +def test_playwright_mcps_typing_arguments_keep_only_their_length_while_a_password_field_has_focus(): + fields = {"fields": [{"name": "Email", "type": "textbox", "value": "ada@x.test"}, + {"name": "Password", "type": "textbox", "value": "hunter2"}, {"name": "Remember"}]} + assert redacted_args("browser_type", {"target": "e3", "text": "hunter2"}, password=True) == \ + {"target": "e3", "text": {"length": 7}} + assert redacted_args("browser_press_key", {"key": "h"}, password=True) == {"key": {"length": 1}} + assert redacted_args("browser_fill_form", fields, password=True) == {"fields": [ + {"name": "Email", "type": "textbox", "value": {"length": 10}}, + {"name": "Password", "type": "textbox", "value": {"length": 7}}, {"name": "Remember"}]} + assert redacted_args("browser_fill_form", fields) == fields + + +def test_a_tab_event_a_tool_calls_command_caused_names_the_call(tmp_path): + log = BrowserLog(tmp_path / "browser-chromium.jsonl") + command = {"seq": 7, "via": "cdp", "who": "playwright-mcp"} + + assert caused({**command, "for": 5}, log, "T1") == {"by": "command", "entry": 7, "via": "cdp", + "who": "playwright-mcp", "for": 5} + assert caused({**command, "for": [5, 6]}, log, "T1")["for"] == [5, 6] + assert "for" not in caused(command, log, "T1") + + def test_a_long_value_is_cut_with_its_length_and_sha256(): script = "x" * (CUT_CHARS + 10) assert redacted_params("Runtime.evaluate", {"expression": script})["expression"] == { diff --git a/src/agent_env/drivers/README.md b/src/agent_env/drivers/README.md index 54e196a3a..4675d80d5 100644 --- a/src/agent_env/drivers/README.md +++ b/src/agent_env/drivers/README.md @@ -19,7 +19,7 @@ and input reaches it as CDP input events, both through `browser/v1`'s `page/*` o a tab that paints nothing shows white. It serves the page group too: `computer_navigate`, `computer_history`, `computer_tabs`, `computer_dialog` and `computer_upload`, and every frame names the page, its tab and what is open on it. The browser ending on its own fails the computer for good: its ready check is -`final`, and its tools answer with why. Each call it makes to the browser for an agent's tool carries that call, its tool, id, arguments and the caller's role, in the `AgentEnv-Tool-Call` and `AgentEnv-Role` headers, so the browser's log credits the action to it. Name it as `page`, or by its id, `@local/agentenv-framework/drivers/page`. +`final`, and its tools answer with why. Each call it makes to the browser for an agent's tool carries that call, its tool, id, arguments and the caller's role, in the `AgentEnv-Tool-Call` and `AgentEnv-Role` headers, so the browser's log credits the action to it. Each `browser_*` call is an entry too, begun and ended with `calls/begin` and `calls/end`, which Playwright MCP's CDP commands meanwhile, and the tab events they cause, name as `for`. Name it as `page`, or by its id, `@local/agentenv-framework/drivers/page`. Beside those it serves Playwright MCP's `browser_*` tools (with `--caps vision`), as `playwright-mcp-tools.json` lists them, from `playwright_mcp.py`. Its install lays Node and every npm package Playwright MCP needs out from pinned diff --git a/src/agent_env/drivers/envs/page/driver.py b/src/agent_env/drivers/envs/page/driver.py index 3f8b87f87..fe3e62d6b 100644 --- a/src/agent_env/drivers/envs/page/driver.py +++ b/src/agent_env/drivers/envs/page/driver.py @@ -8,6 +8,7 @@ import asyncio import base64 +import logging import math import string from pathlib import Path @@ -59,6 +60,8 @@ import playwright_mcp +logger = logging.getLogger(__name__) + BUTTONS = {"left": 1, "right": 2, "middle": 4, "back": 8, "forward": 16} MODIFIERS = {"Alt": 1, "Control": 2, "Meta": 4, "Shift": 8} #: Keys that aren't characters: each one's DOM code, its Windows virtual key code, and the text a press of it types. @@ -117,7 +120,35 @@ def forwarded_tools(self) -> list[ForwardedTool]: return playwright_mcp.tools(self._playwright_call) async def _playwright_call(self, tool: str, /, **arguments: Any) -> CallToolResult: - return await self._playwright.call(tool, **arguments) + """Playwright MCP's ``tool``, an entry in the browser's log from start to end, which Playwright MCP's commands + meanwhile are credited to; a call the browser can't log still goes.""" + entry = await self._call_begun(tool) + result: Optional[CallToolResult] = None + try: + result = await self._playwright.call(tool, **arguments) + return result + finally: + if entry is not None: + await self._call_ended(entry, result) + + async def _call_begun(self, tool: str) -> Optional[int]: + params = {"user": playwright_mcp.CDP_USER, "acts": playwright_mcp.acts(tool), + "within_s": playwright_mcp.CALL_BOUND_S} + try: + return (await self._call("calls/begin", params))["entry"] + except (DeviceUnavailable, InvalidArgument, Unsupported) as e: + logger.warning(f"the browser didn't begin an entry for {tool}: {e}") + return None + + async def _call_ended(self, entry: int, result: Optional[CallToolResult]) -> None: + ok = result is not None and not result.isError + error = None if ok else "cancelled" if result is None else \ + " ".join(part.text for part in result.content if part.type == "text") + try: + await self._call("calls/end", {"entry": entry, "ok": ok, **({"error": error} if error else {})}, + credited=False) + except (DeviceUnavailable, InvalidArgument, Unsupported) as e: + logger.warning(f"the browser didn't end entry {entry}: {e}") async def _playwright_endpoint(self) -> str: """The CDP address the browser issues Playwright MCP, at this driver's own loopback address, where its relay is diff --git a/src/agent_env/drivers/envs/page/playwright_mcp.py b/src/agent_env/drivers/envs/page/playwright_mcp.py index a3d61c3cf..4e24e9839 100644 --- a/src/agent_env/drivers/envs/page/playwright_mcp.py +++ b/src/agent_env/drivers/envs/page/playwright_mcp.py @@ -46,6 +46,9 @@ #: before it's killed. POLL_S = 0.1 STOP_S = 10.0 +#: How long one call may take at most, its start and its session's together with its own bound: as long as the browser +#: credits Playwright MCP's commands to it unless told it ended sooner. +CALL_BOUND_S = 2 * START_S + CALL_TIMEOUT_S #: How long a caller's session on Playwright MCP is kept with no call: a caller's own MCP session ends with no word to #: the driver, and each holds a connection to the browser. SESSION_IDLE_S = 3600.0 @@ -57,7 +60,17 @@ def tools(call: Callable[..., Awaitable[CallToolResult]]) -> list[ForwardedTool] """Playwright MCP's tools, as its manifest lists them, each called through ``call`` with its name first; those that run the agent's own code hidden until a task's tool access names them.""" return [ForwardedTool.listed(t, functools.partial(call, t["name"]), hidden_by_default=t["name"] in HIDDEN_TOOLS) - for t in json.loads(TOOLS.read_text(encoding="utf-8"))["tools"]] + for t in _manifest()] + + +def acts(tool: str) -> bool: + """Whether ``tool`` can change the browser or a page: any its manifest doesn't mark read-only.""" + return not next(t for t in _manifest() if t["name"] == tool)["annotations"].get("readOnlyHint", False) + + +@functools.cache +def _manifest() -> list[dict]: + return json.loads(TOOLS.read_text(encoding="utf-8"))["tools"] class PlaywrightMcp: diff --git a/tst/integration/computer/browser_computer_test.py b/tst/integration/computer/browser_computer_test.py index 805922da3..73e4ab26e 100644 --- a/tst/integration/computer/browser_computer_test.py +++ b/tst/integration/computer/browser_computer_test.py @@ -2,8 +2,9 @@ and the page driver, whose screen is chromium's active tab, headful on the X server only the browser uses. Gate 3 makes this machine's arch ready with it, its input checks and page tools verified on the HTML desk; deployed through the computer provider like any computer, its computer_* tools see and drive the page, Playwright MCP's browser_* tools beside them type into it and answer its -dialog, the two that run code hidden until tool access names one, as the installed Playwright MCP lists them, and -back after Playwright MCP or the driver is killed, and its page tools navigate, move in history, answer a dialog, use tabs +dialog, each call an entry in the browser's log that its CDP commands and the navigation they cause point at, the two +that run code hidden until tool access names one, as the installed Playwright MCP lists them, and back after Playwright +MCP or the driver is killed, and its page tools navigate, move in history, answer a dialog, use tabs and upload; the browser's X server runs but no desktop, and nothing listens but the driver and the site proxy; a killed app server or driver is started again by supervisord and the computer is ready again; chromium killed fails it for good, computer-ready/v1 saying why and the tools answering with it, and so does its X server killed, which takes @@ -1027,6 +1028,7 @@ async def test_playwright_mcps_tools_drive_its_page_hide_the_code_tools_and_come serving = asyncio.ensure_future(inside.run(deployed, ["python3", "-c", SERVE_SITE, "900"], timeout_s=960)) await asyncio.sleep(SERVE_START_S) confirmed = await _mcp(deployed, _confirm_ask) + logged = (await _browser_log(deployed))["lines"] await deployed.invoke(ENABLE_TOOL_EXTENSION_URI, ENABLE, {"role": "default", "tools": ["browser_evaluate"]}) shown, evaluated = await _tool_names(deployed), await _mcp(deployed, _evaluate_title) running = (await inside.run(deployed, ["sh", "-c", "ps -eo args | grep -m1 '[@]playwright/mcp/cli.js'"], @@ -1051,6 +1053,19 @@ async def test_playwright_mcps_tools_drive_its_page_hide_the_code_tools_and_come assert not {"browser_evaluate", "browser_run_code_unsafe"} & set(listed) and "computer_screenshot" in listed assert "hello" in typed and state["v"] == "hello" and after_typing == viewport assert "confirmed true" in confirmed, confirmed + entries = [line for line in logged if "action" in line] + calls = {line["action"]["tool"]: line["seq"] for line in entries + if line["action"]["via"] == "mcp" and line["action"]["tool"].startswith("browser_")} + typing = next(line["action"] for line in entries if line["action"].get("tool") == "browser_type") + sent = {line["action"]["method"]: line["action"].get("for") for line in entries + if line["action"]["via"] == "cdp" and line["action"]["method"] in ("Page.navigate", "Page.handleJavaScriptDialog")} + clicks = {line["action"].get("for") for line in entries if line["action"].get("method") == "Input.dispatchMouseEvent"} + navigated = next(line["tab"] for line in logged if line.get("tab", {}).get("url") == SITE) + assert {"browser_snapshot", "browser_type", "browser_navigate", "browser_click", "browser_handle_dialog"} <= set(calls) + assert (typing["who"], typing["ok"], typing["args"]["text"]) == ("default", True, "hello") + assert sent == {"Page.navigate": calls["browser_navigate"], "Page.handleJavaScriptDialog": calls["browser_handle_dialog"]} + assert clicks == {calls["browser_click"]}, clicks + assert (navigated["who"], navigated["for"]) == ("playwright-mcp", calls["browser_navigate"]), navigated assert "browser_evaluate" in shown and "browser_run_code_unsafe" not in shown and "confirmed true" in evaluated assert installed == json.loads(PLAYWRIGHT_MCP_TOOLS.read_text())["tools"] assert "Page URL" in after_its_kill and "Page URL" in after_driver_kill and count == "1" diff --git a/tst/unit/drivers/page_driver_test.py b/tst/unit/drivers/page_driver_test.py index cf1569aec..1ab736567 100644 --- a/tst/unit/drivers/page_driver_test.py +++ b/tst/unit/drivers/page_driver_test.py @@ -21,6 +21,7 @@ from unittest import mock import httpx +from mcp.types import CallToolResult, TextContent import pytest from pytest_socket import enable_socket @@ -48,7 +49,8 @@ } NO_DISPLAY = Platform(os="linux", distribution="ubuntu", version="24.04", arch="amd64", display="none") PAGE_OPS = ("page/viewport", "page/capture", "page/input", "page/state", "navigate", "history", "tabs/list", - "tabs/open", "tabs/activate", "tabs/close", "dialog/answer", "files/choose", "cdp") + "tabs/open", "tabs/activate", "tabs/close", "dialog/answer", "files/choose", "cdp", "calls/begin", + "calls/end") GETS = ("page/viewport", "page/capture", "page/state", "tabs/list") CARD = {"name": "chromium", "capabilities": {"extensions": [{"uri": BROWSER, "params": {"methods": { op: {"method": "GET" if op in GETS else "POST", "endpoint": f"/agentenv/ext/browser/{op}"} @@ -310,6 +312,55 @@ def test_its_manifest_is_of_the_playwright_mcp_its_install_downloads(): assert versions == [manifest["version"]] +@pytest.mark.asyncio +async def test_each_playwright_mcp_call_is_an_entry_in_the_browsers_log_begun_as_the_agents_call_ended_with_how_it_went( + page, monkeypatch): + page.served["calls/begin"] = {"entry": 7} + results = iter([CallToolResult(content=[TextContent(type="text", text="clicked")]), + CallToolResult(isError=True, content=[TextContent(type="text", text="no such element")])]) + + async def call(tool, **arguments): + return next(results) + + monkeypatch.setattr(page._playwright, "call", call) + with bound(ToolContext(tool="browser_click", call_id="c1", mcp=True)): + clicked = await page._playwright_call("browser_click", target="e3") + with bound(ToolContext(tool="browser_snapshot", call_id="c2", mcp=True)): + failed = await page._playwright_call("browser_snapshot") + + assert page.sent == [("calls/begin", {"user": "playwright-mcp", "acts": True, "within_s": 360.0}), + ("calls/end", {"entry": 7, "ok": True}), + ("calls/begin", {"user": "playwright-mcp", "acts": False, "within_s": 360.0}), + ("calls/end", {"entry": 7, "ok": False, "error": "no such element"})] + assert TOOL_CALL_HEADER in page.headers[0] and TOOL_CALL_HEADER not in page.headers[1] + assert clicked.content[0].text == "clicked" and failed.isError + + +@pytest.mark.asyncio +async def test_a_call_the_browser_cant_log_still_goes_and_a_cancelled_one_is_ended_as_cancelled(page, monkeypatch): + page.served["calls/begin"] = httpx.Response(500, text="the browser is restarting") + calls = [] + + async def call(tool, **arguments): + calls.append(tool) + if tool == "browser_wait_for": + await asyncio.Event().wait() + return CallToolResult(content=[TextContent(type="text", text="ok")]) + + monkeypatch.setattr(page._playwright, "call", call) + unlogged = await page._playwright_call("browser_snapshot") + page.served["calls/begin"] = {"entry": 8} + waiting = asyncio.ensure_future(page._playwright_call("browser_wait_for", time=60)) + await asyncio.sleep(0.05) + waiting.cancel() + with pytest.raises(asyncio.CancelledError): + await waiting + + assert calls == ["browser_snapshot", "browser_wait_for"] and unlogged.content[0].text == "ok" + assert [op for op, _ in page.sent] == ["calls/begin", "calls/begin", "calls/end"] + assert page.sent[-1] == ("calls/end", {"entry": 8, "ok": False, "error": "cancelled"}) + + @pytest.mark.asyncio async def test_it_asks_the_browser_for_playwright_mcps_address_under_its_own_loopback_origin_as_its_own_call( page, monkeypatch): From 20c6327929dfad845bb2110ad71e0370e438f416 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Wed, 11 Nov 2026 16:39:30 -0800 Subject: [PATCH 2/3] fix(drivers): bound a browser_* call from when it asks for its session A call's entry in the browser's log was bounded by the call's own limits, but not the wait for the session lock while another caller's Playwright MCP starts, so a call within every limit could outlast its entry and lose its credit. The whole call is bounded now, from when it asks for its session, and the browser waits that and the closing request's time. A session whose opening is given up on is closed rather than left half open. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/drivers/envs/page/driver.py | 5 +++- .../drivers/envs/page/playwright_mcp.py | 28 +++++++++++++------ tst/unit/drivers/page_driver_test.py | 22 +++++++++++++-- 3 files changed, 44 insertions(+), 11 deletions(-) diff --git a/src/agent_env/drivers/envs/page/driver.py b/src/agent_env/drivers/envs/page/driver.py index fe3e62d6b..9e0d514c8 100644 --- a/src/agent_env/drivers/envs/page/driver.py +++ b/src/agent_env/drivers/envs/page/driver.py @@ -89,6 +89,9 @@ BLANK = b"\xff\xff\xff" #: How long one call to the browser's server may take. CALL_TIMEOUT_S = 30.0 +#: How long the browser waits for a browser_* call's end before writing it as never said to have ended: the call's own +#: bound, then its closing request's. +CALL_ENDS_WITHIN_S = playwright_mcp.CALL_BOUND_S + CALL_TIMEOUT_S @computer_driver(name="page", supports=[{"os": {"type": "linux", "display": ["none"]}, "arch": ["amd64", "arm64"]}]) @@ -133,7 +136,7 @@ async def _playwright_call(self, tool: str, /, **arguments: Any) -> CallToolResu async def _call_begun(self, tool: str) -> Optional[int]: params = {"user": playwright_mcp.CDP_USER, "acts": playwright_mcp.acts(tool), - "within_s": playwright_mcp.CALL_BOUND_S} + "within_s": CALL_ENDS_WITHIN_S} try: return (await self._call("calls/begin", params))["entry"] except (DeviceUnavailable, InvalidArgument, Unsupported) as e: diff --git a/src/agent_env/drivers/envs/page/playwright_mcp.py b/src/agent_env/drivers/envs/page/playwright_mcp.py index 4e24e9839..40d130046 100644 --- a/src/agent_env/drivers/envs/page/playwright_mcp.py +++ b/src/agent_env/drivers/envs/page/playwright_mcp.py @@ -46,9 +46,9 @@ #: before it's killed. POLL_S = 0.1 STOP_S = 10.0 -#: How long one call may take at most, its start and its session's together with its own bound: as long as the browser -#: credits Playwright MCP's commands to it unless told it ended sooner. -CALL_BOUND_S = 2 * START_S + CALL_TIMEOUT_S +#: How long one call may take at most, from when it asks for its session to its answer, however long it waits for +#: another's start: a stop, a start and a session's opening, then its own bound. +CALL_BOUND_S = STOP_S + 2 * START_S + CALL_TIMEOUT_S #: How long a caller's session on Playwright MCP is kept with no call: a caller's own MCP session ends with no word to #: the driver, and each holds a connection to the browser. SESSION_IDLE_S = 3600.0 @@ -89,8 +89,17 @@ def __init__(self, endpoint: Callable[[], Awaitable[str]]) -> None: self._sessions: dict[Optional[str], _Session] = {} async def call(self, tool: str, /, **arguments: Any) -> CallToolResult: - """``tool`` called on Playwright MCP as it answers, or refused saying why it couldn't be. A call Playwright MCP - or its session ends under fails then, and isn't sent again, since it may have acted.""" + """``tool`` called on Playwright MCP as it answers, or refused saying why it couldn't be, within CALL_BOUND_S + of asking for its session. A call Playwright MCP or its session ends under, or that runs past its bound, fails + then, and isn't sent again, since it may have acted.""" + try: + async with asyncio.timeout(CALL_BOUND_S): + return await self._answer(tool, **arguments) + except TimeoutError: + return _refused(f"Playwright MCP didn't answer {tool} within {CALL_BOUND_S:g} s of its asking for a " + "session, which may have acted") + + async def _answer(self, tool: str, /, **arguments: Any) -> CallToolResult: try: held, process = await self._session(ToolContext.current().caller.session) except DeviceUnavailable as e: @@ -189,16 +198,19 @@ def __init__(self, url: str) -> None: @classmethod async def open(cls, url: str) -> _Session: - """A session on Playwright MCP at ``url``; DeviceUnavailable when it doesn't open one in time, wedged.""" + """A session on Playwright MCP at ``url``; DeviceUnavailable when it doesn't open one in time, wedged. One whose + opening is given up on is closed, not left half open.""" held = cls(url) ready: asyncio.Future = asyncio.get_running_loop().create_future() held.owner = asyncio.create_task(held._own(ready)) try: held.session = await asyncio.wait_for(ready, START_S) - except asyncio.TimeoutError: + except BaseException as e: held.close.set() held.owner.cancel() - raise DeviceUnavailable(f"Playwright MCP didn't open a session within {START_S:.0f} s") + if isinstance(e, TimeoutError): + raise DeviceUnavailable(f"Playwright MCP didn't open a session within {START_S:.0f} s") from None + raise return held async def _own(self, ready: asyncio.Future) -> None: diff --git a/tst/unit/drivers/page_driver_test.py b/tst/unit/drivers/page_driver_test.py index 1ab736567..c017a4511 100644 --- a/tst/unit/drivers/page_driver_test.py +++ b/tst/unit/drivers/page_driver_test.py @@ -328,9 +328,9 @@ async def call(tool, **arguments): with bound(ToolContext(tool="browser_snapshot", call_id="c2", mcp=True)): failed = await page._playwright_call("browser_snapshot") - assert page.sent == [("calls/begin", {"user": "playwright-mcp", "acts": True, "within_s": 360.0}), + assert page.sent == [("calls/begin", {"user": "playwright-mcp", "acts": True, "within_s": 400.0}), ("calls/end", {"entry": 7, "ok": True}), - ("calls/begin", {"user": "playwright-mcp", "acts": False, "within_s": 360.0}), + ("calls/begin", {"user": "playwright-mcp", "acts": False, "within_s": 400.0}), ("calls/end", {"entry": 7, "ok": False, "error": "no such element"})] assert TOOL_CALL_HEADER in page.headers[0] and TOOL_CALL_HEADER not in page.headers[1] assert clicked.content[0].text == "clicked" and failed.isError @@ -537,6 +537,24 @@ async def test_a_call_slower_than_its_bound_is_refused_and_its_session_opened_af assert first.close.is_set() and not after.isError +@pytest.mark.asyncio +async def test_a_call_is_bounded_from_when_it_asks_for_its_session_and_leaves_no_session_half_open(stand_in, monkeypatch): + module, playwright, endpoints, called = stand_in + monkeypatch.setattr(module, "CALL_BOUND_S", 1.0) + try: + async with playwright._lock: + waited = await asyncio.wait_for(called("ada"), 10) + endpoints.append("ws://127.0.0.1:18765/svc/chromium/cdp/wedged-once") + opening = await asyncio.wait_for(called("bo"), 10) + await asyncio.sleep(0.1) + owners = [task for task in asyncio.all_tasks() if task.get_coro().__name__ == "_own"] + finally: + await playwright._stop() + + assert waited.isError and "didn't answer browser_snapshot within 1 s" in waited.content[0].text + assert opening.isError and "within 1 s" in opening.content[0].text and owners == [] + + @pytest.mark.asyncio async def test_a_playwright_mcp_that_never_opens_a_session_is_stopped_and_the_next_call_starts_afresh( stand_in, monkeypatch, tmp_path): From 99b0948db758f19da0986ac02b364d7f95a49a2d Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Wed, 11 Nov 2026 16:57:36 -0800 Subject: [PATCH 3/3] test(browser): the browser's card lists calls/begin and calls/end Co-Authored-By: Claude Opus 5.5 --- tst/integration/computer/website_browser_test.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tst/integration/computer/website_browser_test.py b/tst/integration/computer/website_browser_test.py index f2b0f7a58..da702a8cf 100644 --- a/tst/integration/computer/website_browser_test.py +++ b/tst/integration/computer/website_browser_test.py @@ -155,12 +155,11 @@ async def test_chromium_runs_with_agentenvs_flags_on_its_pipe_and_a_reset_relaun policies["stdout"]).items() params = protocol_v1.extension_params(card, BROWSER) assert (params["chromium"], params["clock"], sorted(params["methods"])) == ( - "156.0.8078.4", "real", ["cdp", "cookies/delete", "cookies/get", "cookies/set", "dialog/answer", "downloads/get", - "downloads/list", "files/choose", "history", "local_storage/clear", - "local_storage/get", "local_storage/set", "log", "navigate", "page/capture", - "page/input", - "page/state", "page/viewport", "record", "sites/set", "tabs/activate", "tabs/close", - "tabs/list", "tabs/open"]) + "156.0.8078.4", "real", ["calls/begin", "calls/end", "cdp", "cookies/delete", "cookies/get", "cookies/set", + "dialog/answer", "downloads/get", "downloads/list", "files/choose", "history", + "local_storage/clear", "local_storage/get", "local_storage/set", "log", "navigate", + "page/capture", "page/input", "page/state", "page/viewport", "record", "sites/set", + "tabs/activate", "tabs/close", "tabs/list", "tabs/open"]) assert readiness["ready"] is True, readiness assert relaunched["pid"] != browser["pid"] and relaunched["listening"] == 0 assert relaunched_readiness["ready"] is True, relaunched_readiness