Skip to content

Commit 072c7a6

Browse files
switch back into derive_exec_args
1 parent b025f8f commit 072c7a6

File tree

7 files changed

+84
-56
lines changed

7 files changed

+84
-56
lines changed

codex-rs/core/src/powershell.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,10 @@ use crate::shell::ShellType;
77
use crate::shell::detect_shell_type;
88

99
const POWERSHELL_FLAGS: &[&str] = &["-nologo", "-noprofile", "-command", "-c"];
10-
/// Prelude injected into PowerShell scripts to force UTF-8 stdout encoding.
11-
///
12-
/// This must stay in sync with similar constants in other crates (e.g. codex-apply-patch),
13-
/// since some parsers strip this prefix before analyzing scripts.
10+
11+
/// Prefixed command for powershell shell calls to force UTF-8 console output.
1412
pub(crate) const UTF8_OUTPUT_PREFIX: &str =
15-
"[Console]::OutputEncoding=[System.Text.Encoding]::UTF8;";
13+
"[Console]::OutputEncoding=[System.Text.Encoding]::UTF8;\n";
1614

1715
pub(crate) fn prefix_utf8_output(script: &str) -> String {
1816
let trimmed = script.trim_start();

codex-rs/core/src/shell.rs

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use serde::Serialize;
33
use std::path::PathBuf;
44
use std::sync::Arc;
55

6+
use crate::powershell::prefix_utf8_output;
67
use crate::shell_snapshot::ShellSnapshot;
78

89
#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
@@ -35,7 +36,12 @@ impl Shell {
3536

3637
/// Takes a string of shell and returns the full list of command args to
3738
/// use with `exec()` to run the shell command.
38-
pub fn derive_exec_args(&self, command: &str, use_login_shell: bool) -> Vec<String> {
39+
pub fn derive_exec_args(
40+
&self,
41+
command: &str,
42+
use_login_shell: bool,
43+
powershell_utf8_enabled: bool,
44+
) -> Vec<String> {
3945
match self.shell_type {
4046
ShellType::Zsh | ShellType::Bash | ShellType::Sh => {
4147
let arg = if use_login_shell { "-lc" } else { "-c" };
@@ -52,7 +58,11 @@ impl Shell {
5258
}
5359

5460
args.push("-Command".to_string());
55-
args.push(command.to_string());
61+
if powershell_utf8_enabled {
62+
args.push(prefix_utf8_output(command));
63+
} else {
64+
args.push(command.to_string());
65+
}
5666
args
5767
}
5868
ShellType::Cmd => {
@@ -408,7 +418,7 @@ mod tests {
408418

409419
fn shell_works(shell: Option<Shell>, command: &str, required: bool) -> bool {
410420
if let Some(shell) = shell {
411-
let args = shell.derive_exec_args(command, false);
421+
let args = shell.derive_exec_args(command, false, false);
412422
let output = Command::new(args[0].clone())
413423
.args(&args[1..])
414424
.output()
@@ -429,11 +439,11 @@ mod tests {
429439
shell_snapshot: None,
430440
};
431441
assert_eq!(
432-
test_bash_shell.derive_exec_args("echo hello", false),
442+
test_bash_shell.derive_exec_args("echo hello", false, false),
433443
vec!["/bin/bash", "-c", "echo hello"]
434444
);
435445
assert_eq!(
436-
test_bash_shell.derive_exec_args("echo hello", true),
446+
test_bash_shell.derive_exec_args("echo hello", true, false),
437447
vec!["/bin/bash", "-lc", "echo hello"]
438448
);
439449

@@ -443,11 +453,11 @@ mod tests {
443453
shell_snapshot: None,
444454
};
445455
assert_eq!(
446-
test_zsh_shell.derive_exec_args("echo hello", false),
456+
test_zsh_shell.derive_exec_args("echo hello", false, false),
447457
vec!["/bin/zsh", "-c", "echo hello"]
448458
);
449459
assert_eq!(
450-
test_zsh_shell.derive_exec_args("echo hello", true),
460+
test_zsh_shell.derive_exec_args("echo hello", true, false),
451461
vec!["/bin/zsh", "-lc", "echo hello"]
452462
);
453463

@@ -457,20 +467,37 @@ mod tests {
457467
shell_snapshot: None,
458468
};
459469
assert_eq!(
460-
test_powershell_shell.derive_exec_args("echo hello", false),
470+
test_powershell_shell.derive_exec_args("echo hello", false, false),
471+
vec![
472+
"pwsh.exe".to_string(),
473+
"-NoProfile".to_string(),
474+
"-Command".to_string(),
475+
"echo hello".to_string(),
476+
]
477+
);
478+
assert_eq!(
479+
test_powershell_shell.derive_exec_args("echo hello", true, false),
480+
vec![
481+
"pwsh.exe".to_string(),
482+
"-Command".to_string(),
483+
"echo hello".to_string(),
484+
]
485+
);
486+
assert_eq!(
487+
test_powershell_shell.derive_exec_args("echo hello", false, true),
461488
vec![
462489
"pwsh.exe".to_string(),
463490
"-NoProfile".to_string(),
464491
"-Command".to_string(),
465-
format!("echo hello"),
492+
"[Console]::OutputEncoding=[System.Text.Encoding]::UTF8;\necho hello".to_string(),
466493
]
467494
);
468495
assert_eq!(
469-
test_powershell_shell.derive_exec_args("echo hello", true),
496+
test_powershell_shell.derive_exec_args("echo hello", true, true),
470497
vec![
471498
"pwsh.exe".to_string(),
472499
"-Command".to_string(),
473-
format!("echo hello"),
500+
"[Console]::OutputEncoding=[System.Text.Encoding]::UTF8;\necho hello".to_string(),
474501
]
475502
);
476503
}

codex-rs/core/src/shell_snapshot.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ async fn run_shell_script_with_timeout(
112112
script: &str,
113113
snapshot_timeout: Duration,
114114
) -> Result<String> {
115-
let args = shell.derive_exec_args(script, true);
115+
let args = shell.derive_exec_args(script, true, false);
116116
let shell_name = shell.name();
117117

118118
// Handler is kept as guard to control the drop. The `mut` pattern is required because .args()

codex-rs/core/src/tasks/user_shell.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use crate::exec::StdoutStream;
1616
use crate::exec::StreamOutput;
1717
use crate::exec::execute_exec_env;
1818
use crate::exec_env::create_env;
19+
use crate::features::Feature;
1920
use crate::parse_command::parse_command;
2021
use crate::protocol::EventMsg;
2122
use crate::protocol::ExecCommandBeginEvent;
@@ -68,9 +69,12 @@ impl SessionTask for UserShellCommandTask {
6869
// allows commands that use shell features (pipes, &&, redirects, etc.).
6970
// We do not source rc files or otherwise reformat the script.
7071
let use_login_shell = true;
71-
let command = session
72-
.user_shell()
73-
.derive_exec_args(&self.command, use_login_shell);
72+
let powershell_utf8_enabled = session.features().enabled(Feature::PowershellUtf8);
73+
let command = session.user_shell().derive_exec_args(
74+
&self.command,
75+
use_login_shell,
76+
powershell_utf8_enabled,
77+
);
7478

7579
let call_id = Uuid::new_v4().to_string();
7680
let raw_command = self.command.clone();

codex-rs/core/src/tools/handlers/shell.rs

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,8 @@ use crate::exec_policy::create_exec_approval_requirement_for_command;
1010
use crate::features::Feature;
1111
use crate::function_tool::FunctionCallError;
1212
use crate::is_safe_command::is_known_safe_command;
13-
use crate::powershell::prefix_utf8_output;
1413
use crate::protocol::ExecCommandSource;
1514
use crate::shell::Shell;
16-
use crate::shell::ShellType;
1715
use crate::tools::context::ToolInvocation;
1816
use crate::tools::context::ToolOutput;
1917
use crate::tools::context::ToolPayload;
@@ -46,9 +44,14 @@ impl ShellHandler {
4644
}
4745

4846
impl ShellCommandHandler {
49-
fn base_command(shell: &Shell, command: &str, login: Option<bool>) -> Vec<String> {
47+
fn base_command(
48+
shell: &Shell,
49+
command: &str,
50+
login: Option<bool>,
51+
powershell_utf8_enabled: bool,
52+
) -> Vec<String> {
5053
let use_login_shell = login.unwrap_or(true);
51-
shell.derive_exec_args(command, use_login_shell)
54+
shell.derive_exec_args(command, use_login_shell, powershell_utf8_enabled)
5255
}
5356

5457
fn to_exec_params(
@@ -57,14 +60,12 @@ impl ShellCommandHandler {
5760
turn_context: &TurnContext,
5861
) -> ExecParams {
5962
let shell = session.user_shell();
60-
let command_str = if matches!(shell.shell_type, ShellType::PowerShell)
61-
&& session.features().enabled(Feature::PowershellUtf8)
62-
{
63-
prefix_utf8_output(&params.command)
64-
} else {
65-
params.command.to_string()
66-
};
67-
let command = Self::base_command(shell.as_ref(), &command_str, params.login);
63+
let command = Self::base_command(
64+
shell.as_ref(),
65+
&params.command,
66+
params.login,
67+
session.features().enabled(Feature::PowershellUtf8),
68+
);
6869

6970
ExecParams {
7071
command,
@@ -171,7 +172,8 @@ impl ToolHandler for ShellCommandHandler {
171172
serde_json::from_str::<ShellCommandToolCallParams>(arguments)
172173
.map(|params| {
173174
let shell = invocation.session.user_shell();
174-
let command = Self::base_command(shell.as_ref(), &params.command, params.login);
175+
let command =
176+
Self::base_command(shell.as_ref(), &params.command, params.login, false);
175177
!is_known_safe_command(&command)
176178
})
177179
.unwrap_or(true)
@@ -360,12 +362,18 @@ mod tests {
360362
}
361363

362364
fn assert_safe(shell: &Shell, command: &str) {
363-
assert!(is_known_safe_command(
364-
&shell.derive_exec_args(command, /* use_login_shell */ true)
365-
));
366-
assert!(is_known_safe_command(
367-
&shell.derive_exec_args(command, /* use_login_shell */ false)
368-
));
365+
assert!(is_known_safe_command(&shell.derive_exec_args(
366+
command, /* use_login_shell */ true, false
367+
)));
368+
assert!(is_known_safe_command(&shell.derive_exec_args(
369+
command, /* use_login_shell */ false, false
370+
)));
371+
assert!(is_known_safe_command(&shell.derive_exec_args(
372+
command, /* use_login_shell */ true, true
373+
)));
374+
assert!(is_known_safe_command(&shell.derive_exec_args(
375+
command, /* use_login_shell */ false, true
376+
)));
369377
}
370378

371379
#[tokio::test]
@@ -379,7 +387,7 @@ mod tests {
379387
let sandbox_permissions = SandboxPermissions::RequireEscalated;
380388
let justification = Some("because tests".to_string());
381389

382-
let expected_command = session.user_shell().derive_exec_args(&command, true);
390+
let expected_command = session.user_shell().derive_exec_args(&command, true, false);
383391
let expected_cwd = turn_context.resolve_path(workdir.clone());
384392
let expected_env = create_env(&turn_context.shell_environment_policy);
385393

@@ -415,17 +423,17 @@ mod tests {
415423
};
416424

417425
let login_command =
418-
ShellCommandHandler::base_command(&shell, "echo login shell", Some(true));
426+
ShellCommandHandler::base_command(&shell, "echo login shell", Some(true), false);
419427
assert_eq!(
420428
login_command,
421-
shell.derive_exec_args("echo login shell", true)
429+
shell.derive_exec_args("echo login shell", true, false)
422430
);
423431

424432
let non_login_command =
425-
ShellCommandHandler::base_command(&shell, "echo non login shell", Some(false));
433+
ShellCommandHandler::base_command(&shell, "echo non login shell", Some(false), false);
426434
assert_eq!(
427435
non_login_command,
428-
shell.derive_exec_args("echo non login shell", false)
436+
shell.derive_exec_args("echo non login shell", false, false)
429437
);
430438
}
431439
}

codex-rs/core/src/tools/handlers/unified_exec.rs

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
11
use crate::features::Feature;
22
use crate::function_tool::FunctionCallError;
33
use crate::is_safe_command::is_known_safe_command;
4-
use crate::powershell::prefix_utf8_output;
54
use crate::protocol::EventMsg;
65
use crate::protocol::ExecCommandSource;
76
use crate::protocol::TerminalInteractionEvent;
87
use crate::sandboxing::SandboxPermissions;
98
use crate::shell::Shell;
10-
use crate::shell::ShellType;
119
use crate::shell::get_shell_by_model_provided_path;
1210
use crate::tools::context::ToolInvocation;
1311
use crate::tools::context::ToolOutput;
@@ -275,14 +273,7 @@ fn get_command(
275273
});
276274
let shell = model_shell.as_ref().unwrap_or(session_shell);
277275

278-
let command_str =
279-
if matches!(shell.shell_type, ShellType::PowerShell) && powershell_utf8_enabled {
280-
prefix_utf8_output(&args.cmd)
281-
} else {
282-
args.cmd.to_string()
283-
};
284-
285-
shell.derive_exec_args(&command_str, args.login)
276+
shell.derive_exec_args(&args.cmd, args.login, powershell_utf8_enabled)
286277
}
287278

288279
fn format_response(response: &UnifiedExecResponse) -> String {

codex-rs/core/tests/common/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ pub fn sandbox_network_env_var() -> &'static str {
220220
}
221221

222222
pub fn format_with_current_shell(command: &str) -> Vec<String> {
223-
codex_core::shell::default_user_shell().derive_exec_args(command, true)
223+
codex_core::shell::default_user_shell().derive_exec_args(command, true, false)
224224
}
225225

226226
pub fn format_with_current_shell_display(command: &str) -> String {
@@ -229,7 +229,7 @@ pub fn format_with_current_shell_display(command: &str) -> String {
229229
}
230230

231231
pub fn format_with_current_shell_non_login(command: &str) -> Vec<String> {
232-
codex_core::shell::default_user_shell().derive_exec_args(command, false)
232+
codex_core::shell::default_user_shell().derive_exec_args(command, false, false)
233233
}
234234

235235
pub fn format_with_current_shell_display_non_login(command: &str) -> String {

0 commit comments

Comments
 (0)