fix(core): preserve non-app MCP tools in apps mode#11841
Open
karel-openai wants to merge 1 commit intomainfrom
Open
fix(core): preserve non-app MCP tools in apps mode#11841karel-openai wants to merge 1 commit intomainfrom
karel-openai wants to merge 1 commit intomainfrom
Conversation
Contributor
|
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
| &mcp_tools, | ||
| selected_mcp_tools, | ||
| apps_mcp_tools, | ||
| selected_tool_names.is_none(), |
Contributor
There was a problem hiding this comment.
I think we should include non-apps MCP tools regardless of the current selection.
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.
Summary
This fixes a regression where enabling
Feature::Appscould hide non-codex_appsMCP tools (e.g. Notion MCP write tools) from the model tool list.In
built_tools, the apps path rebuiltmcp_toolsfrom:codex_appstools.When there was no MCP tool selection yet, this effectively dropped non-
codex_appsservers from model-visible tools.Root Cause
built_toolsapplied apps filtering by constructing a reduced MCP map and assigning it back tomcp_tools.That reduced map only guaranteed
codex_appstool inclusion, so tools from other MCP servers could disappear even though those servers were started and healthy.Fix
merge_apps_mode_mcp_tools(...)to merge app-filtered tools with selected tools.codex_appsMCP tools when there is no explicit MCP tool selection.Tests
Added unit tests in
codex-rs/core/src/codex.rs:apps_mode_keeps_non_codex_apps_mcp_tools_without_selectionapps_mode_respects_explicit_tool_selectionRan:
cargo test -p codex-core apps_mode_keeps_non_codex_apps_mcp_tools_without_selection -- --nocapturecargo test -p codex-core apps_mode_respects_explicit_tool_selection -- --nocaptureUser Impact
With apps enabled, MCP tools from non-app servers (including Notion MCP write tools) remain callable unless explicitly filtered by selection semantics.