Merge pull request #862 from arceus77-7/fix/skills-list-source-provenance
Merging — clean fix for local skills mislabeling. Follow-up cleanup coming.
This commit is contained in:
@@ -2399,7 +2399,7 @@ For more help on a command:
|
||||
skills_inspect.add_argument("identifier", help="Skill identifier")
|
||||
|
||||
skills_list = skills_subparsers.add_parser("list", help="List installed skills")
|
||||
skills_list.add_argument("--source", default="all", choices=["all", "hub", "builtin"])
|
||||
skills_list.add_argument("--source", default="all", choices=["all", "hub", "builtin", "local"])
|
||||
|
||||
skills_audit = skills_subparsers.add_parser("audit", help="Re-scan installed hub skills")
|
||||
skills_audit.add_argument("name", nargs="?", help="Specific skill to audit (default: all)")
|
||||
|
||||
@@ -407,14 +407,17 @@ def do_inspect(identifier: str, console: Optional[Console] = None) -> None:
|
||||
|
||||
|
||||
def do_list(source_filter: str = "all", console: Optional[Console] = None) -> None:
|
||||
"""List installed skills, distinguishing builtins from hub-installed."""
|
||||
"""List installed skills, distinguishing hub, builtin, and local skills."""
|
||||
from tools.skills_hub import HubLockFile, ensure_hub_dirs
|
||||
from tools.skills_sync import _read_manifest
|
||||
from tools.skills_tool import _find_all_skills
|
||||
|
||||
c = console or _console
|
||||
ensure_hub_dirs()
|
||||
lock = HubLockFile()
|
||||
hub_installed = {e["name"]: e for e in lock.list_installed()}
|
||||
bundled_manifest = _read_manifest()
|
||||
builtin_names = set(bundled_manifest.keys()) if isinstance(bundled_manifest, dict) else set()
|
||||
|
||||
all_skills = _find_all_skills()
|
||||
|
||||
@@ -424,30 +427,42 @@ def do_list(source_filter: str = "all", console: Optional[Console] = None) -> No
|
||||
table.add_column("Source", style="dim")
|
||||
table.add_column("Trust", style="dim")
|
||||
|
||||
hub_count = 0
|
||||
builtin_count = 0
|
||||
local_count = 0
|
||||
|
||||
for skill in sorted(all_skills, key=lambda s: (s.get("category") or "", s["name"])):
|
||||
name = skill["name"]
|
||||
category = skill.get("category", "")
|
||||
hub_entry = hub_installed.get(name)
|
||||
|
||||
if hub_entry:
|
||||
source_type = "hub"
|
||||
source_display = hub_entry.get("source", "hub")
|
||||
trust = hub_entry.get("trust_level", "community")
|
||||
else:
|
||||
hub_count += 1
|
||||
elif name in builtin_names:
|
||||
source_type = "builtin"
|
||||
source_display = "builtin"
|
||||
trust = "builtin"
|
||||
builtin_count += 1
|
||||
else:
|
||||
source_type = "local"
|
||||
source_display = "local"
|
||||
trust = "local"
|
||||
local_count += 1
|
||||
|
||||
if source_filter == "hub" and not hub_entry:
|
||||
continue
|
||||
if source_filter == "builtin" and hub_entry:
|
||||
if source_filter != "all" and source_filter != source_type:
|
||||
continue
|
||||
|
||||
trust_style = {"builtin": "bright_cyan", "trusted": "green", "community": "yellow"}.get(trust, "dim")
|
||||
trust_style = {"builtin": "bright_cyan", "trusted": "green", "community": "yellow", "local": "dim"}.get(trust, "dim")
|
||||
trust_label = "official" if source_display == "official" else trust
|
||||
table.add_row(name, category, source_display, f"[{trust_style}]{trust_label}[/]")
|
||||
|
||||
c.print(table)
|
||||
c.print(f"[dim]{len(hub_installed)} hub-installed, "
|
||||
f"{len(all_skills) - len(hub_installed)} builtin[/]\n")
|
||||
c.print(
|
||||
f"[dim]{hub_count} hub-installed, {builtin_count} builtin, {local_count} local[/]\n"
|
||||
)
|
||||
|
||||
|
||||
def do_audit(name: Optional[str] = None, console: Optional[Console] = None) -> None:
|
||||
@@ -1014,7 +1029,7 @@ def _print_skills_help(console: Console) -> None:
|
||||
" [cyan]search[/] <query> Search registries for skills\n"
|
||||
" [cyan]install[/] <identifier> Install a skill (with security scan)\n"
|
||||
" [cyan]inspect[/] <identifier> Preview a skill without installing\n"
|
||||
" [cyan]list[/] [--source hub|builtin] List installed skills\n"
|
||||
" [cyan]list[/] [--source hub|builtin|local] List installed skills\n"
|
||||
" [cyan]audit[/] [name] Re-scan hub skills for security\n"
|
||||
" [cyan]uninstall[/] <name> Remove a hub-installed skill\n"
|
||||
" [cyan]publish[/] <path> --repo <r> Publish a skill to GitHub via PR\n"
|
||||
|
||||
@@ -5,6 +5,14 @@ from rich.console import Console
|
||||
from hermes_cli.skills_hub import do_list
|
||||
|
||||
|
||||
class _DummyLockFile:
|
||||
def __init__(self, installed):
|
||||
self._installed = installed
|
||||
|
||||
def list_installed(self):
|
||||
return self._installed
|
||||
|
||||
|
||||
def test_do_list_initializes_hub_dir(monkeypatch, tmp_path):
|
||||
import tools.skills_hub as hub
|
||||
import tools.skills_tool as skills_tool
|
||||
@@ -29,3 +37,82 @@ def test_do_list_initializes_hub_dir(monkeypatch, tmp_path):
|
||||
assert (hub_dir / "lock.json").exists()
|
||||
assert (hub_dir / "quarantine").is_dir()
|
||||
assert (hub_dir / "index-cache").is_dir()
|
||||
|
||||
|
||||
def test_do_list_distinguishes_hub_builtin_and_local(monkeypatch, tmp_path):
|
||||
import tools.skills_hub as hub
|
||||
import tools.skills_sync as skills_sync
|
||||
import tools.skills_tool as skills_tool
|
||||
|
||||
hub_dir = tmp_path / "skills" / ".hub"
|
||||
monkeypatch.setattr(hub, "SKILLS_DIR", tmp_path / "skills")
|
||||
monkeypatch.setattr(hub, "HUB_DIR", hub_dir)
|
||||
monkeypatch.setattr(hub, "LOCK_FILE", hub_dir / "lock.json")
|
||||
monkeypatch.setattr(hub, "QUARANTINE_DIR", hub_dir / "quarantine")
|
||||
monkeypatch.setattr(hub, "AUDIT_LOG", hub_dir / "audit.log")
|
||||
monkeypatch.setattr(hub, "TAPS_FILE", hub_dir / "taps.json")
|
||||
monkeypatch.setattr(hub, "INDEX_CACHE_DIR", hub_dir / "index-cache")
|
||||
|
||||
monkeypatch.setattr(
|
||||
hub,
|
||||
"HubLockFile",
|
||||
lambda: _DummyLockFile([
|
||||
{"name": "hub-skill", "source": "github", "trust_level": "community"},
|
||||
]),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
skills_tool,
|
||||
"_find_all_skills",
|
||||
lambda: [
|
||||
{"name": "hub-skill", "category": "x", "description": "hub"},
|
||||
{"name": "builtin-skill", "category": "x", "description": "builtin"},
|
||||
{"name": "local-skill", "category": "x", "description": "local"},
|
||||
],
|
||||
)
|
||||
monkeypatch.setattr(skills_sync, "_read_manifest", lambda: {"builtin-skill": "abc123"})
|
||||
|
||||
sink = StringIO()
|
||||
console = Console(file=sink, force_terminal=False, color_system=None)
|
||||
|
||||
do_list(console=console)
|
||||
|
||||
output = sink.getvalue()
|
||||
assert "hub-skill" in output
|
||||
assert "builtin-skill" in output
|
||||
assert "local-skill" in output
|
||||
assert "1 hub-installed, 1 builtin, 1 local" in output
|
||||
|
||||
|
||||
def test_do_list_local_filter(monkeypatch, tmp_path):
|
||||
import tools.skills_hub as hub
|
||||
import tools.skills_sync as skills_sync
|
||||
import tools.skills_tool as skills_tool
|
||||
|
||||
hub_dir = tmp_path / "skills" / ".hub"
|
||||
monkeypatch.setattr(hub, "SKILLS_DIR", tmp_path / "skills")
|
||||
monkeypatch.setattr(hub, "HUB_DIR", hub_dir)
|
||||
monkeypatch.setattr(hub, "LOCK_FILE", hub_dir / "lock.json")
|
||||
monkeypatch.setattr(hub, "QUARANTINE_DIR", hub_dir / "quarantine")
|
||||
monkeypatch.setattr(hub, "AUDIT_LOG", hub_dir / "audit.log")
|
||||
monkeypatch.setattr(hub, "TAPS_FILE", hub_dir / "taps.json")
|
||||
monkeypatch.setattr(hub, "INDEX_CACHE_DIR", hub_dir / "index-cache")
|
||||
|
||||
monkeypatch.setattr(hub, "HubLockFile", lambda: _DummyLockFile([]))
|
||||
monkeypatch.setattr(
|
||||
skills_tool,
|
||||
"_find_all_skills",
|
||||
lambda: [
|
||||
{"name": "builtin-skill", "category": "x", "description": "builtin"},
|
||||
{"name": "local-skill", "category": "x", "description": "local"},
|
||||
],
|
||||
)
|
||||
monkeypatch.setattr(skills_sync, "_read_manifest", lambda: {"builtin-skill": "abc123"})
|
||||
|
||||
sink = StringIO()
|
||||
console = Console(file=sink, force_terminal=False, color_system=None)
|
||||
|
||||
do_list(source_filter="local", console=console)
|
||||
|
||||
output = sink.getvalue()
|
||||
assert "local-skill" in output
|
||||
assert "builtin-skill" not in output
|
||||
|
||||
Reference in New Issue
Block a user