mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat(auto-review) policy config (#18959)
## Summary Allow users to customize their own auto-review policy config. ## Testing - [x] added config_tests
This commit is contained in:
committed by
GitHub
Unverified
parent
f67383bcba
commit
78593d72ea
@@ -90,6 +90,10 @@ pub struct ConfigToml {
|
||||
/// ARC.
|
||||
pub approvals_reviewer: Option<ApprovalsReviewer>,
|
||||
|
||||
/// Optional policy instructions for the guardian auto-reviewer.
|
||||
#[serde(default)]
|
||||
pub auto_review: Option<AutoReviewToml>,
|
||||
|
||||
#[serde(default)]
|
||||
pub shell_environment_policy: ShellEnvironmentPolicyToml,
|
||||
|
||||
@@ -401,6 +405,12 @@ pub struct ConfigToml {
|
||||
pub oss_provider: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)]
|
||||
pub struct AutoReviewToml {
|
||||
/// Additional policy instructions inserted into the guardian prompt.
|
||||
pub policy: Option<String>,
|
||||
}
|
||||
|
||||
impl From<ConfigToml> for UserSavedConfig {
|
||||
fn from(config_toml: ConfigToml) -> Self {
|
||||
let profiles = config_toml
|
||||
|
||||
@@ -292,6 +292,15 @@
|
||||
}
|
||||
]
|
||||
},
|
||||
"AutoReviewToml": {
|
||||
"properties": {
|
||||
"policy": {
|
||||
"description": "Additional policy instructions inserted into the guardian prompt.",
|
||||
"type": "string"
|
||||
}
|
||||
},
|
||||
"type": "object"
|
||||
},
|
||||
"BundledSkillsConfig": {
|
||||
"additionalProperties": false,
|
||||
"properties": {
|
||||
@@ -2253,6 +2262,15 @@
|
||||
"default": null,
|
||||
"description": "Machine-local realtime audio device preferences used by realtime voice."
|
||||
},
|
||||
"auto_review": {
|
||||
"allOf": [
|
||||
{
|
||||
"$ref": "#/definitions/AutoReviewToml"
|
||||
}
|
||||
],
|
||||
"default": null,
|
||||
"description": "Optional policy instructions for the guardian auto-reviewer."
|
||||
},
|
||||
"background_terminal_max_timeout": {
|
||||
"description": "Maximum poll window for background terminal output (`write_stdin`), in milliseconds. Default: `300000` (5 minutes).",
|
||||
"format": "uint64",
|
||||
|
||||
@@ -10,6 +10,7 @@ use assert_matches::assert_matches;
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
use codex_config::config_toml::AgentRoleToml;
|
||||
use codex_config::config_toml::AgentsToml;
|
||||
use codex_config::config_toml::AutoReviewToml;
|
||||
use codex_config::config_toml::ConfigToml;
|
||||
use codex_config::config_toml::ProjectConfig;
|
||||
use codex_config::config_toml::RealtimeAudioConfig;
|
||||
@@ -3706,6 +3707,116 @@ async fn load_config_uses_requirements_guardian_policy_config() -> std::io::Resu
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn config_toml_deserializes_auto_review_policy() {
|
||||
let cfg = toml::from_str::<ConfigToml>(
|
||||
r#"
|
||||
[auto_review]
|
||||
policy = "Use the user-configured guardian policy."
|
||||
"#,
|
||||
)
|
||||
.expect("TOML deserialization should succeed");
|
||||
|
||||
assert_eq!(
|
||||
cfg.auto_review
|
||||
.as_ref()
|
||||
.and_then(|auto_review| auto_review.policy.as_deref()),
|
||||
Some("Use the user-configured guardian policy.")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn load_config_uses_auto_review_guardian_policy_config() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let cfg = ConfigToml {
|
||||
auto_review: Some(AutoReviewToml {
|
||||
policy: Some(" Use the user-configured guardian policy. ".to_string()),
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let config = Config::load_from_base_config_with_overrides(
|
||||
cfg,
|
||||
ConfigOverrides {
|
||||
cwd: Some(codex_home.path().to_path_buf()),
|
||||
..Default::default()
|
||||
},
|
||||
codex_home.abs(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
config.guardian_policy_config.as_deref(),
|
||||
Some("Use the user-configured guardian policy.")
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn requirements_guardian_policy_beats_auto_review() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let config_layer_stack = ConfigLayerStack::new(
|
||||
Vec::new(),
|
||||
Default::default(),
|
||||
crate::config_loader::ConfigRequirementsToml {
|
||||
guardian_policy_config: Some("Use the managed guardian policy.".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.map_err(std::io::Error::other)?;
|
||||
let cfg = ConfigToml {
|
||||
auto_review: Some(AutoReviewToml {
|
||||
policy: Some("Use the user-configured guardian policy.".to_string()),
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let config = Config::load_config_with_layer_stack(
|
||||
LOCAL_FS.as_ref(),
|
||||
cfg,
|
||||
ConfigOverrides {
|
||||
cwd: Some(codex_home.path().to_path_buf()),
|
||||
..Default::default()
|
||||
},
|
||||
codex_home.abs(),
|
||||
config_layer_stack,
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
config.guardian_policy_config.as_deref(),
|
||||
Some("Use the managed guardian policy.")
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn load_config_ignores_empty_auto_review_guardian_policy_config() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let cfg = ConfigToml {
|
||||
auto_review: Some(AutoReviewToml {
|
||||
policy: Some(" ".to_string()),
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let config = Config::load_from_base_config_with_overrides(
|
||||
cfg,
|
||||
ConfigOverrides {
|
||||
cwd: Some(codex_home.path().to_path_buf()),
|
||||
..Default::default()
|
||||
},
|
||||
codex_home.abs(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(config.guardian_policy_config, None);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn load_config_ignores_empty_requirements_guardian_policy_config() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
|
||||
@@ -293,7 +293,7 @@ pub struct Config {
|
||||
/// Developer instructions override injected as a separate message.
|
||||
pub developer_instructions: Option<String>,
|
||||
|
||||
/// Guardian-specific tenant policy config override from requirements.toml.
|
||||
/// Guardian-specific policy config override from requirements.toml or config.toml.
|
||||
/// This is inserted into the fixed guardian prompt template under the
|
||||
/// `# Policy Configuration` section rather than replacing the whole
|
||||
/// guardian developer prompt.
|
||||
@@ -2026,7 +2026,14 @@ impl Config {
|
||||
.or(cfg.include_environment_context)
|
||||
.unwrap_or(true);
|
||||
let guardian_policy_config =
|
||||
guardian_policy_config_from_requirements(config_layer_stack.requirements_toml());
|
||||
guardian_policy_config_from_requirements(config_layer_stack.requirements_toml())
|
||||
.or_else(|| {
|
||||
cfg.auto_review
|
||||
.as_ref()
|
||||
.and_then(|auto_review| normalize_guardian_policy_config(
|
||||
auto_review.policy.as_deref(),
|
||||
))
|
||||
});
|
||||
let personality = personality
|
||||
.or(config_profile.personality)
|
||||
.or(cfg.personality)
|
||||
@@ -2490,13 +2497,14 @@ pub(crate) fn uses_deprecated_instructions_file(config_layer_stack: &ConfigLayer
|
||||
fn guardian_policy_config_from_requirements(
|
||||
requirements_toml: &ConfigRequirementsToml,
|
||||
) -> Option<String> {
|
||||
requirements_toml
|
||||
.guardian_policy_config
|
||||
.as_deref()
|
||||
.and_then(|value| {
|
||||
let trimmed = value.trim();
|
||||
(!trimmed.is_empty()).then(|| trimmed.to_string())
|
||||
})
|
||||
normalize_guardian_policy_config(requirements_toml.guardian_policy_config.as_deref())
|
||||
}
|
||||
|
||||
fn normalize_guardian_policy_config(value: Option<&str>) -> Option<String> {
|
||||
value.and_then(|value| {
|
||||
let trimmed = value.trim();
|
||||
(!trimmed.is_empty()).then(|| trimmed.to_string())
|
||||
})
|
||||
}
|
||||
|
||||
fn toml_uses_deprecated_instructions_file(value: &TomlValue) -> bool {
|
||||
|
||||
Reference in New Issue
Block a user