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

Refactor config loading to use filesystem abstraction (#18209)

Browse Source 
Initial pass propagating FileSystem through config loading.
This commit is contained in:
pakrym-oai
2026-04-16 17:51:21 -07:00
committed by GitHub
Unverified
parent 2967900d81
commit 9effa0509f
30 changed files with 507 additions and 315 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/config/agent_roles.rs
+94 -72
View File
@@ -4,6 +4,7 @@ use crate::config_loader::ConfigLayerStackOrdering;
use codex_config::config_toml::AgentRoleToml;
use codex_config::config_toml::AgentsToml;
use codex_config::config_toml::ConfigToml;
use codex_exec_server::ExecutorFileSystem;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_absolute_path::AbsolutePathBufGuard;
use serde::Deserialize;
@@ -14,7 +15,8 @@ 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(
fs: &dyn ExecutorFileSystem,
cfg: &ConfigToml,
config_layer_stack: &ConfigLayerStack,
startup_warnings: &mut Vec<String>,
@@ -24,7 +26,7 @@ pub(crate) fn load_agent_roles(
/*include_disabled*/ false,
);
if layers.is_empty() {
return load_agent_roles_without_layers(cfg);
return load_agent_roles_without_layers(fs, cfg).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(fs, declared_role_name, role_toml).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);
}
@@ -68,10 +71,13 @@ pub(crate) fn load_agent_roles(
if let Some(config_folder) = layer.config_folder() {
for (role_name, role) in discover_agent_roles_in_dir(
config_folder.as_path().join("agents").as_path(),
fs,
&config_folder.join("agents"),
&declared_role_files,
startup_warnings,
)? {
)
.await?
{
if layer_roles.contains_key(&role_name) {
push_agent_role_warning(
startup_warnings,
@@ -113,13 +119,14 @@ 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(
fs: &dyn ExecutorFileSystem,
cfg: &ConfigToml,
) -> 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(fs, declared_role_name, role_toml).await?;
validate_required_agent_role_description(&role_name, role.description.as_deref())?;
if roles.insert(role_name.clone(), role).is_some() {
@@ -134,14 +141,17 @@ fn load_agent_roles_without_layers(
Ok(roles)
}
fn read_declared_role(
async fn read_declared_role(
fs: &dyn ExecutorFileSystem,
declared_role_name: &str,
role_toml: &AgentRoleToml,
) -> std::io::Result<(String, AgentRoleConfig)> {
let mut role = agent_role_config_from_toml(declared_role_name, role_toml)?;
let mut role = agent_role_config_from_toml(fs, declared_role_name, role_toml).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 config_file = AbsolutePathBuf::from_absolute_path(config_file)?;
let parsed_file =
read_resolved_agent_role_file(fs, &config_file, Some(declared_role_name)).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);
@@ -171,12 +181,17 @@ fn agents_toml_from_layer(layer_toml: &TomlValue) -> std::io::Result<Option<Agen
.map_err(|err| std::io::Error::new(std::io::ErrorKind::InvalidData, err))
}
fn agent_role_config_from_toml(
async fn agent_role_config_from_toml(
fs: &dyn ExecutorFileSystem,
role_name: &str,
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 config_file = role
.config_file
.as_ref()
.map(AbsolutePathBuf::from_absolute_path)
.transpose()?;
validate_agent_role_config_file(fs, role_name, config_file.as_ref()).await?;
let description = normalize_agent_role_description(
&format!("agents.{role_name}.description"),
role.description.as_deref(),
@@ -188,7 +203,7 @@ fn agent_role_config_from_toml(
Ok(AgentRoleConfig {
description,
config_file,
config_file: config_file.map(AbsolutePathBuf::into_path_buf),
nickname_candidates,
})
}
@@ -293,15 +308,17 @@ pub(crate) fn parse_agent_role_file_contents(
})
}
fn read_resolved_agent_role_file(
path: &Path,
async fn read_resolved_agent_role_file(
fs: &dyn ExecutorFileSystem,
path: &AbsolutePathBuf,
role_name_hint: Option<&str>,
) -> std::io::Result<ResolvedAgentRoleFile> {
let contents = std::fs::read_to_string(path)?;
let contents = fs.read_file_text(path, /*sandbox*/ None).await?;
let config_base_dir = path.parent().unwrap_or_else(|| path.clone());
parse_agent_role_file_contents(
&contents,
path,
path.parent().unwrap_or(path),
path.as_path(),
config_base_dir.as_path(),
role_name_hint,
)
}
@@ -359,31 +376,35 @@ fn validate_agent_role_file_developer_instructions(
}
}
fn validate_agent_role_config_file(
async fn validate_agent_role_config_file(
fs: &dyn ExecutorFileSystem,
role_name: &str,
config_file: Option<&Path>,
config_file: Option<&AbsolutePathBuf>,
) -> 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 metadata = fs
.get_metadata(config_file, /*sandbox*/ None)
.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.as_path().display()
),
)
})?;
if metadata.is_file {
Ok(())
} else {
Err(std::io::Error::new(
std::io::ErrorKind::InvalidInput,
format!(
"agents.{role_name}.config_file must point to a file: {}",
config_file.display()
config_file.as_path().display()
),
))
}
@@ -441,19 +462,20 @@ fn normalize_agent_role_nickname_candidates(
Ok(Some(normalized_candidates))
}
fn discover_agent_roles_in_dir(
agents_dir: &Path,
async fn discover_agent_roles_in_dir(
fs: &dyn ExecutorFileSystem,
agents_dir: &AbsolutePathBuf,
declared_role_files: &BTreeSet<PathBuf>,
startup_warnings: &mut Vec<String>,
) -> std::io::Result<BTreeMap<String, AgentRoleConfig>> {
let mut roles = BTreeMap::new();
for agent_file in collect_agent_role_files(agents_dir)? {
if declared_role_files.contains(&agent_file) {
for agent_file in collect_agent_role_files(fs, agents_dir).await? {
if declared_role_files.contains(agent_file.as_path()) {
continue;
}
let parsed_file =
match read_resolved_agent_role_file(&agent_file, /*role_name_hint*/ None) {
match read_resolved_agent_role_file(fs, &agent_file, /*role_name_hint*/ None).await {
Ok(parsed_file) => parsed_file,
Err(err) => {
push_agent_role_warning(startup_warnings, err);
@@ -468,7 +490,7 @@ fn discover_agent_roles_in_dir(
std::io::ErrorKind::InvalidInput,
format!(
"duplicate agent role name `{role_name}` discovered in {}",
agents_dir.display()
agents_dir.as_path().display()
),
),
);
@@ -478,7 +500,7 @@ fn discover_agent_roles_in_dir(
role_name,
AgentRoleConfig {
description: parsed_file.description,
config_file: Some(agent_file),
config_file: Some(agent_file.to_path_buf()),
nickname_candidates: parsed_file.nickname_candidates,
},
);
@@ -487,36 +509,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(
fs: &dyn ExecutorFileSystem,
dir: &AbsolutePathBuf,
) -> std::io::Result<Vec<AbsolutePathBuf>> {
let mut files = Vec::new();
collect_agent_role_files_recursive(dir, &mut files)?;
files.sort();
Ok(files)
}
let mut dirs = vec![dir.clone()];
while let Some(dir) = dirs.pop() {
let entries = match fs.read_directory(&dir, /*sandbox*/ None).await {
Ok(entries) => entries,
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 entries {
let path = dir.join(entry.file_name);
if entry.is_directory {
dirs.push(path);
continue;
}
if entry.is_file
&& path
.as_path()
.extension()
.is_some_and(|extension| extension == "toml")
{
files.push(path);
}
}
}
Ok(())
files.sort();
Ok(files)
}
codex-rs/core/src/config/config_tests.rs
+25 -10
View File
@@ -44,6 +44,7 @@ use codex_config::types::SkillsConfig;
use codex_config::types::ToolSuggestDiscoverableType;
use codex_config::types::Tui;
use codex_config::types::TuiNotificationSettings;
use codex_exec_server::LOCAL_FS;
use codex_features::Feature;
use codex_features::FeaturesToml;
use codex_model_provider_info::LMSTUDIO_OSS_PROVIDER_ID;
@@ -1194,7 +1195,7 @@ network_access = false # This should be ignored.
sandbox_mode_override,
/*profile_sandbox_mode*/ None,
WindowsSandboxLevel::Disabled,
&PathBuf::from("/tmp/test"),
/*active_project*/ None,
/*sandbox_policy_constraint*/ None,
)
.await;
@@ -1215,7 +1216,7 @@ network_access = true # This should be ignored.
sandbox_mode_override,
/*profile_sandbox_mode*/ None,
WindowsSandboxLevel::Disabled,
&PathBuf::from("/tmp/test"),
/*active_project*/ None,
/*sandbox_policy_constraint*/ None,
)
.await;
@@ -1232,6 +1233,9 @@ writable_roots = [
]
exclude_tmpdir_env_var = true
exclude_slash_tmp = true
[projects."/tmp/test"]
trust_level = "trusted"
"#,
serde_json::json!(writable_root)
);
@@ -1244,7 +1248,7 @@ exclude_slash_tmp = true
sandbox_mode_override,
/*profile_sandbox_mode*/ None,
WindowsSandboxLevel::Disabled,
&PathBuf::from("/tmp/test"),
/*active_project*/ None,
/*sandbox_policy_constraint*/ None,
)
.await;
@@ -1273,9 +1277,6 @@ writable_roots = [
]
exclude_tmpdir_env_var = true
exclude_slash_tmp = true
[projects."/tmp/test"]
trust_level = "trusted"
"#,
serde_json::json!(writable_root)
);
@@ -1288,7 +1289,7 @@ trust_level = "trusted"
sandbox_mode_override,
/*profile_sandbox_mode*/ None,
WindowsSandboxLevel::Disabled,
&PathBuf::from("/tmp/test"),
/*active_project*/ None,
/*sandbox_policy_constraint*/ None,
)
.await;
@@ -2085,6 +2086,7 @@ async fn managed_config_overrides_oauth_store_mode() -> anyhow::Result<()> {
let cwd = codex_home.path().abs();
let config_layer_stack = load_config_layers_state(
LOCAL_FS.as_ref(),
codex_home.path(),
Some(cwd),
&Vec::new(),
@@ -2218,6 +2220,7 @@ async fn managed_config_wins_over_cli_overrides() -> anyhow::Result<()> {
let cwd = codex_home.path().abs();
let config_layer_stack = load_config_layers_state(
LOCAL_FS.as_ref(),
codex_home.path(),
Some(cwd),
&[("model".to_string(), TomlValue::String("cli".to_string()))],
@@ -3486,6 +3489,7 @@ async fn load_config_uses_requirements_guardian_policy_config() -> std::io::Resu
.map_err(std::io::Error::other)?;
let config = Config::load_config_with_layer_stack(
LOCAL_FS.as_ref(),
ConfigToml::default(),
ConfigOverrides {
cwd: Some(codex_home.path().to_path_buf()),
@@ -3518,6 +3522,7 @@ async fn load_config_ignores_empty_requirements_guardian_policy_config() -> std:
.map_err(std::io::Error::other)?;
let config = Config::load_config_with_layer_stack(
LOCAL_FS.as_ref(),
ConfigToml::default(),
ConfigOverrides {
cwd: Some(codex_home.path().to_path_buf()),
@@ -5330,6 +5335,7 @@ async fn test_requirements_web_search_mode_allowlist_does_not_warn_when_unset()
.expect("config layer stack");
let config = Config::load_config_with_layer_stack(
LOCAL_FS.as_ref(),
fixture.cfg.clone(),
ConfigOverrides {
cwd: Some(fixture.cwd_path()),
@@ -5537,13 +5543,16 @@ trust_level = "untrusted"
let cfg = toml::from_str::<ConfigToml>(config_with_untrusted)
.expect("TOML deserialization should succeed");
let active_project = ProjectConfig {
trust_level: Some(TrustLevel::Untrusted),
};
let resolution = cfg
.derive_sandbox_policy(
/*sandbox_mode_override*/ None,
/*profile_sandbox_mode*/ None,
WindowsSandboxLevel::Disabled,
&PathBuf::from("/tmp/test"),
Some(&active_project),
/*sandbox_policy_constraint*/ None,
)
.await;
@@ -5579,6 +5588,9 @@ async fn derive_sandbox_policy_falls_back_to_constraint_value_for_implicit_defau
)])),
..Default::default()
};
let active_project = ProjectConfig {
trust_level: Some(TrustLevel::Trusted),
};
let constrained = Constrained::new(SandboxPolicy::DangerFullAccess, |candidate| {
if matches!(candidate, SandboxPolicy::DangerFullAccess) {
Ok(())
@@ -5597,7 +5609,7 @@ async fn derive_sandbox_policy_falls_back_to_constraint_value_for_implicit_defau
/*sandbox_mode_override*/ None,
/*profile_sandbox_mode*/ None,
WindowsSandboxLevel::Disabled,
&project_path,
Some(&active_project),
Some(&constrained),
)
.await;
@@ -5621,6 +5633,9 @@ async fn derive_sandbox_policy_preserves_windows_downgrade_for_unsupported_fallb
)])),
..Default::default()
};
let active_project = ProjectConfig {
trust_level: Some(TrustLevel::Trusted),
};
let constrained = Constrained::new(SandboxPolicy::new_workspace_write_policy(), |candidate| {
if matches!(candidate, SandboxPolicy::WorkspaceWrite { .. }) {
Ok(())
@@ -5639,7 +5654,7 @@ async fn derive_sandbox_policy_preserves_windows_downgrade_for_unsupported_fallb
/*sandbox_mode_override*/ None,
/*profile_sandbox_mode*/ None,
WindowsSandboxLevel::Disabled,
&project_path,
Some(&active_project),
Some(&constrained),
)
.await;
codex-rs/core/src/config/mod.rs
+45 -15
View File
@@ -47,6 +47,8 @@ use codex_config::types::ToolSuggestDiscoverable;
use codex_config::types::TuiNotificationSettings;
use codex_config::types::UriBasedFileOpener;
use codex_config::types::WindowsSandboxModeToml;
use codex_exec_server::ExecutorFileSystem;
use codex_exec_server::LOCAL_FS;
use codex_features::Feature;
use codex_features::FeatureConfigSource;
use codex_features::FeatureOverrides;
@@ -54,6 +56,7 @@ use codex_features::FeatureToml;
use codex_features::Features;
use codex_features::FeaturesToml;
use codex_features::MultiAgentV2ConfigToml;
use codex_git_utils::resolve_root_git_project_for_trust;
use codex_login::AuthManagerConfig;
use codex_mcp::McpConfig;
use codex_model_provider_info::LEGACY_OLLAMA_CHAT_PROVIDER_ID;
@@ -689,6 +692,7 @@ impl ConfigBuilder {
};
harness_overrides.cwd = Some(cwd.to_path_buf());
let config_layer_stack = load_config_layers_state(
LOCAL_FS.as_ref(),
&codex_home,
Some(cwd),
&cli_overrides,
@@ -718,6 +722,7 @@ impl ConfigBuilder {
}
};
Config::load_config_with_layer_stack(
LOCAL_FS.as_ref(),
config_toml,
harness_overrides,
codex_home,
@@ -812,6 +817,7 @@ impl Config {
let codex_home = AbsolutePathBuf::from_absolute_path_checked(codex_home)?;
let config_toml = deserialize_config_toml_with_base(merged, &codex_home)?;
Self::load_config_with_layer_stack(
LOCAL_FS.as_ref(),
config_toml,
ConfigOverrides::default(),
codex_home,
@@ -849,6 +855,7 @@ pub async fn load_config_as_toml_with_cli_overrides(
cli_overrides: Vec<(String, TomlValue)>,
) -> std::io::Result<ConfigToml> {
let config_layer_stack = load_config_layers_state(
LOCAL_FS.as_ref(),
codex_home,
cwd.cloned(),
&cli_overrides,
@@ -1019,6 +1026,7 @@ pub async fn load_global_mcp_servers(
// MCP servers defined in in-repo .codex/ folders.
let cwd: Option<AbsolutePathBuf> = None;
let config_layer_stack = load_config_layers_state(
LOCAL_FS.as_ref(),
codex_home,
cwd,
&cli_overrides,
@@ -1420,10 +1428,18 @@ 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).await
Self::load_config_with_layer_stack(
LOCAL_FS.as_ref(),
cfg,
overrides,
codex_home,
config_layer_stack,
)
.await
}
pub(crate) async fn load_config_with_layer_stack(
fs: &dyn ExecutorFileSystem,
cfg: ConfigToml,
overrides: ConfigOverrides,
codex_home: AbsolutePathBuf,
@@ -1545,9 +1561,12 @@ impl Config {
.into_iter()
.map(|path| AbsolutePathBuf::resolve_path_against_base(path, resolved_cwd.as_path()))
.collect();
let repo_root = resolve_root_git_project_for_trust(fs, &resolved_cwd).await;
let active_project = cfg
.get_active_project(resolved_cwd.as_path())
.await
.get_active_project(
resolved_cwd.as_path(),
repo_root.as_ref().map(AbsolutePathBuf::as_path),
)
.unwrap_or(ProjectConfig { trust_level: None });
let permission_config_syntax = resolve_permission_config_syntax(
&config_layer_stack,
@@ -1643,7 +1662,7 @@ impl Config {
sandbox_mode,
config_profile.sandbox_mode,
windows_sandbox_level,
resolved_cwd.as_path(),
Some(&active_project),
Some(&constrained_sandbox_policy),
)
.await;
@@ -1712,7 +1731,8 @@ impl Config {
let multi_agent_v2 = resolve_multi_agent_v2_config(&cfg, &config_profile);
let agent_roles =
agent_roles::load_agent_roles(&cfg, &config_layer_stack, &mut startup_warnings)?;
agent_roles::load_agent_roles(fs, &cfg, &config_layer_stack, &mut startup_warnings)
.await?;
let openai_base_url = cfg
.openai_base_url
@@ -1861,8 +1881,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(
fs,
model_instructions_path,
"model instructions file",
)
.await?;
let base_instructions = base_instructions.or(file_base_instructions);
let developer_instructions = developer_instructions.or(cfg.developer_instructions);
let include_permissions_instructions = config_profile
@@ -1893,9 +1917,11 @@ impl Config {
.as_ref()
.or(cfg.experimental_compact_prompt_file.as_ref());
let file_compact_prompt = Self::try_read_non_empty_file(
fs,
experimental_compact_prompt_path,
"experimental compact prompt file",
)?;
)
.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))
@@ -2218,7 +2244,8 @@ 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(
fs: &dyn ExecutorFileSystem,
path: Option<&AbsolutePathBuf>,
context: &str,
) -> std::io::Result<Option<String>> {
@@ -2226,12 +2253,15 @@ impl Config {
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 = fs
.read_file_text(path, /*sandbox*/ None)
.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() {
codex-rs/core/src/config/service.rs
+2
View File
@@ -29,6 +29,7 @@ use codex_app_server_protocol::OverriddenMetadata;
use codex_app_server_protocol::WriteStatus;
use codex_config::CONFIG_TOML_FILE;
use codex_config::config_toml::ConfigToml;
use codex_exec_server::LOCAL_FS;
use codex_utils_absolute_path::AbsolutePathBuf;
use serde_json::Value as JsonValue;
use std::borrow::Cow;
@@ -424,6 +425,7 @@ impl ConfigService {
async fn load_thread_agnostic_config(&self) -> std::io::Result<ConfigLayerStack> {
let cwd: Option<AbsolutePathBuf> = None;
load_config_layers_state(
LOCAL_FS.as_ref(),
&self.codex_home,
cwd,
&self.cli_overrides,