From fe9095b4066d92d8b458a278bf43ea465945245f Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Sat, 20 Jun 2026 21:03:56 +0200 Subject: [PATCH 1/2] refactor(plugins): load bundled plugins from resource dir (closes #280) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-1.5.1, `ensure_bundled_plugins` copied every bundled plugin (`manifest.toml` + `plugin.wasm`) from the installer's resource tree into `/plugins//` at every boot. That gave us one uniform install root for both bundled and sideloaded plugins at the cost of ~150 KB duplicated per id per install, ~10 ms of boot-time IO, and one bad user-facing surprise: anybody who went folder spelunking found `web-radio` sitting in two places (`%LocalAppData%\WaveFlow\plugins\` AND `%AppData%\Roaming\app.waveflow\waveflow\plugins\`) and rightly asked "why?" (issue #280). This change moves bundled plugins to resolve directly from `BaseDirectory::Resource` and eliminates the copy entirely. ## What moves - `BUNDLED_PLUGINS` + `is_bundled_plugin` shift from `crates/app/src/state.rs` into `waveflow_core::plugin`. They're a static fact about which ids are first-party, and core needs to know to route path resolution. The desktop's existing callsite (`commands::plugins::manifest_to_info` for the `bundled: bool` badge + `uninstall_plugin`'s refusal gate) re-imports from core unchanged. - `PluginPaths` gains an optional `bundled_root: Option` field + a `with_bundled_root` builder. Path-resolution methods (`plugin_dir`, `manifest_path`, `wasm_path`, `assets_dir`) route bundled ids under `bundled_root` when present, sideloaded ids under `plugins_root`. `state_dir` always targets `data_root` regardless of bundling — user state stays in the writable app-data tree so a bundled-plugin upgrade can't lose it. - `AppPaths::from_handle` resolves `BaseDirectory::Resource / plugins` once at startup + stores it as `bundled_plugins_dir`. Failure (dev builds, broken installs) is logged at WARN + recorded as `None` so bundled resolution falls back to the writable tree — pre-1.5.1 behaviour, kept as a safety net. - `AppPaths::plugin_paths()` injects `bundled_plugins_dir` into the `PluginPaths` it returns. Every existing callsite that pulls `PluginPaths` (the wasmtime runtime, every command in `commands/plugins.rs`) gets the new routing transparently. ## What's removed - `ensure_bundled_plugins(handle, paths)` — the copy that was the whole reason `/plugins/web-radio/` ever existed. - Its `BaseDirectory` + `Manager` imports from `state.rs` (now resolved in `paths.rs`). ## What's added - `cleanup_bundled_plugin_leftovers(paths)` — one-shot cleanup that drops any subdirectory of `/plugins/` whose name is in `BUNDLED_PLUGINS`. Handles the 1.5.0 → 1.5.1 transition for users whose disk already carries the pre-refactor copies. Idempotent: a 1.5.1 fresh install finds nothing to remove. - `walk_install_root(root)` helper — pulled out of `list_installed_plugins` so the same parse + dir-id-pin logic serves both the bundled walk and the sideloaded walk. - `list_installed_plugins` now walks BOTH the bundled tree (when resolvable) AND the sideloaded tree, with bundled winning on any id collision (defence-in-depth — post-cleanup collisions shouldn't happen, but a stray sideloaded copy of a reserved id shouldn't load the wrong .wasm). ## Tests Four new unit tests in `core::plugin::tests`: - `bundled_plugin_routes_to_bundled_root` pins the bundled-id -> resource-path routing and confirms `state_dir` still hits the writable tree. - `sideloaded_plugin_stays_in_plugins_root_even_with_bundled_set` confirms non-bundled ids ignore the resource root. - `bundled_plugin_falls_back_to_plugins_root_without_bundled_set` covers the `bundled_root: None` fallback for tests + dev builds. - `is_bundled_plugin_only_matches_known_ids` checks case- sensitivity + the empty-string edge. All 40 `plugin::` unit tests + 84 desktop unit tests pass; full workspace `cargo check --all-targets` is warning-free. ## Disk footprint `%LocalAppData%\WaveFlow\plugins\web-radio\` (resource dir, 143 KB): the installer's read-only copy, source of truth, unchanged. `%AppData%\Roaming\app.waveflow\waveflow\plugins\web-radio\` (was the writable copy): no longer created. Existing copies from a prior 1.5.0 install are removed at boot by the cleanup pass. --- src-tauri/crates/app/src/commands/plugins.rs | 122 +++++++++----- src-tauri/crates/app/src/paths.rs | 43 ++++- src-tauri/crates/app/src/state.rs | 114 +++++-------- src-tauri/crates/core/src/plugin/mod.rs | 162 +++++++++++++++++-- 4 files changed, 315 insertions(+), 126 deletions(-) diff --git a/src-tauri/crates/app/src/commands/plugins.rs b/src-tauri/crates/app/src/commands/plugins.rs index ae989fc9..23326f42 100644 --- a/src-tauri/crates/app/src/commands/plugins.rs +++ b/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/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> { + 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. `/plugins/` — installer-bundled tree (read-only). +/// Source of truth for first-party ids (currently `web-radio`). +/// 2. `/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) -> AppResult> { let paths = state.paths.plugin_paths(); let manifests = tokio::task::spawn_blocking(move || -> AppResult> { + let mut seen: std::collections::HashSet = 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) }) diff --git a/src-tauri/crates/app/src/paths.rs b/src-tauri/crates/app/src/paths.rs index 1deb0885..355beb19 100644 --- a/src-tauri/crates/app/src/paths.rs +++ b/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,6 +20,16 @@ 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. `/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, @@ -26,6 +37,7 @@ pub struct AppPaths { pub avatars_dir: PathBuf, pub metadata_artwork_dir: PathBuf, pub profiles_dir: PathBuf, + pub bundled_plugins_dir: Option, } impl AppPaths { @@ -41,11 +53,31 @@ 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 `/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) => Some(path), + 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 +132,17 @@ impl AppPaths { /// and `new_store_for_plugin`. Layout: /// /// ```text - /// /plugins// (install dir, read-only at runtime) + /// /plugins// (bundled install dir, read-only — when resolvable) + /// /plugins// (sideloaded install dir, writable) /// /plugin-data// (per-user scratch, written by host imports) /// ``` + /// + /// Bundled ids resolve under `/plugins/` (the + /// installer's read-only payload), sideloaded ids under + /// `/plugins/` (writable app-data). State writes always + /// land in `/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()) } } diff --git a/src-tauri/crates/app/src/state.rs b/src-tauri/crates/app/src/state.rs index 3550347e..67005128 100644 --- a/src-tauri/crates/app/src/state.rs +++ b/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,21 @@ 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 `/plugins//` 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 `/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 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 +356,40 @@ impl AppState { } } -/// Hardcoded list of plugins WaveFlow ships bundled in the -/// installer. Phase 5 will replace this with a manifest-driven -/// discovery (loop over `/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) -} - -/// Copy every entry in [`BUNDLED_PLUGINS`] from the installer's -/// resource dir into `//` when the install dir -/// is missing. Idempotent on success — a user who sideloaded a -/// newer version of the same plugin id keeps it. +// `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 +/// `/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. /// -/// 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<()> { +/// 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. +async fn cleanup_bundled_plugin_leftovers(paths: &AppPaths) -> AppResult<()> { 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; + 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}")))? } diff --git a/src-tauri/crates/core/src/plugin/mod.rs b/src-tauri/crates/core/src/plugin/mod.rs index 9aab06d7..2bdea6be 100644 --- a/src-tauri/crates/core/src/plugin/mod.rs +++ b/src-tauri/crates/core/src/plugin/mod.rs @@ -25,21 +25,56 @@ pub mod runtime; use std::path::{Component, Path, PathBuf}; -/// Two parallel roots under the host's app-data dir: +/// Hardcoded list of plugins WaveFlow ships bundled in the +/// installer. Lives in core so [`PluginPaths`] can route bundled +/// ids to the resource dir without round-tripping back through the +/// app layer. /// -/// - `plugins_root` (`/waveflow/plugins/`) — install -/// directory, treated as read-only at runtime. One subdirectory -/// per installed plugin holding `manifest.toml`, `plugin.wasm`, -/// and the `assets/` subtree. +/// Phase 5 will replace this with a manifest-driven discovery +/// (loop over `/plugins/*/manifest.toml`) so adding +/// a plugin to the bundle doesn't require touching app code; for +/// now the static list keeps the diff focused. +pub const BUNDLED_PLUGINS: &[&str] = &["web-radio"]; + +/// True when `plugin_id` is a first-party plugin shipped inside the +/// installer. Used by [`PluginPaths`] to route path resolution to +/// the resource dir, by the app to refuse `uninstall_plugin` (the +/// installer would re-seed the tree on next launch so the uninstall +/// reads as a bug), and by the UI to render a "bundled" badge. +pub fn is_bundled_plugin(plugin_id: &str) -> bool { + BUNDLED_PLUGINS.iter().any(|id| *id == plugin_id) +} + +/// Three parallel roots resolved by the host: +/// +/// - `bundled_root` (optional, `/plugins/`) — read-only +/// install root for plugins shipped inside the application +/// installer. Resolved from `BaseDirectory::Resource` on app side; +/// `None` in core-only tests or when no bundled tree is present. +/// When [`is_bundled_plugin`] returns true, path resolution +/// targets this root instead of `plugins_root`. +/// - `plugins_root` (`/waveflow/plugins/`) — sideloaded +/// install dir, writable. One subdirectory per sideloaded plugin +/// holding `manifest.toml`, `plugin.wasm`, and the `assets/` +/// subtree. Bundled plugins are NEVER copied here (was the case +/// pre-1.5.1 via `ensure_bundled_plugins`; that path wasted +/// ~150 KB per bundled plugin per install and confused users who +/// went folder spelunking — see issue #280). The cleanup pass at +/// boot drops any leftover bundled entry from a 1.5.0 → 1.5.1 +/// upgrade. /// - `data_root` (`/waveflow/plugin-data/`) — per-user /// scratch store the host hands to the plugin via -/// `waveflow:host/storage.{read,write}-state`. Kept separate from -/// the install dir so reinstalls / updates don't risk clobbering -/// the user's plugin state and so a `plugin-data/` wipe doesn't -/// require touching the install tree. +/// `waveflow:host/storage.{read,write}-state`. Always in the +/// writable app-data tree, regardless of where the .wasm itself +/// lives — bundled plugins still write state to the user's data +/// dir. #[derive(Debug, Clone)] pub struct PluginPaths { - /// `/waveflow/plugins/` — install root, one dir per plugin. + /// Optional read-only install root for bundled plugins (resource + /// dir, e.g. `/plugins/` on Windows NSIS, `/usr/lib/WaveFlow/plugins/` + /// on Linux). When `Some`, [`is_bundled_plugin`] ids resolve here. + pub bundled_root: Option, + /// `/waveflow/plugins/` — sideloaded install root. pub plugins_root: PathBuf, /// `/waveflow/plugin-data/` — scratch root, one dir per plugin. pub data_root: PathBuf, @@ -59,11 +94,38 @@ impl PluginPaths { /// subdirectories so every plugin shares one layout. pub fn from_app_data(app_data_dir: &Path) -> Self { Self { + bundled_root: None, plugins_root: app_data_dir.join("plugins"), data_root: app_data_dir.join("plugin-data"), } } + /// Inject the resolved bundled plugins root (typically + /// `/plugins/`). Consumed-then-returned builder + /// style so the app can chain it off `from_app_data` at startup + /// without rebuilding the struct. A `None` argument is accepted + /// and clears the field, which keeps the helper symmetric for + /// tests that want to opt-out. + pub fn with_bundled_root(mut self, bundled_root: Option) -> Self { + self.bundled_root = bundled_root; + self + } + + /// Pick the install root for `plugin_id`. Bundled ids resolve + /// against [`Self::bundled_root`] when present, sideloaded ids + /// against [`Self::plugins_root`]. The fallback (`bundled_root` + /// is `None`) lets a bundled id still resolve under + /// `plugins_root` so unit tests + core-only contexts keep + /// working without a resource tree on disk. + fn install_root_for(&self, plugin_id: &str) -> &Path { + if is_bundled_plugin(plugin_id) { + if let Some(root) = &self.bundled_root { + return root; + } + } + &self.plugins_root + } + /// Sanitise a plugin id so it can never escape `self.plugins_root` /// or `self.data_root`. The manifest validator already restricts /// ids to `[a-z0-9-]+`, but `PluginPaths` is the last line of @@ -92,12 +154,14 @@ impl PluginPaths { } } - /// Path of one plugin's install directory. Returns - /// [`InvalidPluginId`] when `plugin_id` would escape - /// `self.plugins_root` (absolute path, `..` segment, embedded - /// separator). + /// Path of one plugin's install directory. Bundled ids resolve + /// under [`Self::bundled_root`] (read-only, populated from the + /// resource dir), sideloaded ids under [`Self::plugins_root`] + /// (writable, in the app-data tree). Returns [`InvalidPluginId`] + /// when `plugin_id` would escape the chosen root (absolute path, + /// `..` segment, embedded separator). pub fn plugin_dir(&self, plugin_id: &str) -> Result { - Self::sanitise_id(plugin_id).map(|id| self.plugins_root.join(id)) + Self::sanitise_id(plugin_id).map(|id| self.install_root_for(id).join(id)) } /// Path of one plugin's per-user scratch directory under @@ -169,6 +233,74 @@ mod tests { assert!(paths.state_dir("").is_err()); } + #[test] + fn bundled_plugin_routes_to_bundled_root() { + // When `bundled_root` is set, a bundled id (`web-radio` per + // [`BUNDLED_PLUGINS`]) resolves under it for install + manifest + // + wasm + assets — but NOT for state_dir, which always stays + // in the writable app-data tree so user state survives an + // install dir refresh. + let paths = PluginPaths::from_app_data(Path::new("/tmp/waveflow")) + .with_bundled_root(Some(PathBuf::from("/opt/waveflow/resources/plugins"))); + assert_eq!( + paths.plugin_dir("web-radio").unwrap(), + Path::new("/opt/waveflow/resources/plugins/web-radio") + ); + assert_eq!( + paths.manifest_path("web-radio").unwrap(), + Path::new("/opt/waveflow/resources/plugins/web-radio/manifest.toml") + ); + assert_eq!( + paths.wasm_path("web-radio").unwrap(), + Path::new("/opt/waveflow/resources/plugins/web-radio/plugin.wasm") + ); + assert_eq!( + paths.assets_dir("web-radio").unwrap(), + Path::new("/opt/waveflow/resources/plugins/web-radio/assets") + ); + // State lives in the writable tree even for bundled plugins. + assert_eq!( + paths.state_dir("web-radio").unwrap(), + Path::new("/tmp/waveflow/plugin-data/web-radio") + ); + } + + #[test] + fn sideloaded_plugin_stays_in_plugins_root_even_with_bundled_set() { + // Non-bundled ids always resolve under `plugins_root` (the + // sideloaded tree), regardless of whether a `bundled_root` + // was injected. This is what keeps user-installed plugins + // working alongside first-party bundled ones. + let paths = PluginPaths::from_app_data(Path::new("/tmp/waveflow")) + .with_bundled_root(Some(PathBuf::from("/opt/waveflow/resources/plugins"))); + assert_eq!( + paths.plugin_dir("my-custom").unwrap(), + Path::new("/tmp/waveflow/plugins/my-custom") + ); + } + + #[test] + fn bundled_plugin_falls_back_to_plugins_root_without_bundled_set() { + // Tests + core-only contexts that never resolve a resource + // dir must still be able to call `plugin_dir("web-radio")` + // and get a deterministic path back — fall back to + // `plugins_root` so existing fixtures keep working. + let paths = PluginPaths::from_app_data(Path::new("/tmp/waveflow")); + assert!(paths.bundled_root.is_none()); + assert_eq!( + paths.plugin_dir("web-radio").unwrap(), + Path::new("/tmp/waveflow/plugins/web-radio") + ); + } + + #[test] + fn is_bundled_plugin_only_matches_known_ids() { + assert!(is_bundled_plugin("web-radio")); + assert!(!is_bundled_plugin("web-Radio")); // case-sensitive + assert!(!is_bundled_plugin("not-bundled")); + assert!(!is_bundled_plugin("")); + } + #[test] fn plugin_dir_rejects_absolute_id() { let paths = PluginPaths::from_app_data(Path::new("/tmp/waveflow")); From f82ceaacd1d4d4f28827a368e34b2f8ab28308f4 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Sat, 20 Jun 2026 21:23:56 +0200 Subject: [PATCH 2/2] fix(plugins): preserve fallback without bundled resources --- src-tauri/crates/app/src/paths.rs | 9 ++++++++- src-tauri/crates/app/src/state.rs | 23 +++++++++++++++++++++-- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src-tauri/crates/app/src/paths.rs b/src-tauri/crates/app/src/paths.rs index 355beb19..05f1143b 100644 --- a/src-tauri/crates/app/src/paths.rs +++ b/src-tauri/crates/app/src/paths.rs @@ -62,7 +62,14 @@ impl AppPaths { // (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) => Some(path), + 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, diff --git a/src-tauri/crates/app/src/state.rs b/src-tauri/crates/app/src/state.rs index 67005128..9c3fab91 100644 --- a/src-tauri/crates/app/src/state.rs +++ b/src-tauri/crates/app/src/state.rs @@ -122,8 +122,10 @@ impl AppState { // Drop them. Idempotent: re-running finds nothing to remove. // Logged-only on failure — a stuck cleanup must not block // the rest of startup. - if let Err(e) = cleanup_bundled_plugin_leftovers(&paths).await { - tracing::warn!(%e, "bundled plugin leftover cleanup failed"); + 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?; @@ -375,9 +377,26 @@ impl AppState { /// 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() + ) +} + 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; tokio::task::spawn_blocking(move || -> AppResult<()> { + 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) {