diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 5dc32d008..d04e8640f 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -827,7 +827,7 @@ DEFAULT_CONFIG = { }, # Config schema version - bump this when adding new required fields - "_config_version": 20, + "_config_version": 21, } # ============================================================================= @@ -2484,6 +2484,72 @@ def migrate_config(interactive: bool = True, quiet: bool = False) -> Dict[str, A else: print(" ✓ Removed unused compression.summary_* keys") + # ── Version 20 → 21: plugins are now opt-in; grandfather existing user plugins ── + # The loader now requires plugins to appear in ``plugins.enabled`` before + # loading. Existing installs had all discovered plugins loading by default + # (minus anything in ``plugins.disabled``). To avoid silently breaking + # those setups on upgrade, populate ``plugins.enabled`` with the set of + # currently-installed user plugins that aren't already disabled. + # + # Bundled plugins (shipped in the repo itself) are NOT grandfathered — + # they ship off for everyone, including existing users, so any user who + # wants one has to opt in explicitly. + if current_ver < 21: + config = read_raw_config() + plugins_cfg = config.get("plugins") + if not isinstance(plugins_cfg, dict): + plugins_cfg = {} + # Only migrate if the enabled allow-list hasn't been set yet. + if "enabled" not in plugins_cfg: + disabled = plugins_cfg.get("disabled", []) or [] + if not isinstance(disabled, list): + disabled = [] + disabled_set = set(disabled) + + # Scan ``$HERMES_HOME/plugins/`` for currently installed user plugins. + grandfathered: List[str] = [] + try: + from hermes_constants import get_hermes_home as _ghome + user_plugins_dir = _ghome() / "plugins" + if user_plugins_dir.is_dir(): + for child in sorted(user_plugins_dir.iterdir()): + if not child.is_dir(): + continue + manifest_file = child / "plugin.yaml" + if not manifest_file.exists(): + manifest_file = child / "plugin.yml" + if not manifest_file.exists(): + continue + try: + with open(manifest_file) as _mf: + manifest = yaml.safe_load(_mf) or {} + except Exception: + manifest = {} + name = manifest.get("name") or child.name + if name in disabled_set: + continue + grandfathered.append(name) + except Exception: + grandfathered = [] + + plugins_cfg["enabled"] = grandfathered + config["plugins"] = plugins_cfg + save_config(config) + results["config_added"].append( + f"plugins.enabled (opt-in allow-list, {len(grandfathered)} grandfathered)" + ) + if not quiet: + if grandfathered: + print( + f" ✓ Plugins now opt-in: grandfathered " + f"{len(grandfathered)} existing plugin(s) into plugins.enabled" + ) + else: + print( + " ✓ Plugins now opt-in: no existing plugins to grandfather. " + "Use `hermes plugins enable ` to activate." + ) + if current_ver < latest_ver and not quiet: print(f"Config version: {current_ver} → {latest_ver}") diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 6151616da..61b1d38a6 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -7449,6 +7449,17 @@ Examples: action="store_true", help="Remove existing plugin and reinstall", ) + _install_enable_group = plugins_install.add_mutually_exclusive_group() + _install_enable_group.add_argument( + "--enable", + action="store_true", + help="Auto-enable the plugin after install (skip confirmation prompt)", + ) + _install_enable_group.add_argument( + "--no-enable", + action="store_true", + help="Install disabled (skip confirmation prompt); enable later with `hermes plugins enable `", + ) plugins_update = plugins_subparsers.add_parser( "update", help="Pull latest changes for an installed plugin" diff --git a/hermes_cli/plugins.py b/hermes_cli/plugins.py index 621dedde1..23b10c376 100644 --- a/hermes_cli/plugins.py +++ b/hermes_cli/plugins.py @@ -83,7 +83,12 @@ def _env_enabled(name: str) -> bool: def _get_disabled_plugins() -> set: - """Read the disabled plugins list from config.yaml.""" + """Read the disabled plugins list from config.yaml. + + Kept for backward compat and explicit deny-list semantics. A plugin + name in this set will never load, even if it appears in + ``plugins.enabled``. + """ try: from hermes_cli.config import load_config config = load_config() @@ -93,6 +98,36 @@ def _get_disabled_plugins() -> set: return set() +def _get_enabled_plugins() -> Optional[set]: + """Read the enabled-plugins allow-list from config.yaml. + + Plugins are opt-in by default — only plugins whose name appears in + this set are loaded. Returns: + + * ``None`` — the key is missing or malformed. Callers should treat + this as "nothing enabled yet" (the opt-in default); the first + ``migrate_config`` run populates the key with a grandfathered set + of currently-installed user plugins so existing setups don't + break on upgrade. + * ``set()`` — an empty list was explicitly set; nothing loads. + * ``set(...)`` — the concrete allow-list. + """ + try: + from hermes_cli.config import load_config + config = load_config() + plugins_cfg = config.get("plugins") + if not isinstance(plugins_cfg, dict): + return None + if "enabled" not in plugins_cfg: + return None + enabled = plugins_cfg.get("enabled") + if not isinstance(enabled, list): + return None + return set(enabled) + except Exception: + return None + + # --------------------------------------------------------------------------- # Data classes # --------------------------------------------------------------------------- @@ -431,17 +466,17 @@ class PluginManager: # 1. Bundled plugins (/plugins//) # Repo-shipped generic plugins live next to hermes_cli/. Memory and # context_engine subdirs are handled by their own discovery paths, so - # skip those names here. - # Tests can set HERMES_DISABLE_BUNDLED_PLUGINS=1 to get a clean slate. - if not _env_enabled("HERMES_DISABLE_BUNDLED_PLUGINS"): - repo_plugins = Path(__file__).resolve().parent.parent / "plugins" - manifests.extend( - self._scan_directory( - repo_plugins, - source="bundled", - skip_names={"memory", "context_engine"}, - ) + # skip those names here. Bundled plugins are discovered (so they + # show up in `hermes plugins`) but only loaded when added to + # `plugins.enabled` in config.yaml — opt-in like any other plugin. + repo_plugins = Path(__file__).resolve().parent.parent / "plugins" + manifests.extend( + self._scan_directory( + repo_plugins, + source="bundled", + skip_names={"memory", "context_engine"}, ) + ) # 2. User plugins (~/.hermes/plugins/) user_dir = get_hermes_home() / "plugins" @@ -460,16 +495,34 @@ class PluginManager: # take precedence over bundled, project plugins take precedence over # user. Dedup here so we only load the final winner. disabled = _get_disabled_plugins() + enabled = _get_enabled_plugins() # None = opt-in default (nothing enabled) winners: Dict[str, PluginManifest] = {} for manifest in manifests: winners[manifest.name] = manifest for manifest in winners.values(): + # Explicit disable always wins. if manifest.name in disabled: loaded = LoadedPlugin(manifest=manifest, enabled=False) loaded.error = "disabled via config" self._plugins[manifest.name] = loaded logger.debug("Skipping disabled plugin '%s'", manifest.name) continue + # Opt-in gate: plugins must be in the enabled allow-list. + # If the allow-list is missing (None), treat as "nothing enabled" + # — users have to explicitly enable plugins to load them. + # Memory and context_engine providers are excluded from this gate + # since they have their own single-select config (memory.provider + # / context.engine), not the enabled list. + if enabled is None or manifest.name not in enabled: + loaded = LoadedPlugin(manifest=manifest, enabled=False) + loaded.error = "not enabled in config (run `hermes plugins enable {}` to activate)".format( + manifest.name + ) + self._plugins[manifest.name] = loaded + logger.debug( + "Skipping '%s' (not in plugins.enabled)", manifest.name + ) + continue self._load_plugin(manifest) if manifests: diff --git a/hermes_cli/plugins_cmd.py b/hermes_cli/plugins_cmd.py index c92d8b0dc..230e13420 100644 --- a/hermes_cli/plugins_cmd.py +++ b/hermes_cli/plugins_cmd.py @@ -15,6 +15,7 @@ import shutil import subprocess import sys from pathlib import Path +from typing import Optional from hermes_constants import get_hermes_home @@ -281,8 +282,16 @@ def _require_installed_plugin(name: str, plugins_dir: Path, console) -> Path: # --------------------------------------------------------------------------- -def cmd_install(identifier: str, force: bool = False) -> None: - """Install a plugin from a Git URL or owner/repo shorthand.""" +def cmd_install( + identifier: str, + force: bool = False, + enable: Optional[bool] = None, +) -> None: + """Install a plugin from a Git URL or owner/repo shorthand. + + After install, prompt "Enable now? [y/N]" unless *enable* is provided + (True = auto-enable without prompting, False = install disabled). + """ import tempfile from rich.console import Console @@ -391,6 +400,40 @@ def cmd_install(identifier: str, force: bool = False) -> None: _display_after_install(target, identifier) + # Determine the canonical plugin name for enable-list bookkeeping. + installed_name = installed_manifest.get("name") or target.name + + # Decide whether to enable: explicit flag > interactive prompt > default off + should_enable = enable + if should_enable is None: + # Interactive prompt unless stdin isn't a TTY (scripted install). + if sys.stdin.isatty() and sys.stdout.isatty(): + try: + answer = input( + f" Enable '{installed_name}' now? [y/N]: " + ).strip().lower() + should_enable = answer in ("y", "yes") + except (EOFError, KeyboardInterrupt): + should_enable = False + else: + should_enable = False + + if should_enable: + enabled = _get_enabled_set() + disabled = _get_disabled_set() + enabled.add(installed_name) + disabled.discard(installed_name) + _save_enabled_set(enabled) + _save_disabled_set(disabled) + console.print( + f"[green]✓[/green] Plugin [bold]{installed_name}[/bold] enabled." + ) + else: + console.print( + f"[dim]Plugin installed but not enabled. " + f"Run `hermes plugins enable {installed_name}` to activate.[/dim]" + ) + console.print("[dim]Restart the gateway for the plugin to take effect:[/dim]") console.print("[dim] hermes gateway restart[/dim]") console.print() @@ -468,7 +511,11 @@ def cmd_remove(name: str) -> None: def _get_disabled_set() -> set: - """Read the disabled plugins set from config.yaml.""" + """Read the disabled plugins set from config.yaml. + + An explicit deny-list. A plugin name here never loads, even if also + listed in ``plugins.enabled``. + """ try: from hermes_cli.config import load_config config = load_config() @@ -488,103 +535,196 @@ def _save_disabled_set(disabled: set) -> None: save_config(config) +def _get_enabled_set() -> set: + """Read the enabled plugins allow-list from config.yaml. + + Plugins are opt-in: only names here are loaded. Returns ``set()`` if + the key is missing (same behaviour as "nothing enabled yet"). + """ + try: + from hermes_cli.config import load_config + config = load_config() + plugins_cfg = config.get("plugins", {}) + if not isinstance(plugins_cfg, dict): + return set() + enabled = plugins_cfg.get("enabled", []) + return set(enabled) if isinstance(enabled, list) else set() + except Exception: + return set() + + +def _save_enabled_set(enabled: set) -> None: + """Write the enabled plugins list to config.yaml.""" + from hermes_cli.config import load_config, save_config + config = load_config() + if "plugins" not in config: + config["plugins"] = {} + config["plugins"]["enabled"] = sorted(enabled) + save_config(config) + + def cmd_enable(name: str) -> None: - """Enable a previously disabled plugin.""" + """Add a plugin to the enabled allow-list (and remove it from disabled).""" from rich.console import Console console = Console() - plugins_dir = _plugins_dir() - - # Verify the plugin exists - target = plugins_dir / name - if not target.is_dir(): - console.print(f"[red]Plugin '{name}' is not installed.[/red]") + # Discover the plugin — check installed (user) AND bundled. + if not _plugin_exists(name): + console.print(f"[red]Plugin '{name}' is not installed or bundled.[/red]") sys.exit(1) + enabled = _get_enabled_set() disabled = _get_disabled_set() - if name not in disabled: + + if name in enabled and name not in disabled: console.print(f"[dim]Plugin '{name}' is already enabled.[/dim]") return + enabled.add(name) disabled.discard(name) + _save_enabled_set(enabled) _save_disabled_set(disabled) - console.print(f"[green]✓[/green] Plugin [bold]{name}[/bold] enabled. Takes effect on next session.") + console.print( + f"[green]✓[/green] Plugin [bold]{name}[/bold] enabled. " + "Takes effect on next session." + ) def cmd_disable(name: str) -> None: - """Disable a plugin without removing it.""" + """Remove a plugin from the enabled allow-list (and add to disabled).""" from rich.console import Console console = Console() - plugins_dir = _plugins_dir() - - # Verify the plugin exists - target = plugins_dir / name - if not target.is_dir(): - console.print(f"[red]Plugin '{name}' is not installed.[/red]") + if not _plugin_exists(name): + console.print(f"[red]Plugin '{name}' is not installed or bundled.[/red]") sys.exit(1) + enabled = _get_enabled_set() disabled = _get_disabled_set() - if name in disabled: + + if name not in enabled and name in disabled: console.print(f"[dim]Plugin '{name}' is already disabled.[/dim]") return + enabled.discard(name) disabled.add(name) + _save_enabled_set(enabled) _save_disabled_set(disabled) - console.print(f"[yellow]\u2298[/yellow] Plugin [bold]{name}[/bold] disabled. Takes effect on next session.") + console.print( + f"[yellow]\u2298[/yellow] Plugin [bold]{name}[/bold] disabled. " + "Takes effect on next session." + ) -def cmd_list() -> None: - """List installed plugins.""" - from rich.console import Console - from rich.table import Table +def _plugin_exists(name: str) -> bool: + """Return True if a plugin with *name* is installed (user) or bundled.""" + # Installed: directory name or manifest name match in user plugins dir + user_dir = _plugins_dir() + if user_dir.is_dir(): + if (user_dir / name).is_dir(): + return True + for child in user_dir.iterdir(): + if not child.is_dir(): + continue + manifest = _read_manifest(child) + if manifest.get("name") == name: + return True + # Bundled: /plugins// + from pathlib import Path as _P + import hermes_cli + repo_plugins = _P(hermes_cli.__file__).resolve().parent.parent / "plugins" + if repo_plugins.is_dir(): + candidate = repo_plugins / name + if candidate.is_dir() and ( + (candidate / "plugin.yaml").exists() + or (candidate / "plugin.yml").exists() + ): + return True + return False + +def _discover_all_plugins() -> list: + """Return a list of (name, version, description, source, dir_path) for + every plugin the loader can see — user + bundled + project. + + Matches the ordering/dedup of ``PluginManager.discover_and_load``: + bundled first, then user, then project; user overrides bundled on + name collision. + """ try: import yaml except ImportError: yaml = None - console = Console() - plugins_dir = _plugins_dir() + seen: dict = {} # name -> (name, version, description, source, path) - dirs = sorted(d for d in plugins_dir.iterdir() if d.is_dir()) - if not dirs: + # Bundled (/plugins//), excluding memory/ and context_engine/ + import hermes_cli + repo_plugins = Path(hermes_cli.__file__).resolve().parent.parent / "plugins" + for base, source in ((repo_plugins, "bundled"), (_plugins_dir(), "user")): + if not base.is_dir(): + continue + for d in sorted(base.iterdir()): + if not d.is_dir(): + continue + if source == "bundled" and d.name in ("memory", "context_engine"): + continue + manifest_file = d / "plugin.yaml" + if not manifest_file.exists(): + manifest_file = d / "plugin.yml" + if not manifest_file.exists(): + continue + name = d.name + version = "" + description = "" + if yaml: + try: + with open(manifest_file) as f: + manifest = yaml.safe_load(f) or {} + name = manifest.get("name", d.name) + version = manifest.get("version", "") + description = manifest.get("description", "") + except Exception: + pass + # User plugins override bundled on name collision. + if name in seen and source == "bundled": + continue + src_label = source + if source == "user" and (d / ".git").exists(): + src_label = "git" + seen[name] = (name, version, description, src_label, d) + return list(seen.values()) + + +def cmd_list() -> None: + """List all plugins (bundled + user) with enabled/disabled state.""" + from rich.console import Console + from rich.table import Table + + console = Console() + entries = _discover_all_plugins() + if not entries: console.print("[dim]No plugins installed.[/dim]") console.print("[dim]Install with:[/dim] hermes plugins install owner/repo") return + enabled = _get_enabled_set() disabled = _get_disabled_set() - table = Table(title="Installed Plugins", show_lines=False) + table = Table(title="Plugins", show_lines=False) table.add_column("Name", style="bold") table.add_column("Status") table.add_column("Version", style="dim") table.add_column("Description") table.add_column("Source", style="dim") - for d in dirs: - manifest_file = d / "plugin.yaml" - name = d.name - version = "" - description = "" - source = "local" - - if manifest_file.exists() and yaml: - try: - with open(manifest_file) as f: - manifest = yaml.safe_load(f) or {} - name = manifest.get("name", d.name) - version = manifest.get("version", "") - description = manifest.get("description", "") - except Exception: - pass - - # Check if it's a git repo (installed via hermes plugins install) - if (d / ".git").exists(): - source = "git" - - is_disabled = name in disabled or d.name in disabled - status = "[red]disabled[/red]" if is_disabled else "[green]enabled[/green]" + for name, version, description, source, _dir in entries: + if name in disabled: + status = "[red]disabled[/red]" + elif name in enabled: + status = "[green]enabled[/green]" + else: + status = "[yellow]not enabled[/yellow]" table.add_row(name, status, str(version), description, source) console.print() @@ -592,6 +732,7 @@ def cmd_list() -> None: console.print() console.print("[dim]Interactive toggle:[/dim] hermes plugins") console.print("[dim]Enable/disable:[/dim] hermes plugins enable/disable ") + console.print("[dim]Plugins are opt-in by default — only 'enabled' plugins load.[/dim]") # --------------------------------------------------------------------------- @@ -742,41 +883,25 @@ def cmd_toggle() -> None: """Interactive composite UI — general plugins + provider plugin categories.""" from rich.console import Console - try: - import yaml - except ImportError: - yaml = None - console = Console() - plugins_dir = _plugins_dir() - # -- General plugins discovery -- - dirs = sorted(d for d in plugins_dir.iterdir() if d.is_dir()) - disabled = _get_disabled_set() + # -- General plugins discovery (bundled + user) -- + entries = _discover_all_plugins() + enabled_set = _get_enabled_set() + disabled_set = _get_disabled_set() plugin_names = [] plugin_labels = [] plugin_selected = set() - for i, d in enumerate(dirs): - manifest_file = d / "plugin.yaml" - name = d.name - description = "" - - if manifest_file.exists() and yaml: - try: - with open(manifest_file) as f: - manifest = yaml.safe_load(f) or {} - name = manifest.get("name", d.name) - description = manifest.get("description", "") - except Exception: - pass - - plugin_names.append(name) + for i, (name, _version, description, source, _d) in enumerate(entries): label = f"{name} \u2014 {description}" if description else name + if source == "bundled": + label = f"{label} [bundled]" + plugin_names.append(name) plugin_labels.append(label) - - if name not in disabled and d.name not in disabled: + # Selected (enabled) when in enabled-set AND not in disabled-set + if name in enabled_set and name not in disabled_set: plugin_selected.add(i) # -- Provider categories -- @@ -804,10 +929,10 @@ def cmd_toggle() -> None: try: import curses _run_composite_ui(curses, plugin_names, plugin_labels, plugin_selected, - disabled, categories, console) + disabled_set, categories, console) except ImportError: _run_composite_fallback(plugin_names, plugin_labels, plugin_selected, - disabled, categories, console) + disabled_set, categories, console) def _run_composite_ui(curses, plugin_names, plugin_labels, plugin_selected, @@ -1020,18 +1145,29 @@ def _run_composite_ui(curses, plugin_names, plugin_labels, plugin_selected, curses.wrapper(_draw) flush_stdin() - # Persist general plugin changes - new_disabled = set() + # Persist general plugin changes. The new allow-list is the set of + # plugin names that were checked; anything not checked is explicitly + # disabled (written to disabled-list) so it remains off even if the + # plugin code does something clever like auto-enable in the future. + new_enabled: set = set() + new_disabled: set = set(disabled) # preserve existing disabled state for unseen plugins for i, name in enumerate(plugin_names): - if i not in chosen: + if i in chosen: + new_enabled.add(name) + new_disabled.discard(name) + else: new_disabled.add(name) - if new_disabled != disabled: + prev_enabled = _get_enabled_set() + enabled_changed = new_enabled != prev_enabled + disabled_changed = new_disabled != disabled + + if enabled_changed or disabled_changed: + _save_enabled_set(new_enabled) _save_disabled_set(new_disabled) - enabled_count = len(plugin_names) - len(new_disabled) console.print( - f"\n[green]\u2713[/green] General plugins: {enabled_count} enabled, " - f"{len(new_disabled)} disabled." + f"\n[green]\u2713[/green] General plugins: {len(new_enabled)} enabled, " + f"{len(plugin_names) - len(new_enabled)} disabled." ) elif n_plugins > 0: console.print("\n[dim]General plugins unchanged.[/dim]") @@ -1078,11 +1214,17 @@ def _run_composite_fallback(plugin_names, plugin_labels, plugin_selected, return print() - new_disabled = set() + new_enabled: set = set() + new_disabled: set = set(disabled) for i, name in enumerate(plugin_names): - if i not in chosen: + if i in chosen: + new_enabled.add(name) + new_disabled.discard(name) + else: new_disabled.add(name) - if new_disabled != disabled: + prev_enabled = _get_enabled_set() + if new_enabled != prev_enabled or new_disabled != disabled: + _save_enabled_set(new_enabled) _save_disabled_set(new_disabled) # Provider categories @@ -1108,7 +1250,17 @@ def plugins_command(args) -> None: action = getattr(args, "plugins_action", None) if action == "install": - cmd_install(args.identifier, force=getattr(args, "force", False)) + # Map argparse tri-state: --enable=True, --no-enable=False, neither=None (prompt) + enable_arg = None + if getattr(args, "enable", False): + enable_arg = True + elif getattr(args, "no_enable", False): + enable_arg = False + cmd_install( + args.identifier, + force=getattr(args, "force", False), + enable=enable_arg, + ) elif action == "update": cmd_update(args.name) elif action in ("remove", "rm", "uninstall"): diff --git a/tests/conftest.py b/tests/conftest.py index 50fc3f213..ca4a9a970 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -243,11 +243,6 @@ def _hermetic_environment(tmp_path, monkeypatch): # 5. Reset plugin singleton so tests don't leak plugins from # ~/.hermes/plugins/ (which, per step 3, is now empty — but the # singleton might still be cached from a previous test). - # Also disable bundled-plugin discovery by default so the - # repo-shipped /plugins// dirs don't appear in tests - # that assume an empty plugin set. Tests that specifically exercise - # bundled discovery can clear this var explicitly. - monkeypatch.setenv("HERMES_DISABLE_BUNDLED_PLUGINS", "1") try: import hermes_cli.plugins as _plugins_mod monkeypatch.setattr(_plugins_mod, "_plugin_manager", None) diff --git a/tests/hermes_cli/test_config.py b/tests/hermes_cli/test_config.py index e87fe0a52..8c94902e6 100644 --- a/tests/hermes_cli/test_config.py +++ b/tests/hermes_cli/test_config.py @@ -459,7 +459,7 @@ class TestCustomProviderCompatibility: migrate_config(interactive=False, quiet=True) raw = yaml.safe_load(config_path.read_text(encoding="utf-8")) - assert raw["_config_version"] == 20 + assert raw["_config_version"] == 21 assert raw["providers"]["openai-direct"] == { "api": "https://api.openai.com/v1", "api_key": "test-key", @@ -606,7 +606,7 @@ class TestInterimAssistantMessageConfig: migrate_config(interactive=False, quiet=True) raw = yaml.safe_load(config_path.read_text(encoding="utf-8")) - assert raw["_config_version"] == 20 + assert raw["_config_version"] == 21 assert raw["display"]["tool_progress"] == "off" assert raw["display"]["interim_assistant_messages"] is True @@ -626,7 +626,7 @@ class TestDiscordChannelPromptsConfig: migrate_config(interactive=False, quiet=True) raw = yaml.safe_load(config_path.read_text(encoding="utf-8")) - assert raw["_config_version"] == 20 + assert raw["_config_version"] == 21 assert raw["discord"]["auto_thread"] is True assert raw["discord"]["channel_prompts"] == {} diff --git a/tests/hermes_cli/test_plugins.py b/tests/hermes_cli/test_plugins.py index 1286484d0..58c91384f 100644 --- a/tests/hermes_cli/test_plugins.py +++ b/tests/hermes_cli/test_plugins.py @@ -30,8 +30,19 @@ from hermes_cli.plugins import ( def _make_plugin_dir(base: Path, name: str, *, register_body: str = "pass", - manifest_extra: dict | None = None) -> Path: - """Create a minimal plugin directory with plugin.yaml + __init__.py.""" + manifest_extra: dict | None = None, + auto_enable: bool = True) -> Path: + """Create a minimal plugin directory with plugin.yaml + __init__.py. + + If *auto_enable* is True (default), also write the plugin's name into + ``/config.yaml`` under ``plugins.enabled``. Plugins are + opt-in by default, so tests that expect the plugin to actually load + need this. Pass ``auto_enable=False`` for tests that exercise the + unenabled path. + + *base* is expected to be ``/plugins/``; we derive + ```` from it by walking one level up. + """ plugin_dir = base / name plugin_dir.mkdir(parents=True, exist_ok=True) @@ -43,6 +54,31 @@ def _make_plugin_dir(base: Path, name: str, *, register_body: str = "pass", (plugin_dir / "__init__.py").write_text( f"def register(ctx):\n {register_body}\n" ) + + if auto_enable: + # Write/merge plugins.enabled in /config.yaml. + # Config is always read from HERMES_HOME (not from the project + # dir for project plugins), so that's where we opt in. + import os + hermes_home_str = os.environ.get("HERMES_HOME") + if hermes_home_str: + hermes_home = Path(hermes_home_str) + else: + hermes_home = base.parent + hermes_home.mkdir(parents=True, exist_ok=True) + cfg_path = hermes_home / "config.yaml" + cfg: dict = {} + if cfg_path.exists(): + try: + cfg = yaml.safe_load(cfg_path.read_text()) or {} + except Exception: + cfg = {} + plugins_cfg = cfg.setdefault("plugins", {}) + enabled = plugins_cfg.setdefault("enabled", []) + if isinstance(enabled, list) and name not in enabled: + enabled.append(name) + cfg_path.write_text(yaml.safe_dump(cfg)) + return plugin_dir @@ -102,7 +138,12 @@ class TestPluginDiscovery: mgr.discover_and_load() mgr.discover_and_load() # second call should no-op - assert len(mgr._plugins) == 1 + # Filter out bundled plugins — they're always discovered. + non_bundled = { + n: p for n, p in mgr._plugins.items() + if p.manifest.source != "bundled" + } + assert len(non_bundled) == 1 def test_discover_skips_dir_without_manifest(self, tmp_path, monkeypatch): """Directories without plugin.yaml are silently skipped.""" @@ -113,7 +154,12 @@ class TestPluginDiscovery: mgr = PluginManager() mgr.discover_and_load() - assert len(mgr._plugins) == 0 + # Filter out bundled plugins — they're always discovered. + non_bundled = { + n: p for n, p in mgr._plugins.items() + if p.manifest.source != "bundled" + } + assert len(non_bundled) == 0 def test_entry_points_scanned(self, tmp_path, monkeypatch): """Entry-point based plugins are discovered (mocked).""" @@ -152,7 +198,13 @@ class TestPluginLoading: plugin_dir = plugins_dir / "bad_plugin" plugin_dir.mkdir(parents=True) (plugin_dir / "plugin.yaml").write_text(yaml.dump({"name": "bad_plugin"})) - monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes_test")) + # Explicitly enable so the loader tries to import it and hits the + # missing-init error. + hermes_home = tmp_path / "hermes_test" + (hermes_home / "config.yaml").write_text( + yaml.safe_dump({"plugins": {"enabled": ["bad_plugin"]}}) + ) + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) mgr = PluginManager() mgr.discover_and_load() @@ -160,6 +212,8 @@ class TestPluginLoading: assert "bad_plugin" in mgr._plugins assert not mgr._plugins["bad_plugin"].enabled assert mgr._plugins["bad_plugin"].error is not None + # Should be the missing-init error, not "not enabled". + assert "not enabled" not in mgr._plugins["bad_plugin"].error def test_load_missing_register_fn(self, tmp_path, monkeypatch): """Plugin without register() function records an error.""" @@ -168,7 +222,12 @@ class TestPluginLoading: plugin_dir.mkdir(parents=True) (plugin_dir / "plugin.yaml").write_text(yaml.dump({"name": "no_reg"})) (plugin_dir / "__init__.py").write_text("# no register function\n") - monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes_test")) + # Explicitly enable it so the loader actually tries to import. + hermes_home = tmp_path / "hermes_test" + (hermes_home / "config.yaml").write_text( + yaml.safe_dump({"plugins": {"enabled": ["no_reg"]}}) + ) + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) mgr = PluginManager() mgr.discover_and_load() @@ -404,7 +463,11 @@ class TestPluginContext: ' handler=lambda args, **kw: "echo",\n' ' )\n' ) - monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes_test")) + hermes_home = tmp_path / "hermes_test" + (hermes_home / "config.yaml").write_text( + yaml.safe_dump({"plugins": {"enabled": ["tool_plugin"]}}) + ) + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) mgr = PluginManager() mgr.discover_and_load() @@ -438,7 +501,11 @@ class TestPluginToolVisibility: ' handler=lambda args, **kw: "ok",\n' ' )\n' ) - monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes_test")) + hermes_home = tmp_path / "hermes_test" + (hermes_home / "config.yaml").write_text( + yaml.safe_dump({"plugins": {"enabled": ["vis_plugin"]}}) + ) + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) mgr = PluginManager() mgr.discover_and_load() @@ -749,20 +816,24 @@ class TestPluginCommands: def test_commands_in_list_plugins_output(self, tmp_path, monkeypatch): """list_plugins() includes command count.""" plugins_dir = tmp_path / "hermes_test" / "plugins" + # Set HERMES_HOME BEFORE _make_plugin_dir so auto-enable targets + # the right config.yaml. + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes_test")) _make_plugin_dir( plugins_dir, "cmd-plugin", register_body=( 'ctx.register_command("mycmd", lambda a: "ok", description="Test")' ), ) - monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes_test")) mgr = PluginManager() mgr.discover_and_load() info = mgr.list_plugins() - assert len(info) == 1 - assert info[0]["commands"] == 1 + # Filter out bundled plugins — they're always discovered. + cmd_info = [p for p in info if p["name"] == "cmd-plugin"] + assert len(cmd_info) == 1 + assert cmd_info[0]["commands"] == 1 def test_handler_receives_raw_args(self): """The handler is called with the raw argument string.""" diff --git a/tests/plugins/test_disk_cleanup_plugin.py b/tests/plugins/test_disk_cleanup_plugin.py index 5b6473666..e1463bced 100644 --- a/tests/plugins/test_disk_cleanup_plugin.py +++ b/tests/plugins/test_disk_cleanup_plugin.py @@ -366,35 +366,62 @@ class TestSlashCommand: # --------------------------------------------------------------------------- class TestBundledDiscovery: - def test_disk_cleanup_is_discovered_as_bundled(self, _isolate_env, monkeypatch): - # The default hermetic conftest disables bundled plugin discovery. - # This test specifically exercises it, so clear the suppression. - monkeypatch.delenv("HERMES_DISABLE_BUNDLED_PLUGINS", raising=False) + def _write_enabled_config(self, hermes_home, names): + """Write plugins.enabled allow-list to config.yaml.""" + import yaml + cfg_path = hermes_home / "config.yaml" + cfg_path.write_text(yaml.safe_dump({"plugins": {"enabled": list(names)}})) + + def test_disk_cleanup_discovered_but_not_loaded_by_default(self, _isolate_env): + """Bundled plugins are discovered but NOT loaded without opt-in.""" from hermes_cli import plugins as pmod mgr = pmod.PluginManager() mgr.discover_and_load() + # Discovered — appears in the registry assert "disk-cleanup" in mgr._plugins loaded = mgr._plugins["disk-cleanup"] assert loaded.manifest.source == "bundled" + # But NOT enabled — no hooks or commands registered + assert not loaded.enabled + assert loaded.error and "not enabled" in loaded.error + + def test_disk_cleanup_loads_when_enabled(self, _isolate_env): + """Adding to plugins.enabled activates the bundled plugin.""" + self._write_enabled_config(_isolate_env, ["disk-cleanup"]) + from hermes_cli import plugins as pmod + mgr = pmod.PluginManager() + mgr.discover_and_load() + loaded = mgr._plugins["disk-cleanup"] assert loaded.enabled assert "post_tool_call" in loaded.hooks_registered assert "on_session_end" in loaded.hooks_registered assert "disk-cleanup" in loaded.commands_registered - def test_memory_and_context_engine_subdirs_skipped(self, _isolate_env, monkeypatch): + def test_disabled_beats_enabled(self, _isolate_env): + """plugins.disabled wins even if the plugin is also in plugins.enabled.""" + import yaml + cfg_path = _isolate_env / "config.yaml" + cfg_path.write_text(yaml.safe_dump({ + "plugins": { + "enabled": ["disk-cleanup"], + "disabled": ["disk-cleanup"], + } + })) + from hermes_cli import plugins as pmod + mgr = pmod.PluginManager() + mgr.discover_and_load() + loaded = mgr._plugins["disk-cleanup"] + assert not loaded.enabled + assert loaded.error == "disabled via config" + + def test_memory_and_context_engine_subdirs_skipped(self, _isolate_env): """Bundled scan must NOT pick up plugins/memory or plugins/context_engine as top-level plugins — they have their own discovery paths.""" - monkeypatch.delenv("HERMES_DISABLE_BUNDLED_PLUGINS", raising=False) + self._write_enabled_config( + _isolate_env, ["memory", "context_engine", "disk-cleanup"] + ) from hermes_cli import plugins as pmod mgr = pmod.PluginManager() mgr.discover_and_load() assert "memory" not in mgr._plugins assert "context_engine" not in mgr._plugins - - def test_bundled_scan_suppressed_by_env_var(self, _isolate_env, monkeypatch): - """HERMES_DISABLE_BUNDLED_PLUGINS=1 suppresses bundled discovery.""" - monkeypatch.setenv("HERMES_DISABLE_BUNDLED_PLUGINS", "1") - from hermes_cli import plugins as pmod - mgr = pmod.PluginManager() - mgr.discover_and_load() - assert "disk-cleanup" not in mgr._plugins diff --git a/website/docs/user-guide/features/built-in-plugins.md b/website/docs/user-guide/features/built-in-plugins.md index fc9ac2d4d..08cd4af3b 100644 --- a/website/docs/user-guide/features/built-in-plugins.md +++ b/website/docs/user-guide/features/built-in-plugins.md @@ -24,23 +24,31 @@ On name collision, later sources win — a user plugin named `disk-cleanup` woul `plugins/memory/` and `plugins/context_engine/` are deliberately excluded from bundled scanning. Those directories use their own discovery paths because memory providers and context engines are single-select providers configured through `hermes memory setup` / `context.engine` in config. -Bundled plugins respect the same disable mechanism as any other plugin: +## Bundled plugins are opt-in + +Bundled plugins ship disabled. Discovery finds them (they appear in `hermes plugins list` and the interactive `hermes plugins` UI), but none load until you explicitly enable them: + +```bash +hermes plugins enable disk-cleanup +``` + +Or via `~/.hermes/config.yaml`: ```yaml -# ~/.hermes/config.yaml plugins: - disabled: + enabled: - disk-cleanup ``` -Or suppress every bundled plugin at once with an env var: +This is the same mechanism user-installed plugins use. Bundled plugins are never auto-enabled — not on fresh install, not for existing users upgrading to a newer Hermes. You always opt in explicitly. + +To turn a bundled plugin off again: ```bash -HERMES_DISABLE_BUNDLED_PLUGINS=1 hermes chat +hermes plugins disable disk-cleanup +# or: remove it from plugins.enabled in config.yaml ``` -The test suite sets `HERMES_DISABLE_BUNDLED_PLUGINS=1` in its hermetic fixture — tests that exercise bundled discovery clear it explicitly. - ## Currently shipped ### disk-cleanup @@ -87,14 +95,9 @@ Auto-tracks and removes ephemeral files created during sessions — test scripts **Safety** — cleanup only ever touches paths under `HERMES_HOME` or `/tmp/hermes-*`. Windows mounts (`/mnt/c/...`) are rejected. Well-known top-level state dirs (`logs/`, `memories/`, `sessions/`, `cron/`, `cache/`, `skills/`, `plugins/`, `disk-cleanup/` itself) are never removed even when empty — a fresh install does not get gutted on first session end. -To turn it off without uninstalling: +**Enabling:** `hermes plugins enable disk-cleanup` (or check the box in `hermes plugins`). -```yaml -# ~/.hermes/config.yaml -plugins: - disabled: - - disk-cleanup -``` +**Disabling again:** `hermes plugins disable disk-cleanup`. ## Adding a bundled plugin diff --git a/website/docs/user-guide/features/plugins.md b/website/docs/user-guide/features/plugins.md index 0f8bbe627..19d00f906 100644 --- a/website/docs/user-guide/features/plugins.md +++ b/website/docs/user-guide/features/plugins.md @@ -100,7 +100,34 @@ Project-local plugins under `./.hermes/plugins/` are disabled by default. Enable | Project | `.hermes/plugins/` | Project-specific plugins (requires `HERMES_ENABLE_PROJECT_PLUGINS=true`) | | pip | `hermes_agent.plugins` entry_points | Distributed packages | -Later sources override earlier ones on name collision, so a user plugin with the same name as a bundled plugin replaces it. `HERMES_DISABLE_BUNDLED_PLUGINS=1` suppresses the bundled scan entirely. +Later sources override earlier ones on name collision, so a user plugin with the same name as a bundled plugin replaces it. + +## Plugins are opt-in + +**Every plugin — user-installed, bundled, or pip — is disabled by default.** Discovery finds them (so they show up in `hermes plugins` and `/plugins`), but nothing loads until you add the plugin's name to `plugins.enabled` in `~/.hermes/config.yaml`. This stops anything with hooks or tools from running without your explicit consent. + +```yaml +plugins: + enabled: + - my-tool-plugin + - disk-cleanup + disabled: # optional deny-list — always wins if a name appears in both + - noisy-plugin +``` + +Three ways to flip state: + +```bash +hermes plugins # interactive toggle (space to check/uncheck) +hermes plugins enable # add to allow-list +hermes plugins disable # remove from allow-list + add to disabled +``` + +After `hermes plugins install owner/repo`, you're asked `Enable 'name' now? [y/N]` — defaults to no. Skip the prompt for scripted installs with `--enable` or `--no-enable`. + +### Migration for existing users + +When you upgrade to a version of Hermes that has opt-in plugins (config schema v21+), any user plugins already installed under `~/.hermes/plugins/` that weren't already in `plugins.disabled` are **automatically grandfathered** into `plugins.enabled`. Your existing setup keeps working. Bundled plugins are NOT grandfathered — even existing users have to opt in explicitly. ## Available hooks @@ -130,13 +157,15 @@ Memory providers and context engines are **provider plugins** — only one of ea ## Managing plugins ```bash -hermes plugins # unified interactive UI -hermes plugins list # table view with enabled/disabled status -hermes plugins install user/repo # install from Git -hermes plugins update my-plugin # pull latest -hermes plugins remove my-plugin # uninstall -hermes plugins enable my-plugin # re-enable a disabled plugin -hermes plugins disable my-plugin # disable without removing +hermes plugins # unified interactive UI +hermes plugins list # table: enabled / disabled / not enabled +hermes plugins install user/repo # install from Git, then prompt Enable? [y/N] +hermes plugins install user/repo --enable # install AND enable (no prompt) +hermes plugins install user/repo --no-enable # install but leave disabled (no prompt) +hermes plugins update my-plugin # pull latest +hermes plugins remove my-plugin # uninstall +hermes plugins enable my-plugin # add to allow-list +hermes plugins disable my-plugin # remove from allow-list + add to disabled ``` ### Interactive UI @@ -150,14 +179,16 @@ Plugins General Plugins → [✓] my-tool-plugin — Custom search tool [ ] webhook-notifier — Event hooks + [ ] disk-cleanup — Auto-cleanup of ephemeral files [bundled] Provider Plugins Memory Provider ▸ honcho Context Engine ▸ compressor ``` -- **General Plugins section** — checkboxes, toggle with SPACE +- **General Plugins section** — checkboxes, toggle with SPACE. Checked = in `plugins.enabled`, unchecked = in `plugins.disabled` (explicit off). - **Provider Plugins section** — shows current selection. Press ENTER to drill into a radio picker where you choose one active provider. +- Bundled plugins appear in the same list with a `[bundled]` tag. Provider plugin selections are saved to `config.yaml`: @@ -169,15 +200,17 @@ context: engine: "compressor" # default built-in compressor ``` -### Disabling general plugins +### Enabled vs. disabled vs. neither -Disabled plugins remain installed but are skipped during loading. The disabled list is stored in `config.yaml` under `plugins.disabled`: +Plugins occupy one of three states: -```yaml -plugins: - disabled: - - my-noisy-plugin -``` +| State | Meaning | In `plugins.enabled`? | In `plugins.disabled`? | +|---|---|---|---| +| `enabled` | Loaded on next session | Yes | No | +| `disabled` | Explicitly off — won't load even if also in `enabled` | (irrelevant) | Yes | +| `not enabled` | Discovered but never opted in | No | No | + +The default for a newly-installed or bundled plugin is `not enabled`. `hermes plugins list` shows all three distinct states so you can tell what's been explicitly turned off vs. what's just waiting to be enabled. In a running session, `/plugins` shows which plugins are currently loaded.