mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
chore: enusre the logic that creates ConfigLayerStack has access to cwd (#8353)
`load_config_layers_state()` should load config from a `.codex/config.toml` in any folder between the `cwd` for a thread and the project root. Though in order to do that, `load_config_layers_state()` needs to know what the `cwd` is, so this PR does the work to thread the `cwd` through for existing callsites. A notable exception is the `/config` endpoint in app server for which a `cwd` is not guaranteed to be associated with the query, so the `cwd` param is `Option<AbsolutePathBuf>` to account for this case. The logic to make use of the `cwd` will be done in a follow-up PR.
This commit is contained in:
committed by
GitHub
Unverified
parent
f0dc6fd3c7
commit
a6974087e5
@@ -346,8 +346,13 @@ impl ConfigBuilder {
|
||||
let cli_overrides = cli_overrides.unwrap_or_default();
|
||||
let harness_overrides = harness_overrides.unwrap_or_default();
|
||||
let loader_overrides = loader_overrides.unwrap_or_default();
|
||||
let cwd = match harness_overrides.cwd.as_deref() {
|
||||
Some(path) => AbsolutePathBuf::try_from(path)?,
|
||||
None => AbsolutePathBuf::current_dir()?,
|
||||
};
|
||||
let config_layer_stack =
|
||||
load_config_layers_state(&codex_home, &cli_overrides, loader_overrides).await?;
|
||||
load_config_layers_state(&codex_home, Some(cwd), &cli_overrides, loader_overrides)
|
||||
.await?;
|
||||
let merged_toml = config_layer_stack.effective_config();
|
||||
|
||||
// Note that each layer in ConfigLayerStack should have resolved
|
||||
@@ -401,10 +406,16 @@ impl Config {
|
||||
/// applied yet, which risks failing to enforce required constraints.
|
||||
pub async fn load_config_as_toml_with_cli_overrides(
|
||||
codex_home: &Path,
|
||||
cwd: &AbsolutePathBuf,
|
||||
cli_overrides: Vec<(String, TomlValue)>,
|
||||
) -> std::io::Result<ConfigToml> {
|
||||
let config_layer_stack =
|
||||
load_config_layers_state(codex_home, &cli_overrides, LoaderOverrides::default()).await?;
|
||||
let config_layer_stack = load_config_layers_state(
|
||||
codex_home,
|
||||
Some(cwd.clone()),
|
||||
&cli_overrides,
|
||||
LoaderOverrides::default(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let merged_toml = config_layer_stack.effective_config();
|
||||
let cfg = deserialize_config_toml_with_base(merged_toml, codex_home).map_err(|e| {
|
||||
@@ -438,8 +449,12 @@ pub async fn load_global_mcp_servers(
|
||||
// config layers for deprecated fields rather than reporting on the merged
|
||||
// result.
|
||||
let cli_overrides = Vec::<(String, TomlValue)>::new();
|
||||
// There is no cwd/project context for this query, so this will not include
|
||||
// MCP servers defined in in-repo .codex/ folders.
|
||||
let cwd: Option<AbsolutePathBuf> = None;
|
||||
let config_layer_stack =
|
||||
load_config_layers_state(codex_home, &cli_overrides, LoaderOverrides::default()).await?;
|
||||
load_config_layers_state(codex_home, cwd, &cli_overrides, LoaderOverrides::default())
|
||||
.await?;
|
||||
let merged_toml = config_layer_stack.effective_config();
|
||||
let Some(servers_value) = merged_toml.get("mcp_servers") else {
|
||||
return Ok(BTreeMap::new());
|
||||
@@ -1953,8 +1968,9 @@ trust_level = "trusted"
|
||||
managed_preferences_base64: None,
|
||||
};
|
||||
|
||||
let cwd = AbsolutePathBuf::try_from(codex_home.path())?;
|
||||
let config_layer_stack =
|
||||
load_config_layers_state(codex_home.path(), &Vec::new(), overrides).await?;
|
||||
load_config_layers_state(codex_home.path(), Some(cwd), &Vec::new(), overrides).await?;
|
||||
let cfg = deserialize_config_toml_with_base(
|
||||
config_layer_stack.effective_config(),
|
||||
codex_home.path(),
|
||||
@@ -2072,8 +2088,10 @@ trust_level = "trusted"
|
||||
managed_preferences_base64: None,
|
||||
};
|
||||
|
||||
let cwd = AbsolutePathBuf::try_from(codex_home.path())?;
|
||||
let config_layer_stack = load_config_layers_state(
|
||||
codex_home.path(),
|
||||
Some(cwd),
|
||||
&[("model".to_string(), TomlValue::String("cli".to_string()))],
|
||||
overrides,
|
||||
)
|
||||
|
||||
@@ -132,7 +132,7 @@ impl ConfigService {
|
||||
params: ConfigReadParams,
|
||||
) -> Result<ConfigReadResponse, ConfigServiceError> {
|
||||
let layers = self
|
||||
.load_layers_state()
|
||||
.load_thread_agnostic_config()
|
||||
.await
|
||||
.map_err(|err| ConfigServiceError::io("failed to read configuration layers", err))?;
|
||||
|
||||
@@ -185,7 +185,7 @@ impl ConfigService {
|
||||
&self,
|
||||
) -> Result<codex_app_server_protocol::UserSavedConfig, ConfigServiceError> {
|
||||
let layers = self
|
||||
.load_layers_state()
|
||||
.load_thread_agnostic_config()
|
||||
.await
|
||||
.map_err(|err| ConfigServiceError::io("failed to load configuration", err))?;
|
||||
|
||||
@@ -219,7 +219,7 @@ impl ConfigService {
|
||||
}
|
||||
|
||||
let layers = self
|
||||
.load_layers_state()
|
||||
.load_thread_agnostic_config()
|
||||
.await
|
||||
.map_err(|err| ConfigServiceError::io("failed to load configuration", err))?;
|
||||
let user_layer = match layers.get_user_layer() {
|
||||
@@ -328,9 +328,14 @@ impl ConfigService {
|
||||
})
|
||||
}
|
||||
|
||||
async fn load_layers_state(&self) -> std::io::Result<ConfigLayerStack> {
|
||||
/// Loads a "thread-agnostic" config, which means the config layers do not
|
||||
/// include any in-repo .codex/ folders because there is no cwd/project root
|
||||
/// associated with this query.
|
||||
async fn load_thread_agnostic_config(&self) -> std::io::Result<ConfigLayerStack> {
|
||||
let cwd: Option<AbsolutePathBuf> = None;
|
||||
load_config_layers_state(
|
||||
&self.codex_home,
|
||||
cwd,
|
||||
&self.cli_overrides,
|
||||
self.loader_overrides.clone(),
|
||||
)
|
||||
|
||||
@@ -10,7 +10,7 @@ This module is the canonical place to **load and describe Codex configuration la
|
||||
|
||||
Exported from `codex_core::config_loader`:
|
||||
|
||||
- `load_config_layers_state(codex_home, cli_overrides, overrides) -> ConfigLayerStack`
|
||||
- `load_config_layers_state(codex_home, cwd_opt, cli_overrides, overrides) -> ConfigLayerStack`
|
||||
- `ConfigLayerStack`
|
||||
- `effective_config() -> toml::Value`
|
||||
- `origins() -> HashMap<String, ConfigLayerMetadata>`
|
||||
@@ -37,11 +37,14 @@ Most callers want the effective config plus metadata:
|
||||
|
||||
```rust
|
||||
use codex_core::config_loader::{load_config_layers_state, LoaderOverrides};
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use toml::Value as TomlValue;
|
||||
|
||||
let cli_overrides: Vec<(String, TomlValue)> = Vec::new();
|
||||
let cwd = AbsolutePathBuf::current_dir()?;
|
||||
let layers = load_config_layers_state(
|
||||
&codex_home,
|
||||
Some(cwd),
|
||||
&cli_overrides,
|
||||
LoaderOverrides::default(),
|
||||
).await?;
|
||||
|
||||
@@ -55,8 +55,14 @@ const DEFAULT_REQUIREMENTS_TOML_FILE_UNIX: &str = "/etc/codex/requirements.toml"
|
||||
/// (*) Only available on macOS via managed device profiles.
|
||||
///
|
||||
/// See https://developers.openai.com/codex/security for details.
|
||||
///
|
||||
/// When loading the config stack for a thread, there should be a `cwd`
|
||||
/// associated with it such that `cwd` should be `Some(...)`. Only for
|
||||
/// thread-agnostic config loading (e.g., for the app server's `/config`
|
||||
/// endpoint) should `cwd` be `None`.
|
||||
pub async fn load_config_layers_state(
|
||||
codex_home: &Path,
|
||||
cwd: Option<AbsolutePathBuf>,
|
||||
cli_overrides: &[(String, TomlValue)],
|
||||
overrides: LoaderOverrides,
|
||||
) -> io::Result<ConfigLayerStack> {
|
||||
@@ -122,6 +128,7 @@ pub async fn load_config_layers_state(
|
||||
}
|
||||
|
||||
// TODO(mbolin): Add layers for cwd, tree, and repo config files.
|
||||
let _ = cwd;
|
||||
|
||||
// Add a layer for runtime overrides from the CLI or UI, if any exist.
|
||||
if !cli_overrides.is_empty() {
|
||||
|
||||
@@ -5,6 +5,7 @@ use crate::config_loader::ConfigRequirements;
|
||||
use crate::config_loader::config_requirements::ConfigRequirementsToml;
|
||||
use crate::config_loader::load_requirements_toml;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::tempdir;
|
||||
use toml::Value as TomlValue;
|
||||
@@ -40,9 +41,15 @@ extra = true
|
||||
managed_preferences_base64: None,
|
||||
};
|
||||
|
||||
let state = load_config_layers_state(tmp.path(), &[] as &[(String, TomlValue)], overrides)
|
||||
.await
|
||||
.expect("load config");
|
||||
let cwd = AbsolutePathBuf::try_from(tmp.path()).expect("cwd");
|
||||
let state = load_config_layers_state(
|
||||
tmp.path(),
|
||||
Some(cwd),
|
||||
&[] as &[(String, TomlValue)],
|
||||
overrides,
|
||||
)
|
||||
.await
|
||||
.expect("load config");
|
||||
let loaded = state.effective_config();
|
||||
let table = loaded.as_table().expect("top-level table expected");
|
||||
|
||||
@@ -68,9 +75,15 @@ async fn returns_empty_when_all_layers_missing() {
|
||||
managed_preferences_base64: None,
|
||||
};
|
||||
|
||||
let layers = load_config_layers_state(tmp.path(), &[] as &[(String, TomlValue)], overrides)
|
||||
.await
|
||||
.expect("load layers");
|
||||
let cwd = AbsolutePathBuf::try_from(tmp.path()).expect("cwd");
|
||||
let layers = load_config_layers_state(
|
||||
tmp.path(),
|
||||
Some(cwd),
|
||||
&[] as &[(String, TomlValue)],
|
||||
overrides,
|
||||
)
|
||||
.await
|
||||
.expect("load layers");
|
||||
assert!(
|
||||
layers.get_user_layer().is_none(),
|
||||
"no user layer when CODEX_HOME/config.toml does not exist"
|
||||
@@ -138,9 +151,15 @@ flag = true
|
||||
managed_preferences_base64: Some(encoded),
|
||||
};
|
||||
|
||||
let state = load_config_layers_state(tmp.path(), &[] as &[(String, TomlValue)], overrides)
|
||||
.await
|
||||
.expect("load config");
|
||||
let cwd = AbsolutePathBuf::try_from(tmp.path()).expect("cwd");
|
||||
let state = load_config_layers_state(
|
||||
tmp.path(),
|
||||
Some(cwd),
|
||||
&[] as &[(String, TomlValue)],
|
||||
overrides,
|
||||
)
|
||||
.await
|
||||
.expect("load config");
|
||||
let loaded = state.effective_config();
|
||||
let nested = loaded
|
||||
.get("nested")
|
||||
|
||||
Reference in New Issue
Block a user