Commit graph

755 commits

Author SHA1 Message Date
BoulderBadgeDad
826ac0b366 #853 follow-up: don't cache a partial Deezer discography on mid-pagination error
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.
2026-06-11 15:35:36 -07:00
BoulderBadgeDad
814af2cdfb
Merge pull request #853 from ramonskie/artist-discography-cache
Cache artist album lists across metadata sources
2026-06-11 15:31:04 -07:00
BoulderBadgeDad
53c264ab50 Torrents: fix stall handling on "downloading metadata" + stop orphaning in qbit
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.
2026-06-11 14:37:46 -07:00
ramonskie
76d3e25fd4 Cache artist album lists across metadata sources 2026-06-11 22:33:04 +02:00
BoulderBadgeDad
46eccbb237 #852: gate the WebSocket handshake — close the launch-PIN/login bypass
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.
2026-06-11 13:27:03 -07:00
BoulderBadgeDad
87e5e1fa23 #702: make mirrored-playlist cancel/reset/delete idempotent (un-wedge LB weekly sync)
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.
2026-06-11 12:55:55 -07:00
BoulderBadgeDad
9bf7881f7a #704: add "Relocate" fix for AcoustID mismatches — retag + restage for re-import
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.
2026-06-11 12:01:05 -07:00
BoulderBadgeDad
174adf2dc9 #845 tests: cover the verification_status migration backfill
Locks the one-time backfill that derives verification_status for pre-column
library_history rows from acoustid_result: pass->verified, skip->unverified,
fail->force_imported; never overwrites an existing status (NULL-only); leaves
no-acoustid rows NULL; idempotent across repeated inits. Exercises the real
_initialize_database path (clears the per-process init guard to re-trigger it).
2026-06-11 11:17:09 -07:00
BoulderBadgeDad
a207bd943b #845 tests: lift history-path resolver to core/ + seam-test the delete-safety
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).
2026-06-11 11:07:25 -07:00
BoulderBadgeDad
89f843d223 #851: normalize '/' ':' '_' to spaces so slash-titles match underscore sources
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.
2026-06-11 10:46:36 -07:00
BoulderBadgeDad
17440329c1 #845 follow-up: admin-gate the mutating verification-review endpoints
The merged PR left the review-queue's mutating endpoints ungated. Both now require
admin, matching the Phase 3 destructive-endpoint convention:

- /api/verification/<id>/delete (os.remove + drops the history row) — @admin_only,
  so a non-admin on a login/multi-profile instance can't delete library files.
- /api/verification/<id>/approve (flips verification_status + writes the tag) —
  @admin_only; also wrapped its DB writes in `with db._get_connection()` for
  rollback-on-error + codebase consistency (was a bare conn).

Read/playback endpoints (stream/play/compare/entry/config) stay open — the app's
LAN-read model. Tests: non-admin gets 403 on delete + approve; admin isn't blocked.
2026-06-11 10:43:40 -07:00
BoulderBadgeDad
eb35ba86fb
Merge pull request #845 from nick2000713/fix/import-folder-artist-override-optin
feat: import folder-artist override opt-in + verification pipeline review queue
2026-06-11 10:39:59 -07:00
nick2000713
bf5affd03c resolve merge conflict in style.css 2026-06-11 18:21:04 +02:00
BoulderBadgeDad
3284af428d Discogs (#848 follow-up): tag collection album IDs for consistency
The Your Albums Discogs collection sync stored bare release_ids while
search/discography now store tagged ('r<id>') ones (#848). This didn't cause a live
bug — the pool dedups by normalized name, and discogs_release_id is only ever
re-fetched (which handles bare via release-first) — but it left the "type travels
with the ID" invariant half-applied. Now the collection sync tags its IDs too, so
every stored Discogs album ID is uniform and a future ID comparison can't be tripped
by mixed forms.

Collection items are always releases, so they're tagged 'r<id>'. Test locks the
stored value + that a tagged collection ID routes only to /releases (never /masters).
2026-06-11 08:33:26 -07:00
rollingbase
d6d5ef22c9 Merge branch 'main' into fix/discogs-master-release-id-collision 2026-06-11 11:12:34 +02:00
BoulderBadgeDad
613688a9ad Login recovery (DB + backend): security question to reset a forgotten password
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.
2026-06-10 22:24:54 -07:00
BoulderBadgeDad
9e40f5c12d tests: pin/login mode isolation — PIN gate unaffected when login off; both-off = unguarded
Pins the zero-impact guarantee: with require_login off (default), the launch PIN
still enforces exactly as before (the login deferral doesn't fire), and with both
off there's no gate at all (today's behavior).
2026-06-10 22:19:29 -07:00
BoulderBadgeDad
21dfbb39b0 Native login (increment 3/3): login screen, set-password, Settings toggle, logout
The UI that makes opt-in login usable. Off by default → your LAN setup is unchanged
(none of this appears unless security.require_login is on).

- Login screen overlay (reuses the launch-PIN styling): username + password →
  /api/auth/login → reload into the app. Shown when /api/profiles/current reports
  login_required (checked before profile selection).
- POST /api/profiles/<id>/set-password (admin, or self) to set/clear a login
  password, distinct from the PIN.
- Settings → Security: "Login password (admin account)" field + a "Require login"
  toggle (with the anti-lockout note). Wired into the existing settings load/save.
- Sign-out button in the profile bar, revealed only in login mode (login_mode flag
  on /api/profiles/current); soulsyncLogout() → /api/auth/logout → reload.

Tests: set-password sets/clears + verifies; /api/profiles/current signals
login_required. 20 login/password tests pass; 64 script-split integrity pass.

Remaining (small follow-up): a password field in the Manage Profiles edit form so
admins can set OTHER profiles' passwords from the UI (the endpoint already exists).
2026-06-10 22:10:33 -07:00
BoulderBadgeDad
92cbef90f9 Native login (increment 2/3): login/logout endpoints + require_login gate
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).
2026-06-10 22:01:53 -07:00
BoulderBadgeDad
8e1b678d6f Native login (increment 1/3): per-profile password DB layer
Opt-in username/password login — profiles become real accounts. This is the data
layer: a per-profile login password, kept SEPARATE from the quick-switch PIN
(different security purpose; a 4-digit PIN must not become the password guarding a
public instance).

- Additive migration: profiles.password_hash column (idempotent, metadata-flagged).
- set_profile_password / verify_profile_password / profile_has_password /
  get_profile_by_name (the login username = profile name, unique + case-insensitive).
- Security default: a profile with NO password is NOT loginable (verify returns
  False) — unlike the PIN where "no PIN = always valid". You can't authenticate to
  an account with no credential.

Tests: migration adds the column; set/verify; no-password-never-loginable; clearing;
name lookup; and password is fully independent of the PIN. 6 tests pass. Next:
the login endpoint + require_login gate (increment 2).
2026-06-10 21:57:44 -07:00
BoulderBadgeDad
86d0a0dd62 Security: trust a forward-auth proxy user header (Tier 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.
2026-06-10 20:57:48 -07:00
BoulderBadgeDad
5e5bc12e45 Security: add gated security headers in reverse-proxy mode (Tier 2)
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.
2026-06-10 20:48:51 -07:00
BoulderBadgeDad
0d1e949798 Security: brute-force limiter on the launch-PIN unlock (Tier 2)
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.
2026-06-10 20:47:46 -07:00
BoulderBadgeDad
aa3aae695d Security: opt-in reverse-proxy mode (ProxyFix + Secure cookie) + nginx guide
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.
2026-06-10 20:36:49 -07:00
BoulderBadgeDad
d82d02b921 Artist Sync: unify with deep scan — server-diff stale removal, scoped to one artist
Per the original intent, "Sync" is now a single-artist deep scan: it uses the SAME
reconciliation source as the whole-library deep scan instead of a separate
disk-existence check.

- Phase 1 already calls the deep-scan worker's _process_artist_with_content; now it
  passes seen_track_ids so the pull collects the server's current track IDs for the
  artist (existing + new), exactly as the library deep scan does.
- Phase 2 stale = (artist's DB tracks for this server) − seen, then
  delete_stale_tracks(server_source) — identical mechanism to deep scan, scoped to
  one artist. The old os.path.exists disk check (which could mass-delete on an
  unreachable mount) is gone.
- Removal only runs when the server pull SUCCEEDED — no trustworthy 'seen' set
  (no server, unreachable, or a failed pull) → skip, never delete. The
  is_implausible_stale_removal guard (>50% unseen) stays as the same safety net
  deep scan has for a flaky response. @admin_only retained.

Tests rewritten for the server-diff model: removes only tracks the server no longer
has; guard skips when most are unseen; a failed pull skips removal entirely;
admin-only. 8 tests pass.
2026-06-10 19:43:25 -07:00
BoulderBadgeDad
4d1b9a5639 Artist Sync: guard stale-removal against an unreachable mount + gate it admin-only
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.
2026-06-10 19:33:44 -07:00
BoulderBadgeDad
5bc27e6268 #843 follow-up: key the no-state cache save by the FIRST artist
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.
2026-06-10 18:57:43 -07:00
BoulderBadgeDad
0afa3c9705 Fix: "Discovery state not found" when fixing a match after restart/import (#843)
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.
2026-06-10 18:49:32 -07:00
BoulderBadgeDad
a15fe15842 Fix: auto-sync capped public Spotify playlists at 100 tracks (#838)
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.
2026-06-10 18:31:43 -07:00
BoulderBadgeDad
56bdcee434 Fix: rejected slskd download hung the task at 'downloading' forever (#836)
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.
2026-06-10 18:16:22 -07:00
BoulderBadgeDad
27d738e7b1 Fix: Find & Add library search buried exact matches (case-sensitive ordering)
Reported via Find & Add (Billie Eilish "bad guy"): the track was in the library
and on Plex, but never showed in the modal's 20 results. Root cause (proven
against the real 307k-track DB): the search did `ORDER BY tracks.title`, which is
case-SENSITIVE in SQLite (BINARY collation sorts 'B' before 'b'). Billie's title
is lowercase "bad guy"; everyone else's is "Bad Guy", so all the capitalised ones
sorted first, filled the LIMIT, and her exact match landed at ~#25 — cut off.

- search_tracks now ranks by relevance: exact title match first (case-insensitive
  via unidecode_lower), then prefix, then alphabetical — so an exact match can't
  be sorted below the limit by a capital letter. Helps every caller.
- Added a rank-only `rank_artist` hint (never filters): Find & Add already knows
  the source track's artist, so it now passes it and the exact title+artist match
  floats to #1. Filtering was deliberately avoided — if the track is tagged under
  a slightly different artist on the server, a filter would re-hide it.

Verified on the real DB: title-only "bad guy" now surfaces Billie at #4 (was
>#20); with the artist hint she's #1. Seam tests: lowercase exact title isn't
buried; rank hint floats the match without filtering; exact title beats a
superstring title. 10 tests pass.
2026-06-10 17:23:13 -07:00
BoulderBadgeDad
1517794e23 Fix: manual Find & Add recreated the Jellyfin/Emby playlist (#837)
Automations + auto-sync respect 'append' mode and preserve a server playlist's
description + cover image, but manually matching a missing track ("Find & add")
recreated the whole playlist and wiped them.

Root cause: the add-track endpoint's Jellyfin branch called
`update_playlist(<entire track list>)`, which deletes + recreates the playlist on
Jellyfin/Emby. Switched it to the purpose-built `append_to_playlist([the one
found track])` — the same in-place, dedupe-safe op the 'append' sync mode already
uses — so the playlist (and its description/image) is preserved and only the
missing track is added. append_to_playlist reads `.id` off the track, so the
endpoint now sets it (it previously only set ratingKey).

Plex (in-place addItems) and Navidrome (in-place Subsonic updatePlaylist) were
already non-destructive; Emby routes through the jellyfin branch, so this covers
it too.

Tests: the add-track endpoint appends in place and never calls update_playlist;
a link-to-existing-track touches nothing. 18 tests pass (incl. the existing
append-mode suite).
2026-06-10 16:55:08 -07:00
BoulderBadgeDad
b36392c62b Fix: a '/' in a song title was treated as a path separator (#835)
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.
2026-06-10 16:29:09 -07:00
dev
37ea6604c7 Fix import artist override and verification review 2026-06-11 01:28:31 +02:00
dev
97b40cbd43 feat(verification): review queue — listen/compare/approve/delete unverified downloads
- ⚠ 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>
2026-06-11 01:28:31 +02:00
dev
9d1d09a571 feat(verification): persist status (db+tag), surface on Downloads, scan-aware force-imports
- import pipeline writes SOULSYNC_VERIFICATION tag + context status
  (verified / unverified / force_imported via version-mismatch fallback)
- downloads payload + UI badge (tooltip explains each state)
- AcoustID scan reads the tag: refreshes tracks.verification_status,
  reports force-imported mismatches as informational (clearly marked),
  optional skip via job setting skip_force_imported
- evaluate(): empty expected artist = title-only comparison (old scanner
  behaviour); thresholds single-sourced in the core

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-11 01:28:31 +02:00
dev
8e6820dbdf feat(verification): status vocabulary, DB column, SOULSYNC_VERIFICATION tag
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>
2026-06-11 01:27:29 +02:00
dev
b981230d07 refactor(scanner): use shared verification core; stop false-flagging cross-script
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>
2026-06-11 01:27:28 +02:00
dev
967ad2a026 refactor(verification): import path delegates to shared core
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>
2026-06-11 01:27:28 +02:00
dev
d989f25220 feat(verification): shared evaluate() PASS/SKIP/FAIL decision core
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-11 01:27:28 +02:00
dev
cf0a17c14a feat(verification): shared normalize() core for import + scan
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-11 01:27:28 +02:00
BoulderBadgeDad
c3ff333934 tests: make the spotify source-adapter test order-independent
assert against a monkeypatched get_spotify_client sentinel instead of
web_server.spotify_client (which isn't a stable singleton across the suite) — same
fix already used for the per-profile builder tests.
2026-06-10 15:55:41 -07:00
BoulderBadgeDad
783839349c Automations: per-profile playlist source reads in auto-sync (part 2)
The playlist source registry built the Spotify/Tidal adapters with a client
GETTER (resolved fresh on every read), but web_server passed `lambda: <global
client>`. Swapped those to get_spotify_client_for_profile /
get_tidal_client_for_profile.

Combined with part 1 (the engine running each automation as its owner), an
auto-sync pipeline now reads its source playlist through the OWNER's account:
- interactive sync → the user's session profile,
- background automation → the automation owner (via core.profile_context),
- admin / profile 1 → the global client, so the admin's existing auto-sync
  pipelines pull exactly as before.

The adapters re-resolve per read, so a singleton registry is fine. Deezer/Qobuz
getters left global (their playlist login is tangled with downloads — deferred).

Tests: the Spotify/Tidal source adapters resolve the global client under admin
and re-resolve through the profile context per call (unconnected → safe global
fallback). 27 endpoint/profile tests pass.
2026-06-10 15:45:45 -07:00
BoulderBadgeDad
6980253a96 Automations: run each as its OWNER profile in the background (part 1 of per-profile sync)
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.
2026-06-10 15:37:56 -07:00
BoulderBadgeDad
35ffc9f5e2 tests: use soulsync-testdb- prefix in the web_server endpoint tests
My credential/admin-gating endpoint tests override DATABASE_PATH at import to a
temp dir, but with a custom prefix — which tripped the DB-isolation guard
(test_database_path_env_is_isolated requires 'soulsync-testdb-' in the path) once
those modules loaded. Still a temp DB (no real-DB risk), just the wrong prefix.
Switched to the soulsync-testdb- prefix so isolation is preserved and the guard
passes.
2026-06-10 14:27:35 -07:00
BoulderBadgeDad
af1a35385c Profiles: ListenBrainz in My Accounts; Personal Settings now just server library
Third service (the easy one — ListenBrainz already had a working per-profile
token path). Consolidated all per-profile streaming accounts into the My Accounts
modal:
- My Accounts gains a ListenBrainz row with a token-paste connect (a new 'token'
  service type alongside the OAuth-popup ones), reusing the existing
  /api/profiles/me/listenbrainz save + the generic disconnect.
- Connections API reports listenbrainz status (connected + username).
- Personal Settings (the gear modal) dropped its Spotify/Tidal/ListenBrainz
  sections — those duplicated My Accounts — and now shows only the per-profile
  server-library selection (non-admin) or a pointer note (admin). The old
  renderPersonalSettings{Spotify,Tidal,LB} functions are left defined but unused.

So every per-profile account connection (Spotify, Tidal, ListenBrainz) now lives
in one place. Tests: LB connect status + disconnect via the generic endpoint.
23 endpoint tests pass; 64 integrity tests pass.
2026-06-10 13:32:29 -07:00
BoulderBadgeDad
60b9fe10e9 Profiles: per-profile Tidal self-auth (playlists) — with a safe token-save redirect
Second service. Each profile connects its own Tidal; its playlist reads use that
account, everything else stays global. The gotcha vs Spotify: TidalClient loads
AND saves tokens to one global slot (tidal_tokens), so a naive per-profile client
would clobber the admin's tokens on refresh.

- get_tidal_client_for_profile builds a dedicated TidalClient seeded with the
  profile's tokens, refreshed via the shared/global app creds, and OVERRIDES its
  _save_tokens to persist to the PROFILE row — never the global slot. Admin
  (profile 1) + unconnected profiles use the global client unchanged. Cached per
  profile + evicted on (dis)connect.
- DB: set_profile_tidal_tokens / get_profile_tidal (encrypted); the OAuth callback
  now uses them + evicts the cached client.
- Wired the Tidal playlist reads (list + tracks) to the per-profile client; the
  module import line left intact.
- My Accounts: Tidal row (Connect via /auth/tidal?profile_id=, status, Disconnect).
  Connections API extended; disconnect made generic (/<service>/disconnect).
  Admin sees "managed in Settings" for every service.

Tests: per-profile token refresh writes to the profile and leaves the global
tidal_tokens untouched (the safety guarantee); connect status + disconnect;
admin/unconnected → global client. 22 endpoint tests pass.
2026-06-10 13:11:59 -07:00
BoulderBadgeDad
7eaed1d533 Profiles: per-profile Spotify builds for own-app creds OR a token cache
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.
2026-06-10 12:28:03 -07:00
BoulderBadgeDad
e8bd9c8018 Profiles: per-profile Spotify self-auth (shared app) + My Accounts modal + read wiring
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.
2026-06-10 12:21:17 -07:00
BoulderBadgeDad
c154aa5442 Profiles: remove the admin Connected Accounts manager (pivot to pure self-auth)
The model shifted from "admin creates shared credential sets, users pick" to
"each profile self-auths its own playlist accounts". Removed the admin-facing
Connected Accounts manager: the Settings section, credential-sets.js, its CSS,
the script tag + integrity-registry entry, and the loadSettingsData hook.

The credential-sets backend (service_credentials tables + /api/credentials and
/api/profiles/me/services endpoints) is left in place but dormant — additive,
tested, harmless — rather than churn migrations that already ran on installs.
Per-profile self-auth reuses the existing per-profile columns + the
get_*_for_profile client pattern instead. The Service Status modal (admin-only)
is unaffected.
2026-06-10 11:48:31 -07:00