mirror of
https://github.com/openai/codex.git
synced 2026-04-24 14:45:27 +00:00
feat: drop MCP managing tools if no MCP servers (#11900)
Drop MCP tools if no MCP servers to save context For this https://github.com/openai/codex/issues/11049
This commit is contained in:
@@ -4997,14 +4997,13 @@ async fn built_tools(
|
||||
skills_outcome: Option<&SkillLoadOutcome>,
|
||||
cancellation_token: &CancellationToken,
|
||||
) -> CodexResult<Arc<ToolRouter>> {
|
||||
let mut mcp_tools = sess
|
||||
.services
|
||||
.mcp_connection_manager
|
||||
.read()
|
||||
.await
|
||||
let mcp_connection_manager = sess.services.mcp_connection_manager.read().await;
|
||||
let has_mcp_servers = mcp_connection_manager.has_servers();
|
||||
let mut mcp_tools = mcp_connection_manager
|
||||
.list_all_tools()
|
||||
.or_cancel(cancellation_token)
|
||||
.await?;
|
||||
drop(mcp_connection_manager);
|
||||
|
||||
let mut effective_explicitly_enabled_connectors = explicitly_enabled_connectors.clone();
|
||||
effective_explicitly_enabled_connectors.extend(sess.get_connector_selection().await);
|
||||
@@ -5050,12 +5049,12 @@ async fn built_tools(
|
||||
|
||||
Ok(Arc::new(ToolRouter::from_config(
|
||||
&turn_context.tools_config,
|
||||
Some(
|
||||
has_mcp_servers.then(|| {
|
||||
mcp_tools
|
||||
.into_iter()
|
||||
.map(|(name, tool)| (name, tool.tool))
|
||||
.collect(),
|
||||
),
|
||||
.collect()
|
||||
}),
|
||||
app_tools,
|
||||
turn_context.dynamic_tools.as_slice(),
|
||||
)))
|
||||
|
||||
@@ -353,6 +353,10 @@ pub(crate) struct McpConnectionManager {
|
||||
}
|
||||
|
||||
impl McpConnectionManager {
|
||||
pub(crate) fn has_servers(&self) -> bool {
|
||||
!self.clients.is_empty()
|
||||
}
|
||||
|
||||
pub async fn initialize(
|
||||
&mut self,
|
||||
mcp_servers: &HashMap<String, McpServerConfig>,
|
||||
|
||||
@@ -1478,12 +1478,14 @@ pub(crate) fn build_specs(
|
||||
builder.register_handler("shell_command", shell_command_handler);
|
||||
}
|
||||
|
||||
builder.push_spec_with_parallel_support(create_list_mcp_resources_tool(), true);
|
||||
builder.push_spec_with_parallel_support(create_list_mcp_resource_templates_tool(), true);
|
||||
builder.push_spec_with_parallel_support(create_read_mcp_resource_tool(), true);
|
||||
builder.register_handler("list_mcp_resources", mcp_resource_handler.clone());
|
||||
builder.register_handler("list_mcp_resource_templates", mcp_resource_handler.clone());
|
||||
builder.register_handler("read_mcp_resource", mcp_resource_handler);
|
||||
if mcp_tools.is_some() {
|
||||
builder.push_spec_with_parallel_support(create_list_mcp_resources_tool(), true);
|
||||
builder.push_spec_with_parallel_support(create_list_mcp_resource_templates_tool(), true);
|
||||
builder.push_spec_with_parallel_support(create_read_mcp_resource_tool(), true);
|
||||
builder.register_handler("list_mcp_resources", mcp_resource_handler.clone());
|
||||
builder.register_handler("list_mcp_resource_templates", mcp_resource_handler.clone());
|
||||
builder.register_handler("read_mcp_resource", mcp_resource_handler);
|
||||
}
|
||||
|
||||
builder.push_spec(PLAN_TOOL.clone());
|
||||
builder.register_handler("update_plan", plan_handler);
|
||||
@@ -1832,9 +1834,6 @@ mod tests {
|
||||
for spec in [
|
||||
create_exec_command_tool(true),
|
||||
create_write_stdin_tool(),
|
||||
create_list_mcp_resources_tool(),
|
||||
create_list_mcp_resource_templates_tool(),
|
||||
create_read_mcp_resource_tool(),
|
||||
PLAN_TOOL.clone(),
|
||||
create_request_user_input_tool(),
|
||||
create_apply_patch_freeform_tool(),
|
||||
@@ -2039,7 +2038,7 @@ mod tests {
|
||||
features,
|
||||
web_search_mode,
|
||||
});
|
||||
let (tools, _) = build_specs(&tools_config, Some(HashMap::new()), None, &[]).build();
|
||||
let (tools, _) = build_specs(&tools_config, None, None, &[]).build();
|
||||
let tool_names = tools.iter().map(|t| t.spec.name()).collect::<Vec<_>>();
|
||||
assert_eq!(&tool_names, &expected_tools,);
|
||||
}
|
||||
@@ -2106,6 +2105,53 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mcp_resource_tools_are_hidden_without_mcp_servers() {
|
||||
let config = test_config();
|
||||
let model_info =
|
||||
ModelsManager::construct_model_info_offline_for_tests("gpt-5-codex", &config);
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::CollaborationModes);
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
features: &features,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
});
|
||||
let (tools, _) = build_specs(&tools_config, None, None, &[]).build();
|
||||
|
||||
assert!(
|
||||
!tools.iter().any(|tool| matches!(
|
||||
tool.spec.name(),
|
||||
"list_mcp_resources" | "list_mcp_resource_templates" | "read_mcp_resource"
|
||||
)),
|
||||
"MCP resource tools should be omitted when no MCP servers are configured"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mcp_resource_tools_are_included_when_mcp_servers_are_present() {
|
||||
let config = test_config();
|
||||
let model_info =
|
||||
ModelsManager::construct_model_info_offline_for_tests("gpt-5-codex", &config);
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::CollaborationModes);
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
features: &features,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
});
|
||||
let (tools, _) = build_specs(&tools_config, Some(HashMap::new()), None, &[]).build();
|
||||
|
||||
assert_contains_tool_names(
|
||||
&tools,
|
||||
&[
|
||||
"list_mcp_resources",
|
||||
"list_mcp_resource_templates",
|
||||
"read_mcp_resource",
|
||||
],
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_build_specs_gpt5_codex_default() {
|
||||
let mut features = Features::with_defaults();
|
||||
@@ -2116,9 +2162,6 @@ mod tests {
|
||||
Some(WebSearchMode::Cached),
|
||||
"shell_command",
|
||||
&[
|
||||
"list_mcp_resources",
|
||||
"list_mcp_resource_templates",
|
||||
"read_mcp_resource",
|
||||
"update_plan",
|
||||
"request_user_input",
|
||||
"apply_patch",
|
||||
@@ -2138,9 +2181,6 @@ mod tests {
|
||||
Some(WebSearchMode::Cached),
|
||||
"shell_command",
|
||||
&[
|
||||
"list_mcp_resources",
|
||||
"list_mcp_resource_templates",
|
||||
"read_mcp_resource",
|
||||
"update_plan",
|
||||
"request_user_input",
|
||||
"apply_patch",
|
||||
@@ -2162,9 +2202,6 @@ mod tests {
|
||||
&[
|
||||
"exec_command",
|
||||
"write_stdin",
|
||||
"list_mcp_resources",
|
||||
"list_mcp_resource_templates",
|
||||
"read_mcp_resource",
|
||||
"update_plan",
|
||||
"request_user_input",
|
||||
"apply_patch",
|
||||
@@ -2186,9 +2223,6 @@ mod tests {
|
||||
&[
|
||||
"exec_command",
|
||||
"write_stdin",
|
||||
"list_mcp_resources",
|
||||
"list_mcp_resource_templates",
|
||||
"read_mcp_resource",
|
||||
"update_plan",
|
||||
"request_user_input",
|
||||
"apply_patch",
|
||||
@@ -2208,9 +2242,6 @@ mod tests {
|
||||
Some(WebSearchMode::Cached),
|
||||
"shell_command",
|
||||
&[
|
||||
"list_mcp_resources",
|
||||
"list_mcp_resource_templates",
|
||||
"read_mcp_resource",
|
||||
"update_plan",
|
||||
"request_user_input",
|
||||
"apply_patch",
|
||||
@@ -2230,9 +2261,6 @@ mod tests {
|
||||
Some(WebSearchMode::Cached),
|
||||
"shell_command",
|
||||
&[
|
||||
"list_mcp_resources",
|
||||
"list_mcp_resource_templates",
|
||||
"read_mcp_resource",
|
||||
"update_plan",
|
||||
"request_user_input",
|
||||
"apply_patch",
|
||||
@@ -2252,9 +2280,6 @@ mod tests {
|
||||
Some(WebSearchMode::Cached),
|
||||
"shell",
|
||||
&[
|
||||
"list_mcp_resources",
|
||||
"list_mcp_resource_templates",
|
||||
"read_mcp_resource",
|
||||
"update_plan",
|
||||
"request_user_input",
|
||||
"web_search",
|
||||
@@ -2273,9 +2298,6 @@ mod tests {
|
||||
Some(WebSearchMode::Cached),
|
||||
"shell_command",
|
||||
&[
|
||||
"list_mcp_resources",
|
||||
"list_mcp_resource_templates",
|
||||
"read_mcp_resource",
|
||||
"update_plan",
|
||||
"request_user_input",
|
||||
"apply_patch",
|
||||
@@ -2297,9 +2319,6 @@ mod tests {
|
||||
&[
|
||||
"exec_command",
|
||||
"write_stdin",
|
||||
"list_mcp_resources",
|
||||
"list_mcp_resource_templates",
|
||||
"read_mcp_resource",
|
||||
"update_plan",
|
||||
"request_user_input",
|
||||
"apply_patch",
|
||||
|
||||
@@ -75,9 +75,6 @@ async fn model_selects_expected_tools() {
|
||||
expected_default_tools(
|
||||
"shell_command",
|
||||
&[
|
||||
"list_mcp_resources",
|
||||
"list_mcp_resource_templates",
|
||||
"read_mcp_resource",
|
||||
"update_plan",
|
||||
"request_user_input",
|
||||
"apply_patch",
|
||||
@@ -94,9 +91,6 @@ async fn model_selects_expected_tools() {
|
||||
expected_default_tools(
|
||||
"shell_command",
|
||||
&[
|
||||
"list_mcp_resources",
|
||||
"list_mcp_resource_templates",
|
||||
"read_mcp_resource",
|
||||
"update_plan",
|
||||
"request_user_input",
|
||||
"apply_patch",
|
||||
@@ -113,9 +107,6 @@ async fn model_selects_expected_tools() {
|
||||
expected_default_tools(
|
||||
"shell_command",
|
||||
&[
|
||||
"list_mcp_resources",
|
||||
"list_mcp_resource_templates",
|
||||
"read_mcp_resource",
|
||||
"update_plan",
|
||||
"request_user_input",
|
||||
"apply_patch",
|
||||
@@ -132,9 +123,6 @@ async fn model_selects_expected_tools() {
|
||||
expected_default_tools(
|
||||
"shell",
|
||||
&[
|
||||
"list_mcp_resources",
|
||||
"list_mcp_resource_templates",
|
||||
"read_mcp_resource",
|
||||
"update_plan",
|
||||
"request_user_input",
|
||||
"web_search",
|
||||
@@ -150,9 +138,6 @@ async fn model_selects_expected_tools() {
|
||||
expected_default_tools(
|
||||
"shell_command",
|
||||
&[
|
||||
"list_mcp_resources",
|
||||
"list_mcp_resource_templates",
|
||||
"read_mcp_resource",
|
||||
"update_plan",
|
||||
"request_user_input",
|
||||
"apply_patch",
|
||||
|
||||
@@ -144,9 +144,6 @@ async fn prompt_tools_are_consistent_across_requests() -> anyhow::Result<()> {
|
||||
vec!["exec_command", "write_stdin"]
|
||||
};
|
||||
expected_tools_names.extend([
|
||||
"list_mcp_resources",
|
||||
"list_mcp_resource_templates",
|
||||
"read_mcp_resource",
|
||||
"update_plan",
|
||||
"request_user_input",
|
||||
"apply_patch",
|
||||
|
||||
Reference in New Issue
Block a user