fix: the four defects the client campaigns left open

docs/subsonic-compatibility.md

@@ -19,6 +19,14 @@ Automated protocol coverage is enforced by the `subsonic_contract`, `subsonic_br
 > Nothing the 2026-09-16 campaign found is a regression. Everything it surfaced
 > is reachable at `v2.0.0-beta.0` too, and is filed as #224, #225 and #226.
 >
+> **The rows record the runs, not today’s server.** #224, #225 and #226 were
+> fixed after these campaigns, so two observations below have since stopped
+> being true: `getPlayQueue` and `getNowPlaying` named their children `song`
+> when DSub and Juliet met them, and now name them `entry`, with the `username`
+> the schema requires beside them. That is an **observable wire change**, made
+> deliberately during a beta rather than carried into a stable tag — the freeze
+> protects the clients that were validated, not a divergence from the schema.
+>
 > **How the four runs were told apart.** They shared one server and one log, and
 > the trace records path and status but never query strings, so no request names
 > its sender. Each run is bounded by a burst in the request rate, corroborated

src/media.rs

@@ -96,7 +96,10 @@ pub enum MediaError {
     Unauthorized,
     NotFound,
     InvalidRequest,
-    RangeNotSatisfiable(u64),
+    /// The range could not be honoured. The length is `None` when the
+    /// representation does not have one yet — a transcode still being produced
+    /// knows how many bytes it has written, never how many it will.
+    RangeNotSatisfiable(Option<u64>),
     /// A quota the caller cannot spend past. Distinct from a refusal of the
     /// bytes themselves: nothing is wrong with what was sent, there is only no
     /// room for it.
@@ -209,7 +212,7 @@ impl MediaService {
         // 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));
+            return Err(MediaError::RangeNotSatisfiable(None));
         }
 
         if query.offset_ms > 0 {
@@ -1278,15 +1281,35 @@ impl IntoResponse for MediaError {
                 (StatusCode::TOO_MANY_REQUESTS, [(header::RETRY_AFTER, "1")]).into_response()
             }
             Self::RangeNotSatisfiable(size) => {
-                let content_range = format!("bytes */{size}");
-                (
-                    StatusCode::RANGE_NOT_SATISFIABLE,
-                    [
-                        (header::CONTENT_RANGE, content_range.as_str()),
-                        (header::ACCEPT_RANGES, "none"),
-                    ],
-                )
-                    .into_response()
+                // Two refusals wear this status, and they do not mean the same
+                // thing.
+                //
+                // A complete file knows its length, so it can say which range
+                // would have been satisfiable — and it accepts ranges, which
+                // every other answer `serve_file` gives for that same resource
+                // already says. Only the range asked for was wrong.
+                //
+                // A transcode still being produced knows how many bytes it has
+                // written, never how many it will, and takes no range until it
+                // is whole. `bytes */0` would not say "length unknown": it
+                // would say the resource is empty, which a client is entitled
+                // to believe and never ask about again. The unsatisfied-range
+                // form has nowhere to put an unknown length, so the header is
+                // left out instead of filled with a falsehood.
+                let mut response = StatusCode::RANGE_NOT_SATISFIABLE.into_response();
+                let headers = response.headers_mut();
+                match size {
+                    Some(size) => {
+                        headers.insert(header::ACCEPT_RANGES, HeaderValue::from_static("bytes"));
+                        if let Ok(value) = HeaderValue::from_str(&format!("bytes */{size}")) {
+                            headers.insert(header::CONTENT_RANGE, value);
+                        }
+                    }
+                    None => {
+                        headers.insert(header::ACCEPT_RANGES, HeaderValue::from_static("none"));
+                    }
+                }
+                response
             }
             Self::Internal => StatusCode::INTERNAL_SERVER_ERROR.into_response(),
         }
@@ -1371,7 +1394,7 @@ async fn serve_file(
     match range {
         Some(range) => {
             let (start, end) =
-                parse_range(range, size).ok_or(MediaError::RangeNotSatisfiable(size))?;
+                parse_range(range, size).ok_or(MediaError::RangeNotSatisfiable(Some(size)))?;
             serve_partial(file, start, end, size, mime).await
         }
         None => {

src/subsonic/protocol.rs

@@ -210,8 +210,8 @@ pub(super) fn json_array_field(parent: &str, name: &str) -> bool {
             | ("playlist", "entry")
             | ("bookmarks", "bookmark")
             | ("starred2" | "starred", "artist" | "album" | "song")
-            | ("nowPlaying", "song")
-            | ("playQueue", "song")
+            | ("nowPlaying", "entry")
+            | ("playQueue", "entry")
             | ("shares", "share")
             | ("share", "entry")
             | ("users", "user")
@@ -220,8 +220,9 @@ pub(super) fn json_array_field(parent: &str, name: &str) -> bool {
             | ("lyricsList", "structuredLyrics")
             | ("structuredLyrics", "line")
             // A media item is rendered as `song`, and renamed to `entry` inside
-            // a playlist or share and to `child` inside a directory. Its
-            // OpenSubsonic relations are arrays under all three names.
+            // a playlist, a share, a play queue or the now-playing list, and to
+            // `child` inside a directory. Its OpenSubsonic relations are arrays
+            // under all three names.
             | ("song" | "entry" | "child" | "album", "artists" | "genres")
             // `getMusicDirectory` renders an album as `child`, so its arrays
             // have to keep their shape under that name too — otherwise one

src/subsonic/userdata.rs

@@ -185,10 +185,15 @@ pub(super) async fn now_playing(
         .map_err(internal)?;
     Ok(
         Node::new("nowPlaying").children(entries.iter().map(|(username, song, started)| {
-            song_node(song).attr("username", username.clone()).attr(
-                "minutesAgo",
-                ((chrono::Utc::now().timestamp_millis() - started) / 60_000).max(0),
-            )
+            // `entry`, not `song`: the schema renames a media item inside this
+            // container exactly as it does inside a playlist.
+            song_node(song)
+                .renamed("entry")
+                .attr("username", username.clone())
+                .attr(
+                    "minutesAgo",
+                    ((chrono::Utc::now().timestamp_millis() - started) / 60_000).max(0),
+                )
         })),
     )
 }
@@ -198,14 +203,24 @@ pub(super) async fn get_queue(
     principal: &Principal,
 ) -> Result<Node, ProtocolError> {
     let Some(queue) = state.services.queue(principal.id).await.map_err(internal)? else {
+        // Nothing has been saved. The empty container carries no `username`
+        // either, because `changed` and `changedBy` would have to be invented
+        // beside it, and a queue that does not exist has no such facts.
         return Ok(Node::new("playQueue"));
     };
     Ok(Node::new("playQueue")
+        .attr("username", principal.username.clone())
         .maybe_attr("current", queue.current.map(|id| id.to_string()))
         .attr("position", queue.position_ms)
         .maybe_attr("changedBy", queue.changed_by)
         .attr("changed", iso_time(queue.updated_at))
-        .children(queue.songs.iter().map(song_node)))
+        // `entry`, not `song` — see `nowPlaying` above.
+        .children(
+            queue
+                .songs
+                .iter()
+                .map(|song| song_node(song).renamed("entry")),
+        ))
 }
 
 pub(super) async fn save_queue(

src/tags.rs

@@ -113,14 +113,49 @@ const ARTIST_SEPARATORS: [&str; 6] = [" / ", " feat. ", " feat ", " ft. ", " ft
 /// composer's name contains a slash.
 const ROLE_SEPARATORS: [&str; 2] = ["/", ";"];
 
+/// Cuts `value` on any of `separators`, but never inside parentheses.
+///
+/// A separator nested in a parenthesis does not divide two credits: it divides
+/// what qualifies one. `Kobee (Melange / INHOUSE), Holy M (Melange / INHOUSE)`
+/// names two people, and cutting at those slashes made three entities, each
+/// carrying a parenthesis it never opened — `Kobee (Melange`,
+/// `INHOUSE), Holy M (Melange`. They reached the catalogue as artists, and
+/// search answered with them.
+///
+/// The separators are tried in the order given at every position, which is what
+/// keeps ` feat. ` ahead of ` feat `. Depth is clamped at zero so a stray
+/// closing parenthesis cannot make the rest of the value uncuttable, and an
+/// opening one that is never closed simply protects the tail it opened.
 fn split_on(value: &str, separators: &[&str]) -> Vec<String> {
-    let mut parts = vec![value.to_owned()];
-    for separator in separators {
-        parts = parts
-            .into_iter()
-            .flat_map(|part| part.split(separator).map(str::to_owned).collect::<Vec<_>>())
-            .collect();
+    let mut parts = Vec::new();
+    let mut depth: usize = 0;
+    let mut start = 0;
+    let mut cursor = 0;
+    while cursor < value.len() {
+        let rest = &value[cursor..];
+        if depth == 0 {
+            if let Some(separator) = separators
+                .iter()
+                .find(|separator| rest.starts_with(**separator))
+            {
+                parts.push(&value[start..cursor]);
+                cursor += separator.len();
+                start = cursor;
+                continue;
+            }
+        }
+        let character = rest
+            .chars()
+            .next()
+            .expect("the cursor only ever lands on a character boundary");
+        match character {
+            '(' => depth += 1,
+            ')' => depth = depth.saturating_sub(1),
+            _ => {}
+        }
+        cursor += character.len_utf8();
     }
+    parts.push(&value[start..]);
     parts
         .into_iter()
         .map(|part| part.trim().to_owned())
@@ -386,6 +421,72 @@ mod tests {
         );
     }
 
+    /// A separator inside parentheses qualifies one credit; it does not divide
+    /// two. The real tag that exposed this named two people and produced three
+    /// entities, each carrying a parenthesis it never opened.
+    #[test]
+    fn a_role_separator_inside_parentheses_does_not_cut() {
+        let raw = RawCredits {
+            artist: vec!["ITZY".into()],
+            roles: vec![(
+                Role::Composer,
+                vec!["Kobee (Melange / INHOUSE), Holy M (Melange / INHOUSE)".into()],
+            )],
+            ..RawCredits::default()
+        };
+        assert_eq!(
+            names_of(&credits(&raw), Role::Composer),
+            vec!["Kobee (Melange / INHOUSE), Holy M (Melange / INHOUSE)"]
+        );
+    }
+
+    /// The parenthesis rule must not cost the cut it was added beside: a
+    /// separator outside one still divides, in the same value.
+    #[test]
+    fn a_role_separator_outside_parentheses_still_cuts() {
+        let raw = RawCredits {
+            artist: vec!["Nobody".into()],
+            roles: vec![(
+                Role::Composer,
+                vec!["Bach (arr. Gounod)/Liszt (after Bach)".into()],
+            )],
+            ..RawCredits::default()
+        };
+        assert_eq!(
+            names_of(&credits(&raw), Role::Composer),
+            vec!["Bach (arr. Gounod)", "Liszt (after Bach)"]
+        );
+    }
+
+    /// A tag whose parentheses never balance must not swallow the whole value
+    /// or hang the scan. The opening one protects only the tail it opened.
+    #[test]
+    fn an_unbalanced_parenthesis_protects_only_its_tail() {
+        let unclosed = RawCredits {
+            artist: vec!["Nobody".into()],
+            roles: vec![(
+                Role::Composer,
+                vec!["Bach/Gounod (Melange / INHOUSE".into()],
+            )],
+            ..RawCredits::default()
+        };
+        assert_eq!(
+            names_of(&credits(&unclosed), Role::Composer),
+            vec!["Bach", "Gounod (Melange / INHOUSE"]
+        );
+
+        // A stray closing parenthesis must not make the rest uncuttable.
+        let stray = RawCredits {
+            artist: vec!["Nobody".into()],
+            roles: vec![(Role::Composer, vec!["INHOUSE)/Bach".into()])],
+            ..RawCredits::default()
+        };
+        assert_eq!(
+            names_of(&credits(&stray), Role::Composer),
+            vec!["INHOUSE)", "Bach"]
+        );
+    }
+
     /// A role tag is cut on the bare separators, where an artist tag is not.
     #[test]
     fn a_role_tag_is_cut_where_an_artist_tag_would_not_be() {

tests/media.rs

@@ -102,6 +102,26 @@ async fn media_streaming_ranges_transcodes_caches_and_isolates_tenants() {
         .await
         .unwrap();
     assert_eq!(unsatisfiable.status(), StatusCode::RANGE_NOT_SATISFIABLE);
+    // The file has a length, so the refusal states which range would have been
+    // satisfiable — and states the real one, read from the fixture rather than
+    // written down, so the assertion cannot drift away from the file.
+    let length = std::fs::metadata(music.join("Range.wav")).unwrap().len();
+    assert_eq!(
+        unsatisfiable
+            .headers()
+            .get("content-range")
+            .and_then(|value| value.to_str().ok()),
+        Some(format!("bytes */{length}").as_str())
+    );
+    // And it still accepts ranges: only the range asked for was wrong. Saying
+    // `none` here would contradict every other answer this file gives.
+    assert_eq!(
+        unsatisfiable
+            .headers()
+            .get("accept-ranges")
+            .and_then(|value| value.to_str().ok()),
+        Some("bytes")
+    );
 
     let hidden = router
         .clone()
@@ -300,6 +320,24 @@ async fn media_streaming_ranges_transcodes_caches_and_isolates_tenants() {
         .await
         .unwrap();
     assert_eq!(cold_seek.status(), StatusCode::RANGE_NOT_SATISFIABLE);
+    // Nothing has been produced yet, so there is no complete length to give.
+    // The unsatisfied-range form has nowhere to put "unknown", and `bytes */0`
+    // is not unknown — it is a claim that the resource is empty, which a client
+    // is entitled to believe and never ask again. The header is omitted.
+    assert!(
+        cold_seek.headers().get("content-range").is_none(),
+        "a refusal with no known length must not state one: {:?}",
+        cold_seek.headers().get("content-range")
+    );
+    // This one genuinely takes no range until the transcode is whole, which is
+    // the other half of the same distinction.
+    assert_eq!(
+        cold_seek
+            .headers()
+            .get("accept-ranges")
+            .and_then(|value| value.to_str().ok()),
+        Some("none")
+    );
 
     let live_seek = router
         .clone()