From a03a8b9c26f297b90620406d35bdf2494c80c595 Mon Sep 17 00:00:00 2001 From: Elias Biondo Date: Sat, 24 Oct 2026 20:25:17 -0300 Subject: [PATCH] fix: return JSON objects from list tools so outputSchema matches the payload Tools that returned a bare list made FastMCP advertise a wrapped {"result": [...]} outputSchema while the text content stayed a bare JSON array (or no content at all for an empty list). MCP-to-OpenAPI bridges such as mcpo build their response model from outputSchema but return the decoded text content, so every list tool failed with a FastAPI ResponseValidationError ("Input should be a valid dictionary"). List tools now return objects: search tools {"results": [...]}, listing tools {"posts": [...]}, and reddit_get_user {"items": [...]}. --- README.md | 20 +-- pyproject.toml | 1 + .../adapters/inbound/mcp_serialization.py | 44 +++++- .../inbound/mcp_tools/mcp_post_tools.py | 10 +- .../inbound/mcp_tools/mcp_search_tools.py | 23 ++-- .../inbound/mcp_tools/mcp_user_tools.py | 24 ++-- tests/test_mcp_tools.py | 125 ++++++++++++++++++ tests/test_serialization.py | 8 ++ uv.lock | 2 + 9 files changed, 225 insertions(+), 32 deletions(-) create mode 100644 tests/test_mcp_tools.py diff --git a/README.md b/README.md index b8841e2..c08c998 100644 --- a/README.md +++ b/README.md @@ -127,14 +127,18 @@ Add to your `.cursor/mcp.json`: ## 3. Available Tools -| Tool | Description | Key Arguments | -|------|-------------|---------------| -| `search` | Search Reddit for posts | `query`, `limit`, `sort` | -| `search_subreddit` | Search within a subreddit | `subreddit`, `query`, `limit`, `sort` | -| `get_post` | Get post details + comment tree | `permalink` | -| `get_subreddit_posts` | Get subreddit listing | `subreddit`, `limit`, `category`, `time_filter` | -| `get_user` | Get user's activity feed | `username`, `limit` | -| `get_user_posts` | Get user's submitted posts | `username`, `limit`, `category`, `time_filter` | +| Tool | Description | Key Arguments | Returns | +|------|-------------|---------------|---------| +| `search` | Search Reddit for posts | `query`, `limit`, `sort` | `{"results": [...]}` | +| `search_subreddit` | Search within a subreddit | `subreddit`, `query`, `limit`, `sort` | `{"results": [...]}` | +| `get_post` | Get post details + comment tree | `permalink` | post object | +| `get_subreddit_posts` | Get subreddit listing | `subreddit`, `limit`, `category`, `time_filter` | `{"posts": [...]}` | +| `get_user` | Get user's activity feed | `username`, `limit` | `{"items": [...]}` | +| `get_user_posts` | Get user's submitted posts | `username`, `limit`, `category`, `time_filter` | `{"posts": [...]}` | + +Every tool returns a JSON object (never a bare array) that matches its advertised +`outputSchema`, so MCP-to-OpenAPI bridges such as +[mcpo](https://github.com/open-webui/mcpo) / Open WebUI can validate the response. ### Tool Details diff --git a/pyproject.toml b/pyproject.toml index 7bda883..336f7eb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -38,6 +38,7 @@ reddit-no-auth-mcp-server = "reddit_mcp_server.adapters.inbound.mcp_cli_adapter: [dependency-groups] dev = [ + "jsonschema>=4.0.0", "pre-commit>=4.2.0", "pytest>=8.3.5", "pytest-asyncio>=0.26.0", diff --git a/src/reddit_mcp_server/adapters/inbound/mcp_serialization.py b/src/reddit_mcp_server/adapters/inbound/mcp_serialization.py index b2ada56..2b67a15 100644 --- a/src/reddit_mcp_server/adapters/inbound/mcp_serialization.py +++ b/src/reddit_mcp_server/adapters/inbound/mcp_serialization.py @@ -1,7 +1,25 @@ """MCP serialization utilities for converting redd domain models to JSON-serializable dicts.""" from dataclasses import asdict -from typing import Any +from typing import Any, TypedDict + + +class PostListResponse(TypedDict): + """Tool response for listings of posts (subreddit or user submissions).""" + + posts: list[dict[str, Any]] + + +class SearchResultsResponse(TypedDict): + """Tool response for search queries.""" + + results: list[dict[str, Any]] + + +class UserActivityResponse(TypedDict): + """Tool response for a user's activity feed (posts and comments).""" + + items: list[dict[str, Any]] class McpSerializer: @@ -9,14 +27,36 @@ class McpSerializer: Provides helper methods to convert frozen dataclasses into plain dicts suitable for MCP tool responses. + + Tools always return a JSON *object* (never a bare list). FastMCP wraps + non-object return values as ``{"result": ...}`` in the advertised output + schema while the text content stays a bare JSON array, so MCP-to-OpenAPI + bridges such as mcpo (which validate the text content against the output + schema) reject list results. The ``*Response`` types above keep the text + content, the structured content, and the output schema identical. """ @staticmethod def serialize(model: object) -> dict[str, Any]: """Convert a frozen dataclass to a JSON-serializable dict, stripping None values.""" - return {k: v for k, v in asdict(model).items() if v is not None} # type: ignore[arg-type] + return {k: v for k, v in asdict(model).items() if v is not None} # type: ignore[call-overload] @staticmethod def serialize_list(models: list) -> list[dict[str, Any]]: """Serialize a list of dataclass models.""" return [McpSerializer.serialize(m) for m in models] + + @staticmethod + def post_list(models: list) -> PostListResponse: + """Serialize a list of posts as ``{"posts": [...]}``.""" + return {"posts": McpSerializer.serialize_list(models)} + + @staticmethod + def search_results(models: list) -> SearchResultsResponse: + """Serialize search results as ``{"results": [...]}``.""" + return {"results": McpSerializer.serialize_list(models)} + + @staticmethod + def user_activity(models: list) -> UserActivityResponse: + """Serialize a user's activity feed as ``{"items": [...]}``.""" + return {"items": McpSerializer.serialize_list(models)} diff --git a/src/reddit_mcp_server/adapters/inbound/mcp_tools/mcp_post_tools.py b/src/reddit_mcp_server/adapters/inbound/mcp_tools/mcp_post_tools.py index 156dab7..7894860 100644 --- a/src/reddit_mcp_server/adapters/inbound/mcp_tools/mcp_post_tools.py +++ b/src/reddit_mcp_server/adapters/inbound/mcp_tools/mcp_post_tools.py @@ -5,7 +5,7 @@ from fastmcp import Context, FastMCP from reddit_mcp_server.adapters.inbound.mcp_error_mapping import McpErrorMapper -from reddit_mcp_server.adapters.inbound.mcp_serialization import McpSerializer +from reddit_mcp_server.adapters.inbound.mcp_serialization import McpSerializer, PostListResponse from reddit_mcp_server.adapters.inbound.mcp_tools._base import McpToolRegistrar from reddit_mcp_server.application.post_service import PostService @@ -53,7 +53,9 @@ async def reddit_get_post( " limit: Maximum number of posts (default: 25, max: 100)\n" " category: Listing category (hot, top, new, rising). Default: hot\n" " time_filter: Time window for top listings " - "(hour, day, week, month, year, all). Default: all" + "(hour, day, week, month, year, all). Default: all\n\n" + "Returns:\n" + " An object with a 'posts' array." ), ) async def reddit_get_subreddit_posts( @@ -62,11 +64,11 @@ async def reddit_get_subreddit_posts( limit: int = 25, category: str | None = None, time_filter: str | None = None, - ) -> list[dict[str, Any]]: + ) -> PostListResponse: try: results = await service.get_subreddit_posts( subreddit, limit=limit, category=category, time_filter=time_filter ) - return McpSerializer.serialize_list(results) + return McpSerializer.post_list(results) except Exception as e: McpErrorMapper.map(e, "reddit_get_subreddit_posts") diff --git a/src/reddit_mcp_server/adapters/inbound/mcp_tools/mcp_search_tools.py b/src/reddit_mcp_server/adapters/inbound/mcp_tools/mcp_search_tools.py index f9fbfc8..99cabb4 100644 --- a/src/reddit_mcp_server/adapters/inbound/mcp_tools/mcp_search_tools.py +++ b/src/reddit_mcp_server/adapters/inbound/mcp_tools/mcp_search_tools.py @@ -1,11 +1,12 @@ """Search-related MCP tool registrations.""" -from typing import Any - from fastmcp import Context, FastMCP from reddit_mcp_server.adapters.inbound.mcp_error_mapping import McpErrorMapper -from reddit_mcp_server.adapters.inbound.mcp_serialization import McpSerializer +from reddit_mcp_server.adapters.inbound.mcp_serialization import ( + McpSerializer, + SearchResultsResponse, +) from reddit_mcp_server.adapters.inbound.mcp_tools._base import McpToolRegistrar from reddit_mcp_server.application.search_service import SearchService @@ -35,7 +36,9 @@ def register(self, mcp: FastMCP) -> None: " limit: Maximum number of results to return " "(default: 25, max: 100)\n" " sort: Sort order for results " - "(relevance, hot, top, new, comments)" + "(relevance, hot, top, new, comments)\n\n" + "Returns:\n" + " An object with a 'results' array." ), ) async def reddit_search( @@ -43,10 +46,10 @@ async def reddit_search( ctx: Context, limit: int = 25, sort: str | None = None, - ) -> list[dict[str, Any]]: + ) -> SearchResultsResponse: try: results = await service.search(query, limit=limit, sort=sort) - return McpSerializer.serialize_list(results) + return McpSerializer.search_results(results) except Exception as e: McpErrorMapper.map(e, "reddit_search") @@ -61,7 +64,9 @@ async def reddit_search( " limit: Maximum number of results to return " "(default: 25, max: 100)\n" " sort: Sort order for results " - "(relevance, hot, top, new, comments)" + "(relevance, hot, top, new, comments)\n\n" + "Returns:\n" + " An object with a 'results' array." ), ) async def reddit_search_subreddit( @@ -70,9 +75,9 @@ async def reddit_search_subreddit( ctx: Context, limit: int = 25, sort: str | None = None, - ) -> list[dict[str, Any]]: + ) -> SearchResultsResponse: try: results = await service.search_subreddit(subreddit, query, limit=limit, sort=sort) - return McpSerializer.serialize_list(results) + return McpSerializer.search_results(results) except Exception as e: McpErrorMapper.map(e, "reddit_search_subreddit") diff --git a/src/reddit_mcp_server/adapters/inbound/mcp_tools/mcp_user_tools.py b/src/reddit_mcp_server/adapters/inbound/mcp_tools/mcp_user_tools.py index b0969ed..df7cc0f 100644 --- a/src/reddit_mcp_server/adapters/inbound/mcp_tools/mcp_user_tools.py +++ b/src/reddit_mcp_server/adapters/inbound/mcp_tools/mcp_user_tools.py @@ -1,11 +1,13 @@ """User-related MCP tool registrations.""" -from typing import Any - from fastmcp import Context, FastMCP from reddit_mcp_server.adapters.inbound.mcp_error_mapping import McpErrorMapper -from reddit_mcp_server.adapters.inbound.mcp_serialization import McpSerializer +from reddit_mcp_server.adapters.inbound.mcp_serialization import ( + McpSerializer, + PostListResponse, + UserActivityResponse, +) from reddit_mcp_server.adapters.inbound.mcp_tools._base import McpToolRegistrar from reddit_mcp_server.application.user_service import UserService @@ -34,17 +36,19 @@ def register(self, mcp: FastMCP) -> None: " username: Reddit username without u/ prefix " "(e.g., 'spez', 'GallowBoob')\n" " limit: Maximum number of activity items " - "(default: 25, max: 100)" + "(default: 25, max: 100)\n\n" + "Returns:\n" + " An object with an 'items' array of posts and comments." ), ) async def reddit_get_user( username: str, ctx: Context, limit: int = 25, - ) -> list[dict[str, Any]]: + ) -> UserActivityResponse: try: results = await service.get_user(username, limit=limit) - return McpSerializer.serialize_list(results) + return McpSerializer.user_activity(results) except Exception as e: McpErrorMapper.map(e, "reddit_get_user") @@ -60,7 +64,9 @@ async def reddit_get_user( " category: Listing category (hot, top, new). " "Default: new\n" " time_filter: Time window for top listings " - "(hour, day, week, month, year, all). Default: all" + "(hour, day, week, month, year, all). Default: all\n\n" + "Returns:\n" + " An object with a 'posts' array." ), ) async def reddit_get_user_posts( @@ -69,11 +75,11 @@ async def reddit_get_user_posts( limit: int = 25, category: str | None = None, time_filter: str | None = None, - ) -> list[dict[str, Any]]: + ) -> PostListResponse: try: results = await service.get_user_posts( username, limit=limit, category=category, time_filter=time_filter ) - return McpSerializer.serialize_list(results) + return McpSerializer.post_list(results) except Exception as e: McpErrorMapper.map(e, "reddit_get_user_posts") diff --git a/tests/test_mcp_tools.py b/tests/test_mcp_tools.py new file mode 100644 index 0000000..514956f --- /dev/null +++ b/tests/test_mcp_tools.py @@ -0,0 +1,125 @@ +"""End-to-end tests for the MCP tool surface using an in-process FastMCP client. + +These guard the contract that MCP-to-OpenAPI bridges such as mcpo rely on: +every tool advertises an object ``outputSchema`` and both the structured +content and the JSON text content of a result validate against it. +""" + +import json +from types import SimpleNamespace +from unittest.mock import AsyncMock + +import jsonschema +import pytest +from fastmcp import Client +from redd.domain.models import PostDetail, SearchResult, SubredditPost, UserItem + +from reddit_mcp_server.adapters.inbound.mcp_server_adapter import McpServerAdapter +from reddit_mcp_server.application.post_service import PostService +from reddit_mcp_server.application.search_service import SearchService +from reddit_mcp_server.application.user_service import UserService + +_POST = SubredditPost( + title="Hot Post", + author="user2", + permalink="/r/test/comments/abc/hot_post/", + score=100, + num_comments=10, + created_utc=1700000000.0, + subreddit="test", + url="https://reddit.com/r/test/hot", +) +_SEARCH_RESULT = SearchResult( + title="Test Post", + url="https://reddit.com/r/test/1", + subreddit="test", + description="A test post", +) +_USER_ITEM = UserItem( + kind="t3", + subreddit="test", + url="https://reddit.com/r/test/1", + created_utc=1700000000.0, + title="My post", +) +_POST_DETAIL = PostDetail( + title="Post Detail", + author="user1", + body="body text", + score=42, + url="https://reddit.com/r/test/post/1", + subreddit="test", + created_utc=1700000000.0, + num_comments=5, + comments=[], +) + +# tool name -> (arguments, top-level key holding the list, or None for objects) +_TOOLS: dict[str, tuple[dict, str | None]] = { + "reddit_search": ({"query": "python"}, "results"), + "reddit_search_subreddit": ({"subreddit": "Python", "query": "async"}, "results"), + "reddit_get_subreddit_posts": ({"subreddit": "Python"}, "posts"), + "reddit_get_user": ({"username": "spez"}, "items"), + "reddit_get_user_posts": ({"username": "spez"}, "posts"), + "reddit_get_post": ({"permalink": "/r/test/comments/abc/post/"}, None), +} + + +def _build_server(*, empty: bool): + reddit = AsyncMock() + reddit.search.return_value = [] if empty else [_SEARCH_RESULT] + reddit.search_subreddit.return_value = [] if empty else [_SEARCH_RESULT] + reddit.get_subreddit_posts.return_value = [] if empty else [_POST] + reddit.get_user.return_value = [] if empty else [_USER_ITEM] + reddit.get_user_posts.return_value = [] if empty else [_POST] + reddit.get_post.return_value = _POST_DETAIL + + container = SimpleNamespace( + reddit=reddit, + post_service=PostService(reddit), + search_service=SearchService(reddit), + user_service=UserService(reddit), + ) + return McpServerAdapter(container).create() # type: ignore[arg-type] + + +async def test_every_tool_advertises_a_plain_object_output_schema(): + async with Client(_build_server(empty=False)) as client: + tools = {tool.name: tool for tool in await client.list_tools()} + + assert set(tools) == set(_TOOLS) + for name, (_, list_key) in _TOOLS.items(): + schema = tools[name].outputSchema + assert schema is not None, name + assert schema["type"] == "object", name + # FastMCP only adds this marker when it has to wrap a non-object return + # value, which is exactly what breaks mcpo/OpenWebUI (issue #1). + assert "x-fastmcp-wrap-result" not in schema, name + if list_key is not None: + assert schema["required"] == [list_key], name + assert schema["properties"][list_key]["type"] == "array", name + + +@pytest.mark.parametrize("empty", [False, True], ids=["with-results", "empty"]) +@pytest.mark.parametrize("tool_name", list(_TOOLS)) +async def test_tool_result_matches_output_schema(tool_name: str, empty: bool): + arguments, list_key = _TOOLS[tool_name] + + async with Client(_build_server(empty=empty)) as client: + tool = next(t for t in await client.list_tools() if t.name == tool_name) + result = await client.call_tool_mcp(tool_name, arguments) + + assert not result.isError + assert tool.outputSchema is not None + jsonschema.validate(result.structuredContent, tool.outputSchema) + + # mcpo builds its FastAPI response model from outputSchema but returns the + # JSON-decoded *text* content, so that must be the same object. + assert len(result.content) == 1 + text_payload = json.loads(result.content[0].text) # type: ignore[union-attr] + assert text_payload == result.structuredContent + jsonschema.validate(text_payload, tool.outputSchema) + + if list_key is not None: + assert isinstance(text_payload[list_key], list) + assert len(text_payload[list_key]) == (0 if empty else 1) diff --git a/tests/test_serialization.py b/tests/test_serialization.py index 84bcd84..f875a5b 100644 --- a/tests/test_serialization.py +++ b/tests/test_serialization.py @@ -39,3 +39,11 @@ def test_serialize_list(self): def test_serialize_list_empty(self): assert McpSerializer.serialize_list([]) == [] + + def test_list_responses_are_objects(self): + models = [_FakeModel(name="a", value=1)] + + assert McpSerializer.post_list(models) == {"posts": [{"name": "a", "value": 1}]} + assert McpSerializer.search_results(models) == {"results": [{"name": "a", "value": 1}]} + assert McpSerializer.user_activity(models) == {"items": [{"name": "a", "value": 1}]} + assert McpSerializer.post_list([]) == {"posts": []} diff --git a/uv.lock b/uv.lock index 72f5a33..dec7d27 100644 --- a/uv.lock +++ b/uv.lock @@ -1178,6 +1178,7 @@ dependencies = [ [package.dev-dependencies] dev = [ + { name = "jsonschema" }, { name = "pre-commit" }, { name = "pytest" }, { name = "pytest-asyncio" }, @@ -1194,6 +1195,7 @@ requires-dist = [ [package.metadata.requires-dev] dev = [ + { name = "jsonschema", specifier = ">=4.0.0" }, { name = "pre-commit", specifier = ">=4.2.0" }, { name = "pytest", specifier = ">=8.3.5" }, { name = "pytest-asyncio", specifier = ">=0.26.0" },