mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Migrate plugin commands into skills on install (#33411)
## What changed - Convert supported plugin command Markdown into generated skills when a plugin is installed. - Read command sources from the manifest's `commands` field, falling back to the plugin's `commands/` directory. - Load generated command skills alongside native plugin skills while preferring native skills with the same name. - Skip commands with unsupported templates, missing descriptions, name collisions, or generated skills larger than 4 KB, and keep migration failures from blocking plugin installation. ## Testing - Cover default and manifest-configured command paths, stale generated skill cleanup, size limits, native-skill precedence, invalid manifest fields, and non-fatal migration errors. GitOrigin-RevId: 67d910048544b160b191066569bca69b2b951ebc
This commit is contained in:
committed by
copyberry
parent
38b064c31b
commit
2cd6ed7509
@@ -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 }
|
||||
|
||||
@@ -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<String>,
|
||||
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<usize> {
|
||||
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<Vec<String>> {
|
||||
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<Vec<String>> {
|
||||
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<CommandSource>,
|
||||
target_skills: &Path,
|
||||
profile: CommandMigrationProfile,
|
||||
size_limit: CommandSkillSizeLimit,
|
||||
) -> io::Result<Vec<String>> {
|
||||
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<Vec<CommandSource>> {
|
||||
Ok(unique_command_sources(supported_command_sources(
|
||||
source_commands,
|
||||
description_mode,
|
||||
)?))
|
||||
}
|
||||
|
||||
fn supported_command_sources(
|
||||
source_commands: &Path,
|
||||
description_mode: CommandDescriptionMode,
|
||||
) -> io::Result<Vec<CommandSource>> {
|
||||
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<CommandSource>) -> Vec<CommandSource> {
|
||||
let mut by_name = BTreeMap::<String, BTreeMap<PathBuf, String>>::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<Vec<PathBuf>> {
|
||||
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<PathBuf>) -> 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<ParsedCommand> {
|
||||
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<String> {
|
||||
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<String> {
|
||||
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<String> {
|
||||
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<String> {
|
||||
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::<Vec<_>>()
|
||||
.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;
|
||||
@@ -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<Vec<CommandSource>> {
|
||||
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))
|
||||
}
|
||||
@@ -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'_'
|
||||
}
|
||||
@@ -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::<CommandSource>::new()
|
||||
);
|
||||
}
|
||||
@@ -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<codex_config::McpServerConfig
|
||||
pub type PluginLoadOutcome = codex_plugin::PluginLoadOutcome<codex_config::McpServerConfig>;
|
||||
|
||||
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;
|
||||
|
||||
@@ -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::<Vec<_>>();
|
||||
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::<Vec<_>>();
|
||||
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::<HashSet<_>>();
|
||||
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::<Vec<_>>();
|
||||
|
||||
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
|
||||
|
||||
@@ -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<_>>(),
|
||||
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]
|
||||
|
||||
@@ -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<RawPluginManifestInterface>,
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
struct RawPluginCommandManifest {
|
||||
#[serde(default)]
|
||||
commands: Option<RawPluginManifestPaths>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Default, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
struct RawPluginManifestInterface {
|
||||
@@ -139,6 +147,24 @@ pub fn load_plugin_manifest(plugin_root: &Path) -> Option<PluginManifest> {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn load_plugin_command_paths(plugin_root: &Path) -> io::Result<Option<Vec<PathBuf>>> {
|
||||
let Some(manifest_path) = find_plugin_manifest_path(plugin_root) else {
|
||||
return Ok(None);
|
||||
};
|
||||
let manifest =
|
||||
serde_json::from_str::<RawPluginCommandManifest>(&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::<io::Result<Vec<_>>>()
|
||||
.map(Some)
|
||||
}
|
||||
|
||||
pub(crate) fn parse_plugin_manifest(
|
||||
plugin_root: &Path,
|
||||
manifest_path: &Path,
|
||||
|
||||
@@ -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() {
|
||||
|
||||
Reference in New Issue
Block a user