fix(runtime): identify bundled skill entries by path
This commit is contained in:
@@ -961,9 +961,8 @@ async fn list_skills(
|
||||
State(state): State<RuntimeApiState>,
|
||||
) -> Result<Json<SkillsResponse>, ApiError> {
|
||||
let skills_dir = resolve_skills_dir(&state.config, &state.workspace);
|
||||
let registry = crate::skills::discover_for_workspace_and_dir(&state.workspace, &skills_dir);
|
||||
let (registry, directories) = discover_skills_for_runtime_api(&state.workspace, &skills_dir);
|
||||
let skill_state = state.skill_state.lock().await;
|
||||
let directories = skills_search_directories(&state.workspace, &skills_dir);
|
||||
let skills = registry
|
||||
.list()
|
||||
.iter()
|
||||
@@ -972,7 +971,7 @@ async fn list_skills(
|
||||
description: skill.description.clone(),
|
||||
path: skill.path.clone(),
|
||||
enabled: skill_state.is_enabled(&skill.name),
|
||||
is_bundled: crate::skills::is_bundled_skill_name(&skill.name),
|
||||
is_bundled: skill_entry_is_bundled(skill, &skills_dir),
|
||||
})
|
||||
.collect();
|
||||
Ok(Json(SkillsResponse {
|
||||
@@ -989,12 +988,12 @@ async fn set_skill_enabled(
|
||||
Json(req): Json<SetSkillEnabledRequest>,
|
||||
) -> Result<Json<SetSkillEnabledResponse>, ApiError> {
|
||||
let skills_dir = resolve_skills_dir(&state.config, &state.workspace);
|
||||
let registry = crate::skills::discover_for_workspace_and_dir(&state.workspace, &skills_dir);
|
||||
let (registry, directories) = discover_skills_for_runtime_api(&state.workspace, &skills_dir);
|
||||
let exists = registry.list().iter().any(|skill| skill.name == name);
|
||||
if !exists {
|
||||
return Err(ApiError::not_found(format!(
|
||||
"skill '{name}' not found in searched directories: {}",
|
||||
format_skill_search_paths(&skills_search_directories(&state.workspace, &skills_dir))
|
||||
format_skill_search_paths(&directories)
|
||||
)));
|
||||
}
|
||||
|
||||
@@ -1778,6 +1777,31 @@ fn skills_search_directories(workspace: &FsPath, skills_dir: &FsPath) -> Vec<Pat
|
||||
directories
|
||||
}
|
||||
|
||||
fn discover_skills_for_runtime_api(
|
||||
workspace: &FsPath,
|
||||
skills_dir: &FsPath,
|
||||
) -> (crate::skills::SkillRegistry, Vec<PathBuf>) {
|
||||
let directories = skills_search_directories(workspace, skills_dir);
|
||||
let registry = crate::skills::discover_from_directories(directories.clone());
|
||||
(registry, directories)
|
||||
}
|
||||
|
||||
fn skill_entry_is_bundled(skill: &crate::skills::Skill, skills_dir: &FsPath) -> bool {
|
||||
if !crate::skills::is_bundled_skill_name(&skill.name) {
|
||||
return false;
|
||||
}
|
||||
|
||||
let expected_path = skills_dir.join(&skill.name).join("SKILL.md");
|
||||
paths_refer_to_same_file(&skill.path, &expected_path)
|
||||
}
|
||||
|
||||
fn paths_refer_to_same_file(left: &FsPath, right: &FsPath) -> bool {
|
||||
match (fs::canonicalize(left), fs::canonicalize(right)) {
|
||||
(Ok(left), Ok(right)) => left == right,
|
||||
_ => left == right,
|
||||
}
|
||||
}
|
||||
|
||||
fn format_skill_search_paths(directories: &[PathBuf]) -> String {
|
||||
if directories.is_empty() {
|
||||
return "<none>".to_string();
|
||||
@@ -3926,6 +3950,53 @@ mod tests {
|
||||
assert!(message.contains("custom-skills"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn skill_entry_is_bundled_requires_configured_bundle_path() {
|
||||
let tmp = tempfile::tempdir().expect("tempdir");
|
||||
let bundled_skills_dir = tmp.path().join("bundled-skills");
|
||||
let bundled_skill_path = bundled_skills_dir.join("delegate").join("SKILL.md");
|
||||
let override_skill_path = tmp
|
||||
.path()
|
||||
.join("workspace")
|
||||
.join(".agents")
|
||||
.join("skills")
|
||||
.join("delegate")
|
||||
.join("SKILL.md");
|
||||
fs::create_dir_all(bundled_skill_path.parent().expect("bundled parent"))
|
||||
.expect("create bundled skill dir");
|
||||
fs::create_dir_all(override_skill_path.parent().expect("override parent"))
|
||||
.expect("create override skill dir");
|
||||
fs::write(
|
||||
&bundled_skill_path,
|
||||
"---\nname: delegate\ndescription: bundled\n---\n",
|
||||
)
|
||||
.expect("write bundled skill");
|
||||
fs::write(
|
||||
&override_skill_path,
|
||||
"---\nname: delegate\ndescription: override\n---\n",
|
||||
)
|
||||
.expect("write override skill");
|
||||
|
||||
let bundled_skill = crate::skills::Skill {
|
||||
name: "delegate".to_string(),
|
||||
description: String::new(),
|
||||
body: String::new(),
|
||||
path: bundled_skill_path,
|
||||
};
|
||||
let override_skill = crate::skills::Skill {
|
||||
name: "delegate".to_string(),
|
||||
description: String::new(),
|
||||
body: String::new(),
|
||||
path: override_skill_path,
|
||||
};
|
||||
|
||||
assert!(skill_entry_is_bundled(&bundled_skill, &bundled_skills_dir));
|
||||
assert!(!skill_entry_is_bundled(
|
||||
&override_skill,
|
||||
&bundled_skills_dir
|
||||
));
|
||||
}
|
||||
|
||||
/// A `skills` symlink that points outside the workspace must NOT be
|
||||
/// returned as the resolved skills directory. Containment check ensures
|
||||
/// the canonicalized candidate stays under the canonicalized workspace
|
||||
|
||||
@@ -580,6 +580,10 @@ fn discover_for_workspace_dirs_and_dir(mut dirs: Vec<PathBuf>, skills_dir: &Path
|
||||
dirs.push(skills_dir.to_path_buf());
|
||||
}
|
||||
|
||||
discover_from_directories(dirs)
|
||||
}
|
||||
|
||||
pub(crate) fn discover_from_directories(dirs: impl IntoIterator<Item = PathBuf>) -> SkillRegistry {
|
||||
let mut merged = SkillRegistry::default();
|
||||
for dir in dirs {
|
||||
let registry = SkillRegistry::discover(&dir);
|
||||
|
||||
Reference in New Issue
Block a user