From dd4efb19d6a56a979d2c655a1fc61c668f616055 Mon Sep 17 00:00:00 2001 From: Offending Commit Date: Tue, 18 Aug 2026 14:48:37 -0500 Subject: [PATCH 1/3] feat(plugins): add forward-compatible references_dir for plugin skills plugin_skill() and PluginSkill now accept an optional references_dir -- a companion directory of reference files sibling to SKILL.md -- validated the same way SKILL.md is (required unless the skill is optional, in which case a missing directory is dropped with a warning instead of raising). register_plugin() forwards references_dir to the host's register_skill only when the host's live signature actually accepts it (probed via inspect.signature, honoring both an explicit references_dir parameter and a **kwargs catch-all), so older hosts are never called with an argument they don't understand. This does not yet make any directory agent-visible: as of this writing, no released Hermes Agent host reads or serves plugin-skill companion files (confirmed by reading hermes-agent's _serve_plugin_skill, which hardcodes linked_files=None and never receives a directory argument). A warning is logged naming the skill so that gap stays visible instead of silently doing nothing. This is groundwork so plugins that already declare references_dir start working with no further kit-side change once a host adds support -- the actual host-side fix is out of scope for this repo and tracked separately. --- README.md | 27 ++++++++ hermes_plugin_kit/__init__.py | 88 ++++++++++++++++++++----- tests/test_kit.py | 121 ++++++++++++++++++++++++++++++++++ 3 files changed, 219 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 02caa9b..8e270b1 100644 --- a/README.md +++ b/README.md @@ -526,6 +526,33 @@ The validator covers Hermes platform, conditional activation, config, blueprint, environment-variable, and credential-file metadata shapes. Runtime activation and setup behavior remain owned by Hermes Agent. +`plugin_skill` also accepts an optional `references_dir` for a companion +directory of reference files sibling to `SKILL.md` (Hermes' own convention +names these `references`, `templates`, `assets`, or `scripts`, but any +directory name is accepted). It is validated the same way `SKILL.md` is — +required unless the skill itself is `optional`, in which case a missing +directory is dropped with a warning instead of raising: + +```python +plugin_skill( + "temporal-awareness", + Path(__file__).with_name("SKILL.md"), + "Calibrate responses against local time and message gaps.", + references_dir=Path(__file__).with_name("references"), +) +``` + +**This is forward-compatible groundwork, not yet an effective capability.** +`register_plugin` only forwards `references_dir` to `ctx.register_skill` when +the host's own signature accepts that parameter (checked at registration +time via `inspect.signature`, so older hosts are never called with an +argument they don't understand). As of this writing, no released Hermes +Agent host reads or serves plugin-skill companion files, so declaring +`references_dir` today does not make the directory agent-visible — a +warning is logged naming the skill so the gap stays visible instead of +silently doing nothing. Once a host adds support, plugins that already +declare `references_dir` start working with no further kit-side change. + ## Subagents and specialized providers Subagent lifecycle supervision is host-owned. Use the checked accessor instead diff --git a/hermes_plugin_kit/__init__.py b/hermes_plugin_kit/__init__.py index aa28f16..4329efe 100644 --- a/hermes_plugin_kit/__init__.py +++ b/hermes_plugin_kit/__init__.py @@ -62,7 +62,7 @@ def register(ctx): import threading import time from contextlib import contextmanager -from dataclasses import dataclass +from dataclasses import dataclass, replace as _dataclass_replace from enum import Enum from pathlib import Path from typing import Any, Callable, Iterable, Iterator, Mapping, Protocol @@ -391,6 +391,7 @@ class PluginSkill: path: Path description: str optional: bool = False + references_dir: Path | None = None @dataclass(frozen=True) @@ -1262,8 +1263,21 @@ def plugin_skill( path: str | Path, description: str, optional: bool = False, + references_dir: str | Path | None = None, ) -> PluginSkill: - """Declare a plugin-owned ``SKILL.md`` for :func:`register_plugin`.""" + """Declare a plugin-owned ``SKILL.md`` for :func:`register_plugin`. + + ``references_dir``, when given, is a directory of companion reference files sibling to + ``SKILL.md`` (Hermes' own convention names these ``references``, ``templates``, ``assets``, + or ``scripts``, but any directory name is accepted here). It is validated and carried on the + returned :class:`PluginSkill`, then passed to the host's ``register_skill`` **only when the + host's own signature accepts it** -- as of this writing, no released ``hermes-agent`` host + surfaces plugin-skill companion files, so declaring ``references_dir`` today does not yet make + the directory agent-visible. It is forward-compatible groundwork: once a host adds support, + plugins that already declare ``references_dir`` start working with no further kit-side change. + Until then, ``register_plugin`` logs a warning naming the skill so the gap stays visible rather + than silently doing nothing. + """ 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) @@ -1271,13 +1285,22 @@ def plugin_skill( raise ValueError("skill path must point to SKILL.md") if not isinstance(description, str) or not description.strip(): raise ValueError("skill description is required") + + resolved_references_dir: Path | None = None + if references_dir is not None: + resolved_references_dir = Path(references_dir) + if not resolved_references_dir.is_dir(): + if not optional: + raise NotADirectoryError(f"references_dir not found or not a directory: {resolved_references_dir}") + resolved_references_dir = None + try: _validate_plugin_skill_file(name, skill_path, description) except FileNotFoundError: if optional: - return PluginSkill(name, skill_path, description.strip(), True) + return PluginSkill(name, skill_path, description.strip(), True, resolved_references_dir) raise FileNotFoundError(f"SKILL.md not found at {skill_path}") - return PluginSkill(name, skill_path, description.strip(), bool(optional)) + return PluginSkill(name, skill_path, description.strip(), bool(optional), resolved_references_dir) def get_subagent_lifecycle(ctx: Any) -> Any: @@ -2218,17 +2241,28 @@ def register_plugin( skill = declared_skills[name] try: _validate_plugin_skill_file(skill.name, skill.path, skill.description) - available_skills.append(skill) - continue except FileNotFoundError: 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) + log.warning( + "hermes_plugin_kit: optional skill missing; name=%s; path=%s", + skill.name, + skill.path, + ) + skipped_skills.append(name) + continue + if skill.references_dir is not None and not skill.references_dir.is_dir(): + if not skill.optional: + raise NotADirectoryError( + f"references_dir not found or not a directory: {skill.references_dir}" + ) + log.warning( + "hermes_plugin_kit: optional skill's references_dir missing; name=%s; references_dir=%s", + skill.name, + skill.references_dir, + ) + skill = _dataclass_replace(skill, references_dir=None) + available_skills.append(skill) required_registrars = { "register_command": slash_commands, @@ -2357,11 +2391,31 @@ def register_plugin( registered_skills: list[str] = [] for skill in available_skills: - ctx.register_skill( - name=skill.name, - path=skill.path, - description=skill.description, - ) + skill_kwargs: dict[str, Any] = { + "name": skill.name, + "path": skill.path, + "description": skill.description, + } + if skill.references_dir is not None: + try: + register_skill_params = inspect.signature(ctx.register_skill).parameters + host_accepts_references_dir = "references_dir" in register_skill_params or any( + parameter.kind is inspect.Parameter.VAR_KEYWORD + for parameter in register_skill_params.values() + ) + except (TypeError, ValueError): + host_accepts_references_dir = False + if host_accepts_references_dir: + skill_kwargs["references_dir"] = skill.references_dir + else: + log.warning( + "hermes_plugin_kit: host register_skill() does not yet accept references_dir; " + "name=%s references_dir=%s will not be surfaced to the agent until the host " + "adds support", + skill.name, + skill.references_dir, + ) + ctx.register_skill(**skill_kwargs) registered_skills.append(skill.name) registered_memory_providers: list[str] = [] diff --git a/tests/test_kit.py b/tests/test_kit.py index 34bd740..8251889 100644 --- a/tests/test_kit.py +++ b/tests/test_kit.py @@ -1935,6 +1935,127 @@ def test_rejects_duplicate_skill_names(self) -> None: with self.assertRaisesRegex(ValueError, "duplicate skill"): hpk.register_plugin(FakePluginCtx(), self._module(), skills=skills) + def test_plugin_skill_accepts_and_validates_references_dir(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + skill_path = Path(tmp) / "SKILL.md" + skill_path.write_text( + "---\nname: sample\ndescription: Sample.\n---\n# Sample\n" + ) + references_dir = Path(tmp) / "references" + references_dir.mkdir() + + skill = hpk.plugin_skill("sample", skill_path, "Sample.", references_dir=references_dir) + + self.assertEqual(skill.references_dir, references_dir) + + def test_plugin_skill_rejects_missing_references_dir_when_required(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + skill_path = Path(tmp) / "SKILL.md" + skill_path.write_text( + "---\nname: sample\ndescription: Sample.\n---\n# Sample\n" + ) + + with self.assertRaises(NotADirectoryError): + hpk.plugin_skill( + "sample", skill_path, "Sample.", references_dir=Path(tmp) / "missing" + ) + + def test_plugin_skill_tolerates_missing_references_dir_when_optional(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + skill_path = Path(tmp) / "SKILL.md" + skill_path.write_text( + "---\nname: sample\ndescription: Sample.\n---\n# Sample\n" + ) + + skill = hpk.plugin_skill( + "sample", + skill_path, + "Sample.", + optional=True, + references_dir=Path(tmp) / "missing", + ) + + self.assertIsNone(skill.references_dir) + + def test_register_plugin_passes_references_dir_to_a_host_that_accepts_it(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + skill_path = Path(tmp) / "SKILL.md" + skill_path.write_text( + "---\nname: sample\ndescription: Sample.\n---\n# Sample\n" + ) + references_dir = Path(tmp) / "references" + references_dir.mkdir() + skill = hpk.plugin_skill("sample", skill_path, "Sample.", references_dir=references_dir) + + ctx = FakePluginCtx() + hpk.register_plugin(ctx, self._module(), skills=(skill,)) + + self.assertEqual(ctx.skills[0]["references_dir"], references_dir) + + def test_register_plugin_falls_back_gracefully_when_host_lacks_references_dir_support( + self, + ) -> None: + class StrictHostCtx(FakePluginCtx): + """Mirrors today's real hermes-agent PluginContext.register_skill signature.""" + + def register_skill(self, name, path, description="") -> None: + self.skills.append({"name": name, "path": path, "description": description}) + + with tempfile.TemporaryDirectory() as tmp: + skill_path = Path(tmp) / "SKILL.md" + skill_path.write_text( + "---\nname: sample\ndescription: Sample.\n---\n# Sample\n" + ) + references_dir = Path(tmp) / "references" + references_dir.mkdir() + skill = hpk.plugin_skill("sample", skill_path, "Sample.", references_dir=references_dir) + + ctx = StrictHostCtx() + with self.assertLogs(level="WARNING") as logs: + hpk.register_plugin(ctx, self._module(), skills=(skill,)) + + self.assertNotIn("references_dir", ctx.skills[0]) + self.assertIn("does not yet accept references_dir", "\n".join(logs.output)) + + def test_register_plugin_drops_references_dir_removed_after_declaration_for_optional_skill( + self, + ) -> None: + with tempfile.TemporaryDirectory() as tmp: + skill_path = Path(tmp) / "SKILL.md" + skill_path.write_text( + "---\nname: sample\ndescription: Sample.\n---\n# Sample\n" + ) + references_dir = Path(tmp) / "references" + references_dir.mkdir() + skill = hpk.plugin_skill( + "sample", skill_path, "Sample.", optional=True, references_dir=references_dir + ) + references_dir.rmdir() + + ctx = FakePluginCtx() + with self.assertLogs(level="WARNING") as logs: + summary = hpk.register_plugin(ctx, self._module(), skills=(skill,)) + + self.assertEqual(summary.skills, ("sample",)) + self.assertNotIn("references_dir", ctx.skills[0]) + self.assertIn("references_dir missing", "\n".join(logs.output)) + + def test_register_plugin_raises_when_required_references_dir_removed_after_declaration( + self, + ) -> None: + with tempfile.TemporaryDirectory() as tmp: + skill_path = Path(tmp) / "SKILL.md" + skill_path.write_text( + "---\nname: sample\ndescription: Sample.\n---\n# Sample\n" + ) + references_dir = Path(tmp) / "references" + references_dir.mkdir() + skill = hpk.plugin_skill("sample", skill_path, "Sample.", references_dir=references_dir) + references_dir.rmdir() + + with self.assertRaises(NotADirectoryError): + hpk.register_plugin(FakePluginCtx(), self._module(), skills=(skill,)) + if __name__ == "__main__": unittest.main() From d6bbdb99851302637faffcd45b3e5889671292e6 Mon Sep 17 00:00:00 2001 From: Offending Commit Date: Tue, 18 Aug 2026 14:59:17 -0500 Subject: [PATCH 2/3] fix(plugins): close references_dir probe gaps found in review The inspect.signature() capability probe alone is unsound: a bare Mock(spec=...) test double or a decorator applied without functools.wraps both present a (*args, **kwargs) shape that the probe reads as "host accepts references_dir" even when the real host doesn't -- and the actual ctx.register_skill(**kwargs) call had no guard, so a wrong guess crashed registration entirely. register_plugin now retries once without references_dir on a TypeError naming it; Python raises that error at argument binding, before the callee's body runs, so retrying is safe even against a host with side effects. Also fixes a real correctness bug: plugin_skill() was resolving an optional skill's missing references_dir to None before register_plugin ever saw it, so the "dropped with a warning" behavior documented in the README never actually logged anything for the common case (a references_dir that doesn't exist yet at declare time). plugin_skill() now only validates references_dir when the skill is required, mirroring exactly how SKILL.md's own optional handling already works, so register_plugin's existing re-check is the single place that warns and drops it. Extracts _signature_accepts_kwarg as a shared, named probe (previously inlined once for this feature and duplicated in concept from the existing _call_session_db_evolving pattern), updates skills/hermes-plugins/references/plugin-kit.md and README.md per this repo's own doc-sync rule, and adds 6 more tests covering the probe's except-branch, an explicit-named-parameter host, the retry-on-rejection safety net, and an unrelated-TypeError passthrough. --- README.md | 28 ++-- hermes_plugin_kit/__init__.py | 97 +++++++++---- .../hermes-plugins/references/plugin-kit.md | 2 +- tests/test_kit.py | 134 +++++++++++++++++- 4 files changed, 219 insertions(+), 42 deletions(-) diff --git a/README.md b/README.md index 8e270b1..0f06a6f 100644 --- a/README.md +++ b/README.md @@ -529,9 +529,14 @@ activation and setup behavior remain owned by Hermes Agent. `plugin_skill` also accepts an optional `references_dir` for a companion directory of reference files sibling to `SKILL.md` (Hermes' own convention names these `references`, `templates`, `assets`, or `scripts`, but any -directory name is accepted). It is validated the same way `SKILL.md` is — -required unless the skill itself is `optional`, in which case a missing -directory is dropped with a warning instead of raising: +directory name is accepted). It follows the same required/optional split as +`SKILL.md`: a required (non-`optional`) skill whose `references_dir` is +missing raises `NotADirectoryError` immediately from `plugin_skill`, the +directory counterpart to `SKILL.md`'s own `FileNotFoundError`. An `optional` +skill's `references_dir` isn't checked at declaration time at all — it's +carried as declared and checked once, during `register_plugin`, which logs a +warning and drops it if still missing (checking it twice would silence that +warning the second time): ```python plugin_skill( @@ -546,12 +551,17 @@ plugin_skill( `register_plugin` only forwards `references_dir` to `ctx.register_skill` when the host's own signature accepts that parameter (checked at registration time via `inspect.signature`, so older hosts are never called with an -argument they don't understand). As of this writing, no released Hermes -Agent host reads or serves plugin-skill companion files, so declaring -`references_dir` today does not make the directory agent-visible — a -warning is logged naming the skill so the gap stays visible instead of -silently doing nothing. Once a host adds support, plugins that already -declare `references_dir` start working with no further kit-side change. +argument they don't understand). That check is a best-effort probe, not a +guarantee — a host reached through a generic `**kwargs` shape (a decorator +applied without `functools.wraps`, or a test double built with a bare +`Mock(spec=...)` instead of `create_autospec(...)`) can report acceptance it +doesn't actually have, so `register_plugin` also retries once without +`references_dir` if the host rejects it at call time, logging a warning +either way. As of this writing, no released Hermes Agent host reads or +serves plugin-skill companion files, so declaring `references_dir` today +does not make the directory agent-visible. Once a host adds support, plugins +that already declare `references_dir` start working with no further +kit-side change. ## Subagents and specialized providers diff --git a/hermes_plugin_kit/__init__.py b/hermes_plugin_kit/__init__.py index 4329efe..dbf0f80 100644 --- a/hermes_plugin_kit/__init__.py +++ b/hermes_plugin_kit/__init__.py @@ -192,6 +192,26 @@ def _call_session_db( return method(*args, **kwargs) +def _signature_accepts_kwarg(callable_obj: Any, name: str) -> bool: + """Return whether ``callable_obj``'s live signature has ``name`` or accepts ``**kwargs``. + + Shared probe for "evolving host API" call sites (see :func:`_call_session_db_evolving` for + the sibling pattern with multi-kwarg, fail-loud semantics). This variant is single-kwarg and + fails soft (returns ``False``) when the signature can't be introspected -- callers that want + graceful degradation should always pair this with a try/except around the actual call, since + a permissive ``**kwargs`` shape (a bare ``Mock(spec=...)``, or a decorator without + ``functools.wraps``) can make this probe report ``True`` for a host that will still reject the + keyword argument at call time. + """ + try: + parameters = inspect.signature(callable_obj).parameters + except (TypeError, ValueError): + return False + return name in parameters or any( + parameter.kind is inspect.Parameter.VAR_KEYWORD for parameter in parameters.values() + ) + + def _call_session_db_evolving( db: Any, method_name: str, @@ -1269,11 +1289,19 @@ def plugin_skill( ``references_dir``, when given, is a directory of companion reference files sibling to ``SKILL.md`` (Hermes' own convention names these ``references``, ``templates``, ``assets``, - or ``scripts``, but any directory name is accepted here). It is validated and carried on the - returned :class:`PluginSkill`, then passed to the host's ``register_skill`` **only when the - host's own signature accepts it** -- as of this writing, no released ``hermes-agent`` host - surfaces plugin-skill companion files, so declaring ``references_dir`` today does not yet make - the directory agent-visible. It is forward-compatible groundwork: once a host adds support, + or ``scripts``, but any directory name is accepted here). A required (non-``optional``) skill + whose ``references_dir`` does not exist raises :class:`NotADirectoryError` immediately, the + same way a required, missing ``SKILL.md`` raises :class:`FileNotFoundError`. An ``optional`` + skill's ``references_dir`` is *not* validated here -- it is carried on the returned + :class:`PluginSkill` as declared and re-checked once, by :func:`register_plugin`, which logs a + warning and drops it if still missing at registration time. (Validating and dropping it here + too would silence that warning permanently, since :func:`register_plugin`'s re-check only + fires when the field is still non-``None``.) + + ``references_dir`` is passed to the host's ``register_skill`` **only when the host's own + signature accepts it** -- as of this writing, no released ``hermes-agent`` host surfaces + plugin-skill companion files, so declaring ``references_dir`` today does not yet make the + directory agent-visible. It is forward-compatible groundwork: once a host adds support, plugins that already declare ``references_dir`` start working with no further kit-side change. Until then, ``register_plugin`` logs a warning naming the skill so the gap stays visible rather than silently doing nothing. @@ -1289,10 +1317,8 @@ def plugin_skill( resolved_references_dir: Path | None = None if references_dir is not None: resolved_references_dir = Path(references_dir) - if not resolved_references_dir.is_dir(): - if not optional: - raise NotADirectoryError(f"references_dir not found or not a directory: {resolved_references_dir}") - resolved_references_dir = None + if not optional and not resolved_references_dir.is_dir(): + raise NotADirectoryError(f"references_dir not found or not a directory: {resolved_references_dir}") try: _validate_plugin_skill_file(name, skill_path, description) @@ -2396,26 +2422,39 @@ def register_plugin( "path": skill.path, "description": skill.description, } - if skill.references_dir is not None: - try: - register_skill_params = inspect.signature(ctx.register_skill).parameters - host_accepts_references_dir = "references_dir" in register_skill_params or any( - parameter.kind is inspect.Parameter.VAR_KEYWORD - for parameter in register_skill_params.values() - ) - except (TypeError, ValueError): - host_accepts_references_dir = False - if host_accepts_references_dir: - skill_kwargs["references_dir"] = skill.references_dir - else: - log.warning( - "hermes_plugin_kit: host register_skill() does not yet accept references_dir; " - "name=%s references_dir=%s will not be surfaced to the agent until the host " - "adds support", - skill.name, - skill.references_dir, - ) - ctx.register_skill(**skill_kwargs) + wants_references_dir = skill.references_dir is not None + if wants_references_dir and _signature_accepts_kwarg(ctx.register_skill, "references_dir"): + skill_kwargs["references_dir"] = skill.references_dir + elif wants_references_dir: + log.warning( + "hermes_plugin_kit: host register_skill() does not yet accept references_dir; " + "name=%s references_dir=%s will not be surfaced to the agent until the host " + "adds support", + skill.name, + skill.references_dir, + ) + + # The signature probe above can be fooled by a permissive **kwargs shape (a bare + # Mock(spec=...) test double, or a decorator applied without functools.wraps) that + # reports acceptance the underlying host doesn't actually have. Retry once without + # references_dir on a TypeError naming it -- Python raises that error at argument + # binding, before the callee's body runs, so retrying is safe even if the host has + # side effects: the first, rejected call never entered the function. + try: + ctx.register_skill(**skill_kwargs) + except TypeError as exc: + if "references_dir" not in skill_kwargs or "references_dir" not in str(exc): + raise + log.warning( + "hermes_plugin_kit: host register_skill() reported it accepts references_dir " + "but rejected it at call time; name=%s references_dir=%s will not be surfaced " + "to the agent; error=%s", + skill.name, + skill.references_dir, + exc, + ) + del skill_kwargs["references_dir"] + ctx.register_skill(**skill_kwargs) registered_skills.append(skill.name) registered_memory_providers: list[str] = [] diff --git a/skills/hermes-plugins/references/plugin-kit.md b/skills/hermes-plugins/references/plugin-kit.md index 74955c1..d678b43 100644 --- a/skills/hermes-plugins/references/plugin-kit.md +++ b/skills/hermes-plugins/references/plugin-kit.md @@ -27,7 +27,7 @@ guidance, not a second implementation specification. | Session slash command | `@command` | `register_plugin` | Name is bare lowercase kebab-case; handler receives raw trailing text and may be sync or async. | | Request or execution middleware | `@middleware`, `MiddlewareKind` | `register_plugin` | Callback is synchronous; request phases replace payloads, execution phases call single-use `next_call`. | | Lifecycle hook | `@hook` | `register_plugin` | Hermes kwargs and return values pass through; exceptions are re-raised for Hermes isolation. | -| Plugin-owned skill | `plugin_skill` | `register_plugin(..., skills=...)` | Hermes adds the plugin namespace; missing required skills fail, optional skills warn and skip. | +| Plugin-owned skill | `plugin_skill` | `register_plugin(..., skills=...)` | Hermes adds the plugin namespace; missing required skills fail, optional skills warn and skip. Optional `references_dir` (a companion reference-files directory) is forward-compatible groundwork only — no released host surfaces it yet; see [`README.md`](../../../README.md#commands-middleware-hooks-and-plugin-skills). | | Context engine | Hermes `ContextEngine` instance | `register_plugin(..., context_engine=...)` | Singular native engine registration; schemas and recovery dispatch stay in `get_tool_schemas()` / `handle_tool_call()`, never duplicated with `@tool`. | | Host-managed call | `invoke_host_tool` | None | Use for supported non-registry capabilities such as `send_message`; pre/post-tool hooks remain active. | | Local media delivery | `MediaPayload`, `MediaType`, `deliver_media` | Consumer registers suppression hooks | File must be absolute, present, and non-empty; `origin` resolves from task-local Hermes context. | diff --git a/tests/test_kit.py b/tests/test_kit.py index 8251889..5b3a075 100644 --- a/tests/test_kit.py +++ b/tests/test_kit.py @@ -1960,22 +1960,47 @@ def test_plugin_skill_rejects_missing_references_dir_when_required(self) -> None "sample", skill_path, "Sample.", references_dir=Path(tmp) / "missing" ) - def test_plugin_skill_tolerates_missing_references_dir_when_optional(self) -> None: + def test_plugin_skill_does_not_validate_references_dir_when_optional(self) -> None: + # Mirrors SKILL.md's own optional handling: plugin_skill() does not resolve or drop a + # missing optional references_dir -- it's carried as declared so register_plugin()'s + # later re-check (the only place that logs a warning) still sees it as non-None and + # actually fires. Validating here too would silence that warning permanently. with tempfile.TemporaryDirectory() as tmp: skill_path = Path(tmp) / "SKILL.md" skill_path.write_text( "---\nname: sample\ndescription: Sample.\n---\n# Sample\n" ) + missing = Path(tmp) / "missing" + skill = hpk.plugin_skill( + "sample", skill_path, "Sample.", optional=True, references_dir=missing + ) + + self.assertEqual(skill.references_dir, missing) + + def test_register_plugin_warns_and_drops_references_dir_missing_since_declaration( + self, + ) -> None: + with tempfile.TemporaryDirectory() as tmp: + skill_path = Path(tmp) / "SKILL.md" + skill_path.write_text( + "---\nname: sample\ndescription: Sample.\n---\n# Sample\n" + ) skill = hpk.plugin_skill( "sample", skill_path, "Sample.", optional=True, - references_dir=Path(tmp) / "missing", + references_dir=Path(tmp) / "never-created", ) - self.assertIsNone(skill.references_dir) + ctx = FakePluginCtx() + with self.assertLogs(level="WARNING") as logs: + summary = hpk.register_plugin(ctx, self._module(), skills=(skill,)) + + self.assertEqual(summary.skills, ("sample",)) + self.assertNotIn("references_dir", ctx.skills[0]) + self.assertIn("references_dir missing", "\n".join(logs.output)) def test_register_plugin_passes_references_dir_to_a_host_that_accepts_it(self) -> None: with tempfile.TemporaryDirectory() as tmp: @@ -2056,6 +2081,109 @@ def test_register_plugin_raises_when_required_references_dir_removed_after_decla with self.assertRaises(NotADirectoryError): hpk.register_plugin(FakePluginCtx(), self._module(), skills=(skill,)) + def test_register_plugin_treats_uninspectable_register_skill_as_unsupported(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + skill_path = Path(tmp) / "SKILL.md" + skill_path.write_text( + "---\nname: sample\ndescription: Sample.\n---\n# Sample\n" + ) + references_dir = Path(tmp) / "references" + references_dir.mkdir() + skill = hpk.plugin_skill("sample", skill_path, "Sample.", references_dir=references_dir) + + ctx = FakePluginCtx() + with ( + patch.object(hpk, "inspect") as fake_inspect, + self.assertLogs(level="WARNING") as logs, + ): + fake_inspect.signature.side_effect = ValueError("no signature found") + fake_inspect.Parameter = inspect.Parameter + summary = hpk.register_plugin(ctx, self._module(), skills=(skill,)) + + self.assertEqual(summary.skills, ("sample",)) + self.assertNotIn("references_dir", ctx.skills[0]) + self.assertIn("does not yet accept references_dir", "\n".join(logs.output)) + + def test_register_plugin_passes_references_dir_to_a_host_with_an_explicit_parameter( + self, + ) -> None: + class ExplicitParamHostCtx(FakePluginCtx): + """A host whose register_skill names references_dir directly -- no **kwargs.""" + + def register_skill(self, name, path, description="", references_dir=None) -> None: + self.skills.append( + { + "name": name, + "path": path, + "description": description, + "references_dir": references_dir, + } + ) + + with tempfile.TemporaryDirectory() as tmp: + skill_path = Path(tmp) / "SKILL.md" + skill_path.write_text( + "---\nname: sample\ndescription: Sample.\n---\n# Sample\n" + ) + references_dir = Path(tmp) / "references" + references_dir.mkdir() + skill = hpk.plugin_skill("sample", skill_path, "Sample.", references_dir=references_dir) + + ctx = ExplicitParamHostCtx() + hpk.register_plugin(ctx, self._module(), skills=(skill,)) + + self.assertEqual(ctx.skills[0]["references_dir"], references_dir) + + def test_register_plugin_retries_without_references_dir_when_host_rejects_it_at_call_time( + self, + ) -> None: + # Simulates the probe being fooled: a **kwargs-shaped register_skill (a bare + # Mock(spec=...) or a decorator applied without functools.wraps produces this exact + # shape in practice) reports acceptance via inspect.signature, but the real + # implementation still raises TypeError for the unexpected keyword at call time. + class FooledProbeHostCtx(FakePluginCtx): + def register_skill(self, **kwargs) -> None: + if "references_dir" in kwargs: + raise TypeError( + "register_skill() got an unexpected keyword argument 'references_dir'" + ) + self.skills.append(kwargs) + + with tempfile.TemporaryDirectory() as tmp: + skill_path = Path(tmp) / "SKILL.md" + skill_path.write_text( + "---\nname: sample\ndescription: Sample.\n---\n# Sample\n" + ) + references_dir = Path(tmp) / "references" + references_dir.mkdir() + skill = hpk.plugin_skill("sample", skill_path, "Sample.", references_dir=references_dir) + + ctx = FooledProbeHostCtx() + with self.assertLogs(level="WARNING") as logs: + summary = hpk.register_plugin(ctx, self._module(), skills=(skill,)) + + self.assertEqual(summary.skills, ("sample",)) + self.assertEqual(len(ctx.skills), 1) + self.assertNotIn("references_dir", ctx.skills[0]) + self.assertIn("rejected it at call time", "\n".join(logs.output)) + + def test_register_plugin_reraises_unrelated_type_errors_from_register_skill(self) -> None: + class BrokenHostCtx(FakePluginCtx): + def register_skill(self, **kwargs) -> None: + raise TypeError("register_skill() missing 1 required positional argument") + + with tempfile.TemporaryDirectory() as tmp: + skill_path = Path(tmp) / "SKILL.md" + skill_path.write_text( + "---\nname: sample\ndescription: Sample.\n---\n# Sample\n" + ) + references_dir = Path(tmp) / "references" + references_dir.mkdir() + skill = hpk.plugin_skill("sample", skill_path, "Sample.", references_dir=references_dir) + + with self.assertRaises(TypeError): + hpk.register_plugin(BrokenHostCtx(), self._module(), skills=(skill,)) + if __name__ == "__main__": unittest.main() From d3aef1ddf70452fd96dfe3f67f20e55750149b91 Mon Sep 17 00:00:00 2001 From: Offending Commit Date: Tue, 18 Aug 2026 15:24:23 -0500 Subject: [PATCH 3/3] feat(plugins): add plugin_reference_tool for agent-visible skill references plugin_reference_tool(skill, *, toolset, name=None, description=None) builds an ordinary @tool function that lists or reads files under a skill's references_dir, sidestepping register_skill/skill_view entirely -- the agent can reach the directory's contents today, on any host, via a normal tool call, without waiting on hermes-agent to add companion-file support. Review surfaced two real bugs before either landed silently: - references_dir accepted any directory with no requirement that it live near the skill; pointing it at /etc (or a mounted secrets volume) turned this into an arbitrary-file-read tool using only the documented public API. plugin_skill now requires references_dir to resolve as a descendant of the skill's own directory, and plugin_reference_tool re-checks the same constraint defensively in case PluginSkill is constructed directly, bypassing that factory. - The default tool-name derivation only swapped hyphens for underscores, so any skill name plugin_skill legally accepts but containing an uppercase letter or a leading digit (e.g. "Sample-Skill", "2fa-setup") produced an invalid tool name and crashed with ValueError. Fixed with a proper normalize-and-guard helper. Also rejects non-string file_path with a clean error instead of leaking a raw TypeError, and adds test coverage for the containment fix, the naming fix, absolute-path/non-string/empty-directory/ directory-as-file_path edge cases, and registering the tool through register_plugin like any other capability. --- README.md | 48 ++++- hermes_plugin_kit/__init__.py | 119 +++++++++++ .../hermes-plugins/references/plugin-kit.md | 1 + tests/test_kit.py | 200 ++++++++++++++++++ 4 files changed, 360 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 0f06a6f..89b1ece 100644 --- a/README.md +++ b/README.md @@ -529,14 +529,19 @@ activation and setup behavior remain owned by Hermes Agent. `plugin_skill` also accepts an optional `references_dir` for a companion directory of reference files sibling to `SKILL.md` (Hermes' own convention names these `references`, `templates`, `assets`, or `scripts`, but any -directory name is accepted). It follows the same required/optional split as -`SKILL.md`: a required (non-`optional`) skill whose `references_dir` is -missing raises `NotADirectoryError` immediately from `plugin_skill`, the -directory counterpart to `SKILL.md`'s own `FileNotFoundError`. An `optional` -skill's `references_dir` isn't checked at declaration time at all — it's -carried as declared and checked once, during `register_plugin`, which logs a -warning and drops it if still missing (checking it twice would silence that -warning the second time): +directory name is accepted) — as long as it's actually inside the skill's +own directory tree (any depth of subdirectory is fine; a path outside it, +including via a symlink, raises `ValueError` immediately, regardless of +`optional`). Without that check, `references_dir` would accept literally any +directory, and `plugin_reference_tool` (below) would then expose that entire +tree for reading. Existence, in contrast, follows the same required/optional +split as `SKILL.md`: a required (non-`optional`) skill whose `references_dir` +doesn't exist raises `NotADirectoryError` immediately from `plugin_skill`, +the directory counterpart to `SKILL.md`'s own `FileNotFoundError`. An +`optional` skill's `references_dir` existence isn't checked at declaration +time at all — it's carried as declared and checked once, during +`register_plugin`, which logs a warning and drops it if still missing +(checking it twice would silence that warning the second time): ```python plugin_skill( @@ -563,6 +568,33 @@ does not make the directory agent-visible. Once a host adds support, plugins that already declare `references_dir` start working with no further kit-side change. +**`plugin_reference_tool` makes the directory agent-visible today, without +waiting on a host.** It builds an ordinary `@tool`-decorated function — no +`register_skill` involvement at all — that lists or reads files under a +skill's `references_dir`: + +```python +reader = plugin_reference_tool(skill, toolset="temporal-awareness") +# tool name defaults to "_read_reference"; pass name=/description= +# to override either. Include `reader` in the plugin's own declarations +# passed to register_plugin, exactly like any other @tool function. +``` + +Called with no `file_path`, it returns every file under `references_dir` +(recursively, as relative POSIX paths). Called with `file_path` set to a +path relative to `references_dir`, it returns that file's content. A +`file_path` that resolves outside `references_dir` — including through a +symlink, an absolute path, or a `../` chain — is rejected rather than +followed; a non-string `file_path` is rejected with a clean error instead of +an internal exception. `plugin_reference_tool` re-checks that +`references_dir` is scoped inside the skill's own directory even though +`plugin_skill` already enforces it, since `PluginSkill` is a public +dataclass a caller could construct directly, bypassing that factory. It is +not TOCTOU-safe against a `references_dir` writable by an untrusted process +at runtime — fine for the common case of a static directory shipped with +the plugin, insufficient if that assumption doesn't hold for a given +deployment. + ## Subagents and specialized providers Subagent lifecycle supervision is host-owned. Use the checked accessor instead diff --git a/hermes_plugin_kit/__init__.py b/hermes_plugin_kit/__init__.py index dbf0f80..a23559d 100644 --- a/hermes_plugin_kit/__init__.py +++ b/hermes_plugin_kit/__init__.py @@ -1278,6 +1278,26 @@ def _validate_plugin_skill_file( _validate_hermes_skill_metadata(frontmatter) +def _require_references_dir_within_skill(skill_path: Path, references_dir: Path) -> None: + """Reject a references_dir that isn't actually scoped to the skill's own directory. + + references_dir accepts any directory by name -- without this check, a plugin author (by + mistake or otherwise) could point it at an unrelated, much larger directory (a home directory, + a mounted secrets volume, `/etc`), and plugin_reference_tool would then expose that entire + tree for reading, since its own path-traversal check only proves containment within whatever + directory it was handed, not that the directory itself is a legitimate skill companion folder. + Does not require either path to exist -- Path.resolve() normalizes non-existent paths too. + """ + skill_dir = skill_path.resolve().parent + try: + references_dir.resolve().relative_to(skill_dir) + except ValueError: + raise ValueError( + f"references_dir must be inside the skill's own directory ({skill_dir}); " + f"got {references_dir}" + ) from None + + def plugin_skill( name: str, path: str | Path, @@ -1317,6 +1337,7 @@ def plugin_skill( resolved_references_dir: Path | None = None if references_dir is not None: resolved_references_dir = Path(references_dir) + _require_references_dir_within_skill(skill_path, resolved_references_dir) if not optional and not resolved_references_dir.is_dir(): raise NotADirectoryError(f"references_dir not found or not a directory: {resolved_references_dir}") @@ -1329,6 +1350,104 @@ def plugin_skill( return PluginSkill(name, skill_path, description.strip(), bool(optional), resolved_references_dir) +def _default_reference_tool_name(skill_name: str) -> str: + """Derive a tool name that satisfies _TOOL_NAME_RE from any valid skill name. + + plugin_skill()'s own _SKILL_NAME_RE (``[a-zA-Z0-9_-]+``) is looser than the tool-name pattern + (``[a-z][a-z0-9_]*``) -- it permits uppercase letters and a leading digit that a bare + hyphen-to-underscore substitution would carry straight into an invalid tool name. + """ + normalized = re.sub(r"[^a-z0-9_]", "_", skill_name.lower()) + if not normalized or not normalized[0].isalpha(): + normalized = f"skill_{normalized}" + return f"{normalized}_read_reference" + + +def plugin_reference_tool( + skill: PluginSkill, + *, + toolset: str, + name: str | None = None, + description: str | None = None, +) -> Callable: + """Build a ``@tool``-decorated handler that lists or reads ``skill.references_dir``. + + Companion-file support in ``register_skill`` (see ``references_dir`` on :func:`plugin_skill`) + is forward-compatible groundwork only -- no released Hermes Agent host serves those files to + the agent yet. This factory sidesteps that gap entirely: it builds an ordinary tool, using the + same ``@tool``/``register_plugin`` mechanism every other plugin capability already goes + through, so the agent can read the directory's contents today on any host, regardless of + ``register_skill`` support. + + Call with no arguments (or ``file_path=None``) to list every file under ``references_dir``. + Call with ``file_path`` set to a path relative to ``references_dir`` to read that file's + content; a path that resolves outside ``references_dir`` (including via a symlink) is + rejected. + + Returns a ready-to-register tool function -- include it in the plugin's own declarations + passed to ``register_plugin``. Raises :class:`ValueError` immediately if ``skill`` has no + ``references_dir``, or if that directory isn't actually scoped inside the skill's own + directory (re-checked here even though :func:`plugin_skill` already enforces it, since + :class:`PluginSkill` is a public dataclass a caller could construct directly, bypassing that + factory). + + Reads are not TOCTOU-safe against a filesystem with concurrent writers: the containment check + and the eventual read are separate syscalls against the same path string, not a held file + descriptor. Fine for the common case (a static, developer-authored directory shipped with the + plugin); if ``references_dir`` can be written to by an untrusted process at runtime, this + factory is not sufficient on its own. + """ + if skill.references_dir is None: + raise ValueError(f"plugin_reference_tool requires a references_dir on skill {skill.name!r}") + _require_references_dir_within_skill(skill.path, skill.references_dir) + + references_dir = skill.references_dir + tool_name = name or _default_reference_tool_name(skill.name) + tool_description = description or ( + f"List or read companion reference files for the {skill.name!r} skill. Omit file_path " + "to list every available file; pass file_path to read one file's content." + ) + + def _read_plugin_reference(args: dict, **_: Any) -> dict: + file_path = args.get("file_path") + resolved_root = references_dir.resolve() + if not file_path: + files = sorted( + candidate.relative_to(resolved_root).as_posix() + for candidate in resolved_root.rglob("*") + if candidate.is_file() + ) + return {"references_dir": str(references_dir), "files": files} + + if not isinstance(file_path, str): + raise TypeError(f"file_path must be a string, got {type(file_path).__name__}") + + candidate = (references_dir / file_path).resolve() + try: + candidate.relative_to(resolved_root) + except ValueError: + raise ValueError(f"file_path {file_path!r} escapes references_dir") from None + if not candidate.is_file(): + raise FileNotFoundError(f"file_path {file_path!r} not found under references_dir") + return {"file_path": file_path, "content": candidate.read_text(encoding="utf-8")} + + return tool( + toolset=toolset, + name=tool_name, + description=tool_description, + params={ + "file_path": { + "type": "string", + "description": ( + "Relative path within the skill's references directory. Omit to list " + "available files." + ), + }, + }, + validate_required=False, + )(_read_plugin_reference) + + def get_subagent_lifecycle(ctx: Any) -> Any: """Return Hermes' public subagent lifecycle service after contract checking.""" service = getattr(ctx, "subagent_lifecycle", None) diff --git a/skills/hermes-plugins/references/plugin-kit.md b/skills/hermes-plugins/references/plugin-kit.md index d678b43..98a4206 100644 --- a/skills/hermes-plugins/references/plugin-kit.md +++ b/skills/hermes-plugins/references/plugin-kit.md @@ -28,6 +28,7 @@ guidance, not a second implementation specification. | Request or execution middleware | `@middleware`, `MiddlewareKind` | `register_plugin` | Callback is synchronous; request phases replace payloads, execution phases call single-use `next_call`. | | Lifecycle hook | `@hook` | `register_plugin` | Hermes kwargs and return values pass through; exceptions are re-raised for Hermes isolation. | | Plugin-owned skill | `plugin_skill` | `register_plugin(..., skills=...)` | Hermes adds the plugin namespace; missing required skills fail, optional skills warn and skip. Optional `references_dir` (a companion reference-files directory) is forward-compatible groundwork only — no released host surfaces it yet; see [`README.md`](../../../README.md#commands-middleware-hooks-and-plugin-skills). | +| Agent-visible skill reference files (today) | `plugin_reference_tool` | `register_plugin` (its return value is an ordinary `@tool` function — add it to the plugin's declarations) | Sidesteps `register_skill` entirely by exposing `references_dir` as a normal tool call; rejects any `file_path` that resolves outside `references_dir` (symlink, absolute path, or `../`) and any non-string `file_path`. `references_dir` itself must be inside the skill's own directory — enforced by `plugin_skill` and re-checked here. | | Context engine | Hermes `ContextEngine` instance | `register_plugin(..., context_engine=...)` | Singular native engine registration; schemas and recovery dispatch stay in `get_tool_schemas()` / `handle_tool_call()`, never duplicated with `@tool`. | | Host-managed call | `invoke_host_tool` | None | Use for supported non-registry capabilities such as `send_message`; pre/post-tool hooks remain active. | | Local media delivery | `MediaPayload`, `MediaType`, `deliver_media` | Consumer registers suppression hooks | File must be absolute, present, and non-empty; `origin` resolves from task-local Hermes context. | diff --git a/tests/test_kit.py b/tests/test_kit.py index 5b3a075..ade52ec 100644 --- a/tests/test_kit.py +++ b/tests/test_kit.py @@ -2185,5 +2185,205 @@ def register_skill(self, **kwargs) -> None: hpk.register_plugin(BrokenHostCtx(), self._module(), skills=(skill,)) +class PluginReferenceToolTests(unittest.TestCase): + def _skill_with_references(self, tmp: str, **files: str) -> hpk.PluginSkill: + skill_path = Path(tmp) / "SKILL.md" + skill_path.write_text("---\nname: sample\ndescription: Sample.\n---\n# Sample\n") + references_dir = Path(tmp) / "references" + references_dir.mkdir() + for relative, content in files.items(): + target = references_dir / relative + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(content) + return hpk.plugin_skill("sample", skill_path, "Sample.", references_dir=references_dir) + + def test_requires_a_references_dir_on_the_skill(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + skill_path = Path(tmp) / "SKILL.md" + skill_path.write_text("---\nname: sample\ndescription: Sample.\n---\n# Sample\n") + skill = hpk.plugin_skill("sample", skill_path, "Sample.") + + with self.assertRaisesRegex(ValueError, "requires a references_dir"): + hpk.plugin_reference_tool(skill, toolset="sample") + + def test_plugin_skill_rejects_a_references_dir_outside_the_skills_own_directory(self) -> None: + with tempfile.TemporaryDirectory() as skill_tmp, tempfile.TemporaryDirectory() as outside_tmp: + skill_path = Path(skill_tmp) / "SKILL.md" + skill_path.write_text("---\nname: sample\ndescription: Sample.\n---\n# Sample\n") + + with self.assertRaisesRegex(ValueError, "inside the skill's own directory"): + hpk.plugin_skill("sample", skill_path, "Sample.", references_dir=Path(outside_tmp)) + + def test_plugin_reference_tool_rejects_out_of_scope_references_dir_from_direct_construction( + self, + ) -> None: + # plugin_skill() already rejects this; PluginSkill is a public dataclass a caller could + # construct directly, bypassing that factory -- plugin_reference_tool() must not trust it. + with tempfile.TemporaryDirectory() as skill_tmp, tempfile.TemporaryDirectory() as outside_tmp: + skill_path = Path(skill_tmp) / "SKILL.md" + skill_path.write_text("---\nname: sample\ndescription: Sample.\n---\n# Sample\n") + skill = hpk.PluginSkill("sample", skill_path, "Sample.", False, Path(outside_tmp)) + + with self.assertRaisesRegex(ValueError, "inside the skill's own directory"): + hpk.plugin_reference_tool(skill, toolset="sample") + + def test_derives_a_valid_tool_name_from_a_skill_name_with_uppercase_and_leading_digit( + self, + ) -> None: + for skill_name in ("Sample-Skill", "2fa-setup"): + with self.subTest(skill_name=skill_name), tempfile.TemporaryDirectory() as tmp: + skill_path = Path(tmp) / "SKILL.md" + skill_path.write_text( + f"---\nname: {skill_name}\ndescription: Sample.\n---\n# Sample\n" + ) + references_dir = Path(tmp) / "references" + references_dir.mkdir() + skill = hpk.plugin_skill( + skill_name, skill_path, "Sample.", references_dir=references_dir + ) + + reader = hpk.plugin_reference_tool(skill, toolset="sample") + ctx = FakePluginCtx() + hpk.register_plugin(ctx, self._module(reader=reader), skills=(skill,)) + + self.assertEqual(1, len(ctx.tools)) + self.assertRegex(ctx.tools[0]["name"], r"^[a-z][a-z0-9_]*$") + + def test_rejects_an_absolute_file_path(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + secret = Path(tmp) / "secret.txt" + secret.write_text("do not read me") + skill = self._skill_with_references(tmp, **{"a.md": "hi"}) + reader = hpk.plugin_reference_tool(skill, toolset="sample") + + payload = json.loads(reader({"file_path": str(secret)})) + + self.assertFalse(payload["success"]) + self.assertIn("escapes references_dir", payload["error"]) + + def test_rejects_a_non_string_file_path_with_a_clean_error(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + skill = self._skill_with_references(tmp, **{"a.md": "hi"}) + reader = hpk.plugin_reference_tool(skill, toolset="sample") + + for bad_file_path in (5, ["a.md"], {"x": 1}): + with self.subTest(file_path=bad_file_path): + payload = json.loads(reader({"file_path": bad_file_path})) + self.assertFalse(payload["success"]) + self.assertIn("file_path must be a string", payload["error"]) + + def test_lists_an_empty_directory_cleanly(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + skill = self._skill_with_references(tmp) + reader = hpk.plugin_reference_tool(skill, toolset="sample") + + payload = json.loads(reader({})) + + self.assertTrue(payload["success"]) + self.assertEqual([], payload["data"]["files"]) + + def test_rejects_a_directory_passed_as_file_path(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + skill = self._skill_with_references(tmp, **{"nested/a.md": "hi"}) + reader = hpk.plugin_reference_tool(skill, toolset="sample") + + payload = json.loads(reader({"file_path": "nested"})) + + self.assertFalse(payload["success"]) + self.assertIn("not found", payload["error"]) + + def test_derives_a_default_name_and_lists_files_when_file_path_omitted(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + skill = self._skill_with_references( + tmp, **{"a.md": "A", "nested/b.md": "B"} + ) + reader = hpk.plugin_reference_tool(skill, toolset="sample") + + ctx = FakePluginCtx() + hpk.register_plugin(ctx, self._module(reader=reader), skills=(skill,)) + self.assertEqual(["sample_read_reference"], [item["name"] for item in ctx.tools]) + + payload = json.loads(reader({})) + + self.assertTrue(payload["success"]) + self.assertEqual(["a.md", "nested/b.md"], payload["data"]["files"]) + + def test_reads_a_specific_file(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + skill = self._skill_with_references(tmp, **{"a.md": "hello world"}) + reader = hpk.plugin_reference_tool(skill, toolset="sample") + + payload = json.loads(reader({"file_path": "a.md"})) + + self.assertTrue(payload["success"]) + self.assertEqual("hello world", payload["data"]["content"]) + + def test_accepts_custom_name_and_description(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + skill = self._skill_with_references(tmp, **{"a.md": "hi"}) + reader = hpk.plugin_reference_tool( + skill, toolset="sample", name="custom_reader", description="Custom description." + ) + + ctx = FakePluginCtx() + hpk.register_plugin(ctx, self._module(reader=reader), skills=(skill,)) + + self.assertEqual(1, len(ctx.tools)) + self.assertEqual("custom_reader", ctx.tools[0]["name"]) + self.assertEqual("Custom description.", ctx.tools[0]["schema"]["description"]) + + def test_rejects_a_missing_file(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + skill = self._skill_with_references(tmp, **{"a.md": "hi"}) + reader = hpk.plugin_reference_tool(skill, toolset="sample") + + payload = json.loads(reader({"file_path": "missing.md"})) + + self.assertFalse(payload["success"]) + self.assertIn("not found", payload["error"]) + + def test_rejects_relative_path_traversal_outside_references_dir(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + secret = Path(tmp) / "secret.txt" + secret.write_text("do not read me") + skill = self._skill_with_references(tmp, **{"a.md": "hi"}) + reader = hpk.plugin_reference_tool(skill, toolset="sample") + + payload = json.loads(reader({"file_path": "../secret.txt"})) + + self.assertFalse(payload["success"]) + self.assertIn("escapes references_dir", payload["error"]) + + def test_rejects_a_symlink_that_escapes_references_dir(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + secret = Path(tmp) / "secret.txt" + secret.write_text("do not read me") + skill = self._skill_with_references(tmp, **{"a.md": "hi"}) + (skill.references_dir / "escape.txt").symlink_to(secret) + + reader = hpk.plugin_reference_tool(skill, toolset="sample") + payload = json.loads(reader({"file_path": "escape.txt"})) + + self.assertFalse(payload["success"]) + self.assertIn("escapes references_dir", payload["error"]) + + def test_registers_and_answers_through_register_plugin_like_any_other_tool(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + skill = self._skill_with_references(tmp, **{"a.md": "hi"}) + reader = hpk.plugin_reference_tool(skill, toolset="sample") + + ctx = FakePluginCtx() + summary = hpk.register_plugin(ctx, self._module(reader=reader), skills=(skill,)) + + self.assertIn("sample_read_reference", summary.tools) + self.assertEqual({"sample_read_reference"}, {item["name"] for item in ctx.tools}) + + def _module(self, **attrs): + module = types.ModuleType("sample_plugin") + for name, value in attrs.items(): + setattr(module, name, value) + return module + + if __name__ == "__main__": unittest.main()