core/core-agent-ide
8
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Ensure the env values of imported shell_environment_policy.set is string (#13402)

Browse source
This commit is contained in:
alexsong-oai 2026-03-03 16:12:23 -08:00 • committed by GitHub
parent b92146d48b
commit 1afbbc11c3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
1 changed files with 125 additions and 58 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

183
codex-rs/core/src/external_agent_config.rs
View file

@ -126,7 +126,7 @@ impl ExternalAgentConfigService {
items.push(ExternalAgentConfigMigrationItem {
item_type: ExternalAgentConfigMigrationItemType::Config,
description: format!(
"Migrate {} into {}.",
"Migrate {} into {}",
source_settings.display(),
target_config.display()
),
@ -153,7 +153,7 @@ impl ExternalAgentConfigService {
items.push(ExternalAgentConfigMigrationItem {
item_type: ExternalAgentConfigMigrationItemType::Skills,
description: format!(
"Copy skill folders from {} to {}.",
"Copy skill folders from {} to {}",
source_skills.display(),
target_skills.display()
),
@ -161,19 +161,23 @@ impl ExternalAgentConfigService {
});
}
let source_agents_md = repo_root.map_or_else(
|| self.claude_home.join("CLAUDE.md"),
|repo_root| repo_root.join("CLAUDE.md"),
);
let source_agents_md = if let Some(repo_root) = repo_root {
find_repo_agents_md_source(repo_root)?
} else {
let path = self.claude_home.join("CLAUDE.md");
is_non_empty_text_file(&path)?.then_some(path)
};
let target_agents_md = repo_root.map_or_else(
|| self.codex_home.join("AGENTS.md"),
|repo_root| repo_root.join("AGENTS.md"),
);
if source_agents_md.is_file() && is_missing_or_empty_text_file(&target_agents_md)? {
if let Some(source_agents_md) = source_agents_md
&& is_missing_or_empty_text_file(&target_agents_md)?
{
items.push(ExternalAgentConfigMigrationItem {
item_type: ExternalAgentConfigMigrationItemType::AgentsMd,
description: format!(
"Import {} to {}.",
"Import {} to {}",
source_agents_md.display(),
target_agents_md.display()
),
@ -283,7 +287,10 @@ impl ExternalAgentConfigService {
fn import_agents_md(&self, cwd: Option<&Path>) -> io::Result<()> {
let (source_agents_md, target_agents_md) = if let Some(repo_root) = find_repo_root(cwd)? {
(repo_root.join("CLAUDE.md"), repo_root.join("AGENTS.md"))
let Some(source_agents_md) = find_repo_agents_md_source(&repo_root)? else {
return Ok(());
};
(source_agents_md, repo_root.join("AGENTS.md"))
} else if cwd.is_some_and(|cwd| !cwd.as_os_str().is_empty()) {
return Ok(());
} else {
@ -292,7 +299,9 @@ impl ExternalAgentConfigService {
self.codex_home.join("AGENTS.md"),
)
};
if !source_agents_md.is_file() || !is_missing_or_empty_text_file(&target_agents_md)? {
if !is_non_empty_text_file(&source_agents_md)?
|| !is_missing_or_empty_text_file(&target_agents_md)?
{
return Ok(());
}
@ -376,6 +385,27 @@ fn is_missing_or_empty_text_file(path: &Path) -> io::Result<bool> {
Ok(fs::read_to_string(path)?.trim().is_empty())
}
fn is_non_empty_text_file(path: &Path) -> io::Result<bool> {
if !path.is_file() {
return Ok(false);
}
Ok(!fs::read_to_string(path)?.trim().is_empty())
}
fn find_repo_agents_md_source(repo_root: &Path) -> io::Result<Option<PathBuf>> {
for candidate in [
repo_root.join("CLAUDE.md"),
repo_root.join(".claude").join("CLAUDE.md"),
] {
if is_non_empty_text_file(&candidate)? {
return Ok(Some(candidate));
}
}
Ok(None)
}
fn copy_dir_recursive(source: &Path, target: &Path) -> io::Result<()> {
fs::create_dir_all(target)?;
@ -487,7 +517,7 @@ fn build_config_from_external(settings: &JsonValue) -> io::Result<TomlValue> {
shell_policy.insert("inherit".to_string(), TomlValue::String("core".to_string()));
shell_policy.insert(
"set".to_string(),
TomlValue::Table(json_object_to_toml_table(env)?),
TomlValue::Table(json_object_to_env_toml_table(env)),
);
root.insert(
"shell_environment_policy".to_string(),
@ -511,36 +541,25 @@ fn build_config_from_external(settings: &JsonValue) -> io::Result<TomlValue> {
Ok(TomlValue::Table(root))
}
fn json_object_to_toml_table(
fn json_object_to_env_toml_table(
object: &serde_json::Map<String, JsonValue>,
) -> io::Result<toml::map::Map<String, TomlValue>> {
) -> toml::map::Map<String, TomlValue> {
let mut table = toml::map::Map::new();
for (key, value) in object {
table.insert(key.clone(), json_to_toml_value(value)?);
if let Some(value) = json_env_value_to_string(value) {
table.insert(key.clone(), TomlValue::String(value));
}
}
Ok(table)
table
}
fn json_to_toml_value(value: &JsonValue) -> io::Result<TomlValue> {
fn json_env_value_to_string(value: &JsonValue) -> Option<String> {
match value {
JsonValue::Null => Ok(TomlValue::String("null".to_string())),
JsonValue::Bool(v) => Ok(TomlValue::Boolean(*v)),
JsonValue::Number(n) => {
if let Some(i) = n.as_i64() {
return Ok(TomlValue::Integer(i));
}
if let Some(f) = n.as_f64() {
return Ok(TomlValue::Float(f));
}
Err(invalid_data_error("unsupported JSON number"))
}
JsonValue::String(v) => Ok(TomlValue::String(v.clone())),
JsonValue::Array(values) => values
.iter()
.map(json_to_toml_value)
.collect::<io::Result<Vec<_>>>()
.map(TomlValue::Array),
JsonValue::Object(map) => json_object_to_toml_table(map).map(TomlValue::Table),
JsonValue::String(value) => Some(value.clone()),
JsonValue::Null => None,
JsonValue::Bool(value) => Some(value.to_string()),
JsonValue::Number(value) => Some(value.to_string()),
JsonValue::Array(_) | JsonValue::Object(_) => None,
}
}
@ -576,7 +595,7 @@ fn merge_missing_toml_values(existing: &mut TomlValue, incoming: &TomlValue) ->
fn write_toml_file(path: &Path, value: &TomlValue) -> io::Result<()> {
let serialized = toml::to_string_pretty(value)
.map_err(|err| invalid_data_error(format!("failed to serialize config.toml: {err}")))?;
fs::write(path, format!("{serialized}\n"))
fs::write(path, format!("{}\n", serialized.trim_end()))
}
fn is_empty_toml_table(value: &TomlValue) -> bool {
@ -638,7 +657,7 @@ mod tests {
ExternalAgentConfigMigrationItem {
item_type: ExternalAgentConfigMigrationItemType::Config,
description: format!(
"Migrate {} into {}.",
"Migrate {} into {}",
claude_home.join("settings.json").display(),
codex_home.join("config.toml").display()
),
@ -647,7 +666,7 @@ mod tests {
ExternalAgentConfigMigrationItem {
item_type: ExternalAgentConfigMigrationItemType::Skills,
description: format!(
"Copy skill folders from {} to {}.",
"Copy skill folders from {} to {}",
claude_home.join("skills").display(),
agents_skills.display()
),
@ -656,7 +675,7 @@ mod tests {
ExternalAgentConfigMigrationItem {
item_type: ExternalAgentConfigMigrationItemType::AgentsMd,
description: format!(
"Import {} to {}.",
"Import {} to {}",
claude_home.join("CLAUDE.md").display(),
codex_home.join("AGENTS.md").display()
),
@ -687,7 +706,7 @@ mod tests {
ExternalAgentConfigMigrationItem {
item_type: ExternalAgentConfigMigrationItemType::AgentsMd,
description: format!(
"Import {} to {}.",
"Import {} to {}",
repo_root.join("CLAUDE.md").display(),
repo_root.join("AGENTS.md").display(),
),
@ -696,7 +715,7 @@ mod tests {
ExternalAgentConfigMigrationItem {
item_type: ExternalAgentConfigMigrationItemType::AgentsMd,
description: format!(
"Import {} to {}.",
"Import {} to {}",
repo_root.join("CLAUDE.md").display(),
repo_root.join("AGENTS.md").display(),
),
@ -717,7 +736,7 @@ mod tests {
fs::create_dir_all(claude_home.join("skills").join("skill-a")).expect("create skills");
fs::write(
claude_home.join("settings.json"),
r#"{"model":"claude","permissions":{"ask":["git push"]},"env":{"FOO":"bar"},"sandbox":{"enabled":true,"network":{"allowLocalBinding":true}}}"#,
r#"{"model":"claude","permissions":{"ask":["git push"]},"env":{"FOO":"bar","CI":false,"MAX_RETRIES":3,"MY_TEAM":"codex","IGNORED":null,"LIST":["a","b"],"MAP":{"x":1}},"sandbox":{"enabled":true,"network":{"allowLocalBinding":true}}}"#,
)
.expect("write settings");
fs::write(
@ -752,23 +771,10 @@ mod tests {
"Codex guidance"
);
let parsed_config: TomlValue = toml::from_str(
&fs::read_to_string(codex_home.join("config.toml")).expect("read config"),
)
.expect("parse config");
let expected_config: TomlValue = toml::from_str(
r#"
sandbox_mode = "workspace-write"
[shell_environment_policy]
inherit = "core"
[shell_environment_policy.set]
FOO = "bar"
"#,
)
.expect("parse expected");
assert_eq!(parsed_config, expected_config);
assert_eq!(
fs::read_to_string(codex_home.join("config.toml")).expect("read config"),
"sandbox_mode = \"workspace-write\"\n\n[shell_environment_policy]\ninherit = \"core\"\n\n[shell_environment_policy.set]\nCI = \"false\"\nFOO = \"bar\"\nMAX_RETRIES = \"3\"\nMY_TEAM = \"codex\"\n"
);
assert_eq!(
fs::read_to_string(agents_skills.join("skill-a").join("SKILL.md"))
.expect("read copied skill"),
@ -917,4 +923,65 @@ mod tests {
"Codex guidance"
);
}
#[test]
fn detect_repo_prefers_non_empty_dot_claude_agents_source() {
let root = TempDir::new().expect("create tempdir");
let repo_root = root.path().join("repo");
fs::create_dir_all(repo_root.join(".git")).expect("create git");
fs::create_dir_all(repo_root.join(".claude")).expect("create dot claude");
fs::write(repo_root.join("CLAUDE.md"), " \n\t").expect("write empty root source");
fs::write(
repo_root.join(".claude").join("CLAUDE.md"),
"Claude code guidance",
)
.expect("write dot claude source");
let items = service_for_paths(root.path().join(".claude"), root.path().join(".codex"))
.detect(ExternalAgentConfigDetectOptions {
include_home: false,
cwds: Some(vec![repo_root.clone()]),
})
.expect("detect");
assert_eq!(
items,
vec![ExternalAgentConfigMigrationItem {
item_type: ExternalAgentConfigMigrationItemType::AgentsMd,
description: format!(
"Import {} to {}",
repo_root.join(".claude").join("CLAUDE.md").display(),
repo_root.join("AGENTS.md").display(),
),
cwd: Some(repo_root),
}]
);
}
#[test]
fn import_repo_uses_non_empty_dot_claude_agents_source() {
let root = TempDir::new().expect("create tempdir");
let repo_root = root.path().join("repo");
fs::create_dir_all(repo_root.join(".git")).expect("create git");
fs::create_dir_all(repo_root.join(".claude")).expect("create dot claude");
fs::write(repo_root.join("CLAUDE.md"), "").expect("write empty root source");
fs::write(
repo_root.join(".claude").join("CLAUDE.md"),
"Claude code guidance",
)
.expect("write dot claude source");
service_for_paths(root.path().join(".claude"), root.path().join(".codex"))
.import(vec![ExternalAgentConfigMigrationItem {
item_type: ExternalAgentConfigMigrationItemType::AgentsMd,
description: String::new(),
cwd: Some(repo_root.clone()),
}])
.expect("import");
assert_eq!(
fs::read_to_string(repo_root.join("AGENTS.md")).expect("read target"),
"Codex guidance"
);
}
}