Bridge command generation to powershell when on Windows (#2319)

## What? Why? How?
- When running on Windows, codex often tries to invoke bash commands,
which commonly fail (unless WSL is installed)
- Fix: Detect if powershell is available and, if so, route commands to
it
- Also add a shell_name property to environmental context for codex to
default to powershell commands when running in that environment

## Testing
- Tested within WSL and powershell (e.g. get top 5 largest files within
a folder and validated that commands generated were powershell commands)
- Tested within Zsh
- Updated unit tests

---------

Co-authored-by: Eddy Escardo <eddy@openai.com>
This commit is contained in:
eddy-win
2025-08-20 16:30:34 -07:00
committed by GitHub
parent 5ab30c73f3
commit 050b9baeb6
6 changed files with 259 additions and 15 deletions

19
codex-rs/Cargo.lock generated
View File

@@ -737,6 +737,7 @@ dependencies = [
"tree-sitter-bash", "tree-sitter-bash",
"uuid", "uuid",
"walkdir", "walkdir",
"which",
"whoami", "whoami",
"wildmatch", "wildmatch",
"wiremock", "wiremock",
@@ -5599,6 +5600,18 @@ version = "0.1.10"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a751b3277700db47d3e574514de2eced5e54dc8a5436a3bf7a0b248b2cee16f3" checksum = "a751b3277700db47d3e574514de2eced5e54dc8a5436a3bf7a0b248b2cee16f3"
[[package]]
name = "which"
version = "6.0.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b4ee928febd44d98f2f459a4a79bd4d928591333a494a10a868418ac1b39cf1f"
dependencies = [
"either",
"home",
"rustix 0.38.44",
"winsafe",
]
[[package]] [[package]]
name = "whoami" name = "whoami"
version = "1.6.0" version = "1.6.0"
@@ -6011,6 +6024,12 @@ dependencies = [
"memchr", "memchr",
] ]
[[package]]
name = "winsafe"
version = "0.0.19"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d135d17ab770252ad95e9a872d365cf3090e3be864a34ab46f48555993efc904"
[[package]] [[package]]
name = "wiremock" name = "wiremock"
version = "0.6.4" version = "0.6.4"

View File

@@ -71,6 +71,9 @@ openssl-sys = { version = "*", features = ["vendored"] }
[target.aarch64-unknown-linux-musl.dependencies] [target.aarch64-unknown-linux-musl.dependencies]
openssl-sys = { version = "*", features = ["vendored"] } openssl-sys = { version = "*", features = ["vendored"] }
[target.'cfg(target_os = "windows")'.dependencies]
which = "6"
[dev-dependencies] [dev-dependencies]
assert_cmd = "2" assert_cmd = "2"
core_test_support = { path = "tests/common" } core_test_support = { path = "tests/common" }

View File

@@ -511,6 +511,7 @@ impl Session {
turn_context.cwd.to_path_buf(), turn_context.cwd.to_path_buf(),
turn_context.approval_policy, turn_context.approval_policy,
turn_context.sandbox_policy.clone(), turn_context.sandbox_policy.clone(),
sess.user_shell.clone(),
))); )));
sess.record_conversation_items(&conversation_items).await; sess.record_conversation_items(&conversation_items).await;
@@ -1070,6 +1071,7 @@ async fn submission_loop(
new_cwd, new_cwd,
new_approval_policy, new_approval_policy,
new_sandbox_policy, new_sandbox_policy,
sess.user_shell.clone(),
))]) ))])
.await; .await;
} }
@@ -2051,18 +2053,20 @@ pub struct ExecInvokeArgs<'a> {
pub stdout_stream: Option<StdoutStream>, pub stdout_stream: Option<StdoutStream>,
} }
fn maybe_run_with_user_profile( fn maybe_translate_shell_command(
params: ExecParams, params: ExecParams,
sess: &Session, sess: &Session,
turn_context: &TurnContext, turn_context: &TurnContext,
) -> ExecParams { ) -> ExecParams {
if turn_context.shell_environment_policy.use_profile { let should_translate = matches!(sess.user_shell, crate::shell::Shell::PowerShell(_))
let command = sess || turn_context.shell_environment_policy.use_profile;
if should_translate
&& let Some(command) = sess
.user_shell .user_shell
.format_default_shell_invocation(params.command.clone()); .format_default_shell_invocation(params.command.clone())
if let Some(command) = command { {
return ExecParams { command, ..params }; return ExecParams { command, ..params };
}
} }
params params
} }
@@ -2227,7 +2231,7 @@ async fn handle_container_exec_with_params(
), ),
}; };
let params = maybe_run_with_user_profile(params, sess, turn_context); let params = maybe_translate_shell_command(params, sess, turn_context);
let output_result = sess let output_result = sess
.run_exec_with_events( .run_exec_with_events(
turn_diff_tracker, turn_diff_tracker,

View File

@@ -6,6 +6,7 @@ use crate::models::ContentItem;
use crate::models::ResponseItem; use crate::models::ResponseItem;
use crate::protocol::AskForApproval; use crate::protocol::AskForApproval;
use crate::protocol::SandboxPolicy; use crate::protocol::SandboxPolicy;
use crate::shell::Shell;
use codex_protocol::config_types::SandboxMode; use codex_protocol::config_types::SandboxMode;
use std::fmt::Display; use std::fmt::Display;
use std::path::PathBuf; use std::path::PathBuf;
@@ -28,6 +29,7 @@ pub(crate) struct EnvironmentContext {
pub approval_policy: AskForApproval, pub approval_policy: AskForApproval,
pub sandbox_mode: SandboxMode, pub sandbox_mode: SandboxMode,
pub network_access: NetworkAccess, pub network_access: NetworkAccess,
pub shell: Shell,
} }
impl EnvironmentContext { impl EnvironmentContext {
@@ -35,6 +37,7 @@ impl EnvironmentContext {
cwd: PathBuf, cwd: PathBuf,
approval_policy: AskForApproval, approval_policy: AskForApproval,
sandbox_policy: SandboxPolicy, sandbox_policy: SandboxPolicy,
shell: Shell,
) -> Self { ) -> Self {
Self { Self {
cwd, cwd,
@@ -55,6 +58,7 @@ impl EnvironmentContext {
} }
} }
}, },
shell,
} }
} }
} }
@@ -69,6 +73,9 @@ impl Display for EnvironmentContext {
writeln!(f, "Approval policy: {}", self.approval_policy)?; writeln!(f, "Approval policy: {}", self.approval_policy)?;
writeln!(f, "Sandbox mode: {}", self.sandbox_mode)?; writeln!(f, "Sandbox mode: {}", self.sandbox_mode)?;
writeln!(f, "Network access: {}", self.network_access)?; writeln!(f, "Network access: {}", self.network_access)?;
if let Some(shell_name) = self.shell.name() {
writeln!(f, "Shell: {shell_name}")?;
}
Ok(()) Ok(())
} }
} }

View File

@@ -1,14 +1,24 @@
use serde::Deserialize;
use serde::Serialize;
use shlex; use shlex;
use std::path::PathBuf;
#[derive(Debug, PartialEq, Eq)] #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
pub struct ZshShell { pub struct ZshShell {
shell_path: String, shell_path: String,
zshrc_path: String, zshrc_path: String,
} }
#[derive(Debug, PartialEq, Eq)] #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
pub struct PowerShellConfig {
exe: String, // Executable name or path, e.g. "pwsh" or "powershell.exe".
bash_exe_fallback: Option<PathBuf>, // In case the model generates a bash command.
}
#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
pub enum Shell { pub enum Shell {
Zsh(ZshShell), Zsh(ZshShell),
PowerShell(PowerShellConfig),
Unknown, Unknown,
} }
@@ -33,6 +43,61 @@ impl Shell {
} }
Some(result) Some(result)
} }
Shell::PowerShell(ps) => {
// If model generated a bash command, prefer a detected bash fallback
if let Some(script) = strip_bash_lc(&command) {
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(|s| s.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> {
match self {
Shell::Zsh(zsh) => std::path::Path::new(&zsh.shell_path)
.file_name()
.map(|s| s.to_string_lossy().to_string()),
Shell::PowerShell(ps) => Some(ps.exe.clone()),
Shell::Unknown => None, Shell::Unknown => None,
} }
} }
@@ -86,11 +151,51 @@ pub async fn default_user_shell() -> Shell {
} }
} }
#[cfg(not(target_os = "macos"))] #[cfg(all(not(target_os = "macos"), not(target_os = "windows")))]
pub async fn default_user_shell() -> Shell { pub async fn default_user_shell() -> Shell {
Shell::Unknown Shell::Unknown
} }
#[cfg(target_os = "windows")]
pub async fn default_user_shell() -> Shell {
use tokio::process::Command;
// Prefer PowerShell 7+ (`pwsh`) if available, otherwise fall back to Windows PowerShell.
let has_pwsh = Command::new("pwsh")
.arg("-NoLogo")
.arg("-NoProfile")
.arg("-Command")
.arg("$PSVersionTable.PSVersion.Major")
.output()
.await
.map(|o| o.status.success())
.unwrap_or(false);
let bash_exe = if Command::new("bash.exe")
.arg("--version")
.output()
.await
.ok()
.map(|o| o.status.success())
.unwrap_or(false)
{
which::which("bash.exe").ok()
} else {
None
};
if has_pwsh {
Shell::PowerShell(PowerShellConfig {
exe: "pwsh.exe".to_string(),
bash_exe_fallback: bash_exe,
})
} else {
Shell::PowerShell(PowerShellConfig {
exe: "powershell.exe".to_string(),
bash_exe_fallback: bash_exe,
})
}
}
#[cfg(test)] #[cfg(test)]
#[cfg(target_os = "macos")] #[cfg(target_os = "macos")]
mod tests { mod tests {
@@ -231,3 +336,97 @@ mod tests {
} }
} }
} }
#[cfg(test)]
#[cfg(target_os = "windows")]
mod tests_windows {
use super::*;
#[test]
fn test_format_default_shell_invocation_powershell() {
let cases = vec![
(
Shell::PowerShell(PowerShellConfig {
exe: "pwsh.exe".to_string(),
bash_exe_fallback: None,
}),
vec!["bash", "-lc", "echo hello"],
vec!["pwsh.exe", "-NoProfile", "-Command", "echo hello"],
),
(
Shell::PowerShell(PowerShellConfig {
exe: "powershell.exe".to_string(),
bash_exe_fallback: None,
}),
vec!["bash", "-lc", "echo hello"],
vec!["powershell.exe", "-NoProfile", "-Command", "echo hello"],
),
(
Shell::PowerShell(PowerShellConfig {
exe: "pwsh.exe".to_string(),
bash_exe_fallback: Some(PathBuf::from("bash.exe")),
}),
vec!["bash", "-lc", "echo hello"],
vec!["bash.exe", "-lc", "echo hello"],
),
(
Shell::PowerShell(PowerShellConfig {
exe: "pwsh.exe".to_string(),
bash_exe_fallback: Some(PathBuf::from("bash.exe")),
}),
vec![
"bash",
"-lc",
"apply_patch <<'EOF'\n*** Begin Patch\n*** Update File: destination_file.txt\n-original content\n+modified content\n*** End Patch\nEOF",
],
vec![
"bash.exe",
"-lc",
"apply_patch <<'EOF'\n*** Begin Patch\n*** Update File: destination_file.txt\n-original content\n+modified content\n*** End Patch\nEOF",
],
),
(
Shell::PowerShell(PowerShellConfig {
exe: "pwsh.exe".to_string(),
bash_exe_fallback: Some(PathBuf::from("bash.exe")),
}),
vec!["echo", "hello"],
vec!["pwsh.exe", "-NoProfile", "-Command", "echo hello"],
),
(
Shell::PowerShell(PowerShellConfig {
exe: "pwsh.exe".to_string(),
bash_exe_fallback: Some(PathBuf::from("bash.exe")),
}),
vec!["pwsh.exe", "-NoProfile", "-Command", "echo hello"],
vec!["pwsh.exe", "-NoProfile", "-Command", "echo hello"],
),
(
// TODO (CODEX_2900): Handle escaping newlines for powershell invocation.
Shell::PowerShell(PowerShellConfig {
exe: "powershell.exe".to_string(),
bash_exe_fallback: Some(PathBuf::from("bash.exe")),
}),
vec![
"codex-mcp-server.exe",
"--codex-run-as-apply-patch",
"*** Begin Patch\n*** Update File: C:\\Users\\person\\destination_file.txt\n-original content\n+modified content\n*** End Patch",
],
vec![
"codex-mcp-server.exe",
"--codex-run-as-apply-patch",
"*** Begin Patch\n*** Update File: C:\\Users\\person\\destination_file.txt\n-original content\n+modified content\n*** End Patch",
],
),
];
for (shell, input, expected_cmd) in cases {
let actual_cmd = shell
.format_default_shell_invocation(input.iter().map(|s| s.to_string()).collect());
assert_eq!(
actual_cmd,
Some(expected_cmd.iter().map(|s| s.to_string()).collect())
);
}
}
}

View File

@@ -8,6 +8,7 @@ use codex_core::protocol::Op;
use codex_core::protocol::SandboxPolicy; use codex_core::protocol::SandboxPolicy;
use codex_core::protocol_config_types::ReasoningEffort; use codex_core::protocol_config_types::ReasoningEffort;
use codex_core::protocol_config_types::ReasoningSummary; use codex_core::protocol_config_types::ReasoningSummary;
use codex_core::shell::default_user_shell;
use codex_login::CodexAuth; use codex_login::CodexAuth;
use core_test_support::load_default_config_for_test; use core_test_support::load_default_config_for_test;
use core_test_support::load_sse_fixture_with_id; use core_test_support::load_sse_fixture_with_id;
@@ -85,9 +86,15 @@ async fn prefixes_context_and_instructions_once_and_consistently_across_requests
let requests = server.received_requests().await.unwrap(); let requests = server.received_requests().await.unwrap();
assert_eq!(requests.len(), 2, "expected two POST requests"); assert_eq!(requests.len(), 2, "expected two POST requests");
let shell = default_user_shell().await;
let expected_env_text = format!( let expected_env_text = format!(
"<environment_context>\nCurrent working directory: {}\nApproval policy: on-request\nSandbox mode: read-only\nNetwork access: restricted\n</environment_context>", "<environment_context>\nCurrent working directory: {}\nApproval policy: on-request\nSandbox mode: read-only\nNetwork access: restricted\n{}</environment_context>",
cwd.path().to_string_lossy() cwd.path().to_string_lossy(),
match shell.name() {
Some(name) => format!("Shell: {name}\n"),
None => String::new(),
}
); );
let expected_ui_text = let expected_ui_text =
"<user_instructions>\n\nbe consistent and helpful\n\n</user_instructions>"; "<user_instructions>\n\nbe consistent and helpful\n\n</user_instructions>";
@@ -237,9 +244,14 @@ async fn overrides_turn_context_but_keeps_cached_prefix_and_key_constant() {
}); });
// After overriding the turn context, the environment context should be emitted again // After overriding the turn context, the environment context should be emitted again
// reflecting the new cwd, approval policy and sandbox settings. // reflecting the new cwd, approval policy and sandbox settings.
let shell = default_user_shell().await;
let expected_env_text_2 = format!( let expected_env_text_2 = format!(
"<environment_context>\nCurrent working directory: {}\nApproval policy: never\nSandbox mode: workspace-write\nNetwork access: enabled\n</environment_context>", "<environment_context>\nCurrent working directory: {}\nApproval policy: never\nSandbox mode: workspace-write\nNetwork access: enabled\n{}</environment_context>",
new_cwd.path().to_string_lossy() new_cwd.path().to_string_lossy(),
match shell.name() {
Some(name) => format!("Shell: {name}\n"),
None => String::new(),
}
); );
let expected_env_msg_2 = serde_json::json!({ let expected_env_msg_2 = serde_json::json!({
"type": "message", "type": "message",