From d70044072c05e8a5c8b16cac76c75368a860c454 Mon Sep 17 00:00:00 2001 From: jif Date: Mon, 7 Sep 2026 14:51:43 +0000 Subject: [PATCH] Expose shared Guardian reviewer helpers through `guardian_review` (#43490) ## What changed Add a public `codex_core::guardian_review` module exposing `GuardianAssessment`, the assessment parser and output schema, and the review session configuration builder for reuse by the Guardian extension. Extract assessment handling and reviewer configuration into dedicated modules, preserving the existing parsing defaults, policy prompt, and read-only reviewer settings. ## Testing Move the existing embedded-JSON, bare allow/deny, and output-schema tests alongside the assessment implementation. GitOrigin-RevId: cb2aba3ccdb597e2876015e718e923d4f8f36802 --- codex-rs/core/src/guardian/assessment.rs | 123 ++++++++++++++++++ .../core/src/guardian/assessment_tests.rs | 83 ++++++++++++ codex-rs/core/src/guardian/mod.rs | 20 +-- codex-rs/core/src/guardian/prompt.rs | 107 +-------------- codex-rs/core/src/guardian/review.rs | 2 +- codex-rs/core/src/guardian/review_session.rs | 102 +-------------- .../core/src/guardian/review_session_tests.rs | 5 +- codex-rs/core/src/guardian/reviewer_config.rs | 113 ++++++++++++++++ codex-rs/core/src/guardian/tests.rs | 83 ------------ codex-rs/core/src/guardian_review.rs | 8 ++ codex-rs/core/src/lib.rs | 1 + 11 files changed, 343 insertions(+), 304 deletions(-) create mode 100644 codex-rs/core/src/guardian/assessment.rs create mode 100644 codex-rs/core/src/guardian/assessment_tests.rs create mode 100644 codex-rs/core/src/guardian/reviewer_config.rs create mode 100644 codex-rs/core/src/guardian_review.rs diff --git a/codex-rs/core/src/guardian/assessment.rs b/codex-rs/core/src/guardian/assessment.rs new file mode 100644 index 0000000000..5ce3cf57a0 --- /dev/null +++ b/codex-rs/core/src/guardian/assessment.rs @@ -0,0 +1,123 @@ +//! The production synchronous reviewer's output schema and tolerant JSON parser. +//! Parsing defaults and error text are shared unchanged by all reviewer callers. + +use codex_protocol::protocol::GuardianAssessmentOutcome; +use codex_protocol::protocol::GuardianRiskLevel; +use codex_protocol::protocol::GuardianUserAuthorization; +use serde::Deserialize; +use serde::Serialize; +use serde_json::Value; + +/// Structured output contract that the guardian reviewer must satisfy. +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)] +pub struct GuardianAssessment { + pub risk_level: codex_protocol::protocol::GuardianRiskLevel, + pub user_authorization: codex_protocol::protocol::GuardianUserAuthorization, + pub outcome: GuardianAssessmentOutcome, + pub rationale: String, +} + +/// The model is asked for strict JSON, but we still accept a surrounding prose +/// wrapper so transient formatting drift fails less noisily during dogfooding. +/// Non-JSON output is still a review failure; this is only a thin recovery path +/// for cases where the model wrapped the JSON in extra prose. +pub fn parse_guardian_assessment(text: Option<&str>) -> anyhow::Result { + let Some(text) = text else { + anyhow::bail!("guardian review completed without an assessment payload"); + }; + let parsed_payload = + if let Ok(payload) = serde_json::from_str::(text) { + payload + } else if let (Some(start), Some(end)) = (text.find('{'), text.rfind('}')) + && start < end + && let Some(slice) = text.get(start..=end) + { + serde_json::from_str::(slice)? + } else { + anyhow::bail!("guardian assessment was not valid JSON"); + }; + + let outcome = parsed_payload.outcome; + let risk_level = parsed_payload.risk_level.unwrap_or(match outcome { + GuardianAssessmentOutcome::Allow => GuardianRiskLevel::Low, + GuardianAssessmentOutcome::Deny => GuardianRiskLevel::High, + }); + let rationale = parsed_payload + .rationale + .filter(|rationale| !rationale.trim().is_empty()) + .unwrap_or_else(|| match outcome { + GuardianAssessmentOutcome::Allow => { + "Auto-review returned a low-risk allow decision.".to_string() + } + GuardianAssessmentOutcome::Deny => { + "Auto-review returned a deny decision without a rationale.".to_string() + } + }); + + Ok(GuardianAssessment { + risk_level, + user_authorization: parsed_payload + .user_authorization + .unwrap_or(GuardianUserAuthorization::Unknown), + outcome, + rationale, + }) +} + +#[derive(Deserialize)] +struct GuardianAssessmentPayload { + risk_level: Option, + user_authorization: Option, + outcome: GuardianAssessmentOutcome, + rationale: Option, +} + +/// JSON schema supplied as `final_output_json_schema` to guide a structured +/// final answer from the guardian review session. +/// +/// Keep this next to `guardian_output_contract_prompt()` so the prompt text and +/// output schema stay aligned. +pub fn guardian_output_schema() -> Value { + serde_json::json!({ + "type": "object", + "additionalProperties": false, + "properties": { + "risk_level": { + "type": "string", + "enum": ["low", "medium", "high", "critical"] + }, + "user_authorization": { + "type": "string", + "enum": ["unknown", "low", "medium", "high"] + }, + "outcome": { + "type": "string", + "enum": ["allow", "deny"] + }, + "rationale": { + "type": "string" + } + }, + "required": ["outcome"] + }) +} + +/// Prompt fragment that describes the exact JSON contract paired with +/// `guardian_output_schema()`. +pub(super) fn guardian_output_contract_prompt() -> &'static str { + r#"You may use read-only tool checks to gather any additional context you need before deciding. When you are ready to answer, your final message must be strict JSON. + +For low-risk actions, give the final answer directly: {"outcome":"allow"}. + +For anything else, use this JSON schema: +{ + "risk_level": "low" | "medium" | "high" | "critical", + "user_authorization": "unknown" | "low" | "medium" | "high", + "outcome": "allow" | "deny", + "rationale": string +}"# +} + +#[cfg(test)] +#[path = "assessment_tests.rs"] +mod tests; diff --git a/codex-rs/core/src/guardian/assessment_tests.rs b/codex-rs/core/src/guardian/assessment_tests.rs new file mode 100644 index 0000000000..f30113a7aa --- /dev/null +++ b/codex-rs/core/src/guardian/assessment_tests.rs @@ -0,0 +1,83 @@ +use super::*; +use pretty_assertions::assert_eq; + +#[test] +fn parse_guardian_assessment_extracts_embedded_json() { + let parsed = parse_guardian_assessment(Some( + "preface {\"risk_level\":\"medium\",\"user_authorization\":\"low\",\"outcome\":\"allow\",\"rationale\":\"ok\"}", + )) + .expect("guardian assessment"); + + assert_eq!( + parsed, + GuardianAssessment { + risk_level: GuardianRiskLevel::Medium, + user_authorization: GuardianUserAuthorization::Low, + outcome: GuardianAssessmentOutcome::Allow, + rationale: "ok".to_string(), + } + ); +} + +#[test] +fn parse_guardian_assessment_treats_bare_allow_as_low_risk() { + let parsed = + parse_guardian_assessment(Some(r#"{"outcome":"allow"}"#)).expect("guardian assessment"); + + assert_eq!( + parsed, + GuardianAssessment { + risk_level: GuardianRiskLevel::Low, + user_authorization: GuardianUserAuthorization::Unknown, + outcome: GuardianAssessmentOutcome::Allow, + rationale: "Auto-review returned a low-risk allow decision.".to_string(), + } + ); +} + +#[test] +fn parse_guardian_assessment_treats_bare_deny_as_high_risk() { + let parsed = + parse_guardian_assessment(Some(r#"{"outcome":"deny"}"#)).expect("guardian assessment"); + + assert_eq!( + parsed, + GuardianAssessment { + risk_level: GuardianRiskLevel::High, + user_authorization: GuardianUserAuthorization::Unknown, + outcome: GuardianAssessmentOutcome::Deny, + rationale: "Auto-review returned a deny decision without a rationale.".to_string(), + } + ); +} + +#[test] +fn guardian_output_schema_requires_only_outcome_and_allows_optional_details() { + let schema = guardian_output_schema(); + + assert_eq!( + schema, + serde_json::json!({ + "type": "object", + "additionalProperties": false, + "properties": { + "risk_level": { + "type": "string", + "enum": ["low", "medium", "high", "critical"] + }, + "user_authorization": { + "type": "string", + "enum": ["unknown", "low", "medium", "high"] + }, + "outcome": { + "type": "string", + "enum": ["allow", "deny"] + }, + "rationale": { + "type": "string" + } + }, + "required": ["outcome"] + }) + ); +} diff --git a/codex-rs/core/src/guardian/mod.rs b/codex-rs/core/src/guardian/mod.rs index 01ae1e95ea..f8e02251fd 100644 --- a/codex-rs/core/src/guardian/mod.rs +++ b/codex-rs/core/src/guardian/mod.rs @@ -3,6 +3,7 @@ //! review requirements. Each approval retains its issuing context and cancellation. mod approval_request; +mod assessment; mod coverage; mod decision; mod feedback; @@ -10,6 +11,7 @@ mod metrics; mod prompt; mod review; mod review_session; +mod reviewer_config; mod runtime; use std::sync::Arc; @@ -21,8 +23,6 @@ use codex_protocol::openai_models::ModelInfo; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::GuardianAssessmentOutcome; -use serde::Deserialize; -use serde::Serialize; use crate::environment_selection::TurnEnvironmentSnapshot; use crate::session::step_context::StepContext; @@ -165,14 +165,10 @@ impl From<&Arc> for GuardianReviewContext { } } -/// Structured output contract that the guardian reviewer must satisfy. -#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)] -pub(crate) struct GuardianAssessment { - pub(crate) risk_level: codex_protocol::protocol::GuardianRiskLevel, - pub(crate) user_authorization: codex_protocol::protocol::GuardianUserAuthorization, - pub(crate) outcome: GuardianAssessmentOutcome, - pub(crate) rationale: String, -} +pub use assessment::GuardianAssessment; +pub use assessment::guardian_output_schema; +pub use assessment::parse_guardian_assessment; +pub use reviewer_config::build_guardian_review_session_config; #[derive(Debug, Default)] pub(crate) struct GuardianRejectionCircuitBreaker { @@ -267,10 +263,6 @@ use prompt::build_guardian_prompt_items; #[cfg(test)] use prompt::build_guardian_prompt_items_with_parent_turn; #[cfg(test)] -use prompt::guardian_output_schema; -#[cfg(test)] -use prompt::parse_guardian_assessment; -#[cfg(test)] use prompt::render_guardian_transcript_entries; #[cfg(test)] use review::GuardianReviewOutcome; diff --git a/codex-rs/core/src/guardian/prompt.rs b/codex-rs/core/src/guardian/prompt.rs index 47b3678160..1959b4348c 100644 --- a/codex-rs/core/src/guardian/prompt.rs +++ b/codex-rs/core/src/guardian/prompt.rs @@ -13,11 +13,7 @@ use codex_guardian_context::TranscriptEntryLimits; use codex_guardian_context::TranscriptRetentionConfig; use codex_guardian_context::default_registry; use codex_protocol::models::ResponseItem; -use codex_protocol::protocol::GuardianRiskLevel; -use codex_protocol::protocol::GuardianUserAuthorization; use codex_protocol::user_input::UserInput; -use serde::Deserialize; -use serde_json::Value; use crate::context::GuardianReviewEvidence; use crate::context::NodeReplReviewEvidence; @@ -38,7 +34,6 @@ use super::GUARDIAN_MAX_TOOL_ENTRY_TOKENS; use super::GUARDIAN_MAX_TOOL_TRANSCRIPT_TOKENS; use super::GUARDIAN_RECENT_ENTRY_LIMIT; use super::GuardianApprovalRequest; -use super::GuardianAssessment; use super::GuardianReviewContext; use super::approval_request::format_guardian_action_pretty; @@ -531,106 +526,8 @@ pub(crate) fn guardian_truncate_text(content: &str, token_cap: usize) -> (String ) } -/// The model is asked for strict JSON, but we still accept a surrounding prose -/// wrapper so transient formatting drift fails less noisily during dogfooding. -/// Non-JSON output is still a review failure; this is only a thin recovery path -/// for cases where the model wrapped the JSON in extra prose. -pub(crate) fn parse_guardian_assessment(text: Option<&str>) -> anyhow::Result { - let Some(text) = text else { - anyhow::bail!("guardian review completed without an assessment payload"); - }; - let parsed_payload = - if let Ok(payload) = serde_json::from_str::(text) { - payload - } else if let (Some(start), Some(end)) = (text.find('{'), text.rfind('}')) - && start < end - && let Some(slice) = text.get(start..=end) - { - serde_json::from_str::(slice)? - } else { - anyhow::bail!("guardian assessment was not valid JSON"); - }; - - let outcome = parsed_payload.outcome; - let risk_level = parsed_payload.risk_level.unwrap_or(match outcome { - super::GuardianAssessmentOutcome::Allow => GuardianRiskLevel::Low, - super::GuardianAssessmentOutcome::Deny => GuardianRiskLevel::High, - }); - let rationale = parsed_payload - .rationale - .filter(|rationale| !rationale.trim().is_empty()) - .unwrap_or_else(|| match outcome { - super::GuardianAssessmentOutcome::Allow => { - "Auto-review returned a low-risk allow decision.".to_string() - } - super::GuardianAssessmentOutcome::Deny => { - "Auto-review returned a deny decision without a rationale.".to_string() - } - }); - - Ok(GuardianAssessment { - risk_level, - user_authorization: parsed_payload - .user_authorization - .unwrap_or(GuardianUserAuthorization::Unknown), - outcome, - rationale, - }) -} - -#[derive(Deserialize)] -struct GuardianAssessmentPayload { - risk_level: Option, - user_authorization: Option, - outcome: super::GuardianAssessmentOutcome, - rationale: Option, -} - -/// JSON schema supplied as `final_output_json_schema` to guide a structured -/// final answer from the guardian review session. -/// -/// Keep this next to `guardian_output_contract_prompt()` so the prompt text and -/// output schema stay aligned. -pub(crate) fn guardian_output_schema() -> Value { - serde_json::json!({ - "type": "object", - "additionalProperties": false, - "properties": { - "risk_level": { - "type": "string", - "enum": ["low", "medium", "high", "critical"] - }, - "user_authorization": { - "type": "string", - "enum": ["unknown", "low", "medium", "high"] - }, - "outcome": { - "type": "string", - "enum": ["allow", "deny"] - }, - "rationale": { - "type": "string" - } - }, - "required": ["outcome"] - }) -} - -/// Prompt fragment that describes the exact JSON contract paired with -/// `guardian_output_schema()`. -fn guardian_output_contract_prompt() -> &'static str { - r#"You may use read-only tool checks to gather any additional context you need before deciding. When you are ready to answer, your final message must be strict JSON. - -For low-risk actions, give the final answer directly: {"outcome":"allow"}. - -For anything else, use this JSON schema: -{ - "risk_level": "low" | "medium" | "high" | "critical", - "user_authorization": "unknown" | "low" | "medium" | "high", - "outcome": "allow" | "deny", - "rationale": string -}"# -} +use super::assessment::guardian_output_contract_prompt; +pub use super::assessment::parse_guardian_assessment; pub(crate) const BUNDLED_GUARDIAN_POLICY: &str = include_str!("../../assets/guardian/policy.md"); pub(crate) const BUNDLED_GUARDIAN_POLICY_TEMPLATE: &str = diff --git a/codex-rs/core/src/guardian/review.rs b/codex-rs/core/src/guardian/review.rs index c63586fe89..452463796f 100644 --- a/codex-rs/core/src/guardian/review.rs +++ b/codex-rs/core/src/guardian/review.rs @@ -54,8 +54,8 @@ use super::approval_request::guardian_assessment_action; use super::approval_request::guardian_request_target_item_id; use super::approval_request::guardian_request_turn_id; use super::approval_request::guardian_reviewed_action; +use super::assessment::guardian_output_schema; use super::metrics::emit_guardian_review_metrics; -use super::prompt::guardian_output_schema; use super::prompt::parse_guardian_assessment; use super::review_session::GuardianReviewSessionOutcome; use super::review_session::GuardianReviewSessionParams; diff --git a/codex-rs/core/src/guardian/review_session.rs b/codex-rs/core/src/guardian/review_session.rs index 9f7ff85192..cec20eb221 100644 --- a/codex-rs/core/src/guardian/review_session.rs +++ b/codex-rs/core/src/guardian/review_session.rs @@ -22,15 +22,12 @@ use codex_protocol::config_types::Personality; use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig; use codex_protocol::items::TurnItem; use codex_protocol::mcp::is_node_repl_backed_server; -use codex_protocol::models::BaseInstructionsProvenance; use codex_protocol::models::ContentItem; use codex_protocol::models::ImageDetail; -use codex_protocol::models::PermissionProfile; use codex_protocol::models::PermissionProfileSnapshot; use codex_protocol::models::ResponseInputItem; use codex_protocol::models::ResponseItem; use codex_protocol::openai_models::InputModality; -use codex_protocol::openai_models::ModelMessages; use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::CodexErrorInfo; @@ -53,7 +50,6 @@ use crate::codex_delegate::run_codex_thread_interactive; use crate::config::Config; use crate::config::Constrained; use crate::config::ManagedFeatures; -use crate::config::NetworkProxySpec; use crate::config::Permissions; use crate::context::ContextualUserFragment; use crate::context::GuardianContextMode; @@ -88,13 +84,13 @@ use super::GuardianReviewContext; use super::feedback::record_failed_review; #[cfg(test)] use super::prompt::BUNDLED_GUARDIAN_POLICY; -use super::prompt::BUNDLED_GUARDIAN_POLICY_TEMPLATE; use super::prompt::GUARDIAN_TRANSCRIPT_START; use super::prompt::GuardianPromptMode; use super::prompt::GuardianTranscriptCursor; use super::prompt::build_guardian_prompt_items_with_parent_turn; -use super::prompt::guardian_policy_prompt_with_config_and_template; use super::review::guardian_review_session_config; +pub(crate) use super::reviewer_config::build_guardian_review_session_config; +use super::reviewer_config::read_only_guardian_permission_profile; const GUARDIAN_INTERRUPT_DRAIN_TIMEOUT: Duration = Duration::from_secs(5); const GUARDIAN_MAX_IMAGE_ITEM_TOKENS: i64 = 10_000; @@ -1530,100 +1526,6 @@ fn event_matches_turn(event: &Event, expected_turn_id: &str) -> bool { } } -fn read_only_guardian_permission_profile( - permission_profile: &PermissionProfile, -) -> PermissionProfile { - permission_profile - .intersect_with_read_only() - .unwrap_or(PermissionProfile::External { - network: codex_protocol::permissions::NetworkSandboxPolicy::Restricted, - }) -} - -pub(crate) fn build_guardian_review_session_config( - parent_config: &Config, - live_network_config: Option, - active_model: &str, - reasoning_effort: Option, - model_messages: Option<&ModelMessages>, -) -> anyhow::Result { - let mut guardian_config = parent_config.clone(); - guardian_config.model = Some(active_model.to_string()); - guardian_config.model_reasoning_effort = reasoning_effort; - guardian_config.model_provider.request_max_retries = Some(1); - guardian_config.model_provider.stream_max_retries = Some(1); - guardian_config.include_skill_instructions = false; - guardian_config.memories.use_memories = false; - guardian_config.memories.dedicated_tools = false; - let catalog_auto_review = model_messages.and_then(|messages| messages.auto_review.as_ref()); - let tenant_policy_config = parent_config.resolve_guardian_policy(model_messages); - let policy_template = catalog_auto_review - .and_then(|messages| messages.policy_template.as_deref()) - .unwrap_or(BUNDLED_GUARDIAN_POLICY_TEMPLATE); - guardian_config.base_instructions = Some(guardian_policy_prompt_with_config_and_template( - tenant_policy_config, - policy_template, - )); - guardian_config.base_instructions_provenance = Some(BaseInstructionsProvenance::Custom); - guardian_config.notify = None; - guardian_config.developer_instructions = None; - guardian_config.permissions.approval_policy = Constrained::allow_only(AskForApproval::Never); - let guardian_permission_profile = - read_only_guardian_permission_profile(parent_config.permissions.permission_profile()); - guardian_config - .permissions - .set_permission_profile(guardian_permission_profile) - .map_err(|err| { - anyhow::anyhow!("guardian review session could not set permission profile: {err}") - })?; - guardian_config.include_apps_instructions = false; - guardian_config - .mcp_servers - .set(HashMap::new()) - .map_err(|err| { - anyhow::anyhow!("guardian review session could not clear MCP servers: {err}") - })?; - if let Some(live_network_config) = live_network_config - && guardian_config.permissions.network.is_some() - { - let network_constraints = guardian_config - .config_layer_stack - .requirements() - .network - .as_ref() - .map(|network| network.value.clone()); - guardian_config.permissions.network = Some(NetworkProxySpec::from_config_and_constraints( - live_network_config, - network_constraints, - guardian_config.permissions.permission_profile(), - )?); - } - for feature in [ - Feature::Collab, - Feature::MultiAgentV2, - Feature::GuardianV2, - Feature::CodexHooks, - Feature::Apps, - Feature::Plugins, - Feature::WebSearchRequest, - Feature::WebSearchCached, - ] { - guardian_config.features.disable(feature).map_err(|err| { - anyhow::anyhow!( - "guardian review session could not disable `features.{}`: {err}", - feature.key() - ) - })?; - if guardian_config.features.enabled(feature) { - warn!( - "guardian review session could not disable `features.{}`; continuing with the feature enabled", - feature.key() - ); - } - } - Ok(guardian_config) -} - async fn run_before_review_deadline( deadline: tokio::time::Instant, external_cancel: Option<&CancellationToken>, diff --git a/codex-rs/core/src/guardian/review_session_tests.rs b/codex-rs/core/src/guardian/review_session_tests.rs index 7f8558e2d5..67ce82fed2 100644 --- a/codex-rs/core/src/guardian/review_session_tests.rs +++ b/codex-rs/core/src/guardian/review_session_tests.rs @@ -1,8 +1,11 @@ +use super::super::prompt::BUNDLED_GUARDIAN_POLICY_TEMPLATE; +use super::super::prompt::guardian_policy_prompt_with_config_and_template; use super::*; use crate::context_manager::ContextManager; use codex_history::CodexHarnessMetadata; use codex_history::ResponseItemEnvelope; use codex_protocol::openai_models::AutoReviewMessages; +use codex_protocol::openai_models::ModelMessages; use codex_protocol::protocol::AgentStatus; use codex_protocol::protocol::ErrorEvent; use codex_protocol::protocol::Submission; @@ -243,7 +246,7 @@ async fn test_review_params() -> GuardianReviewSessionParams { tty: false, }, reasons: ApprovalRequestReasons::default(), - schema: super::super::prompt::guardian_output_schema(), + schema: super::super::guardian_output_schema(), model, compaction_model_hash: None, reasoning_effort, diff --git a/codex-rs/core/src/guardian/reviewer_config.rs b/codex-rs/core/src/guardian/reviewer_config.rs new file mode 100644 index 0000000000..a792069ba4 --- /dev/null +++ b/codex-rs/core/src/guardian/reviewer_config.rs @@ -0,0 +1,113 @@ +//! Builds the production synchronous reviewer settings without starting a session. +//! Keep these settings and the policy prompt identical for core and extension callers. + +use std::collections::HashMap; + +use codex_features::Feature; +use codex_protocol::models::BaseInstructionsProvenance; +use codex_protocol::models::PermissionProfile; +use codex_protocol::openai_models::ModelMessages; +use codex_protocol::protocol::AskForApproval; +use tracing::warn; + +use crate::config::Config; +use crate::config::Constrained; +use crate::config::NetworkProxySpec; + +use super::prompt::BUNDLED_GUARDIAN_POLICY_TEMPLATE; +use super::prompt::guardian_policy_prompt_with_config_and_template; + +pub(super) fn read_only_guardian_permission_profile( + permission_profile: &PermissionProfile, +) -> PermissionProfile { + permission_profile + .intersect_with_read_only() + .unwrap_or(PermissionProfile::External { + network: codex_protocol::permissions::NetworkSandboxPolicy::Restricted, + }) +} + +/// Builds the existing read-only reviewer configuration with its policy and live network rules. +pub fn build_guardian_review_session_config( + parent_config: &Config, + live_network_config: Option, + active_model: &str, + reasoning_effort: Option, + model_messages: Option<&ModelMessages>, +) -> anyhow::Result { + let mut guardian_config = parent_config.clone(); + guardian_config.model = Some(active_model.to_string()); + guardian_config.model_reasoning_effort = reasoning_effort; + guardian_config.model_provider.request_max_retries = Some(1); + guardian_config.model_provider.stream_max_retries = Some(1); + guardian_config.include_skill_instructions = false; + guardian_config.memories.use_memories = false; + guardian_config.memories.dedicated_tools = false; + let catalog_auto_review = model_messages.and_then(|messages| messages.auto_review.as_ref()); + let tenant_policy_config = parent_config.resolve_guardian_policy(model_messages); + let policy_template = catalog_auto_review + .and_then(|messages| messages.policy_template.as_deref()) + .unwrap_or(BUNDLED_GUARDIAN_POLICY_TEMPLATE); + guardian_config.base_instructions = Some(guardian_policy_prompt_with_config_and_template( + tenant_policy_config, + policy_template, + )); + guardian_config.base_instructions_provenance = Some(BaseInstructionsProvenance::Custom); + guardian_config.notify = None; + guardian_config.developer_instructions = None; + guardian_config.permissions.approval_policy = Constrained::allow_only(AskForApproval::Never); + let guardian_permission_profile = + read_only_guardian_permission_profile(parent_config.permissions.permission_profile()); + guardian_config + .permissions + .set_permission_profile(guardian_permission_profile) + .map_err(|err| { + anyhow::anyhow!("guardian review session could not set permission profile: {err}") + })?; + guardian_config.include_apps_instructions = false; + guardian_config + .mcp_servers + .set(HashMap::new()) + .map_err(|err| { + anyhow::anyhow!("guardian review session could not clear MCP servers: {err}") + })?; + if let Some(live_network_config) = live_network_config + && guardian_config.permissions.network.is_some() + { + let network_constraints = guardian_config + .config_layer_stack + .requirements() + .network + .as_ref() + .map(|network| network.value.clone()); + guardian_config.permissions.network = Some(NetworkProxySpec::from_config_and_constraints( + live_network_config, + network_constraints, + guardian_config.permissions.permission_profile(), + )?); + } + for feature in [ + Feature::Collab, + Feature::MultiAgentV2, + Feature::GuardianV2, + Feature::CodexHooks, + Feature::Apps, + Feature::Plugins, + Feature::WebSearchRequest, + Feature::WebSearchCached, + ] { + guardian_config.features.disable(feature).map_err(|err| { + anyhow::anyhow!( + "guardian review session could not disable `features.{}`: {err}", + feature.key() + ) + })?; + if guardian_config.features.enabled(feature) { + warn!( + "guardian review session could not disable `features.{}`; continuing with the feature enabled", + feature.key() + ); + } + } + Ok(guardian_config) +} diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index ac0745faca..c02973f46c 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -61,8 +61,6 @@ use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::GranularApprovalConfig; use codex_protocol::protocol::GuardianAssessmentStatus; -use codex_protocol::protocol::GuardianRiskLevel; -use codex_protocol::protocol::GuardianUserAuthorization; use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::TurnCompleteEvent; use codex_utils_path_uri::LegacyAppPathString; @@ -1936,87 +1934,6 @@ fn build_guardian_transcript_preserves_recent_tool_context_when_user_history_is_ ); } -#[test] -fn parse_guardian_assessment_extracts_embedded_json() { - let parsed = parse_guardian_assessment(Some( - "preface {\"risk_level\":\"medium\",\"user_authorization\":\"low\",\"outcome\":\"allow\",\"rationale\":\"ok\"}", - )) - .expect("guardian assessment"); - - assert_eq!( - parsed, - GuardianAssessment { - risk_level: GuardianRiskLevel::Medium, - user_authorization: GuardianUserAuthorization::Low, - outcome: GuardianAssessmentOutcome::Allow, - rationale: "ok".to_string(), - } - ); -} - -#[test] -fn parse_guardian_assessment_treats_bare_allow_as_low_risk() { - let parsed = - parse_guardian_assessment(Some(r#"{"outcome":"allow"}"#)).expect("guardian assessment"); - - assert_eq!( - parsed, - GuardianAssessment { - risk_level: GuardianRiskLevel::Low, - user_authorization: GuardianUserAuthorization::Unknown, - outcome: GuardianAssessmentOutcome::Allow, - rationale: "Auto-review returned a low-risk allow decision.".to_string(), - } - ); -} - -#[test] -fn parse_guardian_assessment_treats_bare_deny_as_high_risk() { - let parsed = - parse_guardian_assessment(Some(r#"{"outcome":"deny"}"#)).expect("guardian assessment"); - - assert_eq!( - parsed, - GuardianAssessment { - risk_level: GuardianRiskLevel::High, - user_authorization: GuardianUserAuthorization::Unknown, - outcome: GuardianAssessmentOutcome::Deny, - rationale: "Auto-review returned a deny decision without a rationale.".to_string(), - } - ); -} - -#[test] -fn guardian_output_schema_requires_only_outcome_and_allows_optional_details() { - let schema = guardian_output_schema(); - - assert_eq!( - schema, - serde_json::json!({ - "type": "object", - "additionalProperties": false, - "properties": { - "risk_level": { - "type": "string", - "enum": ["low", "medium", "high", "critical"] - }, - "user_authorization": { - "type": "string", - "enum": ["unknown", "low", "medium", "high"] - }, - "outcome": { - "type": "string", - "enum": ["allow", "deny"] - }, - "rationale": { - "type": "string" - } - }, - "required": ["outcome"] - }) - ); -} - enum GuardianTestCatalog { Bundled, ParentOnly, diff --git a/codex-rs/core/src/guardian_review.rs b/codex-rs/core/src/guardian_review.rs new file mode 100644 index 0000000000..c841dc6213 --- /dev/null +++ b/codex-rs/core/src/guardian_review.rs @@ -0,0 +1,8 @@ +//! Production synchronous reviewer helpers shared with the Guardian extension. +//! These use the current policy, output contract and reviewer configuration; +//! making them available does not select a new transcript mode or start a review. + +pub use crate::guardian::GuardianAssessment; +pub use crate::guardian::build_guardian_review_session_config; +pub use crate::guardian::guardian_output_schema; +pub use crate::guardian::parse_guardian_assessment; diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 035bc44b80..581392fb57 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -68,6 +68,7 @@ mod exec_policy; #[cfg(test)] mod git_info_tests; mod guardian; +pub mod guardian_review; mod hook_mcp_executor; mod hook_runtime; mod image_preparation;