openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-29 19:08:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

Stabilize Windows shell test harnesses

Browse Source 
Add a test-only user shell override so the Windows-flaky shell command
integration tests can pin cmd.exe without changing live session state.
Use that seam in the apply_patch and websocket shell-chain suites, and
trim the redundant live PowerShell discovery from the safe-command
handler regression test. Update the flaky-test triage notes with the
latest Windows-only CI failure pair and this follow-up.

Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
Ahmed Ibrahim
2026-03-17 00:17:02 +00:00
co-authored by Codex
parent 9192637e4a
commit a2b78a6602
11 changed files with 151 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/codex.rs
+9 -1
View File
@@ -371,6 +371,7 @@ pub(crate) struct CodexSpawnArgs {
pub(crate) persist_extended_history: bool,
pub(crate) metrics_service_name: Option<String>,
pub(crate) inherited_shell_snapshot: Option<Arc<ShellSnapshot>>,
pub(crate) user_shell_override: Option<shell::Shell>,
pub(crate) parent_trace: Option<W3cTraceContext>,
}
@@ -422,6 +423,7 @@ impl Codex {
persist_extended_history,
metrics_service_name,
inherited_shell_snapshot,
user_shell_override,
parent_trace: _,
} = args;
let (tx_sub, rx_sub) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY);
@@ -576,6 +578,7 @@ impl Codex {
dynamic_tools,
persist_extended_history,
inherited_shell_snapshot,
user_shell_override,
};
// Generate a unique ID for the lifetime of this Codex session.
@@ -1036,6 +1039,7 @@ pub(crate) struct SessionConfiguration {
dynamic_tools: Vec<DynamicToolSpec>,
persist_extended_history: bool,
inherited_shell_snapshot: Option<Arc<ShellSnapshot>>,
user_shell_override: Option<shell::Shell>,
}
impl SessionConfiguration {
@@ -1616,7 +1620,11 @@ impl Session {
);
let use_zsh_fork_shell = config.features.enabled(Feature::ShellZshFork);
let mut default_shell = if use_zsh_fork_shell {
let mut default_shell = if let Some(user_shell_override) =
session_configuration.user_shell_override.clone()
{
user_shell_override
} else if use_zsh_fork_shell {
let zsh_path = config.zsh_path.as_ref().ok_or_else(|| {
anyhow::anyhow!(
"zsh fork feature enabled, but `zsh_path` is not configured; set `zsh_path` in config.toml"
codex-rs/core/src/codex_delegate.rs
+1
View File
@@ -88,6 +88,7 @@ pub(crate) async fn run_codex_thread_interactive(
persist_extended_history: false,
metrics_service_name: None,
inherited_shell_snapshot: None,
user_shell_override: None,
parent_trace: None,
})
.await?;
codex-rs/core/src/codex_tests.rs
+6
View File
@@ -1563,6 +1563,7 @@ async fn set_rate_limits_retains_previous_credits() {
dynamic_tools: Vec::new(),
persist_extended_history: false,
inherited_shell_snapshot: None,
user_shell_override: None,
};
let mut state = SessionState::new(session_configuration);
@@ -1660,6 +1661,7 @@ async fn set_rate_limits_updates_plan_type_when_present() {
dynamic_tools: Vec::new(),
persist_extended_history: false,
inherited_shell_snapshot: None,
user_shell_override: None,
};
let mut state = SessionState::new(session_configuration);
@@ -2015,6 +2017,7 @@ pub(crate) async fn make_session_configuration_for_tests() -> SessionConfigurati
dynamic_tools: Vec::new(),
persist_extended_history: false,
inherited_shell_snapshot: None,
user_shell_override: None,
}
}
@@ -2245,6 +2248,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_zsh_path() {
dynamic_tools: Vec::new(),
persist_extended_history: false,
inherited_shell_snapshot: None,
user_shell_override: None,
};
let (tx_event, _rx_event) = async_channel::unbounded();
@@ -2339,6 +2343,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
dynamic_tools: Vec::new(),
persist_extended_history: false,
inherited_shell_snapshot: None,
user_shell_override: None,
};
let per_turn_config = Session::build_per_turn_config(&session_configuration);
let model_info = ModelsManager::construct_model_info_offline_for_tests(
@@ -3129,6 +3134,7 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx(
dynamic_tools,
persist_extended_history: false,
inherited_shell_snapshot: None,
user_shell_override: None,
};
let per_turn_config = Session::build_per_turn_config(&session_configuration);
let model_info = ModelsManager::construct_model_info_offline_for_tests(
codex-rs/core/src/codex_tests_guardian.rs
+1
View File
@@ -452,6 +452,7 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() {
persist_extended_history: false,
metrics_service_name: None,
inherited_shell_snapshot: None,
user_shell_override: None,
parent_trace: None,
})
.await
codex-rs/core/src/test_support.rs
+27
View File
@@ -64,6 +64,33 @@ pub fn thread_manager_with_models_provider_and_home(
ThreadManager::with_models_provider_and_home_for_tests(auth, provider, codex_home)
}
pub async fn start_thread_with_user_shell_override(
thread_manager: &ThreadManager,
config: Config,
user_shell_override: crate::shell::Shell,
) -> crate::error::Result<crate::NewThread> {
thread_manager
.start_thread_with_user_shell_override_for_tests(config, user_shell_override)
.await
}
pub async fn resume_thread_from_rollout_with_user_shell_override(
thread_manager: &ThreadManager,
config: Config,
rollout_path: PathBuf,
auth_manager: Arc<AuthManager>,
user_shell_override: crate::shell::Shell,
) -> crate::error::Result<crate::NewThread> {
thread_manager
.resume_thread_from_rollout_with_user_shell_override_for_tests(
config,
rollout_path,
auth_manager,
user_shell_override,
)
.await
}
pub fn models_manager_with_provider(
codex_home: PathBuf,
auth_manager: Arc<AuthManager>,
codex-rs/core/src/thread_manager.rs
+51
View File
@@ -376,6 +376,7 @@ impl ThreadManager {
persist_extended_history,
metrics_service_name,
parent_trace,
None,
))
.await
}
@@ -415,6 +416,48 @@ impl ThreadManager {
persist_extended_history,
None,
parent_trace,
None,
))
.await
}
pub(crate) async fn start_thread_with_user_shell_override_for_tests(
&self,
config: Config,
user_shell_override: crate::shell::Shell,
) -> CodexResult<NewThread> {
Box::pin(self.state.spawn_thread(
config,
InitialHistory::New,
Arc::clone(&self.state.auth_manager),
self.agent_control(),
Vec::new(),
false,
None,
None,
Some(user_shell_override),
))
.await
}
pub(crate) async fn resume_thread_from_rollout_with_user_shell_override_for_tests(
&self,
config: Config,
rollout_path: PathBuf,
auth_manager: Arc<AuthManager>,
user_shell_override: crate::shell::Shell,
) -> CodexResult<NewThread> {
let initial_history = RolloutRecorder::get_rollout_history(&rollout_path).await?;
Box::pin(self.state.spawn_thread(
config,
initial_history,
auth_manager,
self.agent_control(),
Vec::new(),
false,
None,
None,
Some(user_shell_override),
))
.await
}
@@ -500,6 +543,7 @@ impl ThreadManager {
persist_extended_history,
None,
parent_trace,
None,
))
.await
}
@@ -585,6 +629,7 @@ impl ThreadManagerState {
metrics_service_name,
inherited_shell_snapshot,
None,
None,
))
.await
}
@@ -609,6 +654,7 @@ impl ThreadManagerState {
None,
inherited_shell_snapshot,
None,
None,
))
.await
}
@@ -633,6 +679,7 @@ impl ThreadManagerState {
None,
inherited_shell_snapshot,
None,
None,
))
.await
}
@@ -649,6 +696,7 @@ impl ThreadManagerState {
persist_extended_history: bool,
metrics_service_name: Option<String>,
parent_trace: Option<W3cTraceContext>,
user_shell_override: Option<crate::shell::Shell>,
) -> CodexResult<NewThread> {
Box::pin(self.spawn_thread_with_source(
config,
@@ -661,6 +709,7 @@ impl ThreadManagerState {
metrics_service_name,
None,
parent_trace,
user_shell_override,
))
.await
}
@@ -678,6 +727,7 @@ impl ThreadManagerState {
metrics_service_name: Option<String>,
inherited_shell_snapshot: Option<Arc<ShellSnapshot>>,
parent_trace: Option<W3cTraceContext>,
user_shell_override: Option<crate::shell::Shell>,
) -> CodexResult<NewThread> {
let watch_registration = self
.file_watcher
@@ -699,6 +749,7 @@ impl ThreadManagerState {
persist_extended_history,
metrics_service_name,
inherited_shell_snapshot,
user_shell_override,
parent_trace,
})
.await?;
codex-rs/core/src/tools/handlers/shell_tests.rs
+2 -20
View File
@@ -7,8 +7,6 @@ use pretty_assertions::assert_eq;
use crate::codex::make_session_and_context;
use crate::exec_env::create_env;
use crate::is_safe_command::is_known_safe_command;
use crate::powershell::try_find_powershell_executable_blocking;
use crate::powershell::try_find_pwsh_executable_blocking;
use crate::sandboxing::SandboxPermissions;
use crate::shell::Shell;
use crate::shell::ShellType;
@@ -34,24 +32,8 @@ fn commands_generated_by_shell_command_handler_can_be_matched_by_is_known_safe_c
shell_snapshot: crate::shell::empty_shell_snapshot_receiver(),
};
assert_safe(&zsh_shell, "ls -la");
if let Some(path) = try_find_powershell_executable_blocking() {
let powershell = Shell {
shell_type: ShellType::PowerShell,
shell_path: path.to_path_buf(),
shell_snapshot: crate::shell::empty_shell_snapshot_receiver(),
};
assert_safe(&powershell, "ls -Name");
}
if let Some(path) = try_find_pwsh_executable_blocking() {
let pwsh = Shell {
shell_type: ShellType::PowerShell,
shell_path: path.to_path_buf(),
shell_snapshot: crate::shell::empty_shell_snapshot_receiver(),
};
assert_safe(&pwsh, "ls -Name");
}
// PowerShell wrapper safety lives in codex-shell-command, which already
// covers real executable discovery and parser behavior directly.
}
fn assert_safe(shell: &Shell, command: &str) {
codex-rs/core/tests/common/test_codex.rs
+44 -3
View File
@@ -13,6 +13,8 @@ use codex_core::built_in_model_providers;
use codex_core::config::Config;
use codex_core::features::Feature;
use codex_core::models_manager::collaboration_mode_presets::CollaborationModesConfig;
use codex_core::shell::Shell;
use codex_core::shell::get_shell_by_model_provided_path;
use codex_protocol::config_types::ServiceTier;
use codex_protocol::openai_models::ModelsResponse;
use codex_protocol::protocol::AskForApproval;
@@ -64,6 +66,7 @@ pub struct TestCodexBuilder {
auth: CodexAuth,
pre_build_hooks: Vec<Box<PreBuildHook>>,
home: Option<Arc<TempDir>>,
user_shell_override: Option<Shell>,
}
impl TestCodexBuilder {
@@ -100,6 +103,19 @@ impl TestCodexBuilder {
self
}
pub fn with_user_shell(mut self, user_shell: Shell) -> Self {
self.user_shell_override = Some(user_shell);
self
}
pub fn with_windows_cmd_shell(self) -> Self {
if cfg!(windows) {
self.with_user_shell(get_shell_by_model_provided_path(&PathBuf::from("cmd.exe")))
} else {
self
}
}
pub async fn build(&mut self, server: &wiremock::MockServer) -> anyhow::Result<TestCodex> {
let home = match self.home.clone() {
Some(home) => home,
@@ -194,9 +210,23 @@ impl TestCodexBuilder {
)
};
let thread_manager = Arc::new(thread_manager);
let user_shell_override = self.user_shell_override.clone();
let new_conversation = match resume_from {
Some(path) => {
let new_conversation = match (resume_from, user_shell_override) {
(Some(path), Some(user_shell_override)) => {
let auth_manager = codex_core::test_support::auth_manager_from_auth(auth);
Box::pin(
codex_core::test_support::resume_thread_from_rollout_with_user_shell_override(
thread_manager.as_ref(),
config.clone(),
path,
auth_manager,
user_shell_override,
),
)
.await?
}
(Some(path), None) => {
let auth_manager = codex_core::test_support::auth_manager_from_auth(auth);
Box::pin(thread_manager.resume_thread_from_rollout(
config.clone(),
@@ -206,7 +236,17 @@ impl TestCodexBuilder {
))
.await?
}
None => Box::pin(thread_manager.start_thread(config.clone())).await?,
(None, Some(user_shell_override)) => {
Box::pin(
codex_core::test_support::start_thread_with_user_shell_override(
thread_manager.as_ref(),
config.clone(),
user_shell_override,
),
)
.await?
}
(None, None) => Box::pin(thread_manager.start_thread(config.clone())).await?,
};
Ok(TestCodex {
@@ -552,6 +592,7 @@ pub fn test_codex() -> TestCodexBuilder {
auth: CodexAuth::from_api_key("dummy"),
pre_build_hooks: vec![],
home: None,
user_shell_override: None,
}
}
codex-rs/core/tests/suite/agent_websocket.rs
+2 -2
View File
@@ -35,7 +35,7 @@ async fn websocket_test_codex_shell_chain() -> Result<()> {
]])
.await;
let mut builder = test_codex();
let mut builder = test_codex().with_windows_cmd_shell();
let test = builder.build_with_websocket_server(&server).await?;
test.submit_turn_with_policy(
@@ -183,7 +183,7 @@ async fn websocket_v2_test_codex_shell_chain() -> Result<()> {
]])
.await;
let mut builder = test_codex().with_config(|config| {
let mut builder = test_codex().with_windows_cmd_shell().with_config(|config| {
config
.features
.enable(Feature::ResponsesWebsocketsV2)
codex-rs/core/tests/suite/apply_patch_cli.rs
+4 -6
View File
@@ -740,7 +740,9 @@ async fn apply_patch_shell_command_heredoc_with_cd_updates_relative_workdir() ->
async fn apply_patch_cli_can_use_shell_command_output_as_patch_input() -> Result<()> {
skip_if_no_network!(Ok(()));
let harness = apply_patch_harness_with(|builder| builder.with_model("gpt-5.1")).await?;
let harness =
apply_patch_harness_with(|builder| builder.with_model("gpt-5.1").with_windows_cmd_shell())
.await?;
let source_contents = "line1\nnaïve café\nline3\n";
let source_path = harness.path("source.txt");
@@ -786,7 +788,7 @@ async fn apply_patch_cli_can_use_shell_command_output_as_patch_input() -> Result
match call_num {
0 => {
let command = if cfg!(windows) {
"Get-Content -Encoding utf8 source.txt"
r#"powershell.exe -NoLogo -NoProfile -Command "Get-Content -Encoding utf8 source.txt""#
} else {
"cat source.txt"
};
@@ -807,9 +809,7 @@ async fn apply_patch_cli_can_use_shell_command_output_as_patch_input() -> Result
let body_json: serde_json::Value =
request.body_json().expect("request body should be json");
let read_output = function_call_output_text(&body_json, &self.read_call_id);
eprintln!("read_output: \n{read_output}");
let stdout = stdout_from_shell_output(&read_output);
eprintln!("stdout: \n{stdout}");
let patch_lines = stdout
.lines()
.map(|line| format!("+{line}"))
@@ -819,8 +819,6 @@ async fn apply_patch_cli_can_use_shell_command_output_as_patch_input() -> Result
"*** Begin Patch\n*** Add File: target.txt\n{patch_lines}\n*** End Patch"
);
eprintln!("patch: \n{patch}");
let body = sse(vec![
ev_response_created("resp-2"),
ev_apply_patch_custom_tool_call(&self.apply_call_id, &patch),