Parse Guardian review responses once (#59136)

## What changed

Introduce `GuardianReviewResponse` to retain both the parsed assessment and the original response text. Reuse the stored result for authorization checks, denial reconsideration, review record capture, and final assessment extraction instead of reparsing the response.

Preserve raw text for review records and keep invalid responses as completed turns so history and session reuse remain unchanged.

## Testing

Extend review record coverage to wrapped denials and invalid responses, and verify that invalid assessments remain retryable while missing assessments do not.

GitOrigin-RevId: 50c19e788a8d57f951c6140d66306b3538515adc

codex-rs/core/src/guardian/review_session_setup.rs

@@ -283,8 +283,8 @@ impl ReviewerRequest for PreparedReview {
         // All checkpoint capture and lock acquisition must precede validation. No await
         // may follow a valid allow before publication; new guidance during capture must
         // invalidate the attempt without overwriting the last committed review.
-        if let GuardianReviewSessionOutcome::Completed(Ok(Some(message))) = &result.outcome
-            && codex_guardian_reviewer::parse_guardian_assessment(message).is_ok_and(|assessment| {
+        if let GuardianReviewSessionOutcome::Completed(Ok(Some(response))) = &result.outcome
+            && response.assessment().is_ok_and(|assessment| {
                 assessment.outcome == super::super::GuardianAssessmentOutcome::Allow
             })
             && let Some(authorization) = &self.params.authorization

codex-rs/core/src/guardian/review_session_tests.rs

@@ -3,6 +3,7 @@ use crate::agents_md_manager::AgentsMdManager;
 use crate::context::ContextualUserFragment;
 use crate::context_manager::ContextManager;
 use crate::session::Submission;
+use codex_guardian_reviewer::GuardianReviewResponse;
 use codex_guardian_reviewer::ReviewerRequest;
 use codex_guardian_reviewer::guardian_output_contract_prompt;
 use codex_history::CodexHarnessMetadata;
@@ -735,7 +736,10 @@ async fn run_review_on_reused_session_waits_for_submitted_turn() {
     let GuardianReviewSessionOutcome::Completed(Ok(last_agent_message)) = outcome else {
         panic!("expected submitted turn completion");
     };
-    assert_eq!(last_agent_message.as_deref(), Some("fresh"));
+    assert_eq!(
+        last_agent_message.as_ref().map(GuardianReviewResponse::raw),
+        Some("fresh")
+    );
     assert_eq!(analytics_result.time_to_first_token_ms, Some(42));
     assert_eq!(disposition, SessionDisposition::Reusable);
 }
@@ -836,7 +840,7 @@ async fn wait_for_guardian_review_ignores_prior_turn_errors() {
     let GuardianReviewSessionOutcome::Completed(Ok(last_agent_message)) = outcome else {
         panic!("expected current turn completion");
     };
-    assert_eq!(last_agent_message, None);
+    assert!(last_agent_message.is_none());
     assert_eq!(analytics_result.time_to_first_token_ms, Some(42));
     assert_eq!(disposition, SessionDisposition::Reusable);
     assert!(turn_completed);
@@ -924,7 +928,10 @@ async fn wait_for_guardian_review_ignores_prior_turn_aborts() {
     let GuardianReviewSessionOutcome::Completed(Ok(last_agent_message)) = outcome else {
         panic!("expected current turn completion");
     };
-    assert_eq!(last_agent_message.as_deref(), Some("fresh"));
+    assert_eq!(
+        last_agent_message.as_ref().map(GuardianReviewResponse::raw),
+        Some("fresh")
+    );
     assert_eq!(analytics_result.time_to_first_token_ms, Some(42));
     assert_eq!(disposition, SessionDisposition::Reusable);
     assert!(turn_completed);

codex-rs/ext/guardian-reviewer/src/execution.rs

@@ -16,6 +16,7 @@ use codex_protocol::turn_input::TurnInputSubmission;
 use tokio::time::Instant;
 use tokio_util::sync::CancellationToken;
 
+use crate::GuardianReviewResponse;
 use crate::GuardianReviewSessionOutcome;
 use crate::SessionDisposition;
 
@@ -133,7 +134,9 @@ pub async fn wait_for_guardian_review(
                                 };
                             }
                             return ReviewTurnResult {
-                                outcome: GuardianReviewSessionOutcome::Completed(Ok(turn_complete.last_agent_message)),
+                                outcome: GuardianReviewSessionOutcome::Completed(Ok(
+                                    turn_complete.last_agent_message.map(GuardianReviewResponse::new),
+                                )),
                                 disposition: SessionDisposition::Reusable,
                                 turn_completed: true,
                             };

codex-rs/ext/guardian-reviewer/src/feedback.rs

@@ -2,7 +2,6 @@
 //! The host supplies captured context; records outlive short-lived reviewer sessions.
 
 use crate::GuardianReviewSessionOutcome;
-use crate::parse_guardian_assessment;
 use codex_feedback::GuardianReviewRecord;
 use codex_protocol::ThreadId;
 use codex_protocol::models::ResponseItem;
@@ -62,13 +61,13 @@ impl<'a> FailedReviewFeedback<'a> {
             return None;
         }
         let (status, decision) = match outcome {
-            GuardianReviewSessionOutcome::Completed(Ok(Some(decision))) => {
-                match parse_guardian_assessment(decision) {
+            GuardianReviewSessionOutcome::Completed(Ok(Some(response))) => {
+                match response.assessment() {
                     Ok(assessment) if assessment.outcome == GuardianAssessmentOutcome::Allow => {
                         return None;
                     }
-                    Ok(_) => ("denied", Some(decision.as_str())),
-                    Err(_) => ("invalid_decision", Some(decision.as_str())),
+                    Ok(_) => ("denied", Some(response.raw())),
+                    Err(_) => ("invalid_decision", Some(response.raw())),
                 }
             }
             GuardianReviewSessionOutcome::Completed(Ok(None)) => ("missing_decision", None),

codex-rs/ext/guardian-reviewer/src/feedback_tests.rs

@@ -1,14 +1,21 @@
 use super::*;
+use crate::GuardianReviewResponse;
 use pretty_assertions::assert_eq;
 use serde_json::json;
 
-#[test]
-fn capture_honors_privacy_and_keeps_oversized_context_action_and_decision() -> anyhow::Result<()> {
+#[test_case::test_case(r#"{"outcome":"deny","rationale":"Missing approval."}"#, "denied"; "denial")]
+#[test_case::test_case(r#"preface {"outcome":"deny"}"#, "denied"; "wrapped denial")]
+#[test_case::test_case("not valid guardian json", "invalid_decision"; "invalid response")]
+fn capture_honors_privacy_and_keeps_oversized_context_action_and_decision(
+    decision: &str,
+    status: &str,
+) -> anyhow::Result<()> {
     let thread_id = ThreadId::new();
     let reviewer_thread_id = ThreadId::new();
-    let decision = r#"{"outcome":"deny","rationale":"Missing approval."}"#;
     let action = r#"{"command":"git push"}"#;
-    let outcome = GuardianReviewSessionOutcome::Completed(Ok(Some(decision.to_owned())));
+    let outcome = GuardianReviewSessionOutcome::Completed(Ok(Some(GuardianReviewResponse::new(
+        decision.to_owned(),
+    ))));
     for settings in [
         ReviewFeedbackSettings {
             enabled: false,
@@ -49,7 +56,7 @@ fn capture_honors_privacy_and_keeps_oversized_context_action_and_decision() -> a
             "target_item_id": "push-call",
             "reviewer_thread_id": reviewer_thread_id,
             "model": "review-model",
-            "status": "denied",
+            "status": status,
             "decision": decision,
             "action": action,
             "action_truncated": false,

codex-rs/ext/guardian-reviewer/src/lib.rs

@@ -68,7 +68,7 @@ pub use approval_request_json::guardian_approval_request_to_json;
 pub use assessment::GuardianAssessment;
 pub use assessment::guardian_output_contract_prompt;
 pub use assessment::guardian_output_schema;
-pub use assessment::parse_guardian_assessment;
+pub(crate) use assessment::parse_guardian_assessment;
 pub(crate) use circuit_breaker::AUTO_REVIEW_DENIAL_WINDOW_SIZE;
 pub(crate) use circuit_breaker::GuardianRejectionCircuitBreaker;
 pub(crate) use circuit_breaker::GuardianRejectionCircuitBreakerAction;
@@ -78,6 +78,7 @@ pub use model::ReviewModel;
 pub use model::select_review_model;
 pub use outcome::GuardianReviewError;
 pub use outcome::GuardianReviewOutcome;
+pub use outcome::GuardianReviewResponse;
 pub use outcome::GuardianReviewSessionOutcome;
 pub use planned_action::build_guardian_planned_action;
 pub use retry::GuardianReviewSessionLimits;

codex-rs/ext/guardian-reviewer/src/outcome.rs

@@ -1,4 +1,5 @@
-//! Distinguishes completed assessments from failures without assigning risk to errors.
+//! Retains parsed reviewer responses through feedback capture and distinguishes
+//! completed assessments from failures without assigning risk to errors.
 
 use crate::GuardianAssessment;
 use codex_analytics::GuardianReviewFailureReason;
@@ -79,7 +80,7 @@ impl GuardianReviewError {
 
 #[derive(Debug)]
 pub enum GuardianReviewSessionOutcome {
-    Completed(anyhow::Result<Option<String>>),
+    Completed(anyhow::Result<Option<GuardianReviewResponse>>),
     PromptBuildFailed(anyhow::Error),
     InputBudgetExceeded,
     SessionFailed {
@@ -92,12 +93,43 @@ pub enum GuardianReviewSessionOutcome {
     StaleAuthorization,
 }
 
+/// A completed response parsed once, with its original text retained for feedback.
+/// Invalid JSON remains a completed response so history and session reuse are unchanged.
+pub struct GuardianReviewResponse {
+    raw: String,
+    assessment: anyhow::Result<GuardianAssessment>,
+}
+
+impl GuardianReviewResponse {
+    pub fn new(raw: String) -> Self {
+        let assessment = crate::parse_guardian_assessment(&raw);
+        Self { raw, assessment }
+    }
+
+    pub fn raw(&self) -> &str {
+        &self.raw
+    }
+
+    pub fn assessment(&self) -> Result<&GuardianAssessment, &anyhow::Error> {
+        self.assessment.as_ref()
+    }
+}
+
+impl std::fmt::Debug for GuardianReviewResponse {
+    fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        formatter
+            .debug_struct("GuardianReviewResponse")
+            .field("valid", &self.assessment.is_ok())
+            .finish_non_exhaustive()
+    }
+}
+
 impl GuardianReviewSessionOutcome {
     /// Extract the assessment produced by one reviewer session, preserving failures.
     pub fn into_assessment(self) -> Result<GuardianAssessment, GuardianReviewError> {
         match self {
-            Self::Completed(Ok(Some(message))) => {
-                crate::parse_guardian_assessment(&message).map_err(GuardianReviewError::parse)
+            Self::Completed(Ok(Some(response))) => {
+                response.assessment.map_err(GuardianReviewError::parse)
             }
             Self::Completed(Ok(None)) => Err(GuardianReviewError::session(anyhow::anyhow!(
                 "guardian review completed without an assessment payload"

codex-rs/ext/guardian-reviewer/src/reconsideration.rs

@@ -7,7 +7,6 @@ use crate::ReviewDeadline;
 use crate::ReviewTurnResult;
 use crate::ReviewerRuntime;
 use crate::SessionDisposition;
-use crate::parse_guardian_assessment;
 use crate::start_review_turn;
 use crate::wait_for_guardian_review;
 use codex_analytics::GuardianReviewAnalyticsResult;
@@ -28,10 +27,11 @@ pub async fn reconsider_denial(
     analytics: &mut GuardianReviewAnalyticsResult,
     telemetry: &SessionTelemetry,
 ) -> ReviewTurnResult {
-    let GuardianReviewSessionOutcome::Completed(Ok(Some(message))) = &first.outcome else {
+    let GuardianReviewSessionOutcome::Completed(Ok(Some(response))) = &first.outcome else {
         return first;
     };
-    if !parse_guardian_assessment(message)
+    if !response
+        .assessment()
         .is_ok_and(|assessment| assessment.outcome == GuardianAssessmentOutcome::Deny)
     {
         return first;
@@ -53,9 +53,7 @@ pub async fn reconsider_denial(
     // aggregate token usage from the same session after both passes finish.
     analytics.time_to_first_token_ms = first_token_ms;
     let assessment = match &second.outcome {
-        GuardianReviewSessionOutcome::Completed(Ok(Some(message))) => {
-            parse_guardian_assessment(message).ok()
-        }
+        GuardianReviewSessionOutcome::Completed(Ok(Some(response))) => response.assessment().ok(),
         GuardianReviewSessionOutcome::Completed(Ok(None) | Err(_))
         | GuardianReviewSessionOutcome::PromptBuildFailed(_)
         | GuardianReviewSessionOutcome::InputBudgetExceeded
@@ -64,7 +62,7 @@ pub async fn reconsider_denial(
         | GuardianReviewSessionOutcome::TimedOut
         | GuardianReviewSessionOutcome::Aborted => None,
     };
-    let outcome = match assessment.as_ref().map(|assessment| assessment.outcome) {
+    let outcome = match assessment.map(|assessment| assessment.outcome) {
         Some(GuardianAssessmentOutcome::Allow) => "allow",
         Some(GuardianAssessmentOutcome::Deny) => "deny",
         None => match &second.outcome {

codex-rs/ext/guardian-reviewer/src/retry_tests.rs

@@ -1,6 +1,8 @@
 #[cfg(test)]
 use super::*;
 use crate::GuardianAssessment;
+use crate::GuardianReviewResponse;
+use crate::GuardianReviewSessionOutcome;
 use codex_analytics::GuardianReviewFailureReason;
 use codex_protocol::protocol::GuardianAssessmentOutcome;
 use codex_protocol::protocol::GuardianRiskLevel;
@@ -113,9 +115,16 @@ fn guardian_review_retry_only_retries_recoverable_errors() {
             false,
         ),
         (
-            GuardianReviewOutcome::Error(GuardianReviewError::parse(anyhow::anyhow!("parse"))),
+            GuardianReviewSessionOutcome::Completed(Ok(Some(GuardianReviewResponse::new(
+                "invalid assessment".to_owned(),
+            ))))
+            .into(),
             true,
         ),
+        (
+            GuardianReviewSessionOutcome::Completed(Ok(None)).into(),
+            false,
+        ),
         (
             GuardianReviewOutcome::Error(GuardianReviewError::Timeout),
             false,