Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions codex-rs/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions codex-rs/core/src/model_family.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ pub fn find_family_for_model(slug: &str) -> Option<ModelFamily> {
"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,
)
Expand All @@ -176,6 +177,7 @@ pub fn find_family_for_model(slug: &str) -> Option<ModelFamily> {
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") {
Expand Down
49 changes: 49 additions & 0 deletions codex-rs/core/src/parse_command.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -11,6 +12,11 @@ pub fn shlex_join(tokens: &[String]) -> String {
.unwrap_or_else(|_| "<command included NUL byte>".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))
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As noted in the PR description, I don't feel strongly about prioritizing cross-platform shell support in this PR. If we want to keep things simple, we can branch here.

}

/// 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.
Expand Down Expand Up @@ -877,13 +883,55 @@ 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<ParsedCommand> {
if let Some(commands) = parse_shell_lc_commands(command) {
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) {
Expand Down Expand Up @@ -1190,6 +1238,7 @@ fn parse_find_query_and_path(tail: &[String]) -> (Option<String>, Option<String>
}

fn parse_shell_lc_commands(original: &[String]) -> Option<Vec<ParsedCommand>> {
// 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)
Expand Down
93 changes: 93 additions & 0 deletions codex-rs/core/src/powershell.rs
Original file line number Diff line number Diff line change
@@ -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");
}
}
14 changes: 9 additions & 5 deletions codex-rs/core/src/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down Expand Up @@ -192,7 +189,6 @@ pub fn detect_shell_type(shell_path: &PathBuf) -> Option<ShellType> {
Some("powershell") => Some(ShellType::PowerShell),
_ => {
let shell_name = shell_path.file_stem();

if let Some(shell_name) = shell_name
&& shell_name != shell_path
{
Expand Down Expand Up @@ -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)
Expand Down
18 changes: 15 additions & 3 deletions codex-rs/core/src/tools/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
14 changes: 12 additions & 2 deletions codex-rs/core/tests/suite/model_tools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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(),
Expand Down
4 changes: 2 additions & 2 deletions codex-rs/tui/src/exec_command.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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)
Expand Down
Loading