Browse Source

test(hermes): realign suite with the pre_llm_call mechanism; slim docs to the README section

The 20-test suite still exercised the dead on_session_start/inject_message
mechanism (17 failures against the rewritten plugin). Rewritten for the
real contract: pre_llm_call registration + first-turn-only context return,
register_skill receiving pathlib.Path (the conftest mock now raises on str,
mirroring hermes' AttributeError that silently disables a plugin), both
install layouts resolving skills, loud failure when skills are missing,
tool mapping sourced verbatim from hermes-tools.md, and a bootstrap-size
guard against hermes' 10k-char context spill threshold. 19 tests, passing.

Install docs collapse into the README section per maintainer direction:
docs/README.hermes.md and .hermes-plugin/INSTALL.md are gone; the README
carries the two-line install plus the compaction caveat. plugin.yaml
version aligned to 6.1.1.
Jesse Vincent 1 month ago
parent
commit
b6613057ae

+ 0 - 30
.hermes-plugin/INSTALL.md

@@ -1,30 +0,0 @@
-# Hermes Agent — Superpowers Plugin
-
-## Install
-
-```bash
-hermes plugins install obra/superpowers --enable
-```
-
-Restart any active Hermes sessions after installing.
-
-## Smoke check
-
-Start a new session and send:
-> What are your superpowers?
-
-The model should describe brainstorming, TDD, debugging, and planning skills.
-If it doesn't, the bootstrap isn't loading — reinstall and restart.
-
-## Acceptance test
-
-Send in a fresh session:
-> Let's make a react todo list
-
-The `brainstorming` skill must trigger and run its flow before any code is written.
-
-## Uninstall
-
-```bash
-hermes plugins remove superpowers
-```

+ 1 - 1
.hermes-plugin/plugin.yaml

@@ -1,5 +1,5 @@
 name: superpowers
-version: 6.0.3
+version: 6.1.1
 description: Superpowers skills and workflow bootstrap for Hermes Agent
 author: obra
 provides_hooks:

+ 3 - 3
README.md

@@ -207,9 +207,9 @@ Install Superpowers as a Hermes plugin from this repository:
 hermes plugins install obra/superpowers --enable
 ```
 
-Restart any active Hermes sessions after installing.
-
-Detailed docs: [docs/README.hermes.md](docs/README.hermes.md)
+Restart any active Hermes sessions after installing. Note: Hermes has no
+post-compaction hook, so a very long session that compacts over its first
+turn loses the bootstrap — start a fresh session if skills stop triggering.
 
 ## The Basic Workflow
 

+ 0 - 29
docs/README.hermes.md

@@ -1,29 +0,0 @@
-# Hermes Agent
-
-Superpowers supports Hermes Agent via an in-process Python plugin (Shape B).
-
-## Install
-
-```bash
-hermes plugins install obra/superpowers --enable
-```
-
-## What you get
-
-All Superpowers skills auto-trigger in Hermes sessions:
-brainstorming before feature work, systematic-debugging on bugs,
-test-driven-development for implementation, writing-plans before
-touching code, and all other skills in `skills/`.
-
-## How it works
-
-The plugin registers an `on_session_start` hook with the Hermes plugin API.
-At the start of each session, the hook injects the `using-superpowers` bootstrap
-as a user-role message via `ctx.inject_message(role="user")`. A session-id guard
-prevents double-injection if the hook fires more than once per session.
-
-Skills are loaded on demand during the session using `skill_view("skill-name")`.
-
-## Verifying
-
-See `.hermes-plugin/INSTALL.md` for the smoke check and acceptance test.

+ 15 - 4
tests/hermes/conftest.py

@@ -1,3 +1,5 @@
+from pathlib import Path
+
 import pytest
 from unittest.mock import MagicMock
 
@@ -6,14 +8,23 @@ from unittest.mock import MagicMock
 def mock_ctx():
     ctx = MagicMock()
     ctx._hooks = {}
-    ctx._injected = []
+    ctx._skills = {}
 
     def register_hook(event, fn):
         ctx._hooks[event] = fn
 
-    def inject_message(content, role="user"):
-        ctx._injected.append({"content": content, "role": role})
+    def register_skill(name, path):
+        # Mimic hermes' real register_skill, which calls path.exists() and
+        # therefore breaks on a str (the bug that silently disabled the whole
+        # plugin, found 2026-07-23). Keeping that fidelity here means a
+        # regression to str paths fails these tests instead of failing
+        # silently inside hermes.
+        if not isinstance(path, Path):
+            raise AttributeError(
+                f"register_skill requires a pathlib.Path, got {type(path).__name__}"
+            )
+        ctx._skills[name] = path
 
     ctx.register_hook.side_effect = register_hook
-    ctx.inject_message.side_effect = inject_message
+    ctx.register_skill.side_effect = register_skill
     return ctx

+ 62 - 45
tests/hermes/test_bootstrap.py

@@ -1,6 +1,7 @@
+import importlib
 import os
 import sys
-import importlib
+
 import pytest
 
 sys.path.insert(0, os.path.abspath(
@@ -9,6 +10,10 @@ sys.path.insert(0, os.path.abspath(
 
 BOOTSTRAP_MARKER = "superpowers:using-superpowers bootstrap for hermes"
 
+# Hermes spills injected context over 10,000 chars to a file, which breaks
+# inline injection semantics. The bootstrap must stay under it with margin.
+HERMES_CONTEXT_SPILL_LIMIT = 10_000
+
 
 def _load():
     if "__init__" in sys.modules:
@@ -16,6 +21,11 @@ def _load():
     return importlib.import_module("__init__")
 
 
+def _bootstrap():
+    m = _load()
+    return m._build_bootstrap(m._skills_dir())
+
+
 class TestStripFrontmatter:
     def test_strips_yaml_block(self):
         m = _load()
@@ -33,49 +43,56 @@ class TestStripFrontmatter:
         assert m._strip_frontmatter(content) == "# Body"
 
 
-class TestGetBootstrap:
-    def test_returns_none_when_skill_file_missing(self, tmp_path):
-        m = _load()
-        m._bootstrap_cache = None
-        m._SKILLS_DIR = str(tmp_path / "nonexistent")
-        assert m._get_bootstrap() is None
-
-    def test_caches_false_on_missing_file(self, tmp_path):
+class TestSkillsDirResolution:
+    def test_repo_layout_resolves(self):
+        # The repo checkout IS the git-clone layout: .hermes-plugin/ and
+        # skills/ are siblings, so resolution must succeed from here.
         m = _load()
-        m._bootstrap_cache = None
-        m._SKILLS_DIR = str(tmp_path / "nonexistent")
-        m._get_bootstrap()
-        assert m._bootstrap_cache is False
-
-    def test_returns_string_with_real_skill(self):
-        m = _load()
-        m._bootstrap_cache = None
-        result = m._get_bootstrap()
-        assert result is not None
-        assert isinstance(result, str)
-
-    def test_same_object_returned_on_second_call(self):
-        m = _load()
-        m._bootstrap_cache = None
-        r1 = m._get_bootstrap()
-        r2 = m._get_bootstrap()
-        assert r1 is r2
-
-    def test_contains_marker(self):
-        m = _load()
-        m._bootstrap_cache = None
-        result = m._get_bootstrap()
-        assert BOOTSTRAP_MARKER in result
-
-    def test_contains_extremely_important_wrapper(self):
-        m = _load()
-        m._bootstrap_cache = None
-        result = m._get_bootstrap()
-        assert result.startswith("<EXTREMELY_IMPORTANT>")
-        assert result.rstrip().endswith("</EXTREMELY_IMPORTANT>")
-
-    def test_frontmatter_absent_from_output(self):
+        skills = m._skills_dir()
+        assert os.path.isfile(
+            os.path.join(skills, "using-superpowers", "SKILL.md")
+        )
+
+
+class TestBootstrapContent:
+    def test_marker_and_wrapper(self):
+        content = _bootstrap()
+        assert BOOTSTRAP_MARKER in content
+        assert content.startswith("<EXTREMELY_IMPORTANT>")
+        assert content.rstrip().endswith("</EXTREMELY_IMPORTANT>")
+
+    def test_contains_using_superpowers_body(self):
+        content = _bootstrap()
+        # A distinctive line from the skill body proves the real SKILL.md was
+        # embedded, not a stub.
+        assert "You have superpowers" in content
+        assert "## The Rule" in content
+
+    def test_frontmatter_stripped(self):
+        content = _bootstrap()
+        assert "---\nname:" not in content
+
+    def test_tool_mapping_sourced_from_reference_file(self):
         m = _load()
-        m._bootstrap_cache = None
-        result = m._get_bootstrap()
-        assert "---\nname:" not in result
+        content = _bootstrap()
+        ref = os.path.join(
+            m._skills_dir(), "using-superpowers", "references", "hermes-tools.md"
+        )
+        with open(ref, encoding="utf-8") as f:
+            ref_text = f.read().strip()
+        # The mapping is included verbatim from the reference file — the
+        # single source, not a drift-prone inline copy.
+        assert ref_text in content
+        assert "read_file" in content
+
+    def test_skill_view_guidance_present(self):
+        content = _bootstrap()
+        assert 'skill_view("superpowers:brainstorming")' in content
+
+    def test_under_hermes_context_spill_limit(self):
+        content = _bootstrap()
+        assert len(content) < HERMES_CONTEXT_SPILL_LIMIT, (
+            f"bootstrap is {len(content)} chars; hermes spills injected "
+            f"context over {HERMES_CONTEXT_SPILL_LIMIT} to a file, which "
+            "breaks inline injection"
+        )

+ 109 - 72
tests/hermes/test_plugin.py

@@ -1,105 +1,142 @@
+import importlib
+import importlib.util
 import os
+import shutil
 import sys
-import importlib
+from pathlib import Path
+
 import pytest
 
 # Point at the plugin directory
-_PLUGIN_DIR = os.path.join(os.path.dirname(__file__), "../../.hermes-plugin")
-sys.path.insert(0, os.path.abspath(_PLUGIN_DIR))
+_PLUGIN_DIR = os.path.abspath(
+    os.path.join(os.path.dirname(__file__), "../../.hermes-plugin")
+)
+sys.path.insert(0, _PLUGIN_DIR)
 
 BOOTSTRAP_MARKER = "superpowers:using-superpowers bootstrap for hermes"
 
 
 def _load_plugin():
-    """Re-import plugin module fresh (clears module-level cache)."""
+    """Re-import plugin module fresh."""
     if "__init__" in sys.modules:
         del sys.modules["__init__"]
     return importlib.import_module("__init__")
 
 
-class TestPluginRegistration:
-    def test_register_attaches_session_start_hook(self, mock_ctx):
-        plugin = _load_plugin()
-        plugin.register(mock_ctx)
-        mock_ctx.register_hook.assert_called_once()
-        event_name = mock_ctx.register_hook.call_args[0][0]
-        assert event_name == "on_session_start"
-
-
-class TestBootstrapInjection:
-    def test_first_session_start_injects_bootstrap(self, mock_ctx):
-        plugin = _load_plugin()
-        plugin.register(mock_ctx)
-        handler = mock_ctx._hooks["on_session_start"]
-        handler(session_id="sess-1", model="test-model", platform="test")
-        assert len(mock_ctx._injected) == 1
-        assert BOOTSTRAP_MARKER in mock_ctx._injected[0]["content"]
-
-    def test_injection_uses_user_role(self, mock_ctx):
-        plugin = _load_plugin()
-        plugin.register(mock_ctx)
-        handler = mock_ctx._hooks["on_session_start"]
-        handler(session_id="sess-1", model="test-model", platform="test")
-        assert mock_ctx._injected[0]["role"] == "user"
+def _fire_pre_llm(ctx, **kwargs):
+    hook = ctx._hooks["pre_llm_call"]
+    defaults = {
+        "session_id": "s1",
+        "user_message": "hi",
+        "conversation_history": [],
+        "is_first_turn": False,
+        "model": "test-model",
+        "platform": "cli",
+    }
+    defaults.update(kwargs)
+    return hook(**defaults)
 
-    def test_dedup_skips_on_same_session_id(self, mock_ctx):
-        plugin = _load_plugin()
-        plugin.register(mock_ctx)
-        handler = mock_ctx._hooks["on_session_start"]
-        handler(session_id="sess-1", model="test-model", platform="test")
-        handler(session_id="sess-1", model="test-model", platform="test")
-        assert len(mock_ctx._injected) == 1
 
-    def test_reinjects_on_new_session_id(self, mock_ctx):
+class TestPluginRegistration:
+    def test_register_attaches_only_pre_llm_call_hook(self, mock_ctx):
         plugin = _load_plugin()
         plugin.register(mock_ctx)
-        handler = mock_ctx._hooks["on_session_start"]
-        handler(session_id="sess-1", model="test-model", platform="test")
-        handler(session_id="sess-2", model="test-model", platform="test")
-        assert len(mock_ctx._injected) == 2
+        assert list(mock_ctx._hooks.keys()) == ["pre_llm_call"]
 
-    def test_missing_skill_file_skips_silently(self, mock_ctx, tmp_path):
+    def test_register_registers_every_stock_skill_as_path(self, mock_ctx):
         plugin = _load_plugin()
-        plugin._bootstrap_cache = None
-        plugin._SKILLS_DIR = str(tmp_path / "nonexistent")
         plugin.register(mock_ctx)
-        handler = mock_ctx._hooks["on_session_start"]
-        handler(session_id="sess-1", model="test-model", platform="test")
-        assert len(mock_ctx._injected) == 0
-
-    def test_cache_populated_after_first_call(self, mock_ctx):
+        # The conftest mock raises on non-Path (mirroring hermes' real
+        # register_skill), so reaching these asserts proves every
+        # registration passed a pathlib.Path.
+        assert "using-superpowers" in mock_ctx._skills
+        assert "brainstorming" in mock_ctx._skills
+        for name, path in mock_ctx._skills.items():
+            assert isinstance(path, Path)
+            assert path.name == "SKILL.md"
+            assert path.parent.name == name
+            assert path.is_file()
+
+    def test_registered_skills_match_skill_directories(self, mock_ctx):
         plugin = _load_plugin()
-        plugin._bootstrap_cache = None
         plugin.register(mock_ctx)
-        handler = mock_ctx._hooks["on_session_start"]
-        handler(session_id="sess-1", model="test-model", platform="test")
-        assert plugin._bootstrap_cache is not None
-        assert plugin._bootstrap_cache is not False
+        skills_root = plugin._skills_dir()
+        expected = {
+            entry
+            for entry in os.listdir(skills_root)
+            if os.path.isfile(os.path.join(skills_root, entry, "SKILL.md"))
+        }
+        assert set(mock_ctx._skills.keys()) == expected
 
 
-class TestBootstrapContent:
-    def test_contains_extremely_important_tags(self, mock_ctx):
+class TestBootstrapInjection:
+    def test_first_turn_returns_bootstrap_context(self, mock_ctx):
         plugin = _load_plugin()
         plugin.register(mock_ctx)
-        handler = mock_ctx._hooks["on_session_start"]
-        handler(session_id="sess-1", model="test-model", platform="test")
-        content = mock_ctx._injected[0]["content"]
-        assert "<EXTREMELY_IMPORTANT>" in content
-        assert "</EXTREMELY_IMPORTANT>" in content
-
-    def test_frontmatter_stripped(self, mock_ctx):
+        result = _fire_pre_llm(mock_ctx, is_first_turn=True)
+        assert isinstance(result, dict)
+        content = result["context"]
+        assert BOOTSTRAP_MARKER in content
+        assert content.startswith("<EXTREMELY_IMPORTANT>")
+        assert content.rstrip().endswith("</EXTREMELY_IMPORTANT>")
+
+    def test_later_turns_return_none(self, mock_ctx):
         plugin = _load_plugin()
         plugin.register(mock_ctx)
-        handler = mock_ctx._hooks["on_session_start"]
-        handler(session_id="sess-1", model="test-model", platform="test")
-        content = mock_ctx._injected[0]["content"]
-        assert "---\nname:" not in content
+        assert _fire_pre_llm(mock_ctx, is_first_turn=False) is None
+        assert _fire_pre_llm(mock_ctx, is_first_turn=None) is None
 
-    def test_tool_mapping_present(self, mock_ctx):
+    def test_hook_tolerates_future_kwargs(self, mock_ctx):
         plugin = _load_plugin()
         plugin.register(mock_ctx)
-        handler = mock_ctx._hooks["on_session_start"]
-        handler(session_id="sess-1", model="test-model", platform="test")
-        content = mock_ctx._injected[0]["content"]
-        assert "Hermes tool mapping" in content
-        assert "read_file" in content
+        result = _fire_pre_llm(
+            mock_ctx, is_first_turn=True, telemetry_schema_version=3
+        )
+        assert BOOTSTRAP_MARKER in result["context"]
+
+
+class TestLayoutResolution:
+    def _stage(self, tmp_path, layout):
+        """Copy the plugin module + a minimal skills tree in the given layout."""
+        src_skills = Path(_PLUGIN_DIR).parent / "skills"
+        if layout == "clone":
+            plugdir = tmp_path / "superpowers" / ".hermes-plugin"
+        else:  # flat: module at the plugin dir root, skills nested inside it
+            plugdir = tmp_path / "superpowers"
+        skills = tmp_path / "superpowers" / "skills"
+        plugdir.mkdir(parents=True, exist_ok=True)
+        shutil.copy(Path(_PLUGIN_DIR) / "__init__.py", plugdir / "__init__.py")
+        for skill in ("using-superpowers", "brainstorming"):
+            shutil.copytree(src_skills / skill, skills / skill)
+        return plugdir
+
+    def _load_from(self, plugdir):
+        spec = importlib.util.spec_from_file_location(
+            f"hermes_plugin_test_{plugdir.parent.name}_{plugdir.name}",
+            plugdir / "__init__.py",
+        )
+        mod = importlib.util.module_from_spec(spec)
+        spec.loader.exec_module(mod)
+        return mod
+
+    def test_clone_layout_resolves_sibling_skills(self, tmp_path, mock_ctx):
+        # git-clone install: .hermes-plugin/ and skills/ are siblings.
+        plugdir = self._stage(tmp_path, "clone")
+        mod = self._load_from(plugdir)
+        mod.register(mock_ctx)
+        assert "using-superpowers" in mock_ctx._skills
+
+    def test_flat_layout_resolves_nested_skills(self, tmp_path, mock_ctx):
+        # flattened install: module at the plugin dir root, skills/ inside it.
+        plugdir = self._stage(tmp_path, "flat")
+        mod = self._load_from(plugdir)
+        mod.register(mock_ctx)
+        assert "using-superpowers" in mock_ctx._skills
+
+    def test_missing_skills_raises_loudly(self, tmp_path, mock_ctx):
+        plugdir = tmp_path / "superpowers"
+        plugdir.mkdir(parents=True)
+        shutil.copy(Path(_PLUGIN_DIR) / "__init__.py", plugdir / "__init__.py")
+        mod = self._load_from(plugdir)
+        with pytest.raises(RuntimeError, match="cannot find the skills"):
+            mod.register(mock_ctx)