Skip to content

fix(skill_view): exclude skill-internal subdirs from Strategy 3 legacy file matching - #39677

Closed
111-test-111 wants to merge 1 commit into
NousResearch:mainfrom
111-test-111:fix/strategy3-exclude-skill-subdirs
Closed

fix(skill_view): exclude skill-internal subdirs from Strategy 3 legacy file matching#39677
111-test-111 wants to merge 1 commit into
NousResearch:mainfrom
111-test-111:fix/strategy3-exclude-skill-subdirs

Conversation

@111-test-111

Copy link
Copy Markdown

Problem

Strategy 3 in uses to find legacy flat files anywhere under the skills directory. This incorrectly matches files inside skill-internal subdirectories like references/, templates/, scripts/, assets/.

Real-world example: A user has both:

  • mcp/native-mcp/SKILL.md — a standalone MCP skill
  • autonomous-ai-agents/hermes-agent/references/native-mcp.md — a reference doc inside the hermes-agent skill

Calling skill_view("native-mcp") finds both via Strategy 3's rglob and throws an Ambiguous skill name error, even though the reference doc is clearly not a standalone skill.

Fix

Add a check in Strategy 3 to skip any file whose path contains a skill-internal subdirectory component (references, templates, scripts, assets). These directories are reserved for supporting documents within existing skills and should never contain standalone legacy skills.

Tests

  • Regression test reproducing the exact native-mcp collision scenario
  • Parametrized test covering all four excluded subdirectory names
  • All 11 collision detection tests pass

…y file matching

Strategy 3 uses rglob to find legacy flat <name>.md files anywhere under
the skills directory. This incorrectly matched files inside skill-internal
subdirectories (references/, templates/, scripts/, assets/) — e.g., a
skill's references/native-mcp.md was treated as a standalone legacy skill,
causing a false collision with the real mcp/native-mcp skill.

Fix: skip any file whose path contains a skill-internal subdirectory
component. These directories are reserved for supporting documents within
existing skills and should never contain standalone legacy skills.

Includes regression test reproducing the original native-mcp collision
and parametrized tests for all four excluded subdirectory names.
@alt-glitch alt-glitch added type/bug Something isn't working tool/skills Skills system (list, view, manage) P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists labels Jun 5, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to the #35743 skill_view collision cluster — same root cause (Strategy 3 rglob matching .md files inside other skills' references//templates//scripts//assets/ dirs). Overlaps open fix PRs #35768 and #27915. Consolidating approaches.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression report and tests. This is already implemented on current main by a broader follow-up to the skill-view collision cluster noted in the discussion.

  • Automated hermes-sweeper review verified that tools/skills_tool.py:1172-1180 excludes support documents from Strategy 3 legacy-file candidate matching.
  • The shared predicate in agent/skill_utils.py:73-97 distinguishes actual skill support directories from valid top-level categories named scripts, templates, assets, or references.
  • Commit 9137b86a5286e4ea420e9fb894bce34fee546b3d (fix(skills): ignore support docs in skill discovery) shipped this behavior and is included in v2026.6.19.
  • Current regression coverage at tests/tools/test_skills_tool.py:1240-1261 verifies that a nested reference document does not collide with a real skill.

@teknium1 teknium1 closed this Jul 14, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists sweeper:implemented-on-main Sweeper: behavior already present on current main tool/skills Skills system (list, view, manage) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants