mirror of
https://github.com/openai/codex.git
synced 2026-11-02 19:43:24 +00:00
Make config loading fully async with ExecutorFileSystem
This commit is contained in:
Generated
+1
@@ -1476,6 +1476,7 @@ dependencies = [
|
||||
"codex-app-server-protocol",
|
||||
"codex-arg0",
|
||||
"codex-core",
|
||||
"codex-exec-server",
|
||||
"codex-features",
|
||||
"codex-feedback",
|
||||
"codex-protocol",
|
||||
|
||||
@@ -29,6 +29,7 @@ tracing = { workspace = true }
|
||||
url = { workspace = true }
|
||||
|
||||
[dev-dependencies]
|
||||
codex-exec-server = { workspace = true }
|
||||
pretty_assertions = { workspace = true }
|
||||
serde_json = { workspace = true }
|
||||
tokio = { workspace = true, features = ["macros", "rt-multi-thread"] }
|
||||
|
||||
@@ -872,8 +872,12 @@ mod tests {
|
||||
async fn build_test_config() -> Config {
|
||||
match ConfigBuilder::default().build().await {
|
||||
Ok(config) => config,
|
||||
Err(_) => Config::load_default_with_cli_overrides(Vec::new())
|
||||
.expect("default config should load"),
|
||||
Err(_) => Config::load_default_with_cli_overrides(
|
||||
Vec::new(),
|
||||
codex_exec_server::Environment::default().get_filesystem(),
|
||||
)
|
||||
.await
|
||||
.expect("default config should load"),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -744,8 +744,12 @@ mod tests {
|
||||
async fn build_test_config() -> Config {
|
||||
match ConfigBuilder::default().build().await {
|
||||
Ok(config) => config,
|
||||
Err(_) => Config::load_default_with_cli_overrides(Vec::new())
|
||||
.expect("default config should load"),
|
||||
Err(_) => Config::load_default_with_cli_overrides(
|
||||
Vec::new(),
|
||||
codex_exec_server::Environment::default().get_filesystem(),
|
||||
)
|
||||
.await
|
||||
.expect("default config should load"),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -448,12 +448,15 @@ pub async fn run_main_with_transport(
|
||||
Err(err) => {
|
||||
let message = config_warning_from_error("Invalid configuration; using defaults.", &err);
|
||||
config_warnings.push(message);
|
||||
Config::load_default_with_cli_overrides(cli_kv_overrides.clone()).map_err(|e| {
|
||||
std::io::Error::new(
|
||||
ErrorKind::InvalidData,
|
||||
format!("error loading default config after config error: {e}"),
|
||||
)
|
||||
})?
|
||||
let file_system = codex_exec_server::Environment::default().get_filesystem();
|
||||
Config::load_default_with_cli_overrides(cli_kv_overrides.clone(), file_system)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
std::io::Error::new(
|
||||
ErrorKind::InvalidData,
|
||||
format!("error loading default config after config error: {e}"),
|
||||
)
|
||||
})?
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
@@ -71,7 +71,8 @@ async fn apply_role_to_config_inner(
|
||||
role_layer_toml,
|
||||
preserve_current_profile,
|
||||
preserve_current_provider,
|
||||
)?;
|
||||
)
|
||||
.await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -143,7 +144,7 @@ fn preservation_policy(config: &Config, role_layer_toml: &TomlValue) -> (bool, b
|
||||
mod reload {
|
||||
use super::*;
|
||||
|
||||
pub(super) fn build_next_config(
|
||||
pub(super) async fn build_next_config(
|
||||
config: &Config,
|
||||
role_layer_toml: TomlValue,
|
||||
preserve_current_profile: bool,
|
||||
@@ -159,12 +160,15 @@ mod reload {
|
||||
merged_config.profile = None;
|
||||
}
|
||||
|
||||
let file_system = codex_exec_server::Environment::default().get_filesystem();
|
||||
let mut next_config = Config::load_config_with_layer_stack(
|
||||
merged_config,
|
||||
reload_overrides(config, preserve_current_provider),
|
||||
config.codex_home.clone(),
|
||||
config_layer_stack,
|
||||
)?;
|
||||
file_system,
|
||||
)
|
||||
.await?;
|
||||
if preserve_current_profile {
|
||||
next_config.active_profile = config.active_profile.clone();
|
||||
}
|
||||
|
||||
@@ -4,6 +4,7 @@ use super::AgentsToml;
|
||||
use super::ConfigToml;
|
||||
use crate::config_loader::ConfigLayerStack;
|
||||
use crate::config_loader::ConfigLayerStackOrdering;
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_absolute_path::AbsolutePathBufGuard;
|
||||
use serde::Deserialize;
|
||||
@@ -14,17 +15,18 @@ use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use toml::Value as TomlValue;
|
||||
|
||||
pub(crate) fn load_agent_roles(
|
||||
pub(crate) async fn load_agent_roles(
|
||||
cfg: &ConfigToml,
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
startup_warnings: &mut Vec<String>,
|
||||
file_system: &dyn ExecutorFileSystem,
|
||||
) -> std::io::Result<BTreeMap<String, AgentRoleConfig>> {
|
||||
let layers = config_layer_stack.get_layers(
|
||||
ConfigLayerStackOrdering::LowestPrecedenceFirst,
|
||||
/*include_disabled*/ false,
|
||||
);
|
||||
if layers.is_empty() {
|
||||
return load_agent_roles_without_layers(cfg);
|
||||
return load_agent_roles_without_layers(cfg, file_system).await;
|
||||
}
|
||||
|
||||
let mut roles: BTreeMap<String, AgentRoleConfig> = BTreeMap::new();
|
||||
@@ -40,13 +42,14 @@ pub(crate) fn load_agent_roles(
|
||||
};
|
||||
if let Some(agents_toml) = agents_toml {
|
||||
for (declared_role_name, role_toml) in &agents_toml.roles {
|
||||
let (role_name, role) = match read_declared_role(declared_role_name, role_toml) {
|
||||
Ok(role) => role,
|
||||
Err(err) => {
|
||||
push_agent_role_warning(startup_warnings, err);
|
||||
continue;
|
||||
}
|
||||
};
|
||||
let (role_name, role) =
|
||||
match read_declared_role(declared_role_name, role_toml, file_system).await {
|
||||
Ok(role) => role,
|
||||
Err(err) => {
|
||||
push_agent_role_warning(startup_warnings, err);
|
||||
continue;
|
||||
}
|
||||
};
|
||||
if let Some(config_file) = role.config_file.clone() {
|
||||
declared_role_files.insert(config_file);
|
||||
}
|
||||
@@ -71,7 +74,10 @@ pub(crate) fn load_agent_roles(
|
||||
config_folder.as_path().join("agents").as_path(),
|
||||
&declared_role_files,
|
||||
startup_warnings,
|
||||
)? {
|
||||
file_system,
|
||||
)
|
||||
.await?
|
||||
{
|
||||
if layer_roles.contains_key(&role_name) {
|
||||
push_agent_role_warning(
|
||||
startup_warnings,
|
||||
@@ -113,13 +119,15 @@ fn push_agent_role_warning(startup_warnings: &mut Vec<String>, err: std::io::Err
|
||||
startup_warnings.push(message);
|
||||
}
|
||||
|
||||
fn load_agent_roles_without_layers(
|
||||
async fn load_agent_roles_without_layers(
|
||||
cfg: &ConfigToml,
|
||||
file_system: &dyn ExecutorFileSystem,
|
||||
) -> std::io::Result<BTreeMap<String, AgentRoleConfig>> {
|
||||
let mut roles = BTreeMap::new();
|
||||
if let Some(agents_toml) = cfg.agents.as_ref() {
|
||||
for (declared_role_name, role_toml) in &agents_toml.roles {
|
||||
let (role_name, role) = read_declared_role(declared_role_name, role_toml)?;
|
||||
let (role_name, role) =
|
||||
read_declared_role(declared_role_name, role_toml, file_system).await?;
|
||||
validate_required_agent_role_description(&role_name, role.description.as_deref())?;
|
||||
|
||||
if roles.insert(role_name.clone(), role).is_some() {
|
||||
@@ -134,14 +142,19 @@ fn load_agent_roles_without_layers(
|
||||
Ok(roles)
|
||||
}
|
||||
|
||||
fn read_declared_role(
|
||||
async fn read_declared_role(
|
||||
declared_role_name: &str,
|
||||
role_toml: &AgentRoleToml,
|
||||
file_system: &dyn ExecutorFileSystem,
|
||||
) -> std::io::Result<(String, AgentRoleConfig)> {
|
||||
let mut role = agent_role_config_from_toml(declared_role_name, role_toml)?;
|
||||
validate_agent_role_config_file(declared_role_name, role.config_file.as_deref(), file_system)
|
||||
.await?;
|
||||
let mut role_name = declared_role_name.to_string();
|
||||
if let Some(config_file) = role.config_file.as_deref() {
|
||||
let parsed_file = read_resolved_agent_role_file(config_file, Some(declared_role_name))?;
|
||||
let parsed_file =
|
||||
read_resolved_agent_role_file(config_file, Some(declared_role_name), file_system)
|
||||
.await?;
|
||||
role_name = parsed_file.role_name;
|
||||
role.description = parsed_file.description.or(role.description);
|
||||
role.nickname_candidates = parsed_file.nickname_candidates.or(role.nickname_candidates);
|
||||
@@ -176,7 +189,6 @@ fn agent_role_config_from_toml(
|
||||
role: &AgentRoleToml,
|
||||
) -> std::io::Result<AgentRoleConfig> {
|
||||
let config_file = role.config_file.as_ref().map(AbsolutePathBuf::to_path_buf);
|
||||
validate_agent_role_config_file(role_name, config_file.as_deref())?;
|
||||
let description = normalize_agent_role_description(
|
||||
&format!("agents.{role_name}.description"),
|
||||
role.description.as_deref(),
|
||||
@@ -293,11 +305,12 @@ pub(crate) fn parse_agent_role_file_contents(
|
||||
})
|
||||
}
|
||||
|
||||
fn read_resolved_agent_role_file(
|
||||
async fn read_resolved_agent_role_file(
|
||||
path: &Path,
|
||||
role_name_hint: Option<&str>,
|
||||
file_system: &dyn ExecutorFileSystem,
|
||||
) -> std::io::Result<ResolvedAgentRoleFile> {
|
||||
let contents = std::fs::read_to_string(path)?;
|
||||
let contents = super::read_text_file(path, file_system).await?;
|
||||
parse_agent_role_file_contents(
|
||||
&contents,
|
||||
path,
|
||||
@@ -359,24 +372,29 @@ fn validate_agent_role_file_developer_instructions(
|
||||
}
|
||||
}
|
||||
|
||||
fn validate_agent_role_config_file(
|
||||
async fn validate_agent_role_config_file(
|
||||
role_name: &str,
|
||||
config_file: Option<&Path>,
|
||||
file_system: &dyn ExecutorFileSystem,
|
||||
) -> std::io::Result<()> {
|
||||
let Some(config_file) = config_file else {
|
||||
return Ok(());
|
||||
};
|
||||
|
||||
let metadata = std::fs::metadata(config_file).map_err(|e| {
|
||||
std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
format!(
|
||||
"agents.{role_name}.config_file must point to an existing file at {}: {e}",
|
||||
config_file.display()
|
||||
),
|
||||
)
|
||||
})?;
|
||||
if metadata.is_file() {
|
||||
let absolute_path = super::absolute_path_for_filesystem_read(config_file)?;
|
||||
let metadata = file_system
|
||||
.get_metadata(&absolute_path)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
format!(
|
||||
"agents.{role_name}.config_file must point to an existing file at {}: {e}",
|
||||
config_file.display()
|
||||
),
|
||||
)
|
||||
})?;
|
||||
if metadata.is_file {
|
||||
Ok(())
|
||||
} else {
|
||||
Err(std::io::Error::new(
|
||||
@@ -441,25 +459,31 @@ fn normalize_agent_role_nickname_candidates(
|
||||
Ok(Some(normalized_candidates))
|
||||
}
|
||||
|
||||
fn discover_agent_roles_in_dir(
|
||||
async fn discover_agent_roles_in_dir(
|
||||
agents_dir: &Path,
|
||||
declared_role_files: &BTreeSet<PathBuf>,
|
||||
startup_warnings: &mut Vec<String>,
|
||||
file_system: &dyn ExecutorFileSystem,
|
||||
) -> std::io::Result<BTreeMap<String, AgentRoleConfig>> {
|
||||
let mut roles = BTreeMap::new();
|
||||
|
||||
for agent_file in collect_agent_role_files(agents_dir)? {
|
||||
for agent_file in collect_agent_role_files(agents_dir, file_system).await? {
|
||||
if declared_role_files.contains(&agent_file) {
|
||||
continue;
|
||||
}
|
||||
let parsed_file =
|
||||
match read_resolved_agent_role_file(&agent_file, /*role_name_hint*/ None) {
|
||||
Ok(parsed_file) => parsed_file,
|
||||
Err(err) => {
|
||||
push_agent_role_warning(startup_warnings, err);
|
||||
continue;
|
||||
}
|
||||
};
|
||||
let parsed_file = match read_resolved_agent_role_file(
|
||||
&agent_file,
|
||||
/*role_name_hint*/ None,
|
||||
file_system,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(parsed_file) => parsed_file,
|
||||
Err(err) => {
|
||||
push_agent_role_warning(startup_warnings, err);
|
||||
continue;
|
||||
}
|
||||
};
|
||||
let role_name = parsed_file.role_name;
|
||||
if roles.contains_key(&role_name) {
|
||||
push_agent_role_warning(
|
||||
@@ -487,36 +511,36 @@ fn discover_agent_roles_in_dir(
|
||||
Ok(roles)
|
||||
}
|
||||
|
||||
fn collect_agent_role_files(dir: &Path) -> std::io::Result<Vec<PathBuf>> {
|
||||
async fn collect_agent_role_files(
|
||||
dir: &Path,
|
||||
file_system: &dyn ExecutorFileSystem,
|
||||
) -> std::io::Result<Vec<PathBuf>> {
|
||||
let mut pending_dirs = vec![super::absolute_path_for_filesystem_read(dir)?];
|
||||
let mut files = Vec::new();
|
||||
collect_agent_role_files_recursive(dir, &mut files)?;
|
||||
files.sort();
|
||||
Ok(files)
|
||||
}
|
||||
while let Some(next_dir) = pending_dirs.pop() {
|
||||
let read_dir = match file_system.read_directory(&next_dir).await {
|
||||
Ok(read_dir) => read_dir,
|
||||
Err(err) if err.kind() == ErrorKind::NotFound => continue,
|
||||
Err(err) => return Err(err),
|
||||
};
|
||||
|
||||
fn collect_agent_role_files_recursive(dir: &Path, files: &mut Vec<PathBuf>) -> std::io::Result<()> {
|
||||
let read_dir = match std::fs::read_dir(dir) {
|
||||
Ok(read_dir) => read_dir,
|
||||
Err(err) if err.kind() == ErrorKind::NotFound => return Ok(()),
|
||||
Err(err) => return Err(err),
|
||||
};
|
||||
|
||||
for entry in read_dir {
|
||||
let entry = entry?;
|
||||
let path = entry.path();
|
||||
let file_type = entry.file_type()?;
|
||||
if file_type.is_dir() {
|
||||
collect_agent_role_files_recursive(&path, files)?;
|
||||
continue;
|
||||
}
|
||||
if file_type.is_file()
|
||||
&& path
|
||||
.extension()
|
||||
.is_some_and(|extension| extension == "toml")
|
||||
{
|
||||
files.push(path);
|
||||
for entry in read_dir {
|
||||
let path = next_dir.join(&entry.file_name)?;
|
||||
if entry.is_directory {
|
||||
pending_dirs.push(path);
|
||||
continue;
|
||||
}
|
||||
if entry.is_file
|
||||
&& path
|
||||
.as_path()
|
||||
.extension()
|
||||
.is_some_and(|extension| extension == "toml")
|
||||
{
|
||||
files.push(path.into_path_buf());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
files.sort();
|
||||
Ok(files)
|
||||
}
|
||||
|
||||
@@ -2997,8 +2997,8 @@ fn loads_compact_prompt_from_file() -> std::io::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn load_config_uses_requirements_guardian_developer_instructions() -> std::io::Result<()> {
|
||||
#[tokio::test]
|
||||
async fn load_config_uses_requirements_guardian_developer_instructions() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let config_layer_stack = ConfigLayerStack::new(
|
||||
Vec::new(),
|
||||
@@ -3012,6 +3012,7 @@ fn load_config_uses_requirements_guardian_developer_instructions() -> std::io::R
|
||||
)
|
||||
.map_err(std::io::Error::other)?;
|
||||
|
||||
let file_system = codex_exec_server::Environment::default().get_filesystem();
|
||||
let config = Config::load_config_with_layer_stack(
|
||||
ConfigToml::default(),
|
||||
ConfigOverrides {
|
||||
@@ -3020,7 +3021,9 @@ fn load_config_uses_requirements_guardian_developer_instructions() -> std::io::R
|
||||
},
|
||||
codex_home.path().to_path_buf(),
|
||||
config_layer_stack,
|
||||
)?;
|
||||
file_system,
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
config.guardian_developer_instructions.as_deref(),
|
||||
@@ -3030,8 +3033,9 @@ fn load_config_uses_requirements_guardian_developer_instructions() -> std::io::R
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn load_config_ignores_empty_requirements_guardian_developer_instructions() -> std::io::Result<()> {
|
||||
#[tokio::test]
|
||||
async fn load_config_ignores_empty_requirements_guardian_developer_instructions()
|
||||
-> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let config_layer_stack = ConfigLayerStack::new(
|
||||
Vec::new(),
|
||||
@@ -3043,6 +3047,7 @@ fn load_config_ignores_empty_requirements_guardian_developer_instructions() -> s
|
||||
)
|
||||
.map_err(std::io::Error::other)?;
|
||||
|
||||
let file_system = codex_exec_server::Environment::default().get_filesystem();
|
||||
let config = Config::load_config_with_layer_stack(
|
||||
ConfigToml::default(),
|
||||
ConfigOverrides {
|
||||
@@ -3051,7 +3056,9 @@ fn load_config_ignores_empty_requirements_guardian_developer_instructions() -> s
|
||||
},
|
||||
codex_home.path().to_path_buf(),
|
||||
config_layer_stack,
|
||||
)?;
|
||||
file_system,
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(config.guardian_developer_instructions, None);
|
||||
|
||||
@@ -4773,8 +4780,9 @@ fn test_precedence_fixture_with_gpt5_profile() -> std::io::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_requirements_web_search_mode_allowlist_does_not_warn_when_unset() -> anyhow::Result<()> {
|
||||
#[tokio::test]
|
||||
async fn test_requirements_web_search_mode_allowlist_does_not_warn_when_unset() -> anyhow::Result<()>
|
||||
{
|
||||
let fixture = create_test_fixture()?;
|
||||
|
||||
let requirements_toml = crate::config_loader::ConfigRequirementsToml {
|
||||
@@ -4817,6 +4825,7 @@ fn test_requirements_web_search_mode_allowlist_does_not_warn_when_unset() -> any
|
||||
crate::config_loader::ConfigLayerStack::new(Vec::new(), requirements, requirements_toml)
|
||||
.expect("config layer stack");
|
||||
|
||||
let file_system = codex_exec_server::Environment::default().get_filesystem();
|
||||
let config = Config::load_config_with_layer_stack(
|
||||
fixture.cfg.clone(),
|
||||
ConfigOverrides {
|
||||
@@ -4825,7 +4834,9 @@ fn test_requirements_web_search_mode_allowlist_does_not_warn_when_unset() -> any
|
||||
},
|
||||
fixture.codex_home(),
|
||||
config_layer_stack,
|
||||
)?;
|
||||
file_system,
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert!(
|
||||
!config
|
||||
|
||||
+128
-31
@@ -61,6 +61,8 @@ use crate::windows_sandbox::resolve_windows_sandbox_mode;
|
||||
use crate::windows_sandbox::resolve_windows_sandbox_private_desktop;
|
||||
use codex_app_server_protocol::Tools;
|
||||
use codex_app_server_protocol::UserSavedConfig;
|
||||
use codex_exec_server::CreateDirectoryOptions;
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_features::Feature;
|
||||
use codex_features::FeatureConfigSource;
|
||||
use codex_features::FeatureOverrides;
|
||||
@@ -96,6 +98,7 @@ use std::collections::HashMap;
|
||||
use std::io::ErrorKind;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::config::permissions::compile_permission_profile;
|
||||
use crate::config::permissions::network_proxy_config_from_profile_network;
|
||||
@@ -152,6 +155,10 @@ const RESERVED_MODEL_PROVIDER_IDS: [&str; 3] = [
|
||||
LMSTUDIO_OSS_PROVIDER_ID,
|
||||
];
|
||||
|
||||
fn local_executor_file_system() -> Arc<dyn ExecutorFileSystem> {
|
||||
codex_exec_server::Environment::default().get_filesystem()
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
pub fn missing_system_bwrap_warning() -> Option<String> {
|
||||
if Path::new(SYSTEM_BWRAP_PATH).is_file() {
|
||||
@@ -692,12 +699,15 @@ impl ConfigBuilder {
|
||||
return Err(std::io::Error::new(std::io::ErrorKind::InvalidData, err));
|
||||
}
|
||||
};
|
||||
let file_system = local_executor_file_system();
|
||||
Config::load_config_with_layer_stack(
|
||||
config_toml,
|
||||
harness_overrides,
|
||||
codex_home,
|
||||
config_layer_stack,
|
||||
file_system,
|
||||
)
|
||||
.await
|
||||
}
|
||||
}
|
||||
|
||||
@@ -759,11 +769,12 @@ fn push_smart_approvals_alias_migration_edits(
|
||||
/// loads only see the canonical feature flag name.
|
||||
async fn maybe_migrate_smart_approvals_alias(codex_home: &Path) -> std::io::Result<bool> {
|
||||
let config_path = codex_home.join(CONFIG_TOML_FILE);
|
||||
if !tokio::fs::try_exists(&config_path).await? {
|
||||
return Ok(false);
|
||||
}
|
||||
|
||||
let config_contents = tokio::fs::read_to_string(&config_path).await?;
|
||||
let file_system = local_executor_file_system();
|
||||
let config_contents = match read_text_file(config_path.as_path(), &*file_system).await {
|
||||
Ok(contents) => contents,
|
||||
Err(err) if err.kind() == ErrorKind::NotFound => return Ok(false),
|
||||
Err(err) => return Err(err),
|
||||
};
|
||||
let Ok(config_toml) = toml::from_str::<ConfigToml>(&config_contents) else {
|
||||
return Ok(false);
|
||||
};
|
||||
@@ -818,8 +829,9 @@ impl Config {
|
||||
}
|
||||
|
||||
/// Load a default configuration when user config files are invalid.
|
||||
pub fn load_default_with_cli_overrides(
|
||||
pub async fn load_default_with_cli_overrides(
|
||||
cli_overrides: Vec<(String, TomlValue)>,
|
||||
file_system: Arc<dyn ExecutorFileSystem>,
|
||||
) -> std::io::Result<Self> {
|
||||
let codex_home = find_codex_home()?;
|
||||
let mut merged = toml::Value::try_from(ConfigToml::default()).map_err(|e| {
|
||||
@@ -836,7 +848,9 @@ impl Config {
|
||||
ConfigOverrides::default(),
|
||||
codex_home,
|
||||
ConfigLayerStack::default(),
|
||||
file_system,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
/// This is a secondary way of creating [Config], which is appropriate when
|
||||
@@ -900,8 +914,45 @@ pub(crate) fn deserialize_config_toml_with_base(
|
||||
.map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))
|
||||
}
|
||||
|
||||
fn load_catalog_json(path: &AbsolutePathBuf) -> std::io::Result<ModelsResponse> {
|
||||
let file_contents = std::fs::read_to_string(path)?;
|
||||
fn absolute_path_for_filesystem_read(path: &Path) -> std::io::Result<AbsolutePathBuf> {
|
||||
let absolute_path = if path.is_absolute() {
|
||||
path.to_path_buf()
|
||||
} else {
|
||||
std::env::current_dir()?.join(path)
|
||||
};
|
||||
AbsolutePathBuf::try_from(absolute_path.as_path())
|
||||
.map_err(|err| std::io::Error::new(std::io::ErrorKind::InvalidInput, err))
|
||||
}
|
||||
|
||||
async fn read_text_file_with_executor_filesystem(
|
||||
file_system: &dyn ExecutorFileSystem,
|
||||
path: &Path,
|
||||
) -> std::io::Result<String> {
|
||||
let absolute_path = absolute_path_for_filesystem_read(path)?;
|
||||
let bytes = file_system.read_file(&absolute_path).await?;
|
||||
String::from_utf8(bytes).map_err(|err| {
|
||||
std::io::Error::new(
|
||||
ErrorKind::InvalidData,
|
||||
format!(
|
||||
"failed to decode UTF-8 from {}: {err}",
|
||||
absolute_path.display()
|
||||
),
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
async fn read_text_file(
|
||||
path: &Path,
|
||||
file_system: &dyn ExecutorFileSystem,
|
||||
) -> std::io::Result<String> {
|
||||
read_text_file_with_executor_filesystem(file_system, path).await
|
||||
}
|
||||
|
||||
async fn load_catalog_json(
|
||||
path: &AbsolutePathBuf,
|
||||
file_system: &dyn ExecutorFileSystem,
|
||||
) -> std::io::Result<ModelsResponse> {
|
||||
let file_contents = read_text_file(path.as_path(), file_system).await?;
|
||||
let catalog = serde_json::from_str::<ModelsResponse>(&file_contents).map_err(|err| {
|
||||
std::io::Error::new(
|
||||
ErrorKind::InvalidData,
|
||||
@@ -923,12 +974,14 @@ fn load_catalog_json(path: &AbsolutePathBuf) -> std::io::Result<ModelsResponse>
|
||||
Ok(catalog)
|
||||
}
|
||||
|
||||
fn load_model_catalog(
|
||||
async fn load_model_catalog(
|
||||
model_catalog_json: Option<AbsolutePathBuf>,
|
||||
file_system: &dyn ExecutorFileSystem,
|
||||
) -> std::io::Result<Option<ModelsResponse>> {
|
||||
model_catalog_json
|
||||
.map(|path| load_catalog_json(&path))
|
||||
.transpose()
|
||||
let Some(path) = model_catalog_json else {
|
||||
return Ok(None);
|
||||
};
|
||||
load_catalog_json(&path, file_system).await.map(Some)
|
||||
}
|
||||
|
||||
fn filter_mcp_servers_by_requirements(
|
||||
@@ -2113,14 +2166,37 @@ impl Config {
|
||||
) -> std::io::Result<Self> {
|
||||
// Note this ignores requirements.toml enforcement for tests.
|
||||
let config_layer_stack = ConfigLayerStack::default();
|
||||
Self::load_config_with_layer_stack(cfg, overrides, codex_home, config_layer_stack)
|
||||
let runtime = tokio::runtime::Builder::new_multi_thread()
|
||||
.worker_threads(1)
|
||||
.enable_all()
|
||||
.build()
|
||||
.map_err(|err| {
|
||||
std::io::Error::other(format!("failed to build tokio runtime: {err}"))
|
||||
})?;
|
||||
let file_system = local_executor_file_system();
|
||||
let (tx, rx) = std::sync::mpsc::sync_channel(1);
|
||||
runtime.spawn(async move {
|
||||
let result = Self::load_config_with_layer_stack(
|
||||
cfg,
|
||||
overrides,
|
||||
codex_home,
|
||||
config_layer_stack,
|
||||
file_system,
|
||||
)
|
||||
.await;
|
||||
let _ = tx.send(result);
|
||||
});
|
||||
rx.recv().map_err(|err| {
|
||||
std::io::Error::other(format!("failed to receive config from runtime: {err}"))
|
||||
})?
|
||||
}
|
||||
|
||||
pub(crate) fn load_config_with_layer_stack(
|
||||
pub(crate) async fn load_config_with_layer_stack(
|
||||
cfg: ConfigToml,
|
||||
overrides: ConfigOverrides,
|
||||
codex_home: PathBuf,
|
||||
config_layer_stack: ConfigLayerStack,
|
||||
file_system: Arc<dyn ExecutorFileSystem>,
|
||||
) -> std::io::Result<Self> {
|
||||
validate_reserved_model_provider_ids(&cfg.model_providers)
|
||||
.map_err(|message| std::io::Error::new(std::io::ErrorKind::InvalidInput, message))?;
|
||||
@@ -2137,7 +2213,7 @@ impl Config {
|
||||
network: network_requirements,
|
||||
} = config_layer_stack.requirements().clone();
|
||||
|
||||
let user_instructions = Self::load_instructions(Some(&codex_home));
|
||||
let user_instructions = Self::load_instructions(Some(&codex_home), &*file_system).await;
|
||||
let mut startup_warnings = Vec::new();
|
||||
|
||||
// Destructure ConfigOverrides fully to ensure all overrides are applied.
|
||||
@@ -2265,8 +2341,10 @@ impl Config {
|
||||
None => WindowsSandboxLevel::from_features(&features),
|
||||
};
|
||||
let memories_root = memory_root(&codex_home);
|
||||
std::fs::create_dir_all(&memories_root)?;
|
||||
let memories_root = AbsolutePathBuf::from_absolute_path(&memories_root)?;
|
||||
file_system
|
||||
.create_directory(&memories_root, CreateDirectoryOptions { recursive: true })
|
||||
.await?;
|
||||
if !additional_writable_roots
|
||||
.iter()
|
||||
.any(|existing| existing == &memories_root)
|
||||
@@ -2380,8 +2458,13 @@ impl Config {
|
||||
.unwrap_or(WebSearchMode::Cached);
|
||||
let web_search_config = resolve_web_search_config(&cfg, &config_profile);
|
||||
|
||||
let agent_roles =
|
||||
agent_roles::load_agent_roles(&cfg, &config_layer_stack, &mut startup_warnings)?;
|
||||
let agent_roles = agent_roles::load_agent_roles(
|
||||
&cfg,
|
||||
&config_layer_stack,
|
||||
&mut startup_warnings,
|
||||
&*file_system,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let openai_base_url = cfg
|
||||
.openai_base_url
|
||||
@@ -2546,8 +2629,12 @@ impl Config {
|
||||
.model_instructions_file
|
||||
.as_ref()
|
||||
.or(cfg.model_instructions_file.as_ref());
|
||||
let file_base_instructions =
|
||||
Self::try_read_non_empty_file(model_instructions_path, "model instructions file")?;
|
||||
let file_base_instructions = Self::try_read_non_empty_file(
|
||||
model_instructions_path,
|
||||
"model instructions file",
|
||||
&*file_system,
|
||||
)
|
||||
.await?;
|
||||
let base_instructions = base_instructions.or(file_base_instructions);
|
||||
let developer_instructions = developer_instructions.or(cfg.developer_instructions);
|
||||
let guardian_developer_instructions = guardian_developer_instructions_from_requirements(
|
||||
@@ -2569,7 +2656,9 @@ impl Config {
|
||||
let file_compact_prompt = Self::try_read_non_empty_file(
|
||||
experimental_compact_prompt_path,
|
||||
"experimental compact prompt file",
|
||||
)?;
|
||||
&*file_system,
|
||||
)
|
||||
.await?;
|
||||
let compact_prompt = compact_prompt.or(file_compact_prompt);
|
||||
let js_repl_node_path = js_repl_node_path_override
|
||||
.or(config_profile.js_repl_node_path.map(Into::into))
|
||||
@@ -2597,7 +2686,9 @@ impl Config {
|
||||
.model_catalog_json
|
||||
.clone()
|
||||
.or(cfg.model_catalog_json.clone()),
|
||||
)?;
|
||||
&*file_system,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let log_dir = cfg
|
||||
.log_dir
|
||||
@@ -2868,12 +2959,15 @@ impl Config {
|
||||
Ok(config)
|
||||
}
|
||||
|
||||
fn load_instructions(codex_dir: Option<&Path>) -> Option<String> {
|
||||
async fn load_instructions(
|
||||
codex_dir: Option<&Path>,
|
||||
file_system: &dyn ExecutorFileSystem,
|
||||
) -> Option<String> {
|
||||
let base = codex_dir?;
|
||||
for candidate in [LOCAL_PROJECT_DOC_FILENAME, DEFAULT_PROJECT_DOC_FILENAME] {
|
||||
let mut path = base.to_path_buf();
|
||||
path.push(candidate);
|
||||
if let Ok(contents) = std::fs::read_to_string(&path) {
|
||||
if let Ok(contents) = read_text_file(path.as_path(), file_system).await {
|
||||
let trimmed = contents.trim();
|
||||
if !trimmed.is_empty() {
|
||||
return Some(trimmed.to_string());
|
||||
@@ -2886,20 +2980,23 @@ impl Config {
|
||||
/// If `path` is `Some`, attempts to read the file at the given path and
|
||||
/// returns its contents as a trimmed `String`. If the file is empty, or
|
||||
/// is `Some` but cannot be read, returns an `Err`.
|
||||
fn try_read_non_empty_file(
|
||||
async fn try_read_non_empty_file(
|
||||
path: Option<&AbsolutePathBuf>,
|
||||
context: &str,
|
||||
file_system: &dyn ExecutorFileSystem,
|
||||
) -> std::io::Result<Option<String>> {
|
||||
let Some(path) = path else {
|
||||
return Ok(None);
|
||||
};
|
||||
|
||||
let contents = std::fs::read_to_string(path).map_err(|e| {
|
||||
std::io::Error::new(
|
||||
e.kind(),
|
||||
format!("failed to read {context} {}: {e}", path.display()),
|
||||
)
|
||||
})?;
|
||||
let contents = read_text_file(path.as_path(), file_system)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
std::io::Error::new(
|
||||
e.kind(),
|
||||
format!("failed to read {context} {}: {e}", path.display()),
|
||||
)
|
||||
})?;
|
||||
|
||||
let s = contents.trim().to_string();
|
||||
if s.is_empty() {
|
||||
|
||||
@@ -1002,8 +1002,8 @@ fn guardian_review_session_config_uses_parent_active_model_instead_of_hardcoded_
|
||||
assert_eq!(guardian_config.model, Some("active-model".to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn guardian_review_session_config_uses_requirements_guardian_override() {
|
||||
#[tokio::test]
|
||||
async fn guardian_review_session_config_uses_requirements_guardian_override() {
|
||||
let codex_home = tempfile::tempdir().expect("create temp dir");
|
||||
let workspace = tempfile::tempdir().expect("create temp dir");
|
||||
let config_layer_stack = ConfigLayerStack::new(
|
||||
@@ -1017,6 +1017,7 @@ fn guardian_review_session_config_uses_requirements_guardian_override() {
|
||||
},
|
||||
)
|
||||
.expect("config layer stack");
|
||||
let file_system = codex_exec_server::Environment::default().get_filesystem();
|
||||
let parent_config = Config::load_config_with_layer_stack(
|
||||
ConfigToml::default(),
|
||||
ConfigOverrides {
|
||||
@@ -1025,7 +1026,9 @@ fn guardian_review_session_config_uses_requirements_guardian_override() {
|
||||
},
|
||||
codex_home.path().to_path_buf(),
|
||||
config_layer_stack,
|
||||
file_system,
|
||||
)
|
||||
.await
|
||||
.expect("load config");
|
||||
|
||||
let guardian_config =
|
||||
@@ -1038,13 +1041,15 @@ fn guardian_review_session_config_uses_requirements_guardian_override() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn guardian_review_session_config_uses_default_guardian_policy_without_requirements_override() {
|
||||
#[tokio::test]
|
||||
async fn guardian_review_session_config_uses_default_guardian_policy_without_requirements_override()
|
||||
{
|
||||
let codex_home = tempfile::tempdir().expect("create temp dir");
|
||||
let workspace = tempfile::tempdir().expect("create temp dir");
|
||||
let config_layer_stack =
|
||||
ConfigLayerStack::new(Vec::new(), Default::default(), Default::default())
|
||||
.expect("config layer stack");
|
||||
let file_system = codex_exec_server::Environment::default().get_filesystem();
|
||||
let parent_config = Config::load_config_with_layer_stack(
|
||||
ConfigToml::default(),
|
||||
ConfigOverrides {
|
||||
@@ -1053,7 +1058,9 @@ fn guardian_review_session_config_uses_default_guardian_policy_without_requireme
|
||||
},
|
||||
codex_home.path().to_path_buf(),
|
||||
config_layer_stack,
|
||||
file_system,
|
||||
)
|
||||
.await
|
||||
.expect("load config");
|
||||
|
||||
let guardian_config =
|
||||
|
||||
@@ -128,7 +128,11 @@ async fn write_workspace_png(
|
||||
height: u32,
|
||||
rgba: [u8; 4],
|
||||
) -> anyhow::Result<PathBuf> {
|
||||
write_workspace_file(test, rel_path, png_bytes(width, height, rgba)?).await
|
||||
let image = ImageBuffer::from_pixel(width, height, Rgba(rgba));
|
||||
let mut cursor = Cursor::new(Vec::new());
|
||||
DynamicImage::ImageRgba8(image).write_to(&mut cursor, image::ImageFormat::Png)?;
|
||||
let bytes = cursor.into_inner();
|
||||
write_workspace_file(test, rel_path, bytes)?.await
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
|
||||
Reference in New Issue
Block a user