Extend resolver DI to sampling and roots requests#3049
Conversation
📚 Documentation preview
|
1a12981 to
bc3b145
Compare
There was a problem hiding this comment.
4 issues found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/handlers/dependencies.md">
<violation number="1" location="docs/handlers/dependencies.md:145">
P2: This sentence reads as if missing client capability always yields `-32021`, but the rest of this change set documents that non-sendable pre-2026 sessions still fail with the no-back-channel error first. Adding the same qualifier here would keep the dependency docs consistent and avoid misleading migration expectations for legacy transports.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 7 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| `Client(sampling_capabilities=SamplingCapability(tools=...))`. Direct | ||
| `ctx.elicit()` and `ctx.session.*` calls outside resolvers keep their previous | ||
| behavior, including the pre-existing tools check on `create_message`. |
There was a problem hiding this comment.
🟡 The overload/predicate change in this PR also changes runtime behavior of the deprecated create_message/peer.sample: a call passing tool_choice with no tools now returns CreateMessageResultWithTools (not a subclass of CreateMessageResult, and .content may be a list) instead of a plain CreateMessageResult, but this new migration section says direct ctx.session.* calls "keep their previous behavior". Consider adding a sentence documenting the tool_choice-only return-type change and softening that claim.
Extended reasoning...
What changed at runtime. Before this PR, ServerSession.create_message and ClientPeer.sample picked the result model with if tools is not None: only. Commits 557f36b/0b0b478 change that pick to wants_sampling_tools(tools, tool_choice) (src/mcp/server/session.py:259) and tools is not None or tool_choice is not None (src/mcp/shared/peer.py:178). So a call passing tool_choice=ToolChoice(...) with no tools — a shape the pre-PR overloads and validate_sampling_tools already permitted when the client declared sampling.tools — used to return a plain types.CreateMessageResult; it now returns types.CreateMessageResultWithTools.\n\nWhy that is observable. CreateMessageResultWithTools is not a subclass of CreateMessageResult — both derive directly from Result in mcp-types. Existing callers of these (deprecated but still shipped) APIs that do isinstance(result, CreateMessageResult) now get False for this shape, and code assuming a single content block can now legally receive a list. The new tests added in this PR (test_create_message_with_tool_choice_only_returns_with_tools_result, test_peer_sample_with_tool_choice_only_returns_with_tools_result) demonstrate exactly this new behavior.\n\nStep-by-step. (1) A v1/pre-PR server calls ctx.session.create_message(messages, max_tokens=10, tool_choice=ToolChoice(mode="none")) against a client that declared sampling.tools. (2) Pre-PR: tools is None, so the answer is validated as CreateMessageResult and the caller's isinstance(result, CreateMessageResult) / single-block result.content.text code works (for single-content answers). (3) Post-PR: wants_sampling_tools(None, tool_choice) is True, the same call returns CreateMessageResultWithTools; the isinstance check flips and .content may be a list, so the caller's code path changes without any change on their side.\n\nWhy the migration guide is the right place, and where it currently contradicts this. AGENTS.md requires behavioral/breaking changes (even softened ones) to be documented in docs/migration.md. The section this PR adds (docs/migration.md:57-59) ends with: "Direct ctx.elicit() and ctx.session.* calls outside resolvers keep their previous behavior, including the pre-existing tools check on create_message." That sentence is accurate about the -32021 gate (its main point) but overpromises for exactly the tool_choice-without-tools case, whose return type this same PR changes.\n\nWhy this is not merge-blocking. The runtime change itself is deliberate, spec-aligned (tool_choice implies tools-mode), and taken in response to review feedback; it is arguably a fix, since pre-PR a spec-valid array-content answer to a tool_choice request raised a raw ValidationError. The affected shape (tool_choice with no tools, on a deprecated API) is a narrow corner, and CreateMessageResultWithTools still accepts single-content answers. All that keeps this at documentation level.\n\nSuggested fix. Add one sentence to this migration section (or the sampling section) noting that create_message/peer.sample calls carrying tool_choice without tools now return CreateMessageResultWithTools, and soften "keep their previous behavior" to something like "keep their previous behavior, except that a tool_choice-only call is now treated as tools-mode and returns CreateMessageResultWithTools."
Resolvers can now return Sample(...) or ListRoots() in addition to Elicit: on 2026-07-28 sessions the request batches into the multi-round-trip InputRequiredResult flow, on 2025-11-25 it goes over the standalone back-channel request. One rendering produces the identical wire request on both transports, and marker-routed legacy sends bypass the deprecated session wrappers so no SEP-2577 warning fires for the compatibility path. Sampling and roots results are persisted in request_state like elicited answers (the client pays for an LLM call once per tool call, not once per round), pinned to the exact rendered request. Because the response union cannot always discriminate the two sampling result shapes, an answer is validated against the marker's expected model rather than trusting the union member. The elicitation-only capability check generalizes to a per-kind gate applied before sending on either transport: sampling, roots, and elicitation - including sampling.tools when the request carries tools, reported in full in the -32021 requiredCapabilities payload. This also gates the previously unchecked 2025 elicitation leg (documented in the migration guide). Client gains sampling_capabilities so sampling sub-capabilities like tools support can be declared alongside sampling_callback.
A short page for the two deprecated ask-the-client features: the Sample/ListRoots resolver way with tested snippets, the capability gate, and a warning box carrying the SEP-2577 deprecation scope (functional for at least twelve months before eligibility for removal, with the spec's suggested migrations). Also reworks dash-heavy sentences in this branch's earlier doc additions into plainer structure.
The legacy capability gate keyed on whether the handshake's declaration was visible, which let a session initialized with a bare notifications/initialized (no declared capabilities, live back-channel) receive ungated requests. Gate on the request-scoped channel's can_send_request instead, the channel these sends actually ride, so any sendable session is checked and a channel-less one keeps failing with its no-back-channel error. Adds ServerSession.can_send_request and a bare-initialized regression test, and corrects docs wording: tool_choice counts toward sampling.tools, the sampling.tools declaration needs the explicit client field, and the v1 claims are scoped to what v1 checked.
The capability gate already treats tool_choice alone as tools-mode; the answer model now shares the same predicate, so array-capable answers to a tool_choice-only request validate instead of being rejected as the wrong kind. Also rewords 'form elicitation' to 'form-mode elicitation' in the docs.
The resolver path, the deprecated session.create_message, and the peer sample API each hand-rolled the tools-mode test; the latter two still keyed the answer model on tools alone, so a tool_choice-only request was gated as tools-mode but its array-capable answer failed validation. One wants_sampling_tools predicate in mcp.server.validation now drives the capability check and the result model everywhere reachable, with the overloads updated to match and tests on each surface.
The broadened two-overload shape let a caller passing optional tools/tool_choice variables infer CreateMessageResultWithTools while the runtime could return the plain result. Three overloads (neither, tools, tool_choice) keep literal calls exact and type unnarrowed optional arguments as the result union, which matches the runtime.
0b0b478 to
b30ab0f
Compare
Resolvers can now return
Sample(...)orListRoots()in addition toElicit, covering all three request kinds the multi-round-trip flow allows (SEP-2322): elicitation, sampling, and roots.Motivation and Context
The resolver dependency-injection API (#2969, #2986) only supported asking the user via
Elicit. The multi-round-tripinputRequestsunion is a closed set of three request kinds, and the client half already dispatches all three to the standard callbacks — this fills in the server half so a tool dependency can also sample the client's LLM or fetch its roots:Design notes:
_render_requestproduces the wire request used both as the 2026-07-28inputRequestsentry and as the pre-2026 back-channel payload, so the two transports send identical shapes by construction. The legacy legs for sampling/roots callsend_requestdirectly rather than the@deprecatedsession wrappers: the deprecated thing (SEP-2577) is the standalone feature, and marker-routed compatibility sends shouldn't warn — directctx.session.create_message()still does.CreateMessageResult,CreateMessageResultWithToolswhen the request carriestoolsortool_choice,ListRootsResult).requestStatelike elicited answers, pinned to the exact rendered request, so the client pays for an LLM call once per tool call rather than once per retry round. The state encoding is unchanged and byte-compatible with in-flight state.InputResponsesunion cannot discriminate a no-tool-use answer to a tools request (a single content block parses as the plain result shape), so trusting the union member would reject spec-valid responses.elicitationform,sampling— plussampling.toolswhen the request carriestools/tool_choice— orroots) and refuses with-32021 MISSING_REQUIRED_CLIENT_CAPABILITYcarrying the fullrequiredCapabilitiespayload. On 2026-07-28 an absent declaration is meaningful by contract (capabilities arrive per-request, and servers must not infer them from prior requests), so the gate fires unconditionally; on pre-2026 sessions the gate applies wherever the request could actually be sent (it reads the request channel's own sendability), which covers sessions initialized with a barenotifications/initialized, while a session that cannot carry a server-initiated request at all keeps failing with the usual no-back-channel error.Clientgainssampling_capabilitiesso sampling sub-capabilities like tools support can finally be declared from the high-level client (ClientSessionalready accepted it).How Has This Been Tested?
Beyond the unit/e2e suite (all three kinds batched in one round, cross-kind resolver chains over three rounds, capability refusals on both eras, state restore, no-tool-use answers to tools requests), the branch was exercised as a real application: an
MCPServerprocess on streamable HTTP with a separate client process — 2026-07-28 auto negotiation with elicit+sample+roots fulfilled through the retry loop, 2025-11-25 legacy over the back-channel withMCPDeprecationWarningpromoted to error (none fired), a stdio subprocess negotiating 2026-07-28, and live-32021probes verifying the payloads and that the session stays usable after a refusal.Wire compatibility: no new wire shapes — the conformance everything-server already exercises all three embedded kinds, and the suite is unaffected. One gap worth noting: the conformance suite covers
-32021mechanics elsewhere but has no dedicated scenario for the "server MUST NOT send aninputRequestsentry the client has not declared support for" egress rule specifically; happy to raise that on the conformance repo.Breaking Changes
Resolver-routed requests now enforce the capability egress rule on pre-2026 sessions too: a 2025-11-25 client that answered elicitations without declaring the
elicitationcapability now gets-32021instead of being asked. Documented indocs/migration.md(declare the capability — the SDK client does this automatically when the callback is set — or drop the asking dependency). Directctx.elicit()/ctx.session.*calls outside resolvers are unaffected.Types of changes
Checklist
Additional context
Out of scope, noted for follow-ups:
ClientSessionGroupcannot declare sampling sub-capabilities (the same pre-existing gapClienthad before this PR), and the elicitation legacy leg's validation still lives inelicit_with_validationwhile sampling/roots go throughsend_request— kept as-is to leave the shipped elicitation path untouched.AI Disclaimer