Add explicit thread environment selection by starr-openai · Pull Request #18173 · openai/codex

starr-openai · 2026-04-16T18:56:31Z

Summary

add ordered thread/turn environment selection for app-server callers, with required absolute cwd per environment
keep EnvironmentManager owned by ThreadManagerState and resolve protocol params into SessionEnvironment before session state sees them
collapse the prior dual environment carriers so core passes one resolved SessionEnvironment { environment_id, cwd, environment } list through spawn/session/turn state
remove spurious codex.rs rebase churn and keep the modular main layout

Validation

git diff --check
rustfmt run directly on touched Rust files because local cargo is guarded in this shell
Not run: tests/builds for the latest push, per request to update the PR first while review continues

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 346e04c12e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

pakrym-oai

Let's not have a global per-env cwd and make env_manager less hardcoded - imagine we do have a dynamic list of environments but for now populate it from env var/local.

starr-openai · 2026-04-16T23:04:05Z

Superseded by stacked PRs #18225 and #18226.

Adjust the remaining constructor and test callsites for the thread environment plumbing. Co-authored-by: Codex <noreply@openai.com>

pakrym-oai · 2026-04-17T01:21:14Z

-        let environment = self
-            .environment_manager
-            .current()
+        let environment_manager = if environments


This looks surprisingly complicated.

pakrym-oai · 2026-04-17T01:22:01Z

        user_shell_override: None,
        parent_trace: None,
        analytics_events_client: None,
+        environments: vec![codex_protocol::protocol::TurnEnvironment {


Can we keep all tests to None for now?

pakrym-oai · 2026-04-17T01:22:49Z

            config.js_repl_node_path.clone(),
        ),
        environment: Some(Arc::clone(&environment)),
+        environment_manager: Arc::new(codex_exec_server::EnvironmentManager::new(


Why do we need environment manager on session?

pakrym-oai · 2026-04-17T01:25:02Z

+pub struct ThreadEnvironmentParams {
+    pub environment_id: String,
+    #[ts(optional = nullable)]
+    pub cwd: Option<PathBuf>,


AbsolutePathBuf

Not optional

pakrym-oai · 2026-04-17T01:30:01Z

@@ -495,6 +502,11 @@ pub enum Op {
    /// turns that rely on persistent session-level context (for example,
    /// [`Op::UserInput`]).
    OverrideTurnContext {


Why have environment on OverrideTurnContext but not UserTurn?

pakrym-oai · 2026-04-17T01:31:10Z

 #[derive(Debug)]
 pub struct EnvironmentManager {
-    exec_server_url: Option<String>,
+    current_environment_id: Option<EnvironmentId>,


I think environment manager should stay shared and not have a current id. Just the list of available environments. Then each thread get it's own list of Environment structs

pakrym-oai · 2026-04-17T01:32:54Z

        local_runtime_paths: Option<ExecServerRuntimePaths>,
    ) -> Result<Self, ExecServerError> {
        let (exec_server_url, disabled) = normalize_exec_server_url(exec_server_url);
+        let id = if disabled {


This doesn't look right. Env manager should have either:
1 env when REMOTE_ENV_ENV_VAR=none or not set.
2 envs local and remote when REMOTE_ENV_ENV_VAR is set.

pakrym-oai · 2026-04-17T01:34:15Z

    }
 }
+
+fn normalize_environment_id(environment_id: Option<&str>) -> Option<EnvironmentId> {


pakrym-oai

I don't think we should mutate and clone EnvironmentManager per turn. There is a global environment manager that contains a list of all environments registered with app server when a turn is started we create/attach a set of environments to a specific turn.

Add thread/start environment_id plumbing and explicit local/remote selection support while preserving the current default environment behavior. Include focused exec-server and app-server coverage for the new thread environment selection paths. Co-authored-by: Codex <noreply@openai.com>

Co-authored-by: Codex <noreply@openai.com>

Use only the first id for now and fall back to the legacy environmentId field when the list is absent or empty. Co-authored-by: Codex <noreply@openai.com>

Keep current behavior by collapsing to the first id at the app-server boundary for now, and leave a TODO to plumb the full list downstream in a follow-up PR. Co-authored-by: Codex <noreply@openai.com>

Co-authored-by: Codex <noreply@openai.com>

…openai.com>

Co-authored-by: Codex <noreply@openai.com>

Adjust the remaining constructor and test callsites for the thread environment plumbing. Co-authored-by: Codex <noreply@openai.com>

Co-authored-by: Codex <noreply@openai.com>

Keep EnvironmentManager owned by ThreadManagerState as the shared environment registry, and pass resolved per-thread environments into sessions as an attached list. Preserve the existing first-environment runtime behavior while carrying the selected environment descriptors for inheritance and future multi-environment support. Co-authored-by: Codex <noreply@openai.com>

Co-authored-by: Codex <noreply@openai.com>

chatgpt-codex-connector Bot reviewed Apr 16, 2026

View reviewed changes

Comment thread codex-rs/core/src/thread_manager.rs Outdated

Comment thread codex-rs/core/src/thread_manager.rs Outdated

starr-openai force-pushed the starr/thread-env-id-minimal branch from 7555d6b to 404d2f3 Compare April 16, 2026 20:14