From 3bf623bd907251b065845e1b3c3ef194e5201501 Mon Sep 17 00:00:00 2001 From: rhan-oai Date: Tue, 14 Apr 2026 14:50:37 -0700 Subject: [PATCH] [codex-analytics] guardian review analytics schema polishing --- .../analytics/src/analytics_client_tests.rs | 136 ++++++++++++++++++ codex-rs/analytics/src/events.rs | 83 +++-------- codex-rs/analytics/src/lib.rs | 14 +- codex-rs/core/src/compact.rs | 10 +- codex-rs/protocol/src/approvals.rs | 8 ++ codex-rs/protocol/src/protocol.rs | 1 + 6 files changed, 178 insertions(+), 74 deletions(-) diff --git a/codex-rs/analytics/src/analytics_client_tests.rs b/codex-rs/analytics/src/analytics_client_tests.rs index e5aca39b8e..f6bb2b799b 100644 --- a/codex-rs/analytics/src/analytics_client_tests.rs +++ b/codex-rs/analytics/src/analytics_client_tests.rs @@ -7,6 +7,12 @@ use crate::events::CodexCompactionEventRequest; use crate::events::CodexPluginEventRequest; use crate::events::CodexPluginUsedEventRequest; use crate::events::CodexRuntimeMetadata; +use crate::events::GuardianApprovalRequestSource; +use crate::events::GuardianReviewDecision; +use crate::events::GuardianReviewEventParams; +use crate::events::GuardianReviewFailureReason; +use crate::events::GuardianReviewTerminalStatus; +use crate::events::GuardianReviewedAction; use crate::events::ThreadInitializationMode; use crate::events::ThreadInitializedEvent; use crate::events::ThreadInitializedEventParams; @@ -57,6 +63,7 @@ use codex_plugin::AppConnectorId; use codex_plugin::PluginCapabilitySummary; use codex_plugin::PluginId; use codex_plugin::PluginTelemetryMetadata; +use codex_protocol::approvals::NetworkApprovalProtocol; use codex_protocol::protocol::SubAgentSource; use pretty_assertions::assert_eq; use serde_json::json; @@ -685,6 +692,135 @@ async fn compaction_event_ingests_custom_fact() { assert_eq!(payload[0]["event_params"]["status"], "failed"); } +#[tokio::test] +async fn guardian_review_event_ingests_custom_fact_with_optional_target_item() { + let mut reducer = AnalyticsReducer::default(); + let mut events = Vec::new(); + + reducer + .ingest( + AnalyticsFact::Initialize { + connection_id: 7, + params: InitializeParams { + client_info: ClientInfo { + name: "codex-tui".to_string(), + title: None, + version: "1.0.0".to_string(), + }, + capabilities: Some(InitializeCapabilities { + experimental_api: false, + opt_out_notification_methods: None, + }), + }, + product_client_id: DEFAULT_ORIGINATOR.to_string(), + runtime: sample_runtime_metadata(), + rpc_transport: AppServerRpcTransport::Websocket, + }, + &mut events, + ) + .await; + reducer + .ingest( + AnalyticsFact::Response { + connection_id: 7, + response: Box::new(sample_thread_start_response( + "thread-guardian", + /*ephemeral*/ false, + "gpt-5", + )), + }, + &mut events, + ) + .await; + events.clear(); + + reducer + .ingest( + AnalyticsFact::Custom(CustomAnalyticsFact::GuardianReview(Box::new( + GuardianReviewEventParams { + thread_id: "thread-guardian".to_string(), + turn_id: "turn-guardian".to_string(), + review_id: "review-guardian".to_string(), + target_item_id: None, + approval_request_source: GuardianApprovalRequestSource::DelegatedSubagent, + reviewed_action: GuardianReviewedAction::NetworkAccess { + protocol: NetworkApprovalProtocol::Https, + port: 443, + }, + reviewed_action_truncated: false, + decision: GuardianReviewDecision::Denied, + terminal_status: GuardianReviewTerminalStatus::TimedOut, + failure_reason: Some(GuardianReviewFailureReason::Timeout), + risk_level: None, + user_authorization: None, + outcome: None, + guardian_thread_id: None, + guardian_session_kind: None, + guardian_model: None, + guardian_reasoning_effort: None, + had_prior_review_context: None, + review_timeout_ms: 90_000, + tool_call_count: None, + time_to_first_token_ms: None, + completion_latency_ms: Some(90_000), + started_at: 100, + completed_at: Some(190), + input_tokens: None, + cached_input_tokens: None, + output_tokens: None, + reasoning_output_tokens: None, + total_tokens: None, + }, + ))), + &mut events, + ) + .await; + + let payload = serde_json::to_value(&events).expect("serialize events"); + assert_eq!(payload.as_array().expect("events array").len(), 1); + assert_eq!(payload[0]["event_type"], "codex_guardian_review"); + assert_eq!(payload[0]["event_params"]["thread_id"], "thread-guardian"); + assert_eq!(payload[0]["event_params"]["turn_id"], "turn-guardian"); + assert_eq!(payload[0]["event_params"]["review_id"], "review-guardian"); + assert_eq!(payload[0]["event_params"]["target_item_id"], json!(null)); + assert_eq!( + payload[0]["event_params"]["approval_request_source"], + "delegated_subagent" + ); + assert_eq!( + payload[0]["event_params"]["app_server_client"]["product_client_id"], + DEFAULT_ORIGINATOR + ); + assert_eq!( + payload[0]["event_params"]["runtime"]["codex_rs_version"], + "0.1.0" + ); + assert_eq!( + payload[0]["event_params"]["reviewed_action"]["type"], + "network_access" + ); + assert_eq!( + payload[0]["event_params"]["reviewed_action"]["protocol"], + "https" + ); + assert_eq!(payload[0]["event_params"]["reviewed_action"]["port"], 443); + assert!(payload[0]["event_params"].get("retry_reason").is_none()); + assert!(payload[0]["event_params"].get("rationale").is_none()); + assert!( + payload[0]["event_params"]["reviewed_action"] + .get("target") + .is_none() + ); + assert!( + payload[0]["event_params"]["reviewed_action"] + .get("host") + .is_none() + ); + assert_eq!(payload[0]["event_params"]["terminal_status"], "timed_out"); + assert_eq!(payload[0]["event_params"]["failure_reason"], "timeout"); + assert_eq!(payload[0]["event_params"]["review_timeout_ms"], 90_000); +} + #[test] fn subagent_thread_started_review_serializes_expected_shape() { let event = TrackEventRequest::ThreadInitialized(subagent_thread_started_event_request( diff --git a/codex-rs/analytics/src/events.rs b/codex-rs/analytics/src/events.rs index 618dd8ffeb..c0e093b0e9 100644 --- a/codex-rs/analytics/src/events.rs +++ b/codex-rs/analytics/src/events.rs @@ -1,5 +1,11 @@ use crate::facts::AppInvocation; use crate::facts::CodexCompactionEvent; +use crate::facts::CompactionImplementation; +use crate::facts::CompactionPhase; +use crate::facts::CompactionReason; +use crate::facts::CompactionStatus; +use crate::facts::CompactionStrategy; +use crate::facts::CompactionTrigger; use crate::facts::InvocationType; use crate::facts::PluginState; use crate::facts::SubAgentThreadStartedInput; @@ -9,6 +15,10 @@ use codex_plugin::PluginTelemetryMetadata; use codex_protocol::approvals::NetworkApprovalProtocol; use codex_protocol::models::PermissionProfile; use codex_protocol::models::SandboxPermissions; +use codex_protocol::protocol::GuardianAssessmentOutcome; +use codex_protocol::protocol::GuardianCommandSource; +use codex_protocol::protocol::GuardianRiskLevel; +use codex_protocol::protocol::GuardianUserAuthorization; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SubAgentSource; use serde::Serialize; @@ -147,31 +157,6 @@ pub enum GuardianReviewSessionKind { EphemeralForked, } -#[derive(Clone, Copy, Debug, Serialize)] -#[serde(rename_all = "lowercase")] -pub enum GuardianReviewRiskLevel { - Low, - Medium, - High, - Critical, -} - -#[derive(Clone, Copy, Debug, Serialize)] -#[serde(rename_all = "lowercase")] -pub enum GuardianReviewUserAuthorization { - Unknown, - Low, - Medium, - High, -} - -#[derive(Clone, Copy, Debug, Serialize)] -#[serde(rename_all = "lowercase")] -pub enum GuardianReviewOutcome { - Allow, - Deny, -} - #[derive(Clone, Copy, Debug, Serialize)] #[serde(rename_all = "snake_case")] pub enum GuardianApprovalRequestSource { @@ -186,36 +171,21 @@ pub enum GuardianApprovalRequestSource { #[serde(tag = "type", rename_all = "snake_case")] pub enum GuardianReviewedAction { Shell { - command: Vec, - command_display: String, - cwd: String, sandbox_permissions: SandboxPermissions, additional_permissions: Option, - justification: Option, }, UnifiedExec { - command: Vec, - command_display: String, - cwd: String, sandbox_permissions: SandboxPermissions, additional_permissions: Option, - justification: Option, tty: bool, }, Execve { source: GuardianCommandSource, program: String, - argv: Vec, - cwd: String, additional_permissions: Option, }, - ApplyPatch { - cwd: String, - files: Vec, - }, + ApplyPatch {}, NetworkAccess { - target: String, - host: String, protocol: NetworkApprovalProtocol, port: u16, }, @@ -228,37 +198,28 @@ pub enum GuardianReviewedAction { }, } -#[derive(Clone, Copy, Debug, Serialize)] -#[serde(rename_all = "snake_case")] -pub enum GuardianCommandSource { - Shell, - UnifiedExec, -} - #[derive(Clone, Serialize)] pub struct GuardianReviewEventParams { pub thread_id: String, pub turn_id: String, pub review_id: String, - pub target_item_id: String, - pub retry_reason: Option, + pub target_item_id: Option, pub approval_request_source: GuardianApprovalRequestSource, pub reviewed_action: GuardianReviewedAction, pub reviewed_action_truncated: bool, pub decision: GuardianReviewDecision, pub terminal_status: GuardianReviewTerminalStatus, pub failure_reason: Option, - pub risk_level: Option, - pub user_authorization: Option, - pub outcome: Option, - pub rationale: Option, + pub risk_level: Option, + pub user_authorization: Option, + pub outcome: Option, pub guardian_thread_id: Option, pub guardian_session_kind: Option, pub guardian_model: Option, pub guardian_reasoning_effort: Option, pub had_prior_review_context: Option, pub review_timeout_ms: u64, - pub tool_call_count: u64, + pub tool_call_count: Option, pub time_to_first_token_ms: Option, pub completion_latency_ms: Option, pub started_at: u64, @@ -310,12 +271,12 @@ pub(crate) struct CodexCompactionEventParams { pub(crate) thread_source: Option<&'static str>, pub(crate) subagent_source: Option, pub(crate) parent_thread_id: Option, - pub(crate) trigger: crate::facts::CompactionTrigger, - pub(crate) reason: crate::facts::CompactionReason, - pub(crate) implementation: crate::facts::CompactionImplementation, - pub(crate) phase: crate::facts::CompactionPhase, - pub(crate) strategy: crate::facts::CompactionStrategy, - pub(crate) status: crate::facts::CompactionStatus, + pub(crate) trigger: CompactionTrigger, + pub(crate) reason: CompactionReason, + pub(crate) implementation: CompactionImplementation, + pub(crate) phase: CompactionPhase, + pub(crate) strategy: CompactionStrategy, + pub(crate) status: CompactionStatus, pub(crate) error: Option, pub(crate) active_context_tokens_before: i64, pub(crate) active_context_tokens_after: i64, diff --git a/codex-rs/analytics/src/lib.rs b/codex-rs/analytics/src/lib.rs index 9b4cc1e9bc..8c07a26209 100644 --- a/codex-rs/analytics/src/lib.rs +++ b/codex-rs/analytics/src/lib.rs @@ -3,18 +3,17 @@ mod events; mod facts; mod reducer; +use std::time::SystemTime; +use std::time::UNIX_EPOCH; + pub use client::AnalyticsEventsClient; pub use events::AppServerRpcTransport; pub use events::GuardianApprovalRequestSource; -pub use events::GuardianCommandSource; pub use events::GuardianReviewDecision; pub use events::GuardianReviewEventParams; pub use events::GuardianReviewFailureReason; -pub use events::GuardianReviewOutcome; -pub use events::GuardianReviewRiskLevel; pub use events::GuardianReviewSessionKind; pub use events::GuardianReviewTerminalStatus; -pub use events::GuardianReviewUserAuthorization; pub use events::GuardianReviewedAction; pub use facts::AppInvocation; pub use facts::CodexCompactionEvent; @@ -32,3 +31,10 @@ pub use facts::build_track_events_context; #[cfg(test)] mod analytics_client_tests; + +pub fn now_unix_seconds() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_secs() +} diff --git a/codex-rs/core/src/compact.rs b/codex-rs/core/src/compact.rs index e132ce0213..59fb45454d 100644 --- a/codex-rs/core/src/compact.rs +++ b/codex-rs/core/src/compact.rs @@ -1,7 +1,5 @@ use std::sync::Arc; use std::time::Instant; -use std::time::SystemTime; -use std::time::UNIX_EPOCH; use crate::Prompt; use crate::client::ModelClientSession; @@ -19,6 +17,7 @@ use codex_analytics::CompactionReason; use codex_analytics::CompactionStatus; use codex_analytics::CompactionStrategy; use codex_analytics::CompactionTrigger; +use codex_analytics::now_unix_seconds; use codex_features::Feature; use codex_model_provider_info::ModelProviderInfo; use codex_protocol::error::CodexErr; @@ -372,13 +371,6 @@ pub(crate) fn compaction_status_from_result(result: &CodexResult) -> Compa } } -fn now_unix_seconds() -> u64 { - SystemTime::now() - .duration_since(UNIX_EPOCH) - .map(|duration| duration.as_secs()) - .unwrap_or_default() -} - pub fn content_items_to_text(content: &[ContentItem]) -> Option { let mut pieces = Vec::new(); for item in content { diff --git a/codex-rs/protocol/src/approvals.rs b/codex-rs/protocol/src/approvals.rs index b154946f42..366f394433 100644 --- a/codex-rs/protocol/src/approvals.rs +++ b/codex-rs/protocol/src/approvals.rs @@ -99,6 +99,14 @@ pub enum GuardianUserAuthorization { High, } +/// Final allow/deny outcome returned by the guardian reviewer. +#[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "lowercase")] +pub enum GuardianAssessmentOutcome { + Allow, + Deny, +} + #[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] #[serde(rename_all = "snake_case")] pub enum GuardianAssessmentStatus { diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 1fc707469e..cb5dea34f0 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -67,6 +67,7 @@ pub use crate::approvals::ExecPolicyAmendment; pub use crate::approvals::GuardianAssessmentAction; pub use crate::approvals::GuardianAssessmentDecisionSource; pub use crate::approvals::GuardianAssessmentEvent; +pub use crate::approvals::GuardianAssessmentOutcome; pub use crate::approvals::GuardianAssessmentStatus; pub use crate::approvals::GuardianCommandSource; pub use crate::approvals::GuardianRiskLevel;