Skip to content

Commit 833a41f

Browse files
Sainikhil Juluriclaude
andcommitted
fix(mcpserver): omit unreturned TypedDict keys from structuredContent
A tool annotated to return a TypedDict with `NotRequired` keys (or `total=False`) published an outputSchema declaring those properties non-nullable, while `convert_result` materialised every key the tool did not return as an explicit `null`. The response therefore violated the schema the same server had just advertised, and a conforming client -- the SDK's own included -- rejected the call: RuntimeError: Invalid structured content returned by tool get_person: None is not of type 'string' That made every such tool unusable, even though TypedDict is one of the return shapes structured output documents. The unstructured text block and structuredContent also disagreed, so a client skipping validation saw two different answers. `_create_model_from_typeddict` gives non-required keys a `None` default to make them optional on the generated model; the file's own comment already noted that such a model "should use exclude_unset=True when dumping to get TypedDict semantics", but nothing did. Mark those models with a private base class and honour it in `convert_result`, so an omitted key stays omitted. Scoped deliberately to TypedDict-derived models: `exclude_unset` applied to every output shape would change payloads for BaseModel, dataclass and wrapped returns, where emitting defaults is correct. The published outputSchema is unchanged, so no existing expectation moves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent a4f4ccd commit 833a41f

2 files changed

Lines changed: 81 additions & 4 deletions

File tree

src/mcp/server/mcpserver/utilities/func_metadata.py

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,11 @@ def convert_result(self, result: Any) -> CallToolResult | InputRequiredResult:
139139

140140
assert self.output_model is not None, "Output model must be set if output schema is defined"
141141
validated = self.output_model.model_validate(result)
142-
structured_content = validated.model_dump(mode="json", by_alias=True)
142+
# A TypedDict's non-required keys are absent, not null, when the tool omits
143+
# them -- and the published outputSchema declares them non-nullable, so
144+
# emitting nulls would violate the server's own schema.
145+
exclude_unset = issubclass(self.output_model, _TypedDictOutputModel)
146+
structured_content = validated.model_dump(mode="json", by_alias=True, exclude_unset=exclude_unset)
143147

144148
return CallToolResult(content=unstructured_content, structured_content=structured_content)
145149

@@ -494,6 +498,17 @@ def _create_model_from_class(cls: type[Any], type_hints: dict[str, Any]) -> type
494498
return create_model(cls.__name__, __config__=ConfigDict(from_attributes=True), **model_fields)
495499

496500

501+
class _TypedDictOutputModel(BaseModel):
502+
"""Base for output models generated from a TypedDict return annotation.
503+
504+
Non-required TypedDict keys are given a `None` default so they are optional on
505+
the generated model, but `None` is not a valid value for their declared type and
506+
the published `outputSchema` does not accept it. Dumping such a model has to
507+
honour TypedDict semantics -- a key the tool did not return stays absent rather
508+
than becoming an explicit null -- so `convert_result` keys off this base class.
509+
"""
510+
511+
497512
def _create_model_from_typeddict(td_type: type[Any]) -> type[BaseModel]:
498513
"""Create a Pydantic model from a TypedDict.
499514
@@ -507,12 +522,13 @@ def _create_model_from_typeddict(td_type: type[Any]) -> type[BaseModel]:
507522
if field_name not in required_keys:
508523
# For optional TypedDict fields, set default=None
509524
# This makes them not required in the Pydantic model
510-
# The model should use exclude_unset=True when dumping to get TypedDict semantics
525+
# Dumped with exclude_unset=True (see `_TypedDictOutputModel`) so an
526+
# omitted key stays omitted instead of serializing as null
511527
model_fields[field_name] = (field_type, None)
512528
else:
513529
model_fields[field_name] = field_type
514530

515-
return create_model(td_type.__name__, **model_fields)
531+
return create_model(td_type.__name__, __base__=_TypedDictOutputModel, **model_fields)
516532

517533

518534
def _create_wrapped_model(func_name: str, annotation: Any) -> type[BaseModel]:

tests/server/mcpserver/test_func_metadata.py

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,10 @@
55
# pyright: reportUnknownLambdaType=false
66
from collections.abc import Callable
77
from dataclasses import dataclass
8-
from typing import Annotated, Any, Final, NamedTuple, TypedDict
8+
from typing import Annotated, Any, Final, NamedTuple, NotRequired, TypedDict
99

1010
import annotated_types
11+
import jsonschema
1112
import pytest
1213
from dirty_equals import IsPartialDict
1314
from mcp_types import CallToolResult, InputRequiredResult
@@ -811,6 +812,66 @@ def func_returning_typeddict_required() -> PersonTypedDictRequired: # pragma: n
811812
}
812813

813814

815+
def test_typeddict_output_omits_keys_the_tool_did_not_return():
816+
"""A non-required TypedDict key the tool omits stays absent from structuredContent.
817+
818+
SDK-defined: those keys carry a `None` default so they are optional on the generated
819+
model, but the published outputSchema declares them non-nullable, so serializing them
820+
as explicit nulls would make every such tool emit content violating the server's own
821+
schema (and a conforming client reject the call).
822+
"""
823+
824+
class Person(TypedDict):
825+
name: str
826+
age: NotRequired[int]
827+
nickname: NotRequired[str]
828+
829+
def get_person() -> Person:
830+
return {"name": "Dave"}
831+
832+
meta = func_metadata(get_person)
833+
# Call the real function rather than restating its return value: the existing
834+
# tests in this file never invoke the tool body, which is why this went unnoticed.
835+
result = meta.convert_result(get_person())
836+
837+
assert isinstance(result, CallToolResult)
838+
assert result.structured_content == {"name": "Dave"}
839+
840+
# The load-bearing assertion: the payload has to satisfy the schema this same
841+
# metadata published, which is the contract a client validates against.
842+
assert meta.output_schema is not None
843+
jsonschema.validate(instance=result.structured_content, schema=meta.output_schema)
844+
845+
# A key the tool DOES return still travels, including a falsy one.
846+
both = meta.convert_result({"name": "Dave", "age": 0})
847+
assert isinstance(both, CallToolResult)
848+
assert both.structured_content == {"name": "Dave", "age": 0}
849+
jsonschema.validate(instance=both.structured_content, schema=meta.output_schema)
850+
851+
852+
def test_total_false_typeddict_output_omits_every_unreturned_key():
853+
"""`total=False` makes every key non-required, so an empty return stays empty.
854+
855+
SDK-defined: pins the degenerate case, where the old behaviour turned `{}` into a
856+
payload of nothing but nulls.
857+
"""
858+
859+
class Partial(TypedDict, total=False):
860+
name: str
861+
age: int
862+
863+
def get_partial() -> Partial:
864+
return {}
865+
866+
meta = func_metadata(get_partial)
867+
result = meta.convert_result(get_partial())
868+
869+
assert isinstance(result, CallToolResult)
870+
assert result.structured_content == {}
871+
assert meta.output_schema is not None
872+
jsonschema.validate(instance=result.structured_content, schema=meta.output_schema)
873+
874+
814875
def test_structured_output_ordinary_class():
815876
"""Test structured output with ordinary annotated classes"""
816877

0 commit comments

Comments
 (0)