mirror of
https://github.com/openai/codex.git
synced 2026-09-17 12:23:33 +00:00
Move Guardian reviewer configuration into the extension (#45729)
## What changed Move concrete reviewer settings into `guardian-v2` and replace `ReviewerConfigOverrides` with a `ReviewerConfig` callback stored in thread extension data. Apply the callback to each captured parent configuration before preparing context and checking session reuse. Keep model selection, policy prompt construction, and live network state in the core adapter. Preserve the existing read-only reviewer settings and share `reviewer_permission_profile` with inherited environment configuration. ## Testing Update existing Guardian configuration, session reuse, and prewarming tests to use the extension's actual configuration builder, including compiling the shared source in core's test host. GitOrigin-RevId: dfa1dae9f8d4469f2e4f6e64ab842cc662970898
This commit is contained in:
1
codex-rs/Cargo.lock
generated
1
codex-rs/Cargo.lock
generated
@@ -3568,7 +3568,6 @@ dependencies = [
|
||||
"anyhow",
|
||||
"codex-analytics",
|
||||
"codex-extension-api",
|
||||
"codex-features",
|
||||
"codex-feedback",
|
||||
"codex-guardian-context",
|
||||
"codex-mcp",
|
||||
|
||||
@@ -4,6 +4,9 @@ codex_rust_crate(
|
||||
name = "core",
|
||||
compile_data = glob(["assets/**"]),
|
||||
crate_name = "codex_core",
|
||||
crate_srcs = glob(["src/**/*.rs"]) + [
|
||||
"//codex-rs/ext/guardian-v2:src/sync_reviewer/reviewer_config.rs",
|
||||
],
|
||||
extra_binaries = [
|
||||
"//codex-rs/bwrap:bwrap",
|
||||
"//codex-rs/code-mode-host:codex-code-mode-host",
|
||||
|
||||
@@ -167,8 +167,13 @@ pub(super) async fn guardian_review_session_config(
|
||||
.await,
|
||||
)
|
||||
};
|
||||
let reviewer_config = session
|
||||
.services
|
||||
.thread_extension_data
|
||||
.get::<codex_guardian_reviewer::ReviewerConfig<crate::config::Config>>()
|
||||
.ok_or_else(|| anyhow::anyhow!("Guardian reviewer configuration is not installed"))?;
|
||||
let mut spawn_config = build_guardian_review_session_config(
|
||||
turn.config.as_ref(),
|
||||
(reviewer_config.0)(turn.config.as_ref())?,
|
||||
live_network_config,
|
||||
review_model.model.as_str(),
|
||||
review_model.reasoning_effort.clone(),
|
||||
|
||||
@@ -33,7 +33,8 @@ async fn run_review_preserves_evidence_during_parent_compaction() {
|
||||
.await;
|
||||
let mut params = test_review_params().await;
|
||||
params.spawn_config = build_guardian_review_session_config(
|
||||
turn.config.as_ref(),
|
||||
crate::guardian::test_host::build_reviewer_config(turn.config.as_ref())
|
||||
.expect("reviewer config"),
|
||||
/*live_network_config*/ None,
|
||||
¶ms.review_model.model,
|
||||
params.review_model.reasoning_effort.clone(),
|
||||
@@ -207,7 +208,8 @@ async fn test_review_params() -> GuardianReviewSessionParams {
|
||||
#[allow(deprecated)]
|
||||
let cwd = turn.cwd.clone();
|
||||
let spawn_config = build_guardian_review_session_config(
|
||||
turn.config.as_ref(),
|
||||
crate::guardian::test_host::build_reviewer_config(turn.config.as_ref())
|
||||
.expect("reviewer config"),
|
||||
/*live_network_config*/ None,
|
||||
model.as_str(),
|
||||
reasoning_effort.clone(),
|
||||
@@ -307,7 +309,7 @@ async fn spawned_guardian_reuse_key_matches_inherited_instructions() {
|
||||
async fn guardian_review_session_config_change_invalidates_cached_session() {
|
||||
let parent_config = crate::config::test_config().await;
|
||||
let cached_spawn_config = build_guardian_review_session_config(
|
||||
&parent_config,
|
||||
crate::guardian::test_host::build_reviewer_config(&parent_config).expect("reviewer config"),
|
||||
/*live_network_config*/ None,
|
||||
"active-model",
|
||||
/*reasoning_effort*/ None,
|
||||
@@ -327,7 +329,8 @@ async fn guardian_review_session_config_change_invalidates_cached_session() {
|
||||
changed_parent_config.model_provider.base_url =
|
||||
Some("https://guardian.example.invalid/v1".to_string());
|
||||
let next_spawn_config = build_guardian_review_session_config(
|
||||
&changed_parent_config,
|
||||
crate::guardian::test_host::build_reviewer_config(&changed_parent_config)
|
||||
.expect("reviewer config"),
|
||||
/*live_network_config*/ None,
|
||||
"active-model",
|
||||
/*reasoning_effort*/ None,
|
||||
@@ -517,7 +520,7 @@ async fn guardian_prompt_cache_key_is_scoped_to_parent_thread() {
|
||||
async fn guardian_review_session_compact_scope_change_invalidates_cached_session() {
|
||||
let parent_config = crate::config::test_config().await;
|
||||
let cached_spawn_config = build_guardian_review_session_config(
|
||||
&parent_config,
|
||||
crate::guardian::test_host::build_reviewer_config(&parent_config).expect("reviewer config"),
|
||||
/*live_network_config*/ None,
|
||||
"active-model",
|
||||
/*reasoning_effort*/ None,
|
||||
@@ -537,7 +540,8 @@ async fn guardian_review_session_compact_scope_change_invalidates_cached_session
|
||||
changed_parent_config.model_auto_compact_token_limit_scope =
|
||||
AutoCompactTokenLimitScope::BodyAfterPrefix;
|
||||
let next_spawn_config = build_guardian_review_session_config(
|
||||
&changed_parent_config,
|
||||
crate::guardian::test_host::build_reviewer_config(&changed_parent_config)
|
||||
.expect("reviewer config"),
|
||||
/*live_network_config*/ None,
|
||||
"active-model",
|
||||
/*reasoning_effort*/ None,
|
||||
@@ -565,7 +569,7 @@ async fn guardian_review_session_config_disables_hooks() {
|
||||
.expect("enable hooks on parent config");
|
||||
|
||||
let guardian_config = build_guardian_review_session_config(
|
||||
&parent_config,
|
||||
crate::guardian::test_host::build_reviewer_config(&parent_config).expect("reviewer config"),
|
||||
/*live_network_config*/ None,
|
||||
"active-model",
|
||||
/*reasoning_effort*/ None,
|
||||
@@ -584,7 +588,7 @@ async fn guardian_review_session_config_disables_skill_instructions() {
|
||||
parent_config.include_skill_instructions = true;
|
||||
|
||||
let guardian_config = build_guardian_review_session_config(
|
||||
&parent_config,
|
||||
crate::guardian::test_host::build_reviewer_config(&parent_config).expect("reviewer config"),
|
||||
/*live_network_config*/ None,
|
||||
"active-model",
|
||||
/*reasoning_effort*/ None,
|
||||
@@ -627,7 +631,7 @@ async fn guardian_review_session_config_prefers_configured_policy_and_template()
|
||||
};
|
||||
|
||||
let guardian_config = build_guardian_review_session_config(
|
||||
&parent_config,
|
||||
crate::guardian::test_host::build_reviewer_config(&parent_config).expect("reviewer config"),
|
||||
/*live_network_config*/ None,
|
||||
"active-model",
|
||||
/*reasoning_effort*/ None,
|
||||
@@ -671,7 +675,7 @@ async fn guardian_review_session_config_preserves_explicit_empty_catalog_policy(
|
||||
};
|
||||
|
||||
let guardian_config = build_guardian_review_session_config(
|
||||
&parent_config,
|
||||
crate::guardian::test_host::build_reviewer_config(&parent_config).expect("reviewer config"),
|
||||
/*live_network_config*/ None,
|
||||
"active-model",
|
||||
/*reasoning_effort*/ None,
|
||||
@@ -723,7 +727,7 @@ async fn guardian_review_session_config_preserves_explicit_empty_catalog_templat
|
||||
};
|
||||
|
||||
let guardian_config = build_guardian_review_session_config(
|
||||
&parent_config,
|
||||
crate::guardian::test_host::build_reviewer_config(&parent_config).expect("reviewer config"),
|
||||
/*live_network_config*/ None,
|
||||
"active-model",
|
||||
/*reasoning_effort*/ None,
|
||||
@@ -1160,6 +1164,11 @@ async fn prewarm_test_session(
|
||||
Box::pin(async move { Ok(session.lock().await.take().expect("one fixture spawn")) })
|
||||
},
|
||||
);
|
||||
params.parent_session.services.thread_extension_data.insert(
|
||||
codex_guardian_reviewer::ReviewerConfig::<Config>(
|
||||
crate::guardian::test_host::build_reviewer_config,
|
||||
),
|
||||
);
|
||||
let context = setup::prepare_prewarm(
|
||||
Arc::clone(¶ms.parent_session),
|
||||
Arc::clone(params.parent_context.turn()),
|
||||
|
||||
@@ -1,23 +1,18 @@
|
||||
//! Applies extension-owned reviewer settings to host configuration and builds context.
|
||||
//! Managed constraints, live network rules and policy prompt construction stay in the host.
|
||||
|
||||
use std::collections::HashMap;
|
||||
//! Adds model, policy context and live network state to the extension's reviewer configuration.
|
||||
//! Both prewarming and reviews finish this setup before context preparation and reuse checks.
|
||||
|
||||
use codex_protocol::models::BaseInstructionsProvenance;
|
||||
use codex_protocol::openai_models::ModelMessages;
|
||||
use tracing::warn;
|
||||
|
||||
use crate::config::Config;
|
||||
use crate::config::Constrained;
|
||||
use crate::config::NetworkProxySpec;
|
||||
use crate::config::TokenBudgetConfig;
|
||||
|
||||
use super::prompt::BUNDLED_GUARDIAN_POLICY_TEMPLATE;
|
||||
use super::prompt::guardian_policy_prompt_with_config_and_template;
|
||||
|
||||
/// Builds the existing read-only reviewer configuration with its policy and live network rules.
|
||||
/// Adds the captured model, policy prompt and live network rules before reuse selection.
|
||||
pub fn build_guardian_review_session_config(
|
||||
parent_config: &Config,
|
||||
mut guardian_config: Config,
|
||||
live_network_config: Option<codex_network_proxy::NetworkProxyConfig>,
|
||||
active_model: &str,
|
||||
reasoning_effort: Option<codex_protocol::openai_models::ReasoningEffort>,
|
||||
@@ -25,29 +20,13 @@ pub fn build_guardian_review_session_config(
|
||||
personality: Option<codex_protocol::config_types::Personality>,
|
||||
model_messages: Option<&ModelMessages>,
|
||||
) -> anyhow::Result<Config> {
|
||||
let mut guardian_config = parent_config.clone();
|
||||
let overrides = codex_guardian_reviewer::reviewer_config_overrides(
|
||||
parent_config.permissions.permission_profile(),
|
||||
active_model,
|
||||
reasoning_effort,
|
||||
);
|
||||
guardian_config.model = Some(overrides.model);
|
||||
guardian_config.model_reasoning_effort = overrides.reasoning_effort;
|
||||
guardian_config.model = Some(active_model.to_owned());
|
||||
guardian_config.model_reasoning_effort = reasoning_effort;
|
||||
guardian_config.model_reasoning_summary = Some(reasoning_summary);
|
||||
guardian_config.personality = personality;
|
||||
guardian_config.model_provider.request_max_retries = Some(overrides.request_max_retries);
|
||||
guardian_config.model_provider.stream_max_retries = Some(overrides.stream_max_retries);
|
||||
guardian_config.include_skill_instructions = overrides.include_skill_instructions;
|
||||
guardian_config.memories.use_memories = overrides.use_memories;
|
||||
guardian_config.memories.dedicated_tools = overrides.dedicated_memory_tools;
|
||||
if !overrides.inherit_token_budget {
|
||||
// An explicit disabled config prevents model defaults from reactivating it.
|
||||
guardian_config.token_budget_startup_config = None;
|
||||
guardian_config.token_budget = Some(TokenBudgetConfig::default());
|
||||
}
|
||||
let catalog_auto_review = model_messages.and_then(|messages| messages.auto_review.as_ref());
|
||||
let tenant_policy_config = parent_config.resolve_guardian_policy(model_messages);
|
||||
let policy_template = parent_config
|
||||
let tenant_policy_config = guardian_config.resolve_guardian_policy(model_messages);
|
||||
let policy_template = guardian_config
|
||||
.guardian_policy_template
|
||||
.as_deref()
|
||||
.or_else(|| catalog_auto_review.and_then(|messages| messages.policy_template.as_deref()))
|
||||
@@ -57,25 +36,6 @@ pub fn build_guardian_review_session_config(
|
||||
policy_template,
|
||||
));
|
||||
guardian_config.base_instructions_provenance = Some(BaseInstructionsProvenance::Custom);
|
||||
guardian_config.notify = overrides.notify;
|
||||
guardian_config.developer_instructions = overrides.developer_instructions;
|
||||
guardian_config.permissions.approval_policy =
|
||||
Constrained::allow_only(overrides.approval_policy);
|
||||
guardian_config
|
||||
.permissions
|
||||
.set_permission_profile(overrides.permission_profile)
|
||||
.map_err(|err| {
|
||||
anyhow::anyhow!("guardian review session could not set permission profile: {err}")
|
||||
})?;
|
||||
guardian_config.include_apps_instructions = overrides.include_apps_instructions;
|
||||
if !overrides.inherit_mcp_servers {
|
||||
guardian_config
|
||||
.mcp_servers
|
||||
.set(HashMap::new())
|
||||
.map_err(|err| {
|
||||
anyhow::anyhow!("guardian review session could not clear MCP servers: {err}")
|
||||
})?;
|
||||
}
|
||||
if let Some(live_network_config) = live_network_config
|
||||
&& guardian_config.permissions.network.is_some()
|
||||
{
|
||||
@@ -91,19 +51,5 @@ pub fn build_guardian_review_session_config(
|
||||
guardian_config.permissions.permission_profile(),
|
||||
)?);
|
||||
}
|
||||
for feature in overrides.disabled_features {
|
||||
guardian_config.features.disable(feature).map_err(|err| {
|
||||
anyhow::anyhow!(
|
||||
"guardian review session could not disable `features.{}`: {err}",
|
||||
feature.key()
|
||||
)
|
||||
})?;
|
||||
if guardian_config.features.enabled(feature) {
|
||||
warn!(
|
||||
"guardian review session could not disable `features.{}`; continuing with the feature enabled",
|
||||
feature.key()
|
||||
);
|
||||
}
|
||||
}
|
||||
Ok(guardian_config)
|
||||
}
|
||||
|
||||
@@ -5,7 +5,6 @@ use std::sync::Arc;
|
||||
|
||||
use codex_extension_api::SessionIsolation;
|
||||
use codex_home::CodexHomeUserInstructionsProvider;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::InternalSessionSource;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::ThreadSource;
|
||||
@@ -13,9 +12,19 @@ use codex_protocol::protocol::ThreadSource;
|
||||
use super::GuardianReviewSessionManager;
|
||||
use crate::config::Config;
|
||||
use crate::config::Constrained;
|
||||
use crate::config::TokenBudgetConfig;
|
||||
use crate::session::session::Session;
|
||||
|
||||
// Compile the extension's actual setup with this test crate's Config type, rather
|
||||
// than keeping a second settings implementation in the context-adapter test host.
|
||||
#[path = "../../../ext/guardian-v2/src/sync_reviewer/reviewer_config.rs"]
|
||||
mod reviewer_config;
|
||||
pub(super) use reviewer_config::build_reviewer_config;
|
||||
|
||||
pub(crate) fn install(session: &Session, config: &Config) {
|
||||
session.services.thread_extension_data.insert(
|
||||
codex_guardian_reviewer::ReviewerConfig::<Config>(build_reviewer_config),
|
||||
);
|
||||
let manager = Arc::new(crate::ThreadManager::new(
|
||||
config,
|
||||
Arc::clone(&session.services.auth_manager),
|
||||
@@ -56,8 +65,6 @@ pub(crate) fn install(session: &Session, config: &Config) {
|
||||
) {
|
||||
options.config.ephemeral = true;
|
||||
}
|
||||
options.config.permissions.approval_policy =
|
||||
Constrained::allow_only(AskForApproval::Never);
|
||||
options.session_source =
|
||||
Some(SessionSource::Internal(InternalSessionSource::Guardian));
|
||||
options.thread_source = Some(ThreadSource::GuardianReview);
|
||||
|
||||
@@ -3746,7 +3746,7 @@ async fn guardian_review_session_config_preserves_parent_network_proxy() {
|
||||
parent_config.permissions.network = Some(network.clone());
|
||||
|
||||
let guardian_config = build_guardian_review_session_config_for_test(
|
||||
&parent_config,
|
||||
crate::guardian::test_host::build_reviewer_config(&parent_config).expect("reviewer config"),
|
||||
/*live_network_config*/ None,
|
||||
"parent-active-model",
|
||||
Some(codex_protocol::openai_models::ReasoningEffort::Low),
|
||||
@@ -3847,7 +3847,7 @@ async fn guardian_review_session_config_clears_parent_developer_instructions() {
|
||||
Some("parent or managed config should not replace guardian policy".to_string());
|
||||
|
||||
let guardian_config = build_guardian_review_session_config_for_test(
|
||||
&parent_config,
|
||||
crate::guardian::test_host::build_reviewer_config(&parent_config).expect("reviewer config"),
|
||||
/*live_network_config*/ None,
|
||||
"active-model",
|
||||
/*reasoning_effort*/ None,
|
||||
@@ -3876,7 +3876,7 @@ async fn guardian_review_session_config_clears_legacy_notify() {
|
||||
]);
|
||||
|
||||
let guardian_config = build_guardian_review_session_config_for_test(
|
||||
&parent_config,
|
||||
crate::guardian::test_host::build_reviewer_config(&parent_config).expect("reviewer config"),
|
||||
/*live_network_config*/ None,
|
||||
"active-model",
|
||||
/*reasoning_effort*/ None,
|
||||
@@ -3913,7 +3913,7 @@ async fn guardian_review_session_config_uses_live_network_proxy_state() {
|
||||
live_network.set_allowed_domains(vec!["github.com".to_string()]);
|
||||
|
||||
let guardian_config = build_guardian_review_session_config_for_test(
|
||||
&parent_config,
|
||||
crate::guardian::test_host::build_reviewer_config(&parent_config).expect("reviewer config"),
|
||||
Some(live_network.clone()),
|
||||
"active-model",
|
||||
/*reasoning_effort*/ None,
|
||||
@@ -3962,7 +3962,7 @@ async fn guardian_review_session_config_disables_mcp_apps_plugins_memories_and_g
|
||||
parent_config.memories.dedicated_tools = true;
|
||||
|
||||
let guardian_config = build_guardian_review_session_config_for_test(
|
||||
&parent_config,
|
||||
crate::guardian::test_host::build_reviewer_config(&parent_config).expect("reviewer config"),
|
||||
/*live_network_config*/ None,
|
||||
"active-model",
|
||||
/*reasoning_effort*/ None,
|
||||
@@ -3996,7 +3996,7 @@ async fn guardian_review_session_config_allows_pinned_disabled_feature() {
|
||||
.expect("managed features");
|
||||
|
||||
let guardian_config = build_guardian_review_session_config_for_test(
|
||||
&parent_config,
|
||||
crate::guardian::test_host::build_reviewer_config(&parent_config).expect("reviewer config"),
|
||||
/*live_network_config*/ None,
|
||||
"active-model",
|
||||
/*reasoning_effort*/ None,
|
||||
@@ -4017,7 +4017,7 @@ async fn guardian_review_session_config_uses_parent_active_model_instead_of_hard
|
||||
parent_config.model = Some("configured-model".to_string());
|
||||
|
||||
let guardian_config = build_guardian_review_session_config_for_test(
|
||||
&parent_config,
|
||||
crate::guardian::test_host::build_reviewer_config(&parent_config).expect("reviewer config"),
|
||||
/*live_network_config*/ None,
|
||||
"active-model",
|
||||
/*reasoning_effort*/ None,
|
||||
@@ -4038,7 +4038,7 @@ async fn guardian_review_session_config_keeps_bedrock_provider_for_bedrock_gpt_5
|
||||
ModelProviderInfo::create_amazon_bedrock_provider(/*aws*/ None);
|
||||
|
||||
let guardian_config = build_guardian_review_session_config_for_test(
|
||||
&parent_config,
|
||||
crate::guardian::test_host::build_reviewer_config(&parent_config).expect("reviewer config"),
|
||||
/*live_network_config*/ None,
|
||||
AMAZON_BEDROCK_GPT_5_4_MODEL_ID,
|
||||
Some(ReasoningEffort::Low),
|
||||
@@ -4095,7 +4095,7 @@ async fn guardian_review_session_config_uses_requirements_guardian_policy_config
|
||||
.expect("load config");
|
||||
|
||||
let guardian_config = build_guardian_review_session_config_for_test(
|
||||
&parent_config,
|
||||
crate::guardian::test_host::build_reviewer_config(&parent_config).expect("reviewer config"),
|
||||
/*live_network_config*/ None,
|
||||
"active-model",
|
||||
/*reasoning_effort*/ None,
|
||||
@@ -4137,7 +4137,7 @@ async fn guardian_review_session_config_uses_default_guardian_policy_without_req
|
||||
.expect("load config");
|
||||
|
||||
let guardian_config = build_guardian_review_session_config_for_test(
|
||||
&parent_config,
|
||||
crate::guardian::test_host::build_reviewer_config(&parent_config).expect("reviewer config"),
|
||||
/*live_network_config*/ None,
|
||||
"active-model",
|
||||
/*reasoning_effort*/ None,
|
||||
|
||||
@@ -16,7 +16,6 @@ workspace = true
|
||||
anyhow = { workspace = true }
|
||||
codex-analytics = { workspace = true }
|
||||
codex-extension-api = { workspace = true }
|
||||
codex-features = { workspace = true }
|
||||
codex-feedback = { workspace = true }
|
||||
codex-guardian-context = { workspace = true }
|
||||
codex-mcp = { workspace = true }
|
||||
|
||||
@@ -56,10 +56,10 @@ pub use completion::guardian_timeout_message;
|
||||
pub use execution::ReviewerRuntime;
|
||||
pub use execution::start_review_turn;
|
||||
pub use execution::wait_for_guardian_review;
|
||||
pub use settings::ReviewerConfigOverrides;
|
||||
pub use settings::ReviewerConfig;
|
||||
pub use settings::ReviewerTurn;
|
||||
pub use settings::reviewer_allowed_tools;
|
||||
pub use settings::reviewer_config_overrides;
|
||||
pub use settings::reviewer_permission_profile;
|
||||
|
||||
pub use feedback::FailedReviewFeedback;
|
||||
pub use feedback::ReviewFeedbackContext;
|
||||
|
||||
@@ -1,11 +1,10 @@
|
||||
//! Defines the reviewer's runtime settings. Hosts apply these settings to their
|
||||
//! concrete configuration; context construction and managed constraints stay with the host.
|
||||
//! Defines reviewer tools, read-only permissions and per-turn input.
|
||||
//! Guardian supplies concrete session configuration to the temporary host context adapter.
|
||||
|
||||
use std::collections::HashMap;
|
||||
|
||||
use codex_extension_api::AllowedTools;
|
||||
use codex_extension_api::ToolName;
|
||||
use codex_features::Feature;
|
||||
use codex_protocol::config_types::CollaborationMode;
|
||||
use codex_protocol::config_types::ModeKind;
|
||||
use codex_protocol::config_types::Personality;
|
||||
@@ -23,6 +22,11 @@ use codex_protocol::turn_input::TurnStartOptions;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use serde_json::Value;
|
||||
|
||||
/// Guardian's configuration function for the host's concrete config type.
|
||||
/// The host applies it to each captured parent config before building context and reuse keys.
|
||||
/// Keeping it in thread data avoids caching settings that can change between reviews.
|
||||
pub struct ReviewerConfig<C>(pub fn(&C) -> anyhow::Result<C>);
|
||||
|
||||
/// Reviewer tools, including the existing Code Mode dispatchers when enabled.
|
||||
/// The host still applies feature flags and sandbox restrictions to these tools.
|
||||
pub fn reviewer_allowed_tools() -> AllowedTools {
|
||||
@@ -34,62 +38,8 @@ pub fn reviewer_allowed_tools() -> AllowedTools {
|
||||
)
|
||||
}
|
||||
|
||||
/// Configuration policy for a reviewer. The host retains managed constraints and
|
||||
/// live network rules while applying these values to its concrete runtime config.
|
||||
pub struct ReviewerConfigOverrides {
|
||||
pub model: String,
|
||||
pub reasoning_effort: Option<ReasoningEffort>,
|
||||
pub request_max_retries: u64,
|
||||
pub stream_max_retries: u64,
|
||||
pub include_skill_instructions: bool,
|
||||
pub use_memories: bool,
|
||||
pub dedicated_memory_tools: bool,
|
||||
pub inherit_token_budget: bool,
|
||||
pub notify: Option<Vec<String>>,
|
||||
pub developer_instructions: Option<String>,
|
||||
pub approval_policy: AskForApproval,
|
||||
pub permission_profile: PermissionProfile,
|
||||
pub include_apps_instructions: bool,
|
||||
pub inherit_mcp_servers: bool,
|
||||
pub disabled_features: Vec<Feature>,
|
||||
}
|
||||
|
||||
pub fn reviewer_config_overrides(
|
||||
parent_permissions: &PermissionProfile,
|
||||
model: &str,
|
||||
reasoning_effort: Option<ReasoningEffort>,
|
||||
) -> ReviewerConfigOverrides {
|
||||
ReviewerConfigOverrides {
|
||||
model: model.to_owned(),
|
||||
reasoning_effort,
|
||||
request_max_retries: 1,
|
||||
stream_max_retries: 1,
|
||||
include_skill_instructions: false,
|
||||
use_memories: false,
|
||||
dedicated_memory_tools: false,
|
||||
inherit_token_budget: false,
|
||||
notify: None,
|
||||
developer_instructions: None,
|
||||
approval_policy: AskForApproval::Never,
|
||||
permission_profile: read_only_guardian_permission_profile(parent_permissions),
|
||||
include_apps_instructions: false,
|
||||
inherit_mcp_servers: false,
|
||||
disabled_features: vec![
|
||||
Feature::Collab,
|
||||
Feature::MultiAgentV2,
|
||||
Feature::GuardianV2,
|
||||
Feature::TokenBudget,
|
||||
Feature::ContextManagement,
|
||||
Feature::CodexHooks,
|
||||
Feature::Apps,
|
||||
Feature::Plugins,
|
||||
Feature::WebSearchRequest,
|
||||
Feature::WebSearchCached,
|
||||
],
|
||||
}
|
||||
}
|
||||
|
||||
fn read_only_guardian_permission_profile(profile: &PermissionProfile) -> PermissionProfile {
|
||||
/// Applies the same read-only ceiling to the reviewer and each inherited environment.
|
||||
pub fn reviewer_permission_profile(profile: &PermissionProfile) -> PermissionProfile {
|
||||
profile
|
||||
.intersect_with_read_only()
|
||||
.unwrap_or(PermissionProfile::External {
|
||||
@@ -117,10 +67,9 @@ impl ReviewerTurn {
|
||||
// Apply the same read-only ceiling to every inherited environment.
|
||||
for environment in &mut self.environments.environments {
|
||||
if let EnvironmentConfigState::Ready(config) = &mut environment.config {
|
||||
config.permission_profile =
|
||||
PermissionProfileSnapshot::legacy(read_only_guardian_permission_profile(
|
||||
config.permission_profile.permission_profile(),
|
||||
));
|
||||
config.permission_profile = PermissionProfileSnapshot::legacy(
|
||||
reviewer_permission_profile(config.permission_profile.permission_profile()),
|
||||
);
|
||||
}
|
||||
}
|
||||
TurnInputRequest::user_input(self.items)
|
||||
|
||||
@@ -1,5 +1,11 @@
|
||||
load("//:defs.bzl", "codex_rust_crate")
|
||||
|
||||
# Core's context-adapter unit tests compile the same setup function with their Config type.
|
||||
exports_files(
|
||||
["src/sync_reviewer/reviewer_config.rs"],
|
||||
visibility = ["//codex-rs/core:__pkg__"],
|
||||
)
|
||||
|
||||
codex_rust_crate(
|
||||
name = "guardian-v2",
|
||||
compile_data = [
|
||||
|
||||
@@ -8,6 +8,7 @@ use std::sync::Weak;
|
||||
use codex_core::ThreadManager;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::config::Constrained;
|
||||
use codex_core::config::TokenBudgetConfig;
|
||||
use codex_core::guardian_review::GuardianReviewSession;
|
||||
use codex_extension_api::ExtensionFuture;
|
||||
use codex_extension_api::ExtensionRegistryBuilder;
|
||||
@@ -30,6 +31,8 @@ use codex_protocol::protocol::InternalSessionSource;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::ThreadSource;
|
||||
|
||||
mod reviewer_config;
|
||||
|
||||
/// Owns reviewer agents through the same thread manager as the parent conversation.
|
||||
#[derive(Debug)]
|
||||
struct GuardianExtension {
|
||||
@@ -45,6 +48,11 @@ impl ThreadLifecycleContributor<Config> for GuardianExtension {
|
||||
if input.session_source.is_internal() {
|
||||
return;
|
||||
}
|
||||
input
|
||||
.thread_store
|
||||
.insert(codex_guardian_reviewer::ReviewerConfig::<Config>(
|
||||
reviewer_config::build_reviewer_config,
|
||||
));
|
||||
let manager = self.thread_manager.clone();
|
||||
let runtime = input.thread_store.get_or_init(ReviewerTasks::default);
|
||||
input.thread_store.get_or_init(|| {
|
||||
@@ -74,8 +82,6 @@ impl ThreadLifecycleContributor<Config> for GuardianExtension {
|
||||
) {
|
||||
options.config.ephemeral = true;
|
||||
}
|
||||
options.config.permissions.approval_policy =
|
||||
Constrained::allow_only(AskForApproval::Never);
|
||||
options.session_source =
|
||||
Some(SessionSource::Internal(InternalSessionSource::Guardian));
|
||||
options.thread_source = Some(ThreadSource::GuardianReview);
|
||||
|
||||
@@ -0,0 +1,63 @@
|
||||
//! Applies Guardian's reviewer settings to a fresh copy of the captured parent configuration.
|
||||
//! Managed constraints stay enforced by Config; context and live network state are added by
|
||||
//! the temporary host adapter before it decides whether an existing reviewer can be reused.
|
||||
|
||||
use super::Config;
|
||||
use super::Constrained;
|
||||
use super::TokenBudgetConfig;
|
||||
use codex_features::Feature;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use std::collections::HashMap;
|
||||
|
||||
pub(crate) fn build_reviewer_config(parent_config: &Config) -> anyhow::Result<Config> {
|
||||
let mut config = parent_config.clone();
|
||||
config.model_provider.request_max_retries = Some(1);
|
||||
config.model_provider.stream_max_retries = Some(1);
|
||||
config.include_skill_instructions = false;
|
||||
config.memories.use_memories = false;
|
||||
config.memories.dedicated_tools = false;
|
||||
// An explicit disabled config prevents model defaults from reactivating it.
|
||||
config.token_budget_startup_config = None;
|
||||
config.token_budget = Some(TokenBudgetConfig::default());
|
||||
config.notify = None;
|
||||
config.developer_instructions = None;
|
||||
config.permissions.approval_policy = Constrained::allow_only(AskForApproval::Never);
|
||||
config
|
||||
.permissions
|
||||
.set_permission_profile(codex_guardian_reviewer::reviewer_permission_profile(
|
||||
config.permissions.permission_profile(),
|
||||
))
|
||||
.map_err(|err| {
|
||||
anyhow::anyhow!("guardian review session could not set permission profile: {err}")
|
||||
})?;
|
||||
config.include_apps_instructions = false;
|
||||
config.mcp_servers.set(HashMap::new()).map_err(|err| {
|
||||
anyhow::anyhow!("guardian review session could not clear MCP servers: {err}")
|
||||
})?;
|
||||
for feature in [
|
||||
Feature::Collab,
|
||||
Feature::MultiAgentV2,
|
||||
Feature::GuardianV2,
|
||||
Feature::TokenBudget,
|
||||
Feature::ContextManagement,
|
||||
Feature::CodexHooks,
|
||||
Feature::Apps,
|
||||
Feature::Plugins,
|
||||
Feature::WebSearchRequest,
|
||||
Feature::WebSearchCached,
|
||||
] {
|
||||
config.features.disable(feature).map_err(|err| {
|
||||
anyhow::anyhow!(
|
||||
"guardian review session could not disable `features.{}`: {err}",
|
||||
feature.key()
|
||||
)
|
||||
})?;
|
||||
if config.features.enabled(feature) {
|
||||
tracing::warn!(
|
||||
"guardian review session could not disable `features.{}`; continuing with the feature enabled",
|
||||
feature.key()
|
||||
);
|
||||
}
|
||||
}
|
||||
Ok(config)
|
||||
}
|
||||
Reference in New Issue
Block a user