agent/codex
2
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(debug-config, guardian): fix /debug-config rendering and guardian… (#17138)

Browse Source 
## Description

This PR fixes `/debug-config` so it shows more of the active
requirements state, including reviewer requirements and managed feature
pins. This made it clear that legacy MDM config was setting
`approvals_reviewer = "guardian_subagent"` and that we were translating
that into a requirements constraint.

Also, translate `approvals_reviewer = "guardian_subagent"` (from legacy
managed_config.toml) to `allowed_approvals_reviewers: guardian_subagent,
user` instead of `allowed_approvals_reviewers: guardian_subagent`.

Example `/debug-config`:
```
Config layer stack (lowest precedence first):
  1. system (/etc/codex/config.toml) (enabled)
  2. user (/Users/owen/.codex/config.toml) (enabled)
  3. project (/Users/owen/repos/codex/.codex/config.toml) (enabled)
  4. legacy managed_config.toml (MDM) (enabled)
     MDM value:
       ...

       # Enable Guardian Mode
       features.guardian_approval = true
       approvals_reviewer = "guardian_subagent"

Requirements:
  - allowed_approvals_reviewers: guardian_subagent, user (source: MDM managed_config.toml (legacy))
  - features: apps=true, plugins=true (source: cloud requirements)
```

Before this PR, the `Requirements` section showed None.
This commit is contained in:
Owen Lin
2026-04-08 11:08:09 -07:00
committed by GitHub
Unverified
parent 35b5720e8d
commit e794457a59
2 changed files with 102 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/config_loader/mod.rs
+29 -5
View File
@@ -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<AskForApproval>,
@@ -899,7 +900,11 @@ impl From<LegacyManagedConfigToml> 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])
);
}
codex-rs/tui/src/debug_config.rs
+73 -2
View File
@@ -100,6 +100,20 @@ fn render_debug_config_lines(stack: &ConfigLayerStack) -> Vec<Line<'static>> {
));
}
if let Some(reviewers) = requirements_toml.allowed_approvals_reviewers.as_ref() {
let value = join_or_empty(
reviewers
.iter()
.map(ToString::to_string)
.collect::<Vec<_>>(),
);
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<Line<'static>> {
));
}
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::<Vec<_>>(),
);
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::<Vec<_>>());
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 <none>"));
}
#[test]
fn debug_config_output_formats_unix_socket_permissions() {
let requirements = ConfigRequirements {