From 1276b00af6faebfe2589babd05b41866fc6751a9 Mon Sep 17 00:00:00 2001 From: Eman Date: Fri, 14 Aug 2026 08:46:49 -0600 Subject: [PATCH 1/5] fix(plugins): register deferred platform client tools at discovery (#78050) Rebased onto current main. `hermes_cli/plugins.py` grew 103KB -> 265KB across 49 commits since the original branch point, and the attribution mechanism this change hooks into was replaced along the way: the `_tools_before` / `_plugin_tool_names` snapshot diff is now a registration ledger sliced from `registration_start`, and `_plugin_id` is `plugin_key`. Re-anchored accordingly: - Discovery-time pre-registration, module reuse, and the `provides_tools` opt-in are unchanged. - Attribution credits `_predeclared_tools` ahead of the ledger slice, since those tools registered before `registration_start` and the slice cannot see them. - A failed materialization no longer carries attribution across. The failure path now sweeps the whole ownership ledger for the plugin key, not just the `registration_start:` slice, so the pre-registered tools are disposed along with the adapter. Attribution and the registry now agree at zero instead of reporting tools the process is not serving. tests/hermes_cli/test_deferred_platform_client_tools.py 13/13. test_plugins.py, test_plugins_cmd_list.py, test_plugin_cli_registration.py 65/65. Co-Authored-By: Claude Opus 5 --- hermes_cli/plugins.py | 165 +++++- plugins/platforms/a2a/plugin.yaml | 10 + .../test_deferred_platform_client_tools.py | 474 ++++++++++++++++++ 3 files changed, 647 insertions(+), 2 deletions(-) create mode 100644 tests/hermes_cli/test_deferred_platform_client_tools.py diff --git a/hermes_cli/plugins.py b/hermes_cli/plugins.py index 5124516ca034..ac1246e8109e 100644 --- a/hermes_cli/plugins.py +++ b/hermes_cli/plugins.py @@ -3454,6 +3454,14 @@ def __init__(self, scope_key: Optional[str] = None) -> None: # symmetric force-reload lands. self._ownership_ledger: Dict[str, List[PluginRegistration]] = {} self._registration_order: List[PluginRegistration] = [] + # Deferred platform plugins whose client tools were registered at + # discovery time (see _register_deferred_platform_tools). Keyed by + # plugin id: the already-imported package module, so materializing the + # adapter later doesn't re-execute it, and the tool names it + # contributed, so `hermes plugins list` still attributes them once the + # full plugin loads. + self._predeclared_modules: Dict[str, types.ModuleType] = {} + self._predeclared_tools: Dict[str, List[str]] = {} # ----------------------------------------------------------------------- # Registration ledger internals @@ -3717,6 +3725,8 @@ def _unload_scoped( self._system_prompt_sections.clear() self._approval_transports.clear() self._slack_action_handlers.clear() + self._predeclared_modules.clear() + self._predeclared_tools.clear() self._context_engine = None self._discovered = False else: @@ -4507,6 +4517,131 @@ def _loader(_manifest: PluginManifest = manifest) -> None: exc_info=True, ) self._load_plugin(manifest) + return + + self._register_deferred_platform_tools(manifest, loaded) + + def _register_deferred_platform_tools( + self, manifest: PluginManifest, loaded: LoadedPlugin + ) -> None: + """Register a deferred platform's *client* tools without its adapter. + + A platform plugin can ship two independent things: an inbound adapter + (heavy — it imports the platform SDK) and outbound client tools the + agent calls like any other tool. Deferring the plugin defers both, so + in a CLI/TUI process the client tools never register at all: + ``resolve_toolset()`` returns ``[]``, the toolset is missing from the + ``hermes tools`` checklist, and even an explicit ``platform_toolsets`` + entry is dropped because the key is unknown. The same tools work in + gateway/web processes only because those materialize every platform at + startup (issue #78050). + + Client tools that live in a dedicated ``tools`` submodule can be + registered at discovery time instead: importing ``/tools.py`` + does not import the adapter, so the SDK stays unloaded and startup + stays cheap. A plugin taking this path must therefore keep its package + ``__init__`` import-light and pull the adapter in from inside + ``register()`` (as ``plugins/platforms/a2a`` does). + + Opting in is explicit: the manifest must declare ``provides_tools`` + (the field the plugin list and web server already read to name a + plugin's tools, per #78538). Keying off the mere presence of a + ``tools.py`` would opt a plugin in by accident — a platform is free to + put internal helpers there — and would leave the contract invisible to + anyone reading the manifest. ``tools.py`` remains where the code is + imported from; ``provides_tools`` is what asks for it. A platform that + does not declare the field is untouched and stays fully deferred. + """ + if not manifest.provides_tools: + return + + lookup_key = manifest.key or manifest.name + plugin_dir = Path(manifest.path) if manifest.path else None + if plugin_dir is None or not (plugin_dir / "tools.py").is_file(): + # Declared but undeliverable. Staying quiet here reproduces the + # exact symptom this path exists to fix — tools the manifest + # promises, silently absent from the session (#78050) — so say so. + logger.warning( + "Plugin '%s' declares provides_tools %s but has no tools.py; " + "those tools will not be available in CLI/TUI sessions.", + lookup_key, + list(manifest.provides_tools), + ) + return + + # Snapshotted outside the try so the failure path can tell which tools + # a partially-successful register_tools() left behind. + before = set(self._plugin_tool_names) + try: + module = self._load_directory_module(manifest) + # Record the module even if nothing below registers: the package + # body has already run, so materializing the adapter later must + # reuse it rather than execute it a second time. + loaded.module = module + self._predeclared_modules[lookup_key] = module + + tools_module = importlib.import_module(f"{module.__name__}.tools") + register_tools = getattr(tools_module, "register_tools", None) + if register_tools is None: + logger.warning( + "Plugin '%s' declares provides_tools %s but its tools.py " + "has no register_tools(ctx); those tools will not be " + "available in CLI/TUI sessions.", + lookup_key, + list(manifest.provides_tools), + ) + return + + register_tools(PluginContext(manifest, self)) + registered = [ + t for t in self._plugin_tool_names if t not in before + ] + + loaded.tools_registered = registered + self._predeclared_tools[lookup_key] = registered + logger.debug( + "Deferred platform '%s': pre-registered %d client tool(s) %s", + lookup_key, + len(registered), + registered, + ) + except Exception as exc: + # A register_tools() that registered some tools and THEN raised + # leaves those tools live in the registry. Credit them, or + # `hermes plugins list` under-reports what the process is actually + # carrying — and _load_plugin's own diff would miss them later + # too, since they are already in its "before" snapshot. + partial = [t for t in self._plugin_tool_names if t not in before] + if partial: + loaded.tools_registered = partial + self._predeclared_tools[lookup_key] = partial + + # Never let a client-tool import break discovery — the platform + # stays deferred and behaves exactly as it did before. But a + # broken tools.py produces the #78050 symptom itself (declared + # tools missing from the session), so this has to be visible + # without turning on debug logging to find it. + # + # Where it failed is the first thing an operator needs: nothing + # registered points at the import or the module body, a partial + # run points at one tool's definition, and a full run that still + # raised points past the registrations entirely. + declared = len(manifest.provides_tools) + if not partial: + scope = f"before registering any of its {declared} declared tool(s)" + elif len(partial) >= declared: + scope = f"after registering all {declared} declared tool(s)" + else: + scope = f"after registering {len(partial)} of {declared} declared tool(s)" + logger.warning( + "Plugin '%s': client-tool pre-registration failed %s (%s).%s", + lookup_key, + scope, + exc, + "" if len(partial) >= declared else + " The remainder will be missing from CLI/TUI sessions.", + exc_info=_PLUGINS_DEBUG, + ) def _warn_python_dependencies(self, manifest: PluginManifest) -> None: """Surface declared pip dependencies (#64165). @@ -4635,7 +4770,13 @@ def _load_plugin_scoped(self, manifest: PluginManifest) -> None: policy_lease.dispose, ) try: - if manifest.source in {"user", "project", "bundled"}: + # A deferred platform whose client tools were already registered at + # discovery time has its package imported too — reuse it so the + # module body doesn't execute twice (#78050). + preloaded = self._predeclared_modules.pop(plugin_key, None) + if preloaded is not None: + module = preloaded + elif manifest.source in {"user", "project", "bundled"}: module = self._load_directory_module( manifest, module_name=_module_name ) @@ -4657,10 +4798,20 @@ def _load_plugin_scoped(self, manifest: PluginManifest) -> None: for registration in self._registration_order[registration_start:] if registration.plugin_key == plugin_key and registration.active ] - loaded.tools_registered = [ + # Tools this plugin already contributed at discovery time were + # registered before ``registration_start``, so the ledger slice + # above cannot see them and `hermes plugins list` would + # under-report once the deferred adapter materializes (#78050). + # Credit them back to the plugin that actually registered them. + _predeclared = [ + t for t in self._predeclared_tools.pop(plugin_key, []) + if t in self._plugin_tool_names + ] + loaded.tools_registered = _predeclared + [ registration.key for registration in registrations if registration.kind == "tool" + and registration.key not in _predeclared ] loaded.hooks_registered = [ registration.key @@ -4713,6 +4864,16 @@ def _load_plugin_scoped(self, manifest: PluginManifest) -> None: "Failed to load plugin '%s': %s", manifest.name, exc, exc_info=_PLUGINS_DEBUG, ) + # A materialization that did NOT succeed has already had its + # discovery-time pre-registrations disposed: the failure path above + # sweeps the whole ownership ledger for this plugin key, not just the + # ``registration_start:`` slice, so nothing this plugin registered + # survives it. There is no live tool left to credit — attribution and + # the registry agree at zero. Only the success path pops + # _predeclared_tools, so drop the entry here rather than let the + # bookkeeping outlive the load attempt (#78050). + if not loaded.enabled: + self._predeclared_tools.pop(plugin_key, None) self._plugins[manifest.key or manifest.name] = loaded def _load_portable_plugin( diff --git a/plugins/platforms/a2a/plugin.yaml b/plugins/platforms/a2a/plugin.yaml index 9f08b7f9e687..110267dea594 100644 --- a/plugins/platforms/a2a/plugin.yaml +++ b/plugins/platforms/a2a/plugin.yaml @@ -26,6 +26,16 @@ description: > Pure stdlib transport (http.server + urllib) — no a2a-sdk dependency required. author: Nous Research +# The outbound client tools. Declaring them here is what asks discovery to +# import `tools.py` in CLI/TUI processes, where the plugin is otherwise +# deferred and the tools would never register at all (#78050). The inbound +# adapter stays deferred either way — only this submodule is imported. +provides_tools: + - a2a_discover + - a2a_call + - a2a_list + - a2a_history + - a2a_orchestrate # requires_env / optional_env are surfaced in the `hermes config` UI via the # platform-plugin env var injector in hermes_cli/config.py. requires_env: [] diff --git a/tests/hermes_cli/test_deferred_platform_client_tools.py b/tests/hermes_cli/test_deferred_platform_client_tools.py new file mode 100644 index 000000000000..67638f51d4f6 --- /dev/null +++ b/tests/hermes_cli/test_deferred_platform_client_tools.py @@ -0,0 +1,474 @@ +"""Deferred platform plugins must still register their *client* tools. + +Issue #78050: a bundled ``kind: platform`` plugin is registered as a deferred +loader so ``hermes chat`` doesn't import ~20 gateway SDKs. The a2a plugin ships +two independent things behind that one deferral — an inbound adapter (heavy) +and five outbound client tools (``a2a_call``, ``a2a_discover``, ``a2a_list``, +``a2a_history``, ``a2a_orchestrate``). Deferring the plugin deferred both, so +in a CLI/TUI process the client tools never registered at all: +``resolve_toolset("a2a")`` returned ``[]`` and the toolset was absent from the +``hermes tools`` checklist. The same tools worked in gateway/web processes only +because those materialize every platform at startup. + +Client tools that live in a dedicated ``tools`` submodule are now registered at +discovery time; the adapter stays deferred. +""" + +from __future__ import annotations + +import logging +import sys +from pathlib import Path + +import pytest +import yaml + + +A2A_CLIENT_TOOLS = { + "a2a_call", + "a2a_discover", + "a2a_history", + "a2a_list", + "a2a_orchestrate", +} + + +# ── synthetic platform plugin helpers ────────────────────────────────────── + + +def _write_platform_plugin( + root: Path, + platform: str, + *, + with_tools_module: bool, + declares_provides_tools: "bool | None" = None, +) -> "object": + """Create a bundled-style platform plugin and return its manifest. + + The adapter import is the expensive thing we must NOT trigger: it is + modelled as ``adapter.py`` setting a module-level sentinel, imported from + inside ``register()`` exactly as the real a2a plugin does. + + ``declares_provides_tools`` controls the manifest opt-in independently of + whether a ``tools.py`` exists on disk, so a test can pin what actually + triggers pre-registration. Defaults to following ``with_tools_module``, + which is the shape a real plugin ships. + """ + from hermes_cli.plugins import PluginManifest + + if declares_provides_tools is None: + declares_provides_tools = with_tools_module + provides_tools = [f"{platform}_call"] if declares_provides_tools else [] + + plugin_dir = root / platform + plugin_dir.mkdir(parents=True, exist_ok=True) + manifest_data = { + "name": f"{platform}-platform", + "kind": "platform", + "version": "1.0.0", + } + if provides_tools: + manifest_data["provides_tools"] = provides_tools + (plugin_dir / "plugin.yaml").write_text( + yaml.dump(manifest_data), + encoding="utf-8", + ) + + # Sentinels let the tests prove what was and wasn't imported. + (plugin_dir / "adapter.py").write_text( + "import _deferred_probe\n" + "_deferred_probe.adapter_imports += 1\n", + encoding="utf-8", + ) + init_body = [ + "import _deferred_probe", + "_deferred_probe.package_execs += 1", + "", + "def register(ctx):", + " from . import adapter # noqa: F401 (heavy import, deferred)", + ] + if with_tools_module: + (plugin_dir / "tools.py").write_text( + "import _deferred_probe\n" + "_deferred_probe.tools_execs += 1\n" + "\n" + "\n" + "def _handler(**kwargs):\n" + " return 'ok'\n" + "\n" + "\n" + "def register_tools(ctx):\n" + f" ctx.register_tool(\n" + f" name='{platform}_call',\n" + f" toolset='{platform}',\n" + " schema={'type': 'function', 'function': {'name': " + f"'{platform}_call', 'description': 'call a peer', 'parameters': " + "{'type': 'object', 'properties': {}}}},\n" + " handler=_handler,\n" + " description='call a peer',\n" + " )\n", + encoding="utf-8", + ) + init_body.append(" from .tools import register_tools") + init_body.append(" register_tools(ctx)") + (plugin_dir / "__init__.py").write_text("\n".join(init_body) + "\n", encoding="utf-8") + + return PluginManifest( + name=f"{platform}-platform", + kind="platform", + source="bundled", + path=str(plugin_dir), + key=f"{platform}-platform", + provides_tools=provides_tools, + ) + + +@pytest.fixture +def probe(monkeypatch): + """A module the synthetic plugin can count imports into.""" + import types + + mod = types.ModuleType("_deferred_probe") + mod.package_execs = 0 + mod.adapter_imports = 0 + mod.tools_execs = 0 + monkeypatch.setitem(sys.modules, "_deferred_probe", mod) + return mod + + +@pytest.fixture +def clean_registry(): + """Undo everything a synthetic plugin leaves behind. + + Each test writes a fresh plugin to its own tmp_path but reuses the + ``probeplat`` name, so the imported ``hermes_plugins.*`` modules have to go + too — otherwise the next test's ``import_module`` returns the previous + test's cached submodule instead of reading the new file. + """ + from gateway.platform_registry import platform_registry + from tools.registry import registry + + before_tools = set(registry._tools) + before_modules = set(sys.modules) + yield + for name in set(registry._tools) - before_tools: + registry._tools.pop(name, None) + for platform in ("probeplat", "barefoot", "quietplat", "promiseplat"): + platform_registry.unregister(platform) + for name in set(sys.modules) - before_modules: + if name.startswith("hermes_plugins."): + sys.modules.pop(name, None) + + +# ── the reported symptom, against the real a2a plugin ────────────────────── + + +class TestA2AClientToolsInCliProcess: + """The issue's exact repro: a CLI/TUI process, no gateway startup.""" + + def test_manifest_declares_the_client_tools(self): + """The opt-in lives in the manifest, so it is pinned like any contract. + + Dropping ``provides_tools`` from plugin.yaml silently reverts a2a to + the deferred-and-invisible behaviour of #78050, with every other test + here still passing on the synthetic plugins — so assert it directly. + """ + manifest_path = ( + Path(__file__).resolve().parents[2] + / "plugins" / "platforms" / "a2a" / "plugin.yaml" + ) + manifest = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) + + assert set(manifest.get("provides_tools") or []) == A2A_CLIENT_TOOLS + + def test_a2a_toolset_resolves_without_materializing_the_platform(self): + from hermes_cli.plugins import PluginManager + from toolsets import resolve_toolset + + mgr = PluginManager() + mgr.discover_and_load() + + a2a = mgr._plugins.get("a2a-platform") + assert a2a is not None, "bundled a2a platform plugin was not discovered" + + # The whole point of the deferral is preserved: the inbound adapter is + # still not imported in a CLI process. + assert a2a.deferred is True + + # ...but the outbound client tools are now reachable. Before the fix + # this was [] until a gateway/web process called all_entries(). + assert set(resolve_toolset("a2a")) == A2A_CLIENT_TOOLS + + def test_a2a_appears_in_the_hermes_tools_checklist(self): + """`a2a` is in _DEFAULT_OFF_TOOLSETS, so it must be tickable. + + Every other member of that set (homeassistant, spotify, video_gen, + x_search, ...) renders a checkbox; a2a rendered nothing, so the + documented opt-in path had nothing to tick. + """ + from hermes_cli.plugins import discover_plugins, get_plugin_toolsets + + # get_plugin_toolsets() reads the process-wide manager, which is what + # the `hermes tools` checklist does. + discover_plugins() + + assert "a2a" in {key for key, _, _ in get_plugin_toolsets()} + + def test_platform_bundle_includes_the_client_tools(self): + """``hermes-a2a`` sessions get the client tools too. + + The bundle path read the tool registry behind a deliberately cheap + ``is_registered()`` check, so a deferred platform's own tools were + dropped from its bundle as well. + """ + from hermes_cli.plugins import PluginManager + from toolsets import resolve_toolset + + mgr = PluginManager() + mgr.discover_and_load() + + assert A2A_CLIENT_TOOLS.issubset(set(resolve_toolset("hermes-a2a"))) + + +# ── the general mechanism ────────────────────────────────────────────────── + + +class TestDeferredPlatformToolPreregistration: + def test_tools_module_registers_without_importing_the_adapter( + self, tmp_path, probe, clean_registry + ): + from hermes_cli.plugins import PluginManager + from toolsets import resolve_toolset + + manifest = _write_platform_plugin(tmp_path, "probeplat", with_tools_module=True) + + mgr = PluginManager() + mgr._register_deferred_platform(manifest) + + assert resolve_toolset("probeplat") == ["probeplat_call"] + # The expensive half stayed deferred — that's what makes this safe. + assert probe.adapter_imports == 0 + assert probe.tools_execs == 1 + assert mgr._plugins["probeplat-platform"].deferred is True + + def test_plugin_without_tools_module_stays_fully_deferred( + self, tmp_path, probe, clean_registry + ): + """No ``tools.py`` means no behaviour change at all — nothing imported.""" + from hermes_cli.plugins import PluginManager + + manifest = _write_platform_plugin(tmp_path, "barefoot", with_tools_module=False) + + mgr = PluginManager() + mgr._register_deferred_platform(manifest) + + assert probe.package_execs == 0 + assert probe.adapter_imports == 0 + assert mgr._plugins["barefoot-platform"].tools_registered == [] + + def test_tools_module_alone_does_not_opt_a_platform_in( + self, tmp_path, probe, clean_registry + ): + """``provides_tools`` is the trigger, not the presence of a file. + + A platform is free to keep internal helpers in ``tools.py``; without + the manifest declaring what it publishes, discovery must not import + the package at all. Otherwise a plugin opts into an eager import by + naming a file, and the contract is invisible to anyone reading the + manifest. + """ + from hermes_cli.plugins import PluginManager + + manifest = _write_platform_plugin( + tmp_path, + "quietplat", + with_tools_module=True, + declares_provides_tools=False, + ) + + mgr = PluginManager() + mgr._register_deferred_platform(manifest) + + assert probe.package_execs == 0 + assert probe.tools_execs == 0 + assert probe.adapter_imports == 0 + assert mgr._plugins["quietplat-platform"].deferred is True + assert mgr._plugins["quietplat-platform"].tools_registered == [] + + def test_package_body_runs_once_across_discovery_and_materialization( + self, tmp_path, probe, clean_registry + ): + """Pre-importing the package must not double-execute it later. + + Discovery imports ``/__init__.py`` to reach ``tools.py``; when + the gateway later materializes the adapter, ``_load_plugin`` reuses + that module instead of re-running its body. + """ + from gateway.platform_registry import platform_registry + from hermes_cli.plugins import PluginManager + + manifest = _write_platform_plugin(tmp_path, "probeplat", with_tools_module=True) + + mgr = PluginManager() + mgr._register_deferred_platform(manifest) + assert probe.package_execs == 1 + + # What gateway/web startup does. + platform_registry.get("probeplat") + + assert probe.package_execs == 1 + assert probe.adapter_imports == 1 + + def test_tools_stay_attributed_after_materialization( + self, tmp_path, probe, clean_registry + ): + """`hermes plugins list` must still credit the pre-registered tools. + + ``_load_plugin`` attributes tools by diffing the registry around + ``register()``. Tools registered at discovery are already in the + "before" snapshot, so the diff alone would report zero. + """ + from gateway.platform_registry import platform_registry + from hermes_cli.plugins import PluginManager + + manifest = _write_platform_plugin(tmp_path, "probeplat", with_tools_module=True) + + mgr = PluginManager() + mgr._register_deferred_platform(manifest) + assert mgr._plugins["probeplat-platform"].tools_registered == ["probeplat_call"] + + platform_registry.get("probeplat") + + loaded = mgr._plugins["probeplat-platform"] + assert loaded.tools_registered == ["probeplat_call"] + assert loaded.enabled is True + + def test_broken_tools_module_does_not_break_discovery( + self, tmp_path, probe, clean_registry, caplog + ): + """A plugin whose ``tools.py`` raises degrades to the old behaviour. + + Degrading quietly is not enough: the degraded state IS the #78050 + symptom (declared tools absent from the session), so it has to be + visible without enabling debug logging to find it. + """ + from hermes_cli.plugins import PluginManager + + manifest = _write_platform_plugin(tmp_path, "probeplat", with_tools_module=True) + (Path(manifest.path) / "tools.py").write_text( + "raise RuntimeError('boom')\n", encoding="utf-8" + ) + + mgr = PluginManager() + with caplog.at_level(logging.WARNING, logger="hermes_cli.plugins"): + mgr._register_deferred_platform(manifest) # must not raise + + assert mgr._plugins["probeplat-platform"].deferred is True + assert mgr._plugins["probeplat-platform"].tools_registered == [] + assert any( + "probeplat-platform" in r.message and r.levelno == logging.WARNING + for r in caplog.records + ), caplog.text + + def test_partially_registered_tools_are_still_attributed( + self, tmp_path, probe, clean_registry, caplog + ): + """Tools registered before a mid-way failure are live — credit them. + + `register_tools` is not transactional: whatever it registered before + raising stays in the registry. Leaving those unattributed makes + `hermes plugins list` under-report what the process is carrying, and + `_load_plugin`'s own diff cannot recover them later because they are + already inside its "before" snapshot. + """ + from hermes_cli.plugins import PluginManager + + manifest = _write_platform_plugin(tmp_path, "probeplat", with_tools_module=True) + tools_py = (Path(manifest.path) / "tools.py").read_text(encoding="utf-8") + (Path(manifest.path) / "tools.py").write_text( + tools_py + " raise RuntimeError('boom after the first tool')\n", + encoding="utf-8", + ) + + mgr = PluginManager() + with caplog.at_level(logging.WARNING, logger="hermes_cli.plugins"): + mgr._register_deferred_platform(manifest) # must not raise + + # Attribution without a live tool would be a lie, so check the registry + # itself rather than only the bookkeeping maps. + from toolsets import resolve_toolset + + assert resolve_toolset("probeplat") == ["probeplat_call"] + assert mgr._plugins["probeplat-platform"].tools_registered == ["probeplat_call"] + assert mgr._predeclared_tools["probeplat-platform"] == ["probeplat_call"] + assert mgr._plugins["probeplat-platform"].deferred is True + assert any(r.levelno == logging.WARNING for r in caplog.records), caplog.text + + def test_failed_materialization_tears_down_pre_registered_tools( + self, tmp_path, probe, clean_registry, caplog + ): + """A failed materialize takes the pre-registered tools down with it. + + The synthetic plugin's ``register()`` calls the same broken + ``register_tools`` without catching, so materializing raises. + + ``_load_plugin_scoped``'s failure path sweeps the *whole* ownership + ledger for this plugin key — not the ``registration_start:`` slice — + and disposes it, so the discovery-time client tools go with the failed + adapter. Attribution and the registry therefore agree at zero: `hermes + plugins list` reports no tools because the process really is serving + none. + + ``enabled`` stays False on purpose: the adapter genuinely did not load. + """ + from gateway.platform_registry import platform_registry + from hermes_cli.plugins import PluginManager + from toolsets import resolve_toolset + + manifest = _write_platform_plugin(tmp_path, "probeplat", with_tools_module=True) + tools_py = (Path(manifest.path) / "tools.py").read_text(encoding="utf-8") + (Path(manifest.path) / "tools.py").write_text( + tools_py + " raise RuntimeError('boom after the first tool')\n", + encoding="utf-8", + ) + + mgr = PluginManager() + with caplog.at_level(logging.WARNING, logger="hermes_cli.plugins"): + mgr._register_deferred_platform(manifest) + platform_registry.get("probeplat") # gateway startup; register() raises + + loaded = mgr._plugins["probeplat-platform"] + assert resolve_toolset("probeplat") == [] + assert loaded.tools_registered == [] + assert loaded.enabled is False + assert loaded.error + # The bookkeeping entry must not outlive the failed load attempt. + assert "probeplat-platform" not in mgr._predeclared_tools + + def test_declared_tools_with_no_tools_module_warns( + self, tmp_path, probe, clean_registry, caplog + ): + """A manifest promising tools it cannot deliver must say so. + + Returning silently here leaves the operator with exactly the bug this + path fixes and no thread to pull on. + """ + from hermes_cli.plugins import PluginManager + + manifest = _write_platform_plugin( + tmp_path, + "promiseplat", + with_tools_module=False, + declares_provides_tools=True, + ) + + mgr = PluginManager() + with caplog.at_level(logging.WARNING, logger="hermes_cli.plugins"): + mgr._register_deferred_platform(manifest) + + assert probe.package_execs == 0 + assert mgr._plugins["promiseplat-platform"].tools_registered == [] + assert any( + "promiseplat-platform" in r.message and "provides_tools" in r.message + for r in caplog.records + ), caplog.text From d979e8590d53467844c78feaf606120b1444eb6f Mon Sep 17 00:00:00 2001 From: Chen Jin Date: Fri, 14 Aug 2026 20:49:18 -0700 Subject: [PATCH 2/5] fix(toolsets): admit explicitly-configured plugin toolset keys in _get_platform_tools (#81163) Layer 2 of the #81163 / #78050 fix: _get_platform_tools computed plugin_ts_keys = _get_plugin_toolset_keys() but only used CONFIGURABLE_TOOLSETS in the explicit-config filter, so a user-listed plugin key like `a2a` in `platform_toolsets.cli: [hermes-cli, a2a]` was silently dropped. The filter now unions configurable and plugin toolset keys when evaluating has_explicit_config and when admitting per-key entries. Cherry-picked from PR #81190 (Layer 2 hunks only; Layer 1 is covered by the provides_tools mechanism from PR #78842). --- hermes_cli/tools_config.py | 8 ++- tests/hermes_cli/test_tools_config.py | 91 +++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 2 deletions(-) diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index 0f06b115e5c1..e3ff5a977802 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -2316,18 +2316,22 @@ def _get_platform_tools( configurable_keys = {ts_key for ts_key, _, _ in CONFIGURABLE_TOOLSETS} plugin_ts_keys = _get_plugin_toolset_keys() platform_default_keys = {p["default_toolset"] for p in PLATFORMS.values()} + # Plugin-provided toolsets are first-class on a platform-toolsets list — + # explicit config like ``[hermes-cli, a2a]`` must survive filtering just + # like a built-in configurable toolset would. See issue #81163. + explicit_known_keys = configurable_keys | plugin_ts_keys # If the saved list contains any configurable keys directly, the user # has explicitly configured this platform — use direct membership. # This avoids the subset-inference bug where composite toolsets like # "hermes-cli" (which include all _HERMES_CORE_TOOLS) cause disabled # toolsets to re-appear as enabled. - has_explicit_config = any(ts in configurable_keys for ts in toolset_names) + has_explicit_config = any(ts in explicit_known_keys for ts in toolset_names) if has_explicit_config: enabled_toolsets = { ts for ts in toolset_names - if ts in configurable_keys and _toolset_allowed_for_platform(ts, platform) + if ts in explicit_known_keys and _toolset_allowed_for_platform(ts, platform) } # Mixed config: composite toolset alongside configurables (e.g. # ``[hermes-cli, spotify]`` after enabling Spotify via ``hermes diff --git a/tests/hermes_cli/test_tools_config.py b/tests/hermes_cli/test_tools_config.py index dc89c6cae153..b4618848c102 100644 --- a/tests/hermes_cli/test_tools_config.py +++ b/tests/hermes_cli/test_tools_config.py @@ -1078,3 +1078,94 @@ def test_platforms_whose_composite_excludes_it_are_left_narrow(): include_default_mcp_servers=False, ) assert not (_RECENTLY_SHIPPED_TOOLSETS & enabled), platform + + +# Regression for issue #81163 (Layer 2): an explicitly-listed plugin toolset +# in ``platform_toolsets.`` must survive the filter, not be dropped +# because it isn't a built-in CONFIGURABLE_TOOLSETS entry. + + +def test_explicit_plugin_toolset_admitted_in_platform_toolsets(monkeypatch): + """When a plugin toolset key is explicitly listed under + ``platform_toolsets.`` (alongside a composite like + ``hermes-cli``), it MUST be admitted as a configurable key instead of + being silently dropped by the has_explicit_config filter. + + Reproduces the second half of #81163: even after the eager register_tools + fix lands, ``_get_platform_tools`` was filtering against + ``CONFIGURABLE_TOOLSETS`` only, so plugin keys in the explicit list were + excluded from ``enabled_toolsets``. + """ + # Force a plugin toolset key to be present without depending on the a2a + # plugin being installed on disk. _get_plugin_toolset_keys() calls + # discover_plugins(); we patch its source so the test is hermetic. + import hermes_cli.plugins as _plugins_mod + import hermes_cli.tools_config as _tc_mod + + class _StubMgr: + _plugin_tool_names = {"dplat_call"} + + def __getattr__(self, _name): + return lambda *_a, **_kw: None + + monkeypatch.setattr( + _plugins_mod, "get_plugin_toolsets", + lambda: [("dplat_client", "Test", "test toolset")], + ) + monkeypatch.setattr( + _tc_mod, "_get_plugin_toolset_keys", lambda: {"dplat_client"}, + ) + # Discover_plugins must succeed silently under the stub. + monkeypatch.setattr(_plugins_mod, "discover_plugins", lambda: None) + # Resolve dplat_call inside the dplat_client toolset — _get_platform_tools + # ends up calling resolve_toolset() which can fall back to the registry + # for plugin-provided names. Patch resolve_toolset for "dplat_client". + from toolsets import TOOLSETS as _BASE_TOOLSETS + import toolsets as _toolsets_mod + + original_resolve = _toolsets_mod.resolve_toolset + + def _resolve_with_plugin(ts_key, include_registry=True): + if ts_key == "dplat_client": + return ["dplat_call"] + return original_resolve(ts_key, include_registry=include_registry) + + monkeypatch.setattr(_toolsets_mod, "resolve_toolset", _resolve_with_plugin) + monkeypatch.setattr( + _tc_mod, "resolve_toolset", _resolve_with_plugin, + raising=False, + ) + + # An explicit platform_toolsets list with a plugin key alongside the + # standard composite — exactly the "I want hermes-cli AND a2a in my CLI + # session" config the issue's user was trying to write. + config = {"platform_toolsets": {"cli": ["hermes-cli", "dplat_client"]}} + + enabled = _get_platform_tools(config, "cli") + + assert "dplat_client" in enabled, ( + "plugin toolset 'dplat_client' listed in platform_toolsets.cli was " + "dropped by _get_platform_tools — Layer 2 of #81163 not fixed" + ) + + +def test_explicit_plugin_toolset_admitted_against_real_a2a_plugin(monkeypatch): + """End-to-end Layer 2 regression: with the bundled a2a plugin enabled and + a real config like ``platform_toolsets.cli: [hermes-cli, a2a]``, ``a2a`` + must appear in the resolved enabled toolset set. Before the fix, the + filter dropped all non-CONFIGURABLE keys (a2a included).""" + # Discover real plugins so _get_plugin_toolset_keys() sees the a2a key. + # If the worktree lacks bundled plugin manifests, skip — this test + # exercises real bundled state and is meaningless without it. + from hermes_cli.plugins import discover_plugins, get_plugin_toolsets + discover_plugins() + plugin_ts_keys = {k for k, _, _ in get_plugin_toolsets()} + if "a2a" not in plugin_ts_keys: + pytest.skip("bundled a2a plugin not discoverable in this worktree") + + config = {"platform_toolsets": {"cli": ["hermes-cli", "a2a"]}} + enabled = _get_platform_tools(config, "cli") + assert "a2a" in enabled, ( + f"plugin-provided 'a2a' toolset dropped by _get_platform_tools " + f"(Layer 2 of #81163); enabled={sorted(enabled)}" + ) From 610f1480f01459ab8cbade76965e8afda1deba78 Mon Sep 17 00:00:00 2001 From: tachyon-r <291518778+tachyon-r@users.noreply.github.com> Date: Sat, 25 Jul 2026 12:54:35 -0400 Subject: [PATCH 3/5] fix(tools): recognize discovered plugin platforms --- gateway/platform_registry.py | 4 ++ hermes_cli/tools_config.py | 26 +++++++++++- tests/gateway/test_platform_registry.py | 12 ++++++ tests/hermes_cli/test_tools_disable_enable.py | 42 ++++++++++++++++++- 4 files changed, 81 insertions(+), 3 deletions(-) diff --git a/gateway/platform_registry.py b/gateway/platform_registry.py index 541fb75696f6..e5554e160e2a 100644 --- a/gateway/platform_registry.py +++ b/gateway/platform_registry.py @@ -580,6 +580,10 @@ def plugin_entries(self) -> list[PlatformEntry]: self._resolve_all() return [e for e in self.all_entries() if e.source == "plugin"] + def registered_names(self) -> set[str]: + """Return concrete and deferred platform names without loading adapters.""" + return self._entries.keys() | self._deferred.keys() + def is_registered(self, name: str) -> bool: # A deferred (not-yet-imported) platform still counts as registered -- # the loader will materialize it on first real use. This keeps cheap diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index e3ff5a977802..65e9f9e36e52 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -5515,6 +5515,27 @@ def _print_tools_list(enabled_toolsets: set, mcp_servers: dict, platform: str = _print_info(f"{srv_name} {color('all tools enabled', Colors.DIM)}") +def _known_tool_platforms() -> set[str]: + """Return built-in plus discovered plugin platform names. + + Plugin platforms are registered at runtime rather than in the static CLI + display registry. Tool introspection/configuration must recognize those + names too, otherwise an active plugin platform cannot audit its authority. + """ + known = set(PLATFORMS) + try: + from hermes_cli.plugins import discover_plugins + from gateway.platform_registry import platform_registry + + discover_plugins() # idempotent + known.update(platform_registry.registered_names()) + except Exception: + # Plugin discovery is optional. Preserve the built-in CLI path when a + # third-party plugin is malformed or its dependencies are unavailable. + pass + return known + + def tools_disable_enable_command(args): """Enable, disable, or list tools for a platform. @@ -5525,8 +5546,9 @@ def tools_disable_enable_command(args): platform = getattr(args, "platform", "cli") config = load_config() - if platform not in PLATFORMS: - _print_error(f"Unknown platform '{platform}'. Valid: {', '.join(PLATFORMS)}") + valid_platforms = _known_tool_platforms() + if platform not in valid_platforms: + _print_error(f"Unknown platform '{platform}'. Valid: {', '.join(sorted(valid_platforms))}") return if action == "list": diff --git a/tests/gateway/test_platform_registry.py b/tests/gateway/test_platform_registry.py index 8a0442cb0209..24040c8ba33f 100644 --- a/tests/gateway/test_platform_registry.py +++ b/tests/gateway/test_platform_registry.py @@ -99,6 +99,18 @@ def test_create_adapter_no_validate(self): reg.register(entry) assert reg.create_adapter("novalidate", MagicMock()) is mock_adapter + def test_registered_names_includes_deferred_without_materializing(self): + reg = PlatformRegistry() + entry, _ = self._make_entry("concrete") + loader = MagicMock() + reg.register(entry) + reg.register_deferred("deferred", loader) + + assert reg.registered_names() == {"concrete", "deferred"} + loader.assert_not_called() + assert reg.get("concrete") is entry + assert reg.is_registered("deferred") + class TestEnsureDepsFn: """check_fn (PASSIVE probe) vs ensure_deps_fn (ACTIVE installer) split. diff --git a/tests/hermes_cli/test_tools_disable_enable.py b/tests/hermes_cli/test_tools_disable_enable.py index 0b462645bbca..9d5684840caa 100644 --- a/tests/hermes_cli/test_tools_disable_enable.py +++ b/tests/hermes_cli/test_tools_disable_enable.py @@ -1,7 +1,10 @@ """Tests for hermes tools disable/enable/list command (backend).""" from argparse import Namespace -from unittest.mock import patch +from unittest.mock import MagicMock, patch +import pytest + +from gateway.platform_registry import platform_registry from hermes_cli.tools_config import tools_disable_enable_command @@ -79,3 +82,40 @@ def test_mixed_valid_and_invalid_applies_valid_only(self): saved = mock_save.call_args[0][0] assert "web" not in saved["platform_toolsets"]["cli"] assert "memory" in saved["platform_toolsets"]["cli"] + + +@pytest.mark.parametrize("action", ["list", "enable", "disable"]) +def test_tools_action_accepts_deferred_plugin_without_materializing(action, capsys): + platform = "deferred-tools-test" + loader = MagicMock() + configured_tools = ["memory", "web"] if action == "disable" else ["memory"] + config = {"platform_toolsets": {platform: configured_tools}} + args = Namespace(tools_action=action, platform=platform) + if action != "list": + args.names = ["web"] + + def discover_deferred_platform(): + platform_registry.register_deferred(platform, loader) + + try: + with patch( + "hermes_cli.plugins.discover_plugins", + side_effect=discover_deferred_platform, + ) as discover, \ + patch("hermes_cli.tools_config.load_config", return_value=config), \ + patch("hermes_cli.tools_config.save_config") as save: + tools_disable_enable_command(args) + + out = capsys.readouterr().out + assert "Unknown platform" not in out + discover.assert_called() + loader.assert_not_called() + if action == "list": + assert f"Built-in toolsets ({platform}):" in out + save.assert_not_called() + else: + save.assert_called() + saved_tools = save.call_args.args[0]["platform_toolsets"][platform] + assert ("web" in saved_tools) is (action == "enable") + finally: + platform_registry.unregister(platform) From 3412f049a58226d82b28dc59750520698588c3ce Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Fri, 14 Aug 2026 20:49:50 -0700 Subject: [PATCH 4/5] chore: map contributor email for attribution audit --- contributors/emails/eman1369a@gmail.com | 1 + 1 file changed, 1 insertion(+) create mode 100644 contributors/emails/eman1369a@gmail.com diff --git a/contributors/emails/eman1369a@gmail.com b/contributors/emails/eman1369a@gmail.com new file mode 100644 index 000000000000..a52314448825 --- /dev/null +++ b/contributors/emails/eman1369a@gmail.com @@ -0,0 +1 @@ +thelonewander3r From dcb52417353ce3fb00b44eceb9bc78a0600112e1 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Fri, 14 Aug 2026 20:51:26 -0700 Subject: [PATCH 5/5] fix(gateway): registered_names() honors profile scope like is_registered() The salvaged registered_names() from PR #71582 predates the scoped platform registry: it read only the process-global _entries/_deferred maps, but plugin platforms register their deferred loaders under a profile scope. Result: `hermes tools enable a2a --platform a2a` still rejected the platform. Union the current-scope maps with the global ones, mirroring is_registered()'s semantics, under the registry lock. --- gateway/platform_registry.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/gateway/platform_registry.py b/gateway/platform_registry.py index e5554e160e2a..e639bc838bd1 100644 --- a/gateway/platform_registry.py +++ b/gateway/platform_registry.py @@ -581,8 +581,22 @@ def plugin_entries(self) -> list[PlatformEntry]: return [e for e in self.all_entries() if e.source == "plugin"] def registered_names(self) -> set[str]: - """Return concrete and deferred platform names without loading adapters.""" - return self._entries.keys() | self._deferred.keys() + """Return concrete and deferred platform names without loading adapters. + + Mirrors ``is_registered()``'s scope semantics: names registered under + the current profile scope AND process-global names both count. Plugin + platforms register deferred loaders under a profile scope, so reading + only the global maps would miss every plugin platform. + """ + with self._lock: + scope = self.current_scope_key() + entries, deferred = self._scope_maps(scope) + return ( + entries.keys() + | deferred.keys() + | self._entries.keys() + | self._deferred.keys() + ) def is_registered(self, name: str) -> bool: # A deferred (not-yet-imported) platform still counts as registered --