Skip to content

Memory-provider plugins: _ProviderCollector doesn't delegate register_command / register_skill to PluginManager #23603

Description

@groksrc

Summary

Memory-provider plugins are loaded through plugins/memory/_load_provider_from_dir, which passes the plugin's register(ctx) a _ProviderCollector instance. That collector captures only register_memory_provider; every other registration method is missing or a no-op. As a result, memory-provider plugins that try to register slash commands (ctx.register_command(...)) or bundled skills (ctx.register_skill(...)) silently lose those registrations — even though PluginManager has fully working _plugin_commands and _plugin_skills registries that general plugins use successfully.

Where it happens

plugins/memory/__init__.py:288-305:

class _ProviderCollector:
    """Fake plugin context that captures register_memory_provider calls."""

    def __init__(self):
        self.provider = None

    def register_memory_provider(self, provider):
        self.provider = provider

    # No-op for other registration methods
    def register_tool(self, *args, **kwargs):
        pass

    def register_hook(self, *args, **kwargs):
        pass

    def register_cli_command(self, *args, **kwargs):
        pass

register_skill and register_command are not present at all — so hasattr(ctx, "register_skill") returns False, defensive guards skip the call entirely, and the plugin runs as if the bundled SKILL.md and slash commands never existed.

Impact

Any memory-provider plugin that wants in-session UX beyond the seven MemoryProvider hooks has no supported path. Two concrete cases we've hit in basicmachines-co/hermes-basic-memory:

  • register_skill has been silently no-opping since we added it in 0.1.5. The bundled SKILL.md was never reachable via skill:view basic-memory:basic-memory in real installs (manual symlink to ~/.hermes/skills/ was the only path that worked).
  • register_command for our new /bm-* surface (issue Support passing morph snapshot id #2 in our repo) silently dropped all eight commands.

This is also masked by the most natural unit-test idiom: passing a MagicMock() as ctx makes every attribute exist, so the calls "succeed" in tests while skipping in production. We only caught it because a code review (Codex) flagged the loader-path mismatch.

Proposed fix

A small surgical change in plugins/memory/__init__.py — teach _ProviderCollector to delegate to PluginManager. The dicts and dispatch are already there; only the write side is missing for memory-provider plugins. Roughly:

class _ProviderCollector:
    def __init__(self, manifest_name: str = ""):
        self.provider = None
        self.manifest_name = manifest_name

    def register_memory_provider(self, provider):
        self.provider = provider

    def register_skill(self, name, path, description=""):
        from hermes_cli.plugins import _ensure_plugins_discovered
        mgr = _ensure_plugins_discovered()
        qualified = f"{self.manifest_name}:{name}"
        mgr._plugin_skills[qualified] = {
            "path": path,
            "plugin": self.manifest_name,
            "bare_name": name,
            "description": description,
        }

    def register_command(self, name, handler, description="", args_hint=""):
        from hermes_cli.plugins import _ensure_plugins_discovered
        from hermes_cli.commands import resolve_command
        mgr = _ensure_plugins_discovered()
        clean = name.lower().strip().lstrip("/").replace(" ", "-")
        if not clean:
            return
        if resolve_command(clean) is not None:
            return  # conflicts with built-in
        mgr._plugin_commands[clean] = {
            "handler": handler,
            "description": description or "Plugin command",
            "plugin": self.manifest_name,
            "args_hint": (args_hint or "").strip(),
        }

    def register_tool(self, *args, **kwargs): pass
    def register_hook(self, *args, **kwargs): pass
    def register_cli_command(self, *args, **kwargs): pass

With _load_provider_from_dir passing manifest_name=provider_dir.name when constructing the collector.

Recursion is safe: PluginManager.discover_and_load is idempotent (plugins.py:699) and already skips memory-provider plugins at the manifest-routing stage (plugins.py:792-802), so the inner discovery call cannot re-enter register().

Happy to send a PR if that fits the project's contribution norms.

Workaround in the wild

Until this lands, plugins can reach into PluginManager from inside their own register() — same logic as the proposed delegation, just on the other side of the import boundary. We shipped this in basicmachines-co/hermes-basic-memory#3 (_register_via_plugin_manager in __init__.py) as a documented workaround. When this issue is fixed, the reach-in becomes a redundant double-write of identical entries — safe to leave in place during the transition, simple to remove later.

Link for reference: basicmachines-co/hermes-basic-memory#3

Metadata

Metadata

Assignees

No one assigned

    Labels

    P3Low — cosmetic, nice to havearea/memoryMemory subsystem: store, providers, sync, background reviewscomp/pluginsPlugin system and bundled pluginstool/memoryMemory tool and memory providerstype/bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions