35 Commits

Author	SHA1	Message	Date
bjarneo	bd8d5d07c0	Fix review findings: concurrency, plugin sandbox, provider bugs, and dedup (#254) ... * fix(playlist): synchronize Playlist for concurrent plugin/UI access Lua plugin callbacks run on goroutines (hooks, timers, keybinds) and read playlist state through StateProvider closures, while the Bubbletea loop mutates the same *Playlist. The struct had no synchronization, so reads of p.tracks/p.order/p.pos raced with Next/Prev/Add/Remove/shuffle, and Current()'s unguarded p.tracks[idx] could panic on a stale index. Add a sync.Mutex and guard every exported method. Reads and writes are now serialized; no exported method calls another, so defer-unlock cannot deadlock. Fixes the high-severity track/player getter race and the playlist-mutation data race. * fix(luaplugin): block SSRF in cliamp.http doHTTP passed plugin-supplied URLs straight to the HTTP client with no validation, letting a plugin reach loopback, RFC1918, and link-local 169.254.169.254 (cloud metadata), and follow redirects into those targets. Add an ssrfGuard on the dialer Control hook so every connection attempt (including redirects and DNS-rebound hosts) is checked against the resolved IP and rejected if non-public, and reject non-http(s) schemes up front. A per-plugin network permission gate remains a possible follow-up; it is omitted here to avoid breaking existing plugins that legitimately use http. * fix(pluginmgr): sandbox plugin VM during metadata extraction extractMetadata ran DoFile on the entire plugin file with full stdlib, so any top-level os.execute/io call ran unsandboxed the moment a user listed installed plugins. Apply the same luaplugin.Sandbox used by the runtime before executing the file. Exports luaplugin.Sandbox for reuse. * fix(luaplugin): error on oversized fs.read instead of silent truncation fs.read used os.ReadFile (whole file into memory) then sliced to 1MB, returning a silently truncated value as success and not bounding memory for huge files. Stream through io.LimitReader(maxSize+1) and return an explicit error when the file exceeds the limit. * fix(luaplugin): make write allowlist symlink-safe isWriteAllowed validated paths lexically: the strings.Contains(abs, "..") check was dead (filepath.Abs already cleans "..", and it false-positived on filenames like "..bar"), and a symlink planted inside an allowed dir could redirect a write outside it. Resolve symlinks on the deepest existing ancestor of the target and on the allow dirs themselves before the prefix check. Also fixes the same gap in the exec cwd check, which reuses this function. * fix(luaplugin): bound timer callbacks with hookTimeout timer.after/every called CallByParam directly with no context timeout, so a runaway callback held the plugin mutex forever, blocking every other hook, keybind, and visualizer for that plugin. Add a shared callBounded helper that applies hookTimeout (mirroring invokeHook) and use it from both timer paths. * fix(luaplugin): bound visualizer render/init/destroy with hookTimeout RenderVis runs on the UI render loop and called CallByParam with no timeout while holding the plugin mutex; a hanging render() froze the UI with no escape. InitVis/DestroyVis had the same gap. Route all three through callBounded so a misbehaving visualizer times out and render falls back to the previous frame. * fix(luaplugin): wait for async hook goroutines before closing LStates Emit spawned untracked goroutines that call CallByParam on a plugin LState. Close stopped timers/execs then closed every LState without waiting, so a UI event dispatched just before shutdown could run against an already-closed LState (p.L.Close does not take the plugin mutex). Track Emit goroutines in a WaitGroup, set a closing flag under mu to reject late dispatch, and Wait before closing the LStates. * fix(ui): guard spectrum visualizers against zero-width panel renderFirework/renderBubbles/renderSakura compute dotCols = PanelWidth*2 and then mod by dotCols/dotRows/len(bands); on a narrow terminal PanelWidth can be 0, so seed % uint64(dotCols) panicked and crashed the TUI. Add the same dotRows<4 || dotCols<4 early-out the braille-grid renderers already use. * fix(history): do not clobber history on a transient load error Record discarded loadLocked's error and proceeded as if history were empty, so a one-off read failure (permission/I-O glitch) made saveLocked atomically rewrite the file with only the new entry, destroying all prior rows. Propagate the error instead, leaving the on-disk file untouched. * fix(history): detect write errors before committing history file saveLocked ignored errors from writeEntry's formatted writes, so a failure mid-write (e.g. ENOSPC) could still rename a truncated temp file over the real history. Thread writes through an errWriter and abort the rename when a write fails. * fix(lyrics): stop cleanQuery from erasing titles with label words The noise regex matched official/lyric/audio/video as bare substrings followed by .*, so 'Videotape', 'Audioslave', and 'Video Games' were cleaned to empty or truncated queries, breaking lyric lookups. Restrict bare-label stripping to genuine trailing labels (dash suffix, or 'official video/audio' and 'lyric(s) video' phrases). Add regression test cases. * fix(jellyfin,emby): synchronize client token/userID/album cache Client.token, userID, and albumCache were lazily read and written from concurrent tea.Cmd goroutines (Playlists, album browse, SearchTracks all run in their own goroutines) with no synchronization, a data race on the lazy auth writes and the cached slice. Add a mutex guarding those fields, taken only around field access and never across network I/O, with double-checked auth so at most a redundant (not racy) auth can occur. * fix(ipc): close in-flight connections on shutdown handleConn's read loop only observed the 60s per-request read deadline, never s.done, and Close closed only the listener. A still-connected client (e.g. a vis-bands polling client) kept its handler goroutine alive, so Close blocked on wg.Wait for up to 60s. Track live connections and close them in Close so their scanner.Scan unblocks immediately; addConn shares the conn mutex with the done check to close the accept-during-shutdown race. * refactor(ipc): route all reply-waits through waitReply The reply/timeout/shutdown select was open-coded in load/theme/vis/bands/ status and waitReply was used elsewhere with a generic 'timeout' message. Parameterize waitReply with a label and timeout and use it everywhere, which also gives the previously-generic commands specific timeout messages. * fix(ipc): handle accept errors instead of spinning silently acceptLoop swallowed every non-shutdown Accept error and retried at 10/s, treating a permanently closed listener as transient. Return on net.ErrClosed and log other errors before backing off. * fix(ui): notify Lua plugins on jump-to-time seek The jump enter handler called notifyPlayback (MPRIS only) plus notifier.Seeked, skipping plugin notification. Use finishSeek, which calls notifyAll, so a jump seek emits the playback-state event to plugins like every other completed seek. Seek stays synchronous (immediate seek), matching the existing test. * fix(resolve): bound startup feed sniff with a short timeout Args runs on the startup path and called sniffFeedURL, which did a HEAD on the 30s feed/M3U client, so one slow CDN could stall cliamp startup for up to 30s during pure URL classification. Give the sniff a dedicated 5s client. * fix(pluginmgr): reject empty download body An empty 200 response left download returning a non-nil empty slice, so Install wrote a 0-byte plugin file. Treat an empty body as a download error so the next candidate URL is tried (or the install fails cleanly). * fix(pluginmgr): reject raw URLs with no usable filename A raw URL ending in '/' made path.Base yield '.' or '/', producing a degenerate plugin filename. Return a clear error instead. * fix(config): let --low-power=false override config-enabled low power The override only set LowPower when the flag was true, so --low-power=false could not disable a config.toml low_power=true. Assign the flag value directly, matching the other boolean overrides. * fix(main): log MPRIS/NowPlaying init failure instead of dropping it wireMediaCtl's error was silently discarded in TUI mode, so a dbus/MPRIS setup failure left media-control silently disabled with no trace. Log it to the app log (not stderr, which would corrupt the TUI). * docs(plugins): remove unimplemented player getters cliamp.player.eq_preset(), visualizer(), and theme() were documented but never registered in the player API (calling them errors). Implementing them would require exposing model state to plugin goroutines, which would add a data race; remove them from the docs so the reference matches the API. * fix(luaplugin): reject partial bands table in set_eq_preset A bands table missing entries silently zeroed the unset bands. Require all 10 values and raise an arg error otherwise so partial input can't corrupt the EQ curve. * fix(luaplugin): track EmitKey goroutines and gate on shutdown EmitKey spawned fire-and-forget goroutines per keypress that were untracked and not gated on shutdown, so a keypress during Close could call into a closed LState. Track them in the manager WaitGroup and skip dispatch once closing, matching Emit. * fix(mediactl): apply MPRIS volume changes in order Each volume Change spawned its own goroutine to send SetVolumeMsg, so rapid changes could be delivered out of order and an older volume win. send (prog.Send) is goroutine-safe and non-blocking, so call it directly. * fix(mediactl): reject stale-track MPRIS SetPosition trackid was a fixed constant for every track, so SetPosition ignored its trackID argument and would seek whatever track became current after the client read its position. Assign a unique track object path per track change and ignore SetPosition when its trackID does not match the current track. * fix(radio): write favorites atomically save() truncated the real favorites file with os.Create and ignored every write error, so a partial write could lose all favorites. Build the content in memory, write a temp file, and rename so a failed write can't corrupt the existing file and the error surfaces to the caller. * fix(radio): propagate save errors from ToggleFavorite ToggleFavorite discarded the error from Add/Remove (which persist favorites), so a failed save was silently swallowed. Return it to the caller. * refactor(spotify): drop pointless lock around local slice append The mutex around appending the Your Music entry only touched the function-local 'all' slice, guarding nothing shared (the other locks in the loop legitimately guard trackCache). * fix(spotify): return cloned playlist list, not the cache slice Playlists returned the cached slice directly (both on cache hit and after building it), so a caller mutating the result corrupted the cache. Return a clone on both paths. * fix(spotify): return cloned cached tracks Tracks handed out the cached track slice directly, so a caller mutating it corrupted the per-playlist cache. Clone on both the cache-hit and freshly built return paths. (The snapshot-based invalidation within the playlist-list TTL is an intentional caching tradeoff and is left as-is.) * fix(spotify): don't sleep before giving up on rate limit On the final retry the loop slept the full backoff (up to 128s) and then returned the rate-limited error without making another request. Break out immediately on the last attempt. * fix(jellyfin): guard AlbumList against negative offset A negative offset passed the offset>=len check and then panicked at out[offset:end]. Clamp offset to 0 first, matching the Emby client. * docs(playlist): correct MoveQueue comment MoveQueue swaps any two positions, not only adjacent ones. * refactor(ui): simplify dead math.Min in flame tier mapping math.Min(0.65, 0.55) is a constant 0.55; replace with the literal and drop the now-unused math import. * refactor(netease): compare app code against its own constant resp.Code is NetEase's application-level status, not an HTTP status; it was compared against http.StatusOK which only coincidentally shares the value 200. Introduce neteaseCodeOK and use it at all four comparison sites. * fix(resolve): strip audio extension from derived title humanizeBasename left a trailing extension (e.g. .mp3) in the title derived from a URL basename. Drop a recognized audio extension first, leaving non-media dotted suffixes untouched. * fix(resolve): bound RSS feed read size resolveFeed decoded the response body with no size limit. Wrap it in an io.LimitReader (32 MB) so an oversized or malicious feed can't exhaust memory. * fix(navidrome): return cloned cache slices Playlists and Tracks handed out the internal cached slices, so a caller mutating the result corrupted the cache. Return slices.Clone on the cache-hit and freshly built return paths. * fix(player): include ffmpeg stderr in decode error decodeFFmpeg wrapped the exec error but dropped ffmpeg's stderr, where the actual failure reason is written. Surface ExitError.Stderr in the message. * refactor(player): forward speedStreamer.Err directly beep.Streamer already declares Err() error, so the local errorer type assertion was redundant. Call ss.s.Err() directly, matching the eq and volume streamers. * fix(ui): don't drop queued seek/lyric cmds on reconnect When the scheduled stream reconnect fired, the tick handler returned early with only playTrack + tick, discarding the seekCmd/lyricCmd computed earlier in the same tick. Fold them into the reconnect batch. * fix(ui): nav list footer counts reflect committed filter The footer only showed the filtered count while the search input bar was open (searching); once a filter was committed with Enter it fell back to the full count. Add navFilteredTotal and use the filtered count whenever a query is active, across the artist, album, and track footers. * refactor: remove dead internal/control package internal/control duplicated the message types in internal/playback and had no importers (only its own test); the live code uses internal/playback. Remove it. * fix(providers): implement Refresh for self-hosted providers Navidrome, Plex, Jellyfin, and Emby cache playlist/track (and album) data but none implemented playlist.Refresher, so the UI refresh key was a silent no-op for them. Add Refresh to clear the caches (including the jellyfin/emby client album cache) so the next fetch hits the server. * refactor: share minimal-TOML section parser The [[section]] parse loop was duplicated three times (radio loadStations, radio loadFavoriteStations, local loadTOML). Extract tomlutil.ParseSections and rewrite all three against it. Behavior is preserved, including the single-section-type assumption of the original parsers. * refactor: extract shared Emby/Jellyfin client into internal/embyapi The emby and jellyfin clients were ~95% identical (~700 duplicated lines) and had already drifted (jellyfin lacked the negative-offset guard and error wrapping). Extract one shared Client into internal/embyapi, isolating the real differences behind a dialect: auth header scheme (Emby Authorization vs Jellyfin X-Emby-Authorization), ping endpoint, user-id discovery, error prefix, and metadata key. emby and jellyfin become thin wrappers (NewClient, IsStreamURL, type aliases) plus their providers. Client tests are consolidated in embyapi: shared behavior once, dialect specifics per dialect. The client now uses a per- instance http.Client (SetHTTPClient) instead of a package global, which is what makes it testable from the provider packages.	2026-05-28 22:42:07 +02:00
Bjarne Øverli	16a79bbea2	spotify: add built-in fallback client_id, friendlier search error ... Bring-your-own-client_id has been the only path to Spotify since the integration landed, but Spotify's Nov 27 2024 dev-mode quota change silently broke /v1/search for every freshly-registered developer app. Users in #205 and #214 hit a misleading 400 'Invalid limit' that no amount of parameter tweaking could resolve — the restriction is on the app, not the request. Ship the librespot keymaster client_id (the same fallback ncspot and spotify-player use) as a built-in alternative. Spotify's loopback exception lets it work with cliamp's existing :19872 redirect URI, and it predates the Nov 27 cutoff so /v1/search keeps working. Setup wizard now prompts for which client_id to use, leading with the recommendation to register your own app (private rate-limit quota) and offering the shared built-in for users who specifically need search to work despite the dev-mode restriction. The trade-off is documented in the picker intro, docs/spotify.md, config.toml.example, and the site. Also surface a friendlier search error: when Spotify returns the canonical 400 'Invalid limit' on /v1/search, rewrite it to explain that the user's client_id is too new for catalog access and how to fix it. The rewrite only fires on the exact misleading-error signature, so unrelated 400s pass through. Drop the now-unused userCountry cache and market= query param from SearchTracks. The market parameter never actually fixed anything (the real cause was dev-mode); user OAuth tokens carry account country implicitly. /v1/me is still called once per session for userID, used by playlistAccessible to filter non-owned playlists for users on their own dev-mode app. Refs #205, #214	2026-05-09 18:07:21 +02:00
Bjarne Øverli	da13d0e0e3	spotify: use real ISO market from /v1/me on search ... The previous fix passed market=from_token on /v1/search, but Spotify's Web API no longer accepts that legacy magic value and rejects it with the same misleading 400 'Invalid limit' error it returns for missing market on accounts in regions where one is required. Resolve the user's ISO 3166-1 alpha-2 country from /v1/me (cached alongside userID) and pass it as market. Fall back to omitting the parameter when /v1/me fails — the OAuth user token still scopes results to the account's country in most regions. The /v1/me lookup is attempted at most once per session via a meFetched flag; failures are cached too so a network blip during first search doesn't trigger a request per keystroke. Fixes #205	2026-05-08 10:41:11 +02:00
Bjarne Øverli	4b6ed1c5da	spotify: pass market=from_token on /v1/search ... Spotify's Web API returns misleading 400 'Invalid limit' errors on /v1/search for tokens whose home market is one of the regions where search refuses to run without an explicit market parameter (Pakistan, Bangladesh, and similar). Scoping results to the token's home market via market=from_token resolves it. The session.go comment already documents the misleading-error pattern for this endpoint; this is the regional flavor of the same misdirection. Fixes #205	2026-05-07 17:51:24 +02:00
Andrew Carter	7a68eee191	Spotify: stop rapid skipping from triggering browser re-auth (#212) ... Skipping tracks aggressively in a Spotify playlist would frequently pop open a browser to re-authenticate, even though the librespot session was still valid. Two issues caused this, plus a contention bug that made the first easier to hit: 1. isAuthError treated any context.DeadlineExceeded as an auth failure. When skipping fast, the per-stream 30s context (or a wrapped DeadlineExceeded surfaced from librespot's chunk fetch when it was interrupted) was misclassified, kicking off the reconnect path. Reclassify: deadline/cancellation are NOT auth signals; only KeyProviderError is. 2. NewStreamer's reconnect path ended in ReconnectInteractive on the second failure, which always opens a browser. Replace the interactive fallback with returning playlist.ErrNeedsAuth so the UI can surface a sign-in prompt rather than yanking the user into a browser tab mid-skip. Silent reconnect from cached creds is still attempted once. 3. Session.NewStream held s.mu across the librespot network call, so concurrent NewStream / webApi calls serialized and were more likely to hit the 30s timeout under rapid skipping. Snapshot s.player under the lock and call NewStream lock-free. Tests: - New TestIsAuthError covers nil, plain errors, deadline/cancellation (wrapped + bare), and KeyProviderError (wrapped + bare). Docs: - docs/spotify.md notes the new behavior: rapid skipping never opens a browser; sign-in prompts surface in the UI instead. spotify: stop rapid skipping from triggering browser re-auth Skipping tracks aggressively in a Spotify playlist would frequently pop open a browser to re-authenticate, even though the librespot session was still valid. Two issues caused this, plus a contention bug that made the first easier to hit: 1. isAuthError treated any context.DeadlineExceeded as an auth failure. When skipping fast, the per-stream 30s context (or a wrapped DeadlineExceeded surfaced from librespot's chunk fetch when it was interrupted) was misclassified, kicking off the reconnect path. Reclassify: deadline/cancellation are NOT auth signals; only KeyProviderError is. 2. NewStreamer's reconnect path ended in ReconnectInteractive on the second failure, which always opens a browser. Replace the interactive fallback with returning playlist.ErrNeedsAuth so the UI can surface a sign-in prompt rather than yanking the user into a browser tab mid-skip. Silent reconnect from cached creds is still attempted once. 3. Session.NewStream held s.mu across the librespot network call, so concurrent NewStream / webApi calls serialized and were more likely to hit the 30s timeout under rapid skipping. Snapshot s.player under the lock and call NewStream lock-free. Tests: - New TestIsAuthError covers nil, plain errors, deadline/cancellation (wrapped + bare), and KeyProviderError (wrapped + bare). Docs: - docs/spotify.md notes the new behavior: rapid skipping never opens a browser; sign-in prompts surface in the UI instead. spotify: stop rapid skipping from triggering browser re-auth Skipping tracks aggressively in a Spotify playlist would frequently pop open a browser to re-authenticate, even though the librespot session was still valid. Two issues caused this, plus a contention bug that made the first easier to hit: 1. isAuthError treated any context.DeadlineExceeded as an auth failure. When skipping fast, the per-stream 30s context (or a wrapped DeadlineExceeded surfaced from librespot's chunk fetch when it was interrupted) was misclassified, kicking off the reconnect path. Reclassify: deadline/cancellation are NOT auth signals; only KeyProviderError is. 2. NewStreamer's reconnect path ended in ReconnectInteractive on the second failure, which always opens a browser. Replace the interactive fallback with returning playlist.ErrNeedsAuth so the UI can surface a sign-in prompt rather than yanking the user into a browser tab mid-skip. Silent reconnect from cached creds is still attempted once. 3. Session.NewStream held s.mu across the librespot network call, so concurrent NewStream / webApi calls serialized and were more likely to hit the 30s timeout under rapid skipping. Snapshot s.player under the lock and call NewStream lock-free. Tests: - New TestIsAuthError covers nil, plain errors, deadline/cancellation (wrapped + bare), and KeyProviderError (wrapped + bare). Docs: - docs/spotify.md notes the new behavior: rapid skipping never opens a browser; sign-in prompts surface in the UI instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>	2026-05-06 08:25:03 +02:00
Bjarne Øverli	cf38560434	Drop spclient fallback for Spotify Web API calls ... The spclient/login5 token from librespot is not accepted by the Spotify Web API on /v1/search and /v1/me/playlists — Spotify returns misleading errors like 'Invalid limit' or 429 instead of a clean auth failure, which made issue #205 hard to diagnose. When there is no OAuth2 Web API token source, fail fast with ErrNeedsAuth and a clear 'run cliamp spotify reset' message rather than attempting the call with the wrong token. spclient is still used for playback (its actual purpose). Also clamp SearchTracks limit to 1..50 defensively.	2026-05-05 18:08:28 +02:00
Bjarne Øverli	3b2f57aac4	Improve provider playlists/song views ... Provider playlists pane: - Mark the currently loaded playlist with a ▶ prefix and active style - Group rows under section headers when providers populate Section; Spotify now buckets playlists into Library / Your playlists / Followed - Refresh with Ctrl+R; status message confirms - Empty state names the provider and offers a remediation hint - Show 'Name · 12 tracks · 1h 23m' on rows when data is available Provider browser overlay (N): - Right-aligned per-track durations and a 'N tracks · 47:22' subtitle - Move the / filter input under the title (matches keymap overlay) - Cursor wraps top↔bottom on every screen - Clearer help labels: Play from here / Queue this / Replace queue / Append all Cross-cutting: - Replace 'Loading X...' text with a time-driven braille-dot spinner used everywhere a list is loading - Quick-switch (S/N/P/J/Y/L/R) now works from inside the nav browser and the playlist manager, not just the main pane	2026-05-04 18:40:47 +02:00
Kitb	ab56af6a7a	fix(spotify): cap track page size at 50 to match API limit (#203) ... * fix(spotify): cap track page size at 50 to match API limit Spotify Web API /v1/playlists/{id}/items silently caps `limit` at 50. The previous spotifyTrackPageSize=100 caused the pagination loop to advance offset by 100 while the server returned only 50 items per page, skipping every other 50-item window. For an 1804-track playlist this fetched 18*50 + 4 = 904 tracks instead of all 1804, presenting as `[1/904]` in the TUI. Adds a regression test asserting spotifyTrackPageSize <= 50. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(spotify): drop dead min(50, limit) and tighten test docstring After capping spotifyTrackPageSize at 50, the min(50, limit) wrapper on the /v1/me/tracks branch always returned the limit unchanged — drop it. Also collapse the regression test's docstring: the WHY already lives at the constant in provider.go, and the prior version pinned to specific [1/904]/[1/1804] track counts that rot. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(spotify): make test docstrings follow Go convention Both test functions in external/spotify/provider_test.go now carry doc comments starting with the function name, satisfying CodeRabbit's docstring coverage check (was 50%, threshold 80%). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(spotify): convert page-size guard to table-driven form Per repo coding guidelines (CLAUDE.md "Favor table-driven tests") and CodeRabbit review on PR #203, restructure the single-case invariant test to follow the table-driven pattern used elsewhere in the file. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-03 20:03:27 +02:00
Bjarne Øverli	c430f581bf	Simplify Spotify credentials helpers ... Three small cleanups from review: 1. Move CredsPath / DeleteCreds into an untagged creds.go so the Windows stub no longer needs its own copy. Drops the credsPath wrapper and the duplicate path-build/remove logic. 2. Change DeleteCreds to (bool, error) so the reset subcommand can distinguish "removed" from "did not exist" without a separate os.Stat round-trip — deleteCreds already swallowed ErrNotExist, so the pre-check was redundant. 3. Trim two over-narration comments. The fallbackMaxAttempts intent is now a single inline note; the invalid_grant block comment is reduced to one line about why we delete on this signal.	2026-04-27 20:26:04 +02:00
Bjarne Øverli	babcdc862b	Add 'cliamp spotify reset' subcommand ... Exposes the credential cleanup that the auto-detection in newSessionFromStored already performs internally, as an explicit recovery action for users who hit a state the detector misses. Removes ~/.config/cliamp/spotify_credentials.json, prints the path that was removed, and tells the user to relaunch and sign in. Updates the Spotify and CLI docs with the new command and refines the user-facing error messages on stale-auth code paths to point at 'cliamp spotify reset' as a concrete next step.	2026-04-27 20:15:48 +02:00
Bjarne Øverli	ed78ae0472	Fix treat 429 as auth failure when using spclient fallback token ... When the OAuth2 silent refresh fails, the Spotify session continues with the librespot spclient token as a Web API fallback. Spotify rate-limits that token aggressively on /v1/me and friends, so the existing 8-retry exponential backoff (~4 minutes) just produces misleading "rate limited" warnings without ever recovering. Detect the fallback-token state via Session.usingFallbackToken() and cap the retry budget at 2 attempts in that mode. After that, return a playlist.ErrNeedsAuth-wrapped error so the existing sign-in UX in ui/model takes over instead of more silent retries. Also elevate the silent-refresh-failed log line from UserWarn to UserError and tell the user that calls will fail until re-auth.	2026-04-27 20:13:05 +02:00
bjarneo	90ca8b5d31	File-based logging with intent-based applog facade (#192) ... * feat: file-based logging with intent-based applog facade Closes #176. Adds a slog-backed file logger at ~/.config/cliamp/cliamp.log with a configurable level (log_level config key, --log-level CLI flag) and refactors applog into an intent-based facade with three tiers: - Debug/Info/Warn/Error: file only - Status: footer only (transient UI feedback) - UserWarn/UserError: both file and footer The footer ring buffer is preserved unchanged; the file sink is layered behind an atomic.Pointer[*slog.Logger] so log calls stay lock-free. Migrated all 11 spotify call sites: failures saving credentials and the auth-callback server error to UserError, reconnect/rate-limit warnings to UserWarn, 're-authenticated successfully' to Info+Status. Plugin-side logging and log rotation deferred to follow-ups. * docs(site): add diagnostic logging feature card Keeps site/index.html in sync with docs/configuration.md after the log_level config key was added in abfdc37. * Address CodeRabbit review - config: silently fall back to default for invalid log_level in TOML, matching the loader's behavior for other keys (volume, repeat, etc.) - spotify: extract duplicate re-auth message literal into a const - main: return applied level from initLogging so the startup log records the level that's actually in effect, not the raw config string * Drop Enabled gate from UserWarn/UserError The footer needs the formatted string regardless of file-log level, so the gate was paying for two fmt.Sprintf sites and a branch in exchange for skipping a sub-nanosecond slog dispatch. Diagnostic-only methods (Debug/Info/Warn/Error/logf) keep the gate where it actually avoids the Sprintf cost. * applog: use t.Cleanup for test logger close Resolves three errcheck violations from `defer closeFn()` by switching to `t.Cleanup(func() { _ = closeFn() })`. The explicit underscore documents the discard intent and t.Cleanup is the idiomatic place to register test resource teardown.	2026-04-25 19:40:21 +02:00
Bjarne Øverli	59db044a86	dedupe bitrate/sample-rate clamping, inline spotify unavailable check	2026-04-13 18:26:16 +02:00
Gjermund Garaba	bf77ca203e	fix: skip unavailable Spotify tracks during playback (#173) ... * fix(playlist): skip unavailable Spotify tracks during playback Mark Spotify tracks as unplayable when the API reports restrictions, skip them in next/prev/activate playback flows, and surface their state in the playlist UI. Add coverage for playlist navigation and playback behavior around unavailable tracks. * fixes and whatnot	2026-04-13 18:02:54 +02:00
Gjermund Garaba	1b53660a63	refactor: go 1.26 modernize (#178) ... * refactor: go 1.26 modernize * coderabbit pr fixes	2026-04-13 18:00:58 +02:00
Gjermund Garaba	5f3586f062	feat(spotify): configurable bitrate (#177) ... * feat(spotify): configurable bitrate * coderabbit pr fixes * coderabbit pr fixes	2026-04-13 18:00:45 +02:00
Bjarne Øverli	85e816b4e7	Cache Spotify playlist list to reduce API calls ... Playlists() results are cached for 5 minutes to avoid redundant API calls when switching providers. Mutations (add track, create playlist) invalidate the cache.	2026-04-03 21:22:16 +02:00
Bjarne Øverli	235f348840	Route spotify stderr messages to in-app log footer ... Replace fmt.Fprintf(os.Stderr) calls in the spotify provider with applog.Printf() so messages appear styled in the TUI footer instead of corrupting the alternate screen buffer. Log entries auto-expire after 6 seconds.	2026-04-03 18:59:49 +02:00
Bjarne Øverli	d89186f371	Fix lint issues	2026-04-02 20:25:56 +02:00
bjarneo	d09af49c9f	Decouple providers behind capability interfaces (#154) ... * Decouple providers behind capability interfaces * Add docs to provider vel * Simplify, refactor and remove dead code * Completely decouple from the ui * Simplify * Dead code cleanup	2026-03-30 16:08:16 +02:00
Bjarne Øverli	53d6516bc0	Simplify	2026-03-30 10:49:09 +02:00
Bjarne Øverli	19633d6f96	Add spotify playlist and search	2026-03-29 18:42:09 +02:00
Michael Ryan	d593f576b8	Add Spotify 'Your Music' support (#116)	2026-03-24 07:57:17 +01:00
Bjarne Øverli	ccb1770fed	Refactor	2026-03-23 21:39:55 +01:00
bjarneo	13c6ffff8d	Fix OAuth port leak by adding context cancellation and defer lis.Close() to prevent address already in use errors when retrying YouTube/Spotify authentication. (#110)	2026-03-21 21:05:28 +01:00
Bjarne Øverli	0aad59437c	Extract magic numbers into named constants and add comments for intentionally ignored errors	2026-03-20 21:54:33 +01:00
Luca Zhang	e8b4d860cd	Fix 403 errors for Spotify playlists saved from other users (#94) ... * Fix 403 errors for Spotify playlists saved from other users The Spotify API returns 403 when listing tracks for playlists owned by other users, even when those playlists appear in /v1/me/playlists (e.g. playlists you have saved/followed from other users). Two changes: - Filter /v1/me/playlists to only show playlists owned by the current user or marked as collaborative. Requires a single /v1/me call to get the user's ID (cached after first fetch). - Show a clear error message when 403 occurs instead of the raw HTTP error, to help users who encounter it despite the filter. Fixes #89 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Extract playlistAccessible helper and add unit tests Refactor the playlist filtering condition into a named function playlistAccessible() to make it independently testable without any HTTP calls or mocking infrastructure. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Address PR review feedback - Move 403 error message from webAPI() into Tracks() where the context is known; avoids misleading message for /v1/me or other endpoints - Clear cached userID in Close(), ensureSession(), and Authenticate() to prevent stale user ID filtering after re-authentication Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-03-16 19:43:13 +01:00
Artale	758557fa0d	feat: add Windows build support (Spotify behind build tags) (#77) ... * feat: add Windows build support (Spotify behind build tags) - Add //go:build !windows to provider.go, session.go, streamer.go - Add stub_windows.go with no-op Spotify implementation - Add windows/amd64 to release CI matrix (CGO_ENABLED=0) Tested: builds and runs on Windows 11 amd64 (cliamp test --version OK) Linux/macOS CI unchanged (still CGO_ENABLED=1 with native libs) * fix: address review feedback on Windows stub - Move //go:build constraint before package doc comment (Go requirement) - NewStreamer now returns errSpotifyUnavailable instead of nil, nil to prevent nil dereference in callers - Update New() doc comment to accurately describe nil return behavior	2026-03-14 18:40:11 +01:00
Bjarne Øverli	fd8dd62eb2	Fix to be compliant with the new spotify api	2026-03-07 14:12:03 +01:00
Bjarne Øverli	c34553de95	Dont have commited the client id as it will prompt you to log in multiple times for different client ids. ... come back to this solution later	2026-03-07 08:36:07 +01:00
Lacy Morrow	815ff6d6c4	fix(spotify): February 2026 API migration (#63) ... * Add Spotify provider using go-librespot for native playback Integrates Spotify streaming directly into cliamp's Beep audio pipeline, giving full EQ, visualizer, and gapless playback support for Spotify Premium accounts. Architecture: - external/spotify/streamer.go: Bridges go-librespot AudioSource (interleaved float32) to beep.StreamSeekCloser ([2]float64 pairs) - external/spotify/session.go: OAuth2 authentication with credential persistence in ~/.config/cliamp/spotify_credentials.json - external/spotify/provider.go: playlist.Provider using Spotify Web API for playlists/tracks, go-librespot player.NewStream for audio - player/player.go: StreamerFactory hook for custom URI schemes - player/pipeline.go: spotify:track:xxx URI detection and routing - config/config.go: [spotify] section with enabled flag Enable with `enabled = true` under `[spotify]` in config.toml. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Improve Spotify OAuth flow: print auth URL, open browser, allow Enter to retry * Add retry with backoff on 429 rate limits, improve auth UX messaging * Add stderr logging for 429 retries, increase timeout to 5min, bump max retries to 8 * Bypass spclient for Web API calls — use direct HTTP with Bearer token to avoid Client-Token header rate limits * Own OAuth2 flow: capture Web API token, auto-close browser tab, use SpotifyTokenCredentials The spclient's internal login5 token gets aggressively rate-limited (429 with Retry-After: 86400) when used against api.spotify.com. Root cause: it's not a standard Web API token. Fix: Run our own OAuth2 flow with the same client_id/scopes, capture the access_token for Web API calls, and pass it to go-librespot via SpotifyTokenCredentials for session auth. Also: custom callback server serves HTML with window.close() script, fixing the browser tab not auto-closing after auth. * Use registered Spotify Developer app client_id for Web API The go-librespot internal client_id (65b708073fc0480ea92a077233ca87bd) is shared across all librespot users and gets aggressively rate-limited for Web API calls (Retry-After: 86400). Now requires a registered Spotify Developer app: - client_id in config.toml [spotify] section - Fixed callback port 19872 for redirect URI - OAuth2 PKCE flow (no client_secret needed) Config: [spotify] enabled = true client_id = "your-client-id" Spotify Developer app redirect URI: http://127.0.0.1:19872/login * Remove internal Spotify scopes that cause 'Illegal scope' with registered apps * Minimal OAuth2 scopes — only playlist-read + streaming + user-read-private * Fix TUI freeze: stop stdin reader goroutine after auth completes The bufio.Scanner goroutine for Enter-to-retry kept reading stdin after OAuth completed, stealing input from Bubbletea's raw terminal handler. Replaced with raw os.Stdin.Read + authDone channel to stop cleanly. * Fix stored credential sessions: do fresh OAuth2 for Web API token on each launch The spclient's login5 token gets 429'd on Web API. On second launch (stored credentials), we were trying refreshWebAPIToken which tested the spclient token — always fails. Now does a quick OAuth2 PKCE flow on each launch to get a fresh Web API token. Falls back to full interactive auth if the token flow fails. * Fix nil CountryCode panic when playing Spotify tracks go-librespot's Player.getUnrestrictedTrack dereferences CountryCode to check media restrictions. We never set it, causing a nil pointer panic. Default to 'US'. * Fix pause/resume restarting track; expand scopes; dynamic country code - Set Stream=false on Spotify tracks — they're seekable, not live streams. The TUI's togglePlayPause treats Stream=true as live (stop+replay on resume), which was restarting songs from the beginning. - Expanded OAuth2 scopes to full standard Web API set (playlist modify, library modify, playback state, recently played, top tracks, follows). Internal Spotify scopes that cause 'Illegal scope' are documented and excluded. - Fetch user's country from /v1/me for accurate media restriction checks instead of hardcoding 'US'. * Dynamic playlist height, persist OAuth2 refresh token - Playlist view now fills available terminal height instead of being capped at 5 items. Recalculates on window resize. 'x' key toggles between compact (5) and full height. Minimum 3 items. - Persist OAuth2 refresh token in spotify_credentials.json. On subsequent launches, silently refresh the Web API token without opening a browser. Falls back to interactive auth if refresh fails. - Extract spotifyOAuthConfig() helper shared by doWebAPIAuth and silentTokenRefresh. * dynamic height * Fix UTF-8 encoding in OAuth callback page (✅ rendered as ✅) Add <meta charset="utf-8"> to both callback HTML pages so the checkmark emoji renders correctly in all browsers. * Fix dynamic playlist height: account for frame padding + 2-line controls Previous calculation used 12 + vis.Rows for fixed UI lines, but missed: - Frame padding (2 lines from Padding(1,3)) - Controls render as 2 lines (VOL + EQ), not 1 Corrected to 17 + vis.Rows. Playlist no longer overflows the terminal. * Fix playlist scroll: account for album separator lines in visible window Album separators (── Album Name (Year) ──) are rendered between tracks from different albums, taking extra lines. adjustScroll only counted track items, so the cursor would move past the visible area before scrolling kicked in. Now counts rendered lines (tracks + separators) to determine when to scroll. * Dynamic frame width: use full terminal width instead of fixed 80 chars The frame and panelWidth were hardcoded to 80/74 chars, causing the help bar to wrap to the next line on wider terminals. Now dynamically sizes to the terminal width on WindowSizeMsg. Album separators, seek bar, controls, and visualizer all scale to the available width. * Fix scroll with mixed album separators: count actual rendered lines Previous fix assumed every track had an album separator. Now uses renderedLineCount() helper that accurately counts lines (tracks + separators) for any range. adjustScroll walks backward from cursor to find the right scroll offset when mixed separator/no-separator tracks are present. * Measure actual UI height instead of counting lines manually The manual fixed-line count (17 + vis.Rows) was wrong — missed various multi-line renders, frame padding, etc. Now renders all non-playlist sections into a probe frame and uses lipgloss.Height() to measure the actual pixel height. Guarantees plVisible matches the real available space regardless of theme, controls layout, or status lines. * Fix x key toggle: properly toggle between compact (5) and dynamic max Previous code had dynMax == plVisible always (broken comparison). Now toggles between 5 and full dynamic height using same probe measurement as WindowSizeMsg. * Add 1-line buffer to dynamic playlist height Probe measurement was consistently 1-2 lines optimistic, causing the bottom of the playlist to clip. Add a 1-line safety margin. * Bump playlist height buffer to -2 lines * Fix playlist height: plVisible is rendered lines, not track count Root cause: renderPlaylist() did `visible := min(m.plVisible, len(tracks))` which conflated rendered line count with track count. When plVisible=15 and len(tracks)=8, visible was capped to 8 — but 8 tracks from different albums render as 16 lines (8 separators + 8 tracks), overflowing. Fix: - Remove min(plVisible, len(tracks)) — plVisible is the rendered line budget - Remove scroll clamping that used track count as line count - Add budget check before separator+track pair: if only 1 line left but need 2 (separator + track), break instead of overflowing - Remove -2 magic buffer from probe measurement — it's now exact - plVisible = height - lipgloss.Height(probeFrame) + 1 (no fudge factors) * Address PR review feedback from Gemini Code Assist - Use strconv.Atoi instead of fmt.Sscanf for year parsing - Add comment explaining Stream: false (pause/resume requires it) - Handle io.ReadAll error on non-OK HTTP response bodies - Log saveCreds errors instead of silently discarding - Log http.Serve errors (filter net.ErrClosed for clean shutdown) - Log country code fetch errors with stderr fallback - Add TODO for configurable bitrate * Add Spotify provider using go-librespot for native playback Integrates Spotify streaming directly into cliamp's Beep audio pipeline, giving full EQ, visualizer, and gapless playback support for Spotify Premium accounts. Architecture: - external/spotify/streamer.go: Bridges go-librespot AudioSource (interleaved float32) to beep.StreamSeekCloser ([2]float64 pairs) - external/spotify/session.go: OAuth2 authentication with credential persistence in ~/.config/cliamp/spotify_credentials.json - external/spotify/provider.go: playlist.Provider using Spotify Web API for playlists/tracks, go-librespot player.NewStream for audio - player/player.go: StreamerFactory hook for custom URI schemes - player/pipeline.go: spotify:track:xxx URI detection and routing - config/config.go: [spotify] section with enabled flag Enable with `enabled = true` under `[spotify]` in config.toml. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Improve Spotify OAuth flow: print auth URL, open browser, allow Enter to retry * Add retry with backoff on 429 rate limits, improve auth UX messaging * Add stderr logging for 429 retries, increase timeout to 5min, bump max retries to 8 * Bypass spclient for Web API calls — use direct HTTP with Bearer token to avoid Client-Token header rate limits * Own OAuth2 flow: capture Web API token, auto-close browser tab, use SpotifyTokenCredentials The spclient's internal login5 token gets aggressively rate-limited (429 with Retry-After: 86400) when used against api.spotify.com. Root cause: it's not a standard Web API token. Fix: Run our own OAuth2 flow with the same client_id/scopes, capture the access_token for Web API calls, and pass it to go-librespot via SpotifyTokenCredentials for session auth. Also: custom callback server serves HTML with window.close() script, fixing the browser tab not auto-closing after auth. * Use registered Spotify Developer app client_id for Web API The go-librespot internal client_id (65b708073fc0480ea92a077233ca87bd) is shared across all librespot users and gets aggressively rate-limited for Web API calls (Retry-After: 86400). Now requires a registered Spotify Developer app: - client_id in config.toml [spotify] section - Fixed callback port 19872 for redirect URI - OAuth2 PKCE flow (no client_secret needed) Config: [spotify] enabled = true client_id = "your-client-id" Spotify Developer app redirect URI: http://127.0.0.1:19872/login * Remove internal Spotify scopes that cause 'Illegal scope' with registered apps * Minimal OAuth2 scopes — only playlist-read + streaming + user-read-private * Fix TUI freeze: stop stdin reader goroutine after auth completes The bufio.Scanner goroutine for Enter-to-retry kept reading stdin after OAuth completed, stealing input from Bubbletea's raw terminal handler. Replaced with raw os.Stdin.Read + authDone channel to stop cleanly. * Fix stored credential sessions: do fresh OAuth2 for Web API token on each launch The spclient's login5 token gets 429'd on Web API. On second launch (stored credentials), we were trying refreshWebAPIToken which tested the spclient token — always fails. Now does a quick OAuth2 PKCE flow on each launch to get a fresh Web API token. Falls back to full interactive auth if the token flow fails. * Fix nil CountryCode panic when playing Spotify tracks go-librespot's Player.getUnrestrictedTrack dereferences CountryCode to check media restrictions. We never set it, causing a nil pointer panic. Default to 'US'. * Fix pause/resume restarting track; expand scopes; dynamic country code - Set Stream=false on Spotify tracks — they're seekable, not live streams. The TUI's togglePlayPause treats Stream=true as live (stop+replay on resume), which was restarting songs from the beginning. - Expanded OAuth2 scopes to full standard Web API set (playlist modify, library modify, playback state, recently played, top tracks, follows). Internal Spotify scopes that cause 'Illegal scope' are documented and excluded. - Fetch user's country from /v1/me for accurate media restriction checks instead of hardcoding 'US'. * Dynamic playlist height, persist OAuth2 refresh token - Playlist view now fills available terminal height instead of being capped at 5 items. Recalculates on window resize. 'x' key toggles between compact (5) and full height. Minimum 3 items. - Persist OAuth2 refresh token in spotify_credentials.json. On subsequent launches, silently refresh the Web API token without opening a browser. Falls back to interactive auth if refresh fails. - Extract spotifyOAuthConfig() helper shared by doWebAPIAuth and silentTokenRefresh. * dynamic height * Fix UTF-8 encoding in OAuth callback page (✅ rendered as ✅) Add <meta charset="utf-8"> to both callback HTML pages so the checkmark emoji renders correctly in all browsers. * Fix dynamic playlist height: account for frame padding + 2-line controls Previous calculation used 12 + vis.Rows for fixed UI lines, but missed: - Frame padding (2 lines from Padding(1,3)) - Controls render as 2 lines (VOL + EQ), not 1 Corrected to 17 + vis.Rows. Playlist no longer overflows the terminal. * Fix playlist scroll: account for album separator lines in visible window Album separators (── Album Name (Year) ──) are rendered between tracks from different albums, taking extra lines. adjustScroll only counted track items, so the cursor would move past the visible area before scrolling kicked in. Now counts rendered lines (tracks + separators) to determine when to scroll. * Dynamic frame width: use full terminal width instead of fixed 80 chars The frame and panelWidth were hardcoded to 80/74 chars, causing the help bar to wrap to the next line on wider terminals. Now dynamically sizes to the terminal width on WindowSizeMsg. Album separators, seek bar, controls, and visualizer all scale to the available width. * Fix scroll with mixed album separators: count actual rendered lines Previous fix assumed every track had an album separator. Now uses renderedLineCount() helper that accurately counts lines (tracks + separators) for any range. adjustScroll walks backward from cursor to find the right scroll offset when mixed separator/no-separator tracks are present. * Measure actual UI height instead of counting lines manually The manual fixed-line count (17 + vis.Rows) was wrong — missed various multi-line renders, frame padding, etc. Now renders all non-playlist sections into a probe frame and uses lipgloss.Height() to measure the actual pixel height. Guarantees plVisible matches the real available space regardless of theme, controls layout, or status lines. * Fix x key toggle: properly toggle between compact (5) and dynamic max Previous code had dynMax == plVisible always (broken comparison). Now toggles between 5 and full dynamic height using same probe measurement as WindowSizeMsg. * Add 1-line buffer to dynamic playlist height Probe measurement was consistently 1-2 lines optimistic, causing the bottom of the playlist to clip. Add a 1-line safety margin. * Bump playlist height buffer to -2 lines * Fix playlist height: plVisible is rendered lines, not track count Root cause: renderPlaylist() did `visible := min(m.plVisible, len(tracks))` which conflated rendered line count with track count. When plVisible=15 and len(tracks)=8, visible was capped to 8 — but 8 tracks from different albums render as 16 lines (8 separators + 8 tracks), overflowing. Fix: - Remove min(plVisible, len(tracks)) — plVisible is the rendered line budget - Remove scroll clamping that used track count as line count - Add budget check before separator+track pair: if only 1 line left but need 2 (separator + track), break instead of overflowing - Remove -2 magic buffer from probe measurement — it's now exact - plVisible = height - lipgloss.Height(probeFrame) + 1 (no fudge factors) * Address PR review feedback from Gemini Code Assist - Use strconv.Atoi instead of fmt.Sscanf for year parsing - Add comment explaining Stream: false (pause/resume requires it) - Handle io.ReadAll error on non-OK HTTP response bodies - Log saveCreds errors instead of silently discarding - Log http.Serve errors (filter net.ErrClosed for clean shutdown) - Log country code fetch errors with stderr fallback - Add TODO for configurable bitrate * Add Spotify docs to README, update install instructions for fork - Added Spotify section with setup instructions (Developer app, config, OAuth) - Updated install methods: go install, pre-built binaries, build from source - Removed Homebrew tap update from release workflow (fork-specific) * Auto-refresh Web API token: replace static string with TokenSource The webAPIToken was set once during init and never refreshed. Spotify access tokens expire after 1 hour, causing all Web API calls to fail. Now uses oauth2.TokenSource which automatically refreshes the token using the stored refresh token when it expires. No more browser re-authentication after 1 hour — tokens refresh transparently in the background. * Show shuffle (z) and repeat (r) keys in bottom help bar They were only visible in the Ctrl+K keymap overlay but not in the always-visible bottom controls line. * Persist shuffle/repeat state to config on toggle Saves shuffle and repeat preferences to config.toml when toggled via z/r keys, so they survive restarts across all providers. * Move shuffle/repeat to top of Ctrl+K keymap overlay They were at positions 23-24, below the 12-line visible window. Moved them right after volume controls so they're visible without scrolling. * Responsive help bar: drop low-priority hints when terminal is narrow Each help hint has a priority. When the combined width exceeds the terminal width, the lowest-priority hints are dropped first: 100 Spc(⏯) 95 Q(Quit) 90 <>(Trk) 80 +-(Vol) 70 ←→(Seek) 60 Ctrl+K(Keys) 50 Tab(Focus) 40 /(Search) 30 a(Queue) 20 z(Shfl) 20 r(Rpt) On a narrow terminal you still see play/pause, track nav, vol, and quit. On wide terminals everything shows. * Handle config.Save errors for shuffle/repeat Show a status message via saveMsg when persisting fails instead of silently ignoring the error. Addresses Gemini Code Assist review feedback. * fix: don't auto-play when selecting Spotify playlist during playback When a track is already playing, selecting a new playlist now loads the tracks without stopping playback or auto-playing. Users can browse playlists freely while listening. Auto-play only triggers when nothing is currently playing. * fix: auto re-auth on Spotify AES key / session errors When go-librespot fails to retrieve an audio key (e.g. code 2 from Spotify's AP server due to expired/revoked session), the provider now: 1. Detects auth-related errors (KeyProviderError, DeadlineExceeded) 2. Tears down the dead session and clears stored credentials 3. Triggers a fresh OAuth2 interactive flow automatically 4. Retries the stream once with the new session This prevents users from getting stuck in an error loop — no manual credential deletion or CLI commands needed. Adds Session.Reconnect() for hot-swapping the session/player, and deleteCreds() to clear stale stored credentials. * fix: avoid nil window in Reconnect (address review) Create the new session before tearing down the old one so s.sess and s.player are never nil while the mutex is unlocked. Old session/player are closed after the atomic swap completes. Addresses Gemini review comment on PR #4. * auto commit * auto commit * feat(spotify): fallback client ID pool for zero-config setup Users no longer need to register a Spotify Developer app. When no client_id is configured, a random ID is selected from a built-in fallback pool to spread rate-limit load across apps. - Add external/spotify/fallback.go with FallbackClientID() - Add SpotifyConfig.ResolveClientID() with user > fallback priority - SpotifyConfig.IsSet() now returns true when enabled (even without client_id) - Update config.toml.example with Spotify section docs * migrate: Spotify Feb 2026 API changes Spotify deprecated several endpoints and renamed fields (effective Mar 9, 2026 for existing dev mode apps). See: https://developer.spotify.com/documentation/web-api/tutorials/february-2026-migration-guide Changes: - Playlist tracks endpoint: /playlists/{id}/tracks → /playlists/{id}/items - Playlist response field: track → item (with backwards-compat fallback) - Playlist metadata field: tracks.total → items.total (with fallback) - Remove GET /v1/me call for country (field removed from API), default to US - Remove user-read-email scope (email no longer returned by GET /me) - Remove unused 'io' import from session.go - Document removed fields in scope comments (popularity, available_markets, external_ids, country, followers, product) * perf(spotify): reduce API calls with fields filtering and snapshot caching Three optimizations to reduce rate limit pressure: 1. Use 'fields' parameter on both /me/playlists and /playlists/{id}/items to request only the fields we actually parse. Smaller payloads, lower API cost per call. 2. Cache playlist tracks by snapshot_id. When Playlists() is called, we store each playlist's snapshot_id. If the snapshot hasn't changed on the next Tracks() call, we return cached results without hitting the API at all. 3. Include snapshot_id in playlist list fields so cache invalidation is automatic — changed playlists get re-fetched, unchanged ones don't. Ref: https://developer.spotify.com/documentation/web-api/concepts/rate-limits * fix(spotify): skip session when no client ID is available Don't attempt OAuth flow if both user config and fallback pool are empty — prevents the broken authorize URL with empty client_id. * auto commit * chore: remove build/release changes — keep only Spotify API migration Revert .github/workflows/release.yml and install.sh to upstream/main and remove the added .goreleaser.yml so this branch contains only the Spotify February 2026 API migration changes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>	2026-03-07 08:09:40 +01:00
Bjarne Øverli	a2c59fffd2	Revert back to item	2026-03-06 20:17:04 +01:00
Bjarne Øverli	2453e7e27f	Bug fix (provider.go:126): Changed json:item to json:track — this was preventing ALL track metadata from deserializing. This is likely why playlists appeared empty.	2026-03-06 19:43:47 +01:00
Bjarne Øverli	c89d0225c7	use Spotify /items endpoint replacing deprecated /tracks for playlist retrieval	2026-03-06 19:14:26 +01:00
Lacy Morrow	e55101617c	feat: Spotify playback via go-librespot (#36) ... * Add Spotify provider using go-librespot for native playback Integrates Spotify streaming directly into cliamp's Beep audio pipeline, giving full EQ, visualizer, and gapless playback support for Spotify Premium accounts. Architecture: - external/spotify/streamer.go: Bridges go-librespot AudioSource (interleaved float32) to beep.StreamSeekCloser ([2]float64 pairs) - external/spotify/session.go: OAuth2 authentication with credential persistence in ~/.config/cliamp/spotify_credentials.json - external/spotify/provider.go: playlist.Provider using Spotify Web API for playlists/tracks, go-librespot player.NewStream for audio - player/player.go: StreamerFactory hook for custom URI schemes - player/pipeline.go: spotify:track:xxx URI detection and routing - config/config.go: [spotify] section with enabled flag Enable with `enabled = true` under `[spotify]` in config.toml. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Improve Spotify OAuth flow: print auth URL, open browser, allow Enter to retry * Add retry with backoff on 429 rate limits, improve auth UX messaging * Add stderr logging for 429 retries, increase timeout to 5min, bump max retries to 8 * Bypass spclient for Web API calls — use direct HTTP with Bearer token to avoid Client-Token header rate limits * Own OAuth2 flow: capture Web API token, auto-close browser tab, use SpotifyTokenCredentials The spclient's internal login5 token gets aggressively rate-limited (429 with Retry-After: 86400) when used against api.spotify.com. Root cause: it's not a standard Web API token. Fix: Run our own OAuth2 flow with the same client_id/scopes, capture the access_token for Web API calls, and pass it to go-librespot via SpotifyTokenCredentials for session auth. Also: custom callback server serves HTML with window.close() script, fixing the browser tab not auto-closing after auth. * Use registered Spotify Developer app client_id for Web API The go-librespot internal client_id (65b708073fc0480ea92a077233ca87bd) is shared across all librespot users and gets aggressively rate-limited for Web API calls (Retry-After: 86400). Now requires a registered Spotify Developer app: - client_id in config.toml [spotify] section - Fixed callback port 19872 for redirect URI - OAuth2 PKCE flow (no client_secret needed) Config: [spotify] enabled = true client_id = "your-client-id" Spotify Developer app redirect URI: http://127.0.0.1:19872/login * Remove internal Spotify scopes that cause 'Illegal scope' with registered apps * Minimal OAuth2 scopes — only playlist-read + streaming + user-read-private * Fix TUI freeze: stop stdin reader goroutine after auth completes The bufio.Scanner goroutine for Enter-to-retry kept reading stdin after OAuth completed, stealing input from Bubbletea's raw terminal handler. Replaced with raw os.Stdin.Read + authDone channel to stop cleanly. * Fix stored credential sessions: do fresh OAuth2 for Web API token on each launch The spclient's login5 token gets 429'd on Web API. On second launch (stored credentials), we were trying refreshWebAPIToken which tested the spclient token — always fails. Now does a quick OAuth2 PKCE flow on each launch to get a fresh Web API token. Falls back to full interactive auth if the token flow fails. * Fix nil CountryCode panic when playing Spotify tracks go-librespot's Player.getUnrestrictedTrack dereferences CountryCode to check media restrictions. We never set it, causing a nil pointer panic. Default to 'US'. * Fix pause/resume restarting track; expand scopes; dynamic country code - Set Stream=false on Spotify tracks — they're seekable, not live streams. The TUI's togglePlayPause treats Stream=true as live (stop+replay on resume), which was restarting songs from the beginning. - Expanded OAuth2 scopes to full standard Web API set (playlist modify, library modify, playback state, recently played, top tracks, follows). Internal Spotify scopes that cause 'Illegal scope' are documented and excluded. - Fetch user's country from /v1/me for accurate media restriction checks instead of hardcoding 'US'. * Dynamic playlist height, persist OAuth2 refresh token - Playlist view now fills available terminal height instead of being capped at 5 items. Recalculates on window resize. 'x' key toggles between compact (5) and full height. Minimum 3 items. - Persist OAuth2 refresh token in spotify_credentials.json. On subsequent launches, silently refresh the Web API token without opening a browser. Falls back to interactive auth if refresh fails. - Extract spotifyOAuthConfig() helper shared by doWebAPIAuth and silentTokenRefresh. * dynamic height * Fix UTF-8 encoding in OAuth callback page (✅ rendered as ✅) Add <meta charset="utf-8"> to both callback HTML pages so the checkmark emoji renders correctly in all browsers. * Fix dynamic playlist height: account for frame padding + 2-line controls Previous calculation used 12 + vis.Rows for fixed UI lines, but missed: - Frame padding (2 lines from Padding(1,3)) - Controls render as 2 lines (VOL + EQ), not 1 Corrected to 17 + vis.Rows. Playlist no longer overflows the terminal. * Fix playlist scroll: account for album separator lines in visible window Album separators (── Album Name (Year) ──) are rendered between tracks from different albums, taking extra lines. adjustScroll only counted track items, so the cursor would move past the visible area before scrolling kicked in. Now counts rendered lines (tracks + separators) to determine when to scroll. * Dynamic frame width: use full terminal width instead of fixed 80 chars The frame and panelWidth were hardcoded to 80/74 chars, causing the help bar to wrap to the next line on wider terminals. Now dynamically sizes to the terminal width on WindowSizeMsg. Album separators, seek bar, controls, and visualizer all scale to the available width. * Fix scroll with mixed album separators: count actual rendered lines Previous fix assumed every track had an album separator. Now uses renderedLineCount() helper that accurately counts lines (tracks + separators) for any range. adjustScroll walks backward from cursor to find the right scroll offset when mixed separator/no-separator tracks are present. * Measure actual UI height instead of counting lines manually The manual fixed-line count (17 + vis.Rows) was wrong — missed various multi-line renders, frame padding, etc. Now renders all non-playlist sections into a probe frame and uses lipgloss.Height() to measure the actual pixel height. Guarantees plVisible matches the real available space regardless of theme, controls layout, or status lines. * Fix x key toggle: properly toggle between compact (5) and dynamic max Previous code had dynMax == plVisible always (broken comparison). Now toggles between 5 and full dynamic height using same probe measurement as WindowSizeMsg. * Add 1-line buffer to dynamic playlist height Probe measurement was consistently 1-2 lines optimistic, causing the bottom of the playlist to clip. Add a 1-line safety margin. * Bump playlist height buffer to -2 lines * Fix playlist height: plVisible is rendered lines, not track count Root cause: renderPlaylist() did `visible := min(m.plVisible, len(tracks))` which conflated rendered line count with track count. When plVisible=15 and len(tracks)=8, visible was capped to 8 — but 8 tracks from different albums render as 16 lines (8 separators + 8 tracks), overflowing. Fix: - Remove min(plVisible, len(tracks)) — plVisible is the rendered line budget - Remove scroll clamping that used track count as line count - Add budget check before separator+track pair: if only 1 line left but need 2 (separator + track), break instead of overflowing - Remove -2 magic buffer from probe measurement — it's now exact - plVisible = height - lipgloss.Height(probeFrame) + 1 (no fudge factors) * Address PR review feedback from Gemini Code Assist - Use strconv.Atoi instead of fmt.Sscanf for year parsing - Add comment explaining Stream: false (pause/resume requires it) - Handle io.ReadAll error on non-OK HTTP response bodies - Log saveCreds errors instead of silently discarding - Log http.Serve errors (filter net.ErrClosed for clean shutdown) - Log country code fetch errors with stderr fallback - Add TODO for configurable bitrate * Add Spotify provider using go-librespot for native playback Integrates Spotify streaming directly into cliamp's Beep audio pipeline, giving full EQ, visualizer, and gapless playback support for Spotify Premium accounts. Architecture: - external/spotify/streamer.go: Bridges go-librespot AudioSource (interleaved float32) to beep.StreamSeekCloser ([2]float64 pairs) - external/spotify/session.go: OAuth2 authentication with credential persistence in ~/.config/cliamp/spotify_credentials.json - external/spotify/provider.go: playlist.Provider using Spotify Web API for playlists/tracks, go-librespot player.NewStream for audio - player/player.go: StreamerFactory hook for custom URI schemes - player/pipeline.go: spotify:track:xxx URI detection and routing - config/config.go: [spotify] section with enabled flag Enable with `enabled = true` under `[spotify]` in config.toml. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Improve Spotify OAuth flow: print auth URL, open browser, allow Enter to retry * Add retry with backoff on 429 rate limits, improve auth UX messaging * Add stderr logging for 429 retries, increase timeout to 5min, bump max retries to 8 * Bypass spclient for Web API calls — use direct HTTP with Bearer token to avoid Client-Token header rate limits * Own OAuth2 flow: capture Web API token, auto-close browser tab, use SpotifyTokenCredentials The spclient's internal login5 token gets aggressively rate-limited (429 with Retry-After: 86400) when used against api.spotify.com. Root cause: it's not a standard Web API token. Fix: Run our own OAuth2 flow with the same client_id/scopes, capture the access_token for Web API calls, and pass it to go-librespot via SpotifyTokenCredentials for session auth. Also: custom callback server serves HTML with window.close() script, fixing the browser tab not auto-closing after auth. * Use registered Spotify Developer app client_id for Web API The go-librespot internal client_id (65b708073fc0480ea92a077233ca87bd) is shared across all librespot users and gets aggressively rate-limited for Web API calls (Retry-After: 86400). Now requires a registered Spotify Developer app: - client_id in config.toml [spotify] section - Fixed callback port 19872 for redirect URI - OAuth2 PKCE flow (no client_secret needed) Config: [spotify] enabled = true client_id = "your-client-id" Spotify Developer app redirect URI: http://127.0.0.1:19872/login * Remove internal Spotify scopes that cause 'Illegal scope' with registered apps * Minimal OAuth2 scopes — only playlist-read + streaming + user-read-private * Fix TUI freeze: stop stdin reader goroutine after auth completes The bufio.Scanner goroutine for Enter-to-retry kept reading stdin after OAuth completed, stealing input from Bubbletea's raw terminal handler. Replaced with raw os.Stdin.Read + authDone channel to stop cleanly. * Fix stored credential sessions: do fresh OAuth2 for Web API token on each launch The spclient's login5 token gets 429'd on Web API. On second launch (stored credentials), we were trying refreshWebAPIToken which tested the spclient token — always fails. Now does a quick OAuth2 PKCE flow on each launch to get a fresh Web API token. Falls back to full interactive auth if the token flow fails. * Fix nil CountryCode panic when playing Spotify tracks go-librespot's Player.getUnrestrictedTrack dereferences CountryCode to check media restrictions. We never set it, causing a nil pointer panic. Default to 'US'. * Fix pause/resume restarting track; expand scopes; dynamic country code - Set Stream=false on Spotify tracks — they're seekable, not live streams. The TUI's togglePlayPause treats Stream=true as live (stop+replay on resume), which was restarting songs from the beginning. - Expanded OAuth2 scopes to full standard Web API set (playlist modify, library modify, playback state, recently played, top tracks, follows). Internal Spotify scopes that cause 'Illegal scope' are documented and excluded. - Fetch user's country from /v1/me for accurate media restriction checks instead of hardcoding 'US'. * Dynamic playlist height, persist OAuth2 refresh token - Playlist view now fills available terminal height instead of being capped at 5 items. Recalculates on window resize. 'x' key toggles between compact (5) and full height. Minimum 3 items. - Persist OAuth2 refresh token in spotify_credentials.json. On subsequent launches, silently refresh the Web API token without opening a browser. Falls back to interactive auth if refresh fails. - Extract spotifyOAuthConfig() helper shared by doWebAPIAuth and silentTokenRefresh. * dynamic height * Fix UTF-8 encoding in OAuth callback page (✅ rendered as ✅) Add <meta charset="utf-8"> to both callback HTML pages so the checkmark emoji renders correctly in all browsers. * Fix dynamic playlist height: account for frame padding + 2-line controls Previous calculation used 12 + vis.Rows for fixed UI lines, but missed: - Frame padding (2 lines from Padding(1,3)) - Controls render as 2 lines (VOL + EQ), not 1 Corrected to 17 + vis.Rows. Playlist no longer overflows the terminal. * Fix playlist scroll: account for album separator lines in visible window Album separators (── Album Name (Year) ──) are rendered between tracks from different albums, taking extra lines. adjustScroll only counted track items, so the cursor would move past the visible area before scrolling kicked in. Now counts rendered lines (tracks + separators) to determine when to scroll. * Dynamic frame width: use full terminal width instead of fixed 80 chars The frame and panelWidth were hardcoded to 80/74 chars, causing the help bar to wrap to the next line on wider terminals. Now dynamically sizes to the terminal width on WindowSizeMsg. Album separators, seek bar, controls, and visualizer all scale to the available width. * Fix scroll with mixed album separators: count actual rendered lines Previous fix assumed every track had an album separator. Now uses renderedLineCount() helper that accurately counts lines (tracks + separators) for any range. adjustScroll walks backward from cursor to find the right scroll offset when mixed separator/no-separator tracks are present. * Measure actual UI height instead of counting lines manually The manual fixed-line count (17 + vis.Rows) was wrong — missed various multi-line renders, frame padding, etc. Now renders all non-playlist sections into a probe frame and uses lipgloss.Height() to measure the actual pixel height. Guarantees plVisible matches the real available space regardless of theme, controls layout, or status lines. * Fix x key toggle: properly toggle between compact (5) and dynamic max Previous code had dynMax == plVisible always (broken comparison). Now toggles between 5 and full dynamic height using same probe measurement as WindowSizeMsg. * Add 1-line buffer to dynamic playlist height Probe measurement was consistently 1-2 lines optimistic, causing the bottom of the playlist to clip. Add a 1-line safety margin. * Bump playlist height buffer to -2 lines * Fix playlist height: plVisible is rendered lines, not track count Root cause: renderPlaylist() did `visible := min(m.plVisible, len(tracks))` which conflated rendered line count with track count. When plVisible=15 and len(tracks)=8, visible was capped to 8 — but 8 tracks from different albums render as 16 lines (8 separators + 8 tracks), overflowing. Fix: - Remove min(plVisible, len(tracks)) — plVisible is the rendered line budget - Remove scroll clamping that used track count as line count - Add budget check before separator+track pair: if only 1 line left but need 2 (separator + track), break instead of overflowing - Remove -2 magic buffer from probe measurement — it's now exact - plVisible = height - lipgloss.Height(probeFrame) + 1 (no fudge factors) * Address PR review feedback from Gemini Code Assist - Use strconv.Atoi instead of fmt.Sscanf for year parsing - Add comment explaining Stream: false (pause/resume requires it) - Handle io.ReadAll error on non-OK HTTP response bodies - Log saveCreds errors instead of silently discarding - Log http.Serve errors (filter net.ErrClosed for clean shutdown) - Log country code fetch errors with stderr fallback - Add TODO for configurable bitrate * Add Spotify docs to README, update install instructions for fork - Added Spotify section with setup instructions (Developer app, config, OAuth) - Updated install methods: go install, pre-built binaries, build from source - Removed Homebrew tap update from release workflow (fork-specific) * Auto-refresh Web API token: replace static string with TokenSource The webAPIToken was set once during init and never refreshed. Spotify access tokens expire after 1 hour, causing all Web API calls to fail. Now uses oauth2.TokenSource which automatically refreshes the token using the stored refresh token when it expires. No more browser re-authentication after 1 hour — tokens refresh transparently in the background. * Show shuffle (z) and repeat (r) keys in bottom help bar They were only visible in the Ctrl+K keymap overlay but not in the always-visible bottom controls line. * Persist shuffle/repeat state to config on toggle Saves shuffle and repeat preferences to config.toml when toggled via z/r keys, so they survive restarts across all providers. * Move shuffle/repeat to top of Ctrl+K keymap overlay They were at positions 23-24, below the 12-line visible window. Moved them right after volume controls so they're visible without scrolling. * Responsive help bar: drop low-priority hints when terminal is narrow Each help hint has a priority. When the combined width exceeds the terminal width, the lowest-priority hints are dropped first: 100 Spc(⏯) 95 Q(Quit) 90 <>(Trk) 80 +-(Vol) 70 ←→(Seek) 60 Ctrl+K(Keys) 50 Tab(Focus) 40 /(Search) 30 a(Queue) 20 z(Shfl) 20 r(Rpt) On a narrow terminal you still see play/pause, track nav, vol, and quit. On wide terminals everything shows. * Handle config.Save errors for shuffle/repeat Show a status message via saveMsg when persisting fails instead of silently ignoring the error. Addresses Gemini Code Assist review feedback. * fix: don't auto-play when selecting Spotify playlist during playback When a track is already playing, selecting a new playlist now loads the tracks without stopping playback or auto-playing. Users can browse playlists freely while listening. Auto-play only triggers when nothing is currently playing. * fix: auto re-auth on Spotify AES key / session errors When go-librespot fails to retrieve an audio key (e.g. code 2 from Spotify's AP server due to expired/revoked session), the provider now: 1. Detects auth-related errors (KeyProviderError, DeadlineExceeded) 2. Tears down the dead session and clears stored credentials 3. Triggers a fresh OAuth2 interactive flow automatically 4. Retries the stream once with the new session This prevents users from getting stuck in an error loop — no manual credential deletion or CLI commands needed. Adds Session.Reconnect() for hot-swapping the session/player, and deleteCreds() to clear stale stored credentials. * fix: avoid nil window in Reconnect (address review) Create the new session before tearing down the old one so s.sess and s.player are never nil while the mutex is unlocked. Old session/player are closed after the atomic swap completes. Addresses Gemini review comment on PR #4. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>	2026-03-06 15:32:53 +01:00