From c7828dd0109c75bcb5b07dc2d95c68947e56239a Mon Sep 17 00:00:00 2001 From: sayan-oai Date: Fri, 18 Sep 2026 22:04:21 +0000 Subject: [PATCH] Keep step settings and approval environments consistent (#46556) ## Why Background processes can outlive their launching turn. Their approval reviews need the originating environment's filesystem restrictions, even when the active turn uses a different environment. Remote restrictions must also be resolved using executor paths rather than the local filesystem. ## What changed - Capture settings and environment selections together in `StepInputs`. Preserve the captured selection during active model updates while allowing environment startup to finish. - Carry environment IDs into Guardian network and `execve` requests, and use the owning process's environment with current review settings. Refresh environment readiness for network requests that fall back to the active turn. - Resolve remote denied paths and globs using the executor's policy context. Decline automatic approval when the request's environment is unavailable or an explicitly denied temporary directory cannot be resolved. ## Testing Add regression coverage for active model updates retaining their environment while the next turn adopts a new selection, background approvals using their owning environment, newly ready network environments, and remote denied paths and globs, including Windows paths and missing temporary-directory metadata. GitOrigin-RevId: b620d4b030c597a4f430fe0500864fa5469db3d4 --- codex-rs/core/src/agent/control/spawn.rs | 2 +- codex-rs/core/src/agent/control_tests.rs | 6 +- codex-rs/core/src/codex_delegate.rs | 2 +- codex-rs/core/src/codex_delegate_tests.rs | 8 +- codex-rs/core/src/codex_thread.rs | 4 +- .../core/src/guardian/approval_request.rs | 20 ++ codex-rs/core/src/guardian/mod.rs | 2 +- codex-rs/core/src/guardian/prompt.rs | 63 ++++-- codex-rs/core/src/guardian/runtime.rs | 19 +- codex-rs/core/src/guardian/tests.rs | 114 ++++++++++- codex-rs/core/src/mcp_openai_file.rs | 11 +- codex-rs/core/src/mcp_tool_call_tests.rs | 4 +- codex-rs/core/src/session/daemon_recovery.rs | 10 +- .../src/session/elicitation_holders_tests.rs | 2 +- codex-rs/core/src/session/mcp.rs | 4 +- codex-rs/core/src/session/mod.rs | 17 +- .../core/src/session/notification_tests.rs | 5 +- codex-rs/core/src/session/review.rs | 16 +- codex-rs/core/src/session/step_activation.rs | 16 +- .../core/src/session/step_activation_tests.rs | 34 ++-- codex-rs/core/src/session/step_context.rs | 8 + codex-rs/core/src/session/tests.rs | 192 +++++++++++------- .../core/src/session/tests/guardian_tests.rs | 18 +- codex-rs/core/src/session/turn_context.rs | 35 ++-- codex-rs/core/src/tasks/user_shell.rs | 2 +- codex-rs/core/src/thread_manager_tests.rs | 21 +- codex-rs/core/src/tools/approvals.rs | 4 + .../src/tools/handlers/extension_tools.rs | 2 +- .../src/tools/handlers/multi_agents_tests.rs | 8 +- .../src/tools/handlers/unified_exec_tests.rs | 4 +- .../core/src/tools/handlers/view_image.rs | 24 ++- codex-rs/core/src/tools/network_approval.rs | 32 +-- .../runtimes/zsh_fork/unix_escalation.rs | 6 +- codex-rs/core/src/tools/spec_plan_tests.rs | 33 +-- codex-rs/core/src/unified_exec/mod_tests.rs | 25 ++- .../src/unified_exec/process_manager_tests.rs | 6 +- .../src/unified_exec/stdin_approval_tests.rs | 24 ++- codex-rs/core/tests/suite/network_approval.rs | 106 +++++++++- codex-rs/core/tests/suite/remote_env.rs | 3 +- codex-rs/core/tests/suite/step_settings.rs | 1 + .../step_settings/environment_selection.rs | 116 +++++++++++ .../suite/unified_exec_zsh_fork_approvals.rs | 40 +++- codex-rs/protocol/src/permissions/target.rs | 43 ++++ .../protocol/src/permissions/target_tests.rs | 15 ++ 44 files changed, 872 insertions(+), 255 deletions(-) create mode 100644 codex-rs/core/tests/suite/step_settings/environment_selection.rs diff --git a/codex-rs/core/src/agent/control/spawn.rs b/codex-rs/core/src/agent/control/spawn.rs index 11793c7ceb..156e2dcb0b 100644 --- a/codex-rs/core/src/agent/control/spawn.rs +++ b/codex-rs/core/src/agent/control/spawn.rs @@ -405,7 +405,7 @@ impl LocalAgentControl { "cannot resume multi-agent v2 child {thread_id} with the current parent settings" )) })?; - Some((parent, turn.environments.clone())) + Some((parent, turn.initial_environments.clone())) } else { None }; diff --git a/codex-rs/core/src/agent/control_tests.rs b/codex-rs/core/src/agent/control_tests.rs index c36c58c5b4..9538ae7816 100644 --- a/codex-rs/core/src/agent/control_tests.rs +++ b/codex-rs/core/src/agent/control_tests.rs @@ -900,7 +900,7 @@ async fn check_v2_agent_reload(route: V2ReloadRoute) { .expect("known v2 agent should reload"), V2ReloadRoute::NestedParent => { let environment = parent_turn - .environments + .initial_environments .primary() .expect("parent environment"); let mut owner_config = environment.config().clone(); @@ -950,12 +950,12 @@ async fn check_v2_agent_reload(route: V2ReloadRoute) { let reloaded_turn = reloaded_child.session.new_default_turn().await; assert_eq!( ( - reloaded_turn.environments.to_selections(), + reloaded_turn.initial_environments.to_selections(), reloaded_turn.permission_profile(), reloaded_child.client_mcp_extensions(), ), ( - parent_turn.environments.to_selections(), + parent_turn.initial_environments.to_selections(), parent_turn.permission_profile(), client_mcp_extensions, ), diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 51007b553f..1aca5777c8 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -199,7 +199,7 @@ pub(crate) async fn run_codex_thread_one_shot( // requiring the caller to cancel the parent token. let child_cancel = cancel_token.child_token(); let parent_turn_id = parent_ctx.sub_id.clone(); - let parent_environments = parent_ctx.environments.clone(); + let parent_environments = parent_ctx.initial_environments.clone(); let root_turn_id = parent_ctx.turn_metadata_state.root_turn_id(); let (session, io) = Box::pin(run_codex_thread_interactive( config, diff --git a/codex-rs/core/src/codex_delegate_tests.rs b/codex-rs/core/src/codex_delegate_tests.rs index d2b5f31add..7b145dc07d 100644 --- a/codex-rs/core/src/codex_delegate_tests.rs +++ b/codex-rs/core/src/codex_delegate_tests.rs @@ -189,7 +189,7 @@ async fn run_codex_thread_interactive_respects_pre_cancelled_spawn() { config.permissions.approval_policy = Constrained::allow_only(AskForApproval::Never); let cancel_token = CancellationToken::new(); cancel_token.cancel(); - let parent_environments = parent_ctx.environments.clone(); + let parent_environments = parent_ctx.initial_environments.clone(); let result = timeout( Duration::from_secs(/*secs*/ 1), @@ -262,7 +262,7 @@ async fn delegate_start_analytics_honors_child_opt_out_with_enabled_parent() { Arc::clone(&parent_session.services.models_manager), Arc::clone(&parent_session), Arc::clone(&parent_ctx), - parent_ctx.environments.clone(), + parent_ctx.initial_environments.clone(), CancellationToken::new(), SubAgentSource::Review, codex_extension_api::SessionIsolation::Inherit, @@ -338,7 +338,7 @@ async fn delegate_isolation_does_not_depend_on_attribution() { Arc::clone(&parent_session.services.models_manager), Arc::clone(&parent_session), Arc::clone(&parent_ctx), - parent_ctx.environments.clone(), + parent_ctx.initial_environments.clone(), CancellationToken::new(), subagent_source, isolation, @@ -374,7 +374,7 @@ async fn run_codex_thread_interactive_rejects_approval_policy_that_can_prompt() crate::session::tests::make_session_and_context_with_rx().await; let mut config = parent_ctx.config.as_ref().clone(); config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); - let parent_environments = parent_ctx.environments.clone(); + let parent_environments = parent_ctx.initial_environments.clone(); let result = run_codex_thread_interactive( config, diff --git a/codex-rs/core/src/codex_thread.rs b/codex-rs/core/src/codex_thread.rs index 81fc3110b2..2f1d5323c3 100644 --- a/codex-rs/core/src/codex_thread.rs +++ b/codex-rs/core/src/codex_thread.rs @@ -824,13 +824,13 @@ impl CodexThread { /// Returns the active turn's reviewer, including live updates, or the thread default. pub async fn approvals_reviewer_for_turn(&self, turn_id: &str) -> ApprovalsReviewer { - if let Some((turn, settings, _)) = self + if let Some((turn, inputs, _)) = self .session .active_turn_context_and_strict_auto_review() .await && turn.sub_id == turn_id { - settings.approvals_reviewer() + inputs.settings.approvals_reviewer() } else { self.config_snapshot().await.approvals_reviewer } diff --git a/codex-rs/core/src/guardian/approval_request.rs b/codex-rs/core/src/guardian/approval_request.rs index e1c67e52b0..477d9eebfa 100644 --- a/codex-rs/core/src/guardian/approval_request.rs +++ b/codex-rs/core/src/guardian/approval_request.rs @@ -42,6 +42,7 @@ pub(crate) enum GuardianApprovalRequest { #[cfg(unix)] Execve { id: String, + environment_id: String, source: GuardianCommandSource, program: String, argv: Vec, @@ -57,6 +58,7 @@ pub(crate) enum GuardianApprovalRequest { NetworkAccess { id: String, turn_id: String, + environment_id: String, target: String, host: String, protocol: NetworkApprovalProtocol, @@ -84,6 +86,21 @@ pub(crate) enum GuardianApprovalRequest { }, } +impl GuardianApprovalRequest { + pub(super) fn background_environment_id(&self) -> Option<&str> { + match self { + Self::NetworkAccess { environment_id, .. } => Some(environment_id), + #[cfg(unix)] + Self::Execve { environment_id, .. } => Some(environment_id), + Self::ExecCommand { .. } + | Self::WriteStdin { .. } + | Self::ApplyPatch { .. } + | Self::McpToolCall { .. } + | Self::RequestPermissions { .. } => None, + } + } +} + #[derive(Debug, Clone, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] pub(crate) struct GuardianNetworkAccessTrigger { @@ -289,6 +306,7 @@ pub(crate) fn guardian_approval_request_to_json( #[cfg(unix)] GuardianApprovalRequest::Execve { id: _, + environment_id: _, source, program, argv, @@ -315,6 +333,7 @@ pub(crate) fn guardian_approval_request_to_json( GuardianApprovalRequest::NetworkAccess { id: _, turn_id: _, + environment_id: _, target, host, protocol, @@ -416,6 +435,7 @@ pub(crate) fn guardian_assessment_action( GuardianApprovalRequest::NetworkAccess { id: _id, turn_id: _turn_id, + environment_id: _, target, host, protocol, diff --git a/codex-rs/core/src/guardian/mod.rs b/codex-rs/core/src/guardian/mod.rs index afd2ee70af..c8765943a5 100644 --- a/codex-rs/core/src/guardian/mod.rs +++ b/codex-rs/core/src/guardian/mod.rs @@ -161,7 +161,7 @@ impl From> for GuardianReviewContext { .extension_data .get::() .map(|id| id.0.clone()), - environments: turn.environments.clone(), + environments: turn.initial_environments.clone(), model_info: Arc::clone(turn.model_info()), reasoning_effort: turn.reasoning_effort().cloned(), reasoning_summary: turn.reasoning_summary(), diff --git a/codex-rs/core/src/guardian/prompt.rs b/codex-rs/core/src/guardian/prompt.rs index f1e734d542..c666be0b20 100644 --- a/codex-rs/core/src/guardian/prompt.rs +++ b/codex-rs/core/src/guardian/prompt.rs @@ -27,6 +27,7 @@ use crate::context::NodeReplReviewEvidenceMode; use crate::context::node_repl_review_evidence_mode; use crate::event_mapping::is_contextual_user_message_content; use crate::session::session::Session; +use crate::session::turn_context::TurnEnvironment; use codex_utils_output_truncation::TruncationPolicy; use codex_utils_output_truncation::approx_bytes_for_tokens; use codex_utils_output_truncation::truncate_text; @@ -143,7 +144,9 @@ pub(crate) async fn build_guardian_prompt_items_with_parent_turn( ) }), }; - let permissions = parent_context.map(parent_turn_permissions); + let permissions = parent_context + .map(|context| parent_turn_permissions(context, &request)) + .transpose()?; let node_repl_snapshot = if node_repl_transcripts_enabled { session .services @@ -208,25 +211,61 @@ pub(crate) async fn build_guardian_prompt_items_with_parent_turn( }) } -fn parent_turn_permissions(context: &GuardianReviewContext) -> PermissionContext { +fn parent_turn_permissions( + context: &GuardianReviewContext, + request: &GuardianApprovalRequest, +) -> anyhow::Result { let turn = context.turn(); - let environment = context.environments().primary(); - #[allow(deprecated)] - let cwd = environment - .and_then(|environment| environment.cwd().to_abs_path().ok()) - .unwrap_or_else(|| turn.cwd.clone()); - let permission_profile = context - .environments() - .permission_profile_or_else(|| turn.permission_profile()); + let environment = match request.background_environment_id() { + Some(id) => Some( + context + .environments() + .turn_environments() + .find(|environment| environment.selection.environment_id == id) + .ok_or_else(|| anyhow::anyhow!("approval environment {id} is unavailable"))?, + ), + None => context.environments().primary(), + }; + let native_cwd = environment + .filter(|environment| !environment.environment.is_remote()) + .and_then(|environment| environment.cwd().to_abs_path().ok()); + let permission_profile = environment + .map(TurnEnvironment::permission_profile_with_workspace_roots) + .unwrap_or_else(|| turn.permission_profile()); let file_system_policy = permission_profile.file_system_sandbox_policy(); - PermissionContext { + // Remote restrictions must not be interpreted using the filesystem running Guardian. + // Older executors may not report their temp folders. If a rule explicitly denies those + // folders, decline automatic approval rather than guess. Default rules do not deny them. + if let Some(environment) = environment + && native_cwd.is_none() + { + let sandbox = environment.sandbox_context(/*additional_permissions*/ None); + let paths = sandbox.policy_context(); + let mut denied_globs = file_system_policy + .get_unreadable_globs_with_context(&paths) + .map_err(anyhow::Error::msg)?; + denied_globs.sort(); + denied_globs.dedup(); + return Ok(PermissionContext { + denied_paths: file_system_policy + .get_unreadable_roots_with_context(&paths) + .map_err(anyhow::Error::msg)? + .into_iter() + .map(|path| path.inferred_native_path_string()) + .collect(), + denied_globs, + }); + } + #[allow(deprecated)] + let cwd = native_cwd.unwrap_or_else(|| turn.cwd.clone()); + Ok(PermissionContext { denied_paths: file_system_policy .get_unreadable_roots_with_cwd(&cwd) .into_iter() .map(|root| root.to_string_lossy().into_owned()) .collect(), denied_globs: file_system_policy.get_unreadable_globs_with_cwd(&cwd), - } + }) } /// Exercises the sync profile through the host's existing transcript tests. diff --git a/codex-rs/core/src/guardian/runtime.rs b/codex-rs/core/src/guardian/runtime.rs index 272126d79f..118075ec68 100644 --- a/codex-rs/core/src/guardian/runtime.rs +++ b/codex-rs/core/src/guardian/runtime.rs @@ -58,15 +58,24 @@ impl ReviewAction { tracing::error!(%error, "failed to build automatic approval action"); ReviewDecision::denied("automatic approval review could not prepare the action") })?; - if let GuardianApprovalRequest::WriteStdin { environment_id, .. } = request + let environment_id = + if let GuardianApprovalRequest::WriteStdin { environment_id, .. } = request { + Some(environment_id.as_str()) + } else { + request.background_environment_id() + }; + if let Some(environment_id) = environment_id && !context .environments() .turn_environments() - .any(|environment| environment.selection.environment_id == *environment_id) + .any(|environment| environment.selection.environment_id == environment_id) { - return Err(ReviewDecision::denied( - "automatic approval review cannot access the terminal's environment; select it before retrying", - )); + let message = if matches!(request, GuardianApprovalRequest::WriteStdin { .. }) { + "automatic approval review cannot access the terminal's environment; select it before retrying" + } else { + "automatic approval review cannot access the request's environment" + }; + return Err(ReviewDecision::denied(message)); } Ok(request) } diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index 4f938f8e7e..c2c534b6db 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -517,7 +517,8 @@ async fn build_guardian_prompt_includes_parent_turn_denied_reads() -> anyhow::Re ]), NetworkSandboxPolicy::Restricted, ); - let TurnEnvironmentState::Ready(environment) = &mut turn.environments.environments[0] else { + let TurnEnvironmentState::Ready(environment) = &mut turn.initial_environments.environments[0] + else { panic!("parent environment should be ready"); }; environment.config_mut().permission_profile = @@ -565,6 +566,113 @@ async fn build_guardian_prompt_includes_parent_turn_denied_reads() -> anyhow::Re Ok(()) } +#[tokio::test] +async fn background_approval_permissions_use_the_owning_environment() -> anyhow::Result<()> { + let (session, mut turn) = crate::session::tests::make_session_and_context().await; + let mut secondary = turn.initial_environments.primary().unwrap().clone(); + let cwd = test_path_buf("/guardian-secondary").abs(); + let denied = cwd.join("private"); + secondary.selection.environment_id = "secondary".to_string(); + secondary.selection.cwd = PathUri::from_abs_path(&cwd); + secondary.config_mut().permission_profile = + PermissionProfileSnapshot::legacy(PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: PathUri::from_abs_path(&denied), + }, + access: FileSystemAccessMode::Deny, + missing_path_behavior: None, + }]), + NetworkSandboxPolicy::Restricted, + )); + let mut windows = secondary.clone(); + let windows_cwd = PathUri::parse("file:///C:/guardian-secondary")?; + windows.environment = Arc::new(codex_exec_server::Environment::create_for_tests(Some( + "ws://127.0.0.1:1".into(), + ))?); + windows.selection.environment_id = "windows".into(); + windows.selection.cwd = windows_cwd.clone(); + windows.config_mut().workspace_roots = vec![windows_cwd.clone()]; + windows.config_mut().permission_profile = + PermissionProfileSnapshot::legacy(PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry::new( + windows_cwd.join("private")?.into(), + FileSystemAccessMode::Deny, + ), + FileSystemSandboxEntry::new( + FileSystemPath::GlobPattern { + pattern: "*.secret".into(), + }, + FileSystemAccessMode::Deny, + ), + ]), + NetworkSandboxPolicy::Restricted, + )); + turn.initial_environments.environments.extend([ + TurnEnvironmentState::Ready(secondary), + TurnEnvironmentState::Ready(windows), + ]); + let turn = Arc::new(turn); + let context = GuardianReviewContext::from(&turn); + let network_request = |environment_id: &str| GuardianApprovalRequest::NetworkAccess { + id: "network".to_string(), + turn_id: turn.sub_id.clone(), + environment_id: environment_id.to_string(), + target: "https://example.com:443".to_string(), + host: "example.com".to_string(), + protocol: NetworkApprovalProtocol::Https, + port: 443, + trigger: None, + }; + let requests = [ + network_request("secondary"), + network_request("windows"), + #[cfg(unix)] + GuardianApprovalRequest::Execve { + id: "shell".to_string(), + environment_id: "secondary".to_string(), + source: codex_protocol::approvals::GuardianCommandSource::UnifiedExec, + program: "cat".to_string(), + argv: Vec::new(), + cwd, + additional_permissions: None, + }, + ]; + for request in requests { + let is_windows = request.background_environment_id() == Some("windows"); + let prompt = build_guardian_prompt_items_with_parent_turn( + &session, + session.conversation_history_snapshot().await.as_ref(), + Some(&context), + ApprovalRequestReasons::default(), + request, + GuardianPromptMode::Full, + /*reviewed_node_repl_evidence_sequence*/ 0, + ) + .await?; + let text = guardian_prompt_text(&prompt.context.into_user_inputs()?); + if is_windows { + assert!( + text.contains(r"path `C:\guardian-secondary\private`"), + "{text}" + ); + assert!( + text.contains(r"glob `C:\guardian-secondary\*.secret`"), + "{text}" + ); + } else { + assert!(text.contains(denied.to_string_lossy().as_ref())); + } + } + assert!( + runtime::ReviewAction::from(network_request("missing")) + .validate(&context) + .is_err() + ); + Ok(()) +} + #[tokio::test(flavor = "current_thread")] async fn build_guardian_prompt_delta_mode_preserves_original_numbering() -> anyhow::Result<()> { let (session, turn) = guardian_test_session_and_turn_with_base_url("http://localhost").await; @@ -1091,6 +1199,7 @@ fn guardian_approval_request_to_json_renders_network_access_trigger() -> serde_j let action = GuardianApprovalRequest::NetworkAccess { id: "network-1".to_string(), turn_id: "turn-1".to_string(), + environment_id: "local".to_string(), target: "https://example.com:443".to_string(), host: "example.com".to_string(), protocol: NetworkApprovalProtocol::Https, @@ -1141,6 +1250,7 @@ async fn build_guardian_prompt_items_explains_network_access_review_scope() -> a GuardianApprovalRequest::NetworkAccess { id: "network-1".to_string(), turn_id: "turn-1".to_string(), + environment_id: "local".to_string(), target: "https://example.com:443".to_string(), host: "example.com".to_string(), protocol: NetworkApprovalProtocol::Https, @@ -1339,6 +1449,7 @@ fn guardian_request_turn_id_prefers_network_access_owner_turn() { let network_access = GuardianApprovalRequest::NetworkAccess { id: "network-1".to_string(), turn_id: "owner-turn".to_string(), + environment_id: "local".to_string(), target: "https://example.com:443".to_string(), host: "example.com".to_string(), protocol: NetworkApprovalProtocol::Https, @@ -1368,6 +1479,7 @@ fn guardian_request_target_item_id_omits_network_access_trigger_call_id() { let network_access = GuardianApprovalRequest::NetworkAccess { id: "network-1".to_string(), turn_id: "owner-turn".to_string(), + environment_id: "local".to_string(), target: "https://example.com:443".to_string(), host: "example.com".to_string(), protocol: NetworkApprovalProtocol::Https, diff --git a/codex-rs/core/src/mcp_openai_file.rs b/codex-rs/core/src/mcp_openai_file.rs index 9958f035a3..103303ff92 100644 --- a/codex-rs/core/src/mcp_openai_file.rs +++ b/codex-rs/core/src/mcp_openai_file.rs @@ -280,7 +280,8 @@ mod tests { fn set_primary_environment_cwd(turn_context: &mut TurnContext, cwd: &Path) { let cwd = AbsolutePathBuf::try_from(cwd).expect("absolute path"); - let TurnEnvironmentState::Ready(primary) = &mut turn_context.environments.environments[0] + let TurnEnvironmentState::Ready(primary) = + &mut turn_context.initial_environments.environments[0] else { panic!("expected ready primary environment"); }; @@ -364,7 +365,7 @@ mod tests { .expect("write local file"); set_primary_environment_cwd(&mut turn_context, dir.path()); let environment = turn_context - .environments + .initial_environments .primary() .expect("ready primary environment"); let selection = environment.selection(); @@ -378,16 +379,16 @@ mod tests { /*non_blocking_snapshots*/ true, ); environments.update_selections(std::slice::from_ref(&selection), &environment_config); - turn_context.environments = environments.snapshot().await; + turn_context.initial_environments = environments.snapshot().await; turn_context - .environments + .initial_environments .starting() .next() .expect("environment should initially be starting") .wait_until_ready() .await .expect("environment should become ready"); - let step_environments = turn_context.environments.refresh_readiness(); + let step_environments = turn_context.initial_environments.refresh_readiness(); let mut config = (*turn_context.config).clone(); config.chatgpt_base_url = format!("{}/backend-api", server.uri()); diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index 699cc0472a..8fa73d4098 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -1331,13 +1331,13 @@ async fn mcp_sandbox_cwd_uses_matching_server_environment_uri() -> anyhow::Resul let (_, mut turn_context) = make_session_and_context().await; let secondary_cwd = PathUri::parse("file:///C:/remote/project")?; let environment = turn_context - .environments + .initial_environments .primary() .expect("primary environment") .environment .clone(); turn_context - .environments + .initial_environments .environments .push(TurnEnvironmentState::Ready(TurnEnvironment::new( TurnEnvironmentSelection { diff --git a/codex-rs/core/src/session/daemon_recovery.rs b/codex-rs/core/src/session/daemon_recovery.rs index 1c8fa947d6..ba7d23a85e 100644 --- a/codex-rs/core/src/session/daemon_recovery.rs +++ b/codex-rs/core/src/session/daemon_recovery.rs @@ -21,10 +21,13 @@ impl Session { let context = &task.turn_context; context.extension_data.get::()?; // Remote identities/configuration are not persisted across daemon restarts. - if context.environments.environments.len() != 1 { + if context.initial_environments.environments.len() != 1 { return None; } - let environment = context.environments.single_local_environment()?.selection(); + let environment = context + .initial_environments + .single_local_environment()? + .selection(); if environment.config != EnvironmentConfigState::FromThread { return None; } @@ -34,8 +37,9 @@ impl Session { final_output_json_schema: context.final_output_json_schema.clone(), service_tier: Some( context - .current_settings + .next_step_input .load() + .settings .service_tier .clone() .unwrap_or_else(|| { diff --git a/codex-rs/core/src/session/elicitation_holders_tests.rs b/codex-rs/core/src/session/elicitation_holders_tests.rs index 543bbaeae3..c1d8ab53f0 100644 --- a/codex-rs/core/src/session/elicitation_holders_tests.rs +++ b/codex-rs/core/src/session/elicitation_holders_tests.rs @@ -126,7 +126,7 @@ async fn permission_request_holds_an_elicitation_until_response() { let turn_context = turn_context.clone(); async move { let environment = turn_context - .environments + .initial_environments .primary() .expect("primary environment") .selection(); diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index e16f82c121..0aa971ed8f 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -733,7 +733,7 @@ async fn review_guardian_mcp_elicitation( let Some(mcp_config) = session.services.mcp_runtime.current_config() else { return Ok(None); }; - let step_settings = turn_context.current_settings.load_full(); + let step_settings = Arc::clone(&turn_context.next_step_input.load().settings); // User approval skips ordinary CUA checks, not separate sensitive requests. let user_cua_execution = step_settings.approvals_reviewer() == ApprovalsReviewer::User @@ -749,7 +749,7 @@ async fn review_guardian_mcp_elicitation( // Full Access skips inference, not the active-turn and cancellation checks. if (user_cua_execution - || turn_context.environments.has_full_access( + || turn_context.initial_environments.has_full_access( turn_context.approval_policy(), &turn_context .config diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index b96999f83f..7eb3a8bd69 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -3294,14 +3294,14 @@ impl Session { )] pub(crate) async fn active_turn_context_and_strict_auto_review( &self, - ) -> Option<(Arc, Arc, bool)> { + ) -> Option<(Arc, Arc, bool)> { let active = self.active_turn.lock().await; let active = active.as_ref()?; let task = active.task.as_ref()?; let turn_context = Arc::clone(&task.turn_context); - let step_settings = turn_context.current_settings.load_full(); + let step_inputs = turn_context.next_step_input.load_full(); let ts = active.turn_state.lock().await; - Some((turn_context, step_settings, ts.strict_auto_review_enabled())) + Some((turn_context, step_inputs, ts.strict_auto_review_enabled())) } pub(crate) async fn granted_session_permissions( @@ -3695,9 +3695,10 @@ impl Session { required_servers: &[String], required_plugins: &HashSet, ) -> CodexResult> { - // Capture once before asynchronous planning; all request consumers - // retain this immutable settings version even if the turn is updated. - let mut settings = turn_context.current_settings.load_full(); + // Capture settings and selection together before asynchronous planning. + // Existing steps retain this version even if the turn is updated. + let inputs = turn_context.next_step_input.load_full(); + let mut settings = Arc::clone(&inputs.settings); if matches!( turn_context.session_source, SessionSource::SubAgent(SubAgentSource::ThreadSpawn { .. }) @@ -3722,8 +3723,8 @@ impl Session { settings.model_info.as_ref(), ); let session_telemetry = settings.telemetry(&turn_context.session_telemetry); - // Keep selections fixed for the turn while allowing their startup work to finish. - let environments = turn_context.environments.refresh_readiness(); + // Refresh only the captured step selection, without adopting newer inputs. + let environments = inputs.environments.refresh_readiness(); let (loaded_agents_md, warnings) = self .services .agents_md_manager diff --git a/codex-rs/core/src/session/notification_tests.rs b/codex-rs/core/src/session/notification_tests.rs index a33bb9f60b..42f24ace69 100644 --- a/codex-rs/core/src/session/notification_tests.rs +++ b/codex-rs/core/src/session/notification_tests.rs @@ -45,7 +45,10 @@ async fn task_finish_preserves_notification_budget_and_queued_user_input() { let mut current = turn.initial_settings.as_ref().clone(); Arc::make_mut(&mut current.model_info).truncation_policy = codex_protocol::openai_models::TruncationPolicyConfig::tokens(/*limit*/ 400); - turn.current_settings.store(Arc::new(current)); + turn.next_step_input.store(Arc::new(StepInputs { + settings: Arc::new(current), + environments: turn.next_step_input.load().environments.clone(), + })); session .on_task_finished(Arc::clone(&turn), /*task_result*/ Ok(None)) diff --git a/codex-rs/core/src/session/review.rs b/codex-rs/core/src/session/review.rs index 84223bcc3f..df65e13704 100644 --- a/codex-rs/core/src/session/review.rs +++ b/codex-rs/core/src/session/review.rs @@ -1,3 +1,4 @@ +use super::step_context::StepInputs; use super::step_settings::ResolvedStepSettings; use super::*; use arc_swap::ArcSwap; @@ -116,7 +117,7 @@ pub(super) async fn spawn_review_thread( let review_turn_id = sub_id.to_string(); #[allow(deprecated)] let windows_sandbox_selection = parent_turn_context - .environments + .initial_environments .primary() .map(TurnEnvironment::windows_sandbox_selection_for_turn_metadata) .unwrap_or_else(|| { @@ -161,14 +162,17 @@ pub(super) async fn spawn_review_thread( initial_settings: Arc::clone(&step_settings), disabled_plugin_ids: parent_turn_context.disabled_plugin_ids.clone(), active_host_plugin_identities: None, - current_settings: ArcSwap::from(step_settings), + next_step_input: ArcSwap::from_pointee(StepInputs { + settings: step_settings, + environments: parent_turn_context.initial_environments.clone(), + }), session_telemetry: session_telemetry_for_context, provider: provider_for_context, session_source, history_mode: parent_turn_context.history_mode, parent_thread_id: parent_turn_context.parent_thread_id, originator: parent_turn_context.originator.clone(), - environments: parent_turn_context.environments.clone(), + initial_environments: parent_turn_context.initial_environments.clone(), available_models, unified_exec_shell_mode, current_date: parent_turn_context.current_date.clone(), @@ -202,7 +206,11 @@ pub(super) async fn spawn_review_thread( client_id: None, }]; let tc = Arc::new(review_turn_context); - if tc.environments.single_local_environment_cwd().is_some() { + if tc + .initial_environments + .single_local_environment_cwd() + .is_some() + { tc.turn_metadata_state .spawn_git_enrichment_task(Arc::clone(&sess.services.git_root_discovery)); } diff --git a/codex-rs/core/src/session/step_activation.rs b/codex-rs/core/src/session/step_activation.rs index e5038bbcb7..1086d937b8 100644 --- a/codex-rs/core/src/session/step_activation.rs +++ b/codex-rs/core/src/session/step_activation.rs @@ -2,6 +2,7 @@ use super::session::Session; use super::session::SessionConfiguration; +use super::step_context::StepInputs; use super::step_settings::ResolvedStepSettings; use super::step_settings::StepSettingsConstraints; use super::step_settings::StepSettingsUpdate; @@ -249,7 +250,7 @@ impl Session { ( Arc::clone(&task.turn_context), Arc::clone(&task.done), - task.turn_context.current_settings.load_full(), + task.turn_context.next_step_input.load_full(), ) }) }) @@ -277,7 +278,7 @@ impl Session { // settings rules. The task can progress, finish, or be cancelled while // preparation awaits; no publication locks are held here. let prepared = self - .prepare_step_settings_activation(&turn_context, ¤t, &update) + .prepare_step_settings_activation(&turn_context, ¤t.settings, &update) .await; let active = self.active_turn.lock().await; let Some(task) = active.as_ref().and_then(|active| active.task.as_ref()) else { @@ -288,7 +289,7 @@ impl Session { // A mismatch abandons the update without retrying or retargeting. if !Arc::ptr_eq(&task.done, &task_done) || !Arc::ptr_eq(&task.turn_context, &turn_context) - || !Arc::ptr_eq(&task.turn_context.current_settings.load_full(), ¤t) + || !Arc::ptr_eq(&task.turn_context.next_step_input.load_full(), ¤t) || task.cancellation_token.is_cancelled() { return TurnSettingsUpdateOutcome::TargetUnavailable; @@ -316,7 +317,7 @@ impl Session { } check_legacy_turn_safety( &turn_context, - ¤t, + ¤t.settings, &destination, &state.session_configuration.original_config_do_not_use, ) @@ -327,8 +328,11 @@ impl Session { // Publish the immutable snapshot. Frozen initial settings, existing step // captures, and future thread settings are not changed. task.turn_context - .current_settings - .store(Arc::new(destination)); + .next_step_input + .store(Arc::new(StepInputs { + settings: Arc::new(destination), + environments: current.environments.clone(), + })); TurnSettingsUpdateOutcome::Applied } diff --git a/codex-rs/core/src/session/step_activation_tests.rs b/codex-rs/core/src/session/step_activation_tests.rs index fb4e647457..27b9a31d4b 100644 --- a/codex-rs/core/src/session/step_activation_tests.rs +++ b/codex-rs/core/src/session/step_activation_tests.rs @@ -123,7 +123,7 @@ async fn instruction_refresh_serializes_reads_and_releases_on_cancellation() { session .services .agents_md_manager - .refresh(&turn.config, &turn.environments) + .refresh(&turn.config, &turn.initial_environments) .await .0 .expect("install initial provider"); @@ -195,7 +195,7 @@ async fn thread_instruction_refresh_serializes_reads_and_releases_on_cancellatio session .services .agents_md_manager - .refresh(&turn.config, &turn.environments) + .refresh(&turn.config, &turn.initial_environments) .await .0 .expect("install initial provider"); @@ -739,7 +739,7 @@ async fn delayed_activation_does_not_retarget_a_task(change: TaskChangeDuringLoo lookup, } = activation_fixture(models).await; let desired = desired_step_settings(&session).await; - let original = turn.current_settings.load_full(); + let original = turn.next_step_input.load_full(); let update_session = Arc::clone(&session); let turn_id = turn.sub_id.clone(); let update = tokio::spawn(async move { @@ -763,7 +763,7 @@ async fn delayed_activation_does_not_retarget_a_task(change: TaskChangeDuringLoo .expect("active task"); (task.cancellation_token.clone(), Arc::clone(&task.done)) }; - let (expected_turn, expected_settings) = match change { + let (expected_turn, expected_inputs) = match change { TaskChangeDuringLookup::CancelledWithRejectedDestination => { cancellation_token.cancel(); (Arc::clone(&turn), original) @@ -787,7 +787,7 @@ async fn delayed_activation_does_not_retarget_a_task(change: TaskChangeDuringLoo TaskChangeDuringLookup::FinishedAndReusedContext => Arc::clone(&turn), TaskChangeDuringLookup::CancelledWithRejectedDestination => unreachable!(), }; - let settings = replacement.current_settings.load_full(); + let inputs = replacement.next_step_input.load_full(); session .spawn_task( Arc::clone(&replacement), @@ -798,8 +798,8 @@ async fn delayed_activation_does_not_retarget_a_task(change: TaskChangeDuringLoo }, ) .await; - assert!(Arc::ptr_eq(&turn.current_settings.load_full(), &original)); - (replacement, settings) + assert!(Arc::ptr_eq(&turn.next_step_input.load_full(), &original)); + (replacement, inputs) } }; lookup.release(); @@ -808,8 +808,8 @@ async fn delayed_activation_does_not_retarget_a_task(change: TaskChangeDuringLoo TurnSettingsUpdateOutcome::TargetUnavailable ); assert!(Arc::ptr_eq( - &expected_turn.current_settings.load_full(), - &expected_settings, + &expected_turn.next_step_input.load_full(), + &expected_inputs, )); assert_eq!(desired_step_settings(&session).await, desired); session.abort_all_tasks(TurnAbortReason::Replaced).await; @@ -826,7 +826,7 @@ enum ManagedAuthorizationChange { #[tokio::test] async fn reviewer_only_activation_enforces_managed_authority(required_review: bool) { let ActivationFixture { session, turn, .. } = activation_fixture(activation_models()).await; - let original = turn.current_settings.load_full(); + let original = turn.next_step_input.load_full(); let desired = desired_step_settings(&session).await; let source = RequirementSource::Unknown; let mut sourced = ConfigRequirementsWithSources::default(); @@ -870,7 +870,7 @@ async fn reviewer_only_activation_enforces_managed_authority(required_review: bo .await, TurnSettingsUpdateOutcome::Rejected { .. } )); - assert!(Arc::ptr_eq(&turn.current_settings.load_full(), &original)); + assert!(Arc::ptr_eq(&turn.next_step_input.load_full(), &original)); assert_eq!(desired_step_settings(&session).await, desired); session.abort_all_tasks(TurnAbortReason::Replaced).await; } @@ -887,7 +887,7 @@ async fn delayed_activation_rechecks_live_managed_authorization( lookup, .. } = activation_fixture(activation_models()).await; - let original = turn.current_settings.load_full(); + let original = turn.next_step_input.load_full(); let desired = desired_step_settings(&session).await; let update_session = Arc::clone(&session); let turn_id = turn.sub_id.clone(); @@ -913,7 +913,7 @@ async fn delayed_activation_rechecks_live_managed_authorization( }; let sourced = match change { ManagedAuthorizationChange::ApprovalPolicy => { - let allowed = if original.approval_policy() == AskForApproval::Never { + let allowed = if original.settings.approval_policy() == AskForApproval::Never { AskForApproval::OnRequest } else { AskForApproval::Never @@ -924,7 +924,7 @@ async fn delayed_activation_rechecks_live_managed_authorization( } } ManagedAuthorizationChange::ApprovalsReviewer => { - let allowed = if original.approvals_reviewer() == ApprovalsReviewer::User { + let allowed = if original.settings.approvals_reviewer() == ApprovalsReviewer::User { ApprovalsReviewer::AutoReview } else { ApprovalsReviewer::User @@ -954,10 +954,10 @@ async fn delayed_activation_rechecks_live_managed_authorization( match change { ManagedAuthorizationChange::ApprovalPolicy => requirements .approval_policy - .can_set(&original.approval_policy()), + .can_set(&original.settings.approval_policy()), ManagedAuthorizationChange::ApprovalsReviewer => requirements .approvals_reviewer - .can_set(&original.approvals_reviewer()), + .can_set(&original.settings.approvals_reviewer()), } .expect_err("the refreshed constraint rejects the admitted value") }; @@ -968,7 +968,7 @@ async fn delayed_activation_rechecks_live_managed_authorization( reason: expected_error.to_string(), } ); - assert!(Arc::ptr_eq(&turn.current_settings.load_full(), &original,)); + assert!(Arc::ptr_eq(&turn.next_step_input.load_full(), &original)); assert_eq!(desired_step_settings(&session).await, desired); session.abort_all_tasks(TurnAbortReason::Replaced).await; } diff --git a/codex-rs/core/src/session/step_context.rs b/codex-rs/core/src/session/step_context.rs index 855d854fd0..080243f955 100644 --- a/codex-rs/core/src/session/step_context.rs +++ b/codex-rs/core/src/session/step_context.rs @@ -15,6 +15,14 @@ use codex_otel::SessionTelemetry; use codex_protocol::items::ModelInvocationContext; use codex_protocol::protocol::TurnContextItem; +/// Inputs for the next step, published together so capture cannot mix versions. +#[derive(Debug)] +pub(crate) struct StepInputs { + pub(crate) settings: Arc, + /// Selection is fixed within this version; attachment startup may still finish. + pub(crate) environments: TurnEnvironmentSnapshot, +} + /// Request-scoped state that may change between model sampling requests. pub(crate) struct StepContext { pub(crate) turn: Arc, diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 31762ab732..3bb0bfb433 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -2,6 +2,7 @@ mod notification_tests; use super::mcp_refresh::McpRefresh; +use super::step_context::StepInputs; use super::step_settings::ResolvedStepSettings; use super::step_settings::StepSettings; use super::step_settings::StepSettingsUpdate; @@ -244,12 +245,15 @@ pub(crate) fn update_turn_settings_for_test( update(&mut settings); let settings = Arc::new(settings); turn.initial_settings = Arc::clone(&settings); - turn.current_settings.store(settings); + turn.next_step_input.store(Arc::new(StepInputs { + settings, + environments: turn.initial_environments.clone(), + })); } impl StepContext { pub(crate) fn for_test(turn: Arc) -> Arc { - let environments = turn.environments.clone(); + let environments = turn.initial_environments.clone(); // Unit fixtures still customize the legacy Config directly. // Production capture instead takes the turn's current snapshot. let mut settings = turn.initial_settings.as_ref().clone(); @@ -1540,7 +1544,7 @@ async fn danger_full_access_tool_attempts_do_not_enforce_managed_network() -> an orchestrator .run( &mut tool, - turn.environments + turn.initial_environments .primary() .expect("turn should have a primary environment"), &tool_ctx, @@ -4308,20 +4312,23 @@ async fn turn_context_with_model_updates_model_fields() { }); Arc::make_mut(&mut turn_context.config).service_tier = turn_context.initial_settings.service_tier.clone(); - let captured = turn_context.current_settings.load_full(); - let mut current_selection = captured.selected().clone(); + let captured = turn_context.next_step_input.load_full(); + let mut current_selection = captured.settings.selected().clone(); current_selection.reasoning_summary = Some(ReasoningSummaryConfig::None); current_selection.service_tier = None; current_selection .collaboration_mode .settings .reasoning_effort = Some(ReasoningEffortConfig::High); - let current = Arc::new(ResolvedStepSettings::new( - Arc::new(current_selection), - Arc::clone(turn_context.model_info()), - /*fast_mode_enabled*/ true, - )); - turn_context.current_settings.store(Arc::clone(¤t)); + let current = Arc::new(StepInputs { + settings: Arc::new(ResolvedStepSettings::new( + Arc::new(current_selection), + Arc::clone(turn_context.model_info()), + /*fast_mode_enabled*/ true, + )), + environments: captured.environments.clone(), + }); + turn_context.next_step_input.store(Arc::clone(¤t)); let updated = turn_context .with_model("gpt-5.5".to_string(), &session.services.models_manager) .await; @@ -4356,19 +4363,22 @@ async fn turn_context_with_model_updates_model_fields() { Some(ServiceTier::Fast.request_value()) ), ); - assert!(Arc::ptr_eq(&captured, &turn_context.initial_settings)); + assert!(Arc::ptr_eq( + &captured.settings, + &turn_context.initial_settings + )); assert!(Arc::ptr_eq( ¤t, - &turn_context.current_settings.load_full() + &turn_context.next_step_input.load_full() )); - assert!(!Arc::ptr_eq(&captured, &updated.initial_settings)); + assert!(!Arc::ptr_eq(&captured.settings, &updated.initial_settings)); assert!(Arc::ptr_eq( &updated.initial_settings, - &updated.current_settings.load_full() + &updated.next_step_input.load().settings )); assert!(!Arc::ptr_eq( - &updated.current_settings.load_full(), - &turn_context.current_settings.load_full() + &updated.next_step_input.load_full(), + &turn_context.next_step_input.load_full() )); assert_eq!(updated.config.model.as_deref(), Some("gpt-5.5")); assert_eq!(updated.collaboration_mode().model(), "gpt-5.5"); @@ -5661,7 +5671,7 @@ async fn permission_profile_updates_apply_to_next_turn_environment() { for apply_on_turn_start in [false, true] { let (session, active_turn) = make_session_and_context().await; let active_environment_config = active_turn - .environments + .initial_environments .primary() .expect("active turn environment") .config() @@ -5693,7 +5703,7 @@ async fn permission_profile_updates_apply_to_next_turn_environment() { session.new_default_turn().await }; let next_environment = next_turn - .environments + .initial_environments .primary() .expect("next turn environment"); let mut expected_environment_config = active_environment_config.clone(); @@ -5707,7 +5717,7 @@ async fn permission_profile_updates_apply_to_next_turn_environment() { assert_eq!(next_environment.config(), &expected_environment_config); assert_eq!( active_turn - .environments + .initial_environments .primary() .expect("active turn environment") .config(), @@ -5805,7 +5815,13 @@ async fn absolute_cwd_update_with_turn_environment_is_allowed() { let turn_cwd = turn_context.cwd.clone(); assert_eq!(turn_cwd, absolute_cwd); assert_eq!(turn_context.config.cwd, absolute_cwd); - assert_eq!(turn_context.environments.turn_environments().count(), 1); + assert_eq!( + turn_context + .initial_environments + .turn_environments() + .count(), + 1 + ); } #[tokio::test] @@ -6930,7 +6946,7 @@ async fn request_permissions_emits_event_when_granular_policy_allows_requests() let call_id = call_id.clone(); async move { let environment = turn_context - .environments + .initial_environments .primary() .expect("primary environment") .selection(); @@ -7007,7 +7023,7 @@ async fn request_permissions_tool_resolves_legacy_paths_against_selected_environ })) .expect("test setup should allow updating approval policy"); let current_environment = turn_context_mut - .environments + .initial_environments .primary() .expect("primary environment") .clone(); @@ -7026,7 +7042,8 @@ async fn request_permissions_tool_resolves_legacy_paths_against_selected_environ current_environment.shell, ) }; - turn_context_mut.environments.environments[0] = TurnEnvironmentState::Ready(environment); + turn_context_mut.initial_environments.environments[0] = + TurnEnvironmentState::Ready(environment); let call_id = "call-1".to_string(); let handler = RequestPermissionsHandler; @@ -7115,7 +7132,8 @@ async fn request_permissions_tool_rejects_invalid_requests( Arc::make_mut(&mut turn_context.config) .permissions .approval_policy = codex_config::Constrained::allow_any(AskForApproval::Never); - let TurnEnvironmentState::Ready(environment) = &mut turn_context.environments.environments[0] + let TurnEnvironmentState::Ready(environment) = + &mut turn_context.initial_environments.environments[0] else { panic!("turn environment should be ready"); }; @@ -7193,7 +7211,7 @@ async fn request_permissions_response_materializes_session_cwd_grants_before_rec let requested_permissions = requested_permissions.clone(); async move { let environment = turn_context - .environments + .initial_environments .primary() .expect("primary environment") .selection(); @@ -7286,7 +7304,7 @@ async fn request_permissions_is_auto_denied_when_granular_policy_blocks_tool_req let turn_context = Arc::new(turn_context); let call_id = "call-1".to_string(); let environment = turn_context - .environments + .initial_environments .primary() .expect("primary environment") .selection(); @@ -7490,10 +7508,10 @@ async fn turn_environments_set_primary_environment() { .await .expect("turn should start"); - let turn_environments = &turn_context.environments; + let turn_environments = &turn_context.initial_environments; assert_eq!(turn_environments.turn_environments().count(), 1); let turn_environment = turn_context - .environments + .initial_environments .primary() .expect("primary environment should be set"); assert!(std::sync::Arc::ptr_eq( @@ -7505,7 +7523,7 @@ async fn turn_environments_set_primary_environment() { )); assert!( turn_context - .environments + .initial_environments .turn_environments() .next() .is_some() @@ -7533,7 +7551,7 @@ async fn turn_environments_set_primary_environment() { assert!(Arc::ptr_eq( &stored_environment, &default_turn - .environments + .initial_environments .primary() .expect("default turn primary environment") .environment @@ -7608,10 +7626,10 @@ async fn task_turn_does_not_overlay_legacy_fallback_cwd_onto_stored_thread_envir .new_turn_with_default_settings("task".into(), Default::default()) .await; - let turn_environments = &turn_context.environments; + let turn_environments = &turn_context.initial_environments; assert_eq!(turn_environments.turn_environments().count(), 1); let turn_environment = turn_context - .environments + .initial_environments .primary() .expect("primary environment should be set"); assert!(std::sync::Arc::ptr_eq( @@ -7646,10 +7664,10 @@ async fn task_turn_honors_empty_stored_thread_environments() { .new_turn_with_default_settings("task".into(), Default::default()) .await; - assert!(turn_context.environments.primary().is_none()); + assert!(turn_context.initial_environments.primary().is_none()); assert!( turn_context - .environments + .initial_environments .turn_environments() .next() .is_none() @@ -7658,14 +7676,20 @@ async fn task_turn_honors_empty_stored_thread_environments() { let turn_cwd = turn_context.cwd.clone(); assert_eq!(turn_cwd, session_cwd); assert_eq!(turn_context.config.cwd, session_cwd); - assert_eq!(turn_context.environments.turn_environments().count(), 0); + assert_eq!( + turn_context + .initial_environments + .turn_environments() + .count(), + 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 + .initial_environments .primary() .expect("primary environment") .clone(); @@ -7674,7 +7698,7 @@ async fn primary_environment_uses_first_turn_environment() { let second_cwd_uri = codex_utils_path_uri::PathUri::from_abs_path(&second_cwd); let first_environment_config = first_environment.config().clone(); turn_context - .environments + .initial_environments .environments .push(TurnEnvironmentState::Ready(TurnEnvironment::new( TurnEnvironmentSelection { @@ -7690,7 +7714,7 @@ async fn primary_environment_uses_first_turn_environment() { assert_eq!( turn_context - .environments + .initial_environments .primary() .expect("primary environment") .selection @@ -7699,17 +7723,23 @@ async fn primary_environment_uses_first_turn_environment() { ); assert_eq!( turn_context - .environments + .initial_environments .turn_environments() .find(|environment| environment.selection.environment_id == "second") .expect("second environment") .cwd(), &second_cwd_uri ); - assert_eq!(turn_context.environments.turn_environments().count(), 2); assert_eq!( turn_context - .environments + .initial_environments + .turn_environments() + .count(), + 2 + ); + assert_eq!( + turn_context + .initial_environments .turn_environments() .nth(1) .expect("second environment") @@ -7737,10 +7767,10 @@ async fn empty_turn_environments_clear_primary_environment() { .await .expect("turn should start"); - assert!(turn_context.environments.primary().is_none()); + assert!(turn_context.initial_environments.primary().is_none()); assert!( turn_context - .environments + .initial_environments .turn_environments() .next() .is_none() @@ -8879,7 +8909,7 @@ async fn mcp_elicitation_reviewer_uses_active_reviewer_and_latest_runtime_policy .next() .expect("session should select its executor environment"); let mut owner_config = old_turn - .environments + .initial_environments .primary() .expect("ready environment") .config() @@ -9038,7 +9068,7 @@ async fn conflicting_ready_environment_root_ids_keep_first_location() { selected_root("executor-b", "file:///plugins/b"), ]; let local_environment = turn_context - .environments + .initial_environments .primary() .expect("ready local environment"); let mut turn_environments = Vec::new(); @@ -9111,7 +9141,7 @@ async fn capability_discovery_uses_environment_permission_profile() { .expect("disable legacy Landlock"); turn_context.windows_sandbox_level = WindowsSandboxLevel::RestrictedToken; let mut environment = turn_context - .environments + .initial_environments .primary() .expect("primary environment") .clone(); @@ -9146,13 +9176,13 @@ async fn capability_discovery_uses_environment_permission_profile() { use_legacy_landlock: true, }; let environment_id = environment.selection.environment_id.clone(); - turn_context.environments.environments[0] = TurnEnvironmentState::Ready(environment); + turn_context.initial_environments.environments[0] = TurnEnvironmentState::Ready(environment); let discovery = session .executor_capability_discovery_for_step( &turn_context.config, /*ready_selected_capability_roots*/ &[], - &turn_context.environments, + &turn_context.initial_environments, ) .await .expect("restricted environment should trigger capability discovery"); @@ -9331,12 +9361,12 @@ async fn record_context_updates_emits_environment_item_for_cwd_changes() { .await; let cwd = test_path_buf("/new-repo").abs(); let environment = current_context - .environments + .initial_environments .primary() .expect("primary environment") .clone(); let environment_config = environment.config().clone(); - current_context.environments.environments[0] = + current_context.initial_environments.environments[0] = TurnEnvironmentState::Ready(TurnEnvironment::new( TurnEnvironmentSelection { environment_id: environment.selection.environment_id, @@ -9371,13 +9401,13 @@ async fn record_context_updates_use_environment_permission_profile_and_workspace .set_permission_profile(PermissionProfile::Disabled) .expect("unrestricted permission profile should be allowed"); let mut previous_environment = previous_context - .environments + .initial_environments .primary() .expect("primary environment") .clone(); previous_environment.config_mut().permission_profile = PermissionProfileSnapshot::legacy(PermissionProfile::Disabled); - previous_context.environments.environments[0] = + previous_context.initial_environments.environments[0] = TurnEnvironmentState::Ready(previous_environment); let previous_context = Arc::new(previous_context); let mut current_context = previous_context @@ -9387,7 +9417,7 @@ async fn record_context_updates_use_environment_permission_profile_and_workspace ) .await; let environment = current_context - .environments + .initial_environments .primary() .expect("primary environment") .clone(); @@ -9397,7 +9427,7 @@ async fn record_context_updates_use_environment_permission_profile_and_workspace environment_config.workspace_roots = vec![PathUri::from_abs_path(&workspace_root)]; environment_config.permission_profile = PermissionProfileSnapshot::legacy(PermissionProfile::workspace_write()); - current_context.environments.environments[0] = + current_context.initial_environments.environments[0] = TurnEnvironmentState::Ready(TurnEnvironment::new( TurnEnvironmentSelection { environment_id: environment.selection.environment_id, @@ -9469,12 +9499,12 @@ async fn record_context_updates_omits_environment_item_when_disabled() { config.include_environment_context = false; current_context.config = Arc::new(config); let environment = current_context - .environments + .initial_environments .primary() .expect("primary environment") .clone(); let environment_config = environment.config().clone(); - current_context.environments.environments[0] = + current_context.initial_environments.environments[0] = TurnEnvironmentState::Ready(TurnEnvironment::new( TurnEnvironmentSelection { environment_id: environment.selection.environment_id, @@ -10029,23 +10059,24 @@ fn file_system_policy_with_unreadable_glob(turn_context: &TurnContext) -> FileSy async fn turn_context_item_stores_local_cwd() { let (_session, mut turn_context) = make_session_and_context().await; let environment = turn_context - .environments + .initial_environments .primary() .expect("primary environment") .clone(); let cwd = PathUri::parse("file:///C:/windows").expect("Windows cwd URI"); let environment_config = environment.config().clone(); - turn_context.environments.environments[0] = TurnEnvironmentState::Ready(TurnEnvironment::new( - TurnEnvironmentSelection { - environment_id: "remote".to_string(), - cwd, - workspace_roots: Vec::new(), - config: EnvironmentConfigState::Ready(environment_config), - }, - environment.config_origin, - environment.environment, - environment.shell, - )); + turn_context.initial_environments.environments[0] = + TurnEnvironmentState::Ready(TurnEnvironment::new( + TurnEnvironmentSelection { + environment_id: "remote".to_string(), + cwd, + workspace_roots: Vec::new(), + config: EnvironmentConfigState::Ready(environment_config), + }, + environment.config_origin, + environment.environment, + environment.shell, + )); #[allow(deprecated)] let local_cwd = turn_context.cwd.clone(); @@ -10069,7 +10100,8 @@ async fn turn_context_item_omits_legacy_equivalent_file_system_sandbox_policy() async fn turn_context_item_stores_active_permission_profile() { let (_session, mut turn_context) = make_session_and_context().await; let active_permission_profile = ActivePermissionProfile::read_only(); - let TurnEnvironmentState::Ready(environment) = &mut turn_context.environments.environments[0] + let TurnEnvironmentState::Ready(environment) = + &mut turn_context.initial_environments.environments[0] else { panic!("turn environment should be ready"); }; @@ -10096,7 +10128,8 @@ async fn turn_context_item_stores_split_file_system_sandbox_policy_when_differen &file_system_sandbox_policy, turn_context.network_sandbox_policy(), ); - let TurnEnvironmentState::Ready(environment) = &mut turn_context.environments.environments[0] + let TurnEnvironmentState::Ready(environment) = + &mut turn_context.initial_environments.environments[0] else { panic!("turn environment should be ready"); }; @@ -10275,7 +10308,8 @@ async fn record_context_updates_and_set_reference_context_item_persists_split_fi &file_system_sandbox_policy, turn_context.network_sandbox_policy(), ); - let TurnEnvironmentState::Ready(environment) = &mut turn_context.environments.environments[0] + let TurnEnvironmentState::Ready(environment) = + &mut turn_context.initial_environments.environments[0] else { panic!("turn environment should be ready"); }; @@ -10375,13 +10409,14 @@ async fn build_initial_context_uses_retained_step_after_model_change() { .as_mut() .unwrap() .instructions_template = Some("B instructions".to_string()); - turn_context - .current_settings - .store(Arc::new(ResolvedStepSettings::new( + turn_context.next_step_input.store(Arc::new(StepInputs { + settings: Arc::new(ResolvedStepSettings::new( Arc::new(selected_b), Arc::new(model_b), /*fast_mode_enabled*/ false, - ))); + )), + environments: step_a.environments.clone(), + })); let step_b = session .capture_step_context(Arc::clone(&turn_context), &CancellationToken::new()) .await @@ -11448,7 +11483,10 @@ async fn task_finish_emits_turn_item_lifecycle_for_leftover_pending_user_input() let mut current = tc.initial_settings.as_ref().clone(); Arc::make_mut(&mut current.model_info).supports_image_detail_original = true; - tc.current_settings.store(Arc::new(current)); + tc.next_step_input.store(Arc::new(StepInputs { + settings: Arc::new(current), + environments: tc.next_step_input.load().environments.clone(), + })); sess.on_task_finished(Arc::clone(&tc), /*task_result*/ Ok(None)) .await; diff --git a/codex-rs/core/src/session/tests/guardian_tests.rs b/codex-rs/core/src/session/tests/guardian_tests.rs index a4cbf12b4b..a2888d782b 100644 --- a/codex-rs/core/src/session/tests/guardian_tests.rs +++ b/codex-rs/core/src/session/tests/guardian_tests.rs @@ -253,7 +253,7 @@ async fn request_permissions_routes_to_guardian_when_reviewer_is_enabled() { ..RequestPermissionProfile::default() }; let environment = turn_context - .environments + .initial_environments .primary() .expect("primary environment") .selection(); @@ -459,7 +459,7 @@ async fn request_permissions_guardian_review_stops_when_cancelled( let cancellation_token = cancellation_token.clone(); async move { let environment = turn_context - .environments + .initial_environments .primary() .expect("primary environment") .selection(); @@ -568,7 +568,7 @@ async fn guardian_allows_exec_command_additional_permissions_requests_past_polic .set_permission_profile(codex_protocol::models::PermissionProfile::Disabled) .expect("test setup should allow disabling the permission profile"); let TurnEnvironmentState::Ready(environment) = - &mut turn_context_raw.environments.environments[0] + &mut turn_context_raw.initial_environments.environments[0] else { panic!("primary environment should be ready"); }; @@ -692,7 +692,7 @@ async fn strict_auto_review_turn_grant_forces_guardian_for_exec_command_policy_s }) .expect("test setup should allow external sandbox permissions"); let TurnEnvironmentState::Ready(environment) = - &mut turn_context_raw.environments.environments[0] + &mut turn_context_raw.initial_environments.environments[0] else { panic!("primary environment should be ready"); }; @@ -798,7 +798,8 @@ async fn network_approval_uses_published_task_authority_within_same_turn( .task .as_ref() .expect("active task"); - let mut settings = task.turn_context.current_settings.load_full(); + let current = task.turn_context.next_step_input.load_full(); + let mut settings = Arc::clone(¤t.settings); update_selected_settings_for_test(Arc::make_mut(&mut settings), |selected| { selected .approval_policy @@ -806,7 +807,12 @@ async fn network_approval_uses_published_task_authority_within_same_turn( .expect("update policy"); selected.approvals_reviewer = ApprovalsReviewer::User; }); - task.turn_context.current_settings.store(settings); + task.turn_context + .next_step_input + .store(Arc::new(StepInputs { + settings, + environments: current.environments.clone(), + })); } let decision = session .services diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index fcbafa413a..fbebe5f43e 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -1,3 +1,4 @@ +use super::step_context::StepInputs; use super::step_settings::ResolvedStepSettings; use super::token_budget::has_explicit_settings; use super::token_budget::resolve_token_budget; @@ -320,8 +321,8 @@ pub struct TurnContext { /// Thread-owned plugin selection captured when this turn was admitted. pub(crate) disabled_plugin_ids: Vec, pub(super) active_host_plugin_identities: Option>, - /// Snapshot for the next step; request consumers use their captured StepContext. - pub(super) current_settings: ArcSwap, + /// Inputs for the next step; request consumers use their captured StepContext. + pub(super) next_step_input: ArcSwap, /// Turn-wide telemetry; model-attributed step work should use `StepContext::session_telemetry`. pub(crate) session_telemetry: SessionTelemetry, pub(crate) provider: SharedModelProvider, @@ -329,7 +330,9 @@ pub struct TurnContext { pub(crate) history_mode: ThreadHistoryMode, pub(crate) parent_thread_id: Option, pub(crate) originator: String, - pub(crate) environments: TurnEnvironmentSnapshot, + /// Initial selection retained for legacy turn consumers. Step work uses StepContext. + // TODO(sayan): Migrate all remaining consumers to next_step_input's environments. + pub(crate) initial_environments: TurnEnvironmentSnapshot, /// 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()`. @@ -378,7 +381,7 @@ enum TurnContextBuildMode { impl TurnContext { /// Captures current model metadata without preparing a step. pub(crate) fn capture_current_model_info(&self) -> Arc { - Arc::clone(&self.current_settings.load().model_info) + Arc::clone(&self.next_step_input.load().settings.model_info) } /// Legacy: returns the frozen initial-turn model metadata. @@ -534,7 +537,7 @@ impl TurnContext { /// Returns the selected environment's permissions, or the thread's permissions when none is ready. pub(crate) fn permission_profile(&self) -> PermissionProfile { - self.environments + self.initial_environments .permission_profile_or_else(|| self.config.permissions.effective_permission_profile()) } @@ -556,7 +559,7 @@ impl TurnContext { /// Combines the selected environment's workspace roots with its permission profile roots. pub(crate) fn effective_workspace_roots(&self) -> Vec { - let Some(environment) = self.environments.primary() else { + let Some(environment) = self.initial_environments.primary() else { return self.config.effective_workspace_roots(); }; @@ -673,14 +676,17 @@ impl TurnContext { initial_settings: Arc::clone(&step_settings), disabled_plugin_ids: self.disabled_plugin_ids.clone(), active_host_plugin_identities: self.active_host_plugin_identities.clone(), - current_settings: ArcSwap::from(step_settings), + next_step_input: ArcSwap::from_pointee(StepInputs { + settings: step_settings, + environments: self.initial_environments.clone(), + }), session_telemetry, provider: self.provider.clone(), session_source: self.session_source.clone(), history_mode: self.history_mode, parent_thread_id: self.parent_thread_id, originator: self.originator.clone(), - environments: self.environments.clone(), + initial_environments: self.initial_environments.clone(), #[allow(deprecated)] cwd: self.cwd.clone(), current_date: self.current_date.clone(), @@ -731,7 +737,7 @@ impl TurnContext { // The legacy rollout field still stores host-native paths. Keep its // runtime-root filtering and omit it for unrepresentable profile roots; // the authoritative permission profile retains its concrete entries. - let profile_roots = self.environments.primary().map_or_else( + let profile_roots = self.initial_environments.primary().map_or_else( || self.config.permissions.profile_workspace_roots(), |environment| { environment @@ -767,7 +773,7 @@ impl TurnContext { approvals_reviewer: Some(self.config.approvals_reviewer), sandbox_policy: self.sandbox_policy(), permission_profile: Some(self.permission_profile()), - active_permission_profile: self.environments.primary().map_or_else( + active_permission_profile: self.initial_environments.primary().map_or_else( || self.config.permissions.active_permission_profile(), TurnEnvironment::active_permission_profile, ), @@ -989,14 +995,17 @@ impl Session { initial_settings: Arc::clone(&step_settings), disabled_plugin_ids: session_configuration.disabled_plugin_ids.clone(), active_host_plugin_identities: None, - current_settings: ArcSwap::from(step_settings), + next_step_input: ArcSwap::from_pointee(StepInputs { + settings: step_settings, + environments: environments.clone(), + }), session_telemetry: session_telemetry_for_context, provider, session_source, history_mode: session_configuration.history_mode, parent_thread_id: session_configuration.parent_thread_id, originator: session_configuration.originator.clone(), - environments, + initial_environments: environments, #[allow(deprecated)] cwd, current_date: Some(current_date), @@ -1260,7 +1269,7 @@ impl Session { let turn_context = Arc::new(turn_context); if git_enrichment_policy == GitEnrichmentPolicy::Fresh && turn_context - .environments + .initial_environments .single_local_environment_cwd() .is_some() { diff --git a/codex-rs/core/src/tasks/user_shell.rs b/codex-rs/core/src/tasks/user_shell.rs index d96849589d..80cbeef485 100644 --- a/codex-rs/core/src/tasks/user_shell.rs +++ b/codex-rs/core/src/tasks/user_shell.rs @@ -126,7 +126,7 @@ pub(crate) async fn execute_user_shell_command( } let Some((turn_environment, environment_shell)) = turn_context - .environments + .initial_environments .local() .and_then(|environment| environment.shell.as_ref().map(|shell| (environment, shell))) else { diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index c8df7f0721..67e6c3a484 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -1955,10 +1955,16 @@ async fn resume_and_fork_do_not_restore_thread_environments_from_rollout() { .await .expect("build resumed turn context"); let resumed_turn = prepared_turn; - assert_eq!(resumed_turn.environments.turn_environments().count(), 1); assert_eq!( resumed_turn - .environments + .initial_environments + .turn_environments() + .count(), + 1 + ); + assert_eq!( + resumed_turn + .initial_environments .primary() .expect("primary environment") .cwd(), @@ -1966,7 +1972,7 @@ async fn resume_and_fork_do_not_restore_thread_environments_from_rollout() { ); assert_ne!( resumed_turn - .environments + .initial_environments .primary() .expect("primary environment") .cwd(), @@ -1992,10 +1998,13 @@ async fn resume_and_fork_do_not_restore_thread_environments_from_rollout() { .await .expect("build forked turn context"); let forked_turn = prepared_turn; - assert_eq!(forked_turn.environments.turn_environments().count(), 1); + assert_eq!( + forked_turn.initial_environments.turn_environments().count(), + 1 + ); assert_eq!( forked_turn - .environments + .initial_environments .primary() .expect("primary environment") .cwd(), @@ -2003,7 +2012,7 @@ async fn resume_and_fork_do_not_restore_thread_environments_from_rollout() { ); assert_ne!( forked_turn - .environments + .initial_environments .primary() .expect("primary environment") .cwd(), diff --git a/codex-rs/core/src/tools/approvals.rs b/codex-rs/core/src/tools/approvals.rs index 0ef1bbb4a1..6cad5a90ec 100644 --- a/codex-rs/core/src/tools/approvals.rs +++ b/codex-rs/core/src/tools/approvals.rs @@ -333,6 +333,7 @@ impl ApprovalAction { #[cfg(unix)] Self::Execve { id, + environment_id, source, program, argv, @@ -341,6 +342,7 @@ impl ApprovalAction { .. } => crate::guardian::GuardianApprovalRequest::Execve { id, + environment_id, source, program: program.to_string_lossy().into_owned(), argv, @@ -388,6 +390,7 @@ impl ApprovalAction { Self::NetworkAccess { id, turn_id, + environment_id, target, host, protocol, @@ -397,6 +400,7 @@ impl ApprovalAction { } => crate::guardian::GuardianApprovalRequest::NetworkAccess { id, turn_id, + environment_id, target, host, protocol, diff --git a/codex-rs/core/src/tools/handlers/extension_tools.rs b/codex-rs/core/src/tools/handlers/extension_tools.rs index 0fcb6165c2..83f3310152 100644 --- a/codex-rs/core/src/tools/handlers/extension_tools.rs +++ b/codex-rs/core/src/tools/handlers/extension_tools.rs @@ -430,7 +430,7 @@ mod tests { let model = turn.model_info().slug.clone(); let truncation_policy = turn.model_info().truncation_policy.into(); let expected_sandbox_cwds = turn - .environments + .initial_environments .turn_environments() .map(|environment| environment.cwd().clone()) .collect::>(); diff --git a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs index 8e8ad7d76d..e4e027cc8b 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs @@ -2216,7 +2216,7 @@ async fn spawn_agent_reapplies_runtime_sandbox_after_role_config() { .await .expect("non-empty role config should apply"); let TurnEnvironmentState::Ready(environment) = turn - .environments + .initial_environments .environments .first_mut() .expect("parent environment should exist") @@ -4203,7 +4203,7 @@ async fn tool_handlers_cascade_close_and_resume_and_keep_explicitly_closed_subtr let parent_session = parent.thread.session.clone(); let parent_turn = parent_session.new_default_turn().await; let mut owner_config = parent_turn - .environments + .initial_environments .primary() .expect("parent should have an environment") .config() @@ -4478,7 +4478,7 @@ async fn build_agent_spawn_config_uses_captured_step_settings_and_turn_context_v &file_system_sandbox_policy, network_sandbox_policy, ); - turn.environments.environments.clear(); + turn.initial_environments.environments.clear(); Arc::make_mut(&mut turn.config) .permissions .set_permission_profile(permission_profile) @@ -4566,7 +4566,7 @@ async fn build_agent_resume_config_clears_base_instructions() { PermissionProfile::read_only() }; let TurnEnvironmentState::Ready(environment) = turn - .environments + .initial_environments .environments .first_mut() .expect("parent environment should exist") diff --git a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs index f75b57d213..46fa63dbbb 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs @@ -180,7 +180,7 @@ async fn exec_command_rejects_login_when_selected_environment_disallows_it() { let (session, mut turn) = make_session_and_context().await; assert!(turn.config.permissions.allow_login_shell); let TurnEnvironmentState::Ready(environment) = turn - .environments + .initial_environments .environments .first_mut() .expect("primary environment") @@ -325,7 +325,7 @@ async fn exec_command_reuses_foreign_windows_grant() { { let turn = Arc::get_mut(&mut turn).expect("turn should be uniquely owned"); let TurnEnvironmentState::Ready(environment) = turn - .environments + .initial_environments .environments .first_mut() .expect("primary environment") diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index 29592a3fef..e5ff7f2a43 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -297,7 +297,7 @@ mod tests { fn replace_primary_environment_cwd(turn: &mut crate::TurnContext, cwd: AbsolutePathBuf) { let mut current = turn - .environments + .initial_environments .turn_environments() .next() .cloned() @@ -306,12 +306,13 @@ mod tests { let mut selection = current.selection; selection.cwd = PathUri::from_abs_path(&cwd); selection.workspace_roots.clear(); - turn.environments.environments[0] = TurnEnvironmentState::Ready(TurnEnvironment::new( - selection, - current.config_origin, - current.environment, - current.shell, - )); + turn.initial_environments.environments[0] = + TurnEnvironmentState::Ready(TurnEnvironment::new( + selection, + current.config_origin, + current.environment, + current.shell, + )); } fn tiny_png() -> Vec { @@ -372,7 +373,8 @@ mod tests { .permissions .set_permission_profile(PermissionProfile::Disabled) .expect("set thread permission profile"); - let TurnEnvironmentState::Ready(environment) = &mut turn.environments.environments[0] + let TurnEnvironmentState::Ready(environment) = + &mut turn.initial_environments.environments[0] else { panic!("primary environment should be ready"); }; @@ -450,7 +452,8 @@ mod tests { replace_primary_environment_cwd(&mut turn, image_cwd.clone()); let image_path = image_cwd.join("image.png"); std::fs::write(image_path.as_path(), tiny_png()).expect("write test image"); - let TurnEnvironmentState::Ready(environment) = &mut turn.environments.environments[0] + let TurnEnvironmentState::Ready(environment) = + &mut turn.initial_environments.environments[0] else { panic!("primary environment should be ready"); }; @@ -487,7 +490,8 @@ mod tests { let image_path = image_cwd.join("not-an-image.txt"); std::fs::write(image_path.as_path(), b"arbitrary file contents") .expect("write invalid image"); - let TurnEnvironmentState::Ready(environment) = &mut turn.environments.environments[0] + let TurnEnvironmentState::Ready(environment) = + &mut turn.initial_environments.environments[0] else { panic!("primary environment should be ready"); }; diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs index 71db3b5695..de37326e0c 100644 --- a/codex-rs/core/src/tools/network_approval.rs +++ b/codex-rs/core/src/tools/network_approval.rs @@ -648,11 +648,17 @@ impl NetworkApprovalService { { return NetworkDecision::deny(REASON_NOT_ALLOWED); } - let active_turn = session.active_turn_context_and_strict_auto_review().await; + let active_turn = session + .active_turn_context_and_strict_auto_review() + .await + .map(|(turn, inputs, strict)| { + let environments = inputs.environments.refresh_readiness(); + (turn, inputs, environments, strict) + }); let Some(environment_id) = active_environment_id.or_else(|| { active_turn .as_ref() - .and_then(|(turn_context, _, _)| turn_context.environments.primary()) + .and_then(|(_, _, environments, _)| environments.primary()) .map(|environment| environment.selection.environment_id.clone()) }) else { return NetworkDecision::deny(REASON_NOT_ALLOWED); @@ -679,7 +685,9 @@ impl NetworkApprovalService { format!("Network access to \"{target}\" was blocked by policy."); let prompt_reason = format!("{} is not in the allowed_domains", request.host); - let Some((turn_context, step_settings, strict_auto_review)) = active_turn else { + let Some((turn_context, step_inputs, active_environments, strict_auto_review)) = + active_turn + else { if let Some(owner_call) = owner_call.as_ref() { self.record_call_outcome(&owner_call.registration_id, policy_denial_message); } @@ -709,12 +717,15 @@ impl NetworkApprovalService { pending_owner.disconnect = request.disconnect.clone(); pending_owner.cancellation = request.cancellation.clone(); + // Review under current settings, but an owned process keeps the environments it started in. + let review_environments = owner_call + .as_ref() + .map_or(&active_environments, |call| &call.environments); let permission_profile = owner_call .as_ref() .map(|call| &call.permission_profile) .or_else(|| { - turn_context - .environments + review_environments .turn_environments() .find(|environment| environment.selection.environment_id == environment_id) .map(TurnEnvironment::permission_profile) @@ -728,12 +739,8 @@ impl NetworkApprovalService { } let review_context = GuardianReviewContext::from_resolved_settings( Arc::clone(&turn_context), - &step_settings, - // Review this new request under current settings, in its originating execution's - // environments. Completing the original turn does not transfer process ownership. - owner_call - .as_ref() - .map_or(&turn_context.environments, |call| &call.environments), + &step_inputs.settings, + review_environments, ); if !allows_network_approval_flow(review_context.approval_policy) { if let Some(owner_call) = owner_call.as_ref() { @@ -762,8 +769,7 @@ impl NetworkApprovalService { { cwd } else { - turn_context - .environments + review_environments .turn_environments() .find(|environment| environment.selection.environment_id == environment_id) .and_then(|environment| environment.cwd().to_abs_path().ok()) diff --git a/codex-rs/core/src/tools/runtimes/zsh_fork/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/zsh_fork/unix_escalation.rs index 0fb97dd384..1849e8c2d2 100644 --- a/codex-rs/core/src/tools/runtimes/zsh_fork/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/zsh_fork/unix_escalation.rs @@ -282,7 +282,7 @@ impl CoreShellActionProvider { }; match stopwatch .pause_for(async { - let (turn_context, step_settings, strict_auto_review) = self + let (turn_context, step_inputs, strict_auto_review) = self .session .active_turn_context_and_strict_auto_review() .await @@ -295,8 +295,8 @@ impl CoreShellActionProvider { let approval_ctx = ApprovalContext { review_context: GuardianReviewContext::from_resolved_settings( Arc::clone(&turn_context), - &step_settings, - &turn_context.environments, + &step_inputs.settings, + self.review_context.environments(), ), // The running process can outlive its launching tool or code-mode cell. cancellation_token: None, diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index 5dd6d75eb0..b50228703f 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -230,7 +230,7 @@ fn plan_with_model( let mut registry = build_core_tool_registry( turn, model_info, - &turn.environments, + &turn.initial_environments, &mcp, inputs.tool_suggest_candidates.as_ref(), inputs.wait_for_environment_tool_config.as_ref(), @@ -402,12 +402,12 @@ impl<'call> ToolExecutor> for DeferredExtensionTool { fn duplicate_primary_environment(turn: &mut TurnContext) { let mut second_environment = turn - .environments + .initial_environments .primary() .expect("primary environment") .clone(); second_environment.selection.environment_id = "secondary".to_string(); - turn.environments + turn.initial_environments .environments .push(TurnEnvironmentState::Ready(second_environment)); } @@ -740,7 +740,7 @@ async fn internal_guardian_sessions_require_managed_secondary_environments() { session.allowed_tools = Some(Arc::new(codex_guardian_reviewer::reviewer_allowed_tools())); set_feature(&mut turn, Feature::ViewImage, /*enabled*/ true); let TurnEnvironmentState::Ready(primary) = turn - .environments + .initial_environments .environments .first_mut() .expect("primary environment") @@ -755,7 +755,7 @@ async fn internal_guardian_sessions_require_managed_secondary_environments() { let secondary_workspace_root = codex_utils_path_uri::PathUri::from_abs_path(&turn.config.cwd.join("secondary")); let TurnEnvironmentState::Ready(secondary) = turn - .environments + .initial_environments .environments .get_mut(1) .expect("secondary environment") @@ -1008,7 +1008,7 @@ async fn exec_command_guidance_follows_executor_platform_and_fallbacks() { ConfigShellToolType::UnifiedExec; }); let TurnEnvironmentState::Ready(environment) = turn - .environments + .initial_environments .environments .first_mut() .expect("primary environment") @@ -1045,7 +1045,7 @@ async fn login_shell_parameter_follows_selected_environment() { config.permissions.allow_login_shell = !allow_login_shell; }); let TurnEnvironmentState::Ready(environment) = turn - .environments + .initial_environments .environments .first_mut() .expect("primary environment") @@ -1081,7 +1081,12 @@ async fn login_shell_parameter_is_available_when_any_environment_allows_it() { config.permissions.allow_login_shell = false; }); duplicate_primary_environment(turn); - for (index, environment) in turn.environments.environments.iter_mut().enumerate() { + for (index, environment) in turn + .initial_environments + .environments + .iter_mut() + .enumerate() + { let TurnEnvironmentState::Ready(environment) = environment else { panic!("environment should be ready"); }; @@ -1102,7 +1107,7 @@ async fn disabling_shell_tools_disables_command_tools_for_all_environments() { }); let TurnEnvironmentState::Ready(environment) = turn - .environments + .initial_environments .environments .first_mut() .expect("primary environment") @@ -1238,12 +1243,12 @@ async fn zsh_fork_unified_exec_keeps_shell_parameter_when_remote_environment_ava turn.unified_exec_shell_mode = codex_tools::UnifiedExecShellMode::ZshFork(zsh_fork_config_for_spec_plan_tests()); let remote_cwd = turn - .environments + .initial_environments .primary() .expect("primary environment") .cwd() .clone(); - turn.environments + turn.initial_environments .environments .push(TurnEnvironmentState::Ready( crate::session::turn_context::TurnEnvironment::new( @@ -1295,7 +1300,7 @@ async fn zsh_fork_unified_exec_keeps_shell_parameter_when_remote_environment_ava #[tokio::test] async fn environment_count_controls_environment_backed_tools() { let no_environment = probe(|turn| { - turn.environments.environments.clear(); + turn.initial_environments.environments.clear(); set_feature(turn, Feature::ShellTool, /*enabled*/ true); set_feature(turn, Feature::RequestPermissionsTool, /*enabled*/ true); update_turn_settings_for_test(turn, |settings| { @@ -1358,8 +1363,8 @@ async fn environment_tools_follow_the_step_context() { Some(ApplyPatchToolType::Freeform); }); - let environments = turn.environments.clone(); - turn.environments.environments.clear(); + let environments = turn.initial_environments.clone(); + turn.initial_environments.environments.clear(); let turn = Arc::new(turn); let mcp = Arc::new(codex_mcp::McpBinding::empty(mcp_config_for_test( &turn.config, diff --git a/codex-rs/core/src/unified_exec/mod_tests.rs b/codex-rs/core/src/unified_exec/mod_tests.rs index 58cd48da7e..7a945a074c 100644 --- a/codex-rs/core/src/unified_exec/mod_tests.rs +++ b/codex-rs/core/src/unified_exec/mod_tests.rs @@ -119,7 +119,7 @@ async fn exec_command_with_tty( /*network_policy_decider*/ None, tty, Box::new(NoopSpawnLifecycle), - turn.environments + turn.initial_environments .primary() .expect("turn environment") .environment @@ -147,7 +147,9 @@ async fn exec_command_with_tty( tty, environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), permissions: TerminalPermissions::for_launch( - turn.environments.primary().expect("turn environment"), + turn.initial_environments + .primary() + .expect("turn environment"), turn, TerminalSandboxSource::Native, SandboxPermissions::UseDefault, @@ -621,7 +623,9 @@ async fn terminating_initial_exec_command_rechecks_initial_response_state() -> a tty: true, environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), permissions: TerminalPermissions::for_launch( - turn.environments.primary().expect("turn environment"), + turn.initial_environments + .primary() + .expect("turn environment"), &turn, TerminalSandboxSource::Native, SandboxPermissions::UseDefault, @@ -704,7 +708,9 @@ async fn terminating_during_stdin_poll_returns_exited_response() -> anyhow::Resu tty: true, environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), permissions: TerminalPermissions::for_launch( - turn.environments.primary().expect("turn environment"), + turn.initial_environments + .primary() + .expect("turn environment"), &turn, TerminalSandboxSource::Native, SandboxPermissions::UseDefault, @@ -850,7 +856,8 @@ 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; - let TurnEnvironmentState::Ready(environment) = &mut turn.environments.environments[0] else { + let TurnEnvironmentState::Ready(environment) = &mut turn.initial_environments.environments[0] + else { panic!("expected ready primary environment"); }; environment.environment = Arc::new(remote_test_env.environment().clone()); @@ -876,7 +883,7 @@ async fn remote_exec_server_rejects_inherited_fd_launches() -> anyhow::Result<() Box::new(TestSpawnLifecycle { inherited_fds: vec![42], }), - turn.environments + turn.initial_environments .primary() .expect("turn environment") .environment @@ -928,7 +935,9 @@ async fn stdin_approval_preserves_the_reviewed_terminal() -> anyhow::Result<()> let mut store = manager.process_store.lock().await; let entry = store.processes.get_mut(&process_id).unwrap(); entry.permissions = TerminalPermissions::for_launch( - turn.environments.primary().expect("turn environment"), + turn.initial_environments + .primary() + .expect("turn environment"), &turn, TerminalSandboxSource::Native, SandboxPermissions::RequireEscalated, @@ -984,7 +993,7 @@ async fn stdin_approval_preserves_the_reviewed_terminal() -> anyhow::Result<()> .get_mut(&process_id) .unwrap() .environment_id = turn - .environments + .initial_environments .primary() .unwrap() .selection diff --git a/codex-rs/core/src/unified_exec/process_manager_tests.rs b/codex-rs/core/src/unified_exec/process_manager_tests.rs index f9d81d0abf..297c84fa76 100644 --- a/codex-rs/core/src/unified_exec/process_manager_tests.rs +++ b/codex-rs/core/src/unified_exec/process_manager_tests.rs @@ -463,7 +463,7 @@ async fn failed_initial_end_for_unstored_process_uses_fallback_output() { #[allow(deprecated)] sandbox_cwd: turn.cwd.clone().into(), turn_environment: turn - .environments + .initial_environments .primary() .cloned() .expect("primary environment"), @@ -631,7 +631,9 @@ async fn pruning_does_not_evict_live_process_while_exited_process_is_finalizing( tty: false, environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), permissions: super::super::TerminalPermissions::for_launch( - turn.environments.primary().expect("turn environment"), + turn.initial_environments + .primary() + .expect("turn environment"), &turn, super::super::TerminalSandboxSource::Native, crate::sandboxing::SandboxPermissions::UseDefault, diff --git a/codex-rs/core/src/unified_exec/stdin_approval_tests.rs b/codex-rs/core/src/unified_exec/stdin_approval_tests.rs index 7cb063406f..eafa1040c7 100644 --- a/codex-rs/core/src/unified_exec/stdin_approval_tests.rs +++ b/codex-rs/core/src/unified_exec/stdin_approval_tests.rs @@ -98,7 +98,11 @@ fn denied_reads_reject_file_system_changes_but_only_review_network_changes() { #[tokio::test] async fn captured_network_changes_require_review() -> anyhow::Result<()> { let (_session, mut turn) = make_session_and_context().await; - let mut environment = turn.environments.primary().expect("environment").clone(); + let mut environment = turn + .initial_environments + .primary() + .expect("environment") + .clone(); environment.config_mut().permission_profile = PermissionProfileSnapshot::legacy(PermissionProfile::read_only()); let mut proxy = NetworkProxyConfig { @@ -159,7 +163,11 @@ async fn captured_network_changes_require_review() -> anyhow::Result<()> { #[tokio::test] async fn internal_grants_require_review_without_exposing_paths() -> anyhow::Result<()> { let (_session, turn) = make_session_and_context().await; - let mut environment = turn.environments.primary().expect("environment").clone(); + let mut environment = turn + .initial_environments + .primary() + .expect("environment") + .clone(); environment.config_mut().permission_profile = PermissionProfileSnapshot::legacy(PermissionProfile::read_only()); let grants = serde_json::from_value(json!({ @@ -195,7 +203,11 @@ async fn internal_grants_require_review_without_exposing_paths() -> anyhow::Resu #[tokio::test] async fn readable_snapshot_does_not_require_stdin_approval() -> anyhow::Result<()> { let (_session, turn) = make_session_and_context().await; - let mut environment = turn.environments.primary().expect("environment").clone(); + let mut environment = turn + .initial_environments + .primary() + .expect("environment") + .clone(); environment.config_mut().permission_profile = PermissionProfileSnapshot::legacy(PermissionProfile::read_only()); let snapshot = turn.config.cwd.join("shell-snapshot.sh"); @@ -230,7 +242,11 @@ async fn enabling_windows_sandbox_respects_the_launch_backend( expected: SandboxPermissions, ) -> anyhow::Result<()> { let (_session, turn) = make_session_and_context().await; - let mut environment = turn.environments.primary().expect("environment").clone(); + let mut environment = turn + .initial_environments + .primary() + .expect("environment") + .clone(); environment.selection.cwd = PathUri::parse("file:///C:/workspace")?; environment.config_mut().permission_profile = PermissionProfileSnapshot::legacy(PermissionProfile::read_only()); diff --git a/codex-rs/core/tests/suite/network_approval.rs b/codex-rs/core/tests/suite/network_approval.rs index b1ce06551c..3892b73ce1 100644 --- a/codex-rs/core/tests/suite/network_approval.rs +++ b/codex-rs/core/tests/suite/network_approval.rs @@ -23,6 +23,8 @@ use codex_protocol::approvals::NetworkApprovalContext; use codex_protocol::approvals::NetworkApprovalProtocol; use codex_protocol::approvals::NetworkPolicyAmendment; use codex_protocol::approvals::NetworkPolicyRuleAction; +use codex_protocol::capabilities::CapabilityRootLocation; +use codex_protocol::capabilities::SelectedCapabilityRoot; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Settings; @@ -1637,7 +1639,19 @@ async fn unattributed_network_request_uses_active_turn_environment_fallback( skip_if_sandbox!(Ok(())); let server = start_mock_server().await; - let test = managed_network_unified_exec_test(&server).await?; + let features = if approvals_reviewer == ApprovalsReviewer::User { + &[Feature::DeferredExecutor][..] + } else { + &[] + }; + let test = managed_network_unified_exec_test_with_features(&server, features).await?; + let mut selection = local(test.config.cwd.clone()); + if approvals_reviewer == ApprovalsReviewer::User { + let cwd = test.config.cwd.join("late-environment"); + fs::create_dir(&cwd)?; + selection = local(cwd); + selection.config = EnvironmentConfigState::Pending; + } let pending_model = mount_response_once_match( &server, |request: &wiremock::Request| request_body_contains(request, "hold the active turn"), @@ -1652,13 +1666,62 @@ async fn unattributed_network_request_uses_active_turn_environment_fallback( submit_managed_network_turn( &test, "hold the active turn", - vec![local(test.config.cwd.clone())], + vec![selection.clone()], approvals_reviewer, AskForApproval::OnRequest, ) .await?; wait_for_response_request(&pending_model).await; + if approvals_reviewer == ApprovalsReviewer::User { + let root = SelectedCapabilityRoot { + id: "late-environment".into(), + location: CapabilityRootLocation::Environment { + environment_id: selection.environment_id.clone(), + path: selection.cwd.clone(), + }, + }; + test.codex + .environment_ready( + &selection, + EnvironmentConfig { + allow_login_shell: test.config.permissions.allow_login_shell, + workspace_roots: selection.workspace_roots.clone(), + permission_profile: PermissionProfileSnapshot::legacy( + test.config.permissions.permission_profile().clone(), + ), + shell_environment_policy: test + .config + .permissions + .shell_environment_policy + .clone(), + windows_sandbox_level: WindowsSandboxLevel::from_config(&test.config), + windows_sandbox_type: test.config.permissions.windows_sandbox_type, + windows_sandbox_private_desktop: test + .config + .permissions + .windows_sandbox_private_desktop, + use_legacy_landlock: test.config.features.use_legacy_landlock(), + exec_policy: None, + mcp_policy: None, + network_policy: None, + selected_capability_roots: vec![root.clone()], + }, + ) + .await?; + tokio::time::timeout(Duration::from_secs(/*secs*/ 5), async { + while !test + .codex + .inspect_selected_capability_roots() + .ready_roots + .contains(&root) + { + tokio::task::yield_now().await; + } + }) + .await?; + } + let proxy_addr = test .session_configured .network_proxy @@ -1727,7 +1790,7 @@ async fn unattributed_network_request_uses_active_turn_environment_fallback( let proxy_request = tokio::spawn(raw_http_proxy_request(proxy_addr, NETWORK_TEST_HOST)); let approval = expect_network_approval(&test, LOCAL_ENVIRONMENT_ID).await?; assert_eq!(approval.command, ["network-access", NETWORK_TEST_TARGET]); - assert_eq!(approval.cwd, test.config.cwd.clone().into()); + assert_eq!(approval.cwd, selection.cwd.into()); test.codex .submit(Op::ExecApproval { id: approval.effective_approval_id(), @@ -2433,6 +2496,26 @@ async fn owner_network_policy_follows_the_selected_remote_command() -> Result<() for (scenario, test) in scenarios { let mut remote = test.executor_environment().selection().clone(); + let remote_private_path = remote.cwd.join("secondary-environment-private")?; + let permissions = test.config.permissions.permission_profile(); + let mut filesystem = permissions.file_system_sandbox_policy(); + filesystem.entries.push(FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: remote_private_path.clone(), + }, + access: FileSystemAccessMode::Deny, + missing_path_behavior: None, + }); + filesystem.entries.push(FileSystemSandboxEntry::new( + FileSystemPath::GlobPattern { + pattern: "*.guardian-secret".into(), + }, + FileSystemAccessMode::Deny, + )); + let remote_review_permissions = PermissionProfile::from_runtime_permissions( + &filesystem, + permissions.network_sandbox_policy(), + ); for (suffix, allowed_domain, expected) in [ ( @@ -2464,6 +2547,8 @@ async fn owner_network_policy_follows_the_selected_remote_command() -> Result<() proxy_config.set_allowed_domains(vec![allowed_domain.to_string()]); let mut permission_profile = if restricted { PermissionProfile::workspace_write() + } else if suffix == "REVIEWED" { + remote_review_permissions.clone() } else { test.config.permissions.permission_profile().clone() }; @@ -2612,6 +2697,21 @@ PYTHON"#; guardian_network_triggers(&[&guardian])?, vec![(marker.clone(), command)] ); + let prompt = guardian + .single_request() + .message_input_texts("user") + .join(""); + let permissions = prompt + .split_once("PARENT TURN PERMISSION CONTEXT START") + .and_then(|(_, text)| text.split_once("PARENT TURN PERMISSION CONTEXT END")) + .map(|(permissions, _)| permissions) + .context("network Guardian permissions")?; + assert!(permissions.contains(&remote_private_path.inferred_native_path_string())); + let remote_glob = remote + .cwd + .join("*.guardian-secret")? + .inferred_native_path_string(); + assert!(permissions.contains(&format!("glob `{remote_glob}`"))); } let output = responses .function_call_output_text(&marker) diff --git a/codex-rs/core/tests/suite/remote_env.rs b/codex-rs/core/tests/suite/remote_env.rs index f1e04c4dbe..1a81fa4b04 100644 --- a/codex-rs/core/tests/suite/remote_env.rs +++ b/codex-rs/core/tests/suite/remote_env.rs @@ -2735,7 +2735,8 @@ async fn deferred_executor_guardian_uses_newly_ready_step_environment() -> Resul let remote_cwd = test.cwd.path().join("guardian-remote").abs(); let local_cwd = test.cwd.path().abs(); fs::create_dir_all(remote_cwd.as_path())?; - let remote_denied_path = remote_cwd.canonicalize()?.join("private"); + // Remote policy paths use the executor's spelling; only local paths are canonicalized here. + let remote_denied_path = remote_cwd.join("private"); let local_denied_path = local_cwd.canonicalize()?.join("private"); let remote_selection = TurnEnvironmentSelection { environment_id: REMOTE_ENVIRONMENT_ID.to_string(), diff --git a/codex-rs/core/tests/suite/step_settings.rs b/codex-rs/core/tests/suite/step_settings.rs index 80d9278d13..29bb8b2c1a 100644 --- a/codex-rs/core/tests/suite/step_settings.rs +++ b/codex-rs/core/tests/suite/step_settings.rs @@ -114,6 +114,7 @@ use super::rmcp_client::remote_aware_stdio_server_bin; #[path = "step_settings/agent_spawn_tests.rs"] mod agent_spawn; mod code_mode_notifications; +mod environment_selection; const MODEL_A: &str = "step-settings-a"; const MODEL_B: &str = "step-settings-b"; diff --git a/codex-rs/core/tests/suite/step_settings/environment_selection.rs b/codex-rs/core/tests/suite/step_settings/environment_selection.rs new file mode 100644 index 0000000000..6e6b4697f4 --- /dev/null +++ b/codex-rs/core/tests/suite/step_settings/environment_selection.rs @@ -0,0 +1,116 @@ +//! Active model updates preserve the captured environment until the next turn. + +use super::*; +use codex_exec_server::CreateDirectoryOptions; +use codex_protocol::protocol::TurnEnvironmentSelections; +use core_test_support::submit_thread_settings; +use pretty_assertions::assert_eq; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn model_update_preserves_active_environment_and_next_turn_uses_new_selection() -> Result<()> +{ + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let patch_response = |id, contents| { + sse(vec![ + ev_response_created(id), + ev_apply_patch_custom_tool_call( + id, + &format!("*** Begin Patch\n*** Add File: marker.txt\n+{contents}\n*** End Patch\n"), + ), + ev_completed(id), + ]) + }; + let responses = mount_sse_sequence( + &server, + vec![ + paused_response("resp-a", "pause-a"), + patch_response("active-patch", "active step"), + sse_completed("active-done"), + patch_response("next-turn-patch", "next turn"), + sse_completed("next-turn-done"), + ], + ) + .await; + let test = direct_tool_settings_test() + .with_config(|config| { + config + .permissions + .set_permission_profile(PermissionProfile::Disabled) + .expect("test permissions"); + for model in &mut config.model_catalog.as_mut().expect("models").models { + model.apply_patch_tool_type = Some(ApplyPatchToolType::Freeform); + } + }) + .build_with_auto_env(&server) + .await?; + let mut next_environment = test.executor_environment().selection().clone(); + next_environment.cwd = test.workspace_path_uri("future-environment")?; + next_environment.workspace_roots = vec![next_environment.cwd.clone()]; + test.fs() + .create_directory( + &next_environment.cwd, + CreateDirectoryOptions { + recursive: false, + follow_symlinks: true, + }, + /*sandbox*/ None, + ) + .await?; + let next_marker = next_environment.cwd.join("marker.txt")?; + + let paused = start_paused_turn(&test.codex).await?; + submit_thread_settings( + &test.codex, + ThreadSettingsOverrides { + environments: Some(TurnEnvironmentSelections::new( + test.config.cwd.join("future-environment"), + vec![next_environment], + )), + ..Default::default() + }, + ) + .await?; + apply_turn_settings( + &test.codex, + &paused.turn_id, + TurnSettingsUpdate { + model: Some(MODEL_B.to_string()), + ..Default::default() + }, + ) + .await?; + answer_paused_turn(&test.codex, &paused.turn_id).await?; + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + test.submit_text_turn("write in the newly selected environment") + .await?; + + assert_eq!( + responses + .requests() + .iter() + .map(|request| request.body_json()["model"].clone()) + .collect::>(), + [MODEL_A, MODEL_B, MODEL_B, MODEL_A, MODEL_A].map(|model| json!(model)), + ); + assert_eq!( + ( + test.fs() + .read_file_text( + &test.workspace_path_uri("marker.txt")?, + Default::default(), + /*sandbox*/ None, + ) + .await?, + test.fs() + .read_file_text(&next_marker, Default::default(), /*sandbox*/ None) + .await?, + ), + ("active step\n".to_string(), "next turn\n".to_string()), + ); + Ok(()) +} diff --git a/codex-rs/core/tests/suite/unified_exec_zsh_fork_approvals.rs b/codex-rs/core/tests/suite/unified_exec_zsh_fork_approvals.rs index d8c8a9677b..46b7aa3850 100644 --- a/codex-rs/core/tests/suite/unified_exec_zsh_fork_approvals.rs +++ b/codex-rs/core/tests/suite/unified_exec_zsh_fork_approvals.rs @@ -455,11 +455,12 @@ async fn unified_exec_zsh_fork_guardian_reviews_persistent_terminal_in_current_t skip_if_no_network!(Ok(())); let approval_policy = AskForApproval::OnRequest; - let permission_profile = restrictive_workspace_write_profile(); let outside_dir = tempfile::tempdir_in(std::env::current_dir()?)?; let outside_path = outside_dir .path() .join("unified-exec-zsh-fork-current-turn.txt"); + let initial_denied_path = outside_dir.path().join("original-environment-private"); + let permission_profile = denied_read_permission_profile(&initial_denied_path)?; let rules = r#"prefix_rule(pattern=["touch"], decision="prompt")"#.to_string(); let outside_path_for_hook = outside_path.clone(); @@ -555,13 +556,28 @@ async fn unified_exec_zsh_fork_guardian_reviews_persistent_terminal_in_current_t unreachable!("completion wait only returns turn-complete events"); }; - submit_turn_with_session_permissions( - &test, - "run a command in the persistent terminal with Guardian approvals", - approval_policy, - ApprovalsReviewer::AutoReview, - ) - .await?; + let next_cwd = test.config.cwd.join("next-turn"); + fs::create_dir(&next_cwd)?; + let next_denied_path = next_cwd.join("next-environment-private"); + let (sandbox_policy, permission_profile) = turn_permission_fields( + denied_read_permission_profile(next_denied_path.as_path())?, + next_cwd.as_path(), + ); + test.codex + .start_or_steer_turn( + TurnInputRequest::user_input(vec![UserInput::Text { + text: "run a command in the persistent terminal with Guardian approvals".into(), + text_elements: Vec::new(), + }]) + .with_thread_settings(ThreadSettingsOverrides { + environments: Some(local_selections(next_cwd)), + approvals_reviewer: Some(ApprovalsReviewer::AutoReview), + sandbox_policy: Some(sandbox_policy), + permission_profile, + ..Default::default() + }), + ) + .await?; let mut current_turn_id = None; let mut stdin_assessment = None; @@ -630,6 +646,14 @@ async fn unified_exec_zsh_fork_guardian_reviews_persistent_terminal_in_current_t assert!(guardian_requests[0].body_contains_text(&environment)); assert!(guardian_requests[0].body_contains_text("The `cwd` field is its launch directory")); assert!(guardian_requests[1].body_contains_text(&outside_path.to_string_lossy())); + let guardian_text = guardian_requests[1].message_input_texts("user").join(""); + let permissions = guardian_text + .split_once("PARENT TURN PERMISSION CONTEXT START") + .and_then(|(_, text)| text.split_once("PARENT TURN PERMISSION CONTEXT END")) + .map(|(permissions, _)| permissions) + .context("intercepted command's Guardian permissions")?; + assert!(permissions.contains(initial_denied_path.to_string_lossy().as_ref())); + assert!(!permissions.contains(next_denied_path.to_string_lossy().as_ref())); Ok(()) } diff --git a/codex-rs/protocol/src/permissions/target.rs b/codex-rs/protocol/src/permissions/target.rs index 5536df375e..22037e800f 100644 --- a/codex-rs/protocol/src/permissions/target.rs +++ b/codex-rs/protocol/src/permissions/target.rs @@ -1,6 +1,8 @@ //! Executor-context policy operations that never consult controller paths or environment. +use super::FileSystemAccessMode; use super::FileSystemPath; +use super::FileSystemSandboxKind; use super::FileSystemSandboxPolicy; use super::FileSystemSandboxPolicyContext; use super::FileSystemSpecialPath; @@ -8,8 +10,10 @@ use super::InvalidDenyReadGlobBehavior; use super::ReadDenyMatcher; use super::deny_read_validator::DenyReadValidator; use super::deny_read_validator::has_context_read_denials; +use super::file_system_root; use codex_utils_path_uri::PathConvention; use codex_utils_path_uri::PathUri; +use std::collections::HashSet; const INVALID_PATH: &str = "managed filesystem denial cannot be materialized safely"; const SHADOWED: &str = "managed filesystem denial is shadowed by a selected profile grant"; @@ -73,6 +77,45 @@ impl FileSystemSandboxPolicy { .with_materialized_project_roots_for_path_uris(roots) } + /// Returns explicitly denied roots using only the selected executor's paths. + /// A filesystem-root denial is omitted so it does not obscure narrower readable paths. + /// Fails rather than omit a denied temporary directory the executor did not report. + pub fn get_unreadable_roots_with_context( + &self, + context: &FileSystemSandboxPolicyContext<'_>, + ) -> Result, String> { + if !matches!(self.kind, FileSystemSandboxKind::Restricted) { + return Ok(Vec::new()); + } + if context.temporary_directories.is_none() + && self.entries.iter().any(|entry| { + entry.access == FileSystemAccessMode::Deny + && matches!( + &entry.path, + FileSystemPath::Special { + value: FileSystemSpecialPath::Tmpdir + } + ) + }) + { + return Err("executor did not report its denied temporary directories".into()); + } + let filesystem_root = file_system_root(context); + let mut seen = HashSet::new(); + Ok(self + .resolved_entries(context) + .into_iter() + .filter(|(path, access)| { + *access == FileSystemAccessMode::Deny + && !filesystem_root + .as_ref() + .is_some_and(|root| path.starts_with(root) && root.starts_with(path)) + && seen.insert(path.clone()) + }) + .map(|(path, _)| path) + .collect()) + } + /// Resolves deny glob text using the execution host's convention and home. pub fn get_unreadable_globs_with_context( &self, diff --git a/codex-rs/protocol/src/permissions/target_tests.rs b/codex-rs/protocol/src/permissions/target_tests.rs index 5842440e22..229705f702 100644 --- a/codex-rs/protocol/src/permissions/target_tests.rs +++ b/codex-rs/protocol/src/permissions/target_tests.rs @@ -43,6 +43,10 @@ fn read_denials_use_target_paths_and_native_glob_semantics() { let matcher = ReadDenyMatcher::try_new_with_context(&policy, &context) .unwrap() .unwrap(); + assert_eq!( + policy.get_unreadable_roots_with_context(&context), + Ok(vec![cwd.join("private").unwrap()]), + ); for (path, expected) in [ ("private", true), ("private/file", true), @@ -208,6 +212,17 @@ fn malformed_or_unresolvable_denials_fail_closed() { path: uri("file:///C:/private"), })]); assert!(ReadDenyMatcher::try_new_with_context(&policy, &context).is_err()); + let policy = FileSystemSandboxPolicy::restricted(vec![deny(FileSystemPath::Special { + value: FileSystemSpecialPath::Tmpdir, + })]); + assert!( + policy + .get_unreadable_roots_with_context(&FileSystemSandboxPolicyContext { + temporary_directories: None, + ..context + }) + .is_err() + ); } #[test]