Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions codex-rs/codex-mcp/src/connection_manager_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ fn create_test_tool(server_name: &str, tool_name: &str) -> ToolInfo {
server_name: server_name.to_string(),
callable_name: tool_name.to_string(),
callable_namespace: tool_namespace,
server_instructions: None,
namespace_description: None,
tool: Tool {
name: tool_name.to_string().into(),
title: None,
Expand All @@ -55,7 +55,6 @@ fn create_test_tool(server_name: &str, tool_name: &str) -> ToolInfo {
connector_id: None,
connector_name: None,
plugin_display_names: Vec::new(),
connector_description: None,
}
}

Expand Down
11 changes: 9 additions & 2 deletions codex-rs/codex-mcp/src/rmcp_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,16 +362,23 @@ pub(crate) async fn list_tools_for_client_uncached(
tool_def.title = Some(normalized_title);
}
}
let has_connector_metadata = connector_id.is_some()
|| connector_name.is_some()
|| connector_description.is_some();
let namespace_description = if has_connector_metadata {
connector_description
} else {
server_instructions.map(str::to_string)
};
ToolInfo {
server_name: server_name.to_owned(),
callable_name,
callable_namespace,
server_instructions: server_instructions.map(str::to_string),
namespace_description,
tool: tool_def,
connector_id,
connector_name,
plugin_display_names: Vec::new(),
connector_description,
}
})
.collect();
Expand Down
8 changes: 4 additions & 4 deletions codex-rs/codex-mcp/src/tools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,16 @@ pub struct ToolInfo {
/// Model-visible namespace used for deferred tool loading.
#[serde(rename = "tool_namespace", alias = "callable_namespace")]
pub callable_namespace: String,
/// Instructions from the MCP server initialize result.
#[serde(default)]
pub server_instructions: Option<String>,
/// Model-visible namespace description.
// Keep the old serialized field name readable for cached ToolInfo values.
#[serde(default, alias = "connector_description")]
pub namespace_description: Option<String>,
/// Raw MCP tool definition; `tool.name` is sent back to the MCP server.
pub tool: Tool,
pub connector_id: Option<String>,
pub connector_name: Option<String>,
#[serde(default)]
pub plugin_display_names: Vec<String>,
pub connector_description: Option<String>,
}

impl ToolInfo {
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/core/src/connectors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ pub(crate) fn accessible_connectors_from_mcp_tools(
Some(codex_connectors::accessible::AccessibleConnectorTool {
connector_id: connector_id.to_string(),
connector_name: tool.connector_name.clone(),
connector_description: tool.connector_description.clone(),
connector_description: tool.namespace_description.clone(),
plugin_display_names: tool.plugin_display_names.clone(),
})
});
Expand Down
9 changes: 3 additions & 6 deletions codex-rs/core/src/connectors_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,10 @@ fn codex_app_tool(
server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
callable_name: tool_name.to_string(),
callable_namespace: tool_namespace,
server_instructions: None,
namespace_description: None,
tool: test_tool_definition(tool_name),
connector_id: Some(connector_id.to_string()),
connector_name: connector_name.map(ToOwned::to_owned),
connector_description: None,
plugin_display_names: plugin_names(plugin_display_names),
}
}
Expand Down Expand Up @@ -198,11 +197,10 @@ fn accessible_connectors_from_mcp_tools_carries_plugin_display_names() {
server_name: "sample".to_string(),
callable_name: "echo".to_string(),
callable_namespace: "sample".to_string(),
server_instructions: None,
namespace_description: None,
tool: test_tool_definition("echo"),
connector_id: None,
connector_name: None,
connector_description: None,
plugin_display_names: plugin_names(&["ignored"]),
},
),
Expand Down Expand Up @@ -323,7 +321,7 @@ fn accessible_connectors_from_mcp_tools_preserves_description() {
server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
callable_name: "calendar_create_event".to_string(),
callable_namespace: "mcp__codex_apps__calendar".to_string(),
server_instructions: None,
namespace_description: Some("Plan events".to_string()),
tool: Tool {
name: "calendar_create_event".to_string().into(),
title: None,
Expand All @@ -337,7 +335,6 @@ fn accessible_connectors_from_mcp_tools_preserves_description() {
},
connector_id: Some("calendar".to_string()),
connector_name: Some("Calendar".to_string()),
connector_description: Some("Plan events".to_string()),
plugin_display_names: Vec::new(),
},
)]);
Expand Down
3 changes: 1 addition & 2 deletions codex-rs/core/src/mcp_tool_exposure_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ fn make_mcp_tool(
server_name: server_name.to_string(),
callable_name: tool_name.to_string(),
callable_namespace: tool_namespace,
server_instructions: None,
namespace_description: None,
tool: Tool {
name: tool_name.to_string().into(),
title: None,
Expand All @@ -73,7 +73,6 @@ fn make_mcp_tool(
connector_id: connector_id.map(str::to_string),
connector_name: connector_name.map(str::to_string),
plugin_display_names: Vec::new(),
connector_description: None,
}
}

Expand Down
3 changes: 1 addition & 2 deletions codex-rs/core/src/tools/handlers/tool_search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ mod tests {
server_name: server_name.to_string(),
callable_name: tool_name.to_string(),
callable_namespace: format!("mcp__{server_name}__"),
server_instructions: None,
namespace_description: None,
tool: Tool {
name: tool_name.to_string().into(),
title: None,
Expand All @@ -411,7 +411,6 @@ mod tests {
connector_id: None,
connector_name: None,
plugin_display_names: Vec::new(),
connector_description: None,
}
}

Expand Down
7 changes: 2 additions & 5 deletions codex-rs/core/src/tools/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,7 @@ fn map_mcp_tools_for_plan(mcp_tools: &HashMap<String, ToolInfo>) -> McpToolPlanI
tool.callable_namespace.clone(),
ToolNamespace {
name: tool.callable_namespace.clone(),
description: tool
.connector_description
.clone()
.or_else(|| tool.server_instructions.clone()),
description: tool.namespace_description.clone(),
},
)
})
Expand Down Expand Up @@ -118,7 +115,7 @@ pub(crate) fn build_specs_with_discoverable_tools(
name: tool.canonical_tool_name(),
server_name: tool.server_name.as_str(),
connector_name: tool.connector_name.as_deref(),
connector_description: tool.connector_description.as_deref(),
description: tool.namespace_description.as_deref(),
})
.collect::<Vec<_>>()
});
Expand Down
18 changes: 6 additions & 12 deletions codex-rs/core/src/tools/spec_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,11 @@ fn mcp_tool_info(tool: rmcp::model::Tool) -> ToolInfo {
server_name: "test_server".to_string(),
callable_name: tool.name.to_string(),
callable_namespace: "mcp__test_server__".to_string(),
server_instructions: None,
namespace_description: None,
tool,
connector_id: None,
connector_name: None,
plugin_display_names: Vec::new(),
connector_description: None,
}
}

Expand All @@ -81,12 +80,11 @@ fn mcp_tool_info_with_display_name(display_name: &str, tool: rmcp::model::Tool)
server_name: "test_server".to_string(),
callable_name,
callable_namespace,
server_instructions: None,
namespace_description: None,
tool,
connector_id: None,
connector_name: None,
plugin_display_names: Vec::new(),
connector_description: None,
}
}

Expand Down Expand Up @@ -898,7 +896,7 @@ async fn search_tool_description_falls_back_to_connector_name_without_descriptio
server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
callable_name: "_create_event".to_string(),
callable_namespace: "mcp__codex_apps__calendar".to_string(),
server_instructions: None,
namespace_description: None,
tool: mcp_tool(
"calendar_create_event",
"Create calendar event",
Expand All @@ -907,7 +905,6 @@ async fn search_tool_description_falls_back_to_connector_name_without_descriptio
connector_id: Some("calendar".to_string()),
connector_name: Some("Calendar".to_string()),
plugin_display_names: Vec::new(),
connector_description: None,
},
)])),
&[],
Expand Down Expand Up @@ -950,15 +947,14 @@ async fn search_tool_registers_namespaced_mcp_tool_aliases() {
server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
callable_name: "_create_event".to_string(),
callable_namespace: "mcp__codex_apps__calendar".to_string(),
server_instructions: None,
namespace_description: None,
tool: mcp_tool(
"calendar-create-event",
"Create calendar event",
serde_json::json!({"type": "object"}),
),
connector_id: Some("calendar".to_string()),
connector_name: Some("Calendar".to_string()),
connector_description: None,
plugin_display_names: Vec::new(),
},
),
Expand All @@ -968,15 +964,14 @@ async fn search_tool_registers_namespaced_mcp_tool_aliases() {
server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
callable_name: "_list_events".to_string(),
callable_namespace: "mcp__codex_apps__calendar".to_string(),
server_instructions: None,
namespace_description: None,
tool: mcp_tool(
"calendar-list-events",
"List calendar events",
serde_json::json!({"type": "object"}),
),
connector_id: Some("calendar".to_string()),
connector_name: Some("Calendar".to_string()),
connector_description: None,
plugin_display_names: Vec::new(),
},
),
Expand All @@ -986,11 +981,10 @@ async fn search_tool_registers_namespaced_mcp_tool_aliases() {
server_name: "rmcp".to_string(),
callable_name: "echo".to_string(),
callable_namespace: "mcp__rmcp__".to_string(),
server_instructions: None,
namespace_description: None,
tool: mcp_tool("echo", "Echo", serde_json::json!({"type": "object"})),
connector_id: None,
connector_name: None,
connector_description: None,
plugin_display_names: Vec::new(),
},
),
Expand Down
8 changes: 4 additions & 4 deletions codex-rs/core/src/tools/tool_search_entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ fn mcp_tool_search_entry(info: &ToolInfo) -> Result<ToolSearchEntry, serde_json:
tool_name: info.callable_name.as_str(),
tool: &info.tool,
connector_name: info.connector_name.as_deref(),
connector_description: info.connector_description.as_deref(),
description: info.namespace_description.as_deref(),
})?,
limit_bucket: Some(info.server_name.clone()),
})
Expand Down Expand Up @@ -120,10 +120,10 @@ fn build_mcp_search_text(info: &ToolInfo) -> String {
parts.push(connector_name.to_string());
}

if let Some(connector_description) = info.connector_description.as_deref()
&& !connector_description.trim().is_empty()
if let Some(description) = info.namespace_description.as_deref()
&& !description.trim().is_empty()
{
parts.push(connector_description.to_string());
parts.push(description.to_string());
}

parts.extend(
Expand Down
89 changes: 89 additions & 0 deletions codex-rs/core/tests/suite/search_tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1029,3 +1029,92 @@ async fn tool_search_indexes_only_enabled_non_app_mcp_tools() -> Result<()> {

Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn tool_search_uses_non_app_mcp_server_instructions_as_namespace_description() -> Result<()> {
skip_if_no_network!(Ok(()));

let server = start_mock_server().await;
let apps_server = AppsTestServer::mount_searchable(&server).await?;
let search_call_id = "tool-search-echo";
let mock = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-1"),
ev_tool_search_call(
search_call_id,
&json!({
"query": "Echo back the provided message and include environment data.",
"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,
},
experimental_environment: None,
enabled: true,
required: false,
disabled_reason: None,
startup_timeout_sec: Some(Duration::from_secs(10)),
tool_timeout_sec: None,
default_tools_approval_mode: None,
enabled_tools: Some(vec!["echo".to_string()]),
disabled_tools: None,
scopes: None,
oauth_resource: None,
supports_parallel_tool_calls: false,
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_approval_and_permission_profile(
"Find the rmcp echo tool.",
AskForApproval::Never,
PermissionProfile::Disabled,
)
.await?;

let requests = mock.requests();
assert_eq!(requests.len(), 2);

let tools = tool_search_output_tools(&requests[1], search_call_id);
let rmcp_namespace = tools
.iter()
.find(|tool| tool.get("name").and_then(Value::as_str) == Some("mcp__rmcp__"))
.expect("tool_search should return the rmcp namespace");
assert_eq!(
rmcp_namespace.get("description").and_then(Value::as_str),
Some("Use these tools to exercise the rmcp test server.")
);

Ok(())
}
1 change: 1 addition & 0 deletions codex-rs/rmcp-client/src/bin/test_stdio_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ impl ServerHandler for TestToolServer {
)]));

ServerInfo {
instructions: Some("Use these tools to exercise the rmcp test server.".to_string()),
capabilities,
..ServerInfo::default()
}
Expand Down
Loading
Loading