From 266dcbfe5b1972c027a4c04728e45949ccc1e820 Mon Sep 17 00:00:00 2001
From: Ahmed Ibrahim <aibrahim@openai.com>
Date: Tue, 23 Jun 2026 00:53:17 -0700
Subject: [PATCH] Group Codex Apps client setup (#29583)

## Why

`McpConnectionManager::new` classified the Codex Apps server twice: once
to create its tools cache context and again to select its runtime
authentication provider. Keeping those decisions separate makes it
harder to see that they belong to the same server-specific setup path.

## What changed

- Group Codex Apps cache and authentication setup under one explicit
branch.
- Keep regular MCP server setup in the corresponding `else` branch.
- Limit environment bearer-token inspection to the Codex Apps path where
it affects runtime authentication.
---
 codex-rs/codex-mcp/src/connection_manager.rs | 77 ++++++++++++++------
 1 file changed, 55 insertions(+), 22 deletions(-)

diff --git a/codex-rs/codex-mcp/src/connection_manager.rs b/codex-rs/codex-mcp/src/connection_manager.rs
index be2784307..305716111 100644
--- a/codex-rs/codex-mcp/src/connection_manager.rs
+++ b/codex-rs/codex-mcp/src/connection_manager.rs
@@ -7,6 +7,7 @@
 //! `codex-core`.
 
 use std::collections::HashMap;
+use std::path::Path;
 use std::path::PathBuf;
 use std::sync::Arc;
 use std::sync::atomic::Ordering;
@@ -40,6 +41,7 @@ use anyhow::Context;
 use anyhow::Result;
 use anyhow::anyhow;
 use async_channel::Sender;
+use codex_api::SharedAuthProvider;
 use codex_config::Constrained;
 use codex_config::McpServerTransportConfig;
 use codex_config::types::AuthKeyringBackendKind;
@@ -171,29 +173,16 @@ impl McpConnectionManager {
                 },
             )
             .await;
-            let codex_apps_tools_cache_context = if server_name == CODEX_APPS_MCP_SERVER_NAME {
-                Some(CodexAppsToolsCacheContext {
-                    codex_home: codex_home.clone(),
-                    user_key: codex_apps_tools_cache_key.clone(),
-                })
-            } else {
-                None
-            };
-            let uses_env_bearer_token =
-                server
-                    .configured_config()
-                    .is_some_and(|config| match &config.transport {
-                        McpServerTransportConfig::StreamableHttp {
-                            bearer_token_env_var,
-                            ..
-                        } => bearer_token_env_var.is_some(),
-                        McpServerTransportConfig::Stdio { .. } => false,
-                    });
-            let runtime_auth_provider =
-                if server_name == CODEX_APPS_MCP_SERVER_NAME && !uses_env_bearer_token {
-                    codex_apps_auth_provider.clone()
+            let (codex_apps_tools_cache_context, runtime_auth_provider) =
+                if server_name == CODEX_APPS_MCP_SERVER_NAME {
+                    codex_apps_cache_context_and_auth_provider(
+                        &server,
+                        &codex_home,
+                        &codex_apps_tools_cache_key,
+                        codex_apps_auth_provider.clone(),
+                    )
                 } else {
-                    None
+                    regular_mcp_cache_context_and_auth_provider()
                 };
             let async_managed_client = AsyncManagedClient::new(
                 server_name.clone(),
@@ -856,6 +845,50 @@ impl Drop for McpConnectionManager {
     }
 }
 
+/// Creates the host-owned state used only by the Codex Apps server.
+///
+/// The tools cache is scoped to the authenticated user. Runtime authentication is supplied only
+/// when the server is not already configured to read a bearer token from the environment.
+fn codex_apps_cache_context_and_auth_provider(
+    server: &EffectiveMcpServer,
+    codex_home: &Path,
+    codex_apps_tools_cache_key: &CodexAppsToolsCacheKey,
+    codex_apps_auth_provider: Option<SharedAuthProvider>,
+) -> (
+    Option<CodexAppsToolsCacheContext>,
+    Option<SharedAuthProvider>,
+) {
+    let uses_env_bearer_token =
+        server
+            .configured_config()
+            .is_some_and(|config| match &config.transport {
+                McpServerTransportConfig::StreamableHttp {
+                    bearer_token_env_var,
+                    ..
+                } => bearer_token_env_var.is_some(),
+                McpServerTransportConfig::Stdio { .. } => false,
+            });
+    (
+        Some(CodexAppsToolsCacheContext {
+            codex_home: codex_home.to_path_buf(),
+            user_key: codex_apps_tools_cache_key.clone(),
+        }),
+        if uses_env_bearer_token {
+            None
+        } else {
+            codex_apps_auth_provider
+        },
+    )
+}
+
+/// Keeps regular MCP servers isolated from the host-owned Codex Apps cache and auth provider.
+fn regular_mcp_cache_context_and_auth_provider() -> (
+    Option<CodexAppsToolsCacheContext>,
+    Option<SharedAuthProvider>,
+) {
+    (None, None)
+}
+
 async fn emit_update(
     submit_id: &str,
     tx_event: &Sender<Event>,