From 0a4cea5ab2f78f3522125fec4d38658ece203ce4 Mon Sep 17 00:00:00 2001 From: Yeachan-Heo Date: Wed, 1 Apr 2026 06:55:39 +0000 Subject: [PATCH] feat: plugin hooks + tool registry + CLI integration --- rust/crates/commands/src/lib.rs | 48 ++-- rust/crates/plugins/src/hooks.rs | 24 +- rust/crates/plugins/src/lib.rs | 338 +++++++++++++++++++++-- rust/crates/runtime/src/conversation.rs | 105 +++---- rust/crates/rusty-claude-cli/src/main.rs | 26 +- rust/crates/tools/src/lib.rs | 87 +++++- 6 files changed, 526 insertions(+), 102 deletions(-) diff --git a/rust/crates/commands/src/lib.rs b/rust/crates/commands/src/lib.rs index c8425a0..b7f19b4 100644 --- a/rust/crates/commands/src/lib.rs +++ b/rust/crates/commands/src/lib.rs @@ -1,4 +1,4 @@ -use plugins::{PluginError, PluginKind, PluginManager, PluginSummary}; +use plugins::{PluginError, PluginManager, PluginSummary}; use runtime::{compact_session, CompactionConfig, Session}; #[derive(Debug, Clone, PartialEq, Eq)] @@ -134,7 +134,7 @@ const SLASH_COMMAND_SPECS: &[SlashCommandSpec] = &[ name: "plugins", summary: "List or manage plugins", argument_hint: Some( - "[list|install |enable |disable |uninstall |update ]", + "[list|install |enable |disable |uninstall |update ]", ), resume_supported: false, }, @@ -278,6 +278,7 @@ pub struct PluginsCommandResult { pub reload_runtime: bool, } +#[allow(clippy::too_many_lines)] pub fn handle_plugins_slash_command( action: Option<&str>, target: Option<&str>, @@ -397,15 +398,14 @@ pub fn render_plugins_report(plugins: &[PluginSummary]) -> String { return lines.join("\n"); } for plugin in plugins { - let kind = match plugin.metadata.kind { - PluginKind::Builtin => "builtin", - PluginKind::Bundled => "bundled", - PluginKind::External => "external", + let enabled = if plugin.enabled { + "enabled" + } else { + "disabled" }; - let enabled = if plugin.enabled { "enabled" } else { "disabled" }; lines.push(format!( - " {id:<24} {kind:<8} {enabled:<8} v{version}", - id = plugin.metadata.id, + " {name:<20} v{version:<10} {enabled}", + name = plugin.metadata.name, version = plugin.metadata.version, )); } @@ -606,6 +606,20 @@ mod tests { target: None }) ); + assert_eq!( + SlashCommand::parse("/plugins enable demo"), + Some(SlashCommand::Plugins { + action: Some("enable".to_string()), + target: Some("demo".to_string()) + }) + ); + assert_eq!( + SlashCommand::parse("/plugins disable demo"), + Some(SlashCommand::Plugins { + action: Some("disable".to_string()), + target: Some("demo".to_string()) + }) + ); } #[test] @@ -628,7 +642,7 @@ mod tests { assert!(help.contains("/export [file]")); assert!(help.contains("/session [list|switch ]")); assert!(help.contains( - "/plugins [list|install |enable |disable |uninstall |update ]" + "/plugins [list|install |enable |disable |uninstall |update ]" )); assert_eq!(slash_command_specs().len(), 16); assert_eq!(resume_supported_slash_commands().len(), 11); @@ -748,10 +762,10 @@ mod tests { }, ]); - assert!(rendered.contains("demo@external")); + assert!(rendered.contains("demo")); assert!(rendered.contains("v1.2.3")); assert!(rendered.contains("enabled")); - assert!(rendered.contains("sample@external")); + assert!(rendered.contains("sample")); assert!(rendered.contains("v0.9.0")); assert!(rendered.contains("disabled")); } @@ -778,7 +792,7 @@ mod tests { let list = handle_plugins_slash_command(Some("list"), None, &mut manager) .expect("list command should succeed"); assert!(!list.reload_runtime); - assert!(list.message.contains("demo@external")); + assert!(list.message.contains("demo")); assert!(list.message.contains("v1.0.0")); assert!(list.message.contains("enabled")); @@ -809,7 +823,7 @@ mod tests { let list = handle_plugins_slash_command(Some("list"), None, &mut manager) .expect("list command should succeed"); - assert!(list.message.contains("demo@external")); + assert!(list.message.contains("demo")); assert!(list.message.contains("disabled")); let enable = handle_plugins_slash_command(Some("enable"), Some("demo"), &mut manager) @@ -821,7 +835,7 @@ mod tests { let list = handle_plugins_slash_command(Some("list"), None, &mut manager) .expect("list command should succeed"); - assert!(list.message.contains("demo@external")); + assert!(list.message.contains("demo")); assert!(list.message.contains("enabled")); let _ = fs::remove_dir_all(config_home); @@ -842,8 +856,8 @@ mod tests { let list = handle_plugins_slash_command(Some("list"), None, &mut manager) .expect("list command should succeed"); assert!(!list.reload_runtime); - assert!(list.message.contains("starter@bundled")); - assert!(list.message.contains("bundled")); + assert!(list.message.contains("starter")); + assert!(list.message.contains("v0.1.0")); assert!(list.message.contains("disabled")); let _ = fs::remove_dir_all(config_home); diff --git a/rust/crates/plugins/src/hooks.rs b/rust/crates/plugins/src/hooks.rs index c0a7ec1..d473da8 100644 --- a/rust/crates/plugins/src/hooks.rs +++ b/rust/crates/plugins/src/hooks.rs @@ -133,11 +133,9 @@ impl HookRunner { } } HookCommandOutcome::Deny { message } => { - messages.push( - message.unwrap_or_else(|| { - format!("{} hook denied tool `{tool_name}`", event.as_str()) - }), - ); + messages.push(message.unwrap_or_else(|| { + format!("{} hook denied tool `{tool_name}`", event.as_str()) + })); return HookRunResult { denied: true, messages, @@ -150,7 +148,7 @@ impl HookRunner { HookRunResult::allow(messages) } - #[allow(clippy::too_many_arguments)] + #[allow(clippy::too_many_arguments, clippy::unused_self)] fn run_command( &self, command: &str, @@ -338,8 +336,18 @@ mod tests { let config_home = temp_dir("config"); let first_source_root = temp_dir("source-a"); let second_source_root = temp_dir("source-b"); - write_hook_plugin(&first_source_root, "first", "plugin pre one", "plugin post one"); - write_hook_plugin(&second_source_root, "second", "plugin pre two", "plugin post two"); + write_hook_plugin( + &first_source_root, + "first", + "plugin pre one", + "plugin post one", + ); + write_hook_plugin( + &second_source_root, + "second", + "plugin pre two", + "plugin post two", + ); let mut manager = PluginManager::new(PluginManagerConfig::new(&config_home)); manager diff --git a/rust/crates/plugins/src/lib.rs b/rust/crates/plugins/src/lib.rs index 6ff0d9d..185a877 100644 --- a/rust/crates/plugins/src/lib.rs +++ b/rust/crates/plugins/src/lib.rs @@ -1,6 +1,6 @@ mod hooks; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use std::fmt::{Display, Formatter}; use std::fs; use std::path::{Path, PathBuf}; @@ -38,7 +38,6 @@ impl Display for PluginKind { } } - impl PluginKind { #[must_use] fn marketplace(self) -> &'static str { @@ -109,8 +108,7 @@ pub struct PluginManifest { pub name: String, pub version: String, pub description: String, - #[serde(default)] - pub permissions: Vec, + pub permissions: Vec, #[serde(rename = "defaultEnabled", default)] pub default_enabled: bool, #[serde(default)] @@ -123,6 +121,34 @@ pub struct PluginManifest { pub commands: Vec, } +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum PluginPermission { + Read, + Write, + Execute, +} + +impl PluginPermission { + #[must_use] + pub fn as_str(self) -> &'static str { + match self { + Self::Read => "read", + Self::Write => "write", + Self::Execute => "execute", + } + } + + fn parse(value: &str) -> Option { + match value { + "read" => Some(Self::Read), + "write" => Some(Self::Write), + "execute" => Some(Self::Execute), + _ => None, + } + } +} + #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct PluginToolManifest { pub name: String, @@ -132,8 +158,35 @@ pub struct PluginToolManifest { pub command: String, #[serde(default)] pub args: Vec, - #[serde(rename = "requiredPermission", default = "default_tool_permission")] - pub required_permission: String, + pub required_permission: PluginToolPermission, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub enum PluginToolPermission { + ReadOnly, + WorkspaceWrite, + DangerFullAccess, +} + +impl PluginToolPermission { + #[must_use] + pub fn as_str(self) -> &'static str { + match self { + Self::ReadOnly => "read-only", + Self::WorkspaceWrite => "workspace-write", + Self::DangerFullAccess => "danger-full-access", + } + } + + fn parse(value: &str) -> Option { + match value { + "read-only" => Some(Self::ReadOnly), + "workspace-write" => Some(Self::WorkspaceWrite), + "danger-full-access" => Some(Self::DangerFullAccess), + _ => None, + } + } } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] @@ -152,6 +205,38 @@ pub struct PluginCommandManifest { pub command: String, } +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +struct RawPluginManifest { + pub name: String, + pub version: String, + pub description: String, + #[serde(default)] + pub permissions: Vec, + #[serde(rename = "defaultEnabled", default)] + pub default_enabled: bool, + #[serde(default)] + pub hooks: PluginHooks, + #[serde(default)] + pub lifecycle: PluginLifecycle, + #[serde(default)] + pub tools: Vec, + #[serde(default)] + pub commands: Vec, +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +struct RawPluginToolManifest { + pub name: String, + pub description: String, + #[serde(rename = "inputSchema")] + pub input_schema: Value, + pub command: String, + #[serde(default)] + pub args: Vec, + #[serde(rename = "requiredPermission", default = "default_raw_tool_permission")] + pub required_permission: String, +} + type PluginPackageManifest = PluginManifest; #[derive(Debug, Clone, PartialEq)] @@ -161,7 +246,7 @@ pub struct PluginTool { definition: PluginToolDefinition, command: String, args: Vec, - required_permission: String, + required_permission: PluginToolPermission, root: Option, } @@ -173,7 +258,7 @@ impl PluginTool { definition: PluginToolDefinition, command: impl Into, args: Vec, - required_permission: impl Into, + required_permission: PluginToolPermission, root: Option, ) -> Self { Self { @@ -182,7 +267,7 @@ impl PluginTool { definition, command: command.into(), args, - required_permission: required_permission.into(), + required_permission, root, } } @@ -199,7 +284,7 @@ impl PluginTool { #[must_use] pub fn required_permission(&self) -> &str { - &self.required_permission + self.required_permission.as_str() } pub fn execute(&self, input: &Value) -> Result { @@ -246,7 +331,7 @@ impl PluginTool { } } -fn default_tool_permission() -> String { +fn default_raw_tool_permission() -> String { "danger-full-access".to_string() } @@ -686,10 +771,74 @@ pub struct UpdateOutcome { pub install_path: PathBuf, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum PluginManifestValidationError { + EmptyField { field: &'static str }, + EmptyEntryField { + kind: &'static str, + field: &'static str, + name: Option, + }, + InvalidPermission { permission: String }, + DuplicatePermission { permission: String }, + DuplicateEntry { kind: &'static str, name: String }, + MissingPath { kind: &'static str, path: PathBuf }, + InvalidToolInputSchema { tool_name: String }, + InvalidToolRequiredPermission { + tool_name: String, + permission: String, + }, +} + +impl Display for PluginManifestValidationError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match self { + Self::EmptyField { field } => { + write!(f, "plugin manifest {field} cannot be empty") + } + Self::EmptyEntryField { kind, field, name } => match name { + Some(name) if !name.is_empty() => { + write!(f, "plugin {kind} `{name}` {field} cannot be empty") + } + _ => write!(f, "plugin {kind} {field} cannot be empty"), + }, + Self::InvalidPermission { permission } => { + write!( + f, + "plugin manifest permission `{permission}` must be one of read, write, or execute" + ) + } + Self::DuplicatePermission { permission } => { + write!(f, "plugin manifest permission `{permission}` is duplicated") + } + Self::DuplicateEntry { kind, name } => { + write!(f, "plugin {kind} `{name}` is duplicated") + } + Self::MissingPath { kind, path } => { + write!(f, "{kind} path `{}` does not exist", path.display()) + } + Self::InvalidToolInputSchema { tool_name } => { + write!( + f, + "plugin tool `{tool_name}` inputSchema must be a JSON object" + ) + } + Self::InvalidToolRequiredPermission { + tool_name, + permission, + } => write!( + f, + "plugin tool `{tool_name}` requiredPermission `{permission}` must be read-only, workspace-write, or danger-full-access" + ), + } + } +} + #[derive(Debug)] pub enum PluginError { Io(std::io::Error), Json(serde_json::Error), + ManifestValidation(Vec), InvalidManifest(String), NotFound(String), CommandFailed(String), @@ -700,6 +849,15 @@ impl Display for PluginError { match self { Self::Io(error) => write!(f, "{error}"), Self::Json(error) => write!(f, "{error}"), + Self::ManifestValidation(errors) => { + for (index, error) in errors.iter().enumerate() { + if index > 0 { + write!(f, "; ")?; + } + write!(f, "{error}")?; + } + Ok(()) + } Self::InvalidManifest(message) | Self::NotFound(message) | Self::CommandFailed(message) => write!(f, "{message}"), @@ -992,7 +1150,7 @@ impl PluginManager { let install_path = install_root.join(sanitize_plugin_id(&plugin_id)); let now = unix_time_ms(); let existing_record = registry.plugins.get(&plugin_id); - let needs_sync = existing_record.map_or(true, |record| { + let needs_sync = existing_record.is_none_or(|record| { record.kind != PluginKind::Bundled || record.version != manifest.version || record.name != manifest.name @@ -1010,7 +1168,8 @@ impl PluginManager { } copy_dir_all(&source_root, &install_path)?; - let installed_at_unix_ms = existing_record.map_or(now, |record| record.installed_at_unix_ms); + let installed_at_unix_ms = + existing_record.map_or(now, |record| record.installed_at_unix_ms); registry.plugins.insert( plugin_id.clone(), InstalledPluginRecord { @@ -1261,7 +1420,7 @@ fn plugin_manifest_path(root: &Path) -> Result { } fn validate_named_strings(entries: &[String], kind: &str) -> Result<(), PluginError> { - let mut seen = BTreeMap::<&str, ()>::new(); + let mut seen = BTreeSet::<&str>::new(); for entry in entries { let trimmed = entry.trim(); if trimmed.is_empty() { @@ -1269,7 +1428,7 @@ fn validate_named_strings(entries: &[String], kind: &str) -> Result<(), PluginEr "plugin manifest {kind} cannot be empty" ))); } - if seen.insert(trimmed, ()).is_some() { + if !seen.insert(trimmed) { return Err(PluginError::InvalidManifest(format!( "plugin manifest {kind} `{trimmed}` is duplicated" ))); @@ -1283,7 +1442,7 @@ fn validate_named_commands( entries: &[impl NamedCommand], kind: &str, ) -> Result<(), PluginError> { - let mut seen = BTreeMap::<&str, ()>::new(); + let mut seen = BTreeSet::<&str>::new(); for entry in entries { let name = entry.name().trim(); if name.is_empty() { @@ -1291,7 +1450,7 @@ fn validate_named_commands( "plugin {kind} name cannot be empty" ))); } - if seen.insert(name, ()).is_some() { + if !seen.insert(name) { return Err(PluginError::InvalidManifest(format!( "plugin {kind} `{name}` is duplicated" ))); @@ -1796,6 +1955,59 @@ mod tests { log_path } + fn write_tool_plugin(root: &Path, name: &str, version: &str) { + let script_path = root.join("tools").join("echo-json.sh"); + write_file( + &script_path, + "#!/bin/sh\nINPUT=$(cat)\nprintf '{\"plugin\":\"%s\",\"tool\":\"%s\",\"input\":%s}\\n' \"$CLAWD_PLUGIN_ID\" \"$CLAWD_TOOL_NAME\" \"$INPUT\"\n", + ); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + + let mut permissions = fs::metadata(&script_path).expect("metadata").permissions(); + permissions.set_mode(0o755); + fs::set_permissions(&script_path, permissions).expect("chmod"); + } + write_file( + root.join(MANIFEST_RELATIVE_PATH).as_path(), + format!( + "{{\n \"name\": \"{name}\",\n \"version\": \"{version}\",\n \"description\": \"tool plugin\",\n \"tools\": [\n {{\n \"name\": \"plugin_echo\",\n \"description\": \"Echo JSON input\",\n \"inputSchema\": {{\"type\": \"object\", \"properties\": {{\"message\": {{\"type\": \"string\"}}}}, \"required\": [\"message\"], \"additionalProperties\": false}},\n \"command\": \"./tools/echo-json.sh\",\n \"requiredPermission\": \"workspace-write\"\n }}\n ]\n}}" + ) + .as_str(), + ); + } + + fn write_bundled_plugin(root: &Path, name: &str, version: &str, default_enabled: bool) { + write_file( + root.join(MANIFEST_RELATIVE_PATH).as_path(), + format!( + "{{\n \"name\": \"{name}\",\n \"version\": \"{version}\",\n \"description\": \"bundled plugin\",\n \"defaultEnabled\": {}\n}}", + if default_enabled { "true" } else { "false" } + ) + .as_str(), + ); + } + + fn load_enabled_plugins(path: &Path) -> BTreeMap { + let contents = fs::read_to_string(path).expect("settings should exist"); + let root: Value = serde_json::from_str(&contents).expect("settings json"); + root.get("enabledPlugins") + .and_then(Value::as_object) + .map(|enabled_plugins| { + enabled_plugins + .iter() + .map(|(plugin_id, value)| { + ( + plugin_id.clone(), + value.as_bool().expect("plugin state should be a bool"), + ) + }) + .collect() + }) + .unwrap_or_default() + } + #[test] fn load_plugin_from_directory_validates_required_fields() { let root = temp_dir("manifest-required"); @@ -1977,6 +2189,70 @@ mod tests { let _ = fs::remove_dir_all(source_root); } + #[test] + fn auto_installs_bundled_plugins_into_the_registry() { + let config_home = temp_dir("bundled-home"); + let bundled_root = temp_dir("bundled-root"); + write_bundled_plugin(&bundled_root.join("starter"), "starter", "0.1.0", false); + + let mut config = PluginManagerConfig::new(&config_home); + config.bundled_root = Some(bundled_root.clone()); + let manager = PluginManager::new(config); + + let installed = manager + .list_installed_plugins() + .expect("bundled plugins should auto-install"); + assert!(installed.iter().any(|plugin| { + plugin.metadata.id == "starter@bundled" + && plugin.metadata.kind == PluginKind::Bundled + && !plugin.enabled + })); + + let registry = manager.load_registry().expect("registry should exist"); + let record = registry + .plugins + .get("starter@bundled") + .expect("bundled plugin should be recorded"); + assert_eq!(record.kind, PluginKind::Bundled); + assert!(record.install_path.exists()); + + let _ = fs::remove_dir_all(config_home); + let _ = fs::remove_dir_all(bundled_root); + } + + #[test] + fn persists_bundled_plugin_enable_state_across_reloads() { + let config_home = temp_dir("bundled-state-home"); + let bundled_root = temp_dir("bundled-state-root"); + write_bundled_plugin(&bundled_root.join("starter"), "starter", "0.1.0", false); + + let mut config = PluginManagerConfig::new(&config_home); + config.bundled_root = Some(bundled_root.clone()); + let mut manager = PluginManager::new(config.clone()); + + manager + .enable("starter@bundled") + .expect("enable bundled plugin should succeed"); + assert_eq!( + load_enabled_plugins(&manager.settings_path()).get("starter@bundled"), + Some(&true) + ); + + let mut reloaded_config = PluginManagerConfig::new(&config_home); + reloaded_config.bundled_root = Some(bundled_root.clone()); + reloaded_config.enabled_plugins = load_enabled_plugins(&manager.settings_path()); + let reloaded_manager = PluginManager::new(reloaded_config); + let reloaded = reloaded_manager + .list_installed_plugins() + .expect("bundled plugins should still be listed"); + assert!(reloaded + .iter() + .any(|plugin| { plugin.metadata.id == "starter@bundled" && plugin.enabled })); + + let _ = fs::remove_dir_all(config_home); + let _ = fs::remove_dir_all(bundled_root); + } + #[test] fn validates_plugin_source_before_install() { let config_home = temp_dir("validate-home"); @@ -2062,4 +2338,32 @@ mod tests { let _ = fs::remove_dir_all(config_home); let _ = fs::remove_dir_all(source_root); } + + #[test] + fn aggregates_and_executes_plugin_tools() { + let config_home = temp_dir("tool-home"); + let source_root = temp_dir("tool-source"); + write_tool_plugin(&source_root, "tool-demo", "1.0.0"); + + let mut manager = PluginManager::new(PluginManagerConfig::new(&config_home)); + manager + .install(source_root.to_str().expect("utf8 path")) + .expect("install should succeed"); + + let tools = manager.aggregated_tools().expect("tools should aggregate"); + assert_eq!(tools.len(), 1); + assert_eq!(tools[0].definition().name, "plugin_echo"); + assert_eq!(tools[0].required_permission(), "workspace-write"); + + let output = tools[0] + .execute(&serde_json::json!({ "message": "hello" })) + .expect("plugin tool should execute"); + let payload: Value = serde_json::from_str(&output).expect("valid json"); + assert_eq!(payload["plugin"], "tool-demo@external"); + assert_eq!(payload["tool"], "plugin_echo"); + assert_eq!(payload["input"]["message"], "hello"); + + let _ = fs::remove_dir_all(config_home); + let _ = fs::remove_dir_all(source_root); + } } diff --git a/rust/crates/runtime/src/conversation.rs b/rust/crates/runtime/src/conversation.rs index 3637aa4..ffbff39 100644 --- a/rust/crates/runtime/src/conversation.rs +++ b/rust/crates/runtime/src/conversation.rs @@ -178,8 +178,10 @@ where feature_config: RuntimeFeatureConfig, plugin_registry: PluginRegistry, ) -> Result { - let plugin_hook_runner = PluginHookRunner::from_registry(&plugin_registry) - .map_err(|error| RuntimeError::new(format!("plugin hook registration failed: {error}")))?; + let plugin_hook_runner = + PluginHookRunner::from_registry(&plugin_registry).map_err(|error| { + RuntimeError::new(format!("plugin hook registration failed: {error}")) + })?; plugin_registry .initialize() .map_err(|error| RuntimeError::new(format!("plugin initialization failed: {error}")))?; @@ -202,6 +204,7 @@ where self } + #[allow(clippy::too_many_lines)] pub fn run_turn( &mut self, user_input: impl Into, @@ -275,56 +278,64 @@ where if plugin_pre_hook_result.is_denied() { let deny_message = format!("PreToolUse hook denied tool `{tool_name}`"); + let mut messages = pre_hook_result.messages().to_vec(); + messages.extend(plugin_pre_hook_result.messages().iter().cloned()); ConversationMessage::tool_result( tool_use_id, tool_name, - format_hook_message( - plugin_pre_hook_result.messages(), - &deny_message, - ), + format_hook_message(&messages, &deny_message), true, ) } else { - let (mut output, mut is_error) = - match self.tool_executor.execute(&tool_name, &input) { - Ok(output) => (output, false), - Err(error) => (error.to_string(), true), - }; - output = merge_hook_feedback(pre_hook_result.messages(), output, false); - output = merge_hook_feedback( - plugin_pre_hook_result.messages(), - output, - false, - ); + let (mut output, mut is_error) = + match self.tool_executor.execute(&tool_name, &input) { + Ok(output) => (output, false), + Err(error) => (error.to_string(), true), + }; + output = + merge_hook_feedback(pre_hook_result.messages(), output, false); + output = merge_hook_feedback( + plugin_pre_hook_result.messages(), + output, + false, + ); - let post_hook_result = self - .hook_runner - .run_post_tool_use(&tool_name, &input, &output, is_error); - if post_hook_result.is_denied() { - is_error = true; - } - output = merge_hook_feedback( - post_hook_result.messages(), - output, - post_hook_result.is_denied(), - ); - let plugin_post_hook_result = - self.run_plugin_post_tool_use(&tool_name, &input, &output, is_error); - if plugin_post_hook_result.is_denied() { - is_error = true; - } - output = merge_hook_feedback( - plugin_post_hook_result.messages(), - output, - plugin_post_hook_result.is_denied(), - ); + let hook_output = output.clone(); + let post_hook_result = self.hook_runner.run_post_tool_use( + &tool_name, + &input, + &hook_output, + is_error, + ); + let plugin_post_hook_result = self.run_plugin_post_tool_use( + &tool_name, + &input, + &hook_output, + is_error, + ); + if post_hook_result.is_denied() { + is_error = true; + } + if plugin_post_hook_result.is_denied() { + is_error = true; + } + output = merge_hook_feedback( + post_hook_result.messages(), + output, + post_hook_result.is_denied(), + ); + output = merge_hook_feedback( + plugin_post_hook_result.messages(), + output, + plugin_post_hook_result.is_denied(), + ); - ConversationMessage::tool_result( - tool_use_id, - tool_name, - output, - is_error, - ) + ConversationMessage::tool_result( + tool_use_id, + tool_name, + output, + is_error, + ) } } } @@ -449,11 +460,11 @@ fn flush_text_block(text: &mut String, blocks: &mut Vec) { } } -fn format_hook_message(result: &HookRunResult, fallback: &str) -> String { - if result.messages().is_empty() { +fn format_hook_message(messages: &[String], fallback: &str) -> String { + if messages.is_empty() { fallback.to_string() } else { - result.messages().join("\n") + messages.join("\n") } } diff --git a/rust/crates/rusty-claude-cli/src/main.rs b/rust/crates/rusty-claude-cli/src/main.rs index 4e415dd..e9e3c19 100644 --- a/rust/crates/rusty-claude-cli/src/main.rs +++ b/rust/crates/rusty-claude-cli/src/main.rs @@ -2,7 +2,7 @@ mod init; mod input; mod render; -use std::collections::{BTreeMap, BTreeSet}; +use std::collections::BTreeSet; use std::env; use std::fmt::Write as _; use std::fs; @@ -1912,8 +1912,7 @@ fn build_system_prompt() -> Result, Box> { )?) } -fn build_runtime_plugin_state( -) -> Result< +fn build_runtime_plugin_state() -> Result< ( runtime::RuntimeFeatureConfig, PluginRegistry, @@ -1927,7 +1926,11 @@ fn build_runtime_plugin_state( let plugin_manager = build_plugin_manager(&cwd, &loader, &runtime_config); let plugin_registry = plugin_manager.plugin_registry()?; let tool_registry = GlobalToolRegistry::with_plugin_tools(plugin_registry.aggregated_tools()?)?; - Ok((runtime_config.feature_config().clone(), plugin_registry, tool_registry)) + Ok(( + runtime_config.feature_config().clone(), + plugin_registry, + tool_registry, + )) } fn build_plugin_manager( @@ -2737,12 +2740,12 @@ impl ToolExecutor for CliToolExecutor { } fn permission_policy(mode: PermissionMode, tool_registry: &GlobalToolRegistry) -> PermissionPolicy { - tool_registry - .permission_specs(None) - .into_iter() - .fold(PermissionPolicy::new(mode), |policy, (name, required_permission)| { + tool_registry.permission_specs(None).into_iter().fold( + PermissionPolicy::new(mode), + |policy, (name, required_permission)| { policy.with_tool_requirement(name, required_permission) - }) + }, + ) } fn convert_messages(messages: &[ConversationMessage]) -> Vec { @@ -2893,6 +2896,7 @@ mod tests { use runtime::{AssistantEvent, ContentBlock, ConversationMessage, MessageRole, PermissionMode}; use serde_json::json; use std::path::PathBuf; + use tools::GlobalToolRegistry; #[test] fn defaults_to_repl_when_no_args() { @@ -3106,7 +3110,7 @@ mod tests { .into_iter() .map(str::to_string) .collect(); - let filtered = filter_tool_specs(Some(&allowed)); + let filtered = filter_tool_specs(&GlobalToolRegistry::builtin(), Some(&allowed)); let names = filtered .into_iter() .map(|spec| spec.name) @@ -3140,7 +3144,7 @@ mod tests { assert!(help.contains("/export [file]")); assert!(help.contains("/session [list|switch ]")); assert!(help.contains( - "/plugins [list|install |enable |disable |uninstall |update ]" + "/plugins [list|install |enable |disable |uninstall |update ]" )); assert!(help.contains("/exit")); } diff --git a/rust/crates/tools/src/lib.rs b/rust/crates/tools/src/lib.rs index 74d1b61..c28faf6 100644 --- a/rust/crates/tools/src/lib.rs +++ b/rust/crates/tools/src/lib.rs @@ -218,7 +218,9 @@ impl GlobalToolRegistry { .ok_or_else(|| format!("unsupported tool: {name}"))?; match &entry.handler { RegisteredToolHandler::Builtin => execute_tool(name, input), - RegisteredToolHandler::Plugin(tool) => tool.execute(input).map_err(|error| error.to_string()), + RegisteredToolHandler::Plugin(tool) => { + tool.execute(input).map_err(|error| error.to_string()) + } } } } @@ -3094,8 +3096,9 @@ mod tests { use super::{ agent_permission_policy, allowed_tools_for_subagent, execute_agent_with_spawn, execute_tool, final_assistant_text, mvp_tool_specs, persist_agent_terminal_state, - AgentInput, AgentJob, SubagentToolExecutor, + AgentInput, AgentJob, GlobalToolRegistry, SubagentToolExecutor, }; + use plugins::{PluginTool, PluginToolDefinition}; use runtime::{ApiRequest, AssistantEvent, ConversationRuntime, RuntimeError, Session}; use serde_json::json; @@ -3112,6 +3115,17 @@ mod tests { std::env::temp_dir().join(format!("clawd-tools-{unique}-{name}")) } + fn make_executable(path: &PathBuf) { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + + let mut permissions = std::fs::metadata(path).expect("metadata").permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(path, permissions).expect("chmod"); + } + } + #[test] fn exposes_mvp_tools() { let names = mvp_tool_specs() @@ -3141,6 +3155,75 @@ mod tests { assert!(error.contains("unsupported tool")); } + #[test] + fn global_registry_registers_and_executes_plugin_tools() { + let script = temp_path("plugin-tool.sh"); + std::fs::write( + &script, + "#!/bin/sh\nINPUT=$(cat)\nprintf '{\"plugin\":\"%s\",\"tool\":\"%s\",\"input\":%s}\\n' \"$CLAWD_PLUGIN_ID\" \"$CLAWD_TOOL_NAME\" \"$INPUT\"\n", + ) + .expect("write script"); + make_executable(&script); + + let registry = GlobalToolRegistry::with_plugin_tools(vec![PluginTool::new( + "demo@external", + "demo", + PluginToolDefinition { + name: "plugin_echo".to_string(), + description: Some("Echo plugin input".to_string()), + input_schema: json!({ + "type": "object", + "properties": { "message": { "type": "string" } }, + "required": ["message"], + "additionalProperties": false + }), + }, + script.display().to_string(), + Vec::new(), + "workspace-write", + script.parent().map(PathBuf::from), + )]) + .expect("registry should build"); + + let names = registry + .definitions(None) + .into_iter() + .map(|definition| definition.name) + .collect::>(); + assert!(names.contains(&"bash".to_string())); + assert!(names.contains(&"plugin_echo".to_string())); + + let output = registry + .execute("plugin_echo", &json!({ "message": "hello" })) + .expect("plugin tool should execute"); + let payload: serde_json::Value = serde_json::from_str(&output).expect("valid json"); + assert_eq!(payload["plugin"], "demo@external"); + assert_eq!(payload["tool"], "plugin_echo"); + assert_eq!(payload["input"]["message"], "hello"); + + let _ = std::fs::remove_file(script); + } + + #[test] + fn global_registry_rejects_conflicting_plugin_tool_names() { + let error = GlobalToolRegistry::with_plugin_tools(vec![PluginTool::new( + "demo@external", + "demo", + PluginToolDefinition { + name: "read-file".to_string(), + description: Some("Conflicts with builtin".to_string()), + input_schema: json!({ "type": "object" }), + }, + "echo".to_string(), + Vec::new(), + "read-only", + None, + )]) + .expect_err("conflicting plugin tool should fail"); + + assert!(error.contains("conflicts with already-registered tool `read_file`")); + } + #[test] fn web_fetch_returns_prompt_aware_summary() { let server = TestServer::spawn(Arc::new(|request_line: &str| {