Bladeren bron

Ship tools.py so the declared tools reach CLI and TUI sessions

plugin.yaml declares provides_tools, which is the plugin's opt-in to having
its tools registered without the adapter — and Hermes delivers that by
importing tools.py and calling register_tools(ctx). With no tools.py the loader
only logged a warning, so the three tools existed in gateway/web processes only,
while README.md promised them in the CLI and after-install.md told the user to
check with `hermes tools`.

Both entry points now call register_plugin_tools(), which reads one
_PLUGIN_TOOLS table, so the list cannot exist twice and disagree. Tests pin the
table against provides_tools and the two entry points against each other.

Also name chatto_list_rooms in after-install.md: the sentence named two tools
and then claimed `hermes tools` lists three.
Paul Klumpp 2 weken geleden
bovenliggende
commit
5092bd8397
5 gewijzigde bestanden met toevoegingen van 148 en 25 verwijderingen
  1. 27 0
      AGENTS.md
  2. 29 24
      adapter.py
  3. 2 1
      after-install.md
  4. 58 0
      test_adapter.py
  5. 32 0
      tools.py

+ 27 - 0
AGENTS.md

@@ -531,6 +531,33 @@ The reverse holds too: drop a method and its line disappears from the log on its
 own, but its `_CAPABILITY_LABELS` entry should go with it, along with its row in
 the user docs.
 
+## The plugin tools register through two doors
+
+**A tool that exists in one session kind and not the other is a bug, and the
+usual cause is a second copy of the tool list.**
+
+Hermes defers platform plugins: the CLI and the TUI never materialize a
+platform, so `register()` — where `ctx.register_platform` lives — does not run
+there at all. `plugin.yaml`'s `provides_tools` is what asks the loader for a
+second door; it imports `tools.py` and calls `register_tools(ctx)`
+(`hermes_cli/plugins_loader.py`, `_register_deferred_platform_tools`). Without
+that module Hermes does not fail — it logs that the declared tools will be
+missing, and the agent simply has no way to send.
+
+Both doors call `register_plugin_tools()` in `adapter.py`, which reads the one
+`_PLUGIN_TOOLS` table (name, schema, handler). So when you add a tool:
+
+1. Add it to `_PLUGIN_TOOLS` and to `provides_tools` in `plugin.yaml` — the
+   manifest is what opts the plugin into the second door, and
+   `TestRegistration` pins the two against each other, so neither can drift
+   alone.
+2. Registering it directly inside `register()` instead re-creates the split
+   this structure exists to prevent. The same-toolset re-registration the
+   gateway then performs is allowed (`tools/registry.py`), so there is nothing
+   to work around.
+3. `tools.py` stays a thin shim: the handlers live in `adapter.py`, and a
+   second copy of them would be a second module-level copy of the adapter.
+
 ## Shell Scripts
 
 **Every shell script in this repo must pass `shellcheck` before its change

+ 29 - 24
adapter.py

@@ -4127,35 +4127,40 @@ def _register_bundled_skills(ctx) -> None:
         logger.info("Chatto plugin skill registered: %s", child.name)
 
 
+# The plugin's outbound tools: (tool name, JSON schema, handler). One list, so
+# the two registration entry points below cannot disagree about what exists.
+_PLUGIN_TOOLS = (
+    ("chatto_send_dm", CHATTO_SEND_DM_SCHEMA, _handle_chatto_send_dm),
+    ("chatto_send_room", CHATTO_SEND_ROOM_SCHEMA, _handle_chatto_send_room),
+    ("chatto_list_rooms", CHATTO_LIST_ROOMS_SCHEMA, _handle_chatto_list_rooms),
+)
+
+
+def register_plugin_tools(ctx) -> None:
+    """Register the plugin's outbound tools in the ``chatto-platform`` toolset.
+
+    Reached through two doors: ``register()`` in a gateway/web process, which
+    materializes the platform, and ``tools.register_tools`` in a CLI/TUI one,
+    which never does. A tool list written out twice would mean tools that exist
+    in only half the sessions, so both doors call this.
+    """
+    for name, schema, handler in _PLUGIN_TOOLS:
+        ctx.register_tool(
+            name=name,
+            toolset="chatto-platform",
+            schema=schema,
+            handler=handler,
+            is_async=True,
+            description=schema["description"],
+        )
+
+
 def register(ctx) -> None:
     """Plugin entry point — called by the Hermes plugin system."""
     logger.info("Registering Chatto platform plugin on Hermes Agent")
 
     # --- Plugin tools ---
-    ctx.register_tool(
-        name="chatto_send_dm",
-        toolset="chatto-platform",
-        schema=CHATTO_SEND_DM_SCHEMA,
-        handler=_handle_chatto_send_dm,
-        is_async=True,
-        description=CHATTO_SEND_DM_SCHEMA["description"],
-    )
-    ctx.register_tool(
-        name="chatto_send_room",
-        toolset="chatto-platform",
-        schema=CHATTO_SEND_ROOM_SCHEMA,
-        handler=_handle_chatto_send_room,
-        is_async=True,
-        description=CHATTO_SEND_ROOM_SCHEMA["description"],
-    )
-    ctx.register_tool(
-        name="chatto_list_rooms",
-        toolset="chatto-platform",
-        schema=CHATTO_LIST_ROOMS_SCHEMA,
-        handler=_handle_chatto_list_rooms,
-        is_async=True,
-        description=CHATTO_LIST_ROOMS_SCHEMA["description"],
-    )
+    register_plugin_tools(ctx)
 
     # --- Bundled skills ---
     # Ship the plugin's own usage/development knowledge with the code, so any

+ 2 - 1
after-install.md

@@ -95,4 +95,5 @@ can live in either file. Environment variables take precedence over
    answers in, or to DM someone who has already written to it. Those are the
    `chatto_send_room` / `chatto_send_dm` tools, which obey the same room rules
    as inbound messages — a channel listed in neither mention room list stays
-   read-only. `hermes tools` lists all three.
+   read-only. A third tool, `chatto_list_rooms`, turns a channel name into the
+   room ID those tools want. `hermes tools` lists all three.

+ 58 - 0
test_adapter.py

@@ -102,6 +102,41 @@ _SEEN_CAP = ChattoConstants.SEEN_CAP
 # -- Helpers --
 
 
+def _manifest_provides_tools() -> list[str]:
+    """The tool names plugin.yaml declares as ``provides_tools``."""
+    import yaml
+
+    with open(os.path.join(PLUGIN_ROOT, "plugin.yaml")) as manifest_file:
+        manifest = yaml.safe_load(manifest_file)
+    return list(manifest.get("provides_tools") or [])
+
+
+def _registered_tool_names(ctx) -> list[str]:
+    """Only the tools — ``registered_names`` also carries the platform entry."""
+    return [tool["name"] for tool in ctx.registered_tools]
+
+
+def _load_plugin_tools():
+    """Load the plugin's ``tools.py`` under a private module name.
+
+    Imported bare it would shadow Hermes' own top-level ``tools`` package,
+    which shares this test suite's ``sys.path``.
+    """
+    import importlib.util
+
+    path = os.path.join(PLUGIN_ROOT, "tools.py")
+    if not os.path.isfile(path):
+        pytest.fail(
+            "plugin.yaml declares provides_tools, which Hermes delivers through a "
+            "tools.py — but the plugin ships no tools.py, so CLI/TUI sessions get "
+            "the tools only as a warning log"
+        )
+    spec = importlib.util.spec_from_file_location("chatto_plugin_tools", path)
+    module = importlib.util.module_from_spec(spec)
+    spec.loader.exec_module(module)
+    return module
+
+
 class _MockPluginContext:
     """Minimal mock for the plugin registration context."""
 
@@ -378,6 +413,29 @@ class TestRegistration:
         with patch.object(ChattoAdapter, "send_video", BasePlatformAdapter.send_video):
             assert "video" not in _capabilities()
 
+    def test_provides_tools_is_deliverable_in_a_cli_session(self):
+        """``provides_tools`` asks Hermes to register the tools without the
+        adapter, which it can only do through a ``tools.py``. A manifest that
+        promises them there and ships no such module leaves a CLI session with a
+        warning and no tools at all — so the two must agree, name for name.
+        """
+        declared = _manifest_provides_tools()
+        ctx = _MockPluginContext()
+        _load_plugin_tools().register_tools(ctx)
+        assert sorted(_registered_tool_names(ctx)) == sorted(declared)
+
+    def test_both_entry_points_register_the_same_tools(self):
+        """``register`` (gateway/web) and ``register_tools`` (CLI/TUI) are two
+        doors into one list of tools. Anything they disagree on is a tool that
+        exists in half the sessions."""
+        gateway_ctx = _MockPluginContext()
+        register(gateway_ctx)
+        cli_ctx = _MockPluginContext()
+        _load_plugin_tools().register_tools(cli_ctx)
+        assert sorted(_registered_tool_names(cli_ctx)) == sorted(
+            _registered_tool_names(gateway_ctx)
+        )
+
     def test_check_requirements(self):
         assert check_requirements() is True
 

+ 32 - 0
tools.py

@@ -0,0 +1,32 @@
+"""
+Chatto plugin tools, registered without the Chatto adapter.
+
+Hermes defers platform plugins: a CLI or TUI session never materializes a
+platform, so ``register()`` — where the adapter is registered — does not run
+there, and tools declared only inside it are simply absent from those sessions.
+The ``provides_tools`` declaration in plugin.yaml asks Hermes to import this
+module instead and call ``register_tools(ctx)``; without it, Hermes only logs
+that the declared tools will be missing.
+
+The tool list itself lives in the adapter module, because that is where the
+handlers live. Both entry points call the same registration, so the two doors
+into the list cannot drift apart.
+"""
+
+try:
+    from .adapter import register_plugin_tools
+except ImportError:  # pragma: no cover - loaded outside a package (tests)
+    from adapter import register_plugin_tools
+
+__all__ = ["register_tools"]
+
+
+def register_tools(ctx) -> None:
+    """Register the ``chatto-platform`` toolset with the plugin context.
+
+    Entry point for Hermes' deferred-platform loader, which calls this in a
+    session that never loads the adapter. Importing the adapter module here is
+    deliberate: the handlers need it, and the deferred loader already imported
+    it as this package's ``__init__`` did, so no second copy is loaded.
+    """
+    register_plugin_tools(ctx)