feat: expose streamable HTTP session ID#893
Conversation
github-actions
Bot
added
T-test
DaleSeo
left a comment
DaleSeo
left a comment
There was a problem hiding this comment.
Thanks, @0xWeakSheep! Sorry for the delay in reviewing this. I left some comments. I think we should fix the SemVer check failure before merging, unless the breaking change is intentional.
| join_handle: Option<tokio::task::JoinHandle<Result<(), WorkerQuitReason<W::Error>>>>, | ||
| _drop_guard: tokio_util::sync::DropGuard, | ||
| ct: CancellationToken, | ||
| session_id_handle: Option<TransportSessionIdHandle>, |
There was a problem hiding this comment.
The SemVer check flags this as a breaking change because it removes the auto traits. The simplest fix is probably to require TransportSessionIdProvider to preserve them.
|
|
||
| /// Cloneable read-only handle for the negotiated streamable HTTP session ID. | ||
| #[derive(Debug, Clone, Default)] | ||
| pub struct StreamableHttpClientSession { |
There was a problem hiding this comment.
This looks like an implementation detail.
| pub struct StreamableHttpClientSession { | |
| struct StreamableHttpClientSession { |
|
|
||
| /// Returns a handle for reading the current transport session ID, when the | ||
| /// transport protocol has one. | ||
| fn session_id_handle(&self) -> Option<TransportSessionIdHandle> { |
There was a problem hiding this comment.
Most of the existing handle names in this project refer to task or runtime handles. How about calling it session_id_observer() or session_id_provider() instead so that the API can communicate that this value is read-only more clearly?
2 participants
Closes #863.
This adds a way to read the negotiated
Mcp-Session-Idfrom streamable HTTP clients.The session ID is now available from:
StreamableHttpClientTransport::session_id()StreamableHttpClientTransport::session_id_handle()RunningService::session_id()The value is set after
initialize, updated if the client transparently re-initializes after an expired session, and cleared when the worker exits.I also added test coverage that checks the exposed session ID matches the session stored by the server.
Checks run: