-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Feature: Non-Blocking call_tool and request state externalisation #1209
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
davemssavage
wants to merge
16
commits into
modelcontextprotocol:main
Choose a base branch
from
davemssavage:feature/call-futures
base: main
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
Feature: Non-Blocking call_tool and request state externalisation #1209
davemssavage
wants to merge
16
commits into
modelcontextprotocol:main
from
davemssavage:feature/call-futures
+1,215
−93
Conversation
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
…a later state and cancelled
…lobal to session rather than per request (read the spec)
…s results in the response being consumed prior to the join, also added a capability that identifies whether the server/transport supports resumption that is passed during initialisation
… timeout on join and subsequent rejoin
… behaviour use None when no result retrieved instead
9 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This patch enables a non-blocking mode of running tools and introduces a RequestStateManager plugin api - this allows for long running tasks to be rejoined on the client and across process restarts.
Motivation and Context
Rather than introducing new protocol messages as per modelcontextprotocol/modelcontextprotocol#617 this patch uses the existing protocol messages with client side changes to allow the client application to submit a call request and then later join that request to get the response and/or any progress notifications that may have occured. This works on top of the existing session resume processes so a client can submit a task in one process and rejoin from another if a persistent version of RequestStateManager is used (this has been tested using a redis implementation - not included in this patch)
Long running tasks currently consume resources indefinitely on the client blocking until they return, Tool calls can currently timeout allowing control to return to the client however this leads to a messy control flow that exposes the internal details of the protocol implementaton - having to check the type of code is httpx.codes.REQUEST_TIMEOUT.
This patch instead introduces new methods request_call_tool and join_call_tool that handle various complex issues with resume tokens and via state stored in a pluggable RequestStateManager. An InMemoryRequestStateManager is provided as the default, other external state managers can be provided to the ClientSession to allow for persistence outside of process memory.
How Has This Been Tested?
This has been unit tested and tested on a server application that managed multiple sessions without blocking the server side threads.
Breaking Changes
None
Types of changes
Checklist
Additional context
In order to handle some complex logic around resumption this patch introduces a new ResumeCapability which can be returned during initialisation. It currently is hard coded to assume that if streamable_http protocol is used then server sessions are resumeable. It may be worth while extending this along the lines of modelcontextprotocol/modelcontextprotocol#617 where servers can indicate how long clients might expect a session to be resumable for.
Testing on a non-trivial application has highlighted that at least one progress notification needs to be sent from the server prior to the start_request call returning a request_id. This is due to needing at least one event to trigger the sse.event_id to be passed back to the client for the resume token to be collected. This is a little opaque and relies on the client having passed a progress callback in the start_call_tool method. A new protocol notification sent from the server to indicate it has received the request could be a viable alternative, however one of the benefits of this is patch over modelcontextprotocol/modelcontextprotocol#617 is it is trying to avoid new protocol messages.