Repository navigation
fix: parse sampling content sent as an array - #1029
Conversation
Since 2025-11-25 the content of a sampling message may be a single block or an array of blocks. The client parsed the messages of a sampling request, and the stdio and Streamable HTTP sessions parsed the result, only when the content was a single object. An array, which is what a model returns when it calls more than one tool, reached the handler or RequestSampling as []any of maps. Add mcp.ParseSamplingContent, which parses a single block into a Content and an array into a []Content, and use it in all three places.
|
Connected to Huly®: MCP_G-589 |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (1)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (9)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review. WalkthroughSampling content is now parsed into typed content blocks across client requests and stdio and streamable HTTP responses. The parser handles single content objects and arrays. Tests cover parsing, error cases, and sampling paths. ChangesSampling Content Parsing
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change supports typed sampling-content arrays consistently across client and server paths. No merge-blocking issue is identified; merge after normal checks pass. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change normalizes sampling data without adding tool execution or changing the configured handler boundary. Invalid arrays fail as a whole. Risk is low, with remaining uncertainty about how downstream applications authorize and execute newly typed tool blocks. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
* fix(client): don't write transport and trace headers into the caller's header map (mark3labs#1028) * fix(client): don't write transport and trace headers into the caller's header map * test(client): cover caller header isolation on streamable HTTP and tracing * fix(server): preserve typed JSON-RPC handler errors (mark3labs#1027) Co-authored-by: Fedor Bushlia <fedorbush@yandex-team.ru> * fix(server): send empty arrays instead of null in results (mark3labs#989) tools, prompts, tasks, completion values, prompt messages and resource contents are required arrays, but a nil slice reaching them went out as null: from a tool or prompt filter that hides everything, from tasks/list before any task has run, and from completion providers and prompt or resource handlers that return nil. The schema rejects null there, and so do clients that validate responses. listByPagination now returns an empty slice instead of nil, which covers every list result and supersedes the resources/list guard from mark3labs#665. completion/complete, prompts/get and resources/read fill in an empty array the same way. The prompt handler's result is copied rather than modified, and a result asking for input is left as it is. * fix(server): reject missing client capabilities with invalid params (mark3labs#990) A request declaring protocol version 2026-07-28 without io.modelcontextprotocol/clientCapabilities in _meta was answered with -32021 MissingRequiredClientCapability, and one with a malformed value with -32020 HeaderMismatch. The spec treats a request missing a required _meta field as malformed and requires -32602 Invalid params (400 on HTTP, which is unchanged). -32021 is for a request that needs a capability the client did not declare, and carries data.requiredCapabilities; -32020 is for HTTP header mismatches. * fix(server): send URLElicitationRequiredError from handlers as -32042 (mark3labs#992) * fix(server): send URLElicitationRequiredError from handlers as -32042 The elicitation docs and example have a tool return mcp.URLElicitationRequiredError to tell the client to complete a URL elicitation first. Handler errors were all reported as INTERNAL_ERROR with only the message, though, so the client got -32603 without the elicitations, and mcp-go's own client could not decode it as a URLElicitationRequiredError. For clients before protocol version 2026-07-28, report it as -32042 with data.elicitations, as 2025-11-25 defines. 2026-07-28 reserves the code and asks for URL elicitations through multi round-trip requests, so modern clients still get an internal error. Only this error type is recognized; any other handler error is an internal error as before. * fix(server): also recognise a pointer to URLElicitationRequiredError URLElicitationRequiredError has value receivers, so a handler can return a pointer to one as well. errors.As with a value target does not match the pointer, and the client got an internal error without the elicitations. * fix(streamable-http): deliver notifications on modern subscriptions/listen streams (mark3labs#993) A 2026-07-28 client opens subscriptions/listen to receive notifications such as tools/list_changed. Over Streamable HTTP each modern request is served by an ephemeral, unregistered session, and broadcasts only reach registered sessions, so a listen stream got its acknowledgement and then nothing, while the acknowledgement promised the requested types. Make the listen stream's session reachable by broadcasts while the request runs, through a registry of its own so that no session hooks fire. Delivering to it needs two more things the spec asks of a subscription stream: the HTTP session now records the filter subscriptions/listen sets, and until that happens (and after it is cleared) it receives nothing; and every notification on the stream carries the subscription ID in _meta. * fix(mcp): parse an input_required tool result that has no content (mark3labs#994) A tools/call that needs more input is answered, in protocol version 2026-07-28, with an InputRequiredResult: resultType, inputRequests and requestState, and no content. ParseCallToolResult rejected any result without content before looking at resultType, so against servers that send that shape (the other SDKs, and the conformance reference server) CallTool failed with "content is missing" and the multi round-trip retry never ran. mcp-go's own server sends "content": [] alongside, which is why this did not show between mcp-go peers. Accept a missing content when resultType is input_required, as ParseReadResourceResult already does for contents. A complete result without content is still rejected. * fix(client): exclude tools with invalid x-mcp-header annotations (mark3labs#996) SEP-2243 requires a client using Streamable HTTP to reject a tool whose x-mcp-header annotations break its constraints (empty, non-primitive or number type, duplicate names, characters outside the header token set) by leaving it out of the tools/list result, and SHOULD log a warning. The client listed and cached every tool, so it would call such a tool and mirror headers from it. ListToolsByPage now drops those tools on modern HTTP connections, including transports wrapped for logging, and logs the tool name and reason. A definition cached from an earlier listing is forgotten, so the tool is not called with headers mirrored from it. Other transports may ignore the annotations, and connections before 2026-07-28 do not mirror them, so both are unchanged. * test(client): remove the mock server's build cache after building it (mark3labs#998) compileTestServer gives each build of the mock stdio server a fresh GOCACHE, which mark3labs#241 added to work around a flaky linker error in parallel builds, but never removes it. Each build leaves about 120 MB in the temp directory: one run of go test ./client/... left 16 of them, 1.9 GB. Remove the cache once the build is done, and report a failure to create it instead of quietly building in the shared cache. * fix(server): report handlers that return no result as internal errors (mark3labs#999) A handler returning nil, nil left the server with nothing to send: - prompts/get dereferenced the nil result in the generated dispatcher and panicked. Over streamable HTTP the connection dropped with no response; with the in-process client the panic reached the caller. - tools/call answered with a null result, which no client accepts. - Run as a task, the tool completed with a nil result, and a later tasks/result panicked reading it. Report an internal error for prompts and tools, and fail the task. The checks run after any legacy multi round-trip retry, so a retry that returns nil is caught too. * fix(sse): send a recovered panic's error with the request's id (mark3labs#1000) The SSE message handler recovers a panicking handler and queues an INTERNAL_ERROR so the client doesn't hang, but it built the response with a nil id. Clients match responses to pending requests by id; the mcp-go SSE client treats an id-less message as a notification, so the call still waited for its timeout. Use the id of the request that panicked, as the stdio transport already does. A notification gets no reply at all, so when its handler panics nothing is queued. HandleMessage decides what is a notification the same way, by the missing id. * fix(mcp): accept an Mcp-Param header sent with an empty value (mark3labs#1001) An empty string argument annotated with x-mcp-header is mirrored as a header with an empty value; GenerateParamHeaders produces exactly that. ValidateParamHeaders takes a getHeader func that returns "" for a missing header, so it cannot tell the two apart and rejected the call with "missing header for parameter". A 2026-07-28 client passing an empty string could not call such a tool at all. Add ValidateParamHeadersLookup, whose lookup also reports presence, and use it in the server with http.Header.Values. ValidateParamHeaders keeps its behavior and delegates to it. An empty header for an absent argument is still ignored, as before. * fix(stdio): recover handler panics on the read loop (mark3labs#1002) The tool call workers and handleRequest recover a panicking handler and answer with INTERNAL_ERROR, but two paths still call HandleMessage on the read loop without that protection: notifications, and a tools/call run synchronously because the worker queue is full. A panic in either propagated out of Listen and ended the process, where every other request just got an error. Recover there too, with a handleMessage helper that handleRequest now shares. A request still gets an INTERNAL_ERROR carrying its id; a notification gets no response, as JSON-RPC requires. * fix(oauth): reject authorization server metadata for another issuer (mark3labs#1004) After protected resource metadata names an authorization server, the client builds the well-known URLs from that issuer and uses the first document it gets, whatever issuer the document declares. RFC 8414 §3.3 and OpenID Connect Discovery §4.3 say the issuer in the document must be identical to the one the URL was built from, and the 2026-07-28 MCP authorization spec repeats it: if they differ, the client must not use the metadata. Otherwise a server could send the client to one authorization server's endpoints under another's name. Reject such metadata with an error instead of using it or falling back to default endpoints. A final trailing slash is ignored, as in the official Go SDK. OAuthConfig.SkipIssuerMetadataValidation turns the check off for authorization servers known to publish a mismatched issuer, like the TypeScript SDK's skipIssuerMetadataValidation. Metadata from an explicit AuthServerMetadataURL, which the caller chose, and from the legacy discovery without protected resource metadata is left unchecked, so as not to break servers that rely on it. * fix(oauth): authenticate at the token endpoint the way the server supports (mark3labs#1005) A confidential client always sent its client_id and client_secret in the token request body (client_secret_post), whatever the authorization server said. A server that only accepts HTTP Basic (client_secret_basic) rejected every code exchange and refresh, and the method returned by dynamic client registration was ignored. Pick the method per request, for the code exchange and the refresh: 1. OAuthConfig.TokenEndpointAuthMethod, new, for pre-registered or saved credentials on servers that support more than one method. 2. The token_endpoint_auth_method from the registration response (RFC 7591 §3.2.1). GetTokenEndpointAuthMethod returns it, so it can be saved with the client ID and secret. 3. HTTP Basic when the server's metadata lists client_secret_basic but not client_secret_post. 4. Otherwise the secret in the body, as before, which also covers servers that list nothing. A client registered with "none" sends only its client_id. Basic credentials are form-encoded first, as RFC 6749 §2.3.1 requires, and are not repeated in the body. * fix(client): resume an SSE response stream the server ends early (mark3labs#1010) Before protocol version 2026-07-28, a server that answers a POST with an SSE stream may end the response once it has sent an event ID, before the JSON-RPC response, and the client should poll by reconnecting. It must wait the time given by the server's retry field, and it resumes with a GET that carries Last-Event-ID (SEP-1699). The client ignored the id and retry fields and failed the call with "unexpected nil response". Record the last dispatched event ID and the retry time while reading the stream. When a POST stream ends without the response, on a session before 2026-07-28 and after an event ID, wait the retry time (one second without one, and at least 10ms) and reopen the stream with a GET carrying Last-Event-ID, until the response arrives or the context ends. Each resumed connection is closed as soon as it ends. A GET the server refuses fails the call with the reason, and a stream that breaks off instead of ending still fails it, as before. Protocol version 2026-07-28 removed this mechanism, so modern sessions are unchanged. * fix(client): don't answer a server's request with null (mark3labs#1011) The client answers sampling, elicitation and roots requests with what its handler returns. A handler that returned no result was answered with a null result, and a roots handler with no roots, returning &mcp.ListRootsResult{}, with "roots": null. The schema requires an object and an array, so servers that validate the answer reject it; the TypeScript SDK fails with "expected object, received null" and "expected array, received null". mcp-go's own Streamable HTTP server takes a null result for an empty response, answers 202 and drops it, so its RequestSampling or RequestElicitation waits until its context ends. Report a missing result as an error, as the multi round-trip path already does, and send an empty roots array, there too. The in-process client passed a nil result straight to server code, which now gets the same error. * fix(server): run a session's own tool when it is called as a task (mark3labs#1012) handleToolCall looks a tool up in the session's tools before the server's, and sends a call with task params to handleTaskAugmentedToolCall. That looked the tool up again in the server's tools only. A session tool that supports tasks was reported as not found when called as one. A session tool that shadows a server tool of the same name ran the server tool's handler instead when the server tool supports tasks too, and failed with -32601 when it doesn't. Look up the session's tools first there too. * fix(oauth): share one token refresh among concurrent callers (mark3labs#1013) getValidToken refreshed the token for every caller that found it expired. Callers that did so at the same time, such as requests a client sends in parallel after the token ran out, all sent the same refresh token. The MCP spec requires authorization servers to rotate refresh tokens for public clients, so a server accepts only the first of those requests: every other caller got ErrOAuthAuthorizationRequired and was sent back to authorize, although a new token was stored a moment later. A server that detects the reuse may also revoke the tokens the first request got. Let callers share the refresh in flight, and start one only if none is. The refresh runs on its own context with a timeout, not the context of the caller that started it, so that a caller giving up after the server spent the refresh token doesn't leave the others to send it again. It reads the store first, so a caller that saw the expired token just before another refresh finished uses the new token. Callers waiting on a refresh stop when their context ends, and all of them get its result, failure included, instead of retrying one after another. * fix(oauth): discover the authorization server again after a failure (mark3labs#1014) getServerMetadata runs discovery through a sync.Once and keeps whatever it ends with, an error included. Discovery runs with the context of the first caller that needs it, so when it ended with an error, because that caller's context ended, a metadata request failed on the network, or a configured AuthServerMetadataURL answered 5xx, every later call got the same error back. Refreshing a token and starting authorization both need the metadata, so the handler stayed unusable until a 401 carrying a resource metadata URL reset it, which a server that doesn't send one never does. Keep the metadata a discovery found, but not a failure: report it to the callers that waited on that discovery, then let the next call discover again, with no error left over from the last one. * fix(oauth): send a token without a type as a bearer token (mark3labs#1015) GetAuthorizationHeader builds the header from the token's type and access token, normalizing any case of "bearer" to "Bearer". A token with no type produced " <token>", which net/http sends as the bare token, without the Bearer scheme servers expect. That happens when an application puts a token in the store itself, a custom TokenStore doesn't keep the type, or an authorization server leaves token_type out of a response. Send such a token as a bearer token, which is how MCP authorization says access tokens are sent (Authorization: Bearer <access-token>). * fix(client): cancel requests over stdio when their context ends (mark3labs#1016) When a request's context ended before the response, the client gave up waiting and returned the context's error, but told the server nothing. Over stdio there is no per-request stream to close, so the server kept working on the request, a tool call for example, for as long as it took. Protocol version 2026-07-28 says a client on stdio must send notifications/cancelled referencing the request to cancel it, and earlier versions offer the same notification for it. Send notifications/cancelled for a request whose context ends after it was sent, on transports that aren't HTTP, once the connection is initialized. It isn't sent for initialize or the server/discover probe, which must not be cancelled, for task-augmented requests, which tasks/cancel is for, or over HTTP, where 2026-07-28 makes the end of the request's stream the signal and earlier versions keep their current behaviour. A request whose context has already ended isn't sent at all. The notification goes out from its own goroutine with a timeout, so a peer that doesn't read can't hold up the caller. * fix(sse): wait for room in the event queue instead of dropping a response (mark3labs#1017) When a session's event queue was full, handleMessage dropped the response to the message it had handled, and the client waited until its timeout. The queue fills when events are produced faster than the client reads them, such as a tool sending notifications in a loop. The error sent after a recovered panic had the same fallback. Both now wait for room, or for the session to end. handleSSE closes the session's done channel on every exit, including a panic, so a waiting response can't outlive its stream. * fix(client): support re-initializing after the server ends the session (mark3labs#1018) When the server answers 404 for the session, the transport returns ErrSessionTerminated and the caller is expected to call Initialize again. Two things got in the way. The GET stream stayed on the old session, or stopped for good, so the client got no notifications for the new one. And when the GET stream saw the 404 first, the transport dropped the session ID and later requests went out without one, which some servers reject or treat as the start of another session, so the caller never learned that it had to re-initialize. After a 404 for the session, requests, notifications and responses now fail with ErrSessionTerminated until a request that starts a new connection succeeds: initialize, or server/discover on 2026-07-28. The GET stream waits for the new session and moves to it. A 404 for a GET stream that never opened on the current session means the server doesn't offer the stream: the listener stops, as before, but the session goes on. * fix(client): follow pagination cursors when listing tasks (mark3labs#1020) * fix(client): follow pagination cursors when listing tasks ListTasks issued one tasks/list request and returned that page. The other list helpers follow NextCursor, and the server paginates tasks when a limit is set, so every task after the first page was dropped. ListTasksByPage keeps the single-page call. ListTasks now walks the cursor the same way ListTools does. * fix(client): stop ListTasks when a cursor repeats A server that returns a cursor already requested would keep ListTasks issuing tasks/list until the caller canceled the context. * fix(mcp): omit empty completion context arguments (mark3labs#1024) CompleteContext.Arguments had no omitempty, so a completion request without context arguments was sent as "context":{"arguments":null}. The spec defines arguments as an optional object, and servers that validate it, such as those built on the TypeScript SDK, reject null with -32602. Omit the field when it is empty, as GetPromptParams.Arguments already does. * perf(server): touch a known session without allocating (mark3labs#1026) With WithSessionIdleTTL set, touchSession runs on every request and called sync.Map.LoadOrStore with a freshly allocated atomic.Int64, which also boxed the string key: 2 allocs (24 B) per request even when the session was already tracked. Look the session up with Load first and fall back to LoadOrStore only on a session's first touch. A known session now costs 0 allocs; a first touch is unchanged. A new test pins 0 allocs on a known session. BenchmarkTouchSessionKnown, AMD Ryzen 9 9950X, go1.27.0 windows/amd64, default GC, -count 1 run six times per side, old and new alternated: old new sec/op 29.10n +- 1% 12.20n +- 1% -58.09% (p=0.002 n=6) B/op 24.00 +- 0% 0.00 +- 0% -100.00% (p=0.002 n=6) allocs/op 2.000 +- 0% 0.000 +- 0% -100.00% (p=0.002 n=6) * fix: parse sampling content sent as an array (mark3labs#1029) Since 2025-11-25 the content of a sampling message may be a single block or an array of blocks. The client parsed the messages of a sampling request, and the stdio and Streamable HTTP sessions parsed the result, only when the content was a single object. An array, which is what a model returns when it calls more than one tool, reached the handler or RequestSampling as []any of maps. Add mcp.ParseSamplingContent, which parses a single block into a Content and an array into a []Content, and use it in all three places. * fix(server): reject an unsupported MCP-Protocol-Version header (mark3labs#1030) * fix(server): reject an unsupported MCP-Protocol-Version header The Streamable HTTP transport requires a server that receives an invalid or unsupported MCP-Protocol-Version header to respond with 400 Bad Request. Requests from modern clients are already validated, but on the legacy path the header was never read: a POST, GET or DELETE with an unknown version such as 2025-01-01 was served as usual. Reject such requests with 400 when the header names a version this SDK does not implement. Initialize is not checked, since it negotiates the version, and a missing header is still accepted for clients on 2025-03-26. * fix(server): check the protocol version before accepting a response A POST carrying a ping response or an empty response returned 202 before the MCP-Protocol-Version check ran, so an unsupported header was accepted on that path. Detect the era and run the check right after parsing the body, ahead of those early returns. * fix(mcp): keep structuredContent on tool_result content (mark3labs#1031) * fix(mcp): keep structuredContent on tool_result content The 2025-11-25 schema gives ToolResultContent an optional structuredContent object next to content, like CallToolResult has. ToolResultContent had no such field, so a server could not send it in a sampling request, and UnmarshalContent and ParseContent dropped it when decoding one. Add the field and read it in both decoders. * fix(mcp): keep the original structuredContent bytes on tool_result Decoding structuredContent into an any turns numbers into float64, so an integer above 2^53 changed value on a decode and encode round trip. CallToolResult avoids this with RawStructuredContent; do the same for ToolResultContent and write the raw bytes back when marshaling. * fix(client): reject repeated pagination cursors on list APIs (mark3labs#1033) Servers that echo the same nextCursor would otherwise loop forever when aggregating tools, resources, prompts, and related iterators. * test(conformance): add official MCP server suite to CI (mark3labs#1038) * test(conformance): add official MCP server suite to CI * fix(conformance): harden suite validation * fix(conformance): parse timestamp suffixes by format --------- Co-authored-by: Omid Mirzaei <omidomirzaei@gmail.com> * fix: restore SSE resume and protocol-aware handler errors - resume SSE streams without starting a new session - honor protocol-aware handler errors and preserve wrapped context * fmt * conformance: the line passes the notification scenarios upstream expects to fail * conformance: accept an empty server baseline --------- Co-authored-by: Serhii Zghama <20826225+serhiizghama@users.noreply.github.com> Co-authored-by: FlameHost10 <147077735+FlameHost10@users.noreply.github.com> Co-authored-by: Fedor Bushlia <fedorbush@yandex-team.ru> Co-authored-by: Captain <42566883+po-et@users.noreply.github.com> Co-authored-by: Omid <86771298+o-mid@users.noreply.github.com> Co-authored-by: Hamed Yousefi <hdyousefi@gmail.com> Co-authored-by: Peter Bednarčík <29061766+pbednarcik@users.noreply.github.com> Co-authored-by: mika <spacexstarship01@outlook.com> Co-authored-by: Omid Mirzaei <omidomirzaei@gmail.com> Co-authored-by: Ed Zynda <ezynda3@gmail.com>
Description
Since 2025-11-25, the content of a sampling message can be a single block or an array of blocks:
An array is what a model returns when it calls more than one tool. From the sampling page:
The spec's own example answers with two
tool_useblocks, and the follow-up request sends the twotool_resultblocks back in one user message.mcp-go only parses the single-object form. The other form is left as
[]anyofmap[string]any:sampling/createMessagerequest only whencontentis a map. A sampling handler gets typed content for the first message of the spec example and[]interface {}for the next two.RequestSamplingreturns aCreateMessageResultwhoseContentis[]interface {}, so a tool loop on the server can't type-switch onToolUseContent.This adds
mcp.ParseSamplingContent. It parses a single block withParseContent, as before, and an array into a[]mcp.Content. Any other value, such as content that is already typed, comes back unchanged. The client and both sessions now call it in place of their own map check. TheSamplingMessage.Contentcomment now mentions[]Content.Single-object content is parsed exactly as before. An array with an entry that isn't a valid content block now fails with an error such as
content[1]: unsupported content type: video, the same way a single invalid block already did.Type of Change
Checklist
MCP Spec Compliance
Additional Information
Tests:
TestClient_HandleSamplingRequestArrayContentsends the follow-up request from the spec example throughhandleIncomingRequestand checks the messages the handler receives.TestStdioSessionSamplingResponseArrayContentandTestStreamableHTTPServer_SamplingArrayContentanswer a sampling request with the twotool_useblocks from the spec example and check that the result holds a[]mcp.Content.TestParseSamplingContentcovers the helper, including the error cases.The client and session tests fail on
mainwith[]interface {}in place of the typed content.go test -race ./mcp ./client ./serverandgo test ./...pass, andgolangci-lint runreports no issues.Summary by CodeRabbit