Skip to content

fix(tools): exclude nested skill docs from strategy-3 flat search - #43342

Closed
cjakma wants to merge 1 commit into
NousResearch:mainfrom
cjakma:fix/tools-skill-view-nested-dirs
Closed

fix(tools): exclude nested skill docs from strategy-3 flat search#43342
cjakma wants to merge 1 commit into
NousResearch:mainfrom
cjakma:fix/tools-skill-view-nested-dirs

Conversation

@cjakma

@cjakma cjakma commented Jun 10, 2026

Copy link
Copy Markdown

What changed and why

skill_view strategy-3 uses rglob(f"{name}.md") to find legacy flat skill docs. This was also picking up reference/template .md files that live inside another skill's directory (e.g. skills/foundations/runtime/references/explore-codebase.md) and returning them as match candidates for an unrelated skill name.

Added an ancestor-walk: if any directory between the found file and the search root contains a SKILL.md, the file belongs to that enclosing skill and is excluded from strategy-3 results.

How to test

# Create a reference file inside an existing skill that happens to share a name:
mkdir -p ~/.hermes/skills/some-skill/references/
echo "# doc" > ~/.hermes/skills/some-skill/references/other-skill.md
hermes -q "view skill other-skill"
# Should NOT return the reference file from inside some-skill/

Platforms tested

  • Linux (Ubuntu 22.04)

Changes

  • tools/skills_tool.py: strategy-3 loop now skips files whose ancestor chain contains a SKILL.md

skill_view strategy-3 (rglob) was picking up reference/template .md files
nested inside another skill's SKILL.md directory and returning them as
candidates for a different skill name.  Add an ancestor-walk that skips
any file whose parent chain contains a SKILL.md between it and the search
root — those are assets of the enclosing skill, not standalone skills.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have tool/skills Skills system (list, view, manage) labels Jun 10, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused fix. Current main already provides this behavior.

  • tools/skills_tool.py:1172-1180 excludes support documents from strategy-3 legacy-flat discovery via _is_skill_support_path.
  • tests/tools/test_skills_tool.py:1228-1261 covers a nested references/styles/sketch.md and verifies skill_view("sketch") resolves the real skill instead.
  • This supersedes the behavior proposed by this PR; the current implementation is present in commit d682f320b35a13084371a541a835e1d988c982b8.

Automated hermes-sweeper review.

@teknium1 teknium1 closed this Jul 14, 2026
@teknium1 teknium1 added sweeper:implemented-on-main Sweeper: behavior already present on current main sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:implemented-on-main Sweeper: behavior already present on current main sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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