Skip to content
Closed
33 changes: 32 additions & 1 deletion codex-rs/app-server/src/config_manager_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@ use codex_config::ConfigLayerStack;
use codex_config::ConfigLayerStackOrdering;
use codex_config::ConfigRequirementsToml;
use codex_config::config_toml::ConfigToml;
use codex_config::configured_app_approvals_reviewers;
use codex_config::merge_toml_values;
use codex_config::resolve_app_approvals_reviewers;
use codex_config::resolve_approvals_reviewer;
use codex_config::types::AppConfig as AppConfigToml;
use codex_config::types::ApprovalsReviewer;
use codex_core::config::deserialize_config_toml_with_base;
use codex_core::config::edit::ConfigEdit;
use codex_core::config::edit::ConfigEditsBuilder;
Expand Down Expand Up @@ -125,9 +130,35 @@ impl ConfigManager {
};

let effective = layers.effective_config();
let effective_config_toml: ConfigToml = effective
let mut effective_config_toml: ConfigToml = effective
.try_into()
.map_err(|err| ConfigManagerError::toml("invalid configuration", err))?;
let requirements = layers.requirements();
let global_reviewer = resolve_approvals_reviewer(
effective_config_toml.approvals_reviewer,
ApprovalsReviewer::User,
&requirements.approvals_reviewer,
);
let configured_app_reviewers =
configured_app_approvals_reviewers(effective_config_toml.apps.as_ref());
let app_reviewers = resolve_app_approvals_reviewers(
&configured_app_reviewers,
global_reviewer,
&requirements.approvals_reviewer,
&requirements.app_approvals_reviewers,
);
if !app_reviewers.is_empty() {
let apps = effective_config_toml.apps.get_or_insert_default();
for (app_id, reviewer) in app_reviewers {
apps.apps
.entry(app_id)
.or_insert_with(|| AppConfigToml {
enabled: true,
..Default::default()
})
.approvals_reviewer = Some(reviewer);
}
}

let json_value = serde_json::to_value(&effective_config_toml)
.map_err(|err| ConfigManagerError::json("failed to serialize configuration", err))?;
Expand Down
75 changes: 75 additions & 0 deletions codex-rs/app-server/src/config_manager_service_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use super::*;
use anyhow::Result;
use codex_app_server_protocol::AppConfig;
use codex_app_server_protocol::AppToolApproval;
use codex_app_server_protocol::ApprovalsReviewer;
use codex_app_server_protocol::AppsConfig;
use codex_app_server_protocol::AskForApproval;
use codex_config::CloudConfigBundleLoader;
Expand Down Expand Up @@ -297,6 +298,80 @@ async fn write_value_supports_nested_app_paths() -> Result<()> {
Ok(())
}

#[tokio::test]
async fn read_reports_app_reviewer_after_app_requirements_are_applied() -> Result<()> {
let tmp = tempdir().expect("tempdir");
std::fs::write(
tmp.path().join(CONFIG_TOML_FILE),
r#"
approvals_reviewer = "user"

[apps.calendar]
approvals_reviewer = "user"
"#,
)?;

let service = ConfigManager::new_for_tests(
tmp.path().to_path_buf(),
vec![],
LoaderOverrides::without_managed_config_for_tests(),
CloudConfigBundleFixture::loader_with_enterprise_requirement(
r#"
allowed_approvals_reviewers = ["user", "auto_review"]

[apps.calendar.allowed_approvals_reviewers]
user = false
auto_review = true

[apps.drive.allowed_approvals_reviewers]
user = false
auto_review = true
"#,
),
);

let read = service
.read(ConfigReadParams {
include_layers: true,
cwd: None,
})
.await
.expect("config read succeeds");

assert_eq!(
read.config
.apps
.as_ref()
.and_then(|apps| apps.apps.get("calendar"))
.and_then(|app| app.approvals_reviewer),
Some(ApprovalsReviewer::AutoReview)
);
assert_eq!(
read.config
.apps
.as_ref()
.and_then(|apps| apps.apps.get("drive"))
.map(|app| (app.enabled, app.approvals_reviewer)),
Some((true, Some(ApprovalsReviewer::AutoReview)))
);
assert!(
read.layers
.expect("layers should be included")
.iter()
.any(|layer| {
layer
.config
.get("apps")
.and_then(|apps| apps.get("calendar"))
.and_then(|calendar| calendar.get("approvals_reviewer"))
== Some(&serde_json::json!("user"))
}),
"raw config layers should preserve the configured reviewer"
);

Ok(())
}

#[tokio::test]
async fn write_value_supports_custom_mcp_server_default_tool_approval_mode() -> Result<()> {
let tmp = tempdir().expect("tempdir");
Expand Down
Loading
Loading