mirror of
https://github.com/openai/codex.git
synced 2026-05-02 10:26:45 +00:00
Rebrand approvals reviewer config to auto-review (#18504)
### Why Auto-review is the user-facing name for the approvals reviewer, but the config/API value still exposed the old `guardian_subagent` name. That made new configs and generated schemas point users at Guardian terminology even though the intended product surface is Auto-review. This PR updates the external `approvals_reviewer` value while preserving compatibility for existing configs and clients. ### What changed - Makes `auto_review` the canonical serialized value for `approvals_reviewer`. - Keeps `guardian_subagent` accepted as a legacy alias. - Keeps `user` accepted and serialized as `user`. - Updates generated config and app-server schemas so `approvals_reviewer` includes: - `user` - `auto_review` - `guardian_subagent` - Updates app-server README docs for the reviewer value. - Updates analytics and config requirements tests for the canonical auto_review value. ### Compatibility Existing configs and API payloads using: ```toml approvals_reviewer = "guardian_subagent" ``` continue to load and map to the Auto-review reviewer behavior. New serialization emits: ```toml approvals_reviewer = "auto_review" ``` This PR intentionally does not rename the [features].guardian_approval key or broad internal Guardian symbols. Those are split out for a follow-up PR to keep this migration small and avoid touching large TUI/internal surfaces. **Verification** cargo test -p codex-protocol approvals_reviewer_serializes_auto_review_and_accepts_legacy_guardian_subagent cargo test -p codex-app-server-protocol approvals_reviewer_serializes_auto_review_and_accepts_legacy_guardian_subagent
This commit is contained in:
@@ -1,7 +1,13 @@
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use schemars::JsonSchema;
|
||||
use schemars::r#gen::SchemaGenerator;
|
||||
use schemars::schema::InstanceType;
|
||||
use schemars::schema::Metadata;
|
||||
use schemars::schema::Schema;
|
||||
use schemars::schema::SchemaObject;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use serde_json::Value;
|
||||
use std::num::NonZeroU64;
|
||||
use std::time::Duration;
|
||||
use strum_macros::Display;
|
||||
@@ -69,22 +75,54 @@ pub enum SandboxMode {
|
||||
DangerFullAccess,
|
||||
}
|
||||
|
||||
#[derive(
|
||||
Deserialize, Debug, Clone, Copy, PartialEq, Eq, Default, Serialize, Display, JsonSchema, TS,
|
||||
)]
|
||||
#[serde(rename_all = "snake_case")]
|
||||
#[derive(Deserialize, Debug, Clone, Copy, PartialEq, Eq, Default, Serialize, Display, TS)]
|
||||
#[strum(serialize_all = "snake_case")]
|
||||
#[ts(type = r#""user" | "auto_review" | "guardian_subagent""#)]
|
||||
/// Configures who approval requests are routed to for review. Examples
|
||||
/// include sandbox escapes, blocked network access, MCP approval prompts, and
|
||||
/// ARC escalations. Defaults to `user`. `guardian_subagent` uses a carefully
|
||||
/// ARC escalations. Defaults to `user`. `auto_review` uses a carefully
|
||||
/// prompted subagent to gather relevant context and apply a risk-based
|
||||
/// decision framework before approving or denying the request.
|
||||
pub enum ApprovalsReviewer {
|
||||
#[default]
|
||||
#[serde(rename = "user")]
|
||||
User,
|
||||
#[serde(rename = "auto_review", alias = "guardian_subagent")]
|
||||
#[strum(serialize = "auto_review")]
|
||||
GuardianSubagent,
|
||||
}
|
||||
|
||||
impl JsonSchema for ApprovalsReviewer {
|
||||
fn schema_name() -> String {
|
||||
"ApprovalsReviewer".to_string()
|
||||
}
|
||||
|
||||
fn json_schema(_generator: &mut SchemaGenerator) -> Schema {
|
||||
string_enum_schema_with_description(
|
||||
&["user", "auto_review", "guardian_subagent"],
|
||||
"Configures who approval requests are routed to for review. Examples include sandbox escapes, blocked network access, MCP approval prompts, and ARC escalations. Defaults to `user`. `auto_review` uses a carefully prompted subagent to gather relevant context and apply a risk-based decision framework before approving or denying the request. The legacy value `guardian_subagent` is accepted for compatibility.",
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
fn string_enum_schema_with_description(values: &[&str], description: &str) -> Schema {
|
||||
let mut schema = SchemaObject {
|
||||
instance_type: Some(InstanceType::String.into()),
|
||||
metadata: Some(Box::new(Metadata {
|
||||
description: Some(description.to_string()),
|
||||
..Default::default()
|
||||
})),
|
||||
..Default::default()
|
||||
};
|
||||
schema.enum_values = Some(
|
||||
values
|
||||
.iter()
|
||||
.map(|value| Value::String((*value).to_string()))
|
||||
.collect(),
|
||||
);
|
||||
Schema::Object(schema)
|
||||
}
|
||||
|
||||
#[derive(
|
||||
Deserialize, Debug, Clone, Copy, PartialEq, Eq, Default, Serialize, Display, JsonSchema, TS,
|
||||
)]
|
||||
@@ -562,6 +600,32 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn approvals_reviewer_serializes_auto_review_and_accepts_legacy_guardian_subagent() {
|
||||
assert_eq!(ApprovalsReviewer::User.to_string(), "user");
|
||||
assert_eq!(
|
||||
serde_json::to_string(&ApprovalsReviewer::User).expect("serialize reviewer"),
|
||||
"\"user\""
|
||||
);
|
||||
assert_eq!(
|
||||
serde_json::to_string(&ApprovalsReviewer::GuardianSubagent)
|
||||
.expect("serialize reviewer"),
|
||||
"\"auto_review\""
|
||||
);
|
||||
|
||||
for value in ["user", "auto_review", "guardian_subagent"] {
|
||||
let json = format!("\"{value}\"");
|
||||
let reviewer: ApprovalsReviewer =
|
||||
serde_json::from_str(&json).expect("deserialize reviewer");
|
||||
let expected = if value == "user" {
|
||||
ApprovalsReviewer::User
|
||||
} else {
|
||||
ApprovalsReviewer::GuardianSubagent
|
||||
};
|
||||
assert_eq!(expected, reviewer);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tui_visible_collaboration_modes_match_mode_kind_visibility() {
|
||||
let expected = [ModeKind::Default, ModeKind::Plan];
|
||||
|
||||
Reference in New Issue
Block a user