fix(library): stop deleted songs coming back (recovers #527) - #540
Merged
Conversation
Reported on macOS: a song deleted by clearing the trash, or by Settings ->
"Reset app data", returns later. Deletion had two halves and both swallowed
their failures, so several independent paths produced the same symptom.
The root cause is that restore() adopts any job-shaped directory it finds.
That is right for a library whose registry was lost and wrong for a job the
user deleted whose files outlived the delete. Nothing on the server knew the
difference, so the only thing standing between a failed delete and a
resurrected song was a client-side tombstone -- which "Reset app data" wipes
on its way out.
The registry now keeps its own deletion record. Orphan recovery skips those
ids, so a directory that survives a delete stays gone regardless of what the
client does. Records are pruned once their directory is finally absent, so the
set stays bounded rather than growing for the life of the install.
_rmtree_job reports whether the files actually went away instead of logging
and returning None, and retries once: on macOS the usual failure is Finder or
Spotlight creating a .DS_Store between rmtree's scan and its final rmdir,
which leaves "Directory not empty" on a directory that is about to be empty
again. delete_job records the deletion either way and tells the caller when
files remain.
reset_all returns what it could not remove and records the survivors, and
/api/reset reports the count instead of an unconditional {"ok": true} that the
frontend took as licence to wipe its own tombstone.
On the client, the tombstone write and the DELETE calls are both awaited. They
were fire-and-forget with .catch(() => {}), so quitting soon after clearing the
bin lost the tombstone, and a delete that failed -- a 409 on a job stuck in
"queued", a 500 when files could not be removed -- was invisible. Failures now
surface through notifyFailure, and the button is disabled while it runs.
Verified: removing the deletion record makes the two resurrection tests fail.
Refs #521
thcp
marked this pull request as ready for review
August 31, 2026 21:38
This was referenced Aug 31, 2026
thcp
added a commit
that referenced
this pull request
Sep 1, 2026
…mmend a shape (#545) A trashed song came back on the next launch whenever a second job shared its source URL. addTrackToLibrary evicted the trashed track to make room for the sibling's import, which dropped the catalog entry but not the job, so the directory and its registry record outlived their only reference. syncWithServer then found a job with no track, no trash entry and no tombstone, and re-adopted it. The trashed match is now left where the user put it: the new track is in no folder yet, so it reaches the library on its own, and evicting the old one was never what put it there. A Playwright spec seeds two jobs on one source URL and holds both halves, and seed.py grows a sibling job to make that possible. Three of the eight logos listed in the We Recommend dialog had no file behind them and 404'd for every user on every platform. They are bundled now, along with avatars for Beltr and Seratone, which had none at all. The dialog itself was one flat list of twelve entries with no order a reader could perceive. It is grouped into five categories, r/bass is added so the app matches the README, and the descriptions move out of hardcoded English into the i18n layer across all ten language tables. The README table gains the same categories so the two lists stop drifting apart. library.deleteFailed, English-only since #540, is filled in at the same time because the i18n audit cannot pass without it. Co-authored-by: Thales <>
thcp
added a commit
that referenced
this pull request
Sep 1, 2026
…om the bin, and 14 more fixes (#541) Ships the 0.16.1 fix set to `main`. Eighteen merged changes: sixteen fixes from a pre-release bug scan, one refactor, and one feature. ## Why this release exists #506 fixed the macOS AppleDouble bug but is unreleased, so every macOS 0.16.0 user still has a broken runtime: `import matplotlib.pyplot` fails, which kills `allin1_infer`, which kills automatic song sections. Overwriting the 0.16.0 assets would not reach them, the runtime reinstall is gated on a version-string comparison (`desktop/ui/setup.js`), so a new version is the only route. ## What is in it **Data loss and user-visible bugs** - **#509** `settings.json` was written with `write_text` (truncate, then write), and `_load` could not tell a torn file from a first run. A real user lost `port` and `allow_network` from both the file and its mirror. Now atomic, and a corrupt file is preserved as `settings.json.corrupt-<ts>` and recovered from the mirror. - **#521** Deleted songs came back. `reset_all` swallowed per-directory failures, `/api/reset` reported unconditional success, the frontend then wiped its own tombstone, and `restore()` re-adopted every surviving directory on the next start. A server-side deletion record closes it. - **#542** Trashed songs came back too, by a different route. `addTrackToLibrary` deduplicates by source URL, and when the match was in the Trash it deleted the catalog entry without deleting the job. The directory and its registry record outlived their only reference, and `syncWithServer` re-adopted the orphan on the next launch. Any second job sharing the URL was enough to trigger it. Found while testing this release on Windows. - **#515** The footer scrub bar did nothing, and "set loop in at playhead" always wrote 0, `main.js` drove the silent multitrack while `audioEngine` owned the clock. - **#520** A cancel landing between the queue worker's pop and claim stranded a job at `queued` forever: invisible, still counted against capacity, source file never freed, re-queued on every restart. Also, a malformed `registry.json` raised an uncaught `AttributeError` at import and the backend never started. **Security** - **#510** The in-app updater installed an executable from a WebView-supplied URL with no host allowlist, checked against a SHA from the same caller. - **#511** Fork PRs executed arbitrary code on the self-hosted runners, the same machine that builds and signs releases. - **#518** Linux FFmpeg was downloaded, chmod +x and executed with no integrity check at all. - **#517** Deno pulled from `releases/latest` unpinned and unverified into every image; releases could publish without updater assets and stay green; a failed CPU-torch install was silently ignored, shipping a non-CPU torch in the CPU zip. **Robustness** - **#508** A regression in #506 itself: the per-entry unpack loop lost `Archive::unpack`'s directory deferral, so a read-only directory member would fail extraction outright. - **#512** `end` had no upper bound, reaching a multi-GB `np.zeros` on the event loop; the body-size guard covered two paths and was bypassed by chunked encoding. - **#513** SSE slots leaked permanently when a client disconnected before the body started, 200 of those and every progress stream 503s with nothing connected. - **#514** Worker teardown sat outside the `finally`, so an exception left a poisoned CUDA worker warm; cancel was dropped before the CPU fallback, costing 10+ minutes. - **#516** `child_output_with_timeout` never drained child pipes until exit, deadlocking any chatty child. - **#519** Cancellation never reached several pipeline subprocesses, and two of three workers never armed the parent-PID watchdog, so a Force-Quit orphaned a GPU-holding process. **Presentation** - **#543** Three of the eight logos listed in the We Recommend dialog had no file behind them. Analog4Lyfe, Empress Effects and Thomann showed a broken card on every install, with a 404 in the backend log each time the dialog opened. - **#544** That dialog was one flat list of twelve entries with no order a reader could perceive. It is grouped into five categories now, `r/bass` is added so the app matches the README, and the twelve descriptions move out of hardcoded English into the i18n layer across all ten language tables. **Feature** - **#538** Loop regions can be adjusted rather than redrawn, drag either edge independently, or drag the region to slide it. From discussion #507. ## Verification ``` ruff check All checks passed ruff format 101 files already formatted pytest tests/ 947 passed, 2 failed npm run test:js 11/11 playwright 85 passed i18n audit clean cargo fmt clean cargo clippy 0 errors cargo test 60 passed, 1 failed ``` Every fix was individually confirmed present on this branch by grepping for its introduced symbol, rather than trusting merged state, which is how #527 was caught having merged into an orphaned branch instead of the release branch (recovered as #540). **The 3 failures are all pre-existing and reproduce on `main`:** - `test_all_stems_zip_ogg` and `test_ogg_is_still_streamed`, most likely a local ffmpeg built without libvorbis. Unconfirmed, not filed. - `a_free_port_is_granted_as_asked`, a known parallel-execution flake. It probes port 21000 with `std::net::TcpListener` (which sets `SO_REUSEADDR`) then asserts `claim_port` (socket2, without it) binds the same port. Not equivalent, and there is a TOCTOU gap. ## Tested on a real Windows build The branch was packaged with `make-portable.ps1 -CpuOnly` and driven by hand. What that covered: - **#509** port and `allow_network` survived a quit and relaunch, in both the portable file and the AppData mirror. - **#521** a hard-deleted job stayed deleted across a restart. The deletion record self-pruned once the directory was gone, which is the designed behaviour. - **#542** a trashed job stayed in the Trash across a restart, and again across a fresh import of the same URL. Both cases fail on 0.16.0. - **#520** a cancel while queued removed the job and freed its capacity slot, with nothing stranded. - **#519 / #514** a cancel mid-separation wiped the partial output and the queued job started immediately, with no leftover worker process. A quit mid-separation left zero orphaned `python.exe`, and the interrupted job resumed once with `resume_attempts: 1`. ## Two things reviewers should know before tagging **`make-portable.ps1` now has a parser pass and a real run.** It parses clean under Windows PowerShell 5.1, and the full CPU-only Windows package built end to end from this branch, exit code 0. The earlier caveat here is resolved. **Nothing compiles the Linux Rust shell until release time** (#531). `ci.yml` never invokes cargo; Linux Rust is built only by `linux-release.yml`. #518 ships a change living entirely inside `#[cfg(all(unix, not(target_os = "macos")))]`, which could only be type-checked by temporarily widening the cfg gate. If the release build fails, look there first. **#510 is not manually testable.** The update check runs automatically at startup and only surfaces when GitHub has a newer non-prerelease release, so the host allowlist in the installer is not reachable by hand. It exercises itself at release time. ## Not included The Unraid template still pins `0.16.0`, deliberately left for a separate decision.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings #527 into
0.16.1. It never landed there, despite being marked merged.What happened
#527 was stacked on
fix/520-registry-phantom-job(#526). The intended order was #526 then #527, but merging #526 first movedfix/520into0.16.1and left #527 still targetingfix/520. When #527 was then merged, its commits went onto that now-orphaned branch instead of the release branch.GitHub reports #527 as MERGED, which is true -- just not into anywhere that ships:
Caught while preparing the
0.16.1->mainrelease PR, by checking that each fix's code was actually present on the branch rather than trusting the merged state.Content
Unchanged from #527, rebased onto current
0.16.1. No conflicts.This is the fix for the reported bug where a song deleted via Settings -> "Reset app data" comes back:
reset_allswallowed per-directory failures,/api/resetreported unconditional success, the frontend then wiped its own tombstone, andrestore()re-adopted every surviving directory on the next start. The server-side deletion record is what closes it.Verification
tests/test_deleted_jobs_stay_deleted.pypasses 7/7 on top of the other 14 merges.Note
fix/520-registry-phantom-jobshould be deleted after this merges, so nothing is tempted to merge it again.