codex: address PR review feedback (#28146)

codex-rs/app-server/README.md

@@ -709,7 +709,7 @@ Turns attach user input (text or images) to a thread and trigger Codex generatio
 - `{"type":"image","url":"https://…png"}`
 - `{"type":"localImage","path":"/tmp/screenshot.png"}`
 
-You can optionally specify config overrides on the new turn. If specified, these settings become the default for subsequent turns on the same thread. `outputSchema` applies only to the current turn. Experimental `environments` is turn-scoped: omit it to inherit the thread's sticky environments, pass `[]` to run the turn with no environments, or pass explicit environment objects to override the sticky selection for this turn only. A `cwd` is allowed on each environment object.
+You can optionally specify config overrides on the new turn. If specified, these settings become the default for subsequent turns on the same thread. `outputSchema` applies only to the current turn. Experimental `environments` is turn-scoped: omit it to inherit the thread's sticky environments, pass `[]` to run the turn with no environments, or pass explicit environment ids to override the sticky selection for this turn only.
 
 `approvalsReviewer` accepts:
 

codex-rs/app-server/tests/suite/v2/thread_start.rs

@@ -327,15 +327,13 @@ async fn thread_start_rejects_relative_environment_cwd_as_invalid_request() -> R
     timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
 
     let request_id = mcp
-        .send_raw_request(
-            "thread/start",
-            Some(json!({
-                "environments": [{
-                    "environmentId": "local",
-                    "cwd": "relative",
-                }],
-            })),
-        )
+        .send_thread_start_request(ThreadStartParams {
+            environments: Some(vec![TurnEnvironmentParams {
+                environment_id: "local".to_string(),
+                cwd: serde_json::from_value(json!("relative"))?,
+            }]),
+            ..Default::default()
+        })
         .await?;
     let error: JSONRPCError = timeout(
         DEFAULT_READ_TIMEOUT,

codex-rs/core/src/agents_md_tests.rs

@@ -253,9 +253,10 @@ fn resolved_local_environments<const N: usize>(
                         Environment::create_for_tests(/*exec_server_url*/ None)
                             .expect("local environment"),
                     ),
-                    cwd,
+                    PathUri::from_abs_path(&cwd),
                     /*shell*/ None,
                 )
+                .expect("turn environment")
             })
             .collect(),
     }

codex-rs/core/src/environment_selection.rs

@@ -157,14 +157,15 @@ impl ThreadEnvironments {
         let mut turn_environment = TurnEnvironment::new(
             environment_id,
             environment,
-            selected_environment.cwd.to_abs_path().map_err(|err| {
-                CodexErr::InvalidRequest(format!(
-                    "turn environment cwd `{}` is not valid on this host: {err}",
-                    selected_environment.cwd
-                ))
-            })?,
+            selected_environment.cwd.clone(),
             shell,
-        );
+        )
+        .map_err(|err| {
+            CodexErr::InvalidRequest(format!(
+                "turn environment cwd `{}` is not valid on this host: {err}",
+                selected_environment.cwd
+            ))
+        })?;
         let task = shell_snapshot
             .clone()
             .build(turn_environment.clone())
@@ -560,7 +561,7 @@ url = "ws://127.0.0.1:8765"
             Arc::clone(&local_manager),
             &[TurnEnvironmentSelection {
                 environment_id: LOCAL_ENVIRONMENT_ID.to_string(),
-                cwd: cwd_uri,
+                cwd: cwd_uri.clone(),
             }],
         )
         .await;
@@ -570,12 +571,15 @@ url = "ws://127.0.0.1:8765"
                 .expect("remote environment"),
         );
         let remote = TurnEnvironmentSnapshot {
-            turn_environments: vec![TurnEnvironment::new(
-                REMOTE_ENVIRONMENT_ID.to_string(),
-                remote_environment.clone(),
-                cwd.clone(),
-                /*shell*/ None,
-            )],
+            turn_environments: vec![
+                TurnEnvironment::new(
+                    REMOTE_ENVIRONMENT_ID.to_string(),
+                    remote_environment.clone(),
+                    cwd_uri.clone(),
+                    /*shell*/ None,
+                )
+                .expect("remote turn environment"),
+            ],
         };
         let multiple = TurnEnvironmentSnapshot {
             turn_environments: vec![
@@ -583,9 +587,10 @@ url = "ws://127.0.0.1:8765"
                 TurnEnvironment::new(
                     REMOTE_ENVIRONMENT_ID.to_string(),
                     remote_environment,
-                    cwd.clone(),
+                    cwd_uri,
                     /*shell*/ None,
-                ),
+                )
+                .expect("remote turn environment"),
             ],
         };
 

codex-rs/core/src/session/tests.rs

@@ -5697,9 +5697,10 @@ async fn request_permissions_tool_resolves_relative_paths_against_selected_envir
     turn_context_mut.environments.turn_environments[0] = TurnEnvironment::new(
         "remote".to_string(),
         current_environment.environment,
-        environment_cwd.clone(),
+        codex_utils_path_uri::PathUri::from_abs_path(&environment_cwd),
         current_environment.shell,
-    );
+    )
+    .expect("remote turn environment");
 
     let call_id = "call-1".to_string();
     let handler = RequestPermissionsHandler;
@@ -6316,15 +6317,15 @@ async fn primary_environment_uses_first_turn_environment() {
     let first_environment = turn_context.environments.turn_environments[0].clone();
     #[allow(deprecated)]
     let second_cwd = turn_context.cwd.join("second");
-    turn_context
-        .environments
-        .turn_environments
-        .push(TurnEnvironment::new(
+    turn_context.environments.turn_environments.push(
+        TurnEnvironment::new(
             "second".to_string(),
             Arc::clone(&first_environment.environment),
-            second_cwd.clone(),
+            codex_utils_path_uri::PathUri::from_abs_path(&second_cwd),
             /*shell*/ None,
-        ));
+        )
+        .expect("second turn environment"),
+    );
 
     assert_eq!(
         turn_context

codex-rs/core/src/session/turn_context.rs

@@ -64,18 +64,18 @@ impl TurnEnvironment {
     pub(crate) fn new(
         environment_id: String,
         environment: Arc<Environment>,
-        cwd: AbsolutePathBuf,
+        cwd_uri: PathUri,
         shell: Option<shell::Shell>,
-    ) -> Self {
-        let cwd_uri = PathUri::from_abs_path(&cwd);
-        Self {
+    ) -> std::io::Result<Self> {
+        let cwd = cwd_uri.to_abs_path()?;
+        Ok(Self {
             environment_id,
             environment,
             cwd,
             cwd_uri,
             shell,
             shell_snapshot: futures::future::ready(None).boxed().shared(),
-        }
+        })
     }
 
     pub(crate) fn shell_snapshot(&self, cwd: &AbsolutePathBuf) -> Option<AbsolutePathBuf> {

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

@@ -288,9 +288,10 @@ mod tests {
         turn.environments.turn_environments[0] = TurnEnvironment::new(
             current.environment_id,
             current.environment,
-            cwd,
+            PathUri::from_abs_path(&cwd),
             current.shell,
-        );
+        )
+        .expect("turn environment");
     }
 
     #[test]

codex-rs/core/src/tools/runtimes/apply_patch_tests.rs

@@ -19,9 +19,10 @@ fn test_turn_environment(environment_id: &str) -> crate::session::turn_context::
     crate::session::turn_context::TurnEnvironment::new(
         environment_id.to_string(),
         std::sync::Arc::new(codex_exec_server::Environment::default_for_tests()),
-        std::env::temp_dir().abs(),
+        PathUri::from_abs_path(&std::env::temp_dir().abs()),
         /*shell*/ None,
     )
+    .expect("turn environment")
 }
 
 #[test]

codex-rs/core/src/tools/spec_plan_tests.rs

@@ -597,9 +597,10 @@ async fn zsh_fork_unified_exec_keeps_shell_parameter_when_remote_environment_ava
                     ))
                     .expect("remote test environment"),
                 ),
-                remote_cwd,
+                codex_utils_path_uri::PathUri::from_abs_path(&remote_cwd),
                 /*shell*/ None,
-            ),
+            )
+            .expect("remote turn environment"),
         );
     })
     .await;

codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs

@@ -10,6 +10,7 @@ use app_test_support::write_mock_responses_config_toml;
 use base64::Engine as _;
 use base64::engine::general_purpose::STANDARD;
 use codex_app_server_protocol::AskForApproval as AppAskForApproval;
+use codex_app_server_protocol::CommandAction;
 use codex_app_server_protocol::CommandExecutionStatus;
 use codex_app_server_protocol::ItemCompletedNotification;
 use codex_app_server_protocol::RequestId;
@@ -186,7 +187,7 @@ async fn windows_exec_server_runs_with_native_shell_and_cwd() -> Result<()> {
 async fn app_server_starts_thread_with_windows_environment_native_cwd() -> Result<()> {
     const AGENTS_INSTRUCTIONS: &str = "remote Windows workspace instructions";
     const CALL_ID: &str = "wine-cmd-smoke";
-    const COMMAND: &str = r#"if ((Get-Location).Path -ne 'C:\windows') { exit 1 }"#;
+    const COMMAND: &str = "Get-Content AGENTS.md -ErrorAction Stop";
 
     WineExecServer
         .scope(|exec_server_url| async move {
@@ -310,6 +311,8 @@ async fn app_server_starts_thread_with_windows_environment_native_cwd() -> Resul
             })
             .await??;
             let ThreadItem::CommandExecution {
+                command_actions,
+                cwd,
                 id,
                 status,
                 exit_code,
@@ -319,6 +322,12 @@ async fn app_server_starts_thread_with_windows_environment_native_cwd() -> Resul
                 unreachable!("loop returns only command execution items");
             };
             assert_eq!(id, CALL_ID);
+            // TODO(anp): Make command execution cwd a PathUri so this stays `C:\windows`.
+            assert_eq!(cwd, std::path::PathBuf::from("/C:/windows").abs());
+            // TODO(anp): Parse command actions using the selected environment's path convention so
+            // their paths remain Windows-native instead of degrading the action to Unknown.
+            assert_eq!(command_actions.len(), 1);
+            assert!(matches!(command_actions[0], CommandAction::Unknown { .. }));
             assert_eq!((status, exit_code), (CommandExecutionStatus::Completed, Some(0)));
             timeout(
                 APP_SERVER_READ_TIMEOUT,