diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c1496cde..08d74051 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -178,7 +178,7 @@ jobs: # would scan its bundled extractor files and flag false-positive # secrets that ship inside third-party packages like yt-dlp). - name: trivy fs - uses: aquasecurity/trivy-action@master + uses: aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25 # v0.36.0 with: scan-type: fs scan-ref: . @@ -190,7 +190,7 @@ jobs: skip-dirs: .venv,jobs # Dedicated Dockerfile + compose static analysis (Trivy's IaC linter). - name: trivy config - uses: aquasecurity/trivy-action@master + uses: aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25 # v0.36.0 with: scan-type: config scan-ref: build/ diff --git a/.github/workflows/linux-release.yml b/.github/workflows/linux-release.yml index 5aa06590..fade993b 100644 --- a/.github/workflows/linux-release.yml +++ b/.github/workflows/linux-release.yml @@ -139,6 +139,24 @@ jobs: clamscan --recursive --infected --bell /scan echo "ClamAV scan completed successfully. No infected files reported." + # macos-release.yml asserts its assets exist before uploading; this did + # not. action-gh-release defaults fail_on_unmatched_files to false, so a + # missing updater asset published a release that looked fine and went + # green, and the in-app updater then 404'd for every installed user. + - name: verify every asset exists + if: github.event_name == 'release' + run: | + for f in \ + dist/StemDeck-Linux-x64.tar.gz \ + dist/StemDeck-Linux-x64.tar.gz.sha256 \ + dist/StemDeck-Linux-x64.NVIDIA.tar.gz \ + dist/StemDeck-Linux-x64.NVIDIA.tar.gz.sha256 \ + dist/StemDeck-Linux-x64-app.tar.gz \ + dist/StemDeck-Linux-x64-app.tar.gz.sha256 \ + dist/StemDeck-Linux-x64-runtime-version.json; do + test -f "$f" || { echo "missing release asset: $f" >&2; exit 1; } + done + - name: upload artifacts # Only attach to a real release; a manual test build has nothing to upload to. if: github.event_name == 'release' @@ -150,6 +168,7 @@ jobs: # pushing :latest to GHCR, and makes the in-app updater offer a build # that was never verified. prerelease: ${{ github.event.release.prerelease }} + fail_on_unmatched_files: true files: | dist/StemDeck-Linux-x64.tar.gz dist/StemDeck-Linux-x64.tar.gz.sha256 diff --git a/.github/workflows/macos-check.yml b/.github/workflows/macos-check.yml index de670993..57f62d0e 100644 --- a/.github/workflows/macos-check.yml +++ b/.github/workflows/macos-check.yml @@ -26,6 +26,20 @@ concurrency: jobs: check: + # Never run a fork's code on the self-hosted runner. cargo build/clippy/test + # all execute whatever the PR supplies -- build.rs, proc-macro crates, a + # swapped Cargo.toml dependency, the test bodies themselves -- and this + # runner is the same machine that builds, signs and uploads every macOS + # release. Nothing here cleans the workspace, so an implant in ~/.cargo, + # ~/.rustup or the persistent _work tree would survive into the next + # release. permissions: {} limits the token, not code execution. + # + # GitHub's public-repo default only gates *first-time* contributors, so one + # trivial merged PR is enough to unlock this for a later one. Fork PRs get a + # maintainer-triggered workflow_dispatch run instead. + if: >- + github.event_name != 'pull_request' || + github.event.pull_request.head.repo.full_name == github.repository # Runner must be darwin/arm64 with Xcode CLT and rustup (same requirements # as macos-release.yml, which this intentionally does not replace -- this # only builds/checks, never signs, packages, or uploads anything). diff --git a/.github/workflows/windows-check.yml b/.github/workflows/windows-check.yml index a567d294..e072eb80 100644 --- a/.github/workflows/windows-check.yml +++ b/.github/workflows/windows-check.yml @@ -27,6 +27,15 @@ concurrency: jobs: check: + # Never run a fork's code on the self-hosted runner -- see the same guard in + # macos-check.yml. cargo build/clippy/test execute whatever the PR supplies + # (build.rs, proc-macro crates, a swapped Cargo.toml dependency, the test + # bodies), nothing here cleans the workspace, and this runner also builds + # the Windows release. Fork PRs get a maintainer-triggered + # workflow_dispatch run instead. + if: >- + github.event_name != 'pull_request' || + github.event.pull_request.head.repo.full_name == github.repository # Runner must have rustup and the MSVC toolchain (same requirements as # windows-release.yml, which this intentionally does not replace -- this # only builds/checks, never packages or uploads anything). diff --git a/.github/workflows/windows-release.yml b/.github/workflows/windows-release.yml index 70883061..6103a65b 100644 --- a/.github/workflows/windows-release.yml +++ b/.github/workflows/windows-release.yml @@ -92,6 +92,25 @@ jobs: } Write-Host "ClamAV scan completed successfully. No infected files reported." + # See the same guard in linux-release.yml: action-gh-release silently + # tolerates missing files, so an absent updater asset shipped a green + # release the in-app updater could not use. + - name: verify every asset exists + shell: powershell + run: | + $required = @( + "dist/StemDeck-Windows-x64.NVIDIA.zip", + "dist/StemDeck-Windows-x64.NVIDIA.zip.sha256", + "dist/StemDeck-Windows-x64.zip", + "dist/StemDeck-Windows-x64.zip.sha256", + "dist/StemDeck-Windows-x64-app.zip", + "dist/StemDeck-Windows-x64-app.zip.sha256", + "dist/StemDeck-Windows-x64-runtime-version.json" + ) + foreach ($f in $required) { + if (-not (Test-Path $f)) { Write-Error "missing release asset: $f"; exit 1 } + } + - name: upload artifacts uses: softprops/action-gh-release@3d0d9888cb7fd7b750713d6e236d1fcb99157228 # v3.0.2 with: @@ -101,6 +120,7 @@ jobs: # pushing :latest to GHCR, and makes the in-app updater offer a build # that was never verified. prerelease: ${{ github.event.release.prerelease }} + fail_on_unmatched_files: true files: | dist/StemDeck-Windows-x64.NVIDIA.zip dist/StemDeck-Windows-x64.NVIDIA.zip.sha256 diff --git a/README.md b/README.md index 2bb45ed4..7ac4fc55 100644 --- a/README.md +++ b/README.md @@ -51,20 +51,20 @@ Drop in an MP3, WAV, FLAC, OGG/Opus, MP4, or M4A file, or paste a YouTube URL, a StemDeck is free and **does not accept any money, sponsorship, or funding** from anyone listed below. I share these makers and artists and communities purely for the joy of pointing you toward wonderful people doing beautiful work. Go meet them ❤️ -| Name | What they do | Link | -|---|---|---| -| Analog4Lyfe | All-analog music gear, no digital shortcuts | [@analog4lyfe](https://www.instagram.com/analog4lyfe) | -| r/bass | My beloved bass community on reddit | [r/Bass](https://www.reddit.com/r/Bass) | -| Beltr | Turns the songs you already own into karaoke gold, right on your own machine, no subscription, no cloud, just you and the mic | [beltr.app](https://beltr.app/) | -| Dlima Guitars | Custom guitars and basses, built one at a time | [@dlimaguitars](https://www.instagram.com/dlimaguitars) | -| Empress Effects | Boutique effects pedals for tone chasers who don't settle | [empresseffects.com](https://empresseffects.com) | -| Joao Gaspar | Producer and film scorer, also plays as a touring/session musician | [@jay_glaspar](https://www.instagram.com/jay_glaspar) | -| Kris Luthier | Hand-repairs and restores instruments in Lisbon, one careful fix at a time | [@krisluthier](https://www.instagram.com/krisluthier) | -| Lisbon Guitar Works | Guitars built by hand in Lisbon | [dlimaguitars.com](https://dlimaguitars.com) | -| More Notes Less Talk | Instruments and gear with personality, recorded raw to tape. No hype, no gatekeeping. | [@morenoteslesstalk](https://www.youtube.com/@morenoteslesstalk) | -| Seratone | Turns any TV into a studio-grade karaoke stage | [seratone.audio](https://seratone.audio/) | -| slashCAM | German-language camera and video tech: hands-on tests, industry news, and the post-production details most reviews skip | [@slashcam.de](https://www.instagram.com/slashcam.de) | -| Thomann | One of Europe's largest music gear retailers, practically everything a musician could need | [@thomann.music](https://www.instagram.com/thomann.music) | +| Category | Name | What they do | Link | +|---|---|---|---| +| Artists & Creators | Joao Gaspar | Producer, film scorer, touring/session musician | [@jay_glaspar](https://www.instagram.com/jay_glaspar) | +| Artists & Creators | More Notes Less Talk | Gear-focused creative project with a raw, tape-recorded identity | [@morenoteslesstalk](https://www.youtube.com/@morenoteslesstalk) | +| Instrument Builders & Repair | Dlima Guitars | Custom guitars and basses | [@dlimaguitars](https://www.instagram.com/dlimaguitars) | +| Instrument Builders & Repair | Lisbon Guitar Works | Handmade guitars in Lisbon | [dlimaguitars.com](https://dlimaguitars.com) | +| Instrument Builders & Repair | Kris Luthier | Instrument repair and restoration | [@krisluthier](https://www.instagram.com/krisluthier) | +| Music Gear | Analog4Lyfe | Analog gear specialist | [@analog4lyfe](https://www.instagram.com/analog4lyfe) | +| Music Gear | Empress Effects | Boutique effects pedals | [empresseffects.com](https://empresseffects.com) | +| Music Gear | Thomann | Large music-equipment retailer | [@thomann.music](https://www.instagram.com/thomann.music) | +| Music & Karaoke Technology | Beltr | Local, subscription-free karaoke software | [beltr.app](https://beltr.app/) | +| Music & Karaoke Technology | Seratone | TV-based karaoke system | [seratone.audio](https://seratone.audio/) | +| Media & Community | slashCAM | Camera, video, and post-production media | [@slashcam.de](https://www.instagram.com/slashcam.de) | +| Media & Community | r/bass | Bass-player community | [r/Bass](https://www.reddit.com/r/Bass) | --- diff --git a/app/api/events.py b/app/api/events.py index f63b6370..dedb0b84 100644 --- a/app/api/events.py +++ b/app/api/events.py @@ -38,6 +38,45 @@ def release_sse_slot() -> None: _sse_active -= 1 +class SseSlot: + """One held connection slot, released exactly once. + + Claiming has to happen in the handler so that hitting the cap can still be + answered with a 503 -- once the generator is running the response headers + have gone out and there is no status code left to send. + + That is what leaked slots: release lives in the stream's `finally`, and an + async generator that is never started never runs its `finally`. If the + client disconnects before the body begins, StreamingResponse raises inside + `stream_response` on its first `send()` -- before `__anext__` is ever + called -- so the generator body never executes and the slot was held + forever. 200 of those and every progress stream 503s with nothing actually + connected, until the process restarts (#513). + + The stream releases on its way out as before; `__del__` is the backstop for + the never-started case, where collecting the generator collects the closure + holding this. Release is idempotent so the two cannot double-count. + """ + + __slots__ = ("_held",) + + def __init__(self) -> None: + # Set first: claim_sse_slot raises at the cap, and __del__ still runs on + # a half-built object. Without this it would raise AttributeError from + # __del__ instead of releasing nothing. + self._held = False + claim_sse_slot() # may raise 503; nothing is held if it does + self._held = True + + def release(self) -> None: + if self._held: + self._held = False + release_sse_slot() + + def __del__(self) -> None: + self.release() + + @router.get("/jobs/{job_id}/events") async def job_events(job_id: str) -> StreamingResponse: """Server-Sent Events stream of job state updates. Closes when the job @@ -47,7 +86,7 @@ async def job_events(job_id: str) -> StreamingResponse: job = registry_get(job_id) if job is None: raise HTTPException(status_code=404, detail="job not found") - claim_sse_slot() + slot = SseSlot() async def stream() -> AsyncIterator[str]: try: @@ -82,7 +121,7 @@ async def stream() -> AsyncIterator[str]: keepalive_at = 0 await asyncio.sleep(0.2) finally: - release_sse_slot() + slot.release() return StreamingResponse( stream(), diff --git a/app/api/jobs.py b/app/api/jobs.py index f2be8f41..7e40f3b8 100644 --- a/app/api/jobs.py +++ b/app/api/jobs.py @@ -29,6 +29,7 @@ from app.core.registry import all_jobs as registry_all_jobs from app.core.registry import get as registry_get from app.core.registry import get_proc as registry_get_proc +from app.core.registry import mark_deleted as registry_mark_deleted from app.core.registry import pending_count as registry_pending_count from app.core.registry import persist as registry_persist from app.core.registry import register_if_capacity as registry_register_if_capacity @@ -110,14 +111,29 @@ def _copy_to_dest(src_file: object, dest: Path) -> None: shutil.copyfileobj(src_file, out) # type: ignore[arg-type] -def _rmtree_job(job_id: str) -> None: +def _rmtree_job(job_id: str) -> bool: + """Remove a job's directory. False means files are still on disk. + + The outcome used to be swallowed, so delete_job dropped the registry entry + whether or not anything was actually deleted -- and restore() then adopted + the surviving directory on the next start, which is how deleted songs came + back (#521). + + Retried once: on macOS the common 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.""" job_dir = JOBS_DIR / job_id - if not job_dir.is_dir(): - return - try: - shutil.rmtree(job_dir) - except Exception: - logger.warning("failed to remove job dir %s", job_dir, exc_info=True) + for attempt in (1, 2): + if not job_dir.is_dir(): + return True + try: + shutil.rmtree(job_dir) + return True + except Exception: + logger.warning( + "failed to remove job dir %s (attempt %d)", job_dir, attempt, exc_info=True + ) + return not job_dir.is_dir() def _job_files_missing(job: Job) -> bool: @@ -326,6 +342,16 @@ def cancel_job(job_id: str) -> dict: if job is None: raise HTTPException(status_code=404, detail="job not found") if job.status in ("done", "error", "cancelled"): + # A vocal split only ever runs on a done job, so this early return made + # it uncancellable by construction: the flag was never even set, while + # the split held _pipeline_lock and stalled the whole import queue for + # its full duration (#519). Terminating the worker is enough -- the + # split's own error path marks it failed and releases the lock. + if job.vocal_split == "running": + job.cancel_requested = True + proc = registry_get_proc(job_id) + if proc is not None and proc.poll() is None: + proc.terminate() return job.to_state() job.cancel_requested = True @@ -759,7 +785,16 @@ def delete_job(job_id: str) -> dict[str, str]: raise HTTPException(status_code=404, detail="job not found") if job.status not in ("done", "error", "cancelled"): raise HTTPException(status_code=409, detail="job is still running") - _rmtree_job(job_id) + removed = _rmtree_job(job_id) + # Recorded whether or not the files went away. The user asked for this job + # to be gone; without the record, a directory that outlived the delete is + # re-adopted by restore() on the next start and the track reappears. + registry_mark_deleted(job_id) registry_remove(job_id) registry_persist(JOBS_DIR) + if not removed: + raise HTTPException( + status_code=500, + detail="Removed from the library, but its files could not be deleted.", + ) return {"job_id": job_id, "status": "deleted"} diff --git a/app/api/queue.py b/app/api/queue.py index 6687185a..0c2c65e5 100644 --- a/app/api/queue.py +++ b/app/api/queue.py @@ -22,7 +22,7 @@ from fastapi.responses import StreamingResponse from pydantic import BaseModel -from app.api.events import _MAX_SSE_SECONDS, claim_sse_slot, release_sse_slot +from app.api.events import _MAX_SSE_SECONDS, SseSlot from app.core.config import JOB_ID_RE, JOBS_DIR, MAX_PENDING_UPLOAD_JOBS, MAX_PENDING_URL_JOBS from app.core.registry import get as registry_get from app.core.registry import pending_count as registry_pending_count @@ -130,7 +130,7 @@ async def queue_events() -> StreamingResponse: outlives any individual job and is expected to stay open for the session, so only the 4 h ceiling ends it. """ - claim_sse_slot() + slot = SseSlot() async def stream() -> AsyncIterator[str]: try: @@ -156,7 +156,7 @@ async def stream() -> AsyncIterator[str]: keepalive_at = 0 await asyncio.sleep(0.25) finally: - release_sse_slot() + slot.release() return StreamingResponse( stream(), diff --git a/app/api/stems.py b/app/api/stems.py index e4710a15..f9605169 100644 --- a/app/api/stems.py +++ b/app/api/stems.py @@ -289,7 +289,10 @@ def _click_lane( return None if rendered is None: return None - _prune_mixdown_cache(_CLICK_CACHE_DIR) + # keep=path or a render larger than the cache budget evicts itself the + # instant it is written, and ffmpeg is then handed a missing -i (#512). + # Same reason the mixdown path passes keep= (#482). + _prune_mixdown_cache(_CLICK_CACHE_DIR, keep=path) return _ClickLane(path, g, lead_in, True) if not path.is_file(): @@ -308,10 +311,37 @@ def _click_lane( return None if rendered is None: return None - _prune_mixdown_cache(_CLICK_CACHE_DIR) + _prune_mixdown_cache(_CLICK_CACHE_DIR, keep=path) return _ClickLane(path, g, 0.0, False) +# A trim range is only ever meaningful inside the track. Without a ceiling, +# `end` is an unbounded float that reaches +# np.zeros(int(round(duration * sample_rate))) in the click renderer -- so +# ?start=0&end=20000&count_in=1 asks for a 7 GB allocation, and a larger value +# raises MemoryError inside a blanket except and silently drops the click +# (#512). The job's own duration is the honest ceiling; MAX_TRIM_SECONDS is the +# backstop for a job whose duration was never recorded. +MAX_TRIM_SECONDS = 6 * 60 * 60 + + +def _validate_trim_range(job_id: str, start: float | None, end: float | None) -> None: + """Reject a trim range that is not inside the track.""" + if start is None or end is None: + return + job = registry_get(job_id) + duration = getattr(job, "duration_sec", None) if job is not None else None + ceiling = float(duration) if duration else float(MAX_TRIM_SECONDS) + # A little slack over the recorded duration: it comes from ffprobe and can + # sit a hair under the decoded length, and the UI legitimately asks for the + # very end of a track. + if end > ceiling + 1.0: + raise HTTPException( + status_code=422, + detail="end is beyond the end of the track", + ) + + def _read_beat_grid(job_id: str) -> dict | None: """The grid an export should click to: the user's edits when present, the detected grid otherwise. Mirrors GET /api/jobs/{id}/beats.""" @@ -439,7 +469,7 @@ async def _stream_ffmpeg(cmd: list[str], context: str = "", cache_path: Path | N if tmp_path is not None: if finished and proc.returncode == 0: os.replace(tmp_path, cache_path) - _prune_mixdown_cache(cache_path.parent) + _prune_mixdown_cache(cache_path.parent, keep=cache_path) else: tmp_path.unlink(missing_ok=True) @@ -655,6 +685,7 @@ async def get_stem( status_code=422, detail="start and end are both required and start must be less than end", ) + _validate_trim_range(job_id, start, end) cmd = [ ffmpeg_executable(), @@ -701,6 +732,7 @@ async def get_stem_mp3( status_code=422, detail="start and end are both required and start must be less than end", ) + _validate_trim_range(job_id, start, end) # Full-stem requests (no trim) are cached to disk so repeat loads — the # common case for the mobile player — are instant instead of re-encoding. @@ -799,6 +831,7 @@ async def get_mixdown( status_code=422, detail="start and end are both required and start must be less than end", ) + _validate_trim_range(job_id, start, end) # Validates job_id (404), job done (404), and path traversal (404) per # stem -- deliberately before the cache lookup below, so a deleted or @@ -807,7 +840,8 @@ async def get_mixdown( paths = [_validate_stem_path(job_id, name) for name in names] media_type = MIXDOWN_MEDIA_TYPES[ext] - click_lane = _click_lane( + click_lane = await asyncio.to_thread( + _click_lane, job_id, click, click_mult, @@ -958,7 +992,9 @@ async def get_video_mixdown( # Click is one more audio input. It must be appended before the video input # so the audio indices the filter graph references stay contiguous from 0. - click_lane = _click_lane(job_id, click, click_mult, click_accent, click_gain) + click_lane = await asyncio.to_thread( + _click_lane, job_id, click, click_mult, click_accent, click_gain + ) if click_lane is not None: paths = [*paths, click_lane[0]] parsed_gains = [*parsed_gains, click_lane[1]] diff --git a/app/core/process.py b/app/core/process.py index 609372ba..965c4e21 100644 --- a/app/core/process.py +++ b/app/core/process.py @@ -38,3 +38,53 @@ def process_exists(pid: int) -> bool: kernel32.CloseHandle(handle) return True return ctypes.get_last_error() != ERROR_INVALID_PARAMETER + + +_PARENT_POLL_SECONDS = 1.0 + + +def _watch_parent(parent_pid: int) -> None: + """Exit as soon as the process that spawned us is gone. + + A worker's stdin EOF only covers a parent that exits between jobs. + Mid-inference the worker reads nothing, and the parent may have been killed + in a way that ran no cleanup at all (SIGKILL, Force Quit, Task Manager, a + crash). Without this the worker keeps running -- holding a GPU, in the + demucs and vocal-split cases -- with nobody left to collect the result. + + os._exit rather than sys.exit: this runs on a daemon thread, and raising + SystemExit there would not interrupt inference running in C code. Nothing + here needs flushing. + """ + import sys + import time + + while True: + if not process_exists(parent_pid): + sys.stderr.write("@@ERROR@@parent process exited\n") + sys.stderr.flush() + os._exit(1) + time.sleep(_PARENT_POLL_SECONDS) + + +def arm_parent_watchdog() -> None: + """Start the parent-death watchdog if the parent asked for one. + + Shared by every long-running worker. It lived in demucs_worker.py, which is + why vocal_split_worker and section_worker never had it: a Force-Quit during + a vocal split orphaned an onnxruntime process holding the GPU, and a + section pass outlived the parent whose TIMEOUT_SECTIONS was its only bound + (#519). + """ + import threading + + raw = os.environ.get("STEMDECK_PARENT_PID", "").strip() + if not raw: + return + try: + parent_pid = int(raw) + except ValueError: + return + if parent_pid <= 0 or parent_pid == os.getpid(): + return + threading.Thread(target=_watch_parent, args=(parent_pid,), daemon=True).start() diff --git a/app/core/registry.py b/app/core/registry.py index dc70c39b..eaffc421 100644 --- a/app/core/registry.py +++ b/app/core/registry.py @@ -38,6 +38,14 @@ # Ids restore() wants re-queued, drained once by the app lifespan. _pending_resume: list[str] = [] +# Job ids the user deleted. Orphan recovery in restore() adopts any job-shaped +# directory it finds, which is what resurrected tracks whose files failed to +# delete (#521). A directory that outlived its delete must not come back, and +# the client-side tombstone cannot be relied on for that -- "Reset app data" +# wipes it. Persisted with the registry; see _prune_deleted for why it stays +# small. +_deleted: set[str] = set() + def register(job: Job) -> Job: with _lock: @@ -105,6 +113,24 @@ def registry_path(jobs_dir: Path) -> Path: return jobs_dir / _REGISTRY_FILE +def mark_deleted(job_id: str) -> None: + """Record that the user deleted this job, so restore() will not re-adopt + its directory if the files outlived the delete.""" + with _lock: + _deleted.add(job_id) + + +def _prune_deleted(jobs_dir: Path) -> None: + """Forget deletion records whose directory is finally gone. + + A record only has to outlive the directory it refers to, so this keeps the + set naturally bounded instead of growing for the life of the install. + Caller must not hold _lock.""" + with _lock: + stale = {job_id for job_id in _deleted if not (jobs_dir / job_id).exists()} + _deleted.difference_update(stale) + + def persist(jobs_dir: Path) -> None: """Persist terminal jobs so completed library entries survive restarts. @@ -120,13 +146,24 @@ def persist(jobs_dir: Path) -> None: logger.warning("cannot create jobs dir %s; skipping persist", jobs_dir, exc_info=True) return path = registry_path(jobs_dir) + _prune_deleted(jobs_dir) with _lock: records = [ job.to_record() for job in sorted(_jobs.values(), key=lambda item: item.created_at) if job.status in _PERSISTED ] - payload = json.dumps({"version": REGISTRY_VERSION, "jobs": records}, indent=2) + "\n" + payload = ( + json.dumps( + { + "version": REGISTRY_VERSION, + "jobs": records, + "deleted": sorted(_deleted), + }, + indent=2, + ) + + "\n" + ) tmp = jobs_dir / f".registry.{uuid.uuid4().hex}.tmp" try: tmp.write_text(payload, encoding="utf-8") @@ -145,6 +182,10 @@ def restore(jobs_dir: Path) -> None: if path.is_file(): try: data = _migrate(json.loads(path.read_text(encoding="utf-8"))) + recorded = data.get("deleted") + if isinstance(recorded, list): + with _lock: + _deleted.update(str(job_id) for job_id in recorded) to_add = {} resume: list[Job] = [] for record in data.get("jobs", []): @@ -175,21 +216,38 @@ def restore(jobs_dir: Path) -> None: _pending_resume.extend( j.id for j in sorted(resume, key=lambda j: (j.queue_position, j.created_at)) ) - except (OSError, json.JSONDecodeError, TypeError, ValueError): + except Exception: + # Broad on purpose. restore() runs at import time, so anything that + # escapes here stops the backend booting with no way for a user to + # recover short of deleting the file by hand. A registry.json that + # is valid JSON but not an object (a top-level list, null, a bare + # string) used to do exactly that: _migrate calls data.get() and + # raises AttributeError, which the old tuple did not name (#520). logger.warning("failed to load registry from %s", path, exc_info=True) - with _lock: - known = set(_jobs) - for job_dir in jobs_dir.iterdir(): - if not job_dir.is_dir() or not JOB_ID_RE.match(job_dir.name) or job_dir.name in known: - continue - recovered = _recover_done_job(job_dir) - if recovered is not None: - with _lock: - _jobs[recovered.id] = recovered - changed = True - if changed: - persist(jobs_dir) + # Orphan recovery, and the persist that follows it, were outside the guard + # above -- an unreadable jobs_dir or a failed write was fatal at startup for + # the same reason. + try: + with _lock: + known = set(_jobs) + deleted = set(_deleted) + for job_dir in jobs_dir.iterdir(): + if not job_dir.is_dir() or not JOB_ID_RE.match(job_dir.name) or job_dir.name in known: + continue + if job_dir.name in deleted: + # The user deleted this and its files outlived the delete. + # Adopting it here is what brought songs back (#521). + continue + recovered = _recover_done_job(job_dir) + if recovered is not None: + with _lock: + _jobs[recovered.id] = recovered + changed = True + if changed: + persist(jobs_dir) + except Exception: + logger.warning("failed to recover jobs from %s", jobs_dir, exc_info=True) def _resume_or_recover(job: Job, job_dir: Path) -> Job | None: @@ -306,7 +364,7 @@ def set_proc(job_id: str, proc: subprocess.Popen | None) -> None: _procs[job_id] = proc -def reset_all(jobs_dir: Path) -> None: +def reset_all(jobs_dir: Path) -> list[str]: """Delete every job directory and the registry file, clearing the in-memory registry too. Desktop-only factory reset (Settings -> General -> "Reset app data") -- the caller is responsible for checking no job is @@ -316,7 +374,8 @@ def reset_all(jobs_dir: Path) -> None: _jobs.clear() _procs.clear() if not jobs_dir.is_dir(): - return + return [] + failed: list[str] = [] for entry in jobs_dir.iterdir(): try: if entry.is_dir(): @@ -325,6 +384,17 @@ def reset_all(jobs_dir: Path) -> None: entry.unlink() except OSError: logger.warning("reset: could not remove %s", entry, exc_info=True) + failed.append(entry.name) + # Anything that survived is still a job-shaped directory on disk, so the + # next start would adopt it and the "reset" library would refill itself. + # Record it as deleted and persist that, which also recreates the registry + # file this loop just removed. + surviving = [name for name in failed if JOB_ID_RE.match(name)] + if surviving: + with _lock: + _deleted.update(surviving) + persist(jobs_dir) + return failed def get_proc(job_id: str) -> subprocess.Popen | None: diff --git a/app/core/settings.py b/app/core/settings.py index c50da598..da093aa2 100644 --- a/app/core/settings.py +++ b/app/core/settings.py @@ -25,6 +25,8 @@ import logging import os import threading +import time +import uuid from pathlib import Path from app.core.config import ( @@ -70,16 +72,97 @@ def _default_allow_network() -> bool: return os.environ.get("STEMDECK_DESKTOP") != "1" -def _load() -> dict: +def _mirror_path() -> Path | None: + """Where the per-user copy lives, or None when the shell did not set one. + + The path comes from the shell (STEMDECK_SETTINGS_MIRROR) so the platform + logic stays in one place -- see _mirror_settings.""" + target = os.environ.get("STEMDECK_SETTINGS_MIRROR", "").strip() + return Path(target) if target else None + + +def _read_json_dict(path: Path) -> dict | None: + """Parse `path` as a JSON object. + + None means "there is nothing usable here" -- absent, unreadable, not JSON, + or JSON that is not an object. Callers that need to tell *absent* from + *unusable* must check existence themselves; that distinction is the whole + point of _load below.""" try: - data = json.loads(_SETTINGS_PATH.read_text(encoding="utf-8")) - if isinstance(data, dict): - return data + data = json.loads(path.read_text(encoding="utf-8")) except FileNotFoundError: - pass # no settings file yet — first run; use defaults + return None + except Exception: + _log.warning("could not read settings from %s", path, exc_info=True) + return None + return data if isinstance(data, dict) else None + + +def _atomic_write_json(path: Path, data: dict) -> bool: + """Write `data` to `path` so an interrupted write cannot destroy what was + there before. + + write_text() truncates first and writes second, so a process that dies in + between leaves a file that exists and does not parse -- which _load then + could not distinguish from a first run, and the next setting change + persisted a one-key file over both this and the mirror (#509). Same + same-directory temp + replace the registry already uses; the temp name is + unique per call so two concurrent writers cannot interleave on it.""" + try: + path.parent.mkdir(parents=True, exist_ok=True) + tmp = path.with_name(f"{path.name}.{uuid.uuid4().hex}.tmp") + try: + tmp.write_text(json.dumps(data), encoding="utf-8") + tmp.replace(path) + finally: + tmp.unlink(missing_ok=True) except Exception: - # Corrupt/unreadable file: fall back to defaults rather than crash. - _log.warning("could not read settings from %s", _SETTINGS_PATH, exc_info=True) + _log.warning("could not persist settings to %s", path, exc_info=True) + return False + return True + + +def _quarantine_corrupt(path: Path) -> None: + """Move an unusable settings file aside rather than leaving it to be + overwritten by the next save. + + Renaming keeps the bytes for diagnosis. Deleting or writing over them + destroys the only remaining evidence of what the user had configured.""" + try: + target = path.with_name(f"{path.name}.corrupt-{int(time.time())}") + path.replace(target) + _log.warning("settings at %s were unreadable; moved aside to %s", path, target) + except OSError: + _log.warning("could not move unreadable settings at %s aside", path, exc_info=True) + + +def _load() -> dict: + """Read settings, telling "no file yet" apart from "file we cannot read". + + Conflating the two is what lost real user settings: a torn write left an + unparsable file, this returned {} exactly as it would on a first run, and + the next set_*() then persisted a single key over both settings.json and + the mirror that existed to protect it.""" + if not _SETTINGS_PATH.exists(): + return {} # no settings file yet — genuine first run; use defaults + + data = _read_json_dict(_SETTINGS_PATH) + if data is not None: + return data + + # The file is there but unusable. Preserve it, then try the per-user copy + # the shell keeps outside the install directory. + _quarantine_corrupt(_SETTINGS_PATH) + mirror = _mirror_path() + if mirror is not None: + recovered = _read_json_dict(mirror) + if recovered: + _log.warning("recovered settings from mirror %s", mirror) + # Put them back immediately. Without this the recovery only lasts + # until the next start, which would read a now-absent primary and + # silently fall back to defaults again. + _atomic_write_json(_SETTINGS_PATH, recovered) + return recovered return {} @@ -106,11 +189,7 @@ def _save() -> bool: is still reported back, because one caller (set_jobs_dir) is coupled to something irreversible enough that silently swallowing a failure there would be actively misleading rather than merely inconvenient (#403).""" - try: - _SETTINGS_PATH.parent.mkdir(parents=True, exist_ok=True) - _SETTINGS_PATH.write_text(json.dumps(_ensure()), encoding="utf-8") - except Exception: - _log.warning("could not persist settings to %s", _SETTINGS_PATH, exc_info=True) + if not _atomic_write_json(_SETTINGS_PATH, _ensure()): return False _mirror_settings() return True @@ -135,19 +214,13 @@ def _mirror_settings() -> None: the shell (STEMDECK_SETTINGS_MIRROR) so the platform logic stays in one place and both halves cannot drift apart. """ - target = os.environ.get("STEMDECK_SETTINGS_MIRROR", "").strip() - if not target: + path = _mirror_path() + if path is None: return - try: - path = Path(target) - path.parent.mkdir(parents=True, exist_ok=True) - # Same-directory temp + replace: a torn write here would be restored - # verbatim into the user's next install. - tmp = path.with_suffix(".json.tmp") - tmp.write_text(json.dumps(_ensure()), encoding="utf-8") - tmp.replace(path) - except Exception: - _log.warning("could not mirror settings to %s", target, exc_info=True) + # Same-directory temp + replace: a torn write here would be restored + # verbatim into the user's next install. _atomic_write_json also gives the + # temp file a unique name, so two writers cannot interleave on it. + _atomic_write_json(path, _ensure()) def _num(v: object) -> int | None: diff --git a/app/main.py b/app/main.py index 99dc0ff6..54622795 100644 --- a/app/main.py +++ b/app/main.py @@ -590,8 +590,15 @@ def reset_app_data() -> dict[str, object]: from app.pipeline import jobqueue jobqueue.clear() - reset_registry(JOBS_DIR) - return {"ok": True} + # Anything that could not be removed is reported rather than swallowed: the + # frontend used to take an unconditional {"ok": true} as licence to wipe its + # own deletion tombstone, and any surviving directory was then re-adopted on + # the next start with nothing left to suppress it (#521). reset_all also + # records the survivors server-side, so they stay deleted regardless. + undeleted = reset_registry(JOBS_DIR) + if undeleted: + _log.warning("reset left %d entries on disk: %s", len(undeleted), undeleted) + return {"ok": True, "undeleted": len(undeleted)} @app.get("/api/registry", tags=["settings"]) @@ -850,17 +857,44 @@ def download_logs_zip() -> StreamingResponse: # The ceiling is far above either editor's reach: 10000 sections with the # longest name each is about 1.6 MB, and 20000 beats about 0.4 MB. Uploads are # unaffected -- they are a different path with their own 400 MB limit. -_EDITOR_BODY_LIMIT = 4 * 1024 * 1024 -_EDITOR_PATH_SUFFIXES = ("/sections", "/beats") +_JSON_BODY_LIMIT = 4 * 1024 * 1024 +# Multipart uploads stream to disk and enforce their own, much larger, limit. + + +def _is_upload(request: Request) -> bool: + ctype = request.headers.get("content-type", "") + return ctype.startswith("multipart/form-data") + + +def _is_chunked(request: Request) -> bool: + return "chunked" in request.headers.get("transfer-encoding", "").lower() @app.middleware("http") -async def limit_editor_body_size(request: Request, call_next): - if request.method in ("PATCH", "POST", "PUT") and request.url.path.endswith( - _EDITOR_PATH_SUFFIXES - ): +async def limit_json_body_size(request: Request, call_next): + """Cap JSON request bodies. + + Scoped by path suffix before, which left every other JSON endpoint + uncapped: /api/search, /api/playlist, /api/settings and the JSON branch of + /api/jobs all await request.json(), and Starlette accumulates the whole + body before json.loads runs it on the event loop. A 200 MB body to + /api/search stalled every other request, including a running job's progress + stream, with no valid job or prior state needed (#481, reopened as #512). + + Uploads are exempt: they are multipart, not JSON, and carry their own + 400 MB limit on a path that streams to disk rather than buffering. + """ + if request.method in ("PATCH", "POST", "PUT") and not _is_upload(request): declared = request.headers.get("content-length") - if declared and declared.isdigit() and int(declared) > _EDITOR_BODY_LIMIT: + if declared is None: + # No Content-Length means chunked, which used to skip the check + # entirely and fall through to an unbounded request.body(). + if _is_chunked(request): + return JSONResponse( + {"detail": "request body must declare its length"}, + status_code=411, + ) + elif declared.isdigit() and int(declared) > _JSON_BODY_LIMIT: return JSONResponse({"detail": "request body too large"}, status_code=413) return await call_next(request) diff --git a/app/pipeline/click_render.py b/app/pipeline/click_render.py index 0db91943..e94709a2 100644 --- a/app/pipeline/click_render.py +++ b/app/pipeline/click_render.py @@ -258,7 +258,11 @@ def _render_events( import numpy as np - buf = np.zeros(total, dtype=np.float64) + # float32, not float64: the buffer is one sample per frame for the whole + # render, so a long export was allocating twice what it needed and then + # again in the int16 conversion below. The output is 16-bit PCM, so the + # extra mantissa was never audible (#512). + buf = np.zeros(total, dtype=np.float32) # Only two distinct voices, so render each once and stamp it in. plain = _voice(CLICK_PEAK, CLICK_FREQ, sample_rate) accented = _voice(ACCENT_PEAK, ACCENT_FREQ, sample_rate) diff --git a/app/pipeline/demucs_worker.py b/app/pipeline/demucs_worker.py index 482cc0ae..dc679ca8 100644 --- a/app/pipeline/demucs_worker.py +++ b/app/pipeline/demucs_worker.py @@ -35,14 +35,11 @@ from __future__ import annotations import json -import os import sys -import threading -import time from pathlib import Path from app.core.config import DEMUCS_MODEL -from app.core.process import process_exists +from app.core.process import arm_parent_watchdog def _run_one_job(model, device: str, req: dict) -> None: @@ -89,47 +86,9 @@ def _run_one_job(model, device: str, req: dict) -> None: ) -_PARENT_POLL_SECONDS = 1.0 - - -def _watch_parent(parent_pid: int) -> None: - """Exit as soon as the process that spawned us is gone. - - The stdin EOF in the loop below only covers a parent that exits between - jobs. Mid-separation the worker is inside torch and reads nothing, and the - parent may have been killed in a way that ran no cleanup at all (SIGKILL, - Force Quit, Task Manager, a crash). Without this, the worker would keep a - GPU busy with nobody left to collect the result. - - os._exit rather than sys.exit: this runs on a daemon thread, and raising - SystemExit there would not interrupt inference running in C code. Nothing - here needs flushing -- a half-written model directory is cleared before the - job is retried. - """ - while True: - if not process_exists(parent_pid): - sys.stderr.write("@@ERROR@@parent process exited\n") - sys.stderr.flush() - os._exit(1) - time.sleep(_PARENT_POLL_SECONDS) - - -def _arm_parent_watchdog() -> None: - raw = os.environ.get("STEMDECK_PARENT_PID", "").strip() - if not raw: - return - try: - parent_pid = int(raw) - except ValueError: - return - if parent_pid <= 0 or parent_pid == os.getpid(): - return - threading.Thread(target=_watch_parent, args=(parent_pid,), daemon=True).start() - - def main() -> None: device = sys.argv[1] if len(sys.argv) > 1 else "cpu" - _arm_parent_watchdog() + arm_parent_watchdog() from demucs.pretrained import get_model diff --git a/app/pipeline/jobqueue.py b/app/pipeline/jobqueue.py index ae1b61b0..ae872b23 100644 --- a/app/pipeline/jobqueue.py +++ b/app/pipeline/jobqueue.py @@ -200,6 +200,21 @@ async def _dispatch(job: Job) -> None: await run_pipeline(job, source_url, JOBS_DIR) +def _finalise_dropped_job(job: Job) -> None: + """Complete a cancellation the API could not finish itself. + + cancel_job can only finalise a job it still finds in the queue. Between + _pop_next() and _set_running() a job is in neither the queue nor the + running slot, so a cancel arriving there sets cancel_requested and returns. + The worker owns the job by then and is the only thing that can close it + out (#520).""" + if not job.cancel_requested or job.status in ("done", "error", "cancelled"): + return + _set(job, status="cancelled", stage="Cancelled") + cleanup_job_dir(job.id) + registry_persist(JOBS_DIR) + + async def _worker_loop() -> None: assert _wake is not None while not _stopping: @@ -220,7 +235,19 @@ async def _worker_loop() -> None: if job is None: continue if job.cancel_requested or job.status in ("done", "error", "cancelled"): - # Cancelled or finished while it waited; drop it silently. + # Cancelled or finished while it waited. + # + # Finalising here is not optional. Between _pop_next() above and + # _set_running() below the job is in neither the queue nor the + # running slot, so a cancel arriving in that window finds + # discard() False and running_id() None and returns having only + # set cancel_requested. This worker is the sole consumer and owns + # the job by now, so if it just dropped it the job would sit at + # "queued" forever: absent from the queue view, still counted by + # pending_count against the capacity limit, its uploaded source + # never freed, and -- because "queued" is persisted -- re-queued + # on every restart (#520). + _finalise_dropped_job(job) continue # Claim it. No await between the pop and this status write, so a job is diff --git a/app/pipeline/runner.py b/app/pipeline/runner.py index 07603ab4..f4b3c6a0 100644 --- a/app/pipeline/runner.py +++ b/app/pipeline/runner.py @@ -14,6 +14,7 @@ from app.core.models import Job, JobCancelled, _set from app.core.redact import redact from app.core.registry import persist as persist_registry +from app.core.registry import set_proc from app.pipeline.analyze import analyze from app.pipeline.beatgrid import compute_beat_grid from app.pipeline.collect import ( @@ -49,6 +50,30 @@ def _check_cancel(job: Job) -> None: raise JobCancelled() +def _run_registered_ffmpeg(job: Job, cmd: list[str], timeout: int) -> tuple[int, bytes]: + """Run ffmpeg with the process registered, so cancel can reach it. + + subprocess.run() cannot be interrupted: POST /cancel sets the flag, but + nothing looks at it until the call returns, so a cancel during a large + upload's transcode was a no-op for up to TIMEOUT_FFMPEG per call -- twice + over on the .mp4 path, which runs both this and the video extract (#519). + + Mirrors collect._run_ffmpeg, which registers for exactly this reason. + """ + proc = subprocess.Popen(cmd, stdout=subprocess.DEVNULL, stderr=subprocess.PIPE) + set_proc(job.id, proc) + try: + try: + _, stderr = proc.communicate(timeout=timeout) + except subprocess.TimeoutExpired: + proc.kill() + proc.communicate() + raise + return proc.returncode, stderr or b"" + finally: + set_proc(job.id, None) + + def _extract_video_track(job: Job, source: Path, job_dir: Path) -> None: """For an .mp4 upload, preserve a silent video-only track at video.mp4 so the studio can later mux it with a custom stem mix @@ -76,7 +101,7 @@ def _extract_video_track(job: Job, source: Path, job_dir: Path) -> None: str(dest), ] try: - result = subprocess.run(cmd, capture_output=True, timeout=TIMEOUT_FFMPEG) + returncode, _ = _run_registered_ffmpeg(job, cmd, TIMEOUT_FFMPEG) except (OSError, subprocess.SubprocessError) as e: # ffmpeg missing or timed out. Distinct from an .mp4 that simply has no # video stream, and the only one of the two worth surfacing (#436). @@ -84,7 +109,7 @@ def _extract_video_track(job: Job, source: Path, job_dir: Path) -> None: job.video_status = "failed" logger.warning("video extract failed for job %s: %s", job.id, e) return - if result.returncode != 0 or not dest.is_file() or dest.stat().st_size == 0: + if returncode != 0 or not dest.is_file() or dest.stat().st_size == 0: dest.unlink(missing_ok=True) job.video_status = "unavailable" logger.info("no video track preserved for job %s (source has no video stream?)", job.id) @@ -127,10 +152,10 @@ def _prepare_local_source(job: Job, source: Path, job_dir: Path) -> Path: "-y", str(dest), ] - result = subprocess.run(cmd, capture_output=True, timeout=TIMEOUT_FFMPEG) - if result.returncode != 0: + returncode, stderr = _run_registered_ffmpeg(job, cmd, TIMEOUT_FFMPEG) + if returncode != 0: raise RuntimeError( - "ffmpeg transcode failed: " + result.stderr.decode("utf-8", errors="replace").strip() + "ffmpeg transcode failed: " + stderr.decode("utf-8", errors="replace").strip() ) source.unlink(missing_ok=True) return dest diff --git a/app/pipeline/section_worker.py b/app/pipeline/section_worker.py index 0d336317..7998404d 100644 --- a/app/pipeline/section_worker.py +++ b/app/pipeline/section_worker.py @@ -10,6 +10,7 @@ import threading from pathlib import Path +from app.core.process import arm_parent_watchdog from app.pipeline.section_refine import refine_segments _HEARTBEAT_SECONDS = 10 @@ -66,6 +67,9 @@ def _load_beat_grid(path: Path | None) -> object | None: def main(argv: list[str] | None = None) -> int: + # Without this a CPU inference pass outlives the parent whose + # TIMEOUT_SECTIONS was its only bound (#519). + arm_parent_watchdog() args = _parser().parse_args(argv) for path in (args.stems_dir / f"{name}.wav" for name in ("bass", "drums", "other", "vocals")): if not path.is_file(): diff --git a/app/pipeline/sections.py b/app/pipeline/sections.py index 7e509b40..0a994f16 100644 --- a/app/pipeline/sections.py +++ b/app/pipeline/sections.py @@ -214,6 +214,10 @@ def _terminate(proc: subprocess.Popen) -> None: def _run_registered_process(job: Job, cmd: list[str]) -> tuple[int, list[str], list[str]]: """Run a child with cancellation, total timeout, and output-stall detection.""" env = os.environ.copy() + # The worker arms a watchdog on this and hard-exits when we disappear, so a + # kill that runs no cleanup (SIGKILL, Force Quit, Task Manager, a crash) + # cannot leave it running with nobody to collect the result (#519). + env["STEMDECK_PARENT_PID"] = str(os.getpid()) env["PYTHONIOENCODING"] = "utf-8:replace" proc = subprocess.Popen( cmd, diff --git a/app/pipeline/separate.py b/app/pipeline/separate.py index 609ff3d0..52cd4d6a 100644 --- a/app/pipeline/separate.py +++ b/app/pipeline/separate.py @@ -48,6 +48,11 @@ def _kill_worker() -> None: proc.wait(timeout=5) except subprocess.TimeoutExpired: proc.kill() + # kill() only sends the signal. Without the reap a worker wedged in + # an uninterruptible CUDA call becomes a zombie whose pipes are + # closed only incidentally, whenever the Popen refcount happens to + # drop. vocal_split.py already pairs the two. + proc.communicate() def _get_worker(device: str) -> subprocess.Popen: @@ -108,6 +113,10 @@ def _run_demucs(job: Job, source: Path, job_dir: Path, device: str) -> tuple[int spawn_at = time.monotonic() proc = _get_worker(device) if proc.stdin is None or proc.stderr is None: + # Raised before the try below, so nothing else tears this down, and a + # worker without pipes would otherwise stay cached and be handed to + # every subsequent job. + _kill_worker() raise RuntimeError("demucs worker has no stdin/stderr pipe") set_proc(job.id, proc) @@ -203,13 +212,18 @@ def _watchdog() -> None: _done_evt.set() set_proc(job.id, None) wt.join(timeout=2) - - # Never reuse a worker after anything but a clean success: a cancel - # (proc.terminate() from the API thread) already killed it; a failure's - # GPU/CUDA state afterward isn't something we can vouch for. Only the - # happy path keeps the worker warm for the next job. - if job_ok is not True: - _kill_worker() + # Never reuse a worker after anything but a clean success: a cancel + # (proc.terminate() from the API thread) already killed it; a failure's + # GPU/CUDA state afterward isn't something we can vouch for. Only the + # happy path keeps the worker warm for the next job. + # + # Inside the finally, not after 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 -- skipped this + # entirely and left a worker whose CUDA state followed an exception + # warm for the next job (#514). + if job_ok is not True: + _kill_worker() # POST /cancel calls proc.terminate() directly, which causes the read # loop above to hit EOF. Translate that into JobCancelled before the @@ -253,6 +267,11 @@ def separate(job: Job, source: Path, job_dir: Path) -> Path: # Partial output from the failed attempt must not be mistaken for # results by collect(); CPU restarts from scratch, so does progress. shutil.rmtree(job_dir / DEMUCS_MODEL, ignore_errors=True) + # The rmtree above can take seconds on a multi-GB partial result, and + # nothing is registered for cancel during it. Re-check before paying + # for a full CPU pass the user has already asked to stop (#514). + if job.cancel_requested: + raise JobCancelled() _set(job, progress=0.0, stage="GPU failed — retrying on CPU (slower)...") job.gpu_fallback = True job.compute_device = f"cpu (fallback from {device})" diff --git a/app/pipeline/vocal_split.py b/app/pipeline/vocal_split.py index 65cb224e..a3d1ebf8 100644 --- a/app/pipeline/vocal_split.py +++ b/app/pipeline/vocal_split.py @@ -61,6 +61,10 @@ def split_vocals(job: Job, stems_dir: Path) -> list[str]: # the mismatch simply moves rather than being fixed. Demucs and audio- # separator both emit progress bars and can echo track metadata, neither of # which is guaranteed to be cp1252-safe. + # The worker arms a watchdog on this and hard-exits when we disappear, so a + # kill that runs no cleanup (SIGKILL, Force Quit, Task Manager, a crash) + # cannot leave it running with nobody to collect the result (#519). + env["STEMDECK_PARENT_PID"] = str(os.getpid()) env["PYTHONIOENCODING"] = "utf-8:replace" try: import certifi diff --git a/app/pipeline/vocal_split_worker.py b/app/pipeline/vocal_split_worker.py index 116b9eb1..fe305c6b 100644 --- a/app/pipeline/vocal_split_worker.py +++ b/app/pipeline/vocal_split_worker.py @@ -24,6 +24,7 @@ import sys from app.core.config import VOCAL_SPLIT_MODEL +from app.core.process import arm_parent_watchdog def _run(device: str, vocals_path: str, out_dir: str) -> None: @@ -57,6 +58,10 @@ def _run(device: str, vocals_path: str, out_dir: str) -> None: def main() -> None: + # A Force-Quit of the app otherwise orphans this process holding the GPU: + # onnxruntime reads nothing from stdin mid-inference, so EOF never arrives + # and nothing else bounds it (#519). + arm_parent_watchdog() if len(sys.argv) < 4: sys.stderr.write("@@ERROR@@usage: vocal_split_worker \n") sys.stderr.flush() diff --git a/build/Dockerfile b/build/Dockerfile index 5405f55c..cf0c38db 100644 --- a/build/Dockerfile +++ b/build/Dockerfile @@ -29,13 +29,23 @@ RUN apt-get update \ # deno -- yt-dlp uses it as the JS runtime for YouTube format # extraction. Staged here so the runner doesn't need curl/unzip. +# Pinned and checksummed, not "latest". Deno is the JS runtime yt-dlp feeds +# YouTube's challenge payload to, so it executes against untrusted input -- +# and an unverified binary fetched at build time is a supply-chain hole +# whether or not it is small. The desktop packaging scripts already pin and +# verify QuickJS for exactly this reason; this was the one path that did not. +# Bump DENO_VERSION and both hashes together. ARG TARGETARCH +ARG DENO_VERSION=v2.9.6 +ARG DENO_SHA256_AMD64=394f07f4da2bebe6ce6f1e7ce0fa16429b29b08c35e3fac3fe25972676dff4b2 +ARG DENO_SHA256_ARM64=9a46afc6c392c7cd2ff71a31558935545b46408d0e87f7a86908c712721c046e RUN case "${TARGETARCH:-$(dpkg --print-architecture)}" in \ - amd64) DENO_ARCH=x86_64-unknown-linux-gnu ;; \ - arm64) DENO_ARCH=aarch64-unknown-linux-gnu ;; \ + amd64) DENO_ARCH=x86_64-unknown-linux-gnu; DENO_SHA256="${DENO_SHA256_AMD64}" ;; \ + arm64) DENO_ARCH=aarch64-unknown-linux-gnu; DENO_SHA256="${DENO_SHA256_ARM64}" ;; \ *) echo "Unsupported arch: ${TARGETARCH}" && exit 1 ;; \ esac \ - && curl -fsSL -o /tmp/deno.zip "https://github.com/denoland/deno/releases/latest/download/deno-${DENO_ARCH}.zip" \ + && curl -fsSL -o /tmp/deno.zip "https://github.com/denoland/deno/releases/download/${DENO_VERSION}/deno-${DENO_ARCH}.zip" \ + && echo "${DENO_SHA256} /tmp/deno.zip" | sha256sum -c - \ && unzip /tmp/deno.zip -d /usr/local/bin \ && rm /tmp/deno.zip \ && /usr/local/bin/deno --version diff --git a/desktop/src-tauri/src/main.rs b/desktop/src-tauri/src/main.rs index 40145f5e..4918e156 100644 --- a/desktop/src-tauri/src/main.rs +++ b/desktop/src-tauri/src/main.rs @@ -16,7 +16,7 @@ use std::{ thread, time::{Duration, Instant, SystemTime, UNIX_EPOCH}, }; -use tar::Archive; +use tar::{Archive, EntryType}; use tauri::{Emitter, Manager}; use tauri_plugin_store::StoreExt; #[cfg(windows)] @@ -111,6 +111,24 @@ const SHAKA_FFPROBE_SHA256_X64: &str = #[cfg(all(unix, not(target_os = "macos")))] const DEFAULT_LINUX_FFMPEG_URL: &str = "https://johnvansickle.com/ffmpeg/releases/ffmpeg-release-amd64-static.tar.xz"; +// Pinned like the macOS hashes above, because this binary is downloaded, +// marked executable and run: without it the only thing standing between a +// compromised or MITM'd host and code execution was that the download +// completed (#518). +// +// Upstream publishes only a .md5 companion, which is both cryptographically +// broken for collisions and served by the same host as the tarball -- an +// attacker able to replace one can replace the other, so it evidences +// corruption, not authenticity. This hash was computed from the artifact whose +// MD5 matched upstream's published 7fa72b652e19bf84c9461e332ea1cdf3. +// +// 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 both, for anyone who needs it. +#[cfg(all(unix, not(target_os = "macos")))] +const DEFAULT_LINUX_FFMPEG_SHA256: &str = + "abda8d77ce8309141f83ab8edf0596834087c52467f6badf376a6a2a4c87cf67"; struct BackendHandles { child: Child, @@ -1000,8 +1018,14 @@ async fn download_app_update( // never install something the current plan did not ask for. let _ = fs::remove_file(&app_archive); + validate_release_url(&plan.app_url)?; download_file_with_progress(&plan.app_url, &app_archive, &app_handle).await?; - verify_update_sha256(&app_archive, &plan.app_sha256, "app update") + verify_update_sha256(&app_archive, &plan.app_sha256, "app update")?; + // Record what was verified so apply_app_update can check the bytes it + // is about to extract, rather than trusting that whatever now sits at + // this path is what this function approved. + let _ = fs::write(app_sha_path(&downloads), plan.app_sha256.trim()); + Ok(()) } } @@ -1010,6 +1034,13 @@ async fn download_app_update( /// the frontend) before it is ever extracted. On mismatch the file is removed /// so a corrupt or tampered download can never be applied. #[cfg(any(windows, target_os = "linux"))] +/// Where download_app_update records the checksum it verified, so +/// apply_app_update can re-check the bytes it is about to extract. +#[cfg(any(windows, target_os = "linux"))] +fn app_sha_path(downloads: &Path) -> PathBuf { + downloads.join(format!("{UPDATE_APP_ARCHIVE}.sha256")) +} + fn verify_update_sha256(path: &Path, expected: &str, label: &str) -> Result<(), String> { let actual = sha256_file(path)?; if !actual.eq_ignore_ascii_case(expected.trim()) { @@ -1181,6 +1212,13 @@ fn apply_app_update( "no downloaded app update found -- call download_app_update first".to_string(), ); } + // Re-verify rather than trusting the path. download_app_update checked + // these bytes, but anything able to write into data/downloads between + // the two calls would otherwise be extracted over the live install + // unchecked (#510). + let recorded = fs::read_to_string(app_sha_path(&downloads)) + .map_err(|_| "no verified checksum for the downloaded update -- download it again")?; + verify_update_sha256(&app_archive, recorded.trim(), "app update")?; // ── Phase 1: stage and validate, touching nothing live ── // @@ -2485,6 +2523,38 @@ fn open_url(url: String) -> Result<(), String> { /// Only localhost URLs, and only http(s). Guards against a compromised WebView /// using the desktop shell as an SSRF proxy (#138). +/// Hosts an in-app update may be fetched from. +/// +/// GitHub serves release assets from `github.com` and redirects to +/// `objects.githubusercontent.com`, so both have to be here. +const RELEASE_ASSET_HOSTS: [&str; 2] = ["github.com", "objects.githubusercontent.com"]; + +/// Reject an update URL that does not point at our own release assets. +/// +/// `download_app_update` takes its URL from the WebView, and the SHA-256 it +/// checks against comes from the same place -- so the checksum proves the file +/// arrived intact, not that it came from us. Without a host check, anything +/// able to run script on that page can hand the shell an archive that +/// `apply_app_update` then extracts over StemDeck's own executable and +/// backend/ (#510). 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. +/// +/// Deliberately not `validate_download_url`: that one permits only +/// 127.0.0.1/localhost, for a different caller, and would reject every real +/// release URL. +fn validate_release_url(url: &str) -> Result<(), String> { + let parsed = reqwest::Url::parse(url).map_err(|_| "invalid update URL".to_string())?; + if parsed.scheme() != "https" { + return Err("update URLs must use https".to_string()); + } + let host = parsed.host_str().unwrap_or(""); + if !RELEASE_ASSET_HOSTS.contains(&host) { + return Err(format!("refusing to download an update from {host}")); + } + Ok(()) +} + fn validate_download_url(url: &str) -> Result<(), String> { if !url.starts_with("http://") && !url.starts_with("https://") { return Err("only http/https URLs are permitted".to_string()); @@ -3187,13 +3257,34 @@ fn is_apple_double(path: &Path) -> bool { .is_some_and(|name| name.starts_with("._")) } +/// Stands in for `Archive::unpack`, which cannot be used because it offers no +/// way to skip an entry. It has to reproduce the two things `unpack` does +/// beyond looping over entries, both of which the first version of this +/// function dropped: +/// +/// 1. **Directories are applied last**, reverse-sorted by path, because 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 with EACCES -- first-run setup dies with no fallback. +/// Upstream calls this out as tar-rs#242. +/// 2. **`destination` is canonicalized up front**, which on Windows supplies +/// the `\\?\` prefix so member paths over 260 characters still extract. +/// +/// Traversal protection needs nothing here: `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. fn unpack_without_apple_double( mut archive: Archive, destination: &Path, ) -> Result<(), String> { + let destination = destination + .canonicalize() + .unwrap_or_else(|_| destination.to_path_buf()); + let entries = archive .entries() .map_err(|e| format!("failed to read runtime pack: {e}"))?; + let mut directories = Vec::new(); for entry in entries { let mut entry = entry.map_err(|e| format!("failed to read runtime pack: {e}"))?; let path = entry @@ -3203,8 +3294,20 @@ fn unpack_without_apple_double( if is_apple_double(&path) { continue; } + // Directories hold no data, so deferring them reads nothing back off a + // streaming archive -- only their metadata is applied later. + if entry.header().entry_type() == EntryType::Directory { + directories.push(entry); + continue; + } entry - .unpack_in(destination) + .unpack_in(&destination) + .map_err(|e| format!("failed to extract runtime pack: {e}"))?; + } + + directories.sort_by(|a, b| b.path_bytes().cmp(&a.path_bytes())); + for mut dir in directories { + dir.unpack_in(&destination) .map_err(|e| format!("failed to extract runtime pack: {e}"))?; } Ok(()) @@ -3729,6 +3832,11 @@ fn download_linux_ffmpeg(data_dir: &Path) -> Result<(), String> { .map_err(|e| format!("failed to create {}: {e}", downloads.display()))?; let archive = downloads.join("ffmpeg-linux.tar.xz"); download_file(&url, &archive, Duration::from_secs(30 * 60), "FFmpeg")?; + // Only the pinned artifact is trusted. An override points somewhere we + // cannot have a hash for, so it is the caller's business to vouch for it. + if env_path_override("STEMDECK_FFMPEG_URL").is_none() { + verify_pinned_sha256(&archive, Some(DEFAULT_LINUX_FFMPEG_SHA256), "FFmpeg")?; + } // Extract with the system tar (xz support is standard on desktop Linux). The // static build unpacks to a single ffmpeg--amd64-static/ directory. @@ -4325,34 +4433,65 @@ fn update_setup_config( /// Polls an already-spawned child until it exits or the timeout elapses. /// Mirrors command_output_with_timeout but accepts a pre-spawned Child so the /// caller can record the PID before waiting (e.g. to kill on window close). +/// Wait for `child`, draining its pipes while it runs. +/// +/// The draining is the point. Reading only after `try_wait()` reports an exit +/// deadlocks any child that outruns the OS pipe buffer: it blocks in `write()` +/// with nobody reading, so it never exits, so `try_wait()` never reports an +/// exit, and the whole thing ends at the timeout instead. `warmup_models` +/// pipes both streams and its model downloads emit tqdm progress to stderr in +/// proportion to how long they take -- so the failure lands on slow +/// connections, the users warmup exists to help (#516). +/// +/// Each stream gets its own thread because both must drain concurrently; +/// draining one and then the other reintroduces the deadlock on whichever is +/// second. fn child_output_with_timeout( mut child: Child, timeout: Duration, label: &str, ) -> Result { + let stdout_reader = child.stdout.take().map(|mut pipe| { + thread::spawn(move || { + let mut buf = Vec::new(); + let _ = pipe.read_to_end(&mut buf); + buf + }) + }); + let stderr_reader = child.stderr.take().map(|mut pipe| { + thread::spawn(move || { + let mut buf = Vec::new(); + let _ = pipe.read_to_end(&mut buf); + buf + }) + }); + + let collect = |reader: Option>>| { + reader.and_then(|h| h.join().ok()).unwrap_or_default() + }; + let deadline = Instant::now() + timeout; loop { if let Some(status) = child .try_wait() .map_err(|e| format!("failed to wait for {label}: {e}"))? { - let mut stdout = Vec::new(); - if let Some(mut pipe) = child.stdout.take() { - let _ = pipe.read_to_end(&mut stdout); - } - let mut stderr = Vec::new(); - if let Some(mut pipe) = child.stderr.take() { - let _ = pipe.read_to_end(&mut stderr); - } + // The child is gone, so both pipes are at EOF and these joins + // return promptly. return Ok(Output { status, - stdout, - stderr, + stdout: collect(stdout_reader), + stderr: collect(stderr_reader), }); } if Instant::now() >= deadline { let _ = child.kill(); let _ = child.wait(); + // Joined rather than detached: killing the child closes its ends, + // so the readers finish, and dropping the handles without joining + // would leak two threads per timeout. + let _ = collect(stdout_reader); + let _ = collect(stderr_reader); return Err(format!( "{label} timed out after {} seconds", timeout.as_secs() @@ -4367,44 +4506,12 @@ fn command_output_with_timeout( timeout: Duration, label: &str, ) -> Result { - let mut child = command + let child = command .spawn() .map_err(|e| format!("failed to start {label}: {e}"))?; - let deadline = Instant::now() + timeout; - - loop { - if let Some(status) = child - .try_wait() - .map_err(|e| format!("failed to wait for {label}: {e}"))? - { - let mut stdout = Vec::new(); - if let Some(mut pipe) = child.stdout.take() { - let _ = pipe.read_to_end(&mut stdout); - } - - let mut stderr = Vec::new(); - if let Some(mut pipe) = child.stderr.take() { - let _ = pipe.read_to_end(&mut stderr); - } - - return Ok(Output { - status, - stdout, - stderr, - }); - } - - if Instant::now() >= deadline { - let _ = child.kill(); - let _ = child.wait(); - return Err(format!( - "{label} timed out after {} seconds", - timeout.as_secs() - )); - } - - thread::sleep(Duration::from_millis(100)); - } + // Same pipe-draining requirement as child_output_with_timeout; sharing it + // keeps the two from drifting apart again (#516). + child_output_with_timeout(child, timeout, label) } #[cfg(windows)] @@ -4477,6 +4584,131 @@ mod tests { ); } + #[test] + #[cfg(unix)] + fn extract_tar_archive_survives_a_read_only_directory_member() { + // tar::Archive::unpack applies directory entries last, reverse-sorted, + // so a directory's own mode cannot stop its children being written + // (tar-rs#242). The first version of unpack_without_apple_double + // created them inline in archive order, which turns a 0o555 member into + // a hard extraction failure and a dead first-run setup (#508). + use std::os::unix::fs::PermissionsExt; + + let archive_dir = make_tmp(); + let archive = archive_dir.path().join("runtime.tar.zst"); + let encoder = zstd::Encoder::new(fs::File::create(&archive).unwrap(), 0).unwrap(); + let mut builder = tar::Builder::new(encoder); + + // Directory first, file second -- the order that broke. + let mut dir_header = tar::Header::new_gnu(); + dir_header.set_entry_type(tar::EntryType::Directory); + dir_header.set_mode(0o555); + dir_header.set_size(0); + builder + .append_data(&mut dir_header, "runtime/locked/", std::io::empty()) + .unwrap(); + + let body = b"axes.grid: True"; + let mut file_header = tar::Header::new_gnu(); + file_header.set_entry_type(tar::EntryType::Regular); + file_header.set_mode(0o644); + file_header.set_size(body.len() as u64); + builder + .append_data(&mut file_header, "runtime/locked/style.mplstyle", &body[..]) + .unwrap(); + builder.into_inner().unwrap().finish().unwrap(); + + let destination = make_tmp(); + super::extract_tar_archive(&archive, destination.path()).unwrap(); + + let written = destination.path().join("runtime/locked/style.mplstyle"); + assert!( + written.is_file(), + "a file inside a read-only directory member must still extract" + ); + let mode = fs::metadata(destination.path().join("runtime/locked")) + .unwrap() + .permissions() + .mode(); + assert_eq!(mode & 0o777, 0o555, "the directory keeps its archived mode"); + + // Leave it writable so TempDir cleanup can remove it. + fs::set_permissions( + destination.path().join("runtime/locked"), + fs::Permissions::from_mode(0o755), + ) + .unwrap(); + } + + #[test] + fn only_our_own_release_assets_are_downloadable_as_updates() { + // download_app_update takes its URL from the WebView and checks it + // against a SHA-256 from the same caller, so the checksum proves the + // bytes arrived intact, not that they came from us. apply_app_update + // then extracts the result over StemDeck's own executable (#510). + for ok in [ + "https://github.com/stemdeckapp/stemdeck/releases/download/v0.16.1/x.zip", + "https://objects.githubusercontent.com/github-production-release-asset/1/2", + ] { + assert!(super::validate_release_url(ok).is_ok(), "should allow {ok}"); + } + + for bad in [ + "https://evil.example/x.zip", + // Lookalikes: the check must be on the host, not a substring of it. + "https://github.com.evil.example/x.zip", + "https://notgithub.com/x.zip", + // Plain http would let a LAN attacker swap the bytes in flight, + // which matters because the page itself is served over http. + "http://github.com/stemdeckapp/stemdeck/releases/download/v1/x.zip", + "file:///etc/passwd", + "not a url", + ] { + assert!( + super::validate_release_url(bad).is_err(), + "should reject {bad}" + ); + } + } + + #[test] + #[cfg(unix)] + fn a_chatty_child_is_drained_rather_than_deadlocked() { + // Reading the pipes only after try_wait() reports an exit deadlocks any + // child that outruns the OS pipe buffer (64 KiB on Linux, smaller on + // macOS): it blocks in write() with nobody reading, so it never exits. + // warmup_models pipes both streams and its downloads emit tqdm progress + // to stderr in proportion to how long they take, so the old code failed + // for users on slow connections after burning the full 30-minute + // timeout (#516). + // + // 512 KiB on each stream is comfortably past any pipe buffer. A short + // timeout keeps the failure mode obvious: without concurrent draining + // this returns Err(timed out) instead of the output. + let mut command = Command::new("sh"); + command + .arg("-c") + .arg("yes stdoutstdoutstdout | head -c 524288; yes errerrerr | head -c 524288 >&2") + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + + let output = + super::command_output_with_timeout(command, Duration::from_secs(20), "chatty child") + .expect("a child that fills its pipes must still be collected"); + + assert!(output.status.success()); + assert_eq!( + output.stdout.len(), + 524_288, + "stdout must be drained in full" + ); + assert_eq!( + output.stderr.len(), + 524_288, + "stderr must be drained in full" + ); + } + #[test] fn legacy_migration_preserves_user_settings_when_data_dir_already_exists() { // setup() creates the destination before ensure_workspace() invokes @@ -5339,13 +5571,33 @@ b6052160df96b31c9b1e33854a4dcda3d4b57641b880270f31736fb9f445d384 ffmpeg-n7.1-la // A fixed port is also the honest shape of the thing under test: // reserve_port is given a configured port (8000 by default), never one // the OS just handed out. - let wanted = (21_000..21_200) - .find(|port| std::net::TcpListener::bind(("0.0.0.0", *port)).is_ok()) - .expect("no free port in 21000..21200 to test with"); - - let (got, _guard) = super::reserve_port("0.0.0.0", wanted).unwrap(); + // + // The probe binds through claim_port, not std::net::TcpListener, so + // that "free" means the same thing to the probe and to the code being + // probed. TcpListener sets SO_REUSEADDR on Unix and claim_port + // deliberately does not, so a port sitting in TIME_WAIT accepts one and + // refuses the other. That is what failed on the shared macOS runner: + // the probe picked 21000, claim_port could not take it, and the + // fallback handed back the ephemeral 53969. + // + // Even with matching options the probe has to let go before + // reserve_port can claim it, and cargo runs this binary's tests in + // parallel, so the window is narrowed rather than closed. Walking the + // range absorbs a lost race. A reserve_port that genuinely ignored a + // free port would have to lose all two hundred. + let mut attempts = 0_u32; + let granted = (21_000..21_200).find_map(|wanted| { + drop(super::claim_port("0.0.0.0", wanted).ok()?); + attempts += 1; + let (got, guard) = super::reserve_port("0.0.0.0", wanted).ok()?; + (got == wanted).then_some(guard) + }); - assert_eq!(got, wanted); + assert!(attempts > 0, "no free port in 21000..21200 to test with"); + assert!( + granted.is_some(), + "reserve_port fell back on all {attempts} ports it had just been shown were free" + ); } #[test] diff --git a/scripts/windows/make-portable.ps1 b/scripts/windows/make-portable.ps1 index 4288810c..759fd2cf 100644 --- a/scripts/windows/make-portable.ps1 +++ b/scripts/windows/make-portable.ps1 @@ -9,7 +9,19 @@ param( ) $ErrorActionPreference = "Stop" +# PowerShell 7+ only. CI invokes this script with `powershell` (Windows +# PowerShell 5.1), where this variable does nothing and $ErrorActionPreference +# does not cover native commands either -- so a failed pip install was ignored +# and the build carried on. Kept for a pwsh run; Assert-LastExitCode below is +# what actually enforces it on 5.1 (#517). $PSNativeCommandErrorActionPreference = "Stop" + +function Assert-LastExitCode { + param([Parameter(Mandatory)][string]$What) + if ($LASTEXITCODE -ne 0) { + throw "$What failed with exit code $LASTEXITCODE" + } +} Set-StrictMode -Version Latest if ($env:OS -ne "Windows_NT") { @@ -231,8 +243,10 @@ if (Get-Command "py" -ErrorAction SilentlyContinue) { } else { & python -m venv $PythonDir } +Assert-LastExitCode "creating the virtualenv" & $PythonExe -m pip install --upgrade pip +Assert-LastExitCode "pip self-upgrade" # The project version is git-derived (hatch-vcs). Pin it from $PackageVersion so # the install doesn't depend on git tags in the build checkout (#169). @@ -240,6 +254,7 @@ if ($PackageVersion) { $env:SETUPTOOLS_SCM_PRETEND_VERSION = ($PackageVersion -replace '^v', '') } & $PythonExe -m pip install "$Root" +Assert-LastExitCode "installing the StemDeck package" if ($CpuOnly) { # Force the slim CPU-only wheel. On Windows the default PyPI torch wheel is @@ -249,6 +264,10 @@ if ($CpuOnly) { & $PythonExe -m pip install torch==2.6.0+cpu torchaudio==2.6.0+cpu ` --index-url https://download.pytorch.org/whl/cpu ` --force-reinstall --no-deps + # Unchecked, a transient network failure here left whatever torch was already + # resolved in place and the zip labelled CPU shipped a non-CPU torch. The + # import checks later still pass, because torch imports fine either way. + Assert-LastExitCode "installing the CPU-only torch wheel" } # Do NOT bundle CUDA torch into the NVIDIA (non-CpuOnly) package. It ships base # torch and the desktop app installs the CUDA build on first run via diff --git a/static/css/daw.css b/static/css/daw.css index b25c9210..3ec1eef4 100644 --- a/static/css/daw.css +++ b/static/css/daw.css @@ -1726,6 +1726,14 @@ input, textarea { font-family: inherit; } border-radius: 6px; min-width: 0; } +/* The rule separates the row's label from its controls. It sits after the + label, so "All" reads as the first of four buttons rather than as part of + the words describing them. */ +.daw-panel-toggles-label { + margin-right: 2px; + padding-right: 6px; + border-right: 1px solid var(--border-strong); +} .daw-panel-toggles-label, .daw-panel-toggles-sep { font-size: 8px; @@ -2383,13 +2391,6 @@ input, textarea { font-family: inherit; } display: flex; align-items: center; flex-wrap: nowrap; gap: 8px; min-height: 34px; } -.alpha-badge { - padding: 1px 5px; border-radius: 4px; - background: rgba(74,140,255,0.16); border: 1px solid rgba(74,140,255,0.4); - color: #4a8cff; font-size: 8.5px; font-weight: 700; letter-spacing: 0.05em; - text-transform: uppercase; -} - /* Tier 3: the timeline. Full-bleed rather than an inset rounded panel -- its left edge has to land exactly on the lane waveforms' left edge, and a side border would offset the canvas by its own width. Top and bottom rules only, @@ -2513,7 +2514,18 @@ input, textarea { font-family: inherit; } /* Position group: elapsed / total time, then the loop controls that act on that position (design 1b groups them together, not with the transport). */ -.footer-group-position .footer-group-body { gap: 4px; } +/* The loop column is three rows tall while the rest of this group is one, so + centring it left the two bounds hanging below the elapsed/total readout they + belong to. Aligning to the bottom puts the fields on the readout's line and + lets the button and its hint stack up out of the way. */ +.footer-group-position .footer-group-body { gap: 4px; align-items: flex-end; } +.footer-group-position .footer-elapsed, +.footer-group-position .footer-total, +.footer-group-position .footer-time-sep { + /* Match the fields' box height so the baselines land together, rather than + the text bottom meeting the border bottom. */ + line-height: 22px; +} .footer-elapsed { font-size: 18px; font-weight: 500; letter-spacing: -0.02em; color: var(--fg); } @@ -2523,9 +2535,22 @@ input, textarea { font-family: inherit; } .footer-time-sep { font-size: 12px; color: var(--muted-2); } /* Exact loop start/end inputs (inline, right of the loop button) */ +/* The hint stacks over the two fields rather than beside them: the footer row + is already full, and a note about the fields belongs with the fields. Sized + to sit inside the width the two boxes already take, so it does not widen the + Position group. */ +.footer-loop-times-wrap { + display: flex; flex-direction: column; align-items: stretch; gap: 2px; + margin-left: 6px; +} +.footer-loop-hint { + font-size: 7.5px; font-weight: 600; letter-spacing: 0.02em; + text-transform: uppercase; color: var(--muted); + line-height: 1; white-space: nowrap; text-align: center; user-select: none; +} .footer-loop-times { - display: flex; align-items: center; gap: 5px; - margin-left: 4px; cursor: default; user-select: none; + display: flex; align-items: center; justify-content: center; gap: 5px; + cursor: default; user-select: none; } .loop-times-sep { font-size: 12px; color: var(--muted); flex-shrink: 0; } .loop-time-input { @@ -2536,7 +2561,8 @@ input, textarea { font-family: inherit; } color: var(--fg); font-family: inherit; font-size: 12px; font-weight: 600; font-variant-numeric: tabular-nums; text-align: center; } -.loop-time-input:focus { border-color: var(--accent); } +.loop-time-input { cursor: ns-resize; } +.loop-time-input:focus { cursor: text; border-color: var(--accent); } .loop-time-input:disabled { opacity: 0.45; cursor: not-allowed; } /* Transport group: stop square + a play key wide enough to be the obvious @@ -2572,7 +2598,12 @@ input, textarea { font-family: inherit; } /* Loop sits in the Position group and reads as a modifier on the readout next to it, so it is a short labelled pill rather than a full-height control. */ .footer-group-position .daw-iconbtn.btn-transport.loop { - width: auto; height: 26px; padding: 0 9px; gap: 6px; margin-left: 6px; + /* A tall narrow pill beside the fields, not a bar over them. align-self + overrides the group's flex-end so it fills the two rows the hint and the + bounds take, which is what keeps the footer one line high. */ + flex-direction: column; justify-content: center; + width: 46px; height: auto; align-self: stretch; + padding: 3px 4px; gap: 1px; margin-left: 4px; border-radius: 7px; background: var(--panel-2); border: 1px solid var(--border-strong); color: var(--fg-2); @@ -2581,7 +2612,7 @@ input, textarea { font-family: inherit; } .footer-group-position .btn-transport.loop:hover { background: var(--panel-3); color: var(--fg); } -.loop-btn-label { font-size: 11px; font-weight: 500; white-space: nowrap; } +.loop-btn-label { font-size: 9.5px; font-weight: 500; white-space: nowrap; line-height: 1; } .footer-group-position .btn-transport.loop.active, .footer-group-position .btn-transport.loop.active:hover { background: rgba(74,140,255,0.16); @@ -3373,58 +3404,94 @@ input, textarea { font-family: inherit; } } .lib-tag-count { color: var(--muted); font-size: 11px; } -/* Supporters (partner tiles, shown in the TV-icon dialog) */ -/* Slightly wider card so 3 tiles breathe; grid spans the card. */ -.friends-card { width: min(360px, calc(100vw - 32px)); } +/* Supporters ("We Recommend" dialog): category sections, each a small uppercase + accent label with a rule, over a responsive grid of partner cards. */ +/* Wide enough for two columns of three cards. The single column it replaced + was 420px and always scrolled: five sections stacked is taller than any + normal window, so half the people on the list were never seen. */ +.friends-card { width: min(780px, calc(100vw - 32px)); } .friends-card .lib-friends-grid { width: 100%; margin-top: 4px; } .lib-friends-grid { - /* Masonry columns: each column stacks independently so a tall tile does not - push others down. Tiles are round-robined into these columns in JS. */ - display: flex; - align-items: flex-start; - gap: 6px; - padding: 2px 0 0; + display: grid; + grid-template-columns: 1fr 1px 1fr; + gap: 0 18px; + align-items: start; + text-align: left; + padding: 2px 2px 0 0; + /* Kept as a floor, not as the normal case: a very short window still has to + reach the bottom of the list somehow. */ + max-height: min(74vh, 660px); + overflow-y: auto; } -.lib-friends-col { - flex: 1 1 0; - min-width: 0; +.lib-friends-col { min-width: 0; } +.lib-friends-split { align-self: stretch; background: var(--border); } +/* One column again once two would squeeze the cards below three per row. */ +@media (max-width: 860px) { + .lib-friends-grid { grid-template-columns: 1fr; } + .lib-friends-split { display: none; } +} +.lib-friends-cat { display: flex; - flex-direction: column; + align-items: center; + gap: 8px; + margin: 14px 0 7px; + font-family: var(--font-mono); + font-size: 9.5px; + font-weight: 700; + letter-spacing: 0.09em; + text-transform: uppercase; + color: var(--accent); +} +.lib-friends-col > .lib-friends-cat:first-child { margin-top: 2px; } +/* Thin rule filling the width left over by the label. */ +.lib-friends-cat::after { + content: ""; + flex: 1 1 auto; + height: 1px; + background: var(--border); +} +.lib-friends-row { + display: grid; + /* ~3 per row in the card's default width, dropping to 2 as it narrows. */ + grid-template-columns: repeat(auto-fit, minmax(108px, 1fr)); gap: 6px; } .lib-friend { display: flex; flex-direction: column; align-items: center; - gap: 6px; + gap: 5px; min-width: 0; - padding: 9px 5px 12px; + padding: 10px 6px 9px; background: var(--panel); border: 1px solid var(--border); - border-radius: 8px; + border-radius: 10px; color: var(--fg-2); text-decoration: none; text-align: center; - /* deliberately-uneven "frames on a wall" tilt; per-tile angle set in JS */ - transform: rotate(var(--tilt, 0deg)); - transition: background var(--t-fast), color var(--t-fast), - border-color var(--t-fast), transform var(--t-fast); + transition: background var(--t-fast), color var(--t-fast), border-color var(--t-fast); } .lib-friend:hover { background: var(--panel-2); color: var(--fg); border-color: rgba(255,255,255,0.15); - /* straighten the frame on hover */ - transform: rotate(0deg); } -/* Logos are transparent wordmarks; span the tile width, keep aspect, cap height. */ -.lib-friend-logo { +/* Fixed-height slot so names line up across a row whichever of the three + shapes an entry ends up with: round avatar, wordmark, or monogram. */ +.lib-friend-media { + display: flex; + align-items: center; + justify-content: center; width: 100%; - height: auto; - max-height: 30px; + height: 44px; +} +/* Wordmark logos are transparent and not square: letterbox them into the slot. */ +.lib-friend-logo { + max-width: 100%; + max-height: 40px; object-fit: contain; } -/* Instagram profile photos render as round avatars */ +/* Profile photos are square: crop to a circle. */ .lib-friend-avatar { width: 44px; height: 44px; @@ -3432,7 +3499,7 @@ input, textarea { font-family: inherit; } object-fit: cover; border: 1px solid var(--border); } -/* Monogram fallback when a tile has no image (or it fails to load): matches the +/* Monogram fallback when a card has no image (or it fails to load): matches the round avatar size, in the accent colour, so the grid stays on-brand. */ .lib-friend-monogram { width: 44px; @@ -3448,24 +3515,35 @@ input, textarea { font-family: inherit; } background: var(--panel-3); border: 1px solid var(--border); } -/* Small Instagram glyph under the text on tiles that link to Instagram */ -.lib-friend-ig { +/* Small link glyph at the foot of the card. margin-top:auto pins it to the + bottom so it lines up across cards with different role lengths. */ +.lib-friend-ig, +.lib-friend-link { /* block + no-shrink avoids the WebKit baseline clip on small inline SVGs */ display: block; flex: 0 0 auto; - width: 14px; - height: 14px; - margin-top: 2px; - fill: currentColor; - opacity: 0.65; + width: 13px; + height: 13px; + margin-top: auto; + padding-top: 3px; + opacity: 0.6; } -.lib-friend:hover .lib-friend-ig { - opacity: 1; +.lib-friend-ig { fill: currentColor; } +.lib-friend-link { + fill: none; + stroke: currentColor; + stroke-width: 2; + stroke-linecap: round; + stroke-linejoin: round; } +.lib-friend:hover .lib-friend-ig, +.lib-friend:hover .lib-friend-link { opacity: 1; } .lib-friend-name { width: 100%; - font-size: 10.5px; + font-size: 11px; + font-weight: 600; line-height: 1.2; + color: var(--fg); /* full name across up to 2 lines, then ellipsis */ display: -webkit-box; -webkit-line-clamp: 2; @@ -3474,11 +3552,10 @@ input, textarea { font-family: inherit; } } .lib-friend-role { width: 100%; - margin-top: 2px; - font-size: 9px; - line-height: 1.2; - color: var(--fg-3, var(--fg-2)); - /* show the full role; the tile grows to fit it */ + font-size: 9.5px; + line-height: 1.3; + color: var(--muted); + /* show the full role; the card grows to fit it */ } /* ── Clear bin bar (trash view only) ── */ diff --git a/static/css/waves.css b/static/css/waves.css index ebdfcfc7..6f490621 100644 --- a/static/css/waves.css +++ b/static/css/waves.css @@ -765,10 +765,52 @@ box-shadow: inset 1px 0 0 rgba(255, 255, 255, 0.18), inset -1px 0 0 rgba(255, 255, 255, 0.18); - pointer-events: none; + /* Interactive so the region can be moved and its edges adjusted (#538). + A pointerdown that does not move still falls through to a seek, so + clicking inside the selection behaves as it always did. */ + pointer-events: auto; + cursor: grab; z-index: 3; } +.loop-region.dragging { + cursor: grabbing; +} + +/* Wider than the 2px border they sit on: an edge you cannot reliably grab is + the finnicky behaviour this replaces. Extends outside the region as well as + in, so the handle is catchable from either side. */ +.loop-handle { + position: absolute; + top: 0; + bottom: 0; + width: 12px; + cursor: ew-resize; + z-index: 4; +} + +.loop-handle-start { + left: -7px; +} + +.loop-handle-end { + right: -7px; +} + +/* Only visible while the pointer is on the region, so the selection reads the + same as before at rest. */ +.loop-region:hover .loop-handle::after, +.loop-region.dragging .loop-handle::after { + content: ""; + position: absolute; + top: 0; + bottom: 0; + left: 4px; + width: 4px; + background: var(--gold); + border-radius: 2px; +} + .lane-placeholder { height: 48px; position: relative; diff --git a/static/img/friends/analog4lyfe.jpg b/static/img/friends/analog4lyfe.jpg new file mode 100644 index 00000000..05c6f2cd Binary files /dev/null and b/static/img/friends/analog4lyfe.jpg differ diff --git a/static/img/friends/beltr.jpg b/static/img/friends/beltr.jpg new file mode 100644 index 00000000..7a2d2a9e Binary files /dev/null and b/static/img/friends/beltr.jpg differ diff --git a/static/img/friends/empress-effects.png b/static/img/friends/empress-effects.png new file mode 100644 index 00000000..9cf74b44 Binary files /dev/null and b/static/img/friends/empress-effects.png differ diff --git a/static/img/friends/seratone.jpg b/static/img/friends/seratone.jpg new file mode 100644 index 00000000..9bb77f7f Binary files /dev/null and b/static/img/friends/seratone.jpg differ diff --git a/static/img/friends/thomann.jpg b/static/img/friends/thomann.jpg new file mode 100644 index 00000000..7fb1a838 Binary files /dev/null and b/static/img/friends/thomann.jpg differ diff --git a/static/index.html b/static/index.html index 7d9f58c8..f3d29ad5 100644 --- a/static/index.html +++ b/static/index.html @@ -133,7 +133,23 @@ control on each panel would need that panel to keep a stub, and the stub costs most of what collapsing the smaller ones returns. -->
- Click to collapse + + Collapse + + -
- - - +
+ + Scroll up/down to adjust +
+ + + +
@@ -718,7 +744,7 @@