From 15903301717029920e3f64b648c992f564fd4b80 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 4 Jun 2026 21:56:07 -0700 Subject: [PATCH] Fix #796: Soulseek album bundle left completed files in slskd download folder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The album-bundle path COPIES slskd's completed files into private staging (then on to the library) but never removed slskd's originals, so they piled up in the download folder. (copy, not move, is correct for the torrent/usenet bundle paths — those clients keep seeding — so the shared copier can't just always delete.) Add an opt-in remove_source to copy_audio_files_atomically that deletes each source ONLY after it copies successfully (never on a failed stage), and set it for the Soulseek path only. Torrent/usenet keep their originals. Tests: keeps source by default / removes when requested / keeps on failed copy. --- core/download_plugins/album_bundle.py | 18 ++++++++++++-- core/soulseek_client.py | 5 +++- tests/test_album_bundle.py | 34 +++++++++++++++++++++++++++ tests/test_soulseek_album_fallback.py | 2 +- 4 files changed, 55 insertions(+), 4 deletions(-) diff --git a/core/download_plugins/album_bundle.py b/core/download_plugins/album_bundle.py index 17d9d6fa..2c584a61 100644 --- a/core/download_plugins/album_bundle.py +++ b/core/download_plugins/album_bundle.py @@ -706,13 +706,19 @@ def resolve_reported_save_path( def copy_audio_files_atomically( - sources: Iterable[Path], staging_dir: Path, + sources: Iterable[Path], staging_dir: Path, remove_source: bool = False, ) -> list: """Convenience wrapper: pick a non-colliding staging path for each source, copy via ``atomic_copy_to_staging``. Returns the list of final destination paths (as strings). Files that fail to copy are logged and skipped; the caller decides what to do - with a partial result.""" + with a partial result. + + ``remove_source=True`` deletes each source AFTER it copies + successfully — used by the Soulseek bundle path so slskd's + completed downloads don't pile up in its download folder (#796). + Kept False for torrent/usenet, whose clients must retain the + originals (seeding / client-managed).""" staging_dir.mkdir(parents=True, exist_ok=True) out: list = [] for src in sources: @@ -720,6 +726,14 @@ def copy_audio_files_atomically( try: atomic_copy_to_staging(src, dest) out.append(str(dest)) + if remove_source: + # Only after a verified copy — never lose data on a failed stage. + try: + Path(src).unlink() + except FileNotFoundError: + pass + except Exception as e: + logger.debug("[album_bundle] Could not remove staged source %s: %s", src, e) except Exception as e: logger.warning("[album_bundle] Failed to stage %s -> %s: %s", src, dest, e) return out diff --git a/core/soulseek_client.py b/core/soulseek_client.py index 80d7d4bc..9843b21e 100644 --- a/core/soulseek_client.py +++ b/core/soulseek_client.py @@ -1613,7 +1613,10 @@ class SoulseekClient(DownloadSourcePlugin): return result _emit('staging', release=getattr(picked, 'album_title', folder_path) if picked else folder_path) - copied = copy_audio_files_atomically(completed, Path(staging_dir)) + # remove_source=True: clean slskd's completed files once staged so they + # don't pile up in the download folder (#796). Soulseek has no seeding, + # unlike the torrent/usenet bundle paths which keep their originals. + copied = copy_audio_files_atomically(completed, Path(staging_dir), remove_source=True) if not copied: result['error'] = 'No Soulseek album files copied to staging' return result diff --git a/tests/test_album_bundle.py b/tests/test_album_bundle.py index e4ec05d2..8b85e6c4 100644 --- a/tests/test_album_bundle.py +++ b/tests/test_album_bundle.py @@ -360,6 +360,40 @@ def test_copy_audio_files_atomically_creates_staging_dir(tmp_path: Path) -> None assert staging.exists() +def test_copy_audio_files_atomically_keeps_source_by_default(tmp_path: Path) -> None: + """Default (torrent/usenet): originals stay put (client keeps seeding).""" + src = tmp_path / 'a.flac' + src.write_bytes(b'a') + staging = tmp_path / 'staging' + out = copy_audio_files_atomically([src], staging) + assert len(out) == 1 + assert src.exists() # source retained + + +def test_copy_audio_files_atomically_removes_source_when_requested(tmp_path: Path) -> None: + """#796: Soulseek path removes slskd's completed files once staged so they + don't pile up in the download folder.""" + src_a = tmp_path / 'a.flac'; src_a.write_bytes(b'a') + src_c = tmp_path / 'c.flac'; src_c.write_bytes(b'c') + staging = tmp_path / 'staging' + out = copy_audio_files_atomically([src_a, src_c], staging, remove_source=True) + assert len(out) == 2 # both staged + assert not src_a.exists() and not src_c.exists() # sources removed + assert sorted(Path(p).name for p in out) == ['a.flac', 'c.flac'] + + +def test_copy_audio_files_atomically_keeps_source_when_copy_fails(tmp_path: Path) -> None: + """Data safety: a source whose copy FAILS must never be deleted, even with + remove_source=True.""" + src_ok = tmp_path / 'ok.flac'; src_ok.write_bytes(b'ok') + src_missing = tmp_path / 'gone.flac' # never created -> copy fails + staging = tmp_path / 'staging' + out = copy_audio_files_atomically([src_ok, src_missing], staging, remove_source=True) + assert len(out) == 1 # only ok staged + assert not src_ok.exists() # staged source removed + assert not src_missing.exists() # never existed (and not created) + + # --------------------------------------------------------------------------- # Config-driven poll cadence # --------------------------------------------------------------------------- diff --git a/tests/test_soulseek_album_fallback.py b/tests/test_soulseek_album_fallback.py index 5e2cce07..78f97e5b 100644 --- a/tests/test_soulseek_album_fallback.py +++ b/tests/test_soulseek_album_fallback.py @@ -83,7 +83,7 @@ def test_healthy_folder_does_not_fall_back(): # True. Patch the atomic copy to echo the completed paths through. from pathlib import Path completed = [Path("/staged/01.flac"), Path("/staged/02.flac")] - with patch("core.soulseek_client.copy_audio_files_atomically", lambda files, dest: list(files)): + with patch("core.soulseek_client.copy_audio_files_atomically", lambda files, dest, **kw: list(files)): stub = _Stub(completed) with patch("core.soulseek_client.run_async", lambda x: x): result = SoulseekClient.download_album_to_staging(