refactor(plugins): load bundled plugins from resource dir (closes #280)

src-tauri/crates/app/src/commands/plugins.rs

@@ -23,7 +23,8 @@ use waveflow_core::plugin::runtime::{source_list_entries, source_resolve, source
 
 use crate::audio::{AudioCmd, AudioEngine};
 use crate::error::{AppError, AppResult};
-use crate::state::{AppState, is_bundled_plugin};
+use crate::state::AppState;
+use waveflow_core::plugin::is_bundled_plugin;
 
 /// Acquire the per-plugin write lock. Inserts a fresh `Mutex<()>`
 /// into the runtime's map the first time we see this id; returns
@@ -166,14 +167,66 @@ async fn read_enabled(app_db: &sqlx::SqlitePool, plugin_id: &str) -> AppResult<b
     Ok(row.map(|v| v == "true" || v == "1").unwrap_or(true))
 }
 
-/// List every plugin installed under `<app-data>/waveflow/plugins/`.
+/// Walk one install root and collect (id, manifest) pairs for every
+/// subdirectory whose `manifest.toml` parses cleanly and whose
+/// declared id matches the directory name. Missing dirs return an
+/// empty vec (a fresh install has no sideload tree). Other IO errors
+/// propagate.
 ///
-/// Iterates the install root, parses each subdirectory's
-/// `manifest.toml`, and returns a `PluginInfo` per valid plugin.
-/// Subdirectories with a missing or malformed manifest are silently
-/// skipped + logged at warn level — listing must never fail because
-/// one entry is corrupt; the user should still be able to see (and
-/// uninstall) their other plugins.
+/// Pure helper so [`list_installed_plugins`] can call it twice (once
+/// for `bundled_root`, once for `plugins_root`) and merge.
+fn walk_install_root(root: &std::path::Path) -> AppResult<Vec<(String, Manifest)>> {
+    let mut out = Vec::new();
+    let entries = match fs::read_dir(root) {
+        Ok(e) => e,
+        Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(out),
+        Err(e) => return Err(AppError::Io(e)),
+    };
+    for entry in entries.flatten() {
+        let dir = entry.path();
+        let Some(plugin_id) = dir.file_name().and_then(|n| n.to_str()).map(str::to_string) else {
+            continue;
+        };
+        let manifest_path = dir.join("manifest.toml");
+        match Manifest::load_from_path(&manifest_path) {
+            Ok(manifest) => {
+                // Pin: install dir name MUST match the manifest's
+                // declared id. The runtime refuses to load a
+                // mismatched plugin (Phase 2b's load-time guard),
+                // so skip it here too rather than surfacing a
+                // dangling row the user can't actually run.
+                if manifest.plugin.id != plugin_id {
+                    tracing::warn!(
+                        plugin_id,
+                        manifest_id = %manifest.plugin.id,
+                        "skipping plugin with id mismatch between dir and manifest"
+                    );
+                    continue;
+                }
+                out.push((plugin_id, manifest));
+            }
+            Err(err) => {
+                tracing::warn!(plugin_id, ?err, "skipping unreadable plugin manifest");
+            }
+        }
+    }
+    Ok(out)
+}
+
+/// List every plugin the runtime can load. Walks two roots:
+///
+/// 1. `<resource_dir>/plugins/` — installer-bundled tree (read-only).
+///    Source of truth for first-party ids (currently `web-radio`).
+/// 2. `<app-data>/waveflow/plugins/` — sideloaded tree (writable).
+///    Holds anything the user installs themselves.
+///
+/// Bundled wins on collision: if a sideloaded dir somehow declares a
+/// bundled id (shouldn't happen post-cleanup, but defence-in-depth),
+/// the bundled copy is the one the runtime would actually load via
+/// [`PluginPaths::plugin_dir`], so we surface that one and drop the
+/// duplicate from the list. Sideloaded subdirectories with a missing
+/// or malformed manifest are silently skipped + logged at warn level
+/// — listing must never fail because one entry is corrupt.
 ///
 /// The FS walk + TOML parse run on a blocking thread (each manifest
 /// is a `read_to_string` + `toml::from_str`, both sync); the
@@ -183,40 +236,31 @@ async fn read_enabled(app_db: &sqlx::SqlitePool, plugin_id: &str) -> AppResult<b
 pub async fn list_installed_plugins(state: State<'_, AppState>) -> AppResult<Vec<PluginInfo>> {
     let paths = state.paths.plugin_paths();
     let manifests = tokio::task::spawn_blocking(move || -> AppResult<Vec<(String, Manifest)>> {
+        let mut seen: std::collections::HashSet<String> = std::collections::HashSet::new();
         let mut out = Vec::new();
-        let entries = match fs::read_dir(&paths.plugins_root) {
-            Ok(e) => e,
-            Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(out),
-            Err(e) => return Err(AppError::Io(e)),
-        };
-        for entry in entries.flatten() {
-            let dir = entry.path();
-            let Some(plugin_id) = dir.file_name().and_then(|n| n.to_str()).map(str::to_string)
-            else {
+
+        // Bundled tree FIRST so its ids own the slot on any collision
+        // with a stray sideloaded entry. `bundled_root` is optional
+        // because dev builds without a bundle still need to list
+        // sideloaded plugins.
+        if let Some(bundled_root) = paths.bundled_root.as_deref() {
+            for (id, manifest) in walk_install_root(bundled_root)? {
+                seen.insert(id.clone());
+                out.push((id, manifest));
+            }
+        }
+
+        // Sideloaded tree second; skip any id the bundled walk
+        // already claimed.
+        for (id, manifest) in walk_install_root(&paths.plugins_root)? {
+            if seen.contains(&id) {
+                tracing::warn!(
+                    plugin_id = %id,
+                    "sideloaded plugin shadows a bundled id; skipping the sideloaded copy"
+                );
                 continue;
-            };
-            let manifest_path = dir.join("manifest.toml");
-            match Manifest::load_from_path(&manifest_path) {
-                Ok(manifest) => {
-                    // Pin: install dir name MUST match the manifest's
-                    // declared id. The runtime refuses to load a
-                    // mismatched plugin (Phase 2b's load-time guard),
-                    // so skip it here too rather than surfacing a
-                    // dangling row the user can't actually run.
-                    if manifest.plugin.id != plugin_id {
-                        tracing::warn!(
-                            plugin_id,
-                            manifest_id = %manifest.plugin.id,
-                            "skipping plugin with id mismatch between dir and manifest"
-                        );
-                        continue;
-                    }
-                    out.push((plugin_id, manifest));
-                }
-                Err(err) => {
-                    tracing::warn!(plugin_id, ?err, "skipping unreadable plugin manifest");
-                }
             }
+            out.push((id, manifest));
         }
         Ok(out)
     })

src-tauri/crates/app/src/paths.rs

@@ -1,5 +1,6 @@
 use std::path::PathBuf;
 
+use tauri::path::BaseDirectory;
 use tauri::{AppHandle, Manager};
 use waveflow_core::plugin::PluginPaths;
 
@@ -19,13 +20,24 @@ use crate::error::{AppError, AppResult};
 ///         ├── data.db           (per-profile database)
 ///         └── artwork/          (per-profile artwork cache)
 /// ```
+///
+/// `bundled_plugins_dir` is resolved separately against
+/// [`BaseDirectory::Resource`] and points at the installer-shipped
+/// plugin tree (e.g. `<install>/plugins/` on Windows NSIS,
+/// `/usr/lib/WaveFlow/plugins/` on Linux). It's optional because the
+/// resource resolver can legitimately fail in a few environments
+/// (dev mode without a bundle, packaging mishaps); when missing,
+/// bundled plugin resolution falls back to the writable app-data
+/// tree — the cleanup pass at boot removes any stale copies there
+/// so the fallback only matters for tests + dev builds.
 #[derive(Debug, Clone)]
 pub struct AppPaths {
     pub root: PathBuf,
     pub app_db: PathBuf,
     pub avatars_dir: PathBuf,
     pub metadata_artwork_dir: PathBuf,
     pub profiles_dir: PathBuf,
+    pub bundled_plugins_dir: Option<PathBuf>,
 }
 
 impl AppPaths {
@@ -41,11 +53,38 @@ impl AppPaths {
 
         let root = data_dir.join("waveflow");
 
+        // Bundled plugin resources live next to the binary, resolved
+        // via Tauri's `BaseDirectory::Resource`. A failure here isn't
+        // fatal — `PluginPaths::install_root_for` falls back to the
+        // writable app-data tree when `bundled_root` is `None`, so
+        // the only consequence in dev builds or broken installs is
+        // that bundled plugins resolve under `<app-data>/plugins/`
+        // (matching pre-1.5.1 behaviour). We log the failure so a
+        // mispackaged installer surfaces visibly in tracing.
+        let bundled_plugins_dir = match handle.path().resolve("plugins", BaseDirectory::Resource) {
+            Ok(path) if path.exists() && path.is_dir() => Some(path),
+            Ok(path) => {
+                tracing::warn!(
+                    path = %path.display(),
+                    "bundled plugins resource dir not found; bundled plugins will fall back to app-data tree",
+                );
+                None
+            }
+            Err(e) => {
+                tracing::warn!(
+                    %e,
+                    "bundled plugins resource dir not resolvable; bundled plugins will fall back to app-data tree",
+                );
+                None
+            }
+        };
+
         Ok(Self {
             app_db: root.join("app.db"),
             avatars_dir: root.join("avatars"),
             metadata_artwork_dir: root.join("metadata_artwork"),
             profiles_dir: root.join("profiles"),
+            bundled_plugins_dir,
             root,
         })
     }
@@ -100,10 +139,17 @@ impl AppPaths {
     /// and `new_store_for_plugin`. Layout:
     ///
     /// ```text
-    /// <root>/plugins/<plugin-id>/       (install dir, read-only at runtime)
+    /// <resource_dir>/plugins/<id>/      (bundled install dir, read-only — when resolvable)
+    /// <root>/plugins/<plugin-id>/       (sideloaded install dir, writable)
     /// <root>/plugin-data/<plugin-id>/   (per-user scratch, written by host imports)
     /// ```
+    ///
+    /// Bundled ids resolve under `<resource_dir>/plugins/` (the
+    /// installer's read-only payload), sideloaded ids under
+    /// `<root>/plugins/` (writable app-data). State writes always
+    /// land in `<root>/plugin-data/` regardless of where the .wasm
+    /// lives — bundled plugins still need a writable scratch dir.
     pub fn plugin_paths(&self) -> PluginPaths {
-        PluginPaths::from_app_data(&self.root)
+        PluginPaths::from_app_data(&self.root).with_bundled_root(self.bundled_plugins_dir.clone())
     }
 }

src-tauri/crates/app/src/state.rs

@@ -3,8 +3,7 @@ use std::sync::Arc;
 
 use chrono::Utc;
 use sqlx::SqlitePool;
-use tauri::path::BaseDirectory;
-use tauri::{AppHandle, Manager};
+use tauri::AppHandle;
 use tokio::sync::{Mutex, RwLock};
 use waveflow_core::plugin::runtime::{PluginRuntime, RuntimeConfig};
 
@@ -110,14 +109,23 @@ impl AppState {
         let paths = AppPaths::from_handle(handle)?;
         paths.ensure_dirs()?;
 
-        // First-launch + post-update extract of every plugin we ship
-        // bundled inside the app installer. Idempotent: only copies
-        // when the install dir is missing on disk, so a user who
-        // sideloaded a newer version of the same plugin doesn't see
-        // it clobbered on the next launch. Logged-only on failure
-        // — a corrupt bundle should not block the rest of startup.
-        if let Err(e) = ensure_bundled_plugins(handle, &paths).await {
-            tracing::warn!(%e, "bundled plugin extract failed");
+        // One-shot cleanup for the 1.5.0 → 1.5.1 transition: before
+        // this release, `ensure_bundled_plugins` copied every bundled
+        // plugin into `<app-data>/plugins/<id>/` at boot, wasting
+        // ~150 KB per id and confusing users who went folder
+        // spelunking (issue #280). The new model resolves bundled
+        // plugins directly from `BaseDirectory::Resource`, so any
+        // leftover writable copy under `<app-data>/plugins/` is dead
+        // weight that ALSO shadows the resource copy on case-
+        // insensitive filesystems if `list_installed_plugins` is
+        // ever extended to prefer sideloaded on a name collision.
+        // Drop them. Idempotent: re-running finds nothing to remove.
+        // Logged-only on failure — a stuck cleanup must not block
+        // the rest of startup.
+        if has_valid_bundled_plugins_dir(&paths) {
+            if let Err(e) = cleanup_bundled_plugin_leftovers(&paths).await {
+                tracing::warn!(%e, "bundled plugin leftover cleanup failed");
+            }
         }
 
         let app_db = db::app_db::open(&paths.app_db).await?;
@@ -350,72 +358,57 @@ impl AppState {
     }
 }
 
-/// Hardcoded list of plugins WaveFlow ships bundled in the
-/// installer. Phase 5 will replace this with a manifest-driven
-/// discovery (loop over `<resource_dir>/plugins/*/manifest.toml`)
-/// so adding a plugin to the bundle doesn't require touching app
-/// code; for v1.5.0 the static list keeps the diff focused.
-pub(crate) const BUNDLED_PLUGINS: &[&str] = &["web-radio"];
-
-/// True when `plugin_id` is a first-party plugin shipped inside the
-/// installer (re-seeded at every boot by [`ensure_bundled_plugins`]).
-/// Used to surface a "bundled" badge and refuse `uninstall_plugin` —
-/// the uninstall would only persist until next launch, then the boot
-/// extractor would silently put the plugin back, which reads as a bug
-/// to the user.
-pub(crate) fn is_bundled_plugin(plugin_id: &str) -> bool {
-    BUNDLED_PLUGINS.iter().any(|id| *id == plugin_id)
+// `BUNDLED_PLUGINS` + `is_bundled_plugin` moved to
+// `waveflow_core::plugin` so `PluginPaths` can route bundled ids to
+// the resource dir without re-importing app-layer state. Callers
+// inside `crate::commands::plugins` now `use waveflow_core::plugin::is_bundled_plugin;`.
+
+/// One-shot cleanup of pre-1.5.1 leftovers: drop any subdir of
+/// `<app-data>/plugins/` whose name is in
+/// [`waveflow_core::plugin::BUNDLED_PLUGINS`]. Before this release,
+/// `ensure_bundled_plugins` copied every bundled .wasm + manifest
+/// into the writable app-data tree at boot; the new model resolves
+/// them straight from `BaseDirectory::Resource` so those copies are
+/// dead weight (~150 KB per id) that ALSO confused users who went
+/// folder spelunking (issue #280). Idempotent: a 1.5.1 fresh install
+/// finds no leftovers and does nothing.
+///
+/// FS ops run on `spawn_blocking` — `remove_dir_all` on a multi-MB
+/// plugin tree (future bundled plugins with assets, or a Web Radio
+/// embedding a SQLite seed) can stretch into double-digit ms and
+/// we don't want to tie up a tokio worker during boot.
+fn has_valid_bundled_plugins_dir(paths: &AppPaths) -> bool {
+    matches!(
+        paths.bundled_plugins_dir.as_deref(),
+        Some(path) if path.exists() && path.is_dir()
+    )
 }
 
-/// Copy every entry in [`BUNDLED_PLUGINS`] from the installer's
-/// resource dir into `<plugins_root>/<id>/` when the install dir
-/// is missing. Idempotent on success — a user who sideloaded a
-/// newer version of the same plugin id keeps it.
-///
-/// File ops run on `spawn_blocking`: `fs::copy` of the ~140 KB
-/// .wasm is fast but every plugin call adds another set of
-/// syscalls, and we don't want them tying up an executor worker
-/// during boot.
-async fn ensure_bundled_plugins(handle: &AppHandle, paths: &AppPaths) -> AppResult<()> {
+async fn cleanup_bundled_plugin_leftovers(paths: &AppPaths) -> AppResult<()> {
+    let Some(bundled_root) = paths.bundled_plugins_dir.clone() else {
+        return Ok(());
+    };
     let plugins_root = paths.plugin_paths().plugins_root;
-    let mut resolved: Vec<(String, std::path::PathBuf, std::path::PathBuf)> =
-        Vec::with_capacity(BUNDLED_PLUGINS.len());
-    for id in BUNDLED_PLUGINS {
-        let wasm = handle.path().resolve(
-            format!("plugins/{id}/plugin.wasm"),
-            BaseDirectory::Resource,
-        );
-        let manifest = handle.path().resolve(
-            format!("plugins/{id}/manifest.toml"),
-            BaseDirectory::Resource,
-        );
-        match (wasm, manifest) {
-            (Ok(wasm), Ok(manifest)) => resolved.push(((*id).to_string(), wasm, manifest)),
-            (Err(e), _) => {
-                tracing::warn!(plugin_id = %id, %e, "bundled plugin wasm resource not resolvable, skipping");
-            }
-            (_, Err(e)) => {
-                tracing::warn!(plugin_id = %id, %e, "bundled plugin manifest resource not resolvable, skipping");
-            }
-        }
-    }
-
     tokio::task::spawn_blocking(move || -> AppResult<()> {
-        for (id, wasm, manifest) in resolved {
-            let install_dir = plugins_root.join(&id);
-            // Skip if there's already a manifest at the target —
-            // user might have sideloaded an updated version we
-            // don't want to overwrite.
-            if install_dir.join("manifest.toml").is_file() {
-                continue;
+        if !(bundled_root.exists() && bundled_root.is_dir()) {
+            tracing::warn!(
+                path = %bundled_root.display(),
+                "bundled plugins resource dir unavailable; preserving app-data bundled plugin fallback",
+            );
+            return Ok(());
+        }
+        for id in waveflow_core::plugin::BUNDLED_PLUGINS {
+            let leftover = plugins_root.join(id);
+            match std::fs::remove_dir_all(&leftover) {
+                Ok(()) => {
+                    tracing::info!(plugin_id = %id, "removed pre-1.5.1 bundled plugin leftover");
+                }
+                Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}
+                Err(e) => return Err(AppError::Io(e)),
             }
-            std::fs::create_dir_all(&install_dir)?;
-            std::fs::copy(&wasm, install_dir.join("plugin.wasm"))?;
-            std::fs::copy(&manifest, install_dir.join("manifest.toml"))?;
-            tracing::info!(plugin_id = %id, "extracted bundled plugin");
         }
         Ok(())
     })
     .await
-    .map_err(|e| AppError::Other(format!("bundled plugin extract join: {e}")))?
+    .map_err(|e| AppError::Other(format!("bundled plugin cleanup join: {e}")))?
 }