diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index 902ede574e..1ef25716f1 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -2104,8 +2104,10 @@ fn custom_mcp_tool_approval_config_builder( config: &Config, server: &str, ) -> anyhow::Result> { - if let Some(project_config_folder) = project_mcp_tool_approval_config_folder(config, server) { - return Ok(Some(ConfigEditsBuilder::new(&project_config_folder))); + if let Some(config_path) = mcp_tool_approval_config_path(config, server) { + return Ok(Some(ConfigEditsBuilder::for_config_path( + config_path.as_path(), + ))); } Ok(user_mcp_server_is_configured(config, server)? @@ -2147,35 +2149,35 @@ fn user_mcp_server_is_configured(config: &Config, server: &str) -> anyhow::Resul Ok(servers.contains_key(server)) } -fn project_mcp_tool_approval_config_folder( - config: &Config, - server: &str, -) -> Option { +fn mcp_tool_approval_config_path(config: &Config, server: &str) -> Option { config .config_layer_stack .layers_high_to_low() .into_iter() .find_map(|layer| { - if !matches!(layer.name, ConfigLayerSource::Project { .. }) { - return None; - } - - let servers = layer - .config - .as_table() - .and_then(|table| table.get("mcp_servers")) - .cloned() - .and_then(|value| { - HashMap::::deserialize(value).ok() - })?; - if servers.contains_key(server) { - layer.config_folder() - } else { - None - } + let config_path = match &layer.name { + ConfigLayerSource::Project { dot_codex_folder } => { + dot_codex_folder.join(codex_config::CONFIG_TOML_FILE) + } + ConfigLayerSource::ProjectOverride { dot_codex_folder } => { + dot_codex_folder.join(codex_config::CONFIG_OVERRIDE_TOML_FILE) + } + ConfigLayerSource::UserOverride { file } => file.clone(), + _ => return None, + }; + layer_declares_mcp_server(layer, server).then_some(config_path) }) } +fn layer_declares_mcp_server(layer: &codex_config::ConfigLayerEntry, server: &str) -> bool { + layer + .config + .as_table() + .and_then(|table| table.get("mcp_servers")) + .and_then(toml::Value::as_table) + .is_some_and(|servers| servers.contains_key(server)) +} + fn requires_mcp_tool_approval(annotations: Option<&ToolAnnotations>) -> bool { let destructive_hint = annotations.and_then(|annotations| annotations.destructive_hint); if destructive_hint == Some(true) { diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index 50a6bb4d45..8611a63143 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -6,6 +6,7 @@ use crate::session::tests::make_session_and_context_with_rx; use crate::state::ActiveTurn; use crate::test_support::models_manager_with_provider; use crate::turn_metadata::McpTurnMetadataContext; +use codex_config::CONFIG_OVERRIDE_TOML_FILE; use codex_config::CONFIG_TOML_FILE; use codex_config::config_toml::ConfigToml; use codex_config::types::AppConfig; @@ -2301,6 +2302,168 @@ async fn maybe_persist_mcp_tool_approval_writes_project_config_for_project_serve assert_eq!(mcp_tool_approval_is_remembered(&session, &key).await, true); } +#[tokio::test] +async fn maybe_persist_mcp_tool_approval_writes_project_override_for_override_server() { + let (session, mut turn_context) = make_session_and_context().await; + let codex_home = session.codex_home().await; + let project_dir = tempdir().expect("tempdir"); + std::fs::write(project_dir.path().join(".git"), "gitdir: nowhere").expect("seed git marker"); + let project_codex_dir = project_dir.path().join(".codex"); + std::fs::create_dir_all(&project_codex_dir).expect("create project .codex dir"); + std::fs::write( + project_codex_dir.join(CONFIG_OVERRIDE_TOML_FILE), + "[mcp_servers.docs]\ncommand = \"docs-server\"\n", + ) + .expect("seed project override config"); + ConfigEditsBuilder::new(&codex_home) + .set_project_trust_level( + project_dir.path(), + codex_protocol::config_types::TrustLevel::Trusted, + ) + .apply() + .await + .expect("trust project"); + let config = ConfigBuilder::default() + .codex_home(codex_home.to_path_buf()) + .fallback_cwd(Some(project_dir.path().to_path_buf())) + .build() + .await + .expect("load project override config"); + turn_context.config = Arc::new(config); + let key = McpToolApprovalKey { + server: "docs".to_string(), + connector_id: None, + tool_name: "search".to_string(), + }; + + maybe_persist_mcp_tool_approval(&session, &turn_context, key.clone()).await; + + let contents = std::fs::read_to_string(project_codex_dir.join(CONFIG_OVERRIDE_TOML_FILE)) + .expect("read project override config"); + let parsed: ConfigToml = toml::from_str(&contents).expect("parse project override config"); + let tool = parsed + .mcp_servers + .get("docs") + .and_then(|server| server.tools.get("search")) + .expect("docs/search tool config exists"); + assert_eq!( + tool, + &McpServerToolConfig { + approval_mode: Some(AppToolApproval::Approve), + } + ); + assert!(contents.contains("[mcp_servers.docs.tools.search]")); + assert_eq!(mcp_tool_approval_is_remembered(&session, &key).await, true); +} + +#[tokio::test] +async fn maybe_persist_mcp_tool_approval_writes_user_override_for_override_server() { + let (session, mut turn_context) = make_session_and_context().await; + let codex_home = session.codex_home().await; + std::fs::create_dir_all(&codex_home).expect("create codex home"); + std::fs::write( + codex_home.join(CONFIG_TOML_FILE), + "[mcp_servers.docs]\ncommand = \"docs-server\"\n", + ) + .expect("seed user config"); + let override_path = codex_home.join(CONFIG_OVERRIDE_TOML_FILE); + std::fs::write( + &override_path, + "[mcp_servers.docs.tools.search]\napproval_mode = \"prompt\"\n", + ) + .expect("seed user override config"); + let config = ConfigBuilder::without_managed_config_for_tests() + .codex_home(codex_home.to_path_buf()) + .build() + .await + .expect("load user override config"); + turn_context.config = Arc::new(config); + let key = McpToolApprovalKey { + server: "docs".to_string(), + connector_id: None, + tool_name: "search".to_string(), + }; + + maybe_persist_mcp_tool_approval(&session, &turn_context, key.clone()).await; + + let contents = std::fs::read_to_string(&override_path).expect("read user override config"); + let parsed: toml::Value = toml::from_str(&contents).expect("parse user override config"); + let approval_mode = parsed + .get("mcp_servers") + .and_then(toml::Value::as_table) + .and_then(|servers| servers.get("docs")) + .and_then(toml::Value::as_table) + .and_then(|server| server.get("tools")) + .and_then(toml::Value::as_table) + .and_then(|tools| tools.get("search")) + .and_then(toml::Value::as_table) + .and_then(|tool| tool.get("approval_mode")) + .and_then(toml::Value::as_str); + + assert_eq!(approval_mode, Some("approve")); + assert_eq!(mcp_tool_approval_is_remembered(&session, &key).await, true); +} + +#[tokio::test] +async fn maybe_persist_mcp_tool_approval_writes_partial_project_override_for_override_server() { + let (session, mut turn_context) = make_session_and_context().await; + let codex_home = session.codex_home().await; + let project_dir = tempdir().expect("tempdir"); + std::fs::write(project_dir.path().join(".git"), "gitdir: nowhere").expect("seed git marker"); + let project_codex_dir = project_dir.path().join(".codex"); + std::fs::create_dir_all(&project_codex_dir).expect("create project .codex dir"); + std::fs::write( + project_codex_dir.join(CONFIG_TOML_FILE), + "[mcp_servers.docs]\ncommand = \"docs-server\"\n", + ) + .expect("seed project config"); + let override_path = project_codex_dir.join(CONFIG_OVERRIDE_TOML_FILE); + std::fs::write( + &override_path, + "[mcp_servers.docs.tools.search]\napproval_mode = \"prompt\"\n", + ) + .expect("seed project override config"); + ConfigEditsBuilder::new(&codex_home) + .set_project_trust_level( + project_dir.path(), + codex_protocol::config_types::TrustLevel::Trusted, + ) + .apply() + .await + .expect("trust project"); + let config = ConfigBuilder::default() + .codex_home(codex_home.to_path_buf()) + .fallback_cwd(Some(project_dir.path().to_path_buf())) + .build() + .await + .expect("load project override config"); + turn_context.config = Arc::new(config); + let key = McpToolApprovalKey { + server: "docs".to_string(), + connector_id: None, + tool_name: "search".to_string(), + }; + + maybe_persist_mcp_tool_approval(&session, &turn_context, key.clone()).await; + + let contents = std::fs::read_to_string(&override_path).expect("read project override config"); + let parsed: toml::Value = toml::from_str(&contents).expect("parse project override config"); + let approval_mode = parsed + .get("mcp_servers") + .and_then(toml::Value::as_table) + .and_then(|servers| servers.get("docs")) + .and_then(toml::Value::as_table) + .and_then(|server| server.get("tools")) + .and_then(toml::Value::as_table) + .and_then(|tools| tools.get("search")) + .and_then(toml::Value::as_table) + .and_then(|tool| tool.get("approval_mode")) + .and_then(toml::Value::as_str); + + assert_eq!(approval_mode, Some("approve")); + assert_eq!(mcp_tool_approval_is_remembered(&session, &key).await, true); +} + #[tokio::test] async fn approve_mode_skips_when_annotations_do_not_require_approval() { let (session, turn_context) = make_session_and_context().await;