-
Notifications
You must be signed in to change notification settings - Fork 507
mcp: expose streamable HTTP request summaries #1101
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
CaliLuke
wants to merge
5
commits into
modelcontextprotocol:main
Choose a base branch
from
CaliLuke:issue-1076-request-summary
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.
+496
−19
Open
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
8c2d626
mcp: expose streamable HTTP request summaries
CaliLuke ce596f3
Merge branch 'main' into issue-1076-request-summary
guglielmo-san 4ffd153
mcp: limit request summaries to single messages
CaliLuke 912ed54
Merge branch 'main' into issue-1076-request-summary
CaliLuke 3ac7ea1
Merge branch 'main' into issue-1076-request-summary
guglielmo-san File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -69,6 +69,27 @@ type sessionInfo struct { | |
| timer *time.Timer | ||
| } | ||
|
|
||
| // StreamableHTTPRequestSummary contains redacted metadata about a decoded | ||
| // streamable HTTP POST body. | ||
| type StreamableHTTPRequestSummary struct { | ||
| // Methods contains the methods of decoded JSON-RPC requests in body order. | ||
| // The methods have not yet been validated and may contain arbitrary | ||
| // attacker-controlled strings. The slice does not alias SDK state and may be | ||
| // retained or modified by the callback. | ||
| Methods []string | ||
|
|
||
| // RequestID is valid only when the decoded input contains exactly one | ||
| // JSON-RPC call and no other messages. IDs use the same coercion rules as | ||
| // [jsonrpc.DecodeMessage]. | ||
| RequestID jsonrpc.ID | ||
|
|
||
| // Calls, Notifications, and Responses count the decoded JSON-RPC message | ||
| // kinds in the body. | ||
| Calls int | ||
| Notifications int | ||
| Responses int | ||
| } | ||
|
|
||
| // startPOST signals that a POST request for this session is starting (which | ||
| // carries a client->server message), pausing the session timeout if it was | ||
| // running. | ||
|
|
@@ -218,6 +239,23 @@ type StreamableHTTPOptions struct { | |
| // Requests using older protocol versions (including those routed through | ||
| // the allowsessionsinstateless compatibility path) are unaffected. | ||
| PropagateRequestCancellation bool | ||
|
|
||
| // OnRequestSummary, when non-nil, observes redacted metadata for streamable | ||
| // HTTP POST bodies decoded by the session transport. The callback receives | ||
| // the HTTP request's context and runs synchronously before validation and | ||
| // dispatch of the decoded messages. It may be called concurrently for | ||
| // different requests and should return promptly; in particular, it must not | ||
| // wait for processing of the same request. Panics are not recovered. Only the | ||
| // summary is redacted; the context may contain values added by authentication | ||
| // or other middleware. | ||
| // | ||
| // The callback is not invoked for requests rejected before this point, | ||
| // including HTTP, authorization, session-routing, and connection failures, | ||
| // even if connection setup previously inspected the body. Use HTTP middleware | ||
| // to observe those failures. Use [Server.AddReceivingMiddleware] to observe | ||
| // dispatched messages; this callback additionally observes decoded messages | ||
| // that are rejected before dispatch without exposing their parameters. | ||
| OnRequestSummary func(context.Context, StreamableHTTPRequestSummary) | ||
| } | ||
|
|
||
| // DefaultMaxRequestBodyBytes is the default value used for | ||
|
|
@@ -422,14 +460,8 @@ func (h *StreamableHTTPHandler) serveStateless(w http.ResponseWriter, req *http. | |
| } | ||
| } | ||
|
|
||
| transport := &StreamableServerTransport{ | ||
| SessionID: sessionID, | ||
| Stateless: true, | ||
| EventStore: h.opts.EventStore, | ||
| jsonResponse: h.opts.JSONResponse, | ||
| logger: h.opts.Logger, | ||
| shouldPropagateCancellation: info.usesNewProtocol && (info.isSubscriptionsListen || h.opts.PropagateRequestCancellation), | ||
| } | ||
| transport := h.newStreamableServerTransport(sessionID, true) | ||
| transport.shouldPropagateCancellation = info.usesNewProtocol && (info.isSubscriptionsListen || h.opts.PropagateRequestCancellation) | ||
|
|
||
| session, err := connectStreamable(req.Context(), server, transport, info.opts) | ||
| if err != nil { | ||
|
|
@@ -534,6 +566,17 @@ func connectStreamable(ctx context.Context, server *Server, transport *Streamabl | |
| return s, nil | ||
| } | ||
|
|
||
| func (h *StreamableHTTPHandler) newStreamableServerTransport(sessionID string, stateless bool) *StreamableServerTransport { | ||
| return &StreamableServerTransport{ | ||
| SessionID: sessionID, | ||
| Stateless: stateless, | ||
| EventStore: h.opts.EventStore, | ||
| jsonResponse: h.opts.JSONResponse, | ||
| logger: h.opts.Logger, | ||
| onRequestSummary: h.opts.OnRequestSummary, | ||
| } | ||
| } | ||
|
|
||
| // serveStateful handles requests for stateful servers. | ||
| // Stateful servers support GET, POST, and DELETE, and maintain persistent | ||
| // sessions keyed by session ID. | ||
|
|
@@ -652,13 +695,7 @@ func (h *StreamableHTTPHandler) serveStatefulPOST(w http.ResponseWriter, req *ht | |
| } | ||
| sessionID = server.opts.GetSessionID() | ||
|
|
||
| transport := &StreamableServerTransport{ | ||
| SessionID: sessionID, | ||
| Stateless: false, | ||
| EventStore: h.opts.EventStore, | ||
| jsonResponse: h.opts.JSONResponse, | ||
| logger: h.opts.Logger, | ||
| } | ||
| transport := h.newStreamableServerTransport(sessionID, false) | ||
|
|
||
| // Sessions without a session ID (GetSessionID returned "") are ephemeral: | ||
| // there's no way to address them, so they are closed after the request. | ||
|
|
@@ -832,6 +869,10 @@ type StreamableServerTransport struct { | |
| // [streamableServerConn]. See its docstring. | ||
| shouldPropagateCancellation bool | ||
|
|
||
| // onRequestSummary is forwarded from StreamableHTTPOptions for transports | ||
| // created by StreamableHTTPHandler. | ||
| onRequestSummary func(context.Context, StreamableHTTPRequestSummary) | ||
|
|
||
| // connection is non-nil if and only if the transport has been connected. | ||
| connection *streamableServerConn | ||
| } | ||
|
|
@@ -848,6 +889,7 @@ func (t *StreamableServerTransport) Connect(ctx context.Context) (Connection, er | |
| jsonResponse: t.jsonResponse, | ||
| logger: ensureLogger(t.logger), // see #556: must be non-nil | ||
| shouldPropagateCancellation: t.shouldPropagateCancellation, | ||
| onRequestSummary: t.onRequestSummary, | ||
| incoming: make(chan jsonrpc.Message, 10), | ||
| done: make(chan struct{}), | ||
| streams: make(map[string]*stream), | ||
|
|
@@ -876,10 +918,11 @@ func (t *StreamableServerTransport) SupportsProtocolVersion(version string) bool | |
| } | ||
|
|
||
| type streamableServerConn struct { | ||
| sessionID string | ||
| stateless bool | ||
| jsonResponse bool | ||
| eventStore EventStore | ||
| sessionID string | ||
| stateless bool | ||
| jsonResponse bool | ||
| eventStore EventStore | ||
| onRequestSummary func(context.Context, StreamableHTTPRequestSummary) | ||
|
|
||
| // shouldPropagateCancellation is true when the underlying HTTP request's | ||
| // lifetime IS the connection's cancellation signal (e.g., a stateless | ||
|
|
@@ -1437,6 +1480,10 @@ func (c *streamableServerConn) servePOST(w http.ResponseWriter, req *http.Reques | |
| http.Error(w, fmt.Sprintf("malformed payload: %v", err), http.StatusBadRequest) | ||
| return | ||
| } | ||
| singleMessage := isSingleStreamableMessage(incoming, isBatch) | ||
| if c.onRequestSummary != nil { | ||
| c.onRequestSummary(req.Context(), summarizeStreamableHTTPRequest(incoming)) | ||
| } | ||
|
|
||
| protocolVersion := protocolVersionFromContext(req.Context()) | ||
| if protocolVersion == "" { | ||
|
|
@@ -1582,7 +1629,7 @@ func (c *streamableServerConn) servePOST(w http.ResponseWriter, req *http.Reques | |
| } | ||
|
|
||
| // Validate MCP standard headers (Mcp-Method, Mcp-Name, Mcp-Param-*) | ||
| if !isBatch && len(incoming) == 1 { | ||
| if singleMessage { | ||
| if err := validateMcpHeaders(req.Header, incoming[0], c.toolLookup); err != nil { | ||
| resp := &jsonrpc.Response{ | ||
| Error: jsonrpc2.NewError(CodeHeaderMismatch, err.Error()), | ||
|
|
@@ -1737,6 +1784,33 @@ func (c *streamableServerConn) servePOST(w http.ResponseWriter, req *http.Reques | |
| c.hangResponse(req.Context(), done) | ||
| } | ||
|
|
||
| func summarizeStreamableHTTPRequest(incoming []jsonrpc.Message) StreamableHTTPRequestSummary { | ||
| var summary StreamableHTTPRequestSummary | ||
| for _, msg := range incoming { | ||
| switch msg := msg.(type) { | ||
| case *jsonrpc.Request: | ||
| summary.Methods = append(summary.Methods, msg.Method) | ||
| if msg.IsCall() { | ||
| summary.Calls++ | ||
| } else { | ||
| summary.Notifications++ | ||
| } | ||
| case *jsonrpc.Response: | ||
| summary.Responses++ | ||
| } | ||
| } | ||
| if len(incoming) == 1 { | ||
| if req, ok := incoming[0].(*jsonrpc.Request); ok && req.IsCall() { | ||
| summary.RequestID = req.ID | ||
| } | ||
| } | ||
| return summary | ||
| } | ||
|
|
||
| func isSingleStreamableMessage(incoming []jsonrpc.Message, isBatch bool) bool { | ||
| return !isBatch && len(incoming) == 1 | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i don't think it's worth the helper
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in 4ffd153: removed the helper and inlined the single-message condition at its two call sites. |
||
|
|
||
| // Event IDs: encode both the logical connection ID and the index, as | ||
| // <streamID>_<idx>, to be consistent with the typescript implementation. | ||
|
|
||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Batching is a deprecated feature, I would not add the support for it in the newly added StreamableHTTPRequestSummary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 4ffd153: the public summary now represents one JSON-RPC message with singular fields, and the callback is not invoked for deprecated batch bodies.