Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions codex-rs/core/src/config_loader/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use codex_protocol::config_types::TrustLevel;
use codex_protocol::protocol::AskForApproval;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_absolute_path::AbsolutePathBufGuard;
use dunce::canonicalize as normalize_path;
use serde::Deserialize;
use std::io;
use std::path::Path;
Expand Down Expand Up @@ -226,6 +227,7 @@ pub async fn load_config_layers_state(
&cwd,
&project_trust_context.project_root,
&project_trust_context,
codex_home,
)
.await?;
layers.extend(project_layers);
Expand Down Expand Up @@ -661,7 +663,11 @@ async fn load_project_layers(
cwd: &AbsolutePathBuf,
project_root: &AbsolutePathBuf,
trust_context: &ProjectTrustContext,
codex_home: &Path,
) -> io::Result<Vec<ConfigLayerEntry>> {
let codex_home_abs = AbsolutePathBuf::from_absolute_path(codex_home)?;
let codex_home_normalized =
normalize_path(codex_home_abs.as_path()).unwrap_or_else(|_| codex_home_abs.to_path_buf());
let mut dirs = cwd
.as_path()
.ancestors()
Expand Down Expand Up @@ -692,6 +698,11 @@ async fn load_project_layers(
let layer_dir = AbsolutePathBuf::from_absolute_path(dir)?;
let decision = trust_context.decision_for_dir(&layer_dir);
let dot_codex_abs = AbsolutePathBuf::from_absolute_path(&dot_codex)?;
let dot_codex_normalized =
normalize_path(dot_codex_abs.as_path()).unwrap_or_else(|_| dot_codex_abs.to_path_buf());
if dot_codex_abs == codex_home_abs || dot_codex_normalized == codex_home_normalized {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason you need both here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first is a cheap, reliable equality check. The second handles symlinked or differently-canonicalized paths (which can happen with worktrees, temp dirs, or custom $CODEX_HOME).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this feels unnecessary to me but also don't think it's actively harmful

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah its an uncommon situation but certainly does happen

continue;
}
let config_file = dot_codex_abs.join(CONFIG_TOML_FILE)?;
match tokio::fs::read_to_string(&config_file).await {
Ok(contents) => {
Expand Down
95 changes: 95 additions & 0 deletions codex-rs/core/src/config_loader/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,101 @@ async fn project_layer_is_added_when_dot_codex_exists_without_config_toml() -> s
Ok(())
}

#[tokio::test]
async fn codex_home_is_not_loaded_as_project_layer_from_home_dir() -> std::io::Result<()> {
let tmp = tempdir()?;
let home_dir = tmp.path().join("home");
let codex_home = home_dir.join(".codex");
tokio::fs::create_dir_all(&codex_home).await?;
tokio::fs::write(codex_home.join(CONFIG_TOML_FILE), "foo = \"user\"\n").await?;

let cwd = AbsolutePathBuf::from_absolute_path(&home_dir)?;
let layers = load_config_layers_state(
&codex_home,
Some(cwd),
&[] as &[(String, TomlValue)],
LoaderOverrides::default(),
)
.await?;

let project_layers: Vec<_> = layers
.get_layers(
super::ConfigLayerStackOrdering::HighestPrecedenceFirst,
true,
)
.into_iter()
.filter(|layer| matches!(layer.name, super::ConfigLayerSource::Project { .. }))
.collect();
let expected: Vec<&ConfigLayerEntry> = Vec::new();
assert_eq!(expected, project_layers);
assert_eq!(
layers.effective_config().get("foo"),
Some(&TomlValue::String("user".to_string()))
);

Ok(())
}

#[tokio::test]
async fn codex_home_within_project_tree_is_not_double_loaded() -> std::io::Result<()> {
let tmp = tempdir()?;
let project_root = tmp.path().join("project");
let nested = project_root.join("child");
let project_dot_codex = project_root.join(".codex");
let nested_dot_codex = nested.join(".codex");

tokio::fs::create_dir_all(&nested_dot_codex).await?;
tokio::fs::create_dir_all(project_root.join(".git")).await?;
tokio::fs::write(nested_dot_codex.join(CONFIG_TOML_FILE), "foo = \"child\"\n").await?;

tokio::fs::create_dir_all(&project_dot_codex).await?;
make_config_for_test(&project_dot_codex, &project_root, TrustLevel::Trusted, None).await?;
let user_config_path = project_dot_codex.join(CONFIG_TOML_FILE);
let user_config_contents = tokio::fs::read_to_string(&user_config_path).await?;
tokio::fs::write(
&user_config_path,
format!("foo = \"user\"\n{user_config_contents}"),
)
.await?;

let cwd = AbsolutePathBuf::from_absolute_path(&nested)?;
let layers = load_config_layers_state(
&project_dot_codex,
Some(cwd),
&[] as &[(String, TomlValue)],
LoaderOverrides::default(),
)
.await?;

let project_layers: Vec<_> = layers
.get_layers(
super::ConfigLayerStackOrdering::HighestPrecedenceFirst,
true,
)
.into_iter()
.filter(|layer| matches!(layer.name, super::ConfigLayerSource::Project { .. }))
.collect();

let child_config: TomlValue = toml::from_str("foo = \"child\"\n").expect("parse child config");
assert_eq!(
vec![&ConfigLayerEntry {
name: super::ConfigLayerSource::Project {
dot_codex_folder: AbsolutePathBuf::from_absolute_path(&nested_dot_codex)?,
},
config: child_config.clone(),
version: version_for_toml(&child_config),
disabled_reason: None,
}],
project_layers
);
assert_eq!(
layers.effective_config().get("foo"),
Some(&TomlValue::String("child".to_string()))
);

Ok(())
}

#[tokio::test]
async fn project_layers_disabled_when_untrusted_or_unknown() -> std::io::Result<()> {
let tmp = tempdir()?;
Expand Down
Loading