diff --git a/CLAUDE.md b/CLAUDE.md index 4516da44..e95b75cd 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/README.md b/README.md index 164a2ce7..310bb35b 100644 --- a/README.md +++ b/README.md @@ -218,7 +218,7 @@ async def long_task(files: list[str], ctx: Context) -> str: for i, file in enumerate(files): ctx.info(f"Processing {file}") await ctx.report_progress(i, len(files)) - data = await ctx.read_resource(f"file://{file}") + data, mime_type = await ctx.read_resource(f"file://{file}") return "Processing complete" ``` @@ -436,7 +436,7 @@ async def run(): tools = await session.list_tools() # Read a resource - resource = await session.read_resource("file://some/path") + content, mime_type = await session.read_resource("file://some/path") # Call a tool result = await session.call_tool("tool-name", arguments={"arg1": "value"}) diff --git a/src/mcp/server/fastmcp/server.py b/src/mcp/server/fastmcp/server.py index 45f17914..e8b311ee 100644 --- a/src/mcp/server/fastmcp/server.py +++ b/src/mcp/server/fastmcp/server.py @@ -20,6 +20,7 @@ from mcp.server.fastmcp.utilities.logging import configure_logging, get_logger from mcp.server.fastmcp.utilities.types import Image from mcp.server.lowlevel import Server as MCPServer +from mcp.server.lowlevel.helper_types import ReadResourceContents from mcp.server.sse import SseServerTransport from mcp.server.stdio import stdio_server from mcp.shared.context import RequestContext @@ -197,14 +198,16 @@ async def list_resource_templates(self) -> list[MCPResourceTemplate]: for template in templates ] - async def read_resource(self, uri: AnyUrl | str) -> str | bytes: + async def read_resource(self, uri: AnyUrl | str) -> ReadResourceContents: """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 ReadResourceContents(content=content, mime_type=resource.mime_type) except Exception as e: logger.error(f"Error reading resource {uri}: {e}") raise ResourceError(str(e)) @@ -606,7 +609,7 @@ async def report_progress( 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) -> ReadResourceContents: """Read a resource by URI. Args: diff --git a/src/mcp/server/lowlevel/helper_types.py b/src/mcp/server/lowlevel/helper_types.py new file mode 100644 index 00000000..3d09b250 --- /dev/null +++ b/src/mcp/server/lowlevel/helper_types.py @@ -0,0 +1,9 @@ +from dataclasses import dataclass + + +@dataclass +class ReadResourceContents: + """Contents returned from a read_resource call.""" + + content: str | bytes + mime_type: str | None = None diff --git a/src/mcp/server/lowlevel/server.py b/src/mcp/server/lowlevel/server.py index c3f2abfe..13d4fd91 100644 --- a/src/mcp/server/lowlevel/server.py +++ b/src/mcp/server/lowlevel/server.py @@ -74,6 +74,7 @@ async def main(): from pydantic import AnyUrl import mcp.types as types +from mcp.server.lowlevel.helper_types import ReadResourceContents from mcp.server.models import InitializationOptions from mcp.server.session import ServerSession from mcp.server.stdio import stdio_server as stdio_server @@ -252,25 +253,45 @@ async def handler(_: Any): return decorator def read_resource(self): - def decorator(func: Callable[[AnyUrl], Awaitable[str | bytes]]): + def decorator( + func: Callable[[AnyUrl], Awaitable[str | bytes | ReadResourceContents]], + ): 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 | None): + match data: + case str() as data: + return types.TextResourceContents( + uri=req.params.uri, + text=data, + mimeType=mime_type or "text/plain", + ) + case bytes() as data: + import base64 + + return types.BlobResourceContents( + uri=req.params.uri, + blob=base64.urlsafe_b64encode(data).decode(), + mimeType=mime_type or "application/octet-stream", + ) + match result: - case str(s): - content = types.TextResourceContents( - uri=req.params.uri, - text=s, - mimeType="text/plain", + case str() | bytes() as data: + warnings.warn( + "Returning str or bytes from read_resource is deprecated. " + "Use ReadResourceContents instead.", + DeprecationWarning, + stacklevel=2, ) - 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, None) + case ReadResourceContents() as contents: + content = create_content(contents.content, contents.mime_type) + case _: + raise ValueError( + f"Unexpected return type from read_resource: {type(result)}" ) return types.ServerResult( diff --git a/tests/issues/test_152_resource_mime_type.py b/tests/issues/test_152_resource_mime_type.py new file mode 100644 index 00000000..7a1b6606 --- /dev/null +++ b/tests/issues/test_152_resource_mime_type.py @@ -0,0 +1,146 @@ +import base64 + +import pytest +from pydantic import AnyUrl + +from mcp import types +from mcp.server.fastmcp import FastMCP +from mcp.server.lowlevel import Server +from mcp.server.lowlevel.helper_types import ReadResourceContents +from mcp.shared.memory import ( + create_connected_server_and_client_session as client_session, +) + +pytestmark = pytest.mark.anyio + + +async def test_fastmcp_resource_mime_type(): + """Test that mime_type parameter is respected for resources.""" + mcp = FastMCP("test") + + # Create a small test image as bytes + image_bytes = b"fake_image_data" + base64_string = base64.b64encode(image_bytes).decode("utf-8") + + @mcp.resource("test://image", mime_type="image/png") + def get_image_as_string() -> str: + """Return a test image as base64 string.""" + return base64_string + + @mcp.resource("test://image_bytes", mime_type="image/png") + def get_image_as_bytes() -> bytes: + """Return a test image as bytes.""" + return image_bytes + + # Test that resources are listed with correct mime type + async with client_session(mcp._mcp_server) as client: + # List resources and verify mime types + resources = await client.list_resources() + assert resources.resources is not None + + mapping = {str(r.uri): r for r in resources.resources} + + # Find our resources + string_resource = mapping["test://image"] + bytes_resource = mapping["test://image_bytes"] + + # Verify mime types + assert ( + string_resource.mimeType == "image/png" + ), "String resource mime type not respected" + assert ( + bytes_resource.mimeType == "image/png" + ), "Bytes resource mime type not respected" + + # Also verify the content can be read correctly + string_result = await client.read_resource(AnyUrl("test://image")) + assert len(string_result.contents) == 1 + assert ( + getattr(string_result.contents[0], "text") == base64_string + ), "Base64 string mismatch" + assert ( + string_result.contents[0].mimeType == "image/png" + ), "String content mime type not preserved" + + bytes_result = await client.read_resource(AnyUrl("test://image_bytes")) + assert len(bytes_result.contents) == 1 + assert ( + base64.b64decode(getattr(bytes_result.contents[0], "blob")) == image_bytes + ), "Bytes mismatch" + assert ( + bytes_result.contents[0].mimeType == "image/png" + ), "Bytes content mime type not preserved" + + +async def test_lowlevel_resource_mime_type(): + """Test that mime_type parameter is respected for resources.""" + server = Server("test") + + # Create a small test image as bytes + image_bytes = b"fake_image_data" + base64_string = base64.b64encode(image_bytes).decode("utf-8") + + # Create test resources with specific mime types + test_resources = [ + types.Resource( + uri=AnyUrl("test://image"), name="test image", mimeType="image/png" + ), + types.Resource( + uri=AnyUrl("test://image_bytes"), + name="test image bytes", + mimeType="image/png", + ), + ] + + @server.list_resources() + async def handle_list_resources(): + return test_resources + + @server.read_resource() + async def handle_read_resource(uri: AnyUrl): + if str(uri) == "test://image": + return ReadResourceContents(content=base64_string, mime_type="image/png") + elif str(uri) == "test://image_bytes": + return ReadResourceContents( + content=bytes(image_bytes), mime_type="image/png" + ) + raise Exception(f"Resource not found: {uri}") + + # Test that resources are listed with correct mime type + async with client_session(server) as client: + # List resources and verify mime types + resources = await client.list_resources() + assert resources.resources is not None + + mapping = {str(r.uri): r for r in resources.resources} + + # Find our resources + string_resource = mapping["test://image"] + bytes_resource = mapping["test://image_bytes"] + + # Verify mime types + assert ( + string_resource.mimeType == "image/png" + ), "String resource mime type not respected" + assert ( + bytes_resource.mimeType == "image/png" + ), "Bytes resource mime type not respected" + + # Also verify the content can be read correctly + string_result = await client.read_resource(AnyUrl("test://image")) + assert len(string_result.contents) == 1 + assert ( + getattr(string_result.contents[0], "text") == base64_string + ), "Base64 string mismatch" + assert ( + string_result.contents[0].mimeType == "image/png" + ), "String content mime type not preserved" + + bytes_result = await client.read_resource(AnyUrl("test://image_bytes")) + assert len(bytes_result.contents) == 1 + assert ( + base64.b64decode(getattr(bytes_result.contents[0], "blob")) == image_bytes + ), "Bytes mismatch" + assert ( + bytes_result.contents[0].mimeType == "image/png" + ), "Bytes content mime type not preserved" diff --git a/tests/server/fastmcp/servers/test_file_server.py b/tests/server/fastmcp/servers/test_file_server.py index 28773b1d..edaaa159 100644 --- a/tests/server/fastmcp/servers/test_file_server.py +++ b/tests/server/fastmcp/servers/test_file_server.py @@ -88,8 +88,10 @@ 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 = json.loads(files) + res = await mcp.read_resource("dir://test_dir") + assert res.mime_type == "text/plain" + + files = json.loads(res.content) assert sorted([Path(f).name for f in files]) == [ "config.json", @@ -100,8 +102,8 @@ 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") - assert result == "print('hello world')" + res = await mcp.read_resource("file://test_dir/example.py") + assert res.content == "print('hello world')" @pytest.mark.anyio @@ -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") - assert result == "File not found" + res = await mcp.read_resource("file://test_dir/example.py") + assert res.content == "File not found" diff --git a/tests/server/fastmcp/test_server.py b/tests/server/fastmcp/test_server.py index c9c0aa8f..d90e9939 100644 --- a/tests/server/fastmcp/test_server.py +++ b/tests/server/fastmcp/test_server.py @@ -581,8 +581,8 @@ def test_resource() -> str: @mcp.tool() async def tool_with_resource(ctx: Context) -> str: - data = await ctx.read_resource("test://data") - return f"Read resource: {data}" + r = await ctx.read_resource("test://data") + return f"Read resource: {r.content} with mime type {r.mime_type}" async with client_session(mcp._mcp_server) as client: result = await client.call_tool("tool_with_resource", {}) diff --git a/tests/server/test_read_resource.py b/tests/server/test_read_resource.py new file mode 100644 index 00000000..de00bc3d --- /dev/null +++ b/tests/server/test_read_resource.py @@ -0,0 +1,109 @@ +from pathlib import Path +from tempfile import NamedTemporaryFile + +import pytest +from pydantic import AnyUrl, FileUrl + +import mcp.types as types +from mcp.server.lowlevel.server import ReadResourceContents, Server + + +@pytest.fixture +def temp_file(): + """Create a temporary file for testing.""" + with NamedTemporaryFile(mode="w", delete=False) as f: + f.write("test content") + path = Path(f.name).resolve() + yield path + try: + path.unlink() + except FileNotFoundError: + pass + + +@pytest.mark.anyio +async def test_read_resource_text(temp_file: Path): + server = Server("test") + + @server.read_resource() + async def read_resource(uri: AnyUrl) -> ReadResourceContents: + return ReadResourceContents(content="Hello World", mime_type="text/plain") + + # Get the handler directly from the server + handler = server.request_handlers[types.ReadResourceRequest] + + # Create a request + request = types.ReadResourceRequest( + method="resources/read", + params=types.ReadResourceRequestParams(uri=FileUrl(temp_file.as_uri())), + ) + + # Call the handler + result = await handler(request) + assert isinstance(result.root, types.ReadResourceResult) + assert len(result.root.contents) == 1 + + content = result.root.contents[0] + assert isinstance(content, types.TextResourceContents) + assert content.text == "Hello World" + assert content.mimeType == "text/plain" + + +@pytest.mark.anyio +async def test_read_resource_binary(temp_file: Path): + server = Server("test") + + @server.read_resource() + async def read_resource(uri: AnyUrl) -> ReadResourceContents: + return ReadResourceContents( + content=b"Hello World", mime_type="application/octet-stream" + ) + + # Get the handler directly from the server + handler = server.request_handlers[types.ReadResourceRequest] + + # Create a request + request = types.ReadResourceRequest( + method="resources/read", + params=types.ReadResourceRequestParams(uri=FileUrl(temp_file.as_uri())), + ) + + # Call the handler + result = await handler(request) + assert isinstance(result.root, types.ReadResourceResult) + assert len(result.root.contents) == 1 + + content = result.root.contents[0] + assert isinstance(content, types.BlobResourceContents) + assert content.mimeType == "application/octet-stream" + + +@pytest.mark.anyio +async def test_read_resource_default_mime(temp_file: Path): + server = Server("test") + + @server.read_resource() + async def read_resource(uri: AnyUrl) -> ReadResourceContents: + return ReadResourceContents( + content="Hello World", + # No mime_type specified, should default to text/plain + ) + + # Get the handler directly from the server + handler = server.request_handlers[types.ReadResourceRequest] + + # Create a request + request = types.ReadResourceRequest( + method="resources/read", + params=types.ReadResourceRequestParams(uri=FileUrl(temp_file.as_uri())), + ) + + # Call the handler + result = await handler(request) + assert isinstance(result.root, types.ReadResourceResult) + assert len(result.root.contents) == 1 + + content = result.root.contents[0] + assert isinstance(content, types.TextResourceContents) + assert content.text == "Hello World" + assert content.mimeType == "text/plain"