From 62e937bf2b1ddc9880554a08462417ac85199bc3 Mon Sep 17 00:00:00 2001 From: Brad Smith Date: Wed, 6 May 2026 01:23:33 -0500 Subject: [PATCH] feat(plugins): expose register_slack_action_handler API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plugins that post Block Kit messages with interactive elements (buttons, overflow menus, datepickers, etc.) had no documented way to receive the resulting click events. The plugin API exposed register_tool, register_hook, register_command, register_platform, and register_context_engine, but nothing for slack_bolt action handlers. The only workaround was to monkey-patch SlackAdapter.connect from inside register(), which is fragile and breaks on every Hermes update. This change adds: * PluginContext.register_slack_action_handler(action_id, callback) — validates inputs and queues the handler on the PluginManager. action_id accepts whatever slack_bolt.App.action() accepts (literal string, compiled re.Pattern, or constraint dict). * PluginManager.get_slack_action_handlers() — accessor used by the Slack adapter at connect time. * SlackAdapter.connect — after wiring its built-in approval and slash-confirm buttons, iterates the plugin-registered handlers and registers each via self._app.action(matcher)(callback). Each callback is wrapped defensively so a misbehaving plugin cannot crash slack_bolt's dispatch loop, with a best-effort ack on exception so Slack stops retrying the click. * Defensive fallback when the plugin layer is unhealthy: a RuntimeError from get_plugin_manager() is logged and swallowed rather than blocking the gateway from starting. * Test coverage in tests/gateway/test_slack_plugin_action_handlers.py for input validation, multi-plugin registration, the connect-time wiring, defensive exception handling, and the plugin-loader- failure fallback path. * Documentation in website/docs/guides/build-a-hermes-plugin.md describing the new API alongside the existing register_command / dispatch_tool documentation. Co-Authored-By: Claude Opus 4.7 (1M context) --- gateway/platforms/slack.py | 45 ++ hermes_cli/plugins.py | 82 ++++ .../test_slack_plugin_action_handlers.py | 395 ++++++++++++++++++ website/docs/guides/build-a-hermes-plugin.md | 33 ++ 4 files changed, 555 insertions(+) create mode 100644 tests/gateway/test_slack_plugin_action_handlers.py diff --git a/gateway/platforms/slack.py b/gateway/platforms/slack.py index 1224922271a..55b77c324de 100644 --- a/gateway/platforms/slack.py +++ b/gateway/platforms/slack.py @@ -949,6 +949,51 @@ class SlackAdapter(BasePlatformAdapter): ): self._app.action(_action_id)(self._handle_slash_confirm_action) + # Register plugin-provided Block Kit action handlers. + # + # Plugins call ``ctx.register_slack_action_handler(action_id, cb)`` + # at register() time; the manager queues them and the adapter + # wires them into AsyncApp here so slack_bolt's matcher knows + # about them before Socket Mode starts dispatching events. + # + # Each callback is wrapped so a misbehaving plugin can't take + # down the gateway: any exception inside the plugin handler is + # caught and logged, and slack_bolt still sees a clean ack. + try: + from hermes_cli.plugins import get_plugin_manager + _plugin_handlers = get_plugin_manager().get_slack_action_handlers() + except Exception as e: # pragma: no cover - defensive + logger.warning( + "[Slack] Could not load plugin action handlers: %s", e, + ) + _plugin_handlers = [] + + for _action_id, _cb, _plugin_name in _plugin_handlers: + # Capture loop vars per iteration via default args. + async def _wrapped(ack, body, action, _cb=_cb, _plugin_name=_plugin_name): + try: + await _cb(ack, body, action) + except Exception as exc: # pragma: no cover - defensive + logger.error( + "[Slack] Plugin '%s' action handler raised: %s", + _plugin_name, exc, exc_info=True, + ) + # Best-effort ack so Slack doesn't retry the click. + try: + await ack() + except Exception: + pass + self._app.action(_action_id)(_wrapped) + logger.debug( + "[Slack] Registered plugin action handler %s (from %s)", + _action_id, _plugin_name, + ) + if _plugin_handlers: + logger.info( + "[Slack] Wired %d plugin action handler(s)", + len(_plugin_handlers), + ) + # Bring up the handler and watchdog atomically. ``_running`` only # flips to True after the handler is alive so the watchdog loop # observes the live task immediately; on any failure here we tear diff --git a/hermes_cli/plugins.py b/hermes_cli/plugins.py index 3fd2bb3fe97..3de41edf462 100644 --- a/hermes_cli/plugins.py +++ b/hermes_cli/plugins.py @@ -821,6 +821,64 @@ class PluginContext: name, ) + # -- slack action handler registration ---------------------------------- + + def register_slack_action_handler( + self, + action_id: Any, + callback: Callable, + ) -> None: + """Register a Slack Block Kit action handler from a plugin. + + Hermes' Slack adapter wires registered handlers into its + ``slack_bolt.AsyncApp`` at connect time. The callback is invoked + when a user clicks a button (or interacts with another Block Kit + action element) whose ``action_id`` matches. + + Callback signature follows the slack_bolt convention:: + + async def handler(ack, body, action) -> None: + await ack() # required, within 3 seconds + ... + + Args: + action_id: Whatever ``slack_bolt.App.action()`` accepts — + a literal ``action_id`` string, a compiled ``re.Pattern`` + for matching multiple ids, or a constraint dict + (e.g. ``{"action_id": "...", "block_id": "..."}``). + callback: Async callable receiving ``(ack, body, action)``. + + Raises: + ValueError: if ``callback`` is not callable, or ``action_id`` + is empty/None. + + Example:: + + async def _on_approve(ack, body, action): + await ack() + # apply some workflow keyed on action["value"] + + ctx.register_slack_action_handler("inbox_sweep_approve", _on_approve) + """ + if not callable(callback): + raise ValueError( + f"Plugin '{self.manifest.name}' tried to register a Slack " + f"action handler with a non-callable callback." + ) + if action_id is None or (isinstance(action_id, str) and not action_id.strip()): + raise ValueError( + f"Plugin '{self.manifest.name}' tried to register a Slack " + f"action handler with an empty action_id." + ) + self._manager._slack_action_handlers.append( + (action_id, callback, self.manifest.name) + ) + logger.debug( + "Plugin %s registered Slack action handler: %s", + self.manifest.name, + action_id, + ) + # -- hook registration -------------------------------------------------- # -- auxiliary task registration --------------------------------------- @@ -1045,6 +1103,13 @@ class PluginManager: # Plugin-registered auxiliary tasks: key → {key, display_name, # description, defaults, plugin}. See PluginContext.register_auxiliary_task. self._aux_tasks: Dict[str, Dict[str, Any]] = {} + # Slack Block Kit action handlers registered by plugins. Each entry + # is (matcher, callback, plugin_name); the Slack adapter wires them + # into its slack_bolt App at connect() time. ``matcher`` is whatever + # ``app.action()`` accepts (a literal action_id string, a compiled + # ``re.Pattern``, or a constraint dict); ``callback`` is an async + # function with the slack_bolt signature ``(ack, body, action)``. + self._slack_action_handlers: List[tuple] = [] # ----------------------------------------------------------------------- # Public @@ -1068,6 +1133,7 @@ class PluginManager: self._plugin_commands.clear() self._plugin_skills.clear() self._aux_tasks.clear() + self._slack_action_handlers.clear() self._context_engine = None # Set the flag up front as a re-entrancy guard (a plugin's register() # can transitively trigger discovery again), but reset it if the sweep @@ -1652,6 +1718,22 @@ class PluginManager: ) return results + # ----------------------------------------------------------------------- + # Slack action handler accessor + # ----------------------------------------------------------------------- + + def get_slack_action_handlers(self) -> List[tuple]: + """Return the list of plugin-registered Slack action handlers. + + Each entry is a ``(action_id, callback, plugin_name)`` tuple. + Consumed by the Slack adapter at connect time to wire callbacks + into its ``slack_bolt.AsyncApp``. + + Plugins register handlers via + :meth:`PluginContext.register_slack_action_handler`. + """ + return list(self._slack_action_handlers) + # ----------------------------------------------------------------------- # Introspection # ----------------------------------------------------------------------- diff --git a/tests/gateway/test_slack_plugin_action_handlers.py b/tests/gateway/test_slack_plugin_action_handlers.py new file mode 100644 index 00000000000..2aeba4e7af6 --- /dev/null +++ b/tests/gateway/test_slack_plugin_action_handlers.py @@ -0,0 +1,395 @@ +"""Tests for plugin-registered Slack Block Kit action handlers. + +Covers: +* ``PluginContext.register_slack_action_handler`` validation + queuing +* ``PluginManager.get_slack_action_handlers`` accessor +* ``SlackAdapter.connect`` wiring those handlers into the AsyncApp +* Defensive wrapping: a plugin handler that raises does NOT take down + the gateway and Slack still gets an ack. +""" + +from __future__ import annotations + +import asyncio +import os +import sys +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + + +# --------------------------------------------------------------------------- +# Ensure the repo root is importable when this test runs directly +# --------------------------------------------------------------------------- +_repo = str(Path(__file__).resolve().parents[2]) +if _repo not in sys.path: + sys.path.insert(0, _repo) + + +# --------------------------------------------------------------------------- +# Mock slack-bolt so SlackAdapter can be imported even without the package +# --------------------------------------------------------------------------- + +def _ensure_slack_mock() -> None: + if "slack_bolt" in sys.modules and hasattr(sys.modules["slack_bolt"], "__file__"): + return + slack_bolt = MagicMock() + slack_bolt.async_app.AsyncApp = MagicMock + slack_bolt.adapter.socket_mode.async_handler.AsyncSocketModeHandler = MagicMock + + slack_sdk = MagicMock() + slack_sdk.web.async_client.AsyncWebClient = MagicMock + + for name, mod in [ + ("slack_bolt", slack_bolt), + ("slack_bolt.async_app", slack_bolt.async_app), + ("slack_bolt.adapter", slack_bolt.adapter), + ("slack_bolt.adapter.socket_mode", slack_bolt.adapter.socket_mode), + ("slack_bolt.adapter.socket_mode.async_handler", + slack_bolt.adapter.socket_mode.async_handler), + ("slack_sdk", slack_sdk), + ("slack_sdk.web", slack_sdk.web), + ("slack_sdk.web.async_client", slack_sdk.web.async_client), + ]: + sys.modules.setdefault(name, mod) + sys.modules.setdefault("aiohttp", MagicMock()) + + +_ensure_slack_mock() + +import gateway.platforms.slack as _slack_mod # noqa: E402 +_slack_mod.SLACK_AVAILABLE = True + +from gateway.config import PlatformConfig # noqa: E402 +from gateway.platforms.slack import SlackAdapter # noqa: E402 + +from hermes_cli.plugins import ( # noqa: E402 + PluginContext, + PluginManager, + PluginManifest, +) + + +# --------------------------------------------------------------------------- +# PluginContext.register_slack_action_handler — input validation + queuing +# --------------------------------------------------------------------------- + +def _make_ctx(name: str = "test_plugin") -> tuple[PluginManager, PluginContext]: + """Build a fresh PluginManager + PluginContext bound to it.""" + mgr = PluginManager() + manifest = PluginManifest( + name=name, + version="0.1.0", + description="test", + ) + ctx = PluginContext(manifest=manifest, manager=mgr) + return mgr, ctx + + +class TestRegisterSlackActionHandlerAPI: + """Behaviour of ctx.register_slack_action_handler().""" + + def test_string_action_id_is_queued(self): + mgr, ctx = _make_ctx() + + async def cb(ack, body, action): # pragma: no cover - never called here + await ack() + + ctx.register_slack_action_handler("inbox_sweep_approve", cb) + + handlers = mgr.get_slack_action_handlers() + assert len(handlers) == 1 + action_id, callback, plugin_name = handlers[0] + assert action_id == "inbox_sweep_approve" + assert callback is cb + assert plugin_name == "test_plugin" + + def test_regex_action_id_is_accepted(self): + """slack_bolt accepts re.Pattern matchers — so should the plugin API.""" + import re as _re + mgr, ctx = _make_ctx() + + async def cb(ack, body, action): # pragma: no cover + await ack() + + pat = _re.compile(r"^inbox_sweep_.*$") + ctx.register_slack_action_handler(pat, cb) + handlers = mgr.get_slack_action_handlers() + assert handlers[0][0] is pat + + def test_constraint_dict_action_id_is_accepted(self): + """slack_bolt also accepts {"action_id": ..., "block_id": ...} dicts.""" + mgr, ctx = _make_ctx() + + async def cb(ack, body, action): # pragma: no cover + await ack() + + constraint = {"action_id": "approve", "block_id": "row_3"} + ctx.register_slack_action_handler(constraint, cb) + handlers = mgr.get_slack_action_handlers() + assert handlers[0][0] == constraint + + def test_non_callable_callback_raises(self): + _mgr, ctx = _make_ctx() + with pytest.raises(ValueError, match="non-callable"): + ctx.register_slack_action_handler("approve", "not a function") # type: ignore[arg-type] + + def test_empty_string_action_id_raises(self): + _mgr, ctx = _make_ctx() + + async def cb(ack, body, action): # pragma: no cover + await ack() + + with pytest.raises(ValueError, match="empty action_id"): + ctx.register_slack_action_handler(" ", cb) + + def test_none_action_id_raises(self): + _mgr, ctx = _make_ctx() + + async def cb(ack, body, action): # pragma: no cover + await ack() + + with pytest.raises(ValueError, match="empty action_id"): + ctx.register_slack_action_handler(None, cb) + + def test_get_slack_action_handlers_returns_copy(self): + """The accessor should return a copy so callers can't mutate state.""" + mgr, ctx = _make_ctx() + + async def cb(ack, body, action): # pragma: no cover + await ack() + + ctx.register_slack_action_handler("a", cb) + + handlers = mgr.get_slack_action_handlers() + handlers.clear() + assert len(mgr.get_slack_action_handlers()) == 1 + + def test_multiple_plugins_each_recorded(self): + mgr = PluginManager() + ctx_a = PluginContext( + manifest=PluginManifest(name="plug_a", version="0", description=""), + manager=mgr, + ) + ctx_b = PluginContext( + manifest=PluginManifest(name="plug_b", version="0", description=""), + manager=mgr, + ) + + async def cb_a(ack, body, action): # pragma: no cover + await ack() + + async def cb_b(ack, body, action): # pragma: no cover + await ack() + + ctx_a.register_slack_action_handler("approve", cb_a) + ctx_b.register_slack_action_handler("decline", cb_b) + + handlers = mgr.get_slack_action_handlers() + assert {h[2] for h in handlers} == {"plug_a", "plug_b"} + + +# --------------------------------------------------------------------------- +# SlackAdapter.connect wires plugin-registered handlers into AsyncApp +# --------------------------------------------------------------------------- + + +def _connect_with_recording_app( + adapter: SlackAdapter, + *, + plugin_handlers: list, +) -> tuple[bool, list]: + """Run adapter.connect() with mocks and return (result, registered_actions). + + Captures every action_id passed to ``app.action()`` so tests can + assert that built-in handlers AND plugin-supplied handlers were + wired up. + """ + registered_actions: list = [] # list of (action_id, callback) + + def mock_action(action_id): + def decorator(fn): + registered_actions.append((action_id, fn)) + return fn + return decorator + + def mock_event(_event_type): + def decorator(fn): + return fn + return decorator + + def mock_command(_cmd): + def decorator(fn): + return fn + return decorator + + mock_app = MagicMock() + mock_app.event = mock_event + mock_app.command = mock_command + mock_app.action = mock_action + mock_app.client = AsyncMock() + + mock_web_client = AsyncMock() + mock_web_client.auth_test = AsyncMock(return_value={ + "user_id": "U_BOT", + "user": "testbot", + "team_id": "T_FAKE", + "team": "FakeTeam", + }) + + fake_mgr = MagicMock() + fake_mgr.get_slack_action_handlers.return_value = plugin_handlers + + with patch.object(_slack_mod, "AsyncApp", return_value=mock_app), \ + patch.object(_slack_mod, "AsyncWebClient", return_value=mock_web_client), \ + patch.object(_slack_mod, "AsyncSocketModeHandler", return_value=MagicMock()), \ + patch.dict(os.environ, {"SLACK_APP_TOKEN": "xapp-fake"}), \ + patch("gateway.status.acquire_scoped_lock", return_value=(True, None)), \ + patch("gateway.status.release_scoped_lock"), \ + patch("hermes_cli.plugins.get_plugin_manager", return_value=fake_mgr), \ + patch("asyncio.create_task"): + result = asyncio.run(adapter.connect()) + + return result, registered_actions + + +class TestSlackAdapterPluginActionWiring: + """connect() must register plugin-supplied action handlers on AsyncApp.""" + + def test_plugin_handler_wired_into_app(self): + config = PlatformConfig(enabled=True, token="xoxb-fake") + adapter = SlackAdapter(config) + + async def my_handler(ack, body, action): # pragma: no cover - not invoked + await ack() + + plugin_handlers = [("inbox_sweep_approve", my_handler, "jarvis")] + result, registered = _connect_with_recording_app( + adapter, plugin_handlers=plugin_handlers, + ) + + assert result is True + action_ids = [aid for aid, _cb in registered] + # Built-in approval buttons remain registered… + assert "hermes_approve_once" in action_ids + assert "hermes_deny" in action_ids + # …and the plugin's action_id was added. + assert "inbox_sweep_approve" in action_ids + + def test_no_plugin_handlers_does_not_break_connect(self): + """An empty plugin handler list is the common case — must be a no-op.""" + config = PlatformConfig(enabled=True, token="xoxb-fake") + adapter = SlackAdapter(config) + + result, registered = _connect_with_recording_app( + adapter, plugin_handlers=[], + ) + assert result is True + # Built-ins still wired + action_ids = [aid for aid, _cb in registered] + assert "hermes_approve_once" in action_ids + + def test_plugin_exception_does_not_propagate_to_slack(self): + """A misbehaving plugin handler must NOT crash slack_bolt's dispatch. + + The wrapper installed by connect() catches exceptions, logs them, + and best-effort-acks so Slack stops retrying the click. + """ + config = PlatformConfig(enabled=True, token="xoxb-fake") + adapter = SlackAdapter(config) + + async def boom(ack, body, action): + raise RuntimeError("plugin bug") + + plugin_handlers = [("explode", boom, "buggy_plugin")] + _result, registered = _connect_with_recording_app( + adapter, plugin_handlers=plugin_handlers, + ) + + wrapped = next(cb for aid, cb in registered if aid == "explode") + ack = AsyncMock() + body = {"foo": "bar"} + action = {"action_id": "explode", "value": "x"} + + # Wrapper must swallow the RuntimeError. + asyncio.run(wrapped(ack, body, action)) + + # Slack still got an ack — best-effort fallback after exception. + ack.assert_awaited() + + def test_plugin_handler_invoked_with_slack_args(self): + """Happy path: the plugin's callback receives (ack, body, action).""" + config = PlatformConfig(enabled=True, token="xoxb-fake") + adapter = SlackAdapter(config) + + seen: dict = {} + + async def cb(ack, body, action): + seen["body"] = body + seen["action"] = action + await ack() + + plugin_handlers = [("approve_x", cb, "plug_x")] + _result, registered = _connect_with_recording_app( + adapter, plugin_handlers=plugin_handlers, + ) + + wrapped = next(c for aid, c in registered if aid == "approve_x") + ack = AsyncMock() + asyncio.run(wrapped(ack, {"b": 1}, {"action_id": "approve_x"})) + + ack.assert_awaited_once_with() + assert seen["body"] == {"b": 1} + assert seen["action"] == {"action_id": "approve_x"} + + def test_plugin_loader_failure_does_not_break_connect(self): + """If get_plugin_manager() blows up, connect() must still succeed. + + Defensive belt-and-suspenders: the gateway should not refuse to + start because the plugin layer is unhealthy. + """ + config = PlatformConfig(enabled=True, token="xoxb-fake") + adapter = SlackAdapter(config) + + registered_actions: list = [] + + def mock_action(action_id): + def decorator(fn): + registered_actions.append((action_id, fn)) + return fn + return decorator + + def _noop(_): + def decorator(fn): return fn + return decorator + + mock_app = MagicMock() + mock_app.event = _noop + mock_app.command = _noop + mock_app.action = mock_action + mock_app.client = AsyncMock() + + mock_web_client = AsyncMock() + mock_web_client.auth_test = AsyncMock(return_value={ + "user_id": "U_BOT", + "user": "testbot", + "team_id": "T_FAKE", + "team": "FakeTeam", + }) + + with patch.object(_slack_mod, "AsyncApp", return_value=mock_app), \ + patch.object(_slack_mod, "AsyncWebClient", return_value=mock_web_client), \ + patch.object(_slack_mod, "AsyncSocketModeHandler", return_value=MagicMock()), \ + patch.dict(os.environ, {"SLACK_APP_TOKEN": "xapp-fake"}), \ + patch("gateway.status.acquire_scoped_lock", return_value=(True, None)), \ + patch("gateway.status.release_scoped_lock"), \ + patch("hermes_cli.plugins.get_plugin_manager", + side_effect=RuntimeError("plugins broken")), \ + patch("asyncio.create_task"): + result = asyncio.run(adapter.connect()) + + assert result is True + # Built-ins still wired even when plugin loader failed. + action_ids = [aid for aid, _cb in registered_actions] + assert "hermes_approve_once" in action_ids diff --git a/website/docs/guides/build-a-hermes-plugin.md b/website/docs/guides/build-a-hermes-plugin.md index 1b52f56683f..a48db94ff94 100644 --- a/website/docs/guides/build-a-hermes-plugin.md +++ b/website/docs/guides/build-a-hermes-plugin.md @@ -827,6 +827,39 @@ def register(ctx): This is the public, stable interface for tool dispatch from plugin commands. Plugins should not reach into `ctx._cli_ref.agent` or similar private state. +### Handle Slack Block Kit button clicks + +Plugins that post Block Kit messages with interactive elements (buttons, overflow menus, datepickers, etc.) can register the click handlers directly with the Slack adapter — no monkey-patching of `slack_bolt.AsyncApp` required. + +```python +def register(ctx): + async def _on_approve(ack, body, action): + # ack within 3 seconds — slack_bolt requirement. + await ack() + # body["channel"]["id"], body["user"]["id"], body["message"]["ts"] + # action["action_id"], action["value"] + sweep_id = (action.get("value") or "").split("|", 1)[-1] + # ...do the deterministic work, then post a follow-up. + + ctx.register_slack_action_handler("inbox_sweep_approve", _on_approve) +``` + +**Signature:** `ctx.register_slack_action_handler(action_id, callback) -> None` + +| Parameter | Type | Description | +|-----------|------|-------------| +| `action_id` | `str \| re.Pattern \| dict` | Whatever `slack_bolt.App.action()` accepts: a literal `action_id`, a compiled regex matching multiple ids, or a constraint dict like `{"action_id": "...", "block_id": "..."}` | +| `callback` | async callable | Receives `(ack, body, action)` per the slack_bolt convention | + +**Runtime behavior:** + +- The handler is queued at plugin-load time and wired into the adapter's `slack_bolt.AsyncApp` when the Slack platform connects. +- Each callback is wrapped defensively: if your handler raises, the gateway logs the error and best-effort-acks the click so Slack stops retrying. +- Standard slack_bolt rules apply — `await ack()` within 3 seconds, then do longer work. +- For multi-workspace deployments the handler fires for clicks from any connected workspace; use `body["team"]["id"]` if you need to scope behaviour. + +This is the public way for plugins to participate in Slack interactivity. Older plugins may patch `SlackAdapter.connect`; prefer this API instead. + :::tip This guide covers **general plugins** (tools, hooks, slash commands, CLI commands). The sections below sketch the authoring pattern for each specialized plugin type; each links to its full guide for field reference and examples. :::