From 0b325da3e980d1593c02b0ad68a4aac2cbf60eb3 Mon Sep 17 00:00:00 2001 From: Tyler Richardson-LaPlume <170156756+IamGroot60@users.noreply.github.com> Date: Fri, 29 May 2026 02:04:47 -0400 Subject: [PATCH] =?UTF-8?q?Usenet=20bundle:=20writable=20staging=20dir=20+?= =?UTF-8?q?=20client=E2=86=92local=20path=20resolution=20(#721)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the poll fix, covering the two things that blocked a successful end-to-end album import once the poll itself stopped freezing: 1. Staging dir permissions The album-bundle private staging path defaults to 'storage/album_bundle_staging' -> /app/storage, but /app/storage was never created or chowned by the image (unlike /app/Staging, /app/Transfer, etc.), and /app is root-owned. The copy failed with "[Errno 13] Permission denied: 'storage'" under the non-root soulsync UID. Added /app/storage to the Dockerfile build-time mkdir+chown and the entrypoint PUID/PGID chown, exactly like the sibling runtime dirs. 2. Client->local path resolution Usenet/torrent clients report save paths from inside THEIR OWN container (e.g. SAB '/data/downloads/music/'); SoulSync often mounts the same files at a different point ('/app/downloads/'). Feeding the client path straight to the audio walker yields "No audio files found" though the files are physically present. New resolve_reported_save_path(): a. use the reported path as-is if readable (mirrored mounts), b. apply explicit download_source.usenet_path_mappings ({from,to}, Sonarr/Radarr-style) for non-shared layouts, c. basename fallback under SoulSync's own download roots — zero-config for the standard shared-volume arr setup. Wired into both call sites in usenet.py AND torrent.py (download_album_to_staging + _finalize_download), logging any translation and including the resolved path in the no-audio error. Tests: resolver verbatim / explicit-mapping / basename-fallback / priority / not-found / empty / mapping-miss-then-basename. ruff + compileall + pytest green (645 in the download suites). Co-Authored-By: Claude Opus 4.8 (1M context) --- Dockerfile | 11 ++- core/download_plugins/album_bundle.py | 106 ++++++++++++++++++++++++++ core/download_plugins/torrent.py | 22 +++++- core/download_plugins/usenet.py | 23 +++++- entrypoint.sh | 2 +- tests/test_album_bundle.py | 87 +++++++++++++++++++++ 6 files changed, 240 insertions(+), 11 deletions(-) diff --git a/Dockerfile b/Dockerfile index fdf5a21f..375b4fd9 100644 --- a/Dockerfile +++ b/Dockerfile @@ -79,8 +79,15 @@ COPY --chown=soulsync:soulsync --from=webui-builder /app/webui/static/dist /app/ # fails silently on rootless Docker where the soulsync UID can't write # to /app — playback then errors out with no obvious cause. Pre-baking # at build time (when the layer is owned by root) avoids that path. -RUN mkdir -p /app/config /app/data /app/logs /app/downloads /app/Transfer /app/Staging /app/Stream /app/MusicVideos /app/scripts && \ - chown soulsync:soulsync /app/config /app/data /app/logs /app/downloads /app/Transfer /app/Staging /app/Stream /app/MusicVideos /app/scripts +# NOTE: /app/storage is the PRIVATE album-bundle staging area for the +# torrent / usenet whole-release flow (download_source.album_bundle_staging_path +# defaults to 'storage/album_bundle_staging'). Like /app/Stream it's created +# lazily at runtime via mkdir(parents=True); without pre-baking it owned by +# soulsync, the album-bundle copy fails with "[Errno 13] Permission denied: +# 'storage'" because /app itself is root-owned and the soulsync UID can't +# create a top-level dir there. +RUN mkdir -p /app/config /app/data /app/logs /app/downloads /app/Transfer /app/Staging /app/Stream /app/storage /app/MusicVideos /app/scripts && \ + chown soulsync:soulsync /app/config /app/data /app/logs /app/downloads /app/Transfer /app/Staging /app/Stream /app/storage /app/MusicVideos /app/scripts # Create defaults directory and copy template files # These will be used by entrypoint.sh to initialize empty volumes diff --git a/core/download_plugins/album_bundle.py b/core/download_plugins/album_bundle.py index 6abe6328..cf9030e4 100644 --- a/core/download_plugins/album_bundle.py +++ b/core/download_plugins/album_bundle.py @@ -487,6 +487,111 @@ def poll_album_download( return None +def _candidate_download_roots(config_get: Callable[..., Any]) -> list: + """Directories where THIS process can read finished downloads — used by + ``resolve_reported_save_path`` for the basename fallback. + + Order matters: most-specific usenet/torrent roots first, then the + general Soulseek download / transfer dirs, which in the standard + shared-volume arr setup are bind-mounted to the very directory the + usenet client writes its completed downloads into. Relative values + (e.g. ``./downloads``) resolve against the process CWD — the + container's ``/app`` — which is exactly where those mounts live. + """ + roots: list = [] + for key in ( + 'download_source.usenet_download_path', + 'usenet_client.completed_path', + 'usenet_client.download_path', + 'download_source.torrent_download_path', + 'soulseek.download_path', + 'soulseek.transfer_path', + ): + value = config_get(key, None) + if value: + roots.append(str(value)) + seen: set = set() + out: list = [] + for root in roots: + if root not in seen: + seen.add(root) + out.append(root) + return out + + +def resolve_reported_save_path( + reported_path: Optional[str], + config_get: Optional[Callable[..., Any]] = None, +) -> Optional[str]: + """Translate a downloader-reported save_path into one THIS process can read. + + Usenet / torrent clients report paths from inside THEIR OWN container + (e.g. SAB hands back ``/data/downloads/music/``); SoulSync often + mounts the very same files at a different point (``/app/downloads/``). + Feeding the client's path straight to the audio walker then yields + "No audio files found" even though the files are physically present — + the classic arr-stack remote-path mismatch. + + Resolution order: + 1. The reported path verbatim, if it's a readable directory here + (deployments that mirror the client's mount paths). + 2. Explicit prefix mappings from ``download_source.usenet_path_mappings`` + — a list of ``{"from": "...", "to": "..."}`` (Sonarr/Radarr-style + remote path mapping) for non-shared / oddly-mounted layouts. + 3. Basename fallback: a same-named folder under a known SoulSync + download root. Zero-config for the standard shared-volume setup — + the album folder shows up under SoulSync's own ``./downloads`` + mount with the same name the client reported. + + Returns the best resolved path, or ``reported_path`` unchanged when + nothing better is found (so the caller's existing "no audio" error still + surfaces, with both paths logged). + """ + if not reported_path: + return reported_path + if config_get is None: + config_get = config_manager.get + + def _is_dir(candidate) -> bool: + try: + return Path(candidate).is_dir() + except OSError: + return False + + # 1. Reported path is directly readable — mounts already line up. + if _is_dir(reported_path): + return reported_path + + normalized = str(reported_path).replace('\\', '/') + + # 2. Explicit prefix mappings (remote-path-mapping escape hatch). + mappings = config_get('download_source.usenet_path_mappings', None) or [] + if isinstance(mappings, (list, tuple)): + for mapping in mappings: + if not isinstance(mapping, dict): + continue + frm = str(mapping.get('from') or '').replace('\\', '/').rstrip('/') + to = str(mapping.get('to') or '') + if not frm or not to: + continue + if normalized == frm or normalized.startswith(frm + '/'): + rest = normalized[len(frm):].lstrip('/') + candidate = str(Path(to) / rest) if rest else to + if _is_dir(candidate): + return candidate + + # 3. Basename fallback under known download roots — covers the standard + # shared-volume layout with zero configuration. + basename = Path(normalized).name + if basename: + for root in _candidate_download_roots(config_get): + candidate = Path(root) / basename + if _is_dir(candidate): + return str(candidate) + + return reported_path + + def copy_audio_files_atomically( sources: Iterable[Path], staging_dir: Path, ) -> list: @@ -525,6 +630,7 @@ __all__ = [ "get_poll_interval", "get_poll_timeout", "get_transient_miss_threshold", + "resolve_reported_save_path", "pick_best_album_release", "poll_album_download", "quality_score", diff --git a/core/download_plugins/torrent.py b/core/download_plugins/torrent.py index 6c7cf4c3..17ef566b 100644 --- a/core/download_plugins/torrent.py +++ b/core/download_plugins/torrent.py @@ -65,6 +65,7 @@ from core.download_plugins.album_bundle import ( get_poll_timeout, pick_best_album_release, poll_album_download, + resolve_reported_save_path, ) from core.download_plugins.base import DownloadSourcePlugin from core.download_plugins.types import AlbumResult, DownloadStatus, TrackResult @@ -354,13 +355,21 @@ class TorrentDownloadPlugin(DownloadSourcePlugin): if not save_path: self._mark_error(download_id, "Torrent completed but no save_path reported") return + # Resolve the client-reported path to one this process can read + # (the client may report its own container's mount). See + # ``resolve_reported_save_path``. + local_path = resolve_reported_save_path(save_path) + if local_path != save_path: + logger.info("Torrent %s: resolved client path %r -> %r", + download_id[:8], save_path, local_path) try: - audio_files = collect_audio_after_extraction(Path(save_path)) + audio_files = collect_audio_after_extraction(Path(local_path)) except Exception as e: self._mark_error(download_id, f"Post-extract walk failed: {e}") return if not audio_files: - self._mark_error(download_id, f"No audio files found in {save_path}") + suffix = f" (resolved: {local_path})" if local_path != save_path else "" + self._mark_error(download_id, f"No audio files found in {save_path}{suffix}") return primary = audio_files[0] with self._lock: @@ -539,13 +548,18 @@ class TorrentDownloadPlugin(DownloadSourcePlugin): # Phase 4: extract + walk + copy to staging. _emit('staging', release=picked.title) + # Resolve the client-reported path to one this process can read. + local_path = resolve_reported_save_path(save_path) + if local_path != save_path: + logger.info("[Torrent album] Resolved client path %r -> %r", save_path, local_path) try: - audio_files = collect_audio_after_extraction(Path(save_path)) + audio_files = collect_audio_after_extraction(Path(local_path)) except Exception as e: result['error'] = f'Failed to walk audio files: {e}' return result if not audio_files: - result['error'] = f'No audio files found in {save_path}' + suffix = f' (resolved: {local_path})' if local_path != save_path else '' + result['error'] = f'No audio files found in {save_path}{suffix}' return result copied = copy_audio_files_atomically(audio_files, Path(staging_dir)) diff --git a/core/download_plugins/usenet.py b/core/download_plugins/usenet.py index 75302c50..a8b0c529 100644 --- a/core/download_plugins/usenet.py +++ b/core/download_plugins/usenet.py @@ -27,6 +27,7 @@ from core.download_plugins.album_bundle import ( get_completed_no_path_window_seconds, pick_best_album_release, poll_album_download, + resolve_reported_save_path, ) from core.download_plugins.base import DownloadSourcePlugin from core.download_plugins.torrent import ( @@ -337,13 +338,21 @@ class UsenetDownloadPlugin(DownloadSourcePlugin): if not save_path: self._mark_error(download_id, "Usenet job completed but no save_path reported") return + # Translate the client-reported path to one THIS process can read + # (SAB reports its own container path; SoulSync may see the same + # files at a different mount). See ``resolve_reported_save_path``. + local_path = resolve_reported_save_path(save_path) + if local_path != save_path: + logger.info("Usenet %s: resolved client path %r -> %r", + download_id[:8], save_path, local_path) try: - audio_files = collect_audio_after_extraction(Path(save_path)) + audio_files = collect_audio_after_extraction(Path(local_path)) except Exception as e: self._mark_error(download_id, f"Post-extract walk failed: {e}") return if not audio_files: - self._mark_error(download_id, f"No audio files found in {save_path}") + suffix = f" (resolved: {local_path})" if local_path != save_path else "" + self._mark_error(download_id, f"No audio files found in {save_path}{suffix}") return primary = audio_files[0] with self._lock: @@ -497,13 +506,19 @@ class UsenetDownloadPlugin(DownloadSourcePlugin): return result _emit('staging', release=picked.title) + # SAB reports its own container path; SoulSync may mount the same + # files elsewhere. Resolve to a locally-readable path before walking. + local_path = resolve_reported_save_path(save_path) + if local_path != save_path: + logger.info("[Usenet album] Resolved client path %r -> %r", save_path, local_path) try: - audio_files = collect_audio_after_extraction(Path(save_path)) + audio_files = collect_audio_after_extraction(Path(local_path)) except Exception as e: result['error'] = f'Failed to walk audio files: {e}' return result if not audio_files: - result['error'] = f'No audio files found in {save_path}' + suffix = f' (resolved: {local_path})' if local_path != save_path else '' + result['error'] = f'No audio files found in {save_path}{suffix}' return result copied = copy_audio_files_atomically(audio_files, Path(staging_dir)) diff --git a/entrypoint.sh b/entrypoint.sh index 8cde8e0f..28bae10a 100644 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -40,7 +40,7 @@ if [ "$CURRENT_UID" != "$PUID" ] || [ "$CURRENT_GID" != "$PGID" ]; then DATA_OWNER=$(stat -c '%u:%g' /app/data 2>/dev/null || echo "unknown") if [ "$DATA_OWNER" != "$PUID:$PGID" ]; then echo "🔒 Fixing permissions on app directories..." - chown -R soulsync:soulsync /app/config /app/data /app/logs /app/downloads /app/Transfer /app/Staging /app/Stream 2>/dev/null || true + chown -R soulsync:soulsync /app/config /app/data /app/logs /app/downloads /app/Transfer /app/Staging /app/Stream /app/storage 2>/dev/null || true else echo "✅ App directory permissions already correct" fi diff --git a/tests/test_album_bundle.py b/tests/test_album_bundle.py index 57790286..6a237b22 100644 --- a/tests/test_album_bundle.py +++ b/tests/test_album_bundle.py @@ -32,6 +32,7 @@ from core.download_plugins.album_bundle import ( get_poll_timeout, pick_best_album_release, quality_score, + resolve_reported_save_path, unique_staging_path, ) @@ -325,6 +326,92 @@ def test_get_completed_no_path_window_falls_back_on_garbage() -> None: assert get_completed_no_path_window_seconds() == DEFAULT_COMPLETED_NO_PATH_WINDOW_SECONDS +# --------------------------------------------------------------------------- +# resolve_reported_save_path — downloader→local path translation. The arr +# remote-path problem: SAB reports its own container path, SoulSync mounts +# the same files elsewhere. +# --------------------------------------------------------------------------- + + +def _cfg(values: dict): + """Build a config_manager.get-shaped callable from a dict.""" + def _get(key, default=None): + return values.get(key, default) + return _get + + +def test_resolve_returns_reported_path_verbatim_when_readable(tmp_path: Path) -> None: + """If the client's path is already readable here (mounts mirror the + client), return it unchanged — no translation needed.""" + album = tmp_path / "MyAlbum" + album.mkdir() + # config_get should never even be consulted on the happy path. + assert resolve_reported_save_path(str(album), config_get=_cfg({})) == str(album) + + +def test_resolve_uses_explicit_prefix_mapping(tmp_path: Path) -> None: + """Sonarr/Radarr-style remote path mapping: SAB's prefix is rewritten + to a SoulSync-visible root.""" + (tmp_path / "MyAlbum").mkdir() + cfg = _cfg({'download_source.usenet_path_mappings': [ + {'from': '/data/downloads/music', 'to': str(tmp_path)}, + ]}) + resolved = resolve_reported_save_path('/data/downloads/music/MyAlbum', config_get=cfg) + assert resolved == str(tmp_path / "MyAlbum") + + +def test_resolve_basename_fallback_against_download_root(tmp_path: Path) -> None: + """Zero-config shared-volume case: the album folder shows up under + SoulSync's own download root with the same name SAB reported.""" + (tmp_path / "MyAlbum").mkdir() + cfg = _cfg({'soulseek.download_path': str(tmp_path)}) + resolved = resolve_reported_save_path('/data/downloads/music/MyAlbum', config_get=cfg) + assert resolved == str(tmp_path / "MyAlbum") + + +def test_resolve_mapping_takes_priority_over_basename(tmp_path: Path) -> None: + """An explicit mapping that resolves wins over the basename scan.""" + mapped_root = tmp_path / "mapped" + dl_root = tmp_path / "dl" + (mapped_root / "MyAlbum").mkdir(parents=True) + (dl_root / "MyAlbum").mkdir(parents=True) + cfg = _cfg({ + 'download_source.usenet_path_mappings': [ + {'from': '/data/downloads/music', 'to': str(mapped_root)}, + ], + 'soulseek.download_path': str(dl_root), + }) + resolved = resolve_reported_save_path('/data/downloads/music/MyAlbum', config_get=cfg) + assert resolved == str(mapped_root / "MyAlbum") + + +def test_resolve_returns_reported_unchanged_when_nothing_found(tmp_path: Path) -> None: + """No readable path, no mapping hit, no basename match → return the + original so the caller's 'no audio' error still surfaces.""" + cfg = _cfg({'soulseek.download_path': str(tmp_path)}) # empty root + reported = '/data/downloads/music/Missing' + assert resolve_reported_save_path(reported, config_get=cfg) == reported + + +def test_resolve_handles_empty_and_none(tmp_path: Path) -> None: + assert resolve_reported_save_path('', config_get=_cfg({})) == '' + assert resolve_reported_save_path(None, config_get=_cfg({})) is None + + +def test_resolve_skips_mapping_when_target_missing_then_tries_basename(tmp_path: Path) -> None: + """A mapping whose translated path doesn't exist must not short-circuit + — fall through to the basename scan.""" + (tmp_path / "MyAlbum").mkdir() + cfg = _cfg({ + 'download_source.usenet_path_mappings': [ + {'from': '/data/downloads/music', 'to': '/nope/not/mounted'}, + ], + 'soulseek.download_path': str(tmp_path), + }) + resolved = resolve_reported_save_path('/data/downloads/music/MyAlbum', config_get=cfg) + assert resolved == str(tmp_path / "MyAlbum") + + # --------------------------------------------------------------------------- # poll_album_download — lifted poll loop for both torrent + usenet plugins. # ---------------------------------------------------------------------------