diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 08d0ac32bda7..3c8eeda7a92d 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -7031,6 +7031,27 @@ async def update_skills_hub(): } +def _skill_hub_source_entry(source_id: str, *, error: str | None = None) -> dict: + entry = { + "id": source_id, + "label": _SKILL_HUB_SOURCE_LABELS.get(source_id, source_id), + } + if source_id == "hermes-index": + entry["available"] = False + if source_id == "github": + entry["rate_limited"] = False + if error: + entry["error"] = error + return entry + + +def _default_skill_hub_source_entries(*, error: str | None = None) -> list[dict]: + return [ + _skill_hub_source_entry(source_id, error=error) + for source_id in _SKILL_HUB_SOURCE_LABELS + ] + + def _skill_meta_to_payload(m) -> dict: return { "name": m.name, @@ -7079,12 +7100,31 @@ async def list_skills_hub_sources(): def _run(): from tools.skills_hub import create_source_router - sources = create_source_router() + try: + sources = create_source_router() + except Exception as exc: + source_error = str(exc) or exc.__class__.__name__ + return { + "sources": _default_skill_hub_source_entries(error=source_error), + "index_available": False, + "featured": [], + "installed": _installed_hub_identifiers(), + } + out = [] index_available = False featured = [] for src in sources: - sid = src.source_id() + try: + sid = src.source_id() + except Exception as exc: + out.append( + _skill_hub_source_entry( + "unknown", + error=str(exc) or exc.__class__.__name__, + ) + ) + continue entry = { "id": sid, "label": _SKILL_HUB_SOURCE_LABELS.get(sid, sid), @@ -7093,13 +7133,15 @@ def _run(): if sid == "github": try: entry["rate_limited"] = bool(getattr(src, "is_rate_limited", False)) - except Exception: + except Exception as exc: entry["rate_limited"] = False + entry["error"] = str(exc) or exc.__class__.__name__ if sid == "hermes-index": try: index_available = bool(getattr(src, "is_available", False)) - except Exception: + except Exception as exc: index_available = False + entry["error"] = str(exc) or exc.__class__.__name__ entry["available"] = index_available # Empty-query search on the index returns featured/popular skills. if index_available: @@ -7107,8 +7149,9 @@ def _run(): featured = [ _skill_meta_to_payload(m) for m in src.search("", limit=12) ] - except Exception: + except Exception as exc: featured = [] + entry["error"] = str(exc) or exc.__class__.__name__ out.append(entry) return { "sources": out, @@ -7142,8 +7185,18 @@ def _run(): sources = create_source_router() capped = min(max(limit, 1), 50) + per_source_limits = {} + for src in sources: + try: + per_source_limits[src.source_id()] = capped + except Exception: + pass all_results, source_counts, timed_out = parallel_search_sources( - sources, query=query, source_filter=source or "all", overall_timeout=30 + sources, + query=query, + per_source_limits=per_source_limits, + source_filter=source or "all", + overall_timeout=30, ) # Dedupe by identifier, preferring higher trust (mirrors unified_search). diff --git a/tests/hermes_cli/test_dashboard_admin_endpoints.py b/tests/hermes_cli/test_dashboard_admin_endpoints.py index 5171f3ade05c..d9642b95de93 100644 --- a/tests/hermes_cli/test_dashboard_admin_endpoints.py +++ b/tests/hermes_cli/test_dashboard_admin_endpoints.py @@ -362,6 +362,41 @@ def test_empty_query_returns_empty(self): assert body["timed_out"] == [] assert body["installed"] == {} + def test_search_returns_partial_results_with_timeout_metadata(self, monkeypatch): + class _Src: + def __init__(self, sid): + self._sid = sid + + def source_id(self): + return self._sid + + class _Meta: + name = "fast" + description = "desc" + source = "fast" + identifier = "fast/skill" + trust_level = "community" + repo = "owner/repo" + tags = ["a"] + + def _fake_search(sources, **kwargs): + assert kwargs["per_source_limits"] == {"fast": 3, "slow": 3} + assert kwargs["source_filter"] == "all" + return [_Meta()], {"fast": 1}, ["slow"] + + monkeypatch.setattr( + "tools.skills_hub.create_source_router", + lambda: [_Src("fast"), _Src("slow")], + ) + monkeypatch.setattr("tools.skills_hub.parallel_search_sources", _fake_search) + + r = self.client.get("/api/skills/hub/search?q=foo&limit=3") + assert r.status_code == 200 + body = r.json() + assert body["results"][0]["identifier"] == "fast/skill" + assert body["source_counts"] == {"fast": 1} + assert body["timed_out"] == ["slow"] + class _FakeMeta: """Minimal SkillMeta stand-in for monkeypatched source search.""" @@ -440,6 +475,22 @@ def _fake_router(): assert body["featured"][0]["trust_level"] == "trusted" assert isinstance(body["installed"], dict) + def test_sources_returns_known_hubs_when_router_fails(self, monkeypatch): + def _boom(): + raise RuntimeError("router down") + + monkeypatch.setattr("tools.skills_hub.create_source_router", _boom) + + r = self.client.get("/api/skills/hub/sources") + assert r.status_code == 200 + body = r.json() + ids = {s["id"] for s in body["sources"]} + assert {"official", "hermes-index", "github"} <= ids + assert body["index_available"] is False + assert body["featured"] == [] + assert all(s.get("label") for s in body["sources"]) + assert all(s.get("error") == "router down" for s in body["sources"]) + class TestSkillsHubPreviewEndpoint: @pytest.fixture(autouse=True) @@ -953,4 +1004,3 @@ def test_endpoints_require_session_token(self): kwargs["json"] = payload r = fn(path, **kwargs) assert r.status_code == 401, f"{method} {path} not gated" - diff --git a/tests/tools/test_skills_hub.py b/tests/tools/test_skills_hub.py index ec2f108072aa..c789bd24a99b 100644 --- a/tests/tools/test_skills_hub.py +++ b/tests/tools/test_skills_hub.py @@ -1,6 +1,7 @@ """Tests for tools/skills_hub.py — source adapters, lock file, taps, dedup logic.""" import json +import time from unittest.mock import patch, MagicMock import httpx @@ -21,6 +22,7 @@ bundle_content_hash, check_for_skill_updates, create_source_router, + parallel_search_sources, unified_search, append_audit_log, _skill_meta_to_dict, @@ -1493,6 +1495,73 @@ def test_missing_meta(self): assert "name: bare-agent" in result +# --------------------------------------------------------------------------- +# parallel_search_sources — timeout/error handling +# --------------------------------------------------------------------------- + + +class TestParallelSearchSources: + def _make_source(self, source_id, results=None, *, delay=0.0, error=None): + class _Source: + def source_id(self): + return source_id + + def search(self, query, limit=10): + if delay: + time.sleep(delay) + if error: + raise error + return results or [] + + return _Source() + + def test_timeout_returns_partial_results_without_waiting_for_slow_source(self): + fast_result = SkillMeta( + name="fast", + description="d", + source="fast", + identifier="fast/skill", + trust_level="community", + ) + fast = self._make_source("fast", [fast_result]) + slow = self._make_source("slow", [], delay=0.8) + + start = time.monotonic() + results, counts, timed_out = parallel_search_sources( + [fast, slow], + query="skill", + overall_timeout=0.05, + ) + elapsed = time.monotonic() - start + + assert elapsed < 0.5 + assert results == [fast_result] + assert counts == {"fast": 1} + assert timed_out == ["slow"] + + def test_source_error_returns_empty_results_for_that_source(self): + ok_result = SkillMeta( + name="ok", + description="d", + source="ok", + identifier="ok/skill", + trust_level="community", + ) + failing = self._make_source("fail", error=RuntimeError("boom")) + ok = self._make_source("ok", [ok_result]) + + results, counts, timed_out = parallel_search_sources( + [failing, ok], + query="skill", + overall_timeout=1, + ) + + assert results == [ok_result] + assert counts["fail"] == 0 + assert counts["ok"] == 1 + assert timed_out == [] + + # --------------------------------------------------------------------------- # unified_search — dedup logic # --------------------------------------------------------------------------- diff --git a/tools/skills_hub.py b/tools/skills_hub.py index 31a88c973fb4..6df80176e345 100644 --- a/tools/skills_hub.py +++ b/tools/skills_hub.py @@ -3739,7 +3739,11 @@ def parallel_search_sources( *on_source_done* is an optional callback ``(source_id, count) -> None`` invoked as each source completes — useful for progress indicators. """ - from concurrent.futures import ThreadPoolExecutor, as_completed + from concurrent.futures import ( + ThreadPoolExecutor, + TimeoutError as FuturesTimeoutError, + as_completed, + ) per_source_limits = per_source_limits or {} @@ -3774,8 +3778,22 @@ def parallel_search_sources( if not active: return all_results, source_counts, timed_out_ids - with ThreadPoolExecutor(max_workers=min(len(active), 8)) as pool: - futures = {} + pool = ThreadPoolExecutor(max_workers=min(len(active), 8)) + futures = {} + processed = set() + + def _record_future(fut) -> None: + processed.add(fut) + try: + sid, results = fut.result(timeout=0) + source_counts[sid] = len(results) + all_results.extend(results) + if on_source_done: + on_source_done(sid, len(results)) + except Exception: + pass + + try: for src in active: lim = per_source_limits.get(src.source_id(), 50) fut = pool.submit(_search_one_source, src, query, lim) @@ -3783,23 +3801,24 @@ def parallel_search_sources( try: for fut in as_completed(futures, timeout=overall_timeout): - try: - sid, results = fut.result(timeout=0) - source_counts[sid] = len(results) - all_results.extend(results) - if on_source_done: - on_source_done(sid, len(results)) - except Exception: - pass - except TimeoutError: + _record_future(fut) + except FuturesTimeoutError: + for fut in futures: + if fut.done() and fut not in processed: + _record_future(fut) timed_out_ids = [ futures[f] for f in futures if not f.done() ] + for fut in futures: + if not fut.done(): + fut.cancel() if timed_out_ids: logger.debug( "Skills browse timed out waiting for: %s", ", ".join(timed_out_ids), ) + finally: + pool.shutdown(wait=False, cancel_futures=True) return all_results, source_counts, timed_out_ids diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index 980faf3d11f1..ea857f81f25f 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -963,9 +963,10 @@ export const api = { }), updateSkillsFromHub: () => fetchJSON("/api/skills/hub/update", { method: "POST" }), - searchSkillsHub: (q: string, source = "all", limit = 20) => + searchSkillsHub: (q: string, source = "all", limit = 20, init?: RequestInit) => fetchJSON( `/api/skills/hub/search?q=${encodeURIComponent(q)}&source=${encodeURIComponent(source)}&limit=${limit}`, + init, ), getSkillHubSources: () => fetchJSON("/api/skills/hub/sources"), @@ -1057,6 +1058,8 @@ export interface SkillHubSource { rate_limited?: boolean; /** hermes-index only: whether the centralized index loaded. */ available?: boolean; + /** Best-effort source status detail when probing this source failed. */ + error?: string; } export interface SkillHubSourcesResponse { diff --git a/web/src/pages/SkillsPage.tsx b/web/src/pages/SkillsPage.tsx index 4ece3105f760..1460b0af8b57 100644 --- a/web/src/pages/SkillsPage.tsx +++ b/web/src/pages/SkillsPage.tsx @@ -141,7 +141,7 @@ export default function SkillsPage() { }) .catch(() => showToast(t.common.loading, "error")) .finally(() => setLoading(false)); - }, []); + }, [showToast, t.common.loading]); /* ---- Toggle skill ---- */ const handleToggleSkill = async (skill: SkillInfo) => { @@ -666,6 +666,13 @@ const SEVERITY_TONE: Record>({}); const [timedOut, setTimedOut] = useState([]); const [searchMs, setSearchMs] = useState(null); + const [searchError, setSearchError] = useState(null); // Landing state: which hubs are wired up + featured skills. const [sources, setSources] = useState([]); const [featured, setFeatured] = useState([]); const [sourcesLoading, setSourcesLoading] = useState(true); + const [sourcesError, setSourcesError] = useState(null); // identifier -> installed entry (drives "Installed" badges). const [installed, setInstalled] = useState>({}); @@ -705,9 +714,10 @@ function HubBrowser({ setSources(r.sources); setFeatured(r.featured); setInstalled(r.installed); + setSourcesError(null); }) - .catch(() => { - /* leave landing minimal on failure */ + .catch((e) => { + if (!cancelled) setSourcesError(describeHubError(e)); }) .finally(() => { if (!cancelled) setSourcesLoading(false); @@ -723,19 +733,33 @@ function HubBrowser({ if (!q) return; setSearching(true); setSearched(true); + setSearchError(null); const t0 = performance.now(); + const controller = new AbortController(); + let clientTimedOut = false; + const timer = window.setTimeout(() => { + clientTimedOut = true; + controller.abort(); + }, SKILL_HUB_SEARCH_TIMEOUT_MS); try { - const r = await api.searchSkillsHub(q); + const r = await api.searchSkillsHub(q, "all", 20, { + signal: controller.signal, + }); setResults(r.results); setSourceCounts(r.source_counts || {}); setTimedOut(r.timed_out || []); setInstalled((prev) => ({ ...prev, ...(r.installed || {}) })); } catch (e) { - showToast(`Hub search failed: ${e}`, "error"); + const message = clientTimedOut + ? `Hub search timed out after ${SKILL_HUB_SEARCH_TIMEOUT_MS / 1000}s. Try again or use a more specific query.` + : `Hub search failed: ${describeHubError(e)}`; + setSearchError(message); + showToast(message, "error"); setResults([]); setSourceCounts({}); setTimedOut([]); } finally { + window.clearTimeout(timer); setSearchMs(Math.round(performance.now() - t0)); setSearching(false); } @@ -844,7 +868,11 @@ function HubBrowser({ {/* Connected hubs strip — proves the tab is wired up. */} - + @@ -934,7 +962,17 @@ function HubBrowser({ timedOut={timedOut} ms={searchMs} /> - {results.length === 0 ? ( + {searchError && ( + + +
+ + {searchError} +
+ + + )} + {results.length === 0 && !searchError ? ( No matching skills found in the hub. @@ -957,6 +995,7 @@ function HubBrowser({ {/* ── Detail dialog: preview + scan ── */} {detail && ( setDetail(null)} @@ -972,9 +1011,11 @@ function HubBrowser({ function ConnectedHubs({ sources, loading, + error, }: { sources: SkillHubSource[]; loading: boolean; + error: string | null; }) { if (loading) { return ( @@ -984,39 +1025,59 @@ function ConnectedHubs({ if (sources.length === 0) { return (

- Results come from the same sources as{" "} - hermes skills search. + {error ? ( + <> + Skill hub sources unavailable:{" "} + {error} + + ) : ( + <> + Results come from the same sources as{" "} + hermes skills search. + + )}

); } return ( -
- - - Connected hubs: - - {sources.map((s) => { - const down = - (s.id === "hermes-index" && s.available === false) || - (s.id === "github" && s.rate_limited === true); - return ( - - {s.label} - {s.id === "github" && s.rate_limited ? " (rate-limited)" : ""} - - ); - })} +
+ {error && ( + + + Source status refresh failed: {error} + + )} +
+ + + Connected hubs: + + {sources.map((s) => { + const down = + Boolean(s.error) || + (s.id === "hermes-index" && s.available === false) || + (s.id === "github" && s.rate_limited === true); + const title = s.error + ? s.error + : s.id === "github" && s.rate_limited + ? "GitHub API rate-limited - set GITHUB_TOKEN to raise the limit" + : s.id === "hermes-index" && s.available === false + ? "Centralized index unavailable - falling back to live sources" + : undefined; + return ( + + {s.label} + {s.error ? " (degraded)" : ""} + {s.id === "github" && s.rate_limited ? " (rate-limited)" : ""} + + ); + })} +
); } @@ -1165,7 +1226,6 @@ function SkillDetailDialog({ useEffect(() => { let cancelled = false; - setPreviewLoading(true); api .previewSkillFromHub(result.identifier) .then((p) => !cancelled && setPreview(p))