diff --git a/codex-rs/codex-mcp/src/lib.rs b/codex-rs/codex-mcp/src/lib.rs index 70f0b61405..ae73563c1e 100644 --- a/codex-rs/codex-mcp/src/lib.rs +++ b/codex-rs/codex-mcp/src/lib.rs @@ -1,37 +1,43 @@ -pub(crate) mod mcp; -pub(crate) mod mcp_connection_manager; -pub(crate) mod mcp_tool_names; - -pub use mcp::CODEX_APPS_MCP_SERVER_NAME; -pub use mcp::McpAuthStatusEntry; -pub use mcp::McpConfig; -pub use mcp::McpOAuthLoginConfig; -pub use mcp::McpOAuthLoginSupport; -pub use mcp::McpOAuthScopesSource; -pub use mcp::McpServerStatusSnapshot; -pub use mcp::McpSnapshotDetail; -pub use mcp::ResolvedMcpOAuthScopes; -pub use mcp::ToolPluginProvenance; -pub use mcp::collect_mcp_server_status_snapshot_with_detail; -pub use mcp::collect_mcp_snapshot_from_manager; -pub use mcp::compute_auth_statuses; -pub use mcp::configured_mcp_servers; -pub use mcp::discover_supported_scopes; -pub use mcp::effective_mcp_servers; -pub use mcp::mcp_permission_prompt_is_auto_approved; -pub use mcp::oauth_login_support; -pub use mcp::qualified_mcp_tool_name_prefix; -pub use mcp::read_mcp_resource; -pub use mcp::resolve_oauth_scopes; -pub use mcp::should_retry_without_scopes; -pub use mcp::tool_plugin_provenance; -pub use mcp::with_codex_apps_mcp; -pub use mcp_connection_manager::CodexAppsToolsCacheKey; pub use mcp_connection_manager::MCP_SANDBOX_STATE_META_CAPABILITY; pub use mcp_connection_manager::McpConnectionManager; pub use mcp_connection_manager::McpRuntimeEnvironment; pub use mcp_connection_manager::SandboxState; pub use mcp_connection_manager::ToolInfo; + +pub use mcp::CODEX_APPS_MCP_SERVER_NAME; +pub use mcp::McpConfig; +pub use mcp::ToolPluginProvenance; + +pub use mcp_connection_manager::CodexAppsToolsCacheKey; pub use mcp_connection_manager::codex_apps_tools_cache_key; + +pub use mcp::configured_mcp_servers; +pub use mcp::effective_mcp_servers; +pub use mcp::tool_plugin_provenance; +pub use mcp::with_codex_apps_mcp; + +pub use mcp::McpServerStatusSnapshot; +pub use mcp::McpSnapshotDetail; +pub use mcp::collect_mcp_server_status_snapshot_with_detail; +pub use mcp::collect_mcp_snapshot_from_manager; +pub use mcp::read_mcp_resource; + +pub use mcp::McpAuthStatusEntry; +pub use mcp::McpOAuthLoginConfig; +pub use mcp::McpOAuthLoginSupport; +pub use mcp::McpOAuthScopesSource; +pub use mcp::ResolvedMcpOAuthScopes; +pub use mcp::compute_auth_statuses; +pub use mcp::discover_supported_scopes; +pub use mcp::oauth_login_support; +pub use mcp::resolve_oauth_scopes; +pub use mcp::should_retry_without_scopes; + +pub use mcp::mcp_permission_prompt_is_auto_approved; +pub use mcp::qualified_mcp_tool_name_prefix; pub use mcp_connection_manager::declared_openai_file_input_param_names; pub use mcp_connection_manager::filter_non_codex_apps_mcp_tools_only; + +pub(crate) mod mcp; +pub(crate) mod mcp_connection_manager; +pub(crate) mod mcp_tool_names; diff --git a/codex-rs/codex-mcp/src/mcp/auth.rs b/codex-rs/codex-mcp/src/mcp/auth.rs index 9c605c16fb..6a97b52789 100644 --- a/codex-rs/codex-mcp/src/mcp/auth.rs +++ b/codex-rs/codex-mcp/src/mcp/auth.rs @@ -43,6 +43,12 @@ pub struct ResolvedMcpOAuthScopes { pub source: McpOAuthScopesSource, } +#[derive(Debug, Clone)] +pub struct McpAuthStatusEntry { + pub config: McpServerConfig, + pub auth_status: McpAuthStatus, +} + pub async fn oauth_login_support(transport: &McpServerTransportConfig) -> McpOAuthLoginSupport { let McpServerTransportConfig::StreamableHttp { url, @@ -119,12 +125,6 @@ pub fn should_retry_without_scopes(scopes: &ResolvedMcpOAuthScopes, error: &anyh && error.downcast_ref::().is_some() } -#[derive(Debug, Clone)] -pub struct McpAuthStatusEntry { - pub config: McpServerConfig, - pub auth_status: McpAuthStatus, -} - pub async fn compute_auth_statuses<'a, I>( servers: I, store_mode: OAuthCredentialsStoreMode, diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index bf98aeb0f4..3c2a971081 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -1,4 +1,3 @@ -pub(crate) mod auth; pub use auth::McpAuthStatusEntry; pub use auth::McpOAuthLoginConfig; pub use auth::McpOAuthLoginSupport; @@ -10,6 +9,8 @@ pub use auth::oauth_login_support; pub use auth::resolve_oauth_scopes; pub use auth::should_retry_without_scopes; +pub(crate) mod auth; + use std::collections::HashMap; use std::env; use std::path::PathBuf; @@ -37,9 +38,9 @@ use crate::mcp_connection_manager::McpConnectionManager; use crate::mcp_connection_manager::McpRuntimeEnvironment; use crate::mcp_connection_manager::codex_apps_tools_cache_key; +pub const CODEX_APPS_MCP_SERVER_NAME: &str = "codex_apps"; const MCP_TOOL_NAME_PREFIX: &str = "mcp"; const MCP_TOOL_NAME_DELIMITER: &str = "__"; -pub const CODEX_APPS_MCP_SERVER_NAME: &str = "codex_apps"; const CODEX_CONNECTORS_TOKEN_ENV_VAR: &str = "CODEX_CONNECTORS_TOKEN"; #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] @@ -55,26 +56,6 @@ impl McpSnapshotDetail { } } -/// The Responses API requires tool names to match `^[a-zA-Z0-9_-]+$`. -/// MCP server/tool names are user-controlled, so sanitize the fully-qualified -/// name we expose to the model by replacing any disallowed character with `_`. -pub(crate) fn sanitize_responses_api_tool_name(name: &str) -> String { - let mut sanitized = String::with_capacity(name.len()); - for c in name.chars() { - if c.is_ascii_alphanumeric() || c == '_' { - sanitized.push(c); - } else { - sanitized.push('_'); - } - } - - if sanitized.is_empty() { - "_".to_string() - } else { - sanitized - } -} - pub fn qualified_mcp_tool_name_prefix(server_name: &str) -> String { sanitize_responses_api_tool_name(&format!( "{MCP_TOOL_NAME_PREFIX}{MCP_TOOL_NAME_DELIMITER}{server_name}{MCP_TOOL_NAME_DELIMITER}" @@ -192,67 +173,6 @@ impl ToolPluginProvenance { } } -fn codex_apps_mcp_bearer_token_env_var() -> Option { - match env::var(CODEX_CONNECTORS_TOKEN_ENV_VAR) { - Ok(value) if !value.trim().is_empty() => Some(CODEX_CONNECTORS_TOKEN_ENV_VAR.to_string()), - Ok(_) => None, - Err(env::VarError::NotPresent) => None, - Err(env::VarError::NotUnicode(_)) => Some(CODEX_CONNECTORS_TOKEN_ENV_VAR.to_string()), - } -} - -fn normalize_codex_apps_base_url(base_url: &str) -> String { - let mut base_url = base_url.trim_end_matches('/').to_string(); - if (base_url.starts_with("https://chatgpt.com") - || base_url.starts_with("https://chat.openai.com")) - && !base_url.contains("/backend-api") - { - base_url = format!("{base_url}/backend-api"); - } - base_url -} - -fn codex_apps_mcp_url_for_base_url(base_url: &str) -> String { - let base_url = normalize_codex_apps_base_url(base_url); - if base_url.contains("/backend-api") { - format!("{base_url}/wham/apps") - } else if base_url.contains("/api/codex") { - format!("{base_url}/apps") - } else { - format!("{base_url}/api/codex/apps") - } -} - -pub(crate) fn codex_apps_mcp_url(config: &McpConfig) -> String { - codex_apps_mcp_url_for_base_url(&config.chatgpt_base_url) -} - -fn codex_apps_mcp_server_config(config: &McpConfig) -> McpServerConfig { - let url = codex_apps_mcp_url(config); - - McpServerConfig { - transport: McpServerTransportConfig::StreamableHttp { - url, - bearer_token_env_var: codex_apps_mcp_bearer_token_env_var(), - http_headers: None, - env_http_headers: None, - }, - experimental_environment: None, - enabled: true, - required: false, - supports_parallel_tool_calls: false, - disabled_reason: None, - startup_timeout_sec: Some(Duration::from_secs(30)), - tool_timeout_sec: None, - default_tools_approval_mode: None, - enabled_tools: None, - disabled_tools: None, - scopes: None, - oauth_resource: None, - tools: HashMap::new(), - } -} - pub fn with_codex_apps_mcp( mut servers: HashMap, auth: Option<&CodexAuth>, @@ -395,6 +315,99 @@ pub async fn collect_mcp_server_status_snapshot_with_detail( snapshot } +pub async fn collect_mcp_snapshot_from_manager( + mcp_connection_manager: &McpConnectionManager, + auth_status_entries: HashMap, +) -> McpListToolsResponseEvent { + collect_mcp_snapshot_from_manager_with_detail( + mcp_connection_manager, + auth_status_entries, + McpSnapshotDetail::Full, + ) + .await +} + +pub(crate) fn codex_apps_mcp_url(config: &McpConfig) -> String { + codex_apps_mcp_url_for_base_url(&config.chatgpt_base_url) +} + +/// The Responses API requires tool names to match `^[a-zA-Z0-9_-]+$`. +/// MCP server/tool names are user-controlled, so sanitize the fully-qualified +/// name we expose to the model by replacing any disallowed character with `_`. +pub(crate) fn sanitize_responses_api_tool_name(name: &str) -> String { + let mut sanitized = String::with_capacity(name.len()); + for c in name.chars() { + if c.is_ascii_alphanumeric() || c == '_' { + sanitized.push(c); + } else { + sanitized.push('_'); + } + } + + if sanitized.is_empty() { + "_".to_string() + } else { + sanitized + } +} + +fn codex_apps_mcp_bearer_token_env_var() -> Option { + match env::var(CODEX_CONNECTORS_TOKEN_ENV_VAR) { + Ok(value) if !value.trim().is_empty() => Some(CODEX_CONNECTORS_TOKEN_ENV_VAR.to_string()), + Ok(_) => None, + Err(env::VarError::NotPresent) => None, + Err(env::VarError::NotUnicode(_)) => Some(CODEX_CONNECTORS_TOKEN_ENV_VAR.to_string()), + } +} + +fn normalize_codex_apps_base_url(base_url: &str) -> String { + let mut base_url = base_url.trim_end_matches('/').to_string(); + if (base_url.starts_with("https://chatgpt.com") + || base_url.starts_with("https://chat.openai.com")) + && !base_url.contains("/backend-api") + { + base_url = format!("{base_url}/backend-api"); + } + base_url +} + +fn codex_apps_mcp_url_for_base_url(base_url: &str) -> String { + let base_url = normalize_codex_apps_base_url(base_url); + if base_url.contains("/backend-api") { + format!("{base_url}/wham/apps") + } else if base_url.contains("/api/codex") { + format!("{base_url}/apps") + } else { + format!("{base_url}/api/codex/apps") + } +} + +fn codex_apps_mcp_server_config(config: &McpConfig) -> McpServerConfig { + let url = codex_apps_mcp_url(config); + + McpServerConfig { + transport: McpServerTransportConfig::StreamableHttp { + url, + bearer_token_env_var: codex_apps_mcp_bearer_token_env_var(), + http_headers: None, + env_http_headers: None, + }, + experimental_environment: None, + enabled: true, + required: false, + supports_parallel_tool_calls: false, + disabled_reason: None, + startup_timeout_sec: Some(Duration::from_secs(30)), + tool_timeout_sec: None, + default_tools_approval_mode: None, + enabled_tools: None, + disabled_tools: None, + scopes: None, + oauth_resource: None, + tools: HashMap::new(), + } +} + fn protocol_tool_from_rmcp_tool(name: &str, tool: &rmcp::model::Tool) -> Option { match serde_json::to_value(tool) { Ok(value) => match Tool::from_mcp_value(value) { @@ -543,18 +556,6 @@ async fn collect_mcp_server_status_snapshot_from_manager( } } -pub async fn collect_mcp_snapshot_from_manager( - mcp_connection_manager: &McpConnectionManager, - auth_status_entries: HashMap, -) -> McpListToolsResponseEvent { - collect_mcp_snapshot_from_manager_with_detail( - mcp_connection_manager, - auth_status_entries, - McpSnapshotDetail::Full, - ) - .await -} - async fn collect_mcp_snapshot_from_manager_with_detail( mcp_connection_manager: &McpConnectionManager, auth_status_entries: HashMap, diff --git a/codex-rs/codex-mcp/src/mcp_connection_manager.rs b/codex-rs/codex-mcp/src/mcp_connection_manager.rs index 5a504b1c8d..3b2dffc903 100644 --- a/codex-rs/codex-mcp/src/mcp_connection_manager.rs +++ b/codex-rs/codex-mcp/src/mcp_connection_manager.rs @@ -112,21 +112,6 @@ const MCP_TOOLS_LIST_DURATION_METRIC: &str = "codex.mcp.tools.list.duration_ms"; const MCP_TOOLS_FETCH_UNCACHED_DURATION_METRIC: &str = "codex.mcp.tools.fetch_uncached.duration_ms"; const MCP_TOOLS_CACHE_WRITE_DURATION_METRIC: &str = "codex.mcp.tools.cache_write.duration_ms"; -fn sha1_hex(s: &str) -> String { - let mut hasher = Sha1::new(); - hasher.update(s.as_bytes()); - let sha1 = hasher.finalize(); - format!("{sha1:x}") -} - -pub fn codex_apps_tools_cache_key(auth: Option<&CodexAuth>) -> CodexAppsToolsCacheKey { - CodexAppsToolsCacheKey { - account_id: auth.and_then(CodexAuth::get_account_id), - chatgpt_user_id: auth.and_then(CodexAuth::get_chatgpt_user_id), - is_workspace_account: auth.is_some_and(CodexAuth::is_workspace_account), - } -} - #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ToolInfo { /// Raw MCP server name used for routing the tool call. @@ -155,8 +140,6 @@ impl ToolInfo { } } -const META_OPENAI_FILE_PARAMS: &str = "openai/fileParams"; - pub fn declared_openai_file_input_param_names( meta: Option<&Map>, ) -> Vec { @@ -174,70 +157,6 @@ pub fn declared_openai_file_input_param_names( .collect() } -/// 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. -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())); - } -} - #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct CodexAppsToolsCacheKey { account_id: Option, @@ -245,6 +164,120 @@ pub struct CodexAppsToolsCacheKey { is_workspace_account: bool, } +pub fn codex_apps_tools_cache_key(auth: Option<&CodexAuth>) -> CodexAppsToolsCacheKey { + CodexAppsToolsCacheKey { + account_id: auth.and_then(CodexAuth::get_account_id), + chatgpt_user_id: auth.and_then(CodexAuth::get_chatgpt_user_id), + is_workspace_account: auth.is_some_and(CodexAuth::is_workspace_account), + } +} + +pub fn filter_non_codex_apps_mcp_tools_only( + mcp_tools: &HashMap, +) -> HashMap { + mcp_tools + .iter() + .filter(|(_, tool)| tool.server_name != CODEX_APPS_MCP_SERVER_NAME) + .map(|(name, tool)| (name.clone(), tool.clone())) + .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"; + +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct SandboxState { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub permission_profile: Option, + pub sandbox_policy: SandboxPolicy, + pub codex_linux_sandbox_exe: Option, + pub sandbox_cwd: PathBuf, + #[serde(default)] + pub use_legacy_landlock: bool, +} + +/// A thin wrapper around a set of running [`RmcpClient`] instances. +pub struct McpConnectionManager { + clients: HashMap, + server_origins: HashMap, + elicitation_requests: ElicitationRequestManager, +} + +/// Runtime placement information used when starting MCP server transports. +/// +/// `McpConfig` describes what servers exist. This value describes where those +/// servers should run for the current caller. Keep it explicit at manager +/// construction time so status/snapshot paths and real sessions make the same +/// local-vs-remote decision. `fallback_cwd` is not a per-server override; it is +/// used when a stdio server omits `cwd` and the launcher needs a concrete +/// process working directory. +#[derive(Clone)] +pub struct McpRuntimeEnvironment { + environment: Arc, + fallback_cwd: PathBuf, +} + +impl McpRuntimeEnvironment { + pub fn new(environment: Arc, fallback_cwd: PathBuf) -> Self { + Self { + environment, + fallback_cwd, + } + } + + fn environment(&self) -> Arc { + Arc::clone(&self.environment) + } + + fn fallback_cwd(&self) -> PathBuf { + self.fallback_cwd.clone() + } +} + +/// 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 { + enabled: Option>, + disabled: HashSet, +} + +impl ToolFilter { + 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 } + } + + fn allows(&self, tool_name: &str) -> bool { + if let Some(enabled) = &self.enabled + && !enabled.contains(tool_name) + { + return false; + } + + !self.disabled.contains(tool_name) + } +} + +fn sha1_hex(s: &str) -> String { + let mut hasher = Sha1::new(); + hasher.update(s.as_bytes()); + let sha1 = hasher.finalize(); + format!("{sha1:x}") +} + #[derive(Clone)] struct CodexAppsToolsCacheContext { codex_home: PathBuf, @@ -630,60 +663,6 @@ impl AsyncManagedClient { } } -/// 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"; - -#[derive(Debug, Clone, Serialize, Deserialize)] -#[serde(rename_all = "camelCase")] -pub struct SandboxState { - #[serde(default, skip_serializing_if = "Option::is_none")] - pub permission_profile: Option, - pub sandbox_policy: SandboxPolicy, - pub codex_linux_sandbox_exe: Option, - pub sandbox_cwd: PathBuf, - #[serde(default)] - pub use_legacy_landlock: bool, -} - -/// A thin wrapper around a set of running [`RmcpClient`] instances. -pub struct McpConnectionManager { - clients: HashMap, - server_origins: HashMap, - elicitation_requests: ElicitationRequestManager, -} - -/// Runtime placement information used when starting MCP server transports. -/// -/// `McpConfig` describes what servers exist. This value describes where those -/// servers should run for the current caller. Keep it explicit at manager -/// construction time so status/snapshot paths and real sessions make the same -/// local-vs-remote decision. `fallback_cwd` is not a per-server override; it is -/// used when a stdio server omits `cwd` and the launcher needs a concrete -/// process working directory. -#[derive(Clone)] -pub struct McpRuntimeEnvironment { - environment: Arc, - fallback_cwd: PathBuf, -} - -impl McpRuntimeEnvironment { - pub fn new(environment: Arc, fallback_cwd: PathBuf) -> Self { - Self { - environment, - fallback_cwd, - } - } - - fn environment(&self) -> Arc { - Arc::clone(&self.environment) - } - - fn fallback_cwd(&self) -> PathBuf { - self.fallback_cwd.clone() - } -} - impl McpConnectionManager { pub fn new_uninitialized( approval_policy: &Constrained, @@ -858,15 +837,6 @@ impl McpConnectionManager { (manager, cancel_token) } - async fn client_by_name(&self, name: &str) -> Result { - self.clients - .get(name) - .ok_or_else(|| anyhow!("unknown MCP server '{name}'"))? - .client() - .await - .context("failed to get client") - } - pub async fn resolve_elicitation( &self, server_name: String, @@ -1218,6 +1188,81 @@ impl McpConnectionManager { .into_values() .find(|tool| tool.canonical_tool_name() == *tool_name) } + + async fn client_by_name(&self, name: &str) -> Result { + self.clients + .get(name) + .ok_or_else(|| anyhow!("unknown MCP server '{name}'"))? + .client() + .await + .context("failed to get client") + } +} + +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. +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())); + } } async fn emit_update( @@ -1233,41 +1278,6 @@ async fn emit_update( .await } -/// 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 { - enabled: Option>, - disabled: HashSet, -} - -impl ToolFilter { - 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 } - } - - fn allows(&self, tool_name: &str) -> bool { - if let Some(enabled) = &self.enabled - && !enabled.contains(tool_name) - { - return false; - } - - !self.disabled.contains(tool_name) - } -} - fn filter_tools(tools: Vec, filter: &ToolFilter) -> Vec { tools .into_iter() @@ -1275,16 +1285,6 @@ fn filter_tools(tools: Vec, filter: &ToolFilter) -> Vec { .collect() } -pub fn filter_non_codex_apps_mcp_tools_only( - mcp_tools: &HashMap, -) -> HashMap { - mcp_tools - .iter() - .filter(|(_, tool)| tool.server_name != CODEX_APPS_MCP_SERVER_NAME) - .map(|(name, tool)| (name.clone(), tool.clone())) - .collect() -} - fn normalize_codex_apps_tool_title( server_name: &str, connector_name: Option<&str>, diff --git a/codex-rs/codex-mcp/src/mcp_tool_names.rs b/codex-rs/codex-mcp/src/mcp_tool_names.rs index 5a323dfe71..2d2d100c0a 100644 --- a/codex-rs/codex-mcp/src/mcp_tool_names.rs +++ b/codex-rs/codex-mcp/src/mcp_tool_names.rs @@ -14,95 +14,6 @@ const MCP_TOOL_NAME_DELIMITER: &str = "__"; const MAX_TOOL_NAME_LENGTH: usize = 64; const CALLABLE_NAME_HASH_LEN: usize = 12; -fn sha1_hex(s: &str) -> String { - let mut hasher = Sha1::new(); - hasher.update(s.as_bytes()); - let sha1 = hasher.finalize(); - format!("{sha1:x}") -} - -fn callable_name_hash_suffix(raw_identity: &str) -> String { - let hash = sha1_hex(raw_identity); - format!("_{}", &hash[..CALLABLE_NAME_HASH_LEN]) -} - -fn append_hash_suffix(value: &str, raw_identity: &str) -> String { - format!("{value}{}", callable_name_hash_suffix(raw_identity)) -} - -fn append_namespace_hash_suffix(namespace: &str, raw_identity: &str) -> String { - if let Some(namespace) = namespace.strip_suffix(MCP_TOOL_NAME_DELIMITER) { - format!( - "{}{}{}", - namespace, - callable_name_hash_suffix(raw_identity), - MCP_TOOL_NAME_DELIMITER - ) - } else { - append_hash_suffix(namespace, raw_identity) - } -} - -fn truncate_name(value: &str, max_len: usize) -> String { - value.chars().take(max_len).collect() -} - -fn fit_callable_parts_with_hash( - namespace: &str, - tool_name: &str, - raw_identity: &str, -) -> (String, String) { - let suffix = callable_name_hash_suffix(raw_identity); - let max_tool_len = MAX_TOOL_NAME_LENGTH.saturating_sub(namespace.len()); - if max_tool_len >= suffix.len() { - let prefix_len = max_tool_len - suffix.len(); - return ( - namespace.to_string(), - format!("{}{}", truncate_name(tool_name, prefix_len), suffix), - ); - } - - let max_namespace_len = MAX_TOOL_NAME_LENGTH - suffix.len(); - (truncate_name(namespace, max_namespace_len), suffix) -} - -fn unique_callable_parts( - namespace: &str, - tool_name: &str, - raw_identity: &str, - used_names: &mut HashSet, -) -> (String, String, String) { - let qualified_name = format!("{namespace}{tool_name}"); - if qualified_name.len() <= MAX_TOOL_NAME_LENGTH && used_names.insert(qualified_name.clone()) { - return (namespace.to_string(), tool_name.to_string(), qualified_name); - } - - let mut attempt = 0_u32; - loop { - let hash_input = if attempt == 0 { - raw_identity.to_string() - } else { - format!("{raw_identity}\0{attempt}") - }; - let (namespace, tool_name) = - fit_callable_parts_with_hash(namespace, tool_name, &hash_input); - let qualified_name = format!("{namespace}{tool_name}"); - if used_names.insert(qualified_name.clone()) { - return (namespace, tool_name, qualified_name); - } - attempt = attempt.saturating_add(1); - } -} - -#[derive(Debug)] -struct CallableToolCandidate { - tool: ToolInfo, - raw_namespace_identity: String, - raw_tool_identity: String, - callable_namespace: String, - callable_name: String, -} - /// Returns a qualified-name lookup for MCP tools. /// /// Raw MCP server/tool names are kept on each [`ToolInfo`] for protocol calls, while @@ -200,3 +111,92 @@ where } qualified_tools } + +#[derive(Debug)] +struct CallableToolCandidate { + tool: ToolInfo, + raw_namespace_identity: String, + raw_tool_identity: String, + callable_namespace: String, + callable_name: String, +} + +fn sha1_hex(s: &str) -> String { + let mut hasher = Sha1::new(); + hasher.update(s.as_bytes()); + let sha1 = hasher.finalize(); + format!("{sha1:x}") +} + +fn callable_name_hash_suffix(raw_identity: &str) -> String { + let hash = sha1_hex(raw_identity); + format!("_{}", &hash[..CALLABLE_NAME_HASH_LEN]) +} + +fn append_hash_suffix(value: &str, raw_identity: &str) -> String { + format!("{value}{}", callable_name_hash_suffix(raw_identity)) +} + +fn append_namespace_hash_suffix(namespace: &str, raw_identity: &str) -> String { + if let Some(namespace) = namespace.strip_suffix(MCP_TOOL_NAME_DELIMITER) { + format!( + "{}{}{}", + namespace, + callable_name_hash_suffix(raw_identity), + MCP_TOOL_NAME_DELIMITER + ) + } else { + append_hash_suffix(namespace, raw_identity) + } +} + +fn truncate_name(value: &str, max_len: usize) -> String { + value.chars().take(max_len).collect() +} + +fn fit_callable_parts_with_hash( + namespace: &str, + tool_name: &str, + raw_identity: &str, +) -> (String, String) { + let suffix = callable_name_hash_suffix(raw_identity); + let max_tool_len = MAX_TOOL_NAME_LENGTH.saturating_sub(namespace.len()); + if max_tool_len >= suffix.len() { + let prefix_len = max_tool_len - suffix.len(); + return ( + namespace.to_string(), + format!("{}{}", truncate_name(tool_name, prefix_len), suffix), + ); + } + + let max_namespace_len = MAX_TOOL_NAME_LENGTH - suffix.len(); + (truncate_name(namespace, max_namespace_len), suffix) +} + +fn unique_callable_parts( + namespace: &str, + tool_name: &str, + raw_identity: &str, + used_names: &mut HashSet, +) -> (String, String, String) { + let qualified_name = format!("{namespace}{tool_name}"); + if qualified_name.len() <= MAX_TOOL_NAME_LENGTH && used_names.insert(qualified_name.clone()) { + return (namespace.to_string(), tool_name.to_string(), qualified_name); + } + + let mut attempt = 0_u32; + loop { + let hash_input = if attempt == 0 { + raw_identity.to_string() + } else { + format!("{raw_identity}\0{attempt}") + }; + let (namespace, tool_name) = + fit_callable_parts_with_hash(namespace, tool_name, &hash_input); + let qualified_name = format!("{namespace}{tool_name}"); + if used_names.insert(qualified_name.clone()) { + return (namespace, tool_name, qualified_name); + } + attempt = attempt.saturating_add(1); + } +}