From 6ef04f32a1c7d7060ece8018ea4eeb6bd4c26d82 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 10 Sep 2026 16:34:18 +0000 Subject: [PATCH 1/5] chore(deps): update mcp requirement from <2,>=1.28.1 to >=2.2.0,<3 Updates the requirements on [mcp](https://github.com/modelcontextprotocol/python-sdk) to permit the latest version. - [Release notes](https://github.com/modelcontextprotocol/python-sdk/releases) - [Changelog](https://github.com/modelcontextprotocol/python-sdk/blob/main/RELEASE.md) - [Commits](https://github.com/modelcontextprotocol/python-sdk/compare/v1.28.1...v2.2.0) --- updated-dependencies: - dependency-name: mcp dependency-version: 2.2.0 dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- requirements/base.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/base.txt b/requirements/base.txt index 6769a9256..c488622ee 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -80,7 +80,7 @@ posthog==7.47.0 # https://github.com/posthog/posthog-python # Model Context Protocol # ------------------------------------------------------------------------------ -mcp>=1.28.1,<2 # https://github.com/modelcontextprotocol/python-sdk +mcp>=2.2.0,<3 # https://github.com/modelcontextprotocol/python-sdk # ^ pydantic-ai-slim[mcp] -> fastmcp-slim caps mcp<2.0 across its whole # published range as of 2026-08; mcp 2.0 is a breaking rewrite (decorator-based # handler registration on mcp.server.lowlevel.Server removed in favor of From 3fc6fde90ec7e0c679c98df4e7505d15c138aaa0 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 14 Sep 2026 08:32:44 +0000 Subject: [PATCH 2/5] Migrate MCP server to python-sdk 2.x alongside the mcp>=2.2.0 bump python-sdk 2.0 removed decorator-based handler registration on mcp.server.Server in favour of on_*= constructor kwargs whose handlers take (ctx, params) and return typed result models. opencontractserver/mcp/server.py still used the 1.x decorators, so the dependabot bump alone would have broken both MCP entrypoints at import time (see the review on #2334). - server.py: build the global and corpus-scoped servers through shared adapters (_build_on_call_tool, _build_on_list_tools, _build_on_list_resource_templates, _on_read_resource). Argument validation against inputSchema and isError wrapping of dispatcher exceptions -- which the 1.x call_tool decorator did implicitly -- are now explicit. Tool and template catalogues move to get_tool_definitions() / get_resource_template_definitions(). SDK types use their 2.x snake_case fields; resource URIs are plain str. - resources/read payloads are stamped application/json (MCP_RESOURCE_MIME_TYPE) to match the advertised templates; caller-side read failures return JSON-RPC INVALID_PARAMS with the message. - tests: replace the removed server.request_handlers seam with mcp.client.Client in-memory sessions and add MCPSdkClientRoundTripTest, which drives both servers through the SDK runtime (list/call/read, validation errors, permission propagation) plus a real stateless Streamable HTTP JSON-RPC request through StreamableHTTPSessionManager. - requirements/base.txt + .pre-commit-config.yaml: mcp>=2.2.0,<3 with the stale "fastmcp-slim caps mcp<2" note replaced (fastmcp-slim resolves against 2.x; pip check is clean). - docs/mcp/README.md: SDK integration section; changelog fragment added. --- .pre-commit-config.yaml | 11 +- changelog.d/2334-mcp-sdk-2.changed.md | 1 + docs/mcp/README.md | 26 + opencontractserver/constants/mcp.py | 5 + opencontractserver/mcp/server.py | 734 ++++++++++++++--------- opencontractserver/mcp/tests/test_mcp.py | 449 ++++++++++++-- requirements/base.txt | 15 +- 7 files changed, 895 insertions(+), 346 deletions(-) create mode 100644 changelog.d/2334-mcp-sdk-2.changed.md diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 9cf954b46..e2eecca51 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -156,11 +156,12 @@ repos: - spacy - tokenizers>=0.21,<0.23 - posthog==7.12.0 - # Ceiling matches requirements/base.txt's runtime pin. mcp>=1.0.0 - # (no ceiling) let this hook resolve mcp 2.x, whose SDK renamed - # fields/methods that opencontractserver/mcp/server.py (1.x API) - # doesn't have -- turning main red with no commit touching mcp. - - mcp>=1.28.1,<2 + # Range matches requirements/base.txt's runtime pin exactly. + # opencontractserver/mcp/server.py targets the python-sdk 2.x + # API (on_*= handler kwargs, snake_case type fields); a hook env + # resolving a different major than the runtime turns main red + # with no commit touching mcp. Bump both pins together. + - mcp>=2.2.0,<3 - argon2-cffi==25.1.0 - cryptography==46.0.7 - pyjwt==2.12.1 diff --git a/changelog.d/2334-mcp-sdk-2.changed.md b/changelog.d/2334-mcp-sdk-2.changed.md new file mode 100644 index 000000000..308f62cb1 --- /dev/null +++ b/changelog.d/2334-mcp-sdk-2.changed.md @@ -0,0 +1 @@ +- **Migrated the MCP server to `mcp` (python-sdk) 2.x** (`requirements/base.txt`: `mcp>=2.2.0,<3`; #2334). python-sdk 2.0 removed the decorator-based handler registration on `mcp.server.Server` (`@server.list_tools()`, `server.call_tool()(...)`) in favour of `on_*=` constructor kwargs whose handlers take `(ctx, params)` and return typed result models. `opencontractserver/mcp/server.py` now builds both the global server (`create_mcp_server`) and the per-corpus scoped server (`create_scoped_mcp_server`) through a shared set of adapters (`_build_on_call_tool`, `_build_on_list_tools`, `_build_on_list_resource_templates`, `_on_read_resource`) so argument validation, error wrapping and resource serialisation live in one place. Behaviour the 1.x decorators used to provide is preserved explicitly: tool arguments are validated against the advertised `inputSchema` (jsonschema; mistyped arguments return an `isError` result, `"Input validation error: ..."`), and any exception escaping a dispatcher (unknown tool, rate limit) still becomes an `isError` result rather than a transport error. The global tool/template catalogues moved out of the factory closure into `get_tool_definitions()` / `get_resource_template_definitions()` (mirroring the scoped equivalents). Wire-level changes: `resources/read` payloads are now stamped `application/json` (`MCP_RESOURCE_MIME_TYPE`, matching the advertised templates; 1.x's deprecated str-return path stamped `text/plain`), and a resource read that fails on the caller's side (unrecognised URI, invisible/missing corpus or document) returns JSON-RPC `INVALID_PARAMS` with the message instead of a bare internal error. SDK types now use their 2.x snake_case fields (`input_schema`, `uri_template`, `mime_type`; URIs are plain `str`). Tests: the SDK-level tests drive both servers through `mcp.client.Client` in-memory sessions (`MCPSdkClientRoundTripTest`) and a real stateless Streamable HTTP JSON-RPC request through `StreamableHTTPSessionManager`, replacing the removed `server.request_handlers[...]` seam. diff --git a/docs/mcp/README.md b/docs/mcp/README.md index 1ea9af3b3..4ff5c7d67 100644 --- a/docs/mcp/README.md +++ b/docs/mcp/README.md @@ -256,6 +256,32 @@ python -m opencontractserver.mcp.server - `config/asgi.py` - HTTP routing (`/mcp/*` and `/sse/*` → MCP app) - `compose/production/traefik/traefik.yml` - Production routing (Traefik) +### SDK integration + +Both servers are `mcp.server.Server` instances from the official +[python-sdk](https://github.com/modelcontextprotocol/python-sdk) **2.x** +(`requirements/base.txt` pins `mcp>=2.2.0,<3`). Request handlers are passed as +`on_*=` constructor kwargs and return typed result models; the 1.x decorator +API no longer exists. The adapters in `server.py` (section `MCP SDK HANDLER +ADAPTERS`: `_build_on_call_tool`, `_build_on_list_tools`, +`_build_on_list_resource_templates`, `_on_read_resource`) are the only code +that touches that SDK surface — `create_mcp_server` and +`create_scoped_mcp_server` compose them over the transport-agnostic +dispatchers (`call_tool_handler`, `read_resource_handler`, the scoped +`call_tool` closure), so a future SDK change is a one-place edit. + +Contract preserved from 1.x and pinned by `MCPSdkClientRoundTripTest` +(`opencontractserver/mcp/tests/test_mcp.py`): + +- Tool arguments are validated against the advertised `inputSchema`; a + mismatch returns an `isError` result (`Input validation error: ...`). +- Exceptions escaping a dispatcher (unknown tool, rate limit) become `isError` + results, never transport errors. Django `PermissionDenied` / + `ValidationError` / `DoesNotExist` are structured `{"error": ...}` payloads. +- `resources/read` returns `application/json` text contents; a URI the caller + cannot resolve (unknown pattern, invisible corpus) is a JSON-RPC + `INVALID_PARAMS` error carrying the message. + --- ## Authentication diff --git a/opencontractserver/constants/mcp.py b/opencontractserver/constants/mcp.py index 7ea76ea39..c78c72044 100644 --- a/opencontractserver/constants/mcp.py +++ b/opencontractserver/constants/mcp.py @@ -22,3 +22,8 @@ # over-fetching a small ``limit`` could be consumed entirely by duplicates. MCP_SEARCH_CANDIDATE_MULTIPLIER: int = 3 # candidate fetch = limit * this MCP_SEARCH_CANDIDATE_MAX: int = 150 # absolute cap on candidate fetch per half + +# MIME type stamped on every ``resources/read`` payload. All MCP resources +# (corpus, document, annotation, thread) serialise to JSON, matching the +# ``mime_type`` advertised on their ``ResourceTemplate`` entries. +MCP_RESOURCE_MIME_TYPE: str = "application/json" diff --git a/opencontractserver/mcp/server.py b/opencontractserver/mcp/server.py index d6c1954da..1257c367b 100644 --- a/opencontractserver/mcp/server.py +++ b/opencontractserver/mcp/server.py @@ -28,6 +28,7 @@ if TYPE_CHECKING: from opencontractserver.users.types import UserOrAnonymous +import jsonschema from asgiref.sync import sync_to_async from django.conf import settings from django.core.exceptions import ( @@ -36,11 +37,27 @@ ValidationError, ) from mcp.server import Server +from mcp.server.lowlevel.server import ServerRequestContext from mcp.server.sse import SseServerTransport from mcp.server.stdio import stdio_server from mcp.server.streamable_http_manager import StreamableHTTPSessionManager -from mcp.types import Resource, ResourceTemplate, TextContent, Tool -from pydantic import AnyUrl +from mcp.shared.exceptions import MCPError +from mcp.types import ( + INVALID_PARAMS, + CallToolRequestParams, + CallToolResult, + ListResourcesResult, + ListResourceTemplatesResult, + ListToolsResult, + PaginatedRequestParams, + ReadResourceRequestParams, + ReadResourceResult, + Resource, + ResourceTemplate, + TextContent, + TextResourceContents, + Tool, +) from starlette.applications import Starlette from starlette.requests import Request from starlette.responses import Response @@ -53,7 +70,10 @@ from config.jwt_utils import get_user_from_jwt_token from config.ratelimit.decorators import MCPRateLimitError, check_mcp_rate_limit from config.ratelimit.keys import get_client_ip_from_scope -from opencontractserver.constants.mcp import MAX_THREAD_MESSAGE_LENGTH +from opencontractserver.constants.mcp import ( + MAX_THREAD_MESSAGE_LENGTH, + MCP_RESOURCE_MIME_TYPE, +) from .resources import ( get_annotation_resource, @@ -489,7 +509,7 @@ async def read_resource_handler(uri: str) -> str: This is the handler function for MCP resource reads. Exposed at module level for testability. """ - # Convert AnyUrl to string if needed (MCP library uses pydantic AnyUrl) + # Defensive: callers hand us a ``str`` (mcp 2.x) but tolerate URL objects. uri_str = str(uri) resource_type = "unknown" @@ -751,280 +771,410 @@ async def call_tool_handler(name: str, arguments: dict) -> list[TextContent]: raise -def create_mcp_server() -> Server: - """Create and configure the MCP server instance.""" - mcp_server = Server("opencontracts") - - @mcp_server.list_resources() - async def list_resources() -> list[Resource]: - """List available resources (none - use templates instead).""" - # All resources require parameters, so we return empty list - # Use list_resource_templates for URI patterns - return [] +# ============================================================================= +# MCP SDK HANDLER ADAPTERS +# ============================================================================= +# python-sdk 2.x registers request handlers as ``on_*=`` constructor kwargs on +# ``mcp.server.Server``. Each handler receives ``(ctx, params)`` and returns a +# typed result model. The adapters below bridge that shape to this module's +# transport-agnostic dispatchers (``call_tool_handler`` / ``read_resource_handler`` +# and the scoped ``call_tool`` closure) so the global and corpus-scoped servers +# share ONE implementation of argument validation, error wrapping and resource +# serialisation. + +# ``(tool_name, arguments) -> content blocks`` — the dispatcher contract shared +# by ``call_tool_handler`` and the scoped ``call_tool`` closure. +ToolDispatcher = Callable[[str, dict], Awaitable[list[TextContent]]] + + +def _tool_error_result(message: str) -> CallToolResult: + """Build the ``isError`` result shape MCP clients expect for tool failures.""" + return CallToolResult( + content=[TextContent(type="text", text=message)], is_error=True + ) - @mcp_server.list_resource_templates() - async def list_resource_templates() -> list[ResourceTemplate]: - """List available resource URI templates.""" - return [ - ResourceTemplate( - uriTemplate="corpus://{corpus_slug}", - name="Public Corpus", - description="Access public corpus metadata and contents", - mimeType="application/json", - ), - ResourceTemplate( - uriTemplate="document://{corpus_slug}/{document_slug}", - name="Public Document", - description="Access public document with extracted text", - mimeType="application/json", - ), - ResourceTemplate( - uriTemplate="annotation://{corpus_slug}/{document_slug}/{annotation_id}", - name="Document Annotation", - description="Access specific annotation on a document", - mimeType="application/json", - ), - ResourceTemplate( - uriTemplate="thread://{corpus_slug}/threads/{thread_id}", - name="Discussion Thread", - description="Access public discussion thread with messages", - mimeType="application/json", - ), + +def _build_on_call_tool( + dispatch: ToolDispatcher, tools: list[Tool] +) -> Callable[[ServerRequestContext, CallToolRequestParams], Awaitable[CallToolResult]]: + """Wrap a tool dispatcher as an ``on_call_tool`` handler. + + Preserves the two behaviours the 1.x ``@server.call_tool()`` decorator + used to provide for us: + + * Arguments are validated against the advertised ``inputSchema`` before + dispatch, so a mistyped argument (``limit="ten"``) comes back as an + ``isError`` result naming the problem instead of a ``TypeError`` deep + inside a handler. + * Any exception escaping the dispatcher (unknown tool, rate limit, an + unexpected handler failure) becomes an ``isError`` result rather than a + JSON-RPC transport error, so LLM clients can read and react to it. + + ``PermissionDenied`` / ``ValidationError`` / ``ObjectDoesNotExist`` never + reach the ``except`` here — both dispatchers already convert those into a + structured ``{"error": ...}`` payload (``_record_and_return_tool_error``). + """ + tools_by_name = {tool.name: tool for tool in tools} + + async def on_call_tool( + ctx: ServerRequestContext, params: CallToolRequestParams + ) -> CallToolResult: + arguments = params.arguments or {} + tool = tools_by_name.get(params.name) + if tool is not None: + try: + jsonschema.validate(instance=arguments, schema=tool.input_schema) + except jsonschema.ValidationError as e: + return _tool_error_result(f"Input validation error: {e.message}") + try: + content = await dispatch(params.name, arguments) + except Exception as e: + return _tool_error_result(str(e)) + return CallToolResult(content=list(content)) + + return on_call_tool + + +def _build_on_list_tools( + tools: list[Tool], +) -> Callable[ + [ServerRequestContext, PaginatedRequestParams | None], Awaitable[ListToolsResult] +]: + """Wrap a static tool catalogue as an ``on_list_tools`` handler.""" + + async def on_list_tools( + ctx: ServerRequestContext, params: PaginatedRequestParams | None + ) -> ListToolsResult: + return ListToolsResult(tools=tools) + + return on_list_tools + + +def _build_on_list_resource_templates( + templates: list[ResourceTemplate], +) -> Callable[ + [ServerRequestContext, PaginatedRequestParams | None], + Awaitable[ListResourceTemplatesResult], +]: + """Wrap a static template catalogue as an ``on_list_resource_templates`` handler.""" + + async def on_list_resource_templates( + ctx: ServerRequestContext, params: PaginatedRequestParams | None + ) -> ListResourceTemplatesResult: + return ListResourceTemplatesResult(resource_templates=templates) + + return on_list_resource_templates + + +async def _on_read_resource( + ctx: ServerRequestContext, params: ReadResourceRequestParams +) -> ReadResourceResult: + """``on_read_resource`` handler shared by the global and scoped servers. + + ``read_resource_handler`` returns the JSON payload as a string; this wraps + it in the ``TextResourceContents`` envelope. Caller-side failures (a URI + that matches no pattern, a corpus/document the caller cannot see, a + missing object) are surfaced as ``INVALID_PARAMS`` JSON-RPC errors carrying + the human-readable message — anything else is left to the SDK, which + reports a generic internal error without leaking the exception text. + """ + try: + text = await read_resource_handler(params.uri) + except (ValueError, PermissionDenied, ValidationError, ObjectDoesNotExist) as e: + raise MCPError(INVALID_PARAMS, str(e)) from e + return ReadResourceResult( + contents=[ + TextResourceContents( + uri=params.uri, text=text, mime_type=MCP_RESOURCE_MIME_TYPE + ) ] + ) - # Register the module-level handler with the MCP server - mcp_server.read_resource()(read_resource_handler) - @mcp_server.list_tools() - async def list_tools() -> list[Tool]: - """List available tools.""" - return [ - Tool( - name="list_public_corpuses", - description=( - "List corpuses visible to the caller. Anonymous callers " - "see only public, published corpuses. Authenticated " - "callers additionally see private corpuses they own or " - "have been granted read access to. (Name retained for " - "backwards compatibility with existing MCP clients.)" - ), - inputSchema={ - "type": "object", - "properties": { - "limit": { - "type": "integer", - "default": 20, - "description": "Max results (1-100)", - }, - "offset": { - "type": "integer", - "default": 0, - "description": "Pagination offset", - }, - "search": { - "type": "string", - "default": "", - "description": "Search filter", - }, +def get_resource_template_definitions() -> list[ResourceTemplate]: + """Resource URI templates advertised by the global (non-scoped) server.""" + return [ + ResourceTemplate( + uri_template="corpus://{corpus_slug}", + name="Public Corpus", + description="Access public corpus metadata and contents", + mime_type="application/json", + ), + ResourceTemplate( + uri_template="document://{corpus_slug}/{document_slug}", + name="Public Document", + description="Access public document with extracted text", + mime_type="application/json", + ), + ResourceTemplate( + uri_template="annotation://{corpus_slug}/{document_slug}/{annotation_id}", + name="Document Annotation", + description="Access specific annotation on a document", + mime_type="application/json", + ), + ResourceTemplate( + uri_template="thread://{corpus_slug}/threads/{thread_id}", + name="Discussion Thread", + description="Access public discussion thread with messages", + mime_type="application/json", + ), + ] + + +def get_tool_definitions() -> list[Tool]: + """Tool catalogue advertised by the global (non-scoped) MCP server. + + Keep in lockstep with ``TOOL_HANDLERS`` — ``MCPNonScopedListToolsTest`` + pins the two registries against each other. + """ + return [ + Tool( + name="list_public_corpuses", + description=( + "List corpuses visible to the caller. Anonymous callers " + "see only public, published corpuses. Authenticated " + "callers additionally see private corpuses they own or " + "have been granted read access to. (Name retained for " + "backwards compatibility with existing MCP clients.)" + ), + input_schema={ + "type": "object", + "properties": { + "limit": { + "type": "integer", + "default": 20, + "description": "Max results (1-100)", + }, + "offset": { + "type": "integer", + "default": 0, + "description": "Pagination offset", + }, + "search": { + "type": "string", + "default": "", + "description": "Search filter", }, }, - ), - Tool( - name="list_documents", - description="List documents in a corpus", - inputSchema={ - "type": "object", - "properties": { - "corpus_slug": { - "type": "string", - "description": "Corpus identifier", - }, - "limit": {"type": "integer", "default": 50}, - "offset": {"type": "integer", "default": 0}, - "search": {"type": "string", "default": ""}, + }, + ), + Tool( + name="list_documents", + description="List documents in a corpus", + input_schema={ + "type": "object", + "properties": { + "corpus_slug": { + "type": "string", + "description": "Corpus identifier", }, - "required": ["corpus_slug"], + "limit": {"type": "integer", "default": 50}, + "offset": {"type": "integer", "default": 0}, + "search": {"type": "string", "default": ""}, }, - ), - Tool( - name="get_document_text", - description="Get extracted document text in bounded slices (char_offset/max_chars)", - inputSchema={ - "type": "object", - "properties": { - "corpus_slug": { - "type": "string", - "description": "Corpus identifier", - }, - "document_slug": { - "type": "string", - "description": "Document identifier", - }, - "char_offset": { - "type": "integer", - "default": 0, - "description": "Start offset into the extracted text", - }, - "max_chars": { - "type": "integer", - "description": "Window size; use next_offset to paginate", - }, + "required": ["corpus_slug"], + }, + ), + Tool( + name="get_document_text", + description="Get extracted document text in bounded slices (char_offset/max_chars)", + input_schema={ + "type": "object", + "properties": { + "corpus_slug": { + "type": "string", + "description": "Corpus identifier", + }, + "document_slug": { + "type": "string", + "description": "Document identifier", + }, + "char_offset": { + "type": "integer", + "default": 0, + "description": "Start offset into the extracted text", + }, + "max_chars": { + "type": "integer", + "description": "Window size; use next_offset to paginate", }, - "required": ["corpus_slug", "document_slug"], }, + "required": ["corpus_slug", "document_slug"], + }, + ), + Tool( + name="list_annotations", + description=( + "List/search a document's annotations (filter by page, " + "label_text, text_contains, structural)" ), - Tool( - name="list_annotations", - description=( - "List/search a document's annotations (filter by page, " - "label_text, text_contains, structural)" - ), - inputSchema={ - "type": "object", - "properties": { - "corpus_slug": {"type": "string"}, - "document_slug": {"type": "string"}, - "page": { - "type": "integer", - "description": "Filter to page number", - }, - "label_text": { - "type": "string", - "description": "Filter by exact label text", - }, - "text_contains": { - "type": "string", - "description": "Filter annotations whose text contains this substring", - }, - "structural": { - "type": "boolean", - "description": "Filter: omit=all, true=structural only, false=human/analysis only", - }, - "limit": {"type": "integer", "default": 100}, - "offset": {"type": "integer", "default": 0}, + input_schema={ + "type": "object", + "properties": { + "corpus_slug": {"type": "string"}, + "document_slug": {"type": "string"}, + "page": { + "type": "integer", + "description": "Filter to page number", }, - "required": ["corpus_slug", "document_slug"], + "label_text": { + "type": "string", + "description": "Filter by exact label text", + }, + "text_contains": { + "type": "string", + "description": "Filter annotations whose text contains this substring", + }, + "structural": { + "type": "boolean", + "description": "Filter: omit=all, true=structural only, false=human/analysis only", + }, + "limit": {"type": "integer", "default": 100}, + "offset": {"type": "integer", "default": 0}, }, + "required": ["corpus_slug", "document_slug"], + }, + ), + Tool( + name="list_relationships", + description=( + "List labeled source→target relationships in a corpus " + "(or a single document) for explicit graph navigation" ), - Tool( - name="list_relationships", - description=( - "List labeled source→target relationships in a corpus " - "(or a single document) for explicit graph navigation" - ), - inputSchema={ - "type": "object", - "properties": { - "corpus_slug": {"type": "string"}, - "document_slug": { - "type": "string", - "description": "Optional document filter (corpus-wide when omitted)", - }, - "structural": { - "type": "boolean", - "description": "Filter: omit=all, true=structural only, false=human/analysis only", - }, - "label_text": { - "type": "string", - "description": "Filter by exact relationship label", - }, - "limit": {"type": "integer", "default": 50}, - "offset": {"type": "integer", "default": 0}, + input_schema={ + "type": "object", + "properties": { + "corpus_slug": {"type": "string"}, + "document_slug": { + "type": "string", + "description": "Optional document filter (corpus-wide when omitted)", + }, + "structural": { + "type": "boolean", + "description": "Filter: omit=all, true=structural only, false=human/analysis only", + }, + "label_text": { + "type": "string", + "description": "Filter by exact relationship label", }, - "required": ["corpus_slug"], + "limit": {"type": "integer", "default": 50}, + "offset": {"type": "integer", "default": 0}, }, + "required": ["corpus_slug"], + }, + ), + Tool( + name="search_corpus", + description=( + "Search a corpus. Returns a ranked feed of passage and " + "block hits (each tagged 'type'); semantic with a text fallback" ), - Tool( - name="search_corpus", - description=( - "Search a corpus. Returns a ranked feed of passage and " - "block hits (each tagged 'type'); semantic with a text fallback" - ), - inputSchema={ - "type": "object", - "properties": { - "corpus_slug": {"type": "string"}, - "query": {"type": "string", "description": "Search query"}, - "limit": { - "type": "integer", - "default": 10, - "description": "Max hits (1-50)", - }, - "granularity": { - "type": "string", - "enum": ["passage", "block", "both"], - "default": "both", - "description": ( - "passage = annotation hits; block = aggregated " - "subtree-group hits; both = merged feed" - ), - }, - "structural": { - "type": "boolean", - "description": "Filter passages: omit=all, true=structural only, false=human/analysis only", - }, + input_schema={ + "type": "object", + "properties": { + "corpus_slug": {"type": "string"}, + "query": {"type": "string", "description": "Search query"}, + "limit": { + "type": "integer", + "default": 10, + "description": "Max hits (1-50)", + }, + "granularity": { + "type": "string", + "enum": ["passage", "block", "both"], + "default": "both", + "description": ( + "passage = annotation hits; block = aggregated " + "subtree-group hits; both = merged feed" + ), + }, + "structural": { + "type": "boolean", + "description": "Filter passages: omit=all, true=structural only, false=human/analysis only", }, - "required": ["corpus_slug", "query"], }, - ), - Tool( - name="list_threads", - description="List discussion threads in a corpus", - inputSchema={ - "type": "object", - "properties": { - "corpus_slug": {"type": "string"}, - "document_slug": { - "type": "string", - "description": "Optional document filter", - }, - "limit": {"type": "integer", "default": 20}, - "offset": {"type": "integer", "default": 0}, + "required": ["corpus_slug", "query"], + }, + ), + Tool( + name="list_threads", + description="List discussion threads in a corpus", + input_schema={ + "type": "object", + "properties": { + "corpus_slug": {"type": "string"}, + "document_slug": { + "type": "string", + "description": "Optional document filter", }, - "required": ["corpus_slug"], + "limit": {"type": "integer", "default": 20}, + "offset": {"type": "integer", "default": 0}, }, - ), - Tool( - name="get_thread_messages", - description="Get messages in a thread", - inputSchema={ - "type": "object", - "properties": { - "corpus_slug": {"type": "string"}, - "thread_id": {"type": "integer"}, - "flatten": { - "type": "boolean", - "default": False, - "description": "Return flat list", - }, + "required": ["corpus_slug"], + }, + ), + Tool( + name="get_thread_messages", + description="Get messages in a thread", + input_schema={ + "type": "object", + "properties": { + "corpus_slug": {"type": "string"}, + "thread_id": {"type": "integer"}, + "flatten": { + "type": "boolean", + "default": False, + "description": "Return flat list", }, - "required": ["corpus_slug", "thread_id"], }, - ), - Tool( - name="create_thread_message", - description="Create a new message in a thread (requires authenticated MCP session)", - inputSchema={ - "type": "object", - "properties": { - "corpus_slug": {"type": "string"}, - "thread_id": {"type": "integer"}, - "content": { - "type": "string", - "minLength": 1, - "maxLength": MAX_THREAD_MESSAGE_LENGTH, - }, - "parent_message_id": {"type": "integer"}, + "required": ["corpus_slug", "thread_id"], + }, + ), + Tool( + name="create_thread_message", + description="Create a new message in a thread (requires authenticated MCP session)", + input_schema={ + "type": "object", + "properties": { + "corpus_slug": {"type": "string"}, + "thread_id": {"type": "integer"}, + "content": { + "type": "string", + "minLength": 1, + "maxLength": MAX_THREAD_MESSAGE_LENGTH, }, - "required": ["corpus_slug", "thread_id", "content"], + "parent_message_id": {"type": "integer"}, }, - ), - ] + "required": ["corpus_slug", "thread_id", "content"], + }, + ), + ] + - # Register the module-level handler with the MCP server - mcp_server.call_tool()(call_tool_handler) +async def _on_list_no_resources( + ctx: ServerRequestContext, params: PaginatedRequestParams | None +) -> ListResourcesResult: + """The global server exposes no concrete resources — only URI templates. + + Every resource requires parameters (a corpus slug at minimum), so clients + discover them via ``resources/templates/list`` instead. + """ + return ListResourcesResult(resources=[]) - return mcp_server + +def create_mcp_server() -> Server: + """Create and configure the global MCP server instance.""" + tools = get_tool_definitions() + return Server( + "opencontracts", + on_list_resources=_on_list_no_resources, + on_list_resource_templates=_build_on_list_resource_templates( + get_resource_template_definitions() + ), + on_read_resource=_on_read_resource, + on_list_tools=_build_on_list_tools(tools), + on_call_tool=_build_on_call_tool(call_tool_handler, tools), + ) # Create the global MCP server instance @@ -1054,7 +1204,7 @@ def get_scoped_tool_definitions(corpus_slug: str) -> list[Tool]: Tool( name="get_corpus_info", description=f"Get detailed information about the '{corpus_slug}' corpus", - inputSchema={ + input_schema={ "type": "object", "properties": {}, }, @@ -1062,7 +1212,7 @@ def get_scoped_tool_definitions(corpus_slug: str) -> list[Tool]: Tool( name="list_documents", description=f"List documents in the '{corpus_slug}' corpus", - inputSchema={ + input_schema={ "type": "object", "properties": { "limit": {"type": "integer", "default": 50}, @@ -1074,7 +1224,7 @@ def get_scoped_tool_definitions(corpus_slug: str) -> list[Tool]: Tool( name="get_document_text", description="Get extracted document text in bounded slices (char_offset/max_chars)", - inputSchema={ + input_schema={ "type": "object", "properties": { "document_slug": { @@ -1100,7 +1250,7 @@ def get_scoped_tool_definitions(corpus_slug: str) -> list[Tool]: "List/search a document's annotations (filter by page, " "label_text, text_contains, structural)" ), - inputSchema={ + input_schema={ "type": "object", "properties": { "document_slug": {"type": "string"}, @@ -1132,7 +1282,7 @@ def get_scoped_tool_definitions(corpus_slug: str) -> list[Tool]: f"List labeled source→target relationships in the " f"'{corpus_slug}' corpus (or a single document)" ), - inputSchema={ + input_schema={ "type": "object", "properties": { "document_slug": { @@ -1162,7 +1312,7 @@ def get_scoped_tool_definitions(corpus_slug: str) -> list[Tool]: f"passage and block hits (each tagged 'type'); semantic with a " f"text fallback" ), - inputSchema={ + input_schema={ "type": "object", "properties": { "query": {"type": "string", "description": "Search query"}, @@ -1191,7 +1341,7 @@ def get_scoped_tool_definitions(corpus_slug: str) -> list[Tool]: Tool( name="list_threads", description=f"List discussion threads in the '{corpus_slug}' corpus", - inputSchema={ + input_schema={ "type": "object", "properties": { "document_slug": { @@ -1206,7 +1356,7 @@ def get_scoped_tool_definitions(corpus_slug: str) -> list[Tool]: Tool( name="get_thread_messages", description="Get messages in a thread", - inputSchema={ + input_schema={ "type": "object", "properties": { "thread_id": {"type": "integer"}, @@ -1227,7 +1377,7 @@ def get_scoped_tool_definitions(corpus_slug: str) -> list[Tool]: "Note: corpus_slug is injected from the endpoint URL and must " "not be supplied by the client." ), - inputSchema={ + input_schema={ "type": "object", "properties": { "thread_id": {"type": "integer"}, @@ -1281,10 +1431,10 @@ def get_scoped_resource_definitions( # Add corpus resource resources.append( Resource( - uri=AnyUrl(f"corpus://{corpus_slug}"), + uri=f"corpus://{corpus_slug}", name="Corpus", description=f"Access the '{corpus_slug}' corpus metadata and contents", - mimeType="application/json", + mime_type="application/json", ) ) @@ -1295,10 +1445,10 @@ def get_scoped_resource_definitions( for doc in documents: resources.append( Resource( - uri=AnyUrl(f"document://{corpus_slug}/{doc.slug}"), + uri=f"document://{corpus_slug}/{doc.slug}", name=f"Document: {doc.title or doc.slug}", description=doc.description[:100] if doc.description else "Document", - mimeType="application/json", + mime_type="application/json", ) ) @@ -1314,14 +1464,14 @@ def get_scoped_resource_definitions( for thread in threads: resources.append( Resource( - uri=AnyUrl(f"thread://{corpus_slug}/threads/{thread.id}"), + uri=f"thread://{corpus_slug}/threads/{thread.id}", name=f"Thread: {thread.title or f'Thread {thread.id}'}", description=( thread.description[:100] if thread.description else "Discussion thread" ), - mimeType="application/json", + mime_type="application/json", ) ) @@ -1342,22 +1492,22 @@ def get_scoped_resource_template_definitions( """ return [ ResourceTemplate( - uriTemplate=f"document://{corpus_slug}/{{document_slug}}", + uri_template=f"document://{corpus_slug}/{{document_slug}}", name="Document", description="Access document with extracted text", - mimeType="application/json", + mime_type="application/json", ), ResourceTemplate( - uriTemplate=f"annotation://{corpus_slug}/{{document_slug}}/{{annotation_id}}", + uri_template=f"annotation://{corpus_slug}/{{document_slug}}/{{annotation_id}}", name="Annotation", description="Access specific annotation on a document", - mimeType="application/json", + mime_type="application/json", ), ResourceTemplate( - uriTemplate=f"thread://{corpus_slug}/threads/{{thread_id}}", + uri_template=f"thread://{corpus_slug}/threads/{{thread_id}}", name="Discussion Thread", description="Access discussion thread with messages", - mimeType="application/json", + mime_type="application/json", ), ] @@ -1376,10 +1526,8 @@ def create_scoped_mcp_server(corpus_slug: str) -> Server: Returns: Configured MCP Server instance scoped to the corpus """ - scoped_server = Server(f"opencontracts-corpus-{corpus_slug}") - - # Get scoped tool handlers scoped_handlers = get_scoped_tool_handlers(corpus_slug) + scoped_tools = get_scoped_tool_definitions(corpus_slug) def _validate_corpus_sync(user: UserOrAnonymous | None = None) -> bool: """Synchronously validate the scoped corpus is still visible to the caller.""" @@ -1394,28 +1542,16 @@ def _validate_corpus_sync(user: UserOrAnonymous | None = None) -> bool: .exists() ) - @scoped_server.list_resources() - async def list_resources() -> list[Resource]: - """List available concrete resources for this scoped corpus.""" + async def on_list_resources( + ctx: ServerRequestContext, params: PaginatedRequestParams | None + ) -> ListResourcesResult: + """List concrete resources visible to the caller in this scoped corpus.""" # Use sync_to_async since this queries the database - return await sync_to_async(get_scoped_resource_definitions)( + resources = await sync_to_async(get_scoped_resource_definitions)( corpus_slug, user=_mcp_user.get() ) + return ListResourcesResult(resources=resources) - @scoped_server.list_resource_templates() - async def list_resource_templates() -> list[ResourceTemplate]: - """List available resource templates for this scoped corpus.""" - return get_scoped_resource_template_definitions(corpus_slug) - - # Resource handler - reuse the global handler (it validates corpus access) - scoped_server.read_resource()(read_resource_handler) - - @scoped_server.list_tools() - async def list_tools() -> list[Tool]: - """List available tools for this scoped corpus.""" - return get_scoped_tool_definitions(corpus_slug) - - @scoped_server.call_tool() async def call_tool(name: str, arguments: dict) -> list[TextContent]: """ Execute scoped tool and return results. @@ -1507,7 +1643,17 @@ async def call_tool(name: str, arguments: dict) -> list[TextContent]: ) raise - return scoped_server + return Server( + f"opencontracts-corpus-{corpus_slug}", + on_list_resources=on_list_resources, + on_list_resource_templates=_build_on_list_resource_templates( + get_scoped_resource_template_definitions(corpus_slug) + ), + # Resource reads reuse the global handler (it validates corpus access). + on_read_resource=_on_read_resource, + on_list_tools=_build_on_list_tools(scoped_tools), + on_call_tool=_build_on_call_tool(call_tool, scoped_tools), + ) # ============================================================================= diff --git a/opencontractserver/mcp/tests/test_mcp.py b/opencontractserver/mcp/tests/test_mcp.py index 5d629a145..aa6b5c794 100644 --- a/opencontractserver/mcp/tests/test_mcp.py +++ b/opencontractserver/mcp/tests/test_mcp.py @@ -3251,28 +3251,28 @@ def test_get_scoped_tool_definitions(self): # list_documents should not require corpus_slug list_docs_tool = next(t for t in tools if t.name == "list_documents") - required = list_docs_tool.inputSchema.get("required", []) + required = list_docs_tool.input_schema.get("required", []) self.assertNotIn("corpus_slug", required) # search_corpus should only require query search_tool = next(t for t in tools if t.name == "search_corpus") - self.assertEqual(search_tool.inputSchema.get("required", []), ["query"]) + self.assertEqual(search_tool.input_schema.get("required", []), ["query"]) # list_relationships must be advertised by the scoped endpoint and, # since corpus_slug is bound from the URL, expose no required params. self.assertIn("list_relationships", tool_names) rels_tool = next(t for t in tools if t.name == "list_relationships") - self.assertEqual(rels_tool.inputSchema.get("required", []), []) + self.assertEqual(rels_tool.input_schema.get("required", []), []) # The scoped create_thread_message variant must drop ``corpus_slug`` # from required (auto-injected from the URL) and expose the content # length bounds in its JSON Schema for client-side validation. create_tool = next(t for t in tools if t.name == "create_thread_message") - create_required = create_tool.inputSchema.get("required", []) + create_required = create_tool.input_schema.get("required", []) self.assertNotIn("corpus_slug", create_required) self.assertIn("thread_id", create_required) self.assertIn("content", create_required) - content_schema = create_tool.inputSchema["properties"]["content"] + content_schema = create_tool.input_schema["properties"]["content"] self.assertEqual(content_schema.get("minLength"), 1) self.assertGreater(content_schema.get("maxLength", 0), 0) @@ -4761,7 +4761,7 @@ def test_get_scoped_tool_definitions_has_no_corpus_slug_required(self): self.assertIsNotNone(list_docs_tool) # Verify corpus_slug is not required - required_params = list_docs_tool.inputSchema.get("required", []) + required_params = list_docs_tool.input_schema.get("required", []) self.assertNotIn("corpus_slug", required_params) def test_get_scoped_tool_definitions_includes_get_corpus_info(self): @@ -4775,7 +4775,7 @@ def test_get_scoped_tool_definitions_includes_get_corpus_info(self): self.assertIsNotNone(corpus_info_tool) # get_corpus_info should have no required params - required_params = corpus_info_tool.inputSchema.get("required", []) + required_params = corpus_info_tool.input_schema.get("required", []) self.assertEqual(required_params, []) @@ -6902,8 +6902,11 @@ async def run_test(): self.assertIn("empty", payload["error"].lower()) def test_scoped_validation_error_returns_error_payload(self): - """Same contract via the scoped corpus closure.""" - from mcp.types import CallToolRequest, CallToolRequestParams + """Same contract via the scoped corpus server, driven through a real + in-memory MCP client so the SDK's ``on_call_tool`` wiring (argument + validation, result envelope) is exercised rather than bypassed. + """ + from mcp.client import Client from opencontractserver.mcp.server import ( _mcp_user, @@ -6911,30 +6914,28 @@ def test_scoped_validation_error_returns_error_payload(self): ) server = create_scoped_mcp_server(self.corpus.slug) - call_tool = server.request_handlers[CallToolRequest] async def run_test(): + # Set BEFORE entering the client: the SDK spawns the server task + # inside ``__aenter__`` and that task inherits this context. token = _mcp_user.set(self.owner) try: - request = CallToolRequest( - method="tools/call", - params=CallToolRequestParams( - name="create_thread_message", - arguments={ + async with Client(server) as client: + return await client.call_tool( + "create_thread_message", + { "thread_id": self.thread.id, "content": " ", # whitespace-only -> ValidationError }, - ), - ) - return await call_tool(request) + ) finally: _mcp_user.reset(token) - response = self._run(run_test()) - # The MCP server wraps tool results in ServerResult; the text - # content is on response.root.content[0].text. - text = response.root.content[0].text - payload = json.loads(text) + result = self._run(run_test()) + # A Django ValidationError is a *structured* tool result (the LLM can + # read and correct it), not an ``isError`` transport-level failure. + self.assertFalse(result.is_error) + payload = json.loads(result.content[0].text) self.assertIn("error", payload) self.assertIn("empty", payload["error"].lower()) @@ -6943,7 +6944,7 @@ def test_scoped_authenticated_create_thread_message_succeeds(self): ``create_thread_message`` write tool must work end-to-end when an authenticated user is set in ``_mcp_user``. """ - from mcp.types import CallToolRequest, CallToolRequestParams + from mcp.client import Client from opencontractserver.conversations.models import ChatMessage from opencontractserver.mcp.server import ( @@ -6952,28 +6953,24 @@ def test_scoped_authenticated_create_thread_message_succeeds(self): ) server = create_scoped_mcp_server(self.corpus.slug) - call_tool = server.request_handlers[CallToolRequest] async def run_test(): token = _mcp_user.set(self.owner) try: - request = CallToolRequest( - method="tools/call", - params=CallToolRequestParams( - name="create_thread_message", - arguments={ + async with Client(server) as client: + return await client.call_tool( + "create_thread_message", + { "thread_id": self.thread.id, "content": "scoped write happy path", }, - ), - ) - return await call_tool(request) + ) finally: _mcp_user.reset(token) - response = self._run(run_test()) - text = response.root.content[0].text - payload = json.loads(text) + result = self._run(run_test()) + self.assertFalse(result.is_error) + payload = json.loads(result.content[0].text) self.assertNotIn("error", payload) self.assertEqual(payload["content"], "scoped write happy path") # And the message really was persisted with the right creator. @@ -6994,13 +6991,16 @@ class MCPNonScopedListToolsTest(_MCPAsyncRunMixin, TestCase): """ def test_create_thread_message_advertised_in_list_tools(self): - from mcp.types import ListToolsRequest + from mcp.client import Client from opencontractserver.mcp.server import TOOL_HANDLERS, mcp_server - handler = mcp_server.request_handlers[ListToolsRequest] - result = self._run(handler(ListToolsRequest(method="tools/list"))) - tool_names = {t.name for t in result.root.tools} + async def run_test(): + async with Client(mcp_server) as client: + return await client.list_tools() + + result = self._run(run_test()) + tool_names = {t.name for t in result.tools} self.assertIn( "create_thread_message", @@ -7013,6 +7013,375 @@ def test_create_thread_message_advertised_in_list_tools(self): self.assertEqual(tool_names, set(TOOL_HANDLERS.keys())) +class MCPSdkClientRoundTripTest(_MCPAsyncRunMixin, TransactionTestCase): + """End-to-end contract of both MCP servers against the python-sdk 2.x + runtime. + + Every test here drives a server through the SDK's own client + (``mcp.client.Client`` over in-memory streams, or a real stateless + Streamable HTTP JSON-RPC request through ``StreamableHTTPSessionManager``) + rather than calling our dispatchers directly. That is the seam the 2.x + migration changed — handler registration via ``on_*=`` constructor + kwargs, typed result envelopes, argument validation, error wrapping — so + these are the tests that fail if the SDK contract drifts again. + + TransactionTestCase + ``asyncio.run`` for the same reasons as the other + async MCP classes: the server task talks to the ORM via ``sync_to_async`` + on a worker thread, which needs committed data on its own connection. + """ + + def setUp(self): + self.owner = User.objects.create_user( + username="sdkroundtrip", + email="sdkroundtrip@test.com", + password="testpass123", + ) + self.corpus = Corpus.objects.create( + title="SDK Round Trip Corpus", + description="Public corpus for SDK contract tests", + creator=self.owner, + is_public=True, + ) + self.private_corpus = Corpus.objects.create( + title="SDK Private Corpus", + creator=self.owner, + is_public=False, + ) + + def tearDown(self): + from django import db + + self._close_async_db_connections() + db.connections.close_all() + + # ------------------------------------------------------------------ helpers + + @staticmethod + async def _with_client(server, coro_factory, user=None): + """Run ``coro_factory(client)`` inside an in-memory client session. + + ``_mcp_user`` is set BEFORE the client is entered: the SDK spawns the + server task inside ``Client.__aenter__`` and the task inherits the + current context, which is exactly how the ASGI layer hands the + authenticated user to handlers in production. + """ + from mcp.client import Client + + from opencontractserver.mcp.server import _mcp_user + + token = _mcp_user.set(user) + try: + async with Client(server) as client: + return await coro_factory(client) + finally: + _mcp_user.reset(token) + + @staticmethod + async def _expect_mcp_error(awaitable): + """Await a client call that must fail and hand back the ``MCPError``. + + Caught *inside* the client session on purpose: an exception escaping + ``async with Client(...)`` is re-raised by anyio as an + ``ExceptionGroup``, which hides the JSON-RPC code under test. + """ + from mcp.shared.exceptions import MCPError + + try: + await awaitable + except MCPError as exc: + return exc + return None + + # ------------------------------------------------------------ global server + + def test_global_server_advertises_tools_and_templates(self): + from opencontractserver.mcp.server import TOOL_HANDLERS, create_mcp_server + + async def scenario(client): + tools = await client.list_tools() + templates = await client.list_resource_templates() + resources = await client.list_resources() + return tools, templates, resources + + tools, templates, resources = self._run( + self._with_client(create_mcp_server(), scenario) + ) + self.assertEqual({t.name for t in tools.tools}, set(TOOL_HANDLERS)) + self.assertEqual( + {t.uri_template for t in templates.resource_templates}, + { + "corpus://{corpus_slug}", + "document://{corpus_slug}/{document_slug}", + "annotation://{corpus_slug}/{document_slug}/{annotation_id}", + "thread://{corpus_slug}/threads/{thread_id}", + }, + ) + # The global server exposes templates only — concrete resources need + # a corpus slug, which only the scoped server can bind. + self.assertEqual(resources.resources, []) + + def test_global_server_call_tool_returns_json_payload(self): + from opencontractserver.mcp.server import create_mcp_server + + result = self._run( + self._with_client( + create_mcp_server(), + lambda c: c.call_tool("list_public_corpuses", {"limit": 10}), + ) + ) + self.assertFalse(result.is_error) + payload = json.loads(result.content[0].text) + slugs = {c["slug"] for c in payload["corpuses"]} + self.assertIn(self.corpus.slug, slugs) + self.assertNotIn(self.private_corpus.slug, slugs) + + def test_global_server_rejects_mistyped_arguments(self): + """The 1.x decorator validated arguments against ``inputSchema``; the + 2.x adapter must keep doing so, and report it as an ``isError`` result + (not a transport error) so the LLM can self-correct. + """ + from opencontractserver.mcp.server import create_mcp_server + + result = self._run( + self._with_client( + create_mcp_server(), + lambda c: c.call_tool("list_public_corpuses", {"limit": "ten"}), + ) + ) + self.assertTrue(result.is_error) + self.assertIn("Input validation error", result.content[0].text) + self.assertIn("integer", result.content[0].text) + + def test_global_server_unknown_tool_is_error_result(self): + from opencontractserver.mcp.server import create_mcp_server + + result = self._run( + self._with_client( + create_mcp_server(), lambda c: c.call_tool("no_such_tool", {}) + ) + ) + self.assertTrue(result.is_error) + self.assertIn("Unknown tool: no_such_tool", result.content[0].text) + + def test_global_server_reads_corpus_resource_as_json(self): + from opencontractserver.constants.mcp import MCP_RESOURCE_MIME_TYPE + from opencontractserver.mcp.server import create_mcp_server + + uri = f"corpus://{self.corpus.slug}" + result = self._run( + self._with_client(create_mcp_server(), lambda c: c.read_resource(uri)) + ) + self.assertEqual(len(result.contents), 1) + contents = result.contents[0] + self.assertEqual(contents.uri, uri) + self.assertEqual(contents.mime_type, MCP_RESOURCE_MIME_TYPE) + self.assertEqual(json.loads(contents.text)["title"], self.corpus.title) + + def test_global_server_invalid_resource_uri_is_invalid_params(self): + from mcp.shared.exceptions import MCPError + from mcp.types import INVALID_PARAMS + + from opencontractserver.mcp.server import create_mcp_server + + error = self._run( + self._with_client( + create_mcp_server(), + lambda c: self._expect_mcp_error(c.read_resource("bogus://x")), + ) + ) + self.assertIsInstance(error, MCPError) + self.assertEqual(error.code, INVALID_PARAMS) + self.assertIn("unrecognized resource URI", error.message) + + def test_global_server_private_resource_hidden_from_anonymous(self): + from mcp.shared.exceptions import MCPError + from mcp.types import INVALID_PARAMS + + from opencontractserver.mcp.server import create_mcp_server + + uri = f"corpus://{self.private_corpus.slug}" + error = self._run( + self._with_client( + create_mcp_server(), + lambda c: self._expect_mcp_error(c.read_resource(uri)), + ) + ) + self.assertIsInstance(error, MCPError) + self.assertEqual(error.code, INVALID_PARAMS) + + # ...but the owner, carried via the ``_mcp_user`` context, can read it. + result = self._run( + self._with_client( + create_mcp_server(), lambda c: c.read_resource(uri), user=self.owner + ) + ) + self.assertEqual( + json.loads(result.contents[0].text)["slug"], self.private_corpus.slug + ) + + # ------------------------------------------------------------ scoped server + + def test_scoped_server_round_trip(self): + from opencontractserver.mcp.server import create_scoped_mcp_server + + server = create_scoped_mcp_server(self.corpus.slug) + + async def scenario(client): + tools = await client.list_tools() + resources = await client.list_resources() + info = await client.call_tool("get_corpus_info", {}) + return tools, resources, info + + tools, resources, info = self._run(self._with_client(server, scenario)) + tool_names = {t.name for t in tools.tools} + self.assertIn("get_corpus_info", tool_names) + self.assertNotIn("list_public_corpuses", tool_names) + # ``corpus_slug`` is bound from the URL, never required of the client. + for tool in tools.tools: + self.assertNotIn("corpus_slug", tool.input_schema.get("required", [])) + self.assertIn( + f"corpus://{self.corpus.slug}", {str(r.uri) for r in resources.resources} + ) + self.assertFalse(info.is_error) + self.assertEqual(json.loads(info.content[0].text)["title"], self.corpus.title) + + def test_scoped_server_honors_authenticated_user_context(self): + """A private corpus is a structured permission error for anonymous + callers but fully usable by its owner — through the SDK runtime, so + the context propagation from ``_mcp_user`` into the server task is + what is under test. + """ + from opencontractserver.mcp.server import create_scoped_mcp_server + + server = create_scoped_mcp_server(self.private_corpus.slug) + + anonymous = self._run( + self._with_client(server, lambda c: c.call_tool("get_corpus_info", {})) + ) + self.assertFalse(anonymous.is_error) + self.assertIn("not accessible", json.loads(anonymous.content[0].text)["error"]) + + owner = self._run( + self._with_client( + server, lambda c: c.call_tool("get_corpus_info", {}), user=self.owner + ) + ) + self.assertFalse(owner.is_error) + self.assertEqual( + json.loads(owner.content[0].text)["title"], self.private_corpus.title + ) + + # --------------------------------------------------- streamable HTTP (ASGI) + + def test_stateless_streamable_http_json_rpc_round_trip(self): + """Drive a real JSON-RPC request through ``create_mcp_asgi_app`` and a + live ``StreamableHTTPSessionManager`` in stateless mode. + + Covers the transport wiring the in-memory client skips: the ASGI + routing/rate-limit/auth shell, the SDK's HTTP session manager, and the + SSE response framing. A fresh manager is run inside this test's event + loop (and patched in) so the module-level singleton is never bound to + a loop that closes when the test ends. + """ + from unittest.mock import AsyncMock, patch + + from mcp.server.streamable_http_manager import StreamableHTTPSessionManager + + from opencontractserver.mcp.server import ( + TOOL_HANDLERS, + create_mcp_asgi_app, + mcp_server, + ) + + async def json_rpc(app, body: dict) -> dict: + payload = json.dumps(body).encode() + sent: list[dict] = [] + delivered = False + finished = asyncio.Event() + + async def receive(): + nonlocal delivered + if not delivered: + delivered = True + return {"type": "http.request", "body": payload, "more_body": False} + await finished.wait() + return {"type": "http.disconnect"} + + async def send(message): + sent.append(message) + if message["type"] == "http.response.body" and not message.get( + "more_body", False + ): + finished.set() + + scope = { + "type": "http", + "method": "POST", + "path": "/mcp/", + "query_string": b"", + "headers": [ + (b"content-type", b"application/json"), + (b"accept", b"application/json, text/event-stream"), + (b"content-length", str(len(payload)).encode()), + ], + "client": ("127.0.0.1", 12345), + "server": ("127.0.0.1", 8000), + } + await asyncio.wait_for(app(scope, receive, send), timeout=20) + + start = next(m for m in sent if m["type"] == "http.response.start") + self.assertEqual(start["status"], 200) + headers = {k.decode(): v.decode() for k, v in start["headers"]} + self.assertTrue(headers["content-type"].startswith("text/event-stream")) + body = b"".join( + m.get("body", b"") for m in sent if m["type"] == "http.response.body" + ).decode() + data_lines = [ + line[len("data:") :].strip() + for line in body.splitlines() + if line.startswith("data:") + ] + self.assertEqual(len(data_lines), 1, body) + return json.loads(data_lines[0]) + + async def run_test(): + manager = StreamableHTTPSessionManager(app=mcp_server, stateless=True) + async with manager.run(): + with patch( + "opencontractserver.mcp.server.get_session_manager", + return_value=manager, + ), patch( + "opencontractserver.mcp.server.lifespan_manager.ensure_started", + new=AsyncMock(), + ): + app = create_mcp_asgi_app() + listing = await json_rpc( + app, {"jsonrpc": "2.0", "id": 1, "method": "tools/list"} + ) + call = await json_rpc( + app, + { + "jsonrpc": "2.0", + "id": 2, + "method": "tools/call", + "params": { + "name": "list_public_corpuses", + "arguments": {"search": "SDK Round Trip"}, + }, + }, + ) + return listing, call + + listing, call = self._run(run_test()) + self.assertEqual(listing["id"], 1) + self.assertEqual( + {t["name"] for t in listing["result"]["tools"]}, set(TOOL_HANDLERS) + ) + self.assertFalse(call["result"]["isError"]) + payload = json.loads(call["result"]["content"][0]["text"]) + self.assertEqual([c["slug"] for c in payload["corpuses"]], [self.corpus.slug]) + + class MCPExtractBearerTokenTest(TestCase): """Edge cases for ``_extract_bearer_token``. diff --git a/requirements/base.txt b/requirements/base.txt index 9b1ccb3c5..3c3d84420 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -81,13 +81,14 @@ posthog==7.47.1 # https://github.com/posthog/posthog-python # Model Context Protocol # ------------------------------------------------------------------------------ mcp>=2.2.0,<3 # https://github.com/modelcontextprotocol/python-sdk -# ^ pydantic-ai-slim[mcp] -> fastmcp-slim caps mcp<2.0 across its whole -# published range as of 2026-08; mcp 2.0 is a breaking rewrite (decorator-based -# handler registration on mcp.server.lowlevel.Server removed in favor of -# on_*= constructor kwargs) that opencontractserver/mcp/server.py does not yet -# speak. Bump this pin only alongside a migration of that file, once -# fastmcp-slim ships v2 support (or fastmcp-slim/mcp is dropped in favor of -# calling the v2 API directly). +# ^ python-sdk 2.x: request handlers are registered as on_*= constructor +# kwargs on mcp.server.Server and return typed result models; the 1.x +# decorator API (@server.list_tools() etc.) is gone. opencontractserver/mcp/ +# server.py targets the 2.x shape (see its "MCP SDK HANDLER ADAPTERS" +# section). The <3 cap is deliberate: 3.0 is slated to flip auth defaults +# (AuthSettings.validate_token_resource, mandatory issuer=) — re-audit +# server.py before widening. fastmcp-slim (via pydantic-ai-slim[mcp]) +# resolves against mcp 2.x; `pip check` on the resolved tree stays clean. # Not directly required, pinned by Snyk to avoid a vulnerability # ------------------------------------------------------------------------------ From e9da3dc33db36e2bba85fd316c80bf9eb097d5c0 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 14 Sep 2026 08:44:19 +0000 Subject: [PATCH 3/5] Account for schema-rejected MCP tool calls; pin jsonschema explicitly Follow-up to the python-sdk 2.x migration, addressing the automated review on #2334: - A call rejected by inputSchema validation never reached the dispatcher, so it consumed no per-tool rate-limit budget and left no telemetry row. _reject_malformed_arguments now runs _check_per_tool_rate_limit and records an InputValidationError event before returning the isError result; scoped servers pass their URL-bound corpus_slug for attribution. (1.x's call_tool decorator had the same pre-validation gap; this closes it rather than merely restoring parity.) - The adapter's catch-all now also covers a jsonschema.SchemaError from a malformed tool definition, and MCPToolSchemaValidityTest checks every advertised inputSchema against the JSON Schema meta-schema so such a typo fails in CI instead of at runtime. - jsonschema is imported directly by server.py, so it gets an explicit pin in requirements/base.txt instead of riding on mcp's dependency tree. --- changelog.d/2334-mcp-sdk-2.changed.md | 2 +- docs/mcp/README.md | 3 +- opencontractserver/mcp/server.py | 58 ++++++++-- opencontractserver/mcp/tests/test_mcp.py | 128 +++++++++++++++++++++++ requirements/base.txt | 1 + 5 files changed, 181 insertions(+), 11 deletions(-) diff --git a/changelog.d/2334-mcp-sdk-2.changed.md b/changelog.d/2334-mcp-sdk-2.changed.md index 308f62cb1..3f6712ab6 100644 --- a/changelog.d/2334-mcp-sdk-2.changed.md +++ b/changelog.d/2334-mcp-sdk-2.changed.md @@ -1 +1 @@ -- **Migrated the MCP server to `mcp` (python-sdk) 2.x** (`requirements/base.txt`: `mcp>=2.2.0,<3`; #2334). python-sdk 2.0 removed the decorator-based handler registration on `mcp.server.Server` (`@server.list_tools()`, `server.call_tool()(...)`) in favour of `on_*=` constructor kwargs whose handlers take `(ctx, params)` and return typed result models. `opencontractserver/mcp/server.py` now builds both the global server (`create_mcp_server`) and the per-corpus scoped server (`create_scoped_mcp_server`) through a shared set of adapters (`_build_on_call_tool`, `_build_on_list_tools`, `_build_on_list_resource_templates`, `_on_read_resource`) so argument validation, error wrapping and resource serialisation live in one place. Behaviour the 1.x decorators used to provide is preserved explicitly: tool arguments are validated against the advertised `inputSchema` (jsonschema; mistyped arguments return an `isError` result, `"Input validation error: ..."`), and any exception escaping a dispatcher (unknown tool, rate limit) still becomes an `isError` result rather than a transport error. The global tool/template catalogues moved out of the factory closure into `get_tool_definitions()` / `get_resource_template_definitions()` (mirroring the scoped equivalents). Wire-level changes: `resources/read` payloads are now stamped `application/json` (`MCP_RESOURCE_MIME_TYPE`, matching the advertised templates; 1.x's deprecated str-return path stamped `text/plain`), and a resource read that fails on the caller's side (unrecognised URI, invisible/missing corpus or document) returns JSON-RPC `INVALID_PARAMS` with the message instead of a bare internal error. SDK types now use their 2.x snake_case fields (`input_schema`, `uri_template`, `mime_type`; URIs are plain `str`). Tests: the SDK-level tests drive both servers through `mcp.client.Client` in-memory sessions (`MCPSdkClientRoundTripTest`) and a real stateless Streamable HTTP JSON-RPC request through `StreamableHTTPSessionManager`, replacing the removed `server.request_handlers[...]` seam. +- **Migrated the MCP server to `mcp` (python-sdk) 2.x** (`requirements/base.txt`: `mcp>=2.2.0,<3`; #2334). python-sdk 2.0 removed the decorator-based handler registration on `mcp.server.Server` (`@server.list_tools()`, `server.call_tool()(...)`) in favour of `on_*=` constructor kwargs whose handlers take `(ctx, params)` and return typed result models. `opencontractserver/mcp/server.py` now builds both the global server (`create_mcp_server`) and the per-corpus scoped server (`create_scoped_mcp_server`) through a shared set of adapters (`_build_on_call_tool`, `_build_on_list_tools`, `_build_on_list_resource_templates`, `_on_read_resource`) so argument validation, error wrapping and resource serialisation live in one place. Behaviour the 1.x decorators used to provide is preserved explicitly: tool arguments are validated against the advertised `inputSchema` (jsonschema; mistyped arguments return an `isError` result, `"Input validation error: ..."`, and — new — still consume the per-tool rate-limit bucket and record an `InputValidationError` telemetry event via `_reject_malformed_arguments`, so malformed calls cannot bypass either), and any exception escaping a dispatcher (unknown tool, rate limit) still becomes an `isError` result rather than a transport error. The global tool/template catalogues moved out of the factory closure into `get_tool_definitions()` / `get_resource_template_definitions()` (mirroring the scoped equivalents). Wire-level changes: `resources/read` payloads are now stamped `application/json` (`MCP_RESOURCE_MIME_TYPE`, matching the advertised templates; 1.x's deprecated str-return path stamped `text/plain`), and a resource read that fails on the caller's side (unrecognised URI, invisible/missing corpus or document) returns JSON-RPC `INVALID_PARAMS` with the message instead of a bare internal error. `jsonschema` is now an explicit pin in `requirements/base.txt` (it was only transitive via `mcp`). SDK types now use their 2.x snake_case fields (`input_schema`, `uri_template`, `mime_type`; URIs are plain `str`). Tests: the SDK-level tests drive both servers through `mcp.client.Client` in-memory sessions (`MCPSdkClientRoundTripTest`) and a real stateless Streamable HTTP JSON-RPC request through `StreamableHTTPSessionManager`, replacing the removed `server.request_handlers[...]` seam. diff --git a/docs/mcp/README.md b/docs/mcp/README.md index 4ff5c7d67..a4fd79138 100644 --- a/docs/mcp/README.md +++ b/docs/mcp/README.md @@ -274,7 +274,8 @@ Contract preserved from 1.x and pinned by `MCPSdkClientRoundTripTest` (`opencontractserver/mcp/tests/test_mcp.py`): - Tool arguments are validated against the advertised `inputSchema`; a - mismatch returns an `isError` result (`Input validation error: ...`). + mismatch returns an `isError` result (`Input validation error: ...`) and + still consumes the per-tool rate-limit bucket and records telemetry. - Exceptions escaping a dispatcher (unknown tool, rate limit) become `isError` results, never transport errors. Django `PermissionDenied` / `ValidationError` / `DoesNotExist` are structured `{"error": ...}` payloads. diff --git a/opencontractserver/mcp/server.py b/opencontractserver/mcp/server.py index 1257c367b..29f21cb92 100644 --- a/opencontractserver/mcp/server.py +++ b/opencontractserver/mcp/server.py @@ -794,8 +794,37 @@ def _tool_error_result(message: str) -> CallToolResult: ) +async def _reject_malformed_arguments( + name: str, + arguments: dict, + error: jsonschema.ValidationError, + *, + corpus_slug: str | None, +) -> CallToolResult: + """Account for a call rejected by schema validation, then report it. + + A schema failure never reaches the dispatcher, which is where per-tool + rate limiting and telemetry normally happen. Do both here so a malformed + call is billed and recorded exactly like one that fails inside the + dispatcher — otherwise a client could hammer a tool with deliberately + bad arguments without ever touching its rate-limit bucket, and + dashboards built on ``arecord_mcp_tool_call`` would undercount it. + ``_check_per_tool_rate_limit`` raises ``MCPRateLimitError`` (recording + the rejection itself); the caller turns that into an ``isError`` result. + """ + await _check_per_tool_rate_limit(name) + await arecord_mcp_tool_call( + name, + success=False, + error_type="InputValidationError", + corpus_slug=corpus_slug or arguments.get("corpus_slug"), + document_slug=arguments.get("document_slug"), + ) + return _tool_error_result(f"Input validation error: {error.message}") + + def _build_on_call_tool( - dispatch: ToolDispatcher, tools: list[Tool] + dispatch: ToolDispatcher, tools: list[Tool], *, corpus_slug: str | None = None ) -> Callable[[ServerRequestContext, CallToolRequestParams], Awaitable[CallToolResult]]: """Wrap a tool dispatcher as an ``on_call_tool`` handler. @@ -805,14 +834,21 @@ def _build_on_call_tool( * Arguments are validated against the advertised ``inputSchema`` before dispatch, so a mistyped argument (``limit="ten"``) comes back as an ``isError`` result naming the problem instead of a ``TypeError`` deep - inside a handler. + inside a handler. Rejected calls still consume the per-tool rate-limit + bucket and are recorded in telemetry (``_reject_malformed_arguments``). * Any exception escaping the dispatcher (unknown tool, rate limit, an unexpected handler failure) becomes an ``isError`` result rather than a - JSON-RPC transport error, so LLM clients can read and react to it. + JSON-RPC transport error, so LLM clients can read and react to it. The + same net catches a ``jsonschema.SchemaError`` from a malformed tool + definition — ``MCPToolSchemaValidityTest`` keeps that from shipping. ``PermissionDenied`` / ``ValidationError`` / ``ObjectDoesNotExist`` never reach the ``except`` here — both dispatchers already convert those into a structured ``{"error": ...}`` payload (``_record_and_return_tool_error``). + + ``corpus_slug`` is the URL-bound corpus of a scoped server (``None`` for + the global server, whose tools carry the slug in their arguments); it only + feeds telemetry for schema-rejected calls. """ tools_by_name = {tool.name: tool for tool in tools} @@ -821,12 +857,14 @@ async def on_call_tool( ) -> CallToolResult: arguments = params.arguments or {} tool = tools_by_name.get(params.name) - if tool is not None: - try: - jsonschema.validate(instance=arguments, schema=tool.input_schema) - except jsonschema.ValidationError as e: - return _tool_error_result(f"Input validation error: {e.message}") try: + if tool is not None: + try: + jsonschema.validate(instance=arguments, schema=tool.input_schema) + except jsonschema.ValidationError as e: + return await _reject_malformed_arguments( + params.name, arguments, e, corpus_slug=corpus_slug + ) content = await dispatch(params.name, arguments) except Exception as e: return _tool_error_result(str(e)) @@ -1652,7 +1690,9 @@ async def call_tool(name: str, arguments: dict) -> list[TextContent]: # Resource reads reuse the global handler (it validates corpus access). on_read_resource=_on_read_resource, on_list_tools=_build_on_list_tools(scoped_tools), - on_call_tool=_build_on_call_tool(call_tool, scoped_tools), + on_call_tool=_build_on_call_tool( + call_tool, scoped_tools, corpus_slug=corpus_slug + ), ) diff --git a/opencontractserver/mcp/tests/test_mcp.py b/opencontractserver/mcp/tests/test_mcp.py index aa6b5c794..7a49f6f38 100644 --- a/opencontractserver/mcp/tests/test_mcp.py +++ b/opencontractserver/mcp/tests/test_mcp.py @@ -7013,6 +7013,32 @@ async def run_test(): self.assertEqual(tool_names, set(TOOL_HANDLERS.keys())) +class MCPToolSchemaValidityTest(TestCase): + """Every advertised ``inputSchema`` must itself be a valid JSON Schema. + + ``_build_on_call_tool`` validates client arguments with ``jsonschema``; + a typo in a hand-written schema would surface at runtime as a + ``SchemaError`` (an ``isError`` result for every call to that tool) + rather than at import. Pin it here so it fails in CI instead. + """ + + def test_global_and_scoped_tool_schemas_are_valid(self): + import jsonschema + + from opencontractserver.mcp.server import ( + get_scoped_tool_definitions, + get_tool_definitions, + ) + + tools = get_tool_definitions() + get_scoped_tool_definitions("some-corpus") + self.assertTrue(tools) + for tool in tools: + with self.subTest(tool=tool.name): + schema = tool.input_schema + self.assertEqual(schema.get("type"), "object", tool.name) + jsonschema.validators.validator_for(schema).check_schema(schema) + + class MCPSdkClientRoundTripTest(_MCPAsyncRunMixin, TransactionTestCase): """End-to-end contract of both MCP servers against the python-sdk 2.x runtime. @@ -7152,6 +7178,108 @@ def test_global_server_rejects_mistyped_arguments(self): self.assertIn("Input validation error", result.content[0].text) self.assertIn("integer", result.content[0].text) + def test_schema_rejection_still_rate_limits_and_records_telemetry(self): + """A call rejected by ``inputSchema`` validation never reaches the + dispatcher, so the adapter must do the per-tool rate-limit accounting + and telemetry itself — otherwise malformed calls would be free. + """ + from unittest.mock import AsyncMock, patch + + from opencontractserver.mcp.server import _mcp_asgi_scope, create_mcp_server + + scope = {"type": "http", "path": "/mcp/", "client": ("127.0.0.1", 1)} + rate_limit = AsyncMock(return_value=(False, "", 0)) + record = AsyncMock() + + async def run_test(): + token = _mcp_asgi_scope.set(scope) + try: + with patch( + "opencontractserver.mcp.server.check_mcp_rate_limit", rate_limit + ), patch("opencontractserver.mcp.server.arecord_mcp_tool_call", record): + return await self._with_client( + create_mcp_server(), + lambda c: c.call_tool( + "list_documents", + {"corpus_slug": self.corpus.slug, "limit": "ten"}, + ), + ) + finally: + _mcp_asgi_scope.reset(token) + + result = self._run(run_test()) + self.assertTrue(result.is_error) + self.assertIn("Input validation error", result.content[0].text) + rate_limit.assert_awaited_once_with( + scope, tool_name="list_documents", skip_global=True + ) + record.assert_awaited_once_with( + "list_documents", + success=False, + error_type="InputValidationError", + corpus_slug=self.corpus.slug, + document_slug=None, + ) + + def test_schema_rejection_honors_per_tool_rate_limit(self): + """When the per-tool bucket is exhausted, a malformed call is rejected + as rate-limited (``isError``) before any validation message leaks. + """ + from unittest.mock import AsyncMock, patch + + from opencontractserver.mcp.server import _mcp_asgi_scope, create_mcp_server + + scope = {"type": "http", "path": "/mcp/", "client": ("127.0.0.1", 1)} + rate_limit = AsyncMock(return_value=(True, "Rate limit exceeded", 30)) + + async def run_test(): + token = _mcp_asgi_scope.set(scope) + try: + with patch( + "opencontractserver.mcp.server.check_mcp_rate_limit", rate_limit + ), patch( + "opencontractserver.mcp.server.arecord_mcp_tool_call", AsyncMock() + ): + return await self._with_client( + create_mcp_server(), + lambda c: c.call_tool("list_public_corpuses", {"limit": "x"}), + ) + finally: + _mcp_asgi_scope.reset(token) + + result = self._run(run_test()) + self.assertTrue(result.is_error) + self.assertEqual(result.content[0].text, "Rate limit exceeded") + + def test_scoped_schema_rejection_records_url_bound_corpus(self): + """Scoped tools carry no ``corpus_slug`` argument; telemetry for a + rejected call must still attribute it to the URL-bound corpus. + """ + from unittest.mock import AsyncMock, patch + + from opencontractserver.mcp.server import create_scoped_mcp_server + + record = AsyncMock() + + async def run_test(): + with patch("opencontractserver.mcp.server.arecord_mcp_tool_call", record): + return await self._with_client( + create_scoped_mcp_server(self.corpus.slug), + lambda c: c.call_tool( + "list_documents", {"document_slug_typo": 1, "limit": "x"} + ), + ) + + result = self._run(run_test()) + self.assertTrue(result.is_error) + record.assert_awaited_once_with( + "list_documents", + success=False, + error_type="InputValidationError", + corpus_slug=self.corpus.slug, + document_slug=None, + ) + def test_global_server_unknown_tool_is_error_result(self): from opencontractserver.mcp.server import create_mcp_server diff --git a/requirements/base.txt b/requirements/base.txt index 3c3d84420..9b6bd911f 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -81,6 +81,7 @@ posthog==7.47.1 # https://github.com/posthog/posthog-python # Model Context Protocol # ------------------------------------------------------------------------------ mcp>=2.2.0,<3 # https://github.com/modelcontextprotocol/python-sdk +jsonschema>=4.20.0,<5 # https://github.com/python-jsonschema/jsonschema - MCP tool-argument validation (server.py); also an mcp dependency, pinned explicitly so it cannot vanish under us # ^ python-sdk 2.x: request handlers are registered as on_*= constructor # kwargs on mcp.server.Server and return typed result models; the 1.x # decorator API (@server.list_tools() etc.) is gone. opencontractserver/mcp/ From 29f5f5fdfb1a36e32bedd0f8fb4f8599c6ee31c6 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 14 Sep 2026 08:51:13 +0000 Subject: [PATCH 4/5] Use MCP_RESOURCE_MIME_TYPE at every resource definition; drop stale AnyUrl test comment Follow-up nits from the automated review on #2334: the constant added for resources/read payloads now also backs every Resource/ResourceTemplate mime_type so the advertised and served types cannot drift, and the scoped resource test no longer claims Resource.uri is an AnyUrl (mcp 2.x types it as str). --- opencontractserver/mcp/server.py | 20 ++++++++++---------- opencontractserver/mcp/tests/test_mcp.py | 4 ++-- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/opencontractserver/mcp/server.py b/opencontractserver/mcp/server.py index 29f21cb92..e55fd5c19 100644 --- a/opencontractserver/mcp/server.py +++ b/opencontractserver/mcp/server.py @@ -936,25 +936,25 @@ def get_resource_template_definitions() -> list[ResourceTemplate]: uri_template="corpus://{corpus_slug}", name="Public Corpus", description="Access public corpus metadata and contents", - mime_type="application/json", + mime_type=MCP_RESOURCE_MIME_TYPE, ), ResourceTemplate( uri_template="document://{corpus_slug}/{document_slug}", name="Public Document", description="Access public document with extracted text", - mime_type="application/json", + mime_type=MCP_RESOURCE_MIME_TYPE, ), ResourceTemplate( uri_template="annotation://{corpus_slug}/{document_slug}/{annotation_id}", name="Document Annotation", description="Access specific annotation on a document", - mime_type="application/json", + mime_type=MCP_RESOURCE_MIME_TYPE, ), ResourceTemplate( uri_template="thread://{corpus_slug}/threads/{thread_id}", name="Discussion Thread", description="Access public discussion thread with messages", - mime_type="application/json", + mime_type=MCP_RESOURCE_MIME_TYPE, ), ] @@ -1472,7 +1472,7 @@ def get_scoped_resource_definitions( uri=f"corpus://{corpus_slug}", name="Corpus", description=f"Access the '{corpus_slug}' corpus metadata and contents", - mime_type="application/json", + mime_type=MCP_RESOURCE_MIME_TYPE, ) ) @@ -1486,7 +1486,7 @@ def get_scoped_resource_definitions( uri=f"document://{corpus_slug}/{doc.slug}", name=f"Document: {doc.title or doc.slug}", description=doc.description[:100] if doc.description else "Document", - mime_type="application/json", + mime_type=MCP_RESOURCE_MIME_TYPE, ) ) @@ -1509,7 +1509,7 @@ def get_scoped_resource_definitions( if thread.description else "Discussion thread" ), - mime_type="application/json", + mime_type=MCP_RESOURCE_MIME_TYPE, ) ) @@ -1533,19 +1533,19 @@ def get_scoped_resource_template_definitions( uri_template=f"document://{corpus_slug}/{{document_slug}}", name="Document", description="Access document with extracted text", - mime_type="application/json", + mime_type=MCP_RESOURCE_MIME_TYPE, ), ResourceTemplate( uri_template=f"annotation://{corpus_slug}/{{document_slug}}/{{annotation_id}}", name="Annotation", description="Access specific annotation on a document", - mime_type="application/json", + mime_type=MCP_RESOURCE_MIME_TYPE, ), ResourceTemplate( uri_template=f"thread://{corpus_slug}/threads/{{thread_id}}", name="Discussion Thread", description="Access discussion thread with messages", - mime_type="application/json", + mime_type=MCP_RESOURCE_MIME_TYPE, ), ] diff --git a/opencontractserver/mcp/tests/test_mcp.py b/opencontractserver/mcp/tests/test_mcp.py index 7a49f6f38..9675b84d7 100644 --- a/opencontractserver/mcp/tests/test_mcp.py +++ b/opencontractserver/mcp/tests/test_mcp.py @@ -3287,8 +3287,8 @@ def test_get_scoped_resource_definitions(self): # Corpus resource should have the scoped slug corpus_resource = next(r for r in resources if r.name == "Corpus") - # Compare as strings since Resource.uri is an AnyUrl type - self.assertEqual(str(corpus_resource.uri), f"corpus://{self.corpus.slug}") + # mcp 2.x types ``Resource.uri`` as a plain ``str``. + self.assertEqual(corpus_resource.uri, f"corpus://{self.corpus.slug}") def test_get_scoped_resource_template_definitions(self): """Test scoped resource template definitions.""" From a7a1b81b6fdff9e403cc6a6144062837d62e216f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 14 Sep 2026 22:54:08 +0000 Subject: [PATCH 5/5] Import ServerRequestContext from mcp.server; log unexpected MCP tool failures Follow-up notes from the automated review on #2334: ServerRequestContext is part of mcp.server's public __all__, so import it from there instead of the lowlevel submodule (one less internal seam to drift). The adapter's catch-all now also emits a server-side warning so a failure that surfaces to the client as an isError result is visible in logs too. The list() copy on the result content stays: list is invariant and mypy needs the copy to widen list[TextContent] to the SDK's content-block union (comment added). --- opencontractserver/mcp/server.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/opencontractserver/mcp/server.py b/opencontractserver/mcp/server.py index e55fd5c19..ba7a08f3e 100644 --- a/opencontractserver/mcp/server.py +++ b/opencontractserver/mcp/server.py @@ -36,8 +36,7 @@ PermissionDenied, ValidationError, ) -from mcp.server import Server -from mcp.server.lowlevel.server import ServerRequestContext +from mcp.server import Server, ServerRequestContext from mcp.server.sse import SseServerTransport from mcp.server.stdio import stdio_server from mcp.server.streamable_http_manager import StreamableHTTPSessionManager @@ -867,7 +866,17 @@ async def on_call_tool( ) content = await dispatch(params.name, arguments) except Exception as e: + # Client-facing shape is the isError result; keep a server-side + # trace too so a failure is visible in logs, not only to the + # caller (rate limits and unknown tools land here as well). + logger.warning( + "MCP tool %s failed: %s: %s", params.name, type(e).__name__, e + ) return _tool_error_result(str(e)) + # ``list(...)`` is not redundant: ``list`` is invariant, so a + # ``list[TextContent]`` does not type-check against the SDK's + # ``list[ContentBlock]`` union; the copy lets mypy infer the wider + # element type from context. return CallToolResult(content=list(content)) return on_call_tool