-
Notifications
You must be signed in to change notification settings - Fork 119
fix: Use JSON for stdio, similiar to the HTTP Transport and use EJSON deserialize when necessary MCP-207 #571
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
Conversation
….deserialize when necessary We can't leverage using EJSON as the underlying transport because it diverges from the HTTP Transport, and because not all EJSON is equivalent to it's underlying JSON. For example, take this sample into consideration: ```js $> JSON.stringify({ long: 23098479807234 }) <= {"long":23098479807234} $> EJSON.stringify({ long: 23098479807234 }) // relaxed = true <= {"long":23098479807234} $> EJSON.stringify({ long: 23098479807234 }, { relaxed: false }) <= "long":{"$numberLong":"23098479807234"}} ``` This behaviour forbids us on using `{relaxed: false}` always, as it breaks the shape of the underlying JSON. Moreover, if we use dates, it gets worse, as we lose type info in the protocol when serialising/deserializing. The approach here is to use a custom `zod` type that leverages on EJSON.deserialize, a function that reads a plain EJSON object and transforms it to the underlying types (for example, $date to js.Date). This is important because the Node.js driver does not work with EJSON, it works with actual JS objects (like Date), so by using this approach documents and queries using non-primitive types like BinData or Date work as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR transitions from using EJSON transport to JSON for stdio communication, while adding EJSON deserialization for MongoDB operations. The change ensures compatibility with the HTTP transport while maintaining proper handling of BSON types like ObjectId and Date through a custom Zod type that leverages EJSON.deserialize.
- Removed custom EJSON transport layer and reverted to standard JSON stdio transport
- Added zEJSON helper function for proper BSON type handling in tool arguments
- Updated integration tests to use proper EJSON format for MongoDB operations
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/transports/stdio.ts | Removes custom EJSON transport implementation and reverts to standard StdioServerTransport |
src/tools/args.ts | Adds zEJSON helper function for EJSON deserialization in Zod schemas |
src/tools/mongodb/*/*.ts | Updates tool argument schemas to use zEJSON for proper BSON type handling |
tests/unit/transports/stdio.test.ts | Removes tests for custom EJSON transport functionality |
tests/integration/**/*.ts | Updates test cases to use proper EJSON format and parsing |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
arguments: { | ||
database: integration.randomDbName(), | ||
collection: "foo", | ||
filter: { _id: fooObject._id }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test was misleading, because everything runs in the same process, there was no proper ObjectID Serde, it was using the real JavaScript types. This not happens in the usual scenario where the ID has to be serialized.
This change ensures that the ID is properly serialized and sent in EJSON.
📊 Accuracy Test Results📈 Summary
📎 Download Full HTML Report - Look for the Report generated on: 9/18/2025, 4:53:47 PM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the detailed write-up and the elegant solution, looks great 💯
Proposed changes
We can't leverage using EJSON as the underlying transport because it diverges from the HTTP Transport, and because not all EJSON is equivalent to it's underlying JSON. For example, take this sample into consideration:
This behaviour forbids us on using
{relaxed: false}
always, as it breaks the shape of the underlying JSON. Moreover, if we use dates, it gets worse, as we lose type info in the protocol when serialising/deserializing.The approach here is to use a custom
zod
type that leverages on EJSON.deserialize, a function that reads a plain EJSON object and transforms it to the underlying types (for example, $date to js.Date).This is important because the Node.js driver does not work with EJSON, it works with actual JS objects (like Date), so by using this approach documents and queries using non-primitive types like BinData or Date work as expected.
It tackles the following issues:
Checklist