Add explicit Assistant listener registration mode#1519
Conversation
ee9fb84 to
18fae4a
Compare
WilliamBergamin
left a comment
There was a problem hiding this comment.
Hi @Cr1stal thanks for your time working on this issue & interest in our project 💯
Unfortunately I can't accept these changes, they introduce more complexity to the already complex assistant logic, this increases the maintenance burden on our team 😅 and can slow down future feature implementation
The ideal solution to #1346 might be to make the assistant middleware a none global middleware, but this might require some server side changes
All this to say we'd prefer to solve #1346 by simplifying the the assistant logic rather then increasing it's complexity
|
@WilliamBergamin Thanks, that makes sense. I understand the main concern is not the goal of #1346, but that this PR adds a second Assistant dispatch path and listener ordering machinery, which increases maintenance cost. When you say the ideal solution might be making the Assistant middleware non-global, do you mean moving Assistant handling closer to the normal app listener pipeline while avoiding a new public registration mode and priority registry? Also, could you clarify what server-side change you have in mind? My current read is that assistant user messages are hard to distinguish from broad message listeners without either server-side classification or client-side ordering rules. |
Summary
Addresses #1346 by adding an explicit Assistant registration mode to
App.assistant()andAsyncApp.assistant().By default,
app.assistant(assistant)continues to behave as it does today: it registers Assistant as middleware. This preserves the existing Assistant middleware dispatch path and keepsapp.use(assistant)/app.middleware(assistant)behavior unchanged.Apps can now opt into listener-based registration:
In this mode, Assistant handlers are registered as normal App listeners. This lets assistant-specific handlers participate in the App listener pipeline and inherit app-level middleware such as authorization, auditing, rate limiting, or request enrichment.
The PR also adds an internal listener registry so Assistant listeners can be evaluated before broad catch-all listeners without exposing a general listener priority API. Registration order is preserved within Assistant listeners and ordinary App listeners, and sync/async behavior is kept mirrored.
Testing
./scripts/format.sh --no-install./scripts/lint.sh --no-install./scripts/run_tests.sh tests/slack_bolt/app/test_app_assistant_middleware.py(14 passed)./scripts/run_mypy.sh --no-install(Success: no issues found in 235 source files)./scripts/install_all_and_run_tests.sh(924 passed,81 warnings; mypy:Success: no issues found in 235 source files)Category
slack_bolt.Appand/or its core componentsslack_bolt.async_app.AsyncAppand/or its core componentsslack_bolt.adapter/docsRequirements
Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.
./scripts/install_all_and_run_tests.shafter making the changes.