mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Use tool search for MCP tools by default (#29486)
## Why MCP tools were only placed behind `tool_search` when a feature flag was enabled or when there were at least 100 tools. That made the model's tool flow depend on both rollout configuration and the number of installed tools. The searched-tool flow is now the intended behavior. Making it unconditional when the model and provider support it gives every supported setup the same behavior and lets us retire the feature flag safely. ## What changed - Defer all effective MCP tools when `tool_search` and namespaced tools are supported. - Keep exposing MCP tools directly when search cannot be used, so older or unsupported model/provider combinations still work. - Mark `tool_search_always_defer_mcp_tools` as removed and ignore old configured values. - Keep plugin filtering, app-only filtering, file handling, and MCP calls working through the searched-tool flow. ## Why many tests changed Many tests used to act as if the model could see MCP tools in its first request and call them immediately. That is no longer the real flow: the model first receives `tool_search`, searches for a tool, receives the matching MCP tool, and then calls it in the next request. The tests therefore needed an extra search step, and checks for tool names, descriptions, and input fields had to move from the first request to the search result. These are not separate product changes; they make the tests follow what the model will actually see after this change. The plugin tests still check which tools are allowed and where they came from, the file tests still check upload fields and behavior, and the MCP round-trip test still checks a successful call from start to finish. ## Tests - `just test -p codex-features` - Focused `codex-core` tests for MCP exposure and tool planning - `just test -p codex-core explicit_plugin_mentions` - `just test -p codex-core stdio_server_round_trip` - Focused `codex-core` tests for tool search, app-only tools, and MCP file uploads
This commit is contained in:
committed by
GitHub
Unverified
parent
4a82ecc3c9
commit
c53b1dae09
@@ -2,7 +2,6 @@ use std::collections::HashSet;
|
||||
|
||||
use codex_connectors::AppToolPolicyEvaluator;
|
||||
use codex_connectors::AppToolPolicyInput;
|
||||
use codex_features::Feature;
|
||||
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
|
||||
use codex_mcp::ToolInfo as McpToolInfo;
|
||||
use codex_mcp::tool_is_model_visible;
|
||||
@@ -11,8 +10,6 @@ use tracing::instrument;
|
||||
use crate::config::Config;
|
||||
use crate::connectors;
|
||||
|
||||
pub(crate) const DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD: usize = 100;
|
||||
|
||||
pub(crate) struct McpToolExposure {
|
||||
pub(crate) direct_tools: Vec<McpToolInfo>,
|
||||
pub(crate) deferred_tools: Option<Vec<McpToolInfo>>,
|
||||
@@ -34,13 +31,7 @@ pub(crate) fn build_mcp_tool_exposure(
|
||||
));
|
||||
}
|
||||
|
||||
let should_defer = search_tool_enabled
|
||||
&& (config
|
||||
.features
|
||||
.enabled(Feature::ToolSearchAlwaysDeferMcpTools)
|
||||
|| deferred_tools.len() >= DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD);
|
||||
|
||||
if !should_defer {
|
||||
if !search_tool_enabled {
|
||||
return McpToolExposure {
|
||||
direct_tools: deferred_tools,
|
||||
deferred_tools: None,
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
use std::collections::HashSet;
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_features::Feature;
|
||||
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
|
||||
use codex_mcp::ToolInfo;
|
||||
use codex_tools::ToolName;
|
||||
@@ -95,12 +94,12 @@ fn with_visibility(mut tool: ToolInfo, visibility: &[&str]) -> ToolInfo {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn directly_exposes_small_effective_tool_sets() {
|
||||
async fn directly_exposes_effective_tool_sets_when_search_is_unavailable() {
|
||||
let config = test_config().await;
|
||||
let mcp_tools = numbered_mcp_tools(DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD - 1);
|
||||
let mcp_tools = numbered_mcp_tools(/*count*/ 2);
|
||||
|
||||
let exposure = build_mcp_tool_exposure(
|
||||
&mcp_tools, /*connectors*/ None, &config, /*search_tool_enabled*/ true,
|
||||
&mcp_tools, /*connectors*/ None, &config, /*search_tool_enabled*/ false,
|
||||
);
|
||||
|
||||
assert_eq!(tool_names(&exposure.direct_tools), tool_names(&mcp_tools));
|
||||
@@ -237,9 +236,9 @@ enabled = true
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn searches_large_effective_tool_sets() {
|
||||
async fn defers_effective_tool_sets_when_search_is_available() {
|
||||
let config = test_config().await;
|
||||
let mcp_tools = numbered_mcp_tools(DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD);
|
||||
let mcp_tools = numbered_mcp_tools(/*count*/ 2);
|
||||
|
||||
let exposure = build_mcp_tool_exposure(
|
||||
&mcp_tools, /*connectors*/ None, &config, /*search_tool_enabled*/ true,
|
||||
@@ -249,17 +248,13 @@ async fn searches_large_effective_tool_sets() {
|
||||
let deferred_tools = exposure
|
||||
.deferred_tools
|
||||
.as_ref()
|
||||
.expect("large tool sets should be discoverable through tool_search");
|
||||
.expect("MCP tools should be discoverable through tool_search");
|
||||
assert_eq!(tool_names(deferred_tools), tool_names(&mcp_tools));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn always_defer_feature_defers_apps_too() {
|
||||
let mut config = test_config().await;
|
||||
config
|
||||
.features
|
||||
.enable(Feature::ToolSearchAlwaysDeferMcpTools)
|
||||
.expect("test config should allow feature update");
|
||||
async fn defers_apps_and_non_app_mcp_tools() {
|
||||
let config = test_config().await;
|
||||
let mcp_tools = vec![
|
||||
make_mcp_tool(
|
||||
"rmcp",
|
||||
|
||||
@@ -326,7 +326,7 @@ fn hosted_model_tool_specs(context: &CoreToolPlanContext<'_>) -> Vec<ToolSpec> {
|
||||
}
|
||||
|
||||
pub(crate) fn search_tool_enabled(turn_context: &TurnContext) -> bool {
|
||||
turn_context.model_info.supports_search_tool
|
||||
turn_context.model_info.supports_search_tool && namespace_tools_enabled(turn_context)
|
||||
}
|
||||
|
||||
pub(crate) fn tool_suggest_enabled(turn_context: &TurnContext) -> bool {
|
||||
@@ -820,12 +820,11 @@ fn add_collaboration_tools(context: &CoreToolPlanContext<'_>, planned_tools: &mu
|
||||
} else {
|
||||
let agent_type_description =
|
||||
agent_type_description(turn_context, context.default_agent_type_description);
|
||||
let exposure =
|
||||
if search_tool_enabled(turn_context) && namespace_tools_enabled(turn_context) {
|
||||
ToolExposure::Deferred
|
||||
} else {
|
||||
ToolExposure::Direct
|
||||
};
|
||||
let exposure = if search_tool_enabled(turn_context) {
|
||||
ToolExposure::Deferred
|
||||
} else {
|
||||
ToolExposure::Direct
|
||||
};
|
||||
planned_tools.add_with_exposure(
|
||||
SpawnAgentHandler::new(SpawnAgentToolOptions {
|
||||
available_models: turn_context.available_models.clone(),
|
||||
@@ -943,7 +942,7 @@ fn append_tool_search_executor(
|
||||
planned_tools: &mut PlannedTools,
|
||||
) {
|
||||
let turn_context = context.turn_context;
|
||||
if !(search_tool_enabled(turn_context) && namespace_tools_enabled(turn_context)) {
|
||||
if !search_tool_enabled(turn_context) {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -990,7 +989,6 @@ fn append_extension_tool_executors(
|
||||
reserved_tool_names.insert(ToolName::plain(codex_code_mode::WAIT_TOOL_NAME));
|
||||
}
|
||||
if search_tool_enabled(turn_context)
|
||||
&& namespace_tools_enabled(turn_context)
|
||||
&& planned_tools
|
||||
.runtimes()
|
||||
.iter()
|
||||
|
||||
Reference in New Issue
Block a user