A task stuck in 'post_processing' past the cutoff was force-marked 'completed'
("assume it worked"). In a large batch, post-processing (AcoustID + quality +
import) is serialized and backs up, so tasks sit in post_processing while merely
QUEUED — then got falsely completed, showing as downloaded with no file on disk
(/Transfer empty).
Now: the cutoff is 30 min (was 5) so legit backlog isn't cut off, and when it
does fire the task is only completed if it actually produced a file
(final_file_path exists on disk) — otherwise marked failed (honest + retryable).
Applied at both stuck-detection sites (check_batch_completion + _v2).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The reconcile read each completed task's final_file_path to find paths — but not
every import path sets it (the verification worker marks the task completed
without it), so tracks that imported via that path were silently dropped (user
saw 3 of 5 symlinks). Root cause: leaning on a fragile per-task field.
Now reconcile_batch_playlists identifies the organize playlists the batch touched
(its own + any reached via a completed track's source_info provenance) and
rebuilds each from CURRENT library ownership via _rebuild_one_from_db
(check_track_exists over membership). It just asks the library what's owned, so
it's robust to HOW a track imported (modal worker / slskd monitor / verification
worker) and still prunes tracks that left. Takes a db handle; all three callers
pass MusicDatabase().
Reconcile tests rewritten for the DB-rebuild form (organize batch, wishlist
provenance, non-organize skip, plain no-op). 973 downloads/imports/playlist
tests pass.
on_download_completed and check_batch_completion_v2 are duplicate completion
paths. Monitor-detected downloads (Deezer / slskd-monitor / verification-worker
imports) finish the batch via the V2 path, but the materialize reconcile was
only added to on_download_completed — so those batches never built playlist
folders (no '[Playlist Folder] Rebuilt' line at all). Add the same non-fatal
reconcile to the V2 path. Now all three completion points (both lifecycle paths
+ the master.py all-owned path) materialize. 550 tests pass.
Replaces the two organize-only triggers with a single reconcile_batch_playlists
called at both batch-completion points. It groups the batch's newly-resolved
tracks by their per-track playlist provenance:
- the batch's OWN organize playlist → full (re)build with prune, and
- a track that completed for a DIFFERENT playlist (e.g. a WISHLIST fulfilling a
track that belongs to an organize playlist) → ADDED to that folder, no prune.
So a late wishlist arrival now lands in its playlist folder immediately, instead
of only on the next sync/manual rebuild — the folder = the playlist's owned
members, kept true on every ownership change regardless of download path. Uses
the paths the batch already captured (no DB re-match, no waiting on the server
scan/sync). Non-fatal.
3 new reconcile tests (organize full-rebuild, wishlist add-without-prune, plain
batch no-op). 983 downloads/imports/playlist tests pass.
- Routing (step 5): organize-by-playlist tracks no longer set the per-track
_playlist_folder_mode flag, so they import NORMALLY into Artist/Album — exactly
what a normal download does. _playlist_name provenance is kept (origin.py).
- Triggers (step 4): build the playlist folder from the batch's own payload at
both end-of-flow points — the all-owned path in master.py (no downloads, so the
lifecycle never runs) and the batch-complete hook in lifecycle.py (after
downloads). Both gated on playlist_folder_mode, both non-fatal.
Works for the all-owned case (the smack test that did nothing before) and for
mixed owned/downloaded, with no source-ID or mirrored-playlist dependency. The
materialized folder uses the default ./Playlists root + symlink mode until the
Settings UI is added.
Updated the master test to assert the new contract (provenance kept, routing
flag gone). 979 tests pass.
Two related leaks in ``storage/album_bundle_staging/<batch_id>/``:
1. **Soulseek bundle cleanup was excluded.** The per-batch cleanup
at the end of a bundle download gated on:
(album_bundle_source or '').lower() in ('torrent', 'usenet')
The comment justified it as "slskd keeps its own completed
folders" — but the Soulseek bundle path ALSO copies completed
files into the private staging dir (``soulseek_client.py:1599``,
``copy_audio_files_atomically(completed, Path(staging_dir))``)
for the per-track workers to claim. Those copies persisted
forever; long-running installs accumulated stale GB. Extended
the cleanup gate's allow-list to include ``soulseek`` so the
per-batch dir is removed on bundle completion — same code path
that already worked for torrent / usenet.
2. **No sweep for orphan dirs.** Any leftover ``<batch_id>``
subdir from a previous-session crash, an errored batch, or a
pre-fix Soulseek bundle stayed on disk forever. Added
``sweep_orphan_album_bundle_staging(staging_root, active_batch_ids)``
that runs ONCE at server startup, before any batch can register
a staging dir. Removes every ``<batch_id>``-shaped subdir
whose id isn't in the active set. Safe by construction:
- Only touches subdirs of the configured staging root.
- Name-shape check (``entry.name == _safe_batch_dirname(entry.name)``)
rejects hand-placed dirs like ``.git`` or stray docs.
- ``shutil.rmtree`` errors log + continue — sweep must not
crash app startup over a permission glitch.
- active_batch_ids normalised through ``_safe_batch_dirname``
so colon-bearing batch_ids match their on-disk form.
Wired into the web_server startup right after the stuck-flags
diagnostic so it fires before anything else touches batches.
Tests
- ``test_downloads_lifecycle.py`` gained one regression test
pinning that Soulseek bundles now have their staging dir
cleaned (sibling to the existing torrent test).
- ``test_album_bundle_staging_sweep.py`` (NEW, 11 tests)
covers: orphan removal with no actives, active dirs preserved,
special-char batch_id normalisation, no-op on missing /empty
/empty-string staging root, non-dir entries skipped, unsafe-
name dirs preserved (.git etc.), partial rmtree failure doesn't
abort the rest, listdir failure returns 0 cleanly, default
None active set, defensive against empty / None entries in
the active set.
488 downloads tests pass.
For users with an existing "clean up old files" automation pointed
at this dir: stop pointing it there if you want — the auto-cleanup
+ startup sweep cover it now. Or leave it as belt-and-suspenders
with a relaxed (1h+) mtime threshold so it can't race a mid-batch
download.
Adds torrent/usenet as release-oriented download sources with album-bundle staging, live progress reporting, and post-processing that selects the requested audio file from completed releases instead of blindly importing the first file.
Keeps album-bundle behavior gated to single-source torrent/usenet album downloads, excludes release sources from hybrid album per-track searches, and allows hybrid non-album tracks to use release results safely.
Improves staged-release matching for featured/bonus track filenames while preserving version mismatches, records torrent/usenet provenance in library history, and updates service/status UI labels.
Covers the flow with focused lifecycle, status, staging, validation, task worker, post-processing, and import side-effect tests.
Do not mark a monitored transfer as successful as soon as slskd reports completion. The monitor now only submits the post-processing worker; that worker reports the real success or failure after finding, verifying, and importing the file. If post-processing cannot be scheduled, mark the task failed and release the batch slot. Add a regression test for the premature success path.
- Switch the download lifecycle over to the neutral wishlist track helper name
- Keep the old Spotify helper as a compatibility alias for older callers
- Store track_data as the primary failed-download wishlist payload key and add regression coverage
Seventh sub-PR in the download orchestrator series. Strict 1:1 lift —
zero behavior change. ~570 lines moved out of web_server.py.
What moved (lifted as 3 tightly-coupled functions in one module):
- _start_next_batch_of_downloads → start_next_batch_of_downloads
- _on_download_completed → on_download_completed
- _check_batch_completion_v2 → check_batch_completion_v2
Dependencies bundled in `LifecycleDeps` (15+ refs):
- config_manager, automation_engine, download_monitor, repair_worker,
mb_worker (live globals)
- is_shutting_down (lambda over IS_SHUTTING_DOWN flag)
- get_batch_lock (web_server helper for batch_locks dict)
- submit_download_track_worker (lambda wrapping
missing_download_executor.submit + _download_track_worker)
- submit_failed_to_wishlist + submit_failed_to_wishlist_with_auto_completion
(async, used by on_download_completed) AND process_failed_to_wishlist
+ process_failed_to_wishlist_with_auto_completion (sync, used by
check_batch_completion_v2 — direct call matches original v2 behavior;
the non-v2 path always submitted to executor)
- ensure_spotify_track_format, get_track_artist_name,
check_and_remove_from_wishlist, regenerate_batch_m3u (web_server
helpers — large, will lift in follow-up PRs)
- youtube_playlist_states, tidal_discovery_states,
deezer_discovery_states, spotify_public_discovery_states
(per-source playlist state dicts — phase transitions on batch
completion)
Direct imports for already-lifted helpers:
- core.runtime_state.{download_tasks, download_batches, tasks_lock,
add_activity_item}
- core.downloads.history.record_sync_history_completion (PR4a)
- core.album_consistency.run_album_consistency
- core.metadata.common.get_file_lock
Behavior parity verified line-by-line:
- start_next_batch: same batch lock acquisition, same shutdown gate,
same V2-cancelled-task skip, same searching-status-set-before-submit,
same submit-fails-no-ghost-worker semantics
- on_download_completed: same duplicate-call detection (skip decrement
but still check completion), same failed-track tracking with
spotify_track formatting + activity items + automation event emission,
same wishlist removal on success, same active_count decrement, same
stuck-detection (searching > 10min → not_found, post_processing >
5min → completed), same M3U regeneration + repair worker hand-off
+ album consistency pass + wishlist failed-tracks submission
- check_batch_completion_v2: same finished-count tally, same stuck
detection, same already-complete short-circuit returning True, same
per-source playlist phase updates, same album consistency pass,
same DIRECT (sync) wishlist processing call (NOT submit-to-executor
— matches original v2 which called process_* functions directly)
CRITICAL drift caught + fixed during review:
- Initial lift had v2 routing wishlist calls through submit_* deps
(async). Original v2 called process_* directly (sync). Added separate
process_* deps to LifecycleDeps and routed v2 to them. Tests updated.
Two minor defensive additions documented:
- `is_auto_batch = False` initialized before conditional in v2 (Python
scope rules made this unnecessary in original, but explicit is safer)
- Variable rename inside the queue-completion-check loop in
on_download_completed: `task_id` → `queue_task_id` to avoid shadowing
the outer parameter. Log output preserves the same task ID.
Tests: 28 new under tests/downloads/test_downloads_lifecycle.py
covering start-next (early-returns, shutdown gate, max_concurrent,
cancelled-task skip, searching-status-set, submit-failure-no-ghost,
orphan task), on-complete (decrement, duplicate skip, failed/cancelled
tracking, automation emit, wishlist removal, batch completion + emit
+ source phase update, stuck detection, auto vs manual routing),
check-v2 (missing batch, not-complete, complete-marking, already-
complete, auto routing, exception handling).
Full suite: 1029 passing (was 1001). Ruff clean.
