mirror of
https://github.com/openai/codex.git
synced 2026-04-26 07:35:29 +00:00
feat: change ConfigLayerName into a disjoint union rather than a simple enum (#8095)
This attempts to tighten up the types related to "config layers." Currently, `ConfigLayerEntry` is defined as follows:bef36f4ae7/codex-rs/core/src/config_loader/state.rs (L19-L25)but the `source` field is a bit of a lie, as: - for `ConfigLayerName::Mdm`, it is `"com.openai.codex/config_toml_base64"` - for `ConfigLayerName::SessionFlags`, it is `"--config"` - for `ConfigLayerName::User`, it is `"config.toml"` (just the file name, not the path to the `config.toml` on disk that was read) - for `ConfigLayerName::System`, it seems like it is usually `/etc/codex/managed_config.toml` in practice, though on Windows, it is `%CODEX_HOME%/managed_config.toml`:bef36f4ae7/codex-rs/core/src/config_loader/layer_io.rs (L84-L101)All that is to say, in three out of the four `ConfigLayerName`, `source` is a `PathBuf` that is not an absolute path (or even a true path). This PR tries to uplevel things by eliminating `source` from `ConfigLayerEntry` and turning `ConfigLayerName` into a disjoint union named `ConfigLayerSource` that has the appropriate metadata for each variant, favoring the use of `AbsolutePathBuf` where appropriate: ```rust pub enum ConfigLayerSource { /// Managed preferences layer delivered by MDM (macOS only). #[serde(rename_all = "camelCase")] #[ts(rename_all = "camelCase")] Mdm { domain: String, key: String }, /// Managed config layer from a file (usually `managed_config.toml`). #[serde(rename_all = "camelCase")] #[ts(rename_all = "camelCase")] System { file: AbsolutePathBuf }, /// Session-layer overrides supplied via `-c`/`--config`. SessionFlags, /// User config layer from a file (usually `config.toml`). #[serde(rename_all = "camelCase")] #[ts(rename_all = "camelCase")] User { file: AbsolutePathBuf }, } ```
This commit is contained in:
@@ -6,7 +6,7 @@ use app_test_support::to_response;
|
||||
use codex_app_server_protocol::AskForApproval;
|
||||
use codex_app_server_protocol::ConfigBatchWriteParams;
|
||||
use codex_app_server_protocol::ConfigEdit;
|
||||
use codex_app_server_protocol::ConfigLayerName;
|
||||
use codex_app_server_protocol::ConfigLayerSource;
|
||||
use codex_app_server_protocol::ConfigReadParams;
|
||||
use codex_app_server_protocol::ConfigReadResponse;
|
||||
use codex_app_server_protocol::ConfigValueWriteParams;
|
||||
@@ -18,6 +18,7 @@ use codex_app_server_protocol::RequestId;
|
||||
use codex_app_server_protocol::SandboxMode;
|
||||
use codex_app_server_protocol::ToolsV2;
|
||||
use codex_app_server_protocol::WriteStatus;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use tempfile::TempDir;
|
||||
@@ -42,6 +43,8 @@ model = "gpt-user"
|
||||
sandbox_mode = "workspace-write"
|
||||
"#,
|
||||
)?;
|
||||
let codex_home_path = codex_home.path().canonicalize()?;
|
||||
let user_file = AbsolutePathBuf::try_from(codex_home_path.join("config.toml"))?;
|
||||
|
||||
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
@@ -65,12 +68,14 @@ sandbox_mode = "workspace-write"
|
||||
assert_eq!(config.model.as_deref(), Some("gpt-user"));
|
||||
assert_eq!(
|
||||
origins.get("model").expect("origin").name,
|
||||
ConfigLayerName::User
|
||||
ConfigLayerSource::User {
|
||||
file: user_file.clone(),
|
||||
}
|
||||
);
|
||||
let layers = layers.expect("layers present");
|
||||
assert_eq!(layers.len(), 2);
|
||||
assert_eq!(layers[0].name, ConfigLayerName::SessionFlags);
|
||||
assert_eq!(layers[1].name, ConfigLayerName::User);
|
||||
assert_eq!(layers[0].name, ConfigLayerSource::SessionFlags);
|
||||
assert_eq!(layers[1].name, ConfigLayerSource::User { file: user_file });
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -88,6 +93,8 @@ web_search = true
|
||||
view_image = false
|
||||
"#,
|
||||
)?;
|
||||
let codex_home_path = codex_home.path().canonicalize()?;
|
||||
let user_file = AbsolutePathBuf::try_from(codex_home_path.join("config.toml"))?;
|
||||
|
||||
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
@@ -118,17 +125,21 @@ view_image = false
|
||||
);
|
||||
assert_eq!(
|
||||
origins.get("tools.web_search").expect("origin").name,
|
||||
ConfigLayerName::User
|
||||
ConfigLayerSource::User {
|
||||
file: user_file.clone(),
|
||||
}
|
||||
);
|
||||
assert_eq!(
|
||||
origins.get("tools.view_image").expect("origin").name,
|
||||
ConfigLayerName::User
|
||||
ConfigLayerSource::User {
|
||||
file: user_file.clone(),
|
||||
}
|
||||
);
|
||||
|
||||
let layers = layers.expect("layers present");
|
||||
assert_eq!(layers.len(), 2);
|
||||
assert_eq!(layers[0].name, ConfigLayerName::SessionFlags);
|
||||
assert_eq!(layers[1].name, ConfigLayerName::User);
|
||||
assert_eq!(layers[0].name, ConfigLayerSource::SessionFlags);
|
||||
assert_eq!(layers[1].name, ConfigLayerSource::User { file: user_file });
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -153,8 +164,11 @@ network_access = true
|
||||
serde_json::json!(user_dir)
|
||||
),
|
||||
)?;
|
||||
let codex_home_path = codex_home.path().canonicalize()?;
|
||||
let user_file = AbsolutePathBuf::try_from(codex_home_path.join("config.toml"))?;
|
||||
|
||||
let managed_path = codex_home.path().join("managed_config.toml");
|
||||
let managed_file = AbsolutePathBuf::try_from(managed_path.clone())?;
|
||||
std::fs::write(
|
||||
&managed_path,
|
||||
format!(
|
||||
@@ -197,19 +211,25 @@ writable_roots = [{}]
|
||||
assert_eq!(config.model.as_deref(), Some("gpt-system"));
|
||||
assert_eq!(
|
||||
origins.get("model").expect("origin").name,
|
||||
ConfigLayerName::System
|
||||
ConfigLayerSource::System {
|
||||
file: managed_file.clone(),
|
||||
}
|
||||
);
|
||||
|
||||
assert_eq!(config.approval_policy, Some(AskForApproval::Never));
|
||||
assert_eq!(
|
||||
origins.get("approval_policy").expect("origin").name,
|
||||
ConfigLayerName::System
|
||||
ConfigLayerSource::System {
|
||||
file: managed_file.clone(),
|
||||
}
|
||||
);
|
||||
|
||||
assert_eq!(config.sandbox_mode, Some(SandboxMode::WorkspaceWrite));
|
||||
assert_eq!(
|
||||
origins.get("sandbox_mode").expect("origin").name,
|
||||
ConfigLayerName::User
|
||||
ConfigLayerSource::User {
|
||||
file: user_file.clone(),
|
||||
}
|
||||
);
|
||||
|
||||
let sandbox = config
|
||||
@@ -222,7 +242,9 @@ writable_roots = [{}]
|
||||
.get("sandbox_workspace_write.writable_roots.0")
|
||||
.expect("origin")
|
||||
.name,
|
||||
ConfigLayerName::System
|
||||
ConfigLayerSource::System {
|
||||
file: managed_file.clone(),
|
||||
}
|
||||
);
|
||||
|
||||
assert!(sandbox.network_access);
|
||||
@@ -231,14 +253,19 @@ writable_roots = [{}]
|
||||
.get("sandbox_workspace_write.network_access")
|
||||
.expect("origin")
|
||||
.name,
|
||||
ConfigLayerName::User
|
||||
ConfigLayerSource::User {
|
||||
file: user_file.clone(),
|
||||
}
|
||||
);
|
||||
|
||||
let layers = layers.expect("layers present");
|
||||
assert_eq!(layers.len(), 3);
|
||||
assert_eq!(layers[0].name, ConfigLayerName::System);
|
||||
assert_eq!(layers[1].name, ConfigLayerName::SessionFlags);
|
||||
assert_eq!(layers[2].name, ConfigLayerName::User);
|
||||
assert_eq!(
|
||||
layers[0].name,
|
||||
ConfigLayerSource::System { file: managed_file }
|
||||
);
|
||||
assert_eq!(layers[1].name, ConfigLayerSource::SessionFlags);
|
||||
assert_eq!(layers[2].name, ConfigLayerSource::User { file: user_file });
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user