diff --git a/codex-rs/core/src/config_loader/mod.rs b/codex-rs/core/src/config_loader/mod.rs index f40482fd5..36fc956bf 100644 --- a/codex-rs/core/src/config_loader/mod.rs +++ b/codex-rs/core/src/config_loader/mod.rs @@ -877,8 +877,9 @@ async fn load_project_layers( /// exactly one value rather than a list of allowed values. /// /// If present, re-interpret `managed_config.toml` as a `requirements.toml` -/// where each specified field is treated as a constraint allowing only that -/// value. +/// where each specified field is treated as a constraint. Most fields allow +/// only the specified value. `approvals_reviewer = "guardian_subagent"` also +/// allows `user` so people can opt out of the guardian reviewer. #[derive(Deserialize, Debug, Clone, Default, PartialEq)] struct LegacyManagedConfigToml { approval_policy: Option, @@ -899,7 +900,11 @@ impl From for ConfigRequirementsToml { config_requirements_toml.allowed_approval_policies = Some(vec![approval_policy]); } if let Some(approvals_reviewer) = approvals_reviewer { - config_requirements_toml.allowed_approvals_reviewers = Some(vec![approvals_reviewer]); + let mut allowed_reviewers = vec![approvals_reviewer]; + if approvals_reviewer == ApprovalsReviewer::GuardianSubagent { + allowed_reviewers.push(ApprovalsReviewer::User); + } + config_requirements_toml.allowed_approvals_reviewers = Some(allowed_reviewers); } if let Some(sandbox_mode) = sandbox_mode { let required_mode: SandboxModeRequirement = sandbox_mode.into(); @@ -980,7 +985,7 @@ foo = "xyzzy" } #[test] - fn legacy_managed_config_backfill_includes_approvals_reviewer() { + fn legacy_managed_config_backfill_allows_user_when_guardian_is_required() { let legacy = LegacyManagedConfigToml { approval_policy: None, approvals_reviewer: Some(ApprovalsReviewer::GuardianSubagent), @@ -991,7 +996,26 @@ foo = "xyzzy" assert_eq!( requirements.allowed_approvals_reviewers, - Some(vec![ApprovalsReviewer::GuardianSubagent]) + Some(vec![ + ApprovalsReviewer::GuardianSubagent, + ApprovalsReviewer::User + ]) + ); + } + + #[test] + fn legacy_managed_config_backfill_preserves_user_only_approvals_reviewer() { + let legacy = LegacyManagedConfigToml { + approval_policy: None, + approvals_reviewer: Some(ApprovalsReviewer::User), + sandbox_mode: None, + }; + + let requirements = ConfigRequirementsToml::from(legacy); + + assert_eq!( + requirements.allowed_approvals_reviewers, + Some(vec![ApprovalsReviewer::User]) ); } diff --git a/codex-rs/tui/src/debug_config.rs b/codex-rs/tui/src/debug_config.rs index bc7230497..c9294a4ec 100644 --- a/codex-rs/tui/src/debug_config.rs +++ b/codex-rs/tui/src/debug_config.rs @@ -100,6 +100,20 @@ fn render_debug_config_lines(stack: &ConfigLayerStack) -> Vec> { )); } + if let Some(reviewers) = requirements_toml.allowed_approvals_reviewers.as_ref() { + let value = join_or_empty( + reviewers + .iter() + .map(ToString::to_string) + .collect::>(), + ); + requirement_lines.push(requirement_line( + "allowed_approvals_reviewers", + value, + requirements.approvals_reviewer.source.as_ref(), + )); + } + if let Some(modes) = requirements_toml.allowed_sandbox_modes.as_ref() { let value = join_or_empty( modes @@ -130,6 +144,22 @@ fn render_debug_config_lines(stack: &ConfigLayerStack) -> Vec> { )); } + if let Some(feature_requirements) = requirements.feature_requirements.as_ref() { + let value = join_or_empty( + feature_requirements + .value + .entries + .iter() + .map(|(feature, enabled)| format!("{feature}={enabled}")) + .collect::>(), + ); + requirement_lines.push(requirement_line( + "features", + value, + Some(&feature_requirements.source), + )); + } + if let Some(servers) = requirements_toml.mcp_servers.as_ref() { let value = join_or_empty(servers.keys().cloned().collect::>()); requirement_lines.push(requirement_line( @@ -434,6 +464,7 @@ mod tests { use codex_core::config_loader::ConfigRequirements; use codex_core::config_loader::ConfigRequirementsToml; use codex_core::config_loader::ConstrainedWithSource; + use codex_core::config_loader::FeatureRequirementsToml; use codex_core::config_loader::McpServerIdentity; use codex_core::config_loader::McpServerRequirement; use codex_core::config_loader::NetworkConstraints; @@ -446,6 +477,7 @@ mod tests { use codex_core::config_loader::SandboxModeRequirement; use codex_core::config_loader::Sourced; use codex_core::config_loader::WebSearchModeRequirement; + use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::config_types::WebSearchMode; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SandboxPolicy; @@ -529,6 +561,10 @@ mod tests { Constrained::allow_any(AskForApproval::OnRequest), Some(RequirementSource::CloudRequirements), ), + approvals_reviewer: ConstrainedWithSource::new( + Constrained::allow_any(ApprovalsReviewer::GuardianSubagent), + Some(RequirementSource::LegacyManagedConfigTomlFromMdm), + ), sandbox_policy: ConstrainedWithSource::new( Constrained::allow_any(SandboxPolicy::new_read_only_policy()), Some(RequirementSource::SystemRequirementsToml { @@ -554,6 +590,12 @@ mod tests { Constrained::allow_any(WebSearchMode::Cached), Some(RequirementSource::CloudRequirements), ), + feature_requirements: Some(Sourced::new( + FeatureRequirementsToml { + entries: BTreeMap::from([("guardian_approval".to_string(), true)]), + }, + RequirementSource::CloudRequirements, + )), network: Some(Sourced::new( NetworkConstraints { enabled: Some(true), @@ -573,11 +615,13 @@ mod tests { let requirements_toml = ConfigRequirementsToml { allowed_approval_policies: Some(vec![AskForApproval::OnRequest]), - allowed_approvals_reviewers: None, + allowed_approvals_reviewers: Some(vec![ApprovalsReviewer::GuardianSubagent]), allowed_sandbox_modes: Some(vec![SandboxModeRequirement::ReadOnly]), allowed_web_search_modes: Some(vec![WebSearchModeRequirement::Cached]), guardian_developer_instructions: None, - feature_requirements: None, + feature_requirements: Some(FeatureRequirementsToml { + entries: BTreeMap::from([("guardian_approval".to_string(), true)]), + }), mcp_servers: Some(BTreeMap::from([( "docs".to_string(), McpServerRequirement { @@ -611,6 +655,9 @@ mod tests { assert!( rendered.contains("allowed_approval_policies: on-request (source: cloud requirements)") ); + assert!(rendered.contains( + "allowed_approvals_reviewers: guardian_subagent (source: MDM managed_config.toml (legacy))" + )); assert!( rendered.contains( format!( @@ -625,6 +672,7 @@ mod tests { "allowed_web_search_modes: cached, disabled (source: cloud requirements)" ) ); + assert!(rendered.contains("features: guardian_approval=true (source: cloud requirements)")); assert!(rendered.contains("mcp_servers: docs (source: MDM managed_config.toml (legacy))")); assert!(rendered.contains("enforce_residency: us (source: cloud requirements)")); assert!(rendered.contains( @@ -633,6 +681,29 @@ mod tests { assert!(!rendered.contains(" - rules:")); } + #[test] + fn debug_config_output_lists_approvals_reviewer_as_requirement() { + let requirements = ConfigRequirements { + approvals_reviewer: ConstrainedWithSource::new( + Constrained::allow_any(ApprovalsReviewer::GuardianSubagent), + Some(RequirementSource::LegacyManagedConfigTomlFromMdm), + ), + ..ConfigRequirements::default() + }; + let requirements_toml = ConfigRequirementsToml { + allowed_approvals_reviewers: Some(vec![ApprovalsReviewer::GuardianSubagent]), + ..ConfigRequirementsToml::default() + }; + let stack = ConfigLayerStack::new(Vec::new(), requirements, requirements_toml) + .expect("config layer stack"); + + let rendered = render_to_text(&render_debug_config_lines(&stack)); + assert!(rendered.contains( + "allowed_approvals_reviewers: guardian_subagent (source: MDM managed_config.toml (legacy))" + )); + assert!(!rendered.contains("Requirements:\n ")); + } + #[test] fn debug_config_output_formats_unix_socket_permissions() { let requirements = ConfigRequirements {