mirror of
https://github.com/openai/codex.git
synced 2026-10-20 17:38:45 +00:00
Migrate Claude commands without frontmatter (#40643)
## What changed - Import supported Claude commands that lack a usable frontmatter description by deriving the skill description from the source command name. - Preserve commands with explicit descriptions when multiple source paths normalize to the same skill name, and skip ambiguous fallback-only collisions. - Apply the expanded detection and import behavior to both user-level and repository-level command directories. ## Testing - Add service tests for commands without frontmatter, normalized-name collisions, unsupported templates, and post-import detection. GitOrigin-RevId: 53691193128a6b1be192c4f83bdf563980a0e55d
This commit is contained in:
committed by
copyberry
parent
da4cf1cdea
commit
a9e447a69d
@@ -69,6 +69,190 @@ async fn detect_home_lists_config_skills_and_agents_md() {
|
||||
assert_eq!(items, expected);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn detect_and_import_claude_commands_without_frontmatter() {
|
||||
let (root, external_agent_home, codex_home) = fixture_paths();
|
||||
let repo_root = root.path().join("repo");
|
||||
fs::create_dir_all(repo_root.join(".git")).expect("create git dir");
|
||||
let service = service_for_paths(external_agent_home.clone(), codex_home);
|
||||
|
||||
for (cwd, source_commands, target_skills) in [
|
||||
(
|
||||
None,
|
||||
external_agent_home.join("commands"),
|
||||
root.path().join(".agents").join("skills"),
|
||||
),
|
||||
(
|
||||
Some(repo_root.clone()),
|
||||
repo_root.join(EXTERNAL_AGENT_DIR).join("commands"),
|
||||
repo_root.join(".agents").join("skills"),
|
||||
),
|
||||
] {
|
||||
fs::create_dir_all(source_commands.join("pr")).expect("create commands");
|
||||
fs::write(
|
||||
source_commands.join("pr").join("review.md"),
|
||||
format!("Review changes with {SOURCE_EXTERNAL_AGENT_PRODUCT_NAME} and {EXTERNAL_AGENT_CONFIG_MD}.\n"),
|
||||
)
|
||||
.expect("write plain command");
|
||||
fs::write(source_commands.join("deploy.md"), "Deploy $ARGUMENTS.\n")
|
||||
.expect("write unsupported command");
|
||||
|
||||
let options = ExternalAgentConfigDetectOptions {
|
||||
include_home: cwd.is_none(),
|
||||
include_memory: false,
|
||||
cwds: cwd.clone().map(|cwd| vec![cwd]),
|
||||
};
|
||||
let items = service.detect(options.clone()).await.expect("detect");
|
||||
assert_eq!(
|
||||
items,
|
||||
vec![ExternalAgentConfigMigrationItem {
|
||||
item_type: ExternalAgentConfigMigrationItemType::Commands,
|
||||
description: format!(
|
||||
"Migrate commands from {} to {}",
|
||||
source_commands.display(),
|
||||
target_skills.display()
|
||||
),
|
||||
cwd,
|
||||
details: Some(MigrationDetails {
|
||||
commands: named_migrations(vec!["source-command-pr-review".to_string()]),
|
||||
..Default::default()
|
||||
}),
|
||||
}]
|
||||
);
|
||||
|
||||
service.import(items).await;
|
||||
assert_eq!(
|
||||
fs::read_to_string(
|
||||
target_skills
|
||||
.join("source-command-pr-review")
|
||||
.join("SKILL.md")
|
||||
)
|
||||
.expect("read migrated command"),
|
||||
"---\nname: \"source-command-pr-review\"\ndescription: \"Migrated source command `pr-review`\"\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\nReview changes with Codex and AGENTS.md.\n"
|
||||
);
|
||||
assert!(!target_skills.join("source-command-deploy").exists());
|
||||
assert_eq!(
|
||||
service.detect(options).await.expect("detect after import"),
|
||||
Vec::<ExternalAgentConfigMigrationItem>::new()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn detect_and_import_claude_commands_preserves_described_commands_on_collision() {
|
||||
let (root, external_agent_home, codex_home) = fixture_paths();
|
||||
let repo_root = root.path().join("repo");
|
||||
fs::create_dir_all(repo_root.join(".git")).expect("create git dir");
|
||||
let service = service_for_paths(external_agent_home.clone(), codex_home);
|
||||
|
||||
for (cwd, source_commands, target_skills) in [
|
||||
(
|
||||
None,
|
||||
external_agent_home.join("commands"),
|
||||
root.path().join(".agents").join("skills"),
|
||||
),
|
||||
(
|
||||
Some(repo_root.clone()),
|
||||
repo_root.join(EXTERNAL_AGENT_DIR).join("commands"),
|
||||
repo_root.join(".agents").join("skills"),
|
||||
),
|
||||
] {
|
||||
fs::create_dir_all(source_commands.join("foo")).expect("create commands");
|
||||
for (path, content) in [
|
||||
("aaa-fallback.md", "Run the standalone fallback.\n"),
|
||||
(
|
||||
"zzz-described.md",
|
||||
"---\ndescription: Standalone described command\n---\nRun the standalone described command.\n",
|
||||
),
|
||||
(
|
||||
"foo-bar.md",
|
||||
"---\ndescription: Original description\n---\nRun the described command.\n",
|
||||
),
|
||||
("foo bar.md", "Run the plain fallback.\n"),
|
||||
(
|
||||
"foo_bar.md",
|
||||
"---\ndescription: ' '\n---\nRun the blank-description fallback.\n",
|
||||
),
|
||||
(
|
||||
"foo/bar.md",
|
||||
"---\ndescription: [unusable]\n---\nRun the unusable-description fallback.\n",
|
||||
),
|
||||
(
|
||||
"ambiguous-name.md",
|
||||
"---\ndescription: First\n---\nRun the first described command.\n",
|
||||
),
|
||||
(
|
||||
"ambiguous_name.md",
|
||||
"---\ndescription: Second\n---\nRun the second described command.\n",
|
||||
),
|
||||
("ambiguous name.md", "Run the ambiguous fallback.\n"),
|
||||
("fallback-name.md", "Run the first fallback.\n"),
|
||||
("fallback_name.md", "Run the second fallback.\n"),
|
||||
(
|
||||
"unsupported-name.md",
|
||||
"---\ndescription: Deploy\n---\nDeploy $ARGUMENTS.\n",
|
||||
),
|
||||
("unsupported_name.md", "Run the supported fallback.\n"),
|
||||
] {
|
||||
fs::write(source_commands.join(path), content).expect("write command");
|
||||
}
|
||||
|
||||
let options = ExternalAgentConfigDetectOptions {
|
||||
include_home: cwd.is_none(),
|
||||
include_memory: false,
|
||||
cwds: cwd.clone().map(|cwd| vec![cwd]),
|
||||
};
|
||||
let items = service.detect(options.clone()).await.expect("detect");
|
||||
assert_eq!(
|
||||
items,
|
||||
vec![ExternalAgentConfigMigrationItem {
|
||||
item_type: ExternalAgentConfigMigrationItemType::Commands,
|
||||
description: format!(
|
||||
"Migrate commands from {} to {}",
|
||||
source_commands.display(),
|
||||
target_skills.display()
|
||||
),
|
||||
cwd,
|
||||
details: Some(MigrationDetails {
|
||||
commands: named_migrations(vec![
|
||||
"source-command-aaa-fallback".to_string(),
|
||||
"source-command-foo-bar".to_string(),
|
||||
"source-command-unsupported-name".to_string(),
|
||||
"source-command-zzz-described".to_string(),
|
||||
]),
|
||||
..Default::default()
|
||||
}),
|
||||
}]
|
||||
);
|
||||
|
||||
service.import(items).await;
|
||||
assert_eq!(
|
||||
fs::read_to_string(
|
||||
target_skills
|
||||
.join("source-command-foo-bar")
|
||||
.join("SKILL.md")
|
||||
)
|
||||
.expect("read described command"),
|
||||
"---\nname: \"source-command-foo-bar\"\ndescription: \"Original description\"\n---\n\n# source-command-foo-bar\n\nUse this skill when the user asks to run the migrated source command `foo-bar`.\n\n## Command Template\n\nRun the described command.\n"
|
||||
);
|
||||
assert_eq!(
|
||||
fs::read_to_string(
|
||||
target_skills
|
||||
.join("source-command-unsupported-name")
|
||||
.join("SKILL.md")
|
||||
)
|
||||
.expect("read supported fallback command"),
|
||||
"---\nname: \"source-command-unsupported-name\"\ndescription: \"Migrated source command `unsupported_name`\"\n---\n\n# source-command-unsupported-name\n\nUse this skill when the user asks to run the migrated source command `unsupported_name`.\n\n## Command Template\n\nRun the supported fallback.\n"
|
||||
);
|
||||
assert!(!target_skills.join("source-command-ambiguous-name").exists());
|
||||
assert!(!target_skills.join("source-command-fallback-name").exists());
|
||||
assert_eq!(
|
||||
service.detect(options).await.expect("detect after import"),
|
||||
Vec::<ExternalAgentConfigMigrationItem>::new()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn detect_cursor_home_lists_user_and_managed_skills() {
|
||||
let root = TempDir::new().expect("create tempdir");
|
||||
|
||||
@@ -3,7 +3,6 @@ use crate::RewriteProfile;
|
||||
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_core_plugins::missing_command_names_with_profile;
|
||||
@@ -24,14 +23,23 @@ pub(super) const OFFICIAL_MARKETPLACE_SOURCE: &str = "anthropics/claude-plugins-
|
||||
pub(super) const CLAUDE_CODE_MARKETPLACE_NAME: &str = "claude-code-plugins";
|
||||
pub(super) const CLAUDE_CODE_MARKETPLACE_SOURCE: &str = "anthropics/claude-code";
|
||||
pub(super) const REWRITE_PROFILE: RewriteProfile = ClaSource::REWRITE_PROFILE;
|
||||
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,
|
||||
);
|
||||
const COMMAND_REWRITE_PROFILE: CommandRewriteProfile = CommandRewriteProfile::new(
|
||||
REWRITE_PROFILE.doc_file_name(),
|
||||
REWRITE_PROFILE.term_variants(),
|
||||
)
|
||||
.with_case_sensitive_term_variants(REWRITE_PROFILE.case_sensitive_term_variants());
|
||||
|
||||
// Preserve described commands first. The fallback pass still checks all sources for collisions.
|
||||
const COMMAND_MIGRATION_PROFILES: [CommandMigrationProfile; 2] = [
|
||||
CommandMigrationProfile::new(
|
||||
COMMAND_REWRITE_PROFILE,
|
||||
CommandDescriptionMode::RequireFrontmatter,
|
||||
),
|
||||
CommandMigrationProfile::new(
|
||||
COMMAND_REWRITE_PROFILE,
|
||||
CommandDescriptionMode::UseSourceNameFallback,
|
||||
),
|
||||
];
|
||||
|
||||
pub(super) fn marketplace_import_sources(
|
||||
settings: &JsonValue,
|
||||
@@ -127,21 +135,41 @@ pub(super) fn import_source_commands(
|
||||
source_commands: &Path,
|
||||
target_skills: &Path,
|
||||
) -> io::Result<Vec<String>> {
|
||||
import_commands_with_profile(source_commands, target_skills, COMMAND_MIGRATION_PROFILE)
|
||||
let mut imported = Vec::new();
|
||||
for profile in COMMAND_MIGRATION_PROFILES {
|
||||
imported.extend(import_commands_with_profile(
|
||||
source_commands,
|
||||
target_skills,
|
||||
profile,
|
||||
)?);
|
||||
}
|
||||
imported.sort();
|
||||
imported.dedup();
|
||||
Ok(imported)
|
||||
}
|
||||
|
||||
pub(super) fn count_missing_source_commands(
|
||||
source_commands: &Path,
|
||||
target_skills: &Path,
|
||||
) -> io::Result<usize> {
|
||||
count_missing_commands_with_profile(source_commands, target_skills, COMMAND_MIGRATION_PROFILE)
|
||||
Ok(missing_source_command_names(source_commands, target_skills)?.len())
|
||||
}
|
||||
|
||||
pub(super) fn missing_source_command_names(
|
||||
source_commands: &Path,
|
||||
target_skills: &Path,
|
||||
) -> io::Result<Vec<String>> {
|
||||
missing_command_names_with_profile(source_commands, target_skills, COMMAND_MIGRATION_PROFILE)
|
||||
let mut names = Vec::new();
|
||||
for profile in COMMAND_MIGRATION_PROFILES {
|
||||
names.extend(missing_command_names_with_profile(
|
||||
source_commands,
|
||||
target_skills,
|
||||
profile,
|
||||
)?);
|
||||
}
|
||||
names.sort();
|
||||
names.dedup();
|
||||
Ok(names)
|
||||
}
|
||||
|
||||
pub(crate) fn extract_plugin_migration_details(
|
||||
|
||||
Reference in New Issue
Block a user