diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index dd3ca48..f520d51 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -18,8 +18,8 @@ jobs: # Validate the kit against the *real* hermes-agent source so it can't silently # drift from upstream. Checks out NousResearch/hermes-agent and points the - # contract tests at it; they skip if the import fails, so this never blocks on - # an upstream layout change — it goes red only on a genuine contract break. + # contract tests at it; explicit checkout import or layout drift fails this + # job so an upstream contract break cannot become a misleading skipped green. hermes-contract: runs-on: ubuntu-latest steps: diff --git a/AGENTS.md b/AGENTS.md index 5dd27ff..c12603b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,13 +1,14 @@ # hermes-plugin-kit -Convention-correct helper library for registering `hermes-agent` plugin tools. +Convention-correct helper library for registering `hermes-agent` plugin tools, +hooks, and skills. This repository is an installable Python package, not a path-loaded runtime plugin. ## Working Rules -- Keep the public API centered on `@tool`, `register_all`, and the argument - helpers exported from `hermes_plugin_kit`. +- Keep `@tool` and `register_all` backward compatible. Use `@hook`, + `plugin_skill`, and `register_plugin` for full plugin lifecycle registration. - Use `tool_name(namespace, verb, noun)` for new tools and prefer explicit verbs such as `read`, `write`, and `patch`. Do not use Hermes agent-loop names (`memory`, `todo`, `session_search`, `delegate_task`) as plugin tools. diff --git a/README.md b/README.md index e6b0b61..5a95ba1 100644 --- a/README.md +++ b/README.md @@ -1,13 +1,15 @@ # hermes-plugin-kit -> A `@tool` decorator for [hermes-agent](https://github.com/NousResearch/hermes-agent) plugins — convention-correct LLM tool schemas, validation, logging, and the JSON envelope, baked in. +> Lifecycle helpers for [hermes-agent](https://github.com/NousResearch/hermes-agent) plugins — convention-correct tools, hooks, skills, validation, and safe logging, baked in. [![test](https://github.com/offendingcommit/hermes-plugin-kit/actions/workflows/test.yml/badge.svg)](https://github.com/offendingcommit/hermes-plugin-kit/actions/workflows/test.yml) ![python](https://img.shields.io/badge/python-3.11%2B-blue) -`hermes-plugin-kit` is a tiny, dependency-free helper for authoring **tools** in -[hermes-agent](https://github.com/NousResearch/hermes-agent) plugins. Decorate a -handler with `@tool`, register it with `register_all`, and the LLM-facing schema, +`hermes-plugin-kit` is a tiny, dependency-free helper for authoring plugins for +[hermes-agent](https://github.com/NousResearch/hermes-agent). Decorate a tool +with `@tool` or a lifecycle callback with `@hook`, then use `register_plugin` to +register tools, hooks, and plugin-owned skills together. Existing tool-only +plugins can keep using `register_all`; the LLM-facing schema, argument validation, structured logging, and the JSON result envelope are all generated for you — correctly, every time. @@ -122,6 +124,42 @@ That's it. `discord_read_thread` is registered with a `parameters`-wrapped schem self-documenting description, required-argument validation, logging, and the JSON envelope — none of which you had to write. +## Hooks and plugin skills + +Use the lifecycle entrypoint when a plugin provides more than tools: + +```python +from pathlib import Path +from hermes_plugin_kit import hook, plugin_skill, register_plugin + +@hook("pre_llm_call") +def inject_context(**kwargs): + return {"context": build_context(kwargs)} + +SKILLS = ( + plugin_skill( + "temporal-awareness", + Path(__file__).with_name("SKILL.md"), + "Calibrate responses against local time and message gaps.", + optional=True, + ), +) + +def register(ctx): + return register_plugin(ctx, __name__, skills=SKILLS) +``` + +`@hook` forwards Hermes keyword arguments and return values unchanged. It logs +only the hook name, elapsed time, result type, and supplied `session_id` or +`task_id`; callback payloads and exception messages are never logged. Exceptions +are re-raised so Hermes retains its normal per-plugin isolation behavior. + +`plugin_skill` validates the bare skill name, `SKILL.md` path, and description. +Required missing skills fail registration; optional missing skills warn and are +reported in the returned `RegistrationSummary`. Hermes supplies the plugin +namespace, so a declared `temporal-awareness` skill from plugin +`temporal-awareness` resolves as `temporal-awareness:temporal-awareness`. + ## Tool names Hermes uses one global tool registry, and the agent loop intercepts core names diff --git a/hermes_plugin_kit/__init__.py b/hermes_plugin_kit/__init__.py index b3dbf4b..48c1342 100644 --- a/hermes_plugin_kit/__init__.py +++ b/hermes_plugin_kit/__init__.py @@ -50,10 +50,17 @@ def register(ctx): import re import sys import time +from dataclasses import dataclass +from pathlib import Path from typing import Any, Callable __all__ = [ "tool", + "hook", + "plugin_skill", + "register_plugin", + "PluginSkill", + "RegistrationSummary", "register_all", "build_schema", "tool_name", @@ -65,11 +72,33 @@ def register(ctx): ] _SPEC_ATTR = "_hpk_tool_spec" +_HOOK_SPEC_ATTR = "_hpk_hook_spec" _REDACT_HINTS = ("token", "secret", "password", "passwd", "api_key", "apikey", "auth") _MAX_LOG_CHARS = 200 _TOOL_NAME_RE = re.compile(r"^[a-z][a-z0-9_]*$") _AGENT_LOOP_TOOL_NAMES = frozenset({"todo", "memory", "session_search", "delegate_task"}) _RESERVED_NAMESPACE_PREFIXES = ("memory_",) +_SKILL_NAME_RE = re.compile(r"^[a-zA-Z0-9_-]+$") + + +@dataclass(frozen=True) +class PluginSkill: + """Validated declaration for a plugin-owned, read-only Hermes skill.""" + + name: str + path: Path + description: str + optional: bool = False + + +@dataclass(frozen=True) +class RegistrationSummary: + """Inventory of lifecycle surfaces registered by :func:`register_plugin`.""" + + tools: tuple[str, ...] = () + hooks: tuple[str, ...] = () + skills: tuple[str, ...] = () + skipped_optional_skills: tuple[str, ...] = () # --------------------------------------------------------------------------- @@ -239,10 +268,85 @@ def _truncate(value: Any) -> str: return text if len(text) <= _MAX_LOG_CHARS else text[:_MAX_LOG_CHARS] + "…" +def _safe_context(kwargs: dict[str, Any]) -> dict[str, Any]: + """Return only Hermes correlation identifiers that are safe to log.""" + return { + key: kwargs[key] + for key in ("session_id", "task_id") + if kwargs.get(key) is not None + } + + # --------------------------------------------------------------------------- # The decorator # --------------------------------------------------------------------------- +def hook(name: str) -> Callable: + """Mark and instrument a Hermes lifecycle hook callback. + + The wrapper preserves Hermes' callback contract: keyword arguments and the + return value pass through unchanged, and exceptions are re-raised for the + plugin manager to isolate. Logs contain correlation identifiers only, never + message payloads or exception messages. + """ + if not isinstance(name, str) or not name.strip(): + raise ValueError("hook name is required") + hook_name = name.strip() + + def decorate(fn: Callable) -> Callable: + log = logging.getLogger(fn.__module__ or "hermes_plugin_kit") + + @functools.wraps(fn) + def wrapper(**kwargs: Any) -> Any: + started = time.perf_counter() + context = _truncate(_safe_context(kwargs)) + log.debug( + "%s: invoked; context=%s", + hook_name, + context, + ) + try: + result = fn(**kwargs) + except Exception as exc: + log.warning( + "%s: callback raised; elapsed_ms=%.2f; error_type=%s; context=%s", + hook_name, + (time.perf_counter() - started) * 1000, + type(exc).__name__, + context, + ) + raise + log.info( + "%s: ok; elapsed_ms=%.2f; result=%s; context=%s", + hook_name, + (time.perf_counter() - started) * 1000, + type(result).__name__, + context, + ) + return result + + setattr(wrapper, _HOOK_SPEC_ATTR, {"name": hook_name}) + return wrapper + + return decorate + + +def plugin_skill( + name: str, + path: str | Path, + description: str, + optional: bool = False, +) -> PluginSkill: + """Declare a plugin-owned ``SKILL.md`` for :func:`register_plugin`.""" + if not isinstance(name, str) or not _SKILL_NAME_RE.fullmatch(name): + raise ValueError("skill name must match [a-zA-Z0-9_-]+ and contain no namespace") + skill_path = Path(path) + if skill_path.name != "SKILL.md": + raise ValueError("skill path must point to SKILL.md") + if not isinstance(description, str) or not description.strip(): + raise ValueError("skill description is required") + return PluginSkill(name, skill_path, description.strip(), bool(optional)) + def tool( *, toolset: str, @@ -280,11 +384,7 @@ def wrapper(args: dict, **kwargs: Any) -> str: args = args or {} started = time.perf_counter() safe_args = _truncate(_redacted_args(args)) - context = { - key: kwargs[key] - for key in ("session_id", "task_id") - if kwargs.get(key) is not None - } + context = _safe_context(kwargs) log.debug( "%s: invoked; args=%s; context=%s", tool_name, @@ -370,15 +470,7 @@ def register_all(ctx: Any, module: Any) -> int: if not spec or spec["name"] in seen: continue seen.add(spec["name"]) - ctx.register_tool( - name=spec["name"], - toolset=spec["toolset"], - schema=spec["schema"], - handler=obj, - requires_env=spec["requires_env"], - description=spec["schema"]["description"], - emoji=spec["emoji"], - ) + _register_tool(ctx, obj, spec) count += 1 log.info( "hermes_plugin_kit: registered %d tool(s); names=%s", @@ -386,3 +478,110 @@ def register_all(ctx: Any, module: Any) -> int: ",".join(sorted(seen)) or "", ) return count + + +def _register_tool(ctx: Any, handler: Callable, spec: dict[str, Any]) -> None: + """Bind one kit-decorated tool to the current Hermes plugin context.""" + ctx.register_tool( + name=spec["name"], + toolset=spec["toolset"], + schema=spec["schema"], + handler=handler, + requires_env=spec["requires_env"], + description=spec["schema"]["description"], + emoji=spec["emoji"], + ) + + +def register_plugin( + ctx: Any, + module: Any, + skills: tuple[PluginSkill, ...] | list[PluginSkill] = (), +) -> RegistrationSummary: + """Register decorated tools, hooks, and declared skills from *module*. + + Unlike the backward-compatible :func:`register_all`, this lifecycle-level + entrypoint rejects distinct declarations that share a public name. Missing + optional skills are warned and skipped; missing required skills fail fast. + """ + if isinstance(module, str): + module = sys.modules[module] + log = logging.getLogger(getattr(module, "__name__", "hermes_plugin_kit")) + + tools: dict[str, Callable] = {} + hooks: dict[str, Callable] = {} + for _, obj in inspect.getmembers(module): + tool_spec = getattr(obj, _SPEC_ATTR, None) + if tool_spec: + existing = tools.get(tool_spec["name"]) + if existing is not None and existing is not obj: + raise ValueError(f"duplicate tool name: {tool_spec['name']}") + tools[tool_spec["name"]] = obj + + hook_spec = getattr(obj, _HOOK_SPEC_ATTR, None) + if hook_spec: + existing = hooks.get(hook_spec["name"]) + if existing is not None and existing is not obj: + raise ValueError(f"duplicate hook name: {hook_spec['name']}") + hooks[hook_spec["name"]] = obj + + declared_skills: dict[str, PluginSkill] = {} + for skill in skills: + if not isinstance(skill, PluginSkill): + raise TypeError("skills must contain plugin_skill() declarations") + if skill.name in declared_skills: + raise ValueError(f"duplicate skill name: {skill.name}") + declared_skills[skill.name] = skill + + available_skills: list[PluginSkill] = [] + skipped_skills: list[str] = [] + for name in sorted(declared_skills): + skill = declared_skills[name] + if skill.path.is_file(): + available_skills.append(skill) + continue + if not skill.optional: + raise FileNotFoundError(f"SKILL.md not found at {skill.path}") + log.warning( + "hermes_plugin_kit: optional skill missing; name=%s; path=%s", + skill.name, + skill.path, + ) + skipped_skills.append(name) + + registered_tools: list[str] = [] + for name in sorted(tools): + obj = tools[name] + spec = getattr(obj, _SPEC_ATTR) + _register_tool(ctx, obj, spec) + registered_tools.append(name) + + registered_hooks: list[str] = [] + for name in sorted(hooks): + ctx.register_hook(name, hooks[name]) + registered_hooks.append(name) + + registered_skills: list[str] = [] + for skill in available_skills: + ctx.register_skill( + name=skill.name, + path=skill.path, + description=skill.description, + ) + registered_skills.append(skill.name) + + summary = RegistrationSummary( + tools=tuple(registered_tools), + hooks=tuple(registered_hooks), + skills=tuple(registered_skills), + skipped_optional_skills=tuple(skipped_skills), + ) + log.info( + "hermes_plugin_kit: registered plugin lifecycle; tools=%s; hooks=%s; " + "skills=%s; skipped_optional_skills=%s", + ",".join(summary.tools) or "", + ",".join(summary.hooks) or "", + ",".join(summary.skills) or "", + ",".join(summary.skipped_optional_skills) or "", + ) + return summary diff --git a/pyproject.toml b/pyproject.toml index 7e4ce79..6d7d6c4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -9,7 +9,7 @@ build-backend = "setuptools.build_meta" [project] name = "hermes-plugin-kit" version = "0.1.0" -description = "Convention-correct tool registration for hermes-agent plugins." +description = "Convention-correct lifecycle registration for hermes-agent plugins." readme = "README.md" requires-python = ">=3.11" license = { text = "MIT" } diff --git a/tests/test_hermes_contract.py b/tests/test_hermes_contract.py index caf98b9..9ba7a79 100644 --- a/tests/test_hermes_contract.py +++ b/tests/test_hermes_contract.py @@ -27,26 +27,50 @@ def _import_real_hermes(): - """Return a namespace with the real PluginContext / VALID_HOOKS / registry, or None.""" + """Return real plugin runtime APIs, failing when an explicit checkout is invalid.""" candidates: list[Path] = [] env_path = os.environ.get("HERMES_AGENT_PATH") if env_path: - candidates.append(Path(env_path)) + explicit_root = Path(env_path) + if not (explicit_root / "hermes_cli" / "plugins.py").exists(): + raise FileNotFoundError( + f"HERMES_AGENT_PATH has no hermes_cli/plugins.py: {explicit_root}" + ) + sys.path.insert(0, str(explicit_root)) candidates.append(Path.home() / "hermes-agent") candidates.append(Path.home() / ".hermes" / "hermes-agent") def _try(): - from hermes_cli.plugins import PluginContext, VALID_HOOKS # type: ignore + from hermes_cli.plugins import ( # type: ignore + PluginContext, + PluginManager, + PluginManifest, + VALID_HOOKS, + ) from tools.registry import registry # type: ignore + # A stale checkout that predates plugin-owned skills is not the + # lifecycle contract this suite is intended to certify. + if not hasattr(PluginContext, "register_skill"): + raise ImportError("hermes-agent PluginContext.register_skill is unavailable") + if not hasattr(PluginManager, "find_plugin_skill"): + raise ImportError("hermes-agent PluginManager.find_plugin_skill is unavailable") + return types.SimpleNamespace( - PluginContext=PluginContext, VALID_HOOKS=set(VALID_HOOKS), registry=registry + PluginContext=PluginContext, + PluginManager=PluginManager, + PluginManifest=PluginManifest, + VALID_HOOKS=set(VALID_HOOKS), + registry=registry, ) try: return _try() except Exception: - pass + if env_path: + # An explicit contract checkout is authoritative in CI. Do not turn + # import or layout drift into a misleading skipped-green build. + raise for root in candidates: if not (root / "hermes_cli" / "plugins.py").exists(): @@ -144,6 +168,46 @@ def test_register_all_binds_to_real_plugincontext_signature(self) -> None: except TypeError as exc: # pragma: no cover - failure path self.fail(f"register_all call does not match PluginContext.register_tool: {exc}") + def test_lifecycle_registration_invokes_hook_and_resolves_qualified_skill(self) -> None: + @hpk.hook("pre_llm_call") + def contract_hook(**kwargs): + return {"context": kwargs["message"]} + + module = types.ModuleType("contract_lifecycle_plugin") + module.contract_hook = contract_hook + manager = _REAL.PluginManager() + manifest = _REAL.PluginManifest(name="contract-plugin") + ctx = _REAL.PluginContext(manifest, manager) + + from tempfile import TemporaryDirectory + + with TemporaryDirectory() as tmp: + path = Path(tmp) / "SKILL.md" + path.write_text("# Contract skill\n") + summary = hpk.register_plugin( + ctx, + module, + skills=(hpk.plugin_skill("probe", path, "Contract probe"),), + ) + self.assertEqual(summary.hooks, ("pre_llm_call",)) + self.assertEqual( + manager.invoke_hook("pre_llm_call", message="gateway-shaped"), + [{"context": "gateway-shaped"}], + ) + self.assertEqual(manager.find_plugin_skill("contract-plugin:probe"), path) + + def test_lifecycle_calls_bind_to_real_plugincontext_signatures(self) -> None: + hook_sig = inspect.signature(_REAL.PluginContext.register_hook) + hook_sig.bind(None, "pre_llm_call", lambda **kwargs: None) + + skill_sig = inspect.signature(_REAL.PluginContext.register_skill) + skill_sig.bind( + None, + name="probe", + path=Path("SKILL.md"), + description="Probe", + ) + if __name__ == "__main__": unittest.main() diff --git a/tests/test_kit.py b/tests/test_kit.py index 8c52a61..ad444af 100644 --- a/tests/test_kit.py +++ b/tests/test_kit.py @@ -1,7 +1,10 @@ from __future__ import annotations import json +import tempfile +import types import unittest +from pathlib import Path import hermes_plugin_kit as hpk @@ -14,6 +17,19 @@ def register_tool(self, **kwargs) -> None: self.tools.append(kwargs) +class FakePluginCtx(FakeCtx): + def __init__(self) -> None: + super().__init__() + self.hooks: list[tuple[str, object]] = [] + self.skills: list[dict] = [] + + def register_hook(self, hook_name, callback) -> None: + self.hooks.append((hook_name, callback)) + + def register_skill(self, **kwargs) -> None: + self.skills.append(kwargs) + + @hpk.tool( toolset="messaging", namespace="sample", @@ -187,5 +203,147 @@ def no_doc(args, **kwargs): return {} +class HookBehaviorTests(unittest.TestCase): + def test_forwards_kwargs_and_return_value_exactly(self) -> None: + marker = object() + + @hpk.hook("pre_llm_call") + def callback(**kwargs): + self.assertIs(kwargs["payload"], marker) + return marker + + with self.assertLogs(level="DEBUG") as cap: + self.assertIs(callback(payload=marker, session_id="session-1"), marker) + joined = "\n".join(cap.output) + self.assertIn("pre_llm_call: invoked", joined) + self.assertIn("session-1", joined) + self.assertNotIn(repr(marker), joined) + self.assertRegex(joined, r"elapsed_ms=\d+\.\d{2}") + + def test_reraises_and_does_not_log_payload_or_exception_message(self) -> None: + @hpk.hook("pre_llm_call") + def callback(**kwargs): + raise RuntimeError("private exception text") + + with self.assertLogs(level="WARNING") as cap: + with self.assertRaisesRegex(RuntimeError, "private exception text"): + callback(message="private message text", task_id="task-1") + joined = "\n".join(cap.output) + self.assertIn("RuntimeError", joined) + self.assertIn("task-1", joined) + self.assertNotIn("private exception text", joined) + self.assertNotIn("private message text", joined) + + def test_hook_name_is_required(self) -> None: + with self.assertRaisesRegex(ValueError, "hook name"): + hpk.hook("") + + +class RegisterPluginTests(unittest.TestCase): + def _module(self, **attrs): + module = types.ModuleType("sample_plugin") + for name, value in attrs.items(): + setattr(module, name, value) + return module + + def test_registers_tools_hooks_and_skills_with_summary(self) -> None: + @hpk.hook("pre_llm_call") + def callback(**kwargs): + return kwargs + + with tempfile.TemporaryDirectory() as tmp: + skill_path = Path(tmp) / "SKILL.md" + skill_path.write_text("# Skill\n") + skill = hpk.plugin_skill( + "temporal-awareness", skill_path, "Use local timing context." + ) + ctx = FakePluginCtx() + module = self._module(callback=callback, sample_read=sample_read) + with self.assertLogs(level="INFO") as cap: + summary = hpk.register_plugin(ctx, module, skills=(skill,)) + + self.assertEqual(summary.tools, ("sample_read_thread",)) + self.assertEqual(summary.hooks, ("pre_llm_call",)) + self.assertEqual(summary.skills, ("temporal-awareness",)) + self.assertEqual(summary.skipped_optional_skills, ()) + self.assertEqual(ctx.hooks, [("pre_llm_call", callback)]) + self.assertEqual(ctx.skills[0]["name"], "temporal-awareness") + self.assertIn("tools=sample_read_thread", "\n".join(cap.output)) + self.assertIn("hooks=pre_llm_call", "\n".join(cap.output)) + self.assertIn("skills=temporal-awareness", "\n".join(cap.output)) + + def test_missing_optional_skill_is_skipped_with_warning(self) -> None: + ctx = FakePluginCtx() + skill = hpk.plugin_skill("optional", "/missing/SKILL.md", "Optional", optional=True) + with self.assertLogs(level="WARNING"): + summary = hpk.register_plugin(ctx, self._module(), skills=(skill,)) + self.assertEqual(summary.skipped_optional_skills, ("optional",)) + self.assertEqual(ctx.skills, []) + + def test_missing_required_skill_raises(self) -> None: + @hpk.hook("pre_llm_call") + def callback(**kwargs): + return kwargs + + ctx = FakePluginCtx() + skill = hpk.plugin_skill("required", "/missing/SKILL.md", "Required") + with self.assertRaises(FileNotFoundError): + hpk.register_plugin( + ctx, + self._module(callback=callback, sample_read=sample_read), + skills=(skill,), + ) + self.assertEqual(ctx.tools, []) + self.assertEqual(ctx.hooks, []) + self.assertEqual(ctx.skills, []) + + def test_validates_skill_name_path_and_description(self) -> None: + for args in [ + ("bad:name", "SKILL.md", "Description"), + ("good", "README.md", "Description"), + ("good", "SKILL.md", ""), + ]: + with self.subTest(args=args), self.assertRaises(ValueError): + hpk.plugin_skill(*args) + + def test_rejects_duplicate_hook_names(self) -> None: + @hpk.hook("pre_llm_call") + def first(**kwargs): + return None + + @hpk.hook("pre_llm_call") + def second(**kwargs): + return None + + with self.assertRaisesRegex(ValueError, "duplicate hook"): + hpk.register_plugin( + FakePluginCtx(), self._module(first=first, second=second) + ) + + def test_rejects_duplicate_tool_names(self) -> None: + @hpk.tool(toolset="sample", name="sample_duplicate") + def first(args, **kwargs): + """First duplicate tool.""" + return {} + + @hpk.tool(toolset="sample", name="sample_duplicate") + def second(args, **kwargs): + """Second duplicate tool.""" + return {} + + ctx = FakePluginCtx() + with self.assertRaisesRegex(ValueError, "duplicate tool"): + hpk.register_plugin(ctx, self._module(first=first, second=second)) + self.assertEqual(ctx.tools, []) + + def test_rejects_duplicate_skill_names(self) -> None: + skills = ( + hpk.plugin_skill("same", "one/SKILL.md", "First", optional=True), + hpk.plugin_skill("same", "two/SKILL.md", "Second", optional=True), + ) + with self.assertRaisesRegex(ValueError, "duplicate skill"): + hpk.register_plugin(FakePluginCtx(), self._module(), skills=skills) + + if __name__ == "__main__": unittest.main()