Feature: Non-Blocking call_tool and request state externalisation#1209
Closed
davemssavage wants to merge 21 commits intomodelcontextprotocol:mainmodelcontextprotocol/python-sdk:mainfrom
davemssavage:feature/call-futuresdavemssavage/python-sdk:feature/call-futuresCopy head branch name to clipboard
Closed
Feature: Non-Blocking call_tool and request state externalisation#1209davemssavage wants to merge 21 commits intomodelcontextprotocol:mainmodelcontextprotocol/python-sdk:mainfrom davemssavage:feature/call-futuresdavemssavage/python-sdk:feature/call-futuresCopy head branch name to clipboard
davemssavage wants to merge 21 commits intomodelcontextprotocol:mainmodelcontextprotocol/python-sdk:mainfrom
davemssavage:feature/call-futuresdavemssavage/python-sdk:feature/call-futuresCopy head branch name to clipboard
Conversation
…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
…er doesn't send an event in a reasonable time period
Contributor
|
This seems closely related to modelcontextprotocol/modelcontextprotocol#1391 where there's an ongoing discussion about the correct abstraction for long-running or async tools. In order to support this we'll likely need to align on the right path forward for implementation - if we end up adding this at the protocol layer we'll likely want to leverage any messages provided there. |
Contributor
|
Converting this to draft for now to remove it from the review queue while we wait for the outcome of modelcontextprotocol/modelcontextprotocol#1391 |
Contributor
|
Closing as the SEP this was waiting on (modelcontextprotocol/modelcontextprotocol#1391) has been closed. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.