From 8603ca0b8fd94a0ccb1956fbafe0c58415b51341 Mon Sep 17 00:00:00 2001
From: ciregenz <ciregenz@berkeley.edu>
Date: Thu, 13 Aug 2026 07:26:37 -0700
Subject: [PATCH] [eric] mcp-registry: cold open waits for the first crawl, not
 an empty page (ENG-288)

---
 backend/apps/mcp_registry/mcp_registry.py     | 41 +++++++++--
 .../test_mcp_registry_crawls_on_demand.py     | 69 +++++++++++++++++++
 2 files changed, 105 insertions(+), 5 deletions(-)

diff --git a/backend/apps/mcp_registry/mcp_registry.py b/backend/apps/mcp_registry/mcp_registry.py
index 7f175c9b..bccb4f25 100644
--- a/backend/apps/mcp_registry/mcp_registry.py
+++ b/backend/apps/mcp_registry/mcp_registry.py
@@ -8,6 +8,8 @@ from typing import Optional
 
 import httpx
 from fastapi import Query
+from typeguard import typechecked
+
 from backend.config.Apps import SubApp
 
 logger = logging.getLogger(__name__)
@@ -15,6 +17,7 @@ logger = logging.getLogger(__name__)
 REGISTRY_BASE = "https://registry.modelcontextprotocol.io/v0.1"
 PAGE_LIMIT = 100
 REFRESH_INTERVAL_S = 3600
+FIRST_LOAD_WAIT_S = 25.0
 
 GITHUB_TOKEN = os.environ.get("GITHUB_TOKEN", "")
 GITHUB_BATCH = 4000 if GITHUB_TOKEN else 50
@@ -322,6 +325,34 @@ def arm_registry_refresh() -> None:
         p_start_refresh_task()
 
 
+@typechecked
+def registry_server_count() -> int:
+    """How many servers are cached right now; 0 means the first crawl has not landed."""
+    return len(p_cache)
+
+
+@typechecked
+async def ensure_registry_ready(timeout_s: float = FIRST_LOAD_WAIT_S) -> bool:
+    """Arm the crawl and wait for the first server list, so a cold open is not a blank page.
+
+    Arming alone was not enough and shipped a real regression: the route fired the task and then
+    read the cache in the same breath, so the first Marketplace open after a boot returned zero
+    servers and a detail lookup 404'd, for as long as the ~215-request crawl took. Idle cost went
+    to zero and the feature went with it.
+
+    Polls rather than waiting on an Event: an asyncio primitive here would have to be loop-local to
+    avoid the ENG-219 hang, and this needs no cross-task signalling to earn that complexity.
+
+    Bounded on purpose. If the crawl is slow the caller still gets whatever is cached rather than
+    hanging, which is exactly the old behaviour and never worse.
+    """
+    arm_registry_refresh()
+    deadline = time.monotonic() + timeout_s
+    while not registry_server_count() and time.monotonic() < deadline:
+        await asyncio.sleep(0.05)
+    return registry_server_count() > 0
+
+
 @asynccontextmanager
 async def mcp_registry_lifespan():
     global p_refresh_task
@@ -340,11 +371,11 @@ mcp_registry = SubApp("mcp-registry", mcp_registry_lifespan)
 
 @mcp_registry.router.get("/stats")
 async def registry_stats():
-    arm_registry_refresh()
+    await ensure_registry_ready()
     google = sum(1 for s in p_cache.values() if s.get("source") == "google")
     community = sum(1 for s in p_cache.values() if s.get("source") == "community")
     return {
-        "total": len(p_cache),
+        "total": registry_server_count(),
         "google": google,
         "community": community,
         "lastUpdated": p_cache_updated_at,
@@ -359,7 +390,7 @@ async def registry_search(
     sort: str = Query("name", description="Sort by: name, stars"),
     source: str = Query("", description="Filter by source: google, community, or empty for all"),
 ):
-    arm_registry_refresh()
+    ready = await ensure_registry_ready()
     pool = p_cache.values()
     if source:
         pool = [s for s in pool if s.get("source") == source]
@@ -400,12 +431,12 @@ async def registry_search(
         for s in page
     ]
 
-    return {"servers": summary, "total": total, "offset": offset, "limit": limit}
+    return {"servers": summary, "total": total, "offset": offset, "limit": limit, "loading": not ready}
 
 
 @mcp_registry.router.get("/detail/{server_name:path}")
 async def registry_detail(server_name: str):
-    arm_registry_refresh()
+    await ensure_registry_ready()
     srv = p_cache.get(server_name)
     if not srv:
         return {"error": "Server not found"}, 404
diff --git a/backend/tests/test_mcp_registry_crawls_on_demand.py b/backend/tests/test_mcp_registry_crawls_on_demand.py
index be7b1300..febf3911 100644
--- a/backend/tests/test_mcp_registry_crawls_on_demand.py
+++ b/backend/tests/test_mcp_registry_crawls_on_demand.py
@@ -72,3 +72,72 @@ async def test_arming_twice_starts_one_crawl(monkeypatch: Any) -> None:
         mcp_registry.arm_registry_refresh()
     assert started == ["armed"], f"armed {len(started)} crawlers for 5 requests"
     mcp_registry.p_refresh_task = None
+
+
+# --- Going lazy cost the feature on a cold open, which is the regression these three cover. ---
+#
+# Arming is not the same as being ready. The first version of the fix fired the task and then read
+# the cache in the same breath, so the first Marketplace open after a boot returned zero servers and
+# a detail lookup 404'd for as long as the ~215-request crawl ran. Idle cost went to zero and so did
+# the feature. Waiting for the first server list restores it without re-arming anything at boot.
+
+
+@pytest.fixture
+def p_cold_registry() -> Any:
+    """Cold cache, crawl already 'armed' so these tests exercise the wait and not the arming."""
+    mcp_registry.p_cache = {}
+    mcp_registry.p_refresh_task = object()
+    yield
+    mcp_registry.p_cache = {}
+    mcp_registry.p_refresh_task = None
+
+
+@pytest.mark.asyncio
+async def test_a_cold_request_waits_for_the_first_server_list(p_cold_registry: Any) -> None:
+    """The regression: a cold open must not be answered with an empty list."""
+    import asyncio
+
+    async def p_crawl_finishes_shortly() -> None:
+        await asyncio.sleep(0.05)
+        mcp_registry.p_cache = {"acme/thing": {"name": "acme/thing"}}
+
+    task = asyncio.create_task(p_crawl_finishes_shortly())
+    ready = await mcp_registry.ensure_registry_ready(timeout_s=3.0)
+    # Snapshot BEFORE awaiting the crawl: reading the cache afterwards lets the background task fill
+    # it in and the test passes even when the wait was skipped entirely. That vacuous version
+    # survived the mutation run, which is the whole reason this line exists.
+    cache_when_it_returned = mcp_registry.registry_server_count()
+    await task
+
+    assert ready is True, "a cold request gave up instead of waiting for the crawl it just armed"
+    assert cache_when_it_returned > 0, (
+        "returned ready while the cache was still empty, so the Marketplace renders a blank page "
+        "on the first open after a boot"
+    )
+
+
+@pytest.mark.asyncio
+async def test_a_slow_crawl_never_hangs_the_request(p_cold_registry: Any) -> None:
+    """Bounded the other way: nobody sets the gate, so the caller must still get an answer."""
+    import time
+
+    t0 = time.perf_counter()
+    ready = await mcp_registry.ensure_registry_ready(timeout_s=0.1)
+    elapsed = time.perf_counter() - t0
+
+    assert ready is False, "claimed ready while the cache was empty and nothing had loaded"
+    assert elapsed < 2.0, f"waited {elapsed:.2f}s on a 0.1s budget, so a slow crawl blocks the request"
+
+
+@pytest.mark.asyncio
+async def test_a_warm_cache_does_not_wait(p_cold_registry: Any) -> None:
+    """Every request after the first must be free; a per-request wait would be its own regression."""
+    import time
+
+    mcp_registry.p_cache = {"acme/thing": {"name": "acme/thing"}}
+    t0 = time.perf_counter()
+    ready = await mcp_registry.ensure_registry_ready(timeout_s=5.0)
+    elapsed = time.perf_counter() - t0
+
+    assert ready is True
+    assert elapsed < 0.05, f"a warm request still waited {elapsed:.3f}s"