Repository navigation
Conversation
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. |
There was a problem hiding this comment.
💡 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".
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
📚 Documentation preview
|
Fixes #3640
ClientCapabilitiesandServerCapabilitiesare 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 likeevents(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 (viaOPEN_CLASSESinscripts/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.