diff --git a/codex-rs/core/src/guardian/mod.rs b/codex-rs/core/src/guardian/mod.rs index b4920f1ff6..176ce5c14e 100644 --- a/codex-rs/core/src/guardian/mod.rs +++ b/codex-rs/core/src/guardian/mod.rs @@ -72,6 +72,13 @@ pub(crate) struct GuardianAssessment { pub(crate) struct GuardianRejection { pub(crate) rationale: String, pub(crate) source: GuardianAssessmentDecisionSource, + pub(crate) kind: GuardianRejectionKind, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum GuardianRejectionKind { + PolicyDenial, + ReviewFailure, } #[derive(Debug, Default)] diff --git a/codex-rs/core/src/guardian/review.rs b/codex-rs/core/src/guardian/review.rs index 934e68042f..a27df23040 100644 --- a/codex-rs/core/src/guardian/review.rs +++ b/codex-rs/core/src/guardian/review.rs @@ -33,6 +33,7 @@ use super::GuardianAssessment; use super::GuardianAssessmentOutcome; use super::GuardianRejection; use super::GuardianRejectionCircuitBreakerAction; +use super::GuardianRejectionKind; use super::approval_request::guardian_assessment_action; use super::approval_request::guardian_request_target_item_id; use super::approval_request::guardian_request_turn_id; @@ -73,13 +74,20 @@ pub(crate) async fn guardian_rejection_message(session: &Session, review_id: &st .unwrap_or_else(|| GuardianRejection { rationale: "Auto-reviewer denied the action without a specific rationale.".to_string(), source: GuardianAssessmentDecisionSource::Agent, + kind: GuardianRejectionKind::PolicyDenial, }); match rejection.source { - GuardianAssessmentDecisionSource::Agent => format!( - "This action was rejected due to unacceptable risk.\nReason: {}\n{}", - rejection.rationale.trim(), - GUARDIAN_REJECTION_INSTRUCTIONS - ), + GuardianAssessmentDecisionSource::Agent => { + let rationale = rejection.rationale.trim(); + match rejection.kind { + GuardianRejectionKind::PolicyDenial => format!( + "This action was rejected due to unacceptable risk.\nReason: {rationale}\n{GUARDIAN_REJECTION_INSTRUCTIONS}" + ), + GuardianRejectionKind::ReviewFailure => format!( + "Automatic approval review failed, so this action was not run.\nReason: {rationale}\n{GUARDIAN_REJECTION_INSTRUCTIONS}" + ), + } + } } } @@ -346,7 +354,7 @@ async fn run_guardian_review( .await; let completed_at_ms = now_unix_timestamp_ms(); - let (assessment, count_denial_for_circuit_breaker) = match outcome { + let (assessment, count_denial_for_circuit_breaker, rejection_kind) = match outcome { GuardianReviewOutcome::Completed(assessment) => { let approved = matches!(assessment.outcome, GuardianAssessmentOutcome::Allow); track_guardian_review( @@ -375,7 +383,11 @@ async fn run_guardian_review( ); let count_denial_for_circuit_breaker = matches!(assessment.outcome, GuardianAssessmentOutcome::Deny); - (assessment, count_denial_for_circuit_breaker) + ( + assessment, + count_denial_for_circuit_breaker, + GuardianRejectionKind::PolicyDenial, + ) } GuardianReviewOutcome::Error(error) => match error { GuardianReviewError::Timeout => { @@ -492,6 +504,7 @@ async fn run_guardian_review( rationale, }, false, + GuardianRejectionKind::ReviewFailure, ) } }, @@ -532,6 +545,7 @@ async fn run_guardian_review( let rejection = GuardianRejection { rationale: assessment.rationale.clone(), source: GuardianAssessmentDecisionSource::Agent, + kind: rejection_kind, }; rationales.insert(review_id.clone(), rejection); } diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index ffadd801ca..577d9216bf 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -2072,10 +2072,59 @@ async fn guardian_review_surfaces_responses_api_errors_in_rejection_reason() -> let rejection_message = guardian_rejection_message(session.as_ref(), "review-shell-guardian-error").await; assert!( - rejection_message.contains("Reason: Automatic approval review failed:") + rejection_message.contains("Automatic approval review failed, so this action was not run.") + && rejection_message.contains("Reason: Automatic approval review failed:") && rejection_message.contains(error_message), "rejection message should include guardian rationale: {rejection_message}" ); + assert!(!rejection_message.contains("rejected due to unacceptable risk")); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn guardian_review_completed_denial_uses_policy_rejection_message() -> anyhow::Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let _request_log = mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-guardian-deny"), + ev_assistant_message( + "msg-guardian-deny", + "{\"risk_level\":\"high\",\"user_authorization\":\"low\",\"outcome\":\"deny\",\"rationale\":\"Pushing without review is too risky.\"}", + ), + ev_completed("resp-guardian-deny"), + ]), + ) + .await; + + let (session, turn) = guardian_test_session_and_turn(&server).await; + seed_guardian_parent_history(&session, &turn).await; + + let decision = review_approval_request( + &session, + &turn, + "review-shell-guardian-deny".to_string(), + GuardianApprovalRequest::Shell { + id: "shell-guardian-deny".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: Some("Need to push the reviewed docs fix.".to_string()), + }, + /*retry_reason*/ None, + ) + .await; + + assert_eq!(decision, ReviewDecision::Denied); + let rejection_message = + guardian_rejection_message(session.as_ref(), "review-shell-guardian-deny").await; + assert!(rejection_message.contains("This action was rejected due to unacceptable risk.")); + assert!(rejection_message.contains("Reason: Pushing without review is too risky.")); + assert!(!rejection_message.contains("Automatic approval review failed")); Ok(()) } diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index bd71c3a054..dfde95d50b 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -1668,6 +1668,7 @@ async fn guardian_review_decision_maps_to_mcp_tool_decision() { crate::guardian::GuardianRejection { rationale: "too risky".to_string(), source: codex_protocol::protocol::GuardianAssessmentDecisionSource::Agent, + kind: crate::guardian::GuardianRejectionKind::PolicyDenial, }, ); let denial = mcp_tool_approval_decision_from_guardian(