diff --git a/routstr/core/main.py b/routstr/core/main.py index 05d4c615..aeaea5e8 100644 --- a/routstr/core/main.py +++ b/routstr/core/main.py @@ -103,8 +103,11 @@ async def lifespan(_: FastAPI) -> AsyncGenerator[None, None]: btc_price_task = asyncio.create_task(update_prices_periodically()) pricing_task = asyncio.create_task(update_sats_pricing()) if global_settings.models_refresh_interval_seconds > 0: + # Pass the accessor (not its current value) so the loop sees providers + # added/changed via reinitialize_upstreams() instead of staying pinned + # to the startup snapshot. models_refresh_task = asyncio.create_task( - refresh_upstreams_models_periodically(get_upstreams()) + refresh_upstreams_models_periodically(get_upstreams) ) model_maps_refresh_task = asyncio.create_task(refresh_model_maps_periodically()) payout_task = asyncio.create_task(periodic_payout()) diff --git a/routstr/upstream/helpers.py b/routstr/upstream/helpers.py index 92c3a46d..535b6fef 100644 --- a/routstr/upstream/helpers.py +++ b/routstr/upstream/helpers.py @@ -3,7 +3,7 @@ from __future__ import annotations import asyncio import os import re -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Callable if TYPE_CHECKING: from ..core.settings import Settings @@ -122,12 +122,16 @@ async def get_all_models_with_overrides( async def refresh_upstreams_models_periodically( - upstreams: list[BaseUpstreamProvider], + upstreams_provider: ( + Callable[[], list[BaseUpstreamProvider]] | list[BaseUpstreamProvider] + ), ) -> None: """Background task to periodically refresh models cache for all providers. Args: - upstreams: List of upstream provider instances + upstreams_provider: Either a callable returning the live upstream list + (preferred — picks up providers added/changed via reinitialize_upstreams), + or a static list (legacy, will go stale after reinitialize_upstreams). """ import asyncio import random @@ -139,9 +143,14 @@ async def refresh_upstreams_models_periodically( logger.info("Provider models refresh disabled (interval <= 0)") return + def _resolve_upstreams() -> list[BaseUpstreamProvider]: + if callable(upstreams_provider): + return upstreams_provider() + return upstreams_provider + while True: try: - for upstream in upstreams: + for upstream in _resolve_upstreams(): try: await upstream.refresh_models_cache() except Exception as e: diff --git a/tests/unit/test_models_refresh_loop.py b/tests/unit/test_models_refresh_loop.py new file mode 100644 index 00000000..1073e868 --- /dev/null +++ b/tests/unit/test_models_refresh_loop.py @@ -0,0 +1,117 @@ +"""Regression tests for the periodic upstream models refresh loop.""" + +from __future__ import annotations + +import asyncio +import os +from typing import cast +from unittest.mock import AsyncMock + +import pytest + +os.environ.setdefault("UPSTREAM_BASE_URL", "http://test") +os.environ.setdefault("UPSTREAM_API_KEY", "test") + +from routstr.upstream.base import BaseUpstreamProvider # noqa: E402 + + +class _FakeUpstream: + """Minimal stand-in for BaseUpstreamProvider used by the refresh loop. + + Only ``base_url`` (for error logging) and ``refresh_models_cache`` (the call + under test) are exercised; everything else stays unused. + """ + + def __init__(self, name: str) -> None: + self.base_url = f"http://{name}" + self.refresh_models_cache = AsyncMock() + + +def _make_fake_upstream(name: str) -> BaseUpstreamProvider: + # The loop only uses duck-typed attributes — cast keeps the test type-clean + # without dragging in BaseUpstreamProvider's full constructor. + return cast(BaseUpstreamProvider, _FakeUpstream(name)) + + +@pytest.mark.asyncio +async def test_refresh_loop_picks_up_providers_added_after_startup( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """If a provider is added after the loop starts (e.g. via reinitialize_upstreams), + the next loop iteration must refresh it. Previously the loop captured the upstream + list at startup and missed any later additions.""" + from routstr.core.settings import settings as global_settings + from routstr.upstream.helpers import refresh_upstreams_models_periodically + + # Tight interval so the test finishes quickly. + monkeypatch.setattr( + global_settings, "models_refresh_interval_seconds", 1, raising=False + ) + + initial_upstream = _make_fake_upstream("initial") + live_list: list[BaseUpstreamProvider] = [initial_upstream] + + # Stub out the post-iteration sats-pricing refresh so the loop body has no DB deps. + async def _noop_pricing_refresh() -> None: # pragma: no cover - trivial stub + return None + + monkeypatch.setattr( + "routstr.payment.models._update_sats_pricing_once", + _noop_pricing_refresh, + ) + + task = asyncio.create_task( + refresh_upstreams_models_periodically(lambda: live_list) + ) + + try: + # Wait for the first iteration to refresh the initial upstream. + for _ in range(40): + if initial_upstream.refresh_models_cache.await_count >= 1: # type: ignore[attr-defined] + break + await asyncio.sleep(0.05) + assert initial_upstream.refresh_models_cache.await_count >= 1, ( # type: ignore[attr-defined] + "loop did not refresh the initial upstream within the timeout" + ) + + # Simulate reinitialize_upstreams: replace the live list contents with new + # provider instances. The loop must observe the swap on its next tick. + new_upstream = _make_fake_upstream("added-after-startup") + live_list[:] = [new_upstream] + + for _ in range(60): + if new_upstream.refresh_models_cache.await_count >= 1: # type: ignore[attr-defined] + break + await asyncio.sleep(0.05) + + assert new_upstream.refresh_models_cache.await_count >= 1, ( # type: ignore[attr-defined] + "loop did not refresh the upstream added after startup — " + "regression: list snapshot captured at startup" + ) + finally: + task.cancel() + try: + await task + except asyncio.CancelledError: + pass + + +@pytest.mark.asyncio +async def test_refresh_loop_disabled_when_interval_non_positive( + monkeypatch: pytest.MonkeyPatch, +) -> None: + from routstr.core.settings import settings as global_settings + from routstr.upstream.helpers import refresh_upstreams_models_periodically + + monkeypatch.setattr( + global_settings, "models_refresh_interval_seconds", 0, raising=False + ) + + upstream = _make_fake_upstream("never-refreshed") + + # Loop must return immediately without ever touching the upstream. + await asyncio.wait_for( + refresh_upstreams_models_periodically(lambda: [upstream]), + timeout=1.0, + ) + upstream.refresh_models_cache.assert_not_awaited() # type: ignore[attr-defined]