types: Setting default value for jsonrpc

[sreenaths]

The JSON-RPC version is fixed at 2.0 for MCP messages. However, as the value is not set by default, we have to pass the correct value every time a model is instantiated.

Motivation and Context

This change simplifies model instantiation by automatically setting the JSON-RPC version. Additionally, if the version ever needs to change in the future, it only has to be updated in a single place.

For example

JSONRPCMessage(
    root=JSONRPCRequest(
        # jsonrpc="2.0", - This wouldn't be required
        id=1,
        method="ping"
    )
),

How Has This Been Tested?

Tests were improved and are working as expected.

Breaking Changes

No

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• I have read the MCP Documentation
• My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling
• I have added or updated documentation as needed

Additional context

None

[sreenaths]

Hi @ochafik
When you have a chance, please review this PR.
It is similar to PR #1292 that you had helped with before.
Thank you.

[hesreallyhim]

this seems like a move in the right direction but my feeling is - why isn't this just defined at the type that is at the top of the message hierarchy? It's mandatory in every protocol message that goes over the wire. Intuitively, I would expect that to be JSONRPCMessage, I think maybe it's not, but this is the "correct" ontology in my opinion. Whatever type is at the root of all message types (if there is one, which there should be) should have this field set always.

[sreenaths]

Hi @hesreallyhim
Thank you so much for the suggestion. It would be nice to have a common base class for JSONRPCRequest, JSONRPCNotification, JSONRPCResponse, and JSONRPCError.

But, as the original schema is defined in TypeScript, and types.py depends on it... I am not sure if we can change the base structure in Python SDK. Additionally, we might have to use multiple inheritance as JSONRPCRequest already inherits from Request and JSONRPCNotification from Notification.

I hope one of the maintainers can guide us.

\cc @ochafik

[hesreallyhim]

But, as the original schema is defined in TypeScript, and types.py depends on it... I am not sure if we can change the base structure in Python SDK. Additionally, we might have to use multiple inheritance as JSONRPCRequest already inherits from Request and JSONRPCNotification from Notification.

yeah... well i think they've kind of fixed this in the draft spec, but the sdk may still choose to do things a little differently.

[Kludex]

We need to migrate to this: https://github.com/Kludex/mcp/blob/main/src/mcp-types/mcp_types/__init__.py

[ochafik]

Thanks @sreenaths !

[ochafik]

We need to migrate to this: https://github.com/Kludex/mcp/blob/main/src/mcp-types/mcp_types/__init__.py

@Kludex not sure it would support passing the literal values as defaults?

[hesreallyhim]

sorry, perhaps I should have been more direct - i would need to study the codebase more closely to understand how the sdk consumes the spec schema.ts - in the case of the typescript SDK i think they had decided to declare the internal types as not including jsonprc, and adding this in at a "protocol" layer before sending the message over the wire. In any case, the observations made above by @sreenaths, such as:

It would be nice to have a common base class for JSONRPCRequest, JSONRPCNotification, JSONRPCResponse, and JSONRPCError.

was actually just an oversight in the MCP schema.ts (or so I argued) - the schema hierarchy was not type-theoretically correct. I merged a change into the draft so that all requests inherit from JSONRPCRequest, and notifications extend JSONRPCNotification - see modelcontextprotocol/modelcontextprotocol#1026 for more info if interested. I hope this is relevant, sorry if it's not, just flagging that there is an upcoming change surrounding this part of the schema.

[sreenaths]

Thank you for the review, @ochafik.

@hesreallyhim It looks like your changes are one level down the class hierarchy, so I believe our changes are independent.

Thanks, @Kludex. Will mcp_types be included in the Python SDK? As @ochafik mentioned, does mcp_types set literal values as defaults? From what I can see, it only defines the types here: mcp_types

[sreenaths]

Thank you, @felixweinberger
Hope this can be merged soon.