Extract unified exec command preparation into a reusable module (#57435)

## What changed

Move argument parsing, working-directory and shell resolution, permission validation, and command lifetime setup into `unified_exec::preparation` with crate-visible parsed and prepared command types.

Update `ExecCommandHandler` to use the extracted preparation flow, preserving implicit skill event emission between parsing and preparation.

GitOrigin-RevId: 79e9be5d87c1b0ab905afdf1e343ac141fb7d4c6

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

@@ -284,7 +284,7 @@ pub(crate) fn normalize_and_validate_additional_permissions(
     }
 }
 
-pub(super) struct EffectiveAdditionalPermissions {
+pub(crate) struct EffectiveAdditionalPermissions {
     pub sandbox_permissions: SandboxPermissions,
     pub additional_permissions: Option<AdditionalPermissionProfile>,
     pub permissions_preapproved: bool,

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

@@ -20,6 +20,7 @@ use std::path::PathBuf;
 use std::sync::Arc;
 
 mod exec_command;
+pub(crate) mod preparation;
 mod write_stdin;
 
 pub use exec_command::ExecCommandHandler;

codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs

@@ -1,9 +1,5 @@
-use std::path::Path;
 use std::sync::Arc;
-use std::time::Duration;
 
-use crate::exec::DEFAULT_EXEC_COMMAND_TIMEOUT_MS;
-use crate::exec_policy::prompt_is_rejected_by_policy;
 use crate::function_tool::FunctionCallError;
 use crate::maybe_emit_implicit_skill_invocation;
 use crate::tools::context::ExecCommandToolOutput;
@@ -12,15 +8,9 @@ use crate::tools::context::ToolInvocation;
 use crate::tools::context::ToolPayload;
 use crate::tools::context::boxed_tool_output;
 use crate::tools::handlers::ToolEnvironmentArgs;
-use crate::tools::handlers::apply_granted_turn_permissions;
 use crate::tools::handlers::apply_patch::intercept_apply_patch;
-use crate::tools::handlers::file_system_sandbox_policy_context_for_cwd;
 use crate::tools::handlers::function_tool_environment;
-use crate::tools::handlers::implicit_granted_permissions;
-use crate::tools::handlers::normalize_and_validate_additional_permissions;
 use crate::tools::handlers::parse_arguments;
-use crate::tools::handlers::parse_arguments_with_base_path;
-use crate::tools::handlers::resolve_sandbox_permissions;
 use crate::tools::handlers::rewrite_function_string_argument;
 use crate::tools::handlers::selected_executor;
 use crate::tools::handlers::updated_hook_command;
@@ -39,15 +29,10 @@ use crate::unified_exec::output::CapturedStream;
 use codex_features::Feature;
 use codex_otel::SessionTelemetry;
 use codex_otel::TOOL_CALL_UNIFIED_EXEC_METRIC;
-use codex_sandboxing::SandboxManager;
-use codex_sandboxing::SandboxType;
-use codex_sandboxing::SandboxablePreference;
-use codex_shell_command::shell_detect::detect_shell_type;
 use codex_tools::JsonSchema;
 use codex_tools::ToolName;
 use codex_tools::ToolSpec;
 use codex_utils_output_truncation::approx_token_count;
-use codex_utils_path_uri::PathConvention;
 use codex_utils_path_uri::PathUri;
 use codex_utils_string::truncate_middle_chars;
 
@@ -55,8 +40,9 @@ use super::super::shell_spec::CommandToolOptions;
 use super::super::shell_spec::create_exec_command_tool_with_environment_id;
 use super::ExecCommandArgs;
 use super::ExecCommandWorkdirArgs;
-use super::get_command;
 use super::post_unified_exec_tool_use_payload;
+use super::preparation::ExecCommandLifetime;
+use super::preparation::ParsedExecCommand;
 
 // A byte limit is a conservative hard token bound even for byte-fallback tokenizers.
 const EXEC_COMMAND_REJECTION_MAX_BYTES: usize = 900;
@@ -69,12 +55,6 @@ pub(crate) struct ExecCommandHandlerOptions {
     pub(crate) include_windows_shell_guidance: bool,
 }
 
-#[derive(Clone, Copy)]
-enum ExecCommandLifetime {
-    Interactive,
-    OneShot,
-}
-
 pub struct ExecCommandHandler {
     options: ExecCommandHandlerOptions,
     lifetime: ExecCommandLifetime,
@@ -158,7 +138,6 @@ impl ExecCommandHandler {
         let ToolInvocation {
             environments,
             session,
-            turn,
             step_context,
             cancellation_token,
             tracker,
@@ -183,173 +162,24 @@ impl ExecCommandHandler {
             cancellation_token,
             call_id.clone(),
         );
-        let workdir_args: ExecCommandWorkdirArgs = parse_arguments(&arguments)?;
         let turn_environment = selected_executor(&environments, &step_context.environments)?;
-        let cwd = workdir_args.resolve_cwd(turn_environment)?;
-        let environment = Arc::clone(&turn_environment.environment);
-        let fs = environment.get_filesystem();
-
-        // Remote executors enforce URI-native sandbox policy themselves. Only a host-local
-        // sandbox needs a native cwd for resolving paths nested in the permissions config.
-        let requires_host_native_cwd = !environment.is_remote()
-            && SandboxManager::new().select_initial(
-                turn_environment.permission_profile(),
-                SandboxablePreference::Auto,
-                codex_protocol::sandbox::effective_windows_sandbox_type(
-                    turn_environment.config().windows_sandbox_type,
-                    turn_environment.config().windows_sandbox_level,
-                ),
-                turn.network.is_some(),
-            ) != SandboxType::None;
-        // `to_abs_path()` alone cannot identify foreign drive paths: `file:///C:/repo` is
-        // representable as `/C:/repo` on POSIX. Require the inferred convention to match too.
-        let cwd_uses_native_convention =
-            cwd.infer_path_convention() == Some(PathConvention::native());
-        let native_cwd = match cwd.to_abs_path() {
-            Ok(cwd) if cwd_uses_native_convention => Some(cwd),
-            _ if !requires_host_native_cwd => None,
-            Err(err) => return Err(FunctionCallError::RespondToModel(err.to_string())),
-            Ok(_) => {
-                return Err(FunctionCallError::RespondToModel(format!(
-                    "path URI `{cwd}` does not use the host's native {} path convention",
-                    PathConvention::native()
-                )));
-            }
-        };
-        let mut args: ExecCommandArgs = match native_cwd.as_ref() {
-            Some(native_cwd) => {
-                // The base path only resolves paths nested in the permissions config types.
-                parse_arguments_with_base_path(&arguments, native_cwd)?
-            }
-            None => {
-                // Foreign executor cwd values cannot seed this host's AbsolutePathBufGuard.
-                // Sandbox intent and URI-native roots are still sent to the executor.
-                parse_arguments(&arguments)?
-            }
-        };
-        if args.tty && !session.features().enabled(Feature::UnifiedExecTty) {
-            return Err(FunctionCallError::RespondToModel(
-                "TTY execution is disabled by config; omit `tty` or set it to false.".to_string(),
-            ));
-        }
-        let sandbox_permissions =
-            resolve_sandbox_permissions(args.sandbox_permissions, args.justification.as_deref())?;
-        let hook_command = args.cmd.clone();
+        let parsed =
+            ParsedExecCommand::parse(&arguments, &session, &step_context, turn_environment)?;
         maybe_emit_implicit_skill_invocation(
-            session.as_ref(),
-            context.step_context.as_ref(),
-            &hook_command,
-            &cwd,
-            native_cwd.as_ref(),
+            &session,
+            &step_context,
+            &parsed.args.cmd,
+            &parsed.cwd,
+            parsed.native_cwd.as_ref(),
             &turn_environment.selection.environment_id,
         )
         .await;
-        // Remote environments may use a different OS and must build commands with their native
-        // shell; fall back to the session shell when the environment did not report one.
-        let shell = turn_environment
-            .shell
-            .clone()
-            .map(Arc::new)
-            .unwrap_or_else(|| session.user_shell());
-        // TODO(anp): Resolve requested shells in remote environments instead of restricting
-        // commands to the reported default shell.
-        if environment.is_remote()
-            && let Some(requested_shell) = args.shell.take()
-        {
-            let Some(remote_shell) = turn_environment.shell.as_ref() else {
-                return Err(FunctionCallError::RespondToModel(format!(
-                    "environment `{}` does not report a shell",
-                    turn_environment.selection.environment_id
-                )));
-            };
-            if detect_shell_type(Path::new(&requested_shell)) != Some(remote_shell.shell_type) {
-                return Err(FunctionCallError::RespondToModel(format!(
-                    "environment `{}` only supports `{}`",
-                    turn_environment.selection.environment_id,
-                    remote_shell.name()
-                )));
-            }
-        }
-        let resolved_command =
-            get_command(&args, shell, turn_environment.config().allow_login_shell)
-                .map_err(FunctionCallError::RespondToModel)?;
-        let command = resolved_command.command;
-        let ExecCommandArgs {
-            mut tty,
-            yield_time_ms,
-            timeout_ms,
-            max_output_tokens,
-            sandbox_permissions: _,
-            additional_permissions,
-            justification,
-            prefix_rule,
-            ..
-        } = args;
-        let completion_timeout = match self.lifetime {
-            ExecCommandLifetime::Interactive => None,
-            ExecCommandLifetime::OneShot => {
-                tty = false;
-                Some(Duration::from_millis(
-                    timeout_ms.unwrap_or(DEFAULT_EXEC_COMMAND_TIMEOUT_MS),
-                ))
-            }
-        };
-
-        let exec_permission_approvals_enabled =
-            session.features().enabled(Feature::ExecPermissionApprovals);
-        let requested_additional_permissions = additional_permissions.clone();
-        let sandbox_context =
-            turn_environment.sandbox_context(/*additional_permissions*/ None);
-        let permission_context = file_system_sandbox_policy_context_for_cwd(&sandbox_context, &cwd);
-        let effective_additional_permissions = apply_granted_turn_permissions(
-            &context.step_context,
-            turn_environment,
-            &cwd,
-            sandbox_permissions,
-            additional_permissions,
-        );
-        let additional_permissions_allowed = exec_permission_approvals_enabled
-            || (session.features().enabled(Feature::RequestPermissionsTool)
-                && effective_additional_permissions.permissions_preapproved);
-
-        // Sticky turn permissions have already been approved, so they should
-        // continue through the normal exec approval flow for the command.
-        let approval_policy = context.step_context.settings.approval_policy();
-        if effective_additional_permissions
-            .sandbox_permissions
-            .requests_sandbox_override()
-            && !effective_additional_permissions.permissions_preapproved
-            && prompt_is_rejected_by_policy(approval_policy, /*prompt_is_rule*/ false).is_some()
-        {
-            return Err(FunctionCallError::RespondToModel(format!(
-                "approval policy is {approval_policy:?}; reject command — you cannot ask for escalated permissions if the approval policy is {approval_policy:?}"
-            )));
-        }
-
-        let normalized_additional_permissions = match implicit_granted_permissions(
-            sandbox_permissions,
-            requested_additional_permissions.as_ref(),
-            &effective_additional_permissions,
-        )
-        .map_or_else(
-            || {
-                normalize_and_validate_additional_permissions(
-                    additional_permissions_allowed,
-                    approval_policy,
-                    effective_additional_permissions.sandbox_permissions,
-                    effective_additional_permissions.additional_permissions,
-                    effective_additional_permissions.permissions_preapproved,
-                    &permission_context,
-                )
-            },
-            |permissions| Ok(Some(permissions)),
-        ) {
-            Ok(normalized) => normalized,
-            Err(err) => {
-                return Err(FunctionCallError::RespondToModel(err));
-            }
-        };
-
+        let prepared = parsed.prepare(self.lifetime)?;
+        let command = prepared.resolved.command;
+        let cwd = prepared.cwd;
+        let hook_command = prepared.hook_command;
+        let max_output_tokens = prepared.max_output_tokens;
+        let fs = turn_environment.environment.get_filesystem();
         let intercepted_patch = intercept_apply_patch(
             &command,
             &cwd,
@@ -382,7 +212,7 @@ impl ExecCommandHandler {
             }));
         }
 
-        emit_unified_exec_tty_metric(&step_context.session_telemetry, tty);
+        emit_unified_exec_tty_metric(&step_context.session_telemetry, prepared.tty);
         crate::tools::lifecycle::notify_command_start(
             context.session.as_ref(),
             context.step_context.turn.as_ref(),
@@ -396,24 +226,23 @@ impl ExecCommandHandler {
         let process_id = manager.allocate_process_id().await;
         let request = ExecCommandRequest {
             command,
-            shell: resolved_command.shell,
+            shell: prepared.resolved.shell,
             hook_command: hook_command.clone(),
             process_id,
-            yield_time_ms,
+            yield_time_ms: prepared.yield_time_ms,
             max_output_tokens,
             cwd: cwd.clone(),
             sandbox_cwd: turn_environment.cwd().clone(),
             turn_environment: turn_environment.clone(),
             network: context.step_context.turn.network.clone(),
-            tty,
-            sandbox_permissions: effective_additional_permissions.sandbox_permissions,
-            additional_permissions: normalized_additional_permissions,
-            additional_permissions_preapproved: effective_additional_permissions
-                .permissions_preapproved,
-            justification,
-            prefix_rule,
+            tty: prepared.tty,
+            sandbox_permissions: prepared.permissions.sandbox_permissions,
+            additional_permissions: prepared.permissions.additional_permissions,
+            additional_permissions_preapproved: prepared.permissions.permissions_preapproved,
+            justification: prepared.justification,
+            prefix_rule: prepared.prefix_rule,
         };
-        let result = match completion_timeout {
+        let result = match prepared.completion_timeout {
             Some(timeout) => {
                 UnifiedExecProcessManager::exec_command_to_completion(request, &context, timeout)
                     .await

codex-rs/core/src/tools/handlers/unified_exec/preparation.rs

@@ -0,0 +1,247 @@
+//! Resolves exec arguments against the selected environment and effective turn permissions.
+//! The live handler emits its skill event between parsing and preparation.
+
+use super::ExecCommandArgs;
+use super::ExecCommandWorkdirArgs;
+use super::ResolvedCommand;
+use super::get_command;
+use crate::exec::DEFAULT_EXEC_COMMAND_TIMEOUT_MS;
+use crate::exec_policy::prompt_is_rejected_by_policy;
+use crate::function_tool::FunctionCallError;
+use crate::sandboxing::SandboxPermissions;
+use crate::session::session::Session;
+use crate::session::step_context::StepContext;
+use crate::session::turn_context::TurnEnvironment;
+use crate::tools::handlers::EffectiveAdditionalPermissions;
+use crate::tools::handlers::apply_granted_turn_permissions;
+use crate::tools::handlers::file_system_sandbox_policy_context_for_cwd;
+use crate::tools::handlers::implicit_granted_permissions;
+use crate::tools::handlers::normalize_and_validate_additional_permissions;
+use crate::tools::handlers::parse_arguments;
+use crate::tools::handlers::parse_arguments_with_base_path;
+use crate::tools::handlers::resolve_sandbox_permissions;
+use codex_features::Feature;
+use codex_sandboxing::SandboxManager;
+use codex_sandboxing::SandboxType;
+use codex_sandboxing::SandboxablePreference;
+use codex_shell_command::shell_detect::detect_shell_type;
+use codex_utils_absolute_path::AbsolutePathBuf;
+use codex_utils_path_uri::PathConvention;
+use codex_utils_path_uri::PathUri;
+use std::path::Path;
+use std::sync::Arc;
+use std::time::Duration;
+
+#[derive(Clone, Copy)]
+pub(crate) enum ExecCommandLifetime {
+    Interactive,
+    OneShot,
+}
+
+/// Command details available before the live handler reserves a process.
+pub(crate) struct PreparedExecCommand {
+    pub(crate) resolved: ResolvedCommand,
+    pub(crate) hook_command: String,
+    pub(crate) cwd: PathUri,
+    pub(crate) permissions: EffectiveAdditionalPermissions,
+    pub(crate) justification: Option<String>,
+    pub(crate) prefix_rule: Option<Vec<String>>,
+    pub(crate) tty: bool,
+    pub(crate) yield_time_ms: u64,
+    pub(crate) max_output_tokens: Option<usize>,
+    pub(crate) completion_timeout: Option<Duration>,
+}
+
+pub(crate) struct ParsedExecCommand<'a> {
+    session: &'a Session,
+    step: &'a StepContext,
+    environment: &'a TurnEnvironment,
+    pub(super) args: ExecCommandArgs,
+    pub(super) cwd: PathUri,
+    pub(super) native_cwd: Option<AbsolutePathBuf>,
+    sandbox_permissions: SandboxPermissions,
+}
+
+impl<'a> ParsedExecCommand<'a> {
+    /// Resolve this call's workdir and arguments using the executor selected by the caller.
+    pub(crate) fn parse(
+        arguments: &str,
+        session: &'a Session,
+        step: &'a StepContext,
+        turn_environment: &'a TurnEnvironment,
+    ) -> Result<Self, FunctionCallError> {
+        let workdir: ExecCommandWorkdirArgs = parse_arguments(arguments)?;
+        let cwd = workdir.resolve_cwd(turn_environment)?;
+        let environment = &turn_environment.environment;
+
+        // Remote executors enforce URI-native sandbox policy themselves. Only a host-local
+        // sandbox needs a native cwd for resolving paths nested in the permissions config.
+        let requires_host_native_cwd = !environment.is_remote()
+            && SandboxManager::new().select_initial(
+                turn_environment.permission_profile(),
+                SandboxablePreference::Auto,
+                codex_protocol::sandbox::effective_windows_sandbox_type(
+                    turn_environment.config().windows_sandbox_type,
+                    turn_environment.config().windows_sandbox_level,
+                ),
+                step.turn.network.is_some(),
+            ) != SandboxType::None;
+        // `to_abs_path()` alone cannot identify foreign drive paths: `file:///C:/repo` is
+        // representable as `/C:/repo` on POSIX. Require the inferred convention to match too.
+        let cwd_uses_native_convention =
+            cwd.infer_path_convention() == Some(PathConvention::native());
+        let native_cwd = match cwd.to_abs_path() {
+            Ok(cwd) if cwd_uses_native_convention => Some(cwd),
+            _ if !requires_host_native_cwd => None,
+            Err(err) => return Err(FunctionCallError::RespondToModel(err.to_string())),
+            Ok(_) => {
+                return Err(FunctionCallError::RespondToModel(format!(
+                    "path URI `{cwd}` does not use the host's native {} path convention",
+                    PathConvention::native()
+                )));
+            }
+        };
+        let args: ExecCommandArgs = match native_cwd.as_ref() {
+            Some(native_cwd) => {
+                // The base path only resolves paths nested in the permissions config types.
+                parse_arguments_with_base_path(arguments, native_cwd)?
+            }
+            None => {
+                // Foreign executor cwd values cannot seed this host's AbsolutePathBufGuard.
+                // Sandbox intent and URI-native roots are still sent to the executor.
+                parse_arguments(arguments)?
+            }
+        };
+        if args.tty && !session.features().enabled(Feature::UnifiedExecTty) {
+            return Err(FunctionCallError::RespondToModel(
+                "TTY execution is disabled by config; omit `tty` or set it to false.".to_string(),
+            ));
+        }
+        let sandbox_permissions =
+            resolve_sandbox_permissions(args.sandbox_permissions, args.justification.as_deref())?;
+        Ok(Self {
+            session,
+            step,
+            environment: turn_environment,
+            args,
+            cwd,
+            native_cwd,
+            sandbox_permissions,
+        })
+    }
+
+    pub(crate) fn prepare(
+        self,
+        lifetime: ExecCommandLifetime,
+    ) -> Result<PreparedExecCommand, FunctionCallError> {
+        let Self {
+            session,
+            step,
+            environment: turn_environment,
+            mut args,
+            cwd,
+            sandbox_permissions,
+            ..
+        } = self;
+        // Remote environments may use a different OS and must build commands with their native
+        // shell; fall back to the session shell when the environment did not report one.
+        let shell = turn_environment
+            .shell
+            .clone()
+            .map(Arc::new)
+            .unwrap_or_else(|| session.user_shell());
+        // TODO(anp): Resolve requested shells in remote environments instead of restricting
+        // commands to the reported default shell.
+        if turn_environment.environment.is_remote()
+            && let Some(requested_shell) = args.shell.take()
+        {
+            let Some(remote_shell) = turn_environment.shell.as_ref() else {
+                return Err(FunctionCallError::RespondToModel(format!(
+                    "environment `{}` does not report a shell",
+                    turn_environment.selection.environment_id
+                )));
+            };
+            if detect_shell_type(Path::new(&requested_shell)) != Some(remote_shell.shell_type) {
+                return Err(FunctionCallError::RespondToModel(format!(
+                    "environment `{}` only supports `{}`",
+                    turn_environment.selection.environment_id,
+                    remote_shell.name()
+                )));
+            }
+        }
+        let resolved_command =
+            get_command(&args, shell, turn_environment.config().allow_login_shell)
+                .map_err(FunctionCallError::RespondToModel)?;
+        let completion_timeout = match lifetime {
+            ExecCommandLifetime::Interactive => None,
+            ExecCommandLifetime::OneShot => {
+                args.tty = false;
+                Some(Duration::from_millis(
+                    args.timeout_ms.unwrap_or(DEFAULT_EXEC_COMMAND_TIMEOUT_MS),
+                ))
+            }
+        };
+
+        let exec_permission_approvals_enabled =
+            session.features().enabled(Feature::ExecPermissionApprovals);
+        let requested_additional_permissions = args.additional_permissions.take();
+        let sandbox_context =
+            turn_environment.sandbox_context(/*additional_permissions*/ None);
+        let permission_context = file_system_sandbox_policy_context_for_cwd(&sandbox_context, &cwd);
+        let mut permissions = apply_granted_turn_permissions(
+            step,
+            turn_environment,
+            &cwd,
+            sandbox_permissions,
+            requested_additional_permissions.clone(),
+        );
+        let additional_permissions_allowed = exec_permission_approvals_enabled
+            || (session.features().enabled(Feature::RequestPermissionsTool)
+                && permissions.permissions_preapproved);
+
+        // Sticky turn permissions have already been approved, so they should
+        // continue through the normal exec approval flow for the command.
+        let approval_policy = step.settings.approval_policy();
+        if permissions.sandbox_permissions.requests_sandbox_override()
+            && !permissions.permissions_preapproved
+            && prompt_is_rejected_by_policy(approval_policy, /*prompt_is_rule*/ false).is_some()
+        {
+            return Err(FunctionCallError::RespondToModel(format!(
+                "approval policy is {approval_policy:?}; reject command — you cannot ask for escalated permissions if the approval policy is {approval_policy:?}"
+            )));
+        }
+
+        permissions.additional_permissions = implicit_granted_permissions(
+            sandbox_permissions,
+            requested_additional_permissions.as_ref(),
+            &permissions,
+        )
+        .map_or_else(
+            || {
+                normalize_and_validate_additional_permissions(
+                    additional_permissions_allowed,
+                    approval_policy,
+                    permissions.sandbox_permissions,
+                    permissions.additional_permissions,
+                    permissions.permissions_preapproved,
+                    &permission_context,
+                )
+            },
+            |permissions| Ok(Some(permissions)),
+        )
+        .map_err(FunctionCallError::RespondToModel)?;
+
+        Ok(PreparedExecCommand {
+            resolved: resolved_command,
+            hook_command: args.cmd,
+            yield_time_ms: args.yield_time_ms,
+            max_output_tokens: args.max_output_tokens,
+            completion_timeout,
+            cwd,
+            tty: args.tty,
+            permissions,
+            justification: args.justification,
+            prefix_rule: args.prefix_rule,
+        })
+    }
+}