From 7f45e35efeb198596d9963c0987b45b47b17a8c0 Mon Sep 17 00:00:00 2001 From: vinsew Date: Sat, 23 May 2026 19:06:44 +0800 Subject: [PATCH] fix(skills): resolve skill_view by frontmatter name when dir name differs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit skill_view matched only on directory/path (Strategies 1-3), but skills are listed and displayed by their frontmatter `name` (see _find_all_skills). When a skill's on-disk directory name differs from its frontmatter name — e.g. a skill renamed on disk while keeping its display name — the name the agent sees in listings became impossible to load: skill_view always returned "not found", with the not-found hint echoing the same unusable name. Add Strategy 4: when no path strategy matches, fall back to matching the frontmatter `name`. Path/dir matches still take precedence (only consulted when candidates is empty), platform-mismatched skills are filtered to stay consistent with listings, and multiple same-name matches fall through to the existing collision guard rather than silently guessing. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/test_plugin_skills.py | 80 +++++++++++++++++++++++++++++++++++++ tools/skills_tool.py | 31 ++++++++++++++ 2 files changed, 111 insertions(+) diff --git a/tests/test_plugin_skills.py b/tests/test_plugin_skills.py index d528b99b5ad45..005f61a757b4c 100644 --- a/tests/test_plugin_skills.py +++ b/tests/test_plugin_skills.py @@ -266,6 +266,86 @@ def test_stale_entry_self_heals(self, tmp_path): assert "no longer exists" in result["error"] assert self.pm.find_plugin_skill("superpowers:writing-plans") is None + def test_resolves_by_frontmatter_name_when_dir_differs(self, tmp_path, monkeypatch): + """A skill whose frontmatter `name` differs from its directory name must + still resolve when called by the name shown in skills listings. + + Regression: listings display `frontmatter.name` (e.g. a Chinese name), + but skill_view only matched by directory/path. Skills whose dir was + renamed (e.g. by the curator) became impossible to load by their + displayed name. + """ + from tools.skills_tool import skill_view + + local = tmp_path / "local-skills" + skill_dir = local / "productivity" / "getnote" + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text( + "---\nname: Get笔记\ndescription: note keeper\n---\nGetNote body.\n" + ) + monkeypatch.setattr("tools.skills_tool.SKILLS_DIR", local) + + result = json.loads(skill_view("Get笔记")) + assert result["success"] is True + assert "GetNote body." in result["content"] + + def test_dir_name_takes_precedence_over_frontmatter_name(self, tmp_path, monkeypatch): + """Path/dir-name matching wins; the frontmatter-name fallback only fires + when no path strategy matched (must not change existing behavior).""" + from tools.skills_tool import skill_view + + local = tmp_path / "local-skills" + # Skill whose DIRECTORY name is "alpha". + d1 = local / "alpha" + d1.mkdir(parents=True) + (d1 / "SKILL.md").write_text("---\nname: alpha-display\n---\nAlpha by dir.\n") + # Different skill whose FRONTMATTER name is "alpha" (dir is "beta"). + d2 = local / "beta" + d2.mkdir(parents=True) + (d2 / "SKILL.md").write_text("---\nname: alpha\n---\nBeta by frontmatter.\n") + monkeypatch.setattr("tools.skills_tool.SKILLS_DIR", local) + + result = json.loads(skill_view("alpha")) + assert result["success"] is True + assert "Alpha by dir." in result["content"] + + def test_ambiguous_frontmatter_name_refuses(self, tmp_path, monkeypatch): + """Two skills sharing the same frontmatter name (no path match) must + refuse rather than silently guessing.""" + from tools.skills_tool import skill_view + + local = tmp_path / "local-skills" + for d in ("note-a", "note-b"): + sd = local / d + sd.mkdir(parents=True) + (sd / "SKILL.md").write_text(f"---\nname: 笔记\n---\n{d} body.\n") + monkeypatch.setattr("tools.skills_tool.SKILLS_DIR", local) + + result = json.loads(skill_view("笔记")) + assert result["success"] is False + assert "ambiguous" in result["error"].lower() + + def test_frontmatter_name_fallback_respects_platform(self, tmp_path, monkeypatch): + """A skill hidden from listings by a platform mismatch must not be + resolvable via the frontmatter-name fallback either. Listings filter + by platform, so the fallback must too — otherwise skill_view reports + 'unsupported platform' for a name the agent never sees in listings.""" + import sys + from tools.skills_tool import skill_view + + other = "linux" if sys.platform.startswith("darwin") else "macos" + local = tmp_path / "local-skills" + skill_dir = local / "platform-only" + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text( + f"---\nname: 平台限定\nplatforms: [{other}]\n---\nBody.\n" + ) + monkeypatch.setattr("tools.skills_tool.SKILLS_DIR", local) + + result = json.loads(skill_view("平台限定")) + assert result["success"] is False + assert "not found" in result["error"].lower() + class TestSkillViewPluginGuards: @pytest.fixture(autouse=True) diff --git a/tools/skills_tool.py b/tools/skills_tool.py index 04bf35abe69d9..5c28bb580e52e 100644 --- a/tools/skills_tool.py +++ b/tools/skills_tool.py @@ -986,6 +986,37 @@ def _record(sd: Optional[Path], smd: Path) -> None: if found_md.name != "SKILL.md": _record(None, found_md) + # Strategy 4: fall back to the skill's frontmatter `name`. Skills are + # listed and displayed by `frontmatter.name` (see _find_all_skills), + # but Strategies 1-3 only match on directory/path. When a skill's + # on-disk directory name differs from its frontmatter name (e.g. the + # directory was renamed while the display name was kept), it is + # otherwise impossible to load by the name the agent actually sees in + # listings. Only consulted when no path strategy matched, so path/dir + # matches always take precedence; multiple frontmatter-name matches + # fall through to the collision guard below. (_find_all_skills dedupes + # display names by first-seen, so surfacing every match here means a + # name that looks unique in listings can still report ambiguity — that + # is intentional: refuse over silently guessing which skill was meant.) + if not candidates: + for search_dir in all_dirs: + for found_skill_md in iter_skill_index_files(search_dir, "SKILL.md"): + try: + fm, _ = _parse_frontmatter( + found_skill_md.read_text(encoding="utf-8")[:4000] + ) + except Exception: + continue + # Mirror listing semantics: skills hidden from listings by a + # platform mismatch must stay unresolvable here too, so a + # name the agent never sees can't resolve to an "unsupported + # platform" error. + if not skill_matches_platform(fm): + continue + fm_name = fm.get("name") + if fm_name and str(fm_name)[:MAX_NAME_LENGTH] == name: + _record(found_skill_md.parent, found_skill_md) + if len(candidates) > 1: paths = [str(smd) for _, smd in candidates] logging.getLogger(__name__).warning(