mirror of
https://github.com/openai/codex.git
synced 2026-04-28 00:25:56 +00:00
[mcp] Expand tool search to custom MCPs. (#16944)
- [x] Expand tool search to custom MCPs.
- [x] Rename several variables/fields to be more generic.
Updated tool & server name lifecycles:
**Raw Identity**
ToolInfo.server_name is raw MCP server name.
ToolInfo.tool.name is raw MCP tool name.
MCP calls route back to raw via parse_tool_name() returning
(tool.server_name, tool.tool.name).
mcpServerStatus/list now groups by raw server and keys tools by
Tool.name: mod.rs:599
App-server just forwards that grouped raw snapshot:
codex_message_processor.rs:5245
**Callable Names**
On list-tools, we create provisional callable_namespace / callable_name:
mcp_connection_manager.rs:1556
For non-app MCP, provisional callable name starts as raw tool name.
For codex-apps, provisional callable name is sanitized and strips
connector name/id prefix; namespace includes connector name.
Then qualify_tools() sanitizes callable namespace + name to ASCII alnum
/ _ only: mcp_tool_names.rs:128
Note: this is stricter than Responses API. Hyphen is currently replaced
with _ for code-mode compatibility.
**Collision Handling**
We do initially collapse example-server and example_server to the same
base.
Then qualify_tools() detects distinct raw namespace identities behind
the same sanitized namespace and appends a hash to the callable
namespace: mcp_tool_names.rs:137
Same idea for tool-name collisions: hash suffix goes on callable tool
name.
Final list_all_tools() map key is callable_namespace + callable_name:
mcp_connection_manager.rs:769
**Direct Model Tools**
Direct MCP tool declarations use the full qualified sanitized key as the
Responses function name.
The raw rmcp Tool is converted but renamed for model exposure.
**Tool Search / Deferred**
Tool search result namespace = final ToolInfo.callable_namespace:
tool_search.rs:85
Tool search result nested name = final ToolInfo.callable_name:
tool_search.rs:86
Deferred tool handler is registered as "{namespace}:{name}":
tool_registry_plan.rs:248
When a function call comes back, core recombines namespace + name, looks
up the full qualified key, and gets the raw server/tool for MCP
execution: codex.rs:4353
**Separate Legacy Snapshot**
collect_mcp_snapshot_from_manager_with_detail() still returns a map
keyed by qualified callable name.
mcpServerStatus/list no longer uses that; it uses
McpServerStatusSnapshot, which is raw-inventory shaped.
This commit is contained in:
@@ -2,6 +2,8 @@
|
||||
#![allow(clippy::unwrap_used, clippy::expect_used)]
|
||||
|
||||
use anyhow::Result;
|
||||
use codex_config::types::McpServerConfig;
|
||||
use codex_config::types::McpServerTransportConfig;
|
||||
use codex_core::config::Config;
|
||||
use codex_features::Feature;
|
||||
use codex_login::CodexAuth;
|
||||
@@ -24,15 +26,18 @@ use core_test_support::responses::mount_sse_sequence;
|
||||
use core_test_support::responses::sse;
|
||||
use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::stdio_server_bin;
|
||||
use core_test_support::test_codex::TestCodexBuilder;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::wait_for_event;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use serde_json::json;
|
||||
use std::collections::HashMap;
|
||||
use std::time::Duration;
|
||||
|
||||
const SEARCH_TOOL_DESCRIPTION_SNIPPETS: [&str; 2] = [
|
||||
"You have access to all the tools of the following apps/connectors",
|
||||
"You have access to tools from the following MCP servers/connectors",
|
||||
"- Calendar: Plan events and manage your calendar.",
|
||||
];
|
||||
const TOOL_SEARCH_TOOL_NAME: &str = "tool_search";
|
||||
@@ -165,7 +170,7 @@ async fn search_tool_flag_adds_tool_search() -> Result<()> {
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"query": {"type": "string", "description": "Search query for apps tools."},
|
||||
"query": {"type": "string", "description": "Search query for MCP tools."},
|
||||
"limit": {"type": "number", "description": "Maximum number of tools to return (defaults to 8)."},
|
||||
},
|
||||
"required": ["query"],
|
||||
@@ -582,3 +587,123 @@ async fn tool_search_returns_deferred_tools_without_follow_up_tool_injection() -
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn tool_search_indexes_only_enabled_non_app_mcp_tools() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let apps_server = AppsTestServer::mount_searchable(&server).await?;
|
||||
let echo_call_id = "tool-search-echo";
|
||||
let image_call_id = "tool-search-image";
|
||||
let mock = mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_tool_search_call(
|
||||
echo_call_id,
|
||||
&json!({
|
||||
"query": "Echo back the provided message and include environment data.",
|
||||
"limit": 8,
|
||||
}),
|
||||
),
|
||||
ev_tool_search_call(
|
||||
image_call_id,
|
||||
&json!({
|
||||
"query": "Return a single image content block.",
|
||||
"limit": 8,
|
||||
}),
|
||||
),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-2"),
|
||||
ev_assistant_message("msg-1", "done"),
|
||||
ev_completed("resp-2"),
|
||||
]),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
|
||||
let rmcp_test_server_bin = stdio_server_bin()?;
|
||||
let mut builder =
|
||||
configured_builder(apps_server.chatgpt_base_url.clone()).with_config(move |config| {
|
||||
let mut servers = config.mcp_servers.get().clone();
|
||||
servers.insert(
|
||||
"rmcp".to_string(),
|
||||
McpServerConfig {
|
||||
transport: McpServerTransportConfig::Stdio {
|
||||
command: rmcp_test_server_bin,
|
||||
args: Vec::new(),
|
||||
env: None,
|
||||
env_vars: Vec::new(),
|
||||
cwd: None,
|
||||
},
|
||||
enabled: true,
|
||||
required: false,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: Some(Duration::from_secs(10)),
|
||||
tool_timeout_sec: None,
|
||||
enabled_tools: Some(vec!["echo".to_string(), "image".to_string()]),
|
||||
disabled_tools: Some(vec!["image".to_string()]),
|
||||
scopes: None,
|
||||
oauth_resource: None,
|
||||
tools: HashMap::new(),
|
||||
},
|
||||
);
|
||||
config
|
||||
.mcp_servers
|
||||
.set(servers)
|
||||
.expect("test mcp servers should accept any configuration");
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
test.submit_turn_with_policies(
|
||||
"Find the rmcp echo and image tools.",
|
||||
AskForApproval::Never,
|
||||
SandboxPolicy::DangerFullAccess,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let requests = mock.requests();
|
||||
assert_eq!(requests.len(), 2);
|
||||
|
||||
let first_request_tools = tool_names(&requests[0].body_json());
|
||||
assert!(
|
||||
first_request_tools
|
||||
.iter()
|
||||
.any(|name| name == TOOL_SEARCH_TOOL_NAME),
|
||||
"first request should advertise tool_search: {first_request_tools:?}"
|
||||
);
|
||||
assert!(
|
||||
!first_request_tools
|
||||
.iter()
|
||||
.any(|name| name == "mcp__rmcp__echo"),
|
||||
"non-app MCP tools should be hidden before search in large-search mode: {first_request_tools:?}"
|
||||
);
|
||||
|
||||
let echo_tools = tool_search_output_tools(&requests[1], echo_call_id);
|
||||
let rmcp_echo_tools = echo_tools
|
||||
.iter()
|
||||
.filter(|tool| tool.get("name").and_then(Value::as_str) == Some("mcp__rmcp__"))
|
||||
.flat_map(|namespace| namespace.get("tools").and_then(Value::as_array))
|
||||
.flatten()
|
||||
.filter_map(|tool| tool.get("name").and_then(Value::as_str).map(str::to_string))
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(rmcp_echo_tools, vec!["echo".to_string()]);
|
||||
|
||||
let image_tools = tool_search_output_tools(&requests[1], image_call_id);
|
||||
let found_rmcp_image_tool = image_tools
|
||||
.iter()
|
||||
.filter(|tool| tool.get("name").and_then(Value::as_str) == Some("mcp__rmcp__"))
|
||||
.flat_map(|namespace| namespace.get("tools").and_then(Value::as_array))
|
||||
.flatten()
|
||||
.any(|tool| tool.get("name").and_then(Value::as_str).is_some());
|
||||
assert!(
|
||||
!found_rmcp_image_tool,
|
||||
"disabled non-app MCP tools should not be searchable: {image_tools:?}"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user