[codex] Preserve disabled MCP servers across runtime overlays#27414
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1db1d33d1
ℹ️ 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".
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn runtime_overlay_preserves_disabled_server() -> TestResult { |
There was a problem hiding this comment.
Cover disabled Apps tools through a Codex turn
This only asserts effective_servers, so it can pass even if a Codex turn still sends codex_apps tools in /responses. Because this overlay changes agent MCP tool exposure, add a core/suite test_codex turn asserting disabled Apps tools are absent from the model request, per guidance.
Useful? React with 👍 / 👎.
Why
Recent MCP runtime overlay changes replace same-name configured server entries with compatibility or extension-provided configs. Those replacement configs default to enabled, so an MCP server explicitly configured with
enabled = falsecould be initialized anyway.The connection manager still filters disabled servers correctly, but the configured disabled state was lost before initialization reached that filter.
What changed
enabled = falsefor those servers after overlays, while leaving all other overlay fields andRemoveprecedence unchanged.codex_appsserver.Testing
just fmtjust test -p codex-mcp-extensionjust fix -p codex-corejust fix -p codex-mcp-extensionThe full workspace
just testsuite was not run.