Conversation
macos-check.yml and windows-check.yml trigger on pull_request and run on
[self-hosted, macOS, ARM64] and [self-hosted, windows, x64] with no fork
guard, on a public repository.
cargo fmt/build/clippy all execute code the pull request supplies: build.rs,
proc-macro crates, a swapped Cargo.toml dependency or [patch], and the test
bodies themselves. permissions: {} limits what the token can reach; it does
nothing about code execution on the machine.
That machine is the same one that builds, signs and uploads every macOS DMG
and runtime pack. Neither check workflow cleans its workspace and the release
workflows only remove .build and dist, so an implant in ~/.cargo, ~/.npm,
~/.rustup or the persistent _work tree would survive straight into the next
release. GitHub's public-repo default only gates first-time contributors, so
one trivial merged pull request is enough to unlock this for a later one.
Gate both jobs on the pull request coming from this repository. The
workflow_dispatch path is unaffected, so a fork's Rust change can still get
its compiler pass when a maintainer asks for one -- which keeps the reason
the trigger exists (#421: cfg-gated Rust merging without ever being compiled,
because ci.yml is entirely ubuntu-latest).
Refs #511
#506 replaced Archive::unpack with a per-entry unpack_in loop so AppleDouble "._" members could be skipped. That loop reproduced the iteration but not the two things unpack does around it. Directories are now applied last, reverse-sorted by path, the way Archive::_unpack does (tar-rs#242): a directory carries its own mode, so creating it inline in archive order means a 0o555 member exists before its contents are written and the next file inside it fails with EACCES. On a runtime pack that contains one, first-run setup dies with no fallback. `ditto` and `tar` both record such modes faithfully, so this was reachable. `destination` is canonicalized before the loop again, which on Windows supplies the \\?\ prefix so member paths over 260 characters still extract. Traversal protection was never affected and is unchanged: unpack_in rejects ParentDir components, strips RootDir/Prefix, and canonicalizes against dst on every entry, so zip-slip, absolute members and symlink escapes stay blocked. This was a robustness regression, not a security one. Deferring costs nothing on a streaming archive: directory entries carry no data, so only their metadata is applied in the second pass. The new test builds a .tar.zst whose directory member is 0o555 and whose file member follows it, which is the order that broke. Verified it fails against inline creation and passes with the deferral. Refs #508
_save() called write_text, which truncates first and writes second. A process
that died in between -- app quit, the parent watchdog's SIGTERM, power loss --
left settings.json present and unparsable.
_load() then could not tell that apart from a first run: it caught the parse
error, returned {}, and the next set_*() persisted a single key over both
settings.json and the per-user mirror that exists to protect it. A real user
lost port and allow_network from both copies, with only a warning in the log.
jobs_dir is the worse case, since losing it makes a relocated library look
empty -- precisely what the mirror was added to prevent.
Three changes:
- _atomic_write_json() writes through a uniquely-named same-directory temp and
replaces, so an interrupted write cannot truncate what was already there.
_save() and _mirror_settings() both use it; the mirror previously used a
fixed ".json.tmp" name that two writers could interleave on.
- _load() distinguishes absent from unusable. Absent stays a first run. A file
that exists but does not parse -- or parses to something other than an
object -- is moved aside as settings.json.corrupt-<timestamp>, so the bytes
survive for diagnosis instead of being overwritten by the next save, and the
mirror is consulted before falling back to defaults.
- Settings recovered from the mirror are written straight back to the primary.
Recovering only into memory would last until the next start, which would
read a now-absent primary and quietly return to defaults again.
The tests cover the exact sequence that lost data: a torn write, then a
restart. Four of them fail against the old _load and pass against this one.
Refs #509
…oot with a bad registry Two ways the registry stranded state. A cancel arriving between the worker's _pop_next() and _set_running() found the job in neither the queue nor the running slot. discard() returned False so cancel_job never finalised it, running_id() was None so nothing was terminated, and the worker then dropped it silently. The job stayed at "queued" forever: absent from the queue view, still counted by pending_count against the capacity limit, its uploaded source (up to 400 MB) never freed, and -- because "queued" is persisted -- re-queued on every restart. The worker is the sole consumer and owns the job once it has popped it, so it now closes out the cancellation itself via _finalise_dropped_job(). Extracted rather than inlined so the drop path can be tested directly, and so an already-terminal job is explicitly left alone rather than having a real result rewritten. Separately, restore() caught (OSError, JSONDecodeError, TypeError, ValueError). A registry.json that was valid JSON but not an object -- a top-level list, null, a bare string -- makes _migrate call data.get() and raise AttributeError, which that tuple does not name. restore() runs at import time, so the backend simply never started, with no self-healing path and no way for a user to recover short of deleting the file by hand. Orphan recovery and the persist that follows it sat outside the guard entirely, so an unreadable jobs_dir was fatal at startup for the same reason. Both are now caught broadly and logged. A registry we cannot use should cost the user their job list, not their app. The cancel test drives the real worker loop rather than calling the helper, so it also fails if the worker stops calling it. Verified: reverting the worker's call fails that test, and reverting restore()'s except fails five others. Refs #520
claim_sse_slot() runs in the handler and release_sse_slot() lived in the stream's finally. An async generator that is never started never runs its finally, so a client that disconnected before the response body began held a slot for the life of the process: StreamingResponse raises inside stream_response on its first send(), before __anext__ is ever called, and the generator body never executes. Two hundred of those and every job-progress stream and the queue stream answer 503 with nothing actually connected, until a restart. It does not need malice either -- a flaky network or a rapidly reloaded page leaks slots the same way, just slower. Claiming cannot simply move inside the generator: by the time it runs the response headers have gone out and there is no status code left, so hitting the cap could no longer answer 503. SseSlot keeps the claim in the handler and makes release idempotent, with __del__ as the backstop for the never-started case -- collecting the generator collects the closure holding the slot. _held is assigned before the claim because __del__ runs on a half-built object too: a refused claim would otherwise raise AttributeError out of __del__ rather than releasing nothing. The test for that caught it. queue.py takes the same guard, so both streams that share the budget also share the fix. Verified: removing the __del__ backstop fails the never-started test. Refs #513
…loop Four related holes, all reachable with one unauthenticated request. The trim range had no ceiling. `end` is a float that reaches np.zeros(int(round(duration * sample_rate))) in the click renderer, so ?start=0&end=20000&count_in=1 asked for roughly 7 GB, and another 7 GB in the int16 conversion. A larger value raised MemoryError inside a blanket except, which silently shipped the export with no click rather than failing. _validate_trim_range bounds it by the job's own recorded duration, with a six-hour backstop for a job whose duration was never recorded, and a second of slack because ffprobe's duration can sit a hair under the decoded length. The click render ran synchronously inside async handlers, so all of that allocation and a Python loop over every beat blocked the event loop -- every SSE progress stream and the queue worker stalled behind it. It goes through asyncio.to_thread now. The buffer is float32 rather than float64: the output is 16-bit PCM, so the extra mantissa was never audible and a long export was allocating twice what it needed. The click cache pruned without keep=, so a render larger than the cache budget evicted itself the instant it was written and ffmpeg was handed a missing -i. That is the #482 bug, unfixed on this path. The mixdown write had the same gap against a concurrent render's prune. The body-size guard was scoped to paths ending /sections or /beats, leaving /api/search, /api/playlist, /api/settings and the JSON branch of /api/jobs uncapped -- Starlette buffers the whole body, then json.loads runs it on the event loop. A 200 MB body to /api/search stalled every other request with no valid job or prior state needed. It now applies by method, exempting multipart uploads, which stream to disk under their own 400 MB limit. A chunked request with no Content-Length used to skip the check entirely and fall through to an unbounded request.body(); it gets a 411 now. Six of the eight new tests fail against the old code. Refs #512
… before the CPU retry The persistent worker teardown sat after the finally block, not inside it. An exception out of the read loop -- proc.stderr.read(1) raising OSError when the API thread's terminate() races the read, or _set() raising -- propagated without it, so _worker still held the process and the next job's _get_worker() saw a matching device and a live poll() and reused a worker whose CUDA state followed an exception. ml-pipeline.md is explicit that any non-success must tear it down. A second path missed it entirely: the pipe check raises before the try, so a worker that came back without stdin/stderr stayed cached and would be handed to every subsequent job. Found while writing the test for the first one. separate() also had no cancel check between its two attempts. The rmtree of a multi-GB partial result takes seconds and nothing is registered for cancel during it, so a cancel landing there was invisible and the full CPU pass ran to completion -- 10+ minutes -- before JobCancelled was finally raised. The UI showed "Cancelling" throughout. _kill_worker now reaps after kill(). Without communicate() a worker wedged in an uninterruptible CUDA call becomes a zombie whose pipes close only when the Popen refcount happens to drop; vocal_split.py already pairs the two. An entry-point cancel check was tried and removed. It broke test_cancel_kills_worker_next_job_spawns_fresh, which verifies that a cancelled job tears the worker down so the next one spawns fresh -- a check that never spawns has no worker to tear down, and that test encodes the rule this change is meant to protect. The queue worker already gates dispatch on cancellation, so the fallback check covers the gap that was actually reported. Verified: reverting each half fails its test. Refs #514
Three stages ignored cancel entirely. The local-upload ffmpeg calls used subprocess.run(), which cannot be interrupted: POST /cancel sets the flag but nothing looks at it until the call returns. Cancelling during "Preparing audio..." on a 400 MB .mp4 was a no-op for up to TIMEOUT_FFMPEG per call, twice over on that path since it runs both the video extract and the transcode. Both go through _run_registered_ffmpeg now, mirroring collect._run_ffmpeg, which registers for exactly this reason. Only demucs_worker armed the parent-death watchdog, and only separate.py exported STEMDECK_PARENT_PID. A Force-Quit during a vocal split therefore orphaned an onnxruntime process holding the GPU with nobody to collect the result, and a section pass outlived the parent whose TIMEOUT_SECTIONS was its only bound. The watchdog moves to app/core/process.py -- where process_exists already lived for it -- and all three workers arm it, all three spawn sites export the pid. Poll interval stays 1.0s, matching what demucs_worker used. A running vocal split was uncancellable by construction: cancel_job returns early for a done job, and a split only ever runs on a done job, so the flag was never even set while the split held _pipeline_lock and stalled the import queue for its full duration. Cancel now terminates the worker for that case; the split's own error path marks it failed and releases the lock. _run_registered_ffmpeg deregisters in a finally, so a failure cannot leave a stale entry that a later cancel would terminate on the wrong job. Two existing test files needed updating rather than fixing: test_worker_parent_watchdog targeted demucs_worker._arm_parent_watchdog, now app.core.process.arm_parent_watchdog; test_video_status stubbed subprocess.run, which the extract no longer calls. Behaviour is unchanged in both. Refs #519
Fixes #511. `macos-check.yml` and `windows-check.yml` trigger on `pull_request` and run on `[self-hosted, macOS, ARM64]` / `[self-hosted, windows, x64]` with no fork guard, on a public repository. `cargo fmt`, `cargo build` and `cargo clippy` all execute code the PR supplies: `build.rs`, proc-macro crates, a swapped `Cargo.toml` dependency or `[patch]`, and the test bodies. `permissions: {}` limits the token, not code execution. That runner is the same machine that builds, signs and uploads every macOS DMG and runtime pack. Neither check workflow cleans its workspace, and the release workflows only `rm -rf .build dist` -- so an implant in `~/.cargo`, `~/.npm`, `~/.rustup` or the persistent `_work` tree survives into the next release. GitHub's public-repo default only gates *first-time* contributors, so one trivial merged PR unlocks this for a later one. ## Change Both jobs gain: ```yaml if: >- github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository ``` ## Why not move to GitHub-hosted runners That would lose the reason the trigger exists. `ci.yml` is entirely `ubuntu-latest`, and a Linux runner cannot type-check code behind `#[cfg(target_os = "macos")]` or `#[cfg(windows)]` at all -- it is stripped before semantic analysis. #421 added ~600 lines of mostly cfg-gated Rust and every CI check passed without compiling any of it. `workflow_dispatch` is unaffected, so a fork's Rust change can still get a real compiler pass when a maintainer triggers one. ## Verification Both files parse and the guard resolves as intended: ``` macos-check.yml if: github.event_name != 'pull_request' || ...head.repo.full_name == github.repository windows-check.yml if: github.event_name != 'pull_request' || ...head.repo.full_name == github.repository ``` Behaviour: `workflow_dispatch` runs (first clause), same-repo PRs run (second clause), fork PRs are skipped. Worth pairing later with a workspace clean at the start of the release workflows, so a compromised check run cannot persist into a build. Not done here to keep this change minimal.
…tor (#523) Fixes #508. Follow-up to #506 -- a regression introduced by that PR. #506 replaced `Archive::unpack` with a per-entry `unpack_in` loop so AppleDouble `._` members could be skipped. The loop reproduced the iteration but not the two things `unpack` does around it. ## 1. Directory deferral `Archive::_unpack` (tar-0.4.45 `archive.rs:245-265`): ```rust // Delay any directory entries until the end (they will be created if needed by // descendants), to ensure that directory permissions do not interfere with // descendant extraction. directories.sort_by(|a, b| b.path_bytes().cmp(&a.path_bytes())); ``` A directory carries its own mode. Created inline in archive order, a `0o555` member exists before its contents are written, and the next file inside it fails `File::create` with EACCES -- `unpack_without_apple_double` returns `Err` and first-run setup dies with no fallback. `ditto` and `tar` both record such modes faithfully, so this was reachable, not theoretical. Deferring costs nothing on a streaming archive: directory entries carry no data, so the second pass applies metadata only. ## 2. `destination` canonicalization Restored before the loop. On Windows this supplies the `\\?\` extended-length prefix so member paths over 260 characters still extract. ## Not affected: traversal protection `unpack_in` rejects `ParentDir` components, strips `RootDir`/`Prefix`, and calls `validate_inside_dst` -- which canonicalizes on every entry -- so zip-slip, absolute members and symlink escapes were blocked throughout. This was a robustness regression, not a security one. ## Impact Latent on shipped artifacts. The published 0.16.0 pack has no restrictive directory members: extracting it end to end produced 27,797 files and 0 sidecars. Windows takes the zip path for updates, so the long-path loss was theoretical there. Fixed because the next pack is not guaranteed to be as forgiving. ## Verification New test `extract_tar_archive_survives_a_read_only_directory_member` builds a `.tar.zst` whose `0o555` directory member precedes its file member -- the order that broke -- and asserts the file extracts and the directory keeps its archived mode. Confirmed the test is not vacuous: with directories created inline it **fails**; with deferral it **passes**. ``` cargo fmt --check OK cargo clippy 0 errors cargo test 59 passed, 0 failed ```
…gs (#524) Fixes #509. This is the bug that lost real settings: a user's `settings.json` went from five keys to one, dropping `port` and `allow_network` from both the file and its backup. ## Cause `_save()` used `write_text`, which truncates first and writes second: ```python _SETTINGS_PATH.write_text(json.dumps(_ensure()), encoding="utf-8") ``` Every other persistence path in the codebase already used temp+rename -- including `_mirror_settings()` eleven lines below, whose comment says "a torn write here would be restored verbatim into the user's next install", and `registry.persist()`. **The primary file was the only unprotected writer.** The sequence: 1. `_save()` truncates; the process dies mid-write. The file now exists and does not parse. 2. `_load()` caught the parse error and returned `{}` -- **indistinguishable from a first run**. 3. The next `set_*()` persisted a one-key file, then mirrored it **over the good backup**. `jobs_dir` is the worst case: `config._stored_jobs_dir()` falls back to the default and a relocated library looks empty, exactly the failure the mirror was added to prevent. ## Changes **`_atomic_write_json()`** -- uniquely-named same-directory temp, then `replace`. Used by both `_save()` and `_mirror_settings()`. The mirror previously used a fixed `.json.tmp` name that two concurrent writers could interleave on; that goes away too. **`_load()` distinguishes absent from unusable:** ``` parses -> use it absent -> defaults (genuine first run) exists, unparsable -> rename to settings.json.corrupt-<timestamp> seed from mirror if present, else defaults ``` Renaming rather than overwriting keeps the bytes for diagnosis. Deleting them, or letting the next save clobber them, destroys the only evidence of what the user had configured. A file that is valid JSON but not an object (`[1,2,3]`) is treated as unusable too -- returning it would make every later `.get()` raise. **Recovered settings are written straight back to the primary.** Recovering only into memory would last until the next start, which would read a now-absent primary and silently return to defaults. ## Verification New `tests/test_settings_durability.py`, 8 tests covering the exact loss sequence. Confirmed not vacuous: reverting `_load()` alone makes **4 of them fail**: ``` FAILED test_an_unreadable_file_is_not_mistaken_for_a_first_run FAILED test_an_unreadable_file_is_kept_for_diagnosis FAILED test_recovered_settings_are_written_back_immediately FAILED test_a_non_object_settings_file_is_treated_as_unusable ``` ``` ruff check All checks passed ruff format already formatted pytest tests/ 901 passed, 2 failed ``` The 2 failures are `test_stems_api.py::test_all_stems_zip_ogg` and `::test_ogg_is_still_streamed`. **Both fail identically on `0.16.1` without this change** -- verified by checking out the base branch and re-running. They look like a local ffmpeg build without libvorbis, not a code defect, but worth a look separately. ## Note for reviewers `test_a_failed_write_leaves_the_previous_settings_intact` deliberately avoids `monkeypatch.undo()`. The same `monkeypatch` instance carries conftest's `_SETTINGS_PATH` isolation, so undoing mid-test points assertions at the developer's real settings file -- which is exactly what happened while writing this, and is worth knowing about for any future test in this area.
…oot with a bad registry (#526) Fixes #520. Prerequisite for #521 -- that fix builds on `restore()`'s hardening here. ## 1. A cancel in the pop-to-claim window stranded the job forever The worker pops at `jobqueue.py:213` and only claims with `_set_running()` at `:229`. In between, the job is in neither the queue nor the running slot. A cancel landing there: 1. `cancel_job` sets `cancel_requested = True` 2. `jobqueue.discard()` returns **False** -- already popped -- so it never sets `status="cancelled"`, never calls `cleanup_job_dir`, never persists 3. `running_id()` is still `None`, so the terminate branch is skipped 4. The worker sees the flag and `continue`s -- "drop it silently" The job stayed at `"queued"`: - **Permanent capacity loss** -- `pending_count` counts `status == "queued"`, so `register_if_capacity` counts it forever - **Invisible** -- not in `_queue`, not running, so absent from the queue view - **Disk never freed** -- a queued upload holds its source, up to 400 MB - **Resurrected every restart** -- `"queued"` is in `_RESUMABLE`, hence `_PERSISTED` The worker is the sole consumer and owns the job once popped, so it now finalises the cancellation itself. Extracted as `_finalise_dropped_job()` rather than inlined, so the drop path is directly testable and so an already-terminal job is explicitly left alone rather than having a real result rewritten. ## 2. A malformed registry.json stopped the backend booting `restore()` caught `(OSError, json.JSONDecodeError, TypeError, ValueError)`. A `registry.json` that is valid JSON but not an object -- `[1,2,3]`, `null`, `"a string"` -- makes `_migrate` call `data.get()` and raise **`AttributeError`**, which that tuple does not name. `restore_registry(JOBS_DIR)` runs at module scope in `app/main.py`, so the import fails and the backend never starts. No self-healing, no user-visible recovery short of deleting the file by hand. Orphan recovery and the trailing `persist()` sat **outside** the try entirely, so an unreadable `jobs_dir` was fatal at startup for the same reason. Both are now caught broadly and logged. A registry we cannot use should cost the user their job list, not their app. ## Verification New `tests/test_registry_resilience.py`, 11 tests. Confirmed not vacuous, both halves independently: - Reverting the worker's `_finalise_dropped_job` call -> `test_cancel_between_pop_and_claim_finalises_the_job` **fails** - Reverting `restore()`'s except back to the tuple -> **5 tests fail** The cancel test deliberately drives the real `_worker_loop` rather than calling the helper, so it also catches the worker simply not calling it -- an earlier draft called the helper directly and did **not** catch that. ``` ruff check All checks passed ruff format 94 files already formatted pytest tests/ 904 passed, 2 failed ``` The 2 failures are `test_stems_api.py::test_all_stems_zip_ogg` and `::test_ogg_is_still_streamed`, which fail identically on `0.16.1` without this change (verified on the base branch). Local ffmpeg without libvorbis, most likely -- unrelated, and not filed.
download_app_update took its URL straight from the WebView and verified the download against a SHA-256 supplied by the same caller, so the checksum proved the bytes arrived intact -- not that they came from us. apply_app_update then extracts that archive over StemDeck's own executable and backend/ and relaunches. Reachability is narrower than it first looks: in the normal flow the URL is appAsset.browser_download_url from the GitHub API, no XSS was found, and the CSP is script-src 'self'. But the page is served over http by the Python backend, which Tauri treats as a remote origin, and these app-defined commands are not ACL-gated by the capability config -- both facts the code already documents. A LAN attacker reaches this once network access is enabled. validate_release_url pins the host to github.com and objects.githubusercontent.com (GitHub redirects release assets to the latter) and requires https, so bytes cannot be swapped in flight on a network where the page itself is already plain http. Deliberately a new function rather than the existing validate_download_url: that one permits only 127.0.0.1/localhost, serves a different caller, and would reject every legitimate release URL. apply_app_update also re-verifies now. It trusted that whatever sat at the archive path was what download_app_update had approved, so anything able to write into data/downloads between the two calls was extracted unchecked. The verified digest is recorded next to the archive and re-checked before extraction. The test covers host lookalikes (github.com.evil.example, notgithub.com) as well as plain rejection, since a substring check would pass those. Refs #510
) Fixes #510. ## The problem `download_app_update` took `plan.app_url` straight from the WebView and verified against `plan.app_sha256` from the same caller. A checksum supplied alongside the URL proves the bytes arrived intact, not that they came from us -- an attacker supplies both. `apply_app_update` then extracts that archive over `StemDeck.exe` and `backend/` and relaunches. ## Reachability Narrower than "remote code execution", but real: - In the normal flow the URL is `appAsset.browser_download_url` from the GitHub API - No XSS was found in the frontend audit; CSP is `script-src 'self'` with no `unsafe-inline` - **However**, `catalog.js:2386-2393` documents that the page is served over http by the Python backend -- a remote origin to Tauri -- and that these app-defined commands are **not ACL-gated** by the capability config. A LAN attacker reaches this once "Make StemDeck available on your network" is on. So: missing defence-in-depth on a code-execution path. ## Change ```rust const RELEASE_ASSET_HOSTS: [&str; 2] = ["github.com", "objects.githubusercontent.com"]; fn validate_release_url(url: &str) -> Result<(), String> { ... } ``` Both hosts are needed -- GitHub redirects release assets to `objects.githubusercontent.com`. `https` is required, so bytes cannot be swapped in flight on a network where the page itself is already plain http. **Deliberately a new function, not `validate_download_url`.** #510 originally suggested reusing that one; it permits *only* `127.0.0.1`/`localhost`, serves a different caller, and would reject every legitimate release URL. **`apply_app_update` re-verifies.** It trusted that whatever sat at the archive path was what `download_app_update` approved, so anything able to write into `data/downloads` between the two calls was extracted unchecked. The verified digest is recorded beside the archive and re-checked before extraction. ## Verification New test `only_our_own_release_assets_are_downloadable_as_updates`, covering host lookalikes a naive substring check would pass: ``` github.com.evil.example rejected notgithub.com rejected http://github.com/... rejected (scheme) file:///etc/passwd rejected ``` ``` cargo fmt --check OK cargo clippy 0 errors cargo test 58 passed, 1 failed ``` The 1 failure is `tests::a_free_port_is_granted_as_asked`. **It fails 3/3 on `0.16.1` without this change too** -- verified by checking out the base branch and running the full suite three times. It is the known parallel-execution flake: it probes port 21000 with `std::net::TcpListener` (which sets `SO_REUSEADDR`) then asserts `claim_port` (socket2, no `SO_REUSEADDR`) binds the same port. Currently failing 100% on this machine; unrelated to this PR and not filed. ## Left for a follow-up `fetch_text` (`main.rs:943-966`) will still GET any URL the WebView passes as `runtimeIdUrl`/`appShaUrl`. Weaker than this -- the body is not returned on success, only reachability and parse outcome leak -- but it is the same class and would suit the same allowlist. Kept out of scope here.
child_output_with_timeout read stdout and stderr only once try_wait() had reported an exit. A child that outruns the OS pipe buffer blocks in write() with nobody reading, so it never exits, so try_wait() never reports an exit, and the call ends at the timeout with no output at all. warmup_models pipes both streams, and its model downloads emit tqdm progress to stderr in proportion to how long they take rather than how large they are. So the failure lands on slow connections -- the users warmup exists to spare a mid-pipeline download. Each stream now drains on its own thread. Both are needed: draining one and then the other reintroduces the deadlock on whichever is second. The readers are joined rather than detached on the timeout path too, since killing the child closes its ends and dropping the handles would leak two threads per timeout. command_output_with_timeout had the same shape and now delegates, so the two cannot drift apart again. The test writes 512 KiB to each stream, comfortably past any pipe buffer. Against the old implementation it fails after burning the full 20-second timeout; with concurrent draining it completes in 0.11s. Worth noting because a measurement of a real cold-cache warmup on a fast connection showed only 8,360 bytes of stderr -- under the buffer, which is why this had not been seen in practice rather than why it could not happen. Refs #516
#529) Fixes #516. **Targets `fix/510-updater-url-allowlist`** -- both touch `main.rs` and the plan sequences the Rust changes. Merge #528 first. ## The problem `child_output_with_timeout` read the pipes only after `try_wait()` reported an exit: ```rust loop { if let Some(status) = child.try_wait()? { // read_to_end on stdout/stderr -- only reachable once the child is gone } thread::sleep(Duration::from_millis(100)); } ``` A child that outruns the OS pipe buffer blocks in `write()` with nobody reading, so it never exits, so `try_wait()` never reports an exit. The call ends at the timeout with no output. `warmup_models` pipes both streams (`main.rs:1414-1415`) and its model downloads emit tqdm progress to stderr in proportion to how **long** they take, not how large they are. The failure therefore lands on slow connections -- exactly the users warmup exists to spare a mid-pipeline download. ## Change Each stream drains on its own thread. Both are required: draining one then the other reintroduces the deadlock on whichever is second. Readers are joined rather than detached on the timeout path, since killing the child closes its ends and dropping the handles would leak two threads per timeout. `command_output_with_timeout` had the identical shape and now delegates, so the two cannot drift apart again. ## Verification -- the deadlock is reproduced, not assumed New test `a_chatty_child_is_drained_rather_than_deadlocked` writes 512 KiB to each stream. | implementation | result | |---|---| | read-after-exit (old) | **FAILED** -- `"chatty child timed out after 20 seconds"`, took 20.09s | | concurrent draining (this PR) | passed in 0.11s | Worth recording: when I filed #516 I measured a real cold-cache warmup on a fast connection and found only **8,360 bytes** of stderr -- under the buffer. That is why this had not been hit in practice, not why it could not happen. The test settles it. ``` cargo fmt --check OK cargo clippy 0 errors cargo test 59 passed, 1 failed ``` The 1 failure is `tests::a_free_port_is_granted_as_asked`, the known parallel-execution flake, failing 3/3 on the base branch without this change. ## One thing to check in review While rewriting `command_output_with_timeout` I initially sliced out the `#[cfg(windows)]` attribute above `hide_console_window`, which `cargo check` caught immediately. Restored, and both `hide_console_window` definitions keep their cfg guards -- worth a glance since a Linux/macOS-only compile would not exercise the Windows one.
download_linux_ffmpeg fetched a tarball from a rolling URL on a single host, extracted it with the system tar, marked the binaries executable and ran them, with no integrity check of any kind. Windows verifies against BtbN's published checksums.sha256 and macOS against four pinned hashes; Linux verified nothing. verify_ffmpeg only proves the binary runs and has the encoders we need, which says nothing about where it came from. Pinned rather than verified against upstream's .md5 companion, which was the obvious move but is worth less than it looks: MD5 is broken for collisions, and the companion is served by the same host as the tarball, so anyone able to replace one can replace the other. It evidences corruption, not authenticity. The pinned hash was computed from the artifact whose MD5 matched upstream's published 7fa72b652e19bf84c9461e332ea1cdf3, so this pin is anchored to what upstream currently vouches for. The URL is a rolling one, so this needs a manual bump when upstream publishes a new build; the current one is dated 2024-08-24. A stale pin fails closed with a checksum error rather than silently accepting whatever arrives. STEMDECK_FFMPEG_URL still overrides, and skips the check -- an override points somewhere we cannot have a hash for, so vouching for it is the caller's business, matching how the macOS override already behaves. Note this code cannot be compiled on macOS or by ci.yml, which does not build the Rust shell at all. It was type-checked by temporarily widening its cfg gate to #[cfg(unix)]; cargo check reported no errors. Refs #518
Fixes #518. **Targets `fix/516-drain-child-pipes`** -- last of the sequential Rust changes. Merge #529 first. ## The problem `download_linux_ffmpeg` fetched a tarball from a rolling URL on a single non-CDN host, extracted it with the system `tar`, marked the binaries executable and ran them -- with **no integrity check of any kind**. - **Windows** verifies against BtbN's published `checksums.sha256` - **macOS** verifies against four pinned hashes - **Linux** verified nothing `verify_ffmpeg` only proves the binary runs and has the encoders StemDeck needs. That says nothing about where it came from. ## Why a pinned SHA256 rather than upstream's .md5 The obvious move was the `.md5` companion, matching the Windows shape. I checked it and it is worth less than it looks: - **MD5 is broken for collisions** - **The companion is served by the same host as the tarball.** Anyone able to replace one can replace the other. It evidences corruption, not authenticity. - Upstream publishes no `.sha256` (confirmed: 404) So this pins our own SHA256, the way the macOS path already does. The pinned value was computed from the artifact whose MD5 matched upstream's published `7fa72b652e19bf84c9461e332ea1cdf3`, so the pin is anchored to what upstream currently vouches for. ``` url .../ffmpeg-release-amd64-static.tar.xz (last-modified 2024-08-24) md5 7fa72b652e19bf84c9461e332ea1cdf3 (matches upstream) sha256 abda8d77ce8309141f83ab8edf0596834087c52467f6badf376a6a2a4c87cf67 ``` **Trade-off:** the URL is rolling, so this needs a manual bump when upstream publishes a new build. A stale pin fails closed with a checksum error rather than silently accepting whatever arrives, which is the right direction to fail. `STEMDECK_FFMPEG_URL` still overrides and skips the check -- an override points somewhere we cannot have a hash for, matching how the macOS override already behaves. ## Verification -- and a gap reviewers should know about **This code cannot be compiled on macOS**, and **`ci.yml` does not build the Rust shell at all** -- Linux Rust is compiled only by `linux-release.yml`, at release time. So a type error here would first surface during a release. To check it locally I temporarily widened the cfg gate on the two constants and `download_linux_ffmpeg` from `#[cfg(all(unix, not(target_os = "macos")))]` to `#[cfg(unix)]` and ran `cargo check`: ``` error count: 0 ``` Then restored all 7 gates (verified by count). ``` cargo fmt --check OK cargo clippy 0 errors cargo test 59 passed, 1 failed (the known port flake, fails on base too) ``` ## Worth filing separately There is no `linux-check.yml`. #421 added `macos-check.yml` and `windows-check.yml` precisely because `ci.yml` is 100% ubuntu and could not compile cfg-gated code for those platforms -- but nothing compiles the **Linux** Rust either, despite the runner being ubuntu. That is a real hole and it is what made this PR awkward to verify. Happy to open an issue.
…ly (#532) Fixes #513. Independent of the other open PRs; branches off `0.16.1`. ## The leak `claim_sse_slot()` runs in the handler; `release_sse_slot()` lived in the stream's `finally`. **An async generator that is never started never runs its `finally`.** `StreamingResponse.__call__` awaits `stream_response`, whose first statement is `await send({"type": "http.response.start", ...})`. If the client is already gone that raises `ClientDisconnect` *before* `async for` ever calls `__anext__` -- so the generator body never executes and the slot is held for the life of the process. Connect to `/api/jobs/<id>/events`, RST immediately, repeat 200 times: every progress stream and the queue stream answer 503 with zero live connections, until a restart. `_sse_active` has no ceiling reset and no reconciliation. This does not need malice. A flaky network, or a page reloaded rapidly, leaks slots the same way. ## Why not just claim inside the generator That was the obvious fix and it does not work. By the time the generator runs, the response headers have gone out -- there is no status code left to send, so hitting the cap could no longer answer 503. The claim has to stay in the handler. ## The fix `SseSlot` keeps the claim in the handler and makes release **idempotent**, with `__del__` as the backstop for the never-started case: collecting the generator collects the closure holding the slot. The stream still releases on its normal path, and the two cannot double-count. `queue.py` takes the same guard, so both streams sharing the budget share the fix. ## A bug the tests caught in the fix itself `_held` is assigned **before** the claim: ```python def __init__(self) -> None: self._held = False claim_sse_slot() # may raise 503 self._held = True ``` `__del__` runs on a half-built object too. Without that first line, a refused claim raised `AttributeError` out of `__del__` instead of releasing nothing -- `test_a_refused_claim_holds_nothing` failed on the first run and found it. ## Verification New `tests/test_sse_slot_budget.py`, 5 tests. Confirmed not vacuous -- removing the `__del__` backstop fails `test_a_slot_dropped_without_release_is_reclaimed`. ``` ruff check All checks passed ruff format 94 files already formatted pytest tests/ 898 passed, 2 failed ``` The 2 failures are the pre-existing ogg pair, which fail identically on `0.16.1`. ## One caveat for reviewers `_sse_active` is a plain module-level int with no lock. That predates this change, and everything normally touches it from the event loop -- but `__del__` can run on whichever thread triggers collection, so the backstop is a slightly wider surface than the existing code had. Given the value only ever moves by one and the alternative was a permanent leak, that felt like the right trade; worth a second opinion. A lock around both helpers would close it if you want belt and braces.
…loop (#533) Fixes #512. Independent; branches off `0.16.1`. Four related holes, each reachable with a single unauthenticated request. ## 1. The trim range had no ceiling `end: float | None = Query(default=None, gt=0)` -- no `le=`, and the only cross-check was `start >= end`. It reaches: ```python buf = np.zeros(int(round(duration * sample_rate)), dtype=np.float64) ``` `?start=0&end=20000&count_in=1` asks for ~7 GB, plus another ~7 GB in the int16 conversion. A larger value raises `MemoryError` **inside a blanket `except`**, which silently shipped the export with no click rather than failing. `_validate_trim_range` bounds it by the job's own `duration_sec`, with a six-hour backstop for a job whose duration was never recorded, and 1s of slack -- ffprobe's duration can sit a hair under the decoded length and the UI legitimately asks for the very end of a track. Wired into all three handlers that accept a range. ## 2. The render blocked the event loop `_click_lane` was a plain `def` called inside `async def`. All of the above allocation plus a Python loop over every beat ran on the loop, so every SSE progress stream and the queue worker stalled behind it. Now `asyncio.to_thread`. Buffer switched to **float32**: output is 16-bit PCM, so the extra mantissa was never audible, and a long export was allocating twice what it needed. ## 3. The click cache evicted its own render `_prune_mixdown_cache(_CLICK_CACHE_DIR)` was called **without `keep=`**. A render larger than the 500 MB budget evicted itself the instant it was written, and ffmpeg was handed a missing `-i`. That is the #482 bug, unfixed on this path. The mixdown write had the same gap against a *concurrent* render's prune; both now pass `keep=`. ## 4. The body guard covered two paths and had a chunked bypass Scoped by path suffix (`/sections`, `/beats`), leaving `/api/search`, `/api/playlist`, `/api/playlist/preview`, `/api/settings` and the JSON branch of `/api/jobs` uncapped. A 200 MB body to `/api/search` stalled every other request. Now applied by **method**, exempting multipart uploads (they stream to disk under their own 400 MB limit). A chunked request with no `Content-Length` made `declared` `None` and skipped the check entirely -- it gets a **411** now rather than falling through to an unbounded `request.body()`. ## Verification New `tests/test_request_body_limits.py`, 8 tests. Confirmed not vacuous -- reverting the guard scope and the trim bound fails **6 of 8**. ``` ruff check All checks passed ruff format 94 files already formatted pytest tests/ 901 passed, 2 failed (pre-existing ogg pair) ``` ## Two review notes **`_EDITOR_BODY_LIMIT` was renamed to `_JSON_BODY_LIMIT`** since it is no longer editor-specific. `tests/test_jobs_api.py` imported it directly and was updated. **411 is an unusual status to return.** The alternative was streaming and counting the body ourselves, which is more code in a middleware that runs on every request. Given nothing in the app sends chunked JSON, refusing it outright seemed proportionate -- but if any client does, this would break it.
… before the CPU retry (#534) Fixes #514. Independent; branches off `0.16.1`. ## 1. Worker teardown sat outside the `finally` ```python finally: _done_evt.set() set_proc(job.id, None) wt.join(timeout=2) if job_ok is not True: # <- outside _kill_worker() ``` An exception out of the read loop -- `proc.stderr.read(1)` raising `OSError` when the API thread's `terminate()` races the read, or `_set()` raising -- propagated without it. `_worker` still held the process, so the next job's `_get_worker()` saw a matching device and a live `poll()` and **reused a worker whose CUDA state followed an exception**. `.claude/rules/ml-pipeline.md`: *"torn down after **any** non-success (cancellation or failure) -- post-exception CUDA state can't be trusted. Both halves of this rule matter; don't relax either side."* ## 2. A second path missed teardown entirely The pipe check raises **before** the `try`: ```python if proc.stdin is None or proc.stderr is None: raise RuntimeError("demucs worker has no stdin/stderr pipe") ``` so a worker that came back without pipes stayed cached and would be handed to every subsequent job. Found while writing the test for #1 -- it failed for this reason rather than the one I was targeting. ## 3. Cancel was dropped before the CPU retry `separate()` had no cancel check between attempts. The `rmtree` of a multi-GB partial takes seconds with nothing registered for cancel, so a cancel landing there was invisible and the **entire CPU pass ran to completion** -- 10+ minutes -- before `JobCancelled` was raised at the end. The UI showed "Cancelling" throughout. ## 4. `_kill_worker` killed without reaping `kill()` only sends the signal. Without `communicate()` a worker wedged in an uninterruptible CUDA call becomes a zombie whose pipes close only when the `Popen` refcount happens to drop. `vocal_split.py` already pairs the two. ## A check I tried and removed I initially added a cancel check at the top of `_run_demucs`. It broke `test_cancel_kills_worker_next_job_spawns_fresh`, which verifies that a cancelled job tears the worker down so the next spawns fresh -- a check that never spawns has no worker to tear down, so the test's premise collapses. That test encodes exactly the rule this PR is protecting, so the check went rather than the test. The queue worker already gates dispatch on cancellation, and the fallback check covers the window that was actually reported, so nothing is lost. ## Verification New `tests/test_separate_worker_lifecycle.py`, 3 tests. Confirmed not vacuous -- reverting the teardown placement and the fallback check fails them. ``` ruff check All checks passed pytest tests/ 896 passed, 2 failed (pre-existing ogg pair) ``` `tests/test_separate_fallback.py` (13 tests) still passes unchanged -- worth noting, since that is the suite that pushed back on the entry-point check.
…op (#535) Fixes #519. Independent; branches off `0.16.1`. ## 1. Two ffmpeg calls were unregistered, so cancel could not reach them `runner.py:79` and `:130` used `subprocess.run()`, which cannot be interrupted -- `POST /cancel` sets the flag but nothing looks at it until the call returns. Cancelling during "Preparing audio..." on a 400 MB `.mp4` was a no-op for up to `TIMEOUT_FFMPEG` **per call**, and that path runs both the video extract and the transcode. Both now go through `_run_registered_ffmpeg`, mirroring `collect._run_ffmpeg` which registers for exactly this reason. It deregisters in a `finally`, so a failure cannot leave a stale entry a later cancel would terminate on the wrong job. ## 2. Two of three workers never armed the parent-death watchdog `STEMDECK_PARENT_PID` was set in exactly one place (`separate.py`) and read in exactly one place (`demucs_worker.py`). - A **Force-Quit during a vocal split** orphaned an onnxruntime process holding the GPU, with nobody to collect the result. - A **section pass** outlived the parent whose `TIMEOUT_SECTIONS` was its only bound. The watchdog moves to `app/core/process.py` -- where `process_exists` already lived specifically for it -- so all three workers share one implementation rather than three copies. All three spawn sites export the pid. **Poll interval preserved at 1.0s.** I initially wrote 5s in the shared version, which would have silently slowed the existing demucs watchdog. Caught before commit. ## 3. A running vocal split was uncancellable by construction `cancel_job` returns early for a `done` job -- and a vocal split only ever runs on a done job, so `cancel_requested` was never even set, while the split held `_pipeline_lock` and stalled the whole import queue for its duration. Cancel now terminates the worker for that specific case. The split's own error path marks it failed and releases the lock, so nothing else was needed. ## Verification New `tests/test_cancellation_reach.py`, 12 tests. Confirmed not vacuous -- reverting the vocal-split cancel and one parent-pid export fails 2. Two of them are deliberately structural (`test_every_worker_spawn_exports_the_parent_pid`, `test_every_worker_arms_the_watchdog`): they assert across *all three* workers, so a fourth worker added later without the watchdog fails the suite rather than silently orphaning a process. ``` ruff check All checks passed ruff format 94 files already formatted pytest tests/ 905 passed, 2 failed (pre-existing ogg pair) ``` ## Two existing test files updated, not fixed - `test_worker_parent_watchdog.py` targeted `demucs_worker._arm_parent_watchdog` and `demucs_worker.threading`; both moved to `app.core.process`. - `test_video_status.py` stubbed `runner_mod.subprocess.run`, which the extract no longer calls; it now stubs `_run_registered_ffmpeg`. Behaviour is unchanged in both cases -- worth a look to confirm you agree the seams moved sensibly.
…multitrack (#536) engineMode() returns "chunked" unless the user has set the audioEngine flag to "0", and on that path audioEngine owns the clock while the multitrack is mounted with url: null for visuals only. transport.js handled this everywhere via `audioEngine ?? multitrack`; main.js imported only `multitrack` and called it bare. So on the default configuration: - The footer scrub bar did nothing. It is a full-size cursor: pointer overlay, and clicking or dragging anywhere on it moved neither the playhead nor the audio. - [ and ] seeked the silent multitrack, so nothing happened. - I and O read multitrack.getCurrentTime(), which is pinned at 0 there, so "set loop in at playhead" always wrote 0 no matter where the playhead was, and "set loop out" always wrote max(0, loopStart + 0.5). Space and ruler clicks were unaffected because they live in transport.js. Rather than patching five call sites, transport.js exports the accessor it was already using internally, plus setPlayheadTime. Seeking now goes through setPlayheadTime, which also updates the playhead marker, footer times and presence playhead -- none of which the old multitrack.setTime call did, so the scrub bar would have left the marker stale even had it worked. Same anti-drift shape as apply_ffmpeg_path in #506. The keydown guard excluded HTMLInputElement but not HTMLTextAreaElement, so Space in the settings log viewer started playback instead of scrolling. Fixed alongside, since the guard was being edited anyway. The test is structural rather than DOM-driven: the bug was not a wrong value from a function, it was reaching for the wrong object, so it asserts main.js holds no bare multitrack playback calls. All 8 checks fail against the previous code. Refs #515
Deno was fetched from releases/latest/download with no version pin and no checksum, into every published GHCR image. It is the JS runtime yt-dlp feeds YouTube's challenge payload to, so it executes against untrusted input, and every build took whatever Deno published that day. The desktop packaging scripts already pin and verify QuickJS with the rationale written out in a comment -- Docker was the one path that did not follow it. Now pinned to v2.9.6 with a per-arch SHA256 verified before the zip is unpacked. linux-release.yml and windows-release.yml uploaded with action-gh-release, whose fail_on_unmatched_files defaults to false, and neither asserted the files existed first -- macOS is the only path that did. A missing updater asset therefore published a release that went green while the in-app updater 404'd for every installed user. Both now check every asset before uploading and set fail_on_unmatched_files. make-portable.ps1 set $PSNativeCommandErrorActionPreference, which exists only in PowerShell 7+, while CI invokes it with `powershell` -- Windows PowerShell 5.1, where it does nothing and $ErrorActionPreference does not cover native commands either. A failed CPU-torch --force-reinstall was ignored, leaving whatever torch was already resolved in place, and the zip labelled CPU shipped a non-CPU torch; the later import checks still passed because torch imports fine either way. Assert-LastExitCode now guards the venv creation and all three pip installs. The preference stays for a pwsh run. Switching the invocation to pwsh would have been the tidier fix, but nothing in this repo uses pwsh and the runner is self-hosted, so there is no way to confirm PowerShell 7 is installed there. An exit-code check works on both. trivy-action was pinned to @master, the only unpinned action in the repo while everything else is SHA-pinned. Now on v0.36.0's commit. Both Deno hashes were verified against the published artifacts, including a negative control confirming a wrong hash is rejected. Refs #517
Reported in discussion #507: changing the size of a selection meant clicking and dragging again from scratch, which restarts the whole thing and loses the precision you had. The numeric fields are the only exact route, and finding a seamless loop through them is guesswork. The region could not be touched at all -- .loop-region was pointer-events: none, so every pointerdown reached the create-drag underneath, which resets both edges to the click position. Adjusting one edge necessarily destroyed the other. Three gestures now: - a handle at either edge moves that edge alone, so a loop can be tightened one side at a time; - the body moves both edges together, preserving length, so a loop found by ear can be slid without being re-measured; - a press that does not travel still seeks, which is what clicking inside the selection did before the region took pointer events. Losing that would have been a regression for anyone who just wants to move the playhead. The handles are 12px and overhang the 2px border on both sides, because an edge you cannot reliably grab is the finnicky behaviour this is meant to replace. They only render on hover or while dragging, so the selection looks the same at rest. create-drag already guarded with `e.target.closest(".loop-region")` -- dead code until now, since the element could not receive events. It is a real guard again, alongside the stopPropagation on the region's own handler. The geometry lives in its own module rather than in transport.js, which cannot be imported outside a browser: it pulls in state.js, which touches document at module load. Same reason playbackStems.js is separate. That makes the clamping testable, which is the half that matters -- an edge crossing its partner, or a region pushed against either end of the track. MIN_LOOP_SEC moves with it and now has one definition. Pointer events throughout, so this works with touch and pen. Closes #538.
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
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
thcp
marked this pull request as ready for review
September 1, 2026 07:03
…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
changed the title
…pha badge (#547) * feat(panels): clear the studio down to the mixer in one press #480 gave each panel around the mixer a toggle, but collapsing all three still left the library holding its column, which is the largest thing on screen that is not the mixer. Getting to a bare mixer meant three presses in the toggles row and a fourth on a control somewhere else, then four more to undo it. "All" sits at the end of the same row and takes the three panels and the sidebar together. It keeps no state of its own: it drives the same apply/persist the individual toggles already use, and reads its own pressed state back off .app. A fourth flag would be a fourth thing to disagree with the other three the moment the library was collapsed from its own button instead. A MutationObserver on that one class attribute is what keeps it honest, so no other handler has to remember it exists. setSidebarCollapsed moves out of wireCatalogToggle's scope and is exported, because the sidebar's state is one class plus one localStorage flag and two writers reproducing that pair is how it drifts. setCatalogView was already the second writer: it wrote the flag by hand and left the collapse button's aria-expanded claiming the sidebar was still shut. It routes through the helper now. panels.all and panels.allTitle are in all ten language tables. ptPT needs no override, the European wording is the same. * feat(footer): scroll a loop bound, and stop the panel row shouting Three things in the footer, all of them small and all of them in the same three files. A loop bound could only be changed by typing it. A loop is never right first time: you drag a rough region, play it, and want the start forty milliseconds earlier because it clips the transient. That meant clicking in and retyping nine characters, which is slower and less precise than dragging the region again, so the fields went unused for the one job they are best at. The wheel now adjusts them, in seconds left of the decimal point and milliseconds right of it, a tenth and a hundredth of a second per notch. One millisecond per notch would need a hundred notches to cover something audible. Character-level hit-testing inside an input is not reliable across browsers, but only the decimal point matters, so the text up to it is measured and compared. A nudge past either end of the track, or one that would squeeze the loop under the minimum, is dropped rather than clamped, so holding the wheel at the end does not drag the other bound along. The panel row read "Click to collapse" as a sentence it had no width for, and All was fused to it. The label is one word now, the rule sits after it, and All is the first of four buttons. Its pressed state greys and strikes it through like the other three, but only once all four are away: read as "is anything hidden" it struck itself through the moment Analysis was hidden alone, which looks exactly like you pressed it. Loop stands beside the two bounds rather than over them. As a row of its own it stretched the footer to three lines for one button. The Alpha badge is gone from Click track. The feature has been through several releases and is covered by its own spec; a warning that has stopped being true also teaches people to ignore the next one. Removed properly: markup, the CSS rule that had no other user, and the dead click.alpha key in all ten tables. --------- Co-authored-by: Thales <>
… test does not use a_free_port_is_granted_as_asked asked for 21000 and asserted against 53969 on a shared macOS runner. 53969 is an ephemeral port, so claim_port had refused 21000 and reserve_port had correctly fallen back. The failure was the probe, not the code. The probe bound with std::net::TcpListener, which sets SO_REUSEADDR on Unix. claim_port deliberately does not. A port sitting in TIME_WAIT therefore accepts the probe and refuses the claim, and the test reads "free" where reserve_port reads "taken". Probing through claim_port makes the word mean one thing. Matching options narrows the race but cannot close it: the probe has to let go before reserve_port can take the port, and cargo runs this binary's tests in parallel with several of them standing up throwaway listeners. So the test walks the range instead of committing to the first candidate. Losing one race costs a retry; a reserve_port that genuinely ignored a free port would have to lose all two hundred, and the assertion says exactly that when it fails. cargo test 57 passed, cargo clippy clean, cargo fmt clean.
Five sections stacked in one 420px column is taller than the dialog on any normal window, so the list scrolled and the people below the fold were never seen. That is a poor way to point at them. Two columns with a rule between, in a 780px card. The split is after the third section, which is where the two sides come out closest in height. Each column is still wide enough for three cards per row, so nothing got smaller to make room. The overflow stays as a floor rather than the normal case: a very short window still has to reach the bottom somehow. Below 860px the card drops back to one column and the rule goes with it, instead of squeezing the cards under three per row. playwright 85 passed, node tests/js 11/11.
This was referenced Sep 1, 2026
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.
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.pyplotfails, which killsallin1_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
settings.jsonwas written withwrite_text(truncate, then write), and_loadcould not tell a torn file from a first run. A real user lostportandallow_networkfrom both the file and its mirror. Now atomic, and a corrupt file is preserved assettings.json.corrupt-<ts>and recovered from the mirror.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. A server-side deletion record closes it.addTrackToLibrarydeduplicates 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, andsyncWithServerre-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.main.jsdrove the silent multitrack whileaudioEngineowned the clock.queuedforever: invisible, still counted against capacity, source file never freed, re-queued on every restart. Also, a malformedregistry.jsonraised an uncaughtAttributeErrorat import and the backend never started.Security
releases/latestunpinned 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
Archive::unpack's directory deferral, so a read-only directory member would fail extraction outright.endhad no upper bound, reaching a multi-GBnp.zeroson the event loop; the body-size guard covered two paths and was bypassed by chunked encoding.finally, so an exception left a poisoned CUDA worker warm; cancel was dropped before the CPU fallback, costing 10+ minutes.child_output_with_timeoutnever drained child pipes until exit, deadlocking any chatty child.Presentation
r/bassis 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
Verification
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_oggandtest_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 withstd::net::TcpListener(which setsSO_REUSEADDR) then assertsclaim_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 -CpuOnlyand driven by hand. What that covered:allow_networksurvived a quit and relaunch, in both the portable file and the AppData mirror.python.exe, and the interrupted job resumed once withresume_attempts: 1.Two things reviewers should know before tagging
make-portable.ps1now 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.ymlnever invokes cargo; Linux Rust is built only bylinux-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.