From fa49ebd2ebc66d22cbe1eb31afe57647a14c8885 Mon Sep 17 00:00:00 2001 From: Michel de Graaf Date: Fri, 28 Aug 2026 12:44:31 +0000 Subject: [PATCH 1/4] feat(lib): per-member room statistics from the JoinRoom reply The JoinRoom (code 14) reply carries, after the membership list, four parallel vectors describing each member: status, share statistics, free upload slots and country. The handler stopped after the usernames, so all of it was discarded. It is now parsed and exposed as Client::room_member_stats. Issue #48 asked for RoomStats (57) and RoomListStats (55). Those codes are UserInterests and MyRecommendations in the Soulseek protocol; there is no RoomStats message on the wire. Per-room user counts already arrive with RoomList (64), which the client handles, and the per-member statistics are part of the JoinRoom reply -- so that is where this adds them. - join_room.rs: parse the stat vectors, each bounded by the payload so a hostile count cannot hang or overallocate - types::RoomUserStats; every field looked up positionally so a server sending short vectors (or omitting countries) does not shift the roster - ServerMessage/ClientOperation::RoomMemberStats and the context snapshot behind Client::room_member_stats, replaced on each rejoin --- soulseek-rs-lib/src/actor/server_actor.rs | 14 +- soulseek-rs-lib/src/client/mod.rs | 35 ++- soulseek-rs-lib/src/client/operations.rs | 10 + soulseek-rs-lib/src/client/rooms.rs | 20 +- soulseek-rs-lib/src/client/tests.rs | 50 ++++ .../src/message/server/join_room.rs | 259 +++++++++++++++++- soulseek-rs-lib/src/types.rs | 19 ++ 7 files changed, 391 insertions(+), 16 deletions(-) diff --git a/soulseek-rs-lib/src/actor/server_actor.rs b/soulseek-rs-lib/src/actor/server_actor.rs index 57c1778..e5193b1 100644 --- a/soulseek-rs-lib/src/actor/server_actor.rs +++ b/soulseek-rs-lib/src/actor/server_actor.rs @@ -27,7 +27,8 @@ use crate::message::{Message, MessageReader}; use crate::peer::ConnectionType; use crate::peer::Peer; use crate::types::{ - ClientVersion, RoomEvent, RoomInfo, SessionLoss, SessionWatch, + ClientVersion, RoomEvent, RoomInfo, RoomUserStats, SessionLoss, + SessionWatch, }; use crate::utils::lock::RwLockExt; @@ -213,6 +214,11 @@ pub enum ServerMessage { room: String, users: Vec, }, + /// Per-member statistics carried by the same `JoinRoom` (code 14) reply. + RoomMemberStats { + room: String, + stats: Vec, + }, RoomLeft { room: String, }, @@ -480,6 +486,12 @@ impl ServerActor { ServerMessage::RoomJoined { room, users } => { self.forward_room_event(RoomEvent::Joined { room, users }); } + ServerMessage::RoomMemberStats { room, stats } => { + self.forward_to_client(ClientOperation::RoomMemberStats { + room, + stats, + }); + } ServerMessage::RoomLeft { room } => { self.forward_room_event(RoomEvent::Left { room }); } diff --git a/soulseek-rs-lib/src/client/mod.rs b/soulseek-rs-lib/src/client/mod.rs index 8d11cb4..f804eac 100644 --- a/soulseek-rs-lib/src/client/mod.rs +++ b/soulseek-rs-lib/src/client/mod.rs @@ -5,7 +5,8 @@ use crate::actor::server_actor::{ use crate::download_store::{DownloadStore, collect_failed_tokens}; use crate::types::{ ClientVersion, DownloadMetadata, DownloadStatus, RoomEvent, RoomInfo, - SessionLoss, SessionWatch, UserInfo, UserPresence, UserStats, UserStatus, + RoomUserStats, SessionLoss, SessionWatch, UserInfo, UserPresence, + UserStats, UserStatus, }; use crate::utils::logger; use crate::{ @@ -278,6 +279,11 @@ pub enum ClientOperation { /// Something happened in the chat-room subsystem (list refreshed, a room /// joined/left, a message said, a member joined/left). RoomEvent(RoomEvent), + /// Per-member statistics from a `JoinRoom` reply. + RoomMemberStats { + room: String, + stats: Vec, + }, /// The server announced how many seconds must pass between wishlist /// searches. WishlistInterval(u32), @@ -325,6 +331,9 @@ pub struct ClientContext { /// Who is in each room we have joined, kept current from the membership /// the server sends on join plus the later join/leave events. room_members: HashMap>, + /// Per-member statistics for each joined room, from the stat vectors the + /// server sends alongside the membership list. + room_member_stats: HashMap>, /// What the server has told us about other users, merged across the /// separate status and statistics replies. user_info: HashMap, @@ -415,6 +424,7 @@ impl ClientContext { room_list: Vec::new(), room_events: Vec::new(), room_members: HashMap::new(), + room_member_stats: HashMap::new(), user_info: HashMap::new(), wishlist_interval: None, privileged_users: HashSet::new(), @@ -515,6 +525,29 @@ impl ClientContext { self.room_members.get(room).cloned().unwrap_or_default() } + /// Record the per-member statistics of a room we just joined, replacing + /// any previous snapshot for it. + pub fn apply_room_member_stats( + &mut self, + room: String, + stats: Vec, + ) { + self.room_member_stats.insert(room, stats); + } + + /// What the server reported about the members of `room`, sorted by name, + /// or empty for a room we have not joined. + #[must_use] + pub fn room_member_stats(&self, room: &str) -> Vec { + let mut stats = self + .room_member_stats + .get(room) + .cloned() + .unwrap_or_default(); + stats.sort_by(|a, b| a.username.cmp(&b.username)); + stats + } + /// The latest snapshot of the public chat-room list. #[must_use] pub fn room_list(&self) -> Vec { diff --git a/soulseek-rs-lib/src/client/operations.rs b/soulseek-rs-lib/src/client/operations.rs index d80758f..e4f2857 100644 --- a/soulseek-rs-lib/src/client/operations.rs +++ b/soulseek-rs-lib/src/client/operations.rs @@ -479,6 +479,16 @@ impl Client { Err(e) => error!("[client] RoomEvent write: {}", e), } } + ClientOperation::RoomMemberStats { room, stats } => { + match client_context.write_safe() { + Ok(mut ctx) => { + ctx.apply_room_member_stats(room, stats); + } + Err(e) => { + error!("[client] RoomMemberStats write: {}", e); + } + } + } ClientOperation::WishlistInterval(seconds) => { if let Ok(mut ctx) = client_context.write_safe() { ctx.wishlist_interval = Some(seconds); diff --git a/soulseek-rs-lib/src/client/rooms.rs b/soulseek-rs-lib/src/client/rooms.rs index 8afc871..f1b15de 100644 --- a/soulseek-rs-lib/src/client/rooms.rs +++ b/soulseek-rs-lib/src/client/rooms.rs @@ -2,7 +2,7 @@ use super::{ Client, PeerMessage, Result, RoomEvent, RoomInfo, RwLockExt, ServerMessage, SharedDirectory, SoulseekRs, UserMessage, error, }; -use crate::types::UserInfo; +use crate::types::{RoomUserStats, UserInfo}; impl Client { /// Send a private message to another user via the server. @@ -156,6 +156,24 @@ impl Client { } } + /// What the server reported about the members of `room` when we joined: + /// their status, share statistics, free upload slots and country. + /// + /// Populated from the same reply that fills [`Client::room_members`], so + /// it lands shortly after joining. Unlike the roster it is not updated by + /// later join and leave events -- it is the snapshot taken at join time. + /// Returns an empty list for a room this client has not joined. + #[must_use] + pub fn room_member_stats(&self, room: &str) -> Vec { + match self.context.read_safe() { + Ok(ctx) => ctx.room_member_stats(room), + Err(e) => { + error!("[client] room_member_stats: {}", e); + Vec::new() + } + } + } + /// Remove and return all chat-room events received since the last call. #[must_use] pub fn take_room_events(&self) -> Vec { diff --git a/soulseek-rs-lib/src/client/tests.rs b/soulseek-rs-lib/src/client/tests.rs index fd90b76..aa29d45 100644 --- a/soulseek-rs-lib/src/client/tests.rs +++ b/soulseek-rs-lib/src/client/tests.rs @@ -622,3 +622,53 @@ fn a_replayed_transfer_response_does_not_start_a_second_transfer() { "a replayed TransferResponse must not dial the peer again" ); } + +#[test] +fn room_member_stats_are_returned_sorted_and_scoped_to_their_room() { + use crate::types::{RoomUserStats, UserStatus}; + let stat = |username: &str| RoomUserStats { + username: username.to_string(), + status: UserStatus::Online, + average_speed: 1, + shared_files: 2, + shared_folders: 3, + slots_free: 1, + country: None, + }; + + let mut context = ClientContext::new(); + context.apply_room_member_stats( + "jazz".to_string(), + vec![stat("carol"), stat("alice")], + ); + + let names: Vec = context + .room_member_stats("jazz") + .into_iter() + .map(|s| s.username) + .collect(); + assert_eq!(names, vec!["alice", "carol"]); + assert!(context.room_member_stats("unjoined").is_empty()); +} + +#[test] +fn rejoining_replaces_the_previous_member_stats() { + use crate::types::{RoomUserStats, UserStatus}; + let stat = |username: &str| RoomUserStats { + username: username.to_string(), + status: UserStatus::Online, + average_speed: 1, + shared_files: 2, + shared_folders: 3, + slots_free: 0, + country: None, + }; + + let mut context = ClientContext::new(); + context.apply_room_member_stats("jazz".to_string(), vec![stat("alice")]); + context.apply_room_member_stats("jazz".to_string(), vec![stat("bob")]); + + let stats = context.room_member_stats("jazz"); + assert_eq!(stats.len(), 1, "a rejoin supersedes the old snapshot"); + assert_eq!(stats[0].username, "bob"); +} diff --git a/soulseek-rs-lib/src/message/server/join_room.rs b/soulseek-rs-lib/src/message/server/join_room.rs index b1fb385..0e48716 100644 --- a/soulseek-rs-lib/src/message/server/join_room.rs +++ b/soulseek-rs-lib/src/message/server/join_room.rs @@ -1,6 +1,7 @@ use crate::{ actor::server_actor::ServerMessage, message::{Message, MessageHandler}, + types::{RoomUserStats, UserStatus}, }; use std::sync::mpsc::Sender; @@ -12,24 +13,104 @@ impl MessageHandler for JoinRoomHandler { } fn handle(&self, message: &mut Message, sender: Sender) { - // JoinRoom (code 14): room name, then a vector of member usernames. - // Per-user stat vectors follow but are not needed here, so we stop - // after reading the names. + // JoinRoom (code 14): room name, the member usernames, then the + // parallel per-member stat vectors. let room = message.read_string(); - let user_count = message.read_int32(); - let mut users = Vec::new(); - for _ in 0..user_count { - // Guard against a hostile user_count outrunning the payload; each - // username is a length-prefixed string (>= 4 bytes). - if message.get_pointer() + 4 > message.get_size() { - break; - } - users.push(message.read_string()); + let users = read_strings(message); + let stats = parse_member_stats(message, &users); + let _ = sender.send(ServerMessage::RoomJoined { + room: room.clone(), + users, + }); + if !stats.is_empty() { + let _ = sender.send(ServerMessage::RoomMemberStats { room, stats }); } - let _ = sender.send(ServerMessage::RoomJoined { room, users }); } } +/// Read a length-prefixed vector of strings, stopping early if the count +/// outruns the payload: a bogus (possibly hostile) count must not spin us +/// into an OOM allocation loop. A 4-byte floor is the smallest a +/// length-prefixed string can be. +fn read_strings(message: &mut Message) -> Vec { + let count = message.read_int32(); + let mut values = Vec::new(); + for _ in 0..count { + if message.get_pointer() + 4 > message.get_size() { + break; + } + values.push(message.read_string()); + } + values +} + +/// Read a length-prefixed vector of 32-bit values, bounded the same way. +fn read_int32s(message: &mut Message) -> Vec { + let count = message.read_int32(); + let mut values = Vec::new(); + for _ in 0..count { + if message.get_pointer() + 4 > message.get_size() { + break; + } + values.push(message.read_int32()); + } + values +} + +/// Parse the per-member statistics that follow the membership list of a +/// `JoinRoom` (code 14) reply: statuses, then a record of share statistics per +/// member, then free upload slots, then country codes. +/// +/// Each vector is independently length-prefixed and a server may send fewer +/// entries than there are members (or, on an older server, omit the country +/// vector entirely), so every field is looked up positionally and defaults +/// when absent rather than shifting the whole roster. +#[must_use] +fn parse_member_stats( + message: &mut Message, + users: &[String], +) -> Vec { + let statuses = read_int32s(message); + + let stat_count = message.read_int32(); + let mut shares = Vec::new(); + for _ in 0..stat_count { + // A share record is 20 bytes: speed, an obsolete 64-bit upload + // number, files and folders. + if message.get_pointer() + 20 > message.get_size() { + break; + } + let average_speed = message.read_int32(); + let _upload_number = message.read_int64(); // obsolete + let shared_files = message.read_int32(); + let shared_folders = message.read_int32(); + shares.push((average_speed, shared_files, shared_folders)); + } + + let slots_free = read_int32s(message); + let countries = read_strings(message); + + users + .iter() + .enumerate() + .map(|(i, username)| { + let (average_speed, shared_files, shared_folders) = + shares.get(i).copied().unwrap_or((0, 0, 0)); + RoomUserStats { + username: username.clone(), + status: UserStatus::from_code( + statuses.get(i).copied().unwrap_or(0), + ), + average_speed, + shared_files, + shared_folders, + slots_free: slots_free.get(i).copied().unwrap_or(0), + country: countries.get(i).cloned().filter(|c| !c.is_empty()), + } + }) + .collect() +} + #[cfg(test)] mod tests { use super::*; @@ -53,6 +134,158 @@ mod tests { } } + /// A full JoinRoom payload: room, members, then the four parallel stat + /// vectors the server sends for them. + fn join_payload() -> Message { + framed(|m| { + m.write_string("nicotine") + .write_int32(2) + .write_string("alice") + .write_string("bob") + // statuses + .write_int32(2) + .write_int32(2) + .write_int32(1) + // share records + .write_int32(2) + .write_int32(1024) + .write_int64(0) + .write_int32(4321) + .write_int32(77) + .write_int32(50) + .write_int64(0) + .write_int32(10) + .write_int32(2) + // free slots + .write_int32(2) + .write_int32(1) + .write_int32(0) + // countries + .write_int32(2) + .write_string("NL") + .write_string(""); + }) + } + + #[test] + fn member_stats_are_parsed_alongside_the_roster() { + let (tx, rx) = std::sync::mpsc::channel(); + JoinRoomHandler.handle(&mut join_payload(), tx); + + assert!(matches!( + rx.try_recv(), + Ok(ServerMessage::RoomJoined { .. }) + )); + match rx.try_recv() { + Ok(ServerMessage::RoomMemberStats { room, stats }) => { + assert_eq!(room, "nicotine"); + assert_eq!( + stats, + vec![ + RoomUserStats { + username: "alice".to_string(), + status: UserStatus::Online, + average_speed: 1024, + shared_files: 4321, + shared_folders: 77, + slots_free: 1, + country: Some("NL".to_string()), + }, + RoomUserStats { + username: "bob".to_string(), + status: UserStatus::Away, + average_speed: 50, + shared_files: 10, + shared_folders: 2, + slots_free: 0, + country: None, + }, + ] + ); + } + other => panic!("unexpected: {other:?}"), + } + } + + #[test] + fn a_server_that_omits_the_stat_vectors_still_yields_a_roster() { + // Nothing after the membership list: every stat reads as a default + // rather than shifting the roster or dropping members. + let (tx, rx) = std::sync::mpsc::channel(); + let mut message = framed(|m| { + m.write_string("jazz").write_int32(1).write_string("alice"); + }); + + JoinRoomHandler.handle(&mut message, tx); + assert!(matches!( + rx.try_recv(), + Ok(ServerMessage::RoomJoined { .. }) + )); + match rx.try_recv() { + Ok(ServerMessage::RoomMemberStats { stats, .. }) => { + assert_eq!(stats.len(), 1); + assert_eq!(stats[0].username, "alice"); + assert_eq!(stats[0].average_speed, 0); + assert_eq!(stats[0].country, None); + } + other => panic!("unexpected: {other:?}"), + } + } + + #[test] + fn short_stat_vectors_do_not_shift_the_roster() { + // Stats for the first member only: the second must still be reported, + // with defaults, rather than inheriting the first member's figures. + let (tx, rx) = std::sync::mpsc::channel(); + let mut message = framed(|m| { + m.write_string("jazz") + .write_int32(2) + .write_string("alice") + .write_string("bob") + .write_int32(1) + .write_int32(2) + .write_int32(1) + .write_int32(1024) + .write_int64(0) + .write_int32(9) + .write_int32(1) + .write_int32(0) + .write_int32(0); + }); + + JoinRoomHandler.handle(&mut message, tx); + let _ = rx.try_recv(); + match rx.try_recv() { + Ok(ServerMessage::RoomMemberStats { stats, .. }) => { + assert_eq!(stats.len(), 2); + assert_eq!(stats[0].shared_files, 9); + assert_eq!(stats[1].username, "bob"); + assert_eq!(stats[1].shared_files, 0); + assert_eq!(stats[1].status, UserStatus::Offline); + } + other => panic!("unexpected: {other:?}"), + } + } + + #[test] + fn hostile_stat_counts_do_not_hang_or_overallocate() { + // Vectors claiming ~4 billion entries in a tiny frame must return + // promptly, bounded by the payload. + let (tx, rx) = std::sync::mpsc::channel(); + let mut message = framed(|m| { + m.write_string("jazz") + .write_int32(1) + .write_string("alice") + .write_int32(u32::MAX); + }); + + JoinRoomHandler.handle(&mut message, tx); + assert!(matches!( + rx.try_recv(), + Ok(ServerMessage::RoomJoined { .. }) + )); + } + #[test] fn forwards_room_and_member_list() { let (tx, rx) = std::sync::mpsc::channel(); diff --git a/soulseek-rs-lib/src/types.rs b/soulseek-rs-lib/src/types.rs index b10b44b..7fd3e1f 100644 --- a/soulseek-rs-lib/src/types.rs +++ b/soulseek-rs-lib/src/types.rs @@ -328,6 +328,25 @@ impl UserInfo { } } +/// What the server reports about one member of a room we joined, from the +/// per-member stat vectors of a `JoinRoom` (code 14) reply. +/// +/// A server may send shorter stat vectors than the membership list (older +/// servers omit country codes entirely), so absent fields read as zero and an +/// unreported country is `None` rather than an empty string. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct RoomUserStats { + pub username: String, + pub status: UserStatus, + pub average_speed: u32, + pub shared_files: u32, + pub shared_folders: u32, + /// Upload slots the member has free; zero means they are busy. + pub slots_free: u32, + /// Two-letter country code, when the server reports one. + pub country: Option, +} + /// A public chat room advertised by the server (`RoomList`, code 64). #[derive(Debug, Clone, PartialEq, Eq)] pub struct RoomInfo { From 60f7100e309d98f4b0749fa7adfb8cd26ae637d4 Mon Sep 17 00:00:00 2001 From: Michel de Graaf Date: Fri, 28 Aug 2026 13:27:09 +0000 Subject: [PATCH 2/4] feat(daemon): publish room member statistics over RPC and the CLI The statistics parsed from the JoinRoom reply were reachable only from the library. They answer the question `room users` already exists to ask -- who is in here, and are they worth downloading from -- so they belong on its output rather than behind a second call. - proto: room.member_stats, with a RoomUserStatsDto carrying status, share counts, free slots and country - SessionApi::room_member_stats, so local and remote sessions stay interchangeable; the status word is decoded through one shared helper rather than a third copy of the match - `room users` now emits status, average_speed, shared_files, shared_folders, slots_free and country, each null when the server said nothing, documented in SKILL.md - docs/openrpc.json regenerated from the wire types --- soulseek-rs/docs/openrpc.json | 82 ++++++++++++++++++++++++++ soulseek-rs/src/SKILL.md | 6 +- soulseek-rs/src/api.rs | 9 ++- soulseek-rs/src/commands/social.rs | 11 ++++ soulseek-rs/src/daemon/openrpc.rs | 20 ++++--- soulseek-rs/src/daemon/proto/dto.rs | 61 +++++++++++++++++-- soulseek-rs/src/daemon/proto/method.rs | 8 +++ soulseek-rs/src/daemon/proto/mod.rs | 1 + soulseek-rs/src/daemon/server/mod.rs | 69 ++++++++++++++++++++-- soulseek-rs/src/output.rs | 21 ++++++- soulseek-rs/src/remote/mod.rs | 21 +++++-- soulseek-rs/src/ui/main_tui/mod.rs | 6 ++ 12 files changed, 288 insertions(+), 27 deletions(-) diff --git a/soulseek-rs/docs/openrpc.json b/soulseek-rs/docs/openrpc.json index 213bfc7..d8b6ead 100644 --- a/soulseek-rs/docs/openrpc.json +++ b/soulseek-rs/docs/openrpc.json @@ -525,6 +525,21 @@ ], "type": "object" }, + "MemberStats": { + "description": "What the server said about each member of a room when it was joined.", + "properties": { + "members": { + "items": { + "$ref": "#/components/schemas/RoomUserStatsDto" + }, + "type": "array" + } + }, + "required": [ + "members" + ], + "type": "object" + }, "Members": { "properties": { "users": { @@ -733,6 +748,53 @@ ], "type": "object" }, + "RoomUserStatsDto": { + "description": "One member of a room, as the JoinRoom reply described them.", + "properties": { + "average_speed": { + "format": "uint32", + "minimum": 0, + "type": "integer" + }, + "country": { + "type": [ + "string", + "null" + ] + }, + "shared_files": { + "format": "uint32", + "minimum": 0, + "type": "integer" + }, + "shared_folders": { + "format": "uint32", + "minimum": 0, + "type": "integer" + }, + "slots_free": { + "format": "uint32", + "minimum": 0, + "type": "integer" + }, + "status": { + "description": "`online`, `away`, or `offline`.", + "type": "string" + }, + "username": { + "type": "string" + } + }, + "required": [ + "username", + "status", + "average_speed", + "shared_files", + "shared_folders", + "slots_free" + ], + "type": "object" + }, "SayParams": { "properties": { "message": { @@ -1776,6 +1838,26 @@ }, "summary": "Who is currently in a room." }, + { + "name": "room.member_stats", + "paramStructure": "by-name", + "params": [ + { + "name": "room", + "required": true, + "schema": { + "type": "string" + } + } + ], + "result": { + "name": "result", + "schema": { + "$ref": "#/components/schemas/MemberStats" + } + }, + "summary": "What the server said about each member of a joined room." + }, { "name": "message.send", "paramStructure": "by-name", diff --git a/soulseek-rs/src/SKILL.md b/soulseek-rs/src/SKILL.md index 44594b3..dc3f173 100644 --- a/soulseek-rs/src/SKILL.md +++ b/soulseek-rs/src/SKILL.md @@ -140,7 +140,11 @@ cheapest way to prove the credentials and connection work. server did not answer — those are different facts, so do not treat null as 0. **`room list`** — `room`, `users`. -**`room users `** — `room`, `user`. +**`room users `** — `room`, `user`, `status`, `average_speed`, +`shared_files`, `shared_folders`, `slots_free`, `country`. Everything but +`room` and `user` is `null` when the server sent no statistics for that +member — `null` means unknown, not zero. `slots_free` of 0 means the peer has +no free upload slot right now, so a download from them will queue. **`room listen `** — streams `type`, `room`, `user`, `message`. **`room say `** — prints nothing; exit 0 is the confirmation. diff --git a/soulseek-rs/src/api.rs b/soulseek-rs/src/api.rs index 4006a84..a2a52d0 100644 --- a/soulseek-rs/src/api.rs +++ b/soulseek-rs/src/api.rs @@ -13,7 +13,7 @@ //! remote session has no borrowed client to hand a `&str` out of. use crate::daemon::proto::ChatMessageDto; -use soulseek_rs::types::{Download, DownloadMetadata}; +use soulseek_rs::types::{Download, DownloadMetadata, RoomUserStats}; use soulseek_rs::{ Client, DownloadStatus, Result, RoomEvent, SearchResult, SessionLoss, SharedDirectory, UploadInfo, UserInfo, UserMessage, @@ -126,6 +126,9 @@ pub trait SessionApi: Send + Sync { fn leave_room(&self, room: &str) -> Result<()>; fn say_in_room(&self, room: &str, message: &str) -> Result<()>; fn room_members(&self, room: &str) -> Vec; + /// What the server said about each member when the room was joined: + /// status, share statistics, free upload slots and country. + fn room_member_stats(&self, room: &str) -> Vec; fn take_room_events(&self) -> Vec; fn send_private_message(&self, username: &str, message: &str) @@ -329,6 +332,10 @@ impl SessionApi for Client { Self::room_members(self, room) } + fn room_member_stats(&self, room: &str) -> Vec { + Self::room_member_stats(self, room) + } + fn take_room_events(&self) -> Vec { Self::take_room_events(self) } diff --git a/soulseek-rs/src/commands/social.rs b/soulseek-rs/src/commands/social.rs index 1bdde18..6c33a4d 100644 --- a/soulseek-rs/src/commands/social.rs +++ b/soulseek-rs/src/commands/social.rs @@ -151,10 +151,21 @@ pub fn room_users(ctx: &Ctx, room: &str, timeout: Duration) -> CliResult { timeout.as_secs() ))); }; + // The stats ride along with the membership the server sent, so they are + // already here; a server that sent none leaves every field null rather + // than reporting a zero that reads like an answer. + let stats = session.client.room_member_stats(room); for user in members { + let stat = stats.iter().find(|s| s.username == user); ctx.out.emit(&RoomMemberRecord { room: room.to_string(), user, + status: stat.map(|s| s.status.to_string()), + average_speed: stat.map(|s| s.average_speed), + shared_files: stat.map(|s| s.shared_files), + shared_folders: stat.map(|s| s.shared_folders), + slots_free: stat.map(|s| s.slots_free), + country: stat.and_then(|s| s.country.clone()), }); } Ok(()) diff --git a/soulseek-rs/src/daemon/openrpc.rs b/soulseek-rs/src/daemon/openrpc.rs index e58b9c4..6acb736 100644 --- a/soulseek-rs/src/daemon/openrpc.rs +++ b/soulseek-rs/src/daemon/openrpc.rs @@ -11,10 +11,11 @@ use super::proto::{ Ack, AuthParams, AuthResult, BrowseEvent, DaemonStatus, DirectoriesParams, DirectoryParams, DownloadStartParams, DownloadStarted, DownloadStatusEvent, - Downloads, Event, IntervalSeconds, Members, MessageParams, Messages, - Method, QueryParams, RoomEventDto, RoomRef, SayParams, SearchResults, - Searches, Seconds, SessionLossEvent, SharesStatus, SlotsParams, - TransferRef, UploadInfoDto, Uploads, UserMessageDto, UserRef, UserResult, + Downloads, Event, IntervalSeconds, MemberStats, Members, MessageParams, + Messages, Method, QueryParams, RoomEventDto, RoomRef, SayParams, + SearchResults, Searches, Seconds, SessionLossEvent, SharesStatus, + SlotsParams, TransferRef, UploadInfoDto, Uploads, UserMessageDto, UserRef, + UserResult, }; use schemars::{JsonSchema, SchemaGenerator, generate::SchemaSettings}; use serde_json::{Value, json}; @@ -46,9 +47,10 @@ fn params_schema(method: Method, generator: &mut SchemaGenerator) -> Value { | Method::UploadCancel => schema_of::(generator), Method::DownloadSetDir => schema_of::(generator), Method::UploadSlots => schema_of::(generator), - Method::RoomJoin | Method::RoomLeave | Method::RoomMembers => { - schema_of::(generator) - } + Method::RoomJoin + | Method::RoomLeave + | Method::RoomMembers + | Method::RoomMemberStats => schema_of::(generator), Method::RoomSay => schema_of::(generator), Method::MessageSend => schema_of::(generator), Method::BrowseUser | Method::UserRequest | Method::UserInfoOf => { @@ -88,6 +90,7 @@ fn result_schema(method: Method, generator: &mut SchemaGenerator) -> Value { Method::UploadList => schema_of::(generator), Method::PrivilegesOwn => schema_of::(generator), Method::RoomMembers => schema_of::(generator), + Method::RoomMemberStats => schema_of::(generator), Method::MessageHistory => schema_of::(generator), Method::UserInfoOf => schema_of::(generator), Method::SharesStatusOf | Method::SharesSet | Method::SharesReindex => { @@ -172,6 +175,9 @@ fn summary(method: Method) -> &'static str { Method::RoomLeave => "Leave a chat room.", Method::RoomSay => "Post a message to a room.", Method::RoomMembers => "Who is currently in a room.", + Method::RoomMemberStats => { + "What the server said about each member of a joined room." + } Method::MessageSend => "Send a private message.", Method::MessageHistory => { "Private messages the daemon collected, including while nobody was attached." diff --git a/soulseek-rs/src/daemon/proto/dto.rs b/soulseek-rs/src/daemon/proto/dto.rs index c5f513d..dea5140 100644 --- a/soulseek-rs/src/daemon/proto/dto.rs +++ b/soulseek-rs/src/daemon/proto/dto.rs @@ -7,7 +7,8 @@ use serde::{Deserialize, Serialize}; use soulseek_rs::types::{ - Download, DownloadMetadata, UserPresence, UserStats, UserStatus, + Download, DownloadMetadata, RoomUserStats, UserPresence, UserStats, + UserStatus, }; use soulseek_rs::{ DownloadStatus, File, RoomEvent, RoomInfo, SearchResult, SessionLoss, @@ -447,6 +448,58 @@ pub struct UserStatsDto { pub shared_folders: u32, } +/// The inverse of [`UserStatus`]'s `Display`. An unrecognised word reads as +/// offline, matching how the wire code is decoded. +fn status_from_str(status: &str) -> UserStatus { + match status { + "online" => UserStatus::Online, + "away" => UserStatus::Away, + _ => UserStatus::Offline, + } +} + +/// One member of a room, as the JoinRoom reply described them. +#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] +#[cfg_attr(test, derive(schemars::JsonSchema))] +pub struct RoomUserStatsDto { + pub username: String, + /// `online`, `away`, or `offline`. + pub status: String, + pub average_speed: u32, + pub shared_files: u32, + pub shared_folders: u32, + pub slots_free: u32, + pub country: Option, +} + +impl From<&RoomUserStats> for RoomUserStatsDto { + fn from(stats: &RoomUserStats) -> Self { + Self { + username: stats.username.clone(), + status: stats.status.to_string(), + average_speed: stats.average_speed, + shared_files: stats.shared_files, + shared_folders: stats.shared_folders, + slots_free: stats.slots_free, + country: stats.country.clone(), + } + } +} + +impl From for RoomUserStats { + fn from(dto: RoomUserStatsDto) -> Self { + Self { + username: dto.username, + status: status_from_str(&dto.status), + average_speed: dto.average_speed, + shared_files: dto.shared_files, + shared_folders: dto.shared_folders, + slots_free: dto.slots_free, + country: dto.country, + } + } +} + /// Each half is `None` until its reply lands, so a consumer can tell "the /// server said offline" from "the server has not answered yet". #[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] @@ -480,11 +533,7 @@ impl From for UserInfo { fn from(dto: UserInfoDto) -> Self { let mut info = Self::pending(dto.username); info.presence = dto.presence.map(|presence| UserPresence { - status: match presence.status.as_str() { - "online" => UserStatus::Online, - "away" => UserStatus::Away, - _ => UserStatus::Offline, - }, + status: status_from_str(&presence.status), privileged: presence.privileged, }); info.stats = dto.stats.map(|stats| UserStats { diff --git a/soulseek-rs/src/daemon/proto/method.rs b/soulseek-rs/src/daemon/proto/method.rs index 96a69c2..7b22f03 100644 --- a/soulseek-rs/src/daemon/proto/method.rs +++ b/soulseek-rs/src/daemon/proto/method.rs @@ -1,5 +1,6 @@ //! What each method takes and returns, and what each pushed event carries. +use super::dto::RoomUserStatsDto; use super::dto::{ DownloadDto, DownloadMetadataDto, DownloadStatusDto, SearchResultDto, SessionLossDto, SharedDirectoryDto, UploadInfoDto, UserInfoDto, @@ -139,6 +140,13 @@ pub struct Members { pub users: Vec, } +/// What the server said about each member of a room when it was joined. +#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] +#[cfg_attr(test, derive(schemars::JsonSchema))] +pub struct MemberStats { + pub members: Vec, +} + /// One line of chat history. Unlike a live [`UserMessageDto`] this carries a /// direction, because a conversation the daemon collected includes what this /// account sent as well as what it received. diff --git a/soulseek-rs/src/daemon/proto/mod.rs b/soulseek-rs/src/daemon/proto/mod.rs index f91b83c..9079da7 100644 --- a/soulseek-rs/src/daemon/proto/mod.rs +++ b/soulseek-rs/src/daemon/proto/mod.rs @@ -101,6 +101,7 @@ wire_vocabulary! { RoomLeave => "room.leave", RoomSay => "room.say", RoomMembers => "room.members", + RoomMemberStats => "room.member_stats", MessageSend => "message.send", MessageHistory => "message.history", BrowseUser => "browse.user", diff --git a/soulseek-rs/src/daemon/server/mod.rs b/soulseek-rs/src/daemon/server/mod.rs index 718a23a..13726ce 100644 --- a/soulseek-rs/src/daemon/server/mod.rs +++ b/soulseek-rs/src/daemon/server/mod.rs @@ -9,11 +9,12 @@ use super::hub::{Hub, PendingBrowses, PendingDownload}; use super::proto::{ Ack, AuthResult, CODE_APPLICATION, CODE_INVALID_PARAMS, ChatMessageDto, DaemonStatus, DirectoriesParams, DirectoryParams, DownloadDto, - DownloadStartParams, DownloadStarted, Downloads, IntervalSeconds, Members, - MessageParams, Messages, Method, OPENRPC, PROTOCOL_VERSION, QueryParams, - RoomRef, RpcError, SayParams, SearchResultDto, SearchResults, - SearchSummary, Searches, Seconds, SharesStatus, SlotsParams, TransferRef, - UploadInfoDto, Uploads, UserInfoDto, UserRef, UserResult, + DownloadStartParams, DownloadStarted, Downloads, IntervalSeconds, + MemberStats, Members, MessageParams, Messages, Method, OPENRPC, + PROTOCOL_VERSION, QueryParams, RoomRef, RoomUserStatsDto, RpcError, + SayParams, SearchResultDto, SearchResults, SearchSummary, Searches, + Seconds, SharesStatus, SlotsParams, TransferRef, UploadInfoDto, Uploads, + UserInfoDto, UserRef, UserResult, }; use crate::api::SessionApi; use crate::output::Exit; @@ -252,6 +253,18 @@ impl Daemon { }) } + Method::RoomMemberStats => { + let room: RoomRef = parse(params)?; + ok(MemberStats { + members: self + .session + .room_member_stats(&room.room) + .iter() + .map(RoomUserStatsDto::from) + .collect(), + }) + } + Method::MessageSend => { let message: MessageParams = parse(params)?; self.session @@ -593,6 +606,29 @@ mod tests { ); } + #[test] + fn member_stats_are_published_for_a_joined_room() { + let daemon = daemon_with_token("t"); + let stats = daemon + .call(Method::RoomMemberStats, json!({ "room": STUB_ROOM })) + .expect("member stats"); + assert_eq!(stats["members"][0]["username"], "alice"); + assert_eq!(stats["members"][0]["status"], "online"); + assert_eq!(stats["members"][0]["slots_free"], 1); + assert_eq!(stats["members"][0]["country"], "NL"); + } + + #[test] + fn a_room_that_was_never_joined_has_no_member_stats() { + // Empty, not an error: asking about a room this session is not in is + // an ordinary thing for a client to do. + let daemon = daemon_with_token("t"); + let stats = daemon + .call(Method::RoomMemberStats, json!({ "room": "elsewhere" })) + .expect("member stats"); + assert_eq!(stats["members"], json!([])); + } + #[test] fn stop_is_recorded_so_the_daemon_can_wind_down() { let daemon = daemon_with_token("t"); @@ -605,8 +641,14 @@ mod tests { // A session that answers nothing: enough to exercise dispatch, parameter // validation, and the auth gate without a Soulseek server. + /// A session that answers nothing, except for one room whose members it + /// describes: the stats method is only meaningful if it can return + /// something for a joined room and nothing for any other. struct SilentSession; + /// The one room `SilentSession` claims to have joined. + const STUB_ROOM: &str = "jazz"; + fn daemon_with_token(token: &str) -> Daemon { Daemon { session: Arc::new(SilentSession), @@ -764,6 +806,23 @@ mod tests { fn room_members(&self, _room: &str) -> Vec { Vec::new() } + fn room_member_stats( + &self, + room: &str, + ) -> Vec { + if room != STUB_ROOM { + return Vec::new(); + } + vec![soulseek_rs::types::RoomUserStats { + username: "alice".to_string(), + status: soulseek_rs::types::UserStatus::Online, + average_speed: 1024, + shared_files: 20, + shared_folders: 3, + slots_free: 1, + country: Some("NL".to_string()), + }] + } fn take_room_events(&self) -> Vec { Vec::new() } diff --git a/soulseek-rs/src/output.rs b/soulseek-rs/src/output.rs index d50a853..85209fd 100644 --- a/soulseek-rs/src/output.rs +++ b/soulseek-rs/src/output.rs @@ -370,16 +370,33 @@ impl Record for UserRecord { } } -/// One member of a chat room. +/// One member of a chat room, with the server's description of them. +/// +/// The statistics come from the reply that named the members, and are `None` +/// for a server that sent none — so a consumer never reads "shares nothing" +/// from a missing answer. #[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub struct RoomMemberRecord { pub room: String, pub user: String, + /// `online`, `away`, or `offline`. + pub status: Option, + pub average_speed: Option, + pub shared_files: Option, + pub shared_folders: Option, + pub slots_free: Option, + pub country: Option, } impl Record for RoomMemberRecord { fn text(&self) -> String { - sanitize(&self.user) + let unknown = || "-".to_string(); + format!( + "{}\t{}\t{}", + sanitize(&self.user), + self.status.clone().unwrap_or_else(unknown), + self.shared_files.map_or_else(unknown, |v| v.to_string()) + ) } } diff --git a/soulseek-rs/src/remote/mod.rs b/soulseek-rs/src/remote/mod.rs index e236d05..01ebde5 100644 --- a/soulseek-rs/src/remote/mod.rs +++ b/soulseek-rs/src/remote/mod.rs @@ -12,14 +12,14 @@ use crate::daemon::proto::{ Ack, AuthParams, AuthResult, CODE_APPLICATION, ChatMessageDto, DaemonStatus, DirectoriesParams, DirectoryParams, DownloadDto, DownloadStartParams, DownloadStarted, Downloads, Event, IntervalSeconds, - Members, MessageParams, Messages, Method, PROTOCOL_VERSION, QueryParams, - Request, Response, RoomRef, RpcError, SayParams, SearchResults, Searches, - Seconds, SharesStatus, SlotsParams, TransferRef, Uploads, UserRef, - UserResult, + MemberStats, Members, MessageParams, Messages, Method, PROTOCOL_VERSION, + QueryParams, Request, Response, RoomRef, RpcError, SayParams, + SearchResults, Searches, Seconds, SharesStatus, SlotsParams, TransferRef, + Uploads, UserRef, UserResult, }; use mirror::Mirror; use serde::de::DeserializeOwned; -use soulseek_rs::types::{Download, DownloadMetadata}; +use soulseek_rs::types::{Download, DownloadMetadata, RoomUserStats}; use soulseek_rs::{ DownloadStatus, Result, RoomEvent, SearchResult, SessionLoss, SharedDirectory, SoulseekRs, UploadInfo, UserInfo, UserMessage, @@ -710,6 +710,17 @@ impl SessionApi for RemoteSession { .unwrap_or_default() } + fn room_member_stats(&self, room: &str) -> Vec { + self.request::<_, MemberStats>( + Method::RoomMemberStats, + RoomRef { + room: room.to_string(), + }, + ) + .map(|stats| stats.members.into_iter().map(Into::into).collect()) + .unwrap_or_default() + } + fn take_room_events(&self) -> Vec { self.mirror.take_rooms() } diff --git a/soulseek-rs/src/ui/main_tui/mod.rs b/soulseek-rs/src/ui/main_tui/mod.rs index 0b80188..4d47014 100644 --- a/soulseek-rs/src/ui/main_tui/mod.rs +++ b/soulseek-rs/src/ui/main_tui/mod.rs @@ -461,6 +461,12 @@ mod tests { fn room_members(&self, _room: &str) -> Vec { Vec::new() } + fn room_member_stats( + &self, + _room: &str, + ) -> Vec { + Vec::new() + } fn take_room_events(&self) -> Vec { Vec::new() } From 35a6fed3c3d0a78d13365f3cbafae67dbaf71f50 Mon Sep 17 00:00:00 2001 From: Michel de Graaf Date: Fri, 28 Aug 2026 14:39:34 +0000 Subject: [PATCH 3/4] fix: slotsfull is occupancy, not free slots, and cover it end to end Running the e2e suite against a real soulfind showed the per-member slots field was implemented backwards. The wire field is `slotsfull` -- a boolean that is true when the member's upload slots are all occupied -- not a count of free ones. `slots_free: u32` named it, documented it and printed it as the opposite of what the server means, so a caller reading it would pick the busiest peer to download from. - types: `slots_full: bool`, with the wire name in the doc comment - the DTO, the `room users` output and SKILL.md follow - a new e2e test joins a room with a member sharing a known three files and asserts the counts, the roster alignment and the slot flag against soulfind, so the parsing is checked against a real server rather than only against frames this repo wrote itself --- soulseek-rs-lib/src/client/tests.rs | 4 +- .../src/message/server/join_room.rs | 13 +-- soulseek-rs-lib/src/types.rs | 6 +- soulseek-rs-lib/tests/e2e.rs | 81 +++++++++++++++++++ soulseek-rs/docs/openrpc.json | 9 +-- soulseek-rs/src/SKILL.md | 6 +- soulseek-rs/src/commands/social.rs | 2 +- soulseek-rs/src/daemon/proto/dto.rs | 7 +- soulseek-rs/src/daemon/server/mod.rs | 4 +- soulseek-rs/src/output.rs | 2 +- 10 files changed, 109 insertions(+), 25 deletions(-) diff --git a/soulseek-rs-lib/src/client/tests.rs b/soulseek-rs-lib/src/client/tests.rs index aa29d45..c7a0697 100644 --- a/soulseek-rs-lib/src/client/tests.rs +++ b/soulseek-rs-lib/src/client/tests.rs @@ -632,7 +632,7 @@ fn room_member_stats_are_returned_sorted_and_scoped_to_their_room() { average_speed: 1, shared_files: 2, shared_folders: 3, - slots_free: 1, + slots_full: true, country: None, }; @@ -660,7 +660,7 @@ fn rejoining_replaces_the_previous_member_stats() { average_speed: 1, shared_files: 2, shared_folders: 3, - slots_free: 0, + slots_full: false, country: None, }; diff --git a/soulseek-rs-lib/src/message/server/join_room.rs b/soulseek-rs-lib/src/message/server/join_room.rs index 0e48716..273e10c 100644 --- a/soulseek-rs-lib/src/message/server/join_room.rs +++ b/soulseek-rs-lib/src/message/server/join_room.rs @@ -59,7 +59,8 @@ fn read_int32s(message: &mut Message) -> Vec { /// Parse the per-member statistics that follow the membership list of a /// `JoinRoom` (code 14) reply: statuses, then a record of share statistics per -/// member, then free upload slots, then country codes. +/// member, then whether each member's upload slots are full, then country +/// codes. /// /// Each vector is independently length-prefixed and a server may send fewer /// entries than there are members (or, on an older server, omit the country @@ -87,7 +88,7 @@ fn parse_member_stats( shares.push((average_speed, shared_files, shared_folders)); } - let slots_free = read_int32s(message); + let slots_full = read_int32s(message); let countries = read_strings(message); users @@ -104,7 +105,7 @@ fn parse_member_stats( average_speed, shared_files, shared_folders, - slots_free: slots_free.get(i).copied().unwrap_or(0), + slots_full: slots_full.get(i).copied().unwrap_or(0) != 0, country: countries.get(i).cloned().filter(|c| !c.is_empty()), } }) @@ -156,7 +157,7 @@ mod tests { .write_int64(0) .write_int32(10) .write_int32(2) - // free slots + // slots full .write_int32(2) .write_int32(1) .write_int32(0) @@ -188,7 +189,7 @@ mod tests { average_speed: 1024, shared_files: 4321, shared_folders: 77, - slots_free: 1, + slots_full: true, country: Some("NL".to_string()), }, RoomUserStats { @@ -197,7 +198,7 @@ mod tests { average_speed: 50, shared_files: 10, shared_folders: 2, - slots_free: 0, + slots_full: false, country: None, }, ] diff --git a/soulseek-rs-lib/src/types.rs b/soulseek-rs-lib/src/types.rs index 7fd3e1f..be94a22 100644 --- a/soulseek-rs-lib/src/types.rs +++ b/soulseek-rs-lib/src/types.rs @@ -341,8 +341,10 @@ pub struct RoomUserStats { pub average_speed: u32, pub shared_files: u32, pub shared_folders: u32, - /// Upload slots the member has free; zero means they are busy. - pub slots_free: u32, + /// True when the member's upload slots are all occupied, so a download + /// from them will queue. The wire field is `slotsfull`, a boolean — not a + /// count, and not a count of *free* slots. + pub slots_full: bool, /// Two-letter country code, when the server reports one. pub country: Option, } diff --git a/soulseek-rs-lib/tests/e2e.rs b/soulseek-rs-lib/tests/e2e.rs index 072bc21..53f18fe 100644 --- a/soulseek-rs-lib/tests/e2e.rs +++ b/soulseek-rs-lib/tests/e2e.rs @@ -2622,3 +2622,84 @@ fn a_peer_that_hangs_up_its_control_connection_still_delivers_the_file() { let _ = std::fs::remove_dir_all(&download_dir); } + +#[test] +fn a_joined_room_reports_its_members_statistics() { + let server = server_or_skip!(); + + let room = "e2e_room_stats"; + + // A member with a known share, so the statistics are checkable rather + // than merely present. + let share_dir = unique_download_dir().join("stats_share"); + std::fs::create_dir_all(share_dir.join("album")).unwrap(); + for name in ["one.flac", "two.flac", "three.flac"] { + std::fs::write(share_dir.join("album").join(name), b"xxxx").unwrap(); + } + let mut sharer = Client::with_settings(ClientSettings { + shared_directories: vec![share_dir.display().to_string()], + ..server.settings("e2e_stats_sharer", "pw") + }); + sharer.connect().expect("sharer connect"); + assert!(sharer.login().expect("sharer login")); + sharer.join_room(room).expect("sharer joins room"); + + // Let the server register the sharer's membership and share counts, so + // the observer's own join reply describes a room that already has them. + std::thread::sleep(Duration::from_millis(750)); + + let mut observer = + Client::with_settings(server.settings("e2e_stats_observer", "pw")); + observer.connect().expect("observer connect"); + assert!(observer.login().expect("observer login")); + observer.join_room(room).expect("observer joins room"); + + // The stats ride along with the membership list, so poll for the member + // rather than for a fixed delay. + let deadline = Instant::now() + Duration::from_secs(15); + let mut sharer_stats = None; + while Instant::now() < deadline { + if let Some(found) = observer + .room_member_stats(room) + .into_iter() + .find(|s| s.username == "e2e_stats_sharer") + { + sharer_stats = Some(found); + break; + } + std::thread::sleep(Duration::from_millis(100)); + } + let stats = + sharer_stats.expect("the join reply should describe the members"); + + assert!( + stats.status.is_reachable(), + "a member who is in the room cannot read as offline, got {}", + stats.status + ); + assert_eq!( + stats.shared_files, 3, + "the member's file count must survive the parallel stat vectors" + ); + assert_eq!(stats.shared_folders, 1); + assert!( + !stats.slots_full, + "a client that never filled its slots must not read as full" + ); + + // Every member is described, not just the one we looked up: a short or + // misread vector would drop or shift the rest. + let members = observer.room_member_stats(room); + let names: Vec<&str> = + members.iter().map(|s| s.username.as_str()).collect(); + assert!( + names.contains(&"e2e_stats_sharer") + && names.contains(&"e2e_stats_observer"), + "both members should be described, got {names:?}" + ); + assert_eq!( + members.len(), + observer.room_members(room).len(), + "the stats must line up one-to-one with the roster" + ); +} diff --git a/soulseek-rs/docs/openrpc.json b/soulseek-rs/docs/openrpc.json index d8b6ead..dcdf9ec 100644 --- a/soulseek-rs/docs/openrpc.json +++ b/soulseek-rs/docs/openrpc.json @@ -772,10 +772,9 @@ "minimum": 0, "type": "integer" }, - "slots_free": { - "format": "uint32", - "minimum": 0, - "type": "integer" + "slots_full": { + "description": "True when the member's upload slots are all occupied.", + "type": "boolean" }, "status": { "description": "`online`, `away`, or `offline`.", @@ -791,7 +790,7 @@ "average_speed", "shared_files", "shared_folders", - "slots_free" + "slots_full" ], "type": "object" }, diff --git a/soulseek-rs/src/SKILL.md b/soulseek-rs/src/SKILL.md index dc3f173..8a603d0 100644 --- a/soulseek-rs/src/SKILL.md +++ b/soulseek-rs/src/SKILL.md @@ -141,10 +141,10 @@ server did not answer — those are different facts, so do not treat null as 0. **`room list`** — `room`, `users`. **`room users `** — `room`, `user`, `status`, `average_speed`, -`shared_files`, `shared_folders`, `slots_free`, `country`. Everything but +`shared_files`, `shared_folders`, `slots_full`, `country`. Everything but `room` and `user` is `null` when the server sent no statistics for that -member — `null` means unknown, not zero. `slots_free` of 0 means the peer has -no free upload slot right now, so a download from them will queue. +member — `null` means unknown, not zero. `slots_full` is true when the peer's +upload slots are all occupied, so a download from them will queue. **`room listen `** — streams `type`, `room`, `user`, `message`. **`room say `** — prints nothing; exit 0 is the confirmation. diff --git a/soulseek-rs/src/commands/social.rs b/soulseek-rs/src/commands/social.rs index 6c33a4d..4bedea4 100644 --- a/soulseek-rs/src/commands/social.rs +++ b/soulseek-rs/src/commands/social.rs @@ -164,7 +164,7 @@ pub fn room_users(ctx: &Ctx, room: &str, timeout: Duration) -> CliResult { average_speed: stat.map(|s| s.average_speed), shared_files: stat.map(|s| s.shared_files), shared_folders: stat.map(|s| s.shared_folders), - slots_free: stat.map(|s| s.slots_free), + slots_full: stat.map(|s| s.slots_full), country: stat.and_then(|s| s.country.clone()), }); } diff --git a/soulseek-rs/src/daemon/proto/dto.rs b/soulseek-rs/src/daemon/proto/dto.rs index dea5140..c7ad6aa 100644 --- a/soulseek-rs/src/daemon/proto/dto.rs +++ b/soulseek-rs/src/daemon/proto/dto.rs @@ -468,7 +468,8 @@ pub struct RoomUserStatsDto { pub average_speed: u32, pub shared_files: u32, pub shared_folders: u32, - pub slots_free: u32, + /// True when the member's upload slots are all occupied. + pub slots_full: bool, pub country: Option, } @@ -480,7 +481,7 @@ impl From<&RoomUserStats> for RoomUserStatsDto { average_speed: stats.average_speed, shared_files: stats.shared_files, shared_folders: stats.shared_folders, - slots_free: stats.slots_free, + slots_full: stats.slots_full, country: stats.country.clone(), } } @@ -494,7 +495,7 @@ impl From for RoomUserStats { average_speed: dto.average_speed, shared_files: dto.shared_files, shared_folders: dto.shared_folders, - slots_free: dto.slots_free, + slots_full: dto.slots_full, country: dto.country, } } diff --git a/soulseek-rs/src/daemon/server/mod.rs b/soulseek-rs/src/daemon/server/mod.rs index 13726ce..b710a76 100644 --- a/soulseek-rs/src/daemon/server/mod.rs +++ b/soulseek-rs/src/daemon/server/mod.rs @@ -614,7 +614,7 @@ mod tests { .expect("member stats"); assert_eq!(stats["members"][0]["username"], "alice"); assert_eq!(stats["members"][0]["status"], "online"); - assert_eq!(stats["members"][0]["slots_free"], 1); + assert_eq!(stats["members"][0]["slots_full"], true); assert_eq!(stats["members"][0]["country"], "NL"); } @@ -819,7 +819,7 @@ mod tests { average_speed: 1024, shared_files: 20, shared_folders: 3, - slots_free: 1, + slots_full: true, country: Some("NL".to_string()), }] } diff --git a/soulseek-rs/src/output.rs b/soulseek-rs/src/output.rs index 85209fd..3e0f70e 100644 --- a/soulseek-rs/src/output.rs +++ b/soulseek-rs/src/output.rs @@ -384,7 +384,7 @@ pub struct RoomMemberRecord { pub average_speed: Option, pub shared_files: Option, pub shared_folders: Option, - pub slots_free: Option, + pub slots_full: Option, pub country: Option, } From 62c99e4ab839c4f98c8601bbcfbbb35f4edb0358 Mon Sep 17 00:00:00 2001 From: Michel de Graaf Date: Fri, 28 Aug 2026 14:55:02 +0000 Subject: [PATCH 4/4] fix(cli): keep `room users` text output a bare roster The e2e job caught this: adding status and file columns to the text record broke `room_users_lists_who_is_in_the_room`, which asserts the line equals the username. That test encodes a real contract -- the text output is a roster to pipe into other commands -- and reformatting it was scope this change did not need. The statistics stay on the record for `--json`. Reproduced the failure locally against soulfind, then confirmed the same test passing with the fix. --- soulseek-rs/src/output.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/soulseek-rs/src/output.rs b/soulseek-rs/src/output.rs index 3e0f70e..ec385fd 100644 --- a/soulseek-rs/src/output.rs +++ b/soulseek-rs/src/output.rs @@ -389,14 +389,11 @@ pub struct RoomMemberRecord { } impl Record for RoomMemberRecord { + /// The username alone, unchanged by the statistics added alongside it: + /// the text line is a roster a caller pipes into other commands, and the + /// statistics are there for `--json` to carry. fn text(&self) -> String { - let unknown = || "-".to_string(); - format!( - "{}\t{}\t{}", - sanitize(&self.user), - self.status.clone().unwrap_or_else(unknown), - self.shared_files.map_or_else(unknown, |v| v.to_string()) - ) + sanitize(&self.user) } }