test(plugins): cover _discover_all_plugins recursion + cross-link loader

Add a TestDiscoverAllPlugins class covering the six cases the recursive
scan needs to handle:

- flat plugin uses its manifest ``name:`` as the key
- category-namespaced plugin keys off ``<category>/<dirname>`` even when
  the manifest ``name:`` is bare (regression test for the original bug —
  ``plugins/observability/langfuse/`` with ``name: langfuse`` must
  surface as ``observability/langfuse``, not ``langfuse``)
- user-installed plugin overrides bundled on key collision
- depth cap: anything below ``<root>/<category>/<plugin>/`` is ignored
- bundled ``memory/`` and ``context_engine/`` are skipped (they have
  their own loaders), but user plugins under those category names are
  still scanned

Also add an in-source comment next to the key derivation pointing at the
loader's matching line (``PluginManager._parse_manifest`` in
plugins.py:1027-1028), so future renames of one site flag the other.

Both items raised in Copilot review on #27161.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Guillaume Meyer 2026-05-16 23:14:26 +00:00 committed by Teknium
parent 21be7025c5
commit 5cbe0b1c4f
2 changed files with 116 additions and 0 deletions

View file

@ -761,6 +761,11 @@ def _discover_all_plugins() -> list:
description = manifest.get("description", "") description = manifest.get("description", "")
except Exception: except Exception:
pass pass
# Path-derived key, intentionally ignoring the manifest
# ``name:`` field for category-namespaced plugins — mirrors
# ``PluginManager._parse_manifest`` in plugins.py:1027-1028
# so renaming a directory (without touching plugin.yaml) shifts
# the registry key in both places consistently.
key = f"{prefix}/{d.name}" if prefix else manifest_name key = f"{prefix}/{d.name}" if prefix else manifest_name
src_label = source src_label = source
if source == "user" and (d / ".git").exists(): if source == "user" and (d / ".git").exists():

View file

@ -396,6 +396,117 @@ class TestCmdList:
cmd_list() cmd_list()
# ── _discover_all_plugins tests ───────────────────────────────────────────────
class TestDiscoverAllPlugins:
"""Exercise the recursive scan that powers ``hermes plugins list``.
Mirrors the layouts the runtime loader handles
(:meth:`PluginManager._scan_directory_level`): flat plugins at the root,
category-namespaced plugins one level deeper, and user-overrides-bundled
on key collision.
"""
@staticmethod
def _write_plugin(root: Path, segments: list, manifest_name: str = None) -> None:
plugin_dir = root
for seg in segments:
plugin_dir = plugin_dir / seg
plugin_dir.mkdir(parents=True, exist_ok=True)
manifest = {
"name": manifest_name or segments[-1],
"version": "0.1.0",
"description": f"Test plugin {'/'.join(segments)}",
}
(plugin_dir / "plugin.yaml").write_text(yaml.dump(manifest))
def _entries_by_key(self, tmp_path, monkeypatch) -> dict:
from hermes_cli import plugins_cmd
bundled = tmp_path / "bundled"
user = tmp_path / "user"
bundled.mkdir()
user.mkdir()
monkeypatch.setattr(
"hermes_cli.plugins.get_bundled_plugins_dir", lambda: bundled
)
monkeypatch.setattr(plugins_cmd, "_plugins_dir", lambda: user)
return bundled, user, lambda: {
e[0]: e for e in plugins_cmd._discover_all_plugins()
}
def test_flat_plugin_uses_manifest_name_as_key(self, tmp_path, monkeypatch):
bundled, _, discover = self._entries_by_key(tmp_path, monkeypatch)
self._write_plugin(bundled, ["disk-cleanup"])
entries = discover()
assert "disk-cleanup" in entries
assert entries["disk-cleanup"][3] == "bundled"
def test_category_namespaced_plugin_uses_path_derived_key(
self, tmp_path, monkeypatch
):
"""Regression test for the original bug — ``observability/langfuse``
and ``image_gen/openai`` must surface under their path-derived key,
not vanish because the category directory has no ``plugin.yaml``."""
bundled, _, discover = self._entries_by_key(tmp_path, monkeypatch)
# langfuse's real manifest declares ``name: langfuse`` (bare), but it
# lives under ``observability/`` — the key must reflect the path.
self._write_plugin(
bundled, ["observability", "langfuse"], manifest_name="langfuse"
)
self._write_plugin(bundled, ["image_gen", "openai"])
entries = discover()
assert "observability/langfuse" in entries
assert "image_gen/openai" in entries
# Bare manifest name must NOT leak through as a top-level key.
assert "langfuse" not in entries
assert "openai" not in entries
def test_user_overrides_bundled_on_key_collision(self, tmp_path, monkeypatch):
bundled, user, discover = self._entries_by_key(tmp_path, monkeypatch)
self._write_plugin(bundled, ["observability", "langfuse"])
self._write_plugin(user, ["observability", "langfuse"])
entries = discover()
assert entries["observability/langfuse"][3] == "user"
def test_depth_cap_skips_third_level(self, tmp_path, monkeypatch):
"""Anything deeper than ``<root>/<category>/<plugin>/`` is ignored,
matching the loader's depth cap."""
bundled, _, discover = self._entries_by_key(tmp_path, monkeypatch)
# plugins/a/b/c/plugin.yaml — too deep, must NOT be discovered.
self._write_plugin(bundled, ["a", "b", "c"])
entries = discover()
assert not any(k.startswith("a/") for k in entries), entries
def test_bundled_memory_and_context_engine_skipped(self, tmp_path, monkeypatch):
"""``plugins/memory/`` and ``plugins/context_engine/`` use their own
loaders; bundled entries inside them must not appear in the general
list (matches the pre-refactor skip set)."""
bundled, _, discover = self._entries_by_key(tmp_path, monkeypatch)
self._write_plugin(bundled, ["memory", "honcho"])
self._write_plugin(bundled, ["context_engine", "compressor"])
self._write_plugin(bundled, ["observability", "langfuse"])
entries = discover()
assert "memory/honcho" not in entries
assert "context_engine/compressor" not in entries
assert "observability/langfuse" in entries
def test_user_memory_subdir_is_still_scanned(self, tmp_path, monkeypatch):
"""The memory/context_engine skip only applies to *bundled* — a user
plugin at ``~/.hermes/plugins/memory/<x>/`` should still be discovered
so the user can see what they installed."""
bundled, user, discover = self._entries_by_key(tmp_path, monkeypatch)
self._write_plugin(user, ["memory", "my-custom-store"])
entries = discover()
assert "memory/my-custom-store" in entries
# ── _copy_example_files tests ───────────────────────────────────────────────── # ── _copy_example_files tests ─────────────────────────────────────────────────