fix: keep the streaming OAuth callback alive through browser noise

Cargo.toml

@@ -226,16 +226,19 @@ pkg-fmt = "zip"
 # Native streaming fixes: librespot 0.8.0 + backport of upstream PR #1722 (fall back to the
 # next CDN URL when one returns a non-206 status, e.g. HTTP 530) + port of upstream PR #1692
 # (dealer reconnects handled in place without restarting spirc; prompt spirc-task exit with
-# saved playback state on session TCP loss instead of hanging, enabling seamless recovery).
+# saved playback state on session TCP loss instead of hanging, enabling seamless recovery)
+# + cherry-pick of upstream PR #1706 and follow-up hardening of the OAuth callback listener,
+# which used to accept one connection and give up if it was not the redirect, closing port
+# 8989 before the real callback arrived (issue #414, upstream issue #1705).
 # Pinned to our maintained fork (spotatui-librespot) until a fixed librespot is published to
 # crates.io. Pinned by immutable `rev` (not a branch) so release builds stay reproducible even
 # as the `spotatui` branch advances. Note: crates.io publishes ignore [patch], so `cargo install`
 # users need the upstream release.
 [patch.crates-io]
-librespot-core     = { git = "https://github.com/LargeModGames/spotatui-librespot", rev = "4e9e5c72f079ffe79743ba5245b894c0ab20ce5a" }
-librespot-audio    = { git = "https://github.com/LargeModGames/spotatui-librespot", rev = "4e9e5c72f079ffe79743ba5245b894c0ab20ce5a" }
-librespot-playback = { git = "https://github.com/LargeModGames/spotatui-librespot", rev = "4e9e5c72f079ffe79743ba5245b894c0ab20ce5a" }
-librespot-connect  = { git = "https://github.com/LargeModGames/spotatui-librespot", rev = "4e9e5c72f079ffe79743ba5245b894c0ab20ce5a" }
-librespot-oauth    = { git = "https://github.com/LargeModGames/spotatui-librespot", rev = "4e9e5c72f079ffe79743ba5245b894c0ab20ce5a" }
-librespot-metadata = { git = "https://github.com/LargeModGames/spotatui-librespot", rev = "4e9e5c72f079ffe79743ba5245b894c0ab20ce5a" }
-librespot-protocol = { git = "https://github.com/LargeModGames/spotatui-librespot", rev = "4e9e5c72f079ffe79743ba5245b894c0ab20ce5a" }
+librespot-core     = { git = "https://github.com/LargeModGames/spotatui-librespot", rev = "514061e4df71f5827105611eab1b2b92a87c430a" }
+librespot-audio    = { git = "https://github.com/LargeModGames/spotatui-librespot", rev = "514061e4df71f5827105611eab1b2b92a87c430a" }
+librespot-playback = { git = "https://github.com/LargeModGames/spotatui-librespot", rev = "514061e4df71f5827105611eab1b2b92a87c430a" }
+librespot-connect  = { git = "https://github.com/LargeModGames/spotatui-librespot", rev = "514061e4df71f5827105611eab1b2b92a87c430a" }
+librespot-oauth    = { git = "https://github.com/LargeModGames/spotatui-librespot", rev = "514061e4df71f5827105611eab1b2b92a87c430a" }
+librespot-metadata = { git = "https://github.com/LargeModGames/spotatui-librespot", rev = "514061e4df71f5827105611eab1b2b92a87c430a" }
+librespot-protocol = { git = "https://github.com/LargeModGames/spotatui-librespot", rev = "514061e4df71f5827105611eab1b2b92a87c430a" }

docs/configuration.md

@@ -15,7 +15,7 @@ Machine-managed runtime state lives separately in `$XDG_STATE_HOME/spotatui/stat
 
 ## Safe by default
 
-A typo in `config.yml` never prevents the app from starting. Structural mistakes — an unknown sort field, a bad template placeholder, an invalid column id, an icon that is too wide — are logged as warnings and the affected value falls back to its built-in default. Warnings go to the log file whose path is printed at startup (`/tmp/spotatui_logs/spotatuilog<pid>`).
+A typo in `config.yml` never prevents the app from starting. Structural mistakes (an unknown sort field, a bad template placeholder, an invalid column id, an icon that is too wide) are logged as warnings and the affected value falls back to its built-in default. Warnings go to the log file whose path is printed at startup, a `spotatui_logs/spotatuilog<pid>` file inside your system temp directory (`%TEMP%` on Windows, `$TMPDIR` where set, otherwise `/tmp`). The startup line reports the resolved path, so copy it from there rather than guessing.
 
 Only two kinds of errors are fatal: YAML syntax errors (the file cannot be parsed at all) and a handful of out-of-range numeric values that bypass the warn-and-fallback policy: `volume_increment` outside 0–100, a tick rate (or animation tick rate) outside 1–999ms, an unparseable `auto_update_delay`, `playback_poll_seconds` below 1, and `like_animation_frames` below 1.
 

flake.nix

@@ -40,7 +40,9 @@
             pkgs.apple-sdk
             pkgs.portaudio
           ];
-        librespotOutputHash = "sha256-F1pAhVgxYsbmLXOcbOEU6mGYZO1n94EWCXn5yjhPxv0=";
+        # Update alongside the [patch.crates-io] rev in Cargo.toml; the Nix build
+        # fails with a hash mismatch that reports the correct value otherwise.
+        librespotOutputHash = "sha256-N/ImWrtEyhKyjvZd8zVCelKtsAV1kHoFHMwCoe5ddI0=";
       in
       {
         # Build dependencies for rust

src/core/app.rs

@@ -1884,6 +1884,9 @@ pub struct App {
   pub plugin_popup: Option<crate::core::plugin_api::PluginPopup>,
   /// Scroll offset for the plugin popup.
   pub plugin_popup_scroll: u16,
+  /// Where this run's log file is being written, resolved once here so draw
+  /// code can show it without doing the environment lookup every frame.
+  pub log_path: String,
 }
 
 #[derive(Clone, Copy, PartialEq, Debug)]
@@ -2176,6 +2179,7 @@ impl Default for App {
       plugin_playbar_segments: std::collections::BTreeMap::new(),
       plugin_popup: None,
       plugin_popup_scroll: 0,
+      log_path: crate::core::paths::app_log_path().display().to_string(),
     }
   }
 }

src/core/paths.rs

@@ -51,6 +51,25 @@ pub(crate) fn app_state_dir() -> Option<PathBuf> {
   )
 }
 
+/// Directory holding this run's log file.
+///
+/// The OS temp directory rather than the state dir: a log file is written per
+/// process id, so they accumulate, and temp is the one location the platform
+/// clears on its own. Resolved through `std::env::temp_dir` so Windows lands in
+/// `%TEMP%` instead of the literal `/tmp` this used to hard-code, which is
+/// drive-relative there and left Windows users looking for a path the app
+/// reported but their shell could not find.
+pub(crate) fn app_log_dir() -> PathBuf {
+  std::env::temp_dir().join("spotatui_logs")
+}
+
+/// Path of this process's log file. Callers on both the writing side
+/// (`setup_logging`) and the reporting side (the help screen) use this so the
+/// path shown to a user is always the path actually written.
+pub(crate) fn app_log_path() -> PathBuf {
+  app_log_dir().join(format!("spotatuilog{}", std::process::id()))
+}
+
 /// Ensure a directory that stores credentials or other private app data exists
 /// and is owner-only where supported.
 pub(crate) fn ensure_private_dir(dir: &Path) -> Result<()> {

src/infra/player/streaming.rs

@@ -520,26 +520,27 @@ const SPOTIFY_PLAYER_CLIENT_ID: &str = "65b708073fc0480ea92a077233ca87bd";
 /// spotify-player's redirect_uri - must match what's registered with their client_id
 const SPOTIFY_PLAYER_REDIRECT_URI: &str = "http://127.0.0.1:8989/login";
 
-fn wait_for_oauth_callback_port(
-  address: &str,
-  max_wait: Duration,
-  retry_delay: Duration,
-) -> Result<()> {
+/// Wait until `address` looks bindable, so the streaming consent is not opened
+/// while the Web API callback server still holds the port.
+///
+/// Advisory only, and deliberately so. librespot binds the port itself, so the
+/// most this can do is probe and release; between that release and librespot's
+/// own bind the port is unowned, which no amount of retrying can close. Returns
+/// whether the port came free rather than `Result`, because the caller must not
+/// treat "still busy" as fatal: librespot's bind is the authority and reports a
+/// far better error than this probe can.
+fn wait_for_oauth_callback_port(address: &str, max_wait: Duration, retry_delay: Duration) -> bool {
   let deadline = Instant::now() + max_wait;
   loop {
     match std::net::TcpListener::bind(address) {
       Ok(listener) => {
         drop(listener);
-        return Ok(());
+        return true;
       }
       Err(_) if Instant::now() < deadline => {
         std::thread::sleep(retry_delay.min(deadline.saturating_duration_since(Instant::now())));
       }
-      Err(error) => {
-        return Err(anyhow!(
-          "OAuth callback port {address} did not become available: {error}"
-        ));
-      }
+      Err(_) => return false,
     }
   }
 }
@@ -548,11 +549,15 @@ fn request_streaming_oauth_credentials() -> Result<Credentials> {
   // The Web API and streaming OAuth clients both use port 8989. On a fresh
   // profile their callback servers run back-to-back, so wait for the first
   // listener to be fully released before librespot opens the second consent.
-  wait_for_oauth_callback_port(
+  // Advisory: if it is still busy we go ahead anyway and let librespot's own
+  // bind produce the real error, rather than refusing to try at all.
+  if !wait_for_oauth_callback_port(
     "127.0.0.1:8989",
     Duration::from_secs(5),
     Duration::from_millis(50),
-  )?;
+  ) {
+    warn!("OAuth callback port 127.0.0.1:8989 still busy; attempting the streaming login anyway");
+  }
 
   let client_builder = OAuthClientBuilder::new(
     SPOTIFY_PLAYER_CLIENT_ID,
@@ -1412,13 +1417,34 @@ mod tests {
       drop(listener);
     });
 
-    wait_for_oauth_callback_port(&address, Duration::from_secs(1), Duration::from_millis(10))
-      .expect("callback port should become available after the first listener exits");
+    assert!(
+      wait_for_oauth_callback_port(&address, Duration::from_secs(1), Duration::from_millis(10)),
+      "callback port should become available after the first listener exits"
+    );
     release.join().unwrap();
 
     std::net::TcpListener::bind(address).expect("callback port should remain available");
   }
 
+  /// A port that never frees is reported, not fatal: the caller logs and lets
+  /// librespot's own bind produce the authoritative error.
+  #[test]
+  fn oauth_callback_port_reports_a_port_that_stays_busy() {
+    let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap();
+    let address = listener.local_addr().unwrap().to_string();
+
+    assert!(
+      !wait_for_oauth_callback_port(
+        &address,
+        Duration::from_millis(50),
+        Duration::from_millis(10)
+      ),
+      "a port held for the whole wait should report as unavailable"
+    );
+
+    drop(listener);
+  }
+
   #[test]
   fn auth_failure_with_cached_creds_triggers_retry() {
     assert!(should_retry_with_fresh_credentials(true, true, false));