diff --git a/soulseek-rs-lib/src/client.rs b/soulseek-rs-lib/src/client.rs index 875f7ce..3319abc 100644 --- a/soulseek-rs-lib/src/client.rs +++ b/soulseek-rs-lib/src/client.rs @@ -1101,6 +1101,23 @@ impl Client { } } + /// Remove every download for `username`/`filename` regardless of status. + /// Call this before re-issuing [`Client::download`] for a failed download, + /// otherwise the stale entry (whose md5-derived token equals the retry's) + /// shadows the fresh one and its completion is misrouted. + /// + /// Returns whether anything was removed. + #[must_use] + pub fn remove_download(&self, username: &str, filename: &str) -> bool { + match self.context.write_safe() { + Ok(mut ctx) => ctx.downloads.remove_by_file(username, filename), + Err(e) => { + error!("[client] remove_download: {}", e); + false + } + } + } + pub fn download( &self, filename: String, diff --git a/soulseek-rs-lib/src/download_store.rs b/soulseek-rs-lib/src/download_store.rs index e5fcf60..c07f89b 100644 --- a/soulseek-rs-lib/src/download_store.rs +++ b/soulseek-rs-lib/src/download_store.rs @@ -84,6 +84,17 @@ impl DownloadStore { true } + /// Remove every download matching `username`/`filename` regardless of + /// status. Used before retrying a failed download so the stale entry (whose + /// md5-derived token collides with the retry's) can't shadow the fresh one. + /// Returns whether anything was removed. + pub fn remove_by_file(&mut self, username: &str, filename: &str) -> bool { + let before = self.downloads.len(); + self.downloads + .retain(|d| !(d.username == username && d.filename == filename)); + self.downloads.len() != before + } + pub fn pause_by_file(&mut self, username: &str, filename: &str) -> bool { let Some(download) = self.get_by_file_mut(username, filename) else { return false; @@ -272,6 +283,28 @@ mod tests { assert!(store.get_by_token(456).is_some()); } + #[test] + fn remove_by_file_removes_regardless_of_status() { + let mut store = DownloadStore::new(); + // A failed download (the retry case) plus a same-name duplicate that a + // token-migration could have left behind — both must go. + let mut failed = make_download(1, DownloadStatus::Failed(None)); + failed.filename = "song.mp3".to_string(); + store.add(failed); + let mut dup = make_download(2, DownloadStatus::Queued); + dup.filename = "song.mp3".to_string(); + store.add(dup); + let mut other = make_download(3, DownloadStatus::Queued); + other.filename = "other.mp3".to_string(); + store.add(other); + + assert!(store.remove_by_file("peer", "song.mp3")); + assert!(store.get_by_token(1).is_none()); + assert!(store.get_by_token(2).is_none()); + assert!(store.get_by_token(3).is_some(), "other file untouched"); + assert!(!store.remove_by_file("peer", "song.mp3"), "idempotent"); + } + #[test] fn collect_failed_tokens_notifies_and_lists_matching() { let mut store = DownloadStore::new(); diff --git a/soulseek-rs/src/models/rooms.rs b/soulseek-rs/src/models/rooms.rs index 01913ec..46464a7 100644 --- a/soulseek-rs/src/models/rooms.rs +++ b/soulseek-rs/src/models/rooms.rs @@ -284,6 +284,16 @@ impl RoomsState { } } } + // The active room's member list may have grown/shrunk (join/leave or a + // wholesale replace on Joined); keep the selection highlight in range so + // 'b'/'m' never act on the wrong member or a phantom one. + self.clamp_user_selected(); + } + + /// Keep `user_selected` within the active room's member list. + fn clamp_user_selected(&mut self) { + let len = self.active_room().map_or(0, |r| r.users.len()); + self.user_selected = self.user_selected.min(len.saturating_sub(1)); } /// Index of the open room `name`, creating an empty tab if necessary. @@ -526,6 +536,54 @@ mod tests { assert_eq!(state.selected_user(), None); // blues has no members yet } + #[test] + fn user_selection_stays_in_range_when_members_leave() { + let mut state = RoomsState::new(); + state.apply_event( + RoomEvent::Joined { + room: "jazz".to_string(), + users: vec![ + "alice".to_string(), + "bob".to_string(), + "carol".to_string(), + ], + }, + None, + ); + state.focus_or_open("jazz"); + state.user_selected = 2; // carol highlighted (last) + + // carol leaves: selection must clamp to the new last index, not point + // past the end (which would make 'b'/'m' silent no-ops). + state.apply_event( + RoomEvent::UserLeft { + room: "jazz".to_string(), + username: "carol".to_string(), + }, + None, + ); + assert_eq!(state.user_selected, 1); + assert_eq!(state.selected_user().as_deref(), Some("bob")); + + // Everyone leaves: selection collapses to 0 and yields no user. + state.apply_event( + RoomEvent::UserLeft { + room: "jazz".to_string(), + username: "bob".to_string(), + }, + None, + ); + state.apply_event( + RoomEvent::UserLeft { + room: "jazz".to_string(), + username: "alice".to_string(), + }, + None, + ); + assert_eq!(state.user_selected, 0); + assert_eq!(state.selected_user(), None); + } + #[test] fn user_joined_and_left_update_membership() { let mut state = RoomsState::new(); diff --git a/soulseek-rs/src/ui/main_tui.rs b/soulseek-rs/src/ui/main_tui.rs index 29c2eea..9583108 100644 --- a/soulseek-rs/src/ui/main_tui.rs +++ b/soulseek-rs/src/ui/main_tui.rs @@ -1324,9 +1324,13 @@ impl MainTui { let client = self.client.clone(); let sender = self.state.downloads_sender_channel.clone().unwrap(); - // Drop the old failed entry; the retry pushes a fresh one. + // Drop the old failed entry from both the UI list and the client's + // download store; otherwise the stale entry (same md5-derived token and + // filename) shadows the retry and its completion is misrouted, leaving + // the retried download stuck showing "Queued". self.state.downloads.remove(index); self.select_download_after_removal(index); + let _ = self.client.remove_download(&username, &filename); thread::spawn(move || { match client.download(filename.clone(), username, size, directory) {