design: mrtr implementation proposal#950
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. |
| ``` | ||
|
|
||
| `InputRequests` and `RequestState` fields are added directly to `CallToolResult`, `GetPromptResult`, and `ReadResourceResult` as exported. | ||
| Result type discriminator (completed, input_required) is unexported so that SDK users don't need to set it to the correct constant in addition to setting either `Content` or `InputRequests`. Handler execution result is validated and augmented before marshaling: |
There was a problem hiding this comment.
How would the error be propagated to the tool developer?
|
|
||
| An unexported middleware is installed in the client, which similarly to `urlElicitationMiddleware` will automatically invoke handlers for the corresponding methods on incomplete results and retry the original request. `ClientOptions` will be extended with configuration knobs: | ||
| ```go | ||
| type MRTROptions struct { |
There was a problem hiding this comment.
I'm not sure I would introduce these knobs from the start. If the user need is real, someone will come to use and ask about it. We can always add them later in a backwards compatible manner.
|
|
||
| // RequireInput constructs a tool call, prompt or resource result with input requests set. | ||
| // mrtrResult provides methods for setting private fields on these types. | ||
| func RequireInput[T any, TP interface { *T; mrtrResult }](r InputRequiredResult) TP { ... } |
There was a problem hiding this comment.
We can always introduce this even with exported fields, as a "util" that guarantees correctness. But likely not from the start.
|
|
||
| ### Protocol version bridging | ||
|
|
||
| When a handler returns an input-required result to an old client the SDK could bridge by invoking `ServerSession.Elicit`/`CreateMessage`/`ListRoots` on the `ServerSession` and re-invoking the handler with the collected `inputResponses`: |
There was a problem hiding this comment.
I think this behavior would be valuable. Without it, the server creators are not incentivized to use the new MRTR format because migrating would mean they will stop working with all older clients. It would slow down the adoption of the new protocol version.
| } | ||
| ``` | ||
|
|
||
| Or even more radically: instead of making `ServerSession.Elicit`, `CreateMessage`, and `ListRoots` return errors for newer clients, we could convert these invocations into MRTR wire format transparently: suspend the handler, return `input_required`, and resume when the client retries with `inputResponses`: |
There was a problem hiding this comment.
I agree that this should not be done.
2 participants
This PR presents a proposal for implementing Multi Round-Trip Requests (MRTR) as defined in SEP-2322.
In the new protocol version servers can't initiate requests to clients, but when a server requires additional input for completing
tools/call,prompts/get, orresources/readit can return an incomplete result along with a set ofinputRequests. The client fulfills them locally and retries the same call withinputResponsesattached.