From 590a19332e898fc9bda55a31999926572d8fbc26 Mon Sep 17 00:00:00 2001 From: Ben Date: Mon, 6 Jul 2026 14:32:08 +1000 Subject: [PATCH] fix(skills): don't request Brotli for the centralized skills index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Skills Hub 'Browse Hub' landing page and index-backed search render empty on fresh deployments (e.g. Fly.io VPS agents) with no stale cache. Root cause: the centralized index at /docs/api/skills-index.json is a large body (~34MB, tens of MB compressed) served with Content-Encoding: br. httpx's streaming Brotli decoder — backed by brotlicffi 1.2.0.1, which is pinned so aiohttp can decode Discord attachments — trips over its own output_buffer_limit on a payload this size and raises: DecodingError("brotli: decoder process called with data when 'can_accept_more_data()' is False") _load_hermes_index() catches that (DecodingError is an httpx.HTTPError subclass) and silently falls back to the on-disk cache. On a fresh box that cache never existed, so HermesIndexSource.is_available is False, the index contributes 0 skills, and the hub landing page — which is built solely from an empty-query index search — is blank. Existing installs only appear to work because they serve a (possibly weeks-)stale cached index instead. Fix: request 'gzip, deflate' on the index fetch so httpx never negotiates the broken Brotli path, and retry once with 'identity' if a DecodingError still occurs (defends against a proxy that ignores the header). Falls through to the stale cache only when both attempts fail. Verified on a live staging VPS agent: index_available flips False->True and the featured landing list repopulates from 0 to 12. Also un-freezes already-deployed images: skills added after an image was built (e.g. the 'unbroker' optional skill) become reachable again via the index, which is the whole point of the centralized catalog. --- tests/tools/test_skills_hub.py | 100 +++++++++++++++++++++++++++++++++ tools/skills_hub.py | 48 +++++++++++++--- 2 files changed, 140 insertions(+), 8 deletions(-) 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