Skip to content

Commit 414caef

Browse files
committed
Harden claim and binding construction invariants
Reject an empty claim sequence at the session constructor: the ad-filter would otherwise treat the identifier as claim-bearing and silently drop it from the capability ad at every version, leaving the invariant to caller discipline. Validate ResultClaim.method against the closed verb set at construction so an unchecked runtime value cannot fold into tools/call parsing. Create notification binding queues before the dispatcher starts so the enqueue path indexes a complete dict by construction rather than by scheduling order. Copy the extensions ad dict at the constructor boundary. Pins added: modern re-adoption after legacy reactivates claims; a legacy-version discover probe drops claim-bearing identifiers from its ad.
1 parent c73a27f commit 414caef

4 files changed

Lines changed: 75 additions & 4 deletions

File tree

src/mcp/client/extension.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
from collections.abc import Awaitable, Callable, Sequence
1414
from dataclasses import dataclass
15-
from typing import TYPE_CHECKING, Any, Generic, Literal, TypeVar, get_args
15+
from typing import TYPE_CHECKING, Any, Final, Generic, Literal, TypeVar, get_args
1616

1717
from mcp_types import CORE_RESULT_TYPES, CallToolResult, InputRequiredResult, Result
1818
from mcp_types.version import MODERN_PROTOCOL_VERSIONS
@@ -32,6 +32,9 @@
3232
"advertise",
3333
]
3434

35+
_CLAIM_METHODS: Final[frozenset[str]] = frozenset({"tools/call"})
36+
"""The closed set of verbs a claim may attach to (widened with the `method` Literal)."""
37+
3538
ClaimedT = TypeVar("ClaimedT", bound=Result)
3639
NotifyParamsT = TypeVar("NotifyParamsT", bound=BaseModel)
3740

@@ -82,6 +85,8 @@ class ResultClaim(Generic[ClaimedT]):
8285
protocol_versions: frozenset[str] | None = None
8386

8487
def __post_init__(self) -> None:
88+
if self.method not in _CLAIM_METHODS:
89+
raise ValueError(f"claims attach to {sorted(_CLAIM_METHODS)} only; got method {self.method!r}")
8590
if self.result_type in CORE_RESULT_TYPES:
8691
raise ValueError(f"resultType {self.result_type!r} is core protocol vocabulary")
8792
if issubclass(self.model, CallToolResult | InputRequiredResult):

src/mcp/client/session.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,11 @@ def _index_claims(
277277
f"result_claims key {identifier!r} has no extensions entry; a claim is only "
278278
"advertised through its extension's capability ad"
279279
)
280+
if not claims:
281+
raise ValueError(
282+
f"result_claims[{identifier!r}] is empty; an empty claim set would drop the "
283+
"extension from the capability ad at every version — omit the key instead"
284+
)
280285
for claim in claims:
281286
key = (claim.method, claim.result_type)
282287
if key in seen:
@@ -344,7 +349,7 @@ def __init__(
344349
self._client_info = client_info or DEFAULT_CLIENT_INFO
345350
self._sampling_callback = sampling_callback or _default_sampling_callback
346351
self._sampling_capabilities = sampling_capabilities
347-
self._extensions = extensions
352+
self._extensions = dict(extensions) if extensions is not None else None
348353
self._result_claims = _index_claims(result_claims, extensions)
349354
self._notification_bindings = _index_bindings(notification_bindings)
350355
self._active_claims: dict[str, ResultClaim[Any]] = {}
@@ -387,10 +392,14 @@ async def __aenter__(self) -> Self:
387392
self._task_group = anyio.create_task_group()
388393
await self._task_group.__aenter__()
389394
try:
390-
await self._task_group.start(self._dispatcher.run, self._on_request, self._on_notify)
395+
# Queues exist before the dispatcher can deliver: _on_notify may run as
396+
# soon as the dispatcher starts, and its enqueue indexes this dict.
391397
for binding in self._notification_bindings.values():
392398
send, receive = anyio.create_memory_object_stream[BaseModel](_NOTIFICATION_QUEUE_SIZE)
393399
self._binding_queues[binding.method] = (send, receive)
400+
await self._task_group.start(self._dispatcher.run, self._on_request, self._on_notify)
401+
for binding in self._notification_bindings.values():
402+
_, receive = self._binding_queues[binding.method]
394403
self._task_group.start_soon(self._deliver_bound_notifications, binding, receive)
395404
except BaseException:
396405
# Unwind the entered task group before propagating: a cancellation

tests/client/test_extension.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
"""
88

99
from dataclasses import FrozenInstanceError
10-
from typing import Any, Literal
10+
from typing import Any, Literal, cast
1111

1212
import pytest
1313
from inline_snapshot import snapshot
@@ -142,6 +142,15 @@ def test_claim_rejects_mismatched_result_type_literal() -> None:
142142
assert str(exc_info.value) == snapshot("_OtherTagResult.result_type must be Literal['task']")
143143

144144

145+
def test_claim_rejects_method_outside_the_closed_verb_set() -> None:
146+
"""SDK-defined: claims attach to `tools/call` only (the Literal is the static gate);
147+
an unchecked runtime value must not fold into tools/call parsing silently."""
148+
with pytest.raises(ValueError) as exc_info:
149+
_claim(method=cast("Literal['tools/call']", "prompts/get"))
150+
151+
assert str(exc_info.value) == snapshot("claims attach to ['tools/call'] only; got method 'prompts/get'")
152+
153+
145154
def test_claim_rejects_empty_protocol_versions() -> None:
146155
"""SDK-defined: an empty version set could never activate; `None` is the
147156
spelling for "every modern version"."""

tests/client/test_session_claims.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,19 @@ def test_claims_keyed_to_unadvertised_extension_rejected() -> None:
166166
)
167167

168168

169+
def test_empty_claim_sequence_rejected() -> None:
170+
"""SDK-defined: an empty claim set would make the ad-filter treat the identifier as
171+
claim-bearing and drop it from the capability ad at every version; "claim-less" and
172+
"advertises everywhere" stay the same thing by rejecting the empty spelling."""
173+
with pytest.raises(ValueError) as exc_info:
174+
ClientSession(dispatcher=_RecordingDispatcher(), extensions={_TASKS_EXT: {}}, result_claims={_TASKS_EXT: []})
175+
176+
assert str(exc_info.value) == snapshot(
177+
"result_claims['com.example/tasks'] is empty; an empty claim set would drop the "
178+
"extension from the capability ad at every version — omit the key instead"
179+
)
180+
181+
169182
def test_empty_settings_count_as_an_advertised_extension() -> None:
170183
"""SDK-defined: an extension advertised with empty settings ({}) is still an ad —
171184
claims keyed to it construct fine."""
@@ -228,6 +241,25 @@ async def test_legacy_adopt_clears_active_claims() -> None:
228241
assert dispatcher.calls[-1][0] == "tools/call"
229242

230243

244+
@pytest.mark.anyio
245+
async def test_modern_readopt_after_legacy_reactivates_claims() -> None:
246+
"""SDK-defined: adoption is re-entrant in both directions — after modern→legacy→
247+
modern the claims are active again and the adopt-built adapter routes claimed raws."""
248+
dispatcher = _RecordingDispatcher(tool_result=_CLAIMED_TASK_RESULT)
249+
session = _claims_session(dispatcher, _task_claim())
250+
with anyio.fail_after(5):
251+
async with session:
252+
_adopt_modern(session)
253+
_adopt_handshake(session)
254+
assert session._call_tool_adapter is _CallToolResultAdapter
255+
256+
_adopt_modern(session)
257+
result = await session.call_tool("t", {}, allow_claimed=True)
258+
259+
assert isinstance(result, _TaskResult)
260+
assert session._call_tool_adapter is not _CallToolResultAdapter
261+
262+
231263
# ── The version-aware capability ad ─────────────────────────────────────────
232264

233265

@@ -303,6 +335,22 @@ async def test_discover_probe_ad_includes_claim_identifiers_at_the_probe_version
303335
assert capabilities["extensions"] == {_TASKS_EXT: {}}
304336

305337

338+
@pytest.mark.anyio
339+
async def test_discover_probe_ad_drops_claim_identifiers_at_a_legacy_probe_version() -> None:
340+
"""SDK-defined: a lowlevel `send_discover` at a non-modern version string builds an
341+
ad where no claim can be active, so the claim-bearing identifier drops coherently."""
342+
dispatcher = _RecordingDispatcher()
343+
session = _claims_session(dispatcher, _task_claim())
344+
with anyio.fail_after(5):
345+
async with session:
346+
await session.send_discover(LATEST_HANDSHAKE_VERSION)
347+
348+
[(_, params, _)] = dispatcher.calls
349+
assert params is not None
350+
capabilities = params["_meta"][CLIENT_CAPABILITIES_META_KEY]
351+
assert "extensions" not in capabilities
352+
353+
306354
# ── Routing through the adopt-built adapter ─────────────────────────────────
307355

308356

0 commit comments

Comments
 (0)