-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat: multi round-trip requests — neutral contract and client auto-fulfilment engine #2313
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
felixweinberger
wants to merge
4
commits into
v2-2026-07-28
Choose a base branch
from
fweinberger/mrtr-neutral-contract-engine
base: v2-2026-07-28
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
4d1f4aa
feat(core): neutral multi-round-trip contract and 2026-07-28 in-band …
felixweinberger 1648d9c
feat(client): multi-round-trip auto-fulfilment engine and manual mode
felixweinberger 7434108
docs: fix unresolvable JSDoc links and a stray doc block; scope the c…
felixweinberger e461f95
fix(client): era-consistent embedded sampling validation, whole-flow …
felixweinberger File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| --- | ||
| '@modelcontextprotocol/core': minor | ||
| '@modelcontextprotocol/client': minor | ||
| --- | ||
|
|
||
| Add the client side of multi round-trip requests (protocol revision 2026-07-28, SEP-2322). The neutral `InputRequest`/`InputResponse`/`InputRequests`/`InputResponses`/`InputRequiredResult` types and the `isInputRequiredResult()` guard ship as the neutral surface (the | ||
| `inputRequired()` builder family and the `acceptedContent()` reader are exported by the server package as part of the server-side change); the 2026-07-28 wire codec models the in-band vocabulary (embedded requests and bare responses) and the retry-channel request fields. On the | ||
| client, an `input_required` answer to `tools/call`, `prompts/get`, or `resources/read` on a 2026-07-28 connection is now fulfilled automatically by default: the embedded requests are dispatched to the client's already-registered elicitation/sampling/roots handlers, and the | ||
| original call is retried with the collected `inputResponses`, a byte-exact echo of the opaque `requestState`, and a fresh request id, up to `inputRequired.maxRounds` rounds (default 10; exhaustion raises a typed `InputRequiredRoundsExceeded` error carrying the last result). | ||
| `client.callTool()` and its siblings keep returning their plain result types. `ClientOptions.inputRequired` (`autoFulfill`, `maxRounds`) configures the driver; manual mode is `autoFulfill: false` plus the per-call `allowInputRequired: true` request option and the | ||
| `withInputRequired()` schema wrapper. Retried requests surface their `inputResponses` to server handlers as bare response objects — entries in a wrapped `{method, result}` shape are dropped and reported via `ctx.mcpReq.droppedInputResponseKeys`. 2025-era behavior is unchanged: | ||
| the legacy wire has no `input_required` vocabulary and the legacy server-to-client request flow is untouched. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 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_requiredresults 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. Becausedocs/migration.mdis 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-299wires the driver into theClientconstructor:resolveInputRequiredDriverConfig(ininputRequiredDriver.ts) defaultsautoFulfilltoDEFAULT_INPUT_REQUIRED_AUTO_FULFILL = true. Inprotocol.ts_requestWithSchemaViaCodec, when a response decodes toinput_required, the funnel now routes to the auto-fulfilment driver wheneverdriverConfig.autoFulfillis true; the oldUnsupportedResultTypereject arm is only reached when there is no driver orautoFulfill: falseand no per-callallowInputRequired: true.Why the existing prose is now wrong
Two passages in
docs/migration.mddescribe the superseded behavior: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.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.mddocumenting 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 whetherdocs/**/*.mddescribes the old behavior and flag prose that now contradicts the implementation; breaking changes are to be documented indocs/migration.md.Impact
A reader of the migration guide will believe a 2026-era
input_requiredanswer surfaces as anUnsupportedResultTypeerror 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-callallowInputRequired: true+ thewithInputRequired()schema wrapper), and the newSdkErrorCode.InputRequiredRoundsExceedederror 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.mdto state the new default: on 2026-07-28 connectionsinput_requiredis auto-fulfilled by default through the already-registered elicitation/sampling/roots handlers, with the original call retried (fresh id, byte-exactrequestStateecho, collectedinputResponses) up toinputRequired.maxRounds(default 10). Document theUnsupportedResultTypereject arm as the manual path (autoFulfill: falseand noallowInputRequired), and add the newSdkErrorCode.InputRequiredRoundsExceededand theClientOptions.inputRequired/allowInputRequired/withInputRequired()surface.Step-by-step proof
new Client(info)with noinputRequiredoption and connects to a 2026-07-28 server, registering anelicitation/createhandler.client.callTool({ name: 'deploy', arguments: {} }).{ 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.callToolresolves to the plainCallToolResult. (This is exactly whatinputRequiredEngine.test.ts'auto-fulfilment (default on)' asserts.)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.