fix(mcp): a vision model sees MCP ImageContent instead of only its MEDIA: path
An MCP tool's ImageContent was cached and the model got back only a `MEDIA:/path` line, so a vision main model could never read the image (a raw cua-driver MCP server could not see the screen at all). The registry handler now returns the `_multimodal` envelope when the active route takes images inside tool results, decided by the same gate vision_analyze and browser screenshots use (`agent.image_input_mode`, an explicit `auxiliary.vision` backend, catalog vision, provider support). The image is re-encoded from the sniffed cache file, normalized to a provider-accepted format and sized to `vision.embed_target_bytes` / 1568 px (it is re-sent every later turn), at most 4 per result; the text half keeps the MEDIA: paths. Paths come from this call's own cache writes, never parsed out of server text. The resize runs on the tool thread, off the shared MCP loop. Plugins' `ctx.call_mcp` keeps its string contract (native_images is set only on the registry handler). Builds on #131374 by @sarahwu185, which reused the computer_use capture gate (ignores image_input_mode, native for catalog text-only models) and inlined the server's raw base64 unvalidated and unresized.
This commit is contained in:
@@ -119,3 +119,49 @@ class TestCacheMcpImageBlock:
|
||||
tag = _cache_mcp_image_block(block)
|
||||
assert tag.startswith("MEDIA:")
|
||||
assert tag.endswith(".jpg"), f"expected .jpg extension, got {tag!r}"
|
||||
|
||||
|
||||
class TestNativeImageAttach:
|
||||
"""The registry handler hands a vision route the pixels through the shared native gate; the image is
|
||||
re-encoded from the sniffed cache file within the embed budget, never the server's raw payload."""
|
||||
|
||||
def _call(self, monkeypatch, tmp_path, cfg):
|
||||
import asyncio
|
||||
from unittest.mock import AsyncMock, patch
|
||||
from PIL import Image
|
||||
from tools import mcp_tool, mcp_tool_handlers
|
||||
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
buf = __import__("io").BytesIO()
|
||||
Image.new("RGB", (3000, 2000), (20, 90, 200)).save(buf, "PNG") # past the 1568 px embed edge
|
||||
result = SimpleNamespace(isError=False, structuredContent=None, meta=None, content=[
|
||||
SimpleNamespace(type="text", text="snapshot"),
|
||||
SimpleNamespace(type="image", data=base64.b64encode(buf.getvalue()).decode(), mimeType="image/png")])
|
||||
session = SimpleNamespace(call_tool=AsyncMock(return_value=result))
|
||||
server = SimpleNamespace(session=session, _rpc_lock=None)
|
||||
|
||||
def run(factory, timeout=30):
|
||||
async def go():
|
||||
server._rpc_lock = asyncio.Lock()
|
||||
return await factory()
|
||||
return asyncio.run(go())
|
||||
|
||||
with patch.dict(mcp_tool._servers, {"srv": server}), \
|
||||
patch("tools.mcp_tool_loop._run_on_mcp_loop", side_effect=run), \
|
||||
patch("hermes_cli.config.load_config", return_value=cfg), \
|
||||
patch("agent.auxiliary_client._read_main_provider", return_value="anthropic"), \
|
||||
patch("agent.auxiliary_client._read_main_model", return_value="claude-opus-4-5"), \
|
||||
patch("agent.image_routing._lookup_supports_vision", return_value=True):
|
||||
return mcp_tool_handlers._make_tool_handler("srv", "snap", 30, native_images=True)({})
|
||||
|
||||
def test_vision_route_gets_a_resized_envelope_and_keeps_the_media_path(self, tmp_path, monkeypatch):
|
||||
out = self._call(monkeypatch, tmp_path, {})
|
||||
assert isinstance(out, dict) and out["_multimodal"] is True
|
||||
url = out["content"][1]["image_url"]["url"]
|
||||
from tools.vision_tools_history_budget import resolve_embed_target_bytes
|
||||
assert url.startswith("data:image/jpeg;base64,") and len(url) <= resolve_embed_target_bytes()
|
||||
assert "MEDIA:" in out["text_summary"]
|
||||
|
||||
def test_image_input_mode_text_keeps_the_string_result(self, tmp_path, monkeypatch):
|
||||
out = self._call(monkeypatch, tmp_path, {"agent": {"image_input_mode": "text"}})
|
||||
assert isinstance(out, str) and "MEDIA:" in out
|
||||
|
||||
@@ -5,7 +5,7 @@ embedded resources."""
|
||||
import base64
|
||||
import logging
|
||||
import mimetypes
|
||||
from typing import Any, Dict, Optional, Tuple
|
||||
from typing import Any, Dict, List, Optional, Tuple
|
||||
from tools.ansi_strip import strip_unicode_tags
|
||||
from tools.mcp_tool_common import mcp_field
|
||||
from tools.mcp_tool_schema import mcp_prefixed_tool_name
|
||||
@@ -125,6 +125,55 @@ def _cache_mcp_image_block(block) -> str:
|
||||
return _cache_mcp_media_block(block, "image", "cache_image_from_bytes", _mcp_image_extension_for_mime_type)
|
||||
|
||||
|
||||
_MCP_NATIVE_IMAGE_MAX = 4 # images attached natively from one tool result; the rest stay MEDIA: paths
|
||||
|
||||
|
||||
def _mcp_native_image_part(path: str) -> Optional[Dict[str, Any]]:
|
||||
"""``image_url`` part for one cached MCP image, sized like every other native embed (the result is
|
||||
re-sent each later turn: ``vision.embed_target_bytes``, 1568 px long edge, JPEG) and normalized to a
|
||||
provider-accepted format (BMP and friends → PNG). None when the file cannot be embedded."""
|
||||
from pathlib import Path
|
||||
from tools.vision_tools import _EMBED_MAX_DIMENSION, _MAX_BASE64_BYTES, _resize_image_for_vision
|
||||
from tools.vision_tools_history_budget import resolve_embed_target_bytes
|
||||
from tools.vision_tools_image_prep import _detect_image_mime_type_from_bytes, _normalize_to_supported_image
|
||||
src = Path(path)
|
||||
mime = _detect_image_mime_type_from_bytes(src.read_bytes())
|
||||
if not mime:
|
||||
return None
|
||||
normalized, mime, err = _normalize_to_supported_image(src, mime)
|
||||
if err or normalized is None:
|
||||
return None
|
||||
try:
|
||||
url = _resize_image_for_vision(normalized, mime_type=mime, max_base64_bytes=resolve_embed_target_bytes(),
|
||||
max_dimension=_EMBED_MAX_DIMENSION, force_jpeg=True)
|
||||
finally:
|
||||
if normalized != src:
|
||||
normalized.unlink(missing_ok=True)
|
||||
return {"type": "image_url", "image_url": {"url": url}} if len(url) <= _MAX_BASE64_BYTES else None
|
||||
|
||||
|
||||
def _mcp_result_with_native_images(text: str, image_paths: List[str]) -> Any:
|
||||
"""*text* as-is, or the ``_multimodal`` envelope carrying the call's cached images when the active route
|
||||
takes images inside tool results — the same gate as ``vision_analyze`` and ``computer_use`` captures
|
||||
(``agent.image_input_mode``, an explicit ``auxiliary.vision`` backend, catalog vision, provider support).
|
||||
The text half keeps the ``MEDIA:`` paths so sharing and full-resolution reads still work. Any failure
|
||||
keeps the text: the paths already carry the images."""
|
||||
if not image_paths:
|
||||
return text
|
||||
try:
|
||||
from tools.vision_tools import _should_use_native_vision_fast_path
|
||||
if not _should_use_native_vision_fast_path():
|
||||
return text
|
||||
parts = [p for p in map(_mcp_native_image_part, image_paths[:_MCP_NATIVE_IMAGE_MAX]) if p]
|
||||
except Exception: # deliberate boundary: the MEDIA: paths already carry the images, so any failure keeps the text
|
||||
logger.debug("MCP native image attach failed, keeping MEDIA: paths", exc_info=True)
|
||||
return text
|
||||
if not parts:
|
||||
return text
|
||||
attached = text + "\n\nThe image(s) from this call are attached — inspect them with your native vision."
|
||||
return {"_multimodal": True, "content": [{"type": "text", "text": attached}, *parts], "text_summary": text}
|
||||
|
||||
|
||||
def _cache_mcp_audio_block(block) -> str:
|
||||
"""Cache an ``AudioContent`` block and return a ``MEDIA:<path>`` tag ("" on any failure)."""
|
||||
return _cache_mcp_media_block(
|
||||
|
||||
@@ -18,7 +18,7 @@ from tools.ansi_strip import strip_unicode_tags
|
||||
from tools.mcp_tool_common import _exc_str, _sanitize_error, mcp_field, _core
|
||||
from tools import mcp_tool_loop as _loop
|
||||
from tools.mcp_tool_content import (
|
||||
_MCP_HARD_RESULT_CAP_CHARS, _base_mime, _cache_mcp_audio_block, _cache_mcp_image_block,
|
||||
_MCP_HARD_RESULT_CAP_CHARS, _cache_mcp_audio_block, _cache_mcp_image_block, _mcp_result_with_native_images,
|
||||
_render_mcp_dropped_block_notice, _render_mcp_resource_block, _strip_reserved_meta_keys,
|
||||
_truncate_mcp_text_result)
|
||||
from tools.mcp_tool_errors import _is_auth_error, _is_session_expired_error
|
||||
@@ -437,13 +437,16 @@ def _error_result_text(result) -> str:
|
||||
return "".join(str(t) for t in texts if t)
|
||||
|
||||
|
||||
def _render_content_blocks(result, server_name: str) -> Tuple[str, int]:
|
||||
def _render_content_blocks(result, server_name: str) -> Tuple[str, int, List[str]]:
|
||||
"""Text passes through; image/audio blocks are cached (MEDIA: tags); resource blocks are
|
||||
materialized rather than silently dropped; unsupported blocks become an inline drop notice
|
||||
(kimi-code#3227). Returns ``(text, usable_parts)`` — the count of REAL rendered blocks
|
||||
(whitespace-only text and drop notices excluded) that the structuredContent arbitration uses."""
|
||||
(kimi-code#3227). Returns ``(text, usable_parts, image_paths)``: ``usable_parts`` counts REAL
|
||||
rendered blocks (whitespace-only text and drop notices excluded) for the structuredContent
|
||||
arbitration; ``image_paths`` are the files THIS call's image blocks were cached to — never
|
||||
parsed back out of the text, where a server-written ``MEDIA:`` line could name any local file."""
|
||||
parts: List[str] = []
|
||||
usable_parts = 0
|
||||
image_paths: List[str] = []
|
||||
# MCP tool results can also include ImageContent blocks (screenshot / Blockbench / Playwright etc.);
|
||||
# cache those via the gateway's image-cache helper so they flow through Hermes' MEDIA: tag convention
|
||||
# and out to messaging adapters that render images natively. Without this, image blocks were silently
|
||||
@@ -456,7 +459,10 @@ def _render_content_blocks(result, server_name: str) -> Tuple[str, int]:
|
||||
if block.text.strip():
|
||||
usable_parts += 1
|
||||
continue
|
||||
rendered = _cache_mcp_image_block(block) or _cache_mcp_audio_block(block) or _render_mcp_resource_block(block, server_name)
|
||||
image = _cache_mcp_image_block(block)
|
||||
if image.startswith("MEDIA:"):
|
||||
image_paths.append(image[len("MEDIA:"):])
|
||||
rendered = image or _cache_mcp_audio_block(block) or _render_mcp_resource_block(block, server_name)
|
||||
if rendered:
|
||||
parts.append(rendered)
|
||||
usable_parts += 1
|
||||
@@ -470,7 +476,7 @@ def _render_content_blocks(result, server_name: str) -> Tuple[str, int]:
|
||||
# believing the tool returned less than it did, with no way to recover.
|
||||
parts.append(_render_mcp_dropped_block_notice(block, block_type))
|
||||
# Hard-cap pathological payloads; ordinary large results pass to spillover.
|
||||
return _truncate_mcp_text_result("\n".join(parts)), usable_parts
|
||||
return _truncate_mcp_text_result("\n".join(parts)), usable_parts, image_paths
|
||||
|
||||
|
||||
def _capped_structured_content(result):
|
||||
@@ -514,79 +520,9 @@ def _content_dual_emits_structured(result, structured) -> bool:
|
||||
return False
|
||||
|
||||
|
||||
_MCP_INLINE_IMAGE_MAX = 4 # most images inlined from one tool result
|
||||
_MCP_INLINE_IMAGE_MAX_B64 = 12_000_000 # ~9 MB of pixels; past this the MEDIA: path stands alone
|
||||
_MCP_MULTIMODAL_ROUTE_CACHE: Dict[Tuple[str, str, str], bool] = {}
|
||||
|
||||
|
||||
def _route_takes_multimodal_tool_results() -> bool:
|
||||
"""True when the active main model can consume an image INSIDE a tool result.
|
||||
|
||||
Reuses the shared capture gate rather than adding a second predicate: it already fails CLOSED
|
||||
toward text when provider/model metadata is missing, which is what is wanted here -- a
|
||||
multimodal tool result sent to a text-only route is a hard 400/404 at the provider boundary.
|
||||
Any failure keeps the MEDIA: tag, so a broken config degrades instead of breaking the call."""
|
||||
try:
|
||||
from agent.auxiliary_client import _read_main_model, _read_main_provider
|
||||
from hermes_cli.config import load_config
|
||||
from hermes_constants import hermes_home_key
|
||||
from tools.computer_use.vision_routing import should_route_capture_to_aux_vision
|
||||
|
||||
provider, model = _read_main_provider() or "", _read_main_model() or ""
|
||||
key = (hermes_home_key(), str(provider), str(model))
|
||||
if (cached := _MCP_MULTIMODAL_ROUTE_CACHE.get(key)) is not None:
|
||||
return cached
|
||||
# Aux routing means "this route cannot read the image itself" -- invert it.
|
||||
decision = not bool(should_route_capture_to_aux_vision(provider, model, load_config()))
|
||||
_MCP_MULTIMODAL_ROUTE_CACHE[key] = decision
|
||||
return decision
|
||||
except Exception:
|
||||
logger.debug("MCP multimodal route check failed; keeping MEDIA: tags", exc_info=True)
|
||||
return False
|
||||
|
||||
|
||||
def _mcp_inline_image_parts(result) -> List[Dict[str, Any]]:
|
||||
"""OpenAI-shaped image_url parts for a result's ImageContent blocks.
|
||||
|
||||
Empty when there are none, the payload is malformed, or the inline budget is spent: an MCP
|
||||
server may return many large images, and each one rides the cached prefix for the rest of the
|
||||
conversation. The MEDIA: path stays in the text half, so nothing is lost by declining."""
|
||||
parts: List[Dict[str, Any]] = []
|
||||
budget = _MCP_INLINE_IMAGE_MAX_B64
|
||||
for block in (getattr(result, "content", None) or []):
|
||||
if len(parts) >= _MCP_INLINE_IMAGE_MAX:
|
||||
break
|
||||
data = getattr(block, "data", None)
|
||||
mime = _base_mime(mcp_field(block, "mime_type", "mimeType"))
|
||||
if not data or not mime.startswith("image/"):
|
||||
continue
|
||||
if len(data) > budget:
|
||||
logger.debug("MCP image block exceeds the inline budget; the MEDIA: path stands alone")
|
||||
break
|
||||
budget -= len(data)
|
||||
parts.append({"type": "image_url", "image_url": {"url": f"data:{mime};base64,{data}"}})
|
||||
return parts
|
||||
|
||||
|
||||
def _render_call_tool_result(result, server_name: str):
|
||||
"""``CallToolResult`` -> handler payload: the JSON string, or the multimodal envelope when the
|
||||
result carries images AND the route can read them (registry._normalize_handler_result accepts
|
||||
the envelope from any tool). The text half keeps the MEDIA: paths, so sharing and later
|
||||
full-resolution reads still work; the image half is what the model actually sees. Errors stay
|
||||
text -- an error payload has nothing to show."""
|
||||
text = _render_call_tool_result_text(result, server_name)
|
||||
if mcp_field(result, "is_error", "isError", False):
|
||||
return text
|
||||
images = _mcp_inline_image_parts(result)
|
||||
if not images or not _route_takes_multimodal_tool_results():
|
||||
return text
|
||||
return {"_multimodal": True,
|
||||
"content": [{"type": "text", "text": text}, *images],
|
||||
"text_summary": text}
|
||||
|
||||
|
||||
def _render_call_tool_result_text(result, server_name: str) -> str:
|
||||
"""Pure: ``CallToolResult`` -> handler JSON. ``content`` and ``structuredContent`` are both
|
||||
def _render_call_tool_result(result, server_name: str, image_paths: Optional[List[str]] = None) -> str:
|
||||
"""Pure: ``CallToolResult`` -> handler JSON (``image_paths``, when given, receives the files this
|
||||
call's image blocks were cached to). ``content`` and ``structuredContent`` are both
|
||||
forwarded, except that a ``structuredContent`` whose JSON also sits verbatim in a text block
|
||||
(the spec's backwards-compat dual-emit; compared as parsed JSON) is dropped, because that copy
|
||||
would reach the model twice (kimi-code#3234). Any other usable text — a status line, a prose
|
||||
@@ -598,7 +534,9 @@ def _render_call_tool_result_text(result, server_name: str) -> str:
|
||||
(structuredContent-only servers); ``_meta`` minus reserved keys is always surfaced."""
|
||||
if mcp_field(result, "is_error", "isError", False):
|
||||
return tool_error(_sanitize_error(_truncate_mcp_text_result(_error_result_text(result) or "MCP tool returned an error")))
|
||||
text_result, usable_parts = _render_content_blocks(result, server_name)
|
||||
text_result, usable_parts, cached_images = _render_content_blocks(result, server_name)
|
||||
if image_paths is not None:
|
||||
image_paths.extend(cached_images)
|
||||
structured = _capped_structured_content(result)
|
||||
meta = _strip_reserved_meta_keys(mcp_field(result, "meta", "meta"))
|
||||
# A str here is the over-cap truncation stand-in (wire structuredContent is always an object): next to
|
||||
@@ -623,9 +561,11 @@ def _render_call_tool_result_text(result, server_name: str) -> str:
|
||||
return json.dumps({"result": text_result}, ensure_ascii=False)
|
||||
|
||||
|
||||
def _make_tool_handler(server_name: str, tool_name: str, tool_timeout: float):
|
||||
"""Sync registry handler (``handler(args_dict, **kwargs) -> str | dict``) calling an MCP tool via the
|
||||
background loop. A dict is the multimodal envelope -- see ``_render_call_tool_result``."""
|
||||
def _make_tool_handler(server_name: str, tool_name: str, tool_timeout: float, *, native_images: bool = False):
|
||||
"""Sync handler (``handler(args_dict, **kwargs)``) calling an MCP tool via the background loop. Always a JSON
|
||||
string, except with ``native_images`` (the registry handler the agent loop dispatches): a result carrying
|
||||
images then becomes the ``_multimodal`` envelope when the active route takes images in tool results.
|
||||
Direct callers (plugins' ``ctx.call_mcp``) keep the string contract and the ``MEDIA:`` paths."""
|
||||
op = f"tools/call {tool_name}"
|
||||
|
||||
def _handler(args: dict, **kwargs) -> Any:
|
||||
@@ -639,6 +579,7 @@ def _make_tool_handler(server_name: str, tool_name: str, tool_timeout: float):
|
||||
# Only a tool annotated readOnlyHint=True is replayed after session expiry; a 401 is always
|
||||
# pre-dispatch so the auth recoverer keeps its retry for every tool.
|
||||
read_only = _tool_is_read_only(server_name, tool_name)
|
||||
image_paths: List[str] = [] # the LAST attempt's cached images (a recoverer may retry _call)
|
||||
|
||||
async def _call():
|
||||
async with server._rpc_lock, _track_inflight_rpc(server, server_name, op, retry_safe=read_only):
|
||||
@@ -649,16 +590,19 @@ def _make_tool_handler(server_name: str, tool_name: str, tool_timeout: float):
|
||||
server._pending_call_context = None
|
||||
if getattr(server, "_mark_session_proven", None) is not None: # round-trip done: transport healthy
|
||||
server._mark_session_proven()
|
||||
return _render_call_tool_result(result, server_name)
|
||||
image_paths.clear()
|
||||
return _render_call_tool_result(result, server_name, image_paths)
|
||||
|
||||
def _on_failure(exc):
|
||||
_core._bump_server_error(server_name)
|
||||
logger.error("MCP tool %s/%s call failed: %s", server_name, tool_name, exc)
|
||||
session_expired = partial(_handle_session_expired_and_retry, call_may_have_side_effects=not read_only)
|
||||
return _dispatch(
|
||||
text = _dispatch(
|
||||
server_name, server, op, _call, tool_timeout,
|
||||
(_handle_stdio_child_exited_and_retry, _handle_auth_error_and_retry, session_expired),
|
||||
_on_failure, record_outcome=True)
|
||||
# Off the MCP loop on purpose: the image resize is CPU work that must not stall other servers' I/O.
|
||||
return _mcp_result_with_native_images(text, image_paths) if native_images and not _result_is_error(text) else text
|
||||
return _handler
|
||||
|
||||
|
||||
|
||||
@@ -265,7 +265,7 @@ def _tool_candidates(name: str, tools: Iterable[Any], should_register: Callable[
|
||||
continue
|
||||
_schema._scan_mcp_description(name, t.name, t.description or "")
|
||||
schema = _schema._convert_mcp_schema(name, t)
|
||||
handler = _handlers._make_tool_handler(name, t.name, tool_timeout)
|
||||
handler = _handlers._make_tool_handler(name, t.name, tool_timeout, native_images=True)
|
||||
out.append(_Candidate(schema["name"], f"tool {t.name!r}", schema, handler))
|
||||
return out
|
||||
|
||||
|
||||
@@ -587,6 +587,10 @@ Two behaviors apply to every MCP tool result before the model sees it:
|
||||
- **Invisible Unicode TAG characters are stripped.** Characters in the U+E0000–U+E007F range render as nothing in terminals and chat UIs but are fully visible to the model — a classic prompt-injection smuggling channel for a malicious or compromised server. Hermes strips them from tool results, resource content, and tool descriptions. Legitimate emoji tag sequences (regional flags like 🏴) are preserved.
|
||||
- **Vendor `_meta` is surfaced; protocol-reserved keys are not.** When a server attaches a `_meta` mapping to a tool result (vendor namespaces like `com.example/handoff`), Hermes passes it through to the model alongside the result content. Keys under protocol-reserved prefixes — a `modelcontextprotocol` or `mcp` label followed by another label, e.g. `modelcontextprotocol.io/...` or `tools.mcp.com/...` — are dropped, matching the MCP spec's key-name rules. If nothing model-facing remains, the `_meta` field is omitted entirely.
|
||||
|
||||
### Image results
|
||||
|
||||
An `ImageContent` block (a screenshot, a rendered chart, a page capture) is saved to Hermes' image cache and its path is reported in the result as `MEDIA:/path/to/image.png`, so you can ask for the file on any attachment-capable surface. When the main model can see images inside tool results, the image itself is attached to the result too, so the model reads the pixels directly. MCP results follow the same rule as `vision_analyze`: `agent.image_input_mode`, an explicit `auxiliary.vision` backend and the model's vision capability all apply (see [Vision](vision.md#vision_analyze-has-the-same-dual-behavior)). Attached images are downscaled to `vision.embed_target_bytes` (up to 4 per result) because they are re-sent on every later turn. Text-only models get the path only.
|
||||
|
||||
## MCP utility tools
|
||||
|
||||
When supported, Hermes also registers utility tools around MCP resources and prompts:
|
||||
|
||||
Reference in New Issue
Block a user