mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Prepare selected environment plumbing (#20669)
## Why This is a prep PR in the multi-environment process-tool stack. It separates ownership/config cleanup from the behavior change that teaches process tools to route by selected environment, so the follow-up PR can focus on model-facing `environment_id` behavior. ## Stack 1. https://github.com/openai/codex/pull/20646 - `EnvironmentContext` rendering for selected environments 2. https://github.com/openai/codex/pull/20669 - selected-environment ownership and tool config prep (this PR) 3. https://github.com/openai/codex/pull/20647 - process-tool `environment_id` routing ## What Changed - keep the resolved turn environment list wrapped in `ResolvedTurnEnvironments` through `TurnContext` instead of unwrapping it back to a raw `Vec` - add `TurnContext::resolve_path_against` so cwd-relative path resolution has one shared helper - replace the old tool config boolean with `ToolEnvironmentMode::{None, Single, Multiple}` ## Testing - Tests not run locally; this prep refactor is covered by GitHub CI for the stack. Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
committed by
GitHub
Unverified
parent
5c1ec8f4fd
commit
905987c08f
@@ -221,7 +221,7 @@ impl Session {
|
||||
let mcp_servers = with_codex_apps_mcp(mcp_servers, auth.as_ref(), &mcp_config);
|
||||
let auth_statuses =
|
||||
compute_auth_statuses(mcp_servers.iter(), store_mode, auth.as_ref()).await;
|
||||
let mcp_runtime_environment = match turn_context.primary_environment() {
|
||||
let mcp_runtime_environment = match turn_context.environments.primary() {
|
||||
Some(turn_environment) => McpRuntimeEnvironment::new(
|
||||
Arc::clone(&turn_environment.environment),
|
||||
turn_environment.cwd.to_path_buf(),
|
||||
|
||||
@@ -355,6 +355,7 @@ use codex_protocol::protocol::TokenUsage;
|
||||
use codex_protocol::protocol::TokenUsageInfo;
|
||||
use codex_protocol::protocol::WarningEvent;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use codex_tools::ToolEnvironmentMode;
|
||||
use codex_tools::ToolsConfig;
|
||||
use codex_tools::ToolsConfigParams;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
|
||||
@@ -52,7 +52,7 @@ pub(super) async fn spawn_review_thread(
|
||||
)
|
||||
.with_web_search_config(/*web_search_config*/ None)
|
||||
.with_allow_login_shell(config.permissions.allow_login_shell)
|
||||
.with_has_environment(parent_turn_context.tools_config.has_environment)
|
||||
.with_environment_mode(parent_turn_context.tools_config.environment_mode)
|
||||
.with_spawn_agent_usage_hint(config.multi_agent_v2.usage_hint_enabled)
|
||||
.with_spawn_agent_usage_hint_text(config.multi_agent_v2.usage_hint_text.clone())
|
||||
.with_hide_spawn_agent_metadata(config.multi_agent_v2.hide_spawn_agent_metadata)
|
||||
|
||||
@@ -968,7 +968,7 @@ impl Session {
|
||||
"unknown stored MCP environment id",
|
||||
))
|
||||
})?
|
||||
.primary_turn_environment()
|
||||
.primary()
|
||||
.cloned();
|
||||
let mcp_runtime_environment = match turn_environment {
|
||||
Some(turn_environment) => McpRuntimeEnvironment::new(
|
||||
|
||||
@@ -2944,13 +2944,15 @@ pub(crate) async fn make_session_configuration_for_tests() -> SessionConfigurati
|
||||
fn turn_environments_for_tests(
|
||||
environment: &Arc<codex_exec_server::Environment>,
|
||||
cwd: &codex_utils_absolute_path::AbsolutePathBuf,
|
||||
) -> Vec<TurnEnvironment> {
|
||||
vec![TurnEnvironment {
|
||||
environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(),
|
||||
environment: Arc::clone(environment),
|
||||
cwd: cwd.clone(),
|
||||
shell: "bash".to_string(),
|
||||
}]
|
||||
) -> crate::environment_selection::ResolvedTurnEnvironments {
|
||||
crate::environment_selection::ResolvedTurnEnvironments {
|
||||
turn_environments: vec![TurnEnvironment {
|
||||
environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(),
|
||||
environment: Arc::clone(environment),
|
||||
cwd: cwd.clone(),
|
||||
shell: "bash".to_string(),
|
||||
}],
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -3399,7 +3401,7 @@ async fn absolute_cwd_update_with_turn_environment_is_allowed() {
|
||||
|
||||
assert_eq!(turn_context.cwd, absolute_cwd);
|
||||
assert_eq!(turn_context.config.cwd, absolute_cwd);
|
||||
assert_eq!(turn_context.environments.len(), 1);
|
||||
assert_eq!(turn_context.environments.turn_environments.len(), 1);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -4418,15 +4420,16 @@ async fn turn_environments_set_primary_environment() {
|
||||
.expect("turn should start");
|
||||
|
||||
let turn_environments = &turn_context.environments;
|
||||
assert_eq!(turn_environments.len(), 1);
|
||||
assert_eq!(turn_environments.turn_environments.len(), 1);
|
||||
let turn_environment = turn_context
|
||||
.primary_environment()
|
||||
.environments
|
||||
.primary()
|
||||
.expect("primary environment should be set");
|
||||
assert!(std::sync::Arc::ptr_eq(
|
||||
&turn_environment.environment,
|
||||
&turn_environments[0].environment
|
||||
&turn_environments.turn_environments[0].environment
|
||||
));
|
||||
assert!(!turn_context.environments.is_empty());
|
||||
assert!(!turn_context.environments.turn_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());
|
||||
}
|
||||
@@ -4449,13 +4452,14 @@ async fn default_turn_overlays_session_cwd_onto_stored_thread_environments() {
|
||||
let turn_context = session.new_default_turn().await;
|
||||
|
||||
let turn_environments = &turn_context.environments;
|
||||
assert_eq!(turn_environments.len(), 1);
|
||||
assert_eq!(turn_environments.turn_environments.len(), 1);
|
||||
let turn_environment = turn_context
|
||||
.primary_environment()
|
||||
.environments
|
||||
.primary()
|
||||
.expect("primary environment should be set");
|
||||
assert!(std::sync::Arc::ptr_eq(
|
||||
&turn_environment.environment,
|
||||
&turn_environments[0].environment
|
||||
&turn_environments.turn_environments[0].environment
|
||||
));
|
||||
assert_eq!(turn_context.cwd, session_cwd);
|
||||
assert_eq!(turn_context.config.cwd, session_cwd);
|
||||
@@ -4473,28 +4477,32 @@ async fn default_turn_honors_empty_stored_thread_environments() {
|
||||
|
||||
let turn_context = session.new_default_turn().await;
|
||||
|
||||
assert!(turn_context.primary_environment().is_none());
|
||||
assert!(turn_context.environments.is_empty());
|
||||
assert!(turn_context.environments.primary().is_none());
|
||||
assert!(turn_context.environments.turn_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);
|
||||
assert_eq!(turn_context.environments.turn_environments.len(), 0);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
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 first_environment = turn_context.environments.turn_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(),
|
||||
shell: first_environment.shell.clone(),
|
||||
});
|
||||
turn_context
|
||||
.environments
|
||||
.turn_environments
|
||||
.push(TurnEnvironment {
|
||||
environment_id: "second".to_string(),
|
||||
environment: Arc::clone(&first_environment.environment),
|
||||
cwd: second_cwd.clone(),
|
||||
shell: first_environment.shell.clone(),
|
||||
});
|
||||
|
||||
assert_eq!(
|
||||
turn_context
|
||||
.primary_environment()
|
||||
.environments
|
||||
.primary()
|
||||
.expect("primary environment")
|
||||
.environment_id,
|
||||
first_environment.environment_id
|
||||
@@ -4502,14 +4510,18 @@ async fn primary_environment_uses_first_turn_environment() {
|
||||
assert_eq!(
|
||||
turn_context
|
||||
.environments
|
||||
.turn_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);
|
||||
assert_eq!(turn_context.environments.turn_environments.len(), 2);
|
||||
assert_eq!(
|
||||
turn_context.environments.turn_environments[1].cwd,
|
||||
second_cwd
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -4527,8 +4539,8 @@ async fn empty_turn_environments_clear_primary_environment() {
|
||||
.await
|
||||
.expect("turn should start");
|
||||
|
||||
assert!(turn_context.primary_environment().is_none());
|
||||
assert!(turn_context.environments.is_empty());
|
||||
assert!(turn_context.environments.primary().is_none());
|
||||
assert!(turn_context.environments.turn_environments.is_empty());
|
||||
assert_eq!(turn_context.cwd, session.get_config().await.cwd);
|
||||
assert_eq!(turn_context.config.cwd, session.get_config().await.cwd);
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use super::*;
|
||||
use crate::config::GhostSnapshotConfig;
|
||||
use crate::environment_selection::ResolvedTurnEnvironments;
|
||||
use codex_model_provider::SharedModelProvider;
|
||||
use codex_model_provider::create_model_provider;
|
||||
use codex_protocol::models::AdditionalPermissionProfile;
|
||||
@@ -60,7 +61,7 @@ pub(crate) struct TurnContext {
|
||||
pub(crate) reasoning_effort: Option<ReasoningEffortConfig>,
|
||||
pub(crate) reasoning_summary: ReasoningSummaryConfig,
|
||||
pub(crate) session_source: SessionSource,
|
||||
pub(crate) environments: Vec<TurnEnvironment>,
|
||||
pub(crate) environments: ResolvedTurnEnvironments,
|
||||
/// The session's absolute working directory. All relative paths provided
|
||||
/// by the model as well as sandbox policies are resolved against this path
|
||||
/// instead of `std::env::current_dir()`.
|
||||
@@ -106,10 +107,6 @@ 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();
|
||||
@@ -198,7 +195,7 @@ impl TurnContext {
|
||||
.with_unified_exec_shell_mode(self.tools_config.unified_exec_shell_mode.clone())
|
||||
.with_web_search_config(self.tools_config.web_search_config.clone())
|
||||
.with_allow_login_shell(self.tools_config.allow_login_shell)
|
||||
.with_has_environment(self.tools_config.has_environment)
|
||||
.with_environment_mode(self.tools_config.environment_mode)
|
||||
.with_spawn_agent_usage_hint(config.multi_agent_v2.usage_hint_enabled)
|
||||
.with_spawn_agent_usage_hint_text(config.multi_agent_v2.usage_hint_text.clone())
|
||||
.with_hide_spawn_agent_metadata(config.multi_agent_v2.hide_spawn_agent_metadata)
|
||||
@@ -435,7 +432,7 @@ impl Session {
|
||||
model_info: ModelInfo,
|
||||
models_manager: &SharedModelsManager,
|
||||
network: Option<NetworkProxy>,
|
||||
environments: Vec<TurnEnvironment>,
|
||||
environments: ResolvedTurnEnvironments,
|
||||
cwd: AbsolutePathBuf,
|
||||
sub_id: String,
|
||||
skills_outcome: Arc<SkillLoadOutcome>,
|
||||
@@ -476,7 +473,9 @@ 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(!environments.is_empty())
|
||||
.with_environment_mode(ToolEnvironmentMode::from_count(
|
||||
environments.turn_environments.len(),
|
||||
))
|
||||
.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)
|
||||
@@ -647,12 +646,11 @@ impl Session {
|
||||
fn resolve_turn_environments(
|
||||
&self,
|
||||
environments: &[TurnEnvironmentSelection],
|
||||
) -> CodexResult<Vec<TurnEnvironment>> {
|
||||
) -> CodexResult<ResolvedTurnEnvironments> {
|
||||
crate::environment_selection::resolve_environment_selections(
|
||||
self.services.environment_manager.as_ref(),
|
||||
environments,
|
||||
)
|
||||
.map(|resolved| resolved.turn_environments)
|
||||
}
|
||||
|
||||
async fn new_turn_from_configuration(
|
||||
@@ -660,9 +658,9 @@ impl Session {
|
||||
sub_id: String,
|
||||
session_configuration: SessionConfiguration,
|
||||
final_output_json_schema: Option<Option<Value>>,
|
||||
turn_environments: Vec<TurnEnvironment>,
|
||||
turn_environments: ResolvedTurnEnvironments,
|
||||
) -> Arc<TurnContext> {
|
||||
let primary_turn_environment = turn_environments.first();
|
||||
let primary_turn_environment = turn_environments.primary();
|
||||
let cwd = primary_turn_environment
|
||||
.map(|turn_environment| turn_environment.cwd.clone())
|
||||
.unwrap_or_else(|| session_configuration.cwd.clone());
|
||||
@@ -769,7 +767,7 @@ impl Session {
|
||||
Ok(turn_environments) => turn_environments,
|
||||
Err(err) => {
|
||||
warn!("failed to resolve stored session environments: {err}");
|
||||
Vec::new()
|
||||
ResolvedTurnEnvironments::default()
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user