diff --git a/gateway/platforms/slack.py b/gateway/platforms/slack.py index 90bb4481c6e..6aac3bf4806 100644 --- a/gateway/platforms/slack.py +++ b/gateway/platforms/slack.py @@ -890,6 +890,18 @@ class SlackAdapter(BasePlatformAdapter): async def handle_file_change(event, say): pass + # Reactions are useful lightweight acknowledgements in Slack, but + # Hermes does not currently need to route them into the agent loop. + # Ack the events explicitly so high-traffic channels do not fill + # gateway.error.log with Slack Bolt "Unhandled request" warnings. + @self._app.event("reaction_added") + async def handle_reaction_added(event, say): + pass + + @self._app.event("reaction_removed") + async def handle_reaction_removed(event, say): + pass + @self._app.event("assistant_thread_started") async def handle_assistant_thread_started(event, say): await self._handle_assistant_thread_lifecycle_event(event) diff --git a/hermes_cli/plugins.py b/hermes_cli/plugins.py index 3de41edf462..8d1e3ca9e80 100644 --- a/hermes_cli/plugins.py +++ b/hermes_cli/plugins.py @@ -1129,6 +1129,7 @@ class PluginManager: self._hooks.clear() self._middleware.clear() self._plugin_tool_names.clear() + self._plugin_platform_names.clear() self._cli_commands.clear() self._plugin_commands.clear() self._plugin_skills.clear() @@ -1531,39 +1532,35 @@ class PluginManager: logger.warning("Plugin '%s' has no register() function", manifest.name) else: ctx = PluginContext(manifest, self) + # Snapshot registry state BEFORE register() so each registry's + # attribution counts only what THIS plugin actually added. + # The previous approach diffed names against all already-loaded + # plugins, which mis-credited a plugin that registered a hook / + # middleware / tool name an earlier plugin had already used: + # the shared name was attributed to the first plugin only, so + # later plugins under-reported in `hermes plugins list`. + _tools_before = set(self._plugin_tool_names) + _hook_counts_before = { + h: len(cbs) for h, cbs in self._hooks.items() + } + _mw_counts_before = { + kind: len(cbs) for kind, cbs in self._middleware.items() + } register_fn(ctx) loaded.tools_registered = [ t for t in self._plugin_tool_names - if t not in { - n - for name, p in self._plugins.items() - for n in p.tools_registered - } + if t not in _tools_before + ] + loaded.hooks_registered = [ + h + for h, cbs in self._hooks.items() + if len(cbs) > _hook_counts_before.get(h, 0) + ] + loaded.middleware_registered = [ + kind + for kind, cbs in self._middleware.items() + if len(cbs) > _mw_counts_before.get(kind, 0) ] - loaded.hooks_registered = list( - { - h - for h, cbs in self._hooks.items() - if cbs # non-empty - } - - { - h - for name, p in self._plugins.items() - for h in p.hooks_registered - } - ) - loaded.middleware_registered = list( - { - kind - for kind, cbs in self._middleware.items() - if cbs - } - - { - kind - for name, p in self._plugins.items() - for kind in p.middleware_registered - } - ) loaded.commands_registered = [ c for c in self._plugin_commands if self._plugin_commands[c].get("plugin") == manifest.name diff --git a/tests/gateway/test_slack.py b/tests/gateway/test_slack.py index 97618f4482a..9654927ef2a 100644 --- a/tests/gateway/test_slack.py +++ b/tests/gateway/test_slack.py @@ -234,6 +234,8 @@ class TestAppMentionHandler: assert "message" in registered_events assert "app_mention" in registered_events + assert "reaction_added" in registered_events + assert "reaction_removed" in registered_events assert "assistant_thread_started" in registered_events assert "assistant_thread_context_changed" in registered_events # Slack slash commands are registered via a single regex matcher diff --git a/tests/hermes_cli/test_plugins.py b/tests/hermes_cli/test_plugins.py index 6e424e98450..effeaa0120f 100644 --- a/tests/hermes_cli/test_plugins.py +++ b/tests/hermes_cli/test_plugins.py @@ -439,6 +439,50 @@ class TestPluginDiscovery: assert "ep_plugin" in mgr._plugins + def test_force_rediscover_clears_all_plugin_registries(self, monkeypatch): + """force=True must clear every plugin-populated registry. + + Regression: ``_plugin_platform_names`` was populated by + ``register_platform`` but omitted from the ``discover_and_load(force=True)`` + clear block, so a platform plugin disabled between force-rediscovers + left a stale entry behind forever (the set diverged from the real + platform_registry / _plugins truth). This asserts the clear block + empties the full set of per-plugin registries so no future addition + silently leaks across a force pass either. + """ + mgr = PluginManager() + + # Seed every registry that a plugin's register() can populate, then + # mark discovery done so force=True takes the clear path (we stub the + # inner sweep so the test doesn't depend on any on-disk plugins). + mgr._plugins["p"] = MagicMock() + mgr._hooks["pre_tool_call"] = [lambda **_: None] + mgr._middleware["llm_request"] = [lambda **_: None] + mgr._plugin_tool_names.add("some_tool") + mgr._plugin_platform_names.add("irc") + mgr._cli_commands["c"] = {"plugin": "p"} + mgr._plugin_commands["cmd"] = {"plugin": "p"} + mgr._plugin_skills["p:skill"] = {} + mgr._aux_tasks["task"] = {"plugin": "p"} + mgr._slack_action_handlers.append(("aid", lambda **_: None, "p")) + mgr._discovered = True + + monkeypatch.setattr(PluginManager, "_discover_and_load_inner", lambda self_inner: None) + mgr.discover_and_load(force=True) + + assert mgr._plugins == {} + assert mgr._hooks == {} + assert mgr._middleware == {} + assert mgr._plugin_tool_names == set() + assert mgr._plugin_platform_names == set(), ( + "_plugin_platform_names was not cleared on force-rediscover" + ) + assert mgr._cli_commands == {} + assert mgr._plugin_commands == {} + assert mgr._plugin_skills == {} + assert mgr._aux_tasks == {} + assert mgr._slack_action_handlers == [] + # ── TestPluginLoading ────────────────────────────────────────────────────── @@ -1159,6 +1203,36 @@ class TestPluginManagerList: assert "tools" in p assert "hooks" in p + def test_shared_hook_name_credited_to_every_plugin(self, tmp_path, monkeypatch): + """Two plugins registering the SAME hook name are each credited. + + Regression: hook/middleware/tool attribution diffed names against all + already-loaded plugins, so when a later plugin registered a hook name + an earlier plugin had already used, the shared name was attributed to + the first plugin only and the later plugin reported 0 hooks in + `hermes plugins list`. Attribution now counts what each plugin's own + register() added (per-registration delta), so both get credit. + """ + plugins_dir = tmp_path / "hermes_test" / "plugins" + _make_plugin_dir( + plugins_dir, "first_hooker", + register_body='ctx.register_hook("post_tool_call", lambda **kw: None)', + ) + _make_plugin_dir( + plugins_dir, "second_hooker", + register_body='ctx.register_hook("post_tool_call", lambda **kw: None)', + ) + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes_test")) + + mgr = PluginManager() + mgr.discover_and_load() + + by_name = {p["name"]: p for p in mgr.list_plugins()} + assert by_name["first_hooker"]["hooks"] == 1 + assert by_name["second_hooker"]["hooks"] == 1, ( + "second plugin sharing a hook name was not credited with its hook" + ) + class TestPreLlmCallTargetRouting: