diff --git a/hermes_cli/plugins.py b/hermes_cli/plugins.py index 80142c15db1d5..eea3cd0b8a5c7 100644 --- a/hermes_cli/plugins.py +++ b/hermes_cli/plugins.py @@ -34,11 +34,13 @@ from __future__ import annotations import asyncio +import concurrent.futures import importlib.metadata import importlib.util import inspect import logging import os +import re import sys import threading import types @@ -580,6 +582,84 @@ def register_command( } logger.debug("Plugin %s registered command: /%s", self.manifest.name, clean) + # -- inline-button callback prefix registration --------------------------- + + # Callback prefixes consumed by the platform adapters' built-in button + # flows. A plugin prefix may not shadow (or be shadowed by) any of these. + _RESERVED_CALLBACK_PREFIXES = ( + "mp:", "mpg:", "mpv:", "mm:", "mc:", "mb", "mx", "mg:", + "cp:", "gt:", "ea:", "sc:", "cl:", "update_prompt:", + ) + _CALLBACK_PREFIX_RE = re.compile(r"^[a-z0-9][a-z0-9_-]{0,14}:$") + + def register_callback_prefix( + self, + prefix: str, + handler: Callable, + description: str = "", + ) -> None: + """Register an inline-button callback prefix (e.g. ``"em:"``). + + Platform adapters route button presses whose ``callback_data`` starts + with *prefix* to *handler* — only after the presser passes the same + authorization check as the built-in approval buttons. The handler + signature is ``fn(data: str) -> str | None`` (sync or async): ``data`` + is the full callback payload including the prefix, and the return + value (truncated by the adapter) becomes the callback answer. + + Handlers own their payload grammar and must validate it fail-closed: + refuse anything malformed rather than guessing. Prefixes are lowercase + ``[a-z0-9_-]``, 2-16 chars, ending in ``:``; built-in prefixes and + prefixes claimed by another plugin are rejected with a warning. + + A non-callable *handler* is a programming error and raises immediately + (as ``register_slack_action_handler`` does) rather than failing later, + when a user presses the button. Prefix rejections stay warn-and-skip: + losing a prefix to a built-in or to another plugin is a policy outcome + a plugin can legitimately survive, not a bug in its own code. + + Raises: + ValueError: if *handler* is not callable. + """ + if not callable(handler): + raise ValueError( + f"Plugin '{self.manifest.name}' tried to register callback " + f"prefix {prefix!r} with a non-callable handler." + ) + clean = (prefix or "").strip() + if not self._CALLBACK_PREFIX_RE.match(clean): + logger.warning( + "Plugin '%s' tried to register invalid callback prefix %r. Skipping.", + self.manifest.name, prefix, + ) + return + if any( + clean.startswith(reserved) or reserved.startswith(clean) + for reserved in self._RESERVED_CALLBACK_PREFIXES + ): + logger.warning( + "Plugin '%s' tried to register callback prefix %r which conflicts " + "with a built-in prefix. Skipping.", + self.manifest.name, prefix, + ) + return + existing = self._manager._callback_prefixes.get(clean) + if existing is not None and existing.get("plugin") != self.manifest.name: + logger.warning( + "Plugin '%s' tried to register callback prefix %r already claimed " + "by plugin '%s'. Skipping.", + self.manifest.name, prefix, existing.get("plugin"), + ) + return + self._manager._callback_prefixes[clean] = { + "handler": handler, + "description": (description or "").strip(), + "plugin": self.manifest.name, + } + logger.debug( + "Plugin %s registered callback prefix: %s", self.manifest.name, clean + ) + # -- tool dispatch ------------------------------------------------------- def dispatch_tool(self, tool_name: str, args: dict, **kwargs) -> str: @@ -1257,6 +1337,7 @@ def __init__(self) -> None: self._cli_commands: Dict[str, dict] = {} self._context_engine = None # Set by a plugin via register_context_engine() self._plugin_commands: Dict[str, dict] = {} # Slash commands registered by plugins + self._callback_prefixes: Dict[str, dict] = {} # Inline-button callback prefixes registered by plugins self._discovered: bool = False self._cli_ref = None # Set by CLI after plugin discovery # Plugin skill registry: qualified name → metadata dict. @@ -1297,6 +1378,7 @@ def discover_and_load(self, force: bool = False) -> None: self._plugin_platform_names.clear() self._cli_commands.clear() self._plugin_commands.clear() + self._callback_prefixes.clear() self._plugin_skills.clear() self._aux_tasks.clear() self._slack_action_handlers.clear() @@ -2348,6 +2430,85 @@ def get_plugin_command_handler(name: str) -> Optional[Callable]: return entry["handler"] if entry else None +def get_plugin_callback_prefix(data: str) -> Optional[tuple]: + """Return ``(prefix, entry)`` for the plugin callback prefix matching *data*. + + ``entry`` is the registration dict (``handler`` / ``plugin`` / + ``description``) stored by ``PluginContext.register_callback_prefix``. + Registered prefixes are colon-terminated and cannot shadow each other or + any built-in prefix, so at most one entry matches. Returns ``None`` when + nothing matches. + """ + registry = _ensure_plugins_discovered()._callback_prefixes + for prefix, entry in registry.items(): + if data.startswith(prefix): + return prefix, entry + return None + + +# Inline-button callbacks are answered while the platform holds the press open, +# so the bound here is tighter than the plugin-command one below. +_PLUGIN_CALLBACK_AWAIT_TIMEOUT_SECS = 15.0 + +# Synchronous handlers run on a small dedicated pool rather than the default +# executor: a plugin that wedges its workers then starves only other callback +# handlers, never every other ``to_thread`` caller in the process. +_PLUGIN_CALLBACK_MAX_WORKERS = 4 +_plugin_callback_executor: Optional[concurrent.futures.ThreadPoolExecutor] = None +_plugin_callback_executor_lock = threading.Lock() + + +def _get_plugin_callback_executor() -> concurrent.futures.ThreadPoolExecutor: + """Return the lazily-created pool used to run sync callback handlers.""" + global _plugin_callback_executor + with _plugin_callback_executor_lock: + if _plugin_callback_executor is None: + _plugin_callback_executor = concurrent.futures.ThreadPoolExecutor( + max_workers=_PLUGIN_CALLBACK_MAX_WORKERS, + thread_name_prefix="hermes-plugin-callback", + ) + return _plugin_callback_executor + + +async def _invoke_plugin_callback_handler(handler: Callable, data: str) -> Any: + """Await *handler* without ever running plugin code on the caller's loop.""" + if inspect.iscoroutinefunction(handler): + return await handler(data) + loop = asyncio.get_running_loop() + result = await loop.run_in_executor(_get_plugin_callback_executor(), handler, data) + # A sync callable may still hand back an awaitable (e.g. an object whose + # ``__call__`` is async); finish it on the loop. + if inspect.isawaitable(result): + return await result + return result + + +async def run_plugin_callback_handler( + handler: Callable, + data: str, + *, + timeout: Optional[float] = _PLUGIN_CALLBACK_AWAIT_TIMEOUT_SECS, +) -> Any: + """Run a plugin inline-button handler off the caller's event loop, bounded. + + Platform adapters call this instead of invoking the registered handler + directly. Async handlers are awaited; synchronous handlers run on a worker + thread, so a blocking handler cannot stall the adapter's update processing. + The whole invocation shares one *timeout* budget, letting the adapter answer + the button press on a deadline instead of waiting indefinitely. + + Raises: + asyncio.TimeoutError: if the handler does not finish within *timeout*. + Like every thread-offload in Python this cancels the *wait*, not the + worker: a runaway sync handler keeps its thread until it returns, it + just stops holding up the answer. Handlers should stay short and do + long work in the background. + """ + return await asyncio.wait_for( + _invoke_plugin_callback_handler(handler, data), timeout=timeout + ) + + _PLUGIN_COMMAND_AWAIT_TIMEOUT_SECS = 30.0 diff --git a/plugins/platforms/telegram/adapter.py b/plugins/platforms/telegram/adapter.py index 55ae362bfe043..a39c1796d1279 100644 --- a/plugins/platforms/telegram/adapter.py +++ b/plugins/platforms/telegram/adapter.py @@ -6458,6 +6458,56 @@ async def _handle_callback_query( ) return + # --- Plugin-registered callback prefixes --- + # Plugins may claim a callback prefix via ctx.register_callback_prefix() + # (e.g. an approvals plugin routing its own inline buttons). Built-in + # prefixes above always win; registration rejects any prefix that could + # shadow them. The presser must pass the same authorization check as + # the built-in approval buttons before the plugin handler ever runs, + # and the handler's answer is bounded before it reaches Telegram. + plugin_match = None + try: + from hermes_cli.plugins import ( + get_plugin_callback_prefix, + run_plugin_callback_handler, + ) + plugin_match = get_plugin_callback_prefix(data) + except Exception as exc: + logger.warning("[%s] plugin callback lookup failed: %s", self.name, exc) + if plugin_match is not None: + plugin_prefix, plugin_entry = plugin_match + caller_id = str(getattr(query.from_user, "id", "")) + if not self._is_callback_user_authorized( + caller_id, + chat_id=query_chat_id, + chat_type=str(query_chat_type) if query_chat_type is not None else None, + thread_id=str(query_thread_id) if query_thread_id is not None else None, + user_name=query_user_name, + ): + await query.answer(text="⛔ You are not authorized to use this button.") + return + try: + plugin_result = await run_plugin_callback_handler( + plugin_entry["handler"], data + ) + except asyncio.TimeoutError: + logger.warning( + "[%s] plugin '%s' callback handler for %r timed out", + self.name, plugin_entry.get("plugin"), plugin_prefix, + ) + await query.answer(text="⏳ Action timed out.") + return + except Exception as exc: + logger.warning( + "[%s] plugin '%s' callback handler for %r failed: %s", + self.name, plugin_entry.get("plugin"), plugin_prefix, exc, + ) + await query.answer(text="❌ Action failed.") + return + plugin_answer = str(plugin_result).strip() if plugin_result else "Done." + await query.answer(text=plugin_answer[:180]) + return + # --- Update prompt callbacks --- if not data.startswith("update_prompt:"): return diff --git a/tests/gateway/test_plugin_callback_prefixes.py b/tests/gateway/test_plugin_callback_prefixes.py new file mode 100644 index 0000000000000..500ee261da976 --- /dev/null +++ b/tests/gateway/test_plugin_callback_prefixes.py @@ -0,0 +1,442 @@ +"""Tests for plugin-registered inline-button callback prefixes. + +Covers ``PluginContext.register_callback_prefix`` validation, the +``get_plugin_callback_prefix`` lookup, and the Telegram adapter dispatch: +authorization before the handler, sync and async handlers, bounded answers, +handler failure containment, and built-in prefixes always winning. + +Mirrors the fixture pattern of test_telegram_clarify_buttons.py. +""" + +import asyncio +import os +import sys +import threading +import time +import types +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +# --------------------------------------------------------------------------- +# Ensure the repo root is importable +# --------------------------------------------------------------------------- +_repo = str(Path(__file__).resolve().parents[2]) +if _repo not in sys.path: + sys.path.insert(0, _repo) + + +# --------------------------------------------------------------------------- +# Minimal Telegram mock so TelegramAdapter can be imported (mirrors +# test_telegram_clarify_buttons.py) +# --------------------------------------------------------------------------- +def _ensure_telegram_mock(): + if "telegram" in sys.modules and hasattr(sys.modules["telegram"], "__file__"): + return + + mod = MagicMock() + mod.ext.ContextTypes.DEFAULT_TYPE = type(None) + mod.constants.ParseMode.MARKDOWN = "Markdown" + mod.constants.ParseMode.MARKDOWN_V2 = "MarkdownV2" + mod.constants.ParseMode.HTML = "HTML" + mod.constants.ChatType.PRIVATE = "private" + mod.constants.ChatType.GROUP = "group" + mod.constants.ChatType.SUPERGROUP = "supergroup" + mod.constants.ChatType.CHANNEL = "channel" + mod.error.NetworkError = type("NetworkError", (OSError,), {}) + mod.error.TimedOut = type("TimedOut", (OSError,), {}) + mod.error.BadRequest = type("BadRequest", (Exception,), {}) + + for name in ("telegram", "telegram.ext", "telegram.constants", "telegram.request"): + sys.modules.setdefault(name, mod) + sys.modules.setdefault("telegram.error", mod.error) + + +_ensure_telegram_mock() + +from gateway.config import PlatformConfig +from hermes_cli.plugins import PluginContext, run_plugin_callback_handler +from plugins.platforms.telegram.adapter import TelegramAdapter + + +def _make_context(plugin_name="p1", registry=None): + manifest = types.SimpleNamespace(name=plugin_name, key=plugin_name) + manager = types.SimpleNamespace(_callback_prefixes=registry if registry is not None else {}) + return PluginContext(manifest, manager), manager._callback_prefixes + + +def _make_adapter(): + config = PlatformConfig(enabled=True, token="test-token", extra={}) + adapter = TelegramAdapter(config) + adapter._bot = AsyncMock() + adapter._app = MagicMock() + return adapter + + +def _make_query(data, user_id="777"): + query = AsyncMock() + query.data = data + query.message = MagicMock() + query.message.chat_id = 12345 + query.message.chat.type = "private" + query.message.message_thread_id = None + query.from_user = MagicMock() + query.from_user.id = user_id + query.from_user.first_name = "Tester" + query.answer = AsyncMock() + query.edit_message_text = AsyncMock() + + update = MagicMock() + update.callback_query = query + return update, query + + +# =========================================================================== +# register_callback_prefix — validation +# =========================================================================== + +class TestRegisterCallbackPrefix: + def test_valid_prefix_stored(self): + ctx, registry = _make_context() + handler = lambda data: "ok" # noqa: E731 + ctx.register_callback_prefix("em:", handler, description="email approvals") + assert registry["em:"]["handler"] is handler + assert registry["em:"]["plugin"] == "p1" + assert registry["em:"]["description"] == "email approvals" + + @pytest.mark.parametrize( + "bad", + ["", "em", ":", "EM:", "em :", "a" * 20 + ":", "e m:", "em::", None], + ) + def test_invalid_shapes_rejected(self, bad): + ctx, registry = _make_context() + ctx.register_callback_prefix(bad, lambda data: None) + assert registry == {} + + @pytest.mark.parametrize( + "reserved", + ["ea:", "gt:", "cp:", "cl:", "sc:", "mp:", "mpg:", "mg:", "mb2:", "mxx:"], + ) + def test_reserved_and_shadowing_rejected(self, reserved): + # mb2:/mxx: start with the built-in bare prefixes mb/mx and would be + # consumed by the built-in branch before ever reaching the registry. + ctx, registry = _make_context() + ctx.register_callback_prefix(reserved, lambda data: None) + assert registry == {} + + def test_other_plugins_prefix_not_stolen(self): + registry = {} + ctx1, _ = _make_context("p1", registry) + ctx2, _ = _make_context("p2", registry) + ctx1.register_callback_prefix("em:", lambda data: "one") + original = registry["em:"]["handler"] + ctx2.register_callback_prefix("em:", lambda data: "two") + assert registry["em:"]["handler"] is original + assert registry["em:"]["plugin"] == "p1" + + def test_same_plugin_may_rebind(self): + ctx, registry = _make_context() + ctx.register_callback_prefix("em:", lambda data: "one") + replacement = lambda data: "two" # noqa: E731 + ctx.register_callback_prefix("em:", replacement) + assert registry["em:"]["handler"] is replacement + + @pytest.mark.parametrize("bad", [None, "not-a-function", 42, object(), ["x"]]) + def test_non_callable_handler_rejected_at_registration(self, bad): + # A non-callable handler is a bug in the plugin, not a policy outcome: + # it raises here rather than failing after a user presses the button. + ctx, registry = _make_context() + with pytest.raises(ValueError, match="non-callable"): + ctx.register_callback_prefix("em:", bad) + assert registry == {} + + def test_non_callable_does_not_clobber_existing_registration(self): + ctx, registry = _make_context() + good = lambda data: "ok" # noqa: E731 + ctx.register_callback_prefix("em:", good) + with pytest.raises(ValueError): + ctx.register_callback_prefix("em:", None) + assert registry["em:"]["handler"] is good + + +# =========================================================================== +# run_plugin_callback_handler — off-loop execution and bounded completion +# =========================================================================== + +class TestRunPluginCallbackHandler: + @pytest.mark.asyncio + async def test_sync_handler_runs_off_the_event_loop(self): + # A blocking sync handler must not stall the caller's loop: while it + # blocks, the loop still has to service other coroutines. + started = threading.Event() + release = threading.Event() + thread_names = [] + + def handler(data): + thread_names.append(threading.current_thread().name) + started.set() + release.wait(timeout=5) + return "done" + + task = asyncio.create_task(run_plugin_callback_handler(handler, "em:x")) + started_at = time.monotonic() + for _ in range(200): + if started.is_set(): + break + await asyncio.sleep(0.01) + assert started.is_set(), "handler never started off-loop" + + # The discriminator: we are back on the loop *while* the handler is + # still blocked. Running it inline could only return control after the + # handler finished, so the task would already be done here. + assert not task.done(), "handler ran to completion on the event loop" + assert time.monotonic() - started_at < 2.0, "event loop was stalled" + + release.set() + assert await asyncio.wait_for(task, timeout=5) == "done" + # ...on the dedicated pool, not the process-wide default executor. + assert thread_names[0].startswith("hermes-plugin-callback") + + @pytest.mark.asyncio + async def test_async_handler_awaited_on_the_loop(self): + loop = asyncio.get_running_loop() + seen = {} + + async def handler(data): + seen["loop"] = asyncio.get_running_loop() + return "async ok" + + assert await run_plugin_callback_handler(handler, "em:x") == "async ok" + assert seen["loop"] is loop + + @pytest.mark.asyncio + async def test_sync_callable_returning_awaitable_is_resolved(self): + class AsyncCallable: + async def __call__(self, data): + return "resolved" + + assert await run_plugin_callback_handler(AsyncCallable(), "em:x") == "resolved" + + @pytest.mark.asyncio + async def test_slow_async_handler_is_bounded(self): + async def handler(data): + await asyncio.sleep(5) + return "too late" + + with pytest.raises(asyncio.TimeoutError): + await run_plugin_callback_handler(handler, "em:x", timeout=0.05) + + @pytest.mark.asyncio + async def test_slow_sync_handler_is_bounded(self): + release = threading.Event() + + def handler(data): + release.wait(timeout=5) + return "too late" + + try: + with pytest.raises(asyncio.TimeoutError): + await run_plugin_callback_handler(handler, "em:x", timeout=0.05) + finally: + release.set() # let the worker thread retire + + @pytest.mark.asyncio + async def test_handler_exception_propagates(self): + def handler(data): + raise RuntimeError("boom") + + with pytest.raises(RuntimeError, match="boom"): + await run_plugin_callback_handler(handler, "em:x") + + +# =========================================================================== +# Telegram adapter dispatch +# =========================================================================== + +def _patched_registry(entry_handler, prefix="em:", plugin="p1"): + entry = {"handler": entry_handler, "plugin": plugin, "description": ""} + + def lookup(data): + return (prefix, entry) if data.startswith(prefix) else None + + return patch( + "hermes_cli.plugins.get_plugin_callback_prefix", + side_effect=lookup, + ) + + +class TestTelegramPluginCallbackDispatch: + @pytest.mark.asyncio + async def test_authorized_press_dispatches_and_answers(self): + adapter = _make_adapter() + seen = [] + + def handler(data): + seen.append(data) + return "✅ archived" + + update, query = _make_query("em:approve:tg-7:archive") + with patch.dict(os.environ, {"TELEGRAM_ALLOWED_USERS": "*"}, clear=False): + with _patched_registry(handler): + await adapter._handle_callback_query(update, MagicMock()) + + assert seen == ["em:approve:tg-7:archive"] + query.answer.assert_awaited_once() + assert query.answer.call_args[1]["text"] == "✅ archived" + + @pytest.mark.asyncio + async def test_unauthorized_press_never_reaches_handler(self): + adapter = _make_adapter() + handler = MagicMock() + + update, query = _make_query("em:approve:tg-7:archive", user_id="777") + with patch.dict(os.environ, {"TELEGRAM_ALLOWED_USERS": "999"}, clear=False): + with _patched_registry(handler): + await adapter._handle_callback_query(update, MagicMock()) + + handler.assert_not_called() + query.answer.assert_awaited_once() + assert "not authorized" in query.answer.call_args[1]["text"] + + @pytest.mark.asyncio + async def test_async_handler_awaited(self): + adapter = _make_adapter() + + async def handler(data): + return "async ok" + + update, query = _make_query("em:dismiss:tg-9") + with patch.dict(os.environ, {"TELEGRAM_ALLOWED_USERS": "*"}, clear=False): + with _patched_registry(handler): + await adapter._handle_callback_query(update, MagicMock()) + + assert query.answer.call_args[1]["text"] == "async ok" + + @pytest.mark.asyncio + async def test_answer_is_bounded(self): + adapter = _make_adapter() + + update, query = _make_query("em:approve:tg-7:archive") + with patch.dict(os.environ, {"TELEGRAM_ALLOWED_USERS": "*"}, clear=False): + with _patched_registry(lambda data: "x" * 5000): + await adapter._handle_callback_query(update, MagicMock()) + + assert len(query.answer.call_args[1]["text"]) <= 180 + + @pytest.mark.asyncio + async def test_none_result_answers_done(self): + adapter = _make_adapter() + + update, query = _make_query("em:dismiss:tg-9") + with patch.dict(os.environ, {"TELEGRAM_ALLOWED_USERS": "*"}, clear=False): + with _patched_registry(lambda data: None): + await adapter._handle_callback_query(update, MagicMock()) + + assert query.answer.call_args[1]["text"] == "Done." + + @pytest.mark.asyncio + async def test_handler_exception_contained(self): + adapter = _make_adapter() + + def handler(data): + raise RuntimeError("secret internals") + + update, query = _make_query("em:approve:tg-7:archive") + with patch.dict(os.environ, {"TELEGRAM_ALLOWED_USERS": "*"}, clear=False): + with _patched_registry(handler): + await adapter._handle_callback_query(update, MagicMock()) + + text = query.answer.call_args[1]["text"] + assert text == "❌ Action failed." + assert "secret internals" not in text + + @pytest.mark.asyncio + async def test_unmatched_data_falls_through_silently(self): + adapter = _make_adapter() + handler = MagicMock() + + update, query = _make_query("zz:whatever") + with patch.dict(os.environ, {"TELEGRAM_ALLOWED_USERS": "*"}, clear=False): + with _patched_registry(handler): + await adapter._handle_callback_query(update, MagicMock()) + + handler.assert_not_called() + query.answer.assert_not_awaited() + + @pytest.mark.asyncio + async def test_dispatch_goes_through_the_bounded_runner(self): + # The adapter must never call the handler inline — that is what would + # put plugin code on Telegram's event loop. + adapter = _make_adapter() + handler = MagicMock(return_value="unused") + + update, query = _make_query("em:approve:tg-7:archive") + with patch.dict(os.environ, {"TELEGRAM_ALLOWED_USERS": "*"}, clear=False): + with _patched_registry(handler): + with patch( + "hermes_cli.plugins.run_plugin_callback_handler", + new=AsyncMock(return_value="✅ via runner"), + ) as runner: + await adapter._handle_callback_query(update, MagicMock()) + + handler.assert_not_called() + runner.assert_awaited_once_with(handler, "em:approve:tg-7:archive") + assert query.answer.call_args[1]["text"] == "✅ via runner" + + @pytest.mark.asyncio + async def test_timed_out_handler_answers_and_does_not_hang(self): + adapter = _make_adapter() + + update, query = _make_query("em:approve:tg-7:archive") + with patch.dict(os.environ, {"TELEGRAM_ALLOWED_USERS": "*"}, clear=False): + with _patched_registry(lambda data: "unused"): + with patch( + "hermes_cli.plugins.run_plugin_callback_handler", + new=AsyncMock(side_effect=asyncio.TimeoutError), + ): + await adapter._handle_callback_query(update, MagicMock()) + + query.answer.assert_awaited_once() + assert query.answer.call_args[1]["text"] == "⏳ Action timed out." + + @pytest.mark.asyncio + async def test_blocking_sync_handler_does_not_stall_the_adapter(self): + # End-to-end: a blocking handler still gets answered, and the loop + # stays responsive throughout the press. + adapter = _make_adapter() + release = threading.Event() + + def handler(data): + release.wait(timeout=5) + return "✅ eventually" + + update, query = _make_query("em:approve:tg-7:archive") + with patch.dict(os.environ, {"TELEGRAM_ALLOWED_USERS": "*"}, clear=False): + with _patched_registry(handler): + task = asyncio.create_task( + adapter._handle_callback_query(update, MagicMock()) + ) + await asyncio.sleep(0.05) + assert not task.done() + release.set() + await asyncio.wait_for(task, timeout=5) + + assert query.answer.call_args[1]["text"] == "✅ eventually" + + @pytest.mark.asyncio + async def test_builtin_prefix_wins_over_registry(self): + # A registry that would greedily match anything must never see a + # built-in prefix: the mp: branch returns before the plugin lookup. + adapter = _make_adapter() + handler = MagicMock() + + update, query = _make_query("mp:whatever") + with patch.dict(os.environ, {"TELEGRAM_ALLOWED_USERS": "*"}, clear=False): + with patch( + "hermes_cli.plugins.get_plugin_callback_prefix", + side_effect=lambda data: ("mp:", {"handler": handler, "plugin": "p1"}), + ): + await adapter._handle_callback_query(update, MagicMock()) + + handler.assert_not_called() diff --git a/website/docs/developer-guide/plugins/index.md b/website/docs/developer-guide/plugins/index.md index 7e748b6c1e1cf..1998cbc02eb57 100644 --- a/website/docs/developer-guide/plugins/index.md +++ b/website/docs/developer-guide/plugins/index.md @@ -911,6 +911,50 @@ def register(ctx): This is the public way for plugins to participate in Slack interactivity. Older plugins may patch `SlackAdapter.connect`; prefer this API instead. +### Handle inline-button clicks (callback prefixes) + +Plugins that post inline keyboards can claim a `callback_data` prefix and route their own button presses — no special-casing inside a platform adapter. + +```python +def register(ctx): + def _on_press(data: str) -> str: + # data is the full payload, prefix included: "em:approve:tg-7:archive" + parts = data.split(":") + if len(parts) != 4 or parts[1] != "approve": + return "❌ Rejected." # validate fail-closed, never guess + return "✅ Archived." + + ctx.register_callback_prefix("em:", _on_press, description="email approvals") +``` + +**Signature:** `ctx.register_callback_prefix(prefix, handler, description="") -> None` + +| Parameter | Type | Description | +|-----------|------|-------------| +| `prefix` | `str` | Lowercase `[a-z0-9_-]`, 2–16 chars, ending in `:` (e.g. `"em:"`) | +| `handler` | callable | `fn(data: str) -> str \| None`, sync or async. `data` is the full payload including the prefix; the return value becomes the button's answer | +| `description` | `str` | Optional human-readable label for diagnostics | + +**Payload contract:** + +- Your handler receives the payload **verbatim**, prefix included, and owns its grammar. Validate fail-closed — refuse anything malformed rather than guessing. +- Keep payloads small. Telegram caps `callback_data` at 64 bytes total, prefix included. +- The return value is stringified and truncated to 180 characters for the answer. Returning `None` answers `Done.` +- Treat the payload as **untrusted input**. It comes off the wire and a button can be pressed more than once — make handlers idempotent, and never build one that depends on the press being unique. + +**Authorization contract:** + +- The presser is checked against the **same** authorization gate as the built-in approval buttons *before* your handler runs. An unauthorized press is answered `⛔ You are not authorized to use this button.` and never reaches your code. +- Authorization says *who* pressed, not *what* they may do. Any further permission logic is yours. + +**Runtime behavior:** + +- Registration is fail-closed. A prefix that is malformed, collides with a built-in prefix, or is already claimed by another plugin is rejected with a warning and skipped; the plugin still loads. A **non-callable handler raises `ValueError`** at registration, like `register_slack_action_handler` — that is a bug in your plugin, not a policy outcome. +- A plugin may re-register its own prefix to swap the handler; it can never take one from another plugin. Built-in prefixes always win in the adapter. +- Sync handlers run on a worker thread, so a blocking handler cannot stall the gateway's update processing. Async handlers are awaited directly. +- The whole invocation is bounded at 15 seconds; on expiry the press is answered `⏳ Action timed out.` The bound cancels the *wait*, not a running sync handler — keep handlers short and push long work to the background. +- Handler exceptions are contained: the press is answered `❌ Action failed.` and the exception is logged, never relayed to the chat. + :::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. :::