diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 8444142e0ca2..5f056d5e9ed2 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2821,6 +2821,7 @@ dependencies = [ "semver", "serde", "serde_json", + "serde_yaml", "tar", "tempfile", "thiserror 2.0.18", diff --git a/codex-rs/app-server/src/external_agent_migration/service/source_cla.rs b/codex-rs/app-server/src/external_agent_migration/service/source_cla.rs index 6f7a05886b43..c38b39fb254a 100644 --- a/codex-rs/app-server/src/external_agent_migration/service/source_cla.rs +++ b/codex-rs/app-server/src/external_agent_migration/service/source_cla.rs @@ -1,14 +1,15 @@ +use codex_core_plugins::CommandDescriptionMode; +use codex_core_plugins::CommandMigrationProfile; +use codex_core_plugins::CommandRewriteProfile; +use codex_core_plugins::count_missing_commands_with_profile; +use codex_core_plugins::import_commands_with_profile; use codex_core_plugins::marketplace_add::is_local_marketplace_source; -use codex_external_agent_migration::CommandDescriptionMode; -use codex_external_agent_migration::CommandMigrationProfile; +use codex_core_plugins::missing_command_names_with_profile; use codex_external_agent_migration::RewriteProfile; use codex_external_agent_migration::build_mcp_config_from_external; -use codex_external_agent_migration::count_missing_commands_with_profile; use codex_external_agent_migration::hook_migration_event_names_cla; -use codex_external_agent_migration::import_commands_with_profile; use codex_external_agent_migration::import_hooks_cla; use codex_external_agent_migration::import_subagents_with_rewrite_profile; -use codex_external_agent_migration::missing_command_names_with_profile; use codex_plugin::PluginId; use serde_json::Value as JsonValue; use std::collections::BTreeMap; @@ -42,8 +43,14 @@ pub(super) const REWRITE_PROFILE: RewriteProfile = RewriteProfile::new( "claude", ], ); -const COMMAND_MIGRATION_PROFILE: CommandMigrationProfile = - CommandMigrationProfile::new(REWRITE_PROFILE, CommandDescriptionMode::RequireFrontmatter); +const COMMAND_MIGRATION_PROFILE: CommandMigrationProfile = CommandMigrationProfile::new( + CommandRewriteProfile::new( + REWRITE_PROFILE.doc_file_name(), + REWRITE_PROFILE.term_variants(), + ) + .with_case_sensitive_term_variants(REWRITE_PROFILE.case_sensitive_term_variants()), + CommandDescriptionMode::RequireFrontmatter, +); pub(super) fn effective_settings(project_settings: &Path) -> io::Result> { let mut effective = super::read_external_settings(project_settings)?; diff --git a/codex-rs/core-plugins/Cargo.toml b/codex-rs/core-plugins/Cargo.toml index e85d16a126bd..0c98eb2b3e1d 100644 --- a/codex-rs/core-plugins/Cargo.toml +++ b/codex-rs/core-plugins/Cargo.toml @@ -41,6 +41,7 @@ regex = { workspace = true } semver = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } +serde_yaml = { workspace = true } tar = { workspace = true } tempfile = { workspace = true } thiserror = { workspace = true } diff --git a/codex-rs/core-plugins/src/command_migration.rs b/codex-rs/core-plugins/src/command_migration.rs new file mode 100644 index 000000000000..b49da8798216 --- /dev/null +++ b/codex-rs/core-plugins/src/command_migration.rs @@ -0,0 +1,439 @@ +mod plugin; +mod render; + +use render::rewrite_terms; +use render::slugify_name; +use render::yaml_string; +use serde_yaml::Value as YamlValue; +use std::collections::BTreeMap; +use std::fs; +use std::io; +use std::path::Path; +use std::path::PathBuf; + +const COMMAND_SKILL_PREFIX: &str = "source-command"; +const MAX_SKILL_NAME_LEN: usize = 64; + +pub(crate) use plugin::migrate_plugin_commands; +pub(crate) use plugin::migrated_command_skills_root; + +/// Describes source-specific terms that should be rewritten in migrated command skills. +#[derive(Clone, Copy)] +pub struct RewriteProfile { + doc_file_name: &'static str, + term_variants: &'static [&'static str], + case_sensitive_term_variants: &'static [&'static str], +} + +impl RewriteProfile { + pub const fn new(doc_file_name: &'static str, term_variants: &'static [&'static str]) -> Self { + Self { + doc_file_name, + term_variants, + case_sensitive_term_variants: &[], + } + } + + pub const fn with_case_sensitive_term_variants( + mut self, + term_variants: &'static [&'static str], + ) -> Self { + self.case_sensitive_term_variants = term_variants; + self + } +} + +/// Controls how migrated commands obtain the description required by a Codex skill. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum CommandDescriptionMode { + /// Skip source commands that do not declare a non-empty frontmatter description. + RequireFrontmatter, + /// Derive a stable description from the source command name when frontmatter is absent. + UseSourceNameFallback, +} + +/// Describes source-specific command migration behavior. +#[derive(Clone, Copy)] +pub struct CommandMigrationProfile { + rewrite_profile: RewriteProfile, + description_mode: CommandDescriptionMode, +} + +impl CommandMigrationProfile { + pub const fn new( + rewrite_profile: RewriteProfile, + description_mode: CommandDescriptionMode, + ) -> Self { + Self { + rewrite_profile, + description_mode, + } + } +} + +#[derive(Debug)] +struct ParsedCommand { + description: Option, + body: String, +} + +#[derive(Debug, PartialEq, Eq)] +struct CommandSource { + source_file: PathBuf, + name: String, + source_name: String, +} + +#[derive(Clone, Copy)] +enum CommandSkillSizeLimit { + Unbounded, + MaxBytes(usize), +} + +pub fn count_missing_commands_with_profile( + source_commands: &Path, + target_skills: &Path, + profile: CommandMigrationProfile, +) -> io::Result { + Ok(missing_command_names_with_profile(source_commands, target_skills, profile)?.len()) +} + +pub fn missing_command_names_with_profile( + source_commands: &Path, + target_skills: &Path, + profile: CommandMigrationProfile, +) -> io::Result> { + Ok( + unique_supported_command_sources(source_commands, profile.description_mode)? + .into_iter() + .filter(|source| !target_skills.join(&source.name).exists()) + .map(|source| source.name) + .collect(), + ) +} + +pub fn import_commands_with_profile( + source_commands: &Path, + target_skills: &Path, + profile: CommandMigrationProfile, +) -> io::Result> { + if !source_commands.is_dir() { + return Ok(Vec::new()); + } + fs::create_dir_all(target_skills)?; + import_command_sources( + unique_supported_command_sources(source_commands, profile.description_mode)?, + target_skills, + profile, + CommandSkillSizeLimit::Unbounded, + ) +} + +fn import_command_sources( + command_sources: Vec, + target_skills: &Path, + profile: CommandMigrationProfile, + size_limit: CommandSkillSizeLimit, +) -> io::Result> { + if command_sources.is_empty() { + return Ok(Vec::new()); + } + + let mut imported = Vec::new(); + for CommandSource { + source_file, + name, + source_name, + } in command_sources + { + let document = parse_command(&source_file)?; + let target_dir = target_skills.join(&name); + if target_dir.exists() { + continue; + } + let Some(description) = + command_skill_description(&document, &source_name, profile.description_mode) + else { + continue; + }; + let rendered = render_command_skill( + &document.body, + &name, + &description, + &source_name, + profile.rewrite_profile, + ); + if let CommandSkillSizeLimit::MaxBytes(max_bytes) = size_limit + && rendered.len() > max_bytes + { + continue; + } + fs::create_dir_all(&target_dir)?; + fs::write(target_dir.join("SKILL.md"), rendered)?; + imported.push(name); + } + + Ok(imported) +} + +fn unique_supported_command_sources( + source_commands: &Path, + description_mode: CommandDescriptionMode, +) -> io::Result> { + Ok(unique_command_sources(supported_command_sources( + source_commands, + description_mode, + )?)) +} + +fn supported_command_sources( + source_commands: &Path, + description_mode: CommandDescriptionMode, +) -> io::Result> { + let mut sources = Vec::new(); + for source_file in command_source_files(source_commands)? { + let document = parse_command(&source_file)?; + let source_name = command_source_name(source_commands, &source_file); + let Some(name) = command_skill_name_if_supported( + &source_name, + &source_file, + &document, + description_mode, + ) else { + continue; + }; + sources.push(CommandSource { + source_file, + name, + source_name, + }); + } + Ok(sources) +} + +fn unique_command_sources(command_sources: Vec) -> Vec { + let mut by_name = BTreeMap::>::new(); + for source in command_sources { + by_name + .entry(source.name) + .or_default() + .insert(source.source_file, source.source_name); + } + + by_name + .into_iter() + .filter_map(|(name, source_files)| { + let mut source_files = source_files.into_iter(); + let (source_file, source_name) = source_files.next()?; + if source_files.next().is_some() { + return None; + } + Some(CommandSource { + source_file, + name, + source_name, + }) + }) + .collect() +} + +fn command_source_files(source_commands: &Path) -> io::Result> { + if source_commands.is_file() { + return Ok( + if source_commands.extension().and_then(|ext| ext.to_str()) == Some("md") { + vec![source_commands.to_path_buf()] + } else { + Vec::new() + }, + ); + } + + let mut files = Vec::new(); + collect_markdown_files(source_commands, &mut files)?; + files.sort(); + Ok(files) +} + +fn collect_markdown_files(dir: &Path, files: &mut Vec) -> io::Result<()> { + if !dir.is_dir() { + return Ok(()); + } + + for entry in fs::read_dir(dir)? { + let entry = entry?; + let path = entry.path(); + let file_type = entry.file_type()?; + if file_type.is_dir() { + collect_markdown_files(&path, files)?; + } else if file_type.is_file() && path.extension().and_then(|ext| ext.to_str()) == Some("md") + { + files.push(path); + } + } + Ok(()) +} + +fn parse_command(source_file: &Path) -> io::Result { + Ok(parse_command_content(&fs::read_to_string(source_file)?)) +} + +fn parse_command_content(content: &str) -> ParsedCommand { + let Some(rest) = content + .strip_prefix("---\n") + .or_else(|| content.strip_prefix("---\r\n")) + else { + return ParsedCommand { + description: None, + body: content.to_string(), + }; + }; + let Some((end, body_start)) = frontmatter_end(rest) else { + return ParsedCommand { + description: None, + body: content.to_string(), + }; + }; + + ParsedCommand { + description: parse_command_description(&rest[..end]), + body: rest[body_start..].to_string(), + } +} + +fn frontmatter_end(rest: &str) -> Option<(usize, usize)> { + [ + "\r\n---\r\n", + "\r\n---\n", + "\n---\r\n", + "\n---\n", + "\r\n---", + "\n---", + ] + .into_iter() + .filter_map(|delimiter| rest.find(delimiter).map(|end| (end, end + delimiter.len()))) + .min_by_key(|(end, _body_start)| *end) +} + +fn parse_command_description(raw_frontmatter: &str) -> Option { + let parsed: YamlValue = serde_yaml::from_str(raw_frontmatter).ok()?; + let mapping = parsed.as_mapping()?; + mapping.iter().find_map(|(key, value)| { + if key.as_str()?.trim() == "description" { + yaml_scalar(value) + } else { + None + } + }) +} + +fn yaml_scalar(value: &YamlValue) -> Option { + match value { + YamlValue::String(value) => Some(value.trim().to_string()), + YamlValue::Bool(value) => Some(value.to_string()), + YamlValue::Number(value) => Some(value.to_string()), + YamlValue::Null | YamlValue::Sequence(_) | YamlValue::Mapping(_) | YamlValue::Tagged(_) => { + None + } + } +} + +fn command_skill_name(source_name: &str) -> String { + slugify_name(&format!("{COMMAND_SKILL_PREFIX}-{source_name}")) +} + +fn command_skill_name_if_supported( + source_name: &str, + source_file: &Path, + document: &ParsedCommand, + description_mode: CommandDescriptionMode, +) -> Option { + if source_file.file_stem().and_then(|stem| stem.to_str()) == Some("README") { + return None; + } + command_skill_description(document, source_name, description_mode)?; + let name = command_skill_name(source_name); + if name.chars().count() > MAX_SKILL_NAME_LEN + || has_unsupported_command_template_features(&document.body) + { + return None; + } + Some(name) +} + +fn command_skill_description( + document: &ParsedCommand, + source_name: &str, + description_mode: CommandDescriptionMode, +) -> Option { + document + .description + .as_deref() + .filter(|value| !value.trim().is_empty()) + .map(ToOwned::to_owned) + .or_else(|| match description_mode { + CommandDescriptionMode::RequireFrontmatter => None, + CommandDescriptionMode::UseSourceNameFallback => { + Some(format!("Migrated source command `{source_name}`")) + } + }) +} + +fn command_source_name(source_commands: &Path, source_file: &Path) -> String { + if source_commands.is_file() { + return source_file + .file_stem() + .and_then(|stem| stem.to_str()) + .unwrap_or_default() + .to_string(); + } + source_file + .strip_prefix(source_commands) + .unwrap_or(source_file) + .with_extension("") + .components() + .filter_map(|component| component.as_os_str().to_str()) + .collect::>() + .join("-") +} + +fn render_command_skill( + body: &str, + name: &str, + description: &str, + source_name: &str, + rewrite_profile: RewriteProfile, +) -> String { + let body = rewrite_terms(body.trim(), rewrite_profile); + let template_body = if body.is_empty() { + "No command template body was found.".to_string() + } else { + body + }; + format!( + "---\nname: {}\ndescription: {}\n---\n\n# {name}\n\nUse this skill when the user asks to run the migrated source command `{source_name}`.\n\n## Command Template\n\n{template_body}\n", + yaml_string(name), + yaml_string(&rewrite_terms(description, rewrite_profile)), + ) +} + +fn has_unsupported_command_template_features(template: &str) -> bool { + template.contains("$ARGUMENTS") + || contains_numbered_argument_placeholder(template) + || (template.contains("{{") && template.contains("}}")) + || template.contains("!`") + || template.contains("! `") + || template + .split_whitespace() + .any(|token| token.strip_prefix('@').is_some_and(|rest| !rest.is_empty())) +} + +fn contains_numbered_argument_placeholder(template: &str) -> bool { + let bytes = template.as_bytes(); + bytes + .windows(2) + .any(|window| window[0] == b'$' && window[1].is_ascii_digit()) +} + +#[cfg(test)] +#[path = "command_migration_tests.rs"] +mod tests; diff --git a/codex-rs/core-plugins/src/command_migration/plugin.rs b/codex-rs/core-plugins/src/command_migration/plugin.rs new file mode 100644 index 000000000000..0725326ce7e1 --- /dev/null +++ b/codex-rs/core-plugins/src/command_migration/plugin.rs @@ -0,0 +1,61 @@ +use super::CommandDescriptionMode; +use super::CommandMigrationProfile; +use super::CommandSkillSizeLimit; +use super::CommandSource; +use super::RewriteProfile; +use super::import_command_sources; +use super::supported_command_sources; +use super::unique_command_sources; +use crate::manifest::load_plugin_command_paths; +use codex_utils_absolute_path::AbsolutePathBuf; +use std::fs; +use std::io; +use std::path::Path; + +const PLUGIN_COMMANDS_DIR: &str = "commands"; +const PLUGIN_METADATA_DIR: &str = ".codex-plugin"; +const MIGRATED_COMMAND_SKILLS_DIR: &str = "migrated-command-skills"; +const MAX_MIGRATED_COMMAND_SKILL_BYTES: usize = 4_000; + +const PLUGIN_REWRITE_PROFILE: RewriteProfile = RewriteProfile::new("AGENTS.md", &[]); +const PLUGIN_MIGRATION_PROFILE: CommandMigrationProfile = CommandMigrationProfile::new( + PLUGIN_REWRITE_PROFILE, + CommandDescriptionMode::RequireFrontmatter, +); + +pub(crate) fn migrate_plugin_commands(plugin_root: &Path) -> io::Result<()> { + let target_skills = plugin_root + .join(PLUGIN_METADATA_DIR) + .join(MIGRATED_COMMAND_SKILLS_DIR); + if target_skills.is_dir() { + fs::remove_dir_all(&target_skills)?; + } else if target_skills.exists() { + fs::remove_file(&target_skills)?; + } + import_command_sources( + plugin_command_sources(plugin_root)?, + &target_skills, + PLUGIN_MIGRATION_PROFILE, + CommandSkillSizeLimit::MaxBytes(MAX_MIGRATED_COMMAND_SKILL_BYTES), + )?; + Ok(()) +} + +pub(crate) fn migrated_command_skills_root(plugin_root: &AbsolutePathBuf) -> AbsolutePathBuf { + plugin_root + .join(PLUGIN_METADATA_DIR) + .join(MIGRATED_COMMAND_SKILLS_DIR) +} + +fn plugin_command_sources(plugin_root: &Path) -> io::Result> { + let command_paths = load_plugin_command_paths(plugin_root)? + .unwrap_or_else(|| vec![plugin_root.join(PLUGIN_COMMANDS_DIR)]); + let mut sources = Vec::new(); + for command_path in command_paths { + sources.extend(supported_command_sources( + &command_path, + CommandDescriptionMode::RequireFrontmatter, + )?); + } + Ok(unique_command_sources(sources)) +} diff --git a/codex-rs/core-plugins/src/command_migration/render.rs b/codex-rs/core-plugins/src/command_migration/render.rs new file mode 100644 index 000000000000..8e03f3065912 --- /dev/null +++ b/codex-rs/core-plugins/src/command_migration/render.rs @@ -0,0 +1,95 @@ +use super::RewriteProfile; + +pub(super) fn rewrite_terms(content: &str, profile: RewriteProfile) -> String { + let mut rewritten = + replace_case_insensitive_with_boundaries(content, profile.doc_file_name, "AGENTS.md"); + for from in profile.term_variants { + rewritten = replace_case_insensitive_with_boundaries(&rewritten, from, "Codex"); + } + for from in profile.case_sensitive_term_variants { + rewritten = replace_with_boundaries(&rewritten, from, "Codex"); + } + rewritten +} + +fn replace_with_boundaries(input: &str, needle: &str, replacement: &str) -> String { + if needle.is_empty() { + return input.to_string(); + } + + replace_with_boundaries_impl(input, needle, replacement, input) +} + +fn replace_case_insensitive_with_boundaries( + input: &str, + needle: &str, + replacement: &str, +) -> String { + let needle_lower = needle.to_ascii_lowercase(); + if needle_lower.is_empty() { + return input.to_string(); + } + let haystack_lower = input.to_ascii_lowercase(); + replace_with_boundaries_impl(input, &needle_lower, replacement, &haystack_lower) +} + +fn replace_with_boundaries_impl( + input: &str, + needle: &str, + replacement: &str, + searchable_input: &str, +) -> String { + let bytes = input.as_bytes(); + let mut output = String::with_capacity(input.len()); + let mut last_emitted = 0usize; + let mut search_start = 0usize; + + while let Some(relative_pos) = searchable_input[search_start..].find(needle) { + let start = search_start + relative_pos; + let end = start + needle.len(); + let boundary_before = start == 0 || !is_word_byte(bytes[start - 1]); + let boundary_after = end == bytes.len() || !is_word_byte(bytes[end]); + + if boundary_before && boundary_after { + output.push_str(&input[last_emitted..start]); + output.push_str(replacement); + last_emitted = end; + } + search_start = start + 1; + } + + if last_emitted == 0 { + return input.to_string(); + } + output.push_str(&input[last_emitted..]); + output +} + +pub(super) fn yaml_string(value: &str) -> String { + format!("\"{}\"", value.replace('\\', "\\\\").replace('"', "\\\"")) +} + +pub(super) fn slugify_name(value: &str) -> String { + let mut slug = String::new(); + let mut last_was_dash = false; + for ch in value.chars() { + if ch.is_ascii_alphanumeric() { + slug.push(ch.to_ascii_lowercase()); + last_was_dash = false; + } else if !last_was_dash { + slug.push('-'); + last_was_dash = true; + } + } + + let slug = slug.trim_matches('-').to_string(); + if slug.is_empty() { + "migrated".to_string() + } else { + slug + } +} + +fn is_word_byte(byte: u8) -> bool { + byte.is_ascii_alphanumeric() || byte == b'_' +} diff --git a/codex-rs/core-plugins/src/command_migration_tests.rs b/codex-rs/core-plugins/src/command_migration_tests.rs new file mode 100644 index 000000000000..3bf67dd94ab5 --- /dev/null +++ b/codex-rs/core-plugins/src/command_migration_tests.rs @@ -0,0 +1,146 @@ +use super::*; +use pretty_assertions::assert_eq; +use std::fs; +use std::path::Path; + +const TEST_REWRITE_PROFILE: RewriteProfile = RewriteProfile::new( + "CLAUDE.md", + &[ + "claude code", + "claude-code", + "claude_code", + "claudecode", + "claude", + ], +); + +#[test] +fn command_skill_names_must_fit_codex_skill_loader_limit() { + let source_name = "this-is-a-deeply-nested-command-with-a-very-long-name"; + let file = Path::new("commands/this/is/a/deeply/nested/command/with/a/very/long/name.md"); + let document = parse_command_content("---\ndescription: Review PR\n---\nReview\n"); + + assert!( + command_skill_name_if_supported( + source_name, + file, + &document, + CommandDescriptionMode::RequireFrontmatter, + ) + .is_none() + ); +} + +#[test] +fn commands_with_overlong_descriptions_are_preserved() { + let description = "x".repeat(1025); + let document = + parse_command_content(&format!("---\ndescription: {description}\n---\nReview\n")); + + assert_eq!( + command_skill_name_if_supported( + "review", + Path::new("commands/review.md"), + &document, + CommandDescriptionMode::RequireFrontmatter, + ), + Some("source-command-review".to_string()) + ); + + let rendered = render_command_skill( + &document.body, + "source-command-review", + &description, + "review", + TEST_REWRITE_PROFILE, + ); + assert_eq!( + parse_command_content(&rendered).description.as_deref(), + Some(description.as_str()) + ); +} + +#[test] +fn commands_with_provider_runtime_expansion_are_skipped() { + let document = parse_command_content( + "---\ndescription: Deploy\n---\nDeploy $ARGUMENTS from @release.yaml\n", + ); + + assert!( + command_skill_name_if_supported( + "deploy", + Path::new("commands/deploy.md"), + &document, + CommandDescriptionMode::RequireFrontmatter, + ) + .is_none() + ); +} + +#[test] +fn commands_without_description_are_skipped() { + let document = parse_command_content("Review the current change.\n"); + + assert!( + command_skill_name_if_supported( + "review", + Path::new("commands/review.md"), + &document, + CommandDescriptionMode::RequireFrontmatter, + ) + .is_none() + ); +} + +#[test] +fn commands_can_derive_descriptions_from_source_names() { + let root = tempfile::TempDir::new().expect("tempdir"); + let commands = root.path().join("commands"); + let target_skills = root.path().join("skills"); + fs::create_dir_all(&commands).expect("create commands"); + fs::write( + commands.join("review-code.md"), + "Review the current change.\n", + ) + .expect("write command"); + let profile = CommandMigrationProfile::new( + TEST_REWRITE_PROFILE, + CommandDescriptionMode::UseSourceNameFallback, + ); + + assert_eq!( + import_commands_with_profile(&commands, &target_skills, profile).unwrap(), + vec!["source-command-review-code".to_string()] + ); + let rendered = fs::read_to_string( + target_skills + .join("source-command-review-code") + .join("SKILL.md"), + ) + .expect("read migrated command"); + assert!(rendered.contains("description: \"Migrated source command `review-code`\"")); + assert!(rendered.contains("Review the current change.")); +} + +#[test] +fn command_slug_collisions_are_skipped() { + let root = tempfile::TempDir::new().expect("tempdir"); + let commands = root.path().join("commands"); + fs::create_dir_all(&commands).expect("create commands"); + fs::write( + commands.join("foo-bar.md"), + "---\ndescription: First\n---\nRun the first command.\n", + ) + .expect("write first command"); + fs::write( + commands.join("foo_bar.md"), + "---\ndescription: Second\n---\nRun the second command.\n", + ) + .expect("write second command"); + + assert_eq!( + unique_supported_command_sources(&commands, CommandDescriptionMode::RequireFrontmatter,) + .unwrap(), + Vec::::new() + ); +} diff --git a/codex-rs/core-plugins/src/lib.rs b/codex-rs/core-plugins/src/lib.rs index 2241eda43c32..eee1939ce507 100644 --- a/codex-rs/core-plugins/src/lib.rs +++ b/codex-rs/core-plugins/src/lib.rs @@ -1,4 +1,5 @@ mod app_mcp_routing; +mod command_migration; mod discoverable; pub mod installed_marketplaces; pub mod loader; @@ -37,6 +38,12 @@ pub type LoadedPlugin = codex_plugin::LoadedPlugin; pub use app_mcp_routing::apps_route_available; +pub use command_migration::CommandDescriptionMode; +pub use command_migration::CommandMigrationProfile; +pub use command_migration::RewriteProfile as CommandRewriteProfile; +pub use command_migration::count_missing_commands_with_profile; +pub use command_migration::import_commands_with_profile; +pub use command_migration::missing_command_names_with_profile; pub use discoverable::ToolSuggestDiscoverablePlugin; pub use discoverable::ToolSuggestPluginDiscoveryInput; pub use loader::PluginHookLoadOutcome; diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 160b710e9b8f..b3fd3c9759f0 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -1,5 +1,6 @@ use crate::app_mcp_routing::apply_app_mcp_routing_policy; use crate::app_mcp_routing::apps_route_available; +use crate::command_migration::migrated_command_skills_root; use crate::is_openai_curated_marketplace_name; use crate::manifest::PluginManifest; use crate::manifest::PluginManifestHooks; @@ -958,11 +959,36 @@ pub(crate) async fn load_plugin_skill_inventory( .collect::>(); let outcome = load_skills_from_roots(roots, plugin_skill_snapshots, root_scan_slots).await; let had_errors = !outcome.errors.is_empty(); + let migrated_command_skills = migrated_command_skills_root(plugin_root); + let migrated_command_skills = fs::canonicalize(migrated_command_skills.as_path()) + .ok() + .and_then(|path| AbsolutePathBuf::from_absolute_path_checked(path).ok()) + .unwrap_or(migrated_command_skills); let skills = outcome .skills .into_iter() .filter(|skill| skill.matches_product_restriction_for_product(restriction_product)) .collect::>(); + let native_skill_names = skills + .iter() + .filter(|skill| { + !skill + .path_to_skills_md + .as_path() + .starts_with(migrated_command_skills.as_path()) + }) + .map(|skill| skill.name.clone()) + .collect::>(); + let skills = skills + .into_iter() + .filter(|skill| { + !skill + .path_to_skills_md + .as_path() + .starts_with(migrated_command_skills.as_path()) + || !native_skill_names.contains(&skill.name) + }) + .collect::>(); PluginSkillInventory { skills, had_errors } } @@ -976,6 +1002,10 @@ fn plugin_skill_roots( } else { manifest_paths.skills.clone() }; + let migrated_command_skills = migrated_command_skills_root(plugin_root); + if migrated_command_skills.is_dir() { + paths.push(migrated_command_skills); + } paths.sort_unstable(); paths.dedup(); paths diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 0819f0a6fbbc..fbb518e91f9b 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -1885,6 +1885,153 @@ async fn load_plugins_uses_manifest_configured_component_paths() { } } +#[tokio::test] +async fn install_plugin_materializes_default_command_skills() { + let codex_home = TempDir::new().unwrap(); + let source_root = codex_home.path().join("source/sample"); + + write_file( + &source_root.join(".codex-plugin/plugin.json"), + r#"{ + "name": "sample", + "skills": "./custom-skills/" +}"#, + ); + fs::create_dir_all(source_root.join("custom-skills")).unwrap(); + write_file( + &source_root.join("custom-skills/source-command-pr-review/SKILL.md"), + "---\nname: source-command-pr-review\ndescription: Native review skill\n---\n", + ); + write_file( + &source_root.join("commands/pr/review.md"), + "---\ndescription: Review a pull request\n---\nInspect the proposed changes.\n", + ); + write_file( + &source_root.join("commands/summarize.md"), + "---\ndescription: Summarize a change\n---\nSummarize the proposed changes.\n", + ); + write_file( + &source_root.join("commands/oversized.md"), + &format!("---\ndescription: Oversized\n---\n{}", "x".repeat(4_000)), + ); + write_file( + &source_root.join(".codex-plugin/migrated-command-skills/undeclared-command/SKILL.md"), + "---\nname: undeclared-command\ndescription: undeclared command\n---\n", + ); + let result = PluginStore::new(codex_home.path().to_path_buf()) + .install( + source_root.abs(), + PluginId::parse("sample@test").expect("plugin id should parse"), + ) + .unwrap(); + let migrated_skill = result + .installed_path + .join(".codex-plugin/migrated-command-skills/source-command-pr-review/SKILL.md"); + let expected_migrated_skill = "---\nname: \"source-command-pr-review\"\ndescription: \"Review a pull request\"\n---\n\n# source-command-pr-review\n\nUse this skill when the user asks to run the migrated source command `pr-review`.\n\n## Command Template\n\nInspect the proposed changes.\n"; + assert_eq!( + fs::read_to_string(&migrated_skill).unwrap(), + expected_migrated_skill + ); + assert!( + !result + .installed_path + .join(".codex-plugin/migrated-command-skills/undeclared-command") + .exists() + ); + assert!( + !result + .installed_path + .join(".codex-plugin/migrated-command-skills/source-command-oversized") + .exists() + ); + + let manifest = crate::manifest::load_plugin_manifest(&result.installed_path).unwrap(); + let resolved = load_plugin_skills( + &result.installed_path, + &result.plugin_id, + &manifest, + /*restriction_product*/ None, + &SkillConfigRules::default(), + /*plugin_skill_snapshots*/ None, + Arc::new(Semaphore::new(MAX_CONCURRENT_ROOT_SCANS)), + ) + .await; + assert_eq!( + resolved + .skills + .iter() + .map(|skill| skill.path_to_skills_md.clone()) + .collect::>(), + vec![ + AbsolutePathBuf::from_absolute_path_checked( + fs::canonicalize( + result + .installed_path + .join("custom-skills/source-command-pr-review/SKILL.md") + ) + .unwrap() + ) + .unwrap(), + AbsolutePathBuf::from_absolute_path_checked( + fs::canonicalize(result.installed_path.join( + ".codex-plugin/migrated-command-skills/source-command-summarize/SKILL.md" + )) + .unwrap() + ) + .unwrap() + ] + ); +} + +#[test] +fn install_plugin_ignores_invalid_commands_manifest_field() { + let codex_home = TempDir::new().unwrap(); + let source_root = codex_home.path().join("source/sample"); + write_file( + &source_root.join(".codex-plugin/plugin.json"), + r#"{"name":"sample","commands":{}}"#, + ); + write_file( + &source_root.join("commands/review.md"), + "---\ndescription: Review\n---\nReview the current change.\n", + ); + + let result = PluginStore::new(codex_home.path().to_path_buf()) + .install( + source_root.abs(), + PluginId::parse("sample@test").expect("plugin id should parse"), + ) + .unwrap(); + + assert!( + !result + .installed_path + .join(".codex-plugin/migrated-command-skills") + .exists() + ); +} + +#[test] +fn install_plugin_ignores_command_migration_errors() { + let codex_home = TempDir::new().unwrap(); + let source_root = codex_home.path().join("source/sample"); + write_file( + &source_root.join(".codex-plugin/plugin.json"), + r#"{"name":"sample","commands":"./commands/review.md"}"#, + ); + fs::create_dir_all(source_root.join("commands")).unwrap(); + fs::write(source_root.join("commands/review.md"), [0xff]).unwrap(); + + let result = PluginStore::new(codex_home.path().to_path_buf()) + .install( + source_root.abs(), + PluginId::parse("sample@test").expect("plugin id should parse"), + ) + .unwrap(); + + assert!(result.installed_path.join("commands/review.md").is_file()); +} + #[tokio::test] async fn load_plugin_skills_dedupes_overlapping_manifest_roots() { let codex_home = TempDir::new().unwrap(); @@ -2851,6 +2998,10 @@ async fn install_plugin_writes_marketplace_manifest_fallback_when_missing_plugin "review skill", ) .unwrap(); + write_file( + &plugin_root.join("commands/review.md"), + "---\ndescription: Review code\n---\nReview the current change.\n", + ); fs::write( repo_root.join(".agents/plugins/marketplace.json"), r#"{ @@ -2866,6 +3017,7 @@ async fn install_plugin_writes_marketplace_manifest_fallback_when_missing_plugin "skills": [ "./skills/thermo-nuclear-code-quality-review" ], + "commands": ["./commands/review.md"], "category": "code-review" } ] @@ -2931,6 +3083,14 @@ async fn install_plugin_writes_marketplace_manifest_fallback_when_missing_plugin serde_json::json!({ "name": "Byron Grogan" }) ); assert_eq!(fallback_json["category"], "code-review"); + assert_eq!( + fs::read_to_string( + installed_path + .join(".codex-plugin/migrated-command-skills/source-command-review/SKILL.md") + ) + .unwrap(), + "---\nname: \"source-command-review\"\ndescription: \"Review code\"\n---\n\n# source-command-review\n\nUse this skill when the user asks to run the migrated source command `review`.\n\n## Command Template\n\nReview the current change.\n" + ); } #[tokio::test] diff --git a/codex-rs/core-plugins/src/manifest.rs b/codex-rs/core-plugins/src/manifest.rs index 0edebe3699a3..ff6d2c902a1a 100644 --- a/codex-rs/core-plugins/src/manifest.rs +++ b/codex-rs/core-plugins/src/manifest.rs @@ -6,7 +6,9 @@ use codex_utils_plugins::find_plugin_manifest_path; use serde::Deserialize; use serde_json::Value as JsonValue; use std::fs; +use std::io; use std::path::Path; +use std::path::PathBuf; const MAX_DEFAULT_PROMPT_COUNT: usize = 3; const MAX_DEFAULT_PROMPT_LEN: usize = 128; @@ -44,6 +46,12 @@ struct RawPluginManifest { interface: Option, } +#[derive(Deserialize)] +struct RawPluginCommandManifest { + #[serde(default)] + commands: Option, +} + #[derive(Debug, Default, Deserialize)] #[serde(rename_all = "camelCase")] struct RawPluginManifestInterface { @@ -139,6 +147,24 @@ pub fn load_plugin_manifest(plugin_root: &Path) -> Option { } } +pub(crate) fn load_plugin_command_paths(plugin_root: &Path) -> io::Result>> { + let Some(manifest_path) = find_plugin_manifest_path(plugin_root) else { + return Ok(None); + }; + let manifest = + serde_json::from_str::(&fs::read_to_string(manifest_path)?) + .map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err))?; + let Some(commands) = manifest.commands else { + return Ok(None); + }; + let plugin_root = PathUri::from_host_native_path(plugin_root)?; + resolve_manifest_paths(&plugin_root, "commands", Some(&commands)) + .into_iter() + .map(|path| Ok(path.to_abs_path()?.into_path_buf())) + .collect::>>() + .map(Some) +} + pub(crate) fn parse_plugin_manifest( plugin_root: &Path, manifest_path: &Path, diff --git a/codex-rs/core-plugins/src/store.rs b/codex-rs/core-plugins/src/store.rs index 819f9731be05..92c531938a27 100644 --- a/codex-rs/core-plugins/src/store.rs +++ b/codex-rs/core-plugins/src/store.rs @@ -1,3 +1,4 @@ +use crate::command_migration::migrate_plugin_commands; use crate::manifest::PluginManifest; use crate::manifest::load_plugin_manifest; use crate::manifest::parse_plugin_manifest; @@ -563,6 +564,9 @@ fn replace_plugin_root_atomically( fs::write(&manifest_path, contents) .map_err(|err| PluginStoreError::io("failed to write fallback plugin manifest", err))?; } + if let Err(err) = migrate_plugin_commands(&staged_version_root) { + tracing::warn!(%err, "failed to migrate plugin commands into skills"); + } let target_version_root = target_root.join(plugin_version); if target_root.exists() && !target_version_root.exists() { diff --git a/codex-rs/external-agent-migration/src/lib.rs b/codex-rs/external-agent-migration/src/lib.rs index e87c9d613eb4..52506a9a8f27 100644 --- a/codex-rs/external-agent-migration/src/lib.rs +++ b/codex-rs/external-agent-migration/src/lib.rs @@ -21,8 +21,6 @@ const SOURCE_EXTERNAL_AGENT_NAME: &str = "claude"; const EXTERNAL_AGENT_MCP_CONFIG_FILE: &str = ".mcp.json"; const EXTERNAL_AGENT_HOOKS_SUBDIR: &str = "hooks"; const EXTERNAL_AGENT_MIGRATED_HOOKS_SUBDIR: &str = "hooks"; -const COMMAND_SKILL_PREFIX: &str = "source-command"; -const MAX_SKILL_NAME_LEN: usize = 64; /// Describes source-specific terms that should be rewritten in migrated artifacts. #[derive(Clone, Copy)] @@ -62,34 +60,6 @@ impl RewriteProfile { } } -/// Controls how migrated commands obtain the description required by a Codex skill. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum CommandDescriptionMode { - /// Skip source commands that do not declare a non-empty frontmatter description. - RequireFrontmatter, - /// Derive a stable description from the source command name when frontmatter is absent. - UseSourceNameFallback, -} - -/// Describes source-specific command migration behavior. -#[derive(Clone, Copy)] -pub struct CommandMigrationProfile { - rewrite_profile: RewriteProfile, - description_mode: CommandDescriptionMode, -} - -impl CommandMigrationProfile { - pub const fn new( - rewrite_profile: RewriteProfile, - description_mode: CommandDescriptionMode, - ) -> Self { - Self { - rewrite_profile, - description_mode, - } - } -} - #[derive(Debug)] struct ParsedDocument { frontmatter: BTreeMap, @@ -228,70 +198,6 @@ pub fn import_subagents_with_rewrite_profile( Ok(imported) } -pub fn count_missing_commands_with_profile( - source_commands: &Path, - target_skills: &Path, - profile: CommandMigrationProfile, -) -> io::Result { - Ok(missing_command_names_with_profile(source_commands, target_skills, profile)?.len()) -} - -pub fn missing_command_names_with_profile( - source_commands: &Path, - target_skills: &Path, - profile: CommandMigrationProfile, -) -> io::Result> { - Ok( - unique_supported_command_sources(source_commands, profile.description_mode)? - .into_iter() - .filter(|(_source_file, name)| !target_skills.join(name).exists()) - .map(|(_source_file, name)| name) - .collect(), - ) -} - -pub fn import_commands_with_profile( - source_commands: &Path, - target_skills: &Path, - profile: CommandMigrationProfile, -) -> io::Result> { - if !source_commands.is_dir() { - return Ok(Vec::new()); - } - - fs::create_dir_all(target_skills)?; - let mut imported = Vec::new(); - for (source_file, name) in - unique_supported_command_sources(source_commands, profile.description_mode)? - { - let document = parse_document(&source_file)?; - let target_dir = target_skills.join(&name); - if target_dir.exists() { - continue; - } - fs::create_dir_all(&target_dir)?; - let source_name = command_source_name(source_commands, &source_file); - let Some(description) = - command_skill_description(&document, &source_name, profile.description_mode) - else { - continue; - }; - fs::write( - target_dir.join("SKILL.md"), - render_command_skill( - &document.body, - &name, - &description, - &source_name, - profile.rewrite_profile, - ), - )?; - imported.push(name); - } - - Ok(imported) -} - fn read_external_mcp_servers( source_root: &Path, external_agent_home: Option<&Path>, @@ -851,61 +757,6 @@ fn subagent_target_file(source_file: &Path, target_agents: &Path) -> Option io::Result> { - let mut files = Vec::new(); - collect_markdown_files(source_commands, &mut files)?; - files.sort(); - Ok(files) -} - -fn unique_supported_command_sources( - source_commands: &Path, - description_mode: CommandDescriptionMode, -) -> io::Result> { - let mut by_name = BTreeMap::>::new(); - for source_file in command_source_files(source_commands)? { - let document = parse_document(&source_file)?; - let Some(name) = command_skill_name_if_supported( - source_commands, - &source_file, - &document, - description_mode, - ) else { - continue; - }; - by_name.entry(name).or_default().push(source_file); - } - - Ok(by_name - .into_iter() - .filter_map(|(name, source_files)| { - let [source_file] = source_files.as_slice() else { - return None; - }; - Some((source_file.clone(), name)) - }) - .collect()) -} - -fn collect_markdown_files(dir: &Path, files: &mut Vec) -> io::Result<()> { - if !dir.is_dir() { - return Ok(()); - } - - for entry in fs::read_dir(dir)? { - let entry = entry?; - let path = entry.path(); - let file_type = entry.file_type()?; - if file_type.is_dir() { - collect_markdown_files(&path, files)?; - } else if file_type.is_file() && path.extension().and_then(|ext| ext.to_str()) == Some("md") - { - files.push(path); - } - } - Ok(()) -} - fn parse_document(source_file: &Path) -> io::Result { let content = fs::read_to_string(source_file)?; Ok(parse_document_content(&content)) @@ -1067,102 +918,6 @@ fn render_agent_body(body: &str, rewrite_profile: RewriteProfile) -> String { } } -fn command_skill_name(source_commands: &Path, source_file: &Path) -> String { - slugify_name(&format!( - "{COMMAND_SKILL_PREFIX}-{}", - command_source_name(source_commands, source_file) - )) -} - -fn command_skill_name_if_supported( - source_commands: &Path, - source_file: &Path, - document: &ParsedDocument, - description_mode: CommandDescriptionMode, -) -> 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); - command_skill_description(document, &source_name, description_mode)?; - let name = command_skill_name(source_commands, source_file); - if name.chars().count() > MAX_SKILL_NAME_LEN { - return None; - } - if has_unsupported_command_template_features(&document.body) { - return None; - } - Some(name) -} - -fn command_skill_description( - document: &ParsedDocument, - source_name: &str, - description_mode: CommandDescriptionMode, -) -> Option { - let frontmatter_description = document - .frontmatter - .get("description") - .and_then(FrontmatterValue::as_scalar) - .filter(|value| !value.trim().is_empty()) - .map(ToOwned::to_owned); - frontmatter_description.or_else(|| match description_mode { - CommandDescriptionMode::RequireFrontmatter => None, - CommandDescriptionMode::UseSourceNameFallback => { - Some(format!("Migrated source command `{source_name}`")) - } - }) -} - -fn command_source_name(source_commands: &Path, source_file: &Path) -> String { - source_file - .strip_prefix(source_commands) - .unwrap_or(source_file) - .with_extension("") - .components() - .filter_map(|component| component.as_os_str().to_str()) - .collect::>() - .join("-") -} - -fn render_command_skill( - body: &str, - name: &str, - description: &str, - source_name: &str, - rewrite_profile: RewriteProfile, -) -> String { - let body = rewrite_external_agent_terms(body.trim(), rewrite_profile); - let template_body = if body.is_empty() { - "No command template body was found.".to_string() - } else { - body - }; - format!( - "---\nname: {}\ndescription: {}\n---\n\n# {name}\n\nUse this skill when the user asks to run the migrated source command `{source_name}`.\n\n## Command Template\n\n{template_body}\n", - yaml_string(name), - yaml_string(&rewrite_external_agent_terms(description, rewrite_profile)), - ) -} - -fn has_unsupported_command_template_features(template: &str) -> bool { - template.contains("$ARGUMENTS") - || contains_numbered_argument_placeholder(template) - || (template.contains("{{") && template.contains("}}")) - || template.contains("!`") - || template.contains("! `") - || template - .split_whitespace() - .any(|token| token.strip_prefix('@').is_some_and(|rest| !rest.is_empty())) -} - -fn contains_numbered_argument_placeholder(template: &str) -> bool { - let bytes = template.as_bytes(); - bytes - .windows(2) - .any(|window| window[0] == b'$' && window[1].is_ascii_digit()) -} - fn frontmatter_string( frontmatter: &BTreeMap, key: &str, @@ -1217,31 +972,6 @@ fn json_u64(value: &JsonValue) -> Option { value.as_u64().or_else(|| value.as_str()?.parse().ok()) } -fn yaml_string(value: &str) -> String { - format!("\"{}\"", value.replace('\\', "\\\\").replace('"', "\\\"")) -} - -fn slugify_name(value: &str) -> String { - let mut slug = String::new(); - let mut last_was_dash = false; - for ch in value.chars() { - if ch.is_ascii_alphanumeric() { - slug.push(ch.to_ascii_lowercase()); - last_was_dash = false; - } else if !last_was_dash { - slug.push('-'); - last_was_dash = true; - } - } - - let slug = slug.trim_matches('-').to_string(); - if slug.is_empty() { - "migrated".to_string() - } else { - slug - } -} - impl FrontmatterValue { fn as_scalar(&self) -> Option<&str> { match self { diff --git a/codex-rs/external-agent-migration/src/lib_tests.rs b/codex-rs/external-agent-migration/src/lib_tests.rs index 049f2acb415b..609c6cfb93b1 100644 --- a/codex-rs/external-agent-migration/src/lib_tests.rs +++ b/codex-rs/external-agent-migration/src/lib_tests.rs @@ -4,7 +4,6 @@ use super::hooks_cla::rewrite_hook_command_cla; use super::*; use pretty_assertions::assert_eq; -const MAX_SKILL_DESCRIPTION_LEN: usize = 1024; const TEST_REWRITE_PROFILE: RewriteProfile = RewriteProfile::new( "CLAUDE.md", &[ @@ -333,155 +332,6 @@ command = "enabled-server" ); } -#[test] -fn command_skill_names_include_nested_paths() { - let root = source_path("commands"); - let file = source_path("commands/pr/review.md"); - - assert_eq!(command_skill_name(&root, &file), "source-command-pr-review"); -} - -#[test] -fn command_skill_names_must_fit_codex_skill_loader_limit() { - let root = source_path("commands"); - let file = source_path("commands/this/is/a/deeply/nested/command/with/a/very/long/name.md"); - let document = parse_document_content("---\ndescription: Review PR\n---\nReview\n"); - - assert!( - command_skill_name_if_supported( - &root, - &file, - &document, - CommandDescriptionMode::RequireFrontmatter, - ) - .is_none() - ); -} - -#[test] -fn commands_with_overlong_descriptions_are_preserved() { - let root = source_path("commands"); - let file = source_path("commands/review.md"); - let description = "x".repeat(MAX_SKILL_DESCRIPTION_LEN + 1); - let document = - parse_document_content(&format!("---\ndescription: {description}\n---\nReview\n")); - - assert_eq!( - command_skill_name_if_supported( - &root, - &file, - &document, - CommandDescriptionMode::RequireFrontmatter, - ), - Some("source-command-review".to_string()) - ); - - let rendered = render_command_skill( - &document.body, - "source-command-review", - &description, - "review", - TEST_REWRITE_PROFILE, - ); - let rendered_document = parse_document_content(&rendered); - assert_eq!( - rendered_document - .frontmatter - .get("description") - .and_then(FrontmatterValue::as_scalar), - Some(description.as_str()) - ); -} - -#[test] -fn commands_with_provider_runtime_expansion_are_skipped() { - let root = source_path("commands"); - let file = source_path("commands/deploy.md"); - let document = parse_document_content( - "---\ndescription: Deploy\n---\nDeploy $ARGUMENTS from @release.yaml\n", - ); - - assert!( - command_skill_name_if_supported( - &root, - &file, - &document, - CommandDescriptionMode::RequireFrontmatter, - ) - .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, - CommandDescriptionMode::RequireFrontmatter, - ) - .is_none() - ); -} - -#[test] -fn commands_can_derive_descriptions_from_source_names() { - let root = tempfile::TempDir::new().expect("tempdir"); - let commands = root.path().join("commands"); - let target_skills = root.path().join("skills"); - fs::create_dir_all(&commands).expect("create commands"); - fs::write( - commands.join("review-code.md"), - "Review the current change.\n", - ) - .expect("write command"); - let profile = CommandMigrationProfile::new( - TEST_REWRITE_PROFILE, - CommandDescriptionMode::UseSourceNameFallback, - ); - - assert_eq!( - import_commands_with_profile(&commands, &target_skills, profile).unwrap(), - vec!["source-command-review-code".to_string()] - ); - let rendered = fs::read_to_string( - target_skills - .join("source-command-review-code") - .join("SKILL.md"), - ) - .expect("read migrated command"); - assert!(rendered.contains("description: \"Migrated source command `review-code`\"")); - assert!(rendered.contains("Review the current change.")); -} - -#[test] -fn command_slug_collisions_are_skipped() { - let root = tempfile::TempDir::new().expect("tempdir"); - let commands = root.path().join("commands"); - fs::create_dir_all(&commands).expect("create commands"); - fs::write( - commands.join("foo-bar.md"), - "---\ndescription: First\n---\nRun the first command.\n", - ) - .expect("write first command"); - fs::write( - commands.join("foo_bar.md"), - "---\ndescription: Second\n---\nRun the second command.\n", - ) - .expect("write second command"); - - assert_eq!( - unique_supported_command_sources(&commands, CommandDescriptionMode::RequireFrontmatter,) - .unwrap(), - Vec::<(PathBuf, String)>::new() - ); -} - #[test] fn subagent_accepts_yaml_block_lists_by_ignoring_unsupported_fields() { let document = parse_document_content(