feat: multi-round-trip request implementation (SEP-2322)#950
feat: multi-round-trip request implementation (SEP-2322)#950yarolegovich wants to merge 8 commits into
Conversation
yarolegovich
force-pushed
the
yarolegovich/mrtr
branch
from
8fa6e4f to
ec8b204
Compare
|
I think the main question we should ask ourselves is how we would design the API for this if there was no backwards compatibility requirement. If we have an approach that will likely be chosen for v2, sometimes it may make sense to move toward its direction, even if it has some disadvantages if they would disappear with v2 release. |
I don't think my proposal would change significantly if we didn't need backward compatibility. I would want a sealed interface return type for If we make the client API return a sealed interface as well the "base case" becomes overly verbose - all clients need to unpack result to a specific type and handle it accordingly, even though most of the time retry is auto-handled by the middleware. Now this breaks the proposed client side "manual handling" where if you disable MRTR through options you can just inspect |
|
I think it's also worth adding a section on what should happen with the existing methods that are replaced with MRTR. |
yarolegovich
changed the title
| type MRTROptions struct { | ||
| // MaxRetries is the maximum number of MRTR retry attempts after the | ||
| // initial call. Defaults to 3 if the provided value is <= 0. | ||
| MaxRetries int |
There was a problem hiding this comment.
It looks like you're giving the client a fixed number of chances to satisfy the input requests. I'm not sure I understand why. I would say you should keep retrying as long as the client is making progress, by providing the answer to at least one request.
| // middleware. The middleware is enabled by default and automatically fulfills input | ||
| // requests from the server by invoking the appropriate client handlers and | ||
| // retrying the original call. | ||
| type MRTROptions struct { |
There was a problem hiding this comment.
Do you know if other SDKs are using "MRTR" or do they use something else? I'm not suggesting spelling out the full phrase, but maybe MultiRequest, etc. I think MRTR is fine if other SDKs don't have anything better.
|
|
||
| // ResultType indicates whether a result is complete or requires further input | ||
| // from the client via the MRTR (Multi Round-Trip Requests) protocol. | ||
| type ResultType string |
There was a problem hiding this comment.
Does this need to be exported?
| // InputRequest is a sealed interface for parameters that a server can include | ||
| // in an MRTR input-required result. Implementations are [*ElicitParams], | ||
| // [*CreateMessageParams], and [*ListRootsParams]. | ||
| type InputRequest interface{ isInputRequest() } |
There was a problem hiding this comment.
We usually don't mention that something is an interface, and the word "sealed" is not a standard Go term. Just say what it is. Example:
Params is a parameter (input) type for an MCP call or notification.
| return nil | ||
| } | ||
|
|
||
| // InputResponse is a sealed interface for results that a client sends back |
3 participants
Context
Design (
design/mrtr.md) and implementation proposal for Multi Round-Trip Requests (MRTR) per SEP-2322.Changes
InputRequestandInputResponsesealed interfaces with correspondingInputRequestMapandInputResponseMaptypes for custom map JSON codec implementation.CallToolParams,GetPromptParams, andReadResourceParamswithInputResponses,RequestStateand an unexportedresultType("complete" or "input_required") for retry round-trips. The type is set by the SDK based on input requests presence.