fix(skills): don't request Brotli for the centralized skills index
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.
This commit is contained in:
parent
d3602e6308
commit
590a19332e
2 changed files with 140 additions and 8 deletions
|
|
@ -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"}]}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue