From 28ebe1c97a0c7cd01388890c66b6e77286c98518 Mon Sep 17 00:00:00 2001 From: Dylan Hurd Date: Mon, 17 Nov 2025 22:23:53 -0800 Subject: [PATCH] fix(windows) shell_command on windows, minor parsing (#6811) ## Summary Enables shell_command for windows users, and starts adding some basic command parsing here, to at least remove powershell prefixes. We'll follow this up with command parsing but I wanted to land this change separately with some basic UX. **NOTE**: This implementation parses bash and powershell on both platforms. In theory this is possible, since you can use git bash on windows or powershell on linux. In practice, this may not be worth the complexity of supporting, so I don't feel strongly about the current approach vs. platform-specific branching. ## Testing - [x] Added a bunch of tests - [x] Ran on both windows and os x --- codex-rs/core/src/lib.rs | 1 + codex-rs/core/src/model_family.rs | 2 + codex-rs/core/src/parse_command.rs | 49 +++++++++++++ codex-rs/core/src/powershell.rs | 93 ++++++++++++++++++++++++ codex-rs/core/src/shell.rs | 14 ++-- codex-rs/core/src/tools/spec.rs | 18 ++++- codex-rs/core/tests/suite/model_tools.rs | 14 +++- codex-rs/tui/src/exec_command.rs | 4 +- 8 files changed, 183 insertions(+), 12 deletions(-) create mode 100644 codex-rs/core/src/powershell.rs diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index e68478119..85618ffa2 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -13,6 +13,7 @@ mod client; mod client_common; pub mod codex; mod codex_conversation; +pub mod powershell; pub use codex_conversation::CodexConversation; mod codex_delegate; mod command_safety; diff --git a/codex-rs/core/src/model_family.rs b/codex-rs/core/src/model_family.rs index 4a10a6295..db37119d1 100644 --- a/codex-rs/core/src/model_family.rs +++ b/codex-rs/core/src/model_family.rs @@ -161,6 +161,7 @@ pub fn find_family_for_model(slug: &str) -> Option { "list_dir".to_string(), "read_file".to_string(), ], + shell_type: if cfg!(windows) { ConfigShellToolType::ShellCommand } else { ConfigShellToolType::Default }, supports_parallel_tool_calls: true, support_verbosity: true, ) @@ -176,6 +177,7 @@ pub fn find_family_for_model(slug: &str) -> Option { reasoning_summary_format: ReasoningSummaryFormat::Experimental, base_instructions: GPT_5_CODEX_INSTRUCTIONS.to_string(), apply_patch_tool_type: Some(ApplyPatchToolType::Freeform), + shell_type: if cfg!(windows) { ConfigShellToolType::ShellCommand } else { ConfigShellToolType::Default }, support_verbosity: false, ) } else if slug.starts_with("gpt-5.1") { diff --git a/codex-rs/core/src/parse_command.rs b/codex-rs/core/src/parse_command.rs index f9819ba83..f33534704 100644 --- a/codex-rs/core/src/parse_command.rs +++ b/codex-rs/core/src/parse_command.rs @@ -1,6 +1,7 @@ use crate::bash::extract_bash_command; use crate::bash::try_parse_shell; use crate::bash::try_parse_word_only_commands_sequence; +use crate::powershell::extract_powershell_command; use codex_protocol::parse_command::ParsedCommand; use shlex::split as shlex_split; use shlex::try_join as shlex_try_join; @@ -11,6 +12,11 @@ pub fn shlex_join(tokens: &[String]) -> String { .unwrap_or_else(|_| "".to_string()) } +/// Extracts the shell and script from a command, regardless of platform +pub fn extract_shell_command(command: &[String]) -> Option<(&str, &str)> { + extract_bash_command(command).or_else(|| extract_powershell_command(command)) +} + /// DO NOT REVIEW THIS CODE BY HAND /// This parsing code is quite complex and not easy to hand-modify. /// The easiest way to iterate is to add unit tests and have Codex fix the implementation. @@ -877,6 +883,42 @@ mod tests { }], ); } + + #[test] + fn powershell_command_is_stripped() { + assert_parsed( + &vec_str(&["powershell", "-Command", "Get-ChildItem"]), + vec![ParsedCommand::Unknown { + cmd: "Get-ChildItem".to_string(), + }], + ); + } + + #[test] + fn pwsh_with_noprofile_and_c_alias_is_stripped() { + assert_parsed( + &vec_str(&["pwsh", "-NoProfile", "-c", "Write-Host hi"]), + vec![ParsedCommand::Unknown { + cmd: "Write-Host hi".to_string(), + }], + ); + } + + #[test] + fn powershell_with_path_is_stripped() { + let command = if cfg!(windows) { + "C:\\windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe" + } else { + "/usr/local/bin/powershell.exe" + }; + + assert_parsed( + &vec_str(&[command, "-NoProfile", "-c", "Write-Host hi"]), + vec![ParsedCommand::Unknown { + cmd: "Write-Host hi".to_string(), + }], + ); + } } pub fn parse_command_impl(command: &[String]) -> Vec { @@ -884,6 +926,12 @@ pub fn parse_command_impl(command: &[String]) -> Vec { return commands; } + if let Some((_, script)) = extract_powershell_command(command) { + return vec![ParsedCommand::Unknown { + cmd: script.to_string(), + }]; + } + let normalized = normalize_tokens(command); let parts = if contains_connectors(&normalized) { @@ -1190,6 +1238,7 @@ fn parse_find_query_and_path(tail: &[String]) -> (Option, Option } fn parse_shell_lc_commands(original: &[String]) -> Option> { + // Only handle bash/zsh here; PowerShell is stripped separately without bash parsing. let (_, script) = extract_bash_command(original)?; if let Some(tree) = try_parse_shell(script) diff --git a/codex-rs/core/src/powershell.rs b/codex-rs/core/src/powershell.rs new file mode 100644 index 000000000..844866262 --- /dev/null +++ b/codex-rs/core/src/powershell.rs @@ -0,0 +1,93 @@ +use std::path::PathBuf; + +use crate::shell::ShellType; +use crate::shell::detect_shell_type; + +const POWERSHELL_FLAGS: &[&str] = &["-nologo", "-noprofile", "-command", "-c"]; + +/// Extract the PowerShell script body from an invocation such as: +/// +/// - ["pwsh", "-NoProfile", "-Command", "Get-ChildItem -Recurse | Select-String foo"] +/// - ["powershell.exe", "-Command", "Write-Host hi"] +/// - ["powershell", "-NoLogo", "-NoProfile", "-Command", "...script..."] +/// +/// Returns (`shell`, `script`) when the first arg is a PowerShell executable and a +/// `-Command` (or `-c`) flag is present followed by a script string. +pub fn extract_powershell_command(command: &[String]) -> Option<(&str, &str)> { + if command.len() < 3 { + return None; + } + + let shell = &command[0]; + if detect_shell_type(&PathBuf::from(shell)) != Some(ShellType::PowerShell) { + return None; + } + + // Find the first occurrence of -Command (accept common short alias -c as well) + let mut i = 1usize; + while i + 1 < command.len() { + let flag = &command[i]; + // Reject unknown flags + if !POWERSHELL_FLAGS.contains(&flag.to_ascii_lowercase().as_str()) { + return None; + } + if flag.eq_ignore_ascii_case("-Command") || flag.eq_ignore_ascii_case("-c") { + let script = &command[i + 1]; + return Some((shell, script.as_str())); + } + i += 1; + } + None +} + +#[cfg(test)] +mod tests { + use super::extract_powershell_command; + + #[test] + fn extracts_basic_powershell_command() { + let cmd = vec![ + "powershell".to_string(), + "-Command".to_string(), + "Write-Host hi".to_string(), + ]; + let (_shell, script) = extract_powershell_command(&cmd).expect("extract"); + assert_eq!(script, "Write-Host hi"); + } + + #[test] + fn extracts_lowercase_flags() { + let cmd = vec![ + "powershell".to_string(), + "-nologo".to_string(), + "-command".to_string(), + "Write-Host hi".to_string(), + ]; + let (_shell, script) = extract_powershell_command(&cmd).expect("extract"); + assert_eq!(script, "Write-Host hi"); + } + + #[test] + fn extracts_full_path_powershell_command() { + let command = if cfg!(windows) { + "C:\\windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe".to_string() + } else { + "/usr/local/bin/powershell.exe".to_string() + }; + let cmd = vec![command, "-Command".to_string(), "Write-Host hi".to_string()]; + let (_shell, script) = extract_powershell_command(&cmd).expect("extract"); + assert_eq!(script, "Write-Host hi"); + } + + #[test] + fn extracts_with_noprofile_and_alias() { + let cmd = vec![ + "pwsh".to_string(), + "-NoProfile".to_string(), + "-c".to_string(), + "Get-ChildItem | Select-String foo".to_string(), + ]; + let (_shell, script) = extract_powershell_command(&cmd).expect("extract"); + assert_eq!(script, "Get-ChildItem | Select-String foo"); + } +} diff --git a/codex-rs/core/src/shell.rs b/codex-rs/core/src/shell.rs index ed52491da..7bfec089c 100644 --- a/codex-rs/core/src/shell.rs +++ b/codex-rs/core/src/shell.rs @@ -61,10 +61,7 @@ impl Shell { ] } Shell::PowerShell(ps) => { - let mut args = vec![ - ps.shell_path.to_string_lossy().to_string(), - "-NoLogo".to_string(), - ]; + let mut args = vec![ps.shell_path.to_string_lossy().to_string()]; if !use_login_shell { args.push("-NoProfile".to_string()); } @@ -192,7 +189,6 @@ pub fn detect_shell_type(shell_path: &PathBuf) -> Option { Some("powershell") => Some(ShellType::PowerShell), _ => { let shell_name = shell_path.file_stem(); - if let Some(shell_name) = shell_name && shell_name != shell_path { @@ -251,6 +247,14 @@ mod detect_shell_type_tests { detect_shell_type(&PathBuf::from("powershell.exe")), Some(ShellType::PowerShell) ); + assert_eq!( + detect_shell_type(&PathBuf::from(if cfg!(windows) { + "C:\\windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe" + } else { + "/usr/local/bin/pwsh" + })), + Some(ShellType::PowerShell) + ); assert_eq!( detect_shell_type(&PathBuf::from("pwsh.exe")), Some(ShellType::PowerShell) diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 28e88c381..88ddbf551 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -1292,7 +1292,11 @@ mod tests { "gpt-5-codex", &Features::with_defaults(), &[ - "shell", + if cfg!(windows) { + "shell_command" + } else { + "shell" + }, "list_mcp_resources", "list_mcp_resource_templates", "read_mcp_resource", @@ -1309,7 +1313,11 @@ mod tests { "gpt-5.1-codex", &Features::with_defaults(), &[ - "shell", + if cfg!(windows) { + "shell_command" + } else { + "shell" + }, "list_mcp_resources", "list_mcp_resource_templates", "read_mcp_resource", @@ -1384,7 +1392,11 @@ mod tests { "gpt-5.1-codex-mini", &Features::with_defaults(), &[ - "shell", + if cfg!(windows) { + "shell_command" + } else { + "shell" + }, "list_mcp_resources", "list_mcp_resource_templates", "read_mcp_resource", diff --git a/codex-rs/core/tests/suite/model_tools.rs b/codex-rs/core/tests/suite/model_tools.rs index 42bc655f5..28593a55b 100644 --- a/codex-rs/core/tests/suite/model_tools.rs +++ b/codex-rs/core/tests/suite/model_tools.rs @@ -119,7 +119,12 @@ async fn model_selects_expected_tools() { assert_eq!( gpt5_codex_tools, vec![ - "shell".to_string(), + if cfg!(windows) { + "shell_command" + } else { + "shell" + } + .to_string(), "list_mcp_resources".to_string(), "list_mcp_resource_templates".to_string(), "read_mcp_resource".to_string(), @@ -133,7 +138,12 @@ async fn model_selects_expected_tools() { assert_eq!( gpt51_codex_tools, vec![ - "shell".to_string(), + if cfg!(windows) { + "shell_command" + } else { + "shell" + } + .to_string(), "list_mcp_resources".to_string(), "list_mcp_resource_templates".to_string(), "read_mcp_resource".to_string(), diff --git a/codex-rs/tui/src/exec_command.rs b/codex-rs/tui/src/exec_command.rs index 6f2212b09..8ce6c2632 100644 --- a/codex-rs/tui/src/exec_command.rs +++ b/codex-rs/tui/src/exec_command.rs @@ -1,7 +1,7 @@ use std::path::Path; use std::path::PathBuf; -use codex_core::bash::extract_bash_command; +use codex_core::parse_command::extract_shell_command; use dirs::home_dir; use shlex::try_join; @@ -10,7 +10,7 @@ pub(crate) fn escape_command(command: &[String]) -> String { } pub(crate) fn strip_bash_lc_and_escape(command: &[String]) -> String { - if let Some((_, script)) = extract_bash_command(command) { + if let Some((_, script)) = extract_shell_command(command) { return script.to_string(); } escape_command(command)