From 643dea487bafb8eaf836e718f22a7edca47acd09 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Mon, 21 Sep 2026 23:18:21 -0700 Subject: [PATCH] fix(plugins): canonical activation keys + remove/toggle bookkeeping across CLI, dashboard and RPC Symptoms fixed (all live-reproduced on origin/main in a fake HERMES_HOME): - `hermes plugins disable photon-platform` wrote `platforms/photon` while the loader keyed the bundled adapter `photon-platform`, so the disable never applied (#27548). The loader now keys bundled platforms `platforms/` like every other category (one call site in plugins_discovery.py); the manifest name stays an accepted alias through gate_manifest. - `plugins.manage toggle` / dashboard toggle wrote the raw identifier: enabling by bare leaf or manifest name returned ok while a stale canonical key in plugins.disabled kept the plugin off. dashboard_set_agent_plugin_enabled resolves the canonical key and purges every alias from the opposing list (shared _activate_key, also used by cmd_enable/cmd_disable); the RPC reports the key. - remove/uninstall (CLI, dashboard, RPC) left the plugin in plugins.enabled/disabled and plugins.entries (#54336), left memory.provider dangling so the next agent init re-cloned the provider from the catalog (uninstall silently reverted), and, for a symlink inside the plugins dir, deleted the TARGET plugin and its metadata while the link stayed dangling. _remove_user_plugin is the shared tail: unlink a link only, then forget config under every alias and reset memory.provider (reported as cleared_memory_provider). - `hermes plugins list` / dashboard hub / TUI hub called bundled backends, bundled platforms and the live memory.provider "not enabled" (#73131, #82898): _plugin_status mirrors gate_manifest. - A user-installed memory provider parked in plugins.disabled kept loading: load_memory_provider honours the deny-list (name, dir name or manifest name) and says so once. --- hermes_cli/plugins_cmd.py | 162 ++++++++++++++++++++++------- hermes_cli/plugins_discovery.py | 8 +- hermes_cli/web_server_dashboard.py | 14 ++- plugins/memory/__init__.py | 28 +++++ tui_gateway/methods_tools.py | 15 +-- 5 files changed, 173 insertions(+), 54 deletions(-) diff --git a/hermes_cli/plugins_cmd.py b/hermes_cli/plugins_cmd.py index c21871f167..9454f3d980 100644 --- a/hermes_cli/plugins_cmd.py +++ b/hermes_cli/plugins_cmd.py @@ -13,7 +13,7 @@ import sys import tempfile import urllib.parse from pathlib import Path -from typing import Any, Optional +from typing import Any, NoReturn, Optional from hermes_constants import get_hermes_home from hermes_cli._subprocess_compat import noninteractive_git_env @@ -77,7 +77,7 @@ def _is_tty() -> bool: return sys.stdin.isatty() and sys.stdout.isatty() -def _fail(console, message: str) -> None: +def _fail(console, message: str) -> NoReturn: """Print *message* and exit 1.""" console.print(message) sys.exit(1) @@ -1041,14 +1041,38 @@ def cmd_remove(name: str) -> None: plugins_dir = _plugins_dir() target = _require_installed_plugin(name, plugins_dir, console) try: - _remove_plugin_core(target) + result = _remove_user_plugin(plugins_dir, name, target) except (OSError, PluginOperationError) as exc: _fail(console, f"[red]Error:[/red] Could not remove plugin '{name}': {exc}") console.print() console.print(f"[red]✗[/red] Plugin [bold]{name}[/bold] removed from {plugins_dir}") + if result.get("cleared_memory_provider"): + console.print("[yellow]memory.provider pointed at this plugin and was reset; " + "run `hermes memory setup` to pick another.[/yellow]") console.print() +def _remove_user_plugin(plugins_dir: Path, name: str, target: Path) -> dict[str, Any]: + """Shared ``remove`` tail for the CLI, the dashboard and the ``plugins.manage`` RPC. + + *target* is the resolved directory; when ``plugins_dir/name`` itself is a symlink only the link + goes — the tree it points at may be another installed plugin (a dev alias to a sibling checkout), + and following it deleted that plugin plus its install metadata while the alias stayed dangling. + Config bookkeeping (aliases, toolset) is gathered before the tree disappears. + """ + link = plugins_dir / name.strip("/") + if link.is_symlink(): + link.unlink() + return {"ok": True, "name": name, **_forget_plugin_config({link.name})} + entry = next((e for e in _discover_all_plugins() if Path(str(e[4])) == target), None) + key = entry[5] if entry else target.name + aliases = _plugin_aliases(key) | {target.name} + if _read_manifest(target).get("provides_tools"): + _toggle_plugin_toolset(key, enable=False) + _remove_plugin_core(target) + return {"ok": True, "name": name, **_forget_plugin_config(aliases)} + + # ``plugins.disabled`` is an explicit deny-list that wins over the ``plugins.enabled`` allow-list. _get_disabled_set = functools.partial(_config_name_set, "plugins", "disabled") _get_enabled_set = functools.partial(_config_name_set, "plugins", "enabled") @@ -1092,6 +1116,61 @@ def _discard_key_and_leaf(names: set, key: str) -> None: names.discard(key.split("/")[-1]) +def _plugin_aliases(key: str) -> set: + """Every spelling a config list may hold for *key*: the key, its bare leaf and the manifest name. + The loader matches BOTH the canonical key (``web/firecrawl``) and the manifest name + (``web-firecrawl``), so a stale entry under any form vetoes an enable ("explicit disable wins").""" + names = {key, key.split("/")[-1]} + names.update(e[0] for e in _discover_all_plugins() if e[5] == key) + return names + + +def _activate_key(key: str, *, enable: bool) -> bool: + """Persist canonical *key* as enabled/disabled, purging every alias from the opposing list. + False when the lists already say so (nothing written).""" + enabled, disabled = _get_enabled_set(), _get_disabled_set() + aliases = _plugin_aliases(key) + target, other = (enabled, disabled) if enable else (disabled, enabled) + if key in target and not (aliases & other): + return False + target.add(key) + other.difference_update(aliases) + _save_plugin_sets(enabled, disabled) + return True + + +def _forget_plugin_config(aliases: set) -> dict[str, Any]: + """Drop every trace of a removed plugin (its :func:`_plugin_aliases`, taken BEFORE the tree went) + from config.yaml: allow/deny-list entries, ``plugins.entries.`` grants and a ``memory.provider`` + selection naming it. A later reinstall under the same name must start from the "Enable now?" + decision, not inherit a stale enable or grant (#54336); a dangling ``memory.provider`` would make + the next agent init re-clone the plugin from the catalog, silently undoing the uninstall. + Returns ``{"cleared_memory_provider": True}`` when the selection was reset.""" + from hermes_cli.config import load_config, save_config + config = load_config() + changed = False + result: dict[str, Any] = {} + plugins_cfg = config.get("plugins") + if isinstance(plugins_cfg, dict): + for list_key in ("enabled", "disabled"): + names = plugins_cfg.get(list_key) + if isinstance(names, list) and aliases & set(names): + plugins_cfg[list_key] = sorted(set(names) - aliases) + changed = True + entries = plugins_cfg.get("entries") + if isinstance(entries, dict) and aliases & set(entries): + for alias in aliases & set(entries): + del entries[alias] + changed = True + memory_cfg = config.get("memory") + if isinstance(memory_cfg, dict) and str(memory_cfg.get("provider") or "").strip() in aliases: + memory_cfg["provider"] = "" + changed, result = True, {"cleared_memory_provider": True} + if changed: + save_config(config) + return result + + def _set_plugin_enabled(name: str, *, enable: bool) -> None: """Move *name* between the enabled allow-list and the disabled deny-list and persist both.""" enabled = _get_enabled_set() @@ -1165,21 +1244,10 @@ def cmd_enable(name: str, allow_tool_override: Optional[bool] = None) -> None: except PluginOperationError as exc: _fail(console, f"[red]Error:[/red] {exc}") - enabled = _get_enabled_set() - disabled = _get_disabled_set() - if key in enabled and key not in disabled: - console.print(f"[dim]Plugin '{key}' is already enabled.[/dim]") - else: - enabled.add(key) - # The loader's disable check matches BOTH the canonical key (``web/firecrawl``) and the - # manifest name (``web-firecrawl``); a stale entry under either form would silently veto - # this enable ("explicit disable wins"), so drop the key, its bare leaf, and the name. - _discard_key_and_leaf(disabled, key) - manifest_name = next((e[0] for e in _discover_all_plugins() if e[5] == key), None) - if manifest_name is not None: - disabled.discard(manifest_name) - _save_plugin_sets(enabled, disabled) + if _activate_key(key, enable=True): console.print(f"[green]✓[/green] Plugin [bold]{key}[/bold] enabled. Takes effect on next session.") + else: + console.print(f"[dim]Plugin '{key}' is already enabled.[/dim]") # Built-in tool override is a privileged grant; bundled plugins are trusted. if source == "bundled": @@ -1345,15 +1413,9 @@ def cmd_disable(name: str) -> None: key = _resolve_plugin_key(name) if key is None: _fail(console, _unknown_plugin_message(name)) - enabled = _get_enabled_set() - disabled = _get_disabled_set() - if key not in enabled and key in disabled: + if not _activate_key(key, enable=False): console.print(f"[dim]Plugin '{key}' is already disabled.[/dim]") return - # Also drop a stale legacy bare-name entry so it can't keep a nested plugin loading. - _discard_key_and_leaf(enabled, key) - disabled.add(key) - _save_plugin_sets(enabled, disabled) console.print( f"[yellow]\u2298[/yellow] Plugin [bold]{key}[/bold] disabled. Takes effect on next session.") @@ -1453,10 +1515,26 @@ def _discover_all_plugins() -> list: return list(seen.values()) -def _plugin_status(name: str, enabled: set, disabled: set, key: str = "") -> str: - """User-facing activation state for a plugin name or key.""" +def _category_active_names() -> set: + """Provider names switched on through ``.provider`` config rather than + ``plugins.enabled`` (the live memory provider), so status never calls them "not enabled".""" + return {n for n in (_get_current_memory_provider(),) if n} + + +def _plugin_status(name: str, enabled: set, disabled: set, key: str = "", *, source: str = "", + dir_path=None, active: "frozenset | set" = frozenset()) -> str: + """User-facing activation state for a plugin name or key. Mirrors ``gate_manifest``: an explicit + disable wins, then the allow-list, then the activations that need no list entry — bundled + backends/platforms/model providers (*source* + *dir_path*) and category-selected providers + (*active*, see :func:`_category_active_names`).""" names = {name, key} - return "disabled" if names & disabled else "enabled" if names & enabled else "not enabled" + if names & disabled: + return "disabled" + if names & enabled or names & active: + return "enabled" + if source == "bundled" and dir_path is not None and _bundled_default_on(dir_path): + return "enabled" + return "not enabled" def _filter_plugin_entries(entries: list, args: Any, enabled: set, disabled: set) -> list: @@ -1465,9 +1543,11 @@ def _filter_plugin_entries(entries: list, args: Any, enabled: set, disabled: set if getattr(args, "no_bundled", False) or getattr(args, "user", False): filtered = [entry for entry in filtered if entry[3] != "bundled"] if getattr(args, "enabled", False): + active = _category_active_names() filtered = [ entry for entry in filtered - if _plugin_status(entry[0], enabled, disabled, key=entry[5]) == "enabled" + if _plugin_status(entry[0], enabled, disabled, key=entry[5], source=entry[3], dir_path=entry[4], + active=active) == "enabled" ] return filtered @@ -1494,8 +1574,10 @@ def cmd_list(args: Any | None = None) -> None: # One kill-list resolution for the whole listing: resolving per row costs a live-catalog # fetch per installed plugin when the catalog host is slow or unreachable. removed_entries = catalog.resolved_removed_entries() + active = _category_active_names() rows = [ - (name, _plugin_status(name, enabled, disabled, key=key), str(version), description, + (name, _plugin_status(name, enabled, disabled, key=key, source=source, dir_path=_dir, active=active), + str(version), description, catalog.catalog_annotation(_dir) or _pin_annotation(name, pins) or source, catalog.removed_annotation(name, _dir, removed_entries)) for name, version, description, source, _dir, key in entries @@ -1987,16 +2069,17 @@ def _toggle_plugin_toolset(name: str, *, enable: bool) -> None: def dashboard_set_agent_plugin_enabled(name: str, *, enabled: bool) -> dict[str, Any]: - """Enable or disable a plugin in ``config.yaml`` (runtime allow/deny lists).""" - if _resolve_plugin_key(name) is None: + """Enable or disable a plugin in ``config.yaml`` (runtime allow/deny lists). *name* may be the + canonical key, the manifest name or a unique bare leaf; the canonical key is what gets written + (the loader never matches a bare leaf, and a stale key in ``disabled`` outranks a manifest-name + entry in ``enabled`` — so writing the raw identifier reported success while the plugin stayed off).""" + key = _resolve_plugin_key(name) + if key is None: return {"ok": False, "error": f"Plugin '{name}' is not installed or bundled."} - en = _get_enabled_set() - dis = _get_disabled_set() - if ((name in en and name not in dis) if enabled else (name not in en and name in dis)): - return {"ok": True, "name": name, "unchanged": True} - _set_plugin_enabled(name, enable=enabled) - _toggle_plugin_toolset(name, enable=enabled) - return {"ok": True, "name": name, "unchanged": False} + changed = _activate_key(key, enable=enabled) + if changed: + _toggle_plugin_toolset(key, enable=enabled) + return {"ok": True, "name": key, "unchanged": not changed} def _user_installed_plugin_dir(name: str) -> Optional[Path]: @@ -2180,10 +2263,9 @@ def dashboard_remove_user_plugin(name: str) -> dict[str, Any]: if target is None: return {"ok": False, "error": f"Plugin '{name}' was not found under {plugins_dir}."} try: - _remove_plugin_core(target) + return _remove_user_plugin(plugins_dir, name, target) except (OSError, PluginOperationError) as exc: return {"ok": False, "error": f"Could not remove plugin '{name}': {exc}"} - return {"ok": True, "name": name} def cmd_plugin_doctor(target: str = ".", *, ci: bool = False) -> None: diff --git a/hermes_cli/plugins_discovery.py b/hermes_cli/plugins_discovery.py index 03c594423c..e7013a9efa 100644 --- a/hermes_cli/plugins_discovery.py +++ b/hermes_cli/plugins_discovery.py @@ -173,12 +173,14 @@ def collect_directory_manifests() -> List[PluginManifest]: logger.debug(" %s: %d manifest(s)", label, len(found)) manifests.extend(found) - # Excluded bundled top-level categories have their own discovery; platforms scan separately. + # Excluded bundled top-level categories have their own discovery. ``platforms/`` is an ordinary category + # dir: the recursion keys its adapters ``platforms/`` like every other category (``web/firecrawl``), + # which is the key `hermes plugins enable/disable` and the dashboard write (#27548); the manifest name + # (``photon-platform``) stays an accepted alias through ``gate_manifest``. repo_plugins = _origin.get_bundled_plugins_dir() logger.debug("Scanning bundled plugins: %s", repo_plugins) _scan("bundled (top-level)", repo_plugins, "bundled", - {"memory", "context_engine", "platforms", "model-providers", "cron_providers"}) - _scan("bundled/platforms", repo_plugins / "platforms", "bundled") + {"memory", "context_engine", "model-providers", "cron_providers"}) user_dir = get_hermes_home() / "plugins" logger.debug("Scanning user plugins: %s", user_dir) _scan("user", user_dir, "user") diff --git a/hermes_cli/web_server_dashboard.py b/hermes_cli/web_server_dashboard.py index 8363a7d9db..9ec07ce343 100644 --- a/hermes_cli/web_server_dashboard.py +++ b/hermes_cli/web_server_dashboard.py @@ -677,12 +677,14 @@ def _merged_plugins_hub(force_refresh: bool = False) -> Dict[str, Any]: started_at = time.monotonic() from hermes_cli.plugins_cmd import ( + _category_active_names, _discover_all_plugins, _get_current_context_engine, _get_current_memory_provider, _discover_context_engines, _get_disabled_set, _get_enabled_set, + _plugin_status, _read_manifest as _read_plugin_manifest_at, ) from hermes_cli.plugins_cmd_catalog import removed_annotation @@ -699,12 +701,16 @@ def _merged_plugins_hub(force_refresh: bool = False) -> Dict[str, Any]: # One kill-list resolution for the whole rebuild: resolving per row costs a live-catalog # fetch per installed plugin when the catalog host is slow or unreachable. removed_entries = resolved_removed_entries() + active = _category_active_names() for name, version, description, source, dir_str, key in _discover_all_plugins(): - # Both the path-derived key (nested category plugins) and the bare manifest name - # count for enabled/disabled state, matching the runtime loader's back-compat lookup. - aliases = {name, key} if key else {name} - runtime_status = _plugin_runtime_status(aliases, enabled_set, disabled_set) + # Same verdict as `hermes plugins list` / the TUI hub: name+key aliases for the lists, bundled + # backends/platforms/providers and the live memory provider count as enabled without a list + # entry (#73131, #82898). + runtime_status = _plugin_status( + name, enabled_set, disabled_set, key=key, source=source, dir_path=dir_str, active=active) + if runtime_status == "not enabled": + runtime_status = "inactive" dir_path = Path(dir_str) dm = dash_by_name.get(name) diff --git a/plugins/memory/__init__.py b/plugins/memory/__init__.py index c1a06c8f8d..22ef4b6404 100644 --- a/plugins/memory/__init__.py +++ b/plugins/memory/__init__.py @@ -202,6 +202,12 @@ def load_memory_provider(name: str, *, register_skills: Optional[bool] = None) - if not provider_dir and entry_point is None: logger.debug("Memory provider '%s' not found in bundled, user plugins, or entry points", name) return None + if provider_dir is not None and _explicitly_disabled(name, provider_dir): + # The Plugins hub / `hermes plugins disable` park a user-installed provider in + # ``plugins.disabled``; the loader must honour it or "disabled" is a lie in the UI. + logger.warning("Memory provider '%s' is disabled via plugins.disabled; run `hermes plugins enable %s` " + "or change memory.provider.", name, name) + return None def _load(_dir): if provider_dir: @@ -421,6 +427,28 @@ def _get_active_memory_provider() -> Optional[str]: return None +def _explicitly_disabled(name: str, provider_dir: Path) -> bool: + """True when a NON-bundled provider is parked in ``plugins.disabled`` under any spelling the + plugin commands write: the provider name, its directory name or its manifest ``name:``.""" + if _MEMORY_PLUGINS_DIR in provider_dir.parents: + return False + try: + from hermes_cli.config import load_config + disabled = cfg_get(load_config(), "plugins", "disabled") + except Exception: + return False + if not isinstance(disabled, list): + return False + names = {name, provider_dir.name} + try: + import yaml + with open(provider_dir / "plugin.yaml", encoding="utf-8-sig") as f: + names.add(str((yaml.safe_load(f) or {}).get("name") or "")) + except Exception: + pass + return bool(names & {v for v in disabled if isinstance(v, str)}) + + def _prune_inactive_memory_provider_skills(active_provider: Optional[str] = None) -> None: """Remove tracked skills that no longer belong to the active provider.""" if active_provider is None: diff --git a/tui_gateway/methods_tools.py b/tui_gateway/methods_tools.py index 6926240797..abb6dbb3bb 100644 --- a/tui_gateway/methods_tools.py +++ b/tui_gateway/methods_tools.py @@ -1446,12 +1446,11 @@ def _plugin_rows() -> list[dict]: versions = cat.catalog_versions() ref_pins = pc._read_install_metadata() # ``--ref`` installs: pinned_sha so the desktop can show the pin out = [] + active = pc._category_active_names() for name, version, desc, source, _dir, key in sorted(pc._discover_all_plugins()): - status = pc._plugin_status(name, enabled, disabled, key=key) - # Bundled backends/platforms/providers run without an explicit enable: report the - # truthful default instead of "not enabled" (reads as OFF). - if status == "not enabled" and source == "bundled" and pc._bundled_default_on(_dir): - status = "enabled" + # Bundled backends/platforms/providers and the live memory provider run without an explicit + # enable: _plugin_status reports the truthful default instead of "not enabled" (reads as OFF). + status = pc._plugin_status(name, enabled, disabled, key=key, source=source, dir_path=_dir, active=active) # key = canonical registry key (names collide across category dirs); portable = Agent Plugins v1. # ``has_desktop_half``: the package also ships a Desktop UI half (``desktop/plugin.js``). The # desktop app pairs its app-level copy of that half with this row so one package is ONE row. @@ -1481,8 +1480,10 @@ def _plugins_toggle(rid, params): result = toggle(ident, enabled=bool(params.get("enable"))) if not result.get("ok"): return _err(rid, 5026, result.get("error") or "toggle failed") - row = next((r for r in _plugin_rows() if ident in (r["key"], r["name"])), None) - return _ok(rid, {"ok": True, "unchanged": bool(result.get("unchanged")), "name": ident, "plugin": row}) + # The toggle resolves a bare leaf / manifest name to the canonical key it wrote; report that key. + key = result.get("name") or ident + row = next((r for r in _plugin_rows() if key in (r["key"], r["name"])), None) + return _ok(rid, {"ok": True, "unchanged": bool(result.get("unchanged")), "name": key, "plugin": row}) def _plugins_install(rid, params):