From ebc7a17f9221f4ae3588affd248089216dffc557 Mon Sep 17 00:00:00 2001 From: Charles Cunningham Date: Tue, 24 Mar 2026 20:15:50 -0700 Subject: [PATCH] Remove approval timeout message shim Co-authored-by: Codex --- codex-rs/core/src/approval_review.rs | 4 ---- codex-rs/core/src/guardian/mod.rs | 1 + codex-rs/core/src/guardian/review.rs | 8 ++++++-- codex-rs/core/src/lib.rs | 1 - codex-rs/core/src/mcp_tool_call.rs | 6 +++--- codex-rs/core/src/mcp_tool_call_tests.rs | 6 +++--- codex-rs/core/src/tools/network_approval.rs | 6 +++--- codex-rs/core/src/tools/orchestrator.rs | 10 +++------- .../core/src/tools/runtimes/shell/unix_escalation.rs | 8 ++++---- 9 files changed, 23 insertions(+), 27 deletions(-) delete mode 100644 codex-rs/core/src/approval_review.rs diff --git a/codex-rs/core/src/approval_review.rs b/codex-rs/core/src/approval_review.rs deleted file mode 100644 index e3c857f4a6..0000000000 --- a/codex-rs/core/src/approval_review.rs +++ /dev/null @@ -1,4 +0,0 @@ -pub(crate) const APPROVAL_REVIEW_TIMEOUT_MESSAGE: &str = concat!( - "Automatic approval review timed out before it could finish evaluating this action. ", - "The action was not intentionally rejected, and a retry may succeed.", -); diff --git a/codex-rs/core/src/guardian/mod.rs b/codex-rs/core/src/guardian/mod.rs index 9163f7a38a..fc0d02be89 100644 --- a/codex-rs/core/src/guardian/mod.rs +++ b/codex-rs/core/src/guardian/mod.rs @@ -25,6 +25,7 @@ pub(crate) use approval_request::GuardianApprovalRequest; pub(crate) use approval_request::GuardianMcpAnnotations; pub(crate) use approval_request::guardian_approval_request_to_json; pub(crate) use review::GUARDIAN_REJECTION_MESSAGE; +pub(crate) use review::GUARDIAN_TIMEOUT_MESSAGE; pub(crate) use review::GuardianApprovalDecision; pub(crate) use review::is_guardian_reviewer_source; pub(crate) use review::review_approval_request; diff --git a/codex-rs/core/src/guardian/review.rs b/codex-rs/core/src/guardian/review.rs index 7d623f7ba1..f0ce8413b9 100644 --- a/codex-rs/core/src/guardian/review.rs +++ b/codex-rs/core/src/guardian/review.rs @@ -11,7 +11,6 @@ use codex_protocol::protocol::SubAgentSource; use codex_protocol::protocol::WarningEvent; use tokio_util::sync::CancellationToken; -use crate::approval_review::APPROVAL_REVIEW_TIMEOUT_MESSAGE; use crate::codex::Session; use crate::codex::TurnContext; @@ -38,6 +37,11 @@ pub(crate) const GUARDIAN_REJECTION_MESSAGE: &str = concat!( "Otherwise, stop and request user input.", ); +pub(crate) const GUARDIAN_TIMEOUT_MESSAGE: &str = concat!( + "Automatic approval review timed out before it could finish evaluating this action. ", + "The action was not intentionally rejected, and a retry may succeed.", +); + #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) enum GuardianApprovalDecision { Approved, @@ -177,7 +181,7 @@ async fn run_guardian_review( format!("Automatic approval review failed: {err}"), ), GuardianReviewOutcome::TimedOut => { - let warning_event_message = APPROVAL_REVIEW_TIMEOUT_MESSAGE.to_string(); + let warning_event_message = GUARDIAN_TIMEOUT_MESSAGE.to_string(); session .send_event( turn.as_ref(), diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 0df944e174..decebc84e0 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -8,7 +8,6 @@ mod analytics_client; pub mod api_bridge; mod apply_patch; -mod approval_review; mod apps; mod arc_monitor; pub use codex_login as auth; diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index e2219c327d..9d8a7acbe7 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -11,7 +11,6 @@ use tracing::error; use crate::analytics_client::AppInvocation; use crate::analytics_client::InvocationType; use crate::analytics_client::build_track_events_context; -use crate::approval_review::APPROVAL_REVIEW_TIMEOUT_MESSAGE; use crate::arc_monitor::ArcMonitorOutcome; use crate::arc_monitor::monitor_action; use crate::codex::Session; @@ -20,6 +19,7 @@ use crate::config::edit::ConfigEdit; use crate::config::edit::ConfigEditsBuilder; use crate::config::types::AppToolApproval; use crate::connectors; +use crate::guardian::GUARDIAN_TIMEOUT_MESSAGE; use crate::guardian::GuardianApprovalDecision; use crate::guardian::GuardianApprovalRequest; use crate::guardian::GuardianMcpAnnotations; @@ -768,7 +768,7 @@ fn mcp_tool_approval_decision_from_guardian( McpToolApprovalDecision::Decline } GuardianApprovalDecision::TimedOut => { - McpToolApprovalDecision::TimedOut(APPROVAL_REVIEW_TIMEOUT_MESSAGE.to_string()) + McpToolApprovalDecision::TimedOut(GUARDIAN_TIMEOUT_MESSAGE.to_string()) } } } @@ -1200,7 +1200,7 @@ fn parse_mcp_tool_approval_response( .iter() .any(|answer| answer == MCP_TOOL_APPROVAL_TIMED_OUT_SYNTHETIC) { - McpToolApprovalDecision::TimedOut(APPROVAL_REVIEW_TIMEOUT_MESSAGE.to_string()) + McpToolApprovalDecision::TimedOut(GUARDIAN_TIMEOUT_MESSAGE.to_string()) } else if answers .iter() .any(|answer| answer == MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION) diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index b3c20b8244..8531000738 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -1,5 +1,4 @@ use super::*; -use crate::approval_review::APPROVAL_REVIEW_TIMEOUT_MESSAGE; use crate::codex::make_session_and_context; use crate::config::ApprovalsReviewer; use crate::config::ConfigToml; @@ -7,6 +6,7 @@ use crate::config::types::AppConfig; use crate::config::types::AppToolConfig; use crate::config::types::AppToolsConfig; use crate::config::types::AppsConfigToml; +use crate::guardian::GUARDIAN_TIMEOUT_MESSAGE; use crate::guardian::GuardianApprovalDecision; use codex_config::CONFIG_TOML_FILE; use core_test_support::responses::ev_assistant_message; @@ -708,7 +708,7 @@ fn guardian_review_decision_maps_to_mcp_tool_decision() { ); assert_eq!( mcp_tool_approval_decision_from_guardian(GuardianApprovalDecision::TimedOut), - McpToolApprovalDecision::TimedOut(APPROVAL_REVIEW_TIMEOUT_MESSAGE.to_string()) + McpToolApprovalDecision::TimedOut(GUARDIAN_TIMEOUT_MESSAGE.to_string()) ); } @@ -863,7 +863,7 @@ fn synthetic_timeout_request_user_input_response_stays_timed_out() { assert_eq!( response, - McpToolApprovalDecision::TimedOut(APPROVAL_REVIEW_TIMEOUT_MESSAGE.to_string()) + McpToolApprovalDecision::TimedOut(GUARDIAN_TIMEOUT_MESSAGE.to_string()) ); } diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs index f50e1a7f51..e467e525a5 100644 --- a/codex-rs/core/src/tools/network_approval.rs +++ b/codex-rs/core/src/tools/network_approval.rs @@ -1,6 +1,6 @@ -use crate::approval_review::APPROVAL_REVIEW_TIMEOUT_MESSAGE; use crate::codex::Session; use crate::guardian::GUARDIAN_REJECTION_MESSAGE; +use crate::guardian::GUARDIAN_TIMEOUT_MESSAGE; use crate::guardian::GuardianApprovalDecision; use crate::guardian::GuardianApprovalRequest; use crate::guardian::review_approval_request; @@ -393,7 +393,7 @@ impl NetworkApprovalService { self.record_call_outcome( &owner_call.registration_id, NetworkApprovalOutcome::TimedOutByReviewer( - APPROVAL_REVIEW_TIMEOUT_MESSAGE.to_string(), + GUARDIAN_TIMEOUT_MESSAGE.to_string(), ), ) .await; @@ -520,7 +520,7 @@ impl NetworkApprovalService { self.record_call_outcome( &owner_call.registration_id, NetworkApprovalOutcome::TimedOutByReviewer( - APPROVAL_REVIEW_TIMEOUT_MESSAGE.to_string(), + GUARDIAN_TIMEOUT_MESSAGE.to_string(), ), ) .await; diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index 5ed8fe3376..dc49a2e8b8 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -6,11 +6,11 @@ simple sequence for any ToolRuntime: approval → select sandbox → attempt → retry with an escalated sandbox strategy on denial (no re‑approval thanks to caching). */ -use crate::approval_review::APPROVAL_REVIEW_TIMEOUT_MESSAGE; use crate::error::CodexErr; use crate::error::SandboxErr; use crate::exec::ExecToolCallOutput; use crate::guardian::GUARDIAN_REJECTION_MESSAGE; +use crate::guardian::GUARDIAN_TIMEOUT_MESSAGE; use crate::guardian::routes_approval_to_guardian; use crate::network_policy_decision::network_approval_context_from_payload; use crate::tools::network_approval::DeferredNetworkApproval; @@ -163,9 +163,7 @@ impl ToolOrchestrator { match approval_outcome { ApprovalOutcome::TimedOut => { - return Err(ToolError::Message( - APPROVAL_REVIEW_TIMEOUT_MESSAGE.to_string(), - )); + return Err(ToolError::Message(GUARDIAN_TIMEOUT_MESSAGE.to_string())); } ApprovalOutcome::Decision { decision: ReviewDecision::Denied | ReviewDecision::Abort, @@ -324,9 +322,7 @@ impl ToolOrchestrator { match approval_outcome { ApprovalOutcome::TimedOut => { - return Err(ToolError::Message( - APPROVAL_REVIEW_TIMEOUT_MESSAGE.to_string(), - )); + return Err(ToolError::Message(GUARDIAN_TIMEOUT_MESSAGE.to_string())); } ApprovalOutcome::Decision { decision: ReviewDecision::Denied | ReviewDecision::Abort, diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index 79e2e766fb..303ab48465 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -1,5 +1,4 @@ use super::ShellRequest; -use crate::approval_review::APPROVAL_REVIEW_TIMEOUT_MESSAGE; use crate::error::CodexErr; use crate::error::SandboxErr; use crate::exec::ExecCapturePolicy; @@ -7,6 +6,7 @@ use crate::exec::ExecExpiration; use crate::exec::ExecToolCallOutput; use crate::exec::is_likely_sandbox_denied; use crate::guardian::GUARDIAN_REJECTION_MESSAGE; +use crate::guardian::GUARDIAN_TIMEOUT_MESSAGE; use crate::guardian::GuardianApprovalDecision; use crate::guardian::GuardianApprovalRequest; use crate::guardian::review_approval_request; @@ -549,9 +549,9 @@ impl CoreShellActionProvider { EscalationDecision::run() } } - GuardianApprovalDecision::TimedOut => EscalationDecision::deny(Some( - APPROVAL_REVIEW_TIMEOUT_MESSAGE.to_string(), - )), + GuardianApprovalDecision::TimedOut => { + EscalationDecision::deny(Some(GUARDIAN_TIMEOUT_MESSAGE.to_string())) + } GuardianApprovalDecision::Denied => { EscalationDecision::deny(Some(GUARDIAN_REJECTION_MESSAGE.to_string())) }