mirror of
https://github.com/openai/codex.git
synced 2026-11-10 21:00:47 +00:00
Add local-only Claude Code skill migration (#60492)
## What changed - Recognize `claude-code-local-skills` as a migration source that excludes the home skills directory's `synced` folder from detection and import. - Exclude the home skills directory's `.trash` folder from Claude Code skill detection and import in all modes. - Keep importing `synced` skills in the default mode while excluding `.trash` directories within that tree. ## Testing Extend the skill import regression test to cover local-only imports, default imports of synced skills, and exclusion of both top-level and nested trash directories. GitOrigin-RevId: 2c036319d022820659a60179bf58c37621f6ba14
This commit is contained in:
@@ -212,6 +212,11 @@ impl ExternalAgentConfigService {
|
||||
&target_skills,
|
||||
)?);
|
||||
}
|
||||
skill_names.retain(|name| {
|
||||
!self
|
||||
.source
|
||||
.is_reserved_skill_directory(scope, name.as_ref(), self.skill_import_mode)
|
||||
});
|
||||
skill_names.sort();
|
||||
skill_names.dedup();
|
||||
let skills_count = skill_names.len();
|
||||
|
||||
@@ -55,14 +55,22 @@ pub(super) enum ExternalAgentSource {
|
||||
Cur,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub(super) enum SkillImportMode {
|
||||
All,
|
||||
LocalOnly,
|
||||
}
|
||||
|
||||
impl ExternalAgentSource {
|
||||
pub(super) fn from_migration_source(migration_source: Option<&str>) -> Self {
|
||||
pub(super) fn from_migration_source(migration_source: Option<&str>) -> (Self, SkillImportMode) {
|
||||
if migration_source
|
||||
.is_some_and(|source| source.eq_ignore_ascii_case(CurSource::MIGRATION_SOURCE))
|
||||
{
|
||||
Self::Cur
|
||||
(Self::Cur, SkillImportMode::All)
|
||||
} else if migration_source == Some("claude-code-local-skills") {
|
||||
(Self::Cla, SkillImportMode::LocalOnly)
|
||||
} else {
|
||||
Self::Cla
|
||||
(Self::Cla, SkillImportMode::All)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -81,6 +89,26 @@ impl ExternalAgentSource {
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn is_reserved_skill_directory(
|
||||
self,
|
||||
scope: &MigrationScope,
|
||||
name: &std::ffi::OsStr,
|
||||
mode: SkillImportMode,
|
||||
) -> bool {
|
||||
self == Self::Cla
|
||||
&& matches!(scope, MigrationScope::Home)
|
||||
&& (name == ".trash" || (mode == SkillImportMode::LocalOnly && name == "synced"))
|
||||
}
|
||||
|
||||
pub(super) fn excluded_skill_subdirectory(
|
||||
self,
|
||||
scope: &MigrationScope,
|
||||
root: &std::ffi::OsStr,
|
||||
) -> Option<&'static std::ffi::OsStr> {
|
||||
(self == Self::Cla && matches!(scope, MigrationScope::Home) && root == "synced")
|
||||
.then_some(std::ffi::OsStr::new(".trash"))
|
||||
}
|
||||
|
||||
pub(super) fn supports_memory(self) -> bool {
|
||||
match self {
|
||||
Self::Cla => true,
|
||||
|
||||
@@ -6,6 +6,7 @@ use crate::config_values::write_toml_file;
|
||||
use crate::memory_import;
|
||||
use crate::migration_source::ExternalAgentSource;
|
||||
use crate::migration_source::InstructionSourceGroup;
|
||||
use crate::migration_source::SkillImportMode;
|
||||
pub use crate::model::DetectedConnectorCandidate;
|
||||
pub use crate::model::DetectedConnectorSource;
|
||||
pub use crate::model::ExternalAgentConfigDetectOptions;
|
||||
@@ -74,6 +75,7 @@ pub struct ExternalAgentConfigService {
|
||||
pub(crate) auth_manager: Arc<AuthManager>,
|
||||
pub(crate) source: ExternalAgentSource,
|
||||
pub(crate) session_import_limits: ExternalAgentSessionImportLimits,
|
||||
pub(super) skill_import_mode: SkillImportMode,
|
||||
state_db: Option<StateDbHandle>,
|
||||
pub(crate) memory_version: MemoryVersion,
|
||||
}
|
||||
@@ -95,6 +97,7 @@ impl ExternalAgentConfigService {
|
||||
analytics_events_client: Some(analytics_events_client),
|
||||
auth_manager,
|
||||
source,
|
||||
skill_import_mode: SkillImportMode::All,
|
||||
session_import_limits: ExternalAgentSessionImportLimits::default(),
|
||||
state_db,
|
||||
memory_version: MemoryVersion::V1,
|
||||
@@ -102,7 +105,8 @@ impl ExternalAgentConfigService {
|
||||
}
|
||||
|
||||
pub fn with_migration_source(&self, migration_source: Option<&str>) -> Self {
|
||||
let source = ExternalAgentSource::from_migration_source(migration_source);
|
||||
let (source, skill_import_mode) =
|
||||
ExternalAgentSource::from_migration_source(migration_source);
|
||||
let external_agent_home = default_external_agent_home(source);
|
||||
let connector_metadata_roots = source.connector_metadata_roots(&external_agent_home);
|
||||
Self {
|
||||
@@ -112,6 +116,7 @@ impl ExternalAgentConfigService {
|
||||
analytics_events_client: self.analytics_events_client.clone(),
|
||||
auth_manager: Arc::clone(&self.auth_manager),
|
||||
source,
|
||||
skill_import_mode,
|
||||
session_import_limits: self.session_import_limits,
|
||||
state_db: self.state_db.clone(),
|
||||
memory_version: self.memory_version,
|
||||
@@ -177,6 +182,7 @@ impl ExternalAgentConfigService {
|
||||
/*auth*/ None,
|
||||
),
|
||||
source,
|
||||
skill_import_mode: SkillImportMode::All,
|
||||
session_import_limits: ExternalAgentSessionImportLimits::default(),
|
||||
state_db: None,
|
||||
memory_version: MemoryVersion::V1,
|
||||
@@ -716,7 +722,7 @@ impl ExternalAgentConfigService {
|
||||
return Ok(Vec::new());
|
||||
};
|
||||
let source_skills_dir_names = self.source.skills_dir_names(&scope);
|
||||
let (source_config_dir, target_skills) = match scope {
|
||||
let (source_config_dir, target_skills) = match &scope {
|
||||
MigrationScope::Home => (
|
||||
self.external_agent_home.clone(),
|
||||
self.home_target_skills_dir(),
|
||||
@@ -742,7 +748,13 @@ impl ExternalAgentConfigService {
|
||||
for entry in fs::read_dir(&source_skills_dir)? {
|
||||
let entry = entry?;
|
||||
let file_type = entry.file_type()?;
|
||||
if !file_type.is_dir() {
|
||||
if !file_type.is_dir()
|
||||
|| self.source.is_reserved_skill_directory(
|
||||
&scope,
|
||||
&entry.file_name(),
|
||||
self.skill_import_mode,
|
||||
)
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -751,7 +763,13 @@ impl ExternalAgentConfigService {
|
||||
continue;
|
||||
}
|
||||
|
||||
copy_dir_recursive(&entry.path(), &target, self.source.rewrite_profile())?;
|
||||
copy_dir_recursive(
|
||||
&entry.path(),
|
||||
&target,
|
||||
self.source.rewrite_profile(),
|
||||
self.source
|
||||
.excluded_skill_subdirectory(&scope, &entry.file_name()),
|
||||
)?;
|
||||
copied_names.push(entry.file_name().to_string_lossy().to_string());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -716,13 +716,35 @@ fn import_skills_returns_only_new_skill_directory_names() {
|
||||
.expect("create source a");
|
||||
fs::create_dir_all(external_agent_home.join("skills").join("skill-b"))
|
||||
.expect("create source b");
|
||||
for reserved_skill in [
|
||||
"synced/test-account/cloud-example",
|
||||
"synced/.trash/deleted-example",
|
||||
".trash/deleted-example",
|
||||
] {
|
||||
let directory = external_agent_home.join("skills").join(reserved_skill);
|
||||
fs::create_dir_all(&directory).expect("create fixture directory");
|
||||
fs::write(directory.join("SKILL.md"), "# Example").expect("write fixture skill");
|
||||
}
|
||||
fs::create_dir_all(agents_skills.join("skill-a")).expect("create existing target");
|
||||
|
||||
let copied_names = service_for_paths(external_agent_home, codex_home)
|
||||
.import_skills(/*cwd*/ None)
|
||||
.expect("import skills");
|
||||
|
||||
assert_eq!(copied_names, vec!["skill-b".to_string()]);
|
||||
let service = service_for_paths(external_agent_home, codex_home);
|
||||
let mut split = service.with_migration_source(Some("claude-code-local-skills"));
|
||||
split.external_agent_home = service.external_agent_home.clone();
|
||||
assert_eq!(
|
||||
split.import_skills(/*cwd*/ None).expect("split import"),
|
||||
vec!["skill-b"]
|
||||
);
|
||||
assert_eq!(
|
||||
service.import_skills(/*cwd*/ None).expect("legacy import"),
|
||||
vec!["synced"]
|
||||
);
|
||||
assert!(!agents_skills.join(".trash").exists());
|
||||
assert!(!agents_skills.join("synced/.trash").exists());
|
||||
assert!(
|
||||
agents_skills
|
||||
.join("synced/test-account/cloud-example/SKILL.md")
|
||||
.is_file()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -45,6 +45,7 @@ pub(super) fn copy_dir_recursive(
|
||||
source: &Path,
|
||||
target: &Path,
|
||||
rewrite_profile: RewriteProfile,
|
||||
excluded_directory: Option<&std::ffi::OsStr>,
|
||||
) -> io::Result<()> {
|
||||
fs::create_dir_all(target)?;
|
||||
|
||||
@@ -55,7 +56,15 @@ pub(super) fn copy_dir_recursive(
|
||||
let file_type = entry.file_type()?;
|
||||
|
||||
if file_type.is_dir() {
|
||||
copy_dir_recursive(&source_path, &target_path, rewrite_profile)?;
|
||||
if excluded_directory == Some(entry.file_name().as_os_str()) {
|
||||
continue;
|
||||
}
|
||||
copy_dir_recursive(
|
||||
&source_path,
|
||||
&target_path,
|
||||
rewrite_profile,
|
||||
excluded_directory,
|
||||
)?;
|
||||
continue;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user