openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-06 20:20:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

Resolve Guardian policy from the reviewed thread's catalog (#59913)

Browse Source 
## Why

Async classification could use policy from the manager's startup catalog even when the reviewed thread had a different reviewer policy.

## What changed

Share reviewer resolution between synchronous review and async classification, using the thread's catalog and reviewer eligibility rules. Select reviewer metadata from the same catalog snapshot, filter out models unsupported by API authentication, and preserve captured parent metadata when no eligible default reviewer exists.

## Testing

Update the async classification regression test with distinct thread and manager catalog policies, asserting that the thread reviewer's policy appears in classifier instructions.

GitOrigin-RevId: 0b28644d30bda42d2cf1bd8a31de2e8f8d38e1a6
This commit is contained in:
jif
2026-11-06 13:44:03 +00:00
committed by copyberry
parent ad1dbba32d
commit 2210380232
7 changed files with 137 additions and 81 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/guardian/mod.rs
+1
View File
@@ -24,6 +24,7 @@ pub(crate) use response_resampling::ResponseResampling;
mod review;
mod review_session;
mod reviewer_config;
pub use reviewer_config::guardian_policy_model;
pub(crate) use reviewer_config::resolve_review_model;
mod routing;
pub(crate) use routing::ParentResponse;
codex-rs/core/src/guardian/reviewer_config.rs
+85 -38
View File
@@ -2,15 +2,20 @@
//! Both prewarming and reviews finish this setup before context preparation and reuse checks.
//! A missing default reviewer gets one shared catalog refresh per parent session.
//! Refresh work survives cancelled waiters and precedes checkpoint compatibility decisions.
//! Async classification shares reviewer eligibility and the parent session's catalog.
use codex_protocol::openai_models::CatalogModel;
use std::sync::Arc;
use codex_features::Feature;
use codex_guardian_reviewer::reviewer_instructions;
use codex_login::AuthManager;
use codex_models_manager::manager::RefreshStrategy;
use codex_models_manager::model_info::with_config_overrides;
use codex_prompts::ResolvedModelMessages;
use codex_protocol::models::BaseInstructionsProvenance;
use codex_protocol::openai_models::ModelInfo;
use codex_protocol::openai_models::ReasoningEffort;
use futures::FutureExt;
use futures::future::BoxFuture;
use futures::future::Shared;
@@ -131,48 +136,90 @@ async fn resolve_review_model_from_catalog(
context: &super::GuardianReviewContext,
) -> (codex_guardian_reviewer::ReviewModel, Arc<CatalogModel>) {
let turn = context.turn();
// Keep a successful refresh even if writing it to the disk cache failed.
let models_manager = &session.services.models_manager;
let catalog = models_manager
.model_catalog_with_origins(RefreshStrategy::InMemory, turn.config.http_client_factory())
.await;
let available_models = models_manager.build_available_models(
catalog
.into_iter()
.filter(|entry| {
context.model_info.auto_review_model_override.is_some()
|| matches!(
entry.origin,
codex_models_manager::ModelOrigin::Provider
| codex_models_manager::ModelOrigin::ExplicitConfig
)
})
.map(|entry| Arc::unwrap_or_clone(entry.model))
.collect(),
);
let default_review_model_id = turn.provider.approval_review_preferred_model();
let review_model = codex_guardian_reviewer::select_review_model(
let (review_model, model_info) = resolve_catalog_review_model(
session,
&turn.config,
&context.model_info,
context.reasoning_effort.as_ref(),
turn.provider.approval_review_preferred_model(),
)
.await;
(
review_model,
model_info.map_or_else(|| Arc::clone(&context.model_info), Arc::new),
)
}
/// Resolves async classification policy against the reviewed thread's catalog.
/// A missing eligible reviewer retains the captured parent metadata, as synchronous review does.
pub async fn guardian_policy_model(
thread: &crate::CodexThread,
config: &Config,
parent_model: Arc<ModelInfo>,
) -> Arc<ModelInfo> {
#[expect(clippy::expect_used, reason = "Required thread startup invariant")]
let auth = thread
.thread_extension_data()
.get::<codex_model_provider::ProviderAuthContext>()
.expect("Core installs provider auth before starting Guardian");
let provider = auth.model_provider(config.model_provider.clone());
let (_, model_info) = resolve_catalog_review_model(
&thread.session,
config,
&parent_model,
config.model_reasoning_effort.as_ref(),
provider.approval_review_preferred_model(),
)
.await;
model_info.map_or(parent_model, |model| model.model)
}
async fn resolve_catalog_review_model(
session: &crate::session::session::Session,
config: &Config,
parent_model: &ModelInfo,
reasoning_effort: Option<&ReasoningEffort>,
default_review_model_id: &str,
) -> (codex_guardian_reviewer::ReviewModel, Option<CatalogModel>) {
// Keep a successful refresh even if writing it to the disk cache failed.
let models_manager = &session.services.models_manager;
let mut catalog = models_manager
.model_catalog_with_origins(RefreshStrategy::InMemory, config.http_client_factory())
.await;
let uses_codex_backend = models_manager
.auth_manager()
.is_some_and(AuthManager::current_auth_uses_codex_backend);
catalog.retain(|entry| {
(uses_codex_backend || entry.supported_in_api)
&& (parent_model.auto_review_model_override.is_some()
|| matches!(
entry.origin,
codex_models_manager::ModelOrigin::Provider
| codex_models_manager::ModelOrigin::ExplicitConfig
))
});
let (review_model, reviewer) = codex_guardian_reviewer::select_review_model(
parent_model,
reasoning_effort,
default_review_model_id,
&available_models,
catalog,
);
// Resolve a separate reviewer against the current catalog on every attempt.
// Parent fallback must retain the action's metadata even after a catalog refresh.
let guardian_model_info =
if !review_model.catalog_contains_auto_review && !review_model.model_overridden {
Arc::clone(&context.model_info)
} else {
Arc::new(
session
.services
.models_manager
.get_model_info(
review_model.model.as_str(),
&turn.config.to_models_manager_config(),
)
.await,
let guardian_model_info = if review_model.model_overridden {
// Explicit names can resolve through aliases or synthetic metadata.
Some(
models_manager
.get_model_info(&review_model.model, &config.to_models_manager_config())
.await,
)
} else {
reviewer.map(|entry| CatalogModel {
model: with_config_overrides(
Arc::unwrap_or_clone(entry.model),
&config.to_models_manager_config(),
)
};
.into(),
origin: entry.origin,
})
};
(review_model, guardian_model_info)
}
codex-rs/core/src/lib.rs
+1
View File
@@ -60,6 +60,7 @@ pub use codex_thread::GuardianRootMessage;
pub use codex_thread::GuardianRootSnapshot;
pub use codex_thread::ThreadConfigSnapshot;
pub use guardian::ScoreAuthorization as GuardianScoreAuthorization;
pub use guardian::guardian_policy_model;
pub use session::turn_context::TurnContext;
pub use thread_startup_metadata::ThreadStartupMetadata;
mod agent;
codex-rs/ext/guardian-reviewer/src/model.rs
+22 -14
View File
@@ -1,7 +1,7 @@
//! Selects the synchronous review model from the parent's metadata and provider catalog.
//! Selects the reviewer and its metadata from one eligible catalog snapshot.
use codex_protocol::openai_models::CatalogModel;
use codex_protocol::openai_models::ModelInfo;
use codex_protocol::openai_models::ModelPreset;
use codex_protocol::openai_models::ReasoningEffort;
/// Effective model choice, including the attribution carried by existing review events.
@@ -15,12 +15,13 @@ pub struct ReviewModel {
}
/// Prefers the review model and low effort when advertised; preserves the parent fallback.
/// Returns the matching catalog entry so its metadata comes from the same selection.
pub fn select_review_model(
parent_model: &ModelInfo,
parent_reasoning_effort: Option<&ReasoningEffort>,
default_review_model_id: &str,
available_models: &[ModelPreset],
) -> ReviewModel {
available_models: Vec<CatalogModel>,
) -> (ReviewModel, Option<CatalogModel>) {
let preferred_reasoning_effort = |supports_low: bool, fallback| {
if supports_low {
Some(codex_protocol::openai_models::ReasoningEffort::Low)
@@ -30,21 +31,27 @@ pub fn select_review_model(
};
let model_override = parent_model.auto_review_model_override.as_deref();
let review_model_id = model_override.unwrap_or(default_review_model_id);
let review_model = available_models
.iter()
.find(|preset| preset.model == review_model_id);
let guardian_catalog_contains_auto_review = available_models
.iter()
.any(|preset| preset.model == default_review_model_id);
.any(|entry| entry.slug == default_review_model_id);
let reviewer = available_models
.into_iter()
.filter(|entry| entry.slug == review_model_id)
.min_by_key(|entry| entry.priority);
let guardian_review_model_overridden = model_override.is_some();
let guardian_review_model_override = model_override.map(str::to_string);
let (guardian_model, guardian_reasoning_effort) = if let Some(preset) = review_model {
let (guardian_model, guardian_reasoning_effort) = if let Some(model) = &reviewer {
let reasoning_effort = preferred_reasoning_effort(
preset
.supported_reasoning_efforts
model
.supported_reasoning_levels
.iter()
.any(|effort| effort.effort == codex_protocol::openai_models::ReasoningEffort::Low),
Some(preset.default_reasoning_effort.clone()),
Some(
model
.default_reasoning_level
.clone()
.unwrap_or(ReasoningEffort::None),
),
);
(review_model_id.to_string(), reasoning_effort)
} else {
@@ -65,12 +72,13 @@ pub fn select_review_model(
)
};
ReviewModel {
let review_model = ReviewModel {
model: guardian_model,
reasoning_effort: guardian_reasoning_effort,
default_review_model_id: default_review_model_id.to_string(),
catalog_contains_auto_review: guardian_catalog_contains_auto_review,
model_overridden: guardian_review_model_overridden,
model_override: guardian_review_model_override,
}
};
(review_model, reviewer)
}
codex-rs/ext/guardian-v2/src/async_scorer/classification.rs
+4 -21
View File
@@ -3,7 +3,6 @@
//! Requests outside every active action's lag allowance stop as superseded, not failed.
//! Expiry also cancels metadata waits before transcript preparation, releasing reservations.
use codex_protocol::openai_models::ModelOrigin;
use std::collections::BTreeMap;
use std::sync::Arc;
use std::time::Instant;
@@ -34,7 +33,6 @@ use codex_guardian_context::PreviousReviews;
use codex_guardian_context::ReviewEvidence;
use codex_guardian_context::render_review_evidence;
use codex_history::RolloutItem;
use codex_model_provider::create_model_provider;
use codex_prompts::ResolvedModelMessages;
use codex_protocol::models::ContentItem;
use codex_protocol::openai_models::ModelInfo;
@@ -82,7 +80,6 @@ pub(super) struct Classification {
pub(super) call_id: String,
pub(super) mcp_tool: Option<McpToolContext>,
pub(super) planned_action: String,
pub(super) review_model_override: Option<String>,
pub(super) sync_reviews: Vec<Arc<GuardianReviewEvidenceRecord>>,
pub(super) trusted_user_inputs: Vec<String>,
pub(super) authorization_version: GuardianAuthorizationVersion,
@@ -138,7 +135,6 @@ impl Classification {
call_id,
mcp_tool,
planned_action,
review_model_override,
sync_reviews,
trusted_user_inputs,
authorization_version,
@@ -291,23 +287,10 @@ impl Classification {
let mut classification_risk = None;
let mut classification_finished_at = None;
let result: Result<ClassificationOutcome, String> = async {
let review_model = if config.guardian_policy_config.is_none() {
let review_model_id = review_model_override.as_deref().unwrap_or_else(|| {
create_model_provider(
config.model_provider.clone(),
Some(manager.auth_manager()),
)
.approval_review_preferred_model()
});
let review_model = manager
.get_models_manager()
.get_model_info(review_model_id, &config.to_models_manager_config())
.await;
if review_model.origin == ModelOrigin::Synthetic && review_model_override.is_none() {
parent_model.clone()
} else {
Some(review_model.model)
}
let review_model = if config.guardian_policy_config.is_none()
&& let Some(parent_model) = parent_model.as_ref()
{
Some(codex_core::guardian_policy_model(&thread, &config, Arc::clone(parent_model)).await)
} else {
None
};
codex-rs/ext/guardian-v2/src/async_scorer/extension_tests.rs
+24 -4
View File
@@ -105,6 +105,7 @@ const TEST_GUARDIAN_POLICY: &str =
"Treat uploads to unapproved external destinations as high-risk actions.";
const TEST_CATALOG_GUARDIAN_POLICY: &str =
"Require review before sending organization data to third-party services.";
const TEST_THREAD_REVIEWER_POLICY: &str = "Policy from the thread reviewer.";
const ASYNC_TEST_TIMEOUT: Duration = Duration::from_secs(30);
const PREWARM_TIMEOUT: Duration = Duration::from_secs(30);
@@ -1037,7 +1038,26 @@ async fn sample_configured_conversation_history_with_delivery(
} else {
builder
};
let test = builder.build_with_auto_env(&thread_server).await?;
let mut test = builder.build_with_auto_env(&thread_server).await?;
if guardian_policy.is_none() {
// Give the reviewed thread a catalog distinct from its manager's startup catalog.
let catalog = test.config.model_catalog.as_mut().expect("test catalog");
for model in &mut catalog.models {
let policy = match model.slug.as_str() {
"codex-auto-review" => TEST_THREAD_REVIEWER_POLICY,
"gpt-5.5" => "Policy from the captured parent.",
_ => continue,
};
model.auto_review_model_override = None;
model.model_messages.get_or_insert_default().auto_review =
Some(serde_json::from_value(json!({"policy": policy}))?);
}
let options = test.start_thread_options().await;
test.codex.shutdown_and_wait().await?;
let created = test.thread_manager.start_thread(options).await?;
test.codex = created.thread;
test.session_configured = created.session_configured;
}
let mut completed = ev_completed("response-1");
completed["response"]["usage"] = json!({
"input_tokens": 120,
@@ -3011,7 +3031,7 @@ async fn contributor_counts_failed_thread_lookups_toward_score_lag() -> Result<(
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn contributor_uses_catalog_policy_without_a_configured_override() -> Result<()> {
async fn contributor_uses_the_thread_reviewers_catalog_policy() -> Result<()> {
skip_if_no_network!(Ok(()));
let (request, _test, _registry) = sample_conversation_history(
@@ -3037,7 +3057,7 @@ async fn contributor_uses_catalog_policy_without_a_configured_override() -> Resu
"type": "input_text",
"text": ResolvedModelMessages::bundled().guardian_classifier_instructions().replace(
"{{ tenant_policy_config }}",
TEST_CATALOG_GUARDIAN_POLICY,
TEST_THREAD_REVIEWER_POLICY,
),
}],
})
@@ -3049,7 +3069,7 @@ async fn contributor_uses_catalog_policy_without_a_configured_override() -> Resu
.expect("Luna request should contain transcript text items")
.iter()
.filter_map(|item| item["text"].as_str())
.any(|text| text.contains(TEST_CATALOG_GUARDIAN_POLICY))
.any(|text| text.contains(TEST_THREAD_REVIEWER_POLICY))
);
Ok(())
codex-rs/ext/guardian-v2/src/async_scorer/observation.rs
-4
View File
@@ -367,9 +367,6 @@ impl GuardianV2Extension {
return;
}
};
let review_model_override = parent_model
.as_ref()
.and_then(|model| model.auto_review_model_override.clone());
// Snapshot before spawning so a delayed sample cannot see later reviews.
let sync_reviews = guardian_evidence.snapshot();
let codex_core::context::GuardianUserInputSnapshot {
@@ -434,7 +431,6 @@ impl GuardianV2Extension {
call_id,
mcp_tool,
planned_action,
review_model_override,
sync_reviews,
trusted_user_inputs,
authorization_version,