fix: change codex/sandbox-state/update from a notification to a request #8142
+98
−62
Conversation
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
bolinfest
added a commit
to bolinfest/rust-sdk
that referenced
this pull request
Dec 16, 2025
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.
alexhancock
pushed a commit
to modelcontextprotocol/rust-sdk
that referenced
this pull request
Dec 18, 2025
#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.
bolinfest
added a commit
that referenced
this pull request
Dec 18, 2025
Version `0.12.0` includes modelcontextprotocol/rust-sdk#590, which I will use in #8142. Changes: - `rmcp::model::CustomClientNotification` was renamed to `rmcp::model::CustomNotification` - a bunch of types have a `meta` field now, but it is `Option`, so I added `meta: None` to a bunch of things
bolinfest
force-pushed
the
pr8142
branch
2 times, most recently
from
fd36082 to
b6ec022
Compare
gpeal
approved these changes
Dec 18, 2025
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Historically,
accept_elicitation_for_prompt_rule()was flaky because we were using a notification to update the sandbox followed by ashelltool request that we expected to be subject to the new sandbox config, but because rmcp MCP servers delegate each incoming message to a new Tokio task, messages are not guaranteed to be processed in order, so sometimes theshelltool call would run before the notification was processed.Prior to this PR, we relied on a generous
sleep()between the notification and the request to reduce the change of the test flaking out.This PR implements a proper fix, which is to use a request instead of a notification for the sandbox update so that we can wait for the response to the sandbox request before sending the request to the
shelltool call. Previously,rmcpdid not support custom requests, but I fixed that in modelcontextprotocol/rust-sdk#590, which made it into the0.12.0release (see #8288).This PR updates
shell-tool-mcpto expect"codex/sandbox-state/update"as a request instead of a notification and sends the appropriate ack. Note this behavior is tied to our customcodex/sandbox-statecapability, which Codex honors as an MCP client, which is whycore/src/mcp_connection_manager.rshad to be updated as part of this PR, as well.This PR also updates the docs at
shell-tool-mcp/README.md.