From 0f91ffdcf31eab9e4a51edc06841c5cff3461240 Mon Sep 17 00:00:00 2001 From: Michel de Graaf Date: Thu, 27 Aug 2026 13:22:39 +0200 Subject: [PATCH 01/12] feat(tui): dismiss a finished download with d MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A completed row used to ignore d: only a queued transfer could be pulled out of the list, so a finished one sat there until the next restart. d now goes through one handler that drops a finished row whatever the session thinks of it — a completed transfer can live only in this window, restored from an earlier run, where there is nothing to forget. The finished-status test was spelled out twice in clear_finished_downloads and had to agree by hand; both callers now share is_finished(). --- soulseek-rs/src/ui/main_tui/downloads.rs | 55 ++++++++++++--------- soulseek-rs/src/ui/main_tui/input.rs | 2 +- soulseek-rs/src/ui/main_tui/mod.rs | 61 +++++++++++++++++++++++- soulseek-rs/src/ui/main_tui/render.rs | 2 +- 4 files changed, 93 insertions(+), 27 deletions(-) diff --git a/soulseek-rs/src/ui/main_tui/downloads.rs b/soulseek-rs/src/ui/main_tui/downloads.rs index 9fb3722..5fefec1 100644 --- a/soulseek-rs/src/ui/main_tui/downloads.rs +++ b/soulseek-rs/src/ui/main_tui/downloads.rs @@ -3,6 +3,16 @@ use crate::models::{DownloadEntry, FileDisplayData}; use soulseek_rs::{DownloadStatus, types::Download}; use std::{sync::mpsc, thread}; +/// Done, given up, or ran out of time: the row is all that is left. +const fn is_finished(download: &Download) -> bool { + matches!( + download.status, + DownloadStatus::Completed + | DownloadStatus::Failed(_) + | DownloadStatus::TimedOut + ) +} + impl MainTui { /// Channel for queued downloads, created on first use. pub(super) fn downloads_sender( @@ -63,26 +73,16 @@ impl MainTui { // session holds, so dropping them only here would last until the next // poll put them straight back. for entry in &self.state.downloads { - if matches!( - entry.download.status, - DownloadStatus::Completed - | DownloadStatus::Failed(_) - | DownloadStatus::TimedOut - ) { + if is_finished(&entry.download) { self.client.remove_download( &entry.download.username, &entry.download.filename, ); } } - self.state.downloads.retain(|entry| { - !matches!( - entry.download.status, - DownloadStatus::Completed - | DownloadStatus::Failed(_) - | DownloadStatus::TimedOut - ) - }); + self.state + .downloads + .retain(|entry| !is_finished(&entry.download)); let len = self.state.downloads.len(); if len == 0 { self.state.downloads_table_state.select(None); @@ -121,7 +121,9 @@ impl MainTui { } } - pub(super) fn remove_selected_queued_download(&mut self) { + /// Drop the selected transfer: a queued one leaves the queue, a finished + /// one leaves the list. + pub(super) fn remove_selected_download(&mut self) { let Some(index) = self.state.downloads_table_state.selected() else { return; }; @@ -130,14 +132,21 @@ impl MainTui { }; let download = &download_entry.download; - if !matches!(download.status, DownloadStatus::Queued) { - return; - } - - if !self - .client - .remove_queued_download(&download.username, &download.filename) - { + let dismissed = match download.status { + DownloadStatus::Queued => self + .client + .remove_queued_download(&download.username, &download.filename), + // A finished transfer may exist only in this window (restored from + // an earlier run), so the session's "nothing to forget" is not a + // reason to keep the row. + DownloadStatus::Completed => { + self.client + .remove_download(&download.username, &download.filename); + true + } + _ => false, + }; + if !dismissed { return; } diff --git a/soulseek-rs/src/ui/main_tui/input.rs b/soulseek-rs/src/ui/main_tui/input.rs index 4be6277..4c320b4 100644 --- a/soulseek-rs/src/ui/main_tui/input.rs +++ b/soulseek-rs/src/ui/main_tui/input.rs @@ -428,7 +428,7 @@ impl MainTui { self.toggle_selected_download_pause(); } KeyCode::Char('d') => { - self.remove_selected_queued_download(); + self.remove_selected_download(); } KeyCode::Char('r') => { self.retry_selected_download(); diff --git a/soulseek-rs/src/ui/main_tui/mod.rs b/soulseek-rs/src/ui/main_tui/mod.rs index aa4fb1a..0b80188 100644 --- a/soulseek-rs/src/ui/main_tui/mod.rs +++ b/soulseek-rs/src/ui/main_tui/mod.rs @@ -265,7 +265,8 @@ pub fn launch_main_tui( mod tests { use super::*; use crate::daemon::proto::ChatMessageDto; - use crate::models::{MessageDirection, SearchStatus}; + use crate::models::{FocusedPane, MessageDirection, SearchStatus}; + use ratatui::crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; /// A stand-in for a daemon: it already holds a conversation, a transfer /// queue, and a search cache, none of which this window created. @@ -291,6 +292,21 @@ mod tests { } fn queued(username: &str, filename: &str) -> soulseek_rs::types::Download { + at_status(username, filename, soulseek_rs::DownloadStatus::Queued) + } + + fn completed( + username: &str, + filename: &str, + ) -> soulseek_rs::types::Download { + at_status(username, filename, soulseek_rs::DownloadStatus::Completed) + } + + fn at_status( + username: &str, + filename: &str, + status: soulseek_rs::DownloadStatus, + ) -> soulseek_rs::types::Download { let (sender, _) = std::sync::mpsc::channel(); soulseek_rs::types::Download { username: username.to_string(), @@ -298,13 +314,17 @@ mod tests { token: 1, size: 4096, download_directory: "/tmp".to_string(), - status: soulseek_rs::DownloadStatus::Queued, + status, sender, queue_position: None, metadata: soulseek_rs::types::DownloadMetadata::default(), } } + fn key(ch: char) -> KeyEvent { + KeyEvent::new(KeyCode::Char(ch), KeyModifiers::NONE) + } + impl SessionApi for TalkativeSession { fn message_history(&self) -> Vec { self.history.clone() @@ -619,6 +639,43 @@ mod tests { ); } + #[test] + fn d_clears_a_finished_row_but_not_a_running_one() { + // Finished transfers are what a list fills up with after a restart, so + // `d` has to be able to dismiss one — restored or not. A running + // transfer is not ours to discard, and stays put. + let mut tui = with_session(TalkativeSession::default()); + tui.state.downloads.push(crate::models::DownloadEntry { + download: completed("bob", "done.mp3"), + receiver: None, + }); + tui.state.downloads_table_state.select(Some(0)); + tui.state.focused_pane = FocusedPane::Downloads; + + tui.handle_key_event(key('d')); + + assert!(tui.state.downloads.is_empty(), "the row goes too"); + assert_eq!(tui.state.downloads_table_state.selected(), None); + + tui.state.downloads.push(crate::models::DownloadEntry { + download: at_status( + "bob", + "still.mp3", + soulseek_rs::DownloadStatus::InProgress { + bytes_downloaded: 1, + total_bytes: 2, + speed_bytes_per_sec: 1.0, + }, + ), + receiver: None, + }); + tui.state.downloads_table_state.select(Some(0)); + + tui.handle_key_event(key('d')); + + assert_eq!(tui.state.downloads.len(), 1, "it is not disposable"); + } + #[test] fn a_window_shows_searches_run_in_another() { let mut tui = diff --git a/soulseek-rs/src/ui/main_tui/render.rs b/soulseek-rs/src/ui/main_tui/render.rs index 027c433..9e7c42e 100644 --- a/soulseek-rs/src/ui/main_tui/render.rs +++ b/soulseek-rs/src/ui/main_tui/render.rs @@ -393,7 +393,7 @@ impl MainTui { vec![ ("p", "pause/resume"), ("r", "retry failed"), - ("d", "delete queued"), + ("d", "delete queued/done"), ("c", "clear finished"), ("b", "browse user"), ("1-3", "focus pane"), From fa49ebd2ebc66d22cbe1eb31afe57647a14c8885 Mon Sep 17 00:00:00 2001 From: Michel de Graaf Date: Fri, 28 Aug 2026 12:44:31 +0000 Subject: [PATCH 02/12] 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 b505718db9055a2023b2dfb77913dbeb8d6bfb55 Mon Sep 17 00:00:00 2001 From: Michel de Graaf Date: Fri, 28 Aug 2026 12:42:13 +0000 Subject: [PATCH 03/12] feat(lib): watch and unwatch users (WatchUser 5 / UnwatchUser 6) Adds the server-side watch subscription a buddy list needs: WatchUser (5) asks the server to keep pushing a user's status, UnwatchUser (6) cancels it. The reply to a watch carries the user's current status and share statistics, which merge into the existing per-user snapshot read via Client::user_info. Issue #47 asked for AddUserList (141) / WatchedUserAdded (142) / WatchedUserRemoved (143). Those codes are EnableRoomInvitations, ChangePassword and AddRoomOperator in the Soulseek protocol -- there are no such messages on the wire. Watching is codes 5 and 6, so that is what this implements. - message/server/watch_user.rs: WatchUserHandler (code 5), tolerant of an absent user and of a truncated frame - MessageFactory::build_watch_user / build_unwatch_user - Client::watch_user / unwatch_user / watched_users - The watch reply has no privileged flag, so it preserves whatever an earlier GetUserStatus recorded rather than asserting false --- soulseek-rs-lib/src/actor/server_actor.rs | 30 ++++ soulseek-rs-lib/src/client/mod.rs | 77 ++++++++++ soulseek-rs-lib/src/client/operations.rs | 20 +++ soulseek-rs-lib/src/client/rooms.rs | 40 +++++ soulseek-rs-lib/src/client/tests.rs | 80 ++++++++++ .../src/message/server/message_factory.rs | 28 ++++ soulseek-rs-lib/src/message/server/mod.rs | 2 + .../src/message/server/watch_user.rs | 140 ++++++++++++++++++ 8 files changed, 417 insertions(+) create mode 100644 soulseek-rs-lib/src/message/server/watch_user.rs diff --git a/soulseek-rs-lib/src/actor/server_actor.rs b/soulseek-rs-lib/src/actor/server_actor.rs index 57c1778..5b3f6e2 100644 --- a/soulseek-rs-lib/src/actor/server_actor.rs +++ b/soulseek-rs-lib/src/actor/server_actor.rs @@ -18,6 +18,7 @@ use crate::message::server::ReloggedHandler; use crate::message::server::SayChatroomHandler; use crate::message::server::UserJoinedRoomHandler; use crate::message::server::UserLeftRoomHandler; +use crate::message::server::WatchUserHandler; use crate::message::server::WishListIntervalHandler; use crate::message::server::{ GetUserStatsHandler, GetUserStatusHandler, RoomListHandler, @@ -202,6 +203,17 @@ pub enum ServerMessage { status: u32, privileged: bool, }, + /// A `WatchUser` (code 5) reply: the initial snapshot for a user we just + /// started watching. The stats are absent when the server does not know + /// the username. + WatchedUserReceived { + username: String, + exists: bool, + status: Option, + average_speed: Option, + shared_files: Option, + shared_folders: Option, + }, /// A `GetUserStats` (code 36) reply. UserStatsReceived { username: String, @@ -359,6 +371,7 @@ impl ServerActor { handlers.register_handler(ReloggedHandler); handlers.register_handler(RoomListHandler); handlers.register_handler(GetUserStatusHandler); + handlers.register_handler(WatchUserHandler); handlers.register_handler(GetUserStatsHandler); handlers.register_handler(JoinRoomHandler); handlers.register_handler(LeaveRoomHandler); @@ -464,6 +477,23 @@ impl ServerActor { privileged, }); } + ServerMessage::WatchedUserReceived { + username, + exists, + status, + average_speed, + shared_files, + shared_folders, + } => { + self.forward_to_client(ClientOperation::WatchedUserReceived { + username, + exists, + status, + average_speed, + shared_files, + shared_folders, + }); + } ServerMessage::UserStatsReceived { username, average_speed, diff --git a/soulseek-rs-lib/src/client/mod.rs b/soulseek-rs-lib/src/client/mod.rs index 8d11cb4..798e175 100644 --- a/soulseek-rs-lib/src/client/mod.rs +++ b/soulseek-rs-lib/src/client/mod.rs @@ -245,6 +245,15 @@ pub enum ClientOperation { shared_files: u32, shared_folders: u32, }, + /// The server answered `WatchUser` for a user we started watching. + WatchedUserReceived { + username: String, + exists: bool, + status: Option, + average_speed: Option, + shared_files: Option, + shared_folders: Option, + }, PeerConnected(String), /// A search distributed to us by the server; reply if our shares match. IncomingSearch { @@ -328,6 +337,10 @@ pub struct ClientContext { /// What the server has told us about other users, merged across the /// separate status and statistics replies. user_info: HashMap, + /// Users we asked the server to watch (code 5), so it keeps pushing their + /// status changes. Kept so a UI can render the watch list and so an + /// unwatch can be rejected for someone we never watched. + watched_users: HashSet, /// Seconds the server wants between wishlist searches (code 104), once it /// has told us. wishlist_interval: Option, @@ -416,6 +429,7 @@ impl ClientContext { room_events: Vec::new(), room_members: HashMap::new(), user_info: HashMap::new(), + watched_users: HashSet::new(), wishlist_interval: None, privileged_users: HashSet::new(), own_privileges: None, @@ -503,6 +517,69 @@ impl ClientContext { }); } + /// Record that we are now watching `username`. + pub fn add_watched_user(&mut self, username: &str) { + self.watched_users.insert(username.to_string()); + } + + /// Forget a watch, dropping any snapshot we held for that user so a later + /// re-watch reports a fresh answer rather than a stale one. + pub fn remove_watched_user(&mut self, username: &str) { + self.watched_users.remove(username); + self.user_info.remove(username); + } + + /// Everyone we are currently watching, sorted by name. + #[must_use] + pub fn watched_users(&self) -> Vec { + let mut users: Vec = + self.watched_users.iter().cloned().collect(); + users.sort(); + users + } + + /// Record the initial snapshot from a `WatchUser` reply. A username the + /// server does not know carries no stats and is dropped from the watch + /// list, since the server will never push anything for it. + pub fn apply_watched_user( + &mut self, + username: String, + exists: bool, + status: Option, + average_speed: Option, + shared_files: Option, + shared_folders: Option, + ) { + if !exists { + self.watched_users.remove(&username); + return; + } + if let Some(status) = status { + let entry = self + .user_info + .entry(username.clone()) + .or_insert_with(|| UserInfo::pending(username.clone())); + // The watch reply carries no privileged flag, so keep whatever a + // previous GetUserStatus told us rather than asserting `false`. + let privileged = + entry.presence.as_ref().is_some_and(|p| p.privileged); + entry.presence = Some(UserPresence { + status: UserStatus::from_code(status), + privileged, + }); + } + if let (Some(average_speed), Some(shared_files), Some(shared_folders)) = + (average_speed, shared_files, shared_folders) + { + self.apply_user_stats( + username, + average_speed, + shared_files, + shared_folders, + ); + } + } + /// What the server has said about `username` so far. #[must_use] pub fn user_info(&self, username: &str) -> Option { diff --git a/soulseek-rs-lib/src/client/operations.rs b/soulseek-rs-lib/src/client/operations.rs index d80758f..e5baf46 100644 --- a/soulseek-rs-lib/src/client/operations.rs +++ b/soulseek-rs-lib/src/client/operations.rs @@ -473,6 +473,26 @@ impl Client { error!("[client] UserStatsReceived write: {}", e); } }, + ClientOperation::WatchedUserReceived { + username, + exists, + status, + average_speed, + shared_files, + shared_folders, + } => match client_context.write_safe() { + Ok(mut ctx) => ctx.apply_watched_user( + username, + exists, + status, + average_speed, + shared_files, + shared_folders, + ), + Err(e) => { + error!("[client] WatchedUserReceived write: {}", e); + } + }, ClientOperation::RoomEvent(event) => { match client_context.write_safe() { Ok(mut ctx) => ctx.apply_room_event(event), diff --git a/soulseek-rs-lib/src/client/rooms.rs b/soulseek-rs-lib/src/client/rooms.rs index 8afc871..5b5361c 100644 --- a/soulseek-rs-lib/src/client/rooms.rs +++ b/soulseek-rs-lib/src/client/rooms.rs @@ -138,6 +138,46 @@ impl Client { } } + /// Ask the server to watch `username` (code 5). + /// + /// The server replies with that user's current status and share + /// statistics — readable with [`Client::user_info`] — and then keeps + /// pushing their status changes until [`Client::unwatch_user`]. Watching + /// a user the server does not know is answered with a "does not exist" + /// flag, and that user is dropped from [`Client::watched_users`]. + /// + /// # Errors + /// Returns [`SoulseekRs::NotConnected`] if the client is not connected. + pub fn watch_user(&self, username: &str) -> Result<()> { + use crate::message::server::MessageFactory; + self.send_server_message(MessageFactory::build_watch_user(username))?; + self.context.write_safe()?.add_watched_user(username); + Ok(()) + } + + /// Stop watching `username` (code 6) and drop what we knew about them. + /// + /// # Errors + /// Returns [`SoulseekRs::NotConnected`] if the client is not connected. + pub fn unwatch_user(&self, username: &str) -> Result<()> { + use crate::message::server::MessageFactory; + self.send_server_message(MessageFactory::build_unwatch_user(username))?; + self.context.write_safe()?.remove_watched_user(username); + Ok(()) + } + + /// Everyone this client is currently watching, sorted by name. + #[must_use] + pub fn watched_users(&self) -> Vec { + match self.context.read_safe() { + Ok(ctx) => ctx.watched_users(), + Err(e) => { + error!("[client] watched_users: {}", e); + Vec::new() + } + } + } + /// Who is currently in `room`, sorted by name. /// /// The roster is built from the membership the server sends when diff --git a/soulseek-rs-lib/src/client/tests.rs b/soulseek-rs-lib/src/client/tests.rs index fd90b76..13409b1 100644 --- a/soulseek-rs-lib/src/client/tests.rs +++ b/soulseek-rs-lib/src/client/tests.rs @@ -622,3 +622,83 @@ fn a_replayed_transfer_response_does_not_start_a_second_transfer() { "a replayed TransferResponse must not dial the peer again" ); } + +#[test] +fn a_watch_reply_fills_both_halves_of_the_snapshot() { + let mut context = ClientContext::new(); + context.add_watched_user("alice"); + context.apply_watched_user( + "alice".to_string(), + true, + Some(2), + Some(1024), + Some(20), + Some(3), + ); + + let info = context.user_info("alice").expect("a snapshot"); + assert!(info.is_complete(), "a watch reply carries status and stats"); + assert_eq!(info.stats.map(|s| s.average_speed), Some(1024)); + assert_eq!(context.watched_users(), vec!["alice".to_string()]); +} + +#[test] +fn a_watch_reply_keeps_a_privileged_flag_it_cannot_carry() { + // WatchUser has no privileged field, so it must not overwrite what + // GetUserStatus already told us with a fabricated `false`. + let mut context = ClientContext::new(); + context.apply_user_status("bob".to_string(), 1, true); + context.apply_watched_user( + "bob".to_string(), + true, + Some(2), + Some(1), + Some(2), + Some(3), + ); + + let info = context.user_info("bob").expect("a snapshot"); + assert_eq!(info.presence.map(|p| p.privileged), Some(true)); +} + +#[test] +fn watching_an_unknown_user_drops_them_from_the_list() { + // The server will never push status for a name it does not know, so + // keeping it in the watch list would show a permanently blank row. + let mut context = ClientContext::new(); + context.add_watched_user("ghost"); + context.apply_watched_user( + "ghost".to_string(), + false, + None, + None, + None, + None, + ); + + assert!(context.watched_users().is_empty()); + assert!(context.user_info("ghost").is_none()); +} + +#[test] +fn unwatching_forgets_the_users_snapshot() { + let mut context = ClientContext::new(); + context.add_watched_user("alice"); + context.apply_user_status("alice".to_string(), 2, false); + + context.remove_watched_user("alice"); + assert!(context.watched_users().is_empty()); + assert!( + context.user_info("alice").is_none(), + "a later re-watch must report a fresh answer" + ); +} + +#[test] +fn watched_users_are_listed_in_a_stable_order() { + let mut context = ClientContext::new(); + context.add_watched_user("carol"); + context.add_watched_user("alice"); + context.add_watched_user("bob"); + assert_eq!(context.watched_users(), vec!["alice", "bob", "carol"]); +} diff --git a/soulseek-rs-lib/src/message/server/message_factory.rs b/soulseek-rs-lib/src/message/server/message_factory.rs index cb09a27..4ce3256 100644 --- a/soulseek-rs-lib/src/message/server/message_factory.rs +++ b/soulseek-rs-lib/src/message/server/message_factory.rs @@ -152,6 +152,20 @@ impl MessageFactory { .clone() } + /// Watch `username` (server code 5): the server replies with their + /// current status and share statistics, then pushes every later status + /// change as a `GetUserStatus` (code 7) until we unwatch them. + #[must_use] + pub fn build_watch_user(username: &str) -> Message { + Message::new().write_int32(5).write_string(username).clone() + } + + /// Stop watching `username` (server code 6). The server sends no reply. + #[must_use] + pub fn build_unwatch_user(username: &str) -> Message { + Message::new().write_int32(6).write_string(username).clone() + } + /// Ask the server (code 64) for the list of public chat rooms. #[must_use] pub fn build_room_list_request() -> Message { @@ -424,3 +438,17 @@ fn test_build_file_search_message() { .to_vec(); assert_eq!(expect, message.get_data()); } + +#[test] +fn test_build_watch_user() { + let message = MessageFactory::build_watch_user("bob"); + let expect: Vec = [5, 0, 0, 0, 3, 0, 0, 0, b'b', b'o', b'b'].to_vec(); + assert_eq!(expect, message.get_data()); +} + +#[test] +fn test_build_unwatch_user() { + let message = MessageFactory::build_unwatch_user("bob"); + let expect: Vec = [6, 0, 0, 0, 3, 0, 0, 0, b'b', b'o', b'b'].to_vec(); + assert_eq!(expect, message.get_data()); +} diff --git a/soulseek-rs-lib/src/message/server/mod.rs b/soulseek-rs-lib/src/message/server/mod.rs index 104082b..b2d93b7 100644 --- a/soulseek-rs-lib/src/message/server/mod.rs +++ b/soulseek-rs-lib/src/message/server/mod.rs @@ -16,6 +16,7 @@ mod say_chatroom; mod user_info; mod user_joined_room; mod user_left_room; +mod watch_user; mod wish_list_interval; pub use connect_to_peer::ConnectToPeerHandler; @@ -36,4 +37,5 @@ pub use say_chatroom::SayChatroomHandler; pub use user_info::{GetUserStatsHandler, GetUserStatusHandler}; pub use user_joined_room::UserJoinedRoomHandler; pub use user_left_room::UserLeftRoomHandler; +pub use watch_user::WatchUserHandler; pub use wish_list_interval::WishListIntervalHandler; diff --git a/soulseek-rs-lib/src/message/server/watch_user.rs b/soulseek-rs-lib/src/message/server/watch_user.rs new file mode 100644 index 0000000..ecbd50d --- /dev/null +++ b/soulseek-rs-lib/src/message/server/watch_user.rs @@ -0,0 +1,140 @@ +//! `WatchUser` (code 5): subscribe to a user's status and share statistics. +//! +//! Watching is how a client keeps a buddy list current. After a `WatchUser` +//! request the server answers on the same code with the user's current +//! presence and stats, and from then on pushes every later change as an +//! ordinary `GetUserStatus` (code 7). `UnwatchUser` (code 6) cancels the +//! subscription and has no reply. +//! +//! Note on codes: the protocol has no `AddUserList`/`WatchedUserAdded` +//! /`WatchedUserRemoved` messages — 141/142/143 are `EnableRoomInvitations`, +//! `ChangePassword` and `AddRoomOperator`. Watching is 5/6, per the reference +//! protocol documentation. + +use crate::actor::server_actor::ServerMessage; +use crate::message::{Message, MessageHandler}; +use std::sync::mpsc::Sender; + +/// Receives the `WatchUser` reply (code 5): `username`, `exists`, and — when +/// the user exists — status plus the same share statistics `GetUserStats` +/// carries. +pub struct WatchUserHandler; + +impl MessageHandler for WatchUserHandler { + fn get_code(&self) -> u8 { + 5 + } + + fn handle(&self, message: &mut Message, sender: Sender) { + let username = message.read_string(); + let exists = message.read_bool(); + let watched = if exists { + let status = message.read_int32(); + 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(); + ServerMessage::WatchedUserReceived { + username, + exists, + status: Some(status), + average_speed: Some(average_speed), + shared_files: Some(shared_files), + shared_folders: Some(shared_folders), + } + } else { + // An unknown username: the server stops after the flag, so there + // is nothing further to read and nothing to record. + ServerMessage::WatchedUserReceived { + username, + exists, + status: None, + average_speed: None, + shared_files: None, + shared_folders: None, + } + }; + let _ = sender.send(watched); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::message::framed; + + #[test] + fn existing_user_carries_status_and_stats() { + let (tx, rx) = std::sync::mpsc::channel(); + let mut message = framed(|m| { + m.write_string("alice") + .write_int8(1) // exists + .write_int32(2) // status: online + .write_int32(1024) // average speed + .write_int64(0) // obsolete upload number + .write_int32(4321) // shared files + .write_int32(77); // shared folders + }); + + WatchUserHandler.handle(&mut message, tx); + match rx.try_recv() { + Ok(ServerMessage::WatchedUserReceived { + username, + exists, + status, + average_speed, + shared_files, + shared_folders, + }) => { + assert_eq!(username, "alice"); + assert!(exists); + assert_eq!(status, Some(2)); + assert_eq!(average_speed, Some(1024)); + assert_eq!(shared_files, Some(4321)); + assert_eq!(shared_folders, Some(77)); + } + other => panic!("unexpected: {other:?}"), + } + } + + #[test] + fn unknown_user_reports_absence_without_stats() { + let (tx, rx) = std::sync::mpsc::channel(); + let mut message = framed(|m| { + m.write_string("ghost").write_int8(0); + }); + + WatchUserHandler.handle(&mut message, tx); + match rx.try_recv() { + Ok(ServerMessage::WatchedUserReceived { + username, + exists, + status, + shared_files, + .. + }) => { + assert_eq!(username, "ghost"); + assert!(!exists); + assert_eq!(status, None); + assert_eq!(shared_files, None); + } + other => panic!("unexpected: {other:?}"), + } + } + + #[test] + fn truncated_payload_does_not_panic() { + // exists = 1 but the stats were cut off: the bounds-checked readers + // must yield zeros rather than panicking on a hostile frame. + let (tx, rx) = std::sync::mpsc::channel(); + let mut message = framed(|m| { + m.write_string("alice").write_int8(1).write_int32(2); + }); + + WatchUserHandler.handle(&mut message, tx); + assert!(matches!( + rx.try_recv(), + Ok(ServerMessage::WatchedUserReceived { .. }) + )); + } +} From a91d35b672eadcba96ff445028e988cd29db5586 Mon Sep 17 00:00:00 2001 From: Michel de Graaf Date: Fri, 28 Aug 2026 13:22:50 +0000 Subject: [PATCH 04/12] feat(daemon): expose watching over the RPC surface and the CLI The watch subscription was reachable only from the library. It is most useful against a daemon -- that is the session that stays connected long enough to receive the server's status pushes -- so it needs the wire. - proto: user.watch / user.unwatch / user.watched, with a Watched response - SessionApi gains watch_user / unwatch_user / watched_users, so a local and a remote session are still interchangeable above the seam - CLI: `watch add`, `watch remove`, `watch list`, documented in SKILL.md including the caveat that a one-shot run's watch ends with the run - docs/openrpc.json regenerated from the wire types --- soulseek-rs/docs/openrpc.json | 67 +++++++++++++++++++ soulseek-rs/src/SKILL.md | 7 ++ soulseek-rs/src/api.rs | 18 +++++ soulseek-rs/src/cli.rs | 24 +++++++ soulseek-rs/src/commands/mod.rs | 1 + soulseek-rs/src/commands/peer.rs | 39 ++++++++++- soulseek-rs/src/daemon/openrpc.rs | 22 +++++-- soulseek-rs/src/daemon/proto/method.rs | 7 ++ soulseek-rs/src/daemon/proto/mod.rs | 3 + soulseek-rs/src/daemon/server/mod.rs | 91 +++++++++++++++++++++++++- soulseek-rs/src/output.rs | 19 ++++++ soulseek-rs/src/remote/mod.rs | 26 +++++++- soulseek-rs/src/ui/main_tui/mod.rs | 9 +++ 13 files changed, 322 insertions(+), 11 deletions(-) diff --git a/soulseek-rs/docs/openrpc.json b/soulseek-rs/docs/openrpc.json index 213bfc7..46e5886 100644 --- a/soulseek-rs/docs/openrpc.json +++ b/soulseek-rs/docs/openrpc.json @@ -1216,6 +1216,21 @@ "shared_folders" ], "type": "object" + }, + "Watched": { + "description": "Everyone this session is watching, sorted by name.", + "properties": { + "users": { + "items": { + "type": "string" + }, + "type": "array" + } + }, + "required": [ + "users" + ], + "type": "object" } } }, @@ -1875,6 +1890,58 @@ }, "summary": "What the server has said about a user so far." }, + { + "name": "user.watch", + "paramStructure": "by-name", + "params": [ + { + "name": "username", + "required": true, + "schema": { + "type": "string" + } + } + ], + "result": { + "name": "result", + "schema": { + "$ref": "#/components/schemas/Ack" + } + }, + "summary": "Watch a user, so the server keeps pushing their status." + }, + { + "name": "user.unwatch", + "paramStructure": "by-name", + "params": [ + { + "name": "username", + "required": true, + "schema": { + "type": "string" + } + } + ], + "result": { + "name": "result", + "schema": { + "$ref": "#/components/schemas/Ack" + } + }, + "summary": "Stop watching a user." + }, + { + "name": "user.watched", + "paramStructure": "by-name", + "params": [], + "result": { + "name": "result", + "schema": { + "$ref": "#/components/schemas/Watched" + } + }, + "summary": "Everyone this session is watching." + }, { "name": "shares.status", "paramStructure": "by-name", diff --git a/soulseek-rs/src/SKILL.md b/soulseek-rs/src/SKILL.md index 44594b3..2ec67ae 100644 --- a/soulseek-rs/src/SKILL.md +++ b/soulseek-rs/src/SKILL.md @@ -133,6 +133,13 @@ wishlist later rather than repeating the same search. be `null` when the server has not answered that half — `null` means unknown, not offline. +**`watch add `** / **`watch remove `** — prints nothing; exit 0 is +the confirmation. **`watch list`** — one object per watched user: `user`, +`status`. A watch lasts only as long as the session holding it, so these are +worth using against a running daemon (`--daemon`), where the connection stays +up and keeps receiving the server's status pushes. A one-shot run's watch ends +with the run. + **`whoami`** — one object: `user`, `server`, `listening`, `listen_port`, `shared_folders`, `shared_files`, `download_dir`, `privilege_seconds`. The cheapest way to prove the credentials and connection work. diff --git a/soulseek-rs/src/api.rs b/soulseek-rs/src/api.rs index 4006a84..bd39684 100644 --- a/soulseek-rs/src/api.rs +++ b/soulseek-rs/src/api.rs @@ -140,6 +140,12 @@ pub trait SessionApi: Send + Sync { fn request_user_info(&self, username: &str) -> Result<()>; fn user_info(&self, username: &str) -> Option; + /// Ask the server to keep pushing `username`'s status until unwatched. + fn watch_user(&self, username: &str) -> Result<()>; + fn unwatch_user(&self, username: &str) -> Result<()>; + /// Everyone this session is watching, sorted by name. + fn watched_users(&self) -> Vec; + /// The conversation this session already knows about. /// /// Empty locally, where the TUI's own snapshot on disk is the record. A @@ -364,6 +370,18 @@ impl SessionApi for Client { Self::user_info(self, username) } + fn watch_user(&self, username: &str) -> Result<()> { + Self::watch_user(self, username) + } + + fn unwatch_user(&self, username: &str) -> Result<()> { + Self::unwatch_user(self, username) + } + + fn watched_users(&self) -> Vec { + Self::watched_users(self) + } + fn shared_counts(&self) -> (u32, u32) { Self::shared_counts(self) } diff --git a/soulseek-rs/src/cli.rs b/soulseek-rs/src/cli.rs index 7b3f90c..e7ae4d6 100644 --- a/soulseek-rs/src/cli.rs +++ b/soulseek-rs/src/cli.rs @@ -207,6 +207,10 @@ pub enum Commands { /// What the server knows about another user User(UserArgs), + /// Keep a daemon updated about users you care about + #[command(subcommand)] + Watch(WatchCommand), + /// Confirm the credentials and connection work Whoami, @@ -571,6 +575,26 @@ pub struct WishRunArgs { pub sort: SortKey, } +/// Watching only lasts as long as the session doing it, so these are aimed at +/// a running daemon: it stays connected and keeps receiving the status pushes. +#[derive(Subcommand, Debug)] +pub enum WatchCommand { + /// Start watching a user + Add { + /// The user to watch + user: String, + }, + + /// Stop watching a user + Remove { + /// The user to stop watching + user: String, + }, + + /// List the users being watched + List, +} + #[derive(Subcommand, Debug)] pub enum RoomCommand { /// List public rooms and their user counts diff --git a/soulseek-rs/src/commands/mod.rs b/soulseek-rs/src/commands/mod.rs index 28161f7..f678aae 100644 --- a/soulseek-rs/src/commands/mod.rs +++ b/soulseek-rs/src/commands/mod.rs @@ -309,6 +309,7 @@ fn dispatch(ctx: &Ctx, command: Commands) -> CliResult { social::message_read(ctx, listen_span(duration, follow)) } Commands::User(args) => peer::user(ctx, &args), + Commands::Watch(command) => peer::watch(ctx, &command), Commands::Whoami => peer::whoami(ctx), // main.rs runs these itself: some need no account, the rest need the // config store in scope. The arms only keep the match exhaustive. diff --git a/soulseek-rs/src/commands/peer.rs b/soulseek-rs/src/commands/peer.rs index 4101175..ba6218c 100644 --- a/soulseek-rs/src/commands/peer.rs +++ b/soulseek-rs/src/commands/peer.rs @@ -1,9 +1,10 @@ //! Being a peer on the network: `serve`, `whoami`, and `user`. use super::{Ctx, Session}; -use crate::cli::{ServeArgs, UserArgs}; +use crate::cli::{ServeArgs, UserArgs, WatchCommand}; use crate::output::{ - CliError, CliResult, UploadRecord, UserRecord, WhoamiRecord, + CliError, CliResult, UploadRecord, UserRecord, WatchedUserRecord, + WhoamiRecord, }; use soulseek_rs::types::{UploadInfo, UploadStatus, UserInfo}; use std::collections::HashMap; @@ -94,6 +95,40 @@ pub fn user(ctx: &Ctx, args: &UserArgs) -> CliResult { Ok(()) } +/// Start or stop watching a user, or list who is being watched. +/// +/// A watch lives in the session that made it, so `add` against a one-shot CLI +/// run ends with that run. Pointed at a daemon it persists for that daemon's +/// lifetime, which is the point of the command. +pub fn watch(ctx: &Ctx, command: &WatchCommand) -> CliResult { + let session = Session::open(ctx)?; + match command { + WatchCommand::Add { user } => { + session.client.watch_user(user).map_err(|e| { + CliError::connection(format!("cannot watch {user}: {e}")) + })?; + ctx.out.status(&format!("watching {user}")); + } + WatchCommand::Remove { user } => { + session.client.unwatch_user(user).map_err(|e| { + CliError::connection(format!("cannot unwatch {user}: {e}")) + })?; + ctx.out.status(&format!("no longer watching {user}")); + } + WatchCommand::List => { + for user in session.client.watched_users() { + let status = session + .client + .user_info(&user) + .and_then(|info| info.presence) + .map(|p| p.status.to_string()); + ctx.out.emit(&WatchedUserRecord { user, status }); + } + } + } + Ok(()) +} + /// Stay online sharing files, and — with `--wishlist` — re-run the standing /// searches on the interval the server sets while doing so. pub fn serve( diff --git a/soulseek-rs/src/daemon/openrpc.rs b/soulseek-rs/src/daemon/openrpc.rs index e58b9c4..a489826 100644 --- a/soulseek-rs/src/daemon/openrpc.rs +++ b/soulseek-rs/src/daemon/openrpc.rs @@ -15,6 +15,7 @@ use super::proto::{ Method, QueryParams, RoomEventDto, RoomRef, SayParams, SearchResults, Searches, Seconds, SessionLossEvent, SharesStatus, SlotsParams, TransferRef, UploadInfoDto, Uploads, UserMessageDto, UserRef, UserResult, + Watched, }; use schemars::{JsonSchema, SchemaGenerator, generate::SchemaSettings}; use serde_json::{Value, json}; @@ -51,9 +52,11 @@ fn params_schema(method: Method, generator: &mut SchemaGenerator) -> Value { } Method::RoomSay => schema_of::(generator), Method::MessageSend => schema_of::(generator), - Method::BrowseUser | Method::UserRequest | Method::UserInfoOf => { - schema_of::(generator) - } + Method::BrowseUser + | Method::UserRequest + | Method::UserInfoOf + | Method::UserWatch + | Method::UserUnwatch => schema_of::(generator), Method::SharesSet => schema_of::(generator), // Methods that take no parameters. Method::DaemonStatus @@ -68,7 +71,8 @@ fn params_schema(method: Method, generator: &mut SchemaGenerator) -> Value { | Method::MessageHistory | Method::SearchList | Method::SharesStatusOf - | Method::SharesReindex => json!({ "type": "null" }), + | Method::SharesReindex + | Method::UserWatched => json!({ "type": "null" }), } } @@ -88,6 +92,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::UserWatched => schema_of::(generator), Method::MessageHistory => schema_of::(generator), Method::UserInfoOf => schema_of::(generator), Method::SharesStatusOf | Method::SharesSet | Method::SharesReindex => { @@ -112,7 +117,9 @@ fn result_schema(method: Method, generator: &mut SchemaGenerator) -> Value { | Method::MessageSend | Method::BrowseUser | Method::SearchForget - | Method::UserRequest => schema_of::(generator), + | Method::UserRequest + | Method::UserWatch + | Method::UserUnwatch => schema_of::(generator), } } @@ -172,6 +179,11 @@ 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::UserWatch => { + "Watch a user, so the server keeps pushing their status." + } + Method::UserUnwatch => "Stop watching a user.", + Method::UserWatched => "Everyone this session is watching.", 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/method.rs b/soulseek-rs/src/daemon/proto/method.rs index 96a69c2..3b71247 100644 --- a/soulseek-rs/src/daemon/proto/method.rs +++ b/soulseek-rs/src/daemon/proto/method.rs @@ -139,6 +139,13 @@ pub struct Members { pub users: Vec, } +/// Everyone this session is watching, sorted by name. +#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] +#[cfg_attr(test, derive(schemars::JsonSchema))] +pub struct Watched { + pub users: 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..7bd337a 100644 --- a/soulseek-rs/src/daemon/proto/mod.rs +++ b/soulseek-rs/src/daemon/proto/mod.rs @@ -106,6 +106,9 @@ wire_vocabulary! { BrowseUser => "browse.user", UserRequest => "user.request", UserInfoOf => "user.info", + UserWatch => "user.watch", + UserUnwatch => "user.unwatch", + UserWatched => "user.watched", SharesStatusOf => "shares.status", SharesSet => "shares.set", SharesReindex => "shares.reindex", diff --git a/soulseek-rs/src/daemon/server/mod.rs b/soulseek-rs/src/daemon/server/mod.rs index 718a23a..0bd8470 100644 --- a/soulseek-rs/src/daemon/server/mod.rs +++ b/soulseek-rs/src/daemon/server/mod.rs @@ -13,7 +13,7 @@ use super::proto::{ MessageParams, Messages, Method, OPENRPC, PROTOCOL_VERSION, QueryParams, RoomRef, RpcError, SayParams, SearchResultDto, SearchResults, SearchSummary, Searches, Seconds, SharesStatus, SlotsParams, TransferRef, - UploadInfoDto, Uploads, UserInfoDto, UserRef, UserResult, + UploadInfoDto, Uploads, UserInfoDto, UserRef, UserResult, Watched, }; use crate::api::SessionApi; use crate::output::Exit; @@ -306,6 +306,24 @@ impl Daemon { }) } + Method::UserWatch => { + let user: UserRef = parse(params)?; + self.session.watch_user(&user.username).map_err(|e| { + RpcError::application(Exit::Connection, e.to_string()) + })?; + ok(Ack::OK) + } + Method::UserUnwatch => { + let user: UserRef = parse(params)?; + self.session.unwatch_user(&user.username).map_err(|e| { + RpcError::application(Exit::Connection, e.to_string()) + })?; + ok(Ack::OK) + } + Method::UserWatched => ok(Watched { + users: self.session.watched_users(), + }), + Method::SharesStatusOf => ok(self.shares()), Method::SharesSet => { let directories: DirectoriesParams = parse(params)?; @@ -593,6 +611,49 @@ mod tests { ); } + #[test] + fn a_watch_is_readable_back_and_survives_until_unwatched() { + // The three methods are only useful together: what `user.watch` + // records, `user.watched` must report, and `user.unwatch` must undo. + let daemon = daemon_with_token("t"); + let user = |name: &str| json!({ "username": name }); + + daemon + .call(Method::UserWatch, user("alice")) + .expect("watch"); + daemon.call(Method::UserWatch, user("bob")).expect("watch"); + let watched = daemon + .call(Method::UserWatched, Value::Null) + .expect("watched"); + assert_eq!(watched["users"], json!(["alice", "bob"])); + + daemon + .call(Method::UserUnwatch, user("alice")) + .expect("unwatch"); + let watched = daemon + .call(Method::UserWatched, Value::Null) + .expect("watched"); + assert_eq!( + watched["users"], + json!(["bob"]), + "unwatching one must not clear the rest" + ); + } + + #[test] + fn watching_the_same_user_twice_lists_them_once() { + let daemon = daemon_with_token("t"); + let alice = json!({ "username": "alice" }); + daemon + .call(Method::UserWatch, alice.clone()) + .expect("watch"); + daemon.call(Method::UserWatch, alice).expect("watch"); + let watched = daemon + .call(Method::UserWatched, Value::Null) + .expect("watched"); + assert_eq!(watched["users"], json!(["alice"])); + } + #[test] fn stop_is_recorded_so_the_daemon_can_wind_down() { let daemon = daemon_with_token("t"); @@ -605,11 +666,17 @@ mod tests { // A session that answers nothing: enough to exercise dispatch, parameter // validation, and the auth gate without a Soulseek server. - struct SilentSession; + /// A session that answers nothing, except that it remembers watches: + /// the watch methods are only meaningful if what one call records the + /// next call can read back. + #[derive(Default)] + struct SilentSession { + watched: Mutex>, + } fn daemon_with_token(token: &str) -> Daemon { Daemon { - session: Arc::new(SilentSession), + session: Arc::new(SilentSession::default()), hub: Arc::new(Hub::new()), browses: Arc::new(PendingBrowses::default()), downloads: Arc::new(Mutex::new(Vec::new())), @@ -792,6 +859,24 @@ mod tests { ) -> soulseek_rs::Result<()> { Ok(()) } + fn watch_user(&self, username: &str) -> soulseek_rs::Result<()> { + let mut watched = self.watched.lock().expect("watch list"); + if !watched.iter().any(|u| u == username) { + watched.push(username.to_string()); + watched.sort(); + } + Ok(()) + } + fn unwatch_user(&self, username: &str) -> soulseek_rs::Result<()> { + self.watched + .lock() + .expect("watch list") + .retain(|u| u != username); + Ok(()) + } + fn watched_users(&self) -> Vec { + self.watched.lock().expect("watch list").clone() + } fn user_info(&self, _username: &str) -> Option { None } diff --git a/soulseek-rs/src/output.rs b/soulseek-rs/src/output.rs index d50a853..d8fe980 100644 --- a/soulseek-rs/src/output.rs +++ b/soulseek-rs/src/output.rs @@ -383,6 +383,25 @@ impl Record for RoomMemberRecord { } } +/// One user this session is watching, with whatever the server has already +/// said about them. The status is `None` until the watch reply lands. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct WatchedUserRecord { + pub user: String, + /// `online`, `away`, or `offline`. + pub status: Option, +} + +impl Record for WatchedUserRecord { + fn text(&self) -> String { + format!( + "{}\t{}", + sanitize(&self.user), + self.status.clone().unwrap_or_else(|| "-".to_string()) + ) + } +} + /// One configured share folder. #[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub struct ShareRecord { diff --git a/soulseek-rs/src/remote/mod.rs b/soulseek-rs/src/remote/mod.rs index e236d05..24277f8 100644 --- a/soulseek-rs/src/remote/mod.rs +++ b/soulseek-rs/src/remote/mod.rs @@ -15,7 +15,7 @@ use crate::daemon::proto::{ Members, MessageParams, Messages, Method, PROTOCOL_VERSION, QueryParams, Request, Response, RoomRef, RpcError, SayParams, SearchResults, Searches, Seconds, SharesStatus, SlotsParams, TransferRef, Uploads, UserRef, - UserResult, + UserResult, Watched, }; use mirror::Mirror; use serde::de::DeserializeOwned; @@ -757,6 +757,30 @@ impl SessionApi for RemoteSession { ) } + fn watch_user(&self, username: &str) -> Result<()> { + self.tell( + Method::UserWatch, + UserRef { + username: username.to_string(), + }, + ) + } + + fn unwatch_user(&self, username: &str) -> Result<()> { + self.tell( + Method::UserUnwatch, + UserRef { + username: username.to_string(), + }, + ) + } + + fn watched_users(&self) -> Vec { + self.request::<_, Watched>(Method::UserWatched, ()) + .map(|watched| watched.users) + .unwrap_or_default() + } + fn user_info(&self, username: &str) -> Option { self.request::<_, UserResult>( Method::UserInfoOf, diff --git a/soulseek-rs/src/ui/main_tui/mod.rs b/soulseek-rs/src/ui/main_tui/mod.rs index 0b80188..1fe8624 100644 --- a/soulseek-rs/src/ui/main_tui/mod.rs +++ b/soulseek-rs/src/ui/main_tui/mod.rs @@ -489,6 +489,15 @@ mod tests { ) -> soulseek_rs::Result<()> { Ok(()) } + fn watch_user(&self, _username: &str) -> soulseek_rs::Result<()> { + Ok(()) + } + fn unwatch_user(&self, _username: &str) -> soulseek_rs::Result<()> { + Ok(()) + } + fn watched_users(&self) -> Vec { + Vec::new() + } fn user_info(&self, _username: &str) -> Option { None } From 60f7100e309d98f4b0749fa7adfb8cd26ae637d4 Mon Sep 17 00:00:00 2001 From: Michel de Graaf Date: Fri, 28 Aug 2026 13:27:09 +0000 Subject: [PATCH 05/12] 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 06/12] 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 9c1a6d317e0cf256defb5a83cd0b1e983ca55206 Mon Sep 17 00:00:00 2001 From: Michel de Graaf Date: Fri, 28 Aug 2026 14:42:44 +0000 Subject: [PATCH 07/12] test(e2e): cover watching against soulfind, and read the reply as written The e2e job proved nothing about watching: it passed because the suite has no test that exercises it. Two now run against a real soulfind. - a watch reply carries status and share statistics in one message, checked against a subject sharing a known two files, and unwatching drops both the watch and the snapshot - a username the server does not know is answered with an absence flag and must not stay in the watch list Also reads the two obsolete fields of the reply as the two 32-bit values the server writes, matching soulfind and the sibling GetUserStats (36) handler. The byte count is identical, so this is not a behaviour change -- but the handler now says what the wire actually carries. --- .../src/message/server/watch_user.rs | 10 +- soulseek-rs-lib/tests/e2e.rs | 98 +++++++++++++++++++ 2 files changed, 106 insertions(+), 2 deletions(-) diff --git a/soulseek-rs-lib/src/message/server/watch_user.rs b/soulseek-rs-lib/src/message/server/watch_user.rs index ecbd50d..1499189 100644 --- a/soulseek-rs-lib/src/message/server/watch_user.rs +++ b/soulseek-rs-lib/src/message/server/watch_user.rs @@ -31,7 +31,12 @@ impl MessageHandler for WatchUserHandler { let watched = if exists { let status = message.read_int32(); let average_speed = message.read_int32(); - let _upload_number = message.read_int64(); // obsolete + // Two obsolete 32-bit fields, exactly as GetUserStats (36) + // carries them -- not one 64-bit value. The byte count is the + // same either way, but reading them as the server writes them + // keeps this handler and its sibling honest. + let _upload_number = message.read_int32(); // obsolete, always 0 + let _unknown = message.read_int32(); // obsolete, always 0 let shared_files = message.read_int32(); let shared_folders = message.read_int32(); ServerMessage::WatchedUserReceived { @@ -71,7 +76,8 @@ mod tests { .write_int8(1) // exists .write_int32(2) // status: online .write_int32(1024) // average speed - .write_int64(0) // obsolete upload number + .write_int32(0) // obsolete upload number + .write_int32(0) // obsolete unknown .write_int32(4321) // shared files .write_int32(77); // shared folders }); diff --git a/soulseek-rs-lib/tests/e2e.rs b/soulseek-rs-lib/tests/e2e.rs index 072bc21..430a4ce 100644 --- a/soulseek-rs-lib/tests/e2e.rs +++ b/soulseek-rs-lib/tests/e2e.rs @@ -2622,3 +2622,101 @@ fn a_peer_that_hangs_up_its_control_connection_still_delivers_the_file() { let _ = std::fs::remove_dir_all(&download_dir); } + +#[test] +fn watching_a_user_returns_their_status_and_share_counts() { + let server = server_or_skip!(); + + // A subject with a known share, so the watch reply's statistics are + // checkable rather than merely present. + let share_dir = unique_download_dir().join("watch_share"); + std::fs::create_dir_all(share_dir.join("album")).unwrap(); + for name in ["one.flac", "two.flac"] { + std::fs::write(share_dir.join("album").join(name), b"xxxx").unwrap(); + } + let mut subject = Client::with_settings(ClientSettings { + shared_directories: vec![share_dir.display().to_string()], + ..server.settings("e2e_watch_subject", "pw") + }); + subject.connect().expect("subject connect"); + assert!(subject.login().expect("subject login")); + + let mut watcher = + Client::with_settings(server.settings("e2e_watch_watcher", "pw")); + watcher.connect().expect("watcher connect"); + assert!(watcher.login().expect("watcher login")); + + // Let the subject's share counts reach the server before watching, so + // the reply describes a user it already knows the statistics for. + std::thread::sleep(Duration::from_millis(750)); + + watcher + .watch_user("e2e_watch_subject") + .expect("watch the subject"); + assert_eq!( + watcher.watched_users(), + vec!["e2e_watch_subject".to_string()] + ); + + // One reply carries both halves, unlike the two GetUserStatus and + // GetUserStats answers request_user_info waits on. + let deadline = Instant::now() + Duration::from_secs(15); + let mut info = None; + while Instant::now() < deadline { + match watcher.user_info("e2e_watch_subject") { + Some(found) if found.is_complete() => { + info = Some(found); + break; + } + _ => std::thread::sleep(Duration::from_millis(100)), + } + } + let info = info.expect("the watch reply should carry status and stats"); + + let presence = info.presence.expect("presence should have arrived"); + assert!( + presence.status.is_reachable(), + "a logged-in user should not read as offline, got {}", + presence.status + ); + let stats = info.stats.expect("stats should have arrived"); + assert_eq!( + stats.shared_files, 2, + "the file count must survive the obsolete fields ahead of it" + ); + assert_eq!(stats.shared_folders, 1); + + // Unwatching drops the user and what we knew about them, so a later + // re-watch reports a fresh answer rather than this stale one. + watcher + .unwatch_user("e2e_watch_subject") + .expect("unwatch the subject"); + assert!(watcher.watched_users().is_empty()); + assert!(watcher.user_info("e2e_watch_subject").is_none()); +} + +#[test] +fn watching_a_user_the_server_does_not_know_reports_their_absence() { + let server = server_or_skip!(); + + let mut watcher = + Client::with_settings(server.settings("e2e_watch_ghost_asker", "pw")); + watcher.connect().expect("watcher connect"); + assert!(watcher.login().expect("watcher login")); + + watcher + .watch_user("e2e_watch_nobody_here") + .expect("watch an unknown user"); + + // The server answers that it has no such user; the watch is dropped + // rather than left showing a permanently blank row. + let deadline = Instant::now() + Duration::from_secs(10); + while Instant::now() < deadline && !watcher.watched_users().is_empty() { + std::thread::sleep(Duration::from_millis(100)); + } + assert!( + watcher.watched_users().is_empty(), + "a username the server does not know must not stay watched" + ); + assert!(watcher.user_info("e2e_watch_nobody_here").is_none()); +} From 62c99e4ab839c4f98c8601bbcfbbb35f4edb0358 Mon Sep 17 00:00:00 2001 From: Michel de Graaf Date: Fri, 28 Aug 2026 14:55:02 +0000 Subject: [PATCH 08/12] 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) } } From 693926434d39bff4a3107e2cf77887a20d0b0a98 Mon Sep 17 00:00:00 2001 From: Michel de Graaf Date: Tue, 18 Aug 2026 08:40:23 +0200 Subject: [PATCH 09/12] chore: release v14.1.2 --- CHANGELOG.md | 6 ++++++ Cargo.lock | 4 ++-- Cargo.toml | 2 +- soulseek-rs/Cargo.toml | 2 +- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b5a89e..7e46cdd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [14.1.2](https://github.com/michel/soulseek-rs/compare/v14.1.1...v14.1.2) - 2026-08-18 + +### Other + +- drop bot wording in favor of client + ## [14.1.1](https://github.com/michel/soulseek-rs/compare/v14.1.0...v14.1.1) - 2026-08-17 ### Fixed diff --git a/Cargo.lock b/Cargo.lock index 25fb7b8..882d003 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2545,7 +2545,7 @@ checksum = "8ed6a63f02c8539c91a8685a86f4099661ba3da017932f6ebbea6de3f0fa7c90" [[package]] name = "soulseek-rs" -version = "14.1.1" +version = "14.1.2" dependencies = [ "chrono", "clap", @@ -2568,7 +2568,7 @@ dependencies = [ [[package]] name = "soulseek-rs-lib" -version = "14.1.1" +version = "14.1.2" dependencies = [ "libc", ] diff --git a/Cargo.toml b/Cargo.toml index 4cfe559..d516621 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ members = ["soulseek-rs-lib", "soulseek-rs"] resolver = "2" [workspace.package] -version = "14.1.1" +version = "14.1.2" authors = ["Michel de Graaf"] edition = "2024" license = "MIT" diff --git a/soulseek-rs/Cargo.toml b/soulseek-rs/Cargo.toml index 2464872..d37be34 100644 --- a/soulseek-rs/Cargo.toml +++ b/soulseek-rs/Cargo.toml @@ -25,7 +25,7 @@ path = "src/lib.rs" workspace = true [dependencies] -soulseek-rs-lib = { version = "14.1.1", path = "../soulseek-rs-lib" } +soulseek-rs-lib = { version = "14.1.2", path = "../soulseek-rs-lib" } clap = { version = "4.6.2", features = ["derive", "color", "wrap_help", "env"] } clap_complete = "4.6.7" From a9f4ab5e5b5bd4d4aae5ab462c15cbba4b68cb07 Mon Sep 17 00:00:00 2001 From: Michel de Graaf Date: Tue, 18 Aug 2026 16:09:00 +0200 Subject: [PATCH 10/12] docs(website): list Player and Staccato, fix install pill overflow on mobile --- website/src/components/ui/command-pill.tsx | 2 +- website/src/pages/community.tsx | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/website/src/components/ui/command-pill.tsx b/website/src/components/ui/command-pill.tsx index 65a0f05..4f04ed9 100644 --- a/website/src/components/ui/command-pill.tsx +++ b/website/src/components/ui/command-pill.tsx @@ -24,7 +24,7 @@ export const CommandPill = ({ text, className }: CommandPillProps) => { - {text} + {text}