diff --git a/plugins/browser/browser_use/provider.py b/plugins/browser/browser_use/provider.py index 3d371bdd8..46a220333 100644 --- a/plugins/browser/browser_use/provider.py +++ b/plugins/browser/browser_use/provider.py @@ -119,17 +119,20 @@ class BrowserUseBrowserProvider(BrowserProvider): return "Browser Use" def is_available(self) -> bool: - return self._get_config_or_none() is not None + return self._get_config_or_none(refresh_token=False) is not None # ------------------------------------------------------------------ # Config resolution (direct API key OR managed Nous gateway) # ------------------------------------------------------------------ - def _get_config_or_none(self) -> Optional[Dict[str, Any]]: + def _get_config_or_none(self, *, refresh_token: bool = True) -> Optional[Dict[str, Any]]: # Import here to avoid a hard dependency at module-import time — # managed_tool_gateway pulls in the Nous auth stack which can be # heavy and is not needed for direct-API-key users. - from tools.managed_tool_gateway import resolve_managed_tool_gateway + from tools.managed_tool_gateway import ( + peek_nous_access_token, + resolve_managed_tool_gateway, + ) from tools.tool_backend_helpers import prefers_gateway # Direct API key wins unless the user has explicitly opted into the @@ -142,7 +145,11 @@ class BrowserUseBrowserProvider(BrowserProvider): "managed_mode": False, } - managed = resolve_managed_tool_gateway("browser-use") + # Keep availability scans off the synchronous OAuth refresh path. + managed = resolve_managed_tool_gateway( + "browser-use", + token_reader=None if refresh_token else peek_nous_access_token, + ) if managed is None: return None diff --git a/plugins/web/firecrawl/provider.py b/plugins/web/firecrawl/provider.py index 9e3f123e5..0fa99bf58 100644 --- a/plugins/web/firecrawl/provider.py +++ b/plugins/web/firecrawl/provider.py @@ -146,16 +146,16 @@ def _get_firecrawl_gateway_url() -> str: def _is_tool_gateway_ready() -> bool: """Return True when gateway URL + Nous Subscriber token are available. - Reads ``read_nous_access_token`` and ``resolve_managed_tool_gateway`` + Reads ``peek_nous_access_token`` and ``resolve_managed_tool_gateway`` via :mod:`tools.web_tools` rather than direct imports, so unit tests - that ``patch("tools.web_tools._read_nous_access_token", ...)`` see + that ``patch("tools.web_tools._peek_nous_access_token", ...)`` see their patches honored. The names are re-exported on :mod:`tools.web_tools` for exactly this reason. """ import tools.web_tools as _wt return _wt.resolve_managed_tool_gateway( - "firecrawl", token_reader=_wt._read_nous_access_token + "firecrawl", token_reader=_wt._peek_nous_access_token ) is not None diff --git a/tests/tools/test_managed_browserbase_and_modal.py b/tests/tools/test_managed_browserbase_and_modal.py index fc2559dc7..f70538099 100644 --- a/tests/tools/test_managed_browserbase_and_modal.py +++ b/tests/tools/test_managed_browserbase_and_modal.py @@ -234,6 +234,44 @@ def test_browserbase_does_not_use_gateway_only_configuration(): assert provider.is_available() is False +def test_browser_use_availability_skips_refresh_for_expired_cached_gateway_token(tmp_path, monkeypatch): + _install_fake_tools_package() + monkeypatch.delenv("TOOL_GATEWAY_USER_TOKEN", raising=False) + expired_at = "2000-01-01T00:00:00+00:00" + (tmp_path / "auth.json").write_text( + '{"providers":{"nous":{"access_token":"expired-token","refresh_token":"refresh-token","expires_at":"%s"}}}' + % expired_at, + encoding="utf-8", + ) + refresh_calls = [] + + def _record_refresh(*, refresh_skew_seconds=120, **_kwargs): + refresh_calls.append(refresh_skew_seconds) + return "fresh-token" + + monkeypatch.setattr( + "hermes_cli.auth.resolve_nous_access_token", + _record_refresh, + ) + + env = os.environ.copy() + env.pop("BROWSER_USE_API_KEY", None) + env.update({ + "HERMES_HOME": str(tmp_path), + "BROWSER_USE_GATEWAY_URL": "http://127.0.0.1:3009", + }) + + with patch.dict(os.environ, env, clear=True): + browser_use_module = _load_plugin_module( + "plugins.browser.browser_use.provider", + "browser/browser_use/provider.py", + ) + provider = browser_use_module.BrowserUseBrowserProvider() + assert provider.is_available() is True + + assert refresh_calls == [] + + def test_browser_use_managed_gateway_adds_idempotency_key_and_persists_external_call_id(): _install_fake_tools_package() env = os.environ.copy() diff --git a/tests/tools/test_managed_tool_gateway.py b/tests/tools/test_managed_tool_gateway.py index a539fb57c..2973259ba 100644 --- a/tests/tools/test_managed_tool_gateway.py +++ b/tests/tools/test_managed_tool_gateway.py @@ -12,6 +12,7 @@ assert MODULE_SPEC and MODULE_SPEC.loader managed_tool_gateway = module_from_spec(MODULE_SPEC) sys.modules[MODULE_SPEC.name] = managed_tool_gateway MODULE_SPEC.loader.exec_module(managed_tool_gateway) +is_managed_tool_gateway_ready = managed_tool_gateway.is_managed_tool_gateway_ready resolve_managed_tool_gateway = managed_tool_gateway.resolve_managed_tool_gateway @@ -97,3 +98,37 @@ def test_read_nous_access_token_refreshes_expiring_cached_token(tmp_path, monkey ) assert managed_tool_gateway.read_nous_access_token() == "fresh-token" + + +def test_is_managed_tool_gateway_ready_skips_refresh_for_expired_cached_token(tmp_path, monkeypatch): + monkeypatch.delenv("TOOL_GATEWAY_USER_TOKEN", raising=False) + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + expired_at = (datetime.now(timezone.utc) - timedelta(seconds=30)).isoformat() + (tmp_path / "auth.json").write_text(json.dumps({ + "providers": { + "nous": { + "access_token": "expired-token", + "refresh_token": "refresh-token", + "expires_at": expired_at, + } + } + })) + refresh_calls = [] + + def _record_refresh(*, refresh_skew_seconds=120, **_kwargs): + refresh_calls.append(refresh_skew_seconds) + return "fresh-token" + + monkeypatch.setattr( + "hermes_cli.auth.resolve_nous_access_token", + _record_refresh, + ) + + with patch.dict( + os.environ, + {"TOOL_GATEWAY_DOMAIN": "nousresearch.com"}, + clear=False, + ), patch.object(managed_tool_gateway, "managed_nous_tools_enabled", return_value=True): + assert is_managed_tool_gateway_ready("modal") is True + + assert refresh_calls == [] diff --git a/tests/tools/test_web_tools_config.py b/tests/tools/test_web_tools_config.py index 87fc27cc3..e9bcd8e20 100644 --- a/tests/tools/test_web_tools_config.py +++ b/tests/tools/test_web_tools_config.py @@ -623,10 +623,49 @@ class TestCheckWebApiKey: assert check_web_api_key() is True def test_tool_gateway_returns_true(self): - with patch("tools.web_tools._read_nous_access_token", return_value="nous-token"): + with patch("tools.web_tools._peek_nous_access_token", return_value="nous-token"): from tools.web_tools import check_web_api_key assert check_web_api_key() is True + def test_tool_gateway_availability_skips_refresh_for_expired_cached_token( + self, + tmp_path, + monkeypatch, + ): + monkeypatch.delenv("TOOL_GATEWAY_USER_TOKEN", raising=False) + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + expired_at = "2000-01-01T00:00:00+00:00" + (tmp_path / "auth.json").write_text(json.dumps({ + "providers": { + "nous": { + "access_token": "expired-token", + "refresh_token": "refresh-token", + "expires_at": expired_at, + } + } + })) + refresh_calls = [] + + def _record_refresh(*, refresh_skew_seconds=120, **_kwargs): + refresh_calls.append(refresh_skew_seconds) + return "fresh-token" + + monkeypatch.setattr( + "hermes_cli.auth.resolve_nous_access_token", + _record_refresh, + ) + + 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 True + + assert refresh_calls == [] + 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"): @@ -636,7 +675,7 @@ class TestCheckWebApiKey: def test_configured_firecrawl_backend_accepts_managed_gateway(self): with patch("tools.web_tools._load_web_config", return_value={"backend": "firecrawl"}): - with patch("tools.web_tools._read_nous_access_token", return_value="nous-token"): + with patch("tools.web_tools._peek_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 True diff --git a/tools/managed_tool_gateway.py b/tools/managed_tool_gateway.py index cd27537fd..d894dcb4b 100644 --- a/tools/managed_tool_gateway.py +++ b/tools/managed_tool_gateway.py @@ -72,15 +72,34 @@ def _access_token_is_expiring(expires_at: object, skew_seconds: int) -> bool: return remaining <= max(0, int(skew_seconds)) -def read_nous_access_token() -> Optional[str]: - """Read a Nous Subscriber OAuth access token from auth store or env override.""" +def peek_nous_access_token() -> Optional[str]: + """Cheap probe for a Nous gateway token without triggering refresh. + + Availability scans (`hermes tools`, banner/status paint, provider + `is_available()` checks) must stay off the synchronous OAuth refresh path. + This helper therefore only inspects the explicit env override and the + cached auth-store token, without checking expiry and without making any + network calls. Truthful refresh handling stays in request/session paths + that call :func:`read_nous_access_token`. + """ explicit = os.getenv("TOOL_GATEWAY_USER_TOKEN") if isinstance(explicit, str) and explicit.strip(): return explicit.strip() nous_provider = _read_nous_provider_state() or {} access_token = nous_provider.get("access_token") - cached_token = access_token.strip() if isinstance(access_token, str) and access_token.strip() else None + if isinstance(access_token, str) and access_token.strip(): + return access_token.strip() + return None + + +def read_nous_access_token() -> Optional[str]: + """Read a Nous Subscriber OAuth access token from auth store or env override.""" + explicit = os.getenv("TOOL_GATEWAY_USER_TOKEN") + if isinstance(explicit, str) and explicit.strip(): + return explicit.strip() + nous_provider = _read_nous_provider_state() or {} + cached_token = peek_nous_access_token() if cached_token and not _access_token_is_expiring( nous_provider.get("expires_at"), @@ -159,9 +178,15 @@ def is_managed_tool_gateway_ready( gateway_builder: Optional[Callable[[str], str]] = None, token_reader: Optional[Callable[[], Optional[str]]] = None, ) -> bool: - """Return True when gateway URL and Nous access token are available.""" + """Return True when gateway URL and a likely-usable Nous token are present. + + Defaults to :func:`peek_nous_access_token` so read-only availability scans + avoid synchronous OAuth refresh. Callers that are about to make a real + gateway request should use :func:`resolve_managed_tool_gateway` (which + still defaults to the refresh-aware :func:`read_nous_access_token`). + """ return resolve_managed_tool_gateway( vendor, gateway_builder=gateway_builder, - token_reader=token_reader, + token_reader=token_reader or peek_nous_access_token, ) is not None diff --git a/tools/web_tools.py b/tools/web_tools.py index 509546fd5..d03f6865d 100644 --- a/tools/web_tools.py +++ b/tools/web_tools.py @@ -93,6 +93,7 @@ from tools.debug_helpers import DebugSession # tools.web_tools (the firecrawl plugin reads them via its own import chain). from tools.managed_tool_gateway import ( # noqa: F401 — backward-compat names for tests build_vendor_gateway_url, + peek_nous_access_token as _peek_nous_access_token, read_nous_access_token as _read_nous_access_token, resolve_managed_tool_gateway, )