fix(shared): strip envelope fields when forwarding JSONRPCRequest#2573
Open
charles-adedotun wants to merge 1 commit into
Open
Conversation
BaseSession.send_request constructs a JSONRPCRequest as:
JSONRPCRequest(jsonrpc="2.0", id=request_id, **request_data)
When request_data came from model_dump() on an existing JSONRPCRequest
(forwarding/proxy scenarios — e.g. a ServerSession receiving a request
and re-emitting it through a ClientSession), the dump preserves the
envelope fields jsonrpc and id, which then collide with the explicit
kwargs above and raise:
TypeError: got multiple values for keyword argument 'jsonrpc'
Fix: pop jsonrpc and id from request_data before the unpack. Added a
regression test that exercises the forwarding pattern via
create_client_server_memory_streams.
Fixes modelcontextprotocol#2548
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
1 participant
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.
Summary
Fixes #2548.
BaseSession.send_requestbuilds the outgoing wire object as:When
request_dataoriginates frommodel_dump()on an existingJSONRPCRequest(a typical forwarding/proxy scenario — e.g. aServerSessionreceiving an incoming request and re-emitting it via aClientSession), the dump preserves the envelope fieldsjsonrpcandid, which collide with the explicit kwargs above:Approach
Conservative: pop
jsonrpcandidfromrequest_databefore unpacking. This keeps the wire-envelope construction authoritative at this call site and doesn't change any model-layer dump behavior (which would have wider blast radius).As mentioned in the issue thread, I'm happy to take the model-layer approach instead (e.g. excluding envelope fields from
model_dumpon forwarded requests) if the maintainers prefer that — let me know.Test
Added
test_send_request_strips_envelope_fields_when_forwardingintests/shared/test_session.pythat reproduces the bug (test fails without the fix, passes with it) and verifies clean forwarding end-to-end with a mock server.Checklist
tests/shared/test_session.pysuite passes (9/9)tests/suite passes (1173 passed, 98 skipped, 1 xfailed)ruff format/ruff checkcleanpyrightclean (0 errors, 0 warnings)