Separate environment requests from runtime selections (#51221)

## What changed

Introduce `TurnEnvironmentRequest` for caller-supplied environment input and construct `TurnEnvironmentSelection` at session startup and app-server settings boundaries. Update thread startup, default environments, and inherited environment handoffs to use requests, with explicit conversions when reusing runtime selections.

Move the environment types into `protocol::environment` while retaining their `protocol` re-exports. Update test fixtures and existing tests to build request inputs separately from runtime selections.

GitOrigin-RevId: 12e7daaece95aae136afae8b055b4dbaebdffc75
This commit is contained in:
Adam Perry @ OpenAI
2026-10-06 01:39:50 +00:00
committed by copyberry
parent 28b91c7c31
commit 2dbcab90e2
46 changed files with 373 additions and 212 deletions
+1 -1
View File
@@ -1722,7 +1722,7 @@ async fn pending_environment_failure_reaches_child_and_grandchild() {
let root = harness
.manager
.start_thread(StartThreadOptions {
environments: Some(vec![pending.clone()]),
environments: Some(vec![pending.clone().into_request()]),
..StartThreadOptions::new(harness.config.clone())
})
.await
+6 -1
View File
@@ -11,6 +11,7 @@ use codex_protocol::protocol::Op;
use codex_protocol::protocol::SessionSource;
use codex_protocol::protocol::SubAgentSource;
use codex_protocol::protocol::ThreadSource;
use codex_protocol::protocol::TurnEnvironmentSelection;
use codex_protocol::user_input::UserInput;
use serde_json::Value;
use std::time::Duration;
@@ -135,7 +136,11 @@ 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_environments.to_selections(),
environment_requests: parent_environments
.to_selections()
.into_iter()
.map(TurnEnvironmentSelection::into_request)
.collect(),
thread_extension_init,
turn_extension_init: Default::default(),
client_mcp_extensions: parent_session.services.client_mcp_extensions.clone(),
+2 -2
View File
@@ -713,7 +713,7 @@ impl CodexThread {
fn thread_settings_update(overrides: CodexThreadSettingsOverrides) -> SessionSettingsUpdate {
let CodexThreadSettingsOverrides {
turn_extension_init,
environments,
environments: environment_requests,
runtime_workspace_roots,
profile_workspace_roots,
approval_policy,
@@ -742,7 +742,7 @@ impl CodexThread {
approval_policy,
approvals_reviewer,
},
environments,
environments: environment_requests,
runtime_workspace_roots,
profile_workspace_roots,
sandbox_policy,
+24 -26
View File
@@ -26,6 +26,7 @@ use codex_protocol::protocol::EnvironmentConfigState;
use codex_protocol::protocol::EnvironmentConnectionEvent;
use codex_protocol::protocol::Event;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::TurnEnvironmentRequest;
use codex_protocol::protocol::TurnEnvironmentSelection;
use codex_skills_extension::EnvironmentSkillRequirements;
use codex_utils_absolute_path::AbsolutePathBuf;
@@ -122,15 +123,15 @@ pub(crate) fn combine_selected_capability_roots(
combined_roots
}
pub(crate) fn default_thread_environment_selections(
pub(crate) fn default_thread_environment_requests(
environment_manager: &EnvironmentManager,
cwd: &AbsolutePathBuf,
workspace_roots: &[AbsolutePathBuf],
) -> Vec<TurnEnvironmentSelection> {
) -> Vec<TurnEnvironmentRequest> {
environment_manager
.default_environment_ids()
.into_iter()
.map(|environment_id| TurnEnvironmentSelection {
.map(|environment_id| TurnEnvironmentRequest {
environment_id,
cwd: PathUri::from_abs_path(cwd),
workspace_roots: workspace_roots.iter().map(PathUri::from_abs_path).collect(),
@@ -140,30 +141,26 @@ pub(crate) fn default_thread_environment_selections(
}
/// Checks that environment IDs are registered and unique and that working directories are not too long.
pub fn validate_environment_ids_and_cwds(
pub fn validate_environment_ids_and_cwds<'a>(
environment_manager: &EnvironmentManager,
environments: &[TurnEnvironmentSelection],
environments: impl IntoIterator<Item = (&'a str, &'a PathUri)>,
) -> CodexResult<()> {
let mut environment_ids = HashSet::with_capacity(environments.len());
for environment in environments {
if environment.cwd.inferred_native_path_string().len() > MAX_TURN_ENVIRONMENT_CWD_BYTES {
let mut environment_ids = HashSet::new();
for (environment_id, cwd) in environments {
if cwd.inferred_native_path_string().len() > MAX_TURN_ENVIRONMENT_CWD_BYTES {
return Err(CodexErr::InvalidRequest(
"turn environment working directory exceeds the maximum size".to_string(),
));
}
if !environment_ids.insert(environment.environment_id.as_str()) {
if !environment_ids.insert(environment_id) {
return Err(CodexErr::InvalidRequest(format!(
"duplicate turn environment id `{}`",
environment.environment_id
"duplicate turn environment id `{environment_id}`"
)));
}
environment_manager
.get_environment(&environment.environment_id)
.get_environment(environment_id)
.ok_or_else(|| {
CodexErr::InvalidRequest(format!(
"unknown turn environment id `{}`",
environment.environment_id
))
CodexErr::InvalidRequest(format!("unknown turn environment id `{environment_id}`"))
})?;
}
Ok(())
@@ -1321,6 +1318,7 @@ mod tests {
use codex_protocol::models::ActivePermissionProfile;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemSandboxPolicyContext;
use codex_protocol::protocol::TurnEnvironmentRequest;
use codex_protocol::protocol::TurnEnvironmentSelection;
use codex_protocol::sandbox::SandboxType;
use codex_utils_absolute_path::AbsolutePathBuf;
@@ -1444,7 +1442,7 @@ mod tests {
}
#[tokio::test]
async fn default_thread_environment_selections_use_manager_default_id() {
async fn default_thread_environment_requests_use_manager_default_id() {
let cwd = AbsolutePathBuf::current_dir().expect("cwd");
let cwd_uri = PathUri::from_abs_path(&cwd);
let manager = EnvironmentManager::create_for_tests(
@@ -1454,8 +1452,8 @@ mod tests {
.await;
assert_eq!(
default_thread_environment_selections(&manager, &cwd, std::slice::from_ref(&cwd)),
vec![TurnEnvironmentSelection {
default_thread_environment_requests(&manager, &cwd, std::slice::from_ref(&cwd)),
vec![TurnEnvironmentRequest {
environment_id: REMOTE_ENVIRONMENT_ID.to_string(),
cwd: cwd_uri.clone(),
workspace_roots: vec![cwd_uri],
@@ -1465,7 +1463,7 @@ mod tests {
}
#[tokio::test]
async fn toml_default_thread_environment_selections_include_local_and_remote() {
async fn toml_default_thread_environment_requests_include_local_and_remote() {
let temp_dir = tempfile::tempdir().expect("tempdir");
std::fs::write(
temp_dir.path().join("environments.toml"),
@@ -1487,15 +1485,15 @@ url = "ws://127.0.0.1:8765"
.expect("environment manager");
assert_eq!(
default_thread_environment_selections(&manager, &cwd, std::slice::from_ref(&cwd)),
default_thread_environment_requests(&manager, &cwd, std::slice::from_ref(&cwd)),
vec![
TurnEnvironmentSelection {
TurnEnvironmentRequest {
environment_id: LOCAL_ENVIRONMENT_ID.to_string(),
cwd: cwd_uri.clone(),
workspace_roots: vec![cwd_uri.clone()],
config: EnvironmentConfigState::FromThread,
},
TurnEnvironmentSelection {
TurnEnvironmentRequest {
environment_id: REMOTE_ENVIRONMENT_ID.to_string(),
cwd: cwd_uri.clone(),
workspace_roots: vec![cwd_uri],
@@ -1506,13 +1504,13 @@ url = "ws://127.0.0.1:8765"
}
#[tokio::test]
async fn default_thread_environment_selections_empty_when_default_disabled() {
async fn default_thread_environment_requests_empty_when_default_disabled() {
let cwd = AbsolutePathBuf::current_dir().expect("cwd");
let manager = environment_manager_without_environments();
assert_eq!(
default_thread_environment_selections(&manager, &cwd, std::slice::from_ref(&cwd)),
Vec::<TurnEnvironmentSelection>::new()
default_thread_environment_requests(&manager, &cwd, std::slice::from_ref(&cwd)),
Vec::<TurnEnvironmentRequest>::new()
);
}
@@ -8,6 +8,7 @@ use super::*;
use crate::guardian::input_budget::CheckpointRecovery;
use codex_guardian_reviewer::ReviewerPool;
use codex_guardian_reviewer::ReviewerRequest;
use codex_protocol::protocol::TurnEnvironmentSelection;
/// Controls whether selection may reuse a session or must start from the parent checkpoint.
#[derive(Clone, Copy, PartialEq, Eq)]
@@ -148,7 +149,14 @@ impl PreparedGuardianContext {
}),
}),
initial_history: initial_history.unwrap_or(InitialHistory::New),
environments: Some(self.context.environments().to_selections()),
environments: Some(
self.context
.environments()
.to_selections()
.into_iter()
.map(TurnEnvironmentSelection::into_request)
.collect(),
),
inherited_environments: Some(self.context.environments().clone()),
client_mcp_extensions: self.parent.services.client_mcp_extensions.clone(),
..crate::StartThreadOptions::new(config)
+7 -2
View File
@@ -155,6 +155,7 @@ use codex_protocol::protocol::ThreadSource;
use codex_protocol::protocol::TurnAbortReason;
use codex_protocol::protocol::TurnContextItem;
use codex_protocol::protocol::TurnContextNetworkItem;
use codex_protocol::protocol::TurnEnvironmentRequest;
use codex_protocol::protocol::TurnEnvironmentSelection;
use codex_protocol::protocol::TurnStartedEvent;
use codex_protocol::protocol::W3cTraceContext;
@@ -474,7 +475,7 @@ pub(crate) struct SessionSpawnArgs {
pub(crate) parent_rollout_thread_trace: ThreadTraceContext,
pub(crate) user_shell_override: Option<shell::Shell>,
pub(crate) parent_trace: Option<W3cTraceContext>,
pub(crate) environment_selections: Vec<TurnEnvironmentSelection>,
pub(crate) environment_requests: Vec<TurnEnvironmentRequest>,
pub(crate) thread_extension_init: ExtensionDataInit,
pub(crate) turn_extension_init: ExtensionDataInit,
pub(crate) client_mcp_extensions: ClientMcpExtensions,
@@ -583,7 +584,7 @@ impl Session {
inherited_environments,
parent_rollout_thread_trace,
parent_trace: _,
environment_selections,
environment_requests,
thread_extension_init,
turn_extension_init,
client_mcp_extensions,
@@ -604,6 +605,10 @@ impl Session {
.get::<codex_extension_api::SessionIsolation>()
.map(|policy| *policy)
.unwrap_or_default();
let environment_selections = environment_requests
.into_iter()
.map(TurnEnvironmentSelection::new)
.collect::<Vec<_>>();
// Enforce snapshot-only instructions for both managed and inline isolated sessions.
let instructions = if isolation == codex_extension_api::SessionIsolation::Isolated {
SessionInstructions {
+14 -7
View File
@@ -266,7 +266,7 @@ impl Session {
};
let TurnSettingsUpdate {
approvals_reviewer,
environments,
environments: environment_requests,
model,
effort,
summary,
@@ -286,7 +286,9 @@ impl Session {
// progress, finish, or be cancelled while this awaits; we don't hold the update locks here.
let prepared = if updates_step_settings {
let current_environments = self.services.turn_environments.selections();
let proposed = environments.as_deref().unwrap_or(&current_environments);
let proposed = environment_requests
.as_deref()
.unwrap_or(&current_environments);
self.prepare_step_settings_activation(&turn_context, &current, &update, proposed)
.await
.map(Some)
@@ -295,8 +297,9 @@ impl Session {
};
// Validate the environment configuration supplied in the update.
let environment_config_validation =
environments.as_deref().map(validate_environment_configs);
let environment_config_validation = environment_requests
.as_deref()
.map(validate_environment_configs);
// Confirm the task we originally targeted is still running.
let active = self.active_turn.lock().await;
@@ -326,12 +329,16 @@ impl Session {
// Environment configuration can arrive before its executor connects. If this update
// omits environments, use what the running turn's manager knows now.
let current_environments = self.services.turn_environments.selections();
let proposed = environments.as_deref().unwrap_or(&current_environments);
let proposed = environment_requests
.as_deref()
.unwrap_or(&current_environments);
let validation = (|| {
if let Some(configs) = environment_config_validation {
validate_environment_ids_and_cwds(
&self.services.turn_environments.environment_manager(),
proposed,
proposed
.iter()
.map(|selection| (selection.environment_id.as_str(), &selection.cwd)),
)
.map_err(|error| error.to_string())?;
ensure_configs_stay_owner_provided(&current_environments, proposed)
@@ -367,7 +374,7 @@ impl Session {
.next_step_settings
.store(Arc::new(settings));
}
if environments.is_some() {
if environment_requests.is_some() {
self.services.turn_environments.update_selections(proposed);
}
TurnSettingsUpdateOutcome::Applied
@@ -1321,7 +1321,7 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() {
parent_rollout_thread_trace: codex_rollout_trace::ThreadTraceContext::disabled(),
user_shell_override: None,
parent_trace: None,
environment_selections: Vec::new(),
environment_requests: Vec::new(),
thread_extension_init,
turn_extension_init: Default::default(),
client_mcp_extensions: ClientMcpExtensions::default(),
+2 -2
View File
@@ -50,7 +50,7 @@ pub(super) fn prepare_update(
turn_extension_init,
} = overrides.into();
let ThreadSettingsOverrides {
environments,
environments: environment_requests,
runtime_workspace_roots,
profile_workspace_roots,
approval_policy,
@@ -79,7 +79,7 @@ pub(super) fn prepare_update(
approval_policy,
approvals_reviewer,
},
environments,
environments: environment_requests,
runtime_workspace_roots,
profile_workspace_roots,
sandbox_policy,
+35 -15
View File
@@ -16,7 +16,7 @@ use crate::config::Config;
use crate::config::ThreadStoreConfig;
use crate::current_time::TimeProvider;
use crate::environment_selection::TurnEnvironmentSnapshot;
use crate::environment_selection::default_thread_environment_selections;
use crate::environment_selection::default_thread_environment_requests;
use crate::mcp::McpManager;
use crate::rollout::truncation;
use crate::session::ForkPersistence;
@@ -82,6 +82,7 @@ use codex_protocol::protocol::ThreadHistoryMode;
use codex_protocol::protocol::ThreadSource;
use codex_protocol::protocol::TurnAbortReason;
use codex_protocol::protocol::TurnAbortedEvent;
use codex_protocol::protocol::TurnEnvironmentRequest;
use codex_protocol::protocol::TurnEnvironmentSelection;
use codex_protocol::protocol::W3cTraceContext;
use codex_rollout::state_db::StateDbHandle;
@@ -282,7 +283,7 @@ pub struct StartThreadOptions {
pub dynamic_tools: Vec<codex_protocol::dynamic_tools::DynamicToolSpec>,
pub metrics_service_name: Option<String>,
pub parent_trace: Option<W3cTraceContext>,
pub environments: Option<Vec<TurnEnvironmentSelection>>,
pub environments: Option<Vec<TurnEnvironmentRequest>>,
/// Existing environment bindings captured by an internal caller.
pub inherited_environments: Option<TurnEnvironmentSnapshot>,
/// Explicit global instructions carried by an internal caller instead of loading them again.
@@ -874,12 +875,12 @@ impl ThreadManager {
});
}
pub fn default_environment_selections(
pub fn default_environment_requests(
&self,
cwd: &AbsolutePathBuf,
workspace_roots: &[AbsolutePathBuf],
) -> Vec<TurnEnvironmentSelection> {
default_thread_environment_selections(
) -> Vec<TurnEnvironmentRequest> {
default_thread_environment_requests(
self.state.environment_manager.as_ref(),
cwd,
workspace_roots,
@@ -1946,7 +1947,12 @@ impl ThreadManagerState {
session_source: Some(session_source),
thread_source,
metrics_service_name,
environments,
environments: environments.map(|selections| {
selections
.into_iter()
.map(TurnEnvironmentSelection::into_request)
.collect()
}),
client_mcp_extensions,
dynamic_tools,
..StartThreadOptions::new(config)
@@ -1985,7 +1991,12 @@ impl ThreadManagerState {
initial_history,
session_source: Some(session_source),
thread_source,
environments: environment_selections,
environments: environment_selections.map(|selections| {
selections
.into_iter()
.map(TurnEnvironmentSelection::into_request)
.collect()
}),
client_mcp_extensions,
..StartThreadOptions::new(config)
};
@@ -2020,7 +2031,12 @@ impl ThreadManagerState {
history_mode,
session_source: Some(session_source),
thread_source,
environments,
environments: environments.map(|selections| {
selections
.into_iter()
.map(TurnEnvironmentSelection::into_request)
.collect()
}),
thread_extension_init,
client_mcp_extensions,
..StartThreadOptions::new(config)
@@ -2087,7 +2103,7 @@ impl ThreadManagerState {
dynamic_tools,
metrics_service_name,
parent_trace,
environments,
environments: environment_requests,
inherited_environments: captured_environments,
user_instructions: supplied_user_instructions,
mut thread_extension_init,
@@ -2111,18 +2127,22 @@ impl ThreadManagerState {
}
});
thread_extension_init.insert(isolation);
let environments = environments
let environment_requests = environment_requests
.or_else(|| {
let SessionSource::SubAgent(SubAgentSource::ThreadSpawn { .. }) = &session_source
else {
return None;
};
inherited_environments
.as_ref()
.map(TurnEnvironmentSnapshot::inheritable_selections)
inherited_environments.as_ref().map(|snapshot| {
snapshot
.inheritable_selections()
.into_iter()
.map(TurnEnvironmentSelection::into_request)
.collect()
})
})
.unwrap_or_else(|| {
default_thread_environment_selections(
default_thread_environment_requests(
self.environment_manager.as_ref(),
&config.cwd,
&config.workspace_roots,
@@ -2334,7 +2354,7 @@ impl ThreadManagerState {
parent_rollout_thread_trace,
user_shell_override,
parent_trace,
environment_selections: environments,
environment_requests,
thread_extension_init,
turn_extension_init,
client_mcp_extensions,
+13 -2
View File
@@ -1387,7 +1387,12 @@ async fn spawn_internal_session_preserves_parent_lineage_without_forking_history
initial_history: InitialHistory::Forked(vec![RolloutItem::ResponseItem(
user_msg("parent history must not be inherited").into(),
)]),
environments: Some(reviewer_environments),
environments: Some(
reviewer_environments
.into_iter()
.map(TurnEnvironmentSelection::into_request)
.collect(),
),
..StartThreadOptions::new(config)
},
)
@@ -1990,7 +1995,13 @@ async fn resume_and_fork_do_not_restore_thread_environments_from_rollout() {
let source = manager
.start_thread(StartThreadOptions {
history_mode: Some(ThreadHistoryMode::Legacy),
environments: Some(environments.clone()),
environments: Some(
environments
.clone()
.into_iter()
.map(TurnEnvironmentSelection::into_request)
.collect(),
),
..StartThreadOptions::new(source_config)
})
.await