-
Notifications
You must be signed in to change notification settings - Fork 430
feat: add support for custom server notifications #580
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
|
@alexhancock hopefully this is relatively straightforward to review since it mirrors #556 |
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.
LGTM in terms of approach!
But can we unify the CustomClientNotification and CustomServerNotification types into a single CustomNotification type that can be sent either direction? Would cut down on a lot of the duplicated logic we need to implement for each type
|
@alexhancock ah, so with the current implementation, we have these two things: ts_union!(
export type ClientNotification =
| CancelledNotification
| ProgressNotification
| InitializedNotification
| RootsListChangedNotification
| CustomClientNotification;
);ts_union!(
export type ServerNotification =
| CancelledNotification
| ProgressNotification
| LoggingMessageNotification
| ResourceUpdatedNotification
| ResourceListChangedNotification
| ToolListChangedNotification
| PromptListChangedNotification
| CustomServerNotification;
);I didn't realize it was possible to share types across the two (like |
57522cf to
0a1da26
Compare
modelcontextprotocol#556 introduced support for custom client notifications, so this PR makes the complementary change, adding support for custom server notifications. MCP clients, particularly ones that offer "experimental" capabilities, may wish to handle custom server notifications that are not part of the standard MCP specification. This change introduces a new `CustomServerNotification` type that allows a client to process such custom notifications. - introduces `CustomServerNotification` to carry arbitrary methods/params while still preserving meta/extensions; wires it into the `ServerNotification` union and `serde` so `params` can be decoded with `params_as` - allows client handlers to receive custom notifications via a new `on_custom_notification` hook - adds integration coverage that sends a custom server notification end-to-end and asserts the client sees the method and payload Test: ```shell cargo test -p rmcp --features client test_custom_server_notification_reaches_client ```
|
@alexhancock please re-review now that:
|
alexhancock
left a comment
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.
Yeah, looks great now. Thanks!
modelcontextprotocol#580 and modelcontextprotocol#556 introduced support for custom notifications, so this PR takes the next logical step and adds support for custom requests: - Introduce `CustomRequest` and `CustomResult` model types, wire them into the client/server request and result unions, and allow `ClientRequest::method()` to return the dynamic method name. - Implement serde and meta handling for `CustomRequest` so `_meta` is carried through extensions; add default `on_custom_request` handlers that return `METHOD_NOT_FOUND` unless overridden. - Update JSON schema fixtures to include the new request/result shapes and `EmptyObject` strictness. - Add tests for custom request roundtrips and end-to-end client↔server handling, plus a focused integration test in `crates/rmcp/tests/test_custom_request.rs`.
modelcontextprotocol#580 and modelcontextprotocol#556 introduced support for custom notifications, so this PR takes the next logical step and adds support for custom requests: - Introduces `CustomRequest` and `CustomResult` model types, wires them into the client/server request and result unions, and allows `ClientRequest::method()` to return the dynamic method name. - Implements serde and meta handling for `CustomRequest` so `_meta` is carried through extensions; adds default `on_custom_request` handlers that return `METHOD_NOT_FOUND` unless overridden. - Updates JSON schema fixtures to include the new request/result shapes and `EmptyObject` strictness. - Adds tests for custom request roundtrips and end-to-end client↔server handling, plus a focused integration test in `crates/rmcp/tests/test_custom_request.rs`.
modelcontextprotocol#580 and modelcontextprotocol#556 introduced support for custom notifications, so this PR takes the next logical step and adds support for custom requests: - Introduces `CustomRequest` and `CustomResult` model types, wires them into the client/server request and result unions, and allows `ClientRequest::method()` to return the dynamic method name. - Implements serde and meta handling for `CustomRequest` so `_meta` is carried through extensions; adds default `on_custom_request` handlers that return `METHOD_NOT_FOUND` unless overridden. - Updates JSON schema fixtures to include the new request/result shapes and `EmptyObject` strictness. - Adds tests for custom request roundtrips and end-to-end client↔server handling. - Focused integration test in `crates/rmcp/tests/test_custom_request.rs`.
modelcontextprotocol#580 and modelcontextprotocol#556 introduced support for custom notifications, so this PR takes the next logical step and adds support for custom requests: - Introduces `CustomRequest` and `CustomResult` model types, wires them into the client/server request and result unions, and allows `ClientRequest::method()` to return the dynamic method name. - Implements serde and meta handling for `CustomRequest` so `_meta` is carried through extensions; adds default `on_custom_request` handlers that return `METHOD_NOT_FOUND` unless overridden. - Updates JSON schema fixtures to include the new request/result shapes and `EmptyObject` strictness. - Adds tests for custom request roundtrips and end-to-end client↔server handling. - Focused integration test in `crates/rmcp/tests/test_custom_request.rs`.
modelcontextprotocol#580 and modelcontextprotocol#556 introduced support for custom notifications, so this PR takes the next logical step and adds support for custom requests: - Introduces `CustomRequest` and `CustomResult` model types, wires them into the client/server request and result unions, and allows `ClientRequest::method()` to return the dynamic method name. - Implements serde and meta handling for `CustomRequest` so `_meta` is carried through extensions; adds default `on_custom_request` handlers that return `METHOD_NOT_FOUND` unless overridden. - Updates JSON schema fixtures to include the new request/result shapes and `EmptyObject` strictness. - Adds tests for custom request roundtrips and end-to-end client↔server handling. - Focused integration test in `crates/rmcp/tests/test_custom_request.rs`.
modelcontextprotocol#580 and modelcontextprotocol#556 introduced support for custom notifications, so this PR takes the next logical step and adds support for custom requests: - Introduces `CustomRequest` and `CustomResult` model types, wires them into the client/server request and result unions, and allows `ClientRequest::method()` to return the dynamic method name. - Implements serde and meta handling for `CustomRequest` so `_meta` is carried through extensions; adds default `on_custom_request` handlers that return `METHOD_NOT_FOUND` unless overridden. - Updates JSON schema fixtures to include the new request/result shapes and `EmptyObject` strictness. - Adds tests for custom request roundtrips and end-to-end client↔server handling. - Focused integration test in `crates/rmcp/tests/test_custom_request.rs`. For additional testing, I used this locally to update Codex to use a custom request instead of a custom notification so that it gets an "ack" from the MCP server to ensure it has processed the update before sending more messages: openai/codex#8142.
#580 and #556 introduced support for custom notifications, so this PR takes the next logical step and adds support for custom requests: - Introduces `CustomRequest` and `CustomResult` model types, wires them into the client/server request and result unions, and allows `ClientRequest::method()` to return the dynamic method name. - Implements serde and meta handling for `CustomRequest` so `_meta` is carried through extensions; adds default `on_custom_request` handlers that return `METHOD_NOT_FOUND` unless overridden. - Updates JSON schema fixtures to include the new request/result shapes and `EmptyObject` strictness. - Adds tests for custom request roundtrips and end-to-end client↔server handling. - Focused integration test in `crates/rmcp/tests/test_custom_request.rs`. For additional testing, I used this locally to update Codex to use a custom request instead of a custom notification so that it gets an "ack" from the MCP server to ensure it has processed the update before sending more messages: openai/codex#8142.
#556 introduced support for
custom client notifications, so this PR makes the complementary change, adding
support for custom server notifications.
MCP clients, particularly ones that offer "experimental" capabilities,
may wish to handle custom server notifications that are not part of the
standard MCP specification. This change replaces
CustomClientNotificationwith a more general
CustomNotificationtype that can be used in both directions.CustomNotificationcan carry arbitrary methods/params whilestill preserving meta/extensions; wires it into the
ServerNotificationunionand
serdesoparamscan be decoded withparams_ason_custom_notificationhookand asserts the client sees the method and payload
Test:
cargo test -p rmcp --features client test_custom_server_notification_reaches_client