Fix response result deserialization to use the original request method (#601) - #999
Open
remote-controlled-man wants to merge 1 commit into
Conversation
modelcontextprotocol#601) An incoming JSON-RPC response's result was decoded by JSON shape alone, so a response whose payload matches another type's shape (e.g. a tasks/result payload shaped like CallToolResult) was completed with the wrong runtime type and failed at the erased generic cast in Protocol.request. - Capture the raw result JSON while decoding JSONRPCResponse on the wire path, keeping the public JSONRPCResponse API unchanged. - At the request/response correlation point, decode the raw result with the deserializer declared by the original request's method, falling back to the shape-decoded result for custom methods, programmatically constructed responses, and task-augmented calls returning CreateTaskResult. - Rewrite the SSE replay message id in the raw JSON before decoding in StreamableHttpClientTransport instead of copying the decoded message, so the raw result survives. - Results whose shape matches no known type now surface as GetTaskPayloadResult (the raw-payload result type) instead of failing the whole message decode.
This branch has not been deployed
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
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.
Closes #601.
What
Response results were deserialized by JSON shape (
RequestResultPolymorphicSerializer) before the response was correlated with its original request. A response whose payload resembled another result type could be decoded as the wrong runtime type and then fail at the erased-generic completion cast withClassCastException(e.g.tasks/resultvstasks/getpayloads with overlapping shapes). The Streamable HTTP SSE replay path additionally dropped the raw result JSON viamsg.copy(id = …), making recovery impossible there.How
resultJSON during wire decode:JSONRPCResponse.rawResultis internal and@Transient, set at the single polymorphic funnel (JSONRPCMessagePolymorphicSerializer's response branch) that every transport decodes through.request<T>signature; the value being cast now already has the declared runtime type.decodeIncomingMessage) instead of copying an already shape-decoded message, at both SSE sites.Intentional behavior change
A response whose result matches no known shape now surfaces as
GetTaskPayloadResultand reaches correlation, so a type-mismatched caller fails fast viacompleteExceptionallyinstead of the whole message decode failing and the request hanging until timeout.Tests
ProtocolResultDeserializationTest(core, commonTest): on the base, thetasks/result/tasks/getoverlapping-shape cases fail withClassCastExceptionand an unknown-shape payload fails the whole message decode (RED); after the fix all 7 pass (GREEN), with controls for ping,tools/call, task-augmentedtools/call, and direct-consumer decode.StreamableHttpClientTransportReplayTest(client, jvmTest): end-to-end replay throughProtocol+StreamableHttpClientTransport+ MockEngine inline SSE with a server-side id rewritten to the POST id — RED on base, GREEN after.:kotlin-sdk-core:jvmTest604/604,:kotlin-sdk-client:jvmTest79/79,jsTest(core 535, client 66) and a JVM sweep of core/client/server/testing/umbrella all pass;jvmApiCheck,ktlintCheck, anddetektpass — the public API is unchanged.apiCheck/build/check(native/Apple targets) andconformance-test/TS integration suites (requirenpx); the 30:integration-test:jvmTestfailures are pre-existing environmentalnpxabsence, unchanged from the base.The correlation-based direction follows the maintainer's review guidance on #913; #717 and #734 explored earlier approaches. Developed with AI assistance; I have reviewed the change and take responsibility for it.