diff --git a/apps/skit/src/plugin_assets.rs b/apps/skit/src/plugin_assets.rs index 58c2f832..c749b259 100644 --- a/apps/skit/src/plugin_assets.rs +++ b/apps/skit/src/plugin_assets.rs @@ -898,9 +898,10 @@ pub fn plugin_assets_router() -> Router> { /// /// Searches for the manifest in two locations: /// 1. `plugin.yml` / `plugin.yaml` in the same directory as the `.so` file -/// (works when the plugin is in its source tree, e.g. `plugins/native/slint/`). -/// 2. `{stem}.plugin.yml` next to the `.so` file (works with the flat layout -/// produced by `just copy-plugins-native`, e.g. `.plugins/native/slint.plugin.yml`). +/// (works with the directory-per-plugin layout produced by +/// `just copy-plugins-native`, e.g. `.plugins/native/slint/plugin.yml`). +/// 2. `{stem}.plugin.yml` / `{stem}.plugin.yaml` next to the `.so` file +/// (fallback for any non-standard layouts). pub fn read_local_plugin_manifest( library_path: &std::path::Path, ) -> Option { diff --git a/apps/skit/src/plugins.rs b/apps/skit/src/plugins.rs index 8c3c4f04..8495d0a9 100644 --- a/apps/skit/src/plugins.rs +++ b/apps/skit/src/plugins.rs @@ -22,9 +22,18 @@ use tracing::{debug, info, warn}; use crate::{ marketplace::PluginKind, + plugin_assets::read_local_plugin_manifest, plugin_paths, plugin_records::{active_dir as plugin_active_dir, namespaced_kind as active_namespaced_kind}, }; + +/// Sentinel substring used in conflict-detection errors to distinguish +/// expected dedup skips from genuine failures. Referenced by both the +/// producer (`load_native_plugin` / `check_kind_conflict`) and the +/// consumer (`load_native_dir_plugins`). +const ERR_ALREADY_LOADED: &str = "already loaded"; +const ERR_ALREADY_REGISTERED: &str = "already registered"; + /// The type of plugin #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] #[serde(rename_all = "lowercase")] @@ -192,55 +201,180 @@ impl UnifiedPluginManager { }) } - /// Load all native plugins from the native directory. + /// Unified native plugin loader. + /// + /// Discovers native plugins from two sources, loaded in priority order: + /// + /// 1. **Active records** (`.plugins/active/*.json`) — marketplace-installed + /// bundles whose entrypoints live under `.plugins/bundles/`. + /// 2. **Directory bundles** (`.plugins/native//`) — local directory + /// layout where each subdirectory contains a `plugin.yml` manifest and + /// the plugin library. + /// + /// A plugin kind that was already loaded by an earlier source is skipped so + /// that marketplace versions always take precedence. /// - /// Scans the top-level directory for `.so`/`.dylib`/`.dll` files (flat - /// layout) and also one level of subdirectories (bundle layout, e.g. - /// `native/slint/libslint.so`). - fn load_native_plugins_from_dir(&mut self) -> Result> { + /// Errors reading plugin directories are logged but do not propagate; + /// callers cannot distinguish "no plugins found" from "directory unreadable" + /// except via the `warn!` log output. + fn load_all_native_plugins(&mut self) -> Vec { let mut summaries = Vec::new(); - info!("Loading native plugins..."); + info!("Loading native plugins (unified)..."); - let mut lib_paths: Vec = Vec::new(); + // Phase 1: active records (marketplace-installed bundles). + summaries.extend(self.load_active_plugin_records()); - for entry in std::fs::read_dir(&self.native_directory).with_context(|| { - format!("failed to read native plugin directory {}", self.native_directory.display()) - })? { - let entry = entry?; + // Phase 2: directory bundles from the native directory. + // Bare library files are warned about and skipped. + // Plugins already loaded in phase 1 are automatically skipped + // (load_native_plugin checks the map). + summaries.extend(self.load_native_dir_plugins()); + + summaries + } + + /// Phase 1: load marketplace-managed plugins from active records. + fn load_active_plugin_records(&mut self) -> Vec { + let active_dir = plugin_active_dir(&self.plugin_base_dir); + if !active_dir.exists() { + return Vec::new(); + } + + let base_real = std::fs::canonicalize(&self.plugin_base_dir).ok(); + let entries = match std::fs::read_dir(&active_dir) { + Ok(entries) => entries, + Err(err) => { + warn!( + error = %err, + dir = %active_dir.display(), + "Failed to read active plugins directory" + ); + return Vec::new(); + }, + }; + + let mut summaries = Vec::new(); + for entry in entries { + let entry = match entry { + Ok(e) => e, + Err(err) => { + warn!(error = %err, "Failed to read active plugin entry"); + continue; + }, + }; let path = entry.path(); + if path.extension().and_then(|ext| ext.to_str()) != Some("json") { + continue; + } + if let Some(summary) = self.load_active_plugin_record(&path, base_real.as_deref()) { + info!( + plugin = %summary.kind, + source = "active-record", + "Loaded marketplace plugin" + ); + summaries.push(summary); + } + } + summaries + } + + /// Phase 2: scan the native directory for directory bundles, loading + /// plugins from subdirectories of `.plugins/native//`. + fn load_native_dir_plugins(&mut self) -> Vec { + let mut lib_paths: Vec = Vec::new(); - if path.is_dir() { - // Scan one level of subdirectories for plugin bundles. - if let Ok(sub_entries) = std::fs::read_dir(&path) { - for sub_entry in sub_entries.flatten() { - let sub_path = sub_entry.path(); - if Self::is_native_lib(&sub_path) { - lib_paths.push(sub_path); + match std::fs::read_dir(&self.native_directory) { + Ok(entries) => { + for entry in entries.flatten() { + let path = entry.path(); + if !path.is_dir() { + // Warn about bare library files that will be ignored. + if Self::is_native_lib(&path) { + warn!( + file = ?path, + "Bare plugin file found in native directory; \ + move it into a subdirectory (e.g. .plugins/native//)" + ); + } + continue; + } + // Scan one level of subdirectories for plugin libraries. + if let Ok(sub_entries) = std::fs::read_dir(&path) { + for sub_entry in sub_entries.flatten() { + let sub_path = sub_entry.path(); + if Self::is_native_lib(&sub_path) { + lib_paths.push(sub_path); + } } } } - continue; - } - - if Self::is_native_lib(&path) { - lib_paths.push(path); - } + }, + Err(err) => { + warn!( + error = %err, + dir = %self.native_directory.display(), + "Failed to read native plugins directory" + ); + }, } + // Sort for deterministic load order when multiple bundles provide + // the same plugin kind. + lib_paths.sort(); + + // Skip any plugin whose kind is already loaded (from active records + // or an earlier library in this phase). + let mut summaries = Vec::new(); for path in lib_paths { match self.load_native_plugin(&path) { Ok(summary) => { - info!(plugin = %summary.kind, file = ?path, plugin_type = ?summary.plugin_type, "Loaded plugin from disk"); + info!( + plugin = %summary.kind, + file = ?path, + "Loaded native plugin from directory bundle" + ); summaries.push(summary); }, Err(err) => { - warn!(error = %err, file = ?path, "Failed to load native plugin from disk"); + // Uses the shared sentinel constants ERR_ALREADY_LOADED / + // ERR_ALREADY_REGISTERED produced by check_kind_conflict. + // A pre-check is not feasible because the plugin kind is + // only known after dlopen (LoadedNativePlugin::load). + let msg = err.to_string(); + if msg.contains(ERR_ALREADY_LOADED) || msg.contains(ERR_ALREADY_REGISTERED) { + // Read both versions from manifests so operators can + // spot outdated marketplace installs vs patched locals. + let local_manifest = read_local_plugin_manifest(&path); + let local_version = + local_manifest.as_ref().map_or("unknown", |m| m.version.as_str()); + let (loaded_kind, loaded_version) = local_manifest + .as_ref() + .and_then(|m| { + let kind = + streamkit_plugin_native::namespaced_kind(&m.node_kind).ok()?; + let ver = self + .plugins + .get(&kind) + .and_then(|p| p.version.as_deref()) + .unwrap_or("unknown"); + Some((kind, ver.to_owned())) + }) + .unwrap_or_else(|| ("unknown".into(), "unknown".into())); + warn!( + file = ?path, + plugin = %loaded_kind, + loaded_version = %loaded_version, + skipped_version = %local_version, + "Skipping local plugin (already loaded by higher-priority source)" + ); + } else { + warn!(error = %err, file = ?path, "Failed to load native plugin from disk"); + } }, } } - - Ok(summaries) + summaries } /// Returns `true` if the path looks like a native plugin library. @@ -278,33 +412,6 @@ impl UnifiedPluginManager { Ok(summaries) } - /// Load marketplace-managed plugins using active records. - fn load_active_plugins_from_dir(&mut self) -> Result> { - let active_dir = plugin_active_dir(&self.plugin_base_dir); - if !active_dir.exists() { - return Ok(Vec::new()); - } - - let base_real = std::fs::canonicalize(&self.plugin_base_dir).ok(); - let mut summaries = Vec::new(); - - for entry in std::fs::read_dir(&active_dir).with_context(|| { - format!("failed to read active plugins dir {}", active_dir.display()) - })? { - let entry = entry?; - let path = entry.path(); - if path.extension().and_then(|ext| ext.to_str()) != Some("json") { - continue; - } - - if let Some(summary) = self.load_active_plugin_record(&path, base_real.as_deref()) { - summaries.push(summary); - } - } - - Ok(summaries) - } - fn load_active_plugin_record( &mut self, record_path: &Path, @@ -407,16 +514,17 @@ impl UnifiedPluginManager { Some(entrypoint_path) } - /// Loads all existing plugins from both WASM and native directories. - /// Native plugins are loaded first as they are faster to initialize. + /// Loads all existing plugins from both native and WASM directories. + /// + /// Native plugins (including marketplace-installed bundles) are loaded + /// first via the unified loader, then WASM plugins. /// /// # Errors /// - /// Returns an error if the plugin directories cannot be read. + /// Returns an error if the WASM plugin directory cannot be read. /// Individual plugin load failures are logged but do not prevent other plugins from loading. pub fn load_existing(&mut self) -> Result> { - let mut summaries = self.load_active_plugins_from_dir()?; - summaries.extend(self.load_native_plugins_from_dir()?); + let mut summaries = self.load_all_native_plugins(); summaries.extend(self.load_wasm_plugins_from_dir()?); Ok(summaries) } @@ -681,22 +789,7 @@ impl UnifiedPluginManager { .with_context(|| format!("invalid plugin kind '{original_kind}'"))?; let categories = metadata.categories.clone(); - if self.plugins.contains_key(&kind) { - return Err(anyhow!( - "A plugin providing node '{original_kind}' (registered as '{kind}') is already loaded" - )); - } - - // Ensure we don't override an existing node definition - { - let registry = - self.engine.registry.read().map_err(|e| anyhow!("Registry lock poisoned: {e}"))?; - if registry.contains(&kind) { - return Err(anyhow!( - "Node kind '{kind}' is already registered; refusing to overwrite it with a plugin" - )); - } - } + self.check_kind_conflict(&kind, &original_kind)?; // Register with the engine's node registry { @@ -777,6 +870,8 @@ impl UnifiedPluginManager { "Failed to remove plugin file during unload" ); } + + self.try_remove_empty_plugin_dir(&file_path); } Ok(summary) @@ -806,6 +901,77 @@ impl UnifiedPluginManager { self.plugins_loaded_gauge.record(native_count, &[KeyValue::new("plugin_type", "native")]); } + /// Remove the parent directory of a plugin file if it is a now-empty + /// subdirectory inside the native plugins dir (directory-bundle layout). + fn try_remove_empty_plugin_dir(&self, file_path: &Path) { + if let Some(parent) = file_path.parent() { + if parent != self.native_directory && parent.starts_with(&self.native_directory) { + let is_empty = + parent.read_dir().map(|mut entries| entries.next().is_none()).unwrap_or(false); + if is_empty { + let _ = std::fs::remove_dir(parent); + } + } + } + } + + /// Checks whether the given plugin kind conflicts with an already-loaded + /// plugin or an already-registered node kind. + /// + /// Error messages intentionally contain `ERR_ALREADY_LOADED` / + /// `ERR_ALREADY_REGISTERED` so that `load_native_dir_plugins` can + /// distinguish expected dedup skips from genuine failures. + fn check_kind_conflict(&self, kind: &str, original_kind: &str) -> Result<()> { + if self.plugins.contains_key(kind) { + return Err(anyhow!( + "A plugin providing node '{original_kind}' (registered as '{kind}') is {ERR_ALREADY_LOADED}" + )); + } + + { + let registry = + self.engine.registry.read().map_err(|e| anyhow!("Registry lock poisoned: {e}"))?; + if registry.contains(kind) { + return Err(anyhow!( + "Node kind '{kind}' is {ERR_ALREADY_REGISTERED}; refusing to overwrite it with a plugin" + )); + } + } + + Ok(()) + } + + /// Probes a native plugin library to discover its kind and checks for + /// conflicts with already-loaded plugins, without registering anything. + /// + /// Used by the upload path to detect kind conflicts *before* moving the + /// uploaded file to its final location, preventing accidental overwriting + /// of an existing plugin's library. + fn check_native_upload_conflict(&self, probe_path: &Path) -> Result<()> { + // Set executable permissions so dlopen can load the probe file. + // load_from_written_path sets permissions again on the final path + // after the move — the two calls operate on different files. + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mut perms = std::fs::metadata(probe_path)?.permissions(); + perms.set_mode(0o755); + std::fs::set_permissions(probe_path, perms)?; + } + + let plugin = LoadedNativePlugin::load(probe_path) + .with_context(|| format!("failed to probe native plugin {}", probe_path.display()))?; + let metadata = plugin.metadata(); + let original_kind = metadata.kind.clone(); + let kind = streamkit_plugin_native::namespaced_kind(&original_kind) + .with_context(|| format!("invalid plugin kind '{original_kind}'"))?; + // Close the library before the file is moved and re-opened from the + // final path. + drop(plugin); + + self.check_kind_conflict(&kind, &original_kind) + } + /// Saves raw plugin bytes into the managed directory and loads the resulting plugin. /// Automatically detects plugin type based on file extension. /// @@ -816,14 +982,55 @@ impl UnifiedPluginManager { /// - The plugin file cannot be written to disk /// - The plugin fails to load after being written /// - On Unix systems, setting executable permissions fails - #[allow(dead_code)] // Useful for non-streaming callers; HTTP uses `load_from_temp_file`. + #[allow(dead_code)] // Public API for non-streaming callers; HTTP handler uses load_from_temp_file. pub fn load_from_bytes(&mut self, file_name: &str, bytes: &[u8]) -> Result { let (target_path, plugin_type) = self.validate_plugin_upload_target(file_name)?; - std::fs::write(&target_path, bytes) - .with_context(|| format!("failed to write plugin file {}", target_path.display()))?; + // Track whether we actually placed a file at target_path so the error + // handler only cleans up files *we* created, not a pre-existing plugin. + let mut file_placed = false; + + let result = (|| { + if plugin_type == PluginType::Native { + // Write to a temporary file first, probe for kind conflicts, + // then move to the final location. This mirrors the pattern + // used by load_from_temp_file and prevents overwriting an + // existing plugin's library when the kind is already loaded. + let tmp_path = target_path.with_extension("tmp"); + std::fs::write(&tmp_path, bytes).with_context(|| { + format!("failed to write temp plugin file {}", tmp_path.display()) + })?; + + if let Err(e) = self.check_native_upload_conflict(&tmp_path) { + let _ = std::fs::remove_file(&tmp_path); + return Err(e); + } + + std::fs::rename(&tmp_path, &target_path).map_err(|e| { + let _ = std::fs::remove_file(&tmp_path); + anyhow!( + "failed to move temp plugin file from {} to {}: {e}", + tmp_path.display(), + target_path.display() + ) + })?; + } else { + std::fs::write(&target_path, bytes).with_context(|| { + format!("failed to write plugin file {}", target_path.display()) + })?; + } + file_placed = true; - self.load_from_written_path(plugin_type, target_path) + self.load_from_written_path(plugin_type, target_path.clone()) + })(); + + if result.is_err() { + if file_placed { + let _ = std::fs::remove_file(&target_path); + } + self.try_remove_empty_plugin_dir(&target_path); + } + result } /// Moves an already-written plugin file into the managed directory and loads it. @@ -845,37 +1052,87 @@ impl UnifiedPluginManager { ) -> Result { let (target_path, plugin_type) = self.validate_plugin_upload_target(file_name)?; - let meta = std::fs::metadata(temp_path) - .with_context(|| format!("failed to stat temp plugin file {}", temp_path.display()))?; - if !meta.is_file() { - return Err(anyhow!("temp plugin path is not a file: {}", temp_path.display())); - } + // Track whether we actually placed a file at target_path so the error + // handler only cleans up files *we* created, not a pre-existing plugin. + let mut file_placed = false; - // Prefer atomic move; fall back to copy+remove for cross-device temp dirs. - if let Err(e) = std::fs::rename(temp_path, &target_path) { - debug!( - error = %e, - from = %temp_path.display(), - to = %target_path.display(), - "rename failed; falling back to copy+remove" - ); - std::fs::copy(temp_path, &target_path).with_context(|| { - format!( - "failed to copy temp plugin file from {} to {}", - temp_path.display(), - target_path.display() - ) + let result = (|| { + let meta = std::fs::metadata(temp_path).with_context(|| { + format!("failed to stat temp plugin file {}", temp_path.display()) })?; - let _ = std::fs::remove_file(temp_path); - } + if !meta.is_file() { + return Err(anyhow!("temp plugin path is not a file: {}", temp_path.display())); + } - match self.load_from_written_path(plugin_type, target_path.clone()) { - Ok(summary) => Ok(summary), - Err(e) => { + if plugin_type == PluginType::Native { + // Move the temp file into the plugin subdirectory *before* + // probing. The original temp file may live on a noexec mount + // (e.g. /tmp), which would cause dlopen to fail even though + // .plugins/native/ is fine. Using a .tmp extension keeps it + // separate from the final target so a pre-existing plugin is + // never overwritten until the probe passes. + let probe_path = target_path.with_extension("tmp"); + if let Err(e) = std::fs::rename(temp_path, &probe_path) { + debug!( + error = %e, + from = %temp_path.display(), + to = %probe_path.display(), + "rename to probe path failed; falling back to copy+remove" + ); + std::fs::copy(temp_path, &probe_path).with_context(|| { + format!( + "failed to copy temp plugin file from {} to {}", + temp_path.display(), + probe_path.display() + ) + })?; + let _ = std::fs::remove_file(temp_path); + } + + if let Err(e) = self.check_native_upload_conflict(&probe_path) { + let _ = std::fs::remove_file(&probe_path); + return Err(e); + } + + std::fs::rename(&probe_path, &target_path).map_err(|e| { + let _ = std::fs::remove_file(&probe_path); + anyhow!( + "failed to move probe file from {} to {}: {e}", + probe_path.display(), + target_path.display() + ) + })?; + } else { + // WASM plugins don't need dlopen probing; move directly. + if let Err(e) = std::fs::rename(temp_path, &target_path) { + debug!( + error = %e, + from = %temp_path.display(), + to = %target_path.display(), + "rename failed; falling back to copy+remove" + ); + std::fs::copy(temp_path, &target_path).with_context(|| { + format!( + "failed to copy temp plugin file from {} to {}", + temp_path.display(), + target_path.display() + ) + })?; + let _ = std::fs::remove_file(temp_path); + } + } + file_placed = true; + + self.load_from_written_path(plugin_type, target_path.clone()) + })(); + + if result.is_err() { + if file_placed { let _ = std::fs::remove_file(&target_path); - Err(e) - }, + } + self.try_remove_empty_plugin_dir(&target_path); } + result } fn validate_plugin_upload_target(&self, file_name: &str) -> Result<(PathBuf, PluginType)> { @@ -909,7 +1166,24 @@ impl UnifiedPluginManager { let (target_path, plugin_type) = match extension { Some("wasm") => (self.wasm_directory.join(sanitized), PluginType::Wasm), Some("so" | "dylib" | "dll") => { - (self.native_directory.join(sanitized), PluginType::Native) + // Place native plugins inside a subdirectory derived from the + // library stem (e.g. `libgain.so` → `native/gain/libgain.so`). + let stem = Path::new(sanitized) + .file_stem() + .and_then(|s| s.to_str()) + .map(|s| s.strip_prefix("lib").unwrap_or(s)) + .filter(|s| !s.is_empty()) + .unwrap_or(sanitized); + if stem == "." || stem == ".." { + return Err(anyhow!( + "Cannot derive a valid plugin directory name from '{sanitized}'" + )); + } + let subdir = self.native_directory.join(stem); + std::fs::create_dir_all(&subdir).with_context(|| { + format!("failed to create native plugin directory {}", subdir.display()) + })?; + (subdir.join(sanitized), PluginType::Native) }, _ => { return Err(anyhow!( diff --git a/apps/skit/tests/plugin_integration_test.rs b/apps/skit/tests/plugin_integration_test.rs index c0e7a87e..1082c924 100644 --- a/apps/skit/tests/plugin_integration_test.rs +++ b/apps/skit/tests/plugin_integration_test.rs @@ -773,3 +773,98 @@ async fn test_unload_plugin_after_pipeline_use() { println!("✅ Server healthy after unload"); server.shutdown().await; } + +/// Uploading a plugin whose kind is already loaded must be rejected without +/// destroying the existing plugin's library file on disk. +#[tokio::test] +async fn test_duplicate_upload_preserves_existing_plugin() { + let _ = tracing_subscriber::fmt::try_init(); + let _permit = acquire_test_permit().await; + + let Some(server) = TestServer::start().await else { + eprintln!("Skipping plugin integration tests: local TCP bind not permitted"); + return; + }; + let plugin_path = ensure_gain_plugin_built().await; + let plugin_bytes = fs::read(&plugin_path).await.expect("Failed to read plugin file"); + let file_name = plugin_path.file_name().unwrap().to_string_lossy().to_string(); + let client = reqwest::Client::new(); + let url = format!("http://{}/api/v1/plugins", server.addr); + + // First upload — should succeed. + let form = multipart::Form::new() + .part("plugin", multipart::Part::bytes(plugin_bytes.clone()).file_name(file_name.clone())); + let response = timeout(Duration::from_secs(10), client.post(&url).multipart(form).send()) + .await + .expect("Upload timed out") + .expect("Failed to upload plugin"); + assert_eq!(response.status(), StatusCode::CREATED, "First upload should succeed"); + println!("✅ First upload succeeded"); + + // Locate the installed plugin file on disk so we can verify it survives. + #[allow(clippy::used_underscore_binding)] + let plugins_dir = server._temp_dir.path().join("plugins/native"); + let installed_file = find_plugin_file(&plugins_dir).await; + let original_metadata = + fs::metadata(&installed_file).await.expect("Failed to stat installed plugin"); + let original_len = original_metadata.len(); + assert!(original_len > 0, "Installed plugin file should not be empty"); + println!("✅ Plugin installed at {}", installed_file.display()); + + // Second upload of the same plugin — should be rejected. + let form2 = multipart::Form::new() + .part("plugin", multipart::Part::bytes(plugin_bytes).file_name(file_name)); + let response2 = timeout(Duration::from_secs(10), client.post(&url).multipart(form2).send()) + .await + .expect("Second upload timed out") + .expect("Failed to send second upload"); + assert_eq!( + response2.status(), + StatusCode::UNPROCESSABLE_ENTITY, + "Second upload of same plugin kind should be rejected" + ); + println!("✅ Second upload correctly rejected"); + + // Verify the original plugin file is still on disk and intact. + let after_metadata = fs::metadata(&installed_file) + .await + .expect("Plugin file should still exist after rejected duplicate upload"); + assert_eq!( + after_metadata.len(), + original_len, + "Plugin file size should be unchanged after rejected duplicate" + ); + println!("✅ Original plugin file preserved"); + + // Verify the plugin is still loaded and functional. + let list_url = format!("http://{}/api/v1/plugins", server.addr); + let list_response = client.get(&list_url).send().await.expect("Failed to list plugins"); + assert_eq!(list_response.status(), StatusCode::OK); + let plugins: Vec = + list_response.json().await.expect("Failed to parse plugins list"); + assert_eq!(plugins.len(), 1, "Plugin should still be loaded"); + assert_eq!(plugins[0]["kind"], "plugin::native::gain"); + println!("✅ Plugin still loaded and listed"); + + server.shutdown().await; +} + +/// Find the first native plugin library file under the given directory tree. +async fn find_plugin_file(dir: &std::path::Path) -> std::path::PathBuf { + let mut stack = vec![dir.to_path_buf()]; + while let Some(current) = stack.pop() { + let mut entries = fs::read_dir(¤t).await.expect("Failed to read dir"); + while let Some(entry) = entries.next_entry().await.expect("Failed to read entry") { + let path = entry.path(); + if path.is_dir() { + stack.push(path); + } else if path.extension().and_then(|e| e.to_str()) == Some("so") + || path.extension().and_then(|e| e.to_str()) == Some("dylib") + || path.extension().and_then(|e| e.to_str()) == Some("dll") + { + return path; + } + } + } + panic!("No plugin library file found under {}", dir.display()); +} diff --git a/docs/src/content/docs/architecture/plugin-bundle-convention.md b/docs/src/content/docs/architecture/plugin-bundle-convention.md index c7154e86..f5fd82d7 100644 --- a/docs/src/content/docs/architecture/plugin-bundle-convention.md +++ b/docs/src/content/docs/architecture/plugin-bundle-convention.md @@ -2,39 +2,20 @@ # SPDX-FileCopyrightText: © 2025 StreamKit Contributors # SPDX-License-Identifier: MPL-2.0 title: Plugin Bundle Convention -description: Proposed directory layout for plugin bundles +description: Directory layout for plugin bundles --- -> [!NOTE] -> This is a **design proposal** for unifying the plugin directory layout. -> The migration is incremental — existing bare `.so` plugins continue to work. -> See [Issue #254](https://github.com/streamer45/streamkit/issues/254) for -> discussion. - ## Motivation -StreamKit currently has two plugin layouts: - -| Source | Layout | Example path | -|--------|--------|-------------| -| Local / dev | Bare `.so` in a flat directory | `.plugins/native/libslint.so` | -| Marketplace | Extracted bundle directory | `.plugins/bundles/slint/0.1.0/libslint.so` | - -The discrepancy causes problems: +StreamKit plugins can come from multiple sources (local builds, marketplace +installs, manual uploads). A consistent directory layout ensures that the +server can always discover a plugin's manifest — and therefore its asset +type declarations, model requirements, and other metadata — regardless of +how the plugin was installed. -1. **Asset types lost on restart** — marketplace bundles ship `manifest.json` - but the local loader expects `plugin.yml`. Without a YAML manifest next to - the library, `collect_plugin_asset_specs` cannot rediscover asset - declarations after a server restart. -2. **No room for extras** — a bare `.so` cannot carry bundled assets, custom - UI files, or license metadata. -3. **Two code paths** — `load_native_plugins_from_dir` scans for `.so` files - while `load_active_plugins_from_dir` reads JSON records. Both should - converge on the same convention. +## Convention -## Proposed convention - -Every plugin — whether local or marketplace — should be a **directory** +Every plugin — whether local or marketplace — is a **directory** (a.k.a. "bundle") with a well-known internal layout: ``` @@ -65,58 +46,79 @@ additional `bundle` block for download metadata. ### Discovery rules -`read_local_plugin_manifest` already searches for: +`read_local_plugin_manifest` searches for: 1. `plugin.yml` / `plugin.yaml` in the same directory as the `.so` -2. `{stem}.plugin.yml` next to the `.so` (flat layout fallback) +2. `{stem}.plugin.yml` / `{stem}.plugin.yaml` next to the `.so` (fallback) + +The key requirement is that a `plugin.yml` file exists next to the +entrypoint library. -No changes to the discovery logic are needed. The key requirement is that -a `plugin.yml` file exists next to the entrypoint library. +## Unified loader -## Current state & incremental steps +`load_all_native_plugins` discovers native plugins from two sources, +loaded in priority order: -### Already done +1. **Active records** (`.plugins/active/*.json`) — marketplace-installed + bundles whose entrypoints live under `.plugins/bundles/`. +2. **Directory bundles** (`.plugins/native//`) — local directory + layout where each subdirectory contains the plugin library and a + `plugin.yml` manifest. + +A plugin kind that was already loaded by an earlier source is skipped so +that marketplace versions always take precedence. Duplicate-kind skips +are logged at `debug` level; genuine load failures are logged at `warn`. + +## Build tooling + +`just copy-plugins-native` copies built plugins into the directory layout: + +``` +.plugins/native/slint/ +├── libslint.so +└── plugin.yml +``` + +Each plugin gets its own subdirectory under `.plugins/native/`. The +`plugin.yml` is copied from the plugin's source tree +(`plugins/native//plugin.yml`). + +## HTTP uploads + +Plugins uploaded via `POST /api/v1/plugins` are also placed into the +directory layout. The subdirectory name is derived from the library stem +(e.g. `libgain.so` → `.plugins/native/gain/libgain.so`). On unload the +subdirectory is cleaned up if empty. + +## Implemented - **Marketplace bundles write `plugin.yml` on install** — during - `handle_bundle_install`, the marketplace installer now serializes the - verified manifest as `plugin.yml` next to the entrypoint. This ensures + `handle_bundle_install`, the marketplace installer serializes the + verified manifest as `plugin.yml` next to the entrypoint. This ensures `collect_plugin_asset_specs` finds asset declarations on restart. -- **`just copy-plugins-native` copies manifests** — when copying built - plugins from `target/` to `.plugins/native/`, the recipe also copies - `plugin.yml` as `{stem}.plugin.yml` alongside the `.so` file. +- **`just copy-plugins-native` uses directory layout** — when copying + built plugins from `target/` to `.plugins/native/`, the recipe creates + a directory per plugin (e.g. `.plugins/native/slint/`) containing both + the library and `plugin.yml`. -### Future steps (not yet implemented) +- **Unified loader** — `load_all_native_plugins` merges the former + `load_active_plugins_from_dir` and `load_native_plugins_from_dir` into + a single pass that handles active records and directory bundles with + correct priority ordering. -1. **Local plugin directory layout** — allow placing local plugins as - directories under `.plugins/native//` instead of bare `.so` - files. `load_native_plugins_from_dir` would check for directories - containing a `plugin.yml` before falling back to bare `.so` scanning. +- **HTTP uploads use directory layout** — uploaded native plugins are + placed into a subdirectory of `.plugins/native/` matching the library + stem, consistent with the build-time layout. -2. **Unified loader** — merge `load_native_plugins_from_dir` and - `load_active_plugins_from_dir` into a single pass that handles both - directory-based plugins (with `plugin.yml`) and legacy bare `.so` files. +## Future steps -3. **Bundled system assets** — when a plugin bundle includes a `samples/` +1. **Bundled system assets** — when a plugin bundle includes a `samples/` directory, the asset registry should resolve `system_dir` relative to the bundle root. This allows marketplace plugins to ship default assets (e.g. example `.slint` files) without requiring separate downloads. -4. **Custom UI** — plugins could ship frontend components (e.g. +2. **Custom UI** — plugins could ship frontend components (e.g. `ui/config-panel.jsx`) that the dashboard loads at runtime for node-specific configuration UIs. - -## Migration path - -The migration is **backward-compatible**: - -- Bare `.so` files continue to work. `load_native_plugins_from_dir` - keeps its current scanning logic as a fallback. -- Marketplace bundles already ARE directories — they just need a - `plugin.yml` written during installation (now done). -- The `{stem}.plugin.yml` naming convention bridges the flat layout used - by `just copy-plugins-native`. - -No breaking changes are required. New features (directory-based local -plugins, bundled assets) are additive. diff --git a/justfile b/justfile index 71854446..71cddf27 100644 --- a/justfile +++ b/justfile @@ -1011,41 +1011,45 @@ copy-plugins-native: PLUGINS_TARGET="{{plugins_target_dir}}" - # Examples (gain-native has its own target dir, not shared) - cp examples/plugins/gain-native/target/release/libgain_plugin_native.* .plugins/native/ 2>/dev/null || true + # Helper: copy a built plugin into the directory-per-plugin layout. + # copy_plugin + # Creates .plugins/native// with the .so and plugin.yml. + copy_plugin() { + local id="$1" stem="$2" src_dir="$3" + local dest=".plugins/native/$id" + mkdir -p "$dest" - # Official native plugins (shared target dir) - for name in whisper kokoro piper matcha vad sensevoice nllb helsinki supertonic slint; do for f in \ - "$PLUGINS_TARGET"/release/lib"$name".so \ - "$PLUGINS_TARGET"/release/lib"$name".so.* \ - "$PLUGINS_TARGET"/release/lib"$name".dylib \ - "$PLUGINS_TARGET"/release/"$name".dll + "$src_dir"/release/lib"$stem".so \ + "$src_dir"/release/lib"$stem".so.* \ + "$src_dir"/release/lib"$stem".dylib \ + "$src_dir"/release/"$stem".dll do if [[ -f "$f" ]]; then - cp -f "$f" .plugins/native/ + cp -f "$f" "$dest/" fi done - # Copy plugin.yml manifest alongside the .so so asset types can be + + # Copy plugin.yml into the bundle directory so asset types can be # discovered at runtime (see plugin_assets::read_local_plugin_manifest). - if [[ -f "plugins/native/$name/plugin.yml" ]]; then - cp -f "plugins/native/$name/plugin.yml" ".plugins/native/${name}.plugin.yml" - fi - done - for f in \ - "$PLUGINS_TARGET"/release/libpocket_tts.so \ - "$PLUGINS_TARGET"/release/libpocket_tts.so.* \ - "$PLUGINS_TARGET"/release/libpocket_tts.dylib \ - "$PLUGINS_TARGET"/release/pocket_tts.dll - do - if [[ -f "$f" ]]; then - cp -f "$f" .plugins/native/ + if [[ -f "plugins/native/$id/plugin.yml" ]]; then + cp -f "plugins/native/$id/plugin.yml" "$dest/plugin.yml" fi + } + + # Examples (gain-native has its own target dir, not shared) + copy_plugin "gain-native" "gain_plugin_native" "examples/plugins/gain-native/target" + + # Official native plugins (shared target dir). + # For most plugins the lib stem matches the plugin id. + for name in whisper kokoro piper matcha vad sensevoice nllb helsinki supertonic slint; do + copy_plugin "$name" "$name" "$PLUGINS_TARGET" done - # Copy pocket-tts plugin.yml for consistency with the main loop above. - if [[ -f "plugins/native/pocket-tts/plugin.yml" ]]; then - cp -f "plugins/native/pocket-tts/plugin.yml" ".plugins/native/pocket-tts.plugin.yml" - fi + + # Plugins whose lib stem differs from the plugin id. + copy_plugin "pocket-tts" "pocket_tts" "$PLUGINS_TARGET" + copy_plugin "aac-encoder" "aac_encoder" "$PLUGINS_TARGET" + echo "✓ Native plugins copied to .plugins/native/" # --- License Headers (REUSE) ---