Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions codex-rs/app-server-protocol/src/protocol/v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ impl From<CoreAskForApproval> for AskForApproval {
pub enum ApprovalsReviewer {
#[serde(rename = "user")]
User,
#[serde(rename = "auto_review", alias = "guardian_subagent")]
#[serde(rename = "guardian_subagent", alias = "auto_review")]
AutoReview,
}

Expand Down Expand Up @@ -7483,7 +7483,7 @@ mod tests {
);
assert_eq!(
serde_json::to_string(&ApprovalsReviewer::AutoReview).expect("serialize reviewer"),
"\"auto_review\""
"\"guardian_subagent\""
);

for value in ["user", "auto_review", "guardian_subagent"] {
Expand Down
22 changes: 22 additions & 0 deletions codex-rs/core/src/config/config_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6610,6 +6610,28 @@ async fn feature_requirements_normalize_effective_feature_values() -> std::io::R
Ok(())
}

#[tokio::test]
async fn feature_requirements_auto_review_disables_guardian_approval() -> std::io::Result<()> {
let codex_home = TempDir::new()?;

let config = ConfigBuilder::without_managed_config_for_tests()
.codex_home(codex_home.path().to_path_buf())
.cloud_requirements(CloudRequirementsLoader::new(async {
Ok(Some(crate::config_loader::ConfigRequirementsToml {
feature_requirements: Some(crate::config_loader::FeatureRequirementsToml {
entries: BTreeMap::from([("auto_review".to_string(), false)]),
}),
..Default::default()
}))
}))
.build()
.await?;

assert!(!config.features.enabled(Feature::GuardianApproval));

Ok(())
}

#[tokio::test]
async fn browser_feature_requirements_are_valid() -> std::io::Result<()> {
let codex_home = TempDir::new()?;
Expand Down
5 changes: 5 additions & 0 deletions codex-rs/core/src/config/managed_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,11 @@ fn parse_feature_requirements(
) -> BTreeMap<Feature, bool> {
let mut pinned_features = BTreeMap::new();
for (key, enabled) in feature_requirements.entries {
if key == "auto_review" {
pinned_features.insert(Feature::GuardianApproval, enabled);
continue;
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this specifically needed?


if let Some(feature) = canonical_feature_for_key(&key) {
pinned_features.insert(feature, enabled);
continue;
Expand Down
6 changes: 3 additions & 3 deletions codex-rs/protocol/src/config_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ pub enum ApprovalsReviewer {
#[default]
#[serde(rename = "user")]
User,
#[serde(rename = "auto_review", alias = "guardian_subagent")]
#[strum(serialize = "auto_review")]
#[serde(rename = "guardian_subagent", alias = "auto_review")]
#[strum(serialize = "guardian_subagent")]
AutoReview,
}

Expand Down Expand Up @@ -609,7 +609,7 @@ mod tests {
);
assert_eq!(
serde_json::to_string(&ApprovalsReviewer::AutoReview).expect("serialize reviewer"),
"\"auto_review\""
"\"guardian_subagent\""
);

for value in ["user", "auto_review", "guardian_subagent"] {
Expand Down
6 changes: 3 additions & 3 deletions codex-rs/tui/src/app/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1675,7 +1675,7 @@ async fn update_feature_flags_enabling_guardian_selects_auto_review() -> Result<

let config = std::fs::read_to_string(codex_home.path().join("config.toml"))?;
assert!(config.contains("guardian_approval = true"));
assert!(config.contains("approvals_reviewer = \"auto_review\""));
assert!(config.contains("approvals_reviewer = \"guardian_subagent\""));
assert!(config.contains("approval_policy = \"on-request\""));
assert!(config.contains("sandbox_mode = \"workspace-write\""));
Ok(())
Expand Down Expand Up @@ -1835,7 +1835,7 @@ async fn update_feature_flags_enabling_guardian_overrides_explicit_manual_review
);

let config = std::fs::read_to_string(codex_home.path().join("config.toml"))?;
assert!(config.contains("approvals_reviewer = \"auto_review\""));
assert!(config.contains("approvals_reviewer = \"guardian_subagent\""));
assert!(config.contains("guardian_approval = true"));
assert!(config.contains("approval_policy = \"on-request\""));
assert!(config.contains("sandbox_mode = \"workspace-write\""));
Expand Down Expand Up @@ -1969,7 +1969,7 @@ async fn update_feature_flags_enabling_guardian_in_profile_sets_profile_auto_rev
);
assert_eq!(
profile_config.get("approvals_reviewer"),
Some(&TomlValue::String("auto_review".to_string()))
Some(&TomlValue::String("guardian_subagent".to_string()))
);
Ok(())
}
Expand Down
5 changes: 3 additions & 2 deletions codex-rs/tui/src/app/thread_session_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ mod tests {
approval_policy: AskForApproval::Never,
approvals_reviewer: ApprovalsReviewer::User,
sandbox_policy: SandboxPolicy::new_read_only_policy(),
permission_profile: None,
cwd: cwd.abs(),
instruction_source_paths: Vec::new(),
reasoning_effort: None,
Expand Down Expand Up @@ -155,7 +156,7 @@ mod tests {
.insert(side_thread_id, SideThreadState::new(main_thread_id));
app.config.permissions.approval_policy =
codex_config::Constrained::allow_any(AskForApproval::OnRequest);
app.config.approvals_reviewer = ApprovalsReviewer::GuardianSubagent;
app.config.approvals_reviewer = ApprovalsReviewer::AutoReview;
app.config.permissions.sandbox_policy =
codex_config::Constrained::allow_any(SandboxPolicy::new_workspace_write_policy());

Expand All @@ -164,7 +165,7 @@ mod tests {

let expected_main_session = ThreadSessionState {
approval_policy: AskForApproval::OnRequest,
approvals_reviewer: ApprovalsReviewer::GuardianSubagent,
approvals_reviewer: ApprovalsReviewer::AutoReview,
sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
..main_session
};
Expand Down
4 changes: 2 additions & 2 deletions codex-rs/tui/src/debug_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,7 @@ mod tests {
rendered.contains("allowed_approval_policies: on-request (source: cloud requirements)")
);
assert!(rendered.contains(
"allowed_approvals_reviewers: auto_review (source: MDM managed_config.toml (legacy))"
"allowed_approvals_reviewers: guardian_subagent (source: MDM managed_config.toml (legacy))"
));
assert!(
rendered.contains(
Expand Down Expand Up @@ -779,7 +779,7 @@ mod tests {

let rendered = render_to_text(&render_debug_config_lines(&stack));
assert!(rendered.contains(
"allowed_approvals_reviewers: auto_review (source: MDM managed_config.toml (legacy))"
"allowed_approvals_reviewers: guardian_subagent (source: MDM managed_config.toml (legacy))"
));
assert!(!rendered.contains("Requirements:\n <none>"));
}
Expand Down
1 change: 1 addition & 0 deletions sdk/python/src/codex_app_server/generated/v2_all.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ class AppToolsConfig(BaseModel):

class ApprovalsReviewer(Enum):
user = "user"
auto_review = "auto_review"
guardian_subagent = "guardian_subagent"


Expand Down
39 changes: 37 additions & 2 deletions sdk/python/tests/test_client_rpc_methods.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@
from typing import Any

from codex_app_server.client import AppServerClient, _params_dict
from codex_app_server.generated.v2_all import ThreadListParams, ThreadTokenUsageUpdatedNotification
from codex_app_server.generated.v2_all import (
ApprovalsReviewer,
ThreadListParams,
ThreadResumeResponse,
ThreadTokenUsageUpdatedNotification,
)
from codex_app_server.models import UnknownNotification

ROOT = Path(__file__).resolve().parents[1]
Expand Down Expand Up @@ -40,6 +45,34 @@ def test_generated_v2_bundle_has_single_shared_plan_type_definition() -> None:
assert source.count("class PlanType(") == 1


def test_thread_resume_response_accepts_auto_review_reviewer() -> None:
response = ThreadResumeResponse.model_validate(
{
"approvalPolicy": "on-request",
"approvalsReviewer": "auto_review",
"cwd": "/tmp",
"model": "gpt-5",
"modelProvider": "openai",
"sandbox": {"type": "dangerFullAccess"},
"thread": {
"cliVersion": "1.0.0",
"createdAt": 1,
"cwd": "/tmp",
"ephemeral": False,
"id": "thread-1",
"modelProvider": "openai",
"preview": "",
"source": "cli",
"status": {"type": "idle"},
"turns": [],
"updatedAt": 1,
},
}
)

assert response.approvals_reviewer is ApprovalsReviewer.auto_review


def test_notifications_are_typed_with_canonical_v2_methods() -> None:
client = AppServerClient()
event = client._coerce_notification(
Expand Down Expand Up @@ -89,7 +122,9 @@ def test_unknown_notifications_fall_back_to_unknown_payloads() -> None:

def test_invalid_notification_payload_falls_back_to_unknown() -> None:
client = AppServerClient()
event = client._coerce_notification("thread/tokenUsage/updated", {"threadId": "missing"})
event = client._coerce_notification(
"thread/tokenUsage/updated", {"threadId": "missing"}
)

assert event.method == "thread/tokenUsage/updated"
assert isinstance(event.payload, UnknownNotification)
Loading