diff --git a/helpers/mcp_handler.py b/helpers/mcp_handler.py index 2b4e0d8b9..b1f888ec1 100644 --- a/helpers/mcp_handler.py +++ b/helpers/mcp_handler.py @@ -166,23 +166,45 @@ class MCPTool(Tool): encoded: str, mime_type: str, label: str, - ) -> tuple[str, dict[str, Any] | None]: + index: int, + preferred_name: str = "", + ) -> tuple[str, dict[str, Any] | None, str]: try: - image = media_artifacts.image_data_url_from_base64( + safe_mime = media_artifacts.normalize_mime( + mime_type, + default="image/png", + required_prefix="image/", + ) + artifact = media_artifacts.save_base64_artifact( encoded, - mime_type=mime_type, + mime_type=safe_mime, + directory_parts=self._artifact_directory_parts(), + preferred_name=preferred_name, + default_filename=self._default_artifact_filename( + label=label, + index=index, + mime_type=safe_mime, + ), ) except media_artifacts.EmptyBase64Data: - return f"MCP returned an empty {label} attachment.", None + return f"MCP returned an empty {label} attachment.", None, "" except media_artifacts.InvalidBase64Data: - return f"MCP returned a {label} attachment that could not be decoded.", None + return ( + f"MCP returned a {label} attachment that could not be decoded.", + None, + "", + ) return ( - f"MCP returned {label} attachment ({image.mime}, {image.size} bytes).", + ( + f"Saved MCP {label} attachment " + f"({artifact.mime}, {artifact.size} bytes) to {artifact.path}." + ), { "type": "image_url", - "image_url": {"url": image.url}, + "image_url": {"url": artifact.path}, }, + artifact.path, ) def _materialize_binary_content( @@ -266,6 +288,7 @@ class MCPTool(Tool): text_parts: list[str] = [] notes: list[str] = [] raw_images: list[dict[str, Any]] = [] + image_paths: list[str] = [] content_items = list(getattr(response, "content", []) or []) for index, item in enumerate(content_items, start=1): @@ -278,14 +301,17 @@ class MCPTool(Tool): continue if item_type == "image": - note, raw_content = self._format_image_content( + note, raw_content, path = self._format_image_content( encoded=str(_mcp_get(item, "data", "") or ""), mime_type=str(_mcp_get(item, "mimeType", "") or "image/png"), label="image", + index=index, ) notes.append(note) if raw_content: raw_images.append(raw_content) + if path: + image_paths.append(path) continue if item_type == "audio": @@ -312,10 +338,12 @@ class MCPTool(Tool): _mcp_get(resource, "mimeType", "") or "application/octet-stream" ).strip().lower() if mime_type.startswith("image/"): - note, raw_content = self._format_image_content( + note, raw_content, path = self._format_image_content( encoded=blob, mime_type=mime_type, label="resource image", + index=index, + preferred_name=uri, ) else: note = self._materialize_binary_content( @@ -326,9 +354,12 @@ class MCPTool(Tool): preferred_name=uri, ) raw_content = None + path = "" notes.append(note) if raw_content: raw_images.append(raw_content) + if path: + image_paths.append(path) continue if uri: @@ -356,6 +387,8 @@ class MCPTool(Tool): "raw_content": raw_images, "preview": f"", "_tokens": MCP_MEDIA_TOKENS_ESTIMATE * len(raw_images), + "attachments": image_paths, + "media_paths": image_paths, } return message, additional diff --git a/helpers/mcp_handler.py.dox.md b/helpers/mcp_handler.py.dox.md index 906a6f7b1..4faff9fe1 100644 --- a/helpers/mcp_handler.py.dox.md +++ b/helpers/mcp_handler.py.dox.md @@ -81,6 +81,7 @@ - MCP tool names are qualified as `server_name.tool_name`; server names are normalized without dots, and the tool portion may contain dots. - Servers may define `disabled_tools` as a list of MCP tool names. Disabled tools are omitted from agent-facing prompts, status counts, `has_tool`, and calls, while detail views can still retrieve them through `get_all_tools()` with a `disabled` flag so users can re-enable them. - Server-specific `init_timeout` and `tool_timeout` override global MCP client timeout settings for list-tools and call-tool operations. +- MCP image and image-resource content is materialized to scoped artifact files and returned both as model-visible image attachments and as path metadata (`attachments`/`media_paths`) for downstream delivery. - MCP config locks must not be held across awaited server initialization or tool-call operations. Slow or wedged MCP servers must not block status reads, prompt construction, unrelated MCP servers, or later tool calls through the shared config lock. - MCP client session work runs inside disposable isolated `DeferredTask` workers with an outer timeout. Normal `AsyncExitStack` cleanup is also bounded; if cleanup or transport shutdown does not finish, the operation reports failure or warning while Agent Zero keeps control of the agent loop. - Server status marks initialized server objects with cached initialization errors as disconnected, even if the config object exists. diff --git a/tests/test_mcp_handler_multimodal.py b/tests/test_mcp_handler_multimodal.py index 040955e0e..19b4ca123 100644 --- a/tests/test_mcp_handler_multimodal.py +++ b/tests/test_mcp_handler_multimodal.py @@ -443,7 +443,7 @@ def test_mcp_isolated_operation_timeout_returns_control(mcp_handler_module): def test_mcp_image_content_becomes_history_image_attachment(mcp_handler_module, monkeypatch): - module, _tmp_path = mcp_handler_module + module, tmp_path = mcp_handler_module agent, log, tool_results, messages, updates, warnings = _agent_recorder() image_b64 = base64.b64encode(b"image-bytes").decode("ascii") result = _FakeCallToolResult( @@ -470,16 +470,24 @@ def test_mcp_image_content_becomes_history_image_attachment(mcp_handler_module, response = asyncio.run(tool.execute()) assert "[Tool returned no textual content]" not in response.message - assert response.message == "MCP returned image attachment (image/webp, 11 bytes)." + assert ( + "Saved MCP image attachment (image/webp, 11 bytes) to " + "/a0/tmp/mcp/ctx_mcp/venice_image/" + ) in response.message assert response.additional is not None - data_url = response.additional["raw_content"][0]["image_url"]["url"] - assert data_url == f"data:image/webp;base64,{image_b64}" + image_path = response.additional["raw_content"][0]["image_url"]["url"] + assert image_path.startswith("/a0/tmp/mcp/ctx_mcp/venice_image/") + assert response.additional["attachments"] == [image_path] + assert response.additional["media_paths"] == [image_path] + assert (tmp_path / image_path.removeprefix("/a0/")).exists() asyncio.run(tool.after_execution(response)) assert tool_results[0][0] == ("venice_image", response.message) + assert tool_results[0][1]["attachments"] == [image_path] + assert tool_results[0][1]["media_paths"] == [image_path] raw_message = messages[0][1]["content"] - assert raw_message["raw_content"][0]["image_url"]["url"] == data_url + assert raw_message["raw_content"][0]["image_url"]["url"] == image_path assert messages[0][1]["tokens"] == module.MCP_MEDIA_TOKENS_ESTIMATE assert updates[-1]["content"] == response.message assert warnings == [] @@ -527,7 +535,7 @@ def test_mcp_audio_content_is_saved_instead_of_discarded(mcp_handler_module, mon def test_mcp_image_resource_blob_becomes_history_image_attachment(mcp_handler_module, monkeypatch): - module, _tmp_path = mcp_handler_module + module, tmp_path = mcp_handler_module agent, log, tool_results, messages, updates, warnings = _agent_recorder() image_b64 = base64.b64encode(b"resource-image").decode("ascii") result = _FakeCallToolResult( @@ -562,16 +570,21 @@ def test_mcp_image_resource_blob_becomes_history_image_attachment(mcp_handler_mo response = asyncio.run(tool.execute()) - assert response.message == "MCP returned resource image attachment (image/webp, 14 bytes)." + assert ( + "Saved MCP resource image attachment (image/webp, 14 bytes) to " + "/a0/tmp/mcp/ctx_mcp/venice_resource_image/" + ) in response.message assert response.additional is not None - data_url = response.additional["raw_content"][0]["image_url"]["url"] - assert data_url == f"data:image/webp;base64,{image_b64}" + image_path = response.additional["raw_content"][0]["image_url"]["url"] + assert image_path.startswith("/a0/tmp/mcp/ctx_mcp/venice_resource_image/") + assert response.additional["attachments"] == [image_path] + assert (tmp_path / image_path.removeprefix("/a0/")).exists() asyncio.run(tool.after_execution(response)) assert tool_results[0][0] == ("venice_resource_image", response.message) raw_message = messages[0][1]["content"] - assert raw_message["raw_content"][0]["image_url"]["url"] == data_url + assert raw_message["raw_content"][0]["image_url"]["url"] == image_path assert updates[-1]["content"] == response.message assert warnings == []