mcp: add SSE session IDs #99
Conversation
ClientSessions and ServerSessions created by SSE have non-empty session IDs. Fixes modelcontextprotocol#98.
| s := &sseClientConn{ | ||
| sseEndpoint: c.sseEndpoint, | ||
| msgEndpoint: msgEndpoint, | ||
| sessionID: msgEndpoint.Query().Get("sessionid"), |
There was a problem hiding this comment.
This is, unfortunately, not part of the spec.
Why do we want to have session IDs for the SSE connection? In theory, there is no way for the client to actually know its session id.
There was a problem hiding this comment.
It was requested in #98, and I obliged.
We could also say that SSE is unsupported, and drop this.
There was a problem hiding this comment.
👋 I filed this bug.
The part of the spec that led me down this session ID path was this: https://modelcontextprotocol.io/docs/concepts/transports#session-management.
Maybe there's another way to approach this (setting or grabbing the header manually) that I should look into?
There was a problem hiding this comment.
I see SSE is deprecated https://modelcontextprotocol.io/docs/concepts/transports#server-sent-events-sse-deprecated so I think this is my bad and I should just be using mcp.NewStreamableHTTPHandler instead of mcp.NewSSEHandler. Sorry for the noise :(
There was a problem hiding this comment.
No problem! Closing this PR.
3 participants
ClientSessions and ServerSessions created by SSE have non-empty session IDs.
Fixes #98.