openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-04 15:08:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

Validate external agent skills before import

Browse Source
This commit is contained in:
charlesgong-openai
2026-06-17 13:53:18 -07:00
parent 49614a0391
commit 0332a6dd4c
2 changed files with 111 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

57
codex-rs/app-server/src/config/external_agent_config.rs
View File

@@ -1,6 +1,8 @@
use codex_config::types::PluginConfig;
use codex_core::config::Config;
use codex_core::config::ConfigBuilder;
use codex_core::skills::loader::SkillRoot;
use codex_core::skills::loader::load_skills_from_roots;
use codex_core_plugins::PluginInstallRequest;
use codex_core_plugins::PluginsManager;
use codex_core_plugins::marketplace::MarketplacePluginInstallPolicy;
@@ -8,6 +10,7 @@ use codex_core_plugins::marketplace::find_marketplace_manifest_path;
use codex_core_plugins::marketplace_add::MarketplaceAddRequest;
use codex_core_plugins::marketplace_add::add_marketplace;
use codex_core_plugins::marketplace_add::is_local_marketplace_source;
use codex_exec_server::LOCAL_FS;
use codex_external_agent_migration::build_mcp_config_from_external;
use codex_external_agent_migration::count_missing_commands;
use codex_external_agent_migration::count_missing_subagents;
@@ -21,6 +24,8 @@ use codex_external_agent_sessions::ExternalAgentSessionMigration;
use codex_external_agent_sessions::detect_recent_sessions;
use codex_plugin::PluginId;
use codex_protocol::protocol::Product;
use codex_protocol::protocol::SkillScope;
use codex_utils_absolute_path::AbsolutePathBuf;
use serde_json::Value as JsonValue;
use std::collections::BTreeMap;
use std::collections::HashMap;
@@ -30,6 +35,7 @@ use std::fs;
use std::io;
use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;
use toml::Value as TomlValue;
const EXTERNAL_AGENT_CONFIG_DETECT_METRIC: &str = "codex.external_agent_config.detect";
@@ -262,18 +268,19 @@ impl ExternalAgentConfigService {
);
Ok(())
})(),
ExternalAgentConfigMigrationItemType::Skills => (|| {
let imported_skills = self.import_skills(migration_item.cwd.as_deref())?;
emit_migration_metric(
EXTERNAL_AGENT_CONFIG_IMPORT_METRIC,
ExternalAgentConfigMigrationItemType::Skills,
Some(imported_skills.len()),
);
for skill_name in imported_skills {
item_result.record_success(Some(skill_name.clone()), Some(skill_name));
}
Ok(())
})(),
ExternalAgentConfigMigrationItemType::Skills => self
.import_skills(migration_item.cwd.as_deref())
.await
.map(|imported_skills| {
emit_migration_metric(
EXTERNAL_AGENT_CONFIG_IMPORT_METRIC,
ExternalAgentConfigMigrationItemType::Skills,
Some(imported_skills.len()),
);
for skill_name in imported_skills {
item_result.record_success(Some(skill_name.clone()), Some(skill_name));
}
}),
ExternalAgentConfigMigrationItemType::AgentsMd => (|| {
if let Some((source, target)) =
self.import_agents_md(migration_item.cwd.as_deref())?
@@ -1149,7 +1156,7 @@ impl ExternalAgentConfigService {
import_commands(&source_commands, &target_skills)
}
fn import_skills(&self, cwd: Option<&Path>) -> io::Result<Vec<String>> {
async fn import_skills(&self, cwd: Option<&Path>) -> io::Result<Vec<String>> {
let (source_skills, target_skills) = if let Some(repo_root) = find_repo_root(cwd)? {
(
repo_root.join(EXTERNAL_AGENT_DIR).join("skills"),
@@ -1166,6 +1173,30 @@ impl ExternalAgentConfigService {
if !source_skills.is_dir() {
return Ok(Vec::new());
}
let source_skills_root = AbsolutePathBuf::from_absolute_path_checked(&source_skills)
.map_err(|err| {
invalid_data_error(format!(
"invalid skills root {}: {err}",
source_skills.display()
))
})?;
let skill_load_outcome = load_skills_from_roots([SkillRoot {
path: source_skills_root,
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_root: None,
}])
.await;
if !skill_load_outcome.errors.is_empty() {
let details = skill_load_outcome
.errors
.iter()
.map(|error| format!("{}: {}", error.path.display(), error.message))
.collect::<Vec<_>>()
.join("; ");
return Err(invalid_data_error(format!("invalid skills: {details}")));
}
fs::create_dir_all(&target_skills)?;
let mut copied_names = Vec::new();

75
codex-rs/app-server/src/config/external_agent_config_tests.rs
View File

@@ -805,7 +805,7 @@ async fn import_home_migrates_supported_config_fields_skills_and_agents_md() {
.join("skill-a")
.join("SKILL.md"),
format!(
"Use {SOURCE_EXTERNAL_AGENT_PRODUCT_NAME} and {SOURCE_EXTERNAL_AGENT_UPPER_NAME} utilities."
"---\nname: skill-a\ndescription: Source skill\n---\nUse {SOURCE_EXTERNAL_AGENT_PRODUCT_NAME} and {SOURCE_EXTERNAL_AGENT_UPPER_NAME} utilities.\n"
),
)
.expect("write skill");
@@ -865,7 +865,7 @@ MY_TEAM = "codex"
assert_eq!(
fs::read_to_string(agents_skills.join("skill-a").join("SKILL.md"))
.expect("read copied skill"),
"Use Codex and Codex utilities."
"---\nname: skill-a\ndescription: Source skill\n---\nUse Codex and Codex utilities.\n"
);
}
@@ -2769,22 +2769,81 @@ async fn import_plugins_supports_project_relative_external_agent_plugin_marketpl
assert!(config.contains("enabled = true"));
}
#[test]
fn import_skills_returns_only_new_skill_directory_names() {
#[tokio::test]
async fn import_skills_returns_only_new_skill_directory_names() {
let (_root, external_agent_home, codex_home) = fixture_paths();
let agents_skills = codex_home
.parent()
.map(|parent| parent.join(".agents").join("skills"))
.unwrap_or_else(|| PathBuf::from(".agents").join("skills"));
fs::create_dir_all(external_agent_home.join("skills").join("skill-a"))
.expect("create source a");
fs::create_dir_all(external_agent_home.join("skills").join("skill-b"))
.expect("create source b");
let source_skill_a = external_agent_home.join("skills").join("skill-a");
let source_skill_b = external_agent_home.join("skills").join("skill-b");
fs::create_dir_all(&source_skill_a).expect("create source a");
fs::create_dir_all(&source_skill_b).expect("create source b");
fs::write(
source_skill_a.join("SKILL.md"),
"---\nname: skill-a\ndescription: source a\n---\n",
)
.expect("write source a skill");
fs::write(
source_skill_b.join("SKILL.md"),
"---\nname: skill-b\ndescription: source b\n---\n",
)
.expect("write source b 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)
.await
.expect("import skills");
assert_eq!(copied_names, vec!["skill-b".to_string()]);
}
#[tokio::test]
async fn import_skills_rejects_invalid_skill_without_copying() {
let (_root, external_agent_home, codex_home) = fixture_paths();
let source_skill = external_agent_home.join("skills").join("broken-skill");
let agents_skills = codex_home
.parent()
.map(|parent| parent.join(".agents").join("skills"))
.unwrap_or_else(|| PathBuf::from(".agents").join("skills"));
fs::create_dir_all(&source_skill).expect("create source skill");
fs::write(source_skill.join("SKILL.md"), "missing frontmatter\n").expect("write invalid skill");
let outcome = service_for_paths(external_agent_home, codex_home)
.import(vec![ExternalAgentConfigMigrationItem {
item_type: ExternalAgentConfigMigrationItemType::Skills,
description: "Import skills".to_string(),
cwd: None,
details: None,
}])
.await;
assert_eq!(outcome.pending_plugin_imports, Vec::new());
let item_result = outcome
.item_results
.into_iter()
.next()
.expect("skills import result");
assert_eq!(
item_result.item_type,
ExternalAgentConfigMigrationItemType::Skills
);
assert_eq!(item_result.success_count, 0);
assert_eq!(item_result.error_count, 1);
assert_eq!(item_result.successes, Vec::new());
assert_eq!(item_result.raw_errors.len(), 1);
let raw_error = &item_result.raw_errors[0];
assert_eq!(raw_error.failure_stage, "import_request_failed");
assert_eq!(
raw_error.error_type.as_deref(),
Some("external_agent_config_import_error")
);
assert!(
raw_error
.message
.contains("missing YAML frontmatter delimited by ---")
);
assert!(!agents_skills.join("broken-skill").exists());
}