fix(skills): resolve skill_view by frontmatter name when dir name differs - #30906
fix(skills): resolve skill_view by frontmatter name when dir name differs#30906vinsew wants to merge 1 commit into
Conversation
|
Duplicate fix — this is the same frontmatter-name fallback addressed by the existing chain:
Please coordinate with the authors of #18901 and #28305 to consolidate into a single PR. |
Morad37
left a comment
There was a problem hiding this comment.
Nice fix. The frontmatter-name fallback is exactly what's needed for the curator-renamed-directory case -- I've hit this myself where skills listed one name but skill_view couldn't find it.
Particularly like the edge case coverage:
- Ambiguous resolution refusal (two skills with same frontmatter name but different dirs) -- avoids silent wrong answers
- Platform filter gates the fallback -- consistent with listings behaviour
- Dir-name takes precedence so existing path lookups don't break
The test structure is well organised. The resolves_by_frontmatter_name_when_dir_differs test is the core regression case and the setup is clean -- temp dir with isolated SKILL.md, monkeypatch the SKILLS_DIR. Good testing pattern.
One question out of curiosity: does the frontmatter-name fallback also handle the case where the frontmatter name is defined but empty (e.g. someone writes name: with no value)? Might be a YAML parser edge case worth a guard, though probably out of scope for this PR.
e8706fe to
98cc70f
Compare
98cc70f to
864f537
Compare
…fers 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) <noreply@anthropic.com>
864f537 to
7f45e35
Compare
|
Closing as superseded by the implementation now on The current upstream code and regression coverage preserve the intended frontmatter-name fallback, so keeping this older branch open would duplicate the same fix. |
Problem
skill_view(name)resolves a skill by matchingnameagainst the on-disk directory name / path (Strategies 1-3). But skills are listed and displayed by their frontmatternamefield (_find_all_skills,skills_tool.py—frontmatter.get("name", skill_dir.name)).When a skill's directory name differs from its frontmatter
name, the name an agent sees in listings is exactly the oneskill_viewcannot resolve:name: 我的笔记(frontmatter)productivity/getnoteskill_view("我的笔记")→Skill '我的笔记' not foundThe not-found payload then lists
available_skillsusing the same frontmatter display names (truncated to the first 20), so the agent has no way to discover the dir/path form it would need. The skill becomes effectively uncallable by the only name it is shown under. This affects any skill whose directory name and frontmatter name diverge (e.g. a skill whose directory was renamed/normalized on disk while keeping a human-facing display name).Fix
Add a Strategy 4 to
skill_view: when none of the path/dir strategies match, fall back to matching the skill's frontmattername.Constraints kept to make this safe and backward-compatible:
candidatesis empty, so directory/path matches always win and existing resolution is unchanged.skill_matches_platformcheck_find_all_skillsuses, so a skill the agent never sees in listings can't resolve to a misleading "unsupported platform" error.str(name)[:MAX_NAME_LENGTH]) so what the agent sees is what it can call.Tests
tests/test_plugin_skills.py:Known tradeoff
_find_all_skillsdedupes display names by first-seen, while Strategy 4 surfaces every frontmatter-name match. A name that looks unique in listings can therefore report ambiguity if two skills genuinely share it — intentional: refusing is safer than guessing which one the listing happened to show.