From 68c6e5df2be11b87ed7ca448a692e21fc7a18e04 Mon Sep 17 00:00:00 2001 From: rhan-oai Date: Tue, 14 Jul 2026 21:54:14 +0000 Subject: [PATCH] Support model catalog templates for Guardian policy prompts (#33177) ## What changed - Add an optional `policy_template` field to auto-review model messages. - Build Guardian instructions from the catalog template when present, while preserving the existing precedence for managed and catalog policy content. - Fall back independently to the bundled template and bundled policy, and preserve explicit empty catalog values. ## Testing - Cover missing and empty template deserialization, policy precedence, bundled fallbacks, and Guardian prewarm instructions. GitOrigin-RevId: 93116dadbc9bf40527882844a428caed4159fff6 --- codex-rs/core/src/guardian/mod.rs | 4 - codex-rs/core/src/guardian/policy_template.md | 2 +- codex-rs/core/src/guardian/prompt.rs | 26 +++--- codex-rs/core/src/guardian/review_session.rs | 89 +++++++++++++++---- codex-rs/core/src/guardian/tests.rs | 18 +++- .../core/src/session/tests/guardian_tests.rs | 2 +- codex-rs/core/tests/suite/guardian_review.rs | 17 ++-- .../models-manager/src/model_info_tests.rs | 1 + codex-rs/protocol/src/openai_models.rs | 41 +++++++++ 9 files changed, 156 insertions(+), 44 deletions(-) diff --git a/codex-rs/core/src/guardian/mod.rs b/codex-rs/core/src/guardian/mod.rs index b06a2daad3..5402afeaf9 100644 --- a/codex-rs/core/src/guardian/mod.rs +++ b/codex-rs/core/src/guardian/mod.rs @@ -156,10 +156,6 @@ use prompt::collect_guardian_transcript_entries; #[cfg(test)] use prompt::guardian_output_schema; #[cfg(test)] -pub(crate) use prompt::guardian_policy_prompt; -#[cfg(test)] -pub(crate) use prompt::guardian_policy_prompt_with_config; -#[cfg(test)] use prompt::guardian_truncate_text; #[cfg(test)] use prompt::parse_guardian_assessment; diff --git a/codex-rs/core/src/guardian/policy_template.md b/codex-rs/core/src/guardian/policy_template.md index e6ddc7f5a0..f42a491f84 100644 --- a/codex-rs/core/src/guardian/policy_template.md +++ b/codex-rs/core/src/guardian/policy_template.md @@ -36,7 +36,7 @@ Your primary objective is to determine whether the action poses a risk of irreve - Do not assign `high` or `critical` to a user-requested deletion of a specific local path solely because it uses `rm -rf`. If a read-only check shows the target is missing, empty, or narrowly scoped, such as a single small file or empty directory, this is usually `low` or `medium`. # Policy Configuration -{tenant_policy_config} +{{ tenant_policy_config }} # Investigation Guidelines - When risk depends on local state, use available tools to gather evidence before deciding. Prefer read-only checks first. diff --git a/codex-rs/core/src/guardian/prompt.rs b/codex-rs/core/src/guardian/prompt.rs index 1031464ca2..d32be1aab3 100644 --- a/codex-rs/core/src/guardian/prompt.rs +++ b/codex-rs/core/src/guardian/prompt.rs @@ -683,21 +683,27 @@ For anything else, use this JSON schema: }"# } +pub(super) const BUNDLED_GUARDIAN_POLICY: &str = include_str!("policy.md"); +pub(super) const BUNDLED_GUARDIAN_POLICY_TEMPLATE: &str = include_str!("policy_template.md"); +const TENANT_POLICY_CONFIG_PLACEHOLDER: &str = "{{ tenant_policy_config }}"; + /// Guardian policy prompt. /// -/// Keep the prompt in a dedicated markdown file so reviewers can audit prompt -/// changes directly without diffing through code. The output contract is -/// appended from code so it stays near `guardian_output_schema()`. +/// Keep the bundled fallback in a dedicated markdown file so reviewers can +/// audit prompt changes directly without diffing through code. The output +/// contract is appended from code so it stays near `guardian_output_schema()`. /// /// The template is intentionally separated from the default tenant policy /// configuration so workspace-managed overrides can keep the configurable /// section narrower than the full policy. -pub(crate) fn guardian_policy_prompt() -> String { - guardian_policy_prompt_with_config(include_str!("policy.md")) -} - -pub(crate) fn guardian_policy_prompt_with_config(tenant_policy_config: &str) -> String { - let template = include_str!("policy_template.md").trim_end(); - let prompt = template.replace("{tenant_policy_config}", tenant_policy_config.trim()); +pub(super) fn guardian_policy_prompt_with_config_and_template( + tenant_policy_config: &str, + policy_template: &str, +) -> String { + let template = policy_template.trim_end(); + let prompt = template.replace( + TENANT_POLICY_CONFIG_PLACEHOLDER, + tenant_policy_config.trim(), + ); format!("{prompt}\n\n{}\n", guardian_output_contract_prompt()) } diff --git a/codex-rs/core/src/guardian/review_session.rs b/codex-rs/core/src/guardian/review_session.rs index b1bb26b947..565b09e8dc 100644 --- a/codex-rs/core/src/guardian/review_session.rs +++ b/codex-rs/core/src/guardian/review_session.rs @@ -53,11 +53,12 @@ use codex_utils_absolute_path::AbsolutePathBuf; use super::GUARDIAN_REVIEWER_NAME; use super::GuardianApprovalRequest; +use super::prompt::BUNDLED_GUARDIAN_POLICY; +use super::prompt::BUNDLED_GUARDIAN_POLICY_TEMPLATE; use super::prompt::GuardianPromptMode; use super::prompt::GuardianTranscriptCursor; use super::prompt::build_guardian_prompt_items_with_parent_turn; -use super::prompt::guardian_policy_prompt; -use super::prompt::guardian_policy_prompt_with_config; +use super::prompt::guardian_policy_prompt_with_config_and_template; use super::review::guardian_review_session_config; const GUARDIAN_INTERRUPT_DRAIN_TIMEOUT: Duration = Duration::from_secs(5); @@ -1002,17 +1003,19 @@ pub(crate) fn build_guardian_review_session_config( guardian_config.include_skill_instructions = false; guardian_config.memories.use_memories = false; guardian_config.memories.dedicated_tools = false; - let catalog_policy = model_messages - .and_then(|messages| messages.auto_review.as_ref()) - .and_then(|messages| messages.policy.as_deref()); - guardian_config.base_instructions = Some( - parent_config - .guardian_policy_config - .as_deref() - .or(catalog_policy) - .map(guardian_policy_prompt_with_config) - .unwrap_or_else(guardian_policy_prompt), - ); + let catalog_auto_review = model_messages.and_then(|messages| messages.auto_review.as_ref()); + let tenant_policy_config = parent_config + .guardian_policy_config + .as_deref() + .or_else(|| catalog_auto_review.and_then(|messages| messages.policy.as_deref())) + .unwrap_or(BUNDLED_GUARDIAN_POLICY); + 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.notify = None; guardian_config.developer_instructions = None; guardian_config.permissions.approval_policy = Constrained::allow_only(AskForApproval::Never); @@ -1403,9 +1406,10 @@ mod tests { } #[tokio::test] - async fn guardian_review_session_config_prefers_managed_policy_over_catalog_policy() { + async fn guardian_review_session_config_prefers_managed_policy_and_uses_catalog_template() { let mut parent_config = crate::config::test_config().await; let managed_policy = "Use the managed Guardian policy."; + let catalog_template = "Catalog Guardian template:\n{{ tenant_policy_config }}"; parent_config.guardian_policy_config = Some(managed_policy.to_string()); let model_messages = ModelMessages { instructions_template: None, @@ -1413,6 +1417,7 @@ mod tests { approvals: None, auto_review: Some(AutoReviewMessages { policy: Some("Use the catalog Guardian policy.".to_string()), + policy_template: Some(catalog_template.to_string()), }), permissions: None, }; @@ -1428,7 +1433,10 @@ mod tests { assert_eq!( guardian_config.base_instructions, - Some(guardian_policy_prompt_with_config(managed_policy)) + Some(guardian_policy_prompt_with_config_and_template( + managed_policy, + catalog_template, + )) ); } @@ -1441,6 +1449,7 @@ mod tests { approvals: None, auto_review: Some(AutoReviewMessages { policy: Some(String::new()), + policy_template: None, }), permissions: None, }; @@ -1456,11 +1465,57 @@ mod tests { assert_eq!( guardian_config.base_instructions, - Some(guardian_policy_prompt_with_config("")) + Some(guardian_policy_prompt_with_config_and_template( + "", + BUNDLED_GUARDIAN_POLICY_TEMPLATE, + )) ); assert_ne!( guardian_config.base_instructions, - Some(guardian_policy_prompt()) + Some(guardian_policy_prompt_with_config_and_template( + BUNDLED_GUARDIAN_POLICY, + BUNDLED_GUARDIAN_POLICY_TEMPLATE, + )) + ); + } + + #[tokio::test] + async fn guardian_review_session_config_preserves_explicit_empty_catalog_template() { + let parent_config = crate::config::test_config().await; + let catalog_policy = "Use the catalog Guardian policy."; + let model_messages = ModelMessages { + instructions_template: None, + instructions_variables: None, + approvals: None, + auto_review: Some(AutoReviewMessages { + policy: Some(catalog_policy.to_string()), + policy_template: Some(String::new()), + }), + permissions: None, + }; + + let guardian_config = build_guardian_review_session_config( + &parent_config, + /*live_network_config*/ None, + "active-model", + /*reasoning_effort*/ None, + Some(&model_messages), + ) + .expect("guardian config"); + + assert_eq!( + guardian_config.base_instructions, + Some(guardian_policy_prompt_with_config_and_template( + catalog_policy, + "", + )) + ); + assert_ne!( + guardian_config.base_instructions, + Some(guardian_policy_prompt_with_config_and_template( + catalog_policy, + BUNDLED_GUARDIAN_POLICY_TEMPLATE, + )) ); } diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index 4524ab42b8..34ea6acad9 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -6,6 +6,9 @@ use crate::config::ManagedFeatures; use crate::config::NetworkProxySpec; use crate::config::test_config; use crate::guardian::approval_request::guardian_request_target_item_id; +use crate::guardian::prompt::BUNDLED_GUARDIAN_POLICY; +use crate::guardian::prompt::BUNDLED_GUARDIAN_POLICY_TEMPLATE; +use crate::guardian::prompt::guardian_policy_prompt_with_config_and_template; use crate::session::session::Session; use crate::session::turn_context::TurnContext; use crate::test_support; @@ -2949,7 +2952,10 @@ async fn guardian_review_session_config_clears_parent_developer_instructions() { assert_eq!(guardian_config.developer_instructions, None); assert_eq!( guardian_config.base_instructions, - Some(guardian_policy_prompt()) + Some(guardian_policy_prompt_with_config_and_template( + BUNDLED_GUARDIAN_POLICY, + BUNDLED_GUARDIAN_POLICY_TEMPLATE, + )) ); } @@ -3175,8 +3181,9 @@ async fn guardian_review_session_config_uses_requirements_guardian_policy_config assert_eq!(guardian_config.developer_instructions, None); assert_eq!( guardian_config.base_instructions, - Some(guardian_policy_prompt_with_config( - "Use the workspace-managed guardian policy." + Some(guardian_policy_prompt_with_config_and_template( + "Use the workspace-managed guardian policy.", + BUNDLED_GUARDIAN_POLICY_TEMPLATE, )) ); } @@ -3214,6 +3221,9 @@ async fn guardian_review_session_config_uses_default_guardian_policy_without_req assert_eq!(guardian_config.developer_instructions, None); assert_eq!( guardian_config.base_instructions, - Some(guardian_policy_prompt()) + Some(guardian_policy_prompt_with_config_and_template( + BUNDLED_GUARDIAN_POLICY, + BUNDLED_GUARDIAN_POLICY_TEMPLATE, + )) ); } diff --git a/codex-rs/core/src/session/tests/guardian_tests.rs b/codex-rs/core/src/session/tests/guardian_tests.rs index 2532efc0d3..a2af3a2717 100644 --- a/codex-rs/core/src/session/tests/guardian_tests.rs +++ b/codex-rs/core/src/session/tests/guardian_tests.rs @@ -528,7 +528,7 @@ async fn guardian_allows_unified_exec_additional_permissions_requests_past_polic #[tokio::test] async fn process_compacted_history_preserves_separate_guardian_developer_message() { let (session, mut turn_context) = make_session_and_context().await; - let guardian_policy = crate::guardian::guardian_policy_prompt(); + let guardian_policy = "guardian policy".to_string(); let guardian_source = SessionSource::SubAgent(SubAgentSource::Other(GUARDIAN_REVIEWER_NAME.to_string())); diff --git a/codex-rs/core/tests/suite/guardian_review.rs b/codex-rs/core/tests/suite/guardian_review.rs index b8d79a2cf0..e96a374e27 100644 --- a/codex-rs/core/tests/suite/guardian_review.rs +++ b/codex-rs/core/tests/suite/guardian_review.rs @@ -38,7 +38,7 @@ use tempfile::TempDir; async fn guardian_session_prewarms_and_is_reused_for_first_review() -> Result<()> { skip_if_no_network!(Ok(())); - let catalog_policy = "Use the catalog-provided Guardian policy."; + let catalog_template = "Catalog-provided Guardian template:\n{{ tenant_policy_config }}"; let mut review_model = codex_models_manager::bundled_models_response()? .models .into_iter() @@ -49,7 +49,8 @@ async fn guardian_session_prewarms_and_is_reused_for_first_review() -> Result<() .as_mut() .expect("auto-review model messages"); model_messages.auto_review = Some(AutoReviewMessages { - policy: Some(catalog_policy.to_string()), + policy: None, + policy_template: Some(catalog_template.to_string()), }); let tool_args = json!({ @@ -96,11 +97,13 @@ async fn guardian_session_prewarms_and_is_reused_for_first_review() -> Result<() }) .expect("guardian startup prewarm request"); assert_eq!(guardian_prewarm["generate"].as_bool(), Some(false)); - assert!( - guardian_prewarm["instructions"] - .as_str() - .is_some_and(|instructions| instructions.contains(catalog_policy)) - ); + let guardian_instructions = guardian_prewarm["instructions"] + .as_str() + .expect("guardian instructions"); + assert!(guardian_instructions.contains("Catalog-provided Guardian template:")); + assert!(guardian_instructions.contains("- Organization: default generic tenant.")); + assert!(!guardian_instructions.contains("{{ tenant_policy_config }}")); + assert!(guardian_instructions.contains("final message must be strict JSON")); let guardian_thread_id = guardian_prewarm["client_metadata"]["thread_id"] .as_str() .expect("guardian thread id"); diff --git a/codex-rs/models-manager/src/model_info_tests.rs b/codex-rs/models-manager/src/model_info_tests.rs index ef410a1e58..838cdcefc0 100644 --- a/codex-rs/models-manager/src/model_info_tests.rs +++ b/codex-rs/models-manager/src/model_info_tests.rs @@ -89,6 +89,7 @@ fn base_instruction_override_preserves_catalog_auto_review_messages() { let mut model = model_info_from_slug("unknown-model"); let auto_review = AutoReviewMessages { policy: Some("review policy".to_string()), + policy_template: Some("review policy template".to_string()), }; model.model_messages = Some(ModelMessages { instructions_template: Some("template".to_string()), diff --git a/codex-rs/protocol/src/openai_models.rs b/codex-rs/protocol/src/openai_models.rs index 921a1c458b..9734c77d22 100644 --- a/codex-rs/protocol/src/openai_models.rs +++ b/codex-rs/protocol/src/openai_models.rs @@ -518,6 +518,7 @@ pub struct ApprovalMessages { #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)] pub struct AutoReviewMessages { pub policy: Option, + pub policy_template: Option, } #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)] @@ -777,6 +778,46 @@ mod tests { ); } + #[test] + fn auto_review_messages_preserve_missing_and_empty_template_values() { + let missing_template: ModelMessages = from_str( + r#"{ + "instructions_template": null, + "instructions_variables": null, + "auto_review": { + "policy": "policy" + } + }"#, + ) + .expect("auto-review messages should deserialize without a policy template"); + let empty_template: ModelMessages = from_str( + r#"{ + "instructions_template": null, + "instructions_variables": null, + "auto_review": { + "policy": "policy", + "policy_template": "" + } + }"#, + ) + .expect("auto-review messages should deserialize with an empty policy template"); + + assert_eq!( + missing_template.auto_review, + Some(AutoReviewMessages { + policy: Some("policy".to_string()), + policy_template: None, + }) + ); + assert_eq!( + empty_template.auto_review, + Some(AutoReviewMessages { + policy: Some("policy".to_string()), + policy_template: Some(String::new()), + }) + ); + } + #[test] fn permission_messages_preserve_missing_and_empty_values() { let messages: ModelMessages = from_str(