diff --git a/codex-rs/core/src/external_agent_config.rs b/codex-rs/core/src/external_agent_config.rs index 1d466e6f4..d0fde766f 100644 --- a/codex-rs/core/src/external_agent_config.rs +++ b/codex-rs/core/src/external_agent_config.rs @@ -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 { Ok(fs::read_to_string(path)?.trim().is_empty()) } +fn is_non_empty_text_file(path: &Path) -> io::Result { + 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> { + 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 { 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 { Ok(TomlValue::Table(root)) } -fn json_object_to_toml_table( +fn json_object_to_env_toml_table( object: &serde_json::Map, -) -> io::Result> { +) -> toml::map::Map { 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 { +fn json_env_value_to_string(value: &JsonValue) -> Option { 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::>>() - .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" + ); + } }