Skip to content

Commit 1084616

Browse files
committed
Refine extension fold errors and pin read-once for all declarations
A Mapping passed as extensions= gets a migration error naming advertise() instead of an attribute error about str; a self-conflicting extension reads as one owner instead of "extensions 'a' and 'a'". Pins added: claims() and notifications() are read exactly once like settings(), and a claimed shape routes to its owning extension's resolver when two claim-bearing extensions are registered.
1 parent 0f85f31 commit 1084616

2 files changed

Lines changed: 123 additions & 12 deletions

File tree

src/mcp/client/client.py

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,11 @@ def _fold_extensions(extensions: Sequence[ClientExtension] | None) -> _FoldedExt
213213
"""
214214
if not extensions:
215215
return _FoldedExtensions(ad=None, claims=None, bindings=None, by_model={})
216+
if isinstance(extensions, Mapping):
217+
raise TypeError(
218+
"extensions= takes a sequence of ClientExtension instances; the mapping form was "
219+
"replaced — use advertise(identifier, settings) for advertise-only entries"
220+
)
216221
ad: dict[str, dict[str, Any]] = {}
217222
claims: dict[str, tuple[ResultClaim[Any], ...]] = {}
218223
bindings: list[NotificationBinding[Any]] = []
@@ -234,9 +239,14 @@ def _fold_extensions(extensions: Sequence[ClientExtension] | None) -> _FoldedExt
234239
for claim in extension_claims:
235240
key = (claim.method, claim.result_type)
236241
if key in claim_owners:
242+
owner = claim_owners[key]
243+
both = (
244+
f"extension {identifier!r} claims"
245+
if owner == identifier
246+
else (f"extensions {owner!r} and {identifier!r} both claim")
247+
)
237248
raise ValueError(
238-
f"extensions {claim_owners[key]!r} and {identifier!r} both claim {claim.method!r} "
239-
f"resultType {claim.result_type!r}; a wire tag can have only one resolver"
249+
f"{both} {claim.method!r} resultType {claim.result_type!r}; a wire tag can have only one resolver"
240250
)
241251
claim_owners[key] = identifier
242252
# Collision-free by construction: a model's `result_type` Literal pins it to
@@ -246,10 +256,13 @@ def _fold_extensions(extensions: Sequence[ClientExtension] | None) -> _FoldedExt
246256
claims[identifier] = extension_claims
247257
for binding in extension.notifications():
248258
if binding.method in binding_owners:
249-
raise ValueError(
250-
f"extensions {binding_owners[binding.method]!r} and {identifier!r} both bind "
251-
f"notification method {binding.method!r}; a method can have only one observer"
259+
owner = binding_owners[binding.method]
260+
both = (
261+
f"extension {identifier!r} binds"
262+
if owner == identifier
263+
else (f"extensions {owner!r} and {identifier!r} both bind")
252264
)
265+
raise ValueError(f"{both} notification method {binding.method!r}; a method can have only one observer")
253266
binding_owners[binding.method] = identifier
254267
bindings.append(binding)
255268
return _FoldedExtensions(ad=ad, claims=claims or None, bindings=tuple(bindings) or None, by_model=by_model)

tests/client/test_client_extensions.py

Lines changed: 105 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
import logging
1616
from collections.abc import Awaitable, Callable, Sequence
17-
from typing import Any, Literal
17+
from typing import Any, Literal, cast
1818

1919
import anyio
2020
import mcp_types as types
@@ -133,6 +133,64 @@ def add(a: int, b: int) -> int:
133133
# ── Construction-time validation ────────────────────────────────────────────
134134

135135

136+
class _CouponResult(Result):
137+
"""A second claimed shape with its own tag, for multi-claim routing."""
138+
139+
result_type: Literal["coupon"] = "coupon"
140+
141+
142+
async def _unreachable_coupon_resolve(claimed: _CouponResult, ctx: ClaimContext) -> CallToolResult:
143+
raise NotImplementedError # the wrong resolver for a voucher — must never run
144+
145+
146+
class _CouponExtension(ClientExtension):
147+
identifier = "com.example/coupons"
148+
149+
def claims(self) -> Sequence[ResultClaim[Any]]:
150+
return [ResultClaim(result_type="coupon", model=_CouponResult, resolve=_unreachable_coupon_resolve)]
151+
152+
153+
class _SelfConflictingClaims(ClientExtension):
154+
identifier = "com.example/twice"
155+
156+
def claims(self) -> Sequence[ResultClaim[Any]]:
157+
return [
158+
ResultClaim(result_type="twice", model=_TwiceResult, resolve=_unreachable_twice_resolve),
159+
ResultClaim(result_type="twice", model=_TwiceResult, resolve=_unreachable_twice_resolve),
160+
]
161+
162+
163+
class _TwiceResult(Result):
164+
result_type: Literal["twice"] = "twice"
165+
166+
167+
async def _unreachable_twice_resolve(claimed: _TwiceResult, ctx: ClaimContext) -> CallToolResult:
168+
raise NotImplementedError
169+
170+
171+
def test_mapping_extensions_get_the_migration_error() -> None:
172+
"""SDK-defined: the replaced dict form fails with a message naming the new shape,
173+
not an attribute error about `str`."""
174+
with pytest.raises(TypeError) as exc_info:
175+
Client(_add_server(), extensions=cast("Sequence[ClientExtension]", {"com.example/ui": {}}))
176+
177+
assert str(exc_info.value) == snapshot(
178+
"extensions= takes a sequence of ClientExtension instances; the mapping form was "
179+
"replaced — use advertise(identifier, settings) for advertise-only entries"
180+
)
181+
182+
183+
def test_one_extension_claiming_a_tag_twice_reads_as_one_owner() -> None:
184+
"""SDK-defined: a self-conflict names the one extension once instead of
185+
"extensions 'a' and 'a'"."""
186+
with pytest.raises(ValueError) as exc_info:
187+
Client(_add_server(), extensions=[_SelfConflictingClaims()])
188+
189+
assert str(exc_info.value) == snapshot(
190+
"extension 'com.example/twice' claims 'tools/call' resultType 'twice'; a wire tag can have only one resolver"
191+
)
192+
193+
136194
def test_bare_extension_instance_is_rejected_with_the_fix_named() -> None:
137195
"""SDK-defined: an instance whose class never set `identifier` fails Client
138196
construction with an error naming the type and the fix — not an AttributeError."""
@@ -246,32 +304,54 @@ def test_conflicting_notification_bindings_name_both_owners() -> None:
246304
# ── settings() consumption ───────────────────────────────────────────────────
247305

248306

307+
class _CountedResult(Result):
308+
result_type: Literal["counted"] = "counted"
309+
310+
311+
async def _unreachable_counted_resolve(claimed: _CountedResult, ctx: ClaimContext) -> CallToolResult:
312+
raise NotImplementedError # never driven; exists so claims() has something to return
313+
314+
249315
class _CountingSettings(ClientExtension):
250-
"""Counts `settings()` reads to pin the read-once contract."""
316+
"""Counts every declaration read to pin the read-once contract for all three."""
251317

252318
identifier = "com.example/counted"
253319

254320
def __init__(self) -> None:
255321
self.reads = 0
322+
self.claims_reads = 0
323+
self.notifications_reads = 0
256324

257325
def settings(self) -> dict[str, Any]:
258326
self.reads += 1
259327
return {"read": self.reads}
260328

329+
def claims(self) -> Sequence[ResultClaim[Any]]:
330+
self.claims_reads += 1
331+
return [ResultClaim(result_type="counted", model=_CountedResult, resolve=_unreachable_counted_resolve)]
332+
333+
def notifications(self) -> Sequence[NotificationBinding[Any]]:
334+
self.notifications_reads += 1
335+
return [
336+
NotificationBinding(method="notifications/counted", params_type=_EventParams, handler=_unreachable_handler)
337+
]
338+
261339

262-
async def test_settings_is_read_exactly_once_at_construction() -> None:
263-
"""SDK-defined: `settings()` is read once, at Client construction — connecting and
264-
calling tools (each modern request re-stamps the capability ad) never re-reads it."""
340+
async def test_declarations_are_read_exactly_once_at_construction() -> None:
341+
"""SDK-defined: `settings()`, `claims()`, and `notifications()` are each read once,
342+
at Client construction — connecting and calling tools (each modern request re-stamps
343+
the capability ad) never re-reads any of them, so a stateful extension cannot desync
344+
the ad from the claims."""
265345
extension = _CountingSettings()
266346
client = Client(_add_server(), extensions=[extension])
267-
assert extension.reads == 1
347+
assert (extension.reads, extension.claims_reads, extension.notifications_reads) == (1, 1, 1)
268348

269349
with anyio.fail_after(5):
270350
async with client:
271351
await client.call_tool("add", {"a": 1, "b": 2})
272352
await client.call_tool("add", {"a": 3, "b": 4})
273353

274-
assert extension.reads == 1
354+
assert (extension.reads, extension.claims_reads, extension.notifications_reads) == (1, 1, 1)
275355

276356

277357
async def test_settings_dict_is_held_by_reference_not_copied() -> None:
@@ -348,6 +428,24 @@ async def resolve(claimed: VoucherResult, ctx: ClaimContext) -> CallToolResult:
348428
assert result.content == [TextContent(text="honored v-42")]
349429

350430

431+
async def test_claimed_shape_routes_to_its_owning_extensions_resolver() -> None:
432+
"""With two claim-bearing extensions registered, the parsed shape runs ITS owner's
433+
resolver — the coupon extension (registered first) must never see a voucher."""
434+
received: list[VoucherResult] = []
435+
436+
async def resolve(claimed: VoucherResult, ctx: ClaimContext) -> CallToolResult:
437+
received.append(claimed)
438+
return CallToolResult(content=[TextContent(text="routed")])
439+
440+
extensions = [_CouponExtension(), _VoucherExtension(resolve)]
441+
with anyio.fail_after(5):
442+
async with Client(_voucher_server(), extensions=extensions) as client:
443+
result = await client.call_tool("issue", {})
444+
445+
assert [claimed.request_state for claimed in received] == ["v-42"]
446+
assert result.content == [TextContent(text="routed")]
447+
448+
351449
async def test_resolver_product_gets_the_direct_paths_output_schema_revalidation() -> None:
352450
"""The resolver's product passes through `validate_tool_result` exactly like a
353451
directly-returned result: against the tool's output schema, missing structured

0 commit comments

Comments
 (0)