diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 35f289b46b..0f5061d5c4 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -36,6 +36,7 @@ use tokio::time::timeout; use tokio_util::sync::CancellationToken; use crate::config::Config; +use crate::environment_selection::TurnEnvironmentSnapshot; use crate::guardian::GuardianApprovalRequest; use crate::guardian::GuardianReviewOptions; use crate::guardian::new_guardian_review_id; @@ -84,6 +85,7 @@ pub(crate) async fn run_codex_thread_interactive( models_manager: SharedModelsManager, parent_session: Arc, parent_ctx: Arc, + parent_environments: TurnEnvironmentSnapshot, cancel_token: CancellationToken, subagent_source: SubAgentSource, initial_history: Option, @@ -126,11 +128,11 @@ pub(crate) async fn run_codex_thread_interactive( dynamic_tools: Vec::new(), metrics_service_name: None, user_shell_override: None, - inherited_environments: Some(parent_ctx.environments.clone()), + inherited_environments: Some(parent_environments.clone()), inherited_exec_policy: Some(Arc::clone(&parent_session.services.exec_policy)), parent_rollout_thread_trace: codex_rollout_trace::ThreadTraceContext::disabled(), parent_trace: None, - environment_selections: parent_ctx.environments.to_selections(), + environment_selections: parent_environments.to_selections(), thread_extension_init: codex_extension_api::ExtensionDataInit::default(), client_mcp_extensions: parent_session.services.client_mcp_extensions.clone(), analytics_events_client: Some(parent_session.services.analytics_events_client.clone()), @@ -216,12 +218,14 @@ 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 (session, io) = Box::pin(run_codex_thread_interactive( config, auth_manager, models_manager, parent_session, parent_ctx, + parent_environments, child_cancel.clone(), subagent_source, initial_history, diff --git a/codex-rs/core/src/codex_delegate_tests.rs b/codex-rs/core/src/codex_delegate_tests.rs index f23d35c3da..2ee551d84e 100644 --- a/codex-rs/core/src/codex_delegate_tests.rs +++ b/codex-rs/core/src/codex_delegate_tests.rs @@ -201,6 +201,7 @@ async fn run_codex_thread_interactive_respects_pre_cancelled_spawn() { crate::session::tests::make_session_and_context_with_rx().await; let cancel_token = CancellationToken::new(); cancel_token.cancel(); + let parent_environments = parent_ctx.environments.clone(); let result = timeout( Duration::from_secs(/*secs*/ 1), @@ -210,6 +211,7 @@ async fn run_codex_thread_interactive_respects_pre_cancelled_spawn() { Arc::clone(&parent_session.services.models_manager), parent_session, parent_ctx, + parent_environments, cancel_token, SubAgentSource::Review, /*initial_history*/ None, diff --git a/codex-rs/core/src/guardian/mod.rs b/codex-rs/core/src/guardian/mod.rs index acdd467683..42168e0cbf 100644 --- a/codex-rs/core/src/guardian/mod.rs +++ b/codex-rs/core/src/guardian/mod.rs @@ -17,12 +17,16 @@ mod prompt; mod review; mod review_session; +use std::sync::Arc; use std::time::Duration; use codex_protocol::protocol::GuardianAssessmentOutcome; use serde::Deserialize; use serde::Serialize; +use crate::environment_selection::TurnEnvironmentSnapshot; +use crate::session::step_context::StepContext; +use crate::session::turn_context::TurnContext; use crate::tools::sandboxing::ApprovalRequestReasons; pub(crate) use approval_request::GuardianApprovalRequest; @@ -62,6 +66,51 @@ const GUARDIAN_MAX_ACTION_STRING_TOKENS: usize = 16_000; const GUARDIAN_RECENT_ENTRY_LIMIT: usize = 40; const TRUNCATION_TAG: &str = "truncated"; +/// Built from the originating StepContext when available. +/// There are currently two exceptions-- turn-only callers (background network approvals, reviewer +/// prewarming, etc.) and interactive Unix shells that can outlive the step that started them. +/// TODO(sayan): See if we can find a way to model those as StepContext as well without holding +/// step-scoped things past their lifetime (like MCP bindings) +#[derive(Clone)] +pub(crate) struct GuardianReviewContext { + turn: Arc, + environments: TurnEnvironmentSnapshot, +} + +impl GuardianReviewContext { + pub(crate) fn turn(&self) -> &Arc { + &self.turn + } + + pub(crate) fn environments(&self) -> &TurnEnvironmentSnapshot { + &self.environments + } +} + +impl From<&Arc> for GuardianReviewContext { + fn from(step: &Arc) -> Self { + Self { + turn: Arc::clone(&step.turn), + environments: step.environments.clone(), + } + } +} + +impl From> for GuardianReviewContext { + fn from(turn: Arc) -> Self { + Self { + environments: turn.environments.clone(), + turn, + } + } +} + +impl From<&Arc> for GuardianReviewContext { + fn from(turn: &Arc) -> Self { + Self::from(Arc::clone(turn)) + } +} + /// Structured output contract that the guardian reviewer must satisfy. #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)] pub(crate) struct GuardianAssessment { diff --git a/codex-rs/core/src/guardian/prompt.rs b/codex-rs/core/src/guardian/prompt.rs index bef8cb6b84..ea647e2c3a 100644 --- a/codex-rs/core/src/guardian/prompt.rs +++ b/codex-rs/core/src/guardian/prompt.rs @@ -11,7 +11,6 @@ use serde_json::Value; use crate::compact::content_items_to_text; use crate::event_mapping::is_contextual_user_message_content; use crate::session::session::Session; -use crate::session::turn_context::TurnContext; use crate::session::turn_context::TurnEnvironment; use codex_utils_output_truncation::TruncationPolicy; use codex_utils_output_truncation::approx_bytes_for_tokens; @@ -28,6 +27,7 @@ use super::GUARDIAN_MAX_TOOL_TRANSCRIPT_TOKENS; use super::GUARDIAN_RECENT_ENTRY_LIMIT; use super::GuardianApprovalRequest; use super::GuardianAssessment; +use super::GuardianReviewContext; use super::TRUNCATION_TAG; use super::approval_request::format_guardian_action_pretty; @@ -103,7 +103,7 @@ pub(crate) async fn build_guardian_prompt_items( ) -> serde_json::Result { build_guardian_prompt_items_with_parent_turn( session, - /*parent_turn*/ None, + /*parent_context*/ None, ApprovalRequestReasons { approval: None, retry: retry_reason, @@ -116,7 +116,7 @@ pub(crate) async fn build_guardian_prompt_items( pub(crate) async fn build_guardian_prompt_items_with_parent_turn( session: &Session, - parent_turn: Option<&TurnContext>, + parent_context: Option<&GuardianReviewContext>, reasons: ApprovalRequestReasons, request: GuardianApprovalRequest, mode: GuardianPromptMode, @@ -201,7 +201,7 @@ pub(crate) async fn build_guardian_prompt_items_with_parent_turn( if let Some(note) = omission_note { push_text(format!("\n{note}\n")); } - if let Some(denied_reads_context) = parent_turn.and_then(parent_turn_denied_reads_context) { + if let Some(denied_reads_context) = parent_context.and_then(parent_turn_denied_reads_context) { push_text("\n>>> PARENT TURN PERMISSION CONTEXT START\n".to_string()); push_text(denied_reads_context); push_text(">>> PARENT TURN PERMISSION CONTEXT END\n".to_string()); @@ -254,9 +254,9 @@ pub(crate) async fn build_guardian_prompt_items_with_parent_turn( }) } -fn parent_turn_denied_reads_context(turn: &TurnContext) -> Option { - // TODO(sayan): Pass StepContext through Guardian so it sees environments that become ready mid-turn. - let environment = turn.environments.primary(); +fn parent_turn_denied_reads_context(context: &GuardianReviewContext) -> Option { + let turn = context.turn(); + let environment = context.environments().primary(); #[allow(deprecated)] let cwd = environment .and_then(|environment| environment.cwd().to_abs_path().ok()) diff --git a/codex-rs/core/src/guardian/review.rs b/codex-rs/core/src/guardian/review.rs index c1f274c70a..08ceb3a392 100644 --- a/codex-rs/core/src/guardian/review.rs +++ b/codex-rs/core/src/guardian/review.rs @@ -41,6 +41,7 @@ use super::GuardianAssessment; use super::GuardianAssessmentOutcome; use super::GuardianRejectionCircuitBreakerAction; use super::GuardianRejectionCircuitBreakerPolicy; +use super::GuardianReviewContext; use super::approval_request::guardian_assessment_action; use super::approval_request::guardian_request_target_item_id; use super::approval_request::guardian_request_turn_id; @@ -303,12 +304,13 @@ pub(crate) async fn record_guardian_denial_for_test( /// caller as distinct from explicit guardian denials. async fn run_guardian_review( session: Arc, - turn: Arc, + context: GuardianReviewContext, review_id: String, request: GuardianApprovalRequest, reasons: ApprovalRequestReasons, options: GuardianReviewOptions, ) -> ReviewDecision { + let turn = Arc::clone(context.turn()); let GuardianReviewOptions { plugin_attribution_override, approval_request_source, @@ -401,7 +403,7 @@ async fn run_guardian_review( let terminal_action = action_summary.clone(); let (outcome, analytics_result) = Box::pin(run_guardian_review_session_with_retry( session.clone(), - turn.clone(), + context, request, reasons, schema, @@ -643,7 +645,7 @@ pub(crate) struct GuardianReviewOptions { /// Public entrypoint for approval requests that should be reviewed by guardian. pub(crate) async fn review_approval_request( session: &Arc, - turn: &Arc, + context: impl Into, review_id: String, request: GuardianApprovalRequest, reasons: ApprovalRequestReasons, @@ -652,7 +654,7 @@ pub(crate) async fn review_approval_request( // guardian session state machine into their own async stack. Box::pin(run_guardian_review( Arc::clone(session), - Arc::clone(turn), + context.into(), review_id, request, reasons, @@ -667,7 +669,7 @@ pub(crate) async fn review_approval_request( pub(crate) async fn review_approval_request_with_cancel( session: &Arc, - turn: &Arc, + context: impl Into, review_id: String, request: GuardianApprovalRequest, retry_reason: Option, @@ -675,7 +677,7 @@ pub(crate) async fn review_approval_request_with_cancel( ) -> ReviewDecision { run_guardian_review( Arc::clone(session), - Arc::clone(turn), + context.into(), review_id, request, ApprovalRequestReasons { @@ -689,12 +691,13 @@ pub(crate) async fn review_approval_request_with_cancel( pub(crate) fn spawn_approval_request_review( session: Arc, - turn: Arc, + context: impl Into, review_id: String, request: GuardianApprovalRequest, retry_reason: Option, options: GuardianReviewOptions, ) -> oneshot::Receiver { + let context = context.into(); let (tx, rx) = oneshot::channel(); let spawn_result = std::thread::Builder::new() .name("codex-approval-review".to_string()) @@ -710,7 +713,7 @@ pub(crate) fn spawn_approval_request_review( }; let decision = runtime.block_on(review_approval_request_with_cancel( &session, - &turn, + context, review_id, request, retry_reason, @@ -842,13 +845,14 @@ pub(super) async fn guardian_review_session_config( /// rules. async fn run_guardian_review_session_before_deadline( session: Arc, - turn: Arc, + context: GuardianReviewContext, request: GuardianApprovalRequest, reasons: ApprovalRequestReasons, schema: serde_json::Value, external_cancel: Option, deadline: Instant, ) -> (GuardianReviewOutcome, GuardianReviewAnalyticsResult) { + let turn = context.turn(); let session_config = match guardian_review_session_config(session.as_ref(), turn.as_ref()).await { Ok(session_config) => session_config, @@ -864,7 +868,7 @@ async fn run_guardian_review_session_before_deadline( .guardian_review_session .run_review(GuardianReviewSessionParams { parent_session: Arc::clone(&session), - parent_turn: turn.clone(), + parent_context: context.clone(), spawn_config: session_config.spawn_config, request, reasons, @@ -936,20 +940,21 @@ async fn run_guardian_review_session_before_deadline( pub(super) async fn run_guardian_review_session_with_retry( session: Arc, - turn: Arc, + context: impl Into, request: GuardianApprovalRequest, reasons: ApprovalRequestReasons, schema: serde_json::Value, external_cancel: Option, max_attempts: i64, ) -> (GuardianReviewOutcome, GuardianReviewAnalyticsResult) { + let context = context.into(); assert!(max_attempts > 0, "guardian review must run at least once"); let deadline = Instant::now() + GUARDIAN_REVIEW_TIMEOUT; let mut attempt_count = 1; loop { let (outcome, mut analytics_result) = run_guardian_review_session_before_deadline( Arc::clone(&session), - Arc::clone(&turn), + context.clone(), request.clone(), reasons.clone(), schema.clone(), diff --git a/codex-rs/core/src/guardian/review_session.rs b/codex-rs/core/src/guardian/review_session.rs index 804d2f1d8d..2b3814fde0 100644 --- a/codex-rs/core/src/guardian/review_session.rs +++ b/codex-rs/core/src/guardian/review_session.rs @@ -56,6 +56,7 @@ use codex_utils_path_uri::PathUri; use super::ApprovalRequestReasons; use super::GUARDIAN_REVIEWER_NAME; use super::GuardianApprovalRequest; +use super::GuardianReviewContext; use super::prompt::BUNDLED_GUARDIAN_POLICY; use super::prompt::BUNDLED_GUARDIAN_POLICY_TEMPLATE; use super::prompt::GuardianPromptMode; @@ -79,7 +80,7 @@ pub(crate) enum GuardianReviewSessionOutcome { pub(crate) struct GuardianReviewSessionParams { pub(crate) parent_session: Arc, - pub(crate) parent_turn: Arc, + pub(crate) parent_context: GuardianReviewContext, pub(crate) spawn_config: Config, pub(crate) request: GuardianApprovalRequest, pub(crate) reasons: ApprovalRequestReasons, @@ -358,7 +359,7 @@ impl GuardianReviewSessionManager { let spawn_cancel_guard = spawn_cancel_token.clone().drop_guard(); let review_session = spawn_guardian_review_session( &parent_session, - &parent_turn, + &GuardianReviewContext::from(parent_turn), spawn_config, reuse_key, spawn_cancel_token.clone(), @@ -459,7 +460,7 @@ impl GuardianReviewSessionManager { &spawn_cancel_token, Box::pin(spawn_guardian_review_session( ¶ms.parent_session, - ¶ms.parent_turn, + ¶ms.parent_context, params.spawn_config.clone(), next_reuse_key.clone(), spawn_cancel_token.clone(), @@ -678,7 +679,7 @@ impl GuardianReviewSessionManager { &spawn_cancel_token, Box::pin(spawn_guardian_review_session( ¶ms.parent_session, - ¶ms.parent_turn, + ¶ms.parent_context, fork_config, reuse_key, spawn_cancel_token.clone(), @@ -721,7 +722,7 @@ impl GuardianReviewSessionManager { async fn spawn_guardian_review_session( parent_session: &Arc, - parent_turn: &Arc, + parent_context: &GuardianReviewContext, spawn_config: Config, reuse_key: GuardianReviewSessionReuseKey, cancel_token: CancellationToken, @@ -746,7 +747,8 @@ async fn spawn_guardian_review_session( parent_session.services.auth_manager.clone(), parent_session.services.models_manager.clone(), Arc::clone(parent_session), - Arc::clone(parent_turn), + Arc::clone(parent_context.turn()), + parent_context.environments().clone(), cancel_token.clone(), SubAgentSource::Other(GUARDIAN_REVIEWER_NAME.to_string()), initial_history, @@ -836,7 +838,7 @@ async fn run_review_on_session( build_guardian_prompt_items_with_parent_turn( params.parent_session.as_ref(), - Some(params.parent_turn.as_ref()), + Some(¶ms.parent_context), params.reasons.clone(), params.request.clone(), prompt_mode, @@ -867,15 +869,15 @@ async fn run_review_on_session( .await .unwrap_or_default(); let guardian_permission_profile = PermissionProfile::read_only(); - let parent_turn_environments = params.parent_turn.environments.to_selections(); + let parent_turn_environments = params.parent_context.environments().to_selections(); // TODO(anp): Migrate guardian review thread settings to a PathUri fallback cwd so foreign // parent environments do not fall back to the host-native config cwd. let parent_turn_legacy_fallback_cwd = params - .parent_turn - .environments + .parent_context + .environments() .primary() .and_then(|environment| environment.cwd().to_abs_path().ok()) - .unwrap_or_else(|| params.parent_turn.config.cwd.clone()); + .unwrap_or_else(|| params.parent_context.turn().config.cwd.clone()); let submission = review_session.io.submit_with_trace( Op::UserInput { @@ -905,7 +907,7 @@ async fn run_review_on_session( }, }, /*trace*/ None, - Some(params.parent_turn.sub_id.clone()), + Some(params.parent_context.turn().sub_id.clone()), ); let submit_result = run_before_review_deadline( deadline, @@ -1308,7 +1310,7 @@ mod tests { GuardianReviewSessionParams { parent_session: Arc::new(session), - parent_turn: Arc::new(turn), + parent_context: GuardianReviewContext::from(Arc::new(turn)), spawn_config, request: GuardianApprovalRequest::Shell { id: "shell-1".to_string(), @@ -1338,7 +1340,10 @@ mod tests { let params = test_review_params().await; let manager = GuardianReviewSessionManager::default(); manager - .initialize(params.parent_session, params.parent_turn) + .initialize( + params.parent_session, + Arc::clone(params.parent_context.turn()), + ) .await .expect("initialize Guardian session"); let mode = manager diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index 6937d9164d..e09250eb47 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -487,10 +487,11 @@ async fn build_guardian_prompt_full_mode_preserves_initial_review_format() -> an async fn build_guardian_prompt_prefers_retry_reason_over_approval_reason() -> anyhow::Result<()> { let (session, turn) = guardian_test_session_and_turn_with_base_url("http://localhost").await; seed_guardian_parent_history(&session, &turn).await; + let context = GuardianReviewContext::from(&turn); let prompt = build_guardian_prompt_items_with_parent_turn( session.as_ref(), - Some(turn.as_ref()), + Some(&context), ApprovalRequestReasons { approval: Some("A policy rule requires approval.".to_string()), retry: Some("The sandbox blocked the initial command.".to_string()), @@ -518,6 +519,7 @@ async fn build_guardian_prompt_prefers_retry_reason_over_approval_reason() -> an async fn build_guardian_prompt_truncates_oversized_approval_reason() -> anyhow::Result<()> { let (session, turn) = guardian_test_session_and_turn_with_base_url("http://localhost").await; seed_guardian_parent_history(&session, &turn).await; + let context = GuardianReviewContext::from(&turn); let approval_reason = format!("policy-start {} policy-end", "x".repeat(10_000)); let expected_reason = codex_utils_output_truncation::truncate_text( &approval_reason, @@ -526,7 +528,7 @@ async fn build_guardian_prompt_truncates_oversized_approval_reason() -> anyhow:: let prompt = build_guardian_prompt_items_with_parent_turn( session.as_ref(), - Some(turn.as_ref()), + Some(&context), ApprovalRequestReasons { approval: Some(approval_reason), retry: None, @@ -618,10 +620,11 @@ async fn build_guardian_prompt_includes_parent_turn_denied_reads() -> anyhow::Re let session = Arc::new(session); let turn = Arc::new(turn); seed_guardian_parent_history(&session, &turn).await; + let context = GuardianReviewContext::from(&turn); let prompt = build_guardian_prompt_items_with_parent_turn( session.as_ref(), - Some(turn.as_ref()), + Some(&context), ApprovalRequestReasons { approval: None, retry: Some("Sandbox denied reading /repo/private/secret.txt.".to_string()), diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index cffc6baa43..02a63adf37 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -1363,7 +1363,7 @@ async fn maybe_request_mcp_tool_approval( let review_id = new_guardian_review_id(); let decision = review_approval_request( sess, - turn_context, + step_context, review_id.clone(), build_guardian_mcp_tool_review_request(call_id, invocation, Some(metadata)), Default::default(), diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index e2b39499ea..213d009a23 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -2478,12 +2478,14 @@ impl Session { )] pub(crate) async fn request_permissions_for_environment( self: &Arc, - turn_context: &Arc, + review_context: impl Into, call_id: String, args: RequestPermissionsArgs, environment: TurnEnvironmentSelection, cancellation_token: CancellationToken, ) -> Option { + let review_context = review_context.into(); + let turn_context = review_context.turn(); match turn_context.as_ref().approval_policy() { AskForApproval::Never => { return Some(RequestPermissionsResponse { @@ -2527,7 +2529,6 @@ impl Session { }; let review_id = crate::guardian::new_guardian_review_id(); let session = Arc::clone(self); - let turn = Arc::clone(turn_context); let request = crate::guardian::GuardianApprovalRequest::RequestPermissions { id: call_id, turn_id: turn_context.sub_id.clone(), @@ -2536,7 +2537,7 @@ impl Session { }; let review_rx = crate::guardian::spawn_approval_request_review( session, - turn, + review_context.clone(), review_id, request, /*retry_reason*/ None, diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index c25947df89..877ba93dce 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -1291,7 +1291,7 @@ async fn danger_full_access_tool_attempts_do_not_enforce_managed_network() -> an let mut tool = ProbeToolRuntime::default(); let tool_ctx = crate::tools::sandboxing::ToolCtx { session: Arc::clone(&session), - turn: Arc::clone(&turn), + step_context: StepContext::for_test(Arc::clone(&turn)), call_id: "probe-call".to_string(), tool_name: codex_tools::ToolName::plain("probe"), }; @@ -6477,7 +6477,7 @@ async fn request_permissions_emits_event_when_granular_policy_allows_requests() .selection(); session .request_permissions_for_environment( - &turn_context, + turn_context.as_ref(), call_id, codex_protocol::request_permissions::RequestPermissionsArgs { environment_id: None, @@ -6727,7 +6727,7 @@ async fn request_permissions_response_materializes_session_cwd_grants_before_rec .selection(); session .request_permissions_for_environment( - &turn_context, + turn_context.as_ref(), call_id, codex_protocol::request_permissions::RequestPermissionsArgs { environment_id: None, @@ -6819,7 +6819,7 @@ async fn request_permissions_is_auto_denied_when_granular_policy_blocks_tool_req .selection(); let response = session .request_permissions_for_environment( - &turn_context, + turn_context.as_ref(), call_id, codex_protocol::request_permissions::RequestPermissionsArgs { environment_id: None, diff --git a/codex-rs/core/src/tools/approvals.rs b/codex-rs/core/src/tools/approvals.rs index b52b50fe79..708583c466 100644 --- a/codex-rs/core/src/tools/approvals.rs +++ b/codex-rs/core/src/tools/approvals.rs @@ -8,6 +8,7 @@ use crate::guardian::routes_approval_to_guardian_with_reviewer; use crate::hook_runtime::run_permission_request_hooks; use crate::sandboxing::SandboxPermissions; use crate::session::session::Session; +use crate::session::step_context::StepContext; use crate::session::turn_context::TurnContext; use crate::tools::flat_tool_name; use crate::tools::hook_names::HookToolName; @@ -35,7 +36,7 @@ use std::sync::Arc; #[derive(Clone)] pub(crate) struct ApprovalContext { - pub(crate) turn: Arc, + pub(crate) step_context: Arc, pub(crate) call_id: String, pub(crate) tool_name: ToolName, pub(crate) strict_auto_review: bool, @@ -308,7 +309,7 @@ impl Session { // 2. If StrictAutoReview || Guardian enabled, then Guardian. Else, user. let resolution = match run_permission_request_hooks( self, - &ctx.turn, + &ctx.step_context.turn, permission_request_run_id.as_deref().unwrap_or(&ctx.call_id), action.permission_request_payload(), ) @@ -336,7 +337,7 @@ impl Session { let reviewer = if ctx.strict_auto_review { ApprovalReviewer::Guardian } else { - ApprovalReviewer::for_turn(&ctx.turn) + ApprovalReviewer::for_turn(&ctx.step_context.turn) }; let decision = match reviewer { @@ -368,7 +369,7 @@ impl Session { review_approval_request( self, - &ctx.turn, + &ctx.step_context, review_id, action, ApprovalRequestReasons { @@ -422,7 +423,7 @@ impl Session { .or_else(|| justification.clone()); with_cached_approval(&self.services, tool_name, action.cache_keys(), || async { self.request_command_approval( - &ctx.turn, + &ctx.step_context.turn, ctx.call_id.clone(), /*approval_id*/ None, Some(environment_id.clone()), @@ -454,7 +455,7 @@ impl Session { if reason.is_some() { return self .request_patch_approval( - &ctx.turn, + &ctx.step_context.turn, ctx.call_id.clone(), changes.as_ref().clone(), reason, @@ -468,7 +469,7 @@ impl Session { action.cache_keys(), || async { self.request_patch_approval( - &ctx.turn, + &ctx.step_context.turn, ctx.call_id.clone(), changes.as_ref().clone(), /*reason*/ None, @@ -490,7 +491,7 @@ fn record_resolution(ctx: &ApprovalContext, resolution: &ApprovalResolution) { ApprovalResolutionSource::User => ToolDecisionSource::User, }; let tool_name = flat_tool_name(&ctx.tool_name); - ctx.turn.session_telemetry.tool_decision( + ctx.step_context.turn.session_telemetry.tool_decision( tool_name.as_ref(), &ctx.call_id, &resolution.decision, diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index 42c398ed64..f90d8d21db 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -10,6 +10,7 @@ use crate::apply_patch; use crate::apply_patch::convert_apply_patch_to_protocol; use crate::function_tool::FunctionCallError; use crate::session::session::Session; +use crate::session::step_context::StepContext; use crate::session::turn_context::TurnContext; use crate::session::turn_context::TurnEnvironment; use crate::tools::context::ApplyPatchToolOutput; @@ -397,7 +398,7 @@ impl ApplyPatchHandler { codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => { let tool_ctx = ToolCtx { session, - turn, + step_context: Arc::clone(&step_context), call_id, tool_name, }; @@ -487,11 +488,12 @@ pub(crate) async fn intercept_apply_patch( fs: &dyn ExecutorFileSystem, turn_environment: TurnEnvironment, session: Arc, - turn: Arc, + step_context: Arc, tracker: Option<&SharedTurnDiffTracker>, call_id: &str, tool_name: &str, ) -> Result, FunctionCallError> { + let turn = &step_context.turn; let sandbox = turn.file_system_sandbox_context(/*additional_permissions*/ None, &turn_environment); match codex_apply_patch::maybe_parse_apply_patch_verified(command, cwd, fs, Some(&sandbox)) @@ -500,7 +502,7 @@ pub(crate) async fn intercept_apply_patch( codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => { let tool_ctx = ToolCtx { session, - turn, + step_context, call_id: call_id.to_string(), tool_name: ToolName::plain(tool_name), }; @@ -533,7 +535,7 @@ async fn execute_verified_patch( .await .unwrap_or_else(|_| patch_permissions_without_path_matching(&action)); let apply = apply_patch::prepare_apply_patch( - tool_ctx.turn.as_ref(), + tool_ctx.step_context.turn.as_ref(), turn_environment.permission_profile(), &file_system_sandbox_policy, action, @@ -546,7 +548,7 @@ async fn execute_verified_patch( ); let event_ctx = ToolEventCtx::new( tool_ctx.session.as_ref(), - tool_ctx.turn.as_ref(), + tool_ctx.step_context.turn.as_ref(), &tool_ctx.call_id, tracker, ); @@ -568,8 +570,8 @@ async fn execute_verified_patch( &mut runtime, &request, &tool_ctx, - tool_ctx.turn.as_ref(), - tool_ctx.turn.approval_policy(), + tool_ctx.step_context.turn.as_ref(), + tool_ctx.step_context.turn.approval_policy(), ) .await .map(|result| result.output); @@ -579,7 +581,7 @@ async fn execute_verified_patch( }; let event_ctx = ToolEventCtx::new( tool_ctx.session.as_ref(), - tool_ctx.turn.as_ref(), + tool_ctx.step_context.turn.as_ref(), &tool_ctx.call_id, tracker, ); diff --git a/codex-rs/core/src/tools/handlers/request_permissions.rs b/codex-rs/core/src/tools/handlers/request_permissions.rs index 9fcb1c642c..4d66985cb7 100644 --- a/codex-rs/core/src/tools/handlers/request_permissions.rs +++ b/codex-rs/core/src/tools/handlers/request_permissions.rs @@ -46,7 +46,6 @@ impl RequestPermissionsHandler { ) -> Result, FunctionCallError> { let ToolInvocation { session, - turn, step_context, cancellation_token, call_id, @@ -94,7 +93,7 @@ impl RequestPermissionsHandler { let response = session .request_permissions_for_environment( - &turn, + &step_context, call_id, args, turn_environment.selection(), diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 20c6a5a359..95a8c536c9 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -7,7 +7,7 @@ use tokio_util::sync::CancellationToken; use crate::exec::ExecParams; use crate::exec_policy::ExecApprovalRequest; use crate::function_tool::FunctionCallError; -use crate::session::turn_context::TurnContext; +use crate::session::step_context::StepContext; use crate::session::turn_context::TurnEnvironment; use crate::shell::ShellType; use crate::tools::context::FunctionToolOutput; @@ -53,7 +53,7 @@ struct RunExecLikeArgs { additional_permissions: Option, prefix_rule: Option>, session: Arc, - turn: Arc, + step_context: Arc, turn_environment: TurnEnvironment, tracker: crate::tools::context::SharedTurnDiffTracker, call_id: String, @@ -70,12 +70,13 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result Result Result Option { Some(crate::exec::StdoutStream { - sub_id: ctx.turn.sub_id.clone(), + sub_id: ctx.step_context.turn.sub_id.clone(), call_id: ctx.call_id.clone(), tx_event: ctx.session.get_tx_event(), }) diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index 474471ace4..139d8cd766 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -4,6 +4,7 @@ use crate::exec::ExecExpiration; use crate::exec::cancel_when_either; use crate::exec::is_likely_sandbox_denied; use crate::guardian::GuardianApprovalRequest; +use crate::guardian::GuardianReviewContext; use crate::guardian::new_guardian_review_id; use crate::guardian::review_approval_request; use crate::guardian::routes_approval_to_guardian; @@ -199,8 +200,8 @@ pub(super) async fn try_run_zsh_fork( arg0, sandbox_policy_cwd, windows_sandbox_workspace_roots, - codex_linux_sandbox_exe: ctx.turn.config.codex_linux_sandbox_exe.clone(), - use_legacy_landlock: ctx.turn.config.features.use_legacy_landlock(), + codex_linux_sandbox_exe: ctx.step_context.turn.config.codex_linux_sandbox_exe.clone(), + use_legacy_landlock: ctx.step_context.turn.config.features.use_legacy_landlock(), }; let main_execve_wrapper_exe = ctx .session @@ -234,11 +235,11 @@ pub(super) async fn try_run_zsh_fork( let escalation_policy = CoreShellActionProvider { policy: Arc::clone(&exec_policy), session: Arc::clone(&ctx.session), - turn: Arc::clone(&ctx.turn), + review_context: GuardianReviewContext::from(&ctx.step_context), call_id: ctx.call_id.clone(), environment_id: req.turn_environment.environment_id.clone(), tool_name: GuardianCommandSource::Shell, - approval_policy: ctx.turn.approval_policy(), + approval_policy: ctx.step_context.turn.approval_policy(), permission_profile: command_executor.permission_profile.clone(), sandbox_permissions: req.sandbox_permissions, approval_sandbox_permissions, @@ -314,17 +315,17 @@ pub(crate) async fn prepare_unified_exec_zsh_fork( arg0: exec_request.arg0.clone(), sandbox_policy_cwd, windows_sandbox_workspace_roots: exec_request.windows_sandbox_workspace_roots.clone(), - codex_linux_sandbox_exe: ctx.turn.config.codex_linux_sandbox_exe.clone(), - use_legacy_landlock: ctx.turn.config.features.use_legacy_landlock(), + codex_linux_sandbox_exe: ctx.step_context.turn.config.codex_linux_sandbox_exe.clone(), + use_legacy_landlock: ctx.step_context.turn.config.features.use_legacy_landlock(), }; let escalation_policy = CoreShellActionProvider { policy: Arc::clone(&exec_policy), session: Arc::clone(&ctx.session), - turn: Arc::clone(&ctx.turn), + review_context: GuardianReviewContext::from(&ctx.step_context), call_id: ctx.call_id.clone(), environment_id: req.turn_environment.environment_id.clone(), tool_name: GuardianCommandSource::UnifiedExec, - approval_policy: ctx.turn.approval_policy(), + approval_policy: ctx.step_context.turn.approval_policy(), permission_profile: exec_request.permission_profile.clone(), sandbox_permissions: req.sandbox_permissions, approval_sandbox_permissions: approval_sandbox_permissions( @@ -354,7 +355,7 @@ pub(crate) async fn prepare_unified_exec_zsh_fork( struct CoreShellActionProvider { policy: Arc>, session: Arc, - turn: Arc, + review_context: GuardianReviewContext, call_id: String, environment_id: String, tool_name: GuardianCommandSource, @@ -442,7 +443,8 @@ impl CoreShellActionProvider { let command = join_program_and_argv(program, argv); let workdir = workdir.clone(); let session = self.session.clone(); - let turn = self.turn.clone(); + let review_context = self.review_context.clone(); + let turn = Arc::clone(review_context.turn()); let call_id = self.call_id.clone(); let approval_id = Some(Uuid::new_v4().to_string()); let environment_id = Some(self.environment_id.clone()); @@ -477,7 +479,7 @@ impl CoreShellActionProvider { if let Some(review_id) = guardian_review_id { return review_approval_request( &session, - &turn, + review_context, review_id, GuardianApprovalRequest::Execve { id: call_id.clone(), @@ -615,7 +617,7 @@ impl CoreShellActionProvider { InterceptedExecPolicyContext { approval_policy: self.approval_policy, permission_profile: self.permission_profile.clone(), - windows_sandbox_level: self.turn.windows_sandbox_level, + windows_sandbox_level: self.review_context.turn().windows_sandbox_level, sandbox_permissions: self.approval_sandbox_permissions, enable_shell_wrapper_parsing: ENABLE_INTERCEPTED_EXEC_POLICY_SHELL_WRAPPER_PARSING, diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs index cfd95e5f73..009ec80379 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs @@ -8,6 +8,7 @@ use super::extract_shell_script; use super::join_program_and_argv; use super::map_exec_result; use crate::config::Constrained; +use crate::guardian::GuardianReviewContext; use crate::sandboxing::SandboxPermissions; use crate::session::tests::make_session_and_context; use anyhow::Context; @@ -425,7 +426,7 @@ async fn preapproved_additional_permissions_escalate_intercepted_exec() -> anyho let provider = CoreShellActionProvider { policy: Arc::new(RwLock::new(codex_execpolicy::Policy::empty())), session: Arc::new(session), - turn: Arc::new(turn_context), + review_context: GuardianReviewContext::from(Arc::new(turn_context)), call_id: "preapproved-additional-permissions".to_string(), environment_id: "local".to_string(), tool_name: GuardianCommandSource::Shell, @@ -563,7 +564,7 @@ async fn execve_permission_request_hook_short_circuits_prompt() -> anyhow::Resul let provider = CoreShellActionProvider { policy: std::sync::Arc::new(RwLock::new(codex_execpolicy::Policy::empty())), session: std::sync::Arc::new(session), - turn: std::sync::Arc::new(turn_context), + review_context: GuardianReviewContext::from(Arc::new(turn_context)), call_id: "execve-hook-call".to_string(), environment_id: "local".to_string(), tool_name: GuardianCommandSource::Shell, @@ -773,7 +774,7 @@ prefix_rule(pattern = ["{cat_path_literal}"], decision = "allow") let provider = CoreShellActionProvider { policy: Arc::new(RwLock::new(policy)), session: Arc::new(session), - turn: Arc::new(turn_context), + review_context: GuardianReviewContext::from(Arc::new(turn_context)), call_id: "deny-read-prefix-allow".to_string(), environment_id: "local".to_string(), tool_name: GuardianCommandSource::Shell, @@ -809,7 +810,7 @@ async fn denied_reads_keep_granular_sandbox_rejection_for_escalation() -> anyhow let provider = CoreShellActionProvider { policy: Arc::new(RwLock::new(PolicyParser::new().build())), session: Arc::new(session), - turn: Arc::new(turn_context), + review_context: GuardianReviewContext::from(Arc::new(turn_context)), call_id: "deny-read-granular-sandbox-reject".to_string(), environment_id: "local".to_string(), tool_name: GuardianCommandSource::Shell, diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 3e02e215c7..e636d0534b 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -274,7 +274,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt let mut launch = network.remote_launch_config().await.map_err(|err| { ToolError::Codex(CodexErr::Io(io::Error::other(err.to_string()))) })?; - if routes_approval_to_guardian(&ctx.turn) + if routes_approval_to_guardian(&ctx.step_context.turn) && network.remote_policy_decider().is_some() { let timeout = ctx diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 04014405e3..cbc9bc73b0 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -7,7 +7,7 @@ use crate::sandboxing::ExecOptions; use crate::sandboxing::SandboxPermissions; use crate::session::session::Session; -use crate::session::turn_context::TurnContext; +use crate::session::step_context::StepContext; use crate::session::turn_context::TurnEnvironment; use crate::state::SessionServices; use crate::tools::hook_names::HookToolName; @@ -348,7 +348,7 @@ pub(crate) trait Sandboxable { pub(crate) struct ToolCtx { pub session: Arc, - pub turn: Arc, + pub step_context: Arc, pub call_id: String, pub tool_name: ToolName, } diff --git a/codex-rs/core/src/unified_exec/async_watcher.rs b/codex-rs/core/src/unified_exec/async_watcher.rs index 5ffbfaaafd..c301e73298 100644 --- a/codex-rs/core/src/unified_exec/async_watcher.rs +++ b/codex-rs/core/src/unified_exec/async_watcher.rs @@ -56,7 +56,7 @@ pub(crate) fn start_streaming_output( } = process.output_handles().clone(); let session_ref = Arc::clone(&context.session); - let turn_ref = Arc::clone(&context.turn); + let turn_ref = Arc::clone(&context.step_context.turn); let call_id = context.call_id.clone(); tokio::spawn(async move { diff --git a/codex-rs/core/src/unified_exec/async_watcher_tests.rs b/codex-rs/core/src/unified_exec/async_watcher_tests.rs index 66fd8dba11..2899f8cdfe 100644 --- a/codex-rs/core/src/unified_exec/async_watcher_tests.rs +++ b/codex-rs/core/src/unified_exec/async_watcher_tests.rs @@ -49,7 +49,11 @@ async fn streaming_output_harness() -> anyhow::Result { .await?, ); let (session, turn, rx_event) = make_session_and_context_with_rx().await; - let context = UnifiedExecContext::new(session, turn, "streaming-output-test".to_string()); + let context = UnifiedExecContext::new( + session, + crate::session::step_context::StepContext::for_test(turn), + "streaming-output-test".to_string(), + ); let transcript = Arc::new(tokio::sync::Mutex::new(HeadTailBuffer::default())); start_streaming_output(&process, &context, Arc::clone(&transcript)); @@ -154,11 +158,11 @@ async fn exit_watcher_waits_for_late_network_denial_before_classifying_end() -> late_denial_armed_rx.await.expect("late denial armed"); #[allow(deprecated)] - let cwd = context.turn.cwd.clone().into(); + let cwd = context.step_context.turn.cwd.clone().into(); spawn_exit_watcher( Arc::clone(&process), Arc::clone(&context.session), - Arc::clone(&context.turn), + Arc::clone(&context.step_context.turn), context.call_id, vec!["proof".to_string()], cwd, diff --git a/codex-rs/core/src/unified_exec/mod.rs b/codex-rs/core/src/unified_exec/mod.rs index ac146dd448..e3a1363ae7 100644 --- a/codex-rs/core/src/unified_exec/mod.rs +++ b/codex-rs/core/src/unified_exec/mod.rs @@ -38,6 +38,7 @@ use tokio::sync::Mutex; use crate::sandboxing::SandboxPermissions; use crate::session::session::Session; +use crate::session::step_context::StepContext; use crate::session::turn_context::TurnContext; use crate::session::turn_context::TurnEnvironment; use crate::shell::ShellType; @@ -74,15 +75,15 @@ pub(crate) const MAX_UNIFIED_EXEC_PROCESSES: usize = 64; pub(crate) struct UnifiedExecContext { pub session: Arc, - pub turn: Arc, + pub step_context: Arc, pub call_id: String, } impl UnifiedExecContext { - pub fn new(session: Arc, turn: Arc, call_id: String) -> Self { + pub fn new(session: Arc, step_context: Arc, call_id: String) -> Self { Self { session, - turn, + step_context, call_id, } } diff --git a/codex-rs/core/src/unified_exec/mod_tests.rs b/codex-rs/core/src/unified_exec/mod_tests.rs index fa6ae4e5a0..fd842888b1 100644 --- a/codex-rs/core/src/unified_exec/mod_tests.rs +++ b/codex-rs/core/src/unified_exec/mod_tests.rs @@ -124,8 +124,11 @@ async fn exec_command_with_tty( ) .await?, ); - let context = - UnifiedExecContext::new(Arc::clone(session), Arc::clone(turn), "call".to_string()); + let context = UnifiedExecContext::new( + Arc::clone(session), + crate::session::step_context::StepContext::for_test(Arc::clone(turn)), + "call".to_string(), + ); let started_at = Instant::now(); let process_started_alive = !process.has_exited() && process.exit_code().is_none(); if process_started_alive { diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 6511d03d8d..82c3cbd4b5 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -344,7 +344,7 @@ async fn emit_failed_initial_exec_end_if_unstored( emit_failed_exec_end_for_unified_exec( Arc::clone(&context.session), - Arc::clone(&context.turn), + Arc::clone(&context.step_context.turn), context.call_id.clone(), request.command.clone(), cwd, @@ -449,12 +449,13 @@ impl UnifiedExecProcessManager { let transcript = Arc::new(tokio::sync::Mutex::new(HeadTailBuffer::default())); let event_ctx = ToolEventCtx::new( context.session.as_ref(), - context.turn.as_ref(), + context.step_context.turn.as_ref(), &context.call_id, /*turn_diff_tracker*/ None, ); let plugin_attribution = cwd.to_abs_path().ok().and_then(|cwd| { context + .step_context .turn .plugin_attribution_for_command(&request.command, &cwd) }); @@ -627,7 +628,7 @@ impl UnifiedExecProcessManager { let exit = exit_code.unwrap_or(-1); emit_exec_end_for_unified_exec( Arc::clone(&context.session), - Arc::clone(&context.turn), + Arc::clone(&context.step_context.turn), context.call_id.clone(), request.command.clone(), cwd.clone(), @@ -655,7 +656,12 @@ impl UnifiedExecProcessManager { chunk_id, wall_time, raw_output: collected, - truncation_policy: context.turn.model_info.truncation_policy.into(), + truncation_policy: context + .step_context + .turn + .model_info + .truncation_policy + .into(), max_output_tokens: request.max_output_tokens, process_id: response_process_id, exit_code, @@ -952,7 +958,7 @@ impl UnifiedExecProcessManager { spawn_exit_watcher( Arc::clone(&process), Arc::clone(&context.session), - Arc::clone(&context.turn), + Arc::clone(&context.step_context.turn), context.call_id.clone(), command.to_vec(), cwd, @@ -1132,8 +1138,9 @@ impl UnifiedExecProcessManager { cwd: PathUri, context: &UnifiedExecContext, ) -> Result<(UnifiedExecProcess, Option), UnifiedExecError> { + let turn = &context.step_context.turn; let local_policy_env = create_env( - &context.turn.config.permissions.shell_environment_policy, + &turn.config.permissions.shell_environment_policy, /*thread_id*/ None, ); let mut env = local_policy_env.clone(); @@ -1146,7 +1153,7 @@ impl UnifiedExecProcessManager { let env = apply_unified_exec_env(env); let exec_server_env_config = ExecServerEnvConfig { policy: exec_env_policy_from_shell_policy( - &context.turn.config.permissions.shell_environment_policy, + &turn.config.permissions.shell_environment_policy, ), local_policy_env, }; @@ -1158,9 +1165,9 @@ impl UnifiedExecProcessManager { .exec_policy .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &request.command, - approval_policy: context.turn.approval_policy(), + approval_policy: turn.approval_policy(), permission_profile: request.turn_environment.permission_profile().clone(), - windows_sandbox_level: context.turn.windows_sandbox_level, + windows_sandbox_level: turn.windows_sandbox_level, sandbox_permissions: if request.additional_permissions_preapproved { crate::sandboxing::SandboxPermissions::UseDefault } else { @@ -1180,8 +1187,7 @@ impl UnifiedExecProcessManager { turn_environment: request.turn_environment.clone(), env, exec_server_env_config: Some(exec_server_env_config), - explicit_env_overrides: context - .turn + explicit_env_overrides: turn .config .permissions .shell_environment_policy @@ -1198,18 +1204,12 @@ impl UnifiedExecProcessManager { }; let tool_ctx = ToolCtx { session: context.session.clone(), - turn: context.turn.clone(), + step_context: Arc::clone(&context.step_context), call_id: context.call_id.clone(), tool_name: ToolName::plain("exec_command"), }; orchestrator - .run( - &mut runtime, - &req, - &tool_ctx, - &context.turn, - context.turn.approval_policy(), - ) + .run(&mut runtime, &req, &tool_ctx, turn, turn.approval_policy()) .await .map(|result| (result.output, result.deferred_network_approval)) .map_err(|err| match err { 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 6a8ba5f4c7..613b2f06d9 100644 --- a/codex-rs/core/src/unified_exec/process_manager_tests.rs +++ b/codex-rs/core/src/unified_exec/process_manager_tests.rs @@ -377,7 +377,7 @@ async fn failed_initial_end_for_unstored_process_uses_fallback_output() { let (session, turn, rx_event) = crate::session::tests::make_session_and_context_with_rx().await; let context = UnifiedExecContext::new( Arc::clone(&session), - Arc::clone(&turn), + crate::session::step_context::StepContext::for_test(Arc::clone(&turn)), "call-unified-denied".to_string(), ); let request = ExecCommandRequest { diff --git a/codex-rs/core/tests/suite/remote_env.rs b/codex-rs/core/tests/suite/remote_env.rs index 6f7843e614..d3f0f73a24 100644 --- a/codex-rs/core/tests/suite/remote_env.rs +++ b/codex-rs/core/tests/suite/remote_env.rs @@ -1390,6 +1390,178 @@ async fn deferred_executor_spawn_agent_inherits_ready_step_environments( Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn deferred_executor_guardian_uses_newly_ready_step_environment() -> Result<()> { + const WAIT_CALL_ID: &str = "wait-for-guardian-environment"; + const EXEC_CALL_ID: &str = "guardian-ready-environment-command"; + const DENIAL_RATIONALE: &str = "The remote environment policy denies this action."; + + let listener = TcpListener::bind("127.0.0.1:0").await?; + let server = start_mock_server().await; + let completed_response = + |id: &str, item| sse(vec![ev_response_created(id), item, ev_completed(id)]); + let responses = mount_sse_sequence( + &server, + vec![ + completed_response( + "resp-guardian-wait", + ev_function_call( + WAIT_CALL_ID, + "wait_for_environment", + &json!({ "environment_id": REMOTE_ENVIRONMENT_ID }).to_string(), + ), + ), + completed_response( + "resp-guardian-command", + ev_function_call( + EXEC_CALL_ID, + "exec_command", + &json!({ + "cmd": "printf guardian-should-not-run", + "environment_id": REMOTE_ENVIRONMENT_ID, + "sandbox_permissions": SandboxPermissions::RequireEscalated, + "justification": "Review the newly ready remote environment.", + }) + .to_string(), + ), + ), + completed_response( + "resp-guardian-review", + ev_assistant_message( + "msg-guardian-review", + &json!({ "outcome": "deny", "rationale": DENIAL_RATIONALE }).to_string(), + ), + ), + completed_response( + "resp-guardian-done", + ev_assistant_message("msg-guardian-done", "done"), + ), + ], + ) + .await; + let mut builder = test_codex() + .with_exec_server_url(format!("ws://{}", listener.local_addr()?)) + .with_config(|config| { + config.project_doc_max_bytes = 0; + config.use_experimental_unified_exec_tool = true; + config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); + config.approvals_reviewer = ApprovalsReviewer::AutoReview; + assert!(config.features.enable(Feature::DeferredExecutor).is_ok()); + assert!(config.features.enable(Feature::UnifiedExec).is_ok()); + }); + let (attach_tx, attach_rx) = tokio::sync::oneshot::channel(); + let (shutdown_tx, shutdown_rx) = tokio::sync::oneshot::channel(); + let exec_server = tokio::spawn(serve_environment_with_agents_md( + listener, + "", + attach_rx, + shutdown_rx, + )); + let test = timeout( + Duration::from_secs(5), + builder.build_with_remote_and_local_env(&server), + ) + .await + .context("thread startup should not wait for the remote environment")??; + 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"); + let local_denied_path = local_cwd.canonicalize()?.join("private"); + let remote_selection = TurnEnvironmentSelection { + environment_id: REMOTE_ENVIRONMENT_ID.to_string(), + cwd: PathUri::from_abs_path(&remote_cwd), + workspace_roots: vec![PathUri::from_abs_path(&remote_cwd)], + }; + let permission_profile = PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry::new( + FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + FileSystemAccessMode::Read, + ), + FileSystemSandboxEntry::new( + FileSystemPath::Special { + value: FileSystemSpecialPath::project_roots(Some("private".to_string())), + }, + FileSystemAccessMode::Deny, + ), + ]), + NetworkSandboxPolicy::Restricted, + ); + let (sandbox_policy, permission_profile) = + turn_permission_fields(permission_profile, test.config.cwd.as_path()); + + test.codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "review a command after the remote environment becomes ready".into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: ThreadSettingsOverrides { + environments: Some(TurnEnvironmentSelections::new( + test.config.cwd.clone(), + vec![remote_selection, local(local_cwd.clone())], + )), + sandbox_policy: Some(sandbox_policy), + permission_profile, + ..Default::default() + }, + }) + .await?; + wait_for_response_request_count(&responses, /*expected_count*/ 1).await; + attach_tx.send(()).expect("attach remote environment"); + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + + let requests = responses.requests(); + assert!( + requests[1] + .function_call_output_content_and_success(WAIT_CALL_ID) + .is_some(), + "the reviewed command should follow the ready-environment step" + ); + let guardian_request = requests + .iter() + .find(|request| { + request.body_json()["client_metadata"]["x-openai-subagent"].as_str() == Some("guardian") + }) + .context("expected Guardian review request")?; + let guardian_context = guardian_request.message_input_texts("user").join("\n"); + for expected in [ + "".to_string(), + format!("{}", remote_cwd.display()), + format!("- path `{}`", remote_denied_path.display()), + ] { + assert!( + guardian_context.contains(&expected), + "Guardian omitted `{expected}` from the ready environment context: {guardian_context}" + ); + } + assert!( + !guardian_context.contains(&format!("- path `{}`", local_denied_path.display())), + "Guardian used the stale local environment's denied-read policy: {guardian_context}" + ); + let rejection = requests + .iter() + .find_map(|request| request.function_call_output_text(EXEC_CALL_ID)) + .context("Guardian denial should be returned to the parent model")?; + assert!(rejection.contains(DENIAL_RATIONALE)); + + shutdown_tx + .send(()) + .expect("stop remote environment server"); + exec_server.await?; + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn deferred_executor_loads_agents_md_when_environment_becomes_ready() -> Result<()> { const AGENTS_CONTENT: &str = "REMOTE_AGENTS_INSTRUCTIONS";