fix(ollama): cloud-model guard, safe update, safe cache clean (issue #156 M1-M3)

src/modeldock/__init__.py

@@ -102,11 +102,11 @@ def install_category(category: str, backend: Optional[str] = None) -> List[Model
     return _manager().install_category(category)
 
 
-def update(name: str, backend: Optional[str] = None) -> ModelRef:
-    """Pull a newer tag."""
+def update(name: str, backend: Optional[str] = None, confirm: bool = False) -> ModelRef:
+    """Pull a newer tag (destructive: removes then re-downloads)."""
     if backend is not None:
-        return Manager(backend=backend).update(name)
-    return _manager().update(name)
+        return Manager(backend=backend).update(name, confirm=confirm)
+    return _manager().update(name, confirm=confirm)
 
 
 def remove(name: str, backend: Optional[str] = None) -> None:

src/modeldock/adapters/cache/filesystem.py

@@ -72,16 +72,17 @@ def get_record(self, ref: ModelRef) -> Optional[Dict[str, Any]]:
         entry = data.get("entries", {}).get(self._key(ref))
         return cast(Optional[Dict[str, Any]], entry)
 
-    def clean(self) -> List[str]:
+    def clean(self, force: bool = False) -> List[str]:
         removed: List[str] = []
         data = self._read_manifest()
         entries = data.get("entries", {})
         for key, entry in list(entries.items()):
-            # Remove entries whose artifact file is missing.
-            name = entry.get("name", "")
-            artifact = self._cache_dir / f"{name}.bin"
-            if not artifact.exists():
-                removed.append(str(artifact))
+            # Safe default: only drop entries that are corrupt/partial (missing
+            # the fields we recorded). ModelDock does not manage the model blobs
+            # for Ollama, so a missing artifact file is NOT grounds for removal.
+            # force=True wipes every entry.
+            if force or not isinstance(entry, dict) or not entry.get("sha256"):
+                removed.append(key)
                 del entries[key]
         if removed:
             self._write_manifest(data)

src/modeldock/adapters/runtimes/ollama.py

@@ -185,6 +185,16 @@ def _get_client(self, ref: ModelRef) -> Any:
         return self._ensure_client()
 
     def remove(self, ref: ModelRef) -> None:
+        # Cloud/subscription models are not managed by the local runtime; a
+        # delete call would block on the remote service. Fail fast instead.
+        if ref.is_cloud:
+            raise DownloadError(
+                ref.name,
+                reason=(
+                    f"{ref.qualified_name()} is a cloud/subscription model and "
+                    "cannot be removed locally."
+                ),
+            )
         client = self._ensure_client()
         try:
             client.delete(ref.qualified_name())

src/modeldock/core/cache.py

@@ -29,9 +29,13 @@ def status(self) -> List[Dict[str, Any]]:
         """Return a snapshot of cached entries."""
         return self._cache.status()
 
-    def clean(self) -> List[str]:
-        """Remove orphaned/partial artifacts; return removed paths."""
-        return self._cache.clean()
+    def clean(self, force: bool = False) -> List[str]:
+        """Remove orphaned/partial artifacts; return removed paths.
+
+        Safe by default (only corrupt/partial entries); pass ``force=True`` to
+        wipe every cached entry.
+        """
+        return self._cache.clean(force=force)
 
     def path(self) -> str:
         """Return the cache directory path."""

src/modeldock/core/manager.py

@@ -15,6 +15,7 @@
 from modeldock.adapters.runtimes.registry import RuntimeRegistry
 from modeldock.common.config import Settings
 from modeldock.common.errors import (
+    DownloadError,
     ModelNotFoundError,
     RuntimeUnavailableError,
 )
@@ -164,11 +165,32 @@ def install_category(self, category: str) -> List[ModelRef]:
             refs.append(ref)
         return refs
 
-    def update(self, name: str) -> ModelRef:
-        """Pull a newer tag for an installed model."""
+    def update(self, name: str, confirm: bool = False) -> ModelRef:
+        """Pull a newer tag for an installed model.
+
+        Destructive: removes the current copy and re-downloads. Requires
+        ``confirm=True`` to proceed, so a large model is never re-pulled by
+        accident. Cloud/subscription models cannot be updated locally.
+        """
         ref = ModelRef.parse(name, backend=self._backend)
+        if ref.is_cloud:
+            raise DownloadError(
+                ref.name,
+                reason=(
+                    f"{ref.qualified_name()} is a cloud/subscription model and "
+                    "cannot be updated locally."
+                ),
+            )
         if not self._runtime.is_installed(ref):
             raise ModelNotFoundError(name)
+        if not confirm:
+            raise DownloadError(
+                ref.name,
+                reason=(
+                    f"update() removes and re-downloads {ref.qualified_name()}. "
+                    "Pass confirm=True to proceed."
+                ),
+            )
         self._runtime.remove(ref)
         self._download.pull(ref)
         return ref

src/modeldock/domain/model.py

@@ -236,6 +236,15 @@ def qualified_name(self) -> str:
         """Return ``name:tag`` form used by runtimes."""
         return f"{self.name}:{self.tag}"
 
+    @property
+    def is_cloud(self) -> bool:
+        """True for cloud/subscription models (tag contains ``cloud``).
+
+        These cannot be installed, run, or removed through a local runtime and
+        must be short-circuited with a clear error instead of a daemon call.
+        """
+        return "cloud" in self.tag
+
     def __hash__(self) -> int:
         return hash((self.name, self.tag, self.backend))
 

src/modeldock/ports/cache.py

@@ -27,8 +27,12 @@ def get_record(self, ref: ModelRef) -> Optional[Dict[str, Any]]:
         """Return the cached manifest entry for ``ref``, if any."""
         ...
 
-    def clean(self) -> List[str]:
-        """Remove orphaned/partial artifacts; return removed paths."""
+    def clean(self, force: bool = False) -> List[str]:
+        """Remove orphaned/partial artifacts; return removed paths.
+
+        Safe by default: only corrupt/partial manifest entries are removed.
+        Pass ``force=True`` to wipe every cached entry.
+        """
         ...
 
     def status(self) -> List[Dict[str, Any]]:

tests/conftest.py

@@ -82,10 +82,11 @@ def record(self, ref: ModelRef, tag: str, sha256: str, size_bytes: int) -> None:
     def get_record(self, ref: ModelRef) -> Optional[dict]:
         return self.entries.get(self._key(ref))
 
-    def clean(self) -> List[str]:
-        removed = [f"{k}" for k in self.entries]
+    def clean(self, force: bool = False) -> List[str]:
+        removed = [k for k, v in self.entries.items() if force or not v.get("sha256")]
+        for k in removed:
+            del self.entries[k]
         self.cleaned.extend(removed)
-        self.entries.clear()
         return removed
 
     def status(self) -> List[dict]:

tests/unit/test_cache.py

@@ -25,8 +25,11 @@ def test_cache_service_status_and_clean(fake_cache: object) -> None:
     ref = ModelRef.parse("llama3")
     svc.record(ref, "latest", "abc", 10)
     assert len(svc.status()) == 1
-    removed = svc.clean()
-    assert removed
+    # Safe default keeps a valid entry.
+    assert svc.clean() == []
+    assert len(svc.status()) == 1
+    # force=True wipes it.
+    assert svc.clean(force=True)
     assert svc.status() == []
 
 
@@ -49,23 +52,44 @@ def test_filesystem_cache_persists_across_instances(tmp_path: Path) -> None:
     assert again.is_fresh(ref)
 
 
-def test_filesystem_cache_clean_removes_missing_artifacts(tmp_path: Path) -> None:
+def test_filesystem_cache_clean_keeps_valid_entries_by_default(tmp_path: Path) -> None:
+    cache = FilesystemCache(tmp_path)
+    ref = ModelRef.parse("llama3")
+    cache.record(ref, "latest", "x", 1)
+    # Safe default: a valid entry is kept even though no .bin artifact exists.
+    assert cache.clean() == []
+    assert cache.is_fresh(ref)
+
+
+def test_filesystem_cache_clean_removes_corrupt_entries(tmp_path: Path) -> None:
     cache = FilesystemCache(tmp_path)
     ref = ModelRef.parse("llama3")
     cache.record(ref, "latest", "x", 1)
-    # No artifact file was written, so clean() should report it removed.
+    # Corrupt an entry (drop sha256) to simulate a partial/corrupt record.
+    data = cache._read_manifest()
+    data["entries"]["llama3:latest"].pop("sha256")
+    cache._write_manifest(data)
     removed = cache.clean()
-    assert removed
+    assert removed == ["llama3:latest"]
     assert cache.status() == []
 
 
-def test_filesystem_cache_keeps_present_artifacts(tmp_path: Path) -> None:
+def test_filesystem_cache_clean_force_wipes_all(tmp_path: Path) -> None:
     cache = FilesystemCache(tmp_path)
     ref = ModelRef.parse("llama3")
     cache.record(ref, "latest", "x", 1)
-    (tmp_path / "llama3.bin").write_text("blob")
-    assert cache.clean() == []
-    assert cache.is_fresh(ref)
+    removed = cache.clean(force=True)
+    assert removed == ["llama3:latest"]
+    assert cache.status() == []
+
+
+def test_cache_service_clean_passes_force(tmp_path: Path) -> None:
+    cache = FilesystemCache(tmp_path)
+    ref = ModelRef.parse("llama3")
+    cache.record(ref, "latest", "x", 1)
+    svc = CacheService(cache)
+    assert svc.clean() == []
+    assert svc.clean(force=True) == ["llama3:latest"]
 
 
 def test_filesystem_cache_corrupt_manifest_raises(tmp_path: Path) -> None:

tests/unit/test_core.py

@@ -4,7 +4,7 @@
 
 import pytest
 
-from modeldock.common.errors import DownloadError, ModelNotInstalledError
+from modeldock.common.errors import DownloadError, ModelNotFoundError, ModelNotInstalledError
 from modeldock.core.download import DownloadService
 from modeldock.core.lifecycle import LifecycleOrchestrator
 from modeldock.domain.model import ModelRef
@@ -103,13 +103,28 @@ def test_manager_install_category(manager_with_fakes: object) -> None:
     assert all(mgr.verify(r.name) for r in refs)
 
 
-def test_manager_update(manager_with_fakes: object) -> None:
+def test_manager_update_requires_confirm(manager_with_fakes: object) -> None:
     mgr = manager_with_fakes
     mgr.install("llama3")
-    ref = mgr.update("llama3")
+    with pytest.raises(DownloadError):
+        mgr.update("llama3")
+    # With confirm=True it proceeds (remove + re-pull).
+    ref = mgr.update("llama3", confirm=True)
     assert ref.name == "llama3"
 
 
+def test_manager_update_unknown_raises(manager_with_fakes: object) -> None:
+    mgr = manager_with_fakes
+    with pytest.raises(ModelNotFoundError):
+        mgr.update("ghost-model", confirm=True)
+
+
+def test_manager_update_cloud_model_raises(manager_with_fakes: object) -> None:
+    mgr = manager_with_fakes
+    with pytest.raises(DownloadError):
+        mgr.update("glm-5.2:cloud", confirm=True)
+
+
 def test_manager_remove(manager_with_fakes: object) -> None:
     mgr = manager_with_fakes
     mgr.install("llama3")

tests/unit/test_ollama_runtime.py

@@ -179,6 +179,15 @@ def delete(self, name: str) -> dict[str, Any]:
         runtime.remove(ModelRef.parse("llama3:latest"))
 
 
+def test_remove_cloud_model_fails_fast_without_daemon_call() -> None:
+    # Cloud/subscription models must not trigger a (hanging) daemon delete.
+    client = _PullClient([{"models": []}])
+    runtime = _runtime_with(client)
+    with pytest.raises(DownloadError):
+        runtime.remove(ModelRef.parse("glm-5.2:cloud"))
+    assert client.deleted == []
+
+
 def test_host_override_applied_at_client_build(monkeypatch: Any) -> None:
     built: dict[str, Any] = {}
 

tests/unit/test_port_contract.py

@@ -104,5 +104,9 @@ def test_cache_status_and_clean(cache_factory: Callable[[], List[CachePort]]) ->
         ref = ModelRef.parse("llama3")
         impl.record(ref, "latest", "sha", 10)
         assert len(impl.status()) == 1
+        # Safe default keeps valid entries.
         impl.clean()
+        assert len(impl.status()) == 1
+        # force=True wipes everything.
+        impl.clean(force=True)
         assert impl.status() == []