mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-12 08:51:53 +00:00
fix(gateway): keep plugin action wrapper signature to (ack, body, action)
The previous implementation captured loop vars via default arguments::
async def _wrapped(ack, body, action, _cb=_cb, _plugin_name=_plugin_name):
slack_bolt's ``kwargs_injection`` introspects each listener's signature
via ``inspect.signature`` and passes ``None`` for any parameter name it
doesn't recognise (see ``slack_bolt/kwargs_injection/async_utils.py``
``build_async_required_kwargs``). That clobbered ``_cb`` to ``None`` at
dispatch time, so the wrapped plugin handler became ``NoneType`` —
``await _cb(...)`` then raised ``'NoneType' object is not callable`` and
no plugin action handler ever fired.
Replace the default-arg trick with a small closure factory so the
wrapper's public signature is exactly ``(ack, body, action)``. Add a
regression test that introspects the wrapped function's signature.
Found via real Slack click on a Block Kit button registered through
``ctx.register_slack_action_handler`` — gateway log showed
``[Slack] Plugin 'None' action handler raised: 'NoneType' object is
not callable`` despite the registration log line confirming the
handler was wired.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
62e937bf2b
commit
08e8bedae8
2 changed files with 42 additions and 6 deletions
|
|
@ -968,22 +968,30 @@ class SlackAdapter(BasePlatformAdapter):
|
|||
)
|
||||
_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):
|
||||
# Closure factory — keeps the wrapper's signature limited to
|
||||
# ``(ack, body, action)``. slack_bolt inspects listener
|
||||
# signatures via ``inspect.signature`` and passes ``None`` for
|
||||
# any parameter name it doesn't recognise, so capturing loop
|
||||
# vars as default args (``_cb=_cb`` etc.) silently clobbers
|
||||
# them at dispatch time.
|
||||
def _make_wrapper(cb, plugin_name):
|
||||
async def _wrapped(ack, body, action):
|
||||
try:
|
||||
await _cb(ack, body, action)
|
||||
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,
|
||||
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)
|
||||
return _wrapped
|
||||
|
||||
for _action_id, _cb, _plugin_name in _plugin_handlers:
|
||||
self._app.action(_action_id)(_make_wrapper(_cb, _plugin_name))
|
||||
logger.debug(
|
||||
"[Slack] Registered plugin action handler %s (from %s)",
|
||||
_action_id, _plugin_name,
|
||||
|
|
|
|||
|
|
@ -343,6 +343,34 @@ class TestSlackAdapterPluginActionWiring:
|
|||
assert seen["body"] == {"b": 1}
|
||||
assert seen["action"] == {"action_id": "approve_x"}
|
||||
|
||||
def test_wrapper_signature_only_exposes_slack_bolt_args(self):
|
||||
"""Regression: slack_bolt introspects listener signatures and passes
|
||||
``None`` for any parameter name it doesn't recognise. If the wrapper
|
||||
leaks closure variables (e.g. ``_cb``, ``_plugin_name``) into its
|
||||
signature via default args, they get clobbered to None at dispatch
|
||||
time and the wrapped callback becomes ``NoneType``.
|
||||
|
||||
The wrapper must only expose ``(ack, body, action)``.
|
||||
"""
|
||||
import inspect
|
||||
|
||||
config = PlatformConfig(enabled=True, token="xoxb-fake")
|
||||
adapter = SlackAdapter(config)
|
||||
|
||||
async def cb(ack, body, action): # pragma: no cover
|
||||
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")
|
||||
params = list(inspect.signature(wrapped).parameters)
|
||||
assert params == ["ack", "body", "action"], (
|
||||
f"wrapper exposes extra params slack_bolt would clobber: {params}"
|
||||
)
|
||||
|
||||
def test_plugin_loader_failure_does_not_break_connect(self):
|
||||
"""If get_plugin_manager() blows up, connect() must still succeed.
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue