Consolidate apply_patch runtime execution (#36745)

## What changed

- Route verified patches from both direct tool calls and intercepted shell commands through a shared execution helper.
- Simplify patch safety preparation to return either a runtime invocation or a rejection directly.
- Keep permission resolution, approval handling, event emission, runtime execution, and diff tracking in the common path.

GitOrigin-RevId: e16aae8e91d96b8108aefac7f3fbf2da2cbac016

codex-rs/core/src/apply_patch.rs

@@ -11,19 +11,6 @@ use codex_utils_path_uri::PathUri;
 use std::collections::HashMap;
 use std::path::PathBuf;
 
-pub(crate) enum InternalApplyPatchInvocation {
-    /// The `apply_patch` call was handled programmatically, without any sort
-    /// of sandbox, because the user explicitly approved it. This is the
-    /// result to use with the `shell` function call that contained `apply_patch`.
-    Output(Result<String, FunctionCallError>),
-
-    /// The `apply_patch` call was approved, either automatically because it
-    /// appears that it should be allowed based on the user's sandbox policy
-    /// *or* because the user explicitly approved it. The runtime realizes the
-    /// patch through the selected environment filesystem.
-    DelegateToRuntime(ApplyPatchRuntimeInvocation),
-}
-
 #[derive(Debug)]
 pub(crate) struct ApplyPatchRuntimeInvocation {
     pub(crate) action: ApplyPatchAction,
@@ -31,11 +18,11 @@ pub(crate) struct ApplyPatchRuntimeInvocation {
     pub(crate) exec_approval_requirement: ExecApprovalRequirement,
 }
 
-pub(crate) async fn apply_patch(
+pub(crate) fn prepare_apply_patch(
     turn_context: &TurnContext,
     file_system_sandbox_policy: &FileSystemSandboxPolicy,
     action: ApplyPatchAction,
-) -> InternalApplyPatchInvocation {
+) -> Result<ApplyPatchRuntimeInvocation, FunctionCallError> {
     match assess_patch_safety(
         &action,
         turn_context.approval_policy.value(),
@@ -44,12 +31,9 @@ pub(crate) async fn apply_patch(
         &action.cwd,
         turn_context.windows_sandbox_level,
     ) {
-        SafetyCheck::AutoApprove {
-            user_explicitly_approved,
-            ..
-        } => InternalApplyPatchInvocation::DelegateToRuntime(ApplyPatchRuntimeInvocation {
+        SafetyCheck::AutoApprove => Ok(ApplyPatchRuntimeInvocation {
             action,
-            auto_approved: !user_explicitly_approved,
+            auto_approved: true,
             exec_approval_requirement: ExecApprovalRequirement::Skip {
                 bypass_sandbox: false,
                 proposed_execpolicy_amendment: None,
@@ -59,7 +43,7 @@ pub(crate) async fn apply_patch(
             // Delegate the approval prompt (including cached approvals) to the
             // tool runtime, consistent with how shell/unified_exec approvals
             // are orchestrator-driven.
-            InternalApplyPatchInvocation::DelegateToRuntime(ApplyPatchRuntimeInvocation {
+            Ok(ApplyPatchRuntimeInvocation {
                 action,
                 auto_approved: false,
                 exec_approval_requirement: ExecApprovalRequirement::NeedsApproval {
@@ -68,9 +52,9 @@ pub(crate) async fn apply_patch(
                 },
             })
         }
-        SafetyCheck::Reject { reason } => InternalApplyPatchInvocation::Output(Err(
-            FunctionCallError::RespondToModel(format!("patch rejected: {reason}")),
-        )),
+        SafetyCheck::Reject { reason } => Err(FunctionCallError::RespondToModel(format!(
+            "patch rejected: {reason}"
+        ))),
     }
 }
 

codex-rs/core/src/safety.rs

@@ -8,7 +8,6 @@ use codex_protocol::config_types::WindowsSandboxLevel;
 use codex_protocol::models::PermissionProfile;
 use codex_protocol::permissions::FileSystemSandboxPolicy;
 use codex_protocol::protocol::AskForApproval;
-use codex_sandboxing::SandboxType;
 use codex_sandboxing::get_platform_sandbox;
 use codex_utils_path_uri::PathUri;
 
@@ -19,14 +18,9 @@ const PATCH_REJECTED_READ_ONLY_REASON: &str =
 
 #[derive(Debug, PartialEq)]
 pub enum SafetyCheck {
-    AutoApprove {
-        sandbox_type: SandboxType,
-        user_explicitly_approved: bool,
-    },
+    AutoApprove,
     AskUser,
-    Reject {
-        reason: String,
-    },
+    Reject { reason: String },
 }
 
 pub fn assess_patch_safety(
@@ -70,19 +64,13 @@ pub fn assess_patch_safety(
         ) {
             // Disabled and External profiles intentionally do not apply an
             // outer Codex filesystem sandbox.
-            SafetyCheck::AutoApprove {
-                sandbox_type: SandboxType::None,
-                user_explicitly_approved: false,
-            }
+            SafetyCheck::AutoApprove
         } else {
             // Only auto‑approve when we can actually enforce a sandbox. Otherwise
             // fall back to asking the user because the patch may touch arbitrary
             // paths outside the project.
             match get_platform_sandbox(windows_sandbox_level != WindowsSandboxLevel::Disabled) {
-                Some(sandbox_type) => SafetyCheck::AutoApprove {
-                    sandbox_type,
-                    user_explicitly_approved: false,
-                },
+                Some(_) => SafetyCheck::AutoApprove,
                 None => {
                     if rejects_sandbox_approval {
                         SafetyCheck::Reject {

codex-rs/core/src/safety_tests.rs

@@ -88,10 +88,7 @@ fn external_sandbox_auto_approves_in_on_request() {
             &cwd_uri,
             WindowsSandboxLevel::Disabled
         ),
-        SafetyCheck::AutoApprove {
-            sandbox_type: SandboxType::None,
-            user_explicitly_approved: false,
-        }
+        SafetyCheck::AutoApprove
     );
 }
 

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

@@ -7,7 +7,6 @@ use std::time::Duration;
 use std::time::Instant;
 
 use crate::apply_patch;
-use crate::apply_patch::InternalApplyPatchInvocation;
 use crate::apply_patch::convert_apply_patch_to_protocol;
 use crate::function_tool::FunctionCallError;
 use crate::session::session::Session;
@@ -394,82 +393,21 @@ impl ApplyPatchHandler {
         .await
         {
             codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => {
-                let (file_paths, effective_additional_permissions, file_system_sandbox_policy) =
-                    effective_patch_permissions(
-                        session.as_ref(),
-                        turn.as_ref(),
-                        &turn_environment.environment_id,
-                        &changes,
-                        turn_environment.cwd(),
-                    )
-                    .await
-                    .unwrap_or_else(|_| patch_permissions_without_path_matching(&changes));
-                match apply_patch::apply_patch(turn.as_ref(), &file_system_sandbox_policy, changes)
-                    .await
-                {
-                    InternalApplyPatchInvocation::Output(item) => {
-                        let content = item?;
-                        Ok(boxed_tool_output(ApplyPatchToolOutput::from_text(content)))
-                    }
-                    InternalApplyPatchInvocation::DelegateToRuntime(apply) => {
-                        let changes = convert_apply_patch_to_protocol(&apply.action);
-                        let emitter = ToolEmitter::apply_patch_for_environment(
-                            changes.clone(),
-                            apply.auto_approved,
-                            turn_environment.environment_id.clone(),
-                        );
-                        let event_ctx = ToolEventCtx::new(
-                            session.as_ref(),
-                            turn.as_ref(),
-                            &call_id,
-                            Some(&tracker),
-                        );
-                        emitter.begin(event_ctx).await;
-
-                        let req = ApplyPatchRequest {
-                            turn_environment: turn_environment.clone(),
-                            action: apply.action,
-                            file_paths,
-                            changes,
-                            exec_approval_requirement: apply.exec_approval_requirement,
-                            additional_permissions: effective_additional_permissions
-                                .additional_permissions,
-                            permissions_preapproved: effective_additional_permissions
-                                .permissions_preapproved,
-                        };
-
-                        let mut orchestrator = ToolOrchestrator::new();
-                        let mut runtime = ApplyPatchRuntime::new();
-                        let tool_ctx = ToolCtx {
-                            session: session.clone(),
-                            turn: turn.clone(),
-                            call_id: call_id.clone(),
-                            tool_name: tool_name.clone(),
-                        };
-                        let out = orchestrator
-                            .run(
-                                &mut runtime,
-                                &req,
-                                &tool_ctx,
-                                turn.as_ref(),
-                                turn.approval_policy.value(),
-                            )
-                            .await
-                            .map(|result| result.output);
-                        let (out, delta) = match out {
-                            Ok(output) => (Ok(output.exec_output), Some(output.delta)),
-                            Err(error) => (Err(error), Some(runtime.committed_delta().clone())),
-                        };
-                        let event_ctx = ToolEventCtx::new(
-                            session.as_ref(),
-                            turn.as_ref(),
-                            &call_id,
-                            Some(&tracker),
-                        );
-                        let content = emitter.finish(event_ctx, out, delta.as_ref()).await?;
-                        Ok(boxed_tool_output(ApplyPatchToolOutput::from_text(content)))
-                    }
-                }
+                let tool_ctx = ToolCtx {
+                    session,
+                    turn,
+                    call_id,
+                    tool_name,
+                };
+                let content = execute_verified_patch(
+                    changes,
+                    turn_environment.cwd(),
+                    turn_environment.clone(),
+                    Some(&tracker),
+                    tool_ctx,
+                )
+                .await?;
+                Ok(boxed_tool_output(ApplyPatchToolOutput::from_text(content)))
             }
             codex_apply_patch::MaybeApplyPatchVerified::CorrectnessError(parse_error) => {
                 Err(FunctionCallError::RespondToModel(format!(
@@ -558,82 +496,15 @@ pub(crate) async fn intercept_apply_patch(
         .await
     {
         codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => {
-            let (approval_keys, effective_additional_permissions, file_system_sandbox_policy) =
-                effective_patch_permissions(
-                    session.as_ref(),
-                    turn.as_ref(),
-                    &turn_environment.environment_id,
-                    &changes,
-                    cwd,
-                )
-                .await
-                .unwrap_or_else(|_| patch_permissions_without_path_matching(&changes));
-            match apply_patch::apply_patch(turn.as_ref(), &file_system_sandbox_policy, changes)
-                .await
-            {
-                InternalApplyPatchInvocation::Output(item) => {
-                    let content = item?;
-                    Ok(Some(FunctionToolOutput::from_text(content, Some(true))))
-                }
-                InternalApplyPatchInvocation::DelegateToRuntime(apply) => {
-                    let changes = convert_apply_patch_to_protocol(&apply.action);
-                    let emitter = ToolEmitter::apply_patch_for_environment(
-                        changes.clone(),
-                        apply.auto_approved,
-                        turn_environment.environment_id.clone(),
-                    );
-                    let event_ctx = ToolEventCtx::new(
-                        session.as_ref(),
-                        turn.as_ref(),
-                        call_id,
-                        tracker.as_ref().copied(),
-                    );
-                    emitter.begin(event_ctx).await;
-
-                    let req = ApplyPatchRequest {
-                        turn_environment,
-                        action: apply.action,
-                        file_paths: approval_keys,
-                        changes,
-                        exec_approval_requirement: apply.exec_approval_requirement,
-                        additional_permissions: effective_additional_permissions
-                            .additional_permissions,
-                        permissions_preapproved: effective_additional_permissions
-                            .permissions_preapproved,
-                    };
-
-                    let mut orchestrator = ToolOrchestrator::new();
-                    let mut runtime = ApplyPatchRuntime::new();
-                    let tool_ctx = ToolCtx {
-                        session: session.clone(),
-                        turn: turn.clone(),
-                        call_id: call_id.to_string(),
-                        tool_name: ToolName::plain(tool_name),
-                    };
-                    let out = orchestrator
-                        .run(
-                            &mut runtime,
-                            &req,
-                            &tool_ctx,
-                            turn.as_ref(),
-                            turn.approval_policy.value(),
-                        )
-                        .await
-                        .map(|result| result.output);
-                    let (out, delta) = match out {
-                        Ok(output) => (Ok(output.exec_output), Some(output.delta)),
-                        Err(error) => (Err(error), Some(runtime.committed_delta().clone())),
-                    };
-                    let event_ctx = ToolEventCtx::new(
-                        session.as_ref(),
-                        turn.as_ref(),
-                        call_id,
-                        tracker.as_ref().copied(),
-                    );
-                    let content = emitter.finish(event_ctx, out, delta.as_ref()).await?;
-                    Ok(Some(FunctionToolOutput::from_text(content, Some(true))))
-                }
-            }
+            let tool_ctx = ToolCtx {
+                session,
+                turn,
+                call_id: call_id.to_string(),
+                tool_name: ToolName::plain(tool_name),
+            };
+            let content =
+                execute_verified_patch(changes, cwd, turn_environment, tracker, tool_ctx).await?;
+            Ok(Some(FunctionToolOutput::from_text(content, Some(true))))
         }
         codex_apply_patch::MaybeApplyPatchVerified::CorrectnessError(parse_error) => {
             Err(FunctionCallError::RespondToModel(format!(
@@ -648,6 +519,76 @@ pub(crate) async fn intercept_apply_patch(
     }
 }
 
+async fn execute_verified_patch(
+    action: ApplyPatchAction,
+    cwd: &PathUri,
+    turn_environment: TurnEnvironment,
+    tracker: Option<&SharedTurnDiffTracker>,
+    tool_ctx: ToolCtx,
+) -> Result<String, FunctionCallError> {
+    let (file_paths, effective_additional_permissions, file_system_sandbox_policy) =
+        effective_patch_permissions(
+            tool_ctx.session.as_ref(),
+            tool_ctx.turn.as_ref(),
+            &turn_environment.environment_id,
+            &action,
+            cwd,
+        )
+        .await
+        .unwrap_or_else(|_| patch_permissions_without_path_matching(&action));
+    let apply = apply_patch::prepare_apply_patch(
+        tool_ctx.turn.as_ref(),
+        &file_system_sandbox_policy,
+        action,
+    )?;
+    let changes = convert_apply_patch_to_protocol(&apply.action);
+    let emitter = ToolEmitter::apply_patch_for_environment(
+        changes.clone(),
+        apply.auto_approved,
+        turn_environment.environment_id.clone(),
+    );
+    let event_ctx = ToolEventCtx::new(
+        tool_ctx.session.as_ref(),
+        tool_ctx.turn.as_ref(),
+        &tool_ctx.call_id,
+        tracker,
+    );
+    emitter.begin(event_ctx).await;
+
+    let request = ApplyPatchRequest {
+        turn_environment,
+        action: apply.action,
+        file_paths,
+        changes,
+        exec_approval_requirement: apply.exec_approval_requirement,
+        additional_permissions: effective_additional_permissions.additional_permissions,
+        permissions_preapproved: effective_additional_permissions.permissions_preapproved,
+    };
+    let mut orchestrator = ToolOrchestrator::new();
+    let mut runtime = ApplyPatchRuntime::new();
+    let result = orchestrator
+        .run(
+            &mut runtime,
+            &request,
+            &tool_ctx,
+            tool_ctx.turn.as_ref(),
+            tool_ctx.turn.approval_policy.value(),
+        )
+        .await
+        .map(|result| result.output);
+    let (result, delta) = match result {
+        Ok(output) => (Ok(output.exec_output), Some(output.delta)),
+        Err(error) => (Err(error), Some(runtime.committed_delta().clone())),
+    };
+    let event_ctx = ToolEventCtx::new(
+        tool_ctx.session.as_ref(),
+        tool_ctx.turn.as_ref(),
+        &tool_ctx.call_id,
+        tracker,
+    );
+    emitter.finish(event_ctx, result, delta.as_ref()).await
+}
+
 fn require_environment_id(
     parsed_environment_id: Option<&str>,
     allow_environment_id: bool,