From 5457c48df20156072b77f92dbe7272ca03d416d3 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Wed, 16 Sep 2026 12:42:23 -0700 Subject: [PATCH] fix: bound the index-miss fallback and skip it under provider filters and index timeouts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the fallback pass added in the previous commit found three ways it could burn the search budget without ever being able to return anything: - A provider filter (`--source nvidia`) selects like "all", so an index miss fanned out to skills.sh/ClawHub/LobeHub/well-known whose results carry no `extra.provider` and were then all cut by `_filter_results_by_provider` — four guaranteed-empty network calls and, with ClawHub slow, the whole 30 s budget. `_index_miss_fallback_sources` now takes the provider filter and returns [] when one is active. - The fallback ran until the shared `overall_timeout` deadline, so any unfiltered miss (a typo) stalled CLI/TUI/dashboard for the callers' full 30 s whenever ClawHub (documented here as taking minutes) was slow, where the index alone answered instantly. The pass now gets its own budget, `min(remaining, _INDEX_MISS_FALLBACK_BUDGET=8 s)`. - `if source_counts.get("hermes-index")` treated an index that TIMED OUT (key absent) as a miss, so the fallback fired with no budget left and `_fan_out` appended every fallback registry to `timed_out_ids` without calling it — the dashboard then blamed skills.sh/ClawHub/LobeHub falsely. Fallback now requires the index to have completed with zero results (`== 0`). One invariant test per major, each red on the previous commit (the budget test stalls 5 s there; the provider test makes registry calls there). Docs paragraph updated to state the 8 s cap and the provider-filter carve-out. --- tests/tools/test_skills_hub.py | 31 ++++++++++++++++++++++ tools/skills_hub_search.py | 26 +++++++++++++----- website/docs/user-guide/features/skills.md | 2 +- 3 files changed, 51 insertions(+), 8 deletions(-) diff --git a/tests/tools/test_skills_hub.py b/tests/tools/test_skills_hub.py index c2eb9866a4..a7d0e14b42 100644 --- a/tests/tools/test_skills_hub.py +++ b/tests/tools/test_skills_hub.py @@ -1874,6 +1874,37 @@ class TestIndexMissFallback: assert source_counts == {"hermes-index": 1} assert skills_sh.calls == 0 and github.calls == 0 + def test_provider_filter_miss_skips_registries_without_provider_data(self): + # `--source nvidia` selects like "all"; the fallback registries carry no + # extra.provider so re-asking them is guaranteed-empty and only burns budget. + index, skills_sh, github = self._sources([]) + clawhub = _FakeSource("clawhub", sleep=5) + + started = time.monotonic() + results, source_counts, timed_out = parallel_search_sources( + [index, skills_sh, clawhub, github], query="foo", source_filter="nvidia", overall_timeout=5.0) + + assert time.monotonic() - started < 1.0 + assert results == [] and timed_out == [] + assert source_counts == {"hermes-index": 0} + assert skills_sh.calls == 0 and clawhub.calls == 0 + + def test_fallback_pass_has_its_own_short_budget(self, monkeypatch): + # A slow registry (ClawHub takes minutes) must not stall a miss for the + # callers' full 30 s overall_timeout when the index answered instantly. + monkeypatch.setattr("tools.skills_hub_search._INDEX_MISS_FALLBACK_BUDGET", 0.3, raising=False) + index, skills_sh, github = self._sources([]) + clawhub = _FakeSource("clawhub", sleep=5) + + started = time.monotonic() + results, source_counts, timed_out = parallel_search_sources( + [index, skills_sh, clawhub, github], query="humanizar", overall_timeout=30.0) + + assert time.monotonic() - started < 2.0 + assert [r.identifier for r in results] == ["skills-sh/humanizar"] + assert source_counts == {"hermes-index": 0, "skills-sh": 1} + assert timed_out == ["clawhub"] + # --------------------------------------------------------------------------- # _load_hermes_index — centralized index fetch (Browse-hub landing / search) diff --git a/tools/skills_hub_search.py b/tools/skills_hub_search.py index 9f04ae4737..ce06ba96ae 100644 --- a/tools/skills_hub_search.py +++ b/tools/skills_hub_search.py @@ -88,6 +88,10 @@ _API_SOURCE_IDS = frozenset({"github", "skills-sh", "clawhub", "lobehub", "well- # live on skills.sh may not be in it yet. GitHub stays out — one miss (a typo) # would burn an unauthenticated user's whole hourly GitHub budget. _INDEX_MISS_FALLBACK_IDS = _API_SOURCE_IDS - {"github"} +# Cap on the fallback pass. ClawHub can take minutes; without its own budget +# every unfiltered miss (a typo) would stall CLI/TUI/dashboard for the callers' +# full 30 s ``overall_timeout`` where the index alone answered instantly. +_INDEX_MISS_FALLBACK_BUDGET = 8.0 def create_source_router(auth: Optional[GitHubAuth] = None) -> List[SkillSource]: @@ -149,15 +153,20 @@ def _select_active_sources(sources: List[SkillSource], source_filter: str) -> Li def _index_miss_fallback_sources( sources: List[SkillSource], active: List[SkillSource], query: str, source_counts: Dict[str, int], + provider_filter: str = "", ) -> List[SkillSource]: """Registries to consult after the index stood in for them and found nothing. Empty for a browse (no query), when the index was not consulted (no skip - happened), or when it returned matches. + happened), when it returned matches, or when it did not answer at all + (timed out: no budget is left and the registries would only be blamed as + late without being asked). Also empty under a provider filter + (``--source nvidia``): the fallback registries carry no ``extra.provider``, + so their results would all be cut and the calls would only burn budget. """ - if not query.strip() or not any(src.source_id() == "hermes-index" for src in active): + if not query.strip() or provider_filter or not any(src.source_id() == "hermes-index" for src in active): return [] - if source_counts.get("hermes-index"): + if source_counts.get("hermes-index") != 0: return [] return [src for src in sources if src.source_id() in _INDEX_MISS_FALLBACK_IDS and src not in active] @@ -222,8 +231,10 @@ def parallel_search_sources( When the centralized index stood in for the external registries and found nothing for a non-empty query, those registries are queried within the - same ``overall_timeout`` so every caller (CLI, TUI gateway, dashboard) - still finds skills the index has not picked up yet. + same ``overall_timeout`` — capped at ``_INDEX_MISS_FALLBACK_BUDGET`` so a + slow registry cannot turn an instant index miss into a 30 s stall — so + every caller (CLI, TUI gateway, dashboard) still finds skills the index + has not picked up yet. """ per_source_limits = per_source_limits or {} active = _select_active_sources(sources, source_filter) @@ -237,9 +248,10 @@ def parallel_search_sources( deadline = time.monotonic() + overall_timeout _fan_out(active, query, per_source_limits, provider_filter, deadline, on_source_done, all_results, source_counts, timed_out_ids) - fallback = _index_miss_fallback_sources(sources, active, query, source_counts) + fallback = _index_miss_fallback_sources(sources, active, query, source_counts, provider_filter) if fallback: - _fan_out(fallback, query, per_source_limits, provider_filter, deadline, on_source_done, + fallback_deadline = min(deadline, time.monotonic() + _INDEX_MISS_FALLBACK_BUDGET) + _fan_out(fallback, query, per_source_limits, provider_filter, fallback_deadline, on_source_done, all_results, source_counts, timed_out_ids) return all_results, source_counts, timed_out_ids diff --git a/website/docs/user-guide/features/skills.md b/website/docs/user-guide/features/skills.md index e084880892..fd4fc32b56 100644 --- a/website/docs/user-guide/features/skills.md +++ b/website/docs/user-guide/features/skills.md @@ -702,7 +702,7 @@ in the pending JSON file). Memory writes have the same gate under Browse, search, install, and manage skills from online registries, `skills.sh`, direct well-known skill endpoints, and official optional skills. -Unfiltered searches (CLI, TUI, and the dashboard) are answered from a cached centralized index that covers the external registries. That index is rebuilt periodically, so when it has no match for your query Hermes also asks `skills.sh`, ClawHub, LobeHub and well-known endpoints directly within the same search budget — a skill published minutes ago still shows up. Custom GitHub taps are not part of that fallback (search them with `--source github`, or via the index once it catches up). +Unfiltered searches (CLI, TUI, and the dashboard) are answered from a cached centralized index that covers the external registries. That index is rebuilt periodically, so when it has no match for your query Hermes also asks `skills.sh`, ClawHub, LobeHub and well-known endpoints directly — a skill published minutes ago still shows up. That extra pass gets at most 8 seconds of the search budget, so a slow registry cannot turn a miss into a long wait. Custom GitHub taps are not part of that fallback (search them with `--source github`, or via the index once it catches up), and provider filters such as `--source nvidia` do not trigger it (those registries carry no provider data). ### Common commands