centralize approval routing

codex-rs/core/src/mcp_tool_call.rs

@@ -23,22 +23,21 @@ use crate::guardian::guardian_approval_request_to_json;
 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::hook_runtime::run_permission_request_hooks;
 use crate::mcp_openai_file::rewrite_mcp_tool_arguments_for_openai_files;
 use crate::mcp_tool_approval_templates::RenderedMcpToolApprovalParam;
 use crate::mcp_tool_approval_templates::render_mcp_tool_approval_template;
 use crate::session::session::Session;
 use crate::session::turn_context::TurnContext;
-use crate::tools::hook_names::HookToolName;
-use crate::tools::sandboxing::PermissionRequestPayload;
+use crate::tools::approval::ApprovalRequest;
+use crate::tools::approval::ApprovalRequestKind;
+use crate::tools::approval::McpToolCallApprovalRequest;
+use crate::tools::approval::review_before_user_prompt;
 use codex_analytics::AppInvocation;
 use codex_analytics::InvocationType;
 use codex_analytics::build_track_events_context;
 use codex_config::types::AppToolApproval;
 use codex_features::Feature;
-use codex_hooks::PermissionRequestDecision;
 use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
 use codex_mcp::McpPermissionPromptAutoApproveContext;
 use codex_mcp::SandboxState;
@@ -1005,55 +1004,79 @@ async fn maybe_request_mcp_tool_approval(
         return Some(McpToolApprovalDecision::Accept);
     }
 
-    match run_permission_request_hooks(
-        sess,
-        turn_context,
-        call_id,
-        PermissionRequestPayload {
-            tool_name: HookToolName::new(hook_tool_name),
-            tool_input: invocation
-                .arguments
-                .clone()
-                .unwrap_or_else(|| serde_json::Value::Object(serde_json::Map::new())),
-        },
-    )
-    .await
-    {
-        Some(PermissionRequestDecision::Allow) => {
-            return Some(McpToolApprovalDecision::Accept);
-        }
-        Some(PermissionRequestDecision::Deny { message }) => {
-            return Some(McpToolApprovalDecision::Decline {
-                message: Some(message),
-            });
-        }
-        None => {}
-    }
-
     let tool_call_mcp_elicitation_enabled = turn_context
         .config
         .features
         .enabled(Feature::ToolCallMcpElicitation);
-
-    if routes_approval_to_guardian(turn_context) {
-        let review_id = new_guardian_review_id();
-        let decision = review_approval_request(
-            sess,
-            turn_context,
-            review_id.clone(),
-            build_guardian_mcp_tool_review_request(call_id, invocation, metadata),
-            monitor_reason.clone(),
-        )
-        .await;
-        let decision = mcp_tool_approval_decision_from_guardian(sess, &review_id, decision).await;
-        apply_mcp_tool_approval_decision(
-            sess,
-            turn_context,
-            &decision,
-            session_approval_key,
-            persistent_approval_key,
-        )
-        .await;
+    let approval_request = ApprovalRequest::new(
+        call_id.to_string(),
+        /*user_reason*/ None,
+        monitor_reason.clone(),
+        ApprovalRequestKind::McpToolCall(McpToolCallApprovalRequest {
+            id: call_id.to_string(),
+            hook_tool_name: hook_tool_name.to_string(),
+            server: invocation.server.clone(),
+            tool_name: invocation.tool.clone(),
+            arguments: invocation.arguments.clone(),
+            connector_id: metadata.and_then(|metadata| metadata.connector_id.clone()),
+            connector_name: metadata.and_then(|metadata| metadata.connector_name.clone()),
+            connector_description: metadata
+                .and_then(|metadata| metadata.connector_description.clone()),
+            tool_title: metadata.and_then(|metadata| metadata.tool_title.clone()),
+            tool_description: metadata.and_then(|metadata| metadata.tool_description.clone()),
+            annotations: metadata
+                .and_then(|metadata| metadata.annotations.as_ref())
+                .map(|annotations| GuardianMcpAnnotations {
+                    destructive_hint: annotations.destructive_hint,
+                    open_world_hint: annotations.open_world_hint,
+                    read_only_hint: annotations.read_only_hint,
+                }),
+        }),
+    );
+    let guardian_review_id = routes_approval_to_guardian(turn_context).then(new_guardian_review_id);
+    if let Some(outcome) = review_before_user_prompt(
+        sess,
+        turn_context,
+        guardian_review_id,
+        /*evaluate_permission_request_hooks*/ true,
+        &approval_request,
+    )
+    .await
+    {
+        let decision = match outcome.source {
+            crate::tools::approval::ApprovalDecisionSource::PermissionRequestHook => {
+                match outcome.decision {
+                    ReviewDecision::Approved
+                    | ReviewDecision::ApprovedExecpolicyAmendment { .. }
+                    | ReviewDecision::ApprovedForSession
+                    | ReviewDecision::NetworkPolicyAmendment { .. } => {
+                        McpToolApprovalDecision::Accept
+                    }
+                    ReviewDecision::Denied | ReviewDecision::TimedOut | ReviewDecision::Abort => {
+                        McpToolApprovalDecision::Decline {
+                            message: outcome.rejection_message,
+                        }
+                    }
+                }
+            }
+            crate::tools::approval::ApprovalDecisionSource::Guardian { review_id } => {
+                let decision =
+                    mcp_tool_approval_decision_from_guardian(sess, &review_id, outcome.decision)
+                        .await;
+                apply_mcp_tool_approval_decision(
+                    sess,
+                    turn_context,
+                    &decision,
+                    session_approval_key,
+                    persistent_approval_key,
+                )
+                .await;
+                decision
+            }
+            crate::tools::approval::ApprovalDecisionSource::User => {
+                unreachable!("review_before_user_prompt never returns user-prompt outcomes")
+            }
+        };
         return Some(decision);
     }
 

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

@@ -292,12 +292,12 @@ use crate::stream_events_utils::HandleOutputCtx;
 #[cfg(test)]
 use crate::stream_events_utils::handle_output_item_done;
 use crate::tasks::ReviewTask;
+use crate::tools::approval::ApprovalStore;
 use crate::tools::network_approval::NetworkApprovalService;
 use crate::tools::network_approval::build_blocked_request_observer;
 use crate::tools::network_approval::build_network_policy_decider;
 #[cfg(test)]
 use crate::tools::parallel::ToolCallRuntime;
-use crate::tools::sandboxing::ApprovalStore;
 use crate::turn_timing::TurnTimingState;
 use crate::turn_timing::record_turn_ttfm_metric;
 use crate::unified_exec::UnifiedExecProcessManager;

codex-rs/core/src/session/tests.rs

@@ -897,8 +897,14 @@ async fn danger_full_access_tool_attempts_do_not_enforce_managed_network() -> an
             &'a mut self,
             _req: &'a (),
             _ctx: crate::tools::sandboxing::ApprovalCtx<'a>,
-        ) -> futures::future::BoxFuture<'a, ReviewDecision> {
-            Box::pin(async { ReviewDecision::Approved })
+        ) -> futures::future::BoxFuture<'a, crate::tools::approval::ApprovalOutcome> {
+            Box::pin(async {
+                crate::tools::approval::ApprovalOutcome {
+                    decision: ReviewDecision::Approved,
+                    rejection_message: None,
+                    source: crate::tools::approval::ApprovalDecisionSource::User,
+                }
+            })
         }
     }
 

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

@@ -10,9 +10,9 @@ use crate::guardian::GuardianRejection;
 use crate::guardian::GuardianRejectionCircuitBreaker;
 use crate::mcp::McpManager;
 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 arc_swap::ArcSwap;
 use codex_analytics::AnalyticsEventsClient;

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

@@ -0,0 +1,526 @@
+//! Canonical approval request routing for approval-requiring actions.
+//!
+//! Callers describe the action once as an [`ApprovalRequest`]. This module owns
+//! the shared approval pipeline for those requests:
+//! 1. policy hooks
+//! 2. guardian review
+//! 3. user prompting, including session-scoped approval caching
+
+use std::collections::HashMap;
+use std::path::PathBuf;
+use std::sync::Arc;
+
+use codex_hooks::PermissionRequestDecision;
+use codex_protocol::approvals::ExecPolicyAmendment;
+use codex_protocol::approvals::GuardianCommandSource;
+use codex_protocol::approvals::NetworkApprovalContext;
+use codex_protocol::approvals::NetworkApprovalProtocol;
+use codex_protocol::models::AdditionalPermissionProfile;
+use codex_protocol::protocol::FileChange;
+use codex_protocol::protocol::ReviewDecision;
+use codex_utils_absolute_path::AbsolutePathBuf;
+use futures::Future;
+use serde::Serialize;
+
+use crate::guardian::GuardianApprovalRequest;
+use crate::guardian::GuardianMcpAnnotations;
+use crate::guardian::GuardianNetworkAccessTrigger;
+use crate::guardian::review_approval_request;
+use crate::hook_runtime::run_permission_request_hooks;
+use crate::sandboxing::SandboxPermissions;
+use crate::session::session::Session;
+use crate::session::turn_context::TurnContext;
+use crate::tools::hook_names::HookToolName;
+use crate::tools::sandboxing::PermissionRequestPayload;
+
+#[derive(Clone, Default, Debug)]
+pub(crate) struct ApprovalStore {
+    map: HashMap<String, ReviewDecision>,
+}
+
+impl ApprovalStore {
+    pub(crate) fn get<K>(&self, key: &K) -> Option<ReviewDecision>
+    where
+        K: Serialize,
+    {
+        let serialized = serde_json::to_string(key).ok()?;
+        self.map.get(&serialized).cloned()
+    }
+
+    pub(crate) fn put<K>(&mut self, key: K, value: ReviewDecision)
+    where
+        K: Serialize,
+    {
+        if let Ok(serialized) = serde_json::to_string(&key) {
+            self.map.insert(serialized, value);
+        }
+    }
+}
+
+async fn with_cached_approval<K, F, Fut>(
+    session: &Session,
+    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 = session.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;
+
+    session.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 = session.services.tool_approvals.lock().await;
+        for key in keys {
+            store.put(key, ReviewDecision::ApprovedForSession);
+        }
+    }
+
+    decision
+}
+
+#[derive(Clone, Debug, Eq, Hash, PartialEq, Serialize)]
+struct ApprovalCacheKey {
+    namespace: &'static str,
+    value: String,
+}
+
+#[derive(Clone, Debug)]
+struct ApprovalCacheKeys {
+    tool_name: &'static str,
+    keys: Vec<ApprovalCacheKey>,
+}
+
+#[derive(Debug)]
+pub(crate) struct ApprovalOutcome {
+    pub(crate) decision: ReviewDecision,
+    pub(crate) rejection_message: Option<String>,
+    pub(crate) source: ApprovalDecisionSource,
+}
+
+#[derive(Debug)]
+pub(crate) enum ApprovalDecisionSource {
+    PermissionRequestHook,
+    Guardian { review_id: String },
+    User,
+}
+
+impl ApprovalDecisionSource {
+    pub(crate) fn guardian_review_id(&self) -> Option<&str> {
+        match self {
+            ApprovalDecisionSource::Guardian { review_id } => Some(review_id),
+            ApprovalDecisionSource::PermissionRequestHook | ApprovalDecisionSource::User => None,
+        }
+    }
+}
+
+#[derive(Clone, Debug)]
+pub(crate) struct ApprovalRequest {
+    pub(crate) hook_run_id: String,
+    pub(crate) user_reason: Option<String>,
+    pub(crate) guardian_retry_reason: Option<String>,
+    pub(crate) kind: ApprovalRequestKind,
+    cache: Option<ApprovalCacheKeys>,
+}
+
+impl ApprovalRequest {
+    pub(crate) fn new(
+        hook_run_id: String,
+        user_reason: Option<String>,
+        guardian_retry_reason: Option<String>,
+        kind: ApprovalRequestKind,
+    ) -> Self {
+        Self {
+            hook_run_id,
+            user_reason,
+            guardian_retry_reason,
+            kind,
+            cache: None,
+        }
+    }
+
+    pub(crate) fn with_session_cache<T>(mut self, tool_name: &'static str, keys: Vec<T>) -> Self
+    where
+        T: Serialize,
+    {
+        let keys = keys
+            .iter()
+            .map(|key| {
+                serde_json::to_string(key)
+                    .ok()
+                    .map(|value| ApprovalCacheKey {
+                        namespace: tool_name,
+                        value,
+                    })
+            })
+            .collect::<Option<Vec<_>>>();
+        self.cache = keys
+            .filter(|keys| !keys.is_empty())
+            .map(|keys| ApprovalCacheKeys { tool_name, keys });
+        self
+    }
+
+    pub(crate) fn permission_request_payload(&self) -> PermissionRequestPayload {
+        match &self.kind {
+            ApprovalRequestKind::Command(request) => PermissionRequestPayload::bash(
+                request.hook_command.clone(),
+                request.justification.clone(),
+            ),
+            #[cfg(unix)]
+            ApprovalRequestKind::Execve(request) => PermissionRequestPayload::bash(
+                codex_shell_command::parse_command::shlex_join(&request.command),
+                /*description*/ None,
+            ),
+            ApprovalRequestKind::Patch(request) => PermissionRequestPayload {
+                tool_name: HookToolName::apply_patch(),
+                tool_input: serde_json::json!({ "command": request.patch }),
+            },
+            ApprovalRequestKind::NetworkAccess(request) => PermissionRequestPayload::bash(
+                request.hook_command.clone(),
+                Some(format!("network-access {}", request.target)),
+            ),
+            ApprovalRequestKind::McpToolCall(request) => PermissionRequestPayload {
+                tool_name: HookToolName::new(request.hook_tool_name.clone()),
+                tool_input: request
+                    .arguments
+                    .clone()
+                    .unwrap_or_else(|| serde_json::Value::Object(serde_json::Map::new())),
+            },
+        }
+    }
+
+    pub(crate) fn into_guardian_request(self) -> GuardianApprovalRequest {
+        match self.kind {
+            ApprovalRequestKind::Command(request) => match request.source {
+                GuardianCommandSource::Shell => GuardianApprovalRequest::Shell {
+                    id: request.id,
+                    command: request.command,
+                    cwd: request.cwd,
+                    sandbox_permissions: request.sandbox_permissions,
+                    additional_permissions: request.additional_permissions,
+                    justification: request.justification,
+                },
+                GuardianCommandSource::UnifiedExec => GuardianApprovalRequest::ExecCommand {
+                    id: request.id,
+                    command: request.command,
+                    cwd: request.cwd,
+                    sandbox_permissions: request.sandbox_permissions,
+                    additional_permissions: request.additional_permissions,
+                    justification: request.justification,
+                    tty: request.tty,
+                },
+            },
+            #[cfg(unix)]
+            ApprovalRequestKind::Execve(request) => GuardianApprovalRequest::Execve {
+                id: request.id,
+                source: request.source,
+                program: request.program,
+                argv: request.argv,
+                cwd: request.cwd,
+                additional_permissions: request.additional_permissions,
+            },
+            ApprovalRequestKind::Patch(request) => GuardianApprovalRequest::ApplyPatch {
+                id: request.id,
+                cwd: request.cwd,
+                files: request.files,
+                patch: request.patch,
+            },
+            ApprovalRequestKind::NetworkAccess(request) => GuardianApprovalRequest::NetworkAccess {
+                id: request.id,
+                turn_id: request.turn_id,
+                target: request.target,
+                host: request.host,
+                protocol: request.protocol,
+                port: request.port,
+                trigger: request.trigger,
+            },
+            ApprovalRequestKind::McpToolCall(request) => GuardianApprovalRequest::McpToolCall {
+                id: request.id,
+                server: request.server,
+                tool_name: request.tool_name,
+                arguments: request.arguments,
+                connector_id: request.connector_id,
+                connector_name: request.connector_name,
+                connector_description: request.connector_description,
+                tool_title: request.tool_title,
+                tool_description: request.tool_description,
+                annotations: request.annotations,
+            },
+        }
+    }
+}
+
+#[derive(Clone, Debug)]
+pub(crate) enum ApprovalRequestKind {
+    Command(CommandApprovalRequest),
+    #[cfg(unix)]
+    Execve(ExecveApprovalRequest),
+    Patch(PatchApprovalRequest),
+    NetworkAccess(NetworkAccessApprovalRequest),
+    McpToolCall(McpToolCallApprovalRequest),
+}
+
+#[derive(Clone, Debug)]
+pub(crate) struct CommandApprovalRequest {
+    pub(crate) id: String,
+    pub(crate) approval_id: Option<String>,
+    pub(crate) source: GuardianCommandSource,
+    pub(crate) command: Vec<String>,
+    pub(crate) hook_command: String,
+    pub(crate) cwd: AbsolutePathBuf,
+    pub(crate) sandbox_permissions: SandboxPermissions,
+    pub(crate) additional_permissions: Option<AdditionalPermissionProfile>,
+    pub(crate) justification: Option<String>,
+    pub(crate) network_approval_context: Option<NetworkApprovalContext>,
+    pub(crate) proposed_execpolicy_amendment: Option<ExecPolicyAmendment>,
+    pub(crate) available_decisions: Option<Vec<ReviewDecision>>,
+    pub(crate) tty: bool,
+}
+
+#[cfg(unix)]
+#[derive(Clone, Debug)]
+pub(crate) struct ExecveApprovalRequest {
+    pub(crate) id: String,
+    pub(crate) approval_id: String,
+    pub(crate) source: GuardianCommandSource,
+    pub(crate) program: String,
+    pub(crate) argv: Vec<String>,
+    pub(crate) command: Vec<String>,
+    pub(crate) cwd: AbsolutePathBuf,
+    pub(crate) additional_permissions: Option<AdditionalPermissionProfile>,
+}
+
+#[derive(Clone, Debug)]
+pub(crate) struct PatchApprovalRequest {
+    pub(crate) id: String,
+    pub(crate) cwd: AbsolutePathBuf,
+    pub(crate) files: Vec<AbsolutePathBuf>,
+    pub(crate) patch: String,
+    pub(crate) changes: HashMap<PathBuf, FileChange>,
+    pub(crate) grant_root: Option<PathBuf>,
+}
+
+#[derive(Clone, Debug)]
+pub(crate) struct NetworkAccessApprovalRequest {
+    pub(crate) id: String,
+    pub(crate) turn_id: String,
+    pub(crate) target: String,
+    pub(crate) host: String,
+    pub(crate) protocol: NetworkApprovalProtocol,
+    pub(crate) port: u16,
+    pub(crate) cwd: AbsolutePathBuf,
+    pub(crate) hook_command: String,
+    pub(crate) trigger: Option<GuardianNetworkAccessTrigger>,
+}
+
+#[derive(Clone, Debug)]
+pub(crate) struct McpToolCallApprovalRequest {
+    pub(crate) id: String,
+    pub(crate) hook_tool_name: String,
+    pub(crate) server: String,
+    pub(crate) tool_name: String,
+    pub(crate) arguments: Option<serde_json::Value>,
+    pub(crate) connector_id: Option<String>,
+    pub(crate) connector_name: Option<String>,
+    pub(crate) connector_description: Option<String>,
+    pub(crate) tool_title: Option<String>,
+    pub(crate) tool_description: Option<String>,
+    pub(crate) annotations: Option<GuardianMcpAnnotations>,
+}
+
+async fn dispatch_user_approval(
+    session: &Arc<Session>,
+    turn: &Arc<TurnContext>,
+    request: ApprovalRequest,
+) -> ReviewDecision {
+    let ApprovalRequest {
+        user_reason, kind, ..
+    } = request;
+    match kind {
+        ApprovalRequestKind::Command(request) => {
+            session
+                .request_command_approval(
+                    turn.as_ref(),
+                    request.id,
+                    request.approval_id,
+                    request.command,
+                    request.cwd,
+                    user_reason,
+                    request.network_approval_context,
+                    request.proposed_execpolicy_amendment,
+                    request.additional_permissions,
+                    request.available_decisions,
+                )
+                .await
+        }
+        #[cfg(unix)]
+        ApprovalRequestKind::Execve(request) => {
+            session
+                .request_command_approval(
+                    turn.as_ref(),
+                    request.id,
+                    Some(request.approval_id),
+                    request.command,
+                    request.cwd,
+                    user_reason,
+                    /*network_approval_context*/ None,
+                    /*proposed_execpolicy_amendment*/ None,
+                    request.additional_permissions,
+                    Some(vec![ReviewDecision::Approved, ReviewDecision::Abort]),
+                )
+                .await
+        }
+        ApprovalRequestKind::Patch(request) => {
+            let rx_approve = session
+                .request_patch_approval(
+                    turn.as_ref(),
+                    request.id,
+                    request.changes,
+                    user_reason,
+                    request.grant_root,
+                )
+                .await;
+            rx_approve.await.unwrap_or_default()
+        }
+        ApprovalRequestKind::NetworkAccess(request) => {
+            session
+                .request_command_approval(
+                    turn.as_ref(),
+                    request.id,
+                    /*approval_id*/ None,
+                    vec!["network-access".to_string(), request.target],
+                    request.cwd,
+                    user_reason,
+                    Some(NetworkApprovalContext {
+                        host: request.host,
+                        protocol: request.protocol,
+                    }),
+                    /*proposed_execpolicy_amendment*/ None,
+                    /*additional_permissions*/ None,
+                    /*available_decisions*/ None,
+                )
+                .await
+        }
+        ApprovalRequestKind::McpToolCall(_) => {
+            unreachable!("MCP approvals use their own user-prompt transport")
+        }
+    }
+}
+
+async fn request_user_approval(
+    session: &Arc<Session>,
+    turn: &Arc<TurnContext>,
+    request: ApprovalRequest,
+) -> ReviewDecision {
+    if let Some(cache) = request.cache.clone() {
+        with_cached_approval(session, cache.tool_name, cache.keys, || {
+            dispatch_user_approval(session, turn, request)
+        })
+        .await
+    } else {
+        dispatch_user_approval(session, turn, request).await
+    }
+}
+
+pub(crate) async fn review_before_user_prompt(
+    session: &Arc<Session>,
+    turn: &Arc<TurnContext>,
+    guardian_review_id: Option<String>,
+    evaluate_permission_request_hooks: bool,
+    request: &ApprovalRequest,
+) -> Option<ApprovalOutcome> {
+    if evaluate_permission_request_hooks {
+        match run_permission_request_hooks(
+            session,
+            turn,
+            &request.hook_run_id,
+            request.permission_request_payload(),
+        )
+        .await
+        {
+            Some(PermissionRequestDecision::Allow) => {
+                return Some(ApprovalOutcome {
+                    decision: ReviewDecision::Approved,
+                    rejection_message: None,
+                    source: ApprovalDecisionSource::PermissionRequestHook,
+                });
+            }
+            Some(PermissionRequestDecision::Deny { message }) => {
+                return Some(ApprovalOutcome {
+                    decision: ReviewDecision::Denied,
+                    rejection_message: Some(message),
+                    source: ApprovalDecisionSource::PermissionRequestHook,
+                });
+            }
+            None => {}
+        }
+    }
+
+    let guardian_retry_reason = request.guardian_retry_reason.clone();
+    if let Some(review_id) = guardian_review_id {
+        return Some(ApprovalOutcome {
+            decision: review_approval_request(
+                session,
+                turn,
+                review_id.clone(),
+                request.clone().into_guardian_request(),
+                guardian_retry_reason,
+            )
+            .await,
+            rejection_message: None,
+            source: ApprovalDecisionSource::Guardian { review_id },
+        });
+    }
+
+    None
+}
+
+pub(crate) async fn request_approval(
+    session: &Arc<Session>,
+    turn: &Arc<TurnContext>,
+    guardian_review_id: Option<String>,
+    evaluate_permission_request_hooks: bool,
+    request: ApprovalRequest,
+) -> ApprovalOutcome {
+    if let Some(outcome) = review_before_user_prompt(
+        session,
+        turn,
+        guardian_review_id,
+        evaluate_permission_request_hooks,
+        &request,
+    )
+    .await
+    {
+        return outcome;
+    }
+
+    ApprovalOutcome {
+        decision: request_user_approval(session, turn, request).await,
+        rejection_message: None,
+        source: ApprovalDecisionSource::User,
+    }
+}

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

@@ -1,16 +1,15 @@
-use crate::guardian::GuardianApprovalRequest;
 use crate::guardian::GuardianNetworkAccessTrigger;
 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::hook_runtime::run_permission_request_hooks;
 use crate::network_policy_decision::denied_network_policy_message;
 use crate::session::session::Session;
-use crate::tools::sandboxing::PermissionRequestPayload;
+use crate::tools::approval::ApprovalRequest;
+use crate::tools::approval::ApprovalRequestKind;
+use crate::tools::approval::NetworkAccessApprovalRequest;
+use crate::tools::approval::request_approval;
 use crate::tools::sandboxing::ToolError;
-use codex_hooks::PermissionRequestDecision;
 use codex_network_proxy::BlockedRequest;
 use codex_network_proxy::BlockedRequestObserver;
 use codex_network_proxy::NetworkDecision;
@@ -460,51 +459,23 @@ impl NetworkApprovalService {
             protocol,
         };
         let guardian_approval_id = Self::approval_id_for_key(&key);
-        let prompt_command = vec!["network-access".to_string(), target.clone()];
-        let command = owner_call
+        let prompt_command = ["network-access".to_string(), target.clone()];
+        let hook_command = owner_call
             .as_ref()
             .map_or_else(|| prompt_command.join(" "), |call| call.command.clone());
-        if let Some(permission_request_decision) = run_permission_request_hooks(
+        let guardian_review_id =
+            routes_approval_to_guardian(&turn_context).then(new_guardian_review_id);
+        let approval_outcome = request_approval(
             &session,
             &turn_context,
-            &guardian_approval_id,
-            PermissionRequestPayload::bash(command, Some(format!("network-access {target}"))),
-        )
-        .await
-        {
-            match permission_request_decision {
-                PermissionRequestDecision::Allow => {
-                    pending
-                        .set_decision(PendingApprovalDecision::AllowOnce)
-                        .await;
-                    let mut pending_approvals = self.pending_host_approvals.lock().await;
-                    pending_approvals.remove(&key);
-                    return NetworkDecision::Allow;
-                }
-                PermissionRequestDecision::Deny { message } => {
-                    if let Some(owner_call) = owner_call.as_ref() {
-                        self.record_call_outcome(
-                            &owner_call.registration_id,
-                            NetworkApprovalOutcome::DeniedByPolicy(message),
-                        )
-                        .await;
-                    }
-                    pending.set_decision(PendingApprovalDecision::Deny).await;
-                    let mut pending_approvals = self.pending_host_approvals.lock().await;
-                    pending_approvals.remove(&key);
-                    return NetworkDecision::deny(REASON_NOT_ALLOWED);
-                }
-            }
-        }
-        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(),
+            guardian_review_id,
+            /*evaluate_permission_request_hooks*/ true,
+            ApprovalRequest::new(
+                guardian_approval_id.clone(),
+                Some(prompt_reason),
+                Some(policy_denial_message.clone()),
+                ApprovalRequestKind::NetworkAccess(NetworkAccessApprovalRequest {
+                    id: guardian_approval_id,
                     turn_id: owner_call
                         .as_ref()
                         .map_or_else(|| turn_context.sub_id.clone(), |call| call.turn_id.clone()),
@@ -512,28 +483,31 @@ impl NetworkApprovalService {
                     host: request.host,
                     protocol,
                     port: key.port,
+                    cwd: turn_context.cwd.clone(),
+                    hook_command,
                     trigger: owner_call.as_ref().map(|call| call.trigger.clone()),
-                },
-                Some(policy_denial_message.clone()),
+                }),
+            ),
+        )
+        .await;
+        if let Some(message) = approval_outcome.rejection_message.clone()
+            && let Some(owner_call) = owner_call.as_ref()
+        {
+            self.record_call_outcome(
+                &owner_call.registration_id,
+                NetworkApprovalOutcome::DeniedByPolicy(message),
             )
-            .await
-        } else {
-            let available_decisions = None;
-            session
-                .request_command_approval(
-                    turn_context.as_ref(),
-                    guardian_approval_id,
-                    /*approval_id*/ None,
-                    prompt_command,
-                    turn_context.cwd.clone(),
-                    Some(prompt_reason),
-                    Some(network_approval_context.clone()),
-                    /*proposed_execpolicy_amendment*/ None,
-                    /*additional_permissions*/ None,
-                    available_decisions,
-                )
-                .await
-        };
+            .await;
+        }
+        let rejected_by_hook = matches!(
+            approval_outcome.source,
+            crate::tools::approval::ApprovalDecisionSource::PermissionRequestHook
+        );
+        let guardian_review_id = approval_outcome
+            .source
+            .guardian_review_id()
+            .map(str::to_string);
+        let approval_decision = approval_outcome.decision;
 
         let mut cache_session_deny = false;
         let resolved = match approval_decision {
@@ -614,7 +588,9 @@ impl NetworkApprovalService {
                 }
             },
             ReviewDecision::Denied | ReviewDecision::Abort => {
-                if let Some(review_id) = guardian_review_id.as_deref() {
+                if rejected_by_hook {
+                    // Hook denials were already recorded above with their policy message.
+                } else if let Some(review_id) = guardian_review_id.as_deref() {
                     if let Some(owner_call) = owner_call.as_ref() {
                         let message = guardian_rejection_message(session.as_ref(), review_id).await;
                         self.record_call_outcome(

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

@@ -10,7 +10,6 @@ use crate::guardian::guardian_rejection_message;
 use crate::guardian::guardian_timeout_message;
 use crate::guardian::new_guardian_review_id;
 use crate::guardian::routes_approval_to_guardian;
-use crate::hook_runtime::run_permission_request_hooks;
 use crate::network_policy_decision::network_approval_context_from_payload;
 use crate::tools::network_approval::ActiveNetworkApproval;
 use crate::tools::network_approval::DeferredNetworkApproval;
@@ -26,7 +25,6 @@ use crate::tools::sandboxing::ToolCtx;
 use crate::tools::sandboxing::ToolError;
 use crate::tools::sandboxing::ToolRuntime;
 use crate::tools::sandboxing::default_exec_approval_requirement;
-use codex_hooks::PermissionRequestDecision;
 use codex_otel::ToolDecisionSource;
 use codex_protocol::error::CodexErr;
 use codex_protocol::error::SandboxErr;
@@ -157,19 +155,12 @@ impl ToolOrchestrator {
                         turn: &tool_ctx.turn,
                         call_id: &tool_ctx.call_id,
                         guardian_review_id: guardian_review_id.clone(),
+                        evaluate_permission_request_hooks: false,
                         retry_reason: None,
                         network_approval_context: None,
                     };
-                    let decision = Self::request_approval(
-                        tool,
-                        req,
-                        tool_ctx.call_id.as_str(),
-                        approval_ctx,
-                        tool_ctx,
-                        /*evaluate_permission_request_hooks*/ false,
-                        &otel,
-                    )
-                    .await?;
+                    let decision =
+                        Self::request_approval(tool, req, approval_ctx, tool_ctx, &otel).await?;
                     Self::reject_if_not_approved(tool_ctx, guardian_review_id.as_deref(), decision)
                         .await?;
                     already_approved = true;
@@ -192,19 +183,12 @@ impl ToolOrchestrator {
                     turn: &tool_ctx.turn,
                     call_id: &tool_ctx.call_id,
                     guardian_review_id: guardian_review_id.clone(),
+                    evaluate_permission_request_hooks: !strict_auto_review,
                     retry_reason: reason,
                     network_approval_context: None,
                 };
-                let decision = Self::request_approval(
-                    tool,
-                    req,
-                    tool_ctx.call_id.as_str(),
-                    approval_ctx,
-                    tool_ctx,
-                    /*evaluate_permission_request_hooks*/ !strict_auto_review,
-                    &otel,
-                )
-                .await?;
+                let decision =
+                    Self::request_approval(tool, req, approval_ctx, tool_ctx, &otel).await?;
 
                 Self::reject_if_not_approved(tool_ctx, guardian_review_id.as_deref(), decision)
                     .await?;
@@ -325,21 +309,13 @@ impl ToolOrchestrator {
                         turn: &tool_ctx.turn,
                         call_id: &tool_ctx.call_id,
                         guardian_review_id: guardian_review_id.clone(),
+                        evaluate_permission_request_hooks: !strict_auto_review,
                         retry_reason: Some(retry_reason),
                         network_approval_context: network_approval_context.clone(),
                     };
 
-                    let permission_request_run_id = format!("{}:retry", tool_ctx.call_id);
-                    let decision = Self::request_approval(
-                        tool,
-                        req,
-                        &permission_request_run_id,
-                        approval_ctx,
-                        tool_ctx,
-                        /*evaluate_permission_request_hooks*/ !strict_auto_review,
-                        &otel,
-                    )
-                    .await?;
+                    let decision =
+                        Self::request_approval(tool, req, approval_ctx, tool_ctx, &otel).await?;
 
                     Self::reject_if_not_approved(tool_ctx, guardian_review_id.as_deref(), decision)
                         .await?;
@@ -379,69 +355,36 @@ impl ToolOrchestrator {
         }
     }
 
-    // PermissionRequest hooks take top precedence for answering approval
-    // prompts. If no matching hook returns a decision, fall back to the
-    // normal guardian or user approval path.
     async fn request_approval<Rq, Out, T>(
         tool: &mut T,
         req: &Rq,
-        permission_request_run_id: &str,
         approval_ctx: ApprovalCtx<'_>,
         tool_ctx: &ToolCtx,
-        evaluate_permission_request_hooks: bool,
         otel: &codex_otel::SessionTelemetry,
     ) -> Result<ReviewDecision, ToolError>
     where
         T: ToolRuntime<Rq, Out>,
     {
-        if evaluate_permission_request_hooks
-            && let Some(permission_request) = tool.permission_request_payload(req)
-        {
-            match run_permission_request_hooks(
-                approval_ctx.session,
-                approval_ctx.turn,
-                permission_request_run_id,
-                permission_request,
-            )
-            .await
-            {
-                Some(PermissionRequestDecision::Allow) => {
-                    let decision = ReviewDecision::Approved;
-                    otel.tool_decision(
-                        &tool_ctx.tool_name,
-                        &tool_ctx.call_id,
-                        &decision,
-                        ToolDecisionSource::Config,
-                    );
-                    return Ok(decision);
-                }
-                Some(PermissionRequestDecision::Deny { message }) => {
-                    let decision = ReviewDecision::Denied;
-                    otel.tool_decision(
-                        &tool_ctx.tool_name,
-                        &tool_ctx.call_id,
-                        &decision,
-                        ToolDecisionSource::Config,
-                    );
-                    return Err(ToolError::Rejected(message));
-                }
-                None => {}
+        let outcome = tool.start_approval_async(req, approval_ctx).await;
+        let otel_source = match outcome.source {
+            crate::tools::approval::ApprovalDecisionSource::PermissionRequestHook => {
+                ToolDecisionSource::Config
             }
-        }
-
-        let otel_source = if approval_ctx.guardian_review_id.is_some() {
-            ToolDecisionSource::AutomatedReviewer
-        } else {
-            ToolDecisionSource::User
+            crate::tools::approval::ApprovalDecisionSource::Guardian { .. } => {
+                ToolDecisionSource::AutomatedReviewer
+            }
+            crate::tools::approval::ApprovalDecisionSource::User => ToolDecisionSource::User,
         };
-        let decision = tool.start_approval_async(req, approval_ctx).await;
         otel.tool_decision(
             &tool_ctx.tool_name,
             &tool_ctx.call_id,
-            &decision,
+            &outcome.decision,
             otel_source,
         );
-        Ok(decision)
+        if let Some(message) = outcome.rejection_message {
+            return Err(ToolError::Rejected(message));
+        }
+        Ok(outcome.decision)
     }
 
     async fn reject_if_not_approved(

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

@@ -4,19 +4,19 @@
 //! selected turn environment filesystem for both local and remote turns, with
 //! sandboxing enforced by the explicit filesystem sandbox context.
 use crate::exec::is_likely_sandbox_denied;
-use crate::guardian::GuardianApprovalRequest;
-use crate::guardian::review_approval_request;
-use crate::tools::hook_names::HookToolName;
+use crate::tools::approval::ApprovalOutcome;
+use crate::tools::approval::ApprovalRequest;
+use crate::tools::approval::ApprovalRequestKind;
+use crate::tools::approval::PatchApprovalRequest;
+use crate::tools::approval::request_approval;
 use crate::tools::sandboxing::Approvable;
 use crate::tools::sandboxing::ApprovalCtx;
 use crate::tools::sandboxing::ExecApprovalRequirement;
-use crate::tools::sandboxing::PermissionRequestPayload;
 use crate::tools::sandboxing::SandboxAttempt;
 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_exec_server::FileSystemSandboxContext;
 use codex_protocol::error::CodexErr;
@@ -53,18 +53,6 @@ impl ApplyPatchRuntime {
         Self
     }
 
-    fn build_guardian_review_request(
-        req: &ApplyPatchRequest,
-        call_id: &str,
-    ) -> GuardianApprovalRequest {
-        GuardianApprovalRequest::ApplyPatch {
-            id: call_id.to_string(),
-            cwd: req.action.cwd.clone(),
-            files: req.file_paths.clone(),
-            patch: req.action.patch.clone(),
-        }
-    }
-
     fn file_system_sandbox_context_for_attempt(
         req: &ApplyPatchRequest,
         attempt: &SandboxAttempt<'_>,
@@ -105,7 +93,7 @@ impl Approvable<ApplyPatchRequest> for ApplyPatchRuntime {
         &'a mut self,
         req: &'a ApplyPatchRequest,
         ctx: ApprovalCtx<'a>,
-    ) -> BoxFuture<'a, ReviewDecision> {
+    ) -> BoxFuture<'a, ApprovalOutcome> {
         let session = ctx.session;
         let turn = ctx.turn;
         let call_id = ctx.call_id.to_string();
@@ -114,39 +102,41 @@ impl Approvable<ApplyPatchRequest> for ApplyPatchRuntime {
         let changes = req.changes.clone();
         let guardian_review_id = ctx.guardian_review_id.clone();
         Box::pin(async move {
-            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 req.permissions_preapproved && retry_reason.is_none() {
-                return ReviewDecision::Approved;
+                return ApprovalOutcome {
+                    decision: ReviewDecision::Approved,
+                    rejection_message: None,
+                    source: crate::tools::approval::ApprovalDecisionSource::User,
+                };
             }
-            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();
-            }
-
-            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()
+            let request = ApprovalRequest::new(
+                if ctx.retry_reason.is_some() {
+                    format!("{call_id}:retry")
+                } else {
+                    call_id.clone()
                 },
+                retry_reason.clone(),
+                retry_reason,
+                ApprovalRequestKind::Patch(PatchApprovalRequest {
+                    id: call_id,
+                    cwd: req.action.cwd.clone(),
+                    files: req.file_paths.clone(),
+                    patch: req.action.patch.clone(),
+                    changes,
+                    grant_root: None,
+                }),
+            );
+            let request = if request.user_reason.is_none() {
+                request.with_session_cache("apply_patch", approval_keys)
+            } else {
+                request
+            };
+            request_approval(
+                session,
+                turn,
+                guardian_review_id,
+                ctx.evaluate_permission_request_hooks,
+                request,
             )
             .await
         })
@@ -172,16 +162,6 @@ impl Approvable<ApplyPatchRequest> for ApplyPatchRuntime {
     ) -> Option<ExecApprovalRequirement> {
         Some(req.exec_approval_requirement.clone())
     }
-
-    fn permission_request_payload(
-        &self,
-        req: &ApplyPatchRequest,
-    ) -> Option<PermissionRequestPayload> {
-        Some(PermissionRequestPayload {
-            tool_name: HookToolName::apply_patch(),
-            tool_input: serde_json::json!({ "command": req.action.patch }),
-        })
-    }
 }
 
 impl ToolRuntime<ApplyPatchRequest, ExecToolCallOutput> for ApplyPatchRuntime {

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

@@ -1,4 +1,8 @@
 use super::*;
+use crate::guardian::GuardianApprovalRequest;
+use crate::tools::approval::ApprovalRequest;
+use crate::tools::approval::ApprovalRequestKind;
+use crate::tools::approval::PatchApprovalRequest;
 use crate::tools::sandboxing::SandboxAttempt;
 use codex_protocol::config_types::WindowsSandboxLevel;
 use codex_protocol::models::AdditionalPermissionProfile;
@@ -65,7 +69,20 @@ fn guardian_review_request_includes_patch_context() {
         permissions_preapproved: false,
     };
 
-    let guardian_request = ApplyPatchRuntime::build_guardian_review_request(&request, "call-1");
+    let guardian_request = ApprovalRequest::new(
+        "call-1".to_string(),
+        /*user_reason*/ None,
+        /*guardian_retry_reason*/ None,
+        ApprovalRequestKind::Patch(PatchApprovalRequest {
+            id: "call-1".to_string(),
+            cwd: request.action.cwd.clone(),
+            files: request.file_paths.clone(),
+            patch: request.action.patch.clone(),
+            changes: request.changes.clone(),
+            grant_root: None,
+        }),
+    )
+    .into_guardian_request();
 
     assert_eq!(
         guardian_request,
@@ -80,7 +97,6 @@ fn guardian_review_request_includes_patch_context() {
 
 #[test]
 fn permission_request_payload_uses_apply_patch_hook_name_and_aliases() {
-    let runtime = ApplyPatchRuntime::new();
     let path = std::env::temp_dir()
         .join("apply-patch-permission-request-payload.txt")
         .abs();
@@ -98,9 +114,20 @@ fn permission_request_payload_uses_apply_patch_hook_name_and_aliases() {
         permissions_preapproved: false,
     };
 
-    let payload = runtime
-        .permission_request_payload(&req)
-        .expect("permission request payload");
+    let payload = ApprovalRequest::new(
+        "call-1".to_string(),
+        /*user_reason*/ None,
+        /*guardian_retry_reason*/ None,
+        ApprovalRequestKind::Patch(PatchApprovalRequest {
+            id: "call-1".to_string(),
+            cwd: req.action.cwd.clone(),
+            files: req.file_paths.clone(),
+            patch: req.action.patch.clone(),
+            changes: req.changes,
+            grant_root: None,
+        }),
+    )
+    .permission_request_payload();
 
     assert_eq!(payload.tool_name.name(), "apply_patch");
     assert_eq!(

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

@@ -10,13 +10,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::GuardianNetworkAccessTrigger;
-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::ApprovalOutcome;
+use crate::tools::approval::ApprovalRequest;
+use crate::tools::approval::ApprovalRequestKind;
+use crate::tools::approval::CommandApprovalRequest;
+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;
@@ -25,7 +28,6 @@ use crate::tools::runtimes::maybe_wrap_shell_lc_with_snapshot;
 use crate::tools::sandboxing::Approvable;
 use crate::tools::sandboxing::ApprovalCtx;
 use crate::tools::sandboxing::ExecApprovalRequirement;
-use crate::tools::sandboxing::PermissionRequestPayload;
 use crate::tools::sandboxing::SandboxAttempt;
 use crate::tools::sandboxing::SandboxOverride;
 use crate::tools::sandboxing::Sandboxable;
@@ -34,11 +36,9 @@ use crate::tools::sandboxing::ToolError;
 use crate::tools::sandboxing::ToolRuntime;
 use crate::tools::sandboxing::managed_network_for_sandbox_permissions;
 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::AdditionalPermissionProfile;
-use codex_protocol::protocol::ReviewDecision;
 use codex_sandboxing::SandboxablePreference;
 use codex_shell_command::powershell::prefix_powershell_script_with_utf8;
 use codex_utils_absolute_path::AbsolutePathBuf;
@@ -147,7 +147,7 @@ impl Approvable<ShellRequest> for ShellRuntime {
         &'a mut self,
         req: &'a ShellRequest,
         ctx: ApprovalCtx<'a>,
-    ) -> BoxFuture<'a, ReviewDecision> {
+    ) -> BoxFuture<'a, ApprovalOutcome> {
         let keys = self.approval_keys(req);
         let command = req.command.clone();
         let cwd = req.cwd.clone();
@@ -158,42 +158,41 @@ impl Approvable<ShellRequest> for ShellRuntime {
         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: cwd.clone(),
-                        sandbox_permissions: req.sandbox_permissions,
-                        additional_permissions: req.additional_permissions.clone(),
-                        justification: req.justification.clone(),
-                    },
-                    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,
-                        reason,
-                        ctx.network_approval_context.clone(),
-                        req.exec_approval_requirement
-                            .proposed_execpolicy_amendment()
-                            .cloned(),
-                        req.additional_permissions.clone(),
-                        available_decisions,
-                    )
-                    .await
-            })
+            let request = ApprovalRequest::new(
+                if ctx.retry_reason.is_some() {
+                    format!("{call_id}:retry")
+                } else {
+                    call_id.clone()
+                },
+                reason,
+                retry_reason,
+                ApprovalRequestKind::Command(CommandApprovalRequest {
+                    id: call_id,
+                    approval_id: None,
+                    source: codex_protocol::approvals::GuardianCommandSource::Shell,
+                    command,
+                    hook_command: req.hook_command.clone(),
+                    cwd,
+                    sandbox_permissions: req.sandbox_permissions,
+                    additional_permissions: req.additional_permissions.clone(),
+                    justification: req.justification.clone(),
+                    network_approval_context: ctx.network_approval_context.clone(),
+                    proposed_execpolicy_amendment: req
+                        .exec_approval_requirement
+                        .proposed_execpolicy_amendment()
+                        .cloned(),
+                    available_decisions: None,
+                    tty: false,
+                }),
+            )
+            .with_session_cache("shell", keys);
+            request_approval(
+                session,
+                turn,
+                guardian_review_id,
+                ctx.evaluate_permission_request_hooks,
+                request,
+            )
             .await
         })
     }
@@ -202,13 +201,6 @@ impl Approvable<ShellRequest> for ShellRuntime {
         Some(req.exec_approval_requirement.clone())
     }
 
-    fn permission_request_payload(&self, req: &ShellRequest) -> Option<PermissionRequestPayload> {
-        Some(PermissionRequestPayload::bash(
-            req.hook_command.clone(),
-            req.justification.clone(),
-        ))
-    }
-
     fn sandbox_mode_for_first_attempt(&self, req: &ShellRequest) -> SandboxOverride {
         sandbox_override_for_first_attempt(req.sandbox_permissions, &req.exec_approval_requirement)
     }

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

@@ -3,20 +3,20 @@ use crate::exec::ExecCapturePolicy;
 use crate::exec::ExecExpiration;
 use crate::exec::cancel_when_either;
 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::hook_runtime::run_permission_request_hooks;
 use crate::sandboxing::ExecOptions;
 use crate::sandboxing::ExecRequest;
 use crate::sandboxing::SandboxPermissions;
 use crate::shell::ShellType;
+use crate::tools::approval::ApprovalRequest;
+use crate::tools::approval::ApprovalRequestKind;
+use crate::tools::approval::ExecveApprovalRequest;
+use crate::tools::approval::request_approval;
 use crate::tools::runtimes::build_sandbox_command;
 use crate::tools::runtimes::exec_env_for_sandbox_permissions;
-use crate::tools::sandboxing::PermissionRequestPayload;
 use crate::tools::sandboxing::SandboxAttempt;
 use crate::tools::sandboxing::ToolCtx;
 use crate::tools::sandboxing::ToolError;
@@ -27,7 +27,6 @@ use codex_execpolicy::MatchOptions;
 use codex_execpolicy::Policy;
 use codex_execpolicy::RuleMatch;
 use codex_features::Feature;
-use codex_hooks::PermissionRequestDecision;
 use codex_protocol::config_types::WindowsSandboxLevel;
 use codex_protocol::error::CodexErr;
 use codex_protocol::error::SandboxErr;
@@ -401,85 +400,37 @@ impl CoreShellActionProvider {
         let session = self.session.clone();
         let turn = self.turn.clone();
         let call_id = self.call_id.clone();
-        let approval_id = Some(Uuid::new_v4().to_string());
+        let approval_id = 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 {
-                // 1) Run PermissionRequest hooks
-                let permission_request = PermissionRequestPayload::bash(
-                    codex_shell_command::parse_command::shlex_join(&command),
-                    /*description*/ None,
-                );
-                let effective_approval_id = approval_id.clone().unwrap_or_else(|| call_id.clone());
-                match run_permission_request_hooks(
+                let outcome = request_approval(
                     &session,
                     &turn,
-                    &effective_approval_id,
-                    permission_request,
-                )
-                .await
-                {
-                    Some(PermissionRequestDecision::Allow) => {
-                        return PromptDecision {
-                            decision: ReviewDecision::Approved,
-                            guardian_review_id: None,
-                            rejection_message: None,
-                        };
-                    }
-                    Some(PermissionRequestDecision::Deny { message }) => {
-                        return PromptDecision {
-                            decision: ReviewDecision::Denied,
-                            guardian_review_id: None,
-                            rejection_message: Some(message),
-                        };
-                    }
-                    None => {}
-                }
-
-                // 2) Route to Guardian if configured
-                if let Some(review_id) = guardian_review_id.clone() {
-                    let decision = review_approval_request(
-                        &session,
-                        &turn,
-                        review_id.clone(),
-                        GuardianApprovalRequest::Execve {
-                            id: call_id.clone(),
+                    guardian_review_id,
+                    /*evaluate_permission_request_hooks*/ true,
+                    ApprovalRequest::new(
+                        approval_id.clone(),
+                        /*user_reason*/ None,
+                        /*guardian_retry_reason*/ None,
+                        ApprovalRequestKind::Execve(ExecveApprovalRequest {
+                            id: call_id,
+                            approval_id,
                             source,
                             program: program.to_string_lossy().into_owned(),
                             argv: argv.to_vec(),
+                            command,
                             cwd: workdir.clone(),
                             additional_permissions,
-                        },
-                        /*retry_reason*/ None,
-                    )
-                    .await;
-                    return PromptDecision {
-                        decision,
-                        guardian_review_id,
-                        rejection_message: None,
-                    };
-                }
-
-                // 3) Fall back to regular user prompt
-                let decision = session
-                    .request_command_approval(
-                        &turn,
-                        call_id,
-                        approval_id,
-                        command,
-                        workdir.clone(),
-                        /*reason*/ None,
-                        /*network_approval_context*/ None,
-                        /*proposed_execpolicy_amendment*/ None,
-                        additional_permissions,
-                        Some(vec![ReviewDecision::Approved, ReviewDecision::Abort]),
-                    )
-                    .await;
+                        }),
+                    ),
+                )
+                .await;
                 PromptDecision {
-                    decision,
-                    guardian_review_id: None,
-                    rejection_message: None,
+                    decision: outcome.decision,
+                    guardian_review_id: outcome.source.guardian_review_id().map(str::to_string),
+                    rejection_message: outcome.rejection_message,
                 }
             })
             .await)

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

@@ -7,13 +7,16 @@ the process manager to spawn PTYs once an ExecRequest is prepared.
 use crate::command_canonicalization::canonicalize_command_for_approval;
 use crate::exec::ExecCapturePolicy;
 use crate::exec::ExecExpiration;
-use crate::guardian::GuardianApprovalRequest;
 use crate::guardian::GuardianNetworkAccessTrigger;
-use crate::guardian::review_approval_request;
 use crate::sandboxing::ExecOptions;
 use crate::sandboxing::ExecServerEnvConfig;
 use crate::sandboxing::SandboxPermissions;
 use crate::shell::ShellType;
+use crate::tools::approval::ApprovalOutcome;
+use crate::tools::approval::ApprovalRequest;
+use crate::tools::approval::ApprovalRequestKind;
+use crate::tools::approval::CommandApprovalRequest;
+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;
@@ -23,7 +26,6 @@ use crate::tools::runtimes::shell::zsh_fork_backend;
 use crate::tools::sandboxing::Approvable;
 use crate::tools::sandboxing::ApprovalCtx;
 use crate::tools::sandboxing::ExecApprovalRequirement;
-use crate::tools::sandboxing::PermissionRequestPayload;
 use crate::tools::sandboxing::SandboxAttempt;
 use crate::tools::sandboxing::SandboxOverride;
 use crate::tools::sandboxing::Sandboxable;
@@ -32,7 +34,6 @@ use crate::tools::sandboxing::ToolError;
 use crate::tools::sandboxing::ToolRuntime;
 use crate::tools::sandboxing::managed_network_for_sandbox_permissions;
 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;
@@ -41,7 +42,6 @@ use codex_network_proxy::NetworkProxy;
 use codex_protocol::error::CodexErr;
 use codex_protocol::error::SandboxErr;
 use codex_protocol::models::AdditionalPermissionProfile;
-use codex_protocol::protocol::ReviewDecision;
 use codex_sandboxing::SandboxablePreference;
 use codex_shell_command::powershell::prefix_powershell_script_with_utf8;
 use codex_tools::UnifiedExecShellMode;
@@ -139,7 +139,7 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
         &'b mut self,
         req: &'b UnifiedExecRequest,
         ctx: ApprovalCtx<'b>,
-    ) -> BoxFuture<'b, ReviewDecision> {
+    ) -> BoxFuture<'b, ApprovalOutcome> {
         let keys = self.approval_keys(req);
         let session = ctx.session;
         let turn = ctx.turn;
@@ -150,43 +150,41 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
         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: cwd.clone(),
-                        sandbox_permissions: req.sandbox_permissions,
-                        additional_permissions: req.additional_permissions.clone(),
-                        justification: req.justification.clone(),
-                        tty: req.tty,
-                    },
-                    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.clone(),
-                        reason,
-                        ctx.network_approval_context.clone(),
-                        req.exec_approval_requirement
-                            .proposed_execpolicy_amendment()
-                            .cloned(),
-                        req.additional_permissions.clone(),
-                        available_decisions,
-                    )
-                    .await
-            })
+            let request = ApprovalRequest::new(
+                if ctx.retry_reason.is_some() {
+                    format!("{call_id}:retry")
+                } else {
+                    call_id.clone()
+                },
+                reason,
+                retry_reason,
+                ApprovalRequestKind::Command(CommandApprovalRequest {
+                    id: call_id,
+                    approval_id: None,
+                    source: codex_protocol::approvals::GuardianCommandSource::UnifiedExec,
+                    command,
+                    hook_command: req.hook_command.clone(),
+                    cwd,
+                    sandbox_permissions: req.sandbox_permissions,
+                    additional_permissions: req.additional_permissions.clone(),
+                    justification: req.justification.clone(),
+                    network_approval_context: ctx.network_approval_context.clone(),
+                    proposed_execpolicy_amendment: req
+                        .exec_approval_requirement
+                        .proposed_execpolicy_amendment()
+                        .cloned(),
+                    available_decisions: None,
+                    tty: req.tty,
+                }),
+            )
+            .with_session_cache("unified_exec", keys);
+            request_approval(
+                session,
+                turn,
+                guardian_review_id,
+                ctx.evaluate_permission_request_hooks,
+                request,
+            )
             .await
         })
     }
@@ -198,16 +196,6 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
         Some(req.exec_approval_requirement.clone())
     }
 
-    fn permission_request_payload(
-        &self,
-        req: &UnifiedExecRequest,
-    ) -> Option<PermissionRequestPayload> {
-        Some(PermissionRequestPayload::bash(
-            req.hook_command.clone(),
-            req.justification.clone(),
-        ))
-    }
-
     fn sandbox_mode_for_first_attempt(&self, req: &UnifiedExecRequest) -> SandboxOverride {
         sandbox_override_for_first_attempt(req.sandbox_permissions, &req.exec_approval_requirement)
     }

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

@@ -1,14 +1,10 @@
-//! 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.).
+//! Shared sandboxing traits used by tool runtimes.
 
 use crate::sandboxing::ExecOptions;
 use crate::sandboxing::SandboxPermissions;
 use crate::session::session::Session;
 use crate::session::turn_context::TurnContext;
-use crate::state::SessionServices;
+use crate::tools::approval::ApprovalOutcome;
 use crate::tools::hook_names::HookToolName;
 use crate::tools::network_approval::NetworkApprovalSpec;
 use codex_network_proxy::NetworkProxy;
@@ -18,7 +14,6 @@ use codex_protocol::error::CodexErr;
 use codex_protocol::permissions::FileSystemSandboxKind;
 use codex_protocol::permissions::FileSystemSandboxPolicy;
 use codex_protocol::protocol::AskForApproval;
-use codex_protocol::protocol::ReviewDecision;
 #[cfg(test)]
 use codex_protocol::protocol::SandboxPolicy;
 use codex_sandboxing::SandboxCommand;
@@ -28,94 +23,13 @@ use codex_sandboxing::SandboxTransformRequest;
 use codex_sandboxing::SandboxType;
 use codex_sandboxing::SandboxablePreference;
 use codex_utils_absolute_path::AbsolutePathBuf;
-use futures::Future;
 use futures::future::BoxFuture;
 use serde::Serialize;
-use std::collections::HashMap;
 use std::fmt::Debug;
 use std::hash::Hash;
 use std::sync::Arc;
 use tokio_util::sync::CancellationToken;
 
-#[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>,
@@ -128,6 +42,7 @@ pub(crate) struct ApprovalCtx<'a> {
     /// denial handling, overrides, and app-server notifications refer to the
     /// review without overloading the tool call ID as a review ID.
     pub guardian_review_id: Option<String>,
+    pub evaluate_permission_request_hooks: bool,
     pub retry_reason: Option<String>,
     pub network_approval_context: Option<NetworkApprovalContext>,
 }
@@ -309,12 +224,6 @@ pub(crate) trait Approvable<Req> {
         None
     }
 
-    /// Return hook input for approval-time policy hooks when this runtime wants
-    /// hook evaluation to run before guardian or user approval.
-    fn permission_request_payload(&self, _req: &Req) -> Option<PermissionRequestPayload> {
-        None
-    }
-
     /// Decide we can request an approval for no-sandbox execution.
     fn wants_no_sandbox_approval(&self, policy: AskForApproval) -> bool {
         match policy {
@@ -330,7 +239,7 @@ pub(crate) trait Approvable<Req> {
         &'a mut self,
         req: &'a Req,
         ctx: ApprovalCtx<'a>,
-    ) -> BoxFuture<'a, ReviewDecision>;
+    ) -> BoxFuture<'a, ApprovalOutcome>;
 }
 
 pub(crate) trait Sandboxable {