Web search/extract provider architecture and backend selection

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #57779 fix(web_tools): honor plugin-registered provider availability at the tool gate

  merged to main

  files: scripts/release.py, tests/tools/test_web_tools_config.py, tools/web_tools.py

  Show the diff

  • scripts/release.py

    @@ -1135,6 +1135,7 @@
         "[redacted:email]": "hata1234",
         "[redacted:email]": "Hmbown",
         "[redacted:email]": "m0n5t3r",
    +    "[redacted:email]": "m0n5t3r",
         "[redacted:email]": "JiayuuWang",
         "[redacted:email]": "sjh9714",
         "[redacted:email]": "yoniebans",

  • tests/tools/test_web_tools_config.py

    @@ -668,3 +668,122 @@ def test_web_requires_env_includes_exa_key():
         from tools.web_tools import _web_requires_env
     
         assert "EXA_API_KEY" in _web_requires_env()
    +
    +
    +class TestNonBuiltinProviderAvailability:
    +    """Regression: a plugin-registered WebSearchProvider with no built-in
    +    provider credentials must still light up web_search / web_extract tools.
    +
    +    The web_tools availability gate delegates non-legacy backend names to the
    +    web_search_registry's provider ``is_available()``.  This class verifies
    +    that a custom (non-built-in) provider discovered via the registry is
    +    sufficient to make check_web_api_key() return True, _get_backend() return
    +    the custom name, the per-capability selection honor it (issue #32698), and
    +    the tool registry entries remain active.
    +
    +    Original tests contributed by @m0n5t3r (PR #28652 / issue #28651).
    +    """
    +
    +    # All env vars that could make a built-in provider available.
    +    _WEB_ENV_KEYS = (
    +        "EXA_API_KEY",
    +        "PARALLEL_API_KEY",
    +        "FIRECRAWL_API_KEY",
    +        "FIRECRAWL_API_URL",
    +        "FIRECRAWL_GATEWAY_URL",
    +        "TOOL_GATEWAY_DOMAIN",
    +        "TOOL_GATEWAY_SCHEME",
    +        "TOOL_GATEWAY_USER_TOKEN",
    +        "TAVILY_API_KEY",
    +        "SEARXNG_URL",
    +        "BRAVE_SEARCH_API_KEY",
    +        "XAI_API_KEY",
    +    )
    +
    +    @staticmethod
    +    def _create_fake_provider(*, search=True, extract=True):
    +        """Dynamically create a WebSearchProvider subclass.
    +
    +        Uses a local class definition (not a nested class) to avoid
    +        Python 3.13 __bases__ deallocator issue with nested class
    +        reassignment.
    +        """
    +        from agent.web_search_provider import WebSearchProvider
    +
    +        class FakePluginProvider(WebSearchProvider):
    +            @property
    +            def name(self):
    +                return "fake-plugin-prov"
    +
    +            def is_available(self):
    +                return True
    +
    +            def supports_search(self):
    +                return search
    +
    +            def supports_extract(self):
    +                return extract
    +
    +        return FakePluginProvider()
    +
    +    def setup_method(self):
    +        """Strip all built-in web provider env vars and reset the registry."""
    +        for key in self._WEB_ENV_KEYS:
    +            os.environ.pop(key, None)
    +        from agent.web_search_registry import _reset_for_tests, register_provider
    +        _reset_for_tests()
    +        register_provider(self._create_fake_provider())
    +
    +    def teardown_method(self):
    +        """Reset the registry and restore env after each test."""
    +        from agent.web_search_registry import _reset_for_tests
    +        _reset_for_tests()
    +        for key in self._WEB_ENV_KEYS:
    +            os.environ.pop(key, None)
    +
    +    def test_check_web_api_key_returns_true_for_custom_provider(self):
    +        """With only a custom provider registered (no built-in creds),
    +        check_web_api_key() must return True."""
    +        with patch("tools.web_tools._ddgs_package_importable", return_value=False), \
    +             patch("tools.web_tools._peek_nous_access_token", return_value=None):
    +            from tools.web_tools import check_web_api_key
    +            assert check_web_api_key() is True
    +
    +    def test_get_backend_discovers_custom_provider(self):
    +        """_get_backend() must return the custom provider name when it's
    +        the only available provider."""
    +        with patch("tools.web_tools._ddgs_package_importable", return_value=False), \
    +             patch("tools.web_tools._peek_nous_access_token", return_value=None):
    +            from tools.web_tools import _get_backend
    +            assert _get_backend() == "fake-plugin-prov"
    +
    +    def test_is_backend_available_delegates_to_registry(self):
    +        """_is_backend_available() must consult the registry for a
    +        non-legacy backend name."""
    +        from tools.web_tools import _is_backend_available
    +        assert _is_backend_available("fake-plugin-prov") is True
    +        # Unknown, unregistered name -> False (no legacy probe matches).
    +        assert _is_backend_available("totally-unknown-backend") is False
    +
    +    def test_capability_backend_honors_custom_extract_provider(self):
    +        """Per-capability selection (_get_extract_backend) must resolve the
    +        custom provider when configured, instead of dead-ending — issue #32698."""
    +        with patch("tools.web_tools._ddgs_package_importable", return_value=False), \
    +             patch("tools.web_tools._peek_nous_access_token", return_value=None), \
    +             patch("tools.web_tools._load_web_config",
    +                   return_value={"extract_backend": "fake-plugin-prov"}):
    +            from tools.web_tools import _get_extract_backend
    +            assert _get_extract_backend() == "fake-plugin-prov"
    +
    +    def test_tool_registry_entries_not_filtered_out(self):
    +        """web_search and web_extract tool entries must remain in the
    +        registry when only a custom provider is available."""
    +        with patch("tools.web_tools._ddgs_package_importable", return_value=False), \
    +             patch("tools.web_tools._peek_nous_access_token", return_value=None):
    +            import tools.web_tools
    +            web_search_entry = tools.web_tools.registry.get_entry("web_search")
    +            web_extract_entry = tools.web_tools.registry.get_entry("web_extract")
    +            assert web_search_entry is not None, \
    +                "web_search tool was filtered out despite custom provider being available"
    +            assert web_extract_entry is not None, \
    +                "web_extract tool was filtered out despite custom provider being available"

  • tools/web_tools.py

    @@ -136,6 +136,71 @@ def _load_web_config() -> dict:
         except (ImportError, Exception):
             return {}
     
    +
    +# The built-in web backends whose availability is driven by hardcoded
    +# env-var / package / OAuth probes below. Any name NOT in this set is a
    +# candidate plugin-registered provider and must be resolved through the
    +# web_search_registry (``is_available()``) instead. Kept as a single named
    +# constant so the whitelist early-returns and the availability chokepoint
    +# stay in sync.
    +#
    +# NOTE: this intentionally includes ``xai``, which the registry's
    +# ``_LEGACY_PREFERENCE`` does NOT — xai availability is probed via
    +# ``has_xai_credentials()`` (env var OR auth.json OAuth), not a registered
    +# WebSearchProvider. Keep the two sets aligned by hand: if xai ever ships as
    +# a registered provider, drop it here so the registry path takes over.
    +_LEGACY_WEB_BACKENDS = frozenset(
    +    {"parallel", "firecrawl", "tavily", "exa", "searxng", "brave-free", "ddgs", "xai"}
    +)
    +
    +
    +def _registered_web_provider(backend: str):
    +    """Return a plugin-registered web provider by name, or ``None``.
    +
    +    Consults ``agent.web_search_registry`` so backends contributed by the
    +    plugin system (which are absent from :data:`_LEGACY_WEB_BACKENDS`) are
    +    discoverable during availability/selection resolution. Returns ``None``
    +    on any lookup failure so callers can fall through to legacy checks.
    +    """
    +    if not backend:
    +        return None
    +    try:
    +        from agent.web_search_registry import get_provider
    +
    +        return get_provider(backend)
    +    except Exception as exc:  # noqa: BLE001 — registry optional; never fatal
    +        logger.debug("web provider registry lookup failed for %r: %s", backend, exc)
    +        return None
    +
    +
    +def _registered_web_provider_available(backend: str):
    +    """Availability of a *registered* web provider, or ``None`` if unregistered.
    +
    +    Returns ``True``/``False`` when *backend* names a registered provider
    +    (calling its ``is_available()``), or ``None`` when it isn't registered —
    +    letting the caller fall through to the legacy built-in probes.
    +    """
    +    provider = _registered_web_provider(backend)
    +    if provider is None:
    +        return None
    +    try:
    +        return bool(provider.is_available())
    +    except Exception as exc:  # noqa: BLE001 — a broken provider is "unavailable"
    +        logger.debug("web provider %r.is_available() raised: %s", backend, exc)
    +        return False
    +
    +
    +def _list_registered_web_providers():
    +    """Return all plugin-registered web providers (empty list on failure)."""
    +    try:
    +        from agent.web_search_registry import list_providers
    +
    +        return list_providers()
    +    except Exception as exc:  # noqa: BLE001 — registry optional; never fatal
    +        logger.debug("web provider registry list failed: %s", exc)
    +        return []
    +
    +
     def _get_backend() -> str:
         """Determine which web backend to use (shared fallback).
     
    @@ -144,7 +209,7 @@ def _get_backend() -> str:
         keys manually without running setup.
         """
         configured = (_load_web_config().get("backend") or "").lower().strip()
    -    if configured in {"parallel", "firecrawl", "tavily", "exa", "searxng", "brave-free", "ddgs", "xai"}:
    +    if configured in _LEGACY_WEB_BACKENDS or _registered_web_provider(configured) is not None:
             return configured
     
         # Fallback for manual / legacy config — pick the highest-priority
    @@ -168,6 +233,21 @@ def _get_backend() -> str:
             if available:
                 return backend
     
    +    # Final fallback: walk plugin-registered providers so a custom backend
    +    # (with no built-in creds present) still resolves. Built-in names are
    +    # already covered above, so this only surfaces plugin-contributed
    +    # providers via their own is_available() gate. We hold the provider
    +    # object already, so probe it directly rather than round-tripping through
    +    # _is_backend_available() (which would re-do the registry lookup).
    +    for provider in _list_registered_web_providers():
    +        if provider.name in _LEGACY_WEB_BACKENDS:
    +            continue
    +        try:
    +            if provider.is_available():
    +                return provider.name
    +        except Exception as exc:  # noqa: BLE001 — a broken provider is skipped
    +            logger.debug("web provider %r.is_available() raised: %s", provider.name, exc)
    +
         return "firecrawl"  # default (backward compat)
     
     
    @@ -210,7 +290,22 @@ def _get_capability_backend(capability: str) -> str:
     
     
     def _is_backend_available(backend: str) -> bool:
    -    """Return True when the selected backend is currently usable."""
    +    """Return True when the selected backend is currently usable.
    +
    +    For plugin-registered backends (any name outside
    +    :data:`_LEGACY_WEB_BACKENDS`), availability is delegated to the
    +    provider's ``is_available()`` via the web_search_registry. This is the
    +    single chokepoint through which ``_get_backend``,
    +    ``_get_capability_backend``, and ``check_web_api_key`` all resolve
    +    availability — fixing custom-provider discovery for every caller at once
    +    (issues #28651, #31873, #32698). Built-in backends keep their cheap
    +    hardcoded probes below.
    +    """
    +    backend = (backend or "").lower().strip()
    +    if backend not in _LEGACY_WEB_BACKENDS:
    +        registered = _registered_web_provider_available(backend)
    +        if registered is not None:
    +            return registered
         if backend == "exa":
             return _has_env("EXA_API_KEY")
         if backend == "parallel":
    @@ -861,14 +956,39 @@ async def web_extract_tool(
     
     # Convenience function to check Firecrawl credentials
     def check_web_api_key() -> bool:
    -    """Check whether the configured web backend is available."""
    +    """Check whether the configured web backend is available.
    +
    +    Used as the ``check_fn`` gate for the ``web_search`` and ``web_extract``
    +    tool registry entries — so a plugin-registered provider that reports
    +    ``is_available()`` must light the tools up even when no built-in backend
    +    has credentials (issues #28651, #31873). Resolution funnels through
    +    :func:`_is_backend_available`, which delegates non-legacy names to the
    +    registry.
    +    """
         configured = _load_web_config().get("backend", "").lower().strip()
    -    if configured in {"exa", "parallel", "firecrawl", "tavily", "searxng", "brave-free", "ddgs", "xai"}:
    -        return _is_backend_available(configured)
    -    return any(
    -        _is_backend_available(backend)
    -        for backend in ("exa", "parallel", "firecrawl", "tavily", "searxng", "brave-free", "ddgs", "xai")
    -    )
    +    if configured and _is_backend_available(configured):
    +        return True
    +    # Any built-in backend with credentials present. This is a boolean OR, so
    +    # unlike _get_backend() the probe order is irrelevant.
    +    if any(_is_backend_available(backend) for backend in _LEGACY_WEB_BACKENDS):
    +        return True
    +    # Any plugin-registered provider the registry considers active for either
    +    # capability. Delegating to the registry's own availability-filtered
    +    # resolvers keeps a single authority for "is a custom provider usable"
    +    # rather than re-implementing the walk here.
    +    try:
    +        from agent.web_search_registry import (
    +            get_active_search_provider,
    +            get_active_extract_provider,
    +        )
    +
    +        return (
    +            get_active_search_provider() is not None
    +            or get_active_extract_provider() is not None
    +        )
    +    except Exception as exc:  # noqa: BLE001 — registry optional; never fatal
    +        logger.debug("web provider registry availability check failed: %s", exc)
    +        return False
     
     
     if __name__ == "__main__":