From 16072e3a6043be604c05efd561c323fee97e3175 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 19 Aug 2026 02:41:11 +0200 Subject: [PATCH 01/18] fix(media): answer a browser's opening range on a cold transcode A cold transcode refused every Range header, so a browser audio element - which opens each resource with `Range: bytes=0-` - was answered 416 on the first play of a track and 206 on the second, once the cache existed. Feishin therefore failed on the tracks nobody had transcoded yet, and played the ones an earlier client had already warmed. An unbounded range from zero is not a seek, so it is served as the whole resource. A range that names an end or starts elsewhere keeps its refusal: it still has no meaning before the transcode exists, and seeking a live transcode goes through offsetMs. Signed-off-by: InstaZDLL --- src/media.rs | 31 +++++++++++++++++++++++++---- tests/v2_foundations.rs | 43 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 4 deletions(-) diff --git a/src/media.rs b/src/media.rs index 2298534..10ca8cc 100644 --- a/src/media.rs +++ b/src/media.rs @@ -174,9 +174,12 @@ impl MediaService { return serve_file(&cache_path, range, mime_for_format(query.format)).await; } - // A byte range has no stable meaning until a transcode is complete. - // Clients seek a live transcode with offset_ms instead. - if range.is_some() { + // A byte range has no stable meaning until a transcode is complete, so + // seeking a live transcode goes through offset_ms instead. `bytes=0-` + // is not a seek: it is what a browser audio element sends to open any + // resource, and refusing it made every web client fail on the first + // play of a track and succeed on the second, once the cache existed. + if range.is_some_and(|value| !opens_whole_resource(value)) { return Err(MediaError::RangeNotSatisfiable(0)); } @@ -849,6 +852,14 @@ async fn serve_partial( Ok(response) } +/// Whether a `Range` header asks for the resource from its first byte with no +/// end, which is how browsers open an audio element rather than how they seek. +fn opens_whole_resource(value: &str) -> bool { + value + .strip_prefix("bytes=") + .is_some_and(|spec| spec.trim() == "0-") +} + fn parse_range(value: &str, size: u64) -> Option<(u64, u64)> { if size == 0 { return None; @@ -989,7 +1000,7 @@ mod tests { use dashmap::DashMap; use tokio::sync::Semaphore; - use super::{parse_range, prune_cache, secure_track_path, MediaInner}; + use super::{opens_whole_resource, parse_range, prune_cache, secure_track_path, MediaInner}; #[test] fn ranges_are_bounded_and_validated() { @@ -1001,6 +1012,18 @@ mod tests { assert_eq!(parse_range("bytes=0-1,4-5", 10), None); } + #[test] + fn only_an_unbounded_range_from_zero_opens_a_resource() { + assert!(opens_whole_resource("bytes=0-")); + assert!(opens_whole_resource("bytes= 0- ")); + // Anything that names an end, or starts elsewhere, is a seek. + assert!(!opens_whole_resource("bytes=0-1023")); + assert!(!opens_whole_resource("bytes=1-")); + assert!(!opens_whole_resource("bytes=-3")); + assert!(!opens_whole_resource("bytes=0-,2-3")); + assert!(!opens_whole_resource("items=0-")); + } + #[tokio::test] async fn media_paths_reject_parent_components_and_symlinks() { let temp = tempfile::tempdir().unwrap(); diff --git a/tests/v2_foundations.rs b/tests/v2_foundations.rs index a4714d1..468ace4 100644 --- a/tests/v2_foundations.rs +++ b/tests/v2_foundations.rs @@ -1514,6 +1514,49 @@ async fn media_streaming_ranges_transcodes_caches_and_isolates_tenants() { "duplicate consumers must create only one file per cache key" ); + // A browser audio element opens every resource with `Range: bytes=0-`, so + // a cold transcode must answer it rather than refuse the range. Refusing + // made a web client fail on the first play of a track and succeed on the + // second, once the cache existed. + let cold_open = router + .clone() + .oneshot( + Request::get(format!("{uri}?format=mp3&bitrate=128")) + .header("authorization", format!("Bearer {owner_token}")) + .header("range", "bytes=0-") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(cold_open.status(), StatusCode::OK); + assert_eq!(cold_open.headers()["content-type"], "audio/mpeg"); + assert!( + cold_open + .into_body() + .collect() + .await + .unwrap() + .to_bytes() + .len() + > 100 + ); + + // A range that actually seeks still has no meaning before the transcode + // exists, and keeps its refusal. + let cold_seek = router + .clone() + .oneshot( + Request::get(format!("{uri}?format=mp3&bitrate=144")) + .header("authorization", format!("Bearer {owner_token}")) + .header("range", "bytes=64-") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(cold_seek.status(), StatusCode::RANGE_NOT_SATISFIABLE); + let live_seek = router .clone() .oneshot( From dd1336e15631603c9e89c66d21c2ed0168ccf740 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 19 Aug 2026 02:41:11 +0200 Subject: [PATCH 02/18] docs(subsonic): record the Symfonium re-run against the current contract Signed-off-by: InstaZDLL --- docs/subsonic-compatibility.md | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/docs/subsonic-compatibility.md b/docs/subsonic-compatibility.md index c1c1f1b..b3b07c5 100644 --- a/docs/subsonic-compatibility.md +++ b/docs/subsonic-compatibility.md @@ -4,20 +4,21 @@ Automated protocol coverage is enforced by `tests/v2_foundations.rs` for XML, JS | Client | Version | Login | Browse/search | Native/transcode | Playlists/user data | Status | |---|---:|---:|---:|---:|---:|---| -| Symfonium | 14.1.0 | pass | pass | pass | pass | Validated 2026-08-09 on Android 17 through an ephemeral `cloudflared` HTTPS tunnel: account and API-key authentication, full catalogue sync, native MP3/FLAC Range playback, Opus at 64 kbit/s, album favorite, scrobbles and playlist create/update. | +| Symfonium | 14.1.0 | pass | pass | pass | pass | Re-run 2026-08-19 against the current contract, on an Android 17 emulator through an ephemeral `cloudflared` HTTPS tunnel. Every observable change of the six batches was checked against server state rather than against the client's own display: sleeve order in both the album and a synced playlist, two distinct album artists with no composite entity in the artist list, two album genres, a genre spelled four ways answering as one (4 tracks), the extended fields including `explicitStatus`, and a failed login decoded correctly now that it arrives as HTTP 200. User data round-tripped: track and album ratings, track/album/artist favorites, 21 scrobbles, a bookmark resumed at 37 s, three playlists owned by the authenticated user, and 13 Opus cache entries across the 64 and 128 kbit/s ceilings. The earlier 2026-08-09 run on a physical Android 17 device additionally covered API-key authentication, which this one did not re-run. | | Feishin | 1.15.1 | pass | pass | pass | pass | Validated 2026-08-02: native playback, Opus cache, playlist create/add, favorite, rating, scrobble and queue. | | Substreamer | 8.0.91 | pass | pass | pass | pass* | Validated 2026-08-02 from the official release source: native playback, Opus/128 cache, playlist add, favorite, rating and scrobble. `*` Its playback queue is local-only and the client never calls `getPlayQueue`/`savePlayQueue`; those endpoints remain covered by fixtures and Feishin. | | DSub | 5.5.3 (F-Droid 208) | pass | pass | pass | pass* | Validated 2026-08-02 on Android 14, then revalidated 2026-08-15 on the current Android environment: authentication, catalogue, artwork, native playback, seek and playlist. `*` Scrobbling was disabled in the original run; the endpoint remains covered by fixtures and other clients. | | Juliet | iOS build tested 2026-08-15 | pass | pass | native pass; transcode not run | not run | Current iOS compatibility check: authentication, catalogue, artwork and native playback succeeded. Unsupported/unexercised surfaces are not inferred as passes. | -**Every row above predates the move to HTTP 200 on every `/rest` answer.** Each -records a real run against the previous status behaviour and is kept as a -historical result; none of them validates the current Subsonic contract. The M3 -real-client gate was closed on that earlier behaviour, so it is not a satisfied -gate for the next tag: every client must be re-run against the current contract -and its row re-dated first. Missing client features stay covered by automated -fixtures and another real client rather than inferred as passes. Creating any tag -or release remains a separate action requiring an explicit operator request. +**Symfonium is the only row re-run against the current contract.** The four +others predate the move to HTTP 200 on every `/rest` answer: each records a real +run against the previous status behaviour and is kept as a historical result, but +none of them validates what the server answers today. The M3 real-client gate was +closed on that earlier behaviour, so it is not a satisfied gate for the next tag: +the remaining clients must be re-run against the current contract and their rows +re-dated first. Missing client features stay covered by automated fixtures and +another real client rather than inferred as passes. Creating any tag or release +remains a separate action requiring an explicit operator request. The Substreamer row records the successful 2026-08-02 run. It could not be reinstalled on the current Android 17 device during the 2026-08-15 revalidation From 66af38312844e7ecef54d46eab1096aa2f33149d Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 19 Aug 2026 02:58:30 +0200 Subject: [PATCH 03/18] fix(subsonic): createPlaylist replaces the track list it is given Given a playlistId, createPlaylist's songId values name every song of the playlist, not songs to append. Treating them as additions meant a client that removed a song sent back what remained and saw nothing change, and a reorder did nothing at all - Feishin edits a playlist that way and its edits looked lost. PlaylistClear gains a tracks flag, so the replacement runs through the same service method as every other playlist mutation. updatePlaylist is untouched: its songIdToAdd and songIndexToRemove stay incremental, which is what that method means. The native surface does not expose the flag. Signed-off-by: InstaZDLL --- src/services.rs | 12 +++++++++++- src/subsonic.rs | 12 ++++++++++-- tests/v2_foundations.rs | 26 ++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/src/services.rs b/src/services.rs index cc8a38d..f87cc2a 100644 --- a/src/services.rs +++ b/src/services.rs @@ -503,6 +503,11 @@ pub struct ShareClear { #[derive(Debug, Clone, Copy, Default)] pub struct PlaylistClear { pub comment: bool, + /// Drop the existing track list before applying `add`, which turns an + /// update into a replacement. Subsonic's `createPlaylist` needs it: given a + /// `playlistId`, its `songId` values are the whole playlist rather than + /// additions to it. The native surface does not expose it. + pub tracks: bool, } #[derive(Debug, Clone)] @@ -1950,6 +1955,7 @@ impl DomainServices { "add": add, "remove_indexes": &removes, "clear_comment": clear.comment, + "clear_tracks": clear.tracks, }), ); let _writer = self.db.writer_guard().await; @@ -1969,7 +1975,11 @@ impl DomainServices { validate_name(name)?; } self.songs_by_ids_on(&mut tx, user_id, add).await?; - let mut ids = self.playlist_track_ids_on(&mut tx, user_id, id).await?; + let mut ids = if clear.tracks { + Vec::new() + } else { + self.playlist_track_ids_on(&mut tx, user_id, id).await? + }; for index in removes { if index >= ids.len() { return Err(ServiceError::Invalid); diff --git a/src/subsonic.rs b/src/subsonic.rs index 2d6a707..7444025 100644 --- a/src/subsonic.rs +++ b/src/subsonic.rs @@ -1143,8 +1143,13 @@ async fn create_playlist( let playlist = if let Some(id) = params.uuid_optional("playlistId")? { state .services + // Given a playlistId, songId names every song of the playlist, so + // the call replaces the track list rather than adding to it. A + // client that removes a song sends back what remains, and would + // otherwise see nothing change. + // // The Subsonic contract is frozen: it has no way to ask for a - // field to be blanked, so clearing stays off on this surface. + // text field to be blanked, so clearing the comment stays off. .update_playlist( principal.id, id, @@ -1153,7 +1158,10 @@ async fn create_playlist( None, &ids, &[], - Default::default(), + crate::services::PlaylistClear { + comment: false, + tracks: true, + }, ) .await .map_err(service_protocol)? diff --git a/tests/v2_foundations.rs b/tests/v2_foundations.rs index 468ace4..9b3763e 100644 --- a/tests/v2_foundations.rs +++ b/tests/v2_foundations.rs @@ -2339,6 +2339,32 @@ async fn subsonic_xml_json_auth_catalog_and_user_data_are_compatible() { created["subsonic-response"]["playlist"]["owner"], "sub-admin" ); + // Given a playlistId, songId names every song of the playlist. A client + // that removes a song sends back what remains, so treating those ids as + // additions left the removed song in place and the edit looked lost. + let replaced = subsonic_json( + &router, + "createPlaylist", + api_key, + &format!("&playlistId={playlist}&songId={no_artist_song}"), + ) + .await; + assert_eq!(replaced["subsonic-response"]["playlist"]["songCount"], 1); + let reread = subsonic_json(&router, "getPlaylist", api_key, &format!("&id={playlist}")).await; + let entries = reread["subsonic-response"]["playlist"]["entry"] + .as_array() + .unwrap(); + assert_eq!(entries.len(), 1); + assert_eq!(entries[0]["id"], no_artist_song.to_string()); + // Put the original song back, so the checks below see the playlist they + // were written against. + subsonic_json( + &router, + "createPlaylist", + api_key, + &format!("&playlistId={playlist}&songId={song}"), + ) + .await; for (method, extra) in [ ("getPlaylists", String::new()), ("getPlaylist", format!("&id={playlist}")), From 0c9ff22dd0e5bbe5d5b906cfbc8e825ee18fecd6 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 19 Aug 2026 03:07:47 +0200 Subject: [PATCH 04/18] docs(subsonic): record the Feishin re-run and what it found Signed-off-by: InstaZDLL --- docs/subsonic-compatibility.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/subsonic-compatibility.md b/docs/subsonic-compatibility.md index b3b07c5..9d17a78 100644 --- a/docs/subsonic-compatibility.md +++ b/docs/subsonic-compatibility.md @@ -5,18 +5,18 @@ Automated protocol coverage is enforced by `tests/v2_foundations.rs` for XML, JS | Client | Version | Login | Browse/search | Native/transcode | Playlists/user data | Status | |---|---:|---:|---:|---:|---:|---| | Symfonium | 14.1.0 | pass | pass | pass | pass | Re-run 2026-08-19 against the current contract, on an Android 17 emulator through an ephemeral `cloudflared` HTTPS tunnel. Every observable change of the six batches was checked against server state rather than against the client's own display: sleeve order in both the album and a synced playlist, two distinct album artists with no composite entity in the artist list, two album genres, a genre spelled four ways answering as one (4 tracks), the extended fields including `explicitStatus`, and a failed login decoded correctly now that it arrives as HTTP 200. User data round-tripped: track and album ratings, track/album/artist favorites, 21 scrobbles, a bookmark resumed at 37 s, three playlists owned by the authenticated user, and 13 Opus cache entries across the 64 and 128 kbit/s ceilings. The earlier 2026-08-09 run on a physical Android 17 device additionally covered API-key authentication, which this one did not re-run. | -| Feishin | 1.15.1 | pass | pass | pass | pass | Validated 2026-08-02: native playback, Opus cache, playlist create/add, favorite, rating, scrobble and queue. | +| Feishin | 1.15.1 | pass | pass | pass | pass | Re-run 2026-08-19 against the current contract, on Windows desktop. **This run found the two server defects fixed in the same change**: a cold transcode refused the `Range: bytes=0-` that a browser sends to open any resource, so playback failed on every track no earlier client had transcoded; and `createPlaylist` appended its `songId` values instead of replacing the list, so every playlist edit looked lost. Both were then replayed against the fixed server through a logging proxy and confirmed from server state: cold transcode, cache hit and seek all answer, a removal drops to one track, a reorder lands. The rest of the run exercised search (23 `search3`), 145 cover-art reads, genre browsing including 8 `getSongsByGenre`, artists, `getStarred`, `getRandomSongs`, 9 scrobbles, favorites on tracks/album/artists and ratings — all read back from the server rather than from the client. `getAlbumInfo`/`getAlbumInfo2` were never called by this version, so they stay unexercised here rather than counted as a pass, as do the bookmark and play-queue methods. `getTopSongs`, `getArtistInfo` and `getInternetRadioStations` answer empty containers, which is what this server has to say about them. | | Substreamer | 8.0.91 | pass | pass | pass | pass* | Validated 2026-08-02 from the official release source: native playback, Opus/128 cache, playlist add, favorite, rating and scrobble. `*` Its playback queue is local-only and the client never calls `getPlayQueue`/`savePlayQueue`; those endpoints remain covered by fixtures and Feishin. | | DSub | 5.5.3 (F-Droid 208) | pass | pass | pass | pass* | Validated 2026-08-02 on Android 14, then revalidated 2026-08-15 on the current Android environment: authentication, catalogue, artwork, native playback, seek and playlist. `*` Scrobbling was disabled in the original run; the endpoint remains covered by fixtures and other clients. | | Juliet | iOS build tested 2026-08-15 | pass | pass | native pass; transcode not run | not run | Current iOS compatibility check: authentication, catalogue, artwork and native playback succeeded. Unsupported/unexercised surfaces are not inferred as passes. | -**Symfonium is the only row re-run against the current contract.** The four -others predate the move to HTTP 200 on every `/rest` answer: each records a real -run against the previous status behaviour and is kept as a historical result, but -none of them validates what the server answers today. The M3 real-client gate was -closed on that earlier behaviour, so it is not a satisfied gate for the next tag: -the remaining clients must be re-run against the current contract and their rows -re-dated first. Missing client features stay covered by automated fixtures and +**Symfonium and Feishin are the rows re-run against the current contract.** The +three others predate the move to HTTP 200 on every `/rest` answer: each records a +real run against the previous status behaviour and is kept as a historical +result, but none of them validates what the server answers today. The M3 +real-client gate was closed on that earlier behaviour, so it is not a satisfied +gate for the next tag: Substreamer, DSub and Juliet must be re-run against the +current contract and their rows re-dated first. Missing client features stay covered by automated fixtures and another real client rather than inferred as passes. Creating any tag or release remains a separate action requiring an explicit operator request. From d48c7269203771189fce9223783b999570f84372 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 19 Aug 2026 03:11:42 +0200 Subject: [PATCH 05/18] docs(subsonic): drop substreamer from the replayed set, juliet takes its place Signed-off-by: InstaZDLL --- docs/subsonic-compatibility.md | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/docs/subsonic-compatibility.md b/docs/subsonic-compatibility.md index 9d17a78..608e5d3 100644 --- a/docs/subsonic-compatibility.md +++ b/docs/subsonic-compatibility.md @@ -15,16 +15,21 @@ three others predate the move to HTTP 200 on every `/rest` answer: each records real run against the previous status behaviour and is kept as a historical result, but none of them validates what the server answers today. The M3 real-client gate was closed on that earlier behaviour, so it is not a satisfied -gate for the next tag: Substreamer, DSub and Juliet must be re-run against the -current contract and their rows re-dated first. Missing client features stay covered by automated fixtures and +gate for the next tag: DSub and Juliet must be re-run against the current +contract and their rows re-dated first, Substreamer being out of the replayed +set for the reason given below. Missing client features stay covered by automated fixtures and another real client rather than inferred as passes. Creating any tag or release remains a separate action requiring an explicit operator request. The Substreamer row records the successful 2026-08-02 run. It could not be reinstalled on the current Android 17 device during the 2026-08-15 revalidation -because the store marks that legacy build incompatible; Juliet provides the -current iOS sanity check instead. The historical Substreamer evidence is kept, -not silently rewritten as a new run. +because the store marks that legacy build incompatible, and the 2026-08-19 +re-run confirmed it: that build no longer launches on the current emulator. It +is therefore **out of the replayed set** — its row stays as historical evidence +of a real run against the old contract, and is not counted toward the next tag. +Juliet on a physical iPhone takes its place as the fifth client, which also +moves the iOS check off an emulator. The historical Substreamer evidence is +kept, not silently rewritten as a new run. For Substreamer, the Android Media3 session completed the 11-track validation album and the server recorded the corresponding start/submission scrobbles. With the client's streaming profile set to Opus at 128 kbit/s, WaveFlow produced 11 distinct cache entries; `ffprobe` identified the output as an Ogg container with an Opus audio stream. Its playlist mutation, track/album favorites and 4/5 rating were also read back from WaveFlow rather than inferred from local UI state. From 8f3fa12caa83fc058a7b064786d954023ad614fc Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 19 Aug 2026 05:01:11 +0200 Subject: [PATCH 06/18] fix(catalog): an album hangs off its first credited artist The track artist tag was split on the multi-value separator; the album artist tag was not. The whole joined string went into the artist table, so a record credited "Nova Kern; Lior Sand" produced a third artist named after the credit, gave it the album, and left both real artists holding nothing - browsing to either found no album at all. DSub is what surfaced it: clients that render the album's artists[] hid it. The album keeps the joined string as its display name and hangs off the first credited artist, which is as much as one artistId per album can say. The association is also re-derived at the end of every scan, next to the MusicBrainz identifiers. Without that the fix would only reach new libraries: an existing one has unchanged files, so nothing is reindexed and its albums keep pointing at the stale entity forever. An artist left holding neither a track nor an album is then dropped by the same pass. Signed-off-by: InstaZDLL --- src/catalog.rs | 60 ++++++++++++++++++-- tests/v2_foundations.rs | 120 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 176 insertions(+), 4 deletions(-) diff --git a/src/catalog.rs b/src/catalog.rs index 7d4b605..db3f824 100644 --- a/src/catalog.rs +++ b/src/catalog.rs @@ -414,6 +414,48 @@ impl Database { .bind(library_id.to_string()) .execute(&mut *tx) .await?; + // Re-derive which artist an album hangs off, for the same reason the + // identifiers above are re-derived: a library indexed by an earlier + // version has its albums pointing at an entity named after the joined + // credit, and its files have not changed, so nothing would ever be + // reindexed. Deriving it here repairs those albums on the next scan + // instead of requiring a rebuilt library. + let joined: Vec<(String, String)> = sqlx::query_as( + "SELECT id, album_artist_name FROM album \ + WHERE library_id = ? AND album_artist_name LIKE '%;%'", + ) + .bind(library_id.to_string()) + .fetch_all(&mut *tx) + .await?; + let now = now_ms(); + for (album_id, credit) in joined { + let names = split_values(Some(credit.as_str())); + let mut first = None; + for name in &names { + let id = upsert_artist(&mut tx, library_id, name, now).await?; + first.get_or_insert(id); + } + if let Some(first) = first { + sqlx::query("UPDATE album SET album_artist_id = ? WHERE id = ?") + .bind(first.to_string()) + .bind(album_id) + .execute(&mut *tx) + .await?; + } + } + // An artist row outlives the tag that created it: rename a credit, or + // fix a server that used to mint one entity per joined album-artist + // string, and the old name stays in the index forever. An artist that + // holds neither a track nor an album is no longer part of the + // catalogue, so it goes with the same scan that made it unreferenced. + sqlx::query( + "DELETE FROM artist WHERE library_id = ? \ + AND NOT EXISTS (SELECT 1 FROM album al WHERE al.album_artist_id = artist.id) \ + AND NOT EXISTS (SELECT 1 FROM track_artist ta WHERE ta.artist_id = artist.id)", + ) + .bind(library_id.to_string()) + .execute(&mut *tx) + .await?; tx.commit().await?; Ok(()) } @@ -608,10 +650,20 @@ impl Database { .map(str::to_owned) .or_else(|| input.is_compilation.then(|| "Various Artists".to_owned())) .or_else(|| artist_names.first().cloned()); - let album_artist_id = match album_artist.as_deref() { - Some(name) => Some(upsert_artist(tx, library_id, name, now).await?), - None => None, - }; + // The album artist tag carries the same multi-value separator as the + // track artist tag, so it is split the same way. Feeding the whole + // string to upsert_artist created one entity named after the joined + // credit, gave it the album, and left the real artists holding + // nothing: browsing to either of them found no album at all. + // + // The album keeps the joined string as its display name, and hangs + // off the first credited artist. + let album_artist_names = split_values(album_artist.as_deref()); + let mut album_artist_id = None; + for name in &album_artist_names { + let id = upsert_artist(tx, library_id, name, now).await?; + album_artist_id.get_or_insert(id); + } let album_id = upsert_album( tx, library_id, diff --git a/tests/v2_foundations.rs b/tests/v2_foundations.rs index 9b3763e..440c1ab 100644 --- a/tests/v2_foundations.rs +++ b/tests/v2_foundations.rs @@ -4782,6 +4782,126 @@ async fn bookmarks_round_trip_sync_and_isolate_tenants() { subsonic_json(&router, "deleteBookmark", api_key, &format!("&id={track}")).await; } +/// An album artist tag holding two credits is two artists, not one artist whose +/// name contains a semicolon. Feeding the joined string to the artist table +/// minted an entity named after it, gave that entity the album, and left both +/// real artists with nothing: DSub browsed to either of them and found no +/// album. The association is re-derived at the end of every scan, because a +/// library indexed by the earlier version has unchanged files and would +/// otherwise never be repaired. +#[tokio::test] +async fn an_album_hangs_off_its_first_credited_artist_not_the_joined_string() { + let (_temp, config, state) = test_app().await; + let owner = state + .db + .create_account( + "credit-owner", + &security::hash_password("correct horse battery staple").unwrap(), + AccountRole::Admin, + now_ms(), + ) + .await + .unwrap(); + let music = config.data_dir.join("credit-music"); + std::fs::create_dir_all(&music).unwrap(); + let library = state + .db + .create_library( + owner, + "Credits", + &std::fs::canonicalize(&music).unwrap(), + LibraryVisibility::Private, + now_ms(), + ) + .await + .unwrap(); + let scan = state + .db + .create_scan_job(library, Some(owner), "manual") + .await + .unwrap(); + state.db.start_scan_job(scan, 1).await.unwrap(); + let mut input = browse_input( + 800, + "Paired", + "Convergence", + "Nova Kern; Lior Sand", + Some(1), + None, + ); + input.relative_path = "credit-0.flac".into(); + input.quick_hash = format!("{:064x}", 81_000); + input.full_hash = format!("{:064x}", 82_000); + state + .db + .apply_catalog_track(library, scan, &input, None, false) + .await + .unwrap(); + state.db.finish_scan_job(scan, 0).await.unwrap(); + + let names = |state: waveflow_server::AppState| async move { + state + .services + .list_artists(owner, None, Default::default()) + .await + .unwrap() + .into_iter() + .map(|summary| (summary.artist.name, summary.artist.id)) + .collect::>() + }; + // Checked before the post-scan pass runs, so indexing itself has to be + // right rather than leaning on the repair below. + let artists = names(state.clone()).await; + assert_eq!( + artists.keys().cloned().collect::>(), + vec!["Lior Sand".to_owned(), "Nova Kern".to_owned()], + "the joined credit must not become an artist of its own" + ); + state.db.consolidate_musicbrainz_ids(library).await.unwrap(); + let albums = state.services.catalog_snapshot(owner, &[]).await.unwrap(); + assert_eq!(albums.albums[0].artist_id, Some(artists["Nova Kern"])); + // The album still displays the whole credit; only the entity it hangs off + // is the first artist. + assert_eq!( + albums.albums[0].artist.as_deref(), + Some("Nova Kern; Lior Sand") + ); + + // Now the repair path. Put the catalogue back into the shape the earlier + // version produced - a third artist named after the joined credit, holding + // the album - and let a scan pass fix it. + let stale = uuid::Uuid::new_v4(); + sqlx::query( + "INSERT INTO artist (id, library_id, name, canonical_name, created_at, updated_at) \ + VALUES (?, ?, 'Nova Kern; Lior Sand', 'nova kern; lior sand', ?, ?)", + ) + .bind(stale.to_string()) + .bind(library.to_string()) + .bind(now_ms()) + .bind(now_ms()) + .execute(state.db.pool()) + .await + .unwrap(); + sqlx::query("UPDATE album SET album_artist_id = ? WHERE library_id = ?") + .bind(stale.to_string()) + .bind(library.to_string()) + .execute(state.db.pool()) + .await + .unwrap(); + assert_eq!(names(state.clone()).await.len(), 3); + + state.db.consolidate_musicbrainz_ids(library).await.unwrap(); + + let repaired = names(state.clone()).await; + assert_eq!( + repaired.keys().cloned().collect::>(), + vec!["Lior Sand".to_owned(), "Nova Kern".to_owned()], + "the stale entity holds nothing once the album moves, so it goes" + ); + let albums = state.services.catalog_snapshot(owner, &[]).await.unwrap(); + assert_eq!(albums.albums[0].artist_id, Some(repaired["Nova Kern"])); +} + /// A release identifier belongs to the release, not to whichever file was /// scanned last. Tracks of one album routinely disagree — a library assembled /// over years holds files tagged against different releases of the same record From 13c1f57c345131ecac3e4c1a310f97688c65b9a7 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 19 Aug 2026 05:01:51 +0200 Subject: [PATCH 07/18] docs(subsonic): record the dsub re-run and correct two claims of the brief Signed-off-by: InstaZDLL --- docs/subsonic-compatibility.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/subsonic-compatibility.md b/docs/subsonic-compatibility.md index 608e5d3..d2cdd99 100644 --- a/docs/subsonic-compatibility.md +++ b/docs/subsonic-compatibility.md @@ -7,17 +7,17 @@ Automated protocol coverage is enforced by `tests/v2_foundations.rs` for XML, JS | Symfonium | 14.1.0 | pass | pass | pass | pass | Re-run 2026-08-19 against the current contract, on an Android 17 emulator through an ephemeral `cloudflared` HTTPS tunnel. Every observable change of the six batches was checked against server state rather than against the client's own display: sleeve order in both the album and a synced playlist, two distinct album artists with no composite entity in the artist list, two album genres, a genre spelled four ways answering as one (4 tracks), the extended fields including `explicitStatus`, and a failed login decoded correctly now that it arrives as HTTP 200. User data round-tripped: track and album ratings, track/album/artist favorites, 21 scrobbles, a bookmark resumed at 37 s, three playlists owned by the authenticated user, and 13 Opus cache entries across the 64 and 128 kbit/s ceilings. The earlier 2026-08-09 run on a physical Android 17 device additionally covered API-key authentication, which this one did not re-run. | | Feishin | 1.15.1 | pass | pass | pass | pass | Re-run 2026-08-19 against the current contract, on Windows desktop. **This run found the two server defects fixed in the same change**: a cold transcode refused the `Range: bytes=0-` that a browser sends to open any resource, so playback failed on every track no earlier client had transcoded; and `createPlaylist` appended its `songId` values instead of replacing the list, so every playlist edit looked lost. Both were then replayed against the fixed server through a logging proxy and confirmed from server state: cold transcode, cache hit and seek all answer, a removal drops to one track, a reorder lands. The rest of the run exercised search (23 `search3`), 145 cover-art reads, genre browsing including 8 `getSongsByGenre`, artists, `getStarred`, `getRandomSongs`, 9 scrobbles, favorites on tracks/album/artists and ratings — all read back from the server rather than from the client. `getAlbumInfo`/`getAlbumInfo2` were never called by this version, so they stay unexercised here rather than counted as a pass, as do the bookmark and play-queue methods. `getTopSongs`, `getArtistInfo` and `getInternetRadioStations` answer empty containers, which is what this server has to say about them. | | Substreamer | 8.0.91 | pass | pass | pass | pass* | Validated 2026-08-02 from the official release source: native playback, Opus/128 cache, playlist add, favorite, rating and scrobble. `*` Its playback queue is local-only and the client never calls `getPlayQueue`/`savePlayQueue`; those endpoints remain covered by fixtures and Feishin. | -| DSub | 5.5.3 (F-Droid 208) | pass | pass | pass | pass* | Validated 2026-08-02 on Android 14, then revalidated 2026-08-15 on the current Android environment: authentication, catalogue, artwork, native playback, seek and playlist. `*` Scrobbling was disabled in the original run; the endpoint remains covered by fixtures and other clients. | +| DSub | 5.5.3 (F-Droid 208) | pass | pass | pass | pass | Re-run 2026-08-19 against the current contract, on an Android 17 emulator, read from `logcat -s RESTMusicService` and DSub's own disk cache rather than from its interface. **This run found the album-artist defect fixed in the same change**: an album credited to two artists hung off a third entity named after the joined string, and browsing to either real artist found no album. The rest passed: sleeve order in both browse modes, two album genres, one canonical `Jazz` answering 4 tracks, artwork, native FLAC and MP3 with seek, a failed login decoded as an error now that it arrives as HTTP 200, playlist create/add/remove/rename/delete — the removal going through `songIndexToRemove`, by index — and a rating. It settles a question Symfonium left open: DSub does submit the scrobble of the last track in a queue. Two of its three unique surfaces needed the brief corrected. The legacy `getAlbumList` is used **only** when "Browse By Tags" is off, `getAlbumList2` otherwise; both were exercised. And `maxBitRate` is never sent above the source — DSub emits `min(setting, track bitrate)` — so "ceiling above the source" is not expressible from this client; the two reachable cases are correct (128 ceiling on a 128 kbit/s MP3 streams natively, 64 transcodes). The generic `star?id` form resolved a track, an album and an artist, all three surviving a resync. Unresolved, and not a server fault: one FLAC (`Drift`) is fetched repeatedly and never completed by the client, though the server answers 200 with the exact `content-length` and the bytes arrive md5-identical to the source file through the same tunnel. Absent from the client, recorded as absent: the explicit marker, and any separate display of two album artists. | | Juliet | iOS build tested 2026-08-15 | pass | pass | native pass; transcode not run | not run | Current iOS compatibility check: authentication, catalogue, artwork and native playback succeeded. Unsupported/unexercised surfaces are not inferred as passes. | -**Symfonium and Feishin are the rows re-run against the current contract.** The -three others predate the move to HTTP 200 on every `/rest` answer: each records a +**Symfonium, Feishin and DSub are the rows re-run against the current +contract.** The two others predate the move to HTTP 200 on every `/rest` answer: each records a real run against the previous status behaviour and is kept as a historical result, but none of them validates what the server answers today. The M3 real-client gate was closed on that earlier behaviour, so it is not a satisfied -gate for the next tag: DSub and Juliet must be re-run against the current -contract and their rows re-dated first, Substreamer being out of the replayed -set for the reason given below. Missing client features stay covered by automated fixtures and +gate for the next tag: Juliet must be re-run against the current contract and +its row re-dated first, Substreamer being out of the replayed set for the reason +given below. Missing client features stay covered by automated fixtures and another real client rather than inferred as passes. Creating any tag or release remains a separate action requiring an explicit operator request. From d34730a3fdc0081734b158e22001252891cde288 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 19 Aug 2026 09:24:47 +0200 Subject: [PATCH 08/18] fix(media): a cold transcode answers the iOS two-byte probe too The previous fix only recognised `bytes=0-`, the range a browser sends to open an audio element. iOS AVFoundation opens a resource differently: it first probes with `bytes=0-1` to learn the size and type. That names an end, so it was still classified as a seek and refused with 416 before the transcode existed - Juliet failed on the first play of every track and succeeded on the second, once the cache had been built by the very request that failed. Any range starting at the first byte is an open, whatever its end, and is answered with the whole stream: HTTP allows a server to ignore Range and answer 200, and before the transcode exists there is no total length to put in a Content-Range anyway. A range starting anywhere else is still a seek and still refused; seeking a live transcode goes through offsetMs. A multipart range is refused as well, since one body cannot answer it. Signed-off-by: InstaZDLL --- src/media.rs | 64 +++++++++++++++++++++++++++-------------- tests/v2_foundations.rs | 26 +++++++++++++++++ 2 files changed, 69 insertions(+), 21 deletions(-) diff --git a/src/media.rs b/src/media.rs index 10ca8cc..a7614dd 100644 --- a/src/media.rs +++ b/src/media.rs @@ -175,11 +175,13 @@ impl MediaService { } // A byte range has no stable meaning until a transcode is complete, so - // seeking a live transcode goes through offset_ms instead. `bytes=0-` - // is not a seek: it is what a browser audio element sends to open any - // resource, and refusing it made every web client fail on the first - // play of a track and succeed on the second, once the cache existed. - if range.is_some_and(|value| !opens_whole_resource(value)) { + // seeking a live transcode goes through offset_ms instead. A range + // that starts at the first byte is not a seek though: it is how every + // player opens a resource. Refusing it made a client fail on the first + // play of a track and succeed on the second, once the cache existed — + // Feishin with `bytes=0-`, then Juliet with the `bytes=0-1` probe iOS + // sends before anything else. + if range.is_some_and(|value| !starts_at_the_first_byte(value)) { return Err(MediaError::RangeNotSatisfiable(0)); } @@ -852,12 +854,25 @@ async fn serve_partial( Ok(response) } -/// Whether a `Range` header asks for the resource from its first byte with no -/// end, which is how browsers open an audio element rather than how they seek. -fn opens_whole_resource(value: &str) -> bool { - value - .strip_prefix("bytes=") - .is_some_and(|spec| spec.trim() == "0-") +/// Whether a `Range` header starts at the first byte, which is how a player +/// opens a resource rather than how it seeks into one. Browsers send `bytes=0-`; +/// iOS AVFoundation first probes with `bytes=0-1` to learn the size and type. +/// Both mean "begin here", and neither can be refused without breaking the +/// first play of every track. +/// +/// The answer to such a request is the whole stream rather than the bytes +/// asked for, which HTTP allows: a server may always ignore `Range` and answer +/// 200. Before the transcode exists there is no total length to put in a +/// `Content-Range`, so a partial answer is not available to give. +fn starts_at_the_first_byte(value: &str) -> bool { + value.strip_prefix("bytes=").is_some_and(|spec| { + let spec = spec.trim(); + // A multipart range is several ranges, and this answers with one body. + !spec.contains(',') + && spec + .split_once('-') + .is_some_and(|(start, _)| start.trim().parse::() == Ok(0)) + }) } fn parse_range(value: &str, size: u64) -> Option<(u64, u64)> { @@ -1000,7 +1015,9 @@ mod tests { use dashmap::DashMap; use tokio::sync::Semaphore; - use super::{opens_whole_resource, parse_range, prune_cache, secure_track_path, MediaInner}; + use super::{ + parse_range, prune_cache, secure_track_path, starts_at_the_first_byte, MediaInner, + }; #[test] fn ranges_are_bounded_and_validated() { @@ -1013,15 +1030,20 @@ mod tests { } #[test] - fn only_an_unbounded_range_from_zero_opens_a_resource() { - assert!(opens_whole_resource("bytes=0-")); - assert!(opens_whole_resource("bytes= 0- ")); - // Anything that names an end, or starts elsewhere, is a seek. - assert!(!opens_whole_resource("bytes=0-1023")); - assert!(!opens_whole_resource("bytes=1-")); - assert!(!opens_whole_resource("bytes=-3")); - assert!(!opens_whole_resource("bytes=0-,2-3")); - assert!(!opens_whole_resource("items=0-")); + fn a_range_from_the_first_byte_opens_a_resource() { + // How a browser opens an audio element, and how iOS probes one. + assert!(starts_at_the_first_byte("bytes=0-")); + assert!(starts_at_the_first_byte("bytes=0-1")); + assert!(starts_at_the_first_byte("bytes=0-1023")); + assert!(starts_at_the_first_byte("bytes= 0- ")); + // Starting anywhere else is a seek, and has no meaning yet. + assert!(!starts_at_the_first_byte("bytes=1-")); + assert!(!starts_at_the_first_byte("bytes=19924-429882")); + assert!(!starts_at_the_first_byte("bytes=-3")); + // One body cannot answer several ranges. + assert!(!starts_at_the_first_byte("bytes=0-1,4-5")); + assert!(!starts_at_the_first_byte("items=0-")); + assert!(!starts_at_the_first_byte("bytes=00x-1")); } #[tokio::test] diff --git a/tests/v2_foundations.rs b/tests/v2_foundations.rs index 440c1ab..fc1d535 100644 --- a/tests/v2_foundations.rs +++ b/tests/v2_foundations.rs @@ -1542,6 +1542,32 @@ async fn media_streaming_ranges_transcodes_caches_and_isolates_tenants() { > 100 ); + // iOS probes a resource with a two-byte range before it plays anything, so + // a bounded range from zero has to open the stream too. Juliet failed on + // the first play of every track until it did. + let cold_probe = router + .clone() + .oneshot( + Request::get(format!("{uri}?format=mp3&bitrate=136")) + .header("authorization", format!("Bearer {owner_token}")) + .header("range", "bytes=0-1") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(cold_probe.status(), StatusCode::OK); + assert_eq!(cold_probe.headers()["content-type"], "audio/mpeg"); + // Drained and awaited: an unread transcode holds its per-user permit, and + // the checks below would meet 429 instead of what they are testing. + cold_probe.into_body().collect().await.unwrap(); + for _ in 0..100 { + if media.active_transcodes() == 0 { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + } + // A range that actually seeks still has no meaning before the transcode // exists, and keeps its refusal. let cold_seek = router From a24081c77be6d5270f283865761360d9ef129856 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 19 Aug 2026 09:28:04 +0200 Subject: [PATCH 09/18] docs(subsonic): record the juliet re-run, the last of the five rows Signed-off-by: InstaZDLL --- docs/subsonic-compatibility.md | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/docs/subsonic-compatibility.md b/docs/subsonic-compatibility.md index d2cdd99..4a2fb0f 100644 --- a/docs/subsonic-compatibility.md +++ b/docs/subsonic-compatibility.md @@ -8,18 +8,26 @@ Automated protocol coverage is enforced by `tests/v2_foundations.rs` for XML, JS | Feishin | 1.15.1 | pass | pass | pass | pass | Re-run 2026-08-19 against the current contract, on Windows desktop. **This run found the two server defects fixed in the same change**: a cold transcode refused the `Range: bytes=0-` that a browser sends to open any resource, so playback failed on every track no earlier client had transcoded; and `createPlaylist` appended its `songId` values instead of replacing the list, so every playlist edit looked lost. Both were then replayed against the fixed server through a logging proxy and confirmed from server state: cold transcode, cache hit and seek all answer, a removal drops to one track, a reorder lands. The rest of the run exercised search (23 `search3`), 145 cover-art reads, genre browsing including 8 `getSongsByGenre`, artists, `getStarred`, `getRandomSongs`, 9 scrobbles, favorites on tracks/album/artists and ratings — all read back from the server rather than from the client. `getAlbumInfo`/`getAlbumInfo2` were never called by this version, so they stay unexercised here rather than counted as a pass, as do the bookmark and play-queue methods. `getTopSongs`, `getArtistInfo` and `getInternetRadioStations` answer empty containers, which is what this server has to say about them. | | Substreamer | 8.0.91 | pass | pass | pass | pass* | Validated 2026-08-02 from the official release source: native playback, Opus/128 cache, playlist add, favorite, rating and scrobble. `*` Its playback queue is local-only and the client never calls `getPlayQueue`/`savePlayQueue`; those endpoints remain covered by fixtures and Feishin. | | DSub | 5.5.3 (F-Droid 208) | pass | pass | pass | pass | Re-run 2026-08-19 against the current contract, on an Android 17 emulator, read from `logcat -s RESTMusicService` and DSub's own disk cache rather than from its interface. **This run found the album-artist defect fixed in the same change**: an album credited to two artists hung off a third entity named after the joined string, and browsing to either real artist found no album. The rest passed: sleeve order in both browse modes, two album genres, one canonical `Jazz` answering 4 tracks, artwork, native FLAC and MP3 with seek, a failed login decoded as an error now that it arrives as HTTP 200, playlist create/add/remove/rename/delete — the removal going through `songIndexToRemove`, by index — and a rating. It settles a question Symfonium left open: DSub does submit the scrobble of the last track in a queue. Two of its three unique surfaces needed the brief corrected. The legacy `getAlbumList` is used **only** when "Browse By Tags" is off, `getAlbumList2` otherwise; both were exercised. And `maxBitRate` is never sent above the source — DSub emits `min(setting, track bitrate)` — so "ceiling above the source" is not expressible from this client; the two reachable cases are correct (128 ceiling on a 128 kbit/s MP3 streams natively, 64 transcodes). The generic `star?id` form resolved a track, an album and an artist, all three surviving a resync. Unresolved, and not a server fault: one FLAC (`Drift`) is fetched repeatedly and never completed by the client, though the server answers 200 with the exact `content-length` and the bytes arrive md5-identical to the source file through the same tunnel. Absent from the client, recorded as absent: the explicit marker, and any separate display of two album artists. | -| Juliet | iOS build tested 2026-08-15 | pass | pass | native pass; transcode not run | not run | Current iOS compatibility check: authentication, catalogue, artwork and native playback succeeded. Unsupported/unexercised surfaces are not inferred as passes. | +| Juliet | 1.5 (iOS 26.6) | pass | pass | pass | pass | Re-run 2026-08-19 against the current contract, on a physical iPhone — the previous entry was a narrower check that never reached transcoding or user data. **This run found the second half of the cold-transcode defect**: iOS AVFoundation opens a resource with a `bytes=0-1` probe, which the first fix still classified as a seek and refused, so the first play of each track failed and the second succeeded off the cache the failed request had built. Fixed in the same change and verified by replaying the client's exact sequence: `bytes=0-1` cold answers 200, warm answers 206 with two bytes, a real seek answers 206, and a seek into a cold transcode is still refused. The rest of the run: 306 cover-art reads, 115 streams, 64 scrobbles, `getAlbumList2`, `search3`, `getRandomSongs`, `getSongsByGenre`, `getSimilarSongs2`, `getLyrics`, favorites, and playlist create/update/delete. It is also the first real client to exercise `savePlayQueue`/`getPlayQueue`, which the matrix had covered only through Feishin and fixtures. Absent from the client, recorded as absent: Juliet has no rating feature, so `setRating` was never sent; and it saves its queue with `position=0` every time, so playback restarts at zero — the server stores and returns what it is given, queue, current track and `changedBy` included. | -**Symfonium, Feishin and DSub are the rows re-run against the current -contract.** The two others predate the move to HTTP 200 on every `/rest` answer: each records a -real run against the previous status behaviour and is kept as a historical -result, but none of them validates what the server answers today. The M3 -real-client gate was closed on that earlier behaviour, so it is not a satisfied -gate for the next tag: Juliet must be re-run against the current contract and -its row re-dated first, Substreamer being out of the replayed set for the reason -given below. Missing client features stay covered by automated fixtures and -another real client rather than inferred as passes. Creating any tag or release -remains a separate action requiring an explicit operator request. +**Symfonium, Feishin, DSub and Juliet were all re-run on 2026-08-19 against the +current contract**, each on a real device or desktop and each read from server +state rather than from the client's own display. Substreamer is out of the +replayed set for the reason given below, and its row remains a historical +result against the previous status behaviour. + +The replay found **four server defects**, all fixed in the same change and none +of which the automated suite could have produced: it took a browser, a client +that edits a playlist by sending back what remains, a client that browses by +artist index, and an iOS player that probes a resource before reading it. +Missing client features stay covered by automated fixtures and another real +client rather than inferred as passes. + +What remains before a tag is a short re-play of the cold-transcode path on +Feishin and Juliet, since both found their defect rather than passing it: the +fixed behaviour was verified by replaying each client's exact request sequence, +which is not the same as the client itself replaying it. Creating any tag or +release remains a separate action requiring an explicit operator request. The Substreamer row records the successful 2026-08-02 run. It could not be reinstalled on the current Android 17 device during the 2026-08-15 revalidation From c81b1293b9e5b6eb68ea2009700e1b22f136708c Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 19 Aug 2026 09:31:05 +0200 Subject: [PATCH 10/18] docs(subsonic): the fixed cold-transcode path was re-played by the clients Signed-off-by: InstaZDLL --- docs/subsonic-compatibility.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/docs/subsonic-compatibility.md b/docs/subsonic-compatibility.md index 4a2fb0f..fd7f1b2 100644 --- a/docs/subsonic-compatibility.md +++ b/docs/subsonic-compatibility.md @@ -23,11 +23,13 @@ artist index, and an iOS player that probes a resource before reading it. Missing client features stay covered by automated fixtures and another real client rather than inferred as passes. -What remains before a tag is a short re-play of the cold-transcode path on -Feishin and Juliet, since both found their defect rather than passing it: the -fixed behaviour was verified by replaying each client's exact request sequence, -which is not the same as the client itself replaying it. Creating any tag or -release remains a separate action requiring an explicit operator request. +Feishin and Juliet found their defect rather than passing it, so each one +re-played the fixed path itself rather than being credited on a replayed +request sequence: Feishin's MP3 transcode now starts, caches and seeks, and +Juliet's two-byte probe answers 200 cold with no refusal left in the log. The +five rows therefore record the behaviour of the server as it stands. Creating +any tag or release remains a separate action requiring an explicit operator +request. The Substreamer row records the successful 2026-08-02 run. It could not be reinstalled on the current Android 17 device during the 2026-08-15 revalidation From 7f3afe8562d4ba4182d997a0f87a9d51c88fa29a Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 19 Aug 2026 09:50:25 +0200 Subject: [PATCH 11/18] fix(catalog): recompute the album identity when repairing its artist The identity key embeds the album artist id, so moving an album to its first credited artist without recomputing the key left a row indexing could no longer find: the next reindexed track computed the new key, missed, and inserted a duplicate album. The key is recomputed with the move, through one helper shared with upsert_album so the two cannot drift. When indexing already created the album the stale row is becoming, the two are one record: the tracks move over and the stale row goes, because updating in place would violate the identity uniqueness and fail the whole scan. Signed-off-by: InstaZDLL --- src/catalog.rs | 68 ++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 55 insertions(+), 13 deletions(-) diff --git a/src/catalog.rs b/src/catalog.rs index db3f824..14d1310 100644 --- a/src/catalog.rs +++ b/src/catalog.rs @@ -420,27 +420,63 @@ impl Database { // credit, and its files have not changed, so nothing would ever be // reindexed. Deriving it here repairs those albums on the next scan // instead of requiring a rebuilt library. - let joined: Vec<(String, String)> = sqlx::query_as( - "SELECT id, album_artist_name FROM album \ + let joined: Vec<(String, String, String)> = sqlx::query_as( + "SELECT id, canonical_title, album_artist_name FROM album \ WHERE library_id = ? AND album_artist_name LIKE '%;%'", ) .bind(library_id.to_string()) .fetch_all(&mut *tx) .await?; let now = now_ms(); - for (album_id, credit) in joined { + for (album_id, canonical_title, credit) in joined { let names = split_values(Some(credit.as_str())); let mut first = None; for name in &names { let id = upsert_artist(&mut tx, library_id, name, now).await?; first.get_or_insert(id); } - if let Some(first) = first { - sqlx::query("UPDATE album SET album_artist_id = ? WHERE id = ?") + let Some(first) = first else { continue }; + // The identity key embeds the album artist id, so moving the album + // without recomputing it would leave a row that indexing can no + // longer find: the next reindexed track would compute the new key, + // miss, and insert a duplicate album. + let identity = album_identity(&canonical_title, Some(first)); + let collision: Option = sqlx::query_scalar( + "SELECT id FROM album WHERE library_id = ? AND identity_key = ? AND id <> ?", + ) + .bind(library_id.to_string()) + .bind(&identity) + .bind(&album_id) + .fetch_optional(&mut *tx) + .await?; + match collision { + // Indexing already created the album this one is becoming, so + // the two are one record: the tracks move over and the stale + // row goes. Updating in place would violate the identity + // uniqueness and fail the whole scan. + Some(target) => { + sqlx::query("UPDATE track SET album_id = ? WHERE album_id = ?") + .bind(&target) + .bind(&album_id) + .execute(&mut *tx) + .await?; + sqlx::query("DELETE FROM album WHERE id = ?") + .bind(&album_id) + .execute(&mut *tx) + .await?; + } + None => { + sqlx::query( + "UPDATE album SET album_artist_id = ?, identity_key = ?, updated_at = ? \ + WHERE id = ?", + ) .bind(first.to_string()) - .bind(album_id) + .bind(&identity) + .bind(now) + .bind(&album_id) .execute(&mut *tx) .await?; + } } } // An artist row outlives the tag that created it: rename a credit, or @@ -1012,13 +1048,7 @@ async fn upsert_album( return Ok(None); }; let canonical = waveflow_core::scanner::canonical_name(title); - let identity = format!( - "{}:{}", - canonical, - album_artist_id - .map(|id| id.to_string()) - .unwrap_or_else(|| "none".into()) - ); + let identity = album_identity(&canonical, album_artist_id); let id = Uuid::new_v4(); let value: String = sqlx::query_scalar("INSERT INTO album (id, library_id, title, canonical_title, identity_key, album_artist_id, album_artist_name, is_compilation, year, artwork_hash, created_at, updated_at) \ VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT (library_id, identity_key) DO UPDATE SET title=excluded.title, album_artist_name=excluded.album_artist_name, \ @@ -1029,6 +1059,18 @@ async fn upsert_album( parse_uuid(value).map(Some) } +/// How an album is recognised across scans. It embeds the album artist, so any +/// code that moves an album to another artist has to recompute it. +fn album_identity(canonical_title: &str, album_artist_id: Option) -> String { + format!( + "{}:{}", + canonical_title, + album_artist_id + .map(|id| id.to_string()) + .unwrap_or_else(|| "none".into()) + ) +} + fn split_values(raw: Option<&str>) -> Vec { raw.into_iter() .flat_map(|value| value.split(';')) From 0aff1dec4f008721b7865cc8cc2a6f18c57a6e37 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 19 Aug 2026 09:50:25 +0200 Subject: [PATCH 12/18] fix(sync): keep a playlist replay valid across this upgrade The intent is hashed and compared on replay, so naming clear_tracks unconditionally changed the hash of every playlist update this server version had already recorded. A native client retrying an operation across the upgrade would have met a conflict instead of an idempotent replay. The field is now added to the payload only when it is set. Signed-off-by: InstaZDLL --- src/services.rs | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/services.rs b/src/services.rs index f87cc2a..0187ec8 100644 --- a/src/services.rs +++ b/src/services.rs @@ -1945,19 +1945,22 @@ impl DomainServices { let mut removes = remove_indexes.to_vec(); removes.sort_unstable_by(|a, b| b.cmp(a)); removes.dedup(); - let intent = MutationIntent::new( - "update", - &format!("playlist:{id}"), - &serde_json::json!({ - "name": name.map(str::trim), - "comment": comment, - "public": public, - "add": add, - "remove_indexes": &removes, - "clear_comment": clear.comment, - "clear_tracks": clear.tracks, - }), - ); + let mut intent_payload = serde_json::json!({ + "name": name.map(str::trim), + "comment": comment, + "public": public, + "add": add, + "remove_indexes": &removes, + "clear_comment": clear.comment, + }); + // Added to the payload only when set. The intent is hashed and compared + // on replay, so naming a new field unconditionally would change the + // hash of every update this server version ever saw before, and turn a + // client's retry across an upgrade into a conflict. + if clear.tracks { + intent_payload["clear_tracks"] = serde_json::Value::Bool(true); + } + let intent = MutationIntent::new("update", &format!("playlist:{id}"), &intent_payload); let _writer = self.db.writer_guard().await; let mut tx = self.db.pool().begin().await?; if let OperationClaim::Replayed(receipt) = self From e6151802d2c4618688fd3398aeb9049c5f4e1f0a Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 19 Aug 2026 09:50:25 +0200 Subject: [PATCH 13/18] fix(media): refuse a malformed range, honour a range the cache can serve Two narrow corrections to the cold-transcode path. A range whose end is malformed - a second hyphen, a word - was read as an open because only its start was parsed; it is refused now. And a caller that waited on the cache lock was answered the whole file even when it had asked for a range the finished transcode could satisfy, so it gets its 206. Signed-off-by: InstaZDLL --- src/media.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/media.rs b/src/media.rs index a7614dd..780aaef 100644 --- a/src/media.rs +++ b/src/media.rs @@ -210,7 +210,10 @@ impl MediaService { self.inner .cache_access .insert(cache_path.clone(), SystemTime::now()); - return serve_file(&cache_path, None, mime_for_format(query.format)).await; + // Whoever held the lock finished the transcode, so the range that + // had no meaning a moment ago has one now, and the second caller + // gets the same partial answer it would have got from the cache. + return serve_file(&cache_path, range, mime_for_format(query.format)).await; } self.stream_transcode( @@ -866,12 +869,14 @@ async fn serve_partial( /// `Content-Range`, so a partial answer is not available to give. fn starts_at_the_first_byte(value: &str) -> bool { value.strip_prefix("bytes=").is_some_and(|spec| { - let spec = spec.trim(); // A multipart range is several ranges, and this answers with one body. - !spec.contains(',') - && spec - .split_once('-') - .is_some_and(|(start, _)| start.trim().parse::() == Ok(0)) + let Some((start, end)) = spec.trim().split_once('-') else { + return false; + }; + start.trim().parse::() == Ok(0) + // A malformed end - a second hyphen, a word - is not a range this + // understands, so it is refused rather than read as an open. + && (end.trim().is_empty() || end.trim().parse::().is_ok()) }) } @@ -1044,6 +1049,9 @@ mod tests { assert!(!starts_at_the_first_byte("bytes=0-1,4-5")); assert!(!starts_at_the_first_byte("items=0-")); assert!(!starts_at_the_first_byte("bytes=00x-1")); + // A malformed end is not an open either. + assert!(!starts_at_the_first_byte("bytes=0-1-2")); + assert!(!starts_at_the_first_byte("bytes=0-invalid")); } #[tokio::test] From fb4b4f0c1836eccafb68e26ad2955fb95662b7ff Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 19 Aug 2026 09:50:26 +0200 Subject: [PATCH 14/18] docs(subsonic): the http 200 deviation is a result, not an expectation Signed-off-by: InstaZDLL --- docs/subsonic-compatibility.md | 15 ++--- tests/v2_foundations.rs | 116 +++++++++++++++++++++++++++++++-- 2 files changed, 116 insertions(+), 15 deletions(-) diff --git a/docs/subsonic-compatibility.md b/docs/subsonic-compatibility.md index fd7f1b2..db62ca3 100644 --- a/docs/subsonic-compatibility.md +++ b/docs/subsonic-compatibility.md @@ -53,12 +53,11 @@ The contract audit additionally covers administrative folder access: `createUser Protocol failures now answer HTTP 200 and report the outcome in the body (`status="failed"` plus an `error` code), as the Subsonic contract requires. -Previously WaveFlow also set 401, 403, 404, 409 or 429. All five clients in the -matrix above were validated against that old behaviour. Each reads `error/code`, -so none is expected to break — but that is an expectation, not a result, and the -matrix records runs rather than inferences: **every row must be re-run and -re-dated before the next tag**. Range responses keep 206/416, and `/share` and -`/api/v2` are unchanged. +Previously WaveFlow also set 401, 403, 404, 409 or 429. That this would not +break the clients was an expectation until the 2026-08-19 replay made it a +result: **each re-run row above was given a deliberate wrong password and +decoded it as an authentication error**. Range responses keep 206/416, and +`/share` and `/api/v2` are unchanged. The same release adds `tokenInfo` — the half of `apiKeyAuthentication` that was advertised but never served — and `getAlbumInfo`/`getAlbumInfo2`, so Feishin @@ -67,8 +66,8 @@ opens. `playlist.owner` and `share.username` now carry the authenticated username instead of an empty string, which is what Feishin reads to decide whether a playlist is editable. -Five further batches of wire changes have landed against the same unvalidated -matrix, and the re-run covers them all: media items gained the remaining +Five further batches of wire changes landed while the matrix was still +unvalidated, and the 2026-08-19 replay covers them all: media items gained the remaining OpenSubsonic fields under the presence rule (`moods`, `explicitStatus`, `isrc`, `replayGain`, `bpm` and the rest); `startScan`, `getScanStatus`, `search2`, `getStarred` and the bookmark methods were added or backed by real state; diff --git a/tests/v2_foundations.rs b/tests/v2_foundations.rs index fc1d535..d54d47d 100644 --- a/tests/v2_foundations.rs +++ b/tests/v2_foundations.rs @@ -1567,6 +1567,11 @@ async fn media_streaming_ranges_transcodes_caches_and_isolates_tenants() { } tokio::time::sleep(std::time::Duration::from_millis(10)).await; } + assert_eq!( + media.active_transcodes(), + 0, + "a drained transcode must release its permit" + ); // A range that actually seeks still has no meaning before the transcode // exists, and keeps its refusal. @@ -4899,21 +4904,32 @@ async fn an_album_hangs_off_its_first_credited_artist_not_the_joined_string() { let stale = uuid::Uuid::new_v4(); sqlx::query( "INSERT INTO artist (id, library_id, name, canonical_name, created_at, updated_at) \ - VALUES (?, ?, 'Nova Kern; Lior Sand', 'nova kern; lior sand', ?, ?)", + VALUES (?, ?, 'Nova Kern; Lior Sand', ?, ?, ?)", ) .bind(stale.to_string()) .bind(library.to_string()) + // What the production canonicaliser makes of the joined credit: every + // non-alphanumeric character becomes a space, so no semicolon survives. + .bind(waveflow_core::scanner::canonical_name( + "Nova Kern; Lior Sand", + )) .bind(now_ms()) .bind(now_ms()) .execute(state.db.pool()) .await .unwrap(); - sqlx::query("UPDATE album SET album_artist_id = ? WHERE library_id = ?") - .bind(stale.to_string()) - .bind(library.to_string()) - .execute(state.db.pool()) - .await - .unwrap(); + // The identity key embeds the album artist, so the stale state has the + // stale key too - that is what indexing would later fail to match. + sqlx::query( + "UPDATE album SET album_artist_id = ?, identity_key = canonical_title || ':' || ? \ + WHERE library_id = ?", + ) + .bind(stale.to_string()) + .bind(stale.to_string()) + .bind(library.to_string()) + .execute(state.db.pool()) + .await + .unwrap(); assert_eq!(names(state.clone()).await.len(), 3); state.db.consolidate_musicbrainz_ids(library).await.unwrap(); @@ -4926,6 +4942,92 @@ async fn an_album_hangs_off_its_first_credited_artist_not_the_joined_string() { ); let albums = state.services.catalog_snapshot(owner, &[]).await.unwrap(); assert_eq!(albums.albums[0].artist_id, Some(repaired["Nova Kern"])); + // The sweep drops what holds nothing, not what merely holds no album: the + // second credited artist keeps its tracks and stays. + assert!(repaired.contains_key("Lior Sand")); + assert_eq!( + state + .services + .artist(owner, repaired["Lior Sand"]) + .await + .unwrap() + .albums + .len(), + 0 + ); + + // Reindexing a track of a stale album is the case that made recomputing the + // identity key necessary: indexing creates the album under the new key + // while the stale row still carries the old one, and the repair has to + // merge them rather than fail the scan on the uniqueness constraint. + let rescan = state + .db + .create_scan_job(library, Some(owner), "manual") + .await + .unwrap(); + state.db.start_scan_job(rescan, 1).await.unwrap(); + // The sweep above removed the stale entity, so put it back to rebuild the + // starting state. + sqlx::query( + "INSERT INTO artist (id, library_id, name, canonical_name, created_at, updated_at) \ + VALUES (?, ?, 'Nova Kern; Lior Sand', ?, ?, ?)", + ) + .bind(stale.to_string()) + .bind(library.to_string()) + .bind(waveflow_core::scanner::canonical_name( + "Nova Kern; Lior Sand", + )) + .bind(now_ms()) + .bind(now_ms()) + .execute(state.db.pool()) + .await + .unwrap(); + sqlx::query( + "UPDATE album SET album_artist_id = ?, identity_key = canonical_title || ':' || ? \ + WHERE library_id = ?", + ) + .bind(stale.to_string()) + .bind(stale.to_string()) + .bind(library.to_string()) + .execute(state.db.pool()) + .await + .unwrap(); + input.file_size += 1; + let existing = state + .services + .catalog_snapshot(owner, &[]) + .await + .unwrap() + .songs[0] + .id; + state + .db + .apply_catalog_track(library, rescan, &input, Some(existing), false) + .await + .unwrap(); + state.db.finish_scan_job(rescan, 0).await.unwrap(); + assert_eq!( + state + .services + .catalog_snapshot(owner, &[]) + .await + .unwrap() + .albums + .len(), + 2, + "indexing under the new key leaves the stale album behind" + ); + + state.db.consolidate_musicbrainz_ids(library).await.unwrap(); + + let merged = state.services.catalog_snapshot(owner, &[]).await.unwrap(); + assert_eq!(merged.albums.len(), 1, "the two rows are one record"); + assert_eq!(merged.songs.len(), 1); + assert_eq!(merged.songs[0].album_id, Some(merged.albums[0].id)); + assert_eq!( + merged.albums[0].artist_id, + Some(names(state.clone()).await["Nova Kern"]) + ); } /// A release identifier belongs to the release, not to whichever file was From dafe3c34ca291befcb3d8da2a66ee24fab6569c0 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 19 Aug 2026 10:08:07 +0200 Subject: [PATCH 15/18] fix(catalog): an album identity names the whole credit, not just its lead Hanging every album off its first credited artist gave two different records one identity: "Live" by `A; B` and "Live" by `A; C` share a title and a lead, so the key could no longer tell them apart and a scan would merge them into one album. A joined credit now adds its canonical form to the key. A single credit keeps the historical two-part shape, so the keys already stored for the overwhelming majority of albums still match what indexing computes, and the repair path shares the one helper so the two cannot drift. parse_range gains the same trimming starts_at_the_first_byte does: a range spelled with spaces was accepted while the transcode was cold and refused once the cache existed. Signed-off-by: InstaZDLL --- src/catalog.rs | 34 ++++++++++++++++++++++++---------- src/media.rs | 9 ++++++++- tests/v2_foundations.rs | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 11 deletions(-) diff --git a/src/catalog.rs b/src/catalog.rs index 14d1310..d2a9a65 100644 --- a/src/catalog.rs +++ b/src/catalog.rs @@ -440,7 +440,7 @@ impl Database { // without recomputing it would leave a row that indexing can no // longer find: the next reindexed track would compute the new key, // miss, and insert a duplicate album. - let identity = album_identity(&canonical_title, Some(first)); + let identity = album_identity(&canonical_title, Some(first), Some(credit.as_str())); let collision: Option = sqlx::query_scalar( "SELECT id FROM album WHERE library_id = ? AND identity_key = ? AND id <> ?", ) @@ -1048,7 +1048,7 @@ async fn upsert_album( return Ok(None); }; let canonical = waveflow_core::scanner::canonical_name(title); - let identity = album_identity(&canonical, album_artist_id); + let identity = album_identity(&canonical, album_artist_id, album_artist); let id = Uuid::new_v4(); let value: String = sqlx::query_scalar("INSERT INTO album (id, library_id, title, canonical_title, identity_key, album_artist_id, album_artist_name, is_compilation, year, artwork_hash, created_at, updated_at) \ VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT (library_id, identity_key) DO UPDATE SET title=excluded.title, album_artist_name=excluded.album_artist_name, \ @@ -1061,14 +1061,28 @@ async fn upsert_album( /// How an album is recognised across scans. It embeds the album artist, so any /// code that moves an album to another artist has to recompute it. -fn album_identity(canonical_title: &str, album_artist_id: Option) -> String { - format!( - "{}:{}", - canonical_title, - album_artist_id - .map(|id| id.to_string()) - .unwrap_or_else(|| "none".into()) - ) +/// +/// A joined credit adds its whole canonical form. Hanging every album off its +/// first credited artist would otherwise merge two different records that share +/// a title and a lead: "Live" by `A; B` and "Live" by `A; C` are one key +/// without it. A single credit keeps the historical two-part shape, so the keys +/// already stored for the overwhelming majority of albums still match what +/// indexing computes. +fn album_identity( + canonical_title: &str, + album_artist_id: Option, + album_artist: Option<&str>, +) -> String { + let artist = album_artist_id + .map(|id| id.to_string()) + .unwrap_or_else(|| "none".into()); + let credits = split_values(album_artist); + if credits.len() > 1 { + let joined = waveflow_core::scanner::canonical_name(&credits.join(" ")); + format!("{canonical_title}:{artist}:{joined}") + } else { + format!("{canonical_title}:{artist}") + } } fn split_values(raw: Option<&str>) -> Vec { diff --git a/src/media.rs b/src/media.rs index 780aaef..b33680b 100644 --- a/src/media.rs +++ b/src/media.rs @@ -888,7 +888,10 @@ fn parse_range(value: &str, size: u64) -> Option<(u64, u64)> { if value.contains(',') { return None; } - let (start, end) = value.split_once('-')?; + let (start, end) = value.trim().split_once('-')?; + // Trimmed on both bounds, like `starts_at_the_first_byte`: a spelling the + // cold path accepts must not be refused once the cache exists. + let (start, end) = (start.trim(), end.trim()); if start.is_empty() { let suffix = end.parse::().ok()?.min(size).min(MAX_RANGE_BYTES); if suffix == 0 { @@ -1032,6 +1035,10 @@ mod tests { assert_eq!(parse_range("bytes=10-", 10), None); assert_eq!(parse_range("bytes=5-2", 10), None); assert_eq!(parse_range("bytes=0-1,4-5", 10), None); + // Whitespace is tolerated on both bounds, so that a range the cold + // transcode path accepts is still satisfiable from the cache. + assert_eq!(parse_range("bytes= 0- ", 10), Some((0, 9))); + assert_eq!(parse_range("bytes= 2 - 5 ", 10), Some((2, 5))); } #[test] diff --git a/tests/v2_foundations.rs b/tests/v2_foundations.rs index d54d47d..556d9d2 100644 --- a/tests/v2_foundations.rs +++ b/tests/v2_foundations.rs @@ -5028,6 +5028,44 @@ async fn an_album_hangs_off_its_first_credited_artist_not_the_joined_string() { merged.albums[0].artist_id, Some(names(state.clone()).await["Nova Kern"]) ); + + // Two records can share a title and a lead credit and still be different + // albums. Hanging both off the first artist would give them one identity + // key and silently merge them, so the key carries the rest of the credit. + let split = state + .db + .create_scan_job(library, Some(owner), "manual") + .await + .unwrap(); + state.db.start_scan_job(split, 2).await.unwrap(); + for (index, credit) in [("Nova Kern; Ivy Trench"), ("Nova Kern; Rue Delacour")] + .into_iter() + .enumerate() + { + let mut split_input = + browse_input(810 + index, "Facing", "Split Bill", credit, Some(1), None); + split_input.relative_path = format!("split-{index}.flac"); + split_input.quick_hash = format!("{:064x}", 83_000 + index); + split_input.full_hash = format!("{:064x}", 84_000 + index); + state + .db + .apply_catalog_track(library, split, &split_input, None, false) + .await + .unwrap(); + } + state.db.finish_scan_job(split, 0).await.unwrap(); + state.db.consolidate_musicbrainz_ids(library).await.unwrap(); + + let all = state.services.catalog_snapshot(owner, &[]).await.unwrap(); + let split_bills = all + .albums + .iter() + .filter(|album| album.title == "Split Bill") + .count(); + assert_eq!( + split_bills, 2, + "same title and same lead credit, different second artist: two albums" + ); } /// A release identifier belongs to the release, not to whichever file was From 524cd4b2df22a3d0f9418c97ba9e337834001c0d Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 19 Aug 2026 10:08:07 +0200 Subject: [PATCH 16/18] docs(subsonic): attribute the play-queue coverage to the run that has it Feishin's 2026-08-19 re-run never called savePlayQueue or getPlayQueue, so crediting it as client coverage contradicted the Juliet row naming itself the first to exercise them. The coverage is fixtures plus Feishin's historical 2026-08-02 run; Juliet is the one that called them in the replayed set. Signed-off-by: InstaZDLL --- docs/subsonic-compatibility.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/subsonic-compatibility.md b/docs/subsonic-compatibility.md index db62ca3..4ff00e7 100644 --- a/docs/subsonic-compatibility.md +++ b/docs/subsonic-compatibility.md @@ -6,9 +6,9 @@ Automated protocol coverage is enforced by `tests/v2_foundations.rs` for XML, JS |---|---:|---:|---:|---:|---:|---| | Symfonium | 14.1.0 | pass | pass | pass | pass | Re-run 2026-08-19 against the current contract, on an Android 17 emulator through an ephemeral `cloudflared` HTTPS tunnel. Every observable change of the six batches was checked against server state rather than against the client's own display: sleeve order in both the album and a synced playlist, two distinct album artists with no composite entity in the artist list, two album genres, a genre spelled four ways answering as one (4 tracks), the extended fields including `explicitStatus`, and a failed login decoded correctly now that it arrives as HTTP 200. User data round-tripped: track and album ratings, track/album/artist favorites, 21 scrobbles, a bookmark resumed at 37 s, three playlists owned by the authenticated user, and 13 Opus cache entries across the 64 and 128 kbit/s ceilings. The earlier 2026-08-09 run on a physical Android 17 device additionally covered API-key authentication, which this one did not re-run. | | Feishin | 1.15.1 | pass | pass | pass | pass | Re-run 2026-08-19 against the current contract, on Windows desktop. **This run found the two server defects fixed in the same change**: a cold transcode refused the `Range: bytes=0-` that a browser sends to open any resource, so playback failed on every track no earlier client had transcoded; and `createPlaylist` appended its `songId` values instead of replacing the list, so every playlist edit looked lost. Both were then replayed against the fixed server through a logging proxy and confirmed from server state: cold transcode, cache hit and seek all answer, a removal drops to one track, a reorder lands. The rest of the run exercised search (23 `search3`), 145 cover-art reads, genre browsing including 8 `getSongsByGenre`, artists, `getStarred`, `getRandomSongs`, 9 scrobbles, favorites on tracks/album/artists and ratings — all read back from the server rather than from the client. `getAlbumInfo`/`getAlbumInfo2` were never called by this version, so they stay unexercised here rather than counted as a pass, as do the bookmark and play-queue methods. `getTopSongs`, `getArtistInfo` and `getInternetRadioStations` answer empty containers, which is what this server has to say about them. | -| Substreamer | 8.0.91 | pass | pass | pass | pass* | Validated 2026-08-02 from the official release source: native playback, Opus/128 cache, playlist add, favorite, rating and scrobble. `*` Its playback queue is local-only and the client never calls `getPlayQueue`/`savePlayQueue`; those endpoints remain covered by fixtures and Feishin. | +| Substreamer | 8.0.91 | pass | pass | pass | pass* | Validated 2026-08-02 from the official release source: native playback, Opus/128 cache, playlist add, favorite, rating and scrobble. `*` Its playback queue is local-only and the client never calls `getPlayQueue`/`savePlayQueue`; those endpoints remain covered by fixtures, and by Feishin's 2026-08-02 run. | | DSub | 5.5.3 (F-Droid 208) | pass | pass | pass | pass | Re-run 2026-08-19 against the current contract, on an Android 17 emulator, read from `logcat -s RESTMusicService` and DSub's own disk cache rather than from its interface. **This run found the album-artist defect fixed in the same change**: an album credited to two artists hung off a third entity named after the joined string, and browsing to either real artist found no album. The rest passed: sleeve order in both browse modes, two album genres, one canonical `Jazz` answering 4 tracks, artwork, native FLAC and MP3 with seek, a failed login decoded as an error now that it arrives as HTTP 200, playlist create/add/remove/rename/delete — the removal going through `songIndexToRemove`, by index — and a rating. It settles a question Symfonium left open: DSub does submit the scrobble of the last track in a queue. Two of its three unique surfaces needed the brief corrected. The legacy `getAlbumList` is used **only** when "Browse By Tags" is off, `getAlbumList2` otherwise; both were exercised. And `maxBitRate` is never sent above the source — DSub emits `min(setting, track bitrate)` — so "ceiling above the source" is not expressible from this client; the two reachable cases are correct (128 ceiling on a 128 kbit/s MP3 streams natively, 64 transcodes). The generic `star?id` form resolved a track, an album and an artist, all three surviving a resync. Unresolved, and not a server fault: one FLAC (`Drift`) is fetched repeatedly and never completed by the client, though the server answers 200 with the exact `content-length` and the bytes arrive md5-identical to the source file through the same tunnel. Absent from the client, recorded as absent: the explicit marker, and any separate display of two album artists. | -| Juliet | 1.5 (iOS 26.6) | pass | pass | pass | pass | Re-run 2026-08-19 against the current contract, on a physical iPhone — the previous entry was a narrower check that never reached transcoding or user data. **This run found the second half of the cold-transcode defect**: iOS AVFoundation opens a resource with a `bytes=0-1` probe, which the first fix still classified as a seek and refused, so the first play of each track failed and the second succeeded off the cache the failed request had built. Fixed in the same change and verified by replaying the client's exact sequence: `bytes=0-1` cold answers 200, warm answers 206 with two bytes, a real seek answers 206, and a seek into a cold transcode is still refused. The rest of the run: 306 cover-art reads, 115 streams, 64 scrobbles, `getAlbumList2`, `search3`, `getRandomSongs`, `getSongsByGenre`, `getSimilarSongs2`, `getLyrics`, favorites, and playlist create/update/delete. It is also the first real client to exercise `savePlayQueue`/`getPlayQueue`, which the matrix had covered only through Feishin and fixtures. Absent from the client, recorded as absent: Juliet has no rating feature, so `setRating` was never sent; and it saves its queue with `position=0` every time, so playback restarts at zero — the server stores and returns what it is given, queue, current track and `changedBy` included. | +| Juliet | 1.5 (iOS 26.6) | pass | pass | pass | pass | Re-run 2026-08-19 against the current contract, on a physical iPhone — the previous entry was a narrower check that never reached transcoding or user data. **This run found the second half of the cold-transcode defect**: iOS AVFoundation opens a resource with a `bytes=0-1` probe, which the first fix still classified as a seek and refused, so the first play of each track failed and the second succeeded off the cache the failed request had built. Fixed in the same change and verified by replaying the client's exact sequence: `bytes=0-1` cold answers 200, warm answers 206 with two bytes, a real seek answers 206, and a seek into a cold transcode is still refused. The rest of the run: 306 cover-art reads, 115 streams, 64 scrobbles, `getAlbumList2`, `search3`, `getRandomSongs`, `getSongsByGenre`, `getSimilarSongs2`, `getLyrics`, favorites, and playlist create/update/delete. It is also the first client in the replayed set to call `savePlayQueue`/`getPlayQueue` - 48 times - where the matrix otherwise rests on fixtures and on Feishin's historical 2026-08-02 run, its 2026-08-19 re-run having never called them. Absent from the client, recorded as absent: Juliet has no rating feature, so `setRating` was never sent; and it saves its queue with `position=0` every time, so playback restarts at zero — the server stores and returns what it is given, queue, current track and `changedBy` included. | **Symfonium, Feishin, DSub and Juliet were all re-run on 2026-08-19 against the current contract**, each on a real device or desktop and each read from server From 6151cab0e41831291434a37361f10246347383ff Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 19 Aug 2026 10:16:21 +0200 Subject: [PATCH 17/18] fix(catalog): the album identity keeps its credit boundaries Canonicalising the joined credit erased where one credit ended and the next began: `A; B C` and `A; B; C` both flattened to the same words, so two records crediting different people answered one identity key and a scan merged them. Each credit is canonicalised on its own and rejoined on a separator the canonical form cannot produce, since canonical_name emits only alphanumerics and single spaces. Signed-off-by: InstaZDLL --- src/catalog.rs | 10 +++++++++- tests/v2_foundations.rs | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/catalog.rs b/src/catalog.rs index d2a9a65..37821bf 100644 --- a/src/catalog.rs +++ b/src/catalog.rs @@ -1078,7 +1078,15 @@ fn album_identity( .unwrap_or_else(|| "none".into()); let credits = split_values(album_artist); if credits.len() > 1 { - let joined = waveflow_core::scanner::canonical_name(&credits.join(" ")); + // Canonicalised one credit at a time and rejoined on a separator the + // canonical form cannot contain. Canonicalising the joined string + // instead would erase where one credit ends and the next begins, so + // `A; B C` and `A; B; C` would answer the same key and merge. + let joined = credits + .iter() + .map(|credit| waveflow_core::scanner::canonical_name(credit)) + .collect::>() + .join(";"); format!("{canonical_title}:{artist}:{joined}") } else { format!("{canonical_title}:{artist}") diff --git a/tests/v2_foundations.rs b/tests/v2_foundations.rs index 556d9d2..b3570ed 100644 --- a/tests/v2_foundations.rs +++ b/tests/v2_foundations.rs @@ -5066,6 +5066,46 @@ async fn an_album_hangs_off_its_first_credited_artist_not_the_joined_string() { split_bills, 2, "same title and same lead credit, different second artist: two albums" ); + + // Where the credits divide matters as much as what they say: canonicalising + // the joined string would flatten both of these to the same words and merge + // two records that credit different people. + let boundary = state + .db + .create_scan_job(library, Some(owner), "manual") + .await + .unwrap(); + state.db.start_scan_job(boundary, 2).await.unwrap(); + for (index, credit) in ["Vale; Ivy Trench", "Vale; Ivy; Trench"] + .into_iter() + .enumerate() + { + let mut edge = browse_input(820 + index, "Edge", "Boundary", credit, Some(1), None); + edge.relative_path = format!("boundary-{index}.flac"); + edge.quick_hash = format!("{:064x}", 85_000 + index); + edge.full_hash = format!("{:064x}", 86_000 + index); + state + .db + .apply_catalog_track(library, boundary, &edge, None, false) + .await + .unwrap(); + } + state.db.finish_scan_job(boundary, 0).await.unwrap(); + state.db.consolidate_musicbrainz_ids(library).await.unwrap(); + + assert_eq!( + state + .services + .catalog_snapshot(owner, &[]) + .await + .unwrap() + .albums + .iter() + .filter(|album| album.title == "Boundary") + .count(), + 2, + "`A; B C` and `A; B; C` are different credits and different albums" + ); } /// A release identifier belongs to the release, not to whichever file was From 725576a13649abf860ccc42bda04a27b06db026d Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 19 Aug 2026 11:22:22 +0200 Subject: [PATCH 18/18] test(catalog): drive the credit-boundary case from the album credit alone browse_input fills the track artist and the album artist from the same string, so the two boundary tracks could have been told apart by either one. They now share a track artist and differ only in their album credit, which is what the identity key is supposed to read. The assertion reads the two credits back rather than counting rows, so a merge is reported as which record was lost. Signed-off-by: InstaZDLL --- tests/v2_foundations.rs | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/tests/v2_foundations.rs b/tests/v2_foundations.rs index b3570ed..b3f48f8 100644 --- a/tests/v2_foundations.rs +++ b/tests/v2_foundations.rs @@ -5081,6 +5081,10 @@ async fn an_album_hangs_off_its_first_credited_artist_not_the_joined_string() { .enumerate() { let mut edge = browse_input(820 + index, "Edge", "Boundary", credit, Some(1), None); + // The two tracks share a track artist, so only the album credit can be + // what tells the records apart. + edge.artist = Some("Vale".to_owned()); + edge.album_artist = Some(credit.to_owned()); edge.relative_path = format!("boundary-{index}.flac"); edge.quick_hash = format!("{:064x}", 85_000 + index); edge.full_hash = format!("{:064x}", 86_000 + index); @@ -5093,17 +5097,24 @@ async fn an_album_hangs_off_its_first_credited_artist_not_the_joined_string() { state.db.finish_scan_job(boundary, 0).await.unwrap(); state.db.consolidate_musicbrainz_ids(library).await.unwrap(); + let boundaries = state + .services + .catalog_snapshot(owner, &[]) + .await + .unwrap() + .albums + .into_iter() + .filter(|album| album.title == "Boundary") + .filter_map(|album| album.artist) + .collect::>(); assert_eq!( - state - .services - .catalog_snapshot(owner, &[]) - .await - .unwrap() - .albums - .iter() - .filter(|album| album.title == "Boundary") - .count(), - 2, + boundaries, + [ + "Vale; Ivy Trench".to_owned(), + "Vale; Ivy; Trench".to_owned() + ] + .into_iter() + .collect::>(), "`A; B C` and `A; B; C` are different credits and different albums" ); }