diff --git a/tests/tools/test_skills_hub.py b/tests/tools/test_skills_hub.py index 987995066..b1310e853 100644 --- a/tests/tools/test_skills_hub.py +++ b/tests/tools/test_skills_hub.py @@ -2471,3 +2471,103 @@ class TestParallelSearchSourcesTimeout: assert source_counts.get("a") == 1 assert source_counts.get("b") == 1 assert len(all_results) == 2 + + +# --------------------------------------------------------------------------- +# _load_hermes_index — centralized index fetch (Browse-hub landing / search) +# --------------------------------------------------------------------------- + + +class TestLoadHermesIndex: + """Regression coverage for the Skills-Hub index fetch. + + The centralized index is a large body served with Content-Encoding: br. + httpx's streaming Brotli decoder (brotlicffi 1.2.0.1, pinned for Discord + attachment decoding) raises DecodingError on payloads this size, which + used to cascade into a silently-empty Skills Hub. The fetch must therefore + (a) not ask for Brotli, and (b) survive a DecodingError by retrying + uncompressed instead of blanking the hub. + """ + + @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) + 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 + + self._isolate_cache(monkeypatch, tmp_path) + + captured = {} + + def fake_get(url, *args, **kwargs): + captured["headers"] = kwargs.get("headers", {}) + resp = MagicMock() + resp.status_code = 200 + resp.json.return_value = {"skills": [{"name": "x"}]} + return resp + + monkeypatch.setattr(hub.httpx, "get", fake_get) + + data = hub._load_hermes_index() + assert data == {"skills": [{"name": "x"}]} + + accept = captured["headers"].get("Accept-Encoding", "") + assert "br" not in [tok.strip() for tok in accept.split(",")], ( + f"index fetch must not request Brotli, got Accept-Encoding={accept!r}" + ) + + def test_decoding_error_retries_uncompressed(self, monkeypatch, tmp_path): + """A DecodingError on the first attempt retries with identity, not a blank hub.""" + import tools.skills_hub as hub + + self._isolate_cache(monkeypatch, tmp_path) + + attempts = [] + + def fake_get(url, *args, **kwargs): + enc = kwargs.get("headers", {}).get("Accept-Encoding", "") + attempts.append(enc) + if len(attempts) == 1: + raise httpx.DecodingError("brotli: decoder process called with data") + resp = MagicMock() + resp.status_code = 200 + resp.json.return_value = {"skills": [{"name": "recovered"}]} + return resp + + monkeypatch.setattr(hub.httpx, "get", fake_get) + + data = hub._load_hermes_index() + assert data == {"skills": [{"name": "recovered"}]} + assert len(attempts) == 2, "should retry once after a DecodingError" + # The retry must be uncompressed (identity) so a Brotli-ignoring proxy + # can't fail the same way twice. + assert attempts[1].strip() == "identity" + + def test_persistent_decoding_error_falls_back_to_stale_cache( + self, monkeypatch, tmp_path + ): + """If every attempt fails to decode, serve the stale cache rather than None.""" + import tools.skills_hub as hub + + 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) + import os + + os.utime(cache_file, (old, old)) + + def fake_get(url, *args, **kwargs): + raise httpx.DecodingError("brotli boom") + + monkeypatch.setattr(hub.httpx, "get", fake_get) + + data = hub._load_hermes_index() + assert data == {"skills": [{"name": "stale"}]} diff --git a/tools/skills_hub.py b/tools/skills_hub.py index db169b37d..9883b720e 100644 --- a/tools/skills_hub.py +++ b/tools/skills_hub.py @@ -3679,15 +3679,47 @@ def _load_hermes_index() -> Optional[dict]: except (OSError, json.JSONDecodeError): pass - # Fetch from docs site - try: - resp = httpx.get(HERMES_INDEX_URL, timeout=15, follow_redirects=True) - if resp.status_code != 200: - logger.debug("Hermes index fetch returned %d", resp.status_code) + # Fetch from docs site. + # + # We deliberately DON'T let httpx negotiate Brotli here. The index is a + # large body (tens of MB); httpx's streaming Brotli decoder, backed by + # brotlicffi 1.2.0.1 (pinned for Discord attachment decoding), trips over + # its own output_buffer_limit on payloads this size and raises + # DecodingError("brotli: decoder process called with data when + # 'can_accept_more_data()' is False"). That surfaces as an empty Skills + # Hub (blank Browse-hub landing, index contributes 0 search hits) because + # the error is caught below and we silently fall back to a (often absent) + # stale cache. Requesting gzip/deflate sidesteps the broken decoder while + # still compressing the transfer. The identity retry is belt-and-braces + # for any future proxy that ignores the header and returns Brotli anyway. + data = None + for accept_encoding in ("gzip, deflate", "identity"): + try: + resp = httpx.get( + HERMES_INDEX_URL, + timeout=15, + follow_redirects=True, + headers={"Accept-Encoding": accept_encoding}, + ) + if resp.status_code != 200: + logger.debug("Hermes index fetch returned %d", resp.status_code) + return _load_stale_index_cache() + data = resp.json() + break + except httpx.DecodingError as e: + # Content-Encoding decode failed — retry once uncompressed before + # giving up on the network path entirely. + logger.debug( + "Hermes index decode failed (Accept-Encoding=%s): %s", + accept_encoding, + e, + ) + continue + except (httpx.HTTPError, json.JSONDecodeError) as e: + logger.debug("Hermes index fetch failed: %s", e) return _load_stale_index_cache() - data = resp.json() - except (httpx.HTTPError, json.JSONDecodeError) as e: - logger.debug("Hermes index fetch failed: %s", e) + + if data is None: return _load_stale_index_cache() # Validate structure