Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion codex-rs/codex-mcp/src/mcp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,17 @@ pub fn host_owned_codex_apps_enabled(config: &McpConfig, auth: Option<&CodexAuth
config.apps_enabled && auth.is_some_and(CodexAuth::uses_codex_backend)
}

fn plugin_owned_mcp_server_routes_to_app_surface(config: &McpConfig, server_name: &str) -> bool {
let Some(plugin_id) = config.plugin_ids_by_mcp_server_name.get(server_name) else {
return false;
};

config
.plugin_capability_summaries
.iter()
.any(|plugin| plugin.config_name == *plugin_id && !plugin.app_connector_ids.is_empty())
}

pub fn configured_mcp_servers(config: &McpConfig) -> HashMap<String, McpServerConfig> {
config.configured_mcp_servers.clone()
}
Expand All @@ -241,7 +252,9 @@ pub fn effective_mcp_servers_from_configured(
.into_iter()
.map(|(name, server)| (name, EffectiveMcpServer::configured(server)))
.collect::<HashMap<_, _>>();
if !host_owned_codex_apps_enabled(config, auth) {
if host_owned_codex_apps_enabled(config, auth) {
servers.retain(|name, _| !plugin_owned_mcp_server_routes_to_app_surface(config, name));
} else {
servers.remove(CODEX_APPS_MCP_SERVER_NAME);
}
servers
Expand Down
198 changes: 154 additions & 44 deletions codex-rs/codex-mcp/src/mcp/mod_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,57 @@ fn test_mcp_config(codex_home: PathBuf) -> McpConfig {
}
}

fn streamable_mcp_server_config(url: &str) -> McpServerConfig {
McpServerConfig {
transport: McpServerTransportConfig::StreamableHttp {
url: url.to_string(),
bearer_token_env_var: None,
http_headers: None,
env_http_headers: None,
},
environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(),
enabled: true,
required: false,
supports_parallel_tool_calls: false,
disabled_reason: None,
startup_timeout_sec: None,
tool_timeout_sec: None,
default_tools_approval_mode: None,
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
}
}

fn plugin_summary(
config_name: &str,
app_connector_ids: &[&str],
mcp_server_names: &[&str],
) -> PluginCapabilitySummary {
PluginCapabilitySummary {
config_name: config_name.to_string(),
display_name: config_name.to_string(),
app_connector_ids: app_connector_ids
.iter()
.map(|id| AppConnectorId((*id).to_string()))
.collect(),
mcp_server_names: mcp_server_names
.iter()
.map(|name| (*name).to_string())
.collect(),
..PluginCapabilitySummary::default()
}
}

fn sorted_server_names(servers: &HashMap<String, EffectiveMcpServer>) -> Vec<String> {
let mut names = servers.keys().cloned().collect::<Vec<_>>();
names.sort();
names
}

#[test]
fn qualified_mcp_tool_name_prefix_sanitizes_server_names_without_lowercasing() {
assert_eq!(
Expand Down Expand Up @@ -228,6 +279,107 @@ fn codex_apps_server_config_forwards_configured_product_sku_header() {
}
}

#[test]
fn effective_mcp_servers_routes_plugin_surfaces_by_auth() {
#[derive(Clone, Copy)]
enum AuthKind {
ChatGpt,
ApiKey,
}

struct Case {
name: &'static str,
apps_enabled: bool,
auth: AuthKind,
plugin_owned_mcp: bool,
app_connector_ids: &'static [&'static str],
expected_server_names: &'static [&'static str],
}

let cases = [
Case {
name: "api key keeps plugin mcp and removes codex apps",
apps_enabled: true,
auth: AuthKind::ApiKey,
plugin_owned_mcp: true,
app_connector_ids: &["connector_sample"],
expected_server_names: &["plugin-mcp"],
},
Case {
name: "chatgpt uses app route for dual-surface plugin",
apps_enabled: true,
auth: AuthKind::ChatGpt,
plugin_owned_mcp: true,
app_connector_ids: &["connector_sample"],
expected_server_names: &[CODEX_APPS_MCP_SERVER_NAME],
},
Case {
name: "chatgpt keeps mcp-only plugin mcp",
apps_enabled: true,
auth: AuthKind::ChatGpt,
plugin_owned_mcp: true,
app_connector_ids: &[],
expected_server_names: &[CODEX_APPS_MCP_SERVER_NAME, "plugin-mcp"],
},
Case {
name: "apps disabled keeps plugin mcp and removes codex apps",
apps_enabled: false,
auth: AuthKind::ChatGpt,
plugin_owned_mcp: true,
app_connector_ids: &["connector_sample"],
expected_server_names: &["plugin-mcp"],
},
Case {
name: "chatgpt keeps user-configured mcp even with matching plugin metadata",
apps_enabled: true,
auth: AuthKind::ChatGpt,
plugin_owned_mcp: false,
app_connector_ids: &["connector_sample"],
expected_server_names: &[CODEX_APPS_MCP_SERVER_NAME, "plugin-mcp"],
},
];

for case in cases {
let codex_home = tempfile::tempdir().expect("tempdir");
let mut config = test_mcp_config(codex_home.path().to_path_buf());
config.apps_enabled = case.apps_enabled;
config.configured_mcp_servers.insert(
"plugin-mcp".to_string(),
streamable_mcp_server_config("https://plugin.example/mcp"),
);
config.configured_mcp_servers.insert(
CODEX_APPS_MCP_SERVER_NAME.to_string(),
codex_apps_mcp_server_config(
&config.chatgpt_base_url,
config.apps_mcp_product_sku.as_deref(),
),
);
if case.plugin_owned_mcp {
config
.plugin_ids_by_mcp_server_name
.insert("plugin-mcp".to_string(), "sample@personal".to_string());
}
config.plugin_capability_summaries = vec![plugin_summary(
"sample@personal",
case.app_connector_ids,
&["plugin-mcp"],
)];

let auth = match case.auth {
AuthKind::ChatGpt => CodexAuth::create_dummy_chatgpt_auth_for_testing(),
AuthKind::ApiKey => CodexAuth::from_api_key("test-api-key"),
};
let effective = effective_mcp_servers(&config, Some(&auth));

assert_eq!(
sorted_server_names(&effective),
case.expected_server_names,
"{}",
case.name
);
}
}

#[tokio::test]
async fn effective_mcp_servers_preserve_runtime_servers() {
let codex_home = tempfile::tempdir().expect("tempdir");
Expand All @@ -237,53 +389,11 @@ async fn effective_mcp_servers_preserve_runtime_servers() {

config.configured_mcp_servers.insert(
"sample".to_string(),
McpServerConfig {
transport: McpServerTransportConfig::StreamableHttp {
url: "https://user.example/mcp".to_string(),
bearer_token_env_var: None,
http_headers: None,
env_http_headers: None,
},
environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(),
enabled: true,
required: false,
supports_parallel_tool_calls: false,
disabled_reason: None,
startup_timeout_sec: None,
tool_timeout_sec: None,
default_tools_approval_mode: None,
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
streamable_mcp_server_config("https://user.example/mcp"),
);
config.configured_mcp_servers.insert(
"docs".to_string(),
McpServerConfig {
transport: McpServerTransportConfig::StreamableHttp {
url: "https://docs.example/mcp".to_string(),
bearer_token_env_var: None,
http_headers: None,
env_http_headers: None,
},
environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(),
enabled: true,
required: false,
supports_parallel_tool_calls: false,
disabled_reason: None,
startup_timeout_sec: None,
tool_timeout_sec: None,
default_tools_approval_mode: None,
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
streamable_mcp_server_config("https://docs.example/mcp"),
);
config.configured_mcp_servers.insert(
CODEX_APPS_MCP_SERVER_NAME.to_string(),
Expand Down
Loading
Loading