From 08e8bedae82f98084deaa2fe15067e3e69d5fbd2 Mon Sep 17 00:00:00 2001 From: Brad Smith Date: Wed, 6 May 2026 09:20:34 -0500 Subject: [PATCH] fix(gateway): keep plugin action wrapper signature to (ack, body, action) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- gateway/platforms/slack.py | 20 +++++++++---- .../test_slack_plugin_action_handlers.py | 28 +++++++++++++++++++ 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/gateway/platforms/slack.py b/gateway/platforms/slack.py index 55b77c324de..90bb4481c6e 100644 --- a/gateway/platforms/slack.py +++ b/gateway/platforms/slack.py @@ -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, diff --git a/tests/gateway/test_slack_plugin_action_handlers.py b/tests/gateway/test_slack_plugin_action_handlers.py index 2aeba4e7af6..611446802b2 100644 --- a/tests/gateway/test_slack_plugin_action_handlers.py +++ b/tests/gateway/test_slack_plugin_action_handlers.py @@ -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.