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 9aedb8a7e..e40c81f5d 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 @@ -6584,6 +6584,16 @@ }, "AppsDefaultConfig": { "properties": { + "approvals_reviewer": { + "anyOf": [ + { + "$ref": "#/definitions/v2/ApprovalsReviewer" + }, + { + "type": "null" + } + ] + }, "destructive_enabled": { "default": true, "type": "boolean" 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 ebe1efe26..3a36c1022 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 @@ -853,6 +853,16 @@ }, "AppsDefaultConfig": { "properties": { + "approvals_reviewer": { + "anyOf": [ + { + "$ref": "#/definitions/ApprovalsReviewer" + }, + { + "type": "null" + } + ] + }, "destructive_enabled": { "default": true, "type": "boolean" 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 e311090f2..3bc0a8e0a 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json @@ -133,6 +133,16 @@ }, "AppsDefaultConfig": { "properties": { + "approvals_reviewer": { + "anyOf": [ + { + "$ref": "#/definitions/ApprovalsReviewer" + }, + { + "type": "null" + } + ] + }, "destructive_enabled": { "default": true, "type": "boolean" diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/AppsDefaultConfig.ts b/codex-rs/app-server-protocol/schema/typescript/v2/AppsDefaultConfig.ts index e73386027..8277320e6 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/AppsDefaultConfig.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/AppsDefaultConfig.ts @@ -1,5 +1,6 @@ // GENERATED CODE! DO NOT MODIFY BY HAND! // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { ApprovalsReviewer } from "./ApprovalsReviewer"; -export type AppsDefaultConfig = { enabled: boolean, destructive_enabled: boolean, open_world_enabled: boolean, }; +export type AppsDefaultConfig = { enabled: boolean, approvals_reviewer: ApprovalsReviewer | null, destructive_enabled: boolean, open_world_enabled: boolean, }; 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 227adffc3..31568f96d 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/config.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/config.rs @@ -172,6 +172,7 @@ pub enum AppToolApproval { pub struct AppsDefaultConfig { #[serde(default = "default_enabled")] pub enabled: bool, + pub approvals_reviewer: Option, #[serde(default = "default_enabled")] pub destructive_enabled: bool, #[serde(default = "default_enabled")] diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index cc36bb451..0e0e9b856 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -1775,13 +1775,18 @@ 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: +Use `apps._default.approvals_reviewer` to set the reviewer for all apps, and a +per-app value to override that default. When both are omitted, the app inherits +the top-level `approvals_reviewer` value: ```toml approvals_reviewer = "auto_review" -[apps.demo-app] +[apps._default] approvals_reviewer = "user" + +[apps.demo-app] +approvals_reviewer = "auto_review" ``` Setting the app value to `"user"` routes its approval prompts to the user 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 649c4fe8e..343b91b8d 100644 --- a/codex-rs/app-server/tests/suite/v2/config_rpc.rs +++ b/codex-rs/app-server/tests/suite/v2/config_rpc.rs @@ -7,6 +7,7 @@ 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::AppsDefaultConfig; use codex_app_server_protocol::AskForApproval; use codex_app_server_protocol::ConfigBatchWriteParams; use codex_app_server_protocol::ConfigEdit; @@ -332,6 +333,9 @@ async fn config_read_includes_apps() -> Result<()> { write_config( &codex_home, r#" +[apps._default] +approvals_reviewer = "auto_review" + [apps.app1] enabled = false approvals_reviewer = "user" @@ -365,7 +369,12 @@ default_tools_approval_mode = "prompt" assert_eq!( config.apps, Some(AppsConfig { - default: None, + default: Some(AppsDefaultConfig { + enabled: true, + approvals_reviewer: Some(ApprovalsReviewer::AutoReview), + destructive_enabled: true, + open_world_enabled: true, + }), apps: std::collections::HashMap::from([( "app1".to_string(), AppConfig { @@ -380,6 +389,16 @@ default_tools_approval_mode = "prompt" )]), }) ); + assert_eq!( + origins + .get("apps._default.approvals_reviewer") + .expect("origin") + .name, + ConfigLayerSource::User { + file: user_file.clone(), + profile: None, + } + ); assert_eq!( origins.get("apps.app1.enabled").expect("origin").name, ConfigLayerSource::User { diff --git a/codex-rs/config/src/types.rs b/codex-rs/config/src/types.rs index 7b634a518..7cf8ac66a 100644 --- a/codex-rs/config/src/types.rs +++ b/codex-rs/config/src/types.rs @@ -380,6 +380,10 @@ pub struct AppsDefaultConfig { #[serde(default = "default_enabled")] pub enabled: bool, + /// Reviewer for approval prompts unless overridden by per-app settings. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub approvals_reviewer: Option, + /// Whether tools with `destructive_hint = true` are allowed by default. #[serde( default = "default_enabled", diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 5fcd8eb07..78d667a63 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -210,6 +210,14 @@ "additionalProperties": false, "description": "Default settings that apply to all apps.", "properties": { + "approvals_reviewer": { + "allOf": [ + { + "$ref": "#/definitions/ApprovalsReviewer" + } + ], + "description": "Reviewer for approval prompts unless overridden by per-app settings." + }, "destructive_enabled": { "description": "Whether tools with `destructive_hint = true` are allowed by default.", "type": "boolean" diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index 8ccf51ef5..f21912fdb 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -601,6 +601,11 @@ pub(crate) fn mcp_approvals_reviewer( connector_id .and_then(|connector_id| apps_config.apps.get(connector_id)) .and_then(|app| app.approvals_reviewer) + .or_else(|| { + apps_config + .default + .and_then(|defaults| defaults.approvals_reviewer) + }) }) } else { None diff --git a/codex-rs/core/src/connectors_tests.rs b/codex-rs/core/src/connectors_tests.rs index 0427ce5a4..337e69c8d 100644 --- a/codex-rs/core/src/connectors_tests.rs +++ b/codex-rs/core/src/connectors_tests.rs @@ -269,6 +269,7 @@ fn app_tool_policy_uses_global_defaults_for_destructive_hints() { let apps_config = AppsConfigToml { default: Some(AppsDefaultConfig { enabled: true, + approvals_reviewer: None, destructive_enabled: false, open_world_enabled: true, }), @@ -298,6 +299,7 @@ fn app_tool_policy_defaults_missing_destructive_hint_to_true() { let apps_config = AppsConfigToml { default: Some(AppsDefaultConfig { enabled: true, + approvals_reviewer: None, destructive_enabled: false, open_world_enabled: true, }), @@ -327,6 +329,7 @@ fn app_tool_policy_defaults_missing_open_world_hint_to_true() { let apps_config = AppsConfigToml { default: Some(AppsDefaultConfig { enabled: true, + approvals_reviewer: None, destructive_enabled: true, open_world_enabled: false, }), @@ -356,6 +359,7 @@ fn app_is_enabled_uses_default_for_unconfigured_apps() { let apps_config = AppsConfigToml { default: Some(AppsDefaultConfig { enabled: false, + approvals_reviewer: None, destructive_enabled: true, open_world_enabled: true, }), @@ -371,6 +375,7 @@ fn app_is_enabled_prefers_per_app_override_over_default() { let apps_config = AppsConfigToml { default: Some(AppsDefaultConfig { enabled: false, + approvals_reviewer: None, destructive_enabled: true, open_world_enabled: true, }), @@ -393,19 +398,23 @@ fn app_is_enabled_prefers_per_app_override_over_default() { } #[tokio::test] -async fn app_approvals_reviewer_overrides_global_reviewer() { - for (global, app, expected_global, expected_app) in [ +async fn app_approvals_reviewer_uses_app_then_default_then_global() { + for (global, app_default, app, expected_global, expected_default, expected_app) in [ ( "user", "auto_review", + "user", ApprovalsReviewer::User, ApprovalsReviewer::AutoReview, + ApprovalsReviewer::User, ), ( "auto_review", "user", + "auto_review", ApprovalsReviewer::AutoReview, ApprovalsReviewer::User, + ApprovalsReviewer::AutoReview, ), ] { let codex_home = tempdir().expect("tempdir should succeed"); @@ -415,6 +424,9 @@ async fn app_approvals_reviewer_overrides_global_reviewer() { r#" approvals_reviewer = "{global}" +[apps._default] +approvals_reviewer = "{app_default}" + [apps.calendar] approvals_reviewer = "{app}" "# @@ -433,7 +445,15 @@ approvals_reviewer = "{app}" ); assert_eq!( mcp_approvals_reviewer(&config, CODEX_APPS_MCP_SERVER_NAME, Some("drive")), - expected_global + expected_default + ); + assert_eq!( + mcp_approvals_reviewer( + &config, + CODEX_APPS_MCP_SERVER_NAME, + /*connector_id*/ None + ), + expected_default ); assert_eq!( mcp_approvals_reviewer(&config, "custom_server", Some("calendar")), @@ -442,6 +462,36 @@ approvals_reviewer = "{app}" } } +#[tokio::test] +async fn default_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._default] +approvals_reviewer = "user" +"#, + ) + .expect("write config"); + let config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .cloud_config_bundle( + CloudConfigBundleFixture::loader_with_enterprise_requirement( + r#"allowed_approvals_reviewers = ["auto_review"]"#, + ), + ) + .build() + .await + .expect("config should build"); + + assert_eq!( + mcp_approvals_reviewer(&config, CODEX_APPS_MCP_SERVER_NAME, Some("calendar")), + ApprovalsReviewer::AutoReview + ); +} + #[tokio::test] async fn app_approvals_reviewer_respects_global_reviewer_requirements() { let codex_home = tempdir().expect("tempdir should succeed"); @@ -756,6 +806,7 @@ fn app_tool_policy_honors_default_app_enabled_false() { let apps_config = AppsConfigToml { default: Some(AppsDefaultConfig { enabled: false, + approvals_reviewer: None, destructive_enabled: true, open_world_enabled: true, }), @@ -987,6 +1038,7 @@ fn app_tool_policy_allows_per_app_enable_when_default_is_disabled() { let apps_config = AppsConfigToml { default: Some(AppsDefaultConfig { enabled: false, + approvals_reviewer: None, destructive_enabled: true, open_world_enabled: true, }), diff --git a/codex-rs/core/tests/suite/mcp_turn_metadata.rs b/codex-rs/core/tests/suite/mcp_turn_metadata.rs index 49d3c0f2a..21b277bca 100644 --- a/codex-rs/core/tests/suite/mcp_turn_metadata.rs +++ b/codex-rs/core/tests/suite/mcp_turn_metadata.rs @@ -5,6 +5,7 @@ use anyhow::Result; use codex_config::types::AppToolApproval; use codex_core::config::Config; use codex_features::Feature; +use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Settings; @@ -59,6 +60,32 @@ default_tools_approval_mode = "{approval_mode}" .with_user_config(&user_config_path, user_config); } +fn set_calendar_approval_mode_and_default_reviewer( + config: &mut Config, + approval_mode: AppToolApproval, + default_approvals_reviewer: ApprovalsReviewer, +) { + let approval_mode = match approval_mode { + AppToolApproval::Auto => "auto", + AppToolApproval::Prompt => "prompt", + AppToolApproval::Approve => "approve", + }; + let user_config_path = config.codex_home.join("config.toml").abs(); + let user_config = toml::from_str(&format!( + r#" +[apps._default] +approvals_reviewer = "{default_approvals_reviewer}" + +[apps.calendar] +default_tools_approval_mode = "{approval_mode}" +"# + )) + .expect("apps config should parse"); + config.config_layer_stack = config + .config_layer_stack + .with_user_config(&user_config_path, user_config); +} + async fn submit_user_turn( test: &TestCodex, text: &str, @@ -134,11 +161,17 @@ async fn approved_mcp_tool_call_metadata_records_prior_user_input_request() -> R let mut builder = search_capable_apps_builder(apps_server.chatgpt_base_url.clone()) .with_config(|config| { + // Use the opposite global reviewer so this route must come from apps._default. + config.approvals_reviewer = ApprovalsReviewer::AutoReview; config .features .enable(Feature::ToolCallMcpElicitation) .expect("test config should allow feature update"); - set_calendar_approval_mode(config, AppToolApproval::Prompt); + set_calendar_approval_mode_and_default_reviewer( + config, + AppToolApproval::Prompt, + ApprovalsReviewer::User, + ); }); let test = builder.build(&server).await?; @@ -160,11 +193,14 @@ async fn approved_mcp_tool_call_metadata_records_prior_user_input_request() -> R assert_eq!(begin.call_id, call_id); let EventMsg::ElicitationRequest(request) = wait_for_event(&test.codex, |event| { - matches!(event, EventMsg::ElicitationRequest(_)) + matches!( + event, + EventMsg::ElicitationRequest(_) | EventMsg::TurnComplete(_) + ) }) .await else { - unreachable!("event guard guarantees ElicitationRequest"); + panic!("expected apps._default user to route the app approval to the user"); }; test.codex @@ -194,6 +230,110 @@ async fn approved_mcp_tool_call_metadata_records_prior_user_input_request() -> R Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apps_default_auto_review_routes_actual_mcp_approval_to_guardian() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let apps_server = AppsTestServer::mount(&server).await?; + let call_id = "calendar-default-auto-review"; + let calendar_args = serde_json::to_string(&json!({ + "title": "Lunch", + "starts_at": "2026-03-10T12:00:00Z" + }))?; + let responses = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-parent-tool"), + ev_function_call_with_namespace( + call_id, + SEARCH_CALENDAR_NAMESPACE, + SEARCH_CALENDAR_CREATE_TOOL, + &calendar_args, + ), + ev_completed("resp-parent-tool"), + ]), + sse(vec![ + ev_response_created("resp-guardian-review"), + ev_assistant_message( + "msg-guardian-review", + &json!({ + "risk_level": "low", + "user_authorization": "high", + "outcome": "allow", + "rationale": "Creating this calendar event is low risk.", + }) + .to_string(), + ), + ev_completed("resp-guardian-review"), + ]), + sse(vec![ + ev_response_created("resp-parent-done"), + ev_assistant_message("msg-parent-done", "done"), + ev_completed("resp-parent-done"), + ]), + ], + ) + .await; + + let mut builder = search_capable_apps_builder(apps_server.chatgpt_base_url.clone()) + .with_config(|config| { + // Use the opposite global reviewer so this route must come from apps._default. + config.approvals_reviewer = ApprovalsReviewer::User; + config + .features + .enable(Feature::ToolCallMcpElicitation) + .expect("test config should allow feature update"); + set_calendar_approval_mode_and_default_reviewer( + config, + AppToolApproval::Prompt, + ApprovalsReviewer::AutoReview, + ); + }); + let test = builder.build(&server).await?; + + submit_user_turn( + &test, + "Use [$calendar](app://calendar) to create a calendar event.", + AskForApproval::OnRequest, + /*collaboration_mode*/ None, + ) + .await?; + + let route_event = wait_for_event(&test.codex, |event| { + matches!( + event, + EventMsg::ElicitationRequest(_) | EventMsg::TurnComplete(_) + ) + }) + .await; + assert!( + matches!(route_event, EventMsg::TurnComplete(_)), + "expected apps._default auto_review to route the app approval to Guardian" + ); + + let guardian_request = responses + .requests() + .into_iter() + .find(|request| { + request + .instructions_text() + .starts_with("You are judging one planned coding-agent action.") + }) + .expect("expected a Guardian request for the app MCP approval"); + assert!(guardian_request.body_contains_text("calendar_create_event")); + assert!(guardian_request.body_contains_text("Lunch")); + + let apps_tool_call = recorded_apps_tool_call_by_call_id(&server, call_id).await; + assert_eq!( + apps_tool_call.pointer("/params/arguments/title"), + Some(&json!("Lunch")) + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn mcp_tool_call_metadata_records_prior_request_user_input_tool() -> Result<()> { skip_if_no_network!(Ok(()));