mirror of
https://github.com/openai/codex.git
synced 2026-10-19 16:17:49 +00:00
Default Guardian v2 async classification to conversation mode (#53733)
## What changed Use `conversation` when neither a local `async_classifier_mode` override nor a model default specifies a mode. Keep `snapshot` available as an explicit override and update the configuration documentation and schema to describe the new fallback. ## Testing Expand configuration tests to cover all local override and model default combinations. Update app-server approval tests to exercise conversation mode without an explicit override, and explicitly select snapshot mode in tests that depend on it. GitOrigin-RevId: a62479a3f6454bf28d589373a236d8b0ab5023b9
This commit is contained in:
@@ -919,9 +919,12 @@ async fn guardian_v2_routes_scoped_tool_approvals(
|
||||
} else {
|
||||
"prompt"
|
||||
};
|
||||
let classifier_mode = match classifier_mode {
|
||||
codex_protocol::openai_models::AsyncClassifierMode::Snapshot => "snapshot",
|
||||
codex_protocol::openai_models::AsyncClassifierMode::Conversation => "conversation",
|
||||
// Exercise the default conversation path and the explicit snapshot rollback.
|
||||
let classifier_config = match classifier_mode {
|
||||
codex_protocol::openai_models::AsyncClassifierMode::Snapshot => {
|
||||
"\nasync_classifier_mode = \"snapshot\""
|
||||
}
|
||||
codex_protocol::openai_models::AsyncClassifierMode::Conversation => "",
|
||||
};
|
||||
let mut mock_config = MockResponsesConfig::new(&responses_url)
|
||||
.with_model(MODEL)
|
||||
@@ -932,7 +935,7 @@ async fn guardian_v2_routes_scoped_tool_approvals(
|
||||
analytics_server.uri(),
|
||||
))
|
||||
.with_extra_config(&format!(
|
||||
"[mcp_servers.{server_name}]\nurl = \"{mcp_server_url}/mcp\"\ndefault_tools_approval_mode = \"{tool_approval_mode}\"\n\n[analytics]\nenabled = true\n\n[otel]\nmetrics_exporter = {{ otlp-http = {{ endpoint = \"{responses_url}/metrics\", protocol = \"json\" }} }}\n\n[features.guardianv2]\nenabled = true\nasync_classifier_mode = \"{classifier_mode}\"{guardian_scope_config}"
|
||||
"[mcp_servers.{server_name}]\nurl = \"{mcp_server_url}/mcp\"\ndefault_tools_approval_mode = \"{tool_approval_mode}\"\n\n[analytics]\nenabled = true\n\n[otel]\nmetrics_exporter = {{ otlp-http = {{ endpoint = \"{responses_url}/metrics\", protocol = \"json\" }} }}\n\n[features.guardianv2]\nenabled = true{classifier_config}{guardian_scope_config}"
|
||||
))
|
||||
.enable_feature(Feature::GuardianApproval);
|
||||
if lifecycle.has_user_input() || lifecycle.has_root_user_input() {
|
||||
@@ -1259,7 +1262,8 @@ async fn guardian_v2_routes_scoped_tool_approvals(
|
||||
| ThreadLifecycle::RootUserRestriction
|
||||
| ThreadLifecycle::RootRestrictionDuringClassification
|
||||
) {
|
||||
let retention_allowed = classifier_mode == "conversation"
|
||||
let retention_allowed = classifier_mode
|
||||
== codex_protocol::openai_models::AsyncClassifierMode::Conversation
|
||||
&& !fail_after_classification
|
||||
&& !matches!(risk, GuardianRisk::InvalidResponse)
|
||||
&& !late_root_restriction;
|
||||
@@ -1553,7 +1557,7 @@ async fn guardian_v2_routes_scoped_tool_approvals(
|
||||
.filter_map(|item| item["id"].as_str())
|
||||
.filter(|id| id.starts_with("luna-score-message-"))
|
||||
.collect::<Vec<_>>();
|
||||
if classifier_mode == "conversation"
|
||||
if classifier_mode == codex_protocol::openai_models::AsyncClassifierMode::Conversation
|
||||
&& !fail_after_classification
|
||||
&& let Some(latest) = retained_ids.last()
|
||||
{
|
||||
@@ -1907,7 +1911,7 @@ async fn guardian_v2_routes_scoped_tool_approvals(
|
||||
}
|
||||
let post_authorization_change_sample =
|
||||
wait_for_luna_request(responses_state.as_ref(), /*index*/ 2).await?;
|
||||
if classifier_mode == "conversation"
|
||||
if classifier_mode == codex_protocol::openai_models::AsyncClassifierMode::Conversation
|
||||
&& matches!(lifecycle, ThreadLifecycle::RootUserRestriction)
|
||||
{
|
||||
let input = post_authorization_change_sample["input"]
|
||||
|
||||
@@ -1960,7 +1960,7 @@
|
||||
"$ref": "#/definitions/AsyncClassifierMode"
|
||||
}
|
||||
],
|
||||
"description": "Classifier experiment override. Otherwise use the model default, then snapshots."
|
||||
"description": "Classifier mode override. Otherwise use the model default, then conversation."
|
||||
},
|
||||
"classifier_instructions": {
|
||||
"type": "string"
|
||||
|
||||
@@ -94,7 +94,7 @@ async fn host_owned_orchestrator_connector_reaches_guardian_context() -> Result<
|
||||
.with_pre_build_hook(|home| {
|
||||
std::fs::write(
|
||||
home.join("config.toml"),
|
||||
"[features.guardianv2]\nenabled = true\n\n[features.guardianv2.review_scope]\ncomputer_use_only = false\n\n[apps._default]\ndefault_tools_approval_mode = \"prompt\"\n",
|
||||
"[features.guardianv2]\nenabled = true\nasync_classifier_mode = \"snapshot\"\n\n[features.guardianv2.review_scope]\ncomputer_use_only = false\n\n[apps._default]\ndefault_tools_approval_mode = \"prompt\"\n",
|
||||
)
|
||||
.expect("write Guardian configuration");
|
||||
})
|
||||
|
||||
@@ -340,17 +340,23 @@ fn classifier_mode_uses_local_override_then_model_default() {
|
||||
..Default::default()
|
||||
})
|
||||
.unwrap();
|
||||
let defaults = GuardianV2ModelConfig {
|
||||
async_classifier_mode: Some(AsyncClassifierMode::Conversation),
|
||||
..Default::default()
|
||||
};
|
||||
assert_eq!(
|
||||
config.classifier_mode(Some(&defaults)),
|
||||
local.unwrap_or(AsyncClassifierMode::Conversation)
|
||||
);
|
||||
for model in [
|
||||
None,
|
||||
Some(AsyncClassifierMode::Snapshot),
|
||||
Some(AsyncClassifierMode::Conversation),
|
||||
] {
|
||||
let defaults = GuardianV2ModelConfig {
|
||||
async_classifier_mode: model,
|
||||
..Default::default()
|
||||
};
|
||||
assert_eq!(
|
||||
config.classifier_mode(Some(&defaults)),
|
||||
local.or(model).unwrap_or(AsyncClassifierMode::Conversation)
|
||||
);
|
||||
}
|
||||
assert_eq!(
|
||||
config.classifier_mode(/*model_defaults*/ None),
|
||||
local.unwrap_or_default()
|
||||
local.unwrap_or(AsyncClassifierMode::Conversation)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -17,7 +17,7 @@ async fn decisions_disagreement_failures_and_disabled_flag_preserve_baseline() -
|
||||
] {
|
||||
let server = responses::start_mock_server().await;
|
||||
let fixture = GuardianFailureFixture::with_config(&format!(
|
||||
"[features]\nguardianv2_decisions_comparison = {enabled}\n"
|
||||
"[features]\nguardianv2_decisions_comparison = {enabled}\n[features.guardianv2]\nasync_classifier_mode = \"snapshot\"\n"
|
||||
))
|
||||
.await?;
|
||||
let test = &fixture.test;
|
||||
@@ -181,7 +181,11 @@ async fn decisions_disagreement_failures_and_disabled_flag_preserve_baseline() -
|
||||
#[tokio::test]
|
||||
async fn guardian_budget_rejects_both_backends_before_sending() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
let fixture = GuardianFailureFixture::new().await?;
|
||||
// This test replaces the snapshot sampler to force the Responses budget failure.
|
||||
let fixture = GuardianFailureFixture::with_config(
|
||||
"[features.guardianv2]\nenabled = true\nasync_classifier_mode = \"snapshot\"\n",
|
||||
)
|
||||
.await?;
|
||||
let server = responses::start_mock_server().await;
|
||||
let store = fixture.test.codex.thread_extension_data();
|
||||
let mut config = fixture.test.config.clone();
|
||||
|
||||
@@ -2092,7 +2092,7 @@ async fn contributor_samples_tool_calls_with_the_existing_luna_pool() -> Result<
|
||||
conversation_history,
|
||||
r#"{"path":"README.md"}"#,
|
||||
Some(TEST_GUARDIAN_POLICY),
|
||||
"[features.guardianv2.transcript]\nmax_message_entry_tokens = 100\nmax_message_transcript_tokens = 100\n",
|
||||
"[features.guardianv2]\nasync_classifier_mode = \"snapshot\"\n[features.guardianv2.transcript]\nmax_message_entry_tokens = 100\nmax_message_transcript_tokens = 100\n",
|
||||
/*model_defaults*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -152,7 +152,7 @@ pub struct GuardianV2ConfigToml {
|
||||
/// Transcript encoding for synchronous Guardian review and asynchronous scoring. Defaults to line.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub transcript_mode: Option<codex_protocol::TranscriptFormat>,
|
||||
/// Classifier experiment override. Otherwise use the model default, then snapshots.
|
||||
/// Classifier mode override. Otherwise use the model default, then conversation.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub async_classifier_mode: Option<codex_protocol::openai_models::AsyncClassifierMode>,
|
||||
/// Reset retained async history when the next request exceeds this token estimate.
|
||||
|
||||
@@ -10,8 +10,8 @@ use super::ReasoningEffort;
|
||||
#[serde(rename_all = "snake_case")]
|
||||
#[ts(rename_all = "snake_case")]
|
||||
pub enum AsyncClassifierMode {
|
||||
#[default]
|
||||
Snapshot,
|
||||
#[default]
|
||||
Conversation,
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user