mirror of
https://github.com/openai/codex.git
synced 2026-10-29 19:08:43 +00:00
Use selected turn environments for runtime context
Route session runtime cwd and MCP startup state through selected turn environments so multi-environment turns avoid a parallel legacy Environment field. Keep fallback behavior for local/no-selection sessions and add focused coverage for duplicate selections, cwd resolution, and MCP runtime environment selection. Validation: git diff --check Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -1,3 +1,4 @@
|
||||
use std::collections::HashSet;
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_exec_server::Environment;
|
||||
@@ -25,7 +26,14 @@ pub(crate) fn validate_environment_selections(
|
||||
environment_manager: &EnvironmentManager,
|
||||
environments: &[TurnEnvironmentSelection],
|
||||
) -> CodexResult<()> {
|
||||
let mut seen_environment_ids = HashSet::with_capacity(environments.len());
|
||||
for selected_environment in environments {
|
||||
if !seen_environment_ids.insert(selected_environment.environment_id.as_str()) {
|
||||
return Err(CodexErr::InvalidRequest(format!(
|
||||
"duplicate turn environment id `{}`",
|
||||
selected_environment.environment_id
|
||||
)));
|
||||
}
|
||||
if environment_manager
|
||||
.get_environment(&selected_environment.environment_id)
|
||||
.is_none()
|
||||
@@ -59,6 +67,16 @@ pub(crate) fn selected_primary_environment(
|
||||
.transpose()
|
||||
}
|
||||
|
||||
pub(crate) fn primary_selected_cwd_or_fallback(
|
||||
environments: &[TurnEnvironmentSelection],
|
||||
fallback_cwd: &AbsolutePathBuf,
|
||||
) -> AbsolutePathBuf {
|
||||
environments
|
||||
.first()
|
||||
.map(|selected_environment| selected_environment.cwd.clone())
|
||||
.unwrap_or_else(|| fallback_cwd.clone())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use codex_exec_server::ExecServerRuntimePaths;
|
||||
@@ -105,4 +123,51 @@ mod tests {
|
||||
Vec::<TurnEnvironmentSelection>::new()
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn validate_environment_selections_rejects_duplicate_ids() {
|
||||
let cwd = AbsolutePathBuf::current_dir().expect("cwd");
|
||||
let manager = EnvironmentManager::default_for_tests();
|
||||
|
||||
let err = validate_environment_selections(
|
||||
&manager,
|
||||
&[
|
||||
TurnEnvironmentSelection {
|
||||
environment_id: "local".to_string(),
|
||||
cwd: cwd.clone(),
|
||||
},
|
||||
TurnEnvironmentSelection {
|
||||
environment_id: "local".to_string(),
|
||||
cwd: cwd.join("other"),
|
||||
},
|
||||
],
|
||||
)
|
||||
.expect_err("duplicate environment id should fail");
|
||||
|
||||
assert!(err.to_string().contains("duplicate"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn primary_selected_cwd_or_fallback_uses_first_selection() {
|
||||
let cwd = AbsolutePathBuf::current_dir().expect("cwd");
|
||||
let selected_cwd = cwd.join("selected");
|
||||
|
||||
assert_eq!(
|
||||
primary_selected_cwd_or_fallback(
|
||||
&[TurnEnvironmentSelection {
|
||||
environment_id: "local".to_string(),
|
||||
cwd: selected_cwd.clone(),
|
||||
}],
|
||||
&cwd,
|
||||
),
|
||||
selected_cwd
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn primary_selected_cwd_or_fallback_uses_fallback_without_selections() {
|
||||
let cwd = AbsolutePathBuf::current_dir().expect("cwd");
|
||||
|
||||
assert_eq!(primary_selected_cwd_or_fallback(&[], &cwd), cwd);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,6 +1,51 @@
|
||||
use super::*;
|
||||
|
||||
impl Session {
|
||||
pub(crate) fn mcp_runtime_environment_for_turn_context(
|
||||
&self,
|
||||
turn_context: &TurnContext,
|
||||
) -> McpRuntimeEnvironment {
|
||||
if let Some(turn_environment) = turn_context.primary_environment() {
|
||||
return McpRuntimeEnvironment::new(
|
||||
Arc::clone(&turn_environment.environment),
|
||||
turn_environment.cwd.to_path_buf(),
|
||||
);
|
||||
}
|
||||
|
||||
McpRuntimeEnvironment::new(
|
||||
self.services.environment_manager.local_environment(),
|
||||
turn_context.cwd.to_path_buf(),
|
||||
)
|
||||
}
|
||||
|
||||
pub(crate) fn mcp_runtime_environment_for_configuration(
|
||||
&self,
|
||||
session_configuration: &SessionConfiguration,
|
||||
) -> McpRuntimeEnvironment {
|
||||
if let Some(selected_environment) = session_configuration.environments.first() {
|
||||
if let Some(environment) = self
|
||||
.services
|
||||
.environment_manager
|
||||
.get_environment(&selected_environment.environment_id)
|
||||
{
|
||||
return McpRuntimeEnvironment::new(
|
||||
environment,
|
||||
selected_environment.cwd.to_path_buf(),
|
||||
);
|
||||
}
|
||||
|
||||
warn!(
|
||||
"unknown stored MCP environment id `{}`; falling back to local environment",
|
||||
selected_environment.environment_id
|
||||
);
|
||||
}
|
||||
|
||||
McpRuntimeEnvironment::new(
|
||||
self.services.environment_manager.local_environment(),
|
||||
session_configuration.cwd.to_path_buf(),
|
||||
)
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::await_holding_invalid_type,
|
||||
reason = "active turn checks and turn state updates must remain atomic"
|
||||
@@ -234,13 +279,7 @@ impl Session {
|
||||
turn_context.sub_id.clone(),
|
||||
self.get_tx_event(),
|
||||
turn_context.permission_profile(),
|
||||
McpRuntimeEnvironment::new(
|
||||
turn_context
|
||||
.environment
|
||||
.clone()
|
||||
.unwrap_or_else(|| self.services.environment_manager.local_environment()),
|
||||
turn_context.cwd.to_path_buf(),
|
||||
),
|
||||
self.mcp_runtime_environment_for_turn_context(turn_context),
|
||||
config.codex_home.to_path_buf(),
|
||||
codex_apps_tools_cache_key(auth.as_ref()),
|
||||
tool_plugin_provenance,
|
||||
|
||||
@@ -30,6 +30,7 @@ use crate::context::NetworkRuleSaved;
|
||||
use crate::context::PermissionsInstructions;
|
||||
use crate::context::PersonalitySpecInstructions;
|
||||
use crate::default_skill_metadata_budget;
|
||||
use crate::environment_selection::primary_selected_cwd_or_fallback;
|
||||
use crate::environment_selection::selected_primary_environment;
|
||||
use crate::environment_selection::validate_environment_selections;
|
||||
use crate::exec_policy::ExecPolicyManager;
|
||||
@@ -472,13 +473,15 @@ impl Codex {
|
||||
validate_environment_selections(environment_manager.as_ref(), &environments)?;
|
||||
let environment =
|
||||
selected_primary_environment(environment_manager.as_ref(), &environments)?;
|
||||
let mut load_config = config.clone();
|
||||
load_config.cwd = primary_selected_cwd_or_fallback(&environments, &config.cwd);
|
||||
let fs = environment
|
||||
.as_ref()
|
||||
.map(|environment| environment.get_filesystem());
|
||||
let plugins_input = config.plugins_config_input();
|
||||
let plugins_input = load_config.plugins_config_input();
|
||||
let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await;
|
||||
let effective_skill_roots = plugin_outcome.effective_skill_roots();
|
||||
let skills_input = skills_load_input_from_config(&config, effective_skill_roots);
|
||||
let skills_input = skills_load_input_from_config(&load_config, effective_skill_roots);
|
||||
let loaded_skills = skills_manager.skills_for_config(&skills_input, fs).await;
|
||||
|
||||
for err in &loaded_skills.errors {
|
||||
@@ -497,7 +500,7 @@ impl Codex {
|
||||
let _ = config.features.disable(Feature::Collab);
|
||||
}
|
||||
|
||||
let user_instructions = AgentsMdManager::new(&config)
|
||||
let user_instructions = AgentsMdManager::new(&load_config)
|
||||
.user_instructions(environment.as_deref())
|
||||
.await;
|
||||
|
||||
|
||||
@@ -123,7 +123,6 @@ pub(super) async fn spawn_review_thread(
|
||||
reasoning_effort,
|
||||
reasoning_summary,
|
||||
session_source,
|
||||
environment: parent_turn_context.environment.clone(),
|
||||
environments: parent_turn_context.environments.clone(),
|
||||
tools_config,
|
||||
features: parent_turn_context.features.clone(),
|
||||
|
||||
@@ -195,13 +195,11 @@ impl SessionConfiguration {
|
||||
.cwd
|
||||
.as_ref()
|
||||
.map(|cwd| {
|
||||
AbsolutePathBuf::relative_to_current_dir(normalize_for_native_workdir(
|
||||
cwd.as_path(),
|
||||
))
|
||||
.unwrap_or_else(|e| {
|
||||
warn!("failed to normalize update cwd: {cwd:?}: {e}");
|
||||
self.cwd.clone()
|
||||
})
|
||||
crate::environment_selection::primary_selected_cwd_or_fallback(
|
||||
&self.environments,
|
||||
&self.cwd,
|
||||
)
|
||||
.join(normalize_for_native_workdir(cwd.as_path()))
|
||||
})
|
||||
.unwrap_or_else(|| self.cwd.clone());
|
||||
|
||||
@@ -928,13 +926,7 @@ impl Session {
|
||||
INITIAL_SUBMIT_ID.to_owned(),
|
||||
tx_event.clone(),
|
||||
session_configuration.permission_profile(),
|
||||
McpRuntimeEnvironment::new(
|
||||
sess.services
|
||||
.environment_manager
|
||||
.default_environment()
|
||||
.unwrap_or_else(|| sess.services.environment_manager.local_environment()),
|
||||
session_configuration.cwd.to_path_buf(),
|
||||
),
|
||||
sess.mcp_runtime_environment_for_configuration(&session_configuration),
|
||||
config.codex_home.to_path_buf(),
|
||||
codex_apps_tools_cache_key(auth),
|
||||
tool_plugin_provenance,
|
||||
|
||||
@@ -3305,6 +3305,30 @@ async fn session_update_settings_keeps_runtime_cwds_absolute() {
|
||||
assert_eq!(next_turn.config.cwd, updated_cwd);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn relative_cwd_update_without_environments_resolves_under_session_cwd() {
|
||||
let (session, _turn_context) = make_session_and_context().await;
|
||||
let original_cwd = {
|
||||
let mut state = session.state.lock().await;
|
||||
state.session_configuration.environments = Vec::new();
|
||||
state.session_configuration.cwd.clone()
|
||||
};
|
||||
let updated_cwd = original_cwd.join("project");
|
||||
std::fs::create_dir_all(updated_cwd.as_path()).expect("create project dir");
|
||||
|
||||
session
|
||||
.update_settings(SessionSettingsUpdate {
|
||||
cwd: Some(PathBuf::from("project")),
|
||||
..Default::default()
|
||||
})
|
||||
.await
|
||||
.expect("cwd update should succeed");
|
||||
|
||||
let state = session.state.lock().await;
|
||||
assert_eq!(state.session_configuration.cwd, updated_cwd);
|
||||
assert!(state.session_configuration.environments.is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn session_new_fails_when_zsh_fork_enabled_without_zsh_path() {
|
||||
let codex_home = tempfile::tempdir().expect("create temp dir");
|
||||
@@ -3584,7 +3608,6 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
|
||||
model_info,
|
||||
&models_manager,
|
||||
/*network*/ None,
|
||||
Some(environment),
|
||||
turn_environments,
|
||||
session_configuration.cwd.clone(),
|
||||
"turn_id".to_string(),
|
||||
@@ -4323,13 +4346,14 @@ async fn turn_environments_set_primary_environment() {
|
||||
|
||||
let turn_environments = &turn_context.environments;
|
||||
assert_eq!(turn_environments.len(), 1);
|
||||
let primary_environment = turn_context
|
||||
.primary_environment()
|
||||
.expect("primary environment should be set");
|
||||
assert!(std::sync::Arc::ptr_eq(
|
||||
turn_context
|
||||
.environment
|
||||
.as_ref()
|
||||
.expect("primary environment should be set"),
|
||||
&primary_environment.environment,
|
||||
&turn_environments[0].environment
|
||||
));
|
||||
assert!(!turn_context.environments.is_empty());
|
||||
assert_eq!(turn_context.cwd.as_path(), selected_cwd.as_path());
|
||||
assert_eq!(turn_context.config.cwd.as_path(), selected_cwd.as_path());
|
||||
}
|
||||
@@ -4353,11 +4377,11 @@ async fn default_turn_uses_stored_thread_environments() {
|
||||
|
||||
let turn_environments = &turn_context.environments;
|
||||
assert_eq!(turn_environments.len(), 1);
|
||||
let primary_environment = turn_context
|
||||
.primary_environment()
|
||||
.expect("primary environment should be set");
|
||||
assert!(std::sync::Arc::ptr_eq(
|
||||
turn_context
|
||||
.environment
|
||||
.as_ref()
|
||||
.expect("primary environment should be set"),
|
||||
&primary_environment.environment,
|
||||
&turn_environments[0].environment
|
||||
));
|
||||
assert_eq!(turn_context.cwd, selected_cwd);
|
||||
@@ -4376,54 +4400,42 @@ async fn default_turn_honors_empty_stored_thread_environments() {
|
||||
|
||||
let turn_context = session.new_default_turn().await;
|
||||
|
||||
assert!(turn_context.environment.is_none());
|
||||
assert!(turn_context.primary_environment().is_none());
|
||||
assert!(turn_context.environments.is_empty());
|
||||
assert_eq!(turn_context.cwd, session_cwd);
|
||||
assert_eq!(turn_context.config.cwd, session_cwd);
|
||||
assert_eq!(turn_context.environments.len(), 0);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn multiple_turn_environments_use_first_as_primary_environment() {
|
||||
let (session, _turn_context, _rx) = make_session_and_context_with_rx().await;
|
||||
let session_cwd = session.get_config().await.cwd.clone();
|
||||
let first_cwd =
|
||||
AbsolutePathBuf::try_from(session_cwd.as_path().join("first")).expect("absolute path");
|
||||
let second_cwd =
|
||||
AbsolutePathBuf::try_from(session_cwd.as_path().join("second")).expect("absolute path");
|
||||
async fn primary_environment_uses_first_turn_environment() {
|
||||
let (_session, mut turn_context) = make_session_and_context().await;
|
||||
let first_environment = turn_context.environments[0].clone();
|
||||
let second_cwd = turn_context.cwd.join("second");
|
||||
turn_context.environments.push(TurnEnvironment {
|
||||
environment_id: "second".to_string(),
|
||||
environment: Arc::clone(&first_environment.environment),
|
||||
cwd: second_cwd.clone(),
|
||||
});
|
||||
|
||||
let turn_context = session
|
||||
.new_turn_with_sub_id(
|
||||
"sub-1".to_string(),
|
||||
SessionSettingsUpdate {
|
||||
environments: Some(vec![
|
||||
TurnEnvironmentSelection {
|
||||
environment_id: "local".to_string(),
|
||||
cwd: first_cwd.clone(),
|
||||
},
|
||||
TurnEnvironmentSelection {
|
||||
environment_id: "local".to_string(),
|
||||
cwd: second_cwd.clone(),
|
||||
},
|
||||
]),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("turn should start");
|
||||
|
||||
let turn_environments = &turn_context.environments;
|
||||
assert_eq!(turn_environments.len(), 2);
|
||||
assert_eq!(turn_environments[0].cwd, first_cwd);
|
||||
assert_eq!(turn_environments[1].cwd, second_cwd);
|
||||
assert!(std::sync::Arc::ptr_eq(
|
||||
assert_eq!(
|
||||
turn_context
|
||||
.environment
|
||||
.as_ref()
|
||||
.expect("primary environment should be set"),
|
||||
&turn_environments[0].environment
|
||||
));
|
||||
assert_eq!(turn_context.cwd, first_cwd);
|
||||
assert_eq!(turn_context.config.cwd, first_cwd);
|
||||
.primary_environment()
|
||||
.expect("primary environment")
|
||||
.environment_id,
|
||||
first_environment.environment_id
|
||||
);
|
||||
assert_eq!(
|
||||
turn_context
|
||||
.environments
|
||||
.iter()
|
||||
.find(|environment| environment.environment_id == "second")
|
||||
.expect("second environment")
|
||||
.cwd,
|
||||
second_cwd
|
||||
);
|
||||
assert_eq!(turn_context.environments.len(), 2);
|
||||
assert_eq!(turn_context.environments[1].cwd, second_cwd);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -4441,7 +4453,7 @@ async fn empty_turn_environments_clear_primary_environment() {
|
||||
.await
|
||||
.expect("turn should start");
|
||||
|
||||
assert!(turn_context.environment.is_none());
|
||||
assert!(turn_context.primary_environment().is_none());
|
||||
assert_eq!(turn_context.cwd, session.get_config().await.cwd);
|
||||
assert_eq!(turn_context.config.cwd, session.get_config().await.cwd);
|
||||
assert_eq!(turn_context.environments.len(), 0);
|
||||
@@ -4450,6 +4462,10 @@ async fn empty_turn_environments_clear_primary_environment() {
|
||||
#[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(
|
||||
@@ -4457,7 +4473,7 @@ async fn unknown_turn_environment_returns_error() {
|
||||
SessionSettingsUpdate {
|
||||
environments: Some(vec![TurnEnvironmentSelection {
|
||||
environment_id: "missing".to_string(),
|
||||
cwd: session.get_config().await.cwd.clone(),
|
||||
cwd: original_configuration.cwd.clone(),
|
||||
}]),
|
||||
..Default::default()
|
||||
},
|
||||
@@ -4465,8 +4481,148 @@ async fn unknown_turn_environment_returns_error() {
|
||||
.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.environments,
|
||||
original_configuration.environments
|
||||
);
|
||||
}
|
||||
|
||||
#[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(vec![
|
||||
TurnEnvironmentSelection {
|
||||
environment_id: "local".to_string(),
|
||||
cwd: original_configuration.cwd.clone(),
|
||||
},
|
||||
TurnEnvironmentSelection {
|
||||
environment_id: "local".to_string(),
|
||||
cwd: 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.environments,
|
||||
original_configuration.environments
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mcp_runtime_environment_uses_primary_selected_environment() {
|
||||
let (session, _turn_context) = make_session_and_context().await;
|
||||
let selected_cwd = session
|
||||
.get_config()
|
||||
.await
|
||||
.cwd
|
||||
.join("mcp-selected-environment");
|
||||
{
|
||||
let mut state = session.state.lock().await;
|
||||
state.session_configuration.environments = vec![TurnEnvironmentSelection {
|
||||
environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(),
|
||||
cwd: selected_cwd.clone(),
|
||||
}];
|
||||
}
|
||||
|
||||
let session_configuration = {
|
||||
let state = session.state.lock().await;
|
||||
state.session_configuration.clone()
|
||||
};
|
||||
let runtime_environment =
|
||||
session.mcp_runtime_environment_for_configuration(&session_configuration);
|
||||
let selected_environment = session
|
||||
.services
|
||||
.environment_manager
|
||||
.get_environment(codex_exec_server::LOCAL_ENVIRONMENT_ID)
|
||||
.expect("local environment");
|
||||
|
||||
assert!(Arc::ptr_eq(
|
||||
&runtime_environment.environment(),
|
||||
&selected_environment
|
||||
));
|
||||
assert_eq!(
|
||||
runtime_environment.fallback_cwd(),
|
||||
selected_cwd.to_path_buf()
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mcp_runtime_environment_for_turn_context_uses_turn_environment() {
|
||||
let (session, mut turn_context) = make_session_and_context().await;
|
||||
let selected_cwd = turn_context.cwd.join("mcp-turn-environment");
|
||||
turn_context.environments[0].cwd = selected_cwd.clone();
|
||||
{
|
||||
let mut state = session.state.lock().await;
|
||||
state.session_configuration.environments = Vec::new();
|
||||
state.session_configuration.cwd = turn_context.cwd.join("stored-session-cwd");
|
||||
}
|
||||
|
||||
let runtime_environment = session.mcp_runtime_environment_for_turn_context(&turn_context);
|
||||
let primary_environment = turn_context
|
||||
.primary_environment()
|
||||
.expect("primary turn environment");
|
||||
|
||||
assert!(Arc::ptr_eq(
|
||||
&runtime_environment.environment(),
|
||||
&primary_environment.environment
|
||||
));
|
||||
assert_eq!(
|
||||
runtime_environment.fallback_cwd(),
|
||||
selected_cwd.to_path_buf()
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mcp_runtime_environment_without_selection_uses_local_fallback() {
|
||||
let (session, _turn_context) = make_session_and_context().await;
|
||||
{
|
||||
let mut state = session.state.lock().await;
|
||||
state.session_configuration.environments = Vec::new();
|
||||
}
|
||||
|
||||
let session_configuration = {
|
||||
let state = session.state.lock().await;
|
||||
state.session_configuration.clone()
|
||||
};
|
||||
let runtime_environment =
|
||||
session.mcp_runtime_environment_for_configuration(&session_configuration);
|
||||
let local_environment = session.services.environment_manager.local_environment();
|
||||
|
||||
assert!(Arc::ptr_eq(
|
||||
&runtime_environment.environment(),
|
||||
&local_environment
|
||||
));
|
||||
assert_eq!(
|
||||
runtime_environment.fallback_cwd(),
|
||||
session_configuration.cwd.to_path_buf()
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -5013,7 +5169,6 @@ where
|
||||
model_info,
|
||||
&models_manager,
|
||||
/*network*/ None,
|
||||
Some(environment),
|
||||
turn_environments,
|
||||
session_configuration.cwd.clone(),
|
||||
"turn_id".to_string(),
|
||||
|
||||
@@ -59,7 +59,6 @@ pub(crate) struct TurnContext {
|
||||
pub(crate) reasoning_effort: Option<ReasoningEffortConfig>,
|
||||
pub(crate) reasoning_summary: ReasoningSummaryConfig,
|
||||
pub(crate) session_source: SessionSource,
|
||||
pub(crate) environment: Option<Arc<Environment>>,
|
||||
pub(crate) environments: Vec<TurnEnvironment>,
|
||||
/// The session's absolute working directory. All relative paths provided
|
||||
/// by the model as well as sandbox policies are resolved against this path
|
||||
@@ -106,6 +105,10 @@ impl TurnContext {
|
||||
self.permission_profile.network_sandbox_policy()
|
||||
}
|
||||
|
||||
pub(crate) fn primary_environment(&self) -> Option<&TurnEnvironment> {
|
||||
self.environments.first()
|
||||
}
|
||||
|
||||
pub(crate) fn sandbox_policy(&self) -> SandboxPolicy {
|
||||
let file_system_sandbox_policy = self.file_system_sandbox_policy();
|
||||
let network_sandbox_policy = self.network_sandbox_policy();
|
||||
@@ -230,7 +233,6 @@ impl TurnContext {
|
||||
reasoning_effort,
|
||||
reasoning_summary: self.reasoning_summary,
|
||||
session_source: self.session_source.clone(),
|
||||
environment: self.environment.clone(),
|
||||
environments: self.environments.clone(),
|
||||
cwd: self.cwd.clone(),
|
||||
current_date: self.current_date.clone(),
|
||||
@@ -432,7 +434,6 @@ impl Session {
|
||||
model_info: ModelInfo,
|
||||
models_manager: &SharedModelsManager,
|
||||
network: Option<NetworkProxy>,
|
||||
environment: Option<Arc<Environment>>,
|
||||
environments: Vec<TurnEnvironment>,
|
||||
cwd: AbsolutePathBuf,
|
||||
sub_id: String,
|
||||
@@ -474,7 +475,7 @@ impl Session {
|
||||
)
|
||||
.with_web_search_config(per_turn_config.web_search_config.clone())
|
||||
.with_allow_login_shell(per_turn_config.permissions.allow_login_shell)
|
||||
.with_has_environment(environment.is_some())
|
||||
.with_has_environment(!environments.is_empty())
|
||||
.with_spawn_agent_usage_hint(per_turn_config.multi_agent_v2.usage_hint_enabled)
|
||||
.with_spawn_agent_usage_hint_text(per_turn_config.multi_agent_v2.usage_hint_text.clone())
|
||||
.with_hide_spawn_agent_metadata(per_turn_config.multi_agent_v2.hide_spawn_agent_metadata)
|
||||
@@ -522,7 +523,6 @@ impl Session {
|
||||
reasoning_effort,
|
||||
reasoning_summary,
|
||||
session_source,
|
||||
environment,
|
||||
environments,
|
||||
cwd,
|
||||
current_date: Some(current_date),
|
||||
@@ -641,6 +641,10 @@ impl Session {
|
||||
&self,
|
||||
environments: &[TurnEnvironmentSelection],
|
||||
) -> CodexResult<Vec<TurnEnvironment>> {
|
||||
crate::environment_selection::validate_environment_selections(
|
||||
self.services.environment_manager.as_ref(),
|
||||
environments,
|
||||
)?;
|
||||
let mut turn_environments = Vec::with_capacity(environments.len());
|
||||
for selected_environment in environments {
|
||||
let environment_id = selected_environment.environment_id.clone();
|
||||
@@ -672,8 +676,6 @@ impl Session {
|
||||
turn_environments: Vec<TurnEnvironment>,
|
||||
) -> Arc<TurnContext> {
|
||||
let primary_turn_environment = turn_environments.first();
|
||||
let environment = primary_turn_environment
|
||||
.map(|turn_environment| Arc::clone(&turn_environment.environment));
|
||||
let cwd = primary_turn_environment
|
||||
.map(|turn_environment| turn_environment.cwd.clone())
|
||||
.unwrap_or_else(|| session_configuration.cwd.clone());
|
||||
@@ -700,9 +702,8 @@ impl Session {
|
||||
.await;
|
||||
let effective_skill_roots = plugin_outcome.effective_skill_roots();
|
||||
let skills_input = skills_load_input_from_config(&per_turn_config, effective_skill_roots);
|
||||
let fs = environment
|
||||
.as_ref()
|
||||
.map(|environment| environment.get_filesystem());
|
||||
let fs = primary_turn_environment
|
||||
.map(|turn_environment| turn_environment.environment.get_filesystem());
|
||||
let skills_outcome = Arc::new(
|
||||
self.services
|
||||
.skills_manager
|
||||
@@ -731,7 +732,6 @@ impl Session {
|
||||
)
|
||||
.then(|| started_proxy.proxy())
|
||||
}),
|
||||
environment,
|
||||
turn_environments,
|
||||
cwd,
|
||||
sub_id,
|
||||
|
||||
@@ -1072,13 +1072,20 @@ impl ThreadManagerState {
|
||||
user_shell_override: Option<crate::shell::Shell>,
|
||||
) -> CodexResult<NewThread> {
|
||||
let is_resumed_thread = matches!(&initial_history, InitialHistory::Resumed(_));
|
||||
validate_environment_selections(self.environment_manager.as_ref(), &environments)?;
|
||||
let environment =
|
||||
selected_primary_environment(self.environment_manager.as_ref(), &environments)?;
|
||||
let effective_cwd = environments
|
||||
.first()
|
||||
.map(|environment| environment.cwd.clone())
|
||||
.unwrap_or_else(|| config.cwd.clone());
|
||||
let mut load_config = config.clone();
|
||||
load_config.cwd = effective_cwd;
|
||||
let watch_registration = match environment.as_ref() {
|
||||
Some(environment) if !environment.is_remote() => {
|
||||
self.skills_watcher
|
||||
.register_config(
|
||||
&config,
|
||||
&load_config,
|
||||
self.skills_manager.as_ref(),
|
||||
self.plugins_manager.as_ref(),
|
||||
Some(environment.get_filesystem()),
|
||||
|
||||
@@ -363,13 +363,14 @@ impl ToolHandler for ApplyPatchHandler {
|
||||
// Avoid building temporary ExecParams/command vectors; derive directly from inputs.
|
||||
let cwd = turn.cwd.clone();
|
||||
let command = vec!["apply_patch".to_string(), patch_input.clone()];
|
||||
let Some(environment) = turn.environment.as_ref() else {
|
||||
let Some(turn_environment) = turn.primary_environment() else {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"apply_patch is unavailable in this session".to_string(),
|
||||
));
|
||||
};
|
||||
let fs = environment.get_filesystem();
|
||||
let sandbox = environment
|
||||
let fs = turn_environment.environment.get_filesystem();
|
||||
let sandbox = turn_environment
|
||||
.environment
|
||||
.is_remote()
|
||||
.then(|| turn.file_system_sandbox_context(/*additional_permissions*/ None));
|
||||
match codex_apply_patch::maybe_parse_apply_patch_verified(
|
||||
@@ -474,9 +475,8 @@ pub(crate) async fn intercept_apply_patch(
|
||||
tool_name: &str,
|
||||
) -> Result<Option<FunctionToolOutput>, FunctionCallError> {
|
||||
let sandbox = turn
|
||||
.environment
|
||||
.as_ref()
|
||||
.filter(|env| env.is_remote())
|
||||
.primary_environment()
|
||||
.filter(|env| env.environment.is_remote())
|
||||
.map(|_| turn.file_system_sandbox_context(/*additional_permissions*/ None));
|
||||
match codex_apply_patch::maybe_parse_apply_patch_verified(command, cwd, fs, sandbox.as_ref())
|
||||
.await
|
||||
|
||||
@@ -412,12 +412,12 @@ impl ShellHandler {
|
||||
} = args;
|
||||
|
||||
let mut exec_params = exec_params;
|
||||
let Some(environment) = turn.environment.as_ref() else {
|
||||
let Some(turn_environment) = turn.primary_environment() else {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"shell is unavailable in this session".to_string(),
|
||||
));
|
||||
};
|
||||
let fs = environment.get_filesystem();
|
||||
let fs = turn_environment.environment.get_filesystem();
|
||||
|
||||
let dependency_env = session.dependency_env().await;
|
||||
if !dependency_env.is_empty() {
|
||||
|
||||
@@ -196,12 +196,12 @@ impl ToolHandler for UnifiedExecHandler {
|
||||
}
|
||||
};
|
||||
|
||||
let Some(environment) = turn.environment.as_ref() else {
|
||||
let Some(turn_environment) = turn.primary_environment() else {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"unified exec is unavailable in this session".to_string(),
|
||||
));
|
||||
};
|
||||
let fs = environment.get_filesystem();
|
||||
let fs = turn_environment.environment.get_filesystem();
|
||||
|
||||
let manager: &UnifiedExecProcessManager = &session.services.unified_exec_manager;
|
||||
let context = UnifiedExecContext::new(session.clone(), turn.clone(), call_id.clone());
|
||||
|
||||
@@ -88,16 +88,18 @@ impl ToolHandler for ViewImageHandler {
|
||||
};
|
||||
|
||||
let abs_path = turn.resolve_path(Some(args.path));
|
||||
let Some(environment) = turn.environment.as_ref() else {
|
||||
let Some(environment) = turn.primary_environment() else {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"view_image is unavailable in this session".to_string(),
|
||||
));
|
||||
};
|
||||
let sandbox = environment
|
||||
.environment
|
||||
.is_remote()
|
||||
.then(|| turn.file_system_sandbox_context(/*additional_permissions*/ None));
|
||||
|
||||
let metadata = environment
|
||||
.environment
|
||||
.get_filesystem()
|
||||
.get_metadata(&abs_path, sandbox.as_ref())
|
||||
.await
|
||||
@@ -115,6 +117,7 @@ impl ToolHandler for ViewImageHandler {
|
||||
)));
|
||||
}
|
||||
let file_bytes = environment
|
||||
.environment
|
||||
.get_filesystem()
|
||||
.read_file(&abs_path, sandbox.as_ref())
|
||||
.await
|
||||
|
||||
@@ -191,11 +191,11 @@ impl ToolRuntime<ApplyPatchRequest, ExecToolCallOutput> for ApplyPatchRuntime {
|
||||
attempt: &SandboxAttempt<'_>,
|
||||
ctx: &ToolCtx,
|
||||
) -> Result<ExecToolCallOutput, ToolError> {
|
||||
let environment = ctx.turn.environment.as_ref().ok_or_else(|| {
|
||||
let turn_environment = ctx.turn.primary_environment().ok_or_else(|| {
|
||||
ToolError::Rejected("apply_patch is unavailable in this session".to_string())
|
||||
})?;
|
||||
let started_at = Instant::now();
|
||||
let fs = environment.get_filesystem();
|
||||
let fs = turn_environment.environment.get_filesystem();
|
||||
let sandbox = Self::file_system_sandbox_context_for_attempt(req, attempt);
|
||||
let mut stdout = Vec::new();
|
||||
let mut stderr = Vec::new();
|
||||
|
||||
@@ -254,9 +254,8 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
}
|
||||
let environment_is_remote = ctx
|
||||
.turn
|
||||
.environment
|
||||
.as_ref()
|
||||
.is_some_and(|environment| environment.is_remote());
|
||||
.primary_environment()
|
||||
.is_some_and(|turn_environment| turn_environment.environment.is_remote());
|
||||
let command = if environment_is_remote {
|
||||
base_command.to_vec()
|
||||
} else {
|
||||
@@ -293,12 +292,12 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
.await?
|
||||
{
|
||||
Some(prepared) => {
|
||||
let Some(environment) = ctx.turn.environment.as_ref() else {
|
||||
let Some(turn_environment) = ctx.turn.primary_environment() else {
|
||||
return Err(ToolError::Rejected(
|
||||
"exec_command is unavailable in this session".to_string(),
|
||||
));
|
||||
};
|
||||
if environment.is_remote() {
|
||||
if turn_environment.environment.is_remote() {
|
||||
return Err(ToolError::Rejected(
|
||||
"unified_exec zsh-fork is not supported when exec_server_url is configured".to_string(),
|
||||
));
|
||||
@@ -310,7 +309,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
&prepared.exec_request,
|
||||
req.tty,
|
||||
prepared.spawn_lifecycle,
|
||||
environment.as_ref(),
|
||||
turn_environment.environment.as_ref(),
|
||||
)
|
||||
.await
|
||||
.map_err(|err| match err {
|
||||
@@ -338,7 +337,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
.env_for(command, options, managed_network)
|
||||
.map_err(|err| ToolError::Codex(err.into()))?;
|
||||
exec_env.exec_server_env_config = req.exec_server_env_config.clone();
|
||||
let Some(environment) = ctx.turn.environment.as_ref() else {
|
||||
let Some(turn_environment) = ctx.turn.primary_environment() else {
|
||||
return Err(ToolError::Rejected(
|
||||
"exec_command is unavailable in this session".to_string(),
|
||||
));
|
||||
@@ -349,7 +348,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
&exec_env,
|
||||
req.tty,
|
||||
Box::new(NoopSpawnLifecycle),
|
||||
environment.as_ref(),
|
||||
turn_environment.environment.as_ref(),
|
||||
)
|
||||
.await
|
||||
.map_err(|err| match err {
|
||||
|
||||
@@ -96,7 +96,10 @@ async fn exec_command_with_tty(
|
||||
&request,
|
||||
tty,
|
||||
Box::new(NoopSpawnLifecycle),
|
||||
turn.environment.as_ref().expect("turn environment"),
|
||||
turn.primary_environment()
|
||||
.expect("turn environment")
|
||||
.environment
|
||||
.as_ref(),
|
||||
)
|
||||
.await?,
|
||||
);
|
||||
@@ -591,7 +594,7 @@ async fn remote_exec_server_rejects_inherited_fd_launches() -> anyhow::Result<()
|
||||
|
||||
let remote_test_env = remote_test_env().await?;
|
||||
let (_, mut turn) = make_session_and_context().await;
|
||||
turn.environment = Some(Arc::new(remote_test_env.environment().clone()));
|
||||
turn.environments[0].environment = Arc::new(remote_test_env.environment().clone());
|
||||
|
||||
let request = test_exec_request(
|
||||
&turn,
|
||||
@@ -609,7 +612,10 @@ async fn remote_exec_server_rejects_inherited_fd_launches() -> anyhow::Result<()
|
||||
Box::new(TestSpawnLifecycle {
|
||||
inherited_fds: vec![42],
|
||||
}),
|
||||
turn.environment.as_ref().expect("turn environment"),
|
||||
turn.primary_environment()
|
||||
.expect("turn environment")
|
||||
.environment
|
||||
.as_ref(),
|
||||
)
|
||||
.await
|
||||
.expect_err("expected inherited fd rejection");
|
||||
|
||||
Reference in New Issue
Block a user