diff --git a/codex-rs/codex-mcp/src/apps.rs b/codex-rs/codex-mcp/src/apps.rs index f54ab070ba..982602d506 100644 --- a/codex-rs/codex-mcp/src/apps.rs +++ b/codex-rs/codex-mcp/src/apps.rs @@ -3,9 +3,9 @@ use std::path::PathBuf; use std::time::Instant; use crate::mcp::CODEX_APPS_MCP_SERVER_NAME; -use crate::mcp_connection::MCP_TOOLS_CACHE_WRITE_DURATION_METRIC; -use crate::mcp_connection::ToolInfo; use crate::mcp_connection::emit_duration; +use crate::tools::MCP_TOOLS_CACHE_WRITE_DURATION_METRIC; +use crate::tools::ToolInfo; use codex_login::CodexAuth; use codex_utils_plugins::mcp_connector::is_connector_id_allowed; use codex_utils_plugins::mcp_connector::sanitize_name; diff --git a/codex-rs/codex-mcp/src/client.rs b/codex-rs/codex-mcp/src/client.rs index 62635a1018..f803f210dd 100644 --- a/codex-rs/codex-mcp/src/client.rs +++ b/codex-rs/codex-mcp/src/client.rs @@ -24,13 +24,13 @@ use crate::mcp_connection::DEFAULT_STARTUP_TIMEOUT; use crate::mcp_connection::DEFAULT_TOOL_TIMEOUT; use crate::mcp_connection::MCP_SANDBOX_STATE_META_CAPABILITY; use crate::mcp_connection::McpRuntimeEnvironment; -use crate::mcp_connection::ToolFilter; -use crate::mcp_connection::ToolInfo; use crate::mcp_connection::emit_duration; -use crate::mcp_connection::filter_tools; use crate::mcp_connection::resolve_bearer_token; -use crate::mcp_connection::tool_with_model_visible_input_schema; use crate::mcp_connection::validate_mcp_server_name; +use crate::tools::ToolFilter; +use crate::tools::ToolInfo; +use crate::tools::filter_tools; +use crate::tools::tool_with_model_visible_input_schema; use anyhow::Result; use anyhow::anyhow; use async_channel::Sender; diff --git a/codex-rs/codex-mcp/src/lib.rs b/codex-rs/codex-mcp/src/lib.rs index f1a62a873b..4cfa7cd7da 100644 --- a/codex-rs/codex-mcp/src/lib.rs +++ b/codex-rs/codex-mcp/src/lib.rs @@ -2,7 +2,7 @@ pub use manager::McpConnectionManager; pub use mcp_connection::MCP_SANDBOX_STATE_META_CAPABILITY; pub use mcp_connection::McpRuntimeEnvironment; pub use mcp_connection::SandboxState; -pub use mcp_connection::ToolInfo; +pub use tools::ToolInfo; pub use mcp::CODEX_APPS_MCP_SERVER_NAME; pub use mcp::McpConfig; @@ -36,7 +36,7 @@ pub use mcp::should_retry_without_scopes; pub use apps::filter_non_codex_apps_mcp_tools_only; pub use mcp::mcp_permission_prompt_is_auto_approved; pub use mcp::qualified_mcp_tool_name_prefix; -pub use mcp_connection::declared_openai_file_input_param_names; +pub use tools::declared_openai_file_input_param_names; pub(crate) mod apps; pub(crate) mod client; @@ -44,4 +44,4 @@ pub(crate) mod elicitation; pub(crate) mod manager; pub(crate) mod mcp; pub(crate) mod mcp_connection; -pub(crate) mod mcp_tool_names; +pub(crate) mod tools; diff --git a/codex-rs/codex-mcp/src/manager.rs b/codex-rs/codex-mcp/src/manager.rs index de67ffd80e..a5d678c47e 100644 --- a/codex-rs/codex-mcp/src/manager.rs +++ b/codex-rs/codex-mcp/src/manager.rs @@ -18,15 +18,15 @@ use crate::elicitation::ElicitationRequestManager; use crate::mcp::CODEX_APPS_MCP_SERVER_NAME; use crate::mcp::ToolPluginProvenance; use crate::mcp_connection::McpRuntimeEnvironment; -use crate::mcp_connection::ToolInfo; use crate::mcp_connection::emit_duration; use crate::mcp_connection::emit_update; -use crate::mcp_connection::filter_tools; use crate::mcp_connection::mcp_init_error_display; -use crate::mcp_connection::qualify_tools; use crate::mcp_connection::startup_outcome_error_message; -use crate::mcp_connection::tool_with_model_visible_input_schema; use crate::mcp_connection::transport_origin; +use crate::tools::ToolInfo; +use crate::tools::filter_tools; +use crate::tools::qualify_tools; +use crate::tools::tool_with_model_visible_input_schema; use anyhow::Context; use anyhow::Result; use anyhow::anyhow; diff --git a/codex-rs/codex-mcp/src/mcp_connection.rs b/codex-rs/codex-mcp/src/mcp_connection.rs index 99ad8766cb..a07351b80b 100644 --- a/codex-rs/codex-mcp/src/mcp_connection.rs +++ b/codex-rs/codex-mcp/src/mcp_connection.rs @@ -3,7 +3,6 @@ //! This module contains shared types and helpers used by [`McpConnectionManager`]. use std::collections::HashMap; -use std::collections::HashSet; use std::env; use std::path::PathBuf; use std::sync::Arc; @@ -11,26 +10,20 @@ use std::time::Duration; use crate::McpAuthStatusEntry; use crate::client::StartupOutcomeError; -pub(crate) use crate::mcp_tool_names::qualify_tools; use anyhow::Result; use anyhow::anyhow; use async_channel::Sender; use codex_exec_server::Environment; -use codex_protocol::ToolName; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::McpStartupUpdateEvent; use codex_protocol::protocol::SandboxPolicy; -use rmcp::model::Tool; use serde::Deserialize; use serde::Serialize; -use serde_json::Map; -use serde_json::Value as JsonValue; use url::Url; -use codex_config::McpServerConfig; use codex_config::McpServerTransportConfig; /// Default timeout for initializing MCP server & initially listing tools. @@ -39,54 +32,6 @@ pub(crate) const DEFAULT_STARTUP_TIMEOUT: Duration = Duration::from_secs(30); /// Default timeout for individual tool calls. pub(crate) const DEFAULT_TOOL_TIMEOUT: Duration = Duration::from_secs(120); -pub(crate) const MCP_TOOLS_CACHE_WRITE_DURATION_METRIC: &str = - "codex.mcp.tools.cache_write.duration_ms"; - -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct ToolInfo { - /// Raw MCP server name used for routing the tool call. - pub server_name: String, - /// Model-visible tool name used in Responses API tool declarations. - #[serde(rename = "tool_name", alias = "callable_name")] - pub callable_name: String, - /// 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, - /// Raw MCP tool definition; `tool.name` is sent back to the MCP server. - pub tool: Tool, - pub connector_id: Option, - pub connector_name: Option, - #[serde(default)] - pub plugin_display_names: Vec, - pub connector_description: Option, -} - -impl ToolInfo { - pub fn canonical_tool_name(&self) -> ToolName { - ToolName::namespaced(self.callable_namespace.clone(), self.callable_name.clone()) - } -} - -pub fn declared_openai_file_input_param_names( - meta: Option<&Map>, -) -> Vec { - let Some(meta) = meta else { - return Vec::new(); - }; - - meta.get(META_OPENAI_FILE_PARAMS) - .and_then(JsonValue::as_array) - .into_iter() - .flatten() - .filter_map(JsonValue::as_str) - .filter(|value| !value.is_empty()) - .map(str::to_string) - .collect() -} - /// MCP server capability indicating that Codex should include [`SandboxState`] /// in tool-call request `_meta` under this key. pub const MCP_SANDBOX_STATE_META_CAPABILITY: &str = "codex/sandbox-state-meta"; @@ -134,107 +79,6 @@ impl McpRuntimeEnvironment { } } -/// A tool is allowed to be used if both are true: -/// 1. enabled is None (no allowlist is set) or the tool is explicitly enabled. -/// 2. The tool is not explicitly disabled. -#[derive(Default, Clone)] -pub(crate) struct ToolFilter { - pub(crate) enabled: Option>, - pub(crate) disabled: HashSet, -} - -impl ToolFilter { - pub(crate) fn from_config(cfg: &McpServerConfig) -> Self { - let enabled = cfg - .enabled_tools - .as_ref() - .map(|tools| tools.iter().cloned().collect::>()); - let disabled = cfg - .disabled_tools - .as_ref() - .map(|tools| tools.iter().cloned().collect::>()) - .unwrap_or_default(); - - Self { enabled, disabled } - } - - pub(crate) fn allows(&self, tool_name: &str) -> bool { - if let Some(enabled) = &self.enabled - && !enabled.contains(tool_name) - { - return false; - } - - !self.disabled.contains(tool_name) - } -} - -const META_OPENAI_FILE_PARAMS: &str = "openai/fileParams"; - -/// Returns the model-visible view of a tool while preserving the raw metadata -/// used by execution. Keep cache entries raw and call this at manager return -/// boundaries. -pub(crate) fn tool_with_model_visible_input_schema(tool: &Tool) -> Tool { - let file_params = declared_openai_file_input_param_names(tool.meta.as_deref()); - if file_params.is_empty() { - return tool.clone(); - } - - let mut tool = tool.clone(); - let mut input_schema = JsonValue::Object(tool.input_schema.as_ref().clone()); - mask_input_schema_for_file_path_params(&mut input_schema, &file_params); - if let JsonValue::Object(input_schema) = input_schema { - tool.input_schema = Arc::new(input_schema); - } - tool -} - -fn mask_input_schema_for_file_path_params(input_schema: &mut JsonValue, file_params: &[String]) { - let Some(properties) = input_schema - .as_object_mut() - .and_then(|schema| schema.get_mut("properties")) - .and_then(JsonValue::as_object_mut) - else { - return; - }; - - for field_name in file_params { - let Some(property_schema) = properties.get_mut(field_name) else { - continue; - }; - mask_input_property_schema(property_schema); - } -} - -fn mask_input_property_schema(schema: &mut JsonValue) { - let Some(object) = schema.as_object_mut() else { - return; - }; - - let mut description = object - .get("description") - .and_then(JsonValue::as_str) - .map(str::to_string) - .unwrap_or_default(); - let guidance = "This parameter expects an absolute local file path. If you want to upload a file, provide the absolute path to that file here."; - if description.is_empty() { - description = guidance.to_string(); - } else if !description.contains(guidance) { - description = format!("{description} {guidance}"); - } - - let is_array = object.get("type").and_then(JsonValue::as_str) == Some("array") - || object.get("items").is_some(); - object.clear(); - object.insert("description".to_string(), JsonValue::String(description)); - if is_array { - object.insert("type".to_string(), JsonValue::String("array".to_string())); - object.insert("items".to_string(), serde_json::json!({ "type": "string" })); - } else { - object.insert("type".to_string(), JsonValue::String("string".to_string())); - } -} - pub(crate) async fn emit_update( submit_id: &str, tx_event: &Sender, @@ -248,13 +92,6 @@ pub(crate) async fn emit_update( .await } -pub(crate) fn filter_tools(tools: Vec, filter: &ToolFilter) -> Vec { - tools - .into_iter() - .filter(|tool| filter.allows(&tool.tool.name)) - .collect() -} - pub(crate) fn resolve_bearer_token( server_name: &str, bearer_token_env_var: Option<&str>, diff --git a/codex-rs/codex-mcp/src/mcp_connection_manager_tests.rs b/codex-rs/codex-mcp/src/mcp_connection_manager_tests.rs index 4e4ec55a27..ca64a6dd0b 100644 --- a/codex-rs/codex-mcp/src/mcp_connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/mcp_connection_manager_tests.rs @@ -11,11 +11,12 @@ use crate::client::elicitation_capability_for_server; use crate::declared_openai_file_input_param_names; use crate::elicitation::ElicitationRequestManager; use crate::elicitation::elicitation_is_rejected_by_policy; -use crate::mcp_connection::ToolFilter; -use crate::mcp_connection::ToolInfo; -use crate::mcp_connection::filter_tools; -use crate::mcp_connection::tool_with_model_visible_input_schema; use crate::mcp_connection::transport_origin; +use crate::tools::ToolFilter; +use crate::tools::ToolInfo; +use crate::tools::filter_tools; +use crate::tools::qualify_tools; +use crate::tools::tool_with_model_visible_input_schema; use codex_config::Constrained; use codex_protocol::ToolName; use codex_protocol::protocol::GranularApprovalConfig; diff --git a/codex-rs/codex-mcp/src/mcp_tool_names.rs b/codex-rs/codex-mcp/src/tools.rs similarity index 55% rename from codex-rs/codex-mcp/src/mcp_tool_names.rs rename to codex-rs/codex-mcp/src/tools.rs index cceb74b615..7aca9a1ba4 100644 --- a/codex-rs/codex-mcp/src/mcp_tool_names.rs +++ b/codex-rs/codex-mcp/src/tools.rs @@ -2,17 +2,178 @@ use std::collections::HashMap; use std::collections::HashSet; +use std::sync::Arc; +use codex_config::McpServerConfig; +use codex_protocol::ToolName; +use rmcp::model::Tool; +use serde::Deserialize; +use serde::Serialize; +use serde_json::Map; +use serde_json::Value as JsonValue; use sha1::Digest; use sha1::Sha1; use tracing::warn; use crate::mcp::sanitize_responses_api_tool_name; -use crate::mcp_connection::ToolInfo; const MCP_TOOL_NAME_DELIMITER: &str = "__"; const MAX_TOOL_NAME_LENGTH: usize = 64; const CALLABLE_NAME_HASH_LEN: usize = 12; +const META_OPENAI_FILE_PARAMS: &str = "openai/fileParams"; +pub(crate) const MCP_TOOLS_CACHE_WRITE_DURATION_METRIC: &str = + "codex.mcp.tools.cache_write.duration_ms"; + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ToolInfo { + /// Raw MCP server name used for routing the tool call. + pub server_name: String, + /// Model-visible tool name used in Responses API tool declarations. + #[serde(rename = "tool_name", alias = "callable_name")] + pub callable_name: String, + /// 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, + /// Raw MCP tool definition; `tool.name` is sent back to the MCP server. + pub tool: Tool, + pub connector_id: Option, + pub connector_name: Option, + #[serde(default)] + pub plugin_display_names: Vec, + pub connector_description: Option, +} + +impl ToolInfo { + pub fn canonical_tool_name(&self) -> ToolName { + ToolName::namespaced(self.callable_namespace.clone(), self.callable_name.clone()) + } +} + +pub fn declared_openai_file_input_param_names( + meta: Option<&Map>, +) -> Vec { + let Some(meta) = meta else { + return Vec::new(); + }; + + meta.get(META_OPENAI_FILE_PARAMS) + .and_then(JsonValue::as_array) + .into_iter() + .flatten() + .filter_map(JsonValue::as_str) + .filter(|value| !value.is_empty()) + .map(str::to_string) + .collect() +} + +/// A tool is allowed to be used if both are true: +/// 1. enabled is None (no allowlist is set) or the tool is explicitly enabled. +/// 2. The tool is not explicitly disabled. +#[derive(Default, Clone)] +pub(crate) struct ToolFilter { + pub(crate) enabled: Option>, + pub(crate) disabled: HashSet, +} + +impl ToolFilter { + pub(crate) fn from_config(cfg: &McpServerConfig) -> Self { + let enabled = cfg + .enabled_tools + .as_ref() + .map(|tools| tools.iter().cloned().collect::>()); + let disabled = cfg + .disabled_tools + .as_ref() + .map(|tools| tools.iter().cloned().collect::>()) + .unwrap_or_default(); + + Self { enabled, disabled } + } + + pub(crate) fn allows(&self, tool_name: &str) -> bool { + if let Some(enabled) = &self.enabled + && !enabled.contains(tool_name) + { + return false; + } + + !self.disabled.contains(tool_name) + } +} + +/// Returns the model-visible view of a tool while preserving the raw metadata +/// used by execution. Keep cache entries raw and call this at manager return +/// boundaries. +pub(crate) fn tool_with_model_visible_input_schema(tool: &Tool) -> Tool { + let file_params = declared_openai_file_input_param_names(tool.meta.as_deref()); + if file_params.is_empty() { + return tool.clone(); + } + + let mut tool = tool.clone(); + let mut input_schema = JsonValue::Object(tool.input_schema.as_ref().clone()); + mask_input_schema_for_file_path_params(&mut input_schema, &file_params); + if let JsonValue::Object(input_schema) = input_schema { + tool.input_schema = Arc::new(input_schema); + } + tool +} + +fn mask_input_schema_for_file_path_params(input_schema: &mut JsonValue, file_params: &[String]) { + let Some(properties) = input_schema + .as_object_mut() + .and_then(|schema| schema.get_mut("properties")) + .and_then(JsonValue::as_object_mut) + else { + return; + }; + + for field_name in file_params { + let Some(property_schema) = properties.get_mut(field_name) else { + continue; + }; + mask_input_property_schema(property_schema); + } +} + +fn mask_input_property_schema(schema: &mut JsonValue) { + let Some(object) = schema.as_object_mut() else { + return; + }; + + let mut description = object + .get("description") + .and_then(JsonValue::as_str) + .map(str::to_string) + .unwrap_or_default(); + let guidance = "This parameter expects an absolute local file path. If you want to upload a file, provide the absolute path to that file here."; + if description.is_empty() { + description = guidance.to_string(); + } else if !description.contains(guidance) { + description = format!("{description} {guidance}"); + } + + let is_array = object.get("type").and_then(JsonValue::as_str) == Some("array") + || object.get("items").is_some(); + object.clear(); + object.insert("description".to_string(), JsonValue::String(description)); + if is_array { + object.insert("type".to_string(), JsonValue::String("array".to_string())); + object.insert("items".to_string(), serde_json::json!({ "type": "string" })); + } else { + object.insert("type".to_string(), JsonValue::String("string".to_string())); + } +} + +pub(crate) fn filter_tools(tools: Vec, filter: &ToolFilter) -> Vec { + tools + .into_iter() + .filter(|tool| filter.allows(&tool.tool.name)) + .collect() +} /// Returns a qualified-name lookup for MCP tools. ///