Skip to content

Commit 3507ce7

Browse files
committed
Address review: spec-correct x-mcp-header validation, drop server filter, typed Context params, strip-IIR schema derivation
Six review comments addressed in one pass: - Drop the server-side x-mcp-header tools/list filter (_drop_invalid_header_tools). The spec MUST is on the client only; the filter ran on one transport and masked author bugs inconsistently. ClientSession.list_tools carries the requirement. - find_invalid_x_mcp_header: walk nested schemas per the spec's 'statically reachable via a chain of properties keys' rule — an annotation under items/anyOf/allOf/oneOf/not/if/then/else/$ref/$defs/patternProperties makes the whole tool invalid, and uniqueness is over all annotations in the schema. Iterative walk over JSON Schema 2020-12 schema positions; instance- data keywords and $ref are not entered so termination is structural. Also drop 'number' from the admitted types (spec names integer/string/boolean only; conformance#344 tracks the harness's stale wording). - Context.input_responses/.request_state: read from the typed InputResponseRequestParams the runner already parsed instead of re-spelling wire aliases against the raw dict via a parallel TypeAdapter. - func_metadata: strip InputRequiredResult arms from a union return annotation and derive on the residual, so 'SomeModel | InputRequiredResult' keeps its output schema and 'CallToolResult | str | InputRequiredResult' raises InvalidSignature consistently with the two-arm case. One noqa:UP007 on the runtime Union[tuple] rebuild — PEP 604 has no runtime-sequence syntax.
1 parent eb5519d commit 3507ce7

8 files changed

Lines changed: 231 additions & 125 deletions

File tree

src/mcp/server/_streamable_http_modern.py

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@
4545
ERROR_CODE_HTTP_STATUS,
4646
InboundLadderRejection,
4747
classify_inbound_request,
48-
find_invalid_x_mcp_header,
4948
)
5049
from mcp.shared.jsonrpc_dispatcher import handler_exception_to_error_data
5150
from mcp.shared.message import MessageMetadata, ServerMessageMetadata
@@ -127,30 +126,6 @@ async def _to_jsonrpc_response(
127126
return JSONRPCResponse(jsonrpc="2.0", id=request_id, result=result)
128127

129128

130-
def _drop_invalid_header_tools(result: dict[str, Any]) -> None:
131-
"""Remove tools whose `x-mcp-header` annotations fail validation from a `tools/list` result, in place.
132-
133-
Runs at the modern HTTP boundary so the version gate is structural (this
134-
entry only ever serves a 2026-07-28+ request) rather than a conditional in
135-
handler code. Dropped tools are logged so a server author sees why their
136-
tool vanished from the wire.
137-
"""
138-
match result.get("tools"):
139-
case [*tools]:
140-
pass
141-
case _:
142-
return
143-
kept: list[Any] = []
144-
for tool in tools:
145-
match tool:
146-
case {"inputSchema": schema, "name": name} if reason := find_invalid_x_mcp_header(schema):
147-
logger.warning("dropping tool %r from tools/list: %s", name, reason)
148-
case _:
149-
kept.append(tool)
150-
if len(kept) != len(tools):
151-
result["tools"] = kept
152-
153-
154129
async def _write(
155130
msg: JSONRPCResponse | JSONRPCError,
156131
scope: Scope,
@@ -248,6 +223,4 @@ async def handle_modern_request(
248223
msg = await _to_jsonrpc_response(
249224
req.id, serve_one(app, dctx, req.method, req.params, connection=connection, lifespan_state=lifespan_state)
250225
)
251-
if req.method == "tools/list" and isinstance(msg, JSONRPCResponse):
252-
_drop_invalid_header_tools(msg.result)
253226
await _write(msg, scope, receive, send)

src/mcp/server/mcpserver/context.py

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
from collections.abc import Iterable
44
from typing import TYPE_CHECKING, Any, Generic
55

6-
from mcp_types import ClientCapabilities, InputResponses, LoggingLevel
7-
from pydantic import AnyUrl, BaseModel, TypeAdapter
6+
from mcp_types import ClientCapabilities, InputResponseRequestParams, InputResponses, LoggingLevel
7+
from pydantic import AnyUrl, BaseModel
88
from typing_extensions import deprecated
99

1010
from mcp.server.context import LifespanContextT, RequestT, ServerRequestContext
@@ -18,8 +18,6 @@
1818
from mcp.server.lowlevel.helper_types import ReadResourceContents
1919
from mcp.shared.exceptions import MCPDeprecationWarning
2020

21-
_INPUT_RESPONSES_ADAPTER: TypeAdapter[InputResponses] = TypeAdapter(InputResponses)
22-
2321
if TYPE_CHECKING:
2422
from mcp.server.mcpserver.server import MCPServer
2523

@@ -60,19 +58,22 @@ async def my_tool(x: int, ctx: Context) -> str:
6058

6159
_request_context: ServerRequestContext[LifespanContextT, RequestT] | None
6260
_mcp_server: MCPServer | None
61+
_input_params: InputResponseRequestParams | None
6362

6463
# TODO(maxisbey): Consider making request_context/mcp_server required, or refactor Context entirely.
6564
def __init__(
6665
self,
6766
*,
6867
request_context: ServerRequestContext[LifespanContextT, RequestT] | None = None,
6968
mcp_server: MCPServer | None = None,
69+
input_params: InputResponseRequestParams | None = None,
7070
# TODO(Marcelo): We should drop this kwargs parameter.
7171
**kwargs: Any,
7272
):
7373
super().__init__(**kwargs)
7474
self._request_context = request_context
7575
self._mcp_server = mcp_server
76+
self._input_params = input_params
7677

7778
@property
7879
def mcp_server(self) -> MCPServer:
@@ -226,20 +227,17 @@ def input_responses(self) -> InputResponses | None:
226227
"""Client responses to a prior `InputRequiredResult.input_requests`.
227228
228229
`None` on the initial round, or when the client retried without
229-
responses. Values are parsed into the typed result models.
230+
responses.
230231
"""
231-
params = self.request_context.params
232-
raw = params.get("inputResponses") if params else None
233-
return None if raw is None else _INPUT_RESPONSES_ADAPTER.validate_python(raw)
232+
return self._input_params.input_responses if self._input_params else None
234233

235234
@property
236235
def request_state(self) -> str | None:
237236
"""Opaque state echoed from a prior `InputRequiredResult.request_state`.
238237
239238
`None` on the initial round.
240239
"""
241-
params = self.request_context.params
242-
return params.get("requestState") if params else None
240+
return self._input_params.request_state if self._input_params else None
243241

244242
@property
245243
def client_capabilities(self) -> ClientCapabilities | None:

src/mcp/server/mcpserver/server.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ async def _handle_list_tools(
308308
async def _handle_call_tool(
309309
self, ctx: ServerRequestContext[LifespanResultT], params: CallToolRequestParams
310310
) -> CallToolResult | InputRequiredResult:
311-
context = Context(request_context=ctx, mcp_server=self)
311+
context = Context(request_context=ctx, mcp_server=self, input_params=params)
312312
try:
313313
return await self.call_tool(params.name, params.arguments or {}, context)
314314
except MCPError:
@@ -324,7 +324,7 @@ async def _handle_list_resources(
324324
async def _handle_read_resource(
325325
self, ctx: ServerRequestContext[LifespanResultT], params: ReadResourceRequestParams
326326
) -> ReadResourceResult:
327-
context = Context(request_context=ctx, mcp_server=self)
327+
context = Context(request_context=ctx, mcp_server=self, input_params=params)
328328
try:
329329
results = await self.read_resource(params.uri, context)
330330
except ResourceNotFoundError as err:
@@ -366,7 +366,7 @@ async def _handle_list_prompts(
366366
async def _handle_get_prompt(
367367
self, ctx: ServerRequestContext[LifespanResultT], params: GetPromptRequestParams
368368
) -> GetPromptResult:
369-
context = Context(request_context=ctx, mcp_server=self)
369+
context = Context(request_context=ctx, mcp_server=self, input_params=params)
370370
return await self.get_prompt(params.name, params.arguments, context)
371371

372372
async def list_tools(self) -> list[MCPTool]:

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

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
from collections.abc import Awaitable, Callable, Sequence
55
from itertools import chain
66
from types import GenericAlias
7-
from typing import Annotated, Any, cast, get_args, get_origin, get_type_hints
7+
from typing import Annotated, Any, Union, cast, get_args, get_origin, get_type_hints
88

99
import anyio
1010
import anyio.to_thread
@@ -29,6 +29,10 @@
2929
logger = get_logger(__name__)
3030

3131

32+
def _is_input_required_type(obj: Any) -> bool:
33+
return isinstance(obj, type) and issubclass(obj, InputRequiredResult)
34+
35+
3236
class StrictJsonSchema(GenerateJsonSchema):
3337
"""A JSON schema generator that raises exceptions instead of emitting warnings.
3438
@@ -272,19 +276,29 @@ def func_metadata(
272276
# unknown (i.e. a bare `Final`).
273277
assert return_type_expr is not UNKNOWN
274278

275-
if isinstance(return_type_expr, type) and issubclass(return_type_expr, InputRequiredResult):
279+
if _is_input_required_type(return_type_expr):
276280
# A tool annotated to return only InputRequiredResult never produces structured content.
277281
return FuncMetadata(arg_model=arguments_model)
278282

283+
# The annotation fed to schema derivation. Starts as the raw return annotation (preserving any
284+
# Annotated[...] wrapper) and is narrowed below if InputRequiredResult arms are stripped.
285+
effective_annotation: Any = sig.return_annotation
286+
279287
if is_union_origin(get_origin(return_type_expr)):
280288
args = get_args(return_type_expr)
281-
# A union containing InputRequiredResult means the tool may return either a complete
282-
# value or a multi-round input request; treat the complete arm as unstructured (the
283-
# pass-through in convert_result handles the InputRequiredResult arm).
284-
if any(isinstance(arg, type) and issubclass(arg, InputRequiredResult) for arg in args):
289+
# InputRequiredResult is a control-flow signal, not data: strip it so the residual arms
290+
# drive schema derivation. convert_result short-circuits on an InputRequiredResult instance
291+
# before output validation, so the schema only ever sees the data arms at runtime.
292+
residual = tuple(a for a in args if not _is_input_required_type(a))
293+
if not residual:
285294
return FuncMetadata(arg_model=arguments_model)
286-
# Check if CallToolResult appears in the union (excluding None for Optional check)
287-
if any(isinstance(arg, type) and issubclass(arg, CallToolResult) for arg in args if arg is not type(None)):
295+
if len(residual) != len(args):
296+
# PEP 604 has no syntax for "union of a runtime tuple"; Union[...] is the only spelling.
297+
return_type_expr = residual[0] if len(residual) == 1 else Union[residual] # noqa: UP007
298+
effective_annotation = return_type_expr
299+
if len(residual) > 1 and any(
300+
isinstance(a, type) and issubclass(a, CallToolResult) for a in residual if a is not type(None)
301+
):
288302
raise InvalidSignature(
289303
f"Function {func.__name__}: CallToolResult cannot be used in Union or Optional types. "
290304
"To return empty results, use: CallToolResult(content=[])"
@@ -310,7 +324,7 @@ def func_metadata(
310324
else:
311325
return FuncMetadata(arg_model=arguments_model)
312326
else:
313-
original_annotation = sig.return_annotation
327+
original_annotation = effective_annotation
314328

315329
output_model, output_schema, wrap_output = _try_create_model_and_schema(
316330
original_annotation, return_type_expr, func.__name__

src/mcp/shared/inbound.py

Lines changed: 78 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
import base64
1515
import binascii
1616
import re
17-
from collections.abc import Mapping, Sequence
17+
from collections.abc import Iterator, Mapping, Sequence
1818
from dataclasses import dataclass
1919
from types import MappingProxyType
2020
from typing import Any, Final, cast
@@ -81,10 +81,63 @@
8181
_HEADER_SAFE = re.compile(r"^[\x20-\x7E]*$")
8282
# RFC 9110 §5.6.2 token: the only characters permitted in an HTTP field name.
8383
_RFC9110_TOKEN = re.compile(r"^[!#$%&'*+\-.^_`|~0-9A-Za-z]+$")
84-
# JSON-Schema types that stringify cleanly into a single header value. The spec
85-
# names string/integer/boolean; number is admitted because the conformance
86-
# harness emits it and float→str round-trips to within tolerance.
87-
_X_MCP_HEADER_PRIMITIVE_TYPES: Final = frozenset({"string", "integer", "boolean", "number"})
84+
# JSON-Schema types the spec permits to carry `x-mcp-header` (transports.mdx
85+
# §Custom Headers). `number` is explicitly forbidden — float→str is not
86+
# portable across implementations.
87+
_X_MCP_HEADER_PRIMITIVE_TYPES: Final = frozenset({"string", "integer", "boolean"})
88+
89+
# JSON Schema 2020-12 applicator keywords whose values are themselves schema
90+
# positions, grouped by value shape. `properties` is handled separately as the
91+
# only keyword that preserves the statically-reachable chain; every keyword
92+
# here drops the chain to None. Instance-data keywords (`default`, `examples`,
93+
# `const`, `enum`) and `$ref`/`$dynamicRef` are deliberately absent so the
94+
# walk never mistakes data for an annotation and never dereferences.
95+
_SUBSCHEMA_SINGLE: Final = frozenset(
96+
{
97+
"items",
98+
"contains",
99+
"unevaluatedItems",
100+
"additionalProperties",
101+
"propertyNames",
102+
"unevaluatedProperties",
103+
"not",
104+
"if",
105+
"then",
106+
"else",
107+
"contentSchema",
108+
}
109+
)
110+
_SUBSCHEMA_LIST: Final = frozenset({"allOf", "anyOf", "oneOf", "prefixItems"})
111+
_SUBSCHEMA_MAP: Final = frozenset({"patternProperties", "dependentSchemas", "$defs", "definitions"})
112+
113+
114+
def _walk_schema_positions(root: Any) -> Iterator[tuple[tuple[str, ...] | None, dict[str, Any]]]:
115+
"""Yield `(properties_path, schema)` for every schema position in `root`.
116+
117+
`properties_path` is the chain of `properties` keys from the root to the
118+
position, or `None` once any other applicator keyword has been crossed.
119+
The root itself yields `()`. Only the JSON Schema 2020-12 applicators
120+
listed above are entered; instance-data keywords are not, and `$ref` is
121+
not dereferenced, so the walk terminates on any finite JSON value. An
122+
explicit stack keeps the function total even on pathologically deep input.
123+
"""
124+
stack: list[tuple[tuple[str, ...] | None, Any]] = [((), root)]
125+
while stack:
126+
path, node = stack.pop()
127+
if not isinstance(node, dict):
128+
continue
129+
schema = cast(dict[str, Any], node)
130+
yield path, schema
131+
for kw, val in schema.items():
132+
if kw == "properties" and isinstance(val, dict):
133+
for name, sub in cast(dict[str, Any], val).items():
134+
stack.append(((*path, name) if path is not None else None, sub))
135+
elif kw in _SUBSCHEMA_SINGLE:
136+
stack.append((None, val))
137+
elif kw in _SUBSCHEMA_LIST and isinstance(val, list):
138+
stack.extend((None, sub) for sub in cast(list[Any], val))
139+
elif kw in _SUBSCHEMA_MAP and isinstance(val, dict):
140+
stack.extend((None, sub) for sub in cast(dict[str, Any], val).values())
88141

89142

90143
def encode_header_value(value: str) -> str:
@@ -123,31 +176,33 @@ def decode_header_value(value: str | None) -> str | None:
123176
def find_invalid_x_mcp_header(input_schema: Any) -> str | None:
124177
"""Return a reason string if any `x-mcp-header` annotation in `input_schema` is invalid; else `None`.
125178
126-
The spec restricts the annotation to top-level primitive properties whose
127-
header name is a non-empty RFC 9110 token unique (case-insensitively) within
128-
the schema. A `None` / non-object / property-less schema has nothing to
129-
validate and returns `None`.
179+
Walks every JSON Schema 2020-12 schema position. An annotation is valid
180+
only when it sits on a property statically reachable from the root via a
181+
chain of pure `properties` keys, names a non-empty RFC 9110 token, is on
182+
an integer/string/boolean property, and is case-insensitively unique
183+
across the whole schema. A `None` / non-mapping schema has no schema
184+
positions and returns `None`.
130185
"""
131-
match input_schema:
132-
case {"properties": {**properties}}:
133-
pass
134-
case _:
135-
return None
136186
seen: dict[str, str] = {}
137-
for prop_name, raw in properties.items():
138-
if not isinstance(raw, dict) or X_MCP_HEADER_KEY not in raw:
187+
for path, schema in _walk_schema_positions(input_schema):
188+
if X_MCP_HEADER_KEY not in schema:
139189
continue
140-
prop_schema = cast(dict[str, Any], raw)
141-
header = prop_schema[X_MCP_HEADER_KEY]
190+
if not path: # None (off the pure-properties chain) or () (the root itself)
191+
return f"{X_MCP_HEADER_KEY} found at a schema position not reachable via a pure `properties` chain"
192+
where = ".".join(path)
193+
header = schema[X_MCP_HEADER_KEY]
142194
if not isinstance(header, str) or not _RFC9110_TOKEN.fullmatch(header):
143-
return f"property {prop_name!r}: {X_MCP_HEADER_KEY} {header!r} is not an RFC 9110 token"
144-
prop_type = prop_schema.get("type")
195+
return f"property {where!r}: {X_MCP_HEADER_KEY} {header!r} is not an RFC 9110 token"
196+
prop_type = schema.get("type")
145197
if not isinstance(prop_type, str) or prop_type not in _X_MCP_HEADER_PRIMITIVE_TYPES:
146-
return f"property {prop_name!r}: {X_MCP_HEADER_KEY} is only permitted on primitive-typed properties"
198+
return (
199+
f"property {where!r}: {X_MCP_HEADER_KEY} is only permitted on "
200+
f"integer/string/boolean properties (got {prop_type!r})"
201+
)
147202
lower = header.lower()
148203
if lower in seen:
149-
return f"{X_MCP_HEADER_KEY} {header!r} on property {prop_name!r} duplicates property {seen[lower]!r}"
150-
seen[lower] = prop_name
204+
return f"{X_MCP_HEADER_KEY} {header!r} on property {where!r} duplicates property {seen[lower]!r}"
205+
seen[lower] = where
151206
return None
152207

153208

0 commit comments

Comments
 (0)