feat(guardian): emit guardian review analytics

This commit is contained in:
Owen Lin
2026-04-07 16:15:41 -07:00
parent 409a3b3b32
commit f80ad0f5da
7 changed files with 559 additions and 37 deletions
+2 -2
View File
@@ -39,7 +39,7 @@ use crate::codex::TurnContext;
use crate::codex::emit_subagent_session_started;
use crate::config::Config;
use crate::guardian::GuardianApprovalRequest;
use crate::guardian::review_approval_request_with_cancel;
use crate::guardian::review_delegated_approval_request_with_cancel;
use crate::guardian::routes_approval_to_guardian;
use crate::mcp_tool_call::MCP_TOOL_APPROVAL_ACCEPT;
use crate::mcp_tool_call::MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION;
@@ -739,7 +739,7 @@ fn spawn_guardian_review(
let _ = tx.send(ReviewDecision::Denied);
return;
};
let decision = runtime.block_on(review_approval_request_with_cancel(
let decision = runtime.block_on(review_delegated_approval_request_with_cancel(
&session,
&turn,
request,
+3
View File
@@ -14,6 +14,7 @@
mod approval_request;
mod prompt;
mod review;
mod review_analytics;
mod review_session;
mod review_session_analytics;
@@ -28,7 +29,9 @@ pub(crate) use approval_request::guardian_approval_request_to_json;
pub(crate) use review::GUARDIAN_REJECTION_MESSAGE;
pub(crate) use review::is_guardian_reviewer_source;
pub(crate) use review::review_approval_request;
#[cfg(test)]
pub(crate) use review::review_approval_request_with_cancel;
pub(crate) use review::review_delegated_approval_request_with_cancel;
pub(crate) use review::routes_approval_to_guardian;
pub(crate) use review_session::GuardianReviewSessionManager;
+205 -30
View File
@@ -1,5 +1,9 @@
use std::sync::Arc;
use std::time::Instant;
use codex_analytics::GuardianReviewDecision;
use codex_analytics::GuardianReviewFailureKind;
use codex_analytics::GuardianReviewTerminalStatus;
use codex_protocol::config_types::ApprovalsReviewer;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
@@ -24,9 +28,15 @@ use super::approval_request::guardian_request_turn_id;
use super::prompt::build_guardian_prompt_items;
use super::prompt::guardian_output_schema;
use super::prompt::parse_guardian_assessment;
use super::review_analytics::GuardianReviewAnalyticsInput;
use super::review_analytics::duration_millis_u64;
use super::review_analytics::guardian_reviewed_action;
use super::review_analytics::now_unix_timestamp_secs;
use super::review_analytics::track_guardian_review;
use super::review_session::GuardianReviewSessionOutcome;
use super::review_session::GuardianReviewSessionParams;
use super::review_session::build_guardian_review_session_config;
use super::review_session_analytics::GuardianReviewSessionReport;
pub(crate) const GUARDIAN_REJECTION_MESSAGE: &str = concat!(
"This action was rejected due to unacceptable risk. ",
@@ -39,9 +49,17 @@ pub(crate) const GUARDIAN_REJECTION_MESSAGE: &str = concat!(
#[derive(Debug)]
pub(super) enum GuardianReviewOutcome {
Completed(anyhow::Result<GuardianAssessment>),
TimedOut,
Aborted,
Completed {
result: anyhow::Result<GuardianAssessment>,
report: Option<GuardianReviewSessionReport>,
failure_kind: Option<GuardianReviewFailureKind>,
},
TimedOut {
report: Option<GuardianReviewSessionReport>,
},
Aborted {
report: Option<GuardianReviewSessionReport>,
},
}
fn guardian_risk_level_str(level: GuardianRiskLevel) -> &'static str {
@@ -78,10 +96,15 @@ async fn run_guardian_review(
request: GuardianApprovalRequest,
retry_reason: Option<String>,
external_cancel: Option<CancellationToken>,
delegated_review: bool,
) -> ReviewDecision {
let review_started_at = Instant::now();
let started_at = now_unix_timestamp_secs();
let assessment_id = guardian_request_id(&request).to_string();
let assessment_turn_id = guardian_request_turn_id(&request, &turn.sub_id).to_string();
let action_summary = guardian_assessment_action(&request);
let (trigger, reviewed_action, reviewed_action_truncated) = guardian_reviewed_action(&request);
let retry_reason_for_analytics = retry_reason.clone();
session
.send_event(
turn.as_ref(),
@@ -101,6 +124,28 @@ async fn run_guardian_review(
.as_ref()
.is_some_and(CancellationToken::is_cancelled)
{
track_guardian_review(
session.as_ref(),
GuardianReviewAnalyticsInput {
review_id: assessment_id.clone(),
target_item_id: assessment_id.clone(),
turn_id: assessment_turn_id.clone(),
trigger,
retry_reason: retry_reason_for_analytics,
delegated_review,
reviewed_action,
reviewed_action_truncated,
decision: GuardianReviewDecision::Aborted,
terminal_status: GuardianReviewTerminalStatus::Aborted,
failure_kind: Some(GuardianReviewFailureKind::Cancelled),
assessment: None,
report: None,
started_at,
completed_at: Some(now_unix_timestamp_secs()),
completion_latency_ms: Some(duration_millis_u64(review_started_at.elapsed())),
},
)
.await;
session
.send_event(
turn.as_ref(),
@@ -131,26 +176,68 @@ async fn run_guardian_review(
)
.await
}
Err(err) => GuardianReviewOutcome::Completed(Err(err.into())),
Err(err) => GuardianReviewOutcome::Completed {
result: Err(err.into()),
report: None,
failure_kind: Some(GuardianReviewFailureKind::PromptBuildError),
},
};
let assessment = match outcome {
GuardianReviewOutcome::Completed(Ok(assessment)) => assessment,
GuardianReviewOutcome::Completed(Err(err)) => GuardianAssessment {
risk_level: GuardianRiskLevel::High,
risk_score: 100,
rationale: format!("Automatic approval review failed: {err}"),
evidence: vec![],
},
GuardianReviewOutcome::TimedOut => GuardianAssessment {
risk_level: GuardianRiskLevel::High,
risk_score: 100,
rationale:
"Automatic approval review timed out while evaluating the requested approval."
.to_string(),
evidence: vec![],
},
GuardianReviewOutcome::Aborted => {
let (assessment, report, failure_kind) = match outcome {
GuardianReviewOutcome::Completed {
result: Ok(assessment),
report,
failure_kind,
} => (assessment, report, failure_kind),
GuardianReviewOutcome::Completed {
result: Err(err),
report,
failure_kind,
} => (
GuardianAssessment {
risk_level: GuardianRiskLevel::High,
risk_score: 100,
rationale: format!("Automatic approval review failed: {err}"),
evidence: vec![],
},
report,
failure_kind.or(Some(GuardianReviewFailureKind::SessionError)),
),
GuardianReviewOutcome::TimedOut { report } => (
GuardianAssessment {
risk_level: GuardianRiskLevel::High,
risk_score: 100,
rationale:
"Automatic approval review timed out while evaluating the requested approval."
.to_string(),
evidence: vec![],
},
report,
Some(GuardianReviewFailureKind::Timeout),
),
GuardianReviewOutcome::Aborted { report } => {
track_guardian_review(
session.as_ref(),
GuardianReviewAnalyticsInput {
review_id: assessment_id.clone(),
target_item_id: assessment_id.clone(),
turn_id: assessment_turn_id.clone(),
trigger,
retry_reason: retry_reason_for_analytics,
delegated_review,
reviewed_action,
reviewed_action_truncated,
decision: GuardianReviewDecision::Aborted,
terminal_status: GuardianReviewTerminalStatus::Aborted,
failure_kind: Some(GuardianReviewFailureKind::Cancelled),
assessment: None,
report,
started_at,
completed_at: Some(now_unix_timestamp_secs()),
completion_latency_ms: Some(duration_millis_u64(review_started_at.elapsed())),
},
)
.await;
session
.send_event(
turn.as_ref(),
@@ -170,6 +257,42 @@ async fn run_guardian_review(
};
let approved = assessment.risk_score < GUARDIAN_APPROVAL_RISK_THRESHOLD;
let decision = if approved {
GuardianReviewDecision::Approved
} else {
GuardianReviewDecision::Denied
};
let terminal_status = if approved {
GuardianReviewTerminalStatus::Approved
} else if matches!(failure_kind, Some(GuardianReviewFailureKind::Timeout)) {
GuardianReviewTerminalStatus::TimedOut
} else if failure_kind.is_some() {
GuardianReviewTerminalStatus::FailedClosed
} else {
GuardianReviewTerminalStatus::Denied
};
track_guardian_review(
session.as_ref(),
GuardianReviewAnalyticsInput {
review_id: assessment_id.clone(),
target_item_id: assessment_id.clone(),
turn_id: assessment_turn_id.clone(),
trigger,
retry_reason: retry_reason_for_analytics,
delegated_review,
reviewed_action,
reviewed_action_truncated,
decision,
terminal_status,
failure_kind,
assessment: Some(&assessment),
report,
started_at,
completed_at: Some(now_unix_timestamp_secs()),
completion_latency_ms: Some(duration_millis_u64(review_started_at.elapsed())),
},
)
.await;
let verdict = if approved { "approved" } else { "denied" };
let warning = format!(
"Automatic approval review {verdict} (risk: {}): {}",
@@ -222,10 +345,12 @@ pub(crate) async fn review_approval_request(
request,
retry_reason,
/*external_cancel*/ None,
/*delegated_review*/ false,
)
.await
}
#[cfg(test)]
pub(crate) async fn review_approval_request_with_cancel(
session: &Arc<Session>,
turn: &Arc<TurnContext>,
@@ -239,6 +364,25 @@ pub(crate) async fn review_approval_request_with_cancel(
request,
retry_reason,
Some(cancel_token),
/*delegated_review*/ false,
)
.await
}
pub(crate) async fn review_delegated_approval_request_with_cancel(
session: &Arc<Session>,
turn: &Arc<TurnContext>,
request: GuardianApprovalRequest,
retry_reason: Option<String>,
cancel_token: CancellationToken,
) -> ReviewDecision {
run_guardian_review(
Arc::clone(session),
Arc::clone(turn),
request,
retry_reason,
Some(cancel_token),
/*delegated_review*/ true,
)
.await
}
@@ -267,7 +411,13 @@ pub(super) async fn run_guardian_review_session(
let live_network_config = match session.services.network_proxy.as_ref() {
Some(network_proxy) => match network_proxy.proxy().current_cfg().await {
Ok(config) => Some(config),
Err(err) => return GuardianReviewOutcome::Completed(Err(err)),
Err(err) => {
return GuardianReviewOutcome::Completed {
result: Err(err),
report: None,
failure_kind: Some(GuardianReviewFailureKind::SessionError),
};
}
},
None => None,
};
@@ -317,7 +467,13 @@ pub(super) async fn run_guardian_review_session(
);
let guardian_config = match guardian_config {
Ok(config) => config,
Err(err) => return GuardianReviewOutcome::Completed(Err(err)),
Err(err) => {
return GuardianReviewOutcome::Completed {
result: Err(err),
report: None,
failure_kind: Some(GuardianReviewFailureKind::SessionError),
};
}
};
match session
@@ -336,13 +492,32 @@ pub(super) async fn run_guardian_review_session(
})
.await
{
GuardianReviewSessionOutcome::Completed { result, report: _ } => match result {
Ok(last_agent_message) => GuardianReviewOutcome::Completed(parse_guardian_assessment(
last_agent_message.as_deref(),
)),
Err(err) => GuardianReviewOutcome::Completed(Err(err)),
GuardianReviewSessionOutcome::Completed { result, report } => match result {
Ok(last_agent_message) => {
match parse_guardian_assessment(last_agent_message.as_deref()) {
Ok(assessment) => GuardianReviewOutcome::Completed {
result: Ok(assessment),
report,
failure_kind: None,
},
Err(err) => GuardianReviewOutcome::Completed {
result: Err(err),
report,
failure_kind: Some(GuardianReviewFailureKind::ParseError),
},
}
}
Err(err) => GuardianReviewOutcome::Completed {
result: Err(err),
report,
failure_kind: Some(GuardianReviewFailureKind::SessionError),
},
},
GuardianReviewSessionOutcome::TimedOut { report: _ } => GuardianReviewOutcome::TimedOut,
GuardianReviewSessionOutcome::Aborted { report: _ } => GuardianReviewOutcome::Aborted,
GuardianReviewSessionOutcome::TimedOut { report } => {
GuardianReviewOutcome::TimedOut { report }
}
GuardianReviewSessionOutcome::Aborted { report } => {
GuardianReviewOutcome::Aborted { report }
}
}
}
@@ -0,0 +1,334 @@
use std::time::Duration;
use std::time::SystemTime;
use std::time::UNIX_EPOCH;
use codex_analytics::GuardianCommandSource as AnalyticsGuardianCommandSource;
use codex_analytics::GuardianReviewDecision;
use codex_analytics::GuardianReviewEventParams;
use codex_analytics::GuardianReviewFailureKind;
use codex_analytics::GuardianReviewRiskLevel as AnalyticsGuardianRiskLevel;
use codex_analytics::GuardianReviewTerminalStatus;
use codex_analytics::GuardianReviewTrigger;
use codex_analytics::GuardianReviewedAction;
use codex_features::Feature;
use codex_protocol::protocol::GuardianCommandSource;
use codex_protocol::protocol::GuardianRiskLevel;
use codex_shell_command::parse_command::shlex_join;
use crate::codex::Session;
use super::GUARDIAN_MAX_ACTION_STRING_TOKENS;
use super::GUARDIAN_REVIEW_TIMEOUT;
use super::GuardianApprovalRequest;
use super::GuardianAssessment;
use super::prompt::guardian_truncate_text;
use super::review_session_analytics::GuardianReviewSessionReport;
pub(super) struct GuardianReviewAnalyticsInput<'a> {
pub(super) review_id: String,
pub(super) target_item_id: String,
pub(super) turn_id: String,
pub(super) trigger: GuardianReviewTrigger,
pub(super) retry_reason: Option<String>,
pub(super) delegated_review: bool,
pub(super) reviewed_action: GuardianReviewedAction,
pub(super) reviewed_action_truncated: bool,
pub(super) decision: GuardianReviewDecision,
pub(super) terminal_status: GuardianReviewTerminalStatus,
pub(super) failure_kind: Option<GuardianReviewFailureKind>,
pub(super) assessment: Option<&'a GuardianAssessment>,
pub(super) report: Option<GuardianReviewSessionReport>,
pub(super) started_at: u64,
pub(super) completed_at: Option<u64>,
pub(super) completion_latency_ms: Option<u64>,
}
pub(super) async fn track_guardian_review(
session: &Session,
input: GuardianReviewAnalyticsInput<'_>,
) {
if !session.enabled(Feature::GeneralAnalytics) {
return;
}
let client_metadata = session.app_server_client_metadata().await;
let GuardianReviewAnalyticsInput {
review_id,
target_item_id,
turn_id,
trigger,
retry_reason,
delegated_review,
reviewed_action,
reviewed_action_truncated,
decision,
terminal_status,
failure_kind,
assessment,
report,
started_at,
completed_at,
completion_latency_ms,
} = input;
let (risk_score, risk_level, rationale) = assessment.map_or((None, None, None), |assessment| {
(
Some(assessment.risk_score),
Some(analytics_risk_level(assessment.risk_level)),
Some(assessment.rationale.clone()),
)
});
let (
guardian_thread_id,
guardian_session_kind,
guardian_model,
guardian_reasoning_effort,
had_prior_review_context,
guardian_tool_call_counts,
guardian_time_to_first_token_ms,
token_usage,
) = match report {
Some(report) => (
Some(report.guardian_thread_id),
Some(report.session_kind),
report.guardian_model,
report.guardian_reasoning_effort,
Some(report.had_prior_review_context),
report.tool_call_counts,
report.time_to_first_token_ms,
report.token_usage,
),
None => (None, None, None, None, None, Default::default(), None, None),
};
let guardian_tool_call_count = guardian_tool_call_counts.total();
session
.services
.analytics_events_client
.track_guardian_review(GuardianReviewEventParams {
thread_id: session.conversation_id.to_string(),
turn_id,
review_id,
target_item_id,
product_client_id: client_metadata.client_name,
trigger,
retry_reason,
delegated_review,
reviewed_action,
reviewed_action_truncated,
decision,
terminal_status,
failure_kind,
risk_score,
risk_level,
rationale,
guardian_thread_id,
guardian_session_kind,
guardian_model,
guardian_reasoning_effort,
had_prior_review_context,
review_timeout_ms: duration_millis_u64(GUARDIAN_REVIEW_TIMEOUT),
guardian_tool_call_count,
guardian_tool_call_counts,
guardian_time_to_first_token_ms,
guardian_completion_latency_ms: completion_latency_ms,
started_at,
completed_at,
input_tokens: token_usage.as_ref().map(|usage| usage.input_tokens),
cached_input_tokens: token_usage.as_ref().map(|usage| usage.cached_input_tokens),
output_tokens: token_usage.as_ref().map(|usage| usage.output_tokens),
reasoning_output_tokens: token_usage
.as_ref()
.map(|usage| usage.reasoning_output_tokens),
total_tokens: token_usage.as_ref().map(|usage| usage.total_tokens),
});
}
pub(super) fn guardian_reviewed_action(
request: &GuardianApprovalRequest,
) -> (GuardianReviewTrigger, GuardianReviewedAction, bool) {
match request {
GuardianApprovalRequest::Shell {
command,
cwd,
sandbox_permissions,
additional_permissions,
justification,
..
} => (
GuardianReviewTrigger::Shell,
GuardianReviewedAction::Shell {
command: command.clone(),
command_display: shlex_join(command),
cwd: cwd.display().to_string(),
sandbox_permissions: *sandbox_permissions,
additional_permissions: additional_permissions.clone(),
justification: justification.clone(),
},
false,
),
GuardianApprovalRequest::ExecCommand {
command,
cwd,
sandbox_permissions,
additional_permissions,
justification,
tty,
..
} => (
GuardianReviewTrigger::UnifiedExec,
GuardianReviewedAction::UnifiedExec {
command: command.clone(),
command_display: shlex_join(command),
cwd: cwd.display().to_string(),
sandbox_permissions: *sandbox_permissions,
additional_permissions: additional_permissions.clone(),
justification: justification.clone(),
tty: *tty,
},
false,
),
#[cfg(unix)]
GuardianApprovalRequest::Execve {
source,
program,
argv,
cwd,
additional_permissions,
..
} => (
GuardianReviewTrigger::Execve,
GuardianReviewedAction::Execve {
source: analytics_command_source(*source),
program: program.clone(),
argv: argv.clone(),
cwd: cwd.display().to_string(),
additional_permissions: additional_permissions.clone(),
},
false,
),
GuardianApprovalRequest::ApplyPatch {
cwd, files, patch, ..
} => {
let (patch, truncated) = truncate_analytics_string(patch);
(
GuardianReviewTrigger::ApplyPatch,
GuardianReviewedAction::ApplyPatch {
cwd: cwd.display().to_string(),
files: files
.iter()
.map(|path| path.to_path_buf().display().to_string())
.collect(),
patch: Some(patch),
},
truncated,
)
}
GuardianApprovalRequest::NetworkAccess {
target,
host,
protocol,
port,
..
} => (
GuardianReviewTrigger::NetworkAccess,
GuardianReviewedAction::NetworkAccess {
target: target.clone(),
host: host.clone(),
protocol: *protocol,
port: *port,
},
false,
),
GuardianApprovalRequest::McpToolCall {
server,
tool_name,
arguments,
connector_id,
connector_name,
tool_title,
..
} => {
let (arguments, truncated) = arguments
.clone()
.map(truncate_analytics_json_value)
.map_or((None, false), |(value, truncated)| (Some(value), truncated));
(
GuardianReviewTrigger::McpToolCall,
GuardianReviewedAction::McpToolCall {
server: server.clone(),
tool_name: tool_name.clone(),
arguments,
connector_id: connector_id.clone(),
connector_name: connector_name.clone(),
tool_title: tool_title.clone(),
},
truncated,
)
}
}
}
pub(super) fn now_unix_timestamp_secs() -> u64 {
SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap_or_default()
.as_secs()
}
pub(super) fn duration_millis_u64(duration: Duration) -> u64 {
u64::try_from(duration.as_millis()).unwrap_or(u64::MAX)
}
fn analytics_risk_level(risk_level: GuardianRiskLevel) -> AnalyticsGuardianRiskLevel {
match risk_level {
GuardianRiskLevel::Low => AnalyticsGuardianRiskLevel::Low,
GuardianRiskLevel::Medium => AnalyticsGuardianRiskLevel::Medium,
GuardianRiskLevel::High => AnalyticsGuardianRiskLevel::High,
}
}
fn analytics_command_source(source: GuardianCommandSource) -> AnalyticsGuardianCommandSource {
match source {
GuardianCommandSource::Shell => AnalyticsGuardianCommandSource::Shell,
GuardianCommandSource::UnifiedExec => AnalyticsGuardianCommandSource::UnifiedExec,
}
}
fn truncate_analytics_json_value(value: serde_json::Value) -> (serde_json::Value, bool) {
match value {
serde_json::Value::String(text) => {
let (text, truncated) = truncate_analytics_string(&text);
(serde_json::Value::String(text), truncated)
}
serde_json::Value::Array(values) => {
let mut truncated = false;
let values = values
.into_iter()
.map(|value| {
let (value, value_truncated) = truncate_analytics_json_value(value);
truncated |= value_truncated;
value
})
.collect();
(serde_json::Value::Array(values), truncated)
}
serde_json::Value::Object(values) => {
let mut truncated = false;
let values = values
.into_iter()
.map(|(key, value)| {
let (value, value_truncated) = truncate_analytics_json_value(value);
truncated |= value_truncated;
(key, value)
})
.collect();
(serde_json::Value::Object(values), truncated)
}
value => (value, false),
}
}
fn truncate_analytics_string(text: &str) -> (String, bool) {
let truncated = guardian_truncate_text(text, GUARDIAN_MAX_ACTION_STRING_TOKENS);
let was_truncated = truncated != text;
(truncated, was_truncated)
}
@@ -60,7 +60,6 @@ const GUARDIAN_FOLLOWUP_REVIEW_REMINDER: &str = concat!(
);
#[derive(Debug)]
#[allow(dead_code)]
pub(crate) enum GuardianReviewSessionOutcome {
Completed {
result: anyhow::Result<Option<String>>,
@@ -7,7 +7,6 @@ use codex_protocol::protocol::ExecCommandSource;
use codex_protocol::protocol::TokenUsage;
#[derive(Debug, Clone)]
#[allow(dead_code)]
pub(crate) struct GuardianReviewSessionReport {
pub(crate) guardian_thread_id: String,
pub(crate) session_kind: GuardianReviewSessionKind,
+15 -3
View File
@@ -603,7 +603,11 @@ async fn guardian_review_request_layout_matches_model_visible_request_snapshot()
/*external_cancel*/ None,
)
.await;
let GuardianReviewOutcome::Completed(Ok(assessment)) = outcome else {
let GuardianReviewOutcome::Completed {
result: Ok(assessment),
..
} = outcome
else {
panic!("expected guardian assessment");
};
assert_eq!(assessment.risk_score, 35);
@@ -707,10 +711,18 @@ async fn guardian_reuses_prompt_cache_key_and_appends_prior_reviews() -> anyhow:
)
.await;
let GuardianReviewOutcome::Completed(Ok(first_assessment)) = first_outcome else {
let GuardianReviewOutcome::Completed {
result: Ok(first_assessment),
..
} = first_outcome
else {
panic!("expected first guardian assessment");
};
let GuardianReviewOutcome::Completed(Ok(second_assessment)) = second_outcome else {
let GuardianReviewOutcome::Completed {
result: Ok(second_assessment),
..
} = second_outcome
else {
panic!("expected second guardian assessment");
};
assert_eq!(first_assessment.risk_score, 5);