diff --git a/agent/models_dev.py b/agent/models_dev.py index 5fcad6b6615..54030c0965c 100644 --- a/agent/models_dev.py +++ b/agent/models_dev.py @@ -273,27 +273,52 @@ def _mark_stale_cache_grace() -> None: _models_dev_cache_time = grace_time +def _commit_registry(data: Dict[str, Any], *, where: str) -> None: + """Persist a freshly fetched registry: disk + in-mem + clear backoff. + + Callers must hold ``_models_dev_fetch_lock`` so a failing refresh on one + path can never stomp the state a succeeding refresh on the other path + just committed (e.g. a failing background worker re-arming the backoff + immediately after a successful ``force_refresh``). + """ + global _models_dev_cache, _models_dev_cache_time, _models_dev_retry_after + _save_disk_cache(data) + _models_dev_cache = data + _models_dev_cache_time = time.time() + _models_dev_retry_after = 0 + logger.debug( + "Refreshed models.dev registry (%s): %d providers, %d total models", + where, + len(data), + sum(len(p.get("models", {})) for p in data.values() if isinstance(p, dict)), + ) + + +def _note_refresh_failure(exc: Exception, *, where: str) -> None: + """Record a failed refresh: arm the process-wide 5-minute backoff. + + Callers must hold ``_models_dev_fetch_lock`` (see ``_commit_registry``). + """ + global _models_dev_retry_after + _models_dev_retry_after = time.time() + _MODELS_DEV_RETRY_DELAY + logger.debug( + "models.dev refresh failed (%s); retry suppressed for %ds: %s", + where, + _MODELS_DEV_RETRY_DELAY, + exc, + ) + + def _background_refresh_models_dev() -> None: """Best-effort refresh after serving stale cache data.""" - global _models_dev_cache, _models_dev_cache_time - global _models_dev_retry_after, _models_dev_refresh_in_flight + global _models_dev_refresh_in_flight try: data = _fetch_models_dev_from_network() - _save_disk_cache(data) - _models_dev_cache = data - _models_dev_cache_time = time.time() - _models_dev_retry_after = 0 - logger.debug( - "Refreshed models.dev registry in background: %d providers", - len(data), - ) + with _models_dev_fetch_lock: + _commit_registry(data, where="background") except Exception as e: - _models_dev_retry_after = time.time() + _MODELS_DEV_RETRY_DELAY - logger.debug( - "Background models.dev refresh failed; retry suppressed for %ds: %s", - _MODELS_DEV_RETRY_DELAY, - e, - ) + with _models_dev_fetch_lock: + _note_refresh_failure(e, where="background") finally: with _models_dev_refresh_lock: _models_dev_refresh_in_flight = False @@ -436,27 +461,10 @@ def fetch_models_dev( try: data = _fetch_models_dev_from_network() - _save_disk_cache(data) - _models_dev_cache = data - _models_dev_cache_time = time.time() - _models_dev_retry_after = 0 - logger.debug( - "Fetched models.dev registry: %d providers, %d total models", - len(data), - sum( - len(p.get("models", {})) - for p in data.values() - if isinstance(p, dict) - ), - ) + _commit_registry(data, where="foreground") return data except Exception as e: - _models_dev_retry_after = time.time() + _MODELS_DEV_RETRY_DELAY - logger.debug( - "Failed to fetch models.dev; retry suppressed for %ds: %s", - _MODELS_DEV_RETRY_DELAY, - e, - ) + _note_refresh_failure(e, where="foreground") # Stage 5: network failed — return any stale memory/disk cache. Cache # freshness remains expired; the retry-after timestamp controls when diff --git a/gateway/run.py b/gateway/run.py index 3fbcb43cf81..f5f902bc9c9 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -13129,10 +13129,9 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew _msg_config_ctx = None if _msg_config_ctx is not None and isinstance(_msg_model_cfg, dict): try: - from hermes_cli.route_identity import should_clear_context_pin + from hermes_cli.route_identity import should_clear_context_pin_async - if await asyncio.to_thread( - should_clear_context_pin, + if await should_clear_context_pin_async( None, # model match already checked above None, _msg_model_cfg.get("base_url"), @@ -13725,10 +13724,9 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew if _hyg_config_context_length is not None: try: - from hermes_cli.route_identity import should_clear_context_pin + from hermes_cli.route_identity import should_clear_context_pin_async - if await asyncio.to_thread( - should_clear_context_pin, + if await should_clear_context_pin_async( _hyg_configured_model, _hyg_model, _hyg_configured_base_url, diff --git a/gateway/slash_commands.py b/gateway/slash_commands.py index 43daaaa3c90..c8e28a14101 100644 --- a/gateway/slash_commands.py +++ b/gateway/slash_commands.py @@ -2008,10 +2008,9 @@ class GatewaySlashCommandsMixin: _persist_model_cfg = {} _persist_cfg["model"] = _persist_model_cfg try: - from hermes_cli.route_identity import should_clear_context_pin + from hermes_cli.route_identity import should_clear_context_pin_async - if await asyncio.to_thread( - should_clear_context_pin, + if await should_clear_context_pin_async( _persist_model_cfg.get("default") or _persist_model_cfg.get("model"), result.new_model, @@ -2337,10 +2336,9 @@ class GatewaySlashCommandsMixin: model_cfg = {} cfg["model"] = model_cfg try: - from hermes_cli.route_identity import should_clear_context_pin + from hermes_cli.route_identity import should_clear_context_pin_async - if await asyncio.to_thread( - should_clear_context_pin, + if await should_clear_context_pin_async( model_cfg.get("default") or model_cfg.get("model"), result.new_model, model_cfg.get("base_url"), diff --git a/hermes_cli/route_identity.py b/hermes_cli/route_identity.py index cb64a3ed817..03ae6ea7c9a 100644 --- a/hermes_cli/route_identity.py +++ b/hermes_cli/route_identity.py @@ -74,3 +74,31 @@ def should_clear_context_pin( ) except Exception: return True + + +async def should_clear_context_pin_async( + configured_model: Any, + active_model: Any, + configured_base_url: Any, + active_base_url: Any, + configured_provider: Any, + active_provider: Any, +) -> bool: + """Async wrapper for ``should_clear_context_pin``. + + Offloads the route comparison to a worker thread so async gateway + handlers never run it on the event loop — the resolution chain is + cache-only (``allow_network=False``) but can still do cold-start disk + I/O. Shares all logic with the sync version — no code duplication. + """ + import asyncio + + return await asyncio.to_thread( + should_clear_context_pin, + configured_model, + active_model, + configured_base_url, + active_base_url, + configured_provider, + active_provider, + ) diff --git a/tests/agent/test_models_dev.py b/tests/agent/test_models_dev.py index 4b9ad5f71fb..b76490ec330 100644 --- a/tests/agent/test_models_dev.py +++ b/tests/agent/test_models_dev.py @@ -344,11 +344,12 @@ class TestFetchModelsDev: return_value=md._MODELS_DEV_CACHE_TTL + 60, ), patch.object(md, "_load_disk_cache", return_value=SAMPLE_REGISTRY): first = fetch_models_dev() - # Wait for the background refresh worker to finish so its - # failure backoff is observable and requests.get stays patched. - deadline = time.time() + 5 - while md._models_dev_refresh_in_flight and time.time() < deadline: - time.sleep(0.01) + # Join the background refresh worker so its failure backoff is + # observable and requests.get stays patched for its lifetime. + for worker in threading.enumerate(): + if worker.name == "models-dev-refresh": + worker.join(timeout=5) + assert not worker.is_alive() assert first == SAMPLE_REGISTRY assert not md._models_dev_refresh_in_flight @@ -364,6 +365,30 @@ class TestFetchModelsDev: assert not md._models_dev_refresh_in_flight mock_get.assert_called_once() + @patch("agent.models_dev.requests.get") + def test_background_refresh_success_commits_registry(self, mock_get): + """The bg worker must save disk + swap mem cache + clear backoff.""" + import agent.models_dev as md + + response = MagicMock() + response.json.return_value = SAMPLE_REGISTRY + mock_get.return_value = response + + md._models_dev_cache = {"stale": {}} + md._models_dev_cache_time = 0 + md._models_dev_retry_after = time.time() - 1 + + with patch.object(md, "_save_disk_cache") as mock_save: + # Run the worker synchronously — deterministic, no thread. + md._models_dev_refresh_in_flight = True + md._background_refresh_models_dev() + + mock_save.assert_called_once_with(SAMPLE_REGISTRY) + assert md._models_dev_cache == SAMPLE_REGISTRY + assert md._models_dev_cache_time > 0 + assert md._models_dev_retry_after == 0 + assert not md._models_dev_refresh_in_flight + @patch("agent.models_dev.requests.get") def test_missing_cache_failure_enters_backoff(self, mock_get): import agent.models_dev as md