mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-12 08:51:53 +00:00
fix(plugins): alias-normalize enable/disable for nested category plugins (follow-up to #41076)
#41076 makes `hermes plugins list` discover nested category plugins (e.g. observability/nemo_relay). This adds the missing enable/disable mutation path so those plugins can actually be toggled, and fixes two incomplete-update breakages on the #41076 base. Before: `hermes plugins enable nemo_relay` -> "Plugin 'nemo_relay' is not installed or bundled." (exit 1), because cmd_enable/cmd_disable went through _plugin_exists(), which only checked top-level plugins/<name>/. Changes: - Add _resolve_plugin_key(): resolve a bare manifest/leaf name OR a full path-derived key (observability/nemo_relay) to the canonical key the runtime loader gates on, reusing #41076's _discover_all_plugins(). A bare leaf name ambiguous across two categories resolves to None rather than silently picking one. - cmd_enable/cmd_disable resolve first, persist the canonical key, and drop any stale legacy bare-name alias so the enabled/disabled lists can't drift into a contradictory state. _plugin_exists delegates to the same resolver. - Fix #41076 base breakages: _discover_all_plugins now returns 6-tuples, but web_server._merged_plugins_hub() still unpacked 5 (ValueError on the dashboard plugins-hub endpoint) and several test_plugins_cmd_list.py fixtures were still 5-tuples. Both updated; the hub status check is now key-aware. Verified e2e on the real CLI + runtime loader (isolated HERMES_HOME): `hermes plugins enable nemo_relay` writes observability/nemo_relay to config.yaml and the loader then loads it (enabled=True, error=None); a stale bare-name alias is cleared on disable; the dashboard _merged_plugins_hub() runs without crashing. Adds resolution + enable/disable tests; full tests/hermes_cli/test_plugins_cmd* + web_server plugin tests green. Follow-up to #41076 (#41066). Branched from that PR's head.
This commit is contained in:
parent
ccacfdbd6d
commit
2b89afec79
4 changed files with 265 additions and 48 deletions
193
tests/hermes_cli/test_plugins_cmd_enable_disable_nested.py
Normal file
193
tests/hermes_cli/test_plugins_cmd_enable_disable_nested.py
Normal file
|
|
@ -0,0 +1,193 @@
|
|||
"""Tests for nested/alias-normalized enable & disable flows.
|
||||
|
||||
Companion to test_plugins_cmd_category_discovery.py. That file covers the
|
||||
*listing* side of nested category plugins (issue #41066). These tests cover
|
||||
the *mutation* side: `hermes plugins enable/disable` must resolve a bare name
|
||||
OR a full path-derived key (e.g. `observability/nemo_relay`) to the canonical
|
||||
registry key and write THAT — the same string PluginManager gates on — so a
|
||||
nested bundled plugin can actually be toggled.
|
||||
"""
|
||||
|
||||
import sys # noqa: F401
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
def _make_plugin_dir(parent: Path, name: str, manifest: dict) -> Path:
|
||||
d = parent / name
|
||||
d.mkdir(parents=True, exist_ok=True)
|
||||
import yaml
|
||||
(d / "plugin.yaml").write_text(yaml.dump(manifest), encoding="utf-8")
|
||||
(d / "__init__.py").write_text("def register(ctx): pass\n", encoding="utf-8")
|
||||
return d
|
||||
|
||||
|
||||
def _make_category_plugin(parent: Path, category: str, name: str, manifest: dict) -> Path:
|
||||
return _make_plugin_dir(parent / category, name, manifest)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def nested_plugin_env(tmp_path):
|
||||
"""A user-plugins dir containing one nested and one flat plugin, with the
|
||||
bundled dir pointed at an empty path. Returns the tmp_path."""
|
||||
_make_category_plugin(tmp_path, "observability", "nemo_relay", {
|
||||
"name": "nemo_relay", "version": "1.0.0", "description": "relay obs"
|
||||
})
|
||||
_make_plugin_dir(tmp_path, "disk-cleanup", {
|
||||
"name": "disk-cleanup", "version": "1.0.0"
|
||||
})
|
||||
return tmp_path
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _resolve_plugin_key
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestResolvePluginKey:
|
||||
@patch("hermes_cli.plugins.get_bundled_plugins_dir")
|
||||
@patch("hermes_cli.plugins_cmd._plugins_dir")
|
||||
def test_full_key_resolves_to_itself(self, mock_user, mock_bundled, nested_plugin_env):
|
||||
from hermes_cli.plugins_cmd import _resolve_plugin_key
|
||||
mock_user.return_value = nested_plugin_env
|
||||
mock_bundled.return_value = nested_plugin_env / "nonexistent"
|
||||
assert _resolve_plugin_key("observability/nemo_relay") == "observability/nemo_relay"
|
||||
|
||||
@patch("hermes_cli.plugins.get_bundled_plugins_dir")
|
||||
@patch("hermes_cli.plugins_cmd._plugins_dir")
|
||||
def test_bare_leaf_name_resolves_to_key(self, mock_user, mock_bundled, nested_plugin_env):
|
||||
from hermes_cli.plugins_cmd import _resolve_plugin_key
|
||||
mock_user.return_value = nested_plugin_env
|
||||
mock_bundled.return_value = nested_plugin_env / "nonexistent"
|
||||
# "nemo_relay" (bare) must normalize to the path-derived key.
|
||||
assert _resolve_plugin_key("nemo_relay") == "observability/nemo_relay"
|
||||
|
||||
@patch("hermes_cli.plugins.get_bundled_plugins_dir")
|
||||
@patch("hermes_cli.plugins_cmd._plugins_dir")
|
||||
def test_flat_plugin_resolves_to_name(self, mock_user, mock_bundled, nested_plugin_env):
|
||||
from hermes_cli.plugins_cmd import _resolve_plugin_key
|
||||
mock_user.return_value = nested_plugin_env
|
||||
mock_bundled.return_value = nested_plugin_env / "nonexistent"
|
||||
assert _resolve_plugin_key("disk-cleanup") == "disk-cleanup"
|
||||
|
||||
@patch("hermes_cli.plugins.get_bundled_plugins_dir")
|
||||
@patch("hermes_cli.plugins_cmd._plugins_dir")
|
||||
def test_unknown_returns_none(self, mock_user, mock_bundled, nested_plugin_env):
|
||||
from hermes_cli.plugins_cmd import _resolve_plugin_key
|
||||
mock_user.return_value = nested_plugin_env
|
||||
mock_bundled.return_value = nested_plugin_env / "nonexistent"
|
||||
assert _resolve_plugin_key("does-not-exist") is None
|
||||
|
||||
@patch("hermes_cli.plugins.get_bundled_plugins_dir")
|
||||
@patch("hermes_cli.plugins_cmd._plugins_dir")
|
||||
def test_ambiguous_leaf_name_returns_none(self, mock_user, mock_bundled, tmp_path):
|
||||
"""Same leaf name under two categories must NOT silently pick one."""
|
||||
from hermes_cli.plugins_cmd import _resolve_plugin_key
|
||||
_make_category_plugin(tmp_path, "image_gen", "openai", {"name": "image-gen-openai"})
|
||||
_make_category_plugin(tmp_path, "model-providers", "openai", {"name": "mp-openai"})
|
||||
mock_user.return_value = tmp_path
|
||||
mock_bundled.return_value = tmp_path / "nonexistent"
|
||||
# Bare "openai" is ambiguous -> None; the full key still resolves.
|
||||
assert _resolve_plugin_key("openai") is None
|
||||
assert _resolve_plugin_key("image_gen/openai") == "image_gen/openai"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# cmd_enable / cmd_disable — write the canonical key
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestEnableDisableNested:
|
||||
@patch("hermes_cli.plugins.get_bundled_plugins_dir")
|
||||
@patch("hermes_cli.plugins_cmd._plugins_dir")
|
||||
@patch("hermes_cli.plugins_cmd._save_disabled_set")
|
||||
@patch("hermes_cli.plugins_cmd._save_enabled_set")
|
||||
@patch("hermes_cli.plugins_cmd._get_disabled_set", return_value=set())
|
||||
@patch("hermes_cli.plugins_cmd._get_enabled_set", return_value=set())
|
||||
def test_enable_bare_name_writes_key(
|
||||
self, mock_en, mock_dis, mock_save_en, mock_save_dis,
|
||||
mock_user, mock_bundled, nested_plugin_env,
|
||||
):
|
||||
from hermes_cli.plugins_cmd import cmd_enable
|
||||
mock_user.return_value = nested_plugin_env
|
||||
mock_bundled.return_value = nested_plugin_env / "nonexistent"
|
||||
|
||||
cmd_enable("nemo_relay") # bare name
|
||||
|
||||
saved = mock_save_en.call_args[0][0]
|
||||
# The canonical key — NOT the bare name — must be persisted, because
|
||||
# that is what PluginManager matches when deciding to load.
|
||||
assert "observability/nemo_relay" in saved
|
||||
assert "nemo_relay" not in saved or "observability/nemo_relay" in saved
|
||||
|
||||
@patch("hermes_cli.plugins.get_bundled_plugins_dir")
|
||||
@patch("hermes_cli.plugins_cmd._plugins_dir")
|
||||
@patch("hermes_cli.plugins_cmd._save_disabled_set")
|
||||
@patch("hermes_cli.plugins_cmd._save_enabled_set")
|
||||
@patch("hermes_cli.plugins_cmd._get_disabled_set", return_value=set())
|
||||
@patch("hermes_cli.plugins_cmd._get_enabled_set", return_value=set())
|
||||
def test_enable_full_key_writes_key(
|
||||
self, mock_en, mock_dis, mock_save_en, mock_save_dis,
|
||||
mock_user, mock_bundled, nested_plugin_env,
|
||||
):
|
||||
from hermes_cli.plugins_cmd import cmd_enable
|
||||
mock_user.return_value = nested_plugin_env
|
||||
mock_bundled.return_value = nested_plugin_env / "nonexistent"
|
||||
|
||||
cmd_enable("observability/nemo_relay")
|
||||
saved = mock_save_en.call_args[0][0]
|
||||
assert "observability/nemo_relay" in saved
|
||||
|
||||
@patch("hermes_cli.plugins.get_bundled_plugins_dir")
|
||||
@patch("hermes_cli.plugins_cmd._plugins_dir")
|
||||
@patch("hermes_cli.plugins_cmd._save_disabled_set")
|
||||
@patch("hermes_cli.plugins_cmd._save_enabled_set")
|
||||
@patch("hermes_cli.plugins_cmd._get_disabled_set", return_value=set())
|
||||
@patch("hermes_cli.plugins_cmd._get_enabled_set", return_value=set())
|
||||
def test_disable_bare_name_writes_key_and_clears_alias(
|
||||
self, mock_en, mock_dis, mock_save_en, mock_save_dis,
|
||||
mock_user, mock_bundled, nested_plugin_env,
|
||||
):
|
||||
from hermes_cli.plugins_cmd import cmd_disable
|
||||
mock_user.return_value = nested_plugin_env
|
||||
mock_bundled.return_value = nested_plugin_env / "nonexistent"
|
||||
# Simulate an existing config where the plugin was enabled under the
|
||||
# legacy bare name — disabling must clear that too, or the plugin would
|
||||
# keep loading (PluginManager accepts the bare name as well).
|
||||
mock_en.return_value = {"nemo_relay"}
|
||||
|
||||
cmd_disable("nemo_relay")
|
||||
saved_dis = mock_save_dis.call_args[0][0]
|
||||
saved_en = mock_save_en.call_args[0][0]
|
||||
assert "observability/nemo_relay" in saved_dis
|
||||
assert "nemo_relay" not in saved_en # stale bare alias dropped
|
||||
|
||||
@patch("hermes_cli.plugins.get_bundled_plugins_dir")
|
||||
@patch("hermes_cli.plugins_cmd._plugins_dir")
|
||||
def test_enable_unknown_plugin_exits(self, mock_user, mock_bundled, nested_plugin_env):
|
||||
from hermes_cli.plugins_cmd import cmd_enable
|
||||
mock_user.return_value = nested_plugin_env
|
||||
mock_bundled.return_value = nested_plugin_env / "nonexistent"
|
||||
with pytest.raises(SystemExit):
|
||||
cmd_enable("does-not-exist")
|
||||
|
||||
@patch("hermes_cli.plugins.get_bundled_plugins_dir")
|
||||
@patch("hermes_cli.plugins_cmd._plugins_dir")
|
||||
@patch("hermes_cli.plugins_cmd._save_disabled_set")
|
||||
@patch("hermes_cli.plugins_cmd._save_enabled_set")
|
||||
@patch("hermes_cli.plugins_cmd._get_disabled_set", return_value=set())
|
||||
@patch("hermes_cli.plugins_cmd._get_enabled_set", return_value=set())
|
||||
def test_enable_flat_plugin_unchanged(
|
||||
self, mock_en, mock_dis, mock_save_en, mock_save_dis,
|
||||
mock_user, mock_bundled, nested_plugin_env,
|
||||
):
|
||||
"""Flat plugins keep writing their bare name (key == name) — no regression."""
|
||||
from hermes_cli.plugins_cmd import cmd_enable
|
||||
mock_user.return_value = nested_plugin_env
|
||||
mock_bundled.return_value = nested_plugin_env / "nonexistent"
|
||||
|
||||
cmd_enable("disk-cleanup")
|
||||
saved = mock_save_en.call_args[0][0]
|
||||
assert "disk-cleanup" in saved
|
||||
|
|
@ -18,9 +18,9 @@ def _args(**kwargs):
|
|||
|
||||
def test_filter_plugin_entries_enabled_only():
|
||||
entries = [
|
||||
("disk-cleanup", "2.0.0", "Bundled", "bundled", None),
|
||||
("web-search-plus", "2.2.0", "Search", "git", None),
|
||||
("old-plugin", "1.0.0", "Old", "user", None),
|
||||
("disk-cleanup", "2.0.0", "Bundled", "bundled", None, "disk-cleanup"),
|
||||
("web-search-plus", "2.2.0", "Search", "git", None, "web-search-plus"),
|
||||
("old-plugin", "1.0.0", "Old", "user", None, "old-plugin"),
|
||||
]
|
||||
|
||||
filtered = plugins_cmd._filter_plugin_entries(
|
||||
|
|
@ -35,9 +35,9 @@ def test_filter_plugin_entries_enabled_only():
|
|||
|
||||
def test_filter_plugin_entries_no_bundled():
|
||||
entries = [
|
||||
("disk-cleanup", "2.0.0", "Bundled", "bundled", None),
|
||||
("drawthings-grpc", "0.3.0", "Draw Things", "user", None),
|
||||
("web-search-plus", "2.2.0", "Search", "git", None),
|
||||
("disk-cleanup", "2.0.0", "Bundled", "bundled", None, "disk-cleanup"),
|
||||
("drawthings-grpc", "0.3.0", "Draw Things", "user", None, "drawthings-grpc"),
|
||||
("web-search-plus", "2.2.0", "Search", "git", None, "web-search-plus"),
|
||||
]
|
||||
|
||||
filtered = plugins_cmd._filter_plugin_entries(
|
||||
|
|
@ -52,8 +52,8 @@ def test_filter_plugin_entries_no_bundled():
|
|||
|
||||
def test_cmd_list_plain_compact_output(monkeypatch, capsys):
|
||||
entries = [
|
||||
("disk-cleanup", "2.0.0", "Bundled", "bundled", None),
|
||||
("web-search-plus", "2.2.0", "Search", "git", None),
|
||||
("disk-cleanup", "2.0.0", "Bundled", "bundled", None, "disk-cleanup"),
|
||||
("web-search-plus", "2.2.0", "Search", "git", None, "web-search-plus"),
|
||||
]
|
||||
monkeypatch.setattr(plugins_cmd, "_discover_all_plugins", lambda: entries)
|
||||
monkeypatch.setattr(plugins_cmd, "_get_enabled_set", lambda: {"web-search-plus"})
|
||||
|
|
@ -69,7 +69,7 @@ def test_cmd_list_plain_compact_output(monkeypatch, capsys):
|
|||
|
||||
|
||||
def test_cmd_list_json_output(monkeypatch, capsys):
|
||||
entries = [("web-search-plus", "2.2.0", "Search", "git", None)]
|
||||
entries = [("web-search-plus", "2.2.0", "Search", "git", None, "web-search-plus")]
|
||||
monkeypatch.setattr(plugins_cmd, "_discover_all_plugins", lambda: entries)
|
||||
monkeypatch.setattr(plugins_cmd, "_get_enabled_set", lambda: {"web-search-plus"})
|
||||
monkeypatch.setattr(plugins_cmd, "_get_disabled_set", lambda: set())
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue