Skip to content

Keep unknown keys on client and server capabilities - #3812

Open
Kludex wants to merge 4 commits into
mainfrom
open-capability-keys
Open

Kludex wants to merge 4 commits into
mainfrom
open-capability-keys

Conversation

@Kludex

@Kludex Kludex commented Nov 3, 2026

Copy link
Copy Markdown
Member

Fixes #3640

ClientCapabilities and ServerCapabilities are documented as "not a closed set", and the schema doesn't forbid extra keys, but both the public models and the generated wire models dropped them. A server couldn't advertise a top-level capability like events (ChatGPT's MCP Events) without patching the serialized result in middleware.

Both capability models now use extra="allow", in the public types and in the wire models (via OPEN_CLASSES in scripts/gen_surface_types.py). Nested known capability objects stay closed.

AI Disclaimer

This PR was developed with the assistance of either Claude or Codex. I've reviewed and verified the changes.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-03T12:16:56.725431Z 153661c PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 5 files

Turn on auto-fix | Re-trigger cubic

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 153661c9ab

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/mcp-types/mcp_types/_types.py Resolved
Comment thread tests/types/test_methods.py Resolved

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, I also checked the generated surface modules for drift from the generator: the extra="allow" counts in _v2025_11_25/__init__.py (8) and _v2026_07_28/__init__.py (10) match the OPEN_CLASSES set sizes that allow_open_class_extras asserts on, and those are the only two surface modules, so no protocol version is left closed. I could not run the generator here, so this is a read of the output, not a regeneration.

Extended reasoning...

The change opens ClientCapabilities and ServerCapabilities to extra keys in the public models, the generator's OPEN_CLASSES, and both generated wire modules, with two new tests. It touches no auth or crypto surface, but it changes observable behaviour of two public 2.x models and what the per-version sieve lets through, which the inline comments cover. Not approved because findings are reported on this run.

One optional note from this repository's REVIEW.md or CLAUDE.md checks was not posted as a comment, over this review's limit for such notes; it is on this commit's check card.

Comment thread scripts/gen_surface_types.py Resolved
Comment thread src/mcp-types/mcp_types/_types.py Resolved
Comment thread tests/types/test_methods.py Resolved
Comment thread src/mcp-types/mcp_types/_types.py Resolved

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 6 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

Turn on auto-fix | Re-trigger cubic

Comment thread docs/migration.md Resolved
@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

📚 Documentation preview

Preview https://pr-3812.mcp-python-docs.pages.dev
Deployment https://8c61e537.mcp-python-docs.pages.dev
Commit 892ec45
Triggered by @Kludex
Updated 2026-11-03 13:35:42 UTC

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No new blocking issues. 1 optional suggestion (a nit or a note on pre-existing code) was found and not posted. Nothing in this review needs a push before merging.

Still open from earlier reviews (2):

  • Unresolved: 2 minor or pre-existing.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review found no issues

No high-confidence issues detected in this change.

Still open from earlier reviews (1):

  • Unresolved: 1 minor or pre-existing.

This branch has not been deployed

No deployments
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.

ServerCapabilities drops unknown keys, so a server can't advertise events for ChatGPT's MCP Events

1 participant