Repository navigation
Conversation
Add the optional securitySchemes field and a matching Tool builder for clients that consume draft SEP-1488 metadata. Omit the field when unset and retain separate application metadata. The declarations describe authentication requirements and do not change credential or permission enforcement. Verify legacy payload compatibility, noauth and OAuth scope round trips, default constructors, typed tool catalogs, and trait-based tool construction. Focused tests pass. Rust 1.98 Clippy reports the same 13 result_large_err diagnostics on the unmodified 3.8.0 base; no unrelated lint changes are included.
github-actions
Bot
added
T-test
Contributor
|
@gocamille is on triage rotation this week and is the reviewer for this one. Labels come from the PR labeler. Please review the change. Automated triage. Labels are a first pass and the assigned maintainer has the final say. |
starsy
changed the title
starsy
marked this pull request as ready for review
October 27, 2026 18:34
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
ChatGPT-compatible MCP tools need to advertise top-level
securitySchemes. The current typedToolmodel cannot emit this field, and deserialization drops it. Applications therefore need a local SDK patch to preserve the declaration.This change adds optional
Tool.security_schemesand a matching builder. It preservesnoauthand OAuth scope objects through typed tool catalogs. Existing constructors leave the field unset, so tools without this extension retain their JSON representation. Application_metaremains independent.The field follows draft SEP-1488. It describes authentication requirements only. It does not validate credentials, change permissions, or alter transport behavior. This implements interoperability support while the proposal remains a draft.
Verification on macOS ARM64 with Rust 1.98.0:
cargo fmt --all --checkpasses. The existing nightly-only formatting options produce warnings under stable.server,transport-io) and client-only feature checks pass without compiler warnings.cargo clippy -p rmcp --lib --features client -- -D warningsreports 13result_large_errdiagnostics. The same command reports the same 13 locations on unmodified 3.8.0. This PR includes no unrelated lint fixes. The upstream pinned 1.96 toolchain and nightly formatter were not installed for this local check.The patch is based on
rmcp-v3.8.0(cd88056ffdc97756fbd98e5a12bbeb41758f0ce8).Design discussion: SEP-1488 already discusses this wire field. This draft proposes the narrow SDK change for review. Please advise if the SDK should expose general descriptor extensions instead while the SEP remains a draft.
An existing downstream integration uses this field for ChatGPT custom MCP connections. OpenAI's authenticated server example also emits it. No downstream application code or credentials are included.