From 69d07dba84411db33bf1f5191a69f80e5069c6ea Mon Sep 17 00:00:00 2001 From: Ogulcan Celik Date: Mon, 20 Jul 2026 22:58:49 +0300 Subject: fix: make plugin state global across sessions refs #1174 --- src/persist/plugin_registry.rs | 107 +++++++++++++++++++++++++---------------- 1 file changed, 65 insertions(+), 42 deletions(-) (limited to 'src/persist/plugin_registry.rs') diff --git a/src/persist/plugin_registry.rs b/src/persist/plugin_registry.rs index 5cd1a1f5..04cdd845 100644 --- a/src/persist/plugin_registry.rs +++ b/src/persist/plugin_registry.rs @@ -1,3 +1,4 @@ +use std::fs::OpenOptions; use std::path::{Path, PathBuf}; use tracing::warn; @@ -5,9 +6,29 @@ use tracing::warn; use crate::api::schema::InstalledPluginInfo; pub const MANIFEST_UNAVAILABLE_WARNING_PREFIX: &str = "manifest unavailable: "; +const REGISTRY_LOCK_FILE: &str = ".plugins.lock"; fn registry_path() -> PathBuf { - crate::session::data_dir().join("plugins.json") + crate::config::config_dir().join("plugins.json") +} + +fn registry_lock_path() -> PathBuf { + crate::config::config_dir().join(REGISTRY_LOCK_FILE) +} + +fn with_registry_lock(operation: impl FnOnce() -> std::io::Result) -> std::io::Result { + let lock_path = registry_lock_path(); + if let Some(parent) = lock_path.parent() { + std::fs::create_dir_all(parent)?; + } + let lock = OpenOptions::new() + .create(true) + .truncate(false) + .read(true) + .write(true) + .open(lock_path)?; + lock.lock()?; + operation() } fn save_json_to_path(path: &Path, value: &T) -> std::io::Result<()> { @@ -16,7 +37,7 @@ fn save_json_to_path(path: &Path, value: &T) -> st } let json = serde_json::to_string_pretty(value)?; let tmp_path = path.with_extension("json.tmp"); - std::fs::write(&tmp_path, &json)?; + std::fs::write(&tmp_path, json)?; #[cfg(windows)] if path.exists() { if let Err(err) = std::fs::remove_file(path) { @@ -31,42 +52,58 @@ fn save_json_to_path(path: &Path, value: &T) -> st Ok(()) } -/// Atomically write `plugins.json` next to `session.json`. -pub fn save(plugins: &[InstalledPluginInfo]) -> std::io::Result<()> { - let path = registry_path(); - save_to_path(&path, plugins) -} - pub fn save_to_path(path: &Path, plugins: &[InstalledPluginInfo]) -> std::io::Result<()> { save_json_to_path(path, plugins) } -/// Load `plugins.json`. Returns an empty vec on any failure so a corrupt or -/// missing file never blocks server startup. -pub fn load() -> Vec { - load_from_path(®istry_path()) +pub fn update( + mutation: impl FnOnce(&mut Vec) -> T, +) -> std::io::Result<(T, Vec)> { + with_registry_lock(|| { + let mut plugins = load_from_path_strict(®istry_path())?; + let result = mutation(&mut plugins); + plugins.sort_by(|left, right| left.plugin_id.cmp(&right.plugin_id)); + save_to_path(®istry_path(), &plugins)?; + Ok((result, plugins)) + }) } -pub fn load_from_path(path: &Path) -> Vec { - if !path.exists() { - return Vec::new(); - } - let content = match std::fs::read_to_string(path) { - Ok(c) => c, +pub fn try_load() -> std::io::Result> { + with_registry_lock(|| load_from_path_strict(®istry_path())) +} + +/// Load the global registry. Returns an empty vec on failure so a corrupt or +/// missing file never blocks server startup; mutations still use strict reads. +pub fn load() -> Vec { + match try_load() { + Ok(plugins) => plugins, Err(err) => { - warn!(path = %path.display(), err = %err, "failed to read plugin registry"); - return Vec::new(); + warn!(path = %registry_path().display(), err = %err, "failed to load plugin registry"); + Vec::new() } - }; - match serde_json::from_str::>(&content) { + } +} + +#[cfg(test)] +pub fn load_from_path(path: &Path) -> Vec { + match load_from_path_strict(path) { Ok(entries) => entries, Err(err) => { - warn!(path = %path.display(), err = %err, "failed to parse plugin registry, starting with empty registry"); + warn!(path = %path.display(), err = %err, "failed to read plugin registry"); Vec::new() } } } +fn load_from_path_strict(path: &Path) -> std::io::Result> { + if !path.exists() { + return Ok(Vec::new()); + } + let content = std::fs::read_to_string(path)?; + serde_json::from_str::>(&content) + .map_err(|err| std::io::Error::new(std::io::ErrorKind::InvalidData, err)) +} + /// Re-read each entry's manifest from disk using the provided reload function. /// /// If the manifest parses successfully, replace cached fields but keep the @@ -160,10 +197,12 @@ mod tests { if let Some(parent) = path.parent() { std::fs::create_dir_all(parent).unwrap(); } - std::fs::write(&path, b"this is not valid json {{{{").unwrap(); + let corrupt = b"this is not valid json {{{{"; + std::fs::write(&path, corrupt).unwrap(); - let loaded = load_from_path(&path); - assert!(loaded.is_empty()); + assert!(load_from_path_strict(&path).is_err()); + assert!(load_from_path(&path).is_empty()); + assert_eq!(std::fs::read(path).unwrap(), corrupt); } #[test] @@ -230,22 +269,6 @@ mod tests { assert!(result[0].warnings.is_empty()); } - #[test] - fn atomic_write_temp_file_is_cleaned_up_on_rename_failure() { - // Write to a path whose parent does not yet exist, then verify the - // tmp file is removed when the write fails mid-way. Here we just - // confirm a successful write leaves no .tmp file behind. - let path = temp_registry_path("cleanup"); - save_to_path(&path, &[sample_plugin("example.cleanup")]).unwrap(); - - let tmp = path.with_extension("json.tmp"); - assert!( - !tmp.exists(), - "tmp file should be cleaned up after successful rename" - ); - assert!(path.exists()); - } - #[test] fn save_replaces_existing_registry_file() { let path = temp_registry_path("replace-existing"); -- cgit v1.2.3