diff --git a/codex-rs/app-server/src/request_processors/apps_processor.rs b/codex-rs/app-server/src/request_processors/apps_processor.rs index ea942e7eca..1da9db4bb4 100644 --- a/codex-rs/app-server/src/request_processors/apps_processor.rs +++ b/codex-rs/app-server/src/request_processors/apps_processor.rs @@ -1,5 +1,6 @@ use super::*; use crate::app_info::app_info_to_api; +use codex_connectors::AppToolPolicyEvaluator; mod installed; mod read; @@ -250,12 +251,13 @@ impl AppsRequestProcessor { let mut codex_apps_ready = true; let mut last_notified_apps = None; let mut sent_app_list_update = false; + let app_policy = AppToolPolicyEvaluator::new(&config.config_layer_stack); if accessible_connectors.is_some() || all_connectors.is_some() { - let merged = connectors::with_app_enabled_state( - merge_loaded_apps(all_connectors.as_deref(), accessible_connectors.as_deref()), - &config, - ); + let merged = app_policy.apply_app_enabled_state(merge_loaded_apps( + all_connectors.as_deref(), + accessible_connectors.as_deref(), + )); if !force_refetch { last_notified_apps = Some(merged); } else if should_send_app_list_updated_notification( @@ -314,10 +316,10 @@ impl AppsRequestProcessor { } else { accessible_connectors.as_deref() }; - let merged = connectors::with_app_enabled_state( - merge_loaded_apps(all_connectors_for_update, accessible_connectors_for_update), - &config, - ); + let merged = app_policy.apply_app_enabled_state(merge_loaded_apps( + all_connectors_for_update, + accessible_connectors_for_update, + )); if should_send_app_list_updated_notification( merged.as_slice(), accessible_loaded, diff --git a/codex-rs/chatgpt/src/connectors.rs b/codex-rs/chatgpt/src/connectors.rs index 538a99543a..561889e780 100644 --- a/codex-rs/chatgpt/src/connectors.rs +++ b/codex-rs/chatgpt/src/connectors.rs @@ -6,6 +6,7 @@ use crate::chatgpt_client::chatgpt_get_request_with_timeout; use crate::chatgpt_client::chatgpt_post_request_with_timeout; use codex_connectors::AppInfo; +use codex_connectors::AppToolPolicyEvaluator; use codex_connectors::ConnectorDirectoryCacheContext; use codex_connectors::ConnectorDirectoryCacheKey; use codex_connectors::ConnectorMetadata; @@ -21,7 +22,6 @@ pub use codex_core::connectors::list_accessible_connectors_from_mcp_tools_with_m pub use codex_core::connectors::list_accessible_connectors_from_mcp_tools_with_options; pub use codex_core::connectors::list_accessible_connectors_from_mcp_tools_with_options_and_status; pub use codex_core::connectors::list_cached_accessible_connectors_from_mcp_tools; -pub use codex_core::connectors::with_app_enabled_state; use codex_login::AuthManager; use codex_login::CodexAuth; use codex_plugin::AppConnectorId; @@ -65,12 +65,13 @@ pub async fn list_connectors(config: &Config) -> anyhow::Result> { ); let connectors = connectors_result?; let accessible = accessible_result?; - Ok(with_app_enabled_state( - merge_connectors_with_accessible( - connectors, accessible, /*all_connectors_loaded*/ true, + Ok( + AppToolPolicyEvaluator::new(&config.config_layer_stack).apply_app_enabled_state( + merge_connectors_with_accessible( + connectors, accessible, /*all_connectors_loaded*/ true, + ), ), - config, - )) + ) } pub async fn list_all_connectors(config: &Config) -> anyhow::Result> { diff --git a/codex-rs/connectors/src/app_tool_policy.rs b/codex-rs/connectors/src/app_tool_policy.rs index 533af0719c..c7a58c9514 100644 --- a/codex-rs/connectors/src/app_tool_policy.rs +++ b/codex-rs/connectors/src/app_tool_policy.rs @@ -4,6 +4,8 @@ use codex_config::types::AppToolApproval; use codex_config::types::AppsConfigToml; use serde::Deserialize; +use crate::AppInfo; + /// The effective enablement and approval policy for one app tool. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct AppToolPolicy { @@ -63,6 +65,21 @@ impl<'a> AppToolPolicyEvaluator<'a> { .unwrap_or(true) } + /// Applies app policy without overriding source state for unconfigured apps. + pub fn apply_app_enabled_state(&self, mut apps: Vec) -> Vec { + let Some(apps_config) = self.apps_config.as_ref() else { + return apps; + }; + + for app in &mut apps { + if apps_config.default.is_some() || apps_config.apps.contains_key(app.id.as_str()) { + app.is_enabled = self.app_enabled(app.id.as_str()); + } + } + + apps + } + fn from_parts( apps_config: Option, requirements_apps_config: Option<&'a AppsRequirementsToml>, diff --git a/codex-rs/connectors/src/app_tool_policy_tests.rs b/codex-rs/connectors/src/app_tool_policy_tests.rs index 964e983799..d795774122 100644 --- a/codex-rs/connectors/src/app_tool_policy_tests.rs +++ b/codex-rs/connectors/src/app_tool_policy_tests.rs @@ -189,6 +189,61 @@ fn app_enablement_uses_defaults_and_per_app_overrides() { ], [true, false, false] ); + + let evaluator = AppToolPolicyEvaluator::from_parts( + Some(apps_config), + /*requirements_apps_config*/ None, + ); + assert_eq!( + evaluator.apply_app_enabled_state(vec![ + app("calendar", /*enabled*/ false), + app("drive", /*enabled*/ true), + ]), + vec![ + app("calendar", /*enabled*/ true), + app("drive", /*enabled*/ false), + ] + ); +} + +#[test] +fn app_enablement_preserves_source_state_and_honors_local_and_managed_overrides() { + let apps_config = AppsConfigToml { + default: None, + apps: HashMap::from([ + ( + "calendar".to_string(), + AppConfig { + enabled: true, + ..Default::default() + }, + ), + ( + "drive".to_string(), + AppConfig { + enabled: true, + ..Default::default() + }, + ), + ]), + }; + let requirements = app_enabled_requirement("drive", /*enabled*/ false); + let evaluator = AppToolPolicyEvaluator::from_parts(Some(apps_config), Some(&requirements)); + + assert_eq!( + evaluator.apply_app_enabled_state(vec![ + app("calendar", /*enabled*/ false), + app("drive", /*enabled*/ true), + app("slack", /*enabled*/ false), + app("gmail", /*enabled*/ true), + ]), + vec![ + app("calendar", /*enabled*/ true), + app("drive", /*enabled*/ false), + app("slack", /*enabled*/ false), + app("gmail", /*enabled*/ true), + ] + ); } #[test] @@ -657,6 +712,26 @@ fn input<'a>(tool_name: &'a str, tool_title: Option<&'a str>) -> AppToolPolicyIn } } +fn app(id: &str, enabled: bool) -> AppInfo { + AppInfo { + id: id.to_string(), + name: id.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: enabled, + plugin_display_names: Vec::new(), + } +} + fn policy_from_apps_config( apps_config: Option<&AppsConfigToml>, connector_id: Option<&str>, diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index e543f2ae38..0fadc2c924 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -10,7 +10,6 @@ pub use codex_connectors::AppInfo; pub use codex_connectors::AppMetadata; use codex_connectors::ConnectorDirectoryCacheContext; use codex_connectors::ConnectorDirectoryCacheKey; -use codex_connectors::app_is_enabled; use codex_connectors::apps_config_from_layer_stack; use codex_connectors::connector_runtime_context_key; use codex_exec_server::EnvironmentManager; @@ -490,32 +489,6 @@ fn accessible_connectors_for_app_list_from_mcp_tools(mcp_tools: &[ToolInfo]) -> collect_accessible_connectors_from_mcp_tools(non_synthetic_tools) } -pub fn with_app_enabled_state(mut connectors: Vec, config: &Config) -> Vec { - let user_apps_config = apps_config_from_layer_stack(&config.config_layer_stack); - let requirements_apps_config = config.config_layer_stack.requirements_toml().apps.as_ref(); - if user_apps_config.is_none() && requirements_apps_config.is_none() { - return connectors; - } - - for connector in &mut connectors { - if let Some(apps_config) = user_apps_config.as_ref() - && (apps_config.default.is_some() - || apps_config.apps.contains_key(connector.id.as_str())) - { - connector.is_enabled = app_is_enabled(apps_config, Some(connector.id.as_str())); - } - - if requirements_apps_config - .and_then(|apps| apps.apps.get(connector.id.as_str())) - .is_some_and(|app| app.enabled == Some(false)) - { - connector.is_enabled = false; - } - } - - connectors -} - pub fn with_app_plugin_sources( mut connectors: Vec, tool_plugin_provenance: &ToolPluginProvenance, diff --git a/codex-rs/core/src/connectors_tests.rs b/codex-rs/core/src/connectors_tests.rs index 37aedcce63..f481baf99b 100644 --- a/codex-rs/core/src/connectors_tests.rs +++ b/codex-rs/core/src/connectors_tests.rs @@ -1,11 +1,6 @@ use super::*; use crate::config::CONFIG_TOML_FILE; use crate::config::ConfigBuilder; -use codex_config::AppRequirementToml; -use codex_config::AppsRequirementsToml; -use codex_config::ConfigLayerStack; -use codex_config::ConfigRequirements; -use codex_config::ConfigRequirementsToml; use codex_config::test_support::CloudConfigBundleFixture; use codex_config::types::ApprovalsReviewer; use codex_connectors::merge::plugin_connector_to_app_info; @@ -19,31 +14,10 @@ use pretty_assertions::assert_eq; use rmcp::model::JsonObject; use rmcp::model::MetaObject; use rmcp::model::Tool; -use std::collections::BTreeMap; use std::collections::HashSet; use std::sync::Arc; use tempfile::tempdir; -fn app(id: &str) -> AppInfo { - AppInfo { - id: id.to_string(), - name: id.to_string(), - description: None, - logo_url: None, - logo_url_dark: None, - icon_assets: None, - icon_dark_assets: None, - distribution_channel: None, - install_url: None, - branding: None, - app_metadata: None, - labels: None, - is_accessible: false, - is_enabled: true, - plugin_display_names: Vec::new(), - } -} - fn plugin_names(names: &[&str]) -> Vec { names.iter().map(ToString::to_string).collect() } @@ -452,44 +426,6 @@ approvals_reviewer = "user" ); } -#[tokio::test] -async fn with_app_enabled_state_preserves_unrelated_disabled_connector() { - let codex_home = tempdir().expect("tempdir should succeed"); - let mut config = ConfigBuilder::default() - .codex_home(codex_home.path().to_path_buf()) - .fallback_cwd(Some(codex_home.path().to_path_buf())) - .build() - .await - .expect("config should build"); - - let requirements = ConfigRequirementsToml { - apps: Some(AppsRequirementsToml { - apps: BTreeMap::from([( - "connector_drive".to_string(), - AppRequirementToml { - enabled: Some(false), - tools: None, - }, - )]), - }), - ..Default::default() - }; - config.config_layer_stack = - ConfigLayerStack::new(Vec::new(), ConfigRequirements::default(), requirements) - .expect("requirements stack"); - - let mut slack = app("connector_slack"); - slack.is_enabled = false; - - let mut drive = app("connector_drive"); - drive.is_enabled = false; - - assert_eq!( - with_app_enabled_state(vec![slack.clone(), app("connector_drive")], &config), - vec![slack, drive] - ); -} - #[tokio::test] async fn tool_suggest_connector_ids_include_configured_tool_suggest_discoverables() { let codex_home = tempdir().expect("tempdir should succeed"); diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index b2e719deac..a873582796 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -71,6 +71,7 @@ use codex_analytics::InvocationType; use codex_analytics::TurnResolvedConfigFact; use codex_analytics::build_track_events_context; use codex_async_utils::OrCancelExt; +use codex_connectors::AppToolPolicyEvaluator; use codex_core_plugins::RecommendedPluginCandidatesInput; use codex_core_skills::injection::InjectedHostSkillPrompts; use codex_extension_api::ExtensionData; @@ -754,7 +755,8 @@ async fn build_skills_and_plugins( .map(|connector_id| connector_id.0.clone()), connectors::accessible_connectors_from_mcp_tools(mcp_tools), ); - connectors::with_app_enabled_state(connectors, &turn_context.config) + AppToolPolicyEvaluator::new(&turn_context.config.config_layer_stack) + .apply_app_enabled_state(connectors) } else { Vec::new() }; @@ -1476,21 +1478,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 connectors = if apps_enabled { - let connectors = codex_connectors::merge::merge_plugin_connectors_with_accessible( - connector_snapshot - .connector_ids() - .iter() - .map(|connector_id| connector_id.0.clone()), - accessible_connectors.clone().unwrap_or_default(), - ); - Some(connectors::with_app_enabled_state( - connectors, - &turn_context.config, - )) - } else { - None - }; let tool_suggest_is_enabled = tool_suggest_enabled(turn_context); let PreparedToolRecommendations { auth, @@ -1510,7 +1497,7 @@ pub(crate) async fn built_tools( .collect::>(); async { if apps_enabled && tool_suggest_is_enabled { - if let Some(accessible_connectors) = connectors.as_ref() { + if let Some(accessible_connectors) = accessible_connectors.as_ref() { match connectors::list_tool_suggest_discoverable_tools_with_auth( &turn_context.config, sess.services.plugins_manager.as_ref(), diff --git a/codex-rs/core/src/session/world_state.rs b/codex-rs/core/src/session/world_state.rs index a74ebd6500..f9b660c55e 100644 --- a/codex-rs/core/src/session/world_state.rs +++ b/codex-rs/core/src/session/world_state.rs @@ -19,6 +19,7 @@ use crate::context::world_state::PluginsInstructionsState; use crate::context::world_state::RealtimeState; use crate::context::world_state::ToolsState; use crate::context::world_state::WorldState; +use codex_connectors::AppToolPolicyEvaluator; use codex_extension_api::WorldStateContributionInput; use codex_features::Feature; use codex_protocol::error::CodexErr; @@ -178,12 +179,12 @@ impl Session { )); let apps_available = if turn_context.config.include_apps_instructions && turn_context.apps_enabled() { - connectors::with_app_enabled_state( - connectors::accessible_connectors_from_mcp_tools(step_context.mcp.tools()), - &turn_context.config, - ) - .into_iter() - .any(|connector| connector.is_accessible && connector.is_enabled) + AppToolPolicyEvaluator::new(&turn_context.config.config_layer_stack) + .apply_app_enabled_state(connectors::accessible_connectors_from_mcp_tools( + step_context.mcp.tools(), + )) + .into_iter() + .any(|connector| connector.is_accessible && connector.is_enabled) } else { false }; diff --git a/codex-rs/core/src/tools/handlers/request_plugin_install.rs b/codex-rs/core/src/tools/handlers/request_plugin_install.rs index 6bc7507a40..50739c669d 100644 --- a/codex-rs/core/src/tools/handlers/request_plugin_install.rs +++ b/codex-rs/core/src/tools/handlers/request_plugin_install.rs @@ -456,20 +456,15 @@ async fn refresh_missing_requested_connectors( } let mcp_tools = mcp.tools(); - let accessible_connectors = connectors::with_app_enabled_state( - connectors::accessible_connectors_from_mcp_tools(mcp_tools), - &turn.config, - ); + let accessible_connectors = connectors::accessible_connectors_from_mcp_tools(mcp_tools); if all_requested_connectors_picked_up(expected_connector_ids, &accessible_connectors) { return Some(accessible_connectors); } match session.hard_refresh_latest_codex_apps_tools().await { Ok(mcp_tools) => { - let accessible_connectors = connectors::with_app_enabled_state( - connectors::accessible_connectors_from_mcp_tools(&mcp_tools), - &turn.config, - ); + let accessible_connectors = + connectors::accessible_connectors_from_mcp_tools(&mcp_tools); connectors::refresh_accessible_connectors_cache_from_mcp_tools( &turn.config, auth,