From 0226907255d52d277d248ba58b7fff5b67e6428f Mon Sep 17 00:00:00 2001 From: Thales <> Date: Mon, 17 Aug 2026 17:46:29 +0100 Subject: [PATCH 1/5] Surface unavailable/broken tracks in stem collections with one-click reimport The backend now checks the stems folder on disk for every "done" job and reports "unavailable" when it's missing, replacing the old client-side heuristic that only reacted to a 404 on the single-job endpoint and missed the case where the registry entry survived but the folder did not. Desktop shows a yellow "click to reimport" warning wired to the existing importFromUrl restore path; mobile gets the same detection and one-tap reimport from scratch, since it had none before. Closes #380 --- app/api/jobs.py | 28 +++++- static/css/daw.css | 5 +- static/js/catalog.js | 82 +++++++++++++----- static/js/shared/jobs.js | 4 + static/mobile/app.js | 49 +++++++++-- static/mobile/styles.css | 4 + tests/test_jobs_api.py | 131 +++++++++++++++++++++++++++++ tests/test_registry_persistence.py | 1 + 8 files changed, 276 insertions(+), 28 deletions(-) diff --git a/app/api/jobs.py b/app/api/jobs.py index cdc9440f..e6545b84 100644 --- a/app/api/jobs.py +++ b/app/api/jobs.py @@ -108,6 +108,30 @@ def _rmtree_job(job_id: str) -> None: logger.warning("failed to remove job dir %s", job_dir, exc_info=True) +def _job_files_missing(job: Job) -> bool: + """True when a "done" job's stem files are gone from disk: the folder was + deleted or moved outside the app, not just an in-flight relocation (#354), + which is a known, temporary absence and must not flap the library.""" + if is_relocating(): + return False + stems_dir = (JOBS_DIR / job.id / "stems").resolve() + if not stems_dir.is_relative_to(JOBS_DIR.resolve()): + return True + return not stems_dir.is_dir() or not any(stems_dir.iterdir()) + + +def _job_state(job: Job) -> dict: + """job.to_state() with "done" downgraded to "unavailable" when the stem + files are missing from disk - ground truth for the client, replacing the + old approach of the frontend guessing from a 404 or a disappearance from + the job list, neither of which caught a job whose registry entry survived + but whose stems folder did not.""" + state = job.to_state() + if job.status == "done" and _job_files_missing(job): + state["status"] = "unavailable" + return state + + class JobRequest(BaseModel): url: str # Subset of stems to include in the post-processing "selected mix" @@ -259,7 +283,7 @@ async def _create_local_job(request: Request) -> dict[str, str]: def list_jobs() -> list[dict]: """List all completed jobs in the library, sorted by creation time.""" return [ - job.to_state() + _job_state(job) for job in sorted(registry_all_jobs().values(), key=lambda j: j.created_at) if job.status == "done" ] @@ -271,7 +295,7 @@ def get_job(job_id: str) -> dict: job = registry_get(job_id) if job is None: raise HTTPException(status_code=404, detail="job not found") - return job.to_state() + return _job_state(job) @router.post("/{job_id}/cancel") diff --git a/static/css/daw.css b/static/css/daw.css index ceddb875..ba41f072 100644 --- a/static/css/daw.css +++ b/static/css/daw.css @@ -670,8 +670,9 @@ input, textarea { font-family: inherit; } .cat-status { width: 6px; height: 6px; border-radius: 50%; background: #5fbc56; flex-shrink: 0; } .cat-status.processing { background: var(--accent); animation: cat-pulse 1.4s infinite; } .cat-status.unavailable { background: #666; } -.cat-item.unavailable { cursor: not-allowed; } -.cat-item.unavailable .cat-meta { opacity: 0.45; } +.cat-item.unavailable { cursor: pointer; } +.cat-item.unavailable .cat-title { opacity: 0.55; } +.cat-unavailable-warning { color: var(--accent); font-weight: 600; opacity: 1; } @keyframes cat-pulse { 0%,100%{opacity:1;} 50%{opacity:0.3;} } /* Import queue: a waiting row and a running row must be told apart at a diff --git a/static/js/catalog.js b/static/js/catalog.js index 26f64ba5..eb84d352 100644 --- a/static/js/catalog.js +++ b/static/js/catalog.js @@ -529,11 +529,26 @@ function applyStoredStemSelection(track) { } } +// A track's files went missing (folder deleted or moved outside the app). +// URL-sourced tracks can be rebuilt from the source, so trigger that +// directly rather than making the user hunt for a "resync" button in +// Settings. Local uploads have no source bytes left to rebuild from (#354's +// cleanup deletes the upload once the pipeline is done with it), so the +// only path back is a fresh re-upload. +function reimportUnavailableTrack(trackId, track) { + updateTrackStatus(trackId, "unavailable"); + if (track.sourceUrl && !track.sourceUrl.startsWith("local:")) { + importFromUrl(track.sourceUrl, { title: track.title, stems: track.selectedStems }); + return; + } + showError("This track's audio is no longer available. Re-upload to restore it."); +} + async function loadTrackIntoStudio(trackId) { let track = tracks[trackId]; if (!track) return; if (track.status === "unavailable") { - showError("This track's audio is no longer available. Re-upload to restore it."); + reimportUnavailableTrack(trackId, track); return; } // The user has chosen to look at something else, so the running import gives @@ -561,6 +576,10 @@ async function loadTrackIntoStudio(trackId) { track = stateMetadataToTrack(state, track); tracks[trackId] = track; saveState(); + if (state.status === "unavailable") { + reimportUnavailableTrack(trackId, track); + return; + } } else if (res.status === 404) { track = { ...track, status: "unavailable" }; tracks[trackId] = track; @@ -1016,6 +1035,20 @@ function makeSectionEl(labelText) { return section; } +// A URL-sourced track can be rebuilt by re-running the import; a local +// upload has no source bytes left to rebuild from, only a re-upload gets it +// back (see reimportUnavailableTrack). +function canReimportTrack(track) { + return Boolean(track.sourceUrl) && !track.sourceUrl.startsWith("local:"); +} + +function unavailableWarningHtml(track) { + const label = canReimportTrack(track) + ? "Track unavailable - click to reimport" + : "Track unavailable - re-upload to restore"; + return `${esc(label)}`; +} + function renderRecentItem(trackId) { const track = tracks[trackId]; if (!track) return null; @@ -1030,7 +1063,7 @@ function renderRecentItem(trackId) {
${thumbHtml(track)}
${esc(displayTitle(track.title))}
-
${esc(sub)}
+
${isUnavailable ? unavailableWarningHtml(track) : `${esc(sub)}`}
`; @@ -1093,15 +1126,16 @@ function renderTrackItem(trackId, { inTrash = false } = {}) { el.dataset.id = trackId; const stemCount = track.stems?.length ?? 0; + const subHtml = isUnavailable + ? unavailableWarningHtml(track) + : `${esc(track.channel ?? "")} + · + ${inTrash ? "Removed" : `${stemCount} stem${stemCount !== 1 ? "s" : ""}`}`; el.innerHTML = `
${thumbHtml(track)}
${esc(displayTitle(track.title))}
-
- ${esc(track.channel ?? "")} - · - ${inTrash ? "Removed" : `${stemCount} stem${stemCount !== 1 ? "s" : ""}`} -
+
${subHtml}
${inTrash ? "" : ` -
+
${artLabel(t)}
-
${esc(t.title)}
${esc(t.sub)}
${esc(t.meta)}
+
${infoHtml}
- +
-
`).join("")}`; + `; + }).join("")}`; } function libraryScreen() { @@ -898,6 +907,11 @@ app.addEventListener("click", (e) => { if (track) openTrack(track); return; } + case "reimport": { + const track = state.tracks.find((x) => x.id === t.dataset.id); + if (track) reimportTrack(track); + return; + } case "reload": loadLibrary(); return; @@ -919,6 +933,31 @@ app.addEventListener("change", (e) => { } }); +// A track's files are gone (folder deleted or moved outside the app). +// URL-sourced tracks can be rebuilt by re-running the import; a local upload +// has no source bytes left (the pipeline deletes the upload once it's done +// with it), so that just gets a toast pointing at re-upload instead. +async function reimportTrack(card) { + if (!card.sourceUrl || card.sourceUrl.startsWith("local:")) { + toast("This track's audio is gone. Re-upload it to restore it."); + return; + } + toast("Reimporting…"); + try { + const res = await fetch("/api/jobs", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ url: card.sourceUrl, stems: card.selectedStems || [] }), + }); + const data = await res.json().catch(() => ({})); + if (!res.ok) throw new Error(data.detail || res.statusText); + await loadLibrary(); + } catch (e) { + console.warn("[mobile] reimport failed:", e); + toast(`Couldn't reimport: ${e.message}`); + } +} + // Load the real library from /api/jobs (newest first). On success, seed the // Mixer with the most recent track if nothing is selected yet. async function loadLibrary() { diff --git a/static/mobile/styles.css b/static/mobile/styles.css index 81d9ef9b..fd79276e 100644 --- a/static/mobile/styles.css +++ b/static/mobile/styles.css @@ -655,6 +655,10 @@ button { color: var(--muted-3); margin-top: 1px; } +.track-info .s.track-warn { + color: var(--accent); + font-weight: 600; +} .track-info .m { font-family: var(--mono); font-size: 10.5px; diff --git a/tests/test_jobs_api.py b/tests/test_jobs_api.py index 1b126b3b..10e5e338 100644 --- a/tests/test_jobs_api.py +++ b/tests/test_jobs_api.py @@ -361,3 +361,134 @@ def test_sse_503_when_connection_cap_reached(client): assert r.status_code == 503 finally: events_mod._sse_active = original + + +# ─── Track unavailable when stem files are missing from disk ───────────────── +# +# A "done" job's status field in the registry stays "done" even after its +# stems folder is deleted or moved outside the app (the registry has no way to +# know) - list_jobs/get_job check disk on every request and report +# "unavailable" instead, so the client never has to guess from a 404 or from +# the job disappearing off the list, neither of which catches this case. + + +def test_list_jobs_flags_a_done_job_with_no_stems_dir_as_unavailable(client, tmp_path, monkeypatch): + import app.api.jobs as jobs_mod + + monkeypatch.setattr(jobs_mod, "JOBS_DIR", tmp_path) + job = Job(id="abcdefabcdef", status="done", title="Gone") + _jobs[job.id] = job + + r = client.get("/api/jobs") + assert r.status_code == 200 + [state] = r.json() + assert state["status"] == "unavailable" + + +def test_get_job_flags_a_done_job_with_an_empty_stems_dir_as_unavailable( + client, tmp_path, monkeypatch +): + import app.api.jobs as jobs_mod + + monkeypatch.setattr(jobs_mod, "JOBS_DIR", tmp_path) + job = Job(id="abcdefabcdef", status="done", title="Emptied") + _jobs[job.id] = job + (tmp_path / job.id / "stems").mkdir(parents=True) + + r = client.get(f"/api/jobs/{job.id}") + assert r.status_code == 200 + assert r.json()["status"] == "unavailable" + + +def test_list_jobs_reports_done_when_stems_are_present(client, tmp_path, monkeypatch): + import app.api.jobs as jobs_mod + + monkeypatch.setattr(jobs_mod, "JOBS_DIR", tmp_path) + job = Job(id="abcdefabcdef", status="done", title="Still here") + _jobs[job.id] = job + stems_dir = tmp_path / job.id / "stems" + stems_dir.mkdir(parents=True) + (stems_dir / "vocals.wav").write_bytes(b"RIFF1234") + + r = client.get("/api/jobs") + [state] = r.json() + assert state["status"] == "done" + + +def test_missing_stems_are_not_flagged_while_relocating(client, tmp_path, monkeypatch): + """A library move (#354) makes the stems folder briefly absent on purpose - + that is not the same failure as a user deleting it, and must not flash the + unavailable badge mid-move.""" + import app.api.jobs as jobs_mod + import app.core.stems_location as stems_location + + monkeypatch.setattr(jobs_mod, "JOBS_DIR", tmp_path) + monkeypatch.setattr(stems_location, "_relocating", True) + job = Job(id="abcdefabcdef", status="done", title="Mid-move") + _jobs[job.id] = job + + r = client.get("/api/jobs") + [state] = r.json() + assert state["status"] == "done" + + +# ─── GET /jobs/{id}/failure: stderr tail and traceback sections ────────────── + + +def test_get_failure_separates_tail_from_traceback(client, tmp_path, monkeypatch): + """error.txt can carry both a `--- stderr tail ---` and a + `--- traceback ---` section; the parser must not lump the second into the + first just because it also comes after a "---" marker.""" + import app.api.jobs as jobs_mod + + monkeypatch.setattr(jobs_mod, "JOBS_DIR", tmp_path) + failed_dir = tmp_path / "failed" / "abcdefabcdef" + failed_dir.mkdir(parents=True) + (failed_dir / "error.txt").write_text( + "\n".join( + [ + "time: 2026-08-17T16:50:02+00:00", + "stage: Error: Processing failed", + "device: cpu", + "cause: unknown", + "exception: RuntimeError('boom')", + "", + "--- stderr tail ---", + "line one of stderr", + "line two of stderr", + "", + "--- traceback ---", + "Traceback (most recent call last):", + ' File "/app/pipeline/runner.py", line 320, in _run_async', + "RuntimeError: boom", + ] + ) + + "\n", + encoding="utf-8", + ) + + r = client.get("/api/jobs/abcdefabcdef/failure") + assert r.status_code == 200 + body = r.json() + assert body["tail"] == ["line one of stderr", "line two of stderr"] + assert body["traceback"] == [ + "Traceback (most recent call last):", + ' File "/app/pipeline/runner.py", line 320, in _run_async', + "RuntimeError: boom", + ] + + +def test_get_failure_traceback_is_empty_list_when_absent(client, tmp_path, monkeypatch): + """Older quarantined jobs (or ones that never reached the pipeline) may + have no traceback section -- the field must still be present as [], not + missing, so the client doesn't have to special-case it.""" + import app.api.jobs as jobs_mod + + monkeypatch.setattr(jobs_mod, "JOBS_DIR", tmp_path) + failed_dir = tmp_path / "failed" / "abcdefabcdef" + failed_dir.mkdir(parents=True) + (failed_dir / "error.txt").write_text("stage: Error\ncause: unknown\n", encoding="utf-8") + + r = client.get("/api/jobs/abcdefabcdef/failure") + assert r.status_code == 200 + assert r.json()["traceback"] == [] diff --git a/tests/test_registry_persistence.py b/tests/test_registry_persistence.py index 82aa9e40..10f4e9b7 100644 --- a/tests/test_registry_persistence.py +++ b/tests/test_registry_persistence.py @@ -289,6 +289,7 @@ def test_restored_job_serves_stems(tmp_path: Path, monkeypatch): (tmp_path / "registry.json").write_text(json.dumps(data), encoding="utf-8") monkeypatch.setattr("app.api.stems.JOBS_DIR", tmp_path) + monkeypatch.setattr("app.api.jobs.JOBS_DIR", tmp_path) restore_registry(tmp_path) from app.main import app From ab3832bfa34b7b31ee30f3f2d1bd9c116b1ef236 Mon Sep 17 00:00:00 2001 From: Thales <> Date: Mon, 17 Aug 2026 17:46:41 +0100 Subject: [PATCH 2/5] Accept /live/, /embed/, and youtube-nocookie.com links for YouTube import normalize_youtube_url() rejected these outright with "could not extract a video ID from URL" or "unsupported host". /live/ is what premieres and creator livestreams keep once they end and become a normal VOD - common for concert/DJ-set recordings. youtube-nocookie.com (the privacy-embed domain) wasn't recognized as a YouTube host at all; added alongside /embed/ support on the regular domain too. Closes #382 --- app/pipeline/download.py | 25 ++++++++++++++----- tests/test_url_validation.py | 48 ++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/app/pipeline/download.py b/app/pipeline/download.py index c482afea..30e3d62b 100644 --- a/app/pipeline/download.py +++ b/app/pipeline/download.py @@ -94,6 +94,10 @@ def _with_retries(job: Job, fn, *, what: str): "m.youtube.com", "music.youtube.com", "youtu.be", + # The "privacy-enhanced mode" embed domain -- same site, same extractor, + # shows up in copy-pasted embed/share code rather than the address bar. + "youtube-nocookie.com", + "www.youtube-nocookie.com", ) ) # Note: on.soundcloud.com (the share shortener) is intentionally excluded — it @@ -169,12 +173,18 @@ def normalize_youtube_url(url: str) -> str: the playlist extractor. Pass non-YouTube URLs through unchanged. Cases handled: - * `watch?v=X&list=...` -> `watch?v=X` (drop the playlist context) + * `watch?v=X&list=...` -> `watch?v=X` (drop the playlist context, + regardless of what other tracking/context params ride along -- + `si=`, `t=`, `app=desktop`, etc.) * `?list=RD&start_radio=1` -> `watch?v=` (Radio playlists embed the seed in the list ID; YouTube refuses to view the playlist directly with "This playlist type is unviewable.") * `youtu.be/` -> `watch?v=` * `youtube.com/shorts/` -> `watch?v=` + * `youtube.com/live/` -> `watch?v=` (premieres and + creator livestreams keep this URL once they end and become a normal + VOD -- common for concert/DJ-set recordings) + * `youtube-nocookie.com/...` -> the same forms on `youtube.com` Everything else (PL/OL/algorithmic playlists with no derivable seed) is left alone -- yt-dlp will surface its own error. """ @@ -187,7 +197,7 @@ def normalize_youtube_url(url: str) -> str: if host.startswith(prefix): host = host[len(prefix) :] break - if host not in ("youtube.com", "youtu.be"): + if host not in ("youtube.com", "youtu.be", "youtube-nocookie.com"): return url qs = urllib.parse.parse_qs(parsed.query) @@ -206,10 +216,13 @@ def normalize_youtube_url(url: str) -> str: if _VIDEO_ID_RE.match(vid): return f"https://www.youtube.com/watch?v={vid}" - if host == "youtube.com" and parsed.path.startswith("/shorts/"): - vid = parsed.path[len("/shorts/") :].lstrip("/").split("/")[0] - if _VIDEO_ID_RE.match(vid): - return f"https://www.youtube.com/watch?v={vid}" + if host in ("youtube.com", "youtube-nocookie.com"): + for path_prefix in ("/shorts/", "/live/", "/embed/"): + if parsed.path.startswith(path_prefix): + vid = parsed.path[len(path_prefix) :].lstrip("/").split("/")[0] + if _VIDEO_ID_RE.match(vid): + return f"https://www.youtube.com/watch?v={vid}" + break return url diff --git a/tests/test_url_validation.py b/tests/test_url_validation.py index b1ee9c39..8f4a4c61 100644 --- a/tests/test_url_validation.py +++ b/tests/test_url_validation.py @@ -36,6 +36,54 @@ "https://m.youtube.com/shorts/dQw4w9WgXcQ", "https://www.youtube.com/watch?v=dQw4w9WgXcQ", ), + # Radio/"Mix" context: RD embeds the video id, and YouTube + # refuses to view the playlist directly ("unviewable"), so this must + # fall through to the single video rather than 422 or reach yt-dlp's + # playlist extractor. + ( + "https://www.youtube.com/watch?v=dQw4w9WgXcQ&list=RDdQw4w9WgXcQ&start_radio=1", + "https://www.youtube.com/watch?v=dQw4w9WgXcQ", + ), + # A real (non-radio) playlist attached to a specific video: the video + # still wins, since `v=` is matched before `list=` is even inspected. + ( + "https://www.youtube.com/watch?v=dQw4w9WgXcQ&list=PLfoo&index=3", + "https://www.youtube.com/watch?v=dQw4w9WgXcQ", + ), + # Mobile share links carry a `si=` tracking token; timestamp links + # carry `t=`. Neither should affect extraction. + ( + "https://www.youtube.com/watch?v=dQw4w9WgXcQ&si=aBcDeFgHiJkLmNoP", + "https://www.youtube.com/watch?v=dQw4w9WgXcQ", + ), + ( + "https://youtu.be/dQw4w9WgXcQ?si=aBcDeFgHiJkLmNoP&t=42", + "https://www.youtube.com/watch?v=dQw4w9WgXcQ", + ), + # Param order is not guaranteed by every client that builds these URLs. + ( + "https://www.youtube.com/watch?app=desktop&v=dQw4w9WgXcQ", + "https://www.youtube.com/watch?v=dQw4w9WgXcQ", + ), + # Premieres/livestreams keep this URL once they end and become a + # normal VOD -- common for concert/DJ-set recordings. + ( + "https://www.youtube.com/live/dQw4w9WgXcQ?feature=share", + "https://www.youtube.com/watch?v=dQw4w9WgXcQ", + ), + # Embed src URLs, on both the regular and "privacy-enhanced" domain. + ( + "https://www.youtube.com/embed/dQw4w9WgXcQ", + "https://www.youtube.com/watch?v=dQw4w9WgXcQ", + ), + ( + "https://www.youtube-nocookie.com/embed/dQw4w9WgXcQ", + "https://www.youtube.com/watch?v=dQw4w9WgXcQ", + ), + ( + "https://www.youtube-nocookie.com/watch?v=dQw4w9WgXcQ", + "https://www.youtube.com/watch?v=dQw4w9WgXcQ", + ), ], ) def test_accepts_youtube_urls(url: str, expected: str) -> None: From d0f51de6c20ed15b6ec3a18284e1c3327fd6dca3 Mon Sep 17 00:00:00 2001 From: Thales <> Date: Mon, 17 Aug 2026 17:46:47 +0100 Subject: [PATCH 3/5] Raise max track duration ceiling from 20 to 60 minutes The Settings API silently clamped any requested max_duration_sec back down to 1200 seconds regardless of what was sent - _DURATION_MAX was a hardcoded product ceiling, not just a default. Full albums, DJ sets, and concert recordings routinely exceed 20 minutes. Closes #383 --- app/core/settings.py | 4 ++-- tests/test_network_gate.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/core/settings.py b/app/core/settings.py index 21ae06ef..20efc2e8 100644 --- a/app/core/settings.py +++ b/app/core/settings.py @@ -39,8 +39,8 @@ _LOCK = threading.RLock() _state: dict | None = None # whole settings dict, loaded lazily -# Clamp bounds. Max track length is capped at 20 min (the product ceiling). -_DURATION_MIN, _DURATION_MAX = 60, 1200 # 1 min .. 20 min +# Clamp bounds. Max track length is capped at 60 min (the product ceiling). +_DURATION_MIN, _DURATION_MAX = 60, 3600 # 1 min .. 60 min _HEIGHT_MIN, _HEIGHT_MAX = 144, 2160 _PLAYLIST_MIN, _PLAYLIST_MAX = 1, 200 _PORT_MIN, _PORT_MAX = 1024, 65535 diff --git a/tests/test_network_gate.py b/tests/test_network_gate.py index e0e02ca7..7984d42a 100644 --- a/tests/test_network_gate.py +++ b/tests/test_network_gate.py @@ -67,7 +67,7 @@ def test_runtime_settings_round_trip_and_clamp(): # Out-of-range values are clamped, not rejected. assert settings_mod.set_max_duration_sec(5) == 60 # floor - assert settings_mod.set_max_duration_sec(99999) == 1200 # ceiling = 20 min + assert settings_mod.set_max_duration_sec(99999) == 3600 # ceiling = 60 min assert settings_mod.set_video_max_height(99999) == 2160 # ceil From 7d0ef1230253137b2ae5820eba2745b8025d4111 Mon Sep 17 00:00:00 2001 From: Thales <> Date: Mon, 17 Aug 2026 17:47:07 +0100 Subject: [PATCH 4/5] Rework the notification centre's failure report: fix the Windows Explorer bug, add Discord, full traceback, opt-in logs, and anonymization Root cause of the Explorer bug: the pre-filled GitHub URL carried the full diagnostic dump (up to 6000 chars) as a query param, and Windows opens it via explorer.exe, which silently falls back to a plain File Explorer window past roughly 2000 characters instead of erroring. buildReportUrl() now fills the "Logs / screenshots" field directly with as much of the traceback/stderr tail as fits (keeping the end, where the actual error is - no paste needed for the common case), and only points at the clipboard for what doesn't fit. buildReportText() always has the complete, untruncated version. Also added: - A second "Report on Discord" button next to "Report on GitHub". - Full backend traceback capture (_quarantine_failed_job), not just a one-line exception repr - fixed a latent bug in the same change where the tail parser would have silently swallowed a second section into the first. - An opt-in "Include recent logs" button pulling from the backend/ application/setup log views already exposed by Settings -> Logs, scoped to a window around the failure's own timestamp. - Anonymization (app/core/redact.py): strips the reporter's home directory, any YouTube/SoundCloud source URL (download.py logs every job's URL, not just the failing one - a raw log tail would otherwise leak everything imported in the fetched window), and any IPv4 address (the mobile UI talks to this backend over the LAN). Applied unconditionally in GET /api/logs/{view}, not just for the report flow, and to the per-job traceback/tail/exception before error.txt is ever written. title:/source: stay unredacted in that file on purpose - they're already excluded from the public API response, so redacting them there loses local diagnostic value for no privacy gain. Closes #381, #384 --- app/api/jobs.py | 30 +++-- app/core/redact.py | 58 ++++++++++ app/main.py | 14 ++- app/pipeline/runner.py | 29 ++++- static/css/daw.css | 12 ++ static/index.html | 4 +- static/js/notifications.js | 203 +++++++++++++++++++++++++++------- tests/js/report-url.test.mjs | 120 ++++++++++++++++++-- tests/test_logs_api.py | 47 ++++++++ tests/test_pipeline_runner.py | 51 +++++++++ tests/test_redact.py | 75 +++++++++++++ 11 files changed, 573 insertions(+), 70 deletions(-) create mode 100644 app/core/redact.py create mode 100644 tests/test_redact.py diff --git a/app/api/jobs.py b/app/api/jobs.py index e6545b84..9002005d 100644 --- a/app/api/jobs.py +++ b/app/api/jobs.py @@ -464,9 +464,10 @@ def get_failure(job_id: str) -> dict: _quarantine_failed_job writes jobs/failed//error.txt on every pipeline failure (#277) and until now nothing ever read it back: the UI had only the - one-line `error_detail`, so a bug report could not carry the stderr tail - that says *why* demucs died. Read-only, and never serves the whole file -- - only the technical keys above, plus the tail. + one-line `error_detail`, so a bug report could not carry the stderr tail or + the full traceback that say *why* demucs died. Read-only, and never serves + the whole file -- only the technical keys above, plus the tail and + traceback (both already home-directory-redacted by the writer). """ if not JOB_ID_RE.match(job_id): raise HTTPException(status_code=404, detail="job not found") @@ -488,19 +489,32 @@ def get_failure(job_id: str) -> dict: fields: dict[str, str] = {} tail: list[str] = [] - in_tail = False + tb: list[str] = [] + section = "fields" for line in text.splitlines(): - if line.strip() == "--- stderr tail ---": - in_tail = True + stripped = line.strip() + if stripped == "--- stderr tail ---": + section = "tail" continue - if in_tail: + if stripped == "--- traceback ---": + # The writer separates sections with a blank line for readability + # in the raw file; drop it here rather than let it show up as a + # trailing empty entry in `tail`. + if tail and tail[-1] == "": + tail.pop() + section = "traceback" + continue + if section == "tail": tail.append(line) continue + if section == "traceback": + tb.append(line) + continue key, sep, value = line.partition(":") if sep and key in _FAILURE_PUBLIC_KEYS: fields[key] = value.strip() - return {"job_id": job_id, **fields, "tail": tail} + return {"job_id": job_id, **fields, "tail": tail, "traceback": tb} @router.get("/{job_id}/beats") diff --git a/app/core/redact.py b/app/core/redact.py new file mode 100644 index 00000000..514baa5f --- /dev/null +++ b/app/core/redact.py @@ -0,0 +1,58 @@ +"""Stripping locally- or personally-identifying text before it can reach a +public report. + +The notification centre's "Report on GitHub"/"Report on Discord" flow and its +opt-in "include recent logs" button both end with this text pasted somewhere +public. Three things in particular have no business being there: + + - The reporter's home directory. On Windows the Python install path alone + (`C:\\Users\\\\...`) carries the OS username. + - A track's source URL. `download.py` logs it for every job + ("download starting: "), so a raw log tail carries the YouTube/ + SoundCloud link for everything the reporter has ever imported in the + fetched window -- not just the one that failed. The per-job failure + report has always kept this out deliberately (see + app/api/jobs.py's _FAILURE_PUBLIC_KEYS); a raw log has no such allowlist + of its own, so it is enforced here instead. + - An IP address. The mobile UI talks to this backend over the LAN when + network access is on, so uvicorn's access log (captured into + backend.log on desktop) can carry another device's address on the + reporter's home network, not just their own. + +Every place that can hand text to that report flow -- the per-job failure +evidence (runner.py) and the Settings -> Logs viewer (main.py) -- redacts +through this one function rather than each carrying its own copy. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +# Mirrors app/pipeline/download.py's _YOUTUBE_HOSTS | _SOUNDCLOUD_HOSTS. Kept +# as a literal copy rather than importing it: this module is used from +# main.py's request path, and pipeline.download pulls in yt_dlp, which is not +# something a log-tail redaction pass should need to import. +_SOURCE_URL_RE = re.compile( + r"https?://(?:[\w-]+\.)*" + r"(?:youtube\.com|youtu\.be|youtube-nocookie\.com|soundcloud\.com)\S*", + re.IGNORECASE, +) + +# IPv4 only. The app's own LAN detection (app/main.py's _is_lan_ipv4) is +# IPv4-only too, and a loose IPv6 pattern is a real hazard here: something +# like "(?:[0-9a-f]{1,4}:){2,7}[0-9a-f]{1,4}" also matches an HH:MM:SS +# timestamp (every log line has one), which would mangle every line's time +# instead of catching the rare home IPv6 address. +_IPV4_RE = re.compile(r"\b(?:\d{1,3}\.){3}\d{1,3}\b") + + +def redact(text: str) -> str: + """Strip the reporter's home directory, any YouTube/SoundCloud source + URL, and any IPv4 address from `text`.""" + home = str(Path.home()) + if home and home not in (".", "/"): + text = text.replace(home, "") + text = _SOURCE_URL_RE.sub("", text) + text = _IPV4_RE.sub("", text) + return text diff --git a/app/main.py b/app/main.py index 7e35b494..6cf27001 100644 --- a/app/main.py +++ b/app/main.py @@ -33,6 +33,7 @@ ) from app.core.logging_setup import configure_logging from app.core.process import process_exists as _process_exists +from app.core.redact import redact from app.core.registry import all_jobs as registry_all_jobs from app.core.registry import registry_path, take_pending_resume from app.core.registry import reset_all as reset_registry @@ -666,9 +667,16 @@ def get_log_tail(view: str, minutes: int = 60) -> PlainTextResponse: if len(kept) > _LOG_TAIL_LINES: truncated = f"[... {len(kept) - _LOG_TAIL_LINES} earlier lines not shown ...]\n" kept = kept[-_LOG_TAIL_LINES:] - return PlainTextResponse( - truncated + "\n".join(kept) + "\n", media_type="text/plain; charset=utf-8" - ) + # Redacted unconditionally, not just for the report flow's callers: a log + # line is a log line regardless of who asks for it, and the notification + # centre's "include recent logs" button hands this straight to a public + # GitHub issue or Discord message without a second filtering step. Strips + # the reporter's home directory, any YouTube/SoundCloud source URL (every + # job's download start is logged at info level -- not just the failing + # one), and any IPv4 address (the mobile UI talks to this backend over + # the LAN, so uvicorn's access log can carry another device's address). + body = redact(truncated + "\n".join(kept) + "\n") + return PlainTextResponse(body, media_type="text/plain; charset=utf-8") @app.get("/api/logs.zip", tags=["settings"]) diff --git a/app/pipeline/runner.py b/app/pipeline/runner.py index 75be08c3..577a6337 100644 --- a/app/pipeline/runner.py +++ b/app/pipeline/runner.py @@ -6,11 +6,13 @@ import shutil import subprocess import time +import traceback from datetime import datetime, timezone from pathlib import Path from app.core.config import DEMUCS_MODEL, TIMEOUT_FFMPEG 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.pipeline.analyze import analyze from app.pipeline.beatgrid import compute_beat_grid @@ -256,18 +258,30 @@ def _quarantine_failed_job(job: Job, job_dir: Path, jobs_dir: Path, exc: Excepti """Preserve failure evidence instead of destroying it (#277). Writes error.txt (stage, device, model, timings, classified cause, stderr - tail), strips the heavy audio payloads, and moves the dir to - jobs/failed/ where sweep_failed_jobs expires it after FAILED_TTL. + tail, full traceback), strips the heavy audio payloads, and moves the dir + to jobs/failed/ where sweep_failed_jobs expires it after FAILED_TTL. Best-effort throughout: any step failing falls back to plain removal so a pathological error can never leak disk.""" tail: list[str] = getattr(exc, "tail", None) or [] cause = classify_failure("\n".join([*tail, repr(exc)])) detail = cause if tail: - detail += f" — {tail[-1][:200]}" + # error_detail reaches the client directly (job state, notification + # card, and the report URL's "what" field) -- redact before the [:200] + # truncation, not after, so a redaction placeholder never gets cut in + # half. + detail += f" — {redact(tail[-1])[:200]}" job.error_detail = detail try: + # title/source stay unredacted: they never leave this file (the + # /failure API's allowlist excludes both, see app/api/jobs.py), so + # this is purely local diagnostic value for the person looking at + # their own disk. Everything below IS served to the client and is + # redacted accordingly -- exc!r can embed a source URL (yt-dlp errors + # often do), and the stderr tail/traceback can carry either a source + # URL or the reporter's home directory. + redacted_tail = [redact(line) for line in tail] lines = [ f"time: {datetime.now(timezone.utc).isoformat(timespec='seconds')}", f"job: {job.id}", @@ -278,10 +292,13 @@ def _quarantine_failed_job(job: Job, job_dir: Path, jobs_dir: Path, exc: Excepti f"model: {DEMUCS_MODEL}", f"cause: {cause}", f"timings: {json.dumps(job.stage_timings) if job.stage_timings else '(none)'}", - f"exception: {exc!r}", + f"exception: {redact(repr(exc))}", ] - if tail: - lines += ["", "--- stderr tail ---", *tail] + if redacted_tail: + lines += ["", "--- stderr tail ---", *redacted_tail] + tb = redact("".join(traceback.format_exception(type(exc), exc, exc.__traceback__))).rstrip() + if tb: + lines += ["", "--- traceback ---", tb] (job_dir / "error.txt").write_text("\n".join(lines) + "\n", encoding="utf-8") # Strip heavy payloads: the quarantine keeps diagnostics, not audio. diff --git a/static/css/daw.css b/static/css/daw.css index ba41f072..5d7111e7 100644 --- a/static/css/daw.css +++ b/static/css/daw.css @@ -2792,6 +2792,18 @@ input, textarea { font-family: inherit; } font-family: var(--font-mono); font-size: 11px; line-height: 1.55; color: var(--fg-2); white-space: pre-wrap; overflow-wrap: anywhere; } +/* Opt-in, not automatic: this pulls extra log content into what's about to be + copied into a public report, so it gets its own deliberate click rather + than firing every time the dialog opens (matches .library-editor-sync's + amber "extra action" treatment). */ +.failure-logs-btn { + align-self: flex-start; + min-height: 28px; margin-top: 8px; padding: 0 12px; + border-radius: 7px; border: 1px solid rgba(244, 183, 64, 0.3); + background: rgba(244, 183, 64, 0.16); color: var(--accent); + font-family: var(--font-mono); font-size: 11px; cursor: pointer; +} +.failure-logs-btn:disabled { opacity: 0.55; cursor: default; } /* Reporting is the reason this dialog exists, so it gets the app's primary button treatment (as Export Mix does) rather than the muted link style the About/release dialogs use for their several equal-weight links. */ diff --git a/static/index.html b/static/index.html index bf64997b..895ed1c3 100644 --- a/static/index.html +++ b/static/index.html @@ -886,10 +886,12 @@

New release available

Something failed

-

These details go in the report. Your track title and source link are not included — add them yourself if they help.

+

These details go in the report. Your track title and source link are not included - add them yourself if they help. Clicking a button below copies this to your clipboard.

+
Report on GitHub + Report on Discord
diff --git a/static/js/notifications.js b/static/js/notifications.js index 0e8eb0a0..830c5a4a 100644 --- a/static/js/notifications.js +++ b/static/js/notifications.js @@ -17,10 +17,14 @@ const FAILURES_KEY = "stemdeck:failures"; // Enough to cover a bad session without letting a crash loop fill the store. const MAX_FAILURES = 20; const NEW_ISSUE_URL = "https://github.com/stemdeckapp/stemdeck/issues/new"; -// Practical ceiling for a URL handed to a browser or, on Windows, to -// explorer.exe. GitHub itself tolerates more, but nothing here is worth -// risking a silently truncated link for -- the tail is trimmed to fit. -const MAX_URL_LENGTH = 6000; +// Support channel for a quicker back-and-forth than a public issue -- same +// invite as the About dialog's Discord icon (index.html). +const DISCORD_URL = "https://discord.gg/JGk7FdZb9N"; +// Named views the Settings -> Logs viewer offers (app/main.py _LOG_VIEWS). +// Opt-in only (fetchRecentLogs): unlike the per-job traceback, these cover +// more than one failure and are worth a deliberate click, not an automatic +// fetch every time the dialog opens. +const LOG_VIEWS = ["backend", "application", "setup"]; // What each failure class is called in the report and on the card. const KIND_LABELS = { @@ -82,7 +86,45 @@ function technicalBlock(record, diag) { } /** - * Build the pre-filled bug-report URL. + * The technical block plus the full (untruncated) stderr tail and backend + * traceback, for copying to the clipboard rather than the URL -- and for the + * dialog's own on-screen preview, so what's shown matches what gets pasted. + * Pure and exported for the same reason as buildReportUrl. + */ +export function buildReportText(record, diag = {}) { + const tail = Array.isArray(record.tail) ? record.tail : []; + const tb = Array.isArray(record.traceback) ? record.traceback : []; + const parts = [technicalBlock(record, diag)]; + if (tail.length) parts.push("", "Stderr tail:", "```", ...tail, "```"); + if (tb.length) parts.push("", "Traceback:", "```", ...tb, "```"); + if (record.logs) { + for (const view of LOG_VIEWS) { + const text = record.logs[view]; + if (text) parts.push("", `Recent ${view} log:`, "```", text, "```"); + } + } + return parts.join("\n"); +} + +// Windows opens this URL via explorer.exe, which silently falls back to a +// plain File Explorer window past roughly 2000 characters rather than +// erroring, instead of opening GitHub (the bug this whole scheme guards +// against). Kept a healthy margin under that so the fill reliably lands. +// Exported so the length-ceiling test can check against the real value +// rather than a duplicated magic number. +export const SAFE_URL_CHARS = 1800; + +/** + * Build the pre-filled bug-report URL, filling the "Logs / screenshots" + * field directly with as much of the traceback/stderr tail as safely fits -- + * no paste needed for the common case. + * + * What doesn't fit (a long trace, or anything from the opt-in "include + * recent logs" button, which never goes in the URL at all -- it can be far + * larger than a URL should ever carry) falls back to the clipboard, which + * buildReportText always has the complete version of; a note in the field + * says so. Truncation keeps the END of the trace, since that is where the + * actual failure line is. * * Pure and exported so the field mapping can be tested without a browser -- * an OS string that does not match the dropdown option exactly is dropped by @@ -90,7 +132,7 @@ function technicalBlock(record, diag) { */ export function buildReportUrl(record, diag = {}) { const label = KIND_LABELS[record.kind] || "Something failed"; - const title = `[Bug]: ${label}${record.cause ? ` — ${record.cause}` : ""}`; + const title = `[Bug]: ${label}${record.cause ? ` - ${record.cause}` : ""}`; const what = [ `${label} in StemDeck.`, @@ -106,35 +148,48 @@ export function buildReportUrl(record, diag = {}) { record.context?.stage ? `2. StemDeck failed at: ${record.context.stage}` : "2. It failed.", ].join("\n"); - const tail = Array.isArray(record.tail) ? record.tail : []; - const build = (tailLines, note) => { - const parts = [technicalBlock(record, diag)]; - if (tailLines.length) parts.push("", "```", ...tailLines, "```"); - if (note) parts.push("", note); - const params = new URLSearchParams({ - template: "bug_report.yml", - title, - what, - steps, - os: osOption(diag.buildTarget), - version: diag.version ? `v${diag.version}` : "", - install: installOption(diag.buildTarget, Boolean(diag.isDesktop)), - extra: parts.join("\n"), - }); - return `${NEW_ISSUE_URL}?${params}`; + const base = { + template: "bug_report.yml", + title, + what, + steps, + os: osOption(diag.buildTarget), + version: diag.version ? `v${diag.version}` : "", + install: installOption(diag.buildTarget, Boolean(diag.isDesktop)), + }; + const toUrl = (extra) => `${NEW_ISSUE_URL}?${new URLSearchParams({ ...base, extra })}`; + + const tech = technicalBlock(record, diag); + const hasTraceback = Array.isArray(record.traceback) && record.traceback.length > 0; + const source = hasTraceback ? record.traceback : Array.isArray(record.tail) ? record.tail : []; + const sourceLabel = hasTraceback ? "Traceback" : "Stderr tail"; + const hasLogs = Boolean(record.logs) && Object.values(record.logs).some(Boolean); + const pasteNote = + "_Full report (including any traceback beyond what fits here, and any recent logs you " + + "included) is on your clipboard from this click - paste to see everything._"; + + const build = (keep) => { + const parts = [tech]; + if (keep > 0) { + const clipped = keep < source.length; + parts.push("", `${sourceLabel}${clipped ? " (end)" : ""}:`, "```", ...source.slice(-keep), "```"); + } + if (keep < source.length || hasLogs) parts.push("", pasteNote); + return toUrl(parts.join("\n")); }; - let url = build(tail, ""); - if (url.length <= MAX_URL_LENGTH) return url; - - // Too long: keep the end of the tail, which is where the actual error is, - // and point at the full logs rather than silently losing them. - const note = "_Tail truncated — full logs via Settings → Export logs._"; - for (let keep = Math.min(tail.length, 20); keep > 0; keep--) { - url = build(tail.slice(-keep), note); - if (url.length <= MAX_URL_LENGTH) return url; + let url = build(source.length); + if (url.length <= SAFE_URL_CHARS) return url; + for (let keep = Math.min(source.length, 60); keep > 0; keep--) { + url = build(keep); + if (url.length <= SAFE_URL_CHARS) return url; } - return build([], note); + url = build(0); + if (url.length <= SAFE_URL_CHARS) return url; + // Pathological case: even the bare technical block didn't fit (an + // enormous "what"/"steps"). Fall back to clipboard-only, same as the + // original fix for this bug. + return toUrl("Copied to your clipboard - paste it into this field."); } // ─── Records ────────────────────────────────────────────────────────────── @@ -320,24 +375,35 @@ async function openFailureDialog(record) { const msgEl = document.getElementById("failureMessage"); const techEl = document.getElementById("failureTech"); const reportEl = document.getElementById("failureReport"); + const discordEl = document.getElementById("failureReportDiscord"); + const logsBtn = document.getElementById("failureIncludeLogs"); if (titleEl) titleEl.textContent = KIND_LABELS[record.kind] || "Something failed"; if (whenEl) whenEl.textContent = new Date(record.at).toLocaleString(); if (msgEl) msgEl.textContent = record.detail ? `${record.message}\n${record.detail}` : record.message; if (techEl) techEl.textContent = "Collecting details…"; if (reportEl) reportEl.removeAttribute("href"); + if (discordEl) discordEl.href = DISCORD_URL; + if (logsBtn) { + delete record.logs; + logsBtn.disabled = false; + logsBtn.textContent = "Include recent logs"; + } dialog.classList.remove("hidden"); - // The stderr tail lives in the quarantined error.txt and is fetched now - // rather than at capture time -- one request when a user actually looks, - // instead of one per failure whether or not they care. + // The stderr tail and full traceback live in the quarantined error.txt and + // are fetched now rather than at capture time -- one request when a user + // actually looks, instead of one per failure whether or not they care. + // Both are already home-directory-redacted server-side (redact_home) before + // this response is built, since this is headed for a public report. if (!record.tail && record.context?.jobId) { try { const res = await fetch(`/api/jobs/${record.context.jobId}/failure`); if (res.ok) { const data = await res.json(); record.tail = Array.isArray(data.tail) ? data.tail : []; + record.traceback = Array.isArray(data.traceback) ? data.traceback : []; record.cause = record.cause || cleanCause(data.cause); record.context = { ...record.context, @@ -349,21 +415,64 @@ async function openFailureDialog(record) { persist(); } else { record.tail = []; + record.traceback = []; } } catch (e) { console.warn("[notifications] failure detail fetch failed:", e); record.tail = []; + record.traceback = []; } } const diag = await getDiagnostics(); - if (techEl) { - const tail = record.tail?.length ? `\n\n${record.tail.join("\n")}` : ""; - techEl.textContent = `${technicalBlock(record, diag)}${tail}`; - } + if (techEl) techEl.textContent = buildReportText(record, diag); + // One shared payload: the logs button mutates record.logs in place, and the + // report buttons must see that mutation on their next click without a + // separate re-sync step. + const payload = { record, diag }; // Set at open time; the global external-link handler reads href on click and // routes it through Tauri's open_url on desktop, a new tab in a browser. - if (reportEl) reportEl.href = buildReportUrl(record, diag); + // The click handlers wired in wireFailureDialog read record/diag back off + // this element's dataset to copy the full report to the clipboard - the + // reason the URL itself can stay short (see buildReportUrl). + if (reportEl) { + reportEl.href = buildReportUrl(record, diag); + reportEl._reportPayload = payload; + } + if (discordEl) discordEl._reportPayload = payload; + if (logsBtn) logsBtn._reportPayload = payload; +} + +/** Opt-in: the notification centre never fetches these on its own. Scoped to + * the failure's own timestamp (plus a small buffer) rather than a blind "last + * hour", since a user can open this dialog long after the failure happened. + * Already home-directory-redacted server-side (redact_home in main.py). */ +async function fetchRecentLogs(record) { + const minutes = Math.min(1440, Math.max(15, Math.ceil((Date.now() - record.at) / 60000) + 5)); + const logs = {}; + for (const view of LOG_VIEWS) { + try { + const res = await fetch(`/api/logs/${view}?minutes=${minutes}`); + logs[view] = res.ok ? (await res.text()).trim() : ""; + } catch (e) { + console.warn(`[notifications] failed to fetch the ${view} log:`, e); + logs[view] = ""; + } + } + return logs; +} + +/** Full diagnostic text is only useful pasted somewhere, so both report + * buttons copy it to the clipboard on click - the GitHub link's `extra` + * field says as much, and Discord has nowhere to prefill at all. */ +async function copyReportToClipboard(el) { + const payload = el._reportPayload; + if (!payload) return; + try { + await navigator.clipboard.writeText(buildReportText(payload.record, payload.diag)); + } catch (e) { + console.warn("[notifications] clipboard write failed:", e); + } } function wireFailureDialog() { @@ -378,6 +487,20 @@ function wireFailureDialog() { e.stopPropagation(); clearFailures(); }); + document.getElementById("failureReport")?.addEventListener("click", (e) => copyReportToClipboard(e.currentTarget)); + document.getElementById("failureReportDiscord")?.addEventListener("click", (e) => copyReportToClipboard(e.currentTarget)); + document.getElementById("failureIncludeLogs")?.addEventListener("click", async (e) => { + const btn = e.currentTarget; + const payload = btn._reportPayload; + if (!payload || btn.disabled) return; + btn.disabled = true; + btn.textContent = "Fetching logs…"; + payload.record.logs = await fetchRecentLogs(payload.record); + persist(); + const techEl = document.getElementById("failureTech"); + if (techEl) techEl.textContent = buildReportText(payload.record, payload.diag); + btn.textContent = "Logs included"; + }); } export async function initNotifications({ diagnostics } = {}) { diff --git a/tests/js/report-url.test.mjs b/tests/js/report-url.test.mjs index ca988241..cbde6ab3 100644 --- a/tests/js/report-url.test.mjs +++ b/tests/js/report-url.test.mjs @@ -13,7 +13,13 @@ // // Run: node tests/js/report-url.test.mjs -import { buildReportUrl, osOption, installOption } from "../../static/js/notifications.js"; +import { + buildReportUrl, + buildReportText, + osOption, + installOption, + SAFE_URL_CHARS, +} from "../../static/js/notifications.js"; let pass = 0, fail = 0; @@ -28,6 +34,11 @@ const check = (name, cond, detail = "") => { }; const params = (url) => new URL(url).searchParams; +// Substring checks against the raw URL miss content URLSearchParams encoded +// (a space becomes "+", which plain decodeURIComponent does NOT reverse -- +// that's an application/x-www-form-urlencoded convention) -- decode properly +// so a check for e.g. "line 399" actually means what it says. +const decoded = (url) => decodeURIComponent(url.replace(/\+/g, " ")); // The exact option strings from bug_report.yml. If the template changes, this // list changes with it -- that is the point. @@ -95,7 +106,7 @@ check( "blank issues are disabled, so template must be present", url.startsWith("https://github.com/stemdeckapp/stemdeck/issues/new?"), ); -check("title leads with the cause", q.get("title") === "[Bug]: Import failed — out-of-memory"); +check("title leads with the cause", q.get("title") === "[Bug]: Import failed - out-of-memory"); check("version is prefixed with v", q.get("version") === "v0.9.1"); check("os matches the dropdown", q.get("os") === "Windows"); check("install matches the dropdown", q.get("install") === "Windows ZIP"); @@ -103,11 +114,82 @@ check("what carries StemDeck's own message", q.get("what").includes("Audio proce check("what carries the classified detail", q.get("what").includes("out-of-memory")); check("steps names the stage it died at", q.get("steps").includes("Error: Processing failed")); -const extra = q.get("extra"); -check("extra reports the device and the CPU fallback", extra.includes("cuda (fell back to CPU)")); -check("extra reports the model", extra.includes("htdemucs_6s")); -check("extra carries the stderr tail", extra.includes("Tried to allocate 2.40 GiB")); -check("tail is fenced so GitHub renders it as code", extra.includes("```")); +// A short tail like this one fits comfortably inside the safe URL budget, so +// it's filled directly into the "Logs / screenshots" field -- no paste +// needed for the common case (see the length-ceiling section below for what +// happens when it doesn't fit). +check("extra carries the short stderr tail directly", decoded(url).includes("Tried to allocate 2.40 GiB")); +check( + "extra is not just a clipboard pointer when the content already fit", + !q.get("extra").startsWith("Copied to your clipboard"), +); + +const text = buildReportText(RECORD, DIAG); +check("report text reports the device and the CPU fallback", text.includes("cuda (fell back to CPU)")); +check("report text reports the model", text.includes("htdemucs_6s")); +check("report text has no traceback section when none was fetched", !text.includes("Traceback:")); + +// ─── traceback ─── +// +// get_failure() now also returns the full backend traceback (already +// home-directory-redacted server-side), fetched into record.traceback the +// same way record.tail is. It must show up in the clipboard text, and -- when +// short enough, as it is here -- directly in the URL too, preferred over the +// stderr tail when both are present. + +const WITH_TRACEBACK = { + ...RECORD, + traceback: [ + "Traceback (most recent call last):", + ' File "/AppData/Local/Programs/Python/Python312/Lib/asyncio/threads.py", line 25, in to_thread', + "torch.OutOfMemoryError: CUDA out of memory.", + ], +}; +const tracebackText = buildReportText(WITH_TRACEBACK, DIAG); +check("report text carries the traceback", tracebackText.includes("CUDA out of memory.")); +check("report text labels the traceback section", tracebackText.includes("Traceback:")); +check( + "the traceback stays fenced as code, same as the stderr tail", + tracebackText.split("```").length - 1 === 4, + `${tracebackText.split("```").length - 1} fence markers`, +); +const tracebackUrl = buildReportUrl(WITH_TRACEBACK, DIAG); +const tracebackDecoded = decoded(tracebackUrl); +check("a short traceback is filled directly into the URL too", tracebackDecoded.includes("threads.py")); +check( + "the traceback is preferred over the stderr tail when both are present", + !tracebackDecoded.includes("Tried to allocate"), +); +check("report text carries the stderr tail", text.includes("Tried to allocate 2.40 GiB")); +check("tail is fenced so GitHub renders it as code", text.includes("```")); + +// ─── opt-in "include recent logs" ─── +// +// Unlike tail/traceback, record.logs is never fetched automatically -- only +// present once the user clicks the button in notifications.js. An empty +// string for a view (fetch failed, or nothing in the window) must not render +// an empty section. + +const WITH_LOGS = { + ...RECORD, + logs: { + backend: "2026-08-17 16:50:02 E stemdeck backend crashed", + application: "", + // setup intentionally absent -- same as "not fetched" + }, +}; +const logsText = buildReportText(WITH_LOGS, DIAG); +check("report text carries the backend log when present", logsText.includes("backend crashed")); +check("report text labels which log a section came from", logsText.includes("Recent backend log:")); +check("an empty log view produces no section", !logsText.includes("Recent application log:")); +check("an unfetched log view produces no section", !logsText.includes("Recent setup log:")); +check("no logs section at all when record.logs was never set", !text.includes("Recent backend log:")); +const logsUrl = buildReportUrl(WITH_LOGS, DIAG); +check("recent logs never reach the URL, even when the rest of the content fit", !decoded(logsUrl).includes("crashed")); +check( + "a clipboard note is added when logs were fetched, even though the short tail fit on its own", + decoded(logsUrl).includes("clipboard"), +); // preflight is a checkboxes field: GitHub cannot prefill it, and we must not // pretend otherwise -- the user ticking it is the "I searched for duplicates" @@ -129,17 +211,31 @@ check("never carries a track title", !privateUrl.includes("Private%20Demo") && ! check("never carries a source URL", !privateUrl.includes("youtube.com") && !privateUrl.includes("dQw4w9WgXcQ")); // ─── length ceiling ─── +// +// Windows opens this URL via explorer.exe, which silently opens a plain File +// Explorer window instead of erroring past roughly 2000 characters -- the bug +// this file exists to guard against. A short trace is filled directly into +// the URL (see above); an oversized one must still keep the whole URL under +// that ceiling, filling in as much of the END of the tail as fits (that's +// where the actual error is) and pointing at the clipboard for the rest. const HUGE = { ...RECORD, tail: Array.from({ length: 400 }, (_, i) => `line ${i}: ${"x".repeat(120)}`), }; const hugeUrl = buildReportUrl(HUGE, DIAG); -check("stays inside the URL ceiling", hugeUrl.length <= 6000, `${hugeUrl.length} chars`); -const hugeExtra = params(hugeUrl).get("extra"); -check("truncation keeps the END of the tail, where the error is", hugeExtra.includes("line 399")); -check("truncation says so and points at the logs", hugeExtra.includes("Export logs")); -check("truncation does not eat the report body", params(hugeUrl).get("what").includes("Audio processing failed")); +check( + "URL stays well under the platform ceiling regardless of tail size", + hugeUrl.length <= SAFE_URL_CHARS, + `${hugeUrl.length} chars (ceiling ${SAFE_URL_CHARS})`, +); +const hugeDecoded = decoded(hugeUrl); +check("truncation fills in as much of the END of the tail as fits", hugeDecoded.includes("line 399")); +check("truncation drops the START of an oversized tail", !hugeDecoded.includes("line 0:")); +check("truncation points at the clipboard for what didn't fit", hugeDecoded.includes("clipboard")); +const hugeText = buildReportText(HUGE, DIAG); +check("the clipboard text keeps the full, untruncated tail", hugeText.includes("line 399")); +check("the clipboard text keeps the start of the tail too", hugeText.includes("line 0:")); // ─── degenerate input ─── diff --git a/tests/test_logs_api.py b/tests/test_logs_api.py index 3abc6d96..8b704037 100644 --- a/tests/test_logs_api.py +++ b/tests/test_logs_api.py @@ -194,6 +194,53 @@ def test_tail_reads_the_previous_rotation_too(client, logs_dir): ) +def test_tail_redacts_a_source_url(client, logs_dir): + """download.py logs every job's source URL at info level, not just the + failing job's -- a raw log tail would otherwise leak the YouTube/ + SoundCloud link for everything the reporter has imported in the fetched + window into a public GitHub issue or Discord message.""" + (logs_dir / "stemdeck.log").write_text( + f"{_stamp(1)} I stemdeck.download [abc] download starting: " + "https://www.youtube.com/watch?v=dQw4w9WgXcQ\n", + encoding="utf-8", + ) + body = client.get("/api/logs/application?minutes=60").text + assert "youtube.com" not in body + assert "dQw4w9WgXcQ" not in body + assert "" in body + + +def test_tail_redacts_an_ip_address(client, logs_dir): + """The mobile UI talks to this backend over the LAN when network access + is on, so uvicorn's access log (captured into backend.log on desktop) can + carry another device's address on the reporter's home network.""" + (logs_dir / "backend.log").write_text( + f'{_stamp(1)} I stemdeck INFO: 192.168.1.14:52341 - "GET /api/jobs HTTP/1.1" 200 OK\n', + encoding="utf-8", + ) + body = client.get("/api/logs/backend?minutes=60").text + assert "192.168.1.14" not in body + assert "" in body + + +def test_tail_redacts_the_users_home_directory(client, logs_dir): + """The notification centre's opt-in "include recent logs" button hands this + straight to a public GitHub issue or Discord message with no filtering step + of its own -- redaction has to happen here, not trust every future caller + to remember it (#report-full-stack).""" + from pathlib import Path + + home = str(Path.home()) + (logs_dir / "stemdeck.log").write_text( + f'{_stamp(1)} E stemdeck File "{home}\\AppData\\Local\\Programs\\Python\\Python312\\Lib\\threads.py", line 25\n', + encoding="utf-8", + ) + body = client.get("/api/logs/application?minutes=60").text + assert home not in body + assert "" in body + assert "threads.py" in body, "the rest of the path must survive -- it's the useful part" + + def test_tail_parses_the_setup_log_epoch_format(client, logs_dir): """setup.log is written by the Tauri shell with epoch seconds, because the crate has no date library.""" diff --git a/tests/test_pipeline_runner.py b/tests/test_pipeline_runner.py index 49a06c83..fc2afd92 100644 --- a/tests/test_pipeline_runner.py +++ b/tests/test_pipeline_runner.py @@ -186,6 +186,57 @@ def boom(*args, **kwargs): assert '"download": 1.2' in report assert not (quarantined / "source.wav").exists() assert not (quarantined / "stems").exists() + # Full traceback is captured too (#report-full-stack), not just the + # classified cause/tail -- named after the function that actually raised. + assert "--- traceback ---" in report + assert "in boom" in report + assert "SeparationError" in report + + +def test_redact_home_strips_the_users_home_directory(): + """A traceback carries absolute paths, and on Windows the Python install + path alone embeds the reporter's OS username -- this text is headed for a + public GitHub issue or Discord message, so it must never reach one raw.""" + from app.core.redact import redact + + home = str(Path.home()) + text = f'File "{home}\\AppData\\Local\\Programs\\Python\\Python312\\Lib\\asyncio\\threads.py", line 25' + redacted = redact(text) + assert home not in redacted + assert "" in redacted + assert "threads.py" in redacted, "the rest of the path must survive -- it's the useful part" + + +@pytest.mark.asyncio +async def test_quarantine_redacts_a_source_url_embedded_in_the_exception(tmp_path: Path): + """yt-dlp errors often embed the URL they were fetching in the message + itself (e.g. "Unsupported URL: ") -- exc!r reaching error.txt + unredacted would leak it even though title:/source: are already excluded + from the public API response.""" + job = Job(id="abcdefabcde7", source_url="https://www.youtube.com/watch?v=dQw4w9WgXcQ") + job_dir = tmp_path / job.id + (job_dir / "stems").mkdir(parents=True) + (job_dir / "stems" / "vocals.wav").write_bytes(b"RIFF" + b"\x00" * 64) + (job_dir / "source.wav").write_bytes(b"RIFF" + b"\x00" * 64) + source = job_dir / "source.wav" + + def boom(*args, **kwargs): + raise RuntimeError("Unsupported URL: https://www.youtube.com/watch?v=dQw4w9WgXcQ") + + with patch("app.pipeline.runner._run_local_blocking", side_effect=boom): + await run_local_pipeline(job, source, tmp_path) + + report = (tmp_path / "failed" / job.id / "error.txt").read_text(encoding="utf-8") + lines = report.splitlines() + source_line = next(line for line in lines if line.startswith("source:")) + exception_line = next(line for line in lines if line.startswith("exception:")) + # title:/source: are local-only (never served by the /failure API) and + # keep the real URL, unredacted, for the person looking at their own disk. + assert source_line == "source: https://www.youtube.com/watch?v=dQw4w9WgXcQ" + # exception: IS served by the /failure API, and yt-dlp errors often embed + # the URL they were fetching in the message itself -- must be redacted. + assert "youtube.com" not in exception_line + assert "" in exception_line @pytest.mark.asyncio diff --git a/tests/test_redact.py b/tests/test_redact.py new file mode 100644 index 00000000..79041103 --- /dev/null +++ b/tests/test_redact.py @@ -0,0 +1,75 @@ +from __future__ import annotations + +from pathlib import Path + +from app.core.redact import redact + + +def test_strips_the_home_directory(): + home = str(Path.home()) + text = f"{home}\\AppData\\Local\\Programs\\Python\\Python312\\Lib\\threads.py" + result = redact(text) + assert home not in result + assert "" in result + assert "threads.py" in result + + +def test_strips_a_youtube_url(): + text = "download starting: https://www.youtube.com/watch?v=dQw4w9WgXcQ&list=RDdQw4w9WgXcQ" + result = redact(text) + assert "youtube.com" not in result + assert "dQw4w9WgXcQ" not in result + assert "" in result + + +def test_strips_a_youtu_be_url(): + result = redact("source: https://youtu.be/dQw4w9WgXcQ") + assert "youtu.be" not in result + assert "" in result + + +def test_strips_a_soundcloud_url(): + result = redact("source: https://soundcloud.com/artist/track") + assert "soundcloud.com" not in result + assert "" in result + + +def test_does_not_touch_an_unrelated_url(): + """Only the hosts this app actually pulls tracks from are source URLs -- + a link to the app's own site/repo, or anything else, is not personal + information and must survive (it's often the useful part of a log line).""" + text = "see https://github.com/stemdeckapp/stemdeck/issues/277 and https://stemdeck.app" + result = redact(text) + assert result == text + + +def test_strips_an_ipv4_address(): + result = redact('192.168.1.14:52341 - "GET /api/jobs HTTP/1.1" 200 OK') + assert "192.168.1.14" not in result + assert "" in result + + +def test_does_not_mistake_a_version_string_for_an_ip(): + result = redact("StemDeck v0.9.1.dev2+g682ab90d7.d20260816") + assert "0.9.1" in result + assert "" not in result + + +def test_does_not_mistake_a_timestamp_for_anything(): + """The pipeline's own log format is `YYYY-MM-DD HH:MM:SS ...` -- every + single line has one, so any redaction pattern with false positives here + would mangle the entire report, not just the rare real leak.""" + text = "2026-08-17 16:50:02 E stemdeck.pipeline pipeline failed for job abcdefabcdef" + assert redact(text) == text + + +def test_composes_all_three_kinds_in_one_pass(): + home = str(Path.home()) + text = f"{home}\\stemdeck 192.168.1.14 requested https://www.youtube.com/watch?v=dQw4w9WgXcQ" + result = redact(text) + assert home not in result + assert "192.168.1.14" not in result + assert "youtube.com" not in result + assert "" in result + assert "" in result + assert "" in result From 6c3cd582c9d73aa3c8b8bc6ece3d5cb53eab5206 Mon Sep 17 00:00:00 2001 From: Thales <> Date: Mon, 17 Aug 2026 18:04:14 +0100 Subject: [PATCH 5/5] Update StemDeck repository version to 0.11.0 Matches the pattern of the actual last version-bump commit (b2379ed): only templates/stemdeck.xml is hand-edited. desktop/src-tauri/Cargo.toml, tauri.conf.json, and package.json stay at their 0.0.0 placeholder - they're sed-rewritten transiently inside each release workflow run, never committed. pyproject.toml's version is derived from the git tag by hatch-vcs and is never hand-edited at all (see its own header comment). Note: v0.11.0 is not yet a published tag/release, so this pin won't resolve to a real image until one is cut. --- templates/stemdeck.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/stemdeck.xml b/templates/stemdeck.xml index daa11ae0..57c8c2ce 100644 --- a/templates/stemdeck.xml +++ b/templates/stemdeck.xml @@ -1,7 +1,7 @@ StemDeck - ghcr.io/stemdeckapp/stemdeck:0.10.0 + ghcr.io/stemdeckapp/stemdeck:0.11.0 https://github.com/stemdeckapp/stemdeck/pkgs/container/stemdeck bridge sh