From 338061f0df446d2213647f3994f163009a68339b Mon Sep 17 00:00:00 2001 From: Addel Hamoudhy Date: Thu, 25 Jun 2026 23:38:46 +1000 Subject: [PATCH 1/2] fix(skills): prefer GitHub repo skill for owner/repo installs --- hermes_cli/skills_hub.py | 91 ++++++++++++++ .../hermes_cli/test_skills_hub_resolution.py | 111 ++++++++++++++++++ 2 files changed, 202 insertions(+) create mode 100644 tests/hermes_cli/test_skills_hub_resolution.py diff --git a/hermes_cli/skills_hub.py b/hermes_cli/skills_hub.py index 4e664944276d2..3d34bd5353e7c 100644 --- a/hermes_cli/skills_hub.py +++ b/hermes_cli/skills_hub.py @@ -122,8 +122,99 @@ def _format_extra_metadata_lines(extra: Dict[str, Any]) -> list[str]: return lines +_GITHUB_REPO_IDENTIFIER_RE = re.compile(r"^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$") + + +def _repo_slug_skill_name_variants(repo_name: str) -> set[str]: + """Return likely skill-directory names for a bare GitHub repo slug. + + Several skill repos are named ``-skill`` while the actual runtime + skill lives under ``skills//``. When a user types + ``owner/repo-name`` they usually mean "install the skill from this GitHub + repo", not "search every registry for a same-named legacy package". + """ + slug = repo_name.strip().strip("/").lower() + variants = {slug} if slug else set() + for suffix in ("-skill", "_skill", "-skills", "_skills"): + if slug.endswith(suffix): + variants.add(slug[: -len(suffix)]) + return {v for v in variants if v} + + +def _resolve_bare_github_repo_skill(identifier: str, sources): + """Resolve ``owner/repo`` to the repo's canonical skill before registries. + + Direct GitHub repo identifiers are ambiguous because they lack a skill path. + If the repository contains exactly one ``*/SKILL.md`` below the repo root, + or a skill directory whose basename matches the repo slug (with common + ``-skill`` / ``-skills`` suffixes stripped), prefer that GitHub skill. This + avoids stale registry aliases (ClawHub/skills.sh) shadowing the current + GitHub repo, while leaving short names and explicit registry identifiers + unchanged. + """ + if not _GITHUB_REPO_IDENTIFIER_RE.match(identifier): + return None, None, None + + github = next((src for src in sources if src.source_id() == "github"), None) + if github is None or not hasattr(github, "_get_repo_tree"): + return None, None, None + + repo = identifier + repo_name = repo.split("/", 1)[1] + preferred_names = _repo_slug_skill_name_variants(repo_name) + + try: + cached = github._get_repo_tree(repo) # type: ignore[attr-defined] + except Exception: + cached = None + if cached is None: + return None, None, None + + _default_branch, tree_entries = cached + skill_dirs: list[str] = [] + for entry in tree_entries: + if entry.get("type") != "blob": + continue + path = entry.get("path", "") + if not isinstance(path, str) or path == "SKILL.md" or not path.endswith("/SKILL.md"): + continue + skill_dir = path[: -len("/SKILL.md")] + if any(part.startswith((".", "_")) for part in skill_dir.split("/")): + continue + skill_dirs.append(skill_dir) + + if not skill_dirs: + return None, None, None + + preferred = [d for d in skill_dirs if d.rstrip("/").split("/")[-1].lower() in preferred_names] + if len(preferred) == 1: + skill_dir = preferred[0] + elif len(skill_dirs) == 1: + skill_dir = skill_dirs[0] + else: + return None, None, None + + resolved_identifier = f"{repo}/{skill_dir}" + try: + bundle = github.fetch(resolved_identifier) + except Exception: + bundle = None + if not bundle: + return None, None, None + + try: + meta = github.inspect(resolved_identifier) + except Exception: + meta = None + return meta, bundle, github + + def _resolve_source_meta_and_bundle(identifier: str, sources): """Resolve metadata and bundle for a specific identifier.""" + meta, bundle, matched_source = _resolve_bare_github_repo_skill(identifier, sources) + if bundle: + return meta, bundle, matched_source + meta = None bundle = None matched_source = None diff --git a/tests/hermes_cli/test_skills_hub_resolution.py b/tests/hermes_cli/test_skills_hub_resolution.py new file mode 100644 index 0000000000000..ec84753f60018 --- /dev/null +++ b/tests/hermes_cli/test_skills_hub_resolution.py @@ -0,0 +1,111 @@ +"""Tests for CLI skill install source resolution.""" + +from hermes_cli.skills_hub import _resolve_source_meta_and_bundle +from tools.skills_hub import SkillBundle, SkillMeta + + +class _FakeGitHubSource: + def __init__(self, tree_paths): + self.tree_paths = tree_paths + self.fetched = [] + self.inspected = [] + + def source_id(self): + return "github" + + def _get_repo_tree(self, repo): + return ( + "main", + [{"type": "blob", "path": path} for path in self.tree_paths], + ) + + def fetch(self, identifier): + self.fetched.append(identifier) + if identifier == "mvanhorn/last30days-skill/skills/last30days": + return SkillBundle( + name="last30days", + files={"SKILL.md": "---\nname: last30days\n---\n"}, + source="github", + identifier=identifier, + trust_level="community", + ) + return None + + def inspect(self, identifier): + self.inspected.append(identifier) + if identifier == "mvanhorn/last30days-skill/skills/last30days": + return SkillMeta( + name="last30days", + description="Current GitHub runtime skill", + source="github", + identifier=identifier, + trust_level="community", + repo="mvanhorn/last30days-skill", + path="skills/last30days", + ) + return None + + +class _FakeClawHubSource: + def source_id(self): + return "clawhub" + + def fetch(self, identifier): + if identifier == "mvanhorn/last30days-skill": + return SkillBundle( + name="last30days-skill", + files={"SKILL.md": "stale root package"}, + source="clawhub", + identifier="last30days-skill", + trust_level="community", + ) + return None + + def inspect(self, identifier): + if identifier == "mvanhorn/last30days-skill": + return SkillMeta( + name="Last30days Skill", + description="Stale registry package", + source="clawhub", + identifier="last30days-skill", + trust_level="community", + ) + return None + + +def test_bare_github_repo_prefers_current_repo_skill_over_registry_alias(): + github = _FakeGitHubSource(["skills/last30days/SKILL.md"]) + clawhub = _FakeClawHubSource() + + meta, bundle, matched = _resolve_source_meta_and_bundle( + "mvanhorn/last30days-skill", + # Put ClawHub first to prove explicit owner/repo still prefers GitHub. + [clawhub, github], + ) + + assert matched is github + assert bundle is not None + assert bundle.source == "github" + assert bundle.identifier == "mvanhorn/last30days-skill/skills/last30days" + assert meta is not None + assert meta.path == "skills/last30days" + assert github.fetched == ["mvanhorn/last30days-skill/skills/last30days"] + + +def test_bare_github_repo_falls_back_to_sources_when_repo_skill_is_ambiguous(): + github = _FakeGitHubSource([ + "skills/alpha/SKILL.md", + "skills/beta/SKILL.md", + ]) + clawhub = _FakeClawHubSource() + + meta, bundle, matched = _resolve_source_meta_and_bundle( + "mvanhorn/last30days-skill", + [github, clawhub], + ) + + assert matched is clawhub + assert bundle is not None + assert bundle.source == "clawhub" + assert meta is not None + assert meta.source == "clawhub" From 596797874a45029fde4bb77858450fffc7e95cf2 Mon Sep 17 00:00:00 2001 From: addel Date: Sun, 16 Aug 2026 13:31:15 +1000 Subject: [PATCH 2/2] fix(skills): preserve registered source resolution --- hermes_cli/skills_hub.py | 4 ++ .../hermes_cli/test_skills_hub_resolution.py | 46 +++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/hermes_cli/skills_hub.py b/hermes_cli/skills_hub.py index 3d34bd5353e7c..5a774d0eced03 100644 --- a/hermes_cli/skills_hub.py +++ b/hermes_cli/skills_hub.py @@ -155,6 +155,10 @@ def _resolve_bare_github_repo_skill(identifier: str, sources): if not _GITHUB_REPO_IDENTIFIER_RE.match(identifier): return None, None, None + prefix = identifier.split("/", 1)[0] + if any(src.source_id() == prefix for src in sources): + return None, None, None + github = next((src for src in sources if src.source_id() == "github"), None) if github is None or not hasattr(github, "_get_repo_tree"): return None, None, None diff --git a/tests/hermes_cli/test_skills_hub_resolution.py b/tests/hermes_cli/test_skills_hub_resolution.py index ec84753f60018..4942980d125e0 100644 --- a/tests/hermes_cli/test_skills_hub_resolution.py +++ b/tests/hermes_cli/test_skills_hub_resolution.py @@ -7,6 +7,7 @@ class _FakeGitHubSource: def __init__(self, tree_paths): self.tree_paths = tree_paths + self.repo_tree_requests = [] self.fetched = [] self.inspected = [] @@ -14,6 +15,7 @@ def source_id(self): return "github" def _get_repo_tree(self, repo): + self.repo_tree_requests.append(repo) return ( "main", [{"type": "blob", "path": path} for path in self.tree_paths], @@ -73,6 +75,33 @@ def inspect(self, identifier): return None +class _FakeOfficialSource: + def source_id(self): + return "official" + + def fetch(self, identifier): + if identifier == "official/last30days": + return SkillBundle( + name="last30days", + files={"SKILL.md": "official skill"}, + source="official", + identifier=identifier, + trust_level="builtin", + ) + return None + + def inspect(self, identifier): + if identifier == "official/last30days": + return SkillMeta( + name="last30days", + description="Official skill", + source="official", + identifier=identifier, + trust_level="builtin", + ) + return None + + def test_bare_github_repo_prefers_current_repo_skill_over_registry_alias(): github = _FakeGitHubSource(["skills/last30days/SKILL.md"]) clawhub = _FakeClawHubSource() @@ -109,3 +138,20 @@ def test_bare_github_repo_falls_back_to_sources_when_repo_skill_is_ambiguous(): assert bundle.source == "clawhub" assert meta is not None assert meta.source == "clawhub" + + +def test_registered_source_identifier_bypasses_bare_github_preflight(): + github = _FakeGitHubSource(["skills/last30days/SKILL.md"]) + official = _FakeOfficialSource() + + meta, bundle, matched = _resolve_source_meta_and_bundle( + "official/last30days", + [github, official], + ) + + assert github.repo_tree_requests == [] + assert matched is official + assert bundle is not None + assert bundle.source == "official" + assert meta is not None + assert meta.source == "official"