revert(web): remove keyless Parallel search fallback (#46350)
Remove the free Parallel Search MCP path and restore the keyed Parallel backend behavior from before it was introduced. Also drops the keyless fallback registration/display labeling tests and returns the Parallel SDK pin to the prior version.
This commit is contained in:
Teknium
@@ -1,100 +0,0 @@
|
||||
"""Regression: the keyless Parallel web default must survive a failed sweep.
|
||||
|
||||
``web_search`` / ``web_extract`` are documented to work out of the box with
|
||||
zero setup via the bundled keyless Parallel free-MCP backend. That guarantee
|
||||
only holds if the bundled ``plugins/web/*`` providers are registered in
|
||||
``agent.web_search_registry``. The dispatch triggers the general plugin sweep
|
||||
(:func:`hermes_cli.plugins._ensure_plugins_discovered`) to do that — but the
|
||||
sweep can finish without registering them (its exception swallowed as a
|
||||
warning, a packaged layout where it ran before the bundled tree was
|
||||
importable, or a stale empty-discovery cache). When that happened, *both*
|
||||
tools dead-ended on "No web {search,extract} provider configured" even though
|
||||
no setup should be needed.
|
||||
|
||||
These tests pin the invariant that :func:`tools.web_tools._ensure_web_plugins_loaded`
|
||||
guarantees the keyless default is registered regardless of the sweep's outcome,
|
||||
and that the direct-registration fallback honors an explicit ``plugins.disabled``
|
||||
entry. Real imports from the bundled plugin modules — no provider mocking.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
import agent.web_search_registry as reg
|
||||
import hermes_cli.plugins as plugins
|
||||
from tools import web_tools
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _clean_registry():
|
||||
reg._reset_for_tests()
|
||||
yield
|
||||
reg._reset_for_tests()
|
||||
|
||||
|
||||
def _boom(*_a, **_k):
|
||||
raise RuntimeError("discovery boom")
|
||||
|
||||
|
||||
def test_keyless_default_registered_when_discovery_raises(monkeypatch):
|
||||
"""A swallowed discovery failure must not strand the keyless default."""
|
||||
monkeypatch.setattr(plugins, "_ensure_plugins_discovered", _boom)
|
||||
assert reg.get_provider("parallel") is None
|
||||
|
||||
web_tools._ensure_web_plugins_loaded()
|
||||
|
||||
parallel = reg.get_provider("parallel")
|
||||
assert parallel is not None, "keyless Parallel default not restored"
|
||||
# It is the universal keyless default precisely because it does both.
|
||||
assert parallel.supports_search()
|
||||
assert parallel.supports_extract()
|
||||
|
||||
|
||||
def test_fallback_registers_full_bundled_set(monkeypatch):
|
||||
"""The fix covers the whole bundled provider class, not just parallel."""
|
||||
monkeypatch.setattr(plugins, "_ensure_plugins_discovered", _boom)
|
||||
|
||||
web_tools._ensure_web_plugins_loaded()
|
||||
|
||||
names = {p.name for p in reg.list_providers()}
|
||||
# Every bundled backend a user might have configured should be reachable
|
||||
# again, so an explicit ``web.extract_backend: firecrawl`` etc. resolves.
|
||||
for expected in ("parallel", "firecrawl", "tavily", "exa"):
|
||||
assert expected in names, f"{expected} missing after fallback"
|
||||
|
||||
|
||||
def test_fallback_honors_explicit_disable(monkeypatch):
|
||||
"""A backend the user turned off via plugins.disabled stays off."""
|
||||
monkeypatch.setattr(plugins, "_get_disabled_plugins", lambda: {"web-parallel"})
|
||||
|
||||
web_tools._register_bundled_web_providers_directly()
|
||||
|
||||
names = {p.name for p in reg.list_providers()}
|
||||
assert "parallel" not in names, "explicit disable was ignored"
|
||||
# Other bundled backends are unaffected by the parallel disable.
|
||||
assert "tavily" in names
|
||||
|
||||
|
||||
def test_fallback_is_noop_when_discovery_already_registered(monkeypatch):
|
||||
"""Healthy path: don't pay for the direct sweep when parallel is present."""
|
||||
# Pretend the general sweep already registered the keyless default.
|
||||
import importlib
|
||||
|
||||
class _Ctx:
|
||||
def register_web_search_provider(self, provider):
|
||||
reg.register_provider(provider)
|
||||
|
||||
importlib.import_module("plugins.web.parallel").register(_Ctx())
|
||||
monkeypatch.setattr(plugins, "_ensure_plugins_discovered", lambda *a, **k: None)
|
||||
|
||||
calls = {"n": 0}
|
||||
real = web_tools._register_bundled_web_providers_directly
|
||||
|
||||
def _spy():
|
||||
calls["n"] += 1
|
||||
real()
|
||||
|
||||
monkeypatch.setattr(web_tools, "_register_bundled_web_providers_directly", _spy)
|
||||
web_tools._ensure_web_plugins_loaded()
|
||||
|
||||
assert calls["n"] == 0, "direct-registration ran on the healthy path"
|
||||
@@ -167,21 +167,6 @@ class TestPerCapabilityBackendSelection:
|
||||
monkeypatch.setenv("TAVILY_API_KEY", "test-key")
|
||||
assert web_tools._get_search_backend() == "tavily"
|
||||
|
||||
def test_explicit_extract_backend_honored_when_unavailable(self, monkeypatch):
|
||||
"""An explicit per-capability backend is honored even with no creds, so
|
||||
its setup error surfaces instead of silently rerouting to the keyless
|
||||
Parallel default (which would send user URLs to a different provider)."""
|
||||
from tools import web_tools
|
||||
|
||||
monkeypatch.setattr(web_tools, "_load_web_config", lambda: {
|
||||
"extract_backend": "firecrawl",
|
||||
})
|
||||
for key in ("FIRECRAWL_API_KEY", "FIRECRAWL_API_URL", "FIRECRAWL_GATEWAY_URL"):
|
||||
monkeypatch.delenv(key, raising=False)
|
||||
monkeypatch.setattr(web_tools, "_is_tool_gateway_ready", lambda: False, raising=False)
|
||||
# Resolves to firecrawl (not parallel) despite firecrawl being unavailable.
|
||||
assert web_tools._get_extract_backend() == "firecrawl"
|
||||
|
||||
def test_falls_back_to_generic_backend_when_extract_backend_empty(self, monkeypatch):
|
||||
from tools import web_tools
|
||||
|
||||
@@ -192,7 +177,7 @@ class TestPerCapabilityBackendSelection:
|
||||
monkeypatch.setenv("PARALLEL_API_KEY", "test-key")
|
||||
assert web_tools._get_extract_backend() == "parallel"
|
||||
|
||||
def test_explicit_search_backend_honored_when_unavailable(self, monkeypatch):
|
||||
def test_search_backend_ignored_when_not_available(self, monkeypatch):
|
||||
from tools import web_tools
|
||||
|
||||
monkeypatch.setattr(web_tools, "_load_web_config", lambda: {
|
||||
@@ -201,10 +186,8 @@ class TestPerCapabilityBackendSelection:
|
||||
})
|
||||
monkeypatch.delenv("EXA_API_KEY", raising=False)
|
||||
monkeypatch.setenv("FIRECRAWL_API_KEY", "fc-key")
|
||||
# The explicit per-capability choice (exa) is honored even though it's
|
||||
# unavailable, so its setup error surfaces — we don't silently reroute
|
||||
# to the shared backend (or the keyless Parallel default).
|
||||
assert web_tools._get_search_backend() == "exa"
|
||||
# Should fall back to firecrawl since exa isn't configured
|
||||
assert web_tools._get_search_backend() == "firecrawl"
|
||||
|
||||
def test_fully_backward_compatible_with_web_backend_only(self, monkeypatch):
|
||||
from tools import web_tools
|
||||
@@ -308,55 +291,26 @@ class TestUnconfiguredErrorEnvelopeParity:
|
||||
):
|
||||
monkeypatch.delenv(k, raising=False)
|
||||
|
||||
def test_extract_empty_urls_does_not_raise(self, monkeypatch):
|
||||
"""Regression: empty (or fully SSRF-blocked) URL sets skip the dispatch
|
||||
branch; the free-Parallel flag must still be initialized so the tool
|
||||
returns an error envelope instead of UnboundLocalError."""
|
||||
import asyncio
|
||||
from tools import web_tools
|
||||
self._clear_web_creds(monkeypatch)
|
||||
monkeypatch.setattr(web_tools, "_load_web_config", lambda: {})
|
||||
out = asyncio.run(web_tools.web_extract_tool([], "markdown"))
|
||||
# The key assertion is that it returns a normal error envelope (a
|
||||
# string) rather than raising UnboundLocalError.
|
||||
assert isinstance(out, str)
|
||||
result = json.loads(out)
|
||||
assert "error" in result
|
||||
|
||||
def test_unconfigured_search_falls_back_to_free_parallel(self, monkeypatch):
|
||||
"""``web_search_tool`` with no creds routes to Parallel's free Search
|
||||
MCP rather than erroring. The MCP transport is mocked so the test
|
||||
stays offline; we assert dispatch landed on parallel and returned the
|
||||
standard search envelope.
|
||||
def test_unconfigured_search_emits_top_level_error(self, monkeypatch):
|
||||
"""``web_search_tool`` with no creds returns ``{"error": "Error searching web: ..."}``
|
||||
— matching main's ``tool_error()`` envelope, not a per-result shape.
|
||||
"""
|
||||
from tools import web_tools
|
||||
import plugins.web.parallel.provider as parallel_provider
|
||||
|
||||
self._clear_web_creds(monkeypatch)
|
||||
# Reset firecrawl client cache so the unconfigured state is re-evaluated
|
||||
monkeypatch.setattr(web_tools, "_firecrawl_client", None, raising=False)
|
||||
monkeypatch.setattr(web_tools, "_firecrawl_client_config", None, raising=False)
|
||||
monkeypatch.setattr(web_tools, "_ddgs_package_importable", lambda: False)
|
||||
monkeypatch.setattr(web_tools, "_load_web_config", lambda: {})
|
||||
|
||||
captured = {}
|
||||
|
||||
def _fake_mcp(query, limit, api_key):
|
||||
captured["query"] = query
|
||||
captured["api_key"] = api_key
|
||||
return {
|
||||
"success": True,
|
||||
"data": {"web": [
|
||||
{"url": "https://example.com", "title": "Example",
|
||||
"description": "hit", "position": 1},
|
||||
]},
|
||||
}
|
||||
|
||||
monkeypatch.setattr(parallel_provider, "_mcp_web_search", _fake_mcp)
|
||||
|
||||
result = json.loads(web_tools.web_search_tool("hello world", limit=3))
|
||||
assert result.get("success") is True, f"expected success, got {result}"
|
||||
assert result["data"]["web"][0]["url"] == "https://example.com"
|
||||
# Keyless path: dispatched to parallel with no Bearer token.
|
||||
assert captured == {"query": "hello world", "api_key": None}
|
||||
assert "error" in result, f"expected top-level 'error' key, got {result}"
|
||||
# ``Error searching web:`` prefix comes from web_tools' top-level except handler
|
||||
assert "Error searching web:" in result["error"]
|
||||
assert "FIRECRAWL_API_KEY" in result["error"]
|
||||
# No per-result burying
|
||||
assert "results" not in result
|
||||
|
||||
|
||||
class TestDispatchersTriggerPluginDiscovery:
|
||||
|
||||
@@ -190,11 +190,7 @@ class TestDDGSBackendWiring:
|
||||
monkeypatch.setattr(web_tools, "_ddgs_package_importable", lambda: True)
|
||||
assert web_tools._get_backend() == "exa"
|
||||
|
||||
def test_auto_detect_prefers_keyless_parallel_over_ddgs(self, monkeypatch):
|
||||
# With no credentials, keyless Parallel is the auto-detect default even
|
||||
# when the ddgs package is installed — ddgs is search-only (can't
|
||||
# extract), so Parallel is preferred so both search and extract work.
|
||||
# ddgs remains reachable via an explicit web.backend=ddgs.
|
||||
def test_auto_detect_picks_ddgs_as_last_resort(self, monkeypatch):
|
||||
from tools import web_tools
|
||||
monkeypatch.setattr(web_tools, "_load_web_config", lambda: {})
|
||||
for key in ("FIRECRAWL_API_KEY", "FIRECRAWL_API_URL", "PARALLEL_API_KEY",
|
||||
@@ -202,7 +198,7 @@ class TestDDGSBackendWiring:
|
||||
monkeypatch.delenv(key, raising=False)
|
||||
monkeypatch.setattr(web_tools, "_is_tool_gateway_ready", lambda: False)
|
||||
monkeypatch.setattr(web_tools, "_ddgs_package_importable", lambda: True)
|
||||
assert web_tools._get_backend() == "parallel"
|
||||
assert web_tools._get_backend() == "ddgs"
|
||||
|
||||
def test_check_web_api_key_true_when_ddgs_configured(self, monkeypatch):
|
||||
from tools import web_tools
|
||||
|
||||
@@ -313,9 +313,7 @@ class TestCheckWebApiKey:
|
||||
)
|
||||
assert web_tools.check_web_api_key() is True
|
||||
|
||||
def test_no_credentials_usable_via_free_parallel(self, monkeypatch):
|
||||
"""No credentials → check_web_api_key True: the keyless Parallel free MCP
|
||||
services calls, so web is usable out of the box."""
|
||||
def test_no_credentials_fails(self, monkeypatch):
|
||||
from tools import web_tools
|
||||
monkeypatch.setattr(web_tools, "_load_web_config", lambda: {})
|
||||
monkeypatch.delenv("FIRECRAWL_API_KEY", raising=False)
|
||||
@@ -327,7 +325,7 @@ class TestCheckWebApiKey:
|
||||
monkeypatch.setattr(web_tools, "_is_tool_gateway_ready", lambda: False)
|
||||
monkeypatch.setattr(web_tools, "check_firecrawl_api_key", lambda: False)
|
||||
monkeypatch.setattr(web_tools, "_ddgs_package_importable", lambda: False)
|
||||
assert web_tools.check_web_api_key() is True
|
||||
assert web_tools.check_web_api_key() is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -384,14 +384,12 @@ class TestBackendSelection:
|
||||
patch.dict(os.environ, {"FIRECRAWL_API_KEY": "fc-test"}):
|
||||
assert _get_backend() == "firecrawl"
|
||||
|
||||
def test_fallback_no_keys_defaults_to_parallel(self):
|
||||
"""No credentials, no config → 'parallel' (free Search MCP works
|
||||
keyless). Selection is purely credential-based."""
|
||||
def test_fallback_no_keys_defaults_to_firecrawl(self):
|
||||
"""No keys, no config → 'firecrawl' (will fail at client init)."""
|
||||
from tools.web_tools import _get_backend
|
||||
with patch("tools.web_tools._load_web_config", return_value={}), \
|
||||
patch("tools.web_tools._is_tool_gateway_ready", return_value=False), \
|
||||
patch("tools.web_tools._ddgs_package_importable", return_value=False):
|
||||
assert _get_backend() == "parallel"
|
||||
assert _get_backend() == "firecrawl"
|
||||
|
||||
def test_invalid_config_falls_through_to_fallback(self):
|
||||
"""web.backend=invalid → ignored, uses key-based fallback."""
|
||||
@@ -626,73 +624,9 @@ class TestCheckWebApiKey:
|
||||
from tools.web_tools import check_web_api_key
|
||||
assert check_web_api_key() is True
|
||||
|
||||
def test_no_keys_usable_via_free_parallel(self):
|
||||
"""No credentials → check_web_api_key True: selection resolves to the
|
||||
keyless Parallel free MCP, which genuinely services calls (web works out
|
||||
of the box). check_web_api_key is a usability probe, not a key check."""
|
||||
def test_no_keys_returns_false(self):
|
||||
from tools.web_tools import check_web_api_key
|
||||
with patch("tools.web_tools._load_web_config", return_value={}), \
|
||||
patch("tools.web_tools._is_tool_gateway_ready", return_value=False), \
|
||||
patch("tools.web_tools._ddgs_package_importable", return_value=False), \
|
||||
patch.dict(os.environ, {}, clear=False):
|
||||
for k in ("PARALLEL_API_KEY", "FIRECRAWL_API_KEY", "FIRECRAWL_API_URL",
|
||||
"TAVILY_API_KEY", "EXA_API_KEY", "SEARXNG_URL", "BRAVE_SEARCH_API_KEY"):
|
||||
os.environ.pop(k, None)
|
||||
assert check_web_api_key() is True
|
||||
|
||||
def test_typo_extract_backend_not_masked_by_parallel(self):
|
||||
"""A typo'd per-capability backend is honored (so dispatch errors)
|
||||
rather than silently falling through to keyless Parallel."""
|
||||
from tools.web_tools import _get_extract_backend, check_web_api_key
|
||||
with patch("tools.web_tools._load_web_config",
|
||||
return_value={"extract_backend": "parrallel"}):
|
||||
assert _get_extract_backend() == "parrallel" # not "parallel"
|
||||
assert check_web_api_key() is False # unknown → unusable
|
||||
|
||||
def test_keyless_parallel_unusable_when_provider_disabled(self):
|
||||
"""If the bundled web-parallel provider is disabled/unregistered, the
|
||||
keyless free-MCP path must NOT report web as usable — otherwise setup is
|
||||
skipped but web tools fail at runtime with no provider."""
|
||||
from tools.web_tools import check_web_api_key
|
||||
with patch("tools.web_tools._load_web_config", return_value={}), \
|
||||
patch("tools.web_tools._parallel_provider_registered", return_value=False), \
|
||||
patch("tools.web_tools._is_tool_gateway_ready", return_value=False), \
|
||||
patch("tools.web_tools.check_firecrawl_api_key", return_value=False), \
|
||||
patch("tools.web_tools._ddgs_package_importable", return_value=False), \
|
||||
patch.dict(os.environ, {}, clear=False):
|
||||
for var in (
|
||||
"PARALLEL_API_KEY", "FIRECRAWL_API_KEY", "FIRECRAWL_API_URL",
|
||||
"TAVILY_API_KEY", "EXA_API_KEY", "BRAVE_SEARCH_API_KEY", "SEARXNG_URL",
|
||||
):
|
||||
os.environ.pop(var, None)
|
||||
assert check_web_api_key() is False
|
||||
|
||||
def test_extract_autodetect_skips_search_only_for_keyless_parallel(self):
|
||||
"""A search-only env credential (SEARXNG_URL) must not shadow the keyless
|
||||
Parallel free-MCP extract fallback: extract auto-detect skips search-only
|
||||
backends, so _get_extract_backend resolves to parallel (which can fetch),
|
||||
while search auto-detect still prefers the configured searxng."""
|
||||
from tools.web_tools import _get_extract_backend, _get_search_backend
|
||||
with patch("tools.web_tools._load_web_config", return_value={}), \
|
||||
patch.dict(os.environ, {}, clear=False):
|
||||
for var in (
|
||||
"PARALLEL_API_KEY", "FIRECRAWL_API_KEY", "FIRECRAWL_API_URL",
|
||||
"TAVILY_API_KEY", "EXA_API_KEY", "BRAVE_SEARCH_API_KEY",
|
||||
):
|
||||
os.environ.pop(var, None)
|
||||
os.environ["SEARXNG_URL"] = "http://localhost:8080"
|
||||
with patch("tools.web_tools._is_tool_gateway_ready", return_value=False):
|
||||
assert _get_search_backend() == "searxng"
|
||||
assert _get_extract_backend() == "parallel"
|
||||
|
||||
def test_configured_but_unavailable_backend_reports_unusable(self):
|
||||
"""An explicitly configured backend with no creds (exa, no key) →
|
||||
check_web_api_key False so diagnostics flag the misconfiguration —
|
||||
even though the tools stay registered."""
|
||||
from tools.web_tools import check_web_api_key
|
||||
with patch("tools.web_tools._load_web_config", return_value={"backend": "exa"}), \
|
||||
patch.dict(os.environ, {}, clear=False):
|
||||
os.environ.pop("EXA_API_KEY", None)
|
||||
with patch("tools.web_tools._ddgs_package_importable", return_value=False):
|
||||
assert check_web_api_key() is False
|
||||
|
||||
def test_both_keys_returns_true(self):
|
||||
@@ -756,18 +690,12 @@ class TestCheckWebApiKey:
|
||||
|
||||
assert refresh_calls == []
|
||||
|
||||
def test_web_tools_registered_even_when_configured_backend_unavailable(self):
|
||||
# Registration is unconditional (web_tools_registered) so an explicitly
|
||||
# configured but unavailable backend (exa without EXA_API_KEY) keeps the
|
||||
# tools registered to surface exa's setup error at call time — while the
|
||||
# readiness probe (check_web_api_key) honestly reports not-configured.
|
||||
from tools.web_tools import web_tools_registered, check_web_api_key
|
||||
assert web_tools_registered() is True
|
||||
with patch("tools.web_tools._load_web_config", return_value={"backend": "exa"}), \
|
||||
patch.dict(os.environ, {}, clear=False):
|
||||
os.environ.pop("EXA_API_KEY", None)
|
||||
assert web_tools_registered() is True
|
||||
assert check_web_api_key() is False
|
||||
def test_configured_backend_must_match_available_provider(self):
|
||||
with patch("tools.web_tools._load_web_config", return_value={"backend": "parallel"}):
|
||||
with patch("tools.web_tools._read_nous_access_token", return_value="nous-token"):
|
||||
with patch.dict(os.environ, {"FIRECRAWL_GATEWAY_URL": "http://127.0.0.1:3002"}, clear=False):
|
||||
from tools.web_tools import check_web_api_key
|
||||
assert check_web_api_key() is False
|
||||
|
||||
def test_configured_firecrawl_backend_accepts_managed_gateway(self):
|
||||
with patch("tools.web_tools._load_web_config", return_value={"backend": "firecrawl"}):
|
||||
|
||||
Reference in New Issue
Block a user