Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ units under `src/`:
| `infra/` | Spotify Web API (`network/`), native librespot streaming (`player/`), alternative sources (`local/`, `subsonic/`, `qobuz/`, `radio/`, `youtube/`, `queue/`), audio viz (`audio/`), Lua scripting (`scripting/`), AI DJ + MCP (`dj/`, `mcp/`), OS integrations (Discord RPC, MPRIS, macOS/Windows media) |
| `tui/` | Terminal UI: the event/render loop (`runner.rs`), key plumbing (`event/`), per-block input handlers (`handlers/`), immutable draw fns (`ui/`) |
| `cli/` | clap subcommands: playback control, listening history, self-update, MCP relay, plugin management |
| `runtime/` | `mod.rs::run_cli` (entry point + CLI dispatch), `bootstrap.rs::boot` (frontend-neutral config/auth/`App` construction, `run_cli` its sole caller), `cli.rs` (clap assembly + self-update), `pump.rs::start_tokio` (the IoEvent pump), `streaming/` (native-streaming startup every frontend shares: the pure saved-device decision in `mod.rs`, the librespot bring-up in `launch.rs`, gated on `streaming`), `startup.rs` (the UI-launch half, gated on `tui`) |
| `runtime/` | `mod.rs::run_cli` (entry point + CLI dispatch), `bootstrap.rs::boot` (frontend-neutral config/auth/`App` construction, `run_cli` its sole caller, plus the boot auth rule `spotify_auth_mode`: interactive only right after the client wizard or `--reconfigure-auth`, a subcommand needs a cached token, a UI launch tolerates no session), `cli.rs` (clap assembly + self-update), `pump.rs::start_tokio` (the IoEvent pump), `streaming/` (native-streaming startup every frontend shares: the pure saved-device decision in `mod.rs`, the librespot bring-up in `launch.rs`, gated on `streaming`), `startup.rs` (the UI-launch half, gated on `tui`) |

### Data flow

Expand Down Expand Up @@ -124,6 +124,11 @@ worth knowing before adding an event:
(`qobuz:`) → `route_radio_event` (`radio:`) → `route_youtube_event`
(`youtube:`) → `Network::handle_network_event`.
This is what keeps `infra/network/` Spotify-only.
- **Claim gate**: before the routers, `start_playback_has_taker` drops a
`StartPlayback` whose URI scheme (`core::queue::queue_item_source`) names no
compiled-in source and that no Spotify session can take. The routers'
foreign-start teardown arms therefore only run for a real source-to-source
handoff.
- **Service lane**: `Network::runs_on_service_lane` lists events that run on a
detached task so slow, source-agnostic work cannot head-of-line-block the serial
pump. The service lane's `Network` is built with **no Spotify client** - adding a
Expand Down Expand Up @@ -187,7 +192,11 @@ fixtures are `pub(super) fn`s in `test_support.rs`, imported as
Multiple players share one UI, and the predicate order is the #1 source of
regressions. Check in this order: `queue_owns_playback()` /
`queue_now_is_spotify()`, then `active_decoded_source()`, then
`is_native_streaming_active_for_playback()`.
`is_native_streaming_active_for_playback()`. `App::playback_owner()` folds them
into one `PlaybackOwner`, and the transport chains (play/pause,
next, previous, shuffle, repeat, volume) end on `dispatch_spotify_fallback`,
which answers "Nothing is playing" instead of a Spotify dispatch when no
session exists.

- Starting a decoded source (Local/Subsonic/Qobuz/Radio/YouTube) only **pauses**
librespot - the native flag stays true, so driving librespot directly resumes
Expand Down
13 changes: 11 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ units under `src/`:
| `infra/` | Spotify Web API (`network/`), native librespot streaming (`player/`), alternative sources (`local/`, `subsonic/`, `qobuz/`, `radio/`, `youtube/`, `queue/`), audio viz (`audio/`), Lua scripting (`scripting/`), AI DJ + MCP (`dj/`, `mcp/`), OS integrations (Discord RPC, MPRIS, macOS/Windows media) |
| `tui/` | Terminal UI: the event/render loop (`runner.rs`), key plumbing (`event/`), per-block input handlers (`handlers/`), immutable draw fns (`ui/`) |
| `cli/` | clap subcommands: playback control, listening history, self-update, MCP relay, plugin management |
| `runtime/` | `mod.rs::run_cli` (entry point + CLI dispatch), `bootstrap.rs::boot` (frontend-neutral config/auth/`App` construction, `run_cli` its sole caller), `cli.rs` (clap assembly + self-update), `pump.rs::start_tokio` (the IoEvent pump), `streaming/` (native-streaming startup every frontend shares: the pure saved-device decision in `mod.rs`, the librespot bring-up in `launch.rs`, gated on `streaming`), `startup.rs` (the UI-launch half, gated on `tui`) |
| `runtime/` | `mod.rs::run_cli` (entry point + CLI dispatch), `bootstrap.rs::boot` (frontend-neutral config/auth/`App` construction, `run_cli` its sole caller, plus the boot auth rule `spotify_auth_mode`: interactive only right after the client wizard or `--reconfigure-auth`, a subcommand needs a cached token, a UI launch tolerates no session), `cli.rs` (clap assembly + self-update), `pump.rs::start_tokio` (the IoEvent pump), `streaming/` (native-streaming startup every frontend shares: the pure saved-device decision in `mod.rs`, the librespot bring-up in `launch.rs`, gated on `streaming`), `startup.rs` (the UI-launch half, gated on `tui`) |

### Data flow

Expand Down Expand Up @@ -126,6 +126,11 @@ worth knowing before adding an event:
(`qobuz:`) → `route_radio_event` (`radio:`) → `route_youtube_event`
(`youtube:`) → `Network::handle_network_event`.
This is what keeps `infra/network/` Spotify-only.
- **Claim gate**: before the routers, `start_playback_has_taker` drops a
`StartPlayback` whose URI scheme (`core::queue::queue_item_source`) names no
compiled-in source and that no Spotify session can take. The routers'
foreign-start teardown arms therefore only run for a real source-to-source
handoff.
- **Service lane**: `Network::runs_on_service_lane` lists events that run on a
detached task so slow, source-agnostic work cannot head-of-line-block the serial
pump. The service lane's `Network` is built with **no Spotify client** - adding a
Expand Down Expand Up @@ -189,7 +194,11 @@ fixtures are `pub(super) fn`s in `test_support.rs`, imported as
Multiple players share one UI, and the predicate order is the #1 source of
regressions. Check in this order: `queue_owns_playback()` /
`queue_now_is_spotify()`, then `active_decoded_source()`, then
`is_native_streaming_active_for_playback()`.
`is_native_streaming_active_for_playback()`. `App::playback_owner()` folds them
into one `PlaybackOwner`, and the transport chains (play/pause,
next, previous, shuffle, repeat, volume) end on `dispatch_spotify_fallback`,
which answers "Nothing is playing" instead of a Spotify dispatch when no
session exists.

- Starting a decoded source (Local/Subsonic/Qobuz/Radio/YouTube) only **pauses**
librespot - the native flag stays true, so driving librespot directly resumes
Expand Down
13 changes: 11 additions & 2 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ units under `src/`:
| `infra/` | Spotify Web API (`network/`), native librespot streaming (`player/`), alternative sources (`local/`, `subsonic/`, `qobuz/`, `radio/`, `youtube/`, `queue/`), audio viz (`audio/`), Lua scripting (`scripting/`), AI DJ + MCP (`dj/`, `mcp/`), OS integrations (Discord RPC, MPRIS, macOS/Windows media) |
| `tui/` | Terminal UI: the event/render loop (`runner.rs`), key plumbing (`event/`), per-block input handlers (`handlers/`), immutable draw fns (`ui/`) |
| `cli/` | clap subcommands: playback control, listening history, self-update, MCP relay, plugin management |
| `runtime/` | `mod.rs::run_cli` (entry point + CLI dispatch), `bootstrap.rs::boot` (frontend-neutral config/auth/`App` construction, `run_cli` its sole caller), `cli.rs` (clap assembly + self-update), `pump.rs::start_tokio` (the IoEvent pump), `streaming/` (native-streaming startup every frontend shares: the pure saved-device decision in `mod.rs`, the librespot bring-up in `launch.rs`, gated on `streaming`), `startup.rs` (the UI-launch half, gated on `tui`) |
| `runtime/` | `mod.rs::run_cli` (entry point + CLI dispatch), `bootstrap.rs::boot` (frontend-neutral config/auth/`App` construction, `run_cli` its sole caller, plus the boot auth rule `spotify_auth_mode`: interactive only right after the client wizard or `--reconfigure-auth`, a subcommand needs a cached token, a UI launch tolerates no session), `cli.rs` (clap assembly + self-update), `pump.rs::start_tokio` (the IoEvent pump), `streaming/` (native-streaming startup every frontend shares: the pure saved-device decision in `mod.rs`, the librespot bring-up in `launch.rs`, gated on `streaming`), `startup.rs` (the UI-launch half, gated on `tui`) |

### Data flow

Expand Down Expand Up @@ -126,6 +126,11 @@ worth knowing before adding an event:
(`qobuz:`) → `route_radio_event` (`radio:`) → `route_youtube_event`
(`youtube:`) → `Network::handle_network_event`.
This is what keeps `infra/network/` Spotify-only.
- **Claim gate**: before the routers, `start_playback_has_taker` drops a
`StartPlayback` whose URI scheme (`core::queue::queue_item_source`) names no
compiled-in source and that no Spotify session can take. The routers'
foreign-start teardown arms therefore only run for a real source-to-source
handoff.
- **Service lane**: `Network::runs_on_service_lane` lists events that run on a
detached task so slow, source-agnostic work cannot head-of-line-block the serial
pump. The service lane's `Network` is built with **no Spotify client** - adding a
Expand Down Expand Up @@ -189,7 +194,11 @@ fixtures are `pub(super) fn`s in `test_support.rs`, imported as
Multiple players share one UI, and the predicate order is the #1 source of
regressions. Check in this order: `queue_owns_playback()` /
`queue_now_is_spotify()`, then `active_decoded_source()`, then
`is_native_streaming_active_for_playback()`.
`is_native_streaming_active_for_playback()`. `App::playback_owner()` folds them
into one `PlaybackOwner`, and the transport chains (play/pause,
next, previous, shuffle, repeat, volume) end on `dispatch_spotify_fallback`,
which answers "Nothing is playing" instead of a Spotify dispatch when no
session exists.

- Starting a decoded source (Local/Subsonic/Qobuz/Radio/YouTube) only **pauses**
librespot - the native flag stays true, so driving librespot directly resumes
Expand Down
10 changes: 3 additions & 7 deletions src/core/action/apply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl App {
RepeatSetting::Track => RepeatState::Track,
RepeatSetting::Context => RepeatState::Context,
};
self.dispatch(IoEvent::Repeat(state));
self.dispatch_spotify_fallback(IoEvent::Repeat(state));
}
Action::PlayUris { uris, offset } => self.start_playback_uris(uris, offset),
Action::PlayContext { uri, offset } => self.start_playback_context(uri, offset),
Expand Down Expand Up @@ -226,12 +226,8 @@ impl App {
Action::RecommendFromTrackId { id, name } => {
self.load_recommendations_for_track_id(id, name);
}
Action::StartParty => {
self.dispatch(IoEvent::StartParty(
crate::infra::network::sync::ControlMode::HostOnly,
));
}
Action::JoinParty { code, name } => self.dispatch(IoEvent::JoinParty { code, name }),
Action::StartParty => self.start_party(),
Action::JoinParty { code, name } => self.join_party(code, name),
Action::LeaveParty => self.dispatch(IoEvent::LeaveParty),
Action::TogglePartyControlMode => self.toggle_party_control_mode(),
Action::SetPlaybarSegment { plugin, text } => match text {
Expand Down
107 changes: 106 additions & 1 deletion src/core/action/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use std::sync::mpsc::{channel, Receiver};
use std::time::SystemTime;

use super::{Action, NavTarget, RepeatSetting};
use crate::core::app::{App, RouteId, UserInfo};
use crate::core::app::{App, RouteId, UserInfo, NOTHING_PLAYING_STATUS};
use crate::core::theme::{Color, Theme, ThemeField};
use crate::core::user_config::UserConfig;
use crate::infra::network::IoEvent;
Expand All @@ -22,6 +22,13 @@ fn app_with_channel() -> (App, Receiver<IoEvent>) {
(app, rx)
}

/// The same fixture with no Spotify session (`spotify_connected == false`).
fn session_free_app_with_channel() -> (App, Receiver<IoEvent>) {
let (tx, rx) = channel();
let app = App::new(tx, UserConfig::new(), None);
(app, rx)
}

#[allow(deprecated)]
fn playback_context(
is_playing: bool,
Expand Down Expand Up @@ -266,6 +273,62 @@ fn toggle_shuffle_flips_the_spotify_shuffle_state() {
assert!(matches!(rx.try_recv(), Ok(IoEvent::Shuffle(true))));
}

// --- transport without a Spotify session ---

#[test]
fn next_track_without_a_session_reports_nothing_playing() {
let (mut app, rx) = session_free_app_with_channel();

app.apply(Action::NextTrack);

assert!(rx.try_recv().is_err());
assert_eq!(app.status_message.as_deref(), Some(NOTHING_PLAYING_STATUS));
}

#[test]
fn toggle_playback_without_a_session_reports_nothing_playing() {
let (mut app, rx) = session_free_app_with_channel();

app.apply(Action::TogglePlayback);

assert!(rx.try_recv().is_err());
assert_eq!(app.status_message.as_deref(), Some(NOTHING_PLAYING_STATUS));
}

#[test]
fn volume_down_without_a_session_leaves_no_latch() {
// The fixture starts at 100%, so only a decrease reaches the API fallback.
let (mut app, rx) = session_free_app_with_channel();

app.apply(Action::VolumeDown);

assert!(rx.try_recv().is_err());
assert_eq!(app.status_message.as_deref(), Some(NOTHING_PLAYING_STATUS));
assert!(!app.is_volume_change_in_flight);
assert!(app.pending_volume.is_none());
}

#[test]
fn set_repeat_without_a_session_reports_nothing_playing() {
let (mut app, rx) = session_free_app_with_channel();

app.apply(Action::SetRepeat(RepeatSetting::Track));

assert!(rx.try_recv().is_err());
assert_eq!(app.status_message.as_deref(), Some(NOTHING_PLAYING_STATUS));
}

#[test]
fn flush_pending_volume_without_a_session_clears_the_pending_value() {
let (mut app, rx) = session_free_app_with_channel();
app.pending_volume = Some(40);

app.flush_pending_volume();

assert!(rx.try_recv().is_err());
assert!(app.pending_volume.is_none());
}

// --- jump-to navigation ---

#[test]
Expand Down Expand Up @@ -388,6 +451,18 @@ fn play_uris_carries_the_offset() {
));
}

#[test]
fn play_uris_with_an_empty_list_dispatches_nothing() {
let (mut app, rx) = app_with_channel();

app.apply(Action::PlayUris {
uris: vec![],
offset: None,
});

assert!(rx.try_recv().is_err());
}

#[test]
fn play_context_dispatches_a_context_start() {
let (mut app, rx) = app_with_channel();
Expand Down Expand Up @@ -2130,6 +2205,36 @@ fn select_source_spotify_fetches_no_sidebar() {
assert!(rx.try_recv().is_err(), "expected no IoEvent dispatched");
}

#[test]
fn selecting_spotify_without_a_session_does_not_reach_disk() {
let dir = tempfile::tempdir().unwrap();
let (mut app, _rx) = session_free_app_with_channel();
app.state_path = Some(dir.path().join("state.yml"));

app.apply(Action::SelectSource(Source::Spotify));

assert_eq!(app.active_source, Source::Spotify);
assert!(!dir.path().join("state.yml").exists());

app.spotify_connected = true;
app.persist_active_source();

let written = std::fs::read_to_string(dir.path().join("state.yml")).unwrap();
assert!(written.contains("active_source") && written.contains("Spotify"));
}

#[test]
fn selecting_a_free_source_without_a_session_still_persists() {
let dir = tempfile::tempdir().unwrap();
let (mut app, _rx) = session_free_app_with_channel();
app.state_path = Some(dir.path().join("state.yml"));

app.apply(Action::SelectSource(Source::Local));

let written = std::fs::read_to_string(dir.path().join("state.yml")).unwrap();
assert!(written.contains("active_source") && written.contains("Local"));
}

// --- the now-playing item family ---

use super::CopyTarget;
Expand Down
10 changes: 10 additions & 0 deletions src/core/app/construction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,16 @@ impl Default for App {
}

impl App {
/// `App::default()` with a Spotify session, for tests that need one without
/// an `IoEvent` channel.
#[cfg(all(test, feature = "tui"))]
pub(crate) fn default_connected() -> App {
App {
spotify_connected: true,
..App::default()
}
}

#[cfg(test)]
pub fn new(
io_tx: Sender<IoEvent>,
Expand Down
1 change: 1 addition & 0 deletions src/core/app/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ pub use native_backend::*;
pub use native_recovery::*;
#[cfg(feature = "streaming")]
pub(crate) use native_shuffle::*;
pub use playback_routing::*;
pub use playlist_folders::*;
pub use playlists::*;
pub use plugins::*;
Expand Down
80 changes: 80 additions & 0 deletions src/core/app/party.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,31 @@
use super::*;

const PARTY_NEEDS_SPOTIFY: &str =
"Listening Party needs Spotify. Press `d` and pick Spotify to log in.";

impl App {
/// Host a party; needs a Spotify session, the relay drives Spotify playback.
pub(crate) fn start_party(&mut self) {
if !self.spotify_connected {
self.set_status_message(PARTY_NEEDS_SPOTIFY, 6);
return;
}
self.dispatch(IoEvent::StartParty(ControlMode::HostOnly));
}

/// Join a party; same session requirement as `start_party`.
pub(crate) fn join_party(&mut self, code: String, name: String) {
if !self.spotify_connected {
self.set_status_message(PARTY_NEEDS_SPOTIFY, 6);
return;
}
self.dispatch(IoEvent::JoinParty { code, name });
// The typed code and name are consumed only by a join that went out.
self.view.party_input.clear();
self.view.party_input_idx = 0;
self.view.party_join_name.clear();
}

/// The local write is optimistic on purpose: the relay handler never writes
/// the session back, so the popup's "Control" label renders from it.
pub(crate) fn toggle_party_control_mode(&mut self) {
Expand All @@ -15,3 +40,58 @@ impl App {
self.dispatch(IoEvent::SetPartyControlMode(updated_mode));
}
}

#[cfg(test)]
mod tests {
use super::*;
use crate::core::action::Action;
use crate::core::app::test_support::*;
Comment thread
coderabbitai[bot] marked this conversation as resolved.

#[test]
fn start_party_without_a_session_dispatches_nothing_and_says_why() {
let (mut app, rx) = session_free_app();

app.apply(Action::StartParty);

assert!(rx.try_recv().is_err());
assert_eq!(app.status_message.as_deref(), Some(PARTY_NEEDS_SPOTIFY));
}

#[test]
fn join_party_without_a_session_dispatches_nothing_and_keeps_the_input() {
let (mut app, rx) = session_free_app();
app.view.party_input = "ABC123".chars().collect();
app.view.party_input_idx = 6;
app.view.party_join_name = "Guest".chars().collect();

app.apply(Action::JoinParty {
code: "ABC123".to_string(),
name: "Guest".to_string(),
});

assert!(rx.try_recv().is_err());
assert_eq!(app.status_message.as_deref(), Some(PARTY_NEEDS_SPOTIFY));
assert_eq!(app.view.party_input.iter().collect::<String>(), "ABC123");
assert_eq!(app.view.party_input_idx, 6);
assert_eq!(app.view.party_join_name.iter().collect::<String>(), "Guest");
}

#[test]
fn join_party_with_a_session_dispatches_and_clears_the_input() {
let (tx, rx) = channel();
let mut app = App::new(tx, UserConfig::new(), Some(SystemTime::now()));
app.view.party_input = "ABC123".chars().collect();
app.view.party_input_idx = 6;
app.view.party_join_name = "Guest".chars().collect();

app.apply(Action::JoinParty {
code: "ABC123".to_string(),
name: "Guest".to_string(),
});

assert!(matches!(rx.try_recv(), Ok(IoEvent::JoinParty { .. })));
assert!(app.view.party_input.is_empty());
assert_eq!(app.view.party_input_idx, 0);
assert!(app.view.party_join_name.is_empty());
}
}
Loading
Loading