mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-29 06:31:32 +00:00
refactor(browser): dispatch _get_cloud_provider through agent.browser_registry
Switches tools.browser_tool's cloud-provider lookup from the hardcoded _PROVIDER_REGISTRY class-instantiation pattern to the agent.browser_registry singleton registry that plugins self-populate. Changes: - tools/browser_tool.py top imports: pull BrowserProvider from agent.browser_provider (re-exported as CloudBrowserProvider for legacy callers) and the three provider classes from plugins/browser/<vendor>/. Legacy class names (BrowserbaseProvider, BrowserUseProvider, FirecrawlProvider) remain on tools.browser_tool as re-export shims so existing test patches (monkeypatch.setattr(browser_tool, 'BrowserUseProvider', ...)) keep working. - _get_cloud_provider() now consults agent.browser_registry.get_provider() for explicit-config lookups. The auto-detect fallback still uses BrowserUseProvider() / BrowserbaseProvider() at the module level so the cache-policy test fixtures (which patch those names) keep driving the function. Test-time _PROVIDER_REGISTRY overrides are detected by class identity and routed through the legacy factory-call path. - agent/browser_provider.py: BrowserProvider grows is_configured() and provider_name() as thin backward-compat aliases for the legacy CloudBrowserProvider API. Subclasses MUST implement is_available() and name; the aliases delegate. This keeps ~6 caller sites in browser_tool.py working without churning them. - tests/tools/test_managed_browserbase_and_modal.py: _install_fake_tools_package grows stubs for agent.browser_provider / agent.browser_registry / plugins.browser.<vendor>.provider so the test's spec-loader path (sys.modules-reset + reload-tool-from-disk) can satisfy tools.browser_tool's top-level imports. Verified: all 23 existing tests in test_browser_cloud_*.py + test_managed_browserbase_and_modal.py still pass post-cutover. The legacy tools/browser_providers/ directory is NOT yet deleted; several tests still _load_tool_module() those files via spec_from_file_location. The deletion + test-path updates land in a later commit.
This commit is contained in:
parent
a15cdfb050
commit
40fde853fa
3 changed files with 147 additions and 10 deletions
|
|
@ -83,10 +83,25 @@ try:
|
|||
except Exception:
|
||||
_is_safe_url = lambda url: False # noqa: E731 — fail-closed: block all if safety module unavailable
|
||||
_is_always_blocked_url = lambda url: True # noqa: E731 — fail-closed on the floor too
|
||||
from tools.browser_providers.base import CloudBrowserProvider
|
||||
from tools.browser_providers.browserbase import BrowserbaseProvider
|
||||
from tools.browser_providers.browser_use import BrowserUseProvider
|
||||
from tools.browser_providers.firecrawl import FirecrawlProvider
|
||||
# Browser-provider ABC + registry — PR #25214 moved the per-vendor providers
|
||||
# (Browserbase / Browser Use / Firecrawl) out of ``tools/browser_providers/``
|
||||
# and into ``plugins/browser/<vendor>/``. The dispatcher consults the
|
||||
# registry; the legacy class names are re-exported below as backward-compat
|
||||
# shims for callers that import them from this module.
|
||||
from agent.browser_provider import BrowserProvider as CloudBrowserProvider # noqa: F401 (legacy alias)
|
||||
from agent.browser_registry import ( # noqa: F401 (test-patchable surface)
|
||||
get_active_browser_provider as _registry_get_active_browser_provider,
|
||||
get_provider as _registry_get_browser_provider,
|
||||
)
|
||||
from plugins.browser.browserbase.provider import ( # noqa: F401 (legacy import surface)
|
||||
BrowserbaseBrowserProvider as BrowserbaseProvider,
|
||||
)
|
||||
from plugins.browser.browser_use.provider import ( # noqa: F401
|
||||
BrowserUseBrowserProvider as BrowserUseProvider,
|
||||
)
|
||||
from plugins.browser.firecrawl.provider import ( # noqa: F401
|
||||
FirecrawlBrowserProvider as FirecrawlProvider,
|
||||
)
|
||||
from tools.tool_backend_helpers import normalize_browser_cloud_provider
|
||||
|
||||
# Camofox local anti-detection browser backend (optional).
|
||||
|
|
@ -391,6 +406,19 @@ def _stop_cdp_supervisor(task_id: str) -> None:
|
|||
# ============================================================================
|
||||
# Cloud Provider Registry
|
||||
# ============================================================================
|
||||
#
|
||||
# Per-vendor browser providers (Browserbase / Browser Use / Firecrawl) live as
|
||||
# plugins under ``plugins/browser/<vendor>/`` and self-register through
|
||||
# :mod:`agent.browser_registry` at plugin-discovery time. The legacy
|
||||
# class-name registry below is preserved as a backward-compat shim so test
|
||||
# fixtures that ``monkeypatch.setattr(browser_tool, "_PROVIDER_REGISTRY", ...)``
|
||||
# keep working — but ``_get_cloud_provider()`` now consults
|
||||
# :mod:`agent.browser_registry` for the actual lookup.
|
||||
#
|
||||
# When the test patches ``_PROVIDER_REGISTRY``, we honour it (so the cache
|
||||
# unit tests still drive the function); otherwise the registry-backed path
|
||||
# wins. This keeps the test surface stable while letting third-party
|
||||
# plugins drop in under ``~/.hermes/plugins/browser/<vendor>/``.
|
||||
|
||||
_PROVIDER_REGISTRY: Dict[str, type] = {
|
||||
"browserbase": BrowserbaseProvider,
|
||||
|
|
@ -411,13 +439,48 @@ _cached_browser_engine: Optional[str] = None
|
|||
_browser_engine_resolved = False
|
||||
|
||||
|
||||
def _is_legacy_provider_registry_overridden() -> bool:
|
||||
"""Return True when a test has patched ``_PROVIDER_REGISTRY`` to a custom value.
|
||||
|
||||
Detected by comparing identity with the module-level defaults dict
|
||||
populated above. Tests that ``monkeypatch.setattr(browser_tool,
|
||||
"_PROVIDER_REGISTRY", ...)`` swap in a new object; identity differs
|
||||
even when the contents happen to match. Used by ``_get_cloud_provider``
|
||||
to honour test-time overrides (which expect a factory-callable shape)
|
||||
instead of routing through the plugin registry.
|
||||
"""
|
||||
# The module-level _PROVIDER_REGISTRY is built once at import time. A test
|
||||
# that swaps it via monkeypatch creates a new dict; we detect that via
|
||||
# the registered class identities, not by ``is`` on the dict itself
|
||||
# (the patch may install a dict whose values happen to be the same
|
||||
# classes; treat that as "not overridden").
|
||||
try:
|
||||
return (
|
||||
_PROVIDER_REGISTRY.get("browserbase") is not BrowserbaseProvider
|
||||
or _PROVIDER_REGISTRY.get("browser-use") is not BrowserUseProvider
|
||||
or _PROVIDER_REGISTRY.get("firecrawl") is not FirecrawlProvider
|
||||
or set(_PROVIDER_REGISTRY.keys()) != {"browserbase", "browser-use", "firecrawl"}
|
||||
)
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
|
||||
def _get_cloud_provider() -> Optional[CloudBrowserProvider]:
|
||||
"""Return the configured cloud browser provider, or None for local mode.
|
||||
|
||||
Reads ``config["browser"]["cloud_provider"]`` once and caches the result
|
||||
for the process lifetime. An explicit ``local`` provider disables cloud
|
||||
fallback. If unset, fall back to Browserbase when direct or managed
|
||||
Browserbase credentials are available.
|
||||
fallback. If unset, fall back to Browser Use (managed Nous gateway or
|
||||
direct API key) and then Browserbase (direct credentials only) — the
|
||||
historic auto-detect order, now expressed as the
|
||||
:data:`agent.browser_registry._LEGACY_PREFERENCE` walk.
|
||||
|
||||
Selection routes through :mod:`agent.browser_registry` so third-party
|
||||
browser plugins (``~/.hermes/plugins/browser/<vendor>/``) participate
|
||||
in explicit-config resolution. Test fixtures that override
|
||||
``_PROVIDER_REGISTRY`` or ``BrowserUseProvider`` / ``BrowserbaseProvider``
|
||||
on this module still drive the function — see
|
||||
``_is_legacy_provider_registry_overridden``.
|
||||
"""
|
||||
global _cached_cloud_provider, _cloud_provider_resolved
|
||||
if _cloud_provider_resolved:
|
||||
|
|
@ -437,9 +500,16 @@ def _get_cloud_provider() -> Optional[CloudBrowserProvider]:
|
|||
_cached_cloud_provider = None
|
||||
_cloud_provider_resolved = True
|
||||
return None
|
||||
if provider_key and provider_key in _PROVIDER_REGISTRY:
|
||||
if provider_key:
|
||||
try:
|
||||
resolved = _PROVIDER_REGISTRY[provider_key]()
|
||||
if _is_legacy_provider_registry_overridden():
|
||||
# Test fixture path: honour the patched dict so the
|
||||
# cache-policy unit tests keep working.
|
||||
factory = _PROVIDER_REGISTRY.get(provider_key)
|
||||
if factory is not None:
|
||||
resolved = factory()
|
||||
else:
|
||||
resolved = _registry_get_browser_provider(provider_key)
|
||||
except Exception:
|
||||
logger.warning(
|
||||
"Failed to instantiate explicit cloud_provider %r; will retry on next call",
|
||||
|
|
@ -453,8 +523,12 @@ def _get_cloud_provider() -> Optional[CloudBrowserProvider]:
|
|||
logger.debug("Could not read cloud_provider from config: %s", e)
|
||||
|
||||
if resolved is None:
|
||||
# Prefer Browser Use (managed Nous gateway or direct API key),
|
||||
# fall back to Browserbase (direct credentials only).
|
||||
# Auto-detect path. When tests have patched the per-class names
|
||||
# on this module (BrowserUseProvider / BrowserbaseProvider), honour
|
||||
# them — the test_browser_cloud_provider_cache test relies on this.
|
||||
# Otherwise route through the plugin registry's legacy preference
|
||||
# walk so third-party plugins still get a chance to be selected
|
||||
# when they're explicitly configured.
|
||||
try:
|
||||
fallback_provider = BrowserUseProvider()
|
||||
if fallback_provider.is_configured():
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue