diff --git a/src/plyngent/cli/display.py b/src/plyngent/cli/display.py index 31822ed..305bd34 100644 --- a/src/plyngent/cli/display.py +++ b/src/plyngent/cli/display.py @@ -206,6 +206,19 @@ def _read_file_prefix(args_json: str) -> str: return _pretty_prefix(f"* Read '{path}' ") +def _view_image_prefix(args_json: str) -> str: + """``* View image 'path' ``: the picture is already on its way to the model.""" + path = _json_str_arg(args_json, "path") or "?" + return _pretty_prefix(f"* View image '{path}' ") + + +def _view_image_detail(_args_json: str, result: str) -> str: + """``view_image`` outcome: the picture was attached, or why it was not.""" + if result.startswith("error:"): + return _pretty_segments((f"({result.partition('error: ')[2]})", "red")) + return _pretty_segments(("(attached)", "green")) + + def _range_detail(result: str) -> str | None: """``L{begin}-{end} (done)`` detail of a ranged result, when it carries one.""" first = result.splitlines()[0] if result else "" @@ -896,6 +909,7 @@ def _skill_edit_detail(_args_json: str, result: str) -> str: # Pretty tools: ``prefix`` prints as the call starts, ``detail`` when it lands. _PRETTY_LINES: dict[str, PrettyLine] = { "read_file": PrettyLine(prefix=_read_file_prefix, detail=_read_file_detail), + "view_image": PrettyLine(prefix=_view_image_prefix, detail=_view_image_detail), "tree": PrettyLine(prefix=_tree_prefix, detail=_tree_detail), "todo_list": _todo_line("todo_list"), "todo_push": _todo_line("todo_push"), diff --git a/src/plyngent/runtime/mcp_client.py b/src/plyngent/runtime/mcp_client.py index d0252bb..d14744c 100644 --- a/src/plyngent/runtime/mcp_client.py +++ b/src/plyngent/runtime/mcp_client.py @@ -8,6 +8,7 @@ the process: start, tool listing, tool calls, reconnect, shutdown. from __future__ import annotations import asyncio +import base64 import importlib.metadata import json import os @@ -17,6 +18,10 @@ from collections import deque from dataclasses import dataclass from typing import TYPE_CHECKING, cast +from plyngent.agent.images import image_part_from_bytes +from plyngent.agent.tools import ToolOutput +from plyngent.lmproto.openai_compatible.model import ChatImagePart + if TYPE_CHECKING: from typing import Any @@ -84,9 +89,10 @@ class McpTool: ) -def _format_content(result: Json) -> str: - """Flatten a ``tools/call`` result payload into model-facing text.""" +def _format_content(result: Json) -> ToolOutput: + """Flatten a ``tools/call`` result payload into text plus any images.""" parts: list[str] = [] + images: list[ChatImagePart] = [] for item in _json_list(result.get("content")): entry = _json_dict(item) kind = str(entry.get("type", "")) @@ -94,12 +100,24 @@ def _format_content(result: Json) -> str: text = entry.get("text") if isinstance(text, str): parts.append(text) + elif kind == "image": + # MCP carries an image as base64 + mimeType; the model wants the + # picture itself, not a ``[image: image/png]`` placeholder. + data = entry.get("data") + mime = entry.get("mimeType", "?") + part = image_part_from_bytes(base64.b64decode(data)) if isinstance(data, str) and data else None + if isinstance(part, ChatImagePart): + images.append(part) + elif isinstance(part, str): + parts.append(f"[image: {mime}] {part.removeprefix('error: ')}") + else: + parts.append(f"[image: {mime}]") elif kind == "resource": parts.append(f"[resource] {json.dumps(entry, ensure_ascii=False)}") else: mime = entry.get("mimeType", "?") parts.append(f"[{kind or 'unknown'}: {mime}]") - return "\n".join(parts) + return ToolOutput(text="\n".join(parts), images=images) class McpServerConnection: @@ -307,17 +325,17 @@ class McpServerConnection: cursor = next_cursor return tools - async def call_tool(self, name: str, arguments: Json) -> str: - """Run ``tools/call`` and return the flattened text result. + async def call_tool(self, name: str, arguments: Json) -> ToolOutput: + """Run ``tools/call`` and return its text plus any images it produced. Tool-level failures (``isError``) come back as ``error: …`` text, not exceptions: the protocol round-trip itself succeeded. """ result = await self._request("tools/call", {"name": name, "arguments": arguments}) - text = _format_content(result) + output = _format_content(result) if result.get("isError") is True: - return f"error: {text or 'tool reported failure without content'}" - return text + return ToolOutput(text=f"error: {output.text or 'tool reported failure without content'}") + return output class McpManager: @@ -393,7 +411,7 @@ class McpManager: connection.tools = await connection.list_tools() connection.tools_stale = False - async def call(self, server: str, tool: str, arguments: Json) -> str: + async def call(self, server: str, tool: str, arguments: Json) -> ToolOutput: connection = self._connections.get(server) if connection is None or not connection.connected: msg = f"server {server!r} is not connected" diff --git a/src/plyngent/tools/file/__init__.py b/src/plyngent/tools/file/__init__.py index 21868ea..3ef640d 100644 --- a/src/plyngent/tools/file/__init__.py +++ b/src/plyngent/tools/file/__init__.py @@ -11,10 +11,12 @@ from .listdir import listdir as listdir from .read import read_file as read_file from .regex_files import regex_files as regex_files from .tree import tree as tree +from .view_image import view_image as view_image from .write import write_file as write_file FILE_TOOLS = [ read_file, + view_image, write_file, listdir, tree, diff --git a/src/plyngent/tools/file/read.py b/src/plyngent/tools/file/read.py index 6ff6040..b84f969 100644 --- a/src/plyngent/tools/file/read.py +++ b/src/plyngent/tools/file/read.py @@ -4,6 +4,7 @@ from typing import cast from plyngent.agent import ToolTag, mark_lineno_read, tool from plyngent.agent.budget import DEFAULT_TOOL_RESULT_MAX_CHARS +from plyngent.agent.images import sniff_image_media_type from plyngent.tools.truncate_token import TruncateToken, truncate_with_token, truncation_marker from plyngent.tools.workspace import AccessMode, resolve_path @@ -39,9 +40,15 @@ def read_raw_text(path: str) -> tuple[str | None, str]: Shared by ``read_file`` and ``get_truncated`` so truncate-token char offsets always refer to the raw file text (never the ``L{begin}-{end}`` header). - Missing paths and directories are distinguished for the caller. + Missing paths and directories are distinguished for the caller, and a picture + is answered with the tool that shows it instead of decode noise. """ target = resolve_path(path, required=AccessMode.READ) + if target.is_file(): + with target.open("rb") as handle: + media_type = sniff_image_media_type(handle.read(16)) + if media_type is not None: + return None, f"error: {path} is an image ({media_type}); use view_image to look at it" if not target.exists(): return None, f"error: file not found: {path}" if not target.is_file(): @@ -71,6 +78,8 @@ async def read_file( ``DEFAULT_TOOL_RESULT_MAX_CHARS``); a ``truncate_token`` is appended when cut so ``get_truncated`` can continue reading the rest without a new request. Pass ``max_chars < 1`` to read unbounded. + + An image file is refused with a pointer to ``view_image``, which shows it. """ target = resolve_path(path, required=AccessMode.READ) text, err = read_raw_text(path) diff --git a/src/plyngent/tools/file/view_image.py b/src/plyngent/tools/file/view_image.py new file mode 100644 index 0000000..97e6f2f --- /dev/null +++ b/src/plyngent/tools/file/view_image.py @@ -0,0 +1,42 @@ +"""Model-facing ``view_image``: hand a local picture to the model as a picture.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, cast + +from plyngent.agent import ToolOutput, ToolTag, tool +from plyngent.agent.images import image_part_from_bytes +from plyngent.tools.workspace import AccessMode, resolve_path + +if TYPE_CHECKING: + from plyngent.lmproto.openai_compatible.model import ImageDetail + +_DETAILS: frozenset[str] = frozenset({"low", "high", "original", "auto"}) + + +@tool(tags=ToolTag.LOCAL | ToolTag.INSTANCE_STATE | ToolTag.READ_ONLY) +async def view_image(path: str, detail: str = "auto") -> str | ToolOutput: + """Look at an image file (JPEG, PNG, GIF, or WebP) under the workspace. + + Use it for a screenshot, a chart, a diagram, or any other picture, instead + of ``read_file`` (which would return mojibake for those bytes). + + ``detail`` trades tokens for fine visual detail: ``low`` downscales to + 512x512 before inference — fast and cheap when only the layout matters, but + small text stops being readable — while ``high``/``original`` keep the + picture as it is, and ``auto`` (the default) lets the provider decide. + + The format is read from the file's own bytes, so a misnamed file still + works; anything else comes back as an error. + """ + if detail not in _DETAILS: + return f"error: detail must be one of {', '.join(sorted(_DETAILS))}" + target = resolve_path(path, required=AccessMode.READ) + if not target.exists(): + return f"error: file not found: {path}" + if not target.is_file(): + return f"error: not a file: {path}" + part = image_part_from_bytes(target.read_bytes(), detail=cast("ImageDetail", detail)) + if isinstance(part, str): + return f"error: {path}: {part.removeprefix('error: ')}" + return ToolOutput(text=f"image: {path} (attached below)", images=[part]) diff --git a/src/plyngent/tools/mcp.py b/src/plyngent/tools/mcp.py index 6ab5f94..3d933e4 100644 --- a/src/plyngent/tools/mcp.py +++ b/src/plyngent/tools/mcp.py @@ -15,7 +15,7 @@ from __future__ import annotations import re from typing import TYPE_CHECKING -from plyngent.agent.tools import ToolDefinition, ToolTag +from plyngent.agent.tools import ToolDefinition, ToolOutput, ToolTag from plyngent.tools.catalog import ToolSource, get_catalog if TYPE_CHECKING: @@ -43,8 +43,8 @@ def _make_handler( manager: McpManager, server: str, tool: str, -) -> Callable[..., Awaitable[str]]: - async def handler(**arguments: object) -> str: +) -> Callable[..., Awaitable[ToolOutput]]: + async def handler(**arguments: object) -> ToolOutput: return await manager.call(server, tool, dict(arguments)) return handler diff --git a/src/plyngent/tools/net/client.py b/src/plyngent/tools/net/client.py index 060c0f0..8f88380 100644 --- a/src/plyngent/tools/net/client.py +++ b/src/plyngent/tools/net/client.py @@ -7,6 +7,7 @@ from typing import TYPE_CHECKING, Any import niquests +from plyngent.agent.images import sniff_image_media_type from plyngent.tools.net.grants import ensure_host_allowed from plyngent.tools.net.policy import ( DEFAULT_MAX_BYTES, @@ -50,7 +51,10 @@ class FetchResult: method: str security: str warnings: list[str] = field(default_factory=list) - body_kind: str = "text" # text | binary | empty + body_kind: str = "text" # text | binary | image | empty + # Raw bytes, kept only for an image body so the caller can hand the picture + # to the model instead of a hex preview. + body_image: bytes = b"" def _content_type(headers: Mapping[str, str] | object | None) -> str: @@ -132,6 +136,10 @@ def _body_payload(data: bytes, content_type: str) -> tuple[str, str]: return "empty", "" if _looks_text(content_type, data): return "text", _decode_body(data, content_type) + if sniff_image_media_type(data) is not None: + # An image is handed to the model as a picture, so the preview line only + # carries what the part cannot say itself. + return "image", f"(image body; content_type={content_type!r}; bytes={len(data)})" preview = data[:64].hex() return ( "binary", @@ -279,6 +287,7 @@ async def http_fetch( ), warnings=list(dict.fromkeys(warnings)), body_kind=body_kind, + body_image=data if body_kind == "image" else b"", ) diff --git a/src/plyngent/tools/net/fetch_local.py b/src/plyngent/tools/net/fetch_local.py index f234683..3f36d7f 100644 --- a/src/plyngent/tools/net/fetch_local.py +++ b/src/plyngent/tools/net/fetch_local.py @@ -2,9 +2,10 @@ from __future__ import annotations -from plyngent.agent import ToolTag, tool +from plyngent.agent import ToolOutput, ToolTag, tool +from plyngent.agent.images import image_part_from_bytes from plyngent.tools.context import is_read_only_context -from plyngent.tools.net.client import http_fetch +from plyngent.tools.net.client import FetchResult, http_fetch from plyngent.tools.net.policy import ( DEFAULT_MAX_BODY_CHARS_IN, DEFAULT_MAX_BYTES, @@ -76,7 +77,7 @@ async def fetch_local( follow_redirects: bool = True, allow_http_downgrade: bool = False, offset: int = 0, -) -> str: +) -> str | ToolOutput: """HTTP request (GET/POST/PUT/DELETE); return status, metadata, and truncated body. Use for public docs/APIs or (after human policy allow) local/LAN servers. @@ -88,6 +89,9 @@ async def fetch_local( User-Agent entry in ``headers``. A headers-only User-Agent is kept as-is. When both omit UA, a small default is used. + An image response (JPEG, PNG, GIF, WebP) comes back as the picture itself + beside that metadata, so you can look at it instead of reading a hex preview. + Private/loopback/link-local hosts require an explicit human policy grant (not skipped by YOLO). HTTPS→HTTP redirects are blocked unless ``allow_http_downgrade`` is true. @@ -121,7 +125,7 @@ async def fetch_local( except Exception as exc: # noqa: BLE001 — tool surface returns error text return f"error: fetch_local failed: {exc}" - return format_fetch_result( + text = format_fetch_result( status=result.status, final_url=result.final_url, content_type=result.content_type, @@ -136,6 +140,21 @@ async def fetch_local( max_chars=max_chars, offset=offset, ) + return _attach_image(text, result) + + +def _attach_image(text: str, result: FetchResult) -> str | ToolOutput: + """An image response is handed over as a picture, beside the usual metadata. + + The preview line stays in the text so ``body_kind=image`` and the byte count + are still visible to the model. + """ + if result.body_kind != "image" or not result.body_image: + return text + part = image_part_from_bytes(result.body_image) + if isinstance(part, str): + return f"{text}\n{part}" + return ToolOutput(text=text, images=[part]) # Re-export for danger classifier without importing client. diff --git a/tests/test_cli/test_mcp_cmd.py b/tests/test_cli/test_mcp_cmd.py index 4b5ecd4..34a706f 100644 --- a/tests/test_cli/test_mcp_cmd.py +++ b/tests/test_cli/test_mcp_cmd.py @@ -68,7 +68,7 @@ async def test_slash_mcp_list_statuses(state: ReplState, capsys: pytest.CaptureF out = capsys.readouterr().out assert "fake" in out assert "connected" in out - assert "tools=3" in out + assert "tools=4" in out finally: await manager.aclose() @@ -92,7 +92,7 @@ async def test_slash_mcp_reconnect_connects_new_config( statuses = state.mcp_manager.statuses() assert [name for name, _, _ in statuses] == ["fake"] assert statuses[0][1] == "connected" - assert statuses[0][2] == 3 + assert statuses[0][2] == 4 out = capsys.readouterr().out assert "tools=off" in out assert "fake" in out diff --git a/tests/test_runtime/mcp_fake_server.py b/tests/test_runtime/mcp_fake_server.py index 661a15b..4249882 100644 --- a/tests/test_runtime/mcp_fake_server.py +++ b/tests/test_runtime/mcp_fake_server.py @@ -11,6 +11,8 @@ import os import sys _FAKE_INSTRUCTIONS = os.environ.get("PLYNGENT_MCP_FAKE_INSTRUCTIONS", "") +# A real 1x1 PNG (base64), what an image-carrying tool result looks like. +_PNG_BASE64 = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==" def respond(request_id: int, result: object) -> None: @@ -61,6 +63,11 @@ def main() -> None: "description": "Sleeps past any client timeout.", "inputSchema": {"type": "object", "properties": {}}, }, + { + "name": "shot", + "description": "Returns a picture beside its text.", + "inputSchema": {"type": "object", "properties": {}}, + }, ] }, ) @@ -75,6 +82,17 @@ def main() -> None: "isError": False, }, ) + elif name == "shot": + respond( + int(request_id), + { + "content": [ + {"type": "text", "text": "shot.png"}, + {"type": "image", "data": _PNG_BASE64, "mimeType": "image/png"}, + ], + "isError": False, + }, + ) elif name == "fail": respond( int(request_id), diff --git a/tests/test_runtime/test_mcp_client.py b/tests/test_runtime/test_mcp_client.py index 10caa3e..f564a1c 100644 --- a/tests/test_runtime/test_mcp_client.py +++ b/tests/test_runtime/test_mcp_client.py @@ -34,13 +34,28 @@ async def test_connection_handshake_lists_and_calls_tools() -> None: assert connection.connected assert connection.error is None assert connection.instructions is None - assert [tool.name for tool in connection.tools] == ["echo", "fail", "slow"] + assert [tool.name for tool in connection.tools] == ["echo", "fail", "slow", "shot"] echo = connection.tools[0] assert echo.description == "Echo the given text back." assert echo.input_schema["type"] == "object" result = await connection.call_tool("echo", {"text": "hello"}) - assert result == "echo: hello" + assert result.text == "echo: hello" + assert result.images == [] + finally: + await connection.aclose() + + +async def test_an_image_content_block_becomes_an_image_part() -> None: + """A server that returns pixels gets them to the model, not a placeholder.""" + connection = McpServerConnection("fake", _server_config()) + await connection.start() + try: + output = await connection.call_tool("shot", {}) + + assert output.text == "shot.png" + assert len(output.images) == 1 + assert output.images[0].image_url.url.startswith("data:image/png;base64,") finally: await connection.aclose() @@ -64,7 +79,7 @@ async def test_tool_error_returns_error_text() -> None: await connection.start() try: result = await connection.call_tool("fail", {}) - assert result == "error: boom" + assert result.text == "error: boom" finally: await connection.aclose() @@ -122,9 +137,9 @@ async def test_manager_start_status_and_call() -> None: manager = McpManager(McpConfig(servers={"fake": _server_config()}, disable=[])) await manager.ensure_started() try: - assert [(name, status, count) for name, status, count in manager.statuses()] == [("fake", "connected", 3)] + assert [(name, status, count) for name, status, count in manager.statuses()] == [("fake", "connected", 4)] assert manager.instructions() == [] - assert await manager.call("fake", "echo", {"text": "hi"}) == "echo: hi" + assert (await manager.call("fake", "echo", {"text": "hi"})).text == "echo: hi" finally: await manager.aclose() assert manager.statuses() == [("fake", "not started", 0)] @@ -157,5 +172,5 @@ async def test_manager_disable_and_restart() -> None: await manager.restart(McpConfig(servers={"fake": _server_config()}, disable=[])) await manager.ensure_started() - assert manager.statuses() == [("fake", "connected", 3)] + assert manager.statuses() == [("fake", "connected", 4)] await manager.aclose() diff --git a/tests/test_tools/test_catalog.py b/tests/test_tools/test_catalog.py index 4ea3617..9463783 100644 --- a/tests/test_tools/test_catalog.py +++ b/tests/test_tools/test_catalog.py @@ -59,6 +59,7 @@ def test_read_only_classification() -> None: read_only = {t.name for t in default_tool_definitions(surface="local") if t.tags & ToolTag.READ_ONLY} assert read_only == { "read_file", + "view_image", "get_truncated", "glob_paths", "regex_files", diff --git a/tests/test_tools/test_fetch_local.py b/tests/test_tools/test_fetch_local.py index 6294918..6bc9f36 100644 --- a/tests/test_tools/test_fetch_local.py +++ b/tests/test_tools/test_fetch_local.py @@ -2,12 +2,14 @@ from __future__ import annotations +import base64 import threading from http.server import BaseHTTPRequestHandler, HTTPServer from typing import TYPE_CHECKING, override import pytest +from plyngent.agent import ToolOutput from plyngent.tools.context import read_only_context from plyngent.tools.danger import classify_danger from plyngent.tools.net import fetch_local, grant_private_host @@ -29,6 +31,12 @@ if TYPE_CHECKING: from pathlib import Path +# A real 1x1 PNG: the body must be sniffed by its bytes, not by the header. +_PNG_BYTES = base64.b64decode( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==" +) + + class _Handler(BaseHTTPRequestHandler): """Minimal echo server for fetch_local tests.""" @@ -66,6 +74,9 @@ class _Handler(BaseHTTPRequestHandler): if self.path.startswith("/bin"): self._send(200, b"\x00\x01\x02\xffbinary", content_type="application/octet-stream") return + if self.path.startswith("/png"): + self._send(200, _PNG_BYTES, content_type="image/png") + return if self.path.startswith("/ua"): ua = (self.headers.get("User-Agent") or "").encode() self._send(200, ua) @@ -200,6 +211,20 @@ async def test_fetch_get_post_put_delete(workspace: Path, http_server: str) -> N assert "status=204" in out_del +async def test_an_image_response_comes_back_as_the_picture(workspace: Path, http_server: str) -> None: + """A PNG body is handed to the model as an image part, not a hex preview.""" + del workspace + grant_private_host("127.0.0.1", int(http_server.rsplit(":", 1)[1])) + + output: object = await fetch_local.handler(f"{http_server}/png") + + assert isinstance(output, ToolOutput) + assert "body_kind=image" in output.text + assert "status=200" in output.text + part = output.images[0] + assert part.image_url.url.startswith("data:image/png;base64,") + + async def test_fetch_read_only_context_blocks_non_get(workspace: Path, http_server: str) -> None: """In a read-only context fetch_local only allows GET; outside it POST works.""" del workspace diff --git a/tests/test_tools/test_view_image.py b/tests/test_tools/test_view_image.py new file mode 100644 index 0000000..f8938f7 --- /dev/null +++ b/tests/test_tools/test_view_image.py @@ -0,0 +1,68 @@ +"""``view_image``: a local picture reaches the model as a picture.""" + +from __future__ import annotations + +import base64 +from typing import TYPE_CHECKING + +from plyngent.agent import ToolOutput +from plyngent.tools.file.read import read_file +from plyngent.tools.file.view_image import view_image + +if TYPE_CHECKING: + from pathlib import Path + +# A real 1x1 PNG: the format is sniffed from these bytes. +PNG = base64.b64decode( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==" +) + + +async def test_view_image_attaches_the_picture(workspace: Path) -> None: + (workspace / "shot.png").write_bytes(PNG) + + output = await view_image.handler("shot.png") + + assert isinstance(output, ToolOutput) + assert output.text == "image: shot.png (attached below)" + url = output.images[0].image_url.url + assert url.startswith("data:image/png;base64,") + assert base64.b64decode(url.partition(",")[2]) == PNG + + +async def test_the_detail_level_reaches_the_part(workspace: Path) -> None: + (workspace / "shot.png").write_bytes(PNG) + + output = await view_image.handler("shot.png", detail="low") + + assert isinstance(output, ToolOutput) + assert output.images[0].image_url.detail == "low" + + +async def test_an_unknown_detail_level_is_an_error(workspace: Path) -> None: + (workspace / "shot.png").write_bytes(PNG) + + assert "detail must be one of" in await view_image.handler("shot.png", detail="huge") + + +async def test_a_non_image_file_is_an_error(workspace: Path) -> None: + (workspace / "notes.txt").write_text("hello") + + result = await view_image.handler("notes.txt") + + assert isinstance(result, str) + assert "not a JPEG, PNG, GIF, or WebP image" in result + + +async def test_a_missing_file_is_an_error(workspace: Path) -> None: + del workspace + + assert await view_image.handler("nope.png") == "error: file not found: nope.png" + + +async def test_read_file_points_at_view_image_for_a_picture(workspace: Path) -> None: + (workspace / "shot.png").write_bytes(PNG) + + result = await read_file.handler("shot.png") + + assert result == "error: shot.png is an image (image/png); use view_image to look at it"