fix: keep the streaming OAuth callback alive through browser noise (#423)

# Summary

Fixes #414. The streaming login hands port 8989 to librespot, whose
callback server accepted exactly one connection and gave up if it was
not the redirect, dropping the listener and closing the port. Some
browsers, LibreWolf in particular, send a bare CRLF or open a connection
without writing to it before the real callback arrives. librespot
consumed that, failed to parse it, and closed the port, so the redirect
carrying the code hit a dead port. The user is left on a browser "unable
to connect" page, and because the flow errors before `save_credentials`
it repeats on every launch, which is the "streaming cookie does not get
cached" the reporter described.

Not Windows specific. Three independent reports line up: upstream
[librespot#1705](librespot-org/librespot#1705)
(LibreWolf + ncspot on Fedora, with a debug trace showing the empty line
and vanilla Firefox working as a control), #234 (LibreWolf on Gentoo,
same "web API login works, streaming fails" split), and #364 (a
commenter with `AuthCodeListenerParse` in their log who confirmed Chrome
works). The common variable is the browser.

spotatui's own callback server never had this problem:
`extract_callback_url` runs `split_whitespace()` over the whole buffer,
which steps over a leading CRLF, and it answers unrelated requests with
a 400 and keeps waiting. `read_line` stops at the first `\n`. Only the
librespot half was fragile, which is why the web API login succeeded and
the streaming one did not.

Fixed in our librespot fork rather than here, since that is where the
defective listener lives. The fork cherry-picks upstream PR
[#1706](librespot-org/librespot#1706) with its
original attribution, then hardens it: #1706 skips to the next
*connection* on a blank line, which is correct only if the blank line
arrives on its own connection and hangs if it is a leading CRLF on the
same one. This PR bumps the `[patch.crates-io]` rev to pick that up.

Three unrelated startup bugs found while tracing this are fixed in their
own commits:

- **Auto-update deadlock.** `run_auto_update` ran concurrently with
authentication in a `tokio::join!` and re-exec'd immediately on a
successful install. The re-exec blocks the task in `Command::status()`,
so the joined authentication future stops being polled while it still
owns the callback port, and the child, which repeats startup from
scratch, cannot bind that port. The parent waits on the child, the child
waits on a port the parent will never release. The check still runs
concurrently; only the restart moves to after the join.
- **Log path.** `setup_logging` hard-coded `/tmp/spotatui_logs/`, which
on Windows is drive-relative, so the app printed a location the user's
shell could not resolve directly above the line inviting them to report
bugs. Now resolved through `std::env::temp_dir`.
- **OAuth port probe.** `wait_for_oauth_callback_port` refused to start
the login when its probe timed out, so we never even attempted and
reported something vaguer than the bind error librespot would have
produced. It now warns and proceeds.

# Testing

- `cargo fmt --all` (and `--check`, clean)
- `cargo clippy --no-default-features --features telemetry -- -D
warnings` (clean)
- `cargo clippy -- -D warnings` (clean, default features)
- `cargo test --no-default-features --features telemetry` (544 passed)
- `cargo test` (809 passed)

In the fork, `cargo test -p librespot-oauth` (14 passed) and `cargo
clippy -p librespot-oauth --all-targets -- -D warnings` (clean). The
listener tests cover both shapes of librespot#1705 (blank line on the
same connection, and on a separate one), a `/favicon.ico` request before
the callback, a code surviving a browser that hangs up before the
success page is written, and a peer dribbling bytes being cut off at the
deadline. They run behind a watchdog so a regression fails the suite
instead of hanging it.

Reviewed across five rounds with the Codex CLI, which caught four real
defects: a captured authorization code being discarded when the success
page failed to write, the accept loop having no overall deadline, HTTP
header lines being read as if each were a request, and the deadline not
covering an in-flight read.

# Additional notes

Not verified against a live LibreWolf on Windows. I am on Linux and
cannot reproduce the reporter's setup, so this rests on code reading
plus the three corroborating reports above rather than an observed
repro.

A fork fix reaches everyone who installs today: GitHub releases, winget,
Homebrew, both AUR packages, and `install.sh`/`install.ps1` all build
with `[patch]` active. The one gap is `cargo install spotatui` from
crates.io, which is stuck at 0.40.2 because the v0.40.3 publish job
fails to compile against upstream librespot (`unresolved import
librespot_connect::SavedPlaybackState`). Worth fixing separately; those
users cannot get 0.40.3+ by any route today.

Workaround for anyone hitting this before the next release: set
`enable_streaming: false` in `client.yml`. That stops the every-launch
browser flow and leaves Spotify Connect working.


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Log files now use a platform-appropriate temporary directory and
include the process ID.
  - The changelog displays the actual log-file location.
- Automatic updates restart after authentication resources are released.

- **Bug Fixes**
- Improved OAuth callback handling when the callback port is
unavailable.
  - Streaming authentication errors now include more actionable details.
  - Updated playback and connection reliability fixes.

- **Documentation**
- Updated safe-by-default guidance to explain how to find the log-file
location.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Author: LargeModGames

Cargo.lock

@@ -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" }

Cargo.toml

@@ -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.
 

docs/configuration.md

@@ -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

flake.nix

@@ -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/app.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/core/paths.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));