mirror of
https://github.com/openai/codex.git
synced 2026-11-07 20:30:33 +00:00
Centralize turn environment selection state (#38461)
## What changed - Store `TurnEnvironmentSelection` directly on each resolved `TurnEnvironment` instead of copying its environment ID, working directory, and workspace roots into separate fields. - Update environment resolution, tool execution, approvals, and related tests to access selection state through the retained value. GitOrigin-RevId: ab723d21e112b8af5bcaf1f772a5e376553312bc
This commit is contained in:
@@ -282,7 +282,7 @@ async fn effective_patch_permissions(
|
||||
crate::tools::handlers::EffectiveAdditionalPermissions,
|
||||
codex_protocol::permissions::FileSystemSandboxPolicy,
|
||||
)> {
|
||||
let environment_id = environment.environment_id.as_str();
|
||||
let environment_id = environment.selection.environment_id.as_str();
|
||||
let file_paths = file_paths_for_action(action);
|
||||
let native_cwd = cwd.to_abs_path()?;
|
||||
let granted_permissions = merge_permission_profiles(
|
||||
@@ -565,7 +565,7 @@ async fn execute_verified_patch(
|
||||
let emitter = ToolEmitter::apply_patch_for_environment(
|
||||
changes.clone(),
|
||||
apply.auto_approved,
|
||||
turn_environment.environment_id.clone(),
|
||||
turn_environment.selection.environment_id.clone(),
|
||||
);
|
||||
let event_ctx = ToolEventCtx::new(
|
||||
tool_ctx.session.as_ref(),
|
||||
|
||||
@@ -150,7 +150,7 @@ async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall {
|
||||
};
|
||||
let additional_permissions = apply_granted_turn_permissions(
|
||||
invocation.session.as_ref(),
|
||||
&environment.environment_id,
|
||||
&environment.selection.environment_id,
|
||||
native_cwd.as_path(),
|
||||
SandboxPermissions::UseDefault,
|
||||
/*additional_permissions*/ None,
|
||||
@@ -161,7 +161,7 @@ async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall {
|
||||
.turn
|
||||
.file_system_sandbox_context(additional_permissions, environment);
|
||||
environments.push(ToolEnvironment {
|
||||
environment_id: environment.environment_id.clone(),
|
||||
environment_id: environment.selection.environment_id.clone(),
|
||||
cwd: native_cwd,
|
||||
file_system: environment.environment.get_filesystem(),
|
||||
file_system_sandbox_context,
|
||||
|
||||
@@ -175,7 +175,7 @@ fn resolve_tool_environment<'a>(
|
||||
|environment_id| {
|
||||
environments
|
||||
.turn_environments()
|
||||
.find(|environment| environment.environment_id == environment_id)
|
||||
.find(|environment| environment.selection.environment_id == environment_id)
|
||||
.map(Some)
|
||||
.ok_or_else(|| {
|
||||
FunctionCallError::RespondToModel(format!(
|
||||
|
||||
@@ -95,7 +95,7 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
|
||||
let requested_additional_permissions = additional_permissions.clone();
|
||||
let effective_additional_permissions = apply_granted_turn_permissions(
|
||||
session.as_ref(),
|
||||
&turn_environment.environment_id,
|
||||
&turn_environment.selection.environment_id,
|
||||
exec_params.cwd.as_path(),
|
||||
exec_params.sandbox_permissions,
|
||||
additional_permissions,
|
||||
|
||||
@@ -125,7 +125,7 @@ impl ShellCommandHandler {
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
env,
|
||||
network: turn_context.network.clone(),
|
||||
network_environment_id: Some(turn_environment.environment_id.clone()),
|
||||
network_environment_id: Some(turn_environment.selection.environment_id.clone()),
|
||||
sandbox_permissions,
|
||||
windows_sandbox_level: turn_context.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: turn_context
|
||||
|
||||
@@ -25,6 +25,7 @@ use crate::tools::handlers::ShellCommandHandler;
|
||||
use crate::tools::hook_names::HookToolName;
|
||||
use crate::tools::registry::CoreToolRuntime;
|
||||
use crate::turn_diff_tracker::TurnDiffTracker;
|
||||
use codex_protocol::protocol::TurnEnvironmentSelection;
|
||||
use codex_shell_command::is_safe_command::is_known_safe_command;
|
||||
use codex_shell_command::powershell::try_find_powershell_executable_blocking;
|
||||
use codex_shell_command::powershell::try_find_pwsh_executable_blocking;
|
||||
@@ -103,7 +104,11 @@ async fn shell_command_handler_to_exec_params_uses_selected_environment() {
|
||||
let expected_cwd = selected_cwd.join("subdir");
|
||||
let active_permission_profile = ActivePermissionProfile::new("selected-profile");
|
||||
let selected_environment = TurnEnvironment::new(
|
||||
"selected-environment".to_string(),
|
||||
TurnEnvironmentSelection {
|
||||
environment_id: "selected-environment".to_string(),
|
||||
cwd: PathUri::from_abs_path(&selected_cwd),
|
||||
workspace_roots: Vec::new(),
|
||||
},
|
||||
Arc::clone(
|
||||
&turn_context
|
||||
.environments
|
||||
@@ -111,8 +116,6 @@ async fn shell_command_handler_to_exec_params_uses_selected_environment() {
|
||||
.expect("primary environment")
|
||||
.environment,
|
||||
),
|
||||
PathUri::from_abs_path(&selected_cwd),
|
||||
Vec::new(),
|
||||
Some(selected_shell),
|
||||
TurnEnvironmentConfig {
|
||||
allow_login_shell: true,
|
||||
|
||||
@@ -197,7 +197,7 @@ impl ExecCommandHandler {
|
||||
&hook_command,
|
||||
&cwd,
|
||||
native_cwd.as_ref(),
|
||||
&turn_environment.environment_id,
|
||||
&turn_environment.selection.environment_id,
|
||||
)
|
||||
.await;
|
||||
let shell_mode =
|
||||
@@ -217,13 +217,13 @@ impl ExecCommandHandler {
|
||||
let Some(remote_shell) = turn_environment.shell.as_ref() else {
|
||||
return Err(FunctionCallError::RespondToModel(format!(
|
||||
"environment `{}` does not report a shell",
|
||||
turn_environment.environment_id
|
||||
turn_environment.selection.environment_id
|
||||
)));
|
||||
};
|
||||
if detect_shell_type(Path::new(&requested_shell)) != Some(remote_shell.shell_type) {
|
||||
return Err(FunctionCallError::RespondToModel(format!(
|
||||
"environment `{}` only supports `{}`",
|
||||
turn_environment.environment_id,
|
||||
turn_environment.selection.environment_id,
|
||||
remote_shell.name()
|
||||
)));
|
||||
}
|
||||
@@ -258,7 +258,7 @@ impl ExecCommandHandler {
|
||||
let permission_cwd = native_cwd.as_ref().unwrap_or(&turn.config.cwd);
|
||||
let effective_additional_permissions = apply_granted_turn_permissions(
|
||||
context.session.as_ref(),
|
||||
&turn_environment.environment_id,
|
||||
&turn_environment.selection.environment_id,
|
||||
permission_cwd.as_path(),
|
||||
sandbox_permissions,
|
||||
additional_permissions,
|
||||
|
||||
@@ -285,11 +285,12 @@ mod tests {
|
||||
.next()
|
||||
.cloned()
|
||||
.expect("default local turn environment");
|
||||
let mut selection = current.selection;
|
||||
selection.cwd = PathUri::from_abs_path(&cwd);
|
||||
selection.workspace_roots.clear();
|
||||
turn.environments.environments[0] = TurnEnvironmentState::Ready(TurnEnvironment::new(
|
||||
current.environment_id,
|
||||
selection,
|
||||
current.environment,
|
||||
PathUri::from_abs_path(&cwd),
|
||||
Vec::new(),
|
||||
current.shell,
|
||||
current.config,
|
||||
));
|
||||
|
||||
@@ -123,7 +123,7 @@ impl ToolExecutor<ToolInvocation> for WaitForEnvironmentHandler {
|
||||
let already_ready = step_context
|
||||
.environments
|
||||
.turn_environments()
|
||||
.any(|environment| environment.environment_id == environment_id);
|
||||
.any(|environment| environment.selection.environment_id == environment_id);
|
||||
if !already_ready {
|
||||
let Some(environment) = step_context
|
||||
.environments
|
||||
|
||||
@@ -604,7 +604,7 @@ impl NetworkApprovalService {
|
||||
active_turn
|
||||
.as_ref()
|
||||
.and_then(|(turn_context, _)| turn_context.environments.primary())
|
||||
.map(|environment| environment.environment_id.clone())
|
||||
.map(|environment| environment.selection.environment_id.clone())
|
||||
}) else {
|
||||
return NetworkDecision::deny(REASON_NOT_ALLOWED);
|
||||
};
|
||||
@@ -666,7 +666,7 @@ impl NetworkApprovalService {
|
||||
turn_context
|
||||
.environments
|
||||
.turn_environments()
|
||||
.find(|environment| environment.environment_id == environment_id)
|
||||
.find(|environment| environment.selection.environment_id == environment_id)
|
||||
.map(TurnEnvironment::permission_profile)
|
||||
});
|
||||
if !permission_profile.is_some_and(permission_profile_allows_network_approval_flow) {
|
||||
@@ -705,7 +705,7 @@ impl NetworkApprovalService {
|
||||
turn_context
|
||||
.environments
|
||||
.turn_environments()
|
||||
.find(|environment| environment.environment_id == environment_id)
|
||||
.find(|environment| environment.selection.environment_id == environment_id)
|
||||
.and_then(|environment| environment.cwd().to_abs_path().ok())
|
||||
.unwrap_or_else(|| {
|
||||
#[allow(deprecated)]
|
||||
|
||||
@@ -74,7 +74,7 @@ impl ApplyPatchRuntime {
|
||||
fn build_approval_action(req: &ApplyPatchRequest, call_id: &str) -> ApprovalAction {
|
||||
ApprovalAction::ApplyPatch {
|
||||
id: call_id.to_string(),
|
||||
environment_id: req.turn_environment.environment_id.clone(),
|
||||
environment_id: req.turn_environment.selection.environment_id.clone(),
|
||||
cwd: req.action.cwd.clone(),
|
||||
files: req.file_paths.clone(),
|
||||
patch: req.action.patch.clone(),
|
||||
|
||||
@@ -8,6 +8,7 @@ use codex_protocol::models::FileSystemPermissions;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_protocol::protocol::GranularApprovalConfig;
|
||||
use codex_protocol::protocol::TurnEnvironmentSelection;
|
||||
use codex_sandboxing::SandboxManager;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy;
|
||||
@@ -18,10 +19,12 @@ use pretty_assertions::assert_eq;
|
||||
use std::collections::HashMap;
|
||||
fn test_turn_environment(environment_id: &str) -> crate::session::turn_context::TurnEnvironment {
|
||||
crate::session::turn_context::TurnEnvironment::new(
|
||||
environment_id.to_string(),
|
||||
TurnEnvironmentSelection {
|
||||
environment_id: environment_id.to_string(),
|
||||
cwd: PathUri::from_abs_path(&std::env::temp_dir().abs()),
|
||||
workspace_roots: Vec::new(),
|
||||
},
|
||||
std::sync::Arc::new(codex_exec_server::Environment::default_for_tests()),
|
||||
PathUri::from_abs_path(&std::env::temp_dir().abs()),
|
||||
Vec::new(),
|
||||
/*shell*/ None,
|
||||
TurnEnvironmentConfig {
|
||||
allow_login_shell: true,
|
||||
|
||||
@@ -129,7 +129,7 @@ impl Approvable<ShellRequest> for ShellRuntime {
|
||||
) -> std::io::Result<ApprovalAction> {
|
||||
Ok(ApprovalAction::Shell {
|
||||
id: call_id.to_string(),
|
||||
environment_id: req.turn_environment.environment_id.clone(),
|
||||
environment_id: req.turn_environment.selection.environment_id.clone(),
|
||||
command: req.command.clone(),
|
||||
hook_command: req.hook_command.clone(),
|
||||
cwd: PathUri::from_abs_path(&req.cwd),
|
||||
@@ -186,7 +186,7 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
|
||||
tty: None,
|
||||
},
|
||||
command: req.hook_command.clone(),
|
||||
environment_id: req.turn_environment.environment_id.clone(),
|
||||
environment_id: req.turn_environment.selection.environment_id.clone(),
|
||||
permission_profile: req.turn_environment.permission_profile().clone(),
|
||||
})
|
||||
}
|
||||
@@ -307,7 +307,7 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
|
||||
command,
|
||||
options,
|
||||
managed_network,
|
||||
Some(&req.turn_environment.environment_id),
|
||||
Some(&req.turn_environment.selection.environment_id),
|
||||
)
|
||||
.map_err(ToolError::Codex)?;
|
||||
execute_env(env, Self::stdout_stream(ctx))
|
||||
|
||||
@@ -149,7 +149,7 @@ pub(super) async fn try_run_zsh_fork(
|
||||
command,
|
||||
options,
|
||||
managed_network_for_sandbox_permissions(req.network.as_ref(), req.sandbox_permissions),
|
||||
Some(&req.turn_environment.environment_id),
|
||||
Some(&req.turn_environment.selection.environment_id),
|
||||
)
|
||||
.map_err(ToolError::Codex)?;
|
||||
let crate::sandboxing::ExecRequest {
|
||||
@@ -244,7 +244,7 @@ pub(super) async fn try_run_zsh_fork(
|
||||
session: Arc::clone(&ctx.session),
|
||||
review_context: GuardianReviewContext::from(&ctx.step_context),
|
||||
call_id: ctx.call_id.clone(),
|
||||
environment_id: req.turn_environment.environment_id.clone(),
|
||||
environment_id: req.turn_environment.selection.environment_id.clone(),
|
||||
source: GuardianCommandSource::Shell,
|
||||
tool_name: ctx.tool_name.clone(),
|
||||
approval_policy: ctx.step_context.turn.approval_policy(),
|
||||
@@ -331,7 +331,7 @@ pub(crate) async fn prepare_unified_exec_zsh_fork(
|
||||
session: Arc::clone(&ctx.session),
|
||||
review_context: GuardianReviewContext::from(&ctx.step_context),
|
||||
call_id: ctx.call_id.clone(),
|
||||
environment_id: req.turn_environment.environment_id.clone(),
|
||||
environment_id: req.turn_environment.selection.environment_id.clone(),
|
||||
source: GuardianCommandSource::UnifiedExec,
|
||||
tool_name: ctx.tool_name.clone(),
|
||||
approval_policy: ctx.step_context.turn.approval_policy(),
|
||||
|
||||
@@ -4,6 +4,7 @@ use crate::session::turn_context::TurnEnvironmentConfig;
|
||||
use crate::tools::approvals::ApprovalCacheKey;
|
||||
use codex_exec_server::Environment;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::TurnEnvironmentSelection;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::sync::Arc;
|
||||
@@ -15,10 +16,12 @@ async fn approval_key_uses_path_uri_and_includes_environment_id() {
|
||||
let mut request = ShellRequest {
|
||||
command: vec!["echo".to_string(), "hello".to_string()],
|
||||
turn_environment: TurnEnvironment::new(
|
||||
"remote".to_string(),
|
||||
TurnEnvironmentSelection {
|
||||
environment_id: "remote".to_string(),
|
||||
cwd: PathUri::from_abs_path(&cwd),
|
||||
workspace_roots: Vec::new(),
|
||||
},
|
||||
Arc::new(Environment::default_for_tests()),
|
||||
PathUri::from_abs_path(&cwd),
|
||||
Vec::new(),
|
||||
/*shell*/ None,
|
||||
TurnEnvironmentConfig {
|
||||
allow_login_shell: true,
|
||||
@@ -61,7 +64,7 @@ async fn approval_key_uses_path_uri_and_includes_environment_id() {
|
||||
additional_permissions: request.additional_permissions.clone(),
|
||||
})]
|
||||
);
|
||||
request.turn_environment.environment_id = "other".to_string();
|
||||
request.turn_environment.selection.environment_id = "other".to_string();
|
||||
let other_key = runtime
|
||||
.approval_action(&request, "call-1")
|
||||
.expect("build approval action")
|
||||
|
||||
@@ -168,7 +168,7 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
|
||||
) -> std::io::Result<ApprovalAction> {
|
||||
Ok(ApprovalAction::ExecCommand {
|
||||
id: call_id.to_string(),
|
||||
environment_id: req.turn_environment.environment_id.clone(),
|
||||
environment_id: req.turn_environment.selection.environment_id.clone(),
|
||||
command: req.command.clone(),
|
||||
hook_command: req.hook_command.clone(),
|
||||
cwd: req.cwd.clone(),
|
||||
@@ -237,7 +237,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecAttempt> for UnifiedExecRunt
|
||||
tty: Some(req.tty),
|
||||
},
|
||||
command: req.hook_command.clone(),
|
||||
environment_id: req.turn_environment.environment_id.clone(),
|
||||
environment_id: req.turn_environment.selection.environment_id.clone(),
|
||||
permission_profile: req.turn_environment.permission_profile().clone(),
|
||||
})
|
||||
}
|
||||
@@ -325,12 +325,12 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecAttempt> for UnifiedExecRunt
|
||||
let prepared = network
|
||||
.prepare_for_optional_environment(
|
||||
env,
|
||||
Some(&req.turn_environment.environment_id),
|
||||
Some(&req.turn_environment.selection.environment_id),
|
||||
)
|
||||
.map_err(|err| {
|
||||
ToolError::Codex(CodexErr::Io(io::Error::other(format!(
|
||||
"failed to prepare network proxy for environment `{}`: {err}",
|
||||
req.turn_environment.environment_id
|
||||
req.turn_environment.selection.environment_id
|
||||
))))
|
||||
})?;
|
||||
(prepared.env, Some(prepared.sandbox_context), None)
|
||||
@@ -419,7 +419,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecAttempt> for UnifiedExecRunt
|
||||
command,
|
||||
options,
|
||||
managed_network,
|
||||
Some(&req.turn_environment.environment_id),
|
||||
Some(&req.turn_environment.selection.environment_id),
|
||||
)
|
||||
.map_err(ToolError::Codex)?;
|
||||
exec_env.exec_server_env_config = req.exec_server_env_config.clone();
|
||||
@@ -495,7 +495,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecAttempt> for UnifiedExecRunt
|
||||
attempt,
|
||||
managed_network,
|
||||
network_proxy_launch,
|
||||
/*environment_id*/ Some(&req.turn_environment.environment_id),
|
||||
/*environment_id*/ Some(&req.turn_environment.selection.environment_id),
|
||||
req.exec_server_env_config.clone(),
|
||||
windows_sandbox_proxy_settings_mode,
|
||||
req.tty,
|
||||
@@ -520,6 +520,7 @@ mod tests {
|
||||
use codex_exec_server::Environment;
|
||||
use codex_exec_server::LOCAL_ENVIRONMENT_ID;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::TurnEnvironmentSelection;
|
||||
use codex_tools::ZshForkConfig;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
@@ -530,10 +531,12 @@ mod tests {
|
||||
|
||||
fn test_turn_environment(cwd: PathUri) -> TurnEnvironment {
|
||||
TurnEnvironment::new(
|
||||
LOCAL_ENVIRONMENT_ID.to_string(),
|
||||
TurnEnvironmentSelection {
|
||||
environment_id: LOCAL_ENVIRONMENT_ID.to_string(),
|
||||
cwd,
|
||||
workspace_roots: Vec::new(),
|
||||
},
|
||||
Arc::new(Environment::default_for_tests()),
|
||||
cwd,
|
||||
Vec::new(),
|
||||
/*shell*/ None,
|
||||
TurnEnvironmentConfig {
|
||||
allow_login_shell: true,
|
||||
@@ -578,12 +581,12 @@ mod tests {
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
);
|
||||
request.turn_environment.environment_id = "remote".to_string();
|
||||
request.turn_environment.selection.environment_id = "remote".to_string();
|
||||
let original_key = runtime
|
||||
.approval_action(&request, "call-1")
|
||||
.expect("build approval action")
|
||||
.cache_keys();
|
||||
request.turn_environment.environment_id = "other".to_string();
|
||||
request.turn_environment.selection.environment_id = "other".to_string();
|
||||
let other_key = runtime
|
||||
.approval_action(&request, "call-1")
|
||||
.expect("build approval action")
|
||||
|
||||
@@ -24,6 +24,7 @@ use codex_protocol::openai_models::WebSearchToolType;
|
||||
use codex_protocol::protocol::MultiAgentVersion;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::SubAgentSource;
|
||||
use codex_protocol::protocol::TurnEnvironmentSelection;
|
||||
use codex_tools::DiscoverablePluginInfo;
|
||||
use codex_tools::DiscoverableTool;
|
||||
use codex_tools::ResponsesApiNamespaceTool;
|
||||
@@ -373,7 +374,7 @@ fn duplicate_primary_environment(turn: &mut TurnContext) {
|
||||
.primary()
|
||||
.expect("primary environment")
|
||||
.clone();
|
||||
second_environment.environment_id = "secondary".to_string();
|
||||
second_environment.selection.environment_id = "secondary".to_string();
|
||||
turn.environments
|
||||
.environments
|
||||
.push(TurnEnvironmentState::Ready(second_environment));
|
||||
@@ -752,7 +753,7 @@ async fn shell_command_is_not_registered_without_a_single_local_environment() {
|
||||
else {
|
||||
panic!("primary environment should be ready");
|
||||
};
|
||||
environment.environment_id = "remote".to_string();
|
||||
environment.selection.environment_id = "remote".to_string();
|
||||
environment.environment = Arc::new(
|
||||
codex_exec_server::Environment::create_for_tests(Some(
|
||||
"ws://127.0.0.1:1/remote-exec-server".to_string(),
|
||||
@@ -902,15 +903,17 @@ async fn zsh_fork_unified_exec_keeps_shell_parameter_when_remote_environment_ava
|
||||
.environments
|
||||
.push(TurnEnvironmentState::Ready(
|
||||
crate::session::turn_context::TurnEnvironment::new(
|
||||
"remote".to_string(),
|
||||
TurnEnvironmentSelection {
|
||||
environment_id: "remote".to_string(),
|
||||
cwd: remote_cwd,
|
||||
workspace_roots: Vec::new(),
|
||||
},
|
||||
Arc::new(
|
||||
codex_exec_server::Environment::create_for_tests(Some(
|
||||
"ws://127.0.0.1:1/remote-exec-server".to_string(),
|
||||
))
|
||||
.expect("remote test environment"),
|
||||
),
|
||||
remote_cwd,
|
||||
Vec::new(),
|
||||
/*shell*/ None,
|
||||
crate::session::turn_context::TurnEnvironmentConfig {
|
||||
allow_login_shell: true,
|
||||
|
||||
Reference in New Issue
Block a user