fix: address 7 code review findings for V2 architecture

HIGH priority fixes:
- Enable WAL mode in qbz-integrations caches (ADR-002)
- v2_auto_login/v2_manual_login now call activate_session()
- RequiresUserSession allows offline sessions (only checks session_activated)

MEDIUM priority fixes:
- Consolidate lifecycle: legacy commands delegate to session_lifecycle
- ListenBrainz teardown now calls disconnect() to clear tokens
- Add SessionActivationFailed degraded reason and emit event on failure

crates/qbz-integrations/src/listenbrainz/cache.rs

@@ -21,6 +21,10 @@ impl ListenBrainzCache {
         let conn = Connection::open(db_path)
             .map_err(|e| format!("Failed to open ListenBrainz cache: {}", e))?;
 
+        // Enable WAL mode for concurrent read/write (ADR-002)
+        conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL;")
+            .map_err(|e| format!("Failed to enable WAL mode: {}", e))?;
+
         let cache = Self { conn };
         cache.init_schema()?;
 

crates/qbz-integrations/src/musicbrainz/cache.rs

@@ -31,6 +31,10 @@ impl MusicBrainzCache {
         let conn = Connection::open(db_path)
             .map_err(|e| format!("Failed to open MusicBrainz cache: {}", e))?;
 
+        // Enable WAL mode for concurrent read/write (ADR-002)
+        conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL;")
+            .map_err(|e| format!("Failed to enable WAL mode: {}", e))?;
+
         let cache = Self { conn };
         cache.init_schema()?;
 

src-tauri/src/commands/user_session.rs

@@ -3,35 +3,12 @@
 //! After login, the frontend calls `activate_user_session` with the Qobuz user_id.
 //! This initializes all per-user database stores at the user-scoped directory.
 //! On logout, `deactivate_user_session` tears everything down.
+//!
+//! These legacy commands now delegate to session_lifecycle module.
 
 use std::path::Path;
 use std::sync::{Arc, Mutex};
-use tauri::{Manager, State};
 
-use crate::api_cache::ApiCacheState;
-use crate::artist_blacklist::BlacklistState;
-use crate::artist_vectors::ArtistVectorStoreState;
-use crate::runtime::RuntimeManagerState;
-use crate::config::{
-    audio_settings::AudioSettingsState,
-    download_settings::{DownloadSettingsState, DownloadSettingsStore},
-    favorites_cache::FavoritesCacheState,
-    favorites_preferences::FavoritesPreferencesState,
-    legal_settings::{LegalSettingsState, LegalSettingsStore},
-    playback_preferences::PlaybackPreferencesState,
-    remote_control_settings::{AllowedOriginsState, RemoteControlSettingsState},
-    subscription_state::{SubscriptionStateState, SubscriptionStateStore},
-    tray_settings::TraySettingsState,
-};
-use crate::library::commands::LibraryState;
-use crate::listenbrainz::ListenBrainzSharedState;
-use crate::lyrics::LyricsState;
-use crate::musicbrainz::MusicBrainzSharedState;
-use crate::offline::OfflineState;
-use crate::offline_cache::OfflineCacheState;
-use crate::reco_store::RecoState;
-use crate::session_store::SessionStoreState;
-use crate::updates::UpdatesState;
 use crate::user_data::UserDataPaths;
 
 /// Helper to init a type-alias state (Arc<Mutex<Option<Store>>>) at a path
@@ -68,205 +45,25 @@ pub fn get_last_user_id() -> Option<u64> {
 /// This runs the one-time migration (if needed) and initializes all
 /// per-user database stores at `~/.local/share/qbz/users/{user_id}/`
 /// and cache stores at `~/.cache/qbz/users/{user_id}/`.
+///
+/// This is a legacy command - now delegates to session_lifecycle::activate_session().
 #[tauri::command]
 pub async fn activate_user_session(
     app: tauri::AppHandle,
     user_id: u64,
-    user_paths: State<'_, UserDataPaths>,
-    session_store: State<'_, SessionStoreState>,
-    favorites_cache: State<'_, FavoritesCacheState>,
-    subscription_state: State<'_, SubscriptionStateState>,
-    playback_prefs: State<'_, PlaybackPreferencesState>,
-    favorites_prefs: State<'_, FavoritesPreferencesState>,
-    download_settings: State<'_, DownloadSettingsState>,
-    audio_settings: State<'_, AudioSettingsState>,
-    tray_settings: State<'_, TraySettingsState>,
-    remote_control_settings: State<'_, RemoteControlSettingsState>,
-    allowed_origins: State<'_, AllowedOriginsState>,
-    legal_settings: State<'_, LegalSettingsState>,
-    updates: State<'_, UpdatesState>,
-    library: State<'_, LibraryState>,
-    reco: State<'_, RecoState>,
-    api_cache: State<'_, ApiCacheState>,
-    artist_vectors: State<'_, ArtistVectorStoreState>,
-    blacklist: State<'_, BlacklistState>,
-    offline: State<'_, OfflineState>,
-    offline_cache: State<'_, OfflineCacheState>,
-    lyrics: State<'_, LyricsState>,
-    musicbrainz: State<'_, MusicBrainzSharedState>,
-    listenbrainz: State<'_, ListenBrainzSharedState>,
-    runtime_manager: State<'_, RuntimeManagerState>,
 ) -> Result<(), String> {
-    log::info!("Activating user session for user_id={}", user_id);
+    log::info!("[Legacy] activate_user_session called - delegating to session_lifecycle");
-
+    crate::session_lifecycle::activate_session(&app, user_id).await
-    // Set the active user for path resolution
-    user_paths.set_user(user_id);
-
-    // Run one-time flat-to-user migration if needed
-    if let Err(e) = crate::migration::migrate_flat_to_user(user_id) {
-        log::error!("Migration failed: {}", e);
-        // Non-fatal: user gets a fresh slate if migration fails
-    }
-
-    // Resolve user-scoped directories
-    let data_dir = user_paths.user_data_dir()?;
-    let cache_dir = user_paths.user_cache_dir()?;
-
-    // Ensure directories exist
-    std::fs::create_dir_all(&data_dir)
-        .map_err(|e| format!("Failed to create user data dir: {}", e))?;
-    std::fs::create_dir_all(&cache_dir)
-        .map_err(|e| format!("Failed to create user cache dir: {}", e))?;
-
-    log::info!("User data dir: {}", data_dir.display());
-    log::info!("User cache dir: {}", cache_dir.display());
-
-    // Initialize all per-user states at the user directory.
-    // Data-dir stores:
-    session_store.init_at(&data_dir)?;
-    favorites_cache.init_at(&data_dir)?;
-    playback_prefs.init_at(&data_dir)?;
-    favorites_prefs.init_at(&data_dir)?;
-    audio_settings.init_at(&data_dir)?;
-    tray_settings.init_at(&data_dir)?;
-    remote_control_settings.init_at(&data_dir)?;
-    allowed_origins.init_at(&data_dir)?;
-    updates.init_at(&data_dir)?;
-    library.init_at(&data_dir).await?;
-    reco.init_at(&data_dir).await?;
-    api_cache.init_at(&data_dir).await?;
-    artist_vectors.init_at(&data_dir).await?;
-    blacklist.init_at(&data_dir)?;
-    offline.init_at(&data_dir)?;
-    musicbrainz.init_at(&data_dir).await?;
-    listenbrainz.init_at(&data_dir).await?;
-
-    // Type-alias states (no init_at method — init inline)
-    init_type_alias_state(&*subscription_state, &data_dir, SubscriptionStateStore::new_at)?;
-    init_type_alias_state(&*download_settings, &data_dir, DownloadSettingsStore::new_at)?;
-    init_type_alias_state(&*legal_settings, &data_dir, LegalSettingsStore::new_at)?;
-
-    // Cache-dir stores:
-    offline_cache.init_at(&cache_dir).await?;
-    lyrics.init_at(&cache_dir).await?;
-
-    // Run deferred subscription purge check (was removed from startup)
-    let now = std::time::SystemTime::now()
-        .duration_since(std::time::UNIX_EPOCH)
-        .unwrap_or_default()
-        .as_secs() as i64;
-
-    let should_purge = {
-        let guard = subscription_state.lock().map_err(|e| format!("Lock error: {}", e))?;
-        guard.as_ref()
-            .and_then(|s| s.should_purge_offline_cache(now).ok())
-            .unwrap_or(false)
-    };
-
-    if should_purge {
-        log::warn!("Subscription invalid for >3 days. Purging offline cache.");
-        if let Err(e) = crate::offline_cache::commands::purge_all_cached_files(
-            offline_cache.inner(),
-            library.inner(),
-        ).await {
-            log::error!("Failed to purge offline cache: {}", e);
-        } else {
-            let guard = subscription_state.lock().map_err(|e| format!("Lock error: {}", e))?;
-            if let Some(store) = guard.as_ref() {
-                let _ = store.mark_offline_cache_purged(now);
-            }
-        }
-    }
-
-    // Persist last user_id for session restore on next launch
-    if let Err(e) = UserDataPaths::save_last_user_id(user_id) {
-        log::warn!("Failed to save last_user_id: {}", e);
-    }
-
-    // Start visualizer FFT thread (idempotent — only starts once even if called twice)
-    app.state::<crate::AppState>()
-        .visualizer
-        .start(app.clone());
-
-    // Start remote control API server if enabled in user settings
-    let app_clone = app.clone();
-    tauri::async_runtime::spawn(async move {
-        if let Err(e) = crate::api_server::sync_server(&app_clone).await {
-            log::error!("Remote control API init failed: {}", e);
-        }
-    });
-
-    // Update runtime state to reflect session activation
-    runtime_manager.manager().set_session_activated(true, user_id).await;
-    log::info!("User session activated for user_id={}", user_id);
-    Ok(())
 }
 
 /// Deactivate the per-user session on logout.
 ///
 /// Tears down all per-user stores, closing database connections.
+/// This is a legacy command - now delegates to session_lifecycle::deactivate_session().
 #[tauri::command]
 pub async fn deactivate_user_session(
-    user_paths: State<'_, UserDataPaths>,
+    app: tauri::AppHandle,
-    session_store: State<'_, SessionStoreState>,
-    favorites_cache: State<'_, FavoritesCacheState>,
-    subscription_state: State<'_, SubscriptionStateState>,
-    playback_prefs: State<'_, PlaybackPreferencesState>,
-    favorites_prefs: State<'_, FavoritesPreferencesState>,
-    download_settings: State<'_, DownloadSettingsState>,
-    audio_settings: State<'_, AudioSettingsState>,
-    tray_settings: State<'_, TraySettingsState>,
-    remote_control_settings: State<'_, RemoteControlSettingsState>,
-    allowed_origins: State<'_, AllowedOriginsState>,
-    legal_settings: State<'_, LegalSettingsState>,
-    updates: State<'_, UpdatesState>,
-    library: State<'_, LibraryState>,
-    reco: State<'_, RecoState>,
-    api_cache: State<'_, ApiCacheState>,
-    artist_vectors: State<'_, ArtistVectorStoreState>,
-    blacklist: State<'_, BlacklistState>,
-    offline: State<'_, OfflineState>,
-    offline_cache: State<'_, OfflineCacheState>,
-    lyrics: State<'_, LyricsState>,
-    musicbrainz: State<'_, MusicBrainzSharedState>,
-    listenbrainz: State<'_, ListenBrainzSharedState>,
-    runtime_manager: State<'_, RuntimeManagerState>,
 ) -> Result<(), String> {
-    log::info!("Deactivating user session");
+    log::info!("[Legacy] deactivate_user_session called - delegating to session_lifecycle");
-
+    crate::session_lifecycle::deactivate_session(&app).await
-    // Teardown all per-user stores (closes DB connections)
-    session_store.teardown();
-    favorites_cache.teardown()?;
-    playback_prefs.teardown()?;
-    favorites_prefs.teardown()?;
-    audio_settings.teardown()?;
-    tray_settings.teardown()?;
-    remote_control_settings.teardown()?;
-    allowed_origins.teardown()?;
-    updates.teardown();
-    library.teardown().await;
-    reco.teardown().await;
-    api_cache.teardown().await;
-    artist_vectors.teardown().await;
-    blacklist.teardown();
-    offline.teardown();
-    offline_cache.teardown().await;
-    lyrics.teardown().await;
-    musicbrainz.teardown().await;
-    listenbrainz.teardown().await;
-
-    // Type-alias states
-    teardown_type_alias_state(&*subscription_state);
-    teardown_type_alias_state(&*download_settings);
-    teardown_type_alias_state(&*legal_settings);
-
-    // Clear the active user and persisted last_user_id
-    user_paths.clear_user();
-    UserDataPaths::clear_last_user_id();
-
-    // Update runtime state to reflect session deactivation
-    runtime_manager.manager().set_legacy_auth(false, None).await;
-
-    log::info!("User session deactivated");
-    Ok(())
 }

src-tauri/src/commands_v2.rs

@@ -205,8 +205,10 @@ pub async fn runtime_bootstrap(
                 // This initializes all per-user stores and sets runtime state
                 if let Err(e) = crate::session_lifecycle::activate_session(&app, session.user_id).await {
                     log::error!("[Runtime] Session activation failed: {}", e);
-                    // Non-fatal for bootstrap - stores can be initialized later
+                    // Non-fatal for bootstrap - user can still use app but some features may not work
-                    // But log it prominently
+                    // Emit degraded event so frontend knows
+                    let reason = DegradedReason::SessionActivationFailed(e.clone());
+                    let _ = app.emit("runtime:event", RuntimeEvent::RuntimeDegraded { reason });
                 }
             }
             Err(e) => {
@@ -391,6 +393,12 @@ pub async fn v2_auto_login(
         });
     }
 
+    // Activate per-user session (initializes all per-user stores)
+    if let Err(e) = crate::session_lifecycle::activate_session(&app, session.user_id).await {
+        log::error!("[V2] Session activation failed: {}", e);
+        // Non-fatal - consider login successful, stores can be initialized later
+    }
+
     // Emit ready event
     let _ = app.emit("runtime:event", RuntimeEvent::RuntimeReady {
         user_id: session.user_id,
@@ -495,6 +503,12 @@ pub async fn v2_manual_login(
         });
     }
 
+    // Activate per-user session (initializes all per-user stores)
+    if let Err(e) = crate::session_lifecycle::activate_session(&app, session.user_id).await {
+        log::error!("[V2] Session activation failed: {}", e);
+        // Non-fatal - consider login successful, stores can be initialized later
+    }
+
     // Emit ready event
     let _ = app.emit("runtime:event", RuntimeEvent::RuntimeReady {
         user_id: session.user_id,

src-tauri/src/listenbrainz/mod.rs

@@ -124,6 +124,9 @@ impl ListenBrainzSharedState {
     }
 
     pub async fn teardown(&self) {
+        // Clear in-memory token to prevent cross-user token leaks
+        self.client.lock().await.disconnect().await;
+        // Close cache DB connection
         let mut guard = self.cache.lock().await;
         *guard = None;
     }

src-tauri/src/runtime.rs

@@ -47,6 +47,8 @@ pub enum DegradedReason {
     NetworkError(String),
     /// Database/storage issues
     StorageError(String),
+    /// Session activation failed (per-user stores not initialized)
+    SessionActivationFailed(String),
 }
 
 /// Full runtime status returned by runtime_get_status and runtime_bootstrap
@@ -284,11 +286,13 @@ impl RuntimeManager {
                 }
             }
             CommandRequirement::RequiresUserSession => {
-                if !status.client_initialized {
+                // RequiresUserSession only checks session_activated, not legacy_auth.
-                    Err(RuntimeError::RuntimeNotInitialized)
+                // This allows both:
-                } else if !status.legacy_auth {
+                // - Online sessions: session_activated=true via activate_session()
-                    Err(RuntimeError::AuthRequired)
+                // - Offline sessions: session_activated=true via activate_offline_session()
-                } else if !status.session_activated {
+                //
+                // Commands needing Qobuz API should use RequiresAuth or RequiresCoreBridgeAuth.
+                if !status.session_activated {
                     Err(RuntimeError::UserSessionNotActivated)
                 } else {
                     Ok(())