diff --git a/docs/plans/issue-1108-scoped-scan.md b/docs/plans/issue-1108-scoped-scan.md index 585af411..ebce80c8 100644 --- a/docs/plans/issue-1108-scoped-scan.md +++ b/docs/plans/issue-1108-scoped-scan.md @@ -364,8 +364,9 @@ differs per mode and that matters: unioned rather than replaced whenever the scan did not verify every root); the next full refresh drops them. - `root_details[].reachable` is a live `os.path.exists()` per root, so a root that disappears - mid-session shows as offline; a root that was already gone at startup is filtered out by - `Config` and therefore absent from the list. + mid-session shows as offline; a root that was already gone at startup is still listed thanks + to Wave 6 (`available: false`, cached count), and `/roots` re-admits it once the directory + is back. ## Verification checklist diff --git a/py/services/errors.py b/py/services/errors.py index 56c26ae4..d6cf437d 100644 --- a/py/services/errors.py +++ b/py/services/errors.py @@ -45,6 +45,12 @@ class ResourceNotFoundError(RuntimeError): pass +class MetadataPersistError(RuntimeError): + """Raised when the metadata sidecar cannot be written to disk.""" + + pass + + class LLMNotConfiguredError(RuntimeError): """Raised when an LLM-dependent operation is attempted but no provider is configured.""" diff --git a/py/services/metadata_sync_service.py b/py/services/metadata_sync_service.py index c69c47c7..cc5763e9 100644 --- a/py/services/metadata_sync_service.py +++ b/py/services/metadata_sync_service.py @@ -14,7 +14,7 @@ from ..utils.model_utils import determine_base_model from ..utils.models import autov3_from_civitai_files from ..utils.sidecar_paths import get_metadata_path from .connectivity_guard import OFFLINE_FRIENDLY_MESSAGE, is_expected_offline_error -from .errors import RateLimitError +from .errors import MetadataPersistError, RateLimitError from .model_metadata_provider import _LOCAL_PROVIDER_LABELS from .model_sources import get_source_platform, has_external_source @@ -231,7 +231,16 @@ class MetadataSyncService: metadata_path, local_metadata, civitai_metadata.get("images", []) ) - await self._metadata_manager.save_metadata(metadata_path, local_metadata) + saved = await self._metadata_manager.save_metadata(metadata_path, local_metadata) + if not saved: + # A swallowed write failure would update the cache while the + # durable sidecar never lands (e.g. the drive went offline), and + # the model would be skipped by every later fetch as "already + # fetched". Fail loudly instead so the caller reports the item + # and it stays eligible for the next run. + raise MetadataPersistError( + f"Failed to write metadata sidecar: {metadata_path}" + ) return local_metadata async def fetch_and_update_model( diff --git a/tests/services/test_metadata_sync_service.py b/tests/services/test_metadata_sync_service.py index c76dcdce..df014615 100644 --- a/tests/services/test_metadata_sync_service.py +++ b/tests/services/test_metadata_sync_service.py @@ -5,7 +5,7 @@ from unittest.mock import AsyncMock import pytest from py.services.connectivity_guard import OFFLINE_COOLDOWN_ERROR, OFFLINE_FRIENDLY_MESSAGE -from py.services.errors import RateLimitError +from py.services.errors import MetadataPersistError, RateLimitError from py.services.metadata_sync_service import MetadataSyncService, _merge_ordered_unique @@ -285,6 +285,56 @@ async def test_fetch_and_update_model_success_updates_cache(tmp_path): update_cache.assert_awaited_once() +@pytest.mark.asyncio +async def test_update_model_metadata_raises_when_sidecar_write_fails(): + """A failed sidecar write must surface instead of being swallowed.""" + helpers = build_service() + helpers.metadata_manager.save_metadata.return_value = False + + with pytest.raises(MetadataPersistError): + await helpers.service.update_model_metadata( + "path/to/model.metadata.json", + {"civitai": {}, "model_name": "Local"}, + {"source": "api", "model": {"name": "Remote"}, "images": []}, + helpers.default_provider, + ) + + +@pytest.mark.asyncio +async def test_fetch_and_update_model_reports_sidecar_write_failure(tmp_path): + """Fetch fails the item (and leaves the cache untouched) when the sidecar + cannot be written, so the model stays eligible for the next run.""" + helpers = build_service() + helpers.metadata_manager.save_metadata.return_value = False + + civitai_payload = { + "source": "api", + "model": {"name": "Remote", "description": "", "tags": ["tag"]}, + "images": [], + "baseModel": "sdxl", + } + helpers.default_provider.get_model_by_hash.return_value = (civitai_payload, None) + + model_path = tmp_path / "model.safetensors" + model_data: Dict[str, Any] = { + "model_name": "Local", + "folder": "root", + "file_path": str(model_path), + } + update_cache = AsyncMock(return_value=True) + + ok, error = await helpers.service.fetch_and_update_model( + sha256="abc", + file_path=str(model_path), + model_data=model_data, + update_cache_func=update_cache, + ) + + assert ok is False + assert "sidecar" in (error or "") + update_cache.assert_not_awaited() + + @pytest.mark.asyncio async def test_fetch_and_update_model_keeps_deleted_flag_false_for_archive_source(tmp_path): helpers = build_service()