fix a bug where adding track using `now` when shuffle is on does not work as expected

3 files changed, 149 insertions, 29 deletions

diff --git a/src/daemon.rs b/src/daemon.rs
index 21f1a3b..bdee362 100644
--- a/src/daemon.rs
+++ b/src/daemon.rs
@@ -1075,7 +1075,7 @@ fn process_command(
                             let new_pos = q.position() + 1;
                             q.set_position(new_pos);
                         }
-                        if let Some(track) = q.track_at(q.position()) {
+                        if let Some(track) = q.current() {
                             audio.play(track.path.clone());
                             spawn_send_now_playing(scrobbler, track);
                             if let Ok(mut s) = scrobble_state.lock() {
@@ -1181,12 +1181,12 @@ fn process_command(
                     match position {
                         "now" => {
                             spawn_try_scrobble_current(scrobbler, scrobble_state);
-                            q.push_track(track.clone());
+                            q.insert_after_current(PathBuf::from(path_str.clone()));
                             if !was_empty {
                                 let new_pos = q.position() + 1;
                                 q.set_position(new_pos);
                             }
-                            if let Some(track) = q.track_at(q.position()) {
+                            if let Some(track) = q.current() {
                                 audio.play(track.path.clone());
                                 spawn_send_now_playing(scrobbler, track);
                                 if let Ok(mut s) = scrobble_state.lock() {
@@ -1199,7 +1199,7 @@ fn process_command(
                             format!("Playing now: {}\n", path_str)
                         }
                         "next" => {
-                            q.push_track(track);
+                            q.insert_after_current(PathBuf::from(path_str.clone()));
                             format!("Queued next: {}\n", path_str)
                         }
                         _ => {
@@ -1237,7 +1237,7 @@ fn process_command(
                                     let new_pos = q.position() + 1;
                                     q.set_position(new_pos);
                                 }
-                                if let Some(track) = q.track_at(q.position()) {
+                                if let Some(track) = q.current() {
                                     audio.play(track.path.clone());
                                     spawn_send_now_playing(scrobbler, track);
                                     if let Ok(mut s) = scrobble_state.lock() {
@@ -1291,7 +1291,7 @@ fn process_command(
                                             let new_pos = q.position() + 1;
                                             q.set_position(new_pos);
                                         }
-                                        if let Some(track) = q.track_at(q.position()) {
+                                        if let Some(track) = q.current() {
                                             audio.play(track.path.clone());
                                             spawn_send_now_playing(scrobbler, track);
                                             if let Ok(mut s) = scrobble_state.lock() {
@@ -1389,7 +1389,7 @@ fn process_command(
                                                 let new_pos = q.position() + 1;
                                                 q.set_position(new_pos);
                                             }
-                                            if let Some(track) = q.track_at(q.position()) {
+                                            if let Some(track) = q.current() {
                                                 audio.play(track.path.clone());
                                                 spawn_send_now_playing(scrobbler, track);
                                                 if let Ok(mut s) = scrobble_state.lock() {
diff --git a/src/queue.rs b/src/queue.rs
index c3f8dbe..da79441 100644
--- a/src/queue.rs
+++ b/src/queue.rs
@@ -5,7 +5,9 @@
  * https://codeberg.org/salmonik/RSPlayer
 */
 
+use rand::rngs::StdRng;
 use rand::seq::SliceRandom;
+use rand::SeedableRng;
 use std::path::{Path, PathBuf};
 
 #[derive(Debug, Clone, PartialEq)]
@@ -202,6 +204,7 @@ pub struct Queue {
     position: usize,
     shuffle: ShuffleMode,
     shuffled_indices: Vec<usize>,
+    rng: StdRng,
 }
 
 impl Default for Queue {
@@ -217,6 +220,20 @@ impl Queue {
             position: 0,
             shuffle: ShuffleMode::None,
             shuffled_indices: Vec::new(),
+            rng: rand::make_rng(),
+        }
+    }
+
+    /// Construct a queue with a fixed RNG seed, making shuffle order fully
+    /// reproducible. Used by tests and handy for debugging a specific order.
+    #[allow(dead_code)] // used only by integration tests (bin crate does not link the lib)
+    pub fn with_seed(seed: u64) -> Self {
+        Queue {
+            tracks: Vec::new(),
+            position: 0,
+            shuffle: ShuffleMode::None,
+            shuffled_indices: Vec::new(),
+            rng: StdRng::seed_from_u64(seed),
         }
     }
 
@@ -225,7 +242,6 @@ impl Queue {
         self.update_shuffle_indices();
     }
 
-    #[allow(dead_code)]
     pub fn push_track(&mut self, track: Track) {
         self.tracks.push(track);
         self.update_shuffle_indices();
@@ -234,11 +250,27 @@ impl Queue {
     pub fn insert_after_current(&mut self, path: PathBuf) {
         if self.tracks.is_empty() {
             self.tracks.push(Track::from_path(path));
-        } else {
-            let insert_idx = self.position + 1;
-            self.tracks.insert(insert_idx, Track::from_path(path));
+            self.update_shuffle_indices();
+            return;
         }
-        self.update_shuffle_indices();
+        let insert_idx = self.position + 1;
+        self.tracks.insert(insert_idx, Track::from_path(path));
+        if self.shuffle == ShuffleMode::None {
+            self.update_shuffle_indices();
+            return;
+        }
+        // Under shuffle, splice the new entry into the play order right after
+        // the current slot instead of re-shuffling the tail: the inserted
+        // track deterministically occupies slot `position + 1`, so "play
+        // now/next" is meaningful and `current()` agrees with what the daemon
+        // starts playing. The remaining play order is preserved (indices that
+        // shifted right in `tracks` are bumped), not re-randomized.
+        for v in self.shuffled_indices.iter_mut() {
+            if *v >= insert_idx {
+                *v += 1;
+            }
+        }
+        self.shuffled_indices.insert(self.position + 1, insert_idx);
     }
 
     pub fn track_at(&self, idx: usize) -> Option<&Track> {
@@ -309,10 +341,9 @@ impl Queue {
         self.shuffled_indices = (0..len).collect();
         match self.shuffle {
             ShuffleMode::All => {
-                let mut rng = rand::rng();
                 let pos = self.position.min(len.saturating_sub(1));
                 if pos + 1 < len {
-                    self.shuffled_indices[pos + 1..].shuffle(&mut rng);
+                    self.shuffled_indices[pos + 1..].shuffle(&mut self.rng);
                 }
             }
             ShuffleMode::Album => {
@@ -330,10 +361,14 @@ impl Queue {
         self.shuffled_indices = (0..len).collect();
 
         if len > 1 {
-            let mut rng = rand::rng();
             let pos = self.position.min(len.saturating_sub(1));
             if pos + 1 < len {
-                self.shuffle_album_range(pos + 1..len, &mut rng);
+                Self::shuffle_album_range(
+                    &self.tracks,
+                    &mut self.shuffled_indices,
+                    pos + 1..len,
+                    &mut self.rng,
+                );
             }
         }
     }
@@ -352,7 +387,6 @@ impl Queue {
         }
 
         let range_start = pos + 1;
-        let mut rng = rand::rng();
 
         // Build album groups within the range, shuffling tracks within each group
         let mut groups: Vec<Vec<usize>> = Vec::new();
@@ -368,7 +402,7 @@ impl Queue {
             } else if album != current_album {
                 // Shuffle tracks within this album group before adding it
                 if current_group.len() > 1 {
-                    current_group.shuffle(&mut rng);
+                    current_group.shuffle(&mut self.rng);
                 }
                 groups.push(std::mem::take(&mut current_group));
                 current_group.push(i);
@@ -379,13 +413,13 @@ impl Queue {
         }
         if !current_group.is_empty() {
             if current_group.len() > 1 {
-                current_group.shuffle(&mut rng);
+                current_group.shuffle(&mut self.rng);
             }
             groups.push(current_group);
         }
 
         // Shuffle the album group order
-        groups.shuffle(&mut rng);
+        groups.shuffle(&mut self.rng);
 
         let mut new_indices: Vec<usize> = self.shuffled_indices[..=pos].to_vec();
         for group in &groups {
@@ -394,7 +428,12 @@ impl Queue {
         self.shuffled_indices = new_indices;
     }
 
-    fn shuffle_album_range(&mut self, range: std::ops::Range<usize>, rng: &mut impl rand::Rng) {
+    fn shuffle_album_range(
+        tracks: &[Track],
+        indices: &mut [usize],
+        range: std::ops::Range<usize>,
+        rng: &mut impl rand::Rng,
+    ) {
         if range.start >= range.end {
             return;
         }
@@ -404,7 +443,7 @@ impl Queue {
         let mut range_start: Option<usize> = None;
 
         for i in range.clone() {
-            let album = self.tracks.get(i).and_then(|t| t.album.as_deref());
+            let album = tracks.get(i).and_then(|t| t.album.as_deref());
             match (current_album, range_start) {
                 (None, None) => {
                     current_album = album;
@@ -424,7 +463,7 @@ impl Queue {
 
         for (start, end) in album_ranges {
             if start + 1 < end {
-                self.shuffled_indices[start..end].shuffle(rng);
+                indices[start..end].shuffle(rng);
             }
         }
     }
@@ -498,13 +537,17 @@ impl Queue {
         if self.position != 0 || self.tracks.len() <= 1 {
             return;
         }
-        let mut rng = rand::rng();
         match self.shuffle {
             ShuffleMode::All => {
-                self.shuffled_indices.shuffle(&mut rng);
+                self.shuffled_indices.shuffle(&mut self.rng);
             }
             ShuffleMode::Album => {
-                self.shuffle_album_range(0..self.tracks.len(), &mut rng);
+                Self::shuffle_album_range(
+                    &self.tracks,
+                    &mut self.shuffled_indices,
+                    0..self.tracks.len(),
+                    &mut self.rng,
+                );
             }
             ShuffleMode::Albums => {
                 let mut groups: Vec<Vec<usize>> = Vec::new();
@@ -519,7 +562,7 @@ impl Queue {
                         current_album = album;
                     } else if album != current_album {
                         if current_group.len() > 1 {
-                            current_group.shuffle(&mut rng);
+                            current_group.shuffle(&mut self.rng);
                         }
                         groups.push(std::mem::take(&mut current_group));
                         current_group.push(i);
@@ -530,12 +573,12 @@ impl Queue {
                 }
                 if !current_group.is_empty() {
                     if current_group.len() > 1 {
-                        current_group.shuffle(&mut rng);
+                        current_group.shuffle(&mut self.rng);
                     }
                     groups.push(current_group);
                 }
 
-                groups.shuffle(&mut rng);
+                groups.shuffle(&mut self.rng);
 
                 self.shuffled_indices = groups.concat();
             }
diff --git a/tests/queue.rs b/tests/queue.rs
index d338f2a..67cbabf 100644
--- a/tests/queue.rs
+++ b/tests/queue.rs
@@ -567,6 +567,83 @@ fn test_shuffle_off_keeps_unplayed_duplicate_entries() {
     assert_eq!(q.position(), 1);
 }
 
+#[test]
+fn test_shuffle_play_now_current_is_inserted_track() {
+    // BUG: under shuffle, `insert_after_current` re-shuffles the tail
+    // (`update_shuffle_indices`), so the inserted track's play-order slot is a
+    // random draw instead of `position + 1`. The daemon's "now" flow (insert →
+    // `set_position(pos+1)` → play `track_at(position)`, src/daemon.rs) then
+    // starts the right file while `current()`/`next()`/the queue display
+    // follow a *different* track, and auto-advance can replay or skip the
+    // inserted track entirely.
+    //
+    // Contract: after the "now" sequence, `current()` must be the inserted
+    // track. The seed sweep keeps the test deterministic — for a fixed seed the
+    // outcome never varies between runs, and pre-fix a given seed satisfies the
+    // invariant with probability 1/(tail size) = 1/3, so the chance all 40
+    // seeds accidentally pass is ~3e-19. The fix makes the invariant hold
+    // structurally (insert into the play order at `position+1`), independent
+    // of the seed.
+    let inserted = PathBuf::from("/music/new.mp3");
+    for seed in 1..=40u64 {
+        let mut q = Queue::with_seed(seed);
+        add_track(&mut q, "/music/a.mp3");
+        add_track(&mut q, "/music/b.mp3");
+        add_track(&mut q, "/music/c.mp3");
+        q.set_shuffle(ShuffleMode::All);
+
+        // `rsp play --now new.mp3` with a non-empty queue: insert after the
+        // current track and move the cursor onto it.
+        q.insert_after_current(inserted.clone());
+        let new_pos = q.position() + 1;
+        q.set_position(new_pos);
+
+        assert_eq!(q.position(), 1);
+        let cur = q.current().expect("queue is not empty");
+        assert_eq!(
+            cur.path, inserted,
+            "seed {}: current() after play-now is {:?}, expected {:?}; shuffled_indices={:?}",
+            seed,
+            cur.path,
+            inserted,
+            q.shuffled_indices()
+        );
+    }
+}
+
+// ---------- Seedable RNG ----------
+// The queue uses a seeded PRNG (rand::rngs::StdRng) instead of the OS-seeded
+// thread RNG, so shuffle order is reproducible via Queue::with_seed. Shuffle
+// is not a security domain — a PRNG suffices, and deterministic tests need a
+// fixed seed.
+
+#[test]
+fn test_same_seed_produces_same_shuffle_order() {
+    let mut q1 = Queue::with_seed(0x5EED);
+    let mut q2 = Queue::with_seed(0x5EED);
+    for i in 0..10 {
+        add_track(&mut q1, &format!("/music/t{}.mp3", i));
+        add_track(&mut q2, &format!("/music/t{}.mp3", i));
+    }
+    q1.set_shuffle(ShuffleMode::All);
+    q2.set_shuffle(ShuffleMode::All);
+    assert_eq!(q1.shuffled_indices(), q2.shuffled_indices());
+}
+
+#[test]
+fn test_different_seed_produces_different_shuffle_order() {
+    let mut q1 = Queue::with_seed(1);
+    let mut q2 = Queue::with_seed(2);
+    for i in 0..20 {
+        add_track(&mut q1, &format!("/music/t{}.mp3", i));
+        add_track(&mut q2, &format!("/music/t{}.mp3", i));
+    }
+    q1.set_shuffle(ShuffleMode::All);
+    q2.set_shuffle(ShuffleMode::All);
+    // 20! permutations — identical order for two seeds is practically impossible.
+    assert_ne!(q1.shuffled_indices(), q2.shuffled_indices());
+}
+
 // ---------- Track struct ----------
 
 #[test]