mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
fix: add shell fallback paths for pwsh/powershell that work on GitHub Actions Windows runners (#16617)
Recently, I merged a number of PRs to increase startup timeouts for scripts that ran under PowerShell, but in the failure for `suite::codex_tool::test_shell_command_approval_triggers_elicitation`, I found this in the error logs when running on Bazel with BuildBuddy: ``` [mcp stderr] 2026-04-02T19:54:10.758951Z ERROR codex_core::tools::router: error=Exit code: 1 [mcp stderr] Wall time: 0.2 seconds [mcp stderr] Output: [mcp stderr] 'New-Item' is not recognized as an internal or external command, [mcp stderr] operable program or batch file. [mcp stderr] ``` This error implies that the command was run under `cmd.exe` instead of `pwsh.exe`. Under GitHub Actions, I suspect that the `%PATH%` that is passed to our Bazel builder is scrubbed such that our tests cannot find PowerShell where GitHub installs it. Having these explicit fallback paths should help. While we could enable these only for tests, I don't see any harm in keeping them in production, as well.
This commit is contained in:
committed by
GitHub
Unverified
parent
57b98bc4cd
commit
93380a6fac
+36
-12
@@ -166,7 +166,7 @@ fn get_shell_path(
|
||||
shell_type: ShellType,
|
||||
provided_path: Option<&PathBuf>,
|
||||
binary_name: &str,
|
||||
fallback_paths: Vec<&str>,
|
||||
fallback_paths: &[&str],
|
||||
) -> Option<PathBuf> {
|
||||
// If exact provided path exists, use it
|
||||
if provided_path.and_then(file_exists).is_some() {
|
||||
@@ -197,8 +197,10 @@ fn get_shell_path(
|
||||
None
|
||||
}
|
||||
|
||||
const ZSH_FALLBACK_PATHS: &[&str] = &["/bin/zsh"];
|
||||
|
||||
fn get_zsh_shell(path: Option<&PathBuf>) -> Option<Shell> {
|
||||
let shell_path = get_shell_path(ShellType::Zsh, path, "zsh", vec!["/bin/zsh"]);
|
||||
let shell_path = get_shell_path(ShellType::Zsh, path, "zsh", ZSH_FALLBACK_PATHS);
|
||||
|
||||
shell_path.map(|shell_path| Shell {
|
||||
shell_type: ShellType::Zsh,
|
||||
@@ -207,8 +209,10 @@ fn get_zsh_shell(path: Option<&PathBuf>) -> Option<Shell> {
|
||||
})
|
||||
}
|
||||
|
||||
const BASH_FALLBACK_PATHS: &[&str] = &["/bin/bash"];
|
||||
|
||||
fn get_bash_shell(path: Option<&PathBuf>) -> Option<Shell> {
|
||||
let shell_path = get_shell_path(ShellType::Bash, path, "bash", vec!["/bin/bash"]);
|
||||
let shell_path = get_shell_path(ShellType::Bash, path, "bash", BASH_FALLBACK_PATHS);
|
||||
|
||||
shell_path.map(|shell_path| Shell {
|
||||
shell_type: ShellType::Bash,
|
||||
@@ -217,8 +221,10 @@ fn get_bash_shell(path: Option<&PathBuf>) -> Option<Shell> {
|
||||
})
|
||||
}
|
||||
|
||||
const SH_FALLBACK_PATHS: &[&str] = &["/bin/sh"];
|
||||
|
||||
fn get_sh_shell(path: Option<&PathBuf>) -> Option<Shell> {
|
||||
let shell_path = get_shell_path(ShellType::Sh, path, "sh", vec!["/bin/sh"]);
|
||||
let shell_path = get_shell_path(ShellType::Sh, path, "sh", SH_FALLBACK_PATHS);
|
||||
|
||||
shell_path.map(|shell_path| Shell {
|
||||
shell_type: ShellType::Sh,
|
||||
@@ -227,14 +233,32 @@ fn get_sh_shell(path: Option<&PathBuf>) -> Option<Shell> {
|
||||
})
|
||||
}
|
||||
|
||||
// Note the `pwsh` and `powershell` fallback paths are where the respective
|
||||
// shells are commonly installed on GitHub Actions Windows runners, but may not
|
||||
// be present on all Windows machines:
|
||||
// https://docs.github.com/en/actions/tutorials/build-and-test-code/powershell
|
||||
|
||||
#[cfg(windows)]
|
||||
const PWSH_FALLBACK_PATHS: &[&str] = &[r#"C:\Program Files\PowerShell\7\pwsh.exe"#];
|
||||
#[cfg(not(windows))]
|
||||
const PWSH_FALLBACK_PATHS: &[&str] = &["/usr/local/bin/pwsh"];
|
||||
|
||||
#[cfg(windows)]
|
||||
const POWERSHELL_FALLBACK_PATHS: &[&str] =
|
||||
&[r#"C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe"#];
|
||||
#[cfg(not(windows))]
|
||||
const POWERSHELL_FALLBACK_PATHS: &[&str] = &[];
|
||||
|
||||
fn get_powershell_shell(path: Option<&PathBuf>) -> Option<Shell> {
|
||||
let shell_path = get_shell_path(
|
||||
ShellType::PowerShell,
|
||||
path,
|
||||
"pwsh",
|
||||
vec!["/usr/local/bin/pwsh"],
|
||||
)
|
||||
.or_else(|| get_shell_path(ShellType::PowerShell, path, "powershell", vec![]));
|
||||
let shell_path = get_shell_path(ShellType::PowerShell, path, "pwsh", PWSH_FALLBACK_PATHS)
|
||||
.or_else(|| {
|
||||
get_shell_path(
|
||||
ShellType::PowerShell,
|
||||
path,
|
||||
"powershell",
|
||||
POWERSHELL_FALLBACK_PATHS,
|
||||
)
|
||||
});
|
||||
|
||||
shell_path.map(|shell_path| Shell {
|
||||
shell_type: ShellType::PowerShell,
|
||||
@@ -244,7 +268,7 @@ fn get_powershell_shell(path: Option<&PathBuf>) -> Option<Shell> {
|
||||
}
|
||||
|
||||
fn get_cmd_shell(path: Option<&PathBuf>) -> Option<Shell> {
|
||||
let shell_path = get_shell_path(ShellType::Cmd, path, "cmd", vec![]);
|
||||
let shell_path = get_shell_path(ShellType::Cmd, path, "cmd", &[]);
|
||||
|
||||
shell_path.map(|shell_path| Shell {
|
||||
shell_type: ShellType::Cmd,
|
||||
|
||||
Reference in New Issue
Block a user