fix(library): stop deleted songs coming back - #527
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:02
This was referenced Aug 31, 2026
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.
Fixes #521. Targets
fix/520-registry-phantom-job, not0.16.1directly -- it builds on that branch's hardening ofrestore(). Merge #526 first.Reported on macOS: a song deleted by clearing the trash, or via Settings -> "Reset app data", comes back later.
Root cause
restore()adopts any job-shaped directory it finds. That is correct 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.That is why the reset path was the worst one: it destroys the only defence, then trusts an unconditional
{"ok": true}.Changes
Server keeps its own deletion record.
mark_deleted()records an id; orphan recovery skips it;persist()writes it alongside the jobs._prune_deleted()forgets a record once its directory is finally gone, so the set stays bounded instead of growing for the life of the install. With this, the client tombstone becomes belt-and-braces rather than load-bearing._rmtree_jobreports its outcome instead of logging and returningNone, and retries once. On macOS the usual failure is Finder or Spotlight creating a.DS_Storebetweenrmtree's scan and its finalrmdir, which leaves "Directory not empty" on a directory that is about to be empty again -- a retry clears exactly that.delete_jobrecords the deletion either way, and returns an error when files remain rather than reporting success.reset_allreturns what it could not remove and records the survivors so they cannot be re-adopted./api/resetnow returns{"ok": true, "undeleted": N}.Client awaits both halves.
markJobsDeletedis awaited before the purge, and the DELETE calls are awaited with failures surfaced throughnotifyFailure. Both were fire-and-forget with.catch(() => {}). The button is disabled while it runs.Verification
New
tests/test_deleted_jobs_stay_deleted.py, 7 tests. Confirmed not vacuous -- removing the orphan-recovery skip makes 2 fail:The first test in the file deliberately pins the behaviour we must not break: an orphan directory that was never deleted is still adopted.
Two things reviewers should weigh
delete_jobnow returns 500 when files remain, after having already removed the registry entry and recorded the deletion. The row goes and stays gone -- which is what the user asked for -- but the response is an error so the failure is visible. The alternative was keeping the row and forcing a retry; that felt worse given the.DS_Storerace usually clears on retry anyway./api/reset's response shape changed.tests/test_reset.pywas updated for the newundeletedfield. Any other consumer of that endpoint would need the same.Still unconfirmed
The macOS
.DS_Storerace is a hypothesis, not a verified root cause. Checkingjobs/for surviving directories andbackend.logforreset: could not removeafter a reset that appeared to work would settle it. The fix does not depend on that being the trigger -- the deletion record closes the resurrection path whatever caused the delete to fail.