Skip to content

Commit 4552035

Browse files
staging-devin-ai-integration[bot]streamkit-devinstreamer45
authored
feat(plugins): directory-per-plugin layout and unified native loader (#272)
* feat(plugins): directory-per-plugin layout and unified native loader Update `just copy-plugins-native` to output a directory-per-plugin structure under `.plugins/native/<id>/` instead of the flat layout. Each plugin now gets its own subdirectory containing the library and `plugin.yml` manifest. Also adds aac-encoder to the copy loop. Merge `load_active_plugins_from_dir` and `load_native_plugins_from_dir` into a single `load_all_native_plugins` method that discovers plugins from three sources in priority order: 1. Active records (.plugins/active/*.json) — marketplace bundles 2. Directory bundles (.plugins/native/<id>/) — local directory layout 3. Bare .so files (.plugins/native/*.so) — legacy flat fallback Plugins already loaded by a higher-priority source are skipped. Update bundle convention docs to reflect implemented changes. Closes #254 Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com> * fix(plugins): address review feedback on unified loader - Use copy_plugin helper for gain-native instead of raw cp + glob, ensuring consistent handling of versioned .so.* and plugin.yml. - Log a warning when the native plugins directory cannot be read, matching the behavior of load_active_plugin_records. - Distinguish 'already loaded' skips (debug level) from genuine load failures (warn level) to avoid misleading log output. - Sort discovered library paths for deterministic load order. - Add comment explaining the source-label heuristic. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com> * refactor(plugins): drop bare .so fallback, require directory layout Since we are pre-1.0 (v0.x), enforce the directory-per-plugin layout everywhere instead of maintaining a legacy flat-file fallback: - load_native_dir_plugins now only scans subdirectories of .plugins/native/, ignoring top-level .so files. - validate_plugin_upload_target places HTTP-uploaded native plugins into a subdirectory derived from the library stem (e.g. libgain.so → .plugins/native/gain/libgain.so). - unload_plugin cleans up the parent directory if empty after removing the .so file. - Updated docs and code comments to reflect the two-source model (active records + directory bundles). Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com> * fix(plugins): handle empty stem edge case and warn on bare .so files - Guard against lib.so/lib.dylib/lib.dll uploads where stripping the 'lib' prefix yields an empty stem, which would place the file flat in the native directory. Now falls back to the full filename. - Log a warning when bare .so files are found directly in the native directory, guiding users to move them into subdirectories. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com> * fix(plugins): fix stale comment and clean up orphaned dirs on failed upload - Update comment from 'Phase 2 & 3' to 'Phase 2' since bare .so files are no longer loaded. - Clean up the subdirectory when a native plugin upload fails to load, mirroring the cleanup logic in unload_plugin. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com> * refactor(plugins): extract try_remove_empty_plugin_dir helper Deduplicate the directory cleanup logic used by unload_plugin, load_from_bytes, and load_from_temp_file into a shared helper method. Also fixes load_from_bytes which was missing cleanup on failure. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com> * fix(plugins): validate stem edge cases and document error-string coupling - Reject stems that resolve to '.' or '..' (e.g. lib..so) which would create problematic directories. - Document the coupling between load_native_plugin's error messages and load_native_dir_plugins' string-matching control flow, at both the producer and consumer sites. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com> * fix(plugins): ensure directory cleanup on all upload error paths Use closure-based error handling in load_from_bytes and load_from_temp_file so that try_remove_empty_plugin_dir runs on every error path after validate_plugin_upload_target creates the subdirectory, not just when load_from_written_path fails. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com> * fix(plugins): probe native plugin kind before overwriting on upload When uploading a native plugin whose kind is already loaded, the old code would move the new file to the target path (overwriting the existing library), then discover the conflict and delete the file — destroying the original plugin's .so on disk. Now load_from_temp_file probes the plugin kind from the temp file (via check_native_upload_conflict) before touching the final location. If the kind conflicts, the upload is rejected immediately and the existing library file is never overwritten. Also adds an integration test (test_duplicate_upload_preserves_existing_plugin) that verifies: duplicate upload is rejected with 422, the original .so file is preserved on disk, and the plugin remains loaded. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com> * fix(plugins): only clean up target_path if file was actually placed there The error handler in load_from_temp_file was unconditionally deleting target_path on any error. When the conflict probe (check_native_upload_conflict) fails *before* the move/copy, target_path points to the pre-existing plugin's library — deleting it destroys the original. Track placement with a file_placed flag so we only remove files we created. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com> * fix(plugins): load_from_bytes writes to temp file before probing conflicts Mirrors the load_from_temp_file pattern: write bytes to a .tmp file, run check_native_upload_conflict on it, then rename to the final path. This prevents overwriting an existing plugin's library file when the uploaded kind is already loaded. Also adds the file_placed guard so the error handler only cleans up files that were actually placed at target_path. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com> * refactor(plugins): address review feedback on conflict detection and cleanup - Extract shared check_kind_conflict() helper used by both load_native_plugin and check_native_upload_conflict, eliminating duplicated conflict-check logic. - Extract ERR_ALREADY_LOADED / ERR_ALREADY_REGISTERED as shared constants, replacing fragile string literals in both the producer (check_kind_conflict) and consumer (load_native_dir_plugins). - Fix temp file leak in load_from_bytes: clean up .tmp file on rename failure instead of letting it accumulate on disk. - Fix empty subdirectory leak: call try_remove_empty_plugin_dir unconditionally on error (not only when file_placed is true), so subdirectories created by validate_plugin_upload_target are cleaned up even when the conflict check fails before placement. - Fix stale 'Phase 2 & 3' comment (bare .so fallback was removed). - Clarify redundant-looking permissions comment (probe file vs final file are different). - Improve dead_code annotation rationale on load_from_bytes. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com> * feat(plugins): log both versions when skipping duplicate plugin Bump the dedup-skip log from debug to warn and include both the loaded (marketplace) version and the skipped (local) version read from plugin.yml manifests. This helps operators spot cases where a marketplace install is outdated relative to a locally patched plugin. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com> * fix(plugins): avoid probing native uploads from noexec temp mounts Move the uploaded temp file into the plugin subdirectory (.tmp extension) before dlopen-probing it. The original temp file may reside on a noexec mount (e.g. /tmp on hardened hosts), causing dlopen to fail even though .plugins/native/ would work fine. The probe file is cleaned up on conflict or rename failure, and an existing plugin's library is never overwritten until the probe passes. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com> --------- Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-authored-by: StreamKit Devin <devin@streamkit.dev> Co-authored-by: Claudio Costa <cstcld91@gmail.com>
1 parent 2b29d5d commit 4552035

File tree

5 files changed

+574
-198
lines changed

5 files changed

+574
-198
lines changed

apps/skit/src/plugin_assets.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -898,9 +898,10 @@ pub fn plugin_assets_router() -> Router<Arc<AppState>> {
898898
///
899899
/// Searches for the manifest in two locations:
900900
/// 1. `plugin.yml` / `plugin.yaml` in the same directory as the `.so` file
901-
/// (works when the plugin is in its source tree, e.g. `plugins/native/slint/`).
902-
/// 2. `{stem}.plugin.yml` next to the `.so` file (works with the flat layout
903-
/// produced by `just copy-plugins-native`, e.g. `.plugins/native/slint.plugin.yml`).
901+
/// (works with the directory-per-plugin layout produced by
902+
/// `just copy-plugins-native`, e.g. `.plugins/native/slint/plugin.yml`).
903+
/// 2. `{stem}.plugin.yml` / `{stem}.plugin.yaml` next to the `.so` file
904+
/// (fallback for any non-standard layouts).
904905
pub fn read_local_plugin_manifest(
905906
library_path: &std::path::Path,
906907
) -> Option<crate::marketplace::PluginManifest> {

0 commit comments

Comments
 (0)