openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-09 15:58:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(guardian): clean up guardian config and context

Browse Source
This commit is contained in:
Owen Lin
2026-04-09 12:20:01 -07:00
parent 01537f0bd2
commit 8eb640f9c6
8 changed files with 309 additions and 121 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
codex-rs/Cargo.lock generated
View File

@@ -1676,6 +1676,7 @@ dependencies = [
"codex-config",
"codex-core",
"codex-exec",
"codex-exec-server",
"codex-execpolicy",
"codex-features",
"codex-login",

146
codex-rs/core/src/codex.rs
View File

@@ -3731,81 +3731,89 @@ impl Session {
{
developer_sections.push(developer_instructions.to_string());
}
// Add developer instructions for memories.
if turn_context.features.enabled(Feature::MemoryTool)
&& turn_context.config.memories.use_memories
&& let Some(memory_prompt) =
build_memory_tool_developer_instructions(&turn_context.config.codex_home).await
{
developer_sections.push(memory_prompt);
}
// Add developer instructions from collaboration_mode if they exist and are non-empty
if let Some(collab_instructions) =
DeveloperInstructions::from_collaboration_mode(&collaboration_mode)
{
developer_sections.push(collab_instructions.into_text());
}
if let Some(realtime_update) = crate::context_manager::updates::build_initial_realtime_item(
reference_context_item.as_ref(),
previous_turn_settings.as_ref(),
turn_context,
) {
developer_sections.push(realtime_update.into_text());
}
if self.features.enabled(Feature::Personality)
&& let Some(personality) = turn_context.personality
{
let model_info = turn_context.model_info.clone();
let has_baked_personality = model_info.supports_personality()
&& base_instructions == model_info.get_model_instructions(Some(personality));
if !has_baked_personality
&& let Some(personality_message) =
crate::context_manager::updates::personality_message_for(
&model_info,
personality,
)
if !separate_guardian_developer_message {
// Add developer instructions for memories.
if turn_context.features.enabled(Feature::MemoryTool)
&& turn_context.config.memories.use_memories
&& let Some(memory_prompt) =
build_memory_tool_developer_instructions(&turn_context.config.codex_home).await
{
developer_sections.push(
DeveloperInstructions::personality_spec_message(personality_message)
.into_text(),
);
developer_sections.push(memory_prompt);
}
}
if turn_context.config.include_apps_instructions && turn_context.apps_enabled() {
let mcp_connection_manager = self.services.mcp_connection_manager.read().await;
let accessible_and_enabled_connectors =
connectors::list_accessible_and_enabled_connectors_from_manager(
&mcp_connection_manager,
&turn_context.config,
// Add developer instructions from collaboration_mode if they exist and are non-empty
if let Some(collab_instructions) =
DeveloperInstructions::from_collaboration_mode(&collaboration_mode)
{
developer_sections.push(collab_instructions.into_text());
}
if let Some(realtime_update) =
crate::context_manager::updates::build_initial_realtime_item(
reference_context_item.as_ref(),
previous_turn_settings.as_ref(),
turn_context,
)
.await;
if let Some(apps_section) = render_apps_section(&accessible_and_enabled_connectors) {
developer_sections.push(apps_section);
{
developer_sections.push(realtime_update.into_text());
}
if self.features.enabled(Feature::Personality)
&& let Some(personality) = turn_context.personality
{
let model_info = turn_context.model_info.clone();
let has_baked_personality = model_info.supports_personality()
&& base_instructions == model_info.get_model_instructions(Some(personality));
if !has_baked_personality
&& let Some(personality_message) =
crate::context_manager::updates::personality_message_for(
&model_info,
personality,
)
{
developer_sections.push(
DeveloperInstructions::personality_spec_message(personality_message)
.into_text(),
);
}
}
if turn_context.config.include_apps_instructions && turn_context.apps_enabled() {
let mcp_connection_manager = self.services.mcp_connection_manager.read().await;
let accessible_and_enabled_connectors =
connectors::list_accessible_and_enabled_connectors_from_manager(
&mcp_connection_manager,
&turn_context.config,
)
.await;
if let Some(apps_section) = render_apps_section(&accessible_and_enabled_connectors)
{
developer_sections.push(apps_section);
}
}
let implicit_skills = turn_context
.turn_skills
.outcome
.allowed_skills_for_implicit_invocation();
if let Some(skills_section) = render_skills_section(&implicit_skills) {
developer_sections.push(skills_section);
}
let loaded_plugins = self
.services
.plugins_manager
.plugins_for_config(&turn_context.config);
if let Some(plugin_section) =
render_plugins_section(loaded_plugins.capability_summaries())
{
developer_sections.push(plugin_section);
}
if turn_context.features.enabled(Feature::CodexGitCommit)
&& let Some(commit_message_instruction) = commit_message_trailer_instruction(
turn_context.config.commit_attribution.as_deref(),
)
{
developer_sections.push(commit_message_instruction);
}
}
let implicit_skills = turn_context
.turn_skills
.outcome
.allowed_skills_for_implicit_invocation();
if let Some(skills_section) = render_skills_section(&implicit_skills) {
developer_sections.push(skills_section);
}
let loaded_plugins = self
.services
.plugins_manager
.plugins_for_config(&turn_context.config);
if let Some(plugin_section) = render_plugins_section(loaded_plugins.capability_summaries())
if !separate_guardian_developer_message
&& let Some(user_instructions) = turn_context.user_instructions.as_deref()
{
developer_sections.push(plugin_section);
}
if turn_context.features.enabled(Feature::CodexGitCommit)
&& let Some(commit_message_instruction) = commit_message_trailer_instruction(
turn_context.config.commit_attribution.as_deref(),
)
{
developer_sections.push(commit_message_instruction);
}
if let Some(user_instructions) = turn_context.user_instructions.as_deref() {
contextual_user_sections.push(
UserInstructions {
text: user_instructions.to_string(),

5
codex-rs/core/src/guardian/mod.rs
View File

@@ -6,8 +6,9 @@
//! relevant recent assistant and tool context.
//! 2. Ask a dedicated guardian review session to assess the exact planned
//! action and return strict JSON.
//! The guardian clones the parent config, so it inherits any managed
//! network proxy / allowlist that the parent turn already had.
//! The guardian runs with a sanitized review-session config. It may inherit
//! the parent managed-network proxy / allowlist for read-only checks, but it
//! does not inherit parent prompt context.
//! 3. Fail closed on timeout, execution failure, or malformed output.
//! 4. Apply the guardian's explicit allow/deny outcome.

2
codex-rs/core/src/guardian/review.rs
View File

@@ -365,7 +365,7 @@ pub(super) async fn run_guardian_review_session(
model: guardian_model,
reasoning_effort: guardian_reasoning_effort,
reasoning_summary: turn.reasoning_summary,
personality: turn.personality,
personality: None,
external_cancel,
})
.await

118
codex-rs/core/src/guardian/review_session.rs
View File

@@ -1,4 +1,3 @@
use std::collections::HashMap;
use std::future::Future;
use std::path::PathBuf;
use std::sync::Arc;
@@ -31,11 +30,9 @@ use crate::codex::TurnContext;
use crate::codex_delegate::run_codex_thread_interactive;
use crate::config::Config;
use crate::config::Constrained;
use crate::config::ManagedFeatures;
use crate::config::NetworkProxySpec;
use crate::config::Permissions;
use crate::rollout::recorder::RolloutRecorder;
use codex_config::types::McpServerConfig;
use codex_features::Feature;
use codex_model_provider_info::ModelProviderInfo;
@@ -45,6 +42,37 @@ use super::prompt::guardian_policy_prompt;
use super::prompt::guardian_policy_prompt_with_config;
const GUARDIAN_INTERRUPT_DRAIN_TIMEOUT: Duration = Duration::from_secs(5);
const GUARDIAN_BASE_INSTRUCTIONS: &str = concat!(
"You are Codex Guardian, a focused approval-review agent. ",
"Follow the guardian developer policy. ",
"Return only the required final JSON."
);
const GUARDIAN_DISABLED_FEATURES: &[Feature] = &[
Feature::SpawnCsv,
Feature::MultiAgentV2,
Feature::Collab,
Feature::WebSearchRequest,
Feature::WebSearchCached,
Feature::CodeModeOnly,
Feature::CodeMode,
Feature::JsReplToolsOnly,
Feature::JsRepl,
Feature::MemoryTool,
Feature::ChildAgentsMd,
Feature::Apps,
Feature::ToolSearch,
Feature::ToolSuggest,
Feature::Plugins,
Feature::ImageGeneration,
Feature::RequestPermissionsTool,
Feature::ExecPermissionApprovals,
Feature::RequestRule,
Feature::ShellSnapshot,
Feature::ShellZshFork,
Feature::UnifiedExec,
Feature::ApplyPatchFreeform,
Feature::CodexGitCommit,
];
const GUARDIAN_FOLLOWUP_REVIEW_REMINDER: &str = concat!(
"Use prior reviews as context, not binding precedent. ",
"Follow the Workspace Policy. ",
@@ -111,19 +139,7 @@ struct GuardianReviewSessionReuseKey {
model_reasoning_summary: Option<ReasoningSummaryConfig>,
permissions: Permissions,
developer_instructions: Option<String>,
base_instructions: Option<String>,
user_instructions: Option<String>,
compact_prompt: Option<String>,
cwd: PathBuf,
mcp_servers: Constrained<HashMap<String, McpServerConfig>>,
codex_linux_sandbox_exe: Option<PathBuf>,
main_execve_wrapper_exe: Option<PathBuf>,
js_repl_node_path: Option<PathBuf>,
js_repl_node_module_dirs: Vec<PathBuf>,
zsh_path: Option<PathBuf>,
features: ManagedFeatures,
include_apply_patch_tool: bool,
use_experimental_unified_exec_tool: bool,
}
impl GuardianReviewSessionReuseKey {
@@ -138,19 +154,7 @@ impl GuardianReviewSessionReuseKey {
model_reasoning_summary: spawn_config.model_reasoning_summary,
permissions: spawn_config.permissions.clone(),
developer_instructions: spawn_config.developer_instructions.clone(),
base_instructions: spawn_config.base_instructions.clone(),
user_instructions: spawn_config.user_instructions.clone(),
compact_prompt: spawn_config.compact_prompt.clone(),
cwd: spawn_config.cwd.to_path_buf(),
mcp_servers: spawn_config.mcp_servers.clone(),
codex_linux_sandbox_exe: spawn_config.codex_linux_sandbox_exe.clone(),
main_execve_wrapper_exe: spawn_config.main_execve_wrapper_exe.clone(),
js_repl_node_path: spawn_config.js_repl_node_path.clone(),
js_repl_node_module_dirs: spawn_config.js_repl_node_module_dirs.clone(),
zsh_path: spawn_config.zsh_path.clone(),
features: spawn_config.features.clone(),
include_apply_patch_tool: spawn_config.include_apply_patch_tool,
use_experimental_unified_exec_tool: spawn_config.use_experimental_unified_exec_tool,
}
}
}
@@ -643,6 +647,9 @@ pub(crate) fn build_guardian_review_session_config(
let mut guardian_config = parent_config.clone();
guardian_config.model = Some(active_model.to_string());
guardian_config.model_reasoning_effort = reasoning_effort;
guardian_config.personality = None;
guardian_config.base_instructions = Some(GUARDIAN_BASE_INSTRUCTIONS.to_string());
guardian_config.user_instructions = None;
guardian_config.developer_instructions = Some(
parent_config
.guardian_policy_config
@@ -650,6 +657,19 @@ pub(crate) fn build_guardian_review_session_config(
.map(guardian_policy_prompt_with_config)
.unwrap_or_else(guardian_policy_prompt),
);
guardian_config.compact_prompt = None;
guardian_config.include_permissions_instructions = false;
guardian_config.include_apps_instructions = false;
guardian_config.include_environment_context = false;
guardian_config.project_doc_max_bytes = 0;
guardian_config.mcp_servers = Constrained::allow_only(Default::default());
guardian_config.js_repl_node_path = None;
guardian_config.js_repl_node_module_dirs = Vec::new();
guardian_config.zsh_path = None;
guardian_config.main_execve_wrapper_exe = None;
guardian_config.include_apply_patch_tool = false;
guardian_config.use_experimental_unified_exec_tool = false;
guardian_config.permissions.allow_login_shell = false;
guardian_config.permissions.approval_policy = Constrained::allow_only(AskForApproval::Never);
guardian_config.permissions.sandbox_policy =
Constrained::allow_only(SandboxPolicy::new_read_only_policy());
@@ -668,12 +688,12 @@ pub(crate) fn build_guardian_review_session_config(
&SandboxPolicy::new_read_only_policy(),
)?);
}
for feature in [
Feature::SpawnCsv,
Feature::Collab,
Feature::WebSearchRequest,
Feature::WebSearchCached,
] {
disable_guardian_feature_set(&mut guardian_config)?;
Ok(guardian_config)
}
fn disable_guardian_feature_set(guardian_config: &mut Config) -> anyhow::Result<()> {
for &feature in GUARDIAN_DISABLED_FEATURES {
guardian_config.features.disable(feature).map_err(|err| {
anyhow::anyhow!(
"guardian review session could not disable `features.{}`: {err}",
@@ -687,7 +707,7 @@ pub(crate) fn build_guardian_review_session_config(
);
}
}
Ok(guardian_config)
Ok(())
}
async fn run_before_review_deadline<T>(
@@ -777,6 +797,36 @@ mod tests {
);
}
#[test]
fn guardian_review_session_cwd_change_does_not_invalidate_cached_session() {
let mut parent_config = crate::config::test_config();
let cached_spawn_config = build_guardian_review_session_config(
&parent_config,
/*live_network_config*/ None,
"active-model",
/*reasoning_effort*/ None,
)
.expect("cached guardian config");
let cached_reuse_key =
GuardianReviewSessionReuseKey::from_spawn_config(&cached_spawn_config);
parent_config.cwd =
codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path("/tmp/guardian-cwd")
.expect("absolute cwd");
let next_spawn_config = build_guardian_review_session_config(
&parent_config,
/*live_network_config*/ None,
"active-model",
/*reasoning_effort*/ None,
)
.expect("next guardian config");
assert_eq!(
cached_reuse_key,
GuardianReviewSessionReuseKey::from_spawn_config(&next_spawn_config)
);
}
#[tokio::test(flavor = "current_thread")]
async fn run_before_review_deadline_times_out_before_future_completes() {
let outcome = run_before_review_deadline(

18
codex-rs/core/src/guardian/snapshots/codex_core__guardian__tests__guardian_followup_review_request_layout.snap
View File

File diff suppressed because one or more lines are too long

6
codex-rs/core/src/guardian/snapshots/codex_core__guardian__tests__guardian_review_request_layout.snap
View File

File diff suppressed because one or more lines are too long

134
codex-rs/core/src/guardian/tests.rs
View File

@@ -15,7 +15,10 @@ use crate::config_loader::NetworkDomainPermissionsToml;
use crate::config_loader::RequirementSource;
use crate::config_loader::Sourced;
use crate::test_support;
use codex_config::McpServerConfig;
use codex_config::McpServerTransportConfig;
use codex_config::config_toml::ConfigToml;
use codex_features::Feature;
use codex_network_proxy::NetworkProxyConfig;
use codex_protocol::approvals::NetworkApprovalProtocol;
use codex_protocol::config_types::ApprovalsReviewer;
@@ -560,6 +563,13 @@ async fn guardian_review_request_layout_matches_model_visible_request_snapshot()
let mut config = (*turn.config).clone();
config.cwd = temp_cwd.abs();
config.model_provider.base_url = Some(format!("{}/v1", server.uri()));
config.base_instructions = Some("PARENT_BASE_INSTRUCTIONS_SHOULD_NOT_BE_VISIBLE".to_string());
config.user_instructions = Some("PARENT_USER_INSTRUCTIONS_SHOULD_NOT_BE_VISIBLE".to_string());
config.developer_instructions =
Some("PARENT_DEVELOPER_INSTRUCTIONS_SHOULD_NOT_BE_VISIBLE".to_string());
config.include_permissions_instructions = true;
config.include_apps_instructions = true;
config.include_environment_context = true;
let config = Arc::new(config);
let models_manager = Arc::new(test_support::models_manager_with_provider(
config.codex_home.clone(),
@@ -608,6 +618,20 @@ async fn guardian_review_request_layout_matches_model_visible_request_snapshot()
assert_eq!(assessment.outcome, GuardianAssessmentOutcome::Allow);
let request = request_log.single_request();
let request_body = request.body_json();
let request_body_text = request_body.to_string();
for omitted_parent_text in [
"PARENT_BASE_INSTRUCTIONS_SHOULD_NOT_BE_VISIBLE",
"PARENT_USER_INSTRUCTIONS_SHOULD_NOT_BE_VISIBLE",
"PARENT_DEVELOPER_INSTRUCTIONS_SHOULD_NOT_BE_VISIBLE",
"<environment_context>",
"<permissions instructions>",
] {
assert!(
!request_body_text.contains(omitted_parent_text),
"guardian review request should omit inherited parent text: {omitted_parent_text}"
);
}
let mut settings = Settings::clone_current();
settings.set_snapshot_path("snapshots");
settings.set_prepend_module_to_snapshot(false);
@@ -1083,6 +1107,116 @@ fn guardian_review_session_config_overrides_parent_developer_instructions() {
);
}
#[test]
fn guardian_review_session_config_strips_parent_prompt_and_tooling_surface() {
let mut parent_config = test_config();
parent_config.base_instructions = Some("parent base instructions".to_string());
parent_config.user_instructions = Some("parent user instructions".to_string());
parent_config.developer_instructions = Some("parent developer instructions".to_string());
parent_config.compact_prompt = Some("parent compact prompt".to_string());
parent_config.include_permissions_instructions = true;
parent_config.include_apps_instructions = true;
parent_config.include_environment_context = true;
parent_config.project_doc_max_bytes = 4096;
parent_config
.mcp_servers
.set(std::collections::HashMap::from([(
"parent-mcp".to_string(),
McpServerConfig {
transport: McpServerTransportConfig::Stdio {
command: "parent-mcp".to_string(),
args: Vec::new(),
env: None,
env_vars: Vec::new(),
cwd: None,
},
enabled: true,
required: false,
disabled_reason: None,
startup_timeout_sec: None,
tool_timeout_sec: None,
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth_resource: None,
tools: std::collections::HashMap::new(),
},
)]))
.expect("set parent mcp servers");
parent_config.js_repl_node_path = Some(PathBuf::from("/parent/node"));
parent_config.js_repl_node_module_dirs = vec![PathBuf::from("/parent/node_modules")];
parent_config.zsh_path = Some(PathBuf::from("/parent/zsh"));
parent_config.main_execve_wrapper_exe = Some(PathBuf::from("/parent/execve-wrapper"));
parent_config.include_apply_patch_tool = true;
parent_config.use_experimental_unified_exec_tool = true;
parent_config
.features
.enable(Feature::JsRepl)
.expect("enable js repl");
parent_config
.features
.enable(Feature::CodeMode)
.expect("enable code mode");
parent_config
.features
.enable(Feature::MemoryTool)
.expect("enable memory");
parent_config
.features
.enable(Feature::Collab)
.expect("enable collab");
parent_config
.features
.enable(Feature::SpawnCsv)
.expect("enable spawn csv");
parent_config
.features
.enable(Feature::ApplyPatchFreeform)
.expect("enable apply patch");
let guardian_config = build_guardian_review_session_config_for_test(
&parent_config,
/*live_network_config*/ None,
"active-model",
/*reasoning_effort*/ None,
)
.expect("guardian config");
assert_eq!(
guardian_config.base_instructions.as_deref(),
Some(
"You are Codex Guardian, a focused approval-review agent. Follow the guardian developer policy. Return only the required final JSON."
)
);
assert_eq!(guardian_config.user_instructions, None);
assert_eq!(guardian_config.compact_prompt, None);
assert!(!guardian_config.include_permissions_instructions);
assert!(!guardian_config.include_apps_instructions);
assert!(!guardian_config.include_environment_context);
assert_eq!(guardian_config.project_doc_max_bytes, 0);
assert!(guardian_config.mcp_servers.is_empty());
assert_eq!(guardian_config.js_repl_node_path, None);
assert_eq!(
guardian_config.js_repl_node_module_dirs,
Vec::<PathBuf>::new()
);
assert_eq!(guardian_config.zsh_path, None);
assert_eq!(guardian_config.main_execve_wrapper_exe, None);
assert!(!guardian_config.include_apply_patch_tool);
assert!(!guardian_config.use_experimental_unified_exec_tool);
assert!(!guardian_config.permissions.allow_login_shell);
for disabled_feature in [
Feature::JsRepl,
Feature::CodeMode,
Feature::MemoryTool,
Feature::Collab,
Feature::SpawnCsv,
Feature::ApplyPatchFreeform,
] {
assert!(!guardian_config.features.enabled(disabled_feature));
}
}
#[test]
fn guardian_review_session_config_uses_live_network_proxy_state() {
let mut parent_config = test_config();