diff --git a/codex-rs/core/src/mcp_tool_exposure.rs b/codex-rs/core/src/mcp_tool_exposure.rs index 60ce08c502..0991e4cf41 100644 --- a/codex-rs/core/src/mcp_tool_exposure.rs +++ b/codex-rs/core/src/mcp_tool_exposure.rs @@ -12,23 +12,23 @@ use tracing::instrument; use tracing::warn; use crate::config::Config; -use crate::connectors; use crate::tools::handlers::McpHandler; use crate::tools::registry::ToolRegistry; #[instrument(level = "trace", skip_all)] pub(crate) fn append_mcp_tools( all_mcp_tools: &[McpToolInfo], - connectors: Option<&[connectors::AppInfo]>, config: &Config, + apps_enabled: bool, 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 + let app_tools = apps_enabled + .then(|| filter_codex_apps_mcp_tools(all_mcp_tools, config)) .into_iter() - .flat_map(move |connectors| filter_codex_apps_mcp_tools(all_mcp_tools, connectors, config)); + .flatten(); let exposure = if search_tool_enabled { ToolExposure::Deferred } else { @@ -61,13 +61,8 @@ fn filter_non_codex_apps_mcp_tools_only( fn filter_codex_apps_mcp_tools<'a>( mcp_tools: &'a [McpToolInfo], - connectors: &'a [connectors::AppInfo], config: &'a Config, ) -> impl Iterator + 'a { - let allowed: HashSet<&str> = connectors - .iter() - .map(|connector| connector.id.as_str()) - .collect(); let app_tool_policy = AppToolPolicyEvaluator::new(&config.config_layer_stack); mcp_tools.iter().filter(move |tool| { @@ -81,18 +76,15 @@ fn filter_codex_apps_mcp_tools<'a>( return false; }; let annotations = tool.tool.annotations.as_ref(); - allowed.contains(connector_id) - && app_tool_policy - .policy(AppToolPolicyInput { - connector_id: Some(connector_id), - tool_name: &tool.tool.name, - tool_title: tool.tool.title.as_deref(), - destructive_hint: annotations - .and_then(|annotations| annotations.destructive_hint), - open_world_hint: annotations - .and_then(|annotations| annotations.open_world_hint), - }) - .enabled + app_tool_policy + .policy(AppToolPolicyInput { + connector_id: Some(connector_id), + tool_name: &tool.tool.name, + tool_title: tool.tool.title.as_deref(), + destructive_hint: annotations.and_then(|annotations| annotations.destructive_hint), + open_world_hint: annotations.and_then(|annotations| annotations.open_world_hint), + }) + .enabled }) } diff --git a/codex-rs/core/src/mcp_tool_exposure_test.rs b/codex-rs/core/src/mcp_tool_exposure_test.rs index 844bf16632..782ea0f9fb 100644 --- a/codex-rs/core/src/mcp_tool_exposure_test.rs +++ b/codex-rs/core/src/mcp_tool_exposure_test.rs @@ -15,29 +15,8 @@ use crate::config::CONFIG_TOML_FILE; use crate::config::Config; use crate::config::ConfigBuilder; use crate::config::test_config; -use crate::connectors::AppInfo; use tempfile::tempdir; -fn make_connector(id: &str, name: &str) -> AppInfo { - AppInfo { - id: id.to_string(), - name: name.to_string(), - description: None, - logo_url: None, - logo_url_dark: None, - icon_assets: None, - icon_dark_assets: None, - distribution_channel: None, - branding: None, - app_metadata: None, - labels: None, - install_url: None, - is_accessible: true, - is_enabled: true, - plugin_display_names: Vec::new(), - } -} - fn make_mcp_tool( server_name: &str, tool_name: &str, @@ -93,15 +72,15 @@ fn expected_runtimes( fn runtimes_by_name( tools: &[ToolInfo], - connectors: Option<&[AppInfo]>, config: &Config, + apps_enabled: bool, search_tool_enabled: bool, ) -> HashMap { let mut registry = ToolRegistry::default(); append_mcp_tools( tools, - connectors, config, + apps_enabled, search_tool_enabled, &mut registry, ); @@ -127,7 +106,7 @@ async fn directly_exposes_effective_tool_sets_when_search_is_unavailable() { let mcp_tools = numbered_mcp_tools(/*count*/ 2); let runtimes = runtimes_by_name( - &mcp_tools, /*connectors*/ None, &config, /*search_tool_enabled*/ false, + &mcp_tools, &config, /*apps_enabled*/ false, /*search_tool_enabled*/ false, ); assert_eq!( @@ -198,13 +177,8 @@ async fn excludes_tools_hidden_from_model_exposure() { visible_app_tool.clone(), hidden_app_tool, ]; - let connectors = vec![make_connector("calendar", "Calendar")]; - let runtimes = runtimes_by_name( - &mcp_tools, - Some(connectors.as_slice()), - &config, - /*search_tool_enabled*/ false, + &mcp_tools, &config, /*apps_enabled*/ true, /*search_tool_enabled*/ false, ); assert_eq!( @@ -213,6 +187,83 @@ async fn excludes_tools_hidden_from_model_exposure() { ); } +#[tokio::test] +async fn app_tool_registration_uses_trusted_catalog_metadata_and_preserves_source_order() { + let config = test_config().await; + let app_tool = make_mcp_tool( + CODEX_APPS_MCP_SERVER_NAME, + "calendar_list_events", + "mcp__codex_apps__calendar", + "list_events", + Some("calendar"), + Some("Calendar"), + ); + let missing_connector_id = make_mcp_tool( + CODEX_APPS_MCP_SERVER_NAME, + "unknown_tool", + "mcp__codex_apps__unknown", + "unknown", + /*connector_id*/ None, + /*connector_name*/ None, + ); + let mut synthetic_app_tool = make_mcp_tool( + CODEX_APPS_MCP_SERVER_NAME, + "gmail_batch_read_email", + "mcp__codex_apps__gmail", + "batch_read_email", + Some("gmail"), + Some("Gmail"), + ); + synthetic_app_tool.tool.meta = Some(MetaObject( + serde_json::json!({ "_codex_apps": { "synthetic_link": true } }) + .as_object() + .expect("metadata should be an object") + .clone(), + )); + let regular_tool = make_mcp_tool( + "rmcp", + "regular_tool", + "mcp__rmcp", + "regular_tool", + /*connector_id*/ None, + /*connector_name*/ None, + ); + let mcp_tools = [ + app_tool.clone(), + missing_connector_id, + synthetic_app_tool.clone(), + regular_tool.clone(), + ]; + let mut registry = ToolRegistry::default(); + + append_mcp_tools( + &mcp_tools, + &config, + /*apps_enabled*/ true, + /*search_tool_enabled*/ false, + &mut registry, + ); + + let registered_names = registry + .entries() + .map(|entry| entry.runtime.tool_name()) + .collect::>(); + assert_eq!( + registered_names, + vec![ + regular_tool.canonical_tool_name(), + app_tool.canonical_tool_name(), + synthetic_app_tool.canonical_tool_name(), + ] + ); + assert_eq!( + runtimes_by_name( + &mcp_tools, &config, /*apps_enabled*/ false, /*search_tool_enabled*/ false, + ), + expected_runtimes(&[regular_tool], ToolExposure::Direct) + ); +} + #[tokio::test] async fn applies_per_tool_app_policy_across_the_exposure_build() { let codex_home = tempdir().expect("tempdir should succeed"); @@ -248,14 +299,9 @@ enabled = true Some("calendar"), Some("Calendar"), ); - let connectors = vec![make_connector("calendar", "Calendar")]; - let mcp_tools = [enabled_tool.clone(), disabled_tool]; let runtimes = runtimes_by_name( - &mcp_tools, - Some(connectors.as_slice()), - &config, - /*search_tool_enabled*/ false, + &mcp_tools, &config, /*apps_enabled*/ true, /*search_tool_enabled*/ false, ); assert_eq!( @@ -270,7 +316,7 @@ async fn defers_effective_tool_sets_when_search_is_available() { let mcp_tools = numbered_mcp_tools(/*count*/ 2); let runtimes = runtimes_by_name( - &mcp_tools, /*connectors*/ None, &config, /*search_tool_enabled*/ true, + &mcp_tools, &config, /*apps_enabled*/ false, /*search_tool_enabled*/ true, ); assert_eq!( @@ -300,13 +346,8 @@ async fn defers_apps_and_non_app_mcp_tools() { Some("Calendar"), ), ]; - let connectors = vec![make_connector("calendar", "Calendar")]; - let runtimes = runtimes_by_name( - &mcp_tools, - Some(connectors.as_slice()), - &config, - /*search_tool_enabled*/ true, + &mcp_tools, &config, /*apps_enabled*/ true, /*search_tool_enabled*/ true, ); assert_eq!( diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index d334bfc861..ff56fa0108 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -1476,10 +1476,6 @@ pub(crate) async fn built_tools( let apps_enabled = turn_context.apps_enabled(); let accessible_connectors = apps_enabled.then(|| connectors::accessible_connectors_from_mcp_tools(all_mcp_tools)); - let accessible_connectors_with_enabled_state = - accessible_connectors.as_ref().map(|connectors| { - connectors::with_app_enabled_state(connectors.clone(), &turn_context.config) - }); let connectors = if apps_enabled { let connectors = codex_connectors::merge::merge_plugin_connectors_with_accessible( connector_snapshot @@ -1514,9 +1510,7 @@ pub(crate) async fn built_tools( .collect::>(); async { if apps_enabled && tool_suggest_is_enabled { - if let Some(accessible_connectors) = - accessible_connectors_with_enabled_state.as_ref() - { + if let Some(accessible_connectors) = connectors.as_ref() { match connectors::list_tool_suggest_discoverable_tools_with_auth( &turn_context.config, sess.services.plugins_manager.as_ref(), @@ -1556,7 +1550,7 @@ pub(crate) async fn built_tools( turn_context, environments, mcp, - connectors.as_deref(), + apps_enabled, step_store, tool_suggest_candidates.as_ref(), )) diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index a76c58fa41..b6209c52bb 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -1,6 +1,5 @@ use crate::agent::exceeds_thread_spawn_depth_limit; use crate::agent::next_thread_spawn_depth; -use crate::connectors::AppInfo; use crate::environment_selection::TurnEnvironmentSnapshot; use crate::mcp_tool_exposure::append_mcp_tools; use crate::session::session::Session; @@ -119,7 +118,7 @@ pub(crate) fn build_tool_router( turn_context: &TurnContext, environments: &TurnEnvironmentSnapshot, mcp: &codex_mcp::McpBinding, - connectors: Option<&[AppInfo]>, + apps_enabled: bool, step_store: &ExtensionData, tool_suggest_candidates: Option<&crate::tools::router::ToolSuggestCandidates>, ) -> ToolRouter { @@ -147,8 +146,8 @@ pub(crate) fn build_tool_router( } else { let registered_mcp_tools = append_mcp_tools( mcp.tools(), - connectors, &turn_context.config, + apps_enabled, search_tool_enabled(turn_context), &mut registry, );