Skip to content

feat(gateway): add pre_callback_query_dispatch plugin hook for Telegram inline keyboard callbacks - #21471

Open
ChaseFlorell wants to merge 1 commit into
NousResearch:mainfrom
ChaseFlorell:feat/pre-callback-query-dispatch-hook
Open

feat(gateway): add pre_callback_query_dispatch plugin hook for Telegram inline keyboard callbacks#21471
ChaseFlorell wants to merge 1 commit into
NousResearch:mainfrom
ChaseFlorell:feat/pre-callback-query-dispatch-hook

Conversation

@ChaseFlorell

Copy link
Copy Markdown

What does this PR do?

Adds a pre_callback_query_dispatch plugin hook that fires at the entry point of _handle_callback_query in TelegramAdapter, before any built-in prefix routing (mp:, ea:, sc:, etc.).

Plugins can register for this hook to intercept inline keyboard button clicks. Returning {"action": "skip"} claims the callback and suppresses all built-in handling. Returning {"action": "allow"} or None falls through to existing logic unchanged — so this is a non-breaking, purely additive change.

The motivation is to allow custom callback logic (e.g. a news-feedback plugin handling nf:yes:<id> / nf:no:<id>) to live in a plugin rather than in telegram.py directly. Without this hook, any customisation must be patched into core gateway files and is silently lost on hermes update.

Related Issue

Fixes #21469

Extends the pattern introduced in #21461 (forum topic events → pre_gateway_dispatch). That issue covered messages; this one covers button interactions, which cannot be represented as MessageEvent objects and therefore cannot go through pre_gateway_dispatch.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • hermes_cli/plugins.py: Added "pre_callback_query_dispatch" to VALID_HOOKS with a doc-comment describing the kwargs and supported return values.
  • gateway/platforms/telegram.py: Added _fire_plugin_hook("pre_callback_query_dispatch", ...) call at the top of _handle_callback_query, immediately after the data variable is set. Early-returns if a plugin returns {"action": "skip"}.
  • tests/gateway/test_telegram_callback_query_hook.py: 7 new tests covering: hook fires with correct kwargs, skip suppresses built-in handling, allow and None fall through, early-exit when query or query.data is absent, and pre_callback_query_dispatch present in VALID_HOOKS.

How to Test

  1. pytest tests/gateway/test_telegram_callback_query_hook.py -v — all 7 tests pass.
  2. Register a plugin hook for pre_callback_query_dispatch with a handler that returns {"action": "skip"} for a custom prefix; press an inline button with that prefix — the hook fires and built-in handling is suppressed.
  3. Press a built-in button (ea:, sc:, mp:) — hook fires but returns None; built-in logic proceeds normally.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15.5

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Fires a new `pre_callback_query_dispatch` hook at the entry point of
`_handle_callback_query` so plugins can intercept inline keyboard button
clicks before any built-in prefix handling runs.

A plugin returning `{"action": "skip"}` claims the callback and
suppresses all built-in handling. Returning `{"action": "allow"}` or
`None` falls through to the existing model-picker / approval / confirm
logic unchanged.

Relates to NousResearch#21461 (pre_gateway_dispatch for forum topic events).
@alt-glitch alt-glitch added type/feature New feature or request comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins platform/telegram Telegram bot adapter P3 Low — cosmetic, nice to have labels May 7, 2026
@sh940701

Copy link
Copy Markdown

Hi @ChaseFlorell — I opened #34539 earlier today not realizing this PR already existed (my fault: I searched for pre_callback_dispatch rather than callback_query). I've closed mine in favor of this one.

While I'm here, a few additive surface ideas from my implementation that may or may not be worth folding into your simpler hook — entirely up to you and the maintainer whether any of these are in-scope.

  1. rewrite action — alongside skip / allow, let a plugin return {"action": "rewrite", "data": "..."} to replace query.data before the built-in prefix branches see it. Cheap to add, useful for legacy-prefix translation or A/B routing.

  2. answer_text payload on skip — let {"action": "skip", "reason": "...", "answer_text": "Button retired — please reply instead"} carry a string. The adapter then awaits query.answer(text=answer_text) before returning, so a synchronous hook can both intercept the click AND show the user a toast without having to be async-aware. Today a plugin has to be async to call query.answer() itself.

  3. Gateway back-reference — a BasePlatformAdapter._gateway_ref (set via set_gateway_ref(self) from GatewayRunner after adapter creation) lets hooks fired from inside adapter callbacks reach the live GatewayRunner when they need it. Not strictly needed for the callback hook alone, but is the natural place to add it if other callback-style hooks land later (e.g. inline-query / chosen-inline-result hooks for the same reason).

  4. First-result-wins precedence with multiple hooks — if two plugins register, document that the first non-None result wins so plugins don't accidentally veto each other. Trivial to add, prevents head-scratching later.

  5. BaseException propagation — fail-open on Exception but let KeyboardInterrupt / SystemExit / asyncio.CancelledError propagate.

If any of these are useful, happy to send them as small follow-up PRs once this lands. If none are — totally fine, the simpler surface is also a defensible choice.

Reference (closed): #34539 with the full implementation of all five.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding a concrete plugin seam for Telegram inline callbacks. The interception point is still absent on current main, but this needs a rework against the current adapter and hook API.

Problems

  • gateway/platforms/telegram.py:1923 awaits self._fire_plugin_hook, but neither TelegramAdapter nor BasePlatformAdapter defines that method. Current dispatch is synchronous via hermes_cli/plugins.py:2047; this path would raise on every callback query.
  • PluginManager.invoke_hook() returns a list (hermes_cli/plugins.py:1890-1925), so the proposed single-result handling does not specify multiple-plugin precedence.
  • A skip returns without query.answer(), unlike built-in callback success paths (plugins/platforms/telegram/adapter.py:5363, :5427). Async plugin callbacks are not awaited by the current dispatcher.

Suggested changes

  • Port the hook to plugins/platforms/telegram/adapter.py::_handle_callback_query (Telegram moved there in 560010547) and use invoke_hook() synchronously.
  • Apply the existing pre_gateway_dispatch first-recognized-action model from gateway/run.py:8905-8924, define callback acknowledgement behavior, and test the real dispatcher rather than a synthetic _fire_plugin_hook attribute.
  • Add the new behavior-changing hook to website/docs/user-guide/features/hooks.md.

Automated hermes-sweeper review.

data = query.data
query_message = getattr(query, "message", None)

hook_result = await self._fire_plugin_hook(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_fire_plugin_hook is not defined by TelegramAdapter or BasePlatformAdapter; the current plugin API is synchronous hermes_cli.plugins.invoke_hook() (hermes_cli/plugins.py:2047) and returns a list. As written, every non-empty callback query will raise AttributeError; port this to the current adapter and aggregate returned action dicts explicitly.

message_id=str(query_message.message_id) if query_message else None,
raw_query=query,
)
if hook_result and hook_result.get("action") == "skip":

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please define the acknowledgement contract for skip. This returns without query.answer(), whereas built-in successful callback paths answer the query. The plugin manager invokes callbacks synchronously (hermes_cli/plugins.py:1913-1925), so plugins cannot rely on an async callback being awaited to acknowledge it.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
@cresslank

Copy link
Copy Markdown
Contributor

I have a concrete dynamic callback consumer that currently has to monkey-patch plugins.platforms.telegram.adapter.TelegramAdapter._handle_callback_query because there is no supported interception point.

Its callback payload is cd:<decision_id>:<choice>. The handler validates the bounded id and choice, applies the adapter's callback authorization check, acknowledges the query, and records/enqueues the decision through a deterministic runner. It deliberately does not turn the click into an LLM message. The plugin also has to signature-check the private adapter method and expose compatibility health because any adapter refactor can break the patch.

That gives me four requirements for a useful hook contract:

  1. fire from the current plugin Telegram adapter, not the retired gateway adapter;
  2. preserve the raw query plus user/chat/message/thread context;
  3. define first-recognized-result precedence when several plugins register; and
  4. make callback acknowledgement ownership explicit, so a consumed callback cannot leave Telegram's spinner running or be acknowledged twice.

A first-recognized consume/skip action is enough for this consumer; it does not need rewrite support or a gateway back-reference. Config-driven callback-to-text routes such as #43949 are complementary, but cannot replace this hook for dynamic ids and non-conversational side effects.

So the feature remains useful on current main, but I agree with the existing review that it should be rebuilt around the real invoke_hook() API and plugins/platforms/telegram/adapter.py, with acknowledgement and multiple-plugin behavior tested explicitly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have platform/telegram Telegram bot adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Add pre_callback_query_dispatch plugin hook for Telegram inline keyboard callbacks

5 participants