mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
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
This commit is contained in:
@@ -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;
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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())
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
))
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -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,
|
||||
))
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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()));
|
||||
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -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()),
|
||||
|
||||
@@ -518,6 +518,7 @@ pub struct ApprovalMessages {
|
||||
#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)]
|
||||
pub struct AutoReviewMessages {
|
||||
pub policy: Option<String>,
|
||||
pub policy_template: Option<String>,
|
||||
}
|
||||
|
||||
#[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(
|
||||
|
||||
Reference in New Issue
Block a user