From 63a27ad6c6749dbc5e35205cb4619a8957f3fd19 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Wed, 6 May 2026 12:54:26 -0700 Subject: [PATCH] Avoid hard-coded environment context shell (#21390) ## Summary - make resolved turn environment shell metadata optional instead of hard-coding bash - render environment context shells from explicit environment metadata when present, falling back to the existing session shell - update environment context tests for inherited PowerShell-style fallback and explicit per-environment shell override ## Testing - Not run (not requested; formatted with `just fmt`). Co-authored-by: Codex --- .../core/src/context/environment_context.rs | 14 +++--- .../src/context/environment_context_tests.rs | 43 ++++++++++++++++++ codex-rs/core/src/context_manager/updates.rs | 2 +- codex-rs/core/src/environment_selection.rs | 5 +-- codex-rs/core/src/session/mod.rs | 3 +- codex-rs/core/src/session/tests.rs | 45 ++++++++++++++++++- codex-rs/core/src/session/turn_context.rs | 2 +- codex-rs/core/tests/suite/prompt_caching.rs | 3 +- 8 files changed, 103 insertions(+), 14 deletions(-) diff --git a/codex-rs/core/src/context/environment_context.rs b/codex-rs/core/src/context/environment_context.rs index c647550ee..ca1ac5f2f 100644 --- a/codex-rs/core/src/context/environment_context.rs +++ b/codex-rs/core/src/context/environment_context.rs @@ -1,4 +1,6 @@ use crate::session::turn_context::TurnContext; +use crate::session::turn_context::TurnEnvironment; +use crate::shell::Shell; use codex_protocol::protocol::TurnContextItem; use codex_protocol::protocol::TurnContextNetworkItem; use codex_utils_absolute_path::AbsolutePathBuf; @@ -30,15 +32,16 @@ impl EnvironmentContextEnvironment { } } - fn from_turn_environments( - environments: &[crate::session::turn_context::TurnEnvironment], - ) -> Vec { + fn from_turn_environments(environments: &[TurnEnvironment], shell: &Shell) -> Vec { environments .iter() .map(|environment| Self { id: environment.environment_id.clone(), cwd: environment.cwd.clone(), - shell: environment.shell.clone(), + shell: environment + .shell + .clone() + .unwrap_or_else(|| shell.name().to_string()), }) .collect() } @@ -174,10 +177,11 @@ impl EnvironmentContext { ) } - pub(crate) fn from_turn_context(turn_context: &TurnContext) -> Self { + pub(crate) fn from_turn_context(turn_context: &TurnContext, shell: &Shell) -> Self { Self::new( EnvironmentContextEnvironment::from_turn_environments( &turn_context.environments.turn_environments, + shell, ), turn_context.current_date.clone(), turn_context.timezone.clone(), diff --git a/codex-rs/core/src/context/environment_context_tests.rs b/codex-rs/core/src/context/environment_context_tests.rs index 24ff4bbff..bc0a17ca5 100644 --- a/codex-rs/core/src/context/environment_context_tests.rs +++ b/codex-rs/core/src/context/environment_context_tests.rs @@ -259,3 +259,46 @@ fn serialize_environment_context_with_multiple_selected_environments() { assert_eq!(context.render(), expected); } + +#[test] +fn serialize_environment_context_prefers_environment_shell_when_present() { + let local_cwd = test_path_buf("/repo/local"); + let remote_cwd = test_path_buf("/repo/remote"); + let context = EnvironmentContext::new( + vec![ + EnvironmentContextEnvironment { + id: "local".to_string(), + cwd: local_cwd.abs(), + shell: "powershell".to_string(), + }, + EnvironmentContextEnvironment { + id: "remote".to_string(), + cwd: remote_cwd.abs(), + shell: "cmd".to_string(), + }, + ], + /*current_date*/ None, + /*timezone*/ None, + /*network*/ None, + /*subagents*/ None, + ); + + let expected = format!( + r#" + + + {} + powershell + + + {} + cmd + + +"#, + local_cwd.display(), + remote_cwd.display() + ); + + assert_eq!(context.render(), expected); +} diff --git a/codex-rs/core/src/context_manager/updates.rs b/codex-rs/core/src/context_manager/updates.rs index db7850008..1bc2cb089 100644 --- a/codex-rs/core/src/context_manager/updates.rs +++ b/codex-rs/core/src/context_manager/updates.rs @@ -29,7 +29,7 @@ fn build_environment_update_item( let prev = previous?; let prev_context = EnvironmentContext::from_turn_context_item(prev, shell.name().to_string()); - let next_context = EnvironmentContext::from_turn_context(next); + let next_context = EnvironmentContext::from_turn_context(next, shell); if prev_context.equals_except_shell(&next_context) { return None; } diff --git a/codex-rs/core/src/environment_selection.rs b/codex-rs/core/src/environment_selection.rs index e9d617cbf..b4bd9cbe8 100644 --- a/codex-rs/core/src/environment_selection.rs +++ b/codex-rs/core/src/environment_selection.rs @@ -75,9 +75,7 @@ pub(crate) fn resolve_environment_selections( environment_id, environment, cwd: selected_environment.cwd.clone(), - // TODO(starr): Resolve shell metadata per environment instead of - // hardcoding bash. - shell: "bash".to_string(), + shell: None, }); } @@ -176,5 +174,6 @@ mod tests { .environment_id, "local" ); + assert_eq!(resolved.primary().expect("primary environment").shell, None); } } diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index bacd1708e..8782f14b3 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -2703,13 +2703,14 @@ impl Session { ); } if turn_context.config.include_environment_context { + let shell = self.user_shell(); let subagents = self .services .agent_control .format_environment_context_subagents(self.conversation_id) .await; contextual_user_sections.push( - crate::context::EnvironmentContext::from_turn_context(turn_context) + crate::context::EnvironmentContext::from_turn_context(turn_context, shell.as_ref()) .with_subagents(subagents) .render(), ); diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 4cce19f6b..cae3e1f97 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -3057,7 +3057,7 @@ fn turn_environments_for_tests( environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), environment: Arc::clone(environment), cwd: cwd.clone(), - shell: "bash".to_string(), + shell: None, }] } @@ -4803,7 +4803,7 @@ async fn primary_environment_uses_first_turn_environment() { environment_id: "second".to_string(), environment: Arc::clone(&first_environment.environment), cwd: second_cwd.clone(), - shell: "bash".to_string(), + shell: None, }); assert_eq!( @@ -5749,6 +5749,47 @@ async fn build_settings_update_items_emits_environment_item_for_network_changes( assert!(environment_update.contains("blocked.example.com")); } +#[tokio::test] +async fn environment_context_uses_session_shell_when_environment_shell_is_absent() { + let (mut session, mut turn_context) = make_session_and_context().await; + session.services.user_shell = Arc::new(crate::shell::Shell { + shell_type: crate::shell::ShellType::PowerShell, + shell_path: PathBuf::from("powershell"), + shell_snapshot: crate::shell::empty_shell_snapshot_receiver(), + }); + for environment in &mut turn_context.environments.turn_environments { + environment.shell = None; + } + + let session_shell = session.user_shell(); + let environment_context = crate::context::EnvironmentContext::from_turn_context( + &turn_context, + session_shell.as_ref(), + ) + .render(); + assert!( + environment_context.contains("powershell"), + "{environment_context}" + ); + + let primary_environment = turn_context + .environments + .turn_environments + .first_mut() + .expect("primary environment"); + primary_environment.shell = Some("cmd".to_string()); + + let environment_context = crate::context::EnvironmentContext::from_turn_context( + &turn_context, + session_shell.as_ref(), + ) + .render(); + assert!( + environment_context.contains("cmd"), + "{environment_context}" + ); +} + #[tokio::test] async fn build_settings_update_items_emits_environment_item_for_time_changes() { let (session, previous_context) = make_session_and_context().await; diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 6d8443e93..769953475 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -39,7 +39,7 @@ pub(crate) struct TurnEnvironment { pub(crate) environment_id: String, pub(crate) environment: Arc, pub(crate) cwd: AbsolutePathBuf, - pub(crate) shell: String, + pub(crate) shell: Option, } impl TurnEnvironment { diff --git a/codex-rs/core/tests/suite/prompt_caching.rs b/codex-rs/core/tests/suite/prompt_caching.rs index cc8e57f0f..b81bb06bb 100644 --- a/codex-rs/core/tests/suite/prompt_caching.rs +++ b/codex-rs/core/tests/suite/prompt_caching.rs @@ -1,6 +1,7 @@ #![allow(clippy::unwrap_used)] use codex_apply_patch::APPLY_PATCH_TOOL_INSTRUCTIONS; +use codex_core::shell::default_user_shell; use codex_features::Feature; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; @@ -54,7 +55,7 @@ fn assert_default_env_context(text: &str, cwd: &str) { "expected cwd in environment context: {text}" ); assert!( - text.contains("bash"), + text.contains(&format!("{}", default_user_shell().name())), "expected shell in environment context: {text}" ); assert!(