feat: make built-in MCPs first-class runtime servers#21356
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e71090ea16
ℹ️ 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".
Co-authored-by: Codex <noreply@openai.com>
|
@codex review |
💡 Codex Reviewcodex/codex-rs/builtin-mcps/Cargo.toml Line 18 in c53fd0a This commit changes Rust dependencies/features, including adding ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Codex is wrong... |
DISCLAIMER
This is experimental and no production service must rely on this
Why
Built-in MCPs are product-owned runtime capabilities, but they were previously flattened into the same config-backed stdio path as user-configured servers. That made them depend on a hidden
codex builtin-mcpre-exec path, exposed them through config-oriented CLI flows, and erased distinctions the runtime needs to preserve—most notably whether an MCP call should count as external context for memory-mode pollution.What changed
BuiltinMcpServerandEffectiveMcpServer.builtin-mcpstdio subcommand.codex mcp list/get/login/logoutscoped to configured servers, while merging built-ins only into the effective runtime server set.memoriesis now classified as local Codex state rather than external context.Test plan
cargo test -p codex-mcpcargo test -p codex-core --test suite builtin_memories_mcp_call_does_not_mark_thread_memory_mode_polluted_when_configured