From a9d2564bcbe98dbc44e5395ca9f2f373265571bf Mon Sep 17 00:00:00 2001 From: jif Date: Tue, 15 Sep 2026 16:14:37 +0000 Subject: [PATCH] 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 --- codex-rs/Cargo.lock | 1 - codex-rs/core/BUILD.bazel | 3 + codex-rs/core/src/guardian/review.rs | 7 +- .../core/src/guardian/review_session_tests.rs | 31 +++++--- codex-rs/core/src/guardian/reviewer_config.rs | 70 ++--------------- codex-rs/core/src/guardian/test_host.rs | 13 +++- codex-rs/core/src/guardian/tests.rs | 20 ++--- codex-rs/ext/guardian-reviewer/Cargo.toml | 1 - codex-rs/ext/guardian-reviewer/src/lib.rs | 4 +- .../ext/guardian-reviewer/src/settings.rs | 75 +++---------------- codex-rs/ext/guardian-v2/BUILD.bazel | 6 ++ .../ext/guardian-v2/src/sync_reviewer/mod.rs | 10 ++- .../src/sync_reviewer/reviewer_config.rs | 63 ++++++++++++++++ 13 files changed, 148 insertions(+), 156 deletions(-) create mode 100644 codex-rs/ext/guardian-v2/src/sync_reviewer/reviewer_config.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 97d8c77f98..c5a8bda798 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3568,7 +3568,6 @@ dependencies = [ "anyhow", "codex-analytics", "codex-extension-api", - "codex-features", "codex-feedback", "codex-guardian-context", "codex-mcp", diff --git a/codex-rs/core/BUILD.bazel b/codex-rs/core/BUILD.bazel index 4c90f833e2..cb35952df0 100644 --- a/codex-rs/core/BUILD.bazel +++ b/codex-rs/core/BUILD.bazel @@ -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", diff --git a/codex-rs/core/src/guardian/review.rs b/codex-rs/core/src/guardian/review.rs index 7235cce9e3..d36457ed36 100644 --- a/codex-rs/core/src/guardian/review.rs +++ b/codex-rs/core/src/guardian/review.rs @@ -167,8 +167,13 @@ pub(super) async fn guardian_review_session_config( .await, ) }; + let reviewer_config = session + .services + .thread_extension_data + .get::>() + .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(), diff --git a/codex-rs/core/src/guardian/review_session_tests.rs b/codex-rs/core/src/guardian/review_session_tests.rs index d69e37918f..21fa071bf5 100644 --- a/codex-rs/core/src/guardian/review_session_tests.rs +++ b/codex-rs/core/src/guardian/review_session_tests.rs @@ -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::( + crate::guardian::test_host::build_reviewer_config, + ), + ); let context = setup::prepare_prewarm( Arc::clone(¶ms.parent_session), Arc::clone(params.parent_context.turn()), diff --git a/codex-rs/core/src/guardian/reviewer_config.rs b/codex-rs/core/src/guardian/reviewer_config.rs index cd346cb45f..cf5e224a49 100644 --- a/codex-rs/core/src/guardian/reviewer_config.rs +++ b/codex-rs/core/src/guardian/reviewer_config.rs @@ -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, active_model: &str, reasoning_effort: Option, @@ -25,29 +20,13 @@ pub fn build_guardian_review_session_config( personality: Option, model_messages: Option<&ModelMessages>, ) -> anyhow::Result { - 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) } diff --git a/codex-rs/core/src/guardian/test_host.rs b/codex-rs/core/src/guardian/test_host.rs index 7fdcfd5d03..75c42343b3 100644 --- a/codex-rs/core/src/guardian/test_host.rs +++ b/codex-rs/core/src/guardian/test_host.rs @@ -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::(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); diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index 68888d9973..8587c4ce2d 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -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, diff --git a/codex-rs/ext/guardian-reviewer/Cargo.toml b/codex-rs/ext/guardian-reviewer/Cargo.toml index 982e35ceed..a882ae7dea 100644 --- a/codex-rs/ext/guardian-reviewer/Cargo.toml +++ b/codex-rs/ext/guardian-reviewer/Cargo.toml @@ -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 } diff --git a/codex-rs/ext/guardian-reviewer/src/lib.rs b/codex-rs/ext/guardian-reviewer/src/lib.rs index 4fdfc6ebbd..86347f863e 100644 --- a/codex-rs/ext/guardian-reviewer/src/lib.rs +++ b/codex-rs/ext/guardian-reviewer/src/lib.rs @@ -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; diff --git a/codex-rs/ext/guardian-reviewer/src/settings.rs b/codex-rs/ext/guardian-reviewer/src/settings.rs index 2564045622..b26e6fa36f 100644 --- a/codex-rs/ext/guardian-reviewer/src/settings.rs +++ b/codex-rs/ext/guardian-reviewer/src/settings.rs @@ -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(pub fn(&C) -> anyhow::Result); + /// 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, - 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>, - pub developer_instructions: Option, - pub approval_policy: AskForApproval, - pub permission_profile: PermissionProfile, - pub include_apps_instructions: bool, - pub inherit_mcp_servers: bool, - pub disabled_features: Vec, -} - -pub fn reviewer_config_overrides( - parent_permissions: &PermissionProfile, - model: &str, - reasoning_effort: Option, -) -> 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) diff --git a/codex-rs/ext/guardian-v2/BUILD.bazel b/codex-rs/ext/guardian-v2/BUILD.bazel index f965d11c86..3bca217639 100644 --- a/codex-rs/ext/guardian-v2/BUILD.bazel +++ b/codex-rs/ext/guardian-v2/BUILD.bazel @@ -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 = [ diff --git a/codex-rs/ext/guardian-v2/src/sync_reviewer/mod.rs b/codex-rs/ext/guardian-v2/src/sync_reviewer/mod.rs index 4bb6193c4a..ec162a6a0c 100644 --- a/codex-rs/ext/guardian-v2/src/sync_reviewer/mod.rs +++ b/codex-rs/ext/guardian-v2/src/sync_reviewer/mod.rs @@ -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 for GuardianExtension { if input.session_source.is_internal() { return; } + input + .thread_store + .insert(codex_guardian_reviewer::ReviewerConfig::( + 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 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); diff --git a/codex-rs/ext/guardian-v2/src/sync_reviewer/reviewer_config.rs b/codex-rs/ext/guardian-v2/src/sync_reviewer/reviewer_config.rs new file mode 100644 index 0000000000..c8748cb159 --- /dev/null +++ b/codex-rs/ext/guardian-v2/src/sync_reviewer/reviewer_config.rs @@ -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 { + 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) +}