Track approval cancellation sources and Guardian context changes (#54046)

## What changed

- Preserve approval resolution sources through tool errors and tag cancelled MCP approvals with `error_type=approval`, `error_code=cancelled`, and `approval_source` (`guardian`, `user`, `hook`, or `unknown`). The source identifies the reviewer, not who caused cancellation.
- Record `context_changed` when validating a Guardian allow against changed authorization or review context, even when a concurrent stop or history reset determines the outcome. Retain the flag across retries and fresh-checkpoint recovery, and include it in the Guardian review counter.

GitOrigin-RevId: a19a986a7565f9a78a9bb2eaabdb5651834d28b8

codex-rs/analytics/src/events.rs

@@ -538,6 +538,9 @@ pub struct GuardianReviewAnalyticsResult {
     pub terminal_status: GuardianReviewTerminalStatus,
     pub failure_reason: Option<GuardianReviewFailureReason>,
     pub attempt_count: i64,
+    /// An allow was validated against changed parent/root authorization or review context.
+    /// Retained across attempts; this does not identify the cause of a later cancellation.
+    pub context_changed: bool,
     pub risk_level: Option<GuardianRiskLevel>,
     pub user_authorization: Option<GuardianUserAuthorization>,
     pub outcome: Option<GuardianAssessmentOutcome>,
@@ -564,6 +567,7 @@ impl GuardianReviewAnalyticsResult {
             terminal_status: GuardianReviewTerminalStatus::FailedClosed,
             failure_reason: None,
             attempt_count: 1,
+            context_changed: false,
             risk_level: None,
             user_authorization: None,
             outcome: None,

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

@@ -34,12 +34,19 @@ pub(in crate::guardian) struct ReviewAuthorization {
     history_reset: CancellationToken,
 }
 
+/// Keeps observed context changes separate from the invalidation outcome.
+/// A concurrent stop or history reset can take precedence over changed context.
+pub(in crate::guardian) struct ReviewInvalidation {
+    pub(in crate::guardian) outcome: codex_guardian_reviewer::GuardianReviewSessionOutcome,
+    pub(in crate::guardian) context_changed: bool,
+}
+
 impl ReviewAuthorization {
     pub(in crate::guardian) async fn invalidation(
         &self,
         session: &Session,
         cancellation: Option<&CancellationToken>,
-    ) -> Option<codex_guardian_reviewer::GuardianReviewSessionOutcome> {
+    ) -> Option<ReviewInvalidation> {
         use codex_guardian_reviewer::GuardianReviewSessionOutcome;
 
         let root_snapshot = session
@@ -63,7 +70,7 @@ impl ReviewAuthorization {
         let review_context_changed = root_review_changed
             || self.review_context_revision != history.guardian_review_context_revision();
         // An actual stop or history reset wins over a concurrent evidence change.
-        if self.history_reset.is_cancelled()
+        let outcome = if self.history_reset.is_cancelled()
             || cancellation.is_some_and(CancellationToken::is_cancelled)
             || root_history_changed
         {
@@ -78,7 +85,11 @@ impl ReviewAuthorization {
             Some(GuardianReviewSessionOutcome::StaleAuthorization)
         } else {
             None
-        }
+        };
+        outcome.map(|outcome| ReviewInvalidation {
+            outcome,
+            context_changed: authorization_changed || review_context_changed,
+        })
     }
 }
 

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

@@ -314,13 +314,15 @@ impl ReviewerRequest for PreparedReview {
                 assessment.outcome == super::super::GuardianAssessmentOutcome::Allow
             })
             && let Some(authorization) = &self.params.authorization
-            && let Some(outcome) = authorization
+            && let Some(invalidation) = authorization
                 .invalidation(
                     &self.params.parent_session,
                     self.params.external_cancel.as_ref(),
                 )
                 .await
         {
+            let outcome = invalidation.outcome;
+            result.analytics.context_changed |= invalidation.context_changed;
             result.disposition =
                 if matches!(outcome, GuardianReviewSessionOutcome::StaleAuthorization)
                     && result.disposition == SessionDisposition::Reusable
@@ -378,7 +380,9 @@ pub(crate) async fn run_guardian_review_session(
                 Arc::make_mut(&mut prepared.context).reviewer_selection =
                     ReviewerSelection::FreshParentCheckpoint;
                 prepared.recovery_requested = Arc::default();
-                pool.review(prepared).await
+                let (outcome, mut analytics) = pool.review(prepared).await;
+                analytics.context_changed |= result.1.context_changed;
+                (outcome, analytics)
             } else {
                 result
             }

codex-rs/core/src/mcp_tool_call.rs

@@ -16,6 +16,7 @@ use crate::session::session::Session;
 use crate::session::step_context::StepContext;
 use crate::session::turn_context::TurnContext;
 use crate::tools::ApprovalContext;
+use crate::tools::ApprovalResolutionSource;
 use crate::tools::context::ToolCallOrigin;
 use crate::tools::hook_names::HookToolName;
 use crate::tools::lifecycle::process_mcp_tool_result;
@@ -275,7 +276,7 @@ pub(crate) async fn handle_mcp_tool_call(
     } else {
         McpToolApprovalPolicy::for_server(approval_mode)
     };
-    if let Some(decision) = maybe_request_mcp_tool_approval(
+    if let Some(approval) = maybe_request_mcp_tool_approval(
         &sess,
         step_context,
         cancellation_token,
@@ -290,7 +291,9 @@ pub(crate) async fn handle_mcp_tool_call(
     )
     .await
     {
-        let result = match decision {
+        let cancellation_outcome = matches!(approval.decision, ReviewDecision::Abort)
+            .then(|| McpCallMetricOutcome::approval_cancelled(approval.source));
+        let result = match approval.decision {
             decision @ (ReviewDecision::Approved
             | ReviewDecision::ApprovedForSession
             | ReviewDecision::ApprovedMcpPolicyAmendment
@@ -356,7 +359,8 @@ pub(crate) async fn handle_mcp_tool_call(
         };
 
         let status = if result.is_ok() { "ok" } else { "error" };
-        let outcome = McpCallMetricOutcome::from_status(status);
+        let outcome =
+            cancellation_outcome.unwrap_or_else(|| McpCallMetricOutcome::from_status(status));
         emit_mcp_call_metrics(
             &step_context.session_telemetry,
             &outcome,
@@ -1467,6 +1471,12 @@ fn mcp_tool_approval_prompt_options(
     }
 }
 
+#[derive(Debug, PartialEq, Eq)]
+struct McpToolApproval {
+    decision: ReviewDecision,
+    source: Option<ApprovalResolutionSource>,
+}
+
 #[expect(clippy::too_many_arguments)]
 async fn maybe_request_mcp_tool_approval(
     sess: &Arc<Session>,
@@ -1480,7 +1490,7 @@ async fn maybe_request_mcp_tool_approval(
     config: &codex_mcp::McpConfig,
     permission_profile: &PermissionProfile,
     policy: McpToolApprovalPolicy,
-) -> Option<ReviewDecision> {
+) -> Option<McpToolApproval> {
     let turn_context = &step_context.turn;
     let strict_auto_review = turn_context.strict_auto_review_enabled();
     let approvals_reviewer = connectors::mcp_approvals_reviewer_from_layers(
@@ -1522,7 +1532,10 @@ async fn maybe_request_mcp_tool_approval(
         && let Some(key) = session_approval_key.as_ref()
         && mcp_tool_approval_is_remembered(sess, key).await
     {
-        return Some(ReviewDecision::Approved);
+        return Some(McpToolApproval {
+            decision: ReviewDecision::Approved,
+            source: None,
+        });
     }
 
     let action = ApprovalAction::McpToolCall {
@@ -1563,13 +1576,15 @@ async fn maybe_request_mcp_tool_approval(
         retry_reason: None,
         network_approval_context: None,
     };
-    Some(
-        match sess.request_approval(action, approval_context).await {
+    let approval = sess.request_approval(action, approval_context).await;
+    Some(McpToolApproval {
+        decision: match approval.result {
             Ok(decision) => decision,
             Err(ToolError::Rejected(rejection)) => ReviewDecision::denied(rejection),
             Err(ToolError::Codex(_)) => ReviewDecision::Abort,
         },
-    )
+        source: approval.source,
+    })
 }
 
 pub(crate) async fn request_mcp_tool_user_approval(

codex-rs/core/src/mcp_tool_call/telemetry.rs

@@ -7,6 +7,8 @@ use codex_protocol::mcp::CallToolResult;
 use serde_json::Value as JsonValue;
 use tracing::Span;
 
+use crate::tools::ApprovalResolutionSource;
+
 const MCP_CALL_COUNT_METRIC: &str = "codex.mcp.call";
 const MCP_CALL_DURATION_METRIC: &str = "codex.mcp.call.duration_ms";
 const MCP_CALL_ERROR_COUNT_METRIC: &str = "codex.mcp.call.error";
@@ -25,6 +27,7 @@ pub(super) struct McpCallMetricOutcome {
     status: &'static str,
     error_type: Option<&'static str>,
     error_code: Option<String>,
+    approval_source: Option<&'static str>,
 }
 
 impl McpCallMetricOutcome {
@@ -33,6 +36,21 @@ impl McpCallMetricOutcome {
             status,
             error_type: None,
             error_code: None,
+            approval_source: None,
+        }
+    }
+
+    pub(super) fn approval_cancelled(source: Option<ApprovalResolutionSource>) -> Self {
+        Self {
+            status: "error",
+            error_type: Some("approval"),
+            error_code: Some("cancelled".to_string()),
+            approval_source: Some(match source {
+                Some(ApprovalResolutionSource::Guardian) => "guardian",
+                Some(ApprovalResolutionSource::User) => "user",
+                Some(ApprovalResolutionSource::Hook) => "hook",
+                None => "unknown",
+            }),
         }
     }
 }
@@ -69,6 +87,9 @@ pub(super) fn emit_mcp_call_metrics(
     let mut error_tags = tags;
     error_tags.push(("error_type", sanitize_metric_tag_value(error_type)));
     error_tags.push(("error_code", error_code.to_string()));
+    if let Some(approval_source) = outcome.approval_source {
+        error_tags.push(("approval_source", approval_source.to_string()));
+    }
     let error_tag_refs: Vec<(&str, &str)> = error_tags
         .iter()
         .map(|(key, value)| (*key, value.as_str()))
@@ -138,6 +159,7 @@ pub(super) fn mcp_call_metric_outcome(
                 status: "error",
                 error_type: Some(MCP_CALL_ERROR_TYPE_TOOL_RESULT),
                 error_code: Some(sanitize_metric_tag_value(&error_code)),
+                approval_source: None,
             }
         }
         Ok(_) => McpCallMetricOutcome::from_status("ok"),
@@ -145,6 +167,7 @@ pub(super) fn mcp_call_metric_outcome(
             status: "error",
             error_type: Some(MCP_CALL_ERROR_TYPE_MCP_REQUEST),
             error_code: Some(MCP_CALL_ERROR_CODE_UNKNOWN.to_string()),
+            approval_source: None,
         },
     }
 }

codex-rs/core/src/mcp_tool_call/telemetry_tests.rs

@@ -43,6 +43,7 @@ fn mcp_call_metric_outcome_distinguishes_request_and_tool_errors() {
             status: "ok",
             error_type: None,
             error_code: None,
+            approval_source: None,
         }
     );
     assert_eq!(
@@ -54,6 +55,7 @@ fn mcp_call_metric_outcome_distinguishes_request_and_tool_errors() {
             status: "error",
             error_type: Some(MCP_CALL_ERROR_TYPE_TOOL_RESULT),
             error_code: Some("RATE_LIMITED".to_string()),
+            approval_source: None,
         }
     );
     assert_eq!(
@@ -62,6 +64,7 @@ fn mcp_call_metric_outcome_distinguishes_request_and_tool_errors() {
             status: "error",
             error_type: Some(MCP_CALL_ERROR_TYPE_MCP_REQUEST),
             error_code: Some(MCP_CALL_ERROR_CODE_UNKNOWN.to_string()),
+            approval_source: None,
         }
     );
 }
@@ -79,6 +82,7 @@ fn mcp_call_metric_outcome_reports_server_tool_error_codes() {
             status: "error",
             error_type: Some(MCP_CALL_ERROR_TYPE_TOOL_RESULT),
             error_code: Some("arbitrary-user-value".to_string()),
+            approval_source: None,
         }
     );
 }
@@ -105,6 +109,7 @@ fn mcp_call_metric_outcome_reads_auth_error_code_from_meta() {
             status: "error",
             error_type: Some(MCP_CALL_ERROR_TYPE_TOOL_RESULT),
             error_code: Some("UNAUTHORIZED".to_string()),
+            approval_source: None,
         }
     );
 }
@@ -123,6 +128,7 @@ fn mcp_call_metric_outcome_bounds_and_sanitizes_error_code() {
             status: "error",
             error_type: Some(MCP_CALL_ERROR_TYPE_TOOL_RESULT),
             error_code: Some(format!("BAD_CODE_{}", "x".repeat(247))),
+            approval_source: None,
         }
     );
 }

codex-rs/core/src/mcp_tool_call_tests.rs

@@ -2944,7 +2944,13 @@ async fn permission_request_hook_allows_mcp_tool_call() {
     )
     .await;
 
-    assert_eq!(decision, Some(ReviewDecision::Approved));
+    assert_eq!(
+        decision,
+        Some(McpToolApproval {
+            decision: ReviewDecision::Approved,
+            source: Some(ApprovalResolutionSource::Hook),
+        })
+    );
     let log = std::fs::read_to_string(log_path).expect("read MCP permission hook log");
     let inputs = log
         .lines()
@@ -3015,7 +3021,13 @@ async fn permission_request_hook_uses_hook_tool_name_without_metadata() {
     )
     .await;
 
-    assert_eq!(decision, Some(ReviewDecision::Approved));
+    assert_eq!(
+        decision,
+        Some(McpToolApproval {
+            decision: ReviewDecision::Approved,
+            source: Some(ApprovalResolutionSource::Hook),
+        })
+    );
     let log = std::fs::read_to_string(log_path).expect("read MCP permission hook log");
     let inputs = log
         .lines()
@@ -3102,7 +3114,13 @@ async fn permission_request_hook_runs_after_remembered_mcp_approval() {
     )
     .await;
 
-    assert_eq!(decision, Some(ReviewDecision::Approved));
+    assert_eq!(
+        decision,
+        Some(McpToolApproval {
+            decision: ReviewDecision::Approved,
+            source: None,
+        })
+    );
     assert!(
         !log_path.exists(),
         "remembered approval should skip PermissionRequest hooks"
@@ -3202,7 +3220,11 @@ async fn strict_auto_review_forces_guardian_for_mcp_policy_skip() {
     )
     .await;
 
-    let Some(ReviewDecision::Denied { rejection: message }) = decision else {
+    let Some(McpToolApproval {
+        decision: ReviewDecision::Denied { rejection: message },
+        source: Some(ApprovalResolutionSource::Guardian),
+    }) = decision
+    else {
         panic!("guardian-denied MCP approval should carry a rejection message");
     };
     assert!(message.contains("Reason: The tool call would expose private calendar data"));
@@ -3313,7 +3335,10 @@ async fn assert_mcp_user_approval_persistence(
             .await
             .expect("MCP approval task timed out")
             .expect("MCP approval task failed"),
-        Some(expected_decision),
+        Some(McpToolApproval {
+            decision: expected_decision,
+            source: Some(ApprovalResolutionSource::User),
+        }),
     );
 }
 

codex-rs/core/src/tools/approvals.rs

@@ -388,12 +388,20 @@ impl ApprovalAction {
 }
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
-enum ApprovalResolutionSource {
+pub(crate) enum ApprovalResolutionSource {
     Hook,
     Guardian,
     User,
 }
 
+/// Preserves the approval path even when the decision becomes a tool error.
+/// The source identifies the reviewer, not the actor that caused cancellation.
+#[must_use]
+pub(crate) struct ApprovalResult {
+    pub(crate) result: Result<ReviewDecision, ToolError>,
+    pub(crate) source: Option<ApprovalResolutionSource>,
+}
+
 #[derive(Clone, Debug, PartialEq, Eq)]
 struct ApprovalResolution {
     decision: ReviewDecision,
@@ -440,7 +448,7 @@ impl Session {
         self: &Arc<Self>,
         action: ApprovalAction,
         ctx: ApprovalContext,
-    ) -> Result<ReviewDecision, ToolError> {
+    ) -> ApprovalResult {
         // Stdin that exceeds current permissions needs a fresh sandbox approval.
         // Strict review of ordinary input follows the same routing as ordinary exec.
         let policy = ctx.review_context.turn().approval_policy();
@@ -450,7 +458,10 @@ impl Session {
             && let Some(reason) =
                 prompt_is_rejected_by_policy(policy, /*prompt_is_rule*/ false)
         {
-            return Err(ToolError::Rejected(reason.to_string()));
+            return ApprovalResult {
+                result: Err(ToolError::Rejected(reason.to_string())),
+                source: None,
+            };
         }
         let is_mcp_tool_call = matches!(&action, ApprovalAction::McpToolCall { .. });
         let is_network_approval = matches!(&action, ApprovalAction::NetworkAccess { .. });
@@ -485,8 +496,12 @@ impl Session {
         if !is_network_approval {
             record_resolution(&ctx, &resolution);
         }
+        let source = Some(resolution.source);
         if is_mcp_tool_call && resolution.decision == ReviewDecision::ApprovedMcpPolicyAmendment {
-            return Ok(resolution.decision);
+            return ApprovalResult {
+                result: Ok(resolution.decision),
+                source,
+            };
         }
         if is_network_approval {
             match (&resolution.decision, resolution.source) {
@@ -496,17 +511,26 @@ impl Session {
                     },
                     _,
                 ) if network_policy_amendment.action == NetworkPolicyRuleAction::Deny => {
-                    return Ok(resolution.decision);
+                    return ApprovalResult {
+                        result: Ok(resolution.decision),
+                        source,
+                    };
                 }
                 (ReviewDecision::Abort, ApprovalResolutionSource::Guardian) => {
-                    return Err(ToolError::Rejected(
-                        "automatic approval review was cancelled".to_string(),
-                    ));
+                    return ApprovalResult {
+                        result: Err(ToolError::Rejected(
+                            "automatic approval review was cancelled".to_string(),
+                        )),
+                        source,
+                    };
                 }
                 _ => {}
             }
         }
-        resolution.into_tool_result(ctx.review_context.turn().model_info())
+        ApprovalResult {
+            result: resolution.into_tool_result(ctx.review_context.turn().model_info()),
+            source,
+        }
     }
 
     async fn request_reviewer_approval(

codex-rs/core/src/tools/mod.rs

@@ -29,6 +29,7 @@ use std::borrow::Cow;
 
 use crate::session::turn_context::TurnContext;
 pub(crate) use approvals::ApprovalContext;
+pub(crate) use approvals::ApprovalResolutionSource;
 use codex_features::Feature;
 use codex_protocol::exec_output::ExecToolCallOutput;
 use codex_protocol::openai_models::ModelInfo;

codex-rs/core/src/tools/network_approval.rs

@@ -819,7 +819,7 @@ impl NetworkApprovalService {
         } else {
             approval.await
         };
-        let approval_decision = match approval_result {
+        let approval_decision = match approval_result.result {
             Ok(decision) => decision,
             Err(ToolError::Rejected(rejection)) => {
                 if let Some(owner_call) = owner_call.as_ref() {

codex-rs/core/src/tools/orchestrator.rs

@@ -187,7 +187,8 @@ impl ToolOrchestrator {
                     tool_ctx
                         .session
                         .request_approval(action, approval_ctx)
-                        .await?;
+                        .await
+                        .result?;
                     already_approved = true;
                 } else {
                     otel.tool_decision(
@@ -220,7 +221,8 @@ impl ToolOrchestrator {
                 tool_ctx
                     .session
                     .request_approval(action, approval_ctx)
-                    .await?;
+                    .await
+                    .result?;
                 already_approved = true;
             }
         }
@@ -511,7 +513,8 @@ impl ToolOrchestrator {
                     tool_ctx
                         .session
                         .request_approval(action, approval_ctx)
-                        .await?;
+                        .await
+                        .result?;
                 }
 
                 let retry_sandbox_requested = !unsandboxed_allowed

codex-rs/core/src/unified_exec/process_manager.rs

@@ -977,6 +977,7 @@ impl UnifiedExecProcessManager {
                 .session
                 .request_approval(approval, approval_context)
                 .await
+                .result
                 .map_err(UnifiedExecError::StdinApproval)?;
         }
 

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

@@ -46,6 +46,7 @@ pub(crate) fn emit_guardian_review_metrics(
     };
     let mut counter_tag_refs = tag_refs.clone();
     counter_tag_refs.extend(mcp_tags.iter().map(|(key, value)| (*key, value.as_str())));
+    counter_tag_refs.push(("context_changed", bool_tag(result.context_changed)));
     session_telemetry.counter(
         GUARDIAN_REVIEW_COUNT_METRIC,
         /*inc*/ 1,
@@ -463,6 +464,7 @@ mod tests {
         ]);
         let mut expected_counter_tags = expected_base_tags.clone();
         expected_counter_tags.extend(expected_mcp_tags);
+        expected_counter_tags.insert("context_changed".to_string(), "false".to_string());
         assert_eq!((attrs, value), (expected_counter_tags, 1));
 
         assert_eq!(

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

@@ -45,9 +45,12 @@ where
     } = limits;
     assert!(max_attempts > 0, "guardian review must run at least once");
     let mut attempt_count = 1;
+    let mut context_changed = false;
     loop {
         let (outcome, mut analytics_result, evidence) = run_attempt(deadline).await;
         analytics_result.attempt_count = attempt_count;
+        context_changed |= analytics_result.context_changed;
+        analytics_result.context_changed = context_changed;
         if attempt_count >= max_attempts || !should_retry_guardian_review(&outcome) {
             return (outcome, analytics_result, evidence);
         }