[codex] Order codex-mcp items by visibility (#19526)

## Why The visibility cleanup in the base PR reduced what `codex-mcp` exposes, but several files still made reviewers read private support machinery before the public or crate-facing entry points. This ordering pass makes each file easier to scan: exported API first, crate-visible MCP internals next, then private helpers in breadth-first order from the higher-level MCP flows to leaf utilities. ## What Changed - Reordered `codex-mcp` exports so the runtime, configuration, snapshot, auth, and helper surfaces are grouped by visibility and reader importance. - Moved public and crate-visible MCP items ahead of private helpers in the auth, MCP planning/snapshot, connection manager, and tool-name modules. - Kept the change mechanical, with no behavior changes intended. ## Verification - `cargo check -p codex-mcp`
2026-07-01 00:31:56 +08:00 · 2026-04-25 07:17:30 -07:00
parent 706490ab1b
commit 022f81df1f
5 changed files with 415 additions and 408 deletions
@@ -43,6 +43,12 @@ pub struct ResolvedMcpOAuthScopes {
    pub source: McpOAuthScopesSource,
 }

+#[derive(Debug, Clone)]
+pub struct McpAuthStatusEntry {
+    pub config: McpServerConfig,
+    pub auth_status: McpAuthStatus,
+}
+
 pub async fn oauth_login_support(transport: &McpServerTransportConfig) -> McpOAuthLoginSupport {
    let McpServerTransportConfig::StreamableHttp {
        url,
@@ -119,12 +125,6 @@ pub fn should_retry_without_scopes(scopes: &ResolvedMcpOAuthScopes, error: &anyh
        && error.downcast_ref::<OAuthProviderError>().is_some()
 }

-#[derive(Debug, Clone)]
-pub struct McpAuthStatusEntry {
-    pub config: McpServerConfig,
-    pub auth_status: McpAuthStatus,
-}
-
 pub async fn compute_auth_statuses<'a, I>(
    servers: I,
    store_mode: OAuthCredentialsStoreMode,
@@ -1,4 +1,3 @@
-pub(crate) mod auth;
 pub use auth::McpAuthStatusEntry;
 pub use auth::McpOAuthLoginConfig;
 pub use auth::McpOAuthLoginSupport;
@@ -10,6 +9,8 @@ pub use auth::oauth_login_support;
 pub use auth::resolve_oauth_scopes;
 pub use auth::should_retry_without_scopes;

+pub(crate) mod auth;
+
 use std::collections::HashMap;
 use std::env;
 use std::path::PathBuf;
@@ -37,9 +38,9 @@ use crate::mcp_connection_manager::McpConnectionManager;
 use crate::mcp_connection_manager::McpRuntimeEnvironment;
 use crate::mcp_connection_manager::codex_apps_tools_cache_key;

+pub const CODEX_APPS_MCP_SERVER_NAME: &str = "codex_apps";
 const MCP_TOOL_NAME_PREFIX: &str = "mcp";
 const MCP_TOOL_NAME_DELIMITER: &str = "__";
-pub const CODEX_APPS_MCP_SERVER_NAME: &str = "codex_apps";
 const CODEX_CONNECTORS_TOKEN_ENV_VAR: &str = "CODEX_CONNECTORS_TOKEN";

 #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
@@ -55,26 +56,6 @@ impl McpSnapshotDetail {
    }
 }

-/// The Responses API requires tool names to match `^[a-zA-Z0-9_-]+$`.
-/// MCP server/tool names are user-controlled, so sanitize the fully-qualified
-/// name we expose to the model by replacing any disallowed character with `_`.
-pub(crate) fn sanitize_responses_api_tool_name(name: &str) -> String {
-    let mut sanitized = String::with_capacity(name.len());
-    for c in name.chars() {
-        if c.is_ascii_alphanumeric() || c == '_' {
-            sanitized.push(c);
-        } else {
-            sanitized.push('_');
-        }
-    }
-
-    if sanitized.is_empty() {
-        "_".to_string()
-    } else {
-        sanitized
-    }
-}
-
 pub fn qualified_mcp_tool_name_prefix(server_name: &str) -> String {
    sanitize_responses_api_tool_name(&format!(
        "{MCP_TOOL_NAME_PREFIX}{MCP_TOOL_NAME_DELIMITER}{server_name}{MCP_TOOL_NAME_DELIMITER}"
@@ -192,67 +173,6 @@ impl ToolPluginProvenance {
    }
 }

-fn codex_apps_mcp_bearer_token_env_var() -> Option<String> {
-    match env::var(CODEX_CONNECTORS_TOKEN_ENV_VAR) {
-        Ok(value) if !value.trim().is_empty() => Some(CODEX_CONNECTORS_TOKEN_ENV_VAR.to_string()),
-        Ok(_) => None,
-        Err(env::VarError::NotPresent) => None,
-        Err(env::VarError::NotUnicode(_)) => Some(CODEX_CONNECTORS_TOKEN_ENV_VAR.to_string()),
-    }
-}
-
-fn normalize_codex_apps_base_url(base_url: &str) -> String {
-    let mut base_url = base_url.trim_end_matches('/').to_string();
-    if (base_url.starts_with("https://chatgpt.com")
-        || base_url.starts_with("https://chat.openai.com"))
-        && !base_url.contains("/backend-api")
-    {
-        base_url = format!("{base_url}/backend-api");
-    }
-    base_url
-}
-
-fn codex_apps_mcp_url_for_base_url(base_url: &str) -> String {
-    let base_url = normalize_codex_apps_base_url(base_url);
-    if base_url.contains("/backend-api") {
-        format!("{base_url}/wham/apps")
-    } else if base_url.contains("/api/codex") {
-        format!("{base_url}/apps")
-    } else {
-        format!("{base_url}/api/codex/apps")
-    }
-}
-
-pub(crate) fn codex_apps_mcp_url(config: &McpConfig) -> String {
-    codex_apps_mcp_url_for_base_url(&config.chatgpt_base_url)
-}
-
-fn codex_apps_mcp_server_config(config: &McpConfig) -> McpServerConfig {
-    let url = codex_apps_mcp_url(config);
-
-    McpServerConfig {
-        transport: McpServerTransportConfig::StreamableHttp {
-            url,
-            bearer_token_env_var: codex_apps_mcp_bearer_token_env_var(),
-            http_headers: None,
-            env_http_headers: None,
-        },
-        experimental_environment: None,
-        enabled: true,
-        required: false,
-        supports_parallel_tool_calls: false,
-        disabled_reason: None,
-        startup_timeout_sec: Some(Duration::from_secs(30)),
-        tool_timeout_sec: None,
-        default_tools_approval_mode: None,
-        enabled_tools: None,
-        disabled_tools: None,
-        scopes: None,
-        oauth_resource: None,
-        tools: HashMap::new(),
-    }
-}
-
 pub fn with_codex_apps_mcp(
    mut servers: HashMap<String, McpServerConfig>,
    auth: Option<&CodexAuth>,
@@ -395,6 +315,99 @@ pub async fn collect_mcp_server_status_snapshot_with_detail(
    snapshot
 }

+pub async fn collect_mcp_snapshot_from_manager(
+    mcp_connection_manager: &McpConnectionManager,
+    auth_status_entries: HashMap<String, McpAuthStatusEntry>,
+) -> McpListToolsResponseEvent {
+    collect_mcp_snapshot_from_manager_with_detail(
+        mcp_connection_manager,
+        auth_status_entries,
+        McpSnapshotDetail::Full,
+    )
+    .await
+}
+
+pub(crate) fn codex_apps_mcp_url(config: &McpConfig) -> String {
+    codex_apps_mcp_url_for_base_url(&config.chatgpt_base_url)
+}
+
+/// The Responses API requires tool names to match `^[a-zA-Z0-9_-]+$`.
+/// MCP server/tool names are user-controlled, so sanitize the fully-qualified
+/// name we expose to the model by replacing any disallowed character with `_`.
+pub(crate) fn sanitize_responses_api_tool_name(name: &str) -> String {
+    let mut sanitized = String::with_capacity(name.len());
+    for c in name.chars() {
+        if c.is_ascii_alphanumeric() || c == '_' {
+            sanitized.push(c);
+        } else {
+            sanitized.push('_');
+        }
+    }
+
+    if sanitized.is_empty() {
+        "_".to_string()
+    } else {
+        sanitized
+    }
+}
+
+fn codex_apps_mcp_bearer_token_env_var() -> Option<String> {
+    match env::var(CODEX_CONNECTORS_TOKEN_ENV_VAR) {
+        Ok(value) if !value.trim().is_empty() => Some(CODEX_CONNECTORS_TOKEN_ENV_VAR.to_string()),
+        Ok(_) => None,
+        Err(env::VarError::NotPresent) => None,
+        Err(env::VarError::NotUnicode(_)) => Some(CODEX_CONNECTORS_TOKEN_ENV_VAR.to_string()),
+    }
+}
+
+fn normalize_codex_apps_base_url(base_url: &str) -> String {
+    let mut base_url = base_url.trim_end_matches('/').to_string();
+    if (base_url.starts_with("https://chatgpt.com")
+        || base_url.starts_with("https://chat.openai.com"))
+        && !base_url.contains("/backend-api")
+    {
+        base_url = format!("{base_url}/backend-api");
+    }
+    base_url
+}
+
+fn codex_apps_mcp_url_for_base_url(base_url: &str) -> String {
+    let base_url = normalize_codex_apps_base_url(base_url);
+    if base_url.contains("/backend-api") {
+        format!("{base_url}/wham/apps")
+    } else if base_url.contains("/api/codex") {
+        format!("{base_url}/apps")
+    } else {
+        format!("{base_url}/api/codex/apps")
+    }
+}
+
+fn codex_apps_mcp_server_config(config: &McpConfig) -> McpServerConfig {
+    let url = codex_apps_mcp_url(config);
+
+    McpServerConfig {
+        transport: McpServerTransportConfig::StreamableHttp {
+            url,
+            bearer_token_env_var: codex_apps_mcp_bearer_token_env_var(),
+            http_headers: None,
+            env_http_headers: None,
+        },
+        experimental_environment: None,
+        enabled: true,
+        required: false,
+        supports_parallel_tool_calls: false,
+        disabled_reason: None,
+        startup_timeout_sec: Some(Duration::from_secs(30)),
+        tool_timeout_sec: None,
+        default_tools_approval_mode: None,
+        enabled_tools: None,
+        disabled_tools: None,
+        scopes: None,
+        oauth_resource: None,
+        tools: HashMap::new(),
+    }
+}
+
 fn protocol_tool_from_rmcp_tool(name: &str, tool: &rmcp::model::Tool) -> Option<Tool> {
    match serde_json::to_value(tool) {
        Ok(value) => match Tool::from_mcp_value(value) {
@@ -543,18 +556,6 @@ async fn collect_mcp_server_status_snapshot_from_manager(
    }
 }

-pub async fn collect_mcp_snapshot_from_manager(
-    mcp_connection_manager: &McpConnectionManager,
-    auth_status_entries: HashMap<String, McpAuthStatusEntry>,
-) -> McpListToolsResponseEvent {
-    collect_mcp_snapshot_from_manager_with_detail(
-        mcp_connection_manager,
-        auth_status_entries,
-        McpSnapshotDetail::Full,
-    )
-    .await
-}
-
 async fn collect_mcp_snapshot_from_manager_with_detail(
    mcp_connection_manager: &McpConnectionManager,
    auth_status_entries: HashMap<String, McpAuthStatusEntry>,