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.
This commit is contained in:
Ahmed Ibrahim
2026-06-23 00:53:17 -07:00
committed by GitHub
Unverified
parent 83c4934e45
commit 266dcbfe5b
+55 -22
View File
@@ -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>,