diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs index 4991f88dc8..ef82161f85 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -114,7 +114,6 @@ pub use marketplace_edit::RemoveMarketplaceConfigOutcome; pub use marketplace_edit::record_user_marketplace; pub use marketplace_edit::remove_user_marketplace; pub use marketplace_edit::remove_user_marketplace_config; -pub use mcp_edit::ConfigEditsBuilder; pub use mcp_edit::load_global_mcp_servers; pub use mcp_requirements::McpServerCommandMatcher; pub use mcp_requirements::McpServerIdentity; diff --git a/codex-rs/config/src/mcp_edit.rs b/codex-rs/config/src/mcp_edit.rs index ef0d8eae48..2ed4352cf7 100644 --- a/codex-rs/config/src/mcp_edit.rs +++ b/codex-rs/config/src/mcp_edit.rs @@ -1,22 +1,11 @@ use std::collections::BTreeMap; -use std::fs; use std::io::ErrorKind; use std::path::Path; -use std::path::PathBuf; -use tokio::task; use toml::Value as TomlValue; -use toml_edit::DocumentMut; -use toml_edit::Item as TomlItem; -use toml_edit::Table as TomlTable; -use toml_edit::value; -use crate::AppToolApproval; use crate::CONFIG_TOML_FILE; -use crate::McpServerAuth; use crate::McpServerConfig; -use crate::McpServerEnvVar; -use crate::McpServerTransportConfig; pub async fn load_global_mcp_servers( codex_home: &Path, @@ -59,242 +48,3 @@ fn ensure_no_inline_bearer_tokens(value: &TomlValue) -> std::io::Result<()> { Ok(()) } - -pub struct ConfigEditsBuilder { - codex_home: PathBuf, - mcp_servers: Option>, -} - -impl ConfigEditsBuilder { - pub fn new(codex_home: &Path) -> Self { - Self { - codex_home: codex_home.to_path_buf(), - mcp_servers: None, - } - } - - pub fn replace_mcp_servers(mut self, servers: &BTreeMap) -> Self { - self.mcp_servers = Some(servers.clone()); - self - } - - pub async fn apply(self) -> std::io::Result<()> { - task::spawn_blocking(move || self.apply_blocking()) - .await - .map_err(|err| { - std::io::Error::other(format!("config persistence task panicked: {err}")) - })? - } - - fn apply_blocking(self) -> std::io::Result<()> { - let config_path = self.codex_home.join(CONFIG_TOML_FILE); - let mut doc = read_or_create_document(&config_path)?; - if let Some(servers) = self.mcp_servers.as_ref() { - replace_mcp_servers(&mut doc, servers); - } - fs::create_dir_all(&self.codex_home)?; - fs::write(config_path, doc.to_string()) - } -} - -fn read_or_create_document(config_path: &Path) -> std::io::Result { - match fs::read_to_string(config_path) { - Ok(raw) => raw - .parse::() - .map_err(|err| std::io::Error::new(ErrorKind::InvalidData, err)), - Err(err) if err.kind() == ErrorKind::NotFound => Ok(DocumentMut::new()), - Err(err) => Err(err), - } -} - -fn replace_mcp_servers(doc: &mut DocumentMut, servers: &BTreeMap) { - let root = doc.as_table_mut(); - if servers.is_empty() { - root.remove("mcp_servers"); - return; - } - - let mut table = TomlTable::new(); - table.set_implicit(true); - for (name, config) in servers { - table.insert(name, serialize_mcp_server(config)); - } - root.insert("mcp_servers", TomlItem::Table(table)); -} - -fn serialize_mcp_server(config: &McpServerConfig) -> TomlItem { - let mut entry = TomlTable::new(); - entry.set_implicit(false); - - match &config.transport { - McpServerTransportConfig::Stdio { - command, - args, - env, - env_vars, - cwd, - } => { - entry["command"] = value(command.clone()); - if !args.is_empty() { - entry["args"] = array_from_strings(args); - } - if let Some(env) = env - && !env.is_empty() - { - entry["env"] = table_from_pairs(env.iter()); - } - if !env_vars.is_empty() { - entry["env_vars"] = array_from_env_vars(env_vars); - } - if let Some(cwd) = cwd { - entry["cwd"] = value(cwd.as_str()); - } - } - McpServerTransportConfig::StreamableHttp { - url, - bearer_token_env_var, - http_headers, - env_http_headers, - } => { - entry["url"] = value(url.clone()); - if let Some(env_var) = bearer_token_env_var { - entry["bearer_token_env_var"] = value(env_var.clone()); - } - if let Some(headers) = http_headers - && !headers.is_empty() - { - entry["http_headers"] = table_from_pairs(headers.iter()); - } - if let Some(headers) = env_http_headers - && !headers.is_empty() - { - entry["env_http_headers"] = table_from_pairs(headers.iter()); - } - } - } - - if matches!(&config.auth, McpServerAuth::ChatGpt) { - entry["auth"] = value("chatgpt"); - } - if !config.enabled { - entry["enabled"] = value(false); - } - if !config.is_local_environment() { - entry["environment_id"] = value(config.environment_id.clone()); - } - if config.required { - entry["required"] = value(true); - } - if config.supports_parallel_tool_calls { - entry["supports_parallel_tool_calls"] = value(true); - } - if let Some(timeout) = config.startup_timeout_sec { - entry["startup_timeout_sec"] = value(timeout.as_secs_f64()); - } - if let Some(timeout) = config.tool_timeout_sec { - entry["tool_timeout_sec"] = value(timeout.as_secs_f64()); - } - if let Some(approval_mode) = config.default_tools_approval_mode { - entry["default_tools_approval_mode"] = value(match approval_mode { - AppToolApproval::Auto => "auto", - AppToolApproval::Prompt => "prompt", - AppToolApproval::Writes => "writes", - AppToolApproval::Approve => "approve", - }); - } - if let Some(enabled_tools) = &config.enabled_tools - && !enabled_tools.is_empty() - { - entry["enabled_tools"] = array_from_strings(enabled_tools); - } - if let Some(disabled_tools) = &config.disabled_tools - && !disabled_tools.is_empty() - { - entry["disabled_tools"] = array_from_strings(disabled_tools); - } - if let Some(scopes) = &config.scopes - && !scopes.is_empty() - { - entry["scopes"] = array_from_strings(scopes); - } - if let Some(oauth) = &config.oauth - && let Some(client_id) = &oauth.client_id - && !client_id.is_empty() - { - let mut oauth_table = TomlTable::new(); - oauth_table.set_implicit(false); - oauth_table["client_id"] = value(client_id.clone()); - entry["oauth"] = TomlItem::Table(oauth_table); - } - if let Some(resource) = &config.oauth_resource - && !resource.is_empty() - { - entry["oauth_resource"] = value(resource.clone()); - } - if !config.tools.is_empty() { - let mut tools = TomlTable::new(); - tools.set_implicit(false); - let mut tool_entries: Vec<_> = config.tools.iter().collect(); - tool_entries.sort_by_key(|(name, _)| *name); - for (name, tool_config) in tool_entries { - let mut tool_entry = TomlTable::new(); - tool_entry.set_implicit(false); - if let Some(approval_mode) = tool_config.approval_mode { - tool_entry["approval_mode"] = value(match approval_mode { - AppToolApproval::Auto => "auto", - AppToolApproval::Prompt => "prompt", - AppToolApproval::Writes => "writes", - AppToolApproval::Approve => "approve", - }); - } - tools.insert(name, TomlItem::Table(tool_entry)); - } - entry.insert("tools", TomlItem::Table(tools)); - } - - TomlItem::Table(entry) -} - -fn array_from_strings(values: &[String]) -> TomlItem { - let mut array = toml_edit::Array::new(); - for value in values { - array.push(value.clone()); - } - TomlItem::Value(array.into()) -} - -fn array_from_env_vars(env_vars: &[McpServerEnvVar]) -> TomlItem { - let mut array = toml_edit::Array::new(); - for env_var in env_vars { - match env_var { - McpServerEnvVar::Name(name) => array.push(name.clone()), - McpServerEnvVar::Config { name, source } => { - let mut table = toml_edit::InlineTable::new(); - table.insert("name", name.clone().into()); - if let Some(source) = source { - table.insert("source", source.clone().into()); - } - array.push(table); - } - } - } - TomlItem::Value(array.into()) -} - -fn table_from_pairs<'a, I>(pairs: I) -> TomlItem -where - I: IntoIterator, -{ - let mut entries: Vec<_> = pairs.into_iter().collect(); - entries.sort_by_key(|(key, _)| *key); - let mut table = TomlTable::new(); - table.set_implicit(false); - for (key, value_str) in entries { - table.insert(key, value(value_str.clone())); - } - TomlItem::Table(table) -} - -#[cfg(test)] -#[path = "mcp_edit_tests.rs"] -mod tests; diff --git a/codex-rs/config/src/mcp_edit_tests.rs b/codex-rs/config/src/mcp_edit_tests.rs deleted file mode 100644 index 10ef23401b..0000000000 --- a/codex-rs/config/src/mcp_edit_tests.rs +++ /dev/null @@ -1,148 +0,0 @@ -use super::*; -use crate::McpServerOAuthConfig; -use crate::McpServerToolConfig; -use pretty_assertions::assert_eq; -use std::collections::HashMap; -use std::time::SystemTime; -use std::time::UNIX_EPOCH; - -#[tokio::test] -async fn replace_mcp_servers_serializes_per_tool_approval_overrides() -> anyhow::Result<()> { - let unique_suffix = SystemTime::now().duration_since(UNIX_EPOCH)?.as_nanos(); - let codex_home = std::env::temp_dir().join(format!( - "codex-config-mcp-edit-test-{}-{unique_suffix}", - std::process::id() - )); - let servers = BTreeMap::from([( - "docs".to_string(), - McpServerConfig { - auth: Default::default(), - transport: McpServerTransportConfig::Stdio { - command: "docs-server".to_string(), - args: Vec::new(), - env: None, - env_vars: Vec::new(), - cwd: None, - }, - environment_id: crate::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), - enabled: true, - required: false, - supports_parallel_tool_calls: true, - disabled_reason: None, - startup_timeout_sec: None, - tool_timeout_sec: None, - default_tools_approval_mode: Some(AppToolApproval::Auto), - enabled_tools: None, - disabled_tools: None, - scopes: None, - oauth: None, - oauth_resource: None, - tools: HashMap::from([ - ( - "search".to_string(), - McpServerToolConfig { - approval_mode: Some(AppToolApproval::Approve), - }, - ), - ( - "read".to_string(), - McpServerToolConfig { - approval_mode: Some(AppToolApproval::Prompt), - }, - ), - ]), - }, - )]); - - ConfigEditsBuilder::new(&codex_home) - .replace_mcp_servers(&servers) - .apply() - .await?; - - let config_path = codex_home.join(CONFIG_TOML_FILE); - let serialized = std::fs::read_to_string(&config_path)?; - assert_eq!( - serialized, - r#"[mcp_servers.docs] -command = "docs-server" -supports_parallel_tool_calls = true -default_tools_approval_mode = "auto" - -[mcp_servers.docs.tools] - -[mcp_servers.docs.tools.read] -approval_mode = "prompt" - -[mcp_servers.docs.tools.search] -approval_mode = "approve" -"# - ); - - let loaded = load_global_mcp_servers(&codex_home).await?; - assert_eq!(loaded, servers); - - std::fs::remove_dir_all(&codex_home)?; - - Ok(()) -} - -#[tokio::test] -async fn replace_mcp_servers_serializes_oauth_client_id() -> anyhow::Result<()> { - let unique_suffix = SystemTime::now().duration_since(UNIX_EPOCH)?.as_nanos(); - let codex_home = std::env::temp_dir().join(format!( - "codex-config-mcp-oauth-edit-test-{}-{unique_suffix}", - std::process::id() - )); - let servers = BTreeMap::from([( - "maas_outlook".to_string(), - McpServerConfig { - auth: Default::default(), - transport: McpServerTransportConfig::StreamableHttp { - url: "https://example.com/mcp".to_string(), - bearer_token_env_var: None, - http_headers: None, - env_http_headers: None, - }, - environment_id: crate::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: Some(McpServerOAuthConfig { - client_id: Some("eci-prd-pub-codex-123".to_string()), - }), - oauth_resource: None, - tools: HashMap::new(), - }, - )]); - - ConfigEditsBuilder::new(&codex_home) - .replace_mcp_servers(&servers) - .apply() - .await?; - - let config_path = codex_home.join(CONFIG_TOML_FILE); - let serialized = std::fs::read_to_string(&config_path)?; - assert_eq!( - serialized, - r#"[mcp_servers.maas_outlook] -url = "https://example.com/mcp" - -[mcp_servers.maas_outlook.oauth] -client_id = "eci-prd-pub-codex-123" -"# - ); - - let loaded = load_global_mcp_servers(&codex_home).await?; - assert_eq!(loaded, servers); - - std::fs::remove_dir_all(&codex_home)?; - - Ok(()) -} diff --git a/codex-rs/core/src/mcp_skill_dependencies.rs b/codex-rs/core/src/mcp_skill_dependencies.rs index 9510edce8e..6f4e84a5d7 100644 --- a/codex-rs/core/src/mcp_skill_dependencies.rs +++ b/codex-rs/core/src/mcp_skill_dependencies.rs @@ -2,7 +2,6 @@ use std::collections::HashMap; use std::collections::HashSet; use std::sync::Arc; -use codex_config::ConfigEditsBuilder; use codex_config::McpServerConfig; use codex_config::McpServerTransportConfig; use codex_config::load_global_mcp_servers; @@ -18,6 +17,7 @@ use tokio_util::sync::CancellationToken; use tracing::warn; use crate::SkillMetadata; +use crate::config::edit::ConfigEditsBuilder; use crate::session::session::Session; use crate::session::turn_context::TurnContext; use crate::skills::model::SkillToolDependency;