[codex] retain resolved environments across turns#27955
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53814c2004
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
f00d3ed to
b9f12dc
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e19fb2d6c0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @@ -108,7 +111,7 @@ pub(crate) async fn run_codex_thread_interactive( | |||
| inherited_exec_policy: Some(Arc::clone(&parent_session.services.exec_policy)), | |||
| parent_rollout_thread_trace: codex_rollout_trace::ThreadTraceContext::disabled(), | |||
| parent_trace: None, | |||
| environment_selections: parent_ctx.environments.clone(), | |||
| environment_selections: parent_ctx.environments.to_selections(), | |||
There was a problem hiding this comment.
Preserve parent environment handles for child spawn
Converting the parent's resolved environment snapshot back to ID/cwd selections makes the child re-resolve in Codex::spawn. If that remote ID was upserted after the turn began, the child can load AGENTS/skills and run in a different environment than the active parent context. guidance
Useful? React with 👍 / 👎.
e19fb2d to
f4c74a4
Compare
| warn!("failed to resolve turn environments: {err}"); | ||
| ResolvedTurnEnvironments::default() | ||
| }); | ||
| let turn_environments = self.services.turn_environments.snapshot().await; |
There was a problem hiding this comment.
should we have an integraiton test confirming the same env is used across two UserInputs?
or is that too heavy for this stage
| if let Some(environments) = &updates.environments { | ||
| self.services | ||
| .turn_environments | ||
| .update_selections(&environments.environments) | ||
| .await; |
There was a problem hiding this comment.
[codex] this makes a window where SessionConfiguration is stored before this await, so another turn build during env resolution could combine new settings with prev environment (run against wrong cwd/host)
Why
Selected execution environments are thread-scoped resources, but startup and turn construction repeatedly resolved their IDs and working directories. That discarded existing environment handles and shell metadata even when a selection had not changed.
Session configuration updates also need to affect future turns without changing the resolved environment set already captured by a running turn.
What changed
ThreadEnvironmentsservice insideCodexfrom the spawnedEnvironmentManagerand raw environment selections, then store it onSessionServices.update_selections, allowing session configuration updates to mutate the resolved set in place.TurnEnvironmentwhen its environment ID and working directory match; resolve only added or changed selections and remove selections that are no longer present.TurnContexta clonedTurnEnvironmentSnapshot, so later session configuration updates affect future turns without rewriting an active turn.Test plan
cargo check -p codex-core --testsjust test -p codex-core environment_selectionjust test -p codex-core turn_environmentsjust test -p codex-core session_update_settings_does_not_rewrite_sticky_environment_cwdsjust test -p codex-core default_turn_does_not_overlay_legacy_fallback_cwd_onto_stored_thread_environments