diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 92549d74c8e..187289527a6 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -5951,6 +5951,16 @@ }, "AppConfig": { "properties": { + "approvals_reviewer": { + "anyOf": [ + { + "$ref": "#/definitions/v2/ApprovalsReviewer" + }, + { + "type": "null" + } + ] + }, "default_tools_approval_mode": { "anyOf": [ { diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index d266a3e9773..17a9e390866 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -323,6 +323,16 @@ }, "AppConfig": { "properties": { + "approvals_reviewer": { + "anyOf": [ + { + "$ref": "#/definitions/ApprovalsReviewer" + }, + { + "type": "null" + } + ] + }, "default_tools_approval_mode": { "anyOf": [ { diff --git a/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json index 4a104b3bd51..2abdc94a324 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json @@ -19,6 +19,16 @@ }, "AppConfig": { "properties": { + "approvals_reviewer": { + "anyOf": [ + { + "$ref": "#/definitions/ApprovalsReviewer" + }, + { + "type": "null" + } + ] + }, "default_tools_approval_mode": { "anyOf": [ { diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/AppsConfig.ts b/codex-rs/app-server-protocol/schema/typescript/v2/AppsConfig.ts index b22997de97f..a4a0f220812 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/AppsConfig.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/AppsConfig.ts @@ -3,6 +3,7 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { AppToolApproval } from "./AppToolApproval"; import type { AppToolsConfig } from "./AppToolsConfig"; +import type { ApprovalsReviewer } from "./ApprovalsReviewer"; import type { AppsDefaultConfig } from "./AppsDefaultConfig"; -export type AppsConfig = { _default: AppsDefaultConfig | null, } & ({ [key in string]?: { enabled: boolean, destructive_enabled: boolean | null, open_world_enabled: boolean | null, default_tools_approval_mode: AppToolApproval | null, default_tools_enabled: boolean | null, tools: AppToolsConfig | null, } }); +export type AppsConfig = { _default: AppsDefaultConfig | null, } & ({ [key in string]?: { enabled: boolean, approvals_reviewer: ApprovalsReviewer | null, destructive_enabled: boolean | null, open_world_enabled: boolean | null, default_tools_approval_mode: AppToolApproval | null, default_tools_enabled: boolean | null, tools: AppToolsConfig | null, } }); diff --git a/codex-rs/app-server-protocol/src/protocol/v2/config.rs b/codex-rs/app-server-protocol/src/protocol/v2/config.rs index e30b3012efc..47602c478b0 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/config.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/config.rs @@ -185,6 +185,7 @@ pub struct AppToolsConfig { pub struct AppConfig { #[serde(default = "default_enabled")] pub enabled: bool, + pub approvals_reviewer: Option, pub destructive_enabled: Option, pub open_world_enabled: Option, pub default_tools_approval_mode: Option, diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 2bf6035b271..908b3bda9cc 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -1715,6 +1715,20 @@ The server also emits `app/list/updated` notifications whenever either source (a } ``` +Connected apps may override the thread's approval reviewer in `config.toml`. +When omitted, the app inherits the top-level `approvals_reviewer` value: + +```toml +approvals_reviewer = "auto_review" + +[apps.demo-app] +approvals_reviewer = "user" +``` + +Setting the app value to `"user"` routes its approval prompts to the user +instead of Guardian; setting it to `"auto_review"` opts that app into Guardian +review when allowed by configuration requirements. + Invoke an app by inserting `$` in the text input. The slug is derived from the app name and lowercased with non-alphanumeric characters replaced by `-` (for example, "Demo App" becomes `$demo-app`). Add a `mention` input item (recommended) so the server uses the exact `app://` path rather than guessing by name. Plugins use the same `mention` item shape, but with `plugin://@` paths from `plugin/installed` or `plugin/list`. Example: diff --git a/codex-rs/app-server/src/config_manager_service_tests.rs b/codex-rs/app-server/src/config_manager_service_tests.rs index 5c6e3de5a10..da2437ca17a 100644 --- a/codex-rs/app-server/src/config_manager_service_tests.rs +++ b/codex-rs/app-server/src/config_manager_service_tests.rs @@ -284,6 +284,7 @@ async fn write_value_supports_nested_app_paths() -> Result<()> { "app1".to_string(), AppConfig { enabled: false, + approvals_reviewer: None, destructive_enabled: None, open_world_enabled: None, default_tools_approval_mode: Some(AppToolApproval::Prompt), diff --git a/codex-rs/app-server/tests/suite/v2/config_rpc.rs b/codex-rs/app-server/tests/suite/v2/config_rpc.rs index 0bfd8ec876e..af9c284d41e 100644 --- a/codex-rs/app-server/tests/suite/v2/config_rpc.rs +++ b/codex-rs/app-server/tests/suite/v2/config_rpc.rs @@ -5,6 +5,7 @@ use app_test_support::test_tmp_path_buf; use app_test_support::to_response; use codex_app_server_protocol::AppConfig; use codex_app_server_protocol::AppToolApproval; +use codex_app_server_protocol::ApprovalsReviewer; use codex_app_server_protocol::AppsConfig; use codex_app_server_protocol::AskForApproval; use codex_app_server_protocol::ConfigBatchWriteParams; @@ -333,6 +334,7 @@ async fn config_read_includes_apps() -> Result<()> { r#" [apps.app1] enabled = false +approvals_reviewer = "user" destructive_enabled = false default_tools_approval_mode = "prompt" "#, @@ -368,6 +370,7 @@ default_tools_approval_mode = "prompt" "app1".to_string(), AppConfig { enabled: false, + approvals_reviewer: Some(ApprovalsReviewer::User), destructive_enabled: Some(false), open_world_enabled: None, default_tools_approval_mode: Some(AppToolApproval::Prompt), @@ -384,6 +387,16 @@ default_tools_approval_mode = "prompt" profile: None, } ); + assert_eq!( + origins + .get("apps.app1.approvals_reviewer") + .expect("origin") + .name, + ConfigLayerSource::User { + file: user_file.clone(), + profile: None, + } + ); assert_eq!( origins .get("apps.app1.destructive_enabled") diff --git a/codex-rs/config/src/types.rs b/codex-rs/config/src/types.rs index 30d297030ac..af9959202e5 100644 --- a/codex-rs/config/src/types.rs +++ b/codex-rs/config/src/types.rs @@ -424,6 +424,10 @@ pub struct AppConfig { #[serde(default = "default_enabled")] pub enabled: bool, + /// Reviewer for approval prompts from this app, overriding the thread default. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub approvals_reviewer: Option, + /// Whether tools with `destructive_hint = true` are allowed for this app. #[serde(default, skip_serializing_if = "Option::is_none")] pub destructive_enabled: Option, diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 2d7fd818c95..980fb0a12f1 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -102,6 +102,14 @@ "additionalProperties": false, "description": "Config values for a single app/connector.", "properties": { + "approvals_reviewer": { + "allOf": [ + { + "$ref": "#/definitions/ApprovalsReviewer" + } + ], + "description": "Reviewer for approval prompts from this app, overriding the thread default." + }, "default_tools_approval_mode": { "allOf": [ { diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 3c55f637614..444e6202a12 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -34,6 +34,7 @@ use crate::config::Config; use crate::guardian::GuardianApprovalRequest; use crate::guardian::new_guardian_review_id; use crate::guardian::routes_approval_to_guardian; +use crate::guardian::routes_approval_to_guardian_with_reviewer; use crate::guardian::spawn_approval_request_review; use crate::mcp_tool_call::MCP_TOOL_APPROVAL_ACCEPT; use crate::mcp_tool_call::MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION; @@ -41,6 +42,7 @@ use crate::mcp_tool_call::MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC; use crate::mcp_tool_call::build_guardian_mcp_tool_review_request; use crate::mcp_tool_call::is_mcp_tool_approval_question_id; use crate::mcp_tool_call::lookup_mcp_tool_metadata; +use crate::mcp_tool_call::mcp_approvals_reviewer; use crate::session::Codex; use crate::session::CodexSpawnArgs; use crate::session::CodexSpawnOk; @@ -628,15 +630,14 @@ async fn handle_request_user_input( event: RequestUserInputEvent, cancel_token: &CancellationToken, ) { - if routes_approval_to_guardian(parent_ctx) - && let Some(response) = maybe_auto_review_mcp_request_user_input( - parent_session, - parent_ctx, - pending_mcp_invocations, - &event, - cancel_token, - ) - .await + if let Some(response) = maybe_auto_review_mcp_request_user_input( + parent_session, + parent_ctx, + pending_mcp_invocations, + &event, + cancel_token, + ) + .await { let _ = codex.submit(Op::UserInputAnswer { id, response }).await; return; @@ -690,6 +691,12 @@ async fn maybe_auto_review_mcp_request_user_input( &invocation.tool, ) .await; + if !routes_approval_to_guardian_with_reviewer( + parent_ctx, + mcp_approvals_reviewer(parent_ctx, &invocation, metadata.as_ref()), + ) { + return None; + } let review_cancel = cancel_token.child_token(); let review_rx = spawn_approval_request_review( Arc::clone(parent_session), diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index 09717bb121a..91ff3fe0046 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -26,6 +26,7 @@ use crate::plugins::list_tool_suggest_discoverable_plugins; use crate::session::INITIAL_SUBMIT_ID; use codex_config::AppsRequirementsToml; use codex_config::types::AppToolApproval; +use codex_config::types::ApprovalsReviewer; use codex_config::types::AppsConfigToml; use codex_config::types::ToolSuggestDiscoverableType; use codex_core_plugins::PluginsManager; @@ -566,6 +567,25 @@ pub(crate) fn codex_app_tool_is_enabled(config: &Config, tool_info: &ToolInfo) - .enabled } +pub(crate) fn app_approvals_reviewer( + config: &Config, + connector_id: Option<&str>, +) -> ApprovalsReviewer { + read_user_apps_config(config) + .as_ref() + .and_then(|apps_config| connector_id.and_then(|id| apps_config.apps.get(id))) + .and_then(|app| app.approvals_reviewer) + .filter(|reviewer| { + config + .config_layer_stack + .requirements() + .approvals_reviewer + .can_set(reviewer) + .is_ok() + }) + .unwrap_or(config.approvals_reviewer) +} + fn read_apps_config(config: &Config) -> Option { let apps_config = read_user_apps_config(config); let had_apps_config = apps_config.is_some(); diff --git a/codex-rs/core/src/connectors_tests.rs b/codex-rs/core/src/connectors_tests.rs index 3733d78c380..3f4b27bfbbd 100644 --- a/codex-rs/core/src/connectors_tests.rs +++ b/codex-rs/core/src/connectors_tests.rs @@ -12,6 +12,7 @@ use codex_config::ConfigRequirementsToml; use codex_config::types::AppConfig; use codex_config::types::AppToolConfig; use codex_config::types::AppToolsConfig; +use codex_config::types::ApprovalsReviewer; use codex_config::types::AppsDefaultConfig; use codex_connectors::merge::plugin_connector_to_app_info; use codex_connectors::metadata::connector_install_url; @@ -377,6 +378,7 @@ fn app_is_enabled_prefers_per_app_override_over_default() { "calendar".to_string(), AppConfig { enabled: true, + approvals_reviewer: None, destructive_enabled: None, open_world_enabled: None, default_tools_approval_mode: None, @@ -390,6 +392,96 @@ fn app_is_enabled_prefers_per_app_override_over_default() { assert!(!app_is_enabled(&apps_config, Some("drive"))); } +#[tokio::test] +async fn app_approvals_reviewer_allows_app_to_enable_guardian() { + let codex_home = tempdir().expect("tempdir should succeed"); + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + r#" +approvals_reviewer = "user" + +[apps.calendar] +approvals_reviewer = "auto_review" +"#, + ) + .expect("write config"); + let config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .build() + .await + .expect("config should build"); + + assert_eq!( + app_approvals_reviewer(&config, Some("calendar")), + ApprovalsReviewer::AutoReview + ); + assert_eq!( + app_approvals_reviewer(&config, Some("drive")), + ApprovalsReviewer::User + ); +} + +#[tokio::test] +async fn app_approvals_reviewer_allows_app_to_disable_guardian() { + let codex_home = tempdir().expect("tempdir should succeed"); + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + r#" +approvals_reviewer = "auto_review" + +[apps.calendar] +approvals_reviewer = "user" +"#, + ) + .expect("write config"); + let config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .build() + .await + .expect("config should build"); + + assert_eq!( + app_approvals_reviewer(&config, Some("calendar")), + ApprovalsReviewer::User + ); + assert_eq!( + app_approvals_reviewer(&config, Some("drive")), + ApprovalsReviewer::AutoReview + ); +} + +#[tokio::test] +async fn app_approvals_reviewer_respects_global_reviewer_requirements() { + let codex_home = tempdir().expect("tempdir should succeed"); + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + r#" +approvals_reviewer = "auto_review" + +[apps.calendar] +approvals_reviewer = "user" +"#, + ) + .expect("write config"); + let requirements = ConfigRequirementsToml { + allowed_approvals_reviewers: Some(vec![ApprovalsReviewer::AutoReview]), + ..Default::default() + }; + let config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .cloud_requirements(CloudRequirementsLoader::new(async move { + Ok(Some(requirements)) + })) + .build() + .await + .expect("config should build"); + + assert_eq!( + app_approvals_reviewer(&config, Some("calendar")), + ApprovalsReviewer::AutoReview + ); +} + #[test] fn requirements_disabled_connector_overrides_enabled_connector() { let mut effective_apps = AppsConfigToml { @@ -932,6 +1024,7 @@ fn app_tool_policy_allows_per_app_enable_when_default_is_disabled() { "calendar".to_string(), AppConfig { enabled: true, + approvals_reviewer: None, destructive_enabled: None, open_world_enabled: None, default_tools_approval_mode: None, @@ -969,6 +1062,7 @@ fn app_tool_policy_per_tool_enabled_true_overrides_app_level_disable_flags() { "calendar".to_string(), AppConfig { enabled: true, + approvals_reviewer: None, destructive_enabled: Some(false), open_world_enabled: Some(false), default_tools_approval_mode: None, @@ -1012,6 +1106,7 @@ fn app_tool_policy_default_tools_enabled_true_overrides_app_level_tool_hints() { "calendar".to_string(), AppConfig { enabled: true, + approvals_reviewer: None, destructive_enabled: Some(false), open_world_enabled: Some(false), default_tools_approval_mode: None, @@ -1047,6 +1142,7 @@ fn app_tool_policy_default_tools_enabled_false_overrides_app_level_tool_hints() "calendar".to_string(), AppConfig { enabled: true, + approvals_reviewer: None, destructive_enabled: Some(true), open_world_enabled: Some(true), default_tools_approval_mode: Some(AppToolApproval::Approve), @@ -1084,6 +1180,7 @@ fn app_tool_policy_uses_default_tools_approval_mode() { "calendar".to_string(), AppConfig { enabled: true, + approvals_reviewer: None, destructive_enabled: None, open_world_enabled: None, default_tools_approval_mode: Some(AppToolApproval::Prompt), @@ -1123,6 +1220,7 @@ fn app_tool_policy_matches_prefix_stripped_tool_name_for_tool_config() { "calendar".to_string(), AppConfig { enabled: true, + approvals_reviewer: None, destructive_enabled: Some(false), open_world_enabled: Some(false), default_tools_approval_mode: Some(AppToolApproval::Auto), diff --git a/codex-rs/core/src/guardian/mod.rs b/codex-rs/core/src/guardian/mod.rs index 8cc04e5c423..b4920f1ff6f 100644 --- a/codex-rs/core/src/guardian/mod.rs +++ b/codex-rs/core/src/guardian/mod.rs @@ -39,6 +39,7 @@ pub(crate) use review::review_approval_request; #[cfg(test)] pub(crate) use review::review_approval_request_with_cancel; pub(crate) use review::routes_approval_to_guardian; +pub(crate) use review::routes_approval_to_guardian_with_reviewer; pub(crate) use review::spawn_approval_request_review; pub(crate) use review_session::GuardianReviewSessionManager; pub(crate) use review_session::prompt_cache_key_override_for_review_session; diff --git a/codex-rs/core/src/guardian/review.rs b/codex-rs/core/src/guardian/review.rs index db4343448fe..bf6d649c28d 100644 --- a/codex-rs/core/src/guardian/review.rs +++ b/codex-rs/core/src/guardian/review.rs @@ -145,10 +145,18 @@ fn guardian_risk_level_str(level: GuardianRiskLevel) -> &'static str { /// reviewer instead of surfacing them to the user. ARC may still block actions /// earlier in the flow. pub(crate) fn routes_approval_to_guardian(turn: &TurnContext) -> bool { + routes_approval_to_guardian_with_reviewer(turn, turn.config.approvals_reviewer) +} + +/// Whether an approval with its own reviewer selection should be routed through guardian. +pub(crate) fn routes_approval_to_guardian_with_reviewer( + turn: &TurnContext, + approvals_reviewer: ApprovalsReviewer, +) -> bool { matches!( turn.approval_policy.value(), AskForApproval::OnRequest | AskForApproval::Granular(_) - ) && turn.config.approvals_reviewer == ApprovalsReviewer::AutoReview + ) && approvals_reviewer == ApprovalsReviewer::AutoReview } pub(crate) fn is_guardian_reviewer_source( diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index 3d44fa41de5..282a8c91ec4 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -1099,6 +1099,20 @@ async fn routes_approval_to_guardian_requires_guardian_reviewer() { assert!(routes_approval_to_guardian(&turn)); } +#[tokio::test] +async fn routes_approval_to_guardian_can_use_app_reviewer_override() { + let (_session, turn) = crate::session::tests::make_session_and_context().await; + + assert!(!routes_approval_to_guardian_with_reviewer( + &turn, + ApprovalsReviewer::User + )); + assert!(routes_approval_to_guardian_with_reviewer( + &turn, + ApprovalsReviewer::AutoReview + )); +} + #[tokio::test] async fn routes_approval_to_guardian_allows_granular_review_policy() { let (_session, mut turn) = crate::session::tests::make_session_and_context().await; diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index e514fa0d03c..d51d202f238 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -13,7 +13,7 @@ use crate::guardian::guardian_rejection_message; use crate::guardian::guardian_timeout_message; use crate::guardian::new_guardian_review_id; use crate::guardian::review_approval_request; -use crate::guardian::routes_approval_to_guardian; +use crate::guardian::routes_approval_to_guardian_with_reviewer; use crate::hook_runtime::run_permission_request_hooks; use crate::mcp_openai_file::rewrite_mcp_tool_arguments_for_openai_files; use crate::mcp_tool_approval_templates::RenderedMcpToolApprovalParam; @@ -32,6 +32,7 @@ use codex_app_server_protocol::McpElicitationSchema; use codex_app_server_protocol::McpServerElicitationRequest; use codex_app_server_protocol::McpServerElicitationRequestParams; use codex_config::types::AppToolApproval; +use codex_config::types::ApprovalsReviewer; use codex_features::Feature; use codex_hooks::PermissionRequestDecision; use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; @@ -1162,11 +1163,12 @@ async fn maybe_request_mcp_tool_approval( metadata: Option<&McpToolApprovalMetadata>, approval_mode: AppToolApproval, ) -> Option { + let approvals_reviewer = mcp_approvals_reviewer(turn_context, invocation, metadata); if mcp_permission_prompt_is_auto_approved( turn_context.approval_policy.value(), &turn_context.permission_profile(), McpPermissionPromptAutoApproveContext { - approvals_reviewer: Some(turn_context.config.approvals_reviewer), + approvals_reviewer: Some(approvals_reviewer), tool_approval_mode: Some(approval_mode), }, ) { @@ -1218,7 +1220,7 @@ async fn maybe_request_mcp_tool_approval( .features .enabled(Feature::ToolCallMcpElicitation); - if routes_approval_to_guardian(turn_context) { + if routes_approval_to_guardian_with_reviewer(turn_context, approvals_reviewer) { let review_id = new_guardian_review_id(); let decision = review_approval_request( sess, @@ -1328,6 +1330,21 @@ async fn maybe_request_mcp_tool_approval( Some(decision) } +pub(crate) fn mcp_approvals_reviewer( + turn_context: &TurnContext, + invocation: &McpInvocation, + metadata: Option<&McpToolApprovalMetadata>, +) -> ApprovalsReviewer { + if invocation.server == CODEX_APPS_MCP_SERVER_NAME { + return connectors::app_approvals_reviewer( + &turn_context.config, + metadata.and_then(|metadata| metadata.connector_id.as_deref()), + ); + } + + turn_context.config.approvals_reviewer +} + fn session_mcp_tool_approval_key( invocation: &McpInvocation, metadata: Option<&McpToolApprovalMetadata>, diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index 534c2f01182..0161da74c36 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -1886,6 +1886,7 @@ async fn persist_codex_app_tool_approval_writes_tool_override() { "calendar".to_string(), AppConfig { enabled: true, + approvals_reviewer: None, destructive_enabled: None, open_world_enabled: None, default_tools_approval_mode: None, @@ -2669,6 +2670,55 @@ async fn guardian_mode_mcp_denial_returns_rationale_message() { ); } +#[tokio::test] +async fn codex_apps_mcp_uses_connector_approvals_reviewer_override() { + let codex_home = tempdir().expect("tempdir should succeed"); + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + r#" +approvals_reviewer = "user" + +[apps.calendar] +approvals_reviewer = "auto_review" +"#, + ) + .expect("write config"); + let config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .build() + .await + .expect("config should build"); + let (_session, mut turn_context) = make_session_and_context().await; + turn_context.config = Arc::new(config); + let metadata = approval_metadata( + Some("calendar"), + Some("Calendar"), + /*connector_description*/ None, + /*tool_title*/ None, + /*tool_description*/ None, + ); + let connected_app_invocation = McpInvocation { + server: CODEX_APPS_MCP_SERVER_NAME.to_string(), + tool: "create_event".to_string(), + arguments: None, + }; + + assert_eq!( + mcp_approvals_reviewer(&turn_context, &connected_app_invocation, Some(&metadata),), + ApprovalsReviewer::AutoReview + ); + + let custom_mcp_invocation = McpInvocation { + server: "custom_server".to_string(), + tool: "create_event".to_string(), + arguments: None, + }; + assert_eq!( + mcp_approvals_reviewer(&turn_context, &custom_mcp_invocation, Some(&metadata)), + ApprovalsReviewer::User + ); +} + #[tokio::test] async fn prompt_mode_waits_for_approval_when_annotations_do_not_require_approval() { let (session, turn_context, _rx_event) = make_session_and_context_with_rx().await; diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index a775c8b8363..060c18941eb 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -1,4 +1,5 @@ use super::*; +use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; use codex_mcp::ElicitationReviewRequest; use codex_mcp::ElicitationReviewer; use codex_mcp::ElicitationReviewerHandle; @@ -455,7 +456,25 @@ async fn review_guardian_mcp_elicitation( return Ok(None); }; - if !crate::guardian::routes_approval_to_guardian(turn_context.as_ref()) { + let connector_id = if request.server_name == CODEX_APPS_MCP_SERVER_NAME { + match &request.elicitation { + CreateElicitationRequestParams::FormElicitationParams { meta, .. } + | CreateElicitationRequestParams::UrlElicitationParams { meta, .. } => meta + .as_ref() + .and_then(|meta| metadata_str(&meta.0, MCP_ELICITATION_CONNECTOR_ID_KEY)), + } + } else { + None + }; + let approvals_reviewer = if request.server_name == CODEX_APPS_MCP_SERVER_NAME { + crate::connectors::app_approvals_reviewer(&turn_context.config, connector_id) + } else { + turn_context.config.approvals_reviewer + }; + if !crate::guardian::routes_approval_to_guardian_with_reviewer( + turn_context.as_ref(), + approvals_reviewer, + ) { return Ok(None); }