From 862158b9e990f06c64419258f7ec661b94a4f8a1 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Thu, 2 Apr 2026 17:28:47 -0700 Subject: [PATCH] app-server: make thread/shellCommand tests shell-aware (#16635) ## Why `thread/shellCommand` executes the raw command string through the current user shell, which is PowerShell on Windows. The two v2 app-server tests in `app-server/tests/suite/v2/thread_shell_command.rs` used POSIX `printf`, so Bazel CI on Windows failed with `printf` not being recognized as a PowerShell command. For reference, the user-shell task wraps commands with the active shell before execution: [`core/src/tasks/user_shell.rs`](https://github.com/openai/codex/blob/7a3eec6fdb356bd71f80582119eb829179ff0da1/codex-rs/core/src/tasks/user_shell.rs#L120-L126). ## What Changed Added a test-local helper that builds a shell-appropriate output command and expected newline sequence from `default_user_shell()`: - PowerShell: `Write-Output '...'` with `\r\n` - Cmd: `echo ...` with `\r\n` - POSIX shells: `printf '%s\n' ...` with `\n` Both `thread_shell_command_runs_as_standalone_turn_and_persists_history` and `thread_shell_command_uses_existing_active_turn` now use that helper. ## Verification - `cargo test -p codex-app-server thread_shell_command` --- .../tests/suite/v2/thread_shell_command.rs | 35 +++++++++++++++---- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/codex-rs/app-server/tests/suite/v2/thread_shell_command.rs b/codex-rs/app-server/tests/suite/v2/thread_shell_command.rs index 4e8a40fca..d5e690327 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_shell_command.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_shell_command.rs @@ -26,6 +26,7 @@ use codex_app_server_protocol::TurnCompletedNotification; use codex_app_server_protocol::TurnStartParams; use codex_app_server_protocol::TurnStartResponse; use codex_app_server_protocol::UserInput as V2UserInput; +use codex_core::shell::default_user_shell; use codex_features::FEATURES; use codex_features::Feature; use pretty_assertions::assert_eq; @@ -67,11 +68,12 @@ async fn thread_shell_command_runs_as_standalone_turn_and_persists_history() -> ) .await??; let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; + let (shell_command, expected_output) = current_shell_output_command("hello from bang")?; let shell_id = mcp .send_thread_shell_command_request(ThreadShellCommandParams { thread_id: thread.id.clone(), - command: "printf 'hello from bang\\n'".to_string(), + command: shell_command, }) .await?; let shell_resp: JSONRPCResponse = timeout( @@ -93,7 +95,7 @@ async fn thread_shell_command_runs_as_standalone_turn_and_persists_history() -> assert_eq!(status, &CommandExecutionStatus::InProgress); let delta = wait_for_command_execution_output_delta(&mut mcp, &command_id).await?; - assert_eq!(delta.delta, "hello from bang\n"); + assert_eq!(delta.delta, expected_output); let completed = wait_for_command_execution_completed(&mut mcp, Some(&command_id)).await?; let ThreadItem::CommandExecution { @@ -110,7 +112,7 @@ async fn thread_shell_command_runs_as_standalone_turn_and_persists_history() -> assert_eq!(id, &command_id); assert_eq!(source, &CommandExecutionSource::UserShell); assert_eq!(status, &CommandExecutionStatus::Completed); - assert_eq!(aggregated_output.as_deref(), Some("hello from bang\n")); + assert_eq!(aggregated_output.as_deref(), Some(expected_output.as_str())); assert_eq!(*exit_code, Some(0)); timeout( @@ -147,7 +149,7 @@ async fn thread_shell_command_runs_as_standalone_turn_and_persists_history() -> }; assert_eq!(source, &CommandExecutionSource::UserShell); assert_eq!(status, &CommandExecutionStatus::Completed); - assert_eq!(aggregated_output.as_deref(), Some("hello from bang\n")); + assert_eq!(aggregated_output.as_deref(), Some(expected_output.as_str())); Ok(()) } @@ -196,6 +198,7 @@ async fn thread_shell_command_uses_existing_active_turn() -> Result<()> { ) .await??; let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; + let (shell_command, expected_output) = current_shell_output_command("active turn bang")?; let turn_id = mcp .send_turn_start_request(TurnStartParams { @@ -240,7 +243,7 @@ async fn thread_shell_command_uses_existing_active_turn() -> Result<()> { let shell_id = mcp .send_thread_shell_command_request(ThreadShellCommandParams { thread_id: thread.id.clone(), - command: "printf 'active turn bang\\n'".to_string(), + command: shell_command, }) .await?; let shell_resp: JSONRPCResponse = timeout( @@ -269,7 +272,7 @@ async fn thread_shell_command_uses_existing_active_turn() -> Result<()> { unreachable!("helper returns command execution item"); }; assert_eq!(source, &CommandExecutionSource::UserShell); - assert_eq!(aggregated_output.as_deref(), Some("active turn bang\n")); + assert_eq!(aggregated_output.as_deref(), Some(expected_output.as_str())); mcp.send_response( request_id, @@ -309,7 +312,7 @@ async fn thread_shell_command_uses_existing_active_turn() -> Result<()> { source: CommandExecutionSource::UserShell, aggregated_output, .. - } if aggregated_output.as_deref() == Some("active turn bang\n") + } if aggregated_output.as_deref() == Some(expected_output.as_str()) ) }), "expected active-turn shell command to be persisted on the existing turn" @@ -318,6 +321,24 @@ async fn thread_shell_command_uses_existing_active_turn() -> Result<()> { Ok(()) } +fn current_shell_output_command(text: &str) -> Result<(String, String)> { + let command_and_output = match default_user_shell().name() { + "powershell" => { + let escaped_text = text.replace('\'', "''"); + ( + format!("Write-Output '{escaped_text}'"), + format!("{text}\r\n"), + ) + } + "cmd" => (format!("echo {text}"), format!("{text}\r\n")), + _ => { + let quoted_text = shlex::try_quote(text)?; + (format!("printf '%s\\n' {quoted_text}"), format!("{text}\n")) + } + }; + Ok(command_and_output) +} + async fn wait_for_command_execution_started( mcp: &mut McpProcess, expected_id: Option<&str>,