From 7ce1b4a9bad9e7d08b10e8b6188c6cfe3d84cdae Mon Sep 17 00:00:00 2001 From: Oleg Date: Tue, 29 Sep 2026 12:41:13 +0200 Subject: [PATCH] fix(browser): render helper-tool guidance only for tools the worker has The Playwright worker system prompt was a static string that told the model to use browser_probe_interactives, browser_probe_cards, browser_batch_interact and browser_custom_action unconditionally. The tool list the model receives often lacks them: BrowserAgentRuntime never adds browser_custom_action / browser_list_custom_actions to the worker, and callers that narrow the surface (allowed_tool_names, ability removal after ensure_started(), as the WebVoyager harness does) drop the rest. Evidence (WebVoyager, 105 tasks, @playwright/mcp 0.0.78, agent-core 18b840cf): every request had tool_count=14 (CORE_BROWSER_TOOL_NAMES) while the prompt pushed probe/batch tools; the model noted on step 1 that those tools were not in its list. Tool use fell back to browser_find + browser_click pairs (evaluate 326, find 255, navigate 241, click 143 over 90 tasks); Google Flights averaged 47.8 tool calls per task (max 60, ~304 s against a 600 s timeout), Amazon 15.7, Booking 15.3; the prompt cost ~5.4k input tokens per request. Invariant: every tool name the worker prompt tells the model to use is in the tool list sent on that request. Guidance for an absent tool is not rendered at all, not even as a negation. - build_browser_worker_system_prompt() takes keyword-only tool_names and renders each helper fragment (probes, PageState/generation_id, batch_interact, the batch-vs-custom_action note) only when its tools are present. tool_names=None keeps every fragment. - refresh_browser_worker_system_prompt() re-renders the worker's prompt template from ability_manager.list_tool_info(), the same call ReActAgent uses to build the request's tool list. BrowserService calls it before each worker invoke, so helpers added by the runtime and abilities removed after ensure_started() are both reflected. Full tool surface: prompt unchanged (byte-identical, pinned by sha256). The conditional browser_run_code / browser_take_screenshot wording is left as it was. --- .../browser_move/playwright_runtime/agents.py | 193 +++++++++++----- .../playwright_runtime/service.py | 8 +- .../test_browser_prompt_tool_surface.py | 210 ++++++++++++++++++ 3 files changed, 358 insertions(+), 53 deletions(-) create mode 100644 tests/unit_tests/harness/tools/browser_move/test_browser_prompt_tool_surface.py diff --git a/openjiuwen/harness/tools/browser_move/playwright_runtime/agents.py b/openjiuwen/harness/tools/browser_move/playwright_runtime/agents.py index 10153c38dc..23bede364b 100644 --- a/openjiuwen/harness/tools/browser_move/playwright_runtime/agents.py +++ b/openjiuwen/harness/tools/browser_move/playwright_runtime/agents.py @@ -166,61 +166,130 @@ def _normalize_execute_call(args: tuple[Any, ...], kwargs: dict[str, Any]) -> tu return ctx, tool_call, session, tag, extra_kwargs +_BROWSER_WORKER_HELPER_TOOL_NAMES = ( + "browser_probe_interactives", + "browser_probe_cards", + "browser_batch_interact", + "browser_custom_action", + "browser_list_custom_actions", +) + + def build_browser_worker_system_prompt( screenshot_subdir: str = "screenshots", artifacts_subdir: str = "artifacts", + *, + tool_names: Optional[Iterable[str]] = None, ) -> str: + """Render the worker system prompt; helper guidance only for helpers in ``tool_names`` (``None``: all).""" + available = set(_BROWSER_WORKER_HELPER_TOOL_NAMES if tool_names is None else tool_names) + interactives = "browser_probe_interactives" in available + cards = "browser_probe_cards" in available + probes = interactives or cards + batch = "browser_batch_interact" in available + batch_vs_custom_action = batch and interactives and cards and "browser_custom_action" in available + helper_guidance = "".join( + text + for enabled, text in ( + ( + probes, + "Before broad page snapshots, full-body scans, or generic DOM scraping, " + "choose the smallest compact probe that matches the task. ", + ), + ( + interactives, + "Use browser_probe_interactives for buttons, links, inputs, forms, navigation controls, " + "login controls, pagination controls, menus, and other visible interactive elements. " + "When using browser_probe_interactives only for page-level controls, prefer max_items around 20-30 " + "unless the task explicitly requires a larger inventory. ", + ), + ( + cards, + "On product pages, marketplace pages, search-result pages, catalog pages, article-list pages, " + "or any page with repeated visible cards/listings, call browser_probe_cards before broad extraction. " + "Use browser_probe_cards to identify compact repeated structures such as product cards, result cards, " + "book cards, article cards, listing rows, title/author/source/summary, " + "title/price/rating/review/availability fields, primary links, " + "visible controls, generation-scoped target IDs, and recurring structure signatures. ", + ), + ( + cards and interactives, + "For product/listing/item-data tasks, prefer browser_probe_cards first; call " + "browser_probe_interactives " + "only if you also need page-level navigation, filters, forms, or controls outside the cards. ", + ), + ( + cards, + "If browser_probe_cards returns the fields needed for the task, including article/result-row " + "title, link, author/source, or summary fields, use that compact result as the evidence " + "for extraction and final reporting instead of taking screenshots, snapshots, or running extra DOM " + "scans. " + "After a successful card probe, use browser_evaluate, or a browser_run_code tool only when it is " + "explicitly present, for a clearly missing required field or a specific target-based action; " + "do not repeat broad evaluation " + "just to re-read the same cards. ", + ), + ( + probes, + "Treat page_id/generation_id/url/title/interactives/cards/" + "field_coverage/blockers as one PageState contract. ", + ), + ( + probes and batch, + "Pass the current generation_id with a " + "probe target_id directly to browser_batch_interact; never translate it into guessed CSS. " + "Explicit selectors are reserved for validated read-only extraction and condition waits. ", + ), + ( + cards, + "If a card has primary_link or href, navigate to it directly. ", + ), + ( + batch_vs_custom_action, + "browser_batch_interact is a standalone runtime helper like browser_probe_interactives and " + "browser_probe_cards, not a browser_custom_action wrapper. Use it directly. ", + ), + ( + batch, + "For multi-field forms with several known controls, search flows with autocomplete, " + "dropdown/date-picker " + "flows, filter panels, or any short sequence where two or more next click/type/wait/extract steps are " + "already known, call browser_batch_interact before falling back to repeated " + "browser_click/browser_type and separate wait turns. " + "Extract multiple fields from the same page in one batch with named extract_text/extract_value " + "steps, or use one card probe result that already contains every requested field. " + "Do not force browser_batch_interact for a single uncertain click, one simple text field, or a page " + "state " + "that still needs inspection. Prefer generation-scoped target_id in batch steps. " + "Use autocomplete steps for type-then-choose widgets, and condition-based " + "wait_for_selector/wait_for_text " + "steps instead of fixed sleeps. For dynamic pages use wait_for_url, " + "wait_for_first_card_title, wait_for_sort_state, wait_for_result_count, " + "wait_for_dom_text_change, wait_for_stable, or wait_for_tab with a short poll interval and total " + "timeout. " + "When an action opens a new tab, use click plus wait_for_tab in the same batch; wait_for_tab " + "activates the matched tab by default. Do not wait for a new URL on the old tab. ", + ), + ( + True, + "Interpret 'fully loaded' or an 'appropriate pause' as condition stability, not a fixed sleep. ", + ), + ( + batch, + "Do not split click+type+wait, click+wait+click, " + "date open+choose, or search submit+result wait into separate ReAct turns unless " + "browser_batch_interact " + "failed or the next target is genuinely unknown. ", + ), + ) + if enabled + ) return ( "You are a browser worker agent.\n" "Execute browser tasks step-by-step with Playwright MCP tools and approved runtime helper tools only.\n" "Before interacting, ensure page or selector readiness.\n" "Keep actions targeted and avoid unnecessary page snapshots.\n" - "Before broad page snapshots, full-body scans, or generic DOM scraping, " - "choose the smallest compact probe that matches the task. " - "Use browser_probe_interactives for buttons, links, inputs, forms, navigation controls, " - "login controls, pagination controls, menus, and other visible interactive elements. " - "When using browser_probe_interactives only for page-level controls, prefer max_items around 20-30 " - "unless the task explicitly requires a larger inventory. " - "On product pages, marketplace pages, search-result pages, catalog pages, article-list pages, " - "or any page with repeated visible cards/listings, call browser_probe_cards before broad extraction. " - "Use browser_probe_cards to identify compact repeated structures such as product cards, result cards, " - "book cards, article cards, listing rows, title/author/source/summary, " - "title/price/rating/review/availability fields, primary links, " - "visible controls, generation-scoped target IDs, and recurring structure signatures. " - "For product/listing/item-data tasks, prefer browser_probe_cards first; call browser_probe_interactives " - "only if you also need page-level navigation, filters, forms, or controls outside the cards. " - "If browser_probe_cards returns the fields needed for the task, including article/result-row " - "title, link, author/source, or summary fields, use that compact result as the evidence " - "for extraction and final reporting instead of taking screenshots, snapshots, or running extra DOM scans. " - "After a successful card probe, use browser_evaluate, or a browser_run_code tool only when it is " - "explicitly present, for a clearly missing required field or a specific target-based action; " - "do not repeat broad evaluation " - "just to re-read the same cards. Treat page_id/generation_id/url/title/interactives/cards/" - "field_coverage/blockers as one PageState contract. Pass the current generation_id with a " - "probe target_id directly to browser_batch_interact; never translate it into guessed CSS. " - "Explicit selectors are reserved for validated read-only extraction and condition waits. " - "If a card has primary_link " - "or href, navigate to it directly. " - "browser_batch_interact is a standalone runtime helper like browser_probe_interactives and " - "browser_probe_cards, not a browser_custom_action wrapper. Use it directly. " - "For multi-field forms with several known controls, search flows with autocomplete, dropdown/date-picker " - "flows, filter panels, or any short sequence where two or more next click/type/wait/extract steps are " - "already known, call browser_batch_interact before falling back to repeated " - "browser_click/browser_type and separate wait turns. " - "Extract multiple fields from the same page in one batch with named extract_text/extract_value " - "steps, or use one card probe result that already contains every requested field. " - "Do not force browser_batch_interact for a single uncertain click, one simple text field, or a page state " - "that still needs inspection. Prefer generation-scoped target_id in batch steps. " - "Use autocomplete steps for type-then-choose widgets, and condition-based wait_for_selector/wait_for_text " - "steps instead of fixed sleeps. For dynamic pages use wait_for_url, " - "wait_for_first_card_title, wait_for_sort_state, wait_for_result_count, " - "wait_for_dom_text_change, wait_for_stable, or wait_for_tab with a short poll interval and total timeout. " - "When an action opens a new tab, use click plus wait_for_tab in the same batch; wait_for_tab " - "activates the matched tab by default. Do not wait for a new URL on the old tab. " - "Interpret 'fully loaded' or an 'appropriate pause' as condition stability, not a fixed sleep. " - "Do not split click+type+wait, click+wait+click, " - "date open+choose, or search submit+result wait into separate ReAct turns unless browser_batch_interact " - "failed or the next target is genuinely unknown. " + f"{helper_guidance}" "Keep using browser_fill_form for ordinary visible text fields when that official tool is enough. " "Use browser_snapshot only when the compact probes are insufficient, when accessibility structure is needed, " "or when you need exact element references required by a Playwright MCP action. " @@ -325,6 +394,10 @@ async def execute_with_timeout(*args, **kwargs): setattr(agent.ability_manager.execute, "_playwright_timeout_wrapped", True) +def _normalize_output_subdir(subdir: str, default: str) -> str: + return (subdir or default).strip().replace("\\", "/").strip("/") or default + + # pylint: disable=too-many-arguments def build_browser_worker_agent( provider: str, @@ -348,12 +421,8 @@ def build_browser_worker_agent( "Playwright worker received tool allowlist: %s", resolved_allowed_tool_names, ) - screenshot_subdir = ( - (screenshot_subdir or "screenshots").strip().replace("\\", "/").strip("/") or "screenshots" - ) - artifacts_subdir = ( - (artifacts_subdir or "artifacts").strip().replace("\\", "/").strip("/") or "artifacts" - ) + screenshot_subdir = _normalize_output_subdir(screenshot_subdir, "screenshots") + artifacts_subdir = _normalize_output_subdir(artifacts_subdir, "artifacts") card = AgentCard( id="agent.playwright.browser_worker", name="playwright_browser_worker", @@ -395,3 +464,23 @@ def build_browser_worker_agent( ) ensure_execute_signature_compat(agent, tool_result_observer=tool_result_observer) return agent + + +async def refresh_browser_worker_system_prompt( + agent: ReActAgent, + *, + screenshot_subdir: str = "screenshots", + artifacts_subdir: str = "artifacts", +) -> None: + """Re-render the worker system prompt for the tools its next invoke sends to the model.""" + tool_names = tuple(tool.name for tool in await agent.ability_manager.list_tool_info()) + logger.info( + "Playwright worker prompt rendered for helper tools: %s", + tuple(name for name in _BROWSER_WORKER_HELPER_TOOL_NAMES if name in tool_names), + ) + prompt = build_browser_worker_system_prompt( + _normalize_output_subdir(screenshot_subdir, "screenshots"), + _normalize_output_subdir(artifacts_subdir, "artifacts"), + tool_names=tool_names, + ) + agent.config.configure_prompt_template([{"role": "system", "content": prompt}]) diff --git a/openjiuwen/harness/tools/browser_move/playwright_runtime/service.py b/openjiuwen/harness/tools/browser_move/playwright_runtime/service.py index 613753f73d..966fb5f015 100644 --- a/openjiuwen/harness/tools/browser_move/playwright_runtime/service.py +++ b/openjiuwen/harness/tools/browser_move/playwright_runtime/service.py @@ -27,7 +27,7 @@ from ..drivers.managed_browser import ManagedBrowserDriver, _default_chrome_user_data_dir from ..utils.parsing import extract_json_object -from .agents import build_browser_worker_agent +from .agents import build_browser_worker_agent, refresh_browser_worker_system_prompt from .config import BrowserInstanceConfig, BrowserRunGuardrails, parse_command_args, resolve_playwright_mcp_cwd from .profiles import BrowserProfile, BrowserProfileStore from .service_registry import BROWSER_SERVICE_REGISTRY, BrowserServiceIdentity @@ -1533,6 +1533,12 @@ async def _run_task_once(self, task: str, session_id: str, request_id: str) -> D f"Task:\n{task}\n\n" "Perform the task in the current logical browser session/tab for this session id." ) + if isinstance(self._browser_agent, ReActAgent): + await refresh_browser_worker_system_prompt( + self._browser_agent, + screenshot_subdir=self._screenshot_subdir, + artifacts_subdir=self._artifacts_subdir, + ) worker_conversation_id = self._build_worker_conversation_id(session_id, request_id) token_session = _ctx_observer_session_id.set(session_id) token_request = _ctx_observer_request_id.set(request_id) diff --git a/tests/unit_tests/harness/tools/browser_move/test_browser_prompt_tool_surface.py b/tests/unit_tests/harness/tools/browser_move/test_browser_prompt_tool_surface.py new file mode 100644 index 0000000000..f025a7cac0 --- /dev/null +++ b/tests/unit_tests/harness/tools/browser_move/test_browser_prompt_tool_surface.py @@ -0,0 +1,210 @@ +# coding: utf-8 +# Copyright (c) Huawei Technologies Co., Ltd. 2026. All rights reserved. +"""The browser worker prompt may only name tools the worker receives on that request.""" + +from __future__ import annotations + +import asyncio +import hashlib +import json +import re +from typing import Any +from unittest.mock import MagicMock, patch + +import pytest + +from openjiuwen.core.foundation.llm import AssistantMessage +from openjiuwen.core.foundation.tool import McpServerConfig, ToolInfo +from openjiuwen.core.foundation.tool.mcp.base import mcp_model_tool_prefix +from openjiuwen.harness.tools.browser_move.playwright_runtime.agents import ( + build_browser_worker_agent, + build_browser_worker_system_prompt, +) +from openjiuwen.harness.tools.browser_move.playwright_runtime.browser_capabilities import ( + CORE_BROWSER_TOOL_NAMES, +) +from openjiuwen.harness.tools.browser_move.playwright_runtime.config import BrowserRunGuardrails +from openjiuwen.harness.tools.browser_move.playwright_runtime.runtime_tools import ( + BrowserBatchInteractTool, + BrowserCustomActionTool, + BrowserListActionsTool, + BrowserProbeCardsTool, + BrowserProbeInteractivesTool, +) +from openjiuwen.harness.tools.browser_move.playwright_runtime.service import BrowserService + +HELPER_TOOLS = { + "browser_probe_interactives": BrowserProbeInteractivesTool, + "browser_probe_cards": BrowserProbeCardsTool, + "browser_batch_interact": BrowserBatchInteractTool, + "browser_custom_action": BrowserCustomActionTool, + "browser_list_custom_actions": BrowserListActionsTool, +} +# The helpers BrowserAgentRuntime.ensure_started() actually gives the worker. +RUNTIME_WORKER_HELPERS = ("browser_probe_interactives", "browser_probe_cards", "browser_batch_interact") +# Only mentioned in conditional wording ("only when it is present"). +CONDITIONAL_MENTIONS = {"browser_run_code", "browser_take_screenshot"} +# Prompt with the full tool surface at 18b840cf; it must stay byte-identical. +FULL_SURFACE_PROMPT_SHA256 = "a814c53b1b2571d807ff0bba21411541397f281390bdb82497a31554fdeb9f89" + +_SERVER_NAME = "playwright-official" +_MCP_SERVER_TOOL_NAMES = CORE_BROWSER_TOOL_NAMES + ("browser_take_screenshot", "browser_wait_for") +_FINAL_JSON = json.dumps( + {"ok": True, "final": "done", "page": {"url": "", "title": ""}, "screenshot": None, "error": None} +) + + +def _render(tool_names: tuple[str, ...] | None) -> str: + return build_browser_worker_system_prompt("screenshots", "artifacts", tool_names=tool_names) + + +def _mentioned_tools(prompt: str) -> set[str]: + return set(re.findall(r"browser_\w+", prompt)) - CONDITIONAL_MENTIONS + + +class _RecordingLLM: + """Answers every request with final JSON and records what the model was sent.""" + + def __init__(self) -> None: + self.requests: list[dict[str, Any]] = [] + + async def invoke(self, *args: Any, **kwargs: Any) -> AssistantMessage: + del args + self.requests.append(kwargs) + return AssistantMessage(content=_FINAL_JSON) + + +async def _fake_mcp_tool_infos(*args: Any, **kwargs: Any) -> list[ToolInfo]: + del args, kwargs + return [ + ToolInfo(name=name, description=f"{name} tool", parameters={"type": "object", "properties": {}}) + for name in _MCP_SERVER_TOOL_NAMES + ] + + +def _run_worker_request(helpers_added: tuple[str, ...], helpers_removed: tuple[str, ...]) -> tuple[str, set[str]]: + """Build the worker as the service does, run one task, return (system prompt, tool names) of the request.""" + mcp_cfg = McpServerConfig( + server_id="playwright_official_stdio", + server_name=_SERVER_NAME, + server_path="stdio://playwright", + client_type="stdio", + ) + worker = build_browser_worker_agent( + provider="openai", + api_key="mock-api-key", + api_base="https://example.invalid/v1", + model_name="mock-model", + mcp_cfg=mcp_cfg, + max_steps=2, + allowed_tool_names=CORE_BROWSER_TOOL_NAMES, + ) + llm = _RecordingLLM() + worker.set_llm(llm) + # Mirrors BrowserAgentRuntime.ensure_started(): helper cards are added after the worker is built. + for name in helpers_added: + worker.ability_manager.add(HELPER_TOOLS[name](MagicMock(), language="en").card) + # Mirrors the WebVoyager harness, which strips helpers after ensure_started(). + for name in helpers_removed: + worker.ability_manager.remove_ability(name) + + service = BrowserService( + provider="openai", + api_key="mock-api-key", + api_base="https://example.invalid/v1", + model_name="mock-model", + mcp_cfg=mcp_cfg, + guardrails=BrowserRunGuardrails(max_steps=2, max_failures=1, timeout_s=30), + ) + service.browser_agent = worker + with patch( + "openjiuwen.core.runner.Runner.resource_mgr.get_mcp_tool_infos", + side_effect=_fake_mcp_tool_infos, + ): + result = asyncio.run(service.run_task_once(task="Open example.com", session_id="s-1", request_id="r-1")) + + assert result["ok"] is True + assert llm.requests, "the worker never called the model" + request = llm.requests[0] + system_prompt = "\n".join( + str(message.content) for message in request["messages"] if getattr(message, "role", "") == "system" + ) + prefix = mcp_model_tool_prefix(_SERVER_NAME) + tool_names = {tool.name.removeprefix(prefix) for tool in request["tools"] or []} + return system_prompt, tool_names + + +class TestWorkerRequestPromptMatchesTools: + def test_narrowed_surface_prompt_names_no_helper(self) -> None: + system_prompt, tool_names = _run_worker_request( + helpers_added=RUNTIME_WORKER_HELPERS, + helpers_removed=tuple(HELPER_TOOLS), + ) + + assert tool_names == set(CORE_BROWSER_TOOL_NAMES) + for fragment in (*HELPER_TOOLS, "generation_id", "target_id"): + assert fragment not in system_prompt, f"{fragment!r} leaked into the narrowed worker prompt" + + @pytest.mark.parametrize( + "helpers_added, helpers_removed", + [ + pytest.param(tuple(HELPER_TOOLS), (), id="all-helpers"), + pytest.param(RUNTIME_WORKER_HELPERS, (), id="runtime-default"), + pytest.param(RUNTIME_WORKER_HELPERS, tuple(HELPER_TOOLS), id="harness-narrowed"), + pytest.param(("browser_probe_cards",), (), id="cards-only"), + ], + ) + def test_every_named_tool_is_sent( + self, + helpers_added: tuple[str, ...], + helpers_removed: tuple[str, ...], + ) -> None: + system_prompt, tool_names = _run_worker_request(helpers_added, helpers_removed) + + missing = _mentioned_tools(system_prompt) - tool_names + assert not missing, f"prompt names tools the model was not given: {sorted(missing)}" + + def test_runtime_default_surface_does_not_name_custom_action(self) -> None: + # BrowserAgentRuntime registers browser_custom_action but never adds it to the worker. + system_prompt, tool_names = _run_worker_request(RUNTIME_WORKER_HELPERS, ()) + + assert set(RUNTIME_WORKER_HELPERS) <= tool_names + assert "browser_custom_action" not in tool_names + assert "browser_custom_action" not in system_prompt + assert "browser_probe_cards before broad extraction" in system_prompt + assert "call browser_batch_interact before falling back" in system_prompt + + +class TestWorkerPromptRendering: + def test_full_surface_prompt_is_unchanged(self) -> None: + full_surface = CORE_BROWSER_TOOL_NAMES + tuple(HELPER_TOOLS) + + prompt = _render(full_surface) + + assert hashlib.sha256(prompt.encode("utf-8")).hexdigest() == FULL_SURFACE_PROMPT_SHA256 + assert prompt == _render(None) == build_browser_worker_system_prompt() + assert ( + "browser_batch_interact is a standalone runtime helper like browser_probe_interactives and " + "browser_probe_cards, not a browser_custom_action wrapper. Use it directly. " + ) in prompt + + def test_core_surface_prompt_names_only_core_tools(self) -> None: + prompt = _render(CORE_BROWSER_TOOL_NAMES) + + assert _mentioned_tools(prompt) <= set(CORE_BROWSER_TOOL_NAMES) + assert "generation_id" not in prompt + assert "target_id" not in prompt + assert "PageState" not in prompt + assert "Final output MUST be a single JSON object" in prompt + assert "Use a browser_run_code tool only when it is present" in prompt + + def test_partial_surface_renders_only_present_helper(self) -> None: + prompt = _render(CORE_BROWSER_TOOL_NAMES + ("browser_probe_cards",)) + + assert "call browser_probe_cards before broad extraction" in prompt + assert "Use browser_probe_cards to identify compact repeated structures" in prompt + assert "PageState contract" in prompt + assert "browser_probe_interactives" not in prompt + assert "browser_batch_interact" not in prompt + assert "browser_custom_action" not in prompt + assert _mentioned_tools(prompt) <= set(CORE_BROWSER_TOOL_NAMES) | {"browser_probe_cards"}