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/app/api/plugins/manifest.rs | 7 +- src/app/api/plugins/mod.rs | 229 ++++++++++++++++++++++++++++++---------- src/app/api/plugins/runtime.rs | 4 + 3 files changed, 179 insertions(+), 61 deletions(-) (limited to 'src/app/api/plugins') diff --git a/src/app/api/plugins/manifest.rs b/src/app/api/plugins/manifest.rs index f6464285..657f8257 100644 --- a/src/app/api/plugins/manifest.rs +++ b/src/app/api/plugins/manifest.rs @@ -292,12 +292,7 @@ pub(super) fn normalize_plugin_source( let plugin_root = std::path::PathBuf::from(&plugin.plugin_root) .canonicalize() .map_err(|err| ("invalid_plugin_source", err.to_string()))?; - let expected = crate::session::data_dir() - .join("plugins") - .join("github") - .join(crate::api::schema::plugin_managed_path_component( - &plugin.plugin_id, - )) + let expected = crate::plugin_paths::managed_checkout_path(&plugin.plugin_id) .canonicalize() .map_err(|err| ("invalid_plugin_source", err.to_string()))?; if managed_path != expected { diff --git a/src/app/api/plugins/mod.rs b/src/app/api/plugins/mod.rs index e48a1506..d7a7b0ed 100644 --- a/src/app/api/plugins/mod.rs +++ b/src/app/api/plugins/mod.rs @@ -25,6 +25,46 @@ pub(crate) use manifest::load_plugin_manifest; use runtime::{read_capped_plugin_output, MAX_PLUGIN_COMMANDS_IN_FLIGHT}; impl App { + fn replace_installed_plugins(&mut self, entries: Vec) { + let entries = + crate::persist::plugin_registry::reload_manifests(entries, |path, enabled| { + load_plugin_manifest(path, enabled).map_err(|(_, message)| message) + }); + self.state.installed_plugins = entries + .into_iter() + .map(|plugin| (plugin.plugin_id.clone(), plugin)) + .collect(); + } + + fn refresh_installed_plugins(&mut self) -> std::io::Result<()> { + if self.no_session { + return Ok(()); + } + let entries = crate::persist::plugin_registry::try_load()?; + self.replace_installed_plugins(entries); + Ok(()) + } + + fn update_installed_plugins( + &mut self, + mutation: impl FnOnce(&mut crate::app::state::InstalledPluginRegistry) -> T, + ) -> std::io::Result { + if self.no_session { + return Ok(mutation(&mut self.state.installed_plugins)); + } + let (result, entries) = crate::persist::plugin_registry::update(|entries| { + let mut registry = entries + .drain(..) + .map(|plugin| (plugin.plugin_id.clone(), plugin)) + .collect(); + let result = mutation(&mut registry); + *entries = registry.into_values().collect(); + result + })?; + self.replace_installed_plugins(entries); + Ok(result) + } + pub(super) fn handle_plugin_link(&mut self, id: String, params: PluginLinkParams) -> String { let mut plugin = match load_plugin_manifest(¶ms.path, params.enabled) { Ok(plugin) => plugin, @@ -39,21 +79,9 @@ impl App { if let Err(err) = env::ensure_plugin_user_dirs(&plugin) { return encode_error(id, "plugin_user_dir_create_failed", err.to_string()); } - let previous = self.state.installed_plugins.get(&plugin.plugin_id).cloned(); - self.state - .installed_plugins - .insert(plugin.plugin_id.clone(), plugin.clone()); - if let Err(err) = self.save_plugin_registry() { - match previous { - Some(previous) => { - self.state - .installed_plugins - .insert(previous.plugin_id.clone(), previous); - } - None => { - self.state.installed_plugins.remove(&plugin.plugin_id); - } - } + if let Err(err) = self.update_installed_plugins(|plugins| { + plugins.insert(plugin.plugin_id.clone(), plugin.clone()); + }) { return encode_error(id, "plugin_registry_save_failed", err.to_string()); } encode_success(id, ResponseResult::PluginLinked { plugin }) @@ -64,6 +92,9 @@ impl App { Ok(plugin_id) => plugin_id, Err(response) => return response, }; + if let Err(err) = self.refresh_installed_plugins() { + return encode_error(id, "plugin_registry_load_failed", err.to_string()); + } let mut plugins = self .state .installed_plugins @@ -87,29 +118,18 @@ impl App { let Some(plugin_id) = normalize_plugin_id(¶ms.plugin_id) else { return invalid_plugin_id(id); }; - let previous = self.state.installed_plugins.remove(&plugin_id); - let removed = previous.is_some(); - let previous_panes = if removed { - Some(self.state.plugin_panes.clone()) - } else { - None - }; + let removed = + match self.update_installed_plugins(|plugins| plugins.remove(&plugin_id).is_some()) { + Ok(removed) => removed, + Err(err) => { + return encode_error(id, "plugin_registry_save_failed", err.to_string()); + } + }; if removed { // Drop plugin_panes records for this plugin (panes keep running). self.state .plugin_panes .retain(|_, record| record.plugin_id != plugin_id); - if let Err(err) = self.save_plugin_registry() { - if let Some(previous) = previous { - self.state - .installed_plugins - .insert(plugin_id.clone(), previous); - } - if let Some(previous_panes) = previous_panes { - self.state.plugin_panes = previous_panes; - } - return encode_error(id, "plugin_registry_save_failed", err.to_string()); - } self.clear_agent_view_for_source(&format!("plugin:{plugin_id}")); } encode_success(id, ResponseResult::PluginUnlinked { plugin_id, removed }) @@ -140,6 +160,9 @@ impl App { Ok(plugin_id) => plugin_id, Err(response) => return response, }; + if let Err(err) = self.refresh_installed_plugins() { + return encode_error(id, "plugin_registry_load_failed", err.to_string()); + } let mut actions = manifest_actions(&self.state.installed_plugins) .filter(|action| { plugin_id @@ -156,6 +179,9 @@ impl App { id: String, params: PluginActionInvokeParams, ) -> String { + if let Err(err) = self.refresh_installed_plugins() { + return encode_error(id, "plugin_registry_load_failed", err.to_string()); + } let (plugin, action) = match self.find_plugin_action(params.plugin_id.as_deref(), ¶ms.action_id) { Ok(pair) => pair, @@ -200,6 +226,8 @@ impl App { &mut self, action_id: String, ) -> Result<(), String> { + self.refresh_installed_plugins() + .map_err(|err| format!("failed to load plugin registry: {err}"))?; let (plugin, action) = self .find_plugin_action(None, &action_id) .map_err(|(_, message)| message)?; @@ -230,6 +258,8 @@ impl App { url: &str, pane_id: crate::layout::PaneId, ) -> Result { + self.refresh_installed_plugins() + .map_err(|err| format!("failed to load plugin registry: {err}"))?; let Some((plugin, handler)) = self.find_plugin_link_handler(url) else { return Ok(false); }; @@ -308,6 +338,9 @@ impl App { id: String, params: PluginPaneOpenParams, ) -> String { + if let Err(err) = self.refresh_installed_plugins() { + return encode_error(id, "plugin_registry_load_failed", err.to_string()); + } let Some(plugin_id) = normalize_plugin_id(¶ms.plugin_id) else { return invalid_plugin_id(id); }; @@ -580,16 +613,21 @@ impl App { let Some(plugin_id) = normalize_plugin_id(&plugin_id) else { return invalid_plugin_id(id); }; - let Some(plugin) = self.state.installed_plugins.get_mut(&plugin_id) else { - return encode_error(id, "plugin_not_found", "plugin not found"); - }; - let previous_enabled = plugin.enabled; - plugin.enabled = enabled; - if let Err(err) = self.save_plugin_registry() { - if let Some(plugin) = self.state.installed_plugins.get_mut(&plugin_id) { - plugin.enabled = previous_enabled; + let found = match self.update_installed_plugins(|plugins| { + if let Some(plugin) = plugins.get_mut(&plugin_id) { + plugin.enabled = enabled; + true + } else { + false } - return encode_error(id, "plugin_registry_save_failed", err.to_string()); + }) { + Ok(found) => found, + Err(err) => { + return encode_error(id, "plugin_registry_save_failed", err.to_string()); + } + }; + if !found { + return encode_error(id, "plugin_not_found", "plugin not found"); } let Some(plugin) = self.state.installed_plugins.get(&plugin_id).cloned() else { return encode_error(id, "plugin_not_found", "plugin not found"); @@ -603,19 +641,6 @@ impl App { encode_success(id, ResponseResult::PluginDisabled { plugin }) } } - - pub(crate) fn save_plugin_registry(&self) -> std::io::Result<()> { - if self.no_session { - return Ok(()); - } - let plugins = self - .state - .installed_plugins - .values() - .cloned() - .collect::>(); - crate::persist::plugin_registry::save(&plugins) - } } fn invalid_plugin_id(id: String) -> String { @@ -2141,6 +2166,100 @@ command = ["sh", "-c", "printf %s ${{HERDR_PANE_ID-unset}} > '{}'; sleep 1"] assert_eq!(value["error"]["code"], "plugin_manifest_unavailable"); } + #[test] + fn non_cli_plugin_consumers_refresh_global_enabled_state() { + let _guard = crate::config::test_config_env_lock().lock().unwrap(); + let previous_config_home = std::env::var_os("XDG_CONFIG_HOME"); + let base = unique_temp_path("plugin-global-refresh"); + std::env::set_var("XDG_CONFIG_HOME", &base); + let root = base.join("plugin"); + write_manifest(&root); + let plugin = load_plugin_manifest(&root.display().to_string(), false).unwrap(); + crate::persist::plugin_registry::update(|plugins| { + plugins.retain(|entry| entry.plugin_id != plugin.plugin_id); + plugins.push(plugin.clone()); + }) + .unwrap(); + + let mut app = test_app(); + app.no_session = false; + let workspace = crate::workspace::Workspace::test_new("plugin-refresh"); + let pane_id = workspace.tabs[0].root_pane; + app.state.workspaces = vec![workspace]; + app.state.ensure_test_terminals(); + app.state.active = Some(0); + app.state.selected = 0; + + let make_stale = |app: &mut App| { + let mut stale = plugin.clone(); + stale.enabled = true; + app.state + .installed_plugins + .insert(stale.plugin_id.clone(), stale); + }; + + make_stale(&mut app); + assert!(app + .invoke_plugin_action_from_keybind("bootstrap".into()) + .unwrap_err() + .contains("disabled")); + + make_stale(&mut app); + assert!(!app + .invoke_plugin_link_handler_for_url( + "https://github.com/ogulcancelik/herdr/issues/1174", + pane_id, + ) + .unwrap()); + + make_stale(&mut app); + let pane = app.handle_api_request(Request { + id: "pane-disabled".into(), + method: Method::PluginPaneOpen(PluginPaneOpenParams { + plugin_id: "example.worktree-bootstrap".into(), + entrypoint: "board".into(), + placement: Some(PluginPanePlacement::Overlay), + width: None, + height: None, + workspace_id: None, + target_pane_id: None, + direction: None, + cwd: None, + focus: true, + env: std::collections::HashMap::new(), + }), + }); + let pane: serde_json::Value = serde_json::from_str(&pane).unwrap(); + assert_eq!(pane["error"]["code"], "plugin_disabled"); + + make_stale(&mut app); + let logs_before = app.state.plugin_command_logs.len(); + let workspace = app.workspace_info(0); + app.run_plugin_event_hooks(&crate::api::schema::EventEnvelope { + event: crate::api::schema::EventKind::WorktreeCreated, + data: crate::api::schema::EventData::WorktreeCreated { + workspace: workspace.clone(), + worktree: crate::api::schema::WorktreeInfo { + path: "/tmp/repo".into(), + branch: Some("feature".into()), + is_bare: false, + is_detached: false, + is_prunable: false, + is_linked_worktree: true, + open_workspace_id: Some(workspace.workspace_id), + label: "feature".into(), + }, + }, + }); + assert_eq!(app.state.plugin_command_logs.len(), logs_before); + + let _ = std::fs::remove_dir_all(&base); + match previous_config_home { + Some(previous) => std::env::set_var("XDG_CONFIG_HOME", previous), + None => std::env::remove_var("XDG_CONFIG_HOME"), + } + } + #[cfg(unix)] #[test] fn manifest_action_invoke_runs_command_and_captures_log() { diff --git a/src/app/api/plugins/runtime.rs b/src/app/api/plugins/runtime.rs index 71004fe5..9106886c 100644 --- a/src/app/api/plugins/runtime.rs +++ b/src/app/api/plugins/runtime.rs @@ -220,6 +220,10 @@ impl App { if !crate::api::schema::PLUGIN_HOOK_EVENT_KINDS.contains(&event.event) { return; } + if let Err(err) = self.refresh_installed_plugins() { + tracing::warn!(err = %err, "failed to refresh plugin registry before event hooks"); + return; + } let plugins = self .state .installed_plugins -- cgit v1.2.3