Merge pull request #44674 from kshitijk4poor/fix/slack-reactions-plugin-registry-bookkeeping

fix(plugins,slack): registry bookkeeping fixes + ack reaction events (salvage #42561)

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)

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

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

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: