Skip to content

Enforce String or Integer non-null ID for MCP JSON-RPC Messages with a new McpSchema subclass #401

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

Closed
wants to merge 2 commits into from

Conversation

ZachGerman
Copy link

@ZachGerman ZachGerman commented Jul 15, 2025

I considered JSONRPCMessageID, but didn't want the JSONRPC prefix as it might imply nullability or possibly a float, as is standard for a JSON-RPC id.
I also considered id but figured it was too ambiguous.
I can see an argument for McpMessageId, but I figured it being in the McpSchema namespace was good enough for someone to identify the MessageId type as specific to MCP.

Motivation and Context

The specification conditions for JSON-RPC Message id values include the requirements of the value being a (non-null) String or Integer.

How Has This Been Tested?

Multiple existing tests use this field, have been updated accordingly, and confirmed to work correctly after the changes.

Breaking Changes

N/A

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

Edit: After sleeping on it, I realized this IS a breaking change, because people are possibly building requests in tool handlers without the requirements this would enforce.

@tzolov
Copy link
Contributor

tzolov commented Jul 16, 2025

@ZachGerman if you just want to validate the request ID this looks unnecessarily complicated?

Wouldn't something like this achieve the same:

	@JsonInclude(JsonInclude.Include.NON_ABSENT)
	@JsonIgnoreProperties(ignoreUnknown = true)
	public record JSONRPCRequest( // @formatter:off
		@JsonProperty("jsonrpc") String jsonrpc,
		@JsonProperty("method") String method,
		@JsonProperty("id") Object id,
		@JsonProperty("params") Object params) implements JSONRPCMessage { // @formatter:on

		/**
		 * Constructor that validates MCP-specific ID requirements. Unlike base JSON-RPC,
		 * MCP requires that: (1) Requests MUST include a string or integer ID; (2) The ID
		 * MUST NOT be null
		 */
		public JSONRPCRequest {
			Assert.notNull(id, "MCP requests MUST include an ID - null IDs are not allowed");
			Assert.isTrue(id instanceof String || id instanceof Integer || id instanceof Long,
					"MCP requests MUST have an ID that is either a string or integer");
		}
	}

@tzolov
Copy link
Contributor

tzolov commented Jul 16, 2025

@ZachGerman here is how I would address this requirement. Let me know if i'm missing something:
#403

@ZachGerman
Copy link
Author

ZachGerman commented Jul 16, 2025

@tzolov nice catch on the Long case!

@ZachGerman here is how I would address this requirement. Let me know if i'm missing something: #403

I think it's great. I just wanted to be sure the folks writing tools are informed of the rules conflicting with base JSON-RPC somewhere along the lines when confronting the types they use to write their tool. In hindsight, this was a bit like taking a flamethrower to an anthill 😅.

@ZachGerman ZachGerman closed this Jul 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants