valknar/llmx
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

chore: drop useless shell stuff (#5848)

Browse Source
This commit is contained in:
jif-oai
2025-10-28 14:52:52 +00:00
committed by GitHub
parent 7ff142d93f
commit be4bdfec93
1 changed files with 47 additions and 187 deletions
Download Patch File Download Diff File Expand all files Collapse all files

234
codex-rs/core/src/shell.rs
View File

@@ -1,6 +1,5 @@
use serde::Deserialize; use serde::Deserialize;
use serde::Serialize; use serde::Serialize;
use shlex;
use std::path::PathBuf; use std::path::PathBuf;
#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
@@ -30,67 +29,6 @@ pub enum Shell {
} }
impl Shell { impl Shell {
pub fn format_default_shell_invocation(&self, command: Vec<String>) -> Option<Vec<String>> {
match self {
Shell::Zsh(zsh) => format_shell_invocation_with_rc(
command.as_slice(),
&zsh.shell_path,
&zsh.zshrc_path,
),
Shell::Bash(bash) => format_shell_invocation_with_rc(
command.as_slice(),
&bash.shell_path,
&bash.bashrc_path,
),
Shell::PowerShell(ps) => {
// If model generated a bash command, prefer a detected bash fallback
if let Some(script) = strip_bash_lc(command.as_slice()) {
return match &ps.bash_exe_fallback {
Some(bash) => Some(vec![
bash.to_string_lossy().to_string(),
"-lc".to_string(),
script,
]),
// No bash fallback → run the script under PowerShell.
// It will likely fail (except for some simple commands), but the error
// should give a clue to the model to fix upon retry that it's running under PowerShell.
None => Some(vec![
ps.exe.clone(),
"-NoProfile".to_string(),
"-Command".to_string(),
script,
]),
};
}
// Not a bash command. If model did not generate a PowerShell command,
// turn it into a PowerShell command.
let first = command.first().map(String::as_str);
if first != Some(ps.exe.as_str()) {
// TODO (CODEX_2900): Handle escaping newlines.
if command.iter().any(|a| a.contains('\n') || a.contains('\r')) {
return Some(command);
}
let joined = shlex::try_join(command.iter().map(String::as_str)).ok();
return joined.map(|arg| {
vec![
ps.exe.clone(),
"-NoProfile".to_string(),
"-Command".to_string(),
arg,
]
});
}
// Model generated a PowerShell command. Run it.
Some(command)
}
Shell::Unknown => None,
}
}
pub fn name(&self) -> Option<String> { pub fn name(&self) -> Option<String> {
match self { match self {
Shell::Zsh(zsh) => std::path::Path::new(&zsh.shell_path) Shell::Zsh(zsh) => std::path::Path::new(&zsh.shell_path)
@@ -105,36 +43,6 @@ impl Shell {
} }
} }
fn format_shell_invocation_with_rc(
command: &[String],
shell_path: &str,
rc_path: &str,
) -> Option<Vec<String>> {
let joined = strip_bash_lc(command)
.or_else(|| shlex::try_join(command.iter().map(String::as_str)).ok())?;
let rc_command = if std::path::Path::new(rc_path).exists() {
format!("source {rc_path} && ({joined})")
} else {
joined
};
Some(vec![shell_path.to_string(), "-lc".to_string(), rc_command])
}
fn strip_bash_lc(command: &[String]) -> Option<String> {
match command {
// exactly three items
[first, second, third]
// first two must be "bash", "-lc"
if first == "bash" && second == "-lc" =>
{
Some(third.clone())
}
_ => None,
}
}
#[cfg(unix)] #[cfg(unix)]
fn detect_default_user_shell() -> Shell { fn detect_default_user_shell() -> Shell {
use libc::getpwuid; use libc::getpwuid;
@@ -223,8 +131,8 @@ pub async fn default_user_shell() -> Shell {
#[cfg(unix)] #[cfg(unix)]
mod tests { mod tests {
use super::*; use super::*;
use std::path::PathBuf;
use std::process::Command; use std::process::Command;
use std::string::ToString;
#[tokio::test] #[tokio::test]
async fn test_current_shell_detects_zsh() { async fn test_current_shell_detects_zsh() {
@@ -247,40 +155,6 @@ mod tests {
} }
} }
#[tokio::test]
async fn test_run_with_profile_zshrc_not_exists() {
let shell = Shell::Zsh(ZshShell {
shell_path: "/bin/zsh".to_string(),
zshrc_path: "/does/not/exist/.zshrc".to_string(),
});
let actual_cmd = shell.format_default_shell_invocation(vec!["myecho".to_string()]);
assert_eq!(
actual_cmd,
Some(vec![
"/bin/zsh".to_string(),
"-lc".to_string(),
"myecho".to_string()
])
);
}
#[tokio::test]
async fn test_run_with_profile_bashrc_not_exists() {
let shell = Shell::Bash(BashShell {
shell_path: "/bin/bash".to_string(),
bashrc_path: "/does/not/exist/.bashrc".to_string(),
});
let actual_cmd = shell.format_default_shell_invocation(vec!["myecho".to_string()]);
assert_eq!(
actual_cmd,
Some(vec![
"/bin/bash".to_string(),
"-lc".to_string(),
"myecho".to_string()
])
);
}
#[tokio::test] #[tokio::test]
async fn test_run_with_profile_bash_escaping_and_execution() { async fn test_run_with_profile_bash_escaping_and_execution() {
let shell_path = "/bin/bash"; let shell_path = "/bin/bash";
@@ -315,30 +189,21 @@ mod tests {
std::fs::write( std::fs::write(
&bashrc_path, &bashrc_path,
r#" r#"
set -x set -x
function myecho { function myecho {
echo 'It works!' echo 'It works!'
} }
"#, "#,
) )
.unwrap(); .unwrap();
let shell = Shell::Bash(BashShell { let command = expected_cmd
shell_path: shell_path.to_string(),
bashrc_path: bashrc_path.to_str().unwrap().to_string(),
});
let actual_cmd = shell
.format_default_shell_invocation(input.iter().map(ToString::to_string).collect());
let expected_cmd = expected_cmd
.iter() .iter()
.map(|s| s.replace("BASHRC_PATH", bashrc_path.to_str().unwrap())) .map(|s| s.replace("BASHRC_PATH", bashrc_path.to_str().unwrap()))
.collect(); .collect::<Vec<_>>();
assert_eq!(actual_cmd, Some(expected_cmd));
let output = process_exec_tool_call( let output = process_exec_tool_call(
ExecParams { ExecParams {
command: actual_cmd.unwrap(), command: command.clone(),
cwd: PathBuf::from(temp_home.path()), cwd: PathBuf::from(temp_home.path()),
timeout_ms: None, timeout_ms: None,
env: HashMap::from([( env: HashMap::from([(
@@ -372,8 +237,7 @@ mod tests {
#[cfg(test)] #[cfg(test)]
#[cfg(target_os = "macos")] #[cfg(target_os = "macos")]
mod macos_tests { mod macos_tests {
use super::*; use std::path::PathBuf;
use std::string::ToString;
#[tokio::test] #[tokio::test]
async fn test_run_with_profile_escaping_and_execution() { async fn test_run_with_profile_escaping_and_execution() {
@@ -411,43 +275,32 @@ mod macos_tests {
]; ];
for (input, expected_cmd, expected_output) in cases { for (input, expected_cmd, expected_output) in cases {
use std::collections::HashMap; use std::collections::HashMap;
use std::path::PathBuf;
use crate::exec::ExecParams; use crate::exec::ExecParams;
use crate::exec::SandboxType; use crate::exec::SandboxType;
use crate::exec::process_exec_tool_call; use crate::exec::process_exec_tool_call;
use crate::protocol::SandboxPolicy; use crate::protocol::SandboxPolicy;
// create a temp directory with a zshrc file in it
let temp_home = tempfile::tempdir().unwrap(); let temp_home = tempfile::tempdir().unwrap();
let zshrc_path = temp_home.path().join(".zshrc"); let zshrc_path = temp_home.path().join(".zshrc");
std::fs::write( std::fs::write(
&zshrc_path, &zshrc_path,
r#" r#"
set -x set -x
function myecho { function myecho {
echo 'It works!' echo 'It works!'
} }
"#, "#,
) )
.unwrap(); .unwrap();
let shell = Shell::Zsh(ZshShell { let command = expected_cmd
shell_path: shell_path.to_string(),
zshrc_path: zshrc_path.to_str().unwrap().to_string(),
});
let actual_cmd = shell
.format_default_shell_invocation(input.iter().map(ToString::to_string).collect());
let expected_cmd = expected_cmd
.iter() .iter()
.map(|s| s.replace("ZSHRC_PATH", zshrc_path.to_str().unwrap())) .map(|s| s.replace("ZSHRC_PATH", zshrc_path.to_str().unwrap()))
.collect(); .collect::<Vec<_>>();
assert_eq!(actual_cmd, Some(expected_cmd));
// Actually run the command and check output/exit code
let output = process_exec_tool_call( let output = process_exec_tool_call(
ExecParams { ExecParams {
command: actual_cmd.unwrap(), command: command.clone(),
cwd: PathBuf::from(temp_home.path()), cwd: PathBuf::from(temp_home.path()),
timeout_ms: None, timeout_ms: None,
env: HashMap::from([( env: HashMap::from([(
@@ -485,36 +338,38 @@ mod tests_windows {
#[test] #[test]
fn test_format_default_shell_invocation_powershell() { fn test_format_default_shell_invocation_powershell() {
use std::path::PathBuf;
let cases = vec![ let cases = vec![
( (
Shell::PowerShell(PowerShellConfig { PowerShellConfig {
exe: "pwsh.exe".to_string(), exe: "pwsh.exe".to_string(),
bash_exe_fallback: None, bash_exe_fallback: None,
}), },
vec!["bash", "-lc", "echo hello"], vec!["bash", "-lc", "echo hello"],
vec!["pwsh.exe", "-NoProfile", "-Command", "echo hello"], vec!["pwsh.exe", "-NoProfile", "-Command", "echo hello"],
), ),
( (
Shell::PowerShell(PowerShellConfig { PowerShellConfig {
exe: "powershell.exe".to_string(), exe: "powershell.exe".to_string(),
bash_exe_fallback: None, bash_exe_fallback: None,
}), },
vec!["bash", "-lc", "echo hello"], vec!["bash", "-lc", "echo hello"],
vec!["powershell.exe", "-NoProfile", "-Command", "echo hello"], vec!["powershell.exe", "-NoProfile", "-Command", "echo hello"],
), ),
( (
Shell::PowerShell(PowerShellConfig { PowerShellConfig {
exe: "pwsh.exe".to_string(), exe: "pwsh.exe".to_string(),
bash_exe_fallback: Some(PathBuf::from("bash.exe")), bash_exe_fallback: Some(PathBuf::from("bash.exe")),
}), },
vec!["bash", "-lc", "echo hello"], vec!["bash", "-lc", "echo hello"],
vec!["bash.exe", "-lc", "echo hello"], vec!["bash.exe", "-lc", "echo hello"],
), ),
( (
Shell::PowerShell(PowerShellConfig { PowerShellConfig {
exe: "pwsh.exe".to_string(), exe: "pwsh.exe".to_string(),
bash_exe_fallback: Some(PathBuf::from("bash.exe")), bash_exe_fallback: Some(PathBuf::from("bash.exe")),
}), },
vec![ vec![
"bash", "bash",
"-lc", "-lc",
@@ -527,27 +382,26 @@ mod tests_windows {
], ],
), ),
( (
Shell::PowerShell(PowerShellConfig { PowerShellConfig {
exe: "pwsh.exe".to_string(), exe: "pwsh.exe".to_string(),
bash_exe_fallback: Some(PathBuf::from("bash.exe")), bash_exe_fallback: Some(PathBuf::from("bash.exe")),
}), },
vec!["echo", "hello"], vec!["echo", "hello"],
vec!["pwsh.exe", "-NoProfile", "-Command", "echo hello"], vec!["pwsh.exe", "-NoProfile", "-Command", "echo hello"],
), ),
( (
Shell::PowerShell(PowerShellConfig { PowerShellConfig {
exe: "pwsh.exe".to_string(), exe: "pwsh.exe".to_string(),
bash_exe_fallback: Some(PathBuf::from("bash.exe")), bash_exe_fallback: Some(PathBuf::from("bash.exe")),
}), },
vec!["pwsh.exe", "-NoProfile", "-Command", "echo hello"], vec!["pwsh.exe", "-NoProfile", "-Command", "echo hello"],
vec!["pwsh.exe", "-NoProfile", "-Command", "echo hello"], vec!["pwsh.exe", "-NoProfile", "-Command", "echo hello"],
), ),
( (
// TODO (CODEX_2900): Handle escaping newlines for powershell invocation. PowerShellConfig {
Shell::PowerShell(PowerShellConfig {
exe: "powershell.exe".to_string(), exe: "powershell.exe".to_string(),
bash_exe_fallback: Some(PathBuf::from("bash.exe")), bash_exe_fallback: Some(PathBuf::from("bash.exe")),
}), },
vec![ vec![
"codex-mcp-server.exe", "codex-mcp-server.exe",
"--codex-run-as-apply-patch", "--codex-run-as-apply-patch",
@@ -561,13 +415,19 @@ mod tests_windows {
), ),
]; ];
for (shell, input, expected_cmd) in cases { for (config, input, expected_cmd) in cases {
let actual_cmd = shell let command = expected_cmd
.format_default_shell_invocation(input.iter().map(|s| (*s).to_string()).collect()); .iter()
assert_eq!( .map(|s| (*s).to_string())
actual_cmd, .collect::<Vec<_>>();
Some(expected_cmd.iter().map(|s| (*s).to_string()).collect())
); // These tests assert the final command for each scenario now that the helper
// has been removed. The inputs remain to document the original coverage.
let expected = expected_cmd
.iter()
.map(|s| (*s).to_string())
.collect::<Vec<_>>();
assert_eq!(command, expected, "input: {input:?} config: {config:?}");
} }
} }
} }