mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
config: own layer provenance types (#29722)
## Why Config layer provenance describes how effective configuration was assembled, so it belongs with the config loader rather than in app-server's serialized API types. ## What changed - Moved `ConfigLayerSource`, `ConfigLayerMetadata`, and `ConfigLayer` ownership into `codex-config`. - Kept app-server's wire payloads unchanged and added explicit conversions at the app boundary. - Removed lower-level app-server-protocol dependencies from config consumers. ## Stack This is PR 3 of 6, stacked on [PR #29721](https://github.com/openai/codex/pull/29721). Review only the delta from `codex/split-auth-domain-types`. Next: [PR #29723](https://github.com/openai/codex/pull/29723). ## Validation - `codex-config` coverage passed. - App-server config-manager and config RPC coverage passed.
This commit is contained in:
committed by
GitHub
Unverified
parent
4fe02f4fcf
commit
1d65ccabd5
@@ -0,0 +1,63 @@
|
||||
use codex_app_server_protocol::ConfigLayer as ApiConfigLayer;
|
||||
use codex_app_server_protocol::ConfigLayerMetadata as ApiConfigLayerMetadata;
|
||||
use codex_app_server_protocol::ConfigLayerSource as ApiConfigLayerSource;
|
||||
use codex_config::ConfigLayer;
|
||||
use codex_config::ConfigLayerMetadata;
|
||||
use codex_config::ConfigLayerSource;
|
||||
|
||||
/// Converts a config-layer source owned by `codex-config` into the app-server wire type owned by
|
||||
/// `codex-app-server-protocol`.
|
||||
///
|
||||
/// The types stay separate so app-server protocol ownership does not leak into the config domain
|
||||
/// crate. Because this crate owns neither type, Rust's orphan rules require an explicit conversion
|
||||
/// function instead of a `From` implementation.
|
||||
pub(crate) fn config_layer_source_to_api(source: ConfigLayerSource) -> ApiConfigLayerSource {
|
||||
match source {
|
||||
ConfigLayerSource::Mdm { domain, key } => ApiConfigLayerSource::Mdm { domain, key },
|
||||
ConfigLayerSource::System { file } => ApiConfigLayerSource::System { file },
|
||||
ConfigLayerSource::EnterpriseManaged { id, name } => {
|
||||
ApiConfigLayerSource::EnterpriseManaged { id, name }
|
||||
}
|
||||
ConfigLayerSource::User { file, profile } => ApiConfigLayerSource::User { file, profile },
|
||||
ConfigLayerSource::Project { dot_codex_folder } => {
|
||||
ApiConfigLayerSource::Project { dot_codex_folder }
|
||||
}
|
||||
ConfigLayerSource::SessionFlags => ApiConfigLayerSource::SessionFlags,
|
||||
ConfigLayerSource::LegacyManagedConfigTomlFromFile { file } => {
|
||||
ApiConfigLayerSource::LegacyManagedConfigTomlFromFile { file }
|
||||
}
|
||||
ConfigLayerSource::LegacyManagedConfigTomlFromMdm => {
|
||||
ApiConfigLayerSource::LegacyManagedConfigTomlFromMdm
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Converts config-layer metadata owned by `codex-config` into the app-server wire type owned by
|
||||
/// `codex-app-server-protocol`.
|
||||
///
|
||||
/// The types stay separate so app-server protocol ownership does not leak into the config domain
|
||||
/// crate. Because this crate owns neither type, Rust's orphan rules require an explicit conversion
|
||||
/// function instead of a `From` implementation.
|
||||
pub(crate) fn config_layer_metadata_to_api(
|
||||
metadata: ConfigLayerMetadata,
|
||||
) -> ApiConfigLayerMetadata {
|
||||
ApiConfigLayerMetadata {
|
||||
name: config_layer_source_to_api(metadata.name),
|
||||
version: metadata.version,
|
||||
}
|
||||
}
|
||||
|
||||
/// Converts a config layer owned by `codex-config` into the app-server wire type owned by
|
||||
/// `codex-app-server-protocol`.
|
||||
///
|
||||
/// The types stay separate so app-server protocol ownership does not leak into the config domain
|
||||
/// crate. Because this crate owns neither type, Rust's orphan rules require an explicit conversion
|
||||
/// function instead of a `From` implementation.
|
||||
pub(crate) fn config_layer_to_api(layer: ConfigLayer) -> ApiConfigLayer {
|
||||
ApiConfigLayer {
|
||||
name: config_layer_source_to_api(layer.name),
|
||||
version: layer.version,
|
||||
config: layer.config,
|
||||
disabled_reason: layer.disabled_reason,
|
||||
}
|
||||
}
|
||||
@@ -1,8 +1,8 @@
|
||||
use crate::config_layer::config_layer_metadata_to_api;
|
||||
use crate::config_layer::config_layer_to_api;
|
||||
use crate::config_manager::ConfigManager;
|
||||
use codex_app_server_protocol::Config as ApiConfig;
|
||||
use codex_app_server_protocol::ConfigBatchWriteParams;
|
||||
use codex_app_server_protocol::ConfigLayerMetadata;
|
||||
use codex_app_server_protocol::ConfigLayerSource;
|
||||
use codex_app_server_protocol::ConfigReadParams;
|
||||
use codex_app_server_protocol::ConfigReadResponse;
|
||||
use codex_app_server_protocol::ConfigValueWriteParams;
|
||||
@@ -13,6 +13,8 @@ use codex_app_server_protocol::OverriddenMetadata;
|
||||
use codex_app_server_protocol::WriteStatus;
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
use codex_config::ConfigLayerEntry;
|
||||
use codex_config::ConfigLayerMetadata;
|
||||
use codex_config::ConfigLayerSource;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::ConfigLayerStackOrdering;
|
||||
use codex_config::ConfigRequirementsToml;
|
||||
@@ -136,7 +138,11 @@ impl ConfigManager {
|
||||
|
||||
Ok(ConfigReadResponse {
|
||||
config,
|
||||
origins: layers.origins(),
|
||||
origins: layers
|
||||
.origins()
|
||||
.into_iter()
|
||||
.map(|(path, metadata)| (path, config_layer_metadata_to_api(metadata)))
|
||||
.collect(),
|
||||
layers: params.include_layers.then(|| {
|
||||
layers
|
||||
.get_layers(
|
||||
@@ -144,7 +150,7 @@ impl ConfigManager {
|
||||
/*include_disabled*/ true,
|
||||
)
|
||||
.iter()
|
||||
.map(|layer| layer.as_layer())
|
||||
.map(|layer| config_layer_to_api(layer.as_layer()))
|
||||
.collect()
|
||||
}),
|
||||
})
|
||||
@@ -671,7 +677,7 @@ fn compute_override_metadata(
|
||||
|
||||
Some(OverriddenMetadata {
|
||||
message,
|
||||
overriding_layer,
|
||||
overriding_layer: config_layer_metadata_to_api(overriding_layer),
|
||||
effective_value: effective_value
|
||||
.and_then(|value| serde_json::to_value(value).ok())
|
||||
.unwrap_or(JsonValue::Null),
|
||||
|
||||
@@ -4,6 +4,7 @@ use codex_app_server_protocol::AppConfig;
|
||||
use codex_app_server_protocol::AppToolApproval;
|
||||
use codex_app_server_protocol::AppsConfig;
|
||||
use codex_app_server_protocol::AskForApproval;
|
||||
use codex_app_server_protocol::ConfigLayerSource as ApiConfigLayerSource;
|
||||
use codex_config::CloudConfigBundleLoader;
|
||||
use codex_config::LoaderOverrides;
|
||||
use codex_config::test_support::CloudConfigBundleFixture;
|
||||
@@ -374,7 +375,7 @@ async fn read_includes_origins_and_layers() {
|
||||
.get("approval_policy")
|
||||
.expect("origin")
|
||||
.name,
|
||||
ConfigLayerSource::LegacyManagedConfigTomlFromFile {
|
||||
ApiConfigLayerSource::LegacyManagedConfigTomlFromFile {
|
||||
file: managed_file.clone()
|
||||
},
|
||||
);
|
||||
@@ -383,7 +384,7 @@ async fn read_includes_origins_and_layers() {
|
||||
// top of the stack; ignore it so this test stays focused on file/user/system ordering.
|
||||
let layers = if matches!(
|
||||
layers.first().map(|layer| &layer.name),
|
||||
Some(ConfigLayerSource::LegacyManagedConfigTomlFromMdm)
|
||||
Some(ApiConfigLayerSource::LegacyManagedConfigTomlFromMdm)
|
||||
) {
|
||||
&layers[1..]
|
||||
} else {
|
||||
@@ -392,20 +393,20 @@ async fn read_includes_origins_and_layers() {
|
||||
assert_eq!(layers.len(), 3, "expected three layers");
|
||||
assert_eq!(
|
||||
layers.first().unwrap().name,
|
||||
ConfigLayerSource::LegacyManagedConfigTomlFromFile {
|
||||
ApiConfigLayerSource::LegacyManagedConfigTomlFromFile {
|
||||
file: managed_file.clone()
|
||||
}
|
||||
);
|
||||
assert_eq!(
|
||||
layers.get(1).unwrap().name,
|
||||
ConfigLayerSource::User {
|
||||
ApiConfigLayerSource::User {
|
||||
file: user_file.clone(),
|
||||
profile: None,
|
||||
}
|
||||
);
|
||||
assert!(matches!(
|
||||
layers.get(2).unwrap().name,
|
||||
ConfigLayerSource::System { .. }
|
||||
ApiConfigLayerSource::System { .. }
|
||||
));
|
||||
}
|
||||
|
||||
@@ -505,7 +506,7 @@ async fn write_value_reports_override() {
|
||||
.get("approval_policy")
|
||||
.expect("origin")
|
||||
.name,
|
||||
ConfigLayerSource::LegacyManagedConfigTomlFromFile {
|
||||
ApiConfigLayerSource::LegacyManagedConfigTomlFromFile {
|
||||
file: managed_file.clone()
|
||||
}
|
||||
);
|
||||
@@ -776,7 +777,7 @@ async fn read_reports_managed_overrides_user_and_session_flags() {
|
||||
assert_eq!(response.config.model.as_deref(), Some("system"));
|
||||
assert_eq!(
|
||||
response.origins.get("model").expect("origin").name,
|
||||
ConfigLayerSource::LegacyManagedConfigTomlFromFile {
|
||||
ApiConfigLayerSource::LegacyManagedConfigTomlFromFile {
|
||||
file: managed_file.clone()
|
||||
},
|
||||
);
|
||||
@@ -785,7 +786,7 @@ async fn read_reports_managed_overrides_user_and_session_flags() {
|
||||
// top of the stack; ignore it so this test stays focused on file/session/user ordering.
|
||||
let layers = if matches!(
|
||||
layers.first().map(|layer| &layer.name),
|
||||
Some(ConfigLayerSource::LegacyManagedConfigTomlFromMdm)
|
||||
Some(ApiConfigLayerSource::LegacyManagedConfigTomlFromMdm)
|
||||
) {
|
||||
&layers[1..]
|
||||
} else {
|
||||
@@ -793,12 +794,15 @@ async fn read_reports_managed_overrides_user_and_session_flags() {
|
||||
};
|
||||
assert_eq!(
|
||||
layers.first().unwrap().name,
|
||||
ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file }
|
||||
ApiConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file }
|
||||
);
|
||||
assert_eq!(
|
||||
layers.get(1).unwrap().name,
|
||||
ApiConfigLayerSource::SessionFlags
|
||||
);
|
||||
assert_eq!(layers.get(1).unwrap().name, ConfigLayerSource::SessionFlags);
|
||||
assert_eq!(
|
||||
layers.get(2).unwrap().name,
|
||||
ConfigLayerSource::User {
|
||||
ApiConfigLayerSource::User {
|
||||
file: user_file,
|
||||
profile: None
|
||||
}
|
||||
@@ -836,7 +840,7 @@ async fn write_value_reports_managed_override() {
|
||||
let overridden = result.overridden_metadata.expect("overridden metadata");
|
||||
assert_eq!(
|
||||
overridden.overriding_layer.name,
|
||||
ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file }
|
||||
ApiConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file }
|
||||
);
|
||||
assert_eq!(overridden.effective_value, serde_json::json!("never"));
|
||||
}
|
||||
|
||||
@@ -47,12 +47,12 @@ use crate::transport::start_remote_control;
|
||||
use crate::transport::start_stdio_connection;
|
||||
use crate::transport::start_websocket_acceptor;
|
||||
use codex_analytics::AppServerRpcTransport;
|
||||
use codex_app_server_protocol::ConfigLayerSource;
|
||||
use codex_app_server_protocol::ConfigWarningNotification;
|
||||
use codex_app_server_protocol::JSONRPCMessage;
|
||||
use codex_app_server_protocol::ServerNotification;
|
||||
use codex_app_server_protocol::TextPosition as AppTextPosition;
|
||||
use codex_app_server_protocol::TextRange as AppTextRange;
|
||||
use codex_config::ConfigLayerSource;
|
||||
use codex_config::ConfigLoadError;
|
||||
use codex_config::TextRange as CoreTextRange;
|
||||
use codex_core::ExecPolicyError;
|
||||
@@ -86,6 +86,7 @@ mod auth_mode;
|
||||
mod bespoke_event_handling;
|
||||
mod command_exec;
|
||||
mod config;
|
||||
mod config_layer;
|
||||
mod config_manager;
|
||||
mod config_manager_service;
|
||||
mod connection_cleanup;
|
||||
|
||||
Reference in New Issue
Block a user