harden(#4555): isolate stale-cache path in test fixture + enforce schema-version equality on stale fallback
Applies Opus ship-gate findings: (1) the autouse fixture now points _get_models_cache_path at an isolated temp file so the over-budget static-fallback test can't read a real ~/.../models_cache.json and become an order-dependent flake; (2) _load_stale_models_cache_from_disk rejects a cross-schema cache (its groups/badge shape may be incompatible with the current picker). Adds a cross-schema-rejection test. Co-authored-by: starship-s <starship-s@users.noreply.github.com>
This commit is contained in:
parent
3d9487cbaf
commit
fbafe1e623
2 changed files with 33 additions and 2 deletions
|
|
@ -4618,7 +4618,10 @@ def _load_stale_models_cache_from_disk() -> dict | None:
|
|||
The main cache loader enforces metadata stamps for a full cold-path cache hit.
|
||||
This helper intentionally does not apply that stricter policy, so we can still
|
||||
recover a useful fallback payload when the strict loader rejected cache because
|
||||
metadata or fingerprint fields are stale.
|
||||
metadata or fingerprint fields are stale. It DOES still enforce the schema
|
||||
version: a cross-schema cache can have an incompatible groups/badge shape, so
|
||||
serving it to the picker could surface a broken catalog — schema mismatch is a
|
||||
hard reject even on the fallback path.
|
||||
"""
|
||||
try:
|
||||
import json as _j
|
||||
|
|
@ -4630,6 +4633,8 @@ def _load_stale_models_cache_from_disk() -> dict | None:
|
|||
cache = _j.load(f)
|
||||
if not _is_valid_models_cache(cache):
|
||||
return None
|
||||
if cache.get("_schema_version") != _MODELS_CACHE_SCHEMA_VERSION:
|
||||
return None
|
||||
aliases = cache.get("aliases")
|
||||
return {
|
||||
"active_provider": cache["active_provider"],
|
||||
|
|
|
|||
|
|
@ -29,6 +29,13 @@ def isolate_models_catalog_state(monkeypatch, tmp_path):
|
|||
monkeypatch.setattr(cfg, "_get_auth_store_path", lambda: auth_store_path)
|
||||
monkeypatch.setattr(cfg, "_load_models_cache_from_disk", lambda: None)
|
||||
monkeypatch.setattr(cfg, "_save_models_cache_to_disk", lambda *_a, **_k: None)
|
||||
# Point the stale-cache loader's path at an isolated (by default nonexistent)
|
||||
# temp file so the over-budget stale fallback (#3928 follow-up) stays
|
||||
# hermetic — otherwise it would read the real ~/.../models_cache.json and
|
||||
# tests asserting the static fallback become order/environment-dependent
|
||||
# flakes. Tests exercising the stale path override _get_models_cache_path
|
||||
# to write their own payload.
|
||||
monkeypatch.setattr(cfg, "_get_models_cache_path", lambda: tmp_path / "models_cache.json")
|
||||
monkeypatch.setattr(cfg, "_delete_models_cache_on_disk", lambda: None)
|
||||
monkeypatch.setattr(
|
||||
cfg,
|
||||
|
|
@ -129,7 +136,10 @@ def _build_stale_disk_cache_payload() -> dict:
|
|||
"aliases": {
|
||||
"chat": "ollama-cloud/chat-1",
|
||||
},
|
||||
"_schema_version": 999,
|
||||
# Current schema (so the stale-cache loader's schema guard accepts it),
|
||||
# but a deliberately stale _webui_version + fingerprint so the STRICT
|
||||
# loader rejects it — this is exactly the "recoverable stale cache" case.
|
||||
"_schema_version": cfg._MODELS_CACHE_SCHEMA_VERSION,
|
||||
"_webui_version": "v999",
|
||||
"_source_fingerprint": {
|
||||
"catalog": "stale",
|
||||
|
|
@ -375,6 +385,22 @@ def test_load_stale_models_cache_from_disk_defaults_missing_aliases(
|
|||
assert stale["aliases"] == {}
|
||||
|
||||
|
||||
def test_load_stale_models_cache_from_disk_rejects_cross_schema(
|
||||
monkeypatch,
|
||||
isolate_models_catalog_state,
|
||||
):
|
||||
"""A shape-valid but cross-schema cache must be rejected even on the stale
|
||||
fallback path — its groups/badge shape may be incompatible with the current
|
||||
picker and serving it could surface a broken catalog."""
|
||||
payload = _build_stale_disk_cache_payload()
|
||||
payload["_schema_version"] = cfg._MODELS_CACHE_SCHEMA_VERSION + 1
|
||||
models_cache_path = isolate_models_catalog_state["models_cache_path"]
|
||||
monkeypatch.setattr(cfg, "_get_models_cache_path", lambda: models_cache_path)
|
||||
models_cache_path.write_text(json.dumps(payload), encoding="utf-8")
|
||||
|
||||
assert cfg._load_stale_models_cache_from_disk() is None
|
||||
|
||||
|
||||
def test_default_group_survives_only_as_emergency_last_resort(
|
||||
monkeypatch,
|
||||
isolate_models_catalog_state,
|
||||
|
|
|
|||
Loading…
Reference in a new issue