From bd4fe31c5a89316051e086ec672cd63b7339e99a Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Mon, 15 Jun 2026 10:17:34 -0700 Subject: [PATCH] [codex] preserve explicit environment cwd (#27995) ## Why `TurnEnvironmentSelections::new` rewrote the primary environment's explicit `cwd` to the legacy fallback cwd. For a remote-first selection, this could replace the remote working directory with a local fallback path and made the legacy cwd overlay authoritative over environment-owned state. ## What changed - Preserve every explicit environment cwd when constructing turn environment selections. - Keep `cwd`-only app-server updates compatible by rebuilding the default environment selections at the requested cwd. - Cover both explicit primary cwd preservation and cwd-only updates reaching the model-visible execution environment. ## Testing - `just test -p codex-core session_update_settings_does_not_rewrite_sticky_environment_cwds` - `just test -p codex-core environment_settings_preserve_explicit_primary_cwd` - `just test -p codex-app-server thread_settings_update_cwd_retargets_default_environment` --- .../src/request_processors/turn_processor.rs | 56 ++++++++++--------- .../tests/suite/v2/thread_settings_update.rs | 53 ++++++++++++++++++ codex-rs/core/src/session/tests.rs | 21 ++++--- codex-rs/protocol/src/protocol.rs | 16 +----- 4 files changed, 99 insertions(+), 47 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/turn_processor.rs b/codex-rs/app-server/src/request_processors/turn_processor.rs index c8074b8db..ccb45c367 100644 --- a/codex-rs/app-server/src/request_processors/turn_processor.rs +++ b/codex-rs/app-server/src/request_processors/turn_processor.rs @@ -435,8 +435,9 @@ impl TurnRequestProcessor { let additional_context = map_additional_context(params.additional_context); let turn_has_input = !mapped_items.is_empty(); let cwd = resolve_request_cwd(params.cwd)?; - let environments = - Self::build_environment_override(thread.as_ref(), cwd, environment_selections).await; + let environments = self + .build_environment_override(thread.as_ref(), cwd, environment_selections) + .await; let thread_settings = self .build_thread_settings_overrides( thread.as_ref(), @@ -509,28 +510,36 @@ impl TurnRequestProcessor { } async fn build_environment_override( + &self, thread: &CodexThread, cwd: Option, environment_selections: Option>, ) -> Option { - if cwd.is_none() && environment_selections.is_none() { - return None; + match (cwd, environment_selections) { + (None, None) => None, + (Some(cwd), None) => { + let environment_selections = + self.thread_manager.default_environment_selections(&cwd); + Some(TurnEnvironmentSelections::new(cwd, environment_selections)) + } + (cwd, Some(environment_selections)) => { + let legacy_fallback_cwd = match cwd { + Some(cwd) => cwd, + None => { + let snapshot = thread.config_snapshot().await; + environment_selections + .iter() + .find(|selection| selection.environment_id == LOCAL_ENVIRONMENT_ID) + .and_then(|selection| selection.cwd.to_abs_path().ok()) + .unwrap_or_else(|| snapshot.cwd().clone()) + } + }; + Some(TurnEnvironmentSelections::new( + legacy_fallback_cwd, + environment_selections, + )) + } } - - let snapshot = thread.config_snapshot().await; - let environment_selections = - environment_selections.unwrap_or_else(|| snapshot.environment_selections().to_vec()); - let legacy_fallback_cwd = cwd.unwrap_or_else(|| { - environment_selections - .iter() - .find(|selection| selection.environment_id == LOCAL_ENVIRONMENT_ID) - .and_then(|selection| selection.cwd.to_abs_path().ok()) - .unwrap_or_else(|| snapshot.cwd().clone()) - }); - Some(TurnEnvironmentSelections::new( - legacy_fallback_cwd, - environment_selections, - )) } async fn build_thread_settings_overrides( @@ -694,12 +703,9 @@ impl TurnRequestProcessor { ) -> Result { let (_, thread) = self.load_thread(¶ms.thread_id).await?; let cwd = resolve_request_cwd(params.cwd)?; - let environments = Self::build_environment_override( - thread.as_ref(), - cwd, - /*environment_selections*/ None, - ) - .await; + let environments = self + .build_environment_override(thread.as_ref(), cwd, /*environment_selections*/ None) + .await; let thread_settings = self .build_thread_settings_overrides( thread.as_ref(), diff --git a/codex-rs/app-server/tests/suite/v2/thread_settings_update.rs b/codex-rs/app-server/tests/suite/v2/thread_settings_update.rs index 98ad58732..29dbee26d 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_settings_update.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_settings_update.rs @@ -94,6 +94,59 @@ async fn thread_settings_update_emits_notification_and_updates_future_turns() -> Ok(()) } +#[tokio::test] +async fn thread_settings_update_cwd_retargets_default_environment() -> Result<()> { + let server = responses::start_mock_server().await; + let body = responses::sse(vec![ + responses::ev_response_created("resp-1"), + responses::ev_assistant_message("msg-1", "done"), + responses::ev_completed("resp-1"), + ]); + let response_mock = responses::mount_sse_once(&server, body).await; + let codex_home = TempDir::new()?; + let workspace = TempDir::new()?; + create_config_toml(codex_home.path(), &server.uri())?; + + let mut mcp = TestAppServer::new(codex_home.path()).await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + let thread = start_thread(&mut mcp).await?.thread; + + send_thread_settings_update( + &mut mcp, + ThreadSettingsUpdateParams { + thread_id: thread.id.clone(), + cwd: Some(workspace.path().to_path_buf()), + ..Default::default() + }, + ) + .await?; + let updated = read_thread_settings_updated(&mut mcp).await?; + assert_eq!(updated.thread_settings.cwd.as_path(), workspace.path()); + + start_text_turn(&mut mcp, thread.id).await?; + timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + let environment_context = response_mock + .single_request() + .message_input_texts("user") + .into_iter() + .find(|text| text.starts_with("")) + .context("environment context should be model visible")?; + assert!( + environment_context.contains(&format!( + "{}", + workspace.path().to_string_lossy() + )), + "default environment should use the updated cwd: {environment_context}" + ); + + Ok(()) +} + #[tokio::test] async fn thread_settings_update_while_turn_is_active_emits_notification() -> Result<()> { let server = responses::start_mock_server().await; diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 810398e97..d328a6a5a 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -4615,6 +4615,7 @@ async fn session_update_settings_does_not_rewrite_sticky_environment_cwds() { .environment_selections() .to_vec() }; + let expected_environments = current_environments.clone(); std::fs::create_dir_all(updated_cwd.as_path()).expect("create project dir"); session @@ -4628,21 +4629,28 @@ async fn session_update_settings_does_not_rewrite_sticky_environment_cwds() { .await .expect("cwd update should succeed"); - let session_cwd = { + let (session_cwd, stored_environments) = { let state = session.state.lock().await; - state.session_configuration.cwd().clone() + ( + state.session_configuration.cwd().clone(), + state + .session_configuration + .environment_selections() + .to_vec(), + ) }; let config = session.get_config().await; let next_turn = session.new_default_turn().await; assert_eq!(session_cwd, updated_cwd); + assert_eq!(stored_environments, expected_environments); #[allow(deprecated)] let turn_cwd = turn_context.cwd.clone(); #[allow(deprecated)] let next_turn_cwd = next_turn.cwd.clone(); assert_eq!(config.cwd, turn_cwd); - assert_eq!(next_turn_cwd, updated_cwd); - assert_eq!(next_turn.config.cwd, updated_cwd); + assert_eq!(next_turn_cwd, turn_cwd); + assert_eq!(next_turn.config.cwd, turn_cwd); } #[tokio::test] @@ -4678,7 +4686,7 @@ async fn relative_cwd_update_without_environments_resolves_under_session_cwd() { } #[tokio::test] -async fn cwd_update_rewrites_sticky_environment_cwd() { +async fn environment_settings_preserve_explicit_primary_cwd() { let (session, _turn_context) = make_session_and_context().await; let (original_cwd, environment_cwd, environments) = { let mut state = session.state.lock().await; @@ -4706,9 +4714,8 @@ async fn cwd_update_rewrites_sticky_environment_cwd() { assert_eq!(state.session_configuration.cwd(), &updated_cwd); assert_eq!( state.session_configuration.environment_selections()[0].cwd, - PathUri::from_abs_path(&updated_cwd) + PathUri::from_abs_path(&environment_cwd) ); - assert_ne!(environment_cwd, updated_cwd); } #[tokio::test] diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 92b8194b4..be1914e6d 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -106,7 +106,6 @@ pub const COLLABORATION_MODE_CLOSE_TAG: &str = ""; pub const REALTIME_CONVERSATION_OPEN_TAG: &str = ""; pub const REALTIME_CONVERSATION_CLOSE_TAG: &str = ""; pub const USER_MESSAGE_BEGIN: &str = "## My request for Codex:"; -const LOCAL_ENVIRONMENT_ID: &str = "local"; // TODO(anp): Replace `TurnEnvironmentSelection` with `PathUri` once path URIs carry environment // identifiers. @@ -127,22 +126,9 @@ impl TurnEnvironmentSelections { legacy_fallback_cwd: AbsolutePathBuf, environments: Vec, ) -> Self { - let mut settings = Self { + Self { legacy_fallback_cwd, environments, - }; - settings.sync_primary_environment_cwd(); - settings - } - - fn sync_primary_environment_cwd(&mut self) { - let legacy_fallback_cwd = PathUri::from_abs_path(&self.legacy_fallback_cwd); - // Keep remote environments' native cwd instead of replacing it with the local fallback. - if let Some(turn_environment) = self.environments.first_mut() - && turn_environment.environment_id == LOCAL_ENVIRONMENT_ID - && turn_environment.cwd != legacy_fallback_cwd - { - turn_environment.cwd = legacy_fallback_cwd; } } }