Feature/add youtube music

[Skeptic-systems]

Description

Brief description of the changes in this PR.

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Refactoring (no functional changes)
• Performance improvement
• Test coverage improvement

Related Issues

Fixes #(issue number)
Closes #(issue number)
Related to #(issue number)

Changes Made

• Change 1
• Change 2
• Change 3

Testing

• I have tested these changes locally
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published

Screenshots (if applicable)

Add screenshots to help explain your changes.

Checklist

• My code follows the project's style guidelines
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published

Additional Notes

Add any additional notes about the PR here.

Summary by CodeRabbit

• New Features

  • Full YouTube Music support (playback, search, playlists) and a built-in YouTube player
  • "Clear Everything" button in Settings to reset app data

• Improvements

  • AI DJ: reduced duplicates, more variety (shuffle, exclude recent, persistent played history)
  • Persistent last-played track & playback position; per-provider playback caching
  • Provider-aware UI and multi-provider handling (Spotify + YouTube)

• Bug Fixes

  • Authentication race-condition fixes and faster credential availability
  • YouTube volume, auth flow, and playback reliability fixes

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Adds YouTube Music provider and provider abstraction, OS keyring-backed credential caching with blocking I/O, persistent per-provider last-played caching, AI DJ improvements (shuffle, history persistence, de-duplication), a "Clear Everything" reset, and numerous frontend/provider wiring and type migrations.

Changes

Cohort / File(s)	Summary
Changeset docs \.changeset/ai-dj-randomness.md, \.changeset/clear-everything-button.md, \.changeset/fix-keyring-race.md, \.changeset/fix-rust-deps.md, \.changeset/last-played-track.md, \.changeset/secure-data-keyring.md, \.changeset/youtube-music-fixes.md, \.changeset/youtube-music-full-integration.md, \.changeset/youtube-music-support.md	New changelogs describing AI DJ variety/history, Clear Everything, keyring fixes, last-played persistence, secure keyring storage, and staged YouTube integration.
Build & tooling \.github/workflows/lint-desktop.yml, \.github/workflows/release-desktop.yml, package.json, apps/docs/package.json, apps/www/package.json, apps/www/dockerfile, biome.json, apps/desktop/package.json, apps/desktop/src-tauri/Cargo.toml	Action and dependency bumps across JS/Rust stacks, Node base image update, biome config changes.
Keyring & auth (backend) apps/desktop/src-tauri/src/ai_keyring.rs, apps/desktop/src-tauri/src/spotify_auth.rs, apps/desktop/src-tauri/src/youtube_auth.rs, apps/desktop/scripts/clear-auth.js	In-memory caches for credentials, keyring ops offloaded via spawn_blocking, new YouTube OAuth subsystem, token refresh task, clear-auth extended for YouTube and AI keys.
Settings & themes (backend) apps/desktop/src-tauri/src/settings.rs, apps/desktop/src-tauri/src/custom_themes.rs	App-scoped settings and themes storage; new LastPlayedTrack and playback-cache types; custom themes persisted as individual JSON files; APIs now accept AppHandle.
Tauri init & commands apps/desktop/src-tauri/src/lib.rs	Registers youtube_auth handlers, exposes clear_everything command, spawns YouTube token refresh on init.
Provider architecture (frontend) apps/desktop/src/providers/types.ts, apps/desktop/src/providers/factory.ts, apps/desktop/src/providers/index.ts	New MusicProvider contract, UnifiedTrack/playlist types, provider registry/factory, active-provider caching and auth checks.
Spotify & YouTube providers apps/desktop/src/providers/spotify/*, apps/desktop/src/providers/youtube/*, apps/desktop/src/providers/spotify/client.ts, apps/desktop/src/providers/youtube/client.ts	Spotify provider implementation and re-exports; full YouTube provider and API client with token caching, request dedupe, conversion to UnifiedTrack, playback/playlist support.
AI queue & music tools apps/desktop/src/lib/aiQueueService.ts, apps/desktop/src/lib/aiQueueStore.ts, apps/desktop/src/lib/musicTools.ts	Provider-agnostic AI queueing, shuffle/fallbacks, persistent played-URI history (localStorage capped), new musicTools API for AI assistance.
Playback caching & hooks apps/desktop/src/lib/settingLib.ts, apps/desktop/src/hooks/useCurrentlyPlaying.ts, apps/desktop/src/ui/spotifyClient.ts	New last-played/provider playback cache types; useCurrentlyPlaying stores per-provider last-played and debounced saves; spotifyClient GET dedupe and token cache clear, playTrack with optional position.
UI components & views multiple files (e.g., apps/desktop/src/ui/index.tsx, apps/desktop/src/ui/views/Boot.tsx, apps/desktop/src/ui/views/Settings.tsx, apps/desktop/src/ui/components/YouTubePlayer/YouTubePlayer.tsx, apps/desktop/src/ui/components/TrackControls/*, plus Playlist/Search/AddToPlaylist/Volume/AIDJ views and layout files)	Mounts YouTube player, boot flow extended for YouTube setup, provider-aware UI/data fetching, Clear Everything modal, async provider-based playback controls, keyboard seek enhancements, type and prop migrations to UnifiedTrack/provider types.
Website & minor frontend edits apps/www/components/*, apps/www/app/*, apps/www/contexts/*, apps/www/hooks/*	Import reorders, Next.js Image usage in multiple components, docs URL change, small layout/import cleanups.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Boot as Boot UI
    participant YouTube as YouTube OAuth Handler
    participant Axum as Local HTTP Server (Axum)
    participant Google as Google OAuth
    participant Keyring as OS Keyring
    participant Settings as Settings Storage

    User->>Boot: Click "Connect YouTube"
    Boot->>YouTube: start_youtube_oauth_flow()
    YouTube->>Axum: Spawn local server, register callback
    YouTube->>Google: Open consent URL (state, client_id)
    Google->>User: Show consent page
    User->>Google: Approve
    Google->>Axum: Redirect callback with code + state
    Axum->>YouTube: Deliver code
    YouTube->>Google: Exchange code for tokens
    Google->>YouTube: Return access/refresh/expires
    YouTube->>Keyring: Persist tokens & client_id
    Keyring->>YouTube: Ack
    YouTube->>Settings: Set active_music_provider = youtube
    YouTube->>Boot: Emit youtube-oauth-success
    Boot->>User: Show "Connected"

Loading

sequenceDiagram
    participant App
    participant Factory
    participant Settings
    participant Provider as Active Provider
    participant Keyring

    App->>Factory: getActiveProviderType()
    Factory->>Settings: readSettings()
    Settings-->>Factory: active_music_provider
    Factory->>Factory: return provider type
    App->>Factory: getActiveProvider()
    Factory->>Factory: return cached or construct provider
    App->>Provider: getPlaybackState()/playTrack()
    Provider->>Keyring: read token (cached/in-memory)
    Keyring-->>Provider: token
    Provider->>ExternalAPI: call (Spotify/YouTube)
    ExternalAPI-->>Provider: playback/track data
    Provider-->>App: UnifiedTrack / PlaybackState

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

• chore: update package.json and desktop scripts for authentication and… #26 — Overlaps credential clearing and clear-auth script changes (clear-auth.js and backend keyring clears).
• chore: enhance AI integration and update documentation #29 — Related changes touching AI DJ, keyring/AI key handling, and settings persistence.
• Staging #25 — Overlaps custom themes handling and custom_themes.rs changes.

Poem

🐰 I hopped through keys and tiny queues,

I taught the DJ new shuffle moves.
Tokens tucked in keyring nests,
Last-played naps and tidy nests.
One big clear — then fresh playlists, whoo! 🎶

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description is entirely a blank template with no custom content, missing all required sections including purpose, scope of changes, testing details, and related issue references.	Fill in all template sections with actual details: provide a clear description of the YouTube Music feature, specify the type of change, reference related issues, list specific changes made, document testing performed, and complete all checklist items.
Docstring Coverage	⚠️ Warning	Docstring coverage is 18.99% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Feature/add youtube music' directly describes the main change—adding YouTube Music support to the application.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6e8cdab and ee5598a.

⛔ Files ignored due to path filters (5)

• apps/desktop/src-tauri/gen/schemas/acl-manifests.json is excluded by !**/gen/**
• apps/desktop/src-tauri/gen/schemas/capabilities.json is excluded by !**/gen/**
• apps/desktop/src-tauri/gen/schemas/desktop-schema.json is excluded by !**/gen/**
• apps/desktop/src-tauri/gen/schemas/windows-schema.json is excluded by !**/gen/**
• pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

📒 Files selected for processing (46)

• apps/desktop/src-tauri/src/youtube_auth.rs
• apps/desktop/src/hooks/useCurrentlyPlaying.ts
• apps/desktop/src/lib/aiQueueService.ts
• apps/desktop/src/lib/aiQueueStore.ts
• apps/desktop/src/lib/musicTools.ts
• apps/desktop/src/lib/spotifyTools.ts
• apps/desktop/src/loader/themeLoader.ts
• apps/desktop/src/providers/factory.ts
• apps/desktop/src/providers/index.ts
• apps/desktop/src/providers/spotify/index.ts
• apps/desktop/src/providers/youtube/client.ts
• apps/desktop/src/providers/youtube/index.ts
• apps/desktop/src/ui/components/LayoutTrackInfo/TrackInfoLayout.tsx
• apps/desktop/src/ui/components/TrackControls/TrackControls.tsx
• apps/desktop/src/ui/components/YouTubePlayer/YouTubePlayer.tsx
• apps/desktop/src/ui/components/YouTubePlayer/index.ts
• apps/desktop/src/ui/global.css
• apps/desktop/src/ui/index.tsx
• apps/desktop/src/ui/layouts/LayoutB.tsx
• apps/desktop/src/ui/spotifyClient.ts
• apps/desktop/src/ui/views/AIDJView.tsx
• apps/desktop/src/ui/views/AddToPlaylistView.tsx
• apps/desktop/src/ui/views/Boot.tsx
• apps/desktop/src/ui/views/PlaylistView.tsx
• apps/desktop/src/ui/views/Settings.tsx
• apps/desktop/src/ui/views/VolumeView.tsx
• apps/www/app/download/page.tsx
• apps/www/app/layout.tsx
• apps/www/components/ContributorRow.tsx
• apps/www/components/download-section.tsx
• apps/www/components/features/FeatureSection.tsx
• apps/www/components/footer.tsx
• apps/www/components/header.tsx
• apps/www/components/hero-section.tsx
• apps/www/components/open-source-section.tsx
• apps/www/components/player/PlayerPreview.tsx
• apps/www/components/player/ThemeSwitcher.tsx
• apps/www/components/player/index.ts
• apps/www/components/player/layouts/LayoutA.tsx
• apps/www/components/player/layouts/LayoutB.tsx
• apps/www/components/player/layouts/LayoutC.tsx
• apps/www/contexts/language-context.tsx
• apps/www/hooks/use-language.tsx
• apps/www/hooks/use-theme.tsx
• apps/www/lib/cookies.ts
• biome.json

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 14

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (5)

apps/desktop/src-tauri/src/spotify_auth.rs (1)

112-129: Fix cache-keyring consistency issue.

The cache is updated at line 118 before the keyring write completes. If the keyring write fails (lines 121-126), the cache will contain the new value but persistence will have failed, causing inconsistency. On next app start, the cached value is lost and the client ID is missing from the keyring.

🔧 Proposed fix: Update cache only after successful keyring write

 #[tauri::command]
 pub async fn save_spotify_client_id(client_id: String) -> Result<(), String> {
     if client_id.trim().is_empty() {
         return Err("Client ID is empty".to_string());
     }
     let client_id_trimmed = client_id.trim().to_string();
     
-    set_cached_client_id(&client_id_trimmed);
-    
     let client_id_clone = client_id_trimmed.clone();
     tokio::task::spawn_blocking(move || {
         entry(SPOTIFY_CLIENT_ID_KEY)
             .map_err(|e| format!("Keyring error: {}", e))?
             .set_password(&client_id_clone)
             .map_err(|e| format!("Failed to save client ID: {}", e))
     })
     .await
-    .map_err(|e| format!("Task failed: {}", e))?
+    .map_err(|e| format!("Task failed: {}", e))??;
+    
+    set_cached_client_id(&client_id_trimmed);
+    Ok(())
 }

apps/desktop/src/ui/views/VolumeView.tsx (1)

24-50: Consider adding error handling for the volume loading effect.

If getActiveProvider(), readSettings(), or provider.setVolume() throws, the loading state will remain true indefinitely for the YouTube path, leaving the UI stuck on "Loading...".

🛡️ Suggested fix

   useEffect(() => {
     const loadVolume = async () => {
       setLoading(true);
+      try {
         const type = await getActiveProviderType();
         setProviderType(type);
         
         if (type === "youtube") {
           setDeviceName("YouTube Music (In-App Player)");
           const settings = await readSettings();
           const savedVolume = settings.youtube_volume ?? 50;
           setLocalVolume(savedVolume);
           // Apply saved volume to player
           const provider = await getActiveProvider();
           provider.setVolume(savedVolume);
           setLoading(false);
           return;
         }
         
         const state = await getPlayerState();
         if (state?.device) {
           setLocalVolume(state.device.volume_percent);
           setDeviceName(state.device.name);
         }
+      } catch (err) {
+        console.error("Failed to load volume:", err);
+      } finally {
         setLoading(false);
+      }
     };
     loadVolume();
   }, []);

apps/desktop/src-tauri/src/ai_keyring.rs (1)

49-69: Cache updated before keyring write succeeds—potential inconsistency.

Line 56 updates the cache before the keyring write (lines 61-68). If the keyring operation fails, the cache will contain a key that isn't actually persisted, leading to inconsistent state until restart.

🛡️ Suggested fix

 pub async fn save_ai_api_key(provider: String, api_key: String) -> Result<(), String> {
     if api_key.trim().is_empty() {
         return Err("API key is empty".to_string());
     }
     
     let api_key_trimmed = api_key.trim().to_string();
-    set_cached_ai_key(&provider, &api_key_trimmed);
     
     let key_name = get_ai_key_name(&provider);
     let api_key_clone = api_key_trimmed.clone();
+    let provider_clone = provider.clone();
     
     tokio::task::spawn_blocking(move || {
         entry(&key_name)
             .map_err(|e| format!("Keyring error: {}", e))?
             .set_password(&api_key_clone)
             .map_err(|e| format!("Failed to save AI API key: {}", e))
     })
     .await
-    .map_err(|e| format!("Task failed: {}", e))?
+    .map_err(|e| format!("Task failed: {}", e))??;
+    
+    // Update cache only after successful keyring write
+    set_cached_ai_key(&provider_clone, &api_key_trimmed);
+    Ok(())
 }

apps/desktop/src/ui/views/AddToPlaylistView.tsx (1)

56-67: Add defensive provider check in handleAddToPlaylist.

Although the UI prevents interaction when providerType === "youtube", the handler should defensively check the provider before calling addTrackToPlaylist with a Spotify-specific URI format (spotify:track:${trackId}).

🛡️ Proposed defensive check

 const handleAddToPlaylist = async (playlist: SimplifiedPlaylist) => {
-  if (!trackId || addingTo) return;
+  if (!trackId || addingTo || providerType !== "spotify") return;

   setAddingTo(playlist.id);
   try {
     await addTrackToPlaylist(playlist.id, `spotify:track:${trackId}`);
     onBack();
   } catch (err) {
     console.error("Failed to add track to playlist:", err);
     setAddingTo(null);
   }
 };

This prevents accidental invocation if the UI state and provider state become inconsistent.

apps/desktop/src/ui/components/LayoutTrackInfo/TrackInfoLayout.tsx (1)

53-53: Handle provider-aware track saving for YouTube Music and other providers.

The saveTrackToLibrary function is hardcoded to use the Spotify API and doesn't check the provider type from UnifiedTrack. When YouTube Music tracks are passed to this function, the operation will fail since YouTube track IDs are incompatible with the Spotify endpoint. Either implement a provider-aware wrapper that dispatches to the correct provider-specific save function, or gracefully handle unsupported providers by skipping the save operation for YouTube Music tracks.

🤖 Fix all issues with AI agents

In @apps/desktop/src-tauri/src/settings.rs:
- Around line 34-41: The Rust CachedTrack struct is missing the uri and provider
fields present in the TypeScript CachedTrack, causing deserialization
mismatches; update the CachedTrack struct (the struct named CachedTrack in
settings.rs) to include pub uri: String and pub provider: String (or the
appropriate type used elsewhere for provider), derive Serialize/Deserialize will
handle it, and then adjust any code that constructs CachedTrack instances to
supply these fields so settings read/write from the frontend succeeds.

In @apps/desktop/src-tauri/src/spotify_auth.rs:
- Around line 177-189: The cache is updated before the keyring write in
set_music_provider, which can leave the cache inconsistent if the keyring
persistence fails; remove the early set_cached_music_provider(&provider) call,
perform the keyring write inside the spawn_blocking task using the provider
clone (entry(MUSIC_PROVIDER_KEY)...set_password(&provider_clone)), await and map
any errors as currently done, and only after the awaited task returns Ok call
set_cached_music_provider with the original provider (or its clone) so the cache
is updated only on successful persistence.
- Around line 215-239: The cache is being updated too early in save_tokens by
calling set_cached_tokens(tokens) before the keyring writes complete, risking
cache/keyring inconsistency if any entry(...) set_password fails; move the
set_cached_tokens call to after the tokio::task::spawn_blocking completes
successfully so that only when all three keyring writes
(entry(ACCESS_TOKEN_KEY), entry(REFRESH_TOKEN_KEY), entry(TOKEN_EXPIRY_KEY) and
their set_password calls) return Ok do you call set_cached_tokens(tokens), and
ensure any errors from the spawn_blocking future (map_err("Task failed: ..."))
propagate without updating the cache.

In @apps/desktop/src/lib/aiQueueService.ts:
- Around line 440-447: The YouTube branch currently only logs added tracks and
doesn't auto-advance; implement logic in the monitor loop (or a YouTube player
event listener) that, when musicProviderType !== "spotify", obtains the active
music provider via getActiveProvider(), checks playback state (e.g.,
state.isPlaying or equivalent), and when the current track has ended and
store.queue has a next item (store.currentIndex + 1 < store.queue.length), call
the provider.playTrack(nextTrack.uri) and increment store.currentIndex; update
any relevant playback-state handling to avoid double-playing and ensure this
runs regularly in the existing monitor loop or as an event handler.

In @apps/desktop/src/ui/components/LayoutTrackInfo/TrackInfoLayout.tsx:
- Around line 9-13: The getLargestImageUrl function in TrackInfoLayout
duplicates logic already implemented in spotifyClient.getLargestImageUrl; remove
the local duplicate and import/reuse spotifyClient.getLargestImageUrl instead,
and if necessary broaden the spotifyClient.getLargestImageUrl parameter type to
accept UnifiedAlbumImage[] (or a common image type) so the import can be used
without casting; update imports in TrackInfoLayout to call
spotifyClient.getLargestImageUrl(images) and delete the local getLargestImageUrl
definition.

In @apps/desktop/src/ui/components/TrackControls/PlaybackBar.tsx:
- Around line 19-22: The seekToPosition helper currently calls
getActiveProvider() and provider.seek(ms) with no error handling; wrap the await
getActiveProvider() and provider.seek(ms) call in a try/catch inside
seekToPosition, log or surface the caught error (e.g., via console.error or the
app logger) including context like the ms value and provider info, and avoid
leaving an unhandled rejection by either returning gracefully or rethrowing a
wrapped error as appropriate for callers.

In @apps/desktop/src/ui/components/TrackControls/TrackControls.tsx:
- Around line 24-49: The handler handleToggle updates UI optimistically by
calling onTogglePlaying(next) before awaiting provider operations, risking UI
desync if provider calls (getActiveProvider, getPlaybackState, playTrack, play,
pause) fail; wrap the provider logic in try/catch, perform the provider
operations first and only call onTogglePlaying(next) after they succeed, or if
you must update optimistically, call onTogglePlaying(next) then catch errors and
call onTogglePlaying(isPlaying) (or the previous state) to revert and
surface/log the error; update handleToggle to use try/catch around
getActiveProvider/getActiveProviderType and subsequent
provider.play/playTrack/playback checks and ensure errors are handled and state
reverted via the same onTogglePlaying callback.

In @apps/docs/dockerfile:
- Line 5: Replace the non-LTS base image in the Dockerfile by updating the FROM
line: change the image tag from node:25-alpine to a Node.js LTS tag such as
node:22-alpine (or a specific 22.x LTS variant) so the builder stage uses an LTS
release rather than the Current release.

In @apps/docs/package.json:
- Line 13: The package.json currently pins "@astrojs/starlight": "^0.37.1" which
requires Astro >=5.5.0 while the project is on Astro 5.1.5; update the Astro
dependency in package.json to at least "astro": ">=5.5.0" (or run npx
@astrojs/upgrade to auto-resolve compatible versions) or alternatively downgrade
"@astrojs/starlight" to a 0.36.x/compatible version; after changing the version,
run install and test the site (pay special attention to table/element wrapping
due to Starlight 0.37.0+ overflow-wrap CSS changes) and update any affected
styles.

In @apps/www/components/header.tsx:
- Line 16: Remove the invalid "pnpm" token at the start of the variable
declaration so the constant DOCS_URL is declared as a normal JS/TS constant
using process.env.NEXT_PUBLIC_DOCS_URL (falling back to the URL) — update the
declaration for DOCS_URL in header.tsx to remove the "pnpm" prefix and ensure
the identifier DOCS_URL and environment var NEXT_PUBLIC_DOCS_URL are used
correctly.

In @package.json:
- Line 23: biome.json is still on schema v1.9.4 while package.json upgrades
Biome to v2.x; run the Biome migration and apply its changes by executing npx
@biomejs/biome migrate --write to update the schema version, convert
files.ignore to files.includes, relocate organizeImports config, and adjust glob
patterns; after running the migration, review the updated biome.json to ensure
the new schema, files.includes, organizeImports placement, and glob patterns
match project standards and update any lint/format rules as needed before
merging.

🟡 Minor comments (10)

apps/desktop/src-tauri/src/custom_themes.rs-188-199 (1)

188-199: Return value doesn't reflect actual success.

The function always returns true even if file deletions fail. This is inconsistent with other functions in this module that return Result, and could mislead callers about whether themes were actually cleared.

Suggested fix to report actual results

-pub fn clear_custom_themes() -> bool {
+pub fn clear_custom_themes() -> Result<u32, String> {
     let dir = get_custom_themes_dir();
-    
-    if let Ok(entries) = fs::read_dir(&dir) {
-        for entry in entries.flatten() {
-            if entry.path().extension().map(|e| e == "json").unwrap_or(false) {
-                let _ = fs::remove_file(entry.path());
-            }
+    let entries = fs::read_dir(&dir)
+        .map_err(|e| format!("Failed to read themes directory: {}", e))?;
+    
+    let mut deleted = 0;
+    for entry in entries.flatten() {
+        if entry.path().extension().map(|e| e == "json").unwrap_or(false) {
+            if fs::remove_file(entry.path()).is_ok() {
+                deleted += 1;
+            }
         }
     }
-    true
+    Ok(deleted)
 }

apps/docs/package.json-19-19 (1)

19-19: Use TypeScript 5.9.2 instead of 5.9.3.

TypeScript 5.8 does exist (stable release Feb 2025), so version continuity is maintained. However, the latest documented stable patch for the 5.9.x series is v5.9.2; v5.9.3 has been reported to have issues with its release notes and lacks full documentation. Update to v5.9.2 for stability.

apps/desktop/src-tauri/src/ai_keyring.rs-116-129 (1)

116-129: Cache cleared before keyring delete succeeds—potential inconsistency.

Similar to save_ai_api_key, line 118 removes from cache before the keyring operation completes. If the keyring delete fails, the key will be missing from cache but still present in keyring.

🛡️ Suggested fix

 pub async fn delete_ai_api_key(provider: String) -> Result<(), String> {
-    remove_cached_ai_key(&provider);
-    
     let key_name = get_ai_key_name(&provider);
+    let provider_clone = provider.clone();
+    
     tokio::task::spawn_blocking(move || {
         entry(&key_name)
             .map_err(|e| format!("Keyring error: {}", e))?
             .delete_password()
             .map_err(|e| format!("Failed to delete AI API key: {}", e))
     })
     .await
-    .map_err(|e| format!("Task failed: {}", e))?
+    .map_err(|e| format!("Task failed: {}", e))??;
+    
+    // Remove from cache only after successful keyring delete
+    remove_cached_ai_key(&provider_clone);
+    Ok(())
 }

apps/desktop/src-tauri/src/lib.rs-12-34 (1)

12-34: Partial clear state on error may leave inconsistent data.

The execute() function runs all clear operations, but if one fails mid-way (e.g., spotify_result? fails), earlier operations (settings, themes) have already succeeded while later ones (youtube, ai_keys) haven't been attempted. Consider either:

1. Collecting all errors and reporting them together
2. Using a transactional approach where possible
3. Documenting this behavior as intentional

🔧 Alternative: Collect all errors

 pub async fn execute() -> Result<(), String> {
     let settings_cleared = settings::clear_settings();
     let themes_cleared = custom_themes::clear_custom_themes();
     let spotify_result = spotify_auth::clear_credentials().await;
     let youtube_result = youtube_auth::clear_youtube_credentials().await;
     let ai_keys_result = ai_keyring::clear_all_ai_keys().await;

+    let mut errors = Vec::new();
+    
     if !settings_cleared {
-        return Err("Failed to clear settings".to_string());
+        errors.push("Failed to clear settings");
     }
     if !themes_cleared {
-        return Err("Failed to clear custom themes".to_string());
+        errors.push("Failed to clear custom themes");
     }
-    spotify_result?;
-    youtube_result?;
-    ai_keys_result?;
+    if let Err(e) = spotify_result {
+        errors.push(&format!("Spotify: {}", e));
+    }
+    if let Err(e) = youtube_result {
+        errors.push(&format!("YouTube: {}", e));
+    }
+    if let Err(e) = ai_keys_result {
+        errors.push(&format!("AI keys: {}", e));
+    }

-    Ok(())
+    if errors.is_empty() {
+        Ok(())
+    } else {
+        Err(errors.join("; "))
+    }
 }

apps/desktop/src/ui/components/TrackControls/TrackControls.tsx-8-8 (1)

8-8: Unused prop: currentTrackUri is defined but never referenced.

The currentTrackUri prop is added to the interface and component signature but is not used anywhere in the component logic. Either remove it or implement its intended usage.

🔧 Remove unused prop if not needed

 type TrackControlsProps = {
   isPlaying: boolean;
-  currentTrackUri?: string | null;
   onTogglePlaying?: (playing: boolean) => void;
   className?: string;
 };

-export function TrackControls({ isPlaying, currentTrackUri, onTogglePlaying, className = "" }: TrackControlsProps) {
+export function TrackControls({ isPlaying, onTogglePlaying, className = "" }: TrackControlsProps) {

apps/desktop/src/hooks/useCurrentlyPlaying.ts-66-88 (1)

66-88: Module-level mutable state can cause issues with React Fast Refresh.

saveTimeout and currentProviderCache are module-level variables that persist across component re-renders and Fast Refresh cycles. This can lead to stale state or duplicate writes.

Additionally, saveTimeout is never cleared on hook unmount, which could cause writes after the component is destroyed.

♻️ Consider moving to a ref or managing cleanup

For the timeout cleanup, at minimum ensure it's cleared when the hook unmounts:

// In useCurrentlyPlaying hook's cleanup:
useEffect(() => {
  return () => {
    if (saveTimeout) {
      clearTimeout(saveTimeout);
      saveTimeout = null;
    }
  };
}, []);

For a more robust solution, consider using useRef for the cache and timeout within the hook, or a dedicated state management solution like Zustand.

apps/desktop/src/ui/components/YouTubePlayer/YouTubePlayer.tsx-316-324 (1)

316-324: Mutating playerRef.current object can break consumer references.

This effect creates a new object and assigns it to playerRef.current, which replaces the reference. Consumers who captured the previous ref object won't see the isReady update. The ref object's properties should be mutated in place, or isReady should be handled differently.

🔧 Suggested fix - mutate in place or skip this effect

Since setupPlayerRef already sets up the ref in handleReady, consider either:

1. Mutate in place:

   useEffect(() => {
     if (playerRef?.current) {
-      const currentRef = playerRef.current;
-      playerRef.current = {
-        ...currentRef,
-        isReady: () => isPlayerReady,
-      };
+      playerRef.current.isReady = () => isPlayerReady;
     }
   }, [playerRef, isPlayerReady]);

2. Or remove this effect entirely since isReady is set in setupPlayerRef at line 211.

apps/desktop/src/lib/settingLib.ts-153-166 (1)

153-166: Fallback defaults missing provider_playback_cache and youtube_volume fields.

The fallback object doesn't include all fields from the Settings type, which may cause runtime issues or TypeScript errors.

🔧 Proposed fix

     return {
       first_boot_done: false,
       layout: "LayoutA",
       theme: "dark",
       ai_providers: [],
       active_ai_provider: null,
       active_music_provider: "spotify",
       show_ai_queue_border: true,
       discord_rpc_enabled: false,
       last_played_track: null,
+      provider_playback_cache: null,
+      youtube_volume: null,
     };

apps/desktop/src/ui/views/Settings.tsx-57-67 (1)

57-67: Type inconsistency: "apple" is not a valid MusicProviderType.

The MUSIC_PROVIDERS array includes "apple" as an ID, but MusicProviderType is defined as "spotify" | "youtube" per the relevant code snippets. This causes a type mismatch.

🔧 Suggested fix

 const MUSIC_PROVIDERS: {
-  id: MusicProviderType | "apple";
+  id: MusicProviderType | "apple"; // "apple" intentionally excluded from MusicProviderType until implemented
   name: string;
   color: string;
   iconColor: string;
   available: boolean;
 }[] = [

Alternatively, consider creating a separate type for UI display purposes that includes future providers:

type DisplayProviderType = MusicProviderType | "apple";

apps/desktop/src/providers/youtube/client.ts-78-80 (1)

78-80: Casting undefined to T may cause runtime issues.

When the response is 204 (No Content), returning undefined as unknown as T could cause issues if the caller doesn't expect undefined. Consider making the return type explicitly nullable or documenting this behavior.

🔧 Consider explicit null handling

-  if (res.status === 204) {
-    return undefined as unknown as T;
-  }
+  if (res.status === 204) {
+    // No content - caller must handle null/undefined
+    return null as unknown as T;
+  }

Or update the function signature:

async function youtubeRequest<T>(url: string, init?: RequestInit): Promise<T | null>

🧹 Nitpick comments (22)

apps/desktop/src/ui/global.css (1)

26-26: Consider documenting the high z-index value.

The z-index increase to 9999 ensures the drag area stays accessible above all content (including the new YouTube UI). However, this magic number could benefit from documentation or a CSS custom property to improve maintainability.

♻️ Suggested approach for z-index management

Consider defining z-index layers in the :root section for better maintainability:

 :root {
   --panel-bg: rgba(0, 0, 0, 0.7);
   --panel-bg-50: rgba(0, 0, 0, 0.5);
   --panel-bg-40: rgba(0, 0, 0, 0.4);
   --text-color: #ffffff;
   --text-color-90: rgba(255, 255, 255, 0.9);
   --text-color-70: rgba(255, 255, 255, 0.7);
+  --z-drag-area: 9999;
 }

Then use it in the rule:

 .drag-area {
   position: absolute;
   top: 0;
   left: 0;
   right: 0;
   height: 24px;
   cursor: move;
   pointer-events: auto;
   background: transparent;
-  z-index: 9999;
+  z-index: var(--z-drag-area);
 }

This makes it easier to maintain z-index hierarchy across the application.

apps/desktop/src-tauri/src/custom_themes.rs (1)

14-19: Consider handling edge cases in filename sanitization.

The current implementation could produce:

• Empty filenames if input is empty or contains only special characters
• Collisions when different names sanitize identically (e.g., "My Theme!" and "My-Theme-" both become "my-theme-")

Suggested hardening

 fn sanitize_filename(name: &str) -> String {
-    name.chars()
+    let sanitized: String = name.chars()
         .map(|c| if c.is_alphanumeric() || c == '-' || c == '_' { c } else { '-' })
         .collect::<String>()
-        .to_lowercase()
+        .to_lowercase();
+    
+    if sanitized.is_empty() || sanitized.chars().all(|c| c == '-') {
+        format!("theme-{}", std::time::SystemTime::now()
+            .duration_since(std::time::UNIX_EPOCH)
+            .map(|d| d.as_millis())
+            .unwrap_or(0))
+    } else {
+        sanitized.trim_matches('-').to_string()
+    }
 }

apps/desktop/src-tauri/src/spotify_auth.rs (2)

29-82: Consider logging lock poisoning failures.

The cache helpers use .ok() to silently ignore lock poisoning errors. While acceptable for non-critical caches, authentication state might benefit from logging these failures for debugging.

📝 Example: Add logging for lock failures

 fn get_cached_tokens() -> Option<SpotifyTokens> {
-    TOKENS_CACHE.lock().ok().and_then(|g| g.clone())
+    TOKENS_CACHE.lock()
+        .map_err(|e| log::warn!("Token cache lock poisoned: {}", e))
+        .ok()
+        .and_then(|g| g.clone())
 }

---

650-663: Remove unused app parameter.

The app parameter is explicitly suppressed at line 651 but never used. Consider removing it from the function signature unless it's required for future functionality.

♻️ Proposed refactor

-pub fn spawn_token_refresh_task(app: AppHandle) {
-    let _ = app;
+pub fn spawn_token_refresh_task() {
     rt::spawn(async move {
         loop {
             sleep(std::time::Duration::from_secs(300)).await;
             if let Ok(tokens) = get_tokens().await {
                 let now = Utc::now().timestamp();
                 if now + 300 >= tokens.expires_at {
                     let _ = refresh_access_token().await;
                 }
             }
         }
     });
 }

apps/desktop/src/providers/spotify/client.ts (1)

1-1: Consider the module dependency direction.

Re-exporting from ui/spotifyClient into providers/spotify/ creates a dependency from the provider layer back to the UI layer, which is architecturally unusual. Typically, UI components depend on provider abstractions, not the reverse.

If spotifyClient contains provider implementation logic (API calls, token management, etc.), consider moving it to providers/spotify/ or a shared utility module to maintain clearer layer boundaries.

apps/desktop/src/ui/views/VolumeView.tsx (1)

52-73: Consider extracting shared YouTube volume persistence logic.

Both handleVolumeChange and handlePreset contain identical YouTube volume persistence code. Consider extracting to a helper:

♻️ Suggested refactor

+  const persistVolume = useCallback(async (newVolume: number) => {
+    const provider = await getActiveProvider();
+    provider.setVolume(newVolume);
+    if (providerType === "youtube") {
+      await writeSettings({ youtube_volume: newVolume });
+    }
+  }, [providerType]);
+
   const handleVolumeChange = useCallback(async (e: React.ChangeEvent<HTMLInputElement>) => {
     const newVolume = Number(e.target.value);
     setLocalVolume(newVolume);
-    const provider = await getActiveProvider();
-    provider.setVolume(newVolume);
-    
-    // Save YouTube volume to settings
-    if (providerType === "youtube") {
-      await writeSettings({ youtube_volume: newVolume });
-    }
+    await persistVolume(newVolume);
-  }, [providerType]);
+  }, [persistVolume]);

   const handlePreset = useCallback(async (preset: number) => {
     setLocalVolume(preset);
-    const provider = await getActiveProvider();
-    provider.setVolume(preset);
-    
-    // Save YouTube volume to settings
-    if (providerType === "youtube") {
-      await writeSettings({ youtube_volume: preset });
-    }
+    await persistVolume(preset);
-  }, [providerType]);
+  }, [persistVolume]);

apps/desktop/src-tauri/src/ai_keyring.rs (2)

27-47: Silently ignoring mutex poison errors may mask bugs.

The .ok() pattern on mutex locks (lines 28, 32, 38, 44) silently ignores poison errors. While mutex poisoning is rare, silently failing could mask thread panic issues during development.

💡 Alternative approach

Consider logging when mutex acquisition fails, at minimum during debug builds:

fn get_cached_ai_key(provider: &str) -> Option<String> {
    match AI_KEY_CACHE.lock() {
        Ok(guard) => guard.get(provider).cloned(),
        Err(e) => {
            #[cfg(debug_assertions)]
            eprintln!("AI_KEY_CACHE mutex poisoned: {}", e);
            None
        }
    }
}

---

131-144: Sequential provider checks could be parallelized.

get_all_ai_providers awaits each provider check sequentially. For better performance, consider using futures::future::join_all to check all providers concurrently.

♻️ Parallel approach

#[tauri::command]
pub async fn get_all_ai_providers() -> Vec<AIProviderKeyring> {
    let providers = vec!["openai", "anthropic", "google", "groq"];
    
    let futures: Vec<_> = providers
        .iter()
        .map(|&p| async move {
            let has_key = has_ai_api_key(p.to_string()).await;
            AIProviderKeyring {
                provider: p.to_string(),
                has_key,
            }
        })
        .collect();
    
    futures::future::join_all(futures).await
}

This requires adding futures to dependencies if not already present.

apps/desktop/src-tauri/src/youtube_auth.rs (3)

340-351: Consistent mutex lock handling needed.

Similar to clear_youtube_credentials, the cancel_youtube_oauth_flow function uses .unwrap() on mutex locks. For consistency and resilience, consider using the same pattern as the cache helper functions.

Proposed fix

 pub async fn cancel_youtube_oauth_flow() -> Result<(), String> {
-    {
-        let mut s = YT_AUTH_STATE.lock().unwrap();
-        *s = None;
-    }
-    let mut shutdown = YT_OAUTH_SHUTDOWN.lock().unwrap();
-    if let Some(tx) = shutdown.take() {
-        let _ = tx.send(());
+    if let Ok(mut s) = YT_AUTH_STATE.lock() {
+        *s = None;
+    }
+    if let Ok(mut shutdown) = YT_OAUTH_SHUTDOWN.lock() {
+        if let Some(tx) = shutdown.take() {
+            let _ = tx.send(());
+        }
     }
     Ok(())
 }

---

614-627: Unused app parameter and silent refresh failures.

The app parameter is explicitly discarded with let _ = app; but could be useful for emitting events when token refresh fails. Silent failures may leave users unaware that their session has expired.

Proposed enhancement

 pub fn spawn_youtube_token_refresh_task(app: AppHandle) {
-    let _ = app;
     rt::spawn(async move {
         loop {
             sleep(std::time::Duration::from_secs(300)).await;
             if let Ok(tokens) = get_youtube_tokens().await {
                 let now = Utc::now().timestamp();
                 if now + 300 >= tokens.expires_at {
-                    let _ = refresh_youtube_access_token().await;
+                    if let Err(e) = refresh_youtube_access_token().await {
+                        let _ = app.emit("youtube-token-refresh-failed", serde_json::json!({ "error": e }));
+                    }
                 }
             }
         }
     });
 }

---

133-143: Duplicate functions: has_youtube_client_id and has_youtube_credentials.

These two Tauri commands have identical implementations. Consider removing one or clarifying if they should have different semantics.

.changeset/clear-everything-button.md (1)

5-9: Consider mentioning YouTube tokens in the changeset.

The description lists "Spotify tokens" but the PR also adds YouTube credential clearing. For completeness, consider updating to mention both providers or use a more generic term like "music provider tokens".

apps/desktop/src/ui/components/TrackControls/PlaybackBar.tsx (1)

19-22: Consider caching the active provider to reduce async calls.

During active dragging or rapid keyboard navigation, getActiveProvider() is called multiple times. Since the active provider typically doesn't change mid-session, consider calling it once on mount and caching it, or implementing request deduplication.

♻️ Optional optimization with useMemo

+import { useCallback, useEffect, useMemo, useRef, useState } from "react";
+import { getActiveProvider } from "../../../providers";
+import type { MusicProvider } from "../../../providers/types";
+
 type PlaybackBarProps = {
   durationMs: number;
   progressMs: number | null;
   isPlaying: boolean;
   onSeek?: (ms: number) => void;
   className?: string;
 };

 function msToTime(ms: number): string {
   const total = Math.max(0, Math.floor(ms / 1000));
   const m = Math.floor(total / 60);
   const s = total % 60;
   return `${m}:${s.toString().padStart(2, "0")}`;
 }

-async function seekToPosition(ms: number): Promise<void> {
-  const provider = await getActiveProvider();
-  provider.seek(ms);
-}
-
 export function PlaybackBar({
   durationMs,
   progressMs,
   isPlaying,
   onSeek,
   className = "",
 }: PlaybackBarProps) {
   const [localProgress, setLocalProgress] = useState(progressMs ?? 0);
+  const [provider, setProvider] = useState<MusicProvider | null>(null);
   const lastTick = useRef<number | null>(null);
   const dragging = useRef(false);

+  useEffect(() => {
+    getActiveProvider().then(setProvider).catch(console.error);
+  }, []);
+
+  const seekToPosition = useCallback(
+    async (ms: number): Promise<void> => {
+      if (!provider) return;
+      try {
+        await provider.seek(ms);
+      } catch (error) {
+        console.error("Failed to seek:", error);
+      }
+    },
+    [provider]
+  );

This approach caches the provider and makes seekToPosition a stable callback.

Also applies to: 74-74, 112-112, 117-117

apps/desktop/src/lib/aiQueueStore.ts (1)

12-23: Consider logging localStorage errors in development.

The empty catch blocks silently swallow parse errors. While this is acceptable for production resilience, consider adding a console.warn in development to aid debugging when localStorage data becomes corrupted.

🔧 Optional: Add development logging

 function loadPlayedUris(): Set<string> {
   try {
     const stored = localStorage.getItem(PLAYED_URIS_STORAGE_KEY);
     if (stored) {
       const arr = JSON.parse(stored) as string[];
       return new Set(arr.slice(-MAX_PLAYED_URIS));
     }
-  } catch {
-    // Ignore parse errors
+  } catch (err) {
+    if (import.meta.env.DEV) {
+      console.warn("Failed to load played URIs from localStorage:", err);
+    }
   }
   return new Set<string>();
 }

apps/desktop/src/providers/spotify/index.ts (1)

140-164: Consider caching user profile for playlist operations.

fetchUserProfile() is called on every getUserPlaylists invocation. If this method is called frequently (e.g., during pagination), consider caching the user ID to avoid redundant API calls.

apps/desktop/src-tauri/src/settings.rs (1)

5-12: Platform-specific path resolution limits cross-platform compatibility.

APPDATA is a Windows-only environment variable. On macOS and Linux, this will fall back to "." (current directory), which is likely incorrect and could cause data loss or permission issues.

Consider using a cross-platform solution like the dirs crate or Tauri's built-in app_data_dir:

♻️ Suggested cross-platform approach

+use tauri::api::path::app_data_dir;
+// Or use the dirs crate:
+// use dirs::config_dir;

 fn get_settings_path() -> PathBuf {
-    let app_data = std::env::var("APPDATA").unwrap_or_else(|_| ".".to_string());
-    let mut path = PathBuf::from(app_data);
-    path.push("MiniFy");
+    let mut path = dirs::config_dir()
+        .unwrap_or_else(|| PathBuf::from("."));
+    path.push("MiniFy");
     fs::create_dir_all(&path).ok();
     path.push("settings.json");
     path
 }

apps/desktop/src/lib/settingLib.ts (1)

4-4: Duplicate MusicProviderType definition.

This type is also defined in apps/desktop/src/providers/types.ts (line 1). Having duplicate definitions can lead to inconsistencies if one is updated but not the other.

Consider re-exporting from a single source of truth:

♻️ Suggested consolidation

-export type MusicProviderType = "spotify" | "youtube";
+export type { MusicProviderType } from "../providers/types";

apps/desktop/src/ui/components/YouTubePlayer/YouTubePlayer.tsx (1)

123-149: Script load failures are not handled.

The loadYouTubeAPI function creates a Promise that never rejects. If the script fails to load (network error, blocked by CSP, etc.), the promise will hang indefinitely, and initPlayer will never complete.

♻️ Add error handling for script load

   apiLoadPromise = new Promise((resolve, reject) => {
     if (window.YT && window.YT.Player) {
       resolve();
       return;
     }

     const existingScript = document.getElementById("youtube-iframe-api");
     if (existingScript) {
       window.onYouTubeIframeAPIReady = () => resolve();
       return;
     }

     const script = document.createElement("script");
     script.id = "youtube-iframe-api";
     script.src = "https://www.youtube.com/iframe_api";
     script.async = true;

     window.onYouTubeIframeAPIReady = () => resolve();
+    script.onerror = () => reject(new Error("Failed to load YouTube IFrame API"));

     document.head.appendChild(script);
   });

apps/desktop/src/ui/views/Boot.tsx (1)

7-7: Local MusicProvider type includes "apple" which isn't in MusicProviderType.

The local type "spotify" | "apple" | "youtube" | null includes "apple", but the canonical MusicProviderType in providers/types.ts only has "spotify" | "youtube". This inconsistency could cause type issues when the selected provider is passed to other typed functions.

♻️ Consider using the canonical type

+import type { MusicProviderType } from "../../providers/types";

-type MusicProvider = "spotify" | "apple" | "youtube" | null;
+type MusicProvider = MusicProviderType | "apple" | null;

Or document that "apple" is intentionally included for UI purposes even though it's not yet supported.

apps/desktop/src/ui/views/PlaylistView.tsx (1)

202-215: Consider adding accessibility attributes to the toggle.

The toggle button lacks accessibility attributes for screen readers. Consider adding role="switch" and aria-checked.

♿ Accessibility improvement

             <button
               type="button"
               onClick={() => setShowOnlyOwn(!showOnlyOwn)}
+              role="switch"
+              aria-checked={showOnlyOwn}
+              aria-label="Only my playlists"
               className={`relative w-9 h-5 rounded-full transition-colors duration-200 cursor-pointer ${
                 showOnlyOwn ? "bg-[--settings-accent]" : "bg-white/20"
               }`}
             >

apps/desktop/src/ui/views/Settings.tsx (2)

1340-1342: Empty onKeyDown handler and missing keyboard accessibility.

The backdrop div has an empty onKeyDown handler which appears to be a placeholder. Consider adding Escape key support for closing the dialog.

⌨️ Add keyboard support

           <div
             className="absolute inset-0 bg-black/60 backdrop-blur-sm"
             onClick={() => !clearingData && setShowClearDialog(false)}
-            onKeyDown={() => {}}
+            onKeyDown={(e) => {
+              if (e.key === "Escape" && !clearingData) {
+                setShowClearDialog(false);
+              }
+            }}
+            tabIndex={-1}
+            role="presentation"
           />

---

363-374: Consider adding user feedback on successful clear.

The function silently closes the dialog on success without explicit user feedback. Consider adding a brief success toast or notification before the dialog closes.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's see the full context of the discord_rpc.rs file around line 30
cat -n apps/desktop/src-tauri/src/discord_rpc.rs | head -50

Repository: ModioStudio/MiniFy

Length of output: 1636

---

🌐 Web query:

discord-rich-presence 1.0 Rust crate DiscordIpcClient::new API

💡 Result:

DiscordIpcClient::new signature (discord-rich-presence 1.0.0):

pub fn new(client_id: &str) -> Self

Creates a new DiscordIpcClient with the given client ID. [1][2]

Sources:

• DiscordIpcClient docs (discord-rich-presence 1.0.0). [1]
• Crate overview / example showing usage. [2]

References:
[1] https://docs.rs/discord-rich-presence/latest/discord_rich_presence/struct.DiscordIpcClient.html
[2] https://docs.rs/discord-rich-presence/latest/discord_rich_presence/

---

Store the Discord Application ID in the OS keychain using keyring per coding guidelines.

The code at line 6 hardcodes DISCORD_APPLICATION_ID as a const string. Per project guidelines for files in apps/desktop/src-tauri/src/**/*.rs, secrets and tokens must be stored in the OS keychain via the keyring crate, not hardcoded in source code.

Note: The API compatibility observation is correct—DiscordIpcClient::new() in discord-rich-presence 1.0 returns a concrete type rather than a Result, so removing error handling at line 30 is appropriate.

[coderabbitai]

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

apps/desktop/src-tauri/src/custom_themes.rs (1)

40-54: Missing headerText field in SettingsConfig.

The TypeScript CustomTheme.settings type (in settingLib.ts lines 96-138) includes a headerText field that is missing from the Rust struct. This could cause the field to be silently dropped during serialization/deserialization.

Proposed fix

 #[derive(Serialize, Deserialize, Debug, Clone)]
 pub struct SettingsConfig {
     #[serde(rename = "panelBg")]
     pub panel_bg: Option<String>,
     #[serde(rename = "panelBorder")]
     pub panel_border: Option<String>,
     pub text: Option<String>,
     #[serde(rename = "textMuted")]
     pub text_muted: Option<String>,
+    #[serde(rename = "headerText")]
+    pub header_text: Option<String>,
     #[serde(rename = "itemHover")]
     pub item_hover: Option<String>,
     #[serde(rename = "itemActive")]
     pub item_active: Option<String>,
     pub accent: Option<String>,
 }

apps/desktop/src-tauri/src/spotify_auth.rs (1)

112-129: Cache-keyring consistency issue in save_spotify_client_id.

The cache is updated at line 118 before the keyring write completes (lines 121-128). If the keyring write fails, the cache will contain a value that isn't persisted, leading to inconsistency on app restart.

Proposed fix

 #[tauri::command]
 pub async fn save_spotify_client_id(client_id: String) -> Result<(), String> {
     if client_id.trim().is_empty() {
         return Err("Client ID is empty".to_string());
     }
     let client_id_trimmed = client_id.trim().to_string();
-    
-    set_cached_client_id(&client_id_trimmed);
-    
     let client_id_clone = client_id_trimmed.clone();
+
     tokio::task::spawn_blocking(move || {
         entry(SPOTIFY_CLIENT_ID_KEY)
             .map_err(|e| format!("Keyring error: {}", e))?
             .set_password(&client_id_clone)
             .map_err(|e| format!("Failed to save client ID: {}", e))
     })
     .await
-    .map_err(|e| format!("Task failed: {}", e))?
+    .map_err(|e| format!("Task failed: {}", e))??;
+
+    set_cached_client_id(&client_id_trimmed);
+    Ok(())
 }

apps/desktop/src/lib/aiQueueService.ts (1)

94-102: String split uses wrong delimiter.

Line 94 joins artists with space (" "), but line 97 splits by comma (,). This means trackArtists.split(",")[0] returns the entire string rather than extracting the first artist.

🔧 Suggested fix

-      const trackArtists = track.artists.map(a => a.name.toLowerCase()).join(" ");
+      const trackArtists = track.artists.map(a => a.name.toLowerCase());
       const trackTitle = track.name.toLowerCase();
       
-      if (trackArtists.includes(artistLower) || artistLower.includes(trackArtists.split(",")[0])) {
+      const artistsStr = trackArtists.join(" ");
+      if (artistsStr.includes(artistLower) || artistLower.includes(trackArtists[0] || "")) {

🤖 Fix all issues with AI agents

In @apps/desktop/src-tauri/src/youtube_auth.rs:
- Around line 201-225: The cache is being updated before the keyring writes
finish in save_youtube_tokens; move the call to set_cached_yt_tokens so it runs
after the spawn_blocking completes successfully (i.e., after the .await and
map_err checks), mirroring the spotify_auth::save_tokens pattern: perform the
entry(...).set_password(...) writes for YT_ACCESS_TOKEN_KEY,
YT_REFRESH_TOKEN_KEY, and YT_TOKEN_EXPIRY_KEY inside the spawned blocking task,
await and propagate errors, then call set_cached_yt_tokens(tokens) only on
success.
- Around line 145-175: The cache is being updated before the keyring writes
complete in save_youtube_credentials, causing possible inconsistency if the
keyring write fails; fix this by performing both keyring writes inside the
tokio::task::spawn_blocking closure (using YT_CLIENT_ID_KEY and
YT_CLIENT_SECRET_KEY) and only after that closure returns Ok(()) call
set_cached_yt_client_id and set_cached_yt_client_secret with the trimmed values
(use the existing client_id_trimmed/client_secret_trimmed or their clones),
propagating any keyring errors out so the cache is only updated on successful
persistence.

In @apps/desktop/src/lib/aiQueueService.ts:
- Around line 419-432: The auto-advance is firing on pause and when lastTrackUri
isn't in the queue; change the guard to detect actual end-of-track using
playback timing instead of just !isPlaying and ensure the last track exists in
the queue. Specifically, replace the `if (!isPlaying && lastTrackUri &&
store.queue.length > 0)` check with a check that uses `PlaybackState.progressMs`
and the corresponding `UnifiedTrack.durationMs` for the track at `lastIndex`
(obtained via `store.queue.findIndex((t) => t.uri === lastTrackUri)`), require
`lastIndex !== -1`, and only advance when progressMs is at or beyond durationMs
(optionally with a small tolerance). Keep the existing calls to
`musicProvider.playTrack`, `store.setCurrentIndex`, and `store.addPlayedUri` but
only run them under this strict "ended" condition.

In @biome.json:
- Line 34: The "organizeImports" value under assist.actions.source is
incorrectly a string; change it to an object with a level property (i.e.,
replace "assist.actions.source.organizeImports": "on" with an object like {
"level": "on" }) and optionally add an "options" object (e.g., groups,
identifierOrder) per Biome v2 schema so the key becomes
assist.actions.source.organizeImports with a level field instead of a bare
string.

🧹 Nitpick comments (5)

apps/desktop/src-tauri/src/custom_themes.rs (1)

187-198: clear_custom_themes silently ignores deletion failures.

The function always returns true even if individual file deletions fail (line 193). Consider returning false or logging if any deletions fail for better observability.

Proposed improvement

 pub fn clear_custom_themes(app: &AppHandle) -> bool {
     let dir = get_custom_themes_dir(app);
-    
+    let mut all_succeeded = true;
+
     if let Ok(entries) = fs::read_dir(&dir) {
         for entry in entries.flatten() {
             if entry.path().extension().map(|e| e == "json").unwrap_or(false) {
-                let _ = fs::remove_file(entry.path());
+                if fs::remove_file(entry.path()).is_err() {
+                    all_succeeded = false;
+                }
             }
         }
     }
-    true
+    all_succeeded
 }

apps/desktop/src-tauri/src/lib.rs (1)

12-35: clear_all::execute runs sequentially but reports first failure only.

The function executes all clear operations but checks for failures sequentially. If settings_cleared fails, the function returns early without reporting whether other operations succeeded or failed. The async operations (spotify_result, youtube_result, ai_keys_result) are awaited sequentially, not concurrently.

Consider using tokio::join! for concurrent execution if performance matters, and collecting all errors for better diagnostics.

Concurrent execution with aggregated errors

 pub async fn execute(app: &AppHandle) -> Result<(), String> {
     let settings_cleared = settings::clear_settings(app.clone());
     let themes_cleared = custom_themes::clear_custom_themes(app);
-    let spotify_result = spotify_auth::clear_credentials().await;
-    let youtube_result = youtube_auth::clear_youtube_credentials().await;
-    let ai_keys_result = ai_keyring::clear_all_ai_keys().await;
+    
+    let (spotify_result, youtube_result, ai_keys_result) = tokio::join!(
+        spotify_auth::clear_credentials(),
+        youtube_auth::clear_youtube_credentials(),
+        ai_keyring::clear_all_ai_keys()
+    );

     if !settings_cleared {
         return Err("Failed to clear settings".to_string());

apps/desktop/src-tauri/src/youtube_auth.rs (2)

133-143: Duplicate implementations of has_youtube_client_id and has_youtube_credentials.

These two functions have identical implementations. Consider removing one or differentiating their purposes. If they're meant to be identical, one could delegate to the other.

Proposed consolidation

 #[tauri::command]
 pub async fn has_youtube_client_id() -> bool {
-    has_embedded_youtube_credentials() || 
-    (get_stored_youtube_client_id().await.is_some() && get_stored_youtube_client_secret().await.is_some())
+    has_youtube_credentials().await
 }

 #[tauri::command]
 pub async fn has_youtube_credentials() -> bool {
     has_embedded_youtube_credentials() || 
     (get_stored_youtube_client_id().await.is_some() && get_stored_youtube_client_secret().await.is_some())
 }

---

528-535: Duplicate escape_html function.

This function is identical to the one in spotify_auth.rs (lines 572-579). Consider extracting to a shared utility module to reduce duplication.

apps/desktop/src/lib/aiQueueService.ts (1)

210-220: Non-Spotify provider uses placeholder artist data.

The hardcoded "Various Artists" for non-Spotify providers limits AI suggestion quality since it lacks real genre/artist context. This is acceptable for initial YouTube support, but consider tracking this as technical debt.

Would you like me to open an issue to track implementing proper artist data fetching for YouTube (e.g., via channel subscriptions or liked videos analysis)?

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e875dd6 and 6e8cdab.

📒 Files selected for processing (11)

• apps/desktop/src-tauri/src/custom_themes.rs
• apps/desktop/src-tauri/src/lib.rs
• apps/desktop/src-tauri/src/settings.rs
• apps/desktop/src-tauri/src/spotify_auth.rs
• apps/desktop/src-tauri/src/youtube_auth.rs
• apps/desktop/src/lib/aiQueueService.ts
• apps/desktop/src/ui/components/TrackControls/PlaybackBar.tsx
• apps/desktop/src/ui/components/TrackControls/TrackControls.tsx
• apps/docs/package.json
• apps/www/components/header.tsx
• biome.json

🚧 Files skipped from review as they are similar to previous changes (2)

• apps/desktop/src/ui/components/TrackControls/PlaybackBar.tsx
• apps/desktop/src/ui/components/TrackControls/TrackControls.tsx

🧰 Additional context used

📓 Path-based instructions (8)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/020-coding-style.mdc)

**/*.{ts,tsx}: NEVER use any types - Always provide proper type definitions in TypeScript
Use unknown instead of any when the type is truly unknown in TypeScript
Use interfaces for object shapes that will be extended in TypeScript
Use type aliases for complex types and unions in TypeScript
Use the latest TypeScript features appropriately
Document public APIs and interfaces in code

Use TypeScript strict types and avoid any type

Files:

• apps/www/components/header.tsx
• apps/desktop/src/lib/aiQueueService.ts

**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/020-coding-style.mdc)

**/*.{ts,tsx,js,jsx}: Avoid dynamic imports - use static imports at the top of files for better performance and code clarity
Write self-documenting code with meaningful names instead of extra comments
Avoid obvious comments that just repeat what the code does
Add comments only for complex logic, business rules, or non-obvious decisions
Use meaningful variable and function names instead of explanatory comments
Always use static imports at the top of files instead of dynamic imports
Import all dependencies at the file beginning for better bundling and performance
Use tree-shaking friendly named imports when possible
Use streaming APIs instead of loading large files into memory
Avoid buffer accumulation for large data processing
Implement proper cleanup for temporary files and streams
Set appropriate file size limits based on available memory
Maintain consistency with existing code style when modifying files
Keep code DRY (Don't Repeat Yourself)
Prefer verbose variable names and maintainability over concise code
Optimize for memory efficiency in data processing applications

Place static imports at the top of files

Files:

• apps/www/components/header.tsx
• apps/desktop/src/lib/aiQueueService.ts

**/*.{tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/020-coding-style.mdc)

**/*.{tsx,jsx}: Use functional components with hooks in React
Use named exports for React components
Keep React components small and focused on a single responsibility
Use proper prop typing in React components
Use Tailwind CSS for styling in React applications
Follow component-based styling practices with Tailwind CSS
Use Tailwind CSS utility classes for one-off styling needs
Extract common styling patterns to shared components in React

Files:

• apps/www/components/header.tsx

apps/desktop/src-tauri/src/settings.rs

📄 CodeRabbit inference engine (.cursor/rules/030-codebase-structure.mdc)

apps/desktop/src-tauri/src/settings.rs: Settings module implementation should be located at apps/desktop/src-tauri/src/settings.rs
Persist app settings under platform config directory as JSON

Files:

• apps/desktop/src-tauri/src/settings.rs

apps/desktop/src-tauri/src/**/*.rs

📄 CodeRabbit inference engine (.cursor/rules/030-codebase-structure.mdc)

Store secrets and tokens in OS keychain via keyring

Files:

• apps/desktop/src-tauri/src/settings.rs
• apps/desktop/src-tauri/src/spotify_auth.rs
• apps/desktop/src-tauri/src/youtube_auth.rs
• apps/desktop/src-tauri/src/lib.rs
• apps/desktop/src-tauri/src/custom_themes.rs

apps/desktop/src-tauri/src/spotify_auth.rs

📄 CodeRabbit inference engine (.cursor/rules/030-codebase-structure.mdc)

Spotify authentication module should be located at apps/desktop/src-tauri/src/spotify_auth.rs

Files:

• apps/desktop/src-tauri/src/spotify_auth.rs

apps/desktop/src/**/*.{tsx,ts}

📄 CodeRabbit inference engine (.cursor/rules/030-codebase-structure.mdc)

Keep React components focused with typed props

Files:

• apps/desktop/src/lib/aiQueueService.ts

apps/desktop/src-tauri/src/lib.rs

📄 CodeRabbit inference engine (.cursor/rules/030-codebase-structure.mdc)

Tauri library crate should be defined in apps/desktop/src-tauri/src/lib.rs

Files:

• apps/desktop/src-tauri/src/lib.rs

🧠 Learnings (11)

📚 Learning: 2025-12-29T21:55:52.426Z

Learnt from: CR
Repo: ModioStudio/MiniFy PR: 0
File: .cursor/rules/020-coding-style.mdc:0-0
Timestamp: 2025-12-29T21:55:52.426Z
Learning: Applies to **/*.{ts,tsx} : Document public APIs and interfaces in code

Applied to files:

• apps/www/components/header.tsx

📚 Learning: 2025-12-29T21:55:52.426Z

Learnt from: CR
Repo: ModioStudio/MiniFy PR: 0
File: .cursor/rules/020-coding-style.mdc:0-0
Timestamp: 2025-12-29T21:55:52.426Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use meaningful variable and function names instead of explanatory comments

Applied to files:

• apps/www/components/header.tsx

📚 Learning: 2025-12-29T21:56:02.730Z

Learnt from: CR
Repo: ModioStudio/MiniFy PR: 0
File: .cursor/rules/030-codebase-structure.mdc:0-0
Timestamp: 2025-12-29T21:56:02.730Z
Learning: Applies to apps/desktop/src-tauri/src/settings.rs : Settings module implementation should be located at `apps/desktop/src-tauri/src/settings.rs`

Applied to files:

• apps/desktop/src-tauri/src/settings.rs
• apps/desktop/src-tauri/src/custom_themes.rs

📚 Learning: 2025-12-29T21:56:02.730Z

Learnt from: CR
Repo: ModioStudio/MiniFy PR: 0
File: .cursor/rules/030-codebase-structure.mdc:0-0
Timestamp: 2025-12-29T21:56:02.730Z
Learning: Applies to apps/desktop/src-tauri/src/settings.rs : Persist app settings under platform config directory as JSON

Applied to files:

• apps/desktop/src-tauri/src/settings.rs
• apps/desktop/src-tauri/src/custom_themes.rs

📚 Learning: 2025-12-29T21:56:02.730Z

Learnt from: CR
Repo: ModioStudio/MiniFy PR: 0
File: .cursor/rules/030-codebase-structure.mdc:0-0
Timestamp: 2025-12-29T21:56:02.730Z
Learning: Applies to apps/desktop/src-tauri/src/bin/clear.rs : CLI utility for clearing settings should be located at `apps/desktop/src-tauri/src/bin/clear.rs`

Applied to files:

• apps/desktop/src-tauri/src/settings.rs
• apps/desktop/src-tauri/src/spotify_auth.rs
• apps/desktop/src-tauri/src/lib.rs
• apps/desktop/src-tauri/src/custom_themes.rs

📚 Learning: 2025-12-29T21:56:02.730Z

Learnt from: CR
Repo: ModioStudio/MiniFy PR: 0
File: .cursor/rules/030-codebase-structure.mdc:0-0
Timestamp: 2025-12-29T21:56:02.730Z
Learning: Applies to apps/desktop/src-tauri/src/spotify_auth.rs : Spotify authentication module should be located at `apps/desktop/src-tauri/src/spotify_auth.rs`

Applied to files:

• apps/desktop/src-tauri/src/settings.rs
• apps/desktop/src-tauri/src/spotify_auth.rs
• apps/desktop/src-tauri/src/youtube_auth.rs
• apps/desktop/src-tauri/src/lib.rs

📚 Learning: 2025-12-29T21:55:52.426Z

Learnt from: CR
Repo: ModioStudio/MiniFy PR: 0
File: .cursor/rules/020-coding-style.mdc:0-0
Timestamp: 2025-12-29T21:55:52.426Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Maintain consistency with existing code style when modifying files

Applied to files:

• biome.json

📚 Learning: 2025-12-29T21:56:02.730Z

Learnt from: CR
Repo: ModioStudio/MiniFy PR: 0
File: .cursor/rules/030-codebase-structure.mdc:0-0
Timestamp: 2025-12-29T21:56:02.730Z
Learning: Applies to apps/desktop/src-tauri/src/**/*.rs : Store secrets and tokens in OS keychain via `keyring`

Applied to files:

• apps/desktop/src-tauri/src/spotify_auth.rs
• apps/desktop/src-tauri/src/youtube_auth.rs
• apps/desktop/src-tauri/src/lib.rs
• apps/desktop/src-tauri/src/custom_themes.rs

📚 Learning: 2025-12-29T21:56:02.730Z

Learnt from: CR
Repo: ModioStudio/MiniFy PR: 0
File: .cursor/rules/030-codebase-structure.mdc:0-0
Timestamp: 2025-12-29T21:56:02.730Z
Learning: Applies to apps/desktop/src-tauri/src/lib.rs : Tauri library crate should be defined in `apps/desktop/src-tauri/src/lib.rs`

Applied to files:

• apps/desktop/src-tauri/src/youtube_auth.rs
• apps/desktop/src-tauri/src/lib.rs
• apps/desktop/src-tauri/src/custom_themes.rs

📚 Learning: 2025-12-29T21:56:02.730Z

Learnt from: CR
Repo: ModioStudio/MiniFy PR: 0
File: .cursor/rules/030-codebase-structure.mdc:0-0
Timestamp: 2025-12-29T21:56:02.730Z
Learning: Applies to apps/desktop/src-tauri/src/main.rs : Tauri backend entry point should be at `apps/desktop/src-tauri/src/main.rs`

Applied to files:

• apps/desktop/src-tauri/src/lib.rs
• apps/desktop/src-tauri/src/custom_themes.rs

📚 Learning: 2025-12-29T21:56:02.730Z

Learnt from: CR
Repo: ModioStudio/MiniFy PR: 0
File: .cursor/rules/030-codebase-structure.mdc:0-0
Timestamp: 2025-12-29T21:56:02.730Z
Learning: Applies to apps/desktop/src/themes/*.json : Theme files should be stored as JSON under `apps/desktop/src/themes/*.json`

Applied to files:

• apps/desktop/src-tauri/src/custom_themes.rs

🧬 Code graph analysis (4)

apps/desktop/src-tauri/src/spotify_auth.rs (2)

apps/desktop/src-tauri/src/youtube_auth.rs (1)

• entry (91-93)

apps/desktop/src-tauri/src/ai_keyring.rs (1)

• entry (23-25)

apps/desktop/src-tauri/src/youtube_auth.rs (1)

apps/desktop/src-tauri/src/spotify_auth.rs (6)

• tokio (421-421)
• tokio (422-422)
• tokio (432-432)
• entry (83-85)
• expires_at_str (279-280)
• escape_html (572-579)

apps/desktop/src-tauri/src/lib.rs (6)

apps/desktop/src-tauri/src/settings.rs (1)

• clear_settings (138-146)

apps/desktop/src-tauri/src/custom_themes.rs (1)

• clear_custom_themes (187-198)

apps/desktop/src-tauri/src/spotify_auth.rs (1)

• clear_credentials (301-324)

apps/desktop/src-tauri/src/youtube_auth.rs (10)

• has_youtube_credentials (140-143)
• has_youtube_client_id (134-137)
• save_youtube_credentials (146-175)
• needs_youtube_setup (178-180)
• get_youtube_tokens (228-257)
• has_valid_youtube_tokens (260-265)
• start_youtube_oauth_flow (352-460)
• cancel_youtube_oauth_flow (339-349)
• refresh_youtube_access_token (553-603)
• spawn_youtube_token_refresh_task (605-618)

apps/desktop/src-tauri/src/ai_keyring.rs (1)

• clear_all_ai_keys (147-163)

apps/desktop/src-tauri/src/discord_rpc.rs (1)

• is_discord_rpc_enabled (195-201)

apps/desktop/src-tauri/src/custom_themes.rs (1)

apps/desktop/src/lib/settingLib.ts (1)

• CustomTheme (97-139)

🔇 Additional comments (26)

apps/www/components/header.tsx (1)

16-16: LGTM! Previous syntax error resolved.

The DOCS_URL constant declaration is now syntactically correct with no "pnpm" prefix. The URL update to "https://minify-docs.modio.studio" is a straightforward configuration change that aligns with the project's documentation domain structure.

biome.json (2)

10-26: The configuration correctly uses the Biome v2 format. The files.includes with negated patterns (preceded by "**") is the recommended approach for Biome v2. The negated patterns properly exclude the intended paths, and this structure achieves the equivalent behavior of the previous ignore array.

---

2-2: Schema upgrade to v2.3.11 is appropriate—this is the current stable version.

The upgrade aligns with the official Biome v2 configuration migration, including the files.ignore → files.includes conversion and organizeImports restructuring.

apps/docs/package.json (3)

13-14: LGTM! Compatibility issue resolved.

The Astro upgrade to 5.5.0 correctly addresses the previous compatibility concern. Starlight 0.37.1 now has the minimum required Astro version (5.5+).

Remember to test the docs site after upgrading, particularly checking table and element wrapping behavior due to Starlight 0.37.0+ CSS changes to overflow-wrap.

---

19-19: No action required—TypeScript 5.9.3 is valid.

TypeScript 5.9.3 is the latest stable release (published September 30, 2025) and exists in the npm registry. The version specification is correct.

Likely an incorrect or invalid review comment.

---

15-15: No issues found with sharp@0.34.5 update.

The version exists, is current, and is compatible with Node 22 (package requires >=21.0.0). No known security vulnerabilities.

apps/desktop/src-tauri/src/custom_themes.rs (4)

6-11: Cross-platform path handling is now correctly implemented.

The migration from APPDATA to app.path().app_data_dir() addresses cross-platform compatibility. The fallback to PathBuf::from(".") is a reasonable last resort, though it would be better to propagate the error.

---

13-18: Good sanitization for filesystem safety.

The function correctly prevents directory traversal and special character issues in filenames.

---

112-125: LGTM!

The function correctly validates JSON, sanitizes the filename, and persists to the app-scoped themes directory.

---

127-149: LGTM!

The function gracefully handles directory read failures and invalid JSON files, returning only valid themes.

apps/desktop/src-tauri/src/lib.rs (1)

83-96: LGTM!

YouTube auth handlers are properly registered and the token refresh task is correctly spawned during app setup, mirroring the Spotify pattern.

apps/desktop/src-tauri/src/spotify_auth.rs (5)

176-190: LGTM!

The cache-keyring consistency issue from the previous review has been addressed. The cache is now updated only after successful keyring persistence.

---

216-241: LGTM!

The cache-keyring consistency issue from the previous review has been addressed. Tokens are cached only after all three keyring writes succeed.

---

300-324: LGTM!

The function uses safe lock handling with .lock().ok() and properly clears both caches and keyring entries. Best-effort deletion is appropriate for a cleanup operation.

---

388-496: LGTM!

The OAuth flow implementation is well-structured with proper state nonce validation, safe lock handling, and a 5-minute timeout to prevent hung flows.

---

643-656: LGTM!

The token refresh task proactively refreshes tokens when within 5 minutes of expiry, ensuring uninterrupted API access.

apps/desktop/src-tauri/src/settings.rs (2)

6-11: LGTM!

Settings path correctly uses Tauri's app data directory, following the coding guideline to persist settings under the platform config directory.

---

102-135: LGTM!

Settings operations handle errors gracefully with appropriate defaults and logging. Pretty-printed JSON aids debugging.

apps/desktop/src-tauri/src/youtube_auth.rs (3)

1-25: Good structure mirroring Spotify auth module.

The module correctly uses OS keyring for credential storage per coding guidelines, with in-memory caching for performance. Based on learnings, secrets and tokens should be stored in OS keychain via keyring.

---

351-460: LGTM!

The YouTube OAuth flow correctly mirrors the Spotify implementation with appropriate port separation (3001 vs 3000), state nonce validation, and timeout handling.

---

605-618: LGTM!

The token refresh task follows the same pattern as Spotify, proactively refreshing tokens when within 5 minutes of expiry.

apps/desktop/src/lib/aiQueueService.ts (5)

6-25: LGTM - Clean import aliasing for multi-provider support.

The Spotify-prefixed aliases prevent naming collisions and make the provider-specific calls explicit throughout the file.

---

48-55: LGTM - Correct Fisher-Yates shuffle implementation.

Non-mutating approach with proper generic typing.

---

67-75: LGTM - Unified track formatting aligns with provider-agnostic architecture.

---

119-147: LGTM - YouTube URI resolution with reasonable fallback strategy.

The "official" query suffix and matching logic are appropriate for finding music videos. Silent error handling is acceptable here since the caller handles null returns gracefully.

---

352-361: LGTM - Provider-appropriate initial playback.

Correctly uses spotifyPlayTracks for batch playback on Spotify and single-track playTrack for YouTube, reflecting their API capabilities.