Revert "Simplify approval routing abstraction"

Co-authored-by: Codex <noreply@openai.com>

codex-rs/core/src/codex.rs

@@ -308,6 +308,7 @@ use crate::tasks::ReviewTask;
 use crate::tasks::SessionTask;
 use crate::tasks::SessionTaskContext;
 use crate::tools::ToolRouter;
+use crate::tools::approval::ApprovalStore;
 use crate::tools::context::SharedTurnDiffTracker;
 use crate::tools::js_repl::JsReplHandle;
 use crate::tools::js_repl::resolve_compatible_node;
@@ -316,7 +317,6 @@ use crate::tools::network_approval::build_blocked_request_observer;
 use crate::tools::network_approval::build_network_policy_decider;
 use crate::tools::parallel::ToolCallRuntime;
 use crate::tools::router::ToolRouterParams;
-use crate::tools::sandboxing::ApprovalStore;
 use crate::turn_diff_tracker::TurnDiffTracker;
 use crate::turn_timing::TurnTimingState;
 use crate::turn_timing::record_turn_ttfm_metric;

codex-rs/core/src/state/service.rs

@@ -11,9 +11,9 @@ use crate::guardian::GuardianRejection;
 use crate::mcp::McpManager;
 use crate::plugins::PluginsManager;
 use crate::skills_watcher::SkillsWatcher;
+use crate::tools::approval::ApprovalStore;
 use crate::tools::code_mode::CodeModeService;
 use crate::tools::network_approval::NetworkApprovalService;
-use crate::tools::sandboxing::ApprovalStore;
 use crate::unified_exec::UnifiedExecProcessManager;
 use codex_analytics::AnalyticsEventsClient;
 use codex_exec_server::Environment;

codex-rs/core/src/tools/approval.rs

@@ -0,0 +1,261 @@
+//! Shared approval coordination primitives used across tool and non-tool flows.
+//!
+//! This module centralizes:
+//! - session-scoped approval caching for "approve for session" decisions
+//! - routing approval prompts to the user or guardian reviewer
+//! - dispatching typed user/guardian approval prompts
+//! - returning prompt metadata needed by caller-specific result handling
+
+use crate::codex::Session;
+use crate::codex::TurnContext;
+use crate::guardian::GuardianApprovalRequest;
+use crate::guardian::new_guardian_review_id;
+use crate::guardian::review_approval_request;
+use crate::guardian::routes_approval_to_guardian;
+use crate::state::SessionServices;
+use codex_protocol::approvals::ExecPolicyAmendment;
+use codex_protocol::approvals::NetworkApprovalContext;
+use codex_protocol::models::PermissionProfile;
+use codex_protocol::protocol::FileChange;
+use codex_protocol::protocol::ReviewDecision;
+use futures::Future;
+use serde::Serialize;
+use std::collections::HashMap;
+use std::path::PathBuf;
+use std::sync::Arc;
+
+#[derive(Clone, Default, Debug)]
+pub(crate) struct ApprovalStore {
+    map: HashMap<String, ReviewDecision>,
+}
+
+impl ApprovalStore {
+    pub fn get<K>(&self, key: &K) -> Option<ReviewDecision>
+    where
+        K: Serialize,
+    {
+        let s = serde_json::to_string(key).ok()?;
+        self.map.get(&s).cloned()
+    }
+
+    pub fn put<K>(&mut self, key: K, value: ReviewDecision)
+    where
+        K: Serialize,
+    {
+        if let Ok(s) = serde_json::to_string(&key) {
+            self.map.insert(s, value);
+        }
+    }
+}
+
+#[derive(Debug)]
+pub(crate) enum ApprovalCache<K> {
+    None,
+    SessionApproveOnly {
+        tool_name: &'static str,
+        keys: Vec<K>,
+    },
+}
+
+#[derive(Debug)]
+pub(crate) struct ApprovalOutcome {
+    pub decision: ReviewDecision,
+    pub guardian_review_id: Option<String>,
+}
+
+#[derive(Debug)]
+pub(crate) struct CommandApprovalRequest {
+    pub call_id: String,
+    pub approval_id: Option<String>,
+    pub command: Vec<String>,
+    pub cwd: PathBuf,
+    pub reason: Option<String>,
+    pub network_approval_context: Option<NetworkApprovalContext>,
+    pub proposed_execpolicy_amendment: Option<ExecPolicyAmendment>,
+    pub additional_permissions: Option<PermissionProfile>,
+    pub available_decisions: Option<Vec<ReviewDecision>>,
+}
+
+#[derive(Debug)]
+pub(crate) struct PatchApprovalRequest {
+    pub call_id: String,
+    pub changes: HashMap<PathBuf, FileChange>,
+    pub reason: Option<String>,
+    pub grant_root: Option<PathBuf>,
+}
+
+#[derive(Debug)]
+pub(crate) enum UserApprovalRequest {
+    Command(CommandApprovalRequest),
+    Patch(PatchApprovalRequest),
+}
+
+#[derive(Debug)]
+pub(crate) struct GuardianApproval {
+    pub request: GuardianApprovalRequest,
+    pub retry_reason: Option<String>,
+}
+
+impl GuardianApproval {
+    pub(crate) fn new(request: GuardianApprovalRequest, retry_reason: Option<String>) -> Self {
+        Self {
+            request,
+            retry_reason,
+        }
+    }
+}
+
+#[derive(Debug)]
+pub(crate) struct ApprovalPlan<K> {
+    pub cache: ApprovalCache<K>,
+    pub user: UserApprovalRequest,
+    pub guardian: GuardianApproval,
+}
+
+pub(crate) fn guardian_review_id_for_turn(turn: &crate::codex::TurnContext) -> Option<String> {
+    routes_approval_to_guardian(turn).then(new_guardian_review_id)
+}
+
+/// Takes a vector of approval keys and returns a ReviewDecision.
+/// There will be one key in most cases, but apply_patch can modify multiple files at once.
+///
+/// - If all keys are already approved for session, we skip prompting.
+/// - If the user approves for session, we store the decision for each key individually
+///   so future requests touching any subset can also skip prompting.
+pub(crate) async fn with_cached_approval<K, F, Fut>(
+    services: &SessionServices,
+    tool_name: &str,
+    keys: Vec<K>,
+    fetch: F,
+) -> ReviewDecision
+where
+    K: Serialize,
+    F: FnOnce() -> Fut,
+    Fut: Future<Output = ReviewDecision>,
+{
+    if keys.is_empty() {
+        return fetch().await;
+    }
+
+    let already_approved = {
+        let store = services.tool_approvals.lock().await;
+        keys.iter()
+            .all(|key| matches!(store.get(key), Some(ReviewDecision::ApprovedForSession)))
+    };
+
+    if already_approved {
+        return ReviewDecision::ApprovedForSession;
+    }
+
+    let decision = fetch().await;
+
+    services.session_telemetry.counter(
+        "codex.approval.requested",
+        /*inc*/ 1,
+        &[
+            ("tool", tool_name),
+            ("approved", decision.to_opaque_string()),
+        ],
+    );
+
+    if matches!(decision, ReviewDecision::ApprovedForSession) {
+        let mut store = services.tool_approvals.lock().await;
+        for key in keys {
+            store.put(key, ReviewDecision::ApprovedForSession);
+        }
+    }
+
+    decision
+}
+
+async fn dispatch_user_approval(
+    session: &Arc<Session>,
+    turn: &Arc<TurnContext>,
+    request: UserApprovalRequest,
+) -> ReviewDecision {
+    match request {
+        UserApprovalRequest::Command(request) => {
+            session
+                .request_command_approval(
+                    turn.as_ref(),
+                    request.call_id,
+                    request.approval_id,
+                    request.command,
+                    request.cwd,
+                    request.reason,
+                    request.network_approval_context,
+                    request.proposed_execpolicy_amendment,
+                    request.additional_permissions,
+                    request.available_decisions,
+                )
+                .await
+        }
+        UserApprovalRequest::Patch(request) => {
+            let rx_approve = session
+                .request_patch_approval(
+                    turn.as_ref(),
+                    request.call_id,
+                    request.changes,
+                    request.reason,
+                    request.grant_root,
+                )
+                .await;
+            rx_approve.await.unwrap_or_default()
+        }
+    }
+}
+
+pub(crate) async fn request_approval<K>(
+    session: &Arc<Session>,
+    turn: &Arc<TurnContext>,
+    guardian_review_id: Option<String>,
+    plan: ApprovalPlan<K>,
+) -> ApprovalOutcome
+where
+    K: Serialize,
+{
+    let ApprovalPlan {
+        cache,
+        user,
+        guardian,
+    } = plan;
+    if let Some(review_id) = guardian_review_id.clone() {
+        return ApprovalOutcome {
+            decision: review_approval_request(
+                session,
+                turn,
+                review_id,
+                guardian.request,
+                guardian.retry_reason,
+            )
+            .await,
+            guardian_review_id,
+        };
+    }
+
+    let decision = match cache {
+        ApprovalCache::None => dispatch_user_approval(session, turn, user).await,
+        ApprovalCache::SessionApproveOnly { tool_name, keys } => {
+            with_cached_approval(&session.services, tool_name, keys, || {
+                dispatch_user_approval(session, turn, user)
+            })
+            .await
+        }
+    };
+
+    ApprovalOutcome {
+        decision,
+        guardian_review_id: None,
+    }
+}
+
+pub(crate) async fn request_approval_for_turn<K>(
+    session: &Arc<Session>,
+    turn: &Arc<TurnContext>,
+    plan: ApprovalPlan<K>,
+) -> ApprovalOutcome
+where
+    K: Serialize,
+{
+    request_approval(session, turn, guardian_review_id_for_turn(turn), plan).await
+}

codex-rs/core/src/tools/mod.rs

@@ -1,3 +1,4 @@
+pub(crate) mod approval;
 pub(crate) mod code_mode;
 pub(crate) mod context;
 pub(crate) mod events;

codex-rs/core/src/tools/network_approval.rs

@@ -2,10 +2,13 @@ use crate::codex::Session;
 use crate::guardian::GuardianApprovalRequest;
 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::network_policy_decision::denied_network_policy_message;
+use crate::tools::approval::ApprovalCache;
+use crate::tools::approval::ApprovalPlan;
+use crate::tools::approval::CommandApprovalRequest;
+use crate::tools::approval::GuardianApproval;
+use crate::tools::approval::UserApprovalRequest;
+use crate::tools::approval::request_approval_for_turn;
 use crate::tools::sandboxing::ToolError;
 use codex_network_proxy::BlockedRequest;
 use codex_network_proxy::BlockedRequestObserver;
@@ -370,46 +373,41 @@ impl NetworkApprovalService {
             protocol,
         };
         let owner_call = self.resolve_single_active_call().await;
-        let guardian_approval_id = Self::approval_id_for_key(&key);
-        let use_guardian = routes_approval_to_guardian(&turn_context);
-        let guardian_review_id = use_guardian.then(new_guardian_review_id);
-        let approval_decision = if let Some(review_id) = guardian_review_id.clone() {
-            review_approval_request(
-                &session,
-                &turn_context,
-                review_id,
-                GuardianApprovalRequest::NetworkAccess {
-                    id: guardian_approval_id.clone(),
-                    turn_id: owner_call
-                        .as_ref()
-                        .map_or_else(|| turn_context.sub_id.clone(), |call| call.turn_id.clone()),
-                    target,
-                    host: request.host,
-                    protocol,
-                    port: key.port,
-                },
-                Some(policy_denial_message.clone()),
-            )
-            .await
-        } else {
-            let approval_id = Self::approval_id_for_key(&key);
-            let prompt_command = vec!["network-access".to_string(), target.clone()];
-            let available_decisions = None;
-            session
-                .request_command_approval(
-                    turn_context.as_ref(),
-                    approval_id,
-                    /*approval_id*/ None,
-                    prompt_command,
-                    turn_context.cwd.to_path_buf(),
-                    Some(prompt_reason),
-                    Some(network_approval_context.clone()),
-                    /*proposed_execpolicy_amendment*/ None,
-                    /*additional_permissions*/ None,
-                    available_decisions,
-                )
-                .await
-        };
+        let approval_outcome = request_approval_for_turn(
+            &session,
+            &turn_context,
+            ApprovalPlan {
+                cache: ApprovalCache::<String>::None,
+                user: UserApprovalRequest::Command(CommandApprovalRequest {
+                    call_id: Self::approval_id_for_key(&key),
+                    approval_id: None,
+                    command: vec!["network-access".to_string(), target.clone()],
+                    cwd: turn_context.cwd.to_path_buf(),
+                    reason: Some(prompt_reason),
+                    network_approval_context: Some(network_approval_context.clone()),
+                    proposed_execpolicy_amendment: None,
+                    additional_permissions: None,
+                    available_decisions: None,
+                }),
+                guardian: GuardianApproval::new(
+                    GuardianApprovalRequest::NetworkAccess {
+                        id: Self::approval_id_for_key(&key),
+                        turn_id: owner_call.as_ref().map_or_else(
+                            || turn_context.sub_id.clone(),
+                            |call| call.turn_id.clone(),
+                        ),
+                        target,
+                        host: request.host,
+                        protocol,
+                        port: key.port,
+                    },
+                    Some(policy_denial_message.clone()),
+                ),
+            },
+        )
+        .await;
+        let guardian_review_id = approval_outcome.guardian_review_id;
+        let approval_decision = approval_outcome.decision;
 
         let mut cache_session_deny = false;
         let resolved = match approval_decision {

codex-rs/core/src/tools/runtimes/apply_patch.rs

@@ -7,9 +7,14 @@
 //! `SandboxAttempt` with a minimal environment for local turns.
 use crate::exec::ExecCapturePolicy;
 use crate::guardian::GuardianApprovalRequest;
-use crate::guardian::review_approval_request;
 use crate::sandboxing::ExecOptions;
 use crate::sandboxing::execute_env;
+use crate::tools::approval::ApprovalCache;
+use crate::tools::approval::ApprovalPlan;
+use crate::tools::approval::GuardianApproval;
+use crate::tools::approval::PatchApprovalRequest;
+use crate::tools::approval::UserApprovalRequest;
+use crate::tools::approval::request_approval;
 use crate::tools::sandboxing::Approvable;
 use crate::tools::sandboxing::ApprovalCtx;
 use crate::tools::sandboxing::ExecApprovalRequirement;
@@ -18,7 +23,6 @@ use crate::tools::sandboxing::Sandboxable;
 use crate::tools::sandboxing::ToolCtx;
 use crate::tools::sandboxing::ToolError;
 use crate::tools::sandboxing::ToolRuntime;
-use crate::tools::sandboxing::with_cached_approval;
 use codex_apply_patch::ApplyPatchAction;
 use codex_apply_patch::CODEX_CORE_APPLY_PATCH_ARG1;
 use codex_protocol::exec_output::ExecToolCallOutput;
@@ -146,43 +150,56 @@ impl Approvable<ApplyPatchRequest> for ApplyPatchRuntime {
         let retry_reason = ctx.retry_reason.clone();
         let approval_keys = self.approval_keys(req);
         let changes = req.changes.clone();
-        let guardian_review_id = ctx.guardian_review_id.clone();
         Box::pin(async move {
             if req.permissions_preapproved && retry_reason.is_none() {
                 return ReviewDecision::Approved;
             }
-            if let Some(review_id) = guardian_review_id {
-                let action = ApplyPatchRuntime::build_guardian_review_request(req, ctx.call_id);
-                return review_approval_request(session, turn, review_id, action, retry_reason)
-                    .await;
-            }
-            if let Some(reason) = retry_reason {
-                let rx_approve = session
-                    .request_patch_approval(
-                        turn,
-                        call_id,
-                        changes.clone(),
-                        Some(reason),
-                        /*grant_root*/ None,
-                    )
-                    .await;
-                return rx_approve.await.unwrap_or_default();
+            if let Some(reason) = retry_reason.clone() {
+                return request_approval(
+                    session,
+                    turn,
+                    ctx.guardian_review_id.clone(),
+                    ApprovalPlan {
+                        cache: ApprovalCache::<AbsolutePathBuf>::None,
+                        user: UserApprovalRequest::Patch(PatchApprovalRequest {
+                            call_id: call_id.clone(),
+                            changes: changes.clone(),
+                            reason: Some(reason),
+                            grant_root: None,
+                        }),
+                        guardian: GuardianApproval::new(
+                            ApplyPatchRuntime::build_guardian_review_request(req, &call_id),
+                            retry_reason,
+                        ),
+                    },
+                )
+                .await
+                .decision;
             }
 
-            with_cached_approval(
-                &session.services,
-                "apply_patch",
-                approval_keys,
-                || async move {
-                    let rx_approve = session
-                        .request_patch_approval(
-                            turn, call_id, changes, /*reason*/ None, /*grant_root*/ None,
-                        )
-                        .await;
-                    rx_approve.await.unwrap_or_default()
+            request_approval(
+                session,
+                turn,
+                ctx.guardian_review_id.clone(),
+                ApprovalPlan {
+                    cache: ApprovalCache::SessionApproveOnly {
+                        tool_name: "apply_patch",
+                        keys: approval_keys,
+                    },
+                    user: UserApprovalRequest::Patch(PatchApprovalRequest {
+                        call_id: call_id.clone(),
+                        changes,
+                        reason: None,
+                        grant_root: None,
+                    }),
+                    guardian: GuardianApproval::new(
+                        ApplyPatchRuntime::build_guardian_review_request(req, ctx.call_id),
+                        retry_reason,
+                    ),
                 },
             )
             .await
+            .decision
         })
     }
 

codex-rs/core/src/tools/runtimes/shell.rs

@@ -11,11 +11,16 @@ pub(crate) mod zsh_fork_backend;
 use crate::command_canonicalization::canonicalize_command_for_approval;
 use crate::exec::ExecCapturePolicy;
 use crate::guardian::GuardianApprovalRequest;
-use crate::guardian::review_approval_request;
 use crate::sandboxing::ExecOptions;
 use crate::sandboxing::SandboxPermissions;
 use crate::sandboxing::execute_env;
 use crate::shell::ShellType;
+use crate::tools::approval::ApprovalCache;
+use crate::tools::approval::ApprovalPlan;
+use crate::tools::approval::CommandApprovalRequest;
+use crate::tools::approval::GuardianApproval;
+use crate::tools::approval::UserApprovalRequest;
+use crate::tools::approval::request_approval;
 use crate::tools::network_approval::NetworkApprovalMode;
 use crate::tools::network_approval::NetworkApprovalSpec;
 use crate::tools::runtimes::build_sandbox_command;
@@ -30,7 +35,6 @@ use crate::tools::sandboxing::ToolCtx;
 use crate::tools::sandboxing::ToolError;
 use crate::tools::sandboxing::ToolRuntime;
 use crate::tools::sandboxing::sandbox_override_for_first_attempt;
-use crate::tools::sandboxing::with_cached_approval;
 use codex_network_proxy::NetworkProxy;
 use codex_protocol::exec_output::ExecToolCallOutput;
 use codex_protocol::models::PermissionProfile;
@@ -151,45 +155,45 @@ impl Approvable<ShellRequest> for ShellRuntime {
         let session = ctx.session;
         let turn = ctx.turn;
         let call_id = ctx.call_id.to_string();
-        let guardian_review_id = ctx.guardian_review_id.clone();
         Box::pin(async move {
-            if let Some(review_id) = guardian_review_id {
-                return review_approval_request(
-                    session,
-                    turn,
-                    review_id,
-                    GuardianApprovalRequest::Shell {
-                        id: call_id,
-                        command,
-                        cwd,
-                        sandbox_permissions: req.sandbox_permissions,
-                        additional_permissions: req.additional_permissions.clone(),
-                        justification: req.justification.clone(),
+            request_approval(
+                session,
+                turn,
+                ctx.guardian_review_id.clone(),
+                ApprovalPlan {
+                    cache: ApprovalCache::SessionApproveOnly {
+                        tool_name: "shell",
+                        keys,
                     },
-                    retry_reason,
-                )
-                .await;
-            }
-            with_cached_approval(&session.services, "shell", keys, move || async move {
-                let available_decisions = None;
-                session
-                    .request_command_approval(
-                        turn,
-                        call_id,
-                        /*approval_id*/ None,
-                        command,
-                        cwd,
+                    user: UserApprovalRequest::Command(CommandApprovalRequest {
+                        call_id: call_id.clone(),
+                        approval_id: None,
+                        command: command.clone(),
+                        cwd: cwd.clone(),
                         reason,
-                        ctx.network_approval_context.clone(),
-                        req.exec_approval_requirement
+                        network_approval_context: ctx.network_approval_context.clone(),
+                        proposed_execpolicy_amendment: req
+                            .exec_approval_requirement
                             .proposed_execpolicy_amendment()
                             .cloned(),
-                        req.additional_permissions.clone(),
-                        available_decisions,
-                    )
-                    .await
-            })
+                        additional_permissions: req.additional_permissions.clone(),
+                        available_decisions: None,
+                    }),
+                    guardian: GuardianApproval::new(
+                        GuardianApprovalRequest::Shell {
+                            id: call_id,
+                            command,
+                            cwd,
+                            sandbox_permissions: req.sandbox_permissions,
+                            additional_permissions: req.additional_permissions.clone(),
+                            justification: req.justification.clone(),
+                        },
+                        retry_reason,
+                    ),
+                },
+            )
             .await
+            .decision
         })
     }
 

codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs

@@ -5,13 +5,16 @@ use crate::exec::is_likely_sandbox_denied;
 use crate::guardian::GuardianApprovalRequest;
 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::sandboxing::ExecOptions;
 use crate::sandboxing::ExecRequest;
 use crate::sandboxing::SandboxPermissions;
 use crate::shell::ShellType;
+use crate::tools::approval::ApprovalCache;
+use crate::tools::approval::ApprovalPlan;
+use crate::tools::approval::CommandApprovalRequest;
+use crate::tools::approval::GuardianApproval;
+use crate::tools::approval::UserApprovalRequest;
+use crate::tools::approval::request_approval_for_turn;
 use crate::tools::runtimes::build_sandbox_command;
 use crate::tools::sandboxing::SandboxAttempt;
 use crate::tools::sandboxing::ToolCtx;
@@ -391,47 +394,44 @@ impl CoreShellActionProvider {
         let call_id = self.call_id.clone();
         let approval_id = Some(Uuid::new_v4().to_string());
         let source = self.tool_name;
-        let guardian_review_id = routes_approval_to_guardian(&turn).then(new_guardian_review_id);
         Ok(stopwatch
             .pause_for(async move {
-                if let Some(review_id) = guardian_review_id.clone() {
-                    let decision = review_approval_request(
-                        &session,
-                        &turn,
-                        review_id,
-                        GuardianApprovalRequest::Execve {
-                            id: call_id.clone(),
-                            source,
-                            program: program.to_string_lossy().into_owned(),
-                            argv: argv.to_vec(),
-                            cwd: workdir,
-                            additional_permissions,
-                        },
-                        /*retry_reason*/ None,
-                    )
-                    .await;
-                    return PromptDecision {
-                        decision,
-                        guardian_review_id,
-                    };
-                }
-                let decision = session
-                    .request_command_approval(
-                        &turn,
-                        call_id,
-                        approval_id,
-                        command,
-                        workdir,
-                        /*reason*/ None,
-                        /*network_approval_context*/ None,
-                        /*proposed_execpolicy_amendment*/ None,
-                        additional_permissions,
-                        Some(vec![ReviewDecision::Approved, ReviewDecision::Abort]),
-                    )
-                    .await;
+                let outcome = request_approval_for_turn(
+                    &session,
+                    &turn,
+                    ApprovalPlan {
+                        cache: ApprovalCache::<String>::None,
+                        user: UserApprovalRequest::Command(CommandApprovalRequest {
+                            call_id: call_id.clone(),
+                            approval_id,
+                            command: command.clone(),
+                            cwd: workdir.clone(),
+                            reason: None,
+                            network_approval_context: None,
+                            proposed_execpolicy_amendment: None,
+                            additional_permissions: additional_permissions.clone(),
+                            available_decisions: Some(vec![
+                                ReviewDecision::Approved,
+                                ReviewDecision::Abort,
+                            ]),
+                        }),
+                        guardian: GuardianApproval::new(
+                            GuardianApprovalRequest::Execve {
+                                id: call_id,
+                                source,
+                                program: program.to_string_lossy().into_owned(),
+                                argv: argv.to_vec(),
+                                cwd: workdir,
+                                additional_permissions,
+                            },
+                            /*retry_reason*/ None,
+                        ),
+                    },
+                )
+                .await;
                 PromptDecision {
-                    decision,
-                    guardian_review_id: None,
+                    decision: outcome.decision,
+                    guardian_review_id: outcome.guardian_review_id,
                 }
             })
             .await)

codex-rs/core/src/tools/runtimes/unified_exec.rs

@@ -8,10 +8,15 @@ use crate::command_canonicalization::canonicalize_command_for_approval;
 use crate::exec::ExecCapturePolicy;
 use crate::exec::ExecExpiration;
 use crate::guardian::GuardianApprovalRequest;
-use crate::guardian::review_approval_request;
 use crate::sandboxing::ExecOptions;
 use crate::sandboxing::SandboxPermissions;
 use crate::shell::ShellType;
+use crate::tools::approval::ApprovalCache;
+use crate::tools::approval::ApprovalPlan;
+use crate::tools::approval::CommandApprovalRequest;
+use crate::tools::approval::GuardianApproval;
+use crate::tools::approval::UserApprovalRequest;
+use crate::tools::approval::request_approval;
 use crate::tools::network_approval::NetworkApprovalMode;
 use crate::tools::network_approval::NetworkApprovalSpec;
 use crate::tools::runtimes::build_sandbox_command;
@@ -27,7 +32,6 @@ use crate::tools::sandboxing::ToolCtx;
 use crate::tools::sandboxing::ToolError;
 use crate::tools::sandboxing::ToolRuntime;
 use crate::tools::sandboxing::sandbox_override_for_first_attempt;
-use crate::tools::sandboxing::with_cached_approval;
 use crate::unified_exec::NoopSpawnLifecycle;
 use crate::unified_exec::UnifiedExecError;
 use crate::unified_exec::UnifiedExecProcess;
@@ -127,46 +131,46 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
         let cwd = req.cwd.to_path_buf();
         let retry_reason = ctx.retry_reason.clone();
         let reason = retry_reason.clone().or_else(|| req.justification.clone());
-        let guardian_review_id = ctx.guardian_review_id.clone();
         Box::pin(async move {
-            if let Some(review_id) = guardian_review_id {
-                return review_approval_request(
-                    session,
-                    turn,
-                    review_id,
-                    GuardianApprovalRequest::ExecCommand {
-                        id: call_id,
-                        command,
-                        cwd,
-                        sandbox_permissions: req.sandbox_permissions,
-                        additional_permissions: req.additional_permissions.clone(),
-                        justification: req.justification.clone(),
-                        tty: req.tty,
+            request_approval(
+                session,
+                turn,
+                ctx.guardian_review_id.clone(),
+                ApprovalPlan {
+                    cache: ApprovalCache::SessionApproveOnly {
+                        tool_name: "unified_exec",
+                        keys,
                     },
-                    retry_reason,
-                )
-                .await;
-            }
-            with_cached_approval(&session.services, "unified_exec", keys, || async move {
-                let available_decisions = None;
-                session
-                    .request_command_approval(
-                        turn,
-                        call_id,
-                        /*approval_id*/ None,
-                        command,
-                        cwd,
+                    user: UserApprovalRequest::Command(CommandApprovalRequest {
+                        call_id: call_id.clone(),
+                        approval_id: None,
+                        command: command.clone(),
+                        cwd: cwd.clone(),
                         reason,
-                        ctx.network_approval_context.clone(),
-                        req.exec_approval_requirement
+                        network_approval_context: ctx.network_approval_context.clone(),
+                        proposed_execpolicy_amendment: req
+                            .exec_approval_requirement
                             .proposed_execpolicy_amendment()
                             .cloned(),
-                        req.additional_permissions.clone(),
-                        available_decisions,
-                    )
-                    .await
-            })
+                        additional_permissions: req.additional_permissions.clone(),
+                        available_decisions: None,
+                    }),
+                    guardian: GuardianApproval::new(
+                        GuardianApprovalRequest::ExecCommand {
+                            id: call_id,
+                            command,
+                            cwd,
+                            sandbox_permissions: req.sandbox_permissions,
+                            additional_permissions: req.additional_permissions.clone(),
+                            justification: req.justification.clone(),
+                            tty: req.tty,
+                        },
+                        retry_reason,
+                    ),
+                },
+            )
             .await
+            .decision
         })
     }
 

codex-rs/core/src/tools/sandboxing.rs

@@ -1,14 +1,13 @@
 //! Shared approvals and sandboxing traits used by tool runtimes.
 //!
-//! Consolidates the approval flow primitives (`ApprovalDecision`, `ApprovalStore`,
-//! `ApprovalCtx`, `Approvable`) together with the sandbox orchestration traits
-//! and helpers (`Sandboxable`, `ToolRuntime`, `SandboxAttempt`, etc.).
+//! Consolidates the sandbox orchestration traits and helpers used by tool
+//! runtimes (`ApprovalCtx`, `Approvable`, `Sandboxable`, `ToolRuntime`,
+//! `SandboxAttempt`, etc.).
 
 use crate::codex::Session;
 use crate::codex::TurnContext;
 use crate::sandboxing::ExecOptions;
 use crate::sandboxing::SandboxPermissions;
-use crate::state::SessionServices;
 use crate::tools::network_approval::NetworkApprovalSpec;
 use codex_network_proxy::NetworkProxy;
 use codex_protocol::approvals::ExecPolicyAmendment;
@@ -27,94 +26,13 @@ use codex_sandboxing::SandboxTransformError;
 use codex_sandboxing::SandboxTransformRequest;
 use codex_sandboxing::SandboxType;
 use codex_sandboxing::SandboxablePreference;
-use futures::Future;
 use futures::future::BoxFuture;
 use serde::Serialize;
-use std::collections::HashMap;
 use std::fmt::Debug;
 use std::hash::Hash;
 use std::path::Path;
 use std::sync::Arc;
 
-#[derive(Clone, Default, Debug)]
-pub(crate) struct ApprovalStore {
-    // Store serialized keys for generic caching across requests.
-    map: HashMap<String, ReviewDecision>,
-}
-
-impl ApprovalStore {
-    pub fn get<K>(&self, key: &K) -> Option<ReviewDecision>
-    where
-        K: Serialize,
-    {
-        let s = serde_json::to_string(key).ok()?;
-        self.map.get(&s).cloned()
-    }
-
-    pub fn put<K>(&mut self, key: K, value: ReviewDecision)
-    where
-        K: Serialize,
-    {
-        if let Ok(s) = serde_json::to_string(&key) {
-            self.map.insert(s, value);
-        }
-    }
-}
-
-/// Takes a vector of approval keys and returns a ReviewDecision.
-/// There will be one key in most cases, but apply_patch can modify multiple files at once.
-///
-/// - If all keys are already approved for session, we skip prompting.
-/// - If the user approves for session, we store the decision for each key individually
-///   so future requests touching any subset can also skip prompting.
-pub(crate) async fn with_cached_approval<K, F, Fut>(
-    services: &SessionServices,
-    // Name of the tool, used for metrics collection.
-    tool_name: &str,
-    keys: Vec<K>,
-    fetch: F,
-) -> ReviewDecision
-where
-    K: Serialize,
-    F: FnOnce() -> Fut,
-    Fut: Future<Output = ReviewDecision>,
-{
-    // To be defensive here, don't bother with checking the cache if keys are empty.
-    if keys.is_empty() {
-        return fetch().await;
-    }
-
-    let already_approved = {
-        let store = services.tool_approvals.lock().await;
-        keys.iter()
-            .all(|key| matches!(store.get(key), Some(ReviewDecision::ApprovedForSession)))
-    };
-
-    if already_approved {
-        return ReviewDecision::ApprovedForSession;
-    }
-
-    let decision = fetch().await;
-
-    services.session_telemetry.counter(
-        "codex.approval.requested",
-        /*inc*/ 1,
-        &[
-            ("tool", tool_name),
-            ("approved", decision.to_opaque_string()),
-        ],
-    );
-
-    if matches!(decision, ReviewDecision::ApprovedForSession) {
-        let mut store = services.tool_approvals.lock().await;
-        for key in keys {
-            store.put(key, ReviewDecision::ApprovedForSession);
-        }
-    }
-
-    decision
-}
-
 #[derive(Clone)]
 pub(crate) struct ApprovalCtx<'a> {
     pub session: &'a Arc<Session>,