Pin the MCP tool surface and give every tool an output schema

mcp_server.py had no tests, and it is the file least able to afford that:
`_tool_names` walks the route tree through three version-dependent shapes
(DefaultPlaceholder unwrapping, nested `.routes`, FastAPI >= 0.141's
`original_router`), each a reasonable guess about a library's internals.
When one stops resolving, tools quietly revert to generated operationIds
like `search_arxiv_serp_search_arxiv_post`, tool selection degrades, and
nothing fails.

tests/test_mcp_server.py pins the surface an agent actually sees: the
curated six-tool set, handler names rather than operationIds, the
/ops/* and single-backend exclusions holding, and anchored exclusion
patterns.

It also caught a live defect. FastMCP derives tool schemas from the app's
OpenAPI document, and three handlers had no return annotation, so FastAPI
emitted an empty 200 schema and those tools advertised no output schema at
all - `search`, `search_arxiv` and `search_google_scholar`. `search` is the
tool MCP_INSTRUCTIONS pushes agents toward first. Adding the three
`-> SerpResults` annotations types all six, and the tests keep them typed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MSNSYnceqvdVz7Csis4K9e

app.py

@@ -154,7 +154,7 @@ async def _run_serp_queries(fn, params: SerpQuery, context: str) -> SerpResults:
 
 
 @serp_router.post("/search_scholar")
-async def search_google_scholar(params: SerpQuery):
+async def search_google_scholar(params: SerpQuery) -> SerpResults:
     """Queries google scholar for the specified query"""
     logging.info(f"Searching Google Scholar for queries: {params.queries}")
     return await _run_serp_queries(
@@ -162,7 +162,7 @@ async def search_google_scholar(params: SerpQuery):
 
 
 @serp_router.post("/search_arxiv")
-async def search_arxiv(params: SerpQuery):
+async def search_arxiv(params: SerpQuery) -> SerpResults:
     """Searches arxiv for the specified queries and returns the found documents."""
     logging.info(f"Searching Arxiv for queries: {params.queries}")
     return await _run_serp_queries(
@@ -256,7 +256,7 @@ async def _search_one(q: str, n_results: int) -> tuple[str, list[dict], Optional
 
 
 @serp_router.post("/search")
-async def search(params: SerpQuery):
+async def search(params: SerpQuery) -> SerpResults:
     """Attempts to search the specified queries using ALL backends"""
     outcomes = await asyncio.gather(*[_search_one(q, params.n_results) for q in params.queries])
 

tests/test_mcp_server.py

@@ -0,0 +1,102 @@
+"""The MCP tool surface.
+
+mcp_server.py had no tests, and it degrades silently. `_tool_names` walks
+the route tree through three version-dependent shapes - DefaultPlaceholder
+unwrapping, nested `.routes`, and FastAPI >= 0.141's `original_router` -
+each a reasonable guess about a library's internals. If any stops
+resolving, tools quietly revert to names like
+`search_arxiv_serp_search_arxiv_post`, tool selection degrades, and
+nothing goes red.
+
+These pin the surface an agent actually sees, so a dependency bump reports
+the breakage instead of shipping it.
+"""
+
+import pytest
+
+import app as app_module
+from mcp_server import EXCLUDED_ROUTES, _tool_names
+
+# The curated set. /ops/* and the single-backend search endpoints are
+# deliberately excluded (see EXCLUDED_ROUTES): they stay available over
+# REST, but exposing them as tools invites a model to pick a path that
+# skips the fallback chains.
+EXPECTED_TOOLS = {
+    "search",
+    "search_arxiv",
+    "search_google_scholar",
+    "search_patents",
+    "scrap_patent",
+    "scrap_patents",
+}
+
+
+@pytest.fixture
+async def tools():
+    return {t.name: t for t in await app_module.mcp.list_tools()}
+
+
+async def test_the_exposed_tool_set_is_the_curated_one(tools):
+    assert set(tools) == EXPECTED_TOOLS
+
+
+async def test_tool_names_are_handler_names_not_generated_operation_ids(tools):
+    """FastAPI's generated operationIds (`search_arxiv_serp_search_arxiv_post`)
+    are what an LLM would otherwise see. _tool_names maps them back to the
+    Python handler name.
+    """
+    for name in tools:
+        assert "_post" not in name and "_get" not in name
+        assert "_serp_" not in name and "_scrap_" not in name
+
+
+async def test_operation_ids_all_resolve_to_a_handler_name():
+    """If the route walk stops finding routes, this mapping silently
+    empties and every tool falls back to its generated operationId."""
+    names = _tool_names(app_module.app)
+
+    assert len(names) >= len(EXPECTED_TOOLS)
+    assert "search_arxiv_serp_search_arxiv_post" in names
+    assert names["search_arxiv_serp_search_arxiv_post"] == "search_arxiv"
+
+
+@pytest.mark.parametrize("excluded", [
+    "ops_keyword_search", "ops_get_patent", "ops_get_patents_bulk",
+    "search_brave", "search_bing", "search_duck",
+])
+async def test_excluded_endpoints_are_not_exposed_as_tools(tools, excluded):
+    assert excluded not in tools
+
+
+def test_excluded_routes_patterns_are_anchored():
+    """An unanchored pattern would silently exclude more than intended."""
+    for pattern in EXCLUDED_ROUTES:
+        assert pattern.startswith("^")
+
+
+# ---------------------------------- output schemas ----------------------------------
+
+
+@pytest.mark.parametrize("tool_name", sorted(EXPECTED_TOOLS))
+async def test_every_tool_declares_its_output_schema(tools, tool_name):
+    """FastMCP builds tool schemas from the app's OpenAPI document, so a
+    handler with no return annotation advertises an untyped result to the
+    model. `search` - the tool the instructions push agents toward first -
+    was one of three that did.
+    """
+    assert tools[tool_name].output_schema, (
+        f"tool '{tool_name}' advertises no output schema; "
+        "the handler is probably missing a return annotation")
+
+
+@pytest.mark.parametrize("path", [
+    "/serp/search", "/serp/search_arxiv", "/serp/search_scholar",
+    "/serp/search_patents", "/scrap/scrap_patent/{patent_id}",
+    "/scrap/scrap_patents_bulk",
+])
+def test_every_exposed_route_declares_a_200_response_model(path):
+    schema = app_module.app.openapi()
+    for operation in schema["paths"][path].values():
+        content = (operation.get("responses", {}).get("200", {})
+                   .get("content", {}).get("application/json", {}).get("schema", {}))
+        assert content, f"{path} has no 200 response schema"