mirror of
https://github.com/openai/codex.git
synced 2026-11-05 16:29:31 +00:00
Make Guardian risk score persistence opt-in (#40911)
## What changed - Add the `features.guardianv2.persist_scores` configuration option for debugging. - Default the option to `false`, so Guardian V2 reviewed actions and risk scores are only written to rollout files when explicitly enabled and the session is not ephemeral. ## Testing - Cover configuration deserialization, default non-persistence, and opt-in persistence for nested code-mode actions. GitOrigin-RevId: 452af99ef14553d45140340ececc2913389124d0
This commit is contained in:
1 parent
bde9db1375
commit
5b92c2d2f5
6 files changed
+24
-22
No files matched your search
@@ -1441,6 +1441,10 @@
|
|||||||
"minimum": 0.0,
|
"minimum": 0.0,
|
||||||
"type": "integer"
|
"type": "integer"
|
||||||
},
|
},
|
||||||
|
"persist_scores": {
|
||||||
|
"description": "Persist reviewed actions and risk scores to rollout files for debugging.",
|
||||||
|
"type": "boolean"
|
||||||
|
},
|
||||||
"reasoning_effort": {
|
"reasoning_effort": {
|
||||||
"$ref": "#/definitions/ReasoningEffort"
|
"$ref": "#/definitions/ReasoningEffort"
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -33,6 +33,7 @@ pub(crate) enum GuardianV2ReviewScope {
|
|||||||
#[derive(Clone, Debug, PartialEq)]
|
#[derive(Clone, Debug, PartialEq)]
|
||||||
pub(crate) struct GuardianV2Config {
|
pub(crate) struct GuardianV2Config {
|
||||||
local_overrides: GuardianV2ConfigToml,
|
local_overrides: GuardianV2ConfigToml,
|
||||||
|
pub(crate) persist_scores: bool,
|
||||||
pub(crate) classifier_instructions: String,
|
pub(crate) classifier_instructions: String,
|
||||||
pub(crate) review_threshold: f64,
|
pub(crate) review_threshold: f64,
|
||||||
pub(crate) max_tool_call_lag: usize,
|
pub(crate) max_tool_call_lag: usize,
|
||||||
@@ -229,6 +230,7 @@ impl GuardianV2Config {
|
|||||||
|
|
||||||
Ok(Self {
|
Ok(Self {
|
||||||
local_overrides: configured.clone(),
|
local_overrides: configured.clone(),
|
||||||
|
persist_scores: configured.persist_scores.unwrap_or(false),
|
||||||
classifier_instructions: configured
|
classifier_instructions: configured
|
||||||
.classifier_instructions
|
.classifier_instructions
|
||||||
.unwrap_or_else(|| DEFAULT_CLASSIFIER_INSTRUCTIONS.to_owned()),
|
.unwrap_or_else(|| DEFAULT_CLASSIFIER_INSTRUCTIONS.to_owned()),
|
||||||
|
|||||||
@@ -804,7 +804,8 @@ impl GuardianV2Extension {
|
|||||||
.latest_scored_tool_call
|
.latest_scored_tool_call
|
||||||
.fetch_max(tool_call_index, Ordering::Release);
|
.fetch_max(tool_call_index, Ordering::Release);
|
||||||
classification_finished_at = Some(Instant::now());
|
classification_finished_at = Some(Instant::now());
|
||||||
if !config.ephemeral
|
if guardian_config.persist_scores
|
||||||
|
&& !config.ephemeral
|
||||||
&& let Err(error) = thread
|
&& let Err(error) = thread
|
||||||
.append_rollout_items(&[RolloutItem::SecurityRiskScore(score)])
|
.append_rollout_items(&[RolloutItem::SecurityRiskScore(score)])
|
||||||
.await
|
.await
|
||||||
|
|||||||
@@ -1770,26 +1770,16 @@ async fn contributor_samples_tool_calls_with_the_existing_luna_pool() -> Result<
|
|||||||
);
|
);
|
||||||
assert!(score.sampled_at.is_some());
|
assert!(score.sampled_at.is_some());
|
||||||
test.codex.ensure_rollout_materialized().await;
|
test.codex.ensure_rollout_materialized().await;
|
||||||
let persisted_score = tokio::time::timeout(Duration::from_secs(5), async {
|
assert!(
|
||||||
loop {
|
!test
|
||||||
if let Some(persisted_score) = test
|
.codex
|
||||||
.codex
|
.load_history(/*include_archived*/ false)
|
||||||
.load_history(/*include_archived*/ false)
|
.await?
|
||||||
.await?
|
.items
|
||||||
.items
|
.into_iter()
|
||||||
.into_iter()
|
.any(|item| matches!(item, RolloutItem::SecurityRiskScore(_))),
|
||||||
.find_map(|item| match item {
|
"risk scores should not be persisted unless explicitly enabled"
|
||||||
RolloutItem::SecurityRiskScore(score) => Some(score),
|
);
|
||||||
_ => None,
|
|
||||||
})
|
|
||||||
{
|
|
||||||
return Ok::<_, anyhow::Error>(persisted_score);
|
|
||||||
}
|
|
||||||
tokio::task::yield_now().await;
|
|
||||||
}
|
|
||||||
})
|
|
||||||
.await??;
|
|
||||||
assert_eq!(&persisted_score, score.as_ref());
|
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
registry
|
registry
|
||||||
.fast_approval_decision(
|
.fast_approval_decision(
|
||||||
@@ -1867,7 +1857,7 @@ async fn contributor_persists_nested_code_mode_action_with_score() -> Result<()>
|
|||||||
Vec::new(),
|
Vec::new(),
|
||||||
r#"{"path":"README.md"}"#,
|
r#"{"path":"README.md"}"#,
|
||||||
Some(TEST_GUARDIAN_POLICY),
|
Some(TEST_GUARDIAN_POLICY),
|
||||||
"",
|
"[features.guardianv2]\nenabled = true\npersist_scores = true\n",
|
||||||
/*model_defaults*/ None,
|
/*model_defaults*/ None,
|
||||||
ToolCallSource::CodeMode {
|
ToolCallSource::CodeMode {
|
||||||
cell_id: "cell-1".to_owned(),
|
cell_id: "cell-1".to_owned(),
|
||||||
|
|||||||
@@ -129,6 +129,9 @@ pub struct GuardianV2ConfigToml {
|
|||||||
/// Route Guardian review and classification through the unmetered Codex endpoints.
|
/// Route Guardian review and classification through the unmetered Codex endpoints.
|
||||||
#[serde(skip_serializing_if = "Option::is_none")]
|
#[serde(skip_serializing_if = "Option::is_none")]
|
||||||
pub free_guardian: Option<bool>,
|
pub free_guardian: Option<bool>,
|
||||||
|
/// Persist reviewed actions and risk scores to rollout files for debugging.
|
||||||
|
#[serde(skip_serializing_if = "Option::is_none")]
|
||||||
|
pub persist_scores: Option<bool>,
|
||||||
#[serde(skip_serializing_if = "Option::is_none")]
|
#[serde(skip_serializing_if = "Option::is_none")]
|
||||||
pub classifier_instructions: Option<String>,
|
pub classifier_instructions: Option<String>,
|
||||||
#[serde(skip_serializing_if = "Option::is_none")]
|
#[serde(skip_serializing_if = "Option::is_none")]
|
||||||
|
|||||||
@@ -164,6 +164,7 @@ fn guardian_v2_feature_config_deserializes_classifier_and_transcript_settings()
|
|||||||
[guardianv2]
|
[guardianv2]
|
||||||
enabled = true
|
enabled = true
|
||||||
free_guardian = true
|
free_guardian = true
|
||||||
|
persist_scores = true
|
||||||
classifier_instructions = "Review this action"
|
classifier_instructions = "Review this action"
|
||||||
review_threshold = 0.65
|
review_threshold = 0.65
|
||||||
max_tool_call_lag = 2
|
max_tool_call_lag = 2
|
||||||
@@ -194,6 +195,7 @@ max_recent_non_user_entries = 12
|
|||||||
Some(FeatureToml::Config(crate::GuardianV2ConfigToml {
|
Some(FeatureToml::Config(crate::GuardianV2ConfigToml {
|
||||||
enabled: Some(true),
|
enabled: Some(true),
|
||||||
free_guardian: Some(true),
|
free_guardian: Some(true),
|
||||||
|
persist_scores: Some(true),
|
||||||
classifier_instructions: Some("Review this action".to_owned()),
|
classifier_instructions: Some("Review this action".to_owned()),
|
||||||
review_threshold: Some(0.65),
|
review_threshold: Some(0.65),
|
||||||
max_tool_call_lag: Some(2),
|
max_tool_call_lag: Some(2),
|
||||||
|
|||||||
Reference in new issue
Block a user