fix(skills): resolve source dir by SKILL.md anchor instead of name (#4153)

* fix(skills): resolve source dir by SKILL.md anchor instead of name

resolve_skill_source_dir previously guessed the source dir via root.join(name).is_dir() without verifying SKILL.md, misjudging same-name non-skill dirs (e.g. the ast-grep plugin wrapper dir in ast-grep/agent-skill) and causing install failure #4141.

Now anchors on SKILL.md: direct + SKILL.md check -> root manifest explicit skills[] -> fallback by name -> root fallback. Adds 5 layout tests.

Closes #4141

* fix(skills): drop speculative manifest resolver path

resolve_via_manifest (parsing root .claude-plugin/marketplace.json &
plugin.json explicit skills[]) is inert for the actual #4141 case: the
real ast-grep/agent-skill marketplace.json declares no skills[] array,
so the manifest branch never produces a candidate. The #4141 fix is
delivered entirely by resolve_skill_source_dir step 1's SKILL.md anchor
plus the pre-existing find_skill_dir_by_name DFS.

Keeping the manifest path would pull npx-skills package-parity semantics
(pluginRoot / source / remote-object source / skills[] / "./"-validation
/ ...) into a bug hotfix, with no real manifest proving it is not dead
code. Drop it to keep this PR a focused #4141 hotfix.

- remove SkillMarketplaceMetadata / SkillManifestPlugin /
  SkillMarketplaceManifest, resolve_via_manifest, sanitize_manifest_path
- narrow resolve_skill_source_dir to 3 steps
  (direct+SKILL.md -> by-name DFS+SKILL.md -> root+SKILL.md -> None)
- replace the two synthetic manifest tests with a negative case:
  same-name wrapper dir without SKILL.md and no inner skill -> None

cargo test --lib resolve_skill_source_dir: 7 passed
cargo clippy --lib: clean
This commit is contained in:
makoMakoGo
2026-08-03 04:05:51 -07:00
committed by GitHub
parent f38722a440
commit eb356e15bd
+87 -10
View File
@@ -2374,31 +2374,38 @@ impl SkillService {
/// 将 discoverable skill 的目录信息重新解析为解压目录中的真实源目录。
///
/// 兼容三种情况
/// 1. `skills/foo` 这类直接相对路径
/// 2. 仅持有安装名 `foo`,需要在仓库中递归查找真实目录;
/// 3. 仓库根目录本身就是 skill,此时回退到解压根目录
/// **核心原则:返回的目录必定含 `SKILL.md`**(以 SKILL.md 为锚点)。解析顺序
/// 1. 直接相对路径命中(如 `skills/foo`),校验含 `SKILL.md`——明确路径优先
/// 2. 按安装名递归查找名字匹配 **且** 含 `SKILL.md` 的目录;
/// 3. 兜底:仓库根本身含 `SKILL.md`
fn resolve_skill_source_dir(root: &Path, raw_directory: &str) -> Option<PathBuf> {
let source_rel = Self::sanitize_skill_source_path(raw_directory)?;
let install_name = source_rel
.file_name()
.map(|n| n.to_string_lossy().to_string())?;
// 1. 直接相对路径命中(明确路径优先)——必须校验 SKILL.md,否则同名空壳目录
// (如 ast-grep/agent-skill 根下的 plugin 包目录 ast-grep/)会被误判为源目录。
let direct = root.join(&source_rel);
if direct.is_dir() {
if direct.is_dir() && direct.join("SKILL.md").is_file() {
return Some(direct);
}
let target_name = source_rel.file_name()?.to_string_lossy().to_string();
if let Some(found) = Self::find_skill_dir_by_name(root, &target_name) {
// 2. 按名字递归查找(find_skill_dir_by_name 已校验 SKILL.md
if let Some(found) = Self::find_skill_dir_by_name(root, &install_name) {
log::info!(
"Skill directory '{}' not found at direct path, using fallback: {}",
target_name,
install_name,
found.display()
);
return Some(found);
}
if root.is_dir() && root.join("SKILL.md").exists() {
// 3. 兜底:仓库根本身是 skill
if root.join("SKILL.md").is_file() {
log::info!(
"Skill directory '{}' not found, but SKILL.md exists at root, using repo root",
target_name,
install_name,
);
return Some(root.to_path_buf());
}
@@ -4452,4 +4459,74 @@ mod tests {
"existing destination skill should be preserved"
);
}
#[test]
fn resolve_skill_source_dir_rejects_same_name_wrapper_without_skill_md() {
// 复刻 issue #4141ast-grep/agent-skill 结构。仓库根下有同名目录 ast-grep/
// plugin 包,无 SKILL.md),真正的 skill 在 ast-grep/skills/ast-grep/SKILL.md。
let temp = tempdir().expect("tempdir");
let wrapper = temp.path().join("ast-grep");
fs::create_dir_all(wrapper.join(".claude-plugin")).expect("create wrapper plugin dir");
fs::write(
wrapper.join(".claude-plugin").join("plugin.json"),
"{\"name\":\"ast-grep\"}",
)
.expect("write plugin.json");
let real_skill = wrapper.join("skills").join("ast-grep");
write_skill(&real_skill, "ast-grep");
// directory 只给了 skill 名 "ast-grep"skills.sh API 的语义),不能命中空壳 wrapper。
let resolved = SkillService::resolve_skill_source_dir(temp.path(), "ast-grep")
.expect("should resolve to the inner skill dir, not the same-name wrapper");
assert_eq!(resolved, real_skill);
assert!(resolved.join("SKILL.md").is_file());
}
#[test]
fn resolve_skill_source_dir_finds_two_level_catalog_skill() {
// catalog layoutskills/category/foo/SKILL.mddepth 3find_skill_dir_by_name 可达)。
let temp = tempdir().expect("tempdir");
let catalog_skill = temp.path().join("skills").join("category").join("foo");
write_skill(&catalog_skill, "Foo Skill");
let resolved = SkillService::resolve_skill_source_dir(temp.path(), "foo")
.expect("should resolve the two-level catalog skill by name");
assert_eq!(resolved, catalog_skill);
}
#[test]
fn resolve_skill_source_dir_returns_none_for_wrapper_without_inner_skill() {
// 同名 wrapper 存在、无 SKILL.md,且无 inner skill / root SKILL.md 可兜底时,
// 必须返回 None——守住 #4141 这个 bug class 的负例(不能把空壳目录当源目录)。
let temp = tempdir().expect("tempdir");
let wrapper = temp.path().join("ast-grep");
fs::create_dir_all(wrapper.join(".claude-plugin")).expect("create wrapper plugin dir");
fs::write(
wrapper.join(".claude-plugin").join("plugin.json"),
"{\"name\":\"ast-grep\"}",
)
.expect("write plugin.json");
let resolved = SkillService::resolve_skill_source_dir(temp.path(), "ast-grep");
assert!(
resolved.is_none(),
"wrapper dir without SKILL.md and no inner skill must resolve to None, got {:?}",
resolved
);
}
#[test]
fn resolve_skill_source_dir_returns_none_when_no_skill_md_anywhere() {
let temp = tempdir().expect("tempdir");
fs::create_dir_all(temp.path().join("skills").join("foo")).expect("create empty skill dir");
fs::write(temp.path().join("README.md"), "no skills here").expect("write README");
let resolved = SkillService::resolve_skill_source_dir(temp.path(), "foo");
assert!(
resolved.is_none(),
"no SKILL.md anywhere must resolve to None"
);
}
}