[codex] resolve environment shell metadata eagerly

codex-rs/app-server/src/request_processors/request_errors.rs

@@ -1,8 +1,8 @@
 use super::*;
 
-pub(super) fn environment_selection_error_message(err: CodexErr) -> String {
+pub(super) fn environment_selection_error(err: CodexErr) -> JSONRPCErrorError {
     match err {
-        CodexErr::InvalidRequest(message) => message,
-        err => err.to_string(),
+        CodexErr::InvalidRequest(message) => invalid_request(message),
+        err => internal_error(format!("failed to validate environment selections: {err}")),
     }
 }

codex-rs/app-server/src/request_processors/thread_processor.rs

@@ -1299,7 +1299,7 @@ impl ThreadRequestProcessor {
         if let Some(environment_selections) = environment_selections.as_ref() {
             self.thread_manager
                 .validate_environment_selections(environment_selections)
-                .map_err(|err| invalid_request(environment_selection_error_message(err)))?;
+                .map_err(environment_selection_error)?;
         }
         Ok(environment_selections)
     }

codex-rs/app-server/src/request_processors/turn_processor.rs

@@ -346,7 +346,7 @@ impl TurnRequestProcessor {
         if let Some(environment_selections) = environment_selections.as_ref() {
             self.thread_manager
                 .validate_environment_selections(environment_selections)
-                .map_err(|err| invalid_request(environment_selection_error_message(err)))?;
+                .map_err(environment_selection_error)?;
         }
         Ok(environment_selections)
     }

codex-rs/core/src/context/environment_context.rs

@@ -49,7 +49,8 @@ impl EnvironmentContextEnvironment {
                 cwd: environment.cwd.clone(),
                 shell: environment
                     .shell
-                    .clone()
+                    .as_ref()
+                    .map(|shell| shell.name().to_string())
                     .unwrap_or_else(|| shell.name().to_string()),
             })
             .collect()

codex-rs/core/src/environment_selection.rs

@@ -9,6 +9,7 @@ use codex_protocol::protocol::TurnEnvironmentSelection;
 use codex_utils_absolute_path::AbsolutePathBuf;
 
 use crate::session::turn_context::TurnEnvironment;
+use crate::shell::Shell;
 
 pub(crate) fn default_thread_environment_selections(
     environment_manager: &EnvironmentManager,
@@ -60,7 +61,7 @@ impl ResolvedTurnEnvironments {
     }
 }
 
-pub(crate) fn resolve_environment_selections(
+pub(crate) async fn resolve_environment_selections(
     environment_manager: &EnvironmentManager,
     environments: &[TurnEnvironmentSelection],
 ) -> CodexResult<ResolvedTurnEnvironments> {
@@ -79,19 +80,34 @@ pub(crate) fn resolve_environment_selections(
             .ok_or_else(|| {
                 CodexErr::InvalidRequest(format!("unknown turn environment id `{environment_id}`"))
             })?;
+        let shell = match environment.info().await {
+            Ok(info) => match Shell::from_environment_shell_info(info.shell) {
+                Ok(shell) => Some(shell),
+                Err(err) => {
+                    tracing::warn!(
+                        "failed to resolve shell for environment `{environment_id}`: {err}"
+                    );
+                    None
+                }
+            },
+            Err(err) => {
+                tracing::warn!("failed to get info for environment `{environment_id}`: {err}");
+                None
+            }
+        };
         turn_environments.push(TurnEnvironment {
             environment_id,
             environment,
             cwd: selected_environment.cwd.clone(),
-            shell: None,
+            shell,
         });
     }
-
     Ok(ResolvedTurnEnvironments { turn_environments })
 }
 
 #[cfg(test)]
 mod tests {
+    use codex_exec_server::Environment;
     use codex_exec_server::ExecServerRuntimePaths;
     use codex_exec_server::LOCAL_ENVIRONMENT_ID;
     use codex_exec_server::REMOTE_ENVIRONMENT_ID;
@@ -189,6 +205,7 @@ url = "ws://127.0.0.1:8765"
                 },
             ],
         )
+        .await
         .expect_err("duplicate environment id should fail");
 
         assert!(err.to_string().contains("duplicate"));
@@ -207,6 +224,7 @@ url = "ws://127.0.0.1:8765"
                 cwd: selected_cwd,
             }],
         )
+        .await
         .expect("environment selections should resolve");
 
         assert_eq!(
@@ -216,7 +234,21 @@ url = "ws://127.0.0.1:8765"
                 .environment_id,
             "local"
         );
-        assert_eq!(resolved.primary().expect("primary environment").shell, None);
+        assert_eq!(
+            resolved.primary().expect("primary environment").shell,
+            Some(
+                Shell::from_environment_shell_info(
+                    manager
+                        .get_environment("local")
+                        .expect("local environment")
+                        .info()
+                        .await
+                        .expect("local environment info")
+                        .shell
+                )
+                .expect("resolved shell")
+            )
+        );
     }
 
     #[tokio::test]
@@ -230,40 +262,31 @@ url = "ws://127.0.0.1:8765"
                 cwd: cwd.clone(),
             }],
         )
+        .await
         .expect("local environment should resolve");
-        let remote_manager = EnvironmentManager::create_for_tests(
-            Some("ws://127.0.0.1:8765".to_string()),
-            Some(test_runtime_paths()),
-        )
-        .await;
-        let remote = resolve_environment_selections(
-            &remote_manager,
-            &[TurnEnvironmentSelection {
+        let remote_environment = Arc::new(
+            Environment::create_for_tests(Some("ws://127.0.0.1:8765".to_string()))
+                .expect("remote environment"),
+        );
+        let remote = ResolvedTurnEnvironments {
+            turn_environments: vec![TurnEnvironment {
                 environment_id: REMOTE_ENVIRONMENT_ID.to_string(),
+                environment: remote_environment.clone(),
                 cwd: cwd.clone(),
+                shell: None,
             }],
-        )
-        .expect("remote environment should resolve");
-        local_manager
-            .upsert_environment(
-                REMOTE_ENVIRONMENT_ID.to_string(),
-                "ws://127.0.0.1:8765".to_string(),
-            )
-            .expect("remote environment should register");
-        let multiple = resolve_environment_selections(
-            &local_manager,
-            &[
-                TurnEnvironmentSelection {
-                    environment_id: LOCAL_ENVIRONMENT_ID.to_string(),
-                    cwd: cwd.clone(),
-                },
-                TurnEnvironmentSelection {
+        };
+        let multiple = ResolvedTurnEnvironments {
+            turn_environments: vec![
+                local.primary().expect("local environment").clone(),
+                TurnEnvironment {
                     environment_id: REMOTE_ENVIRONMENT_ID.to_string(),
+                    environment: remote_environment,
                     cwd: cwd.clone(),
+                    shell: None,
                 },
             ],
-        )
-        .expect("multiple environments should resolve");
+        };
 
         assert_eq!(local.single_local_environment_cwd(), Some(&cwd));
         assert_eq!(remote.single_local_environment_cwd(), None);

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

@@ -444,6 +444,7 @@ async fn warm_plugins_and_skills_for_session_init(
         environment_manager.as_ref(),
         &environments,
     )
+    .await
     .ok()
     .and_then(|resolved| resolved.primary_filesystem());
     let plugins_input = config.plugins_config_input();
@@ -1117,6 +1118,7 @@ impl Session {
                 sess.services.environment_manager.as_ref(),
                 session_configuration.environment_selections(),
             )
+            .await
             .map_err(|err| {
                 CodexErr::InvalidRequest(err.to_string().replace(
                     "unknown turn environment id",

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

@@ -55,7 +55,6 @@ use codex_protocol::permissions::FileSystemSandboxPolicy;
 use codex_protocol::permissions::FileSystemSpecialPath;
 use codex_protocol::protocol::NonSteerableTurnKind;
 use codex_protocol::protocol::SandboxPolicy;
-use codex_protocol::protocol::TurnEnvironmentSelection;
 use codex_protocol::protocol::TurnEnvironmentSelections;
 use codex_protocol::request_permissions::PermissionGrantScope;
 use codex_protocol::request_permissions::RequestPermissionProfile;
@@ -6307,82 +6306,6 @@ async fn empty_turn_environments_clear_primary_environment() {
     assert_eq!(turn_context.config.cwd, session.get_config().await.cwd);
 }
 
-#[tokio::test]
-async fn unknown_turn_environment_returns_error() {
-    let (session, _turn_context, _rx) = make_session_and_context_with_rx().await;
-    let original_configuration = {
-        let state = session.state.lock().await;
-        state.session_configuration.clone()
-    };
-
-    let err = session
-        .new_turn_with_sub_id(
-            "sub-1".to_string(),
-            SessionSettingsUpdate {
-                environments: Some(TurnEnvironmentSelections::new(
-                    original_configuration.cwd().clone(),
-                    vec![TurnEnvironmentSelection {
-                        environment_id: "missing".to_string(),
-                        cwd: original_configuration.cwd().clone(),
-                    }],
-                )),
-                ..Default::default()
-            },
-        )
-        .await
-        .expect_err("unknown environment should fail");
-
-    let current_configuration = {
-        let state = session.state.lock().await;
-        state.session_configuration.clone()
-    };
-    assert!(matches!(err, CodexErr::InvalidRequest(_)));
-    assert!(err.to_string().contains("missing"));
-    assert_eq!(current_configuration.cwd(), original_configuration.cwd());
-    assert_eq!(
-        current_configuration.environment_selections(),
-        original_configuration.environment_selections()
-    );
-}
-
-#[tokio::test]
-async fn duplicate_turn_environment_returns_error_without_mutating_session() {
-    let (session, _turn_context, _rx) = make_session_and_context_with_rx().await;
-    let original_configuration = {
-        let state = session.state.lock().await;
-        state.session_configuration.clone()
-    };
-
-    let err = session
-        .new_turn_with_sub_id(
-            "sub-1".to_string(),
-            SessionSettingsUpdate {
-                environments: Some(TurnEnvironmentSelections::new(
-                    original_configuration.cwd().clone(),
-                    vec![
-                        local(original_configuration.cwd().clone()),
-                        local(original_configuration.cwd().join("second")),
-                    ],
-                )),
-                ..Default::default()
-            },
-        )
-        .await
-        .expect_err("duplicate environment should fail");
-
-    let current_configuration = {
-        let state = session.state.lock().await;
-        state.session_configuration.clone()
-    };
-    assert!(matches!(err, CodexErr::InvalidRequest(_)));
-    assert!(err.to_string().contains("duplicate"));
-    assert_eq!(current_configuration.cwd(), original_configuration.cwd());
-    assert_eq!(
-        current_configuration.environment_selections(),
-        original_configuration.environment_selections()
-    );
-}
-
 #[tokio::test]
 async fn spawn_task_turn_span_inherits_dispatch_trace_context() {
     struct TraceCaptureTask {
@@ -7323,7 +7246,11 @@ async fn environment_context_uses_session_shell_when_environment_shell_is_absent
         .turn_environments
         .first_mut()
         .expect("primary environment");
-    primary_environment.shell = Some("cmd".to_string());
+    primary_environment.shell = Some(crate::shell::Shell {
+        shell_type: crate::shell::ShellType::Cmd,
+        shell_path: PathBuf::from("cmd"),
+        shell_snapshot: crate::shell::empty_shell_snapshot_receiver(),
+    });
 
     let environment_context = crate::context::EnvironmentContext::from_turn_context(
         &turn_context,