From 8ff4b5e9d30d5c7a1c3e6e886f5dc42ddb3d69a4 Mon Sep 17 00:00:00 2001 From: David Soria Parra Date: Fri, 24 Jan 2025 14:20:42 +0000 Subject: [PATCH] fix: respect resource mime type in responses The server was ignoring mime types set on resources, defaulting to text/plain for strings and application/octet-stream for bytes. Now properly preserves the specified mime type in both FastMCP and low-level server implementations. Note that this is breaks backwards compatibility as it changes the return values of read_resource() on FastMCP. It is BC compatible on lowlevel since it only extends the callback. Github-Issue: #152 Reported-by: eiseleMichael --- CLAUDE.md | 37 +++++++---- src/mcp/server/fastmcp/server.py | 8 ++- src/mcp/server/lowlevel/server.py | 61 +++++++++++++------ tests/issues/test_152_resource_mime_type.py | 4 +- .../fastmcp/servers/test_file_server.py | 8 ++- tests/server/fastmcp/test_server.py | 4 +- 6 files changed, 82 insertions(+), 40 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 4516da4..62a907d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -25,16 +25,31 @@ This document contains critical information about working with this codebase. Fo - New features require tests - Bug fixes require regression tests -4. Version Control - - Commit messages: conventional format (fix:, feat:) - - PR scope: minimal, focused changes - - PR requirements: description, test plan - - Always include issue numbers - - Quote handling: - ```bash - git commit -am "\"fix: message\"" - gh pr create --title "\"title\"" --body "\"body\"" - ``` +- For commits fixing bugs or adding features based on user reports add: + ```bash + git commit --trailer "Reported-by:" + ``` + Where `` is the name of the user. + +- For commits related to a Github issue, add + ```bash + git commit --trailer "Github-Issue:" + ``` +- NEVER ever mention a `co-authored-by` or similar aspects. In particular, never + mention the tool used to create the commit message or PR. + +## Pull Requests + +- Create a detailed message of what changed. Focus on the high level description of + the problem it tries to solve, and how it is solved. Don't go into the specifics of the + code unless it adds clarity. + +- Always add `jerome3o-anthropic` and `jspahrsummers` as reviewer. + +- NEVER ever mention a `co-authored-by` or similar aspects. In particular, never + mention the tool used to create the commit message or PR. + +## Python Tools ## Code Formatting @@ -96,4 +111,4 @@ This document contains critical information about working with this codebase. Fo - Keep changes minimal - Follow existing patterns - Document public APIs - - Test thoroughly \ No newline at end of file + - Test thoroughly diff --git a/src/mcp/server/fastmcp/server.py b/src/mcp/server/fastmcp/server.py index 45f1791..e935dd8 100644 --- a/src/mcp/server/fastmcp/server.py +++ b/src/mcp/server/fastmcp/server.py @@ -197,14 +197,16 @@ class FastMCP: for template in templates ] - async def read_resource(self, uri: AnyUrl | str) -> str | bytes: + async def read_resource(self, uri: AnyUrl | str) -> tuple[str | bytes, str]: """Read a resource by URI.""" + resource = await self._resource_manager.get_resource(uri) if not resource: raise ResourceError(f"Unknown resource: {uri}") try: - return await resource.read() + content = await resource.read() + return (content, resource.mime_type) except Exception as e: logger.error(f"Error reading resource {uri}: {e}") raise ResourceError(str(e)) @@ -606,7 +608,7 @@ class Context(BaseModel): progress_token=progress_token, progress=progress, total=total ) - async def read_resource(self, uri: str | AnyUrl) -> str | bytes: + async def read_resource(self, uri: str | AnyUrl) -> tuple[str | bytes, str]: """Read a resource by URI. Args: diff --git a/src/mcp/server/lowlevel/server.py b/src/mcp/server/lowlevel/server.py index c3f2abf..753590d 100644 --- a/src/mcp/server/lowlevel/server.py +++ b/src/mcp/server/lowlevel/server.py @@ -252,32 +252,55 @@ class Server: return decorator def read_resource(self): - def decorator(func: Callable[[AnyUrl], Awaitable[str | bytes]]): + def decorator( + func: Callable[[AnyUrl], Awaitable[str | bytes | tuple[str | bytes, str]]], + ): logger.debug("Registering handler for ReadResourceRequest") async def handler(req: types.ReadResourceRequest): result = await func(req.params.uri) + + def create_content(data: str | bytes, mime_type: str): + match data: + case str() as data: + return types.TextResourceContents( + uri=req.params.uri, + text=data, + mimeType=mime_type, + ) + case bytes() as data: + import base64 + + return types.BlobResourceContents( + uri=req.params.uri, + blob=base64.urlsafe_b64encode(data).decode(), + mimeType=mime_type, + ) + match result: - case str(s): - content = types.TextResourceContents( - uri=req.params.uri, - text=s, - mimeType="text/plain", + case str() | bytes() as data: + default_mime = ( + "text/plain" + if isinstance(data, str) + else "application/octet-stream" ) - case bytes(b): - import base64 - - content = types.BlobResourceContents( - uri=req.params.uri, - blob=base64.urlsafe_b64encode(b).decode(), - mimeType="application/octet-stream", + content = create_content(data, default_mime) + return types.ServerResult( + types.ReadResourceResult( + contents=[content], + ) + ) + case (data, mime_type): + content = create_content(data, mime_type) + return types.ServerResult( + types.ReadResourceResult( + contents=[content], + ) + ) + case _: + raise ValueError( + f"Unexpected return type from read_resource: {type(result)}" ) - - return types.ServerResult( - types.ReadResourceResult( - contents=[content], - ) - ) self.request_handlers[types.ReadResourceRequest] = handler return func diff --git a/tests/issues/test_152_resource_mime_type.py b/tests/issues/test_152_resource_mime_type.py index 2c8639a..0e655c8 100644 --- a/tests/issues/test_152_resource_mime_type.py +++ b/tests/issues/test_152_resource_mime_type.py @@ -98,9 +98,9 @@ async def test_lowlevel_resource_mime_type(): @server.read_resource() async def handle_read_resource(uri: AnyUrl): if str(uri) == "test://image": - return base64_string + return (base64_string, "image/png") elif str(uri) == "test://image_bytes": - return image_bytes + return (bytes(image_bytes), "image/png") raise Exception(f"Resource not found: {uri}") # Test that resources are listed with correct mime type diff --git a/tests/server/fastmcp/servers/test_file_server.py b/tests/server/fastmcp/servers/test_file_server.py index 28773b1..ccf2580 100644 --- a/tests/server/fastmcp/servers/test_file_server.py +++ b/tests/server/fastmcp/servers/test_file_server.py @@ -88,7 +88,9 @@ async def test_list_resources(mcp: FastMCP): @pytest.mark.anyio async def test_read_resource_dir(mcp: FastMCP): - files = await mcp.read_resource("dir://test_dir") + files, mime_type = await mcp.read_resource("dir://test_dir") + assert mime_type == "text/plain" + files = json.loads(files) assert sorted([Path(f).name for f in files]) == [ @@ -100,7 +102,7 @@ async def test_read_resource_dir(mcp: FastMCP): @pytest.mark.anyio async def test_read_resource_file(mcp: FastMCP): - result = await mcp.read_resource("file://test_dir/example.py") + result, _ = await mcp.read_resource("file://test_dir/example.py") assert result == "print('hello world')" @@ -117,5 +119,5 @@ async def test_delete_file_and_check_resources(mcp: FastMCP, test_dir: Path): await mcp.call_tool( "delete_file", arguments=dict(path=str(test_dir / "example.py")) ) - result = await mcp.read_resource("file://test_dir/example.py") + result, _ = await mcp.read_resource("file://test_dir/example.py") assert result == "File not found" diff --git a/tests/server/fastmcp/test_server.py b/tests/server/fastmcp/test_server.py index c9c0aa8..61f122c 100644 --- a/tests/server/fastmcp/test_server.py +++ b/tests/server/fastmcp/test_server.py @@ -581,8 +581,8 @@ class TestContextInjection: @mcp.tool() async def tool_with_resource(ctx: Context) -> str: - data = await ctx.read_resource("test://data") - return f"Read resource: {data}" + data, mime_type = await ctx.read_resource("test://data") + return f"Read resource: {data} with mime type {mime_type}" async with client_session(mcp._mcp_server) as client: result = await client.call_tool("tool_with_resource", {})