mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-28 01:21:43 +00:00
fix: use Path.parts for hidden directory filter in skill listing
The hidden directory filter used hardcoded forward-slash strings like '/.git/' and '/.hub/' to exclude internal directories. On Windows, Path returns backslash-separated strings, so the filter never matched. This caused quarantined skills in .hub/quarantine/ to appear as installed skills and available slash commands on Windows. Replaced string-based checks with Path.parts membership test which works on both Windows and Unix.
This commit is contained in:
parent
70a0a5ff4a
commit
f93b48226c
3 changed files with 97 additions and 4 deletions
|
|
@ -26,8 +26,7 @@ def scan_skill_commands() -> Dict[str, Dict[str, Any]]:
|
||||||
if not SKILLS_DIR.exists():
|
if not SKILLS_DIR.exists():
|
||||||
return _skill_commands
|
return _skill_commands
|
||||||
for skill_md in SKILLS_DIR.rglob("SKILL.md"):
|
for skill_md in SKILLS_DIR.rglob("SKILL.md"):
|
||||||
path_str = str(skill_md)
|
if any(part in ('.git', '.github', '.hub') for part in skill_md.parts):
|
||||||
if '/.git/' in path_str or '/.github/' in path_str or '/.hub/' in path_str:
|
|
||||||
continue
|
continue
|
||||||
try:
|
try:
|
||||||
content = skill_md.read_text(encoding='utf-8')
|
content = skill_md.read_text(encoding='utf-8')
|
||||||
|
|
|
||||||
95
tests/tools/test_hidden_dir_filter.py
Normal file
95
tests/tools/test_hidden_dir_filter.py
Normal file
|
|
@ -0,0 +1,95 @@
|
||||||
|
"""Tests for the hidden directory filter in skills listing.
|
||||||
|
|
||||||
|
Regression test: the original filter used hardcoded forward-slash strings
|
||||||
|
like '/.git/' which never match on Windows where Path uses backslashes.
|
||||||
|
This caused quarantined skills (.hub/quarantine/) to appear as installed.
|
||||||
|
|
||||||
|
Now uses Path.parts which is platform-independent.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import os
|
||||||
|
from pathlib import Path, PurePosixPath, PureWindowsPath
|
||||||
|
|
||||||
|
|
||||||
|
def _old_filter_matches(path_str: str) -> bool:
|
||||||
|
"""The BROKEN filter that used hardcoded forward slashes.
|
||||||
|
|
||||||
|
Returns True when the path SHOULD be filtered out.
|
||||||
|
"""
|
||||||
|
return '/.git/' in path_str or '/.github/' in path_str or '/.hub/' in path_str
|
||||||
|
|
||||||
|
|
||||||
|
def _new_filter_matches(path: Path) -> bool:
|
||||||
|
"""The FIXED filter using Path.parts.
|
||||||
|
|
||||||
|
Returns True when the path SHOULD be filtered out.
|
||||||
|
"""
|
||||||
|
return any(part in ('.git', '.github', '.hub') for part in path.parts)
|
||||||
|
|
||||||
|
|
||||||
|
class TestOldFilterBrokenOnWindows:
|
||||||
|
"""Demonstrate the bug: hardcoded '/' never matches Windows backslash paths."""
|
||||||
|
|
||||||
|
def test_old_filter_misses_hub_on_windows_path(self):
|
||||||
|
"""Old filter fails to catch .hub in a Windows-style path string."""
|
||||||
|
win_path = r"C:\Users\me\.hermes\skills\.hub\quarantine\evil-skill\SKILL.md"
|
||||||
|
assert _old_filter_matches(win_path) is False # Bug: should be True
|
||||||
|
|
||||||
|
def test_old_filter_misses_git_on_windows_path(self):
|
||||||
|
"""Old filter fails to catch .git in a Windows-style path string."""
|
||||||
|
win_path = r"C:\Users\me\.hermes\skills\.git\config\SKILL.md"
|
||||||
|
assert _old_filter_matches(win_path) is False # Bug: should be True
|
||||||
|
|
||||||
|
def test_old_filter_works_on_unix_path(self):
|
||||||
|
"""Old filter works fine on Unix paths (the original platform)."""
|
||||||
|
unix_path = "/home/user/.hermes/skills/.hub/quarantine/evil-skill/SKILL.md"
|
||||||
|
assert _old_filter_matches(unix_path) is True
|
||||||
|
|
||||||
|
|
||||||
|
class TestNewFilterCrossPlatform:
|
||||||
|
"""The fixed filter works on both Windows and Unix paths."""
|
||||||
|
|
||||||
|
def test_hub_quarantine_filtered(self, tmp_path):
|
||||||
|
"""A SKILL.md inside .hub/quarantine/ must be filtered out."""
|
||||||
|
p = tmp_path / ".hermes" / "skills" / ".hub" / "quarantine" / "evil" / "SKILL.md"
|
||||||
|
assert _new_filter_matches(p) is True
|
||||||
|
|
||||||
|
def test_git_dir_filtered(self, tmp_path):
|
||||||
|
"""A SKILL.md inside .git/ must be filtered out."""
|
||||||
|
p = tmp_path / ".hermes" / "skills" / ".git" / "hooks" / "SKILL.md"
|
||||||
|
assert _new_filter_matches(p) is True
|
||||||
|
|
||||||
|
def test_github_dir_filtered(self, tmp_path):
|
||||||
|
"""A SKILL.md inside .github/ must be filtered out."""
|
||||||
|
p = tmp_path / ".hermes" / "skills" / ".github" / "workflows" / "SKILL.md"
|
||||||
|
assert _new_filter_matches(p) is True
|
||||||
|
|
||||||
|
def test_normal_skill_not_filtered(self, tmp_path):
|
||||||
|
"""A regular skill SKILL.md must NOT be filtered out."""
|
||||||
|
p = tmp_path / ".hermes" / "skills" / "my-cool-skill" / "SKILL.md"
|
||||||
|
assert _new_filter_matches(p) is False
|
||||||
|
|
||||||
|
def test_nested_skill_not_filtered(self, tmp_path):
|
||||||
|
"""A deeply nested regular skill must NOT be filtered out."""
|
||||||
|
p = tmp_path / ".hermes" / "skills" / "org" / "deep-skill" / "SKILL.md"
|
||||||
|
assert _new_filter_matches(p) is False
|
||||||
|
|
||||||
|
def test_dot_prefix_not_false_positive(self, tmp_path):
|
||||||
|
"""A skill dir starting with dot but not in the filter list passes."""
|
||||||
|
p = tmp_path / ".hermes" / "skills" / ".my-hidden-skill" / "SKILL.md"
|
||||||
|
assert _new_filter_matches(p) is False
|
||||||
|
|
||||||
|
|
||||||
|
class TestWindowsPathParts:
|
||||||
|
"""Verify Path.parts correctly splits on the native separator."""
|
||||||
|
|
||||||
|
def test_parts_contains_hidden_dir(self, tmp_path):
|
||||||
|
"""Path.parts includes each directory component individually."""
|
||||||
|
p = tmp_path / "skills" / ".hub" / "quarantine" / "SKILL.md"
|
||||||
|
assert ".hub" in p.parts
|
||||||
|
|
||||||
|
def test_parts_does_not_contain_combined_string(self, tmp_path):
|
||||||
|
"""Path.parts splits by separator, not by substring."""
|
||||||
|
p = tmp_path / "skills" / "my-hub-skill" / "SKILL.md"
|
||||||
|
# ".hub" should NOT match "my-hub-skill" as a part
|
||||||
|
assert ".hub" not in p.parts
|
||||||
|
|
@ -196,8 +196,7 @@ def _find_all_skills() -> List[Dict[str, Any]]:
|
||||||
return skills
|
return skills
|
||||||
|
|
||||||
for skill_md in SKILLS_DIR.rglob("SKILL.md"):
|
for skill_md in SKILLS_DIR.rglob("SKILL.md"):
|
||||||
path_str = str(skill_md)
|
if any(part in ('.git', '.github', '.hub') for part in skill_md.parts):
|
||||||
if '/.git/' in path_str or '/.github/' in path_str or '/.hub/' in path_str:
|
|
||||||
continue
|
continue
|
||||||
|
|
||||||
skill_dir = skill_md.parent
|
skill_dir = skill_md.parent
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue