Address Codex review: PluginManager reach-in + bm-recent list shape

Two fixes for the issues raised on PR #3.

P1 — ctx.register_command and ctx.register_skill silently no-op in real
Hermes installs. Memory-provider plugins are loaded through a stripped-
down _ProviderCollector (plugins/memory/__init__.py) that captures only
register_memory_provider; other registration methods are not delegated
to PluginManager. The bundled SKILL.md has been hitting this same path
since 0.1.5 — the call ran but the entry never landed.

Workaround: write directly to PluginManager._plugin_commands and
_plugin_skills from inside register(). Mirror the exact entry shape,
name normalization, and built-in-conflict guard from
PluginContext.register_command (plugins.py:401-453) and
register_skill (plugins.py:622-665). When the upstream collector is
patched, ctx.register_command/register_skill will write identical
entries — the reach-in becomes a redundant idempotent overwrite.

Recursion is safe: PluginManager.discover_and_load is idempotent
(plugins.py:699) and explicitly skips memory-provider plugins at the
manifest-routing stage (plugins.py:792-802), so calling
_ensure_plugins_discovered() from inside register() cannot re-enter.

New tests in tests/test_commands.py use a _ProviderCollector-shaped
ctx (no register_command attribute) and assert the reach-in actually
populates the fake PluginManager. The original tests used MagicMock,
which masked the silent skip by making every attribute exist.

P2 — bm_recent dropped real results. recent_activity(output_format=
"json") returns a bare list[dict] per the BM signature; the handler
only checked dict keys. Added the list branch + regression test
matching the real JSON shape.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: groksrc

CHANGELOG.md

@@ -7,10 +7,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 ## [0.2.0] — 2026-05-11
 
 ### Added
-- **Plugin-owned `/bm-*` slash commands** for CLI/gateway sessions. Eight commands give humans direct memory-graph access without going through the agent: `/bm-search`, `/bm-read`, `/bm-context`, `/bm-recent`, `/bm-status`, `/bm-remember`, `/bm-project`, `/bm-workspace`. Registered via `ctx.register_command(...)` (Hermes ≥ v0.11.0); older Hermes installs silently skip the slash surface. Closes #2.
+- **Plugin-owned `/bm-*` slash commands** for CLI/gateway sessions. Eight commands give humans direct memory-graph access without going through the agent: `/bm-search`, `/bm-read`, `/bm-context`, `/bm-recent`, `/bm-status`, `/bm-remember`, `/bm-project`, `/bm-workspace`. Closes #2.
 - **`bm_recent` tool** wrapping BM's `recent_activity`. Surfaces notes updated within a timeframe (`7d` default, accepts natural language like `"2 weeks"` or `"yesterday"`). Agent-facing and reused by `/bm-recent`.
 - **`remember_folder` config key** (default `"bm-remember"`). Separate from `capture_folder` so manual captures via `/bm-remember` don't intermix with auto-generated session transcripts. Notes are tagged `manual-capture` for further disambiguation.
 
+### Fixed
+- **`ctx.register_skill(...)` was silently no-opping since 0.1.5** in real Hermes installs. Hermes loads memory-provider plugins through a stripped-down `_ProviderCollector` context (`plugins/memory/__init__.py`) that captures only `register_memory_provider`; `register_skill` and `register_command` are not delegated. The plugin now writes directly to `PluginManager._plugin_commands` and `_plugin_skills`, matching the entry shape and name normalization `PluginContext.register_command` / `register_skill` produce. This makes both the new slash commands and the bundled SKILL.md work in current Hermes installs. The clean fix lives upstream — a small patch to teach `_ProviderCollector` to delegate — and once that lands, the reach-in becomes a redundant double-write of identical entries. Forward-compat `ctx.register_command` / `ctx.register_skill` calls remain in place for the future code path.
+
 ### Notes
 - `/bm-remember` derives the title from the first non-empty line of the input, trimmed to 80 chars; falls back to `Note YYYY-MM-DD HHMM UTC`.
 - `/bm-workspace` short-circuits in local mode with a one-line explanation. Workspaces are a BM Cloud concept.

__init__.py

@@ -1338,9 +1338,12 @@ def _bm_recent(raw_args: str) -> str:
             return f"bm-recent: {e}"
         data = _unwrap_json_or_text(raw)
         results: List[Any] = []
-        if isinstance(data, dict):
-            # BM groups recent activity under several possible keys depending
-            # on version; check the common ones.
+        if isinstance(data, list):
+            # BM's recent_activity returns `list[dict]` in JSON mode — that's
+            # the documented signature (`-> str | list[dict]`).
+            results = data
+        elif isinstance(data, dict):
+            # Older BM versions or wrapping layers may bury the rows under a key.
             for key in ("results", "items", "activity", "primary_results"):
                 val = data.get(key)
                 if isinstance(val, list):
@@ -1499,6 +1502,113 @@ def _bm_workspace(raw_args: str) -> str:
     ]
 
 
+# ---------------------------------------------------------------------------
+# PluginManager reach-in — workaround for Hermes's memory-provider collector
+# ---------------------------------------------------------------------------
+#
+# Hermes loads memory-provider plugins through a stripped-down `_ProviderCollector`
+# context (plugins/memory/__init__.py) that only captures `register_memory_provider`;
+# `register_command` and `register_skill` are not delegated. The result is that
+# `ctx.register_command(...)` and `ctx.register_skill(...)` calls in this plugin
+# silently no-op in real installs, even though Hermes's PluginManager *does*
+# expose working slash-command and skill registries (used by general plugins).
+#
+# The clean fix lives upstream — a ~15-line patch to teach `_ProviderCollector`
+# to delegate to PluginManager. Until that lands, we write to PluginManager's
+# registries ourselves, matching exactly the entry shape and normalization
+# `PluginContext.register_command` / `register_skill` produce. Idempotent with
+# the future upstream fix: both code paths write identical entries to the same
+# dicts.
+#
+# Recursion is safe: PluginManager.discover_and_load is idempotent
+# (plugins.py:699) and explicitly skips memory-provider plugins at the
+# manifest-routing stage (plugins.py:792-802), so calling
+# `_ensure_plugins_discovered()` from inside our register() cannot re-enter us.
+
+_PLUGIN_MANIFEST_NAME = "basic-memory"
+
+_SKILL_DESCRIPTION = (
+    "Reference for using bm_* tools and the Basic Memory knowledge graph "
+    "(search-before-answer, capture decisions, navigate via memory:// URLs)."
+)
+
+
+def _register_via_plugin_manager(
+    provider: "BasicMemoryProvider",
+    skill_path: Optional[Path] = None,
+) -> None:
+    """
+    Reach into Hermes's PluginManager to register slash commands and the
+    bundled skill, bypassing the memory-provider collector's no-op stubs.
+
+    Best-effort: any failure (Hermes not on path, internal API renamed,
+    discovery errored) logs at debug/warning and degrades to "no slash
+    commands" rather than breaking memory-provider registration.
+    """
+    try:
+        from hermes_cli.plugins import _ensure_plugins_discovered
+    except Exception as e:
+        logger.debug(
+            "basic-memory: hermes_cli.plugins unavailable (%s); skipping "
+            "slash-command reach-in",
+            e,
+        )
+        return
+
+    try:
+        mgr = _ensure_plugins_discovered()
+    except Exception as e:
+        logger.warning("basic-memory: PluginManager discovery failed: %s", e)
+        return
+
+    # Mirror PluginContext.register_command's name-conflict guard against
+    # built-in commands. Best-effort: if the import path changed, skip the
+    # check rather than dropping every command.
+    try:
+        from hermes_cli.commands import resolve_command  # type: ignore
+    except Exception:
+        resolve_command = None  # type: ignore[assignment]
+
+    plugin_commands = getattr(mgr, "_plugin_commands", None)
+    if plugin_commands is None:
+        logger.debug(
+            "basic-memory: PluginManager has no _plugin_commands attr; "
+            "slash commands skipped"
+        )
+    else:
+        for name, handler, description, args_hint in _build_slash_commands(provider):
+            # Mirror Hermes's normalization (plugins.py:426).
+            clean = name.lower().strip().lstrip("/").replace(" ", "-")
+            if not clean:
+                continue
+            if resolve_command is not None:
+                try:
+                    if resolve_command(clean) is not None:
+                        logger.warning(
+                            "basic-memory: skipping /%s — conflicts with "
+                            "a built-in command",
+                            clean,
+                        )
+                        continue
+                except Exception:
+                    pass
+            plugin_commands[clean] = {
+                "handler": handler,
+                "description": description or "Plugin command",
+                "plugin": _PLUGIN_MANIFEST_NAME,
+                "args_hint": (args_hint or "").strip(),
+            }
+
+    plugin_skills = getattr(mgr, "_plugin_skills", None)
+    if plugin_skills is not None and skill_path is not None and skill_path.exists():
+        plugin_skills[f"{_PLUGIN_MANIFEST_NAME}:basic-memory"] = {
+            "path": skill_path,
+            "plugin": _PLUGIN_MANIFEST_NAME,
+            "bare_name": "basic-memory",
+            "description": _SKILL_DESCRIPTION,
+        }
+
+
 # ---------------------------------------------------------------------------
 # atexit safety net (mirrors plugins/memory/openviking pattern)
 # ---------------------------------------------------------------------------
@@ -1534,22 +1644,23 @@ def register(ctx: Any) -> None:
     # through `system_prompt_block()`.
     skill_path = Path(__file__).resolve().parent / "skill" / "SKILL.md"
     if skill_path.exists() and hasattr(ctx, "register_skill"):
+        # Forward-compat: if Hermes's memory-provider collector ever delegates
+        # register_skill to PluginManager (or another loader passes us a real
+        # PluginContext), this lands the skill via the supported path. The
+        # reach-in below covers the current production collector either way.
         try:
             ctx.register_skill(
                 "basic-memory",
                 skill_path,
-                description=(
-                    "Reference for using bm_* tools and the Basic Memory "
-                    "knowledge graph (search-before-answer, capture decisions, "
-                    "navigate via memory:// URLs)."
-                ),
+                description=_SKILL_DESCRIPTION,
             )
         except Exception as e:
             logger.warning("basic-memory: register_skill failed: %s", e)
 
-    # Plugin-owned /bm-* slash commands. register_command landed in Hermes
-    # v0.11.0; the hasattr guard keeps older Hermes installs working — they
-    # just don't get the slash-command surface.
+    # Forward-compat: when Hermes's memory-provider collector gains
+    # register_command (PR to NousResearch/hermes-agent pending), this is the
+    # right path. Until then, hasattr returns False and we fall through to
+    # the reach-in below.
     if hasattr(ctx, "register_command"):
         for name, handler, description, args_hint in _build_slash_commands(provider):
             try:
@@ -1563,3 +1674,10 @@ def register(ctx: Any) -> None:
                 logger.warning(
                     "basic-memory: register_command(%s) failed: %s", name, e
                 )
+
+    # Write directly to PluginManager's registries. This is the production
+    # path today; see _register_via_plugin_manager docstring for the why.
+    _register_via_plugin_manager(
+        provider,
+        skill_path=skill_path if skill_path.exists() else None,
+    )