mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
## Why Recent MCP runtime overlay changes replace same-name configured server entries with compatibility or extension-provided configs. Those replacement configs default to enabled, so an MCP server explicitly configured with `enabled = false` could be initialized anyway. The connection manager still filters disabled servers correctly, but the configured disabled state was lost before initialization reached that filter. ## What changed - Remember MCP servers that are disabled in the configured view before applying runtime fallbacks and extension overlays. - Restore `enabled = false` for those servers after overlays, while leaving all other overlay fields and `Remove` precedence unchanged. - Add focused extension-backed regression coverage for a disabled `codex_apps` server. ## Testing - `just fmt` - `just test -p codex-mcp-extension` - `just fix -p codex-core` - `just fix -p codex-mcp-extension` The full workspace `just test` suite was not run.
193 lines
6.7 KiB
Rust
193 lines
6.7 KiB
Rust
use std::sync::Arc;
|
|
|
|
use codex_config::McpServerTransportConfig;
|
|
use codex_core::McpManager;
|
|
use codex_core::config::Config;
|
|
use codex_core::config::ConfigBuilder;
|
|
use codex_core_plugins::PluginsManager;
|
|
use codex_extension_api::ExtensionRegistryBuilder;
|
|
use codex_extension_api::McpServerContribution;
|
|
use codex_extension_api::McpServerContributor;
|
|
use codex_login::CodexAuth;
|
|
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
|
|
use pretty_assertions::assert_eq;
|
|
|
|
type TestResult = Result<(), Box<dyn std::error::Error>>;
|
|
|
|
#[tokio::test]
|
|
async fn contributes_hosted_plugin_runtime_without_an_executor() -> TestResult {
|
|
let codex_home = tempfile::tempdir()?;
|
|
let config = ConfigBuilder::default()
|
|
.codex_home(codex_home.path().to_path_buf())
|
|
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
|
.cli_overrides(vec![
|
|
("features.apps".to_string(), true.into()),
|
|
("chatgpt_base_url".to_string(), "https://chatgpt.com".into()),
|
|
])
|
|
.build()
|
|
.await?;
|
|
let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing();
|
|
let manager = installed_manager(&config);
|
|
|
|
let servers = manager.effective_servers(&config, Some(&auth)).await;
|
|
let server = servers
|
|
.get(CODEX_APPS_MCP_SERVER_NAME)
|
|
.and_then(|server| server.configured_config())
|
|
.ok_or("hosted plugin runtime should be contributed as a configured server")?;
|
|
let McpServerTransportConfig::StreamableHttp { url, .. } = &server.transport else {
|
|
panic!("hosted plugin runtime should use streamable HTTP");
|
|
};
|
|
assert_eq!(url, "https://chatgpt.com/backend-api/ps/mcp");
|
|
|
|
Ok(())
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn runtime_overlay_preserves_disabled_server() -> TestResult {
|
|
let codex_home = tempfile::tempdir()?;
|
|
let config = ConfigBuilder::default()
|
|
.codex_home(codex_home.path().to_path_buf())
|
|
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
|
.cli_overrides(vec![
|
|
("features.apps".to_string(), true.into()),
|
|
(
|
|
"mcp_servers.codex_apps.url".to_string(),
|
|
"https://example.com/mcp".into(),
|
|
),
|
|
("mcp_servers.codex_apps.enabled".to_string(), false.into()),
|
|
])
|
|
.build()
|
|
.await?;
|
|
let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing();
|
|
let manager = installed_manager(&config);
|
|
|
|
let servers = manager.effective_servers(&config, Some(&auth)).await;
|
|
let server = servers
|
|
.get(CODEX_APPS_MCP_SERVER_NAME)
|
|
.ok_or("hosted plugin runtime should remain configured")?;
|
|
|
|
assert!(!server.enabled());
|
|
Ok(())
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn legacy_fallback_overwrites_reserved_config_without_an_extension() -> TestResult {
|
|
let codex_home = tempfile::tempdir()?;
|
|
let config = ConfigBuilder::default()
|
|
.codex_home(codex_home.path().to_path_buf())
|
|
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
|
.cli_overrides(vec![
|
|
("features.apps".to_string(), true.into()),
|
|
(
|
|
"mcp_servers.codex_apps.url".to_string(),
|
|
"https://example.com/mcp".into(),
|
|
),
|
|
])
|
|
.build()
|
|
.await?;
|
|
let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing();
|
|
let manager = McpManager::new(Arc::new(PluginsManager::new(
|
|
config.codex_home.to_path_buf(),
|
|
)));
|
|
|
|
let servers = manager.effective_servers(&config, Some(&auth)).await;
|
|
let server = servers
|
|
.get(CODEX_APPS_MCP_SERVER_NAME)
|
|
.and_then(|server| server.configured_config())
|
|
.ok_or("legacy Apps MCP should be present")?;
|
|
let McpServerTransportConfig::StreamableHttp { url, .. } = &server.transport else {
|
|
panic!("legacy Apps MCP should use streamable HTTP");
|
|
};
|
|
assert_eq!(url, "https://chatgpt.com/backend-api/wham/apps");
|
|
|
|
Ok(())
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn extension_can_remove_legacy_fallback_while_apps_are_enabled() -> TestResult {
|
|
let codex_home = tempfile::tempdir()?;
|
|
let config = ConfigBuilder::default()
|
|
.codex_home(codex_home.path().to_path_buf())
|
|
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
|
.cli_overrides(vec![("features.apps".to_string(), true.into())])
|
|
.build()
|
|
.await?;
|
|
let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing();
|
|
let mut builder = ExtensionRegistryBuilder::new();
|
|
builder.mcp_server_contributor(Arc::new(RemoveCodexApps));
|
|
let manager = McpManager::new_with_extensions(
|
|
Arc::new(PluginsManager::new(config.codex_home.to_path_buf())),
|
|
Arc::new(builder.build()),
|
|
);
|
|
|
|
let servers = manager.effective_servers(&config, Some(&auth)).await;
|
|
|
|
assert!(!servers.contains_key(CODEX_APPS_MCP_SERVER_NAME));
|
|
Ok(())
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn hosted_apps_mcp_requires_chatgpt_auth() -> TestResult {
|
|
let codex_home = tempfile::tempdir()?;
|
|
let config = ConfigBuilder::default()
|
|
.codex_home(codex_home.path().to_path_buf())
|
|
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
|
.cli_overrides(vec![("features.apps".to_string(), true.into())])
|
|
.build()
|
|
.await?;
|
|
let auth = CodexAuth::from_api_key("test");
|
|
let manager = installed_manager(&config);
|
|
|
|
let servers = manager.effective_servers(&config, Some(&auth)).await;
|
|
assert!(!servers.contains_key(CODEX_APPS_MCP_SERVER_NAME));
|
|
|
|
Ok(())
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn disabled_apps_remove_reserved_server_config() -> TestResult {
|
|
let codex_home = tempfile::tempdir()?;
|
|
let config = ConfigBuilder::default()
|
|
.codex_home(codex_home.path().to_path_buf())
|
|
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
|
.cli_overrides(vec![
|
|
("features.apps".to_string(), false.into()),
|
|
(
|
|
"mcp_servers.codex_apps.url".to_string(),
|
|
"https://example.com/mcp".into(),
|
|
),
|
|
])
|
|
.build()
|
|
.await?;
|
|
let manager = installed_manager(&config);
|
|
|
|
let servers = manager.runtime_servers(&config).await;
|
|
|
|
assert!(!servers.contains_key(CODEX_APPS_MCP_SERVER_NAME));
|
|
Ok(())
|
|
}
|
|
|
|
fn installed_manager(config: &Config) -> McpManager {
|
|
let mut builder = ExtensionRegistryBuilder::new();
|
|
codex_mcp_extension::install(&mut builder);
|
|
McpManager::new_with_extensions(
|
|
Arc::new(PluginsManager::new(config.codex_home.to_path_buf())),
|
|
Arc::new(builder.build()),
|
|
)
|
|
}
|
|
|
|
struct RemoveCodexApps;
|
|
|
|
impl McpServerContributor<Config> for RemoveCodexApps {
|
|
fn contribute<'a>(
|
|
&'a self,
|
|
_config: &'a Config,
|
|
) -> codex_extension_api::ExtensionFuture<'a, Vec<McpServerContribution>> {
|
|
Box::pin(async move {
|
|
vec![McpServerContribution::Remove {
|
|
name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
|
|
}]
|
|
})
|
|
}
|
|
}
|