From c13eafc31bfd63f0519567916e19a9ba68ad0875 Mon Sep 17 00:00:00 2001 From: Harvey Date: Sun, 16 Aug 2026 18:31:29 +0100 Subject: [PATCH 1/9] fix(library): bound scans against hangs and outages, surface failed paths, and add a local-library disable switch --- .../api/v1/routes/library_policies_target.py | 17 +- backend/api/v1/routes/library_scan_target.py | 39 +- backend/api/v1/routes/playlists.py | 1 + backend/api/v1/routes/stream.py | 37 +- backend/api/v1/schemas/library_policies.py | 4 + backend/api/v1/schemas/library_scan_target.py | 16 + backend/api/v1/schemas/playlists.py | 1 + backend/core/dependencies/__init__.py | 2 + .../core/dependencies/service_providers.py | 15 +- backend/core/dependencies/type_aliases.py | 5 + backend/core/tasks.py | 312 +++++++------- .../persistence/native_library_schema.py | 12 + .../persistence/native_library_store.py | 252 +++++++++++- backend/models/library_work.py | 9 + backend/services/audiodb_browse_queue.py | 1 + backend/services/jellyfin_library_service.py | 31 +- backend/services/jellyfin_playback_service.py | 40 +- .../native/acquisition_cleanup_service.py | 45 +- .../native/album_identification_service.py | 6 +- backend/services/native/file_processor.py | 18 +- .../native/identification_queue_service.py | 41 +- .../native/identity_repair_service.py | 104 +++-- .../native/library_inventory_scanner.py | 219 +++++++++- .../native/library_policy_resolver.py | 1 + .../services/native/library_policy_service.py | 1 + .../native/library_scan_coordinator.py | 9 + .../native/library_scan_supervisor.py | 17 +- .../native/target_application_lifecycle.py | 44 +- .../native/target_application_runtime.py | 100 ++++- .../native/target_library_policy_service.py | 1 + .../services/navidrome_playback_service.py | 10 +- backend/services/now_playing_poller.py | 2 +- backend/services/playlist_service.py | 12 +- backend/services/plex_playback_service.py | 10 +- backend/services/preferences_service.py | 4 + backend/services/settings_service.py | 14 + backend/target_application.py | 93 ++++- .../infrastructure/test_container_umask.py | 50 +++ .../test_native_library_store.py | 187 +++++++++ .../test_target_scan_lifecycle.py | 86 +++- backend/tests/routes/test_downloads_routes.py | 16 + .../routes/test_library_policies_target.py | 126 ++++++ backend/tests/routes/test_stream_routes.py | 36 +- .../tests/routes/test_target_application.py | 73 +++- .../routes/test_target_library_scan_routes.py | 165 +++++++- .../security/test_auth_on_every_endpoint.py | 3 + .../test_acquisition_cleanup_service.py | 138 ++++++- .../native/test_identification_pipeline.py | 190 ++++++++- .../native/test_library_policy_resolver.py | 20 + .../native/test_library_review_operations.py | 244 +++++++++-- .../test_target_library_policy_service.py | 41 ++ .../native/test_target_scan_runtime.py | 387 +++++++++++++++++- .../tests/services/test_download_service.py | 32 ++ backend/tests/services/test_file_processor.py | 39 ++ .../services/test_jellyfin_library_service.py | 143 +++++++ .../test_jellyfin_playback_service.py | 129 ++++++ .../services/test_navidrome_stream_proxy.py | 68 +++ .../test_playlist_source_resolution.py | 67 +++ .../services/test_plex_playback_service.py | 66 +++ .../test_settings_cache_invalidation.py | 122 ++++++ .../services/test_source_playlist_import.py | 1 + entrypoint.sh | 13 + frontend/src/lib/api/playlists.ts | 1 + .../downloads/HeldTrackCard.svelte.spec.ts | 42 +- .../downloads/HeldTrackReview.svelte | 12 + .../library/LibraryOverviewPanel.svelte | 16 +- .../LibraryOverviewPanel.svelte.spec.ts | 12 + .../library/LibraryRunHistory.svelte | 41 +- .../library/LibraryRunHistory.svelte.spec.ts | 100 ++++- .../library/LibraryScanningPanel.svelte | 118 ++++-- .../LibraryScanningPanel.svelte.spec.ts | 106 ++++- .../settings/SettingsLibrary.svelte | 78 +++- .../settings/SettingsLibrary.svelte.spec.ts | 103 +++++ frontend/src/lib/constants.ts | 8 + frontend/src/lib/player/queueHelpers.spec.ts | 48 ++- frontend/src/lib/player/queueHelpers.ts | 21 +- .../__tests__/integration-coverage.spec.ts | 1 + .../library/LibraryOperationQueries.svelte.ts | 17 + .../queries/library/LibraryOperationsTypes.ts | 17 + .../queries/library/LibraryQueryKeyFactory.ts | 2 + frontend/src/lib/stores/player.spec.ts | 74 ++++ frontend/src/lib/stores/player.svelte.ts | 12 +- .../routes/library/management/+page.svelte | 30 +- .../library/management/page.svelte.spec.ts | 17 + .../routes/playlists/[id]/page.svelte.spec.ts | 1 + 85 files changed, 4443 insertions(+), 421 deletions(-) create mode 100644 backend/tests/routes/test_library_policies_target.py diff --git a/backend/api/v1/routes/library_policies_target.py b/backend/api/v1/routes/library_policies_target.py index 3e3e659f2..58e31f4d1 100644 --- a/backend/api/v1/routes/library_policies_target.py +++ b/backend/api/v1/routes/library_policies_target.py @@ -13,7 +13,10 @@ LibrarySettingsResponse, LibrarySettingsUpdateRequest, ) -from core.dependencies import TargetLibraryPolicyServiceDep +from core.dependencies import ( + LegacyPendingMigrationServiceDep, + TargetLibraryPolicyServiceDep, +) from infrastructure.msgspec_fastapi import MsgSpecBody, MsgSpecRoute from middleware import CurrentAdminDep @@ -39,12 +42,16 @@ async def get_library_settings( @router.put("", response_model=LibrarySettingsResponse) async def update_library_settings( service: TargetLibraryPolicyServiceDep, + pending_migration: LegacyPendingMigrationServiceDep, request: LibrarySettingsUpdateRequest = MsgSpecBody(LibrarySettingsUpdateRequest), ) -> LibrarySettingsResponse: - return await service.save_settings( + response = await service.save_settings( request.settings, expected_policy_revision=request.expected_policy_revision, ) + if response.enabled: + await pending_migration.schedule() + return response @router.get("/policy-tree", response_model=LibraryPolicyTreeResponse) @@ -80,6 +87,10 @@ async def get_restorable_library_roots( @router.post("/restore-roots", response_model=LibrarySettingsResponse) async def restore_library_roots( service: TargetLibraryPolicyServiceDep, + pending_migration: LegacyPendingMigrationServiceDep, request: LibraryRestoreRootsRequest = MsgSpecBody(LibraryRestoreRootsRequest), ) -> LibrarySettingsResponse: - return await service.restore_roots(request) + response = await service.restore_roots(request) + if response.enabled: + await pending_migration.schedule() + return response diff --git a/backend/api/v1/routes/library_scan_target.py b/backend/api/v1/routes/library_scan_target.py index ed58aa819..7bfe026f2 100644 --- a/backend/api/v1/routes/library_scan_target.py +++ b/backend/api/v1/routes/library_scan_target.py @@ -19,6 +19,8 @@ ScanEstimateResponse, ScanRunCurrentResponse, ScanRunDetailResponse, + ScanRunFailureItem, + ScanRunFailuresResponse, ScanRunHistoryResponse, ScanRunRequestBody, ScanRunRequestedResponse, @@ -27,6 +29,8 @@ from core.dependencies import ( LibraryAdministrativeWorkServiceDep, LibraryPolicyResolverDep, + MbProviderAvailabilityDep, + NativeLibraryStoreDep, TargetIdentificationQueueDep, TargetLibraryScanCoordinatorDep, ) @@ -116,6 +120,7 @@ async def library_activity( coordinator: TargetLibraryScanCoordinatorDep, identification: TargetIdentificationQueueDep, administrative_work: LibraryAdministrativeWorkServiceDep, + mb_provider_available: MbProviderAvailabilityDep, ) -> LibraryActivityResponse: revisions = await identification.stream_revisions() runs = await coordinator.current() @@ -298,13 +303,17 @@ async def library_activity( needs_review_count=counts.get("needs_review", 0), failed_count=counts.get("failed", 0), deferred_count=identification_snapshot["deferred_count"], + deferred_reason_counts=identification_snapshot[ + "deferred_reason_counts" + ], + attention_count=identification_snapshot["attention_count"], priority_band=( _IDENTIFICATION_PRIORITY_LABELS.get(active_priority, "Queued work") if active_priority is not None else None ), oldest_backlog_at=identification_snapshot["started_at"], - provider_unavailable=bool(identification_snapshot["deferred_count"]), + provider_unavailable=not mb_provider_available(), control_revision=identification_snapshot["control_revision"], failure_event_id=identification_snapshot["failure_event_id"], failure_at=identification_snapshot["failure_at"], @@ -532,6 +541,34 @@ async def scan_run_detail( return ScanRunDetailResponse(snapshot=await coordinator.snapshot(run_id)) +@router.get("/scan-runs/{run_id}/failures", response_model=ScanRunFailuresResponse) +async def scan_run_failures( + run_id: str, + _: CurrentAdminDep, + store: NativeLibraryStoreDep, + limit: int = Query(default=50, ge=1, le=200), + cursor: int | None = Query(default=None, ge=1), +) -> ScanRunFailuresResponse: + await store.get_scan_run(run_id) + items, next_cursor = await store.list_scan_run_failures( + run_id, limit=limit, cursor_rowid=cursor + ) + return ScanRunFailuresResponse( + items=[ + ScanRunFailureItem( + root_id=item.root_id, + relative_path=item.relative_path, + failure_code=item.failure_code, + failure_detail=item.failure_detail, + phase=item.phase, + recorded_at=item.recorded_at, + ) + for item in items + ], + next_cursor=next_cursor, + ) + + async def _control( run_id: str, action: str, diff --git a/backend/api/v1/routes/playlists.py b/backend/api/v1/routes/playlists.py index 6d64fe3f7..f157b60e2 100644 --- a/backend/api/v1/routes/playlists.py +++ b/backend/api/v1/routes/playlists.py @@ -99,6 +99,7 @@ def _track_to_response(t) -> PlaylistTrackResponse: duration=t.duration, created_at=t.created_at, plex_rating_key=getattr(t, "plex_rating_key", None), + library_file_id=getattr(t, "library_file_id", None), ) diff --git a/backend/api/v1/routes/stream.py b/backend/api/v1/routes/stream.py index c27712f01..b4f367a86 100644 --- a/backend/api/v1/routes/stream.py +++ b/backend/api/v1/routes/stream.py @@ -15,7 +15,12 @@ get_navidrome_playback_service, get_plex_playback_service, ) -from core.exceptions import ExternalServiceError, PlaybackNotAllowedError, ResourceNotFoundError +from core.exceptions import ( + ExternalServiceError, + JellyfinAuthError, + PlaybackNotAllowedError, + ResourceNotFoundError, +) from infrastructure.msgspec_fastapi import MsgSpecBody, MsgSpecRoute from middleware import CurrentUserDep from services.jellyfin_playback_service import JellyfinPlaybackService @@ -32,16 +37,22 @@ async def stream_jellyfin_audio( item_id: str, request: Request, + current_user: CurrentUserDep, playback_service: JellyfinPlaybackService = Depends(get_jellyfin_playback_service), ) -> StreamingResponse: try: range_header = request.headers.get("Range") - return await playback_service.proxy_stream(item_id, range_header=range_header) + return await playback_service.proxy_stream( + item_id, range_header=range_header, user_id=current_user.id + ) except ResourceNotFoundError: raise HTTPException(status_code=404, detail="Audio item not found") except PlaybackNotAllowedError as e: logger.warning("Playback not allowed for %s: %s", item_id, e) raise HTTPException(status_code=403, detail="Playback not allowed") + except JellyfinAuthError as e: + logger.warning("Jellyfin auth failure streaming %s: %s", item_id, e) + raise HTTPException(status_code=502, detail="Failed to stream from Jellyfin") except ExternalServiceError as e: if "416" in str(e): raise HTTPException(status_code=416, detail="Range not satisfiable") @@ -51,15 +62,19 @@ async def stream_jellyfin_audio( @router.head("/jellyfin/{item_id}") async def head_jellyfin_audio( item_id: str, + current_user: CurrentUserDep, playback_service: JellyfinPlaybackService = Depends(get_jellyfin_playback_service), ) -> Response: try: - return await playback_service.proxy_head(item_id) + return await playback_service.proxy_head(item_id, user_id=current_user.id) except ResourceNotFoundError: raise HTTPException(status_code=404, detail="Audio item not found") except PlaybackNotAllowedError as e: logger.warning("Playback not allowed for %s: %s", item_id, e) raise HTTPException(status_code=403, detail="Playback not allowed") + except JellyfinAuthError as e: + logger.warning("Jellyfin auth failure heading %s: %s", item_id, e) + raise HTTPException(status_code=502, detail="Failed to resolve Jellyfin stream") except ExternalServiceError as e: logger.error("Jellyfin head stream error for %s: %s", item_id, e) raise HTTPException(status_code=502, detail="Failed to resolve Jellyfin stream") @@ -194,10 +209,11 @@ async def stream_local_file( @router.head("/navidrome/{item_id}") async def head_navidrome_audio( item_id: str, + current_user: CurrentUserDep, playback_service: NavidromePlaybackService = Depends(get_navidrome_playback_service), ) -> Response: try: - return await playback_service.proxy_head(item_id) + return await playback_service.proxy_head(item_id, user_id=current_user.id) except ValueError: raise HTTPException(status_code=400, detail="Invalid stream request") except ExternalServiceError: @@ -208,10 +224,13 @@ async def head_navidrome_audio( async def stream_navidrome_audio( item_id: str, request: Request, + current_user: CurrentUserDep, playback_service: NavidromePlaybackService = Depends(get_navidrome_playback_service), ) -> StreamingResponse: try: - return await playback_service.proxy_stream(item_id, request.headers.get("Range")) + return await playback_service.proxy_stream( + item_id, request.headers.get("Range"), user_id=current_user.id + ) except ValueError: raise HTTPException(status_code=400, detail="Invalid stream request") except ExternalServiceError as e: @@ -254,10 +273,11 @@ async def navidrome_stopped( @router.head("/plex/{part_key:path}") async def head_plex_audio( part_key: str, + current_user: CurrentUserDep, playback_service: PlexPlaybackService = Depends(get_plex_playback_service), ) -> Response: try: - return await playback_service.proxy_head(part_key) + return await playback_service.proxy_head(part_key, user_id=current_user.id) except ValueError: raise HTTPException(status_code=400, detail="Invalid stream request") except ExternalServiceError: @@ -268,10 +288,13 @@ async def head_plex_audio( async def stream_plex_audio( part_key: str, request: Request, + current_user: CurrentUserDep, playback_service: PlexPlaybackService = Depends(get_plex_playback_service), ) -> StreamingResponse: try: - return await playback_service.proxy_stream(part_key, request.headers.get("Range")) + return await playback_service.proxy_stream( + part_key, request.headers.get("Range"), user_id=current_user.id + ) except ValueError: raise HTTPException(status_code=400, detail="Invalid stream request") except ExternalServiceError as e: diff --git a/backend/api/v1/schemas/library_policies.py b/backend/api/v1/schemas/library_policies.py index b011dfdc2..7828b3a58 100644 --- a/backend/api/v1/schemas/library_policies.py +++ b/backend/api/v1/schemas/library_policies.py @@ -31,6 +31,10 @@ class TypedLibrarySettings(AppStruct): staging_path: str = "" naming_template: str = DEFAULT_NAMING_TEMPLATE acoustid_api_key: str = "" + # Master switch (GH #276): when False the target application stops claiming + # new scan/identification/organization work. Deliberately excluded from the + # policy revision hash so toggling never churns boundary transitions. + enabled: bool = True class LibrarySettingsResponse(TypedLibrarySettings): diff --git a/backend/api/v1/schemas/library_scan_target.py b/backend/api/v1/schemas/library_scan_target.py index d640e3181..d1d2f6c5a 100644 --- a/backend/api/v1/schemas/library_scan_target.py +++ b/backend/api/v1/schemas/library_scan_target.py @@ -52,6 +52,20 @@ class ScanRunDetailResponse(AppStruct): snapshot: ScanRunSnapshot +class ScanRunFailureItem(AppStruct): + root_id: str + relative_path: str + failure_code: str + failure_detail: str + phase: Literal["discovering", "indexing", "reconciling"] + recorded_at: float + + +class ScanRunFailuresResponse(AppStruct): + items: list[ScanRunFailureItem] + next_cursor: int | None = None + + class LibraryActivityItem(AppStruct): kind: Literal["scan", "identification"] state: str @@ -67,6 +81,8 @@ class LibraryActivityItem(AppStruct): needs_review_count: int = 0 failed_count: int = 0 deferred_count: int = 0 + deferred_reason_counts: dict[str, int] = msgspec.field(default_factory=dict) + attention_count: int = 0 priority_band: str | None = None oldest_backlog_at: float | None = None provider_unavailable: bool = False diff --git a/backend/api/v1/schemas/playlists.py b/backend/api/v1/schemas/playlists.py index f38c9ba85..b0cfc5433 100644 --- a/backend/api/v1/schemas/playlists.py +++ b/backend/api/v1/schemas/playlists.py @@ -20,6 +20,7 @@ class PlaylistTrackResponse(AppStruct): duration: int | None = None created_at: str = "" plex_rating_key: str | None = None + library_file_id: str | None = None class PlaylistSummaryResponse(AppStruct): diff --git a/backend/core/dependencies/__init__.py b/backend/core/dependencies/__init__.py index e0cc16c0a..6768241c7 100644 --- a/backend/core/dependencies/__init__.py +++ b/backend/core/dependencies/__init__.py @@ -122,6 +122,7 @@ get_target_identification_queue, get_library_administrative_work_service, get_target_album_identification_service, + get_mb_provider_availability, get_target_album_coverage_service, get_target_reidentification_service, get_target_album_edition_finder_service, @@ -279,6 +280,7 @@ TargetIdentificationQueueDep, LibraryAdministrativeWorkServiceDep, TargetAlbumIdentificationServiceDep, + MbProviderAvailabilityDep, TargetAlbumCoverageServiceDep, TargetReidentificationServiceDep, TargetAlbumEditionFinderServiceDep, diff --git a/backend/core/dependencies/service_providers.py b/backend/core/dependencies/service_providers.py index 097f40f85..74136ed61 100644 --- a/backend/core/dependencies/service_providers.py +++ b/backend/core/dependencies/service_providers.py @@ -2,6 +2,7 @@ import asyncio import logging +from collections.abc import Callable from infrastructure.cache.cache_keys import ( library_raw_albums_key, @@ -488,6 +489,13 @@ def get_library_administrative_work_service() -> "LibraryAdministrativeWorkServi return LibraryAdministrativeWorkService(get_native_library_store()) +def get_mb_provider_availability() -> Callable[[], bool]: + """Live MusicBrainz breaker read shared by identification and activity routes.""" + from repositories.musicbrainz_base import mb_circuit_breaker + + return lambda: not mb_circuit_breaker.is_open() + + @singleton def get_target_album_identification_service() -> "AlbumIdentificationService": from services.native.album_candidate_service import AlbumCandidateService @@ -496,7 +504,6 @@ def get_target_album_identification_service() -> "AlbumIdentificationService": from services.native.conditional_fingerprint_service import ( ConditionalFingerprintService, ) - from repositories.musicbrainz_base import mb_circuit_breaker from .cache_providers import get_native_library_store @@ -520,7 +527,7 @@ async def invalidate(_domains: set[str]) -> None: ConditionalFingerprintService(store, get_audio_fingerprinter()), invalidate, _schedule_identified_album_work, - provider_available=lambda: not mb_circuit_breaker.is_open(), + provider_available=get_mb_provider_availability(), ) @@ -634,6 +641,7 @@ def get_target_identity_repair_service() -> "IdentityRepairService": get_musicbrainz_identification_repository(), AlbumEvidenceEngine(), get_musicbrainz_repository(), + provider_available=get_mb_provider_availability(), ) @@ -2580,6 +2588,9 @@ def get_acquisition_cleanup_service() -> "AcquisitionCleanupService": lambda: Path( get_preferences_service().get_sabnzbd_connection_raw().downloads_mount ), + sab_category_getter=lambda: get_preferences_service() + .get_sabnzbd_connection_raw() + .category, ) diff --git a/backend/core/dependencies/type_aliases.py b/backend/core/dependencies/type_aliases.py index 40fe6ab8f..347bf6394 100644 --- a/backend/core/dependencies/type_aliases.py +++ b/backend/core/dependencies/type_aliases.py @@ -2,6 +2,7 @@ from __future__ import annotations +from collections.abc import Callable from typing import Annotated from fastapi import Depends @@ -168,6 +169,7 @@ get_library_administrative_work_service, get_target_album_coverage_service, get_target_album_identification_service, + get_mb_provider_availability, get_target_reidentification_service, get_target_album_edition_finder_service, get_target_library_review_service, @@ -302,6 +304,9 @@ TargetAlbumIdentificationServiceDep = Annotated[ AlbumIdentificationService, Depends(get_target_album_identification_service) ] +MbProviderAvailabilityDep = Annotated[ + Callable[[], bool], Depends(get_mb_provider_availability) +] TargetAlbumCoverageServiceDep = Annotated[ AlbumCoverageService, Depends(get_target_album_coverage_service) ] diff --git a/backend/core/tasks.py b/backend/core/tasks.py index bfcf5c762..2efa6c068 100644 --- a/backend/core/tasks.py +++ b/backend/core/tasks.py @@ -424,9 +424,14 @@ async def warm_navidrome_mbid_cache(service_getter=None) -> None: try: service = service_getter() await service.warm_mbid_cache() + except asyncio.CancelledError: + break except Exception as e: logger.error("Navidrome MBID cache warming failed: %s", e, exc_info=True) - await asyncio.sleep(14400) + try: + await asyncio.sleep(14400) + except asyncio.CancelledError: + break async def warm_plex_mbid_cache(service_getter=None) -> None: @@ -441,9 +446,14 @@ async def warm_plex_mbid_cache(service_getter=None) -> None: service = service_getter() await service.warm_mbid_cache() await service.persist_if_dirty() + except asyncio.CancelledError: + break except Exception as e: logger.error("Plex MBID cache warming failed: %s", e, exc_info=True) - await asyncio.sleep(14400) + try: + await asyncio.sleep(14400) + except asyncio.CancelledError: + break async def warm_artist_discovery_cache_periodically( @@ -761,155 +771,160 @@ async def warm_audiodb_cache_periodically( await asyncio.sleep(_AUDIODB_SWEEP_INITIAL_DELAY) while True: + try: + await _run_audiodb_sweep_cycle( + audiodb_image_service, + library_db, + preferences_service, + precache_service, + workload_gate, + ) + except asyncio.CancelledError: + break + except Exception as e: + logger.error("AudioDB sweep cycle failed: %s", e, exc_info=True) try: await asyncio.sleep(_AUDIODB_SWEEP_INTERVAL) + except asyncio.CancelledError: + break - if workload_gate is not None: - await workload_gate.wait_until_available() - - settings = preferences_service.get_advanced_settings() - if not settings.audiodb_enabled: - continue - - cursor = preferences_service.get_setting("audiodb_sweep_cursor") - all_items = await library_db.get_enrichment_candidates( - after_mbid=cursor, - limit=_AUDIODB_SWEEP_MAX_ITEMS, - ) - if not all_items: - preferences_service.save_setting("audiodb_sweep_cursor", None) - preferences_service.save_setting("audiodb_sweep_last_completed", time()) - continue - items_needing_refresh: list[tuple[str, str, dict]] = [] - inspected_cursor = cursor - inspection_complete = True - for entity_type, mbid, data in all_items: - if workload_gate is not None and workload_gate.scan_active: - inspection_complete = False - break - inspected_cursor = f"{entity_type}:{mbid}" - if len(items_needing_refresh) >= _AUDIODB_SWEEP_MAX_ITEMS: - break - if entity_type == "artist": - cached = await audiodb_image_service.get_cached_artist_images(mbid) - else: - cached = await audiodb_image_service.get_cached_album_images(mbid) - if cached is None: - items_needing_refresh.append((entity_type, mbid, data)) - - if not items_needing_refresh: - page_complete = inspection_complete and ( - len(all_items) < _AUDIODB_SWEEP_MAX_ITEMS - ) - preferences_service.save_setting( - "audiodb_sweep_cursor", None if page_complete else inspected_cursor - ) - if page_complete: - preferences_service.save_setting( - "audiodb_sweep_last_completed", time() - ) - continue +async def _run_audiodb_sweep_cycle( + audiodb_image_service: "AudioDBImageService", + library_db: "LibraryDB", + preferences_service: "PreferencesService", + precache_service: "LibraryPrecacheService | None", + workload_gate: "BackgroundWorkloadGate | None", +) -> None: + if workload_gate is not None: + await workload_gate.wait_until_available() - processed = 0 - processed_cursor = cursor - bytes_ok = 0 - bytes_fail = 0 - for entity_type, mbid, data in items_needing_refresh: - if workload_gate is not None and workload_gate.scan_active: - break - if not preferences_service.get_advanced_settings().audiodb_enabled: - break + settings = preferences_service.get_advanced_settings() + if not settings.audiodb_enabled: + return - try: - if entity_type == "artist": - name = data.get("name") if isinstance(data, dict) else None - result = ( - await audiodb_image_service.fetch_and_cache_artist_images( - mbid, - name, - is_monitored=True, - ) - ) - if ( - result - and not result.is_negative - and result.thumb_url - and precache_service - ): - if await precache_service._download_audiodb_bytes( - result.thumb_url, "artist", mbid - ): - bytes_ok += 1 - else: - bytes_fail += 1 - else: - artist_name = ( - data.get("artist_name") - if isinstance(data, dict) - else getattr(data, "artist_name", None) - ) - album_name = ( - data.get("title") - if isinstance(data, dict) - else getattr(data, "title", None) - ) - result = ( - await audiodb_image_service.fetch_and_cache_album_images( - mbid, - artist_name=artist_name, - album_name=album_name, - is_monitored=True, - ) - ) - if ( - result - and not result.is_negative - and result.album_thumb_url - and precache_service - ): - if await precache_service._download_audiodb_bytes( - result.album_thumb_url, "album", mbid - ): - bytes_ok += 1 - else: - bytes_fail += 1 - except Exception as e: - logger.error( - "audiodb.sweep action=item_error entity_type=%s mbid=%s error=%s", - entity_type, - mbid[:8], - e, - exc_info=True, - ) + cursor = preferences_service.get_setting("audiodb_sweep_cursor") + all_items = await library_db.get_enrichment_candidates( + after_mbid=cursor, + limit=_AUDIODB_SWEEP_MAX_ITEMS, + ) + if not all_items: + preferences_service.save_setting("audiodb_sweep_cursor", None) + preferences_service.save_setting("audiodb_sweep_last_completed", time()) + return - processed += 1 - processed_cursor = f"{entity_type}:{mbid}" - if processed % _AUDIODB_SWEEP_CURSOR_PERSIST_INTERVAL == 0: - preferences_service.save_setting( - "audiodb_sweep_cursor", processed_cursor - ) + items_needing_refresh: list[tuple[str, str, dict]] = [] + inspected_cursor = cursor + inspection_complete = True + for entity_type, mbid, data in all_items: + if workload_gate is not None and workload_gate.scan_active: + inspection_complete = False + break + inspected_cursor = f"{entity_type}:{mbid}" + if len(items_needing_refresh) >= _AUDIODB_SWEEP_MAX_ITEMS: + break + if entity_type == "artist": + cached = await audiodb_image_service.get_cached_artist_images(mbid) + else: + cached = await audiodb_image_service.get_cached_album_images(mbid) + if cached is None: + items_needing_refresh.append((entity_type, mbid, data)) + + if not items_needing_refresh: + page_complete = inspection_complete and ( + len(all_items) < _AUDIODB_SWEEP_MAX_ITEMS + ) + preferences_service.save_setting( + "audiodb_sweep_cursor", None if page_complete else inspected_cursor + ) + if page_complete: + preferences_service.save_setting("audiodb_sweep_last_completed", time()) + return - await asyncio.sleep(_AUDIODB_SWEEP_INTER_ITEM_DELAY) + processed = 0 + processed_cursor = cursor + bytes_ok = 0 + bytes_fail = 0 + for entity_type, mbid, data in items_needing_refresh: + if workload_gate is not None and workload_gate.scan_active: + break + if not preferences_service.get_advanced_settings().audiodb_enabled: + break - if processed >= len(items_needing_refresh) and inspection_complete: - page_complete = len(all_items) < _AUDIODB_SWEEP_MAX_ITEMS - preferences_service.save_setting( - "audiodb_sweep_cursor", None if page_complete else inspected_cursor + try: + if entity_type == "artist": + name = data.get("name") if isinstance(data, dict) else None + result = await audiodb_image_service.fetch_and_cache_artist_images( + mbid, + name, + is_monitored=True, ) - if page_complete: - preferences_service.save_setting( - "audiodb_sweep_last_completed", time() - ) + if ( + result + and not result.is_negative + and result.thumb_url + and precache_service + ): + if await precache_service._download_audiodb_bytes( + result.thumb_url, "artist", mbid + ): + bytes_ok += 1 + else: + bytes_fail += 1 else: - preferences_service.save_setting( - "audiodb_sweep_cursor", processed_cursor + artist_name = ( + data.get("artist_name") + if isinstance(data, dict) + else getattr(data, "artist_name", None) ) - - except asyncio.CancelledError: - break + album_name = ( + data.get("title") + if isinstance(data, dict) + else getattr(data, "title", None) + ) + result = await audiodb_image_service.fetch_and_cache_album_images( + mbid, + artist_name=artist_name, + album_name=album_name, + is_monitored=True, + ) + if ( + result + and not result.is_negative + and result.album_thumb_url + and precache_service + ): + if await precache_service._download_audiodb_bytes( + result.album_thumb_url, "album", mbid + ): + bytes_ok += 1 + else: + bytes_fail += 1 except Exception as e: - logger.error("AudioDB sweep cycle failed: %s", e, exc_info=True) + logger.error( + "audiodb.sweep action=item_error entity_type=%s mbid=%s error=%s", + entity_type, + mbid[:8], + e, + exc_info=True, + ) + + processed += 1 + processed_cursor = f"{entity_type}:{mbid}" + if processed % _AUDIODB_SWEEP_CURSOR_PERSIST_INTERVAL == 0: + preferences_service.save_setting("audiodb_sweep_cursor", processed_cursor) + + await asyncio.sleep(_AUDIODB_SWEEP_INTER_ITEM_DELAY) + + if processed >= len(items_needing_refresh) and inspection_complete: + page_complete = len(all_items) < _AUDIODB_SWEEP_MAX_ITEMS + preferences_service.save_setting( + "audiodb_sweep_cursor", None if page_complete else inspected_cursor + ) + if page_complete: + preferences_service.save_setting("audiodb_sweep_last_completed", time()) + else: + preferences_service.save_setting("audiodb_sweep_cursor", processed_cursor) def start_audiodb_sweep_task( @@ -1342,18 +1357,19 @@ async def prune_recycle_bin_periodically( await asyncio.sleep(600) while True: try: - policy = preferences_service.get_download_policy() library = preferences_service.get_typed_library_settings() - bin_path = resolve_bin_path( - policy.recycle_bin_path, - [root.path for root in library.library_roots], - ) - if bin_path is not None: - removed = await asyncio.to_thread( - prune, bin_path, policy.recycle_retention_days + if library.enabled: + policy = preferences_service.get_download_policy() + bin_path = resolve_bin_path( + policy.recycle_bin_path, + [root.path for root in library.library_roots], ) - if removed: - logger.info("Recycle bin prune removed %d entries", removed) + if bin_path is not None: + removed = await asyncio.to_thread( + prune, bin_path, policy.recycle_retention_days + ) + if removed: + logger.info("Recycle bin prune removed %d entries", removed) except asyncio.CancelledError: break except Exception as e: diff --git a/backend/infrastructure/persistence/native_library_schema.py b/backend/infrastructure/persistence/native_library_schema.py index 32624b328..1f32125af 100644 --- a/backend/infrastructure/persistence/native_library_schema.py +++ b/backend/infrastructure/persistence/native_library_schema.py @@ -1288,6 +1288,17 @@ PRIMARY KEY(run_id, root_id, relative_path) ); +CREATE TABLE IF NOT EXISTS library_scan_failures ( + run_id TEXT NOT NULL REFERENCES library_scan_runs(id) ON DELETE CASCADE, + root_id TEXT NOT NULL, + relative_path TEXT NOT NULL, + failure_code TEXT NOT NULL, + failure_detail TEXT NOT NULL DEFAULT '', + phase TEXT NOT NULL CHECK(phase IN ('discovering','indexing','reconciling')), + recorded_at REAL NOT NULL, + PRIMARY KEY(run_id, root_id, relative_path, phase, failure_code) +); + CREATE TABLE IF NOT EXISTS library_scan_management_candidates ( run_id TEXT NOT NULL REFERENCES library_scan_runs(id) ON DELETE CASCADE, local_album_id TEXT NOT NULL REFERENCES local_albums(id) ON DELETE CASCADE, @@ -1710,6 +1721,7 @@ CREATE UNIQUE INDEX IF NOT EXISTS idx_scan_runs_single_queued ON library_scan_runs((1)) WHERE state = 'queued'; CREATE INDEX IF NOT EXISTS idx_scan_inventory_processing ON library_scan_inventory(run_id, processing_state, root_id, relative_path); +CREATE INDEX IF NOT EXISTS idx_scan_failures_run ON library_scan_failures(run_id); CREATE INDEX IF NOT EXISTS idx_scan_inventory_management_candidates ON library_scan_inventory(run_id, processing_state, comparison_result, local_track_id); CREATE INDEX IF NOT EXISTS idx_scan_management_candidates_due ON library_scan_management_candidates(state, next_attempt_at, run_id, local_album_id); CREATE INDEX IF NOT EXISTS idx_scan_grouping_pending ON library_scan_grouping_contexts(run_id, state, root_id, relative_directory); diff --git a/backend/infrastructure/persistence/native_library_store.py b/backend/infrastructure/persistence/native_library_store.py index 343540d1b..b314a9a3a 100644 --- a/backend/infrastructure/persistence/native_library_store.py +++ b/backend/infrastructure/persistence/native_library_store.py @@ -100,6 +100,7 @@ OperationWorkItem, RepairFinding, ReviewDecision, + ScanFailureRecord, ScanInventoryItem, ScanRequest, ScanRequestResult, @@ -131,6 +132,7 @@ AUTOMATIC_SAFE_EVIDENCE_REASONS = frozenset( {"SUPPORTED", "ACCEPTED", "SUPPORTED_EMBEDDED_IDS"} ) +ATTENTION_FAILURE_CODES = frozenset({"MAX_DEFERRALS_EXCEEDED", "SUBJECT_NOT_AVAILABLE"}) BULK_PREVIEW_BATCH_SIZE = 500 BULK_PREVIEW_CLEANUP_BATCH_SIZE = 5_000 MANAGEMENT_PERSISTENCE_BATCH_SIZE = 500 @@ -6660,7 +6662,8 @@ def _enqueue_identification_job_result( (job.created_at, decision["id"]), ) existing = connection.execute( - "SELECT id FROM library_identification_jobs WHERE dedupe_key = ? " + "SELECT id, state, last_failure_code FROM library_identification_jobs " + "WHERE dedupe_key = ? " + ( "AND state IN ('queued','running','paused') " if job.kind == "review_retry" @@ -6670,6 +6673,23 @@ def _enqueue_identification_job_result( (job.dedupe_key,), ).fetchone() if existing is not None: + if ( + str(existing["state"]) == "failed" + and str(existing["last_failure_code"] or "") in ATTENTION_FAILURE_CODES + ): + # A terminal-failed attention job must not block every future + # enqueue of the same dedupe key: resurrect it so a fixed album + # identifies again instead of silently never retrying. + connection.execute( + "UPDATE library_identification_jobs SET state = 'queued', " + "attempt_count = 0, not_before = ?, last_failure_code = NULL, " + "terminal_at = NULL, updated_at = ?, " + "row_revision = row_revision + 1, event_revision = event_revision + 1 " + "WHERE id = ?", + (job.not_before, job.created_at, existing["id"]), + ) + self._bump_stream(connection, "identification") + return str(existing["id"]), True return str(existing["id"]), False queued_subject = connection.execute( f"SELECT id FROM library_identification_jobs WHERE {subject_column} = ? " @@ -7442,6 +7462,88 @@ def operation(connection: sqlite3.Connection) -> int: self.work_wakeups.notify_after("identification", not_before - now) return result + async def terminal_fail_identification_job( + self, + job_id: str, + *, + worker_id: str, + expected_job_revision: int, + failure_code: str, + now: float, + ) -> int: + """Fail a running job terminally, keeping the row for auditability.""" + + def operation(connection: sqlite3.Connection) -> int: + row = connection.execute( + "UPDATE library_identification_jobs SET state = 'failed', " + "last_failure_code = ?, terminal_at = ?, lease_owner = NULL, " + "lease_expires_at = NULL, heartbeat_at = NULL, updated_at = ?, " + "row_revision = row_revision + 1, event_revision = event_revision + 1 " + "WHERE id = ? AND state = 'running' AND lease_owner = ? " + "AND row_revision = ? RETURNING row_revision", + (failure_code, now, now, job_id, worker_id, expected_job_revision), + ).fetchone() + if row is None: + raise StaleRevisionError( + "The identification job changed before it could be failed." + ) + self._bump_stream(connection, "identification") + return int(row["row_revision"]) + + return await self._write(operation) + + async def reset_provider_identification_deferrals(self, *, now: float) -> int: + """Clear backoff on provider-deferred queued jobs after recovery. + + Only rows deferred for PROVIDER_TEMPORARILY_UNAVAILABLE are reset; other + deferral reasons keep their backoff untouched. + """ + + def operation(connection: sqlite3.Connection) -> int: + cursor = connection.execute( + "UPDATE library_identification_jobs SET attempt_count = 0, " + "not_before = 0, last_failure_code = NULL, updated_at = ?, " + "row_revision = row_revision + 1, event_revision = event_revision + 1 " + "WHERE state = 'queued' " + "AND last_failure_code = 'PROVIDER_TEMPORARILY_UNAVAILABLE' " + "AND row_revision < ? AND event_revision < ?", + (now, MAX_REVISION, MAX_REVISION), + ) + if cursor.rowcount: + self._bump_stream(connection, "identification") + return cursor.rowcount + + result = await self._write(operation) + if result: + self.work_wakeups.notify("identification") + return result + + async def gc_stale_identification_jobs( + self, *, now: float, grace_seconds: float + ) -> int: + """Terminally fail queued SUBJECT_NOT_AVAILABLE jobs past the grace period. + + Tracks can legitimately reappear, so the worker keeps deferring until the + grace expires; this sweep is the backstop for rows that are never claimed + again (the deferral cap bounds actively-claimed rows first). + """ + + def operation(connection: sqlite3.Connection) -> int: + cursor = connection.execute( + "UPDATE library_identification_jobs SET state = 'failed', " + "terminal_at = ?, updated_at = ?, " + "row_revision = row_revision + 1, event_revision = event_revision + 1 " + "WHERE state = 'queued' " + "AND last_failure_code = 'SUBJECT_NOT_AVAILABLE' " + "AND updated_at < ? AND row_revision < ? AND event_revision < ?", + (now, now, now - grace_seconds, MAX_REVISION, MAX_REVISION), + ) + if cursor.rowcount: + self._bump_stream(connection, "identification") + return cursor.rowcount + + return await self._write(operation) + async def pause_identification_queue( self, *, @@ -7614,6 +7716,22 @@ def operation(connection: sqlite3.Connection) -> dict[str, Any]: "WHERE state IN ('queued','running','paused')" ).fetchone()[0] ) + attention_count = int( + connection.execute( + "SELECT COUNT(*) FROM library_identification_jobs " + "WHERE state = 'failed' AND last_failure_code IN " + "('MAX_DEFERRALS_EXCEEDED','SUBJECT_NOT_AVAILABLE')" + ).fetchone()[0] + ) + deferred_reason_counts = { + str(row["last_failure_code"]): int(row["count"]) + for row in connection.execute( + "SELECT last_failure_code, COUNT(*) AS count " + "FROM library_identification_jobs " + "WHERE state IN ('queued','running','paused') " + "AND last_failure_code IS NOT NULL GROUP BY last_failure_code" + ).fetchall() + } return { "control_state": str(control["state"]), "control_revision": int(control["row_revision"]), @@ -7621,6 +7739,8 @@ def operation(connection: sqlite3.Connection) -> dict[str, Any]: "started_at": active["started_at"], "updated_at": active["updated_at"], "deferred_count": int(active["deferred_count"] or 0), + "deferred_reason_counts": deferred_reason_counts, + "attention_count": attention_count, "kept_local_count": kept_local_count, "active_priority": ( int(active_priority["priority"]) @@ -9053,6 +9173,13 @@ def operation( ) return None, 0, True run_id = str(row["id"]) + deleted_failures = connection.execute( + "DELETE FROM library_scan_failures WHERE rowid IN (" + "SELECT rowid FROM library_scan_failures WHERE run_id=? LIMIT ?)", + (run_id, max(1, limit)), + ).rowcount + if deleted_failures: + return run_id, deleted_failures, False for table in ( "library_scan_grouping_evidence", "library_scan_grouping_edges", @@ -9725,6 +9852,15 @@ def operation(connection: sqlite3.Connection) -> ScanRun: for root_id, path, failure_code in failures ], ) + connection.executemany( + "INSERT OR IGNORE INTO library_scan_failures " + "(run_id, root_id, relative_path, failure_code, failure_detail, " + "phase, recorded_at) VALUES (?, ?, ?, ?, '', 'indexing', ?)", + [ + (run_id, root_id, path, failure_code, updated_at) + for root_id, path, failure_code in failures + ], + ) if catalog_changed: self._bump_catalog(connection) allowed = { @@ -9765,6 +9901,69 @@ def operation(connection: sqlite3.Connection) -> ScanRun: return await self._write_scan(operation) + async def record_scan_failures( + self, run_id: str, failures: list[ScanFailureRecord] + ) -> None: + """Persist per-path scan failures; the PK dedupes repeated reports.""" + + if not failures: + return + + def operation(connection: sqlite3.Connection) -> None: + connection.executemany( + "INSERT OR IGNORE INTO library_scan_failures " + "(run_id, root_id, relative_path, failure_code, failure_detail, " + "phase, recorded_at) VALUES (?, ?, ?, ?, ?, ?, ?)", + [ + ( + run_id, + failure.root_id, + failure.relative_path, + failure.failure_code, + failure.failure_detail, + failure.phase, + failure.recorded_at, + ) + for failure in failures + ], + ) + + return await super()._background_write(operation) + + async def list_scan_run_failures( + self, + run_id: str, + *, + limit: int = 50, + cursor_rowid: int | None = None, + ) -> tuple[list[ScanFailureRecord], int | None]: + """Read one rowid-keyset page of recorded scan failures for a run.""" + + def operation( + connection: sqlite3.Connection, + ) -> tuple[list[ScanFailureRecord], int | None]: + rows = connection.execute( + "SELECT rowid, root_id, relative_path, failure_code, failure_detail, " + "phase, recorded_at FROM library_scan_failures WHERE run_id = ? " + "AND (? IS NULL OR rowid > ?) ORDER BY rowid LIMIT ?", + (run_id, cursor_rowid, cursor_rowid, max(1, limit) + 1), + ).fetchall() + page = rows[: max(1, limit)] + next_cursor = int(page[-1]["rowid"]) if len(rows) > len(page) else None + return [ + ScanFailureRecord( + root_id=str(row["root_id"]), + relative_path=str(row["relative_path"]), + failure_code=str(row["failure_code"]), + recorded_at=float(row["recorded_at"]), + failure_detail=str(row["failure_detail"]), + phase=row["phase"], + ) + for row in page + ], next_cursor + + return await self._read(operation) + async def upsert_scanned_track( self, *, @@ -27469,6 +27668,57 @@ def operation(connection: sqlite3.Connection) -> dict[str, int]: return await self._read(operation) + async def defer_repair_audit_work( + self, + *, + job_id: str, + ordinal: int | None, + worker_id: str, + reason_code: str, + now: float, + retry_not_before: float | None = None, + ) -> dict[str, Any]: + """Release a running repair audit back to queued for a later retry. + + The running work item (when one is claimed) is returned to 'pending' + with the failure code; the whole job is queued with next_attempt_at so + the audit resumes exactly at the deferred item. Nothing is marked + succeeded and no finding row is written. + """ + + def operation(connection: sqlite3.Connection) -> dict[str, Any]: + if ordinal is not None: + work_update = connection.execute( + "UPDATE library_operation_work SET state = 'pending', failure_code = ?, " + "updated_at = ?, row_revision = row_revision + 1 " + "WHERE job_id = ? AND ordinal = ? AND state = 'running'", + (reason_code, now, job_id, ordinal), + ) + if work_update.rowcount != 1: + raise StaleRevisionError( + "The repair audit work lease changed while it was deferred." + ) + updated = connection.execute( + "UPDATE library_operation_jobs SET state = 'queued', lease_owner = NULL, " + "lease_expires_at = NULL, heartbeat_at = NULL, updated_at = ?, " + "next_attempt_at = ?, " + "row_revision = row_revision + 1, event_revision = event_revision + 1 " + "WHERE id = ? AND kind = 'repair' AND state = 'running' " + "AND lease_owner = ? RETURNING *", + (now, retry_not_before, job_id, worker_id), + ).fetchone() + if updated is None: + raise StaleRevisionError( + "The repair audit lease changed while it was deferred." + ) + self._bump_stream(connection, "operation") + return dict(updated) + + result = await self._write(operation) + if retry_not_before is not None: + self.work_wakeups.notify_after("operation", retry_not_before - now) + return result + async def save_repair_finding_for_work( self, job_id: str, diff --git a/backend/models/library_work.py b/backend/models/library_work.py index 88c316a9e..98479f839 100644 --- a/backend/models/library_work.py +++ b/backend/models/library_work.py @@ -225,6 +225,15 @@ class ScanInventoryItem(AppStruct): scope_relative_path: str = "." +class ScanFailureRecord(AppStruct): + root_id: str + relative_path: str + failure_code: str + recorded_at: float + failure_detail: str = "" + phase: Literal["discovering", "indexing", "reconciling"] = "discovering" + + class ScannedTrackWrite(msgspec.Struct): artist: LocalArtist album: LocalAlbum diff --git a/backend/services/audiodb_browse_queue.py b/backend/services/audiodb_browse_queue.py index 7162aabc9..8887b8614 100644 --- a/backend/services/audiodb_browse_queue.py +++ b/backend/services/audiodb_browse_queue.py @@ -94,6 +94,7 @@ async def _process_queue( try: settings = preferences_service.get_advanced_settings() if not settings.audiodb_enabled: + await asyncio.sleep(_BROWSE_QUEUE_INTER_ITEM_DELAY) continue if item.entity_type == "artist": diff --git a/backend/services/jellyfin_library_service.py b/backend/services/jellyfin_library_service.py index 77b16dce7..5095efb54 100644 --- a/backend/services/jellyfin_library_service.py +++ b/backend/services/jellyfin_library_service.py @@ -239,6 +239,20 @@ async def match_album_by_mbid(self, musicbrainz_id: str) -> JellyfinAlbumMatch: tracks=tracks, ) + async def resolve_album_mbid(self, album_id: str) -> str | None: + """Map a Jellyfin album GUID to its MusicBrainz release-group MBID. + + Reads ProviderIds.MusicBrainzReleaseGroup, falling back to + MusicBrainzAlbum - the same precedence JellyfinAlbumDetail uses. + Returns None when the album or its provider ids are unavailable + (get_album_detail degrades to None on upstream failure). + """ + item = await self._jellyfin.get_album_detail(album_id) + if not item: + return None + pids = item.provider_ids or {} + return pids.get("MusicBrainzReleaseGroup") or pids.get("MusicBrainzAlbum") + async def get_artists( self, limit: int = 50, offset: int = 0 ) -> list[JellyfinArtistSummary]: @@ -584,6 +598,21 @@ async def import_playlist( return JellyfinImportResult(droppedneedle_playlist_id=re_check.id, already_imported=True) raise + # Map each distinct Jellyfin album GUID to its MusicBrainz MBID so the + # stored album_id can match the MBID-keyed local catalog (#150). One + # deduped fetch per album; failures keep the GUID (today's behavior). + album_mbids: dict[str, str] = {} + distinct_album_ids = sorted({t.album_id for t in detail.tracks if t.album_id}) + if distinct_album_ids: + resolved = await asyncio.gather( + *(self.resolve_album_mbid(guid) for guid in distinct_album_ids) + ) + album_mbids = { + guid: mbid + for guid, mbid in zip(distinct_album_ids, resolved) + if mbid + } + track_dicts = [] failed = 0 for t in detail.tracks: @@ -595,7 +624,7 @@ async def import_playlist( "duration": t.duration_seconds, "track_source_id": t.id, "source_type": "jellyfin", - "album_id": t.album_id, + "album_id": album_mbids.get(t.album_id) or t.album_id, "artist_id": t.artist_id, "track_number": t.track_number, "disc_number": t.disc_number, diff --git a/backend/services/jellyfin_playback_service.py b/backend/services/jellyfin_playback_service.py index 74644c07b..0948d720f 100644 --- a/backend/services/jellyfin_playback_service.py +++ b/backend/services/jellyfin_playback_service.py @@ -151,16 +151,44 @@ async def stop_playback( except (httpx.HTTPError, ExternalServiceError) as e: logger.warning("Stop report failed for %s: %s", item_id, e) - async def proxy_head(self, item_id: str) -> Response: - result: StreamProxyResult = await self._jellyfin.proxy_head_stream(item_id) + async def proxy_head(self, item_id: str, user_id: str | None = None) -> Response: + repo = await self._repo_for(user_id) + try: + result: StreamProxyResult = await repo.proxy_head_stream(item_id) + except JellyfinAuthError: + if repo is self._jellyfin: + raise + # linked account's token was revoked: the stream still rides the + # app-level account (attribution stays fail closed) + logger.warning( + "Per-user Jellyfin token rejected for user %s, " + "streaming %s via app account", + user_id, + item_id, + ) + result = await self._jellyfin.proxy_head_stream(item_id) return Response(status_code=200, headers=result.headers) async def proxy_stream( - self, item_id: str, range_header: str | None = None + self, item_id: str, range_header: str | None = None, user_id: str | None = None ) -> StreamingResponse: - result: StreamProxyResult = await self._jellyfin.proxy_get_stream( - item_id, range_header=range_header - ) + repo = await self._repo_for(user_id) + try: + result: StreamProxyResult = await repo.proxy_get_stream( + item_id, range_header=range_header + ) + except JellyfinAuthError: + if repo is self._jellyfin: + raise + logger.warning( + "Per-user Jellyfin token rejected for user %s, " + "streaming %s via app account", + user_id, + item_id, + ) + result = await self._jellyfin.proxy_get_stream( + item_id, range_header=range_header + ) return StreamingResponse( content=result.body_chunks, status_code=result.status_code, diff --git a/backend/services/native/acquisition_cleanup_service.py b/backend/services/native/acquisition_cleanup_service.py index 5da735321..b113bf93e 100644 --- a/backend/services/native/acquisition_cleanup_service.py +++ b/backend/services/native/acquisition_cleanup_service.py @@ -61,12 +61,14 @@ def __init__( client_getter: Callable[[str], DownloadClientProtocol], sab_mount_getter: Callable[[], Path], *, + sab_category_getter: Callable[[], str] = lambda: "*", clock: Callable[[], float] = time.time, ) -> None: self._store = download_store self._library_store = library_store self._client_getter = client_getter self._sab_mount_getter = sab_mount_getter + self._sab_category_getter = sab_category_getter self._clock = clock async def run_once(self, worker_id: str) -> int: @@ -591,6 +593,7 @@ async def reconcile_legacy_mount( return 0 processed = 0 + category = self._sab_category_getter() while processed < max(1, limit): if progress.current_directory is None: if not progress.pending_directories: @@ -625,7 +628,9 @@ async def reconcile_legacy_mount( match, is_symlink=is_symlink, ) - elif is_directory and not is_symlink: + elif is_directory and not is_symlink and _descend_allowed( + name, category + ): progress.pending_directories.append(relative.as_posix()) if processed >= max(1, limit): break @@ -994,14 +999,34 @@ def _remove_tree_contents(directory_fd: int) -> None: os.unlink(name, dir_fd=directory_fd) +def _descend_allowed(name: str, category: str) -> bool: + """Descend only into DN-owned trees: the job prefix, plus the configured SAB + category dir (case-insensitive) when it isn't ``*``. Sonarr/Radarr/Whisparr + trees sharing the mount are never visited.""" + if name.startswith("droppedneedle"): + return True + return category != "*" and name.lower() == category.lower() + + def _directory_entries(path: Path) -> list[tuple[str, bool, bool]]: - with os.scandir(path) as entries: - result = [ - ( - entry.name, - entry.is_dir(follow_symlinks=False), - entry.is_symlink(), - ) - for entry in entries - ] + try: + with os.scandir(path) as entries: + result = [] + for entry in entries: + try: + result.append( + ( + entry.name, + entry.is_dir(follow_symlinks=False), + entry.is_symlink(), + ) + ) + except (FileNotFoundError, NotADirectoryError): + logger.debug( + "Acquisition cleanup entry vanished mid-scan: %s", + path / entry.name, + ) + except (FileNotFoundError, NotADirectoryError): + logger.debug("Acquisition cleanup directory vanished: %s", path) + return [] return sorted(result, key=lambda value: value[0]) diff --git a/backend/services/native/album_identification_service.py b/backend/services/native/album_identification_service.py index c84abe913..b8ddd591c 100644 --- a/backend/services/native/album_identification_service.py +++ b/backend/services/native/album_identification_service.py @@ -374,10 +374,12 @@ async def run_claimed_job( str(job["local_album_id"]) ) if context is None: - await self._queue.defer( + # The album row is gone or retired: durable catalog state, so fail + # terminally (auditable) instead of deferring until the cap. + await self._queue.fail( job, worker_id, "SUBJECT_NOT_AVAILABLE", now=timestamp ) - return "provider_deferred" + return "attention" raw_tracks: list[dict] = [ row for row in context["tracks"] if row["availability"] == "indexed" ] diff --git a/backend/services/native/file_processor.py b/backend/services/native/file_processor.py index 9eee613b8..d260bc452 100644 --- a/backend/services/native/file_processor.py +++ b/backend/services/native/file_processor.py @@ -25,7 +25,7 @@ from rapidfuzz import fuzz -from core.exceptions import AutomaticManagementHoldError +from core.exceptions import AutomaticManagementHoldError, ConfigurationError from infrastructure.msgspec_fastapi import AppStruct from models.audio import AudioInfo, AudioTag from models.download_manifest import DownloadManifest, ExpectedFile, ExpectedTrack @@ -1472,6 +1472,18 @@ async def _hold_for_review( logger.warning("Could not hold %s for review: %s", source.name, exc) return False + def _root_library_path(self) -> Path: + """The first configured library root, checked at the moment of use. + + Held-retry paths run long after the original import - the roots may have + been cleared since. Unlike a missing held file (terminal), that is + recoverable, so it raises an actionable 400 instead of an IndexError 500.""" + if not self._library_paths: + raise ConfigurationError( + "No library root is configured - restore one in Settings → Library, then try again." + ) + return self._library_paths[0] + async def place_held_management_bundle( self, held_files: list["HeldImport"] ) -> list[Path]: @@ -1528,7 +1540,7 @@ async def place_held_management_bundle( held.artist_mbid or tag.musicbrainz_album_artist_id ), ) - target_path = self._library_paths[0] / self._naming.format_path( + target_path = self._root_library_path() / self._naming.format_path( held.naming_template or "", target_tag, info.file_format ) replacement: dict | None = None @@ -1612,7 +1624,7 @@ async def place_held_file(self, held: "HeldImport") -> Path: musicbrainz_album_artist_id=held.artist_mbid or tag.musicbrainz_album_artist_id, ) - target_path = self._library_paths[0] / self._naming.format_path( + target_path = self._root_library_path() / self._naming.format_path( held.naming_template or "", target_tag, info.file_format ) # D10 confirm-replace: an upgrade's held file (AcoustID disagreed, a human diff --git a/backend/services/native/identification_queue_service.py b/backend/services/native/identification_queue_service.py index 6cf092475..dfe7c99ae 100644 --- a/backend/services/native/identification_queue_service.py +++ b/backend/services/native/identification_queue_service.py @@ -14,6 +14,8 @@ PRIORITY_SUPPORTING_MAINTENANCE = 50 LEASE_SECONDS = 60.0 MAX_BACKOFF_SECONDS = 6 * 60 * 60 +MAX_DEFERRAL_ATTEMPTS = 10 +SUBJECT_NOT_AVAILABLE_GRACE_SECONDS = 24 * 60 * 60 class IdentificationQueueService: @@ -140,6 +142,14 @@ async def defer( ) -> int: timestamp = time.time() if now is None else now attempts = max(1, int(job.get("attempt_count", 1))) + if attempts >= MAX_DEFERRAL_ATTEMPTS: + return await self._store.terminal_fail_identification_job( + str(job["id"]), + worker_id=worker_id, + expected_job_revision=int(job["row_revision"]), + failure_code="MAX_DEFERRALS_EXCEEDED", + now=timestamp, + ) backoff = min(MAX_BACKOFF_SECONDS, 30 * (2 ** min(attempts - 1, 10))) return await self._store.defer_identification_job( str(job["id"]), @@ -150,6 +160,28 @@ async def defer( now=timestamp, ) + async def fail( + self, + job: dict, + worker_id: str, + failure_code: str, + *, + now: float | None = None, + ) -> int: + """Terminally fail a running job, keeping the row for auditability.""" + return await self._store.terminal_fail_identification_job( + str(job["id"]), + worker_id=worker_id, + expected_job_revision=int(job["row_revision"]), + failure_code=failure_code, + now=time.time() if now is None else now, + ) + + async def reset_provider_deferrals(self, *, now: float | None = None) -> int: + return await self._store.reset_provider_identification_deferrals( + now=time.time() if now is None else now + ) + async def pause( self, requested_by_user_id: str | None, @@ -191,9 +223,14 @@ async def is_paused(self) -> bool: return (await self._store.get_identification_control())["state"] == "paused" async def recover(self, *, now: float | None = None) -> int: - return await self._store.recover_expired_identification_leases( - now=time.time() if now is None else now + timestamp = time.time() if now is None else now + recovered = await self._store.recover_expired_identification_leases( + now=timestamp + ) + await self._store.gc_stale_identification_jobs( + now=timestamp, grace_seconds=SUBJECT_NOT_AVAILABLE_GRACE_SECONDS ) + return recovered async def activity_snapshot(self) -> dict: return await self._store.get_identification_activity_snapshot() diff --git a/backend/services/native/identity_repair_service.py b/backend/services/native/identity_repair_service.py index 53173c1ef..53b380c1f 100644 --- a/backend/services/native/identity_repair_service.py +++ b/backend/services/native/identity_repair_service.py @@ -19,6 +19,7 @@ ) from core.exceptions import ExternalServiceError, ResourceNotFoundError, ValidationError from infrastructure.queue.priority_queue import RequestPriority +from infrastructure.resilience.retry import CircuitOpenError from infrastructure.persistence.native_library_store import NativeLibraryStore from models.identification import ( AlbumCandidate, @@ -58,6 +59,15 @@ MANAGEMENT_READINESS_PURPOSE = "management_readiness" MANAGEMENT_MAPPING_VERSION = "management-edition-readiness-v3" +# MusicBrainz breaker timeout is 60 s; this 2x window (matching the artist +# reconciliation service) gives the breaker a recovery window between attempts. +_PROVIDER_DEFERRED_RETRY_SECONDS = 120.0 + + +class _ProviderUnavailable(Exception): + """Control-flow: the identity provider is unavailable, so the audit defers + the whole job instead of writing 'unverifiable' findings during the outage.""" + class IdentityRepairService: def __init__( @@ -66,11 +76,13 @@ def __init__( provider: IdentificationProviderProtocol | None = None, evidence: AlbumEvidenceEngine | None = None, canonical_provider: CanonicalMusicBrainzRepositoryProtocol | None = None, + provider_available: Callable[[], bool] | None = None, ) -> None: self._store = store self._provider = provider self._evidence = evidence or AlbumEvidenceEngine() self._canonical_provider = canonical_provider + self._provider_available = provider_available self._operations = LibraryOperationService(store) async def create( @@ -207,6 +219,7 @@ async def run_claimed_audit( *, now: float | None = None, checkpoint: Callable[[], Awaitable[None]] | None = None, + provider_available: Callable[[], bool] | None = None, ) -> OperationResponse: snapshot = await self._store.get_operation_snapshot(str(job["id"])) scope = ( @@ -219,8 +232,13 @@ async def run_claimed_audit( if scope else "existing_matches" ) + availability = ( + self._provider_available if provider_available is None else provider_available + ) while True: timestamp = time.time() if now is None else now + if availability is not None and not availability(): + return await self._defer_audit(str(job["id"]), worker_id, timestamp) controlled = await self._store.checkpoint_operation_control( str(job["id"]), worker_id, now=timestamp ) @@ -245,13 +263,21 @@ async def run_claimed_audit( ) if not renewed: raise ResourceNotFoundError("The identity check lease changed.") - if purpose == MANAGEMENT_READINESS_PURPOSE: - finding, attempt, evidence = await self._classify_management_readiness( - str(job["id"]), work, context, timestamp - ) - else: - finding, attempt, evidence = await self._classify( - str(job["id"]), work, context + try: + if purpose == MANAGEMENT_READINESS_PURPOSE: + finding, attempt, evidence = await self._classify_management_readiness( + str(job["id"]), work, context, timestamp + ) + else: + finding, attempt, evidence = await self._classify( + str(job["id"]), work, context + ) + except _ProviderUnavailable: + return await self._defer_audit( + str(job["id"]), + worker_id, + timestamp, + ordinal=int(work["ordinal"]), ) await self._store.save_repair_finding_for_work( str(job["id"]), @@ -266,6 +292,24 @@ async def run_claimed_audit( if checkpoint is not None: await checkpoint() + async def _defer_audit( + self, + job_id: str, + worker_id: str, + timestamp: float, + *, + ordinal: int | None = None, + ) -> OperationResponse: + deferred = await self._store.defer_repair_audit_work( + job_id=job_id, + ordinal=ordinal, + worker_id=worker_id, + reason_code="PROVIDER_DEFERRED", + now=timestamp, + retry_not_before=timestamp + _PROVIDER_DEFERRED_RETRY_SECONDS, + ) + return self._operations._response(deferred) + async def _classify_management_readiness( self, job_id: str, @@ -359,19 +403,10 @@ async def _classify_management_readiness( includes=("artist-credits", "recordings", "release-groups"), priority=RequestPriority.BACKGROUND_SYNC, ) - except ExternalServiceError: - return ( - self._finding( - job_id, - work, - "unverifiable", - "PROVIDER_DEFERRED", - False, - identity_revision=int(identity["row_revision"]), - ), - None, - [], - ) + except (ExternalServiceError, CircuitOpenError) as error: + raise _ProviderUnavailable( + "MusicBrainz is unavailable; deferring the identity audit." + ) from error if release is None: return ( self._finding( @@ -415,19 +450,10 @@ async def _classify_management_readiness( tracks, candidate, ) - except ExternalServiceError: - return ( - self._finding( - job_id, - work, - "unverifiable", - "PROVIDER_DEFERRED", - False, - identity_revision=int(identity["row_revision"]), - ), - None, - [], - ) + except (ExternalServiceError, CircuitOpenError) as error: + raise _ProviderUnavailable( + "MusicBrainz is unavailable; deferring the identity audit." + ) from error evaluated = self._evidence.evaluate_candidate(local_tracks, candidate) self._disambiguate_duplicate_recordings( local_tracks, @@ -1011,7 +1037,6 @@ async def _classify( ) attempt: IdentificationAttempt | None = None records: list[IdentificationEvidenceRecord] = [] - provider_deferred = False stored: IdentificationEvidenceRecord | None = None candidate: AlbumCandidate | None = None fingerprint_filled = False @@ -1021,9 +1046,10 @@ async def _classify( str(identity["release_mbid"]), RequestPriority.BACKGROUND_SYNC, ) - except ExternalServiceError: - candidate = None - provider_deferred = True + except (ExternalServiceError, CircuitOpenError) as error: + raise _ProviderUnavailable( + "MusicBrainz is unavailable; deferring the identity audit." + ) from error if candidate is not None: grouping_tracks = [_to_grouping_track(row) for row in tracks] for track, row in zip(grouping_tracks, tracks, strict=True): @@ -1085,9 +1111,7 @@ async def _classify( job_id, work, "unverifiable", - "PROVIDER_DEFERRED" - if provider_deferred - else "EVIDENCE_UNAVAILABLE", + "EVIDENCE_UNAVAILABLE", False, identity_revision=int(identity["row_revision"]), ), diff --git a/backend/services/native/library_inventory_scanner.py b/backend/services/native/library_inventory_scanner.py index 7a732acf0..b4fc0f7d6 100644 --- a/backend/services/native/library_inventory_scanner.py +++ b/backend/services/native/library_inventory_scanner.py @@ -3,6 +3,7 @@ from __future__ import annotations import asyncio +import errno import os import threading import time @@ -14,7 +15,12 @@ import msgspec from infrastructure.persistence.native_library_store import NativeLibraryStore -from models.library_work import ScanInventoryItem, ScanRun, ScanScope +from models.library_work import ( + ScanFailureRecord, + ScanInventoryItem, + ScanRun, + ScanScope, +) from services.local_files_service import AUDIO_EXTENSIONS from services.native.library_filesystem_coordinator import ( LibraryFilesystemCoordinator, @@ -28,6 +34,7 @@ Checkpoint = Callable[[str, str], Awaitable[bool]] DirectoryWalker = Callable[..., Iterator[tuple[str, list[str], list[str]]]] +DirectoryProbe = Callable[[Path], bool] logger = logging.getLogger(__name__) @@ -36,6 +43,25 @@ def _uncoordinated_read() -> Iterator[None]: yield +class _WalkHeartbeat: + """Thread-safe liveness signal written by the walk producer thread.""" + + def __init__(self) -> None: + self._lock = threading.Lock() + self._touched_at = time.monotonic() + self.last_directory = "" + + def touch(self, directory: str = "") -> None: + with self._lock: + self._touched_at = time.monotonic() + if directory: + self.last_directory = directory + + def age(self) -> float: + with self._lock: + return time.monotonic() - self._touched_at + + class LibraryInventoryScanner: def __init__( self, @@ -43,10 +69,73 @@ def __init__( *, directory_walker: DirectoryWalker = os.walk, filesystem_coordinator: LibraryFilesystemCoordinator | None = None, + walk_deadline_seconds: float = 30.0, + directory_probe: DirectoryProbe = Path.is_dir, + max_detached_walkers: int = 4, ) -> None: self._store = store self._directory_walker = directory_walker self._filesystem = filesystem_coordinator + self._walk_deadline_seconds = walk_deadline_seconds + self._directory_probe = directory_probe + self._max_detached_walkers = max_detached_walkers + self._detached_walkers: set[asyncio.Task[None]] = set() + + def _finish_detached_walker(self, task: asyncio.Task[None]) -> None: + self._detached_walkers.discard(task) + if not task.cancelled(): + task.exception() + + def _detach_walker(self, task: asyncio.Task[None]) -> None: + if task.done(): + return + if len(self._detached_walkers) < self._max_detached_walkers: + self._detached_walkers.add(task) + task.add_done_callback(self._finish_detached_walker) + + async def _record_failure( + self, + run_id: str, + scope: ScanScope, + *, + relative_path: str, + failure_code: str, + failure_detail: str, + ) -> None: + await self._store.record_scan_failures( + run_id, + [ + ScanFailureRecord( + root_id=scope.root_id, + relative_path=relative_path, + failure_code=failure_code, + recorded_at=time.time(), + failure_detail=failure_detail, + phase="discovering", + ) + ], + ) + + @staticmethod + def _relativize(path: Path, root: Path) -> str: + try: + return PurePosixPath(*path.relative_to(root).parts).as_posix() + except ValueError: + return path.as_posix() + + @staticmethod + def _failure_relative_path( + exc: BaseException, root: Path, heartbeat: _WalkHeartbeat + ) -> str: + filename = getattr(exc, "filename", None) + candidate = ( + Path(str(filename)) + if filename + else (Path(heartbeat.last_directory) if heartbeat.last_directory else None) + ) + if candidate is None: + return "." + return LibraryInventoryScanner._relativize(candidate, root) async def discover( self, @@ -71,6 +160,13 @@ async def discover( if root is None and scope.root_path is not None: root = Path(scope.root_path) if root is None: + await self._record_failure( + run.id, + scope, + relative_path=scope.relative_path, + failure_code="ROOT_UNAVAILABLE", + failure_detail="The library root has no configured path.", + ) await self._store.complete_scan_scope_discovery( run.id, scope.root_id, @@ -89,8 +185,51 @@ async def discover( selected = ( root if scope.relative_path == "." else root / scope.relative_path ) - exists = await asyncio.to_thread(selected.is_dir) + try: + exists = await asyncio.wait_for( + asyncio.to_thread(self._directory_probe, selected), + timeout=self._walk_deadline_seconds, + ) + except TimeoutError: + logger.warning( + "library_scan event=walk_timeout run_id=%s root_id=%s path=%s", + run.id, + scope.root_id, + scope.relative_path, + ) + await self._record_failure( + run.id, + scope, + relative_path=scope.relative_path, + failure_code="WALK_TIMEOUT", + failure_detail=( + "The library root probe exceeded " + f"{self._walk_deadline_seconds:.1f}s." + ), + ) + await self._store.complete_scan_scope_discovery( + run.id, + scope.root_id, + scope.relative_path, + state="unavailable", + error_code="WALK_TIMEOUT", + ) + return await self._store.transition_scan_run( + run.id, + expected_state=current.state, + expected_revision=current.row_revision, + new_state="failed", + now=current.updated_at, + terminal_code="WALK_TIMEOUT", + ) if not exists: + await self._record_failure( + run.id, + scope, + relative_path=scope.relative_path, + failure_code="ROOT_UNAVAILABLE", + failure_detail=f"The library root path is missing: {selected}", + ) await self._store.complete_scan_scope_discovery( run.id, scope.root_id, @@ -117,7 +256,7 @@ async def discover( if self._filesystem is not None else None ) - current, completed = await self._walk_scope( + current, completed, walk_failure_code = await self._walk_scope( current, scope, root, @@ -146,7 +285,7 @@ async def discover( expected_revision=current.row_revision, new_state="failed", now=current.updated_at, - terminal_code="ROOT_PERMISSION_DENIED", + terminal_code=walk_failure_code or "ROOT_PERMISSION_DENIED", ) return current await self._store.complete_scan_scope_discovery( @@ -167,12 +306,13 @@ async def _walk_scope( resolver: LibraryPolicyResolver, checkpoint: Checkpoint, discovery_generation: int = 1, - ) -> tuple[ScanRun, bool]: + ) -> tuple[ScanRun, bool, str | None]: loop = asyncio.get_running_loop() queue: asyncio.Queue[tuple[Path, os.stat_result] | BaseException | None] = ( asyncio.Queue(maxsize=INVENTORY_QUEUE_SIZE) ) stopped = threading.Event() + heartbeat = _WalkHeartbeat() def producer() -> None: try: @@ -196,6 +336,7 @@ def onerror(error: OSError) -> None: directory, subdirectories, filenames = next(walker) except StopIteration: break + heartbeat.touch(directory) subdirectories[:] = [ name for name in subdirectories @@ -205,6 +346,7 @@ def onerror(error: OSError) -> None: tuple[Path, os.stat_result] | BaseException ] = [] for filename in filenames: + heartbeat.touch(directory) path = Path(directory) / filename if ( path.suffix.casefold() not in AUDIO_EXTENSIONS @@ -232,6 +374,8 @@ def onerror(error: OSError) -> None: current = run completed = True discard_remaining = False + detached = False + walk_failure_code: str | None = None discovered = 0 stale_cleanup_pending = True last_checkpoint = time.monotonic() @@ -241,6 +385,36 @@ def onerror(error: OSError) -> None: try: item = await asyncio.wait_for(queue.get(), timeout=0.25) except TimeoutError: + if heartbeat.age() > self._walk_deadline_seconds: + completed = False + stopped.set() + walk_failure_code = "WALK_TIMEOUT" + logger.warning( + "library_scan event=walk_timeout run_id=%s root_id=%s " + "path=%s", + run.id, + scope.root_id, + heartbeat.last_directory or scope.relative_path, + ) + await self._record_failure( + run.id, + scope, + relative_path=( + self._relativize(Path(heartbeat.last_directory), root) + if heartbeat.last_directory + else scope.relative_path + ), + failure_code="WALK_TIMEOUT", + failure_detail=( + "The directory walk made no progress for " + f"{self._walk_deadline_seconds:.1f}s." + ), + ) + # The producer thread is wedged in a syscall; awaiting it + # would wedge the scan worker, so it is detached instead. + self._detach_walker(producer_task) + detached = True + break if not await checkpoint(run.id, scope.policy_revision): completed = False stopped.set() @@ -255,6 +429,28 @@ def onerror(error: OSError) -> None: completed = False stopped.set() discard_remaining = True + walk_failure_code = "ROOT_PERMISSION_DENIED" + relative_path = self._failure_relative_path(item, root, heartbeat) + failure_code = ( + "WALK_" + errno.errorcode.get(item.errno, "EUNKNOWN") + if isinstance(item, OSError) and item.errno is not None + else "WALK_ERROR" + ) + logger.warning( + "library_scan event=walk_error run_id=%s root_id=%s path=%s " + "error=%s", + run.id, + scope.root_id, + relative_path, + item, + ) + await self._record_failure( + run.id, + scope, + relative_path=relative_path, + failure_code=failure_code, + failure_detail=str(item), + ) continue batch.append(item) if len(batch) >= INVENTORY_BATCH_SIZE: @@ -307,12 +503,17 @@ def onerror(error: OSError) -> None: try: await asyncio.wait_for(queue.get(), timeout=0.1) except TimeoutError: + if heartbeat.age() > self._walk_deadline_seconds: + self._detach_walker(producer_task) + detached = True + break continue - await asyncio.shield(producer_task) + if not detached: + await asyncio.shield(producer_task) raise finally: stopped.set() - if not producer_task.done(): + if not detached and not producer_task.done(): await producer_task if not completed: await self._store.complete_scan_scope_discovery( @@ -320,9 +521,9 @@ def onerror(error: OSError) -> None: scope.root_id, scope.relative_path, state="partially_read", - error_code="ROOT_PERMISSION_DENIED", + error_code=walk_failure_code or "ROOT_PERMISSION_DENIED", ) - return current, completed + return current, completed, walk_failure_code async def _persist_batch( self, diff --git a/backend/services/native/library_policy_resolver.py b/backend/services/native/library_policy_resolver.py index a5da9456f..4fe2afec9 100644 --- a/backend/services/native/library_policy_resolver.py +++ b/backend/services/native/library_policy_resolver.py @@ -181,6 +181,7 @@ def _normalise_and_validate( staging_path=str(staging) if staging is not None else "", naming_template=settings.naming_template, acoustid_api_key=settings.acoustid_api_key, + enabled=settings.enabled, ), warnings, ) diff --git a/backend/services/native/library_policy_service.py b/backend/services/native/library_policy_service.py index f267541b2..0164077a1 100644 --- a/backend/services/native/library_policy_service.py +++ b/backend/services/native/library_policy_service.py @@ -274,6 +274,7 @@ def _settings_response( staging_path=settings.staging_path, naming_template=settings.naming_template, acoustid_api_key=settings.acoustid_api_key, + enabled=settings.enabled, policy_revision=resolver.policy_revision, reconciliation_required=reconciliation_required, reconciliation_state=( diff --git a/backend/services/native/library_scan_coordinator.py b/backend/services/native/library_scan_coordinator.py index 6377793da..198fcecbe 100644 --- a/backend/services/native/library_scan_coordinator.py +++ b/backend/services/native/library_scan_coordinator.py @@ -97,6 +97,11 @@ def _log_progress(self, run: ScanRun, event: str, *, force: bool = False) -> Non ) async def request_run(self, request: ScanRequest) -> ScanRequestResult: + if not self._resolver_getter().settings.enabled: + raise ValidationError( + "The local library is disabled. Enable it in Settings → Library " + "before starting a scan." + ) if not request.scopes: raise ValidationError("Select at least one library scope.") if any( @@ -179,6 +184,8 @@ async def control( ) async def recover(self) -> list[ScanRun]: + if not self._resolver_getter().settings.enabled: + return [] runs = await self._store.recover_scan_runs(now=self._clock()) self._pending_control_run_ids.clear() for run in runs: @@ -257,6 +264,8 @@ async def checkpoint(self, run_id: str, frozen_policy_revision: str) -> bool: return run.state in {"discovering", "indexing", "reconciling"} async def run_once(self, root_paths: dict[str, Path]) -> ScanRun | None: + if not self._resolver_getter().settings.enabled: + return None await self._store.cleanup_terminal_scan_inventory(limit=5_000) run = await self._store.get_resumable_scan_run() newly_claimed = run is None diff --git a/backend/services/native/library_scan_supervisor.py b/backend/services/native/library_scan_supervisor.py index a75bea9e8..3eb86a7b1 100644 --- a/backend/services/native/library_scan_supervisor.py +++ b/backend/services/native/library_scan_supervisor.py @@ -69,7 +69,10 @@ async def supervise_target_scans( ) -> None: wakeups = work_wakeups or DurableWorkWakeups() try: - await coordinator_getter().recover() + # None resolver getter means the scheduler is not wired up; the library + # is treated as enabled so plain supervisor deployments keep working. + if resolver_getter is None or resolver_getter().settings.enabled: + await coordinator_getter().recover() except asyncio.CancelledError: return except Exception: # noqa: BLE001 - startup recovery failure must not kill the supervisor @@ -80,21 +83,25 @@ async def supervise_target_scans( wait_seconds = EMPTY_RECOVERY_INTERVAL_SECONDS try: coordinator = coordinator_getter() + resolver = resolver_getter() if resolver_getter is not None else None + enabled = resolver is None or resolver.settings.enabled if ( - scheduler_getter is not None - and resolver_getter is not None + enabled + and scheduler_getter is not None + and resolver is not None and schedule_settings_getter is not None ): schedule = schedule_settings_getter() await scheduler_getter().tick( coordinator, - resolver_getter(), + resolver, frequency=schedule["frequency"], daily_time=schedule["daily_time"], timezone_name=schedule["timezone_name"], now=now_getter(), ) - processed = await coordinator.run_once(root_paths_getter()) is not None + if enabled: + processed = await coordinator.run_once(root_paths_getter()) is not None except asyncio.CancelledError: break except Exception: # noqa: BLE001 - the lifetime supervisor records and survives run failures diff --git a/backend/services/native/target_application_lifecycle.py b/backend/services/native/target_application_lifecycle.py index 44228c778..6e6d11540 100644 --- a/backend/services/native/target_application_lifecycle.py +++ b/backend/services/native/target_application_lifecycle.py @@ -127,7 +127,11 @@ async def _migrate_global_connections( async def run_target_one_time_migrations( - *, auth_store: Any, preferences: Any, cache_dir: Path + *, + auth_store: Any, + preferences: Any, + cache_dir: Path, + library_enabled: bool = True, ) -> None: """Run target data ratchets without touching the retained legacy catalog.""" @@ -161,23 +165,29 @@ async def run_target_one_time_migrations( ) if release_year_count: logger.info("Backfilled %d accepted release catalog years", release_year_count) - from core.dependencies.service_providers import ( - get_artist_identity_reconciliation_service, - get_catalog_identity_hygiene_service, - ) + if not library_enabled: + logger.info( + "Skipped catalog hygiene and artist reconciliation backfills: " + "the local library is disabled" + ) + else: + from core.dependencies.service_providers import ( + get_artist_identity_reconciliation_service, + get_catalog_identity_hygiene_service, + ) - hygiene_job = await get_catalog_identity_hygiene_service().enqueue_backfill() - logger.info( - "Queued bounded catalog identity hygiene backfill %s", - hygiene_job["id"], - ) - reconciliation_job = ( - await get_artist_identity_reconciliation_service().enqueue_backfill() - ) - logger.info( - "Queued bounded artist identity reconciliation backfill %s", - reconciliation_job["id"], - ) + hygiene_job = await get_catalog_identity_hygiene_service().enqueue_backfill() + logger.info( + "Queued bounded catalog identity hygiene backfill %s", + hygiene_job["id"], + ) + reconciliation_job = ( + await get_artist_identity_reconciliation_service().enqueue_backfill() + ) + logger.info( + "Queued bounded artist identity reconciliation backfill %s", + reconciliation_job["id"], + ) await auth_store.backfill_usernames() await auth_store.migrate_local_provider_to_username() await _migrate_shared_avatar(auth_store, cache_dir) diff --git a/backend/services/native/target_application_runtime.py b/backend/services/native/target_application_runtime.py index 1b2835931..c964d7c50 100644 --- a/backend/services/native/target_application_runtime.py +++ b/backend/services/native/target_application_runtime.py @@ -6,10 +6,12 @@ import logging import os import socket -from collections.abc import Callable +import time +from collections.abc import Awaitable, Callable from core.task_registry import TaskRegistry from infrastructure.queue.durable_work_wakeup import DurableWorkWakeups +from infrastructure.resilience.retry import CircuitState from services.native.album_identification_service import AlbumIdentificationService from services.native.identification_queue_service import IdentificationQueueService from services.native.library_operation_supervisor import LibraryOperationSupervisor @@ -28,6 +30,13 @@ OPERATION_RECOVERY_INTERVAL_SECONDS = 37.0 CONTRIBUTION_RECOVERY_INTERVAL_SECONDS = 43.0 ERROR_RETRY_INTERVAL_SECONDS = 1.0 +PROVIDER_HEALTH_SWEEP_INTERVAL_SECONDS = 60.0 +WATCHDOG_INTERVAL_SECONDS = 30.0 + +IDENTIFICATION_WORKER_TASK_NAME = "target-library-identification-worker" +OPERATION_WORKER_TASK_NAME = "target-library-operation-worker" +CONTRIBUTION_VERIFICATION_WORKER_TASK_NAME = "library-contribution-verification-worker" +TARGET_WORKER_WATCHDOG_TASK_NAME = "target-worker-watchdog" def _worker_id(kind: str) -> str: @@ -53,15 +62,21 @@ async def run_target_identification_worker( worker_id: str | None = None, workload_gate: BackgroundWorkloadGate | None = None, work_wakeups: DurableWorkWakeups | None = None, + provider_state_getter: Callable[[], CircuitState] | None = None, + probe_provider: Callable[[], Awaitable[None]] | None = None, + enabled_getter: Callable[[], bool] | None = None, ) -> None: owner = worker_id or _worker_id("identification") wakeups = work_wakeups or DurableWorkWakeups() + last_provider_sweep_at = 0.0 while True: revision = wakeups.revision("identification") processed = False wait_seconds = IDENTIFICATION_RECOVERY_INTERVAL_SECONDS try: - if workload_gate is None or not workload_gate.scan_active: + if (enabled_getter is None or enabled_getter()) and ( + workload_gate is None or not workload_gate.scan_active + ): queue = queue_getter() await queue.recover() if not await queue.is_paused(): @@ -78,6 +93,24 @@ async def run_target_identification_worker( wait_seconds = ERROR_RETRY_INTERVAL_SECONDS if processed: continue + if provider_state_getter is not None: + now = time.time() + if now - last_provider_sweep_at >= PROVIDER_HEALTH_SWEEP_INTERVAL_SECONDS: + last_provider_sweep_at = now + try: + state = provider_state_getter() + if state is CircuitState.CLOSED: + # Provider recovered: immediately release provider-deferred + # jobs instead of waiting out up to 6h of stale backoff. + await queue_getter().reset_provider_deferrals(now=now) + elif state is CircuitState.HALF_OPEN and probe_provider is not None: + # Without traffic the breaker would sit HALF_OPEN forever; + # one bounded background probe resolves it either way. + await probe_provider() + except asyncio.CancelledError: + break + except Exception: # noqa: BLE001 - a failed probe must not kill the worker + logger.exception("Identification provider health sweep failed") try: await wakeups.wait( "identification", @@ -94,6 +127,7 @@ async def run_target_operation_worker( *, worker_id: str | None = None, work_wakeups: DurableWorkWakeups | None = None, + enabled_getter: Callable[[], bool] | None = None, ) -> None: owner = worker_id or _worker_id("operation") wakeups = work_wakeups or DurableWorkWakeups() @@ -102,11 +136,12 @@ async def run_target_operation_worker( processed = False wait_seconds = OPERATION_RECOVERY_INTERVAL_SECONDS try: - supervisor = supervisor_getter() - await supervisor.recover() - if recovery_getter is not None: - await recovery_getter().recover_once() - processed = await supervisor.run_once(owner) is not None + if enabled_getter is None or enabled_getter(): + supervisor = supervisor_getter() + await supervisor.recover() + if recovery_getter is not None: + await recovery_getter().recover_once() + processed = await supervisor.run_once(owner) is not None except asyncio.CancelledError: break except Exception: # noqa: BLE001 - a durable worker must survive one failed item @@ -159,19 +194,48 @@ async def run_library_contribution_verification_worker( break +async def run_target_worker_watchdog( + starters: dict[str, Callable[[], asyncio.Task[None]]], + *, + interval_seconds: float = WATCHDOG_INTERVAL_SECONDS, +) -> None: + """Restart target workers whose tasks died (crashes auto-unregister them).""" + registry = TaskRegistry.get_instance() + while True: + try: + for name, starter in starters.items(): + if not registry.is_running(name): + logger.warning("Restarting stopped target worker %s", name) + starter() + except asyncio.CancelledError: + break + except Exception: # noqa: BLE001 - the supervisor must survive a failed restart + logger.exception("Target worker watchdog iteration failed") + try: + await asyncio.sleep(interval_seconds) + except asyncio.CancelledError: + break + + def start_target_identification_worker( queue_getter: Callable[[], IdentificationQueueService], service_getter: Callable[[], AlbumIdentificationService], work_wakeups: DurableWorkWakeups, workload_gate: BackgroundWorkloadGate | None = None, + provider_state_getter: Callable[[], CircuitState] | None = None, + probe_provider: Callable[[], Awaitable[None]] | None = None, + enabled_getter: Callable[[], bool] | None = None, ) -> asyncio.Task[None]: - name = "target-library-identification-worker" + name = IDENTIFICATION_WORKER_TASK_NAME task = asyncio.create_task( run_target_identification_worker( queue_getter, service_getter, workload_gate=workload_gate, work_wakeups=work_wakeups, + provider_state_getter=provider_state_getter, + probe_provider=probe_provider, + enabled_getter=enabled_getter, ) ) TaskRegistry.get_instance().register(name, task) @@ -183,11 +247,15 @@ def start_target_operation_worker( supervisor_getter: Callable[[], LibraryOperationSupervisor], work_wakeups: DurableWorkWakeups, recovery_getter: Callable[[], LibraryManagementRecoveryService] | None = None, + enabled_getter: Callable[[], bool] | None = None, ) -> asyncio.Task[None]: - name = "target-library-operation-worker" + name = OPERATION_WORKER_TASK_NAME task = asyncio.create_task( run_target_operation_worker( - supervisor_getter, recovery_getter, work_wakeups=work_wakeups + supervisor_getter, + recovery_getter, + work_wakeups=work_wakeups, + enabled_getter=enabled_getter, ) ) TaskRegistry.get_instance().register(name, task) @@ -199,7 +267,7 @@ def start_library_contribution_verification_worker( worker_getter: Callable[[], LibraryContributionVerificationWorker], work_wakeups: DurableWorkWakeups, ) -> asyncio.Task[None]: - name = "library-contribution-verification-worker" + name = CONTRIBUTION_VERIFICATION_WORKER_TASK_NAME task = asyncio.create_task( run_library_contribution_verification_worker( worker_getter, work_wakeups=work_wakeups @@ -208,3 +276,13 @@ def start_library_contribution_verification_worker( TaskRegistry.get_instance().register(name, task) task.add_done_callback(lambda item: _log_worker_error(item, name=name)) return task + + +def start_target_worker_watchdog( + starters: dict[str, Callable[[], asyncio.Task[None]]], +) -> asyncio.Task[None]: + name = TARGET_WORKER_WATCHDOG_TASK_NAME + task = asyncio.create_task(run_target_worker_watchdog(starters)) + TaskRegistry.get_instance().register(name, task) + task.add_done_callback(lambda item: _log_worker_error(item, name=name)) + return task diff --git a/backend/services/native/target_library_policy_service.py b/backend/services/native/target_library_policy_service.py index 326937911..c6f87f8a4 100644 --- a/backend/services/native/target_library_policy_service.py +++ b/backend/services/native/target_library_policy_service.py @@ -301,6 +301,7 @@ async def restore_roots( staging_path=current.staging_path, naming_template=current.naming_template, acoustid_api_key=current.acoustid_api_key, + enabled=current.enabled, ), expected_policy_revision=request.expected_policy_revision, ) diff --git a/backend/services/navidrome_playback_service.py b/backend/services/navidrome_playback_service.py index 93e195736..f0af1796b 100644 --- a/backend/services/navidrome_playback_service.py +++ b/backend/services/navidrome_playback_service.py @@ -44,16 +44,18 @@ async def _repo_for(self, user_id: str | None) -> NavidromeRepositoryProtocol: def get_stream_url(self, song_id: str) -> str: return self._navidrome.build_stream_url(song_id) - async def proxy_head(self, item_id: str) -> Response: + async def proxy_head(self, item_id: str, user_id: str | None = None) -> Response: """Proxy a HEAD request to Navidrome and return a FastAPI Response.""" - result: StreamProxyResult = await self._navidrome.proxy_head_stream(item_id) + repo = await self._repo_for(user_id) + result: StreamProxyResult = await repo.proxy_head_stream(item_id) return Response(status_code=200, headers=result.headers) async def proxy_stream( - self, item_id: str, range_header: str | None = None + self, item_id: str, range_header: str | None = None, user_id: str | None = None ) -> StreamingResponse: """Proxy a GET stream from Navidrome and return a FastAPI StreamingResponse.""" - result: StreamProxyResult = await self._navidrome.proxy_get_stream( + repo = await self._repo_for(user_id) + result: StreamProxyResult = await repo.proxy_get_stream( item_id, range_header=range_header ) return StreamingResponse( diff --git a/backend/services/now_playing_poller.py b/backend/services/now_playing_poller.py index 5dc8fc099..88e794b58 100644 --- a/backend/services/now_playing_poller.py +++ b/backend/services/now_playing_poller.py @@ -147,7 +147,7 @@ async def run_now_playing_presence_loop( plex_service_getter(), ) except asyncio.CancelledError: - raise + break except Exception as e: # noqa: BLE001 logger.warning("now-playing presence loop cycle failed: %s", e) await asyncio.sleep(interval) diff --git a/backend/services/playlist_service.py b/backend/services/playlist_service.py index 77e594166..1c77ee0a5 100644 --- a/backend/services/playlist_service.py +++ b/backend/services/playlist_service.py @@ -801,9 +801,19 @@ async def _resolve_album_sources( nd_by_num: dict[tuple[int, int], tuple[str, str]] = {} plex_by_num: dict[tuple[int, int], tuple[str, str, str]] = {} + # Pre-fix Jellyfin imports stored the Jellyfin album GUID as album_id, + # which the MBID-keyed Jellyfin/local lookups can never match. Re-key it + # via the album's provider ids so legacy rows resolve without a + # migration. The cache key above stays the original album_id. + match_album_id = album_id if jf_service is not None: try: match = await jf_service.match_album_by_mbid(album_id) + if not match.found: + mbid = await jf_service.resolve_album_mbid(album_id) + if isinstance(mbid, str) and mbid and mbid != album_id: + match_album_id = mbid + match = await jf_service.match_album_by_mbid(mbid) if match.found: for t in match.tracks: key = _safe_track_number(t.track_number) @@ -821,7 +831,7 @@ async def _resolve_album_sources( if local_service is not None: try: - match = await local_service.match_album_by_mbid(album_id) + match = await local_service.match_album_by_mbid(match_album_id) if match.found: for t in match.tracks: key = _safe_track_number(t.track_number) diff --git a/backend/services/plex_playback_service.py b/backend/services/plex_playback_service.py index 32bdbf803..0ec2e6b3a 100644 --- a/backend/services/plex_playback_service.py +++ b/backend/services/plex_playback_service.py @@ -36,14 +36,16 @@ async def _repo_for(self, user_id: str | None) -> PlexRepositoryProtocol: return per_user return self._plex - async def proxy_head(self, part_key: str) -> Response: - result: StreamProxyResult = await self._plex.proxy_head_stream(part_key) + async def proxy_head(self, part_key: str, user_id: str | None = None) -> Response: + repo = await self._repo_for(user_id) + result: StreamProxyResult = await repo.proxy_head_stream(part_key) return Response(status_code=result.status_code, headers=result.headers) async def proxy_stream( - self, part_key: str, range_header: str | None = None + self, part_key: str, range_header: str | None = None, user_id: str | None = None ) -> StreamingResponse: - result: StreamProxyResult = await self._plex.proxy_get_stream( + repo = await self._repo_for(user_id) + result: StreamProxyResult = await repo.proxy_get_stream( part_key, range_header=range_header ) return StreamingResponse( diff --git a/backend/services/preferences_service.py b/backend/services/preferences_service.py index a8f186dbc..ee3f45efb 100644 --- a/backend/services/preferences_service.py +++ b/backend/services/preferences_service.py @@ -1237,6 +1237,7 @@ def get_typed_library_settings(self) -> TypedLibrarySettings: staging_path=settings.staging_path, naming_template=settings.naming_template, acoustid_api_key=ACOUSTID_KEY_MASK if settings.acoustid_api_key else "", + enabled=settings.enabled, ) def get_typed_library_settings_raw(self) -> TypedLibrarySettings: @@ -1249,6 +1250,7 @@ def get_typed_library_settings_raw(self) -> TypedLibrarySettings: staging_path=settings.staging_path, naming_template=settings.naming_template, acoustid_api_key=api_key, + enabled=settings.enabled, ) def get_library_settings(self) -> LibrarySettings: @@ -1340,6 +1342,7 @@ def retarget_library_roots_for_upgrade(self, replacements: dict[str, str]) -> No staging_path=current.staging_path, naming_template=current.naming_template, acoustid_api_key=current.acoustid_api_key, + enabled=current.enabled, ), allow_root_path_changes=True, ) @@ -1416,6 +1419,7 @@ def _save_typed_library_settings( naming_template=normalized.naming_template or DEFAULT_NAMING_TEMPLATE, acoustid_api_key=api_key, + enabled=normalized.enabled, ), ) except ConfigurationError: diff --git a/backend/services/settings_service.py b/backend/services/settings_service.py index 21559a53e..7b107066f 100644 --- a/backend/services/settings_service.py +++ b/backend/services/settings_service.py @@ -647,6 +647,7 @@ async def on_musicbrainz_settings_changed( self, settings: MusicBrainzConnectionSettings ) -> None: from repositories.musicbrainz_base import ( + get_mb_api_base, set_mb_api_base, mb_rate_limiter, mb_circuit_breaker, @@ -664,6 +665,19 @@ async def on_musicbrainz_settings_changed( settings.concurrent_searches, _OFFICIAL_MB_CONCURRENT_SEARCHES ) + # Compare against live module state, not stored settings: the route + # saves before calling this handler, so stored == incoming always. + if ( + get_mb_api_base() == settings.api_url + and mb_rate_limiter.rate == settings.rate_limit + and mb_rate_limiter.capacity == settings.concurrent_searches + ): + logger.info( + "MusicBrainz connection settings unchanged; " + "skipping circuit breaker reset and cache clear" + ) + return + set_mb_api_base(settings.api_url) mb_rate_limiter.update_rate(settings.rate_limit) mb_rate_limiter.update_capacity(settings.concurrent_searches) diff --git a/backend/target_application.py b/backend/target_application.py index 7f4b46581..aab226798 100644 --- a/backend/target_application.py +++ b/backend/target_application.py @@ -8,6 +8,7 @@ import logging import os +import asyncio from pathlib import Path from zoneinfo import ZoneInfo, ZoneInfoNotFoundError @@ -89,6 +90,7 @@ get_discovery_batch_service, get_home_service, get_home_charts_service, + get_legacy_pending_migration_service, get_local_files_service, get_navidrome_library_service, get_plex_library_service, @@ -187,7 +189,7 @@ StaleRevisionError, ValidationError, ) -from infrastructure.resilience.retry import CircuitOpenError +from infrastructure.resilience.retry import CircuitOpenError, CircuitState from core.task_registry import TaskRegistry from core.tasks import ( start_cache_cleanup_task, @@ -205,9 +207,14 @@ ) from services.native.library_scan_supervisor import start_target_scan_supervisor from services.native.target_application_runtime import ( + CONTRIBUTION_VERIFICATION_WORKER_TASK_NAME, + IDENTIFICATION_WORKER_TASK_NAME, + OPERATION_WORKER_TASK_NAME, + TARGET_WORKER_WATCHDOG_TASK_NAME, start_library_contribution_verification_worker, start_target_identification_worker, start_target_operation_worker, + start_target_worker_watchdog, ) from services.native.target_application_lifecycle import ( run_target_one_time_migrations, @@ -555,6 +562,7 @@ async def production_target_lifespan(app: FastAPI): auth_store=auth_store, preferences=preferences, cache_dir=settings.cache_dir, + library_enabled=preferences.get_typed_library_settings().enabled, ) logger.info("target_startup.data_ratchets_completed") async with target_startup_progress(settings, "management_recovery"): @@ -603,6 +611,10 @@ def schedule_settings() -> dict[str, str]: } work_wakeups = get_native_library_store().work_wakeups + + def library_enabled() -> bool: + return get_preferences_service().get_typed_library_settings().enabled + start_target_scan_supervisor( get_target_library_scan_coordinator, root_paths, @@ -611,35 +623,78 @@ def schedule_settings() -> dict[str, str]: resolver_getter=get_library_policy_resolver, schedule_settings_getter=schedule_settings, ) - start_target_identification_worker( - get_target_identification_queue, - get_target_album_identification_service, - work_wakeups, - workload_gate=get_background_workload_gate(), - ) - start_target_operation_worker( - get_target_library_operation_supervisor, - work_wakeups, - recovery_getter=get_library_management_recovery_service, - ) - start_library_contribution_verification_worker( - get_library_contribution_verification_worker, - work_wakeups, - ) + + def mb_provider_state() -> CircuitState: + from repositories.musicbrainz_base import mb_circuit_breaker + + return mb_circuit_breaker.state + + async def probe_mb_provider() -> None: + # Thin background-priority probe mirroring verify_musicbrainz; the + # breaker records the outcome, resolving HALF_OPEN without traffic. + from infrastructure.queue.priority_queue import RequestPriority + from repositories.musicbrainz_base import mb_api_get + + await mb_api_get( + "/artist", + params={"query": "test", "limit": 1}, + priority=RequestPriority.BACKGROUND_SYNC, + ) + + def start_identification_worker() -> asyncio.Task[None]: + return start_target_identification_worker( + get_target_identification_queue, + get_target_album_identification_service, + work_wakeups, + workload_gate=get_background_workload_gate(), + provider_state_getter=mb_provider_state, + probe_provider=probe_mb_provider, + enabled_getter=library_enabled, + ) + + def start_operation_worker() -> asyncio.Task[None]: + return start_target_operation_worker( + get_target_library_operation_supervisor, + work_wakeups, + recovery_getter=get_library_management_recovery_service, + enabled_getter=library_enabled, + ) + + def start_contribution_worker() -> asyncio.Task[None]: + return start_library_contribution_verification_worker( + get_library_contribution_verification_worker, + work_wakeups, + ) + + worker_starters = { + IDENTIFICATION_WORKER_TASK_NAME: start_identification_worker, + OPERATION_WORKER_TASK_NAME: start_operation_worker, + CONTRIBUTION_VERIFICATION_WORKER_TASK_NAME: start_contribution_worker, + } + for start_worker in worker_starters.values(): + start_worker() + start_target_worker_watchdog(worker_starters) await start_target_operational_runtime( settings=settings, preferences=preferences, auth_store=auth_store, ) + if library_enabled(): + try: + await get_legacy_pending_migration_service().schedule() + except Exception: # noqa: BLE001 + logger.exception("Legacy pending migration scheduling failed") logger.info("target_startup.operational_runtime_started") logger.info("DroppedNeedle target application started") try: yield finally: - await TaskRegistry.get_instance().cancel_all( - grace_period=settings.shutdown_grace_period - ) + registry = TaskRegistry.get_instance() + # Cancel the watchdog before the snapshot+cancel_all pass so it cannot + # restart a worker mid-shutdown and orphan the task. + await registry.cancel(TARGET_WORKER_WATCHDOG_TASK_NAME) + await registry.cancel_all(grace_period=settings.shutdown_grace_period) await cleanup_app_state( queue_manager_getter=get_target_discover_queue_manager, genre_prewarm_getter=get_target_genre_cover_prewarm_service, diff --git a/backend/tests/infrastructure/test_container_umask.py b/backend/tests/infrastructure/test_container_umask.py index 490fad1a4..c50c00731 100644 --- a/backend/tests/infrastructure/test_container_umask.py +++ b/backend/tests/infrastructure/test_container_umask.py @@ -39,6 +39,56 @@ def test_entrypoint_rejects_invalid_umask_before_startup(value: str) -> None: assert "must be three or four octal digits" in result.stdout +def test_entrypoint_fails_fast_when_app_is_shadowed(tmp_path: Path) -> None: + """A bind mount over /app (code files missing from an existing /app-like + directory) must fail with the actionable message before any other init + step, while a missing directory (outside the container) must not.""" + entrypoint = REPOSITORY_ROOT / "entrypoint.sh" + script = entrypoint.read_text() + + shadowed_root = tmp_path / "app" + shadowed_root.mkdir() + relocated = script.replace("/app", str(shadowed_root)) + script_file = tmp_path / "entrypoint.sh" + script_file.write_text(relocated) + + result = subprocess.run( + ["sh", str(script_file), "true"], + check=False, + capture_output=True, + text=True, + env={"UMASK": "888"}, + ) + + assert result.returncode == 1 + assert "does not contain the DroppedNeedle application code" in result.stdout + assert "Mount data subdirectories only" in result.stdout + + +def test_entrypoint_skips_shadow_check_when_app_missing(tmp_path: Path) -> None: + """Without an /app directory the shadow check is skipped (host/test + environment) and normal init validation runs instead.""" + entrypoint = REPOSITORY_ROOT / "entrypoint.sh" + script = entrypoint.read_text() + missing_root = tmp_path / "no-such-app" + assert not missing_root.exists() + relocated = script.replace("/app", str(missing_root)) + script_file = tmp_path / "entrypoint.sh" + script_file.write_text(relocated) + + result = subprocess.run( + ["sh", str(script_file), "true"], + check=False, + capture_output=True, + text=True, + env={"UMASK": "888"}, + ) + + assert result.returncode == 1 + assert "must be three or four octal digits" in result.stdout + assert "does not contain the DroppedNeedle application code" not in result.stdout + + def test_unraid_template_uses_the_secure_default() -> None: root = ElementTree.parse(REPOSITORY_ROOT / "templates/droppedneedle.xml").getroot() setting = next( diff --git a/backend/tests/infrastructure/test_native_library_store.py b/backend/tests/infrastructure/test_native_library_store.py index 50c639ad2..411c954e9 100644 --- a/backend/tests/infrastructure/test_native_library_store.py +++ b/backend/tests/infrastructure/test_native_library_store.py @@ -33,6 +33,7 @@ OperationWorkItem, RepairFinding, ReviewDecision, + ScanFailureRecord, ScanInventoryItem, ScanRun, ScanScope, @@ -601,6 +602,7 @@ async def test_schema_is_idempotent_and_contains_complete_target_surface( "library_policy_transitions", "library_scan_runs", "library_scan_inventory", + "library_scan_failures", "library_scan_management_candidates", "library_scan_management_staging", "library_scan_grouping_contexts", @@ -657,6 +659,131 @@ def test_scan_management_candidate_schema_upgrade_is_idempotent( } <= indexes +@pytest.mark.asyncio +async def test_scan_failures_round_trip_pagination_and_dedupe( + store: NativeLibraryStore, +) -> None: + await store.create_scan_run( + ScanRun(id="scan-fail", kind="incremental", trigger="manual", queued_at=1) + ) + records = [ + ScanFailureRecord( + root_id="root-1", + relative_path=f"dir-{ordinal}", + failure_code="WALK_EACCES", + recorded_at=100 + ordinal, + failure_detail=f"detail {ordinal}", + phase="discovering", + ) + for ordinal in range(3) + ] + await store.record_scan_failures("scan-fail", records) + await store.record_scan_failures("scan-fail", [records[0]]) + + first_page, cursor = await store.list_scan_run_failures("scan-fail", limit=2) + + assert [item.relative_path for item in first_page] == ["dir-0", "dir-1"] + assert first_page[0].failure_code == "WALK_EACCES" + assert first_page[0].failure_detail == "detail 0" + assert first_page[0].phase == "discovering" + assert first_page[0].recorded_at == 100 + assert cursor is not None + + second_page, next_cursor = await store.list_scan_run_failures( + "scan-fail", limit=2, cursor_rowid=cursor + ) + assert [item.relative_path for item in second_page] == ["dir-2"] + assert next_cursor is None + + +@pytest.mark.asyncio +async def test_scan_failures_cascade_with_their_run( + store: NativeLibraryStore, db_path: Path +) -> None: + await store.create_scan_run( + ScanRun(id="scan-cascade", kind="incremental", trigger="manual", queued_at=1) + ) + await store.record_scan_failures( + "scan-cascade", + [ + ScanFailureRecord( + root_id="root-1", + relative_path="dir", + failure_code="WALK_TIMEOUT", + recorded_at=100, + ) + ], + ) + + with sqlite3.connect(db_path) as connection: + connection.execute("PRAGMA foreign_keys = ON") + connection.execute("DELETE FROM library_scan_runs WHERE id = 'scan-cascade'") + remaining = connection.execute( + "SELECT COUNT(*) FROM library_scan_failures" + ).fetchone()[0] + + assert remaining == 0 + + +@pytest.mark.asyncio +async def test_commit_scan_index_batch_records_failure_rows( + store: NativeLibraryStore, +) -> None: + await store.create_scan_run( + ScanRun(id="scan-index", kind="incremental", trigger="manual", queued_at=1) + ) + + await store.commit_scan_index_batch( + "scan-index", + writes=[], + states={}, + failures=[("root-1", "broken.flac", "TAG_READ_TIMEOUT")], + increments={}, + updated_at=2.0, + ) + + items, next_cursor = await store.list_scan_run_failures("scan-index") + assert next_cursor is None + assert [ + (item.root_id, item.relative_path, item.failure_code, item.phase) + for item in items + ] == [("root-1", "broken.flac", "TAG_READ_TIMEOUT", "indexing")] + assert items[0].recorded_at == 2.0 + + +@pytest.mark.asyncio +async def test_cleanup_terminal_scan_inventory_prunes_failure_rows( + store: NativeLibraryStore, db_path: Path +) -> None: + await store.create_scan_run( + ScanRun(id="scan-prune", kind="incremental", trigger="manual", queued_at=1) + ) + await store.record_scan_failures( + "scan-prune", + [ + ScanFailureRecord( + root_id="root-1", + relative_path="dir", + failure_code="WALK_EACCES", + recorded_at=100, + ) + ], + ) + with sqlite3.connect(db_path) as connection: + connection.execute( + "UPDATE library_scan_runs SET terminal_at = 10, " + "inventory_cleanup_pending = 1, state = 'failed' WHERE id = 'scan-prune'" + ) + + while True: + _run_id, _deleted, done = await store.cleanup_terminal_scan_inventory() + if done: + break + + items, _cursor = await store.list_scan_run_failures("scan-prune") + assert items == [] + + @pytest.mark.asyncio async def test_scan_management_candidates_follow_album_and_run_lifetimes( store: NativeLibraryStore, db_path: Path @@ -1354,6 +1481,66 @@ async def test_revision_failures_have_specific_safe_api_codes() -> None: assert b"secret path and counter" not in overflow.body +@pytest.mark.asyncio +async def test_identification_snapshot_counts_attention_and_deferral_reasons( + store: NativeLibraryStore, db_path: Path +) -> None: + subjects = { + "job-provider-queued": "album-1", + "job-provider-paused": "album-2", + "job-subject-queued": "album-3", + "job-attention-cap": "album-4", + "job-attention-subject": "album-5", + "job-failed-other": "album-6", + } + for suffix in ("1", "2", "3", "4", "5", "6"): + await store.create_catalog_membership(_membership(suffix)) + for job_id, album_id in subjects.items(): + await store.enqueue_identification_job( + IdentificationJob( + id=job_id, + dedupe_key=f"automatic:{album_id}:rev", + local_album_id=album_id, + created_at=10, + ) + ) + with sqlite3.connect(db_path) as connection: + connection.execute( + "UPDATE library_identification_jobs SET last_failure_code = " + "'PROVIDER_TEMPORARILY_UNAVAILABLE' WHERE id IN " + "('job-provider-queued', 'job-provider-paused')" + ) + connection.execute( + "UPDATE library_identification_jobs SET state = 'paused' " + "WHERE id = 'job-provider-paused'" + ) + connection.execute( + "UPDATE library_identification_jobs SET last_failure_code = " + "'SUBJECT_NOT_AVAILABLE' WHERE id = 'job-subject-queued'" + ) + connection.execute( + "UPDATE library_identification_jobs SET state = 'failed', terminal_at = 12, " + "last_failure_code = 'MAX_DEFERRALS_EXCEEDED' WHERE id = 'job-attention-cap'" + ) + connection.execute( + "UPDATE library_identification_jobs SET state = 'failed', terminal_at = 13, " + "last_failure_code = 'SUBJECT_NOT_AVAILABLE' " + "WHERE id = 'job-attention-subject'" + ) + connection.execute( + "UPDATE library_identification_jobs SET state = 'failed', terminal_at = 14, " + "last_failure_code = 'UNRELATED_CODE' WHERE id = 'job-failed-other'" + ) + + snapshot = await store.get_identification_activity_snapshot() + assert snapshot["attention_count"] == 2 + assert snapshot["deferred_reason_counts"] == { + "PROVIDER_TEMPORARILY_UNAVAILABLE": 2, + "SUBJECT_NOT_AVAILABLE": 1, + } + assert snapshot["deferred_count"] == 3 + + @pytest.mark.asyncio async def test_identification_activity_snapshot_and_revisioned_controls( store: NativeLibraryStore, db_path: Path diff --git a/backend/tests/infrastructure/test_target_scan_lifecycle.py b/backend/tests/infrastructure/test_target_scan_lifecycle.py index 05f8f458c..a74adc44c 100644 --- a/backend/tests/infrastructure/test_target_scan_lifecycle.py +++ b/backend/tests/infrastructure/test_target_scan_lifecycle.py @@ -1,9 +1,11 @@ from __future__ import annotations import asyncio +import errno import os import sqlite3 import threading +import time from collections.abc import Awaitable, Callable from datetime import datetime from pathlib import Path @@ -110,14 +112,16 @@ def _coordinator( *, tag_read_timeout_seconds: float = 30.0, max_detached_tag_reads: int = 4, + walk_deadline_seconds: float = 30.0, on_indexed_album: Callable[[str], Awaitable[object]] | None = None, clock: Callable[[], float] = lambda: 1_800_000_000.0, ) -> LibraryScanCoordinator: reader = tag_reader or _TagReader() - scanner = ( - LibraryInventoryScanner(store) - if directory_walker is None - else LibraryInventoryScanner(store, directory_walker=directory_walker) + walker_kwargs: dict[str, DirectoryWalker] = {} + if directory_walker is not None: + walker_kwargs["directory_walker"] = directory_walker + scanner = LibraryInventoryScanner( + store, walk_deadline_seconds=walk_deadline_seconds, **walker_kwargs ) return LibraryScanCoordinator( store, @@ -701,6 +705,80 @@ def counted_walk(*args, **kwargs): ] +@pytest.mark.asyncio +async def test_walk_permission_error_fails_run_and_records_failed_path( + target_store: NativeLibraryStore, tmp_path: Path +) -> None: + root = tmp_path / "music" + root.mkdir() + resolver = _resolver(root) + + def denied_walk(*_args, **_kwargs): + raise PermissionError(errno.EACCES, "Permission denied", str(root / "secret")) + yield + + coordinator = _coordinator(target_store, resolver, directory_walker=denied_walk) + await coordinator.request_run(_request(resolver)) + failed = await coordinator.run_once({"root-a": root}) + + assert failed is not None and failed.state == "failed" + assert failed.terminal_code == "ROOT_PERMISSION_DENIED" + failures, next_cursor = await target_store.list_scan_run_failures(failed.id) + assert next_cursor is None + assert [ + (failure.failure_code, failure.relative_path, failure.phase) + for failure in failures + ] == [("WALK_EACCES", "secret", "discovering")] + + +@pytest.mark.asyncio +async def test_wedged_walk_fails_bounded_and_the_next_run_claims( + target_store: NativeLibraryStore, tmp_path: Path +) -> None: + root = tmp_path / "music" + root.mkdir() + (root / "track-1.flac").write_bytes(b"one") + resolver = _resolver(root) + wedged = threading.Event() + calls = 0 + + def walker(*_args, **_kwargs): + nonlocal calls + calls += 1 + if calls == 1: + yield (str(root), [], ["track-1.flac"]) + wedged.wait() + return + yield from os.walk(str(root), followlinks=False) + + coordinator = _coordinator( + target_store, + resolver, + directory_walker=walker, + walk_deadline_seconds=0.05, + ) + try: + await coordinator.request_run(_request(resolver)) + started = time.monotonic() + failed = await asyncio.wait_for( + coordinator.run_once({"root-a": root}), timeout=10 + ) + elapsed = time.monotonic() - started + finally: + wedged.set() + + assert elapsed < 5.0 + assert failed is not None and failed.state == "failed" + assert failed.terminal_code == "WALK_TIMEOUT" + failures, _cursor = await target_store.list_scan_run_failures(failed.id) + assert [failure.failure_code for failure in failures] == ["WALK_TIMEOUT"] + assert await coordinator.run_once({"root-a": root}) is None + + await coordinator.request_run(_request(resolver, trigger="automatic")) + completed = await coordinator.run_once({"root-a": root}) + assert completed is not None and completed.state == "completed" + + @pytest.mark.asyncio async def test_queued_scan_is_completed_before_staged_management_callbacks( target_store: NativeLibraryStore, tmp_path: Path diff --git a/backend/tests/routes/test_downloads_routes.py b/backend/tests/routes/test_downloads_routes.py index 3028360d3..ef9e7b5ec 100644 --- a/backend/tests/routes/test_downloads_routes.py +++ b/backend/tests/routes/test_downloads_routes.py @@ -10,6 +10,7 @@ from core.dependencies import get_download_service from core.exceptions import ( ConflictError, + ConfigurationError, PermissionDeniedError, ResourceNotFoundError, ValidationError, @@ -538,6 +539,21 @@ def test_discard_management_hold_returns_album_level_result(): service.discard_management_hold.assert_awaited_once_with("t1", "admin-1", "admin") +def test_import_held_without_library_root_is_400_configuration_error(): + """Empty roots at 'Import anyway' time surfaces as an actionable 400, not a 500.""" + service = AsyncMock() + service.import_held.side_effect = ConfigurationError( + "No library root is configured - restore one in Settings → Library, then try again." + ) + + response = build_test_client(_app(service)).post("/downloads/held/1/import") + + assert response.status_code == 400 + body = response.json()["error"] + assert body["code"] == "CONFIGURATION_ERROR" + assert "library root" in body["message"] + + def test_held_audio_streams_file_and_supports_range(tmp_path): f = tmp_path / "held.flac" f.write_bytes(b"FLACDATA-0123456789") diff --git a/backend/tests/routes/test_library_policies_target.py b/backend/tests/routes/test_library_policies_target.py new file mode 100644 index 000000000..4ce416509 --- /dev/null +++ b/backend/tests/routes/test_library_policies_target.py @@ -0,0 +1,126 @@ +from unittest.mock import AsyncMock + +import pytest +from fastapi import FastAPI + +from api.v1.routes.library_policies_target import router +from api.v1.schemas.library_policies import LibrarySettingsResponse +from core.dependencies import get_legacy_pending_migration_service +from core.dependencies.service_providers import get_target_library_policy_service +from core.exceptions import StaleRevisionError +from tests.helpers import build_test_client, override_admin_auth + + +@pytest.fixture +def app() -> tuple[FastAPI, AsyncMock, AsyncMock]: + application = FastAPI() + application.include_router(router) + target = AsyncMock() + target.get_settings.return_value = LibrarySettingsResponse( + policy_revision="policy-2", + reconciliation_required=True, + reconciliation_state="awaiting_reconciliation", + pending_policy_revision="policy-2", + affected_scope_ids=["root"], + ) + target.save_settings.return_value = target.get_settings.return_value + target.restore_roots.return_value = target.get_settings.return_value + pending_migration = AsyncMock() + pending_migration.schedule.return_value = False + application.dependency_overrides[get_target_library_policy_service] = ( + lambda: target + ) + application.dependency_overrides[get_legacy_pending_migration_service] = ( + lambda: pending_migration + ) + override_admin_auth(application) + return application, target, pending_migration + + +def test_update_library_settings_schedules_pending_migration( + app: tuple[FastAPI, AsyncMock, AsyncMock], +) -> None: + application, _, pending_migration = app + client = build_test_client(application) + response = client.put( + "/settings/library", + json={ + "settings": {"library_roots": []}, + "expected_policy_revision": "policy-2", + }, + ) + assert response.status_code == 200 + pending_migration.schedule.assert_awaited_once() + + +def test_restore_roots_schedules_pending_migration( + app: tuple[FastAPI, AsyncMock, AsyncMock], +) -> None: + application, _, pending_migration = app + client = build_test_client(application) + response = client.post( + "/settings/library/restore-roots", + json={ + "expected_policy_revision": "policy-2", + "paths": {"root": "/music"}, + }, + ) + assert response.status_code == 200 + pending_migration.schedule.assert_awaited_once() + + +def test_update_library_settings_does_not_schedule_when_save_is_stale( + app: tuple[FastAPI, AsyncMock, AsyncMock], +) -> None: + application, target, pending_migration = app + target.save_settings.side_effect = StaleRevisionError("stale policy revision") + client = build_test_client(application) + response = client.put( + "/settings/library", + json={ + "settings": {"library_roots": []}, + "expected_policy_revision": "policy-1", + }, + ) + assert response.status_code == 409 + pending_migration.schedule.assert_not_awaited() + + +def test_update_library_settings_skips_schedule_when_library_disabled( + app: tuple[FastAPI, AsyncMock, AsyncMock], +) -> None: + application, target, pending_migration = app + target.save_settings.return_value = LibrarySettingsResponse( + policy_revision="policy-2", enabled=False + ) + client = build_test_client(application) + response = client.put( + "/settings/library", + json={ + "settings": {"library_roots": [], "enabled": False}, + "expected_policy_revision": "policy-2", + }, + ) + assert response.status_code == 200 + assert response.json()["enabled"] is False + pending_migration.schedule.assert_not_awaited() + + +def test_restore_roots_skips_schedule_when_library_disabled( + app: tuple[FastAPI, AsyncMock, AsyncMock], +) -> None: + application, target, pending_migration = app + target.restore_roots.return_value = LibrarySettingsResponse( + policy_revision="policy-2", enabled=False + ) + client = build_test_client(application) + response = client.post( + "/settings/library/restore-roots", + json={ + "expected_policy_revision": "policy-2", + "paths": {"root": "/music"}, + }, + ) + assert response.status_code == 200 + assert response.json()["enabled"] is False + pending_migration.schedule.assert_not_awaited() diff --git a/backend/tests/routes/test_stream_routes.py b/backend/tests/routes/test_stream_routes.py index dce8d27c5..d934863cd 100644 --- a/backend/tests/routes/test_stream_routes.py +++ b/backend/tests/routes/test_stream_routes.py @@ -7,7 +7,12 @@ from api.v1.routes.stream import router from core.dependencies import get_jellyfin_playback_service -from core.exceptions import ExternalServiceError, PlaybackNotAllowedError, ResourceNotFoundError +from core.exceptions import ( + ExternalServiceError, + JellyfinAuthError, + PlaybackNotAllowedError, + ResourceNotFoundError, +) from tests.helpers import override_user_auth @@ -99,11 +104,38 @@ def test_get_stream_returns_416_on_range_error(client, mock_playback_service): assert response.status_code == 416 +def test_get_stream_passes_current_user_id(client, mock_playback_service): + response = client.get("/stream/jellyfin/item-1") + + assert response.status_code == 200 + mock_playback_service.proxy_stream.assert_awaited_once_with( + "item-1", range_header=None, user_id="test-user-id" + ) + + +def test_get_stream_maps_upstream_auth_error_to_502(client, mock_playback_service): + mock_playback_service.proxy_stream.side_effect = JellyfinAuthError("token revoked") + + response = client.get("/stream/jellyfin/item-auth") + + assert response.status_code == 502 + + def test_head_stream_returns_proxied_headers(client, mock_playback_service): response = client.request("HEAD", "/stream/jellyfin/item-1") assert response.status_code == 200 - mock_playback_service.proxy_head.assert_awaited_once_with("item-1") + mock_playback_service.proxy_head.assert_awaited_once_with( + "item-1", user_id="test-user-id" + ) + + +def test_head_stream_maps_upstream_auth_error_to_502(client, mock_playback_service): + mock_playback_service.proxy_head.side_effect = JellyfinAuthError("token revoked") + + response = client.request("HEAD", "/stream/jellyfin/item-auth") + + assert response.status_code == 502 def test_head_stream_returns_404_when_missing(client, mock_playback_service): diff --git a/backend/tests/routes/test_target_application.py b/backend/tests/routes/test_target_application.py index bc428ae8d..2c629adc9 100644 --- a/backend/tests/routes/test_target_application.py +++ b/backend/tests/routes/test_target_application.py @@ -411,6 +411,7 @@ def starter_calls(path: Path, functions: set[str]) -> set[str]: "start_target_scan_supervisor", "start_target_identification_worker", "start_target_operation_worker", + "start_target_worker_watchdog", } <= target @@ -433,13 +434,18 @@ def test_target_lifecycle_events_sweep_uses_target_catalog_authority() -> None: @pytest.mark.parametrize( - ("admission_token", "expected_phase"), - [(None, "steady_state"), ("a" * 32, "admission")], + ("admission_token", "expected_phase", "library_enabled"), + [ + (None, "steady_state", True), + ("a" * 32, "admission", True), + (None, "steady_state", False), + ], ) def test_production_target_lifespan_selects_validation_phase_and_runs_runtime( monkeypatch: pytest.MonkeyPatch, admission_token: str | None, expected_phase: str, + library_enabled: bool, ) -> None: import target_application as target_module from core.dependencies import auth_providers @@ -463,7 +469,9 @@ def test_production_target_lifespan_selects_validation_phase_and_runs_runtime( memory_cache_cleanup_interval=60, disk_cache_cleanup_interval=60, ), - get_typed_library_settings=lambda: SimpleNamespace(library_roots=[]), + get_typed_library_settings=lambda: SimpleNamespace( + library_roots=[], enabled=library_enabled + ), get_library_scan_schedule=lambda: SimpleNamespace( scan_frequency="manual", daily_scan_time="03:00" ), @@ -539,21 +547,49 @@ def test_production_target_lifespan_selects_validation_phase_and_runs_runtime( "start_target_scan_supervisor", lambda *args, **kwargs: scan_supervisor_arguments.update(kwargs), ) + identification_worker_arguments: dict[str, object] = {} monkeypatch.setattr( - target_module, "start_target_identification_worker", lambda *a, **k: None + target_module, + "start_target_identification_worker", + lambda *a, **k: identification_worker_arguments.update(k), ) + operation_worker_arguments: dict[str, object] = {} monkeypatch.setattr( - target_module, "start_target_operation_worker", lambda *a, **k: None + target_module, + "start_target_operation_worker", + lambda *a, **k: operation_worker_arguments.update(k), ) monkeypatch.setattr( target_module, "start_library_contribution_verification_worker", lambda *a, **k: None, ) + watchdog_starters: dict[str, object] = {} + monkeypatch.setattr( + target_module, + "start_target_worker_watchdog", + lambda starters: watchdog_starters.update(starters), + ) + pending_migration = AsyncMock() + pending_migration.schedule.return_value = False + monkeypatch.setattr( + target_module, + "get_legacy_pending_migration_service", + lambda: pending_migration, + ) monkeypatch.setattr(auth_providers, "get_auth_service", lambda: auth) monkeypatch.setattr(auth_providers, "get_auth_store", lambda: auth_store) + registry = target_module.TaskRegistry.get_instance() + shutdown_order: list[str] = [] monkeypatch.setattr( - target_module.TaskRegistry.get_instance(), "cancel_all", AsyncMock() + registry, + "cancel", + AsyncMock(side_effect=lambda *a, **k: shutdown_order.append("cancel")), + ) + monkeypatch.setattr( + registry, + "cancel_all", + AsyncMock(side_effect=lambda *a, **k: shutdown_order.append("cancel_all")), ) monkeypatch.setenv("TZ", "Europe/London") if admission_token is None: @@ -567,7 +603,12 @@ def test_production_target_lifespan_selects_validation_phase_and_runs_runtime( validate.assert_awaited_once_with(expected_phase) admission.assert_awaited_once() - migrate.assert_awaited_once() + migrate.assert_awaited_once_with( + auth_store=auth_store, + preferences=preferences, + cache_dir=target_module.get_settings().cache_dir, + library_enabled=library_enabled, + ) operation_supervisor.recover.assert_awaited_once() recovery_service.recover_startup.assert_awaited_once() operational.assert_awaited_once_with( @@ -575,11 +616,29 @@ def test_production_target_lifespan_selects_validation_phase_and_runs_runtime( preferences=preferences, auth_store=auth_store, ) + identification_enabled_getter = identification_worker_arguments["enabled_getter"] + operation_enabled_getter = operation_worker_arguments["enabled_getter"] + assert callable(identification_enabled_getter) + assert callable(operation_enabled_getter) + assert identification_enabled_getter() is library_enabled + assert operation_enabled_getter() is library_enabled + if library_enabled: + pending_migration.schedule.assert_awaited_once() + else: + pending_migration.schedule.assert_not_awaited() schedule_settings_getter = scan_supervisor_arguments["schedule_settings_getter"] assert callable(schedule_settings_getter) assert schedule_settings_getter()["timezone_name"] == "Europe/London" assert schedule_settings_getter()["timezone_name"] == "Europe/London" timezone_name.assert_called_once_with() + assert set(watchdog_starters) == { + "target-library-identification-worker", + "target-library-operation-worker", + "library-contribution-verification-worker", + } + assert all(callable(starter) for starter in watchdog_starters.values()) + registry.cancel.assert_awaited_once_with("target-worker-watchdog") + assert shutdown_order == ["cancel", "cancel_all"] cleanup.assert_awaited_once() assert lifecycle_order == [ "validate", diff --git a/backend/tests/routes/test_target_library_scan_routes.py b/backend/tests/routes/test_target_library_scan_routes.py index 92549edb5..aa9fa3738 100644 --- a/backend/tests/routes/test_target_library_scan_routes.py +++ b/backend/tests/routes/test_target_library_scan_routes.py @@ -1,7 +1,7 @@ from __future__ import annotations from types import SimpleNamespace -from unittest.mock import AsyncMock +from unittest.mock import AsyncMock, MagicMock import pytest from fastapi import FastAPI, HTTPException @@ -15,12 +15,19 @@ from core.dependencies import ( get_library_administrative_work_service, get_library_policy_resolver, + get_mb_provider_availability, + get_native_library_store, get_target_identification_queue, get_target_library_scan_coordinator, ) -from core.exceptions import ResourceNotFoundError, StaleRevisionError +from core.exceptions import ResourceNotFoundError, StaleRevisionError, ValidationError from middleware import _get_current_admin -from models.library_work import LibraryWorkItem, ScanControlResult, ScanRequestResult +from models.library_work import ( + LibraryWorkItem, + ScanControlResult, + ScanFailureRecord, + ScanRequestResult, +) from services.native.library_policy_resolver import LibraryPolicyResolver from services.native.library_activity_events import activity_events from tests.helpers import build_test_client, override_admin_auth, override_user_auth @@ -86,6 +93,8 @@ def identification_queue() -> AsyncMock: "started_at": None, "updated_at": None, "deferred_count": 0, + "deferred_reason_counts": {}, + "attention_count": 0, "failure_event_id": None, "failure_at": None, "foreground_operation_count": 0, @@ -107,11 +116,25 @@ def administrative_work() -> AsyncMock: return service +@pytest.fixture +def native_store() -> AsyncMock: + store = AsyncMock() + store.list_scan_run_failures.return_value = ([], None) + return store + + +@pytest.fixture +def mb_availability() -> MagicMock: + return MagicMock(return_value=True) + + @pytest.fixture def app( coordinator: AsyncMock, identification_queue: AsyncMock, administrative_work: AsyncMock, + native_store: AsyncMock, + mb_availability: MagicMock, resolver: LibraryPolicyResolver, ) -> FastAPI: application = FastAPI() @@ -126,6 +149,10 @@ def app( application.dependency_overrides[get_library_administrative_work_service] = ( lambda: administrative_work ) + application.dependency_overrides[get_native_library_store] = lambda: native_store + application.dependency_overrides[get_mb_provider_availability] = ( + lambda: mb_availability + ) return application @@ -278,6 +305,8 @@ def test_activity_is_authenticated_and_redacted( "started_at": 2.0, "updated_at": 11.0, "deferred_count": 1, + "deferred_reason_counts": {"PROVIDER_TEMPORARILY_UNAVAILABLE": 1}, + "attention_count": 2, "kept_local_count": 4, "active_priority": 30, "failure_event_id": "failure-opaque", @@ -301,9 +330,13 @@ def test_activity_is_authenticated_and_redacted( "needs_review_count": 2, "failed_count": 0, "deferred_count": 1, + "deferred_reason_counts": {"PROVIDER_TEMPORARILY_UNAVAILABLE": 1}, + "attention_count": 2, "priority_band": "Administrator retries", "oldest_backlog_at": 2.0, - "provider_unavailable": True, + # Regression: a live breaker read, not bool(deferred_count) - deferrals + # with a healthy provider must not raise a false provider alert. + "provider_unavailable": False, "control_revision": 8, "failure_event_id": "failure-opaque", "failure_at": 9.0, @@ -325,6 +358,37 @@ def test_activity_is_authenticated_and_redacted( assert failure["failure_event_id"] == "failure-opaque" +def test_activity_marks_provider_unavailable_from_live_breaker_read( + app: FastAPI, + identification_queue: AsyncMock, + mb_availability: MagicMock, +) -> None: + identification_queue.activity_snapshot.return_value = { + "control_state": "running", + "control_revision": 1, + "counts": {"queued": 2}, + "started_at": 2.0, + "updated_at": 11.0, + "deferred_count": 2, + "deferred_reason_counts": {"PROVIDER_TEMPORARILY_UNAVAILABLE": 2}, + "attention_count": 0, + "kept_local_count": 0, + "active_priority": None, + "failure_event_id": None, + "failure_at": None, + "foreground_operation_count": 0, + } + mb_availability.return_value = False + override_user_auth(app, role="user") + + payload = build_test_client(app).get("/library/activity").json() + item = next(item for item in payload["items"] if item["kind"] == "identification") + assert item["provider_unavailable"] is True + assert item["deferred_count"] == 2 + assert item["deferred_reason_counts"] == {"PROVIDER_TEMPORARILY_UNAVAILABLE": 2} + assert item["attention_count"] == 0 + + def test_activity_projects_admin_work_and_scan_finalization_truthfully( admin_client, coordinator: AsyncMock, @@ -497,6 +561,98 @@ def test_missing_and_stale_run_use_typed_error_envelopes( assert response.json()["error"]["code"] == "STALE_REVISION" +def test_scan_runs_start_rejects_a_disabled_library_with_the_switch_message( + admin_client, coordinator: AsyncMock, resolver: LibraryPolicyResolver +) -> None: + coordinator.request_run.side_effect = ValidationError( + "The local library is disabled. Enable it in Settings → Library " + "before starting a scan." + ) + response = admin_client.post( + "/library/scan-runs", + json={ + "kind": "incremental", + "scope_ids": ["root-a"], + "expected_policy_revision": resolver.policy_revision, + }, + ) + + assert response.status_code == 400 + assert response.json()["error"]["code"] == "VALIDATION_ERROR" + assert "disabled" in response.json()["error"]["message"] + + +def test_scan_run_failures_returns_snake_case_items( + admin_client, native_store: AsyncMock +) -> None: + native_store.list_scan_run_failures.return_value = ( + [ + ScanFailureRecord( + root_id="root-a", + relative_path="Artist/Album", + failure_code="WALK_EACCES", + recorded_at=1_800_000_001.0, + failure_detail="[Errno 13] Permission denied", + phase="discovering", + ) + ], + 41, + ) + + response = admin_client.get("/library/scan-runs/run-1/failures?cursor=40") + + assert response.status_code == 200 + assert response.json() == { + "items": [ + { + "root_id": "root-a", + "relative_path": "Artist/Album", + "failure_code": "WALK_EACCES", + "failure_detail": "[Errno 13] Permission denied", + "phase": "discovering", + "recorded_at": 1_800_000_001.0, + } + ], + "next_cursor": 41, + } + native_store.list_scan_run_failures.assert_awaited_once_with( + "run-1", limit=50, cursor_rowid=40 + ) + + +def test_scan_run_failures_limit_bounds_are_validated(admin_client) -> None: + assert admin_client.get("/library/scan-runs/run-1/failures?limit=0").status_code == 422 + assert ( + admin_client.get("/library/scan-runs/run-1/failures?limit=201").status_code + == 422 + ) + + +def test_scan_run_failures_auth_matrix(app: FastAPI, native_store: AsyncMock) -> None: + assert ( + build_test_client(app).get("/library/scan-runs/run-1/failures").status_code + == 401 + ) + + def reject_admin(): + raise HTTPException(status_code=403, detail="Admin access required") + + app.dependency_overrides[_get_current_admin] = reject_admin + assert ( + build_test_client(app).get("/library/scan-runs/run-1/failures").status_code + == 403 + ) + + override_admin_auth(app) + override_user_auth(app, role="admin") + native_store.get_scan_run.side_effect = ResourceNotFoundError( + "Scan run not found: missing" + ) + missing = build_test_client(app).get("/library/scan-runs/missing/failures") + assert missing.status_code == 404 + assert missing.json()["error"]["code"] == "NOT_FOUND" + + def test_target_route_security_inventory_is_complete() -> None: paths = { route.path @@ -513,6 +669,7 @@ def test_target_route_security_inventory_is_complete() -> None: "/library/scan-runs/current", "/library/scan-runs/estimate", "/library/scan-runs/{run_id}", + "/library/scan-runs/{run_id}/failures", "/library/scan-runs/{run_id}/pause", "/library/scan-runs/{run_id}/resume", "/library/scan-runs/{run_id}/stop", diff --git a/backend/tests/security/test_auth_on_every_endpoint.py b/backend/tests/security/test_auth_on_every_endpoint.py index 4625f1bb5..477b9d59c 100644 --- a/backend/tests/security/test_auth_on_every_endpoint.py +++ b/backend/tests/security/test_auth_on_every_endpoint.py @@ -82,6 +82,7 @@ get_navidrome_folder_scope_service, get_now_playing_service, get_per_user_client_factory, + get_native_library_store, get_personal_mix_service, get_playlist_service, get_plex_playback_service, @@ -148,6 +149,7 @@ get_navidrome_folder_scope_service, get_now_playing_service, get_per_user_client_factory, + get_native_library_store, get_personal_mix_service, get_playlist_service, get_plex_playback_service, @@ -734,6 +736,7 @@ ("GET", "/api/v1/library/scan-runs", None), ("GET", "/api/v1/library/scan-runs/estimate", None), ("GET", "/api/v1/library/scan-runs/run-1", None), + ("GET", "/api/v1/library/scan-runs/run-1/failures", None), ( "POST", "/api/v1/library/scan-runs/run-1/pause", diff --git a/backend/tests/services/native/test_acquisition_cleanup_service.py b/backend/tests/services/native/test_acquisition_cleanup_service.py index 31a015f2a..6e515bd7f 100644 --- a/backend/tests/services/native/test_acquisition_cleanup_service.py +++ b/backend/tests/services/native/test_acquisition_cleanup_service.py @@ -840,7 +840,11 @@ async def test_legacy_reconciliation_cleans_only_unambiguous_terminal_tasks( ] library.bundles["attention-bundle"] = "needs_attention" service = AcquisitionCleanupService( - store, library, lambda source: client, lambda: root + store, + library, + lambda source: client, + lambda: root, + sab_category_getter=lambda: "audio", ) await service.recover_startup() @@ -921,3 +925,135 @@ async def test_reconciliation_refuses_unsafe_mounts(tmp_path: Path, mount: Path) ) assert await service.reconcile_legacy_mount() == 0 + + +@pytest.mark.asyncio +async def test_reconciliation_survives_directory_removed_between_passes( + tmp_path: Path, +): + root = tmp_path / "sab" + vanished = root / "droppedneedle-staging" + vanished.mkdir(parents=True) + store = _store(tmp_path) + client = _Client( + DownloadMaterialization( + state="missing", mount_root=str(root), mount_healthy=True + ) + ) + service = AcquisitionCleanupService( + store, _LibraryStore(), lambda source: client, lambda: root + ) + + assert await service.reconcile_legacy_mount(limit=1) == 1 + vanished.rmdir() + + await service.reconcile_legacy_mount() + + mount_key = hashlib.sha256(str(root.resolve()).encode()).hexdigest() + progress = await store.ensure_cleanup_reconciliation(mount_key, str(root)) + assert progress.completed is True + + +def test_directory_entries_skips_entry_removed_mid_scan(tmp_path: Path, monkeypatch): + class _VanishingEntry: + def __init__(self, name: str) -> None: + self.name = name + self.path = str(tmp_path / name) + + def is_dir(self, follow_symlinks: bool = False) -> bool: + raise FileNotFoundError(self.name) + + def is_symlink(self) -> bool: + return False + + class _FakeEntries: + def __enter__(self): + return iter([_VanishingEntry("gone")]) + + def __exit__(self, *args: object) -> bool: + return False + + monkeypatch.setattr(cleanup_module.os, "scandir", lambda path: _FakeEntries()) + assert cleanup_module._directory_entries(tmp_path) == [] + + +def test_directory_entries_returns_empty_for_vanished_directory(tmp_path: Path): + assert cleanup_module._directory_entries(tmp_path / "missing") == [] + + +@pytest.mark.asyncio +async def test_reconciliation_default_category_scopes_to_droppedneedle_prefix( + tmp_path: Path, monkeypatch +): + root = tmp_path / "sab" + (root / "droppedneedle-staging").mkdir(parents=True) + foreign = root / "sonarr" + foreign_job = foreign / f"droppedneedle-{'f' * 32}-0" + foreign_job.mkdir(parents=True) + store = _store(tmp_path) + client = _Client( + DownloadMaterialization( + state="missing", mount_root=str(root), mount_healthy=True + ) + ) + service = AcquisitionCleanupService( + store, _LibraryStore(), lambda source: client, lambda: root + ) + visited: list[str] = [] + original_entries = cleanup_module._directory_entries + + def recording_entries(path: Path): + visited.append(str(path)) + return original_entries(path) + + monkeypatch.setattr(cleanup_module, "_directory_entries", recording_entries) + + await service.reconcile_legacy_mount() + + assert str(root) in visited + assert str(root / "droppedneedle-staging") in visited + assert not any(str(foreign) in path for path in visited) + assert ( + await store.get_download_attempt_for_job( + "usenet", f"droppedneedle-{'f' * 32}-0" + ) + is None + ) + + +@pytest.mark.asyncio +async def test_reconciliation_configured_category_descends_only_there( + tmp_path: Path, +): + root = tmp_path / "sab" + category_job = root / "audio" / f"droppedneedle-{'f' * 32}-0" + category_job.mkdir(parents=True) + foreign_job = root / "movies" / f"droppedneedle-{'e' * 32}-0" + foreign_job.mkdir(parents=True) + store = _store(tmp_path) + client = _Client( + DownloadMaterialization( + state="missing", mount_root=str(root), mount_healthy=True + ) + ) + service = AcquisitionCleanupService( + store, + _LibraryStore(), + lambda source: client, + lambda: root, + sab_category_getter=lambda: "Audio", + ) + + await service.reconcile_legacy_mount() + + assert ( + await store.get_download_attempt_for_job( + "usenet", f"droppedneedle-{'f' * 32}-0" + ) + ).state == "needs_attention" + assert ( + await store.get_download_attempt_for_job( + "usenet", f"droppedneedle-{'e' * 32}-0" + ) + is None + ) diff --git a/backend/tests/services/native/test_identification_pipeline.py b/backend/tests/services/native/test_identification_pipeline.py index a1eb298c8..18502093c 100644 --- a/backend/tests/services/native/test_identification_pipeline.py +++ b/backend/tests/services/native/test_identification_pipeline.py @@ -48,7 +48,11 @@ from services.native.identification_evidence_projector import ( IdentificationEvidenceProjector, ) -from services.native.identification_queue_service import IdentificationQueueService +from services.native.identification_queue_service import ( + MAX_BACKOFF_SECONDS, + MAX_DEFERRAL_ATTEMPTS, + IdentificationQueueService, +) from services.native.identification_revisions import ( album_identity_revision, album_input_revisions, @@ -1125,6 +1129,190 @@ async def test_transient_queue_backoff_is_typed_and_respects_not_before( assert await queue.claim("worker", now=40) is not None +@pytest.mark.asyncio +async def test_deferral_cap_terminates_job_in_attention_state( + store: NativeLibraryStore, db_path: Path +) -> None: + await _seed_album(store) + queue = IdentificationQueueService(store) + await queue.enqueue_album("album-1", input_revision="revision", now=1) + now = 2.0 + for attempt in range(1, MAX_DEFERRAL_ATTEMPTS + 1): + claimed = await queue.claim("worker", now=now) + assert claimed is not None + assert claimed["attempt_count"] == attempt + await queue.defer( + claimed, "worker", "PROVIDER_TEMPORARILY_UNAVAILABLE", now=now + ) + now += MAX_BACKOFF_SECONDS + 60 + with sqlite3.connect(db_path) as connection: + row = connection.execute( + "SELECT state, last_failure_code, terminal_at " + "FROM library_identification_jobs" + ).fetchone() + assert row[0] == "failed" + assert row[1] == "MAX_DEFERRALS_EXCEEDED" + assert row[2] == pytest.approx( + 2.0 + (MAX_DEFERRAL_ATTEMPTS - 1) * (MAX_BACKOFF_SECONDS + 60) + ) + assert await queue.claim("worker", now=now) is None + assert (await queue.activity_snapshot())["attention_count"] == 1 + + +@pytest.mark.asyncio +async def test_attention_failed_job_resurrects_on_same_key_enqueue( + store: NativeLibraryStore, db_path: Path +) -> None: + await _seed_album(store) + await _seed_album(store, "2") + queue = IdentificationQueueService(store) + job_id = await queue.enqueue_album("album-1", input_revision="revision", now=1) + claimed = await queue.claim("worker", now=2) + assert claimed is not None + await queue.fail(claimed, "worker", "MAX_DEFERRALS_EXCEEDED", now=3) + + resurrected_id, created = await queue.enqueue_album_with_disposition( + "album-1", input_revision="revision", now=4 + ) + assert resurrected_id == job_id + assert created is True + with sqlite3.connect(db_path) as connection: + row = connection.execute( + "SELECT state, attempt_count, not_before, last_failure_code, terminal_at " + "FROM library_identification_jobs WHERE id = ?", + (job_id,), + ).fetchone() + assert row == ("queued", 0, 0, None, None) + reclaimed = await queue.claim("worker", now=4) + assert reclaimed is not None + assert reclaimed["id"] == job_id + + # A terminal failure without an attention code still dedupes (no resurrection). + await queue.enqueue_album("album-2", input_revision="revision", now=1) + other = await queue.claim("worker", now=5) + assert other is not None + await queue.fail(other, "worker", "UNRELATED_TERMINAL_CODE", now=6) + deduped_id, created = await queue.enqueue_album_with_disposition( + "album-2", input_revision="revision", now=7 + ) + assert deduped_id == str(other["id"]) + assert created is False + with sqlite3.connect(db_path) as connection: + state = connection.execute( + "SELECT state FROM library_identification_jobs WHERE id = ?", + (other["id"],), + ).fetchone()[0] + assert state == "failed" + + +@pytest.mark.asyncio +async def test_deleted_or_retired_subject_terminates_immediately( + store: NativeLibraryStore, db_path: Path +) -> None: + await _seed_album(store) + job = await _claimed_job(store) + provider = FakeProvider() + with sqlite3.connect(db_path) as connection: + connection.execute( + "UPDATE local_albums SET retired_into_album_id = 'album-merged' " + "WHERE id = 'album-1'" + ) + + outcome = await _service( + store, + provider, + FakeFingerprinter(FingerprintResult(status="disabled"), enabled=False), + ).run_claimed_job(job, "worker", now=3) + + assert outcome == "attention" + assert provider.calls == [] + with sqlite3.connect(db_path) as connection: + row = connection.execute( + "SELECT state, last_failure_code, terminal_at " + "FROM library_identification_jobs WHERE id = 'job-album-1'" + ).fetchone() + assert row[0] == "failed" + assert row[1] == "SUBJECT_NOT_AVAILABLE" + assert row[2] == 3 + queue = IdentificationQueueService(store) + assert await queue.claim("worker", now=4) is None + + +@pytest.mark.asyncio +async def test_empty_subject_defers_until_grace_sweep_terminates( + store: NativeLibraryStore, db_path: Path +) -> None: + await _seed_album(store) + job = await _claimed_job(store) + provider = FakeProvider() + with sqlite3.connect(db_path) as connection: + connection.execute( + "UPDATE local_tracks SET availability = 'missing' WHERE id = 'track-1'" + ) + + outcome = await _service( + store, + provider, + FakeFingerprinter(FingerprintResult(status="disabled"), enabled=False), + ).run_claimed_job(job, "worker", now=3) + + assert outcome == "provider_deferred" + queue = IdentificationQueueService(store) + await queue.recover(now=3 + 3600) + with sqlite3.connect(db_path) as connection: + state = connection.execute( + "SELECT state FROM library_identification_jobs WHERE id = 'job-album-1'" + ).fetchone()[0] + assert state == "queued" + + await queue.recover(now=3 + 25 * 3600) + with sqlite3.connect(db_path) as connection: + row = connection.execute( + "SELECT state, last_failure_code, terminal_at " + "FROM library_identification_jobs WHERE id = 'job-album-1'" + ).fetchone() + assert row[0] == "failed" + assert row[1] == "SUBJECT_NOT_AVAILABLE" + assert row[2] == 3 + 25 * 3600 + + +@pytest.mark.asyncio +async def test_reset_provider_deferrals_clears_backoff_and_leaves_other_reasons( + store: NativeLibraryStore, db_path: Path +) -> None: + await _seed_album(store) + await _seed_album(store, "2") + queue = IdentificationQueueService(store) + await queue.enqueue_album("album-1", input_revision="revision", now=1) + await queue.enqueue_album("album-2", input_revision="revision", now=1) + provider_job = await queue.claim("worker", now=2) + assert provider_job is not None + await queue.defer(provider_job, "worker", "PROVIDER_TEMPORARILY_UNAVAILABLE", now=2) + subject_job = await queue.claim("worker", now=3) + assert subject_job is not None + await queue.defer(subject_job, "worker", "SUBJECT_NOT_AVAILABLE", now=3) + assert await queue.claim("worker", now=4) is None + + assert await queue.reset_provider_deferrals(now=100) == 1 + + with sqlite3.connect(db_path) as connection: + provider_row = connection.execute( + "SELECT attempt_count, not_before, last_failure_code " + "FROM library_identification_jobs WHERE id = ?", + (provider_job["id"],), + ).fetchone() + subject_row = connection.execute( + "SELECT attempt_count, not_before, last_failure_code " + "FROM library_identification_jobs WHERE id = ?", + (subject_job["id"],), + ).fetchone() + assert provider_row == (0, 0, None) + assert subject_row == (1, 33, "SUBJECT_NOT_AVAILABLE") + reclaimed = await queue.claim("worker", now=100) + assert reclaimed is not None + assert reclaimed["id"] == provider_job["id"] + + @pytest.mark.asyncio async def test_pause_at_candidate_and_fingerprint_checkpoints_releases_lease_without_attempt_increment( store: NativeLibraryStore, db_path: Path diff --git a/backend/tests/services/native/test_library_policy_resolver.py b/backend/tests/services/native/test_library_policy_resolver.py index 91d8d8bfe..872477bbc 100644 --- a/backend/tests/services/native/test_library_policy_resolver.py +++ b/backend/tests/services/native/test_library_policy_resolver.py @@ -145,3 +145,23 @@ def test_revision_only_tracks_policy_input(tmp_path: Path) -> None: assert first.policy_revision == second.policy_revision assert first.policy_revision != excluded.policy_revision + + +def test_enabled_toggle_does_not_change_policy_revision(tmp_path: Path) -> None: + root = tmp_path / "Music" + root.mkdir() + enabled = LibraryPolicyResolver(_settings(root)) + disabled = _settings(root) + disabled.enabled = False + + assert LibraryPolicyResolver(disabled).policy_revision == enabled.policy_revision + + +def test_normalise_preserves_the_enabled_flag(tmp_path: Path) -> None: + root = tmp_path / "Music" + root.mkdir() + disabled = _settings(root) + disabled.enabled = False + + assert LibraryPolicyResolver(disabled).settings.enabled is False + assert LibraryPolicyResolver(_settings(root)).settings.enabled is True diff --git a/backend/tests/services/native/test_library_review_operations.py b/backend/tests/services/native/test_library_review_operations.py index bcedc0862..0679b4c23 100644 --- a/backend/tests/services/native/test_library_review_operations.py +++ b/backend/tests/services/native/test_library_review_operations.py @@ -33,6 +33,7 @@ ValidationError, ) from infrastructure.persistence.native_library_store import NativeLibraryStore +from infrastructure.resilience.retry import CircuitOpenError from models.audio import FingerprintResult from models.identification import ( AlbumCandidate, @@ -180,6 +181,25 @@ async def get_exact_release_candidate(self, release_mbid, priority): raise ExternalServiceError("private provider failure") +class _OrderedRepairProvider(_RepairProvider): + def __init__(self) -> None: + self.exact_calls: list[str] = [] + + async def get_exact_release_candidate(self, release_mbid, priority): + self.exact_calls.append(release_mbid) + return await super().get_exact_release_candidate(release_mbid, priority) + + +class _CircuitOpenRepairProvider(_IdentificationProvider): + async def get_album_candidate( + self, release_group_mbid, target_track_count, priority + ): + raise CircuitOpenError("MusicBrainz breaker open") + + async def get_exact_release_candidate(self, release_mbid, priority): + raise CircuitOpenError("MusicBrainz breaker open") + + class _CanonicalReleaseProvider: def __init__( self, @@ -4800,7 +4820,7 @@ async def stop_after_first() -> None: @pytest.mark.asyncio -async def test_repair_audit_generates_missing_evidence_and_provider_failure_is_unverifiable( +async def test_repair_audit_generates_missing_evidence_and_defers_whole_job_when_provider_fails( store: NativeLibraryStore, db_path: Path ) -> None: await _seed_album(store, "1") @@ -4859,23 +4879,149 @@ async def test_repair_audit_generates_missing_evidence_and_provider_failure_is_u "worker", now=8, lease_seconds=60, kind="repair" ) assert claimed is not None - await unavailable.run_claimed_audit(claimed, "worker", now=9) - second_findings = await unavailable.findings(second.id) - by_album = {item.local_album_id: item for item in second_findings.items} - assert by_album["album-1"].finding_code == "unverifiable" - assert by_album["album-1"].reason_code == "PROVIDER_DEFERRED" - assert by_album["album-2"].finding_code == "unverifiable" - assert by_album["album-2"].reason_code == "PROVIDER_DEFERRED" - assert by_album["album-2"].apply_eligible is False - filtered = await unavailable.findings(second.id, finding_category="unverifiable") - assert {item.local_album_id for item in filtered.items} == { - "album-1", - "album-2", - } + deferred = await unavailable.run_claimed_audit(claimed, "worker", now=9) + assert deferred.state == "queued" + assert deferred.succeeded_count == 0 + job_row = await store.get_operation_job(second.id) + assert job_row is not None + assert job_row["state"] == "queued" + assert job_row["lease_owner"] is None + assert job_row["next_attempt_at"] == pytest.approx(9 + 120) + with sqlite3.connect(db_path) as connection: + work = connection.execute( + "SELECT local_album_id, state, failure_code FROM library_operation_work " + "WHERE job_id = ? ORDER BY ordinal", + (second.id,), + ).fetchall() + assert work == [ + ("album-1", "pending", "PROVIDER_DEFERRED"), + ("album-2", "pending", None), + ] + assert (await unavailable.findings(second.id)).items == [] + assert ( + await store.claim_operation_job( + "worker", now=100, lease_seconds=60, kind="repair" + ) + is None + ) + reclaimed = await store.claim_operation_job( + "worker", now=129, lease_seconds=60, kind="repair" + ) + assert reclaimed is not None + assert reclaimed["id"] == second.id + assert reclaimed["next_attempt_at"] is None with pytest.raises(ValidationError, match="category is invalid"): await unavailable.findings(second.id, finding_category="not-a-category") +@pytest.mark.asyncio +async def test_repair_audit_resumes_at_the_deferred_item_after_provider_recovery( + store: NativeLibraryStore, db_path: Path +) -> None: + await _seed_album(store, "1", identity_source="legacy_import") + await _seed_album(store, "2", identity_source="legacy_import") + repair = IdentityRepairService( + store, _UnavailableRepairProvider(), AlbumEvidenceEngine() + ) + created = await repair.create( + RepairCreateRequest(idempotency_key="repair-resume"), "admin", now=3 + ) + claimed = await store.claim_operation_job( + "worker", now=4, lease_seconds=60, kind="repair" + ) + assert claimed is not None + deferred = await repair.run_claimed_audit(claimed, "worker", now=5) + assert deferred.state == "queued" + with sqlite3.connect(db_path) as connection: + work = connection.execute( + "SELECT state, failure_code FROM library_operation_work WHERE job_id = ? " + "ORDER BY ordinal", + (created.id,), + ).fetchall() + assert work == [ + ("pending", "PROVIDER_DEFERRED"), + ("pending", None), + ] + + provider = _OrderedRepairProvider() + recovered = IdentityRepairService(store, provider, AlbumEvidenceEngine()) + reclaimed = await store.claim_operation_job( + "worker", now=5 + 120, lease_seconds=60, kind="repair" + ) + assert reclaimed is not None + assert reclaimed["id"] == created.id + ready = await recovered.run_claimed_audit(reclaimed, "worker", now=5 + 121) + assert ready.state == "ready" + assert ready.repair_summary is not None + assert ready.repair_summary.provider_deferred_count == 0 + assert ready.repair_summary.total_identities == 2 + # The deferred item is the first pending on resume: the audit continues + # exactly where the provider failure left it. + assert provider.exact_calls == ["release-1", "release-2"] + + +@pytest.mark.asyncio +async def test_repair_audit_defers_before_any_provider_call_when_breaker_open( + store: NativeLibraryStore, db_path: Path +) -> None: + await _seed_album(store, "1", identity_source="legacy_import") + provider = _OrderedRepairProvider() + repair = IdentityRepairService( + store, + provider, + AlbumEvidenceEngine(), + provider_available=lambda: False, + ) + created = await repair.create( + RepairCreateRequest(idempotency_key="repair-open-breaker"), "admin", now=3 + ) + claimed = await store.claim_operation_job( + "worker", now=4, lease_seconds=60, kind="repair" + ) + assert claimed is not None + deferred = await repair.run_claimed_audit(claimed, "worker", now=5) + assert deferred.state == "queued" + job_row = await store.get_operation_job(created.id) + assert job_row is not None + assert job_row["next_attempt_at"] == pytest.approx(5 + 120) + assert provider.exact_calls == [] + with sqlite3.connect(db_path) as connection: + work = connection.execute( + "SELECT state, failure_code FROM library_operation_work WHERE job_id = ?", + (created.id,), + ).fetchall() + assert work == [("pending", None)] + + +@pytest.mark.asyncio +async def test_repair_audit_defers_when_provider_raises_circuit_open_error( + store: NativeLibraryStore, db_path: Path +) -> None: + await _seed_album(store, "1", identity_source="legacy_import") + repair = IdentityRepairService( + store, _CircuitOpenRepairProvider(), AlbumEvidenceEngine() + ) + created = await repair.create( + RepairCreateRequest(idempotency_key="repair-circuit-open"), "admin", now=3 + ) + claimed = await store.claim_operation_job( + "worker", now=4, lease_seconds=60, kind="repair" + ) + assert claimed is not None + deferred = await repair.run_claimed_audit(claimed, "worker", now=5) + assert deferred.state == "queued" + job_row = await store.get_operation_job(created.id) + assert job_row is not None + assert job_row["next_attempt_at"] == pytest.approx(5 + 120) + with sqlite3.connect(db_path) as connection: + work = connection.execute( + "SELECT state, failure_code FROM library_operation_work WHERE job_id = ?", + (created.id,), + ).fetchall() + assert work == [("pending", "PROVIDER_DEFERRED")] + assert (await repair.findings(created.id)).items == [] + + @pytest.mark.asyncio async def test_repair_reuses_revision_keyed_fingerprint_as_shared_evidence( store: NativeLibraryStore, @@ -5467,17 +5613,21 @@ async def test_management_identity_preparation_defers_a_complete_mapping_when_pr ) assert claimed is not None - ready = await preparation.run_claimed_audit(claimed, "worker", now=5) - finding = ( - await preparation.findings(created.id, finding_category="unverifiable") - ).items[0] + deferred = await preparation.run_claimed_audit(claimed, "worker", now=5) assert provider.calls == ["release-1"] - assert ready.repair_summary is not None - assert ready.repair_summary.ready_album_count == 0 - assert ready.repair_summary.provider_deferred_count == 1 - assert finding.reason_code == "PROVIDER_DEFERRED" - assert finding.apply_eligible is False + assert deferred.state == "queued" + assert deferred.succeeded_count == 0 + job_row = await store.get_operation_job(created.id) + assert job_row is not None + assert job_row["next_attempt_at"] == pytest.approx(5 + 120) + with sqlite3.connect(db_path) as connection: + work = connection.execute( + "SELECT state, failure_code FROM library_operation_work WHERE job_id = ?", + (created.id,), + ).fetchall() + assert work == [("pending", "PROVIDER_DEFERRED")] + assert (await preparation.findings(created.id)).items == [] @pytest.mark.asyncio @@ -5891,10 +6041,8 @@ async def test_management_identity_preparation_defers_recording_redirect_failure "UPDATE local_track_external_identities SET recording_mbid = 'retired-recording' " "WHERE local_track_id = 'track-1-1'" ) - preparation = IdentityRepairService( - store, - canonical_provider=_CanonicalReleaseProvider(recording_unavailable=True), - ) + provider = _CanonicalReleaseProvider(recording_unavailable=True) + preparation = IdentityRepairService(store, canonical_provider=provider) created = await preparation.create_management_preparation( IdentityPreparationCreateRequest(idempotency_key="recording-provider-failure"), "admin", @@ -5904,14 +6052,20 @@ async def test_management_identity_preparation_defers_recording_redirect_failure "worker", now=4, lease_seconds=60, kind="repair" ) assert claimed is not None - await preparation.run_claimed_audit(claimed, "worker", now=5) + deferred = await preparation.run_claimed_audit(claimed, "worker", now=5) - finding = ( - await preparation.findings(created.id, finding_category="unverifiable") - ).items[0] - assert finding.reason_code == "PROVIDER_DEFERRED" - assert finding.evidence_id is None - assert finding.apply_eligible is False + assert provider.recording_calls == ["retired-recording"] + assert deferred.state == "queued" + job_row = await store.get_operation_job(created.id) + assert job_row is not None + assert job_row["next_attempt_at"] == pytest.approx(5 + 120) + with sqlite3.connect(db_path) as connection: + work = connection.execute( + "SELECT state, failure_code FROM library_operation_work WHERE job_id = ?", + (created.id,), + ).fetchall() + assert work == [("pending", "PROVIDER_DEFERRED")] + assert (await preparation.findings(created.id)).items == [] @pytest.mark.asyncio @@ -6125,6 +6279,7 @@ async def test_management_identity_preparation_blocks_missing_conflicting_and_st @pytest.mark.asyncio async def test_management_identity_preparation_defers_provider_failures( store: NativeLibraryStore, + db_path: Path, ) -> None: await _seed_album(store, "1", identity_source="legacy_import") preparation = IdentityRepairService( @@ -6139,14 +6294,19 @@ async def test_management_identity_preparation_defers_provider_failures( "worker", now=4, lease_seconds=60, kind="repair" ) assert claimed is not None - ready = await preparation.run_claimed_audit(claimed, "worker", now=5) - finding = ( - await preparation.findings(created.id, finding_category="unverifiable") - ).items[0] - assert ready.repair_summary is not None - assert ready.repair_summary.provider_deferred_count == 1 - assert finding.reason_code == "PROVIDER_DEFERRED" - assert finding.apply_eligible is False + deferred = await preparation.run_claimed_audit(claimed, "worker", now=5) + assert deferred.state == "queued" + assert deferred.succeeded_count == 0 + job_row = await store.get_operation_job(created.id) + assert job_row is not None + assert job_row["next_attempt_at"] == pytest.approx(5 + 120) + with sqlite3.connect(db_path) as connection: + work = connection.execute( + "SELECT state, failure_code FROM library_operation_work WHERE job_id = ?", + (created.id,), + ).fetchall() + assert work == [("pending", "PROVIDER_DEFERRED")] + assert (await preparation.findings(created.id)).items == [] @pytest.mark.asyncio diff --git a/backend/tests/services/native/test_target_library_policy_service.py b/backend/tests/services/native/test_target_library_policy_service.py index a051fe09f..20028168d 100644 --- a/backend/tests/services/native/test_target_library_policy_service.py +++ b/backend/tests/services/native/test_target_library_policy_service.py @@ -470,6 +470,47 @@ async def test_empty_roots_save_allowed_when_catalog_is_empty() -> None: ) +@pytest.mark.asyncio +async def test_disabled_save_with_roots_succeeds() -> None: + previous = TypedLibrarySettings( + library_roots=[_root("root", "/music", "Music")] + ) + disabled = TypedLibrarySettings( + library_roots=[_root("root", "/music", "Music")], enabled=False + ) + proposed = LibraryPolicyResolver(disabled) + revision = LibraryPolicyResolver(previous).policy_revision + base = Mock() + base.current_settings.return_value = previous + base.current_settings_raw.return_value = previous + base.prepare_change.return_value = (proposed, []) + base.rebase_scopes.return_value = [] + base.collapse_scopes.return_value = [] + base.get_settings.return_value = LibrarySettingsResponse( + library_roots=disabled.library_roots, + policy_revision=proposed.policy_revision, + enabled=False, + ) + store = AsyncMock() + store.get_pending_policy.return_value = None + reconciliation = AsyncMock() + reconciliation.commit_boundary.return_value = {"changed": 0, "cancelled": 0} + service = TargetLibraryPolicyService(base, reconciliation, store) + + response = await service.save_settings( + disabled, expected_policy_revision=revision + ) + assert response.enabled is False + assert response.library_roots[0]["id"] == "root" + assert proposed.settings.enabled is False + base.persist_settings.assert_called_once_with( + proposed.settings, expected_policy_revision=revision + ) + reconciliation.commit_boundary.assert_awaited_once_with( + proposed_policy_revision=proposed.policy_revision + ) + + @pytest.mark.asyncio async def test_restorable_roots_excludes_configured_and_reports_derived_paths() -> None: current = TypedLibrarySettings( diff --git a/backend/tests/services/native/test_target_scan_runtime.py b/backend/tests/services/native/test_target_scan_runtime.py index f4c4993fb..3ef261369 100644 --- a/backend/tests/services/native/test_target_scan_runtime.py +++ b/backend/tests/services/native/test_target_scan_runtime.py @@ -1,6 +1,8 @@ from __future__ import annotations import asyncio +import errno +import logging import threading import time from contextlib import suppress @@ -35,7 +37,9 @@ run_library_contribution_verification_worker, run_target_identification_worker, run_target_operation_worker, + run_target_worker_watchdog, ) +from infrastructure.resilience.retry import CircuitState from services.native.background_workload_gate import BackgroundWorkloadGate @@ -122,7 +126,9 @@ async def test_supervisor_refreshes_scheduler_and_resolver_each_iteration() -> N coordinator = AsyncMock() coordinator.run_once.return_value = None scheduler = AsyncMock() - resolver = SimpleNamespace(policy_revision="one") + resolver = SimpleNamespace( + policy_revision="one", settings=SimpleNamespace(enabled=True) + ) calls = {"scheduler": 0, "resolver": 0, "settings": 0} def scheduler_getter(): @@ -154,10 +160,39 @@ def settings_getter(): settings_getter, ) - assert calls == {"scheduler": 2, "resolver": 2, "settings": 2} + # One extra resolver read comes from the startup recovery gate; the two + # loop iterations then read it once each. + assert calls == {"scheduler": 2, "resolver": 3, "settings": 2} assert scheduler.tick.await_count == 2 +@pytest.mark.asyncio +async def test_supervisor_skips_recover_tick_and_run_when_library_disabled() -> None: + coordinator = AsyncMock() + coordinator.run_once.return_value = None + scheduler = AsyncMock() + resolver = SimpleNamespace( + policy_revision="one", settings=SimpleNamespace(enabled=False) + ) + wakeups = SimpleNamespace( + revision=lambda _kind: 0, + wait=AsyncMock(side_effect=asyncio.CancelledError()), + ) + await supervise_target_scans( + lambda: coordinator, + lambda: {}, + wakeups, + lambda: scheduler, + lambda: resolver, + lambda: {"frequency": "manual", "daily_time": "03:00", "timezone_name": "UTC"}, + ) + + coordinator.recover.assert_not_awaited() + scheduler.tick.assert_not_awaited() + coordinator.run_once.assert_not_awaited() + wakeups.wait.assert_awaited_once() + + @pytest.mark.asyncio async def test_target_identification_worker_recovers_claims_and_survives_iterations() -> ( None @@ -181,6 +216,131 @@ async def test_target_identification_worker_recovers_claims_and_survives_iterati service.run_claimed_job.assert_awaited_once_with({"id": "job-1"}, "test-worker") +def _idle_identification_harness(): + queue = AsyncMock() + queue.is_paused.return_value = False + queue.claim.return_value = None + service = AsyncMock() + wakeups = SimpleNamespace( + revision=lambda _kind: 0, + wait=AsyncMock( + side_effect=[None, asyncio.CancelledError(), asyncio.CancelledError()] + ), + ) + return queue, service, wakeups + + +@pytest.mark.asyncio +async def test_identification_worker_sweeps_provider_deferrals_when_breaker_closed() -> ( + None +): + queue, service, wakeups = _idle_identification_harness() + probe = AsyncMock() + + await run_target_identification_worker( + lambda: queue, + lambda: service, + worker_id="test-worker", + work_wakeups=wakeups, + provider_state_getter=lambda: CircuitState.CLOSED, + probe_provider=probe, + ) + + queue.reset_provider_deferrals.assert_awaited_once() + probe.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_identification_worker_probes_once_per_rate_limit_when_half_open() -> ( + None +): + queue, service, wakeups = _idle_identification_harness() + probe = AsyncMock() + + await run_target_identification_worker( + lambda: queue, + lambda: service, + worker_id="test-worker", + work_wakeups=wakeups, + provider_state_getter=lambda: CircuitState.HALF_OPEN, + probe_provider=probe, + ) + + # Two idle iterations inside the 60s sweep window: exactly one probe. + probe.assert_awaited_once_with() + queue.reset_provider_deferrals.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_identification_worker_skips_provider_sweep_when_breaker_open() -> None: + queue, service, wakeups = _idle_identification_harness() + probe = AsyncMock() + + await run_target_identification_worker( + lambda: queue, + lambda: service, + worker_id="test-worker", + work_wakeups=wakeups, + provider_state_getter=lambda: CircuitState.OPEN, + probe_provider=probe, + ) + + probe.assert_not_awaited() + queue.reset_provider_deferrals.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_worker_watchdog_restarts_dead_worker_only( + monkeypatch: pytest.MonkeyPatch, +) -> None: + registry = TaskRegistry() + monkeypatch.setattr(TaskRegistry, "get_instance", classmethod(lambda cls: registry)) + + async def run_forever() -> None: + await asyncio.Event().wait() + + alive_task = asyncio.get_running_loop().create_task(run_forever()) + registry.register("alive-worker", alive_task) + dead_task = asyncio.get_running_loop().create_task(run_forever()) + registry.register("dead-worker", dead_task) + dead_task.cancel() + with suppress(asyncio.CancelledError): + await dead_task + + restarted: list[asyncio.Task[None]] = [] + + def dead_starter() -> asyncio.Task[None]: + task = asyncio.get_running_loop().create_task(run_forever()) + registry.register("dead-worker", task) + restarted.append(task) + return task + + alive_starter_calls = 0 + + def alive_starter() -> asyncio.Task[None]: + nonlocal alive_starter_calls + alive_starter_calls += 1 + return alive_task + + async def stop_after_first_iteration(_seconds: float) -> None: + raise asyncio.CancelledError + + monkeypatch.setattr(asyncio, "sleep", stop_after_first_iteration) + try: + await run_target_worker_watchdog( + {"dead-worker": dead_starter, "alive-worker": alive_starter} + ) + assert len(restarted) == 1 + assert alive_starter_calls == 0 + assert registry.is_running("dead-worker") + finally: + alive_task.cancel() + for task in restarted: + task.cancel() + with suppress(asyncio.CancelledError): + await asyncio.gather(alive_task, *restarted) + + @pytest.mark.asyncio async def test_identification_worker_starts_no_new_unit_while_scan_is_active() -> None: queue = AsyncMock() @@ -241,6 +401,30 @@ async def activate_scan() -> bool: service.run_claimed_job.assert_not_awaited() +@pytest.mark.asyncio +async def test_identification_worker_skips_claims_when_library_disabled() -> None: + queue = AsyncMock() + queue.is_paused.return_value = False + queue.claim.return_value = None + service = AsyncMock() + wakeups = SimpleNamespace( + revision=lambda _kind: 0, + wait=AsyncMock(side_effect=asyncio.CancelledError()), + ) + await run_target_identification_worker( + lambda: queue, + lambda: service, + worker_id="test-worker", + work_wakeups=wakeups, + enabled_getter=lambda: False, + ) + + queue.recover.assert_not_awaited() + queue.claim.assert_not_awaited() + service.run_claimed_job.assert_not_awaited() + wakeups.wait.assert_awaited_once() + + @pytest.mark.asyncio async def test_target_operation_worker_recovers_and_dispatches_each_iteration() -> None: supervisor = AsyncMock() @@ -263,6 +447,28 @@ async def test_target_operation_worker_recovers_and_dispatches_each_iteration() supervisor.run_once.assert_awaited_with("test-worker") +@pytest.mark.asyncio +async def test_operation_worker_skips_claims_when_library_disabled() -> None: + supervisor = AsyncMock() + recovery = AsyncMock() + wakeups = SimpleNamespace( + revision=lambda _kind: 0, + wait=AsyncMock(side_effect=asyncio.CancelledError()), + ) + await run_target_operation_worker( + lambda: supervisor, + lambda: recovery, + worker_id="test-worker", + work_wakeups=wakeups, + enabled_getter=lambda: False, + ) + + supervisor.recover.assert_not_awaited() + recovery.recover_once.assert_not_awaited() + supervisor.run_once.assert_not_awaited() + wakeups.wait.assert_awaited_once() + + @pytest.mark.asyncio async def test_operation_worker_does_not_reclaim_repairs_during_scan() -> None: store = AsyncMock() @@ -487,6 +693,180 @@ def test_target_compat_module_has_no_legacy_scanner_dependency() -> None: assert "LibraryScanner" not in names +def _scan_run(run_id: str = "run-1") -> ScanRun: + return ScanRun( + id=run_id, + kind="incremental", + trigger="manual", + state="discovering", + phase="discovering", + ) + + +@pytest.mark.asyncio +async def test_walk_oserror_logs_path_and_records_failure_row( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + root = tmp_path / "music" + root.mkdir() + denied = root / "secret" + store = AsyncMock() + store.classify_scan_paths.return_value = {} + store.add_scan_inventory_batch.return_value = (2, 1) + + def denied_walk(*_args, **_kwargs): + raise PermissionError(errno.EACCES, "Permission denied", str(denied)) + yield + + scanner = LibraryInventoryScanner(store, directory_walker=denied_walk) + scope = ScanScope(root_id="root", policy_revision="policy-1") + + with caplog.at_level(logging.WARNING, logger="services.native.library_inventory_scanner"): + _updated, completed, failure_code = await scanner._walk_scope( + _scan_run(), + scope, + root, + root, + SimpleNamespace(resolve=lambda _path: None), + AsyncMock(return_value=True), + ) + + assert completed is False + assert failure_code == "ROOT_PERMISSION_DENIED" + records = store.record_scan_failures.await_args.args[1] + assert [ + (record.failure_code, record.relative_path, record.phase) + for record in records + ] == [("WALK_EACCES", "secret", "discovering")] + store.complete_scan_scope_discovery.assert_awaited_once_with( + "run-1", + "root", + ".", + state="partially_read", + error_code="ROOT_PERMISSION_DENIED", + ) + assert "event=walk_error" in caplog.text + assert "secret" in caplog.text + + +@pytest.mark.asyncio +async def test_wedged_walk_times_out_detaches_producer_and_recovers( + tmp_path: Path, +) -> None: + root = tmp_path / "music" + root.mkdir() + (root / "track.flac").touch() + wedged = threading.Event() + calls = 0 + + def walker(*_args, **_kwargs): + nonlocal calls + calls += 1 + if calls == 1: + yield (str(root), [], ["track.flac"]) + wedged.wait() + return + yield (str(root), [], ["track.flac"]) + + store = AsyncMock() + store.classify_scan_paths.return_value = {"track.flac": ("new", None)} + store.add_scan_inventory_batch.return_value = (2, 1) + scanner = LibraryInventoryScanner( + store, + directory_walker=walker, + walk_deadline_seconds=0.05, + ) + scope = ScanScope(root_id="root", policy_revision="policy-1") + resolver = SimpleNamespace(resolve=lambda _path: None) + checkpoint = AsyncMock(return_value=True) + + started = time.monotonic() + _updated, completed, failure_code = await scanner._walk_scope( + _scan_run(), scope, root, root, resolver, checkpoint + ) + elapsed = time.monotonic() - started + + assert completed is False + assert failure_code == "WALK_TIMEOUT" + assert elapsed < 2.0 + assert len(scanner._detached_walkers) == 1 + records = store.record_scan_failures.await_args.args[1] + assert [record.failure_code for record in records] == ["WALK_TIMEOUT"] + store.complete_scan_scope_discovery.assert_awaited_once_with( + "run-1", + "root", + ".", + state="partially_read", + error_code="WALK_TIMEOUT", + ) + + # Releasing the wedged syscall lets the detached producer finish cleanly. + wedged.set() + deadline = time.monotonic() + 2.0 + while scanner._detached_walkers and time.monotonic() < deadline: + await asyncio.sleep(0.01) + assert not scanner._detached_walkers + + # A subsequent walk on the same scanner is unaffected by the detached one. + _updated, completed, failure_code = await scanner._walk_scope( + _scan_run("run-2"), scope, root, root, resolver, checkpoint + ) + assert completed is True + assert failure_code is None + + +@pytest.mark.asyncio +async def test_root_probe_timeout_fails_the_run_with_walk_timeout( + tmp_path: Path, +) -> None: + root = tmp_path / "music" + root.mkdir() + wedged_probe = threading.Event() + + def probe(_path: Path) -> bool: + wedged_probe.wait() + return True + + store = AsyncMock() + store.get_scan_scope_discovery_state.return_value = "pending" + scanner = LibraryInventoryScanner( + store, + walk_deadline_seconds=0.05, + directory_probe=probe, + ) + scope = ScanScope(root_id="root", policy_revision="policy-1") + + try: + started = time.monotonic() + await asyncio.wait_for( + scanner.discover( + _scan_run(), + [scope], + {"root": root}, + SimpleNamespace(), + AsyncMock(return_value=True), + ), + timeout=2.0, + ) + elapsed = time.monotonic() - started + finally: + wedged_probe.set() + + assert elapsed < 2.0 + records = store.record_scan_failures.await_args.args[1] + assert [record.failure_code for record in records] == ["WALK_TIMEOUT"] + store.complete_scan_scope_discovery.assert_awaited_once_with( + "run-1", + "root", + ".", + state="unavailable", + error_code="WALK_TIMEOUT", + ) + assert ( + store.transition_scan_run.await_args.kwargs["terminal_code"] == "WALK_TIMEOUT" + ) + + @pytest.mark.asyncio async def test_inventory_file_stat_runs_outside_the_event_loop_thread( tmp_path: Path, monkeypatch: pytest.MonkeyPatch @@ -523,7 +903,7 @@ def record_stat(path: Path, *args, **kwargs): ) scope = ScanScope(root_id="root", policy_revision="policy-1") - _updated, completed = await scanner._walk_scope( + _updated, completed, failure_code = await scanner._walk_scope( run, scope, root, @@ -533,6 +913,7 @@ def record_stat(path: Path, *args, **kwargs): ) assert completed is True + assert failure_code is None assert stat_threads assert event_loop_thread not in stat_threads diff --git a/backend/tests/services/test_download_service.py b/backend/tests/services/test_download_service.py index 70ce484be..ebf051e0e 100644 --- a/backend/tests/services/test_download_service.py +++ b/backend/tests/services/test_download_service.py @@ -1096,6 +1096,38 @@ async def test_import_held_places_and_resolves(tmp_path): svc._orchestrator.settle_after_manual_import.assert_awaited_once_with("t-1") +@pytest.mark.asyncio +async def test_import_held_without_library_root_propagates_and_stays_held(tmp_path): + """No library root configured: the ConfigurationError propagates to the route's + 400 mapping and the row stays held - the user restores a root and retries.""" + import threading + + from infrastructure.persistence.download_store import DownloadStore + + store = DownloadStore(db_path=tmp_path / "library.db", write_lock=threading.Lock()) + held_file = tmp_path / "held" / "x.flac" + held_file.parent.mkdir() + held_file.write_bytes(b"audio") + hid = await _record_held(store, held_file) + fp = MagicMock() + fp.place_held_file = AsyncMock( + side_effect=ConfigurationError( + "No library root is configured - restore one in Settings → Library, then try again." + ) + ) + svc = _held_service(store, fp) + store.resolve_held_import = AsyncMock() + + with pytest.raises(ConfigurationError, match="No library root is configured"): + await svc.import_held(hid, "user-a", "user") + + store.resolve_held_import.assert_not_awaited() # NOT resolved: retry stays possible + svc._orchestrator.settle_after_manual_import.assert_not_awaited() + held = await store.list_held_imports("user-a", "user") + assert [value.id for value in held] == [hid] + assert await store.has_unresolved_held_for_task("t-1") is True + + @pytest.mark.asyncio async def test_import_held_unknown_id_raises_not_found(tmp_path): import threading diff --git a/backend/tests/services/test_file_processor.py b/backend/tests/services/test_file_processor.py index 7e3037fb5..dcc076e61 100644 --- a/backend/tests/services/test_file_processor.py +++ b/backend/tests/services/test_file_processor.py @@ -1198,6 +1198,45 @@ async def test_place_held_file_imports_bypassing_verify(tmp_path: Path): assert await manager.has_album("rg-9") is True +@pytest.mark.asyncio +async def test_place_held_file_without_library_root_raises_configuration_error( + tmp_path: Path, +): + """Every root removed since the file was held: an actionable, recoverable 400 + (the user restores a root and retries) instead of an IndexError 500 - and the + publisher is never touched, so nothing lands in a half-imported state.""" + from core.exceptions import ConfigurationError + from models.held_import import HeldImport + + publisher = AsyncMock() + fp, _manager, _client, _library, _downloads = _make_processor( + tmp_path, publisher=publisher + ) + fp._library_paths = [] # roots cleared after the hold was recorded + held_dir = tmp_path / "held" + held_dir.mkdir() + held_file = held_dir / "src.flac" + shutil.copy(_FLAC, held_file) + held = HeldImport( + id=1, + user_id="user-a", + held_path=str(held_file), + reason="fingerprint_mismatch", + source="usenet", + status="held", + created_at=0.0, + release_group_mbid="rg-9", + recording_mbid="rec-3", + track_number=3, + naming_template=_TEMPLATE, + ) + + with pytest.raises(ConfigurationError, match="No library root is configured"): + await fp.place_held_file(held) + + publisher.assert_not_called() + + # -- P2: the file's OWN tags vs the requested identity (2026-07-05 wrong-single) -- diff --git a/backend/tests/services/test_jellyfin_library_service.py b/backend/tests/services/test_jellyfin_library_service.py index 720aaea0d..c844b4f5a 100644 --- a/backend/tests/services/test_jellyfin_library_service.py +++ b/backend/tests/services/test_jellyfin_library_service.py @@ -1,5 +1,6 @@ from __future__ import annotations +from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock import pytest @@ -216,3 +217,145 @@ async def test_artist_summary_has_play_count(self): result = await service.get_favorites_expanded(limit=10) assert len(result.artists) == 1 assert result.artists[0].play_count == 77 + + +class TestImportPlaylistMapsAlbumMbids: + """import_playlist stores the MusicBrainz MBID as album_id, not the + Jellyfin album GUID, so the MBID-keyed local catalog can match (#150).""" + + @staticmethod + def _requesting(): + return SimpleNamespace(id="u1") + + @staticmethod + def _playlist_service(): + ps = MagicMock() + ps.get_by_source_ref = AsyncMock(return_value=None) + ps.create_playlist = AsyncMock(return_value=SimpleNamespace(id="dn-pl-1")) + ps.add_tracks = AsyncMock() + return ps + + @staticmethod + def _track_item(id: str, album_id: str, track_number: int = 1) -> JellyfinItem: + return JellyfinItem( + id=id, + name=f"Song {id}", + type="Audio", + artist_name="Artist", + album_name="Album", + album_id=album_id, + artist_id="artist-1", + index_number=track_number, + parent_index_number=1, + duration_ticks=200 * 10_000_000, + ) + + def _wire_playlist(self, repo, track_items): + repo.get_playlists = AsyncMock( + return_value=[_item(id="pl-1", name="Mix", type="Playlist")] + ) + repo.get_playlist = AsyncMock( + return_value=_item(id="pl-1", name="Mix", type="Playlist") + ) + repo.get_playlist_items = AsyncMock(return_value=track_items) + + @pytest.mark.asyncio + async def test_stores_release_group_mbid_as_album_id(self): + service, repo = _make_service() + self._wire_playlist(repo, [self._track_item("t1", "guid-a")]) + repo.get_album_detail = AsyncMock( + return_value=_item( + id="guid-a", + provider_ids={ + "MusicBrainzReleaseGroup": "rg-123", + "MusicBrainzAlbum": "rel-456", + }, + ) + ) + ps = self._playlist_service() + + await service.import_playlist("pl-1", ps, self._requesting()) + + track_dicts = ps.add_tracks.call_args[0][2] + assert track_dicts[0]["album_id"] == "rg-123" + assert track_dicts[0]["track_source_id"] == "t1" + assert track_dicts[0]["source_type"] == "jellyfin" + + @pytest.mark.asyncio + async def test_falls_back_to_musicbrainz_album_provider_id(self): + service, repo = _make_service() + self._wire_playlist(repo, [self._track_item("t1", "guid-a")]) + repo.get_album_detail = AsyncMock( + return_value=_item(id="guid-a", provider_ids={"MusicBrainzAlbum": "rel-456"}) + ) + ps = self._playlist_service() + + await service.import_playlist("pl-1", ps, self._requesting()) + + track_dicts = ps.add_tracks.call_args[0][2] + assert track_dicts[0]["album_id"] == "rel-456" + + @pytest.mark.asyncio + async def test_keeps_guid_when_album_detail_missing(self): + service, repo = _make_service() + self._wire_playlist(repo, [self._track_item("t1", "guid-a")]) + repo.get_album_detail = AsyncMock(return_value=None) + ps = self._playlist_service() + + await service.import_playlist("pl-1", ps, self._requesting()) + + track_dicts = ps.add_tracks.call_args[0][2] + assert track_dicts[0]["album_id"] == "guid-a" + + @pytest.mark.asyncio + async def test_keeps_guid_when_no_provider_ids(self): + service, repo = _make_service() + self._wire_playlist(repo, [self._track_item("t1", "guid-a")]) + repo.get_album_detail = AsyncMock(return_value=_item(id="guid-a")) + ps = self._playlist_service() + + await service.import_playlist("pl-1", ps, self._requesting()) + + track_dicts = ps.add_tracks.call_args[0][2] + assert track_dicts[0]["album_id"] == "guid-a" + + @pytest.mark.asyncio + async def test_fetches_each_distinct_album_once(self): + service, repo = _make_service() + self._wire_playlist( + repo, + [ + self._track_item("t1", "guid-a", track_number=1), + self._track_item("t2", "guid-a", track_number=2), + self._track_item("t3", "guid-b", track_number=1), + ], + ) + repo.get_album_detail = AsyncMock( + side_effect=lambda guid: _item( + id=guid, provider_ids={"MusicBrainzReleaseGroup": f"rg-{guid}"} + ) + ) + ps = self._playlist_service() + + await service.import_playlist("pl-1", ps, self._requesting()) + + assert repo.get_album_detail.await_count == 2 + awaited = {c.args[0] for c in repo.get_album_detail.await_args_list} + assert awaited == {"guid-a", "guid-b"} + track_dicts = ps.add_tracks.call_args[0][2] + assert [t["album_id"] for t in track_dicts] == [ + "rg-guid-a", + "rg-guid-a", + "rg-guid-b", + ] + + @pytest.mark.asyncio + async def test_already_imported_skips_album_fetches(self): + service, repo = _make_service() + ps = self._playlist_service() + ps.get_by_source_ref = AsyncMock(return_value=SimpleNamespace(id="dn-pl-1")) + + result = await service.import_playlist("pl-1", ps, self._requesting()) + + assert result.already_imported is True + repo.get_album_detail.assert_not_called() diff --git a/backend/tests/services/test_jellyfin_playback_service.py b/backend/tests/services/test_jellyfin_playback_service.py index b99727a9c..9a6bcfa53 100644 --- a/backend/tests/services/test_jellyfin_playback_service.py +++ b/backend/tests/services/test_jellyfin_playback_service.py @@ -1,3 +1,4 @@ +import logging import pytest from unittest.mock import AsyncMock, MagicMock @@ -225,3 +226,131 @@ async def test_start_playback_app_level_auth_failure_still_raises(self): with pytest.raises(JellyfinAuthError): await svc.start_playback("item-1") + + +class TestPerUserProxyStreaming: + """Streaming proxies through the user's linked repo, app account as fallback.""" + + @staticmethod + def _stream_result(): + from repositories.navidrome_models import StreamProxyResult + + async def _chunks(): + yield b"data" + + return StreamProxyResult( + status_code=200, + headers={"Content-Type": "audio/flac"}, + media_type="audio/flac", + body_chunks=_chunks(), + ) + + @staticmethod + def _factory(per_user_repo): + factory = MagicMock() + factory.resolve_jellyfin = AsyncMock(return_value=per_user_repo) + return factory + + @pytest.mark.asyncio + async def test_proxy_stream_uses_per_user_repo_when_linked(self): + app_repo = _make_repo() + per_user = _make_repo() + per_user.proxy_get_stream = AsyncMock(return_value=self._stream_result()) + svc = JellyfinPlaybackService( + jellyfin_repo=app_repo, client_factory=self._factory(per_user) + ) + + await svc.proxy_stream("item-1", range_header="bytes=0-", user_id="u1") + + per_user.proxy_get_stream.assert_awaited_once_with( + "item-1", range_header="bytes=0-" + ) + app_repo.proxy_get_stream.assert_not_called() + + @pytest.mark.asyncio + async def test_proxy_head_uses_per_user_repo_when_linked(self): + app_repo = _make_repo() + per_user = _make_repo() + per_user.proxy_head_stream = AsyncMock(return_value=self._stream_result()) + svc = JellyfinPlaybackService( + jellyfin_repo=app_repo, client_factory=self._factory(per_user) + ) + + await svc.proxy_head("item-1", user_id="u1") + + per_user.proxy_head_stream.assert_awaited_once_with("item-1") + app_repo.proxy_head_stream.assert_not_called() + + @pytest.mark.asyncio + async def test_proxy_stream_falls_back_to_app_repo_when_unlinked(self): + app_repo = _make_repo() + app_repo.proxy_get_stream = AsyncMock(return_value=self._stream_result()) + svc = JellyfinPlaybackService( + jellyfin_repo=app_repo, client_factory=self._factory(None) + ) + + await svc.proxy_stream("item-1", user_id="u1") + + app_repo.proxy_get_stream.assert_awaited_once_with( + "item-1", range_header=None + ) + + @pytest.mark.asyncio + async def test_proxy_stream_falls_back_to_app_repo_on_linked_auth_failure( + self, caplog + ): + from core.exceptions import JellyfinAuthError + + app_repo = _make_repo() + app_repo.proxy_get_stream = AsyncMock(return_value=self._stream_result()) + per_user = _make_repo() + per_user.proxy_get_stream = AsyncMock( + side_effect=JellyfinAuthError("revoked") + ) + svc = JellyfinPlaybackService( + jellyfin_repo=app_repo, client_factory=self._factory(per_user) + ) + + with caplog.at_level(logging.WARNING): + await svc.proxy_stream("item-1", user_id="u1") + + per_user.proxy_get_stream.assert_awaited_once() + app_repo.proxy_get_stream.assert_awaited_once_with( + "item-1", range_header=None + ) + assert any( + "app account" in record.message and "u1" in record.message + for record in caplog.records + ) + + @pytest.mark.asyncio + async def test_proxy_head_falls_back_to_app_repo_on_linked_auth_failure(self): + from core.exceptions import JellyfinAuthError + + app_repo = _make_repo() + app_repo.proxy_head_stream = AsyncMock(return_value=self._stream_result()) + per_user = _make_repo() + per_user.proxy_head_stream = AsyncMock( + side_effect=JellyfinAuthError("revoked") + ) + svc = JellyfinPlaybackService( + jellyfin_repo=app_repo, client_factory=self._factory(per_user) + ) + + await svc.proxy_head("item-1", user_id="u1") + + per_user.proxy_head_stream.assert_awaited_once() + app_repo.proxy_head_stream.assert_awaited_once_with("item-1") + + @pytest.mark.asyncio + async def test_proxy_stream_app_level_auth_failure_still_raises(self): + from core.exceptions import JellyfinAuthError + + app_repo = _make_repo() + app_repo.proxy_get_stream = AsyncMock( + side_effect=JellyfinAuthError("bad api key") + ) + svc = JellyfinPlaybackService(jellyfin_repo=app_repo) + + with pytest.raises(JellyfinAuthError): + await svc.proxy_stream("item-1") diff --git a/backend/tests/services/test_navidrome_stream_proxy.py b/backend/tests/services/test_navidrome_stream_proxy.py index 6ecd29019..c222e4688 100644 --- a/backend/tests/services/test_navidrome_stream_proxy.py +++ b/backend/tests/services/test_navidrome_stream_proxy.py @@ -92,3 +92,71 @@ async def test_proxy_stream_raises_416(): with pytest.raises(ExternalServiceError, match="416"): await service.proxy_stream("song-1", "bytes=9999-") + + +class TestPerUserProxy: + """Streaming proxies through the user's linked repo, app account as fallback.""" + + @staticmethod + def _stream_result(): + from repositories.navidrome_repository import StreamProxyResult + + async def fake_chunks(): + yield b"audio" + + return StreamProxyResult( + status_code=200, + headers={"Content-Type": "audio/flac"}, + media_type="audio/flac", + body_chunks=fake_chunks(), + ) + + @staticmethod + def _factory(per_user_repo): + factory = MagicMock() + factory.resolve_navidrome = AsyncMock(return_value=per_user_repo) + return factory + + @pytest.mark.asyncio + async def test_proxy_stream_uses_per_user_repo_when_linked(self): + app_repo = MagicMock() + per_user = MagicMock() + per_user.proxy_get_stream = AsyncMock(return_value=self._stream_result()) + service = NavidromePlaybackService( + navidrome_repo=app_repo, client_factory=self._factory(per_user) + ) + + await service.proxy_stream("song-1", "bytes=0-", user_id="u1") + + per_user.proxy_get_stream.assert_awaited_once_with( + "song-1", range_header="bytes=0-" + ) + app_repo.proxy_get_stream.assert_not_called() + + @pytest.mark.asyncio + async def test_proxy_head_uses_per_user_repo_when_linked(self): + app_repo = MagicMock() + per_user = MagicMock() + per_user.proxy_head_stream = AsyncMock(return_value=self._stream_result()) + service = NavidromePlaybackService( + navidrome_repo=app_repo, client_factory=self._factory(per_user) + ) + + await service.proxy_head("song-1", user_id="u1") + + per_user.proxy_head_stream.assert_awaited_once_with("song-1") + app_repo.proxy_head_stream.assert_not_called() + + @pytest.mark.asyncio + async def test_proxy_stream_falls_back_to_app_repo_when_unlinked(self): + app_repo = MagicMock() + app_repo.proxy_get_stream = AsyncMock(return_value=self._stream_result()) + service = NavidromePlaybackService( + navidrome_repo=app_repo, client_factory=self._factory(None) + ) + + await service.proxy_stream("song-1", None, user_id="u1") + + app_repo.proxy_get_stream.assert_awaited_once_with( + "song-1", range_header=None + ) diff --git a/backend/tests/services/test_playlist_source_resolution.py b/backend/tests/services/test_playlist_source_resolution.py index 928f7a74b..a9f37dd3d 100644 --- a/backend/tests/services/test_playlist_source_resolution.py +++ b/backend/tests/services/test_playlist_source_resolution.py @@ -480,3 +480,70 @@ async def _match(album_id, album_name, artist_name): assert sorted(result["t-a"]) == ["navidrome"] assert sorted(result["t-b"]) == ["navidrome"] + + +class TestGuidAlbumIdFallback: + """Pre-fix Jellyfin imports stored the Jellyfin album GUID as album_id, which + the MBID-keyed matchers can never hit. Resolution re-keys the GUID to the + album's MusicBrainz id via Jellyfin provider ids (no migration, #150).""" + + @pytest.mark.asyncio + async def test_guid_album_id_resolves_local_source(self, tmp_path): + service, repo = _make_service(tmp_path) + track = _make_track( + album_id="jf-guid-1", + source_type="jellyfin", + available_sources=["jellyfin"], + ) + repo.get_tracks = MagicMock(return_value=[track]) + + jf = _make_jf_service(found=False) + jf.resolve_album_mbid = AsyncMock(return_value="mbid-abc") + local = _make_local_service() + + result = await service.resolve_track_sources( + "p-1", jf_service=jf, local_service=local, + ) + + local.match_album_by_mbid.assert_called_once_with("mbid-abc") + assert sorted(result["t-1"]) == ["jellyfin", "local"] + repo.batch_link_library_files.assert_called_once_with("p-1", {"t-1": "789"}) + updates = repo.batch_update_available_sources.call_args[0][1] + assert sorted(updates["t-1"]) == ["jellyfin", "local"] + + @pytest.mark.asyncio + async def test_guid_fallback_does_not_poison_mbid_cache_key(self, tmp_path): + cache = AsyncMock() + cache.get = AsyncMock(return_value=None) + repo = MagicMock() + service = PlaylistService(repo=repo, cache_dir=tmp_path, cache=cache) + jf = _make_jf_service(found=False) + jf.resolve_album_mbid = AsyncMock(return_value="mbid-abc") + + await service._resolve_album_sources("jf-guid-1", jf, None) + + set_key = cache.set.call_args[0][0] + assert "jf-guid-1" in set_key + assert "mbid-abc" not in set_key + + @pytest.mark.asyncio + async def test_no_fallback_when_mbid_match_found(self, tmp_path): + service, _ = _make_service(tmp_path) + jf = _make_jf_service(found=False) + jf.match_album_by_mbid = AsyncMock( + return_value=SimpleNamespace( + found=True, + tracks=[ + SimpleNamespace( + track_number=1, title="Wall Street Shuffle", jellyfin_id="jf-9" + ) + ], + ) + ) + jf.resolve_album_mbid = AsyncMock(return_value="mbid-other") + local = _make_local_service() + + await service._resolve_album_sources("mbid-abc", jf, local) + + jf.resolve_album_mbid.assert_not_called() + local.match_album_by_mbid.assert_called_once_with("mbid-abc") diff --git a/backend/tests/services/test_plex_playback_service.py b/backend/tests/services/test_plex_playback_service.py index 8f53d29fc..24654320e 100644 --- a/backend/tests/services/test_plex_playback_service.py +++ b/backend/tests/services/test_plex_playback_service.py @@ -186,3 +186,69 @@ async def test_stopped_uses_per_user_repo_when_linked(self): assert result is True per_user.now_playing.assert_awaited_once_with("rk-1", state="stopped") app_repo.now_playing.assert_not_awaited() + + +class TestPerUserProxy: + """Streaming proxies through the user's linked repo, app account as fallback.""" + + @staticmethod + def _factory(per_user_repo): + factory = MagicMock() + factory.resolve_plex = AsyncMock(return_value=per_user_repo) + return factory + + @pytest.mark.asyncio + async def test_proxy_stream_uses_per_user_repo_when_linked(self): + app_repo = MagicMock() + per_user = MagicMock() + per_user.proxy_get_stream = AsyncMock( + return_value=StreamProxyResult( + status_code=200, headers={}, media_type="audio/mpeg", + body_chunks=iter([b"data"]), + ) + ) + service = PlexPlaybackService( + plex_repo=app_repo, client_factory=self._factory(per_user) + ) + + await service.proxy_stream("/part/key", range_header="bytes=0-", user_id="u1") + + per_user.proxy_get_stream.assert_awaited_once_with( + "/part/key", range_header="bytes=0-" + ) + app_repo.proxy_get_stream.assert_not_called() + + @pytest.mark.asyncio + async def test_proxy_head_uses_per_user_repo_when_linked(self): + app_repo = MagicMock() + per_user = MagicMock() + per_user.proxy_head_stream = AsyncMock( + return_value=StreamProxyResult(status_code=200, headers={}, media_type=None) + ) + service = PlexPlaybackService( + plex_repo=app_repo, client_factory=self._factory(per_user) + ) + + await service.proxy_head("/part/key", user_id="u1") + + per_user.proxy_head_stream.assert_awaited_once_with("/part/key") + app_repo.proxy_head_stream.assert_not_called() + + @pytest.mark.asyncio + async def test_proxy_stream_falls_back_to_app_repo_when_unlinked(self): + app_repo = MagicMock() + app_repo.proxy_get_stream = AsyncMock( + return_value=StreamProxyResult( + status_code=200, headers={}, media_type="audio/mpeg", + body_chunks=iter([b"data"]), + ) + ) + service = PlexPlaybackService( + plex_repo=app_repo, client_factory=self._factory(None) + ) + + await service.proxy_stream("/part/key", user_id="u1") + + app_repo.proxy_get_stream.assert_awaited_once_with( + "/part/key", range_header=None + ) diff --git a/backend/tests/services/test_settings_cache_invalidation.py b/backend/tests/services/test_settings_cache_invalidation.py index 40d39c91e..c50566a8f 100644 --- a/backend/tests/services/test_settings_cache_invalidation.py +++ b/backend/tests/services/test_settings_cache_invalidation.py @@ -295,3 +295,125 @@ async def test_youtube_settings_change_clears_home_cache(): mock_repo_fn.cache_clear.assert_called_once() for key in home_keys: assert await cache.get(key) is None + + +@pytest.fixture +def mb_live_state(): + """Snapshot and restore the live MusicBrainz module state around a test.""" + from repositories.musicbrainz_base import ( + get_mb_api_base, + set_mb_api_base, + mb_rate_limiter, + ) + + base = get_mb_api_base() + rate = mb_rate_limiter.rate + capacity = mb_rate_limiter.capacity + yield + set_mb_api_base(base) + mb_rate_limiter.update_rate(rate) + mb_rate_limiter.update_capacity(capacity) + + +def _mb_settings(api_url: str, rate_limit: float, concurrent_searches: int): + from api.v1.schemas.settings import MusicBrainzConnectionSettings + + return MusicBrainzConnectionSettings( + api_url=api_url, + rate_limit=rate_limit, + concurrent_searches=concurrent_searches, + ) + + +def _seed_mb_live_state(settings) -> None: + from repositories.musicbrainz_base import set_mb_api_base, mb_rate_limiter + + set_mb_api_base(settings.api_url) + mb_rate_limiter.update_rate(settings.rate_limit) + mb_rate_limiter.update_capacity(settings.concurrent_searches) + + +def _spy_mb_side_effects(monkeypatch): + from unittest.mock import MagicMock + + from repositories.musicbrainz_base import mb_circuit_breaker, mb_deduplicator + + reset = MagicMock() + clear = MagicMock() + monkeypatch.setattr(mb_circuit_breaker, "reset", reset) + monkeypatch.setattr(mb_deduplicator, "clear", clear) + return reset, clear + + +@pytest.mark.asyncio(loop_scope="function") +async def test_musicbrainz_settings_unchanged_skips_reset_and_cache_clear( + mb_live_state, monkeypatch +): + """Saving identical MusicBrainz settings is a no-op: no breaker reset, no + deduplicator clear, caches survive - endpoint behavior did not change.""" + service, cache = await _build_service() + settings = _mb_settings("https://mb.mirror.example/ws/2", 2.0, 4) + _seed_mb_live_state(settings) + reset, clear = _spy_mb_side_effects(monkeypatch) + + mb_keys = [f"{p}noop" for p in musicbrainz_prefixes()] + await _populate(cache, mb_keys) + + await service.on_musicbrainz_settings_changed(settings) + + reset.assert_not_called() + clear.assert_not_called() + for key in mb_keys: + assert await cache.get(key) == "v" + + +@pytest.mark.asyncio(loop_scope="function") +async def test_musicbrainz_settings_endpoint_change_resets_and_clears( + mb_live_state, monkeypatch +): + """An endpoint change resets the breaker, clears the deduplicator, and + clears the MusicBrainz cache prefixes.""" + service, cache = await _build_service() + _seed_mb_live_state(_mb_settings("https://mb-a.example/ws/2", 2.0, 4)) + reset, clear = _spy_mb_side_effects(monkeypatch) + + mb_keys = [f"{p}endpoint" for p in musicbrainz_prefixes()] + await _populate(cache, mb_keys) + + await service.on_musicbrainz_settings_changed( + _mb_settings("https://mb-b.example/ws/2", 2.0, 4) + ) + + from repositories.musicbrainz_base import get_mb_api_base + + reset.assert_called_once() + clear.assert_called_once() + assert get_mb_api_base() == "https://mb-b.example/ws/2" + for key in mb_keys: + assert await cache.get(key) is None + + +@pytest.mark.asyncio(loop_scope="function") +async def test_musicbrainz_settings_rate_only_change_resets_and_clears( + mb_live_state, monkeypatch +): + """A rate-only change re-arms the limiter, so it is behavior-changing and + must reset the breaker and clear caches too.""" + service, cache = await _build_service() + _seed_mb_live_state(_mb_settings("https://mb-a.example/ws/2", 2.0, 4)) + reset, clear = _spy_mb_side_effects(monkeypatch) + + mb_keys = [f"{p}rate" for p in musicbrainz_prefixes()] + await _populate(cache, mb_keys) + + await service.on_musicbrainz_settings_changed( + _mb_settings("https://mb-a.example/ws/2", 3.0, 4) + ) + + from repositories.musicbrainz_base import mb_rate_limiter + + reset.assert_called_once() + clear.assert_called_once() + assert mb_rate_limiter.rate == 3.0 + for key in mb_keys: + assert await cache.get(key) is None diff --git a/backend/tests/services/test_source_playlist_import.py b/backend/tests/services/test_source_playlist_import.py index 64a2b0669..7b9763bf3 100644 --- a/backend/tests/services/test_source_playlist_import.py +++ b/backend/tests/services/test_source_playlist_import.py @@ -93,6 +93,7 @@ def _jellyfin_service(playlists=None, items=None) -> JellyfinLibraryService: repo.get_most_played_albums = AsyncMock(return_value=[]) repo.get_library_stats = AsyncMock(return_value={"album_count": 0, "artist_count": 0, "track_count": 0}) repo.get_albums = AsyncMock(return_value=([], 0)) + repo.get_album_detail = AsyncMock(return_value=None) type(repo).stats_ttl = PropertyMock(return_value=600) prefs = MagicMock() conn = MagicMock() diff --git a/entrypoint.sh b/entrypoint.sh index 2184b18f6..b107698e7 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -1,6 +1,19 @@ #!/bin/sh set -e +# A bind mount over /app shadows the image's application code with the user's +# data directory; fail fast here instead of emitting confusing writability or +# runtime errors against the shadowing mount later. /app is always present in +# the image, so a missing /app means we are outside the container (e.g. tests +# or a hand-run shell) and the check is skipped. +if [ -d /app ] && { [ ! -f /app/main.py ] || [ ! -f /app/.droppedneedle-source-revision ] || [ ! -f /app/maintenance/automatic_upgrade.py ]; }; then + echo "[init] FATAL: /app does not contain the DroppedNeedle application code." + echo "[init] A bind mount over /app hides the application with your data." + echo "[init] Mount data subdirectories only: /app/config, /app/cache, /app/plugins, /app/imports." + echo "[init] Remove the /app bind mount and restart." + exit 1 +fi + REQUESTED_UMASK=${UMASK:-027} case "$REQUESTED_UMASK" in [0-7][0-7][0-7]|0[0-7][0-7][0-7]) ;; diff --git a/frontend/src/lib/api/playlists.ts b/frontend/src/lib/api/playlists.ts index 84b747ab8..5da63a40f 100644 --- a/frontend/src/lib/api/playlists.ts +++ b/frontend/src/lib/api/playlists.ts @@ -20,6 +20,7 @@ export interface PlaylistTrack { duration: number | null; created_at: string; plex_rating_key: string | null; + library_file_id: string | null; } export interface PlaylistSummary { diff --git a/frontend/src/lib/components/downloads/HeldTrackCard.svelte.spec.ts b/frontend/src/lib/components/downloads/HeldTrackCard.svelte.spec.ts index 4134eefcf..74396e93e 100644 --- a/frontend/src/lib/components/downloads/HeldTrackCard.svelte.spec.ts +++ b/frontend/src/lib/components/downloads/HeldTrackCard.svelte.spec.ts @@ -4,11 +4,28 @@ import { render } from 'vitest-browser-svelte'; import type { HeldImport } from '$lib/types'; -const h = vi.hoisted(() => ({ importMut: vi.fn(), discardMut: vi.fn() })); +const h = vi.hoisted(() => ({ + importMut: vi.fn(), + discardMut: vi.fn(), + importError: null as { message: string } | null, + discardError: null as { message: string } | null +})); vi.mock('$lib/queries/downloads/DownloadMutations.svelte', () => ({ - importHeldTrack: () => ({ mutate: h.importMut, isPending: false }), - discardHeldTrack: () => ({ mutate: h.discardMut, isPending: false }) + importHeldTrack: () => ({ + mutate: h.importMut, + isPending: false, + get error() { + return h.importError; + } + }), + discardHeldTrack: () => ({ + mutate: h.discardMut, + isPending: false, + get error() { + return h.discardError; + } + }) })); import HeldTrackCard from './HeldTrackCard.svelte'; @@ -52,6 +69,8 @@ describe('HeldTrackCard', () => { beforeEach(() => { h.importMut.mockReset(); h.discardMut.mockReset(); + h.importError = null; + h.discardError = null; }); it('shows the track, the couldn’t-verify state, and the AcoustID evidence', async () => { @@ -72,6 +91,23 @@ describe('HeldTrackCard', () => { expect(h.discardMut).not.toHaveBeenCalled(); }); + it('renders a server error inline and leaves the buttons enabled for retry', async () => { + h.importError = { + message: 'No library root is configured - restore one in Settings → Library, then try again.' + }; + renderCard(held()); + + const alert = page.getByRole('alert'); + await expect.element(alert).toHaveTextContent(/No library root is configured/); + const importButton = page.getByRole('button', { name: /Import anyway/ }); + const discardButton = page.getByRole('button', { name: /Discard/ }); + await expect.element(importButton).toBeEnabled(); + await expect.element(discardButton).toBeEnabled(); + + await importButton.click(); + expect(h.importMut).toHaveBeenCalledTimes(1); + }); + it('discards the held track on "Discard"', async () => { renderCard(held()); await page.getByRole('button', { name: /Discard/ }).click(); diff --git a/frontend/src/lib/components/downloads/HeldTrackReview.svelte b/frontend/src/lib/components/downloads/HeldTrackReview.svelte index 6d507925c..258a68b72 100644 --- a/frontend/src/lib/components/downloads/HeldTrackReview.svelte +++ b/frontend/src/lib/components/downloads/HeldTrackReview.svelte @@ -27,6 +27,15 @@ // without this a fast second click re-POSTs an already-consumed held id let done = $state(false); const busy = $derived(importMut.isPending || discardMut.isPending || done); + // server-side failure reason (e.g. no library root configured) shown inline so the + // review card itself says what to fix - the toast alone disappears too fast + const actionError = $derived.by(() => { + const err = importMut.error ?? discardMut.error; + if (!err) return null; + return typeof err === 'object' && 'message' in err && typeof err.message === 'string' + ? err.message + : null; + }); // what the rejecting check SAW - the reason we couldn't auto-confirm it, so the human // decides informed. The evidence source depends on the hold reason: AcoustID's @@ -166,6 +175,9 @@ Discard + {#if actionError} + + {/if}