fix: start the resume point when play comes from mpd or the os media controls

docs/architecture/invariants.md

@@ -66,6 +66,10 @@ That sequence was copy-pasted in `lib.rs` and `media_controls.rs` until #471 (th
 
 They're `async` and await rather than spawn: sync callers (souvlaki, tray, taskbar buttons) wrap in `tauri::async_runtime::spawn` themselves, async ones report the outcome back to their client. `player_actions::toggle_play_pause`, shared by the tray and the taskbar buttons, is sync: it pauses or resumes directly, and only spawns `player_actions::resume_last` from `Idle` / `Ended`. From those states the decoder has no track open and drops `AudioCmd::Resume`, so resuming means loading the persisted resume point — what the in-app Play button does through `player_resume_last`.
 
+`player_actions::play` is the same shape for surfaces with a _separate_ Play button (the OS media overlay, MPD's `play` / `pause 0`): it resumes or loads the resume point and never pauses, so Play on a playing track does nothing instead of pausing it. Sending `AudioCmd::Resume` straight to the engine from those surfaces is what #609 was.
+
+Both funnel into `resume_last`, which holds a one-at-a-time guard (`AudioEngine::begin_resume`, released on every exit path). Two Play events landing together would otherwise each read `Idle`, each await the database, and the second would restart the track the first had just started.
+
 ---
 
 ## Database

docs/features/mpd.md

@@ -38,6 +38,8 @@ For contrast: a player whose audio lives in the webview (an `<audio>` element) h
 
 `next` / `previous` / `play <pos>` go through [`player_actions`](../../src-tauri/crates/app/src/player_actions.rs), shared with the tray menu and the OS media controls. That sequence (advance the queue → `emit_track_changed` → `emit_queue_changed` → hand the track to the decoder) used to be copy-pasted in `lib.rs` and `media_controls.rs`; MPD would have made it a third copy, each free to forget an emit and desync a surface. Any new non-frontend control surface should call into that module rather than re-deriving it.
 
+`play` / `playid` with no argument, `pause 0` and a bare `pause` go through it too. They used to send `AudioCmd::Resume` to the engine, which the decoder drops when no track is open, so `mpc play` did nothing after a launch or at the end of the queue (#609).
+
 ## Configuration
 
 Persisted in the global `app_setting` table — the listener is process-wide, not per-profile.

docs/features/playback.md

@@ -125,6 +125,10 @@ Every failure path that ends with no output thread at all publishes `exclusive_o
 
 Initialised after the main window exists (needs an HWND on Windows). State transitions are driven through `transition_state()` so the OS overlay flips at the same instant as the in-app controls; the brief `Loading` state is skipped to avoid a 50 ms "controls flash off" between tracks.
 
+Play and Toggle from the overlay go through [`player_actions`](../../src-tauri/crates/app/src/player_actions.rs) rather than sending `AudioCmd::Resume` to the engine. The decoder only handles `Resume` inside its pause loop, so with nothing open it was dropped and Play did nothing at all — after a launch, and at the end of the queue (#609). `player_actions::play` resumes or loads the persisted resume point and never pauses; Toggle follows the tray's rule.
+
+The overlay also learns about the restored track **at launch**: `player_get_state` publishes it paused, at its persisted position, starting no audio — and only while the engine holds nothing, since once a track is loaded the decoder's own transitions own the overlay, and that command runs again on profile switch and re-hydration. Before that the only caller of `update_metadata` outside live radio was `emit_track_changed`, on an actual track start, so there was no session for Play to appear on (#609). What the `PlatformConfig` souvlaki is given here cannot express is advertising Previous / Next only when they would do something.
+
 The same `transition_state()` hook also feeds [`discord_presence.rs`](../../src-tauri/crates/app/src/discord_presence.rs) so the user's Discord profile mirrors the playing/paused state. Documented separately under [Integrations → Discord Rich Presence](integrations.md#discord-rich-presence).
 
 ## Playback speed (0.5× – 2×)

src-tauri/crates/app/src/audio/engine.rs

@@ -357,6 +357,12 @@ pub struct AudioEngine {
     /// flap would otherwise queue two concurrent rebuilds that
     /// each interrupt the same track.
     rebuild_in_progress: std::sync::atomic::AtomicBool,
+    /// One resume at a time (#609). Two Play events landing together — a
+    /// double tap on the OS overlay, a client sending `play` twice — both
+    /// read `Idle` and both spawn `player_actions::resume_last`, which
+    /// awaits the database before sending its `LoadAndPlay`. The second
+    /// would restart the track the first just started.
+    resume_in_flight: std::sync::atomic::AtomicBool,
     /// Session-only kill switch for exclusive output after a flap storm
     /// (#322). Once tripped, every rebuild / hot-swap stays on cpal
     /// shared regardless of the `exclusive_output` preference, so a
@@ -553,6 +559,7 @@ impl AudioEngine {
             exclusive_output: std::sync::atomic::AtomicBool::new(exclusive_output),
             exclusive_output_active: std::sync::atomic::AtomicBool::new(exclusive_output_active),
             rebuild_in_progress: std::sync::atomic::AtomicBool::new(false),
+            resume_in_flight: std::sync::atomic::AtomicBool::new(false),
             exclusive_suppressed: std::sync::atomic::AtomicBool::new(false),
             exclusive_flaps: Mutex::new(FlapWindow::default()),
             rebuild_gate: Mutex::new(RebuildGate::default()),
@@ -588,6 +595,22 @@ impl AudioEngine {
         self.radio_resume.lock().ok().and_then(|g| g.clone())
     }
 
+    /// Claim the right to run one resume, or `None` when another is
+    /// already in flight (#609).
+    ///
+    /// The guard clears the slot on every exit path, including the `?`
+    /// returns inside [`crate::player_actions::resume_last`] and a panic.
+    /// A leaked slot would leave Play dead for the rest of the session,
+    /// which is worse than the double load it prevents.
+    pub fn begin_resume(&self) -> Option<ResumeGuard<'_>> {
+        use std::sync::atomic::Ordering;
+        if self.resume_in_flight.swap(true, Ordering::AcqRel) {
+            None
+        } else {
+            Some(ResumeGuard(&self.resume_in_flight))
+        }
+    }
+
     /// Borrow the shared atomic state — used by commands that need to read
     /// current position / volume / state without hitting the decoder.
     pub fn shared(&self) -> &Arc<SharedPlayback> {
@@ -1692,6 +1715,15 @@ impl AudioEngine {
     }
 }
 
+/// Releases the slot [`AudioEngine::begin_resume`] took, when dropped.
+pub struct ResumeGuard<'a>(&'a std::sync::atomic::AtomicBool);
+
+impl Drop for ResumeGuard<'_> {
+    fn drop(&mut self) {
+        self.0.store(false, std::sync::atomic::Ordering::Release);
+    }
+}
+
 /// Whether the old output has to be released *before* the new one is
 /// opened, rather than the other way round. `old_is_exclusive` is `None`
 /// when there is no stream installed at all.
@@ -1892,6 +1924,29 @@ mod reopen_order_tests {
     }
 }
 
+#[cfg(test)]
+mod resume_guard_tests {
+    use std::sync::atomic::{AtomicBool, Ordering};
+
+    use super::ResumeGuard;
+
+    #[test]
+    fn the_slot_is_taken_once_and_released_on_drop() {
+        let slot = AtomicBool::new(false);
+        // The first Play claims it.
+        assert!(!slot.swap(true, Ordering::AcqRel));
+        {
+            let _guard = ResumeGuard(&slot);
+            // A second Play landing while the first resume is still
+            // awaiting the database finds the slot taken and backs off.
+            assert!(slot.swap(true, Ordering::AcqRel));
+        }
+        // Released on drop, including the `?` paths inside `resume_last`:
+        // a leaked slot would leave Play dead for the whole session.
+        assert!(!slot.load(Ordering::Acquire));
+    }
+}
+
 #[cfg(test)]
 mod rebuild_resume_tests {
     use super::super::state::PlayerState;

src-tauri/crates/app/src/commands/player.rs

@@ -464,6 +464,7 @@ pub(crate) async fn emit_options_changed(app: &AppHandle, pool: &sqlx::SqlitePoo
 /// state without auto-playing.
 #[tauri::command]
 pub async fn player_get_state(
+    app: AppHandle,
     state: tauri::State<'_, AppState>,
     engine: tauri::State<'_, Arc<AudioEngine>>,
 ) -> AppResult<PlayerStateSnapshot> {
@@ -762,6 +763,41 @@ pub async fn player_get_state(
     if snapshot.state == "idle" && snapshot.position_ms == 0 {
         snapshot.position_ms = resumed_position;
     }
+
+    // Hand that restored track to the OS media overlay (#609). Until now
+    // the overlay showed no WaveFlow session at all before something
+    // played, so its Play button — which starts the resume point since
+    // this release — had nothing to appear on after a launch.
+    //
+    // Only while the engine holds nothing: once a track is loaded the
+    // decoder's own transitions own the overlay, and this command also
+    // runs on profile switch and re-hydration. Paused, at the persisted
+    // position, and nothing is started — the session describes exactly
+    // what Play would resume.
+    if snapshot.state == "idle" {
+        if let (Some(track), Some(controls)) = (
+            snapshot.current_track.as_ref(),
+            app.try_state::<crate::media_controls::MediaControlsHandle>(),
+        ) {
+            // Re-read the engine immediately before publishing. This
+            // command has just spent its time in the database, and a
+            // surface outside the window — a media key, MPD — can have
+            // started playback since the snapshot was taken. Laying a
+            // stale "restored track, paused" over a session that is
+            // already playing is worse than publishing nothing at all.
+            if engine.shared().state() == crate::audio::PlayerState::Idle {
+                controls.update_metadata(
+                    track.title.clone(),
+                    track.artist_name.clone(),
+                    track.album_title.clone(),
+                    track.artwork_path.clone(),
+                    track.duration_ms,
+                );
+                controls.update_playback(crate::audio::PlayerState::Paused, snapshot.position_ms);
+            }
+        }
+    }
+
     Ok(snapshot)
 }
 

src-tauri/crates/app/src/media_controls.rs

@@ -358,21 +358,18 @@ fn push_metadata(controls: &mut MediaControls, cached: &CachedMetadata) {
 /// profile DB pool is dispatched onto Tauri's tokio runtime.
 fn handle_event(event: MediaControlEvent, app: AppHandle) {
     match event {
-        MediaControlEvent::Play => {
-            let engine = app.state::<Arc<AudioEngine>>();
-            let _ = engine.send(AudioCmd::Resume);
-        }
+        // Both go through `player_actions`: a bare `AudioCmd::Resume` is
+        // dropped by the decoder when no track is open, which left Play
+        // dead on the overlay after a launch and at the end of the queue
+        // (#609). `play` resumes or loads the resume point and never
+        // pauses; `toggle_play_pause` is the rule the tray follows.
+        MediaControlEvent::Play => crate::player_actions::play(&app, "media_controls"),
         MediaControlEvent::Pause => {
             let engine = app.state::<Arc<AudioEngine>>();
             let _ = engine.send(AudioCmd::Pause);
         }
         MediaControlEvent::Toggle => {
-            let engine = app.state::<Arc<AudioEngine>>();
-            let cmd = match engine.shared().state() {
-                PlayerState::Playing => AudioCmd::Pause,
-                _ => AudioCmd::Resume,
-            };
-            let _ = engine.send(cmd);
+            crate::player_actions::toggle_play_pause(&app, "media_controls")
         }
         MediaControlEvent::Stop => {
             let engine = app.state::<Arc<AudioEngine>>();

src-tauri/crates/app/src/mpd/commands.rs

@@ -458,8 +458,18 @@ pub async fn dispatch(ctx: &Ctx, session: &mut Session, cmd: Command) -> Result<
 
         Command::Play(pos) => {
             match pos {
+                // Bare `play` resumes. Through `player_actions` because a
+                // bare `AudioCmd::Resume` is dropped when no track is
+                // open, so `mpc play` did nothing after a launch (#609).
                 None => {
-                    let _ = ctx.engine().send(AudioCmd::Resume);
+                    // Awaited, not spawned: MPD answers its client once the
+                    // work is done, so `status` and the `idle` notification
+                    // below describe the load that actually happened. A
+                    // failure is logged rather than ACKed — real MPD answers
+                    // OK to a bare `play` with nothing to play.
+                    if let Err(err) = player_actions::play_and_wait(&ctx.app).await {
+                        tracing::warn!(%err, surface = SURFACE, "mpd play: resume failed");
+                    }
                 }
                 Some(p) => {
                     // A position past the end is an argument error in MPD, not
@@ -487,8 +497,16 @@ pub async fn dispatch(ctx: &Ctx, session: &mut Session, cmd: Command) -> Result<
 
         Command::PlayId(id) => {
             match id {
+                // Bare `playid` resumes, same as bare `play` (#609).
                 None => {
-                    let _ = ctx.engine().send(AudioCmd::Resume);
+                    // Awaited, not spawned: MPD answers its client once the
+                    // work is done, so `status` and the `idle` notification
+                    // below describe the load that actually happened. A
+                    // failure is logged rather than ACKed — real MPD answers
+                    // OK to a bare `play` with nothing to play.
+                    if let Err(err) = player_actions::play_and_wait(&ctx.app).await {
+                        tracing::warn!(%err, surface = SURFACE, "mpd play: resume failed");
+                    }
                 }
                 Some(id) => {
                     // One snapshot for the id→position lookup AND the jump, so
@@ -515,18 +533,26 @@ pub async fn dispatch(ctx: &Ctx, session: &mut Session, cmd: Command) -> Result<
         }
 
         Command::Pause(want) => {
-            let engine = ctx.engine();
-            let cmd = match want {
-                Some(true) => AudioCmd::Pause,
-                Some(false) => AudioCmd::Resume,
-                // Bare `pause` toggles, which is what a remote's single
-                // play/pause button sends.
-                None => match engine.shared().state() {
-                    PlayerState::Playing => AudioCmd::Pause,
-                    _ => AudioCmd::Resume,
-                },
-            };
-            let _ = engine.send(cmd);
+            match want {
+                Some(true) => {
+                    let _ = ctx.engine().send(AudioCmd::Pause);
+                }
+                // `pause 0` is an explicit resume; a bare `pause` toggles,
+                // which is what a remote's single play/pause button sends.
+                // Both go through `player_actions` so they start the resume
+                // point when the decoder has nothing open (#609).
+                Some(false) => {
+                    // Awaited, not spawned: MPD answers its client once the
+                    // work is done, so `status` and the `idle` notification
+                    // below describe the load that actually happened. A
+                    // failure is logged rather than ACKed — real MPD answers
+                    // OK to a bare `play` with nothing to play.
+                    if let Err(err) = player_actions::play_and_wait(&ctx.app).await {
+                        tracing::warn!(%err, surface = SURFACE, "mpd play: resume failed");
+                    }
+                }
+                None => player_actions::toggle_play_pause(&ctx.app, SURFACE),
+            }
             ctx.idle.notify(Subsystem::Player);
             Ok(Response::new())
         }