diff --git a/Architecture.md b/Architecture.md index bb47d7f..4d1cacb 100644 --- a/Architecture.md +++ b/Architecture.md @@ -459,8 +459,18 @@ current model count. This is the observability/trust requirement made concrete. its own at all, returns an empty list in that case rather than the shared Ollama-tag catalog's invalid names. - `BundledRegistry` — reads `catalog.json` (offline fallback). - - `RemoteRegistry` — optional fetch/refresh from a URL/JSON for community - updates without a package release. + - `RemoteRegistry` (`adapters/registry/remote.py`) — optional fetch of a + `catalog.json`-shaped document from `registry_url`, for community updates + without a package release. Built on `CachedCatalogRegistry`, but + cache-*first*: the disk cache (`/remote_catalog_cache.json`, + 1-hour TTL) answers on its own inside the TTL so a configured URL costs no + round-trip per CLI invocation; `refresh()` bypasses it, and once a live + fetch fails an expired cache is still preferred over none. The remote + entries are merged *over* the bundled catalog rather than replacing it — + a remote entry wins a name collision, but no bundled model is ever + dropped, so a remote URL can only add to discovery. Non-`http(s)` URLs are + rejected up front, the response body is size-capped, and a single + malformed entry is skipped rather than discarding the whole payload. - `CompositeRegistry` (`adapters/registry/composite.py`) — merges an ordered list of `RegistryPort` sources into one: `search`/`list_all`/ `by_category`/`recommend` union every source's results (earlier source @@ -471,6 +481,11 @@ current model count. This is the observability/trust requirement made concrete. - **Configuration:** `catalog_source` setting in `Settings` controls which registry is used: `"ollama"` (dynamic only) and `"bundled"` (static only) are explicit single-source opt-outs with no network beyond that one source. + `"remote"` selects `RemoteRegistry` alone (still merged over bundled, inside + the registry itself) and requires `registry_url`. Setting `registry_url` + under `"auto"` instead merges the remote catalog in ahead of every other + source, so entries published since the last release outrank the shipped + ones; an unusable URL degrades to a warning rather than breaking discovery. `"auto"` (default) tries dynamic Ollama, falls back to bundled, **and** merges in the active backend's own live catalog when it has one — `ModelManager._resolve_backend_catalog` resolves it through diff --git a/CHANGELOG.md b/CHANGELOG.md index 7889531..4371637 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,50 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/), and this ## [Unreleased] +### Added + +- `registry_url` is now wired into discovery. Setting it under + `catalog_source="auto"` merges a remote catalog ahead of every other source; + `catalog_source="remote"` selects it as the only source (and errors when no + `registry_url` is configured). The setting was previously parsed and + displayed but never read. +- `RemoteRegistry` caches its fetch at `/remote_catalog_cache.json` + with a 1-hour TTL, served cache-first so a configured URL costs no network + round-trip per command, and falls back to an expired cache before giving up. + `refresh()` (and therefore `modeldock sources refresh`) bypasses the TTL. +- `catalog_entry_to_spec`/`load_bundled_catalog` (`adapters/registry/bundled.py`) + — the catalog.json entry coercion, shared instead of reached for privately. + +### Changed + +- `RemoteRegistry` now merges remote entries *over* the bundled catalog rather + than replacing it: a remote entry wins a name collision, but no bundled + model is dropped, so a remote URL can only add to discovery. +- `RemoteRegistry` is built on `CachedCatalogRegistry`, so `search`, + `recommend`, `by_category` and `list_all` answer from the merged index, and + `get`/`resolve` honour aliases and casing like every other source. +- `CompositeRegistry.describe()` deduplicates sources by name, so a catalog + merged inside more than one member is listed once by `modeldock sources`. + +### Fixed + +- `RemoteRegistry.search()`/`recommend()` delegated to the bundled fallback, + so a model that existed only in the remote catalog could never be found — + the exact case the remote catalog exists for. +- A successful remote fetch replaced the catalog instead of extending it, + dropping every bundled model from `list_all()`/`by_category()`. +- `RemoteRegistry.get()` matched names exactly, ignoring aliases and casing. +- A single malformed entry in a remote payload discarded the entire fetch. +- `RemoteRegistry` exposed only a private `_refresh`, so `modeldock sources + refresh` and `CompositeRegistry.refresh()` skipped it silently. +- `RemoteRegistry.describe()` reported the URL as its cache path, and reported + the bundled fallback's model count as the remote source's when the fetch had + failed. +- `RemoteRegistry` fetched on every construction with no cache, putting a + network round-trip in front of every CLI invocation. +- `RemoteRegistry` accepted any URL scheme (including `file://`) and buffered + the whole response body unbounded. + ## [0.2.0] - 2026-08-28 Live GGUF catalogs, composite registry, third-party catalog plugins, and diff --git a/Development.md b/Development.md index a6a32be..e7f0368 100644 --- a/Development.md +++ b/Development.md @@ -224,6 +224,19 @@ To add a model to the **bundled fallback**, edit `src/modeldock/data/catalog.jso ``` Optional `RemoteRegistry` can refresh the catalog from a URL without a release. +Point `registry_url` at a document in exactly the shape above (a `models` list +of these entries) and ModelDock merges it over the bundled catalog: + +```toml +# ~/.config/modeldock/config.toml +registry_url = "https://example.com/modeldock-catalog.json" +``` + +Or `MODELDOCK_REGISTRY_URL=... modeldock search llama`. The fetch is cached at +`/remote_catalog_cache.json` for an hour, so only the first command +in that window touches the network; `modeldock sources refresh` forces a +re-fetch, and `modeldock sources` shows what the remote URL contributed. +Setting `catalog_source = "remote"` uses it as the only source. --- diff --git a/QUICKSTART.md b/QUICKSTART.md index 21f4267..f55bf3d 100644 --- a/QUICKSTART.md +++ b/QUICKSTART.md @@ -122,9 +122,14 @@ for 24 hours. Set `catalog_source` in config or `MODELDOCK_CATALOG_SOURCE` env v | Value | Behavior | |-------|----------| -| `auto` | Try dynamic, fallback to bundled (default) | +| `auto` | Try dynamic, fallback to bundled; merges `registry_url` when set (default) | | `ollama` | Dynamic only — requires internet | | `bundled` | Static catalog.json only — fully offline | +| `remote` | `registry_url` only, merged over bundled — requires `registry_url` | + +Set `registry_url` to a `catalog.json`-shaped URL to pick up models published +since the last release. Its entries are merged over the bundled catalog (never +replacing it) and cached for an hour; `modeldock sources refresh` re-fetches. --- diff --git a/README.md b/README.md index 3e47adf..18d1bad 100644 --- a/README.md +++ b/README.md @@ -143,9 +143,14 @@ for 24 hours. Set `catalog_source` in config or `MODELDOCK_CATALOG_SOURCE` env v | Value | Behavior | |-------|----------| -| `auto` | Try dynamic, fallback to bundled (default) | +| `auto` | Try dynamic, fallback to bundled; merges `registry_url` when set (default) | | `ollama` | Dynamic only — requires internet | | `bundled` | Static catalog.json only — fully offline | +| `remote` | `registry_url` only, merged over bundled — requires `registry_url` | + +Set `registry_url` to a `catalog.json`-shaped URL to pick up models published +since the last release. Its entries are merged over the bundled catalog (never +replacing it) and cached for an hour; `modeldock sources refresh` re-fetches. See [Architecture.md](Architecture.md) for the full design contract. diff --git a/src/modeldock/adapters/registry/bundled.py b/src/modeldock/adapters/registry/bundled.py index c91d68d..b5c1676 100644 --- a/src/modeldock/adapters/registry/bundled.py +++ b/src/modeldock/adapters/registry/bundled.py @@ -41,6 +41,30 @@ def _load_catalog() -> List[Dict[str, Any]]: return cast(List[Dict[str, Any]], data.get("models", [])) +def load_bundled_catalog() -> List[Dict[str, Any]]: + """Return the raw entries of the bundled catalog.json. + + Public counterpart to ``_load_catalog`` so other registries can read the + shipped catalog without importing a private name. + """ + return _load_catalog() + + +def catalog_entry_to_spec(raw: Dict[str, Any]) -> ModelSpec: + """Coerce one raw catalog.json entry into a validated ``ModelSpec``. + + Shared by every registry that consumes catalog.json-shaped entries — the + bundled catalog and any remote catalog served in the same format — so the + enum coercion rules live in exactly one place instead of being reached for + across class boundaries. + """ + raw = dict(raw) + raw["category"] = Category.from_value(raw["category"]) + raw["capabilities"] = [Capability.from_value(c) for c in raw.get("capabilities", [])] + raw["backend_hints"] = [RuntimeBackend.from_value(b) for b in raw.get("backend_hints", [])] + return ModelSpec.model_validate(raw) + + class BundledRegistry: """Registry backed by the bundled catalog.json.""" @@ -62,11 +86,7 @@ def _index(self) -> None: @staticmethod def _to_spec(raw: Dict[str, Any]) -> ModelSpec: - raw = dict(raw) - raw["category"] = Category.from_value(raw["category"]) - raw["capabilities"] = [Capability.from_value(c) for c in raw.get("capabilities", [])] - raw["backend_hints"] = [RuntimeBackend.from_value(b) for b in raw.get("backend_hints", [])] - return ModelSpec.model_validate(raw) + return catalog_entry_to_spec(raw) # --- RegistryPort ----------------------------------------------------- @@ -125,4 +145,4 @@ def list_all(self) -> List[ModelSpec]: return list(self._specs.values()) -__all__ = ["BundledRegistry"] +__all__ = ["BundledRegistry", "catalog_entry_to_spec", "load_bundled_catalog"] diff --git a/src/modeldock/adapters/registry/composite.py b/src/modeldock/adapters/registry/composite.py index b50eb71..7b31859 100644 --- a/src/modeldock/adapters/registry/composite.py +++ b/src/modeldock/adapters/registry/composite.py @@ -12,7 +12,7 @@ from __future__ import annotations -from typing import Dict, List, Optional, Sequence +from typing import Dict, List, Optional, Sequence, Set from modeldock.common.errors import ModelNotFoundError from modeldock.domain.model import Category, ModelRef, ModelSpec @@ -78,21 +78,32 @@ def describe(self) -> List[SourceInfo]: Each source that can describe itself contributes its own ``SourceInfo``; a source that predates the ``describe`` contract still appears, as a minimal custom entry, so the enumeration is complete. + + Descriptors are deduplicated by source name, keeping the first: a + member may itself merge a shared source underneath it (``RemoteRegistry`` + overlays the bundled catalog), and listing that source once per member + would misrepresent one catalog as several. """ infos: List[SourceInfo] = [] + seen: Set[str] = set() for source in self._sources: describe = getattr(source, "describe", None) if callable(describe): - infos.extend(describe()) + candidates = list(describe()) else: - infos.append( + candidates = [ SourceInfo( name=type(source).__name__, trust=SourceTrust.CUSTOM, live=True, model_count=len(source.list_all()), ) - ) + ] + for info in candidates: + if info.name in seen: + continue + seen.add(info.name) + infos.append(info) return infos def refresh(self) -> int: diff --git a/src/modeldock/adapters/registry/remote.py b/src/modeldock/adapters/registry/remote.py index c95b241..aaeaf84 100644 --- a/src/modeldock/adapters/registry/remote.py +++ b/src/modeldock/adapters/registry/remote.py @@ -1,94 +1,240 @@ -"""RemoteRegistry — optional refresh of the catalog from a URL. - -Falls back to ``BundledRegistry`` when the remote is unreachable. See -Architecture.md §9/§14. +"""RemoteRegistry — the bundled catalog refreshed from a remote URL. + +Community catalogs move faster than releases: a model published today is +discoverable only once a new ModelDock version ships an updated +``catalog.json``. ``RemoteRegistry`` closes that gap. It fetches a +catalog.json-shaped document from a URL, caches it on disk with a TTL, and +merges it *over* the bundled catalog — so discovery gains the fresh entries +without ever losing the shipped ones, and keeps working offline from the +cache. Built on ``CachedCatalogRegistry``, the shared fetch → cache → index +pipeline every live catalog source uses. See Architecture.md §9/§14. """ from __future__ import annotations -from typing import List, Optional +import json +import sys +from pathlib import Path +from typing import Any, Dict, List, Optional +from urllib.parse import urlparse + +import httpx -from modeldock.adapters.registry.bundled import BundledRegistry -from modeldock.common.errors import ModelNotFoundError +from modeldock.adapters.registry.base import CachedCatalogRegistry +from modeldock.adapters.registry.bundled import BundledRegistry, catalog_entry_to_spec +from modeldock.common.catalog_cache import load_catalog_cache, save_catalog_cache +from modeldock.common.errors import ConfigError from modeldock.common.http import create_client -from modeldock.common.logging import get_logger -from modeldock.domain.model import Category, ModelRef, ModelSpec +from modeldock.common.platform import default_cache_dir +from modeldock.domain.model import ModelSpec from modeldock.domain.source import REMOTE, SourceInfo, SourceTrust - -class RemoteRegistry: - """Registry that fetches catalog entries from a remote URL.""" - - def __init__(self, url: str, fallback: Optional[BundledRegistry] = None) -> None: - self._url = url +_CACHE_FILENAME = "remote_catalog_cache.json" +#: Shorter than the Ollama catalog's 24h: the point of a remote catalog is +#: freshness, and ``modeldock sources refresh`` bypasses the TTL on demand. +_CACHE_TTL_SECONDS = 3600 # 1 hour +_FETCH_TIMEOUT = 15.0 +_MAX_CATALOG_BYTES = 8 * 1024 * 1024 # 8 MiB +#: Sentinel TTL meaning "accept the cache at any age". Used only once a live +#: fetch has already failed, where a stale catalog still beats no catalog. +_ANY_AGE_SECONDS = sys.maxsize +_ALLOWED_SCHEMES = frozenset({"http", "https"}) + + +def _validate_url(url: str) -> str: + """Return ``url`` when it is a usable http(s) catalog URL, else raise. + + The URL comes from user configuration and is fetched unattended, so the + scheme is checked up front: ``file://`` and friends would turn a config + typo into a local-filesystem read. + """ + candidate = (url or "").strip() + if not candidate: + raise ConfigError("registry_url is empty; set a catalog URL to use the remote registry") + parsed = urlparse(candidate) + if parsed.scheme.lower() not in _ALLOWED_SCHEMES: + raise ConfigError(f"Invalid registry_url {url!r}; expected an http:// or https:// URL") + if not parsed.netloc: + raise ConfigError(f"Invalid registry_url {url!r}; missing a host") + return candidate + + +class RemoteRegistry(CachedCatalogRegistry): + """Registry that overlays a remotely fetched catalog onto the bundled one. + + The remote entries are cached on disk and read back for + :data:`_CACHE_TTL_SECONDS` before the network is consulted again, so + ordinary commands pay no round-trip and keep working offline. A remote + entry wins over a bundled entry of the same name — fresher metadata is the + whole reason the remote catalog exists — but a bundled model is never + dropped, so configuring a remote URL can only ever add to discovery. + """ + + def __init__( + self, + url: str, + fallback: Optional[BundledRegistry] = None, + cache_dir: Optional[Path] = None, + ) -> None: + self._url = _validate_url(url) self._fallback = fallback or BundledRegistry() - self._logger = get_logger("registry.remote") - self._specs: List[ModelSpec] = [] - self._refresh() + self._remote_count = 0 + self._remote_available = False + super().__init__( + cache_dir or default_cache_dir(), + _CACHE_FILENAME, + "registry.remote", + source_name=REMOTE, + source_trust=SourceTrust.CUSTOM, + ) - def _refresh(self) -> None: + # --- fetch → cache → index pipeline ------------------------------------ + + def _load(self) -> None: + """Index the bundled catalog, then overlay whatever the remote offers. + + Overrides the base pipeline because the remote catalog is an *overlay*, + not a standalone source: a failed fetch with no usable cache must still + leave the shipped catalog fully intact rather than yield an empty index. + """ + self._build_index(self._remote_entries()) + + def _remote_entries(self) -> List[Dict[str, Any]]: + """A fresh cache if there is one, else the network, else a stale cache. + + Deliberately cache-*first*, unlike the network-first order a scraped + catalog uses: this registry is constructed on every CLI invocation, so + checking the network each time would put a round-trip in front of every + ``search``/``list``. Inside the TTL the cache answers on its own, and + ``refresh()`` bypasses it to force a live fetch. + """ + cached = self._load_cache() + if cached is not None: + self._logger.debug("Serving %d remote entries from cache", len(cached)) + return cached + models = self._fetch_from_network() + if models is not None: + return models + stale = load_catalog_cache(self._cache_path, _ANY_AGE_SECONDS) + if stale is not None: + self._logger.info( + "Remote catalog %s unreachable; serving %d entries from an expired cache", + self._url, + len(stale), + ) + return stale + self._logger.warning( + "Remote catalog %s unavailable and no usable cache; serving the bundled catalog only", + self._url, + ) + return [] + + def _build_index(self, models: List[Dict[str, Any]]) -> None: + """Index the bundled catalog first, then overlay the remote entries. + + A single malformed remote entry is skipped with a warning instead of + discarding the whole payload — one bad record in a community feed must + not cost users every other model in it. + """ + for spec in self._fallback.list_all(): + self._index_spec(spec) + indexed = 0 + for raw in models: + try: + spec = self._to_spec(raw) + except Exception as exc: # noqa: BLE001 - one bad entry must not sink the rest + self._logger.warning("Skipping malformed remote catalog entry (%s)", exc) + continue + if spec.source is None: + spec.source = self._source_name + self._index_spec(spec) + indexed += 1 + self._remote_count = indexed + self._remote_available = indexed > 0 + + def _index_spec(self, spec: ModelSpec) -> None: + """Add ``spec`` to the name/alias indexes, replacing any earlier entry. + + Looking the previous entry up by lowercased name keeps a remote entry + that differs from a bundled one only in casing from leaving a stale + duplicate behind in ``list_all()``. + """ + previous = self._by_alias.get(spec.name.lower()) + if previous is not None and previous != spec.name: + self._specs.pop(previous, None) + self._specs[spec.name] = spec + for alias in spec.aliases: + self._by_alias[alias.lower()] = spec.name + self._by_alias[spec.name.lower()] = spec.name + + def _fetch_from_network(self) -> Optional[List[Dict[str, Any]]]: + """Fetch the remote catalog, writing it to the cache on success. + + Returns ``None`` on any failure — bad status, oversized body, invalid + JSON — so the caller falls back to the cache and then to bundled. + """ try: - with create_client() as client: - resp = client.get(self._url, timeout=10.0) + client = create_client(timeout=_FETCH_TIMEOUT) + with client, client.stream("GET", self._url) as resp: resp.raise_for_status() - data = resp.json() - self._specs = [] - for raw in data.get("models", []): - spec = BundledRegistry._to_spec(raw) - if spec.source is None: - spec.source = REMOTE - self._specs.append(spec) - except Exception as exc: - self._logger.warning("Remote registry unavailable (%s); using bundled", exc) - self._specs = self._fallback.list_all() - - def search(self, query: str) -> List[ModelSpec]: - return self._fallback.search(query) - - def get(self, ref: ModelRef) -> ModelSpec: - for spec in self._specs: - if spec.name == ref.name: - return spec - try: - return self._fallback.get(ref) - except ModelNotFoundError: - raise ModelNotFoundError(ref.name) from None - - def resolve(self, ref: ModelRef) -> ModelSpec: - """Resolve a friendly/alias ``ref`` to its canonical spec.""" - return self.get(ref) - - def versions(self, ref: ModelRef) -> List[str]: - """Return known version tags for ``ref`` (empty when unknown).""" - try: - return self.get(ref).version_tags() - except ModelNotFoundError: - return [] + payload = self._read_capped(resp) + data = json.loads(payload) + if not isinstance(data, dict) or not isinstance(data.get("models"), list): + raise ValueError("catalog payload has no 'models' list") + models: List[Dict[str, Any]] = [m for m in data["models"] if isinstance(m, dict)] + save_catalog_cache(self._cache_path, models) + self._logger.info("Fetched %d models from %s", len(models), self._url) + return models + except Exception as exc: # noqa: BLE001 - being offline is normal, never fatal + self._logger.debug("Remote catalog fetch failed (%s): %s", self._url, exc) + return None + + @staticmethod + def _read_capped(resp: httpx.Response) -> bytes: + """Read a streamed body, refusing to buffer more than the size cap. + + The URL is user-supplied, so the response is streamed and counted + rather than trusted: neither a misconfigured host nor a hostile one + gets to exhaust memory through ``resp.json()``. + """ + declared = resp.headers.get("Content-Length", "") + if declared.isdigit() and int(declared) > _MAX_CATALOG_BYTES: + raise ValueError(f"catalog exceeds the {_MAX_CATALOG_BYTES} byte limit") + chunks: List[bytes] = [] + total = 0 + for chunk in resp.iter_bytes(): + total += len(chunk) + if total > _MAX_CATALOG_BYTES: + raise ValueError(f"catalog exceeds the {_MAX_CATALOG_BYTES} byte limit") + chunks.append(chunk) + return b"".join(chunks) + + def _load_cache(self) -> Optional[List[Dict[str, Any]]]: + return load_catalog_cache(self._cache_path, _CACHE_TTL_SECONDS) + + def _to_spec(self, raw: Dict[str, Any]) -> ModelSpec: + """Convert one catalog.json-shaped remote entry into a ``ModelSpec``.""" + return catalog_entry_to_spec(raw) + + # --- RegistryPort ------------------------------------------------------- def describe(self) -> List[SourceInfo]: - """Describe the remote source (user-supplied; treated as custom trust).""" - return [ - SourceInfo( - name=REMOTE, - trust=SourceTrust.CUSTOM, - live=True, - backend=None, - model_count=len(self._specs), - cache_path=self._url, - available=bool(self._specs), - ) - ] - - def by_category(self, category: Category) -> List[ModelSpec]: - return [s for s in self._specs if s.category == category] or self._fallback.by_category( - category + """Describe the remote source, then the bundled catalog merged under it. + + ``model_count``/``available`` report the *remote* contribution only: + the bundled entries beneath are described by their own source, so + counting them here would overstate what the URL actually provided. + """ + remote = SourceInfo( + name=REMOTE, + trust=SourceTrust.CUSTOM, + live=True, + backend=None, + model_count=self._remote_count, + cache_path=str(self._cache_path), + available=self._remote_available, ) - - def recommend(self, task: str) -> List[ModelSpec]: - return self._fallback.recommend(task) - - def list_all(self) -> List[ModelSpec]: - return self._specs or self._fallback.list_all() + return [remote, *self._fallback.describe()] __all__ = ["RemoteRegistry"] diff --git a/src/modeldock/common/config.py b/src/modeldock/common/config.py index 6ca4121..b8caabc 100644 --- a/src/modeldock/common/config.py +++ b/src/modeldock/common/config.py @@ -39,7 +39,7 @@ class Settings(BaseModel): default_backend: RuntimeBackend = RuntimeBackend.OLLAMA cache_dir: Path = Field(default_factory=default_cache_dir) registry_url: Optional[str] = None - catalog_source: str = "auto" # "auto" | "ollama" | "bundled" + catalog_source: str = "auto" # "auto" | "ollama" | "bundled" | "remote" log_level: str = "ERROR" progress_style: str = "rich" auto_install: bool = False @@ -71,7 +71,7 @@ def _validate_progress_style(cls, value: str) -> str: @field_validator("catalog_source") @classmethod def _validate_catalog_source(cls, value: str) -> str: - allowed = {"auto", "ollama", "bundled"} + allowed = {"auto", "ollama", "bundled", "remote"} if value not in allowed: raise ConfigError( f"Invalid catalog_source {value!r}; expected one of {sorted(allowed)}" diff --git a/src/modeldock/core/manager.py b/src/modeldock/core/manager.py index c2f8368..d541818 100644 --- a/src/modeldock/core/manager.py +++ b/src/modeldock/core/manager.py @@ -17,6 +17,7 @@ from modeldock.adapters.runtimes.registry import RuntimeRegistry from modeldock.common.config import Settings from modeldock.common.errors import ( + ConfigError, DownloadError, ModelNotFoundError, RuntimeUnavailableError, @@ -102,6 +103,11 @@ def _resolve_registry(self, cfg: Settings) -> RegistryPort: active backend cannot actually install. ``"bundled"``/``"ollama"`` stay single-source and network-free/live-only respectively, exactly as before — they are explicit opt-outs of this merge. + + A configured ``registry_url`` adds one more layer: the remote catalog + is merged in ahead of everything else, so entries published since the + last release outrank the shipped ones. ``"remote"`` selects it as the + sole source (still merged over bundled, inside ``RemoteRegistry``). """ source = cfg.catalog_source if source == "bundled": @@ -112,14 +118,22 @@ def _resolve_registry(self, cfg: Settings) -> RegistryPort: from modeldock.adapters.registry.ollama_library import OllamaLibraryRegistry return OllamaLibraryRegistry(cache_dir=cfg.cache_dir) - else: # "auto" — try ollama, fallback to bundled; merge in a backend catalog + elif source == "remote": + return self._resolve_remote_registry(cfg) + else: # "auto" — try ollama, fallback to bundled; merge in extra catalogs base = self._resolve_auto_registry(cfg) + overlays: List[RegistryPort] = [] + remote = self._resolve_optional_remote(cfg) + if remote is not None: + overlays.append(remote) backend_catalog = self._resolve_backend_catalog(cfg) - if backend_catalog is None: + if backend_catalog is not None: + overlays.append(backend_catalog) + if not overlays: return base from modeldock.adapters.registry.composite import CompositeRegistry - return CompositeRegistry([backend_catalog, base]) + return CompositeRegistry([*overlays, base]) def _resolve_auto_registry(self, cfg: Settings) -> RegistryPort: """The general catalog for ``"auto"``: live Ollama, falling back to bundled.""" @@ -140,6 +154,39 @@ def _resolve_auto_registry(self, cfg: Settings) -> RegistryPort: return BundledRegistry() return live + def _resolve_remote_registry(self, cfg: Settings) -> RegistryPort: + """The remote catalog for ``catalog_source="remote"``, bundled merged in. + + Unlike the optional overlay under ``"auto"``, asking for ``"remote"`` + explicitly without a ``registry_url`` is a configuration mistake rather + than something to silently degrade past, so it raises. + """ + from modeldock.adapters.registry.remote import RemoteRegistry + + if not cfg.registry_url: + raise ConfigError( + 'catalog_source is "remote" but no registry_url is set; ' + "set registry_url in your config file or MODELDOCK_REGISTRY_URL" + ) + return RemoteRegistry(cfg.registry_url, cache_dir=cfg.cache_dir) + + def _resolve_optional_remote(self, cfg: Settings) -> Optional[RegistryPort]: + """The remote catalog overlay for ``"auto"``, when a URL is configured. + + Absent or unusable configuration degrades to ``None`` rather than + raising, so a stale or malformed ``registry_url`` can never break + ordinary discovery — exactly like the live catalog's own fallback. + """ + if not cfg.registry_url: + return None + from modeldock.adapters.registry.remote import RemoteRegistry + + try: + return RemoteRegistry(cfg.registry_url, cache_dir=cfg.cache_dir) + except Exception as exc: + self._logger.warning("Remote catalog unusable (%s); continuing without it", exc) + return None + def _resolve_backend_catalog(self, cfg: Settings) -> Optional[RegistryPort]: """The active backend's own live catalog, or None when it has none. diff --git a/tests/unit/test_remote_registry.py b/tests/unit/test_remote_registry.py new file mode 100644 index 0000000..23a8e1a --- /dev/null +++ b/tests/unit/test_remote_registry.py @@ -0,0 +1,503 @@ +"""Tests for RemoteRegistry — a cached remote catalog merged over bundled. + +These drive a real HTTP server on a real socket rather than mocking httpx, so +the caching behaviour that matters (how many requests actually leave the +process) is observable. That request count is the point of several tests: a +catalog that re-fetches on every construction would pass a mocked test and +still put a network round-trip in front of every CLI invocation. +""" + +from __future__ import annotations + +import json +import threading +import time +from http.server import BaseHTTPRequestHandler, HTTPServer +from pathlib import Path +from typing import Any, Callable, Dict, Iterator, List + +import pytest + +from modeldock.adapters.registry import remote as remote_module +from modeldock.adapters.registry.bundled import BundledRegistry +from modeldock.adapters.registry.composite import CompositeRegistry +from modeldock.adapters.registry.remote import RemoteRegistry +from modeldock.common.config import Settings +from modeldock.common.errors import ConfigError, ModelNotFoundError +from modeldock.core.manager import ModelManager +from modeldock.domain.model import Category, ModelRef, RuntimeBackend +from modeldock.domain.source import BUNDLED, REMOTE, SourceTrust + +# --------------------------------------------------------------------------- +# Catalog fixtures +# --------------------------------------------------------------------------- + +#: A model published after the last release — the case the remote catalog exists for. +FRESH_MODEL: Dict[str, Any] = { + "name": "brand-new-model", + "aliases": ["shiny", "Brand-New"], + "category": "chat", + "capabilities": ["chat", "tool_use"], + "default_tag": "latest", + "description": "Published after the last ModelDock release.", + "backend_hints": ["ollama"], + "variants": [{"tag": "latest", "params": "7B"}, {"tag": "70b", "params": "70B"}], +} + +#: Same name as a bundled entry, with metadata the shipped catalog cannot have. +UPDATED_LLAMA3: Dict[str, Any] = { + "name": "llama3", + "aliases": ["llama-3"], + "category": "chat", + "capabilities": ["chat"], + "default_tag": "latest", + "description": "Refreshed llama3 description from the remote catalog.", + "backend_hints": ["ollama"], +} + +#: ``category`` is not a member of the Category enum, so coercion raises. +MALFORMED_MODEL: Dict[str, Any] = {"name": "broken", "category": "not-a-real-category"} + + +def catalog(*models: Dict[str, Any]) -> Dict[str, Any]: + """Wrap entries in the catalog.json envelope the registry expects.""" + return {"version": 1, "models": list(models)} + + +# --------------------------------------------------------------------------- +# Stub catalog server +# --------------------------------------------------------------------------- + + +class StubCatalogServer: + """A real HTTP server serving one catalog document, counting requests.""" + + def __init__(self, payload: Any, status: int = 200) -> None: + self.body = payload if isinstance(payload, bytes) else json.dumps(payload).encode() + self.status = status + self.hits = 0 + state = self + + class _Handler(BaseHTTPRequestHandler): + def do_GET(self) -> None: # noqa: N802 - BaseHTTPRequestHandler API + state.hits += 1 + self.send_response(state.status) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(state.body))) + self.end_headers() + self.wfile.write(state.body) + + def log_message(self, *args: object) -> None: + """Silence the default stderr access log.""" + + self._server = HTTPServer(("127.0.0.1", 0), _Handler) + self._thread = threading.Thread(target=self._server.serve_forever, daemon=True) + self._thread.start() + + @property + def url(self) -> str: + host, port = self._server.server_address[0], self._server.server_address[1] + return f"http://{host}:{port}/catalog.json" + + def stop(self) -> None: + """Shut the server down; its URL then refuses connections.""" + self._server.shutdown() + self._server.server_close() + self._thread.join(timeout=5) + + +@pytest.fixture +def serve() -> Iterator[Callable[..., StubCatalogServer]]: + """Factory for stub catalog servers, all torn down at test end.""" + started: List[StubCatalogServer] = [] + + def _make(payload: Any, status: int = 200) -> StubCatalogServer: + server = StubCatalogServer(payload, status) + started.append(server) + return server + + yield _make + for server in started: + server.stop() + + +@pytest.fixture +def bundled() -> BundledRegistry: + return BundledRegistry() + + +def _expire_cache(cache_dir: Path, age_seconds: float) -> Path: + """Back-date the cache's timestamp so it reads as ``age_seconds`` old.""" + path = cache_dir / "remote_catalog_cache.json" + data = json.loads(path.read_text(encoding="utf-8")) + data["scraped_at"] = time.time() - age_seconds + path.write_text(json.dumps(data), encoding="utf-8") + return path + + +# --------------------------------------------------------------------------- +# Fetching and indexing +# --------------------------------------------------------------------------- + + +def test_fetches_and_indexes_remote_models(serve: Any, tmp_path: Path) -> None: + server = serve(catalog(FRESH_MODEL)) + registry = RemoteRegistry(server.url, cache_dir=tmp_path) + + assert server.hits == 1 + spec = registry.get(ModelRef.parse("brand-new-model")) + assert spec.description == "Published after the last ModelDock release." + assert spec.source == REMOTE + assert registry.versions(ModelRef.parse("brand-new-model")) == ["latest", "70b"] + + +def test_remote_model_is_discoverable_by_search_and_recommend(serve: Any, tmp_path: Path) -> None: + """Search is the whole point: a fresh model nobody can find is not published.""" + server = serve(catalog(FRESH_MODEL)) + registry = RemoteRegistry(server.url, cache_dir=tmp_path) + + assert [s.name for s in registry.search("brand-new-model")] == ["brand-new-model"] + assert "brand-new-model" in [s.name for s in registry.recommend("brand-new-model")] + assert "brand-new-model" in [s.name for s in registry.by_category(Category.CHAT)] + + +def test_resolves_by_alias_and_ignores_case(serve: Any, tmp_path: Path) -> None: + server = serve(catalog(FRESH_MODEL)) + registry = RemoteRegistry(server.url, cache_dir=tmp_path) + + for probe in ("brand-new-model", "shiny", "BRAND-NEW-MODEL", "Brand-New"): + assert registry.resolve(ModelRef.parse(probe)).name == "brand-new-model" + + +def test_unknown_model_still_raises(serve: Any, tmp_path: Path) -> None: + server = serve(catalog(FRESH_MODEL)) + registry = RemoteRegistry(server.url, cache_dir=tmp_path) + + with pytest.raises(ModelNotFoundError): + registry.get(ModelRef.parse("no-such-model")) + assert registry.versions(ModelRef.parse("no-such-model")) == [] + + +# --------------------------------------------------------------------------- +# Merging with the bundled catalog +# --------------------------------------------------------------------------- + + +def test_merges_with_bundled_instead_of_replacing_it( + serve: Any, tmp_path: Path, bundled: BundledRegistry +) -> None: + """A successful fetch must add to the catalog, never shrink it.""" + server = serve(catalog(FRESH_MODEL)) + registry = RemoteRegistry(server.url, cache_dir=tmp_path) + + names = {s.name for s in registry.list_all()} + assert "brand-new-model" in names + assert {s.name for s in bundled.list_all()} <= names + assert len(registry.list_all()) == len(bundled.list_all()) + 1 + # Bundled entries stay reachable through every query path, not just get(). + assert registry.search("llama3") + assert registry.get(ModelRef.parse("llama-3")).name == "llama3" + + +def test_remote_entry_wins_a_name_collision( + serve: Any, tmp_path: Path, bundled: BundledRegistry +) -> None: + server = serve(catalog(UPDATED_LLAMA3)) + registry = RemoteRegistry(server.url, cache_dir=tmp_path) + + spec = registry.get(ModelRef.parse("llama3")) + assert spec.description == "Refreshed llama3 description from the remote catalog." + # Overwritten, not duplicated. + assert [s.name for s in registry.list_all()].count("llama3") == 1 + assert len(registry.list_all()) == len(bundled.list_all()) + + +def test_malformed_entry_is_skipped_without_losing_the_payload( + serve: Any, tmp_path: Path, bundled: BundledRegistry +) -> None: + """One bad record in a community feed must not cost users the whole fetch.""" + server = serve(catalog(FRESH_MODEL, MALFORMED_MODEL)) + registry = RemoteRegistry(server.url, cache_dir=tmp_path) + + names = {s.name for s in registry.list_all()} + assert "brand-new-model" in names + assert "broken" not in names + assert {s.name for s in bundled.list_all()} <= names + + +# --------------------------------------------------------------------------- +# Caching +# --------------------------------------------------------------------------- + + +def test_second_construction_is_served_from_cache(serve: Any, tmp_path: Path) -> None: + """Inside the TTL the cache answers alone — no round-trip per CLI command.""" + server = serve(catalog(FRESH_MODEL)) + RemoteRegistry(server.url, cache_dir=tmp_path) + assert (tmp_path / "remote_catalog_cache.json").exists() + assert server.hits == 1 + + again = RemoteRegistry(server.url, cache_dir=tmp_path) + + assert server.hits == 1 + assert again.get(ModelRef.parse("brand-new-model")).name == "brand-new-model" + + +def test_expired_cache_triggers_a_refetch(serve: Any, tmp_path: Path) -> None: + server = serve(catalog(FRESH_MODEL)) + RemoteRegistry(server.url, cache_dir=tmp_path) + _expire_cache(tmp_path, remote_module._CACHE_TTL_SECONDS + 60) + + RemoteRegistry(server.url, cache_dir=tmp_path) + + assert server.hits == 2 + + +def test_unreachable_server_falls_back_to_cache(serve: Any, tmp_path: Path) -> None: + server = serve(catalog(FRESH_MODEL)) + url = server.url + RemoteRegistry(url, cache_dir=tmp_path) + _expire_cache(tmp_path, remote_module._CACHE_TTL_SECONDS + 60) + server.stop() + + registry = RemoteRegistry(url, cache_dir=tmp_path) + + # An expired cache still beats no catalog once the network is gone. + assert registry.get(ModelRef.parse("brand-new-model")).name == "brand-new-model" + + +def test_no_server_and_no_cache_degrades_to_bundled( + serve: Any, tmp_path: Path, bundled: BundledRegistry +) -> None: + server = serve(catalog(FRESH_MODEL)) + url = server.url + server.stop() + + registry = RemoteRegistry(url, cache_dir=tmp_path) + + assert {s.name for s in registry.list_all()} == {s.name for s in bundled.list_all()} + assert registry.get(ModelRef.parse("llama3")).name == "llama3" + + +def test_error_status_degrades_to_bundled( + serve: Any, tmp_path: Path, bundled: BundledRegistry +) -> None: + server = serve(catalog(FRESH_MODEL), status=500) + + registry = RemoteRegistry(server.url, cache_dir=tmp_path) + + assert len(registry.list_all()) == len(bundled.list_all()) + assert not (tmp_path / "remote_catalog_cache.json").exists() + + +@pytest.mark.parametrize("body", [b"not json at all", b'{"nope": []}', b"[]"]) +def test_unusable_payload_degrades_to_bundled( + serve: Any, tmp_path: Path, bundled: BundledRegistry, body: bytes +) -> None: + server = serve(body) + + registry = RemoteRegistry(server.url, cache_dir=tmp_path) + + assert len(registry.list_all()) == len(bundled.list_all()) + + +def test_oversized_catalog_is_refused( + serve: Any, tmp_path: Path, bundled: BundledRegistry, monkeypatch: pytest.MonkeyPatch +) -> None: + """A user-supplied URL does not get to decide how much memory we buffer.""" + monkeypatch.setattr(remote_module, "_MAX_CATALOG_BYTES", 16) + server = serve(catalog(FRESH_MODEL)) + + registry = RemoteRegistry(server.url, cache_dir=tmp_path) + + assert len(registry.list_all()) == len(bundled.list_all()) + + +# --------------------------------------------------------------------------- +# refresh() +# --------------------------------------------------------------------------- + + +def test_refresh_bypasses_the_cache_ttl(serve: Any, tmp_path: Path) -> None: + """`modeldock sources refresh` must reach the network even on a warm cache.""" + server = serve(catalog(FRESH_MODEL)) + registry = RemoteRegistry(server.url, cache_dir=tmp_path) + assert server.hits == 1 + + total = registry.refresh() + + assert server.hits == 2 + assert total == len(registry.list_all()) + assert registry.get(ModelRef.parse("brand-new-model")).name == "brand-new-model" + + +def test_refresh_is_discoverable_by_the_composite_and_manager(serve: Any, tmp_path: Path) -> None: + """The refresh contract is duck-typed, so a private _refresh would be skipped.""" + server = serve(catalog(FRESH_MODEL)) + registry = RemoteRegistry(server.url, cache_dir=tmp_path) + + assert callable(getattr(registry, "refresh", None)) + CompositeRegistry([registry]).refresh() + assert server.hits == 2 + + +def test_failed_refresh_keeps_the_existing_index(serve: Any, tmp_path: Path) -> None: + server = serve(catalog(FRESH_MODEL)) + registry = RemoteRegistry(server.url, cache_dir=tmp_path) + before = {s.name for s in registry.list_all()} + server.stop() + + registry.refresh() + + assert {s.name for s in registry.list_all()} == before + + +# --------------------------------------------------------------------------- +# URL validation +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "url", + ["file:///etc/passwd", "ftp://example.com/catalog.json", "example.com/catalog.json", "", " "], +) +def test_rejects_non_http_urls(url: str, tmp_path: Path) -> None: + with pytest.raises(ConfigError): + RemoteRegistry(url, cache_dir=tmp_path) + + +# --------------------------------------------------------------------------- +# describe() +# --------------------------------------------------------------------------- + + +def test_describe_reports_remote_contribution_and_bundled(serve: Any, tmp_path: Path) -> None: + server = serve(catalog(FRESH_MODEL)) + registry = RemoteRegistry(server.url, cache_dir=tmp_path) + + infos = {info.name: info for info in registry.describe()} + assert set(infos) == {REMOTE, BUNDLED} + assert infos[REMOTE].trust == SourceTrust.CUSTOM + assert infos[REMOTE].available is True + # The remote count is its own contribution, not the merged total. + assert infos[REMOTE].model_count == 1 + assert infos[REMOTE].cache_path == str(tmp_path / "remote_catalog_cache.json") + + +def test_describe_marks_an_unreachable_remote_unavailable(serve: Any, tmp_path: Path) -> None: + """Reporting the bundled fallback's count as the remote's would be a lie.""" + server = serve(catalog(FRESH_MODEL)) + url = server.url + server.stop() + + infos = {info.name: info for info in RemoteRegistry(url, cache_dir=tmp_path).describe()} + + assert infos[REMOTE].available is False + assert infos[REMOTE].model_count == 0 + + +# --------------------------------------------------------------------------- +# ModelManager wiring +# --------------------------------------------------------------------------- + + +class TestManagerWiring: + def test_remote_source_uses_the_configured_url(self, serve: Any, tmp_path: Path) -> None: + server = serve(catalog(FRESH_MODEL)) + manager = ModelManager( + backend=RuntimeBackend.OLLAMA, + settings=Settings(cache_dir=tmp_path, catalog_source="remote", registry_url=server.url), + ) + + assert isinstance(manager._registry_port, RemoteRegistry) + assert "brand-new-model" in [s.name for s in manager.search("brand-new")] + + def test_remote_source_without_a_url_is_a_config_error(self, tmp_path: Path) -> None: + with pytest.raises(ConfigError): + ModelManager( + backend=RuntimeBackend.OLLAMA, + settings=Settings(cache_dir=tmp_path, catalog_source="remote"), + ) + + def test_auto_merges_the_remote_catalog_when_a_url_is_set( + self, serve: Any, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A configured registry_url must actually reach discovery under "auto".""" + # Force the live Ollama catalog to fail so the base is the bundled one. + monkeypatch.setattr( + "modeldock.adapters.registry.ollama_library.OllamaLibraryRegistry._fetch_from_network", + lambda self: None, + ) + server = serve(catalog(FRESH_MODEL)) + manager = ModelManager( + backend=RuntimeBackend.OLLAMA, + settings=Settings(cache_dir=tmp_path, catalog_source="auto", registry_url=server.url), + ) + + assert isinstance(manager._registry_port, CompositeRegistry) + names = {s.name for s in manager.list()} + assert "brand-new-model" in names + assert "llama3" in names + + def test_auto_without_a_url_keeps_the_previous_behaviour( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setattr( + "modeldock.adapters.registry.ollama_library.OllamaLibraryRegistry._fetch_from_network", + lambda self: None, + ) + manager = ModelManager(backend=RuntimeBackend.OLLAMA, settings=Settings(cache_dir=tmp_path)) + + assert isinstance(manager._registry_port, BundledRegistry) + + def test_sources_lists_the_remote_catalog_once(self, serve: Any, tmp_path: Path) -> None: + server = serve(catalog(FRESH_MODEL)) + manager = ModelManager( + backend=RuntimeBackend.OLLAMA, + settings=Settings(cache_dir=tmp_path, catalog_source="remote", registry_url=server.url), + ) + + names = [info.name for info in manager.sources()] + assert names.count(REMOTE) == 1 + assert names.count(BUNDLED) == 1 + + def test_manager_refresh_reaches_the_remote_source(self, serve: Any, tmp_path: Path) -> None: + server = serve(catalog(FRESH_MODEL)) + manager = ModelManager( + backend=RuntimeBackend.OLLAMA, + settings=Settings(cache_dir=tmp_path, catalog_source="remote", registry_url=server.url), + ) + assert server.hits == 1 + + manager.refresh_sources() + + assert server.hits == 2 + + +def test_invalid_catalog_source_still_rejected() -> None: + """Adding "remote" must not turn the allow-list into anything-goes.""" + with pytest.raises(ConfigError): + Settings(catalog_source="nonsense") + + +def test_optional_remote_never_breaks_discovery( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A malformed registry_url degrades under "auto" rather than raising.""" + monkeypatch.setattr( + "modeldock.adapters.registry.ollama_library.OllamaLibraryRegistry._fetch_from_network", + lambda self: None, + ) + manager = ModelManager( + backend=RuntimeBackend.OLLAMA, + settings=Settings(cache_dir=tmp_path, registry_url="file:///etc/passwd"), + ) + + assert "llama3" in [s.name for s in manager.list()] + + +def test_registry_port_contract_is_satisfied(serve: Any, tmp_path: Path) -> None: + from modeldock.ports.registry import RegistryPort + + server = serve(catalog(FRESH_MODEL)) + assert isinstance(RemoteRegistry(server.url, cache_dir=tmp_path), RegistryPort)