diff --git a/codex-rs/app-server/src/config/external_agent_config.rs b/codex-rs/app-server/src/config/external_agent_config.rs index bf0b3576a..81eea3ec0 100644 --- a/codex-rs/app-server/src/config/external_agent_config.rs +++ b/codex-rs/app-server/src/config/external_agent_config.rs @@ -252,7 +252,7 @@ impl ExternalAgentConfigService { || self.external_agent_home.join("settings.json"), |repo_root| repo_root.join(EXTERNAL_AGENT_DIR).join("settings.json"), ); - let settings = read_external_settings(&source_settings)?; + let settings = effective_external_settings(&source_settings)?; let target_config = repo_root.map_or_else( || self.codex_home.join("config.toml"), |repo_root| repo_root.join(".codex").join("config.toml"), @@ -569,7 +569,7 @@ impl ExternalAgentConfigService { ) -> io::Result> { if repo_root.is_some() && source_settings.is_none() { let home_settings = self.external_agent_home.join("settings.json"); - match read_external_settings(&home_settings) { + match effective_external_settings(&home_settings) { Ok(settings) => Ok(settings), Err(err) => { tracing::warn!( @@ -636,7 +636,7 @@ impl ExternalAgentConfigService { |cwd| cwd.join(EXTERNAL_AGENT_DIR).join("settings.json"), ); let source_root = cwd.unwrap_or(self.external_agent_home.as_path()); - let import_sources = read_external_settings(&source_settings)? + let import_sources = effective_external_settings(&source_settings)? .map(|settings| collect_marketplace_import_sources(&settings, source_root)) .unwrap_or_default(); @@ -697,9 +697,11 @@ impl ExternalAgentConfigService { |cwd| cwd.join(EXTERNAL_AGENT_DIR).join("settings.json"), ); let source_root = cwd.unwrap_or(self.external_agent_home.as_path()); - let import_source = read_external_settings(&source_settings)?.and_then(|settings| { - collect_marketplace_import_sources(&settings, source_root).remove(&marketplace_name) - }); + let import_source = + effective_external_settings(&source_settings)?.and_then(|settings| { + collect_marketplace_import_sources(&settings, source_root) + .remove(&marketplace_name) + }); let Some(import_source) = import_source else { outcome.failed_marketplaces.push(marketplace_name); outcome.failed_plugin_ids.extend(plugin_ids); @@ -767,13 +769,9 @@ impl ExternalAgentConfigService { self.codex_home.join("config.toml"), ) }; - if !source_settings.is_file() { + let Some(settings) = effective_external_settings(&source_settings)? else { return Ok(()); - } - - let raw_settings = fs::read_to_string(&source_settings)?; - let settings: JsonValue = serde_json::from_str(&raw_settings) - .map_err(|err| invalid_data_error(err.to_string()))?; + }; let migrated = build_config_from_external(&settings)?; if is_empty_toml_table(&migrated) { return Ok(()); @@ -822,7 +820,7 @@ impl ExternalAgentConfigService { }; let settings = self.mcp_settings( repo_root.as_deref(), - read_external_settings(&source_settings)?, + effective_external_settings(&source_settings)?, )?; let migrated = build_mcp_config_from_external( self.source_root(repo_root.as_deref()).as_path(), @@ -999,6 +997,43 @@ fn read_external_settings(path: &Path) -> io::Result> { Ok(Some(settings)) } +fn effective_external_settings(project_settings: &Path) -> io::Result> { + let mut effective = read_external_settings(project_settings)?; + let Some(settings_dir) = project_settings.parent() else { + return Ok(effective); + }; + let local_settings = settings_dir.join("settings.local.json"); + let local_settings = match read_external_settings(&local_settings) { + Ok(Some(local_settings)) => local_settings, + Ok(None) => return Ok(effective), + Err(err) if err.kind() == io::ErrorKind::InvalidData => return Ok(effective), + Err(err) => return Err(err), + }; + if let Some(effective) = effective.as_mut() { + merge_json_settings(effective, &local_settings); + } else { + effective = Some(local_settings); + } + Ok(effective) +} + +fn merge_json_settings(existing: &mut JsonValue, incoming: &JsonValue) { + match (existing, incoming) { + (JsonValue::Object(existing), JsonValue::Object(incoming)) => { + for (key, incoming_value) in incoming { + match existing.get_mut(key) { + Some(existing_value) => merge_json_settings(existing_value, incoming_value), + None => { + existing.insert(key.clone(), incoming_value.clone()); + } + } + } + } + (existing, incoming) => { + *existing = incoming.clone(); + } + } +} fn extract_plugin_migration_details( settings: &JsonValue, source_root: &Path, diff --git a/codex-rs/app-server/src/config/external_agent_config_tests.rs b/codex-rs/app-server/src/config/external_agent_config_tests.rs index 208226135..8435f9baf 100644 --- a/codex-rs/app-server/src/config/external_agent_config_tests.rs +++ b/codex-rs/app-server/src/config/external_agent_config_tests.rs @@ -707,6 +707,68 @@ async fn import_home_migrates_supported_config_fields_skills_and_agents_md() { ); } +#[tokio::test] +async fn import_home_config_uses_local_settings_over_project_settings() { + let (_root, external_agent_home, codex_home) = fixture_paths(); + fs::create_dir_all(&external_agent_home).expect("create external agent home"); + fs::write( + external_agent_home.join("settings.json"), + r#"{"env":{"FOO":"project","PROJECT_ONLY":"yes"},"sandbox":{"enabled":false}}"#, + ) + .expect("write project settings"); + fs::write( + external_agent_home.join("settings.local.json"), + r#"{"env":{"FOO":"local","LOCAL_ONLY":true},"sandbox":{"enabled":true}}"#, + ) + .expect("write local settings"); + + service_for_paths(external_agent_home, codex_home.clone()) + .import(vec![ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::Config, + description: String::new(), + cwd: None, + details: None, + }]) + .await + .expect("import"); + + 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]\nFOO = \"local\"\nLOCAL_ONLY = \"true\"\nPROJECT_ONLY = \"yes\"\n" + ); +} + +#[tokio::test] +async fn import_home_config_ignores_invalid_local_settings() { + let (_root, external_agent_home, codex_home) = fixture_paths(); + fs::create_dir_all(&external_agent_home).expect("create external agent home"); + fs::write( + external_agent_home.join("settings.json"), + r#"{"env":{"FOO":"project"},"sandbox":{"enabled":false}}"#, + ) + .expect("write project settings"); + fs::write( + external_agent_home.join("settings.local.json"), + "{invalid json", + ) + .expect("write local settings"); + + service_for_paths(external_agent_home, codex_home.clone()) + .import(vec![ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::Config, + description: String::new(), + cwd: None, + details: None, + }]) + .await + .expect("import"); + + assert_eq!( + fs::read_to_string(codex_home.join("config.toml")).expect("read config"), + "[shell_environment_policy]\ninherit = \"core\"\n\n[shell_environment_policy.set]\nFOO = \"project\"\n" + ); +} + #[tokio::test] async fn import_home_skips_empty_config_migration() { let (_root, external_agent_home, codex_home) = fixture_paths(); @@ -1144,6 +1206,67 @@ command = "allowed-server" assert_eq!(config, expected); } +#[tokio::test] +async fn import_repo_mcp_uses_local_settings_toggles_over_project_settings() { + let root = TempDir::new().expect("create tempdir"); + let repo_root = root.path().join("repo"); + let external_agent_home = root.path().join(EXTERNAL_AGENT_DIR); + fs::create_dir_all(repo_root.join(".git")).expect("create git dir"); + fs::create_dir_all(repo_root.join(EXTERNAL_AGENT_DIR)).expect("create external agent dir"); + fs::write( + repo_root.join(".mcp.json"), + r#"{ + "mcpServers": { + "project-disabled": {"command": "project-disabled-server"}, + "local-disabled": {"command": "local-disabled-server"}, + "local-enabled": {"command": "local-enabled-server"} + } + }"#, + ) + .expect("write mcp"); + fs::write( + repo_root.join(EXTERNAL_AGENT_DIR).join("settings.json"), + r#"{ + "enabledMcpjsonServers": ["project-disabled", "local-disabled"], + "disabledMcpjsonServers": ["project-disabled"] + }"#, + ) + .expect("write project settings"); + fs::write( + repo_root + .join(EXTERNAL_AGENT_DIR) + .join("settings.local.json"), + r#"{ + "enabledMcpjsonServers": ["local-enabled", "local-disabled"], + "disabledMcpjsonServers": ["local-disabled"] + }"#, + ) + .expect("write local settings"); + + service_for_paths(external_agent_home, root.path().join(".codex")) + .import(vec![ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::McpServerConfig, + description: String::new(), + cwd: Some(repo_root.clone()), + details: None, + }]) + .await + .expect("import"); + + let config: TomlValue = toml::from_str( + &fs::read_to_string(repo_root.join(".codex").join("config.toml")).expect("read config"), + ) + .expect("parse config"); + let expected: TomlValue = toml::from_str( + r#" +[mcp_servers.local-enabled] +command = "local-enabled-server" +"#, + ) + .expect("parse expected config"); + assert_eq!(config, expected); +} + #[tokio::test] async fn import_repo_mcp_ignores_invalid_home_settings_when_repo_settings_missing() { let root = TempDir::new().expect("create tempdir"); @@ -1286,6 +1409,64 @@ async fn detect_home_lists_enabled_plugins_from_settings() { ); } +#[tokio::test] +async fn detect_home_plugins_uses_local_settings_over_project_settings() { + let (_root, external_agent_home, codex_home) = fixture_paths(); + fs::create_dir_all(&external_agent_home).expect("create external agent home"); + fs::write( + external_agent_home.join("settings.json"), + r#"{ + "enabledPlugins": { + "formatter@acme-tools": true, + "legacy@acme-tools": true + }, + "extraKnownMarketplaces": { + "acme-tools": { + "source": "acme-corp/external-agent-plugins" + } + } + }"#, + ) + .expect("write project settings"); + fs::write( + external_agent_home.join("settings.local.json"), + r#"{ + "enabledPlugins": { + "formatter@acme-tools": false, + "deployer@acme-tools": true + } + }"#, + ) + .expect("write local settings"); + + let items = service_for_paths(external_agent_home.clone(), codex_home) + .detect(ExternalAgentConfigDetectOptions { + include_home: true, + cwds: None, + }) + .await + .expect("detect"); + + assert_eq!( + items, + vec![ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::Plugins, + description: format!( + "Migrate enabled plugins from {}", + external_agent_home.join("settings.json").display() + ), + cwd: None, + details: Some(MigrationDetails { + plugins: vec![PluginsMigration { + marketplace_name: "acme-tools".to_string(), + plugin_names: vec!["deployer".to_string(), "legacy".to_string()], + }], + ..Default::default() + }), + }] + ); +} + #[tokio::test] async fn detect_repo_skips_plugins_that_are_already_configured_in_codex() { let root = TempDir::new().expect("create tempdir"); diff --git a/codex-rs/external-agent-migration/src/lib.rs b/codex-rs/external-agent-migration/src/lib.rs index e01c92389..d7b880179 100644 --- a/codex-rs/external-agent-migration/src/lib.rs +++ b/codex-rs/external-agent-migration/src/lib.rs @@ -210,7 +210,9 @@ pub fn import_commands(source_commands: &Path, target_skills: &Path) -> io::Resu } fs::create_dir_all(&target_dir)?; let source_name = command_source_name(source_commands, &source_file); - let description = command_skill_description(&document, &source_name); + let Some(description) = command_skill_description(&document, &source_name) else { + continue; + }; fs::write( target_dir.join("SKILL.md"), render_command_skill(&document.body, &name, &description, &source_name), @@ -661,86 +663,197 @@ fn rewrite_hook_command(command: &str, target_config_dir: Option<&Path>) -> Stri let Some(target_config_dir) = target_config_dir else { return command.to_string(); }; + if looks_like_windows_hook_command(command) { + return command.to_string(); + } let target_hooks_dir = target_config_dir.join(EXTERNAL_AGENT_MIGRATED_HOOKS_SUBDIR); - let hook_dir = shell_quote_path_prefix(&target_hooks_dir); - let project_dir_env_var = external_agent_project_dir_env_var(); let source_hooks_path = format!( "{}/{EXTERNAL_AGENT_HOOKS_SUBDIR}/", external_agent_config_dir() ); - let source_hook_prefixes = [ - format!("${{{project_dir_env_var}}}/{source_hooks_path}"), - format!("${project_dir_env_var}/{source_hooks_path}"), - format!("./{source_hooks_path}"), - source_hooks_path.clone(), - ]; - let command = replace_quoted_hook_path_prefixes( - command, - '\'', - &source_hook_prefixes, - &hook_dir, - &target_hooks_dir, - ); - let command = replace_quoted_hook_path_prefixes( - &command, - '"', - &source_hook_prefixes, - &hook_dir, - &target_hooks_dir, - ); - command - .replace( - &format!("\"${project_dir_env_var}\"/{source_hooks_path}"), - &hook_dir, - ) - .replace( - &format!("\"${{{project_dir_env_var}}}\"/{source_hooks_path}"), - &hook_dir, - ) - .replace( - &format!("${{{project_dir_env_var}}}/{source_hooks_path}"), - &hook_dir, - ) - .replace( - &format!("${project_dir_env_var}/{source_hooks_path}"), - &hook_dir, - ) - .replace(&format!("./{source_hooks_path}"), &hook_dir) - .replace(&source_hooks_path, &hook_dir) + let command = replace_quoted_hook_paths(command, '\'', &source_hooks_path, &target_hooks_dir); + let command = replace_quoted_hook_paths(&command, '"', &source_hooks_path, &target_hooks_dir); + replace_unquoted_hook_paths(&command, &source_hooks_path, &target_hooks_dir) } -fn replace_quoted_hook_path_prefixes( +fn replace_quoted_hook_paths( command: &str, quote: char, - source_hook_prefixes: &[String], - hook_dir: &str, + source_hooks_path: &str, target_hooks_dir: &Path, ) -> String { let mut rewritten = command.to_string(); - for source_hook_prefix in source_hook_prefixes { - let quoted_prefix = format!("{quote}{source_hook_prefix}"); - let mut search_start = 0usize; - while let Some(relative_start) = rewritten[search_start..].find("ed_prefix) { - let start = search_start + relative_start; - let path_start = start + quoted_prefix.len(); - if let Some(relative_end) = rewritten[path_start..].find(quote) { - let end = path_start + relative_end; - let suffix = rewritten[path_start..end].to_string(); - let replacement = - shell_single_quote(target_hooks_dir.join(suffix).to_string_lossy().as_ref()); - rewritten.replace_range(start..end + quote.len_utf8(), &replacement); - search_start = start + replacement.len(); - } else { - rewritten.replace_range(start..path_start, hook_dir); - search_start = start + hook_dir.len(); - } + let mut search_start = 0usize; + while let Some(relative_start) = rewritten[search_start..].find(quote) { + let start = search_start + relative_start; + let content_start = start + quote.len_utf8(); + let Some(relative_end) = rewritten[content_start..].find(quote) else { + break; + }; + let end = content_start + relative_end; + let content = &rewritten[content_start..end]; + if let Some(source_hooks_start) = content.find(source_hooks_path) { + let suffix_start = source_hooks_start + source_hooks_path.len(); + let suffix = &content[suffix_start..]; + let Some(replacement) = + target_hook_path_replacement(target_hooks_dir, content, source_hooks_start, suffix) + else { + search_start = end + quote.len_utf8(); + continue; + }; + rewritten.replace_range(start..end + quote.len_utf8(), &replacement); + search_start = start + replacement.len(); + } else { + search_start = end + quote.len_utf8(); } } rewritten } -fn shell_quote_path_prefix(path: &Path) -> String { - format!("{}/", shell_single_quote(path.to_string_lossy().as_ref())) +fn replace_unquoted_hook_paths( + command: &str, + source_hooks_path: &str, + target_hooks_dir: &Path, +) -> String { + let mut rewritten = command.to_string(); + let mut search_start = 0usize; + while let Some(source_hooks_start) = + find_unquoted_source_hook_path(&rewritten, source_hooks_path, search_start) + { + let path_start = shell_path_start(&rewritten, source_hooks_start); + let path_end = shell_path_end(&rewritten, source_hooks_start + source_hooks_path.len()); + if is_assignment_value_start(&rewritten, path_start) { + search_start = source_hooks_start + source_hooks_path.len(); + continue; + } + let path = rewritten[path_start..path_end].to_string(); + let suffix = rewritten[source_hooks_start + source_hooks_path.len()..path_end].to_string(); + if let Some(replacement) = target_hook_path_replacement( + target_hooks_dir, + &path, + source_hooks_start - path_start, + &suffix, + ) { + rewritten.replace_range(path_start..path_end, &replacement); + search_start = path_start + replacement.len(); + } else { + search_start = source_hooks_start + source_hooks_path.len(); + } + } + rewritten +} + +fn find_unquoted_source_hook_path( + command: &str, + source_hooks_path: &str, + start: usize, +) -> Option { + let mut in_single_quote = false; + let mut in_double_quote = false; + let mut escaped = false; + for (offset, ch) in command[start..].char_indices() { + let index = start + offset; + if escaped { + escaped = false; + continue; + } + if !in_single_quote && ch == '\\' { + escaped = true; + continue; + } + match ch { + '\'' if !in_double_quote => { + in_single_quote = !in_single_quote; + } + '"' if !in_single_quote => { + in_double_quote = !in_double_quote; + } + _ if !in_single_quote + && !in_double_quote + && command[index..].starts_with(source_hooks_path) => + { + return Some(index); + } + _ => {} + } + } + None +} + +fn is_pure_shell_path_content(content: &str, source_hooks_start: usize) -> bool { + let prefix = &content[..source_hooks_start]; + (prefix.is_empty() || prefix == "./" || prefix.ends_with('/')) + && !prefix.chars().any(is_shell_path_boundary) +} + +fn shell_path_start(command: &str, end: usize) -> usize { + command[..end] + .char_indices() + .filter_map(|(index, ch)| is_shell_path_boundary(ch).then_some(index + ch.len_utf8())) + .next_back() + .unwrap_or(0) +} + +fn shell_path_end(command: &str, start: usize) -> usize { + let mut escaped = false; + for (offset, ch) in command[start..].char_indices() { + if escaped { + escaped = false; + continue; + } + if ch == '\\' { + escaped = true; + continue; + } + if is_shell_path_boundary(ch) { + return start + offset; + } + } + command.len() +} + +fn is_shell_path_boundary(ch: char) -> bool { + ch.is_whitespace() || matches!(ch, '=' | ';' | '|' | '&' | '<' | '>' | '(' | ')') +} + +fn is_assignment_value_start(command: &str, path_start: usize) -> bool { + command[..path_start] + .chars() + .next_back() + .is_some_and(|ch| ch == '=') +} + +fn target_hook_path_replacement( + target_hooks_dir: &Path, + path: &str, + source_hooks_start: usize, + suffix: &str, +) -> Option { + if !is_pure_shell_path_content(path, source_hooks_start) || !is_static_hook_path_suffix(suffix) + { + return None; + } + Some(shell_single_quote( + target_hooks_dir.join(suffix).to_string_lossy().as_ref(), + )) +} + +fn is_static_hook_path_suffix(suffix: &str) -> bool { + !suffix.is_empty() + && !suffix + .chars() + .any(|ch| matches!(ch, '\\' | '$' | '`' | '*' | '?' | '[' | '{' | '}')) +} + +fn looks_like_windows_hook_command(command: &str) -> bool { + let source_hooks_backslash_path = format!( + r"{}\{EXTERNAL_AGENT_HOOKS_SUBDIR}\", + external_agent_config_dir() + ); + let project_dir_env_var = external_agent_project_dir_env_var(); + command.contains(&source_hooks_backslash_path) + || command.contains(&format!("%{project_dir_env_var}%")) + || command.contains(&format!("$env:{project_dir_env_var}")) } fn shell_single_quote(value: &str) -> String { @@ -1013,12 +1126,15 @@ fn command_skill_name_if_supported( source_file: &Path, document: &ParsedDocument, ) -> Option { + if source_file.file_stem().and_then(|stem| stem.to_str()) == Some("README") { + return None; + } + let source_name = command_source_name(source_commands, source_file); + let description = command_skill_description(document, &source_name)?; let name = command_skill_name(source_commands, source_file); if name.chars().count() > MAX_SKILL_NAME_LEN { return None; } - let source_name = command_source_name(source_commands, source_file); - let description = command_skill_description(document, &source_name); if description.chars().count() > MAX_SKILL_DESCRIPTION_LEN { return None; } @@ -1028,14 +1144,13 @@ fn command_skill_name_if_supported( Some(name) } -fn command_skill_description(document: &ParsedDocument, source_name: &str) -> String { +fn command_skill_description(document: &ParsedDocument, _source_name: &str) -> Option { document .frontmatter .get("description") .and_then(FrontmatterValue::as_scalar) .filter(|value| !value.trim().is_empty()) .map(ToOwned::to_owned) - .unwrap_or_else(|| format!("Run the migrated source command `{source_name}`.")) } fn command_source_name(source_commands: &Path, source_file: &Path) -> String { @@ -1295,10 +1410,7 @@ mod tests { } fn migrated_hook_command(script_name: &str) -> String { - let hook_dir = shell_quote_path_prefix( - &Path::new("/repo/.codex").join(EXTERNAL_AGENT_MIGRATED_HOOKS_SUBDIR), - ); - format!("python3 {hook_dir}{script_name}") + migrated_quoted_hook_command(script_name) } fn migrated_quoted_hook_command(script_name: &str) -> String { @@ -1578,6 +1690,15 @@ command = "enabled-server" assert!(command_skill_name_if_supported(&root, &file, &document).is_none()); } + #[test] + fn commands_without_description_are_skipped() { + let root = source_path("commands"); + let file = source_path("commands/README.md"); + let document = parse_document_content("# Notes\n\nThis documents commands.\n"); + + assert!(command_skill_name_if_supported(&root, &file, &document).is_none()); + } + #[test] fn command_slug_collisions_are_skipped() { let root = tempfile::TempDir::new().expect("tempdir"); @@ -1817,6 +1938,10 @@ Review carefully.""" #[test] fn hook_command_paths_rewrite_to_target_hook_dir() { let project_dir_env_var = external_agent_project_dir_env_var(); + let plugin_root_env_var = format!( + "{}_PLUGIN_ROOT", + SOURCE_EXTERNAL_AGENT_NAME.to_ascii_uppercase() + ); let source_hooks_path = format!( "{}/{EXTERNAL_AGENT_HOOKS_SUBDIR}", external_agent_config_dir() @@ -1828,6 +1953,19 @@ Review carefully.""" ), migrated_hook_command("check.py") ); + assert_eq!( + rewrite_hook_command( + &format!("\"${project_dir_env_var}\"/{source_hooks_path}/check-style.sh"), + Some(Path::new("/repo/.codex")), + ), + shell_single_quote( + Path::new("/repo/.codex") + .join(EXTERNAL_AGENT_MIGRATED_HOOKS_SUBDIR) + .join("check-style.sh") + .to_string_lossy() + .as_ref() + ) + ); assert_eq!( rewrite_hook_command( &source_hook_command("check.py"), @@ -1856,6 +1994,67 @@ Review carefully.""" ), migrated_quoted_hook_command("check.py") ); + assert_eq!( + rewrite_hook_command( + &format!("bash -lc \"python3 {source_hooks_path}/check.py\""), + Some(Path::new("/repo/.codex")), + ), + format!("bash -lc \"python3 {source_hooks_path}/check.py\"") + ); + assert_eq!( + rewrite_hook_command( + &format!( + "HOOK=${{{project_dir_env_var}}}/{source_hooks_path}/check.py python3 \"$HOOK\"" + ), + Some(Path::new("/repo/.codex")), + ), + format!( + "HOOK=${{{project_dir_env_var}}}/{source_hooks_path}/check.py python3 \"$HOOK\"" + ) + ); + assert_eq!( + rewrite_hook_command( + &format!("python3 {source_hooks_path}/${{SCRIPT}}.py"), + Some(Path::new("/repo/.codex")), + ), + format!("python3 {source_hooks_path}/${{SCRIPT}}.py") + ); + assert_eq!( + rewrite_hook_command( + &format!("python3 {source_hooks_path}/{{lint,fmt}}.sh"), + Some(Path::new("/repo/.codex")), + ), + format!("python3 {source_hooks_path}/{{lint,fmt}}.sh") + ); + assert_eq!( + rewrite_hook_command( + &format!("python3 {source_hooks_path}/my\\ script.py"), + Some(Path::new("/repo/.codex")), + ), + format!("python3 {source_hooks_path}/my\\ script.py") + ); + assert_eq!( + rewrite_hook_command( + &format!("python3 .{SOURCE_EXTERNAL_AGENT_NAME}\\hooks\\check.py"), + Some(Path::new("/repo/.codex")), + ), + format!("python3 .{}\\hooks\\check.py", SOURCE_EXTERNAL_AGENT_NAME) + ); + assert_eq!( + rewrite_hook_command( + &format!( + "python3 \"%{}%\\{}\\hooks\\check.py\"", + project_dir_env_var, + external_agent_config_dir() + ), + Some(Path::new("/repo/.codex")), + ), + format!( + "python3 \"%{}%\\{}\\hooks\\check.py\"", + project_dir_env_var, + external_agent_config_dir() + ) + ); assert_eq!( rewrite_hook_command( &format!("python3 '${{{project_dir_env_var}}}/{source_hooks_path}/my script.py'"), @@ -1863,6 +2062,27 @@ Review carefully.""" ), migrated_quoted_hook_command("my script.py") ); + assert_eq!( + rewrite_hook_command( + &format!("/repo/{source_hooks_path}/check.py 2>/dev/null || true"), + Some(Path::new("/repo/.codex")), + ), + format!( + "{} 2>/dev/null || true", + shell_single_quote( + Path::new("/repo/.codex") + .join(EXTERNAL_AGENT_MIGRATED_HOOKS_SUBDIR) + .join("check.py") + .to_string_lossy() + .as_ref() + ) + ) + ); + let plugin_script_command = format!("${{{plugin_root_env_var}}}/scripts/format.sh"); + assert_eq!( + rewrite_hook_command(&plugin_script_command, Some(Path::new("/repo/.codex")),), + plugin_script_command + ); } #[test]