From c46b1a8b1d2dff3a3a56f1820d562c0c19069922 Mon Sep 17 00:00:00 2001
From: Katia Bazzi <kbazzi@openai.com>
Date: Wed, 10 Jun 2026 17:39:00 -0700
Subject: [PATCH] codex: simplify guardian timeout fallback approval flow

---
 codex-rs/core/src/tools/orchestrator.rs       | 72 +++++++------------
 codex-rs/core/src/tools/orchestrator_tests.rs |  4 +-
 2 files changed, 26 insertions(+), 50 deletions(-)

diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs
index d736de0aba..2e8673eab2 100644
--- a/codex-rs/core/src/tools/orchestrator.rs
+++ b/codex-rs/core/src/tools/orchestrator.rs
@@ -176,7 +176,7 @@ impl ToolOrchestrator {
                         retry_reason: None,
                         network_approval_context: None,
                     };
-                    let approval_decision = Self::request_approval_with_manual_fallback(
+                    let approval_decision = Self::request_approval(
                         tool,
                         req,
                         tool_ctx.call_id.as_str(),
@@ -218,7 +218,7 @@ impl ToolOrchestrator {
                     retry_reason: reason.clone(),
                     network_approval_context: None,
                 };
-                let approval_decision = Self::request_approval_with_manual_fallback(
+                let approval_decision = Self::request_approval(
                     tool,
                     req,
                     tool_ctx.call_id.as_str(),
@@ -407,7 +407,7 @@ impl ToolOrchestrator {
                     };
 
                     let permission_request_run_id = format!("{}:retry", tool_ctx.call_id);
-                    let approval_decision = Self::request_approval_with_manual_fallback(
+                    let approval_decision = Self::request_approval(
                         tool,
                         req,
                         &permission_request_run_id,
@@ -520,13 +520,13 @@ impl ToolOrchestrator {
         permission_request_run_id: &str,
         approval_ctx: ApprovalCtx<'_>,
         tool_ctx: &ToolCtx,
-        evaluate_permission_request_hooks: bool,
+        options: ApprovalRequestOptions,
         otel: &codex_otel::SessionTelemetry,
-    ) -> Result<ReviewDecision, ToolError>
+    ) -> Result<ApprovalDecision, ToolError>
     where
         T: ToolRuntime<Rq, Out>,
     {
-        if evaluate_permission_request_hooks
+        if options.evaluate_permission_request_hooks
             && let Some(permission_request) = tool.permission_request_payload(req)
         {
             let tool_name = flat_tool_name(&tool_ctx.tool_name);
@@ -546,7 +546,10 @@ impl ToolOrchestrator {
                         &decision,
                         ToolDecisionSource::Config,
                     );
-                    return Ok(decision);
+                    return Ok(ApprovalDecision {
+                        decision,
+                        guardian_review_id: None,
+                    });
                 }
                 Some(PermissionRequestDecision::Deny { message }) => {
                     let decision = ReviewDecision::Denied;
@@ -562,7 +565,12 @@ impl ToolOrchestrator {
             }
         }
 
-        let otel_source = if approval_ctx.guardian_review_id.is_some() {
+        let guardian_review_id = approval_ctx.guardian_review_id.clone();
+        let session = approval_ctx.session;
+        let turn = approval_ctx.turn;
+        let call_id = approval_ctx.call_id;
+        let network_approval_context = approval_ctx.network_approval_context.clone();
+        let otel_source = if guardian_review_id.is_some() {
             ToolDecisionSource::AutomatedReviewer
         } else {
             ToolDecisionSource::User
@@ -575,36 +583,6 @@ impl ToolOrchestrator {
             &decision,
             otel_source,
         );
-        Ok(decision)
-    }
-
-    async fn request_approval_with_manual_fallback<Rq, Out, T>(
-        tool: &mut T,
-        req: &Rq,
-        permission_request_run_id: &str,
-        approval_ctx: ApprovalCtx<'_>,
-        tool_ctx: &ToolCtx,
-        options: ApprovalRequestOptions,
-        otel: &codex_otel::SessionTelemetry,
-    ) -> Result<ApprovalDecision, ToolError>
-    where
-        T: ToolRuntime<Rq, Out>,
-    {
-        let guardian_review_id = approval_ctx.guardian_review_id.clone();
-        let session = approval_ctx.session;
-        let turn = approval_ctx.turn;
-        let call_id = approval_ctx.call_id;
-        let network_approval_context = approval_ctx.network_approval_context.clone();
-        let decision = Self::request_approval(
-            tool,
-            req,
-            permission_request_run_id,
-            approval_ctx,
-            tool_ctx,
-            options.evaluate_permission_request_hooks,
-            otel,
-        )
-        .await?;
 
         if !options.manual_fallback_for_guardian_timeout
             || !matches!(decision, ReviewDecision::TimedOut)
@@ -624,16 +602,14 @@ impl ToolOrchestrator {
             retry_reason: Some(guardian_timeout_message()),
             network_approval_context,
         };
-        let decision = Self::request_approval(
-            tool,
-            req,
-            permission_request_run_id,
-            fallback_ctx,
-            tool_ctx,
-            /*evaluate_permission_request_hooks*/ false,
-            otel,
-        )
-        .await?;
+        let decision = tool.start_approval_async(req, fallback_ctx).await;
+        let tool_name = flat_tool_name(&tool_ctx.tool_name);
+        otel.tool_decision(
+            tool_name.as_ref(),
+            &tool_ctx.call_id,
+            &decision,
+            ToolDecisionSource::User,
+        );
         Ok(ApprovalDecision {
             decision,
             guardian_review_id: None,
diff --git a/codex-rs/core/src/tools/orchestrator_tests.rs b/codex-rs/core/src/tools/orchestrator_tests.rs
index e03ff1aeed..2e612772f2 100644
--- a/codex-rs/core/src/tools/orchestrator_tests.rs
+++ b/codex-rs/core/src/tools/orchestrator_tests.rs
@@ -86,7 +86,7 @@ async fn guardian_timeout_falls_back_to_manual_approval() {
     };
     let mut runtime = TimeoutThenManualRuntime::default();
 
-    let approval_decision = ToolOrchestrator::request_approval_with_manual_fallback(
+    let approval_decision = ToolOrchestrator::request_approval(
         &mut runtime,
         &(),
         "permission-request-1",
@@ -145,7 +145,7 @@ async fn guardian_timeout_stays_terminal_when_manual_fallback_is_disabled() {
     };
     let mut runtime = TimeoutThenManualRuntime::default();
 
-    let approval_decision = ToolOrchestrator::request_approval_with_manual_fallback(
+    let approval_decision = ToolOrchestrator::request_approval(
         &mut runtime,
         &(),
         "permission-request-1",