Usenet bundle: writable staging dir + client→local path resolution (#721)

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/<album>'); SoulSync often
   mounts the same files at a different point ('/app/downloads/<album>').
   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) <noreply@anthropic.com>
This commit is contained in:
Tyler Richardson-LaPlume 2026-05-29 02:04:47 -04:00
parent b8384beef9
commit 0b325da3e9
6 changed files with 240 additions and 11 deletions

View file

@ -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

View file

@ -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/<album>``); SoulSync often
mounts the very same files at a different point (``/app/downloads/<album>``).
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",

View file

@ -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))

View file

@ -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))

View file

@ -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

View file

@ -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.
# ---------------------------------------------------------------------------