Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions src/mcp/client/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -1126,7 +1126,8 @@ async def validate_tool_result(self, name: str, result: types.CallToolResult) ->
"""Revalidate a `CallToolResult` against the tool's declared output schema.

Raises:
RuntimeError: Structured content is missing or does not conform to the schema.
RuntimeError: Structured content is missing or does not conform to the schema, or the
schema is invalid or has a `$ref` that does not resolve within the schema document.
Comment on lines +1129 to +1130

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 nit: This is a user-visible behaviour change — tool output schemas whose $refs point outside the document (previously auto-fetched by jsonschema, so such tools worked) now make call_tool raise RuntimeError: Invalid schema for tool ... — but no page under docs/ is updated, and AGENTS.md requires docs updates for user-visible behaviour changes in the same PR. docs/advanced/low-level-server.md (the section documenting the client-side validation RuntimeError) or docs/servers/structured-output.md should note that $refs resolve only within the schema document and how the failure surfaces.

Extended reasoning...

The PR's own "Breaking Changes" section states that a result whose validation reaches a $ref outside the schema document now fails with RuntimeError, and that a dangling in-document $ref changes exception type from a referencing error to RuntimeError. On the base branch, jsonschema's default registry auto-retrieves remote/file $refs (with a DeprecationWarning), so a tool declaring e.g. {"$ref": "https://example.com/schema.json"} validated successfully; after merge the same tool's every call_tool raises RuntimeError. AGENTS.md ("Documentation") says: "When a change affects public API or user-visible behaviour, update the relevant page(s) under docs/ in the same PR." The diff touches only src/mcp/client/session.py and tests/client/test_output_schema_validation.py; no docs file changes. docs/advanced/low-level-server.md:112 is the existing page documenting the client-side RuntimeError validation contract that users upgrading will consult when their external-$ref tools start failing, and it says nothing about $ref resolution scope. A correct fix adds a sho

Verification: nit. The diff touches only src/mcp/client/session.py and tests/client/test_output_schema_validation.py (verified via git diff e7284ed..HEAD --name-only) — no file under docs/ is updated. AGENTS.md (applicable per scope) states: "When a change affects public API or user-visible behaviour, update the relevant page(s) under docs/ in the same PR." The change is user-visible: at src/mcp/cli

"""
if name not in self._tool_output_schemas:
# refresh output schema cache
Expand All @@ -1140,17 +1141,22 @@ async def validate_tool_result(self, name: str, result: types.CallToolResult) ->

if output_schema is not None:
from jsonschema import exceptions as jsonschema_exceptions
from referencing.exceptions import Unresolvable

if result.structured_content is None:
raise RuntimeError(f"Tool {name} has an output schema but did not return structured content")
validator = self._output_schema_validator(name, output_schema)
# `best_match` picks the same error the previous `jsonschema.validate()` call raised,
# so the message a caller sees is unchanged. It is untyped upstream.
errors = validator.iter_errors(result.structured_content)
error = cast(
"Exception | None",
jsonschema_exceptions.best_match(errors), # pyright: ignore[reportUnknownMemberType]
)
try:
error = cast(
"Exception | None",
jsonschema_exceptions.best_match(errors), # pyright: ignore[reportUnknownMemberType]
)
except Unresolvable as e:
# A `$ref` did not resolve within the schema document.
raise RuntimeError(f"Invalid schema for tool {name}: {e}") from e
if error is not None:
raise RuntimeError(f"Invalid structured content returned by tool {name}: {error}") from error

Expand All @@ -1168,6 +1174,7 @@ def _output_schema_validator(self, name: str, output_schema: dict[str, Any]) ->
"""
from jsonschema import SchemaError
from jsonschema.validators import validator_for
from referencing import Registry

if (validator := self._tool_output_validators.get(name)) is not None:
return validator
Expand All @@ -1177,9 +1184,8 @@ def _output_schema_validator(self, name: str, output_schema: dict[str, Any]) ->
validator_cls.check_schema(output_schema)
except SchemaError as e:
raise RuntimeError(f"Invalid schema for tool {name}: {e}")
# jsonschema ships no `py.typed`, so pyright reads typeshed's stub, which declares
# `registry` as required (concrete validators default it); cast to a schema-only ctor.
validator = cast("Callable[[dict[str, Any]], Validator]", validator_cls)(output_schema)
# An explicit empty registry: `$ref`s resolve within the schema document and the bundled metaschemas.
validator = validator_cls(output_schema, registry=Registry())
self._tool_output_validators[name] = validator
return validator

Expand Down
25 changes: 25 additions & 0 deletions tests/client/test_output_schema_validation.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import logging
from pathlib import Path
from typing import Any

import pytest
Expand All @@ -10,6 +11,7 @@
TextContent,
Tool,
)
from referencing.exceptions import Unresolvable

from mcp import Client
from mcp.server import Server, ServerRequestContext
Expand Down Expand Up @@ -163,3 +165,26 @@ async def on_call_tool(ctx: ServerRequestContext, params: CallToolRequestParams)
assert result.is_error is False

assert "Tool mystery_tool not listed" in caplog.text


# jsonschema's fallback retriever emits this DeprecationWarning; keep it a plain warning so the
# assertions below decide the outcome rather than the suite's warnings-as-errors filter.
@pytest.mark.filterwarnings("default:Automatically retrieving remote references:DeprecationWarning")
@pytest.mark.anyio
async def test_output_schema_ref_outside_the_document_is_rejected(tmp_path: Path):
"""A `$ref` to a URI outside the output schema is not resolved, and a result whose validation
reaches one fails as an invalid schema (spec `$ref` resolution; applying it to `file:` URIs too
is SDK-defined)."""
target = tmp_path / "schema.json"
target.write_text("{}", encoding="utf-8")
server = _make_server(
tools=[Tool(name="probe", input_schema={"type": "object"}, output_schema={"$ref": target.as_uri()})],
structured_content={"v": 1},
)

async with Client(server) as client:
with pytest.raises(RuntimeError) as exc_info:
await client.call_tool("probe", {})
# SDK-authored prefix only; the tail is `referencing`'s text.
assert str(exc_info.value).startswith("Invalid schema for tool probe: ")
assert isinstance(exc_info.value.__cause__, Unresolvable)
Loading