From 193b6eee76864153fb94df979c0c2afd79005b96 Mon Sep 17 00:00:00 2001 From: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Date: Thu, 17 Sep 2026 05:25:24 -0700 Subject: [PATCH] Add ClientSession/Client opt-out for automatic tool-result validation Motivation: call_tool() revalidates a successful CallToolResult's structured_content against the tool's declared output_schema after every call. When the session's output-schema cache is empty -- which it always is on a short-lived session, the pattern stateless gateways and proxies use (one ClientSession per call) -- that revalidation triggers a tools/list request to discover the schema. This doubles round-trips on every call_tool and, when the server behind the session is itself an aggregator, adds that aggregator's slowest-backend tools/list latency to every single call, with no way to opt out short of subclassing ClientSession. Approach: Add a validate_tool_results: bool = True constructor parameter to both ClientSession and the high-level Client. When False, call_tool skips the automatic validate_tool_result() call entirely -- on both the direct result path and the SEP-2133 claimed-extension-result path -- so no tools/list is issued and no RuntimeError is raised for output that doesn't match a schema the caller never listed. The default stays True, so existing behavior, including the tests that rely on a fresh session auto-discovering the schema via its first validate_tool_result() call, is unchanged. This is the constructor opt-out shape from the issue's three proposed options (the alternative of skipping the refresh only on a wholly empty cache would have changed default behavior on a fresh session, which several existing tests -- test_validate_tool_result_passes_a_conforming_result and friends in tests/client/test_session_promotions.py -- deliberately lock in). Validation: - `uv run --frozen pytest tests/client/` -- 782 passed, 1 skipped, 1 xfailed - `uv run --frozen ruff format --check .` / `ruff check .` -- clean - `uv run --frozen pyright` on changed files -- 0 errors - `./scripts/test` (full coverage-gated suite) -- 5970 passed, 100.00% coverage, strict-no-cover clean - `uv run --frozen pre-commit run --files ` -- markdownlint and ruff hooks pass; the pyright hook's only failure (tests/transports/stdio/test_lifecycle.py:190, os.waitid) is confirmed pre-existing on a clean main via git stash, unrelated to this change - Base branch CI (`gh run list --branch main --event push`) is green as of the last push Report: https://github.com/modelcontextprotocol/python-sdk/issues/3513 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Assisted-by: claude-sonnet-5 (via Claude Code) --- docs/advanced/low-level-server.md | 2 ++ src/mcp/client/client.py | 10 +++++++++- src/mcp/client/session.py | 9 ++++++++- tests/client/test_client_extensions.py | 17 +++++++++++++++++ tests/client/test_session_promotions.py | 17 +++++++++++++++++ 5 files changed, 53 insertions(+), 2 deletions(-) diff --git a/docs/advanced/low-level-server.md b/docs/advanced/low-level-server.md index 5d49846b5f..4f879022a3 100644 --- a/docs/advanced/low-level-server.md +++ b/docs/advanced/low-level-server.md @@ -117,6 +117,8 @@ The `_meta` block is the server's identity stamp: the SDK adds it to every 2026- The server never compares the two fields. This SDK's `Client` does: return `structured_content` that doesn't satisfy the `output_schema` you declared and `call_tool` raises a `RuntimeError` that starts with `Invalid structured content returned by tool search_books` and goes on to quote the `jsonschema` failure. Promising a schema is cheap; keeping it is on you. The whole ladder of return types and schemas is in **[Structured Output](../servers/structured-output.md)**. +That check costs a `tools/list` round-trip on a session that hasn't listed tools yet — the client needs a schema to validate against. A session built for exactly one `call_tool` (as stateless gateways and proxies often do) pays that cost every time; pass `validate_tool_results=False` to `Client`/`ClientSession` to skip the check entirely when the caller already validates elsewhere. + ## The dialect is JSON Schema 2020-12 `input_schema` and `output_schema` are JSON Schema, and the [MCP specification](https://modelcontextprotocol.io/specification/latest/basic#json-schema-usage) fixes the dialect: a schema with no `$schema` key is **JSON Schema 2020-12**. The schemas `MCPServer` generates rely on that default (Pydantic writes 2020-12 and omits the key), and a hand-written dict is held to it too, so the full 2020-12 vocabulary is available: diff --git a/src/mcp/client/client.py b/src/mcp/client/client.py index f921c7e30b..49228e9d3c 100644 --- a/src/mcp/client/client.py +++ b/src/mcp/client/client.py @@ -355,6 +355,13 @@ async def main(): transparently by `call_tool`), and its notification bindings. For an ad-only entry use `mcp.client.advertise(identifier, settings)`.""" + validate_tool_results: bool = True + """Whether `call_tool` revalidates a successful result against the tool's output schema. + + The check costs a `tools/list` round-trip per call on a session that has never listed + tools (e.g. a fresh session per call, as gateways and proxies often use). Set to `False` + when the caller already validates structured output elsewhere.""" + cache: CacheConfig | None = field(default_factory=CacheConfig) """Client-side response caching for the SEP-2549 cacheable methods (2026-07-28). @@ -442,6 +449,7 @@ async def _build_session(self, exit_stack: AsyncExitStack) -> ClientSession: extensions=self._folded_extensions.ad, result_claims=self._folded_extensions.claims, notification_bindings=self._folded_extensions.bindings, + validate_tool_results=self.validate_tool_results, ) async def __aenter__(self) -> Client: @@ -818,7 +826,7 @@ async def retry(r: InputResponses | None, s: str | None) -> CallToolResult | Inp result, ClaimContext(session=self.session, tool_name=name, read_timeout_seconds=read_timeout_seconds), ) - if not final.is_error: + if not final.is_error and self.validate_tool_results: # Match the direct path: revalidate the output schema, but never for isError results. await self.session.validate_tool_result(name, final) return final diff --git a/src/mcp/client/session.py b/src/mcp/client/session.py index a618112153..3519ff4f95 100644 --- a/src/mcp/client/session.py +++ b/src/mcp/client/session.py @@ -399,6 +399,11 @@ class ClientSession: Extension `result_claims` fold into tools/call parsing at `adopt()`; `notification_bindings` observe vendor notifications via bounded FIFOs. + + `validate_tool_results=False` skips the client-side output-schema check + `call_tool` otherwise runs after each successful call. On a session that has + never listed tools, that check costs a `tools/list` round-trip per call; turn + it off when the caller already validates elsewhere. """ def __init__( @@ -419,6 +424,7 @@ def __init__( result_claims: Mapping[str, Sequence[ResultClaim[Any]]] | None = None, notification_bindings: Sequence[NotificationBinding[Any]] | None = None, dispatcher: Dispatcher[Any] | None = None, + validate_tool_results: bool = True, ) -> None: self._session_read_timeout_seconds = read_timeout_seconds self._client_info = client_info or DEFAULT_CLIENT_INFO @@ -437,6 +443,7 @@ def __init__( self._logging_callback = logging_callback or _default_logging_callback self._log_level: types.LoggingLevel | None = log_level self._message_handler = message_handler or _default_message_handler + self._validate_tool_results = validate_tool_results self._tool_output_schemas: dict[str, dict[str, Any] | None] = {} # Compiled output-schema validators, derived from `_tool_output_schemas` and owned by # `_absorb_tool_listing`, which evicts a tool's entry whenever its schema changes. @@ -1098,7 +1105,7 @@ async def call_tool( progress_callback=progress_callback, ) - if isinstance(result, types.CallToolResult) and not result.is_error: + if self._validate_tool_results and isinstance(result, types.CallToolResult) and not result.is_error: await self.validate_tool_result(name, result) # The input_required arm stays first; a claimed shape is terminal for the multi-round-trip driver. diff --git a/tests/client/test_client_extensions.py b/tests/client/test_client_extensions.py index f80cfe8841..7a826c5601 100644 --- a/tests/client/test_client_extensions.py +++ b/tests/client/test_client_extensions.py @@ -421,6 +421,23 @@ async def resolve(claimed: VoucherResult, ctx: ClaimContext) -> CallToolResult: assert str(exc_info.value) == snapshot("Tool issue has an output schema but did not return structured content") +async def test_validate_tool_results_false_skips_revalidation_of_the_resolvers_product() -> None: + """`validate_tool_results=False` must reach the claimed-result path too, not just the direct one: + the same schema-violating product from `test_resolver_product_gets_the_direct_paths_output_schema_revalidation` + comes back unraised here.""" + + async def resolve(claimed: VoucherResult, ctx: ClaimContext) -> CallToolResult: + return CallToolResult(content=[TextContent(text="unstructured")]) + + with anyio.fail_after(5): + async with Client( + _structured_voucher_server(), extensions=[_VoucherExtension(resolve)], validate_tool_results=False + ) as client: + result = await client.call_tool("issue", {}) + + assert result.content == [TextContent(text="unstructured")] + + async def test_resolver_error_result_is_returned_not_raised() -> None: """An `isError` resolver product skips output-schema revalidation and comes back as-is.""" diff --git a/tests/client/test_session_promotions.py b/tests/client/test_session_promotions.py index 6d6b6bc8dc..8b42b6cc70 100644 --- a/tests/client/test_session_promotions.py +++ b/tests/client/test_session_promotions.py @@ -106,6 +106,23 @@ async def test_validate_tool_result_keeps_the_validator_across_a_relisting_of_th assert client.session._tool_output_validators["t"] is compiled +@pytest.mark.anyio +async def test_call_tool_skips_validation_and_the_tools_list_refresh_when_opted_out() -> None: + """`validate_tool_results=False` must skip both the schema check and the `tools/list` + round-trip `validate_tool_result` would otherwise spend discovering it on a fresh session. + + The server has no `on_list_tools` handler at all, so a `tools/list` call would raise + METHOD_NOT_FOUND -- the call succeeding proves the refresh never happened.""" + + async def on_call_tool(ctx: ServerRequestContext, params: types.CallToolRequestParams) -> CallToolResult: + return CallToolResult(content=[], structured_content={"x": 1}) + + server = Server("test-server", on_call_tool=on_call_tool) + async with Client(server, validate_tool_results=False) as client: + result = await client.call_tool("t", {}) + assert result.structured_content == {"x": 1} + + @pytest.mark.anyio async def test_validate_tool_result_recompiles_when_the_server_changes_the_schema() -> None: """A relisted tool must not be validated against the schema it used to declare."""