fix: stabilize plugin config directories

8 files changed, 287 insertions, 19 deletions

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
@@ -756,6 +759,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();
         let root = unique_temp_path("plugin-link");
@@ -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::<Vec<_>>();
         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<i32> {
         "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<i32> {
     }))
 }
 
+fn plugin_config_dir_command(args: &[String]) -> std::io::Result<i32> {
+    let Some(plugin_id) = args.first() else {
+        eprintln!("usage: herdr plugin config-dir <plugin_id>");
+        return Ok(2);
+    };
+    if args.len() != 1 {
+        eprintln!("usage: herdr plugin config-dir <plugin_id>");
+        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<i32> {
     let mut plugin_id = None;
     let mut json = false;
@@ -224,6 +240,10 @@ fn plugin_install(args: &[String]) -> std::io::Result<i32> {
             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<i32>
             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 <plugin_id|owner/repo[/subdir...]>");
     eprintln!("  herdr plugin link <path> [--disabled]");
     eprintln!("  herdr plugin list [--plugin ID] [--json]");
+    eprintln!("  herdr plugin config-dir <plugin_id>");
     eprintln!("  herdr plugin unlink <plugin_id>");
     eprintln!("  herdr plugin enable <plugin_id>");
     eprintln!("  herdr plugin disable <plugin_id>");
@@ -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<PathBuf> {
+    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::<String>();
+        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");
+    }
+}