From 046a8abcad410b795b24e05bb2820d5527aa30f6 Mon Sep 17 00:00:00 2001 From: jxxghp Date: Sat, 12 Sep 2026 13:31:23 +0800 Subject: [PATCH] fix(agent): harden browser session recovery --- app/adapters/network/browser.py | 11 ++++- app/agent/tools/impl/browse_webpage.py | 55 ++++++++++++++++-------- docs/refactor/agent-codex-parity-plan.md | 1 + tests/test_browser_helper.py | 23 ++++++++++ 4 files changed, 72 insertions(+), 18 deletions(-) diff --git a/app/adapters/network/browser.py b/app/adapters/network/browser.py index 37cd6e482..7271ef7c2 100644 --- a/app/adapters/network/browser.py +++ b/app/adapters/network/browser.py @@ -651,14 +651,21 @@ class BrowserSessionHelper: """ if tab_index < 0 or tab_index >= len(session.pages): raise ValueError(f"标签页索引不存在: {tab_index}") + active_index = session.active_index page = session.pages.pop(tab_index) + close_error: Exception | None = None try: page.close() except Exception as err: + close_error = err logger.warning(f"关闭浏览器标签页失败: {str(err)}") if not session.pages: session.pages.append(session.context.new_page()) - session.active_index = min(session.active_index, len(session.pages) - 1) + elif tab_index < active_index: + active_index -= 1 + session.active_index = min(active_index, len(session.pages) - 1) + if close_error is not None: + raise RuntimeError("关闭浏览器标签页失败") from close_error return BrowserSessionHelper.list_tabs(session) @staticmethod @@ -962,6 +969,8 @@ class BrowserSessionHelper: script = f""" () => {{ const limit = {int(max_elements)}; + document.querySelectorAll('[{cls.REF_ATTRIBUTE}]').forEach((el) => + el.removeAttribute('{cls.REF_ATTRIBUTE}')); const selector = [ 'a[href]', 'button', diff --git a/app/agent/tools/impl/browse_webpage.py b/app/agent/tools/impl/browse_webpage.py index 876394522..2c7f690f2 100644 --- a/app/agent/tools/impl/browse_webpage.py +++ b/app/agent/tools/impl/browse_webpage.py @@ -12,6 +12,7 @@ from pydantic import BaseModel, Field from app.adapters.network.browser import BrowserSessionHelper from app.agent.policy.contracts import ExecutionOutcome +from app.agent.policy.sanitizer import summarize_error from app.agent.terminal.ownership import current_terminal_scope from app.agent.tools.base import MoviePilotTool from app.agent.tools.result import inspect_tool_result @@ -22,6 +23,7 @@ from app.runtime.log import logger MAX_CONTENT_LENGTH = 12_000 # 默认超时时间（秒） DEFAULT_TIMEOUT = 30 +MAX_TIMEOUT = 300 # 截图最大宽度 SCREENSHOT_MAX_WIDTH = 1280 # 截图最大高度 @@ -107,7 +109,8 @@ class BrowseWebpageInput(BaseModel): description="Content type for 'get_content' action: 'text' for readable text, 'html' for raw HTML", ) timeout: Optional[int] = Field( - DEFAULT_TIMEOUT, description="Timeout in seconds for the action (default: 30)" + DEFAULT_TIMEOUT, ge=1, le=MAX_TIMEOUT, + description="Timeout in seconds for the action (default: 30, range: 1-300)" ) cookies: Optional[str] = Field( None, @@ -255,18 +258,26 @@ class BrowseWebpageTool(MoviePilotTool): ) try: + if timeout is None or type(timeout) is not int or not 1 <= timeout <= MAX_TIMEOUT: + return self._error_response( + "invalid_timeout", f"timeout 必须是 1 到 {MAX_TIMEOUT} 秒的整数。", + "修正 timeout 后重试，不要重复已经执行的浏览器动作。", + ) # 验证操作类型 try: browser_action = BrowserAction(action) except ValueError: valid_actions = ", ".join([a.value for a in BrowserAction]) - return f"错误: 不支持的操作类型 '{action}'，支持的操作: {valid_actions}" + return self._error_response( + "invalid_action", f"不支持的操作类型 '{action}'，支持的操作: {valid_actions}", + "从支持的 action 中选择后重试。", + ) # 参数校验 if browser_action == BrowserAction.GOTO and not url: - return "错误: 'goto' 操作需要提供 url 参数" + return self._error_response("missing_url", "'goto' 操作需要提供 url 参数", "补充 url 后重试。") if browser_action == BrowserAction.OPEN_TAB and not url: - return "错误: 'open_tab' 操作需要提供 url 参数" + return self._error_response("missing_url", "'open_tab' 操作需要提供 url 参数", "补充 url 后重试。") if ( browser_action in ( @@ -277,7 +288,7 @@ class BrowseWebpageTool(MoviePilotTool): ) and not selector ): - return f"错误: '{action}' 操作需要提供 selector 参数" + return self._error_response("missing_selector", f"'{action}' 操作需要提供 selector 参数", "补充 selector 后重试。") if ( browser_action in ( @@ -287,28 +298,28 @@ class BrowseWebpageTool(MoviePilotTool): ) and not ref ): - return f"错误: '{action}' 操作需要提供 ref 参数" + return self._error_response("missing_ref", f"'{action}' 操作需要提供 ref 参数", "先获取最新 snapshot，再补充 ref 后重试。") if browser_action == BrowserAction.FILL and value is None: - return "错误: 'fill' 操作需要提供 value 参数" + return self._error_response("missing_value", "'fill' 操作需要提供 value 参数", "补充 value 后重试。") if browser_action == BrowserAction.FILL_REF and value is None: - return "错误: 'fill_ref' 操作需要提供 value 参数" + return self._error_response("missing_value", "'fill_ref' 操作需要提供 value 参数", "补充 value 后重试。") if browser_action == BrowserAction.EVALUATE and not script: - return "错误: 'evaluate' 操作需要提供 script 参数" + return self._error_response("missing_script", "'evaluate' 操作需要提供 script 参数", "补充 script 后重试。") if ( browser_action == BrowserAction.EVALUATE and not await self.is_admin_user() ): - return "错误: 'evaluate' 操作仅允许管理员使用" + return self._error_response("admin_required", "'evaluate' 操作仅允许管理员使用", "改用只读浏览器 action 或请求管理员授权。") if ( browser_action == BrowserAction.GET_COOKIES and not await self.is_admin_user() ): - return "错误: 'get_cookies' 操作仅允许管理员使用" + return self._error_response("admin_required", "'get_cookies' 操作仅允许管理员使用", "改用非敏感浏览器 action 或请求管理员授权。") if ( browser_action in (BrowserAction.FOCUS_TAB, BrowserAction.CLOSE_TAB) and tab_index is None ): - return f"错误: '{action}' 操作需要提供 tab_index 参数" + return self._error_response("missing_tab_index", f"'{action}' 操作需要提供 tab_index 参数", "补充 tab_index 后重试。") effective_session_key = session_key or self._session_id @@ -332,10 +343,11 @@ class BrowseWebpageTool(MoviePilotTool): return result except Exception as e: - logger.error(f"浏览器操作失败: {e}", exc_info=True) + error_summary = summarize_error(e) + logger.error(f"浏览器操作失败: {error_summary}", exc_info=True) if action == BrowserAction.SCREENSHOT: return self._screenshot_failure("screenshot_failed", "浏览器截图执行失败") - return f"浏览器操作失败: {str(e)}" + return self._error_response("browser_operation_failed", f"浏览器操作失败: {error_summary}", "检查当前会话和页面状态后再重试。") def _execute_browser_action( self, @@ -402,10 +414,11 @@ class BrowseWebpageTool(MoviePilotTool): ) except Exception as e: - logger.error(f"CloakBrowser 执行失败: {e}", exc_info=True) + error_summary = summarize_error(e) + logger.error(f"CloakBrowser 执行失败: {error_summary}", exc_info=True) if browser_action == BrowserAction.SCREENSHOT: return self._screenshot_failure("screenshot_failed", "浏览器截图执行失败") - return f"CloakBrowser 执行失败: {str(e)}" + return self._error_response("browser_operation_failed", f"CloakBrowser 执行失败: {error_summary}", "检查当前会话和页面状态后再重试。") def _do_action( self, @@ -545,6 +558,14 @@ class BrowseWebpageTool(MoviePilotTool): ) return json.dumps(normalized, ensure_ascii=False, indent=2) + @staticmethod + def _error_response(code: str, message: str, recovery: str) -> str: + """返回不含敏感细节的结构化浏览器失败回执。""" + return BrowseWebpageTool._json_response({ + "success": False, "execution_outcome": "failed", "error": code, + "message": message, "recovery": recovery, + }) + @staticmethod def _action_goto( helper: BrowserSessionHelper, @@ -655,7 +676,7 @@ class BrowseWebpageTool(MoviePilotTool): # 等待可能的页面变化 try: - page.wait_for_load_state("networkidle", timeout=5000) + page.wait_for_load_state("networkidle", timeout=min(timeout * 1000, 5000)) except Exception: pass diff --git a/docs/refactor/agent-codex-parity-plan.md b/docs/refactor/agent-codex-parity-plan.md index 58a6c7855..6806b228d 100644 --- a/docs/refactor/agent-codex-parity-plan.md +++ b/docs/refactor/agent-codex-parity-plan.md @@ -114,4 +114,5 @@ uv run --locked --no-sync python -m scripts.evaluation \ - Shell 会话恢复阶段：一次性命令和后台会话的失败、未知、作用域拒绝、输出游标错误现在都返回对应恢复动作；未知退出状态优先要求读取现有 session，失败命令要求检查 `output/exit_code` 后修正，避免把输出文字当成功或重复启动副作用命令。文件工具的行号与版本冲突保护已在上一阶段验证；本阶段相关回归共 122 项通过。 - 浏览器与子代理状态阶段：浏览器所有成功/失败 JSON 统一带 `execution_outcome`；子代理任务的 running/cancelled 状态带明确的 `pending/failed` 语义和继续观察、取消后重新派发的恢复建议。当前阶段与文件、Shell、浏览器回归共 112 项通过；真实浏览器仍受用户保存权限限制，子代理终端共享仍需原生句柄闭环。 - 浏览器观察回执阶段：`goto`、`snapshot`、`get_content` 及交互动作统一显式返回 `success`，从而稳定映射为 `execution_outcome=succeeded/failed`；页面内容为空与浏览器执行失败可以区分。当前跨浏览器、文件、Shell、子代理回归共 125 项通过。 +- 浏览器会话可靠性阶段：直接 `run()` 的参数/异常现在返回结构化恢复合同并限制 timeout 为 1–300 秒；点击等待遵守调用 deadline；关闭标签页正确调整 active index，关闭异常不再伪报成功；生成新 snapshot 前清理旧 ref，动态页面不会把过期 ref 静默指向新元素。浏览器、文件、Shell、子代理相关回归共 155 项通过。 - 浏览器环境复核：启动本机 Chrome 后用同一官方 OAuth 配置重跑 `/tmp/moviepilot-agent-round-luna-oauth-native-browser-chrome-20260912.json`，原生已加载 Browser plugin、node_repl 并选择 Chrome；真实回环导航仍被保存的浏览器权限拒绝。该结果排除了“CLI 未登录或未启动 Chrome”的解释，下一次只有在用户允许对应回环地址后才继续导航、点击和 oracle 配对。 diff --git a/tests/test_browser_helper.py b/tests/test_browser_helper.py index ff211f65c..207414a70 100644 --- a/tests/test_browser_helper.py +++ b/tests/test_browser_helper.py @@ -14,6 +14,7 @@ import pytest from app.adapters.network.browser import ( BrowserSessionHelper, PlaywrightHelper, + _BrowserSessionState, launch_browser_context, launch_browser_context_async, ) @@ -499,6 +500,15 @@ def test_browse_webpage_returns_snapshot_with_refs_after_goto(): assert payload["interactive_elements"][0]["ref"] == "e1" +def test_snapshot_clears_previous_refs_before_assigning_new_refs() -> None: + """新快照应清理旧 ref，避免动态页面把过期引用映射到新元素。""" + page = MagicMock() + page.evaluate.return_value = [{"ref": "e1", "tag": "button"}] + assert BrowserSessionHelper._extract_interactive_elements(page, max_elements=5) + script = page.evaluate.call_args.args[0] + assert "removeAttribute('data-moviepilot-agent-ref')" in script + + def test_browse_webpage_click_ref_uses_snapshot_selector(): """click_ref 应将 ref 转换为快照注入的稳定选择器。""" page = _FakePage() @@ -527,6 +537,19 @@ def test_browse_webpage_click_ref_uses_snapshot_selector(): assert page.clicks == ['[data-moviepilot-agent-ref="e1"]'] +def test_close_tab_before_active_page_keeps_active_index() -> None: + """关闭活动页之前的标签页不能把活动页错误地指向下一页。""" + first_page = _FakePage("first") + pages = [first_page, _FakePage("active"), _FakePage("last")] + context = _FakeContext(pages) + session = _BrowserSessionState("close-index", context, pages, active_index=1) + tabs = BrowserSessionHelper.close_tab(session, 0) + assert session.active_index == 0 + assert session.active_page.page_id == "active" + assert tabs[0]["active"] is True + assert first_page.closed is True + + def test_browse_webpage_get_cookies_returns_current_domain_cookie_and_ua(): """管理员 Cookie 动作应只返回当前页面域名的会话字段。""" page = _FakePage()