feat: multi round-trip requests — neutral contract and client auto-fulfilment engine#2313
feat: multi round-trip requests — neutral contract and client auto-fulfilment engine#2313felixweinberger wants to merge 4 commits into
Conversation
…wire vocabulary Adds the neutral InputRequest/InputResponse/InputRequests/InputResponses/ InputRequiredResult types, the inputRequired() builder family with the acceptedContent() reader and isInputRequiredResult() guard, and the withInputRequired() schema wrapper. The 2026-07-28 wire module gains the in-band vocabulary (embedded elicitation/sampling/roots request and bare response schemas, the wire InputRequiredResult, the retry-channel request fields, and the input-required result-response unions), exposed to the protocol layer through new inputRequestSchema/inputResponseSchema codec accessors (the 2025-era codec has none). The spec-type parity suite and the example corpus pick up the new artifacts and their pending entries burn down.
An input_required answer to tools/call, prompts/get, or resources/read on a 2026-07-28 connection is now fulfilled automatically: the embedded requests are dispatched to the already-registered elicitation/sampling/roots handlers (through the same stored-handler chain, with a synthesized correlation-only context), and the original call is retried with the collected inputResponses, a byte-exact requestState echo, and a fresh request id. The driver is a layer over the manual path (allowInputRequired), capped at inputRequired.maxRounds (default 10, typed InputRequiredRoundsExceeded on exhaustion), paces requestState-only legs at a fixed 250 ms, surfaces rounds as synthetic progress, and bounds the whole flow with the existing timeout/maxTotalTimeout knobs. ClientOptions.inputRequired configures it; autoFulfill: false plus the per-call allowInputRequired option and withInputRequired() form the manual path. Inbound retried requests surface bare inputResponses to handlers and report wrapped entries via ctx.mcpReq.droppedInputResponseKeys. The raw-result-type scenario pins discrimination with the driver disabled; the SdkErrorCode pin gains the new member.
🦋 Changeset detectedLatest commit: e461f95 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
…lient changeset
- Replace two {@linkcode} references that typedoc cannot resolve (the
inputRequired() builder module path on InputRequiredResult and the nested
droppedInputResponseKeys member path on BaseContext) with plain code text,
so docs:check passes on this change by itself.
- Move the partitionInputResponses doc block next to the function it
describes (it sat above relatedMessagingUnavailable).
- The changeset no longer claims the inputRequired() builder and
acceptedContent() ship here; they are exported by the server package as
part of the server-side change.
…timeout accounting, and a fail-fast check on empty input_required - When the sampling/createMessage request schema comes from the in-band (2026-07-28 embedded-request) fallback, validate the handler's response against the era's embedded response schema instead of the 2025 result schema pair, mirroring the elicitation handler wrapper. Covers forked, tool-bearing responses. - maxTotalTimeout now bounds the whole multi round-trip flow: the flow start is captured when the original request is issued and passed to the auto-fulfilment driver, so the first wire leg counts against the budget. Error shape unchanged. - An input_required result carrying neither inputRequests nor a requestState string is rejected at decode time with a typed error instead of being retried with the original params until the round cap is exhausted. - New tests: tool-bearing in-band sampling response, first-leg timeout accounting (driver and client), empty input_required fail-fast, and a client-side pin that a wire elicitation/create request on a 2026-07-28 connection is never dispatched to the registered handler (the in-band schemas grant no wire-request registry membership).
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| // Multi-round-trip auto-fulfilment driver (2026-07-28): on by default, | ||
| // configurable via ClientOptions.inputRequired. | ||
| this._inputRequiredDriverConfig = resolveInputRequiredDriverConfig(options?.inputRequired); |
There was a problem hiding this comment.
🔴 The migration guide (docs/migration.md, not touched by this PR) still documents the old input_required behavior and now contradicts the shipped default. Line 921 says input_required 'rejects with a typed local error — SdkErrorCode.UnsupportedResultType ... Full multi-round-trip support will replace that error arm', and line 950 says 'input_required / unknown kinds reject with UnsupportedResultType / InvalidResult' — but this PR sets DEFAULT_INPUT_REQUIRED_AUTO_FULFILL = true, so on 2026-07-28 connections input_required is now auto-fulfilled by default and only rejects when autoFulfill:false (without allowInputRequired). The guide should be updated to document the new default auto-fulfilment, ClientOptions.inputRequired (autoFulfill/maxRounds), manual mode (allowInputRequired + withInputRequired), and SdkErrorCode.InputRequiredRoundsExceeded.
Extended reasoning...
What the bug is
This PR ships the client half of multi round-trip requests and flips the default 2026-era posture for input_required results from reject to auto-fulfil. The migration guide, docs/migration.md, still describes the pre-PR reject behavior and even forward-references the exact work this PR delivers as future work. Because docs/migration.md is not among the 25 files this PR changed, the consumer-facing migration guide now actively misstates the default behavior of the version it documents.
The code path that triggers it
packages/client/src/client/client.ts:297-299 wires the driver into the Client constructor:
this._inputRequiredDriverConfig = resolveInputRequiredDriverConfig(options?.inputRequired);resolveInputRequiredDriverConfig (in inputRequiredDriver.ts) defaults autoFulfill to DEFAULT_INPUT_REQUIRED_AUTO_FULFILL = true. In protocol.ts _requestWithSchemaViaCodec, when a response decodes to input_required, the funnel now routes to the auto-fulfilment driver whenever driverConfig.autoFulfill is true; the old UnsupportedResultType reject arm is only reached when there is no driver or autoFulfill: false and no per-call allowInputRequired: true.
Why the existing prose is now wrong
Two passages in docs/migration.md describe the superseded behavior:
- Line 921 (Raw-first result discrimination): '...any other kind (e.g.
input_required) rejects with a typed local error —SdkErrorwith the new codeSdkErrorCode.UnsupportedResultType... Full multi-round-trip support will replace that error arm.' This PR is that replacement, yet the sentence still frames it as future work and asserts thatinput_requiredrejects. - Line 950 (per-era codec): 'On a 2026-era exchange the discrimination is stricter than before:
resultTypeis REQUIRED, an absent value is a spec violation ... andinput_required/ unknown kinds reject withUnsupportedResultType/InvalidResult.' After this PR,input_requiredno longer rejects by default — it is auto-fulfilled.
The PR added .changeset/mrtr-client-engine.md documenting the new behavior, but a changeset is a release-note artifact, not the migration guide consumers read to understand era behavior. The repo's REVIEW.md tests-and-docs convention mandates exactly this flag: on a behavior change, check whether docs/**/*.md describes the old behavior and flag prose that now contradicts the implementation; breaking changes are to be documented in docs/migration.md.
Impact
A reader of the migration guide will believe a 2026-era input_required answer surfaces as an UnsupportedResultType error and that they must handle it themselves, when in fact the SDK now dispatches the embedded requests to their registered handlers and retries automatically. They will also miss the new configuration surface entirely: ClientOptions.inputRequired (autoFulfill, maxRounds), manual mode (autoFulfill: false + per-call allowInputRequired: true + the withInputRequired() schema wrapper), and the new SdkErrorCode.InputRequiredRoundsExceeded error raised on round-cap exhaustion. This is documentation-only — no runtime defect — but the guide is the primary reference for these era behavior changes and currently misleads about the default.
How to fix
Update the two passages in docs/migration.md to state the new default: on 2026-07-28 connections input_required is auto-fulfilled by default through the already-registered elicitation/sampling/roots handlers, with the original call retried (fresh id, byte-exact requestState echo, collected inputResponses) up to inputRequired.maxRounds (default 10). Document the UnsupportedResultType reject arm as the manual path (autoFulfill: false and no allowInputRequired), and add the new SdkErrorCode.InputRequiredRoundsExceeded and the ClientOptions.inputRequired / allowInputRequired / withInputRequired() surface.
Step-by-step proof
- A consumer constructs
new Client(info)with noinputRequiredoption and connects to a 2026-07-28 server, registering anelicitation/createhandler. - They call
client.callTool({ name: 'deploy', arguments: {} }). - The server answers with
{ resultType: 'input_required', inputRequests: { github_login: { method: 'elicitation/create', ... } } }. resolveInputRequiredDriverConfig(undefined)produced{ autoFulfill: true, maxRounds: 10 }at construction (client.ts:297-299), so inprotocol.tsthedecoded.kind === 'input_required'branch findsdriverConfig.autoFulfill === trueand calls_runInputRequiredDriver— it does not reject withUnsupportedResultType.- The driver dispatches the embedded elicitation to the registered handler and retries;
callToolresolves to the plainCallToolResult. (This is exactly whatinputRequiredEngine.test.ts'auto-fulfilment (default on)' asserts.) - The consumer, reading
docs/migration.md:921/:950, expected step 4 to reject withUnsupportedResultTypeand 'full multi-round-trip support' to still be pending. The guide directly contradicts the observed default behavior — confirming the stale-docs bug.
| }); | ||
|
|
||
| /** | ||
| * Reads the accepted content of a form-mode elicitation response from a | ||
| * retried request's `inputResponses` (`ctx.mcpReq.inputResponses`). | ||
| * | ||
| * Returns the response's `content` for `key` when the entry is an accepted | ||
| * elicitation result, and `undefined` otherwise (missing key, declined or | ||
| * cancelled elicitation, or a response of another kind). The values arrive | ||
| * from the client and are not re-validated here — treat them as untrusted | ||
| * input. | ||
| */ | ||
| export function acceptedContent<T extends Record<string, unknown> = Record<string, unknown>>( |
There was a problem hiding this comment.
🟡 acceptedContent<T>(responses, key) is a ~6-line content reader over the SDK-unvalidated inputResponses map (it checks entry.action === 'accept' and returns entry.content) — it does no protocol or validation work the SDK must own, and the bare responses are already surfaced at ctx.mcpReq.inputResponses. Per REVIEW.md Guiding Principle #1 ("content helpers belong in userland") and the API-surface checklist ("helpers users can write themselves belong in a cookbook, not the SDK"), consider moving it to a cookbook/example rather than shipping it as API. (Design/minimalism nit — the inputRequired() builder, which does real wire-vocabulary work, is defensible; only the reader is flagged.)
Extended reasoning...
What this is. This PR adds acceptedContent() to packages/core/src/shared/inputRequired.ts (lines 126–138). It is a pure userland reader: undefined/null/array guards, a key lookup, a single candidate.action !== 'accept' discrimination, an object-presence check on content, and a return of candidate.content. The only protocol knowledge embedded is the one string literal 'accept'. It performs no framing, no schema validation, and no wire mapping — and it explicitly cannot, since the docstring itself states the values "arrive from the client and are not re-validated here — treat them as untrusted input."\n\nWhy REVIEW.md flags it. The repository's review conventions are the governing standard here, and they name this category twice. Guiding Principle #1 (Minimalism) lists "content helpers" alongside middleware engines and builder patterns as things that "belong in userland." The API-surface checklist is even more direct: "helpers users can write themselves belong in a cookbook, not the SDK" and "Every new export is intentional." Principle #2 puts the burden of proof on addition — the default answer to "should we add this?" is no. acceptedContent is the textbook instance: the SDK already surfaces the bare responses at ctx.mcpReq.inputResponses, so a consumer can write responses?.confirm?.action === 'accept' ? responses.confirm.content : undefined inline.\n\nNo concrete consumer. Principle #3 asks every new abstraction to have a concrete callsite in the PR. acceptedContent has none — its only appearances are a JSDoc @example snippet on the inputRequired builder and the unit tests in inputRequired.test.ts. A JSDoc example and a test of the helper itself do not meet the "concrete consumer today" bar.\n\nExport-surface precision. To be exact about what this diff does: acceptedContent is reachable here only on the core internal barrel (packages/core/src/index.ts adds export * from './shared/inputRequired.js'). It is not added to exports/public/index.ts (which exports only isInputRequiredResult) and is not re-exported by packages/client/src/index.ts (which adds only withInputRequired and InputRequiredOptions). The changeset states it "is exported by the server package as part of the server-side change" — so the public export lands in a sibling PR. The function and the changeset advertising it as shipping API are nonetheless introduced here, and a reviewer following REVIEW.md can legitimately ask that it move to a cookbook/example before it reaches the public surface.\n\nImpact and fix. There is no functional defect — this is a design/altitude concern, not a correctness bug, hence nit severity. The cost is API-surface creep: once exported, removal is far harder than non-addition (Principle #2). The fix is to either (a) justify it with a real consumer, or (b) move it to examples//a cookbook entry and drop it from the shipped surface. Note the targeting is narrow: the inputRequired() builder family in the same file does author the protocol input_required shape and enforce the at-least-one rule, so it is defensibly SDK-worthy; only the reader is flagged.\n\nStep-by-step proof. 1) A server handler wants the accepted form content for key confirm. 2) With the SDK as-is in this PR, it would call acceptedContent(ctx.mcpReq.inputResponses, 'confirm'). 3) Trace the body: responses is an object → not undefined/null/array; entry = responses['confirm']; entry is an object → passes; entry.action === 'accept' → passes; entry.content is a non-null object → passes; returns entry.content. 4) The equivalent inline expression const e = ctx.mcpReq.inputResponses?.confirm; const content = (e && e.action === 'accept') ? e.content : undefined; produces the identical result with no SDK dependency. 5) Therefore the helper adds public surface without doing work the consumer could not trivially do themselves — exactly the minimalism case REVIEW.md enumerates.
| // watchdogs around the call see liveness instead of silence. | ||
| requestOptions.onprogress?.({ progress: round, message: `Fulfilling input required by '${method}' (round ${round})` }); | ||
|
|
||
| const entries = Object.entries(payload.inputRequests ?? {}); | ||
| let responses: Record<string, unknown> | undefined; | ||
| if (entries.length > 0) { | ||
| // Fulfil concurrently (the embedded requests are independent); a | ||
| // single failure fails the call. | ||
| const fulfilled = await Promise.all( | ||
| entries.map(async ([key, entry]) => [key, await hooks.dispatchInputRequest(key, entry)] as const) | ||
| ); |
There was a problem hiding this comment.
🟡 When a round's embedded requests are fulfilled concurrently with Promise.all(entries.map(... dispatchInputRequest ...)) (inputRequiredDriver.ts ~188-198), a fast-failing entry (missing handler, unknown kind, or handler throw) rejects the whole call but never aborts the still-running sibling handlers — e.g. input_required with { ask: elicitation/create, sample: sampling/createMessage } and no sampling handler: the sample dispatch throws CapabilityNotSupported immediately while the elicitation handler is still showing a UI prompt, which is then orphaned (its eventual answer is silently discarded). Minor cleanup gap, not a crash; the cheapest fix is to pre-validate every entry (method-known + handler-registered) before invoking any handler, and/or thread a per-round AbortController so a fail-fast cancels signal-cooperative siblings.
Extended reasoning...
What the bug is
runInputRequiredDriver fulfils a round's embedded requests concurrently:
const fulfilled = await Promise.all(
entries.map(async ([key, entry]) => [key, await hooks.dispatchInputRequest(key, entry)] as const)
);Promise.all is fail-fast: the first rejection settles the aggregate promise, but the remaining sibling promises keep running to completion. Nothing in this round links the siblings together for cancellation. In protocol.ts, _dispatchInputRequest synthesizes the embedded handler's context with signal: options?.signal ?? new AbortController().signal — that is either the originating call's signal or a brand-new, never-aborted signal. A sibling's failure does not abort either, so a handler that is mid-flight when a sibling rejects is orphaned: its work runs to completion and its result is discarded.
The code path that triggers it
Multi-entry rounds are an intended shape (inputRequests is a record). _dispatchInputRequest rejects early and synchronously for the common failure modes — an unknown kind (codec.inputRequestSchema(method) === undefined → InvalidResult) or a missing handler (this._requestHandlers.get(method) === undefined → CapabilityNotSupported) — while a well-behaved sibling handler (e.g. elicitation/create) is awaiting user input. This is precisely the scenario the PR's own test fails the call with a typed error when a required handler is not registered exercises, but with a second entry present.
Why existing code doesn't prevent it
There is no per-round AbortController, and the synthesized signal is not chained to one. The driver promise itself settles cleanly and returns the correct typed error, so there is no SDK-owned leak (no dangling timer, controller, or map entry) and no unhandled rejection — Promise.all has already attached reactions to every input promise, so a later sibling rejection is consumed-and-ignored, not surfaced. The only casualty is the user handler's in-flight work.
Impact
Modest and bounded. On the error path, a sibling interactive prompt (or in-progress sampling handler) is left running with no abort signal; when it eventually resolves, the value is silently dropped. For an elicitation/create handler this means a dialog the user is still looking at becomes meaningless — they answer, nothing happens. This is a UX/resource-cleanup gap, not a correctness or crash bug, and it is new to this PR: the multi-round-trip round groups embedded requests under fail-fast semantics, unlike the independent server→client requests of the 2025 wire path, so a handler that does observe ctx.mcpReq.signal is denied the chance to cancel its own work when a SIBLING fails.
Addressing the objection that this is not actionable
The limitations are real and I have kept the severity at nit because of them: there is no unhandled rejection, no SDK-state leak, and JS promises are not force-cancellable — a non-cooperative handler could not be torn down even with a signal. But the conclusion that there is no tradeoff-free fix overstates it. The dominant trigger (unknown kind / missing handler) is a purely synchronous, local check that needs no cancellation machinery at all: validate every entry's method-known + handler-registered status before invoking any handler, so the common fail-fast can never orphan an already-running prompt. That is a minimalist change consistent with the repo's 'do less' posture, and it closes the realistic case. A per-round AbortController threaded into the synthesized signal is an optional second step that additionally helps signal-cooperative handlers; it is not required to make the fix worthwhile.
Step-by-step proof
- A 2026-07-28 server answers
tools/callwith{ resultType: 'input_required', inputRequests: { ask: { method: 'elicitation/create', params: {...} }, sample: { method: 'sampling/createMessage', params: {...} } } }. The client has anelicitation/createhandler registered but nosampling/createMessagehandler. - The driver builds
entries = [['ask', ...], ['sample', ...]]and callsPromise.all(entries.map(async ([key, entry]) => [key, await hooks.dispatchInputRequest(key, entry)])). Both map callbacks start synchronously. dispatchInputRequest('ask', ...)passes validation, looks up the elicitation handler, and invokes it; the handler renders a UI prompt and awaits the user. Itsctx.mcpReq.signalisoptions?.signal ?? new AbortController().signal— not linked to any sibling.dispatchInputRequest('sample', ...)passes the in-band schema check (sampling is 2026 in-band vocabulary) but finds no registered handler and throwsSdkError(CapabilityNotSupported).- The
samplepromise rejects ⇒Promise.allrejects ⇒runInputRequiredDriverrejects ⇒client.callTool()rejects withCapabilityNotSupported. - The
askelicitation handler is still showing its prompt. No signal is aborted. When the user answers, the handler resolves{ action: 'accept', content: {...} }, whichPromise.allhas already discarded. The user's answer vanishes and the prompt was shown for nothing.
| * Related send/notify are unavailable inside an embedded input-request | ||
| * handler: the request is fulfilled locally by the multi-round-trip driver, | ||
| * so there is no live peer request to relate messages to. | ||
| */ | ||
| function relatedMessagingUnavailable(member: string): never { | ||
| throw new SdkError( | ||
| SdkErrorCode.SendFailed, | ||
| `ctx.mcpReq.${member} is not available while fulfilling an embedded input request: ` + | ||
| `the request is fulfilled locally and has no related peer request` | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
🟡 The synthesized embedded-handler context in _dispatchInputRequest binds ctx.mcpReq.send/notify to relatedMessagingUnavailable(), which throws synchronously, but BaseContext.mcpReq.send/notify are declared Promise-returning (and the wire-path implementations are async). A handler reused across wire and MRTR dispatch that uses the documented fire-and-forget idiom ctx.mcpReq.notify(n).catch(...) will have the throw escape before .catch can attach on the MRTR path, hard-failing the whole input-required dispatch (and the callTool) instead of being swallowed. Fix by returning Promise.reject(new SdkError(...)) instead of throwing, so the error surface matches the declared contract on both dispatch paths.
Extended reasoning...
What the bug is. In packages/core/src/shared/protocol.ts, _dispatchInputRequest synthesizes the context for an embedded input-request handler and binds the related-messaging members to relatedMessagingUnavailable:
send: (() => relatedMessagingUnavailable('send')) as BaseContext['mcpReq']['send'],
notify: () => relatedMessagingUnavailable('notify')relatedMessagingUnavailable is typed : never and does a bare throw new SdkError(...) — a synchronous throw. But the declared contract BaseContext.mcpReq.send (both overloads) and notify is Promise-returning, and every other implementation in the codebase honors that: the wire path builds sendRequest/sendNotification over _requestWithSchemaViaCodec/_notificationViaCodec, which genuinely return Promises (and reject on failure). So the same typed API has two different error surfaces depending on whether the handler was reached over the wire (2025-era request) or via the MRTR driver (2026-era embedded dispatch).
Why the type system doesn't catch it. A () => never is assignable to a Promise-returning signature (never is assignable to Promise<void>, and the zero-arg notify arrow is assignable to the one-arg type), and the send arrow is force-cast with as BaseContext['mcpReq']['send']. The mismatch slips through with no compile error while violating the runtime contract.
Where it manifests (the observable path). _dispatchInputRequest ends with return await handler(synthesizedRequest, ctx), called from the driver inside Promise.all(entries.map(async ([key, entry]) => [key, await hooks.dispatchInputRequest(key, entry)])). For the common, documented fire-and-forget idiom inside a handler:
client.setRequestHandler('elicitation/create', async (request, ctx) => {
ctx.mcpReq.notify({ method: 'notifications/progress', params: { progressToken: 1, progress: 1 } }).catch(() => {});
return { action: 'accept', content: { name: 'octocat' } };
});the throw fires while evaluating the call expression ctx.mcpReq.notify(...), before .catch is ever attached. The error therefore escapes synchronously out of the async handler, rejecting await handler(...), which rejects dispatchInputRequest, which rejects the Promise.all, which aborts the whole driver run and fails the originating client.callTool(). On the wire path the identical handler returns a rejected Promise that the .catch(() => {}) swallows, and the handler completes normally. (For the plain await ctx.mcpReq.notify(...) form the two are indistinguishable — a sync throw inside an async fn becomes the same rejection — which is why this is narrow.)
Step-by-step proof.
- Server answers
tools/callwith{ resultType: 'input_required', inputRequests: { github_login: <elicit entry> } }. - The driver calls
_dispatchInputRequest(codec, 'github_login', entry, options), which synthesizesctxwithnotify: () => relatedMessagingUnavailable('notify'). - The registered
elicitation/createhandler runs and evaluatesctx.mcpReq.notify(n).catch(() => {}). - Evaluating
ctx.mcpReq.notify(n)invokesrelatedMessagingUnavailable('notify'), which throws anSdkErrorsynchronously — no value is returned, so.catchis never reached. - The throw propagates out of the handler (it is not in a try/catch and
.catchnever attached), rejecting the handler Promise →dispatchInputRequestrejects → the driver'sPromise.allrejects →runInputRequiredDriverrejects →client.callTool()rejects withSEND_FAILED. - Run the same handler on a 2025-era wire connection:
ctx.mcpReq.notify(n)returns a Promise (which rejects, since the related send may fail),.catch(() => {})swallows it, and the handler returns its result normally.
Same code, two outcomes — the MRTR path crashes the call the consumer explicitly tried to defend against.
Why existing code/tests don't prevent it. The engine test only asserts expect(() => ctx.mcpReq.notify(...)).toThrow(/not available/) and expect(() => ctx.mcpReq.send(...)).toThrow(...). That pins the current synchronous-throw behavior rather than the documented async contract, so it masks the inconsistency instead of guarding against it.
Impact and fix. Low/narrow: it only bites a handler that calls related send/notify via the floating-.catch idiom and is reused across both dispatch paths, and related messaging is intentionally unavailable on this path either way — only throw-vs-reject differs. The contract-honoring fix is to return a rejected Promise instead of throwing, e.g.:
function relatedMessagingUnavailable(member: string): Promise<never> {
return Promise.reject(new SdkError(SdkErrorCode.SendFailed,
`ctx.mcpReq.${member} is not available while fulfilling an embedded input request: ` +
`the request is fulfilled locally and has no related peer request`));
}(and update the test to assert await expect(ctx.mcpReq.notify(...)).rejects.toThrow(/not available/)). This makes the error surface identical on both dispatch paths.
| if (rawResultType === 'input_required') { | ||
| // The driver seam (#13 consumes this payload). | ||
| const inputRequests = raw['inputRequests']; | ||
| const rawInputRequests = raw['inputRequests']; | ||
| const inputRequests = isPlainObject(rawInputRequests) ? rawInputRequests : {}; | ||
| const requestState = raw['requestState']; | ||
| if (Object.keys(inputRequests).length === 0 && typeof requestState !== 'string') { | ||
| // At-least-one rule, client side: with neither inputRequests | ||
| // nor requestState there is nothing to fulfil and nothing to | ||
| // echo — retrying would only resend the original params until | ||
| // the round cap is exhausted, so fail fast instead. | ||
| return { | ||
| kind: 'invalid', | ||
| error: new SdkError( | ||
| SdkErrorCode.InvalidResult, | ||
| `Invalid result for ${method}: input_required carries neither inputRequests nor requestState ` + | ||
| `(every input_required result must include at least one of the two)`, | ||
| { method, violation: 'input-required-missing-both' } | ||
| ) | ||
| }; | ||
| } | ||
| return { | ||
| kind: 'input_required', | ||
| inputRequests: isPlainObject(inputRequests) ? inputRequests : {}, | ||
| ...(typeof raw['requestState'] === 'string' && { requestState: raw['requestState'] }) | ||
| inputRequests, | ||
| ...(typeof requestState === 'string' && { requestState }) | ||
| }; | ||
| } | ||
| if (rawResultType !== 'complete') { |
There was a problem hiding this comment.
🟡 decodeResult() in packages/core/src/wire/rev2026-07-28/codec.ts discriminates input_required on the raw resultType alone, with no check on the method, so a 2026 server that answers a NON-MRTR method (e.g. completion/complete, tools/list) with input_required is driven into the auto-fulfilment driver (dispatching the client's elicitation/sampling/roots handlers and retrying the original call) instead of being rejected. This deviates from the SDK's own wire-response contract in schemas.ts, where only CallTool/GetPrompt/ReadResource responses are unioned with InputRequiredResultSchema. Consider gating the input_required decode branch (or the driver entry) to the three MRTR methods to enforce the contract at runtime; low severity since it only fires against a non-conformant server.
Extended reasoning...
What the bug is
On the 2026-07-28 era, decodeResult(method, raw) in packages/core/src/wire/rev2026-07-28/codec.ts decides a result is input_required purely from the raw payload:
if (rawResultType === 'input_required') {
// ... returns { kind: 'input_required', inputRequests, requestState? }
}The method argument is passed in but is used only for error messages and, on the complete branch, the WIRE_RESULT_SCHEMAS lookup. It is never used to gate the input_required branch. Because this raw-first discrimination (V-1) runs before any response-schema parse, the wire-response schemas never get a say on this path.
That matters because the SDK's own wire contract in schemas.ts restricts input_required to exactly three methods: only CallToolResultResponseSchema, GetPromptResultResponseSchema, and ReadResourceResultResponseSchema are z.union([..., InputRequiredResultSchema]). ListToolsResultResponseSchema, CompleteResultResponseSchema, ListResourcesResultResponseSchema, etc. are not. SEP-2322 likewise scopes input_required to tools/call, prompts/get, and resources/read. The runtime decode is therefore broader than both the spec and the SDK's modeled contract, and that 3-method restriction is never enforced anywhere on the live path.
The code path that triggers it
_requestWithSchemaViaCodec (packages/core/src/shared/protocol.ts, ~line 1344) acts on decoded.kind === 'input_required' generically — there is no method gate. With autoFulfill defaulting to true, it calls _runInputRequiredDriver, which dispatches the embedded requests to the client's registered handlers via _dispatchInputRequest and retries request.method verbatim with inputResponses / requestState, looping until a complete result or maxRounds (→ InputRequiredRoundsExceeded).
Why existing code does not prevent it
The wire-schema restriction is real but inert here: WIRE_RESULT_SCHEMAS (the per-method wrappers that would reject an unexpected shape) is only consulted on the complete branch, which is reached only after the input_required branch has already returned. So for a non-MRTR method answered with input_required, the schema that omits InputRequiredResultSchema is never run. This is a behavioral change introduced by this PR: pre-PR the funnel surfaced input_required as a typed UnsupportedResultType error for every method; now it drives the loop.
Impact
A non-conformant 2026-era server that answers, say, completion/complete or tools/list with input_required will cause the client to dispatch its embedded elicitation/sampling/roots handlers — including user-facing elicitation prompts — and then retry the list/complete call with inputResponses, instead of rejecting the result as invalid. There is no crash or security escalation, and the loop is bounded by maxRounds and maxTotalTimeout, but the client takes a meaningless interactive detour on a method where the result kind is contractually impossible.
Addressing the refutation
The refutation makes fair points, and they cap the severity rather than negate the issue:
- "Intentional, method-blind seam; the schema restriction lives at a different layer." True that the decode is structurally method-blind by design — but the consequence is that a restriction the SDK explicitly models (the 3-method union) is enforced nowhere at runtime. The fix is small and local: gate the
input_requiredbranch (or the driver entry) totools/call/prompts/get/resources/read, falling through to the unknown-kindinvalidpath otherwise. - "Gating to three methods buys nothing — a malicious server can reach the same loop via the legitimate MRTR methods." Correct, and that is why this is not framed as an attack-surface or security fix. The value is contract conformance and not driving a user-visible elicitation loop on a method (e.g.
completion/complete) where the SDK's own schema says the kind cannot appear. - "Requires a spec-violating server, so it never fires in normal operation." Agreed — hence the low severity. This is a robustness/conformance gap, not a defect on the conformant path.
Step-by-step proof
- Client connects on
2026-07-28withautoFulfillleft at its default (true) and anelicitation/createhandler registered. - The client issues
completion/complete(viaclient.complete(...)). - A non-conformant server answers:
{ resultType: 'input_required', inputRequests: { ask: { method: 'elicitation/create', params: { mode: 'form', ... } } } }. decodeResult('completion/complete', raw)seesrawResultType === 'input_required'and returns{ kind: 'input_required', ... }. The methodcompletion/completeis ignored;WIRE_RESULT_SCHEMAS['completion/complete'](which isCompleteResultSchema, with noInputRequiredResultunion) is never consulted because the branch returned earlier.- In
_requestWithSchemaViaCodec,decoded.kind === 'input_required',allowInputRequiredis unset,driverConfig.autoFulfillistrue→_runInputRequiredDriverruns. - The driver dispatches the embedded
elicitation/createto the registered handler — the user is prompted — then retriescompletion/completeverbatim withinputResponses/requestState. - It loops until a complete result or
maxRounds, then rejects withInputRequiredRoundsExceeded.
Contrast with the modeled contract: CompleteResultResponseSchema = wireResultResponse(CompleteResultSchema) is not unioned with InputRequiredResultSchema, so the SDK explicitly models that completion/complete may never answer input_required — yet the runtime drives the loop anyway. Gating the decode branch to the three MRTR methods would route this to the existing invalid / UnsupportedResultType path, matching the contract.
1 participant
This adds the client half of multi round-trip requests (SEP-2322, protocol revision 2026-07-28): the neutral
InputRequest/InputResponse/InputRequiredResult types, the
inputRequired()builder family, and a client engine thatfulfils
input_requiredresults automatically through the already-registered elicitation/sampling/roots handlers andretries the original call with
inputResponses, a byte-exactrequestStateecho, and a fresh request id.Motivation and Context
The 2026-07-28 draft removes the server→client JSON-RPC request channel; servers obtain client input in-band by
answering tools/call, prompts/get, or resources/read with an
input_requiredresult. Clients need a way to fulfilthose results without changing their call sites.
How Has This Been Tested?
path, and the inbound
inputResponsespartition.flows, missing-handler/unknown-kind failures, manual mode, the synthesized handler context).
Breaking Changes
None for 2025-era traffic. On 2026-07-28 connections,
input_requiredanswers are now fulfilled automatically bydefault;
inputRequired: { autoFulfill: false }and the per-callallowInputRequiredoption restore manual handling.Types of changes
Checklist
Additional context
client.callTool()keeps returning a plainCallToolResult; the interactive rounds happen inside the call.cap defaults to 10 (aligned with the other SDK client engines), and requestState-only retries are paced.
requestStateis treated as an opaque string end to end (echoed byte-exact, never parsed).