The dev-nightly build runs `ruff check .` before "Build and push to GHCR" in the
same job, so the three S110 (try/except/pass) errors introduced since the last
green build (ce6ce4d) failed the lint step and SKIPPED the image push entirely —
every dev-nightly since #704 went red, so the dev image was never rebuilt and none
of the recent fixes (incl. the #852 WebSocket login-bypass fix) ever shipped to
the image users pull.
All three are deliberate best-effort swallows; annotate them with the repo's
existing `# noqa: S110 — <reason>` convention rather than adding dead logging:
- relocate.py: tag write is best-effort (re-import re-derives tags)
- acoustid_scanner.py: verification-status tag is optional context
- web_server.py: audio-duration probe falls through to 0
ruff check . + compileall now clean; pytest already passed in CI at ce6ce4d.
PR #853 added artist album-list caching to Deezer, but unlike the Spotify path it
had no equivalent of the truncated-fetch guard: Deezer paginates, and a transient/
malformed response on page 2+ (artist with >100 albums) broke the loop and cached
the PARTIAL list as the full discography — serving an incomplete album list from
cache until TTL.
Fix: track whether pagination finished cleanly. A malformed/empty-of-data response
mid-walk now clears a `complete` flag and the artist→album-LIST is cached only when
complete. Individual album entities still cache regardless (each is complete; we
just have fewer). A clean end (short page / empty page / reached limit) still caches
as before.
Tests: a page-1-ok / page-2-errors walk no longer caches (second call refetches
instead of serving a permanently-incomplete list); a clean two-page walk still
caches (happy path intact). 181 deezer/metadata tests green.
noldevin: a magnet stuck "downloading metadata" ran 11h despite a 15-min stall
timeout, got cleared from SoulSync but left active in qbit, then re-grabbed as a
duplicate. Two bugs:
1. Stall never fired on metaDL. StallTracker reset its clock on any `downloaded`
byte increase, but a metaDL torrent's byte counter still ticks up from DHT/peer
protocol overhead while making no real progress — so the clock reset forever.
Fix: the byte counter only counts once metadata is in (size>0). During the
metadata phase (size==0) the only thing that counts as progress is *obtaining*
metadata, so a magnet that can't even do that within the timeout is correctly
flagged stalled. size=None preserves the old byte-only behavior (back-compat).
2. Orphaned in qbit. The monitor's stall exit removed the torrent, but the `error`
exit and the 6h deadline exit only marked the download failed — leaving the
torrent active in qbit, untracked here, so SoulSync re-grabbed the same dead
torrent (qbit logs the duplicate-add). Fix: both terminal exits now run
_cleanup_torrent (shared with the stall path), which removes+deletes (abandon)
or pauses per the stall action — nothing is left orphaned.
Tests (10 new): metaDL byte-noise no longer resets the clock (stalls at timeout);
obtaining metadata resets it; real byte-progress still tracked after metadata;
_cleanup_torrent removes+delete_files on abandon / pauses on pause / no-ops on
empty hash or no adapter / swallows a client error. 151 torrent tests green.
The #832 fix enforces the launch PIN / login via a Flask before_request hook, but
that hook does NOT run for the socketio handshake — empirically a normal endpoint
401s while /socket.io/ returns 200 with the gate on. So removing the client overlay
(Safari "Hide Distracting Items", devtools) + opening a socket streams live data
(downloads, logs, dashboard, notifications) completely unauthenticated.
Fix: the socketio connect handler now enforces the same check and returns False
(rejects the connection) when a gate is active and the session isn't verified.
Rejecting connect blocks every downstream WS event (subscribe/join), so all live
data is covered. core/security/ws_gate.is_ws_connection_blocked is the pure seam:
login mode (when on) > launch PIN > open, mirroring the HTTP gate exactly. Fails
OPEN on a config-read error, same as the HTTP gate.
Audited every other surface empirically with the gate on + unauthenticated: SSE
streams, catch-all pages, library/dashboard data, admin endpoints, search,
image-proxy, audio-stream (incl. a /etc/passwd traversal probe) all 401; /api/v1
key-gated. The WebSocket was the only hole.
Tests (10): pure gate logic (login>pin precedence, all on/off combos) + real
socketio.test_client integration — connect rejected when gate on + unauthenticated,
allowed when gate off or PIN verified.
Root cause (from the reporter's app.log): a ListenBrainz weekly playlist syncs
through the in-memory youtube_playlist_states discovery machine. When that live
state is lost — a Docker restart, or the discovery process ending while the user
waits for the media-server scan — the DB discover-download snapshot survives but
the live state is gone. Every recovery action (Cancel/Reset/Delete) then hit
`key not in states` and returned 404 "YouTube playlist not found" (hence the
confusing "Youtube" on a ListenBrainz playlist), leaving the playlist permanently
wedged with no way to dismiss or re-sync. Works for the maintainer because a
single session with no restart keeps the live state alive.
Fix — these are cleanup ops, so "the thing is already gone" is SUCCESS, not 404:
- cancel_sync core (shared by YouTube + ListenBrainz + Tidal/Deezer/Qobuz/...) →
missing key returns idempotent success.
- reset_youtube_playlist / delete_youtube_playlist → same.
The playlist becomes recoverable: Cancel/Reset clears the dead state and the user
re-syncs fresh.
Tests: cancel_sync core (missing key = idempotent 200 not 404; present key still
cancels + clears the worker + reverts phase); endpoint-level idempotency for
cancel/reset/delete; updated the old test that locked the 404 wedge. 834 sync/
discovery tests green.
The 'retag' fix corrects a mismatched file's tags/DB but leaves it in the WRONG
artist/album folder, so the library shows the right title while the file sits under
the previous track. AcoustID yields only title+artist (no reliable album), so an
in-place move has no safe target.
New 'relocate' action: retag the file, move it into Staging, drop the stale tracks
row, and clean up the emptied folder. The auto-import worker (which watches Staging)
re-identifies it with full metadata and files it correctly — reusing the import
pipeline instead of guessing a destination.
- core/repair_jobs/relocate.py: pure, injectable orchestration (retag -> move ->
drop row) + collision-safe staging_destination. Row is dropped only AFTER a
successful move, so a failed move never orphans the library entry.
- _fix_acoustid_mismatch gains the 'relocate' branch (thin wrapper: resolve path,
staging dir, drop-row closure, empty-parent cleanup).
- UI: "Relocate" button on the AcoustID-mismatch fix modal.
Tests (8): staging-dest collision suffixing; relocate happy path; tag-write failure
still relocates; FAILED move does NOT drop the row; no-tags skips write; a real
file move through safe_move_file; and a handler integration test (file moved to
staging + tracks row deleted end-to-end). Repair + integrity suites green.
resolve_history_audio_path drives a DESTRUCTIVE delete (os.remove), but lived
endpoint-bound in web_server with zero tests. Lifted to core/matching/history_paths
with injected effects (exists / resolve_library_path / lookup_titled_paths) so the
fallback chain — and the collision-safety that stops delete() from removing the
wrong same-title file — is a clean importable seam. web_server now wraps it (DB
lookup + os.path.exists + prefix resolver injected); behavior preserved.
9 tests lock it: recorded-path hit, prefix-resolve fallback, single tracks-table
candidate, and the safety rules — multiple same-title candidates with NO artist ->
None (refuse to guess), artist filter picks only the matching path, artist named
but unmatched -> None, no-title/empty-lookup -> None. Full suite green (5906).
The candidate matcher rejected valid downloads of titles with a '/' or ':' (e.g.
Sawano's "You See Big Girl / T:T") because the unified normalize() removed those
chars ("t:t" -> "tt") while keeping the '_' that source filenames substitute for
them ("T_T" -> "t_t") — the asymmetry tanked the similarity score. Now '/ \ : _'
all map to spaces before the strip, so "/ T:T" and "_ T_T" both normalize to "t t".
Verified on the real library: similarity for the Sawano pair 0.927 -> 1.000; only
348/40786 strings (0.85%, all containing those separators) change; worst-case
joined-variant match (e.g. "12:05" vs "1205") stays 0.889, well above the 0.70
title threshold — no match regressions. Fixes the matching half of #851.
Closes the forgot-login-password gap. A per-profile recovery question + answer lets
a locked-out user reset their own password.
- DB: additive recovery_question + recovery_answer_hash columns (idempotent
migration). set/get-question/verify/has methods; answer is hashed (pbkdf2) and
matched forgivingly (trim + lowercase + collapse whitespace). No recovery set →
never verifies.
- Endpoints (allowlisted in the login gate so they work pre-auth):
GET /api/auth/recovery-question?username= (generic 404 when absent),
POST /api/auth/recovery-reset {username, answer, new_password} — brute-force
limited; a correct answer sets the new password + authenticates the session.
POST /api/profiles/<id>/set-recovery (admin or self) to configure it.
Tests: set/get/verify, forgiving match, hashed-not-plaintext, no-recovery-never-
verifies, full reset flow (wrong answer rejected + password intact; correct answer
resets), unknown-user 404. 25 tests pass. Next: the Settings + login-screen UI.
The backend auth for opt-in username/password mode (security.require_login, default
off → zero change; the launch PIN + picker behave exactly as today).
- core/security/login_gate.py: pure gate (mirrors launch_lock) — when login mode is
on, an unauthenticated session reaches only the page shell, /api/auth/login,
/api/auth/logout, /api/profiles/current, /api/setup/status, and the key-authed
/api/v1 API. Deliberately does NOT expose the profile list pre-auth (you type your
name, not pick from a roster).
- _enforce_login before_request enforces it; _enforce_launch_pin no-ops when login
mode is on (login replaces the shared PIN, per design).
- POST /api/auth/login (username = profile name, case-insensitive; brute-force
limited per IP; generic error so names don't leak) + POST /api/auth/logout.
- Anti-lockout: the settings save refuses to turn ON login mode until the admin
account has a password.
Tests: gate blocks→login→access→logout→blocked; case-insensitive username; wrong
password / passwordless profile / unknown user all 401 generically; login list not
exposed pre-auth; can't enable login without an admin password. 12 tests pass. Next:
the login screen + set-password UI + the toggle (increment 3).
Lets SoulSync sit behind Authelia/Authentik/oauth2-proxy as the gatekeeper: when
security.auth_proxy_header names a header (e.g. Remote-User), a request carrying it
is treated as already-authenticated and passes the launch lock — the proxy did the
login (with 2FA).
- core/security/auth_proxy.py: trusted_proxy_user(get_header, header_name) — returns
the user iff the configured header is present + non-empty; empty header name (the
default) → always None → feature off.
- _enforce_launch_pin ORs it into pin_verified. OFF by default, so a direct install
is unaffected AND a client-spoofed header does nothing unless the operator opted in.
- Doc'd in Support/REVERSE-PROXY.md with the must-strip-client-headers warning.
This is the lightweight Tier 3 (auth-proxy integration), not a full per-user login —
the proxy owns identity; SoulSync trusts it.
Tests: helper off/on/blank/exception-safe; integration — trusted header passes the
gate, no header is locked, and (the safety pin) a spoofed header is IGNORED when the
feature is off. 6 tests pass.
Fold a conservative security-header set into the SAME opt-in proxy mode, so it's
zero-impact when off. When security.trust_reverse_proxy is on, an after_request
adds X-Content-Type-Options: nosniff, X-Frame-Options: SAMEORIGIN, and HSTS
(safe — only honoured over the proxy's HTTPS), via setdefault so it never clobbers
a header the proxy already set. No CSP (needs per-deploy tuning; better at the
proxy). When OFF (default), the after_request isn't registered → no headers added.
Tests: off adds none of the headers; on adds all three. Doc updated. 6 tests pass.
A publicly-exposed instance gated only by the launch PIN was brute-forceable. Added
a lenient in-memory failed-attempt limiter (core/security/rate_limit.py): 10 wrong
PINs from one IP within 5 min → 429 with Retry-After, failures age out on their own
(self-heal, no persistent lockout), and a CORRECT entry clears that IP instantly.
Wired into /api/profiles/verify-launch-pin. By design it can only ever trigger on a
flood of WRONG PINs — correct entry, a couple of typos, or a no-PIN install are
never affected, so normal use sees no change. Keyed per-IP so an attacker can't
lock out a legit user.
Tests: limiter is lenient under threshold, trips on a flood, success clears it,
failures self-heal, per-IP isolation; endpoint returns 429 after 10 wrong PINs with
Retry-After. 6 tests pass.
Tier 1 of "secure behind a reverse proxy". STRICTLY opt-in so direct/LAN installs
are byte-for-byte unchanged.
- core/security/reverse_proxy.py: apply_reverse_proxy_mode(app, config_get) — a
no-op unless security.trust_reverse_proxy=true. When OFF (default), the app is
untouched: no ProxyFix, X-Forwarded-* stays UNtrusted (a direct client can't
spoof its IP/scheme), session cookie keeps Flask defaults. When ON (operator is
behind nginx/Caddy/Traefik with TLS): trust one proxy hop's X-Forwarded-*, and
mark the session cookie Secure + SameSite=Lax. Any config error → safe no-op,
never breaks startup.
- Wired once at app init.
- Support/REVERSE-PROXY.md: nginx (with the Socket.IO Upgrade headers people
always miss) / Caddy / Traefik configs, the setting, and the "put auth in front
(Authelia/Authentik/oauth2-proxy)" recommendation + the off-for-plain-HTTP note.
Tests: off (and missing-key, and a config exception) is a strict no-op — not
ProxyFix-wrapped, cookie defaults intact; on wraps ProxyFix + secures the cookie;
and the real web_server app is NOT in proxy mode by default. 5 tests pass.
The enhanced-tab "Sync" button's stale-removal phase deleted any track whose file
wasn't on disk, with NO guard — so if the music storage was momentarily
unavailable (sleeping NAS, dropped mount, unmounted Docker volume, WSL hiccup),
os.path.exists returned False for EVERY file and one click wiped the whole artist
(tracks + their now-"empty" albums) from the DB. The deep-scan path already had a
50%-stale safety net (#828); this endpoint never got one.
- New core/library/stale_guard.py: is_implausible_stale_removal(missing, total) —
a tested rule (skip removal when missing > 50% of a >=5-track set), centralised
so every stale-removal site can share it.
- sync_artist_library: if the guard trips, SKIP removal (delete nothing), return
removal_skipped + warn; the frontend shows "storage may be offline — skipped"
instead of silently deleting. Empty-album cleanup now also only runs on the
non-skipped path and uses `album_id IS NOT NULL` (fixes the NOT IN-with-NULL
no-op). Frontend also refreshes the view on additions, not just removals.
- @admin_only on the endpoint — it deletes tracks + albums but was ungated, while
the sibling delete_album endpoint is gated.
Deep scan was already safe (different mechanism: server-diff + its own 50% guard).
Tests: guard unit rules; endpoint skips removal when all files missing (keeps the
tracks), removes only the genuinely-gone few otherwise, and 403s for non-admins.
7 new tests pass.
The #843 fallback saved the discovery-cache match using the client's
original_artist verbatim — but the client sends a joined "A, B, C" string, while
EVERY in-memory + sync path keys the cache by the first artist (artists[0]). So a
multi-artist fix (the reporter's exact "Cherrymoon Traxx, Hermol, SBM, BELS"
case) would have saved under a key the sync never looks up — the fix would
"succeed" with no error but silently never apply.
Reduce the client artist to the first (split on comma) in the no-state branch so
its cache key matches the in-memory/sync convention exactly. Single-artist tracks
are unaffected.
Test: no-state save now keys by the first artist, and a new test pins that the
no-state and in-memory paths produce an IDENTICAL cache key for the same
multi-artist track. 74 discovery tests pass.
The discovery FIX → Confirm flow 404'd with "Discovery state not found" whenever
the in-memory discovery state was gone — a server restart, or an imported
playlist that wasn't discovered in THIS process — even though the card is still
shown from persisted data (the reporter's log shows "Returning 0 stored ...
playlists for hydration", i.e. the in-memory states were empty).
The thing that actually makes a manual fix STICK is writing it to the discovery
cache (save_discovery_cache_match), keyed by the original track's name + artist —
which doesn't need the in-memory state at all. But the endpoint 404'd on the
missing state before reaching that write, so the fix was dead after a restart.
- update_discovery_match (core/discovery/endpoints.py) now only does the in-memory
result update when the state exists; the durable discovery-cache write always
runs, falling back to client-provided original_name/original_artist when there's
no in-memory state. With neither a state nor originals it still 404s (unchanged).
- The FIX confirm (wishlist-tools.js) now sends original_name/original_artist
(from the source track it already has) so the backend can key the cache.
Covers all sources that share the helper (tidal/deezer/qobuz/spotify-public).
Tests: no-state-but-originals saves the cache + returns success; no-state-no-
originals still 404s; existing with-state path unchanged. 73 discovery tests pass.
Regression in 2.6.9. The spotify_public source adapter (used by auto-sync /
refresh_mirrored) called scrape_spotify_embed() directly — the embed widget only
exposes ~100 tracks — instead of fetch_spotify_public(), the wrapper the rest of
the app uses, which pulls the full list via the paginated public API and only
falls back to the embed on failure. So initial discovery got the whole playlist
but every auto-sync re-fetch truncated it to 100.
Switched the adapter to fetch_spotify_public (same return shape — drop-in). Albums
still resolve via the embed (already whole); on any failure it falls back to the
embed exactly as before.
Test: the adapter returns all 150 tracks when the full fetch yields 150 (was
capped at 100). 29 adapter tests pass.
A wishlist track (or tracks in an album) that slskd accepted then REJECTED would
sit at "DOWNLOADING... 0%" indefinitely, spam an ERROR every status poll
("…Completed, Rejected - letting monitor handle retry"), and — for albums —
block the whole batch from ever completing.
Root defect: the status formatter's non-manual error branch keeps the task
'downloading' and trusts the retry monitor to resolve it, with NO backstop. When
the monitor can't make progress (a rejected transfer with no other source), the
task is stuck forever.
Backstop: measure how long the ERROR state has persisted (keyed off the task's
last status transition, so a slow-but-healthy transfer is never failed, and each
monitor-retry episode gets a fresh window). Once it exceeds the monitor's retry
window (60s, vs the monitor's ~15s) with no resolution, mark the task failed and
fire the worker-freeing completion callback so the batch can finish. Also log the
error ONCE per episode instead of every 2s poll. The healthy path is untouched —
a working retry transitions the task before the grace, so this never fires.
Manual picks still fail immediately (unchanged).
Tests: rejected-within-grace stays downloading; rejected-beyond-grace fails +
schedules completion; manual pick fails immediately. 45 status tests pass.
YouTube/Tidal/Qobuz results encode the name as ``id||title``. When the title
itself contains a '/' (e.g. the Sawano AoT track "YouSeeBIGGIRL/T:T"), two places
wrongly basename-split it on the slash and kept only the last segment ("T:T"):
- core/downloads/file_finder.py — the completed-download finder truncated its
search target to "T:T", so the real on-disk file (slash sanitised by the
writer) never matched → "not found after processing" → the download got
QUARANTINED. Now an encoded ``id||title`` keeps the whole title as the target
and contributes no remote-directory components; real Soulseek PATHS still get
basename + dir extraction unchanged.
- webui/static/downloads.js — the manual-search FILE column showed only "T:T".
Added a ``||``-aware short-label helper (mirrors the correct handling already
used elsewhere in the file); real file paths still show their basename.
Tests: the finder locates "YouSeeBIGGIRL∕T: T.mp3" from the encoded title
"…||YouSeeBIGGIRL/T:T" (the screenshot case), doesn't match an unrelated file,
and a genuine Soulseek path still resolves to its last segment. 21 finder tests
+ 64 script-split integrity tests pass.
- ⚠ Unverified filter rows gain actions: inline play (range-streamed from the
history file path, server-side only), YouTube compare, Approve -> new
human_verified status (tag + history + tracks; AcoustID scanner skips these
entirely), Delete (file + entry)
- API: /api/verification/<id>/stream|approve|delete (path only from DB row)
- backfill: history rows with acoustid_result='fail' that exist at all were
imported despite the failure = force_imported (covers pre-fix fallback
imports like the user's 'My Ordinary Life')
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The pipeline has three success exits (simple download, playlist folder mode,
main) but only the main one persisted the verification status — force-imported
playlist tracks got no tag, no history status, and never appeared in the
Unverified filter. Extracted _persist_verification_status() and call it at
every exit. One-time idempotent backfill derives status for existing history
rows from their recorded acoustid_result (pass->verified, skip->unverified).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- '⚠ Unverified' filter pill on the Downloads page lists completed downloads
whose verification status is unverified/force_imported (review queue)
- the quarantine-retry engine's attempt counter (already tracked internally)
is now surfaced: task.retry_info ('2/5') shows next to Searching/Downloading
in the modal and as 🔁 on the Downloads page rows, with the trigger in the
tooltip
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The persistent Completed list is built from library_history (not live tasks),
so the badge never showed after a session ended. Column added (additive),
written at import, passed through _build_history_download_item, rendered by
_adlVerifBadge next to the status label.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Also: evaluate() treats an empty expected artist as title-only comparison
(old scanner behaviour — a missing DB artist is no evidence of a wrong file),
and the thresholds are now defined once in the core and re-exported.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The library AcoustID scan now calls audio_verification.evaluate() (alias-aware
artist match + cross-script SKIP) instead of its own non-ASCII-stripping
_normalize and threshold logic, so it no longer false-flags correct anime-OST /
kanji tracks. Duration-collision guard kept as a scanner pre-check on the top
recording. evaluate() is now purely a title/artist/version/cross-script decision.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
verify_audio_file now calls audio_verification.evaluate() and re-exports
normalize/similarity/_alias_aware_artist_sim from the core, so import and the
library scan can no longer drift apart. Alias-rescue diagnostic moved to the core.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Background automations had no session, so get_current_profile_id() fell back to
admin (1) — wrong for a non-admin's scheduled job. Now the engine declares the
automation's owner around handler execution via a contextvar
(core/profile_context.py), and get_current_profile_id() consults it only when
there's NO web request. So:
- a real logged-in request always wins (foreground unchanged),
- admin + system automations are profile 1 → resolve to admin exactly as before
(the 8 admin-owned auto-sync pipelines behave identically),
- only non-admin-owned automations gain their correct identity, deep through the
whole call chain (incl. the per-profile client resolvers) — no threading
profile_id through dozens of signatures.
Reset in a finally so a pooled thread can't leak the override to the next job.
Tests: contextvar set/reset/nested; get_current_profile_id honours the override
only outside a request (a real session still wins); and end-to-end — the engine
runs a non-admin automation as profile 4, an admin one as 1, an explicit trigger
profile overrides the owner, and the context resets even when the handler raises.
27 + 4 tests pass.
Part 2 (next): point the sync handlers' source-playlist READ at
get_spotify_client_for_profile so a non-admin's auto-sync pulls THEIR playlist.
Review/regression fix: the shared-app gate I added wrongly required a token
cache even for profiles that set their OWN app creds (legacy), breaking the
per-profile caching tests. Now a per-profile client is built when the profile
has its own app creds OR has connected (its own token cache) — otherwise it
falls back to the global/admin client. Made the fallback-safety tests
order-independent (monkeypatch get_spotify_client to a sentinel) since the real
global client isn't a stable singleton across the suite.
10 metadata-cache + resolution tests pass together.
First service of the per-profile playlist-auth feature. Each profile connects
its OWN Spotify account through the shared (admin's) app, getting its own token;
used for that profile's playlist reads. Admin + unconnected profiles + all
background workers keep using the global/admin client — fully non-regressive.
- Shared-app OAuth: get_spotify_client_for_profile + the /auth/spotify init &
callback now use the GLOBAL app creds (falling back from any legacy per-profile
app creds) with the profile's own token cache, and show_dialog=true forces the
account chooser so a user can't silently inherit the admin's Spotify session.
The builder gates on the profile's own token cache existing — no cache → global.
- My Accounts modal (new, all-profile-accessible via the profile bar): one-click
Connect/Disconnect Spotify + connection status (account name). GET
/api/profiles/me/connections + POST .../spotify/disconnect; admin's Spotify is
read-only here (managed in Settings).
- Wired the request-scoped reads to the per-profile client: the playlist LIST,
the playlist TRACKS view, liked-songs count, and user info — so a connected
user sees and opens THEIR OWN (incl. private) playlists, not the admin's.
Tests: builder falls back to the global client for admin/None/unconnected (the
non-regression guarantee); connections status reports unconnected; admin
disconnect rejected. 124 profile/spotify/gate/integrity tests pass.
Still on the global account (next step): sync/download jobs run in background
workers with no profile context — stamping the requesting profile onto the job
is the remaining wiring. Other services (Tidal/Deezer/Qobuz/Last.fm/ListenBrainz)
follow this same pattern.
Regression from the #832 server-side launch gate (Beckid). On a PIN-required,
unverified session the gate 401'd /api/setup/status — which the frontend checks
BEFORE the PIN screen. The 401 left setup_complete undefined, so `!undefined`
relaunched the full setup wizard on every visit (cancel → PIN screen worked,
which was the tell).
Two-layer fix:
- Allowlist /api/setup/status in the launch gate (it's a harmless boolean, no
secrets) so the frontend gets the real answer even while locked.
- Make the frontend fail-safe: only launch the wizard on a definitive
setup_complete === false from an OK response — never on a 401/locked/ambiguous
one.
Test: locked session still 401s data endpoints but /api/setup/status returns
{setup_complete:true}; added a gate-allowlist assertion. 21 gate tests pass.
Follow-up to the modal fix: the sidebar Service Status + dashboard service card
also mislabeled "Spotify (no auth)" as plain "Spotify". They read the status
`source`, which came straight from metadata.fallback_source ('spotify') with no
awareness of the metadata.spotify_free flag.
- get_primary_source_status now reports a DISPLAY source of 'spotify_free' when
fallback_source='spotify' + metadata.spotify_free is set (the raw 'spotify' is
still used for the auth/connected checks), and treats the free path's
availability as "connected" so the dot isn't falsely red on a no-auth setup.
- getMetadataSourceLabel maps 'spotify_free' → "Spotify (no auth)"; the status
presentation treats spotify_free as part of the Spotify family (session /
rate-limit / cooldown display still work). Added a SOURCE_LABELS entry.
- testDashboardConnection normalizes spotify_free → spotify (the only logic
consumer of the source value — the dashboard Test button).
Routing is unchanged (the real source stays 'spotify' + free flag); this is
purely the display layer. Settings was always correct. 64 integrity tests pass;
the 2 failing soundcloud tests are pre-existing (confirmed identical on a clean
tree).
Discogs masters (/masters/{id}) and releases (/releases/{id}) share one
numeric ID namespace, so release N and master N are different albums.
search_albums() (type=release) yields RELEASE ids, but get_album,
get_album_tracks and _fetch_and_cache_album all tried /masters first and
only fell back to /releases. Whenever a release id collided with a real
master id, the master lookup succeeded and returned a valid-but-wrong
album — the fallback never fired.
Tag the object type into the id string ('r12345' / 'm12345') at the single
parse point (Album.from_discogs_release) and route each fetch to the
matching endpoint. Legacy untagged ids are tried release-first (then master),
which self-heals pre-existing bad matches without a migration. The
enrichment worker now persists the tagged id too.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adversarial line-by-line review of the feature diff turned up:
- /api/database/update/stop was NOT @admin_only while its sibling start_update
was — a non-admin could abort a library scan. Gated.
- /api/metadata-cache/evict was NOT gated while its clear siblings were. Gated.
- validate_credential_payload now treats whitespace-only values as missing, so
a blank-but-spacey secret can't be saved to fail confusingly later.
Tests updated: both endpoints added to the admin-gating matrix; a whitespace-only
validation case added. 42 credential/gating tests pass.
Review also confirmed (no change needed): migration is idempotent + additive +
O(1); encryption round-trips with a non-dict guard; no SQL injection; stale
selections fall back to None safely; no secret ever returned to the browser;
the hybrid-drag index math is correct in both directions; the new resolver is
fully DORMANT (zero runtime callers) so existing client behaviour is untouched;
and @admin_only is a no-op for single-profile installs (default profile = admin).
Groundwork for admin-created, per-profile-switchable credential sets ("pills")
across auth services (Spotify/Tidal/Deezer/Qobuz/Plex/Jellyfin/Navidrome).
Strictly additive and dormant — nothing reads it at runtime yet, so zero
behaviour change for existing installs.
- core/credentials/store.py: pure service registry + payload validation +
stale-safe active-set selection (pick_active_credential falls back to None
when a selected set was deleted, so a profile never breaks).
- migration service_credentials_v1: two new tables — service_credentials
(admin-created named sets; payload Fernet-encrypted at rest) and
profile_service_credentials (each profile's selected set per service).
- MusicDatabase CRUD: create/update/delete/list/get_service_credential
(list never returns the payload; get decrypts for the resolver), plus
set/get_profile_service_credential and resolve_profile_service_credential
(returns the profile's active payload or None → caller uses global default).
Tests: 12 — pure validation + stale-safe selection, and real-temp-DB storage
proving encryption round-trips, payload never lists, dup(service,label)
rejected, per-profile/per-service resolution, and delete clearing dangling
selections to a clean fallback. 95 migration/DB tests still pass.
the-hang-man: tracks with an apostrophe (e.g. "I'm Upset") deleted the DB row
but left the file. The library DB stored the title with U+2019 (the curly form
Spotify/Apple metadata uses) while the file was written to disk with U+0027
(ASCII). _resolve_library_file_path compared the curly path byte-for-byte via
os.path.exists, missed every time, and reported "could not be deleted".
Fix: resolve confusable-tolerantly. New core/library/path_resolve.find_on_disk
descends the path component by component, taking an exact match when present and
otherwise folding a small set of typographic look-alikes (curly vs straight
quotes, en/em dash, ellipsis, nbsp) for the comparison ONLY — it never renames,
just finds the file that's actually there. Exact matches always win per
component, so paths that already resolved are byte-for-byte unaffected. This
also fixes existing mismatched files (no re-import) and every caller of
_resolve_library_file_path (sidecar cleanup, dead-file checks, streaming), not
just delete.
Case is deliberately NOT folded: a case-sensitive dataset (ext4/ZFS) can hold
names differing only by case, and folding could resolve the wrong file. The
reported failure is purely typographic.
Tests: real temp-file fixtures exercising the actual byte mismatch — curly-DB →
ascii-disk resolves, exact still works, confusable in a folder component, exact
wins when both encodings present, genuinely-different name does NOT collide,
missing file → None. 10 new tests; 949 resolver-adjacent tests pass.
Beckid: the admin launch PIN was a CLIENT-SIDE overlay only. `launch_pin_required`
just told the frontend to draw a fixed div over the app — removing it (Safari
"Hide Distracting Items", devtools, or any non-browser client like curl) gave
full unauthenticated access to every /api/* endpoint, because the server never
checked it. Anyone who reverse-proxies SoulSync publicly was wide open.
Fix: a before_request gate (_enforce_launch_pin) that rejects every request from
an unverified session while security.require_pin_on_launch is on. The decision
is a pure, unit-tested helper (core/security/launch_lock.request_is_locked) so
the allow/deny matrix can't silently regress. Allowed while locked: the page
shell + static assets, the unlock flow (current/list/select/verify/reset/logout),
and the public REST API /api/v1/ (its own @require_api_key governs it) — EXCEPT
/api/v1/api-keys-internal*, the "no auth required" key-management endpoints,
which stay locked so an attacker can't mint an API key and walk in the side door.
Everything else (data, settings, profile create/edit/delete/set-pin, socket.io)
is blocked.
A blocked top-level browser navigation (deep link / refresh on a sub-page like
/dashboard) is redirected to the root lock screen instead of dumping raw JSON —
detected via Sec-Fetch-Mode: navigate / Accept: text/html (is_html_navigation).
Programmatic fetch/XHR still get the JSON 401 so the frontend can react.
Also fixed the verified flag: get_current_profile POPPED launch_pin_verified
(one page load), but an enforced gate needs it to persist — now READ, so
verification lasts the session (until logout/expiry). No-ops entirely when
require_pin_on_launch is off (default).
Tests: full allow/deny matrix + navigation detection. 20 gate tests + 232
profile/security tests pass.
Tacobell444 (#707 follow-up): the scan summary said "New tracks: 19 • Added to
wishlist: 10" with no way to see which tracks those were — you had to scan your
wishlist and guess what was new.
Scan ledger: the scanner now records a per-run scan_track_events list (track,
artist, album, thumb, status added|skipped — skipped = found-new but declined
by add_to_wishlist: already queued or blocklisted; capped at 500). The status
endpoint already serializes scan_state, so the payload flows free. The
completed (and cancelled) scan summary on the Watchlist page gets a
"Show tracks" toggle expanding a styled list — Added section + Skipped section
with badges, reusing the live-feed row styling.
Download Origins grouping: the modal now groups entries by what triggered them
(watchlist artist / playlist name) with collapsible headers + counts instead of
a flat list with a per-row badge. Entries arrive newest-first so groups order
themselves by their newest download. Same row markup, checkboxes/delete intact.
Provenance: watchlist adds now stamp scan_run_id into wishlist source_info, so
per-run grouping is queryable later (future "what did run X add" views).
Tests: per-run ledger seam test (added + skipped statuses, album/artist fields,
FIFO unchanged). 316 watchlist/wishlist tests pass; JS syntax-checked.
Third round of the multi-artist report. The earlier fixes (Deezer contributors
upgrade, _artists_list, feat_in_title/artist_separator) were all in place and
correct — but gated on source == 'deezer', and on the real Search → Download
Now path NOTHING carried the source: core/search/sources.py serialized tracks
with no source field, search.js's enrichedTrack didn't add one, so
get_import_source() resolved '' and the whole Deezer-specific block silently
skipped. Files were tagged with only the primary artist until a Retag (which
rebuilds context with the source set — exactly why retagging always fixed it).
The earlier tests passed because they set context['source'] directly — the one
field the real flow never had (same mock-drift as the #823 append tests).
Reproduced with Netti93's exact track (deezer 3966840171) through the real
extract_source_metadata: before — source '', artists ['August Burns Red'];
after — source 'deezer', contributors fetched, artists ['August Burns Red',
'Polaris'], title 'Sonic Salvation (feat. Polaris)' per feat_in_title.
Fix, three layers:
- core/search/sources.py: serialized tracks/albums/artists carry "source"
(the canonical name the orchestrator already passes; '' when unnamed).
- core/imports/context.py get_import_source: also reads '_source' from the
nested dicts (track_info/original_search/album/artist) — additionally fixes
the discography/wishlist flows, which always passed '_source' that nothing
read.
- search.js: enrichedTrack + the album-download path carry source through to
the download task.
Tests: real-payload staging-shaped contexts (source in track_info, '_source'
shape, and the pre-fix sourceless shape staying safe — mocked Deezer client),
serializer source-field tests, resolver fallback tests; exact-shape serializer
tests updated for the new key. 1977 import/metadata/search tests pass (the
only 2 failures are the known soundcloud ones).
carlosjfcasero round 2: after 6fa956d6 append stopped recreating the playlist,
but every sync re-appended ALL matched tracks — every track N times. His log
shows it plainly: "added 22 new tracks to 'Disney' (skipped 0 already present)"
on a playlist that already had those 22.
Root cause: the dedupe read `{t.id for t in get_playlist_tracks(...)}` — but
JellyfinTrack only defines `ratingKey`, never `id`, so the existing-ids set was
ALWAYS empty and everything looked new. NavidromeTrack is also ratingKey-only,
so the Navidrome append had the identical bug. Plex (plexapi ratingKey) was
fine. The existing tests were green because they mocked existing tracks as
SimpleNamespace(id=...) — encoding the same wrong assumption as the code.
Fix:
- New pure planner plan_playlist_append(current, desired) in
core/sync/playlist_edit.py (next to the reconcile planner): order-preserving,
drops already-present ids, dedupes within desired, stringifies (Emby numeric
vs string safe).
- Jellyfin/Emby: existing ids fetched from the canonical
/Playlists/{id}/Items endpoint (same as reconcile — works for Jellyfin GUIDs
and Emby numeric ids), ratingKey fallback if that request fails.
- Navidrome: dedupe on ratingKey (the attribute that actually exists).
Tests: planner (skip-present incl. the reporter's unchanged-playlist case,
desired-order, dupes-within-desired, int/str ids, empties) + the append-mode
suite rewritten to pin the REAL shapes (raw Items dicts for Jellyfin,
ratingKey objects for Navidrome) + a new fallback-path test. 524
playlist/sync/jellyfin/navidrome tests pass.
