diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index a1856af46..1ca468827 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -1012,6 +1012,11 @@ Order of messages: `turnId` is best-effort. When the elicitation is correlated with an active turn, the request includes that turn id; otherwise it is `null`. +For MCP tool approval elicitations, form request `meta` includes +`codex_approval_kind: "mcp_tool_call"` and may include `persist: "session"`, +`persist: "always"`, or `persist: ["session", "always"]` to advertise whether +the client can offer session-scoped and/or persistent approval choices. + ### Permission requests The built-in `request_permissions` tool sends an `item/permissions/requestApproval` JSON-RPC request to the client with the requested permission profile. This v2 payload mirrors the standalone tool's narrower permission shape, so it can request network access and additional filesystem access but does not include the broader `macos` branch used by command-execution `additionalPermissions`. diff --git a/codex-rs/app-server/src/config_api.rs b/codex-rs/app-server/src/config_api.rs index 2be0bb859..7f6acc54d 100644 --- a/codex-rs/app-server/src/config_api.rs +++ b/codex-rs/app-server/src/config_api.rs @@ -130,7 +130,7 @@ impl ConfigApi { .unwrap_or_default() } - async fn load_latest_config( + pub(crate) async fn load_latest_config( &self, fallback_cwd: Option, ) -> Result { diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 9a9a145f6..301ef9992 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -20,6 +20,7 @@ use crate::outgoing_message::OutgoingMessageSender; use crate::outgoing_message::RequestContext; use crate::transport::AppServerTransport; use async_trait::async_trait; +use codex_app_server_protocol::AppListUpdatedNotification; use codex_app_server_protocol::ChatgptAuthTokensRefreshParams; use codex_app_server_protocol::ChatgptAuthTokensRefreshReason; use codex_app_server_protocol::ChatgptAuthTokensRefreshResponse; @@ -53,6 +54,7 @@ use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::ServerRequestPayload; use codex_app_server_protocol::experimental_required_message; use codex_arg0::Arg0DispatchPaths; +use codex_chatgpt::connectors; use codex_core::AnalyticsEventsClient; use codex_core::AuthManager; use codex_core::ThreadManager; @@ -880,13 +882,87 @@ impl MessageProcessor { request_id: ConnectionRequestId, params: ExperimentalFeatureEnablementSetParams, ) { - self.handle_config_mutation_result( - request_id, - self.config_api - .set_experimental_feature_enablement(params) - .await, - ) - .await; + let should_refresh_apps_list = params.enablement.get("apps").copied() == Some(true); + match self + .config_api + .set_experimental_feature_enablement(params) + .await + { + Ok(response) => { + self.codex_message_processor.clear_plugin_related_caches(); + self.codex_message_processor + .maybe_start_plugin_startup_tasks_for_latest_config() + .await; + self.outgoing.send_response(request_id, response).await; + if should_refresh_apps_list { + self.refresh_apps_list_after_experimental_feature_enablement_set() + .await; + } + } + Err(error) => self.outgoing.send_error(request_id, error).await, + } + } + + async fn refresh_apps_list_after_experimental_feature_enablement_set(&self) { + let config = match self + .config_api + .load_latest_config(/*fallback_cwd*/ None) + .await + { + Ok(config) => config, + Err(error) => { + tracing::warn!( + "failed to load config for apps list refresh after experimental feature enablement: {}", + error.message + ); + return; + } + }; + if !config.features.apps_enabled(Some(&self.auth_manager)).await { + return; + } + + let outgoing = Arc::clone(&self.outgoing); + tokio::spawn(async move { + let (all_connectors_result, accessible_connectors_result) = tokio::join!( + connectors::list_all_connectors_with_options(&config, /*force_refetch*/ true), + connectors::list_accessible_connectors_from_mcp_tools_with_options( + &config, /*force_refetch*/ true, + ), + ); + let all_connectors = match all_connectors_result { + Ok(connectors) => connectors, + Err(err) => { + tracing::warn!( + "failed to force-refresh directory apps after experimental feature enablement: {err:#}" + ); + return; + } + }; + let accessible_connectors = match accessible_connectors_result { + Ok(connectors) => connectors, + Err(err) => { + tracing::warn!( + "failed to force-refresh accessible apps after experimental feature enablement: {err:#}" + ); + return; + } + }; + + let data = connectors::with_app_enabled_state( + connectors::merge_connectors_with_accessible( + all_connectors, + accessible_connectors, + /*all_connectors_loaded*/ true, + ), + &config, + ); + outgoing + .send_server_notification(ServerNotification::AppListUpdated( + AppListUpdatedNotification { data }, + )) + .await; + }); } async fn handle_config_mutation_result( diff --git a/codex-rs/app-server/tests/suite/v2/app_list.rs b/codex-rs/app-server/tests/suite/v2/app_list.rs index a19cdad85..23ffa80a6 100644 --- a/codex-rs/app-server/tests/suite/v2/app_list.rs +++ b/codex-rs/app-server/tests/suite/v2/app_list.rs @@ -1,4 +1,5 @@ use std::borrow::Cow; +use std::collections::BTreeMap; use std::collections::HashMap; use std::sync::Arc; use std::sync::Mutex as StdMutex; @@ -27,6 +28,7 @@ use codex_app_server_protocol::AppScreenshot; use codex_app_server_protocol::AppsListParams; use codex_app_server_protocol::AppsListResponse; use codex_app_server_protocol::AuthMode; +use codex_app_server_protocol::ExperimentalFeatureEnablementSetParams; use codex_app_server_protocol::JSONRPCError; use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::RequestId; @@ -1201,6 +1203,108 @@ async fn list_apps_force_refetch_patches_updates_from_cached_snapshots() -> Resu Ok(()) } +#[tokio::test] +async fn experimental_feature_enablement_set_refreshes_apps_list_when_apps_turn_on() -> Result<()> { + let initial_connectors = vec![AppInfo { + id: "alpha".to_string(), + name: "Alpha".to_string(), + description: Some("Alpha v1".to_string()), + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + branding: None, + app_metadata: None, + labels: None, + install_url: None, + is_accessible: false, + is_enabled: true, + plugin_display_names: Vec::new(), + }]; + let (server_url, server_handle, server_control) = start_apps_server_with_delays_and_control( + initial_connectors, + Vec::new(), + Duration::ZERO, + Duration::ZERO, + ) + .await?; + + let codex_home = TempDir::new()?; + write_connectors_config(codex_home.path(), &server_url)?; + write_chatgpt_auth( + codex_home.path(), + ChatGptAuthFixture::new("chatgpt-token") + .account_id("account-123") + .chatgpt_user_id("user-enable-refresh") + .chatgpt_account_id("account-123"), + AuthCredentialsStoreMode::File, + )?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + + let disable_request = mcp + .send_experimental_feature_enablement_set_request(ExperimentalFeatureEnablementSetParams { + enablement: BTreeMap::from([("apps".to_string(), false)]), + }) + .await?; + let _disable_response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(disable_request)), + ) + .await??; + + server_control.set_connectors(vec![AppInfo { + id: "alpha".to_string(), + name: "Alpha".to_string(), + description: Some("Alpha v2".to_string()), + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + branding: None, + app_metadata: None, + labels: None, + install_url: None, + is_accessible: false, + is_enabled: true, + plugin_display_names: Vec::new(), + }]); + server_control.set_tools(vec![connector_tool("alpha", "Alpha App")?]); + + let enable_request = mcp + .send_experimental_feature_enablement_set_request(ExperimentalFeatureEnablementSetParams { + enablement: BTreeMap::from([("apps".to_string(), true)]), + }) + .await?; + let _enable_response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(enable_request)), + ) + .await??; + + let update = read_app_list_updated_notification(&mut mcp).await?; + assert_eq!( + update.data, + vec![AppInfo { + id: "alpha".to_string(), + name: "Alpha".to_string(), + description: Some("Alpha v2".to_string()), + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + branding: None, + app_metadata: None, + labels: None, + install_url: Some("https://chatgpt.com/apps/alpha/alpha".to_string()), + is_accessible: true, + is_enabled: true, + plugin_display_names: Vec::new(), + }] + ); + + server_handle.abort(); + Ok(()) +} + async fn read_app_list_updated_notification( mcp: &mut McpProcess, ) -> Result { diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index 30a911cb6..52707f1da 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -306,6 +306,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }; servers.insert(name.clone(), new_entry); diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 853f0235e..1d1d6dd9a 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -745,6 +745,21 @@ } ] }, + "McpServerToolConfig": { + "additionalProperties": false, + "description": "Per-tool approval settings for a single MCP server tool.", + "properties": { + "approval_mode": { + "allOf": [ + { + "$ref": "#/definitions/AppToolApproval" + } + ], + "description": "Approval mode for this tool." + } + }, + "type": "object" + }, "MemoriesToml": { "additionalProperties": false, "description": "Memories settings loaded from config.toml.", @@ -1246,7 +1261,9 @@ "type": "object" }, "RawMcpServerConfig": { - "additionalProperties": false, + "additionalProperties": { + "$ref": "#/definitions/McpServerToolConfig" + }, "properties": { "args": { "default": null, @@ -1313,6 +1330,11 @@ }, "type": "object" }, + "name": { + "default": null, + "description": "Legacy display-name field accepted for backward compatibility.", + "type": "string" + }, "oauth_resource": { "default": null, "type": "string" @@ -1344,6 +1366,13 @@ "format": "double", "type": "number" }, + "tools": { + "additionalProperties": { + "$ref": "#/definitions/McpServerToolConfig" + }, + "default": null, + "type": "object" + }, "url": { "type": "string" } diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 34c41d44b..84ac6d71b 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -1,10 +1,12 @@ use crate::config::edit::ConfigEdit; use crate::config::edit::ConfigEditsBuilder; use crate::config::edit::apply_blocking; +use crate::config::types::AppToolApproval; use crate::config::types::ApprovalsReviewer; use crate::config::types::BundledSkillsConfig; use crate::config::types::FeedbackConfigToml; use crate::config::types::HistoryPersistence; +use crate::config::types::McpServerToolConfig; use crate::config::types::McpServerTransportConfig; use crate::config::types::MemoriesConfig; use crate::config::types::MemoriesToml; @@ -57,6 +59,7 @@ fn stdio_mcp(command: &str) -> McpServerConfig { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), } } @@ -77,6 +80,7 @@ fn http_mcp(url: &str) -> McpServerConfig { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), } } @@ -1853,6 +1857,7 @@ async fn replace_mcp_servers_round_trips_entries() -> anyhow::Result<()> { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, ); @@ -1958,6 +1963,85 @@ startup_timeout_ms = 2500 Ok(()) } +#[test] +fn mcp_servers_toml_parses_per_tool_approval_overrides() { + let config = toml::from_str::( + r#" +[mcp_servers.docs] +command = "docs-server" +name = "Docs" + +[mcp_servers.docs.tools.search] +approval_mode = "approve" +"#, + ) + .expect("TOML deserialization should succeed"); + let tool = config + .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), + } + ); +} + +#[test] +fn mcp_servers_toml_parses_legacy_flattened_per_tool_approval_overrides() { + let config = toml::from_str::( + r#" +[mcp_servers.docs] +command = "docs-server" + +[mcp_servers.docs.search] +approval_mode = "approve" +"#, + ) + .expect("legacy TOML deserialization should succeed"); + let tool = config + .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), + } + ); +} + +#[test] +fn mcp_servers_toml_parses_tool_approval_override_for_reserved_name() { + let config = toml::from_str::( + r#" +[mcp_servers.docs] +command = "docs-server" + +[mcp_servers.docs.tools.command] +approval_mode = "approve" +"#, + ) + .expect("TOML deserialization should succeed"); + let tool = config + .mcp_servers + .get("docs") + .and_then(|server| server.tools.get("command")) + .expect("docs/command tool config exists"); + + assert_eq!( + tool, + &McpServerToolConfig { + approval_mode: Some(AppToolApproval::Approve), + } + ); +} + #[tokio::test] async fn load_global_mcp_servers_rejects_inline_bearer_token() -> anyhow::Result<()> { let codex_home = TempDir::new()?; @@ -2009,6 +2093,7 @@ async fn replace_mcp_servers_serializes_env_sorted() -> anyhow::Result<()> { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, )]); @@ -2081,6 +2166,7 @@ async fn replace_mcp_servers_serializes_env_vars() -> anyhow::Result<()> { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, )]); @@ -2133,6 +2219,7 @@ async fn replace_mcp_servers_serializes_cwd() -> anyhow::Result<()> { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, )]); @@ -2183,6 +2270,7 @@ async fn replace_mcp_servers_streamable_http_serializes_bearer_token() -> anyhow disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, )]); @@ -2249,6 +2337,7 @@ async fn replace_mcp_servers_streamable_http_serializes_custom_headers() -> anyh disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, )]); apply_blocking( @@ -2327,6 +2416,7 @@ async fn replace_mcp_servers_streamable_http_removes_optional_sections() -> anyh disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, )]); @@ -2358,6 +2448,7 @@ async fn replace_mcp_servers_streamable_http_removes_optional_sections() -> anyh disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, ); apply_blocking( @@ -2424,6 +2515,7 @@ async fn replace_mcp_servers_streamable_http_isolates_headers_between_servers() disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, ), ( @@ -2445,6 +2537,7 @@ async fn replace_mcp_servers_streamable_http_isolates_headers_between_servers() disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, ), ]); @@ -2529,6 +2622,7 @@ async fn replace_mcp_servers_serializes_disabled_flag() -> anyhow::Result<()> { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, )]); @@ -2575,6 +2669,7 @@ async fn replace_mcp_servers_serializes_required_flag() -> anyhow::Result<()> { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, )]); @@ -2621,6 +2716,7 @@ async fn replace_mcp_servers_serializes_tool_filters() -> anyhow::Result<()> { disabled_tools: Some(vec!["blocked".to_string()]), scopes: None, oauth_resource: None, + tools: HashMap::new(), }, )]); @@ -2671,6 +2767,7 @@ async fn replace_mcp_servers_streamable_http_serializes_oauth_resource() -> anyh disabled_tools: None, scopes: None, oauth_resource: Some("https://resource.example.com".to_string()), + tools: HashMap::new(), }, )]); diff --git a/codex-rs/core/src/config/edit.rs b/codex-rs/core/src/config/edit.rs index 3f7e3b117..370c46ce4 100644 --- a/codex-rs/core/src/config/edit.rs +++ b/codex-rs/core/src/config/edit.rs @@ -125,7 +125,9 @@ pub fn model_availability_nux_count_edits(shown_count: &HashMap) -> // TODO(jif) move to a dedicated file mod document_helpers { + use crate::config::types::AppToolApproval; use crate::config::types::McpServerConfig; + use crate::config::types::McpServerToolConfig; use crate::config::types::McpServerTransportConfig; use toml_edit::Array as TomlArray; use toml_edit::InlineTable; @@ -248,10 +250,32 @@ mod document_helpers { { entry["oauth_resource"] = value(resource.clone()); } + if !config.tools.is_empty() { + let mut tools = new_implicit_table(); + let mut tool_entries: Vec<_> = config.tools.iter().collect(); + tool_entries.sort_by(|(left, _), (right, _)| left.cmp(right)); + for (name, tool_config) in tool_entries { + tools.insert(name, serialize_mcp_server_tool(tool_config)); + } + entry.insert("tools", TomlItem::Table(tools)); + } entry } + fn serialize_mcp_server_tool(config: &McpServerToolConfig) -> TomlItem { + let mut entry = TomlTable::new(); + entry.set_implicit(false); + if let Some(approval_mode) = config.approval_mode { + entry["approval_mode"] = value(match approval_mode { + AppToolApproval::Auto => "auto", + AppToolApproval::Prompt => "prompt", + AppToolApproval::Approve => "approve", + }); + } + TomlItem::Table(entry) + } + pub(super) fn serialize_mcp_server(config: &McpServerConfig) -> TomlItem { TomlItem::Table(serialize_mcp_server_table(config)) } diff --git a/codex-rs/core/src/config/edit_tests.rs b/codex-rs/core/src/config/edit_tests.rs index 632716f00..d7386c546 100644 --- a/codex-rs/core/src/config/edit_tests.rs +++ b/codex-rs/core/src/config/edit_tests.rs @@ -1,4 +1,6 @@ use super::*; +use crate::config::types::AppToolApproval; +use crate::config::types::McpServerToolConfig; use crate::config::types::McpServerTransportConfig; use codex_protocol::openai_models::ReasoningEffort; use pretty_assertions::assert_eq; @@ -582,6 +584,7 @@ fn blocking_replace_mcp_servers_round_trips() { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, ); @@ -607,6 +610,7 @@ fn blocking_replace_mcp_servers_round_trips() { disabled_tools: Some(vec!["forbidden".to_string()]), scopes: None, oauth_resource: Some("https://resource.example.com".to_string()), + tools: HashMap::new(), }, ); @@ -643,6 +647,53 @@ B = \"2\" assert_eq!(raw, expected); } +#[test] +fn blocking_replace_mcp_servers_serializes_tool_approval_overrides() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); + + let mut servers = BTreeMap::new(); + servers.insert( + "docs".to_string(), + McpServerConfig { + transport: McpServerTransportConfig::Stdio { + command: "docs-server".to_string(), + args: Vec::new(), + env: None, + env_vars: Vec::new(), + cwd: None, + }, + enabled: true, + required: false, + disabled_reason: None, + startup_timeout_sec: None, + tool_timeout_sec: None, + enabled_tools: None, + disabled_tools: None, + scopes: None, + oauth_resource: None, + tools: HashMap::from([( + "search".to_string(), + McpServerToolConfig { + approval_mode: Some(AppToolApproval::Approve), + }, + )]), + }, + ); + + apply_blocking(codex_home, None, &[ConfigEdit::ReplaceMcpServers(servers)]).expect("persist"); + + let raw = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + let expected = "\ +[mcp_servers.docs] +command = \"docs-server\" + +[mcp_servers.docs.tools.search] +approval_mode = \"approve\" +"; + assert_eq!(raw, expected); +} + #[test] fn blocking_replace_mcp_servers_preserves_inline_comments() { let tmp = tempdir().expect("tmpdir"); @@ -676,6 +727,7 @@ foo = { command = "cmd" } disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, ); @@ -721,6 +773,7 @@ foo = { command = "cmd" } # keep me disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, ); @@ -765,6 +818,7 @@ foo = { command = "cmd", args = ["--flag"] } # keep me disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, ); @@ -810,6 +864,7 @@ foo = { command = "cmd" } disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, ); diff --git a/codex-rs/core/src/config/types.rs b/codex-rs/core/src/config/types.rs index c9f83482a..a69fec343 100644 --- a/codex-rs/core/src/config/types.rs +++ b/codex-rs/core/src/config/types.rs @@ -108,6 +108,10 @@ pub struct McpServerConfig { /// Optional OAuth resource parameter to include during MCP login (RFC 8707). #[serde(default, skip_serializing_if = "Option::is_none")] pub oauth_resource: Option, + + /// Per-tool approval settings keyed by tool name. + #[serde(default, skip_serializing_if = "HashMap::is_empty")] + pub tools: HashMap, } // Raw MCP config shape used for deserialization and JSON Schema generation. @@ -154,6 +158,15 @@ pub(crate) struct RawMcpServerConfig { pub scopes: Option>, #[serde(default)] pub oauth_resource: Option, + /// Legacy display-name field accepted for backward compatibility. + #[serde(default, rename = "name")] + pub _name: Option, + #[serde(default)] + pub tools: Option>, + /// Legacy flattened per-tool approval settings accepted for backward compatibility. + #[serde(default)] + #[serde(flatten)] + pub legacy_tools: HashMap, } impl<'de> Deserialize<'de> for McpServerConfig { @@ -178,6 +191,10 @@ impl<'de> Deserialize<'de> for McpServerConfig { let disabled_tools = raw.disabled_tools.clone(); let scopes = raw.scopes.clone(); let oauth_resource = raw.oauth_resource.clone(); + let mut tools = raw.legacy_tools.clone(); + if let Some(nested_tools) = raw.tools.clone() { + tools.extend(nested_tools); + } fn throw_if_set(transport: &str, field: &str, value: Option<&T>) -> Result<(), E> where @@ -236,6 +253,7 @@ impl<'de> Deserialize<'de> for McpServerConfig { disabled_tools, scopes, oauth_resource, + tools, }) } } @@ -496,6 +514,15 @@ pub enum AppToolApproval { Approve, } +/// Per-tool approval settings for a single MCP server tool. +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Default, JsonSchema)] +#[schemars(deny_unknown_fields)] +pub struct McpServerToolConfig { + /// Approval mode for this tool. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub approval_mode: Option, +} + /// Default settings that apply to all apps. #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Default, JsonSchema)] #[schemars(deny_unknown_fields)] diff --git a/codex-rs/core/src/mcp/mod.rs b/codex-rs/core/src/mcp/mod.rs index 81ee0c7fe..a9d2388f7 100644 --- a/codex-rs/core/src/mcp/mod.rs +++ b/codex-rs/core/src/mcp/mod.rs @@ -176,6 +176,7 @@ fn codex_apps_mcp_server_config(config: &Config, auth: Option<&CodexAuth>) -> Mc disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), } } diff --git a/codex-rs/core/src/mcp/mod_tests.rs b/codex-rs/core/src/mcp/mod_tests.rs index dc9465e10..855e71e1f 100644 --- a/codex-rs/core/src/mcp/mod_tests.rs +++ b/codex-rs/core/src/mcp/mod_tests.rs @@ -235,6 +235,7 @@ async fn effective_mcp_servers_include_plugins_without_overriding_user_config() disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, ); config diff --git a/codex-rs/core/src/mcp/skill_dependencies.rs b/codex-rs/core/src/mcp/skill_dependencies.rs index 489dc2e43..ba4ef2648 100644 --- a/codex-rs/core/src/mcp/skill_dependencies.rs +++ b/codex-rs/core/src/mcp/skill_dependencies.rs @@ -428,6 +428,7 @@ fn mcp_dependency_to_server_config( disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }); } @@ -453,6 +454,7 @@ fn mcp_dependency_to_server_config( disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }); } diff --git a/codex-rs/core/src/mcp/skill_dependencies_tests.rs b/codex-rs/core/src/mcp/skill_dependencies_tests.rs index ebabe16f8..34c6a767f 100644 --- a/codex-rs/core/src/mcp/skill_dependencies_tests.rs +++ b/codex-rs/core/src/mcp/skill_dependencies_tests.rs @@ -48,6 +48,7 @@ fn collect_missing_respects_canonical_installed_key() { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, )]); @@ -97,6 +98,7 @@ fn collect_missing_dedupes_by_canonical_key_but_preserves_original_name() { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, )]); diff --git a/codex-rs/core/src/mcp_connection_manager_tests.rs b/codex-rs/core/src/mcp_connection_manager_tests.rs index c5f7fc4a4..2331d0c1f 100644 --- a/codex-rs/core/src/mcp_connection_manager_tests.rs +++ b/codex-rs/core/src/mcp_connection_manager_tests.rs @@ -542,6 +542,7 @@ fn mcp_init_error_display_prompts_for_github_pat() { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, auth_status: McpAuthStatus::Unsupported, }; @@ -590,6 +591,7 @@ fn mcp_init_error_display_reports_generic_errors() { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, auth_status: McpAuthStatus::Unsupported, }; diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index 04341bdad..9855481b0 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -1,7 +1,10 @@ use std::collections::BTreeMap; +use std::collections::HashMap; +use std::path::PathBuf; use std::time::Duration; use std::time::Instant; +use codex_app_server_protocol::ConfigLayerSource; use codex_app_server_protocol::McpElicitationObjectType; use codex_app_server_protocol::McpElicitationSchema; use codex_app_server_protocol::McpServerElicitationRequest; @@ -12,8 +15,10 @@ use crate::arc_monitor::ArcMonitorOutcome; use crate::arc_monitor::monitor_action; use crate::codex::Session; use crate::codex::TurnContext; +use crate::config::Config; use crate::config::edit::ConfigEdit; use crate::config::edit::ConfigEditsBuilder; +use crate::config::load_global_mcp_servers; use crate::config::types::AppToolApproval; use crate::connectors; use crate::guardian::GuardianApprovalRequest; @@ -46,6 +51,7 @@ use codex_protocol::request_user_input::RequestUserInputResponse; use codex_rmcp_client::ElicitationAction; use codex_rmcp_client::ElicitationResponse; use rmcp::model::ToolAnnotations; +use serde::Deserialize; use serde::Serialize; use std::path::Path; use std::sync::Arc; @@ -104,6 +110,11 @@ pub(crate) async fn handle_mcp_tool_call( } else { connectors::AppToolPolicy::default() }; + let approval_mode = if server == CODEX_APPS_MCP_SERVER_NAME { + app_tool_policy.approval + } else { + custom_mcp_tool_approval_mode(turn_context.as_ref(), &server, &tool_name) + }; if server == CODEX_APPS_MCP_SERVER_NAME && !app_tool_policy.enabled { let result = notify_mcp_tool_call_skip( @@ -151,7 +162,7 @@ pub(crate) async fn handle_mcp_tool_call( &call_id, &invocation, metadata.as_ref(), - app_tool_policy.approval, + approval_mode, ) .await { @@ -491,6 +502,27 @@ pub(crate) struct McpToolApprovalMetadata { const MCP_TOOL_CODEX_APPS_META_KEY: &str = "_codex_apps"; +fn custom_mcp_tool_approval_mode( + turn_context: &TurnContext, + server: &str, + tool_name: &str, +) -> AppToolApproval { + turn_context + .config + .config_layer_stack + .effective_config() + .as_table() + .and_then(|table| table.get("mcp_servers")) + .cloned() + .and_then(|value| { + HashMap::::deserialize(value).ok() + }) + .and_then(|servers| servers.get(server).cloned()) + .and_then(|server| server.tools.get(tool_name).cloned()) + .and_then(|tool| tool.approval_mode) + .unwrap_or_default() +} + fn build_mcp_tool_call_request_meta( turn_context: &TurnContext, server: &str, @@ -560,7 +592,6 @@ const MCP_TOOL_APPROVAL_TOOL_PARAMS_KEY: &str = "tool_params"; const MCP_TOOL_APPROVAL_TOOL_PARAMS_DISPLAY_KEY: &str = "tool_params_display"; const MCP_TOOL_CALL_ARC_MONITOR_CALLSITE_DEFAULT: &str = "mcp_tool_call__default"; const MCP_TOOL_CALL_ARC_MONITOR_CALLSITE_ALWAYS_ALLOW: &str = "mcp_tool_call__always_allow"; -const MCP_TOOL_CALL_ARC_MONITOR_CALLSITE_FULL_ACCESS: &str = "mcp_tool_call__full_access"; pub(crate) fn is_mcp_tool_approval_question_id(question_id: &str) -> bool { question_id @@ -595,11 +626,14 @@ async fn maybe_request_mcp_tool_approval( metadata: Option<&McpToolApprovalMetadata>, approval_mode: AppToolApproval, ) -> Option { + if is_full_access_mode(turn_context) { + return None; + } + let annotations = metadata.and_then(|metadata| metadata.annotations.as_ref()); let approval_required = requires_mcp_tool_approval(annotations); let mut monitor_reason = None; - let auto_approved_by_policy = approval_mode == AppToolApproval::Approve - || (approval_mode == AppToolApproval::Auto && is_full_access_mode(turn_context)); + let auto_approved_by_policy = approval_mode == AppToolApproval::Approve; if auto_approved_by_policy { if !approval_required { @@ -812,12 +846,7 @@ fn persistent_mcp_tool_approval_key( metadata: Option<&McpToolApprovalMetadata>, approval_mode: AppToolApproval, ) -> Option { - if invocation.server != CODEX_APPS_MCP_SERVER_NAME { - return None; - } - session_mcp_tool_approval_key(invocation, metadata, approval_mode) - .filter(|key| key.connector_id.is_some()) } pub(crate) fn build_guardian_mcp_tool_review_request( @@ -865,16 +894,12 @@ fn is_full_access_mode(turn_context: &TurnContext) -> bool { fn mcp_tool_approval_callsite_mode( approval_mode: AppToolApproval, - turn_context: &TurnContext, + _turn_context: &TurnContext, ) -> &'static str { match approval_mode { AppToolApproval::Approve => MCP_TOOL_CALL_ARC_MONITOR_CALLSITE_ALWAYS_ALLOW, AppToolApproval::Auto | AppToolApproval::Prompt => { - if approval_mode == AppToolApproval::Auto && is_full_access_mode(turn_context) { - MCP_TOOL_CALL_ARC_MONITOR_CALLSITE_FULL_ACCESS - } else { - MCP_TOOL_CALL_ARC_MONITOR_CALLSITE_DEFAULT - } + MCP_TOOL_CALL_ARC_MONITOR_CALLSITE_DEFAULT } } } @@ -1356,21 +1381,25 @@ async fn maybe_persist_mcp_tool_approval( turn_context: &TurnContext, key: McpToolApprovalKey, ) { - let Some(connector_id) = key.connector_id.clone() else { - remember_mcp_tool_approval(sess, key).await; - return; - }; let tool_name = key.tool_name.clone(); - if let Err(err) = + let persist_result = if key.server == CODEX_APPS_MCP_SERVER_NAME { + let Some(connector_id) = key.connector_id.clone() else { + remember_mcp_tool_approval(sess, key).await; + return; + }; persist_codex_app_tool_approval(&turn_context.config.codex_home, &connector_id, &tool_name) .await - { + } else { + persist_custom_mcp_tool_approval(&turn_context.config, &key.server, &tool_name).await + }; + + if let Err(err) = persist_result { error!( error = %err, - connector_id, + server = key.server, tool_name, - "failed to persist codex app tool approval" + "failed to persist MCP tool approval" ); remember_mcp_tool_approval(sess, key).await; return; @@ -1400,6 +1429,67 @@ async fn persist_codex_app_tool_approval( .await } +async fn persist_custom_mcp_tool_approval( + config: &Config, + server: &str, + tool_name: &str, +) -> anyhow::Result<()> { + let config_folder = if let Some(project_config_folder) = + project_mcp_tool_approval_config_folder(config, server) + { + project_config_folder + } else { + let servers = load_global_mcp_servers(&config.codex_home).await?; + if !servers.contains_key(server) { + anyhow::bail!("MCP server `{server}` is not configured in config.toml"); + } + config.codex_home.clone() + }; + + ConfigEditsBuilder::new(&config_folder) + .with_edits([ConfigEdit::SetPath { + segments: vec![ + "mcp_servers".to_string(), + server.to_string(), + "tools".to_string(), + tool_name.to_string(), + "approval_mode".to_string(), + ], + value: value("approve"), + }]) + .apply() + .await +} + +fn project_mcp_tool_approval_config_folder(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() + .map(|folder| folder.as_path().to_path_buf()) + } else { + None + } + }) +} + 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 edd698c13..cf3c761fa 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -1,11 +1,14 @@ use super::*; use crate::codex::make_session_and_context; use crate::config::ApprovalsReviewer; +use crate::config::ConfigBuilder; use crate::config::ConfigToml; use crate::config::types::AppConfig; use crate::config::types::AppToolConfig; use crate::config::types::AppToolsConfig; use crate::config::types::AppsConfigToml; +use crate::config::types::McpServerConfig; +use crate::config::types::McpServerToolConfig; use codex_config::CONFIG_TOML_FILE; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; @@ -378,13 +381,13 @@ fn codex_apps_tool_question_without_elicitation_omits_always_allow() { } #[test] -fn custom_mcp_tool_question_offers_session_remember_without_always_allow() { +fn custom_mcp_tool_question_offers_session_remember_and_always_allow() { let question = build_mcp_tool_approval_question( "q".to_string(), "custom_server", "run_action", None, - prompt_options(true, false), + prompt_options(true, true), None, ); @@ -398,13 +401,14 @@ fn custom_mcp_tool_question_offers_session_remember_without_always_allow() { vec![ MCP_TOOL_APPROVAL_ACCEPT.to_string(), MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION.to_string(), + MCP_TOOL_APPROVAL_ACCEPT_AND_REMEMBER.to_string(), MCP_TOOL_APPROVAL_CANCEL.to_string(), ] ); } #[test] -fn custom_servers_keep_session_remember_without_persistent_approval() { +fn custom_servers_support_session_and_persistent_approval() { let invocation = McpInvocation { server: "custom_server".to_string(), tool: "run_action".to_string(), @@ -418,11 +422,11 @@ fn custom_servers_keep_session_remember_without_persistent_approval() { assert_eq!( session_mcp_tool_approval_key(&invocation, None, AppToolApproval::Auto), - Some(expected) + Some(expected.clone()) ); assert_eq!( persistent_mcp_tool_approval_key(&invocation, None, AppToolApproval::Auto), - None + Some(expected) ); } @@ -612,7 +616,7 @@ fn approval_elicitation_meta_marks_tool_approvals() { } #[test] -fn approval_elicitation_meta_keeps_session_persist_behavior_for_custom_servers() { +fn approval_elicitation_meta_merges_session_and_always_persist_for_custom_servers() { assert_eq!( build_mcp_tool_approval_elicitation_meta( "custom_server", @@ -625,11 +629,14 @@ fn approval_elicitation_meta_keeps_session_persist_behavior_for_custom_servers() )), Some(&serde_json::json!({"id": 1})), None, - prompt_options(true, false), + prompt_options(true, true), ), Some(serde_json::json!({ MCP_TOOL_APPROVAL_KIND_KEY: MCP_TOOL_APPROVAL_KIND_MCP_TOOL_CALL, - MCP_TOOL_APPROVAL_PERSIST_KEY: MCP_TOOL_APPROVAL_PERSIST_SESSION, + MCP_TOOL_APPROVAL_PERSIST_KEY: [ + MCP_TOOL_APPROVAL_PERSIST_SESSION, + MCP_TOOL_APPROVAL_PERSIST_ALWAYS, + ], MCP_TOOL_APPROVAL_TOOL_TITLE_KEY: "Run Action", MCP_TOOL_APPROVAL_TOOL_DESCRIPTION_KEY: "Runs the selected action.", MCP_TOOL_APPROVAL_TOOL_PARAMS_KEY: { @@ -843,8 +850,8 @@ fn approval_elicitation_meta_merges_session_and_always_persist_with_connector_so } #[tokio::test] -async fn approval_callsite_mode_distinguishes_default_always_allow_and_full_access() { - let (_session, mut turn_context) = make_session_and_context().await; +async fn approval_callsite_mode_distinguishes_default_and_always_allow() { + let (_session, turn_context) = make_session_and_context().await; assert_eq!( mcp_tool_approval_callsite_mode(AppToolApproval::Auto, &turn_context), @@ -858,20 +865,6 @@ async fn approval_callsite_mode_distinguishes_default_always_allow_and_full_acce mcp_tool_approval_callsite_mode(AppToolApproval::Approve, &turn_context), "mcp_tool_call__always_allow" ); - - turn_context - .approval_policy - .set(AskForApproval::Never) - .expect("test setup should allow updating approval policy"); - turn_context - .sandbox_policy - .set(SandboxPolicy::DangerFullAccess) - .expect("test setup should allow updating sandbox policy"); - - assert_eq!( - mcp_tool_approval_callsite_mode(AppToolApproval::Auto, &turn_context), - "mcp_tool_call__full_access" - ); } #[test] @@ -992,6 +985,41 @@ async fn persist_codex_app_tool_approval_writes_tool_override() { assert!(contents.contains("[apps.calendar.tools.\"calendar/list_events\"]")); } +#[tokio::test] +async fn persist_custom_mcp_tool_approval_writes_tool_override() { + let tmp = tempdir().expect("tempdir"); + std::fs::write( + tmp.path().join(CONFIG_TOML_FILE), + "[mcp_servers.docs]\ncommand = \"docs-server\"\n", + ) + .expect("seed config"); + let config = ConfigBuilder::default() + .codex_home(tmp.path().to_path_buf()) + .build() + .await + .expect("load config"); + + persist_custom_mcp_tool_approval(&config, "docs", "search") + .await + .expect("persist approval"); + + let contents = std::fs::read_to_string(tmp.path().join(CONFIG_TOML_FILE)).expect("read config"); + let parsed: ConfigToml = toml::from_str(&contents).expect("parse 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]")); +} + #[tokio::test] async fn maybe_persist_mcp_tool_approval_reloads_session_config() { let (session, turn_context) = make_session_and_context().await; @@ -1031,6 +1059,104 @@ async fn maybe_persist_mcp_tool_approval_reloads_session_config() { assert_eq!(mcp_tool_approval_is_remembered(&session, &key).await, true); } +#[tokio::test] +async fn maybe_persist_mcp_tool_approval_reloads_session_config_for_custom_server() { + let (session, 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 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 config = session.get_config().await; + let mcp_servers_toml = config + .config_layer_stack + .effective_config() + .as_table() + .and_then(|table| table.get("mcp_servers")) + .cloned() + .expect("mcp_servers table"); + let mcp_servers = HashMap::::deserialize(mcp_servers_toml) + .expect("deserialize MCP servers"); + let tool = 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_eq!(mcp_tool_approval_is_remembered(&session, &key).await, true); +} + +#[tokio::test] +async fn maybe_persist_mcp_tool_approval_writes_project_config_for_project_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"); + 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) + .fallback_cwd(Some(project_dir.path().to_path_buf())) + .build() + .await + .expect("load project config"); + turn_context.cwd = config.cwd.clone(); + 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_TOML_FILE)) + .expect("read project config"); + let parsed: ConfigToml = toml::from_str(&contents).expect("parse project 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 approve_mode_skips_when_annotations_do_not_require_approval() { let (session, turn_context) = make_session_and_context().await; @@ -1133,6 +1259,75 @@ async fn approve_mode_blocks_when_arc_returns_interrupt_for_model() { ); } +#[tokio::test] +async fn custom_approve_mode_blocks_when_arc_returns_interrupt_for_model() { + use wiremock::Mock; + use wiremock::MockServer; + use wiremock::ResponseTemplate; + use wiremock::matchers::method; + use wiremock::matchers::path; + + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/codex/safety/arc")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "outcome": "steer-model", + "short_reason": "needs approval", + "rationale": "high-risk action", + "risk_score": 96, + "risk_level": "critical", + "evidence": [{ + "message": "dangerous_tool", + "why": "high-risk action", + }], + }))) + .expect(1) + .mount(&server) + .await; + + let (session, mut turn_context) = make_session_and_context().await; + turn_context.auth_manager = Some(crate::test_support::auth_manager_from_auth( + crate::CodexAuth::create_dummy_chatgpt_auth_for_testing(), + )); + let mut config = (*turn_context.config).clone(); + config.chatgpt_base_url = server.uri(); + turn_context.config = Arc::new(config); + + let session = Arc::new(session); + let turn_context = Arc::new(turn_context); + let invocation = McpInvocation { + server: "docs".to_string(), + tool: "dangerous_tool".to_string(), + arguments: Some(serde_json::json!({ "id": 1 })), + }; + let metadata = McpToolApprovalMetadata { + annotations: Some(annotations(Some(false), Some(true), Some(true))), + connector_id: None, + connector_name: None, + connector_description: None, + tool_title: Some("Dangerous Tool".to_string()), + tool_description: Some("Performs a risky action.".to_string()), + codex_apps_meta: None, + }; + + let decision = maybe_request_mcp_tool_approval( + &session, + &turn_context, + "call-2-custom", + &invocation, + Some(&metadata), + AppToolApproval::Approve, + ) + .await; + + assert_eq!( + decision, + Some(McpToolApprovalDecision::BlockedBySafetyMonitor( + "Tool call was cancelled because of safety risks: high-risk action".to_string(), + )) + ); +} + #[tokio::test] async fn approve_mode_blocks_when_arc_returns_interrupt_without_annotations() { use wiremock::Mock; @@ -1203,7 +1398,7 @@ async fn approve_mode_blocks_when_arc_returns_interrupt_without_annotations() { } #[tokio::test] -async fn full_access_auto_mode_blocks_when_arc_returns_interrupt_for_model() { +async fn full_access_mode_skips_arc_monitor_for_all_approval_modes() { use wiremock::Mock; use wiremock::MockServer; use wiremock::ResponseTemplate; @@ -1224,7 +1419,7 @@ async fn full_access_auto_mode_blocks_when_arc_returns_interrupt_for_model() { "why": "high-risk action", }], }))) - .expect(1) + .expect(0) .mount(&server) .await; @@ -1261,22 +1456,23 @@ async fn full_access_auto_mode_blocks_when_arc_returns_interrupt_for_model() { codex_apps_meta: None, }; - let decision = maybe_request_mcp_tool_approval( - &session, - &turn_context, - "call-2", - &invocation, - Some(&metadata), + for approval_mode in [ AppToolApproval::Auto, - ) - .await; + AppToolApproval::Prompt, + AppToolApproval::Approve, + ] { + let decision = maybe_request_mcp_tool_approval( + &session, + &turn_context, + "call-2", + &invocation, + Some(&metadata), + approval_mode, + ) + .await; - assert_eq!( - decision, - Some(McpToolApprovalDecision::BlockedBySafetyMonitor( - "Tool call was cancelled because of safety risks: high-risk action".to_string(), - )) - ); + assert_eq!(decision, None); + } } #[tokio::test] diff --git a/codex-rs/core/src/plugins/manager_tests.rs b/codex-rs/core/src/plugins/manager_tests.rs index af33f9017..b845e085f 100644 --- a/codex-rs/core/src/plugins/manager_tests.rs +++ b/codex-rs/core/src/plugins/manager_tests.rs @@ -164,6 +164,7 @@ fn load_plugins_loads_default_skills_and_mcp_servers() { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, )]), apps: vec![AppConnectorId("connector_example".to_string())], @@ -483,6 +484,7 @@ fn load_plugins_uses_manifest_configured_component_paths() { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, )]) ); @@ -586,6 +588,7 @@ fn load_plugins_ignores_manifest_component_paths_without_dot_slash() { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, )]) ); @@ -735,6 +738,7 @@ fn capability_index_filters_inactive_and_zero_capability_plugins() { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }; let plugin = |config_name: &str, dir_name: &str, manifest_name: &str| LoadedPlugin { config_name: config_name.to_string(), diff --git a/codex-rs/core/tests/suite/code_mode.rs b/codex-rs/core/tests/suite/code_mode.rs index fa8229ecc..ae38261f1 100644 --- a/codex-rs/core/tests/suite/code_mode.rs +++ b/codex-rs/core/tests/suite/code_mode.rs @@ -207,6 +207,7 @@ async fn run_code_mode_turn_with_rmcp( disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, ); config diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index 6cbf9521b..431895e2e 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -108,6 +108,7 @@ async fn stdio_server_round_trip() -> anyhow::Result<()> { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, ); config @@ -253,6 +254,7 @@ async fn stdio_image_responses_round_trip() -> anyhow::Result<()> { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, ); config @@ -476,6 +478,7 @@ async fn stdio_image_responses_are_sanitized_for_text_only_model() -> anyhow::Re disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, ); config @@ -597,6 +600,7 @@ async fn stdio_server_propagates_whitelisted_env_vars() -> anyhow::Result<()> { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, ); config @@ -759,6 +763,7 @@ async fn streamable_http_tool_call_round_trip() -> anyhow::Result<()> { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, ); config @@ -981,6 +986,7 @@ async fn streamable_http_with_oauth_round_trip_impl() -> anyhow::Result<()> { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, ); config diff --git a/codex-rs/core/tests/suite/sqlite_state.rs b/codex-rs/core/tests/suite/sqlite_state.rs index 2df92dbf9..d955f342a 100644 --- a/codex-rs/core/tests/suite/sqlite_state.rs +++ b/codex-rs/core/tests/suite/sqlite_state.rs @@ -375,6 +375,7 @@ async fn mcp_call_marks_thread_memory_mode_polluted_when_configured() -> Result< disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, ); config diff --git a/codex-rs/core/tests/suite/truncation.rs b/codex-rs/core/tests/suite/truncation.rs index f10567de2..c3f019dd3 100644 --- a/codex-rs/core/tests/suite/truncation.rs +++ b/codex-rs/core/tests/suite/truncation.rs @@ -371,6 +371,7 @@ async fn mcp_tool_call_output_exceeds_limit_truncated_for_model() -> Result<()> disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, ); config @@ -466,6 +467,7 @@ async fn mcp_image_output_preserves_image_and_no_text_summary() -> Result<()> { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, ); config @@ -734,6 +736,7 @@ async fn mcp_tool_call_output_not_truncated_with_custom_limit() -> Result<()> { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, ); config diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 157e5feec..2dee1e846 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -2914,6 +2914,7 @@ mod tests { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }; let mut servers = config.mcp_servers.get().clone(); servers.insert("docs".to_string(), stdio_config); @@ -2938,6 +2939,7 @@ mod tests { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }; servers.insert("http".to_string(), http_config); config diff --git a/codex-rs/tui_app_server/src/history_cell.rs b/codex-rs/tui_app_server/src/history_cell.rs index 445d68446..7e37a6bf9 100644 --- a/codex-rs/tui_app_server/src/history_cell.rs +++ b/codex-rs/tui_app_server/src/history_cell.rs @@ -3143,6 +3143,7 @@ mod tests { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }; let mut servers = config.mcp_servers.get().clone(); servers.insert("docs".to_string(), stdio_config); @@ -3167,6 +3168,7 @@ mod tests { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }; servers.insert("http".to_string(), http_config); config @@ -3237,6 +3239,7 @@ mod tests { disabled_tools: None, scopes: None, oauth_resource: None, + tools: HashMap::new(), }, )]); config diff --git a/docs/config.md b/docs/config.md index d03fb9843..71f3548de 100644 --- a/docs/config.md +++ b/docs/config.md @@ -12,6 +12,16 @@ Codex can connect to MCP servers configured in `~/.codex/config.toml`. See the c - https://developers.openai.com/codex/config-reference +## MCP tool approvals + +Codex stores per-tool approval overrides for custom MCP servers under +`mcp_servers` in `~/.codex/config.toml`: + +```toml +[mcp_servers.docs.tools.search] +approval_mode = "approve" +``` + ## Apps (Connectors) Use `$` in the composer to insert a ChatGPT connector; the popover lists accessible