diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index fe1a8fc918..20dd36b6bd 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -4209,6 +4209,7 @@ dependencies = [ name = "codex-tools" version = "0.0.0" dependencies = [ + "bitflags 2.13.1", "codex-code-mode", "codex-connectors", "codex-extension-items", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 32f34068b5..2f5588034b 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -288,6 +288,7 @@ aws-sigv4 = "1" aws-types = "1" axum = { version = "0.8", default-features = false } base64 = "0.22.1" +bitflags = "2.13.1" bm25 = "2.3.2" bytes = "1.10.1" chardetng = "0.1.17" diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index b157b1b82d..2ebd737cf7 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -371,6 +371,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, diff --git a/codex-rs/codex-mcp/src/catalog_tests.rs b/codex-rs/codex-mcp/src/catalog_tests.rs index 6a0515db39..43c23b985f 100644 --- a/codex-rs/codex-mcp/src/catalog_tests.rs +++ b/codex-rs/codex-mcp/src/catalog_tests.rs @@ -28,6 +28,7 @@ fn server(url: &str) -> McpServerConfig { enabled: true, required: true, supports_parallel_tool_calls: true, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(7)), tool_timeout_sec: Some(Duration::from_secs(11)), diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index 461991c866..4510983bfd 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -3098,6 +3098,7 @@ async fn no_local_runtime_fails_local_stdio_but_keeps_local_http_server() { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -3124,6 +3125,7 @@ async fn no_local_runtime_fails_local_stdio_but_keeps_local_http_server() { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -3238,6 +3240,7 @@ fn mcp_init_error_display_prompts_for_github_pat() { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -3329,6 +3332,7 @@ fn mcp_init_error_display_reports_generic_errors() { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -3380,6 +3384,7 @@ fn reusable_server_config(url: &str) -> McpServerConfig { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index 9dadc8fea0..51fc4f3ce7 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -540,6 +540,7 @@ fn mcp_server_config_for_url( enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(30)), tool_timeout_sec: None, diff --git a/codex-rs/codex-mcp/src/mcp/mod_tests.rs b/codex-rs/codex-mcp/src/mcp/mod_tests.rs index 1aa31c934c..ef44d83507 100644 --- a/codex-rs/codex-mcp/src/mcp/mod_tests.rs +++ b/codex-rs/codex-mcp/src/mcp/mod_tests.rs @@ -384,6 +384,7 @@ async fn effective_mcp_servers_preserve_runtime_servers() { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -410,6 +411,7 @@ async fn effective_mcp_servers_preserve_runtime_servers() { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, diff --git a/codex-rs/codex-mcp/src/plugin_config_tests.rs b/codex-rs/codex-mcp/src/plugin_config_tests.rs index c2f1f71a54..0a5337273b 100644 --- a/codex-rs/codex-mcp/src/plugin_config_tests.rs +++ b/codex-rs/codex-mcp/src/plugin_config_tests.rs @@ -44,6 +44,7 @@ fn stdio_server( enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -78,6 +79,7 @@ fn declared_placement_preserves_local_plugin_normalization() { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -350,6 +352,7 @@ fn local_environment_placement_preserves_http_env_references() { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, diff --git a/codex-rs/codex-mcp/src/runtime.rs b/codex-rs/codex-mcp/src/runtime.rs index c0acaf1b03..4d1746cf26 100644 --- a/codex-rs/codex-mcp/src/runtime.rs +++ b/codex-rs/codex-mcp/src/runtime.rs @@ -483,6 +483,7 @@ mod tests { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs index 519def4232..f45b346a0e 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -48,6 +48,7 @@ pub use cloud_config_layers::CloudConfigLayerError; pub use cloud_config_layers::cloud_config_layers_from_fragments; pub use codex_protocol::config_types::ProfileV2Name; pub use codex_protocol::config_types::ProfileV2NameParseError; +pub use codex_protocol::config_types::ToolExposureSurface; pub use codex_utils_absolute_path::AbsolutePathBuf; pub use config_layer_source::ConfigLayer; pub use config_layer_source::ConfigLayerMetadata; diff --git a/codex-rs/config/src/mcp_requirements_tests.rs b/codex-rs/config/src/mcp_requirements_tests.rs index 68b02316c4..7fcf14b477 100644 --- a/codex-rs/config/src/mcp_requirements_tests.rs +++ b/codex-rs/config/src/mcp_requirements_tests.rs @@ -17,6 +17,7 @@ fn stdio_server(command: &str, args: &[&str]) -> McpServerConfig { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, diff --git a/codex-rs/config/src/mcp_types.rs b/codex-rs/config/src/mcp_types.rs index ed5c34643b..ada6b91394 100644 --- a/codex-rs/config/src/mcp_types.rs +++ b/codex-rs/config/src/mcp_types.rs @@ -7,6 +7,7 @@ use std::time::Duration; use base64::Engine; use base64::engine::general_purpose::URL_SAFE_NO_PAD; +use codex_protocol::config_types::ToolExposureSurface; use codex_utils_path_uri::LegacyAppPathString; use schemars::JsonSchema; use serde::Deserialize; @@ -180,6 +181,11 @@ pub struct McpServerConfig { #[serde(default, skip_serializing_if = "std::ops::Not::not")] pub supports_parallel_tool_calls: bool, + /// Model-facing surfaces from which this server's tools must be omitted. + /// `None` leaves lower-priority configuration unchanged; an empty list clears it. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub omit_tools_from: Option>, + /// Reason this server was disabled after applying requirements. #[serde(skip)] pub disabled_reason: Option, @@ -303,6 +309,8 @@ pub struct RawMcpServerConfig { #[serde(default)] pub supports_parallel_tool_calls: Option, #[serde(default)] + pub omit_tools_from: Option>, + #[serde(default)] pub default_tools_approval_mode: Option, #[serde(default)] pub enabled_tools: Option>, @@ -344,6 +352,7 @@ impl TryFrom for McpServerConfig { enabled, required, supports_parallel_tool_calls, + omit_tools_from, default_tools_approval_mode, enabled_tools, disabled_tools, @@ -421,6 +430,7 @@ impl TryFrom for McpServerConfig { enabled: enabled.unwrap_or_else(default_enabled), required: required.unwrap_or_default(), supports_parallel_tool_calls: supports_parallel_tool_calls.unwrap_or_default(), + omit_tools_from, disabled_reason: None, default_tools_approval_mode, enabled_tools, diff --git a/codex-rs/config/src/mcp_types_tests.rs b/codex-rs/config/src/mcp_types_tests.rs index 598a532971..e291ed392e 100644 --- a/codex-rs/config/src/mcp_types_tests.rs +++ b/codex-rs/config/src/mcp_types_tests.rs @@ -25,6 +25,7 @@ fn deserialize_stdio_command_server_config() { ); assert!(cfg.enabled); assert!(!cfg.required); + assert_eq!(cfg.omit_tools_from, None); assert!(cfg.enabled_tools.is_none()); assert!(cfg.disabled_tools.is_none()); } @@ -362,6 +363,41 @@ fn deserialize_server_config_with_parallel_tool_calls() { assert!(cfg.supports_parallel_tool_calls); } +#[test] +fn serialize_round_trips_server_config_with_omitted_tool_exposure_surfaces() { + for omitted_surfaces in [ + vec![], + vec![ToolExposureSurface::CodeMode], + vec![ToolExposureSurface::Deferred], + vec![ToolExposureSurface::Direct], + vec![ToolExposureSurface::CodeMode, ToolExposureSurface::Deferred], + vec![ToolExposureSurface::CodeMode, ToolExposureSurface::Direct], + vec![ToolExposureSurface::Deferred, ToolExposureSurface::Direct], + vec![ + ToolExposureSurface::CodeMode, + ToolExposureSurface::Deferred, + ToolExposureSurface::Direct, + ], + ] { + let serialized_surfaces = omitted_surfaces + .iter() + .map(|surface| format!("\"{surface}\"")) + .collect::>() + .join(", "); + let config = format!("command = \"echo\"\nomit_tools_from = [{serialized_surfaces}]\n"); + let cfg: McpServerConfig = + toml::from_str(&config).expect("should deserialize omitted MCP exposure surfaces"); + assert_eq!(cfg.omit_tools_from, Some(omitted_surfaces.clone())); + + let serialized = toml::to_string(&cfg).expect("should serialize MCP config"); + assert!(serialized.contains(&format!("omit_tools_from = [{serialized_surfaces}]"))); + + let round_tripped: McpServerConfig = + toml::from_str(&serialized).expect("should deserialize serialized MCP config"); + assert_eq!(round_tripped, cfg); + } +} + #[test] fn deserialize_server_config_with_default_tool_approval_mode() { let cfg: McpServerConfig = toml::from_str( @@ -438,6 +474,7 @@ fn deserialize_ignores_unknown_server_fields() { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 82fa89b83b..c2a90decbd 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -947,6 +947,7 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -1039,6 +1040,7 @@ enabled = true enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -2068,6 +2070,7 @@ async fn load_plugins_uses_manifest_configured_component_paths() { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -2397,6 +2400,7 @@ async fn load_plugins_ignores_manifest_component_paths_without_dot_slash() { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -2649,6 +2653,7 @@ fn capability_index_filters_inactive_and_zero_capability_plugins() { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index e9df85d783..8783b9b0c8 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -2692,6 +2692,13 @@ "default": null, "type": "string" }, + "omit_tools_from": { + "default": null, + "items": { + "$ref": "#/definitions/ToolExposureSurface" + }, + "type": "array" + }, "required": { "default": null, "type": "boolean" @@ -3121,6 +3128,32 @@ }, "type": "object" }, + "ToolExposureSurface": { + "description": "A model-facing surface on which a tool can be exposed.", + "oneOf": [ + { + "description": "Nested tools available to Code Mode scripts.", + "enum": [ + "code_mode" + ], + "type": "string" + }, + { + "description": "Tools discovered later through tool search.", + "enum": [ + "deferred" + ], + "type": "string" + }, + { + "description": "Tools present in the model's initial tool list.", + "enum": [ + "direct" + ], + "type": "string" + } + ] + }, "ToolSuggestConfig": { "additionalProperties": false, "properties": { diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 1c78429ea7..c7e8ddfcca 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -136,6 +136,7 @@ fn stdio_mcp_with_args(command: &str, args: &[&str]) -> McpServerConfig { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -162,6 +163,7 @@ fn http_mcp(url: &str) -> McpServerConfig { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -6093,6 +6095,7 @@ async fn replace_mcp_servers_round_trips_entries() -> anyhow::Result<()> { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(3)), tool_timeout_sec: Some(Duration::from_secs(5)), @@ -6492,6 +6495,7 @@ async fn replace_mcp_servers_serializes_env_sorted() -> anyhow::Result<()> { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -6569,6 +6573,7 @@ async fn replace_mcp_servers_serializes_env_vars() -> anyhow::Result<()> { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -6631,6 +6636,7 @@ async fn replace_mcp_servers_serializes_sourced_env_vars() -> anyhow::Result<()> enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -6684,6 +6690,7 @@ async fn replace_mcp_servers_serializes_cwd() -> anyhow::Result<()> { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -6739,6 +6746,7 @@ async fn replace_mcp_servers_streamable_http_serializes_bearer_token() -> anyhow enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(2)), tool_timeout_sec: None, @@ -6810,6 +6818,7 @@ async fn replace_mcp_servers_streamable_http_serializes_custom_headers() -> anyh enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(2)), tool_timeout_sec: None, @@ -6893,6 +6902,7 @@ async fn replace_mcp_servers_streamable_http_removes_optional_sections() -> anyh enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(2)), tool_timeout_sec: None, @@ -6929,6 +6939,7 @@ async fn replace_mcp_servers_streamable_http_removes_optional_sections() -> anyh enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -7000,6 +7011,7 @@ async fn replace_mcp_servers_streamable_http_isolates_headers_between_servers() enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(2)), tool_timeout_sec: None, @@ -7027,6 +7039,7 @@ async fn replace_mcp_servers_streamable_http_isolates_headers_between_servers() enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -7116,6 +7129,7 @@ async fn replace_mcp_servers_serializes_disabled_flag() -> anyhow::Result<()> { enabled: false, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -7167,6 +7181,7 @@ async fn replace_mcp_servers_serializes_required_flag() -> anyhow::Result<()> { enabled: true, required: true, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -7218,6 +7233,7 @@ async fn replace_mcp_servers_serializes_tool_filters() -> anyhow::Result<()> { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -7273,6 +7289,7 @@ async fn replace_mcp_servers_streamable_http_serializes_oauth_resource() -> anyh enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, diff --git a/codex-rs/core/src/config/edit/document_helpers.rs b/codex-rs/core/src/config/edit/document_helpers.rs index 871ca0e41e..1becb23ebb 100644 --- a/codex-rs/core/src/config/edit/document_helpers.rs +++ b/codex-rs/core/src/config/edit/document_helpers.rs @@ -111,6 +111,9 @@ fn serialize_mcp_server_table(config: &McpServerConfig) -> TomlTable { if config.supports_parallel_tool_calls { entry["supports_parallel_tool_calls"] = value(true); } + if let Some(omit_tools_from) = &config.omit_tools_from { + entry["omit_tools_from"] = array_from_iter(omit_tools_from.iter().map(ToString::to_string)); + } if let Some(timeout) = config.startup_timeout_sec { entry["startup_timeout_sec"] = value(timeout.as_secs_f64()); } diff --git a/codex-rs/core/src/config/edit_tests.rs b/codex-rs/core/src/config/edit_tests.rs index 84989596b3..4e0b1022d6 100644 --- a/codex-rs/core/src/config/edit_tests.rs +++ b/codex-rs/core/src/config/edit_tests.rs @@ -1013,6 +1013,7 @@ fn blocking_replace_mcp_servers_round_trips() { enabled: true, required: false, supports_parallel_tool_calls: true, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -1044,6 +1045,7 @@ fn blocking_replace_mcp_servers_round_trips() { enabled: false, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: Some(std::time::Duration::from_secs(5)), tool_timeout_sec: None, @@ -1116,6 +1118,7 @@ fn blocking_replace_mcp_servers_serializes_tool_approval_overrides() { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -1177,6 +1180,7 @@ foo = { command = "cmd" } enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -1228,6 +1232,7 @@ foo = { command = "cmd" } # keep me enabled: false, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -1278,6 +1283,7 @@ foo = { command = "cmd", args = ["--flag"] } # keep me enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -1329,6 +1335,7 @@ foo = { command = "cmd" } enabled: false, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, diff --git a/codex-rs/core/src/mcp_skill_dependencies.rs b/codex-rs/core/src/mcp_skill_dependencies.rs index 0ff3904599..a02a7db949 100644 --- a/codex-rs/core/src/mcp_skill_dependencies.rs +++ b/codex-rs/core/src/mcp_skill_dependencies.rs @@ -386,6 +386,7 @@ fn mcp_dependency_to_server_config( enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -417,6 +418,7 @@ fn mcp_dependency_to_server_config( enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, diff --git a/codex-rs/core/src/mcp_tool_exposure.rs b/codex-rs/core/src/mcp_tool_exposure.rs index 218dde9509..60ce08c502 100644 --- a/codex-rs/core/src/mcp_tool_exposure.rs +++ b/codex-rs/core/src/mcp_tool_exposure.rs @@ -7,6 +7,7 @@ use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; use codex_mcp::ToolInfo as McpToolInfo; use codex_mcp::tool_is_model_visible; use codex_tools::ToolExposure; +use codex_tools::ToolName; use tracing::instrument; use tracing::warn; @@ -22,7 +23,7 @@ pub(crate) fn append_mcp_tools( config: &Config, search_tool_enabled: bool, registry: &mut ToolRegistry, -) { +) -> HashSet { // Keep regular MCP tools first; Apps tools also require connector and policy checks. let non_app_tools = filter_non_codex_apps_mcp_tools_only(all_mcp_tools); let app_tools = connectors @@ -33,17 +34,21 @@ pub(crate) fn append_mcp_tools( } else { ToolExposure::Direct }; + let mut registered_tools = HashSet::new(); for tool in non_app_tools.chain(app_tools) { let tool_name = tool.canonical_tool_name(); match McpHandler::new(tool.clone()) { Ok(handler) => { - registry.register_external_with_exposure(Arc::new(handler), exposure); + if registry.register_external_with_exposure(Arc::new(handler), exposure) { + registered_tools.insert(tool_name); + } } Err(err) => { warn!("Skipping MCP tool `{tool_name}`: failed to build tool spec: {err}"); } } } + registered_tools } fn filter_non_codex_apps_mcp_tools_only( diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index df385256b5..334d1338ba 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -8502,6 +8502,7 @@ async fn step_context_keeps_its_mcp_runtime_for_tools() -> anyhow::Result<()> { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, diff --git a/codex-rs/core/src/tools/context.rs b/codex-rs/core/src/tools/context.rs index fb6c05cc76..70d77c4298 100644 --- a/codex-rs/core/src/tools/context.rs +++ b/codex-rs/core/src/tools/context.rs @@ -100,10 +100,8 @@ impl ToolOutput for McpToolOutput { } } - fn code_mode_result(&self, _payload: &ToolPayload) -> JsonValue { - serde_json::to_value(&self.result).unwrap_or_else(|err| { - JsonValue::String(format!("failed to serialize mcp result: {err}")) - }) + fn code_mode_result(&self, payload: &ToolPayload) -> JsonValue { + self.result.code_mode_result(payload) } fn post_tool_use_input(&self, _payload: &ToolPayload) -> Option { diff --git a/codex-rs/core/src/tools/context_tests.rs b/codex-rs/core/src/tools/context_tests.rs index caed87dfa5..511705ffe6 100644 --- a/codex-rs/core/src/tools/context_tests.rs +++ b/codex-rs/core/src/tools/context_tests.rs @@ -46,7 +46,7 @@ fn function_payloads_remain_function_outputs() { } #[test] -fn mcp_code_mode_result_serializes_full_call_tool_result() { +fn mcp_code_mode_result_omits_private_metadata() { let output = CallToolResult { content: vec![serde_json::json!({ "type": "text", @@ -78,11 +78,9 @@ fn mcp_code_mode_result_serializes_full_call_tool_result() { "content": "done", }, "isError": false, - "_meta": { - "source": "mcp", - }, }) ); + assert_eq!(output.meta, Some(serde_json::json!({ "source": "mcp" }))); } #[test] @@ -232,7 +230,7 @@ fn mcp_tool_output_response_item_preserves_content_items() { } #[test] -fn mcp_tool_output_code_mode_result_stays_raw_call_tool_result() { +fn mcp_tool_output_code_mode_result_preserves_content_without_private_metadata() { let large_content = "large structured value ".repeat(1_000); let output = McpToolOutput { result: CallToolResult { @@ -244,7 +242,9 @@ fn mcp_tool_output_code_mode_result_stays_raw_call_tool_result() { "content": large_content, })), is_error: Some(false), - meta: None, + meta: Some(serde_json::json!({ + "hive_dispatch_id": "private-dispatch-id", + })), }, tool_input: json!({}), wall_time: std::time::Duration::from_millis(1250), @@ -269,6 +269,10 @@ fn mcp_tool_output_code_mode_result_stays_raw_call_tool_result() { "isError": false, }) ); + assert_eq!( + output.result.meta, + Some(serde_json::json!({ "hive_dispatch_id": "private-dispatch-id" })) + ); } #[test] diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 3990fe049f..898523b090 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -360,7 +360,7 @@ impl ToolRegistry { pub(crate) fn deferred_tool_namespaces(&self) -> BTreeMap { let mut namespaces = BTreeMap::::new(); for (name, tool) in &self.tools { - if tool.exposure != ToolExposure::Deferred { + if !tool.exposure.is_deferred() { continue; } let Some(namespace) = &name.namespace else { diff --git a/codex-rs/core/src/tools/router_tests.rs b/codex-rs/core/src/tools/router_tests.rs index c9654ec7a1..4549eac0b3 100644 --- a/codex-rs/core/src/tools/router_tests.rs +++ b/codex-rs/core/src/tools/router_tests.rs @@ -300,6 +300,15 @@ async fn mcp_parallel_support_uses_handler_data() -> anyhow::Result<()> { "query_with_delay", )) }, + RegisteredTool { + exposure: ToolExposure::CodeModeOnly, + ..mcp_runtime(mcp_tool_info( + "nested_echo", + /*supports_parallel_tool_calls*/ true, + "mcp__nested_echo__", + "query_with_delay", + )) + }, ], Vec::new(), &turn.dynamic_tools, @@ -348,6 +357,16 @@ async fn mcp_parallel_support_uses_handler_data() -> anyhow::Result<()> { assert!(!router.tool_supports_parallel(&hidden_call)); assert!(router.tool_runtime(&hidden_call).is_some()); + let nested_only_call = ToolCall { + tool_name: ToolName::namespaced("mcp__nested_echo__", "query_with_delay"), + call_id: "call-nested-only-server".to_string(), + payload: ToolPayload::Function { + arguments: "{}".to_string(), + }, + encrypted_function_args: None, + }; + assert!(router.tool_supports_parallel(&nested_only_call)); + Ok(()) } diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index c4e10393d7..36ab4758fa 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -78,6 +78,7 @@ use codex_tools::TOOL_SEARCH_TOOL_NAME; use codex_tools::ToolCall as ExtensionToolCall; use codex_tools::ToolEnvironmentMode; use codex_tools::ToolExecutor; +use codex_tools::ToolExposures; use codex_tools::ToolName; use codex_tools::ToolSearchInfo; use codex_tools::ToolSpec; @@ -91,6 +92,8 @@ use codex_tools::shell_command_backend_for_features; use codex_tools::shell_type_for_model_and_features; use futures::future::BoxFuture; use std::collections::BTreeMap; +use std::collections::HashMap; +use std::collections::HashSet; use std::collections::btree_map::Entry; use std::sync::Arc; use tracing::instrument; @@ -142,13 +145,14 @@ pub(crate) fn build_tool_router( { Vec::new() } else { - append_mcp_tools( + let registered_mcp_tools = append_mcp_tools( mcp.tools(), connectors, &turn_context.config, search_tool_enabled(turn_context), &mut registry, ); + apply_mcp_tool_exposure_policy(turn_context, mcp, ®istered_mcp_tools, &mut registry); let standalone_web_search_tool = append_extension_tool_executors( turn_context, extension_tool_executors(session, step_store), @@ -166,6 +170,78 @@ pub(crate) fn build_tool_router( ) } +fn apply_mcp_tool_exposure_policy( + turn_context: &TurnContext, + mcp: &codex_mcp::McpBinding, + registered_mcp_tools: &HashSet, + registry: &mut ToolRegistry, +) { + let mut omitted_exposures_by_tool = HashMap::new(); + for tool in mcp.tools() { + let tool_name = tool.canonical_tool_name(); + if !registered_mcp_tools.contains(&tool_name) { + continue; + } + let Some(server) = mcp.config().mcp_server_catalog.server(&tool.server_name) else { + continue; + }; + omitted_exposures_by_tool + .entry(tool_name) + .or_insert_with(|| { + server + .config() + .omit_tools_from + .as_deref() + .unwrap_or_default() + .iter() + .copied() + .collect::() + }); + } + + for tool in registry.entries_mut() { + let tool_name = tool.runtime.tool_name(); + let Some(omitted_exposures) = omitted_exposures_by_tool.get(&tool_name) else { + continue; + }; + + let mut exposures = ToolExposures::ALL.difference(*omitted_exposures); + if tool_name.namespace.as_ref().is_some_and(|namespace| { + turn_context + .config + .code_mode + .direct_only_tool_namespaces + .contains(namespace) + }) { + exposures = exposures.difference(ToolExposures::DEFERRED | ToolExposures::CODE_MODE); + } + + exposures = if search_tool_enabled(turn_context) + && exposures.contains(ToolExposures::DEFERRED) + && (effective_tool_mode(turn_context) != ToolMode::CodeModeOnly + || exposures.contains(ToolExposures::CODE_MODE)) + { + exposures.difference(ToolExposures::DIRECT) + } else { + exposures.difference(ToolExposures::DEFERRED) + }; + + tool.exposure = match ( + exposures.contains(ToolExposures::DIRECT), + exposures.contains(ToolExposures::DEFERRED), + exposures.contains(ToolExposures::CODE_MODE), + ) { + (false, false, false) => ToolExposure::Hidden, + (false, false, true) => ToolExposure::CodeModeOnly, + (true, false, false) => ToolExposure::DirectModelOnly, + (true, false, true) => ToolExposure::Direct, + (false, true, false) => ToolExposure::DeferredModelOnly, + (false, true, true) => ToolExposure::Deferred, + (true, true, _) => unreachable!("direct and deferred exposure are mutually exclusive"), + }; + } +} + #[cfg(test)] #[instrument(level = "trace", skip_all)] pub(crate) fn build_core_tool_registry( @@ -252,7 +328,7 @@ pub(crate) fn finalize_tool_router( if search_tool_enabled(turn_context) && registry.entries().any(|tool| { tool.runtime.tool_name() != tool_search_name - && tool.exposure == ToolExposure::Deferred + && tool.exposure.is_deferred() && tool.runtime.search_info().is_some() }) { @@ -1021,7 +1097,7 @@ fn append_tool_search_executor( ) { let search_infos = registry .entries() - .filter(|tool| tool.exposure == ToolExposure::Deferred) + .filter(|tool| tool.exposure.is_deferred()) .filter_map(|tool| tool.runtime.search_info()) .collect::>(); let source_listing = if turn_context diff --git a/codex-rs/core/tests/suite/code_mode.rs b/codex-rs/core/tests/suite/code_mode.rs index 30c89d2068..5ed90d2b62 100644 --- a/codex-rs/core/tests/suite/code_mode.rs +++ b/codex-rs/core/tests/suite/code_mode.rs @@ -40,6 +40,7 @@ use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_custom_tool_call; use core_test_support::responses::ev_response_created; +use core_test_support::responses::namespace_child_tool; use core_test_support::responses::sse; use core_test_support::skip_if_no_network; use core_test_support::skip_if_wine_exec; @@ -71,6 +72,9 @@ use wiremock::ResponseTemplate; use wiremock::matchers::method; use wiremock::matchers::path; +use super::rmcp_client::remote_aware_environment_id; +use super::rmcp_client::remote_aware_stdio_server_bin; + fn custom_tool_output_items(req: &ResponsesRequest, call_id: &str) -> Vec { match req.custom_tool_call_output(call_id).get("output") { Some(Value::Array(items)) => items.clone(), @@ -618,6 +622,7 @@ async fn run_code_mode_turn_with_rmcp_config( enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: None, @@ -808,6 +813,360 @@ async fn code_mode_only_restricts_prompt_tools() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn code_mode_excludes_mcp_servers_using_their_configured_identity() -> Result<()> { + skip_if_wine_exec!( + Ok(()), + "requires a Windows test_stdio_server in the Wine-exec environment" + ); + skip_if_no_network!(Ok(())); + + for omit_tools_from in [ + &["code_mode"][..], + &["deferred"][..], + &["direct"][..], + &["code_mode", "deferred"][..], + &["code_mode", "direct"][..], + &["deferred", "direct"][..], + &["code_mode", "deferred", "direct"][..], + &[][..], + ] { + for (code_mode_only, supports_search_tool, non_prefixed_mcp_tool_names) in [ + (false, false, false), + (false, false, true), + (false, true, false), + (false, true, true), + (true, false, false), + (true, false, true), + (true, true, false), + (true, true, true), + ] { + let server = responses::start_mock_server().await; + let response = responses::mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-1"), + ev_assistant_message("msg-1", "done"), + ev_completed("resp-1"), + ]), + ) + .await; + let rmcp_test_server_bin = remote_aware_stdio_server_bin()?; + let environment_id = remote_aware_environment_id(); + let mut builder = test_codex() + .with_model_info_override("gpt-5.4", move |model| { + model.supports_search_tool = supports_search_tool; + }) + .with_config(move |config| { + config + .features + .enable(if code_mode_only { + Feature::CodeModeOnly + } else { + Feature::CodeMode + }) + .expect("test config should allow Code Mode"); + if non_prefixed_mcp_tool_names { + config + .features + .enable(Feature::NonPrefixedMcpToolNames) + .expect("test config should allow unprefixed MCP tools"); + } + + let mut servers = config.mcp_servers.get().clone(); + servers.insert( + "rmcp".to_string(), + serde_json::from_value(serde_json::json!({ + "command": rmcp_test_server_bin, + "environment_id": environment_id, + "omit_tools_from": omit_tools_from, + })) + .expect("test MCP server config should be valid"), + ); + config + .mcp_servers + .set(servers) + .expect("test config should allow MCP servers"); + }); + let base_test = builder.build_with_auto_env(&server).await?; + let namespace = if non_prefixed_mcp_tool_names { + "rmcp" + } else { + "mcp__rmcp" + }; + let new_thread = base_test + .thread_manager + .start_thread(StartThreadOptions { + dynamic_tools: vec![DynamicToolSpec::Namespace(DynamicToolNamespaceSpec { + name: namespace.to_string(), + description: "An unrelated dynamic tool sharing the MCP namespace." + .to_string(), + tools: vec![DynamicToolNamespaceTool::Function( + DynamicToolFunctionSpec { + name: "unrelated".to_string(), + description: "A dynamic tool that must remain in Code Mode." + .to_string(), + input_schema: serde_json::json!({ + "type": "object", + "properties": {}, + "additionalProperties": false, + }), + defer_loading: false, + }, + )], + })], + ..StartThreadOptions::new(base_test.config.clone()) + }) + .await?; + let mut test = base_test; + test.codex = new_thread.thread; + test.session_configured = new_thread.session_configured; + wait_for_mcp_server(&test.codex, "rmcp").await?; + test.submit_turn("inspect the directly callable MCP tool") + .await?; + + let body = response.single_request().body_json(); + let omit_code_mode = omit_tools_from.contains(&"code_mode"); + let omit_deferred = omit_tools_from.contains(&"deferred"); + let omit_direct = omit_tools_from.contains(&"direct"); + let available_directly = !omit_direct + && (!supports_search_tool || omit_deferred || code_mode_only && omit_code_mode); + let visible_directly = available_directly && (!code_mode_only || omit_code_mode); + let echo_tool = namespace_child_tool(&body, namespace, "echo"); + assert_eq!( + echo_tool.is_some(), + visible_directly, + "MCP omissions must determine initial prompt exposure; \ + omit_tools_from={omit_tools_from:?}, code_mode_only={code_mode_only}, \ + supports_search_tool={supports_search_tool}, namespace={namespace}, tools={:?}", + tool_names(&body) + ); + if let Some(echo_tool) = echo_tool { + assert_eq!( + echo_tool["description"] + .as_str() + .is_some_and(|description| description.contains("exec tool declaration:")), + !omit_code_mode, + "the MCP tool description must agree with Code Mode availability; \ + omit_tools_from={omit_tools_from:?}, code_mode_only={code_mode_only}, \ + namespace={namespace}" + ); + } + + let exec_description = body["tools"] + .as_array() + .expect("request should contain tools") + .iter() + .find_map(|tool| { + (tool["name"].as_str() == Some("exec")) + .then(|| tool["description"].as_str()) + .flatten() + }) + .expect("Code Mode exec should remain available"); + let nested_tool_name = format!("{namespace}__echo"); + if code_mode_only { + assert_eq!( + exec_description.contains(&nested_tool_name), + !omit_code_mode && (!supports_search_tool || omit_deferred), + "the Code Mode declaration must agree with MCP tool exposure; \ + omit_tools_from={omit_tools_from:?}, \ + supports_search_tool={supports_search_tool}, \ + nested_tool_name={nested_tool_name}" + ); + } + let unrelated_nested_tool_name = format!("{namespace}__unrelated"); + if code_mode_only { + assert!( + exec_description.contains(&unrelated_nested_tool_name), + "MCP omissions must not exclude unrelated tools sharing its namespace; \ + omit_tools_from={omit_tools_from:?}, \ + supports_search_tool={supports_search_tool}, \ + nested_tool_name={unrelated_nested_tool_name}" + ); + } else { + let unrelated_tool = namespace_child_tool(&body, namespace, "unrelated") + .unwrap_or_else(|| { + panic!( + "MCP omissions must not hide an unrelated same-namespace tool; \ + omit_tools_from={omit_tools_from:?}, \ + supports_search_tool={supports_search_tool}, namespace={namespace}, \ + tools={:?}", + tool_names(&body) + ) + }); + assert!( + unrelated_tool["description"] + .as_str() + .is_some_and(|description| { + description.contains(&unrelated_nested_tool_name) + && description.contains("exec tool declaration:") + }), + "an unrelated same-namespace tool must remain available inside Code Mode; \ + omit_tools_from={omit_tools_from:?}, \ + supports_search_tool={supports_search_tool}, \ + nested_tool_name={unrelated_nested_tool_name}" + ); + } + } + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn mcp_code_mode_exclusion_does_not_change_direct_mode_tool_exposure() -> Result<()> { + skip_if_wine_exec!( + Ok(()), + "requires a Windows test_stdio_server in the Wine-exec environment" + ); + skip_if_no_network!(Ok(())); + + for omit_tools_from in [ + &["code_mode"][..], + &["deferred"][..], + &["direct"][..], + &["code_mode", "deferred"][..], + &["code_mode", "direct"][..], + &["deferred", "direct"][..], + &["code_mode", "deferred", "direct"][..], + &[][..], + ] { + for (supports_search_tool, non_prefixed_mcp_tool_names, direct_only_namespace) in [ + (false, false, false), + (false, false, true), + (false, true, false), + (false, true, true), + (true, false, false), + (true, false, true), + (true, true, false), + (true, true, true), + ] { + let namespace = if non_prefixed_mcp_tool_names { + "rmcp" + } else { + "mcp__rmcp" + }; + let visible_directly = !omit_tools_from.contains(&"direct") + && (!supports_search_tool + || omit_tools_from.contains(&"deferred") + || direct_only_namespace); + let server = responses::start_mock_server().await; + let response = responses::mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-1"), + if visible_directly { + responses::ev_function_call_with_namespace( + "call-1", + namespace, + "echo", + r#"{"message":"ping"}"#, + ) + } else { + ev_assistant_message("msg-1", "done") + }, + ev_completed("resp-1"), + ]), + ) + .await; + let follow_up_response = if visible_directly { + Some( + responses::mount_sse_once( + &server, + sse(vec![ + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]), + ) + .await, + ) + } else { + None + }; + let rmcp_test_server_bin = remote_aware_stdio_server_bin()?; + let environment_id = remote_aware_environment_id(); + let mut builder = test_codex() + .with_model_info_override("gpt-5.4", move |model| { + model.supports_search_tool = supports_search_tool; + }) + .with_config(move |config| { + if non_prefixed_mcp_tool_names { + config + .features + .enable(Feature::NonPrefixedMcpToolNames) + .expect("test config should allow unprefixed MCP tools"); + } + if direct_only_namespace { + config.code_mode.direct_only_tool_namespaces = vec![ + if non_prefixed_mcp_tool_names { + "rmcp" + } else { + "mcp__rmcp" + } + .to_string(), + ]; + } + + let mut servers = config.mcp_servers.get().clone(); + servers.insert( + "rmcp".to_string(), + serde_json::from_value(serde_json::json!({ + "command": rmcp_test_server_bin, + "environment_id": environment_id, + "omit_tools_from": omit_tools_from, + })) + .expect("test MCP server config should be valid"), + ); + config + .mcp_servers + .set(servers) + .expect("test config should allow MCP servers"); + }); + let test = builder.build_with_auto_env(&server).await?; + wait_for_mcp_server(&test.codex, "rmcp").await?; + test.submit_turn("inspect ordinary direct-mode MCP tool exposure") + .await?; + + let body = response.single_request().body_json(); + assert_eq!( + namespace_child_tool(&body, namespace, "echo").is_some(), + visible_directly, + "MCP omissions must determine direct-mode tool exposure; \ + omit_tools_from={omit_tools_from:?}, \ + supports_search_tool={supports_search_tool}, \ + direct_only_namespace={direct_only_namespace}, namespace={namespace}, tools={:?}", + tool_names(&body) + ); + assert_eq!( + tool_names(&body).iter().any(|name| name == "tool_search"), + supports_search_tool, + "MCP omissions must not disable tool search for other tools; \ + omit_tools_from={omit_tools_from:?}, \ + supports_search_tool={supports_search_tool}, \ + direct_only_namespace={direct_only_namespace}, namespace={namespace}, tools={:?}", + tool_names(&body) + ); + if let Some(follow_up_response) = follow_up_response { + let output = follow_up_response + .single_request() + .function_call_output("call-1"); + assert!( + output["output"] + .as_str() + .is_some_and(|output| output.contains("ECHOING: ping")), + "the advertised MCP tool must remain callable; \ + omit_tools_from={omit_tools_from:?}, \ + supports_search_tool={supports_search_tool}, \ + direct_only_namespace={direct_only_namespace}, \ + namespace={namespace}, output={output:?}" + ); + } + } + } + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn code_mode_only_guides_all_tools_search_and_calls_deferred_app_tools() -> Result<()> { skip_if_no_network!(Ok(())); @@ -4292,6 +4651,289 @@ text(JSON.stringify({ Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn code_mode_omits_configured_mcp_server_tools() -> Result<()> { + skip_if_wine_exec!( + Ok(()), + "requires a Windows test_stdio_server in the Wine-exec environment" + ); + skip_if_no_network!(Ok(())); + + let server = responses::start_mock_server().await; + let rmcp_test_server_bin = remote_aware_stdio_server_bin()?; + let environment_id = remote_aware_environment_id(); + let mut builder = test_codex() + .with_model_info_override("gpt-5.4", |model| { + model.supports_search_tool = false; + }) + .with_config(move |config| { + let _ = config.features.enable(Feature::CodeMode); + let mut servers = config.mcp_servers.get().clone(); + servers.insert( + "rmcp".to_string(), + serde_json::from_value(serde_json::json!({ + "command": rmcp_test_server_bin, + "environment_id": environment_id, + "omit_tools_from": ["code_mode"], + })) + .expect("test MCP server config should be valid"), + ); + config + .mcp_servers + .set(servers) + .expect("test config should allow MCP servers"); + }); + let test = builder.build_with_auto_env(&server).await?; + wait_for_mcp_server(&test.codex, "rmcp").await?; + + let first_mock = responses::mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-1"), + ev_custom_tool_call( + "call-1", + "exec", + r#" +text(JSON.stringify({ + excludedType: typeof tools.mcp__rmcp__echo, + excludedMetadata: ALL_TOOLS.some(({ name }) => name === "mcp__rmcp__echo"), + allowedType: typeof tools.update_plan, + allowedMetadata: ALL_TOOLS.some(({ name }) => name === "update_plan"), +})); +"#, + ), + ev_completed("resp-1"), + ]), + ) + .await; + let second_mock = responses::mount_sse_once( + &server, + sse(vec![ + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]), + ) + .await; + + test.submit_turn("use exec to inspect nested tool namespaces") + .await?; + + assert!( + tool_names(&first_mock.single_request().body_json()).contains(&"mcp__rmcp".to_string()), + "omitted MCP server should remain directly exposed in mixed code mode" + ); + let request = second_mock.single_request(); + let (output, success) = custom_tool_output_body_and_success(&request, "call-1"); + assert_ne!( + success, + Some(false), + "exec configured MCP omission failed unexpectedly: {output}" + ); + assert_eq!( + serde_json::from_str::(&output)?, + serde_json::json!({ + "excludedType": "undefined", + "excludedMetadata": false, + "allowedType": "function", + "allowedMetadata": true, + }) + ); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn code_mode_only_keeps_mcp_tools_direct_when_nested_exposure_is_omitted() -> Result<()> { + skip_if_wine_exec!( + Ok(()), + "requires a Windows test_stdio_server in the Wine-exec environment" + ); + skip_if_no_network!(Ok(())); + + let server = responses::start_mock_server().await; + let rmcp_test_server_bin = remote_aware_stdio_server_bin()?; + let environment_id = remote_aware_environment_id(); + let mut builder = test_codex() + .with_model_info_override("gpt-5.4", |model| { + model.supports_search_tool = true; + }) + .with_config(move |config| { + let _ = config.features.enable(Feature::CodeModeOnly); + let mut servers = config.mcp_servers.get().clone(); + servers.insert( + "rmcp".to_string(), + serde_json::from_value(serde_json::json!({ + "command": rmcp_test_server_bin, + "environment_id": environment_id, + "omit_tools_from": ["code_mode"], + })) + .expect("test MCP server config should be valid"), + ); + config + .mcp_servers + .set(servers) + .expect("test config should allow MCP servers"); + }); + let test = builder.build_with_auto_env(&server).await?; + wait_for_mcp_server(&test.codex, "rmcp").await?; + + let first_mock = responses::mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-1"), + responses::ev_function_call_with_namespace( + "call-1", + "mcp__rmcp", + "echo", + r#"{"message":"ping"}"#, + ), + ev_completed("resp-1"), + ]), + ) + .await; + let second_mock = responses::mount_sse_once( + &server, + sse(vec![ + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]), + ) + .await; + + test.submit_turn("call the directly exposed MCP echo tool") + .await?; + + let first_request = first_mock.single_request().body_json(); + assert!( + namespace_child_tool(&first_request, "mcp__rmcp", "echo").is_some(), + "an MCP tool omitted only from Code Mode must remain directly callable" + ); + let exec_description = first_request["tools"] + .as_array() + .expect("request should contain tools") + .iter() + .find_map(|tool| { + (tool["name"].as_str() == Some("exec")) + .then(|| tool["description"].as_str()) + .flatten() + }) + .expect("Code Mode exec should remain available"); + assert!( + !exec_description.contains("mcp__rmcp__echo"), + "an MCP tool omitted from Code Mode must not appear in the exec declaration" + ); + + let output = second_mock.single_request().function_call_output("call-1"); + assert!( + output["output"] + .as_str() + .is_some_and(|output| output.contains("ECHOING: ping")), + "the directly exposed MCP tool must execute successfully: {output:?}" + ); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn code_mode_only_can_call_mcp_tools_hidden_from_direct_and_deferred_exposure() -> Result<()> +{ + skip_if_wine_exec!( + Ok(()), + "requires a Windows test_stdio_server in the Wine-exec environment" + ); + skip_if_no_network!(Ok(())); + + let server = responses::start_mock_server().await; + let rmcp_test_server_bin = remote_aware_stdio_server_bin()?; + let environment_id = remote_aware_environment_id(); + let mut builder = test_codex() + .with_model_info_override("gpt-5.4", |model| { + model.supports_search_tool = true; + }) + .with_config(move |config| { + let _ = config.features.enable(Feature::CodeModeOnly); + let mut servers = config.mcp_servers.get().clone(); + servers.insert( + "rmcp".to_string(), + serde_json::from_value(serde_json::json!({ + "command": rmcp_test_server_bin, + "environment_id": environment_id, + "omit_tools_from": ["direct", "deferred"], + "supports_parallel_tool_calls": true, + })) + .expect("test MCP server config should be valid"), + ); + config + .mcp_servers + .set(servers) + .expect("test config should allow MCP servers"); + }); + let test = builder.build_with_auto_env(&server).await?; + wait_for_mcp_server(&test.codex, "rmcp").await?; + + let first_mock = responses::mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-1"), + ev_custom_tool_call( + "call-1", + "exec", + r#" +const results = await Promise.all([ + tools.mcp__rmcp__echo({ message: "ping" }), + tools.mcp__rmcp__echo({ message: "pong" }), +]); +text(results.map(({ structuredContent }) => structuredContent?.echo ?? "missing").join(",")); +"#, + ), + ev_completed("resp-1"), + ]), + ) + .await; + let second_mock = responses::mount_sse_once( + &server, + sse(vec![ + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]), + ) + .await; + + test.submit_turn("call the MCP echo tool from Code Mode") + .await?; + + let first_request = first_mock.single_request().body_json(); + assert!( + namespace_child_tool(&first_request, "mcp__rmcp", "echo").is_none(), + "MCP tool omitted from direct and deferred exposure must remain hidden at the top level" + ); + let exec_description = first_request["tools"] + .as_array() + .expect("request should contain tools") + .iter() + .find_map(|tool| { + (tool["name"].as_str() == Some("exec")) + .then(|| tool["description"].as_str()) + .flatten() + }) + .expect("Code Mode exec should remain available"); + assert!( + exec_description.contains("mcp__rmcp__echo"), + "hidden MCP tool must remain available inside Code Mode" + ); + + let request = second_mock.single_request(); + let (output, success) = custom_tool_output_body_and_success(&request, "call-1"); + assert_ne!( + success, + Some(false), + "hidden Code Mode MCP tool call failed unexpectedly: {output}" + ); + assert_eq!(output, "ECHOING: ping,ECHOING: pong"); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn code_mode_can_print_content_only_mcp_tool_result_fields() -> Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/core/tests/suite/hooks_mcp.rs b/codex-rs/core/tests/suite/hooks_mcp.rs index 94839fd117..7423bc0678 100644 --- a/codex-rs/core/tests/suite/hooks_mcp.rs +++ b/codex-rs/core/tests/suite/hooks_mcp.rs @@ -196,6 +196,7 @@ fn insert_rmcp_test_server(config: &mut Config, command: String, approval_mode: enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: None, diff --git a/codex-rs/core/tests/suite/mcp_refresh_cleanup.rs b/codex-rs/core/tests/suite/mcp_refresh_cleanup.rs index 620b6c734c..3cbb6e9ca7 100644 --- a/codex-rs/core/tests/suite/mcp_refresh_cleanup.rs +++ b/codex-rs/core/tests/suite/mcp_refresh_cleanup.rs @@ -45,6 +45,7 @@ async fn refresh_keeps_superseded_mcp_server_alive_for_in_flight_calls() -> anyh enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: None, diff --git a/codex-rs/core/tests/suite/mcp_startup_refresh_http_proxy.rs b/codex-rs/core/tests/suite/mcp_startup_refresh_http_proxy.rs index e0a6dc4bdc..2d91238cf2 100644 --- a/codex-rs/core/tests/suite/mcp_startup_refresh_http_proxy.rs +++ b/codex-rs/core/tests/suite/mcp_startup_refresh_http_proxy.rs @@ -118,6 +118,7 @@ async fn local_mcp_startup_and_refresh_use_configured_http_client() -> Result<() enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: None, diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index 297ad4da40..951f80cec4 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -333,6 +333,7 @@ fn insert_mcp_server( enabled: true, required: false, supports_parallel_tool_calls: options.supports_parallel_tool_calls, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: options.tool_timeout_sec, diff --git a/codex-rs/core/tests/suite/search_tool.rs b/codex-rs/core/tests/suite/search_tool.rs index 6a2e83f4db..ee0a5ba4c8 100644 --- a/codex-rs/core/tests/suite/search_tool.rs +++ b/codex-rs/core/tests/suite/search_tool.rs @@ -1193,6 +1193,7 @@ async fn tool_search_indexes_only_enabled_non_app_mcp_tools() -> Result<()> { oauth: None, oauth_resource: None, supports_parallel_tool_calls: false, + omit_tools_from: None, tools: HashMap::new(), }, ); @@ -1320,6 +1321,7 @@ async fn tool_search_surfaced_mcp_tool_errors_are_returned_to_model() -> Result< oauth: None, oauth_resource: None, supports_parallel_tool_calls: false, + omit_tools_from: None, tools: HashMap::new(), }, ); @@ -1469,6 +1471,7 @@ async fn tool_search_uses_non_app_mcp_server_instructions_as_namespace_descripti oauth: None, oauth_resource: None, supports_parallel_tool_calls: false, + omit_tools_from: None, tools: HashMap::new(), }, ); diff --git a/codex-rs/core/tests/suite/sqlite_state.rs b/codex-rs/core/tests/suite/sqlite_state.rs index 7abde52e35..fdc0dd8497 100644 --- a/codex-rs/core/tests/suite/sqlite_state.rs +++ b/codex-rs/core/tests/suite/sqlite_state.rs @@ -683,6 +683,7 @@ async fn mcp_call_marks_thread_memory_mode_polluted_when_configured() -> Result< enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: None, diff --git a/codex-rs/core/tests/suite/token_budget.rs b/codex-rs/core/tests/suite/token_budget.rs index 0af305f38a..088aedb562 100644 --- a/codex-rs/core/tests/suite/token_budget.rs +++ b/codex-rs/core/tests/suite/token_budget.rs @@ -693,6 +693,7 @@ async fn token_budget_context_injects_plain_thread_hint_text() -> Result<()> { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: None, diff --git a/codex-rs/core/tests/suite/truncation.rs b/codex-rs/core/tests/suite/truncation.rs index c4b574abf5..ad15a0d0d8 100644 --- a/codex-rs/core/tests/suite/truncation.rs +++ b/codex-rs/core/tests/suite/truncation.rs @@ -395,6 +395,7 @@ async fn mcp_tool_call_output_exceeds_limit_truncated_for_model() -> Result<()> enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: Some(std::time::Duration::from_secs(10)), tool_timeout_sec: None, @@ -497,6 +498,7 @@ async fn mcp_image_output_preserves_image_and_no_text_summary() -> Result<()> { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: None, @@ -787,6 +789,7 @@ async fn mcp_tool_call_output_not_truncated_with_custom_limit() -> Result<()> { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: Some(std::time::Duration::from_secs(10)), tool_timeout_sec: None, diff --git a/codex-rs/ext/mcp/src/executor_plugin/provider_tests.rs b/codex-rs/ext/mcp/src/executor_plugin/provider_tests.rs index bc59e104a0..d1137a351a 100644 --- a/codex-rs/ext/mcp/src/executor_plugin/provider_tests.rs +++ b/codex-rs/ext/mcp/src/executor_plugin/provider_tests.rs @@ -179,6 +179,7 @@ async fn reads_declared_config_only_through_executor_file_system() { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -205,6 +206,7 @@ async fn reads_declared_config_only_through_executor_file_system() { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, @@ -262,6 +264,7 @@ async fn reads_manifest_object_config_without_executor_file_system_access() { enabled: true, required: false, supports_parallel_tool_calls: false, + omit_tools_from: None, disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, diff --git a/codex-rs/protocol/src/config_types.rs b/codex-rs/protocol/src/config_types.rs index c29ca9488a..4156677256 100644 --- a/codex-rs/protocol/src/config_types.rs +++ b/codex-rs/protocol/src/config_types.rs @@ -362,6 +362,19 @@ pub enum WebSearchMode { Live, } +/// A model-facing surface on which a tool can be exposed. +#[derive(Debug, Serialize, Deserialize, Clone, Copy, PartialEq, Eq, Display, JsonSchema, TS)] +#[serde(rename_all = "snake_case")] +#[strum(serialize_all = "snake_case")] +pub enum ToolExposureSurface { + /// Nested tools available to Code Mode scripts. + CodeMode, + /// Tools discovered later through tool search. + Deferred, + /// Tools present in the model's initial tool list. + Direct, +} + #[derive(Debug, Serialize, Deserialize, Clone, Copy, PartialEq, Eq, Display, JsonSchema, TS)] #[serde(rename_all = "lowercase")] #[strum(serialize_all = "lowercase")] diff --git a/codex-rs/tools/Cargo.toml b/codex-rs/tools/Cargo.toml index 046f9e7a08..55b28f4939 100644 --- a/codex-rs/tools/Cargo.toml +++ b/codex-rs/tools/Cargo.toml @@ -8,6 +8,7 @@ version.workspace = true workspace = true [dependencies] +bitflags = { workspace = true } codex-code-mode = { workspace = true } codex-connectors = { workspace = true } codex-features = { workspace = true } diff --git a/codex-rs/tools/src/lib.rs b/codex-rs/tools/src/lib.rs index 00913518db..be8f9de953 100644 --- a/codex-rs/tools/src/lib.rs +++ b/codex-rs/tools/src/lib.rs @@ -96,6 +96,7 @@ pub use tool_discovery::filter_request_plugin_install_discoverable_tools_for_cli pub use tool_executor::ToolExecutor; pub use tool_executor::ToolExecutorFuture; pub use tool_executor::ToolExposure; +pub use tool_executor::ToolExposures; pub use tool_output::JsonToolOutput; pub use tool_output::ToolOutput; pub use tool_payload::ToolPayload; diff --git a/codex-rs/tools/src/tool_executor.rs b/codex-rs/tools/src/tool_executor.rs index c918abd603..edba56d850 100644 --- a/codex-rs/tools/src/tool_executor.rs +++ b/codex-rs/tools/src/tool_executor.rs @@ -3,6 +3,7 @@ use crate::ToolName; use crate::ToolOutput; use crate::ToolSearchInfo; use crate::ToolSpec; +use codex_protocol::config_types::ToolExposureSurface; use std::future::Future; use std::pin::Pin; @@ -10,6 +11,41 @@ use std::pin::Pin; pub type ToolExecutorFuture<'a> = Pin, FunctionCallError>> + Send + 'a>>; +bitflags::bitflags! { + /// Independent model-facing surfaces supported by a tool. + #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] + pub struct ToolExposures: u8 { + /// Keep the tool registered without making it model-visible. + const NONE = 0; + /// Include the tool in the initial model-visible tool list. + const DIRECT = 0b001; + /// Make the tool discoverable through tool search. + const DEFERRED = 0b010; + /// Make the tool callable from nested Code Mode scripts. + const CODE_MODE = 0b100; + /// Permit every supported model-facing surface. + const ALL = Self::DIRECT.bits() | Self::DEFERRED.bits() | Self::CODE_MODE.bits(); + } +} + +impl From for ToolExposures { + fn from(surface: ToolExposureSurface) -> Self { + match surface { + ToolExposureSurface::CodeMode => Self::CODE_MODE, + ToolExposureSurface::Deferred => Self::DEFERRED, + ToolExposureSurface::Direct => Self::DIRECT, + } + } +} + +impl FromIterator for ToolExposures { + fn from_iter>(surfaces: T) -> Self { + surfaces + .into_iter() + .fold(Self::NONE, |exposures, surface| exposures | surface.into()) + } +} + /// Controls where a tool is exposed to the model. #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum ToolExposure { @@ -25,12 +61,20 @@ pub enum ToolExposure { /// metadata from function and namespace specs. Deferred, + /// Make this tool discoverable through tool search without allowing nested + /// Code Mode calls. + DeferredModelOnly, + /// Include this tool in the initial model-visible tool list only. /// /// In code-mode-only sessions, this keeps the tool callable as a normal /// model tool while excluding it from the nested code-mode tool surface. DirectModelOnly, + /// Expose this tool only to nested Code Mode calls, without including it in + /// the initial model-visible tool list or making it available to tool search. + CodeModeOnly, + /// Keep this tool registered for dispatch without exposing it to the model. Hidden, } @@ -40,11 +84,16 @@ impl ToolExposure { matches!(self, Self::Direct | Self::DirectModelOnly) } + /// Returns whether this tool can be discovered through tool search. + pub fn is_deferred(self) -> bool { + matches!(self, Self::Deferred | Self::DeferredModelOnly) + } + /// Returns whether this tool can participate in code mode. pub fn is_available_in_code_mode(self) -> bool { match self { - Self::Direct | Self::Deferred => true, - Self::DirectModelOnly | Self::Hidden => false, + Self::Direct | Self::Deferred | Self::CodeModeOnly => true, + Self::DirectModelOnly | Self::DeferredModelOnly | Self::Hidden => false, } } } diff --git a/codex-rs/tools/src/tool_output.rs b/codex-rs/tools/src/tool_output.rs index 7d58b35e89..a2e36db9e1 100644 --- a/codex-rs/tools/src/tool_output.rs +++ b/codex-rs/tools/src/tool_output.rs @@ -180,9 +180,14 @@ impl ToolOutput for codex_protocol::mcp::CallToolResult { } fn code_mode_result(&self, _payload: &ToolPayload) -> JsonValue { - serde_json::to_value(self).unwrap_or_else(|err| { + let mut result = serde_json::to_value(self).unwrap_or_else(|err| { JsonValue::String(format!("failed to serialize mcp result: {err}")) - }) + }); + // MCP result metadata is private to clients and must not reach Code Mode. + if let JsonValue::Object(fields) = &mut result { + fields.remove("_meta"); + } + result } }