From 9f5f214d753d68506fd8e0289705ed3c24dcee57 Mon Sep 17 00:00:00 2001 From: Rennie Song Date: Fri, 3 Jul 2026 23:22:40 +0000 Subject: [PATCH] fix Apps cache identity isolation --- .../request_processors/account_processor.rs | 4 + .../src/request_processors/apps_processor.rs | 63 +++++-- .../app-server/tests/suite/v2/app_list.rs | 59 +----- codex-rs/chatgpt/src/connectors.rs | 2 + codex-rs/codex-mcp/src/codex_apps_cache.rs | 19 ++ codex-rs/codex-mcp/src/connection_manager.rs | 36 ++-- .../codex-mcp/src/connection_manager_tests.rs | 177 +++++++++++++++++- codex-rs/codex-mcp/src/lib.rs | 1 + codex-rs/codex-mcp/src/mcp/mod.rs | 83 +++++++- codex-rs/codex-mcp/src/mcp/mod_tests.rs | 114 ++++++++++- codex-rs/codex-mcp/src/rmcp_client.rs | 12 ++ codex-rs/codex-mcp/src/server.rs | 10 + codex-rs/core/src/connectors.rs | 45 ++++- codex-rs/core/src/connectors_tests.rs | 4 +- codex-rs/core/src/mcp_tool_call.rs | 2 +- codex-rs/core/src/session/mcp.rs | 2 +- codex-rs/core/src/session/mod.rs | 3 +- codex-rs/core/src/session/review.rs | 1 - codex-rs/core/src/session/session.rs | 3 +- codex-rs/core/src/session/turn.rs | 11 +- codex-rs/core/src/session/turn_context.rs | 16 +- .../tools/handlers/request_plugin_install.rs | 2 +- codex-rs/rmcp-client/src/rmcp_client.rs | 5 + .../tests/streamable_http_oauth_startup.rs | 2 + .../tests/streamable_http_test_support.rs | 2 + 25 files changed, 552 insertions(+), 126 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/account_processor.rs b/codex-rs/app-server/src/request_processors/account_processor.rs index 4691e22204..d41879b65a 100644 --- a/codex-rs/app-server/src/request_processors/account_processor.rs +++ b/codex-rs/app-server/src/request_processors/account_processor.rs @@ -226,6 +226,10 @@ impl AccountRequestProcessor { ); } } + + if !thread_manager.list_thread_ids().await.is_empty() { + crate::mcp_refresh::queue_best_effort_refresh(thread_manager, config_manager).await; + } } fn spawn_effective_plugins_changed_task( 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 87560a163f..76877bf145 100644 --- a/codex-rs/app-server/src/request_processors/apps_processor.rs +++ b/codex-rs/app-server/src/request_processors/apps_processor.rs @@ -59,7 +59,7 @@ impl AppsRequestProcessor { }; let mut config = self.load_latest_config(fallback_cwd).await?; - if let Some(thread) = thread { + if let Some(thread) = thread.as_ref() { let _ = config .features .set_enabled(Feature::Apps, thread.enabled(Feature::Apps)); @@ -67,7 +67,10 @@ impl AppsRequestProcessor { let auth = self.auth_manager.auth().await; let mcp_manager = self.thread_manager.mcp_manager(); - let mcp_config = mcp_manager.runtime_config(&config).await; + let mcp_config = match thread { + Some(thread) => thread.runtime_mcp_config(&config).await, + None => mcp_manager.runtime_config(&config).await, + }; if !codex_mcp::host_owned_codex_apps_enabled(&mcp_config, auth.as_ref()) { return Ok(Some(AppsListResponse { data: Vec::new(), @@ -98,6 +101,7 @@ impl AppsRequestProcessor { request, params, config, + mcp_config, environment_manager, mcp_manager, plugins_manager, @@ -111,17 +115,20 @@ impl AppsRequestProcessor { self.shutdown_token.cancel(); } + #[allow(clippy::too_many_arguments)] async fn apps_list_task( outgoing: Arc, request_id: ConnectionRequestId, params: AppsListParams, config: Config, + mcp_config: codex_mcp::McpConfig, environment_manager: Arc, mcp_manager: Arc, plugins_manager: Arc, ) { let retry_params = params.clone(); let retry_config = config.clone(); + let retry_mcp_config = mcp_config.clone(); let retry_environment_manager = Arc::clone(&environment_manager); let retry_mcp_manager = Arc::clone(&mcp_manager); let retry_plugins_manager = Arc::clone(&plugins_manager); @@ -129,6 +136,7 @@ impl AppsRequestProcessor { &outgoing, params, config, + mcp_config, environment_manager, mcp_manager, plugins_manager, @@ -148,6 +156,7 @@ impl AppsRequestProcessor { &outgoing, retry_params, retry_config, + retry_mcp_config, retry_environment_manager, retry_mcp_manager, retry_plugins_manager, @@ -163,6 +172,7 @@ impl AppsRequestProcessor { outgoing: &Arc, params: AppsListParams, config: Config, + mcp_config: codex_mcp::McpConfig, environment_manager: Arc, mcp_manager: Arc, plugins_manager: Arc, @@ -189,47 +199,62 @@ impl AppsRequestProcessor { loaded_plugins.capability_summaries(), ); let plugin_apps = connector_snapshot.connector_ids().to_vec(); + let extension_managed_apps_enabled = + codex_mcp::extension_managed_codex_apps_enabled(&mcp_config); + let load_cached_all_connectors = async { + if extension_managed_apps_enabled { + None + } else { + connectors::list_cached_all_connectors(&config, &plugin_apps).await + } + }; let (mut accessible_connectors, mut all_connectors) = tokio::join!( - connectors::list_cached_accessible_connectors_from_mcp_tools( + connectors::list_cached_accessible_connectors_from_mcp_tools_with_mcp_config( &config, - mcp_manager.as_ref(), + &mcp_config, ), - connectors::list_cached_all_connectors(&config, &plugin_apps) + load_cached_all_connectors, ); let cached_all_connectors = all_connectors.clone(); let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel(); let accessible_config = config.clone(); + let accessible_mcp_config = mcp_config.clone(); let accessible_tx = tx.clone(); tokio::spawn(async move { - let result = connectors::list_accessible_connectors_from_mcp_tools_with_mcp_manager( + let result = connectors::list_accessible_connectors_from_mcp_tools_with_mcp_config( &accessible_config, force_refetch, Arc::clone(&environment_manager), mcp_manager, + accessible_mcp_config, ) .await .map_err(|err| format!("failed to load accessible apps: {err}")); let _ = accessible_tx.send(AppListLoadResult::Accessible(result)); }); - let all_config = config.clone(); - let all_plugin_apps = plugin_apps.clone(); - tokio::spawn(async move { - let result = connectors::list_all_connectors_with_options( - &all_config, - force_refetch, - &all_plugin_apps, - ) - .await - .map_err(|err| format!("failed to list apps: {err}")); - let _ = tx.send(AppListLoadResult::Directory(result)); - }); + if !extension_managed_apps_enabled { + let all_config = config.clone(); + let all_plugin_apps = plugin_apps.clone(); + tokio::spawn(async move { + let result = connectors::list_all_connectors_with_options( + &all_config, + force_refetch, + &all_plugin_apps, + ) + .await + .map_err(|err| format!("failed to list apps: {err}")); + let _ = tx.send(AppListLoadResult::Directory(result)); + }); + } else { + drop(tx); + } let app_list_deadline = tokio::time::Instant::now() + APP_LIST_LOAD_TIMEOUT; let mut accessible_loaded = false; - let mut all_loaded = false; + let mut all_loaded = extension_managed_apps_enabled; let mut codex_apps_ready = true; let mut last_notified_apps = None; diff --git a/codex-rs/app-server/tests/suite/v2/app_list.rs b/codex-rs/app-server/tests/suite/v2/app_list.rs index 5fad04929f..1140401806 100644 --- a/codex-rs/app-server/tests/suite/v2/app_list.rs +++ b/codex-rs/app-server/tests/suite/v2/app_list.rs @@ -1185,7 +1185,7 @@ async fn list_apps_force_refetch_preserves_previous_cache_on_failure() -> Result } #[tokio::test] -async fn list_apps_force_refetch_patches_updates_from_cached_snapshots() -> Result<()> { +async fn list_apps_force_refetch_does_not_reuse_extension_accessible_snapshot() -> Result<()> { let initial_connectors = vec![ AppInfo { id: "alpha".to_string(), @@ -1245,6 +1245,8 @@ async fn list_apps_force_refetch_patches_updates_from_cached_snapshots() -> Resu let mut mcp = TestAppServer::new(codex_home.path()).await?; timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + // App server installs the hosted Apps MCP extension, whose catalog must not reuse the + // compatibility server's shared accessible-app snapshot. let warm_request = mcp .send_apps_list_request(AppsListParams { limit: None, @@ -1356,57 +1358,6 @@ async fn list_apps_force_refetch_patches_updates_from_cached_snapshots() -> Resu }) .await?; - let first_update = read_app_list_updated_notification(&mut mcp).await?; - assert_eq!( - first_update.data, - vec![ - AppInfo { - id: "beta".to_string(), - name: "Beta App".to_string(), - description: Some("Beta v1".to_string()), - 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: Some("https://chatgpt.com/apps/beta-app/beta".to_string()), - is_accessible: true, - is_enabled: true, - plugin_display_names: Vec::new(), - }, - AppInfo { - id: "alpha".to_string(), - name: "Alpha".to_string(), - description: Some("Alpha v1".to_string()), - 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: Some("https://chatgpt.com/apps/alpha/alpha".to_string()), - is_accessible: false, - is_enabled: true, - plugin_display_names: Vec::new(), - }, - ] - ); - - let maybe_second_update = timeout( - Duration::from_millis(150), - read_app_list_updated_notification(&mut mcp), - ) - .await; - assert!( - maybe_second_update.is_err(), - "unexpected inaccessible-only app/list update during force refetch" - ); - let expected_final = vec![AppInfo { id: "alpha".to_string(), name: "Alpha".to_string(), @@ -1424,8 +1375,8 @@ async fn list_apps_force_refetch_patches_updates_from_cached_snapshots() -> Resu is_enabled: true, plugin_display_names: Vec::new(), }]; - let second_update = read_app_list_updated_notification(&mut mcp).await?; - assert_eq!(second_update.data, expected_final); + let update = read_app_list_updated_notification(&mut mcp).await?; + assert_eq!(update.data, expected_final); let refetch_response: JSONRPCResponse = timeout( DEFAULT_TIMEOUT, diff --git a/codex-rs/chatgpt/src/connectors.rs b/codex-rs/chatgpt/src/connectors.rs index bc84e5126d..cb23a791e5 100644 --- a/codex-rs/chatgpt/src/connectors.rs +++ b/codex-rs/chatgpt/src/connectors.rs @@ -13,10 +13,12 @@ use codex_connectors::merge::merge_plugin_connectors; use codex_core::config::Config; pub use codex_core::connectors::list_accessible_connectors_from_mcp_tools; pub use codex_core::connectors::list_accessible_connectors_from_mcp_tools_with_environment_manager; +pub use codex_core::connectors::list_accessible_connectors_from_mcp_tools_with_mcp_config; pub use codex_core::connectors::list_accessible_connectors_from_mcp_tools_with_mcp_manager; 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::list_cached_accessible_connectors_from_mcp_tools_with_mcp_config; pub use codex_core::connectors::with_app_enabled_state; use codex_login::AuthManager; use codex_login::CodexAuth; diff --git a/codex-rs/codex-mcp/src/codex_apps_cache.rs b/codex-rs/codex-mcp/src/codex_apps_cache.rs index ea39444628..5e75e0423a 100644 --- a/codex-rs/codex-mcp/src/codex_apps_cache.rs +++ b/codex-rs/codex-mcp/src/codex_apps_cache.rs @@ -40,6 +40,21 @@ pub struct CodexAppsToolsCacheKey { pub(crate) is_workspace_account: bool, } +impl CodexAppsToolsCacheKey { + /// Returns whether this key can safely partition process-shared Apps caches. + pub(crate) fn can_partition_shared_cache(&self) -> bool { + let has_account_id = self + .account_id + .as_deref() + .is_some_and(|id| !id.trim().is_empty()); + let has_chatgpt_user_id = self + .chatgpt_user_id + .as_deref() + .is_some_and(|id| !id.trim().is_empty()); + has_account_id && (!self.is_workspace_account || has_chatgpt_user_id) + } +} + /// Builds the CodexAuth-backed Codex Apps cache key. pub fn codex_apps_tools_cache_key(auth: Option<&CodexAuth>) -> CodexAppsToolsCacheKey { CodexAppsToolsCacheKey { @@ -68,6 +83,10 @@ pub(crate) struct CodexAppsToolsCacheContext { } impl CodexAppsToolsCacheContext { + pub(crate) fn is_for_auth_key(&self, auth_key: &CodexAppsToolsCacheKey) -> bool { + self.entry.identity.auth_key == *auth_key + } + pub(crate) fn tools_cache_path(&self) -> PathBuf { self.entry .identity diff --git a/codex-rs/codex-mcp/src/connection_manager.rs b/codex-rs/codex-mcp/src/connection_manager.rs index 476bfd0166..d51fe174ea 100644 --- a/codex-rs/codex-mcp/src/connection_manager.rs +++ b/codex-rs/codex-mcp/src/connection_manager.rs @@ -17,11 +17,14 @@ use crate::McpAuthStatusEntry; use crate::codex_apps_cache::CodexAppsToolsCache; use crate::codex_apps_cache::CodexAppsToolsCacheKey; use crate::codex_apps_cache::CodexAppsToolsFetchSource; +use crate::codex_apps_cache::codex_apps_tools_cache_key; use crate::elicitation::ElicitationRequestManager; use crate::elicitation::ElicitationRequestRouter; use crate::elicitation::ElicitationReviewerHandle; use crate::mcp::CODEX_APPS_MCP_SERVER_NAME; use crate::mcp::ToolPluginProvenance; +use crate::mcp::codex_apps_server_uses_codex_auth_cache; +use crate::mcp::mcp_server_has_configured_request_auth; use crate::rmcp_client::AsyncManagedClient; use crate::rmcp_client::DEFAULT_STARTUP_TIMEOUT; use crate::rmcp_client::MCP_TOOLS_FETCH_UNCACHED_DURATION_METRIC; @@ -195,6 +198,9 @@ impl McpConnectionManager { }); let shares_codex_apps_tools_cache = should_share_codex_apps_tools_cache( &server_name, + &server, + auth, + &codex_apps_tools_cache_key, uses_env_bearer_token, enable_codex_apps_tools_cache, ); @@ -202,14 +208,8 @@ impl McpConnectionManager { codex_apps_tools_cache .context(codex_home.clone(), codex_apps_tools_cache_key.clone()) }); - // If Codex Apps has an env bearer token, that is its auth path. Do - // not also attach the ambient CodexAuth provider. let runtime_auth_provider = - if server_name == CODEX_APPS_MCP_SERVER_NAME && uses_env_bearer_token { - None - } else { - chatgpt_auth_provider_for_server(&server, chatgpt_auth_provider.clone()) - }; + chatgpt_auth_provider_for_server(&server, chatgpt_auth_provider.clone()); let async_managed_client = AsyncManagedClient::new( server_name.clone(), startup_submit_id.clone(), @@ -437,6 +437,14 @@ impl McpConnectionManager { server_name == CODEX_APPS_MCP_SERVER_NAME && self.server_metadata.contains_key(server_name) } + /// Returns whether the Apps client cache is pinned to this CodexAuth identity. + pub fn uses_shared_codex_apps_tools_cache_for(&self, auth: Option<&CodexAuth>) -> bool { + let auth_key = codex_apps_tools_cache_key(auth); + self.clients + .get(CODEX_APPS_MCP_SERVER_NAME) + .is_some_and(|client| client.uses_codex_apps_tools_cache_for(&auth_key)) + } + pub fn set_approval_policy(&self, approval_policy: &Constrained) { if let Ok(mut policy) = self.elicitation_requests.approval_policy.lock() { *policy = approval_policy.value(); @@ -908,9 +916,9 @@ fn chatgpt_auth_provider_for_server( server: &EffectiveMcpServer, chatgpt_auth_provider: Option, ) -> Option { - if !server - .configured_config() - .is_some_and(|config| matches!(&config.auth, McpServerAuth::ChatGpt)) + let config = server.configured_config()?; + if !matches!(&config.auth, McpServerAuth::ChatGpt) + || mcp_server_has_configured_request_auth(config) { return None; } @@ -919,10 +927,16 @@ fn chatgpt_auth_provider_for_server( fn should_share_codex_apps_tools_cache( server_name: &str, + server: &EffectiveMcpServer, + auth: Option<&CodexAuth>, + cache_key: &CodexAppsToolsCacheKey, uses_env_bearer_token: bool, cache_enabled: bool, ) -> bool { - server_name == CODEX_APPS_MCP_SERVER_NAME && !uses_env_bearer_token && cache_enabled + server_name == CODEX_APPS_MCP_SERVER_NAME + && !uses_env_bearer_token + && cache_enabled + && codex_apps_server_uses_codex_auth_cache(server, auth, cache_key) } async fn emit_update( diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index 94d4a92c24..9947f02f8c 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -732,8 +732,14 @@ fn filter_tools_applies_per_server_filters() { #[test] fn codex_apps_env_bearer_token_bypasses_shared_tools_cache() { + let server = codex_apps_server_for_cache(McpServerAuth::ChatGpt); + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + let cache_key = codex_apps_tools_cache_key(Some(&auth)); assert!(!should_share_codex_apps_tools_cache( CODEX_APPS_MCP_SERVER_NAME, + &server, + Some(&auth), + &cache_key, /*uses_env_bearer_token*/ true, /*cache_enabled*/ true, )); @@ -741,22 +747,189 @@ fn codex_apps_env_bearer_token_bypasses_shared_tools_cache() { #[test] fn extension_managed_codex_apps_bypass_shared_tools_cache() { + let server = codex_apps_server_for_cache(McpServerAuth::ChatGpt); + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + let cache_key = codex_apps_tools_cache_key(Some(&auth)); assert!(!should_share_codex_apps_tools_cache( CODEX_APPS_MCP_SERVER_NAME, + &server, + Some(&auth), + &cache_key, /*uses_env_bearer_token*/ false, /*cache_enabled*/ false, )); } #[test] -fn codex_apps_chatgpt_auth_uses_shared_tools_cache() { - assert!(should_share_codex_apps_tools_cache( +fn codex_apps_oauth_bypasses_shared_tools_cache() { + let server = codex_apps_server_for_cache(McpServerAuth::OAuth); + let cache_key = codex_apps_tools_cache_key(/*auth*/ None); + assert!(!should_share_codex_apps_tools_cache( CODEX_APPS_MCP_SERVER_NAME, + &server, + /*auth*/ None, + &cache_key, /*uses_env_bearer_token*/ false, /*cache_enabled*/ true, )); } +#[test] +fn codex_apps_chatgpt_auth_uses_shared_tools_cache() { + let server = codex_apps_server_for_cache(McpServerAuth::ChatGpt); + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + let cache_key = codex_apps_tools_cache_key(Some(&auth)); + assert!(should_share_codex_apps_tools_cache( + CODEX_APPS_MCP_SERVER_NAME, + &server, + Some(&auth), + &cache_key, + /*uses_env_bearer_token*/ false, + /*cache_enabled*/ true, + )); +} + +#[test] +fn codex_apps_without_chatgpt_auth_bypasses_shared_tools_cache() { + let server = codex_apps_server_for_cache(McpServerAuth::ChatGpt); + let cache_key = codex_apps_tools_cache_key(/*auth*/ None); + assert!(!should_share_codex_apps_tools_cache( + CODEX_APPS_MCP_SERVER_NAME, + &server, + /*auth*/ None, + &cache_key, + /*uses_env_bearer_token*/ false, + /*cache_enabled*/ true, + )); +} + +#[test] +fn codex_apps_unsafe_or_mismatched_cache_key_bypasses_shared_tools_cache() { + let server = codex_apps_server_for_cache(McpServerAuth::ChatGpt); + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + let cache_key = CodexAppsToolsCacheKey { + account_id: Some("workspace-id".to_string()), + chatgpt_user_id: None, + is_workspace_account: true, + }; + let workspace_user_only_key = CodexAppsToolsCacheKey { + account_id: None, + chatgpt_user_id: Some("user-id".to_string()), + is_workspace_account: true, + }; + let empty_identity_key = CodexAppsToolsCacheKey { + account_id: Some(" ".to_string()), + chatgpt_user_id: Some(String::new()), + is_workspace_account: true, + }; + + assert!(!cache_key.can_partition_shared_cache()); + assert!(!workspace_user_only_key.can_partition_shared_cache()); + assert!(!empty_identity_key.can_partition_shared_cache()); + assert!(!should_share_codex_apps_tools_cache( + CODEX_APPS_MCP_SERVER_NAME, + &server, + Some(&auth), + &cache_key, + /*uses_env_bearer_token*/ false, + /*cache_enabled*/ true, + )); +} + +#[test] +fn codex_apps_authorization_header_bypasses_shared_tools_cache() { + let mut config = crate::mcp::codex_apps_mcp_server_config( + "https://chatgpt.com", + /*apps_mcp_product_sku*/ None, + ); + let McpServerTransportConfig::StreamableHttp { http_headers, .. } = &mut config.transport + else { + panic!("expected streamable HTTP Apps server"); + }; + *http_headers = Some(HashMap::from([( + "Authorization".to_string(), + "Bearer external".to_string(), + )])); + let server = EffectiveMcpServer::configured(config); + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + let cache_key = codex_apps_tools_cache_key(Some(&auth)); + + assert!(!should_share_codex_apps_tools_cache( + CODEX_APPS_MCP_SERVER_NAME, + &server, + Some(&auth), + &cache_key, + /*uses_env_bearer_token*/ false, + /*cache_enabled*/ true, + )); +} + +#[test] +fn configured_actor_auth_suppresses_ambient_chatgpt_provider() { + let mut config = crate::mcp::codex_apps_mcp_server_config( + "https://chatgpt.com", + /*apps_mcp_product_sku*/ None, + ); + let McpServerTransportConfig::StreamableHttp { http_headers, .. } = &mut config.transport + else { + panic!("expected streamable HTTP Apps server"); + }; + *http_headers = Some(HashMap::from([( + "X-OpenAI-Actor-Authorization".to_string(), + "actor-biscuit".to_string(), + )])); + let server = EffectiveMcpServer::configured(config); + let provider = codex_model_provider::auth_provider_from_auth( + &CodexAuth::create_dummy_chatgpt_auth_for_testing(), + ); + + assert!(chatgpt_auth_provider_for_server(&server, Some(provider)).is_none()); +} + +#[tokio::test] +async fn shared_codex_apps_cache_requires_matching_auth_identity() { + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + let codex_home = tempdir().expect("tempdir"); + let cache_context = CodexAppsToolsCache::default().context( + codex_home.path().to_path_buf(), + codex_apps_tools_cache_key(Some(&auth)), + ); + let mut client = create_ready_async_managed_client(Vec::new()).await; + client.is_codex_apps_mcp_server = true; + client.codex_apps_tools_cache_context = Some(cache_context); + + let approval_policy = Constrained::allow_any(AskForApproval::OnRequest); + let permission_profile = Constrained::allow_any(PermissionProfile::default()); + let mut manager = McpConnectionManager::new_uninitialized( + &approval_policy, + &permission_profile, + /*prefix_mcp_tool_names*/ true, + ); + manager + .clients + .insert(CODEX_APPS_MCP_SERVER_NAME.to_string(), client); + + assert!(manager.uses_shared_codex_apps_tools_cache_for(Some(&auth))); + assert!(!manager.uses_shared_codex_apps_tools_cache_for(/*auth*/ None)); +} + +fn codex_apps_server_for_cache(auth: McpServerAuth) -> EffectiveMcpServer { + let mut config = crate::mcp::codex_apps_mcp_server_config( + "https://chatgpt.com", + /*apps_mcp_product_sku*/ None, + ); + config.auth = auth; + let McpServerTransportConfig::StreamableHttp { + bearer_token_env_var, + .. + } = &mut config.transport + else { + panic!("expected streamable HTTP Apps server"); + }; + *bearer_token_env_var = None; + EffectiveMcpServer::configured(config) +} + #[tokio::test] async fn list_all_tools_uses_shared_codex_apps_cache_while_client_is_pending() { let codex_home = tempdir().expect("tempdir"); diff --git a/codex-rs/codex-mcp/src/lib.rs b/codex-rs/codex-mcp/src/lib.rs index 29f005166c..aabc52b390 100644 --- a/codex-rs/codex-mcp/src/lib.rs +++ b/codex-rs/codex-mcp/src/lib.rs @@ -40,6 +40,7 @@ pub use codex_apps_cache::CodexAppsToolsCache; pub use codex_apps_cache::CodexAppsToolsCacheKey; pub use codex_apps_cache::codex_apps_tools_cache_key; pub use mcp::codex_apps_mcp_server_config; +pub use mcp::codex_apps_tools_cache_enabled; pub use mcp::configured_mcp_servers; pub use mcp::effective_mcp_servers; pub use mcp::effective_mcp_servers_from_configured; diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index bf15730b19..2a736ca620 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -46,6 +46,7 @@ use tokio_util::sync::CancellationToken; use crate::McpServerSource; use crate::ResolvedMcpCatalog; use crate::codex_apps_cache::CodexAppsToolsCache; +use crate::codex_apps_cache::CodexAppsToolsCacheKey; use crate::codex_apps_cache::codex_apps_tools_cache_key; use crate::connection_manager::McpConnectionManager; use crate::runtime::McpRuntimeContext; @@ -248,10 +249,36 @@ pub fn extension_managed_codex_apps_enabled(config: &McpConfig) -> bool { .server(CODEX_APPS_MCP_SERVER_NAME) .is_some_and(|server| { matches!(server.source(), McpServerSource::Extension { .. }) - && matches!(&server.config().auth, &McpServerAuth::OAuth) + && (matches!(&server.config().auth, &McpServerAuth::OAuth) + || mcp_server_has_configured_request_auth(server.config())) }) } +pub(crate) fn mcp_server_has_configured_request_auth(server: &McpServerConfig) -> bool { + let McpServerTransportConfig::StreamableHttp { + bearer_token_env_var, + http_headers, + env_http_headers, + .. + } = &server.transport + else { + return false; + }; + + bearer_token_env_var.is_some() + || http_headers + .as_ref() + .is_some_and(|headers| headers.keys().any(|name| is_request_auth_header(name))) + || env_http_headers + .as_ref() + .is_some_and(|headers| headers.keys().any(|name| is_request_auth_header(name))) +} + +fn is_request_auth_header(name: &str) -> bool { + name.eq_ignore_ascii_case("authorization") + || name.eq_ignore_ascii_case("x-openai-actor-authorization") +} + pub fn host_owned_codex_apps_enabled(config: &McpConfig, auth: Option<&CodexAuth>) -> bool { extension_managed_codex_apps_enabled(config) || (config.apps_enabled && auth.is_some_and(CodexAuth::uses_codex_backend)) @@ -265,7 +292,55 @@ pub fn effective_mcp_servers( config: &McpConfig, auth: Option<&CodexAuth>, ) -> HashMap { - effective_mcp_servers_from_configured(configured_mcp_servers(config), config, auth) + let mut servers = + effective_mcp_servers_from_configured(configured_mcp_servers(config), config, auth); + if config + .mcp_server_catalog + .server(CODEX_APPS_MCP_SERVER_NAME) + .is_some_and(|server| mcp_server_has_configured_request_auth(server.config())) + && let Some(server) = servers.get_mut(CODEX_APPS_MCP_SERVER_NAME) + { + server.disable_stored_oauth(); + } + servers +} + +/// Returns whether Codex Apps can safely use caches partitioned by CodexAuth. +pub fn codex_apps_tools_cache_enabled(config: &McpConfig, auth: Option<&CodexAuth>) -> bool { + if config.apps_mcp_product_sku.is_some() { + return false; + } + if !config + .mcp_server_catalog + .server(CODEX_APPS_MCP_SERVER_NAME) + .is_some_and(|server| matches!(server.source(), McpServerSource::Compatibility { .. })) + { + return false; + } + + let cache_key = codex_apps_tools_cache_key(auth); + effective_mcp_servers(config, auth) + .get(CODEX_APPS_MCP_SERVER_NAME) + .is_some_and(|server| codex_apps_server_uses_codex_auth_cache(server, auth, &cache_key)) +} + +pub(crate) fn codex_apps_server_uses_codex_auth_cache( + server: &EffectiveMcpServer, + auth: Option<&CodexAuth>, + cache_key: &CodexAppsToolsCacheKey, +) -> bool { + let Some(server) = server.configured_config() else { + return false; + }; + let McpServerTransportConfig::StreamableHttp { .. } = &server.transport else { + return false; + }; + + matches!(&server.auth, McpServerAuth::ChatGpt) + && !mcp_server_has_configured_request_auth(server) + && auth.is_some_and(CodexAuth::uses_codex_backend) + && codex_apps_tools_cache_key(auth) == *cache_key + && cache_key.can_partition_shared_cache() } /// Converts a materialized server map to its auth-gated runtime view. @@ -348,7 +423,7 @@ pub async fn read_mcp_resource( config.codex_home.clone(), codex_apps_tools_cache, codex_apps_tools_cache_key(auth), - /*enable_codex_apps_tools_cache*/ !extension_managed_codex_apps_enabled(config), + /*enable_codex_apps_tools_cache*/ codex_apps_tools_cache_enabled(config, auth), config.prefix_mcp_tool_names, config.client_elicitation_capability.clone(), /*supports_openai_form_elicitation*/ false, @@ -426,7 +501,7 @@ pub async fn collect_mcp_server_status_snapshot_with_detail( config.codex_home.clone(), codex_apps_tools_cache, codex_apps_tools_cache_key(auth), - /*enable_codex_apps_tools_cache*/ !extension_managed_codex_apps_enabled(config), + /*enable_codex_apps_tools_cache*/ codex_apps_tools_cache_enabled(config, auth), config.prefix_mcp_tool_names, config.client_elicitation_capability.clone(), /*supports_openai_form_elicitation*/ false, diff --git a/codex-rs/codex-mcp/src/mcp/mod_tests.rs b/codex-rs/codex-mcp/src/mcp/mod_tests.rs index 458f0debd6..d038f1a6df 100644 --- a/codex-rs/codex-mcp/src/mcp/mod_tests.rs +++ b/codex-rs/codex-mcp/src/mcp/mod_tests.rs @@ -327,6 +327,94 @@ fn effective_mcp_servers_preserve_extension_owned_oauth_codex_apps_without_chatg .and_then(EffectiveMcpServer::configured_config), Some(&expected) ); + assert!( + !effective + .get(CODEX_APPS_MCP_SERVER_NAME) + .expect("extension Apps server") + .allows_stored_oauth() + ); + assert!(!codex_apps_tools_cache_enabled(&config, /*auth*/ None)); +} + +#[test] +fn extension_owned_oauth_without_static_auth_can_use_stored_credentials() { + let mut config = test_mcp_config(PathBuf::new()); + config.apps_enabled = true; + + let mut extension_config = + codex_apps_mcp_server_config(&config.chatgpt_base_url, /*apps_mcp_product_sku*/ None); + extension_config.auth = McpServerAuth::OAuth; + let McpServerTransportConfig::StreamableHttp { + bearer_token_env_var, + .. + } = &mut extension_config.transport + else { + panic!("expected streamable http transport for codex apps"); + }; + *bearer_token_env_var = None; + + let mut catalog = ResolvedMcpCatalog::builder(); + catalog.register(McpServerRegistration::from_extension( + CODEX_APPS_MCP_SERVER_NAME.to_string(), + "host_managed_apps".to_string(), + /*contribution_order*/ 0, + extension_config, + )); + config.mcp_server_catalog = catalog.build(); + + let effective = effective_mcp_servers(&config, /*auth*/ None); + + assert!( + effective + .get(CODEX_APPS_MCP_SERVER_NAME) + .expect("extension Apps server") + .allows_stored_oauth() + ); + assert!(!codex_apps_tools_cache_enabled(&config, /*auth*/ None)); +} + +#[test] +fn effective_mcp_servers_preserve_extension_owned_static_auth_without_chatgpt_auth() { + let mut config = test_mcp_config(PathBuf::new()); + config.apps_enabled = true; + + let compatibility_config = + codex_apps_mcp_server_config(&config.chatgpt_base_url, /*apps_mcp_product_sku*/ None); + let mut extension_config = compatibility_config.clone(); + let McpServerTransportConfig::StreamableHttp { http_headers, .. } = + &mut extension_config.transport + else { + panic!("expected streamable http transport for codex apps"); + }; + *http_headers = Some(HashMap::from([( + "X-OpenAI-Actor-Authorization".to_string(), + "actor-biscuit".to_string(), + )])); + let expected = extension_config.clone(); + + let mut catalog = ResolvedMcpCatalog::builder(); + catalog.register(McpServerRegistration::from_compatibility( + CODEX_APPS_MCP_SERVER_NAME.to_string(), + "legacy_codex_apps".to_string(), + compatibility_config, + )); + catalog.register(McpServerRegistration::from_extension( + CODEX_APPS_MCP_SERVER_NAME.to_string(), + "host_managed_apps".to_string(), + /*contribution_order*/ 0, + extension_config, + )); + config.mcp_server_catalog = catalog.build(); + + let effective = effective_mcp_servers(&config, /*auth*/ None); + + assert_eq!( + effective + .get(CODEX_APPS_MCP_SERVER_NAME) + .and_then(EffectiveMcpServer::configured_config), + Some(&expected) + ); + assert!(!codex_apps_tools_cache_enabled(&config, /*auth*/ None)); } #[test] @@ -334,17 +422,32 @@ fn effective_mcp_servers_hide_chatgpt_codex_apps_without_chatgpt_auth() { let mut config = test_mcp_config(PathBuf::new()); config.apps_enabled = true; + let mut compatibility_config = + codex_apps_mcp_server_config(&config.chatgpt_base_url, /*apps_mcp_product_sku*/ None); + let McpServerTransportConfig::StreamableHttp { + bearer_token_env_var, + .. + } = &mut compatibility_config.transport + else { + panic!("expected streamable http transport for codex apps"); + }; + *bearer_token_env_var = None; let mut catalog = ResolvedMcpCatalog::builder(); catalog.register(McpServerRegistration::from_compatibility( CODEX_APPS_MCP_SERVER_NAME.to_string(), "legacy_codex_apps".to_string(), - codex_apps_mcp_server_config(&config.chatgpt_base_url, /*apps_mcp_product_sku*/ None), + compatibility_config, )); config.mcp_server_catalog = catalog.build(); let effective = effective_mcp_servers(&config, /*auth*/ None); + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); assert!(!effective.contains_key(CODEX_APPS_MCP_SERVER_NAME)); + assert!(!codex_apps_tools_cache_enabled(&config, /*auth*/ None)); + assert!(codex_apps_tools_cache_enabled(&config, Some(&auth))); + config.apps_mcp_product_sku = Some("sku".to_string()); + assert!(!codex_apps_tools_cache_enabled(&config, Some(&auth))); } #[test] @@ -363,8 +466,10 @@ fn effective_mcp_servers_hide_config_owned_oauth_codex_apps_without_chatgpt_auth config.mcp_server_catalog = catalog.build(); let effective = effective_mcp_servers(&config, /*auth*/ None); + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); assert!(!effective.contains_key(CODEX_APPS_MCP_SERVER_NAME)); + assert!(!codex_apps_tools_cache_enabled(&config, Some(&auth))); } #[tokio::test] @@ -437,6 +542,13 @@ async fn effective_mcp_servers_preserve_runtime_servers() { config.mcp_server_catalog = catalog.build(); let effective = effective_mcp_servers(&config, Some(&auth)); + assert!(!codex_apps_tools_cache_enabled(&config, Some(&auth))); + assert!( + effective + .get(CODEX_APPS_MCP_SERVER_NAME) + .expect("configured Apps server") + .allows_stored_oauth() + ); let sample = effective.get("sample").expect("user server should exist"); let docs = effective diff --git a/codex-rs/codex-mcp/src/rmcp_client.rs b/codex-rs/codex-mcp/src/rmcp_client.rs index 37819203f6..c29d29968e 100644 --- a/codex-rs/codex-mcp/src/rmcp_client.rs +++ b/codex-rs/codex-mcp/src/rmcp_client.rs @@ -22,6 +22,7 @@ use crate::codex_apps::normalize_codex_apps_callable_name; use crate::codex_apps::normalize_codex_apps_callable_namespace; use crate::codex_apps::normalize_codex_apps_tool_title; use crate::codex_apps_cache::CodexAppsToolsCacheContext; +use crate::codex_apps_cache::CodexAppsToolsCacheKey; use crate::codex_apps_cache::CodexAppsToolsFetchSource; use crate::codex_apps_cache::load_startup_cached_codex_apps_server_info; use crate::elicitation::ElicitationRequestManager; @@ -498,6 +499,15 @@ impl AsyncManagedClient { .is_some_and(CodexAppsToolsCacheContext::has_current_tools) } + pub(crate) fn uses_codex_apps_tools_cache_for( + &self, + auth_key: &CodexAppsToolsCacheKey, + ) -> bool { + self.codex_apps_tools_cache_context + .as_ref() + .is_some_and(|context| context.is_for_auth_key(auth_key)) + } + fn cached_tools(&self) -> Option> { self.codex_apps_tools_cache_context .as_ref() @@ -925,6 +935,7 @@ async fn make_rmcp_client( runtime_context: McpRuntimeContext, runtime_auth_provider: Option, ) -> Result { + let allow_stored_oauth = server.allows_stored_oauth(); let config = match server.launch() { McpServerLaunch::Configured(config) => config.as_ref().clone(), }; @@ -995,6 +1006,7 @@ async fn make_rmcp_client( env_http_headers, store_mode, keyring_backend_kind, + allow_stored_oauth, http_client, runtime_auth_provider, ) diff --git a/codex-rs/codex-mcp/src/server.rs b/codex-rs/codex-mcp/src/server.rs index 39287b2136..ef31a76e21 100644 --- a/codex-rs/codex-mcp/src/server.rs +++ b/codex-rs/codex-mcp/src/server.rs @@ -14,15 +14,25 @@ pub(crate) enum McpServerLaunch { #[derive(Debug, Clone)] pub struct EffectiveMcpServer { launch: McpServerLaunch, + allow_stored_oauth: bool, } impl EffectiveMcpServer { pub fn configured(config: McpServerConfig) -> Self { Self { launch: McpServerLaunch::Configured(Box::new(config)), + allow_stored_oauth: true, } } + pub(crate) fn disable_stored_oauth(&mut self) { + self.allow_stored_oauth = false; + } + + pub(crate) fn allows_stored_oauth(&self) -> bool { + self.allow_stored_oauth + } + pub(crate) fn launch(&self) -> &McpServerLaunch { &self.launch } diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index 73ffec014c..a8130c1693 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -33,14 +33,15 @@ use codex_login::AuthManager; use codex_login::CodexAuth; use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; use codex_mcp::MCP_TOOL_CODEX_APPS_META_KEY; +use codex_mcp::McpConfig; use codex_mcp::McpConnectionManager; use codex_mcp::McpRuntimeContext; use codex_mcp::ToolInfo; use codex_mcp::ToolPluginProvenance; +use codex_mcp::codex_apps_tools_cache_enabled; use codex_mcp::codex_apps_tools_cache_key; use codex_mcp::compute_auth_statuses; use codex_mcp::effective_mcp_servers; -use codex_mcp::extension_managed_codex_apps_enabled; use codex_mcp::host_owned_codex_apps_enabled; use codex_mcp::tool_plugin_provenance; @@ -133,15 +134,22 @@ pub(crate) async fn list_tool_suggest_discoverable_tools_with_auth( pub async fn list_cached_accessible_connectors_from_mcp_tools( config: &Config, mcp_manager: &McpManager, +) -> Option> { + let mcp_config = mcp_manager.runtime_config(config).await; + list_cached_accessible_connectors_from_mcp_tools_with_mcp_config(config, &mcp_config).await +} + +pub async fn list_cached_accessible_connectors_from_mcp_tools_with_mcp_config( + config: &Config, + mcp_config: &McpConfig, ) -> Option> { let auth_manager = AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false).await; let auth = auth_manager.auth().await; - let mcp_config = mcp_manager.runtime_config(config).await; - if !host_owned_codex_apps_enabled(&mcp_config, auth.as_ref()) { + if !host_owned_codex_apps_enabled(mcp_config, auth.as_ref()) { return Some(Vec::new()); } - if extension_managed_codex_apps_enabled(&mcp_config) { + if !codex_apps_tools_cache_enabled(mcp_config, auth.as_ref()) { return None; } let cache_key = accessible_connectors_cache_key(config, auth.as_ref()); @@ -151,10 +159,10 @@ pub async fn list_cached_accessible_connectors_from_mcp_tools( pub(crate) fn refresh_accessible_connectors_cache_from_mcp_tools( config: &Config, auth: Option<&CodexAuth>, - extension_managed_apps_enabled: bool, + uses_shared_codex_apps_tools_cache: bool, mcp_tools: &[ToolInfo], ) { - if !config.features.enabled(Feature::Apps) || extension_managed_apps_enabled { + if !config.features.enabled(Feature::Apps) || !uses_shared_codex_apps_tools_cache { return; } @@ -217,19 +225,36 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager( force_refetch: bool, environment_manager: Arc, mcp_manager: Arc, +) -> anyhow::Result { + let mcp_config = mcp_manager.runtime_config(config).await; + list_accessible_connectors_from_mcp_tools_with_mcp_config( + config, + force_refetch, + environment_manager, + mcp_manager, + mcp_config, + ) + .await +} + +pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_config( + config: &Config, + force_refetch: bool, + environment_manager: Arc, + mcp_manager: Arc, + mcp_config: McpConfig, ) -> anyhow::Result { let auth_manager = AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false).await; let auth = auth_manager.auth().await; - let mcp_config = mcp_manager.runtime_config(config).await; if !host_owned_codex_apps_enabled(&mcp_config, auth.as_ref()) { return Ok(AccessibleConnectorsStatus { connectors: Vec::new(), codex_apps_ready: true, }); } - let cache_key = (!extension_managed_codex_apps_enabled(&mcp_config)) - .then(|| accessible_connectors_cache_key(config, auth.as_ref())); + let can_share_cache = codex_apps_tools_cache_enabled(&mcp_config, auth.as_ref()); + let cache_key = can_share_cache.then(|| accessible_connectors_cache_key(config, auth.as_ref())); let tool_plugin_provenance = tool_plugin_provenance(&mcp_config); if !force_refetch && let Some(cache_key) = cache_key.as_ref() @@ -282,7 +307,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager( config.codex_home.to_path_buf(), mcp_manager.codex_apps_tools_cache(), codex_apps_tools_cache_key(auth.as_ref()), - /*enable_codex_apps_tools_cache*/ !extension_managed_codex_apps_enabled(&mcp_config), + /*enable_codex_apps_tools_cache*/ can_share_cache, mcp_config.prefix_mcp_tool_names, mcp_config.client_elicitation_capability, /*supports_openai_form_elicitation*/ false, diff --git a/codex-rs/core/src/connectors_tests.rs b/codex-rs/core/src/connectors_tests.rs index 9b24efa370..d274f812f3 100644 --- a/codex-rs/core/src/connectors_tests.rs +++ b/codex-rs/core/src/connectors_tests.rs @@ -275,7 +275,7 @@ async fn refresh_accessible_connectors_cache_from_mcp_tools_writes_latest_instal let cached = with_accessible_connectors_cache_cleared(|| { refresh_accessible_connectors_cache_from_mcp_tools( - &config, /*auth*/ None, /*extension_managed_apps_enabled*/ false, &tools, + &config, /*auth*/ None, /*uses_shared_codex_apps_tools_cache*/ true, &tools, ); read_cached_accessible_connectors(&cache_key).expect("cache should be populated") }); @@ -340,7 +340,7 @@ async fn extension_managed_apps_do_not_write_accessible_connectors_cache() { let cached = with_accessible_connectors_cache_cleared(|| { refresh_accessible_connectors_cache_from_mcp_tools( - &config, /*auth*/ None, /*extension_managed_apps_enabled*/ true, &tools, + &config, /*auth*/ None, /*uses_shared_codex_apps_tools_cache*/ false, &tools, ); read_cached_accessible_connectors(&cache_key) }); diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index 8885108391..3919e2f203 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -700,7 +700,7 @@ async fn refresh_codex_apps_after_connector_auth( connectors::refresh_accessible_connectors_cache_from_mcp_tools( &turn_context.config, auth.as_ref(), - turn_context.extension_managed_apps_enabled, + manager.uses_shared_codex_apps_tools_cache_for(auth.as_ref()), &mcp_tools, ); } diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index d3e903799f..f32004aa4a 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -362,7 +362,7 @@ impl Session { self.services.mcp_manager.codex_apps_tools_cache(), codex_apps_tools_cache_key(auth.as_ref()), /*enable_codex_apps_tools_cache*/ - !codex_mcp::extension_managed_codex_apps_enabled(&mcp_config), + codex_mcp::codex_apps_tools_cache_enabled(&mcp_config, auth.as_ref()), mcp_config.prefix_mcp_tool_names, mcp_config.client_elicitation_capability.clone(), self.services diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 978768ba00..a8e0911d43 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -3239,7 +3239,8 @@ impl Session { .push(PersonalitySpecInstructions::new(personality_message).render()); } } - if turn_context.config.include_apps_instructions && turn_context.apps_enabled() { + if turn_context.config.include_apps_instructions && turn_context.apps_enabled(mcp.config()) + { let accessible_and_enabled_connectors = connectors::list_accessible_and_enabled_connectors_from_manager( mcp.manager(), diff --git a/codex-rs/core/src/session/review.rs b/codex-rs/core/src/session/review.rs index eb4b0f2bd6..1587290d33 100644 --- a/codex-rs/core/src/session/review.rs +++ b/codex-rs/core/src/session/review.rs @@ -111,7 +111,6 @@ pub(super) async fn spawn_review_thread( realtime_active: parent_turn_context.realtime_active, config: per_turn_config, auth_manager: auth_manager_for_context, - extension_managed_apps_enabled: parent_turn_context.extension_managed_apps_enabled, model_info: model_info.clone(), session_telemetry: session_telemetry_for_context, provider: provider_for_context, diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 6fbd654f7a..62058b9281 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -1207,8 +1207,9 @@ impl Session { config.codex_home.to_path_buf(), sess.services.mcp_manager.codex_apps_tools_cache(), codex_apps_tools_cache_key(auth), - /*enable_codex_apps_tools_cache*/ !codex_mcp::extension_managed_codex_apps_enabled( + /*enable_codex_apps_tools_cache*/ codex_mcp::codex_apps_tools_cache_enabled( &mcp_config, + auth, ), config.prefix_mcp_tool_names(), mcp_config.client_elicitation_capability.clone(), diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index f16b525002..072dd1d646 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -545,7 +545,8 @@ async fn build_skills_and_plugins( let mentioned_plugins = collect_explicit_plugin_mentions(&user_input, loaded_plugins.capability_summaries()); let connector_snapshot = step_context.mcp.config().connector_snapshot.clone(); - let mcp_tools = if turn_context.apps_enabled() || !mentioned_plugins.is_empty() { + let apps_enabled = turn_context.apps_enabled(step_context.mcp.config()); + let mcp_tools = if apps_enabled || !mentioned_plugins.is_empty() { // Plugin mentions need raw MCP/app inventory even when app tools // are normally hidden so we can describe the plugin's currently // usable capabilities for this turn. @@ -557,13 +558,13 @@ async fn build_skills_and_plugins( .await { Ok(mcp_tools) => mcp_tools, - Err(_) if turn_context.apps_enabled() => return None, + Err(_) if apps_enabled => return None, Err(_) => Vec::new(), } } else { Vec::new() }; - let available_connectors = if turn_context.apps_enabled() { + let available_connectors = if apps_enabled { let connectors = codex_connectors::merge::merge_plugin_connectors_with_accessible( connector_snapshot .connector_ids() @@ -1171,7 +1172,7 @@ async fn run_sampling_request( fields( turn_id = %step_context.turn.sub_id, model = %step_context.turn.model_info.slug, - apps_enabled = step_context.turn.apps_enabled() + apps_enabled = step_context.turn.apps_enabled(step_context.mcp.config()) ) )] pub(crate) async fn built_tools( @@ -1194,7 +1195,7 @@ pub(crate) async fn built_tools( .await; let connector_snapshot = step_context.mcp.config().connector_snapshot.clone(); - let apps_enabled = turn_context.apps_enabled(); + let apps_enabled = turn_context.apps_enabled(step_context.mcp.config()); let accessible_connectors = apps_enabled.then(|| connectors::accessible_connectors_from_mcp_tools(&all_mcp_tools)); let accessible_connectors_with_enabled_state = diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 08847177d6..77e3d3a73c 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -105,7 +105,6 @@ pub struct TurnContext { pub(crate) realtime_active: bool, pub config: Arc, pub(crate) auth_manager: Option>, - pub(crate) extension_managed_apps_enabled: bool, pub(crate) model_info: ModelInfo, pub(crate) session_telemetry: SessionTelemetry, pub(crate) provider: SharedModelProvider, @@ -195,15 +194,14 @@ impl TurnContext { }) } - pub(crate) fn apps_enabled(&self) -> bool { + pub(crate) fn apps_enabled(&self, mcp_config: &codex_mcp::McpConfig) -> bool { let uses_codex_backend = self .auth_manager .as_deref() .is_some_and(AuthManager::current_auth_uses_codex_backend); - self.config - .features - .apps_enabled_for_auth(uses_codex_backend || self.extension_managed_apps_enabled) - && self.config.orchestrator_mcp_enabled + self.config.features.apps_enabled_for_auth( + uses_codex_backend || codex_mcp::extension_managed_codex_apps_enabled(mcp_config), + ) && self.config.orchestrator_mcp_enabled } pub(crate) async fn with_model( @@ -254,7 +252,6 @@ impl TurnContext { realtime_active: self.realtime_active, config: Arc::new(config), auth_manager: self.auth_manager.clone(), - extension_managed_apps_enabled: self.extension_managed_apps_enabled, model_info: model_info.clone(), session_telemetry: self .session_telemetry @@ -536,7 +533,6 @@ impl Session { realtime_active: false, config: per_turn_config, auth_manager: auth_manager_for_context, - extension_managed_apps_enabled: false, model_info, session_telemetry: session_telemetry_for_context, provider: provider_for_context, @@ -766,10 +762,6 @@ impl Session { sub_id, skills_snapshot, ); - turn_context.extension_managed_apps_enabled = - codex_mcp::extension_managed_codex_apps_enabled( - self.services.latest_mcp_runtime().config(), - ); turn_context.realtime_active = self.conversation.running_state().await.is_some(); if let Some(final_schema) = final_output_json_schema { 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 451af147f7..9fc96dbdd3 100644 --- a/codex-rs/core/src/tools/handlers/request_plugin_install.rs +++ b/codex-rs/core/src/tools/handlers/request_plugin_install.rs @@ -468,7 +468,7 @@ async fn refresh_missing_requested_connectors( connectors::refresh_accessible_connectors_cache_from_mcp_tools( &turn.config, auth, - turn.extension_managed_apps_enabled, + manager.uses_shared_codex_apps_tools_cache_for(auth), &mcp_tools, ); Some(accessible_connectors) diff --git a/codex-rs/rmcp-client/src/rmcp_client.rs b/codex-rs/rmcp-client/src/rmcp_client.rs index c6527990fa..090d6838e1 100644 --- a/codex-rs/rmcp-client/src/rmcp_client.rs +++ b/codex-rs/rmcp-client/src/rmcp_client.rs @@ -126,6 +126,7 @@ enum TransportRecipe { env_http_headers: Option>, store_mode: OAuthCredentialsStoreMode, keyring_backend_kind: AuthKeyringBackendKind, + allow_stored_oauth: bool, http_client: Arc, auth_provider: Option, }, @@ -391,6 +392,7 @@ impl RmcpClient { env_http_headers: Option>, store_mode: OAuthCredentialsStoreMode, keyring_backend_kind: AuthKeyringBackendKind, + allow_stored_oauth: bool, http_client: Arc, auth_provider: Option, ) -> Result { @@ -402,6 +404,7 @@ impl RmcpClient { env_http_headers, store_mode, keyring_backend_kind, + allow_stored_oauth, http_client, auth_provider, }; @@ -781,6 +784,7 @@ impl RmcpClient { env_http_headers, store_mode, keyring_backend_kind, + allow_stored_oauth, http_client, auth_provider, } => { @@ -796,6 +800,7 @@ impl RmcpClient { let initial_oauth_tokens = if bearer_token.is_none() && auth_provider.is_none() && !default_headers.contains_key(AUTHORIZATION) + && *allow_stored_oauth { match load_oauth_tokens(server_name, url, *store_mode, *keyring_backend_kind) { Ok(tokens) => tokens, diff --git a/codex-rs/rmcp-client/tests/streamable_http_oauth_startup.rs b/codex-rs/rmcp-client/tests/streamable_http_oauth_startup.rs index 32f90bc41e..6d1dd0ab12 100644 --- a/codex-rs/rmcp-client/tests/streamable_http_oauth_startup.rs +++ b/codex-rs/rmcp-client/tests/streamable_http_oauth_startup.rs @@ -325,6 +325,7 @@ async fn oauth_startup_child() -> anyhow::Result<()> { /*env_http_headers*/ None, OAuthCredentialsStoreMode::File, AuthKeyringBackendKind::default(), + /*allow_stored_oauth*/ true, Environment::default_for_tests().get_http_client(), /*auth_provider*/ None, ) @@ -365,6 +366,7 @@ async fn expired_unrefreshable_startup_child() -> anyhow::Result<()> { /*env_http_headers*/ None, OAuthCredentialsStoreMode::File, AuthKeyringBackendKind::default(), + /*allow_stored_oauth*/ true, Environment::default_for_tests().get_http_client(), /*auth_provider*/ None, ) diff --git a/codex-rs/rmcp-client/tests/streamable_http_test_support.rs b/codex-rs/rmcp-client/tests/streamable_http_test_support.rs index 87a52f55df..a5c943246d 100644 --- a/codex-rs/rmcp-client/tests/streamable_http_test_support.rs +++ b/codex-rs/rmcp-client/tests/streamable_http_test_support.rs @@ -95,6 +95,7 @@ pub(crate) async fn create_client_with_http_client( /*env_http_headers*/ None, OAuthCredentialsStoreMode::File, AuthKeyringBackendKind::default(), + /*allow_stored_oauth*/ true, http_client, /*auth_provider*/ None, ) @@ -139,6 +140,7 @@ pub(crate) async fn create_remote_client( /*env_http_headers*/ None, OAuthCredentialsStoreMode::File, AuthKeyringBackendKind::default(), + /*allow_stored_oauth*/ true, Arc::new(http_client), /*auth_provider*/ None, )