From 2b415cc1b9ee919f57527645fe665b7ce351fc8a Mon Sep 17 00:00:00 2001 From: jxxghp Date: Sat, 10 Oct 2026 17:00:56 +0800 Subject: [PATCH] fix(cli): unwrap MCP REST tool responses --- app/cli.py | 50 ++-- docs/cli.md | 7 + docs/refactor/optimization-checklist.md | 2 +- moviepilot | 4 +- .../fixtures/architecture/mypy-baseline.json | 2 +- tests/test_cli_tool_gateway.py | 231 +++++++++++++++--- 6 files changed, 238 insertions(+), 58 deletions(-) diff --git a/app/cli.py b/app/cli.py index 191c678a6..a5e7b6ed1 100644 --- a/app/cli.py +++ b/app/cli.py @@ -670,6 +670,7 @@ def _print_json(value: Any) -> None: def _parse_tool_result(result: Any) -> Any: + """解析工具返回的 JSON 文本，普通文本保持原样。""" if not isinstance(result, str): return result try: @@ -679,13 +680,26 @@ def _parse_tool_result(result: Any) -> Any: def _tool_request_headers(runtime: Optional[Dict[str, Any]] = None) -> Dict[str, str]: + """使用本地 API_TOKEN 认证工具请求，缺少配置时明确报错。""" api_token = _runtime_api_token(runtime) if not api_token: raise click.ClickException("本地配置中未找到 API_TOKEN，请先配置后再使用 tool/scheduler 命令") return {"X-API-KEY": api_token} +def _tool_response_data(response: Dict[str, Any], error_message: str) -> Any: + """校验工具 REST 响应并解包；错误只显示原因，避免倾倒大体积响应。""" + payload = response.get("json") + if not isinstance(payload, dict): + raise click.ClickException(f"{error_message}（HTTP {response['status']}，响应格式错误）") + if response["status"] not in {200, 201} or "success" not in payload: + message = payload.get("message") or payload.get("error") or payload.get("detail") + raise click.ClickException(message or f"{error_message}（HTTP {response['status']}）") + return _unwrap_api_data(payload) + + def _call_tool(tool_name: str, arguments: Dict[str, Any], runtime: Optional[Dict[str, Any]] = None) -> Any: + """解包 REST 的 data.result，再解析工具自身的文本或业务响应。""" response = _http_request( "POST", "/api/v1/mcp/tools/call", @@ -694,16 +708,14 @@ def _call_tool(tool_name: str, arguments: Dict[str, Any], runtime: Optional[Dict timeout=30.0, runtime=runtime, ) - payload = response.get("json") or {} - if response["status"] not in {200, 201}: - message = payload.get("error") or payload.get("detail") or response["text"] or "调用工具失败" - raise click.ClickException(message) - if not payload.get("success"): - raise click.ClickException(payload.get("error") or "调用工具失败") - return _parse_tool_result(payload.get("result")) + data = _tool_response_data(response, "调用工具失败") + if not isinstance(data, dict) or not isinstance(data.get("result"), str): + raise click.ClickException("调用工具失败：响应缺少有效的 data.result") + return _parse_tool_result(data["result"]) def _load_tool(tool_name: str, runtime: Optional[Dict[str, Any]] = None) -> Dict[str, Any]: + """读取统一 REST 信封中的工具详情，并保留工具不存在的错误提示。""" response = _http_request( "GET", f"/api/v1/mcp/tools/{tool_name}", @@ -713,12 +725,14 @@ def _load_tool(tool_name: str, runtime: Optional[Dict[str, Any]] = None) -> Dict ) if response["status"] == 404: raise click.ClickException(f"工具不存在：{tool_name}") - if response["status"] != 200 or not isinstance(response.get("json"), dict): - raise click.ClickException(response["text"] or f"获取工具失败（HTTP {response['status']}）") - return response["json"] + data = _tool_response_data(response, "获取工具失败") + if not isinstance(data, dict): + raise click.ClickException("获取工具失败：响应 data 不是工具对象") + return data def _load_tools(runtime: Optional[Dict[str, Any]] = None) -> list[Dict[str, Any]]: + """读取统一 REST 信封中的工具列表，空列表保持合法。""" response = _http_request( "GET", "/api/v1/mcp/tools", @@ -726,9 +740,10 @@ def _load_tools(runtime: Optional[Dict[str, Any]] = None) -> list[Dict[str, Any] timeout=10.0, runtime=runtime, ) - if response["status"] != 200 or not isinstance(response.get("json"), list): - raise click.ClickException(response["text"] or f"获取工具列表失败（HTTP {response['status']}）") - return response["json"] + data = _tool_response_data(response, "获取工具列表失败") + if not isinstance(data, list): + raise click.ClickException("获取工具列表失败：响应 data 不是工具列表") + return data def _normalize_type(schema: Optional[Dict[str, Any]]) -> str: @@ -742,6 +757,7 @@ def _normalize_type(schema: Optional[Dict[str, Any]]) -> str: def _format_tool_detail(tool: Dict[str, Any]) -> None: + """展示工具定义及参数的必填标记，供 CLI 用户构造调用。""" click.echo(f"Command: {tool.get('name')}") click.echo(f"Description: {tool.get('description') or '(none)'}") click.echo("") @@ -1464,7 +1480,9 @@ def tool_run(tool_name: str, args: tuple[str, ...]) -> None: """运行指定工具""" arguments = _parse_key_value_pairs(args) result = _call_tool(tool_name, arguments, runtime=_backend_runtime()) - if isinstance(result, (dict, list)): + if result is None or result == "": + click.echo("工具执行成功（无返回内容）") + elif isinstance(result, (dict, list)): _print_json(result) else: click.echo(result) @@ -1506,7 +1524,9 @@ def scheduler_run(job_id: str) -> None: runtime=_backend_runtime(), ) ) - if isinstance(result, (dict, list)): + if result is None or result == "": + click.echo("调度任务调用成功（无返回内容）") + elif isinstance(result, (dict, list)): _print_json(result) else: click.echo(result) diff --git a/docs/cli.md b/docs/cli.md index 7cd8ebda4..993ce0089 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -602,6 +602,9 @@ moviepilot tool run moviepilot_api operation_id=media.search 'query={"title":" - `tool list` 用于动态发现当前服务可调用的工具 - `tool show` 会输出参数名、类型和描述 - `tool run` 参数格式固定为 `key=value`；对象、数组、布尔值和 `null` 使用合法 JSON +- 工具命令自动解析本地 REST API 的 `success/message/data` 响应信封；`tool run` 输出 + `data.result` 中的 JSON 或普通文本，返回内容为空时显示成功提示。 +- REST 请求失败时显示服务端错误原因并返回非零退出码，不输出整个工具目录响应。 - MoviePilot 业务能力统一通过 `moviepilot_api` 的固定 `operation_id` 调用；不接受任意 URL、method、认证头或 Token，也不兼容旧业务工具名 - 涉及精确媒体身份的 operation 统一使用 `media_source` + `media_id`，两个字段必须成对传递并复用搜索结果 - `read_skill`、`read_file`、`write_file`、`edit_file` 和 `execute_command` @@ -626,3 +629,7 @@ moviepilot scheduler list ```shell moviepilot scheduler run subscribe_refresh ``` + +`scheduler list/run` 通过工具网关解析 REST 信封和内部业务响应。列表显示任务 ID、状态、 +下次运行时间和名称；执行命令显示业务返回值，无返回内容时显示调用成功提示，业务失败时 +显示原因并返回非零退出码。 diff --git a/docs/refactor/optimization-checklist.md b/docs/refactor/optimization-checklist.md index a29b7b500..863b4fd7c 100644 --- a/docs/refactor/optimization-checklist.md +++ b/docs/refactor/optimization-checklist.md @@ -106,7 +106,7 @@ ARCH-201 至 ARCH-204 均达到实现、验证、提交、推送和远端门禁 | Event Contract | 53 | 均已有 payload model，但当前全部是 diagnostic enforcement | | Python 源码量 | 305,884 行 | 排除 `app/plugins/**`；61 个文件超过 1,000 行，11 个超过 2,000 行 | | 长方法 | 290 个超过 80 行 | AST 统计排除 `app/plugins/**`；65 个超过 150 行，21 个超过 250 行 | -| 全量 mypy 历史债务 | 9,174 / 508 文件 | 系统版本查询补齐类型后的现状基线；消息媒体交互类型债务已清零，canonical Facade 与 endpoint 类型边界已补齐，低水位只允许继续下降 | +| 全量 mypy 历史债务 | 9,172 / 508 文件 | CLI 工具响应解包补齐数据类型校验后的现状基线；消息媒体交互类型债务已清零，canonical Facade 与 endpoint 类型边界已补齐，低水位只允许继续下降 | | Ruff 历史诊断 | 100 | 低水位门禁通过，但规则集只覆盖 `E4/E7/E9/F/I` | | 覆盖率固定基线 | Application 80.00%，Domain 80.00% | Chain、Runtime、Agent、Adapter、Startup 未进入包级覆盖率门禁 | diff --git a/moviepilot b/moviepilot index 7c446f0e9..8016afe2e 100755 --- a/moviepilot +++ b/moviepilot @@ -24,8 +24,8 @@ Runtime Commands: moviepilot start|stop|restart|status|logs|version moviepilot doctor [--json] [--fix] [--deep] moviepilot config ... - moviepilot tool ... - moviepilot scheduler ... + moviepilot tool list|show|run ... + moviepilot scheduler list|run ... Discovery Commands: moviepilot help diff --git a/tests/fixtures/architecture/mypy-baseline.json b/tests/fixtures/architecture/mypy-baseline.json index d2ace77cf..1b92f8d5b 100644 --- a/tests/fixtures/architecture/mypy-baseline.json +++ b/tests/fixtures/architecture/mypy-baseline.json @@ -1063,7 +1063,7 @@ "app/cli.py": { "attr-defined": 2, "misc": 57, - "no-any-return": 6, + "no-any-return": 4, "type-arg": 3, "union-attr": 2 }, diff --git a/tests/test_cli_tool_gateway.py b/tests/test_cli_tool_gateway.py index 851d7cc19..1dd9dc4e9 100644 --- a/tests/test_cli_tool_gateway.py +++ b/tests/test_cli_tool_gateway.py @@ -1,6 +1,8 @@ """本地 CLI 统一 Agent 工具目录测试。""" -from unittest.mock import patch +import json +from collections.abc import Iterator +from unittest.mock import Mock, patch import pytest from click import ClickException @@ -9,6 +11,73 @@ from click.testing import CliRunner from app.cli import _parse_key_value_pairs, cli +@pytest.fixture +def http_request() -> Iterator[Mock]: + """只替换 HTTP 边界，保留工具信封解析与命令渲染。""" + with ( + patch("app.cli._backend_runtime", return_value={}), + patch("app.cli._runtime_api_token", return_value="cli-test-token"), + patch("app.cli._http_request") as request, + ): + yield request + + +def _http_response(data: object, status: int = 200) -> dict: + """构造与 MCP REST 端点一致的统一成功信封。""" + payload = {"success": True, "message": "", "data": data} + return {"status": status, "json": payload, "text": json.dumps(payload)} + + +def test_tool_list_unwraps_rest_envelope(http_request: Mock) -> None: + """工具列表应解包后按名称排序，不能把整个成功信封当错误。""" + http_request.return_value = _http_response([{"name": "read_skill"}, {"name": "moviepilot_api"}]) + + result = CliRunner().invoke(cli, ["tool", "list"]) + + assert result.exit_code == 0 + assert result.output == "moviepilot_api\nread_skill\n" + + +def test_tool_show_unwraps_rest_envelope(http_request: Mock) -> None: + """工具详情必须展示 data 内的名称、描述及参数。""" + http_request.return_value = _http_response({ + "name": "moviepilot_api", + "description": "统一 API 网关", + "inputSchema": { + "properties": {"operation_id": {"type": "string", "description": "操作 ID"}}, + "required": ["operation_id"], + }, + }) + + result = CliRunner().invoke(cli, ["tool", "show", "moviepilot_api"]) + + assert result.exit_code == 0 + assert "Command: moviepilot_api" in result.output + assert "Description: 统一 API 网关" in result.output + assert "operation_id*" in result.output + assert "string" in result.output + assert "操作 ID" in result.output + + +@pytest.mark.parametrize("tool_result", ['{"version":"v3.1.3"}', '[1,2]', "执行完成", "false", "null", ""]) +def test_tool_run_unwraps_and_prints_result(http_request: Mock, tool_result: str) -> None: + """执行结果保留 JSON 或文本语义，空结果也必须有明确回显。""" + http_request.return_value = _http_response({"result": tool_result}, status=201) + + result = CliRunner().invoke(cli, ["tool", "run", "moviepilot_api", "operation_id=system.versions"]) + + assert result.exit_code == 0 + assert result.output.strip() + if tool_result.startswith(("{", "[")): + assert json.loads(result.output) == json.loads(tool_result) + elif tool_result == "false": + assert result.output == "False\n" + elif tool_result in {"null", ""}: + assert result.output == "工具执行成功（无返回内容）\n" + else: + assert result.output == f"{tool_result}\n" + + def test_tool_arguments_parse_structured_json() -> None: """tool run 应支持 API 网关的对象、数组和布尔参数。""" assert _parse_key_value_pairs( @@ -32,56 +101,140 @@ def test_tool_arguments_reject_invalid_structured_json() -> None: _parse_key_value_pairs(("query={broken}",)) -def test_scheduler_cli_uses_final_api_gateway_catalog() -> None: +def test_scheduler_cli_uses_final_api_gateway_catalog(http_request: Mock) -> None: """scheduler 子命令不得继续调用已删除的旧工具名。""" runner = CliRunner() - with ( - patch("app.cli._backend_runtime", return_value={}), - patch( - "app.cli._call_tool", - return_value={ - "success": True, - "message": "", - "data": [ - { - "id": "job-1", - "status": "waiting", - "next_run": None, - "name": "测试任务", - } - ], - }, - ) as call_tool, - ): - result = runner.invoke(cli, ["scheduler", "list"]) + http_request.return_value = _http_response({"result": json.dumps({ + "success": True, + "message": "", + "data": [{"id": "job-1", "status": "waiting", "next_run": None, "name": "测试任务"}], + })}) + result = runner.invoke(cli, ["scheduler", "list"]) assert result.exit_code == 0 - assert "job-1" in result.output - call_tool.assert_called_once_with( - "moviepilot_api", - {"operation_id": "scheduler.list"}, + assert result.output == "job-1\twaiting\tNone\t测试任务\n" + http_request.assert_called_once_with( + "POST", + "/api/v1/mcp/tools/call", + json_body={"tool_name": "moviepilot_api", "arguments": {"operation_id": "scheduler.list"}}, + headers={"X-API-KEY": "cli-test-token"}, + timeout=30.0, runtime={}, ) -def test_scheduler_run_uses_structured_operation() -> None: +def test_scheduler_run_uses_structured_operation(http_request: Mock) -> None: """scheduler run 应把 job_id 放入固定 operation 的 query。""" runner = CliRunner() - with ( - patch("app.cli._backend_runtime", return_value={}), - patch( - "app.cli._call_tool", - return_value={"success": True, "message": "", "data": True}, - ) as call_tool, - ): - result = runner.invoke(cli, ["scheduler", "run", "job-1"]) + http_request.return_value = _http_response({ + "result": json.dumps({"success": True, "message": "", "data": True}) + }) + result = runner.invoke(cli, ["scheduler", "run", "job-1"]) assert result.exit_code == 0 - call_tool.assert_called_once_with( - "moviepilot_api", - { - "operation_id": "scheduler.run", - "query": {"job_id": "job-1"}, + assert result.output == "True\n" + http_request.assert_called_once_with( + "POST", + "/api/v1/mcp/tools/call", + json_body={ + "tool_name": "moviepilot_api", + "arguments": {"operation_id": "scheduler.run", "query": {"job_id": "job-1"}}, }, + headers={"X-API-KEY": "cli-test-token"}, + timeout=30.0, runtime={}, ) + + +@pytest.mark.parametrize("command", [ + ["tool", "list"], + ["tool", "show", "moviepilot_api"], + ["tool", "run", "moviepilot_api"], + ["scheduler", "list"], + ["scheduler", "run", "job-1"], +]) +@pytest.mark.parametrize("status, payload, expected", [ + (200, {"success": False, "message": "工具不可用", "data": None}, "工具不可用"), + (401, {"success": False, "message": "认证失败", "data": None}, "认证失败"), + (500, {"detail": "服务异常"}, "服务异常"), + (502, None, "响应格式错误"), + (200, {"data": ["huge directory"]}, "HTTP 200"), +]) +def test_tool_commands_report_http_failures( + http_request: Mock, command: list[str], status: int, payload: object, expected: str, +) -> None: + """传输或信封错误必须明确失败，且不得倾倒完整响应正文。""" + http_request.return_value = {"status": status, "json": payload, "text": "huge response" * 40000} + + result = CliRunner().invoke(cli, command) + + assert result.exit_code == 1 + assert expected in result.output + assert "huge response" not in result.output + assert len(result.output) < 200 + http_request.assert_called_once() + + +@pytest.mark.parametrize("command, data, expected", [ + (["tool", "list"], {}, "不是工具列表"), + (["tool", "show", "moviepilot_api"], [], "不是工具对象"), + (["tool", "run", "moviepilot_api"], {}, "data.result"), + (["tool", "run", "moviepilot_api"], {"result": None}, "data.result"), +]) +def test_tool_commands_reject_invalid_data( + http_request: Mock, command: list[str], data: object, expected: str, +) -> None: + """损坏的成功信封不得变成静默输出或错误的成功回显。""" + http_request.return_value = _http_response(data) + + result = CliRunner().invoke(cli, command) + + assert result.exit_code == 1 + assert expected in result.output + + +def test_tool_show_preserves_missing_tool_error(http_request: Mock) -> None: + """不存在的工具仍使用 CLI 原有的明确错误。""" + http_request.return_value = {"status": 404, "json": {}, "text": ""} + + result = CliRunner().invoke(cli, ["tool", "show", "missing"]) + + assert result.exit_code == 1 + assert "工具不存在：missing" in result.output + + +@pytest.mark.parametrize("command", [["scheduler", "list"], ["scheduler", "run", "job-1"]]) +def test_scheduler_reports_inner_business_failure(http_request: Mock, command: list[str]) -> None: + """REST 调用成功不掩盖网关返回的业务失败。""" + http_request.return_value = _http_response({ + "result": json.dumps({"success": False, "message": "任务不存在", "data": None}), + }) + + result = CliRunner().invoke(cli, command) + + assert result.exit_code == 1 + assert "任务不存在" in result.output + + +@pytest.mark.parametrize("data", [None, ""]) +def test_scheduler_run_reports_empty_success(http_request: Mock, data: object) -> None: + """无业务返回值时仍显示调度调用成功，并且只调用一次。""" + http_request.return_value = _http_response({ + "result": json.dumps({"success": True, "message": "", "data": data}), + }) + + result = CliRunner().invoke(cli, ["scheduler", "run", "job-1"]) + + assert result.exit_code == 0 + assert result.output == "调度任务调用成功（无返回内容）\n" + http_request.assert_called_once() + + +def test_tool_list_accepts_empty_catalog(http_request: Mock) -> None: + """合法空工具目录不应误判为请求失败。""" + http_request.return_value = _http_response([]) + + result = CliRunner().invoke(cli, ["tool", "list"]) + + assert result.exit_code == 0 + assert result.output == ""