Skip to content

feat(plugins): let plugins register inline-button callback prefixes - #71630

Closed
americodias wants to merge 2 commits into
NousResearch:mainfrom
americodias:feat/plugin-callback-prefixes
Closed

americodias wants to merge 2 commits into
NousResearch:mainfrom
americodias:feat/plugin-callback-prefixes

Conversation

@americodias

Copy link
Copy Markdown

What does this PR do?

Lets plugins register inline-button callback prefixes — ctx.register_callback_prefix(prefix, handler) — so a plugin can ship its own button flows without any special-casing in core. The Telegram adapter routes callback_data starting with a registered prefix to the plugin handler, after the same authorization check the built-in approval buttons (ea:) run, and bounds the handler's answer before it reaches Telegram.

This follows CONTRIBUTING's rule for third-party integrations: "If your plugin needs a capability the framework doesn't expose, that's a feature request to widen the generic plugin surface — never special-case your plugin in core." Supersedes #71616, which special-cased my email-approvals plugin in the adapter; with this API that plugin registers em: from its own tree and the core stays product-free.

Related Issue

Supersedes #71616 (same motivation, policy-conforming shape). No standalone issue exists.

Type of Change

  • ✨ New feature (non-breaking change that adds functionality)

Changes Made

  • hermes_cli/plugins.py
    • PluginContext.register_callback_prefix(prefix, handler, description="") — mirrors register_command; fail-closed validation: lowercase colon-terminated [a-z0-9_-] prefixes, 2–16 chars; rejects anything that shadows (or is shadowed by) a built-in prefix (mp:, mg:, cp:, gt:, ea:, sc:, cl:, update_prompt:, …); rejects stealing a prefix claimed by another plugin.
    • PluginManager._callback_prefixes registry (created, cleared on reload alongside _plugin_commands).
    • Module accessor get_plugin_callback_prefix(data) next to get_plugin_command_handler.
  • plugins/platforms/telegram/adapter.py
    • _handle_callback_query: plugin-prefix dispatch inserted after every built-in branch (built-ins always win) and before the legacy update_prompt: fallthrough. Runs _is_callback_user_authorized before the handler; supports sync and async handlers; caps the callback answer (180 chars); contains handler exceptions (generic "Action failed", nothing internal relayed).
  • tests/gateway/test_plugin_callback_prefixes.py — 30 tests: registration validation (shapes, reserved/shadowing, cross-plugin claims), authorized/unauthorized dispatch, async handlers, bounded/None answers, exception containment, silent fallthrough for unmatched data, built-in precedence.

Verification

  • pytest tests/gateway/test_plugin_callback_prefixes.py — 30 passed.
  • pytest tests/gateway/test_telegram_clarify_buttons.py tests/gateway/test_telegram_approval_buttons.py tests/gateway/test_telegram_auth_check.py tests/gateway/test_telegram_group_gating.py tests/hermes_cli/test_plugin*.py — 428 passed, 0 failed.

🤖 Generated with Claude Code

Adds PluginContext.register_callback_prefix(prefix, handler): platform
adapters route callback_data starting with a registered prefix to the
plugin handler — after the same authorization check as the built-in
approval buttons, with the answer bounded before it reaches the platform.

Registration is fail-closed: prefixes are lowercase colon-terminated
tokens, cannot shadow (or be shadowed by) any built-in prefix, and cannot
steal a prefix claimed by another plugin. Built-in branches always win in
the adapter; handler exceptions are contained and never relayed.

This widens the generic plugin surface per CONTRIBUTING ("never
special-case your plugin in core") so product plugins can ship their own
inline-button flows out of tree.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins platform/telegram Telegram bot adapter needs-decision Awaiting maintainer decision before any implementation sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Jul 25, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for converting the product-specific callback into a generic plugin surface. Current main still has no plugin callback dispatch path: plugins/platforms/telegram/adapter.py:6202-6543 only handles built-in callback prefixes.

Problems

  • plugins/platforms/telegram/adapter.py:6487-6489 calls synchronous plugin code on the Telegram event loop and awaits async plugin code with no deadline. A blocking handler can stall update processing; the built-in gmail callback bounds its wait at current-main plugins/platforms/telegram/adapter.py:6642-6649.
  • hermes_cli/plugins.py:597-643 accepts a non-callable handler and delays the failure until a user clicks a button. The analogous Slack API rejects non-callables at current-main hermes_cli/plugins.py:1043-1047.

Suggested changes

  • Offload synchronous handlers or make the API async-only, and bound async completion before answering the callback.
  • Validate callable(handler) at registration time.
  • Add public API documentation in website/docs/developer-guide/plugins/index.md, including the auth and payload contract.

This is an automated hermes-sweeper review.

Comment thread plugins/platforms/telegram/adapter.py Outdated
await query.answer(text="⛔ You are not authorized to use this button.")
return
try:
plugin_result = plugin_entry["handler"](data)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This invokes a synchronous plugin handler on Telegram's event loop, so one blocking handler can stall all callback processing. Please either make this API async-only or run synchronous handlers off-loop, and bound async completion before answering the callback.

Comment thread hermes_cli/plugins.py
self.manifest.name, prefix, existing.get("plugin"),
)
return
self._manager._callback_prefixes[clean] = {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please validate callable(handler) before storing this entry. The analogous register_slack_action_handler rejects non-callables at registration time; otherwise this only fails after an authorized user presses the button.

…llables

Review follow-up on the register_callback_prefix surface.

- register_callback_prefix() raises ValueError on a non-callable handler,
  matching register_slack_action_handler, instead of deferring the failure
  until an authorized 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 survives, not a bug in its own code.

- Plugin handlers no longer run on the platform's event loop. The new
  run_plugin_callback_handler() awaits async handlers and offloads sync ones
  to a small dedicated thread pool, so one blocking handler can no longer
  stall callback processing. The pool is separate from the default executor,
  so a wedged plugin starves only other callback handlers rather than every
  to_thread caller in the process.

- The whole invocation shares one 15s budget — tighter than the plugin-command
  bound because the platform holds the press open. The Telegram adapter
  answers "Action timed out." on expiry instead of waiting indefinitely.

- Document the API in website/docs/developer-guide/plugins/index.md, including
  the authorization and payload contracts.

Tests cover the registration raise, off-loop execution on the dedicated pool,
the sync and async timeout bounds, and the adapter's timeout answer. The
off-loop tests assert the loop regains control while the handler is still
blocked, so an inline implementation fails them rather than passing late.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks @americodias — this use case is covered by #59217 (34393c3): ctx.register_platform_handler("telegram", factory) gives plugins the full PTB surface, so a prefix-scoped CallbackQueryHandler(pattern="^myplugin:") achieves the same button flows with precedence over the core router. Closing as implemented on main; if the authorization-gated dispatch angle still matters for your plugin, happy to look at that as a follow-up on top of the new surface.

@teknium1 teknium1 closed this Aug 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins needs-decision Awaiting maintainer decision before any implementation 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-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants