From d927762003ef37719abf3152b77c3b5a0510f048 Mon Sep 17 00:00:00 2001 From: Jeremiah Lowin <153965+jlowin@users.noreply.github.com> Date: Mon, 20 Jul 2026 18:21:56 -0400 Subject: [PATCH] Drop forked client protocol helpers in favor of the SDK's (#4574) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Delegate forked client protocol helpers to the SDK Replace FastMCP's copies of _fold_extensions, _evicting_message_handler, and _synthesize_discover with imports from mcp.client.client. The fork had drifted: it was missing validate_extension_identifier, so non-reverse-DNS extension identifiers were silently accepted. Full lifecycle composition over mcp.Client stays blocked upstream — mcp.Client hardcodes ClientSession construction (no session_class hook) and forbids reentry. * Drop duplicate local helpers reintroduced by the merge; use SDK versions --- docs/development/v4-notes/change-register.mdx | 10 +- fastmcp_slim/fastmcp/client/client.py | 125 +----------------- 2 files changed, 16 insertions(+), 119 deletions(-) diff --git a/docs/development/v4-notes/change-register.mdx b/docs/development/v4-notes/change-register.mdx index e698ee679..9a512e50a 100644 --- a/docs/development/v4-notes/change-register.mdx +++ b/docs/development/v4-notes/change-register.mdx @@ -209,7 +209,15 @@ client = Client("https://example.com/mcp", mode="legacy") # opt back into the `fastmcp.Client` now accepts `extensions=` (a sequence of SEP-2133 `ClientExtension` instances) and `result_claims=` (extra `ResultClaim`s keyed by an advertised extension's identifier). Each extension's capability advertisement, result claims, and notification bindings are folded into the underlying `ClientSession` on every transport. User-supplied notification bindings **compose** with FastMCP's internal task-status binding rather than clobbering it: the task binding always leads, and a user extension that binds the same method surfaces a clear duplicate-method error at connect time rather than silently winning. Result claims are wired end-to-end: `call_tool()` / `call_tool_mcp()` pass `allow_claimed=True` and resolve a claimed result through the owning claim's resolver (`ClaimContext`), so a server-emitted claimed shape is finished into an ordinary `CallToolResult` instead of raising `UnexpectedClaimedResult`. Claimed shapes are modern-only, so they are inert on a legacy connection. -*Verify:* `fastmcp_slim/fastmcp/client/client.py` (`_fold_extensions`, `_build_extension_kwargs`, `_resolve_claimed_result`, `new()`), `fastmcp_slim/fastmcp/client/mixins/tools.py` (`call_tool_mcp` claim resolution), `fastmcp_slim/fastmcp/client/transports/base.py` (`SessionKwargs.extensions`/`result_claims`), `tests/client/test_client_extensions.py` (fold, composition, live both-bindings-fire, end-to-end claim resolution). +*Verify:* `fastmcp_slim/fastmcp/client/client.py` (`_build_extension_kwargs`, `_resolve_claimed_result`, `new()`), `fastmcp_slim/fastmcp/client/mixins/tools.py` (`call_tool_mcp` claim resolution), `fastmcp_slim/fastmcp/client/transports/base.py` (`SessionKwargs.extensions`/`result_claims`), `tests/client/test_client_extensions.py` (fold, composition, live both-bindings-fire, end-to-end claim resolution). + +### Protocol helpers delegated to the SDK — Absorbed (internal) + +`fastmcp.Client` carried forked copies of three SDK helpers — `_fold_extensions` (with its `_FoldedExtensions` dataclass), `_evicting_message_handler`, and `_synthesize_discover` — written when the SDK had not yet stabilized them. It now imports the SDK's implementations directly. The forks had already drifted: FastMCP's `_fold_extensions` was missing the SEP-2133 `validate_extension_identifier` check, so a non-reverse-DNS extension identifier that the SDK rejects was silently accepted. Adopting the SDK's version closes that gap. No public surface moves; the SDK returns `None` rather than empty collections for the folded claims and bindings, absorbed at the two call sites in `_build_extension_kwargs`. + +Full composition — `fastmcp.Client` holding an `mcp.Client` and delegating the connection lifecycle to it — remains blocked upstream. `mcp.Client._build_session` hardcodes `ClientSession(...)` with no override hook, but FastMCP's `TransportOptions.session_class` is load-bearing: `ProxyClient` supplies a `_ForwardingClientSession` that skips output-schema validation so a backend's schema bug surfaces at the end client rather than as a proxy error. Separately, `mcp.Client.__aenter__` raises on reentry, while FastMCP's refcounted reentrant context manager is depended on by proxy session reuse. Both would need an upstream `session_factory=` hook (the same shape as the `notification_bindings=` ask that unblocked extension composition) before the lifecycle itself can be delegated. + +*Verify:* `fastmcp_slim/fastmcp/client/client.py` (imports from `mcp.client.client`; no local helper definitions), `fastmcp_slim/fastmcp/client/transports/base.py` (`TransportOptions.session_class`), `fastmcp_slim/fastmcp/server/providers/proxy.py` (`_ForwardingClientSession`, `PROXY_TRANSPORT_OPTIONS`). ### Transports yield 2-tuples — Absorbed diff --git a/fastmcp_slim/fastmcp/client/client.py b/fastmcp_slim/fastmcp/client/client.py index 07f631ee9..c62429f29 100644 --- a/fastmcp_slim/fastmcp/client/client.py +++ b/fastmcp_slim/fastmcp/client/client.py @@ -31,6 +31,11 @@ from mcp.client.caching import ( ClientResponseCache, InMemoryResponseCacheStore, ) +from mcp.client.client import ( + _evicting_message_handler, + _fold_extensions, + _synthesize_discover, +) from mcp.client.extension import ( ClaimContext, ClientExtension, @@ -139,22 +144,6 @@ The ``str`` arm is only for the version-pin case; ``Client.__init__`` rejects an """ -def _synthesize_discover(protocol_version: str) -> mcp_types.DiscoverResult: - """Build a minimal ``DiscoverResult`` for a pinned modern version (no wire probe). - - Mirrors the SDK Client's ``_synthesize_discover``: the version is pinned but the - server identity is unknown, so ``server_info`` is empty. - """ - return mcp_types.DiscoverResult( - supported_versions=[protocol_version], - capabilities=mcp_types.ServerCapabilities(), - server_info=mcp_types.Implementation(name="", version=""), - result_type="complete", - ttl_ms=0, - cache_scope="public", - ) - - @asynccontextmanager async def _conformant_discover_only( session: ClientSession, @@ -234,106 +223,6 @@ class _FoldedExtensions: by_model: dict[type[mcp_types.Result], ResultClaim[Any]] -def _fold_extensions( - extensions: Sequence[ClientExtension] | None, -) -> _FoldedExtensions: - """Decompose `ClientExtension` instances into `ClientSession` kwargs. - - Mirrors the SDK Client's own folding, using only the public `ClientExtension` - surface (`settings()`, `claims()`, `notifications()`). Duplicate identifiers, - result-type tags, or notification methods across extensions raise here rather - than at session construction, naming both owners. `by_model` is the model→claim - index the resolution path uses to finish a claimed result. - """ - folded = _FoldedExtensions(ad={}, claims={}, bindings=[], by_model={}) - if not extensions: - return folded - if isinstance(extensions, Mapping): - raise TypeError( - "extensions= takes a sequence of ClientExtension instances; use " - "mcp.client.advertise(identifier, settings) for an advertise-only entry" - ) - claim_owners: dict[str, str] = {} - binding_owners: dict[str, str] = {} - for extension in extensions: - identifier = getattr(extension, "identifier", None) - if identifier is None: - raise ValueError( - f"{type(extension).__name__} has no `identifier`; a ClientExtension " - "must set the `identifier` class attribute (or assign one in " - "`__init__`) before it can be used" - ) - if identifier in folded.ad: - raise ValueError( - f"extension identifier {identifier!r} is passed more than once" - ) - folded.ad[identifier] = extension.settings() - extension_claims = tuple(extension.claims()) - for claim in extension_claims: - tag = claim.result_type - if tag in claim_owners: - owner = claim_owners[tag] - both = ( - f"extension {identifier!r} claims" - if owner == identifier - else f"extensions {owner!r} and {identifier!r} both claim" - ) - raise ValueError( - f"{both} resultType {tag!r}; a wire tag can have only one resolver" - ) - claim_owners[tag] = identifier - # Each model pins its result_type Literal to one tag, so this cannot collide. - folded.by_model[claim.model] = claim - if extension_claims: - folded.claims[identifier] = extension_claims - for binding in extension.notifications(): - if binding.method in binding_owners: - owner = binding_owners[binding.method] - both = ( - f"extension {identifier!r} binds" - if owner == identifier - else f"extensions {owner!r} and {identifier!r} both bind" - ) - raise ValueError( - f"{both} notification method {binding.method!r}; a method can " - "have only one observer" - ) - binding_owners[binding.method] = identifier - folded.bindings.append(binding) - return folded - - -def _evicting_message_handler( - cache: ClientResponseCache, user_handler: MessageHandlerFnT | None -) -> MessageHandlerFnT: - """Compose cache eviction over an existing message handler (SEP-2549). - - A server notification (tools/list_changed, resource updates, etc.) evicts the - entries it invalidates *before* the wrapped handler runs, so a downstream - consumer never observes a change while a stale cached listing is still served. - Mirrors the SDK Client's `_evicting_message_handler`, but delegates to FastMCP's - own handler chain rather than clobbering it. Eviction faults are contained: a - cache-store error must never block notification delivery. - """ - - async def handler( - message: Any, - ) -> None: - if isinstance(message, mcp_types.ServerNotification): - try: - await cache.evict_for_notification(message) - except Exception: - logger.exception( - "Response cache eviction failed; the notification is still delivered" - ) - if user_handler is not None: - await user_handler(message) - else: - await anyio.lowlevel.checkpoint() - - return handler - - @dataclass class ClientSessionState: """Holds all session-related state for a Client instance. @@ -1370,7 +1259,7 @@ class Client( """ folded = _fold_extensions(self._extensions_arg) - claims: dict[str, tuple[ResultClaim[Any], ...]] = dict(folded.claims) + claims: dict[str, tuple[ResultClaim[Any], ...]] = dict(folded.claims or {}) by_model: dict[type[mcp_types.Result], ResultClaim[Any]] = dict(folded.by_model) for identifier, extra in (self._result_claims_arg or {}).items(): existing = claims.get(identifier, ()) @@ -1383,7 +1272,7 @@ class Client( # The internal task binding must lead so user bindings extend it. "notification_bindings": [ self._task_status_binding(), - *folded.bindings, + *(folded.bindings or ()), ], } if folded.ad: