diff --git a/rust/crates/plugins/src/lib.rs b/rust/crates/plugins/src/lib.rs index 1c2f134..e45c7d7 100644 --- a/rust/crates/plugins/src/lib.rs +++ b/rust/crates/plugins/src/lib.rs @@ -1098,6 +1098,7 @@ impl PluginManager { let registry = self.load_registry()?; let mut plugins = Vec::new(); let mut seen_ids = BTreeSet::::new(); + let mut seen_paths = BTreeSet::::new(); for install_path in discover_plugin_dirs(&self.install_root())? { let matched_record = registry @@ -1111,6 +1112,23 @@ impl PluginManager { ); let plugin = load_plugin_definition(&install_path, kind, source, kind.marketplace())?; if seen_ids.insert(plugin.metadata().id.clone()) { + seen_paths.insert(install_path); + plugins.push(plugin); + } + } + + for record in registry.plugins.values() { + if seen_paths.contains(&record.install_path) { + continue; + } + let plugin = load_plugin_definition( + &record.install_path, + record.kind, + describe_install_source(&record.source), + record.kind.marketplace(), + )?; + if seen_ids.insert(plugin.metadata().id.clone()) { + seen_paths.insert(record.install_path.clone()); plugins.push(plugin); } } @@ -1165,17 +1183,15 @@ impl PluginManager { .clone() .unwrap_or_else(Self::bundled_root); let bundled_plugins = discover_plugin_dirs(&bundled_root)?; - if bundled_plugins.is_empty() { - return Ok(()); - } - let mut registry = self.load_registry()?; let mut changed = false; let install_root = self.install_root(); + let mut active_bundled_ids = BTreeSet::new(); for source_root in bundled_plugins { let manifest = load_plugin_from_directory(&source_root)?; let plugin_id = plugin_id(&manifest.name, BUNDLED_MARKETPLACE); + active_bundled_ids.insert(plugin_id.clone()); let install_path = install_root.join(sanitize_plugin_id(&plugin_id)); let now = unix_time_ms(); let existing_record = registry.plugins.get(&plugin_id); @@ -1216,6 +1232,24 @@ impl PluginManager { changed = true; } + let stale_bundled_ids = registry + .plugins + .iter() + .filter_map(|(plugin_id, record)| { + (record.kind == PluginKind::Bundled && !active_bundled_ids.contains(plugin_id)) + .then_some(plugin_id.clone()) + }) + .collect::>(); + + for plugin_id in stale_bundled_ids { + if let Some(record) = registry.plugins.remove(&plugin_id) { + if record.install_path.exists() { + fs::remove_dir_all(&record.install_path)?; + } + changed = true; + } + } + if changed { self.store_registry(®istry)?; } @@ -2482,6 +2516,117 @@ mod tests { let _ = fs::remove_dir_all(config_home); } + #[test] + fn bundled_sync_prunes_removed_bundled_registry_entries() { + let config_home = temp_dir("bundled-prune-home"); + let bundled_root = temp_dir("bundled-prune-root"); + let stale_install_path = config_home + .join("plugins") + .join("installed") + .join("stale-bundled-external"); + write_bundled_plugin(&bundled_root.join("active"), "active", "0.1.0", false); + write_file( + stale_install_path.join(MANIFEST_RELATIVE_PATH).as_path(), + r#"{ + "name": "stale", + "version": "0.1.0", + "description": "stale bundled plugin" +}"#, + ); + + let mut config = PluginManagerConfig::new(&config_home); + config.bundled_root = Some(bundled_root.clone()); + config.install_root = Some(config_home.join("plugins").join("installed")); + let manager = PluginManager::new(config); + + let mut registry = InstalledPluginRegistry::default(); + registry.plugins.insert( + "stale@bundled".to_string(), + InstalledPluginRecord { + kind: PluginKind::Bundled, + id: "stale@bundled".to_string(), + name: "stale".to_string(), + version: "0.1.0".to_string(), + description: "stale bundled plugin".to_string(), + install_path: stale_install_path.clone(), + source: PluginInstallSource::LocalPath { + path: bundled_root.join("stale"), + }, + installed_at_unix_ms: 1, + updated_at_unix_ms: 1, + }, + ); + manager.store_registry(®istry).expect("store registry"); + + let installed = manager + .list_installed_plugins() + .expect("bundled sync should succeed"); + assert!(installed + .iter() + .any(|plugin| plugin.metadata.id == "active@bundled")); + assert!(!installed + .iter() + .any(|plugin| plugin.metadata.id == "stale@bundled")); + + let registry = manager.load_registry().expect("load registry"); + assert!(!registry.plugins.contains_key("stale@bundled")); + assert!(!stale_install_path.exists()); + + let _ = fs::remove_dir_all(config_home); + let _ = fs::remove_dir_all(bundled_root); + } + + #[test] + fn installed_plugin_discovery_keeps_registry_entries_outside_install_root() { + let config_home = temp_dir("registry-fallback-home"); + let bundled_root = temp_dir("registry-fallback-bundled"); + let install_root = config_home.join("plugins").join("installed"); + let external_install_path = temp_dir("registry-fallback-external"); + write_file( + external_install_path.join(MANIFEST_FILE_NAME).as_path(), + r#"{ + "name": "registry-fallback", + "version": "1.0.0", + "description": "Registry fallback plugin" +}"#, + ); + + let mut config = PluginManagerConfig::new(&config_home); + config.bundled_root = Some(bundled_root.clone()); + config.install_root = Some(install_root.clone()); + let manager = PluginManager::new(config); + + let mut registry = InstalledPluginRegistry::default(); + registry.plugins.insert( + "registry-fallback@external".to_string(), + InstalledPluginRecord { + kind: PluginKind::External, + id: "registry-fallback@external".to_string(), + name: "registry-fallback".to_string(), + version: "1.0.0".to_string(), + description: "Registry fallback plugin".to_string(), + install_path: external_install_path.clone(), + source: PluginInstallSource::LocalPath { + path: external_install_path.clone(), + }, + installed_at_unix_ms: 1, + updated_at_unix_ms: 1, + }, + ); + manager.store_registry(®istry).expect("store registry"); + + let installed = manager + .list_installed_plugins() + .expect("registry fallback plugin should load"); + assert!(installed + .iter() + .any(|plugin| plugin.metadata.id == "registry-fallback@external")); + + let _ = fs::remove_dir_all(config_home); + let _ = fs::remove_dir_all(bundled_root); + let _ = fs::remove_dir_all(external_install_path); + } + #[test] fn persists_bundled_plugin_enable_state_across_reloads() { let config_home = temp_dir("bundled-state-home"); diff --git a/rust/crates/tools/src/lib.rs b/rust/crates/tools/src/lib.rs index fea9ce1..33b24e9 100644 --- a/rust/crates/tools/src/lib.rs +++ b/rust/crates/tools/src/lib.rs @@ -136,6 +136,13 @@ impl GlobalToolRegistry { &self.entries } + fn find_entry(&self, name: &str) -> Option<&RegisteredTool> { + let normalized = normalize_registry_tool_name(name); + self.entries.iter().find(|entry| { + normalize_registry_tool_name(entry.definition.name.as_str()) == normalized + }) + } + #[must_use] pub fn definitions(&self, allowed_tools: Option<&BTreeSet>) -> Vec { self.entries @@ -213,12 +220,10 @@ impl GlobalToolRegistry { pub fn execute(&self, name: &str, input: &Value) -> Result { let entry = self - .entries - .iter() - .find(|entry| entry.definition.name == name) + .find_entry(name) .ok_or_else(|| format!("unsupported tool: {name}"))?; match &entry.handler { - RegisteredToolHandler::Builtin => execute_tool(name, input), + RegisteredToolHandler::Builtin => execute_tool(&entry.definition.name, input), RegisteredToolHandler::Plugin(tool) => { tool.execute(input).map_err(|error| error.to_string()) } @@ -233,7 +238,44 @@ impl Default for GlobalToolRegistry { } fn normalize_registry_tool_name(value: &str) -> String { - value.trim().replace('-', "_").to_ascii_lowercase() + let trimmed = value.trim(); + let chars = trimmed.chars().collect::>(); + let mut normalized = String::new(); + + for (index, ch) in chars.iter().copied().enumerate() { + if matches!(ch, '-' | ' ' | '\t' | '\n') { + if !normalized.ends_with('_') { + normalized.push('_'); + } + continue; + } + + if ch == '_' { + if !normalized.ends_with('_') { + normalized.push('_'); + } + continue; + } + + if ch.is_uppercase() { + let prev = chars.get(index.wrapping_sub(1)).copied(); + let next = chars.get(index + 1).copied(); + let needs_separator = index > 0 + && !normalized.ends_with('_') + && (prev.is_some_and(|prev| prev.is_lowercase() || prev.is_ascii_digit()) + || (prev.is_some_and(char::is_uppercase) + && next.is_some_and(char::is_lowercase))); + if needs_separator { + normalized.push('_'); + } + normalized.extend(ch.to_lowercase()); + continue; + } + + normalized.push(ch.to_ascii_lowercase()); + } + + normalized.trim_matches('_').to_string() } fn permission_mode_from_plugin_tool(value: &str) -> Result { @@ -3205,6 +3247,59 @@ mod tests { let _ = std::fs::remove_file(script); } + #[test] + fn global_registry_normalizes_plugin_tool_names_for_allowlists_and_execution() { + let script = temp_path("plugin-tool-normalized.sh"); + std::fs::write( + &script, + "#!/bin/sh\nINPUT=$(cat)\nprintf '{\"tool\":\"%s\",\"input\":%s}\\n' \"$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(), + PluginToolPermission::WorkspaceWrite, + script.parent().map(PathBuf::from), + )]) + .expect("registry should build"); + + let allowed = registry + .normalize_allowed_tools(&[String::from("PLUGIN-ECHO")]) + .expect("plugin tool allowlist should normalize") + .expect("allowlist should be present"); + assert!(allowed.contains("plugin_echo")); + + let output = registry + .execute("plugin-echo", &json!({ "message": "hello" })) + .expect("normalized plugin tool name should execute"); + let payload: serde_json::Value = serde_json::from_str(&output).expect("valid json"); + assert_eq!(payload["tool"], "plugin_echo"); + assert_eq!(payload["input"]["message"], "hello"); + + let builtin_output = GlobalToolRegistry::builtin() + .execute("structured-output", &json!({ "ok": true })) + .expect("normalized builtin tool name should execute"); + let builtin_payload: serde_json::Value = + serde_json::from_str(&builtin_output).expect("valid json"); + assert_eq!(builtin_payload["structured_output"]["ok"], true); + + let _ = std::fs::remove_file(script); + } + #[test] fn global_registry_rejects_conflicting_plugin_tool_names() { let error = GlobalToolRegistry::with_plugin_tools(vec![PluginTool::new(