fix(managed-gateway): keep tool availability scans off the Nous token-refresh path
This commit is contained in:
@ -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
|
||||
|
||||
|
||||
@ -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
|
||||
|
||||
|
||||
|
||||
@ -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()
|
||||
|
||||
@ -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 == []
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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,
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user