From 3015ce0f9d120df7583f838d1a47cb469fe5a727 Mon Sep 17 00:00:00 2001 From: Ogulcan Celik Date: Sun, 14 Jun 2026 23:01:47 +0300 Subject: [PATCH] fix: stabilize plugin config directories --- docs/next/CHANGELOG.md | 6 + .../next/website/src/content/docs/plugins.mdx | 12 +- src/api/schema/plugins.rs | 8 +- src/app/api/plugins/env.rs | 17 ++- src/app/api/plugins/mod.rs | 86 ++++++++++-- src/app/api/plugins/panes.rs | 2 + src/app/api/plugins/runtime.rs | 2 + src/cli/plugin.rs | 62 +++++++++ src/main.rs | 1 + src/plugin_paths.rs | 128 ++++++++++++++++++ 10 files changed, 302 insertions(+), 22 deletions(-) create mode 100644 src/plugin_paths.rs diff --git a/docs/next/CHANGELOG.md b/docs/next/CHANGELOG.md index d30ef572..57251781 100644 --- a/docs/next/CHANGELOG.md +++ b/docs/next/CHANGELOG.md @@ -9,6 +9,9 @@ - Added `herdr plugin install /[/subdir...]`, `plugin uninstall`, source metadata in `plugin.list`, offline registry fallback, and a human-readable default `plugin list` with `--json` for scripts. +- Added `herdr plugin config-dir ` and automatic plugin config/state + directory creation so plugin setup docs can point users at a stable config + path. - Added supporting plugin host APIs for `pane.current`, `pane.process_info`, `client.window_title.set/clear`, `layout.export/apply`, plugin pane placement, plugin invocation context/env injection, and plugin pane ownership across @@ -17,6 +20,9 @@ ### Changed - Bumped the client/server protocol version to 14 for `pane.move` compatibility. (#299) +- Plugin runtime config directories now use stable, readable plugin-id paths + instead of checkout hashes; existing legacy config directories are copied into + the new location when first seen. - Public workspace, tab, and pane ids are now short stable handles such as `w1`, `w1:t1`, and `w1:p1`; closed tab and pane ids no longer retarget later resources. (#569) ### Fixed diff --git a/docs/next/website/src/content/docs/plugins.mdx b/docs/next/website/src/content/docs/plugins.mdx index 0b548b64..cf5b4ff2 100644 --- a/docs/next/website/src/content/docs/plugins.mdx +++ b/docs/next/website/src/content/docs/plugins.mdx @@ -143,6 +143,7 @@ Install an example plugin: ```bash herdr plugin install ogulcancelik/herdr-plugin-examples/agent-telegram-notify +herdr plugin config-dir examples.agent-telegram-notify herdr plugin list herdr plugin action list --plugin examples.agent-telegram-notify ``` @@ -151,6 +152,7 @@ When you are authoring a local plugin, link the working directory instead: ```bash herdr plugin link /path/to/plugin +herdr plugin config-dir example.layout herdr plugin action list --plugin example.layout herdr plugin action invoke example.layout.apply herdr plugin pane open --plugin example.layout --entrypoint board @@ -163,7 +165,9 @@ terminals, runs supported build commands, then stores the checkout under Herdr-managed plugin data and registers it. Use `--yes` for noninteractive installs. Reinstalling a GitHub-managed plugin replaces that managed checkout. Installing over a locally linked plugin is refused; unlink or uninstall the -local plugin first. +local plugin first. `plugin install` and `plugin link` create the plugin's +config and state directories, and `plugin config-dir ` prints the config +directory for setup docs and shell scripts. `plugin uninstall ` unregisters the plugin. For GitHub-managed installs it also removes the managed checkout, and it accepts either the plugin @@ -206,8 +210,10 @@ receive `HERDR_PLUGIN_ACTION_ID`; event hooks receive `HERDR_PLUGIN_EVENT` and user credentials or durable state there, because GitHub-installed plugin roots are managed source checkouts. Put user-editable config such as `.env` files under `HERDR_PLUGIN_CONFIG_DIR`, and put local runtime state under -`HERDR_PLUGIN_STATE_DIR`. Herdr does not read, create, migrate, validate, sync, -or delete their contents. The plugin owns the file format and lifecycle. +`HERDR_PLUGIN_STATE_DIR`. Herdr creates those directories and seeds +`HERDR_PLUGIN_CONFIG_DIR` from the legacy plugin config locations when present, +but it does not validate, sync, or delete their contents. The plugin owns the +file format and lifecycle. `HERDR_PLUGIN_CONTEXT_JSON` can include workspace, tab, focused pane, worktree, agent, selected text, clicked URL, and link handler fields when they are diff --git a/src/api/schema/plugins.rs b/src/api/schema/plugins.rs index be9336c8..907f243b 100644 --- a/src/api/schema/plugins.rs +++ b/src/api/schema/plugins.rs @@ -107,7 +107,7 @@ pub enum PluginSourceKind { pub(crate) fn plugin_managed_path_component(value: &str) -> String { let slug = readable_plugin_path_slug(value); - let hash = short_plugin_id_hash(value); + let hash = short_plugin_id_hash_for_path_component(value); format!("{slug}-{hash}") } @@ -133,14 +133,14 @@ fn readable_plugin_path_slug(value: &str) -> String { } else { slug.chars().take(80).collect() }; - if has_windows_reserved_stem(&slug) { + if has_windows_reserved_stem_for_path_component(&slug) { slug.replace('.', "-") } else { slug } } -fn short_plugin_id_hash(value: &str) -> String { +pub(crate) fn short_plugin_id_hash_for_path_component(value: &str) -> String { use sha2::{Digest, Sha256}; let digest = Sha256::digest(value.as_bytes()); @@ -152,7 +152,7 @@ fn short_plugin_id_hash(value: &str) -> String { hash } -fn has_windows_reserved_stem(value: &str) -> bool { +pub(crate) fn has_windows_reserved_stem_for_path_component(value: &str) -> bool { let stem = value.split('.').next().unwrap_or(value); matches!( stem.to_ascii_uppercase().as_str(), diff --git a/src/app/api/plugins/env.rs b/src/app/api/plugins/env.rs index 13644a19..fcfac62f 100644 --- a/src/app/api/plugins/env.rs +++ b/src/app/api/plugins/env.rs @@ -1,9 +1,20 @@ use crate::api::schema::InstalledPluginInfo; +pub(super) fn plugin_config_dir(plugin_id: &str) -> std::path::PathBuf { + crate::plugin_paths::plugin_config_dir(plugin_id) +} + +pub(super) fn plugin_state_dir(plugin_id: &str) -> std::path::PathBuf { + crate::plugin_paths::plugin_state_dir(plugin_id) +} + +pub(super) fn ensure_plugin_user_dirs(plugin: &InstalledPluginInfo) -> std::io::Result<()> { + crate::plugin_paths::ensure_plugin_user_dirs(&plugin.plugin_id) +} + pub(super) fn plugin_path_env(plugin: &InstalledPluginInfo) -> Vec<(String, String)> { - let component = crate::api::schema::plugin_managed_path_component(&plugin.plugin_id); - let config_dir = crate::config::config_dir().join("plugins").join(&component); - let state_dir = crate::config::state_dir().join("plugins").join(component); + let config_dir = plugin_config_dir(&plugin.plugin_id); + let state_dir = plugin_state_dir(&plugin.plugin_id); vec![ ("HERDR_PLUGIN_ROOT".to_string(), plugin.plugin_root.clone()), diff --git a/src/app/api/plugins/mod.rs b/src/app/api/plugins/mod.rs index d00ad20f..89f7a23d 100644 --- a/src/app/api/plugins/mod.rs +++ b/src/app/api/plugins/mod.rs @@ -36,6 +36,9 @@ impl App { Err((code, message)) => return encode_error(id, code, message), } } + 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 @@ -755,6 +758,71 @@ action = "bootstrap" ); } + #[test] + fn plugin_link_creates_stable_config_and_state_dirs() { + let mut app = test_app(); + let root = unique_temp_path("plugin-link-dirs"); + let config_dir = super::env::plugin_config_dir("example.config-dirs"); + let state_dir = super::env::plugin_state_dir("example.config-dirs"); + let _ = std::fs::remove_dir_all(&config_dir); + let _ = std::fs::remove_dir_all(&state_dir); + write_manifest_content( + &root, + r#" +id = "example.config-dirs" +name = "Config Dirs" +version = "0.1.0" +platforms = ["linux", "macos", "windows"] +"#, + ); + + link_manifest(&mut app, &root); + + assert!(config_dir.is_dir()); + assert!(state_dir.is_dir()); + + let _ = std::fs::remove_dir_all(root); + let _ = std::fs::remove_dir_all(config_dir); + let _ = std::fs::remove_dir_all(state_dir); + } + + #[test] + fn plugin_link_seeds_stable_config_dir_from_legacy_unhashed_dir() { + let mut app = test_app(); + let root = unique_temp_path("plugin-link-legacy-config"); + let config_dir = super::env::plugin_config_dir("example.legacy-config"); + let state_dir = super::env::plugin_state_dir("example.legacy-config"); + let legacy_dir = crate::config::config_dir() + .join("plugins") + .join("example.legacy-config"); + let _ = std::fs::remove_dir_all(&config_dir); + let _ = std::fs::remove_dir_all(&state_dir); + let _ = std::fs::remove_dir_all(&legacy_dir); + std::fs::create_dir_all(&legacy_dir).unwrap(); + std::fs::write(legacy_dir.join(".env"), "TELEGRAM_BOT_TOKEN=test\n").unwrap(); + write_manifest_content( + &root, + r#" +id = "example.legacy-config" +name = "Legacy Config" +version = "0.1.0" +platforms = ["linux", "macos", "windows"] +"#, + ); + + link_manifest(&mut app, &root); + + assert_eq!( + std::fs::read_to_string(config_dir.join(".env")).unwrap(), + "TELEGRAM_BOT_TOKEN=test\n" + ); + + let _ = std::fs::remove_dir_all(root); + let _ = std::fs::remove_dir_all(config_dir); + let _ = std::fs::remove_dir_all(state_dir); + let _ = std::fs::remove_dir_all(legacy_dir); + } + #[test] fn plugin_link_lists_and_unlinks_manifest() { let mut app = test_app(); @@ -1192,9 +1260,8 @@ command = ["sh", "-c", "printf '%s\n%s\n%s\n' \"$HERDR_PLUGIN_ROOT\" \"$HERDR_PL Some( crate::config::config_dir() .join("plugins") - .join(crate::api::schema::plugin_managed_path_component( - "example.path-env" - )) + .join("config") + .join("example.path-env") .display() .to_string() .as_str() @@ -1205,9 +1272,7 @@ command = ["sh", "-c", "printf '%s\n%s\n%s\n' \"$HERDR_PLUGIN_ROOT\" \"$HERDR_PL Some( crate::config::state_dir() .join("plugins") - .join(crate::api::schema::plugin_managed_path_component( - "example.path-env" - )) + .join("example.path-env") .display() .to_string() .as_str() @@ -1570,9 +1635,8 @@ command = ["sh", "-c", "printf '%s\n%s\n%s' \"$HERDR_PLUGIN_ROOT\" \"$HERDR_PLUG Some( crate::config::config_dir() .join("plugins") - .join(crate::api::schema::plugin_managed_path_component( - "example.action-paths" - )) + .join("config") + .join("example.action-paths") .display() .to_string() .as_str() @@ -1583,9 +1647,7 @@ command = ["sh", "-c", "printf '%s\n%s\n%s' \"$HERDR_PLUGIN_ROOT\" \"$HERDR_PLUG Some( crate::config::state_dir() .join("plugins") - .join(crate::api::schema::plugin_managed_path_component( - "example.action-paths" - )) + .join("example.action-paths") .display() .to_string() .as_str() diff --git a/src/app/api/plugins/panes.rs b/src/app/api/plugins/panes.rs index e6fe7b44..5a935a8c 100644 --- a/src/app/api/plugins/panes.rs +++ b/src/app/api/plugins/panes.rs @@ -183,6 +183,8 @@ impl App { let mut env = super::super::env::normalize_launch_env(env)?; let context_json = serde_json::to_string(&context) .map_err(|err| ("invalid_plugin_context".to_string(), err.to_string()))?; + super::env::ensure_plugin_user_dirs(plugin) + .map_err(|err| ("plugin_user_dir_create_failed".to_string(), err.to_string()))?; env.retain(|(key, _)| !plugin_pane_protected_env_key(key)); env.extend(super::env::plugin_path_env(plugin)); env.push(( diff --git a/src/app/api/plugins/runtime.rs b/src/app/api/plugins/runtime.rs index 94a49dd4..0008def2 100644 --- a/src/app/api/plugins/runtime.rs +++ b/src/app/api/plugins/runtime.rs @@ -31,6 +31,8 @@ impl App { let args = command.iter().skip(1).cloned().collect::>(); let context_json = serde_json::to_string(context) .map_err(|err| ("invalid_plugin_context", err.to_string()))?; + super::env::ensure_plugin_user_dirs(plugin) + .map_err(|err| ("plugin_user_dir_create_failed", err.to_string()))?; let log_id = format!("plugin-log-{}", self.state.next_plugin_command_log_id); self.state.next_plugin_command_log_id += 1; let started_unix_ms = current_unix_ms(); diff --git a/src/cli/plugin.rs b/src/cli/plugin.rs index f391fa47..ffdcd711 100644 --- a/src/cli/plugin.rs +++ b/src/cli/plugin.rs @@ -26,6 +26,7 @@ pub(super) fn run_plugin_command(args: &[String]) -> std::io::Result { "uninstall" => plugin_uninstall(&args[1..]), "link" => plugin_link(&args[1..]), "list" => plugin_list(&args[1..]), + "config-dir" => plugin_config_dir_command(&args[1..]), "unlink" => plugin_unlink(&args[1..]), "enable" => plugin_set_enabled(&args[1..], true), "disable" => plugin_set_enabled(&args[1..], false), @@ -74,6 +75,21 @@ fn plugin_link(args: &[String]) -> std::io::Result { })) } +fn plugin_config_dir_command(args: &[String]) -> std::io::Result { + let Some(plugin_id) = args.first() else { + eprintln!("usage: herdr plugin config-dir "); + return Ok(2); + }; + if args.len() != 1 { + eprintln!("usage: herdr plugin config-dir "); + return Ok(2); + } + let path = crate::plugin_paths::plugin_config_dir(plugin_id); + crate::plugin_paths::ensure_plugin_user_dirs(plugin_id)?; + println!("{}", path.display()); + Ok(0) +} + fn plugin_list(args: &[String]) -> std::io::Result { let mut plugin_id = None; let mut json = false; @@ -224,6 +240,10 @@ fn plugin_install(args: &[String]) -> std::io::Result { Err(InstallFailure::KeepCheckout(err)) => return Err(err), }; println!("Installed {} from {}.", plugin.plugin_id, source.display()); + println!( + "Config: {}", + crate::plugin_paths::plugin_config_dir(&plugin.plugin_id).display() + ); Ok(0) })(); let _ = std::fs::remove_dir_all(&temp_root); @@ -892,6 +912,8 @@ fn register_installed_plugin( Err(err) if is_connection_error(&err) => { let mut plugins = crate::persist::plugin_registry::load(); plugins.retain(|entry| entry.plugin_id != plugin.plugin_id); + crate::plugin_paths::ensure_plugin_user_dirs(&plugin.plugin_id) + .map_err(InstallFailure::Rollback)?; plugins.push(plugin); crate::persist::plugin_registry::save(&plugins).map_err(InstallFailure::Rollback) } @@ -1082,6 +1104,10 @@ fn print_plugin_list_human(response: &serde_json::Value) -> std::io::Result source_display(&plugin), warning ); + println!( + " config: {}", + crate::plugin_paths::plugin_config_dir(&plugin.plugin_id).display() + ); for warning in plugin.warnings { println!(" warning: {warning}"); } @@ -1561,6 +1587,7 @@ fn print_plugin_help() { eprintln!(" herdr plugin uninstall "); eprintln!(" herdr plugin link [--disabled]"); eprintln!(" herdr plugin list [--plugin ID] [--json]"); + eprintln!(" herdr plugin config-dir "); eprintln!(" herdr plugin unlink "); eprintln!(" herdr plugin enable "); eprintln!(" herdr plugin disable "); @@ -1586,6 +1613,14 @@ fn print_plugin_pane_help() { mod tests { use super::*; + fn unique_plugin_id(label: &str) -> String { + let nanos = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|duration| duration.as_nanos()) + .unwrap_or(0); + format!("test.{label}.{}.{nanos}", std::process::id()) + } + fn github_plugin( id: &str, owner: &str, @@ -1708,4 +1743,31 @@ mod tests { assert!(plugin_by_github_source([plugin], &source).is_none()); } + + #[test] + fn cli_user_dir_creation_seeds_legacy_config_before_printing_config_dir() { + let plugin_id = unique_plugin_id("legacy-config"); + let config_dir = crate::plugin_paths::plugin_config_dir(&plugin_id); + let state_dir = crate::plugin_paths::plugin_state_dir(&plugin_id); + let legacy_dir = crate::config::config_dir().join("plugins").join(&plugin_id); + let _ = std::fs::remove_dir_all(&config_dir); + let _ = std::fs::remove_dir_all(&state_dir); + let _ = std::fs::remove_dir_all(&legacy_dir); + std::fs::create_dir_all(&legacy_dir).unwrap(); + std::fs::write(legacy_dir.join(".env"), "TOKEN=legacy\n").unwrap(); + + assert_eq!( + plugin_config_dir_command(std::slice::from_ref(&plugin_id)).unwrap(), + 0 + ); + + assert_eq!( + std::fs::read_to_string(config_dir.join(".env")).unwrap(), + "TOKEN=legacy\n" + ); + + let _ = std::fs::remove_dir_all(config_dir); + let _ = std::fs::remove_dir_all(state_dir); + let _ = std::fs::remove_dir_all(legacy_dir); + } } diff --git a/src/main.rs b/src/main.rs index d2efcc90..99428897 100644 --- a/src/main.rs +++ b/src/main.rs @@ -65,6 +65,7 @@ mod pane; mod persist; mod platform; mod plugin_command; +mod plugin_paths; mod product_announcements; mod protocol; mod pty; diff --git a/src/plugin_paths.rs b/src/plugin_paths.rs new file mode 100644 index 00000000..18a16a77 --- /dev/null +++ b/src/plugin_paths.rs @@ -0,0 +1,128 @@ +use std::path::{Path, PathBuf}; + +const PLUGIN_CONFIG_PATH_COMPONENT_MAX_CHARS: usize = 120; + +pub(crate) fn plugin_config_dir(plugin_id: &str) -> PathBuf { + crate::config::config_dir() + .join("plugins") + .join("config") + .join(plugin_config_path_component(plugin_id)) +} + +pub(crate) fn plugin_state_dir(plugin_id: &str) -> PathBuf { + crate::config::state_dir() + .join("plugins") + .join(plugin_config_path_component(plugin_id)) +} + +pub(crate) fn ensure_plugin_user_dirs(plugin_id: &str) -> std::io::Result<()> { + ensure_plugin_config_dir(plugin_id)?; + std::fs::create_dir_all(plugin_state_dir(plugin_id))?; + Ok(()) +} + +fn ensure_plugin_config_dir(plugin_id: &str) -> std::io::Result<()> { + let config_dir = plugin_config_dir(plugin_id); + if config_dir.exists() { + return std::fs::create_dir_all(config_dir); + } + if let Some(legacy_dir) = legacy_plugin_config_dirs(plugin_id) + .into_iter() + .find(|path| path.is_dir()) + { + copy_dir_all(&legacy_dir, &config_dir)?; + return Ok(()); + } + std::fs::create_dir_all(config_dir) +} + +fn legacy_plugin_config_dirs(plugin_id: &str) -> Vec { + let plugins_dir = crate::config::config_dir().join("plugins"); + let old_unhashed = + (!matches!(plugin_id, "config" | "github")).then(|| plugins_dir.join(plugin_id)); + let current_hashed = + plugins_dir.join(crate::api::schema::plugin_managed_path_component(plugin_id)); + let mut candidates = Vec::new(); + if let Some(old_unhashed) = old_unhashed { + if old_unhashed != current_hashed { + candidates.push(old_unhashed); + } + } + candidates.push(current_hashed); + candidates +} + +fn plugin_config_path_component(value: &str) -> String { + let mut component = String::new(); + for byte in value.bytes() { + if byte.is_ascii_lowercase() || byte.is_ascii_digit() || matches!(byte, b'.' | b'_' | b'-') + { + component.push(byte as char); + } else { + use std::fmt::Write as _; + let _ = write!(component, "%{byte:02X}"); + } + } + if component.ends_with('.') { + component.pop(); + component.push_str("%2E"); + } + if component.is_empty() { + return "%plugin".to_string(); + } + if crate::api::schema::has_windows_reserved_stem_for_path_component(&component) { + component = format!("%{component}"); + } + if component.chars().count() > PLUGIN_CONFIG_PATH_COMPONENT_MAX_CHARS { + let hash = crate::api::schema::short_plugin_id_hash_for_path_component(value); + let prefix_len = PLUGIN_CONFIG_PATH_COMPONENT_MAX_CHARS - hash.len() - 1; + let prefix = component.chars().take(prefix_len).collect::(); + return format!("{prefix}-{hash}"); + } + component +} + +fn copy_dir_all(source: &Path, destination: &Path) -> std::io::Result<()> { + std::fs::create_dir_all(destination)?; + for entry in std::fs::read_dir(source)? { + let entry = entry?; + let file_type = entry.file_type()?; + let destination_path = destination.join(entry.file_name()); + if file_type.is_dir() { + copy_dir_all(&entry.path(), &destination_path)?; + } else { + std::fs::copy(entry.path(), destination_path)?; + } + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn plugin_config_path_component_is_readable_and_collision_free() { + assert_eq!( + plugin_config_path_component("examples.agent-telegram-notify"), + "examples.agent-telegram-notify" + ); + assert_eq!(plugin_config_path_component("example:a"), "example%3Aa"); + assert_eq!(plugin_config_path_component("Example"), "%45xample"); + assert_ne!( + plugin_config_path_component("example:a"), + plugin_config_path_component("example-a") + ); + assert_ne!( + plugin_config_path_component("Example"), + plugin_config_path_component("example") + ); + assert_ne!( + plugin_config_path_component(&"A".repeat(120)), + plugin_config_path_component(&"B".repeat(120)) + ); + assert!(plugin_config_path_component(&"A".repeat(120)).len() <= 120); + assert_eq!(plugin_config_path_component("con"), "%con"); + assert_eq!(plugin_config_path_component("example."), "example%2E"); + } +}