Revert "Revert "Overhaul shell detection and centralize command generation for unified exec"" (#6607)

Reverts openai/codex#6606

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

@@ -87,7 +87,7 @@ pub(crate) enum ToolEmitter {
         auto_approved: bool,
     },
     UnifiedExec {
-        command: String,
+        command: Vec<String>,
         cwd: PathBuf,
         // True for `exec_command` and false for `write_stdin`.
         #[allow(dead_code)]
@@ -111,9 +111,9 @@ impl ToolEmitter {
         }
     }
 
-    pub fn unified_exec(command: String, cwd: PathBuf, is_startup_command: bool) -> Self {
+    pub fn unified_exec(command: &[String], cwd: PathBuf, is_startup_command: bool) -> Self {
         Self::UnifiedExec {
-            command,
+            command: command.to_vec(),
             cwd,
             is_startup_command,
         }
@@ -218,7 +218,7 @@ impl ToolEmitter {
                 emit_patch_end(ctx, String::new(), (*message).to_string(), false).await;
             }
             (Self::UnifiedExec { command, cwd, .. }, ToolEventStage::Begin) => {
-                emit_exec_command_begin(ctx, &[command.to_string()], cwd.as_path(), false).await;
+                emit_exec_command_begin(ctx, command, cwd.as_path(), false).await;
             }
             (Self::UnifiedExec { .. }, ToolEventStage::Success(output)) => {
                 emit_exec_end(

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

@@ -324,8 +324,11 @@ impl ShellHandler {
 
 #[cfg(test)]
 mod tests {
+    use std::path::PathBuf;
+
     use crate::is_safe_command::is_known_safe_command;
     use crate::shell::BashShell;
+    use crate::shell::PowerShellConfig;
     use crate::shell::Shell;
     use crate::shell::ZshShell;
 
@@ -335,27 +338,19 @@ mod tests {
     #[test]
     fn commands_generated_by_shell_command_handler_can_be_matched_by_is_known_safe_command() {
         let bash_shell = Shell::Bash(BashShell {
-            shell_path: "/bin/bash".to_string(),
-            bashrc_path: "/home/user/.bashrc".to_string(),
+            shell_path: PathBuf::from("/bin/bash"),
         });
         assert_safe(&bash_shell, "ls -la");
 
         let zsh_shell = Shell::Zsh(ZshShell {
-            shell_path: "/bin/zsh".to_string(),
-            zshrc_path: "/home/user/.zshrc".to_string(),
+            shell_path: PathBuf::from("/bin/zsh"),
         });
         assert_safe(&zsh_shell, "ls -la");
 
-        #[cfg(target_os = "windows")]
-        {
-            use crate::shell::PowerShellConfig;
-
-            let powershell = Shell::PowerShell(PowerShellConfig {
-                exe: "pwsh.exe".to_string(),
-                bash_exe_fallback: None,
-            });
-            assert_safe(&powershell, "ls -Name");
-        }
+        let powershell = Shell::PowerShell(PowerShellConfig {
+            shell_path: PathBuf::from("pwsh.exe"),
+        });
+        assert_safe(&powershell, "ls -Name");
     }
 
     fn assert_safe(shell: &Shell, command: &str) {

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

@@ -5,6 +5,7 @@ use crate::is_safe_command::is_known_safe_command;
 use crate::protocol::EventMsg;
 use crate::protocol::ExecCommandOutputDeltaEvent;
 use crate::protocol::ExecOutputStream;
+use crate::shell::get_shell_by_model_provided_path;
 use crate::tools::context::ToolInvocation;
 use crate::tools::context::ToolOutput;
 use crate::tools::context::ToolPayload;
@@ -92,7 +93,8 @@ impl ToolHandler for UnifiedExecHandler {
         let Ok(params) = serde_json::from_str::<ExecCommandArgs>(arguments) else {
             return true;
         };
-        !is_known_safe_command(&["bash".to_string(), "-lc".to_string(), params.cmd])
+        let command = get_command(&params);
+        !is_known_safe_command(&command)
     }
 
     async fn handle(&self, invocation: ToolInvocation) -> Result<ToolOutput, FunctionCallError> {
@@ -125,15 +127,15 @@ impl ToolHandler for UnifiedExecHandler {
                         "failed to parse exec_command arguments: {err:?}"
                     ))
                 })?;
+
+                let command = get_command(&args);
                 let ExecCommandArgs {
-                    cmd,
                     workdir,
-                    shell,
-                    login,
                     yield_time_ms,
                     max_output_tokens,
                     with_escalated_permissions,
                     justification,
+                    ..
                 } = args;
 
                 if with_escalated_permissions.unwrap_or(false)
@@ -160,15 +162,14 @@ impl ToolHandler for UnifiedExecHandler {
                     &context.call_id,
                     None,
                 );
-                let emitter = ToolEmitter::unified_exec(cmd.clone(), cwd.clone(), true);
+
+                let emitter = ToolEmitter::unified_exec(&command, cwd.clone(), true);
                 emitter.emit(event_ctx, ToolEventStage::Begin).await;
 
                 manager
                     .exec_command(
                         ExecCommandRequest {
-                            command: &cmd,
-                            shell: &shell,
-                            login,
+                            command,
                             yield_time_ms,
                             max_output_tokens,
                             workdir,
@@ -229,6 +230,11 @@ impl ToolHandler for UnifiedExecHandler {
     }
 }
 
+fn get_command(args: &ExecCommandArgs) -> Vec<String> {
+    let shell = get_shell_by_model_provided_path(&PathBuf::from(args.shell.clone()));
+    shell.derive_exec_args(&args.cmd, args.login)
+}
+
 fn format_response(response: &UnifiedExecResponse) -> String {
     let mut sections = Vec::new();