From d1eefe6accde90d15f7b70293b54100e7de3d99b Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 20 Aug 2026 02:04:32 -0700 Subject: [PATCH] =?UTF-8?q?feat:=20keyed=20web=20backends=20get=20a=20one-?= =?UTF-8?q?shot=20keyless=20rescue=20on=20failure=20=E2=80=94=20never=20st?= =?UTF-8?q?icky?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the chosen/keyed backend fails a web_search or web_extract call (bad key, upstream outage, 5xx, raised exception), that single call retries on the keyless free-tier ring instead of erroring. The next call attempts the chosen backend again — no sticky failover, no state. Resolves the keyed half of #78984/#32159 (keyless half landed in the ring PR). - tools/web_tools.py: _rescue_eligible (keyed ring vendors + non-ring backends eligible; keyless-mode calls excluded — they already walked the ring), _rescue_search/_rescue_extract (search annotates rescued_from + backend_error naming the original failure and the retry-next-call semantics; extract rescues only whole-batch failures, partial failures pass through untouched; rescue failure preserves the ORIGINAL backend error with the rescue note appended) - both dispatchers wrap the provider call: failure-results AND raised exceptions rescue; ineligible paths re-raise unchanged - web.keyless_rescue config key (default true; implicitly off when keyless_fallback is off); docs updated Live E2E: keyed Tavily with an invalid key 401'd and the call was served by the real ring with the rescue annotation; a second call re-attempted Tavily first (statelessness proven); whole-batch extract rescue returned real page content. 13 new tests; 67 green across the keyless suites. --- hermes_cli/config_defaults.py | 5 + tests/tools/test_web_keyless_rescue.py | 247 ++++++++++++++++++ tools/web_tools.py | 183 ++++++++++++- website/docs/user-guide/configuration.md | 9 +- .../docs/user-guide/features/web-search.md | 2 + 5 files changed, 436 insertions(+), 10 deletions(-) create mode 100644 tests/tools/test_web_keyless_rescue.py diff --git a/hermes_cli/config_defaults.py b/hermes_cli/config_defaults.py index 0a0d05ee9f..5a3ac68b4f 100644 --- a/hermes_cli/config_defaults.py +++ b/hermes_cli/config_defaults.py @@ -498,6 +498,11 @@ DEFAULT_CONFIG = { # failing over to the next ring vendor on rate limits. Never # pre-empts a configured or keyed backend. Set false to disable. "keyless_fallback": True, + # One-shot keyless rescue: when the chosen/keyed backend fails a + # web_search/web_extract call, THAT call retries once on the keyless + # free-tier ring — the next call attempts the chosen backend again + # (no sticky failover). Off when keyless_fallback is false. + "keyless_rescue": True, # Per-provider tier selection for ring vendors with both a keyless # free endpoint and a keyed paid path (exa, parallel, tavily, # firecrawl, keenable). Set by the `hermes tools` picker's diff --git a/tests/tools/test_web_keyless_rescue.py b/tests/tools/test_web_keyless_rescue.py new file mode 100644 index 0000000000..aac14fce8b --- /dev/null +++ b/tests/tools/test_web_keyless_rescue.py @@ -0,0 +1,247 @@ +"""One-shot keyless rescue: keyed/configured backend fails → THIS call rides +the keyless ring; the NEXT call attempts the chosen backend again. + +Covers: +- eligibility: keyed ring vendors and non-ring backends are eligible; + keyless-mode ring calls are not (they already walked the ring); config + gates (keyless_rescue / keyless_fallback) turn it off +- search dispatcher: failure-result and raised-exception paths both rescue, + result annotated with rescued_from + backend_error +- statelessness: the very next dispatch calls the chosen backend again +- extract dispatcher: whole-batch failure rescues; partial failure passes + through untouched +- rescue failure: original backend error survives, rescue note appended +""" + +import json +from unittest.mock import patch + +import pytest + +import tools.web_tools as web_tools +from plugins.web import keyless_mcp +from plugins.web.tavily.provider import TavilyWebSearchProvider + + +class _KeyedBoomProvider: + """Minimal keyed provider double that always fails.""" + + name = "tavily" + display_name = "Tavily" + + def supports_search(self): + return True + + def supports_extract(self): + return True + + def is_available(self): + return True + + def search(self, query, limit=5): + return {"success": False, "error": "HTTP 500 upstream exploded"} + + def extract(self, urls, **kwargs): + return [ + {"url": u, "title": "", "content": "", "error": "HTTP 500 upstream exploded"} + for u in urls + ] + + +class _RaisingProvider(_KeyedBoomProvider): + def search(self, query, limit=5): + raise RuntimeError("connection reset by peer") + + +@pytest.fixture(autouse=True) +def _keyed_tavily_env(monkeypatch): + """Simulate a keyed Tavily setup with rescue enabled.""" + monkeypatch.setattr( + "agent.web_search_provider.get_provider_env", + lambda name: "tvly-real" if name == "TAVILY_API_KEY" else "", + ) + monkeypatch.setattr(web_tools, "_load_web_config", lambda: {"backend": "tavily"}) + monkeypatch.setattr( + "agent.web_search_registry._keyless_tier_enabled", lambda: True + ) + yield + + +def _ring_ok(vendor="exa"): + return {"success": True, "data": {"web": [{"url": f"https://{vendor}.example"}]}} + + +class TestEligibility: + def test_keyed_ring_vendor_is_eligible(self): + assert web_tools._rescue_eligible(_KeyedBoomProvider()) is True + + def test_keyless_mode_ring_vendor_not_eligible(self, monkeypatch): + # No key: the tavily call already rode the ring; no double-walk. + monkeypatch.setattr( + "agent.web_search_provider.get_provider_env", lambda name: "" + ) + assert web_tools._rescue_eligible(TavilyWebSearchProvider()) is False + + def test_non_ring_backend_is_eligible(self): + class _SearxProvider(_KeyedBoomProvider): + name = "searxng" + + assert web_tools._rescue_eligible(_SearxProvider()) is True + + def test_config_gate_disables(self, monkeypatch): + monkeypatch.setattr( + web_tools, "_load_web_config", + lambda: {"backend": "tavily", "keyless_rescue": False}, + ) + assert web_tools._rescue_eligible(_KeyedBoomProvider()) is False + + def test_keyless_fallback_off_disables(self, monkeypatch): + monkeypatch.setattr( + "agent.web_search_registry._keyless_tier_enabled", lambda: False + ) + assert web_tools._rescue_eligible(_KeyedBoomProvider()) is False + + +class TestSearchRescue: + def _dispatch(self, monkeypatch, provider): + monkeypatch.setattr(web_tools, "_ensure_web_plugins_loaded", lambda: None) + monkeypatch.setattr( + "agent.web_search_registry.get_provider", lambda name: provider + ) + return json.loads(web_tools.web_search_tool("q", limit=2)) + + def test_failure_result_rescued_and_annotated(self, monkeypatch): + with patch.object( + keyless_mcp, "search_with_failover", return_value=_ring_ok() + ) as ring: + out = self._dispatch(monkeypatch, _KeyedBoomProvider()) + assert out["success"] is True + assert out["data"]["rescued_from"] == "tavily" + assert "HTTP 500" in out["data"]["backend_error"] + assert "next call" in out["data"]["backend_error"].lower() + ring.assert_called_once() + + def test_raised_exception_rescued(self, monkeypatch): + with patch.object( + keyless_mcp, "search_with_failover", return_value=_ring_ok() + ): + out = self._dispatch(monkeypatch, _RaisingProvider()) + assert out["success"] is True + assert "connection reset" in out["data"]["backend_error"] + + def test_stateless_next_call_uses_chosen_backend(self, monkeypatch): + calls = {"backend": 0} + + class _Counting(_KeyedBoomProvider): + def search(self, query, limit=5): + calls["backend"] += 1 + return {"success": False, "error": "HTTP 500 upstream exploded"} + + provider = _Counting() + with patch.object( + keyless_mcp, "search_with_failover", return_value=_ring_ok() + ) as ring: + self._dispatch(monkeypatch, provider) + self._dispatch(monkeypatch, provider) + # The chosen backend was attempted on BOTH calls (no sticky failover), + # and each failure triggered its own one-shot rescue. + assert calls["backend"] == 2 + assert ring.call_count == 2 + + def test_rescue_failure_keeps_original_error(self, monkeypatch): + with patch.object( + keyless_mcp, "search_with_failover", + return_value={"success": False, "error": "all throttled"}, + ): + out = self._dispatch(monkeypatch, _KeyedBoomProvider()) + assert out["success"] is False + assert "HTTP 500 upstream exploded" in out["error"] + assert "keyless rescue also failed" in out["error"] + + def test_no_rescue_when_disabled(self, monkeypatch): + monkeypatch.setattr( + web_tools, "_load_web_config", + lambda: {"backend": "tavily", "keyless_rescue": False}, + ) + with patch.object(keyless_mcp, "search_with_failover") as ring: + out = self._dispatch(monkeypatch, _KeyedBoomProvider()) + assert out["success"] is False + ring.assert_not_called() + + +class TestExtractRescue: + async def _dispatch(self, monkeypatch, provider, urls): + monkeypatch.setattr(web_tools, "_ensure_web_plugins_loaded", lambda: None) + monkeypatch.setattr( + "agent.web_search_registry.get_provider", lambda name: provider + ) + + async def _allow_all(url, **kwargs): + return True + + monkeypatch.setattr(web_tools, "async_is_safe_url", _allow_all) + raw = await web_tools.web_extract_tool(list(urls)) + data = json.loads(raw) + return data["results"] if isinstance(data, dict) and "results" in data else data + + @pytest.mark.asyncio + async def test_whole_batch_failure_rescued(self, monkeypatch): + good = [ + {"url": "https://a", "title": "A", "content": "x" * 50, + "raw_content": "x" * 50, "metadata": {"sourceURL": "https://a"}}, + {"url": "https://b", "title": "B", "content": "y" * 50, + "raw_content": "y" * 50, "metadata": {"sourceURL": "https://b"}}, + ] + with patch.object( + keyless_mcp, "extract_with_failover", return_value=good + ) as ring: + results = await self._dispatch( + monkeypatch, _KeyedBoomProvider(), ["https://a", "https://b"] + ) + assert all(not r.get("error") for r in results) + assert results[0]["content"].startswith("x") + ring.assert_called_once() + + def test_rescue_extract_annotates_results(self, monkeypatch): + good = [ + {"url": "https://a", "title": "A", "content": "x", + "raw_content": "x", "metadata": {"sourceURL": "https://a"}}, + ] + failed = [{"url": "https://a", "title": "", "content": "", "error": "HTTP 500"}] + with patch.object( + keyless_mcp, "extract_with_failover", return_value=good + ): + out = web_tools._rescue_extract("tavily", ["https://a"], failed) + assert out[0]["metadata"]["rescued_from"] == "tavily" + assert "HTTP 500" in out[0]["metadata"]["backend_error"] + + @pytest.mark.asyncio + async def test_partial_failure_not_rescued(self, monkeypatch): + class _Partial(_KeyedBoomProvider): + def extract(self, urls, **kwargs): + return [ + {"url": urls[0], "title": "A", "content": "fine", + "raw_content": "fine", "metadata": {}}, + {"url": urls[1], "title": "", "content": "", "error": "404"}, + ] + + with patch.object(keyless_mcp, "extract_with_failover") as ring: + results = await self._dispatch( + monkeypatch, _Partial(), ["https://a", "https://b"] + ) + assert results[1].get("error") + ring.assert_not_called() + + @pytest.mark.asyncio + async def test_rescue_failure_keeps_original_errors(self, monkeypatch): + still_bad = [ + {"url": "https://a", "title": "", "content": "", "error": "ring dead"}, + {"url": "https://b", "title": "", "content": "", "error": "ring dead"}, + ] + with patch.object( + keyless_mcp, "extract_with_failover", return_value=still_bad + ): + results = await self._dispatch( + monkeypatch, _KeyedBoomProvider(), ["https://a", "https://b"] + ) + assert all("HTTP 500" in r.get("error", "") for r in results) diff --git a/tools/web_tools.py b/tools/web_tools.py index 290fbbb73b..d6221b0c72 100644 --- a/tools/web_tools.py +++ b/tools/web_tools.py @@ -427,6 +427,128 @@ def _ddgs_package_importable() -> bool: except ImportError: return False + +# ─── One-shot keyless rescue (keyed/configured backend failed) ─────────────── + +def _keyless_rescue_enabled() -> bool: + """Read ``web.keyless_rescue`` from config (default: enabled). + + Also implicitly off whenever the keyless tier itself is disabled + (``web.keyless_fallback: false``). + """ + cfg = _load_web_config() + if not cfg.get("keyless_rescue", True): + return False + try: + from agent.web_search_registry import _keyless_tier_enabled + + return _keyless_tier_enabled() + except Exception as exc: # noqa: BLE001 — registry optional + logger.debug("keyless rescue tier check failed: %s", exc) + return False + + +def _rescue_eligible(provider) -> bool: + """True when a failed call on *provider* should get a one-shot rescue. + + Eligible: the call ran a keyed/configured path — either a non-ring + backend (searxng, brave-free, xai, custom plugins, managed gateway) or + a ring vendor operating in keyed mode. NOT eligible: the call already + went through the keyless ring (its failure means the ring was walked; + re-walking would just repeat it). + """ + if not _keyless_rescue_enabled(): + return False + if provider is None: + return False + try: + from plugins.web.keyless_mcp import _KEYLESS_RING, use_keyless + + name = getattr(provider, "name", "") + if name in _KEYLESS_RING: + key_var = { + "exa": "EXA_API_KEY", + "parallel": "PARALLEL_API_KEY", + "tavily": "TAVILY_API_KEY", + "firecrawl": "FIRECRAWL_API_KEY", + "keenable": "KEENABLE_API_KEY", + }.get(name, "") + from agent.web_search_provider import get_provider_env + + api_key = get_provider_env(key_var) if key_var else "" + # Keyless-mode ring vendors already walked the ring on failure. + return not use_keyless(name, api_key) + return True + except Exception as exc: # noqa: BLE001 — rescue is best-effort + logger.debug("rescue eligibility check failed: %s", exc) + return False + + +def _rescue_search(provider_name: str, original_error: str, query: str, limit: int) -> dict: + """One-shot keyless-ring rescue for a failed keyed/configured search. + + Stateless by design: this call alone routes to the free-tier ring; the + NEXT web_search call attempts the chosen backend again. The result is + annotated with the original backend failure so the model (and the + user) can see the configured backend needs attention. + """ + from plugins.web.keyless_mcp import search_with_failover + + logger.warning( + "web_search backend '%s' failed (%s); one-shot keyless rescue", + provider_name, (original_error or "")[:200], + ) + rescued = search_with_failover(provider_name, query, limit) + if rescued.get("success"): + data = rescued.setdefault("data", {}) + data["rescued_from"] = provider_name + data["backend_error"] = ( + f"Configured backend '{provider_name}' failed this call " + f"({(original_error or 'unknown error')[:300]}); result served " + "by the keyless free tier. The next call will use " + f"'{provider_name}' again." + ) + return rescued + # Ring also failed: surface the ORIGINAL backend error (it names the + # user's configured setup) with the rescue note appended. + return { + "success": False, + "error": ( + f"{original_error or 'search failed'} " + f"(keyless rescue also failed: {rescued.get('error', 'unknown')})" + ), + } + + +def _rescue_extract(provider_name: str, urls: list, results: list) -> list: + """One-shot keyless-ring rescue for a failed keyed/configured extract. + + Fires only when EVERY url failed (whole-backend failure); partial + results are page problems and pass through untouched. Stateless — + the next web_extract call attempts the chosen backend again. + """ + from plugins.web.keyless_mcp import extract_with_failover + + original_error = next( + (r.get("error") for r in results if r.get("error")), "extract failed" + ) + logger.warning( + "web_extract backend '%s' failed all %d URL(s) (%s); one-shot keyless rescue", + provider_name, len(urls), (original_error or "")[:200], + ) + rescued = extract_with_failover(provider_name, list(urls)) + rescued_errors = [r.get("error", "") for r in rescued] + if rescued and all(e for e in rescued_errors): + return results # rescue also failed everywhere: keep original errors + for r in rescued: + if not r.get("error"): + meta = r.setdefault("metadata", {}) + if isinstance(meta, dict): + meta["rescued_from"] = provider_name + meta["backend_error"] = (original_error or "")[:300] + return rescued + + # ─── Firecrawl Client ──────────────────────────────────────────────────────── # ─── Firecrawl Client ──────────────────────────────────────────────────────── @@ -814,7 +936,28 @@ def web_search_tool(query: str, limit: int = 5) -> str: "Web search via %s: '%s' (limit: %d)", provider.name, query, limit, ) - response_data = provider.search(query, limit) + try: + response_data = provider.search(query, limit) + except Exception as exc: # noqa: BLE001 — candidate for rescue + if _rescue_eligible(provider): + response_data = _rescue_search( + provider.name, str(exc), query, limit + ) + else: + raise + else: + if ( + not response_data.get("success") + and _rescue_eligible(provider) + ): + # One-shot keyless rescue: THIS call rides the free-tier + # ring; the next call attempts the chosen backend again. + response_data = _rescue_search( + provider.name, + str(response_data.get("error", "")), + query, + limit, + ) debug_call_data["results_count"] = len(response_data.get("data", {}).get("web", [])) result_json = json.dumps(response_data, indent=2, ensure_ascii=False) @@ -1057,14 +1200,38 @@ async def web_extract_tool( # Async-or-sync dispatch: parallel + firecrawl have async # extract(); exa + tavily are sync. import inspect - if inspect.iscoroutinefunction(provider.extract): - results = await provider.extract(safe_urls, format=format) + try: + if inspect.iscoroutinefunction(provider.extract): + results = await provider.extract(safe_urls, format=format) + else: + # Run sync extract() in a thread so we don't block the + # event loop on network I/O. + results = await asyncio.to_thread( + provider.extract, safe_urls, format=format + ) + except Exception as exc: # noqa: BLE001 — candidate for rescue + if _rescue_eligible(provider): + failed = [ + {"url": u, "title": "", "content": "", "error": str(exc)} + for u in safe_urls + ] + results = await asyncio.to_thread( + _rescue_extract, provider.name, safe_urls, failed + ) + else: + raise else: - # Run sync extract() in a thread so we don't block the - # event loop on network I/O. - results = await asyncio.to_thread( - provider.extract, safe_urls, format=format - ) + # One-shot keyless rescue when the WHOLE batch failed + # (backend-level outage, not per-page problems). Stateless: + # the next web_extract call uses the chosen backend again. + if ( + results + and all(r.get("error") for r in results) + and _rescue_eligible(provider) + ): + results = await asyncio.to_thread( + _rescue_extract, provider.name, safe_urls, results + ) # Reconstruct the original input order across invalid, blocked, and # provider-processed entries. Providers are expected to preserve the diff --git a/website/docs/user-guide/configuration.md b/website/docs/user-guide/configuration.md index d7e0d63601..05c70aafa6 100644 --- a/website/docs/user-guide/configuration.md +++ b/website/docs/user-guide/configuration.md @@ -2272,10 +2272,15 @@ web: extract_backend: "firecrawl" # Keyless free-tier fallback (default: true). With no backend configured - # and no API keys present, web tools fall back to Parallel's / Exa's - # public anonymous endpoints (rate-limited). Set false to disable. + # and no API keys present, web tools rotate across the Exa/Parallel/ + # Tavily/Firecrawl/Keenable free tiers. Set false to disable. keyless_fallback: true + # One-shot keyless rescue (default: true). When the chosen/keyed backend + # fails a call, that single call retries on the keyless ring; the next + # call attempts the chosen backend again (never sticky). + keyless_rescue: true + # Pin Exa/Parallel to a tier (set by the hermes tools Free/Paid rows). # free = always the anonymous endpoint; paid = always the keyed SDK path; # unset = auto (key present -> paid, otherwise free). diff --git a/website/docs/user-guide/features/web-search.md b/website/docs/user-guide/features/web-search.md index 7e0df647eb..32959102b7 100644 --- a/website/docs/user-guide/features/web-search.md +++ b/website/docs/user-guide/features/web-search.md @@ -374,6 +374,8 @@ If no backend has **ever** been selected (no `web.backend` / per-capability key **Keyless free-tier ring:** when *no* credential above is present, requests rotate across five vendors' public free tiers (Exa, Parallel, Tavily, Firecrawl, Keenable) so web tools work on a fresh install with zero setup — and a rate-limited request fails over to the next vendor in the ring automatically. Pin one vendor in `hermes tools` to stop the rotation (the ring is then only used as failover succession on throttles). All free tiers are vendor-rate-limited under burst load; sustained normal usage goes through fine. Set `web.keyless_fallback: false` to turn the tier off — with it off and no credentials, web tools are unavailable until a provider is configured. +**One-shot keyless rescue for keyed backends:** when your chosen/keyed backend fails a call (bad key, outage, upstream 5xx), that single call automatically retries on the keyless free-tier ring instead of erroring — the result notes which vendor served it and why (`rescued_from` / `backend_error`). The failover is never sticky: the very next `web_search`/`web_extract` call attempts your chosen backend again. Disable with `web.keyless_rescue: false` (also off whenever `keyless_fallback` is off). + xAI Web Search is **not** in the auto-detection chain — having `XAI_API_KEY` set (or being signed in via xAI Grok OAuth) does not automatically route web traffic through xAI, since those credentials are also used for inference / TTS / image gen and the user may want a different backend for web. Opt in explicitly with `web.backend: "xai"`. ---