From 09801d7ee4babdfeab87530242cb9438b889a6f3 Mon Sep 17 00:00:00 2001 From: Alex Zamoshchin Date: Sun, 28 Jun 2026 16:17:57 -0400 Subject: [PATCH] [codex-rs] Add writes app approval mode --- .../codex_app_server_protocol.schemas.json | 1 + .../codex_app_server_protocol.v2.schemas.json | 1 + .../schema/json/v2/ConfigReadResponse.json | 1 + .../schema/typescript/v2/AppToolApproval.ts | 2 +- .../src/protocol/v2/config.rs | 1 + codex-rs/app-server/README.md | 4 +- .../app-server/tests/suite/v2/config_rpc.rs | 4 +- codex-rs/cli/src/mcp_cmd.rs | 1 + codex-rs/config/src/mcp_edit.rs | 2 + codex-rs/config/src/mcp_types.rs | 1 + codex-rs/core/config.schema.json | 1 + .../core/src/config/edit/document_helpers.rs | 2 + codex-rs/core/src/mcp_tool_call.rs | 30 +++-- codex-rs/core/src/mcp_tool_call_tests.rs | 46 +++++-- .../core/tests/suite/mcp_turn_metadata.rs | 124 ++++++++++++++++++ 15 files changed, 198 insertions(+), 23 deletions(-) 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 b77b512b36e3..b839118991aa 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 @@ -6728,6 +6728,7 @@ "enum": [ "auto", "prompt", + "writes", "approve" ], "type": "string" 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 bdd61d6b9436..0f1f4656e4dc 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 @@ -854,6 +854,7 @@ "enum": [ "auto", "prompt", + "writes", "approve" ], "type": "string" 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 16f836341656..19863dd1f30d 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json @@ -78,6 +78,7 @@ "enum": [ "auto", "prompt", + "writes", "approve" ], "type": "string" diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/AppToolApproval.ts b/codex-rs/app-server-protocol/schema/typescript/v2/AppToolApproval.ts index e92cd8e28b27..6704ef0fde97 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/AppToolApproval.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/AppToolApproval.ts @@ -2,4 +2,4 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -export type AppToolApproval = "auto" | "prompt" | "approve"; +export type AppToolApproval = "auto" | "prompt" | "writes" | "approve"; 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 f615b25d4483..24d574a935d6 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/config.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/config.rs @@ -163,6 +163,7 @@ pub struct AnalyticsConfig { pub enum AppToolApproval { Auto, Prompt, + Writes, Approve, } diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index c9ac2f5fb27c..adfc9f6cad25 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -1879,7 +1879,9 @@ review when allowed by configuration requirements. Use `apps._default.default_tools_approval_mode` to set the approval mode for tools without a per-app or per-tool override. Supported values are `"auto"`, -`"prompt"`, and `"approve"`. Tool-level `approval_mode` takes precedence over +`"prompt"`, `"writes"`, and `"approve"`. The `"writes"` mode prompts for tools +that do not advertise `readOnlyHint = true` and skips declared read-only tools. +Tool-level `approval_mode` takes precedence over the per-app `default_tools_approval_mode`, which takes precedence over the `apps._default` value. Managed tool requirements take precedence over all of these settings. When none are configured, the mode defaults to `"auto"`. 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 07b8ccc2e8c0..945ab16d7ebb 100644 --- a/codex-rs/app-server/tests/suite/v2/config_rpc.rs +++ b/codex-rs/app-server/tests/suite/v2/config_rpc.rs @@ -400,7 +400,7 @@ async fn config_read_includes_apps() -> Result<()> { r#" [apps._default] approvals_reviewer = "auto_review" -default_tools_approval_mode = "approve" +default_tools_approval_mode = "writes" [apps.app1] enabled = false @@ -440,7 +440,7 @@ default_tools_approval_mode = "prompt" approvals_reviewer: Some(ApprovalsReviewer::AutoReview), destructive_enabled: true, open_world_enabled: true, - default_tools_approval_mode: Some(AppToolApproval::Approve), + default_tools_approval_mode: Some(AppToolApproval::Writes), }), apps: std::collections::HashMap::from([( "app1".to_string(), diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index fd0b1a0e4fff..848180a517eb 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -963,6 +963,7 @@ async fn run_get(config_overrides: &CliConfigOverrides, get_args: GetArgs) -> Re let approval_mode = match approval_mode { AppToolApproval::Auto => "auto", AppToolApproval::Prompt => "prompt", + AppToolApproval::Writes => "writes", AppToolApproval::Approve => "approve", }; println!(" default_tools_approval_mode: {approval_mode}"); diff --git a/codex-rs/config/src/mcp_edit.rs b/codex-rs/config/src/mcp_edit.rs index 16b3904917bc..ef0d8eae48a8 100644 --- a/codex-rs/config/src/mcp_edit.rs +++ b/codex-rs/config/src/mcp_edit.rs @@ -198,6 +198,7 @@ fn serialize_mcp_server(config: &McpServerConfig) -> TomlItem { entry["default_tools_approval_mode"] = value(match approval_mode { AppToolApproval::Auto => "auto", AppToolApproval::Prompt => "prompt", + AppToolApproval::Writes => "writes", AppToolApproval::Approve => "approve", }); } @@ -242,6 +243,7 @@ fn serialize_mcp_server(config: &McpServerConfig) -> TomlItem { tool_entry["approval_mode"] = value(match approval_mode { AppToolApproval::Auto => "auto", AppToolApproval::Prompt => "prompt", + AppToolApproval::Writes => "writes", AppToolApproval::Approve => "approve", }); } diff --git a/codex-rs/config/src/mcp_types.rs b/codex-rs/config/src/mcp_types.rs index 550de84a0c9f..7043e0cb69d3 100644 --- a/codex-rs/config/src/mcp_types.rs +++ b/codex-rs/config/src/mcp_types.rs @@ -22,6 +22,7 @@ pub enum AppToolApproval { #[default] Auto, Prompt, + Writes, Approve, } diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index ab787ba34f2f..608a97451157 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -150,6 +150,7 @@ "enum": [ "auto", "prompt", + "writes", "approve" ], "type": "string" diff --git a/codex-rs/core/src/config/edit/document_helpers.rs b/codex-rs/core/src/config/edit/document_helpers.rs index 308eb9922f36..871ca0e41e21 100644 --- a/codex-rs/core/src/config/edit/document_helpers.rs +++ b/codex-rs/core/src/config/edit/document_helpers.rs @@ -121,6 +121,7 @@ fn serialize_mcp_server_table(config: &McpServerConfig) -> TomlTable { entry["default_tools_approval_mode"] = value(match approval_mode { AppToolApproval::Auto => "auto", AppToolApproval::Prompt => "prompt", + AppToolApproval::Writes => "writes", AppToolApproval::Approve => "approve", }); } @@ -173,6 +174,7 @@ fn serialize_mcp_server_tool(config: &McpServerToolConfig) -> TomlItem { entry["approval_mode"] = value(match approval_mode { AppToolApproval::Auto => "auto", AppToolApproval::Prompt => "prompt", + AppToolApproval::Writes => "writes", AppToolApproval::Approve => "approve", }); } diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index d03f232d12b3..63d21504b939 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -1223,8 +1223,7 @@ async fn maybe_request_mcp_tool_approval( } let annotations = metadata.and_then(|metadata| metadata.annotations.as_ref()); - let approval_required = requires_mcp_tool_approval(annotations); - if !approval_required && approval_mode != AppToolApproval::Prompt { + if !requires_mcp_tool_approval_for_mode(annotations, approval_mode) { return None; } @@ -1927,12 +1926,13 @@ fn normalize_approval_decision_for_mode( decision: McpToolApprovalDecision, approval_mode: AppToolApproval, ) -> McpToolApprovalDecision { - if approval_mode == AppToolApproval::Prompt - && matches!( - decision, - McpToolApprovalDecision::AcceptForSession | McpToolApprovalDecision::AcceptAndRemember - ) - { + if matches!( + approval_mode, + AppToolApproval::Prompt | AppToolApproval::Writes + ) && matches!( + decision, + McpToolApprovalDecision::AcceptForSession | McpToolApprovalDecision::AcceptAndRemember + ) { McpToolApprovalDecision::Accept } else { decision @@ -2179,6 +2179,20 @@ fn requires_mcp_tool_approval(annotations: Option<&ToolAnnotations>) -> bool { .unwrap_or(true) } +fn requires_mcp_tool_approval_for_mode( + annotations: Option<&ToolAnnotations>, + approval_mode: AppToolApproval, +) -> bool { + match approval_mode { + AppToolApproval::Auto => requires_mcp_tool_approval(annotations), + AppToolApproval::Prompt => true, + AppToolApproval::Writes => !annotations + .and_then(|annotations| annotations.read_only_hint) + .unwrap_or(false), + AppToolApproval::Approve => false, + } +} + async fn notify_mcp_tool_call_skip( sess: &Session, turn_context: &TurnContext, diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index 6b642b1cd0a2..c4087d8e99fa 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -398,23 +398,47 @@ fn approval_not_required_when_read_only_and_other_hints_are_absent() { } #[test] -fn prompt_mode_does_not_allow_persistent_remember() { +fn writes_mode_requires_approval_for_non_read_only_tools() { + let annotations = annotations(Some(false), Some(false), Some(false)); assert_eq!( - normalize_approval_decision_for_mode( - McpToolApprovalDecision::AcceptForSession, - AppToolApproval::Prompt, - ), - McpToolApprovalDecision::Accept + requires_mcp_tool_approval_for_mode(Some(&annotations), AppToolApproval::Writes), + true ); assert_eq!( - normalize_approval_decision_for_mode( - McpToolApprovalDecision::AcceptAndRemember, - AppToolApproval::Prompt, - ), - McpToolApprovalDecision::Accept + requires_mcp_tool_approval_for_mode(/*annotations*/ None, AppToolApproval::Writes), + true + ); +} + +#[test] +fn writes_mode_does_not_require_approval_for_read_only_tools() { + let annotations = annotations(Some(true), Some(true), Some(true)); + assert_eq!( + requires_mcp_tool_approval_for_mode(Some(&annotations), AppToolApproval::Writes), + false ); } +#[test] +fn prompting_modes_do_not_allow_persistent_remember() { + for approval_mode in [AppToolApproval::Prompt, AppToolApproval::Writes] { + assert_eq!( + normalize_approval_decision_for_mode( + McpToolApprovalDecision::AcceptForSession, + approval_mode, + ), + McpToolApprovalDecision::Accept + ); + assert_eq!( + normalize_approval_decision_for_mode( + McpToolApprovalDecision::AcceptAndRemember, + approval_mode, + ), + McpToolApprovalDecision::Accept + ); + } +} + #[tokio::test] async fn mcp_tool_call_span_records_expected_fields() { let buffer: &'static std::sync::Mutex> = diff --git a/codex-rs/core/tests/suite/mcp_turn_metadata.rs b/codex-rs/core/tests/suite/mcp_turn_metadata.rs index 0d403180d7c4..0ecaa80274f3 100644 --- a/codex-rs/core/tests/suite/mcp_turn_metadata.rs +++ b/codex-rs/core/tests/suite/mcp_turn_metadata.rs @@ -20,6 +20,7 @@ use codex_protocol::user_input::UserInput; use core_test_support::PathExt; use core_test_support::apps_test_server::AppsTestServer; use core_test_support::apps_test_server::SEARCH_CALENDAR_CREATE_TOOL; +use core_test_support::apps_test_server::SEARCH_CALENDAR_LIST_TOOL; use core_test_support::apps_test_server::SEARCH_CALENDAR_NAMESPACE; use core_test_support::apps_test_server::recorded_apps_tool_call_by_call_id; use core_test_support::apps_test_server::search_capable_apps_builder; @@ -45,6 +46,7 @@ fn set_calendar_approval_mode(config: &mut Config, approval_mode: AppToolApprova let approval_mode = match approval_mode { AppToolApproval::Auto => "auto", AppToolApproval::Prompt => "prompt", + AppToolApproval::Writes => "writes", AppToolApproval::Approve => "approve", }; let user_config_path = config.codex_home.join("config.toml").abs(); @@ -68,6 +70,7 @@ fn set_default_app_approval_mode_and_reviewer( let approval_mode = match approval_mode { AppToolApproval::Auto => "auto", AppToolApproval::Prompt => "prompt", + AppToolApproval::Writes => "writes", AppToolApproval::Approve => "approve", }; let user_config_path = config.codex_home.join("config.toml").abs(); @@ -333,6 +336,127 @@ async fn apps_default_prompt_with_auto_review_routes_actual_mcp_approval_to_guar Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apps_default_writes_prompts_for_writes_but_not_reads() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let apps_server = AppsTestServer::mount(&server).await?; + let read_call_id = "calendar-read"; + let write_call_id = "calendar-write"; + let list_args = serde_json::to_string(&json!({}))?; + let create_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-read"), + ev_function_call_with_namespace( + read_call_id, + SEARCH_CALENDAR_NAMESPACE, + SEARCH_CALENDAR_LIST_TOOL, + &list_args, + ), + ev_completed("resp-read"), + ]), + sse(vec![ + ev_response_created("resp-write"), + ev_function_call_with_namespace( + write_call_id, + SEARCH_CALENDAR_NAMESPACE, + SEARCH_CALENDAR_CREATE_TOOL, + &create_args, + ), + ev_completed("resp-write"), + ]), + sse(vec![ + ev_response_created("resp-done"), + ev_assistant_message("msg-done", "done"), + ev_completed("resp-done"), + ]), + ], + ) + .await; + + let mut builder = search_capable_apps_builder(apps_server.chatgpt_base_url.clone()) + .with_config(|config| { + config + .features + .enable(Feature::ToolCallMcpElicitation) + .expect("test config should allow feature update"); + set_default_app_approval_mode_and_reviewer( + config, + AppToolApproval::Writes, + ApprovalsReviewer::User, + ); + }); + let test = builder.build(&server).await?; + + submit_user_turn( + &test, + "Use [$calendar](app://calendar) to list events, then create one.", + AskForApproval::OnRequest, + /*collaboration_mode*/ None, + ) + .await?; + + let EventMsg::McpToolCallBegin(read_begin) = wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::McpToolCallBegin(_)) + }) + .await + else { + unreachable!("event guard guarantees McpToolCallBegin"); + }; + assert_eq!(read_begin.call_id, read_call_id); + + let next_route = wait_for_event(&test.codex, |event| { + matches!( + event, + EventMsg::McpToolCallBegin(_) | EventMsg::ElicitationRequest(_) + ) + }) + .await; + let EventMsg::McpToolCallBegin(write_begin) = next_route else { + panic!("read-only app action should not prompt in writes mode"); + }; + assert_eq!(write_begin.call_id, write_call_id); + + let EventMsg::ElicitationRequest(request) = wait_for_event(&test.codex, |event| { + matches!( + event, + EventMsg::ElicitationRequest(_) | EventMsg::TurnComplete(_) + ) + }) + .await + else { + panic!("write app action should prompt in writes mode"); + }; + + test.codex + .submit(Op::ResolveElicitation { + server_name: request.server_name, + request_id: request.id, + decision: ElicitationAction::Accept, + content: None, + meta: None, + }) + .await?; + + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + + assert_eq!(responses.requests().len(), 3); + recorded_apps_tool_call_by_call_id(&server, read_call_id).await; + recorded_apps_tool_call_by_call_id(&server, write_call_id).await; + + 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(()));