From 2dcd97d648d484a4e14ffb457269a76576a14a66 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 3 Sep 2026 13:22:48 -0700 Subject: [PATCH] =?UTF-8?q?simplify(compat):=20tools/skills=5Fhub=20?= =?UTF-8?q?=E2=80=94=20drop=2051=20re-exports/aliases,=20repoint=2025=20ca?= =?UTF-8?q?llers=20+=2011=20test=20files?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/skills_hub.py | 48 +++++----- hermes_cli/web_routers/skills.py | 19 ++-- scripts/build_skills_index.py | 19 ++-- .../test_dashboard_admin_endpoints.py | 46 ++++----- tests/hermes_cli/test_managed_installs.py | 2 +- tests/hermes_cli/test_skills_hub.py | 30 +++--- tests/hermes_cli/test_skills_skip_confirm.py | 6 +- tests/tools/test_pr_6656_regressions.py | 7 +- tests/tools/test_skill_bundle_provenance.py | 34 +++---- tests/tools/test_skills_hub.py | 94 ++++++++----------- tests/tools/test_skills_hub_browse_sh.py | 3 +- tests/tools/test_skills_hub_clawhub.py | 3 +- .../test_windows_agent_loop_papercuts.py | 6 +- tests/tui_gateway/test_protocol.py | 14 ++- tools/skills_guard.py | 2 +- tools/skills_hub.py | 33 +------ tools/skills_hub_install.py | 7 +- tools/skills_hub_official.py | 3 +- tools/skills_hub_search.py | 25 ++--- tui_gateway/methods_tools.py | 17 ++-- 20 files changed, 192 insertions(+), 226 deletions(-) diff --git a/hermes_cli/skills_hub.py b/hermes_cli/skills_hub.py index 2f906088c5..3a252abf0e 100644 --- a/hermes_cli/skills_hub.py +++ b/hermes_cli/skills_hub.py @@ -87,7 +87,8 @@ def _try(fn, *args): def _sources(): """Source router over all registries (authenticated GitHub when available).""" - from tools.skills_hub import GitHubAuth, create_source_router + from tools.skills_hub_github import GitHubAuth + from tools.skills_hub_search import create_source_router return create_source_router(GitHubAuth()) @@ -188,7 +189,7 @@ def _format_extra_metadata_lines(extra: Dict[str, Any]) -> list[str]: def _resolve_short_name(name: str, sources, console: Console) -> str: """Short name -> full identifier via search; "" when ambiguous/missing (one exact match wins, several -> the single official one, else they are listed).""" - from tools.skills_hub import unified_search + from tools.skills_hub_search import unified_search c = console or _console c.print(f"[dim]Resolving '{name}'...[/]") results = unified_search(name, sources, source_filter="all", limit=20) @@ -259,7 +260,8 @@ def _is_valid_installed_skill_name(name: str) -> bool: def _existing_categories() -> List[str]: """Sorted category buckets under ``~/.hermes/skills/`` (children without their own SKILL.md).""" - from tools.skills_hub import SKILLS_DIR, _category_skill_dirs + from tools.skills_hub import SKILLS_DIR + from tools.skills_hub_install import _category_skill_dirs try: return sorted(name for name in set(_category_skill_dirs(SKILLS_DIR)) if not (SKILLS_DIR / name / "SKILL.md").exists()) @@ -315,7 +317,7 @@ def _prompt_for_category(c: Console, existing: List[str]) -> str: def do_search(query: str, source: str = "all", limit: int = 10, console: Optional[Console] = None, as_json: bool = False) -> None: """Search registries -> Rich table, or a clean JSON array (``as_json``) for scripting.""" - from tools.skills_hub import unified_search + from tools.skills_hub_search import unified_search c = console or _console sources = _sources() if as_json: @@ -360,7 +362,7 @@ def _rank_and_page(all_results, page: int, page_size: int): def _fetch_browse_results(c: Console, source: str): """Parallel fetch from all (or filtered) sources with a live per-source progress spinner.""" - from tools.skills_hub import parallel_search_sources + from tools.skills_hub_search import parallel_search_sources with c.status("[bold]Fetching skills from registries...") as status: # parallel_search_sources invokes the callback from the collecting thread as each # source completes; the page itself is rendered once over the final, fully sorted set. @@ -422,7 +424,7 @@ def do_browse(page: int = 1, page_size: int = 20, source: str = "all", return # Provider filter (nvidia/openai/...) narrows GitHub-tap skills by their per-tap # ``extra.provider`` label (the runtime index stores them all under source="github"). - from tools.skills_hub import _PROVIDER_FILTER_VALUES, _filter_results_by_provider + from tools.skills_hub_github import _PROVIDER_FILTER_VALUES, _filter_results_by_provider if source.strip().lower() in _PROVIDER_FILTER_VALUES: all_results = _filter_results_by_provider(all_results, source) if not all_results: @@ -435,7 +437,7 @@ def do_browse(page: int = 1, page_size: int = 20, source: str = "all", def browse_skills(page: int = 1, page_size: int = 20, source: str = "all") -> dict: """Paginated hub browse for programmatic callers (e.g. TUI gateway).""" - from tools.skills_hub import parallel_search_sources + from tools.skills_hub_search import parallel_search_sources page_size = max(1, min(page_size, 100)) # The shared parallel walker carries the index-aware source-skip logic — querying # hermes-index AND the external APIs at once would double-count every skill. @@ -573,7 +575,7 @@ def _announce_blueprint(c: Console, skill_name: str) -> None: def _pinned_sources(c: Console, sources, source_id: Optional[str], identifier: str): """Restrict `sources` to the adapter matching `source_id`; None when it is unknown.""" - from tools.skills_hub import _source_matches + from tools.skills_hub_install import _source_matches pinned = [src for src in sources if _source_matches(src, source_id)] if source_id else sources if pinned: return pinned @@ -599,7 +601,8 @@ def _print_fetch_failure(c: Console, sources, identifier: str) -> None: def _scan_quarantined(c: Console, q_path: Path, bundle, meta, identifier: str): """Run the cached security scan on the quarantined bundle and print the report.""" - from tools.skills_hub import HUB_DIR, source_url_for_bundle + from tools.skills_hub import HUB_DIR + from tools.skills_hub_models import source_url_for_bundle from tools.skills_guard import scan_skill_cached, format_scan_report c.print("[bold]Running security scan...[/]") scan_source = ("official" if bundle.source == "official" @@ -646,8 +649,8 @@ def do_install(identifier: str, category: str = "", force: bool = False, """Fetch, quarantine, scan, confirm, and install a skill. ``source_id`` pins resolution to one adapter; callers that know the provenance (``do_update``) must pass it so a bare identifier cannot resolve to a same-named skill elsewhere.""" - from tools.skills_hub import (ensure_hub_dirs, quarantine_bundle, install_from_quarantine, - HubLockFile) + from tools.skills_hub import HubLockFile, ensure_hub_dirs + from tools.skills_hub_install import install_from_quarantine, quarantine_bundle from tools.skills_guard import should_allow_install c = console or _console ensure_hub_dirs() @@ -793,7 +796,7 @@ def do_list(source_filter: str = "all", enabled_only: bool = False, def do_check(name: Optional[str] = None, console: Optional[Console] = None) -> None: """Check hub-installed skills for upstream updates.""" - from tools.skills_hub import check_for_skill_updates + from tools.skills_hub_install import check_for_skill_updates c = console or _console results = check_for_skill_updates(name=name) if not results: @@ -832,7 +835,8 @@ def do_update(name: Optional[str] = None, console: Optional[Console] = None, paperclipai/paperclip#10978's explicit-merge-mode rule: destructive replacement must be an explicit caller choice, never a rerun default). """ - from tools.skills_hub import HubLockFile, check_for_skill_updates + from tools.skills_hub import HubLockFile + from tools.skills_hub_install import check_for_skill_updates c = console or _console lock = HubLockFile() updates = [entry for entry in check_for_skill_updates(name=name) if entry.get("status") == "update_available"] @@ -900,7 +904,7 @@ def do_audit(name: Optional[str] = None, console: Optional[Console] = None, def do_uninstall(name: str, console: Optional[Console] = None, skip_confirm: bool = False, invalidate_cache: bool = True) -> None: """Remove a hub-installed skill with confirmation.""" - from tools.skills_hub import uninstall_skill + from tools.skills_hub_install import uninstall_skill c = console or _console # skip_confirm bypasses the prompt (TUI mode, where input() hangs) if not skip_confirm and not _confirm_or_cancel(c, f"\n[bold]Uninstall '{name}'?[/]"): @@ -912,7 +916,7 @@ def do_uninstall(name: str, console: Optional[Console] = None, skip_confirm: boo def do_reset(name: str, restore: bool = False, console: Optional[Console] = None, skip_confirm: bool = False, invalidate_cache: bool = True) -> None: """Reset a bundled skill's manifest tracking (+ optionally restore from bundled).""" - from tools.skills_sync import reset_bundled_skill + from tools.skills_sync_bundled_ops import reset_bundled_skill c = console or _console if not skip_confirm and restore and not _confirm_or_cancel( c, f"\n[bold]Restore '{name}' from bundled source?[/]", @@ -930,7 +934,7 @@ def do_reset(name: str, restore: bool = False, console: Optional[Console] = None def do_list_modified(console: Optional[Console] = None, as_json: bool = False) -> None: """List bundled skills the user has edited (which `hermes update` keeps).""" - from tools.skills_sync import list_user_modified_bundled_skills + from tools.skills_sync_bundled_ops import list_user_modified_bundled_skills c = console or _console modified = list_user_modified_bundled_skills() if as_json: @@ -965,7 +969,7 @@ _DIFF_STATUS_LINE = { def do_diff(name: str, console: Optional[Console] = None) -> None: """Show how the user's copy of a bundled skill differs from the stock version.""" - from tools.skills_sync import diff_bundled_skill + from tools.skills_sync_bundled_ops import diff_bundled_skill c = console or _console result = diff_bundled_skill(name) if not result["ok"]: @@ -990,7 +994,7 @@ def do_opt_out(remove: bool = False, console: Optional[Console] = None, skip_con invalidate_cache: bool = True) -> None: """Write the .no-bundled-skills marker; with ``remove`` also delete pristine (tracked AND unmodified) bundled skills. User-edited and non-bundled skills are never touched.""" - from tools.skills_sync import set_bundled_skills_opt_out, remove_pristine_bundled_skills + from tools.skills_sync_bundled_ops import set_bundled_skills_opt_out, remove_pristine_bundled_skills c = console or _console res = set_bundled_skills_opt_out(True) # the marker first: always-safe if not _report_ok(c, res): @@ -1026,7 +1030,8 @@ def do_opt_out(remove: bool = False, console: Optional[Console] = None, skip_con def do_opt_in(sync: bool = False, console: Optional[Console] = None, invalidate_cache: bool = True) -> None: """Remove the opt-out marker so bundled-skill seeding resumes.""" - from tools.skills_sync import set_bundled_skills_opt_out, sync_skills + from tools.skills_sync import sync_skills + from tools.skills_sync_bundled_ops import set_bundled_skills_opt_out c = console or _console if not _report_ok(c, set_bundled_skills_opt_out(False)): return @@ -1040,7 +1045,7 @@ def do_opt_in(sync: bool = False, console: Optional[Console] = None, def do_repair_official(name: str, restore: bool = False, console: Optional[Console] = None, skip_confirm: bool = False, invalidate_cache: bool = True) -> None: """Backfill or restore official optional skills from repo source.""" - from tools.skills_sync import restore_official_optional_skill + from tools.skills_sync_optional import restore_official_optional_skill c = console or _console if restore and not skip_confirm and not _confirm_or_cancel( c, f"\n[bold]Restore official optional skill '{name}' from repo source?[/]", @@ -1107,7 +1112,8 @@ def _read_frontmatter(skill_md: str) -> dict: def do_publish(skill_path: str, target: str = "github", repo: str = "", console: Optional[Console] = None) -> None: """Publish a local skill to a registry (GitHub PR or ClawHub submission).""" - from tools.skills_hub import GitHubAuth, SKILLS_DIR + from tools.skills_hub import SKILLS_DIR + from tools.skills_hub_github import GitHubAuth from tools.skills_guard import scan_skill, format_scan_report c = console or _console path = Path(skill_path) diff --git a/hermes_cli/web_routers/skills.py b/hermes_cli/web_routers/skills.py index d49ab86676..2d936b4d09 100644 --- a/hermes_cli/web_routers/skills.py +++ b/hermes_cli/web_routers/skills.py @@ -13,6 +13,7 @@ from typing import Optional from fastapi import APIRouter, HTTPException from hermes_cli.web_deps import late +from hermes_cli.web_server_profiles import _hub_action_name, _installed_hub_identifiers from hermes_cli.web_models import ( SkillContentUpdate, SkillCreate, SkillInstallRequest, SkillToggle, SkillUninstallRequest, SkillsUpdateRequest) @@ -23,12 +24,8 @@ from hermes_cli.web_routers._common import ( hub_router = APIRouter() router = APIRouter() -_config_profile_scope = late("_config_profile_scope") -_hub_action_name = late("_hub_action_name") -_installed_hub_identifiers = late("_installed_hub_identifiers") -load_config = late("load_config") - - +_config_profile_scope = late("_config_profile_scope", "hermes_cli.web_server_profiles") +load_config = late("load_config", "hermes_cli.config") # Labels per hub source id (matches `hermes skills search` provenance); keep in # sync with create_source_router()'s source list. _SKILL_HUB_SOURCE_LABELS = { @@ -46,7 +43,7 @@ _SKILL_HUB_SOURCE_LABELS = { def _hub_sources(profile: Optional[str]): """Source router built under ``profile``'s config scope.""" - from tools.skills_hub import create_source_router + from tools.skills_hub_search import create_source_router with _config_profile_scope(profile): return create_source_router() @@ -55,7 +52,7 @@ def _hub_sources(profile: Optional[str]): def _resolve_hub_skill(ident: str, profile: Optional[str]): """``(meta, bundle)`` for a hub identifier, resolved under ``profile``'s scope.""" from hermes_cli.skills_hub import _resolve_source_meta_and_bundle - from tools.skills_hub import create_source_router + from tools.skills_hub_search import create_source_router with _config_profile_scope(profile): sources = create_source_router() @@ -128,7 +125,7 @@ async def list_official_skills(profile: Optional[str] = None): """The ENTIRE optional-skills catalog (local scan), marked installed for ``profile``.""" def _run(): - from tools.skills_hub import OptionalSkillSource + from tools.skills_hub_official import OptionalSkillSource installed = _installed_hub_identifiers(profile) out = [] @@ -194,7 +191,7 @@ async def search_skills_hub( return {"results": [], "source_counts": {}, "timed_out": [], "installed": {}} def _run(): - from tools.skills_hub import parallel_search_sources + from tools.skills_hub_search import parallel_search_sources sources = _hub_sources(profile) capped = min(max(limit, 1), 50) @@ -275,7 +272,7 @@ async def scan_skill_hub(identifier: str = "", profile: Optional[str] = None): def _run(): import shutil as _shutil - from tools.skills_hub import quarantine_bundle + from tools.skills_hub_install import quarantine_bundle from tools.skills_guard import scan_skill, should_allow_install meta, bundle = _resolve_hub_skill(ident, profile) diff --git a/scripts/build_skills_index.py b/scripts/build_skills_index.py index 5eb60f8bf6..3895932eb7 100644 --- a/scripts/build_skills_index.py +++ b/scripts/build_skills_index.py @@ -28,20 +28,15 @@ from datetime import datetime, timezone REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) sys.path.insert(0, REPO_ROOT) -# Ensure HERMES_HOME is set (needed by tools/skills_hub.py imports) +# Ensure HERMES_HOME is set (needed by tools/skills_hub*.py imports) os.environ.setdefault("HERMES_HOME", os.path.join(os.path.expanduser("~"), ".hermes")) -from tools.skills_hub import ( - GitHubAuth, - GitHubSource, - SkillsShSource, - OptionalSkillSource, - WellKnownSkillSource, - ClawHubSource, - LobeHubSource, - BrowseShSource, - SkillMeta, -) +from tools.skills_hub_clawhub import ClawHubSource +from tools.skills_hub_github import GitHubAuth, GitHubSource +from tools.skills_hub_models import SkillMeta +from tools.skills_hub_official import OptionalSkillSource +from tools.skills_hub_skillssh import SkillsShSource +from tools.skills_hub_sources import BrowseShSource, LobeHubSource, WellKnownSkillSource import httpx OUTPUT_PATH = os.path.join(REPO_ROOT, "website", "static", "api", "skills-index.json") diff --git a/tests/hermes_cli/test_dashboard_admin_endpoints.py b/tests/hermes_cli/test_dashboard_admin_endpoints.py index f5ee3bec9e..3fc75119b4 100644 --- a/tests/hermes_cli/test_dashboard_admin_endpoints.py +++ b/tests/hermes_cli/test_dashboard_admin_endpoints.py @@ -8,6 +8,10 @@ visible to the CLI data layer), not specific catalog values. """ import pytest +import hermes_cli.config as _cfg_mod +import hermes_cli.web_server_files as _web_server_files +import hermes_cli.web_server_gateway as _web_server_gateway +import hermes_cli.web_server_lifecycle as _web_server_lifecycle def _client(): @@ -351,7 +355,7 @@ class TestWebhookEndpoints: import hermes_cli.web_server as ws from hermes_cli.config import load_config - ws._ACTION_PROCS.pop("gateway-restart", None) + _web_server_gateway._ACTION_PROCS.pop("gateway-restart", None) restart_calls = [] class FakeRestartProc: @@ -361,7 +365,7 @@ class TestWebhookEndpoints: restart_calls.append((subcommand, name)) return FakeRestartProc() - monkeypatch.setattr(ws, "_spawn_hermes_action", fake_spawn_action) + monkeypatch.setattr(_web_server_gateway, "_spawn_hermes_action", fake_spawn_action) r = self.client.post("/api/webhooks/enable") @@ -384,7 +388,7 @@ class TestWebhookEndpoints: import hermes_cli.web_server as ws from hermes_cli.config import load_config - ws._ACTION_PROCS.pop("gateway-restart", None) + _web_server_gateway._ACTION_PROCS.pop("gateway-restart", None) class FakeRunningProc: pid = 5151 @@ -392,12 +396,12 @@ class TestWebhookEndpoints: def poll(self): return None - monkeypatch.setitem(ws._ACTION_PROCS, "gateway-restart", FakeRunningProc()) + monkeypatch.setitem(_web_server_gateway._ACTION_PROCS, "gateway-restart", FakeRunningProc()) def fail_spawn_action(subcommand, name): raise AssertionError("must not spawn a second concurrent restart") - monkeypatch.setattr(ws, "_spawn_hermes_action", fail_spawn_action) + monkeypatch.setattr(_web_server_gateway, "_spawn_hermes_action", fail_spawn_action) r = self.client.post("/api/webhooks/enable") @@ -645,7 +649,7 @@ class TestSkillsHubSourcesEndpoint: return srcs monkeypatch.setattr( - "tools.skills_hub.create_source_router", _fake_router + "tools.skills_hub_search.create_source_router", _fake_router ) r = self.client.get("/api/skills/hub/sources") assert r.status_code == 200 @@ -674,11 +678,11 @@ class TestOfficialSkillsCatalogEndpoint: _FakeMeta("official/creative/ascii-art", "builtin", "official"), ] monkeypatch.setattr( - "tools.skills_hub.OptionalSkillSource.list_local", + "tools.skills_hub_official.OptionalSkillSource.list_local", lambda self: metas, ) monkeypatch.setattr( - "hermes_cli.web_server._installed_hub_identifiers", + "hermes_cli.web_server_profiles._installed_hub_identifiers", lambda profile=None: {"official/gifs/gif-search": {"name": "gif-search"}}, ) r = self.client.get("/api/skills/hub/official") @@ -705,7 +709,7 @@ class TestSkillsHubPreviewEndpoint: def test_preview_returns_skill_md_text(self, monkeypatch): monkeypatch.setattr( - "tools.skills_hub.create_source_router", lambda: [] + "tools.skills_hub_search.create_source_router", lambda: [] ) bundle = _FakeBundle("github/owner/repo/x") meta = _FakeMeta("github/owner/repo/x") @@ -726,7 +730,7 @@ class TestSkillsHubPreviewEndpoint: def test_preview_404_when_unresolved(self, monkeypatch): monkeypatch.setattr( - "tools.skills_hub.create_source_router", lambda: [] + "tools.skills_hub_search.create_source_router", lambda: [] ) monkeypatch.setattr( "hermes_cli.skills_hub._resolve_source_meta_and_bundle", @@ -746,7 +750,7 @@ class TestSkillsHubScanEndpoint: from tools.skills_guard import ScanResult, Finding monkeypatch.setattr( - "tools.skills_hub.create_source_router", lambda: [] + "tools.skills_hub_search.create_source_router", lambda: [] ) bundle = _FakeBundle("github/owner/repo/x", trust_level="community") monkeypatch.setattr( @@ -757,7 +761,7 @@ class TestSkillsHubScanEndpoint: from pathlib import Path monkeypatch.setattr( - "tools.skills_hub.quarantine_bundle", lambda b: Path("/tmp/_fake_q") + "tools.skills_hub_install.quarantine_bundle", lambda b: Path("/tmp/_fake_q") ) fake_result = ScanResult( @@ -841,7 +845,7 @@ class TestUpdateCheckEndpoint: def test_git_install_reports_behind_count(self, monkeypatch): import hermes_cli.web_server as ws - monkeypatch.setattr(ws, "detect_install_method", lambda *a, **k: "git") + monkeypatch.setattr(_cfg_mod, "detect_install_method", lambda *a, **k: "git") # Stub the shared checker so the contract is deterministic (no network). import hermes_cli.banner as banner @@ -870,9 +874,9 @@ class TestUpdateCheckEndpoint: def test_managed_runtime_dashboard_is_not_applyable(self, monkeypatch): import hermes_cli.web_server as ws - monkeypatch.setattr(ws, "_dashboard_local_update_managed_externally", lambda: True) + monkeypatch.setattr(_web_server_files, "_dashboard_local_update_managed_externally", lambda: True) monkeypatch.setattr( - ws, + _cfg_mod, "detect_install_method", lambda *a, **k: pytest.fail( "managed runtime update check should not probe install method" @@ -1012,11 +1016,11 @@ def test_spawn_hermes_action_scrubs_gateway_loop_guard_env(monkeypatch, tmp_path import hermes_cli.web_server as ws monkeypatch.setenv("_HERMES_GATEWAY", "1") - monkeypatch.setattr(ws, "_ACTION_LOG_DIR", tmp_path) + monkeypatch.setattr(_web_server_gateway, "_ACTION_LOG_DIR", tmp_path) # Isolate the module-global proc registry: _spawn_hermes_action stores # _FakeProc (no poll()) in _ACTION_PROCS, and later tests' lifespan # shutdown (_terminate_desktop_managed_gateway) would trip over it. - monkeypatch.setattr(ws, "_ACTION_PROCS", {}) + monkeypatch.setattr(_web_server_gateway, "_ACTION_PROCS", {}) captured = {} @@ -1029,7 +1033,7 @@ def test_spawn_hermes_action_scrubs_gateway_loop_guard_env(monkeypatch, tmp_path monkeypatch.setattr(ws.subprocess, "Popen", _fake_popen) - ws._spawn_hermes_action(["gateway", "restart"], "gateway-restart") + _web_server_gateway._spawn_hermes_action(["gateway", "restart"], "gateway-restart") assert "_HERMES_GATEWAY" not in captured["env"] assert captured["env"]["HERMES_NONINTERACTIVE"] == "1" @@ -1061,7 +1065,7 @@ def test_desktop_lifespan_reaps_orphan_gateways_on_startup( monkeypatch.setenv("HERMES_DESKTOP", "1") # Keep the lifespan cheap: don't re-import the gateway module or spin up the # real cron scheduler thread. - monkeypatch.setattr(ws, "_warm_gateway_module", lambda: None) + monkeypatch.setattr(_web_server_lifecycle, "_warm_gateway_module", lambda: None) monkeypatch.setattr(ws, "_start_desktop_cron_ticker", lambda *_args: None) # web_server imports the reaper lazily from hermes_cli.gateway, so patch it # on that module. @@ -1090,9 +1094,9 @@ def test_desktop_lifespan_terminates_managed_gateway_restart(monkeypatch): calls.append("terminate") monkeypatch.setenv("HERMES_DESKTOP", "1") - monkeypatch.setattr(ws, "_warm_gateway_module", lambda: None) + monkeypatch.setattr(_web_server_lifecycle, "_warm_gateway_module", lambda: None) monkeypatch.setattr(ws, "_start_desktop_cron_ticker", lambda *_args: None) - monkeypatch.setitem(ws._ACTION_PROCS, "gateway-restart", _FakeRunningProc()) + monkeypatch.setitem(_web_server_gateway._ACTION_PROCS, "gateway-restart", _FakeRunningProc()) client, _header = _client() with client: diff --git a/tests/hermes_cli/test_managed_installs.py b/tests/hermes_cli/test_managed_installs.py index f942ffbc16..94a01b0cdf 100644 --- a/tests/hermes_cli/test_managed_installs.py +++ b/tests/hermes_cli/test_managed_installs.py @@ -3,7 +3,7 @@ from unittest.mock import patch from hermes_cli.config import recommended_update_command from hermes_cli.main import cmd_update -from tools.skills_hub import OptionalSkillSource +from tools.skills_hub_official import OptionalSkillSource def test_recommended_update_command_defaults_to_hermes_update(monkeypatch): diff --git a/tests/hermes_cli/test_skills_hub.py b/tests/hermes_cli/test_skills_hub.py index d3287f9a06..cb137f070b 100644 --- a/tests/hermes_cli/test_skills_hub.py +++ b/tests/hermes_cli/test_skills_hub.py @@ -71,24 +71,25 @@ def _capture(source_filter: str = "all") -> str: def _capture_check(monkeypatch, results, name=None) -> str: - import tools.skills_hub as hub + import tools.skills_hub_install as hub_install sink = StringIO() console = Console(file=sink, force_terminal=False, color_system=None) - monkeypatch.setattr(hub, "check_for_skill_updates", lambda **_kwargs: results) + monkeypatch.setattr(hub_install, "check_for_skill_updates", lambda **_kwargs: results) do_check(name=name, console=console) return sink.getvalue() def _capture_update(monkeypatch, results) -> tuple[str, list[tuple[str, str, bool]]]: import tools.skills_hub as hub + import tools.skills_hub_install as hub_install import hermes_cli.skills_hub as cli_hub sink = StringIO() console = Console(file=sink, force_terminal=False, color_system=None) installs = [] - monkeypatch.setattr(hub, "check_for_skill_updates", lambda **_kwargs: results) + monkeypatch.setattr(hub_install, "check_for_skill_updates", lambda **_kwargs: results) monkeypatch.setattr(hub, "HubLockFile", lambda: type("L", (), { "get_installed": lambda self, name: {"install_path": "category/" + name} })()) @@ -145,7 +146,7 @@ def test_check_for_skill_updates_does_not_fall_back_across_registries(): the old code reports `update_available` (sourced from the wrong registry) while the fixed code reports `unavailable`. """ - from tools.skills_hub import check_for_skill_updates + from tools.skills_hub_install import check_for_skill_updates class _ForeignBundle: name = "reddit" @@ -195,7 +196,7 @@ def test_resolve_does_not_pair_catalog_meta_with_foreign_same_name_bundle(): showed the wrong skill. """ from hermes_cli.skills_hub import _resolve_source_meta_and_bundle - from tools.skills_hub import SkillBundle, SkillMeta + from tools.skills_hub_models import SkillBundle, SkillMeta class CatalogSource: def inspect(self, identifier): @@ -246,7 +247,7 @@ def test_resolve_does_not_pair_catalog_meta_with_foreign_same_name_bundle(): def test_resolve_keeps_catalog_meta_when_later_sources_do_not_fetch(): from hermes_cli.skills_hub import _resolve_source_meta_and_bundle - from tools.skills_hub import SkillMeta + from tools.skills_hub_models import SkillMeta class CatalogSource: def inspect(self, identifier): @@ -318,6 +319,8 @@ def _make_url_bundle_fetcher(name="", awaiting_name=True, url="https://example.c def _install_mocks(monkeypatch, tmp_path, source_factory, category_hint=""): """Wire the minimum set of monkeypatches for a do_install dry run.""" import tools.skills_hub as hub + import tools.skills_hub_install as hub_install + import tools.skills_hub_search as hub_search import tools.skills_guard as guard q_path = tmp_path / "skills" / ".hub" / "quarantine" / "pending" @@ -332,9 +335,9 @@ def _install_mocks(monkeypatch, tmp_path, source_factory, category_hint=""): return install_dir monkeypatch.setattr(hub, "ensure_hub_dirs", lambda: None) - monkeypatch.setattr(hub, "create_source_router", lambda auth: [source_factory()]) - monkeypatch.setattr(hub, "quarantine_bundle", lambda bundle: q_path) - monkeypatch.setattr(hub, "install_from_quarantine", _install_from_quarantine) + monkeypatch.setattr(hub_search, "create_source_router", lambda auth: [source_factory()]) + monkeypatch.setattr(hub_install, "quarantine_bundle", lambda bundle: q_path) + monkeypatch.setattr(hub_install, "install_from_quarantine", _install_from_quarantine) monkeypatch.setattr( hub, "HubLockFile", lambda: type("Lock", (), {"get_installed": lambda self, n: None})(), @@ -392,9 +395,9 @@ def test_do_search_json_flag_emits_full_identifiers(capsys): sink = StringIO() console = Console(file=sink, force_terminal=False, color_system=None, width=40) - with patch("tools.skills_hub.unified_search", return_value=[_LONG_RESULT]), \ - patch("tools.skills_hub.create_source_router", return_value={}), \ - patch("tools.skills_hub.GitHubAuth"): + with patch("tools.skills_hub_search.unified_search", return_value=[_LONG_RESULT]), \ + patch("tools.skills_hub_search.create_source_router", return_value={}), \ + patch("tools.skills_hub_github.GitHubAuth"): do_search("weather", console=console, as_json=True) # JSON goes to stdout via print(), not the Rich console sink. @@ -422,6 +425,7 @@ def _update_env(monkeypatch, tmp_path, *, edit_after_install: bool): """ import hermes_cli.skills_hub as cli_hub import tools.skills_hub as hub + import tools.skills_hub_install as hub_install from tools.skills_guard import content_hash skills_dir = tmp_path / "skills" @@ -434,7 +438,7 @@ def _update_env(monkeypatch, tmp_path, *, edit_after_install: bool): (skill_dir / "SKILL.md").write_text("# hub-skill\nuser edited\n") monkeypatch.setattr(hub, "SKILLS_DIR", skills_dir) - monkeypatch.setattr(hub, "check_for_skill_updates", lambda **_kwargs: [{ + monkeypatch.setattr(hub_install, "check_for_skill_updates", lambda **_kwargs: [{ "name": "hub-skill", "identifier": "someone/hub-skill", "source": "github", diff --git a/tests/hermes_cli/test_skills_skip_confirm.py b/tests/hermes_cli/test_skills_skip_confirm.py index caf131b27e..b70132e4ef 100644 --- a/tests/hermes_cli/test_skills_skip_confirm.py +++ b/tests/hermes_cli/test_skills_skip_confirm.py @@ -58,8 +58,8 @@ class TestDoInstallSkipConfirm: from hermes_cli.skills_hub import do_install with patch("hermes_cli.skills_hub._console"), \ patch("tools.skills_hub.ensure_hub_dirs"), \ - patch("tools.skills_hub.GitHubAuth"), \ - patch("tools.skills_hub.create_source_router") as mock_router, \ + patch("tools.skills_hub_github.GitHubAuth"), \ + patch("tools.skills_hub_search.create_source_router") as mock_router, \ patch("hermes_cli.skills_hub._resolve_short_name", return_value="test/skill"), \ patch("hermes_cli.skills_hub._resolve_source_meta_and_bundle") as mock_resolve: @@ -77,7 +77,7 @@ class TestDoUninstallSkipConfirm: """With skip_confirm=True, input() should not be called.""" from hermes_cli.skills_hub import do_uninstall with patch("hermes_cli.skills_hub._console") as mock_console, \ - patch("tools.skills_hub.uninstall_skill", return_value=(True, "Removed")) as mock_uninstall, \ + patch("tools.skills_hub_install.uninstall_skill", return_value=(True, "Removed")) as mock_uninstall, \ patch("builtins.input") as mock_input: do_uninstall("test-skill", skip_confirm=True) mock_input.assert_not_called() diff --git a/tests/tools/test_pr_6656_regressions.py b/tests/tools/test_pr_6656_regressions.py index c3f10a4406..d51a43eb6a 100644 --- a/tests/tools/test_pr_6656_regressions.py +++ b/tests/tools/test_pr_6656_regressions.py @@ -26,11 +26,8 @@ from unittest.mock import patch import pytest -from tools.skills_hub import ( - SkillBundle, - bundle_content_hash, - uninstall_skill, -) +from tools.skills_hub_install import bundle_content_hash, uninstall_skill +from tools.skills_hub_models import SkillBundle from tools.skills_guard import content_hash diff --git a/tests/tools/test_skill_bundle_provenance.py b/tests/tools/test_skill_bundle_provenance.py index 1d5e6e02cf..94ba502a93 100644 --- a/tests/tools/test_skill_bundle_provenance.py +++ b/tests/tools/test_skill_bundle_provenance.py @@ -13,7 +13,10 @@ import pytest from rich.console import Console from tools.skills_guard import SCANNER_VERSION, scan_skill_cached -from tools.skills_hub import GitHubAuth, GitHubSource, HubLockFile, SkillBundle, UrlSource +from tools.skills_hub import HubLockFile +from tools.skills_hub_github import GitHubAuth, GitHubSource +from tools.skills_hub_models import SkillBundle +from tools.skills_hub_sources import UrlSource SKILL_MD = """--- @@ -144,7 +147,7 @@ def test_same_dir_traversal_link_is_rejected(monkeypatch): def test_same_dir_link_without_extension_is_ignored(monkeypatch): """Prose targets that aren't file links (no extension) never fetch.""" - from tools.skills_hub import _referenced_support_paths + from tools.skills_hub_models import _referenced_support_paths skill = "---\nname: x\ndescription: x\n---\nsee [notes](NOTES) and `README`\n" assert _referenced_support_paths(skill) == set() @@ -152,7 +155,7 @@ def test_same_dir_link_without_extension_is_ignored(monkeypatch): def test_same_dir_link_query_and_fragment_are_stripped(): """?query and #fragment never leak into the fetched bundle path.""" - from tools.skills_hub import _referenced_support_paths + from tools.skills_hub_models import _referenced_support_paths skill = ( "---\nname: x\ndescription: x\n---\n" @@ -163,7 +166,7 @@ def test_same_dir_link_query_and_fragment_are_stripped(): def test_case_variant_of_skill_md_is_never_a_sibling_entry(): """skill.md must not ship as a bundle file (case-insensitive FS collision).""" - from tools.skills_hub import _referenced_support_paths + from tools.skills_hub_models import _referenced_support_paths skill = "---\nname: x\ndescription: x\n---\n[home](skill.md)\n" assert _referenced_support_paths(skill) == set() @@ -171,7 +174,7 @@ def test_case_variant_of_skill_md_is_never_a_sibling_entry(): def test_case_folded_sibling_collision_drops_the_pair(): """A.md + a.md would collide on install — neither ships.""" - from tools.skills_hub import _referenced_support_paths + from tools.skills_hub_models import _referenced_support_paths skill = "---\nname: x\ndescription: x\n---\n[a](A.md) [a2](a.md)\n" assert _referenced_support_paths(skill) == set() @@ -329,14 +332,13 @@ def test_lock_file_persists_scan_provenance(tmp_path): def test_real_temp_repo_and_home_install_e2e(served_repo, monkeypatch, tmp_path): from hermes_cli.skills_hub import do_install - import tools.skills_hub as hub _repo, url = served_repo home = tmp_path / "home" monkeypatch.setenv("HERMES_HOME", str(home)) monkeypatch.setattr("tools.skills_hub.is_safe_url", lambda _url: True) monkeypatch.setattr("tools.skills_hub.check_website_access", lambda _url: None) - monkeypatch.setattr(hub, "create_source_router", lambda auth=None: [UrlSource()]) + monkeypatch.setattr("tools.skills_hub_search.create_source_router", lambda auth=None: [UrlSource()]) sink = StringIO() do_install(url, console=Console(file=sink, force_terminal=False), skip_confirm=True) @@ -386,7 +388,6 @@ def test_install_with_junctioned_skills_dir(served_repo, monkeypatch, tmp_path): entry without a content_hash (which then poisons 'hermes skills check'). """ from hermes_cli.skills_hub import do_install - import tools.skills_hub as hub _repo, url = served_repo home = tmp_path / "home" @@ -400,7 +401,7 @@ def test_install_with_junctioned_skills_dir(served_repo, monkeypatch, tmp_path): monkeypatch.setenv("HERMES_HOME", str(home)) monkeypatch.setattr("tools.skills_hub.is_safe_url", lambda _url: True) monkeypatch.setattr("tools.skills_hub.check_website_access", lambda _url: None) - monkeypatch.setattr(hub, "create_source_router", lambda auth=None: [UrlSource()]) + monkeypatch.setattr("tools.skills_hub_search.create_source_router", lambda auth=None: [UrlSource()]) sink = StringIO() do_install(url, console=Console(file=sink, force_terminal=False), skip_confirm=True) @@ -469,14 +470,13 @@ def test_install_skips_unreachable_support_file_e2e(served_repo_missing_support, scan, install, and lock provenance, with only the reachable files landing on disk and recorded in the lock file (#66760).""" from hermes_cli.skills_hub import do_install - import tools.skills_hub as hub _repo, url = served_repo_missing_support home = tmp_path / "home" monkeypatch.setenv("HERMES_HOME", str(home)) monkeypatch.setattr("tools.skills_hub.is_safe_url", lambda _url: True) monkeypatch.setattr("tools.skills_hub.check_website_access", lambda _url: None) - monkeypatch.setattr(hub, "create_source_router", lambda auth=None: [UrlSource()]) + monkeypatch.setattr("tools.skills_hub_search.create_source_router", lambda auth=None: [UrlSource()]) sink = StringIO() do_install(url, console=Console(file=sink, force_terminal=False), skip_confirm=True) @@ -499,7 +499,7 @@ def test_install_skips_unreachable_support_file_e2e(served_repo_missing_support, def test_bundled_optional_source_still_includes_support_files(tmp_path, monkeypatch): - from tools.skills_hub import OptionalSkillSource + from tools.skills_hub_official import OptionalSkillSource root = tmp_path / "optional-skills" skill = root / "category" / "official-demo" @@ -530,7 +530,8 @@ metadata: def test_optional_source_upstream_stub_fetches_from_external_repo(tmp_path, monkeypatch): """A catalog stub with metadata.hermes.upstream installs the upstream repo's content (relabelled official/trusted), not the stub itself.""" - from tools.skills_hub import OptionalSkillSource, SkillBundle + from tools.skills_hub_models import SkillBundle + from tools.skills_hub_official import OptionalSkillSource root = tmp_path / "optional-skills" skill = root / "creative" / "upstream-demo" @@ -569,7 +570,7 @@ def test_optional_source_upstream_stub_fetches_from_external_repo(tmp_path, monk def test_optional_source_upstream_pointer_rejects_malformed(tmp_path): - from tools.skills_hub import OptionalSkillSource + from tools.skills_hub_official import OptionalSkillSource source = OptionalSkillSource() bad = [ @@ -587,7 +588,8 @@ def test_unified_search_trust_rank_survives_limit_cut(): """Official/builtin results must survive the limit truncation even when a high-volume community source floods the merged list first.""" from unittest.mock import patch as _patch - from tools.skills_hub import unified_search, SkillMeta + from tools.skills_hub_models import SkillMeta + from tools.skills_hub_search import unified_search community = [ SkillMeta(name=f"s{i}", description="", source="skills.sh", @@ -597,7 +599,7 @@ def test_unified_search_trust_rank_survives_limit_cut(): official = [SkillMeta(name="s-official", description="", source="official", identifier="official/cat/s-official", trust_level="builtin")] - with _patch("tools.skills_hub.parallel_search_sources", + with _patch("tools.skills_hub_search.parallel_search_sources", return_value=(community + official, {}, [])): results = unified_search("s", [], source_filter="all", limit=10) diff --git a/tests/tools/test_skills_hub.py b/tests/tools/test_skills_hub.py index 41de581486..3abbe3e68d 100644 --- a/tests/tools/test_skills_hub.py +++ b/tests/tools/test_skills_hub.py @@ -8,28 +8,18 @@ from unittest.mock import patch, MagicMock import httpx import pytest -from tools.skills_hub import ( - GitHubAuth, - GitHubSource, - LobeHubSource, - SkillsShSource, - UrlSource, - WellKnownSkillSource, - OptionalSkillSource, - SkillSource, - SkillBundle, - SkillMeta, - HubLockFile, - TapsManager, - bundle_content_hash, - check_for_skill_updates, - create_source_router, - parallel_search_sources, - unified_search, - append_audit_log, - quarantine_bundle, - _referenced_support_paths, +from tools.skills_hub import HubLockFile, TapsManager, append_audit_log +from tools.skills_hub_github import GitHubAuth, GitHubSource +from tools.skills_hub_install import ( + bundle_content_hash, check_for_skill_updates, install_from_quarantine, quarantine_bundle, ) +from tools.skills_hub_models import SkillBundle, SkillMeta, SkillSource, _referenced_support_paths +from tools.skills_hub_official import OptionalSkillSource +from tools.skills_hub_search import ( + HERMES_INDEX_TTL, _load_hermes_index, create_source_router, parallel_search_sources, unified_search, +) +from tools.skills_hub_skillssh import SkillsShSource +from tools.skills_hub_sources import LobeHubSource, UrlSource, WellKnownSkillSource # --------------------------------------------------------------------------- @@ -720,7 +710,7 @@ class TestGithubProviderLabeling: def _make_index_source(skills): """Build a HermesIndexSource pre-loaded with a fixed skill list.""" - from tools.skills_hub import HermesIndexSource + from tools.skills_hub_official import HermesIndexSource src = HermesIndexSource(auth=GitHubAuth()) src._index = {"skills": skills} src._loaded = True @@ -757,7 +747,7 @@ class TestHermesIndexSearch: class TestProviderFilter: def test_filter_results_by_provider_narrows_exactly(self): - from tools.skills_hub import _filter_results_by_provider + from tools.skills_hub_github import _filter_results_by_provider results = [ SkillMeta(name="a", description="", source="github", identifier="NVIDIA/skills/a", trust_level="trusted", extra={"provider": "NVIDIA"}), @@ -1136,7 +1126,7 @@ class TestQuarantineBundleBinaryAssets: """The colon guard covers the whole class, not just ``helper.py:payload``: a colon in any component (leading drive letter, mid-path, or bare) is rejected, while ordinary portable paths still normalize.""" - from tools.skills_hub import _normalize_bundle_path + from tools.skills_hub_models import _normalize_bundle_path rejected = ( "scripts/helper.py:payload", # trailing-component ADS marker @@ -1235,7 +1225,7 @@ class TestInstallPathSafety: def test_uninstall_rejects_poisoned_absolute_path(self, tmp_path, isolated_skills_dir, patch_lock_file): """Hand-edited lock.json with absolute install_path must not delete anything.""" - from tools.skills_hub import uninstall_skill + from tools.skills_hub_install import uninstall_skill lock_path = tmp_path / "lock.json" target = tmp_path / "victim" @@ -1268,7 +1258,7 @@ class TestInstallPathSafety: assert (target / "file.txt").read_text() == "important" def test_uninstall_rejects_traversal(self, tmp_path, isolated_skills_dir, patch_lock_file): - from tools.skills_hub import uninstall_skill + from tools.skills_hub_install import uninstall_skill lock_path = tmp_path / "lock.json" sibling = tmp_path / "sibling" @@ -1296,7 +1286,7 @@ class TestInstallPathSafety: def test_uninstall_rejects_empty_install_path(self, tmp_path, isolated_skills_dir, patch_lock_file): """Empty install_path resolves to SKILLS_DIR itself — must be refused.""" - from tools.skills_hub import uninstall_skill + from tools.skills_hub_install import uninstall_skill # Put a sibling skill alongside to prove rmtree doesn't fire. (isolated_skills_dir / "bystander").mkdir() @@ -1345,7 +1335,7 @@ class TestInstallPathSafety: except (OSError, NotImplementedError): pytest.skip("symlink creation unsupported on this platform") - bundle = hub.SkillBundle( + bundle = SkillBundle( name="bad-skill", files={"SKILL.md": "---\nname: bad-skill\n---\n"}, source="community", @@ -1363,7 +1353,7 @@ class TestInstallPathSafety: patch.object(hub, "QUARANTINE_DIR", quarantine_root), \ patch("tools.skill_usage.record_installed") as record_installed: with pytest.raises(ValueError, match="symlink"): - hub.install_from_quarantine( + install_from_quarantine( q_dir, "bad-skill", "", bundle, scan_result, ) @@ -1399,7 +1389,7 @@ class TestInstallPathSafety: q_dir.mkdir() (q_dir / "SKILL.md").write_text("---\nname: research\n---\n") - bundle = hub.SkillBundle( + bundle = SkillBundle( name="research", files={"SKILL.md": "---\nname: research\n---\n"}, source="community", @@ -1416,7 +1406,7 @@ class TestInstallPathSafety: with patch.object(hub, "SKILLS_DIR", skills_dir), \ patch.object(hub, "QUARANTINE_DIR", quarantine_root): with pytest.raises(ValueError, match="Refusing to overwrite category directory"): - hub.install_from_quarantine( + install_from_quarantine( q_dir, "research", "", bundle, scan_result, ) @@ -1451,7 +1441,7 @@ class TestInstallPathSafety: (q_dir / "refs").mkdir() (q_dir / "refs" / "guide.md").write_text("new guide") - bundle = hub.SkillBundle( + bundle = SkillBundle( name="my-skill", files={"SKILL.md": "---\nname: my-skill\n---\nnew"}, source="community", @@ -1467,7 +1457,7 @@ class TestInstallPathSafety: with patch.object(hub, "SKILLS_DIR", skills_dir), \ patch.object(hub, "QUARANTINE_DIR", quarantine_root): - installed = hub.install_from_quarantine( + installed = install_from_quarantine( q_dir, "my-skill", "", bundle, scan_result, ) @@ -1495,7 +1485,7 @@ class TestInstallPathSafety: q_dir.mkdir() (q_dir / "SKILL.md").write_text("---\nname: research\n---\n") - bundle = hub.SkillBundle( + bundle = SkillBundle( name="research", files={"SKILL.md": "---\nname: research\n---\n"}, source="community", @@ -1511,7 +1501,7 @@ class TestInstallPathSafety: with patch.object(hub, "SKILLS_DIR", skills_dir), \ patch.object(hub, "QUARANTINE_DIR", quarantine_root): - installed = hub.install_from_quarantine( + installed = install_from_quarantine( q_dir, "research", "", bundle, scan_result, ) @@ -1539,7 +1529,7 @@ class TestInstallPathSafety: q_dir.mkdir() (q_dir / "SKILL.md").write_text("---\nname: mlops\n---\n") - bundle = hub.SkillBundle( + bundle = SkillBundle( name="mlops", files={"SKILL.md": "---\nname: mlops\n---\n"}, source="community", @@ -1556,7 +1546,7 @@ class TestInstallPathSafety: with patch.object(hub, "SKILLS_DIR", skills_dir), \ patch.object(hub, "QUARANTINE_DIR", quarantine_root): with pytest.raises(ValueError, match="category directory"): - hub.install_from_quarantine( + install_from_quarantine( q_dir, "mlops", "", bundle, scan_result, ) @@ -1584,7 +1574,7 @@ class TestInstallPathSafety: q_dir.mkdir() (q_dir / "SKILL.md").write_text("---\nname: docker\n---\n") - bundle = hub.SkillBundle( + bundle = SkillBundle( name="docker", files={"SKILL.md": "---\nname: docker\n---\n"}, source="community", @@ -1601,7 +1591,7 @@ class TestInstallPathSafety: with patch.object(hub, "SKILLS_DIR", skills_dir), \ patch.object(hub, "QUARANTINE_DIR", quarantine_root): with pytest.raises(ValueError, match="existing skill directory"): - hub.install_from_quarantine( + install_from_quarantine( q_dir, "docker", "devops", bundle, scan_result, ) @@ -1625,7 +1615,7 @@ class TestInstallPathSafety: q_dir.mkdir() (q_dir / "SKILL.md").write_text("---\nname: notes\n---\n") - bundle = hub.SkillBundle( + bundle = SkillBundle( name="notes", files={"SKILL.md": "---\nname: notes\n---\n"}, source="community", @@ -1642,7 +1632,7 @@ class TestInstallPathSafety: with patch.object(hub, "SKILLS_DIR", skills_dir), \ patch.object(hub, "QUARANTINE_DIR", quarantine_root): with pytest.raises(ValueError, match="not a directory"): - hub.install_from_quarantine( + install_from_quarantine( q_dir, "notes", "", bundle, scan_result, ) @@ -1658,7 +1648,7 @@ class TestInstallPathSafety: q_dir.mkdir(parents=True) skill_md = "---\nname: good-skill\n---\n\n# Good skill\n" (q_dir / "SKILL.md").write_text(skill_md, encoding="utf-8") - bundle = hub.SkillBundle( + bundle = SkillBundle( name="good-skill", files={"SKILL.md": skill_md}, source="community", @@ -1675,7 +1665,7 @@ class TestInstallPathSafety: with patch.object(hub, "SKILLS_DIR", skills_dir), \ patch.object(hub, "QUARANTINE_DIR", quarantine_root), \ patch("tools.skill_usage.record_installed") as record_installed: - installed = hub.install_from_quarantine( + installed = install_from_quarantine( q_dir, "good-skill", "", @@ -1784,15 +1774,13 @@ class TestLoadHermesIndex: @staticmethod def _isolate_cache(monkeypatch, tmp_path): """Point the on-disk cache at an empty tmp dir so no real cache leaks in.""" - import tools.skills_hub as hub - cache_file = tmp_path / "hermes-index.json" - monkeypatch.setattr(hub, "_hermes_index_cache_file", lambda: cache_file) + monkeypatch.setattr("tools.skills_hub_search._hermes_index_cache_file", lambda: cache_file) return cache_file def test_fetch_does_not_request_brotli(self, monkeypatch, tmp_path): """The index fetch must not negotiate Brotli (the broken decoder path).""" - import tools.skills_hub as hub + import tools.skills_hub_search as hub_search self._isolate_cache(monkeypatch, tmp_path) @@ -1805,9 +1793,9 @@ class TestLoadHermesIndex: resp.json.return_value = {"skills": [{"name": "x"}]} return resp - monkeypatch.setattr(hub.httpx, "get", fake_get) + monkeypatch.setattr(hub_search.httpx, "get", fake_get) - data = hub._load_hermes_index() + data = _load_hermes_index() assert data == {"skills": [{"name": "x"}]} accept = captured["headers"].get("Accept-Encoding", "") @@ -1819,12 +1807,12 @@ class TestLoadHermesIndex: self, monkeypatch, tmp_path ): """If every attempt fails to decode, serve the stale cache rather than None.""" - import tools.skills_hub as hub + import tools.skills_hub_search as hub_search cache_file = self._isolate_cache(monkeypatch, tmp_path) cache_file.write_text(json.dumps({"skills": [{"name": "stale"}]})) # Force the cache to look expired so the network path runs. - old = time.time() - (hub.HERMES_INDEX_TTL + 100) + old = time.time() - (HERMES_INDEX_TTL + 100) import os os.utime(cache_file, (old, old)) @@ -1832,9 +1820,9 @@ class TestLoadHermesIndex: def fake_get(url, *args, **kwargs): raise httpx.DecodingError("brotli boom") - monkeypatch.setattr(hub.httpx, "get", fake_get) + monkeypatch.setattr(hub_search.httpx, "get", fake_get) - data = hub._load_hermes_index() + data = _load_hermes_index() assert data == {"skills": [{"name": "stale"}]} diff --git a/tests/tools/test_skills_hub_browse_sh.py b/tests/tools/test_skills_hub_browse_sh.py index 4a49891d0f..0dc11e25a4 100644 --- a/tests/tools/test_skills_hub_browse_sh.py +++ b/tests/tools/test_skills_hub_browse_sh.py @@ -3,7 +3,8 @@ import unittest from unittest.mock import patch -from tools.skills_hub import BrowseShSource, SkillMeta, SkillBundle +from tools.skills_hub_models import SkillBundle, SkillMeta +from tools.skills_hub_sources import BrowseShSource # Catalog shape mirrors the real ``GET https://browse.sh/api/skills`` response: diff --git a/tests/tools/test_skills_hub_clawhub.py b/tests/tools/test_skills_hub_clawhub.py index 599fe02d8a..90a682d8dc 100644 --- a/tests/tools/test_skills_hub_clawhub.py +++ b/tests/tools/test_skills_hub_clawhub.py @@ -3,7 +3,8 @@ import unittest from unittest.mock import patch -from tools.skills_hub import ClawHubSource, SkillMeta +from tools.skills_hub_clawhub import ClawHubSource +from tools.skills_hub_models import SkillMeta class _MockResponse: diff --git a/tests/tools/test_windows_agent_loop_papercuts.py b/tests/tools/test_windows_agent_loop_papercuts.py index 0b2c04c590..820590355a 100644 --- a/tests/tools/test_windows_agent_loop_papercuts.py +++ b/tests/tools/test_windows_agent_loop_papercuts.py @@ -140,7 +140,8 @@ class TestSkillHashSymmetry: def test_disk_and_bundle_hashes_match(self, tmp_path): from tools.skills_guard import content_hash - from tools.skills_hub import SkillBundle, bundle_content_hash + from tools.skills_hub_install import bundle_content_hash + from tools.skills_hub_models import SkillBundle skill = self._make_skill(tmp_path) disk = content_hash(skill) @@ -157,7 +158,8 @@ class TestSkillHashSymmetry: assert bundle_content_hash(bundle) == disk def test_backslash_and_posix_keys_hash_identically(self): - from tools.skills_hub import SkillBundle, bundle_content_hash + from tools.skills_hub_install import bundle_content_hash + from tools.skills_hub_models import SkillBundle posix = SkillBundle( name="s", diff --git a/tests/tui_gateway/test_protocol.py b/tests/tui_gateway/test_protocol.py index b0b7f4825a..0c925ef42b 100644 --- a/tests/tui_gateway/test_protocol.py +++ b/tests/tui_gateway/test_protocol.py @@ -178,6 +178,7 @@ def test_write_json(capture): def test_live_session_payload_replays_pending_approval(server, monkeypatch): """A reattached client receives the approval that was emitted while detached.""" from tools import approval + from tools import approval_gateway_wait session = { "agent": types.SimpleNamespace(), @@ -196,8 +197,8 @@ def test_live_session_payload_replays_pending_approval(server, monkeypatch): second = {"command": "rm -rf /tmp/later", "description": "later"} saved_queue = approval._gateway_queues.pop("stored-session", None) approval._gateway_queues["stored-session"] = [ - approval._ApprovalEntry(first), - approval._ApprovalEntry(second), + approval_gateway_wait._ApprovalEntry(first), + approval_gateway_wait._ApprovalEntry(second), ] monkeypatch.setattr(server, "_approval_request_payload", lambda data: dict(data or {})) @@ -1263,13 +1264,10 @@ def test_skills_manage_search_uses_tools_hub_sources(server): auth = MagicMock(return_value="auth") router = MagicMock(return_value=["source"]) search = MagicMock(return_value=[result]) - fake_hub = types.SimpleNamespace( - GitHubAuth=auth, - create_source_router=router, - unified_search=search, - ) + fake_search = types.SimpleNamespace(create_source_router=router, unified_search=search) + fake_github = types.SimpleNamespace(GitHubAuth=auth) - with patch.dict(sys.modules, {"tools.skills_hub": fake_hub}): + with patch.dict(sys.modules, {"tools.skills_hub_search": fake_search, "tools.skills_hub_github": fake_github}): resp = server.handle_request({ "id": "skills-search", "method": "skills.manage", diff --git a/tools/skills_guard.py b/tools/skills_guard.py index 1155736864..c228978e20 100644 --- a/tools/skills_guard.py +++ b/tools/skills_guard.py @@ -457,7 +457,7 @@ def _content_digest(skill_path: Path) -> str: def content_hash(skill_path: Path) -> str: """Short integrity hash (paths mixed in, so swapping two files' contents changes it). MUST stay symmetric - with ``tools.skills_hub.bundle_content_hash`` — change both at once.""" + with ``tools.skills_hub_install.bundle_content_hash`` — change both at once.""" return f"sha256:{_content_digest(skill_path)[:16]}" diff --git a/tools/skills_hub.py b/tools/skills_hub.py index 74ecb088df..d0d9f5cd0c 100644 --- a/tools/skills_hub.py +++ b/tools/skills_hub.py @@ -6,8 +6,7 @@ Library module (not an agent tool). Owns the hub paths, guarded HTTP, index cache, lock file, taps and audit log. Install/uninstall/update live in ``skills_hub_install``, the index fetch/source router/search in ``skills_hub_search``, and the adapters in the other ``tools.skills_hub_*`` -siblings; all are re-exported here so ``from tools.skills_hub import X`` keeps -working (and stays the test patch target). +siblings; import each name from its defining module. Used by hermes_cli/skills_hub.py for CLI commands and the /skills slash command. """ @@ -25,32 +24,7 @@ import httpx from hermes_constants import get_hermes_home from tools.url_safety import is_safe_url from tools.website_policy import check_website_access -from tools.skills_hub_models import ( # noqa: F401 (re-exported public API) - SkillMeta, SkillBundle, SkillSource, source_url_for_bundle, _referenced_support_paths, - _normalize_bundle_path, _validate_skill_name, _validate_install_parent_path, - _normalize_lock_install_path, _validate_bundle_rel_path, _skill_meta_to_dict, - _parse_frontmatter, _dedupe_by_trust, TRUST_RANK, -) -from tools.skills_hub_github import ( # noqa: F401 - GITHUB_TAP_PROVIDERS, github_provider_for, _PROVIDER_FILTER_VALUES, - _filter_results_by_provider, GitHubAuth, GitHubSource, -) -from tools.skills_hub_skillssh import SkillsShSource # noqa: F401 -from tools.skills_hub_clawhub import ClawHubSource # noqa: F401 -from tools.skills_hub_sources import ( # noqa: F401 - WellKnownSkillSource, UrlSource, LobeHubSource, BrowseShSource, -) -from tools.skills_hub_official import OptionalSkillSource, HermesIndexSource # noqa: F401 -from tools.skills_hub_search import ( # noqa: F401 (re-exported; tests patch tools.skills_hub.) - HERMES_INDEX_TTL, HERMES_INDEX_URL, _API_SOURCE_IDS, _hermes_index_cache_file, - _load_hermes_index, _load_stale_index_cache, _search_one_source, _select_active_sources, - create_source_router, parallel_search_sources, unified_search, -) -from tools.skills_hub_install import ( # noqa: F401 (re-exported; tests patch tools.skills_hub.) - _SOURCE_ID_ALIASES, _category_skill_dirs, _check_install_target, _is_path_redirect, - _resolve_lock_install_path, _source_matches, bundle_content_hash, check_for_skill_updates, - install_from_quarantine, quarantine_bundle, uninstall_skill, -) +from tools.skills_hub_models import _normalize_lock_install_path, _validate_skill_name logger = logging.getLogger(__name__) @@ -60,8 +34,7 @@ logger = logging.getLogger(__name__) # --------------------------------------------------------------------------- # Resolved per-call (not frozen at import) so the profile override is honored; # import-time constants leaked across profiles in single-process multi-profile -# runtimes. Legacy names (SKILLS_DIR, ...) are re-exposed via __getattr__ below -# so external `from tools.skills_hub import SKILLS_DIR` callers still work. +# runtimes. The path names (SKILLS_DIR, ...) resolve live via __getattr__ below. INDEX_CACHE_TTL = 3600 # 1 hour diff --git a/tools/skills_hub_install.py b/tools/skills_hub_install.py index 3d55c16cbd..5c3ea99936 100644 --- a/tools/skills_hub_install.py +++ b/tools/skills_hub_install.py @@ -2,8 +2,8 @@ target safety (symlink/junction, category-bucket and nested-skill checks), lock-file-backed uninstall, bundle hashing and upstream update checks. -Split out of ``tools/skills_hub.py``; every public/patched name is re-imported there, -so ``tools.skills_hub.`` keeps resolving (and monkeypatching) as before. +Split out of ``tools/skills_hub.py``; hub state (paths, ``HubLockFile``, audit log) +is still read from there at call time. """ from __future__ import annotations @@ -259,7 +259,8 @@ def check_for_skill_updates( satisfy the fetch and silently reassign provenance (names are not namespaced across registries), so a missing adapter reports "unavailable". """ - from tools.skills_hub import HubLockFile, create_source_router + from tools.skills_hub import HubLockFile + from tools.skills_hub_search import create_source_router lock = lock or HubLockFile() installed = lock.list_installed() if name: diff --git a/tools/skills_hub_official.py b/tools/skills_hub_official.py index 55ac5a1577..73bec7682e 100644 --- a/tools/skills_hub_official.py +++ b/tools/skills_hub_official.py @@ -290,7 +290,8 @@ class HermesIndexSource(SkillSource): def _ensure_loaded(self) -> dict: if not self._loaded: - self._index, self._loaded = hub()._load_hermes_index(), True + from tools.skills_hub_search import _load_hermes_index + self._index, self._loaded = _load_hermes_index(), True return self._index or {} def _skills(self) -> list: diff --git a/tools/skills_hub_search.py b/tools/skills_hub_search.py index 505cb4cea5..d54cea1fc8 100644 --- a/tools/skills_hub_search.py +++ b/tools/skills_hub_search.py @@ -2,8 +2,8 @@ fallback), the source router, and parallel/unified search across source adapters. -Split out of ``tools/skills_hub.py``; every public/patched name is re-imported there, -so ``tools.skills_hub.`` keeps resolving (and monkeypatching) as before. +Split out of ``tools/skills_hub.py``; hub state (cache dir, ``TapsManager``, JSON +cache reads) is still read from there at call time. """ from __future__ import annotations @@ -12,14 +12,13 @@ import logging import httpx import json from pathlib import Path -from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple -from tools.skills_hub_github import _PROVIDER_FILTER_VALUES +from typing import Any, Dict, List, Optional, Tuple +from tools.skills_hub_clawhub import ClawHubSource +from tools.skills_hub_github import GitHubAuth, GitHubSource, _PROVIDER_FILTER_VALUES, _filter_results_by_provider from tools.skills_hub_models import SkillMeta, SkillSource, TRUST_RANK, _dedupe_by_trust +from tools.skills_hub_official import HermesIndexSource, OptionalSkillSource from tools.skills_hub_skillssh import SkillsShSource -from tools.skills_hub_sources import LobeHubSource, WellKnownSkillSource - -if TYPE_CHECKING: # runtime use resolves through the origin (test patch target) - from tools.skills_hub_github import GitHubAuth +from tools.skills_hub_sources import BrowseShSource, LobeHubSource, UrlSource, WellKnownSkillSource # Log-record parity with the origin module. logger = logging.getLogger("tools.skills_hub") @@ -43,7 +42,7 @@ def _load_hermes_index() -> Optional[dict]: Skills Hub. gzip/deflate first; the identity retry covers proxies that ignore the header and return Brotli anyway. """ - from tools.skills_hub import _hermes_index_cache_file, _read_json_if_fresh + from tools.skills_hub import _read_json_if_fresh cache_file = _hermes_index_cache_file() cached = _read_json_if_fresh(cache_file, HERMES_INDEX_TTL) if cached is not None: @@ -75,7 +74,7 @@ def _load_hermes_index() -> Optional[dict]: def _load_stale_index_cache() -> Optional[dict]: """Fall back to the cache regardless of age when the network fetch fails.""" - from tools.skills_hub import _hermes_index_cache_file, _read_json_if_fresh + from tools.skills_hub import _read_json_if_fresh return _read_json_if_fresh(_hermes_index_cache_file(), float("inf")) @@ -87,10 +86,7 @@ _API_SOURCE_IDS = frozenset({"github", "skills-sh", "clawhub", "lobehub", "well- def create_source_router(auth: Optional[GitHubAuth] = None) -> List[SkillSource]: """All configured source adapters, in priority order.""" - from tools.skills_hub import ( # adapters resolve via origin: tests patch them there - BrowseShSource, ClawHubSource, GitHubAuth, GitHubSource, HermesIndexSource, - OptionalSkillSource, TapsManager, UrlSource, - ) + from tools.skills_hub import TapsManager if auth is None: auth = GitHubAuth() return [ @@ -188,7 +184,6 @@ def parallel_search_sources( def unified_search(query: str, sources: List[SkillSource], source_filter: str = "all", limit: int = 10) -> List[SkillMeta]: """Search all sources (in parallel) and merge results.""" - from tools.skills_hub import _filter_results_by_provider, parallel_search_sources all_results, _, _ = parallel_search_sources(sources, query=query, source_filter=source_filter, overall_timeout=30) # Provider filters target ``extra.provider`` on the merged set, not a source id. if source_filter.strip().lower() in _PROVIDER_FILTER_VALUES: diff --git a/tui_gateway/methods_tools.py b/tui_gateway/methods_tools.py index f4c849fdaf..9d9e457f75 100644 --- a/tui_gateway/methods_tools.py +++ b/tui_gateway/methods_tools.py @@ -278,7 +278,8 @@ def _(rid, params: dict) -> dict: except Exception as exc: return _err(rid, 5019, f"compute-host reload_mcp failed: {exc}") return _ok(rid, {"status": "reloaded", "turn_isolation": True, "host_ack": ack}) - mcp_tool = _tools_mod("tools.mcp_tool") + _mcp_agent, _mcp_lifecycle, _mcp_discovery = ( + _tools_mod("tools.mcp_tool_agent"), _tools_mod("tools.mcp_tool_lifecycle"), _tools_mod("tools.mcp_tool_discovery")) global _mcp_reload_gen, _mcp_reload_loaded_rev # Revision the CALLER wants loaded; empty on legacy clients / manual /reload-mcp # (generation-only coalescing). @@ -292,7 +293,7 @@ def _(rid, params: dict) -> dict: return agent = session["agent"] try: # enabled_override re-resolves toolsets so a server enabled in config this session is picked up - mcp_tool.refresh_agent_mcp_tools(agent, enabled_override=_load_enabled_toolsets(), quiet_mode=True) + _mcp_agent.refresh_agent_mcp_tools(agent, enabled_override=_load_enabled_toolsets(), quiet_mode=True) except Exception as _exc: logger.warning("Failed to refresh cached agent tools after /reload-mcp: %s", _exc) _emit("session.info", params.get("session_id", ""), _session_info(agent, session)) @@ -304,9 +305,9 @@ def _(rid, params: dict) -> dict: global _mcp_reload_gen, _mcp_reload_loaded_rev loaded = _compute_mcp_rev() for _ in range(_MCP_RELOAD_MAX_PASSES): - mcp_tool.shutdown_mcp_servers() - mcp_tool.reprobe_tool_availability() - mcp_tool.discover_mcp_tools() + _mcp_lifecycle.shutdown_mcp_servers() + _mcp_agent.reprobe_tool_availability() + _mcp_discovery.discover_mcp_tools() after = _compute_mcp_rev() if after == loaded: break @@ -1083,8 +1084,8 @@ del _name, _fn, _keys def _skills_search(rid, params, query): - hub = _tools_mod("tools.skills_hub") - raw = hub.unified_search(query, hub.create_source_router(hub.GitHubAuth()), source_filter="all", limit=20) or [] + search, gh = _tools_mod("tools.skills_hub_search"), _tools_mod("tools.skills_hub_github") + raw = search.unified_search(query, search.create_source_router(gh.GitHubAuth()), source_filter="all", limit=20) or [] return _ok(rid, {"results": [{"name": r.name, "description": r.description} for r in raw]}) @@ -1364,7 +1365,7 @@ def _(rid, params: dict) -> dict: if not cmd: return _err(rid, 4004, "empty command") try: - approval = _tools_mod("tools.approval") + approval = _tools_mod("tools.approval_detection") is_hardline, hardline_desc = approval.detect_hardline_command(cmd) if is_hardline: return _err(rid, 4005, f"blocked (hardline): {hardline_desc}. Use the agent for dangerous commands.")