diff --git a/codex-rs/core/src/guardian/mod.rs b/codex-rs/core/src/guardian/mod.rs index 4eafad82aa..b1e181cd30 100644 --- a/codex-rs/core/src/guardian/mod.rs +++ b/codex-rs/core/src/guardian/mod.rs @@ -23,6 +23,8 @@ use codex_protocol::protocol::GuardianAssessmentOutcome; use serde::Deserialize; use serde::Serialize; +use crate::tools::sandboxing::ApprovalRequestReasons; + pub(crate) use approval_request::GuardianApprovalRequest; pub(crate) use approval_request::GuardianMcpAnnotations; pub(crate) use approval_request::GuardianNetworkAccessTrigger; diff --git a/codex-rs/core/src/guardian/prompt.rs b/codex-rs/core/src/guardian/prompt.rs index d8aa425e9d..75b4369bb6 100644 --- a/codex-rs/core/src/guardian/prompt.rs +++ b/codex-rs/core/src/guardian/prompt.rs @@ -12,11 +12,14 @@ 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 codex_utils_output_truncation::TruncationPolicy; use codex_utils_output_truncation::approx_bytes_for_tokens; use codex_utils_output_truncation::approx_token_count; use codex_utils_output_truncation::approx_tokens_from_byte_count; +use codex_utils_output_truncation::truncate_text; use super::AUTO_REVIEW_DENIED_ACTION_APPROVAL_DEVELOPER_PREFIX; +use super::ApprovalRequestReasons; use super::GUARDIAN_MAX_MESSAGE_ENTRY_TOKENS; use super::GUARDIAN_MAX_MESSAGE_TRANSCRIPT_TOKENS; use super::GUARDIAN_MAX_TOOL_ENTRY_TOKENS; @@ -27,6 +30,8 @@ use super::GuardianAssessment; use super::TRUNCATION_TAG; use super::approval_request::format_guardian_action_pretty; +const GUARDIAN_MAX_APPROVAL_REASON_TOKENS: usize = 512; + /// Transcript entry retained for guardian review after filtering. #[derive(Debug, PartialEq, Eq)] pub(crate) struct GuardianTranscriptEntry { @@ -98,7 +103,10 @@ pub(crate) async fn build_guardian_prompt_items( build_guardian_prompt_items_with_parent_turn( session, /*parent_turn*/ None, - retry_reason, + ApprovalRequestReasons { + approval: None, + retry: retry_reason, + }, request, mode, ) @@ -108,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>, - retry_reason: Option, + reasons: ApprovalRequestReasons, request: GuardianApprovalRequest, mode: GuardianPromptMode, ) -> serde_json::Result { @@ -221,7 +229,11 @@ pub(crate) async fn build_guardian_prompt_items_with_parent_turn( _ => { push_text(headings.action_intro.to_string()); push_text(">>> APPROVAL REQUEST START\n".to_string()); - if let Some(reason) = retry_reason { + if let Some(reason) = reasons.retry.or(reasons.approval) { + let reason = truncate_text( + &reason, + TruncationPolicy::Tokens(GUARDIAN_MAX_APPROVAL_REASON_TOKENS), + ); push_text("Retry reason:\n".to_string()); push_text(format!("{reason}\n\n")); } diff --git a/codex-rs/core/src/guardian/review.rs b/codex-rs/core/src/guardian/review.rs index fea53a7322..25adb738a0 100644 --- a/codex-rs/core/src/guardian/review.rs +++ b/codex-rs/core/src/guardian/review.rs @@ -31,6 +31,7 @@ use crate::turn_timing::now_unix_timestamp_ms; use crate::util::backoff; use super::AUTO_REVIEW_DENIAL_WINDOW_SIZE; +use super::ApprovalRequestReasons; use super::GUARDIAN_REVIEW_TIMEOUT; use super::GUARDIAN_REVIEWER_NAME; use super::GuardianApprovalRequest; @@ -295,7 +296,7 @@ async fn run_guardian_review( turn: Arc, review_id: String, request: GuardianApprovalRequest, - retry_reason: Option, + reasons: ApprovalRequestReasons, options: GuardianReviewOptions, ) -> ReviewDecision { let GuardianReviewOptions { @@ -392,7 +393,7 @@ async fn run_guardian_review( session.clone(), turn.clone(), request, - retry_reason.clone(), + reasons, schema, external_cancel, GUARDIAN_REVIEW_MAX_ATTEMPTS, @@ -635,7 +636,7 @@ pub(crate) async fn review_approval_request( turn: &Arc, review_id: String, request: GuardianApprovalRequest, - retry_reason: Option, + reasons: ApprovalRequestReasons, ) -> ReviewDecision { // Box the delegated review future so callers do not inline the entire // guardian session state machine into their own async stack. @@ -644,7 +645,7 @@ pub(crate) async fn review_approval_request( Arc::clone(turn), review_id, request, - retry_reason, + reasons, GuardianReviewOptions { plugin_attribution_override: None, approval_request_source: GuardianApprovalRequestSource::MainTurn, @@ -667,7 +668,10 @@ pub(crate) async fn review_approval_request_with_cancel( Arc::clone(turn), review_id, request, - retry_reason, + ApprovalRequestReasons { + approval: None, + retry: retry_reason, + }, options, ) .await @@ -830,7 +834,7 @@ async fn run_guardian_review_session_before_deadline( session: Arc, turn: Arc, request: GuardianApprovalRequest, - retry_reason: Option, + reasons: ApprovalRequestReasons, schema: serde_json::Value, external_cancel: Option, deadline: Instant, @@ -853,7 +857,7 @@ async fn run_guardian_review_session_before_deadline( parent_turn: turn.clone(), spawn_config: session_config.spawn_config, request, - retry_reason, + reasons, schema, model: session_config.model, reasoning_effort: session_config.reasoning_effort, @@ -924,7 +928,7 @@ pub(super) async fn run_guardian_review_session_with_retry( session: Arc, turn: Arc, request: GuardianApprovalRequest, - retry_reason: Option, + reasons: ApprovalRequestReasons, schema: serde_json::Value, external_cancel: Option, max_attempts: i64, @@ -937,7 +941,7 @@ pub(super) async fn run_guardian_review_session_with_retry( Arc::clone(&session), Arc::clone(&turn), request.clone(), - retry_reason.clone(), + reasons.clone(), schema.clone(), external_cancel.clone(), deadline, diff --git a/codex-rs/core/src/guardian/review_session.rs b/codex-rs/core/src/guardian/review_session.rs index 5b992bfa04..42e696c97d 100644 --- a/codex-rs/core/src/guardian/review_session.rs +++ b/codex-rs/core/src/guardian/review_session.rs @@ -52,6 +52,7 @@ use codex_features::Feature; use codex_model_provider_info::ModelProviderInfo; use codex_utils_path_uri::PathUri; +use super::ApprovalRequestReasons; use super::GUARDIAN_REVIEWER_NAME; use super::GuardianApprovalRequest; use super::prompt::BUNDLED_GUARDIAN_POLICY; @@ -80,7 +81,7 @@ pub(crate) struct GuardianReviewSessionParams { pub(crate) parent_turn: Arc, pub(crate) spawn_config: Config, pub(crate) request: GuardianApprovalRequest, - pub(crate) retry_reason: Option, + pub(crate) reasons: ApprovalRequestReasons, pub(crate) schema: Value, pub(crate) model: String, pub(crate) reasoning_effort: Option, @@ -767,7 +768,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()), - params.retry_reason.clone(), + params.reasons.clone(), params.request.clone(), prompt_mode, ) @@ -1246,7 +1247,7 @@ mod tests { additional_permissions: None, justification: Some("Inspect repo state.".to_string()), }, - retry_reason: None, + reasons: ApprovalRequestReasons::default(), schema: super::super::prompt::guardian_output_schema(), model, reasoning_effort, diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index b80361a2d1..f96562ba7d 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -461,6 +461,85 @@ async fn build_guardian_prompt_full_mode_preserves_initial_review_format() -> an Ok(()) } +#[tokio::test(flavor = "current_thread")] +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 prompt = build_guardian_prompt_items_with_parent_turn( + session.as_ref(), + Some(turn.as_ref()), + ApprovalRequestReasons { + approval: Some("A policy rule requires approval.".to_string()), + retry: Some("The sandbox blocked the initial command.".to_string()), + }, + GuardianApprovalRequest::Shell { + id: "shell-1".to_string(), + command: vec!["git".to_string(), "push".to_string()], + cwd: test_path_buf("/repo/codex-rs/core").abs(), + sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault, + additional_permissions: None, + justification: None, + }, + GuardianPromptMode::Full, + ) + .await?; + + let text = guardian_prompt_text(&prompt.items); + assert!(text.contains("Retry reason:\nThe sandbox blocked the initial command.\n\n")); + assert!(!text.contains("A policy rule requires approval.")); + + Ok(()) +} + +#[tokio::test(flavor = "current_thread")] +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 approval_reason = format!("policy-start {} policy-end", "x".repeat(10_000)); + let expected_reason = codex_utils_output_truncation::truncate_text( + &approval_reason, + codex_utils_output_truncation::TruncationPolicy::Tokens(/*tokens*/ 512), + ); + + let prompt = build_guardian_prompt_items_with_parent_turn( + session.as_ref(), + Some(turn.as_ref()), + ApprovalRequestReasons { + approval: Some(approval_reason), + retry: None, + }, + GuardianApprovalRequest::Shell { + id: "shell-1".to_string(), + command: vec!["git".to_string(), "push".to_string()], + cwd: test_path_buf("/repo/codex-rs/core").abs(), + sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault, + additional_permissions: None, + justification: None, + }, + GuardianPromptMode::Full, + ) + .await?; + + let reason_item = prompt + .items + .iter() + .find_map(|item| match item { + codex_protocol::user_input::UserInput::Text { text, .. } + if text.contains("tokens truncated") => + { + Some(text) + } + _ => None, + }) + .expect("oversized approval reason should include a truncation marker"); + assert!(reason_item.starts_with("policy-start")); + assert!(reason_item.ends_with("policy-end\n\n")); + assert_eq!(reason_item, &format!("{expected_reason}\n\n")); + + Ok(()) +} + #[tokio::test(flavor = "current_thread")] async fn build_guardian_prompt_includes_parent_turn_denied_reads() -> anyhow::Result<()> { let (mut session, mut turn) = crate::session::tests::make_session_and_context().await; @@ -504,7 +583,10 @@ async fn build_guardian_prompt_includes_parent_turn_denied_reads() -> anyhow::Re let prompt = build_guardian_prompt_items_with_parent_turn( session.as_ref(), Some(turn.as_ref()), - Some("Sandbox denied reading /repo/private/secret.txt.".to_string()), + ApprovalRequestReasons { + approval: None, + retry: Some("Sandbox denied reading /repo/private/secret.txt.".to_string()), + }, GuardianApprovalRequest::Shell { id: "shell-1".to_string(), command: vec!["cat".to_string(), "/repo/private/secret.txt".to_string()], @@ -1498,7 +1580,10 @@ async fn guardian_request_model_for_auto_review( additional_permissions: None, justification: None, }, - Some("Sandbox denied outbound git push to github.com.".to_string()), + ApprovalRequestReasons { + approval: None, + retry: Some("Sandbox denied outbound git push to github.com.".to_string()), + }, guardian_output_schema(), /*external_cancel*/ None, /*max_attempts*/ 1, @@ -1741,7 +1826,10 @@ async fn guardian_review_request_layout_matches_model_visible_request_snapshot() Arc::clone(&session), Arc::clone(&turn), request, - Some("Sandbox denied outbound git push to github.com.".to_string()), + ApprovalRequestReasons { + approval: None, + retry: Some("Sandbox denied outbound git push to github.com.".to_string()), + }, guardian_output_schema(), /*external_cancel*/ None, /*max_attempts*/ 1, @@ -1943,7 +2031,10 @@ async fn guardian_reuses_prompt_cache_key_and_appends_prior_reviews() -> anyhow: Arc::clone(&session), Arc::clone(&turn), first_request, - Some("First retry reason".to_string()), + ApprovalRequestReasons { + approval: None, + retry: Some("First retry reason".to_string()), + }, guardian_output_schema(), /*external_cancel*/ None, /*max_attempts*/ 1, @@ -1990,7 +2081,10 @@ async fn guardian_reuses_prompt_cache_key_and_appends_prior_reviews() -> anyhow: Arc::clone(&session), Arc::clone(&turn), second_request, - Some("Second retry reason".to_string()), + ApprovalRequestReasons { + approval: None, + retry: Some("Second retry reason".to_string()), + }, guardian_output_schema(), /*external_cancel*/ None, /*max_attempts*/ 1, @@ -2033,7 +2127,10 @@ async fn guardian_reuses_prompt_cache_key_and_appends_prior_reviews() -> anyhow: Arc::clone(&session), Arc::clone(&turn), third_request, - Some("Third retry reason".to_string()), + ApprovalRequestReasons { + approval: None, + retry: Some("Third retry reason".to_string()), + }, guardian_output_schema(), /*external_cancel*/ None, /*max_attempts*/ 1, @@ -2225,7 +2322,7 @@ async fn guardian_reused_trunk_ignores_stale_prior_turn_completion() -> anyhow:: additional_permissions: None, justification: Some("Need to push the first docs fix.".to_string()), }, - /*retry_reason*/ None, + ApprovalRequestReasons::default(), guardian_output_schema(), /*external_cancel*/ None, /*max_attempts*/ 1, @@ -2270,7 +2367,7 @@ async fn guardian_reused_trunk_ignores_stale_prior_turn_completion() -> anyhow:: additional_permissions: None, justification: Some("Need to push the second docs fix.".to_string()), }, - /*retry_reason*/ None, + ApprovalRequestReasons::default(), guardian_output_schema(), /*external_cancel*/ None, /*max_attempts*/ 1, @@ -2350,7 +2447,7 @@ async fn guardian_review_surfaces_responses_api_errors_in_rejection_reason() -> additional_permissions: None, justification: Some("Need to push the reviewed docs fix.".to_string()), }, - /*retry_reason*/ None, + ApprovalRequestReasons::default(), ) .await; @@ -2436,7 +2533,7 @@ async fn guardian_review_retries_transient_session_failure_then_approves() -> an Arc::clone(&session), Arc::clone(&turn), guardian_shell_request("shell-session-retry"), - /*retry_reason*/ None, + ApprovalRequestReasons::default(), guardian_output_schema(), /*external_cancel*/ None, /*max_attempts*/ 3, @@ -2478,7 +2575,7 @@ async fn guardian_review_does_not_retry_missing_assessment_payload() -> anyhow:: &turn, "review-missing-assessment".to_string(), guardian_shell_request("shell-missing-assessment"), - /*retry_reason*/ None, + ApprovalRequestReasons::default(), ) .await; @@ -2527,7 +2624,7 @@ async fn guardian_review_retries_two_parse_failures_then_approves() -> anyhow::R Arc::clone(&session), Arc::clone(&turn), guardian_shell_request("shell-parse-retry"), - /*retry_reason*/ None, + ApprovalRequestReasons::default(), guardian_output_schema(), /*external_cancel*/ None, /*max_attempts*/ 3, @@ -2582,7 +2679,7 @@ async fn guardian_review_exhausts_three_failures_with_one_terminal_event() -> an &turn, "review-exhausted-retry".to_string(), guardian_shell_request("shell-exhausted-retry"), - /*retry_reason*/ None, + ApprovalRequestReasons::default(), ) .await; @@ -2633,7 +2730,7 @@ async fn guardian_review_does_not_retry_valid_denial() -> anyhow::Result<()> { &turn, "review-valid-denial".to_string(), guardian_shell_request("shell-valid-denial"), - /*retry_reason*/ None, + ApprovalRequestReasons::default(), ) .await; @@ -2736,7 +2833,7 @@ async fn guardian_ephemeral_retry_preserves_parallel_trunk_and_fork_history() -> &turn, "review-shell-guardian-1".to_string(), initial_request, - /*retry_reason*/ None + ApprovalRequestReasons::default() ) .await, ReviewDecision::Approved @@ -2790,7 +2887,10 @@ async fn guardian_ephemeral_retry_preserves_parallel_trunk_and_fork_history() -> &turn_for_second, "review-shell-guardian-2".to_string(), second_request, - Some("trunk follow-up".to_string()), + ApprovalRequestReasons { + approval: None, + retry: Some("trunk follow-up".to_string()), + }, ) .await }); @@ -2837,7 +2937,10 @@ async fn guardian_ephemeral_retry_preserves_parallel_trunk_and_fork_history() -> &turn, "review-shell-guardian-3".to_string(), third_request, - Some("parallel follow-up".to_string()), + ApprovalRequestReasons { + approval: None, + retry: Some("parallel follow-up".to_string()), + }, ) .await; assert_eq!(third_decision, ReviewDecision::Approved); diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index b4cf21c84b..a712aadae4 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -1361,7 +1361,7 @@ async fn maybe_request_mcp_tool_approval( turn_context, review_id.clone(), build_guardian_mcp_tool_review_request(call_id, invocation, Some(metadata)), - /*retry_reason*/ None, + Default::default(), ) .await; let decision = mcp_tool_approval_decision_from_guardian(decision); diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index ca9ac79923..5a37e874d2 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -796,7 +796,7 @@ async fn review_guardian_mcp_elicitation( &turn_context, review_id.clone(), guardian_request, - /*retry_reason*/ None, + Default::default(), ) .await; Ok(Some(mcp_elicitation_response_from_guardian_decision( diff --git a/codex-rs/core/src/tools/approvals.rs b/codex-rs/core/src/tools/approvals.rs index 693f37707a..fb9d19cc93 100644 --- a/codex-rs/core/src/tools/approvals.rs +++ b/codex-rs/core/src/tools/approvals.rs @@ -190,7 +190,6 @@ impl ApprovalResolution { pub(super) async fn resolve_tool_approval( tool: &mut T, req: &Rq, - permission_request_run_id: &str, ctx: ApprovalCtx<'_>, reviewer: ApprovalReviewer, ) -> Result @@ -198,10 +197,15 @@ where T: ToolRuntime, { if let Some(permission_request) = tool.permission_request_payload(req) { + let permission_request_run_id = ctx + .reasons + .retry + .as_ref() + .map(|_| format!("{}:retry", ctx.call_id)); match run_permission_request_hooks( ctx.session, ctx.turn, - permission_request_run_id, + permission_request_run_id.as_deref().unwrap_or(ctx.call_id), permission_request, ) .await @@ -251,7 +255,7 @@ where ctx.turn, review_id, action, - ctx.retry_reason.clone(), + ctx.reasons.clone(), ) .await } diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index fbcbc596b5..80b97a0e5e 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -17,6 +17,7 @@ use crate::tools::network_approval::begin_network_approval; use crate::tools::network_approval::finish_deferred_network_approval; use crate::tools::network_approval::finish_immediate_network_approval; use crate::tools::sandboxing::ApprovalCtx; +use crate::tools::sandboxing::ApprovalRequestReasons; use crate::tools::sandboxing::ExecApprovalRequirement; use crate::tools::sandboxing::SandboxAttempt; use crate::tools::sandboxing::SandboxOverride; @@ -172,17 +173,11 @@ impl ToolOrchestrator { turn: &tool_ctx.turn, call_id: &tool_ctx.call_id, tool_name: &tool_ctx.tool_name, - retry_reason: None, + reasons: ApprovalRequestReasons::default(), network_approval_context: None, }; - resolve_tool_approval( - tool, - req, - tool_ctx.call_id.as_str(), - approval_ctx, - ApprovalReviewer::Guardian, - ) - .await?; + resolve_tool_approval(tool, req, approval_ctx, ApprovalReviewer::Guardian) + .await?; already_approved = true; } else { otel.tool_decision( @@ -202,13 +197,15 @@ impl ToolOrchestrator { turn: &tool_ctx.turn, call_id: &tool_ctx.call_id, tool_name: &tool_ctx.tool_name, - retry_reason: reason.clone(), + reasons: ApprovalRequestReasons { + approval: reason.clone(), + retry: None, + }, network_approval_context: None, }; resolve_tool_approval( tool, req, - tool_ctx.call_id.as_str(), approval_ctx, if strict_auto_review { ApprovalReviewer::Guardian @@ -391,20 +388,26 @@ impl ToolOrchestrator { && tool.should_bypass_approval(approval_policy, already_approved) && network_approval_context.is_none(); if !bypass_retry_approval { + let approval_reason = match &requirement { + ExecApprovalRequirement::NeedsApproval { reason, .. } => reason.clone(), + ExecApprovalRequirement::Skip { .. } + | ExecApprovalRequirement::Forbidden { .. } => None, + }; let approval_ctx = ApprovalCtx { session: &tool_ctx.session, turn: &tool_ctx.turn, call_id: &tool_ctx.call_id, tool_name: &tool_ctx.tool_name, - retry_reason: Some(retry_reason), + reasons: ApprovalRequestReasons { + approval: approval_reason, + retry: Some(retry_reason), + }, network_approval_context: network_approval_context.clone(), }; - let permission_request_run_id = format!("{}:retry", tool_ctx.call_id); resolve_tool_approval( tool, req, - &permission_request_run_id, approval_ctx, if strict_auto_review { ApprovalReviewer::Guardian diff --git a/codex-rs/core/src/tools/runtimes/apply_patch.rs b/codex-rs/core/src/tools/runtimes/apply_patch.rs index 6deb7ce4d9..d2eabd02c3 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch.rs @@ -139,14 +139,15 @@ impl Approvable for ApplyPatchRuntime { let session = ctx.session; let turn = ctx.turn; let call_id = ctx.call_id.to_string(); - let retry_reason = ctx.retry_reason.clone(); + let approval_reason = ctx.reasons.approval.clone(); + let retry_reason = ctx.reasons.retry.clone(); let approval_keys = self.approval_keys(req); let changes = req.changes.clone(); Box::pin(async move { - if req.permissions_preapproved && retry_reason.is_none() { + if req.permissions_preapproved && approval_reason.is_none() && retry_reason.is_none() { return ReviewDecision::Approved; } - if let Some(reason) = retry_reason { + if let Some(reason) = retry_reason.or(approval_reason) { return session .request_patch_approval( turn, diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index 2abbdea0aa..ec74869aee 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -146,8 +146,10 @@ impl Approvable for ShellRuntime { let cwd = req.cwd.clone(); let environment_id = Some(req.turn_environment.environment_id.clone()); let reason = ctx - .retry_reason + .reasons + .retry .clone() + .or_else(|| ctx.reasons.approval.clone()) .or_else(|| req.justification.clone()); let session = ctx.session; let turn = ctx.turn; 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 a2badb000e..8f81d146e6 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -477,7 +477,7 @@ impl CoreShellActionProvider { cwd: workdir.clone(), additional_permissions, }, - /*retry_reason*/ None, + Default::default(), ) .await; } diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 40bc938c22..59c73e5d39 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -185,8 +185,10 @@ impl Approvable for UnifiedExecRuntime<'_> { let command = req.command.clone(); let environment_id = Some(req.turn_environment.environment_id.clone()); let reason = ctx - .retry_reason + .reasons + .retry .clone() + .or_else(|| ctx.reasons.approval.clone()) .or_else(|| req.justification.clone()); Box::pin(async move { let native_cwd = match req.cwd.to_abs_path() { diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 07bfeca90f..d23b4127f3 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -116,13 +116,19 @@ where decision } +#[derive(Clone, Debug, Default)] +pub(crate) struct ApprovalRequestReasons { + pub(crate) approval: Option, + pub(crate) retry: Option, +} + #[derive(Clone)] pub(crate) struct ApprovalCtx<'a> { pub session: &'a Arc, pub turn: &'a Arc, pub call_id: &'a str, pub tool_name: &'a ToolName, - pub retry_reason: Option, + pub reasons: ApprovalRequestReasons, pub network_approval_context: Option, } diff --git a/codex-rs/core/tests/suite/guardian_review.rs b/codex-rs/core/tests/suite/guardian_review.rs index f3704f1d47..39b65848ea 100644 --- a/codex-rs/core/tests/suite/guardian_review.rs +++ b/codex-rs/core/tests/suite/guardian_review.rs @@ -129,6 +129,21 @@ async fn guardian_session_prewarms_and_is_reused_for_first_review() -> Result<() }); let mut builder = test_codex() .with_config(move |config| { + let rules_dir = config.codex_home.join("rules"); + fs::create_dir_all(&rules_dir).expect("create execution policy directory"); + let policy_justification = format!( + "Explicit policy approval required {} policy-justification-end", + "x".repeat(10_000) + ); + let policy_justification = + serde_json::to_string(&policy_justification).expect("serialize policy justification"); + fs::write( + rules_dir.join("default.rules"), + format!( + r#"prefix_rule(pattern=["true"], decision="prompt", justification={policy_justification})"# + ), + ) + .expect("write execution policy rule"); config.model_catalog = Some(ModelsResponse { models: vec![review_model], }); @@ -206,6 +221,12 @@ async fn guardian_session_prewarms_and_is_reused_for_first_review() -> Result<() guardian_review["client_metadata"]["thread_id"].as_str(), Some(guardian_thread_id) ); + let guardian_review_text = guardian_review.to_string(); + assert!(guardian_review_text.contains("Retry reason:")); + assert!(guardian_review_text.contains("Explicit policy approval required")); + assert!(guardian_review_text.contains("tokens truncated")); + assert!(guardian_review_text.contains("policy-justification-end")); + assert!(!guardian_review_text.contains(&"x".repeat(4_096))); let current_date = DateTime::::from_timestamp(CURRENT_TIME_AT, 0) .expect("test timestamp should be valid") .with_timezone(&Local)