From c015663b215c0e14de4295346b0727db602cbb1d Mon Sep 17 00:00:00 2001
From: kshitij <82637225+kshitijk4poor@users.noreply.github.com>
Date: Fri, 7 Aug 2026 21:34:04 +0530
Subject: [PATCH] fix(models): corrupt-at cache rows degrade to live fetch in
 cached_provider_model_ids

Surfaced during the post-merge review pass on our own #81113 follow-up:
cached_fetch_api_models gained _cache_entry_valid (numeric-'at'
validation) but its sibling cached_provider_model_ids still did
float(entry.get('at', 0)), which raises ValueError/TypeError on a
hand-edited or corrupted provider_models_cache.json row and propagates
uncaught into the /model picker call sites. Same fix, same helper:
corrupt rows are now a cache miss (live fetch), never an exception.
Both wrappers now share the identical validity predicate, closing the
divergence the 'mirrors' docstring promised away.

Also two test nits from the same review: unused OrderedDict import
dropped and the drain-order assertion strengthened to pin LRU-first
FIFO order in tests/gateway/test_agent_cache_pressure.py.

Mutation-checked: restoring the raising float() form makes the new
corrupt-at tests fail.
---
 hermes_cli/models.py                       | 17 +++--------------
 tests/gateway/test_agent_cache_pressure.py |  4 +---
 tests/hermes_cli/test_model_cache_swr.py   | 19 +++++++++++++++++++
 3 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/hermes_cli/models.py b/hermes_cli/models.py
index c18b29dc1fb93..546c415f1f25b 100644
--- a/hermes_cli/models.py
+++ b/hermes_cli/models.py
@@ -3287,14 +3287,8 @@ def cached_provider_model_ids(
     entry = cache.get(normalized)
     now = time.time()
 
-    if (
-        not force_refresh
-        and isinstance(entry, dict)
-        and entry.get("fp") == fp
-        and isinstance(entry.get("models"), list)
-        and entry["models"]
-    ):
-        age = now - float(entry.get("at", 0))
+    if not force_refresh and _cache_entry_valid(entry, fp):
+        age = now - entry["at"]
         if age < ttl_seconds:
             return list(entry["models"])
         if age < _PROVIDER_MODELS_STALE_SERVE_MAX:
@@ -3318,12 +3312,7 @@ def cached_provider_model_ids(
     # Live fetch returned nothing. If we have a stale entry with the
     # SAME fingerprint, prefer it over an empty result — stale data
     # beats no data when the network is flaky.
-    if (
-        isinstance(entry, dict)
-        and entry.get("fp") == fp
-        and isinstance(entry.get("models"), list)
-        and entry["models"]
-    ):
+    if _cache_entry_valid(entry, fp):
         return list(entry["models"])
     return list(live or [])
 
diff --git a/tests/gateway/test_agent_cache_pressure.py b/tests/gateway/test_agent_cache_pressure.py
index e77acaefa51d7..38a651427ec34 100644
--- a/tests/gateway/test_agent_cache_pressure.py
+++ b/tests/gateway/test_agent_cache_pressure.py
@@ -433,8 +433,6 @@ class TestSalvageFollowups:
         reference pins the evicted agents during gc.collect + malloc_trim
         (otherwise the in-pass trim frees nothing and the next tick
         over-evicts another batch)."""
-        from collections import OrderedDict as _OD
-
         from gateway.run import GatewayRunner
 
         runner = GatewayRunner.__new__(GatewayRunner)
@@ -455,7 +453,7 @@ class TestSalvageFollowups:
 
         runner._release_pressure_batch(plan)
 
-        assert sorted(released) == ["s0", "s1", "s2"]
+        assert released == ["s0", "s1", "s2"], "LRU-first (FIFO) release order"
         assert plan_len_at_trim["len"] == 0, (
             "plan still held agent references when trim_memory ran"
         )
diff --git a/tests/hermes_cli/test_model_cache_swr.py b/tests/hermes_cli/test_model_cache_swr.py
index e2eeaedbb3135..bdeda9df86c64 100644
--- a/tests/hermes_cli/test_model_cache_swr.py
+++ b/tests/hermes_cli/test_model_cache_swr.py
@@ -183,3 +183,22 @@ class TestCatalogSWR:
         assert out == manifest
         fetch.assert_called_once()
         spawn.assert_not_called()
+
+
+class TestCorruptCacheRowDegradation:
+    """A corrupted 'at' in the user-editable provider_models_cache.json must
+    degrade cached_provider_model_ids to a cache miss (live fetch), never
+    raise through the picker (which has no try/except at its call sites)."""
+
+    @pytest.mark.parametrize("bad_at", ["yesterday", None, True])
+    def test_corrupt_at_falls_back_to_live_fetch(self, bad_at):
+        import hermes_cli.models as mod
+
+        cache = {"openrouter": {"fp": "fp", "at": bad_at, "models": ["corrupt-row"]}}
+        with patch.object(mod, "_load_provider_models_cache", return_value=cache), \
+             patch.object(mod, "_credential_fingerprint", return_value="fp"), \
+             patch.object(mod, "_save_provider_models_cache"), \
+             patch.object(mod, "provider_model_ids", return_value=["live-model"]) as live:
+            out = mod.cached_provider_model_ids("openrouter")
+        assert out == ["live-model"]
+        live.assert_called_once()